[PATCH] D14073: [FPEnv Core 08/14] Do not simplify expressions with FPEnv access

[Sergey Dmitrouk via llvm-commits]

sdmitrouk added inline comments.

================
Comment at: lib/Analysis/ConstantFolding.cpp:1008
@@ -1007,1 +1007,3 @@
+  if (!FMF.any())
+    FMF = CE->getFastMathFlags();
   return ConstantFoldConstantExpressionImpl(CE, DL, TLI, FoldedOps, FMF);
----------------
joker.eph wrote:
> sdmitrouk wrote:
> > joker.eph wrote:
> > > Why not always use `CE->getFastMathFlags();`? Wasn't the `FMF` param added to this function because of the lack of `FMF` in `ConstantExpr` originally? 
> > > Wasn't the FMF param added to this function because of the lack of FMF in ConstantExpr originally?
> > 
> > No, it wasn't. `FMF` can be from instruction which contains `CE` as one of its operands, the argument was added for `llvm::ConstantFoldInstruction`.
> Again, now that constant expression have dedicated FMF, I'm not sure why we should apply the parent instruction FMF?
> If a constant expression does not have FMF enabled and it an operand of an instruction with FMF enabled it does not give license to fold I think.
> And on the opposite, a constant expression with FMF as an operand of an instruction with FMF disabled should be foldable.
> 
> It is very possible that I miss something, David and/or Hal may have an opinion on this?
Maybe we can drop it, I'll need to check (don't remember by now) and see whether it causes any problems. Initially it did make sense to propagate flags of instruction to its operands, I agree that it might be useless now.


Repository:
  rL LLVM

http://reviews.llvm.org/D14073