[CodeEmitter] Support instruction widths > 64 bits

[jmolloy]

Summary

Some VLIW instruction sets are Very Long Indeed. Using uint64_t constricts the Inst encoding to 64 bits (naturally).

This change switches CodeEmitter to a mode that uses APInts when Inst's bitwidth is > 64 bits (NFC for existing targets).

When Inst.BitWidth > 64 the prototype changes to:

void TargetMCCodeEmitter::getBinaryCodeForInstr(const MCInst &MI,
                                                SmallVectorImpl<MCFixup> &Fixups,
                                                APInt &Inst,
                                                APInt &Scratch,
                                                const MCSubtargetInfo &STI);

The Inst parameter returns the encoded instruction, the Scratch parameter is used internally for manipulating operands and is exposed so that the underlying storage can be reused between calls to getBinaryCodeForInstr. The goal is to elide any APInt constructions that we can.

Similarly the operand encoding prototype changes to:

getMachineOpValue(const MCInst &MI, const MCOperand &MO, APInt &op, SmallVectorImpl<MCFixup> &Fixups, const MCSubtargetInfo &STI);

That is, the operand is passed by reference as APInt rather than returned as uint64_t.

To reiterate, this APInt mode is enabled only when Inst.BitWidth > 64, so while no existing in-tree target makes use of that,
there's basic test coverage in llvm/test/TableGen/RegisterEncoder.td.

[bkramer]

I think this is fine. Features like this might bitrot quickly but that's on you ;)

llvm/utils/TableGen/CodeEmitterGen.cpp

309 ↗

(On Diff #219875)

utostr() isn't needed when printing to a raw_ostream

[lebedev.ri]

Can you quote where https://llvm.org/docs/DeveloperPolicy.html#test-cases says it's okay to commit dead code with no test coverage?

[xbolva00]

There are tests in llvm/test/TableGen/BigEncoder.td.

[lebedev.ri]

On Fri, Sep 13, 2019 at 2:03 PM James Molloy <[e-mail]> wrote:

Hi Roman,

There is one diff to a test and one brand new test in this change (llvm/test/TableGen/BigEncoder.td).

Re test coverage: that is not really how the patch description was being read, usually that wording means something different.

As a sidenote, your message could be considered by some unnecessarily passive-aggressive.

Cheers,

James

[tra]

llvm/test/TableGen/BigEncoder.td

2 ↗

(On Diff #219875)

I'd add some details explaining what the test does and why.
Otherwise it's not clear what this test does and why without the full context of the patch.
Those bits<65> are not very noticeable on their own.

59–69 ↗

(On Diff #219875)

checks for results of bar/baz are missing.

llvm/trunk/test/TableGen/BigEncoder.td

def biz : Instruction {
     field bits<65> Inst;
     bits<32> factor;
     let Inst{7-0} = 0xCC;
     let Inst{11-8,15-12} = factor{10-3}; // offset by 3, multipart
     let AsmString = "biz  $factor";
     field bits<16> SoftFail = 0;
     }
 
-def baz : Instruction {
-    let InOperandList = (ins Myi32:$factor);
-    field bits<65> Inst;
-    bits<32> factor;
-    let Inst{7-0} = 0xDD;
-    let Inst{15-8} = factor{11-4}; // offset by 4 + custom decode
-    let AsmString = "baz  $factor";
-    field bits<16> SoftFail = 0;
-    }
 
-}
 // CHECK-LABEL: case ::biz: {
 // CHECK: const APInt [[x:M[0-9]+]] = APInt::getBitsSet(65, 3, 7);
 // CHECK-NEXT: Value |= (op & [[x]]) << 9;
 // CHECK-NEXT: const APInt [[y:M[0-9]+]] = APInt::getBitsSet(65, 7, 11);
 // CHECK-NEXT: Value |= (op & [[y]]) << 1;
 
 // CHECK-LABEL: case ::foo: {
 // CHECK: const APInt [[x:M[0-9]+]] = APInt::getBitsSet(65, 0, 7);
 // CHECK-NEXT: op &= [[x]];
 // CHECK-NEXT: op <<= 8;
 // CHECK-NEXT: Value |= op;
+
+// CHECK-LABEL: case ::bar: {
+// CHECK: const APInt [[x:M[0-9]+]] = APInt::getBitsSet(65, 3, 11);
+// CHECK-NEXT: op &= [[x]];
+// CHECK-NEXT: op <<= 5;
+// CHECK-NEXT: Value |= op;