This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add vector types to GPR for P extension and explict type to existing codegen patterns
Needs ReviewPublic

Authored by craig.topper on Apr 12 2021, 12:10 AM.

Details

Summary

Add RVP vector types to GPR and explict type to existing codegen patterns which couldn't infer all types.

Diff Detail

Event Timeline

Jim created this revision.Apr 12 2021, 12:10 AM
Jim requested review of this revision.Apr 12 2021, 12:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 12 2021, 12:10 AM
This revision is now accepted and ready to land.Apr 12 2021, 9:50 AM

Surely some of this can be inferred by TableGen? This is rather disruptive (especially with my downstream hat on...).

Surely some of this can be inferred by TableGen? This is rather disruptive (especially with my downstream hat on...).

Tablegen was inferring all of these because GPR only contained 1 type in each HwMode. Once you add other types, it can't infer anymore. For RISCVISD nodes we can put XLenVT into the SDTypeProfiles, but for anything using target independent SDNodes, the SDTypeProfiles aren't strong enough. Most of the target independent nodes work on vectors so tablegen doesn't know if you're trying to pattern match vectors or scalars.

I asked for this change rather than adding a P specific GPR regclass because I don't think we should have 2 register classes with identical spill and size information and different legal type lists. MCRegisterInfo ends up thinking they are subclasses of each other which is kind of odd.

Jim added a comment.Apr 12 2021, 6:48 PM

Surely some of this can be inferred by TableGen? This is rather disruptive (especially with my downstream hat on...).

Tablegen was inferring all of these because GPR only contained 1 type in each HwMode. Once you add other types, it can't infer anymore. For RISCVISD nodes we can put XLenVT into the SDTypeProfiles, but for anything using target independent SDNodes, the SDTypeProfiles aren't strong enough. Most of the target independent nodes work on vectors so tablegen doesn't know if you're trying to pattern match vectors or scalars.

I asked for this change rather than adding a P specific GPR regclass because I don't think we should have 2 register classes with identical spill and size information and different legal type lists. MCRegisterInfo ends up thinking they are subclasses of each other which is kind of odd.

Thanks for interpretation.
I added explicit type one by one to make sure it's type is unable to be inferred by TableGen.

craig.topper added inline comments.Apr 13 2021, 11:13 AM
llvm/lib/Target/RISCV/RISCVInstrInfo.td
963

This should only need XLenVT on either $rs1 or $rs2. You don't need it on both.

986–987

I think you only need this on one of $lhs and $rhs and one of $truev and $falsev.

995

Should only need this on one of $rs1 and $rs2

craig.topper commandeered this revision.Apr 13 2021, 12:32 PM
craig.topper edited reviewers, added: Jim; removed: craig.topper.

Commandeering so I can rebase this after making changes to RISCVInstrInfoB.td that reduce the size of this patch.

This revision now requires review to proceed.Apr 13 2021, 12:32 PM

RISCVInstrInfoB.td makes more use of PatGpr* now so some of the changes are no longer needed.

Jim updated this revision to Diff 337312.Apr 13 2021, 7:09 PM

Remove XLenVT from $rs2

Jim added a comment.Apr 14 2021, 6:58 PM

Any comments?

Any comments?

I think it's worth having the whole stack ready before committing this in case later patches motivate changes to this one.

Any comments?

I think it's worth having the whole stack ready before committing this in case later patches motivate changes to this one.

Or at least the MC layer for P

Any comments?

I think it's worth having the whole stack ready before committing this in case later patches motivate changes to this one.

Or at least the MC layer for P

Can we commit the parts of this that aren't really P specific changes so they don't bit rot. Especially the changes to the V extension files.

Any comments?

I think it's worth having the whole stack ready before committing this in case later patches motivate changes to this one.

Or at least the MC layer for P

Can we commit the parts of this that aren't really P specific changes so they don't bit rot. Especially the changes to the V extension files.

I guess, though they don't make a huge deal of sense without P even if they're notionally not P-specific, and those could still bit-rot in the tree until P lands

Any comments?

I think it's worth having the whole stack ready before committing this in case later patches motivate changes to this one.

Or at least the MC layer for P

Can we commit the parts of this that aren't really P specific changes so they don't bit rot. Especially the changes to the V extension files.

I guess, though they don't make a huge deal of sense without P even if they're notionally not P-specific, and those could still bit-rot in the tree until P lands

I should probably just add a PatFrag to the V files and stop repeating the same copy pasted (XLenVT (VLOp (XLenVT GPR:$vl))) everywhere

Jim updated this revision to Diff 342666.May 4 2021, 12:51 AM

Rebase

Jim updated this revision to Diff 345339.May 13 2021, 8:10 PM

Rebase

Jim updated this revision to Diff 349819.Jun 4 2021, 4:53 AM

Rebase

Jim updated this revision to Diff 351149.Jun 10 2021, 6:04 AM

Rebase

Jim updated this revision to Diff 352098.Jun 15 2021, 5:04 AM

Rebase

Jim retitled this revision from [RISCV] Add vector types to GPR for P extension and explict type to codegen patterns to [RISCV] Add vector types to GPR for P extension and explict type to existing codegen patterns.Jun 15 2021, 5:06 AM
Jim edited the summary of this revision. (Show Details)
Jim updated this revision to Diff 353647.Jun 22 2021, 7:51 AM

Rebase

Jim updated this revision to Diff 356607.Jul 5 2021, 11:44 PM

Rebase

Jim updated this revision to Diff 360740.Jul 22 2021, 2:21 AM

Rebase

Jim updated this revision to Diff 363365.Aug 1 2021, 8:25 PM

Rebase

Jim updated this revision to Diff 365356.Aug 9 2021, 10:40 PM

Rebase

Jim updated this revision to Diff 407769.Feb 10 2022, 9:55 PM

Rebase

Jim updated this revision to Diff 433611.Jun 1 2022, 6:34 PM

Rebase

Herald added a project: Restricted Project. · View Herald TranscriptJun 1 2022, 6:34 PM

Hi @Jim , are you still working on this?

sunshaoce updated this revision to Diff 441923.Jul 2 2022, 11:45 PM

@Jim, we are working on the P extension, so I updated this patch.