[llvm] [PHIElimination] Account for PHI operands that appear more than once (PR #146806)
Guy David via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 3 01:48:43 PDT 2025
https://github.com/guy-david updated https://github.com/llvm/llvm-project/pull/146806
>From 8061cf9797601a2624e1bb8188a8cd519e714071 Mon Sep 17 00:00:00 2001
From: Guy David <guyda96 at gmail.com>
Date: Thu, 3 Jul 2025 03:21:39 +0300
Subject: [PATCH] [PHIElimination] Account for PHI operands that appear more
than once
During lowering, the PHI instruction is detached from the function and
thus it is not considered a user of its operands.
The previous implementation, which removed redundant COPY of COPY
instructions, skipped values which had any uses, because otherwise it
can break the function. However, it had a hidden assumption that there's
a single use for that operand in the detached PHI instruction.
By not taking into account that an operand can be referenced more than
once, it interfered with itself and created IMPLICIT_DEF's for some
incoming values instead of copying the correct ones.
---
llvm/lib/CodeGen/PHIElimination.cpp | 42 ++++++++++++-------
.../AArch64/PHIElimination-reuse-copy.mir | 5 ++-
llvm/test/CodeGen/PowerPC/vsx.ll | 3 +-
3 files changed, 32 insertions(+), 18 deletions(-)
diff --git a/llvm/lib/CodeGen/PHIElimination.cpp b/llvm/lib/CodeGen/PHIElimination.cpp
index 86523c22a419d..970844c5bb3d4 100644
--- a/llvm/lib/CodeGen/PHIElimination.cpp
+++ b/llvm/lib/CodeGen/PHIElimination.cpp
@@ -541,6 +541,7 @@ void PHIEliminationImpl::LowerPHINode(MachineBasicBlock &MBB,
// Now loop over all of the incoming arguments, changing them to copy into the
// IncomingReg register in the corresponding predecessor basic block.
SmallPtrSet<MachineBasicBlock *, 8> MBBsInsertedInto;
+ SmallVector<MachineInstr *, 8> InsertedCopies;
for (int i = NumSrcs - 1; i >= 0; --i) {
Register SrcReg = MPhi->getOperand(i * 2 + 1).getReg();
unsigned SrcSubReg = MPhi->getOperand(i * 2 + 1).getSubReg();
@@ -581,20 +582,6 @@ void PHIEliminationImpl::LowerPHINode(MachineBasicBlock &MBB,
continue;
}
- // Reuse an existing copy in the block if possible.
- if (IncomingReg.isVirtual()) {
- MachineInstr *DefMI = MRI->getUniqueVRegDef(SrcReg);
- const TargetRegisterClass *SrcRC = MRI->getRegClass(SrcReg);
- const TargetRegisterClass *IncomingRC = MRI->getRegClass(IncomingReg);
- if (DefMI && DefMI->isCopy() && DefMI->getParent() == &opBlock &&
- MRI->use_empty(SrcReg) && IncomingRC->hasSuperClassEq(SrcRC)) {
- DefMI->getOperand(0).setReg(IncomingReg);
- if (LV)
- LV->getVarInfo(SrcReg).AliveBlocks.clear();
- continue;
- }
- }
-
// Find a safe location to insert the copy, this may be the first terminator
// in the block (or end()).
MachineBasicBlock::iterator InsertPos =
@@ -621,6 +608,7 @@ void PHIEliminationImpl::LowerPHINode(MachineBasicBlock &MBB,
NewSrcInstr = TII->createPHISourceCopy(opBlock, InsertPos, nullptr,
SrcReg, SrcSubReg, IncomingReg);
}
+ InsertedCopies.emplace_back(NewSrcInstr);
}
// We only need to update the LiveVariables kill of SrcReg if this was the
@@ -744,6 +732,32 @@ void PHIEliminationImpl::LowerPHINode(MachineBasicBlock &MBB,
}
}
+ // Remove redundant COPY instruction chains, which were potentially added by
+ // the code above. This can simplify the CFG which later on to prevent a
+ // suboptimal block layout.
+ for (MachineInstr *NewCopy : InsertedCopies) {
+ if (NewCopy->isImplicitDef())
+ continue;
+ Register IncomingReg = NewCopy->getOperand(0).getReg();
+ if (!IncomingReg.isVirtual())
+ continue;
+ Register SrcReg = NewCopy->getOperand(1).getReg();
+ if (!MRI->hasOneNonDBGUse(SrcReg))
+ continue;
+ MachineInstr *DefMI = MRI->getUniqueVRegDef(SrcReg);
+ if (!DefMI || !DefMI->isCopy() ||
+ DefMI->getParent() != NewCopy->getParent())
+ continue;
+ const TargetRegisterClass *SrcRC = MRI->getRegClass(SrcReg);
+ const TargetRegisterClass *IncomingRC = MRI->getRegClass(IncomingReg);
+ if (!IncomingRC->hasSuperClassEq(SrcRC))
+ continue;
+ MRI->replaceRegWith(SrcReg, IncomingReg);
+ NewCopy->removeFromParent();
+ if (LV)
+ LV->getVarInfo(SrcReg).AliveBlocks.clear();
+ }
+
// Really delete the PHI instruction now, if it is not in the LoweredPHIs map.
if (EliminateNow) {
if (LIS)
diff --git a/llvm/test/CodeGen/AArch64/PHIElimination-reuse-copy.mir b/llvm/test/CodeGen/AArch64/PHIElimination-reuse-copy.mir
index 20020a8ed3fb7..fb85a0dbff30d 100644
--- a/llvm/test/CodeGen/AArch64/PHIElimination-reuse-copy.mir
+++ b/llvm/test/CodeGen/AArch64/PHIElimination-reuse-copy.mir
@@ -135,14 +135,15 @@ body: |
; CHECK-NEXT: liveins: $nzcv
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: dead [[COPY2:%[0-9]+]]:gpr32 = COPY killed [[COPY1]]
- ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr32 = COPY [[COPY]]
+ ; CHECK-NEXT: [[COPY3:%[0-9]+]]:gpr32 = COPY [[COPY]]
+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr32 = COPY [[COPY3]]
; CHECK-NEXT: Bcc 1, %bb.1, implicit $nzcv
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.2:
; CHECK-NEXT: successors: %bb.1(0x80000000)
; CHECK-NEXT: liveins: $nzcv
; CHECK-NEXT: {{ $}}
- ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr32 = IMPLICIT_DEF
+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr32 = COPY killed [[COPY3]]
; CHECK-NEXT: B %bb.1
bb.0:
successors: %bb.1
diff --git a/llvm/test/CodeGen/PowerPC/vsx.ll b/llvm/test/CodeGen/PowerPC/vsx.ll
index 9e0dc87f0ab8b..33d7395693b49 100644
--- a/llvm/test/CodeGen/PowerPC/vsx.ll
+++ b/llvm/test/CodeGen/PowerPC/vsx.ll
@@ -2487,8 +2487,7 @@ define double @test82(double %a, double %b, double %c, double %d) {
; CHECK-FISL-LABEL: test82:
; CHECK-FISL: # %bb.0: # %entry
; CHECK-FISL-NEXT: stfd f2, -16(r1) # 8-byte Folded Spill
-; CHECK-FISL-NEXT: fmr f2, f1
-; CHECK-FISL-NEXT: stfd f2, -8(r1) # 8-byte Folded Spill
+; CHECK-FISL-NEXT: stfd f1, -8(r1) # 8-byte Folded Spill
; CHECK-FISL-NEXT: xscmpudp cr0, f3, f4
; CHECK-FISL-NEXT: beq cr0, .LBB67_2
; CHECK-FISL-NEXT: # %bb.1: # %entry
More information about the llvm-commits
mailing list