[compiler-rt] [llvm] [ThinLTO]Clean up 'import-assume-unique-local' flag. (PR #102424)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-pgo

@llvm/pr-subscribers-llvm-transforms

Author: Mingming Liu (minglotus-6)

<details>
<summary>Changes</summary>

While full file paths are used practically, the build automation tool bazel doesn't fail a build if duplicate relative paths are used in a unconventional way.

Clean up this flag as the ROI of enforcing full file paths is not high.  `NumDefs == 1` condition [1] should hold true for many internal-linkage vtables as long as full paths are indeed used to salvage the marginal performance when local-linkage vtables are imported due to indirect reference. https://github.com/llvm/llvm-project/pull/100448#discussion_r1692068402 has more details.

[1] https://github.com/llvm/llvm-project/pull/100448/files#diff-e7cb370fee46f0f773f2b5429dfab36b75126d3909ae98ee87ff3d0e3f75c6e9R215

---
Full diff: https://github.com/llvm/llvm-project/pull/102424.diff


2 Files Affected:

- (modified) compiler-rt/test/profile/Linux/instrprof-vtable-value-prof.cpp (-1) 
- (modified) llvm/lib/Transforms/IPO/FunctionImport.cpp (+1-12) 


``````````diff
diff --git a/compiler-rt/test/profile/Linux/instrprof-vtable-value-prof.cpp b/compiler-rt/test/profile/Linux/instrprof-vtable-value-prof.cpp
index 214c1504db7f45..e6a4af7b631e90 100644
--- a/compiler-rt/test/profile/Linux/instrprof-vtable-value-prof.cpp
+++ b/compiler-rt/test/profile/Linux/instrprof-vtable-value-prof.cpp
@@ -141,7 +141,6 @@
 // RUN:    -g -flto=thin -fwhole-program-vtables -fno-split-lto-unit -O2 \
 // RUN:    -mllvm -enable-vtable-value-profiling -Wl,-mllvm,-enable-vtable-value-profiling \
 // RUN:    -mllvm -enable-vtable-profile-use \
-// RUN:    -Wl,-plugin-opt,-import-assume-unique-local \
 // RUN:    -Wl,-mllvm,-enable-vtable-profile-use -Rpass=pgo-icall-prom \
 // RUN:    -Wl,-mllvm,-print-after=pgo-icall-prom \
 // RUN:    -Wl,-mllvm,-filter-print-funcs=main lib.cpp main.cpp 2>&1 \
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index 6c4f4001aab35f..1260aaa6afbd75 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -174,17 +174,6 @@ static cl::opt<std::string> WorkloadDefinitions(
              "}"),
     cl::Hidden);
 
-static cl::opt<bool> ImportAssumeUniqueLocal(
-    "import-assume-unique-local", cl::init(false),
-    cl::desc(
-        "By default, a local-linkage global variable won't be imported in the "
-        "edge mod1:func -> mod2:local-var (from value profiles) since compiler "
-        "cannot assume mod2 is compiled with full path which gives local-var a "
-        "program-wide unique GUID. Set this option to true will help cross "
-        "module import of such variables. This is only safe if the compiler "
-        "user specify the full module path."),
-    cl::Hidden);
-
 static cl::opt<std::string>
     ContextualProfile("thinlto-pgo-ctx-prof",
                       cl::desc("Path to a contextual profile."), cl::Hidden);
@@ -216,7 +205,7 @@ static bool shouldSkipLocalInAnotherModule(const GlobalValueSummary *RefSummary,
                                            StringRef ImporterModule) {
   // We can import a local from another module if all inputs are compiled
   // with full paths or when there is one definition.
-  if (ImportAssumeUniqueLocal || NumDefs == 1)
+  if (NumDefs == 1)
     return false;
   // In other cases, make sure we import the copy in the caller's module if the
   // referenced value has local linkage. The only time a local variable can

``````````

</details>


https://github.com/llvm/llvm-project/pull/102424