This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] (A + 1) + (B ^ -1) --> A - B
ClosedPublic

Authored by gilr on Jun 25 2018, 1:41 AM.

Details

Summary

Turn canonicalized subtraction back into (-1 - B) and combine it with (A + 1) into (A - B).
This is similar to the folding already done for (B ^ -1) + Const into (-1 + Const) - B.

Diff Detail

Repository
rL LLVM

Event Timeline

gilr created this revision.Jun 25 2018, 1:41 AM
lebedev.ri added inline comments.
lib/Transforms/InstCombine/InstCombineAddSub.cpp
1217 ↗(On Diff #152611)

The constants are already canonicalized to RHS, so it's just m_Add().

gilr updated this revision to Diff 152651.Jun 25 2018, 4:02 AM

Impl. Roman's comment - m_Add instead of m_c_Add.

spatel accepted this revision.Jun 25 2018, 5:42 AM

LGTM - see inline for some test suggestions.

test/Transforms/InstCombine/add.ll
801–802 ↗(On Diff #152651)
  1. Commuting the constants in "%C" and "%D" distracts from the meaningful commute of "%E", so don't do that.
  2. It would provide more coverage if one of the tests used vector types (or add a dedicated test for vectors).
  3. It's a matter of taste, but I prefer to have the code comment above the function rather than embedded in the code.
  4. Similarly, give the test a meaningful name: "@add_not_increment"?
  5. Always prefer to commit the tests with baseline CHECKs as a preliminary step to the patched tests.
This revision is now accepted and ready to land.Jun 25 2018, 5:42 AM
gilr updated this revision to Diff 152684.Jun 25 2018, 7:10 AM

Impl. Sanjay comments.
Thanks for taking the time guys!

This revision was automatically updated to reflect the committed changes.