Page MenuHomePhabricator

[LAA] Correctly return a half-open range in expandBounds
ClosedPublic

Authored by jmolloy on Apr 4 2017, 3:24 AM.

Details

Reviewers
anemet
sbaranga
Summary

This is a latent bug that's been hanging around for a while. For a loop-invariant
pointer, expandBounds would return the range {Ptr, Ptr}, but this was interpreted
as a half-open range, not a closed range. So we ended up planting incorrect
bounds checks. Even worse, they were tautological, so we ended up incorrectly
executing the optimized loop.

Diff Detail

Repository
rL LLVM

Event Timeline

jmolloy created this revision.Apr 4 2017, 3:24 AM
sbaranga added inline comments.Apr 4 2017, 3:52 AM
lib/Analysis/LoopAccessAnalysis.cpp
1944

If we're now always calling expandCodeFor(..), is it still worth checking above if NewPtr is in the loop body?

jmolloy added inline comments.Apr 4 2017, 4:36 AM
lib/Analysis/LoopAccessAnalysis.cpp
1944

We're only calling expandCodeFor unconditionally on ScPlusOne; I suppose if we're expecting instcombine to clean up from that we should also expect it to clean up expandCodeFor(Sc), but I don't see a need to generate worse code?

sbaranga edited edge metadata.Apr 4 2017, 4:46 AM

LGTM

lib/Analysis/LoopAccessAnalysis.cpp
1944

Ok, makes sense.

sbaranga accepted this revision.Apr 4 2017, 4:46 AM
This revision is now accepted and ready to land.Apr 4 2017, 4:46 AM
jmolloy closed this revision.Apr 5 2017, 2:39 AM

r299526, thanks!