yaxunl marked an inline comment as done.
yaxunl added inline comments.
================
Comment at: clang/include/clang/Driver/Options.td:962
NegFlag<SetFalse>>;
+def default_stream_EQ : Joined<["--"], "default-stream=">,
+ HelpText<"Specify default stream. Valid values are 'legacy' and
'per-thread'. The default value is 'legacy'. (HIP only)">,
----------------
tra wrote:
> Naming, as usual, is hard. :-)
>
> Considering that the option tweaks codegen, it may be appropriate to use `-f`
> prefix.
> On the other hand, `--default-stream` is the option used by NVCC, so it may
> be more familiar to the end users.
> The third option would be to use `--gpu-default-stream` or
> `-fgpu-default-stream`.
>
> I'm leaning towards `-fgpu-default-stream`.
>
> WDYT?
Agree. -fgpu-default-stream fits clang better.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6906
+ CmdArgs.push_back(
+ Args.MakeArgString("-DHIP_API_PER_THREAD_DEFAULT_STREAM"));
+ }
----------------
tra wrote:
> This should probably be done by the preprocessor itself where we define other
> HIP/CUDA related environment variables. We may need to pass-through the
> option itself to cc1 for that. I think clang would do that by default if we
> were using `-f*default-stream`.
Right. Will do it in preprocessor.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120298/new/
https://reviews.llvm.org/D120298
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
