Use -0.0 in intrinsics::simd::reduce_add_unordered
#130325
Conversation
|
@bors try |
…, r=<try> Use -0.0 in `intrinsics::simd::reduce_add_unordered` -0.0 is the actual neutral additive float, not +0.0, and this matters to codegen. try-job: aarch64-gnu
This comment was marked as resolved.
This comment was marked as resolved.
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
fdfb822 to
f23173b
Compare
|
@bors try |
This comment was marked as resolved.
This comment was marked as resolved.
…, r=<try> Use -0.0 in `intrinsics::simd::reduce_add_unordered` -0.0 is the actual neutral additive float, not +0.0, and this matters to codegen. try-job: aarch64-gnu
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
f23173b to
9cb8b36
Compare
|
@bors try |
…, r=<try> Use -0.0 in `intrinsics::simd::reduce_add_unordered` -0.0 is the actual neutral additive float, not +0.0, and this matters to codegen. try-job: aarch64-gnu
|
☀️ Try build successful - checks-actions |
99ab363 to
3e350fb
Compare
|
@bors try |
This comment was marked as resolved.
This comment was marked as resolved.
…, r=<try> Use -0.0 in `intrinsics::simd::reduce_add_unordered` -0.0 is the actual neutral additive float, not +0.0, and this matters to codegen. try-job: aarch64-gnu
This comment has been minimized.
This comment has been minimized.
…ux, r=RalfJung,jieyouxu Use -0.0 in `intrinsics::simd::reduce_add_unordered` -0.0 is the actual neutral additive float, not +0.0, and this matters to codegen. try-job: aarch64-gnu
…ux, r=RalfJung,jieyouxu Use -0.0 in `intrinsics::simd::reduce_add_unordered` -0.0 is the actual neutral additive float, not +0.0, and this matters to codegen. try-job: aarch64-gnu
…ux, r=RalfJung,jieyouxu Use -0.0 in `intrinsics::simd::reduce_add_unordered` -0.0 is the actual neutral additive float, not +0.0, and this matters to codegen. try-job: aarch64-gnu
…, r=RalfJung,jieyouxu Use -0.0 in `intrinsics::simd::reduce_add_unordered` -0.0 is the actual neutral additive float, not +0.0, and this matters to codegen. try-job: aarch64-gnu
|
@bors retry |
…kingjubilee Rollup of 3 pull requests Successful merges: - rust-lang#130295 (Fix target-cpu fpu features on Armv8-R.) - rust-lang#130325 (Use -0.0 in `intrinsics::simd::reduce_add_unordered`) - rust-lang#130371 (Correctly account for niche-optimized tags in rustc_transmute) r? `@ghost` `@rustbot` modify labels: rollup
-0.0 is the actual neutral additive float, not +0.0, and this matters to codegen.
914ccea to
ab8c202
Compare
|
So Windows now implies SSE3, and so does Apple, so I'm just going to remove the doubled-up x86 tests and just set it to SSE3, because the instructions look almost the same as the AVX set, they just aren't VEX-encoded. |
|
Makes sense! |
…kingjubilee Rollup of 5 pull requests Successful merges: - rust-lang#127879 (Document futility of printing temporary pointers) - rust-lang#130325 (Use -0.0 in `intrinsics::simd::reduce_add_unordered`) - rust-lang#130336 (simplify `Build::update_existing_submodule`) - rust-lang#130398 (Add system libs for LLVM when cross compiling for Windows) - rust-lang#130420 (Register tool docs for `src/tools/build_helper`) r? `@ghost` `@rustbot` modify labels: rollup
| // aarch64-NEXT: faddp | ||
|
|
||
| // CHECK-NOT: {{f?}}add{{p?s*}} | ||
| // CHECK: ret |
There was a problem hiding this comment.
In order to mitigate LVI vulnerabilities, ret instructions are rewritten as popq %rax; lfence; jmpq *rax on the x86_64-fortanix-unknown-sgx target. So this test currently fails on this platform. Can this test be ignored for the SGX target, please?
There was a problem hiding this comment.
This sort of thing is a constant issue with the SGX target. Please PR compiletest with an appropriate modification that handles this issue globally without having to modify each and every single test with "oh yeah, and SGX is special, as usual".
There was a problem hiding this comment.
I realize I have probably indulged you significantly in the past and I wish to be clear, I do appreciate that Fortanix actually runs the tests in their CI, unlike some, but I must at this point refer to the target tier policy:
Tier 2 targets must not impose burden on the authors of pull requests, or other developers in the community, to ensure that tests pass for the target. In particular, do not post comments (automated or manual) on a PR that derail or suggest a block on the PR based on tests failing for the target. Do not send automated messages or notifications (via any medium, including via @) to a PR author or others involved with a PR regarding the PR breaking tests on a tier 2 target, unless they have opted into such messages.
Like we really need proper turnkey cross-compile testing support per #130375 or even just an SGX exception built in to compiletest or something, twiddling every single test isn't really sustainable for you or for me.
There was a problem hiding this comment.
I don't really understand what the appropriate compiletest modification here would be -- unless you are suggesting to skip all assembly tests on SGX?
Assembly tests in general are very finicky, you're lucky if they merge in less than 3 cycles, for one reason or another.
There was a problem hiding this comment.
A flat skip wouldn't work for them, a nonzero number of assembly tests are specifically for SGX-related codegen.
There was a problem hiding this comment.
Probably, yeah, and that requirement has to go away, which is why I opened #130375
There was a problem hiding this comment.
Did you not dereference the pointer the first time I linked it?
There was a problem hiding this comment.
Maybe only-x86_64 should exlude SGX, and we have a separate only-sgx for tests that want to run on SGX?
There was a problem hiding this comment.
Having only-x86_64 exclude SGX is odd, as SGX is only present on x86_64 platforms. Hence the platform is x86_64-fortanix-unknown-sgx
There was a problem hiding this comment.
#130375 is an interesting approach, but I don't see how this would avoid issues like the SGX special case. There isn't a special flag you need to add for the test to succeed on SGX. It's the test itself that causes issues. For most of the exceptions we currently have for SGX, they're there because of the test using CHECK: ret just to denote that they want to find the end of the function. This usually works pretty well because most assembly languages across platforms have identical instructions.
Rollup merge of rust-lang#130325 - workingjubilee:plus-minus-zero-redux, r=RalfJung,jieyouxu Use -0.0 in `intrinsics::simd::reduce_add_unordered` -0.0 is the actual neutral additive float, not +0.0, and this matters to codegen. try-job: aarch64-gnu
…dered_test, r=RalfJung Ignore reduce-fadd-unordered on SGX platform rust-lang#130325 added the `tests/assembly/simd/reduce-fadd-unordered.rs` test. Unfortunately, the use of `CHECK: ret` makes that this test is not compatible with LVI mitigations applied for the SGX target. This PR makes sure this test is ignored for the SGX target, until a nicer solution is available.
Rollup merge of rust-lang#130436 - fortanix:raoul/fix_reduce_add_unordered_test, r=RalfJung Ignore reduce-fadd-unordered on SGX platform rust-lang#130325 added the `tests/assembly/simd/reduce-fadd-unordered.rs` test. Unfortunately, the use of `CHECK: ret` makes that this test is not compatible with LVI mitigations applied for the SGX target. This PR makes sure this test is ignored for the SGX target, until a nicer solution is available.
-0.0 is the actual neutral additive float, not +0.0, and this matters to codegen.
r? @ghost
Fixes #130028
try-job: aarch64-gnu