[PATCH] D96835: [HIP] Support device sanitizer

Artem Belevich via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 17 10:36:48 PST 2021


tra added inline comments.


================
Comment at: clang/include/clang/Driver/Options.td:939
   "__cyg_profile_func_enter and __cyg_profile_func_exit">;
+def fgpu_sanitize : Flag<["-"], "fgpu-sanitize">,
+  HelpText<"Enable sanitizer for AMDGPU target.">;
----------------
We do have `BoolFOption` for `-fsomething`/`-fno-something` options. 


================
Comment at: clang/lib/Driver/ToolChains/HIP.cpp:117
+                   false))
+    TC.addHIPDeviceLibArgs(Args, LldArgs, /*UseMLinkOpt=*/false);
+
----------------
I'd pass the library prefix argument as a string, instead of a boolean flag. Makes it easier to tell what's going on without having to annotate it as a comment. 

Also, maybe consider separating "get the list of bitcode files" from "construct aguments for tool X for the given list of bitcode files". Right now `addHIPDeviceLibArgs` does both and has to plumb the `UseMLinkOpt`through multiple function calls. Adding the prefix argument can be done at the `constructLldCommand`/`addClangTargetOptions`. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96835/new/

https://reviews.llvm.org/D96835



More information about the cfe-commits mailing list