[llvm] [MachineLICM] Do not rely on hasSideEffect when handling impdefs (PR #145379)
Anatoly Trosinenko via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 23 11:24:15 PDT 2025
https://github.com/atrosinenko created https://github.com/llvm/llvm-project/pull/145379
Take clobbered registers described as implicit-def operands into
account when deciding if an instruction can be moved.
When hasSideEffect was set to 0 in the definition of LOADgotAUTH,
MultiSource/Benchmarks/Ptrdist/ks/ks test from llvm-test-suite started
to crash. The issue was traced down to MachineLICM pass placing
LOADgotAUTH right after an unrelated copy to x16 like rewriting this
code:
bb.0:
renamable $x16 = COPY renamable $x12
B %bb.1
bb.1:
...
/* use $x16 */
...
renamable $x20 = LOADgotAUTH target-flags(aarch64-got) @some_variable, implicit-def dead $x16, implicit-def dead $x17, implicit-def dead $nzcv
/* use $x20 */
...
like the following:
bb.0:
renamable $x16 = COPY renamable $x12
renamable $x20 = LOADgotAUTH target-flags(aarch64-got) @some_variable, implicit-def dead $x16, implicit-def dead $x17, implicit-def dead $nzcv
B %bb.1
bb.1:
...
/* use $x16 */
...
/* use $x20 */
...
>From 32735183224c8bcff2ce24a32819890bb1243938 Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko <atrosinenko at accesssoftek.com>
Date: Mon, 23 Jun 2025 20:22:56 +0300
Subject: [PATCH 1/2] [MachineLICM] Precommit tests for implicit-def handling
---
.../CodeGen/AArch64/mlicm-implicit-defs.mir | 53 +++++++++++++++++++
1 file changed, 53 insertions(+)
create mode 100644 llvm/test/CodeGen/AArch64/mlicm-implicit-defs.mir
diff --git a/llvm/test/CodeGen/AArch64/mlicm-implicit-defs.mir b/llvm/test/CodeGen/AArch64/mlicm-implicit-defs.mir
new file mode 100644
index 0000000000000..1cbf80db451f1
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/mlicm-implicit-defs.mir
@@ -0,0 +1,53 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
+# RUN: llc -mtriple=aarch64 -run-pass machinelicm -verify-machineinstrs -o - %s | FileCheck %s
+# RUN: llc -mtriple=aarch64 -passes machinelicm -o - %s | FileCheck %s
+
+---
+name: test
+tracksRegLiveness: true
+body: |
+ ; CHECK-LABEL: name: test
+ ; CHECK: bb.0:
+ ; CHECK-NEXT: successors: %bb.1(0x80000000)
+ ; CHECK-NEXT: liveins: $x0
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: $x16 = COPY killed $x0
+ ; CHECK-NEXT: $x2 = MOVi64imm 1024, implicit-def dead $x16
+ ; CHECK-NEXT: B %bb.1
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.1:
+ ; CHECK-NEXT: successors: %bb.1(0x40000000), %bb.2(0x40000000)
+ ; CHECK-NEXT: liveins: $x16, $x2
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: $x1 = COPY killed $x16
+ ; CHECK-NEXT: $x16 = LDRXroX killed $x1, $x2, 0, 0
+ ; CHECK-NEXT: $xzr = SUBSXri $x16, 0, 0, implicit-def $nzcv
+ ; CHECK-NEXT: Bcc 1, %bb.1, implicit $nzcv
+ ; CHECK-NEXT: B %bb.2
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.2:
+ ; CHECK-NEXT: liveins: $x1
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: $x0 = COPY killed $x1
+ ; CHECK-NEXT: RET_ReallyLR
+ bb.0:
+ liveins: $x0
+ $x16 = COPY killed $x0
+ B %bb.1
+
+ bb.1:
+ liveins: $x16
+ $x1 = COPY killed $x16
+ /* MOVi64imm below mimics a pseudo instruction that doesn't have any */
+ /* unmodelled side effects, but uses x16 as a scratch register. */
+ $x2 = MOVi64imm 1024, implicit-def dead $x16
+ $x16 = LDRXroX killed $x1, killed $x2, 0, 0
+ $xzr = SUBSXri $x16, 0, 0, implicit-def $nzcv
+ Bcc 1, %bb.1, implicit $nzcv
+ B %bb.2
+
+ bb.2:
+ liveins: $x1
+ $x0 = COPY killed $x1
+ RET_ReallyLR
+...
>From 05c3da95de35527ae56f187165de4828007876a8 Mon Sep 17 00:00:00 2001
From: Anatoly Trosinenko <atrosinenko at accesssoftek.com>
Date: Tue, 10 Jun 2025 18:45:50 +0300
Subject: [PATCH 2/2] [MachineLICM] Do not rely on hasSideEffect when handling
impdefs
Take clobbered registers described as implicit-def operands into
account when deciding if an instruction can be moved.
When hasSideEffect was set to 0 in the definition of LOADgotAUTH,
MultiSource/Benchmarks/Ptrdist/ks/ks test from llvm-test-suite started
to crash. The issue was traced down to MachineLICM pass placing
LOADgotAUTH right after an unrelated copy to x16 like rewriting this
code:
bb.0:
renamable $x16 = COPY renamable $x12
B %bb.1
bb.1:
...
/* use $x16 */
...
renamable $x20 = LOADgotAUTH target-flags(aarch64-got) @some_variable, implicit-def dead $x16, implicit-def dead $x17, implicit-def dead $nzcv
/* use $x20 */
...
like the following:
bb.0:
renamable $x16 = COPY renamable $x12
renamable $x20 = LOADgotAUTH target-flags(aarch64-got) @some_variable, implicit-def dead $x16, implicit-def dead $x17, implicit-def dead $nzcv
B %bb.1
bb.1:
...
/* use $x16 */
...
/* use $x20 */
...
---
llvm/lib/CodeGen/MachineLICM.cpp | 17 +++++++----------
.../CodeGen/AArch64/mlicm-implicit-defs.mir | 6 +++---
2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/llvm/lib/CodeGen/MachineLICM.cpp b/llvm/lib/CodeGen/MachineLICM.cpp
index c9079170ca575..5841a9ffa7a99 100644
--- a/llvm/lib/CodeGen/MachineLICM.cpp
+++ b/llvm/lib/CodeGen/MachineLICM.cpp
@@ -559,18 +559,15 @@ void MachineLICMImpl::ProcessMI(MachineInstr *MI, BitVector &RUDefs,
if (!MO.isDead())
// Non-dead implicit def? This cannot be hoisted.
RuledOut = true;
- // No need to check if a dead implicit def is also defined by
- // another instruction.
- continue;
+ } else {
+ // FIXME: For now, avoid instructions with multiple defs, unless
+ // it's a dead implicit def.
+ if (Def)
+ RuledOut = true;
+ else
+ Def = Reg;
}
- // FIXME: For now, avoid instructions with multiple defs, unless
- // it's a dead implicit def.
- if (Def)
- RuledOut = true;
- else
- Def = Reg;
-
// If we have already seen another instruction that defines the same
// register, then this is not safe. Two defs is indicated by setting a
// PhysRegClobbers bit.
diff --git a/llvm/test/CodeGen/AArch64/mlicm-implicit-defs.mir b/llvm/test/CodeGen/AArch64/mlicm-implicit-defs.mir
index 1cbf80db451f1..51cb35116dc62 100644
--- a/llvm/test/CodeGen/AArch64/mlicm-implicit-defs.mir
+++ b/llvm/test/CodeGen/AArch64/mlicm-implicit-defs.mir
@@ -12,15 +12,15 @@ body: |
; CHECK-NEXT: liveins: $x0
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: $x16 = COPY killed $x0
- ; CHECK-NEXT: $x2 = MOVi64imm 1024, implicit-def dead $x16
; CHECK-NEXT: B %bb.1
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.1:
; CHECK-NEXT: successors: %bb.1(0x40000000), %bb.2(0x40000000)
- ; CHECK-NEXT: liveins: $x16, $x2
+ ; CHECK-NEXT: liveins: $x16
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: $x1 = COPY killed $x16
- ; CHECK-NEXT: $x16 = LDRXroX killed $x1, $x2, 0, 0
+ ; CHECK-NEXT: $x2 = MOVi64imm 1024, implicit-def dead $x16
+ ; CHECK-NEXT: $x16 = LDRXroX killed $x1, killed $x2, 0, 0
; CHECK-NEXT: $xzr = SUBSXri $x16, 0, 0, implicit-def $nzcv
; CHECK-NEXT: Bcc 1, %bb.1, implicit $nzcv
; CHECK-NEXT: B %bb.2
More information about the llvm-commits
mailing list