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

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D152278#4416394 <https://reviews.llvm.org/D152278#4416394>, @vedant-amd wrote:

> In D152278#4416367 <https://reviews.llvm.org/D152278#4416367>, @xbolva00 wrote:
>
>> You have to update that file anyway. Pre-commit builds are clearly failing.
>
> Right, is it fine if I mark the test case as XFAIL for now ?

No, you need to regenerate the test by running the update_test_checks.py script.



================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:7895
+          if (AddOperandCI &&
+              (AShrAmt == 32 || AShrAmt == 48 || AShrAmt == 56)) {
+            // since we truncate to TruncTy, the AddConstant should be of the same
----------------
There should be no restriction on the shift amount.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:7899
+            // Add constant should be shifted right by AShr amount.
+            APInt AddOperand = AddOperandCI->getValue().ashr(AShrAmt);
+            const SCEV *AddConstant =
----------------
We should verify that the add constant has lowest n bits unset, to make reassociation valid. It doesn't matter if n==m, but it matters if n!=m.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:7909
+            AddTruncateExpr =
+                getAddExpr(getTruncateExpr(ShlOp0SCEV, TruncTy), AddConstant);
+          }
----------------
You can keep the truncate in the comment code by making this trunc(add()) instead of add(trunc()). Unless I'm missing something, they're equivalent in this context.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:7923
+      if (AddTruncateExpr && ShlAmtCI) {
+
+        if (ShlAmtCI == CI)
----------------
Stray newline


================
Comment at: llvm/test/Analysis/ScalarEvolution/sext-add-inreg.ll:6
+
+define dso_local i32 @test(ptr nocapture noundef readonly %x) {
+; CHECK-LABEL: 'test'
----------------
You can test these cases with basic patterns using just the shl+add+ashr sequence, without a loop (though it's also ok to keep a loop test as a motivating case). Please also test the case where the shift amounts are different.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152278