[PATCH] D11213: [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.

[Chandler Carruth]

chandlerc created this revision.
chandlerc added reviewers: hfinkel, Gerolf, pete.
chandlerc added a subscriber: llvm-commits.

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.

http://reviews.llvm.org/D11213

Files:
  lib/Analysis/IPA/GlobalsModRef.cpp
  test/Analysis/GlobalsModRef/aliastest.ll
  test/Analysis/GlobalsModRef/indirect-global.ll

Index: test/Analysis/GlobalsModRef/indirect-global.ll
===================================================================
--- test/Analysis/GlobalsModRef/indirect-global.ll
+++ test/Analysis/GlobalsModRef/indirect-global.ll
@@ -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]
 
Index: test/Analysis/GlobalsModRef/aliastest.ll
===================================================================
--- test/Analysis/GlobalsModRef/aliastest.ll
+++ test/Analysis/GlobalsModRef/aliastest.ll
@@ -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]
 
Index: lib/Analysis/IPA/GlobalsModRef.cpp
===================================================================
--- lib/Analysis/IPA/GlobalsModRef.cpp
+++ lib/Analysis/IPA/GlobalsModRef.cpp
@@ -41,6 +41,20 @@
 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 is 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));
+
 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 @@
       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 @@
     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);
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D11213.29758.patch
Type: text/x-patch
Size: 4356 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150715/6b12a689/attachment.bin>