Reduce node checks avx2 accessability for jit impl #5658
Conversation
| @@ -1427,7 +1427,7 @@ void MKLDNNReduceNode::initSupportedPrimitiveDescriptors() { | |||
| Precision inputPrecision = getOriginalInputPrecisionAtPort(REDUCE_DATA); | |||
| Precision outputPrecision = getOriginalOutputPrecisionAtPort(0); | |||
|
|
|||
| jit_mode = (mayiuse(cpu::x64::sse41)) && getParentEdgeAt(REDUCE_DATA)->getDims().ndims() <= 5 && | |||
There was a problem hiding this comment.
What if the machine supports sse41 while not supporting avx2? This case jit_mode gets false. sse41 can't be utilized.
I think the original code here is right. Please also refer to the following comment: #580 (comment)
Thanks Yury!
There was a problem hiding this comment.
Thanks, @xuchen-intel, I agree, that such solution seems to be a workaround, but we still have to fix original bug in sse4.2 case. I found it located in reduce_main() and reduce_tail() functions, but still cannot found the separate 'illegal instruction'. May be, it doesn't connected with specific operator itself. Have you any ideas?
There was a problem hiding this comment.
I couldn't reproduce this problem with CPU-test. It passes on no-AVX2 machine.
There was a problem hiding this comment.
Thanks Yury! If you've already been working on the JIT details, maybe you can try to reproduce this problem by changing the following code.
Remove the condition branch of avx512 and avx2, so that it will be forced to utilize sse4.2 instruction.
Anyway, I'm OK with the workaround. And you can always to assign the issue to me, if you come across any problem.
There was a problem hiding this comment.
I tried to remove those conditions. It doesn't helps, so I have had to reproduce the problem with no-AVX machine via benchmark_app and GNMT model. Solution with additional (mayiuse(cpu::x64::avx2)) works good. I tried to reproduce it in special test for reduce_node to reassign it reasonably, but without result.
There was a problem hiding this comment.
Anyway, thanks for advice. I'll consult with our team about acceptability of workaround in given case.
|
Fixed in #5782 |
Details:
Tickets: