This is an archive of the discontinued LLVM Phabricator instance.

[Frontend] Allow OpenMP offloading to aarch64
ClosedPublic

Authored by bryanpkc on Nov 28 2019, 1:09 AM.

Details

Summary

D30644 added OpenMP offloading to AArch64 targets, then D32035 changed the
frontend to throw an error when offloading is requested for an unsupported
target architecture. However the latter did not include AArch64 in the list
of supported architectures, causing the following unit tests to fail:

libomptarget :: api/omp_get_num_devices.c
libomptarget :: mapping/pr38704.c
libomptarget :: offloading/offloading_success.c
libomptarget :: offloading/offloading_success.cpp

Diff Detail

Event Timeline

bryanpkc created this revision.Nov 28 2019, 1:09 AM
This revision is now accepted and ready to land.Nov 28 2019, 10:04 AM
ABataev requested changes to this revision.Nov 28 2019, 10:20 AM
ABataev added a subscriber: ABataev.

Tests are required

This revision now requires changes to proceed.Nov 28 2019, 10:20 AM

Tests are required

The tests for this were added in D30644, but they are currently failing on AArch64 due to D32035. This patch allows those tests to pass again. What further testing should I add?

Tests are required

The tests for this were added in D30644, but they are currently failing on AArch64 due to D32035. This patch allows those tests to pass again. What further testing should I add?

There should be tests in Clang that the target is indeed supported. Tests in libomptarget are fine, but apparently they are not tested on a regular basis when failing for more than two years.

bryanpkc updated this revision to Diff 231582.Nov 29 2019, 9:14 PM

Add tests for -fopenmp-targets=.

Pinging reviewers.

ABataev added inline comments.Dec 5 2019, 6:29 PM
clang/test/OpenMP/target_messages.cpp
4 ↗(On Diff #231582)

Why do you need the changes in this file?

bryanpkc added inline comments.Dec 5 2019, 8:11 PM
clang/test/OpenMP/target_messages.cpp
4 ↗(On Diff #231582)

The clean-up is not strictly necessary. I came across this file when I was looking for the right file to add the tests, and thought it would be better to group similar tests (and their corresponding CHECK patterns) together.

ABataev added inline comments.Dec 6 2019, 6:29 AM
clang/test/OpenMP/openmp_offload_registration.cpp
3–5

Do not add tests targets other than aarch64

clang/test/OpenMP/target_messages.cpp
4 ↗(On Diff #231582)

Do not do this in this patch, these changes are not related to the patch itself.

bryanpkc marked 6 inline comments as done.Dec 6 2019, 11:57 AM
bryanpkc added inline comments.
clang/test/OpenMP/openmp_offload_registration.cpp
3–5

OK, thank you.

clang/test/OpenMP/target_messages.cpp
4 ↗(On Diff #231582)

OK, I will remove these changes.

bryanpkc updated this revision to Diff 232627.Dec 6 2019, 12:10 PM
bryanpkc marked 2 inline comments as done.

Removed unrelated changes from this patch.

This revision is now accepted and ready to land.Dec 6 2019, 1:38 PM
This revision was automatically updated to reflect the committed changes.