[GitHub] [incubator-tvm] anijain2305 commented on issue #4249: [TOPI][AlterOpLayout][ARM] Enabling NHWC to NCHW layout transformation.
anijain2305 commented on issue #4249: [TOPI][AlterOpLayout][ARM] Enabling NHWC to NCHW layout transformation. URL: https://github.com/apache/incubator-tvm/pull/4249#issuecomment-55356 > Thanks for your detailed explanation @anijain2305 ! I missed you change of removing legalization. > > AFAIK, AlterOpLayout is not always enabled for different _opt level_, in which case the NHWC computing may encounter build failure. Not sure AlterOpLayout is a good solution for all cases. That is a good point. But, even Legalize is enabled at opt_level >= 1. AlterOpLayout is enabled at opt_level = 3. So, I think this is ok. As tq said, I think we should work on the Explicit layout Convertor Pass to solve the opt_level problem separately. I have some thoughts on that and take that task up. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] anijain2305 commented on issue #4249: [TOPI][AlterOpLayout][ARM] Enabling NHWC to NCHW layout transformation.
anijain2305 commented on issue #4249: [TOPI][AlterOpLayout][ARM] Enabling NHWC to NCHW layout transformation. URL: https://github.com/apache/incubator-tvm/pull/4249#issuecomment-552168343 @jackwish I agree that the the implementation is distributed in a long function right now. The reason (that I also got to know very recently while trying to use AutoTVM) is the tight relationship between AutoTVM and AlterOpLayout. In AutoTVM, we need to first extract the right kernels to tune (so we need to run AlterOpLayout to ensure that we have right layouts). And then after tuning is finished, we want to apply those tuned config. This again now needs to go through AlterOpLayout where we set up the config this time while also changing the layouts. Legalize and AlterOpLayout might seem very similar to each other, but they are somewhat different. Legalize is more like a local optimization where when we see a Relay operator, we can replace it with a sequence of another Relay operators. The link that you pasted for Legalize in ARM CPU was basically inserting layout transforms before and after each conv. AlterOpLayout, on the other hand, is somewhat of a global optimization, where the layouts are basically propagated as deep as possible before inserting the layout transform. Therefore, in this CR, I am removing the legalize and moving towards the AlterOpLayout. Once we insert the layout transform for first conv, it propagates it to next operator and so on. One way to reduce all this complexity is to write another pass that basically converts the whole graph from NHWC to NCHW (irrespective of target). One can call this pass before calling relay.build. In current scenario, where ARM only supports NCHW, that pass will be extremely useful. And then the AlterOpLayout remains unchanged. However, that pass is not yet finalized, and might take some time - https://discuss.tvm.ai/t/layout-conversion-pass/4009 In current implementation, I don't know of any better way to solve the problem. @yzhliu @kevinthesun Please let us know what you think. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] anijain2305 commented on issue #4249: [TOPI][AlterOpLayout][ARM] Enabling NHWC to NCHW layout transformation.
anijain2305 commented on issue #4249: [TOPI][AlterOpLayout][ARM] Enabling NHWC to NCHW layout transformation. URL: https://github.com/apache/incubator-tvm/pull/4249#issuecomment-552074719 @jackwish It will be good if you could review this as well :) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] anijain2305 commented on issue #4249: [TOPI][AlterOpLayout][ARM] Enabling NHWC to NCHW layout transformation.
anijain2305 commented on issue #4249: [TOPI][AlterOpLayout][ARM] Enabling NHWC to NCHW layout transformation. URL: https://github.com/apache/incubator-tvm/pull/4249#issuecomment-551966857 @FrozenGene I made it work. The workload had to be modified. I tried with my 2 conv network. It finished tuning, and also was able to apply both direct and winograd templates. Please review. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] anijain2305 commented on issue #4249: [TOPI][AlterOpLayout][ARM] Enabling NHWC to NCHW layout transformation.
anijain2305 commented on issue #4249: [TOPI][AlterOpLayout][ARM] Enabling NHWC to NCHW layout transformation. URL: https://github.com/apache/incubator-tvm/pull/4249#issuecomment-551951671 I tried to call AtlerOpLayout() before extracting the tasks, it was able to extract the NCHW convs and tune them. The problem is while applying them. I think while searching in logs, it is still using the old NHWC layout conv, and cant find one in the logs. So, fallsback to default. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
