Skip to content

Commit

Permalink
[FPGA][SYCL] Add support for memory attributes on device_global varia…
Browse files Browse the repository at this point in the history 
…bles (#12785)

Currently we allow FPGA memory attributes on const variables. This patch
relaxes the restriction that permits the attributes when applied on
non-const device_global variables and updates the error message to
properly convey which types of variables are allowed to have attributes.
Also this patch relaxes the restriction that permits the private_copies
attribute when applied on const variables. Previously we had restriction
that the private_copies attribute would work with local non-const
variables only.

---------

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
  • Loading branch information
@smanna12
smanna12 authored Mar 4, 2024
1 parent 19bb017 commit 3fc6708
Show file tree
Hide file tree
Showing 6 changed files with 630 additions and 66 deletions.
62 changes: 0 additions & 62 deletions clang/include/clang/Basic/Attr.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2646,50 +2646,14 @@ def SYCLIntelEnableLoopPipelining : StmtAttr {
def : MutualExclusions<[SYCLIntelDisableLoopPipelining,
SYCLIntelEnableLoopPipelining]>;

def SYCLIntelLocalNonConstVar : SubsetSubject<Var,
[{S->hasLocalStorage() &&
S->getKind() != Decl::ImplicitParam &&
S->getKind() != Decl::ParmVar &&
S->getKind() != Decl::NonTypeTemplateParm &&
!S->getType().isConstQualified()}],
"local non-const variables">;

def SYCLIntelConstVar : SubsetSubject<Var,
[{S->getKind() != Decl::ImplicitParam &&
S->getKind() != Decl::ParmVar &&
S->getKind() != Decl::NonTypeTemplateParm &&
(S->getType().isConstQualified() ||
S->getType().getAddressSpace() ==
LangAS::opencl_constant)}],
"constant variables">;

def SYCLIntelLocalStaticAgentMemVar : SubsetSubject<Var,
[{S->getKind() != Decl::ImplicitParam &&
S->getKind() != Decl::NonTypeTemplateParm &&
(S->getStorageClass() == SC_Static ||
S->hasLocalStorage())}],
"local variables, static variables, agent memory arguments">;

def SYCLIntelLocalOrStaticVar : SubsetSubject<Var,
[{S->getKind() != Decl::ImplicitParam &&
S->getKind() != Decl::ParmVar &&
S->getKind() != Decl::NonTypeTemplateParm &&
(S->getStorageClass() == SC_Static ||
S->hasLocalStorage())}],
"local variables, static variables">;

def SYCLIntelDoublePump : Attr {
let Spellings = [CXX11<"intel", "doublepump">];
let Subjects = SubjectList<[SYCLIntelConstVar, SYCLIntelLocalOrStaticVar,
Field], ErrorDiag>;
let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost];
let Documentation = [SYCLIntelDoublePumpAttrDocs];
}

def SYCLIntelSinglePump : Attr {
let Spellings = [CXX11<"intel", "singlepump">];
let Subjects = SubjectList<[SYCLIntelConstVar, SYCLIntelLocalOrStaticVar,
Field], ErrorDiag>;
let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost];
let Documentation = [SYCLIntelSinglePumpAttrDocs];
}
Expand All @@ -2708,17 +2672,12 @@ def SYCLIntelMemory : Attr {
}
}
}];
let Subjects = SubjectList<[SYCLIntelConstVar,
SYCLIntelLocalStaticAgentMemVar,
Field], ErrorDiag>;
let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost];
let Documentation = [SYCLIntelMemoryAttrDocs];
}

def SYCLIntelRegister : Attr {
let Spellings = [CXX11<"intel", "fpga_register">];
let Subjects = SubjectList<[SYCLIntelConstVar, SYCLIntelLocalOrStaticVar,
Field], ErrorDiag>;
let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost];
let Documentation = [SYCLIntelRegisterAttrDocs];
}
Expand All @@ -2729,9 +2688,6 @@ def : MutualExclusions<[SYCLIntelDoublePump, SYCLIntelSinglePump,
def SYCLIntelBankWidth : InheritableAttr {
let Spellings = [CXX11<"intel", "bankwidth">];
let Args = [ExprArgument<"Value">];
let Subjects = SubjectList<[SYCLIntelConstVar,
SYCLIntelLocalStaticAgentMemVar,
Field], ErrorDiag>;
let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost];
let Documentation = [SYCLIntelBankWidthAttrDocs];
}
Expand All @@ -2740,9 +2696,6 @@ def : MutualExclusions<[SYCLIntelRegister, SYCLIntelBankWidth]>;
def SYCLIntelNumBanks : InheritableAttr {
let Spellings = [CXX11<"intel", "numbanks">];
let Args = [ExprArgument<"Value">];
let Subjects = SubjectList<[SYCLIntelConstVar,
SYCLIntelLocalStaticAgentMemVar,
Field], ErrorDiag>;
let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost];
let Documentation = [SYCLIntelNumBanksAttrDocs];
}
Expand All @@ -2751,7 +2704,6 @@ def SYCLIntelPrivateCopies : InheritableAttr {
let Spellings = [CXX11<"intel", "private_copies">];
let Args = [ExprArgument<"Value">];
let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost];
let Subjects = SubjectList<[SYCLIntelLocalNonConstVar, Field], ErrorDiag>;
let Documentation = [SYCLIntelPrivateCopiesAttrDocs];
}
def : MutualExclusions<[SYCLIntelRegister, SYCLIntelPrivateCopies]>;
Expand All @@ -2760,8 +2712,6 @@ def : MutualExclusions<[SYCLIntelRegister, SYCLIntelPrivateCopies]>;
def SYCLIntelMerge : Attr {
let Spellings = [CXX11<"intel", "merge">];
let Args = [StringArgument<"Name">, StringArgument<"Direction">];
let Subjects = SubjectList<[SYCLIntelConstVar, SYCLIntelLocalOrStaticVar,
Field], ErrorDiag>;
let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost];
let Documentation = [SYCLIntelMergeAttrDocs];
}
Expand All @@ -2770,19 +2720,13 @@ def : MutualExclusions<[SYCLIntelRegister, SYCLIntelMerge]>;
def SYCLIntelMaxReplicates : InheritableAttr {
let Spellings = [CXX11<"intel", "max_replicates">];
let Args = [ExprArgument<"Value">];
let Subjects = SubjectList<[SYCLIntelConstVar,
SYCLIntelLocalStaticAgentMemVar,
Field], ErrorDiag>;
let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost];
let Documentation = [SYCLIntelMaxReplicatesAttrDocs];
}
def : MutualExclusions<[SYCLIntelRegister, SYCLIntelMaxReplicates]>;

def SYCLIntelSimpleDualPort : Attr {
let Spellings = [CXX11<"intel", "simple_dual_port">];
let Subjects = SubjectList<[SYCLIntelConstVar,
SYCLIntelLocalStaticAgentMemVar,
Field], ErrorDiag>;
let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost];
let Documentation = [SYCLIntelSimpleDualPortAttrDocs];
}
Expand All @@ -2807,9 +2751,6 @@ def SYCLIntelPipeIO : InheritableAttr {
def SYCLIntelBankBits : Attr {
let Spellings = [CXX11<"intel", "bank_bits">];
let Args = [VariadicExprArgument<"Args">];
let Subjects = SubjectList<[SYCLIntelConstVar,
SYCLIntelLocalStaticAgentMemVar,
Field], ErrorDiag>;
let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost];
let Documentation = [SYCLIntelBankBitsDocs];
}
Expand All @@ -2819,9 +2760,6 @@ def : MutualExclusions<[SYCLIntelRegister, SYCLIntelNumBanks]>;
def SYCLIntelForcePow2Depth : InheritableAttr {
let Spellings = [CXX11<"intel", "force_pow2_depth">];
let Args = [ExprArgument<"Value">];
let Subjects = SubjectList<[SYCLIntelConstVar,
SYCLIntelLocalStaticAgentMemVar,
Field], ErrorDiag>;
let LangOpts = [SYCLIsDevice, SilentlyIgnoreSYCLIsHost];
let Documentation = [SYCLIntelForcePow2DepthAttrDocs];
}
Expand Down
7 changes: 7 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12218,6 +12218,13 @@ def err_sycl_attribute_internal_decl
"in an anonymous namespace">;
def err_sycl_attribute_not_device_global
: Error<"%0 attribute can only be applied to 'device_global' variables">;
def err_fpga_attribute_incorrect_variable
: Error<"%0 attribute only applies to constant variables, local variables, "
"static variables, %select{|agent memory arguments, }1non-static data "
"members and device_global variables">;
def err_fpga_attribute_invalid_decl
: Error<"%0 attribute only applies to const variables, local variables, "
"non-static data members and device_global variables">;
def err_sycl_compiletime_property_duplication : Error<
"can't apply %0 property twice to the same accessor">;
def err_sycl_invalid_property_list_param_number : Error<
Expand Down
134 changes: 134 additions & 0 deletions clang/lib/Sema/SemaDeclAttr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7443,6 +7443,24 @@ static bool checkForDuplicateAttribute(Sema &S, Decl *D,
return false;
}

// Checks if FPGA memory attributes apply on valid variables.
// Returns true if an error occured.
static bool CheckValidFPGAMemoryAttributesVar(Sema &S, Decl *D) {
if (const auto *VD = dyn_cast<VarDecl>(D)) {
if (!(isa<FieldDecl>(D) ||
(VD->getKind() != Decl::ImplicitParam &&
VD->getKind() != Decl::NonTypeTemplateParm &&
(S.isTypeDecoratedWithDeclAttribute<SYCLDeviceGlobalAttr>(
VD->getType()) ||
VD->getType().isConstQualified() ||
VD->getType().getAddressSpace() == LangAS::opencl_constant ||
VD->getStorageClass() == SC_Static || VD->hasLocalStorage())))) {
return true;
}
}
return false;
}

void Sema::AddSYCLIntelNoGlobalWorkOffsetAttr(Decl *D,
const AttributeCommonInfo &CI,
Expr *E) {
Expand Down Expand Up @@ -7521,6 +7539,15 @@ static void handleSYCLIntelSinglePumpAttr(Sema &S, Decl *D,
}
}

// Check attribute applies to field, constant variables, local variables,
// static variables, non-static data members, and device_global variables.
if ((D->getKind() == Decl::ParmVar) ||
CheckValidFPGAMemoryAttributesVar(S, D)) {
S.Diag(AL.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< AL << /*agent memory arguments*/ 0;
return;
}

// If the declaration does not have an [[intel::fpga_memory]]
// attribute, this creates one as an implicit attribute.
if (!D->hasAttr<SYCLIntelMemoryAttr>())
Expand All @@ -7544,6 +7571,15 @@ static void handleSYCLIntelDoublePumpAttr(Sema &S, Decl *D,
}
}

// Check attribute applies to field, constant variables, local variables,
// static variables, non-static data members, and device_global variables.
if ((D->getKind() == Decl::ParmVar) ||
CheckValidFPGAMemoryAttributesVar(S, D)) {
S.Diag(AL.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< AL << /*agent memory arguments*/ 0;
return;
}

// If the declaration does not have an [[intel::fpga_memory]]
// attribute, this creates one as an implicit attribute.
if (!D->hasAttr<SYCLIntelMemoryAttr>())
Expand Down Expand Up @@ -7591,6 +7627,15 @@ static void handleSYCLIntelMemoryAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
D->dropAttr<SYCLIntelMemoryAttr>();
}

// Check attribute applies to field, constant variables, local variables,
// static variables, agent memory arguments, non-static data members,
// and device_global variables.
if (CheckValidFPGAMemoryAttributesVar(S, D)) {
S.Diag(AL.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< AL << /*agent memory arguments*/ 1;
return;
}

D->addAttr(::new (S.Context) SYCLIntelMemoryAttr(S.Context, AL, Kind));
}

Expand Down Expand Up @@ -7623,6 +7668,15 @@ static void handleSYCLIntelRegisterAttr(Sema &S, Decl *D,
}
}

// Check attribute applies to field, constant variables, local variables,
// static variables, non-static data members, and device_global variables.
if ((D->getKind() == Decl::ParmVar) ||
CheckValidFPGAMemoryAttributesVar(S, D)) {
S.Diag(A.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< A << /*agent memory arguments*/ 0;
return;
}

if (checkIntelFPGARegisterAttrCompatibility(S, D, A))
return;

Expand Down Expand Up @@ -7661,6 +7715,15 @@ void Sema::AddSYCLIntelBankWidthAttr(Decl *D, const AttributeCommonInfo &CI,
return;
}

// Check attribute applies to field, constant variables, local variables,
// static variables, agent memory arguments, non-static data members,
// and device_global variables.
if (CheckValidFPGAMemoryAttributesVar(*this, D)) {
Diag(CI.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< CI << /*agent memory arguments*/ 1;
return;
}

// Check to see if there's a duplicate attribute with different values
// already applied to the declaration.
if (const auto *DeclAttr = D->getAttr<SYCLIntelBankWidthAttr>()) {
Expand Down Expand Up @@ -7745,6 +7808,15 @@ void Sema::AddSYCLIntelNumBanksAttr(Decl *D, const AttributeCommonInfo &CI,
}
}

// Check attribute applies to constant variables, local variables,
// static variables, agent memory arguments, non-static data members,
// and device_global variables.
if (CheckValidFPGAMemoryAttributesVar(*this, D)) {
Diag(CI.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< CI << /*agent memory arguments*/ 1;
return;
}

// Check to see if there's a duplicate attribute with different values
// already applied to the declaration.
if (const auto *DeclAttr = D->getAttr<SYCLIntelNumBanksAttr>()) {
Expand Down Expand Up @@ -7812,6 +7884,15 @@ static void handleIntelSimpleDualPortAttr(Sema &S, Decl *D,
}
}

// Check attribute applies to field, constant variables, local variables,
// static variables, agent memory arguments, non-static data members,
// and device_global variables.
if (CheckValidFPGAMemoryAttributesVar(S, D)) {
S.Diag(AL.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< AL << /*agent memory arguments*/ 1;
return;
}

if (!D->hasAttr<SYCLIntelMemoryAttr>())
D->addAttr(SYCLIntelMemoryAttr::CreateImplicit(
S.Context, SYCLIntelMemoryAttr::Default));
Expand All @@ -7837,6 +7918,16 @@ void Sema::AddSYCLIntelMaxReplicatesAttr(Decl *D, const AttributeCommonInfo &CI,
<< CI << /*positive*/ 0;
return;
}

// Check attribute applies to field, constant variables, local variables,
// static variables, agent memory arguments, non-static data members,
// and device_global variables.
if (CheckValidFPGAMemoryAttributesVar(*this, D)) {
Diag(CI.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< CI << /*agent memory arguments*/ 1;
return;
}

// Check to see if there's a duplicate attribute with different values
// already applied to the declaration.
if (const auto *DeclAttr = D->getAttr<SYCLIntelMaxReplicatesAttr>()) {
Expand Down Expand Up @@ -7920,6 +8011,15 @@ static void handleSYCLIntelMergeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
return;
}

// Check attribute applies to field, constant variables, local variables,
// static variables, non-static data members, and device_global variables.
if ((D->getKind() == Decl::ParmVar) ||
CheckValidFPGAMemoryAttributesVar(S, D)) {
S.Diag(AL.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< AL << /*agent memory arguments*/ 0;
return;
}

if (!D->hasAttr<SYCLIntelMemoryAttr>())
D->addAttr(SYCLIntelMemoryAttr::CreateImplicit(
S.Context, SYCLIntelMemoryAttr::Default));
Expand Down Expand Up @@ -8005,6 +8105,15 @@ void Sema::AddSYCLIntelBankBitsAttr(Decl *D, const AttributeCommonInfo &CI,
D->addAttr(SYCLIntelNumBanksAttr::CreateImplicit(Context, NBE));
}

// Check attribute applies to field, constant variables, local variables,
// static variables, agent memory arguments, non-static data members,
// and device_global variables.
if (CheckValidFPGAMemoryAttributesVar(*this, D)) {
Diag(CI.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< CI << /*agent memory arguments*/ 1;
return;
}

if (!D->hasAttr<SYCLIntelMemoryAttr>())
D->addAttr(SYCLIntelMemoryAttr::CreateImplicit(
Context, SYCLIntelMemoryAttr::Default));
Expand All @@ -8030,6 +8139,22 @@ void Sema::AddSYCLIntelPrivateCopiesAttr(Decl *D, const AttributeCommonInfo &CI,
<< CI << /*non-negative*/ 1;
return;
}

// Check attribute applies to field as well as const variables, non-static
// local variables, non-static data members, and device_global variables.
if (const auto *VD = dyn_cast<VarDecl>(D)) {
if (!(isa<FieldDecl>(D) ||
(VD->getKind() != Decl::ImplicitParam &&
VD->getKind() != Decl::NonTypeTemplateParm &&
VD->getKind() != Decl::ParmVar &&
(VD->hasLocalStorage() ||
isTypeDecoratedWithDeclAttribute<SYCLDeviceGlobalAttr>(
VD->getType()))))) {
Diag(CI.getLoc(), diag::err_fpga_attribute_invalid_decl) << CI;
return;
}
}

// Check to see if there's a duplicate attribute with different values
// already applied to the declaration.
if (const auto *DeclAttr = D->getAttr<SYCLIntelPrivateCopiesAttr>()) {
Expand Down Expand Up @@ -8080,6 +8205,15 @@ void Sema::AddSYCLIntelForcePow2DepthAttr(Decl *D,
return;
}

// Check attribute applies to field, constant variables, local variables,
// static variables, agent memory arguments, non-static data members,
// and device_global variables.
if (CheckValidFPGAMemoryAttributesVar(*this, D)) {
Diag(CI.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< CI << /*agent memory arguments*/ 1;
return;
}

// Check to see if there's a duplicate attribute with different values
// already applied to the declaration.
if (const auto *DeclAttr = D->getAttr<SYCLIntelForcePow2DepthAttr>()) {
Expand Down
Loading

0 comments on commit 3fc6708

Please sign in to comment.