This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][SVE2] Add the SVE2.1 pext and ptrue predicate-as-counter instructions
ClosedPublic

Authored by david-arm on Oct 25 2022, 6:10 AM.

Details

Summary

This patch adds the assembly/disassembly for the following instructions:

pext (predicate) : Set predicate from predicate-as-counter
ptrue (predicate-as-counter) : Initialise predicate-as-counter to all active

This patch also introduces the predicate-as-counter registers pn8, etc.

The reference can be found here:
https://developer.arm.com/documentation/ddi0602/2022-09

Diff Detail

Event Timeline

david-arm created this revision.Oct 25 2022, 6:10 AM
david-arm requested review of this revision.Oct 25 2022, 6:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 25 2022, 6:10 AM
llvm/lib/Target/AArch64/AArch64RegisterInfo.td
883

Given you specify the range does the _3b part provide any value? I'd rather just PPR_p8_p15 or even PPR_p8to15.

llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
72

I see this as more an SVE feature than SME so can this be SVEPredicateAsCounter?

david-arm updated this revision to Diff 470798.Oct 26 2022, 6:10 AM
  • Renamed isSMEPredicate... to isSVEPredicate...
  • Renamed PPR classes from PPR_3b_p8_p15 to PPR_p8to15
david-arm marked 2 inline comments as done.Oct 26 2022, 6:10 AM
sdesmalen added inline comments.Oct 26 2022, 9:18 AM
llvm/lib/Target/AArch64/AArch64RegisterInfo.td
883

It would be nice to also rename PPR_3b to PPR_p0to7 in that case (although preferably not as part of this patch)

llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
3590

nit: what does the int here mean?

llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
3890–3914

This code is currently untested by this patch.

Matt added a subscriber: Matt.Oct 26 2022, 12:15 PM
david-arm updated this revision to Diff 471053.Oct 27 2022, 1:22 AM
  • Renamed sve2p1_int_ctr_to_mask to sve2p1_pred_as_ctr_to_mask.
  • Removed unused code in tryParseSVEPredicateAsCounter
david-arm marked 2 inline comments as done.Oct 27 2022, 1:22 AM
david-arm added inline comments.
llvm/lib/Target/AArch64/AArch64RegisterInfo.td
883

Yep, I can do that in a follow-up!

sdesmalen accepted this revision.Oct 27 2022, 4:22 AM
This revision is now accepted and ready to land.Oct 27 2022, 4:22 AM
paulwalker-arm added inline comments.Oct 27 2022, 5:22 AM
llvm/lib/Target/AArch64/AArch64RegisterInfo.td
917

InvalidSVE?

921–938

Not sure how good of a suggestion this is but can a predicate-as-counter register be anything other than p8-p15? If no then what about dropping all the _p8to15 from these? We've got the PPR_p8to15 RegisterClass which is required, but all the other instances makes the code look a bit messy.

llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
1204

isSVEPredicateReg()?

2599–2606

Can these ever be hit? or does it help with diagnostics or something?

david-arm added inline comments.Oct 27 2022, 6:10 AM
llvm/lib/Target/AArch64/AArch64RegisterInfo.td
921–938

I think cntp takes a predicate-as-counter in the full range 0-15. See D136747

paulwalker-arm added inline comments.Oct 27 2022, 6:18 AM
llvm/lib/Target/AArch64/AArch64RegisterInfo.td
921–938

I see, I guess things like mov can be used to move the predicate-as-counter without actually using it. Thanks @david-arm.

david-arm updated this revision to Diff 471151.Oct 27 2022, 6:34 AM
  • Renamed InvalidSME... -> InvalidSVE...
  • Renamed isSMEVectorReg -> isSVEPredicateAsCounterReg
david-arm marked 2 inline comments as done.Oct 27 2022, 6:37 AM
david-arm added inline comments.
llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
1204

There is already a isSVEPredicateReg, so I've renamed this to isSVEPredicateAsCounterReg.

2599–2606

Yes this is hit even in the positive path due to this call chain:

tryParseVectorRegister -> matchRegisterNameAlias -> matchSVEPredicateAsCounterRegName
paulwalker-arm accepted this revision.Oct 27 2022, 7:40 AM
paulwalker-arm added inline comments.
llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
2

This random blank line has crept in.

This revision was landed with ongoing or failed builds.Oct 27 2022, 11:23 AM
This revision was automatically updated to reflect the committed changes.
david-arm marked an inline comment as done.
uabelho added inline comments.
llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCCodeEmitter.cpp
540

This method seems to be unused on trunk. gcc warns

../lib/Target/AArch64/MCTargetDesc/AArch64MCCodeEmitter.cpp:550:1: warning: 'uint32_t {anonymous}::AArch64MCCodeEmitter::EncodePPR_p8to15(const llvm::MCInst&, unsigned int, llvm::SmallVectorImpl<llvm::MCFixup>&, const llvm::MCSubtargetInfo&) const' defined but not used [-Wunused-function]

Will it be used or should it perhaps be removed?

MattDevereau added inline comments.
llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCCodeEmitter.cpp
540

Hello, thanks for reporting this. This was unused due to a patch I pushed recently at https://github.com/llvm/llvm-project/pull/65306. I've pushed another patch to remove this function: https://reviews.llvm.org/rG69ba4aed004d

uabelho added inline comments.Sep 25 2023, 6:04 AM
llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCCodeEmitter.cpp
540

Nice! Thanks!