[RISCV] Add way to mark CompressPats that should only be used for compressing.

[craig.topper]

Summary

There can be muliple patterns that map to the same compressed
instruction. Reversing those leads to multiple ways to uncompress
an instruction, but its not easily controllable which one will
be chosen by the tablegen backend.

This patch adds a flag to mark patterns that should only be used
for compressing. This allows us to leave one canonical pattern
for uncompressing.

The obvious benefit of this is getting c.mv to uncompress to
the addi patern that is aliased to the mv pseudoinstruction. For
the add/and/or/xor/li patterns it just removes some unreachable
code from the generated code.

[craig.topper]

Add test updates

[jrtc27]

My instinct is this should be a priority to match InstAlias; is there ever any overlap?

[craig.topper]

In D94894#2504716, @jrtc27 wrote:

My instinct is this should be a priority to match InstAlias; is there ever any overlap?

After this patch, uncompressing is a 1:1 mapping. In the compress direction there are multiple patterns that map to the same sequence. There are also multiple patterns that start from ADD for example. Overlaps seem to be handled by Register classes like GPRNoX0 and by immediate predicates. If a priority is needed, I believe it would be needed in the compress direction, not the uncompress direction.

[jrtc27]

In D94894#2505039, @craig.topper wrote:

In D94894#2504716, @jrtc27 wrote:

My instinct is this should be a priority to match InstAlias; is there ever any overlap?

After this patch, uncompressing is a 1:1 mapping. In the compress direction there are multiple patterns that map to the same sequence. There are also multiple patterns that start from ADD for example. Overlaps seem to be handled by Register classes like GPRNoX0 and by immediate predicates. If a priority is needed, I believe it would be needed in the compress direction, not the uncompress direction.

Right, yes, which corresponds to the InstAlias direction that corresponds to the priority (InstAlias, at least to me, is conceptually the other way round to CompressPat). I wasn't particularly thinking; decompression is of course a (total) function, that's how a lot of cores implement decoding compressed instructions.

[craig.topper]

In D94894#2505076, @jrtc27 wrote:

In D94894#2505039, @craig.topper wrote:

In D94894#2504716, @jrtc27 wrote:

My instinct is this should be a priority to match InstAlias; is there ever any overlap?

After this patch, uncompressing is a 1:1 mapping. In the compress direction there are multiple patterns that map to the same sequence. There are also multiple patterns that start from ADD for example. Overlaps seem to be handled by Register classes like GPRNoX0 and by immediate predicates. If a priority is needed, I believe it would be needed in the compress direction, not the uncompress direction.

Right, yes, which corresponds to the InstAlias direction that corresponds to the priority (InstAlias, at least to me, is conceptually the other way round to CompressPat). I wasn't particularly thinking; decompression is of course a (total) function, that's how a lot of cores implement decoding compressed instructions.

So are you ok with this patch as is?

[jrtc27]

In D94894#2505707, @craig.topper wrote:

In D94894#2505076, @jrtc27 wrote:

In D94894#2505039, @craig.topper wrote:

In D94894#2504716, @jrtc27 wrote:

My instinct is this should be a priority to match InstAlias; is there ever any overlap?

After this patch, uncompressing is a 1:1 mapping. In the compress direction there are multiple patterns that map to the same sequence. There are also multiple patterns that start from ADD for example. Overlaps seem to be handled by Register classes like GPRNoX0 and by immediate predicates. If a priority is needed, I believe it would be needed in the compress direction, not the uncompress direction.

Right, yes, which corresponds to the InstAlias direction that corresponds to the priority (InstAlias, at least to me, is conceptually the other way round to CompressPat). I wasn't particularly thinking; decompression is of course a (total) function, that's how a lot of cores implement decoding compressed instructions.

So are you ok with this patch as is?

Sorry, yes, didn't realise I hadn't made myself clear.

[frasercrmck]

llvm/lib/Target/RISCV/RISCVInstrInfoC.td

904

Is there a reason why this is true but the others are 1?

[craig.topper]

llvm/lib/Target/RISCV/RISCVInstrInfoC.td

904

I think I wrote this one first and forced myself to use true. I made sure it worked then came back and did the others. I guess I fell back into not expecting true/false in tablegen.

[craig.topper]

Consistently use isCompressOnly = true

[frasercrmck]

LGTM

llvm/lib/Target/RISCV/RISCVInstrInfoC.td

 //===----------------------------------------------------------------------===//
 // Compress Instruction tablegen backend.
 //===----------------------------------------------------------------------===//
 
 class CompressPat<dag input, dag output> {
   dag Input  = input;
   dag Output    = output;
   list<Predicate> Predicates = [];
+  bit isCompressOnly = false;
 }
 
 // Patterns are defined in the same order the compressed instructions appear
 // on page 82 of the ISA manual.
 
 // Quadrant 0
 let Predicates = [HasStdExtC] in {
 def : CompressPat<(ADDI GPRC:$rd, SP:$rs1, uimm10_lsb00nonzero:$imm),

 def : CompressPat<(SRAI GPRC:$rs1, GPRC:$rs1, uimmlog2xlennonzero:$imm),
                   (C_SRAI GPRC:$rs1, uimmlog2xlennonzero:$imm)>;
 def : CompressPat<(ANDI GPRC:$rs1, GPRC:$rs1, simm6:$imm),
                   (C_ANDI GPRC:$rs1, simm6:$imm)>;
 def : CompressPat<(SUB GPRC:$rs1, GPRC:$rs1, GPRC:$rs2),
                   (C_SUB GPRC:$rs1, GPRC:$rs2)>;
 def : CompressPat<(XOR GPRC:$rs1, GPRC:$rs1, GPRC:$rs2),
                   (C_XOR GPRC:$rs1, GPRC:$rs2)>;
+let isCompressOnly = true in
 def : CompressPat<(XOR GPRC:$rs1, GPRC:$rs2, GPRC:$rs1),
                   (C_XOR GPRC:$rs1, GPRC:$rs2)>;
 def : CompressPat<(OR GPRC:$rs1, GPRC:$rs1, GPRC:$rs2),
                   (C_OR GPRC:$rs1, GPRC:$rs2)>;
+let isCompressOnly = true in
 def : CompressPat<(OR GPRC:$rs1, GPRC:$rs2, GPRC:$rs1),
                   (C_OR GPRC:$rs1, GPRC:$rs2)>;
 def : CompressPat<(AND GPRC:$rs1, GPRC:$rs1, GPRC:$rs2),
                   (C_AND GPRC:$rs1, GPRC:$rs2)>;
+let isCompressOnly = true in
 def : CompressPat<(AND GPRC:$rs1, GPRC:$rs2, GPRC:$rs1),
                   (C_AND GPRC:$rs1, GPRC:$rs2)>;
 } //  Predicates = [HasStdExtC]
 
 let Predicates = [HasStdExtC, IsRV64] in {
+let isCompressOnly = true in
 def : CompressPat<(ADDIW GPRNoX0:$rd, X0, simm6:$imm),
                   (C_LI GPRNoX0:$rd, simm6:$imm)>;
 def : CompressPat<(SUBW GPRC:$rs1, GPRC:$rs1, GPRC:$rs2),
                   (C_SUBW GPRC:$rs1, GPRC:$rs2)>;
 def : CompressPat<(ADDW GPRC:$rs1, GPRC:$rs1, GPRC:$rs2),
                    (C_ADDW GPRC:$rs1, GPRC:$rs2)>;
+let isCompressOnly = true in
 def : CompressPat<(ADDW GPRC:$rs1, GPRC:$rs2, GPRC:$rs1),
                    (C_ADDW GPRC:$rs1, GPRC:$rs2)>;
 } // Predicates = [HasStdExtC, IsRV64]
 
 let Predicates = [HasStdExtC] in {
 def : CompressPat<(JAL X0, simm12_lsb0:$offset),
                   (C_J simm12_lsb0:$offset)>;
 def : CompressPat<(BEQ GPRC:$rs1, X0, simm9_lsb0:$imm),

 let Predicates = [HasStdExtC, IsRV64] in {
 def : CompressPat<(LD GPRNoX0:$rd, SP:$rs1, uimm9_lsb000:$imm),
                   (C_LDSP GPRNoX0:$rd, SP:$rs1, uimm9_lsb000:$imm)>;
 } // Predicates = [HasStdExtC, IsRV64]
 
 let Predicates = [HasStdExtC] in {
 def : CompressPat<(JALR X0, GPRNoX0:$rs1, 0),
                   (C_JR GPRNoX0:$rs1)>;
+let isCompressOnly = true in {
 def : CompressPat<(ADD GPRNoX0:$rs1, X0, GPRNoX0:$rs2),
                   (C_MV GPRNoX0:$rs1, GPRNoX0:$rs2)>;
 def : CompressPat<(ADD GPRNoX0:$rs1, GPRNoX0:$rs2, X0),
                   (C_MV GPRNoX0:$rs1, GPRNoX0:$rs2)>;
+}
 def : CompressPat<(ADDI GPRNoX0:$rs1, GPRNoX0:$rs2, 0),
                   (C_MV GPRNoX0:$rs1, GPRNoX0:$rs2)>;
 def : CompressPat<(EBREAK), (C_EBREAK)>;
 def : CompressPat<(UNIMP), (C_UNIMP)>;
 def : CompressPat<(JALR X1, GPRNoX0:$rs1, 0),
                   (C_JALR GPRNoX0:$rs1)>;
 def : CompressPat<(ADD GPRNoX0:$rs1, GPRNoX0:$rs1, GPRNoX0:$rs2),
                   (C_ADD GPRNoX0:$rs1, GPRNoX0:$rs2)>;
+let isCompressOnly = true in
 def : CompressPat<(ADD GPRNoX0:$rs1, GPRNoX0:$rs2, GPRNoX0:$rs1),
                   (C_ADD GPRNoX0:$rs1, GPRNoX0:$rs2)>;
 } // Predicates = [HasStdExtC]
 
 let Predicates = [HasStdExtC, HasStdExtD] in {
 def : CompressPat<(FSD FPR64:$rs2, SP:$rs1, uimm9_lsb000:$imm),
                   (C_FSDSP FPR64:$rs2, SP:$rs1, uimm9_lsb000:$imm)>;
 } // Predicates = [HasStdExtC, HasStdExtD]

llvm/test/MC/RISCV/compress-rv32i.s

 # RUN: llvm-mc -triple riscv64 -mattr=+c -filetype=obj < %s \
 # RUN: | llvm-objdump  --triple=riscv64 --mattr=+c -d - \
 # RUN: | FileCheck -check-prefixes=CHECK-BYTES,CHECK-ALIAS,CHECK-ALIASOBJ64 %s
 # RUN: llvm-mc -triple riscv64 -mattr=+c -filetype=obj < %s \
 # RUN: | llvm-objdump  --triple=riscv64 --mattr=+c -d -M no-aliases - \
 # RUN: | FileCheck -check-prefixes=CHECK-BYTES,CHECK-INST,CHECK-INSTOBJ64 %s
 
 # CHECK-BYTES: 2e 85
-# CHECK-ALIAS: add a0, zero, a1
+# CHECK-ALIAS: mv a0, 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]

 
 # CHECK-BYTES: 82 80
 # CHECK-ALIAS: ret
 # CHECK-INST: c.jr ra
 # CHECK: # encoding:  [0x82,0x80]
 jalr zero, 0(ra)
 
 # CHECK-BYTES: 92 80
-# CHECK-ALIAS: add ra, zero, tp
+# CHECK-ALIAS: mv ra, tp
 # CHECK-INST: c.mv ra, tp
 # CHECK:  # encoding: [0x92,0x80]
 add ra, zero, tp
 
 # CHECK-BYTES: 92 80
-# CHECK-ALIAS: add ra, zero, tp
+# CHECK-ALIAS: mv ra, tp
 # CHECK-INST: c.mv ra, tp
 # CHECK:  # encoding: [0x92,0x80]
 add ra, tp, zero
 
 # CHECK-BYTES: 02 90
 # CHECK-ALIAS: ebreak
 # CHECK-INST: c.ebreak
 # CHECK: # encoding: [0x02,0x90]

llvm/test/MC/RISCV/option-rvc.s

 # 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-ALIAS: mv a0, 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]

 # 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-ALIAS: mv a0, 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]

llvm/test/TableGen/AsmPredicateCombiningRISCV.td

 def AsmPred3 : Predicate<"Pred3">, AssemblerPredicate<(any_of AsmCond3a, AsmCond3b)>;
 
 def BigInst : RVInst<1, [AsmPred1]>;
 
 class CompressPat<dag input, dag output, list<Predicate> predicates> {
   dag Input = input;
   dag Output = output;
   list<Predicate> Predicates = predicates;
+  bit isCompressOnly = false;
 }
 
 // COMPRESS-LABEL: static bool compressInst
 // COMPRESS: case arch::BigInst
 def SmallInst1 : RVInst16<1, []>;
 def : CompressPat<(BigInst Regs:$r), (SmallInst1 Regs:$r), [AsmPred1]>;
 // COMPRESS:      if (STI.getFeatureBits()[arch::AsmCond1] &&
 // COMPRESS-NEXT: (MRI.getRegClass(arch::RegsRegClassID).contains(MI.getOperand(0).getReg()))) {

llvm/utils/TableGen/RISCVCompressInstEmitter.cpp

struct CompressPat {
     std::vector<Record *>
         PatReqFeatures; // Required target features to enable pattern.
     IndexedMap<OpData>
         SourceOperandMap; // Maps operands in the Source Instruction to
                           // the corresponding Dest instruction operand.
     IndexedMap<OpData>
         DestOperandMap; // Maps operands in the Dest Instruction
                         // to the corresponding Source instruction operand.
+    bool IsCompressOnly;
     CompressPat(CodeGenInstruction &S, CodeGenInstruction &D,
                 std::vector<Record *> RF, IndexedMap<OpData> &SourceMap,
-                IndexedMap<OpData> &DestMap)
+                IndexedMap<OpData> &DestMap, bool IsCompressOnly)
         : Source(S), Dest(D), PatReqFeatures(RF), SourceOperandMap(SourceMap),
-          DestOperandMap(DestMap) {}
+          DestOperandMap(DestMap), IsCompressOnly(IsCompressOnly) {}
   };
   enum EmitterType { Compress, Uncompress, CheckCompress };
   RecordKeeper &Records;
   CodeGenTarget Target;
   SmallVector<CompressPat, 4> CompressPatterns;
 
   void addDagOperandMapping(Record *Rec, DagInit *Dag, CodeGenInstruction &Inst,
                             IndexedMap<OpData> &OperandMap, bool IsSourceInst);
void RISCVCompressInstEmitter::evaluateCompressPat(Record *Rec) {
   // Get the target features for the CompressPat.
   std::vector<Record *> PatReqFeatures;
   std::vector<Record *> RF = Rec->getValueAsListOfDefs("Predicates");
   copy_if(RF, std::back_inserter(PatReqFeatures), [](Record *R) {
     return R->getValueAsBit("AssemblerMatcherPredicate");
   });
 
   CompressPatterns.push_back(CompressPat(SourceInst, DestInst, PatReqFeatures,
-                                         SourceOperandMap, DestOperandMap));
+                                         SourceOperandMap, DestOperandMap,
+                                         Rec->getValueAsBit("isCompressOnly")));
 }
 
 static void
 getReqFeatures(std::set<std::pair<bool, StringRef>> &FeaturesSet,
                std::set<std::set<std::pair<bool, StringRef>>> &AnyOfFeatureSets,
                const std::vector<Record *> &ReqFeatures) {
   for (auto &R : ReqFeatures) {
     const DagInit *D = R->getValueAsDag("AssemblerCondDag");
void RISCVCompressInstEmitter::emitCompressInstEmitter(raw_ostream &o,
   CaseStream << "    default: return false;\n";
 
   bool CompressOrCheck =
     EType == EmitterType::Compress || EType == EmitterType::CheckCompress;
   bool CompressOrUncompress =
     EType == EmitterType::Compress || EType == EmitterType::Uncompress;
 
   for (auto &CompressPat : CompressPatterns) {
+    if (EType == EmitterType::Uncompress && CompressPat.IsCompressOnly)
+      continue;
+
     std::string CondString;
     std::string CodeString;
     raw_string_ostream CondStream(CondString);
     raw_string_ostream CodeStream(CodeString);
     CodeGenInstruction &Source =
         CompressOrCheck ?  CompressPat.Source : CompressPat.Dest;
     CodeGenInstruction &Dest =
         CompressOrCheck ? CompressPat.Dest : CompressPat.Source;