Skip to content

Commit c2778e3

Browse files
ivafanasAnkur-0429
authored andcommitted
[EarlyIfConverter] Fix reg killed twice after early-if-predicator and ifcvt (llvm#133554)
Bug relates to `early-if-predicator` and `early-ifcvt` passes. If virtual register has "killed" flag in both basic blocks to be merged into head, both instructions in head basic block will have "killed" flag for this register. It makes MIR incorrect. Example: ``` bb.0: ; if ... %0:intregs = COPY $r0 J2_jumpf %2, %bb.2, implicit-def dead $pc J2_jump %bb.1, implicit-def dead $pc bb.1: ; if.then ... S4_storeiri_io killed %0, 0, 1 J2_jump %bb.3, implicit-def dead $pc bb.2: ; if.else ... S4_storeiri_io killed %0, 0, 1 J2_jump %bb.3, implicit-def dead $pc ``` After early-if-predicator will become: ``` bb.0: %0:intregs = COPY $r0 S4_storeirif_io %1, killed %0, 0, 1 S4_storeirit_io %1, killed %0, 0, 1 ``` Having `killed` flag set twice in bb.0 for `%0` is an incorrect MIR.
1 parent acd9fa0 commit c2778e3

File tree

3 files changed

+145
-0
lines changed

3 files changed

+145
-0
lines changed

llvm/lib/CodeGen/EarlyIfConversion.cpp

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
#include "llvm/CodeGen/EarlyIfConversion.h"
1919
#include "llvm/ADT/BitVector.h"
20+
#include "llvm/ADT/DenseSet.h"
2021
#include "llvm/ADT/PostOrderIterator.h"
2122
#include "llvm/ADT/SmallPtrSet.h"
2223
#include "llvm/ADT/SparseSet.h"
@@ -31,6 +32,7 @@
3132
#include "llvm/CodeGen/MachineOptimizationRemarkEmitter.h"
3233
#include "llvm/CodeGen/MachineRegisterInfo.h"
3334
#include "llvm/CodeGen/MachineTraceMetrics.h"
35+
#include "llvm/CodeGen/Register.h"
3436
#include "llvm/CodeGen/TargetInstrInfo.h"
3537
#include "llvm/CodeGen/TargetRegisterInfo.h"
3638
#include "llvm/CodeGen/TargetSubtargetInfo.h"
@@ -163,6 +165,11 @@ class SSAIfConv {
163165
/// Insert selects and rewrite PHI operands to use them.
164166
void rewritePHIOperands();
165167

168+
/// If virtual register has "killed" flag in TBB and FBB basic blocks, remove
169+
/// the flag in TBB instruction.
170+
void clearRepeatedKillFlagsFromTBB(MachineBasicBlock *TBB,
171+
MachineBasicBlock *FBB);
172+
166173
public:
167174
/// init - Initialize per-function data structures.
168175
void init(MachineFunction &MF) {
@@ -675,6 +682,31 @@ void SSAIfConv::rewritePHIOperands() {
675682
}
676683
}
677684

685+
void SSAIfConv::clearRepeatedKillFlagsFromTBB(MachineBasicBlock *TBB,
686+
MachineBasicBlock *FBB) {
687+
assert(TBB != FBB);
688+
689+
// Collect virtual registers killed in FBB.
690+
SmallDenseSet<Register> FBBKilledRegs;
691+
for (MachineInstr &MI : FBB->instrs()) {
692+
for (MachineOperand &MO : MI.operands()) {
693+
if (MO.isReg() && MO.isKill() && MO.getReg().isVirtual())
694+
FBBKilledRegs.insert(MO.getReg());
695+
}
696+
}
697+
698+
if (FBBKilledRegs.empty())
699+
return;
700+
701+
// Find the same killed registers in TBB and clear kill flags for them.
702+
for (MachineInstr &MI : TBB->instrs()) {
703+
for (MachineOperand &MO : MI.operands()) {
704+
if (MO.isReg() && MO.isKill() && FBBKilledRegs.contains(MO.getReg()))
705+
MO.setIsKill(false);
706+
}
707+
}
708+
}
709+
678710
/// convertIf - Execute the if conversion after canConvertIf has determined the
679711
/// feasibility.
680712
///
@@ -690,6 +722,13 @@ void SSAIfConv::convertIf(SmallVectorImpl<MachineBasicBlock *> &RemoveBlocks,
690722
else
691723
++NumDiamondsConv;
692724

725+
// If both blocks are going to be merged into Head, remove "killed" flag in
726+
// TBB for registers, which are killed in TBB and FBB. Otherwise, register
727+
// will be killed twice in Head after splice. Register killed twice is an
728+
// incorrect MIR.
729+
if (TBB != Tail && FBB != Tail)
730+
clearRepeatedKillFlagsFromTBB(TBB, FBB);
731+
693732
// Move all instructions into Head, except for the terminators.
694733
if (TBB != Tail) {
695734
if (Predicate)
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
2+
# RUN: llc -mtriple=aarch64-- -run-pass=early-ifcvt -stress-early-ifcvt %s -o - -verify-machineinstrs | FileCheck %s
3+
4+
# Test that "killed" flag on the same virtual register in merged blocks is
5+
# removed for the first spliced block and is saved for the second one.
6+
# Otherwise, register will be killed twice in a single block in the resulting
7+
# MIR, which is incorrect.
8+
9+
---
10+
name: my_func
11+
tracksRegLiveness: true
12+
liveins:
13+
- { reg: '$w0', virtual-reg: '%0' }
14+
body: |
15+
; CHECK-LABEL: name: my_func
16+
; CHECK: bb.0.entry:
17+
; CHECK-NEXT: liveins: $w0
18+
; CHECK-NEXT: {{ $}}
19+
; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr32common = COPY $w0
20+
; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr64common = COPY $x0
21+
; CHECK-NEXT: [[SUBSWri:%[0-9]+]]:gpr32 = SUBSWri [[COPY]], 1, 0, implicit-def $nzcv
22+
; CHECK-NEXT: [[ADDWri:%[0-9]+]]:gpr32common = ADDWri [[COPY]], 3, 0
23+
; CHECK-NEXT: [[SUBWri:%[0-9]+]]:gpr32common = SUBWri killed [[COPY]], 2, 0
24+
; CHECK-NEXT: [[CSELWr:%[0-9]+]]:gpr32common = CSELWr [[ADDWri]], [[SUBWri]], 1, implicit $nzcv
25+
; CHECK-NEXT: $x2 = COPY [[COPY1]]
26+
; CHECK-NEXT: RET_ReallyLR implicit $x2
27+
bb.0.entry:
28+
successors: %bb.1, %bb.2
29+
liveins: $w0
30+
31+
%0:gpr32common = COPY $w0
32+
%1:gpr64common = COPY $x0
33+
%2:gpr32 = SUBSWri %0, 1, 0, implicit-def $nzcv
34+
Bcc 1, %bb.2, implicit $nzcv
35+
B %bb.1
36+
37+
bb.1:
38+
successors: %bb.3
39+
40+
%3:gpr32common = SUBWri killed %0, 2, 0
41+
B %bb.3
42+
43+
bb.2:
44+
successors: %bb.3
45+
46+
%4:gpr32common = ADDWri killed %0, 3, 0
47+
B %bb.3
48+
49+
bb.3:
50+
%5:gpr32common = PHI %3, %bb.1, %4, %bb.2
51+
$x2 = COPY %1
52+
RET_ReallyLR implicit $x2
53+
54+
...
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
2+
# RUN: llc -mtriple=hexagon -run-pass early-if-predicator %s -o - -verify-machineinstrs | FileCheck %s
3+
4+
# Test that "killed" flag on the same virtual register in merged blocks is
5+
# removed for the first spliced block and is saved for the second one.
6+
# Otherwise, register will be killed twice in a single block in the resulting
7+
# MIR, which is incorrect.
8+
9+
---
10+
name: my_func
11+
alignment: 16
12+
tracksRegLiveness: true
13+
liveins:
14+
- { reg: '$r0', virtual-reg: '%0' }
15+
- { reg: '$r1', virtual-reg: '%1' }
16+
body: |
17+
; CHECK-LABEL: name: my_func
18+
; CHECK: bb.0:
19+
; CHECK-NEXT: liveins: $r0, $r1
20+
; CHECK-NEXT: {{ $}}
21+
; CHECK-NEXT: [[COPY:%[0-9]+]]:intregs = COPY $r0
22+
; CHECK-NEXT: [[COPY1:%[0-9]+]]:intregs = COPY $r1
23+
; CHECK-NEXT: [[S2_tstbit_i:%[0-9]+]]:predregs = S2_tstbit_i [[COPY1]], 0
24+
; CHECK-NEXT: S4_storeirif_io [[S2_tstbit_i]], [[COPY]], 0, 2
25+
; CHECK-NEXT: S4_storeirit_io [[S2_tstbit_i]], killed [[COPY]], 0, 1
26+
; CHECK-NEXT: PS_jmpret $r31, implicit-def dead $pc
27+
bb.0:
28+
successors: %bb.1(0x40000000), %bb.2(0x40000000)
29+
liveins: $r0, $r1
30+
31+
%0:intregs = COPY $r0
32+
%1:intregs = COPY $r1
33+
%2:predregs = S2_tstbit_i %1, 0
34+
J2_jumpf %2, %bb.2, implicit-def dead $pc
35+
J2_jump %bb.1, implicit-def dead $pc
36+
37+
bb.1:
38+
successors: %bb.3(0x80000000)
39+
40+
S4_storeiri_io killed %0, 0, 1
41+
J2_jump %bb.3, implicit-def dead $pc
42+
43+
bb.2:
44+
successors: %bb.3(0x80000000)
45+
46+
S4_storeiri_io killed %0, 0, 2
47+
J2_jump %bb.3, implicit-def dead $pc
48+
49+
bb.3:
50+
PS_jmpret $r31, implicit-def dead $pc
51+
52+
...

0 commit comments

Comments
 (0)