fix mkl batch gemm

[TaoLv]

Since the same m/n/k is used for all single gemms, so we can put all these gemms into one group of mkl batch gemm.

@yajiedesign @piiswrong please review again.

[TaoLv]

@sxjscience

[sxjscience]

Does it further improves the performance? Get Outlook for iOS<https://aka.ms/o0ukef>

…

________________________________ From: Tao Lv <[email protected]> Sent: Tuesday, June 26, 2018 1:54:17 PM To: dmlc/mshadow Cc: Xingjian SHI; Mention Subject: Re: [dmlc/mshadow] fix mkl batch gemm (#343) @sxjscience<https://github.com/sxjscience> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#343 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AE8D7pBiaGFwTzKQScC2htQLdNL3HVS4ks5uAcyJgaJpZM4U3R5z>.

[TaoLv]

@sxjscience I think performance should be same and I have verified that from mxnet level. The main purpose of this PR is for code refine and reducing some memory usage.

[sxjscience]

Yeah. I think it’s a nice improvement. Get Outlook for iOS<https://aka.ms/o0ukef>

…

________________________________ From: Tao Lv <[email protected]> Sent: Tuesday, June 26, 2018 4:02:51 PM To: dmlc/mshadow Cc: Xingjian SHI; Mention Subject: Re: [dmlc/mshadow] fix mkl batch gemm (#343) @sxjscience<https://github.com/sxjscience> I think performance should be same and I have verified that from mxnet level. The main purpose of this PR is for code refine and reducing some memory usage. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#343 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AE8D7oYXthl5KKYXlQtGpTLwkARLd1CJks5uAeqrgaJpZM4U3R5z>.

[xinyu-intel]

This commit may cause perf regression and we are working on it. Please do not merge now. Thanks！

[TaoLv]

Talked with @xinyu-intel offline. His performance regression was caused by other changes and not related to this PR. Code change here has passed MXNet operator unit test: https://github.com/apache/incubator-mxnet/blob/master/tests/python/unittest/test_operator.py#L2576.

I also used the following code to verify the performance change and got similar results before and after this PR:

import mxnet as mx
import numpy as np
import time

x = mx.sym.Variable('x')
y = mx.sym.Variable('y')
input_x = np.random.rand(200, 256, 256)
input_y = np.random.rand(200, 256, 512)

sym = mx.symbol.batch_dot(x, y).bind(mx.cpu(), {'x': mx.nd.array(input_x), 'y': mx.nd.array(input_y)})

start = time.time()
for i in range(1010):
    if i == 10:
        start = time.time()
    sym.forward(is_train=False)[0].asnumpy()

print((time.time() - start))

Is it okay to merge? @sxjscience @piiswrong

[sxjscience]

Why not simply using &m, &n, &k for p_m, p_n, p_k?

[TaoLv]

To make the code easier to understand. MKL_INT p_m[1] = {m}; means this batched GEMM only has one group and all GEMMs in this group share the same m value. Maybe in the future, we can extend this to MKL_INT p_m[2] = {m1, m2};. Then we have two groups in one batched GEMM and the first group has m1 while the second group has m2. Using &m in this API will hide this definition and make it a little confusing.

[TaoLv]

@sxjscience @piiswrong @eric-haibin-lin is this good to merge?

[pengzhao-intel]

@TaoLv could you rebase the code and run CI again?

[eric-haibin-lin]

Is there reference PR in MXNet to test the mshadow change end2end?