[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/lib/Analysis/ScalarEvolution.cpp:7897
+          // As of now only handle cases where n = m, and n should be a value
+          // corresponding to a 8, 16, 32 integer.
+          if (LShift->getOperand(1) == BO->RHS &&
----------------
efriedma wrote:
> This could be extended to cases where n is greater than m?  You can skip that for the initial patch, of course.
> 
> I don't really see any reason to restrict the shift amounts like this; the transform is pretty restricted even without that.  What effect are you worried about?
> This could be extended to cases where n is greater than m? You can skip that for the initial patch, of course.

I could do that, but the original goal was to handle sext(trunc) expression that are expanded into these statements by instcombine.

> I don't really see any reason to restrict the shift amounts like this; the transform is pretty restricted even without that. What effect are you worried about?

We only wanted to support the data types supported by C/C++, and also since these instr are transformed from sext(trunc) it makes sense to just support standard integer types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152278