Page MenuHomePhabricator

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

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

Details

Summary

Add vector types to GPR and explict type to 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
959

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

982–983

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

991

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.Tue, May 4, 12:51 AM

Rebase

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

Rebase