Skip to content
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.

Sign up for GitHub

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

[TRI][RISCV] Add methods to get common register class of two registers #118435

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[TRI][RISCV] Add methods to get common register class of two registers #118435

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7aee1b7
[TRI][RISCV] Add methods to get common register class of two registers
wangpc-pp Dec 3, 2024
7e898ca
Use hasSuperClassEq in FindRegWithEncoding
wangpc-pp Dec 5, 2024
ba1d49a
Get LMUL from TSFlags
wangpc-pp Dec 6, 2024
f395eab
Use template functions
wangpc-pp Dec 6, 2024
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
15 changes: 15 additions & 0 deletions llvm/include/llvm/CodeGen/TargetRegisterInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,13 +350,28 @@ class TargetRegisterInfo : public MCRegisterInfo {
const TargetRegisterClass *getMinimalPhysRegClass(MCRegister Reg,
MVT VT = MVT::Other) const;

/// Returns the common Register Class of two physical registers of the given
/// type, picking the most sub register class of the right type that contains
/// these two physregs.
const TargetRegisterClass *
getCommonMinimalPhysRegClass(MCRegister Reg1, MCRegister Reg2,
MVT VT = MVT::Other) const;

/// Returns the Register Class of a physical register of the given type,
/// picking the most sub register class of the right type that contains this
/// physreg. If there is no register class compatible with the given type,
/// returns nullptr.
const TargetRegisterClass *getMinimalPhysRegClassLLT(MCRegister Reg,
LLT Ty = LLT()) const;

/// Returns the common Register Class of two physical registers of the given
/// type, picking the most sub register class of the right type that contains
/// these two physregs. If there is no register class compatible with the
/// given type, returns nullptr.
const TargetRegisterClass *
getCommonMinimalPhysRegClassLLT(MCRegister Reg1, MCRegister Reg2,
LLT Ty = LLT()) const;

/// Return the maximal subclass of the given register class that is
/// allocatable or NULL.
const TargetRegisterClass *
Expand Down
37 changes: 37 additions & 0 deletions llvm/lib/CodeGen/TargetRegisterInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,25 @@ TargetRegisterInfo::getMinimalPhysRegClass(MCRegister reg, MVT VT) const {
return BestRC;
}

const TargetRegisterClass *TargetRegisterInfo::getCommonMinimalPhysRegClass(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two functions are almost identical, can we maybe use a template to contain all the inner logic? The only difference I see that's hard to factor out is Ty.isValid() vs VT == MVT::Other. Since they're both bools, maybe that could be passed in as a param, and the templating could take care of the rest?

Its very little code, but these kind of need to be kept in sync otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to represent default argument value in a template function?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking more that you'd just put all the logic in a new function that was templated, and let each of these just call that internally. Then you can keep your API as is (assuming it needs to be that way for a reason) and the two won't get out of sync. The only thing the original functions need to do is evaluate the boolean, and then return the call to the common impl.

so Somthing like this, where the current functions are just think wrappers.

const TargetRegisterClass *TargetRegisterInfo::getCommonMinimalPhysRegClass(MCRegister Reg1, MCRegister Reg2, MVT VT) const {
  bool isValid = VT == MVT::Other;
  return commonImpl( Reg1, Reg2, VT, isValid);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I used type traits to decide the value, please have a look.

MCRegister Reg1, MCRegister Reg2, MVT VT) const {
assert(Register::isPhysicalRegister(Reg1) &&
Register::isPhysicalRegister(Reg2) &&
"Reg1/Reg2 must be a physical register");

// Pick the most sub register class of the right type that contains
// this physreg.
const TargetRegisterClass *BestRC = nullptr;
for (const TargetRegisterClass *RC : regclasses()) {
if ((VT == MVT::Other || isTypeLegalForClass(*RC, VT)) &&
RC->contains(Reg1, Reg2) && (!BestRC || BestRC->hasSubClass(RC)))
BestRC = RC;
}

assert(BestRC && "Couldn't find the register class");
return BestRC;
}

const TargetRegisterClass *
TargetRegisterInfo::getMinimalPhysRegClassLLT(MCRegister reg, LLT Ty) const {
assert(Register::isPhysicalRegister(reg) &&
Expand All @@ -239,6 +258,24 @@ TargetRegisterInfo::getMinimalPhysRegClassLLT(MCRegister reg, LLT Ty) const {
return BestRC;
}

const TargetRegisterClass *TargetRegisterInfo::getCommonMinimalPhysRegClassLLT(
MCRegister Reg1, MCRegister Reg2, LLT Ty) const {
assert(Register::isPhysicalRegister(Reg1) &&
Register::isPhysicalRegister(Reg2) &&
"Reg1/Reg2 must be a physical register");

// Pick the most sub register class of the right type that contains
// this physreg.
const TargetRegisterClass *BestRC = nullptr;
for (const TargetRegisterClass *RC : regclasses()) {
if ((!Ty.isValid() || isTypeLegalForClass(*RC, Ty)) &&
RC->contains(Reg1, Reg2) && (!BestRC || BestRC->hasSubClass(RC)))
BestRC = RC;
}

return BestRC;
}

/// getAllocatableSetForRC - Toggle the bits that represent allocatable
/// registers for the specific register class.
static void getAllocatableSetForRC(const MachineFunction &MF,
Expand Down
18 changes: 6 additions & 12 deletions llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ void RISCVInstrInfo::copyPhysRegVector(
auto FindRegWithEncoding = [TRI](const TargetRegisterClass &RegClass,
uint16_t Encoding) {
MCRegister Reg = RISCV::V0 + Encoding;
if (&RegClass == &RISCV::VRRegClass)
if (RegClass.hasSuperClassEq(&RISCV::VRRegClass))
return Reg;
return TRI->getMatchingSuperReg(Reg, RISCV::sub_vrm1_0, &RegClass);
};
Expand Down Expand Up @@ -562,17 +562,11 @@ void RISCVInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
}

// VR->VR copies.
static const TargetRegisterClass *RVVRegClasses[] = {
&RISCV::VRRegClass, &RISCV::VRM2RegClass, &RISCV::VRM4RegClass,
&RISCV::VRM8RegClass, &RISCV::VRN2M1RegClass, &RISCV::VRN2M2RegClass,
&RISCV::VRN2M4RegClass, &RISCV::VRN3M1RegClass, &RISCV::VRN3M2RegClass,
&RISCV::VRN4M1RegClass, &RISCV::VRN4M2RegClass, &RISCV::VRN5M1RegClass,
&RISCV::VRN6M1RegClass, &RISCV::VRN7M1RegClass, &RISCV::VRN8M1RegClass};
for (const auto &RegClass : RVVRegClasses) {
if (RegClass->contains(DstReg, SrcReg)) {
copyPhysRegVector(MBB, MBBI, DL, DstReg, SrcReg, KillSrc, RegClass);
return;
}
const TargetRegisterClass *RegClass =
TRI->getCommonMinimalPhysRegClass(SrcReg, DstReg);
wangpc-pp marked this conversation as resolved.
Show resolved Hide resolved
if (RISCVRegisterInfo::isRVVRegClass(RegClass)) {
copyPhysRegVector(MBB, MBBI, DL, DstReg, SrcReg, KillSrc, RegClass);
return;
}

llvm_unreachable("Impossible reg-to-reg copy");
Expand Down
Loading