This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add vendor-defined XTheadMAC (multiply-accumulate) extension
ClosedPublic

Authored by philipp.tomsich on Feb 12 2023, 9:18 AM.

Details

Summary

The vendor-defined XTHeadMAC (no comparable standard extension exists
at the time of writing) extension adds multiply accumulate 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=4041e11db3ec3611921d10150572a92689aa3154

Depends on D143439

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptFeb 12 2023, 9:18 AM
philipp.tomsich requested review of this revision.Feb 12 2023, 9:18 AM
craig.topper added inline comments.Feb 12 2023, 4:34 PM
llvm/lib/Target/RISCV/RISCVFeatures.td
491

Identation is off here

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

I think we usually use $rd_wb for the destination on tied instructions

360

You can probably use binop_allwusers<add> instead of (sext_inreg (add ...)). Similar for sub. Then add these instructions to the switch in RISCVDAGToDAGISel::doPeepholeSExtW with ADDIW/ADDW/etc.

363

Also better to use binop_allwusers here.

365

"Zbb" -> "Zbb or THeadBb"?

379

Same here

llvm/test/CodeGen/RISCV/attributes.ll
95

xtheadMAC -> xtheadmac

llvm/test/MC/RISCV/rv32xtheadmac-invalid.s
4

Add {{$}} to the end of the CHECK to make sure there is nothing else listed.

  • rework for review comments
  • update comment to not refer to sext_inreg
philipp.tomsich planned changes to this revision.Feb 13 2023, 2:20 PM

We plan to have a major rework tomorrow:

  • introduce a sexti16 complex pattern
  • reduce the number of individual patterns (handling the sext_inreg and shl+sra case together)
  • introduce sexti16 as a complex pattern (matches values that are sign-extended to an i16 and returns the underlying node --- i.e, what is being fed into the sign-extension)
  • refactors sexti32 to share implementation with sexti16 (similarily to what is implemented for zexti32)
  • simplifies XTHeadMac implementation by matching sexti16 instead of having multiple cases spelled out for shl+sra and sext_inreg
philipp.tomsich marked 8 inline comments as done.Feb 14 2023, 3:52 AM
craig.topper accepted this revision.Feb 14 2023, 10:31 AM

LGTM fix those formatting changes.

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

I think I missed it in previous reviews, but hasSideEffects should line up with Predicates on the previous line. That's the prevailing style in RISCVInstrInfo.td

94

(ins is over indented

This revision is now accepted and ready to land.Feb 14 2023, 10:31 AM
This revision was landed with ongoing or failed builds.Feb 14 2023, 11:26 AM
This revision was automatically updated to reflect the committed changes.
philipp.tomsich marked 2 inline comments as done.Feb 14 2023, 11:29 AM