[RISCV] Assembler support for XRivosVizip #127694
Conversation
This implements assembler support for the XRivosVizip custom/vendor extension from Rivos Inc. which is defined in: https://github.com/rivosinc/rivos-custom-extensions (See src/xrivosvizip.adoc) Codegen support will follow in a separate change.
llvmbot
added
clang
|
@llvm/pr-subscribers-backend-risc-v @llvm/pr-subscribers-clang Author: Philip Reames (preames) ChangesThis implements assembler support for the XRivosVizip custom/vendor extension from Rivos Inc. which is defined in: Codegen support will follow in a separate change. Full diff: https://github.com/llvm/llvm-project/pull/127694.diff 7 Files Affected:
diff --git a/clang/test/Driver/print-supported-extensions-riscv.c b/clang/test/Driver/print-supported-extensions-riscv.c
index 3443ff0b69de9..57b6a46677591 100644
--- a/clang/test/Driver/print-supported-extensions-riscv.c
+++ b/clang/test/Driver/print-supported-extensions-riscv.c
@@ -202,6 +202,7 @@
// CHECK-NEXT: xqcilo 0.2 'Xqcilo' (Qualcomm uC Large Offset Load Store Extension)
// CHECK-NEXT: xqcilsm 0.2 'Xqcilsm' (Qualcomm uC Load Store Multiple Extension)
// CHECK-NEXT: xqcisls 0.2 'Xqcisls' (Qualcomm uC Scaled Load Store Extension)
+// CHECK-NEXT: xrivosvizip 0.1 'XRivosVizip' (Rivos Vector Register Zips)
// CHECK-EMPTY:
// CHECK-NEXT: Supported Profiles
// CHECK-NEXT: rva20s64
diff --git a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
index 01648ec0cbe9e..aef470b331cf1 100644
--- a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
+++ b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
@@ -721,6 +721,8 @@ DecodeStatus RISCVDisassembler::getInstruction32(MCInst &MI, uint64_t &Size,
"Qualcomm uC Conditional Move custom opcode table");
TRY_TO_DECODE_FEATURE(RISCV::FeatureVendorXqciint, DecoderTableXqciint32,
"Qualcomm uC Interrupts custom opcode table");
+ TRY_TO_DECODE_FEATURE(RISCV::FeatureVendorXRivosVizip, DecoderTableXRivos32,
+ "Rivos custom opcode table");
TRY_TO_DECODE(true, DecoderTable32, "RISCV32 table");
return MCDisassembler::Fail;
diff --git a/llvm/lib/Target/RISCV/RISCVFeatures.td b/llvm/lib/Target/RISCV/RISCVFeatures.td
index 51aa8d7d307e4..8380d4474d6da 100644
--- a/llvm/lib/Target/RISCV/RISCVFeatures.td
+++ b/llvm/lib/Target/RISCV/RISCVFeatures.td
@@ -1333,6 +1333,15 @@ def HasVendorXqcilo
AssemblerPredicate<(all_of FeatureVendorXqcilo),
"'Xqcilo' (Qualcomm uC Large Offset Load Store Extension)">;
+// Rivos Extension(s)
+
+def FeatureVendorXRivosVizip
+ : RISCVExperimentalExtension<0, 1, "Rivos Vector Register Zips">;
+def HasVendorXRivosVizip
+ : Predicate<"Subtarget->hasVendorXRivosVizip()">,
+ AssemblerPredicate<(all_of FeatureVendorXRivosVizip),
+ "'XRivosVizip' (Rivos Vector Register Zips)">;
+
//===----------------------------------------------------------------------===//
// LLVM specific features and extensions
//===----------------------------------------------------------------------===//
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.td b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
index fde7dc89dd693..a962e64581797 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
@@ -2148,6 +2148,7 @@ include "RISCVInstrInfoXCV.td"
include "RISCVInstrInfoXwch.td"
include "RISCVInstrInfoXqci.td"
include "RISCVInstrInfoXMips.td"
+include "RISCVInstrInfoXRivos.td"
//===----------------------------------------------------------------------===//
// Global ISel
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoXRivos.td b/llvm/lib/Target/RISCV/RISCVInstrInfoXRivos.td
new file mode 100644
index 0000000000000..055fdfe5c1f56
--- /dev/null
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoXRivos.td
@@ -0,0 +1,27 @@
+//===-- RISCVInstrInfoXRivos.td --------------------------*- tablegen -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file describes the vendor extensions defined by Rivos Inc.
+//
+//===----------------------------------------------------------------------===//
+
+//===----------------------------------------------------------------------===//
+// XRivosVizip
+//===----------------------------------------------------------------------===//
+
+
+let Predicates = [HasVendorXRivosVizip], hasSideEffects = 0,
+ mayLoad = 0, mayStore = 0, isCodeGenOnly = 0, DecoderNamespace = "XRivos",
+ Inst<6-0> = OPC_CUSTOM_2.Value in {
+defm RV_VZIPEVEN_V : VALU_IV_V<"rv.vzipeven", 0b001100>;
+defm RV_VZIPODD_V : VALU_IV_V<"rv.vzipodd", 0b011100>;
+defm RV_VZIP2A_V : VALU_IV_V<"rv.vzip2a", 0b000100>;
+defm RV_VZIP2B_V : VALU_IV_V<"rv.vzip2b", 0b010100>;
+defm RV_VUNZIP2A_V : VALU_IV_V<"rv.vunzip2a", 0b001000>;
+defm RV_VUNZIP2B_V : VALU_IV_V<"rv.vunzip2b", 0b011000>;
+}
diff --git a/llvm/test/MC/RISCV/xrivosvizip-valid.s b/llvm/test/MC/RISCV/xrivosvizip-valid.s
new file mode 100644
index 0000000000000..ae339fbfca57d
--- /dev/null
+++ b/llvm/test/MC/RISCV/xrivosvizip-valid.s
@@ -0,0 +1,30 @@
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+experimental-xrivosvizip -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=+experimental-xrivosvizip < %s \
+# RUN: | llvm-objdump --mattr=+experimental-xrivosvizip -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefix=CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+experimental-xrivosvizip -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+experimental-xrivosvizip < %s \
+# RUN: | llvm-objdump --mattr=+experimental-xrivosvizip -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefix=CHECK-ASM-AND-OBJ %s
+
+# CHECK-ASM-AND-OBJ: rv.vzipeven.vv v1, v2, v3
+# CHECK-ASM: encoding: [0xdb,0x80,0x21,0x32]
+rv.vzipeven.vv v1, v2, v3
+# CHECK-ASM-AND-OBJ: rv.vzipodd.vv v1, v2, v3
+# CHECK-ASM: encoding: [0xdb,0x80,0x21,0x72]
+rv.vzipodd.vv v1, v2, v3
+# CHECK-ASM-AND-OBJ: rv.vzip2a.vv v1, v2, v3
+# CHECK-ASM: encoding: [0xdb,0x80,0x21,0x12]
+rv.vzip2a.vv v1, v2, v3
+# CHECK-ASM-AND-OBJ: rv.vzip2b.vv v1, v2, v3
+# CHECK-ASM: encoding: [0xdb,0x80,0x21,0x52]
+rv.vzip2b.vv v1, v2, v3
+# CHECK-ASM-AND-OBJ: rv.vunzip2a.vv v1, v2, v3
+# CHECK-ASM: encoding: [0xdb,0x80,0x21,0x22]
+rv.vunzip2a.vv v1, v2, v3
+# CHECK-ASM-AND-OBJ: rv.vunzip2b.vv v1, v2, v3
+# CHECK-ASM: encoding: [0xdb,0x80,0x21,0x62]
+rv.vunzip2b.vv v1, v2, v3
+
diff --git a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
index 7ebfcf915a7c5..44b04dd0afe0f 100644
--- a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
@@ -1128,6 +1128,7 @@ Experimental extensions
xqcilo 0.2
xqcilsm 0.2
xqcisls 0.2
+ xrivosvizip 0.1
Supported Profiles
rva20s64
|
|
@llvm/pr-subscribers-clang-driver Author: Philip Reames (preames) ChangesThis implements assembler support for the XRivosVizip custom/vendor extension from Rivos Inc. which is defined in: Codegen support will follow in a separate change. Full diff: https://github.com/llvm/llvm-project/pull/127694.diff 7 Files Affected:
diff --git a/clang/test/Driver/print-supported-extensions-riscv.c b/clang/test/Driver/print-supported-extensions-riscv.c
index 3443ff0b69de9..57b6a46677591 100644
--- a/clang/test/Driver/print-supported-extensions-riscv.c
+++ b/clang/test/Driver/print-supported-extensions-riscv.c
@@ -202,6 +202,7 @@
// CHECK-NEXT: xqcilo 0.2 'Xqcilo' (Qualcomm uC Large Offset Load Store Extension)
// CHECK-NEXT: xqcilsm 0.2 'Xqcilsm' (Qualcomm uC Load Store Multiple Extension)
// CHECK-NEXT: xqcisls 0.2 'Xqcisls' (Qualcomm uC Scaled Load Store Extension)
+// CHECK-NEXT: xrivosvizip 0.1 'XRivosVizip' (Rivos Vector Register Zips)
// CHECK-EMPTY:
// CHECK-NEXT: Supported Profiles
// CHECK-NEXT: rva20s64
diff --git a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
index 01648ec0cbe9e..aef470b331cf1 100644
--- a/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
+++ b/llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
@@ -721,6 +721,8 @@ DecodeStatus RISCVDisassembler::getInstruction32(MCInst &MI, uint64_t &Size,
"Qualcomm uC Conditional Move custom opcode table");
TRY_TO_DECODE_FEATURE(RISCV::FeatureVendorXqciint, DecoderTableXqciint32,
"Qualcomm uC Interrupts custom opcode table");
+ TRY_TO_DECODE_FEATURE(RISCV::FeatureVendorXRivosVizip, DecoderTableXRivos32,
+ "Rivos custom opcode table");
TRY_TO_DECODE(true, DecoderTable32, "RISCV32 table");
return MCDisassembler::Fail;
diff --git a/llvm/lib/Target/RISCV/RISCVFeatures.td b/llvm/lib/Target/RISCV/RISCVFeatures.td
index 51aa8d7d307e4..8380d4474d6da 100644
--- a/llvm/lib/Target/RISCV/RISCVFeatures.td
+++ b/llvm/lib/Target/RISCV/RISCVFeatures.td
@@ -1333,6 +1333,15 @@ def HasVendorXqcilo
AssemblerPredicate<(all_of FeatureVendorXqcilo),
"'Xqcilo' (Qualcomm uC Large Offset Load Store Extension)">;
+// Rivos Extension(s)
+
+def FeatureVendorXRivosVizip
+ : RISCVExperimentalExtension<0, 1, "Rivos Vector Register Zips">;
+def HasVendorXRivosVizip
+ : Predicate<"Subtarget->hasVendorXRivosVizip()">,
+ AssemblerPredicate<(all_of FeatureVendorXRivosVizip),
+ "'XRivosVizip' (Rivos Vector Register Zips)">;
+
//===----------------------------------------------------------------------===//
// LLVM specific features and extensions
//===----------------------------------------------------------------------===//
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.td b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
index fde7dc89dd693..a962e64581797 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
@@ -2148,6 +2148,7 @@ include "RISCVInstrInfoXCV.td"
include "RISCVInstrInfoXwch.td"
include "RISCVInstrInfoXqci.td"
include "RISCVInstrInfoXMips.td"
+include "RISCVInstrInfoXRivos.td"
//===----------------------------------------------------------------------===//
// Global ISel
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoXRivos.td b/llvm/lib/Target/RISCV/RISCVInstrInfoXRivos.td
new file mode 100644
index 0000000000000..055fdfe5c1f56
--- /dev/null
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoXRivos.td
@@ -0,0 +1,27 @@
+//===-- RISCVInstrInfoXRivos.td --------------------------*- tablegen -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file describes the vendor extensions defined by Rivos Inc.
+//
+//===----------------------------------------------------------------------===//
+
+//===----------------------------------------------------------------------===//
+// XRivosVizip
+//===----------------------------------------------------------------------===//
+
+
+let Predicates = [HasVendorXRivosVizip], hasSideEffects = 0,
+ mayLoad = 0, mayStore = 0, isCodeGenOnly = 0, DecoderNamespace = "XRivos",
+ Inst<6-0> = OPC_CUSTOM_2.Value in {
+defm RV_VZIPEVEN_V : VALU_IV_V<"rv.vzipeven", 0b001100>;
+defm RV_VZIPODD_V : VALU_IV_V<"rv.vzipodd", 0b011100>;
+defm RV_VZIP2A_V : VALU_IV_V<"rv.vzip2a", 0b000100>;
+defm RV_VZIP2B_V : VALU_IV_V<"rv.vzip2b", 0b010100>;
+defm RV_VUNZIP2A_V : VALU_IV_V<"rv.vunzip2a", 0b001000>;
+defm RV_VUNZIP2B_V : VALU_IV_V<"rv.vunzip2b", 0b011000>;
+}
diff --git a/llvm/test/MC/RISCV/xrivosvizip-valid.s b/llvm/test/MC/RISCV/xrivosvizip-valid.s
new file mode 100644
index 0000000000000..ae339fbfca57d
--- /dev/null
+++ b/llvm/test/MC/RISCV/xrivosvizip-valid.s
@@ -0,0 +1,30 @@
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+experimental-xrivosvizip -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=+experimental-xrivosvizip < %s \
+# RUN: | llvm-objdump --mattr=+experimental-xrivosvizip -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefix=CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+experimental-xrivosvizip -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+experimental-xrivosvizip < %s \
+# RUN: | llvm-objdump --mattr=+experimental-xrivosvizip -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefix=CHECK-ASM-AND-OBJ %s
+
+# CHECK-ASM-AND-OBJ: rv.vzipeven.vv v1, v2, v3
+# CHECK-ASM: encoding: [0xdb,0x80,0x21,0x32]
+rv.vzipeven.vv v1, v2, v3
+# CHECK-ASM-AND-OBJ: rv.vzipodd.vv v1, v2, v3
+# CHECK-ASM: encoding: [0xdb,0x80,0x21,0x72]
+rv.vzipodd.vv v1, v2, v3
+# CHECK-ASM-AND-OBJ: rv.vzip2a.vv v1, v2, v3
+# CHECK-ASM: encoding: [0xdb,0x80,0x21,0x12]
+rv.vzip2a.vv v1, v2, v3
+# CHECK-ASM-AND-OBJ: rv.vzip2b.vv v1, v2, v3
+# CHECK-ASM: encoding: [0xdb,0x80,0x21,0x52]
+rv.vzip2b.vv v1, v2, v3
+# CHECK-ASM-AND-OBJ: rv.vunzip2a.vv v1, v2, v3
+# CHECK-ASM: encoding: [0xdb,0x80,0x21,0x22]
+rv.vunzip2a.vv v1, v2, v3
+# CHECK-ASM-AND-OBJ: rv.vunzip2b.vv v1, v2, v3
+# CHECK-ASM: encoding: [0xdb,0x80,0x21,0x62]
+rv.vunzip2b.vv v1, v2, v3
+
diff --git a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
index 7ebfcf915a7c5..44b04dd0afe0f 100644
--- a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
@@ -1128,6 +1128,7 @@ Experimental extensions
xqcilo 0.2
xqcilsm 0.2
xqcisls 0.2
+ xrivosvizip 0.1
Supported Profiles
rva20s64
|
preames
left a comment
preames
left a comment
There was a problem hiding this comment.
(inline comments on possible discussion points)
| "Qualcomm uC Conditional Move custom opcode table"); | ||
| TRY_TO_DECODE_FEATURE(RISCV::FeatureVendorXqciint, DecoderTableXqciint32, | ||
| "Qualcomm uC Interrupts custom opcode table"); | ||
| TRY_TO_DECODE_FEATURE(RISCV::FeatureVendorXRivosVizip, DecoderTableXRivos32, |
There was a problem hiding this comment.
I noticed that many of the existing extensions have one decode table per extension. I thought the idea was we had one decode table per vendor unless and until that vendor had an encoding conflict in their own extension sets?
There was a problem hiding this comment.
I'm glad you've pointed this out, as it did seem like we are trying to side-step the encoding conflict checking that LLVM is capable of, as well as the fact that the decode tables will also check specific predicates for specific instructions.
I will look at merging all the Qualcomm decode tables, given that all the Xqci instructions are intended not to conflict with each other's encodings. I am less clear on which other vendor extensions should or should not conflict with each other.
I think the TRY_TO_DECODE_FEATURE(... encouraged one decode table per feature - it's relatively more difficult to write an equivalent for when you have any of a few different features - but we can solve this with yet more macros of course.
There was a problem hiding this comment.
JFYI, repeating the existing macro per extension with the same table name works just fine. You could also do other structures, but the direct migration is pretty easy - just change the table names.
| // CHECK-NEXT: xqcilo 0.2 'Xqcilo' (Qualcomm uC Large Offset Load Store Extension) | ||
| // CHECK-NEXT: xqcilsm 0.2 'Xqcilsm' (Qualcomm uC Load Store Multiple Extension) | ||
| // CHECK-NEXT: xqcisls 0.2 'Xqcisls' (Qualcomm uC Scaled Load Store Extension) | ||
| // CHECK-NEXT: xrivosvizip 0.1 'XRivosVizip' (Rivos Vector Register Zips) |
There was a problem hiding this comment.
We seem to have a split in our extension naming. Some use the vendor name (e.g. Ventana, Thead), others use the vendor prefix (e.g. "qc" for qualcomm). Ours happens to match the vendor name convention - mostly as a case of historical accident - which happens to reduce possible confusion from the "rv." prefix. (And also explains why we were somewhat talking past each other on that point. I wasn't seeing the confusion because the prefix was only part of the instruction name for me, not part of the name of the extension.)
Do we care about this split at all?
There was a problem hiding this comment.
My understanding is what you reserved in the toolchains convention document was both extensions starting xrv and also that all such instructions in those extensions would be prefixed rv.. As you note, this is what Qualcomm, SiFive and the OpenHW Group have followed. Arguably, T-Head also follow it, by virtue of their XThead* extensions all starting with Xth, but that might be coincidence more than intention.
I think it is valuable to keep that alignment for newly added vendors -- maybe we need an update to the wording of the guidance in the toolchain document?
There was a problem hiding this comment.
As noted in my original comment, there is no existing consistency. Ventana and THead both use vendor name in the extension name, not the instruction mnemonic prefix. Examples:
$ fgrep ventana test/Driver/print-supported-extensions-riscv.c
// CHECK-NEXT: xventanacondops 1.0 'XVentanaCondOps' (Ventana Conditional Ops)
$ fgrep thead /test/Driver/print-supported-extensions-riscv.c
// CHECK-NEXT: xtheadba 1.0 'XTHeadBa' (T-Head address calculation instructions)
// CHECK-NEXT: xtheadbb 1.0 'XTHeadBb' (T-Head basic bit-manipulation instructions)
// CHECK-NEXT: xtheadbs 1.0 'XTHeadBs' (T-Head single-bit instructions)
// CHECK-NEXT: xtheadcmo 1.0 'XTHeadCmo' (T-Head cache management instructions)
// CHECK-NEXT: xtheadcondmov 1.0 'XTHeadCondMov' (T-Head conditional move instructions)
// CHECK-NEXT: xtheadfmemidx 1.0 'XTHeadFMemIdx' (T-Head FP Indexed Memory Operations)
// CHECK-NEXT: xtheadmac 1.0 'XTHeadMac' (T-Head Multiply-Accumulate Instructions)
// CHECK-NEXT: xtheadmemidx 1.0 'XTHeadMemIdx' (T-Head Indexed Memory Operations)
// CHECK-NEXT: xtheadmempair 1.0 'XTHeadMemPair' (T-Head two-GPR Memory Operations)
// CHECK-NEXT: xtheadsync 1.0 'XTHeadSync' (T-Head multicore synchronization instructions)
// CHECK-NEXT: xtheadvdot 1.0 'XTHeadVdot' (T-Head Vector Extensions for Dot)
There was a problem hiding this comment.
Both are acceptable:
To make it easier to identify and prevent naming conflict, vendor extensions should start with a vendor name, which could be an abbreviation of the full name.
|
I am kind of confused now. So the situation here is that RVIOS has already implemented these vendor extensions in cores and RVIOS is also trying to make these extensions official RVI standards, right? |
You're confusing two things. We (Rivos) have defined a set of custom vendor extensions - this patch is the first in that sub-series. We are also and separately proposing a subset of instructions for consideration at RVI for eventual standardization. As an example of how these are different, see XRivosVisni in the linked repo. That's a vendor extension which relates to gaps identified in RVV, but for which we're not currently proposing an extension at RVI. |
Thanks, I'd like to know if these vendor extensions have been implemented in some type-out-ed cores. I have this question because I saw that these vendor extensions are of version 0.1. If they are going to be changeable (and experimental), then why are we supporting XRivosVizip extension in a hurry since XRivosVizip and Zvzip are overlapped. |
In general, vendor extensions remain in draft (i.e. experimental), until hardware is announced. I can not comment on any hardware plans, or timelines. As the lead of the Zvzip work, I can say that I believe having a vendor extension with in tree support will make it easier for others to evaluate the zvzip proposal. In particular, the current assignment of the zvzip opcodes in custom_2 is about to be removed as that's really not appropriate for an RVI proposal. Given that, landing anything for zvzip is pretty much a non-starter. However, this is only vaguely related to this patch. Also, I disagree with your characterization of anything being done here as "in a hurry". All normal review and discussion timelines have been, and will continue to be, followed. |
|
|
||
|
|
||
| let Predicates = [HasVendorXRivosVizip], DecoderNamespace = "XRivos", | ||
| Constraints = "@earlyclobber $vd", RVVConstraint = Vrgather, |
There was a problem hiding this comment.
Two comments on this line.
-
I'm copying the existing structure for the earlycobber, but as far as I can tell, this has no effect. This has to be placed on the pseudo to have any impact on codegen, and has no relevance for the MC layer.
-
The use of Vrgather for the RVVConstraint type translates as an overlap check for vs2, vs1, and mask register (if any). I had to fix an ambiguity in the specification document to make it clear overlap with the mask register was disallowed. The use of instruction names as keys appears to imply that there's more meaning beyond the implied registers to check, but the code doesn't actually seem to reflect that. I could add a VZip constraint type, but I'd rather move forward with this and then restructure this enum in terms of the registers being checked.
@topperc Could you double check to make sure this still holds? I'd missed the register overlap constraint, so I added that plus testing both for that case, and the masked instruction case which I'd not previously had tests for. |
|
|
||
| # CHECK-ASM-AND-OBJ: rv.vzipeven.vv v1, v2, v0, v0.t | ||
| # CHECK-ASM: encoding: [0xdb,0x00,0x20,0x30] | ||
| rv.vzipeven.vv v1, v2, v0, v0.t |
There was a problem hiding this comment.
This is technically ill-formed by the rule that you can't read the same source with 2 different EEWs, but I don't think we enforce that anywhere today.
|
Could you please add an entry in the release notes for this? |
7 participants
This implements assembler support for the XRivosVizip custom/vendor extension from Rivos Inc. which is defined in:
https://github.com/rivosinc/rivos-custom-extensions (See src/xrivosvizip.adoc)
Codegen support will follow in a separate change.