[RISCV] Support .variant_cc directive for the assembler.

[fakepaper56]

Summary

The patch is split from D103435. The patch supported a new directive .variant_cc
that annotates function with STO_RISCV_VARIANT_CC. Symbols marked with
STO_RISCV_VARIANT_CC do not use standard calling conversion or use parameter not
passed in GPR/FPR.

Related: https://github.com/riscv/riscv-elf-psabi-doc/pull/190

Initial authored by: HsiangKai

[MaskRay]

I think @HsiangKai is still active, so he may want to carry on the work? I agree that splitting the asm part from the feature is nice. I left some comments on D103435 about the MC style.

[kito-cheng]

@MaskRay he (@HsiangKai) has hand-over all RISC-V related patch to other SiFive folks when he join Google :) Or maybe we should commandeer that revision and keep using that?

[fakepaper56]

Address MaskRay's comment in D103435.

[MaskRay]

In D138352#3940093, @kito-cheng wrote:

@MaskRay he (@HsiangKai) has hand-over all RISC-V related patch to other SiFive folks when he join Google :) Or maybe we should commandeer that revision and keep using that?

Thanks for the context! Using this patch is fine.

[MaskRay]

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

2314

in '.variant_cc' directive is not really useful as the diagnostic repeats the source line. ditto below

2318

This code self explains so the comment seems redundant. You may delete the blank line after Parser.Lex() as well.

llvm/test/MC/RISCV/directive-variant_cc-err.s

4 ↗

(On Diff #476790)

Add line/column information like :[[#@LINE+1]]:25:. See some newer tests in llvm/test/MC/

Grep defsym=ERR=1. It's probably better to combine positive and negative tests in one file.

[fakepaper56]

Refine error message and merge directive-variant_cc-err.s and directive-variant_cc.s

[fakepaper56]

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

2318

I use default expected newline for the token missing.

[fakepaper56]

Rebase and ping.

[kito-cheng]

llvm/test/MC/RISCV/directive-variant_cc.s

2

Could you add objdump check like AArch64's test: https://github.com/llvm/llvm-project/blob/main/llvm/test/MC/AArch64/directive-variant_pcs.s#L33

[fakepaper56]

Add "readelf -s" test to address Kito's comment.

[kito-cheng]

LGTM, @MaskRay do you mind have one more look for this? :)

[MaskRay]

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

2321

unneeded blank line

llvm/test/MC/RISCV/directive-variant_cc.s

2

We need local/global definitions as the aarch64 test does.

4

-filetype asm can be omitted.

[fakepaper56]

Remove unneeded blank line and add local/global definitions into test.
I don't add filetype asm test since the patch does not include AsmPrinter
support. I will complete this part in the next patch.

[MaskRay]

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

2312

MCSymbol *Sym = getContext().lookupSymbol(SymbolName); is not the best. See D122507

[MaskRay]

llvm/test/MC/RISCV/directive-variant_cc.s

2

See directive-variant_pcs.s tests. I think local, def1, def2, alias_def1, undef are all useful.

We can do somewhat better by leveraging --defsym=ERR=1 (as you already did).

[fakepaper56]

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

2312

Good point. Thank your reminder.

[fakepaper56]

Allow unregistered symbol and add more tests.

[MaskRay]

llvm/test/MC/RISCV/directive-variant_cc.s

40

Delete // ERR-NEXT: .variant_cc and this line. The error mark is tested by other tests and this without --strict-whitespace --match-full-lines isn't useful.

46

Delete this line. The error mark is tested by other tests and this without --strict-whitespace --match-full-lines isn't useful.

[fakepaper56]

Remove error mark tests.

[smd]

Hi @fakepaper56

Do you think it's possible to support printing .variant_cc label to asm file if the proper flag for symbol is set? I believe this happens(didn't verify though) for .variant_pcs in AArch64ELFStreamer.cpp
This would help me with D132994

Thanks!

[fakepaper56]

This comment was removed by fakepaper56.

[fakepaper56]

I will upstream a patch for printing .variant_cc in asm soon. I think your requirement could be based on that.

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

#include "RISCVGenAsmMatcher.inc"
   OperandMatchResultTy parseInsnDirectiveOpcode(OperandVector &Operands);
   OperandMatchResultTy parseGPRAsFPR(OperandVector &Operands);
 
   bool parseOperand(OperandVector &Operands, StringRef Mnemonic);
 
   bool parseDirectiveOption();
   bool parseDirectiveAttribute();
   bool parseDirectiveInsn(SMLoc L);
+  bool parseDirectiveVariantCC();
 
   void setFeatureBits(uint64_t Feature, StringRef FeatureString) {
     if (!(getSTI().getFeatureBits()[Feature])) {
       MCSubtargetInfo &STI = copySTI();
       setAvailableFeatures(
           ComputeAvailableFeatures(STI.ToggleFeature(FeatureString)));
     }
   }
bool RISCVAsmParser::ParseDirective(AsmToken DirectiveID) {
   StringRef IDVal = DirectiveID.getString();
 
   if (IDVal == ".option")
     return parseDirectiveOption();
   if (IDVal == ".attribute")
     return parseDirectiveAttribute();
   if (IDVal == ".insn")
     return parseDirectiveInsn(DirectiveID.getLoc());
+  if (IDVal == ".variant_cc")
+    return parseDirectiveVariantCC();
 
   return true;
 }
 
 bool RISCVAsmParser::parseDirectiveOption() {
   MCAsmParser &Parser = getParser();
   // Get the option token.
   AsmToken Tok = Parser.getTok();
bool RISCVAsmParser::parseDirectiveInsn(SMLoc L) {
 
   unsigned Opcode;
   uint64_t ErrorInfo;
   return MatchAndEmitInstruction(L, Opcode, Operands, Parser.getStreamer(),
                                  ErrorInfo,
                                  /*MatchingInlineAsm=*/false);
 }
 
+/// parseDirectiveVariantCC
+///  ::= .variant_cc symbol
+bool RISCVAsmParser::parseDirectiveVariantCC() {
+  MCAsmParser &Parser = getParser();
+  const AsmToken &Tok = Parser.getTok();
+  if (Tok.isNot(AsmToken::Identifier))
+    return TokError("expected symbol name");
+
+  StringRef SymbolName = Tok.getIdentifier();
+  MCSymbol *Sym = getContext().lookupSymbol(SymbolName);
+  if (!Sym)
+    return TokError("unknown symbol");
+
+  Parser.Lex();
+  if (Parser.parseEOL())
+    return false;
+
+  getTargetStreamer().emitDirectiveVariantCC(*Sym);
+
+  return false;
+}
+
 void RISCVAsmParser::emitToStreamer(MCStreamer &S, const MCInst &Inst) {
   MCInst CInst;
   bool Res = compressInst(CInst, Inst, getSTI(), S.getContext());
   if (Res)
     ++RISCVNumInstrsCompressed;
   S.emitInstruction((Res ? CInst : Inst), getSTI());
 }
 

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h

public:
   void emitDirectiveOptionPush() override;
   void emitDirectiveOptionPop() override;
   void emitDirectiveOptionPIC() override;
   void emitDirectiveOptionNoPIC() override;
   void emitDirectiveOptionRVC() override;
   void emitDirectiveOptionNoRVC() override;
   void emitDirectiveOptionRelax() override;
   void emitDirectiveOptionNoRelax() override;
+  void emitDirectiveVariantCC(MCSymbol &Symbol) override;
 
   void finish() override;
 };
 
 MCELFStreamer *createRISCVELFStreamer(MCContext &C,
                                       std::unique_ptr<MCAsmBackend> MAB,
                                       std::unique_ptr<MCObjectWriter> MOW,
                                       std::unique_ptr<MCCodeEmitter> MCE,
                                       bool RelaxAll);
 }
 #endif

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp

void RISCVTargetELFStreamer::finish() {
   MCA.setELFHeaderEFlags(EFlags);
 }
 
 void RISCVTargetELFStreamer::reset() {
   AttributeSection = nullptr;
   Contents.clear();
 }
 
+void RISCVTargetELFStreamer::emitDirectiveVariantCC(MCSymbol &Symbol) {
+  cast<MCSymbolELF>(Symbol).setOther(ELF::STO_RISCV_VARIANT_CC);
+}
+
 namespace {
 class RISCVELFStreamer : public MCELFStreamer {
   static std::pair<unsigned, unsigned> getRelocPairForSize(unsigned Size) {
     switch (Size) {
     default:
       llvm_unreachable("unsupported fixup size");
     case 1:
       return std::make_pair(RISCV::fixup_riscv_add_8, RISCV::fixup_riscv_sub_8);

llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h

public:
   virtual void emitDirectiveOptionPush();
   virtual void emitDirectiveOptionPop();
   virtual void emitDirectiveOptionPIC();
   virtual void emitDirectiveOptionNoPIC();
   virtual void emitDirectiveOptionRVC();
   virtual void emitDirectiveOptionNoRVC();
   virtual void emitDirectiveOptionRelax();
   virtual void emitDirectiveOptionNoRelax();
+  virtual void emitDirectiveVariantCC(MCSymbol &Symbol);
   virtual void emitAttribute(unsigned Attribute, unsigned Value);
   virtual void finishAttributeSection();
   virtual void emitTextAttribute(unsigned Attribute, StringRef String);
   virtual void emitIntTextAttribute(unsigned Attribute, unsigned IntValue,
                                     StringRef StringValue);
 
   void emitTargetAttributes(const MCSubtargetInfo &STI);
   void setTargetABI(RISCVABI::ABI ABI);

llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp

 void RISCVTargetStreamer::emitDirectiveOptionPush() {}
 void RISCVTargetStreamer::emitDirectiveOptionPop() {}
 void RISCVTargetStreamer::emitDirectiveOptionPIC() {}
 void RISCVTargetStreamer::emitDirectiveOptionNoPIC() {}
 void RISCVTargetStreamer::emitDirectiveOptionRVC() {}
 void RISCVTargetStreamer::emitDirectiveOptionNoRVC() {}
 void RISCVTargetStreamer::emitDirectiveOptionRelax() {}
 void RISCVTargetStreamer::emitDirectiveOptionNoRelax() {}
+void RISCVTargetStreamer::emitDirectiveVariantCC(MCSymbol &Symbol) {}
 void RISCVTargetStreamer::emitAttribute(unsigned Attribute, unsigned Value) {}
 void RISCVTargetStreamer::finishAttributeSection() {}
 void RISCVTargetStreamer::emitTextAttribute(unsigned Attribute,
                                             StringRef String) {}
 void RISCVTargetStreamer::emitIntTextAttribute(unsigned Attribute,
                                                unsigned IntValue,
                                                StringRef StringValue) {}
 void RISCVTargetStreamer::setTargetABI(RISCVABI::ABI ABI) {

llvm/test/MC/RISCV/directive-variant_cc.s

+// RUN: llvm-mc -triple riscv64 -filetype obj -o - %s | llvm-readobj --symbols - | FileCheck %s
+// RUN: not llvm-mc -triple riscv64 -filetype asm -defsym=ERR=1 -o - %s 2>&1 | FileCheck %s --check-prefix=ERR
+
+.text
+.global foo
+.variant_cc foo
+
+// CHECK: Name: foo
+// CHECK: Other [ (0x80)
+
+.ifdef ERR
+.variant_cc
+// ERR:[[#@LINE-1]]:12: error: expected symbol name
+// ERR-NEXT:   .variant_cc
+// ERR-NEXT:              ^
+
+.variant_cc bar
+// ERR: [[#@LINE-1]]:13: error: unknown symbol
+// ERR-NEXT:   .variant_cc bar
+// ERR-NEXT:               ^
+
+.global fox
+.variant_cc fox bar
+// ERR: [[#@LINE-1]]:17: error: expected newline
+// ERR-NEXT:   .variant_cc fox bar
+// ERR-NEXT:                   ^
+.endif