[PATCH] D133362: [InstCombine] Fold x + (x | -x) to x & (x - 1)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/add_or_sub.ll:83
   %x = mul <4 x i16> %p, %p ; thwart complexity-based canonicalization
   %sub = sub <4 x i16> <i16 -1, i16 poison, i16 poison, i16 -1>, %x
   %or = or <4 x i16> %sub, %x
----------------
Oops - I missed this detail in the pre-commit. Shouldn't the vector constant have zeros rather than -1 to exercise this patch?


================
Comment at: llvm/test/Transforms/InstCombine/add_or_sub.ll:106
 
 define i2 @add_or_sub_comb_i2_multiuse_only_or(i2 %p) {
 ; CHECK-LABEL: @add_or_sub_comb_i2_multiuse_only_or(
----------------
We need to bail out on this pattern, so we don't have extra instructions (ie, this will be a negative test). See matcher for "m_OneUse()".


================
Comment at: llvm/test/Transforms/InstCombine/add_or_sub.ll:124
 
 define i19 @add_or_sub_comb_i19_multiuse_both(i19 %p) {
 ; CHECK-LABEL: @add_or_sub_comb_i19_multiuse_both(
----------------
We need to bail out on this pattern, so we don't have extra instructions (ie, this will be a negative test). See matcher for "m_OneUse()".


================
Comment at: llvm/test/Transforms/InstCombine/add_or_sub.ll:154
   %or = or i10 %sub, %x
   %add = add nsw nuw i10 %or, %x
   ret i10 %add
----------------
Can we propagate the no-wrap flags to the new add (ie, is it correct based on Alive2 proofs)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133362