Readability: Add option to emit constants values in place

* Readability: Add option to emit constants values in place, instead of producing dedicated variables.

Author: lmendesp-amd

mlir/include/mlir/Target/Cpp/CppEmitter.h

@@ -26,7 +26,8 @@ namespace emitc {
 /// arguments are declared at the beginning of the function.
 LogicalResult translateToCpp(Operation *op, raw_ostream &os,
                              bool declareVariablesAtTop = false,
-                             StringRef onlyTu = "");
+                             StringRef onlyTu = "",
+                             bool constantsAsVariables = true);
 } // namespace emitc
 } // namespace mlir
 

mlir/lib/Target/Cpp/TranslateRegistration.cpp

@@ -34,13 +34,19 @@ void registerToCppTranslation() {
       llvm::cl::desc("Only emit the translation unit with the matching id"),
       llvm::cl::init(""));
 
+  static llvm::cl::opt<bool> constantsAsVariables(
+      "constants-as-variables",
+      llvm::cl::desc("Use variables to hold the constant values"),
+      llvm::cl::init(true));
+
   TranslateFromMLIRRegistration reg(
       "mlir-to-cpp", "translate from mlir to cpp",
       [](Operation *op, raw_ostream &output) {
         return emitc::translateToCpp(
             op, output,
             /*declareVariablesAtTop=*/declareVariablesAtTop,
-            /*onlyTu=*/onlyTu);
+            /*onlyTu=*/onlyTu,
+            /*constantsAsVariables=*/constantsAsVariables);
       },
       [](DialectRegistry &registry) {
         // clang-format off

mlir/lib/Target/Cpp/TranslateToCpp.cpp

@@ -116,7 +116,7 @@ namespace {
 /// Emitter that uses dialect specific emitters to emit C++ code.
 struct CppEmitter {
   explicit CppEmitter(raw_ostream &os, bool declareVariablesAtTop,
-                      StringRef onlyTu);
+                      StringRef onlyTu, bool constantsAsVariables);
 
   /// Emits attribute or returns failure.
   LogicalResult emitAttribute(Location loc, Attribute attr);
@@ -235,6 +235,10 @@ struct CppEmitter {
   /// Returns whether this translation unit should be emitted
   bool shouldEmitTu(TranslationUnitOp tu) { return tu.getId() == onlyTu; }
 
+  /// Returns whether the value of ConstantOps should be stored in variables
+  /// or emmited directly in their usage locations.
+  bool shouldUseConstantsAsVariables() { return constantsAsVariables; }
+
   /// Get expression currently being emitted.
   ExpressionOp getEmittedExpression() { return emittedExpression; }
 
@@ -265,6 +269,9 @@ struct CppEmitter {
   /// Only emit translation units whos id matches this value.
   std::string onlyTu;
 
+  /// Use variables to hold the constant values
+  bool constantsAsVariables;
+
   /// Map from value to name of C++ variable that contain the name.
   ValueMapper valueMapper;
 
@@ -365,6 +372,10 @@ static LogicalResult printConstantOp(CppEmitter &emitter, Operation *operation,
 
 static LogicalResult printOperation(CppEmitter &emitter,
                                     emitc::ConstantOp constantOp) {
+  if (!emitter.shouldUseConstantsAsVariables()) {
+    return success();
+  }
+
   Operation *operation = constantOp.getOperation();
   Attribute value = constantOp.getValue();
 
@@ -1218,9 +1229,9 @@ static LogicalResult printOperation(CppEmitter &emitter,
 }
 
 CppEmitter::CppEmitter(raw_ostream &os, bool declareVariablesAtTop,
-                       StringRef onlyTu)
+                       StringRef onlyTu, bool constantsAsVariables)
     : os(os), declareVariablesAtTop(declareVariablesAtTop),
-      onlyTu(onlyTu.str()) {
+      onlyTu(onlyTu.str()), constantsAsVariables(constantsAsVariables) {
   valueInScopeCount.push(0);
   labelInScopeCount.push(0);
 }
@@ -1425,8 +1436,25 @@ LogicalResult CppEmitter::emitExpression(ExpressionOp expressionOp) {
 }
 
 LogicalResult CppEmitter::emitOperand(Value value) {
+  Operation *def = value.getDefiningOp();
+  if (!shouldUseConstantsAsVariables()) {
+    if (auto constant = dyn_cast_if_present<ConstantOp>(def)) {
+      os << "((";
+
+      if (failed(emitType(constant.getLoc(), constant.getType()))) {
+        return failure();
+      }
+      os << ") ";
+
+      if (failed(emitAttribute(constant.getLoc(), constant.getValue()))) {
+        return failure();
+      }
+      os << ")";
+      return success();
+    }
+  }
+
   if (isPartOfCurrentExpression(value)) {
-    Operation *def = value.getDefiningOp();
     assert(def && "Expected operand to be defined by an operation");
     FailureOr<int> precedence = getOperatorPrecedence(def);
     if (failed(precedence))
@@ -1452,7 +1480,7 @@ LogicalResult CppEmitter::emitOperand(Value value) {
     return success();
   }
 
-  auto expressionOp = dyn_cast_if_present<ExpressionOp>(value.getDefiningOp());
+  auto expressionOp = dyn_cast_if_present<ExpressionOp>(def);
   if (expressionOp && shouldBeInlined(expressionOp))
     return emitExpression(expressionOp);
 
@@ -1671,7 +1699,13 @@ LogicalResult CppEmitter::emitOperation(Operation &op, bool trailingSemicolon) {
     trailingSemicolon = false;
   }
 
-  os << (trailingSemicolon ? ";\n" : "\n");
+  bool trailingNewline = true;
+  if (!shouldUseConstantsAsVariables() && isa<emitc::ConstantOp>(op)) {
+    trailingSemicolon = false;
+    trailingNewline = false;
+  }
+
+  os << (trailingSemicolon ? ";" : "") << (trailingNewline ? "\n" : "");
 
   return success();
 }
@@ -1837,7 +1871,8 @@ LogicalResult CppEmitter::emitTupleType(Location loc, ArrayRef<Type> types) {
 
 LogicalResult emitc::translateToCpp(Operation *op, raw_ostream &os,
                                     bool declareVariablesAtTop,
-                                    StringRef onlyTu) {
-  CppEmitter emitter(os, declareVariablesAtTop, onlyTu);
+                                    StringRef onlyTu,
+                                    bool constantsAsVariables) {
+  CppEmitter emitter(os, declareVariablesAtTop, onlyTu, constantsAsVariables);
   return emitter.emitOperation(*op, /*trailingSemicolon=*/false);
 }

mlir/test/Target/Cpp/emitc-constants-as-variables.mlir

@@ -0,0 +1,19 @@
+// RUN: mlir-translate -mlir-to-cpp -constants-as-variables=false %s | FileCheck %s -check-prefix=CPP-DEFAULT
+
+func.func @test() {
+  %start = "emitc.constant"() <{value = 0 : index}> : () -> !emitc.size_t
+  %stop = "emitc.constant"() <{value = 10 : index}> : () -> !emitc.size_t
+  %step = "emitc.constant"() <{value = 1 : index}> : () -> !emitc.size_t
+
+  emitc.for %iter = %start to %stop step %step {
+    emitc.yield
+  }
+
+  return
+}
+
+// CPP-DEFAULT: void test() {
+// CPP-DEFAULT-NEXT:   for (size_t v1 = ((size_t) 0); v1 < ((size_t) 10); v1 += ((size_t) 1)) {
+// CPP-DEFAULT-NEXT:   }
+// CPP-DEFAULT-NEXT:   return;
+// CPP-DEFAULT-NEXT: }