[SYCL][FPGA] Add Intel FPGA bank_bits attribute #876
Conversation
This attribute forces the memory system to implement the variable or struct member in a memory with 2^(N+1) banks, with (b0, ..., bn) forming the bank-select bits. Signed-off-by: Viktoria Maksimova <[email protected]>
| return 0; | ||
| } | ||
| static unsigned getMaxValue() { | ||
| return 1024*1024; |
There was a problem hiding this comment.
IMO, this limit should be documented. The current documentation doesn't imply this value, or, for that matter, any particular number of bits.
There was a problem hiding this comment.
As I see, the limits for another attributes were not documented, so I just followed the common documenting style :)
Signed-off-by: Viktoria Maksimova <[email protected]>
bader
changed the title
|
Overall LGTM. But I'm starting to be more and more confused with all these memory attribute magic and implicit code insertion. |
| /// The values must be consecutive values (i.e. 3,4,5 or 2,1). | ||
| static void handleIntelFPGABankBitsAttr(Sema &S, Decl *D, | ||
| const ParsedAttr &Attr) { | ||
| checkForDuplicateAttribute<IntelFPGABankBitsAttr>(S, D, Attr); |
There was a problem hiding this comment.
Add D decl validity check:
if (D->isInvalidDecl()) return;
There was a problem hiding this comment.
Are you sure we need this check? It seems that other attributes handling has no such check. I think that it is handled higher in the call stack.
There was a problem hiding this comment.
It depends on a contract we have with clang code:
- We trust function inputs;
- We don't trust function inputs.
There was a problem hiding this comment.
I don't have here a strong evidences to push anyone to follow my own preferences, but as far as I'm a pedantic person - I'd prefer to add this check at least in a code what I write.
There was a problem hiding this comment.
Quickly looking into the code I did not find such checks.
There was a problem hiding this comment.
I mean declaration invalidation checks before attribute handling earlier in the code.
This attribute forces the memory system to implement the variable or struct
member in a memory with 2^(N+1) banks, with (b0, ..., bn) forming the
bank-select bits.