[PATCH] D152278: [SCEV] Compute SCEV for ashr(add(shl(x, n), c), m) instr triplet
Nikita Popov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 13 06:07:55 PDT 2023
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
More information about the llvm-commits
mailing list