This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Add support for '--offload-arch=native' to OpenMP offloading
ClosedPublic

Authored by jhuber6 on Jan 5 2023, 7:49 PM.

Details

Summary

This patch adds support for '--offload-arch=native' to OpenMP
offloading. This will automatically generate the toolchains required to
fulfil whatever GPUs the user has installed. Getting this to work
requires a bit of a hack. The problem is that we need the ToolChain to
launch its searching program. But we do not yet have that ToolChain
built. I had to temporarily make the ToolChain and also add some logic
to ignore regular warnings & errors.

Depends on D141078

Diff Detail

Event Timeline

jhuber6 created this revision.Jan 5 2023, 7:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 5 2023, 7:49 PM
Herald added a subscriber: guansong. · View Herald Transcript
jhuber6 requested review of this revision.Jan 5 2023, 7:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 5 2023, 7:49 PM

Possible naming hazard here. march=native means target the local processor architecture, zen2 or whatever, and we have the host CPU as an offloading target already. So what I'd expect this to do is host offloading with the openmp runtime compiled for the local variant of x86 or aarch64, not for it to have a guess at a GPU target.

What you think of offload-arch=GPU for pick a plausible GPU? That distinguishes it from other things we might want to offload to. Open question whether it should create a vgpu instance if it can't detect a physical card.

Possible naming hazard here. march=native means target the local processor architecture, zen2 or whatever, and we have the host CPU as an offloading target already. So what I'd expect this to do is host offloading with the openmp runtime compiled for the local variant of x86 or aarch64, not for it to have a guess at a GPU target.

What you think of offload-arch=GPU for pick a plausible GPU? That distinguishes it from other things we might want to offload to. Open question whether it should create a vgpu instance if it can't detect a physical card.

There is some prior art here, e.g. CUDA and CMake, but we don't necessarily need to follow them. I'm in favor of native because it tracks with what the user "expects" native to do. It may be somewhat ambiguous given that we can offload to the host, but I think native in the future should just go with whatever we can detect no the user's system. So if someone has some future FPGA it'll detect that. We can still use native in the host way with this syntax, we just can't infer the triple. e.g. clang foo.c -fopenmp -fopenmp-targets=x86_64-unknown-linux-gnu --offload-arch=native will do what you except.

jhuber6 updated this revision to Diff 486901.Jan 6 2023, 9:13 AM

Add test

jdoerfert accepted this revision.Jan 10 2023, 10:56 PM

Looks reasonable to me. See comments though

clang/include/clang/Driver/Driver.h
488

I wouldn't call it query but it's not too bad either. I'd call it "SuppressErrors".

clang/lib/Driver/Driver.cpp
894

Does this necessarily mean we failed with =native, if so it's ok. Just didn't follow the logic all the way.

This revision is now accepted and ready to land.Jan 10 2023, 10:56 PM
jhuber6 marked an inline comment as done.Jan 11 2023, 6:05 AM
jhuber6 added inline comments.
clang/lib/Driver/Driver.cpp
894

I think this might trigger if someone passed --offload-arch= maybe we should treat that the same as native?

jhuber6 updated this revision to Diff 488193.Jan 11 2023, 6:42 AM

Change to SuppressError and make`--offload-arch=` just default to native.

This revision was landed with ongoing or failed builds.Jan 11 2023, 8:31 AM
This revision was automatically updated to reflect the committed changes.
cmtice added a subscriber: cmtice.Jan 11 2023, 10:24 PM

Just a heads up: This commit is causing our bootstrap build to fail (your new openmp-system-arch.c test is failing in our stage1 compiler).

Thanks, I forgot to update that test after fixing a similar problem before.