Skip to content

Commit

Permalink
[SYCL] Refactor FPGA Memory Attributes Validation for Clarity and and…
Browse files Browse the repository at this point in the history
… Efficiency (#14566)

This commit refactors the CheckValidFPGAMemoryAttributesVar function in
SemaDeclAttr.cpp to improve the clarity and efficiency of the validation
logic for 11 FPGA memory attributes on variables. The updated
implementation streamlines the condition checks, making the code easier
to read and understand.

Key changes include:
    - Simplifying the conditional logic to reduce complexity.
- Using direct return statements for immediate feedback on validation
status.
- Enhancing readability by clearly separating checks for different
variable types and attributes.
    
This refactor ensures that the function more clearly expresses its
intent, making the codebase more accessible for future development and
optimizations.

No functional changes are intended with this refactor; it aims solely at
improving code quality and readability.

Signed-off-by: Soumi Manna <soumi.manna@intel.com>

---------

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
  • Loading branch information
smanna12 committed Jul 17, 2024
1 parent a3a8aa1 commit 3ae9361
Showing 1 changed file with 48 additions and 38 deletions.
86 changes: 48 additions & 38 deletions clang/lib/Sema/SemaDeclAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6973,19 +6973,40 @@ static bool checkForDuplicateAttribute(Sema &S, Decl *D,
// 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.SYCL().isTypeDecoratedWithDeclAttribute<SYCLDeviceGlobalAttr>(
VD->getType()) ||
VD->getType().isConstQualified() ||
VD->getType().getAddressSpace() == LangAS::opencl_constant ||
VD->getStorageClass() == SC_Static || VD->hasLocalStorage())))) {
return true;
}
// Check for SYCL device compilation context.
if (!S.Context.getLangOpts().SYCLIsDevice) {
return false;
}
return false;

const auto *VD = dyn_cast<VarDecl>(D);
if (!VD)
return false;

// Exclude implicit parameters and non-type template parameters.
if (VD->getKind() == Decl::ImplicitParam ||
VD->getKind() == Decl::NonTypeTemplateParm)
return false;

// Check for non-static data member.
if (isa<FieldDecl>(D))
return false;

// Check for SYCL device global attribute decoration.
if (S.SYCL().isTypeDecoratedWithDeclAttribute<SYCLDeviceGlobalAttr>(
VD->getType()))
return false;

// Check for constant variables and variables in the OpenCL constant
// address space.
if (VD->getType().isConstQualified() ||
VD->getType().getAddressSpace() == LangAS::opencl_constant)
return false;

// Check for static storage class or local storage.
if (VD->getStorageClass() == SC_Static || VD->hasLocalStorage())
return false;

return true;
}

void Sema::AddSYCLIntelNoGlobalWorkOffsetAttr(Decl *D,
Expand Down Expand Up @@ -7069,9 +7090,8 @@ 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
// for the device compilation.
if (S.Context.getLangOpts().SYCLIsDevice &&
((D->getKind() == Decl::ParmVar) ||
CheckValidFPGAMemoryAttributesVar(S, D))) {
if ((D->getKind() == Decl::ParmVar) ||
CheckValidFPGAMemoryAttributesVar(S, D)) {
S.Diag(AL.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< AL << /*agent memory arguments*/ 0;
return;
Expand Down Expand Up @@ -7103,9 +7123,8 @@ 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
// for the device compilation.
if (S.Context.getLangOpts().SYCLIsDevice &&
((D->getKind() == Decl::ParmVar) ||
CheckValidFPGAMemoryAttributesVar(S, D))) {
if ((D->getKind() == Decl::ParmVar) ||
CheckValidFPGAMemoryAttributesVar(S, D)) {
S.Diag(AL.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< AL << /*agent memory arguments*/ 0;
return;
Expand Down Expand Up @@ -7158,8 +7177,7 @@ static void handleSYCLIntelMemoryAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
// Check attribute applies to field, constant variables, local variables,
// static variables, agent memory arguments, non-static data members,
// and device_global variables for the device compilation.
if (S.Context.getLangOpts().SYCLIsDevice &&
CheckValidFPGAMemoryAttributesVar(S, D)) {
if (CheckValidFPGAMemoryAttributesVar(S, D)) {
S.Diag(AL.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< AL << /*agent memory arguments*/ 1;
return;
Expand Down Expand Up @@ -7187,9 +7205,8 @@ 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
// for the device compilation.
if (S.Context.getLangOpts().SYCLIsDevice &&
((D->getKind() == Decl::ParmVar) ||
CheckValidFPGAMemoryAttributesVar(S, D))) {
if ((D->getKind() == Decl::ParmVar) ||
CheckValidFPGAMemoryAttributesVar(S, D)) {
S.Diag(A.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< A << /*agent memory arguments*/ 0;
return;
Expand Down Expand Up @@ -7233,8 +7250,7 @@ void Sema::AddSYCLIntelBankWidthAttr(Decl *D, const AttributeCommonInfo &CI,
// Check attribute applies to field, constant variables, local variables,
// static variables, agent memory arguments, non-static data members,
// and device_global variables for the device compilation.
if (Context.getLangOpts().SYCLIsDevice &&
CheckValidFPGAMemoryAttributesVar(*this, D)) {
if (CheckValidFPGAMemoryAttributesVar(*this, D)) {
Diag(CI.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< CI << /*agent memory arguments*/ 1;
return;
Expand Down Expand Up @@ -7327,8 +7343,7 @@ 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 for the device compilation.
if (Context.getLangOpts().SYCLIsDevice &&
CheckValidFPGAMemoryAttributesVar(*this, D)) {
if (CheckValidFPGAMemoryAttributesVar(*this, D)) {
Diag(CI.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< CI << /*agent memory arguments*/ 1;
return;
Expand Down Expand Up @@ -7404,8 +7419,7 @@ 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 for the device compilation.
if (S.Context.getLangOpts().SYCLIsDevice &&
CheckValidFPGAMemoryAttributesVar(S, D)) {
if (CheckValidFPGAMemoryAttributesVar(S, D)) {
S.Diag(AL.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< AL << /*agent memory arguments*/ 1;
return;
Expand Down Expand Up @@ -7440,8 +7454,7 @@ void Sema::AddSYCLIntelMaxReplicatesAttr(Decl *D, const AttributeCommonInfo &CI,
// Check attribute applies to field, constant variables, local variables,
// static variables, agent memory arguments, non-static data members,
// and device_global variables for the device compilation.
if (Context.getLangOpts().SYCLIsDevice &&
CheckValidFPGAMemoryAttributesVar(*this, D)) {
if (CheckValidFPGAMemoryAttributesVar(*this, D)) {
Diag(CI.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< CI << /*agent memory arguments*/ 1;
return;
Expand Down Expand Up @@ -7533,9 +7546,8 @@ static void handleSYCLIntelMergeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
// Check attribute applies to field, constant variables, local variables,
// static variables, non-static data members, and device_global variables
// for the device compilation.
if (S.Context.getLangOpts().SYCLIsDevice &&
((D->getKind() == Decl::ParmVar) ||
CheckValidFPGAMemoryAttributesVar(S, D))) {
if ((D->getKind() == Decl::ParmVar) ||
CheckValidFPGAMemoryAttributesVar(S, D)) {
S.Diag(AL.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< AL << /*agent memory arguments*/ 0;
return;
Expand Down Expand Up @@ -7629,8 +7641,7 @@ void Sema::AddSYCLIntelBankBitsAttr(Decl *D, const AttributeCommonInfo &CI,
// Check attribute applies to field, constant variables, local variables,
// static variables, agent memory arguments, non-static data members,
// and device_global variables for the device compilation.
if (Context.getLangOpts().SYCLIsDevice &&
CheckValidFPGAMemoryAttributesVar(*this, D)) {
if (CheckValidFPGAMemoryAttributesVar(*this, D)) {
Diag(CI.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< CI << /*agent memory arguments*/ 1;
return;
Expand Down Expand Up @@ -7732,8 +7743,7 @@ void Sema::AddSYCLIntelForcePow2DepthAttr(Decl *D,
// Check attribute applies to field, constant variables, local variables,
// static variables, agent memory arguments, non-static data members,
// and device_global variables for the device compilation.
if (Context.getLangOpts().SYCLIsDevice &&
CheckValidFPGAMemoryAttributesVar(*this, D)) {
if (CheckValidFPGAMemoryAttributesVar(*this, D)) {
Diag(CI.getLoc(), diag::err_fpga_attribute_incorrect_variable)
<< CI << /*agent memory arguments*/ 1;
return;
Expand Down

0 comments on commit 3ae9361

Please sign in to comment.