Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

numbers test failure on ARM #10124

Closed
ViralBShah opened this issue Feb 8, 2015 · 31 comments
Closed

numbers test failure on ARM #10124

ViralBShah opened this issue Feb 8, 2015 · 31 comments
Labels
system:arm ARMv7 and AArch64

Comments

@ViralBShah
Copy link
Member

On x86:

julia> typemax(UInt64) != 2.0 ^ 64
true

On ARM:

julia> typemax(UInt64) != 2.0 ^ 64
false
@ViralBShah ViralBShah added the system:arm ARMv7 and AArch64 label Feb 8, 2015
@ViralBShah
Copy link
Member Author

@code_typed and @code_llvm are the same on both. Here is @code_native typemax(UInt64) != 2.0 ^ 64 on ARM. LLVM version is 3.5:

julia> @code_native typemax(UInt64) != 2.0 ^ 64
    .text
Filename: operators.jl
<MCInst 407 <MCOperand Reg:12> <MCOperand Reg:12> <MCOperand Imm:14> <MCOperand Reg:0> <MCOperand Reg:70> <MCOperand Reg:71> <MCOperand Reg:77> <MCOperand Reg:10>>
<MCInst 28 <MCOperand Reg:77> <MCOperand Reg:12> <MCOperand Imm:8> <MCOperand Imm:14> <MCOperand Reg:0> <MCOperand Reg:0>>
<MCInst 2171 <MCOperand Reg:12> <MCOperand Reg:12> <MCOperand Imm:14> <MCOperand Reg:0> <MCOperand Reg:22> <MCOperand Reg:23>>
<MCInst 223 <MCOperand Reg:70> <MCOperand Reg:67> <MCOperand Imm:14> <MCOperand Reg:0> <MCOperand Reg:0>>
<MCInst 223 <MCOperand Reg:71> <MCOperand Reg:66> <MCOperand Imm:14> <MCOperand Reg:0> <MCOperand Reg:0>>
<MCInst 1342 <MCOperand Reg:22> <MCOperand Reg:14> <MCOperand Imm:14> <MCOperand Reg:0>>
<MCInst 55 <MCOperand Expr:(80)> <MCOperand Imm:14> <MCOperand Reg:0>>
<MCInst 1344 <MCOperand Reg:23> <MCOperand Reg:66> <MCOperand Reg:67> <MCOperand Imm:14> <MCOperand Reg:0>>
<MCInst 55 <MCOperand Expr:(88)> <MCOperand Imm:14> <MCOperand Reg:0>>
<MCInst 698 <MCOperand Reg:23> <MCOperand Reg:22> <MCOperand Imm:14> <MCOperand Reg:0>>
<MCInst 98 <MCOperand Reg:67> <MCOperand Reg:67> <MCOperand Reg:70> <MCOperand Imm:14> <MCOperand Reg:0> <MCOperand Reg:0>>
<MCInst 98 <MCOperand Reg:66> <MCOperand Reg:66> <MCOperand Reg:71> <MCOperand Imm:14> <MCOperand Reg:0> <MCOperand Reg:0>>
<MCInst 245 <MCOperand Reg:66> <MCOperand Reg:66> <MCOperand Reg:67> <MCOperand Imm:14> <MCOperand Reg:0> <MCOperand Reg:3>>
<MCInst 219 <MCOperand Reg:67> <MCOperand Imm:0> <MCOperand Imm:14> <MCOperand Reg:0> <MCOperand Reg:0>>
<MCInst 220 <MCOperand Reg:66> <MCOperand Imm:1> <MCOperand Imm:1> <MCOperand Reg:3>>
<MCInst 106 <MCOperand Imm:14> <MCOperand Reg:0>>
<MCInst 220 <MCOperand Reg:67> <MCOperand Imm:1> <MCOperand Imm:1> <MCOperand Reg:3>>
<MCInst 245 <MCOperand Reg:66> <MCOperand Reg:66> <MCOperand Reg:67> <MCOperand Imm:14> <MCOperand Reg:0> <MCOperand Reg:0>>
<MCInst 1247 <MCOperand Reg:12> <MCOperand Reg:12> <MCOperand Imm:14> <MCOperand Reg:0> <MCOperand Reg:22> <MCOperand Reg:23>>
<MCInst 147 <MCOperand Reg:12> <MCOperand Reg:12> <MCOperand Imm:14> <MCOperand Reg:0> <MCOperand Reg:70> <MCOperand Reg:71> <MCOperand Reg:77> <MCOperand Reg:11>>

@JeffBezanson
Copy link
Sponsor Member

Looks like LLVM's ARM assembly printing needs work?

@ViralBShah
Copy link
Member Author

Perhaps LLVM 3.6 fixed it.

@ihnorton noted a lowering bug here in #8402.

@ViralBShah
Copy link
Member Author

Interestingly, the problem seems to only be with unsigned ints:

julia> typemax(Int64) == 2.0^64
false # Correct

julia> typemax(UInt64) == 2.0^64
true # Wrong

@ViralBShah ViralBShah added the priority This should be addressed urgently label Mar 14, 2015
@ViralBShah
Copy link
Member Author

I think this is the highest priority ARM bug, and if we can fix this, we may actually have a usable Julia on arm. There are various other issues, but they are not blockers like this one.

@physicsd00d
Copy link

I'm having this bug right now too. Also, the problem is for Int64 as well right? typemax of Int64 should be (2^63 -1), and i get

# Good
julia> typemax(Int16) == (2^15 -1)
true

julia> typemax(Int32) == (2^31 -1)
true

# Bad
julia> typemax(Int64) == (2^63 -1)
false

@StefanKarpinski
Copy link
Sponsor Member

Is ARM a 32-bit system? On 32-bit systems 2^63-1 is -1.

@pao
Copy link
Member

pao commented Apr 24, 2015

Is ARM a 32-bit system?

Everything up to and including ARMv7 are. I think I'm the only one working on ARMv8 (AArch64) right now (and I'm stuck in an LLVM assert).

@StefanKarpinski
Copy link
Sponsor Member

Then typemax(Int64) != (2^63 -1) == -1 is expected.

@physicsd00d
Copy link

@StefanKarpinski
I believe almost all ARM systems are 32-bit; mine definitely is. I agree about the right-hand side, but my confusion is how typemax(Int64) isn't also -1.

julia> typemax(Int64)
9223372036854775807

julia> typeof(typemax(Int64))
Int64

So 1.) I'm not sure how julia is representing that number on a 32-bit system and 2.) why I'm even allowed to mess with 64-bit types on a 32-bit build.

In playing around, and assuming that it's okay to emulate 64-bit types on a 32-bit machine, I think now that the problem is in the syntax of the tests and is perhaps not an actual bug in the code? The test typemax(UInt64) != 2.0 ^ 64 that @ViralBShah started with assumes that it's being run on a 64-bit system. If the default types are actually 32-bit, then this will fail because the LHS is 64-bit and the RHS is 32-bit. So simply telling julia to expect a 64-bit works.

julia> typemax(UInt64) != UInt64(2) ^ 64
true

julia> typemax(Int64) == Int64(2)^63-1
true

Some follow-ups. I wonder why there are bit-agnostic types Int and UInt, but no type Float? One must specify Float32 or Float64. I wonder why I'm even allowed to use 64-bit types on this build. Is there a way to disallow julia from trying to use 64-bit data types on 32-bit systems?

Also, Float64 seems to be the default type even on a 32-bit machine.

julia> typeof(float(3))
Float64

@pao
Copy link
Member

pao commented Apr 24, 2015

So 1.) I'm not sure how julia is representing that number on a 32-bit system and 2.) why I'm even allowed to mess with 64-bit types on a 32-bit build.

They can handle 64-bit types just fine, you just have to be explicit about it because Julia's default integer type is a "machine integer". Int and UInt are not bit-agnostic, they are "please give me an integer whose size is appropriate for the system"; on a 32-bit system these will be Int32 and UInt32.

Float64 is always the default floating point type because floating point registers work differently than integer registers on the CPU, and single-precision floating point is its own can of worms.

FURTHER CLARIFICATION: The number of "bits" of a CPU is first and foremost about the size of pointers--that is, how much memory (physical+virtual) the system is capable of addressing. This is often (almost always? I'm not entirely sure) further reflected in the size of the CPU's registers. A consequence is that representing a 64-bit integer requires loading two registers and performing instructions on the both of them, which is slower than working with 32-bit integers. So that's why Julia makes that the default. But "emulating" a 64-bit type is a bit of a strong term for it; it's just that more operations are required.

For floating point, the traditional x87 instructions work with both single- and double-precision floating point values, and more recently the extensions made available in SSE and its extensions, and AVX all work with extremely wide registers (128 bits or more) and can address all or part of the register simultaneously, even when the processor is 32-bit. ARM is a bit different: in 32-bit ARM processors with hardware floating-point, there is a scalar unit, confusingly called the "Vector Floating Point" unit or VFP, which can handle both single- and double-precision arithmetic, and a vector unit, which can handle vectorized single-precision only arithmetic using the NEON instruction set. I'm not sure how eager LLVM's JIT is to emit NEON instructions.

(Any errors in the above are my own. Corrections are welcomed.)

@ihnorton
Copy link
Member

Related: this comment.

@StefanKarpinski
Copy link
Sponsor Member

System word size has nothing to do with what size floating-point numbers to use – the 8087 already had 64-bit IEEE floating-point registers.

@pao
Copy link
Member

pao commented Apr 24, 2015

(I added more details in my comment above, which gets into more detail on floating-point handling on x86 vs. 32-bit ARM. I haven't looked to see what's new for floating point in AArch64.)

@vtjnash
Copy link
Sponsor Member

vtjnash commented Apr 25, 2015

julia> typemax(UInt64) != 2.0 ^ 64
false

this was an intended change, we are just seeing the new behavior show up first on ARM since on x86_64 we are just getting UB from a processor intrinsic (http://reviews.llvm.org/D2804#75076)

@daanhb
Copy link
Contributor

daanhb commented May 15, 2015

Chasing failing tests of rationalize on ARM (Raspberry Pi 2) led to this observation:

julia> convert(Int16, 88776.0)
23240

This throws an InexactError on my x86_64 machine. It seems like it does modulo instead since:

julia> 88776-65536
23240

Is this a related (intended) change, or is this a separate issue?

@ViralBShah
Copy link
Member Author

This is a good place to capture all these, unless we believe the underlying causes are different.

@JeffBezanson JeffBezanson removed the priority This should be addressed urgently label Jul 10, 2015
@ViralBShah
Copy link
Member Author

@vtjnash Is this the behaviour we want? Does this mean the test should change - or perhaps commented out for ARM?

@daanhb Revisiting this issue, I think we want a different issue for rationalize.

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jul 19, 2015

no, we need to fix our code to stop relying on undefined behavior of the gcc 4.x x86 compiler.

@ViralBShah
Copy link
Member Author

As @ihnorton noted in the original issue, wrapping this test in a function does give the correct result.

julia> f() = typemax(UInt64) == 2.0^64
f (generic function with 1 method)

julia> f()
false

julia> typemax(UInt64) == 2.0^64
true

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jul 26, 2015

That's the trouble with UB: it's even allowed to return the expected answer at times.

@daanhb
Copy link
Contributor

daanhb commented Jul 28, 2015

@ViralBShah Thanks, I'll follow-up on the tests with rationalize in a separate issue (but right now I have no access to my Pi, on vacation)

@PallHaraldsson
Copy link
Contributor

"A consequence is that representing a 64-bit integer requires loading two registers and performing instructions on the both of them, which is slower than working with 32-bit integers. So that's why Julia makes that the default."

Note also, while 64-bit would not be necessary for pointers, 64-bit for that and some other things would be dangerous as when you do use two instructions, access is no longer atomic. I would rely on that for stuff I'm thinking up.. Because I'm thinking of when Julia will have threads. Until then 64-bit types will look atomic to you.

@nkottary
Copy link
Contributor

I followed the code with @edit and saw the return value of functions called, side by side, on an Arm machine and an x86 machine. == is defined as

function ==(x::$Tf, y::$Ti)
    fy = ($Tf)(y)
    (x == fy) & (y == unsafe_trunc($Ti,fy))
end

Where Tf is Float64 and Ti is UInt64. fy here is 1.8446744073709552e19 on both machines. The part that is different is (y == unsafe_trunc($Ti,fy). On x86 unsafe_trunc returns 0, which is wrong but happens to be not equal to y (which is 18446744073709551615), so we get false as the result. On Arm it is equal.

unsafe_trunc is defined as:

unsafe_trunc(::Type{$Ti}, x::Float64) = box($Ti,fptoui($Ti,unbox(Float64,x)))

The documentation for fptoui says that the return value is undefined if it cannot convert the float value to unsigned int. The float value given above cannot be converted to UInt64 since it is bigger than typemax(UInt64).

So maybe we need to check the floating point value for being representable as a UInt64 before we call fptoui.

@vtjnash
Copy link
Sponsor Member

vtjnash commented Nov 22, 2015

that is a good summary. I would only add that the challenge has been in attempting to make this test type-generic. for IEEE float to signed twos-complement integer, I think we can use the test from the libgcc / libsupport_rt libraries http://reviews.llvm.org/diffusion/L/browse/compiler-rt/trunk/lib/builtins/fp_fixint_impl.inc$1 of fp.exponent >= 0 && fp.exponent < nbits(Ti). but for unsigned integers, the test is a bit different (from http://reviews.llvm.org/diffusion/L/browse/compiler-rt/trunk/lib/builtins/fp_fixuint_impl.inc$1) of fp.sign > 0 && fp.exponent <= nbits(Ti)

@ViralBShah
Copy link
Member Author

Cc @StefanKarpinski

@nkottary
Copy link
Contributor

I have written some code according to what @vtjnash said.

https://gist.github.com/nkottary/a53749c02697e5434578

I haven't tested it on ARM yet, but it fixes the issue in x86.

@simonbyrne
Copy link
Contributor

I think the following should be sufficient:

function ==(x::$Tf, y::$Ti)
    fy = ($Tf)(y)
    (x == fy) & (fy != $(Tf(typemax(Ti)))) & (unsafe_trunc($Ti,fy) == y)
end

@eschnett
Copy link
Contributor

Sometimes I wish that code like this was accompanied by a comment explaining the reason behind it. Such as:

  • don't need to check typemin(Ti) because it can be represented exactly both as Tf and Ti, whereas typemax(Ti) cannot
  • Tf(typemax(Ti)) will round up, hence cannot be represented as Ti any more, hence the term fy != ... won't exclude anything valid
  • y == typemax(Ti) cannot be equal to any value represented by Tf, given that the loops surrounding this code don't include any integer type smaller than Int64 nor any floating type larger than Float64 (thus be careful when adding a Float128 to Base)

I like terse code as much as anybody else, but there's a line when things cross over to magic. Please leave some breadcrumbs for those who joined the Julia project after you.

@simonbyrne
Copy link
Contributor

@eschnett Good point, I was just being lazy. Here's my attempt at writing up the logic:

Suppose that Ti is a either a base-2 unsigned, or signed, twos-complement integer, with typemax(Ti) == 2^n-1, and Tf some binary floating point type such that (1) typemax(Ti) cannot be represented, and (2) typemin(Ti) can be exactly represented (i.e. doesn't overflow to +/-Inf).

Comparing as floats is easy, but may result in some false positives, as fx = Tf(x) may be rounded to fit in the floating point type. So we check for this by converting fx back to Ti, via unsafe_trunc to check if any rounding occurred.

The problem arises when fx == 2^n (or fx == Inf), as these cannot be represented by Ti: this is what the new part in the middle is checking, by comparison with Tf(typemax(Ti))/

@eschnett
Copy link
Contributor

@simonbyrne Looks good, except the expression "new part in the middle": Readers of the code won't know which part is new; just "part in the middle" suffices.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
system:arm ARMv7 and AArch64
Projects
None yet
Development

No branches or pull requests