Skip to content

Commit 05c3da9

Browse files
committed
[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 */ ...
1 parent 3273518 commit 05c3da9

File tree

2 files changed

+10
-13
lines changed

2 files changed

+10
-13
lines changed

llvm/lib/CodeGen/MachineLICM.cpp

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -559,18 +559,15 @@ void MachineLICMImpl::ProcessMI(MachineInstr *MI, BitVector &RUDefs,
559559
if (!MO.isDead())
560560
// Non-dead implicit def? This cannot be hoisted.
561561
RuledOut = true;
562-
// No need to check if a dead implicit def is also defined by
563-
// another instruction.
564-
continue;
562+
} else {
563+
// FIXME: For now, avoid instructions with multiple defs, unless
564+
// it's a dead implicit def.
565+
if (Def)
566+
RuledOut = true;
567+
else
568+
Def = Reg;
565569
}
566570

567-
// FIXME: For now, avoid instructions with multiple defs, unless
568-
// it's a dead implicit def.
569-
if (Def)
570-
RuledOut = true;
571-
else
572-
Def = Reg;
573-
574571
// If we have already seen another instruction that defines the same
575572
// register, then this is not safe. Two defs is indicated by setting a
576573
// PhysRegClobbers bit.

llvm/test/CodeGen/AArch64/mlicm-implicit-defs.mir

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,15 @@ body: |
1212
; CHECK-NEXT: liveins: $x0
1313
; CHECK-NEXT: {{ $}}
1414
; CHECK-NEXT: $x16 = COPY killed $x0
15-
; CHECK-NEXT: $x2 = MOVi64imm 1024, implicit-def dead $x16
1615
; CHECK-NEXT: B %bb.1
1716
; CHECK-NEXT: {{ $}}
1817
; CHECK-NEXT: bb.1:
1918
; CHECK-NEXT: successors: %bb.1(0x40000000), %bb.2(0x40000000)
20-
; CHECK-NEXT: liveins: $x16, $x2
19+
; CHECK-NEXT: liveins: $x16
2120
; CHECK-NEXT: {{ $}}
2221
; CHECK-NEXT: $x1 = COPY killed $x16
23-
; CHECK-NEXT: $x16 = LDRXroX killed $x1, $x2, 0, 0
22+
; CHECK-NEXT: $x2 = MOVi64imm 1024, implicit-def dead $x16
23+
; CHECK-NEXT: $x16 = LDRXroX killed $x1, killed $x2, 0, 0
2424
; CHECK-NEXT: $xzr = SUBSXri $x16, 0, 0, implicit-def $nzcv
2525
; CHECK-NEXT: Bcc 1, %bb.1, implicit $nzcv
2626
; CHECK-NEXT: B %bb.2

0 commit comments

Comments
 (0)