This is an archive of the discontinued LLVM Phabricator instance.

[SYCL][OpenMP] Fix compilation errors for unsupported __bf16 intrinsics
ClosedPublic

Authored by eandrews on Jan 10 2023, 4:56 AM.

Details

Summary

This patch uses existing deferred diagnostics framework to emit error for unsupported type __bf16 in device code. Error is not emitted in host code.

Diff Detail

Event Timeline

eandrews created this revision.Jan 10 2023, 4:56 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 10 2023, 4:56 AM
eandrews requested review of this revision.Jan 10 2023, 4:56 AM
bader accepted this revision.Jan 10 2023, 1:27 PM

LGTM.

I expect this to be a common issue for all single-source offloading programming models (i.e. CUDA and HIP in addition to SYCL and OpenMP offload). Probably we can generalize the code patterns used in this patch for all of them.

In addition to that, there are other built-in data types not supported either by host or device, which are handled similar way. Right?

This revision is now accepted and ready to land.Jan 10 2023, 1:27 PM

LGTM.
I expect this to be a common issue for all single-source offloading programming models (i.e. CUDA and HIP in addition to SYCL and OpenMP offload). Probably we can generalize the code patterns used in this patch for all of them.

Looks like CUDA added support for the type - https://reviews.llvm.org/D136311, https://reviews.llvm.org/rG678d8946ba2ba790c4c52e96e2134ee136e30057.

In addition to that, there are other built-in data types not supported either by host or device, which are handled similar way. Right?

Yes. Code added here is similar to code added for other unsupported types like __float128

mikerice accepted this revision.Jan 23 2023, 9:08 AM

LGTM too.

Herald added a project: Restricted Project. · View Herald TranscriptJan 25 2023, 12:49 PM

Thanks for the reviews!

tra added a subscriber: tra.Sep 7 2023, 2:41 PM
tra added inline comments.
clang/lib/Sema/Sema.cpp
1978–1979

@eandrews Do you recall what was the reason for *not* issuing the diagnostic on the GPU side?

It appears to do the opposite to what the patch description says. We're supposed to emit error for unsupported type __bf16 in device code, but instead we specifically ignore it during GPU-side compilation. What am I missing?

eandrews added inline comments.Sep 7 2023, 3:44 PM
clang/lib/Sema/Sema.cpp
1978–1979

I don't recall the specifics but I think CUDA had code handling __bf16 differently and this change broke a test with CUDA diagnostics and so I excluded it from the patch. I could try removing this check and seeing what breaks if you'd like.

tra added inline comments.Sep 7 2023, 3:59 PM
clang/lib/Sema/Sema.cpp
1978–1979

It may have been around the time when x86 started exposing bf16 type in the host headers, but NVPTX didn't have any support for the type yet.

This change may have just papered over the problem. Oh, well. That would be just one of the places where we currently don't handle the 'unusual' types across the host/GPU boundary. I'm attempting to clean it up, and will take care of this instance there.