[Moore][ImportVerilog] Add moore.fmt.string, support for $sformatf
#8993
Conversation
Scheremo
force-pushed
the
pr-sformatf
branch
3 times, most recently
from
3625cd5 to
c708732
Compare
Scheremo
changed the title
moore.fmt.string and support for $sformatfmoore.fmt.string, support for $sformatf
Scheremo
force-pushed
the
pr-sformatf
branch
6 times, most recently
from
0dd3e2c to
8a84e67
Compare
This PR adds support for `$sformatf` by adding the `moore.fmt.string` operator, which converts a `StringType` to a `FormatStringType`. `moore.fmt.literal` cannot be used to convert LValue Strings to `FormatStringType` instances. This is required to apply any formatting, i.e. using moore.fmt.concat or resolving the `%s` format specifier with non-constant values. To bridge this gap, I added `moore.string_to_fstring`, which is virtually identical to `moore.fmt.literal` but allows `StringType` arguments. ~I think it is not too useful to keep both `moore.fmt.string` and `moore.fmt.literal` since analyzing whether the argument is constant seems very straightforward, but maybe I am missing something.~ Next issue is that `$sformat` must return a `StringType` according to 1800-2023 Section 21.3.3. In Moore, the output of a formatting operation is modelled as a `FormatString`, so I also added an Op for converting from `FormatString` to `StringType`, `moore.fstring_to_string`. To further follow the semantics of `$sformat`, which writes the formatted string to a string passed as an argument, I used a `BlockingAssignOp`. This PR also integrates support for lowering `$sformatf`/`$sformat`. Since there is already a framework in place for string formatting, I lowered directly into that rather than adding a new builtin for `$sformatf`.
Scheremo
force-pushed
the
pr-sformatf
branch
from
6578fe1 to
650481b
Compare
| // Use the first argument as the output location | ||
| auto *targetExpr = call->arguments().front(); | ||
| if (auto *sym = targetExpr->getSymbolReference() | ||
| ? targetExpr->getSymbolReference() | ||
| ->as_if<slang::ast::ValueSymbol>() | ||
| : nullptr) { | ||
| if (auto reference = context.valueSymbols.lookup(sym); | ||
| reference && isa<moore::RefType>(reference.getType())) { | ||
| auto targetTy = context.convertType(*sym->getDeclaredType()); | ||
| auto convertedValue = builder.createOrFold<moore::ConversionOp>( | ||
| loc, targetTy, strValue); | ||
| moore::BlockingAssignOp::create(builder, loc, reference, | ||
| convertedValue); | ||
| return success(); | ||
| } |
There was a problem hiding this comment.
You should be able to call context.convertLvalueExpression(call->arguments().front()) to get the first argument lowered to and lvalue as if it were the LHS of an assignment. You can then feed that to BlockingAssignOp as the first operand. For the RHS, you can use materializeConversion to map to the inner type of the LHS reference type.
There was a problem hiding this comment.
I did try this before, but context.convertLvalueExpression(call->arguments().front()) actually crashes out, as it tries to return the RValue for some reason.
There was a problem hiding this comment.
Okay, had a closer look at the AST generated by Slang:
"body": {
"kind": "List",
"list": [
{
"kind": "VariableDeclaration",
"symbol": "4865161440032 logicVector"
},
{
"kind": "VariableDeclaration",
"symbol": "4865161440176 test"
},
{
"kind": "ExpressionStatement",
"expr": {
"kind": "Call",
"type": "void",
"subroutine": "$sformat",
"arguments": [
{
"kind": "Assignment",
"type": "string",
"left": {
"kind": "NamedValue",
"type": "string",
"symbol": "4865161439712 outputString"
},
"right": {
"kind": "EmptyArgument",
"type": "string"
},
"isNonBlocking": false
},
{
"kind": "StringLiteral",
"type": "bit[39:0]",
"literal": "%s %s"
},
{
"kind": "NamedValue",
"type": "string",
"symbol": "4865161439360 testStr"
},
{
"kind": "NamedValue",
"type": "string",
"symbol": "4865161439536 otherString"
}
]
}
},
The first argument is indeed an AssignmentExpression but its RHS does not make sense - it's an "EmptyArgument", and the current ExprVisitor rightfully does trip on an "EmptyArgument".
The two/three ways I see to handle this, is keep the special case this PR uses for $sformat as the AssignmentExpression is essentially broken, or we change the existing AssignmentExpression visitor, or we add a new AssignmentExpression visitor that allows "preconverted" RHS. I think the latter versions are worse, because this looks like a very special special case - WDYT?
There was a problem hiding this comment.
Figured out my favourite way to do this:
First cast the first argument as a AssignmentExpression, then only convertLValueExpression the left of the AssignmentExpression, throw away the right, and plug in the newly formatted string instead. Much nicer than it was before!
There was a problem hiding this comment.
Fantastic! I remember Slang using this pattern for instance port connections as well: https://github.com/llvm/circt/blob/main/lib/Conversion/ImportVerilog/Structure.cpp#L335-L338
fabianschuiki
left a comment
fabianschuiki
left a comment
There was a problem hiding this comment.
LGTM! 🚀 Thanks for implementing this!
| // The Slang AST produces a `AssignmentExpression` for the first | ||
| // argument; the RHS of this expression is invalid though | ||
| // (`EmptyArgument`), so we only use the LHS of the | ||
| // `AssignmentExpression` and plug in the formatted string for the RHS. | ||
| if (auto assignExpr = | ||
| lhsExpr->as_if<slang::ast::AssignmentExpression>()) { | ||
| auto lhs = context.convertLvalueExpression(assignExpr->left()); | ||
| if (!lhs) | ||
| return failure(); | ||
|
|
||
| auto convertedValue = context.materializeConversion( | ||
| cast<moore::RefType>(lhs.getType()).getNestedType(), strValue, | ||
| false, loc); | ||
| moore::BlockingAssignOp::create(builder, loc, lhs, convertedValue); | ||
| return success(); | ||
| } else { | ||
| return failure(); | ||
| } |
3 participants
This PR adds support for
$sformatfby adding themoore.fmt.stringoperator, which converts aStringTypeto aFormatStringType.moore.fmt.literalcannot be used to convert LValue Strings toFormatStringTypeinstances. This is required to apply any formatting, i.e. using moore.fmt.concat or resolving the%sformat specifier with non-constant values.To bridge this gap, I added
moore.string_to_fstring, which is virtually identical tomoore.fmt.literalbut allowsStringTypearguments.I think it is not too useful to keep bothmoore.fmt.stringandmoore.fmt.literalsince analyzing whether the argument is constant seems very straightforward, but maybe I am missing something.Next issue is that
$sformatmust return aStringTypeaccording to 1800-2023 Section 21.3.3. In Moore, the output of a formatting operation is modelled as aFormatString, so I also added an Op for converting fromFormatStringtoStringType,moore.fstring_to_string. To further follow the semantics of$sformat, which writes the formatted string to a string passed as an argument, I used aBlockingAssignOp.This PR also integrates support for lowering
$sformatf/$sformat. Since there is already a framework in place for string formatting, I lowered directly into that rather than adding a new builtin for$sformatf.