[PATCH] D152278: [SCEV] Compute SCEV for ashr(add(shl(x, n), c), m) instr triplet

[Vedant Paranjape via Phabricator via llvm-commits]

vedant-amd added inline comments.


================
Comment at: llvm/test/Analysis/ScalarEvolution/sext-add-inreg-unequal.ll:12
+; CHECK-NEXT:    %ashr = ashr exact i64 %shl, 8
+; CHECK-NEXT:    --> (sext i56 (4 * (trunc i64 %a to i56)) to i64) U: [0,-3) S: [-36028797018963968,36028797018963965)
+; CHECK-NEXT:  Determining loop execution counts for: @test00
----------------
vedant-amd wrote:
> efriedma wrote:
> > vedant-amd wrote:
> > > efriedma wrote:
> > > > How is this equivalent?  Say %a is zero; the original function returns 1, this SCEV expression returns 0.  (I think maybe the "ashr" of the constant is shifting by 10, instead of shifting by 8?)
> > > Yeah, the SCEV is wrong. I will fix this. This is what is should have been, but it's a bit opposite.
> > > 
> > > ```
> > > = (a*2^10 + 256)/2*8
> > > = a*4 + 1
> > > ```
> > > So, we need SCEV like this:
> > > 
> > > 2^(shl_amt - ashr_amt) * a + c >> ashr_amt
> > > 
> > > I have updated my code, here's the correct SCEV:
> > > 
> > > (1 + (sext i56 (4 * (trunc i64 %a to i56)) to i64))<nuw><nsw> U: [1,-2) S: [-36028797018963967,36028797018963966)
> > > 
> > > Does it seem correct ?
> > I suspect the updated code isn't right... can you add an example with a larger operand to the add?  The add needs to happen before the sign-extend, but in this specific example, that doesn't matter because the two expressions are equivalent.
> I will add one, but I believe we can move the add out of the sext as the add constant is already shifted right by Ashr amount.
Also, I am confused about one thing, the Type of MulExpr turns out to be i56 and that of the addConstant is i64, I am able to add them before the Sign extend. Adding the m > n part to this ashr model seems to buggy at this point, as I had proposed can this patch just address the case where m=n ? and I submit, the m > n thing later with thorough thinking. But, I think @nikic wants them in the same patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152278