sameerds added inline comments.

================
Comment at: clang/lib/Driver/ToolChains/HIP.cpp:389
 
-  for (Arg *A : Args) {
-    DAL->append(A);
+  if (DeviceOffloadKind != Action::OFK_OpenMP) {
+    for (Arg *A : Args) {
----------------
pdhaliwal wrote:
> arsenm wrote:
> > Needs a comment? I don't understand why openmp is any different here
> `HostTC.TranslateArgs` (HostTC = Generic_GCC, Line#383) returns `DAL` which  
> contains `Args` when offloadkind is OFK_OpenMP (for all other cases, it 
> returns nullptr). Thus, Line#{390,391} is just duplicating the arguments 
> which are propagating to the opt, llc, etc. commands.
> Ref: https://clang.llvm.org/doxygen/Gnu_8cpp_source.html#l02966
I think @arsenm was asking for a comment in the code itself.

Also, I am not sufficiently familiar with the design here, but why is the HIP 
driver checking for OpenMP offloading? Is the offloaded region treated as HIP 
code? It seems a bit strange that we are mixing two different things in the 
same driver.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80996



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D80996: [AMDG... Sameer Sahasrabuddhe via Phabricator via cfe-commits

Reply via email to