[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor added inline comments.


================
Comment at: llvm/include/llvm/IR/IRBuilder.h:262
     Function *F = BB->getParent();
-    if (!F->hasFnAttribute(Attribute::StrictFP)) {
+    if (F && !F->hasFnAttribute(Attribute::StrictFP)) {
       F->addFnAttr(Attribute::StrictFP);
----------------
kpn wrote:
> rjmccall wrote:
> > kpn wrote:
> > > This looks reasonable to me. 
> > > 
> > > It smells like there's a larger strictfp IRBuilder issue, but that's not an issue for this patch here. The larger issue won't be hit since the new options affect the entire compilation. It therefore shouldn't block this patch.
> > Does IRBuilder actually support inserting into an unparented basic block?  I feel like this is exposing a much more serious mis-use of IRBuilder.
> I suspect you are correct. If we let this "F && " change go in we'll have a situation where whether or not a block is currently in a function when a function call is emitted will affect whether or not the eventual function definition gets the strictfp attribute. That seems like an unfortunate inconsistency.
> 
> I'm still looking into it. I hope to have an IRBuilder review up today or tomorrow.
As I just commented on the related patch @kpn posted, it appears that IRBuilder doesn't entirely support inserting into an unparented block. I was surprised by this, but there are places that need to be able to get to the Module from the BasicBlock. So, I think something problematic may be happening in the failing case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731