[llvm] 38dbdde - [Internalize] Simplify comdat renaming with noduplicates after D103043

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Fri May 28 16:58:45 PDT 2021


Author: Fangrui Song
Date: 2021-05-28T16:58:38-07:00
New Revision: 38dbdde7924cb1ab5919dc49bbd7d3ad420967f1

URL: https://github.com/llvm/llvm-project/commit/38dbdde7924cb1ab5919dc49bbd7d3ad420967f1
DIFF: https://github.com/llvm/llvm-project/commit/38dbdde7924cb1ab5919dc49bbd7d3ad420967f1.diff

LOG: [Internalize] Simplify comdat renaming with noduplicates after D103043

I realized that we can use `comdat noduplicates` which is available on ELF.
Add a special case for wasm which doesn't support the feature.

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/IPO/Internalize.h
    llvm/lib/Transforms/IPO/Internalize.cpp
    llvm/test/Transforms/Internalize/comdat.ll

Removed: 
    llvm/test/Transforms/Internalize/comdat-empty-moduleid.ll


################################################################################
diff  --git a/llvm/include/llvm/Transforms/IPO/Internalize.h b/llvm/include/llvm/Transforms/IPO/Internalize.h
index f92d42d319e90..41816df933601 100644
--- a/llvm/include/llvm/Transforms/IPO/Internalize.h
+++ b/llvm/include/llvm/Transforms/IPO/Internalize.h
@@ -40,14 +40,9 @@ class InternalizePass : public PassInfoMixin<InternalizePass> {
     size_t Size = 0;
     // Whether the comdat has an externally visible member.
     bool External = false;
-    // ELF only. Used to rename the comdat.
-    Comdat *Dest = nullptr;
   };
 
-  // For ELF, we need to rename comdat to a uniquified name. We derive the new
-  // comdat name from ModuleId.
-  std::string ModuleId;
-  bool IsELF = false;
+  bool IsWasm = false;
 
   /// Client supplied callback to control wheter a symbol must be preserved.
   const std::function<bool(const GlobalValue &)> MustPreserveGV;

diff  --git a/llvm/lib/Transforms/IPO/Internalize.cpp b/llvm/lib/Transforms/IPO/Internalize.cpp
index ce5429a77e8dd..008712c87988b 100644
--- a/llvm/lib/Transforms/IPO/Internalize.cpp
+++ b/llvm/lib/Transforms/IPO/Internalize.cpp
@@ -124,21 +124,15 @@ bool InternalizePass::maybeInternalize(
     if (auto *GO = dyn_cast<GlobalObject>(&GV)) {
       // If a comdat with one member is not externally visible, we can drop it.
       // Otherwise, the comdat can be used to establish dependencies among the
-      // group of sections. Thus we have to keep the comdat.
-      //
-      // On ELF, GNU ld and gold use the signature name as the comdat
-      // deduplication key. Rename the comdat to suppress deduplication with
-      // other object files. On COFF, non-external selection symbol suppresses
-      // deduplication and thus does not need renaming.
+      // group of sections. Thus we have to keep the comdat but switch it to
+      // noduplicates.
+      // Note: noduplicates is not necessary for COFF. wasm doesn't support
+      // noduplicates.
       ComdatInfo &Info = ComdatMap.find(C)->second;
-      if (Info.Size == 1) {
+      if (Info.Size == 1)
         GO->setComdat(nullptr);
-      } else if (IsELF) {
-        if (Info.Dest == nullptr)
-          Info.Dest = GV.getParent()->getOrInsertComdat(
-              (C->getName() + ModuleId).toStringRef(ComdatName));
-        GO->setComdat(Info.Dest);
-      }
+      else if (!IsWasm)
+        C->setSelectionKind(Comdat::NoDuplicates);
     }
 
     if (GV.hasLocalLinkage())
@@ -202,8 +196,7 @@ bool InternalizePass::internalizeModule(Module &M, CallGraph *CG) {
   }
 
   // Mark all functions not in the api as internal.
-  ModuleId = getUniqueModuleId(&M);
-  IsELF = Triple(M.getTargetTriple()).isOSBinFormatELF();
+  IsWasm = Triple(M.getTargetTriple()).isOSBinFormatWasm();
   for (Function &I : M) {
     if (!maybeInternalize(I, ComdatMap))
       continue;

diff  --git a/llvm/test/Transforms/Internalize/comdat-empty-moduleid.ll b/llvm/test/Transforms/Internalize/comdat-empty-moduleid.ll
deleted file mode 100644
index 87a8c5a228eec..0000000000000
--- a/llvm/test/Transforms/Internalize/comdat-empty-moduleid.ll
+++ /dev/null
@@ -1,17 +0,0 @@
-; RUN: opt < %s -mtriple=x86_64 -internalize -S | FileCheck %s
-
-$c2 = comdat any
-
-;; Without an exported symbol, the module ID is empty, and we don't rename
-;; comdat for ELF. This does not matter in practice because all the symbols
-;; will be optimized out.
-
-; CHECK: define internal void @c2_a() comdat($c2) {
-define void @c2_a() comdat($c2) {
-  ret void
-}
-
-; CHECK: define internal void @c2_b() comdat($c2) {
-define void @c2_b() comdat($c2) {
-  ret void
-}

diff  --git a/llvm/test/Transforms/Internalize/comdat.ll b/llvm/test/Transforms/Internalize/comdat.ll
index a00d06f90cac3..041c519ed0c96 100644
--- a/llvm/test/Transforms/Internalize/comdat.ll
+++ b/llvm/test/Transforms/Internalize/comdat.ll
@@ -1,7 +1,9 @@
 ; RUN: opt < %s -mtriple=x86_64 -internalize -internalize-public-api-list main -internalize-public-api-list c1 -internalize-public-api-list c2 \
-; RUN:   -internalize-public-api-list c3 -internalize-public-api-list c4 -S | FileCheck %s --check-prefixes=CHECK,ELF
+; RUN:   -internalize-public-api-list c3 -internalize-public-api-list c4 -S | FileCheck %s --check-prefixes=CHECK,NODUP
 ; RUN: opt < %s -mtriple=x86_64-windows -internalize -internalize-public-api-list main -internalize-public-api-list c1 -internalize-public-api-list c2 \
-; RUN:   -internalize-public-api-list c3 -internalize-public-api-list c4 -S | FileCheck %s --check-prefixes=CHECK,COFF
+; RUN:   -internalize-public-api-list c3 -internalize-public-api-list c4 -S | FileCheck %s --check-prefixes=CHECK,NODUP
+; RUN: opt < %s -mtriple=wasm32 -internalize -internalize-public-api-list main -internalize-public-api-list c1 -internalize-public-api-list c2 \
+; RUN:   -internalize-public-api-list c3 -internalize-public-api-list c4 -S | FileCheck %s --check-prefixes=CHECK,WASM
 
 $c1 = comdat any
 $c2 = comdat any
@@ -9,15 +11,20 @@ $c3 = comdat any
 $c4 = comdat any
 $c5 = comdat any
 
-; ELF:   $c2.[[MODULEID:[0-9a-f]+]] = comdat any
-; COFF:  $c2 = comdat any
+; CHECK: $c1 = comdat any
+
+;; wasm doesn't support noduplicates.
+; NODUP: $c2 = comdat noduplicates
+; WASM:  $c2 = comdat any
+
+; CHECK: $c3 = comdat any
+; CHECK: $c4 = comdat any
 
 ; CHECK: @c1_c = global i32 0, comdat($c1)
 @c1_c = global i32 0, comdat($c1)
 
 ;; $c2 has more than one member. Keep the comdat.
-; ELF:   @c2_b = internal global i32 0, comdat($c2.[[MODULEID]])
-; COFF:  @c2_b = internal global i32 0, comdat($c2)
+; CHECK: @c2_b = internal global i32 0, comdat($c2)
 @c2_b = global i32 0, comdat($c2)
 
 ; CHECK: @c3 = global i32 0, comdat{{$}}
@@ -45,14 +52,12 @@ define void @c1_a() comdat($c1) {
   ret void
 }
 
-; ELF:   define internal void @c2() comdat($c2.[[MODULEID]]) {
-; COFF:  define internal void @c2() comdat {
+; CHECK: define internal void @c2() comdat {
 define internal void @c2() comdat {
   ret void
 }
 
-; ELF:   define internal void @c2_a() comdat($c2.[[MODULEID]]) {
-; COFF:  define internal void @c2_a() comdat($c2) {
+; CHECK: define internal void @c2_a() comdat($c2) {
 define void @c2_a() comdat($c2) {
   ret void
 }


        


More information about the llvm-commits mailing list