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;
}
----------------
yaxunl wrote:
> 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
If it is out of scope for this patch I am OK to leave this part out. I can
create a patch to refactor this part later.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105981/new/
https://reviews.llvm.org/D105981
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
