Skip to content

Commit

Permalink
[PM/AA] Disable the core unsafe aspect of GlobalsModRef in the face of
Browse files Browse the repository at this point in the history
basic changes to the IR such as folding pointers through PHIs, Selects,
integer casts, store/load pairs, or outlining.

This leaves the feature available behind a flag. This flag's default
could be flipped if necessary, but the real-world performance impact of
this particular feature of GMR may not be sufficiently significant for
many folks to want to run the risk.

Currently, the risk here is somewhat mitigated by half-hearted attempts
to update GlobalsModRef when the rest of the optimizer changes
something. However, I am currently trying to remove that update
mechanism as it makes migrating the AA infrastructure to a form that can
be readily shared between new and old pass managers very challenging.
Without this update mechanism, it is possible that this still unlikely
failure mode will start to trip people, and so I wanted to try to
proactively avoid that.

There is a lengthy discussion on the mailing list about why the core
approach here is flawed, and likely would need to look totally different
to be both reasonably effective and resilient to basic IR changes
occuring. This patch is essentially the first of two which will enact
the result of that discussion. The next patch will remove the current
update mechanism.

Thanks to lots of folks that helped look at this from different angles.
Especial thanks to Michael Zolotukhin for doing some very prelimanary
benchmarking of LTO without GlobalsModRef to get a rough idea of the
impact we could be facing here. So far, it looks very small, but there
are some concerns lingering from other benchmarking. The default here
may get flipped if performance results end up pointing at this as a more
significant issue.

Also thanks to Pete and Gerolf for reviewing!

Differential Revision: http://reviews.llvm.org/D11213

llvm-svn: 242512
  • Loading branch information
chandlerc committed Jul 17, 2015
1 parent 7ef4bf5 commit f55803f
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 8 deletions.
39 changes: 33 additions & 6 deletions llvm/lib/Analysis/IPA/GlobalsModRef.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,20 @@ STATISTIC(NumNoMemFunctions, "Number of functions that do not access memory");
STATISTIC(NumReadMemFunctions, "Number of functions that only read memory");
STATISTIC(NumIndirectGlobalVars, "Number of indirect global objects");

// An option to enable unsafe alias results from the GlobalsModRef analysis.
// When enabled, GlobalsModRef will provide no-alias results which in extremely
// rare cases may not be conservatively correct. In particular, in the face of
// transforms which cause assymetry between how effective GetUnderlyingObject
// is for two pointers, it may produce incorrect results.
//
// These unsafe results have been returned by GMR for many years without
// causing significant issues in the wild and so we provide a mechanism to
// re-enable them for users of LLVM that have a particular performance
// sensitivity and no known issues. The option also makes it easy to evaluate
// the performance impact of these results.
static cl::opt<bool> EnableUnsafeGlobalsModRefAliasResults(
"enable-unsafe-globalsmodref-alias-results", cl::init(false), cl::Hidden);

namespace {
/// FunctionRecord - One instance of this structure is stored for every
/// function in the program. Later, the entries for these functions are
Expand Down Expand Up @@ -508,10 +522,17 @@ AliasResult GlobalsModRef::alias(const MemoryLocation &LocA,
GV2 = nullptr;

// If the two pointers are derived from two different non-addr-taken
// globals, or if one is and the other isn't, we know these can't alias.
if ((GV1 || GV2) && GV1 != GV2)
// globals we know these can't alias.
if (GV1 && GV2 && GV1 != GV2)
return NoAlias;

// If one is and the other isn't, it isn't strictly safe but we can fake
// this result if necessary for performance. This does not appear to be
// a common problem in practice.
if (EnableUnsafeGlobalsModRefAliasResults)
if ((GV1 || GV2) && GV1 != GV2)
return NoAlias;

// Otherwise if they are both derived from the same addr-taken global, we
// can't know the two accesses don't overlap.
}
Expand All @@ -537,12 +558,18 @@ AliasResult GlobalsModRef::alias(const MemoryLocation &LocA,
GV2 = AllocsForIndirectGlobals[UV2];

// Now that we know whether the two pointers are related to indirect globals,
// use this to disambiguate the pointers. If either pointer is based on an
// indirect global and if they are not both based on the same indirect global,
// they cannot alias.
if ((GV1 || GV2) && GV1 != GV2)
// use this to disambiguate the pointers. If the pointers are based on
// different indirect globals they cannot alias.
if (GV1 && GV2 && GV1 != GV2)
return NoAlias;

// If one is based on an indirect global and the other isn't, it isn't
// strictly safe but we can fake this result if necessary for performance.
// This does not appear to be a common problem in practice.
if (EnableUnsafeGlobalsModRefAliasResults)
if ((GV1 || GV2) && GV1 != GV2)
return NoAlias;

return AliasAnalysis::alias(LocA, LocB);
}

Expand Down
5 changes: 4 additions & 1 deletion llvm/test/Analysis/GlobalsModRef/aliastest.ll
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
; RUN: opt < %s -basicaa -globalsmodref-aa -gvn -S | FileCheck %s
; RUN: opt < %s -basicaa -globalsmodref-aa -gvn -S -enable-unsafe-globalsmodref-alias-results | FileCheck %s
;
; Note that this test relies on an unsafe feature of GlobalsModRef. While this
; test is correct and safe, GMR's technique for handling this isn't generally.

@X = internal global i32 4 ; <i32*> [#uses=1]

Expand Down
5 changes: 4 additions & 1 deletion llvm/test/Analysis/GlobalsModRef/indirect-global.ll
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
; RUN: opt < %s -basicaa -globalsmodref-aa -gvn -instcombine -S | FileCheck %s
; RUN: opt < %s -basicaa -globalsmodref-aa -gvn -instcombine -S -enable-unsafe-globalsmodref-alias-results | FileCheck %s
;
; Note that this test relies on an unsafe feature of GlobalsModRef. While this
; test is correct and safe, GMR's technique for handling this isn't generally.

@G = internal global i32* null ; <i32**> [#uses=3]

Expand Down

0 comments on commit f55803f

Please sign in to comment.