[PATCH] D45453: [InstCombine] Enable Add/Sub simplifications with only 'reassoc' FMF

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Apr 14 15:10:41 PDT 2018


spatel added inline comments.


================
Comment at: test/Transforms/InstCombine/fast-math.ll:392
+;
+  %mul = fmul reassoc nsz float %x, 7.000000e+00
+  %sub = fsub reassoc nsz float %mul, %x
----------------
wristow wrote:
> spatel wrote:
> > Maybe independent of this patch, but the fmul shouldn't need any FMF for this fold, so it's worth checking if that actually happens. Another consideration is whether the fmul hasOneUse(). We definitely don't have enough multi-use tests to show that folds are not unintentionally creating extra instructions.
> Hmmm... You're saying that the transformation:
> `c1 * x - x => (c1 - 1.0) * x`
> shouldn't need any FMF?  That doesn't seem right to me.  I //think //both 'reassoc' and 'nsz' should be needed.
The fsub needs FMF, but not the fmul. That's the general rule we're using in FMF-enabled folds. The non-strict final value (the result of the fsub) is what we're reassociating/factoring here, so as long as that instruction has reassoc+nsz, it doesn't matter if the fmul intermediate value is strict.


https://reviews.llvm.org/D45453





More information about the llvm-commits mailing list