-
Notifications
You must be signed in to change notification settings - Fork 432
[WIP] Tensor shape overflow checking in Blas Engine #372
Conversation
mshadow/dot_engine-inl.h
Outdated
pp_C.push_back(C + i * m_n); | ||
} | ||
int m_k = 0; | ||
CHECK(mult_not_overflow(m,k, &m_k)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would a series call of this function cause runtime regression? Can we check this during debug mode?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's going to have some performance cost, this should be measured, but a set of divisions independent of tensor shapes shouldn't be too bad, it's basically O(1) perfomance penalty. Compared to the call to gemm I would guess it's minor, but should be measured.
* Uses division method | ||
*/ | ||
template<typename T> | ||
inline bool mult_not_overflow_binary(T a, T b, T *result = nullptr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use some built-in utility of gcc to check multiplication overflow: https://gcc.gnu.org/onlinedocs/gcc-5.2.0/gcc/Integer-Overflow-Builtins.html?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, can be an optimization if we are under gcc once the PR is working, there's some GPU problems I'm not sure if related to CI instability this weeks in GPU or a deeper problem.
This code base has been donated to the Apache MXNet project per #373, and repo is deprecated. Future development should continue in Apache MXNet. |
|
Fixes apache/mxnet#14522
With this change, trying to multiply large matrices with BLAS Engine will cause the following exception in the python code instead of crashes inside blas.