Skip to content

Commit

Permalink
Make feature macro undefs conditional on -cl-ext input (intel#426)
Browse files Browse the repository at this point in the history
Commits d6563b0, de262c9 and d5a2638 have provided a resolution for
`opencl-c-base.h` defining OCL C 3.0 feature macros unconditionally, which
interfered with Compute Runtime's platform-dependent definitions of extension
lists.

An issue with this approach is that `opencl-c-base.h` provides additional
defines based on `__opencl_c_atomic_scope_all_devices` being set:
```
  memory_scope_all_svm_devices = __OPENCL_MEMORY_SCOPE_ALL_SVM_DEVICES,
  memory_scope_all_devices = memory_scope_all_svm_devices,
```
(from https://github.com/llvm/llvm-project/blob/release/16.x/clang/lib/Headers/opencl-c-base.h#L387-L392)

Simply erasing the macro definition from `opencl-c-base.h` leads to memory
scope values' erasure, as the command line definitions aren't available at that
moment due to headers' inclusion order. Original commit d6563b0 did mention an
alternative approach, which accounts for the above issue:
> Once https://reviews.llvm.org/D141297 (LLVM 16) is integrated,
  an alternative approach could be to pass `-D__undef_<feature_macro>`
  <...> at the OCL Clang's level (upon detecting the lack of
  extension-defining inputs to `-cl-ext`).

Implement just that in scope of the current commit. Said LLORG patch simply
needs to be applied for CClang 14-15 instead of the original "unconditional
undef" one.

Signed-off-by: Artem Gindinson <artem.gindinson@intel.com>
  • Loading branch information
AGindinson authored May 16, 2023
1 parent d5a2638 commit 78c5e3f
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 36 deletions.
20 changes: 20 additions & 0 deletions options_compile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Copyright (c) Intel Corporation (2009-2017).
#include "options.h"

#include "clang/Driver/Options.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/Option/Arg.h"
#include "llvm/Option/ArgList.h"
#include "llvm/Support/ManagedStatic.h"
Expand Down Expand Up @@ -277,11 +278,30 @@ std::string EffectiveOptionsFilter::processOptions(const OpenCLArgList &args,
it->second = enabled;
}
};
llvm::SmallSet<llvm::StringRef, 32> parsedOclCFeatures;
std::for_each(effectiveArgs.begin(), effectiveArgs.end(),
[&](const ArgsVector::value_type &a) {
if (a.find("-cl-ext=") == 0)
parseClExt(a);
else if (a.find("-D__opencl_c_") == 0)
parsedOclCFeatures.insert(a);
});

// "opencl-c-base.h" unconditionally enables a list of so-called "optional
// core" language features. We need to undef those that aren't explicitly
// defined within the compilation command (which would suggest that the
// target platform supports the corresponding feature).
const char* optionalCoreOclCFeaturesList[] = {
"__opencl_c_work_group_collective_functions",
"__opencl_c_atomic_order_seq_cst",
"__opencl_c_atomic_scope_device",
"__opencl_c_atomic_scope_all_devices",
"__opencl_c_read_write_images" };
for (std::string OclCFeature : optionalCoreOclCFeaturesList) {
if (!parsedOclCFeatures.contains(std::string("-D") + OclCFeature))
effectiveArgs.push_back(std::string("-D__undef_") + OclCFeature);
}

// extension is enabled in PCH but disabled or not specifed in options =>
// disable pch
bool useModules =
Expand Down

This file was deleted.

104 changes: 104 additions & 0 deletions patches/clang/0004-OpenCL-Allow-undefining-header-only-macros.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
From a60b8f468119065f8a6cb4a16598263cb00de0b5 Mon Sep 17 00:00:00 2001
From: Sven van Haastregt <sven.vanhaastregt@arm.com>
Date: Mon, 16 Jan 2023 11:32:12 +0000
Subject: [PATCH] [OpenCL] Allow undefining header-only features

`opencl-c-base.h` always defines 5 particular feature macros for
SPIR-V, making it impossible to disable those features.

To allow disabling any of those features, let the header recognize
`__undef_<feature>` macros. The user can then pass the
`-D__undef_<feature>` flag on the command line to disable a specific
feature. The __undef macro could potentially also be set from
`-cl-ext=-feature`, but for now only change the header and only
provide __undef macros for the 5 features that are always enabled in
`opencl-c-base.h`.

Differential Revision: https://reviews.llvm.org/D141297
---
clang/lib/Headers/opencl-c-base.h | 19 ++++++++++++++++
clang/test/SemaOpenCL/features.cl | 37 ++++++++++++++++++++++---------
2 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/clang/lib/Headers/opencl-c-base.h b/clang/lib/Headers/opencl-c-base.h
index c433b4f7eb1af..fad2f9c0272bf 100644
--- a/clang/lib/Headers/opencl-c-base.h
+++ b/clang/lib/Headers/opencl-c-base.h
@@ -74,6 +74,25 @@
#define __opencl_c_atomic_scope_all_devices 1
#define __opencl_c_read_write_images 1
#endif // defined(__SPIR__)
+
+// Undefine any feature macros that have been explicitly disabled using
+// an __undef_<feature> macro.
+#ifdef __undef___opencl_c_work_group_collective_functions
+#undef __opencl_c_work_group_collective_functions
+#endif
+#ifdef __undef___opencl_c_atomic_order_seq_cst
+#undef __opencl_c_atomic_order_seq_cst
+#endif
+#ifdef __undef___opencl_c_atomic_scope_device
+#undef __opencl_c_atomic_scope_device
+#endif
+#ifdef __undef___opencl_c_atomic_scope_all_devices
+#undef __opencl_c_atomic_scope_all_devices
+#endif
+#ifdef __undef___opencl_c_read_write_images
+#undef __opencl_c_read_write_images
+#endif
+
#endif // (__OPENCL_CPP_VERSION__ == 202100 || __OPENCL_C_VERSION__ == 300)

#if !defined(__opencl_c_generic_address_space)
diff --git a/clang/test/SemaOpenCL/features.cl b/clang/test/SemaOpenCL/features.cl
index af058b5e69828..3f59b4ea3b5ae 100644
--- a/clang/test/SemaOpenCL/features.cl
+++ b/clang/test/SemaOpenCL/features.cl
@@ -26,6 +26,15 @@
// 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 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 \
+// RUN: -D__undef___opencl_c_atomic_order_seq_cst=1 \
+// RUN: -D__undef___opencl_c_atomic_scope_device=1 \
+// RUN: -D__undef___opencl_c_atomic_scope_all_devices=1 \
+// RUN: -D__undef___opencl_c_read_write_images=1 \
+// RUN: | FileCheck %s --check-prefix=NO-HEADERONLY-FEATURES
+
// Note that __opencl_c_int64 is always defined assuming
// always compiling for FULL OpenCL profile

@@ -43,14 +52,20 @@
// FEATURES: #define __opencl_c_subgroups 1

// NO-FEATURES: #define __opencl_c_int64 1
-// NO-FEATURES-NOT: __opencl_c_3d_image_writes
-// NO-FEATURES-NOT: __opencl_c_atomic_order_acq_rel
-// NO-FEATURES-NOT: __opencl_c_atomic_order_seq_cst
-// NO-FEATURES-NOT: __opencl_c_device_enqueue
-// NO-FEATURES-NOT: __opencl_c_fp64
-// NO-FEATURES-NOT: __opencl_c_generic_address_space
-// NO-FEATURES-NOT: __opencl_c_images
-// NO-FEATURES-NOT: __opencl_c_pipes
-// NO-FEATURES-NOT: __opencl_c_program_scope_global_variables
-// NO-FEATURES-NOT: __opencl_c_read_write_images
-// NO-FEATURES-NOT: __opencl_c_subgroups
+// NO-FEATURES-NOT: #define __opencl_c_3d_image_writes
+// NO-FEATURES-NOT: #define __opencl_c_atomic_order_acq_rel
+// NO-FEATURES-NOT: #define __opencl_c_atomic_order_seq_cst
+// NO-FEATURES-NOT: #define __opencl_c_device_enqueue
+// NO-FEATURES-NOT: #define __opencl_c_fp64
+// NO-FEATURES-NOT: #define __opencl_c_generic_address_space
+// NO-FEATURES-NOT: #define __opencl_c_images
+// NO-FEATURES-NOT: #define __opencl_c_pipes
+// NO-FEATURES-NOT: #define __opencl_c_program_scope_global_variables
+// NO-FEATURES-NOT: #define __opencl_c_read_write_images
+// NO-FEATURES-NOT: #define __opencl_c_subgroups
+
+// 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
+// NO-HEADERONLY-FEATURES-NOT: #define __opencl_c_atomic_scope_all_devices
+// NO-HEADERONLY-FEATURES-NOT: #define __opencl_c_read_write_images

0 comments on commit 78c5e3f

Please sign in to comment.