Skip to content

[NFC][MachineScheduler] Rename NumLoads parameter of shouldClusterMemOps to ClusterSize #73757

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

Merged
merged 1 commit into from
Nov 29, 2023
Merged
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
7 changes: 4 additions & 3 deletions llvm/include/llvm/CodeGen/TargetInstrInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -1496,13 +1496,14 @@ class TargetInstrInfo : public MCInstrInfo {
/// to TargetPassConfig::createMachineScheduler() to have an effect.
///
/// \p BaseOps1 and \p BaseOps2 are memory operands of two memory operations.
/// \p NumLoads is the number of loads that will be in the cluster if this
/// hook returns true.
/// \p ClusterSize is the number of operations in the resulting load/store
/// cluster if this hook returns true.
/// \p NumBytes is the number of bytes that will be loaded from all the
/// clustered loads if this hook returns true.
virtual bool shouldClusterMemOps(ArrayRef<const MachineOperand *> BaseOps1,
ArrayRef<const MachineOperand *> BaseOps2,
unsigned NumLoads, unsigned NumBytes) const {
unsigned ClusterSize,
unsigned NumBytes) const {
llvm_unreachable("target did not implement shouldClusterMemOps()");
}

Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4231,7 +4231,7 @@ static bool shouldClusterFI(const MachineFrameInfo &MFI, int FI1,
/// Only called for LdSt for which getMemOperandWithOffset returns true.
bool AArch64InstrInfo::shouldClusterMemOps(
ArrayRef<const MachineOperand *> BaseOps1,
ArrayRef<const MachineOperand *> BaseOps2, unsigned NumLoads,
ArrayRef<const MachineOperand *> BaseOps2, unsigned ClusterSize,
unsigned NumBytes) const {
assert(BaseOps1.size() == 1 && BaseOps2.size() == 1);
const MachineOperand &BaseOp1 = *BaseOps1.front();
Expand All @@ -4249,7 +4249,7 @@ bool AArch64InstrInfo::shouldClusterMemOps(
return false;

// Only cluster up to a single pair.
if (NumLoads > 2)
if (ClusterSize > 2)
return false;

if (!isPairableLdStInst(FirstLdSt) || !isPairableLdStInst(SecondLdSt))
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/AArch64/AArch64InstrInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ class AArch64InstrInfo final : public AArch64GenInstrInfo {

bool shouldClusterMemOps(ArrayRef<const MachineOperand *> BaseOps1,
ArrayRef<const MachineOperand *> BaseOps2,
unsigned NumLoads, unsigned NumBytes) const override;
unsigned ClusterSize,
unsigned NumBytes) const override;

void copyPhysRegTuple(MachineBasicBlock &MBB, MachineBasicBlock::iterator I,
const DebugLoc &DL, MCRegister DestReg,
Expand Down
6 changes: 3 additions & 3 deletions llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ static bool memOpsHaveSameBasePtr(const MachineInstr &MI1,

bool SIInstrInfo::shouldClusterMemOps(ArrayRef<const MachineOperand *> BaseOps1,
ArrayRef<const MachineOperand *> BaseOps2,
unsigned NumLoads,
unsigned ClusterSize,
unsigned NumBytes) const {
// If the mem ops (to be clustered) do not have the same base ptr, then they
// should not be clustered
Expand All @@ -568,8 +568,8 @@ bool SIInstrInfo::shouldClusterMemOps(ArrayRef<const MachineOperand *> BaseOps1,
// (3) 9 <= LoadSize <= 12: cluster at max 2 mem ops
// (4) 13 <= LoadSize <= 16: cluster at max 2 mem ops
// (5) LoadSize >= 17: do not cluster
const unsigned LoadSize = NumBytes / NumLoads;
const unsigned NumDWORDs = ((LoadSize + 3) / 4) * NumLoads;
const unsigned LoadSize = NumBytes / ClusterSize;
const unsigned NumDWORDs = ((LoadSize + 3) / 4) * ClusterSize;
return NumDWORDs <= 8;
}

Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/AMDGPU/SIInstrInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,8 @@ class SIInstrInfo final : public AMDGPUGenInstrInfo {

bool shouldClusterMemOps(ArrayRef<const MachineOperand *> BaseOps1,
ArrayRef<const MachineOperand *> BaseOps2,
unsigned NumLoads, unsigned NumBytes) const override;
unsigned ClusterSize,
unsigned NumBytes) const override;

bool shouldScheduleLoadsNear(SDNode *Load0, SDNode *Load1, int64_t Offset0,
int64_t Offset1, unsigned NumLoads) const override;
Expand Down
7 changes: 4 additions & 3 deletions llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2879,7 +2879,7 @@ static bool isClusterableLdStOpcPair(unsigned FirstOpc, unsigned SecondOpc,

bool PPCInstrInfo::shouldClusterMemOps(
ArrayRef<const MachineOperand *> BaseOps1,
ArrayRef<const MachineOperand *> BaseOps2, unsigned NumLoads,
ArrayRef<const MachineOperand *> BaseOps2, unsigned ClusterSize,
unsigned NumBytes) const {

assert(BaseOps1.size() == 1 && BaseOps2.size() == 1);
Expand All @@ -2888,9 +2888,10 @@ bool PPCInstrInfo::shouldClusterMemOps(
assert((BaseOp1.isReg() || BaseOp1.isFI()) &&
"Only base registers and frame indices are supported.");

// The NumLoads means the number of loads that has been clustered.
// ClusterSize means the number of memory operations that will have been
// clustered if this hook returns true.
// Don't cluster memory op if there are already two ops clustered at least.
if (NumLoads > 2)
if (ClusterSize > 2)
return false;

// Cluster the load/store only when they have the same base
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/PowerPC/PPCInstrInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,8 @@ class PPCInstrInfo : public PPCGenInstrInfo {
/// adjacent.
bool shouldClusterMemOps(ArrayRef<const MachineOperand *> BaseOps1,
ArrayRef<const MachineOperand *> BaseOps2,
unsigned NumLoads, unsigned NumBytes) const override;
unsigned ClusterSize,
unsigned NumBytes) const override;

/// Return true if two MIs access different memory addresses and false
/// otherwise
Expand Down