[WIP][AMDGPU][CopyPhysReg] Expand the COPY instructions using the encoded liveness mask. #151124
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
@llvm/pr-subscribers-backend-x86 @llvm/pr-subscribers-backend-amdgpu Author: Vikash Gupta (vg0204) ChangesWe will now use the liveness encoded during VirtRegRewriter within the COPY instruction[#151123 ] to expand only defined parts of the use register. It enables us to stop using implicit and implicit-def tom satisfy the MachineVerifier, thus avoiding unnecessary false dependency among the registers. This is in reference to SWDEV-498533 Patch is 2.74 MiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/151124.diff 273 Files Affected:
diff --git a/llvm/include/llvm/Target/Target.td b/llvm/include/llvm/Target/Target.td
index 4c83f8a580aa0..1f125c2cf87de 100644
--- a/llvm/include/llvm/Target/Target.td
+++ b/llvm/include/llvm/Target/Target.td
@@ -1323,7 +1323,7 @@ def REG_SEQUENCE : StandardPseudoInstruction {
}
def COPY : StandardPseudoInstruction {
let OutOperandList = (outs unknown:$dst);
- let InOperandList = (ins unknown:$src);
+ let InOperandList = (ins unknown:$src, variable_ops);
let AsmString = "";
let hasSideEffects = false;
let isAsCheapAsAMove = true;
diff --git a/llvm/lib/CodeGen/VirtRegMap.cpp b/llvm/lib/CodeGen/VirtRegMap.cpp
index 99ba893d6f096..227c0ae813934 100644
--- a/llvm/lib/CodeGen/VirtRegMap.cpp
+++ b/llvm/lib/CodeGen/VirtRegMap.cpp
@@ -213,6 +213,8 @@ class VirtRegRewriter {
void rewrite();
void addMBBLiveIns();
bool readsUndefSubreg(const MachineOperand &MO) const;
+ uint64_t calcLiveRegUnitMask(const MachineOperand &MO,
+ MCRegister PhysReg) const;
void addLiveInsForSubRanges(const LiveInterval &LI, MCRegister PhysReg) const;
void handleIdentityCopy(MachineInstr &MI);
void expandCopyBundle(MachineInstr &MI) const;
@@ -474,6 +476,77 @@ bool VirtRegRewriter::readsUndefSubreg(const MachineOperand &MO) const {
return true;
}
+// Return LaneBitmask value as unint64_t for PhysReg assigned to MO,
+// representing its live register units at its parent MI. In case of undef or
+// fully live MO, return 0u.
+uint64_t VirtRegRewriter::calcLiveRegUnitMask(const MachineOperand &MO,
+ MCRegister PhysReg) const {
+ Register Reg = MO.getReg();
+ const LiveInterval &LI = LIS->getInterval(Reg);
+ const MachineInstr &MI = *MO.getParent();
+ SlotIndex MIIndex = LIS->getInstructionIndex(MI);
+ unsigned SubRegIdx = MO.getSubReg();
+ LaneBitmask UseMask = SubRegIdx
+ ? TRI->getSubRegIndexLaneMask(SubRegIdx)
+ : (Reg.isVirtual() ? MRI->getMaxLaneMaskForVReg(Reg)
+ : LaneBitmask::getNone());
+
+ LaneBitmask LiveRegUnitMask;
+ DenseSet<unsigned> LiveRegUnits;
+
+ // dbgs() << "\n********** " << printReg(Reg, TRI) << "[ " <<
+ // printReg(PhysReg, TRI) << " ]" << " **********\n";
+
+ if (MO.isUndef())
+ return 0u;
+
+ assert(LI.liveAt(MIIndex) &&
+ "Reads of completely dead register should be marked undef already");
+
+ if (LI.hasSubRanges()) {
+ for (MCRegUnitMaskIterator Units(PhysReg, TRI); Units.isValid(); ++Units) {
+ unsigned Unit = (*Units).first;
+ LaneBitmask Mask = (*Units).second;
+ for (const LiveInterval::SubRange &S : LI.subranges()) {
+ if ((S.LaneMask & UseMask & Mask).any() && S.liveAt(MIIndex)) {
+ LiveRegUnits.insert(Unit);
+ }
+ }
+ }
+ } else {
+ for (MCRegUnitMaskIterator Units(PhysReg, TRI); Units.isValid(); ++Units) {
+ unsigned Unit = (*Units).first;
+ const LiveRange &UnitRange = LIS->getRegUnit(Unit);
+ LaneBitmask Mask = (*Units).second;
+
+ if (UnitRange.liveAt(MIIndex) && (UseMask & Mask).any())
+ LiveRegUnits.insert(Unit);
+ }
+ }
+
+ // Consider the exact subregister & create new UseMask as per the RC for it.
+ if (SubRegIdx != 0) {
+ PhysReg = TRI->getSubReg(PhysReg, SubRegIdx);
+ UseMask = (TRI->getMinimalPhysRegClass(PhysReg))->getLaneMask();
+ }
+
+ for (MCRegUnitMaskIterator Units(PhysReg, TRI); Units.isValid(); ++Units) {
+ unsigned Unit = (*Units).first;
+ LaneBitmask Mask = (*Units).second;
+ if (LiveRegUnits.count(Unit)) {
+ // dbgs() << "LIVE DEF UNIT : " << printRegUnit(Unit, TRI) << '\n';
+ LiveRegUnitMask |= Mask;
+ }
+ }
+
+ // dbgs() << "UseMask : " << PrintLaneMask(UseMask) << '\n';
+ // dbgs() << "LiveRegUnitMask : " << PrintLaneMask(LiveRegUnitMask) << '\n';
+ if (UseMask == LiveRegUnitMask)
+ return 0u;
+
+ return LiveRegUnitMask.getAsInteger();
+}
+
void VirtRegRewriter::handleIdentityCopy(MachineInstr &MI) {
if (!MI.isIdentityCopy())
return;
@@ -495,7 +568,11 @@ void VirtRegRewriter::handleIdentityCopy(MachineInstr &MI) {
// give us additional liveness information: The target (super-)register
// must not be valid before this point. Replace the COPY with a KILL
// instruction to maintain this information.
- if (MI.getOperand(1).isUndef() || MI.getNumOperands() > 2) {
+
+ // Avoid COPY with an exact 3 operand, wiith third operand be Mask, as
+ // it same as a COPY with no additional liveness information.
+ if (MI.getOperand(1).isUndef() || MI.getNumOperands() > 3 ||
+ (MI.getNumOperands() == 3 && !MI.getOperand(2).isImm())) {
MI.setDesc(TII->get(TargetOpcode::KILL));
LLVM_DEBUG(dbgs() << " replace by: " << MI);
return;
@@ -641,11 +718,14 @@ void VirtRegRewriter::rewrite() {
SmallVector<Register, 8> SuperDeads;
SmallVector<Register, 8> SuperDefs;
SmallVector<Register, 8> SuperKills;
+ uint64_t Mask;
for (MachineFunction::iterator MBBI = MF->begin(), MBBE = MF->end();
MBBI != MBBE; ++MBBI) {
LLVM_DEBUG(MBBI->print(dbgs(), Indexes));
for (MachineInstr &MI : llvm::make_early_inc_range(MBBI->instrs())) {
+ // reset for each MI.
+ Mask = 0u;
for (MachineOperand &MO : MI.operands()) {
// Make sure MRI knows about registers clobbered by regmasks.
if (MO.isRegMask())
@@ -663,6 +743,9 @@ void VirtRegRewriter::rewrite() {
RewriteRegs.insert(PhysReg);
assert(!MRI->isReserved(PhysReg) && "Reserved register assignment");
+ if (MO.isUse() && MI.isCopy())
+ Mask = calcLiveRegUnitMask(MO, PhysReg);
+
// Preserve semantics of sub-register operands.
unsigned SubReg = MO.getSubReg();
if (SubReg != 0) {
@@ -739,6 +822,10 @@ void VirtRegRewriter::rewrite() {
MO.setIsRenamable(true);
}
+ // Add LaneBitmask as MO_Imm
+ if (MI.isCopy() && Mask)
+ MI.addOperand(*MF, MachineOperand::CreateImm(Mask));
+
// Add any missing super-register kills after rewriting the whole
// instruction.
while (!SuperKills.empty())
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index 571f3efd68260..29c6d18a65308 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -691,16 +691,8 @@ static void indirectCopyToAGPR(const SIInstrInfo &TII,
I->clearRegisterKills(DefOp.getReg(), &RI);
}
- MachineInstrBuilder Builder =
- BuildMI(MBB, MI, DL, TII.get(AMDGPU::V_ACCVGPR_WRITE_B32_e64), DestReg)
- .add(DefOp);
- if (ImpDefSuperReg)
- Builder.addReg(ImpDefSuperReg, RegState::Define | RegState::Implicit);
-
- if (ImpUseSuperReg) {
- Builder.addReg(ImpUseSuperReg,
- getKillRegState(KillSrc) | RegState::Implicit);
- }
+ BuildMI(MBB, MI, DL, TII.get(AMDGPU::V_ACCVGPR_WRITE_B32_e64), DestReg)
+ .add(DefOp);
return;
}
@@ -744,27 +736,26 @@ static void indirectCopyToAGPR(const SIInstrInfo &TII,
MachineInstrBuilder UseBuilder = BuildMI(MBB, MI, DL, TII.get(TmpCopyOp), Tmp)
.addReg(SrcReg, getKillRegState(KillSrc));
- if (ImpUseSuperReg) {
- UseBuilder.addReg(ImpUseSuperReg,
- getKillRegState(KillSrc) | RegState::Implicit);
- }
- MachineInstrBuilder DefBuilder
- = BuildMI(MBB, MI, DL, TII.get(AMDGPU::V_ACCVGPR_WRITE_B32_e64), DestReg)
- .addReg(Tmp, RegState::Kill);
-
- if (ImpDefSuperReg)
- DefBuilder.addReg(ImpDefSuperReg, RegState::Define | RegState::Implicit);
+ BuildMI(MBB, MI, DL, TII.get(AMDGPU::V_ACCVGPR_WRITE_B32_e64), DestReg)
+ .addReg(Tmp, RegState::Kill);
}
static void expandSGPRCopy(const SIInstrInfo &TII, MachineBasicBlock &MBB,
MachineBasicBlock::iterator MI, const DebugLoc &DL,
MCRegister DestReg, MCRegister SrcReg, bool KillSrc,
- const TargetRegisterClass *RC, bool Forward) {
+ const TargetRegisterClass *RC, bool Forward,
+ uint64_t LiveRegUnitMaskVal) {
const SIRegisterInfo &RI = TII.getRegisterInfo();
ArrayRef<int16_t> BaseIndices = RI.getRegSplitParts(RC, 4);
MachineBasicBlock::iterator I = MI;
- MachineInstr *FirstMI = nullptr, *LastMI = nullptr;
+ bool isSrcRegFullLive = LiveRegUnitMaskVal == 0;
+
+ uint64_t TestMaskVal = 0x0000000000000003;
+ uint8_t ShiftVal = 2;
+
+ if (!Forward)
+ TestMaskVal = TestMaskVal << (ShiftVal * (BaseIndices.size() - 1));
for (unsigned Idx = 0; Idx < BaseIndices.size(); ++Idx) {
int16_t SubIdx = BaseIndices[Idx];
@@ -772,41 +763,47 @@ static void expandSGPRCopy(const SIInstrInfo &TII, MachineBasicBlock &MBB,
Register SrcSubReg = RI.getSubReg(SrcReg, SubIdx);
assert(DestSubReg && SrcSubReg && "Failed to find subregs!");
unsigned Opcode = AMDGPU::S_MOV_B32;
+ bool IsFirstSubreg = Idx == 0;
+
+ if (!IsFirstSubreg) {
+ TestMaskVal = Forward ? TestMaskVal << ShiftVal : TestMaskVal >> ShiftVal;
+ }
+
+ // Check for liveness of current subregister using TestMaskVal.
+ if (!isSrcRegFullLive && (LiveRegUnitMaskVal & TestMaskVal) == uint64_t(0))
+ continue;
// Is SGPR aligned? If so try to combine with next.
bool AlignedDest = ((DestSubReg - AMDGPU::SGPR0) % 2) == 0;
bool AlignedSrc = ((SrcSubReg - AMDGPU::SGPR0) % 2) == 0;
- if (AlignedDest && AlignedSrc && (Idx + 1 < BaseIndices.size())) {
+ bool isSrc64Live = true;
+
+ if (!isSrcRegFullLive)
+ isSrc64Live = Forward
+ ? ((LiveRegUnitMaskVal & (TestMaskVal << ShiftVal)) !=
+ uint64_t(0))
+ : ((LiveRegUnitMaskVal & (TestMaskVal >> ShiftVal)) !=
+ uint64_t(0));
+
+ if (isSrc64Live && AlignedDest && AlignedSrc &&
+ (Idx + 1 < BaseIndices.size())) {
// Can use SGPR64 copy
unsigned Channel = RI.getChannelFromSubReg(SubIdx);
SubIdx = RI.getSubRegFromChannel(Channel, 2);
DestSubReg = RI.getSubReg(DestReg, SubIdx);
SrcSubReg = RI.getSubReg(SrcReg, SubIdx);
assert(DestSubReg && SrcSubReg && "Failed to find subregs!");
+ TestMaskVal = Forward ? TestMaskVal << ShiftVal : TestMaskVal >> ShiftVal;
Opcode = AMDGPU::S_MOV_B64;
Idx++;
}
- LastMI = BuildMI(MBB, I, DL, TII.get(Opcode), DestSubReg)
- .addReg(SrcSubReg)
- .addReg(SrcReg, RegState::Implicit);
-
- if (!FirstMI)
- FirstMI = LastMI;
+ BuildMI(MBB, I, DL, TII.get(Opcode), DestSubReg)
+ .addReg(SrcSubReg, getKillRegState(KillSrc));
if (!Forward)
I--;
}
-
- assert(FirstMI && LastMI);
- if (!Forward)
- std::swap(FirstMI, LastMI);
-
- FirstMI->addOperand(
- MachineOperand::CreateReg(DestReg, true /*IsDef*/, true /*IsImp*/));
-
- if (KillSrc)
- LastMI->addRegisterKilled(SrcReg, &RI);
}
void SIInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
@@ -819,6 +816,13 @@ void SIInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
const TargetRegisterClass *SrcRC = RI.getPhysRegBaseClass(SrcReg);
unsigned SrcSize = RI.getRegSizeInBits(*SrcRC);
+ uint64_t LiveRegUnitMaskVal = 0;
+ if (MI->getNumOperands() > 2 && MI->getOperand(2).isImm()) {
+ LiveRegUnitMaskVal = MI->getOperand(2).getImm();
+ }
+
+ bool isSrcRegFullLive = LiveRegUnitMaskVal == 0;
+
// The rest of copyPhysReg assumes Src and Dst size are the same size.
// TODO-GFX11_16BIT If all true 16 bit instruction patterns are completed can
// we remove Fix16BitCopies and this code block?
@@ -1052,16 +1056,15 @@ void SIInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
}
if (ST.hasPkMovB32()) {
BuildMI(MBB, MI, DL, get(AMDGPU::V_PK_MOV_B32), DestReg)
- .addImm(SISrcMods::OP_SEL_1)
- .addReg(SrcReg)
- .addImm(SISrcMods::OP_SEL_0 | SISrcMods::OP_SEL_1)
- .addReg(SrcReg)
- .addImm(0) // op_sel_lo
- .addImm(0) // op_sel_hi
- .addImm(0) // neg_lo
- .addImm(0) // neg_hi
- .addImm(0) // clamp
- .addReg(SrcReg, getKillRegState(KillSrc) | RegState::Implicit);
+ .addImm(SISrcMods::OP_SEL_1)
+ .addReg(SrcReg, getKillRegState(KillSrc))
+ .addImm(SISrcMods::OP_SEL_0 | SISrcMods::OP_SEL_1)
+ .addReg(SrcReg, getKillRegState(KillSrc))
+ .addImm(0) // op_sel_lo
+ .addImm(0) // op_sel_hi
+ .addImm(0) // neg_lo
+ .addImm(0) // neg_hi
+ .addImm(0); // clamp
return;
}
}
@@ -1074,12 +1077,14 @@ void SIInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
}
const bool CanKillSuperReg = KillSrc && !RI.regsOverlap(SrcReg, DestReg);
expandSGPRCopy(*this, MBB, MI, DL, DestReg, SrcReg, CanKillSuperReg, RC,
- Forward);
+ Forward, LiveRegUnitMaskVal);
return;
}
unsigned EltSize = 4;
unsigned Opcode = AMDGPU::V_MOV_B32_e32;
+ uint64_t TestMaskVal = 0x0000000000000003;
+ uint8_t ShiftVal = 2;
if (RI.isAGPRClass(RC)) {
if (ST.hasGFX90AInsts() && RI.isAGPRClass(SrcRC))
Opcode = AMDGPU::V_ACCVGPR_MOV_B32;
@@ -1094,12 +1099,18 @@ void SIInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
(RI.isProperlyAlignedRC(*RC) &&
(SrcRC == RC || RI.isSGPRClass(SrcRC)))) {
// TODO: In 96-bit case, could do a 64-bit mov and then a 32-bit mov.
+ // TODO: In case of partial liveness, could do mix of 64-bit and 32-bit
+ // moves. Look expandSGPRCopy function for reference.
if (ST.hasMovB64()) {
Opcode = AMDGPU::V_MOV_B64_e32;
EltSize = 8;
+ TestMaskVal = 0x000000000000000F;
+ ShiftVal = 4;
} else if (ST.hasPkMovB32()) {
Opcode = AMDGPU::V_PK_MOV_B32;
EltSize = 8;
+ TestMaskVal = 0x000000000000000F;
+ ShiftVal = 4;
}
}
@@ -1114,6 +1125,10 @@ void SIInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
ArrayRef<int16_t> SubIndices = RI.getRegSplitParts(RC, EltSize);
+ // The TestMaskVal will scan from right to left.
+ if (!Forward)
+ TestMaskVal = TestMaskVal << (ShiftVal * (SubIndices.size() - 1));
+
// If there is an overlap, we can't kill the super-register on the last
// instruction, since it will also kill the components made live by this def.
const bool Overlap = RI.regsOverlap(SrcReg, DestReg);
@@ -1130,7 +1145,14 @@ void SIInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
assert(DestSubReg && SrcSubReg && "Failed to find subregs!");
bool IsFirstSubreg = Idx == 0;
- bool UseKill = CanKillSuperReg && Idx == SubIndices.size() - 1;
+ bool UseKill = CanKillSuperReg;
+
+ if (!IsFirstSubreg) {
+ TestMaskVal = Forward ? TestMaskVal << ShiftVal : TestMaskVal >> ShiftVal;
+ }
+
+ if (!isSrcRegFullLive && (LiveRegUnitMaskVal & TestMaskVal) == uint64_t(0))
+ continue;
if (Opcode == AMDGPU::INSTRUCTION_LIST_END) {
Register ImpDefSuper = IsFirstSubreg ? Register(DestReg) : Register();
@@ -1141,24 +1163,18 @@ void SIInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
MachineInstrBuilder MIB =
BuildMI(MBB, MI, DL, get(AMDGPU::V_PK_MOV_B32), DestSubReg)
.addImm(SISrcMods::OP_SEL_1)
- .addReg(SrcSubReg)
+ .addReg(SrcSubReg, getKillRegState(UseKill))
.addImm(SISrcMods::OP_SEL_0 | SISrcMods::OP_SEL_1)
- .addReg(SrcSubReg)
- .addImm(0) // op_sel_lo
- .addImm(0) // op_sel_hi
- .addImm(0) // neg_lo
- .addImm(0) // neg_hi
- .addImm(0) // clamp
- .addReg(SrcReg, getKillRegState(UseKill) | RegState::Implicit);
- if (IsFirstSubreg)
- MIB.addReg(DestReg, RegState::Define | RegState::Implicit);
+ .addReg(SrcSubReg, getKillRegState(UseKill))
+ .addImm(0) // op_sel_lo
+ .addImm(0) // op_sel_hi
+ .addImm(0) // neg_lo
+ .addImm(0) // neg_hi
+ .addImm(0); // clamp
} else {
MachineInstrBuilder Builder =
- BuildMI(MBB, MI, DL, get(Opcode), DestSubReg).addReg(SrcSubReg);
- if (IsFirstSubreg)
- Builder.addReg(DestReg, RegState::Define | RegState::Implicit);
-
- Builder.addReg(SrcReg, getKillRegState(UseKill) | RegState::Implicit);
+ BuildMI(MBB, MI, DL, get(Opcode), DestSubReg)
+ .addReg(SrcSubReg, getKillRegState(UseKill));
}
}
}
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/addo.ll b/llvm/test/CodeGen/AMDGPU/GlobalISel/addo.ll
index 38374d1689366..3cab3fb763523 100644
--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/addo.ll
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/addo.ll
@@ -676,8 +676,8 @@ define amdgpu_ps i64 @s_saddo_i64(i64 inreg %a, i64 inreg %b) {
; GFX7-LABEL: s_saddo_i64:
; GFX7: ; %bb.0:
; GFX7-NEXT: s_add_u32 s4, s0, s2
-; GFX7-NEXT: v_mov_b32_e32 v0, s0
; GFX7-NEXT: s_addc_u32 s5, s1, s3
+; GFX7-NEXT: v_mov_b32_e32 v0, s0
; GFX7-NEXT: v_mov_b32_e32 v1, s1
; GFX7-NEXT: v_cmp_lt_i64_e32 vcc, s[4:5], v[0:1]
; GFX7-NEXT: v_cmp_lt_i64_e64 s[0:1], s[2:3], 0
@@ -693,8 +693,8 @@ define amdgpu_ps i64 @s_saddo_i64(i64 inreg %a, i64 inreg %b) {
; GFX8-LABEL: s_saddo_i64:
; GFX8: ; %bb.0:
; GFX8-NEXT: s_add_u32 s4, s0, s2
-; GFX8-NEXT: v_mov_b32_e32 v0, s0
; GFX8-NEXT: s_addc_u32 s5, s1, s3
+; GFX8-NEXT: v_mov_b32_e32 v0, s0
; GFX8-NEXT: v_mov_b32_e32 v1, s1
; GFX8-NEXT: v_cmp_lt_i64_e32 vcc, s[4:5], v[0:1]
; GFX8-NEXT: v_cmp_lt_i64_e64 s[0:1], s[2:3], 0
@@ -710,8 +710,8 @@ define amdgpu_ps i64 @s_saddo_i64(i64 inreg %a, i64 inreg %b) {
; GFX9-LABEL: s_saddo_i64:
; GFX9: ; %bb.0:
; GFX9-NEXT: s_add_u32 s4, s0, s2
-; GFX9-NEXT: v_mov_b32_e32 v0, s0
; GFX9-NEXT: s_addc_u32 s5, s1, s3
+; GFX9-NEXT: v_mov_b32_e32 v0, s0
; GFX9-NEXT: v_mov_b32_e32 v1, s1
; GFX9-NEXT: v_cmp_lt_i64_e32 vcc, s[4:5], v[0:1]
; GFX9-NEXT: v_cmp_lt_i64_e64 s[0:1], s[2:3], 0
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/atomic_optimizations_mul_one.ll b/llvm/test/CodeGen/AMDGPU/GlobalISel/atomic_optimizations_mul_one.ll
index 28ed88f4cf8fb..31606dea7d335 100644
--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/atomic_optimizations_mul_one.ll
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/atomic_optimizations_mul_one.ll
@@ -96,8 +96,8 @@ define amdgpu_cs void @atomic_add_and_format(<4 x i32> inreg %arg) {
; GCN-NEXT: s_waitcnt vmcnt(0)
; GCN-NEXT: v_readfirstlane_b32 s4, v1
; GCN-NEXT: v_add_i32_e32 v4, vcc, s4, v0
-; GCN-NEXT: s_waitcnt expcnt(0)
; GCN-NEXT: v_mov_b32_e32 v0, s0
+; GCN-NEXT: s_waitcnt expcnt(0)
; GCN-NEXT: v_mov_b32_e32 v1, s1
; GCN-NEXT: v_mov_b32_e32 v2, s2
; GCN-NEXT: v_mov_b32_e32 v3, s3
@@ -192,8 +192,8 @@ define amdgpu_cs void @atomic_sub_and_format(<4 x i32> inreg %arg) {
; GCN-NEXT: s_waitcnt vmcnt(0)
; GCN-NEXT: v_readfirstlane_b32 s4, v1
; GCN-NEXT: v_sub_i32_e32 v4, vcc, s4, v0
-; GCN-NEXT: s_waitcnt expcnt(0)
; GCN-NEXT: v_mov_b32_e32 v0, s0
+; GCN-NEXT: s_waitcnt expcnt(0)
; GCN-NEXT: v_mov_b32_e32 v1, s1
; GCN-NEXT: v_mov_b32_e32 v2, s2
; GCN-NEXT: v_mov_b32_e32 v3, s3
@@ -294,8 +294,8 @@ define amdgpu_cs void @atomic_xor_and_format(<4 x i32> inreg %arg) {
; GCN-NEXT: v_readfirstlane_b32 s4, v1
; GCN-NEXT: v_and_b32_e32 v0, 1, v0
; GCN-NEXT: v_xor_b32_e32 v4, s4, v0
-; GCN-NEXT: s_waitcnt expcnt(0)
; GCN-NEXT: v_mov_b32_e32 v0, s0
+; GCN-NEXT: s_waitcnt expcnt(0)
; GCN-NEXT: v_mov_b32_e32 v1, s1
; GCN-NEXT: v_mov_b32_e32 v2, s2
; GCN-NEXT: v_mov_b32_e32 v3, s3
@@ -392,8 +392,8 @@ define amdgpu_cs void @atomic_ptr_add_and_format(ptr addrspace(8) inreg %arg) {
; GCN-NEXT: s_waitcnt vmcnt(0)
; GCN-NEXT: v_readfirstlane_b32 s4, v1
; GCN-NEXT: v_add_i32_e32 v4, vcc, s4, v0
-; GCN-NEXT: s_waitcnt expcnt(0)
; GCN-NEXT: v_mov_b32_e32 v0, s0
+; GCN-NEXT: s_waitcnt expcnt(0)
; GCN-NEXT: v_mov_b32_e32 v1, s1
; GCN-NEXT: v_mov_b32_e32 v2, s2
; GCN-NEXT: v_mov_b32_e32 v3, s3
@@ -492,8 +492,8 @@ define amdgpu_cs void @atomic_ptr_sub_and_format(ptr addrspace(8) inreg %arg) {
; GCN-NEXT: s_waitcnt vmcnt(0)
; GCN-NEXT: v_readfirstlane_b32 s4, v1
;...
[truncated]
|
Member
|
@llvm/pr-subscribers-llvm-regalloc Author: Vikash Gupta (vg0204) ChangesWe will now use the liveness encoded during VirtRegRewriter within the COPY instruction[#151123 ] to expand only defined parts of the use register. It enables us to stop using implicit and implicit-def tom satisfy the MachineVerifier, thus avoiding unnecessary false dependency among the registers. This is in reference to SWDEV-498533 Patch is 2.74 MiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/151124.diff 273 Files Affected:
diff --git a/llvm/include/llvm/Target/Target.td b/llvm/include/llvm/Target/Target.td
index 4c83f8a580aa0..1f125c2cf87de 100644
--- a/llvm/include/llvm/Target/Target.td
+++ b/llvm/include/llvm/Target/Target.td
@@ -1323,7 +1323,7 @@ def REG_SEQUENCE : StandardPseudoInstruction {
}
def COPY : StandardPseudoInstruction {
let OutOperandList = (outs unknown:$dst);
- let InOperandList = (ins unknown:$src);
+ let InOperandList = (ins unknown:$src, variable_ops);
let AsmString = "";
let hasSideEffects = false;
let isAsCheapAsAMove = true;
diff --git a/llvm/lib/CodeGen/VirtRegMap.cpp b/llvm/lib/CodeGen/VirtRegMap.cpp
index 99ba893d6f096..227c0ae813934 100644
--- a/llvm/lib/CodeGen/VirtRegMap.cpp
+++ b/llvm/lib/CodeGen/VirtRegMap.cpp
@@ -213,6 +213,8 @@ class VirtRegRewriter {
void rewrite();
void addMBBLiveIns();
bool readsUndefSubreg(const MachineOperand &MO) const;
+ uint64_t calcLiveRegUnitMask(const MachineOperand &MO,
+ MCRegister PhysReg) const;
void addLiveInsForSubRanges(const LiveInterval &LI, MCRegister PhysReg) const;
void handleIdentityCopy(MachineInstr &MI);
void expandCopyBundle(MachineInstr &MI) const;
@@ -474,6 +476,77 @@ bool VirtRegRewriter::readsUndefSubreg(const MachineOperand &MO) const {
return true;
}
+// Return LaneBitmask value as unint64_t for PhysReg assigned to MO,
+// representing its live register units at its parent MI. In case of undef or
+// fully live MO, return 0u.
+uint64_t VirtRegRewriter::calcLiveRegUnitMask(const MachineOperand &MO,
+ MCRegister PhysReg) const {
+ Register Reg = MO.getReg();
+ const LiveInterval &LI = LIS->getInterval(Reg);
+ const MachineInstr &MI = *MO.getParent();
+ SlotIndex MIIndex = LIS->getInstructionIndex(MI);
+ unsigned SubRegIdx = MO.getSubReg();
+ LaneBitmask UseMask = SubRegIdx
+ ? TRI->getSubRegIndexLaneMask(SubRegIdx)
+ : (Reg.isVirtual() ? MRI->getMaxLaneMaskForVReg(Reg)
+ : LaneBitmask::getNone());
+
+ LaneBitmask LiveRegUnitMask;
+ DenseSet<unsigned> LiveRegUnits;
+
+ // dbgs() << "\n********** " << printReg(Reg, TRI) << "[ " <<
+ // printReg(PhysReg, TRI) << " ]" << " **********\n";
+
+ if (MO.isUndef())
+ return 0u;
+
+ assert(LI.liveAt(MIIndex) &&
+ "Reads of completely dead register should be marked undef already");
+
+ if (LI.hasSubRanges()) {
+ for (MCRegUnitMaskIterator Units(PhysReg, TRI); Units.isValid(); ++Units) {
+ unsigned Unit = (*Units).first;
+ LaneBitmask Mask = (*Units).second;
+ for (const LiveInterval::SubRange &S : LI.subranges()) {
+ if ((S.LaneMask & UseMask & Mask).any() && S.liveAt(MIIndex)) {
+ LiveRegUnits.insert(Unit);
+ }
+ }
+ }
+ } else {
+ for (MCRegUnitMaskIterator Units(PhysReg, TRI); Units.isValid(); ++Units) {
+ unsigned Unit = (*Units).first;
+ const LiveRange &UnitRange = LIS->getRegUnit(Unit);
+ LaneBitmask Mask = (*Units).second;
+
+ if (UnitRange.liveAt(MIIndex) && (UseMask & Mask).any())
+ LiveRegUnits.insert(Unit);
+ }
+ }
+
+ // Consider the exact subregister & create new UseMask as per the RC for it.
+ if (SubRegIdx != 0) {
+ PhysReg = TRI->getSubReg(PhysReg, SubRegIdx);
+ UseMask = (TRI->getMinimalPhysRegClass(PhysReg))->getLaneMask();
+ }
+
+ for (MCRegUnitMaskIterator Units(PhysReg, TRI); Units.isValid(); ++Units) {
+ unsigned Unit = (*Units).first;
+ LaneBitmask Mask = (*Units).second;
+ if (LiveRegUnits.count(Unit)) {
+ // dbgs() << "LIVE DEF UNIT : " << printRegUnit(Unit, TRI) << '\n';
+ LiveRegUnitMask |= Mask;
+ }
+ }
+
+ // dbgs() << "UseMask : " << PrintLaneMask(UseMask) << '\n';
+ // dbgs() << "LiveRegUnitMask : " << PrintLaneMask(LiveRegUnitMask) << '\n';
+ if (UseMask == LiveRegUnitMask)
+ return 0u;
+
+ return LiveRegUnitMask.getAsInteger();
+}
+
void VirtRegRewriter::handleIdentityCopy(MachineInstr &MI) {
if (!MI.isIdentityCopy())
return;
@@ -495,7 +568,11 @@ void VirtRegRewriter::handleIdentityCopy(MachineInstr &MI) {
// give us additional liveness information: The target (super-)register
// must not be valid before this point. Replace the COPY with a KILL
// instruction to maintain this information.
- if (MI.getOperand(1).isUndef() || MI.getNumOperands() > 2) {
+
+ // Avoid COPY with an exact 3 operand, wiith third operand be Mask, as
+ // it same as a COPY with no additional liveness information.
+ if (MI.getOperand(1).isUndef() || MI.getNumOperands() > 3 ||
+ (MI.getNumOperands() == 3 && !MI.getOperand(2).isImm())) {
MI.setDesc(TII->get(TargetOpcode::KILL));
LLVM_DEBUG(dbgs() << " replace by: " << MI);
return;
@@ -641,11 +718,14 @@ void VirtRegRewriter::rewrite() {
SmallVector<Register, 8> SuperDeads;
SmallVector<Register, 8> SuperDefs;
SmallVector<Register, 8> SuperKills;
+ uint64_t Mask;
for (MachineFunction::iterator MBBI = MF->begin(), MBBE = MF->end();
MBBI != MBBE; ++MBBI) {
LLVM_DEBUG(MBBI->print(dbgs(), Indexes));
for (MachineInstr &MI : llvm::make_early_inc_range(MBBI->instrs())) {
+ // reset for each MI.
+ Mask = 0u;
for (MachineOperand &MO : MI.operands()) {
// Make sure MRI knows about registers clobbered by regmasks.
if (MO.isRegMask())
@@ -663,6 +743,9 @@ void VirtRegRewriter::rewrite() {
RewriteRegs.insert(PhysReg);
assert(!MRI->isReserved(PhysReg) && "Reserved register assignment");
+ if (MO.isUse() && MI.isCopy())
+ Mask = calcLiveRegUnitMask(MO, PhysReg);
+
// Preserve semantics of sub-register operands.
unsigned SubReg = MO.getSubReg();
if (SubReg != 0) {
@@ -739,6 +822,10 @@ void VirtRegRewriter::rewrite() {
MO.setIsRenamable(true);
}
+ // Add LaneBitmask as MO_Imm
+ if (MI.isCopy() && Mask)
+ MI.addOperand(*MF, MachineOperand::CreateImm(Mask));
+
// Add any missing super-register kills after rewriting the whole
// instruction.
while (!SuperKills.empty())
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index 571f3efd68260..29c6d18a65308 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -691,16 +691,8 @@ static void indirectCopyToAGPR(const SIInstrInfo &TII,
I->clearRegisterKills(DefOp.getReg(), &RI);
}
- MachineInstrBuilder Builder =
- BuildMI(MBB, MI, DL, TII.get(AMDGPU::V_ACCVGPR_WRITE_B32_e64), DestReg)
- .add(DefOp);
- if (ImpDefSuperReg)
- Builder.addReg(ImpDefSuperReg, RegState::Define | RegState::Implicit);
-
- if (ImpUseSuperReg) {
- Builder.addReg(ImpUseSuperReg,
- getKillRegState(KillSrc) | RegState::Implicit);
- }
+ BuildMI(MBB, MI, DL, TII.get(AMDGPU::V_ACCVGPR_WRITE_B32_e64), DestReg)
+ .add(DefOp);
return;
}
@@ -744,27 +736,26 @@ static void indirectCopyToAGPR(const SIInstrInfo &TII,
MachineInstrBuilder UseBuilder = BuildMI(MBB, MI, DL, TII.get(TmpCopyOp), Tmp)
.addReg(SrcReg, getKillRegState(KillSrc));
- if (ImpUseSuperReg) {
- UseBuilder.addReg(ImpUseSuperReg,
- getKillRegState(KillSrc) | RegState::Implicit);
- }
- MachineInstrBuilder DefBuilder
- = BuildMI(MBB, MI, DL, TII.get(AMDGPU::V_ACCVGPR_WRITE_B32_e64), DestReg)
- .addReg(Tmp, RegState::Kill);
-
- if (ImpDefSuperReg)
- DefBuilder.addReg(ImpDefSuperReg, RegState::Define | RegState::Implicit);
+ BuildMI(MBB, MI, DL, TII.get(AMDGPU::V_ACCVGPR_WRITE_B32_e64), DestReg)
+ .addReg(Tmp, RegState::Kill);
}
static void expandSGPRCopy(const SIInstrInfo &TII, MachineBasicBlock &MBB,
MachineBasicBlock::iterator MI, const DebugLoc &DL,
MCRegister DestReg, MCRegister SrcReg, bool KillSrc,
- const TargetRegisterClass *RC, bool Forward) {
+ const TargetRegisterClass *RC, bool Forward,
+ uint64_t LiveRegUnitMaskVal) {
const SIRegisterInfo &RI = TII.getRegisterInfo();
ArrayRef<int16_t> BaseIndices = RI.getRegSplitParts(RC, 4);
MachineBasicBlock::iterator I = MI;
- MachineInstr *FirstMI = nullptr, *LastMI = nullptr;
+ bool isSrcRegFullLive = LiveRegUnitMaskVal == 0;
+
+ uint64_t TestMaskVal = 0x0000000000000003;
+ uint8_t ShiftVal = 2;
+
+ if (!Forward)
+ TestMaskVal = TestMaskVal << (ShiftVal * (BaseIndices.size() - 1));
for (unsigned Idx = 0; Idx < BaseIndices.size(); ++Idx) {
int16_t SubIdx = BaseIndices[Idx];
@@ -772,41 +763,47 @@ static void expandSGPRCopy(const SIInstrInfo &TII, MachineBasicBlock &MBB,
Register SrcSubReg = RI.getSubReg(SrcReg, SubIdx);
assert(DestSubReg && SrcSubReg && "Failed to find subregs!");
unsigned Opcode = AMDGPU::S_MOV_B32;
+ bool IsFirstSubreg = Idx == 0;
+
+ if (!IsFirstSubreg) {
+ TestMaskVal = Forward ? TestMaskVal << ShiftVal : TestMaskVal >> ShiftVal;
+ }
+
+ // Check for liveness of current subregister using TestMaskVal.
+ if (!isSrcRegFullLive && (LiveRegUnitMaskVal & TestMaskVal) == uint64_t(0))
+ continue;
// Is SGPR aligned? If so try to combine with next.
bool AlignedDest = ((DestSubReg - AMDGPU::SGPR0) % 2) == 0;
bool AlignedSrc = ((SrcSubReg - AMDGPU::SGPR0) % 2) == 0;
- if (AlignedDest && AlignedSrc && (Idx + 1 < BaseIndices.size())) {
+ bool isSrc64Live = true;
+
+ if (!isSrcRegFullLive)
+ isSrc64Live = Forward
+ ? ((LiveRegUnitMaskVal & (TestMaskVal << ShiftVal)) !=
+ uint64_t(0))
+ : ((LiveRegUnitMaskVal & (TestMaskVal >> ShiftVal)) !=
+ uint64_t(0));
+
+ if (isSrc64Live && AlignedDest && AlignedSrc &&
+ (Idx + 1 < BaseIndices.size())) {
// Can use SGPR64 copy
unsigned Channel = RI.getChannelFromSubReg(SubIdx);
SubIdx = RI.getSubRegFromChannel(Channel, 2);
DestSubReg = RI.getSubReg(DestReg, SubIdx);
SrcSubReg = RI.getSubReg(SrcReg, SubIdx);
assert(DestSubReg && SrcSubReg && "Failed to find subregs!");
+ TestMaskVal = Forward ? TestMaskVal << ShiftVal : TestMaskVal >> ShiftVal;
Opcode = AMDGPU::S_MOV_B64;
Idx++;
}
- LastMI = BuildMI(MBB, I, DL, TII.get(Opcode), DestSubReg)
- .addReg(SrcSubReg)
- .addReg(SrcReg, RegState::Implicit);
-
- if (!FirstMI)
- FirstMI = LastMI;
+ BuildMI(MBB, I, DL, TII.get(Opcode), DestSubReg)
+ .addReg(SrcSubReg, getKillRegState(KillSrc));
if (!Forward)
I--;
}
-
- assert(FirstMI && LastMI);
- if (!Forward)
- std::swap(FirstMI, LastMI);
-
- FirstMI->addOperand(
- MachineOperand::CreateReg(DestReg, true /*IsDef*/, true /*IsImp*/));
-
- if (KillSrc)
- LastMI->addRegisterKilled(SrcReg, &RI);
}
void SIInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
@@ -819,6 +816,13 @@ void SIInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
const TargetRegisterClass *SrcRC = RI.getPhysRegBaseClass(SrcReg);
unsigned SrcSize = RI.getRegSizeInBits(*SrcRC);
+ uint64_t LiveRegUnitMaskVal = 0;
+ if (MI->getNumOperands() > 2 && MI->getOperand(2).isImm()) {
+ LiveRegUnitMaskVal = MI->getOperand(2).getImm();
+ }
+
+ bool isSrcRegFullLive = LiveRegUnitMaskVal == 0;
+
// The rest of copyPhysReg assumes Src and Dst size are the same size.
// TODO-GFX11_16BIT If all true 16 bit instruction patterns are completed can
// we remove Fix16BitCopies and this code block?
@@ -1052,16 +1056,15 @@ void SIInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
}
if (ST.hasPkMovB32()) {
BuildMI(MBB, MI, DL, get(AMDGPU::V_PK_MOV_B32), DestReg)
- .addImm(SISrcMods::OP_SEL_1)
- .addReg(SrcReg)
- .addImm(SISrcMods::OP_SEL_0 | SISrcMods::OP_SEL_1)
- .addReg(SrcReg)
- .addImm(0) // op_sel_lo
- .addImm(0) // op_sel_hi
- .addImm(0) // neg_lo
- .addImm(0) // neg_hi
- .addImm(0) // clamp
- .addReg(SrcReg, getKillRegState(KillSrc) | RegState::Implicit);
+ .addImm(SISrcMods::OP_SEL_1)
+ .addReg(SrcReg, getKillRegState(KillSrc))
+ .addImm(SISrcMods::OP_SEL_0 | SISrcMods::OP_SEL_1)
+ .addReg(SrcReg, getKillRegState(KillSrc))
+ .addImm(0) // op_sel_lo
+ .addImm(0) // op_sel_hi
+ .addImm(0) // neg_lo
+ .addImm(0) // neg_hi
+ .addImm(0); // clamp
return;
}
}
@@ -1074,12 +1077,14 @@ void SIInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
}
const bool CanKillSuperReg = KillSrc && !RI.regsOverlap(SrcReg, DestReg);
expandSGPRCopy(*this, MBB, MI, DL, DestReg, SrcReg, CanKillSuperReg, RC,
- Forward);
+ Forward, LiveRegUnitMaskVal);
return;
}
unsigned EltSize = 4;
unsigned Opcode = AMDGPU::V_MOV_B32_e32;
+ uint64_t TestMaskVal = 0x0000000000000003;
+ uint8_t ShiftVal = 2;
if (RI.isAGPRClass(RC)) {
if (ST.hasGFX90AInsts() && RI.isAGPRClass(SrcRC))
Opcode = AMDGPU::V_ACCVGPR_MOV_B32;
@@ -1094,12 +1099,18 @@ void SIInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
(RI.isProperlyAlignedRC(*RC) &&
(SrcRC == RC || RI.isSGPRClass(SrcRC)))) {
// TODO: In 96-bit case, could do a 64-bit mov and then a 32-bit mov.
+ // TODO: In case of partial liveness, could do mix of 64-bit and 32-bit
+ // moves. Look expandSGPRCopy function for reference.
if (ST.hasMovB64()) {
Opcode = AMDGPU::V_MOV_B64_e32;
EltSize = 8;
+ TestMaskVal = 0x000000000000000F;
+ ShiftVal = 4;
} else if (ST.hasPkMovB32()) {
Opcode = AMDGPU::V_PK_MOV_B32;
EltSize = 8;
+ TestMaskVal = 0x000000000000000F;
+ ShiftVal = 4;
}
}
@@ -1114,6 +1125,10 @@ void SIInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
ArrayRef<int16_t> SubIndices = RI.getRegSplitParts(RC, EltSize);
+ // The TestMaskVal will scan from right to left.
+ if (!Forward)
+ TestMaskVal = TestMaskVal << (ShiftVal * (SubIndices.size() - 1));
+
// If there is an overlap, we can't kill the super-register on the last
// instruction, since it will also kill the components made live by this def.
const bool Overlap = RI.regsOverlap(SrcReg, DestReg);
@@ -1130,7 +1145,14 @@ void SIInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
assert(DestSubReg && SrcSubReg && "Failed to find subregs!");
bool IsFirstSubreg = Idx == 0;
- bool UseKill = CanKillSuperReg && Idx == SubIndices.size() - 1;
+ bool UseKill = CanKillSuperReg;
+
+ if (!IsFirstSubreg) {
+ TestMaskVal = Forward ? TestMaskVal << ShiftVal : TestMaskVal >> ShiftVal;
+ }
+
+ if (!isSrcRegFullLive && (LiveRegUnitMaskVal & TestMaskVal) == uint64_t(0))
+ continue;
if (Opcode == AMDGPU::INSTRUCTION_LIST_END) {
Register ImpDefSuper = IsFirstSubreg ? Register(DestReg) : Register();
@@ -1141,24 +1163,18 @@ void SIInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
MachineInstrBuilder MIB =
BuildMI(MBB, MI, DL, get(AMDGPU::V_PK_MOV_B32), DestSubReg)
.addImm(SISrcMods::OP_SEL_1)
- .addReg(SrcSubReg)
+ .addReg(SrcSubReg, getKillRegState(UseKill))
.addImm(SISrcMods::OP_SEL_0 | SISrcMods::OP_SEL_1)
- .addReg(SrcSubReg)
- .addImm(0) // op_sel_lo
- .addImm(0) // op_sel_hi
- .addImm(0) // neg_lo
- .addImm(0) // neg_hi
- .addImm(0) // clamp
- .addReg(SrcReg, getKillRegState(UseKill) | RegState::Implicit);
- if (IsFirstSubreg)
- MIB.addReg(DestReg, RegState::Define | RegState::Implicit);
+ .addReg(SrcSubReg, getKillRegState(UseKill))
+ .addImm(0) // op_sel_lo
+ .addImm(0) // op_sel_hi
+ .addImm(0) // neg_lo
+ .addImm(0) // neg_hi
+ .addImm(0); // clamp
} else {
MachineInstrBuilder Builder =
- BuildMI(MBB, MI, DL, get(Opcode), DestSubReg).addReg(SrcSubReg);
- if (IsFirstSubreg)
- Builder.addReg(DestReg, RegState::Define | RegState::Implicit);
-
- Builder.addReg(SrcReg, getKillRegState(UseKill) | RegState::Implicit);
+ BuildMI(MBB, MI, DL, get(Opcode), DestSubReg)
+ .addReg(SrcSubReg, getKillRegState(UseKill));
}
}
}
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/addo.ll b/llvm/test/CodeGen/AMDGPU/GlobalISel/addo.ll
index 38374d1689366..3cab3fb763523 100644
--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/addo.ll
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/addo.ll
@@ -676,8 +676,8 @@ define amdgpu_ps i64 @s_saddo_i64(i64 inreg %a, i64 inreg %b) {
; GFX7-LABEL: s_saddo_i64:
; GFX7: ; %bb.0:
; GFX7-NEXT: s_add_u32 s4, s0, s2
-; GFX7-NEXT: v_mov_b32_e32 v0, s0
; GFX7-NEXT: s_addc_u32 s5, s1, s3
+; GFX7-NEXT: v_mov_b32_e32 v0, s0
; GFX7-NEXT: v_mov_b32_e32 v1, s1
; GFX7-NEXT: v_cmp_lt_i64_e32 vcc, s[4:5], v[0:1]
; GFX7-NEXT: v_cmp_lt_i64_e64 s[0:1], s[2:3], 0
@@ -693,8 +693,8 @@ define amdgpu_ps i64 @s_saddo_i64(i64 inreg %a, i64 inreg %b) {
; GFX8-LABEL: s_saddo_i64:
; GFX8: ; %bb.0:
; GFX8-NEXT: s_add_u32 s4, s0, s2
-; GFX8-NEXT: v_mov_b32_e32 v0, s0
; GFX8-NEXT: s_addc_u32 s5, s1, s3
+; GFX8-NEXT: v_mov_b32_e32 v0, s0
; GFX8-NEXT: v_mov_b32_e32 v1, s1
; GFX8-NEXT: v_cmp_lt_i64_e32 vcc, s[4:5], v[0:1]
; GFX8-NEXT: v_cmp_lt_i64_e64 s[0:1], s[2:3], 0
@@ -710,8 +710,8 @@ define amdgpu_ps i64 @s_saddo_i64(i64 inreg %a, i64 inreg %b) {
; GFX9-LABEL: s_saddo_i64:
; GFX9: ; %bb.0:
; GFX9-NEXT: s_add_u32 s4, s0, s2
-; GFX9-NEXT: v_mov_b32_e32 v0, s0
; GFX9-NEXT: s_addc_u32 s5, s1, s3
+; GFX9-NEXT: v_mov_b32_e32 v0, s0
; GFX9-NEXT: v_mov_b32_e32 v1, s1
; GFX9-NEXT: v_cmp_lt_i64_e32 vcc, s[4:5], v[0:1]
; GFX9-NEXT: v_cmp_lt_i64_e64 s[0:1], s[2:3], 0
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/atomic_optimizations_mul_one.ll b/llvm/test/CodeGen/AMDGPU/GlobalISel/atomic_optimizations_mul_one.ll
index 28ed88f4cf8fb..31606dea7d335 100644
--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/atomic_optimizations_mul_one.ll
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/atomic_optimizations_mul_one.ll
@@ -96,8 +96,8 @@ define amdgpu_cs void @atomic_add_and_format(<4 x i32> inreg %arg) {
; GCN-NEXT: s_waitcnt vmcnt(0)
; GCN-NEXT: v_readfirstlane_b32 s4, v1
; GCN-NEXT: v_add_i32_e32 v4, vcc, s4, v0
-; GCN-NEXT: s_waitcnt expcnt(0)
; GCN-NEXT: v_mov_b32_e32 v0, s0
+; GCN-NEXT: s_waitcnt expcnt(0)
; GCN-NEXT: v_mov_b32_e32 v1, s1
; GCN-NEXT: v_mov_b32_e32 v2, s2
; GCN-NEXT: v_mov_b32_e32 v3, s3
@@ -192,8 +192,8 @@ define amdgpu_cs void @atomic_sub_and_format(<4 x i32> inreg %arg) {
; GCN-NEXT: s_waitcnt vmcnt(0)
; GCN-NEXT: v_readfirstlane_b32 s4, v1
; GCN-NEXT: v_sub_i32_e32 v4, vcc, s4, v0
-; GCN-NEXT: s_waitcnt expcnt(0)
; GCN-NEXT: v_mov_b32_e32 v0, s0
+; GCN-NEXT: s_waitcnt expcnt(0)
; GCN-NEXT: v_mov_b32_e32 v1, s1
; GCN-NEXT: v_mov_b32_e32 v2, s2
; GCN-NEXT: v_mov_b32_e32 v3, s3
@@ -294,8 +294,8 @@ define amdgpu_cs void @atomic_xor_and_format(<4 x i32> inreg %arg) {
; GCN-NEXT: v_readfirstlane_b32 s4, v1
; GCN-NEXT: v_and_b32_e32 v0, 1, v0
; GCN-NEXT: v_xor_b32_e32 v4, s4, v0
-; GCN-NEXT: s_waitcnt expcnt(0)
; GCN-NEXT: v_mov_b32_e32 v0, s0
+; GCN-NEXT: s_waitcnt expcnt(0)
; GCN-NEXT: v_mov_b32_e32 v1, s1
; GCN-NEXT: v_mov_b32_e32 v2, s2
; GCN-NEXT: v_mov_b32_e32 v3, s3
@@ -392,8 +392,8 @@ define amdgpu_cs void @atomic_ptr_add_and_format(ptr addrspace(8) inreg %arg) {
; GCN-NEXT: s_waitcnt vmcnt(0)
; GCN-NEXT: v_readfirstlane_b32 s4, v1
; GCN-NEXT: v_add_i32_e32 v4, vcc, s4, v0
-; GCN-NEXT: s_waitcnt expcnt(0)
; GCN-NEXT: v_mov_b32_e32 v0, s0
+; GCN-NEXT: s_waitcnt expcnt(0)
; GCN-NEXT: v_mov_b32_e32 v1, s1
; GCN-NEXT: v_mov_b32_e32 v2, s2
; GCN-NEXT: v_mov_b32_e32 v3, s3
@@ -492,8 +492,8 @@ define amdgpu_cs void @atomic_ptr_sub_and_format(ptr addrspace(8) inreg %arg) {
; GCN-NEXT: s_waitcnt vmcnt(0)
; GCN-NEXT: v_readfirstlane_b32 s4, v1
;...
[truncated]
|
vg0204
force-pushed
the
vg0204/alternative-for-implicit-usage-in-RegTuplesExpansion
branch
from
cf01d31 to
cad0982
Compare
jrbyrnes
added a commit
that referenced
this pull request
Oct 27, 2025
When lowering spills / restores, we may end up partially lowering the spill via copies and the remaining portion with loads/stores. In this partial lowering case,the implicit-def operands added to the restore load clobber the preceding copies -- telling MachineCopyPropagation to delete them. By also attaching an implicit operand to the load, the COPYs have an artificial use and thus will not be deleted - this is the same strategy taken in #115285 I'm not sure that we need implicit-def operands on any load restore, but I guess it may make sense if it needs to be split into multiple loads and some have been optimized out as containing undef elements. These implicit / implicit-def operands continue to cause correctness issues. A previous / ongoing long term plan to remove them is being addressed via: https://discourse.llvm.org/t/llvm-codegen-rfc-add-mo-lanemask-type-and-a-new-copy-lanemask-instruction/88021 #151123 #151124
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Oct 27, 2025
… (#164911) When lowering spills / restores, we may end up partially lowering the spill via copies and the remaining portion with loads/stores. In this partial lowering case,the implicit-def operands added to the restore load clobber the preceding copies -- telling MachineCopyPropagation to delete them. By also attaching an implicit operand to the load, the COPYs have an artificial use and thus will not be deleted - this is the same strategy taken in llvm/llvm-project#115285 I'm not sure that we need implicit-def operands on any load restore, but I guess it may make sense if it needs to be split into multiple loads and some have been optimized out as containing undef elements. These implicit / implicit-def operands continue to cause correctness issues. A previous / ongoing long term plan to remove them is being addressed via: https://discourse.llvm.org/t/llvm-codegen-rfc-add-mo-lanemask-type-and-a-new-copy-lanemask-instruction/88021 llvm/llvm-project#151123 llvm/llvm-project#151124
dvbuka
pushed a commit
to dvbuka/llvm-project
that referenced
this pull request
Oct 27, 2025
When lowering spills / restores, we may end up partially lowering the spill via copies and the remaining portion with loads/stores. In this partial lowering case,the implicit-def operands added to the restore load clobber the preceding copies -- telling MachineCopyPropagation to delete them. By also attaching an implicit operand to the load, the COPYs have an artificial use and thus will not be deleted - this is the same strategy taken in llvm#115285 I'm not sure that we need implicit-def operands on any load restore, but I guess it may make sense if it needs to be split into multiple loads and some have been optimized out as containing undef elements. These implicit / implicit-def operands continue to cause correctness issues. A previous / ongoing long term plan to remove them is being addressed via: https://discourse.llvm.org/t/llvm-codegen-rfc-add-mo-lanemask-type-and-a-new-copy-lanemask-instruction/88021 llvm#151123 llvm#151124
Lukacma
pushed a commit
to Lukacma/llvm-project
that referenced
this pull request
Oct 29, 2025
When lowering spills / restores, we may end up partially lowering the spill via copies and the remaining portion with loads/stores. In this partial lowering case,the implicit-def operands added to the restore load clobber the preceding copies -- telling MachineCopyPropagation to delete them. By also attaching an implicit operand to the load, the COPYs have an artificial use and thus will not be deleted - this is the same strategy taken in llvm#115285 I'm not sure that we need implicit-def operands on any load restore, but I guess it may make sense if it needs to be split into multiple loads and some have been optimized out as containing undef elements. These implicit / implicit-def operands continue to cause correctness issues. A previous / ongoing long term plan to remove them is being addressed via: https://discourse.llvm.org/t/llvm-codegen-rfc-add-mo-lanemask-type-and-a-new-copy-lanemask-instruction/88021 llvm#151123 llvm#151124
aokblast
pushed a commit
to aokblast/llvm-project
that referenced
this pull request
Oct 30, 2025
When lowering spills / restores, we may end up partially lowering the spill via copies and the remaining portion with loads/stores. In this partial lowering case,the implicit-def operands added to the restore load clobber the preceding copies -- telling MachineCopyPropagation to delete them. By also attaching an implicit operand to the load, the COPYs have an artificial use and thus will not be deleted - this is the same strategy taken in llvm#115285 I'm not sure that we need implicit-def operands on any load restore, but I guess it may make sense if it needs to be split into multiple loads and some have been optimized out as containing undef elements. These implicit / implicit-def operands continue to cause correctness issues. A previous / ongoing long term plan to remove them is being addressed via: https://discourse.llvm.org/t/llvm-codegen-rfc-add-mo-lanemask-type-and-a-new-copy-lanemask-instruction/88021 llvm#151123 llvm#151124
As for the Greedy RA, the virtRegRewriter pass is the last place that holds livenes info, even at subregister level. So, now that information can be extracted and encoded on COPY instruction. This information for COPY can later be used to identify partially live regsiters precisely, assuming the liveness information used is not invalidated by any kind if IR muatation later.
We will now use the liveness encoded during VirtRegRewriter for COPY instruction to expand only defined registers, thus avoiding the undefined registers. It enables us to stop using implicit and implicit-def avoiding unnecessary false dependency among the registers.
vg0204
force-pushed
the
vg0204/alternative-for-implicit-usage-in-RegTuplesExpansion
branch
from
cad0982 to
7bdbd3a
Compare
Contributor
Author
Contributor
I don't understand the overall plan here. It seems like this would only work correctly if liveness does not change between VirtRegRewriter and ExpandPostRAPseudos. But lots of passes run in that window, including ones like MachineLICM that can make major code changes. |
Contributor
Author
That's the idea that if we ensure that each pass in-between can ensure the correct liveness update, this should hold true to be correct. Also, its one of my initial ideas to avoid using implicit-opernad (via using the pre-calculated liveness at sub-register granularity), since VirtRegRewriter is the last pass which holds mapping between virtual & allocated physical registers to be used, avoiding the recalculation. I am open for suggestions/feedback to take care of it alternatively! Refer : https://discourse.llvm.org/t/physical-subregister-liveness/59657 |
Contributor
I don't see how this can work. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch utilizes the liveness encoded during VirtRegRewriter within the COPY instruction[#151123 ] to expand only defined parts of the use register. It enables us to stop using implicit and implicit-def tom satisfy the MachineVerifier, thus avoiding unnecessary false dependency among the registers. This is in reference to SWDEV-498533