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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to