[PATCH] D81178: [FPEnv] Initialization of C++ globals not strictfp aware

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 8 11:35:30 PDT 2020


rjmccall added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.h:4348
+  /// Query for the use of constrained floating point math
+  bool isStrictFP() { return Builder.getIsFPConstrained(); }
+
----------------
kpn wrote:
> rjmccall wrote:
> > How is this set that we don't end up also setting it as a function attribute?
> I admit I don't understand the question. Since the strictfp attribute is required on both definitions and calls then only one accessor function seems needed?
> 
> I don't get to spend as much time in clang proper as I might like.
There is some code that's setting IsFPConstrained on the builder in the first place when building the global initialization function.  We should be making sure that we add the IR attribute to the function under the same conditions.

The main place that sets IsFPConstrained is in CGF::StartFunction where it detects that we're emitting a FunctionDecl and checks whether the FunctionDecl `usesFPIntrin()`.   That place also immediately adds the IR attribute.  But there's another place, in `CGF::SetFPModel`, that sets it based on the global exception and rounding-mode settings, and that place doesn't add the attribute.

Both of these places look buggy, though, because they're both setting `setIsFPConstrained(condition)`, which means that the second one to get called will overwrite the first rather than supplementing it.  We should unify these places so that StartFunction is responsible for deciding whether we're using `strictfp` and, if so, both configures the builder properly and adds the attribute.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81178/new/

https://reviews.llvm.org/D81178





More information about the cfe-commits mailing list