This is an archive of the discontinued LLVM Phabricator instance.

Differential D81513

[MLIR] Print function name when ReturnOp verification fails
ClosedPublic

Authored by jurahul on Jun 9 2020, 3:22 PM.

Details

Reviewers
mehdi_amini
jpienaar
rriddle
Commits
rGa0dd5e876f20: [MLIR] Print function name when ReturnOp verification fails
Summary
  • Print function name when ReturnOp verification fails
  • This helps easily finding the invalid ReturnOp in an IR dump.

Diff Detail

Unit TestsFailed

TimeTest
140 mslinux > MLIR.IR::Unknown Unit Message ("")
220 mswindows > MLIR.IR::Unknown Unit Message ("")

Event Timeline

jurahul created this revision.Jun 9 2020, 3:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 9 2020, 3:22 PM
Herald added subscribers: msifontes, Kayjukh, frgossen and 13 others. · View Herald Transcript
jurahul added reviewers: mehdi_amini, jpienaar, rriddle.Jun 9 2020, 3:24 PM
rriddle added a comment.Jun 9 2020, 3:25 PM

Can you provide justification in your description as to why this change makes sense?

jurahul edited the summary of this revision. (Show Details)Jun 9 2020, 4:04 PM
Harbormaster failed remote builds in B59710: Diff 269690!Jun 9 2020, 4:04 PM
jurahul edited the summary of this revision. (Show Details)Jun 9 2020, 4:05 PM
jurahul updated this revision to Diff 269697.Jun 9 2020, 4:21 PM

Fix test

Harbormaster completed remote builds in B59715: Diff 269697.Jun 9 2020, 5:09 PM
rriddle accepted this revision.Jun 10 2020, 11:09 AM
rriddle added inline comments.
mlir/lib/Dialect/StandardOps/IR/Ops.cpp
2014

nit: Use a @ before the function names.

This revision is now accepted and ready to land.Jun 10 2020, 11:09 AM
jurahul updated this revision to Diff 269936.Jun 10 2020, 12:08 PM

Prefix function names with @

jurahul marked an inline comment as done.Jun 10 2020, 12:10 PM
Harbormaster completed remote builds in B59858: Diff 269936.Jun 10 2020, 12:48 PM
Closed by commit rGa0dd5e876f20: [MLIR] Print function name when ReturnOp verification fails (authored by jurahul, committed by rriddle). · Explain WhyJun 10 2020, 5:47 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
mlir/
lib/
Dialect/
StandardOps/
IR/
7 lines

Diff 269690

mlir/lib/Dialect/StandardOps/IR/Ops.cpp

Show First 20 Lines • Show All 2,004 Lines • ▼ Show 20 Lines
static LogicalResult verify(ReturnOp op) { static LogicalResult verify(ReturnOp op) {
auto function = cast<FuncOp>(op.getParentOp()); auto function = cast<FuncOp>(op.getParentOp());
// The operand number and types must match the function signature. // The operand number and types must match the function signature.
const auto &results = function.getType().getResults(); const auto &results = function.getType().getResults();
if (op.getNumOperands() != results.size()) if (op.getNumOperands() != results.size())
return op.emitOpError("has ") return op.emitOpError("has ")
<< op.getNumOperands() << op.getNumOperands() << " operands, but enclosing function ("
<< " operands, but enclosing function returns " << results.size(); << function.getName() << ") returns " << results.size();
rriddleUnsubmitted
Done
ReplyInline Actions

nit: Use a @ before the function names.

rriddle: nit: Use a @ before the function names.
for (unsigned i = 0, e = results.size(); i != e; ++i) for (unsigned i = 0, e = results.size(); i != e; ++i)
if (op.getOperand(i).getType() != results[i]) if (op.getOperand(i).getType() != results[i])
return op.emitError() return op.emitError()
<< "type of return operand " << i << " (" << "type of return operand " << i << " ("
<< op.getOperand(i).getType() << op.getOperand(i).getType()
<< ") doesn't match function result type (" << results[i] << ")"; << ") doesn't match function result type (" << results[i] << ")"
<< " in function " << function.getName();
return success(); return success();
} }
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
// SelectOp // SelectOp
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
▲ Show 20 Lines • Show All 1,191 Lines • Show Last 20 Lines