Skip to content

Conversation

wenju-he
Copy link
Contributor

@wenju-he wenju-he commented Oct 6, 2025

Add header-side undef hooks for:
__opencl_c_integer_dot_product_input_4x8bit
__opencl_c_integer_dot_product_input_4x8bit_packed

Motivation: our downstream legacy targets lack cl_khr_integer_dot_product builtins (similar to a60b8f4). Providing undef* macros lets users suppress the feature macros without patching headers.

Fix intel/compute-runtime#817

Add header-side undef hooks for:
  __opencl_c_integer_dot_product_input_4x8bit
  __opencl_c_integer_dot_product_input_4x8bit_packed

Motivation: our downstream legacy targets lack cl_khr_integer_dot_product
builtins (similar to a60b8f4). Providing __undef__* macros lets
users suppress the feature macros without patching headers.

Fix intel/compute-runtime#817
@wenju-he wenju-he requested a review from Copilot October 6, 2025 09:42
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:X86 clang:headers Headers provided by Clang, e.g. for intrinsics labels Oct 6, 2025
@wenju-he wenju-he requested a review from svenvh October 6, 2025 09:42
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds header-side undefined hooks for the OpenCL integer dot product feature macros to allow downstream legacy targets to suppress these features without patching headers. The changes enable users to define __undef__* macros to disable the cl_khr_integer_dot_product features when they are not supported by their targets.

Key changes:

  • Added undef hooks for __opencl_c_integer_dot_product_input_4x8bit and __opencl_c_integer_dot_product_input_4x8bit_packed feature macros
  • Added corresponding test coverage to verify the undef functionality works correctly

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
clang/lib/Headers/opencl-c-base.h Added conditional undef blocks for the two integer dot product feature macros
clang/test/SemaOpenCL/features.cl Added test case to verify the undef macros properly suppress the feature definitions

@llvmbot
Copy link
Member

llvmbot commented Oct 6, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-backend-x86

Author: Wenju He (wenju-he)

Changes

Add header-side undef hooks for:
__opencl_c_integer_dot_product_input_4x8bit
__opencl_c_integer_dot_product_input_4x8bit_packed

Motivation: our downstream legacy targets lack cl_khr_integer_dot_product builtins (similar to a60b8f4). Providing undef* macros lets users suppress the feature macros without patching headers.

Fix intel/compute-runtime#817


Full diff: https://github.com/llvm/llvm-project/pull/162055.diff

2 Files Affected:

  • (modified) clang/lib/Headers/opencl-c-base.h (+7)
  • (modified) clang/test/SemaOpenCL/features.cl (+9)
diff --git a/clang/lib/Headers/opencl-c-base.h b/clang/lib/Headers/opencl-c-base.h
index 6206a347852be..7de10c6ecd6a3 100644
--- a/clang/lib/Headers/opencl-c-base.h
+++ b/clang/lib/Headers/opencl-c-base.h
@@ -53,6 +53,13 @@
 #define __opencl_c_kernel_clock_scope_work_group 1
 #define __opencl_c_kernel_clock_scope_sub_group 1
 
+#ifdef __undef___opencl_c_integer_dot_product_input_4x8bit
+#undef __opencl_c_integer_dot_product_input_4x8bit
+#endif
+#ifdef __undef___opencl_c_integer_dot_product_input_4x8bit_packed
+#undef __opencl_c_integer_dot_product_input_4x8bit_packed
+#endif
+
 #endif // defined(__SPIR__) || defined(__SPIRV__)
 #endif // (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
 
diff --git a/clang/test/SemaOpenCL/features.cl b/clang/test/SemaOpenCL/features.cl
index 3f59b4ea3b5ae..dd82689c415ad 100644
--- a/clang/test/SemaOpenCL/features.cl
+++ b/clang/test/SemaOpenCL/features.cl
@@ -26,6 +26,12 @@
 // RUN: %clang_cc1 -triple spir-unknown-unknown %s -E -dM -o - -x cl -cl-std=clc++1.0 \
 // RUN:   | FileCheck -match-full-lines %s  --check-prefix=NO-FEATURES
 
+// For OpenCL C 2.0, header-only features can be disabled using macros.
+// RUN: %clang_cc1 -triple spir-unknown-unknown %s -E -dM -o - -x cl -cl-std=CL2.0 -fdeclare-opencl-builtins -finclude-default-header \
+// RUN:    -D__undef___opencl_c_integer_dot_product_input_4x8bit \
+// RUN:    -D__undef___opencl_c_integer_dot_product_input_4x8bit_packed \
+// RUN:   | FileCheck %s --check-prefix=NO-HEADERONLY-FEATURES-CL20
+
 // For OpenCL C 3.0, header-only features can be disabled using macros.
 // RUN: %clang_cc1 -triple spir-unknown-unknown %s -E -dM -o - -x cl -cl-std=CL3.0 -fdeclare-opencl-builtins -finclude-default-header \
 // RUN:    -D__undef___opencl_c_work_group_collective_functions=1 \
@@ -64,6 +70,9 @@
 // NO-FEATURES-NOT: #define __opencl_c_read_write_images
 // NO-FEATURES-NOT: #define __opencl_c_subgroups
 
+// NO-HEADERONLY-FEATURES-CL20-NOT: #define __opencl_c_integer_dot_product_input_4x8bit
+// NO-HEADERONLY-FEATURES-CL20-NOT: #define __opencl_c_integer_dot_product_input_4x8bit_packed
+
 // NO-HEADERONLY-FEATURES-NOT: #define __opencl_c_work_group_collective_functions
 // NO-HEADERONLY-FEATURES-NOT: #define __opencl_c_atomic_order_seq_cst
 // NO-HEADERONLY-FEATURES-NOT: #define __opencl_c_atomic_scope_device

@wenju-he wenju-he requested a review from bader October 6, 2025 09:45
@wenju-he
Copy link
Contributor Author

wenju-he commented Oct 6, 2025

For simplicity, can we revert https://reviews.llvm.org/D92231 and don't always define the feature macro in headers?
It would be easier for users. For instance, as @aratajew points out, Mesa workarounds the issue by undefining __SPIR__ and __SPIRV__, see https://gitlab.freedesktop.org/mesa/mesa/-/blob/main/src/compiler/clc/clc_helpers.cpp#L946

@svenvh
Copy link
Member

svenvh commented Oct 6, 2025

For simplicity, can we revert https://reviews.llvm.org/D92231 and don't always define the feature macro in headers?

Not really, reverting that does not solve the underlying issue and would be a step backwards regarding moving header-only extensions out of the Clang internals. If I understand correctly, the underlying issue is that historically the SPIR target had become the equivalent of "enable all known extensions". Perhaps that's something we'd want to reconsider, but that's a separate discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 clang:headers Headers provided by Clang, e.g. for intrinsics clang Clang issues not falling into any other category
Projects
None yet
3 participants