search

dillonhuff / clockwork

A polyhedral compiler for hardware accelerators
55 stars 11 forks source link

harris_sch1_onebuf compute units are incorrect #107

Open dillonhuff opened 3 years ago

dillonhuff commented 3 years ago

@jeffsetter I have written a compute unit regression test generator and used it to compare the outputs of the coreir and cpp compute units in isolation for each compute unit in harris_sch1_onebuf.

The C++ and coreir compute units hcompute_hw_output_stencil give different results for some inputs. For example:

ERROR in compute unit: hcompute_hw_output_stencil
in0_padded16_global_wrapper_stencil[0] -> 127
in0_padded16_global_wrapper_stencil[0] -> 0000000001111111
in0_padded16_global_wrapper_stencil[1] -> 35
in0_padded16_global_wrapper_stencil[1] -> 0000000000100011
in0_padded16_global_wrapper_stencil[2] -> 248
in0_padded16_global_wrapper_stencil[2] -> 0000000011111000
in0_padded16_global_wrapper_stencil[3] -> 41
in0_padded16_global_wrapper_stencil[3] -> 0000000000101001
in0_padded16_global_wrapper_stencil[4] -> 248
in0_padded16_global_wrapper_stencil[4] -> 0000000011111000
in0_padded16_global_wrapper_stencil[5] -> 164
in0_padded16_global_wrapper_stencil[5] -> 0000000010100100
in0_padded16_global_wrapper_stencil[6] -> 27
in0_padded16_global_wrapper_stencil[6] -> 0000000000011011
in0_padded16_global_wrapper_stencil[7] -> 19
in0_padded16_global_wrapper_stencil[7] -> 0000000000010011
in0_padded16_global_wrapper_stencil[8] -> 181
in0_padded16_global_wrapper_stencil[8] -> 0000000010110101
in0_padded16_global_wrapper_stencil[9] -> 202
in0_padded16_global_wrapper_stencil[9] -> 0000000011001010
in0_padded16_global_wrapper_stencil[10] -> 78
in0_padded16_global_wrapper_stencil[10] -> 0000000001001110
in0_padded16_global_wrapper_stencil[11] -> 232
in0_padded16_global_wrapper_stencil[11] -> 0000000011101000
in0_padded16_global_wrapper_stencil[12] -> 152
in0_padded16_global_wrapper_stencil[12] -> 0000000010011000
in0_padded16_global_wrapper_stencil[13] -> 50
in0_padded16_global_wrapper_stencil[13] -> 0000000000110010
in0_padded16_global_wrapper_stencil[14] -> 56
in0_padded16_global_wrapper_stencil[14] -> 0000000000111000
in0_padded16_global_wrapper_stencil[15] -> 224
in0_padded16_global_wrapper_stencil[15] -> 0000000011100000
in0_padded16_global_wrapper_stencil[16] -> 121
in0_padded16_global_wrapper_stencil[16] -> 0000000001111001
in0_padded16_global_wrapper_stencil[17] -> 77
in0_padded16_global_wrapper_stencil[17] -> 0000000001001101
in0_padded16_global_wrapper_stencil[18] -> 61
in0_padded16_global_wrapper_stencil[18] -> 0000000000111101
in0_padded16_global_wrapper_stencil[19] -> 52
in0_padded16_global_wrapper_stencil[19] -> 0000000000110100
in0_padded16_global_wrapper_stencil[20] -> 188
in0_padded16_global_wrapper_stencil[20] -> 0000000010111100
in0_padded16_global_wrapper_stencil[21] -> 95
in0_padded16_global_wrapper_stencil[21] -> 0000000001011111
in0_padded16_global_wrapper_stencil[22] -> 78
in0_padded16_global_wrapper_stencil[22] -> 0000000001001110
in0_padded16_global_wrapper_stencil[23] -> 119
in0_padded16_global_wrapper_stencil[23] -> 0000000001110111
in0_padded16_global_wrapper_stencil[24] -> 250
in0_padded16_global_wrapper_stencil[24] -> 0000000011111010
in0_padded16_global_wrapper_stencil[25] -> 203
in0_padded16_global_wrapper_stencil[25] -> 0000000011001011
in0_padded16_global_wrapper_stencil[26] -> 108
in0_padded16_global_wrapper_stencil[26] -> 0000000001101100
in0_padded16_global_wrapper_stencil[27] -> 5
in0_padded16_global_wrapper_stencil[27] -> 0000000000000101
in0_padded16_global_wrapper_stencil[28] -> 172
in0_padded16_global_wrapper_stencil[28] -> 0000000010101100
in0_padded16_global_wrapper_stencil[29] -> 134
in0_padded16_global_wrapper_stencil[29] -> 0000000010000110
in0_padded16_global_wrapper_stencil[30] -> 33
in0_padded16_global_wrapper_stencil[30] -> 0000000000100001
in0_padded16_global_wrapper_stencil[31] -> 43
in0_padded16_global_wrapper_stencil[31] -> 0000000000101011
in0_padded16_global_wrapper_stencil[32] -> 170
in0_padded16_global_wrapper_stencil[32] -> 0000000010101010
in0_padded16_global_wrapper_stencil[33] -> 26
in0_padded16_global_wrapper_stencil[33] -> 0000000000011010
in0_padded16_global_wrapper_stencil[34] -> 85
in0_padded16_global_wrapper_stencil[34] -> 0000000001010101
in0_padded16_global_wrapper_stencil[35] -> 162
in0_padded16_global_wrapper_stencil[35] -> 0000000010100010
in0_padded16_global_wrapper_stencil[36] -> 190
in0_padded16_global_wrapper_stencil[36] -> 0000000010111110
in0_padded16_global_wrapper_stencil[37] -> 112
in0_padded16_global_wrapper_stencil[37] -> 0000000001110000
in0_padded16_global_wrapper_stencil[38] -> 181
in0_padded16_global_wrapper_stencil[38] -> 0000000010110101
in0_padded16_global_wrapper_stencil[39] -> 115
in0_padded16_global_wrapper_stencil[39] -> 0000000001110011
in0_padded16_global_wrapper_stencil[40] -> 59
in0_padded16_global_wrapper_stencil[40] -> 0000000000111011
in0_padded16_global_wrapper_stencil[41] -> 4
in0_padded16_global_wrapper_stencil[41] -> 0000000000000100
in0_padded16_global_wrapper_stencil[42] -> 92
in0_padded16_global_wrapper_stencil[42] -> 0000000001011100
in0_padded16_global_wrapper_stencil[43] -> 211
in0_padded16_global_wrapper_stencil[43] -> 0000000011010011
in0_padded16_global_wrapper_stencil[44] -> 54
in0_padded16_global_wrapper_stencil[44] -> 0000000000110110
in0_padded16_global_wrapper_stencil[45] -> 148
in0_padded16_global_wrapper_stencil[45] -> 0000000010010100
in0_padded16_global_wrapper_stencil[46] -> 179
in0_padded16_global_wrapper_stencil[46] -> 0000000010110011
in0_padded16_global_wrapper_stencil[47] -> 175
in0_padded16_global_wrapper_stencil[47] -> 0000000010101111
in0_padded16_global_wrapper_stencil[48] -> 226
in0_padded16_global_wrapper_stencil[48] -> 0000000011100010
    coreir_result: 0
    cpp_result   : 255

The collateral for this test is here: https://github.com/dillonhuff/clockwork/tree/master/compute_unit_regressions/harris_sch1_onebuf/hcompute_hw_output_stencil

and you can reproduce the test failure by running this script: https://github.com/dillonhuff/clockwork/blob/master/compute_unit_regressions/harris_sch1_onebuf/hcompute_hw_output_stencil/run_hcompute_hw_output_stencil_regression.sh

Could you please fix this problem with the compute units?

jeffsetter commented 3 years ago

The verilog is incorrect. Changing all wires to wire signed fixes the issue.

dillonhuff commented 3 years ago

@jeffsetter thanks for looking into this!

@rdaly525 I assume this is a similar code generation issue to #108