[PATCH] D136340: [tests] precommit tests for D136015

Sat Oct 22 06:00:24 PDT 2022

spatel added inline comments.

================
Comment at: llvm/test/Transforms/InstCombine/mul_fold.ll:303
+  %addc = add i32 %m10, %m01
+  call void @use32(i32 %addc)
+  %shl = shl i32 %addc, 16
----------------
spatel wrote:
> Allen wrote:
> > spatel wrote:
> > > This extra use means we are still using In0Lo and In1Lo in the multiplies. The test comment above says this and the following tests should not be doing this?
> > > "the 2nd half have no extra uses to uses the full in0/in1 directly"
> > so update with "the 2nd half have no extra uses of mul to uses the full in0/in1 directly" to avoid the conflict ?
> No.
> 
> There are 4 variations of the partial multiplies: Hi/Lo and Hi/Lo, Hi/In and Hi/Lo, Hi/Lo and Hi/In, Hi/In and Hi/In.
> 
> The tests should be in canonical form to start - we do not want to rely on some other transform to produce the expected pattern for test.
> 
> Here are 2 examples of the tests and output that I expect to see:
> https://alive2.llvm.org/ce/z/C6ocXQ
> 
> We also want to test 16 commuted variations, so give each Hi/Lo variation 4 different commuted tests for maximum test coverage? If we want to be really thorough, we could just copy all of these patterns and get 16 * 4 = 64 tests, but I don't think that is necessary.
Sorry - I messed up the shift amounts when narrowing the width in those examples. Use this link instead:
https://alive2.llvm.org/ce/z/MkTpYs

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

https://reviews.llvm.org/D136340