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

[AIE2P] Extend AIESubRegConstrainer for FIFO ops #286

Open
wants to merge 1 commit into
base: aie-public
Choose a base branch
from

Conversation

andcarminati
Copy link
Collaborator

@andcarminati andcarminati commented Jan 20, 2025

Covering also *InstrInfo required changes. Also includes some test refactoring.

Based on the original work from @gbossu and @khallouh.

Covering also *InstrInfo required changes.
@andcarminati andcarminati force-pushed the andreu.aie2p.tied.regs branch from 5b69bef to 070c32b Compare January 20, 2025 12:49
$qex2, $p1, $lf1, $r25, $dc1, $dc4 = VLDB_POP_704_3D $plfr1, $d0_3d, implicit-def $srfifo_uf
$qex2, $p1, $lf1, $r25, $dc2, $dc4 = VLDB_POP_704_3D $plfr1, $d0_3d, implicit-def $srfifo_uf
$qex2, $p1, $lf1, $r25, $dc3, $dc6 = VLDB_POP_704_3D $plfr1, $d0_3d, implicit-def $srfifo_uf
$x6, $p1, $lf1, $r25, $dc1, $dc5 = VLDB_POP_512_3D $p0, $lf0, $r24, $d1_3d, implicit-def $srfifo_uf
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you fix and uncomment the other tied_nok verifier tests? They were recently disabled by @mludevid in #266

@@ -94,6 +99,8 @@ bool AIE2PPassConfig::addRegAssignAndRewriteOptimized() {
if (AllocateMRegsFirst)
addPass(createGreedyRegisterAllocator(onlyAllocateMRegisters));
if (EnableStagedRA) {
addPass(createGreedyRegisterAllocator(onlyAllocatePLFRRegisters));
addPass(createAIESuperRegRewriter());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you extend staged-ra-rewrite.mir with a few cases for PLFR registers?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, it would be nice to have an end-to-end test that showcases the benefit of StagedRA for PLFR registers like we have one for 2D and 3D addressing registers? Maybe a reduced example from Conv2D?

Copy link
Collaborator

Choose a reason for hiding this comment

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

See llvm/test/CodeGen/AIE/aie2p/addr_1d2d3d.ll



---
name: test_composed_subreg
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have the same test in tie-subregs-pass-vld.mir. I suppose this is related to that other outdated comment. Maybe you should rebase?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They were actually moved/refactored, they are not duplicated.

%3:edc = COPY $r3
%4:ep = COPY $p0
%10:ed = REG_SEQUENCE %4, %subreg.sub_mod, %1, %subreg.sub_dim_size, %2, %subreg.sub_dim_stride, %3, %subreg.sub_dim_count
$x11, %471:eps, %21:eldfiforeg, %22:erf2, %474:edc = VLDA_POP_512_2D %100, %101, %102, %10, implicit-def $srfifo_uf
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have the same tests in tie-subregs-pass-vld.mir. Could you update those instead or remove them?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They were actually moved/refactored, they are not duplicated.

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.

2 participants