cornell-zhang / heterocl

HeteroCL: A Multi-Paradigm Programming Infrastructure for Software-Defined Heterogeneous Computing
https://cornell-zhang.github.io/heterocl/
Apache License 2.0
322 stars 92 forks source link

Vivado HLS compilation fails for v0.3 with new function signature #194

Closed chhzh123 closed 3 years ago

chhzh123 commented 4 years ago

The new v0.3 generates flat arrays and passes in pointers in default. Vivado HLS cannot infer the size of the arrays, leading to a compile error.

I still use Tutorial: Back-end Support to show different generated code here.

// v0.2
void default_function(ap_int<32> A[10][10], ap_uint<4> B[8][8]) {
#pragma HLS array_partition variable=A complete dim=0
  for (ap_int<32> y = 0; y < 8; ++y) {
    for (ap_int<32> x = 0; x < 8; ++x) {
    #pragma HLS pipeline
      B[y][x] = ((ap_uint<4>)(((ap_int<33>)A[y][x]) + ((ap_int<33>)A[(y + 2)][(x + 2)])));
    }
  }
}

// v0.3
void default_function(ap_int<32>* A, ap_uint<4>* B) { // pass in pointers
                  #pragma HLS array_partition variable=A complete dim=0
  for (ap_int<32> y = 0; y < 8; ++y) {
    for (ap_int<32> x = 0; x < 8; ++x) {
    #pragma HLS pipeline
      B[(x + (y * 8))] = ((ap_uint<4>)(((ap_int<33>)A[(x + (y * 10))]) + ((ap_int<33>)A[((x + (y * 10)) + 22)])));
    }
  }
}

The new function signature results in the following error.

ERROR: [SYNCHK 200-61] ./simple_add.cpp:12: unsupported memory access on variable 'A.V' which is (or contains) an array with unknown size at compile time.
seanlatias commented 4 years ago

@Hecmay can you take a look at this?

hecmay commented 4 years ago

Sure. The recommended way to generate code is to use hcl.build(s, platform) instead of passing in strings. We should also update the document for v0.3 some time.

seanlatias commented 4 years ago

So what is wrong here?

seanlatias commented 4 years ago

The string one is no longer supported?

hecmay commented 4 years ago

It's still supported. I mean if you use the old interface, you need to dump the returned string into a file, manually write the TCL script to run HLS, and run viavdo_hls. Just more efforts, nothing else.

zhangzhiru commented 4 years ago

This sounds like another case where we should prompt error messages.

seanlatias commented 4 years ago

It's still supported. I mean if you use the old interface, you need to dump the returned string into a file, manually write the TCL script to run HLS, and run viavdo_hls. Just more efforts, nothing else.

Not sure if I understand. But the problem here is that it seems like the pointer arguments cause the errors. It doesn't matter if we are using strings or hcl.platform.

hecmay commented 4 years ago

Sure. That's the problem. I will fix the issue.

seanlatias commented 4 years ago

@chhzh123 Please check the fix and see if that solves the problem.

chhzh123 commented 4 years ago

Yes, it looks fine. But it means in v0.3, users need to manually flatten their tensors to 1D arrays before calling the function? As the signature depicts.

void default_function(ap_int<32> A[10*10], ap_uint<4> B[8*8]);
hecmay commented 4 years ago

The users do not need to manually flatten the array index. The reason of (automatic) index flattening is that sometimes Vivado HLS will throw out errors when accessing an array with more than 3 index dimensions. Not sure whether this issue has been fixed in the newer version. I will check it out.

hecmay commented 4 years ago

Similar issue as what I got: https://forums.xilinx.com/t5/High-Level-Synthesis-HLS/Can-HLS-handle-3-dimension-arrays/td-p/799164.

chhzh123 commented 4 years ago

The users do not need to manually flatten the array index. The reason of (automatic) index flattening is that sometimes Vivado HLS will throw out errors when accessing an array with more than 3 index dimensions. Not sure whether this issue has been fixed in the newer version. I will check it out.

206 changes back to multi-dimensional arrays. I have checked the functionality on Vivado HLS (v2018), and N-D arrays can be correctly synthesized.

zhangzhiru commented 4 years ago

@chhzh123 can you help add a test case under tests/issues/194?

chhzh123 commented 4 years ago

If we want to check the correctness of synthesis, we need to fix the mode of DUT first.

chhzh123 commented 4 years ago

I'll discuss with @Hecmay and bring up a solution for DUT. Then add csyn tests for this issue and the programs in tutorials like #233 .

hecmay commented 3 years ago

This should have been fixed in https://github.com/cornell-zhang/heterocl/pull/418. Test case added in tests/issues/test_issue_194.py. Please reopen the issue if the error still exists.