https://github.com/mayanez updated https://github.com/llvm/llvm-project/pull/220721
>From bcc840960b39f905f4034931116983c661d02a0c Mon Sep 17 00:00:00 2001 From: Miguel Arroyo <[email protected]> Date: Wed, 2 Sep 2026 14:24:16 -0700 Subject: [PATCH 1/3] [llvm][GlobalOpt] Preserve COMDATs during SRA splitting --- llvm/lib/Transforms/IPO/GlobalOpt.cpp | 13 ++++- .../Transforms/GlobalOpt/globalsra-comdat.ll | 50 +++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 llvm/test/Transforms/GlobalOpt/globalsra-comdat.ll diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp index 6f8d60ab8e2518..bdec78f09342ee 100644 --- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp +++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp @@ -587,6 +587,7 @@ static GlobalVariable *SRAGlobal(GlobalVariable *GV, const DataLayout &DL) { GV->getThreadLocalMode(), GV->getAddressSpace()); // Start out by copying attributes from the original, including alignment. NGV->copyAttributesFrom(GV); + NGV->setComdat(GV->getComdat()); NewGlobals.insert({OffsetForTy, NGV}); // Calculate the known alignment of the field. If the original aggregate @@ -656,7 +657,17 @@ static GlobalVariable *SRAGlobal(GlobalVariable *GV, const DataLayout &DL) { ++NumSRA; assert(NewGlobals.size() > 0); - return NewGlobals.begin()->second; + + auto *FirstNewGV = NewGlobals.begin()->second; + + // For COFF, the comdat must contain a member which has the same name as the + // group. We rename the first new global to match. + if (auto *C = FirstNewGV->getComdat()) { + auto ComdatName = C->getName(); + FirstNewGV->setName(ComdatName); + } + + return FirstNewGV; } /// Return true if all users of the specified value will trap if the value is diff --git a/llvm/test/Transforms/GlobalOpt/globalsra-comdat.ll b/llvm/test/Transforms/GlobalOpt/globalsra-comdat.ll new file mode 100644 index 00000000000000..d6ccb3ca4a88a0 --- /dev/null +++ b/llvm/test/Transforms/GlobalOpt/globalsra-comdat.ll @@ -0,0 +1,50 @@ +; RUN: opt < %s -S -passes=globalopt | FileCheck %s + +; This global is externally_initialized, so if we split it into scalars we +; should keep the original COMDAT grouping. +; CHECK: @a = internal unnamed_addr externally_initialized global i32 poison, comdat +; CHECK-NOT: @a.1 +$a = comdat any +@a = internal externally_initialized global [2 x i32] poison, comdat, align 4 + +; CHECK: @b = internal unnamed_addr externally_initialized global i32 poison, comdat +; CHECK-NOT: @b.1 +$b = comdat any +@b = internal externally_initialized global {i32, i32} poison, comdat, align 4 + +define i32 @foo() { +; CHECK-LABEL: define i32 @foo +entry: +; This load uses the split global, but cannot be constant-propagated away. +; CHECK: %0 = load i32, ptr @a + %0 = load i32, ptr @a, align 4 + ret i32 %0 +} + +define i32 @bar() { +; CHECK-LABEL: define i32 @bar +entry: +; This load uses the split global, but cannot be constant-propagated away. +; CHECK: %0 = load i32, ptr @b + %0 = load i32, ptr @b, align 4 + ret i32 %0 +} + +define void @init() { +; CHECK-LABEL: define void @init +entry: +; This store uses the split global, but cannot be constant-propagated away. +; CHECK: store i32 1, ptr @a + store i32 1, ptr @a, align 4 +; This store can be removed, because the second element of @a is never read. +; CHECK-NOT: store i32 2, ptr @a.1 + store i32 2, ptr getelementptr inbounds ([2 x i32], ptr @a, i32 0, i32 1), align 4 + +; This store uses the split global, but cannot be constant-propagated away. +; CHECK: store i32 3, ptr @b + store i32 3, ptr @b, align 4 +; This store can be removed, because the second element of @b is never read. +; CHECK-NOT: store i32 4, ptr @b.1 + store i32 4, ptr getelementptr inbounds ({i32, i32}, ptr @b, i32 0, i32 1), align 4 + ret void +} >From 0d771d4c16db650c21160c15d20cc701d98339bb Mon Sep 17 00:00:00 2001 From: Miguel Arroyo <[email protected]> Date: Wed, 9 Sep 2026 19:44:29 -0700 Subject: [PATCH 2/3] Use Dummy Global when doing SRA --- llvm/lib/Transforms/IPO/GlobalOpt.cpp | 24 ++++++-- .../Transforms/GlobalOpt/globalsra-comdat.ll | 61 +++++-------------- 2 files changed, 36 insertions(+), 49 deletions(-) diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp index bdec78f09342ee..aabe2f296bb5c1 100644 --- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp +++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp @@ -660,11 +660,16 @@ static GlobalVariable *SRAGlobal(GlobalVariable *GV, const DataLayout &DL) { auto *FirstNewGV = NewGlobals.begin()->second; - // For COFF, the comdat must contain a member which has the same name as the - // group. We rename the first new global to match. + // For COFF, the comdat must contain a member which has the + // same name as the group. if (auto *C = FirstNewGV->getComdat()) { - auto ComdatName = C->getName(); - FirstNewGV->setName(ComdatName); + auto *DummyGV = new GlobalVariable( + *FirstNewGV->getParent(), Type::getInt1Ty(FirstNewGV->getContext()), + false, FirstNewGV->getLinkage(), + ConstantInt::getFalse(FirstNewGV->getContext()), C->getName(), + FirstNewGV, FirstNewGV->getThreadLocalMode(), + FirstNewGV->getAddressSpace()); + DummyGV->setComdat(C); } return FirstNewGV; @@ -1536,6 +1541,17 @@ processInternalGlobal(GlobalVariable *GV, const GlobalStatus &GS, Changed = CleanupConstantGlobalUsers(GV, DL); } + // For COFF, the Comdat leader must be preserved. + if (auto *C = GV->getComdat()) { + auto IsComdatLeaderWithUses = + C->getName() == GV->getName() && C->getUsers().size() > 1; + if (IsComdatLeaderWithUses) { + LLVM_DEBUG(dbgs() << "GLOBAL IS COMDAT LEADER WITH USES: " << *GV + << "\n"); + return Changed; + } + } + // If the global is dead now, delete it. if (GV->use_empty()) { GV->eraseFromParent(); diff --git a/llvm/test/Transforms/GlobalOpt/globalsra-comdat.ll b/llvm/test/Transforms/GlobalOpt/globalsra-comdat.ll index d6ccb3ca4a88a0..1367abe4eb19d0 100644 --- a/llvm/test/Transforms/GlobalOpt/globalsra-comdat.ll +++ b/llvm/test/Transforms/GlobalOpt/globalsra-comdat.ll @@ -1,50 +1,21 @@ ; RUN: opt < %s -S -passes=globalopt | FileCheck %s -; This global is externally_initialized, so if we split it into scalars we -; should keep the original COMDAT grouping. -; CHECK: @a = internal unnamed_addr externally_initialized global i32 poison, comdat -; CHECK-NOT: @a.1 -$a = comdat any -@a = internal externally_initialized global [2 x i32] poison, comdat, align 4 +$x = comdat any +@x = internal global [2 x i32] zeroinitializer, comdat, align 4 +; CHECK: @x = internal unnamed_addr global i1 false, comdat +; CHECK: @x.0 = internal unnamed_addr global i32 0, comdat($x), align 4 +; CHECK: @x.1 = internal unnamed_addr global i32 0, comdat($x), align 4 -; CHECK: @b = internal unnamed_addr externally_initialized global i32 poison, comdat -; CHECK-NOT: @b.1 -$b = comdat any -@b = internal externally_initialized global {i32, i32} poison, comdat, align 4 - -define i32 @foo() { -; CHECK-LABEL: define i32 @foo -entry: -; This load uses the split global, but cannot be constant-propagated away. -; CHECK: %0 = load i32, ptr @a - %0 = load i32, ptr @a, align 4 - ret i32 %0 -} - -define i32 @bar() { -; CHECK-LABEL: define i32 @bar -entry: -; This load uses the split global, but cannot be constant-propagated away. -; CHECK: %0 = load i32, ptr @b - %0 = load i32, ptr @b, align 4 - ret i32 %0 +define dso_local i32 @f() { + %1 = load i32, ptr @x, align 4 + %2 = add nsw i32 %1, 1 + store i32 %2, ptr @x, align 4 + ret i32 %2 } -define void @init() { -; CHECK-LABEL: define void @init -entry: -; This store uses the split global, but cannot be constant-propagated away. -; CHECK: store i32 1, ptr @a - store i32 1, ptr @a, align 4 -; This store can be removed, because the second element of @a is never read. -; CHECK-NOT: store i32 2, ptr @a.1 - store i32 2, ptr getelementptr inbounds ([2 x i32], ptr @a, i32 0, i32 1), align 4 - -; This store uses the split global, but cannot be constant-propagated away. -; CHECK: store i32 3, ptr @b - store i32 3, ptr @b, align 4 -; This store can be removed, because the second element of @b is never read. -; CHECK-NOT: store i32 4, ptr @b.1 - store i32 4, ptr getelementptr inbounds ({i32, i32}, ptr @b, i32 0, i32 1), align 4 - ret void -} +define dso_local i32 @f2() { + %1 = load i32, ptr getelementptr inbounds ([2 x i32], ptr @x, i64 0, i64 1), align 4 + %2 = add nsw i32 %1, 1 + store i32 %2, ptr getelementptr inbounds ([2 x i32], ptr @x, i64 0, i64 1), align 4 + ret i32 %2 +} \ No newline at end of file >From 942c67eb4f57ed68cba7e3c6c1dc15a64adcf6c1 Mon Sep 17 00:00:00 2001 From: Miguel Arroyo <[email protected]> Date: Thu, 17 Sep 2026 09:02:36 -0700 Subject: [PATCH 3/3] Copy attributes into dummy and use [0 x i8] --- llvm/lib/Transforms/IPO/GlobalOpt.cpp | 7 +++++-- llvm/test/Transforms/GlobalOpt/globalsra-comdat.ll | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp index aabe2f296bb5c1..024a8315be7d3f 100644 --- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp +++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp @@ -663,13 +663,16 @@ static GlobalVariable *SRAGlobal(GlobalVariable *GV, const DataLayout &DL) { // For COFF, the comdat must contain a member which has the // same name as the group. if (auto *C = FirstNewGV->getComdat()) { + Type *GlobalType = ArrayType::get(Type::getInt8Ty(GV->getContext()), + 0); auto *DummyGV = new GlobalVariable( - *FirstNewGV->getParent(), Type::getInt1Ty(FirstNewGV->getContext()), + *FirstNewGV->getParent(), GlobalType, false, FirstNewGV->getLinkage(), - ConstantInt::getFalse(FirstNewGV->getContext()), C->getName(), + UndefValue::get(GlobalType), C->getName(), FirstNewGV, FirstNewGV->getThreadLocalMode(), FirstNewGV->getAddressSpace()); DummyGV->setComdat(C); + DummyGV->copyAttributesFrom(FirstNewGV); } return FirstNewGV; diff --git a/llvm/test/Transforms/GlobalOpt/globalsra-comdat.ll b/llvm/test/Transforms/GlobalOpt/globalsra-comdat.ll index 1367abe4eb19d0..75ae33eb38b8e0 100644 --- a/llvm/test/Transforms/GlobalOpt/globalsra-comdat.ll +++ b/llvm/test/Transforms/GlobalOpt/globalsra-comdat.ll @@ -2,7 +2,7 @@ $x = comdat any @x = internal global [2 x i32] zeroinitializer, comdat, align 4 -; CHECK: @x = internal unnamed_addr global i1 false, comdat +; CHECK: @x = internal unnamed_addr global [0 x i8] undef, comdat ; CHECK: @x.0 = internal unnamed_addr global i32 0, comdat($x), align 4 ; CHECK: @x.1 = internal unnamed_addr global i32 0, comdat($x), align 4 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
