Skip to content

[mlir][emitc] Add 'emitc.while' and 'emitc.do' ops to the dialect #143008

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+1,212 −17
Open

[mlir][emitc] Add 'emitc.while' and 'emitc.do' ops to the dialect #143008

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 153 additions & 1 deletion mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1345,7 +1345,7 @@ def EmitC_AssignOp : EmitC_Op<"assign", []> {
}

def EmitC_YieldOp : EmitC_Op<"yield",
[Pure, Terminator, ParentOneOf<["ExpressionOp", "IfOp", "ForOp", "SwitchOp"]>]> {
[Pure, Terminator, ParentOneOf<["DoOp", "ExpressionOp", "ForOp", "IfOp", "SwitchOp", "WhileOp"]>]> {
let summary = "Block termination operation";
let description = [{
The `emitc.yield` terminates its parent EmitC op's region, optionally yielding
Expand Down Expand Up @@ -1572,4 +1572,156 @@ def EmitC_SwitchOp : EmitC_Op<"switch", [RecursiveMemoryEffects,
let hasVerifier = 1;
}

def EmitC_WhileOp : EmitC_Op<"while",
[HasOnlyGraphRegion, RecursiveMemoryEffects, NoRegionArguments, OpAsmOpInterface, NoTerminator]> {
let summary = "While operation";
let description = [{
The `emitc.while` operation represents a C/C++ while loop construct that
repeatedly executes a body region as long as a condition region evaluates to
true. The operation has two regions:

1. A condition region that must yield a boolean value (i1)
2. A body region that contains the loop body

The condition region is evaluated before each iteration. If it yields true,
the body region is executed. The loop terminates when the condition yields
false. The condition region must contain exactly one block that terminates
with an `emitc.yield` operation producing an i1 value.

Example:

```mlir
emitc.func @foo(%arg0 : !emitc.ptr<i32>) {
%var = "emitc.variable"() <{value = 0 : i32}> : () -> !emitc.lvalue<i32>
%0 = emitc.literal "10" : i32
%1 = emitc.literal "1" : i32

emitc.while {
%var_load = load %var : <i32>
%res = emitc.cmp le, %var_load, %0 : (i32, i32) -> i1
emitc.yield %res : i1
} do {
emitc.verbatim "printf(\"%d\", *{});" args %arg0 : !emitc.ptr<i32>
%var_load = load %var : <i32>
%tmp_add = add %var_load, %1 : (i32, i32) -> i32
"emitc.assign"(%var, %tmp_add) : (!emitc.lvalue<i32>, i32) -> ()
}

return
}
```

```c++
// Code emitted for the operation above.
void foo(int32_t* v1) {
int32_t v2 = 0;
while (v2 <= 10) {
printf("%d", *v1);
int32_t v3 = v2;
int32_t v4 = v3 + 1;
v2 = v4;
}
return;
}
```
}];

let arguments = (ins);
let results = (outs);
let regions = (region MaxSizedRegion<1>:$conditionRegion,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we put condition to arguments? In CPP, the condition should be an expression (or simple declaration).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand what you mean.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did it in a similar way to the SCF dialect to simplify the translation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think maybe keep it similar to 'emitc.if'. No need to use a region for condition.
Something like let arguments = (ins I1:$condition); is fine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this won't work, since you need to load, otherwise what would be the point of such while and do. If we go with inline, you'd also must ensure then that it always happens into the while cond and at this point, it seems like simpler to just put the cond into it.

if on the otherside doesn't have a problem of load-ing, etc and also doesn't care about inline, since it generally.

You also won't be able to translate to such emitc.while from scf.while into anything other than exit = false; do { cond = eval(); } while (cond).

Don't get me wrong, we've tried using plain cond, but the fact that you must have load working and that you must have scf translation, so operation is not dead made us use the current approach.

One could rely on emitc.expression being inlined, but it's much less clear from the user perspective, since you generally pass the result, not the entire expression in such cases.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add for clarification, since I've been helping @Vladislave0-0 to make it up and did some internal reviews before sending upstream, I'd also speak on his behalf on things we've agreed upon sending.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @jacquesguan regarding the relevance of emitc.expression. The C while loop condition is defined as a full expression IINM, so the code lowered to the proposed region will have to conform to those constraints, and replicating emitc.expression's behavior doesn't seem right.

OTOH, if the condition's emitc.expression is located outside the loop then inlining that expression would change program semantics from a single to multiple evaluations of the expression.

How about requiring the condition region of these loop ops to contain a single emitc.expression op feeding an emitc.yield? That should both utilize emitc.expression and avoid any inlining issues.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that you can not use emitc.expression as an operand because it's not a type, so you can only pass a bool pretty much.

OTOH, if the condition's emitc.expression is located outside the loop then inlining that expression would change program semantics from a single to multiple evaluations of the expression.

This will make while completely useless, it must always be inlined for load to be a part of the condition and be inside the cond while (cond).

How about requiring the condition region of these loop ops to contain a single emitc.expression op feeding an emitc.yield? That should both utilize emitc.expression and avoid any inlining issues.

Do you want to nest it one more time? That should work, a bit ugly, but I guess it's fine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to nest it one more time?

Yes, since the operations in the condition region must follow the semantics of emitc.expression anyway.
Consider for example the following IR

emitc.while {
    %a = emitc.call_opaque "foo" (): () -> i32
    %b = emitc.add %a, %a : (i32, i32) -> i32
    %res = emitc.cmp le, %a, %b : (i32, i32) -> i1
    emitc.yield %res : i1
} do {
   // ...
}

which the PR currently translates to

while (foo() <= foo() + foo()) {
   // ...
}

This translation is invalid since the opaque call may have side effects (emitc.expression forbids multiple uses of its sub-expressions to avoid duplication during translation).
We can concentrate all expression checks in a single function and call it from multiple places in the code but IMO it's a weaker solution than actually using emitc.expression, which both simplifies the code and keeps the IR clear and consistent. In addition, the form-expressions pass would be able to merge emitc.expression ops feeding this emitc.expression, increasing readability.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if we nest one more time it'll yield the same result, so we won't solve anything here in particular. It just seems like the intent is prohibit the use in such cases all together, since the only way CExpression works here, is because its inline path can not be triggered, which we force, since it's the intent. In the code, we just call CExpression logic from the while, it has the exact same checks as the CExpression pretty much, with some caveats of forcing the inline in some cases, like those.

The problem you describe is not nesting, but forced inline part, which nesting won't solve, since we'd just face the same logic one more time and bypass it the way we did before, unless we enforce the no-inline.

Just to make it clear, to do you want to have it that way

emitc.while {
    emitc.expression : i1 {
    }
}

?

Just to clarify, this won't change things much, since we routed while right into emitc.expression in translation, etc, but lifted some restrictions.

MaxSizedRegion<1>:$bodyRegion);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SizedRegion


let hasCustomAssemblyFormat = 1;
let hasVerifier = 1;

let extraClassDeclaration = [{
Operation *getRootOp();

//===------------------------------------------------------------------===//
// OpAsmOpInterface Methods
//===------------------------------------------------------------------===//

/// EmitC ops in the body can omit their 'emitc.' prefix in the assembly.
static ::llvm::StringRef getDefaultDialect() {
return "emitc";
}
}];
}

def EmitC_DoOp : EmitC_Op<"do",
[RecursiveMemoryEffects, NoRegionArguments, OpAsmOpInterface, NoTerminator]> {
let summary = "Do-while operation";
let description = [{
The `emitc.do` operation represents a C/C++ do-while loop construct that
executes a body region first and then repeatedly executes it as long as a
condition region evaluates to true. The operation has two regions:

1. A body region that contains the loop body
2. A condition region that must yield a boolean value (i1)

Unlike a while loop, the body region is executed before the first evaluation
of the condition. The loop terminates when the condition yields false. The
condition region must contain exactly one block that terminates with an
`emitc.yield` operation producing an i1 value.

Example:

```mlir
emitc.func @foo(%arg0 : !emitc.ptr<i32>) {
%var = "emitc.variable"() <{value = 0 : i32}> : () -> !emitc.lvalue<i32>
%0 = emitc.literal "10" : i32
%1 = emitc.literal "1" : i32

emitc.do {
emitc.verbatim "printf(\"%d\", *{});" args %arg0 : !emitc.ptr<i32>
%var_load = load %var : <i32>
%tmp_add = add %var_load, %1 : (i32, i32) -> i32
"emitc.assign"(%var, %tmp_add) : (!emitc.lvalue<i32>, i32) -> ()
} while {
%var_load = load %var : <i32>
%res = emitc.cmp le, %var_load, %0 : (i32, i32) -> i1
emitc.yield %res : i1
}

return
}
```

```c++
// Code emitted for the operation above.
void foo(int32_t* v1) {
int32_t v2 = 0;
do {
printf("%d", *v1);
int32_t v3 = v2;
int32_t v4 = v3 + 1;
v2 = v4;
} while (v2 <= 10);
return;
}
```
}];

let arguments = (ins);
let results = (outs);
let regions = (region MaxSizedRegion<1>:$bodyRegion,
MaxSizedRegion<1>:$conditionRegion);

let hasCustomAssemblyFormat = 1;
let hasVerifier = 1;

let extraClassDeclaration = [{
Operation *getRootOp();

//===------------------------------------------------------------------===//
// OpAsmOpInterface Methods
//===------------------------------------------------------------------===//

/// EmitC ops in the body can omit their 'emitc.' prefix in the assembly.
static ::llvm::StringRef getDefaultDialect() {
return "emitc";
}
}];
}

#endif // MLIR_DIALECT_EMITC_IR_EMITC
Loading