This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP][FIX] Allow device constructors for AMD GPU
ClosedPublic

Authored by jdoerfert on Mar 16 2022, 11:47 AM.

Details

Summary

In AMD GPU device code the globals are in AS(1). Before, we crashed if
the global was a structure. Now we simply cast away the AS before we
generate the code to initialize the global.

Diff Detail

Event Timeline

jdoerfert created this revision.Mar 16 2022, 11:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 16 2022, 11:47 AM
jdoerfert requested review of this revision.Mar 16 2022, 11:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 16 2022, 11:47 AM
JonChesterfield accepted this revision.Mar 16 2022, 12:53 PM

Nice, thanks!

This revision is now accepted and ready to land.Mar 16 2022, 12:53 PM
This revision was landed with ongoing or failed builds.Mar 16 2022, 3:05 PM
This revision was automatically updated to reflect the committed changes.

Hi Johannes,
assert in two of the tests

#7 0x00005576b8d055e4 llvm::ConstantExpr::getAddrSpaceCast(llvm::Constant*, llvm::Type*, bool) (/work/omp-vega20-0/openmp-offload-amdgpu-runtime/llvm.build/bin/clang-15+0x1e4b5e4)

https://lab.llvm.org/buildbot/#/builders/193/builds/8594/steps/6/logs/FAIL__libomptarget____x86_64-pc-linux-gnu__global_

jdoerfert reopened this revision.Mar 17 2022, 9:48 AM
This revision is now accepted and ready to land.Mar 17 2022, 9:48 AM
jdoerfert updated this revision to Diff 416218.Mar 17 2022, 9:49 AM

Fix AS(0) -> AS(0) cast error

i applied it locally and ran bot equiv, looks good to me

This revision was landed with ongoing or failed builds.Mar 17 2022, 10:54 AM
This revision was automatically updated to reflect the committed changes.
thakis added a subscriber: thakis.Mar 17 2022, 11:35 AM

Looks like this breaks tests: http://45.33.8.238/linux/71358/step_7.txt

Please take a look and revert for now if it takes a while to fix.

Looks like this breaks tests: http://45.33.8.238/linux/71358/step_7.txt

Please take a look and revert for now if it takes a while to fix.

I relaxed the check line. Should resolve the problem.

kamaub added a subscriber: kamaub.Mar 18 2022, 7:25 AM

The test cases added with this commit failed on clang-ppc64be-linux-lnt # 13809 could you please revert this change, and recommit with the test case corrected? thank you.

jdoerfert added a comment.EditedMar 18 2022, 8:48 AM

The test cases added with this commit failed on clang-ppc64be-linux-lnt # 13809 could you please revert this change, and recommit with the test case corrected? thank you.

[EDIT] I did in https://reviews.llvm.org/rGb4cc3b1dd8d7200640210513263b28187f810703, it seems the metadata is reordered in that bot... I am fixing it right now...