This is an archive of the discontinued LLVM Phabricator instance.

[clang-offload-wrapper] Disabled ELF offload notes embedding by default.
ClosedPublic

Authored by vzakhari on Aug 17 2021, 2:50 PM.

Details

Summary

This change-set puts 93d08acaacec951dbb302f77eeae51974985b6b2 functionality
under -add-omp-offload-notes switch that is OFF by default.
CUDA toolchain is not able to handle ELF images with LLVMOMPOFFLOAD
notes for unknown reason (see https://reviews.llvm.org/D99551#2950272).
I disable the ELF notes embedding until the CUDA issue is triaged and resolved.

Diff Detail

Event Timeline

vzakhari requested review of this revision.Aug 17 2021, 2:50 PM
vzakhari created this revision.
Herald added a project: Restricted Project. · View Herald Transcript
jhuber6 accepted this revision.Aug 17 2021, 2:55 PM

LGTM, applications compile and run now.

This revision is now accepted and ready to land.Aug 17 2021, 2:55 PM
jdoerfert requested changes to this revision.Aug 17 2021, 2:57 PM

I would prefer to revert 93d08acaacec951dbb302f77eeae51974985b6b2 now and then fix the CUDA toolchain rather than making it someone else's problem.

This revision now requires changes to proceed.Aug 17 2021, 2:57 PM

I would prefer to revert 93d08acaacec951dbb302f77eeae51974985b6b2 now and then fix the CUDA toolchain rather than making it someone else's problem.

@jdoerfert, can we please keep the code upstream? I've spent some time already to resolve the conflicts downstream, and I do not really want to do it once again. I am taking the AR to triage the CUDA issue.

vzakhari updated this revision to Diff 367057.Aug 17 2021, 4:29 PM
jdoerfert accepted this revision.Aug 18 2021, 7:35 AM

As discussed, LG, we will look into the NVIDIA GPU problem now to get rid of this again.

This revision is now accepted and ready to land.Aug 18 2021, 7:35 AM
JonChesterfield accepted this revision.Aug 18 2021, 8:08 AM
JonChesterfield added a subscriber: JonChesterfield.

Amusingly similar to D108303. LGTM.

clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
79

I'd have probably gone with an explicit false here but it doesn't make much difference.