[PATCH] D106331: [WIP][ScalarEvolution] Try harder to prove overflow in howManyLessThans.

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 31 17:19:26 PDT 2021


reames accepted this revision.
reames added a comment.
This revision is now accepted and ready to land.

LGTM.  I'm not completely opposed to this and don't want to block progress further.  I do think we need something better and more general.  If you've got some time in the next couple days, I'd really like to get on a call and chat through what that might be.

One, possibly crazy, idea would be to use the SCEV <-> Value maps we have, and then do a forward propagation on the values to see if a given addrec being poison is guaranteed to trigger UB before the latch is taken.  In this case, we should be able to map from the addrec to the IR expressions, and then find the poison reaching latch without needing to explicitly pass in the conditions.



================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:11791
-        getMinusSCEV(getAddExpr(RHS, MinusOne), StartMinusStride);
-    if (!isa<SCEVCouldNotCompute>(Numerator)) {
       BECount = getUDivExpr(Numerator, Stride);
----------------
I don't understand this line of the diff.  Can you remove for the moment, and do this in a separate change?  

The mixed usage of OrigStart and Start here is confusing.  


================
Comment at: llvm/test/Analysis/ScalarEvolution/no-wrap-symbolic-becount.ll:125
 
+define void @pointer_iv_nowrap_guard(i32* %startptr, i32* %endptr) local_unnamed_addr {
+; CHECK-LABEL: 'pointer_iv_nowrap_guard'
----------------
Please land this separately, and then rebase over so that the landed diff shows the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106331



More information about the llvm-commits mailing list