[llvm] [AArch64][CodeGen] Don't try to compute stack addresses in xzr (PR #213009)
Simon Tatham via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 31 01:12:16 PDT 2026
https://github.com/statham-arm updated https://github.com/llvm/llvm-project/pull/213009
>From dd82653eb9404d4a37e8ee704dbef5ee828e14df Mon Sep 17 00:00:00 2001
From: Simon Tatham <simon.tatham at arm.com>
Date: Thu, 30 Jul 2026 10:15:18 +0100
Subject: [PATCH 1/2] [AArch64][CodeGen] Don't try to compute stack addresses
in xzr
Conditional branch tuning can replace an ADD + CBZ with an ADDS + Bcc,
and in the process, rewrite the ADDS destination register to be XZR or
WZR if the result isn't needed for anything other than the comparison.
In unusual cases a stack slot address is computed only to compare it
against zero, so that the ADD in this transformation has a frame index
operand. (The example in #212528 was generated by a fuzzer, but I
could also imagine it happening due to macro expansion.) In this case
it's not always safe to rewrite the destination to XZR, because an
ADDS of that kind can be expanded to multiple instructions (e.g.
because the offset is large, or because the stack frame has variable
size due to SVE vectors). The additional instructions aren't all legal
to use with XZR as the destination, and if they were, their
intermediate results would be zeroed out and the final comparison
wouldn't be checking the intended value.
Fixes #212528.
---
.../Target/AArch64/AArch64CondBrTuning.cpp | 13 +-
llvm/test/CodeGen/AArch64/cmp-frameindex.ll | 2 +-
.../condbr-stack-slot-flag-setting.mir | 149 ++++++++++++++++++
llvm/test/CodeGen/AArch64/large-stack-cmp.ll | 2 +-
4 files changed, 163 insertions(+), 3 deletions(-)
create mode 100644 llvm/test/CodeGen/AArch64/condbr-stack-slot-flag-setting.mir
diff --git a/llvm/lib/Target/AArch64/AArch64CondBrTuning.cpp b/llvm/lib/Target/AArch64/AArch64CondBrTuning.cpp
index 8a8100020d44e..9cc306596ff67 100644
--- a/llvm/lib/Target/AArch64/AArch64CondBrTuning.cpp
+++ b/llvm/lib/Target/AArch64/AArch64CondBrTuning.cpp
@@ -96,7 +96,18 @@ MachineInstr *AArch64CondBrTuning::convertToFlagSetting(MachineInstr &MI,
}
unsigned NewOpc = TII->convertToFlagSettingOpc(MI.getOpcode());
Register NewDestReg = MI.getOperand(0).getReg();
- if (MRI->hasOneNonDBGUse(MI.getOperand(0).getReg()))
+
+ // If the value computed isn't used apart from testing it via the flags, we
+ // can compute it in a zero register. However this isn't safe if the
+ // instruction has a frame index operand. In the common case you end up with
+ // "adds xzr, sp, #constant", i.e. "cmn sp, #constant", but in more difficult
+ // cases the frame index calculation can involve multiple instructions, which
+ // aren't legal with xzr as the destination, and even if they were, the
+ // intermediate values would be zeroed out and the result of the address
+ // calcluation wouldn't be the value you actually wanted to check.
+ if (MRI->hasOneNonDBGUse(MI.getOperand(0).getReg()) &&
+ !any_of(MI.operands(),
+ [](const MachineOperand &Op) { return Op.isFI(); }))
NewDestReg = Is64Bit ? AArch64::XZR : AArch64::WZR;
MachineInstrBuilder MIB = BuildMI(*MI.getParent(), MI, MI.getDebugLoc(),
diff --git a/llvm/test/CodeGen/AArch64/cmp-frameindex.ll b/llvm/test/CodeGen/AArch64/cmp-frameindex.ll
index 186b81ad8b7c3..bbd3e45e35b87 100644
--- a/llvm/test/CodeGen/AArch64/cmp-frameindex.ll
+++ b/llvm/test/CodeGen/AArch64/cmp-frameindex.ll
@@ -7,7 +7,7 @@ define void @test_frameindex_cmp() {
; CHECK-NEXT: str x30, [sp, #-16]! // 8-byte Folded Spill
; CHECK-NEXT: .cfi_def_cfa_offset 16
; CHECK-NEXT: .cfi_offset w30, -16
-; CHECK-NEXT: cmn sp, #12
+; CHECK-NEXT: adds x8, sp, #12
; CHECK-NEXT: b.eq .LBB0_2
; CHECK-NEXT: // %bb.1: // %bb1
; CHECK-NEXT: bl bar
diff --git a/llvm/test/CodeGen/AArch64/condbr-stack-slot-flag-setting.mir b/llvm/test/CodeGen/AArch64/condbr-stack-slot-flag-setting.mir
new file mode 100644
index 0000000000000..5fa96058ed96c
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/condbr-stack-slot-flag-setting.mir
@@ -0,0 +1,149 @@
+# RUN: llc -mtriple=aarch64 -O1 -run-pass=aarch64-cond-br-tuning %s -o - \
+# RUN: | FileCheck %s
+# RUN: llc -mtriple=aarch64 -O1 %s -o - -verify-machineinstrs
+
+# Source: an llvm-reduced version of fuzzer-generated compiler input in
+# https://github.com/llvm/llvm-project/issues/212528
+#
+# The point of this test is to ensure we don't generate an add instruction
+# involving a stack slot with xzr as the destination, e.g.
+#
+# $xzr = ADDSXri %stack.0, 0, 0, implicit-def $nzcv
+#
+# because in this case, since there are SVE registers on the stack, that would
+# expand to a sequence of instructions such as
+#
+# $xzr = ADDXri $sp, 12, 0
+# $xzr = ADDVL_XXI $xzr, 1, implicit $vg
+# $xzr = ADDSXri $xzr, 0, 0, implicit-def $nzcv
+#
+# and none of those is a legal AArch64 instruction (ADD and ADDVL can't target
+# xzr at all, and ADDS can't use it as an input). Also, if they were legal,
+# they wouldn't compute the intended value, since the results of the first two
+# instructions would be thrown away.
+#
+# We check the output of the phase that could have generated this unwanted
+# ADDS, and also check that the full code generation passes
+# -verify-machineinstrs.
+
+--- |
+ ; ModuleID = '<stdin>'
+ source_filename = "<stdin>"
+ target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128-Fn32"
+ target triple = "aarch64"
+
+ define void @launch(i1 %0, ptr %g28, ptr %g10) #0 {
+ %2 = alloca i8, align 4
+ br label %3
+
+ 3: ; preds = %8, %6, %1
+ %4 = phi <2 x i8> [ zeroinitializer, %1 ], [ %5, %8 ], [ zeroinitializer, %6 ]
+ %5 = xor <2 x i8> %4, splat (i8 1)
+ br i1 %0, label %8, label %6
+
+ 6: ; preds = %3
+ %7 = extractelement <2 x i8> %4, i64 0
+ store <4 x i16> splat (i16 1), ptr %g28, align 8
+ br label %3
+
+ 8: ; preds = %3
+ %9 = call ptr @f20()
+ store i8 0, ptr %g10, align 4
+ %10 = icmp eq ptr null, %2
+ br i1 %10, label %3, label %11
+
+ 11: ; preds = %8
+ ret void
+ }
+
+ declare ptr @f20() #1
+
+ attributes #0 = { "frame-pointer"="non-leaf-no-reserve" "target-features"="+sve,+armv9a" }
+ attributes #1 = { "target-features"="+armv9a" }
+...
+---
+name: launch
+alignment: 4
+tracksRegLiveness: true
+noPhis: false
+isSSA: true
+noVRegs: false
+hasFakeUses: false
+registers:
+ - { id: 0, class: fpr64 }
+ - { id: 1, class: fpr64 }
+ - { id: 2, class: gpr32 }
+ - { id: 3, class: gpr64common }
+ - { id: 4, class: gpr64common }
+ - { id: 5, class: gpr32 }
+ - { id: 6, class: fpr64 }
+ - { id: 7, class: fpr128 }
+ - { id: 8, class: zpr }
+ - { id: 9, class: zpr }
+ - { id: 10, class: zpr }
+ - { id: 11, class: fpr64 }
+ - { id: 12, class: fpr64 }
+ - { id: 13, class: fpr128 }
+ - { id: 14, class: gpr64all }
+ - { id: 15, class: gpr32 }
+ - { id: 16, class: gpr64common }
+liveins:
+ - { reg: '$w0', virtual-reg: '%2' }
+ - { reg: '$x1', virtual-reg: '%3' }
+ - { reg: '$x2', virtual-reg: '%4' }
+frameInfo:
+ maxAlignment: 4
+ adjustsStack: true
+ hasCalls: true
+ framePointerPolicy: non-leaf-no-reserve
+ maxCallFrameSize: 0
+ localFrameSize: 4
+stack:
+ - { id: 0, size: 1, alignment: 4, local-offset: -4 }
+machineFunctionInfo: {}
+body: |
+ bb.0 (%ir-block.1):
+ liveins: $w0, $x1, $x2
+
+ %4:gpr64common = COPY $x2
+ %3:gpr64common = COPY $x1
+ %2:gpr32 = COPY $w0
+ %5:gpr32 = COPY %2
+ %7:fpr128 = MOVIv2d_ns 0
+ %6:fpr64 = COPY %7.dsub
+
+ bb.1 (%ir-block.3):
+ %0:fpr64 = PHI %6, %bb.0, %11, %bb.2, %1, %bb.3
+ %9:zpr = IMPLICIT_DEF
+ %8:zpr = INSERT_SUBREG %9, %0, %subreg.dsub
+ %10:zpr = EOR_ZI_PSEUDO killed %8, 0
+ %1:fpr64 = COPY %10.dsub
+ TBNZW %5, 0, %bb.3
+ B %bb.2
+
+ bb.2 (%ir-block.6):
+ %12:fpr64 = MOVIv4i16 1, 0
+ STRDui killed %12, %3, 0 :: (store (s64) into %ir.g28)
+ %13:fpr128 = MOVIv2d_ns 0
+ %11:fpr64 = COPY %13.dsub
+ B %bb.1
+
+ bb.3 (%ir-block.8):
+ successors: %bb.1(0x7c000000), %bb.4(0x04000000)
+
+ ADJCALLSTACKDOWN 0, 0, implicit-def dead $sp, implicit $sp
+ BL @f20, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def $x0
+ ADJCALLSTACKUP 0, 0, implicit-def dead $sp, implicit $sp
+ %15:gpr32 = COPY $wzr
+ STRBBui %15, %4, 0 :: (store (s8) into %ir.g10, align 4)
+ %16:gpr64common = ADDXri %stack.0, 0, 0
+ CBZX killed %16, %bb.1
+ B %bb.4
+
+ ; CHECK: %16:gpr64common = ADDSXri %stack.0, 0, 0, implicit-def $nzcv
+ ; CHECK: Bcc 0, %bb.1, implicit $nzcv
+ ; CHECK: B %bb.4
+
+ bb.4 (%ir-block.11):
+ RET_ReallyLR
+...
diff --git a/llvm/test/CodeGen/AArch64/large-stack-cmp.ll b/llvm/test/CodeGen/AArch64/large-stack-cmp.ll
index 12179d3c944d2..f6174690be701 100644
--- a/llvm/test/CodeGen/AArch64/large-stack-cmp.ll
+++ b/llvm/test/CodeGen/AArch64/large-stack-cmp.ll
@@ -14,7 +14,7 @@ define void @foo() {
; CHECK-NEXT: .cfi_offset w27, -24
; CHECK-NEXT: .cfi_offset w28, -32
; CHECK-NEXT: adds x8, sp, #1, lsl #12 ; =4096
-; CHECK-NEXT: cmn x8, #32
+; CHECK-NEXT: adds x8, x8, #32
; CHECK-NEXT: b.eq LBB0_2
; CHECK-NEXT: ; %bb.1: ; %false
; CHECK-NEXT: bl _baz
>From e0598a3cf0e2ff92541cbdfa559165474c1fc8d3 Mon Sep 17 00:00:00 2001
From: Simon Tatham <simon.tatham at arm.com>
Date: Fri, 31 Jul 2026 09:11:39 +0100
Subject: [PATCH 2/2] Shorten comment
---
llvm/lib/Target/AArch64/AArch64CondBrTuning.cpp | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/Target/AArch64/AArch64CondBrTuning.cpp b/llvm/lib/Target/AArch64/AArch64CondBrTuning.cpp
index 9cc306596ff67..e494451f65167 100644
--- a/llvm/lib/Target/AArch64/AArch64CondBrTuning.cpp
+++ b/llvm/lib/Target/AArch64/AArch64CondBrTuning.cpp
@@ -99,12 +99,8 @@ MachineInstr *AArch64CondBrTuning::convertToFlagSetting(MachineInstr &MI,
// If the value computed isn't used apart from testing it via the flags, we
// can compute it in a zero register. However this isn't safe if the
- // instruction has a frame index operand. In the common case you end up with
- // "adds xzr, sp, #constant", i.e. "cmn sp, #constant", but in more difficult
- // cases the frame index calculation can involve multiple instructions, which
- // aren't legal with xzr as the destination, and even if they were, the
- // intermediate values would be zeroed out and the result of the address
- // calcluation wouldn't be the value you actually wanted to check.
+ // instruction has a frame index operand: that can expand later into multiple
+ // instructions, potentially illegal and calculating the wrong value.
if (MRI->hasOneNonDBGUse(MI.getOperand(0).getReg()) &&
!any_of(MI.operands(),
[](const MachineOperand &Op) { return Op.isFI(); }))
More information about the llvm-commits
mailing list