[tosa] legalize tfl.fully_connected to tosa.conv2d
> This patch changes legalization of tfl.fully_connected to tosa.conv2d instead of tosa.fully_connected

Can you explain why this is preferred over matmul? From a computational standpoint a matmul and bias add seem more correct. > > This patch changes legalization of tfl.fully_connected to tosa.conv2d instead of tosa.fully_connected
> 
> Can you explain why this is preferred over matmul? From a computational standpoint a matmul and bias add seem more correct.

Matmul + add would have following shortcomings:
- To use MATMUL we would need to transpose the second matrix
- MATMUL does not support asymmetric types (like int16 x int8) allowed by FULLY_CONNECTED
- Bias addition for 48 bit accumulator (eg 16x8 operation) would require a 48-bit ADD which would be a new operation Hi @rdzhabarov Can you please review this PR ? Thank you! Hi @Tai78641 Can you please resolve conflicts? Thank you!
 This PR is stale because it has been open for 14 days with no activity. It will be closed if no further activity occurs. Thank you. > This PR is stale because it has been open for 14 days with no activity. It will be closed if no further activity occurs. Thank you.

resolved conflicts @NatashaKnk would you help review this? The code itself looks good, but I'm a bit concerned about this approach. From a clarity standpoint this seems confusing -- The first point of conversion between tfl and tosa should probably be as close of a 1:1 mapping we can get. Happy to review it as a later-on pass though. the motivation for this PR is that we want to deprecate tosa.fully_connected operator.
before we can do that, we wanted to remove its use in tf/tfl loweing to tosa, so that we wont break tf/tfl
when we removed tosa.fully_connected as an op. Therefore this PR. I think if fully_connected is being depricated, it's reasonable to expect that matmul would be able to pick up everything it was able to do before deprecation. It can later be converted into conv2d for optimization purposes, but mapping ftl directly to conv2d seems a bit hacky and confusing to me. The reasons we did not lower to Matmul (followed by an Add), are:

- To use MATMUL we would need to transpose the second matrix
- MATMUL does not support asymmetric types (like int16 x int8) allowed by FULLY_CONNECTED
- Bias addition for 48 bit accumulator (eg 16x8 operation) would require a 48-bit ADD which would be a new TOSA operation

thats why we chose the implementation in this PR I understand why this is the best approach with the existing set-up, I'm just not sure that looking at this PR in isolation is the best way to go about this. If there is no way to support existing fully_connected behavior through an op that is a reasonable default mapping of that op (which I don't consider Conv2D to be), the reasons for/approach to deprecation should be looked at. Hi @Tai78641 Can you please resolve conflicts? Thank you! > Hi @Tai78641 Can you please resolve conflicts? Thank you!

done
This patch changes legalization of tfl.fully_connected to tosa.conv2d instead of tosa.fully_connected