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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked an inline comment as done.
yaxunl 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;
----------------
tra wrote:
> 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()`. 
> 
will change it to ShouldEnableAutolink


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

https://reviews.llvm.org/D57829