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

David Majnemer via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 14 11:41:37 PST 2015


majnemer added inline comments.

================
Comment at: lib/IR/Constants.cpp:1890-1891
@@ +1889,4 @@
+  if (C1->getType()->isFPOrFPVectorTy()) {
+    // ConstantExpression can't store these flags, which can lead to going
+    // against them.
+    if (Strict && (!FMF.noExceptions() || !FMF.noRounding()))
----------------
joker.eph wrote:
> majnemer wrote:
> > Er, why not teach ConstantExpression about these flags?
> I thought that since it was float specific it couldn't be added, but I just checked and saw that we have integer nsw/nuw already on ConstantExpr, so it seems possible.
> 
> `ConstantExprKeyType` accepts `unsigned short SubclassOptionalData`. FastMathFlags are currently `unsigned Flags` but there is not reason they couldn't be `uint16_t` instead right?
> 
> David: do you know why we don't already have FastMath on ConstantExpr? Is it because there was no case with FP where we wouldn't constant fold immediately?
> 
> Now, what are the potential benefit to deal with Floating point ConstantExpr with FastMath flags? And what are the complications (i.e. will we have to add extra care everywhere we deal with FP ConstantExpr?)?
AFAIK, yes.  We didn't believe the flags would meaningfully change the result of what expression we'd give back.  This would change with the new flags.


Repository:
  rL LLVM

http://reviews.llvm.org/D14073





More information about the llvm-commits mailing list