-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
f128
from_bits
/ to_bits
sometimes gets reversed on ppc
#125102
Comments
@rustbot label +F-f16_and_f128 +A-ABI +T-libs +C-bug -needs-triage +A-llvm |
from_bits
/ to_bits
may not work on ppcf128
transmute for from_bits
/ to_bits
may not work on ppc
If we define f128 to be the corresponding IEEE float format, then the transmute is by definition correct. That float format defines the representation in memory and I think it's endianess must work out exactly the same as u128? Cc @eddyb @jcranmer @Muon So at first this looks like a platform bug to me. Certainly we'd have to change a lot more than just from_bits / to_bits if this was deemed correct behavior -- codegen and Miri also need to know how to convert between the raw bits an a floating-point value. Do LLVM optimizations do anything to handle this? |
Are you sure this is not an ABI bug, a mismatch between caller and callee in how the arguments are interpreted? That seems more likely to me than an in-memory float format with mixed endianess. Does it make any difference if you remove the #![feature(f128)]
#[no_mangle]
#[inline(never)]
fn add_entry(a: f128, b: f128) -> f128 {
a + b
}
union U {
i: u128,
f: f128,
}
fn main() { unsafe {
let a = U { i: 0x0 };
let b = U { i: 0x1 };
dbg!(a.f, b.f);
let c = add_entry(a.f, b.f);
dbg!(c);
} } |
Yeah there is definitely something more going on here, setting opt-level=3 makes the problem go away |
f128
transmute for from_bits
/ to_bits
may not work on ppcf128
from_bits
/ to_bits
sometimes gets reversed on ppc
Slightly reduced
Any idea how to narrow further? |
Your suggestion does indeed fix the issue:
edit: but turn on -O and it’s borked again. Interesting. |
I'd define |
IEEE 754 does not specify memory layout. It just gives interpretations for 128-bit integers, but nothing prevents hardware from having, say, little-endian integer arithmetic and big-endian floating-point arithmetic. That said, AFAICT, PowerPC is big-endian for everything. More relevantly, however, PowerPC does not have 128-bit floating-point support, so this is being emulated in software anyway. |
Correction: Power9 does in fact have native IEEE 754 binary128 support, as part of VSX, according to its architecture manual. And indeed it is big endian. Older PowerPC doesn't, but I did see that the compilation command line did request Power9. |
In fact maybe it's all an optimizer bug; |
@ecnelises or @lu-zero any chance you have any ideas here? It is looking like this has to be a bug in LLVM, just don't know exactly what. |
I saw this message from Qiu Chaofan on Zulip, and thought I'd test whether the memory representation of the smallest positive f128 differs from the representation of 1 as a u128. I got an ICE due to unimplemented stuff in const eval in Rust, so I had to resort to C (gcc, clang). However, there were no differences in the representation. This isn't conclusive yet since the two groups of 4 bytes in the middle could still be swapped around, so I did another test with no repeats there (gcc, clang). Again, no differences. It seems that both GCC and Clang assume that u128 is big endian, just like f128, even if the loads and stores in that case are conducted as two lots of 64 bits. |
But that should be an implementation detail, right, and not observable? When the two halves are strung together to form f128, endianess needs to be taken into account properly. I think the best starting point is figuring out why the optimizer breaks #![feature(f128)]
union U {
i: u128,
f: f128,
}
fn main() {
let a = U { i: 0x1 };
dbg!(unsafe { a.f });
} I see no way that's not an LLVM bug, so it might be worth reporting there (after some further minimization to remove the debug machinery from the equation). |
Yes, that's right. I was just checking whether u128 was stored with the lower 64 bits before the higher 64 bits. That would mess it up and look mixed endian. But that's not the case. That is, u128 is stored big endian, so it's definitely an LLVM bug. |
Something must be quite wonky on the LLVM side here, accidentally tried to compile the same thing for 32-bit ppc with the pwr9 target-cpu and it gives me a SIGILL. Not sure if pwr9 is even valid or not but an error message would have been nice :) $ rustc transmute.rs --target powerpc-unknown-linux-gnu -C linker=powerpc-linux-gnu-gcc -Ctarget-cpu=pwr9 -o transmute.rs.ppc.pwr9
Illegal instruction For reference:
bt via gdb:
Edit: reported this one at llvm/llvm-project#92233 |
I at least was able to reduce the original issue enough to remove |
llvm/llvm-project#95931 was merged, which should fix this. |
We should get this with the next LLVM19 bump (after the ongoing rc3 bump) if llvm/llvm-project#105623 merges. |
This was fixed by #130212. |
It seems like using transmute causes the upper and lower 64-bit halves to be loaded reversed. More at https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/f128.20system.20libraries.20noncompliant.20platforms/near/438489330
Espected
b
to print as0x00000000000000000000000000000001
, but withrustc add_test.rs -o add_test.rust --target powerpc64-unknown-linux-gnu -Clinker=powerpc64-linux-gnu-gcc -Ctarget-cpu=pwr9
it instead prints0x00000000000000010000000000000000
.The text was updated successfully, but these errors were encountered: