From df21ee2140517f9db64f0ac91ec7e153136520b5 Mon Sep 17 00:00:00 2001 From: Anatoly Trosinenko Date: Mon, 23 Jun 2025 20:22:56 +0300 Subject: [PATCH 1/4] [MachineLICM] Precommit tests for implicit-def handling --- .../CodeGen/AArch64/mlicm-implicit-defs.mir | 103 ++++++++++++++++++ 1 file changed, 103 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..9c56d647aaeb0 --- /dev/null +++ b/llvm/test/CodeGen/AArch64/mlicm-implicit-defs.mir @@ -0,0 +1,103 @@ +# 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: unsafe_to_move +tracksRegLiveness: true +body: | + ; CHECK-LABEL: name: unsafe_to_move + ; 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 +... + +--- +name: dead_implicit_def +tracksRegLiveness: true +body: | + ; CHECK-LABEL: name: dead_implicit_def + ; CHECK: bb.0: + ; CHECK-NEXT: successors: %bb.1(0x80000000) + ; CHECK-NEXT: liveins: $x0 + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: $x12 = COPY killed $x0 + ; CHECK-NEXT: B %bb.1 + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: bb.1: + ; CHECK-NEXT: successors: %bb.1(0x40000000), %bb.2(0x40000000) + ; CHECK-NEXT: liveins: $x12 + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: $x1 = COPY killed $x12 + ; 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 + ; 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 + $x12 = COPY killed $x0 + B %bb.1 + + bb.1: + liveins: $x12 + $x1 = COPY killed $x12 + /* 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 182a4903f90219ab4c3768e4f19f13aa8d53d6a1 Mon Sep 17 00:00:00 2001 From: Anatoly Trosinenko Date: Tue, 10 Jun 2025 18:45:50 +0300 Subject: [PATCH 2/4] [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 9c56d647aaeb0..d2ec63dd4e2b0 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 From 707a36c02ec15296411d8c631cf15498f9a89614 Mon Sep 17 00:00:00 2001 From: Anatoly Trosinenko Date: Thu, 26 Jun 2025 14:18:07 +0300 Subject: [PATCH 3/4] Fix hoisting safe dead implicit-defs --- llvm/lib/CodeGen/MachineLICM.cpp | 4 ++-- llvm/test/CodeGen/AArch64/mlicm-implicit-defs.mir | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/llvm/lib/CodeGen/MachineLICM.cpp b/llvm/lib/CodeGen/MachineLICM.cpp index 5841a9ffa7a99..dd0237528c8ae 100644 --- a/llvm/lib/CodeGen/MachineLICM.cpp +++ b/llvm/lib/CodeGen/MachineLICM.cpp @@ -554,8 +554,6 @@ void MachineLICMImpl::ProcessMI(MachineInstr *MI, BitVector &RUDefs, } if (MO.isImplicit()) { - for (MCRegUnit Unit : TRI->regunits(Reg)) - RUClobbers.set(Unit); if (!MO.isDead()) // Non-dead implicit def? This cannot be hoisted. RuledOut = true; @@ -582,6 +580,8 @@ void MachineLICMImpl::ProcessMI(MachineInstr *MI, BitVector &RUDefs, } RUDefs.set(Unit); + if (MO.isImplicit()) + RUClobbers.set(Unit); } } diff --git a/llvm/test/CodeGen/AArch64/mlicm-implicit-defs.mir b/llvm/test/CodeGen/AArch64/mlicm-implicit-defs.mir index d2ec63dd4e2b0..2c5a70288cfad 100644 --- a/llvm/test/CodeGen/AArch64/mlicm-implicit-defs.mir +++ b/llvm/test/CodeGen/AArch64/mlicm-implicit-defs.mir @@ -62,15 +62,15 @@ body: | ; CHECK-NEXT: liveins: $x0 ; CHECK-NEXT: {{ $}} ; CHECK-NEXT: $x12 = 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: $x12 + ; CHECK-NEXT: liveins: $x12, $x2 ; CHECK-NEXT: {{ $}} ; CHECK-NEXT: $x1 = COPY killed $x12 - ; CHECK-NEXT: $x2 = MOVi64imm 1024, implicit-def dead $x16 - ; CHECK-NEXT: $x16 = LDRXroX killed $x1, killed $x2, 0, 0 + ; 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 From 4c29db17a57236be23c864ce0e02db26af405a0f Mon Sep 17 00:00:00 2001 From: Anatoly Trosinenko Date: Thu, 3 Jul 2025 18:27:20 +0300 Subject: [PATCH 4/4] Drop useless corner case --- llvm/lib/CodeGen/MachineLICM.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/llvm/lib/CodeGen/MachineLICM.cpp b/llvm/lib/CodeGen/MachineLICM.cpp index dd0237528c8ae..1544298ec587d 100644 --- a/llvm/lib/CodeGen/MachineLICM.cpp +++ b/llvm/lib/CodeGen/MachineLICM.cpp @@ -580,8 +580,6 @@ void MachineLICMImpl::ProcessMI(MachineInstr *MI, BitVector &RUDefs, } RUDefs.set(Unit); - if (MO.isImplicit()) - RUClobbers.set(Unit); } }