Skip to content

Commit

Permalink
Readability: Add option to emit constants values in place
Browse files Browse the repository at this point in the history
* Readability: Add option to emit constants values in place, instead of producing dedicated variables.
  • Loading branch information
lmendesp-amd authored Nov 28, 2024
1 parent e25d207 commit f3f4919
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 10 deletions.
3 changes: 2 additions & 1 deletion mlir/include/mlir/Target/Cpp/CppEmitter.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 7 additions & 1 deletion mlir/lib/Target/Cpp/TranslateRegistration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
51 changes: 43 additions & 8 deletions mlir/lib/Target/Cpp/TranslateToCpp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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; }

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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))
Expand All @@ -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);

Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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);
}
19 changes: 19 additions & 0 deletions mlir/test/Target/Cpp/emitc-constants-as-variables.mlir
Original file line number Diff line number Diff line change
@@ -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: }

0 comments on commit f3f4919

Please sign in to comment.