[PATCH] D120298: [HIP] Support `--default-stream`

[Yaxun Liu via Phabricator via cfe-commits]

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