[GitHub] ZhennanQin edited a comment on issue #13697: [MKLDNN] Enable signed int8 support for convolution.

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

ZhennanQin edited a comment on issue #13697: [MKLDNN] Enable signed int8 support for convolution.
URL: https://github.com/apache/incubator-mxnet/pull/13697#issuecomment-448973617
 
 
   @marcoabreu Looks like CI has trouble to build this PR. Some builds failed to compile this line
   https://github.com/apache/incubator-mxnet/blob/84131807d67bb6a256b78cd75bb12a274c22347b/include/mxnet/tensor_blob.h#L290
   with `comparison between signed and unsigned integer expressions`. But this PR doesn't touch that part, and looking into the code, both sides are returning size_t, which should be all same. Another prove is, clang can build pass with `-Wsign-compare`, which indicated that the gcc build has some problem. Can you have a look? Thanks.
   
   Update: I found the reason, the right side returns from mshadow::Shape<dimension>::Size(void), which is index_t.
   https://github.com/dmlc/mshadow/blob/696803bd7723ade8230af878460d96c68a550fbc/mshadow/tensor.h#L126
   So comparing size_t with index_t causing build failed. Question: why our CI didn't report this previously, and even not from other PRs sent at same time? Does ccache hide this issue? 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services