This is an archive of the discontinued LLVM Phabricator instance.

[AliasSetTracker] Make AST smarter about intrinsics that don't actually affect memory.
ClosedPublic

Authored by mcrosier on Oct 25 2016, 2:56 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

mcrosier updated this revision to Diff 75800.Oct 25 2016, 2:56 PM
mcrosier retitled this revision from to [AliasSetTracker] Make AST smarter about intrinsics that don't actually affect memory..
mcrosier updated this object.
mcrosier added reviewers: dberlin, mkuper, gberry.
mcrosier added a subscriber: llvm-commits.
gberry added inline comments.Oct 25 2016, 3:32 PM
lib/Analysis/AliasSetTracker.cpp
417 ↗(On Diff #75800)

Small nit: use auto for this declaration

hfinkel accepted this revision.Oct 25 2016, 7:12 PM
hfinkel added a reviewer: hfinkel.
hfinkel added a subscriber: hfinkel.

LGTM

This revision is now accepted and ready to land.Oct 25 2016, 7:12 PM
This revision was automatically updated to reflect the committed changes.
seurer added a subscriber: seurer.Oct 26 2016, 10:59 AM

This caused a failure on all PPC machines I have access to:
AddressSanitizer-powerpc64le-linux :: TestCases/use-after-scope-loop-bug.cc

mcrosier marked an inline comment as done.Oct 26 2016, 11:47 AM

This caused a failure on all PPC machines I have access to:
AddressSanitizer-powerpc64le-linux :: TestCases/use-after-scope-loop-bug.cc

Looking now..

dberlin edited edge metadata.EditedOct 26 2016, 11:51 AM

This is definitely calling ASAN failures.

My best guess, and just a guess, is that elimination is happening so that you have something like:

lifetime.start(%a)
use <%a>
lifetime.end(%a)
use of something equivalent to %a

and earlycse is now changing this to:

lifetime.start(%a)
use <%a>
lifetime.end(%a)
use <%a>

without extending the lifetime markers

This caused a failure on all PPC machines I have access to:
AddressSanitizer-powerpc64le-linux :: TestCases/use-after-scope-loop-bug.cc

Looking now..

I believe I've figured out the underlying problem. In short, LICM is relying on lifetime markers to prevent loads/stores from being moved out of a particular scope.

From the broken test case:
int main() {

// Variable goes in and out of scope.
for (int i = 0; i < 3; ++i) {
  int x[3] = {i, i, i};
  p = x + i;
}
return *p;  // BOOM

}

LICM will sink the stores to x[3] after the loop, which is obviously incorrect. I've reverted the change in r285227. Sorry for the breakage.