[PATCH] D69947: [ThinLTO] Simplify attribute propagation

[Eugene Leviant via Phabricator via llvm-commits]

evgeny777 created this revision.
evgeny777 added a reviewer: tejohnson.
Herald added subscribers: arphaman, dexonsmith, hiraditya, inglorion.
Herald added a project: LLVM.
evgeny777 edited the summary of this revision.

It looks like not calling to canImportGlobalVar during attribute propagation doesn't break any tests. Also, technically speaking, whether or not object can be imported shouldn't affect its readonly property


https://reviews.llvm.org/D69947

Files:
  llvm/include/llvm/IR/ModuleSummaryIndex.h
  llvm/lib/IR/ModuleSummaryIndex.cpp
  llvm/lib/Transforms/IPO/FunctionImport.cpp


Index: llvm/lib/Transforms/IPO/FunctionImport.cpp
===================================================================
--- llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -311,7 +311,7 @@
 
     for (auto &RefSummary : VI.getSummaryList())
       if (isa<GlobalVarSummary>(RefSummary.get()) &&
-          Index.canImportGlobalVar(RefSummary.get(), /* AnalyzeRefs */ true) &&
+          Index.canImportGlobalVar(RefSummary.get()) &&
           !LocalNotInModule(RefSummary.get())) {
         auto ILI = ImportList[RefSummary->modulePath()].insert(VI.getGUID());
         // Only update stat if we haven't already imported this variable.
Index: llvm/lib/IR/ModuleSummaryIndex.cpp
===================================================================
--- llvm/lib/IR/ModuleSummaryIndex.cpp
+++ llvm/lib/IR/ModuleSummaryIndex.cpp
@@ -171,11 +171,7 @@
       // 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. We don't analyze references here, because we have to
-        // know exactly if GV is readonly to do so.
-        if (!canImportGlobalVar(S.get(), /* AnalyzeRefs */ false) ||
-            GUIDPreservedSymbols.count(P.first)) {
+        if (GUIDPreservedSymbols.count(P.first)) {
           GVS->setReadOnly(false);
           GVS->setWriteOnly(false);
         }
@@ -194,8 +190,7 @@
           }
 }
 
-bool ModuleSummaryIndex::canImportGlobalVar(GlobalValueSummary *S,
-                                            bool AnalyzeRefs) const {
+bool ModuleSummaryIndex::canImportGlobalVar(GlobalValueSummary *S) const {
   auto HasRefsPreventingImport = [this](const GlobalVarSummary *GVS) {
     return !isReadOnly(GVS) && GVS->refs().size();
   };
@@ -203,12 +198,9 @@
 
   // Global variable with non-trivial initializer can be imported
   // if it's readonly. This gives us extra opportunities for constant
-  // folding and converting indirect calls to direct calls. We don't
-  // analyze GV references during attribute propagation, because we
-  // don't know yet if it is readonly or not.
+  // folding and converting indirect calls to direct calls.
   return !GlobalValue::isInterposableLinkage(S->linkage()) &&
-         !S->notEligibleToImport() &&
-         (!AnalyzeRefs || !HasRefsPreventingImport(GVS));
+         !S->notEligibleToImport() && !HasRefsPreventingImport(GVS);
 }
 
 // TODO: write a graphviz dumper for SCCs (see ModuleSummaryIndex::exportToDot)
Index: llvm/include/llvm/IR/ModuleSummaryIndex.h
===================================================================
--- llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -1071,8 +1071,8 @@
   }
 
   bool withAttributePropagation() const { return WithAttributePropagation; }
-  void setWithAttributePropagation() {
-    WithAttributePropagation = true;
+  void setWithAttributePropagation() { 
+    WithAttributePropagation = true; 
   }
 
   bool isReadOnly(const GlobalVarSummary *GVS) const {
@@ -1375,7 +1375,7 @@
   void propagateAttributes(const DenseSet<GlobalValue::GUID> &PreservedSymbols);
 
   /// Checks if we can import global variable from another module.
-  bool canImportGlobalVar(GlobalValueSummary *S, bool AnalyzeRefs) const;
+  bool canImportGlobalVar(GlobalValueSummary *S) const;
 };
 
 /// GraphTraits definition to build SCC for the index


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D69947.228236.patch
Type: text/x-patch
Size: 3567 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191107/0052e439/attachment.bin>