[llvm] r242512 - [PM/AA] Disable the core unsafe aspect of GlobalsModRef in the face of

[Chandler Carruth]

Author: chandlerc
Date: Fri Jul 17 01:58:24 2015
New Revision: 242512

URL: http://llvm.org/viewvc/llvm-project?rev=242512&view=rev
Log:
[PM/AA] Disable the core unsafe aspect of GlobalsModRef in the face of
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

Modified:
    llvm/trunk/lib/Analysis/IPA/GlobalsModRef.cpp
    llvm/trunk/test/Analysis/GlobalsModRef/aliastest.ll
    llvm/trunk/test/Analysis/GlobalsModRef/indirect-global.ll

Modified: llvm/trunk/lib/Analysis/IPA/GlobalsModRef.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/IPA/GlobalsModRef.cpp?rev=242512&r1=242511&r2=242512&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/IPA/GlobalsModRef.cpp (original)
+++ llvm/trunk/lib/Analysis/IPA/GlobalsModRef.cpp Fri Jul 17 01:58:24 2015
@@ -41,6 +41,20 @@ STATISTIC(NumNoMemFunctions, "Number of
 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
@@ -508,10 +522,17 @@ AliasResult GlobalsModRef::alias(const M
       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.
   }
@@ -537,12 +558,18 @@ AliasResult GlobalsModRef::alias(const M
     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);
 }
 

Modified: llvm/trunk/test/Analysis/GlobalsModRef/aliastest.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/GlobalsModRef/aliastest.ll?rev=242512&r1=242511&r2=242512&view=diff
==============================================================================
--- llvm/trunk/test/Analysis/GlobalsModRef/aliastest.ll (original)
+++ llvm/trunk/test/Analysis/GlobalsModRef/aliastest.ll Fri Jul 17 01:58:24 2015
@@ -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]
 

Modified: llvm/trunk/test/Analysis/GlobalsModRef/indirect-global.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/GlobalsModRef/indirect-global.ll?rev=242512&r1=242511&r2=242512&view=diff
==============================================================================
--- llvm/trunk/test/Analysis/GlobalsModRef/indirect-global.ll (original)
+++ llvm/trunk/test/Analysis/GlobalsModRef/indirect-global.ll Fri Jul 17 01:58:24 2015
@@ -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]