Enable the case N != ldc in EigenBlasGemm. #5976
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Xreki
force-pushed
the
fix_stride_eigen
branch
from
7306a34 to
6040f3a
Compare
… temporary memory.
hedaoyuan
requested changes
Nov 28, 2017
paddle/math/Allocator.h
Outdated
| #include <mutex> | ||
| #include "hl_gpu.h" | ||
| #include "paddle/utils/Logging.h" | ||
| #ifdef PADDLE_WITH_CUDA |
There was a problem hiding this comment.
WITH_GPU=OFF下编译的时候会include hl_cuda_stub.h；下面这些代码中的宏应该是不需要加也能正确编译的？
There was a problem hiding this comment.
加这个是因为，EigenGemm.cpp中引入了#include "paddle/math/MemoryHandle.h"，从而会间接地引入#include "hl_base.h"，这个头文件中定义了using real float，会导致Eigen的编译问题。
paddle/function/EigenGemm.cpp
Outdated
| sizeC[1] = N; | ||
| CHECK_EQ(N, ldc); | ||
| T* gemmC = C; | ||
| if (N != ldc) { |
There was a problem hiding this comment.
这样的fix方式，会给以后带来，性能风险（而且，不熟悉这段代码的人也不容易知道这里有性能问题）。可以，看一下Eigen有没有别的表达方式，可以直接支持stride参数的。
There was a problem hiding this comment.
我查过了，Eigen的stride方式不适合这种情况。
Xreki
force-pushed
the
fix_stride_eigen
branch
from
f41e4cd to
16bdb47
Compare
hedaoyuan
reviewed
Nov 29, 2017
paddle/function/EigenGemm.cpp
Outdated
| Eigen::DefaultDevice device; | ||
| if (alpha == T(1) && beta == T(0)) { | ||
| c.device(device) = a.contract(b, dims); | ||
| c.slice(offsetC, extentC).device(device) = a.contract(b, dims); |
There was a problem hiding this comment.
考虑两种情况：
ldc_1 > Nand use the operationc.slice(offsetC, extentC).device(device) = a.contract(b, dims);ldc_2 == Nand use the operationc.device(device) = a.contract(b, dims);
1和2中的MNK是一样的，但是1中的ldc > N，2中的ldc == N，这两种情况下分别采用这两个不同的计算方式表现出来的性能（gflops）是一样的吗？
There was a problem hiding this comment.
Done. 这样代码显得有点长（重复）了。后面来测下耗时。
hedaoyuan
approved these changes
Nov 30, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #5997