Review comments

Author: Simon Camphausen

mlir/include/mlir/Dialect/EmitC/IR/EmitC.td

@@ -839,10 +839,14 @@ def EmitC_LogicalOrOp : EmitC_BinaryOp<"logical_or", [CExpression]> {
 def EmitC_LValueLoadOp : EmitC_Op<"lvalue_load", [
   TypesMatchWith<"result type matches value type of 'operand'",
                   "operand", "result",
-                  "::llvm::cast<LValueType>($_self).getValue()">
+                  "::llvm::cast<LValueType>($_self).getValueType()">
 ]> {
   let summary = "Load an lvalue into an SSA value.";
-  let description = [{}];
+  let description = [{
+    This operation loads the content of a modifiable lvalue into an SSA value. 
+    Modifications of the lvalue executed after the load are not observable on 
+    the produced value.
+  }];
 
   let arguments = (ins 
       Res<EmitC_LValueType, "", [MemRead<DefaultResource, 0, FullEffect>]>:$operand);
@@ -1198,7 +1202,7 @@ def EmitC_AssignOp : EmitC_Op<"assign", []> {
 
   let arguments = (ins 
       Res<EmitC_LValueType, "", [MemWrite<DefaultResource, 1, FullEffect>]>:$var,
-      Res<EmitCType, "", [MemRead<DefaultResource, 0, FullEffect>]>:$value);
+      EmitCType:$value);
   let results = (outs);
 
   let hasVerifier = 1;

mlir/include/mlir/Dialect/EmitC/IR/EmitCTypes.td

@@ -91,13 +91,13 @@ def EmitC_LValueType : EmitC_Type<"LValue", "lvalue"> {
     Values of this type can be assigned to and their address can be taken.
   }];
 
-  let parameters = (ins "Type":$value);
+  let parameters = (ins "Type":$valueType);
   let builders = [
-    TypeBuilderWithInferredContext<(ins "Type":$value), [{
-      return $_get(value.getContext(), value);
+    TypeBuilderWithInferredContext<(ins "Type":$valueType), [{
+      return $_get(valueType.getContext(), valueType);
     }]>
   ];
-  let assemblyFormat = "`<` qualified($value) `>`";
+  let assemblyFormat = "`<` qualified($valueType) `>`";
   let genVerifyDecl = 1;
 }
 

mlir/lib/Conversion/SCFToEmitC/SCFToEmitC.cpp

@@ -84,7 +84,7 @@ static void assignValues(ValueRange values, SmallVector<Value> &variables,
 SmallVector<Value> loadValues(const SmallVector<Value> &variables,
                               PatternRewriter &rewriter, Location loc) {
   return llvm::map_to_vector<>(variables, [&](Value var) {
-    Type type = cast<emitc::LValueType>(var.getType()).getValue();
+    Type type = cast<emitc::LValueType>(var.getType()).getValueType();
     return rewriter.create<emitc::LValueLoadOp>(loc, type, var).getResult();
   });
 }
@@ -138,7 +138,7 @@ LogicalResult ForLowering::matchAndRewrite(ForOp forOp,
   lowerYield(resultVariables, rewriter,
              cast<scf::YieldOp>(loweredBody->getTerminator()));
 
-  // Copy iterArgs into results after the for loop.
+  // Load variables into SSA values after the for loop.
   SmallVector<Value> resultValues = loadValues(resultVariables, rewriter, loc);
 
   rewriter.replaceOp(forOp, resultValues);

mlir/lib/Dialect/EmitC/IR/EmitC.cpp

@@ -149,7 +149,7 @@ static LogicalResult verifyInitializationAttribute(Operation *op,
 
   Type resultType = op->getResult(0).getType();
   if (auto lType = dyn_cast<LValueType>(resultType))
-    resultType = lType.getValue();
+    resultType = lType.getValueType();
   Type attrType = cast<TypedAttr>(value).getType();
 
   if (isPointerWideType(resultType) && attrType.isIndex())
@@ -229,7 +229,7 @@ LogicalResult emitc::AssignOp::verify() {
     return emitOpError() << "cannot assign to block argument";
 
   Type valueType = getValue().getType();
-  Type variableType = variable.getType().getValue();
+  Type variableType = variable.getType().getValueType();
   if (variableType != valueType)
     return emitOpError() << "requires value's type (" << valueType
                          << ") to match variable's type (" << variableType
@@ -862,7 +862,7 @@ LogicalResult emitc::SubscriptOp::verify() {
     }
     // Check element type.
     Type elementType = arrayType.getElementType();
-    Type resultType = getType().getValue();
+    Type resultType = getType().getValueType();
     if (elementType != resultType) {
       return emitOpError() << "on array operand requires element type ("
                            << elementType << ") and result type (" << resultType
@@ -889,7 +889,7 @@ LogicalResult emitc::SubscriptOp::verify() {
     }
     // Check pointee type.
     Type pointeeType = pointerType.getPointee();
-    Type resultType = getType().getValue();
+    Type resultType = getType().getValueType();
     if (pointeeType != resultType) {
       return emitOpError() << "on pointer operand requires pointee type ("
                            << pointeeType << ") and result type (" << resultType
@@ -1144,9 +1144,9 @@ GetGlobalOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
 
   // global has non-array type
   auto lvalueType = dyn_cast<LValueType>(resultType);
-  if (!lvalueType || lvalueType.getValue() != globalType)
+  if (!lvalueType || lvalueType.getValueType() != globalType)
     return emitOpError("on non-array type expects result inner type ")
-           << lvalueType.getValue() << " to match type " << globalType
+           << lvalueType.getValueType() << " to match type " << globalType
            << " of the global @" << getName();
   return success();
 }

mlir/lib/Target/Cpp/TranslateToCpp.cpp

@@ -1647,7 +1647,7 @@ LogicalResult CppEmitter::emitType(Location loc, Type type) {
     return success();
   }
   if (auto lType = dyn_cast<emitc::LValueType>(type))
-    return emitType(loc, lType.getValue());
+    return emitType(loc, lType.getValueType());
   if (auto pType = dyn_cast<emitc::PointerType>(type)) {
     if (isa<ArrayType>(pType.getPointee()))
       return emitError(loc, "cannot emit pointer to array type ") << type;