jhuber6 added inline comments.
================ Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:258 + if (ToBeDeleted.empty()) + return None; + ---------------- jdoerfert wrote: > if (!StripSections) > return None; Fixed this later, I could rebase it so it applies here if it makes the review easier. ================ Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:435 + Arg = **NewFileOrErr; + } + } ---------------- jdoerfert wrote: > Does this work with the "do not strip option"? I somehow doubt it and would > expect an error. If we don't strip then we just return `None` to indicate we didn't create a new host file and this code won't execute. ================ Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:445 // TODO: Wrap device image in a host binary and pass it to the linker. WithColor::warning(errs(), argv[0]) << "Offload linking not yet supported.\n"; ---------------- jdoerfert wrote: > I know this is not new but I don't understand the warning here. it was just a stand-in to indicate that the tool was indeed running with the `-fopenmp-new-driver` flag, but wasn't doing the offload linking step and thus wouldn't execute on the device. It's removed once offloading actually works. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116545/new/ https://reviews.llvm.org/D116545 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
