jrtc27 added inline comments.
================ Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11170-11175 // RISC-V V-extension def err_riscvv_builtin_requires_v : Error< "builtin requires 'V' extension support to be enabled">; +// RISC-V P-extension +def err_riscvv_builtin_requires_p : Error< + "builtin requires 'P' extension support to be enabled">; ---------------- I seem to recall another patch that includes generalising this to take the extension name rather than adding multiple copies of the same thing ================ Comment at: clang/lib/AST/ASTContext.cpp:4037 +QualType +ASTContext::getRegisterSizedVectorType(QualType vecType, + VectorType::VectorKind VecKind) const { ---------------- Upper-case parameter name ================ Comment at: clang/test/CodeGen/builtins-riscv-rv32p.c:63 + + // RV32: call i32 @llvm.riscv.add8.i32 + ul_r = __rv__add8(ul_a, ul_b); ---------------- One function per test and update_cc_test_checks.py would be a lot nicer IMO ================ Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:715-732 + if (Subtarget.is64Bit()) { + addTypeForP(MVT::v8i8, MVT::i64); + addTypeForP(MVT::v4i16, MVT::i64); + addTypeForP(MVT::v2i32, MVT::i64); + } else { + addTypeForP(MVT::v4i8, MVT::i32); + addTypeForP(MVT::v2i16, MVT::i32); ---------------- This seems like it will interact poorly with V if both are present ================ Comment at: llvm/test/CodeGen/RISCV/intrinsics-rv32p.ll:5 + +define void @test() nounwind { +; RV32P-LABEL: test: ---------------- This is an awful test. One function per intrinsic, and no unnecessary alloca's. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99158/new/ https://reviews.llvm.org/D99158 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits