[IR] Require that global value initializers are sized #137358
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
While external globals can be unsized, I don't think an unsized initializer makes sense. It seems like the backend currently ends up treating this as a zero-size global. If people want that behavior, they should declare it as such.
|
@llvm/pr-subscribers-llvm-ir Author: Nikita Popov (nikic) ChangesWhile external globals can be unsized, I don't think an unsized initializer makes sense. It seems like the backend currently ends up treating this as a zero-size global. If people want that behavior, they should declare it as such. Full diff: https://github.com/llvm/llvm-project/pull/137358.diff 6 Files Affected:
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 5bd1d29487139..f8620bf08151d 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -700,7 +700,7 @@ Global Variables
Global variables define regions of memory allocated at compilation time
instead of run-time.
-Global variable definitions must be initialized.
+Global variable definitions must be initialized with a sized value.
Global variables in other translation units can also be declared, in which
case they don't have an initializer.
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 84054ddd0298c..968ae464b2e70 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -835,6 +835,8 @@ void Verifier::visitGlobalVariable(const GlobalVariable &GV) {
"Global variable initializer type does not match global "
"variable type!",
&GV);
+ Check(GV.getInitializer()->getType()->isSized(),
+ "Global variable initializer must be sized", &GV);
// If the global has common linkage, it must have a zero initializer and
// cannot be constant.
if (GV.hasCommonLinkage()) {
diff --git a/llvm/test/Assembler/2005-05-05-OpaqueUndefValues.ll b/llvm/test/Assembler/2005-05-05-OpaqueUndefValues.ll
index 01456f10c9dc1..e2fdb53058e74 100644
--- a/llvm/test/Assembler/2005-05-05-OpaqueUndefValues.ll
+++ b/llvm/test/Assembler/2005-05-05-OpaqueUndefValues.ll
@@ -2,4 +2,8 @@
; RUN: verify-uselistorder %s
%t = type opaque
-@x = global %t undef
+
+define void @test() {
+ %sel = select i1 true, %t undef, %t poison
+ ret void
+}
diff --git a/llvm/test/Transforms/InstSimplify/gv-alloca-cmp.ll b/llvm/test/Transforms/InstSimplify/gv-alloca-cmp.ll
index 4f2544052b215..f30c6379e47e1 100644
--- a/llvm/test/Transforms/InstSimplify/gv-alloca-cmp.ll
+++ b/llvm/test/Transforms/InstSimplify/gv-alloca-cmp.ll
@@ -46,7 +46,7 @@ define i1 @cmp_gv_weak_alloca() {
}
%opaque = type opaque
-@gv_unsized = weak global %opaque zeroinitializer, align 16
+@gv_unsized = external global %opaque, align 16
define i1 @cmp_gv_unsized_alloca() {
; CHECK-LABEL: define i1 @cmp_gv_unsized_alloca() {
diff --git a/llvm/test/Verifier/global-initializer-sized.ll b/llvm/test/Verifier/global-initializer-sized.ll
new file mode 100644
index 0000000000000..90c2f95f4611a
--- /dev/null
+++ b/llvm/test/Verifier/global-initializer-sized.ll
@@ -0,0 +1,6 @@
+; RUN: not llvm-as < %s 2>&1 | FileCheck %s
+
+%t = type opaque
+@g = global %t undef
+
+; CHECK: Global variable initializer must be sized
diff --git a/llvm/test/Verifier/scalable-global-vars.ll b/llvm/test/Verifier/scalable-global-vars.ll
index fb9a3067acba9..9df644d5e10f0 100644
--- a/llvm/test/Verifier/scalable-global-vars.ll
+++ b/llvm/test/Verifier/scalable-global-vars.ll
@@ -13,7 +13,7 @@
; CHECK-NEXT: Globals cannot contain scalable types
; CHECK-NEXT: ptr @ScalableVecStructGlobal
-@ScalableVecStructGlobal = global { i32, <vscale x 4 x i32> } zeroinitializer
+@ScalableVecStructGlobal = external global { i32, <vscale x 4 x i32> }
; CHECK-NEXT: Globals cannot contain scalable types
; CHECK-NEXT: ptr @StructTestGlobal
@@ -23,9 +23,9 @@
; CHECK-NEXT: Globals cannot contain scalable types
; CHECK-NEXT: ptr @StructArrayTestGlobal
%struct.array.test = type { [2 x <vscale x 1 x double>] }
-@StructArrayTestGlobal = global %struct.array.test zeroinitializer
+@StructArrayTestGlobal = external global %struct.array.test
; CHECK-NEXT: Globals cannot contain scalable types
; CHECK-NEXT: ptr @StructTargetTestGlobal
%struct.target.test = type { target("aarch64.svcount"), target("aarch64.svcount") }
-@StructTargetTestGlobal = global %struct.target.test zeroinitializer
+@StructTargetTestGlobal = external global %struct.target.test
|
|
@llvm/pr-subscribers-llvm-transforms Author: Nikita Popov (nikic) ChangesWhile external globals can be unsized, I don't think an unsized initializer makes sense. It seems like the backend currently ends up treating this as a zero-size global. If people want that behavior, they should declare it as such. Full diff: https://github.com/llvm/llvm-project/pull/137358.diff 6 Files Affected:
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 5bd1d29487139..f8620bf08151d 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -700,7 +700,7 @@ Global Variables
Global variables define regions of memory allocated at compilation time
instead of run-time.
-Global variable definitions must be initialized.
+Global variable definitions must be initialized with a sized value.
Global variables in other translation units can also be declared, in which
case they don't have an initializer.
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 84054ddd0298c..968ae464b2e70 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -835,6 +835,8 @@ void Verifier::visitGlobalVariable(const GlobalVariable &GV) {
"Global variable initializer type does not match global "
"variable type!",
&GV);
+ Check(GV.getInitializer()->getType()->isSized(),
+ "Global variable initializer must be sized", &GV);
// If the global has common linkage, it must have a zero initializer and
// cannot be constant.
if (GV.hasCommonLinkage()) {
diff --git a/llvm/test/Assembler/2005-05-05-OpaqueUndefValues.ll b/llvm/test/Assembler/2005-05-05-OpaqueUndefValues.ll
index 01456f10c9dc1..e2fdb53058e74 100644
--- a/llvm/test/Assembler/2005-05-05-OpaqueUndefValues.ll
+++ b/llvm/test/Assembler/2005-05-05-OpaqueUndefValues.ll
@@ -2,4 +2,8 @@
; RUN: verify-uselistorder %s
%t = type opaque
-@x = global %t undef
+
+define void @test() {
+ %sel = select i1 true, %t undef, %t poison
+ ret void
+}
diff --git a/llvm/test/Transforms/InstSimplify/gv-alloca-cmp.ll b/llvm/test/Transforms/InstSimplify/gv-alloca-cmp.ll
index 4f2544052b215..f30c6379e47e1 100644
--- a/llvm/test/Transforms/InstSimplify/gv-alloca-cmp.ll
+++ b/llvm/test/Transforms/InstSimplify/gv-alloca-cmp.ll
@@ -46,7 +46,7 @@ define i1 @cmp_gv_weak_alloca() {
}
%opaque = type opaque
-@gv_unsized = weak global %opaque zeroinitializer, align 16
+@gv_unsized = external global %opaque, align 16
define i1 @cmp_gv_unsized_alloca() {
; CHECK-LABEL: define i1 @cmp_gv_unsized_alloca() {
diff --git a/llvm/test/Verifier/global-initializer-sized.ll b/llvm/test/Verifier/global-initializer-sized.ll
new file mode 100644
index 0000000000000..90c2f95f4611a
--- /dev/null
+++ b/llvm/test/Verifier/global-initializer-sized.ll
@@ -0,0 +1,6 @@
+; RUN: not llvm-as < %s 2>&1 | FileCheck %s
+
+%t = type opaque
+@g = global %t undef
+
+; CHECK: Global variable initializer must be sized
diff --git a/llvm/test/Verifier/scalable-global-vars.ll b/llvm/test/Verifier/scalable-global-vars.ll
index fb9a3067acba9..9df644d5e10f0 100644
--- a/llvm/test/Verifier/scalable-global-vars.ll
+++ b/llvm/test/Verifier/scalable-global-vars.ll
@@ -13,7 +13,7 @@
; CHECK-NEXT: Globals cannot contain scalable types
; CHECK-NEXT: ptr @ScalableVecStructGlobal
-@ScalableVecStructGlobal = global { i32, <vscale x 4 x i32> } zeroinitializer
+@ScalableVecStructGlobal = external global { i32, <vscale x 4 x i32> }
; CHECK-NEXT: Globals cannot contain scalable types
; CHECK-NEXT: ptr @StructTestGlobal
@@ -23,9 +23,9 @@
; CHECK-NEXT: Globals cannot contain scalable types
; CHECK-NEXT: ptr @StructArrayTestGlobal
%struct.array.test = type { [2 x <vscale x 1 x double>] }
-@StructArrayTestGlobal = global %struct.array.test zeroinitializer
+@StructArrayTestGlobal = external global %struct.array.test
; CHECK-NEXT: Globals cannot contain scalable types
; CHECK-NEXT: ptr @StructTargetTestGlobal
%struct.target.test = type { target("aarch64.svcount"), target("aarch64.svcount") }
-@StructTargetTestGlobal = global %struct.target.test zeroinitializer
+@StructTargetTestGlobal = external global %struct.target.test
|
You can test this locally with the following command:git diff -U0 --pickaxe-regex -S '([^a-zA-Z0-9#_-]undef[^a-zA-Z0-9_-]|UndefValue::get)' 'HEAD~1' HEAD llvm/test/Verifier/global-initializer-sized.ll llvm/lib/IR/Verifier.cpp llvm/test/Assembler/2005-05-05-OpaqueUndefValues.ll llvm/test/Transforms/InstSimplify/gv-alloca-cmp.ll llvm/test/Verifier/scalable-global-vars.llThe following files introduce new uses of undef:
Undef is now deprecated and should only be used in the rare cases where no replacement is possible. For example, a load of uninitialized memory yields In tests, avoid using For example, this is considered a bad practice: define void @fn() {
...
br i1 undef, ...
}Please use the following instead: define void @fn(i1 %cond) {
...
br i1 %cond, ...
}Please refer to the Undefined Behavior Manual for more information. |
| @x = global %t undef | ||
|
|
||
| define void @test() { | ||
| %sel = select i1 true, %t undef, %t poison |
There was a problem hiding this comment.
We should also forbid "select"/"undef"/"poison"/"zeroinitialzer", I think? I don't see any reason to allow writing values with opaque types in any context; it's about as meaningful as a global variable with an opaque type.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While external globals can be unsized, I don't think an unsized initializer makes sense.
It seems like the backend currently ends up treating this as a zero-size global. If people want that behavior, they should declare it as such.