reames added inline comments.
================
Comment at: clang/test/Preprocessor/riscv-target-features.c:437
+
+// RUN: %clang -target riscv64 -march=rv64ixventanacondops -x c -E -dM %s \
+// RUN: -o - | FileCheck --check-prefix=CHECK-XVENTANACONDOPS-EXT %s
----------------
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.
================
Comment at: clang/test/Preprocessor/riscv-target-features.c:439
+// RUN: -o - | FileCheck --check-prefix=CHECK-XVENTANACONDOPS-EXT %s
+// CHECK-XVENTANACONDOPS-EXT: __riscv_xventanacondops 1000000{{$}}
----------------
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?
================
Comment at: llvm/docs/RISCVUsage.rst:161
+``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.
+
----------------
Anything else we want in the documentation? Remember, we're setting precedent
here.
================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:1785
+//===----------------------------------------------------------------------===//
+// Vendor extensions
+//===----------------------------------------------------------------------===//
----------------
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?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137350/new/
https://reviews.llvm.org/D137350
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
