[ThinLTO] Attempt to recommit r365040 after caching fix

It's possible that some function can load and store the same
variable using the same constant expression:

store %Derived* @foo, %Derived** bitcast (%Base** @bar to %Derived**)
%42 = load %Derived*, %Derived** bitcast (%Base** @bar to %Derived**)

The bitcast expression was mistakenly cached while processing loads,
and never examined later when processing store. This caused @bar to
be mistakenly treated as read-only variable. See load-store-caching.ll.

llvm-svn: 365188

1 files changed, 86 insertions, 55 deletions

diff --git a/llvm/lib/IR/ModuleSummaryIndex.cpp b/llvm/lib/IR/ModuleSummaryIndex.cpp
index 18b7ac09388..9f347d8da01 100644
--- a/llvm/lib/IR/ModuleSummaryIndex.cpp
+++ b/llvm/lib/IR/ModuleSummaryIndex.cpp
@@ -23,6 +23,8 @@ using namespace llvm;
 
 STATISTIC(ReadOnlyLiveGVars,
           "Number of live global variables marked read only");
+STATISTIC(WriteOnlyLiveGVars,
+          "Number of live global variables marked write only");
 
 FunctionSummary FunctionSummary::ExternalNode =
     FunctionSummary::makeDummyFunctionSummary({});
@@ -45,15 +47,18 @@ bool ValueInfo::canAutoHide() const {
                       });
 }
 
-// Gets the number of immutable refs in RefEdgeList
-unsigned FunctionSummary::immutableRefCount() const {
-  // Here we take advantage of having all readonly references
+// Gets the number of readonly and writeonly refs in RefEdgeList
+std::pair<unsigned, unsigned> FunctionSummary::specialRefCounts() const {
+  // Here we take advantage of having all readonly and writeonly references
   // located in the end of the RefEdgeList.
   auto Refs = refs();
-  unsigned ImmutableRefCnt = 0;
-  for (int I = Refs.size() - 1; I >= 0 && Refs[I].isReadOnly(); --I)
-    ImmutableRefCnt++;
-  return ImmutableRefCnt;
+  unsigned RORefCnt = 0, WORefCnt = 0;
+  int I;
+  for (I = Refs.size() - 1; I >= 0 && Refs[I].isWriteOnly(); --I)
+    WORefCnt++;
+  for (; I >= 0 && Refs[I].isReadOnly(); --I)
+    RORefCnt++;
+  return {RORefCnt, WORefCnt};
 }
 
 // Collect for the given module the list of function it defines
@@ -99,48 +104,56 @@ bool ModuleSummaryIndex::isGUIDLive(GlobalValue::GUID GUID) const {
   return false;
 }
 
-static void propagateConstantsToRefs(GlobalValueSummary *S) {
-  // If reference is not readonly then referenced summary is not
-  // readonly either. Note that:
+static void propagateAttributesToRefs(GlobalValueSummary *S) {
+  // If reference is not readonly or writeonly then referenced summary is not
+  // read/writeonly either. Note that:
   // - All references from GlobalVarSummary are conservatively considered as
-  //   not readonly. Tracking them properly requires more complex analysis
-  //   then we have now.
+  //   not readonly or writeonly. Tracking them properly requires more complex
+  //   analysis then we have now.
   //
   // - AliasSummary objects have no refs at all so this function is a no-op
   //   for them.
   for (auto &VI : S->refs()) {
-    if (VI.isReadOnly()) {
-      // We only mark refs as readonly when computing function summaries on
-      // analysis phase.
-      assert(isa<FunctionSummary>(S));
-      continue;
-    }
+    assert(VI.getAccessSpecifier() == 0 || isa<FunctionSummary>(S));
     for (auto &Ref : VI.getSummaryList())
-      // If references to alias is not readonly then aliasee is not readonly
-      if (auto *GVS = dyn_cast<GlobalVarSummary>(Ref->getBaseObject()))
-        GVS->setReadOnly(false);
+      // If references to alias is not read/writeonly then aliasee
+      // is not read/writeonly
+      if (auto *GVS = dyn_cast<GlobalVarSummary>(Ref->getBaseObject())) {
+        if (!VI.isReadOnly())
+          GVS->setReadOnly(false);
+        if (!VI.isWriteOnly())
+          GVS->setWriteOnly(false);
+      }
   }
 }
 
-// Do the constant propagation in combined index.
-// The goal of constant propagation is internalization of readonly
-// variables. To determine which variables are readonly and which
-// are not we take following steps:
-// - During analysis we speculatively assign readonly attribute to
-//   all variables which can be internalized. When computing function
-//   summary we also assign readonly attribute to a reference if
-//   function doesn't modify referenced variable.
+// Do the access attribute propagation in combined index.
+// The goal of attribute propagation is internalization of readonly (RO)
+// or writeonly (WO) variables. To determine which variables are RO or WO
+// and which are not we take following steps:
+// - During analysis we speculatively assign readonly and writeonly
+//   attribute to all variables which can be internalized. When computing
+//   function summary we also assign readonly or writeonly attribute to a
+//   reference if function doesn't modify referenced variable (readonly)
+//   or doesn't read it (writeonly).
+//
+// - After computing dead symbols in combined index we do the attribute
+//   propagation. During this step we:
+//   a. clear RO and WO attributes from variables which are preserved or
+//      can't be imported
+//   b. clear RO and WO attributes from variables referenced by any global
+//      variable initializer
+//   c. clear RO attribute from variable referenced by a function when
+//      reference is not readonly
+//   d. clear WO attribute from variable referenced by a function when
+//      reference is not writeonly
 //
-// - After computing dead symbols in combined index we do the constant
-//   propagation. During this step we clear readonly attribute from
-//   all variables which:
-//   a. are preserved or can't be imported
-//   b. referenced by any global variable initializer
-//   c. referenced by a function and reference is not readonly
+//   Because of (c, d) we don't internalize variables read by function A
+//   and modified by function B.
 //
 // Internalization itself happens in the backend after import is finished
-// See internalizeImmutableGVs.
-void ModuleSummaryIndex::propagateConstants(
+// See internalizeGVsAfterImport.
+void ModuleSummaryIndex::propagateAttributes(
     const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols) {
   for (auto &P : *this)
     for (auto &S : P.second.SummaryList) {
@@ -148,29 +161,36 @@ void ModuleSummaryIndex::propagateConstants(
         // We don't examine references from dead objects
         continue;
 
-      // Global variable can't be marked read only if it is not eligible
-      // to import since we need to ensure that all external references
-      // get a local (imported) copy. It also can't be marked read only
-      // if it or any alias (since alias points to the same memory) are
-      // preserved or notEligibleToImport, since either of those means
-      // there could be writes that are not visible (because preserved
-      // means it could have external to DSO writes, and notEligibleToImport
-      // means it could have writes via inline assembly leading it to be
-      // in the @llvm.*used).
+      // Global variable can't be marked read/writeonly if it is not eligible
+      // to import since we need to ensure that all external references get
+      // a local (imported) copy. It also can't be marked read/writeonly if
+      // it or any alias (since alias points to the same memory) are preserved
+      // or notEligibleToImport, since either of those means there could be
+      // writes (or reads in case of writeonly) that are not visible (because
+      // preserved means it could have external to DSO writes or reads, and
+      // notEligibleToImport means it could have writes or reads via inline
+      // assembly leading it to be in the @llvm.*used).
       if (auto *GVS = dyn_cast<GlobalVarSummary>(S->getBaseObject()))
         // Here we intentionally pass S.get() not GVS, because S could be
         // an alias.
-        if (!canImportGlobalVar(S.get()) || GUIDPreservedSymbols.count(P.first))
+        if (!canImportGlobalVar(S.get()) ||
+            GUIDPreservedSymbols.count(P.first)) {
           GVS->setReadOnly(false);
-      propagateConstantsToRefs(S.get());
+          GVS->setWriteOnly(false);
+        }
+      propagateAttributesToRefs(S.get());
     }
   if (llvm::AreStatisticsEnabled())
     for (auto &P : *this)
       if (P.second.SummaryList.size())
         if (auto *GVS = dyn_cast<GlobalVarSummary>(
                 P.second.SummaryList[0]->getBaseObject()))
-          if (isGlobalValueLive(GVS) && GVS->isReadOnly())
-            ReadOnlyLiveGVars++;
+          if (isGlobalValueLive(GVS)) {
+            if (GVS->maybeReadOnly())
+              ReadOnlyLiveGVars++;
+            if (GVS->maybeWriteOnly())
+              WriteOnlyLiveGVars++;
+          }
 }
 
 // TODO: write a graphviz dumper for SCCs (see ModuleSummaryIndex::exportToDot)
@@ -333,7 +353,13 @@ static void defineExternalNode(raw_ostream &OS, const char *Pfx,
 
 static bool hasReadOnlyFlag(const GlobalValueSummary *S) {
   if (auto *GVS = dyn_cast<GlobalVarSummary>(S))
-    return GVS->isReadOnly();
+    return GVS->maybeReadOnly();
+  return false;
+}
+
+static bool hasWriteOnlyFlag(const GlobalValueSummary *S) {
+  if (auto *GVS = dyn_cast<GlobalVarSummary>(S))
+    return GVS->maybeWriteOnly();
   return false;
 }
 
@@ -358,12 +384,14 @@ void ModuleSummaryIndex::exportToDot(raw_ostream &OS) const {
     // 0 - alias
     // 1 - reference
     // 2 - constant reference
-    // Other value: (hotness - 3).
-    TypeOrHotness += 3;
+    // 3 - writeonly reference
+    // Other value: (hotness - 4).
+    TypeOrHotness += 4;
     static const char *EdgeAttrs[] = {
         " [style=dotted]; // alias",
         " [style=dashed]; // ref",
         " [style=dashed,color=forestgreen]; // const-ref",
+        " [style=dashed,color=violetred]; // writeOnly-ref",
         " // call (hotness : Unknown)",
         " [color=blue]; // call (hotness : Cold)",
         " // call (hotness : None)",
@@ -408,6 +436,8 @@ void ModuleSummaryIndex::exportToDot(raw_ostream &OS) const {
         A.add("shape", "Mrecord", "variable");
         if (Flags.Live && hasReadOnlyFlag(SummaryIt.second))
           A.addComment("immutable");
+        if (Flags.Live && hasWriteOnlyFlag(SummaryIt.second))
+          A.addComment("writeOnly");
       }
       if (Flags.DSOLocal)
         A.addComment("dsoLocal");
@@ -429,10 +459,11 @@ void ModuleSummaryIndex::exportToDot(raw_ostream &OS) const {
     for (auto &SummaryIt : GVSMap) {
       auto *GVS = SummaryIt.second;
       for (auto &R : GVS->refs())
-        Draw(SummaryIt.first, R.getGUID(), R.isReadOnly() ? -1 : -2);
+        Draw(SummaryIt.first, R.getGUID(),
+             R.isWriteOnly() ? -1 : (R.isReadOnly() ? -2 : -3));
 
       if (auto *AS = dyn_cast_or_null<AliasSummary>(SummaryIt.second)) {
-        Draw(SummaryIt.first, AS->getAliaseeGUID(), -3);
+        Draw(SummaryIt.first, AS->getAliaseeGUID(), -4);
         continue;
       }