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

Sat Oct 22 06:36:46 PDT 2022

Allen 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:
> 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
So I try to copy the 16 cases, delete the use of **call void @use32(i32 [[ADDC]])** and keep one use of mul similar as your link  **https://alive2.llvm.org/ce/z/MkTpYs** 
i.e. one half only use **%m10** , and the other half only use **%m01** ? Does it enough? (we already have cases with no uses of mull and case with 2 uses of mull)

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

https://reviews.llvm.org/D136340