[llvm] [GlobalOpt] Fix global SRA incorrect alignment on some elements (PR #115328)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Bruno De Fraine (brunodf-snps)

<details>
<summary>Changes</summary>

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

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


4 Files Affected:

- (modified) llvm/lib/Transforms/IPO/GlobalOpt.cpp (+4-3) 
- (modified) llvm/test/DebugInfo/X86/global-sra-struct-fit-segment.ll (+1-1) 
- (modified) llvm/test/DebugInfo/X86/global-sra-struct-part-overlap-segment.ll (+1-1) 
- (modified) llvm/test/Transforms/GlobalOpt/globalsra-align.ll (+4-4) 


``````````diff
diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
index 4647c65a5c850f..9aaa0212fc8445 100644
--- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -575,15 +575,16 @@ static GlobalVariable *SRAGlobal(GlobalVariable *GV, const DataLayout &DL) {
         *GV->getParent(), Ty, false, GlobalVariable::InternalLinkage,
         Initializer, GV->getName() + "." + Twine(NameSuffix++), GV,
         GV->getThreadLocalMode(), GV->getAddressSpace());
+    // Start out by copying attributes from the original, including alignment.
     NGV->copyAttributesFrom(GV);
     NewGlobals.insert({OffsetForTy, NGV});
 
     // Calculate the known alignment of the field.  If the original aggregate
-    // had 256 byte alignment for example, something might depend on that:
+    // had 256 byte alignment for example, then the element at a given offset
+    // may also have a known alignment, and something might depend on that:
     // propagate info to each field.
     Align NewAlign = commonAlignment(StartAlignment, OffsetForTy);
-    if (NewAlign > DL.getABITypeAlign(Ty))
-      NGV->setAlignment(NewAlign);
+    NGV->setAlignment(std::max(NewAlign, DL.getABITypeAlign(Ty)));
 
     // Copy over the debug info for the variable.
     transferSRADebugInfo(GV, NGV, OffsetForTy * 8,
diff --git a/llvm/test/DebugInfo/X86/global-sra-struct-fit-segment.ll b/llvm/test/DebugInfo/X86/global-sra-struct-fit-segment.ll
index 94b7deec95f411..ab48ef2411f35c 100644
--- a/llvm/test/DebugInfo/X86/global-sra-struct-fit-segment.ll
+++ b/llvm/test/DebugInfo/X86/global-sra-struct-fit-segment.ll
@@ -20,7 +20,7 @@
 %struct.BSS1 = type <{ [12 x i8] }>
 
 ;CHECK: @.BSS1.0 = internal unnamed_addr global i32 0, align 32, !dbg ![[GVE1:.*]]
-;CHECK: @.BSS1.1 = internal unnamed_addr global i32 0, align 32, !dbg ![[GVE2:.*]], !dbg ![[GVE4:.*]]
+;CHECK: @.BSS1.1 = internal unnamed_addr global i32 0, align 4, !dbg ![[GVE2:.*]], !dbg ![[GVE4:.*]]
 ;CHECK: @.BSS1.2 = internal unnamed_addr global i32 0, align 8, !dbg ![[GVE3:.*]]
 
 @.BSS1 = internal global %struct.BSS1 zeroinitializer, align 32, !dbg !0, !dbg !7, !dbg !10, !dbg !27, !dbg !29
diff --git a/llvm/test/DebugInfo/X86/global-sra-struct-part-overlap-segment.ll b/llvm/test/DebugInfo/X86/global-sra-struct-part-overlap-segment.ll
index d7854cb822e3e9..9dd865edbbdadd 100644
--- a/llvm/test/DebugInfo/X86/global-sra-struct-part-overlap-segment.ll
+++ b/llvm/test/DebugInfo/X86/global-sra-struct-part-overlap-segment.ll
@@ -26,7 +26,7 @@
 
 @.BSS3 = internal unnamed_addr global %struct.BSS3 zeroinitializer, align 32, !dbg !0, !dbg !7, !dbg !29
 ;CHECK: @.BSS3.0 = internal unnamed_addr global double 0.000000e+00, align 32, !dbg ![[GVE1:.*]], !dbg ![[GVE2:.*]]
-;CHECK: @.BSS3.1 = internal unnamed_addr global double 0.000000e+00, align 32, !dbg ![[GVE3:.*]], !dbg ![[GVE4:.*]], !dbg ![[GVE6:.*]]
+;CHECK: @.BSS3.1 = internal unnamed_addr global double 0.000000e+00, align 8, !dbg ![[GVE3:.*]], !dbg ![[GVE4:.*]], !dbg ![[GVE6:.*]]
 ;CHECK: @.BSS3.2 = internal unnamed_addr global double 0.000000e+00, align 16, !dbg ![[GVE5:.*]]
 
 @.C363_mymod_bar_ = internal constant [2 x i8] c"IF"
diff --git a/llvm/test/Transforms/GlobalOpt/globalsra-align.ll b/llvm/test/Transforms/GlobalOpt/globalsra-align.ll
index aadd5b866a8224..74ccb3f279ed09 100644
--- a/llvm/test/Transforms/GlobalOpt/globalsra-align.ll
+++ b/llvm/test/Transforms/GlobalOpt/globalsra-align.ll
@@ -15,9 +15,9 @@ target datalayout = "p:16:32:64" ; 16-bit pointers with 32-bit ABI alignment and
 
 ;.
 ; CHECK: @[[A_4:[a-zA-Z0-9_$"\\.-]+]] = internal unnamed_addr externally_initialized global ptr null, align 8
-; CHECK: @[[A_5:[a-zA-Z0-9_$"\\.-]+]] = internal unnamed_addr externally_initialized global ptr null, align 16
+; CHECK: @[[A_5:[a-zA-Z0-9_$"\\.-]+]] = internal unnamed_addr externally_initialized global ptr null, align 8
 ; CHECK: @[[A_6:[a-zA-Z0-9_$"\\.-]+]] = internal unnamed_addr externally_initialized global ptr null, align 16
-; CHECK: @[[A_7:[a-zA-Z0-9_$"\\.-]+]] = internal unnamed_addr externally_initialized global ptr null, align 16
+; CHECK: @[[A_7:[a-zA-Z0-9_$"\\.-]+]] = internal unnamed_addr externally_initialized global ptr null, align 8
 ;.
 define ptr @reduce_align_0() {
 ; CHECK-LABEL: @reduce_align_0(
@@ -31,7 +31,7 @@ define ptr @reduce_align_0() {
 
 define ptr @reduce_align_1() {
 ; CHECK-LABEL: @reduce_align_1(
-; CHECK-NEXT:    [[X:%.*]] = load ptr, ptr @a.5, align 16
+; CHECK-NEXT:    [[X:%.*]] = load ptr, ptr @a.5, align 8
 ; CHECK-NEXT:    ret ptr [[X]]
 ;
   %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() {
 
 define ptr @reduce_align_3() {
 ; CHECK-LABEL: @reduce_align_3(
-; CHECK-NEXT:    [[X:%.*]] = load ptr, ptr @a.7, align 16
+; CHECK-NEXT:    [[X:%.*]] = load ptr, ptr @a.7, align 8
 ; CHECK-NEXT:    ret ptr [[X]]
 ;
   %x = load ptr, ptr getelementptr inbounds ([3 x [7 x ptr]], ptr @a, i64 0, i64 2, i64 3), align 4

``````````

</details>


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