[PATCH] D144050: [SCEV] Strengthen nowrap flags via ranges for ARs on construction.

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 7 07:45:12 PST 2023


fhahn marked an inline comment as done.
fhahn added inline comments.


================
Comment at: llvm/test/Analysis/ScalarEvolution/incorrect-exit-count.ll:20
 ; CHECK-NEXT:    %storemerge1921 = phi i32 [ 3, %outer.loop ], [ %dec, %for.end ]
-; CHECK-NEXT:    --> {3,+,-1}<nuw><nsw><%for.cond6> U: [3,4) S: [3,4) Exits: <<Unknown>> LoopDispositions: { %for.cond6: Computable, %outer.loop: Variant }
+; CHECK-NEXT:    --> {3,+,-1}<nsw><%for.cond6> U: [3,4) S: [3,4) Exits: <<Unknown>> LoopDispositions: { %for.cond6: Computable, %outer.loop: Variant }
 ; CHECK-NEXT:    %idxprom20 = zext i32 %storemerge1921 to i64
----------------
mkazantsev wrote:
> mkazantsev wrote:
> > fhahn wrote:
> > > fhahn wrote:
> > > > nikic wrote:
> > > > > Regression
> > > > I still need to investigate this one.
> > > Ok, here's what's going on.
> > > 
> > > Without this patch, this is strengthened by this code here:
> > > https://github.com/llvm/llvm-project/blob/main/llvm/lib/Analysis/ScalarEvolution.cpp#L1651
> > > 
> > > With the new changes, this code now gets called earlier via the call to `proveNoWrapViaConstantRanges`, which in turn computes the backedge taken-count, which in turn calls `getSCEVAtScope`, which in turn creates the SCEV for `%idxprom20`. Because `getSCEVAtScope` constructs the SCEV for `%idxprom20` while computing the backedge taken count, the backedge taken count will be `SCEVCouldNotCompute` at L1651, hence the flags don't get strengthened.
> > > 
> > > I don't know if there's a way to avoid this issue, but it seems a small price to pay in this case. 
> > Interesting. If we have proved that the range here only contains `3`, can we just reduce it to `SCEVConstant`?
> > 
> > I believe that if a loop has zero BE taken count, all its AddRecs are `<nuw><nsw>` too.
> I think we might have a fundamental issue with being unable construct anything but `AddRec` for an IV, even in such loops.
It should be possible to simplify AddRecs (and any other SCEV expression) to a constant if the range for it is a single-element range: https://github.com/llvm/llvm-project/commit/ce3286116dc7e00c5c61db6c92e771f531ff38cd

This naive version has quite a bit of fallout in terms of test changes and some regressions (presumably because various places expect AddRecs and would need updating)

It also has a notable compile-time impact https://llvm-compile-time-tracker.com/compare.php?from=fbb73422260c6eae2ddbde299a266557a5bce8de&to=ce3286116dc7e00c5c61db6c92e771f531ff38cd&stat=instructions:u


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144050



More information about the llvm-commits mailing list