This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Clean up AMD handling for `-fopenmp-targets=amdgcn` arch inference
ClosedPublic

Authored by jhuber6 on Jan 19 2023, 9:51 AM.

Details

Summary

Previously we had some special handling here that errored out if
multiple architectures were detected. This isn't a problem anymore as
the runtime can handle multi-archicture binaries automatically. So it's
safe to simply take the first architecture that we know works. If users
use --offload-arch=native instead it will build for all the
architectures at the same time rather than just picking one. This patch
makes it consisten with the NVPTX version.

Diff Detail

Event Timeline

jhuber6 created this revision.Jan 19 2023, 9:51 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 19 2023, 9:51 AM
jhuber6 requested review of this revision.Jan 19 2023, 9:51 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 19 2023, 9:51 AM

@yaxunl Do you know why AMD uses -mcpu but we use -march here? Would there be any downside to aliasing them in the AMDGPU toolchain?

@arsenm as above, mcpu != march important? llc takes a different one to clang iirc

jdoerfert accepted this revision.Jan 20 2023, 2:39 PM

In a follow up we should default to all system archs not the first one. But let's keep them in sync. LG

clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
100

Swap, shorter case first.

This revision is now accepted and ready to land.Jan 20 2023, 2:39 PM
This revision was landed with ongoing or failed builds.Jan 20 2023, 3:34 PM
This revision was automatically updated to reflect the committed changes.

@arsenm as above, mcpu != march important? llc takes a different one to clang iirc

I don't know what the difference would be