[PATCH] D132192: [RISCV] Add '32bit' feature to rv32 only builtins.
Luís Marques via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 23 16:17:24 PDT 2022
luismarques added a comment.
Overall LGTM.
I have one concern, though. The old error message was more user friendly. Referring to RV32 as an extension is... weird. You're already massaging the error with the `OF = "RV64"` / `OF = "RV32"`. Can't you special case this feature check error handling to make it print something more like the `err_32_bit_builtin_64_bit_tgt` message, "this builtin is only available on 32-bit targets"? (if so, the same goes for the 64-bit case).
Also, arguably this could be two separate patches, but maybe it's not worth splitting...?
================
Comment at: clang/lib/Sema/SemaChecking.cpp:4359
- return Diag(TheCall->getCallee()->getBeginLoc(),
- diag::err_32_bit_builtin_64_bit_tgt);
-
----------------
That tablegen def is still being used for X86. Maybe you could make a similar patch for X86?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132192/new/
https://reviews.llvm.org/D132192
More information about the cfe-commits
mailing list