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] extract invariants pass #24044

Conversation

chenhu-wang
Copy link
Contributor

@chenhu-wang chenhu-wang commented Apr 16, 2024

Details:

  • extract invariants pass

Tickets:

Todo:

  • Add LIR correctness check test

@chenhu-wang chenhu-wang requested review from a team as code owners April 16, 2024 05:22
@github-actions github-actions bot added the category: CPU OpenVINO CPU plugin label Apr 16, 2024
@chenhu-wang chenhu-wang marked this pull request as draft April 16, 2024 05:22
@chenhu-wang chenhu-wang force-pushed the chenhu/snippets_extractLoopInvariants_lower_pass branch 4 times, most recently from 11e4faf to 166390a Compare April 25, 2024 06:03
@chenhu-wang chenhu-wang marked this pull request as ready for review April 25, 2024 06:07
@chenhu-wang chenhu-wang force-pushed the chenhu/snippets_extractLoopInvariants_lower_pass branch 3 times, most recently from d5594f2 to 4e018b8 Compare April 29, 2024 01:55
@a-sidorova a-sidorova self-assigned this May 3, 2024
@chenhu-wang chenhu-wang force-pushed the chenhu/snippets_extractLoopInvariants_lower_pass branch 3 times, most recently from 945d406 to 253ba95 Compare May 17, 2024 05:49
@chenhu-wang chenhu-wang force-pushed the chenhu/snippets_extractLoopInvariants_lower_pass branch 5 times, most recently from 62fc07b to d949778 Compare May 22, 2024 08:40
Copy link
Contributor

@a-sidorova a-sidorova left a comment

Choose a reason for hiding this comment

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

First part of review is completed

My major comment (suggestion): need to split code into semantic blocks: the separate functions.
When you see the body of the function with 200 code lines, it's very difficult to review and maintain 😢
In the best scenario the developer can read the code as a book by chapters (functions) 😃

@chenhu-wang chenhu-wang force-pushed the chenhu/snippets_extractLoopInvariants_lower_pass branch 3 times, most recently from ccbc9c7 to 015a5ae Compare May 31, 2024 08:51
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 21, 2024
@chenhu-wang chenhu-wang force-pushed the chenhu/snippets_extractLoopInvariants_lower_pass branch 2 times, most recently from ac972d6 to 17b1f22 Compare June 23, 2024 11:24
@chenhu-wang chenhu-wang force-pushed the chenhu/snippets_extractLoopInvariants_lower_pass branch from 17b1f22 to e5fef48 Compare June 24, 2024 01:36
@IvanNovoselov IvanNovoselov added this pull request to the merge queue Jun 24, 2024
Merged via the queue into openvinotoolkit:master with commit d6f12a3 Jun 24, 2024
111 checks passed
rkazants added a commit that referenced this pull request Jun 24, 2024
github-merge-queue bot pushed a commit that referenced this pull request Jun 24, 2024
Reverts #24044

Reverted due to failure in TF FE layer tests:
RuntimeError: Exception from
openvino\openvino\src\inference\src\cpp\core.cpp:107:
Exception from openvino\openvino\src\inference\src\dev\plugin.cpp:53:
Exception from
openvino\openvino\src\common\snippets\src\lowered\pass\assign_registers.cpp:315:
can't allocate registers for a snippet
allnes pushed a commit to allnes/openvino that referenced this pull request Jun 27, 2024
### Details:
 - *extract invariants pass*

### Tickets:
 - *CVS-136160*

### Todo:
- [x] Add LIR correctness check test
allnes pushed a commit to allnes/openvino that referenced this pull request Jun 27, 2024
Reverts openvinotoolkit#24044

Reverted due to failure in TF FE layer tests:
RuntimeError: Exception from
openvino\openvino\src\inference\src\cpp\core.cpp:107:
Exception from openvino\openvino\src\inference\src\dev\plugin.cpp:53:
Exception from
openvino\openvino\src\common\snippets\src\lowered\pass\assign_registers.cpp:315:
can't allocate registers for a snippet
github-merge-queue bot pushed a commit that referenced this pull request Jul 3, 2024
### Details:
- *recreation for PR
#24044
 - *fix post commit fail(will be added in pre commit soon)*

### Tickets:
 - *CVS-136160*
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.

4 participants