[RISCV] Support .option rvc and norvc

[kito-cheng]

Summary

Support .option rvc and norvc to enable/disable C extension within file.

[apazos]

test/MC/RISCV/option-rvc.s

23

Kito, can you add an instruction before the first .option rvc to make sure the default behavior is unaltered:
if C ext is enabled, it will compress, but if it is disable it will not compress.

[apazos]

test/MC/RISCV/option-rvc.s

30

and add more than one instruction in an option block to show it compresses or not all of instructions in the block.

[sabuasal]

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

1000

no braces needed.

1012

no braces needed

1023

no braces needed

1035

no braces needed

[sabuasal]

lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h

21

This should be pure virtual method that you only implement in your RISCVTsargetELFstreamer and your RISCVASMstreamer. Making it virtual causes extra entries for your vtable.

Look at MCStreamer.h line 62

[kito-cheng]

Changes:

• Fix coding style issue.
• Add more instructions to testcase.
• Make emitDirectiveOptionRVC and emitDirectiveOptionNoRVC to pure virtual function.
• Return nullptr rather than RISCVTargetStreamer in createRISCVObjectTargetStreamer, because we have pure virtual function in RISCVTargetStreamer now.

[asb]

Thanks Kito. Could you please add some tests for the new warning/failure cases introduced by this patch? e.g. .option with unrecognised identifier etc.

lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp

98–99

Unnecessary comment - the function call is pretty self-descriptive.

[kito-cheng]

Changes:

• Add testcase for warning/failure cases.

[asb]

Thanks for adding the test case, this looks good to me now. Final nit: could youplease get rid of the unnecessary comment for RegisterAsmTargetStreamer (https://reviews.llvm.org/D45864#inline-403118)?

[asb]

There's a corner case we're probably going to encounter here vs gcc.

In GCC, .option rvc or .option norvc in inline asm will have the effect of enabling/disabling instruction compression from that point onwards. I assume it won't have that effect with LLVM unless you use -save-temps. It might be that we decide this use of inline asm isn't supported by LLVM and people should use appropriate function or module attributes instead. Either way, a test which demonstrates the behaviour would be worthwhile.

[kito-cheng]

Changes:

• Add 2 test cases to show .option rvc and .option norvc in inline asm will not effect the code gen.

[asb]

Thanks Kito! I'll add a comment to the test/CodeGen/RISCV files when committing just to indicate their purpose.

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

 //===-- RISCVAsmParser.cpp - Parse RISCV assembly to MCInst instructions --===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "MCTargetDesc/RISCVBaseInfo.h"
 #include "MCTargetDesc/RISCVMCExpr.h"
 #include "MCTargetDesc/RISCVMCTargetDesc.h"
+#include "MCTargetDesc/RISCVTargetStreamer.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/MCParser/MCAsmLexer.h"
 #include "llvm/MC/MCParser/MCParsedAsmOperand.h"
 #include "llvm/MC/MCParser/MCTargetAsmParser.h"

 
 namespace {
 struct RISCVOperand;
 
 class RISCVAsmParser : public MCTargetAsmParser {
   SMLoc getLoc() const { return getParser().getTok().getLoc(); }
   bool isRV64() const { return getSTI().hasFeature(RISCV::Feature64Bit); }
 
+  RISCVTargetStreamer &getTargetStreamer() {
+    MCTargetStreamer &TS = *getParser().getStreamer().getTargetStreamer();
+    return static_cast<RISCVTargetStreamer &>(TS);
+  }
+
   unsigned validateTargetOperandClass(MCParsedAsmOperand &Op,
                                       unsigned Kind) override;
 
   bool generateImmOutOfRangeError(OperandVector &Operands, uint64_t ErrorInfo,
                                   int Lower, int Upper, Twine Msg);
 
   bool MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
                                OperandVector &Operands, MCStreamer &Out,
#include "RISCVGenAsmMatcher.inc"
   OperandMatchResultTy parseImmediate(OperandVector &Operands);
   OperandMatchResultTy parseRegister(OperandVector &Operands,
                                      bool AllowParens = false);
   OperandMatchResultTy parseMemOpBaseReg(OperandVector &Operands);
   OperandMatchResultTy parseOperandWithModifier(OperandVector &Operands);
 
   bool parseOperand(OperandVector &Operands);
 
+  bool parseDirectiveOption();
+
+  void setFeatureBits(uint64_t Feature, StringRef FeatureString) {
+    if (!(getSTI().getFeatureBits()[Feature])) {
+      MCSubtargetInfo &STI = copySTI();
+      setAvailableFeatures(
+          ComputeAvailableFeatures(STI.ToggleFeature(FeatureString)));
+    }
+  }
+
+  void clearFeatureBits(uint64_t Feature, StringRef FeatureString) {
+    if (getSTI().getFeatureBits()[Feature]) {
+      MCSubtargetInfo &STI = copySTI();
+      setAvailableFeatures(
+          ComputeAvailableFeatures(STI.ToggleFeature(FeatureString)));
+    }
+  }
 public:
   enum RISCVMatchResultTy {
     Match_Dummy = FIRST_TARGET_MATCH_RESULT_TY,
 #define GET_OPERAND_DIAGNOSTIC_TYPES
 #include "RISCVGenAsmMatcher.inc"
 #undef GET_OPERAND_DIAGNOSTIC_TYPES
   };
 
bool RISCVAsmParser::classifySymbolRef(const MCExpr *Expr,
   Addend = AddendExpr->getValue();
   if (BE->getOpcode() == MCBinaryExpr::Sub)
     Addend = -Addend;
 
   // It's some symbol reference + a constant addend
   return Kind != RISCVMCExpr::VK_RISCV_Invalid;
 }
 
-bool RISCVAsmParser::ParseDirective(AsmToken DirectiveID) { return true; }
+bool RISCVAsmParser::ParseDirective(AsmToken DirectiveID) {
+  // This returns false if this function recognizes the directive
+  // regardless of whether it is successfully handles or reports an
+  // error. Otherwise it returns true to give the generic parser a
+  // chance at recognizing it.
+  StringRef IDVal = DirectiveID.getString();
+
+  if (IDVal == ".option")
+    return parseDirectiveOption();
+
+  return true;
+}
+
+bool RISCVAsmParser::parseDirectiveOption() {
+  MCAsmParser &Parser = getParser();
+  // Get the option token.
+  AsmToken Tok = Parser.getTok();
+  // At the moment only identifiers are supported.
+  if (Tok.isNot(AsmToken::Identifier))
+    return Error(Parser.getTok().getLoc(),
+                 "unexpected token, expected identifier");
+
+  StringRef Option = Tok.getIdentifier();
+
+  if (Option == "rvc") {
+    getTargetStreamer().emitDirectiveOptionRVC();
+
+    Parser.Lex();
+    if (Parser.getTok().isNot(AsmToken::EndOfStatement))
+      return Error(Parser.getTok().getLoc(),
+                   "unexpected token, expected end of statement");
+
+    setFeatureBits(RISCV::FeatureStdExtC, "c");
+    return false;
+  }
+
+  if (Option == "norvc") {
+    getTargetStreamer().emitDirectiveOptionNoRVC();
+
+    Parser.Lex();
+    if (Parser.getTok().isNot(AsmToken::EndOfStatement))
+      return Error(Parser.getTok().getLoc(),
+                   "unexpected token, expected end of statement");
+
+    clearFeatureBits(RISCV::FeatureStdExtC, "c");
+    return false;
+  }
+
+  // Unknown option.
+  Warning(Parser.getTok().getLoc(),
+          "unknown option, expected 'rvc' or 'norvc'");
+  Parser.eatToEndOfStatement();
+  return false;
+}
 
 extern "C" void LLVMInitializeRISCVAsmParser() {
   RegisterMCAsmParser<RISCVAsmParser> X(getTheRISCV32Target());
   RegisterMCAsmParser<RISCVAsmParser> Y(getTheRISCV64Target());
 }

lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h

 #include "llvm/MC/MCELFStreamer.h"
 
 namespace llvm {
 
 class RISCVTargetELFStreamer : public RISCVTargetStreamer {
 public:
   MCELFStreamer &getStreamer();
   RISCVTargetELFStreamer(MCStreamer &S, const MCSubtargetInfo &STI);
+
+  virtual void emitDirectiveOptionRVC();
+  virtual void emitDirectiveOptionNoRVC();
 };
 }
 #endif

lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp

if (Features[RISCV::FeatureStdExtC])
     EFlags |= ELF::EF_RISCV_RVC;
 
   MCA.setELFHeaderEFlags(EFlags);
 }
 
 MCELFStreamer &RISCVTargetELFStreamer::getStreamer() {
   return static_cast<MCELFStreamer &>(Streamer);
 }
+
+void RISCVTargetELFStreamer::emitDirectiveOptionRVC() {}
+void RISCVTargetELFStreamer::emitDirectiveOptionNoRVC() {}

lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp

static MCInstPrinter *createRISCVMCInstPrinter(const Triple &T,
   return new RISCVInstPrinter(MAI, MII, MRI);
 }
 
 static MCTargetStreamer *
 createRISCVObjectTargetStreamer(MCStreamer &S, const MCSubtargetInfo &STI) {
   const Triple &TT = STI.getTargetTriple();
   if (TT.isOSBinFormatELF())
     return new RISCVTargetELFStreamer(S, STI);
-  return new RISCVTargetStreamer(S);
+  return nullptr;
+}
+
+static MCTargetStreamer *createRISCVAsmTargetStreamer(MCStreamer &S,
+                                                      formatted_raw_ostream &OS,
+                                                      MCInstPrinter *InstPrint,
+                                                      bool isVerboseAsm) {
+  return new RISCVTargetAsmStreamer(S, OS);
 }
 
 extern "C" void LLVMInitializeRISCVTargetMC() {
   for (Target *T : {&getTheRISCV32Target(), &getTheRISCV64Target()}) {
     TargetRegistry::RegisterMCAsmInfo(*T, createRISCVMCAsmInfo);
     TargetRegistry::RegisterMCInstrInfo(*T, createRISCVMCInstrInfo);
     TargetRegistry::RegisterMCRegInfo(*T, createRISCVMCRegisterInfo);
     TargetRegistry::RegisterMCAsmBackend(*T, createRISCVAsmBackend);
     TargetRegistry::RegisterMCCodeEmitter(*T, createRISCVMCCodeEmitter);
     TargetRegistry::RegisterMCInstPrinter(*T, createRISCVMCInstPrinter);
     TargetRegistry::RegisterMCSubtargetInfo(*T, createRISCVMCSubtargetInfo);
     TargetRegistry::RegisterObjectTargetStreamer(
         *T, createRISCVObjectTargetStreamer);
+
+    // Register the asm target streamer.
+    TargetRegistry::RegisterAsmTargetStreamer(*T, createRISCVAsmTargetStreamer);
   }
 }

lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h

 
 #include "llvm/MC/MCStreamer.h"
 
 namespace llvm {
 
 class RISCVTargetStreamer : public MCTargetStreamer {
 public:
   RISCVTargetStreamer(MCStreamer &S);
+
+  virtual void emitDirectiveOptionRVC() = 0;
+  virtual void emitDirectiveOptionNoRVC() = 0;
 };
+
+// This part is for ascii assembly output
+class RISCVTargetAsmStreamer : public RISCVTargetStreamer {
+  formatted_raw_ostream &OS;
+
+public:
+  RISCVTargetAsmStreamer(MCStreamer &S, formatted_raw_ostream &OS);
+
+  void emitDirectiveOptionRVC() override;
+  void emitDirectiveOptionNoRVC() override;
+};
+
 }
 #endif

lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp

 //===-- RISCVTargetStreamer.cpp - RISCV Target Streamer Methods -----------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file provides RISCV specific target streamer methods.
 //
 //===----------------------------------------------------------------------===//
 
 #include "RISCVTargetStreamer.h"
+#include "llvm/Support/FormattedStream.h"
 
 using namespace llvm;
 
 RISCVTargetStreamer::RISCVTargetStreamer(MCStreamer &S) : MCTargetStreamer(S) {}
+
+// This part is for ascii assembly output
+RISCVTargetAsmStreamer::RISCVTargetAsmStreamer(MCStreamer &S,
+                                               formatted_raw_ostream &OS)
+    : RISCVTargetStreamer(S), OS(OS) {}
+
+void RISCVTargetAsmStreamer::emitDirectiveOptionRVC() {
+  OS << "\t.option\trvc\n";
+}
+
+void RISCVTargetAsmStreamer::emitDirectiveOptionNoRVC() {
+  OS << "\t.option\tnorvc\n";
+}

test/MC/RISCV/option-rvc.s

+# RUN: llvm-mc -triple riscv32 -show-encoding < %s \
+# RUN: | FileCheck -check-prefixes=CHECK,CHECK-ALIAS %s
+# RUN: llvm-mc -triple riscv32 -show-encoding \
+# RUN: -riscv-no-aliases <%s | FileCheck -check-prefixes=CHECK,CHECK-INST %s
+# RUN: llvm-mc -triple riscv32 -filetype=obj < %s \
+# RUN: | llvm-objdump  -triple riscv32 -mattr=+c -d - \
+# RUN: | FileCheck -check-prefixes=CHECK-BYTES,CHECK-ALIAS %s
+# RUN: llvm-mc -triple riscv32 -filetype=obj < %s \
+# RUN: | llvm-objdump  -triple riscv32 -mattr=+c -d -riscv-no-aliases - \
+# RUN: | FileCheck -check-prefixes=CHECK-BYTES,CHECK-INST %s
+
+# RUN: llvm-mc -triple riscv64 -show-encoding < %s \
+# RUN: | FileCheck -check-prefixes=CHECK-ALIAS %s
+# RUN: llvm-mc -triple riscv64 -show-encoding \
+# RUN: -riscv-no-aliases <%s | FileCheck -check-prefixes=CHECK-INST %s
+# RUN: llvm-mc -triple riscv64 -filetype=obj < %s \
+# RUN: | llvm-objdump  -triple riscv64 -mattr=+c -d - \
+# RUN: | FileCheck -check-prefixes=CHECK-BYTES,CHECK-ALIAS %s
+# RUN: llvm-mc -triple riscv64 -filetype=obj < %s \
+# RUN: | llvm-objdump  -triple riscv64 -mattr=+c -d -riscv-no-aliases - \
+# RUN: | FileCheck -check-prefixes=CHECK-BYTES,CHECK-INST %s
+
+# CHECK-BYTES: 13 85 05 00
+# CHECK-ALIAS: mv a0, a1
+# CHECK-INST: addi a0, a1, 0
+# CHECK: # encoding:  [0x13,0x85,0x05,0x00]
+addi a0, a1, 0
+
+# CHECK-BYTES: 13 04 c1 3f
+# CHECK-ALIAS: addi s0, sp, 1020
+# CHECK-INST: addi s0, sp, 1020
+# CHECK: # encoding:  [0x13,0x04,0xc1,0x3f]
+addi s0, sp, 1020
+
+
+# CHECK: .option rvc
+.option rvc
+# CHECK-BYTES: 2e 85
+# CHECK-ALIAS: add a0, zero, a1
+# CHECK-INST: c.mv a0, a1
+# CHECK: # encoding:  [0x2e,0x85]
+addi a0, a1, 0
+
+# CHECK-BYTES: e0 1f
+# CHECK-ALIAS: addi s0, sp, 1020
+# CHECK-INST: c.addi4spn s0, sp, 1020
+# CHECK: # encoding:  [0xe0,0x1f]
+addi s0, sp, 1020
+
+# CHECK: .option norvc
+.option norvc
+# CHECK-BYTES: 13 85 05 00
+# CHECK-ALIAS: mv a0, a1
+# CHECK-INST: addi a0, a1, 0
+# CHECK: # encoding:  [0x13,0x85,0x05,0x00]
+addi a0, a1, 0
+
+# CHECK-BYTES: 13 04 c1 3f
+# CHECK-ALIAS: addi s0, sp, 1020
+# CHECK-INST: addi s0, sp, 1020
+# CHECK: # encoding:  [0x13,0x04,0xc1,0x3f]
+addi s0, sp, 1020
+
+# CHECK: .option rvc
+.option rvc
+# CHECK-BYTES: 2e 85
+# CHECK-ALIAS: add a0, zero, a1
+# CHECK-INST: c.mv a0, a1
+# CHECK: # encoding:  [0x2e,0x85]
+addi a0, a1, 0
+
+# CHECK-BYTES: e0 1f
+# CHECK-ALIAS: addi s0, sp, 1020
+# CHECK-INST: c.addi4spn s0, sp, 1020
+# CHECK: # encoding:  [0xe0,0x1f]
+addi s0, sp, 1020
+
+# CHECK: .option norvc
+.option norvc
+# CHECK-BYTES: 13 85 05 00
+# CHECK-ALIAS: mv a0, a1
+# CHECK-INST: addi a0, a1, 0
+# CHECK: # encoding:  [0x13,0x85,0x05,0x00]
+addi a0, a1, 0
+
+# CHECK-BYTES: 13 04 c1 3f
+# CHECK-ALIAS: addi s0, sp, 1020
+# CHECK-INST: addi s0, sp, 1020
+# CHECK: # encoding:  [0x13,0x04,0xc1,0x3f]
+addi s0, sp, 1020