[PATCH] D67434: [InstCombine] Limit FMul constant folding for fma simplifications.

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 11 09:03:59 PDT 2019


reames requested changes to this revision.
reames added a comment.
This revision now requires changes to proceed.

First, please revert the previous patch until this review concludes.  We have an active miscompile in tree, and that should be addressed first.

Second, I'm not a fan of this approach.  It feels very fragile.  For example, what if someone adds a transform which exploits rounding but doesn't involve constants?  (Say, we know some bits in one of the arguments...)

I would *strongly* suggest we restructure the code such that a common utility function is used, but with two cover functions which make the appropriate guarantees about rounding for the distinct cases.



================
Comment at: llvm/test/Transforms/InstCombine/fma.ll:502-511
+define <2 x double> @fmuladd_const_fmul(<2 x double> %b) {
+; CHECK-LABEL: @fmuladd_const_fmul(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[RES:%.*]] = fadd nnan nsz <2 x double> [[B:%.*]], <double 0x3F8DB6C076AD949B, double 0x3FB75A405B6E6D69>
+; CHECK-NEXT:    ret <2 x double> [[RES]]
+;
+entry:
----------------
lebedev.ri wrote:
> I don't think this restriction should apply to `@llvm.fmuladd`, as per https://llvm.org/docs/LangRef.html#llvm-fmuladd-intrinsic
I don't see why not?  The rounding is specified exactly as described.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67434





More information about the llvm-commits mailing list