[llvm] bbb9589 - [TII] NFCI: Simplify the interface for isTriviallyReMaterializable
Sander de Smalen via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 7 06:01:53 PDT 2023
Author: Sander de Smalen
Date: 2023-08-07T13:01:06Z
New Revision: bbb95893ded91fc2a639ea304684b22fc6f2d5a3
URL: https://github.com/llvm/llvm-project/commit/bbb95893ded91fc2a639ea304684b22fc6f2d5a3
DIFF: https://github.com/llvm/llvm-project/commit/bbb95893ded91fc2a639ea304684b22fc6f2d5a3.diff
LOG: [TII] NFCI: Simplify the interface for isTriviallyReMaterializable
Currently `isTriviallyReMaterializable` calls
`isReallyTriviallyReMaterializable` and
`isReallyTriviallyReMaterializableGeneric`. The two interfaces
are confusing, but there are also some real issues with this.
The documentation of this function (see below) suggests that
`isReallyTriviallyRematerializable` allows the target to override the
default behaviour.
/// For instructions with opcodes for which the M_REMATERIALIZABLE flag is
/// set, this hook lets the target specify whether the instruction is actually
/// trivially rematerializable, taking into consideration its operands.
It however implements something different. The default behaviour
is the analysis done in `isReallyTriviallyReMaterializableGeneric`,
which is testing if it is safe to rematerialize the MachineInstr.
The result of `isReallyTriviallyReMaterializable` is only considered if
`isReallyTriviallyReMaterializableGeneric` returns `false`. That means
there is no way to override the default behaviour if
`isReallyTriviallyReMaterializableGeneric` returns true (i.e. it is safe to
rematerialize, but we'd rather not).
By making this a single interface, we can override the interface to do either.
Reviewed By: craig.topper, nemanjai
Differential Revision: https://reviews.llvm.org/D156520
Added:
Modified:
llvm/include/llvm/CodeGen/MachineInstr.h
llvm/include/llvm/CodeGen/TargetInstrInfo.h
llvm/include/llvm/MC/MCInstrDesc.h
llvm/lib/CodeGen/TargetInstrInfo.cpp
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.cpp
llvm/lib/Target/X86/X86InstrInfo.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/MachineInstr.h b/llvm/include/llvm/CodeGen/MachineInstr.h
index 7d1e39a47b9637..2e1f7feb6e9c0f 100644
--- a/llvm/include/llvm/CodeGen/MachineInstr.h
+++ b/llvm/include/llvm/CodeGen/MachineInstr.h
@@ -1202,7 +1202,7 @@ class MachineInstr
/// Returns true if this instruction is a candidate for remat.
/// This flag is deprecated, please don't use it anymore. If this
/// flag is set, the isReallyTriviallyReMaterializable() method is called to
- /// verify the instruction is really rematable.
+ /// verify the instruction is really rematerializable.
bool isRematerializable(QueryType Type = AllInBundle) const {
// It's only possible to re-mat a bundle if all bundled instructions are
// re-materializable.
diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
index c2cc9f862ed5d0..5e9e72c531bb21 100644
--- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
@@ -131,8 +131,7 @@ class TargetInstrInfo : public MCInstrInfo {
bool isTriviallyReMaterializable(const MachineInstr &MI) const {
return MI.getOpcode() == TargetOpcode::IMPLICIT_DEF ||
(MI.getDesc().isRematerializable() &&
- (isReallyTriviallyReMaterializable(MI) ||
- isReallyTriviallyReMaterializableGeneric(MI)));
+ isReallyTriviallyReMaterializable(MI));
}
/// Given \p MO is a PhysReg use return if it can be ignored for the purpose
@@ -148,10 +147,7 @@ class TargetInstrInfo : public MCInstrInfo {
/// predicate must return false if the instruction has any side effects other
/// than producing a value, or if it requres any address registers that are
/// not always available.
- /// Requirements must be check as stated in isTriviallyReMaterializable() .
- virtual bool isReallyTriviallyReMaterializable(const MachineInstr &MI) const {
- return false;
- }
+ virtual bool isReallyTriviallyReMaterializable(const MachineInstr &MI) const;
/// This method commutes the operands of the given machine instruction MI.
/// The operands to be commuted are specified by their indices OpIdx1 and
@@ -186,13 +182,6 @@ class TargetInstrInfo : public MCInstrInfo {
unsigned CommutableOpIdx1,
unsigned CommutableOpIdx2);
-private:
- /// For instructions with opcodes for which the M_REMATERIALIZABLE flag is
- /// set and the target hook isReallyTriviallyReMaterializable returns false,
- /// this function does target-independent tests to determine if the
- /// instruction is really trivially rematerializable.
- bool isReallyTriviallyReMaterializableGeneric(const MachineInstr &MI) const;
-
public:
/// These methods return the opcode of the frame setup/destroy instructions
/// if they exist (-1 otherwise). Some targets use pseudo instructions in
diff --git a/llvm/include/llvm/MC/MCInstrDesc.h b/llvm/include/llvm/MC/MCInstrDesc.h
index 0f406cb719502c..7b8fb033c6f70c 100644
--- a/llvm/include/llvm/MC/MCInstrDesc.h
+++ b/llvm/include/llvm/MC/MCInstrDesc.h
@@ -519,9 +519,8 @@ class MCInstrDesc {
/// Returns true if this instruction is a candidate for remat. This
/// flag is only used in TargetInstrInfo method isTriviallyRematerializable.
///
- /// If this flag is set, the isReallyTriviallyReMaterializable()
- /// or isReallyTriviallyReMaterializableGeneric methods are called to verify
- /// the instruction is really rematable.
+ /// If this flag is set, the isReallyTriviallyReMaterializable() method is
+ /// called to verify the instruction is really rematerializable.
bool isRematerializable() const {
return Flags & (1ULL << MCID::Rematerializable);
}
diff --git a/llvm/lib/CodeGen/TargetInstrInfo.cpp b/llvm/lib/CodeGen/TargetInstrInfo.cpp
index a72018b49baf76..f060b7d45e820a 100644
--- a/llvm/lib/CodeGen/TargetInstrInfo.cpp
+++ b/llvm/lib/CodeGen/TargetInstrInfo.cpp
@@ -1119,7 +1119,7 @@ MachineTraceStrategy TargetInstrInfo::getMachineCombinerTraceStrategy() const {
return MachineTraceStrategy::TS_MinInstrCount;
}
-bool TargetInstrInfo::isReallyTriviallyReMaterializableGeneric(
+bool TargetInstrInfo::isReallyTriviallyReMaterializable(
const MachineInstr &MI) const {
const MachineFunction &MF = *MI.getMF();
const MachineRegisterInfo &MRI = MF.getRegInfo();
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index 393d54747cb1d0..281836c009330e 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -120,12 +120,13 @@ bool SIInstrInfo::isReallyTriviallyReMaterializable(
// There is
diff erence to generic method which does not allow
// rematerialization if there are virtual register uses. We allow this,
// therefore this method includes SOP instructions as well.
- return !MI.hasImplicitDef() &&
- MI.getNumImplicitOperands() == MI.getDesc().implicit_uses().size() &&
- !MI.mayRaiseFPException();
+ if (!MI.hasImplicitDef() &&
+ MI.getNumImplicitOperands() == MI.getDesc().implicit_uses().size() &&
+ !MI.mayRaiseFPException())
+ return true;
}
- return false;
+ return TargetInstrInfo::isReallyTriviallyReMaterializable(MI);
}
// Returns true if the scalar result of a VALU instruction depends on exec.
diff --git a/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp b/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
index f903d583d7c640..e07687a88a222b 100644
--- a/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
+++ b/llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
@@ -6731,7 +6731,8 @@ bool ARMBaseInstrInfo::isReallyTriviallyReMaterializable(
// the tail predication conversion. This means that the element count
// register has to be live for longer, but that has to be better than
// spill/restore and VPT predication.
- return isVCTP(&MI) && !isPredicated(MI);
+ return (isVCTP(&MI) && !isPredicated(MI)) ||
+ TargetInstrInfo::isReallyTriviallyReMaterializable(MI);
}
unsigned llvm::getBLXOpcode(const MachineFunction &MF) {
diff --git a/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp b/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
index 784953dbc8477d..ac34ea55e6f75e 100644
--- a/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
+++ b/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
@@ -1121,7 +1121,7 @@ bool PPCInstrInfo::isReallyTriviallyReMaterializable(
case PPC::XXSETACCZW:
return true;
}
- return false;
+ return TargetInstrInfo::isReallyTriviallyReMaterializable(MI);
}
unsigned PPCInstrInfo::isStoreToStackSlot(const MachineInstr &MI,
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.cpp
index b2dd656ccdda32..f4f193bd3d8ce6 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.cpp
@@ -46,11 +46,11 @@ bool WebAssemblyInstrInfo::isReallyTriviallyReMaterializable(
case WebAssembly::CONST_I64:
case WebAssembly::CONST_F32:
case WebAssembly::CONST_F64:
- // isReallyTriviallyReMaterializableGeneric misses these because of the
- // ARGUMENTS implicit def, so we manualy override it here.
+ // TargetInstrInfo::isReallyTriviallyReMaterializable misses these
+ // because of the ARGUMENTS implicit def, so we manualy override it here.
return true;
default:
- return false;
+ return TargetInstrInfo::isReallyTriviallyReMaterializable(MI);
}
}
diff --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp
index fd219fa5943b90..96d3407d6c0713 100644
--- a/llvm/lib/Target/X86/X86InstrInfo.cpp
+++ b/llvm/lib/Target/X86/X86InstrInfo.cpp
@@ -871,13 +871,14 @@ bool X86InstrInfo::isReallyTriviallyReMaterializable(
if (BaseReg == 0 || BaseReg == X86::RIP)
return true;
// Allow re-materialization of PIC load.
- if (!ReMatPICStubLoad && MI.getOperand(1 + X86::AddrDisp).isGlobal())
- return false;
- const MachineFunction &MF = *MI.getParent()->getParent();
- const MachineRegisterInfo &MRI = MF.getRegInfo();
- return regIsPICBase(BaseReg, MRI);
+ if (!(!ReMatPICStubLoad && MI.getOperand(1 + X86::AddrDisp).isGlobal())) {
+ const MachineFunction &MF = *MI.getParent()->getParent();
+ const MachineRegisterInfo &MRI = MF.getRegInfo();
+ if (regIsPICBase(BaseReg, MRI))
+ return true;
+ }
}
- return false;
+ break;
}
case X86::LEA32r:
@@ -895,11 +896,13 @@ bool X86InstrInfo::isReallyTriviallyReMaterializable(
// Allow re-materialization of lea PICBase + x.
const MachineFunction &MF = *MI.getParent()->getParent();
const MachineRegisterInfo &MRI = MF.getRegInfo();
- return regIsPICBase(BaseReg, MRI);
+ if (regIsPICBase(BaseReg, MRI))
+ return true;
}
- return false;
+ break;
}
}
+ return TargetInstrInfo::isReallyTriviallyReMaterializable(MI);
}
void X86InstrInfo::reMaterialize(MachineBasicBlock &MBB,
More information about the llvm-commits
mailing list