This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add vendor-defined XTheadCondMov (conditional move) extension
ClosedPublic

Authored by philipp.tomsich on Feb 23 2023, 3:01 PM.

Details

Summary

The vendor-defined XTheadCondMov (somewhat related to the upcoming
Zicond and XVentanaCondOps) extension add conditional move
instructions with $rd being an input and an ouput instructions.

It is supported by the C9xx cores (e.g., found in the wild in the
Allwinner D1) by Alibaba T-Head.

The current (as of this commit) public documentation for this
extension is available at:

https://github.com/T-head-Semi/thead-extension-spec/releases/download/2.2.2/xthead-2023-01-30-2.2.2.pdf

Support for these instructions has already landed in GNU Binutils:

https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=73442230966a22b3238b2074691a71d7b4ed914a

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptFeb 23 2023, 3:01 PM
philipp.tomsich requested review of this revision.Feb 23 2023, 3:01 PM
  • rebase after landing the MemIdx patches
craig.topper added inline comments.Feb 23 2023, 7:43 PM
llvm/docs/RISCVUsage.rst
190

mov -> Mov

llvm/lib/Target/RISCV/RISCVInstrInfoXTHead.td
649

As far as tablegen is concerned this pattern is the same as the pattern on line 643.

I think you need to make these instruction commutable and teach the commuting hooks in RISCVInstrInfo about it.

661

This pattern is identical to line 653.

663

This pattern is identical to line 655

679

Redundant

681

Redundant

697

Redundant

699

Redundant

719

Redundant

721

Redundant

I just posted D144700 that can help reduce the number of patterns needed.

llvm/lib/Target/RISCV/RISCVInstrInfoXTHead.td
661

Due to the register constraints ($rd being in/out) it should help to minimize unneeded register moves (my testing may have been biased, though).

Will the pattern at 653 always trigger, even if the register constraint on $rd make this one preferable?

I just posted D144700 that can help reduce the number of patterns needed.

Thanks! I had that on my list for a follow-up improvement, so you may have saved me some of the effort.

craig.topper added inline comments.Feb 23 2023, 11:49 PM
llvm/lib/Target/RISCV/RISCVInstrInfoXTHead.td
661

Yes the pattern at 653 will always trigger. The isel pattern matching doesn't know anything about the register constraint. Adding support to commuteInstruction will allow the TwoAddressInstruction pass to commute it if the tied operand is needed after the instruction, but the untied operand isn't needed.

  • rebased on top of D144700
  • added plumbing for the ISel framework to commute operands, where legal
philipp.tomsich marked 11 inline comments as done.Feb 24 2023, 6:18 AM
craig.topper added inline comments.Feb 24 2023, 10:36 AM
llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
2273

This line exceeds 80 characters

2275

This line has tab characters

craig.topper accepted this revision.Feb 24 2023, 10:50 AM

LGTM other than formatting fixes.

This revision is now accepted and ready to land.Feb 24 2023, 10:50 AM
This revision was landed with ongoing or failed builds.Feb 24 2023, 12:41 PM
This revision was automatically updated to reflect the committed changes.
llvm/test/CodeGen/RISCV/condops.ll