[Mlir-commits] [mlir] [mlir][emitc] Inline expressions with side-effects (PR #161356)

[Gil Rapaport]

================
@@ -377,7 +372,34 @@ static bool shouldBeInlined(ExpressionOp expressionOp) {
   // Do not inline expressions used by other expressions or by ops with the
   // CExpressionInterface. If this was intended, the user could have been merged
   // into the expression op.
-  return !isa<emitc::ExpressionOp, emitc::CExpressionInterface>(*user);
+  if (isa<emitc::ExpressionOp, emitc::CExpressionInterface>(*user))
+    return false;
+
+  // Expressions with no side-effects can safely be inlined.
+  if (!expressionOp.hasSideEffects())
+    return true;
+
+  // Expressions with side-effects can be only inlined if side-effect ordering
+  // in the program is provably retained.
+
+  // Require the user to immediately follow the expression.
+  if (++Block::iterator(expressionOp) != Block::iterator(user))
+    return false;
----------------
aniragil wrote:

Similar to placing the expression just before the user, I think that in order to keep the translator simple this extension:

> Right. As C doesn't define the evaluation order in most cases (e.g. between the operands of `+`) inlining expressions as the operands requires making sure that their accesses are independent (e.g. that none of the variables have its address taken, since it could then be passed to some call that would store to it).

should be implemented in the `form-expressions` pass, while this extension:

> 
> Where evaluation order is defined, e.g. between the arguments to a function call, we should be able to trivially extend it to inline all arguments that are just before the call and in the right order.

can only be implemented in the translator.

https://github.com/llvm/llvm-project/pull/161356