[PATCH] D131142: [InstCombine] Canonicalize "and, add", "or, add", "xor, add"

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

Looks good overall.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1769
+
+  unsigned Width = Ty->getScalarSizeInBits(); // C2->getBitWidth() instead?
+  unsigned LastOneMath = Width - C2->countTrailingZeros();
----------------
The way you've written it is fine. Just remove the comment.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1774
+  case Instruction::And:
+    if (!(C->countLeadingOnes() >= LastOneMath))
+      return nullptr;
----------------
Can you just fold the `!` and `>=` into `<` please


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1783
+  default:
+    return nullptr;
+  }
----------------
This can be llvm_unreachable


================
Comment at: llvm/test/Transforms/InstCombine/integer-round-up-pow2-alignment.ll:13
 ; CHECK-LABEL: @t0(
-; CHECK-NEXT:    [[X_BIASED1:%.*]] = add i8 [[X:%.*]], 15
-; CHECK-NEXT:    [[X_ROUNDEDUP:%.*]] = and i8 [[X_BIASED1]], -16
+; CHECK-NEXT:    [[X_BIASED:%.*]] = add i8 [[X:%.*]], 15
+; CHECK-NEXT:    [[X_ROUNDEDUP:%.*]] = and i8 [[X_BIASED]], -16
----------------
These changes are not significant - it's just update_test_checks choosing a different name for its variables. Could you regenerate the checks for this file first and commit that, and then rebase the current patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131142