This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Remove legacy TA/TU pseudo distinction for VID
ClosedPublic

Authored by reames on Jun 16 2023, 10:41 AM.

Details

Summary

This is a follow on to D152740. The focus of this patch is on actually removing the old TA (unsuffixed) version. I realized we already had plumbing for combined TA/TU pseudos - used by some of the ternary instructions. As such, we can go ahead and fully remove the old TA, and rename the _TU variant to be unsuffixed. (The rename must happen in this patch for the table structure to work out as expected.)

The scheduling difference comes from an omission in D152740. If we selected a _MASK variant - either from manual ISEL or instrincs - we were going through doPeepholeMaskedRVV and still getting the TA variant. The use of the IsCombined flag in the MaskedPseudo table causes us to use the TU (now unsuffixed) variant instead.

Diff Detail

Event Timeline

reames created this revision.Jun 16 2023, 10:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 16 2023, 10:41 AM
reames requested review of this revision.Jun 16 2023, 10:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 16 2023, 10:41 AM
reames abandoned this revision.Jun 16 2023, 10:41 AM

Ignore for now, uploaded from patch and have to fix once back from running an errand.

reames updated this revision to Diff 532307.Jun 16 2023, 3:25 PM

Reopen with corrected patch

craig.topper added inline comments.Jun 17 2023, 7:11 PM
llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
477–489

psuedu.-> pseudo

485

Why does the second sentence start with "If"? Do some instructions in this category not have a policy operand?

1821

Can we wrap this closer to 80 columns?

4311

I think the undef in this pattern will become IMPLICIT_DEF on its own. Do we need this pattern?

craig.topper added inline comments.Jun 21 2023, 9:53 PM
llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
930

Line this up with RegClass:$merge on the previous line. That's usually how I write multiline operand lists.

reames updated this revision to Diff 533800.Jun 22 2023, 3:16 PM

Address review comments

craig.topper added inline comments.Jun 22 2023, 11:06 PM
llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
477–489

pseudu -> pseudo

4321

Didn't we just add a TU_MU defvar? I thought we were moving to shorter name.

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-fmf.ll
12 ↗(On Diff #533800)

Is this change related?

craig.topper retitled this revision from [RISCV] Remove legacy TA/TU pseudo distiction for VID to [RISCV] Remove legacy TA/TU pseudo distinction for VID.Jun 22 2023, 11:37 PM
reames added inline comments.Jun 26 2023, 12:56 PM
llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
477–489

I really don't seem to be able to spell in this changeset...

4321

We did. I'd just written this code in a different order than I'd posted patches and forgot to update.

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-fmf.ll
12 ↗(On Diff #533800)

Nope, spurious regen change. Separate and landed.

reames updated this revision to Diff 534724.Jun 26 2023, 1:00 PM

Address review comments.

This revision is now accepted and ready to land.Jun 26 2023, 3:55 PM
This revision was automatically updated to reflect the committed changes.