linjamaki added a comment.

In D110618#3032899 <https://reviews.llvm.org/D110618#3032899>, @Anastasia wrote:

> Considering that SPIR-V translation step is also required for other languages 
> would it make sense to add `llvm-spirv` as a common tool like for example 
> C/C++ linkers and create a bit of common infrastructure? It might be 
> something we can do as a separate step too but it would be good to make sure 
> nothing prevents us from doing this in the future... We should probably also 
> think about the command line interface unification as it probably doesn't 
> make sense for every language to add their own flags for locating SPIR-V 
> translator.

I don’t think it would work out well. llvm-spirv (or other tool relying on LLVM 
bitcode input) and LLVM may have version differences that cause obscure error 
cases - like newer LLVM producing bitcode with new features the tools are not 
expecting and ready for them. The usage of llvm-spirv tool in the HIPSPV tool 
chain is not intended to be a longer term solution due to this shortcoming.

> Another question I have is whether `--hipspv-pass-plugin` would be needed 
> when we switch to SPIR-V backend or is this something that would be 
> integrated fully upstream eventually. Then we might need to think more of the 
> suitable transition path.

It’s too early to say if we can integrate --hipspv-pass-plugin fully in the 
future. It would be the preferred outcome. We are not finished with the HIP 
expanders we need for supporting various HIP features so we don’t know for 
certain what will be the outcome for the integration. It’s possible that some 
of the solutions are going to be tightly coupled with runtimes (HIPLZ, HIPCL) 
and others that may not generalize for different SPIR-V execution environments.



================
Comment at: clang/include/clang/Driver/Options.td:3701
   " do not include the default CUDA/HIP wrapper headers">;
+def nohipwrapperinc : Flag<["-"], "nohipwrapperinc">,
+  HelpText<"Do not include the default HIP wrapper headers">;
----------------
tra wrote:
> Is the idea to still add relevant include paths to the wrappers and SDK 
> headers, but not `-include` the wrapper?
> 
> If that's the case, it should probably be generalized into `-nogpuwrapperinc` 
> and apply to both CUDA and HIP.
> 
> Is the idea to still add relevant include paths to the wrappers and SDK 
> headers, but not `-include` the wrapper?
> 
Include paths are meant to be excluded too. I’ll fix the option description.

> If that's the case, it should probably be generalized into `-nogpuwrapperinc` 
> and apply to both CUDA and HIP.
> 
I don’t see an immediate need to generalize the option as I don’t think there 
will be a need for it in the CUDA path. The option could be generalized later 
if the need comes (add generalized option, set -nohipwrapperinc to be alias to 
it).



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110618

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to