[PATCH] D96835: [HIP] Support device sanitizer

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 2 inline comments as done.
yaxunl 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.">;
----------------
tra wrote:
> We do have `BoolFOption` for `-fsomething`/`-fno-something` options. 
done


================
Comment at: clang/lib/Driver/ToolChains/HIP.cpp:117
+                   false))
+    TC.addHIPDeviceLibArgs(Args, LldArgs, /*UseMLinkOpt=*/false);
+
----------------
tra wrote:
> 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`. 
Will refactor to get the list of bitcode files.


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

https://reviews.llvm.org/D96835