[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