[PATCH] D49602: Use SCEV to avoid inserting some bounds checks.

[Joel Galenson via Phabricator via llvm-commits]

jgalenson added a comment.

In https://reviews.llvm.org/D49602#1169952, @eugenis wrote:

> Do the tests cover cases where only one or two of the checks can be omitted, but not all of them? Does not look that way.


I'm not quite sure what you mean.  The test cases are essentially:

int foo[1000];
for (int i = 0; i < {1000,2000,n}; i++)

  foo[i];

and

foo[2][2];
for (int i = 0; i < {2,3,n}; i++)

  for (int j = 0; j < {2,3,n}; j++)
    foo[i][j[]

So they do cover the same where you iterate over the whole array and beyond it.  I also just added a testcase that counts down to 0 or below 0.



================
Comment at: lib/Transforms/Instrumentation/BoundsChecking.cpp:233
+    ScalarEvolution SE(F, TLI, ACT, DT, LI);
+    return addBoundsChecking(F, TLI, SE);
   }
----------------
eugenis wrote:
> Legacy pass should also request SCEV with getAnalysis.
> 
> If you were looking at SafeStack, the difference is that SafeStack is added to the pipeline unconditionally and needs to avoid computing SCEV for functions without the attribute, while this pass is only added by the frontend when needed and runs on most functions it sees.
> 
> See SafeStack code prior to https://reviews.llvm.org/D31302
> 
Thanks!  I did in fact copy this code from SafeStack, and it seemed a bit strange.  And thanks for the review link to the old code.


https://reviews.llvm.org/D49602