Skip to content

Commit 0a822f8

Browse files
lei137lei137
authored
[PowerPC] Fix ppc-reduce-cr-ops mishandling of subregister uses (#144405)
Corrects the erroneous assumption that CR-logical operation's operands are always defined by a subreg copy. Fixes #141643 Patch by Nemanja Ivanovic
1 parent 1f7ba23 commit 0a822f8

File tree

2 files changed

+82
-17
lines changed

2 files changed

+82
-17
lines changed

‎llvm/lib/Target/PowerPC/PPCReduceCRLogicals.cpp

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@ struct BlockSplitInfo {
108108
MachineInstr *OrigBranch;
109109
MachineInstr *SplitBefore;
110110
MachineInstr *SplitCond;
111+
unsigned OrigSubreg;
112+
unsigned SplitCondSubreg;
111113
bool InvertNewBranch;
112114
bool InvertOrigBranch;
113115
bool BranchToFallThrough;
@@ -220,7 +222,7 @@ static bool splitMBB(BlockSplitInfo &BSI) {
220222
// Add the branches to ThisMBB.
221223
BuildMI(*ThisMBB, ThisMBB->end(), BSI.SplitBefore->getDebugLoc(),
222224
TII->get(NewBROpcode))
223-
.addReg(BSI.SplitCond->getOperand(0).getReg())
225+
.addReg(BSI.SplitCond->getOperand(0).getReg(), 0, BSI.SplitCondSubreg)
224226
.addMBB(NewBRTarget);
225227
BuildMI(*ThisMBB, ThisMBB->end(), BSI.SplitBefore->getDebugLoc(),
226228
TII->get(PPC::B))
@@ -234,6 +236,7 @@ static bool splitMBB(BlockSplitInfo &BSI) {
234236
assert(FirstTerminator->getOperand(0).isReg() &&
235237
"Can't update condition of unconditional branch.");
236238
FirstTerminator->getOperand(0).setReg(BSI.NewCond->getOperand(0).getReg());
239+
FirstTerminator->getOperand(0).setSubReg(BSI.OrigSubreg);
237240
}
238241
if (BSI.InvertOrigBranch)
239242
FirstTerminator->setDesc(TII->get(InvertedOpcode));
@@ -471,6 +474,7 @@ PPCReduceCRLogicals::createCRLogicalOpInfo(MachineInstr &MIParam) {
471474
} else {
472475
MachineInstr *Def1 = lookThroughCRCopy(MIParam.getOperand(1).getReg(),
473476
Ret.SubregDef1, Ret.CopyDefs.first);
477+
Ret.SubregDef1 = MIParam.getOperand(1).getSubReg();
474478
assert(Def1 && "Must be able to find a definition of operand 1.");
475479
Ret.DefsSingleUse &=
476480
MRI->hasOneNonDBGUse(Def1->getOperand(0).getReg());
@@ -481,6 +485,7 @@ PPCReduceCRLogicals::createCRLogicalOpInfo(MachineInstr &MIParam) {
481485
MachineInstr *Def2 = lookThroughCRCopy(MIParam.getOperand(2).getReg(),
482486
Ret.SubregDef2,
483487
Ret.CopyDefs.second);
488+
Ret.SubregDef2 = MIParam.getOperand(2).getSubReg();
484489
assert(Def2 && "Must be able to find a definition of operand 2.");
485490
Ret.DefsSingleUse &=
486491
MRI->hasOneNonDBGUse(Def2->getOperand(0).getReg());
@@ -535,26 +540,15 @@ PPCReduceCRLogicals::createCRLogicalOpInfo(MachineInstr &MIParam) {
535540
MachineInstr *PPCReduceCRLogicals::lookThroughCRCopy(unsigned Reg,
536541
unsigned &Subreg,
537542
MachineInstr *&CpDef) {
538-
Subreg = -1;
539543
if (!Register::isVirtualRegister(Reg))
540544
return nullptr;
541545
MachineInstr *Copy = MRI->getVRegDef(Reg);
542546
CpDef = Copy;
543547
if (!Copy->isCopy())
544548
return Copy;
545549
Register CopySrc = Copy->getOperand(1).getReg();
546-
Subreg = Copy->getOperand(1).getSubReg();
547550
if (!CopySrc.isVirtual()) {
548551
const TargetRegisterInfo *TRI = &TII->getRegisterInfo();
549-
// Set the Subreg
550-
if (CopySrc == PPC::CR0EQ || CopySrc == PPC::CR6EQ)
551-
Subreg = PPC::sub_eq;
552-
if (CopySrc == PPC::CR0LT || CopySrc == PPC::CR6LT)
553-
Subreg = PPC::sub_lt;
554-
if (CopySrc == PPC::CR0GT || CopySrc == PPC::CR6GT)
555-
Subreg = PPC::sub_gt;
556-
if (CopySrc == PPC::CR0UN || CopySrc == PPC::CR6UN)
557-
Subreg = PPC::sub_un;
558552
// Loop backwards and return the first MI that modifies the physical CR Reg.
559553
MachineBasicBlock::iterator Me = Copy, B = Copy->getParent()->begin();
560554
while (Me != B)
@@ -682,16 +676,21 @@ bool PPCReduceCRLogicals::splitBlockOnBinaryCROp(CRLogicalOpInfo &CRI) {
682676
computeBranchTargetAndInversion(Opc, Branch->getOpcode(), UsingDef1,
683677
InvertNewBranch, InvertOrigBranch,
684678
TargetIsFallThrough);
685-
MachineInstr *SplitCond =
686-
UsingDef1 ? CRI.CopyDefs.second : CRI.CopyDefs.first;
679+
MachineInstr *NewCond = CRI.CopyDefs.first;
680+
MachineInstr *SplitCond = CRI.CopyDefs.second;
681+
if (!UsingDef1) {
682+
std::swap(NewCond, SplitCond);
683+
std::swap(CRI.SubregDef1, CRI.SubregDef2);
684+
}
687685
LLVM_DEBUG(dbgs() << "We will " << (InvertNewBranch ? "invert" : "copy"));
688686
LLVM_DEBUG(dbgs() << " the original branch and the target is the "
689687
<< (TargetIsFallThrough ? "fallthrough block\n"
690688
: "orig. target block\n"));
691689
LLVM_DEBUG(dbgs() << "Original branch instruction: "; Branch->dump());
692-
BlockSplitInfo BSI { Branch, SplitBefore, SplitCond, InvertNewBranch,
693-
InvertOrigBranch, TargetIsFallThrough, MBPI, CRI.MI,
694-
UsingDef1 ? CRI.CopyDefs.first : CRI.CopyDefs.second };
690+
BlockSplitInfo BSI{
691+
Branch, SplitBefore, SplitCond, CRI.SubregDef1,
692+
CRI.SubregDef2, InvertNewBranch, InvertOrigBranch, TargetIsFallThrough,
693+
MBPI, CRI.MI, NewCond};
695694
bool Changed = splitMBB(BSI);
696695
// If we've split on a CR logical that is fed by a CR logical,
697696
// recompute the source CR logical as it may be usable for splitting.

‎llvm/test/CodeGen/PowerPC/crreduce-reg.mir

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
2+
# RUN: llc -mtriple=powerpc64-unknown-linux-gnu -run-pass=ppc-reduce-cr-ops \
3+
# RUN: -verify-machineinstrs -o - %s | FileCheck %s
4+
5+
---
6+
name: subreg_folding_regression
7+
tracksRegLiveness: true
8+
isSSA: true
9+
body: |
10+
; CHECK-LABEL: name: subreg_folding_regression
11+
; CHECK: bb.0:
12+
; CHECK-NEXT: successors: %bb.1(0x80000000)
13+
; CHECK-NEXT: liveins: $x3
14+
; CHECK-NEXT: {{ $}}
15+
; CHECK-NEXT: [[COPY:%[0-9]+]]:g8rc_and_g8rc_nox0 = COPY $x3
16+
; CHECK-NEXT: [[LD:%[0-9]+]]:g8rc = LD 0, [[COPY]] :: (load (s64))
17+
; CHECK-NEXT: {{ $}}
18+
; CHECK-NEXT: bb.1:
19+
; CHECK-NEXT: successors: %bb.2(0x20000000), %bb.3(0x60000000)
20+
; CHECK-NEXT: {{ $}}
21+
; CHECK-NEXT: [[PHI:%[0-9]+]]:g8rc_and_g8rc_nox0 = PHI [[LD]], %bb.0, %3, %bb.3, %4, %bb.2
22+
; CHECK-NEXT: [[LBZ:%[0-9]+]]:gprc = LBZ 0, [[PHI]] :: (load (s8))
23+
; CHECK-NEXT: [[CMPWI:%[0-9]+]]:crrc = CMPWI killed [[LBZ]], 0
24+
; CHECK-NEXT: [[ADDI8_:%[0-9]+]]:g8rc = nuw ADDI8 [[PHI]], 1
25+
; CHECK-NEXT: STD [[ADDI8_]], 0, [[COPY]] :: (store (s64))
26+
; CHECK-NEXT: [[LBZ1:%[0-9]+]]:gprc = LBZ 1, [[PHI]] :: (load (s8))
27+
; CHECK-NEXT: BCn [[CMPWI]].sub_lt, %bb.2
28+
; CHECK-NEXT: B %bb.3
29+
; CHECK-NEXT: {{ $}}
30+
; CHECK-NEXT: bb.3:
31+
; CHECK-NEXT: successors: %bb.1(0x55555555), %bb.2(0x2aaaaaab)
32+
; CHECK-NEXT: {{ $}}
33+
; CHECK-NEXT: [[CMPWI1:%[0-9]+]]:crrc = CMPWI killed [[LBZ1]], 0
34+
; CHECK-NEXT: BC killed [[CMPWI1]].sub_eq, %bb.1
35+
; CHECK-NEXT: B %bb.2
36+
; CHECK-NEXT: {{ $}}
37+
; CHECK-NEXT: bb.2:
38+
; CHECK-NEXT: successors: %bb.1(0x80000000)
39+
; CHECK-NEXT: {{ $}}
40+
; CHECK-NEXT: [[ADDI8_1:%[0-9]+]]:g8rc = nuw ADDI8 [[PHI]], 2
41+
; CHECK-NEXT: STD [[ADDI8_1]], 0, [[COPY]] :: (store (s64))
42+
; CHECK-NEXT: B %bb.1
43+
bb.0:
44+
liveins: $x3
45+
46+
%0:g8rc_and_g8rc_nox0 = COPY $x3
47+
%1:g8rc = LD 0, %0 :: (load (s64))
48+
49+
bb.1:
50+
%2:g8rc_and_g8rc_nox0 = PHI %1, %bb.0, %3, %bb.1, %4, %bb.2
51+
%5:gprc = LBZ 0, %2 :: (load (s8))
52+
%6:crrc = CMPWI killed %5, 0
53+
%3:g8rc = nuw ADDI8 %2, 1
54+
STD %3, 0, %0 :: (store (s64))
55+
%7:gprc = LBZ 1, %2 :: (load (s8))
56+
%8:crrc = CMPWI killed %7, 0
57+
%9:crbitrc = CRAND %8.sub_eq, %6.sub_lt
58+
BC killed %9, %bb.1
59+
B %bb.2
60+
61+
bb.2:
62+
%4:g8rc = nuw ADDI8 %2, 2
63+
STD %4, 0, %0 :: (store (s64))
64+
B %bb.1
65+
66+
...

0 commit comments

Comments
 (0)