[PATCH] D35451: [InstCombine] Improve the expansion in SimplifyUsingDistributiveLaws to handle cases where one side doesn't simplify, but the other side resolves to an identity value

[Sanjay Patel via Phabricator via llvm-commits]

spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

There's an open question about whether InstCombine should be in the refactoring/distribution optimization business in the first place. However, given that this patch both improves that folding and reduces specialized matchers, it will make it easier to lift this code into a different pass when we decide to move this.

LGTM.



================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:651
+
+    // Do "A op C" simplify to the identity value for the inner opcode?
+    if (L && L == ConstantExpr::getBinOpIdentity(InnerOpcode, L->getType())) {
----------------
"Do" -> "Does" (and same for the comments below)


================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:653
+    if (L && L == ConstantExpr::getBinOpIdentity(InnerOpcode, L->getType())) {
+      // They do! Return "B op C".
+      ++NumExpand;
----------------
"They do!" -> "It does!" (and repeat below)


================
Comment at: test/Transforms/InstCombine/and.ll:698
 
 ; Commute the 'or':
 ; ((Y | ~X) & X) -> (X & Y), where 'not' is an inverted cmp
----------------
Should be "Commute the 'and':"
Also reverse the other similar comments; these got inverted from the 'or' tests.


================
Comment at: test/Transforms/InstCombine/and.ll:753
 ; In the next 4 tests, vary the types and predicates for extra coverage.
 ; (~X & (Y | X)) -> (X & Y), where 'not' is an inverted cmp
 
----------------
This is a copy of my mistake in the original comment, so it's repeated 8 times I think: the result is actually (~X & Y).


https://reviews.llvm.org/D35451