[PATCH] D119560: [OpenCL] opencl-c.h: remove arg names from atomics

Tue Feb 15 04:50:47 PST 2022

svenvh added a comment.

In D119560#3322531 <https://reviews.llvm.org/D119560#3322531>, @Anastasia wrote:

>> also makes the header no longer "claim" the identifiers "success",
>> "failure", "desired", "value" (such that you can compile with -Dvalue=...
>> when including the header for example, which currently breaks parsing
>> of the header).
>
> I don't get what you mean by this. :)

Compiling a CL source file with e.g. `clang -cl-std=CL2.0 -Xclang -finclude-default-header -cl-no-stdinc -Dvalue=1 clang/test/CodeGenOpenCL/as_type.cl` gives lots of errors such as the following, because defining `value` as a macro (which is not a reserved identifier as far as I'm aware) collides with the argument names in the header:

  In file included from <built-in>:1:
  lib/clang/15.0.0/include/opencl-c.h:13277:58: error: expected ')'
  void __ovld atomic_init(volatile atomic_int *object, int value);
                                                           ^
  <command line>:1:15: note: expanded from here
  #define value 1

>> This is a big patch and it only touches the OpenCL 2 atomics for now. I
>> wonder if we should remove argument names from the other builtins too?
>> I think it would help unifying the header and tablegen approaches: if we
>> gradually move the header into some canonical form, we might be able
>> to eventually replace it with a tablegen-ed header, while being able to
>> easily confirm equivalence.
>
> The only drawback I see if that we will lose the history a bit in "git blame" but:

Slight nuance: we will not lose any history, but I understand your concern: someone needs to look through this commit to see the previous commit that touched the code.

If there are no objections to removing all argument names from the header, I'll try to prepare patches for doing so.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119560