Skip to content

Commit a934ddc

Browse files
author
Simon Camphausen
authored
[mlir][EmitC] Do not inline expressions used by ops with the CExpression trait (#93691)
Currently an expression is inlined without emitting enclosing parentheses regardless of the context of the user. This could led to wrong evaluation order depending on the precedence of both expressions. If the inlining is intended, the user operation should be merged into the expression op. Fixes #93470.
1 parent fc5254c commit a934ddc

File tree

2 files changed

+83
-3
lines changed

2 files changed

+83
-3
lines changed

mlir/lib/Target/Cpp/TranslateToCpp.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -301,9 +301,9 @@ static bool shouldBeInlined(ExpressionOp expressionOp) {
301301
if (isa<emitc::SubscriptOp>(user))
302302
return false;
303303

304-
// Do not inline expressions used by other expressions, as any desired
305-
// expression folding was taken care of by transformations.
306-
return !user->getParentOfType<ExpressionOp>();
304+
// Do not inline expressions used by ops with the CExpression trait. If this
305+
// was intended, the user could have been merged into the expression op.
306+
return !user->hasTrait<OpTrait::emitc::CExpression>();
307307
}
308308

309309
static LogicalResult printConstantOp(CppEmitter &emitter, Operation *operation,

mlir/test/Target/Cpp/expressions.mlir

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,86 @@ func.func @parentheses_for_same_precedence(%arg0: i32, %arg1: i32, %arg2: i32) -
100100
return %e : i32
101101
}
102102

103+
// CPP-DEFAULT: int32_t user_with_expression_trait(int32_t [[VAL_1:v[0-9]+]], int32_t [[VAL_2:v[0-9]+]], int32_t [[VAL_3:v[0-9]+]]) {
104+
// CPP-DEFAULT-NEXT: int32_t [[VAL_4:v[0-9]+]] = 0;
105+
// CPP-DEFAULT-NEXT: int32_t [[EXP_0:v[0-9]+]] = [[VAL_3]] / ([[VAL_1]] * [[VAL_2]]);
106+
// CPP-DEFAULT-NEXT: int32_t [[EXP_1:v[0-9]+]] = [[VAL_3]] / ([[VAL_1]] * [[VAL_2]]);
107+
// CPP-DEFAULT-NEXT: int32_t [[EXP_2:v[0-9]+]] = [[VAL_3]] / ([[VAL_1]] * [[VAL_2]]);
108+
// CPP-DEFAULT-NEXT: int32_t [[EXP_3:v[0-9]+]] = [[VAL_3]] / ([[VAL_1]] * [[VAL_2]]);
109+
// CPP-DEFAULT-NEXT: bool [[CAST:v[0-9]+]] = (bool) [[EXP_0]];
110+
// CPP-DEFAULT-NEXT: int32_t [[ADD:v[0-9]+]] = [[EXP_1]] + [[VAL_4]];
111+
// CPP-DEFAULT-NEXT: int32_t [[CALL:v[0-9]+]] = bar([[EXP_2]], [[VAL_4]]);
112+
// CPP-DEFAULT-NEXT: int32_t [[COND:v[0-9]+]] = [[CAST]] ? [[EXP_3]] : [[VAL_4]];
113+
// CPP-DEFAULT-NEXT: int32_t [[VAR:v[0-9]+]];
114+
// CPP-DEFAULT-NEXT: [[VAR]] = [[VAL_3]] / ([[VAL_1]] * [[VAL_2]]);
115+
// CPP-DEFAULT-NEXT: return [[VAL_3]] / ([[VAL_1]] * [[VAL_2]]);
116+
// CPP-DEFAULT-NEXT: }
117+
118+
// CPP-DECLTOP: int32_t user_with_expression_trait(int32_t [[VAL_1:v[0-9]+]], int32_t [[VAL_2:v[0-9]+]], int32_t [[VAL_3:v[0-9]+]]) {
119+
// CPP-DECLTOP-NEXT: int32_t [[VAL_4:v[0-9]+]];
120+
// CPP-DECLTOP-NEXT: int32_t [[EXP_0:v[0-9]+]];
121+
// CPP-DECLTOP-NEXT: int32_t [[EXP_1:v[0-9]+]];
122+
// CPP-DECLTOP-NEXT: int32_t [[EXP_2:v[0-9]+]];
123+
// CPP-DECLTOP-NEXT: int32_t [[EXP_3:v[0-9]+]];
124+
// CPP-DECLTOP-NEXT: bool [[CAST:v[0-9]+]];
125+
// CPP-DECLTOP-NEXT: int32_t [[ADD:v[0-9]+]];
126+
// CPP-DECLTOP-NEXT: int32_t [[CALL:v[0-9]+]];
127+
// CPP-DECLTOP-NEXT: int32_t [[COND:v[0-9]+]];
128+
// CPP-DECLTOP-NEXT: int32_t [[VAR:v[0-9]+]];
129+
// CPP-DECLTOP-NEXT: [[VAL_4]] = 0;
130+
// CPP-DECLTOP-NEXT: [[EXP_0]] = [[VAL_3]] / ([[VAL_1]] * [[VAL_2]]);
131+
// CPP-DECLTOP-NEXT: [[EXP_1]] = [[VAL_3]] / ([[VAL_1]] * [[VAL_2]]);
132+
// CPP-DECLTOP-NEXT: [[EXP_2]] = [[VAL_3]] / ([[VAL_1]] * [[VAL_2]]);
133+
// CPP-DECLTOP-NEXT: [[EXP_3]] = [[VAL_3]] / ([[VAL_1]] * [[VAL_2]]);
134+
// CPP-DECLTOP-NEXT: [[CAST]] = (bool) [[EXP_0]];
135+
// CPP-DECLTOP-NEXT: [[ADD]] = [[EXP_1]] + [[VAL_4]];
136+
// CPP-DECLTOP-NEXT: [[CALL]] = bar([[EXP_2]], [[VAL_4]]);
137+
// CPP-DECLTOP-NEXT: [[COND]] = [[CAST]] ? [[EXP_3]] : [[VAL_4]];
138+
// CPP-DECLTOP-NEXT: ;
139+
// CPP-DECLTOP-NEXT: [[VAR]] = [[VAL_3]] / ([[VAL_1]] * [[VAL_2]]);
140+
// CPP-DECLTOP-NEXT: return [[VAL_3]] / ([[VAL_1]] * [[VAL_2]]);
141+
// CPP-DECLTOP-NEXT: }
142+
func.func @user_with_expression_trait(%arg0: i32, %arg1: i32, %arg2: i32) -> i32 {
143+
%c0 = "emitc.constant"() {value = 0 : i32} : () -> i32
144+
%e0 = emitc.expression : i32 {
145+
%0 = emitc.mul %arg0, %arg1 : (i32, i32) -> i32
146+
%1 = emitc.div %arg2, %0 : (i32, i32) -> i32
147+
emitc.yield %1 : i32
148+
}
149+
%e1 = emitc.expression : i32 {
150+
%0 = emitc.mul %arg0, %arg1 : (i32, i32) -> i32
151+
%1 = emitc.div %arg2, %0 : (i32, i32) -> i32
152+
emitc.yield %1 : i32
153+
}
154+
%e2 = emitc.expression : i32 {
155+
%0 = emitc.mul %arg0, %arg1 : (i32, i32) -> i32
156+
%1 = emitc.div %arg2, %0 : (i32, i32) -> i32
157+
emitc.yield %1 : i32
158+
}
159+
%e3 = emitc.expression : i32 {
160+
%0 = emitc.mul %arg0, %arg1 : (i32, i32) -> i32
161+
%1 = emitc.div %arg2, %0 : (i32, i32) -> i32
162+
emitc.yield %1 : i32
163+
}
164+
%e4 = emitc.expression : i32 {
165+
%0 = emitc.mul %arg0, %arg1 : (i32, i32) -> i32
166+
%1 = emitc.div %arg2, %0 : (i32, i32) -> i32
167+
emitc.yield %1 : i32
168+
}
169+
%e5 = emitc.expression : i32 {
170+
%0 = emitc.mul %arg0, %arg1 : (i32, i32) -> i32
171+
%1 = emitc.div %arg2, %0 : (i32, i32) -> i32
172+
emitc.yield %1 : i32
173+
}
174+
%cast = emitc.cast %e0 : i32 to i1
175+
%add = emitc.add %e1, %c0 : (i32, i32) -> i32
176+
%call = emitc.call_opaque "bar" (%e2, %c0) : (i32, i32) -> (i32)
177+
%cond = emitc.conditional %cast, %e3, %c0 : i32
178+
%var = "emitc.variable"() {value = #emitc.opaque<"">} : () -> i32
179+
emitc.assign %e4 : i32 to %var : i32
180+
return %e5 : i32
181+
}
182+
103183
// CPP-DEFAULT: int32_t multiple_uses(int32_t [[VAL_1:v[0-9]+]], int32_t [[VAL_2:v[0-9]+]], int32_t [[VAL_3:v[0-9]+]], int32_t [[VAL_4:v[0-9]+]]) {
104184
// CPP-DEFAULT-NEXT: bool [[VAL_5:v[0-9]+]] = bar([[VAL_1]] * [[VAL_2]], [[VAL_3]]) - [[VAL_4]] < [[VAL_2]];
105185
// CPP-DEFAULT-NEXT: int32_t [[VAL_6:v[0-9]+]];

0 commit comments

Comments
 (0)