[PATCH] D139930: [InstCombine] Combine ZExt (B - A) + ZExt(A) to ZExt(B)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

Please pre-commit the baseline tests, so we just show the functional changes from this patch in the tests.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:1423
+       match(RHS, m_ZExt(m_NUWSub(m_Value(B), m_Specific(A)))))) {
+    Value *Res = Builder.CreateZExt(B, LHS->getType());
+    return replaceInstUsesWith(I, Res);
----------------
Use the raw constructor instead of Builder to preserve the value name:
  return new ZextInst(B, LHS->getType());


================
Comment at: llvm/test/Transforms/InstCombine/add.ll:2350
+
+define i16 @add_sub_zext_2(i8 %x, i8 %y) {
+; CHECK-LABEL: @add_sub_zext_2(
----------------
Add "_commute" to the test name to make the difference clear.


================
Comment at: llvm/test/Transforms/InstCombine/add.ll:2355
+;
+  %1 = sub nuw i8 %y, %x
+  %2 = zext i8 %1 to i16
----------------
Change types to increase test coverage. You could use weird types like <2 x i5>. 
We also want to verify at the limits of the fold, so zext by just 1 bit from an i2 (or even i1?).
It would also be good to add extra uses to at least one test because the extra uses don't change the transform. See examples in this file of `call void @use(i8)`.


================
Comment at: llvm/test/Transforms/InstCombine/add.ll:2370
+;
+  %1 = sub i8 %y, %x
+  %2 = zext i8 %1 to i16
----------------
Add another (negative) test where the sub operands are swapped?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139930