Skip to content

Commit

Permalink
Convert pattern stack from deque to vector, and share it
Browse files Browse the repository at this point in the history 
Also move various vector::reserve calls to State ctor
Negligible perf benefit, but more tidy.
  • Loading branch information
@chrisforbes @dneto0
chrisforbes authored and dneto0 committed Jul 4, 2017
1 parent e842c17 commit 78338d5
Show file tree
Hide file tree
Showing 8 changed files with 148 additions and 134 deletions.
4 changes: 2 additions & 2 deletions source/assembly_grammar.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,9 @@ spv_result_t AssemblyGrammar::lookupExtInst(spv_ext_inst_type_t type,
return spvExtInstTableValueLookup(extInstTable_, type, firstWord, extInst);
}

void AssemblyGrammar::prependOperandTypesForMask(
void AssemblyGrammar::pushOperandTypesForMask(
const spv_operand_type_t type, const uint32_t mask,
spv_operand_pattern_t* pattern) const {
spvPrependOperandTypesForMask(operandTable_, type, mask, pattern);
spvPushOperandTypesForMask(operandTable_, type, mask, pattern);
}
} // namespace libspirv
16 changes: 9 additions & 7 deletions source/assembly_grammar.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,17 +95,19 @@ class AssemblyGrammar {
spv_result_t lookupExtInst(spv_ext_inst_type_t type, uint32_t firstWord,
spv_ext_inst_desc* extInst) const;

// Inserts the operands expected after the given typed mask onto the front
// Inserts the operands expected after the given typed mask onto the end
// of the given pattern.
//
// Each set bit in the mask represents zero or more operand types that should
// be prepended onto the pattern. Operands for a less significant bit always
// appear before operands for a more significant bit.
// Each set bit in the mask represents zero or more operand types that
// should be appended onto the pattern. Operands for a less significant
// bit must always match before operands for a more significant bit, so
// the operands for a less significant bit must appear closer to the end
// of the pattern stack.
//
// If a set bit is unknown, then we assume it has no operands.
void prependOperandTypesForMask(const spv_operand_type_t type,
const uint32_t mask,
spv_operand_pattern_t* pattern) const;
void pushOperandTypesForMask(const spv_operand_type_t type,
const uint32_t mask,
spv_operand_pattern_t* pattern) const;

private:
const spv_target_env target_env_;
Expand Down
46 changes: 22 additions & 24 deletions source/binary.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,14 @@ class Parser {
diagnostic(diagnostic_arg),
word_index(0),
endian(),
requires_endian_conversion(false) {}
requires_endian_conversion(false) {

// Temporary storage for parser state within a single instruction.
// Most instructions require fewer than 25 words or operands.
operands.reserve(25);
endian_converted_words.reserve(25);
expected_operands.reserve(25);
}
State() : State(0, 0, nullptr) {}
const uint32_t* words; // Words in the binary SPIR-V module.
size_t num_words; // Number of words in the module.
Expand All @@ -202,6 +209,7 @@ class Parser {
// Used by parseOperand
std::vector<spv_parsed_operand_t> operands;
std::vector<uint32_t> endian_converted_words;
spv_operand_pattern_t expected_operands;
} _;
};

Expand Down Expand Up @@ -266,25 +274,15 @@ spv_result_t Parser::parseInstruction() {

const uint32_t first_word = peek();

// TODO(dneto): If it's too expensive to construct the following "words"
// and "operands" vectors for each instruction, each instruction, then make
// them class data members instead, and clear them here.

// If the module's endianness is different from the host native endianness,
// then converted_words contains the the endian-translated words in the
// instruction.
_.endian_converted_words.clear();
_.endian_converted_words.push_back(first_word);
if (_.requires_endian_conversion) {
// Most instructions have fewer than 25 words.
_.endian_converted_words.reserve(25);
}

// After a successful parse of the instruction, the inst.operands member
// will point to this vector's storage.
// Most instructions have fewer than 25 logical operands.
_.operands.clear();
_.operands.reserve(25);

assert(_.word_index < _.num_words);
// Decompose and check the first word.
Expand All @@ -310,13 +308,13 @@ spv_result_t Parser::parseInstruction() {
// has its own logical operands (such as the LocalSize operand for
// ExecutionMode), or for extended instructions that may have their
// own operands depending on the selected extended instruction.
spv_operand_pattern_t expected_operands(
opcode_desc->operandTypes,
opcode_desc->operandTypes + opcode_desc->numTypes);
_.expected_operands.clear();
for (auto i = 0; i < opcode_desc->numTypes; i++)
_.expected_operands.push_back(opcode_desc->operandTypes[opcode_desc->numTypes - i - 1]);

while (_.word_index < inst_offset + inst_word_count) {
const uint16_t inst_word_index = uint16_t(_.word_index - inst_offset);
if (expected_operands.empty()) {
if (_.expected_operands.empty()) {
return diagnostic() << "Invalid instruction Op" << opcode_desc->name
<< " starting at word " << inst_offset
<< ": expected no more operands after "
Expand All @@ -325,17 +323,17 @@ spv_result_t Parser::parseInstruction() {
<< inst_word_count << ".";
}

spv_operand_type_t type = spvTakeFirstMatchableOperand(&expected_operands);
spv_operand_type_t type = spvTakeFirstMatchableOperand(&_.expected_operands);

if (auto error =
parseOperand(inst_offset, &inst, type, &_.endian_converted_words,
&_.operands, &expected_operands)) {
&_.operands, &_.expected_operands)) {
return error;
}
}

if (!expected_operands.empty() &&
!spvOperandIsOptional(expected_operands.front())) {
if (!_.expected_operands.empty() &&
!spvOperandIsOptional(_.expected_operands.back())) {
return diagnostic() << "End of input reached while decoding Op"
<< opcode_desc->name << " starting at word "
<< inst_offset << ": expected more operands after "
Expand Down Expand Up @@ -473,7 +471,7 @@ spv_result_t Parser::parseOperand(size_t inst_offset,
spv_ext_inst_desc ext_inst;
if (grammar_.lookupExtInst(inst->ext_inst_type, word, &ext_inst))
return diagnostic() << "Invalid extended instruction number: " << word;
spvPrependOperandTypes(ext_inst->operandTypes, expected_operands);
spvPushOperandTypes(ext_inst->operandTypes, expected_operands);
} break;

case SPV_OPERAND_TYPE_SPEC_CONSTANT_OP_NUMBER: {
Expand All @@ -493,7 +491,7 @@ spv_result_t Parser::parseOperand(size_t inst_offset,
assert(opcode_entry->hasType);
assert(opcode_entry->hasResult);
assert(opcode_entry->numTypes >= 2);
spvPrependOperandTypes(opcode_entry->operandTypes + 2, expected_operands);
spvPushOperandTypes(opcode_entry->operandTypes + 2, expected_operands);
} break;

case SPV_OPERAND_TYPE_LITERAL_INTEGER:
Expand Down Expand Up @@ -627,7 +625,7 @@ spv_result_t Parser::parseOperand(size_t inst_offset,
<< " operand: " << word;
}
// Prepare to accept operands to this operand, if needed.
spvPrependOperandTypes(entry->operandTypes, expected_operands);
spvPushOperandTypes(entry->operandTypes, expected_operands);
} break;

case SPV_OPERAND_TYPE_FP_FAST_MATH_MODE:
Expand Down Expand Up @@ -662,15 +660,15 @@ spv_result_t Parser::parseOperand(size_t inst_offset,
<< mask;
}
remaining_word ^= mask;
spvPrependOperandTypes(entry->operandTypes, expected_operands);
spvPushOperandTypes(entry->operandTypes, expected_operands);
}
}
if (word == 0) {
// An all-zeroes mask *might* also be valid.
spv_operand_desc entry;
if (SPV_SUCCESS == grammar_.lookupOperand(type, 0, &entry)) {
// Prepare for its operands, if any.
spvPrependOperandTypes(entry->operandTypes, expected_operands);
spvPushOperandTypes(entry->operandTypes, expected_operands);
}
}
} break;
Expand Down
13 changes: 7 additions & 6 deletions source/comp/markv_codec.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1178,7 +1178,7 @@ spv_result_t MarkvDecoder::DecodeOperand(
}

// Prepare to accept operands to this operand, if needed.
spvPrependOperandTypes(entry->operandTypes, expected_operands);
spvPushOperandTypes(entry->operandTypes, expected_operands);
break;
}

Expand Down Expand Up @@ -1221,15 +1221,15 @@ spv_result_t MarkvDecoder::DecodeOperand(
<< mask;
}
remaining_word ^= mask;
spvPrependOperandTypes(entry->operandTypes, expected_operands);
spvPushOperandTypes(entry->operandTypes, expected_operands);
}
}
if (word == 0) {
// An all-zeroes mask *might* also be valid.
spv_operand_desc entry;
if (SPV_SUCCESS == grammar_.lookupOperand(type, 0, &entry)) {
// Prepare for its operands, if any.
spvPrependOperandTypes(entry->operandTypes, expected_operands);
spvPushOperandTypes(entry->operandTypes, expected_operands);
}
}
break;
Expand Down Expand Up @@ -1287,9 +1287,10 @@ spv_result_t MarkvDecoder::DecodeInstruction(spv_parsed_instruction_t* inst) {
return vstate_.diag(SPV_ERROR_INVALID_BINARY) << "Invalid opcode";
}

spv_operand_pattern_t expected_operands(
opcode_desc->operandTypes,
opcode_desc->operandTypes + opcode_desc->numTypes);
spv_operand_pattern_t expected_operands;
expected_operands.reserve(opcode_desc->numTypes);
for (auto i = 0; i < opcode_desc->numTypes; i++)
expected_operands.push_back(opcode_desc->operandTypes[opcode_desc->numTypes - i - 1]);

if (!OpcodeHasFixedNumberOfOperands(opcode)) {
if (!reader_.ReadVariableWidthU16(&inst->num_operands,
Expand Down
63 changes: 33 additions & 30 deletions source/operand.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include <assert.h>
#include <string.h>
#include <algorithm>

#include "macro.h"

Expand Down Expand Up @@ -218,26 +219,28 @@ const char* spvOperandTypeStr(spv_operand_type_t type) {
return "unknown";
}

void spvPrependOperandTypes(const spv_operand_type_t* types,
spv_operand_pattern_t* pattern) {
void spvPushOperandTypes(const spv_operand_type_t* types,
spv_operand_pattern_t* pattern) {
const spv_operand_type_t* endTypes;
for (endTypes = types; *endTypes != SPV_OPERAND_TYPE_NONE; ++endTypes)
;
pattern->insert(pattern->begin(), types, endTypes);
while (endTypes-- != types) {
pattern->push_back(*endTypes);
}
}

void spvPrependOperandTypesForMask(const spv_operand_table operandTable,
const spv_operand_type_t type,
const uint32_t mask,
spv_operand_pattern_t* pattern) {
// Scan from highest bits to lowest bits because we will prepend in LIFO
// fashion, and we need the operands for lower order bits to appear first.
void spvPushOperandTypesForMask(const spv_operand_table operandTable,
const spv_operand_type_t type,
const uint32_t mask,
spv_operand_pattern_t* pattern) {
// Scan from highest bits to lowest bits because we will append in LIFO
// fashion, and we need the operands for lower order bits to be consumed first
for (uint32_t candidate_bit = (1u << 31u); candidate_bit; candidate_bit >>= 1) {
if (candidate_bit & mask) {
spv_operand_desc entry = nullptr;
if (SPV_SUCCESS == spvOperandTableValueLookup(operandTable, type,
candidate_bit, &entry)) {
spvPrependOperandTypes(entry->operandTypes, pattern);
spvPushOperandTypes(entry->operandTypes, pattern);
}
}
}
Expand All @@ -262,24 +265,25 @@ bool spvExpandOperandSequenceOnce(spv_operand_type_t type,
spv_operand_pattern_t* pattern) {
switch (type) {
case SPV_OPERAND_TYPE_VARIABLE_ID:
pattern->insert(pattern->begin(), {SPV_OPERAND_TYPE_OPTIONAL_ID, type});
pattern->push_back(type);
pattern->push_back(SPV_OPERAND_TYPE_OPTIONAL_ID);
return true;
case SPV_OPERAND_TYPE_VARIABLE_LITERAL_INTEGER:
pattern->insert(pattern->begin(),
{SPV_OPERAND_TYPE_OPTIONAL_LITERAL_INTEGER, type});
pattern->push_back(type);
pattern->push_back(SPV_OPERAND_TYPE_OPTIONAL_LITERAL_INTEGER);
return true;
case SPV_OPERAND_TYPE_VARIABLE_LITERAL_INTEGER_ID:
// Represents Zero or more (Literal number, Id) pairs,
// where the literal number must be a scalar integer.
pattern->insert(pattern->begin(),
{SPV_OPERAND_TYPE_OPTIONAL_TYPED_LITERAL_INTEGER,
SPV_OPERAND_TYPE_ID, type});
pattern->push_back(type);
pattern->push_back(SPV_OPERAND_TYPE_ID);
pattern->push_back(SPV_OPERAND_TYPE_OPTIONAL_TYPED_LITERAL_INTEGER);
return true;
case SPV_OPERAND_TYPE_VARIABLE_ID_LITERAL_INTEGER:
// Represents Zero or more (Id, Literal number) pairs.
pattern->insert(pattern->begin(),
{SPV_OPERAND_TYPE_OPTIONAL_ID,
SPV_OPERAND_TYPE_LITERAL_INTEGER, type});
pattern->push_back(type);
pattern->push_back(SPV_OPERAND_TYPE_LITERAL_INTEGER);
pattern->push_back(SPV_OPERAND_TYPE_OPTIONAL_ID);
return true;
default:
break;
Expand All @@ -292,25 +296,24 @@ spv_operand_type_t spvTakeFirstMatchableOperand(
assert(!pattern->empty());
spv_operand_type_t result;
do {
result = pattern->front();
pattern->pop_front();
result = pattern->back();
pattern->pop_back();
} while (spvExpandOperandSequenceOnce(result, pattern));
return result;
}

spv_operand_pattern_t spvAlternatePatternFollowingImmediate(
const spv_operand_pattern_t& pattern) {
spv_operand_pattern_t alternatePattern;
for (const auto& operand : pattern) {
if (operand == SPV_OPERAND_TYPE_RESULT_ID) {
alternatePattern.push_back(operand);
alternatePattern.push_back(SPV_OPERAND_TYPE_OPTIONAL_CIV);
return alternatePattern;
}
alternatePattern.push_back(SPV_OPERAND_TYPE_OPTIONAL_CIV);

auto it = std::find(pattern.crbegin(), pattern.crend(), SPV_OPERAND_TYPE_RESULT_ID);
if (it != pattern.crend()) {
spv_operand_pattern_t alternatePattern(it - pattern.crbegin() + 2, SPV_OPERAND_TYPE_OPTIONAL_CIV);
alternatePattern[1] = SPV_OPERAND_TYPE_RESULT_ID;
return alternatePattern;
}

// No result-id found, so just expect CIVs.
return {SPV_OPERAND_TYPE_OPTIONAL_CIV};
return{ SPV_OPERAND_TYPE_OPTIONAL_CIV };
}

bool spvIsIdType(spv_operand_type_t type) {
Expand Down
31 changes: 18 additions & 13 deletions source/operand.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,13 @@
// next on the input.
//
// As we parse an instruction in text or binary form from left to right,
// we pull and push from the front of the pattern.
using spv_operand_pattern_t = std::deque<spv_operand_type_t>;
// we pop and push at the end of the pattern vector. Symbols later in the
// pattern vector are matched against the input before symbols earlier in the
// pattern vector are matched.

// Using a vector in this way reduces memory traffic, which is good for
// performance.
using spv_operand_pattern_t = std::vector<spv_operand_type_t>;

// Finds the named operand in the table. The type parameter specifies the
// operand's group. A handle of the operand table entry for this operand will
Expand Down Expand Up @@ -62,24 +67,24 @@ bool spvOperandIsOptional(spv_operand_type_t type);
// operand.
bool spvOperandIsVariable(spv_operand_type_t type);

// Inserts a list of operand types into the front of the given pattern.
// Append a list of operand types to the end of the pattern vector.
// The types parameter specifies the source array of types, ending with
// SPV_OPERAND_TYPE_NONE.
void spvPrependOperandTypes(const spv_operand_type_t* types,
spv_operand_pattern_t* pattern);
void spvPushOperandTypes(const spv_operand_type_t* types,
spv_operand_pattern_t* pattern);

// Inserts the operands expected after the given typed mask onto the
// front of the given pattern.
// Appends the operands expected after the given typed mask onto the
// end of the given pattern.
//
// Each set bit in the mask represents zero or more operand types that should
// be prepended onto the pattern. Operands for a less significant bit always
// appear before operands for a more significant bit.
// be appended onto the pattern. Operands for a less significant bit always
// appear after operands for a more significant bit.
//
// If a set bit is unknown, then we assume it has no operands.
void spvPrependOperandTypesForMask(const spv_operand_table operand_table,
const spv_operand_type_t mask_type,
const uint32_t mask,
spv_operand_pattern_t* pattern);
void spvPushOperandTypesForMask(const spv_operand_table operand_table,
const spv_operand_type_t mask_type,
const uint32_t mask,
spv_operand_pattern_t* pattern);

// Expands an operand type representing zero or more logical operands,
// exactly once.
Expand Down
Loading

0 comments on commit 78338d5

Please sign in to comment.