Skip to content

[AArch64] Do not promote scalable constants to global variables #146926

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 7, 2025

Conversation

momchil-velikov
Copy link
Collaborator

Following 878d359
IREE/MLIR started generating values like

    [<vscale x 4 x float> zeroinitializer, <vscale x 4 x float> poison]

which then LLVM promoted to global variables in AArch64PromoteConstant pass.
This patch prevents it by explicitly checking the type of the promotion candidate.

@llvmbot
Copy link
Member

llvmbot commented Jul 3, 2025

@llvm/pr-subscribers-backend-aarch64

Author: Momchil Velikov (momchil-velikov)

Changes

Following 878d359
IREE/MLIR started generating values like

    [&lt;vscale x 4 x float&gt; zeroinitializer, &lt;vscale x 4 x float&gt; poison]

which then LLVM promoted to global variables in AArch64PromoteConstant pass.
This patch prevents it by explicitly checking the type of the promotion candidate.


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

2 Files Affected:

  • (modified) llvm/lib/Target/AArch64/AArch64PromoteConstant.cpp (+4)
  • (added) llvm/test/CodeGen/AArch64/no-promote-scalabale-const-to-global.ll (+20)
diff --git a/llvm/lib/Target/AArch64/AArch64PromoteConstant.cpp b/llvm/lib/Target/AArch64/AArch64PromoteConstant.cpp
index 8edf1d0f9296b..3f45d55063b50 100644
--- a/llvm/lib/Target/AArch64/AArch64PromoteConstant.cpp
+++ b/llvm/lib/Target/AArch64/AArch64PromoteConstant.cpp
@@ -345,6 +345,10 @@ static bool shouldConvertImpl(const Constant *Cst) {
   if (Cst->isZeroValue())
     return false;
 
+  // Globals cannot be or contain scalable vectors.
+  if (Cst->getType()->isScalableTy())
+    return false;
+
   if (Stress)
     return true;
 
diff --git a/llvm/test/CodeGen/AArch64/no-promote-scalabale-const-to-global.ll b/llvm/test/CodeGen/AArch64/no-promote-scalabale-const-to-global.ll
new file mode 100644
index 0000000000000..db7d26b6ed139
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/no-promote-scalabale-const-to-global.ll
@@ -0,0 +1,20 @@
+; RUN: llc -mtriple=aarch64 -mattr=+sve -stop-after=aarch64-promote-const < %s | FileCheck %s
+
+; Test that the constant inside the `phi` is not promoted to a global
+; CHECK-NOT: _PromotedConst
+define void @f(i1 %c, ptr %p, ptr %q) {
+entry:
+  br i1 %c, label %if.then, label %if.else
+
+if.then:
+  %u = load [2 x <vscale x 4 x float> ], ptr %p
+  br label %exit
+
+if.else:
+  br label %exit
+
+exit:
+  %v = phi [2 x <vscale x 4 x float> ]  [ %u, %if.then], [[<vscale x 4 x float> zeroinitializer, <vscale x 4 x float> poison], %if.else]
+  store [2 x <vscale x 4 x float>] %v, ptr %q
+  ret void
+}

@momchil-velikov momchil-velikov force-pushed the users/momchil-velikov/no-scalable-globals branch from 2fb735a to 88bd451 Compare July 4, 2025 13:42
@momchil-velikov momchil-velikov force-pushed the users/momchil-velikov/no-scalable-globals branch from 88bd451 to f7a9d95 Compare July 4, 2025 14:39
Copy link
Contributor

@banach-space banach-space left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@momchil-velikov momchil-velikov merged commit c50415b into main Jul 7, 2025
9 checks passed
@momchil-velikov momchil-velikov deleted the users/momchil-velikov/no-scalable-globals branch July 7, 2025 08:17
Max191 pushed a commit to iree-org/llvm-project that referenced this pull request Jul 7, 2025
…#146926)

Following
llvm@878d359
IREE/MLIR started generating values like

    [<vscale x 4 x float> zeroinitializer, <vscale x 4 x float> poison]

which then LLVM promoted to global variables in `AArch64PromoteConstant` pass.
This patch prevents it by explicitly checking the type of the promotion candidate.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants