getAttributeSpellingListIndex exists for this purpose.

But in this situation this statement sifts not just ReqdWorkGroupSize spellings, but also different WorkGroupAttr.

I think, that getNormalizedFullName() more applicable here.

We have 2 attributes with the same spelling!?! That is completely unacceptable.

No, but getAttributeSpellingListIndex() returns unsigned and an attribute spelling index is a just unsigned in fact. And in result we get the same outcome (true) with diff spellings indices of diff attributes.

Okey.

Ah... I think I see the problem in TableGen. I'm going to push a fix to LLOrg in the next hour or so to make it so that this enum still gets emitted, since it is useful here.

Suggested a review, but my change is here: https://reviews.llvm.org/D76289

There, got it in! rG661c950630fb. Once that comes down, this line should be AL.getKind == ?? && AL.getSpelling == ReqdWorkGroupSize::CXX11_intel_reqd_work_group_size.

Should I wait until the merge?

Any examples with 2? What happens when I pass a negative to these? How about when things conflict with '1'?

@fadeeval please apply this.

Since the defaults are 1, why set them so here (and around here)?

Since the advantage to this form of attribute is the 'default' values, perhaps we should have more tests evaluating that.

Okey.

What about GNU spelling?

I don't sure, I will ask about it.

The answer of John Pennycook is following:
"I hadn't thought about this before, but I'm going to say "No" – at least for now. If we're not using the C++11 attribute specifier syntax we can't have a namespace, and I don't want to set a precedent for converting between intel:: and intel_ that might accidentally introduce name conflicts."

So that a GNU spelling doesn't need here.

@Pennycook, please note that this is documentation for the compiler itself, not for the particular language. It is used to generated the following documentation: https://clang.llvm.org/docs/AttributeReference.html

So, this attribute is accepted by the compiler in GNU spelling and therefore, must be documented here as well if you adding such documentation. In SYCL-specific documents we can omit GNU spelling, but not here.

BTW, looking at the web-page I referenced above, it would be okay for me to either mention all declared spellings or just leave reqd_work_group_size generic string

@fadeeval, this comment is the only one I have. I would like header to be aligned with the actual documentation, which describes all three spellings

Ok.

There is an enum class that gets created in the attribute type itself that has these lists. Something like ReqdWorkGroupSizeAttr::CXX11_intel_reqd_work_group_size

Got it.

I cannot see the exact spelling because I don't have your patch installed, so i sort of guessed based on the spelling. Look in your build directory at:
whatever/build/tools/clang/include/clang/Sema/Attrs.inc
You'll see something like this:

 class SectionAttr : public InheritableAttr {
 unsigned nameLength;
 char *name;

 public:
   enum Spelling {
      GNU_section = 0,
      CXX11_gnu_section = 1,
      Declspec_allocate = 2,
    SpellingNotCalculated = 15
    };
    ...

See the Spelling enum which should have the entry for your new spelling.

I watched in this file and I didn't find enum Spelling in class ReqdWorkGroupSize. It is very strange.

Note: Every comparison with this gives me a -Wsign-compare warning. XDim, YDim, and ZDim are all 'int' in type. This should be so as well likely.

The problem is TableGen has no unsigned type.

Okey, this problem I solved, it seems.

@Pennycook, please note that this is documentation for the compiler itself, not for the particular language. It is used to generated the following documentation: https://clang.llvm.org/docs/AttributeReference.html

So, this attribute is accepted by the compiler in GNU spelling and therefore, must be documented here as well if you adding such documentation. In SYCL-specific documents we can omit GNU spelling, but not here.

BTW, looking at the web-page I referenced above, it would be okay for me to either mention all declared spellings or just leave reqd_work_group_size generic string

buildbot/Build_PR_Win didn't pass because Heading, and writes:
"D:/buildbot/Product_worker_intel/Build_PR_With_Lit_Win/llvm.src/clang/include\clang/Basic/AttrDocs.td:2008:17: error: Unknown token when parsing a value
let Heading = "attribute((reqd_work_group_size)), [[cl::reqd_work_group_size]],"

Maybe just "reqd_work_group_size" will be better?

Just "reqd_work_group_size" works for me

unknown token when parsing a value
let Heading = "attribute((reqd_work_group_size)), [[cl::reqd_work_group_size]],"

Probably this is some kind of keyword which is being parsed regardless of the fact that it is within a string literal

I'm just a dumb purple bee, so

This allows the compiler to optimize the
generated code appropriately for this kernel.

Which kernel?

The kernel to which attribute is applied. Maybe "the kernel" will be more correct then "this kernel", I think.

Or better to write: "for the kernel to which attribute is applied"?

Both variants looks good to me. I just copied this from OpenCL spec, but apparently it has more context so "this" is applicable there

Do you mean specification of the attribute is propagated somewhere?

No, it says about function specification, that the attribute setup.

I see that this phrase is actually copy-pasted from the spec, but it looks ugly. Maybe we could ask someone with good enough English to rewrite it...

This is from the spec too? which after which feels ugly too. :)