This patch uses SCEV to avoid inserting some bounds checks when they are not needed. This slightly improves the performance of code compiled with the bounds check sanitizer.
This approach was suggested in https://reviews.llvm.org/D49492.
Paths
| Differential D49602
Use SCEV to avoid inserting some bounds checks. ClosedPublic Authored by jgalenson on Jul 20 2018, 8:23 AM.
Details Summary This patch uses SCEV to avoid inserting some bounds checks when they are not needed. This slightly improves the performance of code compiled with the bounds check sanitizer. This approach was suggested in https://reviews.llvm.org/D49492.
Diff Detail Event TimelineComment Actions I'm not very familiar with SCEV, so I'm not sure if I'm making the correct signed vs. unsigned ranges and comparisons here. What is the proper way to do that? Comment Actions This looks about right.
jgalenson marked 2 inline comments as done. Comment Actions
I'm not quite sure what you mean. The test cases are essentially: int foo[1000]; foo[i]; and foo[2][2]; 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 Actions Out of general curiosity, why is this needed? Comment Actions
The goal is to improve the performance of code that uses the bounds checker. Why can't other passes drop the unneeded checks? Once they're added, they're just comparisons and branches that can be removed if a pass can figure out that the path to a trap will never be taken. That said, the goal of this patch isn't to enable other passes to remove the checks (that was my old approach) but instead not to insert them when they're not needed. Comment Actions
Consider a loop where offset is always inbounds, but offset + access size may be not. Bounds checks is still needed, but it should be cheaper, because Size <= Offset will be constant false. Comment Actions
Do you mean something like an off-by-one test, e.g., for (int i = 0; i <= 1000; i++)? Comment Actions Added a testcase that goes one off the end of the loop counting up (and the previous update added one that counts down one off the end).
jgalenson marked an inline comment as done. This revision is now accepted and ready to land.Jul 23 2018, 12:53 PM Closed by commit rL337830: Use SCEV to avoid inserting some bounds checks. (authored by jgalenson). · Explain WhyJul 24 2018, 8:22 AM This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 156492 lib/Transforms/Instrumentation/BoundsChecking.cpp
test/Instrumentation/BoundsChecking/opt.ll
|
Non-legacy pass should request ScalarEvolution from the analysis manager instead of recomputing it.