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

[Warren Ristow via Phabricator via llvm-commits]

wristow updated this revision to Diff 142334.
wristow added a comment.

I've updated the code-change in "InstCombineAddSub.cpp" to require both 'reassoc' and 'nsz' to do the transformations.  (I haven't changed the name from FAddCombine to FAddSubCombine or ReassociateFAddFSub.  I could do that if we move forward with this.)

>> There could be some kind of symbiotic behavior here, but this is really saying that we don't have a well-defined division of labor between these passes, right?
> 
> Yes, a lack of a well-defined division of labor between these passes is exactly my concern. I'll look into adding or modifying tests. Depending on possible interaction, that may not be fruitful. Ultimately, that should be addressed.

In looking this over, I was pleasantly surprised that I didn't run into the troublesome interaction I was concerned about.  So I've added new tests to "InstCombine/fast-math.ll" to check for the transformations happening when 'reassoc' + 'nsz' are on (with only '-instcombine' used).  I've also added tests that verify that 'reassoc' alone doesn't enable the transformation in the appropriate places.  Lastly, I updated the tests I had previously modified to use both 'reassoc' and 'nsz' where appropriate when testing for the minimal set of needed flags to transform things.


https://reviews.llvm.org/D45453

Files:
  lib/Transforms/InstCombine/InstCombineAddSub.cpp
  test/Transforms/InstCombine/fast-math.ll
  test/Transforms/Reassociate/fast-MissedTree.ll
  test/Transforms/Reassociate/fast-SubReassociate.ll
  test/Transforms/Reassociate/fast-basictest.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D45453.142334.patch
Type: text/x-patch
Size: 27985 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180413/eab9eeae/attachment.bin>