-
Notifications
You must be signed in to change notification settings - Fork 15.5k
[PowerPC] Fix XXPERMDI peephole and ISEL LiveVariables bugs #172122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@llvm/pr-subscribers-backend-powerpc Author: Maryam Moghadas (maryammo) ChangesPrevent XXPERMDI splat optimization when the splat output register is used Full diff: https://github.com/llvm/llvm-project/pull/172122.diff 4 Files Affected:
diff --git a/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp b/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
index 99bef417eaa89..068e641dd01a3 100644
--- a/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
+++ b/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
@@ -3852,7 +3852,8 @@ bool PPCInstrInfo::convertToImmediateForm(MachineInstr &MI,
// If this is not a reg+reg, but the DefMI is LI/LI8, check if its user MI
// can be simpified to LI.
- if (!HasImmForm && simplifyToLI(MI, *DefMI, ForwardingOperand, KilledDef))
+ if (!HasImmForm &&
+ simplifyToLI(MI, *DefMI, ForwardingOperand, KilledDef, &RegsToUpdate))
return true;
return false;
@@ -4619,7 +4620,8 @@ bool PPCInstrInfo::isImmElgibleForForwarding(const MachineOperand &ImmMO,
bool PPCInstrInfo::simplifyToLI(MachineInstr &MI, MachineInstr &DefMI,
unsigned OpNoForForwarding,
- MachineInstr **KilledDef) const {
+ MachineInstr **KilledDef,
+ SmallSet<Register, 4> *RegsToUpdate) const {
if ((DefMI.getOpcode() != PPC::LI && DefMI.getOpcode() != PPC::LI8) ||
!DefMI.getOperand(1).isImm())
return false;
@@ -4687,6 +4689,11 @@ bool PPCInstrInfo::simplifyToLI(MachineInstr &MI, MachineInstr &DefMI,
dbgs() << "Found LI -> CMPI -> ISEL, replacing with a copy.\n");
LLVM_DEBUG(DefMI.dump(); MI.dump(); CompareUseMI.dump());
LLVM_DEBUG(dbgs() << "Is converted to:\n");
+ if (RegsToUpdate) {
+ for (const MachineOperand &MO : CompareUseMI.operands())
+ if (MO.isReg())
+ RegsToUpdate->insert(MO.getReg());
+ }
// Convert to copy and remove unneeded operands.
CompareUseMI.setDesc(get(PPC::COPY));
CompareUseMI.removeOperand(3);
diff --git a/llvm/lib/Target/PowerPC/PPCInstrInfo.h b/llvm/lib/Target/PowerPC/PPCInstrInfo.h
index 8b824bc219ab2..542236c161ad9 100644
--- a/llvm/lib/Target/PowerPC/PPCInstrInfo.h
+++ b/llvm/lib/Target/PowerPC/PPCInstrInfo.h
@@ -297,7 +297,8 @@ class PPCInstrInfo : public PPCGenInstrInfo {
// Replace the instruction with single LI if possible. \p DefMI must be LI or
// LI8.
bool simplifyToLI(MachineInstr &MI, MachineInstr &DefMI,
- unsigned OpNoForForwarding, MachineInstr **KilledDef) const;
+ unsigned OpNoForForwarding, MachineInstr **KilledDef,
+ SmallSet<Register, 4> *RegsToUpdate = nullptr) const;
// If the inst is imm-form and its register operand is produced by a ADDI, put
// the imm into the inst directly and remove the ADDI if possible.
bool transformToNewImmFormFedByAdd(MachineInstr &MI, MachineInstr &DefMI,
diff --git a/llvm/lib/Target/PowerPC/PPCMIPeephole.cpp b/llvm/lib/Target/PowerPC/PPCMIPeephole.cpp
index 94d06422c12be..0be8f2b73432f 100644
--- a/llvm/lib/Target/PowerPC/PPCMIPeephole.cpp
+++ b/llvm/lib/Target/PowerPC/PPCMIPeephole.cpp
@@ -731,6 +731,7 @@ bool PPCMIPeephole::simplifyCode() {
LLVM_DEBUG(dbgs() << "Optimizing swap/swap => copy: ");
LLVM_DEBUG(MI.dump());
addRegToUpdate(MI.getOperand(1).getReg());
+
BuildMI(MBB, &MI, MI.getDebugLoc(), TII->get(PPC::COPY),
MI.getOperand(0).getReg())
.add(DefMI->getOperand(1));
@@ -743,7 +744,19 @@ bool PPCMIPeephole::simplifyCode() {
DefOpc == PPC::XXPERMDIs &&
(DefMI->getOperand(2).getImm() == 0 ||
DefMI->getOperand(2).getImm() == 3)) {
- ToErase = &MI;
+
+ bool OnlyUsedInMI = true;
+ for (MachineInstr &UseMI :
+ MRI->use_instructions(DefMI->getOperand(0).getReg())) {
+ if (UseMI.isDebugInstr())
+ continue;
+ if (&UseMI != &MI) {
+ OnlyUsedInMI = false;
+ break;
+ }
+ }
+ if (!OnlyUsedInMI)
+ break;
Simplified = true;
// Swap of a splat, convert to copy.
if (Immed == 2) {
@@ -753,10 +766,12 @@ bool PPCMIPeephole::simplifyCode() {
MI.getOperand(0).getReg())
.add(MI.getOperand(1));
addRegToUpdate(MI.getOperand(1).getReg());
+ ToErase = &MI;
break;
}
// Splat fed by another splat - switch the output of the first
// and remove the second.
+ ToErase = &MI;
DefMI->getOperand(0).setReg(MI.getOperand(0).getReg());
LLVM_DEBUG(dbgs() << "Removing redundant splat: ");
LLVM_DEBUG(MI.dump());
diff --git a/llvm/test/CodeGen/PowerPC/peephole-livevars-tracking.mir b/llvm/test/CodeGen/PowerPC/peephole-livevars-tracking.mir
new file mode 100644
index 0000000000000..16df0caa9e899
--- /dev/null
+++ b/llvm/test/CodeGen/PowerPC/peephole-livevars-tracking.mir
@@ -0,0 +1,83 @@
+# RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux \
+# RUN: -run-pass=ppc-mi-peepholes %s -o - | FileCheck %s
+
+# Test that peephole optimizations correctly update LiveVariables tracking.
+# XXPERMDI splat optimization skipped when splat register has multiple uses.
+# ISEL to COPY conversion tracks removed operand registers to prevent
+# LiveVariables corruption.
+
+---
+name: test_xxpermdi_splat_multiple_uses
+alignment: 16
+tracksRegLiveness: true
+registers:
+ - { id: 0, class: vrrc }
+ - { id: 28, class: vrrc }
+ - { id: 31, class: f8rc }
+ - { id: 34, class: vrrc }
+ - { id: 35, class: vrrc }
+body: |
+ bb.0:
+ liveins: $f1
+
+ %28:vrrc = XXLXORz
+ %31:f8rc = COPY $f1
+ %0:vrrc = XXPERMDIs %31, 0
+ %34:vrrc = XXPERMDI %28, %0, 3
+ %35:vrrc = XXPERMDI %0, %0, 3
+
+ ; CHECK-LABEL: name: test_xxpermdi_splat_multiple_uses
+ ; CHECK: %1:vrrc = XXLXORz
+ ; CHECK-NEXT: %2:f8rc = COPY killed $f1
+ ; CHECK-NEXT: %0:vrrc = XXPERMDIs killed %2, 0
+ ; CHECK-NEXT: %3:vrrc = XXPERMDI killed %1, %0, 3
+ ; CHECK-NEXT: %4:vrrc = XXPERMDI killed %0, %0, 3
+...
+---
+name: test_swap_isel_livevars
+alignment: 16
+tracksRegLiveness: true
+stack:
+ - { id: 0, size: 16, alignment: 16 }
+registers:
+ - { id: 2, class: vrrc }
+ - { id: 3, class: gprc_and_gprc_nor0 }
+ - { id: 4, class: gprc }
+ - { id: 7, class: gprc }
+ - { id: 8, class: gprc }
+ - { id: 21, class: vsrc }
+ - { id: 22, class: g8rc_and_g8rc_nox0 }
+ - { id: 25, class: vsrc }
+ - { id: 26, class: vsrc }
+ - { id: 29, class: crrc }
+ - { id: 31, class: vsrc }
+body: |
+ bb.0:
+ %7:gprc = LI 0
+ %8:gprc = LI 1
+ %21:vsrc = XXLXORz
+ %22:g8rc_and_g8rc_nox0 = ADDI8 %stack.0, 0
+ %29:crrc = CMPLWI %7, 0
+
+ STXVD2X %21, $zero8, %22 :: (store (s128) into %stack.0)
+ %25:vsrc = LXVD2X $zero8, %22 :: (load (s128) from %stack.0)
+ %26:vsrc = XXPERMDI %25, %25, 2
+ %2:vrrc = COPY %26
+ %31:vsrc = XXPERMDI %2, %2, 2
+ %3:gprc_and_gprc_nor0 = LI 0
+ %4:gprc = ISEL %3, %8, %29.sub_eq
+
+ ; CHECK-LABEL: name: test_swap_isel_livevars
+ ; CHECK: %3:gprc = LI 0
+ ; CHECK-NEXT: %4:gprc = LI 1
+ ; CHECK-NEXT: %5:vsrc = XXLXORz
+ ; CHECK-NEXT: %6:g8rc_and_g8rc_nox0 = ADDI8 %stack.0, 0
+ ; CHECK-NEXT: %9:crrc = CMPLWI killed %3, 0
+ ; CHECK-NEXT: STXVD2X killed %5, $zero8, %6
+ ; CHECK-NEXT: %7:vsrc = LXVD2X $zero8, killed %6
+ ; CHECK-NEXT: %8:vsrc = XXPERMDI %7, %7, 2
+ ; CHECK-NEXT: %0:vrrc = COPY killed %8
+ ; CHECK-NEXT: %10:vsrc = COPY killed %7
+ ; CHECK-NEXT: %1:gprc_and_gprc_nor0 = LI 0
+ ; CHECK-NEXT: %2:gprc = COPY killed %1
+...
|
Fixes #159116
Prevent XXPERMDI splat optimization when the splat output register is used
in other instructions, which caused undefined register references. Also track
removed ISEL operands in simplifyToLI to prevent LiveVariables corruption
during ISEL-to-COPY conversion.