[SYCL][FPGA] Refactor [[intel::max_work_group_size()]] attribute implementation

[smanna12]

This patch

1. refactors FPGA function attribute [[intel::max_work_group_size()]]
   to better fit for community standards and separates the attribute from
   [[sycl::reqd_work_group_size()]] attribute implementation.

2. refactors the way we handled duplicate attributes and
   mutually exclusive attributes logic with when present on a given declaration.

3. handles redeclarations or template instantiations properly.

4. adds tests

5. Before the refactoring patch, we silently accepted this test case below:

struct TRIFuncObjBad {
[[intel::max_work_group_size(4, 4, 4)]] void
operator()() const;
};
[[intel::max_global_work_dim(0)]]
void TRIFuncObjBad::operator()() const {}

This PR fixes the bug and closes #5449

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

[smanna12]

I will refactor "reqd_work_group_size" in a separate patch to avoid big patch here for better code-reviews.

@AaronBallman, could you please provide some feedback on the implementation. I have separated support for "max_work_group_size" that was previously handled together with "reqd_work_group_size" attribute. Thanks

[premanandrao]

This is just a suggestion; you can decide if this reads better or if you want to retain the enum with three states better.

I am fine with it, but feel that this simplifies it at least for this case. Maybe keeping three states makes sense if this routine will generalized?

[smanna12]

This is just a suggestion; you can decide if this reads better or if you want to retain the enum with three states better.

I am fine with it, but feel that this simplifies it at least for this case. Maybe keeping three states makes sense if this routine will generalized?

Thanks @premanandrao for the suggestion. I agree with you that this simplifies it at least for this case and reads better. I do not think we need to have enum with three states here. One state is kind of unused. So, returning true/false from the static bool function would be sufficient for this case. I have named the static bool function "InvalidWorkGroupSizeAttrs" so that i can reuse this later for reqd_work_group_size attribute refactorization work that i plan to do it after this PR.

[premanandrao]

A fix to the comment, but otherwise looks good.

[smanna12]

Thank you for the reviews everyone!

[smanna12]

@bader, can this PR be merged? I have an approval and all checks have passed. Thanks

[bader]

@bader, can this PR be merged? I have an approval and all checks have passed. Thanks

Shouldn't we get approval from @AaronBallman?

[smanna12]

@bader, can this PR be merged? I have an approval and all checks have passed. Thanks

Shouldn't we get approval from @AaronBallman?

Sorry I missed that. Yes, Agreed

Ping @AaronBallman

[AaronBallman]

Assuming CI passes, LGTM

[smanna12]

Assuming CI passes, LGTM

Thanks Aaron for the reviews.

[smanna12]

Thank you for the reviews!