implement gemm and memory_cache

[okdshin]

memory_cache is the base building block for mkldnn backend. It has multiple memories which may have different dims or memory format from each other but they share data contents.

[okdshin]

Thanks review. By using valgrind, I found this code (and other PRs fo other operators) have double free corruption code. I will modify them.

[msakai]

Do you want me to review added commits?

[okdshin]

I integrated this PR and PRs for other operators and tried to infer VGG16. I found this PR has some issues so I'll fix them. Please review then.

[okdshin]

I modified. Please review! @msakai

[msakai]

Sure.

[msakai]

Thanks review. By using valgrind, I found this code (and other PRs fo other operators) have double free corruption code. I will modify them.

Where was the "double free corruption code" and which commit fixed the problem?

[okdshin]

Actually I found two bugs. One is double free corruption here. Both op_output_memory and output_memory free the shared buffer. To fix it, I stopped to use mkldnn::memory for passing output_memory. I added formatted_array class to replace it and manage_output function to gather common lines in operators. Another bug is a conventional invalidated iterator bug. here, add_cached_memory(new_memory) caused vector's reallocation and invalidate found_memory const reference here.

[msakai]

Thank you for the explanation, but I'm a bit confused.

https://github.com/pfnet-research/menoh/pull/137/commits/08761182e11e07c31565cde9e5f1f71b90d6763f#diff-791f7ab18a103d6167001631aec9ec24L122 :

                    op_output_memory = mkldnn::memory(
                      {{{extract_dims(output_memory)},
                        extract_data_type(output_memory),
                        extract_format(gemm_pd.dst_primitive_desc())},
                       engine},
                      output_memory.get_data_handle());
                    output_memory_cache.add_cached_memory(*op_output_memory);

Here, memory(const primitive_desc &adesc, void *ahandle) is called.

Unlike memory(const primitive_desc &adesc), this constructor does not reset the _handle field. Therefore it seems that op_output_memory does not free the shared buffer?

[msakai]

Anyway, my confusion is about the past code and my concern might be irrelevant to the current code.

Current code itself looks good to me.

[okdshin]

You are correct. I misunderstood that. here seems the actual problem line. In this line, array is not stored so buffer is invalidated. (I can't understsand why that bug causes double free corruption tho). Anyway thank you for pointing out.

[okdshin]

Thank you for review @msakai !