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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
