This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][SVE2] Add the SVE2.1 cntp instruction
ClosedPublic

Authored by david-arm on Oct 26 2022, 1:35 AM.

Details

Summary

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

cntp : Set scalar to count from predicate-as-counter

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

Diff Detail

Event Timeline

david-arm created this revision.Oct 26 2022, 1:35 AM
david-arm requested review of this revision.Oct 26 2022, 1:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 26 2022, 1:35 AM
tschuett added a comment.EditedOct 26 2022, 4:41 AM

Feel free to ignore me, but I see SME more often than SVE in this patch.

llvm/lib/Target/AArch64/AArch64SystemOperands.td
317

This an SVE patch?

Feel free to ignore me, but I see SME more often than SVE in this patch.

I agree, this is an SVE feature rather then SME. Sure SME implements it as part of streaming mode but that's just a different execution mode for SVE.

Matt added a subscriber: Matt.Oct 26 2022, 12:15 PM
david-arm updated this revision to Diff 471439.Oct 28 2022, 1:31 AM
  • Rebase.
  • Removed SME from the various names.
david-arm marked an inline comment as done.Oct 28 2022, 1:31 AM
paulwalker-arm added inline comments.Oct 30 2022, 6:08 AM
llvm/lib/Target/AArch64/AArch64RegisterInfo.td
923

Do you need a special PrintMethod here? I likely missed this be reviewing your other patch but printSVERegOp (i.e. the default action for PPRRegOp) looks like it might just work.

llvm/lib/Target/AArch64/AArch64SystemOperands.td
320

Perhaps I'm being picky but I don't see this as a predicate pattern. It's really a vector length specifier.

325

FYI: bit would work here also.

llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
5673–5680

Please agree on a consistent message style for these and the equivalent _p8to15Reg diagnostics.

llvm/lib/Target/AArch64/SVEInstrFormats.td
37

This patch alone doesn't suggest you need this complexity. Is there something coming later that requires it?

9036

Please expose opc at this level and either name the class to match the encoding group or add a comment to say what the encoding group is.

david-arm marked 2 inline comments as done.Oct 31 2022, 8:04 AM
david-arm added inline comments.
llvm/lib/Target/AArch64/AArch64RegisterInfo.td
923

Sadly it doesn't work because it prints out "p8" instead of "pn8", for example. The registers alias and so the function getRegisterName just returns "p8".

llvm/lib/Target/AArch64/AArch64SystemOperands.td
320

OK, I'm a bit short of ideas. :) How about SVEVECLENMUL?

325

I actually tried that already as I thought the same as you, but it wouldn't compile. Here is the error:

AArch64SystemOperands.td:326:7: error: In table 'SVEPREDCNTPATsList' lookup method 'lookupSVEPREDCNTPATByEncoding', key field 'Encoding' has invalid type: bit
  let Encoding = encoding;
david-arm marked 2 inline comments as done.
  • Removed all references to CntPattern and replaced them with VecLenSpecifier.
david-arm marked 4 inline comments as done.Oct 31 2022, 11:10 AM
llvm/lib/Target/AArch64/AArch64RegisterInfo.td
923

That's unfortunate. Thanks for checking. Something to follow up on later that I've just noticed is AArch64InstPrinter::printRegName emits the register name wrapped in <reg:...> markup. I'm not sure if we should be doing likewise for the pn registers.

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

How about "invalid vector length specifier, expected VLx2 or VLx4"?

7463

Is this required? It looks like tryParseSVEPattern() only did this to get access to parseExpression(), which this function doesn't need. Both getTok() and Lex() look directly accessible.

llvm/lib/Target/AArch64/MCTargetDesc/AArch64InstPrinter.cpp
1782

"vector length specifier"

llvm/lib/Target/AArch64/SVEInstrFormats.td
45

Up to you but this isn't an immediate where the bit length is relevant so you could drop the _1b.

david-arm updated this revision to Diff 472248.Nov 1 2022, 3:26 AM
david-arm marked 5 inline comments as done.
paulwalker-arm accepted this revision.Nov 1 2022, 6:14 AM
This revision is now accepted and ready to land.Nov 1 2022, 6:14 AM
This revision was landed with ongoing or failed builds.Nov 1 2022, 6:38 AM
This revision was automatically updated to reflect the committed changes.