t-tye accepted this revision.
t-tye added a comment.
This revision is now accepted and ready to land.

LGTM except for minor suggestions.

Comment at: lib/CodeGen/CGCUDANV.cpp:361-373
+  if (IsHIP)
+    FatbinConstantName = ".hip_fatbin";
+  else if (RelocatableDeviceCode)
     // TODO: Figure out how this is called on mac OS!
     FatbinConstantName = "__nv_relfatbin";
     FatbinConstantName =
Should this all be inside an if (IsHip) so the names can be different for HIP 
and NV CUDA? Seems HIP should not be using names with VV_CUDA in them. In fact 
maybe the following IF should be included as well to consilidate the NV CUDA 
code and HIP code together.

Comment at: lib/CodeGen/CGCUDANV.cpp:394-398
   // Fatbin wrapper magic.
   Values.addInt(IntTy, 0x466243b1);
   // Fatbin version.
   Values.addInt(IntTy, 1);
   // Data.
Should HIP use the same magic value and version number? Perhaps this should 
also be moved into the IsHip IF.

Comment at: lib/CodeGen/CGCUDANV.cpp:438
     // void __cudaRegisterLinkedBinary%NVModuleID%(void (*)(void *), void *,
     // void *, void (*)(void **))
Update comment for HIP

Comment at: lib/Driver/ToolChains/CommonArgs.cpp:150
+    // If the current tool chain refers to an OpenMP or HIP offloading host, we
+    // should ignore inputs that refer to OpenMP offloading devices - they will
+    // be embedded according to a proper linker script.
OpenMP -> OpenMP or HIP

Comment at: lib/Driver/ToolChains/CommonArgs.cpp:1321-1322
+  // Construct clang-offload-bundler command to bundle object files for
+  // for different GPU archs.
+  ArgStringList BundlerArgs;
for for -> for


cfe-commits mailing list

Reply via email to