[PATCH] D124710: [InstCombine] Fold ((A&B)^C)|B

[Sanjay Patel via Phabricator via llvm-commits]

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

LGTM - see inline comments about the tests. I see that those tests existed before this patch, but it would be nice to clean that up as a preliminary step before pushing this patch.



================
Comment at: llvm/test/Transforms/InstCombine/add.ll:1580
 ; CHECK-NEXT:    [[Y:%.*]] = udiv i8 42, [[_Y:%.*]]
 ; CHECK-NEXT:    [[AND:%.*]] = and i8 [[Y]], [[X:%.*]]
 ; CHECK-NEXT:    call void @use(i8 [[AND]])
----------------
This commuted, so it's not testing the pattern that was intended - it became identical to "commuted3". We need another "thwart" instruction to make it stay in the order that was written?

The extra use is a bit distracting in these tests. As long as we have that extra use in any 1 test, that provides coverage to account for that pattern.


================
Comment at: llvm/test/Transforms/InstCombine/add.ll:1631
 ; CHECK-NEXT:    [[X:%.*]] = udiv i8 42, [[_X:%.*]]
 ; CHECK-NEXT:    [[AND:%.*]] = and i8 [[X]], [[Y:%.*]]
 ; CHECK-NEXT:    call void @use(i8 [[AND]])
----------------
Similar to above:
This commuted, so it's not testing the pattern that was intended - it became identical to "commuted4". We need another "thwart" instruction to make it stay in the order that was written?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124710