[PATCH] D119261: [DependenceAnalysis][PR52170] Conservative crash on overflowed loop backedge

[Artem Radzikhovskyy via Phabricator via llvm-commits]

artemrad added a comment.

In D119261#3306107 <https://reviews.llvm.org/D119261#3306107>, @Meinersbur wrote:

>> SCEVs already do the overflow checks that the code was targeting.
>
> Can you clarify? There is a reason why SCEVs have nsw/nuw flags.

If a SCEV detects that an overflow is possible it will not produce an AddRec. For example this is the SCEV produced for the simple example below `{(4 + %array)<nuw>,+,4}<nuw><%for.outer>`. Notice how the starting address is not a constant or a nested AddRec, but rather an expression.

  define void @overflow(i32 %n) {
  entry:
    %array = alloca [5 x i32], align 1
    br label %for.outer
  for.outer:
    %i = phi i32 [ 0, %entry ], [ %i.add, %for.outer ]
    %i.add = add i32 %i, 1
    %i.ptr = getelementptr inbounds [5 x i32], [5 x i32]* %array, i32 0, i32 %i.add
    store i32 0, i32* %i.ptr, align 1
    %cmp = icmp slt i32 %i.add, %n
    br i1 %cmp, label %for.outer, label %other
  other:
    ret void
  }



> Do you know what was the purpose of the check was? Usually those are not added without reason. In particular, it seems that it has been added in rGc0661aeaf8daf371023cf5669be4bd9b428882d0 <https://reviews.llvm.org/rGc0661aeaf8daf371023cf5669be4bd9b428882d0>. What if the code is called when not reclassfying a SIV reclassification?

Purpose was to make sure that collectUpperBound() was never called on overflowing loops. More specifically that DA exited early if a loop that could possibly overflow was detected. 
My claim is that the code I deleted was superfluous. CheckSubscript works on perfectly nested AddRecs. From the answer above we can see an AddRec would not be produced for a possibly overflowing index. The isLoopInvariant() checks inside the checkSubscript function would of already returned false for this SCEV (as they now do with my change). The deleted code was superfluous.

> What if the code is called when not reclassifying a SIV reclassification?

You are correct my statement in the change description was too conservative. This code does get called when we do initial classification of subscripts. It works as intended providing an accurate classification of the subscript. The subscripts that could overflow get the NonLinear classification, and DA conservatively returns [*] for that subscript as it doesn't know how to deal with it.


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

https://reviews.llvm.org/D119261