[llvm] [EarlyIfConverter] Fix reg killed twice after early-if-predicator and ifcvt (PR #133554)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 28 19:19:00 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-aarch64
@llvm/pr-subscribers-backend-hexagon
Author: Afanasyev Ivan (ivafanas)
<details>
<summary>Changes</summary>
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.
Fix proposal: find registers killed in both blocks, TBB and FBB, and clear kill flags for them.
---
Full diff: https://github.com/llvm/llvm-project/pull/133554.diff
3 Files Affected:
- (modified) llvm/lib/CodeGen/EarlyIfConversion.cpp (+34)
- (added) llvm/test/CodeGen/AArch64/early-ifcvt-on-double-killed-reg.mir (+53)
- (added) llvm/test/CodeGen/Hexagon/early-if-predicator-reg-killed-everywhere.mir (+93)
``````````diff
diff --git a/llvm/lib/CodeGen/EarlyIfConversion.cpp b/llvm/lib/CodeGen/EarlyIfConversion.cpp
index 24c6dafc60459..fa9ff37e625a2 100644
--- a/llvm/lib/CodeGen/EarlyIfConversion.cpp
+++ b/llvm/lib/CodeGen/EarlyIfConversion.cpp
@@ -17,6 +17,7 @@
#include "llvm/CodeGen/EarlyIfConversion.h"
#include "llvm/ADT/BitVector.h"
+#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/PostOrderIterator.h"
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SparseSet.h"
@@ -31,6 +32,7 @@
#include "llvm/CodeGen/MachineOptimizationRemarkEmitter.h"
#include "llvm/CodeGen/MachineRegisterInfo.h"
#include "llvm/CodeGen/MachineTraceMetrics.h"
+#include "llvm/CodeGen/Register.h"
#include "llvm/CodeGen/TargetInstrInfo.h"
#include "llvm/CodeGen/TargetRegisterInfo.h"
#include "llvm/CodeGen/TargetSubtargetInfo.h"
@@ -163,6 +165,9 @@ class SSAIfConv {
/// Insert selects and rewrite PHI operands to use them.
void rewritePHIOperands();
+ /// Remove killed flag for virtual regs killed in TBB and FBB simultaniously.
+ void clearDoubleKillFlags(MachineBasicBlock *TBB, MachineBasicBlock *FBB);
+
public:
/// init - Initialize per-function data structures.
void init(MachineFunction &MF) {
@@ -675,6 +680,28 @@ void SSAIfConv::rewritePHIOperands() {
}
}
+void SSAIfConv::clearDoubleKillFlags(MachineBasicBlock *TBB,
+ MachineBasicBlock *FBB) {
+ assert(TBB != FBB);
+
+ // Collect virtual registers killed in TBB.
+ SmallDenseSet<Register> TBBKilledReg;
+ for (MachineInstr &MI : TBB->instrs()) {
+ for (MachineOperand &MO : MI.operands()) {
+ if (MO.isReg() && MO.isKill() && MO.getReg().isVirtual())
+ TBBKilledReg.insert(MO.getReg());
+ }
+ }
+
+ // Find the same killed registers in FBB and clear kill flags for them.
+ for (MachineInstr &MI : FBB->instrs()) {
+ for (MachineOperand &MO : MI.operands()) {
+ if (MO.isReg() && MO.isKill() && TBBKilledReg.contains(MO.getReg()))
+ MRI->clearKillFlags(MO.getReg());
+ }
+ }
+}
+
/// convertIf - Execute the if conversion after canConvertIf has determined the
/// feasibility.
///
@@ -690,6 +717,13 @@ void SSAIfConv::convertIf(SmallVectorImpl<MachineBasicBlock *> &RemoveBlocks,
else
++NumDiamondsConv;
+ // If both blocks are going to be merged into Head, remove "killed" flag from
+ // registers, which are killed in TBB and FBB. Otherwise, register will be
+ // killed twice in Head after splice. Double killed register is an incorrect
+ // MIR.
+ if (TBB != Tail && FBB != Tail)
+ clearDoubleKillFlags(TBB, FBB);
+
// Move all instructions into Head, except for the terminators.
if (TBB != Tail) {
if (Predicate)
diff --git a/llvm/test/CodeGen/AArch64/early-ifcvt-on-double-killed-reg.mir b/llvm/test/CodeGen/AArch64/early-ifcvt-on-double-killed-reg.mir
new file mode 100644
index 0000000000000..bda1972165442
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/early-ifcvt-on-double-killed-reg.mir
@@ -0,0 +1,53 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=aarch64-- -run-pass=early-ifcvt -stress-early-ifcvt %s -o - -verify-machineinstrs | FileCheck %s
+
+# Test that "killed" flag on the same virtual register in merged blocks is not
+# saved. Otherwise, register will be killed twice in a single block in the
+# resulting MIR, which is incorrect.
+
+---
+name: same_def_different_operand
+tracksRegLiveness: true
+liveins:
+ - { reg: '$w0', virtual-reg: '%0' }
+body: |
+ ; CHECK-LABEL: name: same_def_different_operand
+ ; CHECK: bb.0.entry:
+ ; CHECK-NEXT: liveins: $w0
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr32common = COPY $w0
+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr64common = COPY $x0
+ ; CHECK-NEXT: [[SUBSWri:%[0-9]+]]:gpr32 = SUBSWri [[COPY]], 1, 0, implicit-def $nzcv
+ ; CHECK-NEXT: [[SUBWri:%[0-9]+]]:gpr32common = SUBWri [[COPY]], 3, 0
+ ; CHECK-NEXT: [[SUBWri1:%[0-9]+]]:gpr32common = SUBWri [[COPY]], 2, 0
+ ; CHECK-NEXT: [[CSELWr:%[0-9]+]]:gpr32common = CSELWr [[SUBWri]], [[SUBWri1]], 1, implicit $nzcv
+ ; CHECK-NEXT: $x2 = COPY [[COPY1]]
+ ; CHECK-NEXT: RET_ReallyLR implicit $x2
+ bb.0.entry:
+ successors: %bb.1, %bb.2
+ liveins: $w0
+
+ %0:gpr32common = COPY $w0
+ %1:gpr64common = COPY $x0
+ %2:gpr32 = SUBSWri %0, 1, 0, implicit-def $nzcv
+ Bcc 1, %bb.2, implicit $nzcv
+ B %bb.1
+
+ bb.1:
+ successors: %bb.3
+
+ %3:gpr32common = SUBWri killed %0, 2, 0
+ B %bb.3
+
+ bb.2:
+ successors: %bb.3
+
+ %4:gpr32common = SUBWri killed %0, 3, 0
+ B %bb.3
+
+ bb.3:
+ %5:gpr32common = PHI %3, %bb.1, %4, %bb.2
+ $x2 = COPY %1
+ RET_ReallyLR implicit $x2
+
+...
diff --git a/llvm/test/CodeGen/Hexagon/early-if-predicator-reg-killed-everywhere.mir b/llvm/test/CodeGen/Hexagon/early-if-predicator-reg-killed-everywhere.mir
new file mode 100644
index 0000000000000..162fc5e5192bf
--- /dev/null
+++ b/llvm/test/CodeGen/Hexagon/early-if-predicator-reg-killed-everywhere.mir
@@ -0,0 +1,93 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=hexagon -run-pass early-if-predicator %s -o - -verify-machineinstrs | FileCheck %s
+
+# Test that "killed" flag for the same register in merged blocks is not saved.
+# Otherwise, the same register will be killed twice in a single final block.
+
+--- |
+ target datalayout = "e-m:e-p:32:32:32-a:0-n16:32-i64:64:64-i32:32:32-i16:16:16-i1:8:8-f32:32:32-f64:64:64-v32:32:32-v64:64:64-v512:512:512-v1024:1024:1024-v2048:2048:2048"
+
+ define void @if-cvt(ptr %p, i1 %c) {
+ entry:
+ ret void
+ }
+
+...
+---
+name: if-cvt
+alignment: 16
+exposesReturnsTwice: false
+legalized: false
+regBankSelected: false
+selected: false
+failedISel: false
+tracksRegLiveness: true
+hasWinCFI: false
+registers:
+ - { id: 0, class: intregs, preferred-register: '' }
+ - { id: 1, class: intregs, preferred-register: '' }
+ - { id: 2, class: predregs, preferred-register: '' }
+liveins:
+ - { reg: '$r0', virtual-reg: '%0' }
+ - { reg: '$r1', virtual-reg: '%1' }
+frameInfo:
+ isFrameAddressTaken: false
+ isReturnAddressTaken: false
+ hasStackMap: false
+ hasPatchPoint: false
+ stackSize: 0
+ offsetAdjustment: 0
+ maxAlignment: 0
+ adjustsStack: false
+ hasCalls: true
+ stackProtector: ''
+ maxCallFrameSize: 4294967295
+ cvBytesOfCalleeSavedRegisters: 0
+ hasOpaqueSPAdjustment: false
+ hasVAStart: false
+ hasMustTailInVarArgFunc: false
+ localFrameSize: 0
+ savePoint: ''
+ restorePoint: ''
+fixedStack: []
+stack: []
+callSites: []
+constants: []
+machineFunctionInfo: {}
+body: |
+ ; CHECK-LABEL: name: if-cvt
+ ; CHECK: bb.0:
+ ; CHECK-NEXT: liveins: $r0, $r1
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:intregs = COPY $r1
+ ; CHECK-NEXT: [[S2_tstbit_i:%[0-9]+]]:predregs = S2_tstbit_i [[COPY]], 0
+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:intregs = COPY $r0
+ ; CHECK-NEXT: S4_storeirif_io [[S2_tstbit_i]], [[COPY1]], 0, 1
+ ; CHECK-NEXT: S4_storeirit_io [[S2_tstbit_i]], [[COPY1]], 0, 1
+ ; CHECK-NEXT: PS_jmpret $r31, implicit-def dead $pc
+ bb.0:
+ successors: %bb.1(0x40000000), %bb.2(0x40000000)
+ liveins: $r0, $r1
+
+ %1:intregs = COPY $r1
+ %2:predregs = S2_tstbit_i %1, 0
+ %0:intregs = COPY $r0
+ J2_jumpf %2, %bb.2, implicit-def dead $pc
+ J2_jump %bb.1, implicit-def dead $pc
+
+ bb.1:
+ successors: %bb.3(0x80000000)
+
+ S4_storeiri_io killed %0, 0, 1
+ J2_jump %bb.3, implicit-def dead $pc
+
+ bb.2:
+ successors: %bb.3(0x80000000)
+
+ S4_storeiri_io killed %0, 0, 1
+ J2_jump %bb.3, implicit-def dead $pc
+
+ bb.3:
+ PS_jmpret $r31, implicit-def dead $pc
+
+...
``````````
</details>
https://github.com/llvm/llvm-project/pull/133554
More information about the llvm-commits
mailing list