[llvm] [PowerPC] Fix ppc-reduce-cr-ops mishandling of subregister uses (PR #144405)
Lei Huang via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 18 10:56:58 PDT 2025
https://github.com/lei137 updated https://github.com/llvm/llvm-project/pull/144405
>From 5ea4d1d66788e2484a9cc32b3ce88c561d119183 Mon Sep 17 00:00:00 2001
From: Lei Huang <lei at ca.ibm.com>
Date: Mon, 16 Jun 2025 17:30:33 +0000
Subject: [PATCH 1/2] [PowerPC] Fix ppc-reduce-cr-ops mishandling of
subregister uses
Corrects the erroneous assumption that CR-logical operation's operands are
always defined by a subreg copy.
Fixes https://github.com/llvm/llvm-project/issues/141643#top
Patch by Nemanja Ivanovic
---
.../Target/PowerPC/PPCReduceCRLogicals.cpp | 37 ++++++-----
llvm/test/CodeGen/PowerPC/crreduce-reg.mir | 66 +++++++++++++++++++
2 files changed, 86 insertions(+), 17 deletions(-)
create mode 100644 llvm/test/CodeGen/PowerPC/crreduce-reg.mir
diff --git a/llvm/lib/Target/PowerPC/PPCReduceCRLogicals.cpp b/llvm/lib/Target/PowerPC/PPCReduceCRLogicals.cpp
index ec20ad2565e68..c28ff4c468ad2 100644
--- a/llvm/lib/Target/PowerPC/PPCReduceCRLogicals.cpp
+++ b/llvm/lib/Target/PowerPC/PPCReduceCRLogicals.cpp
@@ -108,6 +108,8 @@ struct BlockSplitInfo {
MachineInstr *OrigBranch;
MachineInstr *SplitBefore;
MachineInstr *SplitCond;
+ unsigned OrigSubreg;
+ unsigned SplitCondSubreg;
bool InvertNewBranch;
bool InvertOrigBranch;
bool BranchToFallThrough;
@@ -220,7 +222,7 @@ static bool splitMBB(BlockSplitInfo &BSI) {
// Add the branches to ThisMBB.
BuildMI(*ThisMBB, ThisMBB->end(), BSI.SplitBefore->getDebugLoc(),
TII->get(NewBROpcode))
- .addReg(BSI.SplitCond->getOperand(0).getReg())
+ .addReg(BSI.SplitCond->getOperand(0).getReg(), 0, BSI.SplitCondSubreg)
.addMBB(NewBRTarget);
BuildMI(*ThisMBB, ThisMBB->end(), BSI.SplitBefore->getDebugLoc(),
TII->get(PPC::B))
@@ -234,6 +236,7 @@ static bool splitMBB(BlockSplitInfo &BSI) {
assert(FirstTerminator->getOperand(0).isReg() &&
"Can't update condition of unconditional branch.");
FirstTerminator->getOperand(0).setReg(BSI.NewCond->getOperand(0).getReg());
+ FirstTerminator->getOperand(0).setSubReg(BSI.OrigSubreg);
}
if (BSI.InvertOrigBranch)
FirstTerminator->setDesc(TII->get(InvertedOpcode));
@@ -471,6 +474,8 @@ PPCReduceCRLogicals::createCRLogicalOpInfo(MachineInstr &MIParam) {
} else {
MachineInstr *Def1 = lookThroughCRCopy(MIParam.getOperand(1).getReg(),
Ret.SubregDef1, Ret.CopyDefs.first);
+ if (Ret.SubregDef1 == 0)
+ Ret.SubregDef1 = MIParam.getOperand(1).getSubReg();
assert(Def1 && "Must be able to find a definition of operand 1.");
Ret.DefsSingleUse &=
MRI->hasOneNonDBGUse(Def1->getOperand(0).getReg());
@@ -481,6 +486,8 @@ PPCReduceCRLogicals::createCRLogicalOpInfo(MachineInstr &MIParam) {
MachineInstr *Def2 = lookThroughCRCopy(MIParam.getOperand(2).getReg(),
Ret.SubregDef2,
Ret.CopyDefs.second);
+ if (Ret.SubregDef2 == 0)
+ Ret.SubregDef2 = MIParam.getOperand(2).getSubReg();
assert(Def2 && "Must be able to find a definition of operand 2.");
Ret.DefsSingleUse &=
MRI->hasOneNonDBGUse(Def2->getOperand(0).getReg());
@@ -535,7 +542,6 @@ PPCReduceCRLogicals::createCRLogicalOpInfo(MachineInstr &MIParam) {
MachineInstr *PPCReduceCRLogicals::lookThroughCRCopy(unsigned Reg,
unsigned &Subreg,
MachineInstr *&CpDef) {
- Subreg = -1;
if (!Register::isVirtualRegister(Reg))
return nullptr;
MachineInstr *Copy = MRI->getVRegDef(Reg);
@@ -543,18 +549,10 @@ MachineInstr *PPCReduceCRLogicals::lookThroughCRCopy(unsigned Reg,
if (!Copy->isCopy())
return Copy;
Register CopySrc = Copy->getOperand(1).getReg();
- Subreg = Copy->getOperand(1).getSubReg();
+ // If the copy defines a register with a subreg, set that as the Subreg.
+ Subreg = Copy->getOperand(0).getSubReg();
if (!CopySrc.isVirtual()) {
const TargetRegisterInfo *TRI = &TII->getRegisterInfo();
- // Set the Subreg
- if (CopySrc == PPC::CR0EQ || CopySrc == PPC::CR6EQ)
- Subreg = PPC::sub_eq;
- if (CopySrc == PPC::CR0LT || CopySrc == PPC::CR6LT)
- Subreg = PPC::sub_lt;
- if (CopySrc == PPC::CR0GT || CopySrc == PPC::CR6GT)
- Subreg = PPC::sub_gt;
- if (CopySrc == PPC::CR0UN || CopySrc == PPC::CR6UN)
- Subreg = PPC::sub_un;
// Loop backwards and return the first MI that modifies the physical CR Reg.
MachineBasicBlock::iterator Me = Copy, B = Copy->getParent()->begin();
while (Me != B)
@@ -682,16 +680,21 @@ bool PPCReduceCRLogicals::splitBlockOnBinaryCROp(CRLogicalOpInfo &CRI) {
computeBranchTargetAndInversion(Opc, Branch->getOpcode(), UsingDef1,
InvertNewBranch, InvertOrigBranch,
TargetIsFallThrough);
- MachineInstr *SplitCond =
- UsingDef1 ? CRI.CopyDefs.second : CRI.CopyDefs.first;
+ MachineInstr *NewCond = CRI.CopyDefs.first;
+ MachineInstr *SplitCond = CRI.CopyDefs.second;
+ if (!UsingDef1) {
+ std::swap(NewCond, SplitCond);
+ std::swap(CRI.SubregDef1, CRI.SubregDef2);
+ }
LLVM_DEBUG(dbgs() << "We will " << (InvertNewBranch ? "invert" : "copy"));
LLVM_DEBUG(dbgs() << " the original branch and the target is the "
<< (TargetIsFallThrough ? "fallthrough block\n"
: "orig. target block\n"));
LLVM_DEBUG(dbgs() << "Original branch instruction: "; Branch->dump());
- BlockSplitInfo BSI { Branch, SplitBefore, SplitCond, InvertNewBranch,
- InvertOrigBranch, TargetIsFallThrough, MBPI, CRI.MI,
- UsingDef1 ? CRI.CopyDefs.first : CRI.CopyDefs.second };
+ BlockSplitInfo BSI{
+ Branch, SplitBefore, SplitCond, CRI.SubregDef1,
+ CRI.SubregDef2, InvertNewBranch, InvertOrigBranch, TargetIsFallThrough,
+ MBPI, CRI.MI, NewCond};
bool Changed = splitMBB(BSI);
// If we've split on a CR logical that is fed by a CR logical,
// recompute the source CR logical as it may be usable for splitting.
diff --git a/llvm/test/CodeGen/PowerPC/crreduce-reg.mir b/llvm/test/CodeGen/PowerPC/crreduce-reg.mir
new file mode 100644
index 0000000000000..1b6045d82f9dd
--- /dev/null
+++ b/llvm/test/CodeGen/PowerPC/crreduce-reg.mir
@@ -0,0 +1,66 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
+# RUN: llc -mtriple=powerpc64-unknown-linux-gnu -run-pass=ppc-reduce-cr-ops \
+# RUN: -verify-machineinstrs -o - %s | FileCheck %s
+
+---
+name: subreg_folding_regression
+tracksRegLiveness: true
+isSSA: true
+body: |
+ ; CHECK-LABEL: name: subreg_folding_regression
+ ; CHECK: bb.0:
+ ; CHECK-NEXT: successors: %bb.1(0x80000000)
+ ; CHECK-NEXT: liveins: $x3
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:g8rc_and_g8rc_nox0 = COPY $x3
+ ; CHECK-NEXT: [[LD:%[0-9]+]]:g8rc = LD 0, [[COPY]] :: (load (s64))
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.1:
+ ; CHECK-NEXT: successors: %bb.2(0x20000000), %bb.3(0x60000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[PHI:%[0-9]+]]:g8rc_and_g8rc_nox0 = PHI [[LD]], %bb.0, %3, %bb.3, %4, %bb.2
+ ; CHECK-NEXT: [[LBZ:%[0-9]+]]:gprc = LBZ 0, [[PHI]] :: (load (s8))
+ ; CHECK-NEXT: [[CMPWI:%[0-9]+]]:crrc = CMPWI killed [[LBZ]], 0
+ ; CHECK-NEXT: [[ADDI8_:%[0-9]+]]:g8rc = nuw ADDI8 [[PHI]], 1
+ ; CHECK-NEXT: STD [[ADDI8_]], 0, [[COPY]] :: (store (s64))
+ ; CHECK-NEXT: [[LBZ1:%[0-9]+]]:gprc = LBZ 1, [[PHI]] :: (load (s8))
+ ; CHECK-NEXT: BCn [[CMPWI]].sub_lt, %bb.2
+ ; CHECK-NEXT: B %bb.3
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.3:
+ ; CHECK-NEXT: successors: %bb.1(0x55555555), %bb.2(0x2aaaaaab)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[CMPWI1:%[0-9]+]]:crrc = CMPWI killed [[LBZ1]], 0
+ ; CHECK-NEXT: BC killed [[CMPWI1]].sub_eq, %bb.1
+ ; CHECK-NEXT: B %bb.2
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.2:
+ ; CHECK-NEXT: successors: %bb.1(0x80000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[ADDI8_1:%[0-9]+]]:g8rc = nuw ADDI8 [[PHI]], 2
+ ; CHECK-NEXT: STD [[ADDI8_1]], 0, [[COPY]] :: (store (s64))
+ ; CHECK-NEXT: B %bb.1
+ bb.0:
+ liveins: $x3
+
+ %0:g8rc_and_g8rc_nox0 = COPY $x3
+ %1:g8rc = LD 0, %0 :: (load (s64))
+
+ bb.1:
+ %2:g8rc_and_g8rc_nox0 = PHI %1, %bb.0, %3, %bb.1, %4, %bb.2
+ %5:gprc = LBZ 0, %2 :: (load (s8))
+ %6:crrc = CMPWI killed %5, 0
+ %3:g8rc = nuw ADDI8 %2, 1
+ STD %3, 0, %0 :: (store (s64))
+ %7:gprc = LBZ 1, %2 :: (load (s8))
+ %8:crrc = CMPWI killed %7, 0
+ %9:crbitrc = CRAND %8.sub_eq, %6.sub_lt
+ BC killed %9, %bb.1
+ B %bb.2
+
+ bb.2:
+ %4:g8rc = nuw ADDI8 %2, 2
+ STD %4, 0, %0 :: (store (s64))
+ B %bb.1
+
+...
>From 9fe74ad3fca674e8733da37912b0f72e8170e4a6 Mon Sep 17 00:00:00 2001
From: Lei Huang <lei at ca.ibm.com>
Date: Wed, 18 Jun 2025 13:56:42 -0400
Subject: [PATCH 2/2] apply Nemanja's suggestions
---
llvm/lib/Target/PowerPC/PPCReduceCRLogicals.cpp | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/Target/PowerPC/PPCReduceCRLogicals.cpp b/llvm/lib/Target/PowerPC/PPCReduceCRLogicals.cpp
index c28ff4c468ad2..0ffd35dfa279c 100644
--- a/llvm/lib/Target/PowerPC/PPCReduceCRLogicals.cpp
+++ b/llvm/lib/Target/PowerPC/PPCReduceCRLogicals.cpp
@@ -474,8 +474,7 @@ PPCReduceCRLogicals::createCRLogicalOpInfo(MachineInstr &MIParam) {
} else {
MachineInstr *Def1 = lookThroughCRCopy(MIParam.getOperand(1).getReg(),
Ret.SubregDef1, Ret.CopyDefs.first);
- if (Ret.SubregDef1 == 0)
- Ret.SubregDef1 = MIParam.getOperand(1).getSubReg();
+ Ret.SubregDef1 = MIParam.getOperand(1).getSubReg();
assert(Def1 && "Must be able to find a definition of operand 1.");
Ret.DefsSingleUse &=
MRI->hasOneNonDBGUse(Def1->getOperand(0).getReg());
@@ -486,8 +485,7 @@ PPCReduceCRLogicals::createCRLogicalOpInfo(MachineInstr &MIParam) {
MachineInstr *Def2 = lookThroughCRCopy(MIParam.getOperand(2).getReg(),
Ret.SubregDef2,
Ret.CopyDefs.second);
- if (Ret.SubregDef2 == 0)
- Ret.SubregDef2 = MIParam.getOperand(2).getSubReg();
+ Ret.SubregDef2 = MIParam.getOperand(2).getSubReg();
assert(Def2 && "Must be able to find a definition of operand 2.");
Ret.DefsSingleUse &=
MRI->hasOneNonDBGUse(Def2->getOperand(0).getReg());
@@ -549,8 +547,6 @@ MachineInstr *PPCReduceCRLogicals::lookThroughCRCopy(unsigned Reg,
if (!Copy->isCopy())
return Copy;
Register CopySrc = Copy->getOperand(1).getReg();
- // If the copy defines a register with a subreg, set that as the Subreg.
- Subreg = Copy->getOperand(0).getSubReg();
if (!CopySrc.isVirtual()) {
const TargetRegisterInfo *TRI = &TII->getRegisterInfo();
// Loop backwards and return the first MI that modifies the physical CR Reg.
More information about the llvm-commits
mailing list