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

Mehdi AMINI via llvm-commits llvm-commits at lists.llvm.org
Sat Jan 23 11:03:42 PST 2016


joker.eph 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);
----------------
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?


Repository:
  rL LLVM

http://reviews.llvm.org/D14073





More information about the llvm-commits mailing list