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

[Snippets] Introduced BufferExpression #26413

Merged

Conversation

a-sidorova
Copy link
Contributor

@a-sidorova a-sidorova commented Sep 4, 2024

Details:

  • Created the first specialized expression: BufferExpression - now we don't have to cast the source node to op::Buffer to get attributes - all of them are stored in BufferExpression now
  • United NewMemoryBuffer and IntermediateMemoryBuffer ops into one Buffer since now they are distinguished only by argument count
  • Replaced the pass "SetBrgemmCopyBBuffersShape" with new "InsertBrgemmCopyBBuffers" which inserts BrgemmCopyB-specific BufferExpression after this op

Tickets:

  • 151198

@a-sidorova a-sidorova added this to the 2024.5 milestone Sep 4, 2024
@github-actions github-actions bot added the category: CPU OpenVINO CPU plugin label Sep 4, 2024
const char* get_type_name() const {
return get_type_info().name;
}

protected:
Expression(const Expression& other);
// Note: The constructor initialization is private since an expression can be created only by Linear IR.
// The method must be used only by Linear IR builder of expressions!
Expression(const std::shared_ptr<Node>& n, const std::shared_ptr<IShapeInferSnippetsFactory>& factory, bool need_shape_infer = true);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Open question for discussion: can we move this ctor to public section to avoid writing friend class ExpressionFactory; in each SpecialExpression 🤔

@a-sidorova a-sidorova force-pushed the feature/snippets/buffer_expr branch 2 times, most recently from 3a82f13 to b6daa85 Compare September 5, 2024 10:10
@a-sidorova a-sidorova marked this pull request as ready for review September 5, 2024 11:14
@a-sidorova a-sidorova requested review from a team as code owners September 5, 2024 11:14
src/common/snippets/src/lowered/expression.cpp Outdated Show resolved Hide resolved
src/common/snippets/src/lowered/expression.cpp Outdated Show resolved Hide resolved
src/common/snippets/src/op/buffer.cpp Outdated Show resolved Hide resolved
src/common/snippets/src/lowered/expression.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@v-Golubev v-Golubev left a comment

Choose a reason for hiding this comment

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

Good job 👍

src/common/snippets/include/snippets/op/buffer.hpp Outdated Show resolved Hide resolved
// IntermediateMemoryImpl represents intermediate memory.
// The buffers with this implementation must have source (parents)
class IntermediateMemoryImpl : public BaseImpl {
class ShapeInfer;
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: this class is used only in implementations, forward declaration is not required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, Looks like it's remainder after the previous implementations. Removed

src/common/snippets/include/snippets/op/buffer.hpp Outdated Show resolved Hide resolved
src/common/snippets/src/lowered/expression.cpp Outdated Show resolved Hide resolved
src/common/snippets/src/lowered/linear_ir.cpp Outdated Show resolved Hide resolved
src/common/snippets/src/lowered/linear_ir.cpp Outdated Show resolved Hide resolved
@a-sidorova
Copy link
Contributor Author

@IvanNovoselov your comments have been applied. May I ask you please to take a look one more time?

@IvanNovoselov IvanNovoselov added this pull request to the merge queue Sep 17, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 17, 2024
@a-sidorova a-sidorova added this pull request to the merge queue Sep 17, 2024
Merged via the queue into openvinotoolkit:master with commit 0217615 Sep 17, 2024
152 checks passed
@a-sidorova a-sidorova deleted the feature/snippets/buffer_expr branch September 17, 2024 11:28
github-merge-queue bot pushed a commit that referenced this pull request Oct 1, 2024
### Details:
 - *Implemented BrgemmCopyBExecutor*
 - *Added tests with dynamic input shapes for MatMul INT8/BF16*

### Tickets:
 - *151922*

### Prerequisites:
- [x] #26413
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: CPU OpenVINO CPU plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants