These are basically the same thing and only differ for strictfp,
so add both for future proofing. Note all the elementwise functions are
currently broken for strictfp, and use non-constrained ops. Add a test
that demonstrates this, but doesn't attempt to fix it.
Details
Diff Detail
Event Timeline
clang/docs/LanguageExtensions.rst | ||
---|---|---|
656 | "halfway'" is superfluous here. | |
657 | nearbyint should not rise only inexact exception. Other exceptions, like invalid are allowed. | |
662 | "halfway'" is superfluous here. | |
clang/test/CodeGen/strictfp-elementwise-bulitins.cpp | ||
3 | For X86 -fexperimental-strict-floating-point is not needed. | |
14–15 | Why vector type is bitcasted to scalar? The function must return <2 x float>, no? |
clang/test/CodeGen/strictfp-elementwise-bulitins.cpp | ||
---|---|---|
14–15 | I assume this is some ABI thing. It doesn't happen for other targets and the actual code is correct. I've avoided it by using float4 instead |
clang/docs/LanguageExtensions.rst | ||
---|---|---|
656 | It does exist, we just happen to ignore floating point environment most of the time. The current state of things is we have 3 LLVM intrinsics with the same behavior (nearbyint, rint and roundeven). The distinction only exists with fenv access / strictfp | |
clang/lib/Sema/SemaChecking.cpp | ||
12640 | I didn't delete any code? |
clang/lib/Sema/SemaChecking.cpp | ||
---|---|---|
12640 | Are you looking at a history diff? I see nothing deleted here |
clang/lib/Sema/SemaChecking.cpp | ||
---|---|---|
12640 | I am unsure what I was seeing prior, but I no longer see any deleted code, you can count this comment as resolved. |
clang/test/CodeGen/strictfp-elementwise-bulitins.cpp | ||
---|---|---|
190 | I noticed a missing pattern here. Did you mean to write canonicalize instead of TRUNC? |
I'd like to ask, is there a reason why there isn't a test for these new builtins under SemaCXX/builtins_elementwise_math.cpp?
clang/test/CodeGen/strictfp-elementwise-bulitins.cpp | ||
---|---|---|
190 | This is just the generated check from the IR names. It doesn't matter, but there's a pre-existing "bug" where the debug value name is trunc for canonicalize. |
Should there be tests added for usage of scalable vector types for RISC-V / AArch64?
I typically have added such tests in the past, as shown here:
https://reviews.llvm.org/D135011
Also, would you be able to add something at the top of the strictfp file that states the purpose? Like "testing that we don't use the constrained intrinsics"?
Test coverage seems to be spotty for them. I think a separate patch to get consistent SVE testing of this should be separate
Also, would you be able to add something at the top of the strictfp file that states the purpose? Like "testing that we don't use the constrained intrinsics"?
Done
"halfway'" is superfluous here.