cucapra/dahlia

Partitioning arguments insert pragmas twice, once inside function arguments.

Closed this issue · 2 comments

sa2257 commented

Check this where pragmas are placed in the function arguments which shouldn't happen.

sampsyo commented

That looks bad! Here's the code you're referring to:

#pragma SDS data copy(m1[0:64][0:64])
void gemm(double m1[64][64]

#pragma HLS ARRAY_PARTITION variable=m1 cyclic factor=8 dim=2, double m2[64][64]

#pragma HLS ARRAY_PARTITION variable=m2 cyclic factor=8 dim=1, double prod[64][64]
) {
  
  #pragma HLS ARRAY_PARTITION variable=m1 cyclic factor=8 dim=2
  
  #pragma HLS ARRAY_PARTITION variable=m2 cyclic factor=8 dim=1

These should just go in the body, right?

At the risk of stating the obvious, the problem here is that the double m2[64][64] declaration is on the same line as the #pragma for m1, so the compiler can't see it.

rachitnigam commented

Bug in the code gen phase (damn, that module is not well written :-( ).

Issue across two files: CppLike uses the emitDecl function to generate C++ types for the arguments. emitDecl uses emitArrayDecl which is overridden in VivadoBackend to generate a banking pragma after an array (this is used to support local arrays) which means bank pragmas are generated for arguments too.

The core issue is that pragmas are generated differently for function argument types than they are for local variables but the code generation treats them the same (by overriding the meaning of Decls in multiple contexts).

Feel free to take a hack at it if you want. If not, I'll fix it tonight. One good use case for @ViviYe's new backed is running a simple code validation pass on the run-tests every night to make sure the compiler isn't broken.

Longer term, the RTL backend will need testing through synthesis flows so this is somewhat worth thinking about.

  • 👍1