Onednn: Does convolution backward descriptor really need a forward hint as parameter?

Created on 9 Oct 2017  路  7Comments  路  Source: oneapi-src/oneDNN

The need of hint is quite cumbersome and in fact this parameter seems never be active in convolution implementations. Besides, primitive_desc had no way to wrapper a nullptr_t.

So, is it possible just to remove it from interface (or add a new interface without a hint)?

enhancement mkl-dnn concepts

Most helpful comment

Absolutely agree that a separate primitive is required here.
But still there is something to think about, since de-convolution implementation would rely on the backward direct convolution (hence having the problem of asking the hint).

All 7 comments

Hi @4pao,

forward_hint is a standard way (interface) for all mkl-dnn backward primitives to verify they are compatible with the executed forward ones. At the moment this is crucial for batch-normalization, lrn, and pooling since different implementation may use different workspace layout (hence making impl-a be incompatible with impl-b). Even though backward convolutions do not depend on forward ones at the moment we cannot guarantee that would be always the case. In particular, for winograd implementation it is possible to reuse on backward pass some of the arrays computed on forward pass. That means we should be aware of what happened on that forward pass, and the only way to know is forward_hint...

Hi @emfomenk , thanks for the explanation.

There is a case that uses convolution backward data to implement deconvolution layer. If hint is mandatory, I have to create a dummy forward primitive descriptor which makes the implementation not that clean. So please consider the possibility that allows a empty hint.

Yeah... make sense, you are right.
Will think what we can do with that then.

In my completely uneducated opinion a separate primitive would make more sense. It could use same implementation internally. Use of backward computations this way is bending the semantics quite a bit.

Absolutely agree that a separate primitive is required here.
But still there is something to think about, since de-convolution implementation would rely on the backward direct convolution (hence having the problem of asking the hint).

Having 2 primitive creators, static assert in the hintless one could remind that a non-null hint is required. Some compilers can also warn about explicitly passing a nullptr hint (attribute nonnull).

Regardless of whether a second primitive creator is adopted, hints can still be given as nullptr right now to avoid the tedium of creating fwd descriptor just for hint. It seems checks on proper hint / private data workspace have been added to the various init() functions. So using nullptr hints might in some cases yield only a slow reference impl.

Thanks for the thread - I've been ignoring hint issues in last few days as I iterate over conv impls in benchdnn and now I can document why I can get away with lazy nullptr hints for now.

Resolving this issue should also include handling of hints for the primitive iterators.

Recently added deconvolution primitive addresses this issue.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

kruus picture kruus  路  6Comments

ratan picture ratan  路  6Comments

bhack picture bhack  路  7Comments

CaoZhongZ picture CaoZhongZ  路  6Comments

eric-haibin-lin picture eric-haibin-lin  路  6Comments