[llvm] b8bf94d - [TableGen] Fix excessive compile time issue in FixedLenDecoderEmitter
Jay Foad via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 17 02:38:39 PDT 2021
Author: Jay Foad
Date: 2021-03-17T09:28:50Z
New Revision: b8bf94df2576b2ab1213a17e6e09fed38c684090
URL: https://github.com/llvm/llvm-project/commit/b8bf94df2576b2ab1213a17e6e09fed38c684090
DIFF: https://github.com/llvm/llvm-project/commit/b8bf94df2576b2ab1213a17e6e09fed38c684090.diff
LOG: [TableGen] Fix excessive compile time issue in FixedLenDecoderEmitter
This patch reduces the time taken for clang to compile the generated
disassembler for an out-of-tree target with InsnType bigger than 64 bits
from 4m30s to 48s.
D67686 did a similar thing for CodeEmitterGen.
The idea is to tweak the API of the APInt-like InsnType class so that
we don't need so many temporary InsnTypes. This takes advantage of the
rule stated in D52100 that currently "no string of bits extracted
from the encoding may exceeed 64-bits", so we can use uint64_t for some
temporaries.
D52100 goes on to say that "fields are still permitted to exceed 64-bits
so long as they aren't one contiguous string of bits". This patch breaks
that by always using a "uint64_t tmp" in the generated decodeToMCInst,
but it should be easy to fix in FilterChooser::emitBinaryParser by
choosing to use a different type of tmp based on the known total field
width.
Differential Revision: https://reviews.llvm.org/D98046
Added:
Modified:
llvm/test/TableGen/BitOffsetDecoder.td
llvm/test/TableGen/FixedLenDecoderEmitter/InitValue.td
llvm/utils/TableGen/FixedLenDecoderEmitter.cpp
Removed:
################################################################################
diff --git a/llvm/test/TableGen/BitOffsetDecoder.td b/llvm/test/TableGen/BitOffsetDecoder.td
index f94e8d4f0978..04d6e164d0ee 100644
--- a/llvm/test/TableGen/BitOffsetDecoder.td
+++ b/llvm/test/TableGen/BitOffsetDecoder.td
@@ -59,6 +59,6 @@ def baz : Instruction {
// CHECK: tmp = fieldFromInstruction(insn, 8, 7);
// CHECK: tmp = fieldFromInstruction(insn, 8, 8) << 3;
-// CHECK: tmp |= fieldFromInstruction(insn, 8, 4) << 7;
-// CHECK: tmp |= fieldFromInstruction(insn, 12, 4) << 3;
+// CHECK: insertBits(tmp, fieldFromInstruction(insn, 8, 4), 7, 4);
+// CHECK: insertBits(tmp, fieldFromInstruction(insn, 12, 4), 3, 4);
// CHECK: tmp = fieldFromInstruction(insn, 8, 8) << 4;
diff --git a/llvm/test/TableGen/FixedLenDecoderEmitter/InitValue.td b/llvm/test/TableGen/FixedLenDecoderEmitter/InitValue.td
index f0f7b00af825..03847439ffc2 100644
--- a/llvm/test/TableGen/FixedLenDecoderEmitter/InitValue.td
+++ b/llvm/test/TableGen/FixedLenDecoderEmitter/InitValue.td
@@ -41,6 +41,6 @@ def bax : Instruction {
// CHECK: tmp = fieldFromInstruction(insn, 9, 7) << 1;
// CHECK: tmp = 0x1;
-// CHECK: tmp |= fieldFromInstruction(insn, 9, 7) << 1;
+// CHECK: insertBits(tmp, fieldFromInstruction(insn, 9, 7), 1, 7);
// CHECK: tmp = 0x100000000;
-// CHECK: tmp |= fieldFromInstruction(insn, 8, 7) << 25;
+// CHECK: insertBits(tmp, fieldFromInstruction(insn, 8, 7), 25, 7);
diff --git a/llvm/utils/TableGen/FixedLenDecoderEmitter.cpp b/llvm/utils/TableGen/FixedLenDecoderEmitter.cpp
index 1c03296fe411..a647a754fe7a 100644
--- a/llvm/utils/TableGen/FixedLenDecoderEmitter.cpp
+++ b/llvm/utils/TableGen/FixedLenDecoderEmitter.cpp
@@ -973,7 +973,13 @@ emitDecoderFunction(formatted_raw_ostream &OS, DecoderSet &Decoders,
<< "Address, const void *Decoder, bool &DecodeComplete) {\n";
Indentation += 2;
OS.indent(Indentation) << "DecodeComplete = true;\n";
- OS.indent(Indentation) << "InsnType tmp;\n";
+ // TODO: When InsnType is large, using uint64_t limits all fields to 64 bits
+ // It would be better for emitBinaryParser to use a 64-bit tmp whenever
+ // possible but fall back to an InsnType-sized tmp for truly large fields.
+ OS.indent(Indentation) << "using TmpType = "
+ "std::conditional_t<std::is_integral<InsnType>::"
+ "value, InsnType, uint64_t>;\n";
+ OS.indent(Indentation) << "TmpType tmp;\n";
OS.indent(Indentation) << "switch (Idx) {\n";
OS.indent(Indentation) << "default: llvm_unreachable(\"Invalid index!\");\n";
unsigned Index = 0;
@@ -1107,18 +1113,24 @@ void FilterChooser::emitBinaryParser(raw_ostream &o, unsigned &Indentation,
bool &OpHasCompleteDecoder) const {
const std::string &Decoder = OpInfo.Decoder;
- if (OpInfo.numFields() != 1 || OpInfo.InitValue != 0) {
+ bool UseInsertBits = OpInfo.numFields() != 1 || OpInfo.InitValue != 0;
+
+ if (UseInsertBits) {
o.indent(Indentation) << "tmp = 0x";
o.write_hex(OpInfo.InitValue);
o << ";\n";
}
for (const EncodingField &EF : OpInfo) {
- o.indent(Indentation) << "tmp ";
- if (OpInfo.numFields() != 1 || OpInfo.InitValue != 0) o << '|';
- o << "= fieldFromInstruction"
- << "(insn, " << EF.Base << ", " << EF.Width << ')';
- if (OpInfo.numFields() != 1 || EF.Offset != 0)
+ o.indent(Indentation);
+ if (UseInsertBits)
+ o << "insertBits(tmp, ";
+ else
+ o << "tmp = ";
+ o << "fieldFromInstruction(insn, " << EF.Base << ", " << EF.Width << ')';
+ if (UseInsertBits)
+ o << ", " << EF.Offset << ", " << EF.Width << ')';
+ else if (EF.Offset != 0)
o << " << " << EF.Offset;
o << ";\n";
}
@@ -2142,27 +2154,22 @@ static void emitFieldFromInstruction(formatted_raw_ostream &OS) {
OS << "// Helper functions for extracting fields from encoded instructions.\n"
<< "// InsnType must either be integral or an APInt-like object that "
"must:\n"
- << "// * Have a static const max_size_in_bits equal to the number of bits "
- "in the\n"
- << "// encoding.\n"
<< "// * be default-constructible and copy-constructible\n"
<< "// * be constructible from a uint64_t\n"
<< "// * be constructible from an APInt (this can be private)\n"
- << "// * Support getBitsSet(loBit, hiBit)\n"
- << "// * be convertible to uint64_t\n"
- << "// * Support the ~, &, ==, !=, and |= operators with other objects of "
+ << "// * Support insertBits(bits, startBit, numBits)\n"
+ << "// * Support extractBitsAsZExtValue(numBits, startBit)\n"
+ << "// * be convertible to bool\n"
+ << "// * Support the ~, &, ==, and != operators with other objects of "
"the same type\n"
- << "// * Support shift (<<, >>) with signed and unsigned integers on the "
- "RHS\n"
<< "// * Support put (<<) to raw_ostream&\n"
<< "template <typename InsnType>\n"
<< "#if defined(_MSC_VER) && !defined(__clang__)\n"
<< "__declspec(noinline)\n"
<< "#endif\n"
- << "static InsnType fieldFromInstruction(InsnType insn, unsigned "
- "startBit,\n"
- << " unsigned numBits, "
- "std::true_type) {\n"
+ << "static std::enable_if_t<std::is_integral<InsnType>::value, InsnType>\n"
+ << "fieldFromInstruction(const InsnType &insn, unsigned startBit,\n"
+ << " unsigned numBits) {\n"
<< " assert(startBit + numBits <= 64 && \"Cannot support >64-bit "
"extractions!\");\n"
<< " assert(startBit + numBits <= (sizeof(InsnType) * 8) &&\n"
@@ -2176,22 +2183,32 @@ static void emitFieldFromInstruction(formatted_raw_ostream &OS) {
<< "}\n"
<< "\n"
<< "template <typename InsnType>\n"
- << "static InsnType fieldFromInstruction(InsnType insn, unsigned "
- "startBit,\n"
- << " unsigned numBits, "
- "std::false_type) {\n"
- << " assert(startBit + numBits <= InsnType::max_size_in_bits && "
- "\"Instruction field out of bounds!\");\n"
- << " InsnType fieldMask = InsnType::getBitsSet(0, numBits);\n"
- << " return (insn >> startBit) & fieldMask;\n"
+ << "static std::enable_if_t<!std::is_integral<InsnType>::value, "
+ "uint64_t>\n"
+ << "fieldFromInstruction(const InsnType &insn, unsigned startBit,\n"
+ << " unsigned numBits) {\n"
+ << " return insn.extractBitsAsZExtValue(numBits, startBit);\n"
+ << "}\n\n";
+}
+
+// emitInsertBits - Emit the templated helper function insertBits().
+static void emitInsertBits(formatted_raw_ostream &OS) {
+ OS << "// Helper function for inserting bits extracted from an encoded "
+ "instruction into\n"
+ << "// a field.\n"
+ << "template <typename InsnType>\n"
+ << "static std::enable_if_t<std::is_integral<InsnType>::value>\n"
+ << "insertBits(InsnType &field, InsnType bits, unsigned startBit, "
+ "unsigned numBits) {\n"
+ << " assert(startBit + numBits <= sizeof field * 8);\n"
+ << " field |= (InsnType)bits << startBit;\n"
<< "}\n"
<< "\n"
<< "template <typename InsnType>\n"
- << "static InsnType fieldFromInstruction(InsnType insn, unsigned "
- "startBit,\n"
- << " unsigned numBits) {\n"
- << " return fieldFromInstruction(insn, startBit, numBits, "
- "std::is_integral<InsnType>());\n"
+ << "static std::enable_if_t<!std::is_integral<InsnType>::value>\n"
+ << "insertBits(InsnType &field, uint64_t bits, unsigned startBit, "
+ "unsigned numBits) {\n"
+ << " field.insertBits(bits, startBit, numBits);\n"
<< "}\n\n";
}
@@ -2394,6 +2411,7 @@ void FixedLenDecoderEmitter::run(raw_ostream &o) {
OS << "namespace llvm {\n\n";
emitFieldFromInstruction(OS);
+ emitInsertBits(OS);
Target.reverseBitsForLittleEndianEncoding();
More information about the llvm-commits
mailing list