[clang] [FPEnv] Add strictfp in some C++ constructors lacking a FunctionDecl. (PR #74883)

John McCall via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 14 12:25:23 PST 2023


================
@@ -6,14 +7,80 @@ float z();
 #pragma float_control(except, on)
 class ON {
   float w = 2 + y() * z();
-  // CHECK-LABEL: define {{.*}} @_ZN2ONC2Ev{{.*}}
-  // CHECK: llvm.experimental.constrained.fmul{{.*}}tonearest{{.*}}strict
 };
 ON on;
 #pragma float_control(except, off)
 class OFF {
   float w = 2 + y() * z();
-  // CHECK-LABEL: define {{.*}} @_ZN3OFFC2Ev{{.*}}
-  // CHECK-NOT: llvm.experimental.constrained.fmul{{.*}}tonearest{{.*}}strict
 };
 OFF off;
+// CHECK-LABEL: define internal void @__cxx_global_var_init(
+// CHECK-SAME: ) #[[ATTR0:[0-9]+]] section ".text.startup" {
----------------
rjmccall wrote:

So here's the problem with these autogenerated checks: you're testing for a bunch of stuff you don't really care about and which we'll probably regret testing for in the future, and you've also lost some of the expressive intent of the old test.  Like, you don't actually care that the `this` argument is `noundef nonnull align 4 dereferenceable(4)`, and when we inevitably come up with yet another attribute to slap on `this`, we'll have to modify this test for no good reason. Meanwhile, the old test was pretty clear that it was testing whether the arithmetic in the constructor is done with constrained FP intrinsics, because the check lines were right next to the code involved, and now that isn't obvious at all.

In general:

1. Please position checks in the test file so that they're clearly associated with the code that they're testing.
2. Please edit the autogenerated checks down to make sure they're just checking the things you actually care about.
3. Please think about how other programmers will read this test and understand what you're testing for.

In specific, the only new thing you're really testing for here is, what, that the `strictfp` attribute is on `_ZN2ONC2Ev` and not on `_ZN3OFFC2Ev`?  Can you test for that?

https://github.com/llvm/llvm-project/pull/74883


More information about the cfe-commits mailing list