Skip to content

Commit

Permalink
Refactor FPGA Memory Attributes Validation for Clarity and and Effici…
Browse files Browse the repository at this point in the history
…ency

This commit refactors the CheckValidFPGAMemoryAttributesVar function in SemaDeclAttr.cpp to improve the clarity and efficiency of the validation logic for 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>
  • Loading branch information
smanna12 committed Jul 13, 2024
1 parent 76b7e48 commit 223a3ce
Showing 1 changed file with 35 additions and 38 deletions.
73 changes: 35 additions & 38 deletions clang/lib/Sema/SemaDeclAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6973,19 +6973,27 @@ 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;

// 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 +7077,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 +7110,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 +7164,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 +7192,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 +7237,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 +7330,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 +7406,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 +7441,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 +7533,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 +7628,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 +7730,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 223a3ce

Please sign in to comment.