Closed drankincms closed 1 year ago
The construct
if (condition) {
#pragma HLS ...
}
doesn't work on Vitis, so this fix will be ignored on that backend. Perhaps we can always unroll, and ensure the factor is correct if that is needed?
Ah dang, ok. @vloncar you're suggesting to just have the unroll factor be set to whatever it would have been from the pipeline pragma such that it's invisible in all cases except this one?
Ok, I have updated the PR to remove the
if (condition) {
#pragma HLS ...
}
constructs. I also noticed that the pipeline pragma should have a rewind
. If we would prefer this not be the case let me know and I can get rid of that change.
I tested this and it doesn't break Vitis backend. I have one question: you force the unroll of the loops that iterate over n_in
but not the ones that iterate over n_out
, why? These are automatically unrolled in your example (since they are 1 iteration only), but I'm not sure they would be in general case.
I checked this, and indeed we need to unroll the other loops as well. Also, it turns out we don't need the unroll factor, in case of RF=1 and PF=1, the unroll factor will be ignored, as it means complete unrolling. In cases of RF>1 the enclosing loop will be pipelined and this will control the unrolling, again ignoring the unroll factor.
@vloncar I was going to test your suggestion to confirm it is needed but it looks like you beat me to it :) Thanks a lot!
One question about removing the unroll factors: I had a concern that I didn't check that fully unrolling always might cause long synthesis times in the case where n_partitions
is large. Is this true? Or will the PIPELINE
pragma still cause them all to be unrolled anyway and so it doesn't matter?
In the tests of your model I didn't see this. The logs always have warning that the factor directive is ignored and all loops will be unrolled. There's no change in the synthesis results we just get rid of that warning.
Ok, great, this was my hunch but I hadn't gotten to check. 👍 Thanks!
Description
This is a small fix for 2D conv layers when
io_parallel
is used andParallelizationFactor
is set to the output size. Currently, this causes a pipeline pragma to be ignored which results in no unrolling and very large latency/II. This fix adds explicit unroll pragmas in this case to restore the expected behavior.Type of change
Tests
I have tested with the small model below:
It's not clear to me if this can be easily incorporated into a test. If this is necessary let me know and I will try a bit harder.
Checklist
pre-commit
on the files I edited or added.