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

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov 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
----------------
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.


================
Comment at: clang/test/SemaOpenCL/features.cl:14
+#ifndef __opencl_c_int64
+#error "Macro __opencl_c_int64 should be defined"
+#endif
----------------
Anastasia wrote:
> Do you want to add -verify and expected-no-diagnostics directive for this or just change to FileCheck too?
Ok, I think I'll use FileCheck to be consistent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95776