Skip to content

Commit

Permalink
[MLIR] Fix block label parsing bug
Browse files Browse the repository at this point in the history
Fix bug in `Block` label parsing:
#54343

The `parseOptionalBlockArgList` method was doing the wrong thing
(contrary to its doc comment) and its calling context was also
incorrect. This led to a parse failure for something like "^bb0()".

Fixes #54343

Differential Revision: https://reviews.llvm.org/D121503
  • Loading branch information
bondhugula committed Mar 12, 2022
1 parent 5ac257d commit aea31f6
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 8 deletions.
14 changes: 7 additions & 7 deletions mlir/lib/Parser/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1958,11 +1958,9 @@ ParseResult OperationParser::parseBlock(Block *&block) {
return emitError(nameLoc, "redefinition of block '") << name << "'";

// If an argument list is present, parse it.
if (consumeIf(Token::l_paren)) {
if (parseOptionalBlockArgList(block) ||
parseToken(Token::r_paren, "expected ')' to end argument list"))
if (getToken().is(Token::l_paren))
if (parseOptionalBlockArgList(block))
return failure();
}

if (parseToken(Token::colon, "expected ':' after block name"))
return failure();
Expand Down Expand Up @@ -2025,9 +2023,11 @@ Block *OperationParser::defineBlockNamed(StringRef name, SMLoc loc,
return blockAndLoc.block;
}

/// Parse a (possibly empty) list of SSA operands with types as block arguments.
/// Parse a (possibly empty) list of SSA operands with types as block arguments
/// enclosed in parentheses.
///
/// ssa-id-and-type-list ::= ssa-id-and-type (`,` ssa-id-and-type)*
/// value-id-and-type-list ::= value-id-and-type (`,` ssa-id-and-type)*
/// block-arg-list ::= `(` value-id-and-type-list? `)`
///
ParseResult OperationParser::parseOptionalBlockArgList(Block *owner) {
if (getToken().is(Token::r_brace))
Expand All @@ -2038,7 +2038,7 @@ ParseResult OperationParser::parseOptionalBlockArgList(Block *owner) {
bool definingExistingArgs = owner->getNumArguments() != 0;
unsigned nextArgument = 0;

return parseCommaSeparatedList([&]() -> ParseResult {
return parseCommaSeparatedList(Delimiter::Paren, [&]() -> ParseResult {
return parseSSADefOrUseAndType(
[&](SSAUseInfo useInfo, Type type) -> ParseResult {
BlockArgument arg;
Expand Down
8 changes: 7 additions & 1 deletion mlir/test/IR/invalid.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func @no_terminator() { // expected-error {{empty block: expect at least a ter
// -----

func @block_no_rparen() {
^bb42 (%bb42 : i32: // expected-error {{expected ')' to end argument list}}
^bb42 (%bb42 : i32: // expected-error {{expected ')'}}
return
}

Expand Down Expand Up @@ -188,6 +188,12 @@ func @no_terminator() {
%y = arith.constant 1 : i32 // expected-error {{block with no terminator}}
}

// -----

func @no_block_arg_enclosing_parens() {
^bb %x: i32 : // expected-error {{expected ':' after block name}}
return
}

// -----

Expand Down
6 changes: 6 additions & 0 deletions mlir/test/IR/parser.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,12 @@ func @simpleCFGUsingBBArgs(i32, i64) {
// CHECK: }
}

// CHECK-LABEL: func @block_label_empty_list
func @block_label_empty_list() {
^bb0():
return
}

// CHECK-LABEL: func @multiblock() {
func @multiblock() {
return // CHECK: return
Expand Down

0 comments on commit aea31f6

Please sign in to comment.