This is an archive of the discontinued LLVM Phabricator instance.

Differential D76524

[MLIR] IfOP - Add a builder with result operands
ClosedPublic

Authored by nmostafa on Mar 20 2020, 1:46 PM.

Details

Reviewers
nicolasvasilache
rriddle
Commits
rG141e5890d812: [MLIR] Add an IfOp builder with results
Summary

Add a new builder to IfOp to allow operation to define values.

Diff Detail

Event Timeline

nmostafa created this revision.Mar 20 2020, 1:46 PM
Herald added a reviewer: nicolasvasilache. · View Herald TranscriptMar 20 2020, 1:46 PM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added subscribers: llvm-commits, Joonsoo, liufengdb and 10 others. · View Herald Transcript
nmostafa added a reviewer: rriddle.Mar 20 2020, 1:47 PM
rriddle accepted this revision.Mar 20 2020, 2:17 PM
rriddle added inline comments.
mlir/include/mlir/Dialect/LoopOps/LoopOps.td
226

nit: Can you use TypeRange here instead of ArrayRef?

mlir/lib/Dialect/LoopOps/LoopOps.cpp
204

nit: Can you add a /*...=*/ comment to the llvm::None?

This revision is now accepted and ready to land.Mar 20 2020, 2:17 PM
nmostafa updated this revision to Diff 251793.Mar 20 2020, 4:08 PM

Review fixes.

nmostafa marked 2 inline comments as done.Mar 20 2020, 4:08 PM

Thanks, @rriddle . Will submit it soon.

Harbormaster completed remote builds in B49951: Diff 251752.Mar 20 2020, 4:18 PM
lebedev.ri retitled this revision from IfOP - Add a builder with result operands to [MLIR] IfOP - Add a builder with result operands.Mar 20 2020, 4:18 PM
Closed by commit rG141e5890d812: [MLIR] Add an IfOp builder with results (authored by nmostafa <nagy.h.mostafa@intel.com>). · Explain WhyMar 20 2020, 5:23 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
mlir/
include/
mlir/
Dialect/
LoopOps/
5 lines
lib/
Dialect/
LoopOps/
6 lines

Diff 251809

mlir/include/mlir/Dialect/LoopOps/LoopOps.td

Show First 20 Lines • Show All 215 Lines • ▼ Show 20 Linesdef IfOp : Loop_Op<"if",
}]; }];
let arguments = (ins I1:$condition); let arguments = (ins I1:$condition);
let results = (outs Variadic<AnyType>:$results); let results = (outs Variadic<AnyType>:$results);
let regions = (region SizedRegion<1>:$thenRegion, AnyRegion:$elseRegion); let regions = (region SizedRegion<1>:$thenRegion, AnyRegion:$elseRegion);
let skipDefaultBuilders = 1; let skipDefaultBuilders = 1;
let builders = [ let builders = [
OpBuilder<"Builder *builder, OperationState &result, " OpBuilder<"Builder *builder, OperationState &result, "
"Value cond, bool withElseRegion"> "Value cond, bool withElseRegion">,
OpBuilder<"Builder *builder, OperationState &result, "
"TypeRange resultTypes, Value cond, "
rriddleUnsubmitted
Done
ReplyInline Actions

nit: Can you use TypeRange here instead of ArrayRef?

rriddle: nit: Can you use TypeRange here instead of ArrayRef?
"bool withElseRegion">
]; ];
let extraClassDeclaration = [{ let extraClassDeclaration = [{
OpBuilder getThenBodyBuilder() { OpBuilder getThenBodyBuilder() {
assert(!thenRegion().empty() && "Unexpected empty 'then' region."); assert(!thenRegion().empty() && "Unexpected empty 'then' region.");
Block &body = thenRegion().front(); Block &body = thenRegion().front();
return OpBuilder(&body, std::prev(body.end())); return OpBuilder(&body, std::prev(body.end()));
} }
▲ Show 20 Lines • Show All 168 Lines • Show Last 20 Lines

mlir/lib/Dialect/LoopOps/LoopOps.cpp

Show First 20 Lines • Show All 195 Lines • ▼ Show 20 Lines
} }
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
// IfOp // IfOp
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
void IfOp::build(Builder *builder, OperationState &result, Value cond, void IfOp::build(Builder *builder, OperationState &result, Value cond,
bool withElseRegion) { bool withElseRegion) {
build(builder, result, /*resultTypes=*/llvm::None, cond, withElseRegion);
rriddleUnsubmitted
Done
ReplyInline Actions

nit: Can you add a /*...=*/ comment to the llvm::None?

rriddle: nit: Can you add a /*...=*/ comment to the llvm::None?
}
void IfOp::build(Builder *builder, OperationState &result,
TypeRange resultTypes, Value cond, bool withElseRegion) {
result.addOperands(cond); result.addOperands(cond);
result.addTypes(resultTypes);
Region *thenRegion = result.addRegion(); Region *thenRegion = result.addRegion();
Region *elseRegion = result.addRegion(); Region *elseRegion = result.addRegion();
IfOp::ensureTerminator(*thenRegion, *builder, result.location); IfOp::ensureTerminator(*thenRegion, *builder, result.location);
if (withElseRegion) if (withElseRegion)
IfOp::ensureTerminator(*elseRegion, *builder, result.location); IfOp::ensureTerminator(*elseRegion, *builder, result.location);
} }
static LogicalResult verify(IfOp op) { static LogicalResult verify(IfOp op) {
▲ Show 20 Lines • Show All 376 Lines • Show Last 20 Lines