[RISCV] MC layer support for the load/store instructions of standard compress instruction set

[shiva0217]

Summary

1. Define SubtargetFeature and Predicate for C extension

2. Add IsRV64 Predicate for RV64 only C extension instructions

3. Support decode 16-bit instructions

4. Support encode 16-bit instructions

5. Add GPRC, SP,GPRNonX0 register class for C extension load/store instructions

6. Add compress InstFormat enum format

7. TSFlags in RISCVInstrFormats.td/RISCVInstrFormatsC.td have increase 1 bit

8. Add CI CSS CL CS instruction format for C extension load/store instructions

9. Add C extension load/store instructions

10. Support decode GPRC,GPRNonX0 register class

11. Identify/check UImm8Lsb00/UImm7Lsb00/UImm8Lsb000/UImm9Lsb000 operands by RISCVAsmParser

12. Add imply SP operand while decoding CLWSP/CLWLP/CLDSP/CSDSP

13. Add rv32c-valid.s, rv32c-invalid.s, rv64c-valid.s, rv64c-invalid.s MC testcases

[simoncook]

lib/Target/RISCV/RISCV.td

29 ↗

(On Diff #122779)

Should this be "'C' (Compressed Instructions)", and presumably aligned similarly with FeatureStdExtA?

test/MC/RISCV/compressed-invalid.s

1 ↗

(On Diff #122779)

This test is missing the triple to run the assembler for. I would expect a "-triple=riscv32" or "-triple=riscv64" here.

test/MC/RISCV/compressed-valid.s

1 ↗

(On Diff #122779)

This test is missing the triple to run the assembler for. I would expect a "-triple=riscv32" or "-triple=riscv64" here.

[asb]

Thanks Shiva. A whole bunch of inline comments are below. Thanks to Simon for spotting the missing -triple in the tests, I was struggling to see why I couldn't get the tests to pass.

compressed-valid.s and compressed-invalid.s should be named rv32c-valid.s and rv32c-invalid.s to match the naming conventions of the existing tests. You can also add check lines for a -riscv64 triple. RV64C specific instructions can go in rv64c-valid.s

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

212 ↗

(On Diff #122779)

These methods are sorted by bit-width of the operand, so should be in the order isUImm5, isUImm6Lsb00, isUImm8Lsb00.

424 ↗

(On Diff #122779)

Should ideally be ordered like the methods.

[InvalidFenceArg doesn't match the method order here, I should fix that next time that piece of code gets touched].

lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp

87 ↗

(On Diff #122779)

Shouldn't you be checking RegNo > 8?

172 ↗

(On Diff #122779)

The previous code was using support::endian::read32le(Bytes.data()). Can't you use support::endian::read16le(Bytes.data())) for C instructions and avoid adding the extra helper functions?

lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h

32 ↗

(On Diff #122779)

I've tried to keep the instruction formats matching those in the ISA manual. I'd be inclined to use more bits of TSFlags and have the instruction format names match those in Table 12.1 in the ISA manual.

lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp

86 ↗

(On Diff #122779)

TmpInst seems unnecessary.

94 ↗

(On Diff #122779)

Replace the assert a couple of lines above with an llvm_unreachable here?

lib/Target/RISCV/RISCVInstrFormats.td

41 ↗

(On Diff #122779)

Same comment as before regarding InstFormats

lib/Target/RISCV/RISCVInstrFormatsC.td

15 ↗

(On Diff #122779)

Indentation

31 ↗

(On Diff #122779)

I'd prefer using opcodestr+argstr like the formats in RISCVInstrFormats.td. I found this reduced the amount of string manipulation needed overall.

lib/Target/RISCV/RISCVInstrInfo.cpp

55 ↗

(On Diff #122779)

These RISCVInstrInfo.cpp changes should go in a later codegen patch, as they're not necessary for MC layer support

lib/Target/RISCV/RISCVInstrInfoC.td

60 ↗

(On Diff #122779)

I'd prefer to use headers similarly to RISCVInstrInfo.td. i.e. for "Instruction class templates" and "Instructions" - we can always change later if we want, keeping things consistent across all the .td files.

67 ↗

(On Diff #122779)

Maybe I'm missing something, but can't the CL class take care of placing these immediate bits where appropriate (same for the instructions below)

[shiva0217]

Thanks, Simon, I adding the missing -triple argument in testcases and fix the typo.
Thanks, Alex, I rename the tesecase to rv32/64c-valid/invalid.s and reorganize the code as you suggest.

lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

212 ↗

(On Diff #122779)

Sorting the method as @asb suggest.

424 ↗

(On Diff #122779)

Sorting the method as @asb suggest.
If the order still not as you expect, please feel free to reorder the method in next patch.

lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp

87 ↗

(On Diff #122779)

Yes, should check >8.
Thanks.

172 ↗

(On Diff #122779)

Using support::endian::read32le(Bytes.data()) and support::endian::read16le(Bytes.data())) to avoid adding extra helper functions.

lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h

32 ↗

(On Diff #122779)

Adding all compress instruction InstFormat and increase TSFlags 1 bit.

lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp

86 ↗

(On Diff #122779)

remove redundant TmpInst.

94 ↗

(On Diff #122779)

Adding llvm_unreachable in default which is the case that instruction length, not 2 or 4.

lib/Target/RISCV/RISCV.td

29 ↗

(On Diff #122779)

Yes, Thanks, fix the typo here.

lib/Target/RISCV/RISCVInstrFormats.td

41 ↗

(On Diff #122779)

Add all compress instruction InstFormats

lib/Target/RISCV/RISCVInstrFormatsC.td

15 ↗

(On Diff #122779)

Fix the Indentation

31 ↗

(On Diff #122779)

Using opcodestr+argstr like the formats in RISCVInstrFormats.td.

lib/Target/RISCV/RISCVInstrInfo.cpp

55 ↗

(On Diff #122779)

Remove the code which will add later in C codegen support patch.

lib/Target/RISCV/RISCVInstrInfoC.td

60 ↗

(On Diff #122779)

Remove other header names.
Using the same naming "Instructions" as in RISCVInstrInfo.td

67 ↗

(On Diff #122779)

The reason we put immediate encode in CLW is that CLD use the same encoding format(CL) except the immediate encoding is different.
To reuse the format class, we extract the immediate encoding in each instruction.

test/MC/RISCV/compressed-invalid.s

1 ↗

(On Diff #122779)

Adding missing -triple in testcases.

test/MC/RISCV/compressed-valid.s

1 ↗

(On Diff #122779)

Adding missing -triple in testcases.

[simoncook]

Thanks, I found one nitpick with a couple of the tests, but the rest looks good to me.

test/MC/RISCV/rv32c-valid.s

1 ↗

(On Diff #122980)

Should this be < %s instead to be consistent with the other calls to llvm-mc below?

test/MC/RISCV/rv64c-valid.s

1 ↗

(On Diff #122980)

Likewise with rv32c-valid.s

[shiva0217]

test/MC/RISCV/rv32c-valid.s

1 ↗

(On Diff #122980)

Make the call llvm-mc command in test case consistent as Simon suggest.

test/MC/RISCV/rv64c-valid.s

1 ↗

(On Diff #122980)

Make the call llvm-mc command in test case consistent as Simon suggest.

[mgrang]

lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp

96 ↗

(On Diff #123112)

Period after comment.

lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp

87 ↗

(On Diff #123112)

Period after comment.

98 ↗

(On Diff #123112)

Can you order the cases in the ascending order of Size?

[apazos]

Shiva, next time you update this patch (and the other patches), can you please generate the patch with more context?
git diff HEAD~1 HEAD -U9999999999
See https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp

87 ↗

(On Diff #123112)

no need to add { }

149 ↗

(On Diff #123112)

if bit 0 and 1 are 1.

[shiva0217]

Hi @mgrang, @apazos

Thanks for the comments.
I update the patch using "-U 9999999999" and also using clang-format to check indentation.

lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp

87 ↗

(On Diff #123112)

remove unnecessary {}.

96 ↗

(On Diff #123112)

Add period after comment.

149 ↗

(On Diff #123112)

Fixup comments.

lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp

87 ↗

(On Diff #123112)

Add period after comment.

98 ↗

(On Diff #123112)

Order the case by ascending order as @mgrang suggest.

[asb]

Hi Shiva, now I've got this building on top of head I see I was incorrect to suggest removing the RISCVInstrInfo.cpp changes, as you get assertions in some of the tests if you don't. I haven't done much work with cases like this where you have multiple register classes with overlapping contents but I _think_ the most correct fix to storeRegToStackSlot and loadRegtoStackSlot is to use if (RISCV::GPRRegClass.hasSubClassEq(RC)) rather than if (RC == &RISCV::GPRRegClasss). The original code probably should have been written using this in the first place.

[asb]

Hi Shiva, I think we're almost there. Primarily naming and style nits below. The most important issue to resolve is the implied SP operand I think.

lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp

97 ↗

(On Diff #123533)

Is there a reason we can't use let Uses = [X2] and/or let Defs = [X2] for instructions that implicitly read/write the stack pointer register?

lib/Target/RISCV/RISCVInstrFormatsC.td

37 ↗

(On Diff #123533)

I started off by using very short class names in RISCVInforFormats.td, but found it's ultimately easy to confuse a class name with an instruction. RVInstCI or RVInst16CI and so on would probably be preferable.

lib/Target/RISCV/RISCVInstrInfoC.td

67 ↗

(On Diff #122779)

I see. In that case, it would make sense to add a comment to the CL and CS classes that explains subclasses must set the immediate bits. An alternative could be to define different classes for these variants as is done with RVInstIShift and RVInstIShiftW.

16 ↗

(On Diff #123533)

"where the least significant two bits are zero." (same fix for other operand types defined below).

53 ↗

(On Diff #123533)

StackLoad_ri or `CStackLoad would match the naming convention used in RISCVInstrInfo.td and others. Same for the other classes. See also my query regarding the sp operand and whether we can use Defs and Uses instead.

80 ↗

(On Diff #123533)

It would match the other RISCVInstrInfo*.td files use let Predicates = [HasStdExtC] in a way similar to a C++ namespace.

80 ↗

(On Diff #123533)

Just one space is needed after the instruction name and before :

86 ↗

(On Diff #123533)

I try to keep the MC layer instruction definitions as close to the ISA manual as possible, ordering based on the the quickref table on page 104 of the spec. It might be nice to order based on the tables in section 12.7 "RVC Instruction Set Listings" (page 82 and 83). There's probably less benefit in doing so than for the RV32IMAFD extensions though, so it's up to you. Generally, it's nice to be able to see at a glance that every instruction in that listing is represented.

test/MC/RISCV/rv32c-invalid.s

10 ↗

(On Diff #123533)

It would be good to check values that are just outside the valid range.

test/MC/RISCV/rv32c-valid.s

3 ↗

(On Diff #123533)

There's no need to explicitly state +64bit in this or the other tests.

13 ↗

(On Diff #123533)

It would be good to check the maximum and minimum immediate values (same with other *-valid.s tests).

[shiva0217]

Hi Alex, if (RISCV::GPRRegClass.hasSubClassEq(RC)) in storeRegToStackSlot and loadRegtoStackSlot looks like a good a solution to me. About imply SP I have explained in the inline comments, let me know if you have other thought about it. Thanks for your comments, I really appreciate that.

lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp

97 ↗

(On Diff #123533)

The reason is that InstPrinter has to print SP operand "c.lwsp ra, 12(sp)".
We could print sp as string and remove it in input operand list rather than add imply SP,
but the parser will parse sp as input operand and we'll need to add other logic to deal with it.
If we add imply SP operand while disassembling, then printer and parser could work fine without extra code.

lib/Target/RISCV/RISCVInstrFormatsC.td

37 ↗

(On Diff #123533)

Rename the instruction formats for compress instructions with "RVInst16" prefix.

lib/Target/RISCV/RISCVInstrInfoC.td

67 ↗

(On Diff #122779)

Adding comments to CL and CS classes that subclasses must set the immediate bits.

16 ↗

(On Diff #123533)

Fixup comments.

53 ↗

(On Diff #123533)

Rename the class to CStackLoad to match the naming rule in RISCVInstrInfo.td.

80 ↗

(On Diff #123533)

Using let Predicates = [HasStdExtC] instead of using require.

86 ↗

(On Diff #123533)

OK, I'll list the instructions base on the table in section 12.7 "RVC Instruction Set Listings" (page 82 and 83). Let me know if I still misorder something.

test/MC/RISCV/rv32c-invalid.s

10 ↗

(On Diff #123533)

Testing the invalid value just outside the valid range.

test/MC/RISCV/rv32c-valid.s

3 ↗

(On Diff #123533)

Remove redundant +64bit argument in the testcase.

13 ↗

(On Diff #123533)

check the maximum and minimum immediate values in -valids tests.

[asb]

Many many thanks for the patch update Shiva, this is looking pretty good. I have a concern in the comment about the immediate encoding for C.LWSP.

Shouldn't rv32c-invalid.s also check for cases like c.lwsp with rd as x0?

Thanks for clarifying the reasoning behind implySP - I can see now why you implemented things that way. Let me think a little more about it.

lib/Target/RISCV/RISCVInstrFormatsC.td

52 ↗

(On Diff #124038)

Perhaps phrase as something like "The immediate value encoding differs for each instruction, so each subclass is responsible for setting the appropriate bits in the Inst field." In fact, it would be even better to document the bits that must be set, e.g. Inst{12-10}.

lib/Target/RISCV/RISCVInstrInfoC.td

113 ↗

(On Diff #124038)

What about Inst{12}? I think we need more tests in rv32c-valid.s that verify the immediate encoding is correct.

[shiva0217]

Hi Alex, yes, the test case for operand should not x0 is missing. New revision updates the test case and adds GPRNonX0 register class for it.

lib/Target/RISCV/RISCVInstrFormatsC.td

52 ↗

(On Diff #124038)

Update the comment and specify the bits need to be set for each instruction.

lib/Target/RISCV/RISCVInstrInfoC.td

113 ↗

(On Diff #124038)

The Inst{12} is set by RVInst16CI due to the bit encoding is the same for each instruction in CI format.

[asb]

Thanks Shiva. I think we may revisit the "implied" SP operands later, but I'll go ahead and commit as-is.

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

bool isUImm5() const {
     int64_t Imm;
     RISCVMCExpr::VariantKind VK;
     if (!isImm())
       return false;
     bool IsConstantImm = evaluateConstantImm(Imm, VK);
     return IsConstantImm && isUInt<5>(Imm) && VK == RISCVMCExpr::VK_RISCV_None;
   }
 
+  bool isUImm7Lsb00() const {
+    int64_t Imm;
+    RISCVMCExpr::VariantKind VK;
+    bool IsConstantImm = evaluateConstantImm(Imm, VK);
+    return IsConstantImm && isShiftedUInt<5, 2>(Imm) &&
+           VK == RISCVMCExpr::VK_RISCV_None;
+  }
+
+  bool isUImm8Lsb00() const {
+    int64_t Imm;
+    RISCVMCExpr::VariantKind VK;
+    bool IsConstantImm = evaluateConstantImm(Imm, VK);
+    return IsConstantImm && isShiftedUInt<6, 2>(Imm) &&
+           VK == RISCVMCExpr::VK_RISCV_None;
+  }
+
+  bool isUImm8Lsb000() const {
+    int64_t Imm;
+    RISCVMCExpr::VariantKind VK;
+    bool IsConstantImm = evaluateConstantImm(Imm, VK);
+    return IsConstantImm && isShiftedUInt<5, 3>(Imm) &&
+           VK == RISCVMCExpr::VK_RISCV_None;
+  }
+
+  bool isUImm9Lsb000() const {
+    int64_t Imm;
+    RISCVMCExpr::VariantKind VK;
+    bool IsConstantImm = evaluateConstantImm(Imm, VK);
+    return IsConstantImm && isShiftedUInt<6, 3>(Imm) &&
+           VK == RISCVMCExpr::VK_RISCV_None;
+  }
+
   bool isSImm12() const {
     RISCVMCExpr::VariantKind VK;
     int64_t Imm;
     bool IsValid;
     if (!isImm())
       return false;
     bool IsConstantImm = evaluateConstantImm(Imm, VK);
     if (!IsConstantImm)
case Match_InvalidOperand: {
     return Error(ErrorLoc, "invalid operand for instruction");
   }
   case Match_InvalidUImmLog2XLen:
     if (isRV64())
       return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 6) - 1);
     return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 5) - 1);
   case Match_InvalidUImm5:
     return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 5) - 1);
+  case Match_InvalidUImm7Lsb00:
+    return generateImmOutOfRangeError(
+        Operands, ErrorInfo, 0, (1 << 7) - 4,
+        "immediate must be a multiple of 4 bytes in the range");
+  case Match_InvalidUImm8Lsb00:
+    return generateImmOutOfRangeError(
+        Operands, ErrorInfo, 0, (1 << 8) - 4,
+        "immediate must be a multiple of 4 bytes in the range");
+  case Match_InvalidUImm8Lsb000:
+    return generateImmOutOfRangeError(
+        Operands, ErrorInfo, 0, (1 << 8) - 8,
+        "immediate must be a multiple of 8 bytes in the range");
+  case Match_InvalidUImm9Lsb000:
+    return generateImmOutOfRangeError(
+        Operands, ErrorInfo, 0, (1 << 9) - 8,
+        "immediate must be a multiple of 8 bytes in the range");
   case Match_InvalidSImm12:
     return generateImmOutOfRangeError(Operands, ErrorInfo, -(1 << 11),
                                       (1 << 11) - 1);
   case Match_InvalidUImm12:
     return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 12) - 1);
   case Match_InvalidSImm13Lsb0:
     return generateImmOutOfRangeError(
         Operands, ErrorInfo, -(1 << 12), (1 << 12) - 2,

llvm/trunk/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp

static DecodeStatus DecodeFPR64RegisterClass(MCInst &Inst, uint64_t RegNo,
   // We must define our own mapping from RegNo to register identifier.
   // Accessing index RegNo in the register class will work in the case that
   // registers were added in ascending order, but not in general.
   unsigned Reg = FPR64DecoderTable[RegNo];
   Inst.addOperand(MCOperand::createReg(Reg));
   return MCDisassembler::Success;
 }
 
+static DecodeStatus DecodeGPRNoX0RegisterClass(MCInst &Inst, uint64_t RegNo,
+                                               uint64_t Address,
+                                               const void *Decoder) {
+   if (RegNo == 0) {
+     return MCDisassembler::Fail;
+   }
+
+   return DecodeGPRRegisterClass(Inst, RegNo, Address, Decoder);
+}
+
+static DecodeStatus DecodeGPRCRegisterClass(MCInst &Inst, uint64_t RegNo,
+                                            uint64_t Address,
+                                            const void *Decoder) {
+  if (RegNo > 8)
+    return MCDisassembler::Fail;
+
+  unsigned Reg = GPRDecoderTable[RegNo + 8];
+  Inst.addOperand(MCOperand::createReg(Reg));
+  return MCDisassembler::Success;
+}
+
+// Add implied SP operand for instructions *SP compressed instructions. The SP
+// operand isn't explicitly encoded in the instruction.
+static void addImplySP(MCInst &Inst, int64_t Address, const void *Decoder) {
+  if (Inst.getOpcode() == RISCV::CLWSP || Inst.getOpcode() == RISCV::CSWSP ||
+      Inst.getOpcode() == RISCV::CLDSP || Inst.getOpcode() == RISCV::CSDSP) {
+    DecodeGPRRegisterClass(Inst, 2, Address, Decoder);
+  }
+}
+
 template <unsigned N>
 static DecodeStatus decodeUImmOperand(MCInst &Inst, uint64_t Imm,
                                       int64_t Address, const void *Decoder) {
   assert(isUInt<N>(Imm) && "Invalid immediate");
+  addImplySP(Inst, Address, Decoder);
   Inst.addOperand(MCOperand::createImm(Imm));
   return MCDisassembler::Success;
 }
 
 template <unsigned N>
 static DecodeStatus decodeSImmOperand(MCInst &Inst, uint64_t Imm,
                                       int64_t Address, const void *Decoder) {
   assert(isUInt<N>(Imm) && "Invalid immediate");

 
 #include "RISCVGenDisassemblerTables.inc"
 
 DecodeStatus RISCVDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
                                                ArrayRef<uint8_t> Bytes,
                                                uint64_t Address,
                                                raw_ostream &OS,
                                                raw_ostream &CS) const {
-  // TODO: although assuming 4-byte instructions is sufficient for RV32 and
-  // RV64, this will need modification when supporting the compressed
-  // instruction set extension (RVC) which uses 16-bit instructions. Other
-  // instruction set extensions have the option of defining instructions up to
-  // 176 bits wide.
+  // TODO: This will need modification when supporting instruction set
+  // extensions with instructions > 32-bits (up to 176 bits wide).
+  uint32_t Insn;
+  DecodeStatus Result;
+
+  // It's a 32 bit instruction if bit 0 and 1 are 1.
+  if ((Bytes[0] & 0x3) == 0x3) {
+    Insn = support::endian::read32le(Bytes.data());
+    DEBUG(dbgs() << "Trying RISCV32 table :\n");
+    Result = decodeInstruction(DecoderTable32, MI, Insn, Address, this, STI);
     Size = 4;
-  if (Bytes.size() < 4) {
-    Size = 0;
-    return MCDisassembler::Fail;
+  } else {
+    Insn = support::endian::read16le(Bytes.data());
+    DEBUG(dbgs() << "Trying RISCV_C table (16-bit Instruction):\n");
+    // Calling the auto-generated decoder function.
+    Result = decodeInstruction(DecoderTable16, MI, Insn, Address, this, STI);
+    Size = 2;
   }
 
-  // Get the four bytes of the instruction.
-  uint32_t Inst = support::endian::read32le(Bytes.data());
-
-  return decodeInstruction(DecoderTable32, MI, Inst, Address, this, STI);
+  return Result;
 }

llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h

enum {
   InstFormatPseudo = 0,
   InstFormatR = 1,
   InstFormatR4 = 2,
   InstFormatI = 3,
   InstFormatS = 4,
   InstFormatB = 5,
   InstFormatU = 6,
   InstFormatJ = 7,
-  InstFormatOther = 8,
+  InstFormatCR = 8,
+  InstFormatCI = 9,
+  InstFormatCSS = 10,
+  InstFormatCIW = 11,
+  InstFormatCL = 12,
+  InstFormatCS = 13,
+  InstFormatCB = 14,
+  InstFormatCJ = 15,
+  InstFormatOther = 16,
 
-  InstFormatMask = 15
+  InstFormatMask = 31
 };
 
 enum {
   MO_None,
   MO_LO,
   MO_HI,
   MO_PCREL_HI,
 };

llvm/trunk/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp

MCCodeEmitter *llvm::createRISCVMCCodeEmitter(const MCInstrInfo &MCII,
                                               const MCRegisterInfo &MRI,
                                               MCContext &Ctx) {
   return new RISCVMCCodeEmitter(Ctx, MCII);
 }
 
 void RISCVMCCodeEmitter::encodeInstruction(const MCInst &MI, raw_ostream &OS,
                                            SmallVectorImpl<MCFixup> &Fixups,
                                            const MCSubtargetInfo &STI) const {
-  // For now, we only support RISC-V instructions with 32-bit length
+  const MCInstrDesc &Desc = MCII.get(MI.getOpcode());
+  // Get byte count of instruction.
+  unsigned Size = Desc.getSize();
+
+  switch (Size) {
+  default:
+    llvm_unreachable("Unhandled encodeInstruction length!");
+  case 2: {
+    uint16_t Bits = getBinaryCodeForInstr(MI, Fixups, STI);
+    support::endian::Writer<support::little>(OS).write<uint16_t>(Bits);
+    break;
+  }
+  case 4: {
     uint32_t Bits = getBinaryCodeForInstr(MI, Fixups, STI);
     support::endian::Writer<support::little>(OS).write(Bits);
+    break;
+  }
+  }
+
   ++MCNumEmitted; // Keep track of the # of mi's emitted.
 }
 
 unsigned
 RISCVMCCodeEmitter::getMachineOpValue(const MCInst &MI, const MCOperand &MO,
                                       SmallVectorImpl<MCFixup> &Fixups,
                                       const MCSubtargetInfo &STI) const {
 

llvm/trunk/lib/Target/RISCV/RISCV.td

 
 def FeatureStdExtD
     : SubtargetFeature<"d", "HasStdExtD", "true",
                        "'D' (Double-Precision Floating-Point)",
                        [FeatureStdExtF]>;
 def HasStdExtD : Predicate<"Subtarget->hasStdExtD()">,
                            AssemblerPredicate<"FeatureStdExtD">;
 
+def FeatureStdExtC
+    : SubtargetFeature<"c", "HasStdExtC", "true",
+                       "'C' (Compressed Instructions)">;
+def HasStdExtC : Predicate<"Subtarget->hasStdExtC()">,
+                           AssemblerPredicate<"FeatureStdExtC">;
+
+
 def Feature64Bit
     : SubtargetFeature<"64bit", "HasRV64", "true", "Implements RV64">;
 def IsRV64 : Predicate<"Subtarget->is64Bit()">,
                        AssemblerPredicate<"Feature64Bit">;
 
 def RV64           : HwMode<"+64bit">;
 def RV32           : HwMode<"-64bit">;
 

llvm/trunk/lib/Target/RISCV/RISCVInstrFormats.td

 //  a 21-bit value (where the LSB is always zero), we describe it as an imm20
 //  to match how it is encoded.
 //
 //===----------------------------------------------------------------------===//
 
 // Format specifies the encoding used by the instruction. This is used by
 // RISCVMCCodeEmitter to determine which form of fixup to use. These
 // definitions must be kept in-sync with RISCVBaseInfo.h.
-class InstFormat<bits<4> val> {
-  bits<4> Value = val;
+class InstFormat<bits<5> val> {
+  bits<5> Value = val;
 }
 def InstFormatPseudo : InstFormat<0>;
 def InstFormatR      : InstFormat<1>;
 def InstFormatR4     : InstFormat<2>;
 def InstFormatI      : InstFormat<3>;
 def InstFormatS      : InstFormat<4>;
 def InstFormatB      : InstFormat<5>;
 def InstFormatU      : InstFormat<6>;
 def InstFormatJ      : InstFormat<7>;
-def InstFormatOther  : InstFormat<8>;
+def InstFormatCR     : InstFormat<8>;
+def InstFormatCI     : InstFormat<9>;
+def InstFormatCSS    : InstFormat<10>;
+def InstFormatCIW    : InstFormat<11>;
+def InstFormatCL     : InstFormat<12>;
+def InstFormatCS     : InstFormat<13>;
+def InstFormatCB     : InstFormat<14>;
+def InstFormatCJ     : InstFormat<15>;
+def InstFormatOther  : InstFormat<16>;
 
 // The following opcode names and match those given in Table 19.1 in the
 // RISC-V User-level ISA specification ("RISC-V base opcode map").
 class RISCVOpcode<bits<7> val> {
   bits<7> Value = val;
 }
 def OPC_LOAD      : RISCVOpcode<0b0000011>;
 def OPC_LOAD_FP   : RISCVOpcode<0b0000111>;
class RVInst<dag outs, dag ins, string opcodestr, string argstr,
 
   let Namespace = "RISCV";
 
   dag OutOperandList = outs;
   dag InOperandList = ins;
   let AsmString = opcodestr # "\t" # argstr;
   let Pattern = pattern;
 
-  let TSFlags{3-0} = format.Value;
+  let TSFlags{4-0} = format.Value;
 }
 
 // Pseudo instructions
 class Pseudo<dag outs, dag ins, list<dag> pattern>
     : RVInst<outs, ins, "", "", pattern, InstFormatPseudo> {
   let isPseudo = 1;
   let isCodeGenOnly = 1;
 }

llvm/trunk/lib/Target/RISCV/RISCVInstrFormatsC.td

+//===-- RISCVInstrFormatsC.td - RISCV C Instruction Formats --*- tablegen -*-=//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file describes the RISC-V C extension instruction formats.
+//
+//===----------------------------------------------------------------------===//
+
+class RVInst16<dag outs, dag ins, string opcodestr, string argstr,
+               list<dag> pattern, InstFormat format>
+    : Instruction {
+  field bits<16> Inst;
+  // SoftFail is a field the disassembler can use to provide a way for
+  // instructions to not match without killing the whole decode process. It is
+  // mainly used for ARM, but Tablegen expects this field to exist or it fails
+  // to build the decode table.
+  field bits<16> SoftFail = 0;
+  let Size = 2;
+
+  bits<2> Opcode = 0;
+
+  let Namespace = "RISCV";
+
+  dag OutOperandList = outs;
+  dag InOperandList = ins;
+  let AsmString = opcodestr # "\t" # argstr;
+  let Pattern = pattern;
+
+  let TSFlags{4-0} = format.Value;
+}
+
+// The immediate value encoding differs for each instruction, so each subclass
+// is responsible for setting the appropriate bits in the Inst field.
+// The bits Inst{6-2} must be set for each instruction.
+class RVInst16CI<bits<3> funct3, bits<2> opcode, dag outs, dag ins,
+                 string opcodestr, string argstr>
+    : RVInst16<outs, ins, opcodestr, argstr, [], InstFormatCI> {
+  bits<10> imm;
+  bits<5> rd;
+  bits<5> rs1;
+
+  let Inst{15-13} = funct3;
+  let Inst{12} = imm{5};
+  let Inst{11-7} = rd;
+  let Inst{1-0} = opcode;
+}
+
+// The immediate value encoding differs for each instruction, so each subclass
+// is responsible for setting the appropriate bits in the Inst field.
+// The bits Inst{12-7} must be set for each instruction.
+class RVInst16CSS<bits<3> funct3, bits<2> opcode, dag outs, dag ins,
+                  string opcodestr, string argstr>
+    : RVInst16<outs, ins, opcodestr, argstr, [], InstFormatCSS> {
+  bits<10> imm;
+  bits<5> rs2;
+  bits<5> rs1;
+
+  let Inst{15-13} = funct3;
+  let Inst{6-2} = rs2;
+  let Inst{1-0} = opcode;
+}
+
+// The immediate value encoding differs for each instruction, so each subclass
+// is responsible for setting the appropriate bits in the Inst field.
+// The bits Inst{12-10} and Inst{6-5} must be set for each instruction.
+class RVInst16CL<bits<3> funct3, bits<2> opcode, dag outs, dag ins,
+                 string opcodestr, string argstr>
+    : RVInst16<outs, ins, opcodestr, argstr, [], InstFormatCL> {
+  bits<3> rd;
+  bits<3> rs1;
+
+  let Inst{15-13} = funct3;
+  let Inst{9-7} = rs1;
+  let Inst{4-2} = rd;
+  let Inst{1-0} = opcode;
+}
+
+// The immediate value encoding differs for each instruction, so each subclass
+// is responsible for setting the appropriate bits in the Inst field.
+// The bits Inst{12-10} and Inst{6-5} must be set for each instruction.
+class RVInst16CS<bits<3> funct3, bits<2> opcode, dag outs, dag ins,
+                 string opcodestr, string argstr>
+    : RVInst16<outs, ins, opcodestr, argstr, [], InstFormatCS> {
+  bits<3> rs2;
+  bits<3> rs1;
+
+  let Inst{15-13} = funct3;
+  let Inst{9-7} = rs1;
+  let Inst{4-2} = rs2;
+  let Inst{1-0} = opcode;
+}

llvm/trunk/lib/Target/RISCV/RISCVInstrInfo.td

 //===----------------------------------------------------------------------===//
 // Standard extensions
 //===----------------------------------------------------------------------===//
 
 include "RISCVInstrInfoM.td"
 include "RISCVInstrInfoA.td"
 include "RISCVInstrInfoF.td"
 include "RISCVInstrInfoD.td"
+include "RISCVInstrInfoC.td"

llvm/trunk/lib/Target/RISCV/RISCVInstrInfoC.td

+//===- RISCVInstrInfoC.td - Compressed RISCV instructions -*- tblgen-*-----===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+include "RISCVInstrFormatsC.td"
+
+//===----------------------------------------------------------------------===//
+// Operand definitions.
+//===----------------------------------------------------------------------===//
+
+// A 7-bit unsigned immediate where the least significant two bits are zero.
+def uimm7_lsb00 : Operand<XLenVT>,
+                  ImmLeaf<XLenVT, [{return isShiftedUInt<5, 2>(Imm);}]> {
+  let ParserMatchClass = UImmAsmOperand<7, "Lsb00">;
+  let EncoderMethod = "getImmOpValue";
+  let DecoderMethod = "decodeUImmOperand<7>";
+}
+
+// A 8-bit unsigned immediate where the least significant two bits are zero.
+def uimm8_lsb00 : Operand<XLenVT>,
+                  ImmLeaf<XLenVT, [{return isShiftedUInt<6, 2>(Imm);}]> {
+  let ParserMatchClass = UImmAsmOperand<8, "Lsb00">;
+  let EncoderMethod = "getImmOpValue";
+  let DecoderMethod = "decodeUImmOperand<8>";
+}
+
+// A 8-bit unsigned immediate where the least significant three bits are zero.
+def uimm8_lsb000 : Operand<XLenVT>,
+                   ImmLeaf<XLenVT, [{return isShiftedUInt<5, 3>(Imm);}]> {
+  let ParserMatchClass = UImmAsmOperand<8, "Lsb000">;
+  let EncoderMethod = "getImmOpValue";
+  let DecoderMethod = "decodeUImmOperand<8>";
+}
+
+// A 9-bit unsigned immediate where the least significant three bits are zero.
+def uimm9_lsb000 : Operand<XLenVT>,
+                   ImmLeaf<XLenVT, [{return isShiftedUInt<6, 3>(Imm);}]> {
+  let ParserMatchClass = UImmAsmOperand<9, "Lsb000">;
+  let EncoderMethod = "getImmOpValue";
+  let DecoderMethod = "decodeUImmOperand<9>";
+}
+
+//===----------------------------------------------------------------------===//
+// Instruction Class Templates
+//===----------------------------------------------------------------------===//
+
+let hasSideEffects = 0, mayLoad = 1, mayStore = 0 in
+class CStackLoad<bits<3> funct3, string OpcodeStr,
+                 RegisterClass cls, DAGOperand opnd> :
+      RVInst16CI<funct3, 0b10, (outs cls:$rd), (ins SP:$rs1, opnd:$imm),
+                 OpcodeStr, "$rd, ${imm}(${rs1})">;
+
+let hasSideEffects = 0, mayLoad = 0, mayStore = 1 in
+class CStackStore<bits<3> funct3, string OpcodeStr,
+                  RegisterClass cls, DAGOperand opnd> :
+      RVInst16CSS<funct3, 0b10, (outs), (ins cls:$rs2, SP:$rs1, opnd:$imm),
+                  OpcodeStr, "$rs2, ${imm}(${rs1})">;
+
+let hasSideEffects = 0, mayLoad = 1, mayStore = 0 in
+class CLoad_ri<bits<3> funct3, string OpcodeStr,
+               RegisterClass cls, DAGOperand opnd> :
+      RVInst16CL<funct3, 0b00, (outs cls:$rd), (ins cls:$rs1, opnd:$imm),
+                 OpcodeStr, "$rd, ${imm}(${rs1})">;
+
+let hasSideEffects = 0, mayLoad = 0, mayStore = 1 in
+class CStore_rri<bits<3> funct3, string OpcodeStr,
+                 RegisterClass cls, DAGOperand opnd> :
+      RVInst16CS<funct3, 0b00, (outs), (ins cls:$rs2, cls:$rs1, opnd:$imm),
+                 OpcodeStr, "$rs2, ${imm}(${rs1})">;
+
+//===----------------------------------------------------------------------===//
+// Instructions
+//===----------------------------------------------------------------------===//
+
+let Predicates = [HasStdExtC] in {
+
+def CLW : CLoad_ri<0b010, "c.lw", GPRC, uimm7_lsb00> {
+  bits<7> imm;
+  let Inst{12-10} = imm{5-3};
+  let Inst{6} = imm{2};
+  let Inst{5} = imm{6};
+}
+
+def CLD : CLoad_ri<0b011, "c.ld", GPRC, uimm8_lsb000>,
+          Requires<[IsRV64]> {
+  bits<8> imm;
+  let Inst{12-10} = imm{5-3};
+  let Inst{6-5} = imm{7-6};
+}
+
+def CSW : CStore_rri<0b110, "c.sw", GPRC, uimm7_lsb00> {
+  bits<7> imm;
+  let Inst{12-10} = imm{5-3};
+  let Inst{6} = imm{2};
+  let Inst{5} = imm{6};
+}
+
+def CSD : CStore_rri<0b111, "c.sd", GPRC, uimm8_lsb000>,
+          Requires<[IsRV64]> {
+  bits<8> imm;
+  let Inst{12-10} = imm{5-3};
+  let Inst{6-5} = imm{7-6};
+}
+
+def CLWSP : CStackLoad<0b010, "c.lwsp", GPRNoX0, uimm8_lsb00> {
+  let Inst{6-4} = imm{4-2};
+  let Inst{3-2} = imm{7-6};
+}
+
+def CLDSP : CStackLoad<0b011, "c.ldsp", GPRNoX0, uimm9_lsb000>,
+            Requires<[IsRV64]> {
+  let Inst{6-5} = imm{4-3};
+  let Inst{4-2} = imm{8-6};
+}
+
+def CSWSP : CStackStore<0b110, "c.swsp", GPR, uimm8_lsb00> {
+  let Inst{12-9} = imm{5-2};
+  let Inst{8-7}  = imm{7-6};
+}
+
+def CSDSP : CStackStore<0b111, "c.sdsp", GPR, uimm9_lsb000>,
+            Requires<[IsRV64]> {
+  let Inst{12-10} = imm{5-3};
+  let Inst{9-7}   = imm{8-6};
+}
+
+} // Predicates = [HasStdExtC]

llvm/trunk/lib/Target/RISCV/RISCVRegisterInfo.td

def GPR : RegisterClass<"RISCV", [XLenVT], 32, (add
     (sequence "X%u", 18, 27),
     (sequence "X%u", 0, 4)
   )> {
   let RegInfos = RegInfoByHwMode<
       [RV32,              RV64,              DefaultMode],
       [RegInfo<32,32,32>, RegInfo<64,64,64>, RegInfo<32,32,32>]>;
 }
 
+// The order of registers represents the preferred allocation sequence.
+// Registers are listed in the order caller-save, callee-save, specials.
+def GPRNoX0 : RegisterClass<"RISCV", [XLenVT], 32, (add
+    (sequence "X%u", 10, 17),
+    (sequence "X%u", 5, 7),
+    (sequence "X%u", 28, 31),
+    (sequence "X%u", 8, 9),
+    (sequence "X%u", 18, 27),
+    (sequence "X%u", 1, 4)
+  )> {
+  let RegInfos = RegInfoByHwMode<
+      [RV32,              RV64,              DefaultMode],
+      [RegInfo<32,32,32>, RegInfo<64,64,64>, RegInfo<32,32,32>]>;
+}
+
+def GPRC : RegisterClass<"RISCV", [XLenVT], 32, (add
+    (sequence "X%u", 10, 15),
+    (sequence "X%u", 8, 9)
+  )> {
+  let RegInfos = RegInfoByHwMode<
+      [RV32,              RV64,              DefaultMode],
+      [RegInfo<32,32,32>, RegInfo<64,64,64>, RegInfo<32,32,32>]>;
+}
+
+def SP : RegisterClass<"RISCV", [XLenVT], 32, (add X2)> {
+  let RegInfos = RegInfoByHwMode<
+      [RV32,              RV64,              DefaultMode],
+      [RegInfo<32,32,32>, RegInfo<64,64,64>, RegInfo<32,32,32>]>;
+}
+
 // Floating point registers
 let RegAltNameIndices = [ABIRegAltName] in {
   def F0_32  : RISCVReg32<0, "f0", ["ft0"]>, DwarfRegNum<[32]>;
   def F1_32  : RISCVReg32<1, "f1", ["ft1"]>, DwarfRegNum<[33]>;
   def F2_32  : RISCVReg32<2, "f2", ["ft2"]>, DwarfRegNum<[34]>;
   def F3_32  : RISCVReg32<3, "f3", ["ft3"]>, DwarfRegNum<[35]>;
   def F4_32  : RISCVReg32<4, "f4", ["ft4"]>, DwarfRegNum<[36]>;
   def F5_32  : RISCVReg32<5, "f5", ["ft5"]>, DwarfRegNum<[37]>;

llvm/trunk/lib/Target/RISCV/RISCVSubtarget.h

 class StringRef;
 
 class RISCVSubtarget : public RISCVGenSubtargetInfo {
   virtual void anchor();
   bool HasStdExtM = false;
   bool HasStdExtA = false;
   bool HasStdExtF = false;
   bool HasStdExtD = false;
+  bool HasStdExtC = false;
   bool HasRV64 = false;
   unsigned XLen = 32;
   MVT XLenVT = MVT::i32;
   RISCVFrameLowering FrameLowering;
   RISCVInstrInfo InstrInfo;
   RISCVRegisterInfo RegInfo;
   RISCVTargetLowering TLInfo;
   SelectionDAGTargetInfo TSInfo;
public:
   }
   const SelectionDAGTargetInfo *getSelectionDAGInfo() const override {
     return &TSInfo;
   }
   bool hasStdExtM() const { return HasStdExtM; }
   bool hasStdExtA() const { return HasStdExtA; }
   bool hasStdExtF() const { return HasStdExtF; }
   bool hasStdExtD() const { return HasStdExtD; }
+  bool hasStdExtC() const { return HasStdExtC; }
   bool is64Bit() const { return HasRV64; }
   MVT getXLenVT() const { return XLenVT; }
   unsigned getXLen() const { return XLen; }
 };
 } // End llvm namespace
 
 #endif

llvm/trunk/test/MC/RISCV/rv32c-invalid.s

+# RUN: not llvm-mc -triple=riscv32 -mattr=+c < %s 2>&1 | FileCheck %s
+
+## GPRC
+c.lw  ra, 4(sp) # CHECK: :[[@LINE]]:7: error: invalid operand for instruction
+c.sw  sp, 4(sp) # CHECK: :[[@LINE]]:7: error: invalid operand for instruction
+
+## GPRNoX0
+c.lwsp  x0, 4(sp) # CHECK: :[[@LINE]]:9: error: invalid operand for instruction
+c.lwsp  zero, 4(sp) # CHECK: :[[@LINE]]:9: error: invalid operand for instruction
+
+# Out of range immediates
+
+## uimm8_lsb00
+c.lwsp  ra, 256(sp) # CHECK: :[[@LINE]]:13: error: immediate must be a multiple of 4 bytes in the range [0, 252]
+c.swsp  ra, -4(sp) # CHECK: :[[@LINE]]:13: error: immediate must be a multiple of 4 bytes in the range [0, 252]
+## uimm7_lsb00
+c.lw  s0, -4(sp) # CHECK: :[[@LINE]]:11: error: immediate must be a multiple of 4 bytes in the range [0, 124]
+c.sw  s0, 128(sp) # CHECK: :[[@LINE]]:11: error: immediate must be a multiple of 4 bytes in the range [0, 124]

llvm/trunk/test/MC/RISCV/rv32c-valid.s

+# RUN: llvm-mc -triple=riscv32 -mattr=+c -show-encoding < %s \
+# RUN:     | FileCheck -check-prefixes=CHECK,CHECK-INST %s
+# RUN: llvm-mc -triple=riscv64 -mattr=+c -show-encoding < %s \
+# RUN:     | FileCheck -check-prefixes=CHECK,CHECK-INST %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+c < %s \
+# RUN:     | llvm-objdump -mattr=+c -d - | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+c < %s \
+# RUN:     | llvm-objdump -mattr=+c -d - | FileCheck -check-prefix=CHECK-INST %s
+
+# TODO: more exhaustive testing of immediate encoding.
+
+# CHECK-INST: c.lwsp  ra, 0(sp)
+# CHECK: encoding: [0x82,0x40]
+c.lwsp  ra, 0(sp)
+# CHECK-INST: c.swsp  ra, 252(sp)
+# CHECK: encoding: [0x86,0xdf]
+c.swsp  ra, 252(sp)
+# CHECK-INST: c.lw    a2, 0(a0)
+# CHECK: encoding: [0x10,0x41]
+c.lw    a2, 0(a0)
+# CHECK-INST: c.sw    a5, 124(a3)
+# CHECK: encoding: [0xfc,0xde]
+c.sw    a5, 124(a3)

llvm/trunk/test/MC/RISCV/rv64c-invalid.s

+# RUN: not llvm-mc -triple=riscv64 -mattr=+c < %s 2>&1 | FileCheck %s
+
+## GPRC
+c.ld ra, 4(sp) # CHECK: :[[@LINE]]:6: error: invalid operand for instruction
+c.sd sp, 4(sp) # CHECK: :[[@LINE]]:6: error: invalid operand for instruction
+
+## GPRNoX0
+c.ldsp  x0, 4(sp) # CHECK: :[[@LINE]]:9: error: invalid operand for instruction
+c.ldsp  zero, 4(sp) # CHECK: :[[@LINE]]:9: error: invalid operand for instruction
+
+# Out of range immediates
+
+## uimm9_lsb000
+c.ldsp  ra, 512(sp) # CHECK: :[[@LINE]]:13: error: immediate must be a multiple of 8 bytes in the range [0, 504]
+c.sdsp  ra, -8(sp) # CHECK: :[[@LINE]]:13: error: immediate must be a multiple of 8 bytes in the range [0, 504]
+## uimm8_lsb000
+c.ld  s0, -8(sp) # CHECK: :[[@LINE]]:11: error: immediate must be a multiple of 8 bytes in the range [0, 248]
+c.sd  s0, 256(sp) # CHECK: :[[@LINE]]:11: error: immediate must be a multiple of 8 bytes in the range [0, 248]

llvm/trunk/test/MC/RISCV/rv64c-valid.s

+# RUN: llvm-mc -triple=riscv64 -mattr=+c -show-encoding < %s \
+# RUN:     | FileCheck -check-prefixes=CHECK,CHECK-INST %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+c < %s \
+# RUN:     | llvm-objdump -mattr=+c -d - | FileCheck -check-prefix=CHECK-INST %s
+
+# TODO: more exhaustive testing of immediate encoding.
+
+# CHECK-INST: c.ldsp  ra, 0(sp)
+# CHECK: encoding: [0x82,0x60]
+c.ldsp  ra, 0(sp)
+# CHECK-INST: c.sdsp  ra, 504(sp)
+# CHECK: encoding: [0x86,0xff]
+c.sdsp  ra, 504(sp)
+# CHECK-INST: c.ld    a4, 0(a3)
+# CHECK: encoding: [0x98,0x62]
+c.ld    a4, 0(a3)
+# CHECK-INST: c.sd    a5, 248(a3)
+# CHECK: encoding: [0xfc,0xfe]
+c.sd    a5, 248(a3)