This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Replace OpenMP RTL Functions With OMPIRBuilder and OMPKinds.def
ClosedPublic

Authored by jhuber6 on Sep 28 2020, 10:14 AM.

Details

Summary

Replace the OpenMP Runtime Library functions used in CGOpenMPRuntimeGPU for OpenMP device code generation with ones in OMPKinds.def and use OMPIRBuilder for generating runtime calls. This allows us to consolidate more OpenMP code generation into the OMPIRBuilder.

Diff Detail

Event Timeline

jhuber6 created this revision.Sep 28 2020, 10:14 AM
jhuber6 requested review of this revision.Sep 28 2020, 10:14 AM

This currently fails some Clang tests because of an error with how size_t is handled by Clang versus OMPKinds.def. The compiler will crash when generating code for a 32 bit device on a 64 bit host because Clang uses the size_t from the host while OMPKinds.def uses the size_t from the device.

sstefan1 added inline comments.Sep 28 2020, 12:12 PM
clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
38 ↗(On Diff #294744)

There's already an instance of OpenMPIRBuilder in CGOpenMPRuntime.

Maybe just make it protected?

jhuber6 added inline comments.Sep 28 2020, 12:19 PM
clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
38 ↗(On Diff #294744)

When it's generating __kmpc_shuffle_intxx around line 2480 it gets an instance of CGOpenMPRuntimeGPU indirectly which meant I had to make it public so it could use it. I could probably rewrite all the functions to be inside the class but I'm not sure if that'll break anything. So I just made it public for now.

sstefan1 added inline comments.Sep 28 2020, 12:28 PM
clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
38 ↗(On Diff #294744)

I think that shouldn't be a problem. There is getOMPBuilder() in CGOpenMPRuntime.

jhuber6 updated this revision to Diff 294798.Sep 28 2020, 1:42 PM

Reusing OMPBuilder from CGOpenMPRuntime. Altered the failing test case. This case failed when compiling for a 32 bit device on a 64 bit host. This case should be considered an error because the host and device have incompatible pointer sizes and shouldn't be able to communicate. The test has been changed to only compile for hosts and devices that have compatible pointer sizes.

jhuber6 updated this revision to Diff 294819.Sep 28 2020, 3:12 PM

Removed now unused function for generating convergent runtime functions. Updated Clang to print an error message if the user attempts to specify incompatible architectures due to a difference in pointer sizes.

jhuber6 updated this revision to Diff 295032.Sep 29 2020, 10:26 AM
jhuber6 added a reviewer: erichkeane.

Adding test cases for incompatible architecture messages. Checking the architecture is done by checking all combinations of architectures.

erichkeane resigned from this revision.Sep 29 2020, 10:28 AM

I'm not really a good person to review this, OMP isn't nearly my expertise.

jhuber6 updated this revision to Diff 295342.Sep 30 2020, 10:18 AM

Moving the Clang error checks into D88594.

jhuber6 updated this revision to Diff 295343.Sep 30 2020, 10:22 AM

Forgot to add one of the test changes from D88594 as it makes this one fail too.

jdoerfert accepted this revision.Sep 30 2020, 10:56 AM

LGTM, that should mean OpenMPKinds lists all OpenMP runtime functions clang generates, which is great.

This revision is now accepted and ready to land.Sep 30 2020, 10:56 AM
jhuber6 updated this revision to Diff 297008.Oct 8 2020, 10:06 AM

Updating tests after landing D88594.