Skip to content
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.

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

[FIRRTL] Add list concatenation parser support. #7512

Merged
merged 1 commit into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
49 changes: 49 additions & 0 deletions lib/Dialect/FIRRTL/Import/FIRParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1720,6 +1720,7 @@ struct FIRStmtParser : public FIRParser {
ParseResult parsePrimExp(Value &result);
ParseResult parseIntegerLiteralExp(Value &result);
ParseResult parseListExp(Value &result);
ParseResult parseListConcatExp(Value &result);

std::optional<ParseResult> parseExpWithLeadingKeyword(FIRToken keyword);

Expand Down Expand Up @@ -1978,6 +1979,16 @@ ParseResult FIRStmtParser::parseExpImpl(Value &result, const Twine &message,
return failure();
break;
}

case FIRToken::lp_list_concat: {
if (isLeadingStmt)
return emitError("unexpected list_create() as start of statement");
if (requireFeature(nextFIRVersion, "List concat") ||
parseListConcatExp(result))
return failure();
break;
}

case FIRToken::lp_path:
if (isLeadingStmt)
return emitError("unexpected path() as start of statement");
Expand Down Expand Up @@ -2455,6 +2466,44 @@ ParseResult FIRStmtParser::parseListExp(Value &result) {
return success();
}

/// list-concat-exp ::= 'list_concat' '(' exp* ')'
ParseResult FIRStmtParser::parseListConcatExp(Value &result) {
consumeToken(FIRToken::lp_list_concat);

auto loc = getToken().getLoc();
ListType type;
SmallVector<Value, 3> operands;
if (parseListUntil(FIRToken::r_paren, [&]() -> ParseResult {
Value operand;
locationProcessor.setLoc(loc);
if (parseExp(operand, "expected expression in List concat expression"))
return failure();

if (!type_isa<ListType>(operand.getType()))
return emitError(loc, "unexpected expression of type ")
<< operand.getType() << " in List concat expression";

if (!type)
type = type_cast<ListType>(operand.getType());

if (operand.getType() != type)
return emitError(loc, "unexpected expression of type ")
<< operand.getType() << " in List concat expression of type "
<< type;

operands.push_back(operand);
return success();
}))
return failure();

if (operands.empty())
return emitError(loc, "need at least one List to concatenate");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is legal syntactically according to the spec, but for this specific operation, the IR requires at least one operand. It makes the parser simpler to implement by requiring at least one operand and inferring the type from the types of the operands, but if it would be preferable to not require this during parsing (and instead let the op's verifier enforce it), I can change that.


locationProcessor.setLoc(loc);
result = builder.create<ListConcatOp>(type, operands);
return success();
}

/// The .fir grammar has the annoying property where:
/// 1) some statements start with keywords
/// 2) some start with an expression
Expand Down
1 change: 1 addition & 0 deletions lib/Dialect/FIRRTL/Import/FIRTokenKinds.def
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ TOK_LPKEYWORD(assume)
TOK_LPKEYWORD(cover)

TOK_LPKEYWORD(path)
TOK_LPKEYWORD(list_concat)

TOK_LPKEYWORD(force)
TOK_LPKEYWORD(force_initial)
Expand Down
14 changes: 14 additions & 0 deletions test/Dialect/FIRRTL/parse-basic.fir
Original file line number Diff line number Diff line change
Expand Up @@ -1670,6 +1670,20 @@ circuit IntegerArithmetic :
; CHECK: firrtl.propassign %e, [[E]]
propassign e, integer_shr(a, b)

;// -----
FIRRTL version 4.0.0

; CHECK-LABEL: firrtl.circuit "PropertyListOps"
circuit PropertyListOps :
public module PropertyListOps :
input a : List<Integer>
input b : List<Integer>
output c : List<Integer>

; CHECK: [[C:%.+]] = firrtl.list.concat %a, %b
; CHECK: firrtl.propassign %c, [[C]]
propassign c, list_concat(a, b)

;// -----
FIRRTL version 3.1.0

Expand Down
36 changes: 36 additions & 0 deletions test/Dialect/FIRRTL/parse-errors.fir
Original file line number Diff line number Diff line change
Expand Up @@ -1026,6 +1026,42 @@ circuit Top:
; expected-error @below {{Integer arithmetic expressions are a FIRRTL 4.0.0+ feature, but the specified FIRRTL version was 3.1.0}}
propassign c, integer_shr(a, b)

;// -----
FIRRTL version 4.0.0

; CHECK-LABEL: firrtl.circuit "Top"
circuit Top :
public module Top :
input a : Integer
output b : List<Integer>

; expected-error @below {{unexpected expression of type '!firrtl.integer' in List concat expression}}
propassign b, list_concat(a)

;// -----
FIRRTL version 4.0.0

; CHECK-LABEL: firrtl.circuit "Top"
circuit Top :
public module Top :
input a : List<Integer>
input b : List<String>
output c : List<Integer>

; expected-error @below {{unexpected expression of type '!firrtl.list<string>' in List concat expression of type '!firrtl.list<integer>'}}
propassign c, list_concat(a, b)

;// -----
FIRRTL version 4.0.0

; CHECK-LABEL: firrtl.circuit "Top"
circuit Top :
public module Top :
output c : List<Integer>

; expected-error @below {{need at least one List to concatenate}}
propassign c, list_concat()

;// -----
FIRRTL version 3.3.0

Expand Down
Loading