[GitHub] [incubator-tvm] xqdan edited a comment on pull request #6443: [RELAY][OP] roi_align operator alter layout

[GitBox <gi...@apache.org>]

xqdan edited a comment on pull request #6443:
URL: https://github.com/apache/incubator-tvm/pull/6443#issuecomment-692519854


   
   
   > @xqdan @Beya2019 I'm not quite sure about whether we really need a NHWC layout topi implementation for roi_align on general purpose hardware, since IMO this should not become any major concern for performance. What @anijain2305 and I are suggesting is that we move the check for NCHW from relay type infer into op strategy for corresponding targets which don't have NHWC implementation.
   > 
   > I agree with @xqdan that topi implementation doesn't need to bind with relay layout. As a result, can we have some data showing the performance difference between NCHW and NHWC layouts roi_align for CPU and a new topi implementation is necessary?
   
   Maybe just remove NHWC layouts roi_align cpu op, and make sure relay type infer check for different target, make it easy for @Beya2019 , what do you think? @kevinthesun 


----------------------------------------------------------------
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:
users@infra.apache.org