yaxunl added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:831-860
auto Kind = llvm::AMDGPU::parseArchAMDGCN(GpuArch);
const StringRef CanonArch = llvm::AMDGPU::getArchNameAMDGCN(Kind);
std::string LibDeviceFile = RocmInstallation.getLibDeviceFile(CanonArch);
if (LibDeviceFile.empty()) {
getDriver().Diag(diag::err_drv_no_rocm_device_lib) << 1 << GpuArch;
return;
}
----------------
JonChesterfield wrote:
> yaxunl wrote:
> > I think we'd better absorb this part into the newly added function
> > getCommonDeviceLibOptions so that we have a centralized location for
> > determining device libs. We could use offload kind of the toolchain to
> > differentiate between OpenCL/HIP/OpenMP.
> getCommonBitcodeLibs is called by opencl with some other set of constraints
> around argument names.
>
> Persuading opencl to use the same arguments, getting rid of some of the
> files, doing things with aliasing, or however else we want to dice this
> problem is separable from linking the bitcode into openmp and can be left for
> a later patch. Using a common path for HIP and OpenMP seems a step in the
> right direction.
>
> It might take quite a long time to reach consensus on how to deduplicate the
> two remaining copies, which I'd guess is why they were copy/pasted to begin
> with.
We do not need to use shared options for HIP/OpenMP/OpenCL. We could use
if/else to use different options for HIP/OpenCL.
I am suggesting this because this part of code is largely the same as
getCommonDeviceLibOptions, except the option names. Also the name
getCommonDeviceLibOptions indicates that is a common function for all
languages, otherwise it should be renamed as
getCommonDeviceLibOptionsForHIPAndOpenMP
================
Comment at: clang/lib/Driver/ToolChains/AMDGPU.h:141
+ llvm::SmallVector<std::string, 12>
+ getCommonDeviceLibOptions(const llvm::opt::ArgList &DriverArgs,
+ const std::string &GPUArch) const;
----------------
Maybe rename it as getCommonDeviceLibNames since it returns the bc file names
instead of options.
Pls add a comment like 'returns a list of device library names shared by
different languages".
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105981/new/
https://reviews.llvm.org/D105981
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
