[PATCH] D143014: [InstCombine] Add combines for `(urem/srem (mul/shl X, Y), (mul/shl X, Z))`

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/urem-mul.ll:103
 ; CHECK-LABEL: @urem_CY_CZ_is_mul_X_RemYZ(
-; CHECK-NEXT:    [[BO0:%.*]] = mul nuw i8 [[X:%.*]], 21
-; CHECK-NEXT:    [[BO1:%.*]] = mul i8 [[X]], 6
-; CHECK-NEXT:    [[R:%.*]] = urem i8 [[BO0]], [[BO1]]
+; CHECK-NEXT:    [[R:%.*]] = mul nuw nsw i8 [[X:%.*]], 3
 ; CHECK-NEXT:    ret i8 [[R]]
----------------
MattDevereau wrote:
> goldstein.w.n wrote:
> > MattDevereau wrote:
> > > MattDevereau wrote:
> > > > Is it correct to generate a nsw flag for this case? Running this case through alive (https://alive2.llvm.org/ce/z/j9NY-S) I get a timeout unless I disable undef inputs, whereas just generating nuw accepts the transform (https://alive2.llvm.org/ce/z/332-Em)
> > > Phabricator marked this comment on the old revision, but it is still relevant.
> > Make it `i7` and it verifies: https://alive2.llvm.org/ce/z/5itDPy
> Fair enough, do you have any idea why it does this?
I think after its done all the symbolic simplifications, it does an exhaustive search through valid values. Smaller types have smaller exhaustive range so completes faster.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143014