From 08927308b7fdd0c6c7d5e3280fe1eb34076f6489 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Tue, 2 Jul 2019 11:19:22 -0700 Subject: [PATCH] [spirv] Various small improvements * Added comments * Improved op creation * Used LogicalResult where suitable PiperOrigin-RevId: 256203068 --- mlir/include/mlir/SPIRV/SPIRVBase.td | 4 +++- mlir/include/mlir/SPIRV/Serialization.h | 9 +++++---- mlir/lib/SPIRV/Serialization/ConvertFromBinary.cpp | 15 ++++++-------- mlir/lib/SPIRV/Serialization/ConvertToBinary.cpp | 6 +++--- mlir/lib/SPIRV/Serialization/Serializer.cpp | 23 ++++++++++++++++++---- 5 files changed, 36 insertions(+), 21 deletions(-) diff --git a/mlir/include/mlir/SPIRV/SPIRVBase.td b/mlir/include/mlir/SPIRV/SPIRVBase.td index 9d9cafc..a337ccc 100644 --- a/mlir/include/mlir/SPIRV/SPIRVBase.td +++ b/mlir/include/mlir/SPIRV/SPIRVBase.td @@ -310,7 +310,9 @@ def SPV_SamplerUseAttr: // Base class for all SPIR-V ops. class SPV_Op traits = []> : Op { - // Opcode for the binary format. Ops cannot be directly serialized will + // Opcode for the binary format. + // See https://www.khronos.org/registry/spir-v/specs/unified1/SPIRV.html for + // the opcode for each operation. Ops that cannot be directly serialized will // leave this field as unset. int opcode = ?; diff --git a/mlir/include/mlir/SPIRV/Serialization.h b/mlir/include/mlir/SPIRV/Serialization.h index 80760c7..722dbd5 100644 --- a/mlir/include/mlir/SPIRV/Serialization.h +++ b/mlir/include/mlir/SPIRV/Serialization.h @@ -26,15 +26,16 @@ #include "mlir/Support/LLVM.h" namespace mlir { +class LogicalResult; class MLIRContext; namespace spirv { class ModuleOp; -/// Serializes the given SPIR-V `module` and writes to `binary`. Returns true on -/// success; otherwise, reports errors to the error handler registered with the -/// MLIR context for `module` and returns false. -bool serialize(ModuleOp module, SmallVectorImpl &binary); +/// Serializes the given SPIR-V `module` and writes to `binary`. On failure, +/// reports errors to the error handler registered with the MLIR context for +/// `module`. +LogicalResult serialize(ModuleOp module, SmallVectorImpl &binary); /// Deserializes the given SPIR-V `binary` module and creates a MLIR ModuleOp /// in the given `context`. Returns the ModuleOp on success; otherwise, reports diff --git a/mlir/lib/SPIRV/Serialization/ConvertFromBinary.cpp b/mlir/lib/SPIRV/Serialization/ConvertFromBinary.cpp index 688d1f9..d3eec3e 100644 --- a/mlir/lib/SPIRV/Serialization/ConvertFromBinary.cpp +++ b/mlir/lib/SPIRV/Serialization/ConvertFromBinary.cpp @@ -39,12 +39,9 @@ Block *createOneBlockFunction(Builder builder, Module module) { auto fn = Function::create(builder.getUnknownLoc(), "spirv_module", fnType); module.push_back(fn); - auto *block = new Block(); - fn.push_back(block); - - OperationState state(builder.getUnknownLoc(), ReturnOp::getOperationName()); - ReturnOp::build(&builder, &state); - block->push_back(Operation::create(state)); + fn.addEntryBlock(); + auto *block = &fn.front(); + OpBuilder(block).create(builder.getUnknownLoc()); return block; } @@ -64,15 +61,15 @@ OwningModuleRef deserializeModule(llvm::StringRef inputFilename, // Make sure the input stream can be treated as a stream of SPIR-V words auto start = file->getBufferStart(); - auto end = file->getBufferEnd(); - if ((start - end) % sizeof(uint32_t) != 0) { + auto size = file->getBufferSize(); + if (size % sizeof(uint32_t) != 0) { emitError(UnknownLoc::get(context)) << "SPIR-V binary module must contain integral number of 32-bit words"; return {}; } auto binary = llvm::makeArrayRef(reinterpret_cast(start), - (end - start) / sizeof(uint32_t)); + size / sizeof(uint32_t)); auto spirvModule = spirv::deserialize(binary, context); if (!spirvModule) diff --git a/mlir/lib/SPIRV/Serialization/ConvertToBinary.cpp b/mlir/lib/SPIRV/Serialization/ConvertToBinary.cpp index 7cf9bdb..aed3ab0 100644 --- a/mlir/lib/SPIRV/Serialization/ConvertToBinary.cpp +++ b/mlir/lib/SPIRV/Serialization/ConvertToBinary.cpp @@ -37,7 +37,7 @@ LogicalResult serializeModule(Module module, StringRef outputFilename) { SmallVector binary; bool done = false; - bool success = false; + auto result = failure(); // TODO(antiagainst): we are checking there is only one SPIR-V ModuleOp in // this module and serialize it. This is due to the restriction of the current @@ -53,11 +53,11 @@ LogicalResult serializeModule(Module module, StringRef outputFilename) { } done = true; - success = spirv::serialize(spirvModule, binary); + result = spirv::serialize(spirvModule, binary); }); } - if (!success) + if (failed(result)) return failure(); auto file = openOutputFile(outputFilename); diff --git a/mlir/lib/SPIRV/Serialization/Serializer.cpp b/mlir/lib/SPIRV/Serialization/Serializer.cpp index 74d671f..1e6780f 100644 --- a/mlir/lib/SPIRV/Serialization/Serializer.cpp +++ b/mlir/lib/SPIRV/Serialization/Serializer.cpp @@ -134,6 +134,21 @@ void Serializer::processHeader(SmallVectorImpl &header) { constexpr uint8_t kMajorVersion = 1; constexpr uint8_t kMinorVersion = 0; + // See "2.3. Physical Layout of a SPIR-V Module and Instruction" in the SPIR-V + // spec for the definition of the binary module header. + // + // The first five words of a SPIR-V module must be: + // +-------------------------------------------------------------------------+ + // | Magic number | + // +-------------------------------------------------------------------------+ + // | Version number (bytes: 0 | major number | minor number | 0) | + // +-------------------------------------------------------------------------+ + // | Generator magic number | + // +-------------------------------------------------------------------------+ + // | Bound (all result s in the module guaranteed to be less than it) | + // +-------------------------------------------------------------------------+ + // | 0 (reserved for instruction schema) | + // +-------------------------------------------------------------------------+ header.push_back(spirv::kMagicNumber); header.push_back((kMajorVersion << 16) | (kMinorVersion << 8)); header.push_back(kGeneratorNumber); @@ -152,13 +167,13 @@ void Serializer::processMemoryModel() { {getPrefixedOpcode(kNumWords, spirv::kOpMemoryModelOpcode), am, mm}); } -bool spirv::serialize(spirv::ModuleOp module, - SmallVectorImpl &binary) { +LogicalResult spirv::serialize(spirv::ModuleOp module, + SmallVectorImpl &binary) { Serializer serializer(module); if (failed(serializer.serialize())) - return false; + return failure(); serializer.collect(binary); - return true; + return success(); } -- 2.7.4