[GPU] Fix int8 cache in onednn convolution #30606
Conversation
| strides, dilates, padding_l, padding_r, | ||
| *_attrs.get()); | ||
| _pd = *prim_desc; | ||
| dnnl::memory::desc bias_md = nullptr; |
There was a problem hiding this comment.
nit: dnnl::memory::desc bias_md; will be more explicit for zero_md according to onednn api.
There was a problem hiding this comment.
For convolution_onednn, there is a constructor A that does not receive bias as a parameter and a constructor B that does. Eventually, A calls B, and at this time, it explicitly puts nullptr in the bias part. In this situation, how do you think about this?
There was a problem hiding this comment.
My point is to replace
dnnl::memory::desc bias_md = nullptr; to dnnl::memory::desc bias_md; for explicit use of API. They seems to be basically same.
There was a problem hiding this comment.
Actually, I understood and tried as you suggested. I couldn't prove perfectly why, but the cache works properly only when nullptr is explicitly assigned. I didn't compare memory stack both when nullptr is explicitly assigned and when it isn't, using any specific tool. However, I confirmed through many tests that cache works properly only when nullptr is explicitly assigned.
src/plugins/intel_gpu/src/graph/impls/onednn/convolution_onednn.cpp
Outdated
Show resolved
Hide resolved
| dnnl::prop_kind::forward_inference, dnnl::algorithm::convolution_direct, | ||
| input_md, weights_md, bias_md, output_md, | ||
| strides, dilates, padding_l, padding_r, | ||
| *_attrs.get()); |
There was a problem hiding this comment.
The behavior seems exactly same as below. Am I missing something?
There was a problem hiding this comment.
I doubt the issue is hidden by compilation result change, but the real issue is unknown..
davidsnam-intel
force-pushed
the
david/fix-convolution-onednn--int8-cache
branch
from
226b3f2 to
0278c1d
Compare
Details:
Tickets: