[Mlir-commits] [mlir] 3ed47bc - [mlir][spirv] Propagate LogicalResult in (de)serialization

Lei Zhang llvmlistbot at llvm.org
Fri Dec 10 16:21:19 PST 2021


Author: Lei Zhang
Date: 2021-12-10T19:20:49-05:00
New Revision: 3ed47bcc9618639012a43a0edf8a2f7ceeda36d1

URL: https://github.com/llvm/llvm-project/commit/3ed47bcc9618639012a43a0edf8a2f7ceeda36d1
DIFF: https://github.com/llvm/llvm-project/commit/3ed47bcc9618639012a43a0edf8a2f7ceeda36d1.diff

LOG: [mlir][spirv] Propagate LogicalResult in (de)serialization

`(void)` was added when LogicalResult was marked as non
discard. This commit cleans them up to properly propagate
failures.

Reviewed By: scotttodd

Differential Revision: https://reviews.llvm.org/D115541

Added: 
    

Modified: 
    mlir/include/mlir/Target/SPIRV/SPIRVBinaryUtils.h
    mlir/lib/Target/SPIRV/Deserialization/DeserializeOps.cpp
    mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp
    mlir/lib/Target/SPIRV/Deserialization/Deserializer.h
    mlir/lib/Target/SPIRV/SPIRVBinaryUtils.cpp
    mlir/lib/Target/SPIRV/Serialization/SerializeOps.cpp
    mlir/lib/Target/SPIRV/Serialization/Serializer.cpp
    mlir/lib/Target/SPIRV/Serialization/Serializer.h

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Target/SPIRV/SPIRVBinaryUtils.h b/mlir/include/mlir/Target/SPIRV/SPIRVBinaryUtils.h
index a155b1989e63b..818e39e5b839c 100644
--- a/mlir/include/mlir/Target/SPIRV/SPIRVBinaryUtils.h
+++ b/mlir/include/mlir/Target/SPIRV/SPIRVBinaryUtils.h
@@ -39,8 +39,8 @@ void appendModuleHeader(SmallVectorImpl<uint32_t> &header,
 uint32_t getPrefixedOpcode(uint32_t wordCount, spirv::Opcode opcode);
 
 /// Encodes an SPIR-V `literal` string into the given `binary` vector.
-LogicalResult encodeStringLiteralInto(SmallVectorImpl<uint32_t> &binary,
-                                      StringRef literal);
+void encodeStringLiteralInto(SmallVectorImpl<uint32_t> &binary,
+                             StringRef literal);
 
 /// Decodes a string literal in `words` starting at `wordIndex`. Update the
 /// latter to point to the position in words after the string literal.

diff  --git a/mlir/lib/Target/SPIRV/Deserialization/DeserializeOps.cpp b/mlir/lib/Target/SPIRV/Deserialization/DeserializeOps.cpp
index 1c77dfdddcaa5..ab5d69b49bef2 100644
--- a/mlir/lib/Target/SPIRV/Deserialization/DeserializeOps.cpp
+++ b/mlir/lib/Target/SPIRV/Deserialization/DeserializeOps.cpp
@@ -137,7 +137,8 @@ LogicalResult spirv::Deserializer::processInstruction(
   case spirv::Opcode::OpLine:
     return processDebugLine(operands);
   case spirv::Opcode::OpNoLine:
-    return clearDebugLine();
+    clearDebugLine();
+    return success();
   case spirv::Opcode::OpName:
     return processName(operands);
   case spirv::Opcode::OpString:
@@ -287,7 +288,7 @@ LogicalResult spirv::Deserializer::processOpWithoutGrammarAttr(
     valueMap[valueID] = op->getResult(0);
 
   if (op->hasTrait<OpTrait::IsTerminator>())
-    (void)clearDebugLine();
+    clearDebugLine();
 
   return success();
 }

diff  --git a/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp b/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp
index 646a2df394dec..8d4b7f854b95f 100644
--- a/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp
+++ b/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp
@@ -1437,7 +1437,7 @@ LogicalResult spirv::Deserializer::processBranch(ArrayRef<uint32_t> operands) {
   // the same OpLine information.
   opBuilder.create<spirv::BranchOp>(loc, target);
 
-  (void)clearDebugLine();
+  clearDebugLine();
   return success();
 }
 
@@ -1471,7 +1471,7 @@ spirv::Deserializer::processBranchConditional(ArrayRef<uint32_t> operands) {
       /*trueArguments=*/ArrayRef<Value>(), falseBlock,
       /*falseArguments=*/ArrayRef<Value>(), weights);
 
-  (void)clearDebugLine();
+  clearDebugLine();
   return success();
 }
 
@@ -1980,10 +1980,7 @@ spirv::Deserializer::processDebugLine(ArrayRef<uint32_t> operands) {
   return success();
 }
 
-LogicalResult spirv::Deserializer::clearDebugLine() {
-  debugLine = llvm::None;
-  return success();
-}
+void spirv::Deserializer::clearDebugLine() { debugLine = llvm::None; }
 
 LogicalResult
 spirv::Deserializer::processDebugString(ArrayRef<uint32_t> operands) {

diff  --git a/mlir/lib/Target/SPIRV/Deserialization/Deserializer.h b/mlir/lib/Target/SPIRV/Deserialization/Deserializer.h
index 8351f3a41fa17..e151aa40f0d4f 100644
--- a/mlir/lib/Target/SPIRV/Deserialization/Deserializer.h
+++ b/mlir/lib/Target/SPIRV/Deserialization/Deserializer.h
@@ -312,7 +312,7 @@ class Deserializer {
 
   /// Discontinues any source-level location information that might be active
   /// from a previous OpLine instruction.
-  LogicalResult clearDebugLine();
+  void clearDebugLine();
 
   /// Creates a FileLineColLoc with the OpLine location information.
   Location createFileLineColLoc(OpBuilder opBuilder);

diff  --git a/mlir/lib/Target/SPIRV/SPIRVBinaryUtils.cpp b/mlir/lib/Target/SPIRV/SPIRVBinaryUtils.cpp
index 769718bacffcc..a46bc949f6054 100644
--- a/mlir/lib/Target/SPIRV/SPIRVBinaryUtils.cpp
+++ b/mlir/lib/Target/SPIRV/SPIRVBinaryUtils.cpp
@@ -62,12 +62,11 @@ uint32_t spirv::getPrefixedOpcode(uint32_t wordCount, spirv::Opcode opcode) {
   return (wordCount << 16) | static_cast<uint32_t>(opcode);
 }
 
-LogicalResult spirv::encodeStringLiteralInto(SmallVectorImpl<uint32_t> &binary,
-                                             StringRef literal) {
+void spirv::encodeStringLiteralInto(SmallVectorImpl<uint32_t> &binary,
+                                    StringRef literal) {
   // We need to encode the literal and the null termination.
   auto encodingSize = literal.size() / 4 + 1;
   auto bufferStartSize = binary.size();
   binary.resize(bufferStartSize + encodingSize, 0);
   std::memcpy(binary.data() + bufferStartSize, literal.data(), literal.size());
-  return success();
 }

diff  --git a/mlir/lib/Target/SPIRV/Serialization/SerializeOps.cpp b/mlir/lib/Target/SPIRV/Serialization/SerializeOps.cpp
index ef558aa6c3929..ffb3b51f59cf8 100644
--- a/mlir/lib/Target/SPIRV/Serialization/SerializeOps.cpp
+++ b/mlir/lib/Target/SPIRV/Serialization/SerializeOps.cpp
@@ -70,7 +70,8 @@ LogicalResult Serializer::processSpecConstantOp(spirv::SpecConstantOp op) {
     // Emit the OpDecorate instruction for SpecId.
     if (auto specID = op->getAttrOfType<IntegerAttr>("spec_id")) {
       auto val = static_cast<uint32_t>(specID.getInt());
-      (void)emitDecoration(resultID, spirv::Decoration::SpecId, {val});
+      if (failed(emitDecoration(resultID, spirv::Decoration::SpecId, {val})))
+        return failure();
     }
 
     specConstIDMap[op.sym_name()] = resultID;
@@ -108,8 +109,8 @@ Serializer::processSpecConstantCompositeOp(spirv::SpecConstantCompositeOp op) {
     operands.push_back(constituentID);
   }
 
-  (void)encodeInstructionInto(typesGlobalValues,
-                              spirv::Opcode::OpSpecConstantComposite, operands);
+  encodeInstructionInto(typesGlobalValues,
+                        spirv::Opcode::OpSpecConstantComposite, operands);
   specConstIDMap[op.sym_name()] = resultID;
 
   return processName(resultID, op.sym_name());
@@ -151,8 +152,8 @@ Serializer::processSpecConstantOperationOp(spirv::SpecConstantOperationOp op) {
     operands.push_back(id);
   }
 
-  (void)encodeInstructionInto(typesGlobalValues,
-                              spirv::Opcode::OpSpecConstantOp, operands);
+  encodeInstructionInto(typesGlobalValues, spirv::Opcode::OpSpecConstantOp,
+                        operands);
   valueIDMap[op.getResult()] = resultID;
 
   return success();
@@ -164,11 +165,10 @@ LogicalResult Serializer::processUndefOp(spirv::UndefOp op) {
   if (!id) {
     id = getNextID();
     uint32_t typeID = 0;
-    if (failed(processType(op.getLoc(), undefType, typeID)) ||
-        failed(encodeInstructionInto(typesGlobalValues, spirv::Opcode::OpUndef,
-                                     {typeID, id}))) {
+    if (failed(processType(op.getLoc(), undefType, typeID)))
       return failure();
-    }
+    encodeInstructionInto(typesGlobalValues, spirv::Opcode::OpUndef,
+                          {typeID, id});
   }
   valueIDMap[op.getResult()] = id;
   return success();
@@ -180,7 +180,8 @@ LogicalResult Serializer::processFuncOp(spirv::FuncOp op) {
 
   uint32_t fnTypeID = 0;
   // Generate type of the function.
-  (void)processType(op.getLoc(), op.getType(), fnTypeID);
+  if (failed(processType(op.getLoc(), op.getType(), fnTypeID)))
+    return failure();
 
   // Add the function definition.
   SmallVector<uint32_t, 4> operands;
@@ -199,8 +200,7 @@ LogicalResult Serializer::processFuncOp(spirv::FuncOp op) {
   operands.push_back(funcID);
   operands.push_back(static_cast<uint32_t>(op.function_control()));
   operands.push_back(fnTypeID);
-  (void)encodeInstructionInto(functionHeader, spirv::Opcode::OpFunction,
-                              operands);
+  encodeInstructionInto(functionHeader, spirv::Opcode::OpFunction, operands);
 
   // Add function name.
   if (failed(processName(funcID, op.getName()))) {
@@ -215,9 +215,8 @@ LogicalResult Serializer::processFuncOp(spirv::FuncOp op) {
     }
     auto argValueID = getNextID();
     valueIDMap[arg] = argValueID;
-    (void)encodeInstructionInto(functionHeader,
-                                spirv::Opcode::OpFunctionParameter,
-                                {argTypeID, argValueID});
+    encodeInstructionInto(functionHeader, spirv::Opcode::OpFunctionParameter,
+                          {argTypeID, argValueID});
   }
 
   // Process the body.
@@ -229,9 +228,10 @@ LogicalResult Serializer::processFuncOp(spirv::FuncOp op) {
   // block in the function. These instructions will be put in functionHeader.
   // Thus, we put the label in functionHeader first, and omit it from the first
   // block.
-  (void)encodeInstructionInto(functionHeader, spirv::Opcode::OpLabel,
-                              {getOrCreateBlockID(&op.front())});
-  (void)processBlock(&op.front(), /*omitLabel=*/true);
+  encodeInstructionInto(functionHeader, spirv::Opcode::OpLabel,
+                        {getOrCreateBlockID(&op.front())});
+  if (failed(processBlock(&op.front(), /*omitLabel=*/true)))
+    return failure();
   if (failed(visitInPrettyBlockOrder(
           &op.front(), [&](Block *block) { return processBlock(block); },
           /*skipHeader=*/true))) {
@@ -239,7 +239,7 @@ LogicalResult Serializer::processFuncOp(spirv::FuncOp op) {
   }
 
   // There might be OpPhi instructions who have value references needing to fix.
-  for (auto deferredValue : deferredPhiValues) {
+  for (const auto &deferredValue : deferredPhiValues) {
     Value value = deferredValue.first;
     uint32_t id = getValueID(value);
     LLVM_DEBUG(llvm::dbgs() << "[phi] fix reference of value " << value
@@ -253,10 +253,7 @@ LogicalResult Serializer::processFuncOp(spirv::FuncOp op) {
   LLVM_DEBUG(llvm::dbgs() << "-- completed function '" << op.getName()
                           << "' --\n");
   // Insert OpFunctionEnd.
-  if (failed(encodeInstructionInto(functionBody, spirv::Opcode::OpFunctionEnd,
-                                   {}))) {
-    return failure();
-  }
+  encodeInstructionInto(functionBody, spirv::Opcode::OpFunctionEnd, {});
 
   functions.append(functionHeader.begin(), functionHeader.end());
   functions.append(functionBody.begin(), functionBody.end());
@@ -291,9 +288,9 @@ LogicalResult Serializer::processVariableOp(spirv::VariableOp op) {
     }
     operands.push_back(argID);
   }
-  (void)emitDebugLine(functionHeader, op.getLoc());
-  (void)encodeInstructionInto(functionHeader, spirv::Opcode::OpVariable,
-                              operands);
+  if (failed(emitDebugLine(functionHeader, op.getLoc())))
+    return failure();
+  encodeInstructionInto(functionHeader, spirv::Opcode::OpVariable, operands);
   for (auto attr : op->getAttrs()) {
     if (llvm::any_of(elidedAttrs, [&](StringRef elided) {
           return attr.getName() == elided;
@@ -344,12 +341,10 @@ Serializer::processGlobalVariableOp(spirv::GlobalVariableOp varOp) {
     elidedAttrs.push_back("initializer");
   }
 
-  (void)emitDebugLine(typesGlobalValues, varOp.getLoc());
-  if (failed(encodeInstructionInto(typesGlobalValues, spirv::Opcode::OpVariable,
-                                   operands))) {
-    elidedAttrs.push_back("initializer");
+  if (failed(emitDebugLine(typesGlobalValues, varOp.getLoc())))
     return failure();
-  }
+  encodeInstructionInto(typesGlobalValues, spirv::Opcode::OpVariable, operands);
+  elidedAttrs.push_back("initializer");
 
   // Encode decorations.
   for (auto attr : varOp->getAttrs()) {
@@ -381,11 +376,13 @@ LogicalResult Serializer::processSelectionOp(spirv::SelectionOp selectionOp) {
   // We need to emit an OpSelectionMerge instruction before the selection header
   // block's terminator.
   auto emitSelectionMerge = [&]() {
-    (void)emitDebugLine(functionBody, loc);
+    if (failed(emitDebugLine(functionBody, loc)))
+      return failure();
     lastProcessedWasMergeInst = true;
-    (void)encodeInstructionInto(
+    encodeInstructionInto(
         functionBody, spirv::Opcode::OpSelectionMerge,
         {mergeID, static_cast<uint32_t>(selectionOp.selection_control())});
+    return success();
   };
   // For structured selection, we cannot have blocks in the selection construct
   // branching to the selection header block. Entering the selection (and
@@ -408,7 +405,8 @@ LogicalResult Serializer::processSelectionOp(spirv::SelectionOp selectionOp) {
   // contains a spv.mlir.merge op, itself. But we need to have an OpLabel
   // instruction to start a new SPIR-V block for ops following this SelectionOp.
   // The block should use the <id> for the merge block.
-  return encodeInstructionInto(functionBody, spirv::Opcode::OpLabel, {mergeID});
+  encodeInstructionInto(functionBody, spirv::Opcode::OpLabel, {mergeID});
+  return success();
 }
 
 LogicalResult Serializer::processLoopOp(spirv::LoopOp loopOp) {
@@ -433,8 +431,7 @@ LogicalResult Serializer::processLoopOp(spirv::LoopOp loopOp) {
   // preceding and following ops. So we need to emit unconditional branches to
   // jump to this LoopOp's SPIR-V blocks and jumping back to the normal flow
   // afterwards.
-  (void)encodeInstructionInto(functionBody, spirv::Opcode::OpBranch,
-                              {headerID});
+  encodeInstructionInto(functionBody, spirv::Opcode::OpBranch, {headerID});
 
   // LoopOp's entry block is just there for satisfying MLIR's structural
   // requirements so we omit it and start serialization from the loop header
@@ -444,11 +441,13 @@ LogicalResult Serializer::processLoopOp(spirv::LoopOp loopOp) {
   // need to emit an OpLoopMerge instruction before the loop header block's
   // terminator.
   auto emitLoopMerge = [&]() {
-    (void)emitDebugLine(functionBody, loc);
+    if (failed(emitDebugLine(functionBody, loc)))
+      return failure();
     lastProcessedWasMergeInst = true;
-    (void)encodeInstructionInto(
+    encodeInstructionInto(
         functionBody, spirv::Opcode::OpLoopMerge,
         {mergeID, continueID, static_cast<uint32_t>(loopOp.loop_control())});
+    return success();
   };
   if (failed(processBlock(headerBlock, /*omitLabel=*/false, emitLoopMerge)))
     return failure();
@@ -469,7 +468,8 @@ LogicalResult Serializer::processLoopOp(spirv::LoopOp loopOp) {
   // a spv.mlir.merge op, itself. But we need to have an OpLabel instruction to
   // start a new SPIR-V block for ops following this LoopOp. The block should
   // use the <id> for the merge block.
-  return encodeInstructionInto(functionBody, spirv::Opcode::OpLabel, {mergeID});
+  encodeInstructionInto(functionBody, spirv::Opcode::OpLabel, {mergeID});
+  return success();
 }
 
 LogicalResult Serializer::processBranchConditionalOp(
@@ -484,15 +484,19 @@ LogicalResult Serializer::processBranchConditionalOp(
       arguments.push_back(val.cast<IntegerAttr>().getInt());
   }
 
-  (void)emitDebugLine(functionBody, condBranchOp.getLoc());
-  return encodeInstructionInto(functionBody, spirv::Opcode::OpBranchConditional,
-                               arguments);
+  if (failed(emitDebugLine(functionBody, condBranchOp.getLoc())))
+    return failure();
+  encodeInstructionInto(functionBody, spirv::Opcode::OpBranchConditional,
+                        arguments);
+  return success();
 }
 
 LogicalResult Serializer::processBranchOp(spirv::BranchOp branchOp) {
-  (void)emitDebugLine(functionBody, branchOp.getLoc());
-  return encodeInstructionInto(functionBody, spirv::Opcode::OpBranch,
-                               {getOrCreateBlockID(branchOp.getTarget())});
+  if (failed(emitDebugLine(functionBody, branchOp.getLoc())))
+    return failure();
+  encodeInstructionInto(functionBody, spirv::Opcode::OpBranch,
+                        {getOrCreateBlockID(branchOp.getTarget())});
+  return success();
 }
 
 LogicalResult Serializer::processAddressOfOp(spirv::AddressOfOp addressOfOp) {
@@ -535,7 +539,7 @@ Serializer::processOp<spirv::EntryPointOp>(spirv::EntryPointOp op) {
   }
   operands.push_back(funcID);
   // Add the name of the function.
-  (void)spirv::encodeStringLiteralInto(operands, op.fn());
+  spirv::encodeStringLiteralInto(operands, op.fn());
 
   // Add the interface values.
   if (auto interface = op.interface()) {
@@ -549,8 +553,8 @@ Serializer::processOp<spirv::EntryPointOp>(spirv::EntryPointOp op) {
       operands.push_back(id);
     }
   }
-  return encodeInstructionInto(entryPoints, spirv::Opcode::OpEntryPoint,
-                               operands);
+  encodeInstructionInto(entryPoints, spirv::Opcode::OpEntryPoint, operands);
+  return success();
 }
 
 template <>
@@ -569,8 +573,9 @@ Serializer::processOp<spirv::ControlBarrierOp>(spirv::ControlBarrierOp op) {
     operands.push_back(operand);
   }
 
-  return encodeInstructionInto(functionBody, spirv::Opcode::OpControlBarrier,
-                               operands);
+  encodeInstructionInto(functionBody, spirv::Opcode::OpControlBarrier,
+                        operands);
+  return success();
 }
 
 template <>
@@ -597,8 +602,9 @@ Serializer::processOp<spirv::ExecutionModeOp>(spirv::ExecutionModeOp op) {
           intVal.cast<IntegerAttr>().getValue().getZExtValue()));
     }
   }
-  return encodeInstructionInto(executionModes, spirv::Opcode::OpExecutionMode,
-                               operands);
+  encodeInstructionInto(executionModes, spirv::Opcode::OpExecutionMode,
+                        operands);
+  return success();
 }
 
 template <>
@@ -616,8 +622,8 @@ Serializer::processOp<spirv::MemoryBarrierOp>(spirv::MemoryBarrierOp op) {
     operands.push_back(operand);
   }
 
-  return encodeInstructionInto(functionBody, spirv::Opcode::OpMemoryBarrier,
-                               operands);
+  encodeInstructionInto(functionBody, spirv::Opcode::OpMemoryBarrier, operands);
+  return success();
 }
 
 template <>
@@ -643,8 +649,8 @@ Serializer::processOp<spirv::FunctionCallOp>(spirv::FunctionCallOp op) {
   if (!resultTy.isa<NoneType>())
     valueIDMap[op.getResult(0)] = funcCallID;
 
-  return encodeInstructionInto(functionBody, spirv::Opcode::OpFunctionCall,
-                               operands);
+  encodeInstructionInto(functionBody, spirv::Opcode::OpFunctionCall, operands);
+  return success();
 }
 
 template <>
@@ -686,9 +692,9 @@ Serializer::processOp<spirv::CopyMemoryOp>(spirv::CopyMemoryOp op) {
   }
 
   elidedAttrs.push_back("source_alignment");
-  (void)emitDebugLine(functionBody, op.getLoc());
-  (void)encodeInstructionInto(functionBody, spirv::Opcode::OpCopyMemory,
-                              operands);
+  if (failed(emitDebugLine(functionBody, op.getLoc())))
+    return failure();
+  encodeInstructionInto(functionBody, spirv::Opcode::OpCopyMemory, operands);
 
   return success();
 }

diff  --git a/mlir/lib/Target/SPIRV/Serialization/Serializer.cpp b/mlir/lib/Target/SPIRV/Serialization/Serializer.cpp
index bd618ec4884b8..a306257eaed66 100644
--- a/mlir/lib/Target/SPIRV/Serialization/Serializer.cpp
+++ b/mlir/lib/Target/SPIRV/Serialization/Serializer.cpp
@@ -72,13 +72,11 @@ namespace spirv {
 
 /// Encodes an SPIR-V instruction with the given `opcode` and `operands` into
 /// the given `binary` vector.
-LogicalResult encodeInstructionInto(SmallVectorImpl<uint32_t> &binary,
-                                    spirv::Opcode op,
-                                    ArrayRef<uint32_t> operands) {
+void encodeInstructionInto(SmallVectorImpl<uint32_t> &binary, spirv::Opcode op,
+                           ArrayRef<uint32_t> operands) {
   uint32_t wordCount = 1 + operands.size();
   binary.push_back(spirv::getPrefixedOpcode(wordCount, op));
   binary.append(operands.begin(), operands.end());
-  return success();
 }
 
 Serializer::Serializer(spirv::ModuleOp module,
@@ -167,8 +165,8 @@ uint32_t Serializer::getOrCreateFunctionID(StringRef fnName) {
 
 void Serializer::processCapability() {
   for (auto cap : module.vce_triple()->getCapabilities())
-    (void)encodeInstructionInto(capabilities, spirv::Opcode::OpCapability,
-                                {static_cast<uint32_t>(cap)});
+    encodeInstructionInto(capabilities, spirv::Opcode::OpCapability,
+                          {static_cast<uint32_t>(cap)});
 }
 
 void Serializer::processDebugInfo() {
@@ -179,8 +177,8 @@ void Serializer::processDebugInfo() {
   fileID = getNextID();
   SmallVector<uint32_t, 16> operands;
   operands.push_back(fileID);
-  (void)spirv::encodeStringLiteralInto(operands, fileName);
-  (void)encodeInstructionInto(debug, spirv::Opcode::OpString, operands);
+  spirv::encodeStringLiteralInto(operands, fileName);
+  encodeInstructionInto(debug, spirv::Opcode::OpString, operands);
   // TODO: Encode more debug instructions.
 }
 
@@ -188,10 +186,8 @@ void Serializer::processExtension() {
   llvm::SmallVector<uint32_t, 16> extName;
   for (spirv::Extension ext : module.vce_triple()->getExtensions()) {
     extName.clear();
-    (void)spirv::encodeStringLiteralInto(extName,
-                                         spirv::stringifyExtension(ext));
-    (void)encodeInstructionInto(extensions, spirv::Opcode::OpExtension,
-                                extName);
+    spirv::encodeStringLiteralInto(extName, spirv::stringifyExtension(ext));
+    encodeInstructionInto(extensions, spirv::Opcode::OpExtension, extName);
   }
 }
 
@@ -199,8 +195,7 @@ void Serializer::processMemoryModel() {
   uint32_t mm = module->getAttrOfType<IntegerAttr>("memory_model").getInt();
   uint32_t am = module->getAttrOfType<IntegerAttr>("addressing_model").getInt();
 
-  (void)encodeInstructionInto(memoryModel, spirv::Opcode::OpMemoryModel,
-                              {am, mm});
+  encodeInstructionInto(memoryModel, spirv::Opcode::OpMemoryModel, {am, mm});
 }
 
 LogicalResult Serializer::processDecoration(Location loc, uint32_t resultID,
@@ -259,9 +254,9 @@ LogicalResult Serializer::processName(uint32_t resultID, StringRef name) {
 
   SmallVector<uint32_t, 4> nameOperands;
   nameOperands.push_back(resultID);
-  if (failed(spirv::encodeStringLiteralInto(nameOperands, name)))
-    return failure();
-  return encodeInstructionInto(names, spirv::Opcode::OpName, nameOperands);
+  spirv::encodeStringLiteralInto(nameOperands, name);
+  encodeInstructionInto(names, spirv::Opcode::OpName, nameOperands);
+  return success();
 }
 
 template <>
@@ -293,8 +288,8 @@ LogicalResult Serializer::processMemberDecoration(
   if (memberDecoration.hasValue) {
     args.push_back(memberDecoration.decorationValue);
   }
-  return encodeInstructionInto(decorations, spirv::Opcode::OpMemberDecorate,
-                               args);
+  encodeInstructionInto(decorations, spirv::Opcode::OpMemberDecorate, args);
+  return success();
 }
 
 //===----------------------------------------------------------------------===//
@@ -351,8 +346,7 @@ Serializer::processTypeImpl(Location loc, Type type, uint32_t &typeID,
 
     typeIDMap[type] = typeID;
 
-    if (failed(encodeInstructionInto(typesGlobalValues, typeEnum, operands)))
-      return failure();
+    encodeInstructionInto(typesGlobalValues, typeEnum, operands);
 
     if (recursiveStructInfos.count(type) != 0) {
       // This recursive struct type is emitted already, now the OpTypePointer
@@ -365,9 +359,8 @@ Serializer::processTypeImpl(Location loc, Type type, uint32_t &typeID,
         ptrOperands.push_back(static_cast<uint32_t>(ptrInfo.storageClass));
         ptrOperands.push_back(typeIDMap[type]);
 
-        if (failed(encodeInstructionInto(
-                typesGlobalValues, spirv::Opcode::OpTypePointer, ptrOperands)))
-          return failure();
+        encodeInstructionInto(typesGlobalValues, spirv::Opcode::OpTypePointer,
+                              ptrOperands);
       }
 
       recursiveStructInfos[type].clear();
@@ -471,9 +464,9 @@ LogicalResult Serializer::prepareBasicType(
       forwardPtrOperands.push_back(
           static_cast<uint32_t>(ptrType.getStorageClass()));
 
-      (void)encodeInstructionInto(typesGlobalValues,
-                                  spirv::Opcode::OpTypeForwardPointer,
-                                  forwardPtrOperands);
+      encodeInstructionInto(typesGlobalValues,
+                            spirv::Opcode::OpTypeForwardPointer,
+                            forwardPtrOperands);
 
       // 2. Find the pointee (enclosing) struct.
       auto structType = spirv::StructType::getIdentified(
@@ -534,7 +527,8 @@ LogicalResult Serializer::prepareBasicType(
 
   if (auto structType = type.dyn_cast<spirv::StructType>()) {
     if (structType.isIdentified()) {
-      (void)processName(resultID, structType.getIdentifier());
+      if (failed(processName(resultID, structType.getIdentifier())))
+        return failure();
       serializationCtx.insert(structType.getIdentifier());
     }
 
@@ -699,7 +693,7 @@ uint32_t Serializer::prepareArrayConstant(Location loc, Type constType,
     }
   }
   spirv::Opcode opcode = spirv::Opcode::OpConstantComposite;
-  (void)encodeInstructionInto(typesGlobalValues, opcode, operands);
+  encodeInstructionInto(typesGlobalValues, opcode, operands);
 
   return resultID;
 }
@@ -744,7 +738,7 @@ Serializer::prepareDenseElementsConstant(Location loc, Type constType,
     }
   }
   spirv::Opcode opcode = spirv::Opcode::OpConstantComposite;
-  (void)encodeInstructionInto(typesGlobalValues, opcode, operands);
+  encodeInstructionInto(typesGlobalValues, opcode, operands);
 
   return resultID;
 }
@@ -785,7 +779,7 @@ uint32_t Serializer::prepareConstantBool(Location loc, BoolAttr boolAttr,
                               : spirv::Opcode::OpConstantTrue)
                     : (isSpec ? spirv::Opcode::OpSpecConstantFalse
                               : spirv::Opcode::OpConstantFalse);
-  (void)encodeInstructionInto(typesGlobalValues, opcode, {typeID, resultID});
+  encodeInstructionInto(typesGlobalValues, opcode, {typeID, resultID});
 
   if (!isSpec) {
     constIDMap[boolAttr] = resultID;
@@ -831,8 +825,7 @@ uint32_t Serializer::prepareConstantInt(Location loc, IntegerAttr intAttr,
     } else {
       word = static_cast<uint32_t>(value.getZExtValue());
     }
-    (void)encodeInstructionInto(typesGlobalValues, opcode,
-                                {typeID, resultID, word});
+    encodeInstructionInto(typesGlobalValues, opcode, {typeID, resultID, word});
   } break;
     // According to SPIR-V spec: "When the type's bit width is larger than one
     // word, the literal’s low-order words appear first."
@@ -846,8 +839,8 @@ uint32_t Serializer::prepareConstantInt(Location loc, IntegerAttr intAttr,
     } else {
       words = llvm::bit_cast<DoubleWord>(value.getZExtValue());
     }
-    (void)encodeInstructionInto(typesGlobalValues, opcode,
-                                {typeID, resultID, words.word1, words.word2});
+    encodeInstructionInto(typesGlobalValues, opcode,
+                          {typeID, resultID, words.word1, words.word2});
   } break;
   default: {
     std::string valueStr;
@@ -890,20 +883,18 @@ uint32_t Serializer::prepareConstantFp(Location loc, FloatAttr floatAttr,
 
   if (&value.getSemantics() == &APFloat::IEEEsingle()) {
     uint32_t word = llvm::bit_cast<uint32_t>(value.convertToFloat());
-    (void)encodeInstructionInto(typesGlobalValues, opcode,
-                                {typeID, resultID, word});
+    encodeInstructionInto(typesGlobalValues, opcode, {typeID, resultID, word});
   } else if (&value.getSemantics() == &APFloat::IEEEdouble()) {
     struct DoubleWord {
       uint32_t word1;
       uint32_t word2;
     } words = llvm::bit_cast<DoubleWord>(value.convertToDouble());
-    (void)encodeInstructionInto(typesGlobalValues, opcode,
-                                {typeID, resultID, words.word1, words.word2});
+    encodeInstructionInto(typesGlobalValues, opcode,
+                          {typeID, resultID, words.word1, words.word2});
   } else if (&value.getSemantics() == &APFloat::IEEEhalf()) {
     uint32_t word =
         static_cast<uint32_t>(value.bitcastToAPInt().getZExtValue());
-    (void)encodeInstructionInto(typesGlobalValues, opcode,
-                                {typeID, resultID, word});
+    encodeInstructionInto(typesGlobalValues, opcode, {typeID, resultID, word});
   } else {
     std::string valueStr;
     llvm::raw_string_ostream rss(valueStr);
@@ -932,7 +923,7 @@ uint32_t Serializer::getOrCreateBlockID(Block *block) {
 
 LogicalResult
 Serializer::processBlock(Block *block, bool omitLabel,
-                         function_ref<void()> actionBeforeTerminator) {
+                         function_ref<LogicalResult()> actionBeforeTerminator) {
   LLVM_DEBUG(llvm::dbgs() << "processing block " << block << ":\n");
   LLVM_DEBUG(block->print(llvm::dbgs()));
   LLVM_DEBUG(llvm::dbgs() << '\n');
@@ -942,8 +933,7 @@ Serializer::processBlock(Block *block, bool omitLabel,
                << "[block] " << block << " (id = " << blockID << ")\n");
 
     // Emit OpLabel for this block.
-    (void)encodeInstructionInto(functionBody, spirv::Opcode::OpLabel,
-                                {blockID});
+    encodeInstructionInto(functionBody, spirv::Opcode::OpLabel, {blockID});
   }
 
   // Emit OpPhi instructions for block arguments, if any.
@@ -958,7 +948,8 @@ Serializer::processBlock(Block *block, bool omitLabel,
 
   // Process the terminator.
   if (actionBeforeTerminator)
-    actionBeforeTerminator();
+    if (failed(actionBeforeTerminator()))
+      return failure();
   if (failed(processOperation(&block->back())))
     return failure();
 
@@ -1047,7 +1038,7 @@ LogicalResult Serializer::emitPhiForBlockArguments(Block *block) {
       phiArgs.push_back(predBlockId);
     }
 
-    (void)encodeInstructionInto(functionBody, spirv::Opcode::OpPhi, phiArgs);
+    encodeInstructionInto(functionBody, spirv::Opcode::OpPhi, phiArgs);
     valueIDMap[arg] = phiID;
   }
 
@@ -1067,12 +1058,9 @@ LogicalResult Serializer::encodeExtensionInstruction(
     setID = getNextID();
     SmallVector<uint32_t, 16> importOperands;
     importOperands.push_back(setID);
-    if (failed(
-            spirv::encodeStringLiteralInto(importOperands, extensionSetName)) ||
-        failed(encodeInstructionInto(
-            extendedSets, spirv::Opcode::OpExtInstImport, importOperands))) {
-      return failure();
-    }
+    spirv::encodeStringLiteralInto(importOperands, extensionSetName);
+    encodeInstructionInto(extendedSets, spirv::Opcode::OpExtInstImport,
+                          importOperands);
   }
 
   // The first two operands are the result type <id> and result <id>. The set
@@ -1086,8 +1074,9 @@ LogicalResult Serializer::encodeExtensionInstruction(
   extInstOperands.push_back(setID);
   extInstOperands.push_back(extensionOpcode);
   extInstOperands.append(std::next(operands.begin(), 2), operands.end());
-  return encodeInstructionInto(functionBody, spirv::Opcode::OpExtInst,
-                               extInstOperands);
+  encodeInstructionInto(functionBody, spirv::Opcode::OpExtInst,
+                        extInstOperands);
+  return success();
 }
 
 LogicalResult Serializer::processOperation(Operation *opInst) {
@@ -1146,13 +1135,15 @@ LogicalResult Serializer::processOpWithoutGrammarAttr(Operation *op,
   for (Value operand : op->getOperands())
     operands.push_back(getValueID(operand));
 
-  (void)emitDebugLine(functionBody, loc);
+  if (failed(emitDebugLine(functionBody, loc)))
+    return failure();
 
   if (extInstSet.empty()) {
-    (void)encodeInstructionInto(functionBody,
-                                static_cast<spirv::Opcode>(opcode), operands);
+    encodeInstructionInto(functionBody, static_cast<spirv::Opcode>(opcode),
+                          operands);
   } else {
-    (void)encodeExtensionInstruction(op, extInstSet, opcode, operands);
+    if (failed(encodeExtensionInstruction(op, extInstSet, opcode, operands)))
+      return failure();
   }
 
   if (op->getNumResults() != 0) {
@@ -1189,9 +1180,8 @@ LogicalResult Serializer::emitDebugLine(SmallVectorImpl<uint32_t> &binary,
 
   auto fileLoc = loc.dyn_cast<FileLineColLoc>();
   if (fileLoc)
-    (void)encodeInstructionInto(
-        binary, spirv::Opcode::OpLine,
-        {fileID, fileLoc.getLine(), fileLoc.getColumn()});
+    encodeInstructionInto(binary, spirv::Opcode::OpLine,
+                          {fileID, fileLoc.getLine(), fileLoc.getColumn()});
   return success();
 }
 } // namespace spirv

diff  --git a/mlir/lib/Target/SPIRV/Serialization/Serializer.h b/mlir/lib/Target/SPIRV/Serialization/Serializer.h
index 5f4a4e999a673..0f053c0eb5e3f 100644
--- a/mlir/lib/Target/SPIRV/Serialization/Serializer.h
+++ b/mlir/lib/Target/SPIRV/Serialization/Serializer.h
@@ -23,9 +23,8 @@
 namespace mlir {
 namespace spirv {
 
-LogicalResult encodeInstructionInto(SmallVectorImpl<uint32_t> &binary,
-                                    spirv::Opcode op,
-                                    ArrayRef<uint32_t> operands);
+void encodeInstructionInto(SmallVectorImpl<uint32_t> &binary, spirv::Opcode op,
+                           ArrayRef<uint32_t> operands);
 
 /// A SPIR-V module serializer.
 ///
@@ -246,7 +245,7 @@ class Serializer {
   /// instruction if this is a SPIR-V selection/loop header block.
   LogicalResult
   processBlock(Block *block, bool omitLabel = false,
-               function_ref<void()> actionBeforeTerminator = nullptr);
+               function_ref<LogicalResult()> actionBeforeTerminator = nullptr);
 
   /// Emits OpPhi instructions for the given block if it has block arguments.
   LogicalResult emitPhiForBlockArguments(Block *block);


        


More information about the Mlir-commits mailing list