Skip to content

Commit

Permalink
Do not add spirv::BitcastOp for cast from signed to unsigned type.
Browse files Browse the repository at this point in the history
Since MLIR integer types don't make a distinction between signed vs
unsigned integers, during deserialization of SPIR-V binaries, the
OpBitcast might result in a cast from/to the same type. Do not add a
spv.Bitcast operation to the spv.module in these cases.

PiperOrigin-RevId: 273381887
  • Loading branch information
Mahesh Ravishankar authored and tensorflower-gardener committed Oct 7, 2019
1 parent 5a1108c commit 37e0e8c
Showing 1 changed file with 75 additions and 0 deletions.
75 changes: 75 additions & 0 deletions mlir/lib/Dialect/SPIRV/Serialization/Deserializer.cpp
Expand Up @@ -313,6 +313,9 @@ class Deserializer {
/// insertion point.
LogicalResult processUndef(ArrayRef<uint32_t> operands);

/// Processes an OpBitcast instruction.
LogicalResult processBitcast(ArrayRef<uint32_t> words);

/// Method to dispatch to the specialized deserialization function for an
/// operation in SPIR-V dialect that is a mirror of an instruction in the
/// SPIR-V spec. This is auto-generated from ODS. Dispatch is handled for
Expand Down Expand Up @@ -1849,6 +1852,8 @@ LogicalResult Deserializer::processInstruction(spirv::Opcode opcode,
// First dispatch all the instructions whose opcode does not correspond to
// those that have a direct mirror in the SPIR-V dialect
switch (opcode) {
case spirv::Opcode::OpBitcast:
return processBitcast(operands);
case spirv::Opcode::OpCapability:
return processCapability(operands);
case spirv::Opcode::OpExtension:
Expand Down Expand Up @@ -1946,6 +1951,76 @@ LogicalResult Deserializer::processUndef(ArrayRef<uint32_t> operands) {
return success();
}

// TODO(b/130356985): This method is copied from the auto-generated
// deserialization function for OpBitcast instruction. This is to avoid
// generating a Bitcast operations for cast from signed integer to unsigned
// integer and viceversa. MLIR doesn't have native support for this so they both
// end up mapping to the same type right now which is illegal according to
// OpBitcast semantics (and enforced by the SPIR-V dialect).
LogicalResult Deserializer::processBitcast(ArrayRef<uint32_t> words) {
SmallVector<Type, 1> resultTypes;
size_t wordIndex = 0;
(void)wordIndex;
uint32_t valueID = 0;
(void)valueID;
{
if (wordIndex >= words.size()) {
return emitError(
unknownLoc,
"expected result type <id> while deserializing spirv::BitcastOp");
}
auto ty = getType(words[wordIndex]);
if (!ty) {
return emitError(unknownLoc, "unknown type result <id> : ")
<< words[wordIndex];
}
resultTypes.push_back(ty);
wordIndex++;
if (wordIndex >= words.size()) {
return emitError(
unknownLoc,
"expected result <id> while deserializing spirv::BitcastOp");
}
}
valueID = words[wordIndex++];
SmallVector<Value *, 4> operands;
SmallVector<NamedAttribute, 4> attributes;
if (wordIndex < words.size()) {
auto arg = getValue(words[wordIndex]);
if (!arg) {
return emitError(unknownLoc, "unknown result <id> : ")
<< words[wordIndex];
}
operands.push_back(arg);
wordIndex++;
}
if (wordIndex != words.size()) {
return emitError(unknownLoc,
"found more operands than expected when deserializing "
"spirv::BitcastOp, only ")
<< wordIndex << " of " << words.size() << " processed";
}
if (resultTypes[0] == operands[0]->getType() &&
resultTypes[0].isa<IntegerType>()) {
// TODO(b/130356985): This check is added to ignore error in Op verification
// due to both signed and unsigned integers mapping to the same
// type. Without this check this method is same as what is auto-generated.
valueMap[valueID] = operands[0];
return success();
}

auto op = opBuilder.create<spirv::BitcastOp>(unknownLoc, resultTypes,
operands, attributes);
(void)op;
valueMap[valueID] = op.getResult();

if (decorations.count(valueID)) {
auto attrs = decorations[valueID].getAttrs();
attributes.append(attrs.begin(), attrs.end());
}
return success();
}

LogicalResult Deserializer::processExtInst(ArrayRef<uint32_t> operands) {
if (operands.size() < 4) {
return emitError(unknownLoc,
Expand Down

0 comments on commit 37e0e8c

Please sign in to comment.