Reductions are broken on Xilinx FPGAs

[JamieJQuinn]

Describe the bug When using a reduction (either manual dace.reduce or detected e.g. np.max) dace generates incorrect FPGA code which fails to compile with the error:

/lustre/home/nx08/nx08/jquinn/dace_iterative_solvers/.dacecache/broken_reduction_sym_1/src/xilinx/device/broken_reduction_sym_0_0.cpp: In function 'void broken_reduction_sym_0_0_0(const double*, double&, int)':
/lustre/home/nx08/nx08/jquinn/dace_iterative_solvers/.dacecache/broken_reduction_sym_1/src/xilinx/device/broken_reduction_sym_0_0.cpp:40:34: error: invalid initialization of reference of type 'double*&' from expression of type 'double'
   40 |         reduce_1_0_2(&__A_in[0], __result_out, N);
      |                                  ^~~~~~~~~~~~
/lustre/home/nx08/nx08/jquinn/dace_iterative_solvers/.dacecache/broken_reduction_sym_1/src/xilinx/device/broken_reduction_sym_0_0.cpp:10:47: note: in passing argument 2 of 'void reduce_1_0_2(const double*, double*&, int)'
   10 | void reduce_1_0_2(const double* _in, double*& _out, int N) {
      |                                      ~~~~~~~~~^~~~
gmake[2]: *** [CMakeFiles/broken_reduction_sym_1.dir/lustre/home/nx08/nx08/jquinn/dace_iterative_solvers/.dacecache/broken_reduction_sym_1/src/xilinx/device/broken_reduction_sym_0_0.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/broken_reduction_sym_1.dir/all] Error 2
gmake: *** [all] Error 2

To Reproduce Minimal example:

import dace
import numpy as np
from dace.transformation.interstate import FPGATransformSDFG

N = dace.symbol("N")

@dace.program
def broken_reduction_sym(A: dace.float64[N]):
    # result = np.min(A)
    result = dace.reduce(lambda a, b: a+b, A)

broken_reduction_sdfg = broken_reduction_sym.to_sdfg()
broken_reduction_sdfg.apply_transformations(FPGATransformSDFG)
broken_reduction = broken_reduction_sdfg.compile()

Expected behavior Reductions should produce code that compiles.

Additional context Dace version: 0.13.2 Vitis version: 2021.2 XRT version: 2.11.634 Python version: 3.9.7 Cmake version: 3.19.3 G++ version: 10.2.0

[krishnakumarg1984]

@TizianoDeMatteis A git bisect has revealed that this issue was introduced by the following commit:

commit 681a276fc80c4242246aaa778b46c89ecb907307 (refs/bisect/bad)
Date:   Fri Feb 4 12:56:33 2022 +0100

 `dace.reduce` replacement will now create scalars when the output has size `(1,)`.

The diff is:

diff --git a/dace/frontend/python/replacements.py b/dace/frontend/python/replacements.py
index e2aa04eb..139d49aa 100644
--- a/dace/frontend/python/replacements.py
+++ b/dace/frontend/python/replacements.py
@@ -196,7 +196,11 @@ def _reduce(pv: 'ProgramVisitor',
             output_subset = copy.deepcopy(input_subset)
             output_subset.pop(axis)
             output_shape = output_subset.size()
-        outarr, arr = sdfg.add_temp_transient(output_shape, sdfg.arrays[inarr].dtype, sdfg.arrays[inarr].storage)
+        if (len(output_shape) == 1 and output_shape[0] == 1):
+            outarr = sdfg.temp_data_name()
+            outarr, arr = sdfg.add_scalar(outarr, sdfg.arrays[inarr].dtype, sdfg.arrays[inarr].storage, transient=True)
+        else:
+            outarr, arr = sdfg.add_temp_transient(output_shape, sdfg.arrays[inarr].dtype, sdfg.arrays[inarr].storage)
         output_memlet = Memlet.from_array(outarr, arr)
     else:
         inarr = in_array

[tbennun]

@krishnakumarg1984 Nice find! So a scalar output for an FPGA kernel doesn't seem to produce correct code. @TizianoDeMatteis any thoughts?

[TizianoDeMatteis]

Need to look at this, but makes sense with the error reported above (mix between array and scalar). Thanks @krishnakumarg1984 for the finding!

[tbennun]

@JamieJQuinn @krishnakumarg1984 The issue should now be fixed in the latest master branch. Is it also resolved for you?

[krishnakumarg1984]

@tbennun Yes. Have tested the latest commit on master, and can confirm that the issue is now resolved for us. Thank you so much for helping to fix this.

[tbennun]

We are happy to help, closing the issue :)