[llvm] [ConstantMerge] Only merge constant w/unnamed_addr (PR #124050)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 22 19:01:57 PST 2025
https://github.com/gulfemsavrun created https://github.com/llvm/llvm-project/pull/124050
Currently, ConstantMergePass merges an unnamed_addr with a non-unnamed_addr constant as it is explained in LangRef.
"Note that a constant with significant address can be merged with a unnamed_addr constant, the result being a constant whose address is significant."
https://llvm.org/docs/LangRef.html#global-variables
This can result in a situation where Clang might vioalate C semantics, and here is a small reproducer to explain the problem:
const char foo_string[] = "foo";
const char* foo_func(void) { return "foo"; }
int is_foo(const char* p) { return p == foo_string; }
int main() {
printf("is_foo: %d\n", is_foo("foo"));
}
When we compile with -O0, where ConstantMerge is not applied, Clang and GCC have the same result.
clang -O0 foo.c -o foo
./foo
is_foo: 0
gcc -O0 foo.c -o foo
./foo
is_foo: 0
When we compile -O1 and higher, where ConstantMerge is applied, Clang and GCC have different results.
clang -O3 foo.c -o foo
./foo
is_foo: 1
gcc -O3 foo.c -o foo
./foo
is_foo: 0
Here's the IR before ConstantMergePass pass:
@.str = private unnamed_addr constant [4 x i8] c"foo\00", align 1 @_ZL10foo_string = internal constant [4 x i8] c"foo\00", align 1 @.str.1 = private unnamed_addr constant [12 x i8] c"is_foo: %d\0A\00", align 1
; Function Attrs: mustprogress nofree norecurse nosync nounwind readnone uwtable willreturn
define noundef i8* @_Z8foo_funcv() local_unnamed_addr #0 {
ret i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str, i64 0, i64
0)
}
; Function Attrs: mustprogress nofree norecurse nosync nounwind
readnone
uwtable willreturn
define noundef i32 @_Z6is_fooPKc(i8* noundef readnone %0)
local_unnamed_addr #0 {
%2 = icmp eq i8* %0, getelementptr inbounds ([4 x i8], [4 x i8]*
@_ZL10foo_string, i64 0, i64 0)
%3 = zext i1 %2 to i32
ret i32 %3
}
; Function Attrs: mustprogress nofree norecurse nounwind uwtable
define noundef i32 @main() local_unnamed_addr #1 {
%1 = tail call i32 (i8*, ...) @printf(i8* noundef nonnull
dereferenceable(1) getelementptr inbounds ([12 x i8], [12 x i8]*
@.str.1, i64 0, i64 0), i32 noundef zext (i1 icmp eq (i8* getelementptr
inbounds ([4 x i8], [4 x i8]* @.str, i64 0, i64 0), i8* getelementptr
inbounds ([4 x i8], [4 x i8]* @_ZL10foo_string, i64 0, i64 0)) to i32))
ret i32 0
}
MergeConstantPass merges _ZL10foo_string into .str, where it merges a non-unnamed_addr constant into an unnamed_addr constant.
IR Dump After ConstantMergePass on [module] ***
@.str = private constant [4 x i8] c"foo\00", align 1 @.str.1 = private unnamed_addr constant [12 x i8] c"is_foo: %d\0A\00", align 1
; Function Attrs: mustprogress nofree norecurse nosync nounwind readnone uwtable willreturn
define noundef i8* @_Z8foo_funcv() local_unnamed_addr #0 { ret i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str, i64 0, i64 0) }
; Function Attrs: mustprogress nofree norecurse nosync nounwind readnone uwtable willreturn
define noundef i32 @_Z6is_fooPKc(i8* noundef readnone %0) local_unnamed_addr #0 {
%2 = icmp eq i8* %0, getelementptr inbounds ([4 x i8], [4 x i8]* @.str, i64 0, i64 0)
%3 = zext i1 %2 to i32
ret i32 %3
}
; Function Attrs: mustprogress nofree norecurse nounwind uwtable define noundef i32 @main() local_unnamed_addr #1 { %1 = tail call i32 (i8*, ...) @printf(i8* noundef nonnull dereferenceable(1) getelementptr inbounds ([12 x i8], [12 x i8]* @.str.1, i64 0, i64 0), i32 noundef 1)
ret i32 0
}
This transformation might violate the following C pointer semantics:
"Two pointers compare equal if and only if both are null pointers, both are pointers to the same object (including a pointer to an object and a subobject at its beginning) or function, both are pointers to one past the last element of the same array object, or one is a pointer to one past the end of one array object and the other is a pointer to the start of a different array object that happens to immediately follow the first array object in the address space."
https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2912.pdf
So, this patch changes ConstantMerge pass to only allow merging when when a constant is marked with unnamed_addr attribute. I also found an old GitHub issue where a similar issue about invalid constant merging is explained. https://github.com/llvm/llvm-project/issues/9299
>From 320f01fe6db683d42317506e0f5da8fdd406677a Mon Sep 17 00:00:00 2001
From: Gulfem Savrun Yeniceri <gulfem at google.com>
Date: Wed, 22 Jan 2025 18:52:27 -0800
Subject: [PATCH] [ConstantMerge] Only merge constant w/unnamed_addr
Currently, ConstantMergePass merges an unnamed_addr with a
non-unnamed_addr constant as it is explained in LangRef.
"Note that a constant with significant address can be
merged with a unnamed_addr constant, the result being a
constant whose address is significant."
https://llvm.org/docs/LangRef.html#global-variables
This can result in a situation where Clang might vioalate C
semantics, and here is a small reproducer to explain the
problem:
const char foo_string[] = "foo";
const char* foo_func(void) { return "foo"; }
int is_foo(const char* p) { return p == foo_string; }
int main() {
printf("is_foo: %d\n", is_foo("foo"));
}
When we compile with -O0, where ConstantMerge is not
applied, Clang and GCC have the same result.
clang -O0 foo.c -o foo
./foo
is_foo: 0
gcc -O0 foo.c -o foo
./foo
is_foo: 0
When we compile -O1 and higher, where ConstantMerge is
applied, Clang and GCC have different results.
clang -O3 foo.c -o foo
./foo
is_foo: 1
gcc -O3 foo.c -o foo
./foo
is_foo: 0
Here's the IR before ConstantMergePass pass:
@.str = private unnamed_addr constant [4 x i8] c"foo\00", align 1
@_ZL10foo_string = internal constant [4 x i8] c"foo\00", align 1
@.str.1 = private unnamed_addr constant [12 x i8] c"is_foo: %d\0A\00",
align 1
; Function Attrs: mustprogress nofree norecurse nosync nounwind readnone
uwtable willreturn
define noundef i8* @_Z8foo_funcv() local_unnamed_addr #0 {
ret i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str, i64 0, i64
0)
}
; Function Attrs: mustprogress nofree norecurse nosync nounwind
readnone
uwtable willreturn
define noundef i32 @_Z6is_fooPKc(i8* noundef readnone %0)
local_unnamed_addr #0 {
%2 = icmp eq i8* %0, getelementptr inbounds ([4 x i8], [4 x i8]*
@_ZL10foo_string, i64 0, i64 0)
%3 = zext i1 %2 to i32
ret i32 %3
}
; Function Attrs: mustprogress nofree norecurse nounwind uwtable
define noundef i32 @main() local_unnamed_addr #1 {
%1 = tail call i32 (i8*, ...) @printf(i8* noundef nonnull
dereferenceable(1) getelementptr inbounds ([12 x i8], [12 x i8]*
@.str.1, i64 0, i64 0), i32 noundef zext (i1 icmp eq (i8* getelementptr
inbounds ([4 x i8], [4 x i8]* @.str, i64 0, i64 0), i8* getelementptr
inbounds ([4 x i8], [4 x i8]* @_ZL10foo_string, i64 0, i64 0)) to i32))
ret i32 0
}
MergeConstantPass merges _ZL10foo_string into .str, where it merges
a non-unnamed_addr constant into an unnamed_addr constant.
IR Dump After ConstantMergePass on [module] ***
@.str = private constant [4 x i8] c"foo\00", align 1
@.str.1 = private unnamed_addr constant [12 x i8] c"is_foo: %d\0A\00",
align 1
; Function Attrs: mustprogress nofree norecurse nosync nounwind readnone
uwtable willreturn
define noundef i8* @_Z8foo_funcv() local_unnamed_addr #0 {
ret i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str, i64 0, i64 0)
}
; Function Attrs: mustprogress nofree norecurse nosync nounwind readnone
uwtable willreturn
define noundef i32 @_Z6is_fooPKc(i8* noundef readnone %0)
local_unnamed_addr #0 {
%2 = icmp eq i8* %0, getelementptr inbounds ([4 x i8], [4 x i8]* @.str,
i64 0, i64 0)
%3 = zext i1 %2 to i32
ret i32 %3
}
; Function Attrs: mustprogress nofree norecurse nounwind uwtable
define noundef i32 @main() local_unnamed_addr #1 {
%1 = tail call i32 (i8*, ...) @printf(i8* noundef nonnull
dereferenceable(1) getelementptr inbounds ([12 x i8], [12 x i8]*
@.str.1, i64 0, i64 0), i32 noundef 1)
ret i32 0
}
This transformation might violate the following C pointer
semantics:
"Two pointers compare equal if and only if both are null
pointers, both are pointers to the same object (including
a pointer to an object and a subobject at its beginning)
or function, both are pointers to one past the last element
of the same array object, or one is a pointer to one past
the end of one array object and the other is a pointer to
the start of a different array object that happens to
immediately follow the first array object in the address
space."
https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2912.pdf
So, this patch changes ConstantMerge pass to only allow
merging when when a constant is marked with unnamed_addr
attribute. I also found an old GitHub issue where a similar
issue about invalid constant merging is explained.
https://github.com/llvm/llvm-project/issues/9299
---
llvm/lib/Transforms/IPO/ConstantMerge.cpp | 2 +-
.../ConstantMerge/2011-01-15-EitherOrder.ll | 2 +-
.../test/Transforms/ConstantMerge/merge-both.ll | 17 +++++++++--------
llvm/test/Transforms/ConstantMerge/merge-dbg.ll | 6 +++---
.../Transforms/ConstantMerge/unnamed-addr.ll | 8 +++++---
5 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/llvm/lib/Transforms/IPO/ConstantMerge.cpp b/llvm/lib/Transforms/IPO/ConstantMerge.cpp
index a1face0a6a9c3a..03b831526f4076 100644
--- a/llvm/lib/Transforms/IPO/ConstantMerge.cpp
+++ b/llvm/lib/Transforms/IPO/ConstantMerge.cpp
@@ -102,7 +102,7 @@ isUnmergeableGlobal(GlobalVariable *GV,
enum class CanMerge { No, Yes };
static CanMerge makeMergeable(GlobalVariable *Old, GlobalVariable *New) {
- if (!Old->hasGlobalUnnamedAddr() && !New->hasGlobalUnnamedAddr())
+ if (!Old->hasGlobalUnnamedAddr() || !New->hasGlobalUnnamedAddr())
return CanMerge::No;
if (hasMetadataOtherThanDebugLoc(Old))
return CanMerge::No;
diff --git a/llvm/test/Transforms/ConstantMerge/2011-01-15-EitherOrder.ll b/llvm/test/Transforms/ConstantMerge/2011-01-15-EitherOrder.ll
index 7742459ed4659b..ff73909514ca3d 100644
--- a/llvm/test/Transforms/ConstantMerge/2011-01-15-EitherOrder.ll
+++ b/llvm/test/Transforms/ConstantMerge/2011-01-15-EitherOrder.ll
@@ -6,7 +6,7 @@ declare i32 @zed(ptr, ptr)
%struct.foobar = type { i32 }
; CHECK: bar.d
@bar.d = unnamed_addr constant %struct.foobar zeroinitializer, align 4
-; CHECK-NOT: foo.d
+; CHECK: foo.d
@foo.d = internal constant %struct.foobar zeroinitializer, align 4
define i32 @main() nounwind ssp {
entry:
diff --git a/llvm/test/Transforms/ConstantMerge/merge-both.ll b/llvm/test/Transforms/ConstantMerge/merge-both.ll
index ab442e17b47405..0ed0bdb7609dc2 100644
--- a/llvm/test/Transforms/ConstantMerge/merge-both.ll
+++ b/llvm/test/Transforms/ConstantMerge/merge-both.ll
@@ -1,20 +1,21 @@
; RUN: opt -S < %s -passes=constmerge | FileCheck %s
-; Test that in one run var3 is merged into var2 and var1 into var4.
-; Test that we merge @var5 and @var6 into one with the higher alignment
+; Test that in one run var2 is merged into var4 and var6 is merged into var8.
+; Test that we merge @var6 and @var8 into one with the higher alignment
declare void @zed(ptr, ptr)
%struct.foobar = type { i32 }
@var1 = internal constant %struct.foobar { i32 2 }
- at var2 = unnamed_addr constant %struct.foobar { i32 2 }
+ at var2 = private unnamed_addr constant %struct.foobar { i32 2 }
@var3 = internal constant %struct.foobar { i32 2 }
- at var4 = unnamed_addr constant %struct.foobar { i32 2 }
+ at var4 = private unnamed_addr constant %struct.foobar { i32 2 }
; CHECK: %struct.foobar = type { i32 }
; CHECK-NOT: @
-; CHECK: @var2 = constant %struct.foobar { i32 2 }
-; CHECK-NEXT: @var4 = constant %struct.foobar { i32 2 }
+; CHECK: @var1 = internal constant %struct.foobar { i32 2 }
+; CHECK-NEXT: @var3 = internal constant %struct.foobar { i32 2 }
+; CHECK-NEXT: @var4 = private unnamed_addr constant %struct.foobar { i32 2 }
declare void @helper(ptr)
@var5 = internal constant [16 x i8] c"foo1bar2foo3bar\00", align 16
@@ -22,8 +23,9 @@ declare void @helper(ptr)
@var7 = internal constant [16 x i8] c"foo1bar2foo3bar\00"
@var8 = private unnamed_addr constant [16 x i8] c"foo1bar2foo3bar\00"
+; CHECK: @var5 = internal constant [16 x i8] c"foo1bar2foo3bar\00", align 16
; CHECK-NEXT: @var7 = internal constant [16 x i8] c"foo1bar2foo3bar\00"
-; CHECK-NEXT: @var8 = private constant [16 x i8] c"foo1bar2foo3bar\00", align 16
+; CHECK-NEXT: @var8 = private unnamed_addr constant [16 x i8] c"foo1bar2foo3bar\00", align 1
@var4a = alias %struct.foobar, ptr @var4
@llvm.used = appending global [1 x ptr] [ptr @var4a], section "llvm.metadata"
@@ -38,4 +40,3 @@ entry:
call void @helper(ptr @var8)
ret i32 0
}
-
diff --git a/llvm/test/Transforms/ConstantMerge/merge-dbg.ll b/llvm/test/Transforms/ConstantMerge/merge-dbg.ll
index de83ffbbda058a..5b396f16a20d2e 100644
--- a/llvm/test/Transforms/ConstantMerge/merge-dbg.ll
+++ b/llvm/test/Transforms/ConstantMerge/merge-dbg.ll
@@ -1,8 +1,8 @@
; RUN: opt < %s -passes=constmerge -S | FileCheck %s
-; CHECK: = constant i32 1, !dbg [[A:![0-9]+]], !dbg [[B:![0-9]+]]
- at a = internal constant i32 1, !dbg !0
- at b = unnamed_addr constant i32 1, !dbg !9
+; CHECK: = private unnamed_addr constant i32 1, !dbg [[A:![0-9]+]], !dbg [[B:![0-9]+]]
+ at a = private unnamed_addr constant i32 1, !dbg !0
+ at b = private unnamed_addr constant i32 1, !dbg !9
define void @test1(ptr %P1, ptr %P2) {
store ptr @a, ptr %P1
diff --git a/llvm/test/Transforms/ConstantMerge/unnamed-addr.ll b/llvm/test/Transforms/ConstantMerge/unnamed-addr.ll
index 2490e484c95260..f2b99bae5f89ac 100644
--- a/llvm/test/Transforms/ConstantMerge/unnamed-addr.ll
+++ b/llvm/test/Transforms/ConstantMerge/unnamed-addr.ll
@@ -1,6 +1,6 @@
; RUN: opt -passes=constmerge -S < %s | FileCheck %s
-; Test which corresponding x and y are merged and that unnamed_addr
-; is correctly set.
+; Test which corresponding x and y are merged when they both are marked with
+; unnamed_addr attribute.
declare void @zed(ptr, ptr)
@@ -23,7 +23,9 @@ declare void @zed(ptr, ptr)
; CHECK-NOT: @
; CHECK: @test1.x = internal constant %struct.foobar { i32 1 }
; CHECK-NEXT: @test1.y = constant %struct.foobar { i32 1 }
-; CHECK-NEXT: @test2.y = constant %struct.foobar { i32 2 }
+; CHECK-NEXT: @test2.x = internal constant %struct.foobar { i32 2 }
+; CHECK-NEXT: @test2.y = unnamed_addr constant %struct.foobar { i32 2 }
+; CHECK-NEXT: @test3.x = internal unnamed_addr constant %struct.foobar { i32 3 }
; CHECK-NEXT: @test3.y = constant %struct.foobar { i32 3 }
; CHECK-NEXT: @test4.y = unnamed_addr constant %struct.foobar { i32 4 }
; CHECK-NOT: @
More information about the llvm-commits
mailing list