fix code review comments

Signed-off-by: Davide Grohmann <[email protected]>
Change-Id: I7028dbe4bbcd644f82369785614b16d990a6f11c

Author: davidegrohmann

mlir/lib/Target/SPIRV/Deserialization/DeserializeOps.cpp

@@ -90,8 +90,8 @@ Value spirv::Deserializer::getValue(uint32_t id) {
           graphConstantARMInfo = getGraphConstantARM(id)) {
     IntegerAttr graphConstantID = graphConstantARMInfo->graphConstantID;
     Type resultType = graphConstantARMInfo->resultType;
-    return opBuilder.create<spirv::GraphConstantARMOp>(unknownLoc, resultType,
-                                                       graphConstantID);
+    return spirv::GraphConstantARMOp::create(opBuilder, unknownLoc, resultType,
+                                             graphConstantID);
   }
   return valueMap.lookup(id);
 }

mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp

@@ -677,20 +677,20 @@ spirv::Deserializer::processGraphEntryPointARM(ArrayRef<uint32_t> operands) {
   }
 
   unsigned wordIndex = 0;
-  uint32_t grID = operands[wordIndex++];
-  if (!graphMap.count(grID)) {
+  uint32_t graphID = operands[wordIndex++];
+  if (!graphMap.contains(graphID)) {
     return emitError(unknownLoc,
                      "missing graph definition/declaration with id ")
-           << grID;
+           << graphID;
   }
 
-  spirv::GraphARMOp graphARM = graphMap[grID];
+  spirv::GraphARMOp graphARM = graphMap[graphID];
   StringRef name = decodeStringLiteral(operands, wordIndex);
   graphARM.setSymName(name);
   graphARM.setEntryPoint(true);
 
   SmallVector<Attribute, 4> interface;
-  for (int64_t size = operands.size(); wordIndex < size; wordIndex++) {
+  for (int64_t size = operands.size(); wordIndex < size; ++wordIndex) {
     if (spirv::GlobalVariableOp arg = getGlobalVariable(operands[wordIndex])) {
       interface.push_back(SymbolRefAttr::get(arg.getOperation()));
     } else {
@@ -729,22 +729,22 @@ spirv::Deserializer::processGraphARM(ArrayRef<uint32_t> operands) {
     return emitError(unknownLoc, "expected at least one result");
   }
 
-  uint32_t grID = operands[1];
-  if (graphMap.count(grID)) {
+  uint32_t graphID = operands[1];
+  if (graphMap.count(graphID)) {
     return emitError(unknownLoc, "duplicate graph definition/declaration");
   }
 
-  std::string grName = getGraphSymbol(grID);
+  std::string graphName = getGraphSymbol(graphID);
   auto graphOp =
-      opBuilder.create<spirv::GraphARMOp>(unknownLoc, grName, graphType);
-  curGraph = graphMap[grID] = graphOp;
+      opBuilder.create<spirv::GraphARMOp>(unknownLoc, graphName, graphType);
+  curGraph = graphMap[graphID] = graphOp;
   Block *entryBlock = graphOp.addEntryBlock();
   LLVM_DEBUG({
     logger.startLine()
         << "//===-------------------------------------------===//\n";
-    logger.startLine() << "[graph] name: " << grName << "\n";
+    logger.startLine() << "[graph] name: " << graphName << "\n";
     logger.startLine() << "[graph] type: " << graphType << "\n";
-    logger.startLine() << "[graph] ID: " << grID << "\n";
+    logger.startLine() << "[graph] ID: " << graphID << "\n";
     logger.startLine() << "[graph] entry block: " << entryBlock << "\n";
     logger.indent();
   });
@@ -795,8 +795,8 @@ spirv::Deserializer::processGraphARM(ArrayRef<uint32_t> operands) {
   // deserializing the body of this function.
   OpBuilder::InsertionGuard moduleInsertionGuard(opBuilder);
 
-  blockMap[grID] = entryBlock;
-  if (failed(createGraphBlock(grID))) {
+  blockMap[graphID] = entryBlock;
+  if (failed(createGraphBlock(graphID))) {
     return failure();
   }
 
@@ -1535,7 +1535,7 @@ spirv::Deserializer::processGraphTypeARM(ArrayRef<uint32_t> operands) {
   uint32_t numInputs = operands[1];
   SmallVector<Type, 1> argTypes;
   SmallVector<Type, 1> returnTypes;
-  for (unsigned i = 2; i < size; i++) {
+  for (unsigned i = 2; i < size; ++i) {
     Type inOutTy = getType(operands[i]);
     if (!inOutTy) {
       return emitError(unknownLoc,

mlir/lib/Target/SPIRV/Serialization/Serializer.cpp

@@ -889,20 +889,18 @@ Serializer::prepareGraphType(Location loc, GraphType type,
 
   operands.push_back(type.getNumInputs());
 
-  for (const Type &res : type.getInputs()) {
+  for (Type argType : type.getInputs()) {
     uint32_t argTypeID = 0;
-    if (failed(processType(loc, res, argTypeID))) {
+    if (failed(processType(loc, argType, argTypeID)))
       return failure();
-    }
     operands.push_back(argTypeID);
   }
 
-  for (const Type &res : type.getResults()) {
-    uint32_t resultID = 0;
-    if (failed(processType(loc, res, resultID))) {
+  for (Type resType : type.getResults()) {
+    uint32_t resTypeID = 0;
+    if (failed(processType(loc, resType, resTypeID)))
       return failure();
-    }
-    operands.push_back(resultID);
+    operands.push_back(resTypeID);
   }
 
   return success();