[SPARC][IAS] Recognize more SPARCv9 instructions/pseudoinstructions
Needs ReviewPublic
Actions

Authored by koakuma on Feb 27 2023, 5:58 PM.

Details

Reviewers

Summary

Add support for more SPARCv9 instructions/pseudoinstructions (mainly memory access and privileged ones) to the integrated assembler.

This should make it possible for low-level code, such as OS kernels, to be built with it.

koakuma requested review of this revision.Feb 27 2023, 5:58 PM

llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp
669	Shouldn't this be impossible due to the predicate, and thus be an assert?
676	`isUInt<32>(ImmValue)`?
682	Use MCInstBuilder throughout? Avoids all the `{ ... }` below, too...
1116	Use a TableGen GenericTable for this to avoid having to define the forward mapping here and the backward mapping later? (See RISCVSystemOperands.td for an example).

llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp
669	Probably just me misunderstanding it but during testing I found out that even with 32-bit `-mtriple=sparc` it still recognizes SETX anyway, that's why I'm putting the condition as a normal error instead of an assert.

Implement review suggestions:

Ping?

llvm/lib/Target/Sparc/SparcInstrInfo.td
1834–1838	I'm confused by what this comment means, as you're defining a new instruction. Is the intent to define an assembler alias only?

llvm/lib/Target/Sparc/SparcInstrInfo.td
1834–1838	Yeah, my intent is to define a `rdpr %fq, $rd` alias, but the encoding of `%fq` is different between v8 and v9 versions of the ISA, so I had to do that to convince the assembler to encode it properly when emitting code for v9. Should I do it in a different way?

Ping?

It looks like there are several independent changes. A few smaller diffs would be much easier to review.

llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp
444	It should have its own Kind and union member. Not sure if tablegen provides you with an enum, but if it does, the enum could be used. This is "safer" and allows you to avoid converting to MCExpr and back.
1105	Why not just `getTok().is(AsmToken::Integer)` and `getTok().getIntVal()` on success?
1113	Is space after '#' allowed? If not, `getLexer().peekToken(false)` should be used.
1132	The location is not precise. It points before the last whitespace (there may be several). Ideally, you would `getTok().getEndLoc()` before `Lex()`ing the indentifier or the number.
1218	This should return MatchOperand_ParseFail. NoMatch implies no tokens were consumed.
1233	Missing return?
1237	Why is LParen here?
1603	Do you know why Sparc does not use autogenerated MatchRegisterName? (ShouldEmitMatchRegisterName = 0 in Sparc.td.)
llvm/lib/Target/Sparc/SparcASITags.td
16–21	It looks like all tags have an alternative name, just pass it as template parameter.
llvm/lib/Target/Sparc/SparcInstrAliases.td
513	It should be `simm13Op`, though it does not currently matter that much as it is missing AsmOperandClass, so no overflow checking is done.