Skip to content

Commit a1a77bb

Browse files
michaelmaitlandmichaelmaitland
committed
[RISCV] Use TypeSize in places where needed for RegBankSelection
This is a precommit for llvm#71514 to use TypeSize instead of unsigned to avoid crashes when scalable vectors are used.
1 parent d19616f commit a1a77bb

File tree

7 files changed

+24
-19
lines changed

7 files changed

+24
-19
lines changed

‎llvm/include/llvm/CodeGen/RegisterBankInfo.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ class RegisterBankInfo {
177177
/// \note This method does not check anything when assertions are disabled.
178178
///
179179
/// \return True is the check was successful.
180-
bool verify(const RegisterBankInfo &RBI, unsigned MeaningfulBitWidth) const;
180+
bool verify(const RegisterBankInfo &RBI, TypeSize MeaningfulBitWidth) const;
181181

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

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

@@ -749,7 +749,7 @@ class RegisterBankInfo {
749749
/// virtual register.
750750
///
751751
/// \pre \p Reg != 0 (NoRegister).
752-
unsigned getSizeInBits(Register Reg, const MachineRegisterInfo &MRI,
752+
TypeSize getSizeInBits(Register Reg, const MachineRegisterInfo &MRI,
753753
const TargetRegisterInfo &TRI) const;
754754

755755
/// Check that information hold by this instance make sense for the

‎llvm/lib/CodeGen/MachineVerifier.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2262,7 +2262,7 @@ MachineVerifier::visitMachineOperand(const MachineOperand *MO, unsigned MONum) {
22622262
}
22632263

22642264
// Make sure the register fits into its register bank if any.
2265-
if (RegBank && Ty.isValid() &&
2265+
if (RegBank && Ty.isValid() && !Ty.isScalableVector() &&
22662266
RBI->getMaximumSize(RegBank->getID()) < Ty.getSizeInBits()) {
22672267
report("Register bank is too small for virtual register", MO,
22682268
MONum);

‎llvm/lib/CodeGen/RegisterBankInfo.cpp‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@ void RegisterBankInfo::applyDefaultMapping(const OperandsMapper &OpdMapper) {
495495
}
496496
}
497497

498-
unsigned RegisterBankInfo::getSizeInBits(Register Reg,
498+
TypeSize RegisterBankInfo::getSizeInBits(Register Reg,
499499
const MachineRegisterInfo &MRI,
500500
const TargetRegisterInfo &TRI) const {
501501
if (Reg.isPhysical()) {
@@ -553,7 +553,7 @@ bool RegisterBankInfo::ValueMapping::partsAllUniform() const {
553553
}
554554

555555
bool RegisterBankInfo::ValueMapping::verify(const RegisterBankInfo &RBI,
556-
unsigned MeaningfulBitWidth) const {
556+
TypeSize MeaningfulBitWidth) const {
557557
assert(NumBreakDowns && "Value mapped nowhere?!");
558558
unsigned OrigValueBitWidth = 0;
559559
for (const RegisterBankInfo::PartialMapping &PartMap : *this) {
@@ -565,8 +565,9 @@ bool RegisterBankInfo::ValueMapping::verify(const RegisterBankInfo &RBI,
565565
OrigValueBitWidth =
566566
std::max(OrigValueBitWidth, PartMap.getHighBitIdx() + 1);
567567
}
568-
assert(OrigValueBitWidth >= MeaningfulBitWidth &&
569-
"Meaningful bits not covered by the mapping");
568+
assert(MeaningfulBitWidth.isScalable() ||
569+
OrigValueBitWidth >= MeaningfulBitWidth &&
570+
"Meaningful bits not covered by the mapping");
570571
APInt ValueMask(OrigValueBitWidth, 0);
571572
for (const RegisterBankInfo::PartialMapping &PartMap : *this) {
572573
// Check that the union of the partial mappings covers the whole value,

‎llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ AArch64RegisterBankInfo::AArch64RegisterBankInfo(
216216

217217
unsigned AArch64RegisterBankInfo::copyCost(const RegisterBank &A,
218218
const RegisterBank &B,
219-
unsigned Size) const {
219+
TypeSize Size) const {
220220
// What do we do with different size?
221221
// copy are same size.
222222
// Will introduce other hooks for different size:
@@ -340,12 +340,16 @@ AArch64RegisterBankInfo::getInstrAlternativeMappings(
340340
/*NumOperands*/ 2);
341341
const InstructionMapping &GPRToFPRMapping = getInstructionMapping(
342342
/*ID*/ 3,
343-
/*Cost*/ copyCost(AArch64::GPRRegBank, AArch64::FPRRegBank, Size),
343+
/*Cost*/
344+
copyCost(AArch64::GPRRegBank, AArch64::FPRRegBank,
345+
TypeSize::Fixed(Size)),
344346
getCopyMapping(AArch64::FPRRegBankID, AArch64::GPRRegBankID, Size),
345347
/*NumOperands*/ 2);
346348
const InstructionMapping &FPRToGPRMapping = getInstructionMapping(
347349
/*ID*/ 3,
348-
/*Cost*/ copyCost(AArch64::GPRRegBank, AArch64::FPRRegBank, Size),
350+
/*Cost*/
351+
copyCost(AArch64::GPRRegBank, AArch64::FPRRegBank,
352+
TypeSize::Fixed(Size)),
349353
getCopyMapping(AArch64::GPRRegBankID, AArch64::FPRRegBankID, Size),
350354
/*NumOperands*/ 2);
351355

@@ -709,7 +713,7 @@ AArch64RegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
709713
assert(DstRB && SrcRB && "Both RegBank were nullptr");
710714
unsigned Size = getSizeInBits(DstReg, MRI, TRI);
711715
return getInstructionMapping(
712-
DefaultMappingID, copyCost(*DstRB, *SrcRB, Size),
716+
DefaultMappingID, copyCost(*DstRB, *SrcRB, TypeSize::Fixed(Size)),
713717
getCopyMapping(DstRB->getID(), SrcRB->getID(), Size),
714718
// We only care about the mapping of the destination.
715719
/*NumOperands*/ 1);
@@ -728,7 +732,7 @@ AArch64RegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
728732
const RegisterBank &SrcRB =
729733
SrcIsGPR ? AArch64::GPRRegBank : AArch64::FPRRegBank;
730734
return getInstructionMapping(
731-
DefaultMappingID, copyCost(DstRB, SrcRB, Size),
735+
DefaultMappingID, copyCost(DstRB, SrcRB, TypeSize::Fixed(Size)),
732736
getCopyMapping(DstRB.getID(), SrcRB.getID(), Size),
733737
// We only care about the mapping of the destination for COPY.
734738
/*NumOperands*/ Opc == TargetOpcode::G_BITCAST ? 2 : 1);
@@ -821,7 +825,7 @@ AArch64RegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
821825
Cost = copyCost(
822826
*AArch64GenRegisterBankInfo::PartMappings[OpRegBankIdx[0]].RegBank,
823827
*AArch64GenRegisterBankInfo::PartMappings[OpRegBankIdx[1]].RegBank,
824-
OpSize[0]);
828+
TypeSize::Fixed(OpSize[0]));
825829
break;
826830
case TargetOpcode::G_LOAD: {
827831
// Loading in vector unit is slightly more expensive.

‎llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ class AArch64RegisterBankInfo final : public AArch64GenRegisterBankInfo {
140140
AArch64RegisterBankInfo(const TargetRegisterInfo &TRI);
141141

142142
unsigned copyCost(const RegisterBank &A, const RegisterBank &B,
143-
unsigned Size) const override;
143+
TypeSize Size) const override;
144144

145145
const RegisterBank &getRegBankFromRegClass(const TargetRegisterClass &RC,
146146
LLT) const override;

‎llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ bool AMDGPURegisterBankInfo::isDivergentRegBank(const RegisterBank *RB) const {
229229

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

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

35483548
const ValueMapping &ValMap = getValueMapping(0, Size, *DstBank);

‎llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ class AMDGPURegisterBankInfo final : public AMDGPUGenRegisterBankInfo {
165165
bool isDivergentRegBank(const RegisterBank *RB) const override;
166166

167167
unsigned copyCost(const RegisterBank &A, const RegisterBank &B,
168-
unsigned Size) const override;
168+
TypeSize Size) const override;
169169

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

0 commit comments

Comments
 (0)