[PATCH] D95776: [OpenCL] Add macro definitions of OpenCL C 3.0 features

Thu Feb 4 05:43:36 PST 2021

Anastasia added inline comments.

================
Comment at: clang/test/SemaOpenCL/features.cl:1
+// RUN: %clang_cc1 -triple spir-unknown-unknown -finclude-default-header %s -E -dM -o - -x cl -cl-std=CL2.0 -cl-ext=-all \
+// RUN:   | FileCheck -match-full-lines %s  --check-prefix=FEATURES
----------------
azabaznov wrote:
> Anastasia wrote:
> > since `-cl-ext=-all` doesn't affect the header functionality I suggest to drop it and use one one line with `-finclude-default-header`. Parsing time with this header is 300x slower so we should minimise its use in tests to avoid negative impact on clang testing.
> > 
> > Would it make sense to add a line without the header with `-cl-std=CL2.0` or/and `-cl-std=CL1.2` to check that there are indeed no macros defined?
> > Would it make sense to add a line without the header with -cl-std=CL2.0 or/and -cl-std=CL1.2 to check that there are indeed no macros defined?
> 
> I think this makes no sense since we are planning to use these macros only internally for headers. Or if it's planned to include `opencl-c-base.h` by default, then this kind of lines can be eliminated for < 3.0 at all.
I guess we still need to test the the macros are not added where they are not available which is not only OpenCL 3.0 or OpenCL 2.0 but also earlier standards?

For example for OpenCL 1.2 they shouldn't be defined with or without the header?

Btw I suggest to move the header functionality into clang/test/Headers/opencl-c-header.cl, where we already tests some extension macros for example `cl_khr_subgroup_extended_types` .  In general it would be better to partition the tests but parsing that header has been so slow that we got shouted at for slowing down the clang testing. 

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95776/new/

https://reviews.llvm.org/D95776