Skip to content

Swedev 414443 #65947

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions llvm/include/llvm/CodeGen/TargetInstrInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,15 @@ class TargetInstrInfo : public MCInstrInfo {
return false;
}

/// Attempting to move \p MoveCandidate after \p ModifierInstr .
/// \p MoveCandidate uses \p Reg but \p ModifierInstr redefines \p Reg.
/// Let target check it redefines it using same value.
virtual bool
modifiesRegisterImplicitly(Register Reg, const MachineInstr *MoveCandidate,
const MachineInstr *ModifierInstr) const {
return true;
}

protected:
/// For instructions with opcodes for which the M_REMATERIALIZABLE flag is
/// set, this hook lets the target specify whether the instruction is actually
Expand Down
4 changes: 3 additions & 1 deletion llvm/lib/CodeGen/MachineSink.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,10 @@ static bool blockPrologueInterferes(const MachineBasicBlock *BB,
if (MO.isUse()) {
if (Reg.isPhysical() && MRI && MRI->isConstantPhysReg(Reg))
continue;
if (PI->modifiesRegister(Reg, TRI))
if (PI->modifiesRegister(Reg, TRI) &&
TII->modifiesRegisterImplicitly(Reg, &MI, &*PI)) {
return true;
}
} else {
if (PI->readsRegister(Reg, TRI))
return true;
Expand Down
33 changes: 33 additions & 0 deletions llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,39 @@ bool SIInstrInfo::isIgnorableUse(const MachineOperand &MO) const {
isVALU(*MO.getParent()) && !resultDependsOnExec(*MO.getParent());
}

bool SIInstrInfo::modifiesRegisterImplicitly(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have an ever growing set of convoluted callbacks to deal with this problem. I think we need to look into moving some of the modeling of this into the CFG itself. I think we need to mark block edges as convergent/divergent/something

Register Reg, const MachineInstr *MoveCandidate,
const MachineInstr *ModifierInstr) const {

if (ModifierInstr->getOpcode() == AMDGPU::SI_END_CF && Reg == AMDGPU::EXEC) {
const MachineRegisterInfo &MRI = MoveCandidate->getMF()->getRegInfo();

// Looking if this is a simple case of:
//
// %0 = MoveCandidate %1, %2, implicit $exec
// %EndCF:sreg_64 = SI_IF %cond, %bb.B
// S_BRANCH %bb.A
//
// bb.A
// ...
//
// bb.B
// SI_END_CF %EndCF, implicit-def dead $exec
// ... MoveCandidate should be moved here

// MoveCandidate is from block that started divergent control flow via SI_IF
// it is a simple SI_IF (no loops) - only user of SI_IF is SI_END_CF
// SI_END_CF restores exec mask as it was before SI_IF (unchanged)
Register EndCF = ModifierInstr->getOperand(0).getReg();
MachineInstr *SIIF = MRI.getVRegDef(EndCF);
if (SIIF->getOpcode() == AMDGPU::SI_IF && MRI.hasOneUse(EndCF) &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't even work after lowering

SIIF->getParent() == MoveCandidate->getParent())
return false;
}

return true;
}

bool SIInstrInfo::areLoadsFromSameBasePtr(SDNode *Load0, SDNode *Load1,
int64_t &Offset0,
int64_t &Offset1) const {
Expand Down
4 changes: 4 additions & 0 deletions llvm/lib/Target/AMDGPU/SIInstrInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,10 @@ class SIInstrInfo final : public AMDGPUGenInstrInfo {

bool isIgnorableUse(const MachineOperand &MO) const override;

bool
modifiesRegisterImplicitly(Register Reg, const MachineInstr *MoveCandidate,
const MachineInstr *ModifierInstr) const override;

bool areLoadsFromSameBasePtr(SDNode *Load0, SDNode *Load1, int64_t &Offset0,
int64_t &Offset1) const override;

Expand Down
Loading