Refactoring of MVAU and VVAU #963

mmrahorovic · 2024-01-26T11:52:24Z

This PR refactors the MVAU/VVAU HLS custom-op in a HW custom-op, HLS custom-op and RTL custom-op.

Depends on
2. Support for packed MV(A)Us: PR #794.
3. Support for multi-packed DSP58s for VVUs: PR #907

WIP

… mvu_8sx9

…compute kernel)

…x9_axi

…pute kernels

… and other minor changes

…d PyVerilator-related syntax change

…old layers to be by default true

…custom-op)

…e_to_rtl transformation for MVU

… for DSP48/DSP58

…ns for DSP48

…for MVU

…eusser

…or/mvu_vvu

… refactor/mvu_vvu

auphelia

Thanks @mmrahorovic ! Before I merge, could you please address the comments I made and update your PR?

auphelia · 2024-01-30T10:03:15Z

src/finn/builder/build_dataflow_steps.py

This change looks like it has nothing to do with the new class hierarchy. Could you provide more information on this or take it out of the PR, please?

That's correct, I'll remove it.
Nevertheless, do you think it's worth having this change upstream? My reasoning was that this particular transformation step (step_set_fifo_depths) should only affect the FIFOs as the name suggests. By passing an appropriate node_filter to the ApplyConfig transformation, we ensure that only StreamingFIFO-nodes are affected. This could potentially prevent confusion/bug when the folding config file has changed after the step_apply_folding_config step.
If so, I can create a separate PR with this change and a description.

auphelia · 2024-01-30T10:05:09Z

src/finn/custom_op/fpgadataflow/hlscustomop.py

HLSCustomOp gets deprecated with the refactored system. Could you please take this out of the PR and if it is a necessary change, could you incorporate it into the appropriate new class (HWCustomOp, HLSBackend or RTLBackend)?

Will do, thanks for the feedback!

auphelia · 2024-01-30T10:06:47Z

src/finn/custom_op/fpgadataflow/vectorvectoractivation.py

The title of the PR indicates that it also contains a refactoring of the VVAU, for this there would be more changes required than this added function. Could you either add these changes or factor the VVAU out for now and concentrate on the changes for the MVAU?

Good point, thanks! As discussed, I'll have both the VVAU/MVAU refactoring in one PR.

auphelia · 2024-01-30T10:09:09Z

src/finn/custom_op/fpgadataflow/hlsbackend.py

Thanks for updating the ap int max width bound!

auphelia · 2024-01-30T10:13:37Z

src/finn/custom_op/fpgadataflow/matrixvectoractivation.py

@@ -122,10 +133,14 @@ def get_nodeattr_types(self):
            # vector through the accelerator. This will get rid of any old
            # weight data from the weight FIFOs.
            "runtime_writeable_weights": ("i", False, 0, {0, 1}),
-        }
+            "preferred_impl_style" : ("s", False, "hls", {"hls", "rtl"}),


This node attribute doesn't need to be added here, when executing my_attrs.update(super().get_nodeattr_types()), it is automatically inherited from HWCustomOp. Please remove this. In general, when reviewing the node attributes, the HW abstraction layer for the MatrixVectorActivation should ideally only contain node attributes that the HLS and RTL variant of the MVAU share. Please ensure that this is the case. HLS- or RTL-specific attributes can be added in the _hls or _rtl node variants, like here: https://github.com/Xilinx/finn/blob/refactor/rtl_integration/src/finn/custom_op/fpgadataflow/rtl/streamingfifo_rtl.py#L50

Good point, I've removed it now. Thanks!

auphelia · 2024-01-30T10:22:27Z

src/finn/custom_op/fpgadataflow/matrixvectoractivation.py

@@ -165,6 +180,61 @@ def infer_node_datatype(self, model):
        odt = self.get_output_datatype()
        model.set_tensor_datatype(node.output[0], odt)

+    def get_input_datatype(self, ind=0):


There is a lot of code duplication. get_input_datatype, get_weight_datatype, get_output_datatype, ... are all again defined from line 510 onwards. Please remove repetitions.

auphelia · 2024-01-30T10:24:43Z

src/finn/custom_op/fpgadataflow/matrixvectoractivation.py

@@ -728,6 +751,43 @@ def get_hls_compatible_threshold_tensor(self, orig_thres_matrix):
        rows between PEs is not as expected (n_thres_steps)"""
        return ret.reshape(1, pe, tmem, n_thres_steps)

+    def get_hls_compatible_weight_tensor(self, orig_weight_matrix):


I see that you first deleted and then added this function again. For me this looks like an HLS-specific function and thus should be moved to matrixvectoractivation_hls.py, was there a reason to bring it back into the hw abstraction layer?

I think the name of the function is misleading. It suggests it's exclusively something that belongs to the HLS custom-op, but it's actually used in the make_weight_file method. That one is used both for the HLS and RTL custom-op and hence moved to the HW abstraction layer.
A more appropriate name would be: get_hw_compatible_weight_tensor. If you agree, I'll go on and rename that now.

auphelia · 2024-01-30T10:25:57Z

src/finn/custom_op/fpgadataflow/matrixvectoractivation.py

+            out_bias = -1 if odt_is_bipolar else self.get_nodeattr("ActVal")
+            result = multithreshold(result, mvau_thr, out_scale, out_bias)
+
+        context[node.output[0]] = result

    def code_generation_ipi(self):


This can be split into an HLS- and RTL-specific part and be moved into the HLS or RTL custom op files.

auphelia · 2024-01-30T10:27:15Z

src/finn/transformation/fpgadataflow/convert_to_hw_layers.py

+                            binaryXnorMode=0,
+                            noActivation=1,
+                            numInputVectors=list(mm_in_shape[:-1]),
+                            mem_mode=self.mem_mode,


As a direct result from my comments regarding the HW abstraction layer, you might need to remove some node attributes here.

auphelia · 2024-01-30T10:30:53Z

tests/fpgadataflow/test_fpgadataflow_mvau.py

For now it is fine to only test the HLS implementation of the MVAU.

auphelia · 2024-01-30T14:00:15Z

In general, this PR contains a lot of commits that are not relevant for the refactoring of the MVAU in HW abstraction layer and HLS variant. This might cause confusion and merge conflicts if you want to introduce these commits again at a later point in time.

… refactor/mvu_vvu

mmrahorovic · 2024-02-01T15:49:42Z

Due to many changes introduced to undo the starting point of the branch, a new branch/PR has been created to maintain a clean history. Please see: #971

mmrahorovic added 30 commits January 3, 2023 15:37

First changes to custom_op for RTL-based MVAU

be1503a

Merge remote-tracking branch 'upstream/dev' into feature/dsp_packing

8265985

[rtl custom op]: initial implementation of mvu_8sx9

afab9cd

[rtl custom op]: testbench for mvu_8sx9

a94fc3b

[rtl custom op]: initial implementation of flow control component for…

98f9acc

… mvu_8sx9

[rtl custom op]: implementation of replay buffer for mvu

96925a9

[rtl custom op]: testbench for mvu_8sx9_axi (including axi_wrapper & …

a3d1156

…compute kernel)

[rtl custom op]: initial implementation of verilog wrapper for mvu_8s…

2aea664

…x9_axi

Merge remote-tracking branch 'upstream/dev' into feature/dsp_packing

c92e4e3

[rtl mvu]: fix tab indentation

8b57849

[rtl custom op]: fix to indentation

5e61f42

[rtl custom-op]: minor changes for compiler integration

cbee193

[rtl custom op]: moved testbenches to separate directory

ba5e77b

[rtl custom op]: fixed output width to ACCU_WIDTH

69310b4

[rtl custom op]: renamed file and added generic to switch between com…

cfcff00

…pute kernels

[rtl custom op]: renamed file and added generic to switch between com…

72b5196

…pute kernels

[rtl mvu]: added behavioral model DSP58

c068bb6

[rtl mvu]: extended flow control wrapper with additional compute core…

18f94e7

… and other minor changes

[rtl mvu]: fix to done_len flag when SIMD dimension fully unrolled an…

6d4a0a7

…d PyVerilator-related syntax change

[rtl mvu tb]: updated testbench

90c547d

[builder]: added specialize_to_rtl step and changed standalone thresh…

0c37f1f

…old layers to be by default true

[builder]: added specialize_to_rtl step

5ccb016

[custom op]: added custom op MatrixVectorActivation_rtl

f099f4b

[custom op]: added additional attribute to enable conversion to RTL (…

9a3b0fd

…custom-op)

[custom op]: modified ip-stitching and code generation

38aa930

[tests]: initial version of unit test for RTL custom op and specializ…

4e44934

…e_to_rtl transformation for MVU

[rtl mvu]: specialized compute core for 4-bit weights and activations…

cc361d9

… for DSP48/DSP58

[rtl mvu]: specialized compute core for > 4-bit weights and activatio…

8eefb53

…ns for DSP48

[fpgadataflow transform]: initial specialize_to_rtl_layers-transform …

e7109e7

…for MVU

Merge remote-tracking branch 'upstream/dev' into feature/dsp_packing

d107b4d

mmrahorovic added 13 commits January 11, 2024 12:02

[hls custom-op]: enable reset in sim

4892d66

[test mvu rtl]: updated test flow (DSP58 only)

44f6e0f

[mvu vvu axi]: reworked flow control and backpressure handling by tpr…

9b2cceb

…eusser

Merge remote-tracking branch 'origin/feature/dsp_packing' into refact…

8b46c90

…or/mvu_vvu

[hlsbackend]: update limit HLS axi streams (8k-1)

4ab6596

Merge remote-tracking branch 'upstream/refactor/rtl_integration' into…

3011144

… refactor/mvu_vvu

Merge remote-tracking branch 'upstream/refactor/rtl_integration' into…

c3ce0c6

… refactor/mvu_vvu

[mvau hls]: refactored MVAU_hls custom_op

72ccc83

[refactor]: call to base_op_type method instead of custom_op type

7a9b82b

[hls custom-op]: add mvau_hls

4556d2d

[hw custom-op]: refactor MVAU

0b2fc98

[VVAU hw custom-op]: add base_op_type method

1a40e6a

[transform]: add transformation to infer MVAU hw custom-op

5e1ed9b

mmrahorovic changed the base branch from dev to refactor/rtl_integration January 26, 2024 12:21

mmrahorovic added 6 commits January 26, 2024 12:22

removed mvu rtl code to clean up PR

63c73c2

[test mvau]: modified to support new custom-ops

472ce11

removed rtl refactoring steps

31083d5

removed old rtl custom-op

0032743

removed old specialize_to_rtl transform

04ec562

removed rtl custom-op test

de778cf

auphelia requested changes Jan 30, 2024

View reviewed changes

mmrahorovic added 5 commits February 1, 2024 14:33

Merge remote-tracking branch 'upstream/refactor/rtl_integration' into…

4a47437

… refactor/mvu_vvu

[vvau hls]: add custom op to dict

911239c

[vvu hw-op]: refactored hw custom-op VVAU

b1ee540

[vvau hls-op]: refactored HLS custom-op VVAU

bc44a4d

[convert-to-hw]: added transformations to infer binary-MVAU and VVAU

faabc0f

mmrahorovic closed this Feb 1, 2024

mmrahorovic mentioned this pull request Feb 8, 2024

Refactoring of RTL MVAU/VVAU ops #977

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactoring of MVAU and VVAU #963

Refactoring of MVAU and VVAU #963

mmrahorovic commented Jan 26, 2024 •

edited

Loading

auphelia left a comment

auphelia Jan 30, 2024

mmrahorovic Feb 1, 2024 •

edited

Loading

auphelia Jan 30, 2024

mmrahorovic Feb 1, 2024

auphelia Jan 30, 2024

mmrahorovic Feb 1, 2024

auphelia Jan 30, 2024

auphelia Jan 30, 2024

mmrahorovic Feb 1, 2024

auphelia Jan 30, 2024

auphelia Jan 30, 2024

mmrahorovic Feb 1, 2024

auphelia Jan 30, 2024

auphelia Jan 30, 2024

auphelia Jan 30, 2024

auphelia commented Jan 30, 2024

mmrahorovic commented Feb 1, 2024

Refactoring of MVAU and VVAU #963

Refactoring of MVAU and VVAU #963

Conversation

mmrahorovic commented Jan 26, 2024 • edited Loading

auphelia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmrahorovic Feb 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

auphelia commented Jan 30, 2024

mmrahorovic commented Feb 1, 2024

mmrahorovic commented Jan 26, 2024 •

edited

Loading

mmrahorovic Feb 1, 2024 •

edited

Loading