[PATCH] D57829: [CUDA][HIP] Disable emitting llvm.linker.options in device compilation

Artem Belevich via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 1 10:19:25 PDT 2019


tra added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:389-392
+  // The linker_option directives are intended for host compilation.
+  if (JA.isDeviceOffloading(Action::OFK_Cuda) ||
+      JA.isDeviceOffloading(Action::OFK_HIP))
+    Default = false;
----------------
yaxunl wrote:
> tra wrote:
> > Shouldn't it be `true`, considering that we do want to **disable** autolinking by default for device-side CUDA/HIP?
> > 
> > If we don't support autolinking at all for CUDA/HIP, perhaps we should just return `true` here.
> This variable Default is to be used as the default value of OPT_fautolink. For device compilation we want the default value to be false. However if users want to force it to be true, we may still want to respect it.
You are correct. I've missed the `!` in the return below.  Don't never use double negation. :-/

Nit: Perhaps we can rename `Default` -> `AutolinkEnabledByDefault` because right now it's easy to misinterpret it as the default return value of the function. Maybe, even change the function itself to `ShouldEnableAutolink()`. 



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

https://reviews.llvm.org/D57829





More information about the cfe-commits mailing list