From a1a77bb149c67aa6bbb6755a2d5e5b2f1ab96968 Mon Sep 17 00:00:00 2001 From: Michael Maitland Date: Tue, 7 Nov 2023 06:25:02 -0800 Subject: [PATCH] [RISCV] Use TypeSize in places where needed for RegBankSelection This is a precommit for #71514 to use TypeSize instead of unsigned to avoid crashes when scalable vectors are used. --- llvm/include/llvm/CodeGen/RegisterBankInfo.h | 8 ++++---- llvm/lib/CodeGen/MachineVerifier.cpp | 2 +- llvm/lib/CodeGen/RegisterBankInfo.cpp | 9 +++++---- .../AArch64/GISel/AArch64RegisterBankInfo.cpp | 16 ++++++++++------ .../AArch64/GISel/AArch64RegisterBankInfo.h | 2 +- .../lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp | 4 ++-- llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.h | 2 +- 7 files changed, 24 insertions(+), 19 deletions(-) diff --git a/llvm/include/llvm/CodeGen/RegisterBankInfo.h b/llvm/include/llvm/CodeGen/RegisterBankInfo.h index 1ee1f6b6c32ed..459a31d962ef3 100644 --- a/llvm/include/llvm/CodeGen/RegisterBankInfo.h +++ b/llvm/include/llvm/CodeGen/RegisterBankInfo.h @@ -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; @@ -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 @@ -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::max(); } @@ -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 diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp index 6107fa5c43c57..729dfc67491ee 100644 --- a/llvm/lib/CodeGen/MachineVerifier.cpp +++ b/llvm/lib/CodeGen/MachineVerifier.cpp @@ -2262,7 +2262,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); diff --git a/llvm/lib/CodeGen/RegisterBankInfo.cpp b/llvm/lib/CodeGen/RegisterBankInfo.cpp index f9721d7d93869..6a96bb40f56ae 100644 --- a/llvm/lib/CodeGen/RegisterBankInfo.cpp +++ b/llvm/lib/CodeGen/RegisterBankInfo.cpp @@ -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()) { @@ -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) { @@ -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, diff --git a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp index 21f9f6437e4fe..4ca5b3674461d 100644 --- a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp +++ b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp @@ -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: @@ -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); @@ -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); @@ -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); @@ -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. diff --git a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.h b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.h index 48bd9fbeadb41..b6364c6a64099 100644 --- a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.h +++ b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.h @@ -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; diff --git a/llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp b/llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp index 4ee58c9ef0cb9..49322109bdb74 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp @@ -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)) { @@ -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); diff --git a/llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.h b/llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.h index 06bf3c7275471..b5d16e70ab23a 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.h +++ b/llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.h @@ -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;