[PATCH] D143013: [InstCombine] Add tests for combining (urem/srem (mul/shl X, Y), (mul/shl X, Z)); NFC

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n marked 3 inline comments as done.
goldstein.w.n added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/urem-mul.ll:32
+
+define i8 @urem_CY_CZ_is_zero(i8 %X) {
+; CHECK-LABEL: @urem_CY_CZ_is_zero(
----------------
MattDevereau wrote:
> This test could be better named, since C is actually %X. The important thing here which lets us simplify this to 0 is that the constant LHS multiplicand >= the constant RHS multiplicand of the rem. Maybe something like `urem_XY_XZ_constY_is_gte_constZ`?
> This test could be better named, since C is actually %X. The important thing here which lets us simplify this to 0 is that the constant LHS multiplicand >= the constant RHS multiplicand of the rem. Maybe something like `urem_XY_XZ_constY_is_gte_constZ`?




================
Comment at: llvm/test/Transforms/InstCombine/urem-mul.ll:32
+
+define i8 @urem_CY_CZ_is_zero(i8 %X) {
+; CHECK-LABEL: @urem_CY_CZ_is_zero(
----------------
goldstein.w.n wrote:
> MattDevereau wrote:
> > This test could be better named, since C is actually %X. The important thing here which lets us simplify this to 0 is that the constant LHS multiplicand >= the constant RHS multiplicand of the rem. Maybe something like `urem_XY_XZ_constY_is_gte_constZ`?
> > This test could be better named, since C is actually %X. The important thing here which lets us simplify this to 0 is that the constant LHS multiplicand >= the constant RHS multiplicand of the rem. Maybe something like `urem_XY_XZ_constY_is_gte_constZ`?
> 
> 
Okay, renamed all functions to reflect the conditions rather than the result.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143013