Skip to content
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
4555f44
[SYCL] Support intel::reqd_work_group_size (fix #6)
fadeeval Mar 10, 2020
c704d1a
[SYCL] Support intel::reqd_work_group_size (fix #7)
fadeeval Mar 11, 2020
d6e3575
[SYCL] Support intel::reqd_work_group_size (fix #7.2)
fadeeval Mar 11, 2020
18b0e94
[SYCL] Support intel::reqd_work_group_size (fix #7.3)
fadeeval Mar 11, 2020
6a313bf
[SYCL] Support intel::reqd_work_group_size (fix #7.4)
fadeeval Mar 11, 2020
995352e
[SYCL] Support intel::reqd_work_group_size (fix #8)
fadeeval Mar 12, 2020
421217a
[SYCL] Support intel::reqd_work_group_size (fix #8.2)
fadeeval Mar 12, 2020
f9c5822
[SYCL] Support intel::reqd_work_group_size (fix #8.3)
fadeeval Mar 13, 2020
eb5c797
[SYCL] Support intel::reqd_work_group_size (fix #9)
fadeeval Mar 13, 2020
9c1c130
[SYCL] Support intel::reqd_work_group_size (fix #9.2)
fadeeval Mar 16, 2020
a8b1121
[SYCL] Support intel::reqd_work_group_size (fix #9.3)
fadeeval Mar 16, 2020
93a022d
[SYCL] Support intel::reqd_work_group_size (fix #10)
fadeeval Mar 16, 2020
fb7e14e
[SYCL] Support intel::reqd_work_group_size (fix #10.2)
fadeeval Mar 16, 2020
8a3f1b8
The test commit, didn' have practical benefit
fadeeval Mar 17, 2020
dc0eaac
Implementing DefaultUnsignedArgument class
fadeeval Mar 17, 2020
04a4c83
Impelmention getAttributeSpellingListIndex()
fadeeval Mar 18, 2020
fc10805
Resolving cinflict ClangAttrEmitter.cpp
fadeeval Mar 18, 2020
971eaa4
The AttrDocs.td changing and other little fixes
fadeeval Mar 18, 2020
65fc0ba
Formatting
fadeeval Mar 18, 2020
2776fe5
AttrDocs.td and formatting
fadeeval Mar 19, 2020
545cb9f
adding GNU in AttrDocs.td
fadeeval Mar 19, 2020
60541fd
Docs.td modifying
fadeeval Mar 20, 2020
50e397d
fix AttrDocs.td heading
fadeeval Mar 20, 2020
cf200c4
Minifix AttrDocs
fadeeval Mar 20, 2020
23e0bbb
Formatting
fadeeval Mar 23, 2020
6d2bce7
AttrDocs.td fix
fadeeval Mar 23, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions clang/include/clang/Basic/Attr.td
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,10 @@ class DefaultIntArgument<string name, int default> : IntArgument<name, 1> {
int Default = default;
}

class DefaultUnsignedArgument<string name, int default> : UnsignedArgument<name, 1> {
int Default = default;
}

// This argument is more complex, it includes the enumerator type name,
// a list of strings to accept, and a list of enumerators to map them to.
class EnumArgument<string name, string type, list<string> values,
Expand Down Expand Up @@ -2413,13 +2417,16 @@ def NoDeref : TypeAttr {
let Documentation = [NoDerefDocs];
}

// Default arguments in ReqWorkGroupSize can be used only with
// intel::reqd_work_group_size spelling.
def ReqdWorkGroupSize : InheritableAttr {
let Spellings = [GNU<"reqd_work_group_size">,
CXX11<"cl","reqd_work_group_size">];
let Args = [UnsignedArgument<"XDim">, UnsignedArgument<"YDim">,
UnsignedArgument<"ZDim">];
CXX11<"intel","reqd_work_group_size">,
CXX11<"cl","reqd_work_group_size">];
let Args = [UnsignedArgument<"XDim">, DefaultUnsignedArgument<"YDim", 1>,
DefaultUnsignedArgument<"ZDim", 1>];
let Subjects = SubjectList<[Function], ErrorDiag>;
let Documentation = [Undocumented];
let Documentation = [ReqdWorkGroupSizeAttrDocs];
}

def WorkGroupSizeHint : InheritableAttr {
Expand Down
12 changes: 12 additions & 0 deletions clang/include/clang/Basic/AttrDocs.td
Original file line number Diff line number Diff line change
Expand Up @@ -1983,6 +1983,18 @@ device kernel, the attribute is ignored and it is not propagated to a kernel.
}];
}

def ReqdWorkGroupSizeAttrDocs : Documentation {
let Category = DocCatFunction;
let Heading = "cl::reqd_work_group_size, intel::reqd_work_group_size";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about GNU spelling?

Copy link
Contributor Author

@fadeeval fadeeval Mar 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So that a GNU spelling doesn't need here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.

let Content = [{
Applies to a device function/lambda function. Indicates the dimensions
of a work group. Values must be positive integers. Default arguments
equaling to one can be used for second and third dimensions if
intel::reqd_work_group_size spelling was applied.
This attribute is also valid for OpenCL C.
}];
}

def SYCLIntelMaxWorkGroupSizeAttrDocs : Documentation {
let Category = DocCatFunction;
let Heading = "max_work_group_size (IntelFPGA)";
Expand Down
14 changes: 11 additions & 3 deletions clang/lib/Sema/SemaDeclAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2922,14 +2922,22 @@ static void handleWorkGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) {
return;

uint32_t WGSize[3];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is TableGen has no unsigned type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okey, this problem I solved, it seems.

if (AL.getKind() == ParsedAttr::AT_ReqdWorkGroupSize &&
AL.getAttributeSpellingListIndex() ==
ReqdWorkGroupSizeAttr::CXX11_intel_reqd_work_group_size) {
WGSize[1] = ReqdWorkGroupSizeAttr::DefaultYDim;
WGSize[2] = ReqdWorkGroupSizeAttr::DefaultZDim;
} else if (!checkAttributeNumArgs(S, AL, 3))
return;

for (unsigned i = 0; i < 3; ++i) {
const Expr *E = AL.getArgAsExpr(i);
if (!checkUInt32Argument(S, AL, E, WGSize[i], i,
if (i < AL.getNumArgs() &&
!checkUInt32Argument(S, AL, AL.getArgAsExpr(i), WGSize[i], i,
/*StrictlyUnsigned=*/true))
return;
if (WGSize[i] == 0) {
S.Diag(AL.getLoc(), diag::err_attribute_argument_is_zero)
<< AL << E->getSourceRange();
<< AL << AL.getArgAsExpr(i)->getSourceRange();
return;
}
}
Expand Down
136 changes: 136 additions & 0 deletions clang/test/SemaSYCL/intel-reqd-work-group-size.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
// RUN: %clang_cc1 -fsycl -fsycl-is-device -fsyntax-only -verify -DTRIGGER_ERROR %s
// RUN: %clang_cc1 -fsycl -fsycl-is-device -ast-dump %s | FileCheck %s
// RUN: %clang_cc1 -fsycl -fsycl-is-host -fsyntax-only -verify %s

#ifndef __SYCL_DEVICE_ONLY__
// expected-no-diagnostics
class Functor {
public:
[[intel::reqd_work_group_size(4)]] void operator()() {}
};

template <typename name, typename Func>
void kernel(Func kernelFunc) {
kernelFunc();
}

void bar() {
Functor f;
kernel<class kernel_name>(f);
}
#else
[[intel::reqd_work_group_size(4)]] void f4x1x1() {} // expected-note {{conflicting attribute is here}}
// expected-note@-1 {{conflicting attribute is here}}
[[intel::reqd_work_group_size(32)]] void f32x1x1() {} // expected-note {{conflicting attribute is here}}

[[intel::reqd_work_group_size(16)]] void f16x1x1() {} // expected-note {{conflicting attribute is here}}
[[intel::reqd_work_group_size(16, 16)]] void f16x16x1() {} // expected-note {{conflicting attribute is here}}

[[intel::reqd_work_group_size(32, 32)]] void f32x32x1() {} // expected-note {{conflicting attribute is here}}
[[intel::reqd_work_group_size(32, 32, 32)]] void f32x32x32() {} // expected-note {{conflicting attribute is here}}

#ifdef TRIGGER_ERROR
class Functor32 {
public:
[[cl::reqd_work_group_size(32)]] void operator()() {} // expected-error {{'reqd_work_group_size' attribute requires exactly 3 arguments}}
};
class Functor33 {
public:
[[intel::reqd_work_group_size(32, -4)]] void operator()() {} // expected-error {{'reqd_work_group_size' attribute requires a non-negative integral compile time constant expression}}
};
#endif

class Functor16 {
public:
[[intel::reqd_work_group_size(16)]] void operator()() {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@Fznamznon Fznamznon Mar 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fadeeval please apply this.

};

class Functor64 {
public:
[[intel::reqd_work_group_size(64, 64)]] void operator()() {}
};

class Functor16x16x16 {
public:
[[intel::reqd_work_group_size(16, 16, 16)]] void operator()() {}
};

class Functor8 { // expected-error {{conflicting attributes applied to a SYCL kernel}}
public:
[[intel::reqd_work_group_size(8)]] void operator()() { // expected-note {{conflicting attribute is here}}
f4x1x1();
}
};

class Functor {
public:
void operator()() {
f4x1x1();
}
};

class FunctorAttr {
public:
__attribute__((reqd_work_group_size(128, 128, 128))) void operator()() {}
};

template <typename name, typename Func>
__attribute__((sycl_kernel)) void kernel(Func kernelFunc) {
kernelFunc();
}

void bar() {
Functor16 f16;
kernel<class kernel_name1>(f16);

Functor f;
kernel<class kernel_name2>(f);

Functor16x16x16 f16x16x16;
kernel<class kernel_name3>(f16x16x16);

FunctorAttr fattr;
kernel<class kernel_name4>(fattr);

kernel<class kernel_name5>([]() [[intel::reqd_work_group_size(32, 32, 32)]] {
f32x32x32();
});

#ifdef TRIGGER_ERROR
Functor8 f8;
kernel<class kernel_name6>(f8);

kernel<class kernel_name7>([]() { // expected-error {{conflicting attributes applied to a SYCL kernel}}
f4x1x1();
f32x1x1();
});

kernel<class kernel_name8>([]() { // expected-error {{conflicting attributes applied to a SYCL kernel}}
f16x1x1();
f16x16x1();
});

kernel<class kernel_name9>([]() { // expected-error {{conflicting attributes applied to a SYCL kernel}}
f32x32x32();
f32x32x1();
});

// expected-error@+1 {{expected variable name or 'this' in lambda capture list}}
kernel<class kernel_name10>([[intel::reqd_work_group_size(32, 32, 32)]][]() {
f32x32x32();
});

#endif
}

// CHECK: FunctionDecl {{.*}} {{.*}}kernel_name1
// CHECK: ReqdWorkGroupSizeAttr {{.*}} 1 1 16
// CHECK: FunctionDecl {{.*}} {{.*}}kernel_name2
// CHECK: ReqdWorkGroupSizeAttr {{.*}} 1 1 4
// CHECK: FunctionDecl {{.*}} {{.*}}kernel_name3
// CHECK: ReqdWorkGroupSizeAttr {{.*}} 16 16 16
// CHECK: FunctionDecl {{.*}} {{.*}}kernel_name4
// CHECK: ReqdWorkGroupSizeAttr {{.*}} 128 128 128
// CHECK: FunctionDecl {{.*}} {{.*}}kernel_name5
// CHECK: ReqdWorkGroupSizeAttr {{.*}} 32 32 32
#endif // __SYCL_DEVICE_ONLY__
5 changes: 4 additions & 1 deletion clang/utils/TableGen/ClangAttrEmitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1304,6 +1304,9 @@ createArgument(const Record &Arg, StringRef Attr,
else if (ArgName == "DefaultIntArgument")
Ptr = std::make_unique<DefaultSimpleArgument>(
Arg, Attr, "int", Arg.getValueAsInt("Default"));
else if (ArgName == "DefaultUnsignedArgument")
Ptr = std::make_unique<DefaultSimpleArgument>(
Arg, Attr, "unsigned", Arg.getValueAsInt("Default"));
else if (ArgName == "IntArgument")
Ptr = std::make_unique<SimpleArgument>(Arg, Attr, "int");
else if (ArgName == "StringArgument")
Expand Down Expand Up @@ -2329,7 +2332,7 @@ static void emitAttributes(RecordKeeper &Records, raw_ostream &OS,
SemanticSpellingMap SemanticToSyntacticMap;

std::string SpellingEnum;
if (!ElideSpelling)
if (Spellings.size() > 1)
SpellingEnum = CreateSemanticSpellings(Spellings, SemanticToSyntacticMap);
if (Header)
OS << SpellingEnum;
Expand Down