craig.topper 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 ---------------- reames wrote: > 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. qemu seems to have also used xventanacondops. ================ Comment at: llvm/lib/Target/RISCV/RISCV.td:422 + : SubtargetFeature<"xventanacondops", "HasVendorXVentanaCondOps", "true", + "'XVentanaCondOps' (Ventana Conditional Move)">; +def HasVendorXVentanaCondOps : Predicate<"Subtarget->hasVendorXVentanaCondOps()">, ---------------- It's not quite conditional move. It's conditional move or zero right? Might be better just says "Ops" or "Operations" instead of "Move". ================ Comment at: llvm/lib/Target/RISCV/RISCV.td:426 + "'XVentanaCondOps' (Ventana Conditional Move)">; +//===----------------------------------------------------------------------===// +// LLVM specific features and extensions ---------------- Blank line above this. ================ Comment at: llvm/lib/Target/RISCV/RISCVInstrFormats.td:148 def OPC_SYSTEM : RISCVOpcode<"SYSTEM", 0b1110011>; +def OPC_CUSTOM3 : RISCVOpcode<"CUSTOM3", 0b1111011>; ---------------- 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. 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
