Skip to content

[RISCV] Consolidate some DecoderNamespaces for standard extensions. #128954

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

Merged
merged 1 commit into from
Feb 27, 2025
Merged

[RISCV] Consolidate some DecoderNamespaces for standard extensions. #128954

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 13 additions & 19 deletions llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -657,16 +657,6 @@ DecodeStatus RISCVDisassembler::getInstruction32(MCInst &MI, uint64_t &Size,

uint32_t Insn = support::endian::read32le(Bytes.data());

TRY_TO_DECODE(STI.hasFeature(RISCV::FeatureStdExtZdinx) &&
!STI.hasFeature(RISCV::Feature64Bit),
DecoderTableRV32Zdinx32,
"RV32Zdinx (Double in Integer and rv32)");
TRY_TO_DECODE(STI.hasFeature(RISCV::FeatureStdExtZacas) &&
!STI.hasFeature(RISCV::Feature64Bit),
DecoderTableRV32Zacas32,
"RV32Zacas (Compare-And-Swap and rv32)");
TRY_TO_DECODE_FEATURE(RISCV::FeatureStdExtZfinx, DecoderTableRVZfinx32,
"RVZfinx (Float in Integer)");
TRY_TO_DECODE_FEATURE(RISCV::FeatureVendorXVentanaCondOps,
DecoderTableXVentana32, "XVentanaCondOps");
TRY_TO_DECODE_FEATURE(RISCV::FeatureVendorXTHeadBa, DecoderTableXTHeadBa32,
Expand Down Expand Up @@ -721,6 +711,11 @@ DecodeStatus RISCVDisassembler::getInstruction32(MCInst &MI, uint64_t &Size,
TRY_TO_DECODE_FEATURE_ANY(XRivosFeatureGroup, DecoderTableXRivos32, "Rivos");

TRY_TO_DECODE(true, DecoderTable32, "RISCV32");
TRY_TO_DECODE(true, DecoderTableRV32GPRPair32,
"RV32GPRPair (rv32 and GPR pairs)");
TRY_TO_DECODE(true, DecoderTableZfinx32, "Zfinx (Float in Integer)");
TRY_TO_DECODE(true, DecoderTableZdinxRV32GPRPair32,
"ZdinxRV32GPRPair (rv32 and Double in Integer)");
Comment on lines 713 to +718
Copy link
Member

Choose a reason for hiding this comment

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

Why do we decode all the standard stuff last, rather than before custom extensions? I'd have thought we want to decode the standard extensions with higher priority than the custom extensions, but maybe not?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Why do we decode all the standard stuff last, rather than before custom extensions? I'd have thought we want to decode the standard extensions with higher priority than the custom extensions, but maybe not?

A good question. As far as I know all the vendor extensions are mutually exclusive with the standard extensions, except for Xqcisim which appears to use hint encodings of standard instructions. So they could go last and it would be faster for the common case.

Copy link
Member

Choose a reason for hiding this comment

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

I'm less concerned for speed, more for "we should prioritise decoding the standard extensions", which feels like a good principle. It sounds like doing so might require changing how we implemented Xqcisim, so I can do the reordering as a follow-up.

Copy link
Contributor

Choose a reason for hiding this comment

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

There were some concerns about ordering raised by @jrtc27 in https://reviews.llvm.org/D151309.

Downstream we have a separate encoding mode that replaces some standard instructions with alternative ones; even if the order doesn't matter upstream, it would be preferable for us if the standard table was the last one not the first one.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jrtc27 why couldn't you put your downstream stuff before the standard table even if the standard table is first upstream?


return MCDisassembler::Fail;
}
Expand All @@ -736,23 +731,22 @@ DecodeStatus RISCVDisassembler::getInstruction16(MCInst &MI, uint64_t &Size,
Size = 2;

uint32_t Insn = support::endian::read16le(Bytes.data());
TRY_TO_DECODE_AND_ADD_SP(!STI.hasFeature(RISCV::Feature64Bit),
DecoderTableRISCV32Only_16,
"RISCV32Only_16 (16-bit Instruction)");
TRY_TO_DECODE_FEATURE(RISCV::FeatureStdExtZicfiss, DecoderTableZicfiss16,
"RVZicfiss (Shadow Stack)");
TRY_TO_DECODE_FEATURE(RISCV::FeatureStdExtZcmt, DecoderTableRVZcmt16,
"Zcmt (16-bit Table Jump Instructions)");
TRY_TO_DECODE_FEATURE(RISCV::FeatureStdExtZcmp, DecoderTableRVZcmp16,
"Zcmp (16-bit Push/Pop & Double Move Instructions)");

TRY_TO_DECODE_FEATURE_ANY(XqciFeatureGroup, DecoderTableXqci16,
"Qualcomm uC 16bit");

TRY_TO_DECODE_AND_ADD_SP(STI.hasFeature(RISCV::FeatureVendorXwchc),
DecoderTableXwchc16, "WCH QingKe XW");

// DecoderTableZicfiss16 must be checked before DecoderTable16.
TRY_TO_DECODE(true, DecoderTableZicfiss16, "RVZicfiss (Shadow Stack)");
TRY_TO_DECODE_AND_ADD_SP(true, DecoderTable16,
"RISCV_C (16-bit Instruction)");
TRY_TO_DECODE_AND_ADD_SP(true, DecoderTableRISCV32Only_16,
"RISCV32Only_16 (16-bit Instruction)");
// Zc* instructions incompatible with Zcf or Zcd.
TRY_TO_DECODE(true, DecoderTableZcOverlap16,
"ZcOverlap (16-bit Instructions overlapping with Zcf/Zcd)");

return MCDisassembler::Fail;
}
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Target/RISCV/RISCVInstrInfoD.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ def FPR64IN32X : RegisterOperand<GPRPair> {

def DExt : ExtInfo<"", "", [HasStdExtD], f64, FPR64, FPR32, FPR64, ?>;

def ZdinxExt : ExtInfo<"_INX", "RVZfinx", [HasStdExtZdinx, IsRV64],
def ZdinxExt : ExtInfo<"_INX", "Zfinx", [HasStdExtZdinx, IsRV64],
f64, FPR64INX, FPR32INX, FPR64INX, ?>;
def Zdinx32Ext : ExtInfo<"_IN32X", "RV32Zdinx", [HasStdExtZdinx, IsRV32],
def Zdinx32Ext : ExtInfo<"_IN32X", "ZdinxRV32GPRPair", [HasStdExtZdinx, IsRV32],
f64, FPR64IN32X, FPR32INX, FPR64IN32X, ?>;

defvar DExts = [DExt, ZdinxExt, Zdinx32Ext];
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/RISCV/RISCVInstrInfoF.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ class ExtInfo<string suffix, string space, list<Predicate> predicates,

def FExt : ExtInfo<"", "", [HasStdExtF], f32, FPR32, FPR32, ?, ?>;

def ZfinxExt : ExtInfo<"_INX", "RVZfinx", [HasStdExtZfinx], f32, FPR32INX, FPR32INX, ?, ?>;
def ZfinxExt : ExtInfo<"_INX", "Zfinx", [HasStdExtZfinx], f32, FPR32INX, FPR32INX, ?, ?>;

defvar FExts = [FExt, ZfinxExt];

Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/RISCV/RISCVInstrInfoZa.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ let Predicates = [HasStdExtZacas], IsSignExtendingOpW = 1 in {
defm AMOCAS_W : AMO_cas_aq_rl<0b00101, 0b010, "amocas.w", GPR>;
} // Predicates = [HasStdExtZacas]

let Predicates = [HasStdExtZacas, IsRV32], DecoderNamespace = "RV32Zacas" in {
let Predicates = [HasStdExtZacas, IsRV32], DecoderNamespace = "RV32GPRPair" in {
defm AMOCAS_D_RV32 : AMO_cas_aq_rl<0b00101, 0b011, "amocas.d", GPRPairRV32>;
} // Predicates = [HasStdExtZacas, IsRV32]

Expand Down
12 changes: 6 additions & 6 deletions llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ def C_SH_INX : CStoreH_rri<0b100011, 0b0, "c.sh", GPRF16C>,
} // Predicates = [HasStdExtZcb]

// Zcmp
let DecoderNamespace = "RVZcmp", Predicates = [HasStdExtZcmp],
let DecoderNamespace = "ZcOverlap", Predicates = [HasStdExtZcmp],
hasSideEffects = 0, mayLoad = 0, mayStore = 0 in {
let Defs = [X10, X11] in
def CM_MVA01S : RVInst16CA<0b101011, 0b11, 0b10, (outs),
Expand All @@ -229,9 +229,9 @@ let Uses = [X10, X11] in
def CM_MVSA01 : RVInst16CA<0b101011, 0b01, 0b10, (outs SR07:$rs1, SR07:$rs2),
(ins), "cm.mvsa01", "$rs1, $rs2">,
Sched<[WriteIALU, WriteIALU, ReadIALU, ReadIALU]>;
} // DecoderNamespace = "RVZcmp", Predicates = [HasStdExtZcmp]...
} // DecoderNamespace = "ZcOverlap", Predicates = [HasStdExtZcmp]...

let DecoderNamespace = "RVZcmp", Predicates = [HasStdExtZcmp] in {
let DecoderNamespace = "ZcOverlap", Predicates = [HasStdExtZcmp] in {
let hasSideEffects = 0, mayLoad = 0, mayStore = 1, Uses = [X2], Defs = [X2] in
def CM_PUSH : RVInstZcCPPP<0b11000, "cm.push", negstackadj>,
Sched<[WriteIALU, ReadIALU, ReadStoreData, ReadStoreData,
Expand Down Expand Up @@ -260,9 +260,9 @@ def CM_POP : RVInstZcCPPP<0b11010, "cm.pop">,
Sched<[WriteIALU, WriteLDW, WriteLDW, WriteLDW, WriteLDW,
WriteLDW, WriteLDW, WriteLDW, WriteLDW, WriteLDW, WriteLDW,
WriteLDW, WriteLDW, WriteLDW, ReadIALU]>;
} // DecoderNamespace = "RVZcmp", Predicates = [HasStdExtZcmp]...
} // DecoderNamespace = "ZcOverlap", Predicates = [HasStdExtZcmp]...

let DecoderNamespace = "RVZcmt", Predicates = [HasStdExtZcmt],
let DecoderNamespace = "ZcOverlap", Predicates = [HasStdExtZcmt],
hasSideEffects = 0, mayLoad = 0, mayStore = 0 in {
def CM_JT : RVInst16CJ<0b101, 0b10, (outs), (ins uimm5:$index),
"cm.jt", "$index">{
Expand All @@ -280,7 +280,7 @@ def CM_JALT : RVInst16CJ<0b101, 0b10, (outs), (ins uimm8ge32:$index),
let Inst{12-10} = 0b000;
let Inst{9-2} = index;
}
} // DecoderNamespace = "RVZcmt", Predicates = [HasStdExtZcmt]...
} // DecoderNamespace = "ZcOverlap", Predicates = [HasStdExtZcmt]...


let Predicates = [HasStdExtZcb, HasStdExtZmmul] in{
Expand Down
12 changes: 6 additions & 6 deletions llvm/lib/Target/RISCV/RISCVInstrInfoZfh.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,22 +52,22 @@ def ZfhDExt : ExtInfo<"", "", [HasStdExtZfh, HasStdExtD],
def ZfhminDExt : ExtInfo<"", "", [HasStdExtZfhmin, HasStdExtD],
?, ?, FPR32, FPR64, FPR16>;

def ZhinxExt : ExtInfo<"_INX", "RVZfinx",
def ZhinxExt : ExtInfo<"_INX", "Zfinx",
[HasStdExtZhinx],
f16, FPR16INX, FPR32INX, ?, FPR16INX>;
def ZhinxminExt : ExtInfo<"_INX", "RVZfinx",
def ZhinxminExt : ExtInfo<"_INX", "Zfinx",
[HasStdExtZhinxmin],
f16, FPR16INX, FPR32INX, ?, FPR16INX>;
def ZhinxZdinxExt : ExtInfo<"_INX", "RVZfinx",
def ZhinxZdinxExt : ExtInfo<"_INX", "Zfinx",
[HasStdExtZhinx, HasStdExtZdinx, IsRV64],
?, ?, FPR32INX, FPR64INX, FPR16INX>;
def ZhinxminZdinxExt : ExtInfo<"_INX", "RVZfinx",
def ZhinxminZdinxExt : ExtInfo<"_INX", "Zfinx",
[HasStdExtZhinxmin, HasStdExtZdinx, IsRV64],
?, ?, FPR32INX, FPR64INX, FPR16INX>;
def ZhinxZdinx32Ext : ExtInfo<"_IN32X", "RV32Zdinx",
def ZhinxZdinx32Ext : ExtInfo<"_IN32X", "ZdinxGPRPairRV32",
[HasStdExtZhinx, HasStdExtZdinx, IsRV32],
?, ?, FPR32INX, FPR64IN32X, FPR16INX >;
def ZhinxminZdinx32Ext : ExtInfo<"_IN32X", "RV32Zdinx",
def ZhinxminZdinx32Ext : ExtInfo<"_IN32X", "ZdinxGPRPairRV32",
[HasStdExtZhinxmin, HasStdExtZdinx, IsRV32],
?, ?, FPR32INX, FPR64IN32X, FPR16INX>;

Expand Down