Skip to content

Fix packed cache reuse across source layouts - #382

Open
vera-rubin wants to merge 1 commit into
google:masterfrom
vera-rubin:fix/cache-source-layout
Open

vera-rubin wants to merge 1 commit into
google:masterfrom
vera-rubin:fix/cache-source-layout

Conversation

@vera-rubin

Copy link
Copy Markdown

We omitted source layout, letting transposed LHS and RHS in Mul(A, A) share wrong packed operands and reuse stale entries after view order or stride changes. Keys retain source pointer, packed layout and zero point, adding source dimensions, order and stride to equality and hashing.

Regression checks self-product, changed views and repeated cache reuse using StandardCpp with one thread. Reproducer and regression failed at base. Both and five targeted existing CTest targets (caching, context, matrix layout, float and int8 multiplication) pass.

Version: base commit 2264753777198e4393fb83c44c693462d57a2be1, GCC 13.3.0 Release, Linux x86_64.

Fixes #381

— ChipLab

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the PrepackedCache key in the ruy library to include the source matrix's layout (MatLayout). This ensures that different source views (which might have the same packed layout but different packed values, such as transposed views or different strides) are correctly distinguished in the cache and do not incorrectly reuse cached entries. To support this, an equality operator was added for MatLayout, the cache key hash function was updated to incorporate the source layout, and a new test suite was added to verify cache behavior with different source layouts. There are no review comments to address, and I have no additional feedback to provide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Packed cache omits source layout and reuses the wrong operand for Mul(A, A)

1 participant