[PATCH] D70984: [ThinLTO] Add option to disable readonly/writeonly attribute propagation

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson updated this revision to Diff 232436.
tejohnson marked an inline comment as done.
tejohnson added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70984/new/

https://reviews.llvm.org/D70984

Files:
  llvm/lib/IR/ModuleSummaryIndex.cpp
  llvm/lib/Transforms/IPO/FunctionImport.cpp
  llvm/test/ThinLTO/X86/writeonly.ll


Index: llvm/test/ThinLTO/X86/writeonly.ll
===================================================================
--- llvm/test/ThinLTO/X86/writeonly.ll
+++ llvm/test/ThinLTO/X86/writeonly.ll
@@ -25,6 +25,13 @@
 ; OPTIMIZE-NEXT:   %2 = tail call i32 @rand()
 ; OPTIMIZE-NEXT:   ret i32 0
 
+; Confirm that with -propagate-attrs=false we no longer do write-only importing
+; RUN: llvm-lto -propagate-attrs=false -thinlto-action=import -exported-symbol=main  %t1.bc -thinlto-index=%t3.index.bc -o %t1.imported.bc -stats 2>&1 | FileCheck %s --check-prefix=STATS-NOPROP
+; RUN: llvm-dis %t1.imported.bc -o - | FileCheck %s --check-prefix=IMPORT-NOPROP
+; STATS-NOPROP-NOT: Number of live global variables marked write only
+; IMPORT-NOPROP: @gFoo.llvm.0 = available_externally
+; IMPORT-NOPROP-NEXT: @gBar = available_externally
+
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-linux-gnu"
 
Index: llvm/lib/Transforms/IPO/FunctionImport.cpp
===================================================================
--- llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -901,19 +901,8 @@
     function_ref<PrevailingType(GlobalValue::GUID)> isPrevailing,
     bool ImportEnabled) {
   computeDeadSymbols(Index, GUIDPreservedSymbols, isPrevailing);
-  if (ImportEnabled) {
+  if (ImportEnabled)
     Index.propagateAttributes(GUIDPreservedSymbols);
-  } else {
-    // If import is disabled we should drop read/write-only attribute
-    // from all summaries to prevent internalization.
-    for (auto &P : Index)
-      for (auto &S : P.second.SummaryList)
-        if (auto *GVS = dyn_cast<GlobalVarSummary>(S.get())) {
-          GVS->setReadOnly(false);
-          GVS->setWriteOnly(false);
-        }
-  }
-  Index.setWithAttributePropagation();
 }
 
 /// Compute the set of summaries needed for a ThinLTO backend compilation of
Index: llvm/lib/IR/ModuleSummaryIndex.cpp
===================================================================
--- llvm/lib/IR/ModuleSummaryIndex.cpp
+++ llvm/lib/IR/ModuleSummaryIndex.cpp
@@ -15,6 +15,7 @@
 #include "llvm/ADT/SCCIterator.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
@@ -26,6 +27,10 @@
 STATISTIC(WriteOnlyLiveGVars,
           "Number of live global variables marked write only");
 
+static cl::opt<bool> PropagateAttrs("propagate-attrs", cl::init(true),
+                                    cl::Hidden,
+                                    cl::desc("Propagate attributes in index"));
+
 FunctionSummary FunctionSummary::ExternalNode =
     FunctionSummary::makeDummyFunctionSummary({});
 
@@ -157,6 +162,8 @@
 // See internalizeGVsAfterImport.
 void ModuleSummaryIndex::propagateAttributes(
     const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols) {
+  if (!PropagateAttrs)
+    return;
   for (auto &P : *this)
     for (auto &S : P.second.SummaryList) {
       if (!isGlobalValueLive(S.get()))
@@ -183,6 +190,7 @@
         }
       propagateAttributesToRefs(S.get());
     }
+  setWithAttributePropagation();
   if (llvm::AreStatisticsEnabled())
     for (auto &P : *this)
       if (P.second.SummaryList.size())


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70984.232436.patch
Type: text/x-patch
Size: 3331 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191205/6f400450/attachment.bin>