[Integration Test] Fix cordic

[murphe67]

Variable names in cordic were incorrect for putting data in the BRAM.

[A-Alosius]

Hi @murphe67
I noticed a similar issue with trisolv
There is a name mismatch (xArray instead of x) though that's only one of the two reasons why simulation fails

The second reason is the presence of a division operation on one of the lines

 x[i] = x[i] / A[i][i];

Interestingly, this is an issue only when the data type is int. I replaced all the ints with floats and simulation is successful. Do you mind looking into it as well? I will review my notes on the integration tests (I tried all of them one at a time to find patterns), redo the ones in the list of ignored tests and update you if that's fine with you

[murphe67]

Oh, nice catch, I'll fix that one also in this PR then!

Yes, I don't think we have any open-source integer division implementations, so those kernels only work with the proprietary IP cores from Vitis.

We do have open-source floating point division, so it is a huge choke point, but we would like to try fix this at some point.

[murphe67]

I just checked polybench, which is where we got trisolv from, and it uses double by default.

@Jiahui17 @lana555 what do you think of switching all the integer division integration tests to be double? integer division is kind of a strange thing to be doing in most of these kernels, I'd say we got here by mistake.

[A-Alosius]

Oh, nice catch, I'll fix that one also in this PR then!

Yes, I don't think we have any open-source integer division implementations, so those kernels only work with the proprietary IP cores from Vitis.

We do have open-source floating point division, so it is a huge choke point, but we would like to try fix this at some point.

Oh I see.
I ran some more test last night and here's what I've gotten

1. complexdiv : Fails because of absolute value function in if condition. Dynamatic doesn't support precompiled libraries so this line breaks the compilation

if (abs(br) >= abs(bi)) {...}

I made a custom absolute value replacement

br = (br < 0) ? br*-1 : br;
bi = (bi < 0) ? bi*-1 : bi;

if ((br) >= (bi)) {...}

Once this is fixed, compilation succeeds but simulation fails because of the division signs used on integers (once again). When replaced with floats, simulation succeeds

2. correlation_float: Fails because of sqrt function from the math library. Still the precompiled library issue

3. covariance: Every array must be read at least once (at least one load). This issue is discussed in issue [Backend] Chisel LSQ generator fails on LSQs with 0 load ports  #111.
   Specifically, the symmat array is never read from. When a dummy read is added, write-hdl is successful. Simulation fails, however, because of yet another division. When the read issue is fixed in covariance_float, simulation is successful.

4. dct : fails because of cos function from the math library (precompiled library)

5. external_integration: calls two functions within the target kernel. Firstly, calling other functions in the kernel is not yet supported by Dynamatic. Secondly, the functions here are declared but never defined.

6. path_profiling: The % operation is not available. It is mentioned in issue [export-vhdl] Missing VHDL entities for remainder operations  #33. Legacy Dynamatic had support for it.

7. while_loop_2: fails as long as array index is present within loop condition. This is probably a deeper issue with how Dynamatic handles this use case and I doubt it's related to the structure of the C code itself

[murphe67]

Thanks for the detailed notes!

Will add the while loop 2 passes with the at least one of the better buffering algorithms, so it is possibly something to do with the on-merge buffer approach.

[A-Alosius]

I was using fpl22 for while_loop_2. I switched to fpga20 and simulation was successful.
So on-merge and fpl22 failed but fpga20 succeeds