hls4ml
hls4ml copied to clipboard
Remove #INLINE pragma to fix SeparableConv1D with Vitis backed
Fixes #897
Description
With Vitis backend, when #pragma HLS function_instantiate
is used, #pragma HLS INLINE
can only be used with the off
option to forbid inlining. Using only #pragma HLS INLINE
results in the error mentionned in #897.
From what I understand from https://docs.xilinx.com/r/2022.2-English/ug1399-vitis-hls/Function-Instantiation, removing this pragma does not means that inlining will not be performed when possible.
Type of change
- [x] Bug fix (non-breaking change that fixes an issue)
Tests
As this is tested during CSynth, no PyTest are provided, but the test script in #897 can be used to test that the problem is indeed fixed.
Checklist
- [x] I have read the guidelines for contributing.
- [x] I have commented my code, particularly in hard-to-understand areas.
- [x] My changes generate no new warnings.
- [x] I have installed and run
pre-commit
on the files I edited or added.
This won't get inlined automatically by the compiler, meaning +1 on latency of the inner product for every output pixel due to the extra function call. It's the function_instantiate
pragma that we should remove. That pragma is a leftover from the wild '10s of Vivado HLS where tricks like that were actual optimizations (note that there will only ever be one instance of this function since the weights array is fixed, so it is useless)
If function_instantiate
is removed, there is then a clash with #pragma HLS PIPELINE ...
. If this one is removed, the issue is solved, but again, not sure about the impact on resulting rtl.
Looking at the surrounding code there's already a inline recursive
pragma before each call to this function. Does that get ignored? Is inlining itself a problem (that messes with the pipeline of the depthwise_conv_1d_buffer_cl
that ultimately calls this) or is the issue the existence of additional pipeline directive in the depthwise_product
(which should be ignored with a warning, but is promoted to an error in Vitis)? I think we should test if this works better inlined or not, and if not, prevent inlining by removing those pragmas (and perhaps even keeping the one in depthwise_product
but with off
). Just not with function_instantiate
:-)
To be honest, I have no clue about all this. I just have the impression from the error message that the multiple pragma are simple mutually exclusive with Vitis (obviously no issue with Vivado).
I am willing to dig a bit in all this and do some tests, but not sure when I will have the time to do it..