Skip to content

[RISCV] Add Qualcomm uC Xqciac (Load-Store Adress calculation) extension #121037

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 2 commits into from
Dec 29, 2024
Merged

[RISCV] Add Qualcomm uC Xqciac (Load-Store Adress calculation) extension #121037

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
304147a
[RISCV] Add Qualcomm uC Xqciac (Load-Store Adress calculation) extension
hchandel Dec 24, 2024
cb63d27
Update RISCVInstrInfoXqci.td
svs-quic Dec 27, 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
1 change: 1 addition & 0 deletions clang/test/Driver/print-supported-extensions-riscv.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@
// CHECK-NEXT: ssctr 1.0 'Ssctr' (Control Transfer Records Supervisor Level)
// CHECK-NEXT: svukte 0.3 'Svukte' (Address-Independent Latency of User-Mode Faults to Supervisor Addresses)
// CHECK-NEXT: xqcia 0.2 'Xqcia' (Qualcomm uC Arithmetic Extension)
// CHECK-NEXT: xqciac 0.2 'Xqciac' (Qualcomm uC Load-Store Address Calculation Extension)
// CHECK-NEXT: xqcics 0.2 'Xqcics' (Qualcomm uC Conditional Select Extension)
// CHECK-NEXT: xqcicsr 0.2 'Xqcicsr' (Qualcomm uC CSR Extension)
// CHECK-NEXT: xqcilsm 0.2 'Xqcilsm' (Qualcomm uC Load Store Multiple Extension)
Expand Down
3 changes: 3 additions & 0 deletions llvm/docs/RISCVUsage.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,9 @@ The current vendor extensions supported are:
``experimental-Xqcia``
LLVM implements `version 0.2 of the Qualcomm uC Arithmetic extension specification <https://github.com/quic/riscv-unified-db/releases/latest>`__ by Qualcomm. All instructions are prefixed with `qc.` as described in the specification. These instructions are only available for riscv32.

``experimental-Xqciac``
LLVM implements `version 0.2 of the Qualcomm uC Load-Store Address Calculation extension specification <https://github.com/quic/riscv-unified-db/releases/latest>`__ by Qualcomm. All instructions are prefixed with `qc.` as described in the specification. These instructions are only available for riscv32.

``experimental-Xqcics``
LLVM implements `version 0.2 of the Qualcomm uC Conditional Select extension specification <https://github.com/quic/riscv-unified-db/releases/latest>`__ by Qualcomm. All instructions are prefixed with `qc.` as described in the specification. These instructions are only available for riscv32.

Expand Down
2 changes: 2 additions & 0 deletions llvm/docs/ReleaseNotes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,8 @@ Changes to the RISC-V Backend
extension.
* Adds experimental assembler support for the Qualcomm uC 'Xqcia` (Arithmetic)
extension.
* Adds experimental assembler support for the Qualcomm uC 'Xqciac` (Load-Store Address Calculation)
extension.
* Adds experimental assembler support for the Qualcomm uC 'Xqcics` (Conditonal Select)
extension.
* Adds experimental assembler support for the Qualcomm uC 'Xqcilsm` (Load Store Multiple)
Expand Down
12 changes: 12 additions & 0 deletions llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,16 @@ struct RISCVOperand final : public MCParsedAsmOperand {
VK == RISCVMCExpr::VK_RISCV_None;
}

bool isUImm5GT3() const {
if (!isImm())
return false;
RISCVMCExpr::VariantKind VK = RISCVMCExpr::VK_RISCV_None;
int64_t Imm;
bool IsConstantImm = evaluateConstantImm(getImm(), Imm, VK);
return IsConstantImm && isUInt<5>(Imm) && (Imm > 3) &&
VK == RISCVMCExpr::VK_RISCV_None;
}

bool isUImm8GE32() const {
int64_t Imm;
RISCVMCExpr::VariantKind VK = RISCVMCExpr::VK_RISCV_None;
Expand Down Expand Up @@ -1520,6 +1530,8 @@ bool RISCVAsmParser::matchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 5) - 1);
case Match_InvalidUImm5NonZero:
return generateImmOutOfRangeError(Operands, ErrorInfo, 1, (1 << 5) - 1);
case Match_InvalidUImm5GT3:
return generateImmOutOfRangeError(Operands, ErrorInfo, 4, (1 << 5) - 1);
case Match_InvalidUImm6:
return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 6) - 1);
case Match_InvalidUImm7:
Expand Down
6 changes: 6 additions & 0 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 @@ -692,6 +692,9 @@ DecodeStatus RISCVDisassembler::getInstruction32(MCInst &MI, uint64_t &Size,
"Qualcomm uC Conditional Select custom opcode table");
TRY_TO_DECODE_FEATURE(RISCV::FeatureVendorXqcilsm, DecoderTableXqcilsm32,
"Qualcomm uC Load Store Multiple custom opcode table");
TRY_TO_DECODE_FEATURE(
RISCV::FeatureVendorXqciac, DecoderTableXqciac32,
"Qualcomm uC Load-Store Address Calculation custom opcode table");
TRY_TO_DECODE(true, DecoderTable32, "RISCV32 table");

return MCDisassembler::Fail;
Expand All @@ -718,6 +721,9 @@ DecodeStatus RISCVDisassembler::getInstruction16(MCInst &MI, uint64_t &Size,
TRY_TO_DECODE_FEATURE(
RISCV::FeatureStdExtZcmp, DecoderTableRVZcmp16,
"Zcmp table (16-bit Push/Pop & Double Move Instructions)");
TRY_TO_DECODE_FEATURE(
RISCV::FeatureVendorXqciac, DecoderTableXqciac16,
"Qualcomm uC Load-Store Address Calculation custom 16bit opcode table");
TRY_TO_DECODE_AND_ADD_SP(STI.hasFeature(RISCV::FeatureVendorXwchc),
DecoderTableXwchc16,
"WCH QingKe XW custom opcode table");
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ enum OperandType : unsigned {
OPERAND_UIMM4,
OPERAND_UIMM5,
OPERAND_UIMM5_NONZERO,
OPERAND_UIMM5_GT3,
OPERAND_UIMM5_LSB0,
OPERAND_UIMM6,
OPERAND_UIMM6_LSB0,
Expand Down
8 changes: 8 additions & 0 deletions llvm/lib/Target/RISCV/RISCVFeatures.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1274,6 +1274,14 @@ def HasVendorXqcilsm
AssemblerPredicate<(all_of FeatureVendorXqcilsm),
"'Xqcilsm' (Qualcomm uC Load Store Multiple Extension)">;

def FeatureVendorXqciac
: RISCVExperimentalExtension<0, 2, "Qualcomm uC Load-Store Address Calculation Extension",
[FeatureStdExtZca]>;
def HasVendorXqciac
: Predicate<"Subtarget->hasVendorXqciac()">,
AssemblerPredicate<(all_of FeatureVendorXqciac),
"'Xqciac' (Qualcomm uC Load-Store Address Calculation Extension)">;

//===----------------------------------------------------------------------===//
// LLVM specific features and extensions
//===----------------------------------------------------------------------===//
Expand Down
38 changes: 38 additions & 0 deletions llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@ def uimm5nonzero : RISCVOp<XLenVT>,
let OperandType = "OPERAND_UIMM5_NONZERO";
}

def uimm5gt3 : RISCVOp<XLenVT>, ImmLeaf<XLenVT,
[{return (Imm > 3) && isUInt<5>(Imm);}]> {
let ParserMatchClass = UImmAsmOperand<5, "GT3">;
let DecoderMethod = "decodeUImmOperand<5>";
let OperandType = "OPERAND_UIMM5_GT3";
}

def uimm11 : RISCVUImmLeafOp<11>;

//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -184,6 +191,37 @@ let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in {
} // hasSideEffects = 0, mayLoad = 0, mayStore = 0
} // Predicates = [HasVendorXqcia, IsRV32], DecoderNamespace = "Xqcia"

let Predicates = [HasVendorXqciac, IsRV32], DecoderNamespace = "Xqciac" in {
let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in {
def QC_C_MULADDI : RVInst16CL<0b001, 0b10, (outs GPRC:$rd_wb),
(ins GPRC:$rd, GPRC:$rs1, uimm5:$uimm),
"qc.c.muladdi", "$rd, $rs1, $uimm"> {
let Constraints = "$rd = $rd_wb";
bits<5> uimm;

let Inst{12-10} = uimm{3-1};
let Inst{6} = uimm{0};
let Inst{5} = uimm{4};
}
Copy link
Collaborator

@topperc topperc Dec 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Should we add a CompressPat to turn qc.muladdi to qc.c.muladdi automatically?

Copy link
Contributor

@svs-quic svs-quic Dec 27, 2024

Choose a reason for hiding this comment

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

Given that this patch is mainly to add the instructions and basic assembler support, I think we can add patterns in follow up patches.

Copy link
Collaborator

@topperc topperc Dec 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

CompressPat is used by the assembler to turn addi to c.addi, for example. So it is considered an assembler feature.

Copy link
Contributor

@svs-quic svs-quic Dec 27, 2024

Choose a reason for hiding this comment

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

Fair enough. However, the current implementation of the CompressPat assumes that there are no tied operands in the source dag which is not true for the case of qc.muladdi. As a result I'm running into errors in verifyDagOpCount from CompressInstEmitter.cpp while writing these patterns. We will need to change this behaviour before we can write compressed patterns. I am happy to work on these changes.

Can we merge this patch in its current state and re-visit the compress pattern once we have fixed the above issue?

// Source instructions are non compressed instructions and don't have tied
// operands.
if (IsSource)
  PrintFatalError(Inst.TheDef->getLoc(),
                  "Input operands for Inst '" + Inst.TheDef->getName() +
                      "' and input Dag operand count mismatch");

Copy link
Member

@lenary lenary Dec 27, 2024

Choose a reason for hiding this comment

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

@topperc there are other issues with adding compresspats for Xqci instructions which we are aware of and making plans to address - mostly around the fact there's no way to switch compresspats off for a pat where the larger and smaller instruction are in the same extension. We have a sketch of the solution internally but I've not finished it because we see CompressPats as an optimisation, not part of base assembler/linker support.

Copy link
Collaborator

@topperc topperc Dec 27, 2024

Choose a reason for hiding this comment

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

Why does the whole extension depend on Zca instead of just qc.c.muladdi?

Copy link
Member

@lenary lenary Dec 28, 2024

Choose a reason for hiding this comment

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

Unlike the main RISC-V ISA, the Xqci architects did not want to split instructions into (sub-)extensions by size (as you can see, they have been split by functionality). To make the ABI much easier, they agreed that all of the sub-extensions with any 16- or 48-bit instructions would require Zca, as the compiler will need to be able to insert c.nop when aligning etc etc.

Not splitting instructions into extensions by size does mean that existing assumptions used by CompressPat no longer quite hold. At the moment, it is enough to do .option norvc (or an equivalent with .option arch) for the C extension to prevent compression, but you cannot disable qc.c.muladdi without disabling qc.muladdi, so we cannot just use architecture features to gate compression patterns (There are even more issues around our 48-bit instructions, which I don't want to write out here). As I said, I have been thinking about an approach for this, but I haven't had time to work through the patch, test it, and present it for review. This is why none of our Xqci* patches so far have had CompressPats, and we do not intend to add them for the moment. Most of these issues are reasonably orthogonal to the issue that @svs-quic mentioned in the actual tablegen code for CompressPats.


def QC_MULADDI : RVInstI<0b110, OPC_CUSTOM_0, (outs GPRNoX0:$rd_wb),
(ins GPRNoX0:$rd, GPRNoX0:$rs1, simm12:$imm12),
"qc.muladdi", "$rd, $rs1, $imm12"> {
let Constraints = "$rd = $rd_wb";
}

def QC_SHLADD : RVInstRBase<0b011, OPC_CUSTOM_0, (outs GPR:$rd),
(ins GPRMem:$rs1, GPRNoX0:$rs2, uimm5gt3:$shamt),
"qc.shladd", "$rd, $rs1, $rs2, $shamt"> {
bits<5> shamt;

let Inst{31-30} = 0b01;
let Inst{29-25} = shamt;
}

} // hasSideEffects = 0, mayLoad = 0, mayStore = 0
} // Predicates = [HasVendorXqciac, IsRV32], DecoderNamespace = "Xqciac"

let Predicates = [HasVendorXqcics, IsRV32], DecoderNamespace = "Xqcics" in {
def QC_SELECTIIEQ : QCISELECTIICC <0b010, "qc.selectiieq">;
def QC_SELECTIINE : QCISELECTIICC <0b011, "qc.selectiine">;
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/TargetParser/RISCVISAInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,7 @@ Error RISCVISAInfo::checkDependency() {
bool HasZvl = MinVLen != 0;
bool HasZcmt = Exts.count("zcmt") != 0;
static constexpr StringLiteral XqciExts[] = {
{"xqcia"}, {"xqcics"}, {"xqcicsr"}, {"xqcilsm"}, {"xqcisls"}};
{"xqcia"}, {"xqciac"}, {"xqcics"}, {"xqcicsr"}, {"xqcilsm"}, {"xqcisls"}};

if (HasI && HasE)
return getIncompatibleError("i", "e");
Expand Down
2 changes: 2 additions & 0 deletions llvm/test/CodeGen/RISCV/attributes.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
; RUN: llc -mtriple=riscv32 -mattr=+xtheadsync %s -o - | FileCheck --check-prefix=RV32XTHEADSYNC %s
; RUN: llc -mtriple=riscv32 -mattr=+xwchc %s -o - | FileCheck --check-prefix=RV32XWCHC %s
; RUN: llc -mtriple=riscv32 -mattr=+experimental-xqcia %s -o - | FileCheck --check-prefix=RV32XQCIA %s
; RUN: llc -mtriple=riscv32 -mattr=+experimental-xqciac %s -o - | FileCheck --check-prefix=RV32XQCIAC %s
; RUN: llc -mtriple=riscv32 -mattr=+experimental-xqcics %s -o - | FileCheck --check-prefix=RV32XQCICS %s
; RUN: llc -mtriple=riscv32 -mattr=+experimental-xqcicsr %s -o - | FileCheck --check-prefix=RV32XQCICSR %s
; RUN: llc -mtriple=riscv32 -mattr=+experimental-xqcilsm %s -o - | FileCheck --check-prefix=RV32XQCILSM %s
Expand Down Expand Up @@ -391,6 +392,7 @@
; RV32XTHEADSYNC: .attribute 5, "rv32i2p1_xtheadsync1p0"
; RV32XWCHC: .attribute 5, "rv32i2p1_xwchc2p2"
; RV32XQCIA: .attribute 5, "rv32i2p1_xqcia0p2"
; RV32XQCIAC: .attribute 5, "rv32i2p1_zca1p0_xqciac0p2"
; RV32XQCICS: .attribute 5, "rv32i2p1_xqcics0p2"
; RV32XQCICSR: .attribute 5, "rv32i2p1_xqcicsr0p2"
; RV32XQCILSM: .attribute 5, "rv32i2p1_xqcilsm0p2"
Expand Down
43 changes: 43 additions & 0 deletions llvm/test/MC/RISCV/xqciac-invalid.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# Xqciac - Qualcomm uC Load-Store Address Calculation Extension
# RUN: not llvm-mc -triple riscv32 -mattr=+experimental-xqciac < %s 2>&1 \
# RUN: | FileCheck -check-prefixes=CHECK,CHECK-IMM %s
# RUN: not llvm-mc -triple riscv32 -mattr=-experimental-xqciac < %s 2>&1 \
# RUN: | FileCheck -check-prefixes=CHECK,CHECK-EXT %s

# CHECK: :[[@LINE+1]]:14: error: invalid operand for instruction
qc.c.muladdi x5, x10, 4

# CHECK: :[[@LINE+1]]:1: error: too few operands for instruction
qc.c.muladdi x15

# CHECK-IMM: :[[@LINE+1]]:24: error: immediate must be an integer in the range [0, 31]
qc.c.muladdi x10, x15, 32

# CHECK-EXT: :[[@LINE+1]]:1: error: instruction requires the following: 'Xqciac' (Qualcomm uC Load-Store Address Calculation Extension)
qc.c.muladdi x10, x15, 20


# CHECK: :[[@LINE+1]]:12: error: invalid operand for instruction
qc.muladdi x0, x10, 1048577

# CHECK: :[[@LINE+1]]:1: error: too few operands for instruction
qc.muladdi x10

# CHECK-IMM: :[[@LINE+1]]:22: error: operand must be a symbol with %lo/%pcrel_lo/%tprel_lo modifier or an integer in the range [-2048, 2047]
qc.muladdi x10, x15, 8589934592

# CHECK-EXT: :[[@LINE+1]]:1: error: instruction requires the following: 'Xqciac' (Qualcomm uC Load-Store Address Calculation Extension)
qc.muladdi x10, x15, 577


# CHECK: :[[@LINE+1]]:11: error: invalid operand for instruction
qc.shladd 0, x10, 1048577

# CHECK: :[[@LINE+1]]:1: error: too few operands for instruction
qc.shladd x10

# CHECK-IMM: :[[@LINE+1]]:26: error: immediate must be an integer in the range [4, 31]
qc.shladd x10, x15, x11, 2

# CHECK-EXT: :[[@LINE+1]]:1: error: instruction requires the following: 'Xqciac' (Qualcomm uC Load-Store Address Calculation Extension)
qc.shladd x10, x15, x11, 5
49 changes: 49 additions & 0 deletions llvm/test/MC/RISCV/xqciac-valid.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# Xqciac - Qualcomm uC Load-Store Address Calculation Extension
# RUN: llvm-mc %s -triple=riscv32 -mattr=+experimental-xqciac -riscv-no-aliases -show-encoding \
# RUN: | FileCheck -check-prefixes=CHECK-ENC,CHECK-INST %s
# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+experimental-xqciac < %s \
# RUN: | llvm-objdump --mattr=+experimental-xqciac -M no-aliases --no-print-imm-hex -d - \
# RUN: | FileCheck -check-prefix=CHECK-INST %s
# RUN: llvm-mc %s -triple=riscv32 -mattr=+experimental-xqciac -show-encoding \
# RUN: | FileCheck -check-prefixes=CHECK-ENC,CHECK-INST %s
# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+experimental-xqciac < %s \
# RUN: | llvm-objdump --mattr=+experimental-xqciac --no-print-imm-hex -d - \
# RUN: | FileCheck -check-prefix=CHECK-INST %s

# CHECK-INST: qc.c.muladdi a0, a1, 0
# CHECK-ENC: encoding: [0x8a,0x21]
qc.c.muladdi x10, x11, 0

# CHECK-INST: qc.c.muladdi a0, a1, 31
# CHECK-ENC: encoding: [0xea,0x3d]
qc.c.muladdi x10, x11, 31

# CHECK-INST: qc.c.muladdi a0, a1, 16
# CHECK-ENC: encoding: [0xaa,0x21]
qc.c.muladdi x10, x11, 16


# CHECK-INST: qc.muladdi tp, t0, 1234
# CHECK-ENC: encoding: [0x0b,0xe2,0x22,0x4d]
qc.muladdi x4, x5, 1234

# CHECK-INST: qc.muladdi a0, a1, -2048
# CHECK-ENC: encoding: [0x0b,0xe5,0x05,0x80]
qc.muladdi x10, x11, -2048

# CHECK-INST: qc.muladdi a0, a1, 2047
# CHECK-ENC: encoding: [0x0b,0xe5,0xf5,0x7f]
qc.muladdi x10, x11, 2047


# CHECK-INST: qc.shladd tp, t0, t1, 12
# CHECK-ENC: encoding: [0x0b,0xb2,0x62,0x58]
qc.shladd x4, x5, x6, 12

# CHECK-INST: qc.shladd a0, a1, a2, 4
# CHECK-ENC: encoding: [0x0b,0xb5,0xc5,0x48]
qc.shladd x10, x11, x12, 4

# CHECK-INST: qc.shladd a0, a1, a2, 31
# CHECK-ENC: encoding: [0x0b,0xb5,0xc5,0x7e]
qc.shladd x10, x11, x12, 31
5 changes: 3 additions & 2 deletions llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -654,8 +654,8 @@ TEST(ParseArchString, RejectsConflictingExtensions) {
}

for (StringRef Input :
{"rv64i_xqcisls0p2", "rv64i_xqcia0p2", "rv64i_xqcicsr0p2",
"rv64i_xqcilsm0p2", "rv64i_xqcics0p2"}) {
{"rv64i_xqcisls0p2", "rv64i_xqcia0p2", "rv64i_xqciac0p2",
"rv64i_xqcicsr0p2", "rv64i_xqcilsm0p2", "rv64i_xqcics0p2"}) {
EXPECT_THAT(
toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
::testing::EndsWith(" is only supported for 'rv32'"));
Expand Down Expand Up @@ -1113,6 +1113,7 @@ Experimental extensions
ssctr 1.0
svukte 0.3
xqcia 0.2
xqciac 0.2
xqcics 0.2
xqcicsr 0.2
xqcilsm 0.2
Expand Down
Loading