[RISCV] Implement assembler support for XVentanaCondOps

[reames]

Summary

This change provides an implementation of the XVentanaCondOps vendor extension. This extension is defined in version 1.0.0 of the VTx-family custom instructions specification (https://github.com/ventanamicro/ventana-custom-extensions/releases/download/v1.0.0/ventana-custom-extensions-v1.0.0.pdf) by Ventana Micro Systems.

This change is intended to be a test case for our vendor extension policy. I believe this to be a case we clearly should accept, but it gives us an opportunity to discuss and set precedent on various policy and naming questions. (I will comment on the review to highlight questions I think are worth discussion.)

I intent to bring this up at the next RISCV sync call, and ensure we have consensus. Once this lands, I plan to use this extension to prototype selection lowering to conditional moves. There's an RVI proposal in flight, and the expectation is that lowering to these and the new RVI instructions is likely to be substantially similar.

[reames]

clang/test/Preprocessor/riscv-target-features.c

437

Naming wise, is xventanacondops what we want this called? The toolchain docs linked are a bit ambiguous on this point. They seem to be saying that the extension should maybe be xvtcondops. But that's not what the spec uses, not what we tend to use in discussion, and not what is being discussed for binutils.

439

This patch only includes positive tests for riscv64. Per the current specification, no rv32 chips have shipped with this feature. I chose not to actually reject the flags or elf settings for riscv32, but to not enable the assembler either.

What do we think is the right answer here? Should we accept the riscv32 forms? This would essentially be an LLVM extension. Should we explicitly error on the riscv32 form? Something else?

llvm/docs/RISCVUsage.rst

161

Anything else we want in the documentation? Remember, we're setting precedent here.

llvm/lib/Target/RISCV/RISCVInstrInfo.td

1785

How do we want to manage td files for vendor extensions?

I put them inline - which is probably not the right answer. I'm leaning towards a vendor specific td file with extensions split out if complexity justifies it. So, this patch would add a RISCVInstInfoXVentana.td.

Is that the precedent we want here?

[reames]

Fix silly typo

[craig.topper]

clang/test/Preprocessor/riscv-target-features.c

437

qemu seems to have also used xventanacondops.

llvm/lib/Target/RISCV/RISCV.td

422

It's not quite conditional move. It's conditional move or zero right? Might be better just says "Ops" or "Operations" instead of "Move".

426

Blank line above this.

llvm/lib/Target/RISCV/RISCVInstrFormats.td

148

Since the string here is used for .insn parsing, I think it should be CUSTOM_3. I'm not sure why we don't already have all 4 added.

[craig.topper]

Do these need their own DecoderNameSpace?

[reames]

In D137350#3906126, @craig.topper wrote:

Do these need their own DecoderNameSpace?

What is a decoder namespace? Some quick grepping and googling isn't very informative.

llvm/lib/Target/RISCV/RISCVInstrFormats.td

148

https://reviews.llvm.org/D137355

[reames]

Address review comments from @craig.topper

[craig.topper]

In D137350#3906234, @reames wrote:

In D137350#3906126, @craig.topper wrote:

Do these need their own DecoderNameSpace?

What is a decoder namespace? Some quick grepping and googling isn't very informative.

If another vendor also uses these opcodes for something else, I think we would need to partition the disassembler tables.

Some existing examples

RISCVInstrInfoC.td
330:let DecoderNamespace = "RISCV32Only_",
364:let DecoderNamespace = "RISCV32Only_",
409:    DecoderNamespace = "RISCV32Only_", Defs = [X1],
517:let DecoderNamespace = "RISCV32Only_",
577:let DecoderNamespace = "RISCV32Only_",

RISCVDisassembler::getInstruction checks some subtarget bits to select different tables where there are conflicts.

[jrtc27]

In D137350#3906126, @craig.topper wrote:

Do these need their own DecoderNameSpace?

I was going to come here to request that

[reames]

Add a decoder namespace.

I did one per vendor - i.e. used a Ventana table rather than a VentanaCondOps one. In theory, we could have a vendor who ships incompatible extensions (in two different pieces of hardware), but if we hit that case, we can adjust later. Or at least, I think we can? This is new to me.

[reames]

Rebase

[kito-cheng]

llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp

469 ↗

(On Diff #473285)

Typo, should be "Trying Ventana..."

llvm/lib/Target/RISCV/RISCVInstrInfo.td

1785

I would suggest put into RISCVInstInfoXVentana.td instead of inline here, I could imagine once this get merged, T-head extensions might try to upstream too, so put into separated now could prevent they reference this commit and try to inline their extensions IMO :)

[reames]

Address Kito's review comments

[craig.topper]

llvm/lib/Target/RISCV/RISCVInstrInfoXVentana.td

14 ↗

(On Diff #474620)

Does the - here mean something?

19 ↗

(On Diff #474620)

I don't think we need to this applied to the class and the instantatiations of the class. I think you can drop the curly braces and only apply it to the class.

24 ↗

(On Diff #474620)

space between > and {

[reames]

Address comments from @craig.topper

[craig.topper]

LGTM

clang/test/Preprocessor/riscv-target-features.c

 // CHECK-NOT: __riscv_zksed
 // CHECK-NOT: __riscv_zkr
 // CHECK-NOT: __riscv_zkt
 // CHECK-NOT: __riscv_zk
 // CHECK-NOT: __riscv_zicbom
 // CHECK-NOT: __riscv_zicboz
 // CHECK-NOT: __riscv_svnapot
 // CHECK-NOT: __riscv_svinval
+// CHECK-NOT: __riscv_xventanacondops
 
 // RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32im -x c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-M-EXT %s
 // RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64im -x c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-M-EXT %s
 // CHECK-M-EXT: __riscv_div 1
 // CHECK-M-EXT: __riscv_m 2000000{{$}}
 // CHECK-M-EXT: __riscv_mul 1

 // RUN: -o - | FileCheck --check-prefix=CHECK-SVNAPOT-EXT %s
 // CHECK-SVNAPOT-EXT: __riscv_svnapot 1000000{{$}}
 
 // RUN: %clang -target riscv32 -march=rv32isvinval -x c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-SVINVAL-EXT %s
 // RUN: %clang -target riscv64 -march=rv64isvinval -x c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-SVINVAL-EXT %s
 // CHECK-SVINVAL-EXT: __riscv_svinval 1000000{{$}}
+
+// RUN: %clang -target riscv64 -march=rv64ixventanacondops -x c -E -dM %s \
+// RUN: -o - | FileCheck --check-prefix=CHECK-XVENTANACONDOPS-EXT %s
+// CHECK-XVENTANACONDOPS-EXT: __riscv_xventanacondops 1000000{{$}}

llvm/docs/RISCVUsage.rst

 ``experimental-zvfh``
   LLVM implements `this draft text <https://github.com/riscv/riscv-v-spec/pull/780>`_.
 
 To use an experimental extension from `clang`, you must add `-menable-experimental-extensions` to the command line, and specify the exact version of the experimental extension you are using.  To use an experimental extension with LLVM's internal developer tools (e.g. `llc`, `llvm-objdump`, `llvm-mc`), you must prefix the extension name with `experimental-`.  Note that you don't need to specify the version with internal tools, and shouldn't include the `experimental-` prefix with `clang`.
 
 Vendor Extensions
 =================
 
-Vendor extensions are extensions which are not standardized by RISC-V International, and are instead defined by a hardware vendor.  At the moment, LLVM does not support any vendor extensions for RISC-V, but we expect this to change in the future.
-
-The term vendor extension roughly parallels the definition of a `non-standard` extension from Section 1.3 of the Volume I: RISC-V Unprivileged ISA specification.  In particular, we expect to eventually accept both `custom` extensions and `non-conforming` extensions.
+Vendor extensions are extensions which are not standardized by RISC-V International, and are instead defined by a hardware vendor.  The term vendor extension roughly parallels the definition of a `non-standard` extension from Section 1.3 of the Volume I: RISC-V Unprivileged ISA specification.  In particular, we expect to eventually accept both `custom` extensions and `non-conforming` extensions.
 
 Inclusion of a vendor extension will be considered on a case by case basis.  All proposals should be brought to the bi-weekly RISCV sync calls for discussion.  For a general idea of the factors likely to be considered, please see the `Clang documentation <https://clang.llvm.org/get_involved.html>`_.
 
 It is our intention to follow the naming conventions described in `riscv-non-isa/riscv-toolchain-conventions <https://github.com/riscv-non-isa/riscv-toolchain-conventions#conventions-for-vendor-extensions>`_.  Exceptions to this naming will need to be strongly motivated.
 
+The current vendor extensions supported are:
+
+``XVentanaCondOps``
+  LLVM implements `version 1.0.0 of the VTx-family custom instructions specification <https://github.com/ventanamicro/ventana-custom-extensions/releases/download/v1.0.0/ventana-custom-extensions-v1.0.0.pdf>`_ by Ventana Micro Systems.  All instructions are prefixed with `vt.` as described in the specification, and the riscv-toolchai-convention document linked above.  These instructions are only available for riscv64 at this time.
+
 
 Specification Documents
 =======================
 For ratified specifications, please refer to the `official RISC-V International
 page <https://riscv.org/technical/specifications/>`_.  Make sure to check the
 `wiki for not yet integrated extensions
 <https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions>`_.
 

llvm/lib/Support/RISCVISAInfo.cpp

static const RISCVSupportedExtension SupportedExtensions[] = {
     {"zve64d", RISCVExtensionVersion{1, 0}},
 
     {"zicbom", RISCVExtensionVersion{1, 0}},
     {"zicboz", RISCVExtensionVersion{1, 0}},
     {"zicbop", RISCVExtensionVersion{1, 0}},
 
     {"svnapot", RISCVExtensionVersion{1, 0}},
     {"svinval", RISCVExtensionVersion{1, 0}},
+    {"xventanacondops", RISCVExtensionVersion{1, 0}},
 };
 
 static const RISCVSupportedExtension SupportedExperimentalExtensions[] = {
     {"zihintntl", RISCVExtensionVersion{0, 2}},
 
     {"zca", RISCVExtensionVersion{0, 70}},
     {"zvfh", RISCVExtensionVersion{0, 1}},
     {"zawrs", RISCVExtensionVersion{1, 0}},

llvm/lib/Target/RISCV/RISCV.td

 
 def FeatureStdExtZawrs
     : SubtargetFeature<"experimental-zawrs", "HasStdExtZawrs", "true",
                        "'Zawrs' (Wait on Reservation Set)">;
 def HasStdExtZawrs : Predicate<"Subtarget->hasStdExtZawrs()">,
                                AssemblerPredicate<(all_of FeatureStdExtZawrs),
                                "'Zawrs' (Wait on Reservation Set)">;
 
+//===----------------------------------------------------------------------===//
+// Vendor extensions
+//===----------------------------------------------------------------------===//
+
+def FeatureVendorXVentanaCondOps
+    : SubtargetFeature<"xventanacondops", "HasVendorXVentanaCondOps", "true",
+                       "'XVentanaCondOps' (Ventana Conditional Ops)">;
+def HasVendorXVentanaCondOps : Predicate<"Subtarget->hasVendorXVentanaCondOps()">,
+                                AssemblerPredicate<(all_of FeatureVendorXVentanaCondOps),
+                                "'XVentanaCondOps' (Ventana Conditional Ops)">;
+
+//===----------------------------------------------------------------------===//
+// LLVM specific features and extensions
+//===----------------------------------------------------------------------===//
+
 // Feature32Bit exists to mark CPUs that support RV32 to distinquish them from
 // tuning CPU names.
 def Feature32Bit
     : SubtargetFeature<"32bit", "HasRV32", "true", "Implements RV32">;
 def Feature64Bit
     : SubtargetFeature<"64bit", "HasRV64", "true", "Implements RV64">;
 def IsRV64 : Predicate<"Subtarget->is64Bit()">,
                        AssemblerPredicate<(all_of Feature64Bit),

llvm/lib/Target/RISCV/RISCVInstrFormats.td

 def OPC_NMSUB     : RISCVOpcode<"NMSUB",     0b1001011>;
 def OPC_NMADD     : RISCVOpcode<"NMADD",     0b1001111>;
 def OPC_OP_FP     : RISCVOpcode<"OP_FP",     0b1010011>;
 def OPC_OP_V      : RISCVOpcode<"OP_V",      0b1010111>;
 def OPC_BRANCH    : RISCVOpcode<"BRANCH",    0b1100011>;
 def OPC_JALR      : RISCVOpcode<"JALR",      0b1100111>;
 def OPC_JAL       : RISCVOpcode<"JAL",       0b1101111>;
 def OPC_SYSTEM    : RISCVOpcode<"SYSTEM",    0b1110011>;
+def OPC_CUSTOM3   : RISCVOpcode<"CUSTOM3",   0b1111011>;
 
 class RVInst<dag outs, dag ins, string opcodestr, string argstr,
              list<dag> pattern, InstFormat format>
     : Instruction {
   field bits<32> Inst;
   // SoftFail is a field the disassembler can use to provide a way for
   // instructions to not match without killing the whole decode process. It is
   // mainly used for ARM, but Tablegen expects this field to exist or it fails

llvm/lib/Target/RISCV/RISCVInstrInfo.td

 include "RISCVInstrInfoF.td"
 include "RISCVInstrInfoD.td"
 include "RISCVInstrInfoC.td"
 include "RISCVInstrInfoZb.td"
 include "RISCVInstrInfoZk.td"
 include "RISCVInstrInfoV.td"
 include "RISCVInstrInfoZfh.td"
 include "RISCVInstrInfoZicbo.td"
+
+//===----------------------------------------------------------------------===//
+// Vendor extensions
+//===----------------------------------------------------------------------===//
+
+// -XVentanaCondOps
+let Predicates = [IsRV64, HasVendorXVentanaCondOps], hasSideEffects = 0,
+  mayLoad = 0, mayStore = 0, isCodeGenOnly = 0 in {
+
+class VTMaskedMove<bits<3> funct3, string opcodestr>
+    : RVInstR<0b0000000, funct3, OPC_CUSTOM3, (outs GPR:$rd),
+              (ins GPR:$rs1, GPR:$rs2), opcodestr,
+              "$rd, $rs1, $rs2">{
+}
+
+def VT_MASKC : VTMaskedMove<0b110, "vt.maskc">,
+           Sched<[WriteIALU, ReadIALU, ReadIALU]>;
+
+def VT_MASKCN : VTMaskedMove<0b111, "vt.maskcn">,
+           Sched<[WriteIALU, ReadIALU, ReadIALU]>;
+}

llvm/lib/Target/RISCV/RISCVSubtarget.h

private:
   bool HasStdExtZicbom = false;
   bool HasStdExtZicboz = false;
   bool HasStdExtZicbop = false;
   bool HasStdExtSvnapot = false;
   bool HasStdExtSvinval = false;
   bool HasStdExtZmmul = false;
   bool HasStdExtZawrs = false;
   bool HasStdExtZtso = false;
+  bool HasVendorXVentanaCondOps = false;
   bool HasRV32 = false;
   bool HasRV64 = false;
   bool IsRV32E = false;
   bool EnableLinkerRelax = false;
   bool EnableRVCHintInstrs = true;
   bool EnableDefaultUnroll = true;
   bool EnableSaveRestore = false;
   bool EnableUnalignedScalarMem = false;
public:
   bool hasStdExtZicbom() const { return HasStdExtZicbom; }
   bool hasStdExtZicboz() const { return HasStdExtZicboz; }
   bool hasStdExtZicbop() const { return HasStdExtZicbop; }
   bool hasStdExtSvnapot() const { return HasStdExtSvnapot; }
   bool hasStdExtSvinval() const { return HasStdExtSvinval; }
   bool hasStdExtZawrs() const { return HasStdExtZawrs; }
   bool hasStdExtZmmul() const { return HasStdExtZmmul; }
   bool hasStdExtZtso() const { return HasStdExtZtso; }
+  bool hasVendorXVentanaCondOps() const { return HasVendorXVentanaCondOps; }
   bool is64Bit() const { return HasRV64; }
   bool isRV32E() const { return IsRV32E; }
   bool enableLinkerRelax() const { return EnableLinkerRelax; }
   bool enableRVCHintInstrs() const { return EnableRVCHintInstrs; }
   bool enableDefaultUnroll() const { return EnableDefaultUnroll; }
   bool enableSaveRestore() const { return EnableSaveRestore; }
   bool hasShortForwardBranchOpt() const { return HasShortForwardBranchOpt; }
   bool enableUnalignedScalarMem() const { return EnableUnalignedScalarMem; }

llvm/test/CodeGen/RISCV/attributes.ll

 ; RUN: llc -mtriple=riscv64 -mattr=+zkn,+zkr,+zkt %s -o - | FileCheck --check-prefix=RV64COMBINEINTOZK %s
 ; RUN: llc -mtriple=riscv64 -mattr=+zbkb,+zbkc,+zbkx,+zkne,+zknd,+zknh %s -o - | FileCheck --check-prefix=RV64COMBINEINTOZKN %s
 ; RUN: llc -mtriple=riscv64 -mattr=+zbkb,+zbkc,+zbkx,+zksed,+zksh %s -o - | FileCheck --check-prefix=RV64COMBINEINTOZKS %s
 ; RUN: llc -mtriple=riscv64 -mattr=+zicbom %s -o - | FileCheck --check-prefix=RV64ZICBOM %s
 ; RUN: llc -mtriple=riscv64 -mattr=+zicboz %s -o - | FileCheck --check-prefix=RV64ZICBOZ %s
 ; RUN: llc -mtriple=riscv64 -mattr=+zicbop %s -o - | FileCheck --check-prefix=RV64ZICBOP %s
 ; RUN: llc -mtriple=riscv64 -mattr=+svnapot %s -o - | FileCheck --check-prefix=RV64SVNAPOT %s
 ; RUN: llc -mtriple=riscv64 -mattr=+svinval %s -o - | FileCheck --check-prefix=RV64SVINVAL %s
+; RUN: llc -mtriple=riscv64 -mattr=+xventanacondops %s -o - | FileCheck --check-prefix=RV64XVENTANACONDOPS %s
 ; RUN: llc -mtriple=riscv64 -mattr=+experimental-zawrs %s -o - | FileCheck --check-prefix=RV64ZAWRS %s
 ; RUN: llc -mtriple=riscv64 -mattr=+experimental-ztso %s -o - | FileCheck --check-prefix=RV64ZTSO %s
 
 ; RV32M: .attribute 5, "rv32i2p0_m2p0"
 ; RV32ZMMUL: .attribute 5, "rv32i2p0_zmmul1p0"
 ; RV32MZMMUL: .attribute 5, "rv32i2p0_m2p0_zmmul1p0"
 ; RV32A: .attribute 5, "rv32i2p0_a2p0"
 ; RV32F: .attribute 5, "rv32i2p0_f2p0"

 ; RV64COMBINEINTOZKN: .attribute 5, "rv64i2p0_zbkb1p0_zbkc1p0_zbkx1p0_zkn1p0_zknd1p0_zkne1p0_zknh1p0"
 ; RV64COMBINEINTOZKS: .attribute 5, "rv64i2p0_zbkb1p0_zbkc1p0_zbkx1p0_zks1p0_zksed1p0_zksh1p0"
 ; RV64ZICBOM: .attribute 5, "rv64i2p0_zicbom1p0"
 ; RV64ZICBOZ: .attribute 5, "rv64i2p0_zicboz1p0"
 ; RV64ZAWRS: .attribute 5, "rv64i2p0_zawrs1p0"
 ; RV64ZICBOP: .attribute 5, "rv64i2p0_zicbop1p0"
 ; RV64SVNAPOT: .attribute 5, "rv64i2p0_svnapot1p0"
 ; RV64SVINVAL: .attribute 5, "rv64i2p0_svinval1p0"
+; RV64XVENTANACONDOPS: .attribute 5, "rv64i2p0_xventanacondops1p0"
 ; RV64ZTSO: .attribute 5, "rv64i2p0_ztso0p1"
 
 define i32 @addi(i32 %a) {
   %1 = add i32 %a, 1
   ret i32 %1
 }

llvm/test/MC/RISCV/XVentanaCondOps-valid.s

+# RUN: llvm-mc %s -triple=riscv64 -mattr=+xventanacondops -riscv-no-aliases -show-encoding \
+# RUN:     | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+xventanacondops < %s \
+# RUN:     | llvm-objdump --mattr=+xventanacondops -M no-aliases -d -r - \
+# RUN:     | FileCheck --check-prefix=CHECK-ASM-AND-OBJ %s
+
+# CHECK-ASM-AND-OBJ: vt.maskc zero, zero, zero
+# CHECK-ASM: encoding: [0x7b,0x60,0x00,0x00]
+vt.maskc x0, x0, x0
+
+# CHECK-ASM-AND-OBJ: vt.maskcn zero, zero, zero
+# CHECK-ASM: encoding: [0x7b,0x70,0x00,0x00]
+vt.maskcn x0, x0, x0
+
+# CHECK-ASM-AND-OBJ: vt.maskc ra, sp, gp
+# CHECK-ASM: encoding: [0xfb,0x60,0x31,0x00]
+vt.maskc x1, x2, x3
+
+# CHECK-ASM-AND-OBJ: vt.maskcn ra, sp, gp
+# CHECK-ASM: encoding: [0xfb,0x70,0x31,0x00]
+vt.maskcn x1, x2, x3
+