From 06c412f40f717506e3122ed860827a8f03eb32f4 Mon Sep 17 00:00:00 2001 From: pvanhout Date: Fri, 21 Feb 2025 13:55:16 +0100 Subject: [PATCH 1/5] [Attributor] Do not optimize away externally_initialized loads. Fixes SWDEV-515029 --- llvm/lib/Transforms/IPO/Attributor.cpp | 3 ++- .../Transforms/Attributor/value-simplify.ll | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp index a1e1a51b201b0..0865e4da1a071 100644 --- a/llvm/lib/Transforms/IPO/Attributor.cpp +++ b/llvm/lib/Transforms/IPO/Attributor.cpp @@ -260,7 +260,8 @@ AA::getInitialValueForObj(Attributor &A, const AbstractAttribute &QueryingAA, return nullptr; } else { if (!GV->hasLocalLinkage() && - (GV->isInterposable() || !(GV->isConstant() && GV->hasInitializer()))) + (GV->isInterposable() || GV->isExternallyInitialized() || + !(GV->isConstant() && GV->hasInitializer()))) return nullptr; if (!GV->hasInitializer()) return UndefValue::get(&Ty); diff --git a/llvm/test/Transforms/Attributor/value-simplify.ll b/llvm/test/Transforms/Attributor/value-simplify.ll index a90fb54fbe74a..59160c0834980 100644 --- a/llvm/test/Transforms/Attributor/value-simplify.ll +++ b/llvm/test/Transforms/Attributor/value-simplify.ll @@ -12,6 +12,7 @@ declare ptr @llvm.call.preallocated.arg(token, i32) @ConstPtr = constant i32 0, align 4 @ConstWeakPtr = weak constant i32 0, align 4 @ConstWeakODRPtr = weak_odr constant i32 0, align 4 +@ExtInitZeroInit = externally_initialized constant i32 zeroinitializer, align 4 ;. ; CHECK: @str = private unnamed_addr addrspace(4) constant [1 x i8] zeroinitializer, align 1 @@ -19,6 +20,7 @@ declare ptr @llvm.call.preallocated.arg(token, i32) ; CHECK: @ConstPtr = constant i32 0, align 4 ; CHECK: @ConstWeakPtr = weak constant i32 0, align 4 ; CHECK: @ConstWeakODRPtr = weak_odr constant i32 0, align 4 +; CHECK: @ExtInitZeroInit = externally_initialized constant i32 0, align 4 ; CHECK: @S = external global %struct.X ; CHECK: @g = internal constant { [2 x ptr] } { [2 x ptr] [ptr @f1, ptr @f2] } ; CHECK: @x = external global i32 @@ -1651,6 +1653,23 @@ define i32 @readWeakOdrConst() { ret i32 %l } +define i32 @readExtInitZeroInit() { +; TUNIT: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(none) +; TUNIT-LABEL: define {{[^@]+}}@readExtInitZeroInit +; TUNIT-SAME: () #[[ATTR2]] { +; TUNIT-NEXT: [[L:%.*]] = load i32, ptr @ExtInitZeroInit, align 4 +; TUNIT-NEXT: ret i32 [[L]] +; +; CGSCC: Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(none) +; CGSCC-LABEL: define {{[^@]+}}@readExtInitZeroInit +; CGSCC-SAME: () #[[ATTR1]] { +; CGSCC-NEXT: [[L:%.*]] = load i32, ptr @ExtInitZeroInit, align 4 +; CGSCC-NEXT: ret i32 [[L]] +; + %l = load i32, ptr @ExtInitZeroInit + ret i32 %l +} + ;. ; TUNIT: attributes #[[ATTR0:[0-9]+]] = { nocallback nofree nosync nounwind willreturn } ; TUNIT: attributes #[[ATTR1]] = { memory(readwrite, argmem: none) } From 9ac5ece524825420c4778f2c4ec545015323c6a5 Mon Sep 17 00:00:00 2001 From: pvanhout Date: Fri, 21 Feb 2025 14:41:05 +0100 Subject: [PATCH 2/5] Comments --- llvm/lib/Transforms/IPO/Attributor.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp index 0865e4da1a071..15ea2c8e7330a 100644 --- a/llvm/lib/Transforms/IPO/Attributor.cpp +++ b/llvm/lib/Transforms/IPO/Attributor.cpp @@ -259,12 +259,16 @@ AA::getInitialValueForObj(Attributor &A, const AbstractAttribute &QueryingAA, if (!Initializer) return nullptr; } else { - if (!GV->hasLocalLinkage() && - (GV->isInterposable() || GV->isExternallyInitialized() || - !(GV->isConstant() && GV->hasInitializer()))) - return nullptr; - if (!GV->hasInitializer()) - return UndefValue::get(&Ty); + if (GV->hasLocalLinkage()) { + // uninitialized local variable + if (!GV->hasInitializer()) + return UndefValue::get(&Ty); + } else { + // externally visible variable with uncertain initializer or + // uncertain values. + if (!GV->hasDefinitiveInitializer() || !GV->isConstant()) + return nullptr; + } if (!Initializer) Initializer = GV->getInitializer(); From 4c712ce26705c15f27c1b51d7a3a920a8b9ec054 Mon Sep 17 00:00:00 2001 From: pvanhout Date: Mon, 24 Feb 2025 08:58:57 +0100 Subject: [PATCH 3/5] Commentss --- llvm/lib/Transforms/IPO/Attributor.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp index 15ea2c8e7330a..823aa58fa007b 100644 --- a/llvm/lib/Transforms/IPO/Attributor.cpp +++ b/llvm/lib/Transforms/IPO/Attributor.cpp @@ -260,12 +260,12 @@ AA::getInitialValueForObj(Attributor &A, const AbstractAttribute &QueryingAA, return nullptr; } else { if (GV->hasLocalLinkage()) { - // uninitialized local variable + // Uninitialized global with local linkage. if (!GV->hasInitializer()) return UndefValue::get(&Ty); } else { - // externally visible variable with uncertain initializer or - // uncertain values. + // Externally visible global that's either non-constant, + // or a constant with an uncertain initializer. if (!GV->hasDefinitiveInitializer() || !GV->isConstant()) return nullptr; } From 88bc1821f94b95b5f12d60c040a6287e602c8889 Mon Sep 17 00:00:00 2001 From: pvanhout Date: Mon, 3 Mar 2025 09:29:40 +0100 Subject: [PATCH 4/5] fix check --- llvm/lib/Transforms/IPO/Attributor.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp index 823aa58fa007b..6b1ab4a73821d 100644 --- a/llvm/lib/Transforms/IPO/Attributor.cpp +++ b/llvm/lib/Transforms/IPO/Attributor.cpp @@ -260,9 +260,8 @@ AA::getInitialValueForObj(Attributor &A, const AbstractAttribute &QueryingAA, return nullptr; } else { if (GV->hasLocalLinkage()) { - // Uninitialized global with local linkage. - if (!GV->hasInitializer()) - return UndefValue::get(&Ty); + // Globals with local linkage are always initialized. + assert(GV->hasInitializer()); } else { // Externally visible global that's either non-constant, // or a constant with an uncertain initializer. From 5199b315b136ec2d4f58fe5ca0be5fca4a31ea1e Mon Sep 17 00:00:00 2001 From: pvanhout Date: Mon, 3 Mar 2025 11:40:01 +0100 Subject: [PATCH 5/5] avoid assert only block --- llvm/lib/Transforms/IPO/Attributor.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp index 6b1ab4a73821d..9e9e7689d808a 100644 --- a/llvm/lib/Transforms/IPO/Attributor.cpp +++ b/llvm/lib/Transforms/IPO/Attributor.cpp @@ -259,16 +259,16 @@ AA::getInitialValueForObj(Attributor &A, const AbstractAttribute &QueryingAA, if (!Initializer) return nullptr; } else { - if (GV->hasLocalLinkage()) { - // Globals with local linkage are always initialized. - assert(GV->hasInitializer()); - } else { + if (!GV->hasLocalLinkage()) { // Externally visible global that's either non-constant, // or a constant with an uncertain initializer. if (!GV->hasDefinitiveInitializer() || !GV->isConstant()) return nullptr; } + // Globals with local linkage are always initialized. + assert(!GV->hasLocalLinkage() || GV->hasInitializer()); + if (!Initializer) Initializer = GV->getInitializer(); }