This is an archive of the discontinued LLVM Phabricator instance.

libclc: Add -cl-no-stdinc to clang flags on clang >=13
ClosedPublic

Authored by jvesely on Apr 2 2021, 6:52 AM.

Details

Summary

cf3ef15a6ec5e5b45c6c54e8fbe3769255e815ce ("[OpenCL] Add builtin
declarations by default.")
switched behaviour to include "opencl-c-base.h". We don't want or need
that for libclc so pass the flag to revert to old behaviour.

Fixes build since cf3ef15a6ec5e5b45c6c54e8fbe3769255e815ce

Diff Detail

Event Timeline

jvesely created this revision.Apr 2 2021, 6:52 AM
jvesely requested review of this revision.Apr 2 2021, 6:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 2 2021, 6:52 AM

Btw just out of curiosity are there any reasons for libclc not to use clang implicit headers? They support all OpenCL standards and get a lot of new features and bug fixes regularly.

Btw just out of curiosity are there any reasons for libclc not to use clang implicit headers? They support all OpenCL standards and get a lot of new features and bug fixes regularly.

historic. libclc was ignored by most of the clang opencl work and nobody volunteered to switch it to clang provided headers.
Moreover, it has been stuck in clc 1.1/1.2 so most of the new features don't apply.
The main user is mesa/clover so that one needs to be switched to using clang implicit headers first

LLAsm path still can't use (any) headers so that part of this patch would be needed anyway.

Btw just out of curiosity are there any reasons for libclc not to use clang implicit headers? They support all OpenCL standards and get a lot of new features and bug fixes regularly.

historic. libclc was ignored by most of the clang opencl work and nobody volunteered to switch it to clang provided headers.
Moreover, it has been stuck in clc 1.1/1.2 so most of the new features don't apply.
The main user is mesa/clover so that one needs to be switched to using clang implicit headers first

OK, I see. Thanks for the clarification! Btw the functionality of version 1.1. and 1.2 is fairly complete in clang headers. So it should be relatively low risk if you switch to it. If there are any specific adjustments needed for libclc we can surely find a way to accommodate those.

Btw just out of curiosity are there any reasons for libclc not to use clang implicit headers? They support all OpenCL standards and get a lot of new features and bug fixes regularly.

historic. libclc was ignored by most of the clang opencl work and nobody volunteered to switch it to clang provided headers.
Moreover, it has been stuck in clc 1.1/1.2 so most of the new features don't apply.
The main user is mesa/clover so that one needs to be switched to using clang implicit headers first

OK, I see. Thanks for the clarification! Btw the functionality of version 1.1. and 1.2 is fairly complete in clang headers. So it should be relatively low risk if you switch to it. If there are any specific adjustments needed for libclc we can surely find a way to accommodate those.

Agreed. Removing the header duplication is on my todo list. it should help avoid the need for changes like D83473 going forward.
It's not a high priority though. Integration with llvm-project cmake, and fixing convert_* correctness issues are higher on the list.

ping, @tstellar any concerns about this one?

tstellar accepted this revision.Jul 12 2021, 2:48 PM

LGTM, I agree it would be nice to use the builtin headers, but let's git this fix in first.

This revision is now accepted and ready to land.Jul 12 2021, 2:48 PM
This revision was automatically updated to reflect the committed changes.