[llvm] r336334 - [TableGen] Increase the number of supported decoder fix-ups.
Ahmed Bougacha via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 5 13:37:39 PDT 2018
On Thu, Jul 5, 2018 at 3:39 AM, Sander de Smalen via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: s.desmalen
> Date: Thu Jul 5 03:39:15 2018
> New Revision: 336334
>
> URL: http://llvm.org/viewvc/llvm-project?rev=336334&view=rev
> Log:
> [TableGen] Increase the number of supported decoder fix-ups.
>
> The vast number of added instructions for SVE causes TableGen to fail with an assertion:
>
> Assertion `Delta < 65536U && "disassembler decoding table too large!"'
We also started hitting this downstream, but I'm wondering if we can
avoid the growth.
For instance, I was considering switching the offset to ULEB128, and
that's something we could probably do in a lot of generated
tables. It doesn't seem too complicated (we have APIs in libSupport,
thought they could perhaps be more user-friendly), or expensive (it's
a pretty simple scheme).
Alternatively, I looked at the table, and it seems that most of the
large deltas are just jumping to the final OPC_Fail. Why don't we
just special-case that with all 1s or something? Skimming over the
aarch64 tables, we probably don't even need more than 8 bits if we did
that.
WDYT?
-Ahmed
>
> This patch increases the number of supported decoder fix-ups.
>
> Reviewers: dmgreen, stoklund, petpav01
>
> Reviewed By: dmgreen
>
> Differential Revision: https://reviews.llvm.org/D48937
>
>
> Modified:
> llvm/trunk/test/TableGen/trydecode-emission.td
> llvm/trunk/test/TableGen/trydecode-emission2.td
> llvm/trunk/test/TableGen/trydecode-emission3.td
> llvm/trunk/utils/TableGen/FixedLenDecoderEmitter.cpp
>
> Modified: llvm/trunk/test/TableGen/trydecode-emission.td
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/TableGen/trydecode-emission.td?rev=336334&r1=336333&r2=336334&view=diff
> ==============================================================================
> --- llvm/trunk/test/TableGen/trydecode-emission.td (original)
> +++ llvm/trunk/test/TableGen/trydecode-emission.td Thu Jul 5 03:39:15 2018
> @@ -34,10 +34,10 @@ def InstB : TestInstruction {
> }
>
> // CHECK: /* 0 */ MCD::OPC_ExtractField, 4, 4, // Inst{7-4} ...
> -// CHECK-NEXT: /* 3 */ MCD::OPC_FilterValue, 0, 14, 0, // Skip to: 21
> -// CHECK-NEXT: /* 7 */ MCD::OPC_CheckField, 2, 2, 0, 5, 0, // Skip to: 18
> -// CHECK-NEXT: /* 13 */ MCD::OPC_TryDecode, {{[0-9]+}}, 0, 0, 0, // Opcode: InstB, skip to: 18
> -// CHECK-NEXT: /* 18 */ MCD::OPC_Decode, {{[0-9]+}}, 1, // Opcode: InstA
> -// CHECK-NEXT: /* 21 */ MCD::OPC_Fail,
> +// CHECK-NEXT: /* 3 */ MCD::OPC_FilterValue, 0, 16, 0, 0, // Skip to: 24
> +// CHECK-NEXT: /* 8 */ MCD::OPC_CheckField, 2, 2, 0, 6, 0, 0, // Skip to: 21
> +// CHECK-NEXT: /* 15 */ MCD::OPC_TryDecode, {{[0-9]+}}, 0, 0, 0, 0, // Opcode: InstB, skip to: 21
> +// CHECK-NEXT: /* 21 */ MCD::OPC_Decode, {{[0-9]+}}, 1, // Opcode: InstA
> +// CHECK-NEXT: /* 24 */ MCD::OPC_Fail,
>
> // CHECK: if (DecodeInstB(MI, insn, Address, Decoder) == MCDisassembler::Fail) { DecodeComplete = false; return MCDisassembler::Fail; }
>
> Modified: llvm/trunk/test/TableGen/trydecode-emission2.td
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/TableGen/trydecode-emission2.td?rev=336334&r1=336333&r2=336334&view=diff
> ==============================================================================
> --- llvm/trunk/test/TableGen/trydecode-emission2.td (original)
> +++ llvm/trunk/test/TableGen/trydecode-emission2.td Thu Jul 5 03:39:15 2018
> @@ -31,14 +31,14 @@ def InstB : TestInstruction {
> }
>
> // CHECK: /* 0 */ MCD::OPC_ExtractField, 2, 1, // Inst{2} ...
> -// CHECK-NEXT: /* 3 */ MCD::OPC_FilterValue, 0, 29, 0, // Skip to: 36
> -// CHECK-NEXT: /* 7 */ MCD::OPC_ExtractField, 5, 3, // Inst{7-5} ...
> -// CHECK-NEXT: /* 10 */ MCD::OPC_FilterValue, 0, 22, 0, // Skip to: 36
> -// CHECK-NEXT: /* 14 */ MCD::OPC_CheckField, 0, 2, 3, 5, 0, // Skip to: 25
> -// CHECK-NEXT: /* 20 */ MCD::OPC_TryDecode, {{[0-9]+}}, 0, 0, 0, // Opcode: InstB, skip to: 25
> -// CHECK-NEXT: /* 25 */ MCD::OPC_CheckField, 3, 2, 0, 5, 0, // Skip to: 36
> -// CHECK-NEXT: /* 31 */ MCD::OPC_TryDecode, {{[0-9]+}}, 1, 0, 0, // Opcode: InstA, skip to: 36
> -// CHECK-NEXT: /* 36 */ MCD::OPC_Fail,
> +// CHECK-NEXT: /* 3 */ MCD::OPC_FilterValue, 0, 34, 0, 0, // Skip to: 42
> +// CHECK-NEXT: /* 8 */ MCD::OPC_ExtractField, 5, 3, // Inst{7-5} ...
> +// CHECK-NEXT: /* 11 */ MCD::OPC_FilterValue, 0, 26, 0, 0, // Skip to: 42
> +// CHECK-NEXT: /* 16 */ MCD::OPC_CheckField, 0, 2, 3, 6, 0, 0, // Skip to: 29
> +// CHECK-NEXT: /* 23 */ MCD::OPC_TryDecode, {{[0-9]+}}, 0, 0, 0, 0, // Opcode: InstB, skip to: 29
> +// CHECK-NEXT: /* 29 */ MCD::OPC_CheckField, 3, 2, 0, 6, 0, 0, // Skip to: 42
> +// CHECK-NEXT: /* 36 */ MCD::OPC_TryDecode, {{[0-9]+}}, 1, 0, 0, 0, // Opcode: InstA, skip to: 42
> +// CHECK-NEXT: /* 42 */ MCD::OPC_Fail,
>
> // CHECK: if (DecodeInstB(MI, insn, Address, Decoder) == MCDisassembler::Fail) { DecodeComplete = false; return MCDisassembler::Fail; }
> // CHECK: if (DecodeInstA(MI, insn, Address, Decoder) == MCDisassembler::Fail) { DecodeComplete = false; return MCDisassembler::Fail; }
>
> Modified: llvm/trunk/test/TableGen/trydecode-emission3.td
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/TableGen/trydecode-emission3.td?rev=336334&r1=336333&r2=336334&view=diff
> ==============================================================================
> --- llvm/trunk/test/TableGen/trydecode-emission3.td (original)
> +++ llvm/trunk/test/TableGen/trydecode-emission3.td Thu Jul 5 03:39:15 2018
> @@ -35,10 +35,10 @@ def InstB : TestInstruction {
> }
>
> // CHECK: /* 0 */ MCD::OPC_ExtractField, 4, 4, // Inst{7-4} ...
> -// CHECK-NEXT: /* 3 */ MCD::OPC_FilterValue, 0, 14, 0, // Skip to: 21
> -// CHECK-NEXT: /* 7 */ MCD::OPC_CheckField, 2, 2, 0, 5, 0, // Skip to: 18
> -// CHECK-NEXT: /* 13 */ MCD::OPC_TryDecode, {{[0-9]+}}, 0, 0, 0, // Opcode: InstB, skip to: 18
> -// CHECK-NEXT: /* 18 */ MCD::OPC_Decode, {{[0-9]+}}, 1, // Opcode: InstA
> -// CHECK-NEXT: /* 21 */ MCD::OPC_Fail,
> +// CHECK-NEXT: /* 3 */ MCD::OPC_FilterValue, 0, 16, 0, 0, // Skip to: 24
> +// CHECK-NEXT: /* 8 */ MCD::OPC_CheckField, 2, 2, 0, 6, 0, 0, // Skip to: 21
> +// CHECK-NEXT: /* 15 */ MCD::OPC_TryDecode, {{[0-9]+}}, 0, 0, 0, 0, // Opcode: InstB, skip to: 21
> +// CHECK-NEXT: /* 21 */ MCD::OPC_Decode, {{[0-9]+}}, 1, // Opcode: InstA
> +// CHECK-NEXT: /* 24 */ MCD::OPC_Fail,
>
> // CHECK: if (DecodeInstBOp(MI, tmp, Address, Decoder) == MCDisassembler::Fail) { DecodeComplete = false; return MCDisassembler::Fail; }
>
> Modified: llvm/trunk/utils/TableGen/FixedLenDecoderEmitter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/FixedLenDecoderEmitter.cpp?rev=336334&r1=336333&r2=336334&view=diff
> ==============================================================================
> --- llvm/trunk/utils/TableGen/FixedLenDecoderEmitter.cpp (original)
> +++ llvm/trunk/utils/TableGen/FixedLenDecoderEmitter.cpp Thu Jul 5 03:39:15 2018
> @@ -606,12 +606,13 @@ static void resolveTableFixups(DecoderTa
> // NumToSkip entry itself, so subtract two from the displacement here
> // to account for that.
> uint32_t FixupIdx = *I;
> - uint32_t Delta = DestIdx - FixupIdx - 2;
> - // Our NumToSkip entries are 16-bits. Make sure our table isn't too
> + uint32_t Delta = DestIdx - FixupIdx - 3;
> + // Our NumToSkip entries are 24-bits. Make sure our table isn't too
> // big.
> - assert(Delta < 65536U && "disassembler decoding table too large!");
> + assert(Delta < (1u << 24));
> Table[FixupIdx] = (uint8_t)Delta;
> Table[FixupIdx + 1] = (uint8_t)(Delta >> 8);
> + Table[FixupIdx + 2] = (uint8_t)(Delta >> 16);
> }
> }
>
> @@ -646,7 +647,7 @@ void Filter::emitTableEntry(DecoderTable
> } else {
> Table.push_back(MCD::OPC_FilterValue);
> // Encode and emit the value to filter against.
> - uint8_t Buffer[8];
> + uint8_t Buffer[16];
> unsigned Len = encodeULEB128(Filter.first, Buffer);
> Table.insert(Table.end(), Buffer, Buffer + Len);
> // Reserve space for the NumToSkip entry. We'll backpatch the value
> @@ -654,6 +655,7 @@ void Filter::emitTableEntry(DecoderTable
> PrevFilter = Table.size();
> Table.push_back(0);
> Table.push_back(0);
> + Table.push_back(0);
> }
>
> // We arrive at a category of instructions with the same segment value.
> @@ -666,10 +668,11 @@ void Filter::emitTableEntry(DecoderTable
> // of the filter itself to be able to skip forward when false. Subtract
> // two as to account for the width of the NumToSkip field itself.
> if (PrevFilter) {
> - uint32_t NumToSkip = Table.size() - PrevFilter - 2;
> - assert(NumToSkip < 65536U && "disassembler decoding table too large!");
> + uint32_t NumToSkip = Table.size() - PrevFilter - 3;
> + assert(NumToSkip < (1u << 24) && "disassembler decoding table too large!");
> Table[PrevFilter] = (uint8_t)NumToSkip;
> Table[PrevFilter + 1] = (uint8_t)(NumToSkip >> 8);
> + Table[PrevFilter + 2] = (uint8_t)(NumToSkip >> 16);
> }
> }
>
> @@ -745,13 +748,16 @@ void FixedLenDecoderEmitter::emitTable(f
> OS << (unsigned)*I++ << ", ";
> OS << (unsigned)*I++ << ", ";
>
> - // 16-bit numtoskip value.
> + // 24-bit numtoskip value.
> uint8_t Byte = *I++;
> uint32_t NumToSkip = Byte;
> OS << (unsigned)Byte << ", ";
> Byte = *I++;
> OS << (unsigned)Byte << ", ";
> NumToSkip |= Byte << 8;
> + Byte = *I++;
> + OS << utostr(Byte) << ", ";
> + NumToSkip |= Byte << 16;
> OS << "// Skip to: " << ((I - Table.begin()) + NumToSkip) << "\n";
> break;
> }
> @@ -765,13 +771,16 @@ void FixedLenDecoderEmitter::emitTable(f
> for (; *I >= 128; ++I)
> OS << (unsigned)*I << ", ";
> OS << (unsigned)*I++ << ", ";
> - // 16-bit numtoskip value.
> + // 24-bit numtoskip value.
> uint8_t Byte = *I++;
> uint32_t NumToSkip = Byte;
> OS << (unsigned)Byte << ", ";
> Byte = *I++;
> OS << (unsigned)Byte << ", ";
> NumToSkip |= Byte << 8;
> + Byte = *I++;
> + OS << utostr(Byte) << ", ";
> + NumToSkip |= Byte << 16;
> OS << "// Skip to: " << ((I - Table.begin()) + NumToSkip) << "\n";
> break;
> }
> @@ -782,13 +791,16 @@ void FixedLenDecoderEmitter::emitTable(f
> OS << (unsigned)*I << ", ";
> OS << (unsigned)*I++ << ", ";
>
> - // 16-bit numtoskip value.
> + // 24-bit numtoskip value.
> uint8_t Byte = *I++;
> uint32_t NumToSkip = Byte;
> OS << (unsigned)Byte << ", ";
> Byte = *I++;
> OS << (unsigned)Byte << ", ";
> NumToSkip |= Byte << 8;
> + Byte = *I++;
> + OS << utostr(Byte) << ", ";
> + NumToSkip |= Byte << 16;
> OS << "// Skip to: " << ((I - Table.begin()) + NumToSkip) << "\n";
> break;
> }
> @@ -797,7 +809,7 @@ void FixedLenDecoderEmitter::emitTable(f
> bool IsTry = *I == MCD::OPC_TryDecode;
> ++I;
> // Extract the ULEB128 encoded Opcode to a buffer.
> - uint8_t Buffer[8], *p = Buffer;
> + uint8_t Buffer[16], *p = Buffer;
> while ((*p++ = *I++) >= 128)
> assert((p - Buffer) <= (ptrdiff_t)sizeof(Buffer)
> && "ULEB128 value too large!");
> @@ -822,13 +834,16 @@ void FixedLenDecoderEmitter::emitTable(f
>
> // Fallthrough for OPC_TryDecode.
>
> - // 16-bit numtoskip value.
> + // 24-bit numtoskip value.
> uint8_t Byte = *I++;
> uint32_t NumToSkip = Byte;
> OS << (unsigned)Byte << ", ";
> Byte = *I++;
> OS << (unsigned)Byte << ", ";
> NumToSkip |= Byte << 8;
> + Byte = *I++;
> + OS << utostr(Byte) << ", ";
> + NumToSkip |= Byte << 16;
>
> OS << "// Opcode: "
> << NumberedInstructions[Opc]->TheDef->getName()
> @@ -1226,6 +1241,7 @@ void FilterChooser::emitPredicateTableEn
> TableInfo.FixupStack.back().push_back(TableInfo.Table.size());
> TableInfo.Table.push_back(0);
> TableInfo.Table.push_back(0);
> + TableInfo.Table.push_back(0);
> }
>
> void FilterChooser::emitSoftFailTableEntry(DecoderTableInfo &TableInfo,
> @@ -1311,18 +1327,19 @@ void FilterChooser::emitSingletonTableEn
> TableInfo.Table.push_back(MCD::OPC_CheckField);
> TableInfo.Table.push_back(StartBits[I-1]);
> TableInfo.Table.push_back(NumBits);
> - uint8_t Buffer[8], *p;
> + uint8_t Buffer[16], *p;
> encodeULEB128(FieldVals[I-1], Buffer);
> for (p = Buffer; *p >= 128 ; ++p)
> TableInfo.Table.push_back(*p);
> TableInfo.Table.push_back(*p);
> // Push location for NumToSkip backpatching.
> TableInfo.FixupStack.back().push_back(TableInfo.Table.size());
> - // The fixup is always 16-bits, so go ahead and allocate the space
> + // The fixup is always 24-bits, so go ahead and allocate the space
> // in the table so all our relative position calculations work OK even
> // before we fully resolve the real value here.
> TableInfo.Table.push_back(0);
> TableInfo.Table.push_back(0);
> + TableInfo.Table.push_back(0);
> }
>
> // Check for soft failure of the match.
> @@ -1342,7 +1359,7 @@ void FilterChooser::emitSingletonTableEn
> // can decode it.
> TableInfo.Table.push_back(HasCompleteDecoder ? MCD::OPC_Decode :
> MCD::OPC_TryDecode);
> - uint8_t Buffer[8], *p;
> + uint8_t Buffer[16], *p;
> encodeULEB128(Opc, Buffer);
> for (p = Buffer; *p >= 128 ; ++p)
> TableInfo.Table.push_back(*p);
> @@ -1362,6 +1379,7 @@ void FilterChooser::emitSingletonTableEn
> // Allocate the space for the fixup.
> TableInfo.Table.push_back(0);
> TableInfo.Table.push_back(0);
> + TableInfo.Table.push_back(0);
> }
> }
>
> @@ -2097,9 +2115,10 @@ static void emitDecodeInstruction(format
> << " unsigned Len;\n"
> << " InsnType Val = decodeULEB128(++Ptr, &Len);\n"
> << " Ptr += Len;\n"
> - << " // NumToSkip is a plain 16-bit integer.\n"
> + << " // NumToSkip is a plain 24-bit integer.\n"
> << " unsigned NumToSkip = *Ptr++;\n"
> << " NumToSkip |= (*Ptr++) << 8;\n"
> + << " NumToSkip |= (*Ptr++) << 16;\n"
> << "\n"
> << " // Perform the filter operation.\n"
> << " if (Val != CurFieldValue)\n"
> @@ -2120,9 +2139,10 @@ static void emitDecodeInstruction(format
> << " // Decode the field value.\n"
> << " uint32_t ExpectedValue = decodeULEB128(++Ptr, &Len);\n"
> << " Ptr += Len;\n"
> - << " // NumToSkip is a plain 16-bit integer.\n"
> + << " // NumToSkip is a plain 24-bit integer.\n"
> << " unsigned NumToSkip = *Ptr++;\n"
> << " NumToSkip |= (*Ptr++) << 8;\n"
> + << " NumToSkip |= (*Ptr++) << 16;\n"
> << "\n"
> << " // If the actual and expected values don't match, skip.\n"
> << " if (ExpectedValue != FieldValue)\n"
> @@ -2143,9 +2163,10 @@ static void emitDecodeInstruction(format
> << " // Decode the Predicate Index value.\n"
> << " unsigned PIdx = decodeULEB128(++Ptr, &Len);\n"
> << " Ptr += Len;\n"
> - << " // NumToSkip is a plain 16-bit integer.\n"
> + << " // NumToSkip is a plain 24-bit integer.\n"
> << " unsigned NumToSkip = *Ptr++;\n"
> << " NumToSkip |= (*Ptr++) << 8;\n"
> + << " NumToSkip |= (*Ptr++) << 16;\n"
> << " // Check the predicate.\n"
> << " bool Pred;\n"
> << " if (!(Pred = checkDecoderPredicate(PIdx, Bits)))\n"
> @@ -2185,9 +2206,10 @@ static void emitDecodeInstruction(format
> << " Ptr += Len;\n"
> << " unsigned DecodeIdx = decodeULEB128(Ptr, &Len);\n"
> << " Ptr += Len;\n"
> - << " // NumToSkip is a plain 16-bit integer.\n"
> + << " // NumToSkip is a plain 24-bit integer.\n"
> << " unsigned NumToSkip = *Ptr++;\n"
> << " NumToSkip |= (*Ptr++) << 8;\n"
> + << " NumToSkip |= (*Ptr++) << 16;\n"
> << "\n"
> << " // Perform the decode operation.\n"
> << " MCInst TmpMI;\n"
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list