Skip to content

Commit

Permalink
[RISCV] Use TypeSize in places where needed for RegBankSelection
Browse files Browse the repository at this point in the history
This is a precommit for #71514 to use TypeSize instead of unsigned to
avoid crashes when scalable vectors are used.
  • Loading branch information
michaelmaitland committed Nov 15, 2023
1 parent 170810f commit f219e03
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 19 deletions.
8 changes: 4 additions & 4 deletions llvm/include/llvm/CodeGen/RegisterBankInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ class RegisterBankInfo {
/// \note This method does not check anything when assertions are disabled.
///
/// \return True is the check was successful.
bool verify(const RegisterBankInfo &RBI, unsigned MeaningfulBitWidth) const;
bool verify(const RegisterBankInfo &RBI, TypeSize MeaningfulBitWidth) const;

/// Print this on dbgs() stream.
void dump() const;
Expand Down Expand Up @@ -631,7 +631,7 @@ class RegisterBankInfo {
///
/// \note Since this is a copy, both registers have the same size.
virtual unsigned copyCost(const RegisterBank &A, const RegisterBank &B,
unsigned Size) const {
TypeSize Size) const {
// Optimistically assume that copies are coalesced. I.e., when
// they are on the same bank, they are free.
// Otherwise assume a non-zero cost of 1. The targets are supposed
Expand All @@ -641,7 +641,7 @@ class RegisterBankInfo {

/// \returns true if emitting a copy from \p Src to \p Dst is impossible.
bool cannotCopy(const RegisterBank &Dst, const RegisterBank &Src,
unsigned Size) const {
TypeSize Size) const {
return copyCost(Dst, Src, Size) == std::numeric_limits<unsigned>::max();
}

Expand Down Expand Up @@ -749,7 +749,7 @@ class RegisterBankInfo {
/// virtual register.
///
/// \pre \p Reg != 0 (NoRegister).
unsigned getSizeInBits(Register Reg, const MachineRegisterInfo &MRI,
TypeSize getSizeInBits(Register Reg, const MachineRegisterInfo &MRI,
const TargetRegisterInfo &TRI) const;

/// Check that information hold by this instance make sense for the
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/CodeGen/MachineVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2265,7 +2265,7 @@ MachineVerifier::visitMachineOperand(const MachineOperand *MO, unsigned MONum) {
}

// Make sure the register fits into its register bank if any.
if (RegBank && Ty.isValid() &&
if (RegBank && Ty.isValid() && !Ty.isScalableVector() &&
RBI->getMaximumSize(RegBank->getID()) < Ty.getSizeInBits()) {
report("Register bank is too small for virtual register", MO,
MONum);
Expand Down
9 changes: 5 additions & 4 deletions llvm/lib/CodeGen/RegisterBankInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ void RegisterBankInfo::applyDefaultMapping(const OperandsMapper &OpdMapper) {
}
}

unsigned RegisterBankInfo::getSizeInBits(Register Reg,
TypeSize RegisterBankInfo::getSizeInBits(Register Reg,
const MachineRegisterInfo &MRI,
const TargetRegisterInfo &TRI) const {
if (Reg.isPhysical()) {
Expand Down Expand Up @@ -553,7 +553,7 @@ bool RegisterBankInfo::ValueMapping::partsAllUniform() const {
}

bool RegisterBankInfo::ValueMapping::verify(const RegisterBankInfo &RBI,
unsigned MeaningfulBitWidth) const {
TypeSize MeaningfulBitWidth) const {
assert(NumBreakDowns && "Value mapped nowhere?!");
unsigned OrigValueBitWidth = 0;
for (const RegisterBankInfo::PartialMapping &PartMap : *this) {
Expand All @@ -565,8 +565,9 @@ bool RegisterBankInfo::ValueMapping::verify(const RegisterBankInfo &RBI,
OrigValueBitWidth =
std::max(OrigValueBitWidth, PartMap.getHighBitIdx() + 1);
}
assert(OrigValueBitWidth >= MeaningfulBitWidth &&
"Meaningful bits not covered by the mapping");
assert(MeaningfulBitWidth.isScalable() ||
OrigValueBitWidth >= MeaningfulBitWidth &&
"Meaningful bits not covered by the mapping");
APInt ValueMask(OrigValueBitWidth, 0);
for (const RegisterBankInfo::PartialMapping &PartMap : *this) {
// Check that the union of the partial mappings covers the whole value,
Expand Down
16 changes: 10 additions & 6 deletions llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ AArch64RegisterBankInfo::AArch64RegisterBankInfo(

unsigned AArch64RegisterBankInfo::copyCost(const RegisterBank &A,
const RegisterBank &B,
unsigned Size) const {
TypeSize Size) const {
// What do we do with different size?
// copy are same size.
// Will introduce other hooks for different size:
Expand Down Expand Up @@ -340,12 +340,16 @@ AArch64RegisterBankInfo::getInstrAlternativeMappings(
/*NumOperands*/ 2);
const InstructionMapping &GPRToFPRMapping = getInstructionMapping(
/*ID*/ 3,
/*Cost*/ copyCost(AArch64::GPRRegBank, AArch64::FPRRegBank, Size),
/*Cost*/
copyCost(AArch64::GPRRegBank, AArch64::FPRRegBank,
TypeSize::Fixed(Size)),
getCopyMapping(AArch64::FPRRegBankID, AArch64::GPRRegBankID, Size),
/*NumOperands*/ 2);
const InstructionMapping &FPRToGPRMapping = getInstructionMapping(
/*ID*/ 3,
/*Cost*/ copyCost(AArch64::GPRRegBank, AArch64::FPRRegBank, Size),
/*Cost*/
copyCost(AArch64::GPRRegBank, AArch64::FPRRegBank,
TypeSize::Fixed(Size)),
getCopyMapping(AArch64::GPRRegBankID, AArch64::FPRRegBankID, Size),
/*NumOperands*/ 2);

Expand Down Expand Up @@ -709,7 +713,7 @@ AArch64RegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
assert(DstRB && SrcRB && "Both RegBank were nullptr");
unsigned Size = getSizeInBits(DstReg, MRI, TRI);
return getInstructionMapping(
DefaultMappingID, copyCost(*DstRB, *SrcRB, Size),
DefaultMappingID, copyCost(*DstRB, *SrcRB, TypeSize::Fixed(Size)),
getCopyMapping(DstRB->getID(), SrcRB->getID(), Size),
// We only care about the mapping of the destination.
/*NumOperands*/ 1);
Expand All @@ -728,7 +732,7 @@ AArch64RegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
const RegisterBank &SrcRB =
SrcIsGPR ? AArch64::GPRRegBank : AArch64::FPRRegBank;
return getInstructionMapping(
DefaultMappingID, copyCost(DstRB, SrcRB, Size),
DefaultMappingID, copyCost(DstRB, SrcRB, TypeSize::Fixed(Size)),
getCopyMapping(DstRB.getID(), SrcRB.getID(), Size),
// We only care about the mapping of the destination for COPY.
/*NumOperands*/ Opc == TargetOpcode::G_BITCAST ? 2 : 1);
Expand Down Expand Up @@ -821,7 +825,7 @@ AArch64RegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
Cost = copyCost(
*AArch64GenRegisterBankInfo::PartMappings[OpRegBankIdx[0]].RegBank,
*AArch64GenRegisterBankInfo::PartMappings[OpRegBankIdx[1]].RegBank,
OpSize[0]);
TypeSize::Fixed(OpSize[0]));
break;
case TargetOpcode::G_LOAD: {
// Loading in vector unit is slightly more expensive.
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ class AArch64RegisterBankInfo final : public AArch64GenRegisterBankInfo {
AArch64RegisterBankInfo(const TargetRegisterInfo &TRI);

unsigned copyCost(const RegisterBank &A, const RegisterBank &B,
unsigned Size) const override;
TypeSize Size) const override;

const RegisterBank &getRegBankFromRegClass(const TargetRegisterClass &RC,
LLT) const override;
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ bool AMDGPURegisterBankInfo::isDivergentRegBank(const RegisterBank *RB) const {

unsigned AMDGPURegisterBankInfo::copyCost(const RegisterBank &Dst,
const RegisterBank &Src,
unsigned Size) const {
TypeSize Size) const {
// TODO: Should there be a UniformVGPRRegBank which can use readfirstlane?
if (Dst.getID() == AMDGPU::SGPRRegBankID &&
(isVectorRegisterBank(Src) || Src.getID() == AMDGPU::VCCRegBankID)) {
Expand Down Expand Up @@ -3542,7 +3542,7 @@ AMDGPURegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {

unsigned Size = getSizeInBits(MI.getOperand(0).getReg(), MRI, *TRI);
if (MI.getOpcode() != AMDGPU::G_FREEZE &&
cannotCopy(*DstBank, *SrcBank, Size))
cannotCopy(*DstBank, *SrcBank, TypeSize::Fixed(Size)))
return getInvalidInstructionMapping();

const ValueMapping &ValMap = getValueMapping(0, Size, *DstBank);
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ class AMDGPURegisterBankInfo final : public AMDGPUGenRegisterBankInfo {
bool isDivergentRegBank(const RegisterBank *RB) const override;

unsigned copyCost(const RegisterBank &A, const RegisterBank &B,
unsigned Size) const override;
TypeSize Size) const override;

unsigned getBreakDownCost(const ValueMapping &ValMapping,
const RegisterBank *CurBank = nullptr) const override;
Expand Down

0 comments on commit f219e03

Please sign in to comment.