From 81678d3e7d6955a04702d6cfcc70707602033d87 Mon Sep 17 00:00:00 2001 From: Will Dietz Date: Mon, 28 Aug 2023 16:10:19 -0500 Subject: [PATCH] [FIRRTL] Map: MapCreateOp, parsing support. (#5962) Add operation for creating Map's, add parsing support and tests. --- .../circt/Dialect/FIRRTL/FIRRTLExpressions.td | 18 ++++ include/circt/Dialect/FIRRTL/FIRRTLVisitors.h | 3 +- lib/Dialect/FIRRTL/FIRRTLOps.cpp | 67 +++++++++++++ lib/Dialect/FIRRTL/Import/FIRLexer.cpp | 5 +- lib/Dialect/FIRRTL/Import/FIRParser.cpp | 98 +++++++++++++++++-- lib/Dialect/FIRRTL/Import/FIRTokenKinds.def | 2 + test/Dialect/FIRRTL/errors.mlir | 42 ++++++++ test/Dialect/FIRRTL/parse-basic.fir | 37 +++++++ test/Dialect/FIRRTL/parse-errors.fir | 58 +++++++++++ test/Dialect/FIRRTL/test.mlir | 11 +++ 10 files changed, 332 insertions(+), 9 deletions(-) diff --git a/include/circt/Dialect/FIRRTL/FIRRTLExpressions.td b/include/circt/Dialect/FIRRTL/FIRRTLExpressions.td index 5d134fed78..5393f0ccea 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLExpressions.td +++ b/include/circt/Dialect/FIRRTL/FIRRTLExpressions.td @@ -1170,6 +1170,24 @@ def ListCreateOp : FIRRTLOp<"list.create", [Pure, SameTypeOperands]> { let hasVerifier = 1; } +def MapCreateOp : FIRRTLOp<"map.create", [Pure, SameVariadicOperandSize]> { + let summary = "Produce a map value"; + let description = [{ + Produces a value of map type containing the provided key/value elements. + + Example: + ```mlir + %4 = firrtl.map.create (%0 -> %1, %2 -> %3) : !firrtl.map + ``` + }]; + + let arguments = (ins Variadic:$keys, Variadic:$values); + let results = (outs MapType:$result); + + let hasCustomAssemblyFormat = 1; + let hasVerifier = 1; +} + def BoolConstantOp : FIRRTLOp<"bool", [Pure, ConstantLike]> { let summary = "Produce a constant boolean value"; let description = [{ diff --git a/include/circt/Dialect/FIRRTL/FIRRTLVisitors.h b/include/circt/Dialect/FIRRTL/FIRRTLVisitors.h index d572c34405..926f455be8 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLVisitors.h +++ b/include/circt/Dialect/FIRRTL/FIRRTLVisitors.h @@ -62,7 +62,7 @@ public: UninferredResetCastOp, ConstCastOp, RefCastOp, mlir::UnrealizedConversionCastOp, // Property expressions. - StringConstantOp, FIntegerConstantOp, BoolConstantOp>( + StringConstantOp, FIntegerConstantOp, BoolConstantOp, MapCreateOp>( [&](auto expr) -> ResultType { return thisCast->visitExpr(expr, args...); }) @@ -205,6 +205,7 @@ public: HANDLE(StringConstantOp, Unhandled); HANDLE(FIntegerConstantOp, Unhandled); HANDLE(BoolConstantOp, Unhandled); + HANDLE(MapCreateOp, Unhandled); #undef HANDLE }; diff --git a/lib/Dialect/FIRRTL/FIRRTLOps.cpp b/lib/Dialect/FIRRTL/FIRRTLOps.cpp index 8bbf794d34..c8c35f6765 100644 --- a/lib/Dialect/FIRRTL/FIRRTLOps.cpp +++ b/lib/Dialect/FIRRTL/FIRRTLOps.cpp @@ -3603,6 +3603,73 @@ LogicalResult ListCreateOp::verify() { return success(); } +ParseResult MapCreateOp::parse(OpAsmParser &parser, OperationState &result) { + llvm::SmallVector keys; + llvm::SmallVector values; + + MapType type; + + auto parsePair = [&]() { + OpAsmParser::UnresolvedOperand key, value; + if (parser.parseOperand(key) || parser.parseArrow() || + parser.parseOperand(value)) + return failure(); + keys.push_back(key); + values.push_back(value); + return success(); + }; + if (parser.parseCommaSeparatedList(OpAsmParser::Delimiter::OptionalParen, + parsePair) || + parser.parseOptionalAttrDict(result.attributes) || + parser.parseColonType(type)) + return failure(); + result.addTypes(type); + + if (parser.resolveOperands(keys, type.getKeyType(), result.operands) || + parser.resolveOperands(values, type.getValueType(), result.operands)) + return failure(); + + return success(); +} + +void MapCreateOp::print(OpAsmPrinter &p) { + p << " "; + if (!getKeys().empty()) { + p << "("; + llvm::interleaveComma(llvm::zip_equal(getKeys(), getValues()), p, + [&](auto pair) { + auto &[key, value] = pair; + p.printOperand(key); + p << " -> "; + p.printOperand(value); + }); + p << ")"; + } + p.printOptionalAttrDict((*this)->getAttrs()); + p << " : " << getType(); +} + +LogicalResult MapCreateOp::verify() { + if (getKeys().empty()) + return success(); + + if (!llvm::all_equal(getKeys().getTypes())) + return emitOpError("has keys that are not all the same type"); + if (!llvm::all_equal(getValues().getTypes())) + return emitOpError("has values that are not all the same type"); + + if (getKeys().front().getType() != getType().getKeyType()) + return emitOpError("has unexpected key type ") + << getKeys().front().getType() << " instead of map key type " + << getType().getKeyType(); + + if (getValues().front().getType() != getType().getValueType()) + return emitOpError("has unexpected value type ") + << getValues().front().getType() << " instead of map value type " + << getType().getValueType(); + return success(); +} + LogicalResult BundleCreateOp::verify() { BundleType resultType = getType(); if (resultType.getNumElements() != getFields().size()) diff --git a/lib/Dialect/FIRRTL/Import/FIRLexer.cpp b/lib/Dialect/FIRRTL/Import/FIRLexer.cpp index c8495f71b8..c2285aaf2e 100644 --- a/lib/Dialect/FIRRTL/Import/FIRLexer.cpp +++ b/lib/Dialect/FIRRTL/Import/FIRLexer.cpp @@ -298,8 +298,11 @@ FIRToken FIRLexer::lexTokenImpl() { case '\'': return lexString(tokStart, /*isVerbatim=*/true); - case '+': case '-': + if (*curPtr == '>') + return ++curPtr, formToken(FIRToken::minus_greater, tokStart); + return lexNumber(tokStart); + case '+': case '0': case '1': case '2': diff --git a/lib/Dialect/FIRRTL/Import/FIRParser.cpp b/lib/Dialect/FIRRTL/Import/FIRParser.cpp index 9c1a100829..5ac810654c 100644 --- a/lib/Dialect/FIRRTL/Import/FIRParser.cpp +++ b/lib/Dialect/FIRRTL/Import/FIRParser.cpp @@ -257,7 +257,10 @@ struct FIRParser { const Twine &message); ParseResult parseEnumType(FIRRTLType &result); ParseResult parseListType(FIRRTLType &result); + ParseResult parseMapType(FIRRTLType &result); ParseResult parseType(FIRRTLType &result, const Twine &message); + // Parse a property type specifically. + ParseResult parsePropertyType(PropertyType &result, const Twine &message); ParseResult parseOptionalRUW(RUWAttr &result); @@ -795,25 +798,47 @@ ParseResult FIRParser::parseEnumType(FIRRTLType &result) { return success(); } +ParseResult FIRParser::parsePropertyType(PropertyType &result, + const Twine &message) { + FIRRTLType type; + if (parseType(type, message)) + return failure(); + auto prop = type_dyn_cast(type); + if (!prop) + return emitError("expected property type"); + result = prop; + return success(); +} + /// list-type ::= 'List' '<' type '>' ParseResult FIRParser::parseListType(FIRRTLType &result) { - auto loc = getToken().getLoc(); consumeToken(FIRToken::kw_List); - FIRRTLType type; + PropertyType elementType; if (parseToken(FIRToken::less, "expected '<' in List type") || - parseType(type, "expected List element type") || + parsePropertyType(elementType, "expected List element type") || parseToken(FIRToken::greater, "expected '>' in List type")) return failure(); - auto elementType = type_dyn_cast(type); - if (!elementType) - return emitError(loc, "expected property type"); - result = ListType::get(getContext(), elementType); return success(); } +/// map-type ::= 'Map' '<' type ',' type '>' +ParseResult FIRParser::parseMapType(FIRRTLType &result) { + consumeToken(FIRToken::kw_Map); + + PropertyType key, value; + if (parseToken(FIRToken::less, "expected '<' in Map type") || + parsePropertyType(key, "expected Map key type") || + parsePropertyType(value, "expected Map value type") || + parseToken(FIRToken::greater, "expected '>' in Map type")) + return failure(); + + result = MapType::get(getContext(), key, value); + return success(); +} + /// type ::= 'Clock' /// ::= 'Reset' /// ::= 'AsyncReset' @@ -827,6 +852,7 @@ ParseResult FIRParser::parseListType(FIRRTLType &result) { /// ::= 'const' type /// ::= 'String' /// ::= list-type +/// ::= map-type /// ::= id /// /// field: 'flip'? fieldId ':' type @@ -1040,6 +1066,10 @@ ParseResult FIRParser::parseType(FIRRTLType &result, const Twine &message) { if (requireFeature({3, 2, 0}, "Lists") || parseListType(result)) return failure(); break; + case FIRToken::kw_Map: + if (requireFeature({3, 2, 0}, "Maps") || parseMapType(result)) + return failure(); + break; } // Handle postfix vector sizes. @@ -1544,6 +1574,7 @@ private: ParseResult parsePrimExp(Value &result); ParseResult parseIntegerLiteralExp(Value &result); ParseResult parseListExp(Value &result); + ParseResult parseMapExp(Value &result); std::optional parseExpWithLeadingKeyword(FIRToken keyword); @@ -1847,6 +1878,15 @@ ParseResult FIRStmtParser::parseExpImpl(Value &result, const Twine &message, return failure(); break; } + case FIRToken::kw_Map: { + if (requireFeature({3, 2, 0}, "Maps")) + return failure(); + if (isLeadingStmt) + return emitError("unexpected Map<>() as start of statement"); + if (parseMapExp(result)) + return failure(); + break; + } case FIRToken::lp_path: if (isLeadingStmt) return emitError("unexpected path() as start of statement"); @@ -2295,6 +2335,50 @@ ParseResult FIRStmtParser::parseListExp(Value &result) { return success(); } +/// kv-pair ::= exp '->' exp +/// map-exp ::= map-type '(' ( kv-pair ( ',' kv-pair )* )? ')' +ParseResult FIRStmtParser::parseMapExp(Value &result) { + auto loc = getToken().getLoc(); + FIRRTLType type; + if (parseMapType(type)) + return failure(); + auto mapType = type_cast(type); + auto keyType = mapType.getKeyType(); + auto valueType = mapType.getValueType(); + + if (parseToken(FIRToken::l_paren, "expected '(' in Map expression")) + return failure(); + + SmallVector keys, values; + if (parseListUntil(FIRToken::r_paren, [&]() -> ParseResult { + Value key, value; + locationProcessor.setLoc(loc); + if (parseExp(key, "expected key expression in Map expression") || + parseToken(FIRToken::minus_greater, + "expected '->' in Map expression") || + parseExp(value, "expected value expression in Map expression")) + return failure(); + + if (key.getType() != keyType) + return emitError(loc, "unexpected expression of type ") + << key.getType() << " for key in Map expression, expected " + << keyType; + if (value.getType() != valueType) + return emitError(loc, "unexpected expression of type ") + << value.getType() << " for value in Map expression, expected " + << valueType; + + keys.push_back(key); + values.push_back(value); + return success(); + })) + return failure(); + + locationProcessor.setLoc(loc); + result = builder.create(mapType, keys, values); + return success(); +} + /// The .fir grammar has the annoying property where: /// 1) some statements start with keywords /// 2) some start with an expression diff --git a/lib/Dialect/FIRRTL/Import/FIRTokenKinds.def b/lib/Dialect/FIRRTL/Import/FIRTokenKinds.def index f2c8a469b5..82a0e85a15 100644 --- a/lib/Dialect/FIRRTL/Import/FIRTokenKinds.def +++ b/lib/Dialect/FIRRTL/Import/FIRTokenKinds.def @@ -75,6 +75,7 @@ TOK_PUNCTUATION(r_square, "]") TOK_PUNCTUATION(less, "<") TOK_PUNCTUATION(less_equal, "<=") TOK_PUNCTUATION(less_minus, "<-") +TOK_PUNCTUATION(minus_greater, "->") TOK_PUNCTUATION(greater, ">") TOK_PUNCTUATION(equal, "=") TOK_PUNCTUATION(equal_greater, "=>") @@ -92,6 +93,7 @@ TOK_KEYWORD(Fixed) TOK_KEYWORD(Inst) TOK_KEYWORD(Integer) TOK_KEYWORD(List) +TOK_KEYWORD(Map) TOK_KEYWORD(Path) TOK_KEYWORD(Probe) TOK_KEYWORD(RWProbe) diff --git a/test/Dialect/FIRRTL/errors.mlir b/test/Dialect/FIRRTL/errors.mlir index 0884b5df28..ec88ffe06a 100644 --- a/test/Dialect/FIRRTL/errors.mlir +++ b/test/Dialect/FIRRTL/errors.mlir @@ -1355,6 +1355,35 @@ firrtl.circuit "MixedList" { // ----- +// Reject map.create with mixed key types. +firrtl.circuit "MixedMapKeys" { + firrtl.module @MixedMapKeys( + in %s: !firrtl.string, + // expected-note @below {{prior use here}} + in %int: !firrtl.integer + ) { + // expected-error @below {{use of value '%int' expects different type than prior uses: '!firrtl.string' vs '!firrtl.integer'}} + firrtl.map.create (%s -> %int, %int -> %int) : !firrtl.map + } +} + +// ----- + +// Reject map.create with mixed value types. +firrtl.circuit "MixedMapValues" { + firrtl.module @MixedMapValues( + in %s1: !firrtl.string, + // expected-note @below {{prior use here}} + in %s2: !firrtl.string, + in %int: !firrtl.integer + ) { + // expected-error @below {{use of value '%s2' expects different type than prior uses: '!firrtl.integer' vs '!firrtl.string'}} + firrtl.map.create (%s1 -> %int, %s2 -> %s2) : !firrtl.map + } +} + +// ----- + // Reject list.create with elements of wrong type compared to result type. firrtl.circuit "ListCreateWrongType" { firrtl.module @ListCreateWrongType( @@ -1368,6 +1397,19 @@ firrtl.circuit "ListCreateWrongType" { // ----- +// Reject map.create with consistent but wrong key/value elements. +firrtl.circuit "MapCreateWrongType" { + firrtl.module @MapCreateWrongType( + // expected-note @below {{prior use here}} + in %int: !firrtl.integer + ) { + // expected-error @below {{use of value '%int' expects different type than prior uses: '!firrtl.string' vs '!firrtl.integer'}} + firrtl.map.create (%int -> %int) : !firrtl.map + } +} + +// ----- + // Reject list.create that doesn't create a list. firrtl.circuit "ListCreateNotList" { firrtl.module @ListCreateNotList() { diff --git a/test/Dialect/FIRRTL/parse-basic.fir b/test/Dialect/FIRRTL/parse-basic.fir index efbd0f36df..bd23a5b0e6 100644 --- a/test/Dialect/FIRRTL/parse-basic.fir +++ b/test/Dialect/FIRRTL/parse-basic.fir @@ -1776,3 +1776,40 @@ circuit BasicProps : ; CHECK-NEXT: firrtl.propassign %nested, %[[NESTED]] propassign nested, List>(List(), List(String("test")), List()) + ; CHECK-LABEL: module private @Map + module Map : + ; CHECK-SAME: in %list: !firrtl.list + input list : List + ; CHECK-SAME: out %str2str: !firrtl.map + ; CHECK-SAME: out %str2objs: !firrtl.map> + ; CHECK-SAME: out %int2list: !firrtl.map> + ; CHECK-SAME: out %empty: !firrtl.map + output str2str : Map + output str2objs : Map> + output int2list : Map> + output empty : Map + + ; CHECK-NEXT: %[[HELLO:.+]] = firrtl.string "hello" + ; CHECK-NEXT: %[[WORLD:.+]] = firrtl.string "world" + ; CHECK-NEXT: %[[STRINGS:.+]] = firrtl.map.create (%[[HELLO]] -> %[[WORLD]]) : !firrtl.map + ; CHECK-NEXT: firrtl.propassign %str2str, %[[STRINGS]] : !firrtl.map + propassign str2str, Map(String("hello") -> String("world")) + + ; CHECK-NEXT: %[[OBJ:.+]] = firrtl.object + ; CHECK-NEXT: %[[HELLO:.+]] = firrtl.string "hello" + ; CHECK-NEXT: %[[WORLD:.+]] = firrtl.string "world" + ; CHECK-NEXT: %[[OBJS:.+]] = firrtl.map.create (%[[HELLO]] -> %[[OBJ]], %[[WORLD]] -> %[[OBJ]]) : !firrtl.map> + ; CHECK-NEXT: firrtl.propassign %str2objs, %[[OBJS]] : !firrtl.map>(String("hello") -> obj, String("world") -> obj) + + ; CHECK-NEXT: %[[FIVE:.+]] = firrtl.integer 5 + ; CHECK-NEXT: %[[EMPTY:.+]] = firrtl.list.create : !firrtl.list + ; CHECK-NEXT: %[[TEN:.+]] = firrtl.integer 10 + ; CHECK-NEXT: %[[INTLISTMAP:.+]] = firrtl.map.create (%[[FIVE]] -> %[[EMPTY]], %[[TEN]] -> %list) : !firrtl.map> + ; CHECK-NEXT: firrtl.propassign %int2list, %[[INTLISTMAP]] + propassign int2list, Map>(Integer(5) -> List(), Integer(10) -> list) + + ; CHECK-NEXT: %[[EMPTY:.+]] = firrtl.map.create : !firrtl.map + ; CHECK-NEXT: firrtl.propassign %empty, %[[EMPTY]] + propassign empty, Map() diff --git a/test/Dialect/FIRRTL/parse-errors.fir b/test/Dialect/FIRRTL/parse-errors.fir index 928d14de35..632584b028 100644 --- a/test/Dialect/FIRRTL/parse-errors.fir +++ b/test/Dialect/FIRRTL/parse-errors.fir @@ -1021,3 +1021,61 @@ circuit Top: ; expected-error @below {{only hardware types can be 'const'}} output out : const Bool +;// ----- +; Maps not yet supported. +FIRRTL version 3.1.0 +circuit MapStrToInt: + module MapStrToInt: + ; expected-error @below {{Maps are a FIRRTL 3.2.0+ feature}} + output a : Map + +;// ----- +; Map with non-property key type. +FIRRTL version 3.2.0 +circuit MapUIntTo: + module MapUIntTo: + ; expected-error @below {{expected property type}} + output a : Map + +;// ----- +; Map with non-property value type. +FIRRTL version 3.2.0 +circuit MapToUInt: + module MapToUInt: + ; expected-error @below {{expected property type}} + output a : Map + +;// ----- +; Wrong type of keys in Map expression. +FIRRTL version 3.2.0 +circuit MapToString: + module MapToString: + output a : Map + ; expected-error @below {{unexpected expression of type '!firrtl.string' for key in Map expression, expected '!firrtl.integer'}} + propassign a, Map(String("hello") -> String("world")) + +;// ----- +; Wrong type of values in Map expression. +FIRRTL version 3.2.0 +circuit MapBoolTo: + module MapBoolTo: + output a : Map + ; expected-error @below {{unexpected expression of type '!firrtl.string' for value in Map expression, expected '!firrtl.integer'}} + propassign a, Map(Bool(true) -> String("world")) + + +;// ----- +; Map should not be leading a statement. +FIRRTL version 3.2.0 +circuit LeadingMap: + module LeadingMap: + ; expected-error @below {{unexpected Map<>() as start of statement}} + Map() + +;// ----- +; Map should not be const. +FIRRTL version 3.2.0 +circuit ConstMap: + module ConstMap: + ; expected-error @below {{only hardware types can be 'const'}} + output m: const Map diff --git a/test/Dialect/FIRRTL/test.mlir b/test/Dialect/FIRRTL/test.mlir index a4aae1fcbd..3c835aa47d 100644 --- a/test/Dialect/FIRRTL/test.mlir +++ b/test/Dialect/FIRRTL/test.mlir @@ -294,7 +294,18 @@ firrtl.module @ListTest(in %s1: !firrtl.string, // CHECK-LABEL: MapTest // CHECK-SAME: (in %in: !firrtl.map, out %out: !firrtl.map) firrtl.module @MapTest(in %in: !firrtl.map, out %out: !firrtl.map) { + // CHECK-NEXT: propassign %out, %in : !firrtl.map firrtl.propassign %out, %in : !firrtl.map + + // CHECK-NEXT: %[[EMPTY:.+]] = firrtl.map.create : !firrtl.map + %empty = firrtl.map.create : !firrtl.map + + // CHECK-NEXT: %[[HELLO:.+]] = firrtl.string "hello" + // CHECK-NEXT: %[[WORLD:.+]] = firrtl.string "world" + // CHECK-NEXT: %{{.+}} = firrtl.map.create (%in -> %[[HELLO]], %[[EMPTY]] -> %[[WORLD]]) : !firrtl.map, string> + %hello = firrtl.string "hello" + %world = firrtl.string "world" + %mapmap = firrtl.map.create (%in -> %hello, %empty -> %world) : !firrtl.map, string> } // CHECK-LABEL: PropertyNestedTest