Skip to content

Commit 20d8f8c

Browse files
authored
[GlobalOpt] Fix global SRA incorrect alignment on some elements (#115328)
The logic had a flaw where the alignment from the original aggregate is unintentionally retained for elements when the calculated known alignment is not higher than the element's ABI type alignment. Fixes #115282.
1 parent 5a48162 commit 20d8f8c

File tree

5 files changed

+61
-9
lines changed

5 files changed

+61
-9
lines changed

llvm/lib/Transforms/IPO/GlobalOpt.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -575,15 +575,16 @@ static GlobalVariable *SRAGlobal(GlobalVariable *GV, const DataLayout &DL) {
575575
*GV->getParent(), Ty, false, GlobalVariable::InternalLinkage,
576576
Initializer, GV->getName() + "." + Twine(NameSuffix++), GV,
577577
GV->getThreadLocalMode(), GV->getAddressSpace());
578+
// Start out by copying attributes from the original, including alignment.
578579
NGV->copyAttributesFrom(GV);
579580
NewGlobals.insert({OffsetForTy, NGV});
580581

581582
// Calculate the known alignment of the field. If the original aggregate
582-
// had 256 byte alignment for example, something might depend on that:
583+
// had 256 byte alignment for example, then the element at a given offset
584+
// may also have a known alignment, and something might depend on that:
583585
// propagate info to each field.
584586
Align NewAlign = commonAlignment(StartAlignment, OffsetForTy);
585-
if (NewAlign > DL.getABITypeAlign(Ty))
586-
NGV->setAlignment(NewAlign);
587+
NGV->setAlignment(NewAlign);
587588

588589
// Copy over the debug info for the variable.
589590
transferSRADebugInfo(GV, NGV, OffsetForTy * 8,

llvm/test/DebugInfo/X86/global-sra-struct-fit-segment.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
%struct.BSS1 = type <{ [12 x i8] }>
2121

2222
;CHECK: @.BSS1.0 = internal unnamed_addr global i32 0, align 32, !dbg ![[GVE1:.*]]
23-
;CHECK: @.BSS1.1 = internal unnamed_addr global i32 0, align 32, !dbg ![[GVE2:.*]], !dbg ![[GVE4:.*]]
23+
;CHECK: @.BSS1.1 = internal unnamed_addr global i32 0, align 4, !dbg ![[GVE2:.*]], !dbg ![[GVE4:.*]]
2424
;CHECK: @.BSS1.2 = internal unnamed_addr global i32 0, align 8, !dbg ![[GVE3:.*]]
2525

2626
@.BSS1 = internal global %struct.BSS1 zeroinitializer, align 32, !dbg !0, !dbg !7, !dbg !10, !dbg !27, !dbg !29

llvm/test/DebugInfo/X86/global-sra-struct-part-overlap-segment.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626

2727
@.BSS3 = internal unnamed_addr global %struct.BSS3 zeroinitializer, align 32, !dbg !0, !dbg !7, !dbg !29
2828
;CHECK: @.BSS3.0 = internal unnamed_addr global double 0.000000e+00, align 32, !dbg ![[GVE1:.*]], !dbg ![[GVE2:.*]]
29-
;CHECK: @.BSS3.1 = internal unnamed_addr global double 0.000000e+00, align 32, !dbg ![[GVE3:.*]], !dbg ![[GVE4:.*]], !dbg ![[GVE6:.*]]
29+
;CHECK: @.BSS3.1 = internal unnamed_addr global double 0.000000e+00, align 8, !dbg ![[GVE3:.*]], !dbg ![[GVE4:.*]], !dbg ![[GVE6:.*]]
3030
;CHECK: @.BSS3.2 = internal unnamed_addr global double 0.000000e+00, align 16, !dbg ![[GVE5:.*]]
3131

3232
@.C363_mymod_bar_ = internal constant [2 x i8] c"IF"

llvm/test/Transforms/GlobalOpt/globalsra-align.ll

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@ target datalayout = "p:16:32:64" ; 16-bit pointers with 32-bit ABI alignment and
1515

1616
;.
1717
; CHECK: @[[A_4:[a-zA-Z0-9_$"\\.-]+]] = internal unnamed_addr externally_initialized global ptr null, align 8
18-
; CHECK: @[[A_5:[a-zA-Z0-9_$"\\.-]+]] = internal unnamed_addr externally_initialized global ptr null, align 16
18+
; CHECK: @[[A_5:[a-zA-Z0-9_$"\\.-]+]] = internal unnamed_addr externally_initialized global ptr null, align 8
1919
; CHECK: @[[A_6:[a-zA-Z0-9_$"\\.-]+]] = internal unnamed_addr externally_initialized global ptr null, align 16
20-
; CHECK: @[[A_7:[a-zA-Z0-9_$"\\.-]+]] = internal unnamed_addr externally_initialized global ptr null, align 16
20+
; CHECK: @[[A_7:[a-zA-Z0-9_$"\\.-]+]] = internal unnamed_addr externally_initialized global ptr null, align 8
2121
;.
2222
define ptr @reduce_align_0() {
2323
; CHECK-LABEL: @reduce_align_0(
@@ -31,7 +31,7 @@ define ptr @reduce_align_0() {
3131

3232
define ptr @reduce_align_1() {
3333
; CHECK-LABEL: @reduce_align_1(
34-
; CHECK-NEXT: [[X:%.*]] = load ptr, ptr @a.5, align 16
34+
; CHECK-NEXT: [[X:%.*]] = load ptr, ptr @a.5, align 8
3535
; CHECK-NEXT: ret ptr [[X]]
3636
;
3737
%x = load ptr, ptr getelementptr inbounds ([3 x [7 x ptr]], ptr @a, i64 0, i64 2, i64 1), align 4
@@ -51,7 +51,7 @@ define ptr @reduce_align_2() {
5151

5252
define ptr @reduce_align_3() {
5353
; CHECK-LABEL: @reduce_align_3(
54-
; CHECK-NEXT: [[X:%.*]] = load ptr, ptr @a.7, align 16
54+
; CHECK-NEXT: [[X:%.*]] = load ptr, ptr @a.7, align 8
5555
; CHECK-NEXT: ret ptr [[X]]
5656
;
5757
%x = load ptr, ptr getelementptr inbounds ([3 x [7 x ptr]], ptr @a, i64 0, i64 2, i64 3), align 4
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals all --version 5
2+
; RUN: opt < %s -passes=globalopt -S | FileCheck %s
3+
4+
@params = internal global [4 x i32] zeroinitializer, align 32
5+
6+
;.
7+
; CHECK: @params.0 = internal unnamed_addr global i32 0, align 32
8+
; CHECK: @params.1 = internal unnamed_addr global i32 0, align 4
9+
; CHECK: @params.2 = internal unnamed_addr global i32 0, align 8
10+
; CHECK: @params.3 = internal unnamed_addr global i32 0, align 4
11+
;.
12+
define void @set(i32 %a, i32 %b, i32 %c, i32 %d) {
13+
; CHECK-LABEL: define void @set(
14+
; CHECK-SAME: i32 [[A:%.*]], i32 [[B:%.*]], i32 [[C:%.*]], i32 [[D:%.*]]) local_unnamed_addr {
15+
; CHECK-NEXT: store i32 [[A]], ptr @params.0, align 32
16+
; CHECK-NEXT: store i32 [[B]], ptr @params.1, align 4
17+
; CHECK-NEXT: store i32 [[C]], ptr @params.2, align 8
18+
; CHECK-NEXT: store i32 [[D]], ptr @params.3, align 4
19+
; CHECK-NEXT: ret void
20+
;
21+
store i32 %a, ptr @params
22+
store i32 %b, ptr getelementptr ([4 x i32], ptr @params, i32 0, i32 1)
23+
store i32 %c, ptr getelementptr ([4 x i32], ptr @params, i32 0, i32 2)
24+
store i32 %d, ptr getelementptr ([4 x i32], ptr @params, i32 0, i32 3)
25+
ret void
26+
}
27+
28+
%S = type { i32, i32, i32, i32 }
29+
30+
define %S @get() {
31+
; CHECK-LABEL: define %S @get() local_unnamed_addr {
32+
; CHECK-NEXT: [[A:%.*]] = load i32, ptr @params.0, align 32
33+
; CHECK-NEXT: [[SA:%.*]] = insertvalue [[S:%.*]] undef, i32 [[A]], 0
34+
; CHECK-NEXT: [[B:%.*]] = load i32, ptr @params.1, align 4
35+
; CHECK-NEXT: [[SB:%.*]] = insertvalue [[S]] [[SA]], i32 [[B]], 1
36+
; CHECK-NEXT: [[C:%.*]] = load i32, ptr @params.2, align 8
37+
; CHECK-NEXT: [[SC:%.*]] = insertvalue [[S]] [[SB]], i32 [[C]], 2
38+
; CHECK-NEXT: [[D:%.*]] = load i32, ptr @params.3, align 4
39+
; CHECK-NEXT: [[SD:%.*]] = insertvalue [[S]] [[SC]], i32 [[D]], 3
40+
; CHECK-NEXT: ret [[S]] [[SD]]
41+
;
42+
%a = load i32, ptr @params
43+
%sa = insertvalue %S undef, i32 %a, 0
44+
%b = load i32, ptr getelementptr ([4 x i32], ptr @params, i32 0, i32 1)
45+
%sb = insertvalue %S %sa, i32 %b, 1
46+
%c = load i32, ptr getelementptr ([4 x i32], ptr @params, i32 0, i32 2)
47+
%sc = insertvalue %S %sb, i32 %c, 2
48+
%d = load i32, ptr getelementptr ([4 x i32], ptr @params, i32 0, i32 3)
49+
%sd = insertvalue %S %sc, i32 %d, 3
50+
ret %S %sd
51+
}

0 commit comments

Comments
 (0)