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"; else 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 https://reviews.llvm.org/D46472 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits