Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Native implementation of the wasm matrix multiply interface for Intel architecture #51

Closed
Tracked by #205
abhi-agg opened this issue Aug 18, 2021 · 3 comments
Closed
Tracked by #205
Assignees
Labels
enhancement New feature or request

Comments

@abhi-agg
Copy link

abhi-agg commented Aug 18, 2021

Feature description

As per browsermt/bergamot-translator#205, we would like to have an optimized implementation of the generic matrix multiply interface for Intel architecture

cc @andrenatal @kpu @XapaJIaMnu

@abhi-agg abhi-agg added the enhancement New feature or request label Aug 18, 2021
@abhi-agg abhi-agg changed the title A native matrix-multiply implementation of the wasm generic interface optimized for Intel SSSE3 architecture A native matrix-multiply implementation of the wasm matrix mulitply for Intel SSSE3 Aug 18, 2021
@abhi-agg abhi-agg changed the title A native matrix-multiply implementation of the wasm matrix mulitply for Intel SSSE3 Native implementation of the wasm matrix mulitply interface for Intel SSSE3 Aug 18, 2021
@kpu
Copy link
Member

kpu commented Aug 18, 2021

I remain confused as to the point of this exercise. We had a working matrix multiply interface. Every change is a tax on the working implementation. Yet the justification for coming in to do a redesign is very limited and it's not clear what advantage the new interface has. Why isn't this just comments on top of the existing one?

Index to int32_t ok fine I get that, easy. ARM I get but, as discussed, that's opaque to the user anyway and not a change to the interface. Why are we changing the order of arguments to functions?

If you want a working version of your interface, wrap the existing calls. The separation of somebody drawing up a spec and somebody implementing, which seems to be implied by this issue, just results in a spec that doesn't quite fit the implementation. And a lot more work on the implementation that I am struggling to understand or justify.

@abhi-agg
Copy link
Author

abhi-agg commented Aug 23, 2021

As per #49 (comment) and #49 (comment), I have clarified that the interface in #49 is not a redesign but a well documented version of intgemm interface catering to both intel and arm architecture.

@abhi-agg abhi-agg changed the title Native implementation of the wasm matrix mulitply interface for Intel SSSE3 Native implementation of the wasm matrix multiply interface for Intel SSSE3 Sep 3, 2021
@abhi-agg
Copy link
Author

abhi-agg commented Nov 16, 2021

We already have the native gemm implementation of this interface for intel with intgemm as the backend. So, closing this one. The landing is already tracked here: https://bugzilla.mozilla.org/show_bug.cgi?id=1722102

@abhi-agg abhi-agg changed the title Native implementation of the wasm matrix multiply interface for Intel SSSE3 Native implementation of the wasm matrix multiply interface for Intel architecture Feb 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants