apeskov commented on PR #11966: URL: https://github.com/apache/tvm/pull/11966#issuecomment-1172302140
Thanks @billishyahao, nice patch! You've touched a common tam::relay code a little bit to enhance layouts support of packed dense op. There is one delicate nuance here and I would like to highlife it. "weight_layout" is arbitrary string like "NC", "CN", "CN8c", "CN16n4c" and any others. It should match with regex: `(NC|CN)([:digit:](c|n))*`. Will be perfect to have support all of these possible cases. Let's take a close look on next example. Dense with next shapes: data_shape `[128, 10]`, weight_shape `[17, 10]`, weight_layout `NC`, output_shape will be `[128, 17]`. Assume that we applies alter op layout and change layout to `NC8c`. Weight shape will be changed to `[3, 10, 8]`, with some additional padding. Unexpectedly, output shape will also be changed to `[128, 24]`. Weight layout conversion changes output shape, that's very strange behaviour. I know, `count` attribute should keep original size of output channels, but it can be `None`. So I recommend you to take into account `count` size in "MakeDensePack" implementation and propagate output shape correctly. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
