saiislam added a comment.
Thanks for the detailed review. I will update rest of the patch soon.
================
Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:613-614
CmdArgs.push_back("-shared");
+ std::string ArchArg = std::string("-plugin-opt=mcpu=").append(Arch.str());
+ CmdArgs.push_back(ArchArg);
CmdArgs.push_back("-o");
----------------
jhuber6 wrote:
> Does this just pass the architecture to the AMD link? Is this related? If not
> move it to a separate patch and I'll review it.
Yes, it passes the architecture to AMD link.
================
Comment at: clang/tools/clang-linker-wrapper/OffloadWrapper.cpp:229-232
+ // store value of these variables (i.e. offload archs) into a custom
+ // section which will be used by "offload-arch -f". It won't be
+ // removed during binary stripping.
+ GV->setSection(".offload_arch_list");
----------------
jhuber6 wrote:
> Why does this need a custom section? We should just use it like this, not
> sure why we need these to be anything but some internal struct.
> ```
> for (auto *Image : BinDesc->Images) {
> if (Image->Info)
> // Use Info
> }
> ```
Custom section is required so that it survives binary stripping done by various
OSes. `Offload-arch` tool will look into this section of the binary to print
list of supported architectures.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124525/new/
https://reviews.llvm.org/D124525
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
