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

[ImportVerilog] add unpacked array concatenation #8270

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

chenbo-again
Copy link
Contributor

@chenbo-again chenbo-again commented Feb 24, 2025

This PR is aimed to provide unpacked array concatenation.

// unpacked array concatenation
string s[] = { "hello", "sad", "world" };
// string concatenation
string = { "hello", "sad", "world" };

these two operation do not act the same way, and the result of concat is depends on the Lhs.

close #8173

@chenbo-again
Copy link
Contributor Author

This PR import UnpackedArrayConcatOp operation, This Operation is very similar to array_create operation, but can use unpacked array as operands.

Comment on lines 1075 to 1087
let summary = [{
This operation represents the SystemVerilog unpacked array concatenation,
See IEEE 1800-2017 §10.10 "Unpacked array concatenation".

- An item whose self-determined type is assignment compatible with the
element type of the target array shall represent a single element,
- An item whose self-determined type is an unpacked array whose
slowest-varying dimension's element type is assignment compatible
with the element type of the target array shall represent as many
elements as exist in that item, arranged in the same left-to-right order
as they would appear in the array item itself.
}];
let description = [{}];
Copy link
Member

Choose a reason for hiding this comment

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

nit: please exchange their content.

@@ -993,6 +1003,7 @@ module Expressions;
// CHECK: moore.extract_ref %vec_1 from 11 : <l32> -> <l3>
vec_1[15-:3] = y;

vec_s = { "hello", "sad", "world" };
Copy link
Member

Choose a reason for hiding this comment

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

If there isn't stringConcat, you have to remove it. Otherwise, you should add // CHECK: s to test it.

Comment on lines +442 to +445
def AnyIndexedUnpackedArrayType : MooreType<
Or<[UnpackedArrayType.predicate, OpenUnpackedArrayType.predicate, QueueType.predicate]>,
"unpacked array type except associative array type",
"moore::UnpackedType">;
Copy link
Member

Choose a reason for hiding this comment

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

I noticed that UnpackedArrayType, OpenUnpackedArrayType, and QueueType are all AnyIndexedUnpackedArrayType, but you just tested the OpenUnpackedArrayType, the others you also have to test.

Comment on lines +1192 to +1225
if (isAssignmentContextRhs) {
switch (expr.kind) {
case slang::ast::ExpressionKind::Concatenation: {
auto isUnpackedArray = [](Type arg) {
return isa<moore::UnpackedArrayType, moore::OpenUnpackedArrayType,
moore::QueueType>(arg);
};
if (!isUnpackedArray(requiredType))
return convertRvalueExpression(expr, requiredType);
SmallVector<Value> operands;

const auto &concatExpr =
static_cast<const slang::ast::ConcatenationExpression &>(expr);
Type elementType = getArrayElementsType(requiredType);

for (auto *operand : concatExpr.operands()) {
auto value = convertRvalueExpression(*operand);
if (!value)
continue;

if (!isUnpackedArray(value.getType()) && elementType != value.getType())
value = builder.create<moore::ConversionOp>(loc, elementType, value);

operands.push_back(value);
}
return builder.create<moore::UnpackedArrayConcatOp>(loc, requiredType,
operands);
}

default:
return convertRvalueExpression(expr, requiredType);
}
}

Copy link
Member

@hailongSun2000 hailongSun2000 Mar 6, 2025

Choose a reason for hiding this comment

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

You should move this code snippet to the proper place, like Value visit(const slang::ast::ConcatenationExpression &expr).

Copy link
Member

Choose a reason for hiding this comment

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

By the way, maybe you can check the following case(it's mentioned at SV IEEE Std 10.10.3):

typedef int T_QI[$];
T_QI jagged_array[$] = '{ {1}, T_QI'{2,3,4}, {5,6} };

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ImportVerilog] Crash on ordering-methods-reverse test
2 participants