Skip to content

Commit

Permalink
[MLIR][EmitC] Don't translate expressions inline if user is `emitc.su…
Browse files Browse the repository at this point in the history
…bscript` (llvm#91087)

This change updates the logic that determines whether an `emitc.expression`
result is translated into a dedicated variable assignment. Due to how
the translation of `emitc.subscript` currently works, a previously
inline-able `emitc.expression` would produce incorrect C++ if its single user 
was a `emitc.subscript` operation.
  • Loading branch information
christopherbate authored May 6, 2024
1 parent 363ec6f commit 657eda3
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 1 deletion.
9 changes: 8 additions & 1 deletion mlir/lib/Target/Cpp/TranslateToCpp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -293,9 +293,16 @@ static bool shouldBeInlined(ExpressionOp expressionOp) {
if (!result.hasOneUse())
return false;

Operation *user = *result.getUsers().begin();

// Do not inline expressions used by subscript operations, since the
// way the subscript operation translation is implemented requires that
// variables be materialized.
if (isa<emitc::SubscriptOp>(user))
return false;

// Do not inline expressions used by other expressions, as any desired
// expression folding was taken care of by transformations.
Operation *user = *result.getUsers().begin();
return !user->getParentOfType<ExpressionOp>();
}

Expand Down
15 changes: 15 additions & 0 deletions mlir/test/Target/Cpp/expressions.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -210,3 +210,18 @@ func.func @expression_with_address_taken(%arg0: i32, %arg1: i32, %arg2: !emitc.p
}
return %c : i1
}

// CPP-DEFAULT: int32_t expression_with_subscript_user(void* [[VAL_1:v.+]])
// CPP-DEFAULT-NEXT: int64_t [[VAL_2:v.+]] = 0;
// CPP-DEFAULT-NEXT: int32_t* [[VAL_3:v.+]] = (int32_t*) [[VAL_1]];
// CPP-DEFAULT-NEXT: return [[VAL_3]][[[VAL_2]]];

func.func @expression_with_subscript_user(%arg0: !emitc.ptr<!emitc.opaque<"void">>) -> i32 {
%c0 = "emitc.constant"() {value = 0 : i64} : () -> i64
%0 = emitc.expression : !emitc.ptr<i32> {
%0 = emitc.cast %arg0 : !emitc.ptr<!emitc.opaque<"void">> to !emitc.ptr<i32>
emitc.yield %0 : !emitc.ptr<i32>
}
%1 = emitc.subscript %0[%c0] : (!emitc.ptr<i32>, i64) -> i32
return %1 : i32
}

0 comments on commit 657eda3

Please sign in to comment.