Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
| llvm/lib/Target/RISCV/RISCVInstrFormatsV.td | ||
|---|---|---|
| 127 ↗ | (On Diff #499348) | Move this to RISCVInstrInfoXsf.td |
| llvm/lib/Target/RISCV/RISCVInstrInfoXsf.td | ||
| 101 ↗ | (On Diff #499348) | We need to use DecoderNamespace on these and update RISCVDisassembler to lookup the VCIX table when the extension is enabled. See the other vendor instruction for examples. |
Normally we use the less cumbersome “MC layer” rather than “assembler and dis-assembler” (which also shouldn’t be hyphenated)
| llvm/lib/Support/RISCVISAInfo.cpp | ||
|---|---|---|
| 133 | Please alphabetize above xthead | |
I haven't looked through the .td description yet, but left some first comments (all pretty easily addressable and minor).
| llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp | ||
|---|---|---|
| 3012 | The usual LLVM style is to use early returns to reduce indentation. see https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code Also, we already have unsigned Opcode = Inst.getOpcode(); above, so you can just use that rather than introducing VCIXOpcode. | |
| llvm/lib/Target/RISCV/RISCVFeatures.td | ||
| 644 | Nit: We're not very consistent, but I think overall it's more common we use title case for the description in parentheses. | |
| llvm/lib/Target/RISCV/RISCVInstrInfo.td | ||
| 1930 | Nit: Shouldn't this be RISCVInstrInfoXSf to match the capitalisation you use in the RISCVUsage addition? | |
| llvm/test/MC/RISCV/rvv/xsfvcp-invalid.s | ||
| 3 | We typically have rv32 CHECK lines as well as rv64 | |
| llvm/test/MC/RISCV/rvv/xsfvcp.s | ||
| 10 | We typically have rv32 CHECK lines as well as rv64 | |
| llvm/lib/Target/RISCV/RISCVInstrInfoXSf.td | ||
|---|---|---|
| 118 | What is the difference between this and CustomSiFiveVCIX? | |
| llvm/lib/Target/RISCV/RISCVInstrInfoXSf.td | ||
|---|---|---|
| 118 | CustomSiFiveVCIF inherits RVInstVCFCustom2 and CustomSiFiveVCIX inherits RVInstVCCustom2. | |
Please alphabetize above xthead