jrtc27 added a comment. Please fix the style issues and update the patch with full context (see https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface, or use `arc`)
================ Comment at: clang/include/clang/Basic/BuiltinsRISCV.def:20 +// zbr extension +TARGET_BUILTIN(__builtin_riscv_crc32_b, "LiLi", "nc", "experimental-zbr") ---------------- Zbr ================ Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11170 -// RISC-V V-extension -def err_riscvv_builtin_requires_v : Error< - "builtin requires 'V' extension support to be enabled">; +// RISC-V experimental extension +def err_riscv_builtin_requires_extension : Error< ---------------- This will apply to non-experimental extensions too once V/B/K/etc get ratified ================ Comment at: clang/lib/Sema/SemaChecking.cpp:3400 + + if (!TI.hasFeature(Features)) + { ---------------- This new code assumes there's only one feature in the string ================ Comment at: clang/lib/Sema/SemaChecking.cpp:3405 + << TheCall->getSourceRange() + << Features; + } ---------------- Extension name should have an upper case first letter ================ Comment at: clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbr.c:15-16 +// +long crc32b(long a) +{ + return __builtin_riscv_crc32_b(a); ---------------- `long crc32b(long a) {` for all these ================ Comment at: clang/test/Headers/rvintrin.c:5 +// RUN: -target-feature +experimental-v %s +// expected-no-diagnostics + ---------------- This does nothing unless you add -verify to the Clang command line ================ Comment at: llvm/include/llvm/IR/IntrinsicsRISCV.td:20 + class BitMan_GPR_Intrinsics + : Intrinsic<[llvm_any_ty],[llvm_any_ty], + [IntrNoMem, IntrSpeculatable, IntrWillReturn]>; ---------------- Space after comma ================ Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoB.td:879 +let Predicates = [HasStdExtZbr] in { +def : PatGpr<int_riscv_crc32_b,CRC32B>; +def : PatGpr<int_riscv_crc32_h,CRC32H>; ---------------- Space after comma ================ Comment at: llvm/test/CodeGen/RISCV/rv32Zbr.ll:1 +; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py +; RUN: llc -mtriple=riscv32 -mattr=experimental-zbr -verify-machineinstrs < %s \ ---------------- Lowercase z in the names of these files as it's a (sort of) -march string Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99009/new/ https://reviews.llvm.org/D99009 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits