Skip to content

Commit 09f0bbe

Browse files
authored
[Moore] Fix missing RValue conversion for struct_create (#8988)
This PR fixes a bug where RValue Conversion is not called when a struct is created from immediate values. Per IEEE 1800-2023 Section 10.9.2 "Structure assignment patterns", the following assignment pattern is valid: ``` structType st = '{member: value, memberTwo: valueTwo, ...}; ``` In ImportVerilog, this pattern is recognized and mapped to a `StructCreateOp` operator. Until now the RValue expressions for value and valueTwo were converted without a target type. This leads to a failure to convert e.g. between ArrayType instances and IntType instances. This PR fixes this by passing the structType member types to the convertRValueExpression function to have them be cast explicitly.
1 parent 8d47bf4 commit 09f0bbe

File tree

2 files changed

+145
-25
lines changed

2 files changed

+145
-25
lines changed

lib/Conversion/ImportVerilog/Expressions.cpp

Lines changed: 128 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1024,55 +1024,158 @@ struct RvalueExprVisitor : public ExprVisitor {
10241024
builder, loc, builder.getF64FloatAttr(expr.getValue()));
10251025
}
10261026

1027+
/// Helper function to convert RValues at creation of a new Struct, Array or
1028+
/// Int.
1029+
FailureOr<SmallVector<Value>>
1030+
convertElements(const slang::ast::AssignmentPatternExpressionBase &expr,
1031+
std::variant<Type, ArrayRef<Type>> expectedTypes,
1032+
unsigned replCount) {
1033+
const auto &elts = expr.elements();
1034+
const size_t elementCount = elts.size();
1035+
1036+
// Inspect the variant.
1037+
const bool hasBroadcast =
1038+
std::holds_alternative<Type>(expectedTypes) &&
1039+
static_cast<bool>(std::get<Type>(expectedTypes)); // non-null Type
1040+
1041+
const bool hasPerElem =
1042+
std::holds_alternative<ArrayRef<Type>>(expectedTypes) &&
1043+
!std::get<ArrayRef<Type>>(expectedTypes).empty();
1044+
1045+
// If per-element types are provided, enforce arity.
1046+
if (hasPerElem) {
1047+
auto types = std::get<ArrayRef<Type>>(expectedTypes);
1048+
if (types.size() != elementCount) {
1049+
mlir::emitError(loc)
1050+
<< "assignment pattern arity mismatch: expected " << types.size()
1051+
<< " elements, got " << elementCount;
1052+
return failure();
1053+
}
1054+
}
1055+
1056+
SmallVector<Value> converted;
1057+
converted.reserve(elementCount * std::max(1u, replCount));
1058+
1059+
// Convert each element heuristically, no type is expected
1060+
if (!hasBroadcast && !hasPerElem) {
1061+
// No expected type info.
1062+
for (const auto *elementExpr : elts) {
1063+
Value v = context.convertRvalueExpression(*elementExpr);
1064+
if (!v)
1065+
return failure();
1066+
converted.push_back(v);
1067+
}
1068+
} else if (hasBroadcast) {
1069+
// Same expected type for all elements.
1070+
Type want = std::get<Type>(expectedTypes);
1071+
for (const auto *elementExpr : elts) {
1072+
Value v = want ? context.convertRvalueExpression(*elementExpr, want)
1073+
: context.convertRvalueExpression(*elementExpr);
1074+
if (!v)
1075+
return failure();
1076+
converted.push_back(v);
1077+
}
1078+
} else { // hasPerElem, individual type is expected for each element
1079+
auto types = std::get<ArrayRef<Type>>(expectedTypes);
1080+
for (size_t i = 0; i < elementCount; ++i) {
1081+
Type want = types[i];
1082+
const auto *elementExpr = elts[i];
1083+
Value v = want ? context.convertRvalueExpression(*elementExpr, want)
1084+
: context.convertRvalueExpression(*elementExpr);
1085+
if (!v)
1086+
return failure();
1087+
converted.push_back(v);
1088+
}
1089+
}
1090+
1091+
for (unsigned i = 1; i < replCount; ++i)
1092+
converted.append(converted.begin(), converted.begin() + elementCount);
1093+
1094+
return converted;
1095+
}
1096+
10271097
/// Handle assignment patterns.
10281098
Value visitAssignmentPattern(
10291099
const slang::ast::AssignmentPatternExpressionBase &expr,
10301100
unsigned replCount = 1) {
10311101
auto type = context.convertType(*expr.type);
1032-
1033-
// Convert the individual elements first.
1034-
auto elementCount = expr.elements().size();
1035-
SmallVector<Value> elements;
1036-
elements.reserve(replCount * elementCount);
1037-
for (auto elementExpr : expr.elements()) {
1038-
auto value = context.convertRvalueExpression(*elementExpr);
1039-
if (!value)
1040-
return {};
1041-
elements.push_back(value);
1042-
}
1043-
for (unsigned replIdx = 1; replIdx < replCount; ++replIdx)
1044-
for (unsigned elementIdx = 0; elementIdx < elementCount; ++elementIdx)
1045-
elements.push_back(elements[elementIdx]);
1102+
const auto &elts = expr.elements();
10461103

10471104
// Handle integers.
10481105
if (auto intType = dyn_cast<moore::IntType>(type)) {
1049-
assert(intType.getWidth() == elements.size());
1050-
std::reverse(elements.begin(), elements.end());
1051-
return moore::ConcatOp::create(builder, loc, intType, elements);
1106+
auto elements = convertElements(expr, {}, replCount);
1107+
1108+
if (failed(elements))
1109+
return {};
1110+
1111+
assert(intType.getWidth() == elements->size());
1112+
std::reverse(elements->begin(), elements->end());
1113+
return moore::ConcatOp::create(builder, loc, intType, *elements);
10521114
}
10531115

10541116
// Handle packed structs.
10551117
if (auto structType = dyn_cast<moore::StructType>(type)) {
1056-
assert(structType.getMembers().size() == elements.size());
1057-
return moore::StructCreateOp::create(builder, loc, structType, elements);
1118+
SmallVector<Type> expectedTy;
1119+
expectedTy.reserve(structType.getMembers().size());
1120+
for (auto member : structType.getMembers())
1121+
expectedTy.push_back(member.type);
1122+
1123+
FailureOr<SmallVector<Value>> elements;
1124+
if (expectedTy.size() == elts.size())
1125+
elements = convertElements(expr, expectedTy, replCount);
1126+
else
1127+
elements = convertElements(expr, {}, replCount);
1128+
1129+
if (failed(elements))
1130+
return {};
1131+
1132+
assert(structType.getMembers().size() == elements->size());
1133+
return moore::StructCreateOp::create(builder, loc, structType, *elements);
10581134
}
10591135

10601136
// Handle unpacked structs.
10611137
if (auto structType = dyn_cast<moore::UnpackedStructType>(type)) {
1062-
assert(structType.getMembers().size() == elements.size());
1063-
return moore::StructCreateOp::create(builder, loc, structType, elements);
1138+
SmallVector<Type> expectedTy;
1139+
expectedTy.reserve(structType.getMembers().size());
1140+
for (auto member : structType.getMembers())
1141+
expectedTy.push_back(member.type);
1142+
1143+
FailureOr<SmallVector<Value>> elements;
1144+
if (expectedTy.size() == elts.size())
1145+
elements = convertElements(expr, expectedTy, replCount);
1146+
else
1147+
elements = convertElements(expr, {}, replCount);
1148+
1149+
if (failed(elements))
1150+
return {};
1151+
1152+
assert(structType.getMembers().size() == elements->size());
1153+
1154+
return moore::StructCreateOp::create(builder, loc, structType, *elements);
10641155
}
10651156

10661157
// Handle packed arrays.
10671158
if (auto arrayType = dyn_cast<moore::ArrayType>(type)) {
1068-
assert(arrayType.getSize() == elements.size());
1069-
return moore::ArrayCreateOp::create(builder, loc, arrayType, elements);
1159+
auto elements =
1160+
convertElements(expr, arrayType.getElementType(), replCount);
1161+
1162+
if (failed(elements))
1163+
return {};
1164+
1165+
assert(arrayType.getSize() == elements->size());
1166+
return moore::ArrayCreateOp::create(builder, loc, arrayType, *elements);
10701167
}
10711168

10721169
// Handle unpacked arrays.
10731170
if (auto arrayType = dyn_cast<moore::UnpackedArrayType>(type)) {
1074-
assert(arrayType.getSize() == elements.size());
1075-
return moore::ArrayCreateOp::create(builder, loc, arrayType, elements);
1171+
auto elements =
1172+
convertElements(expr, arrayType.getElementType(), replCount);
1173+
1174+
if (failed(elements))
1175+
return {};
1176+
1177+
assert(arrayType.getSize() == elements->size());
1178+
return moore::ArrayCreateOp::create(builder, loc, arrayType, *elements);
10761179
}
10771180

10781181
mlir::emitError(loc) << "unsupported assignment pattern with type " << type;

test/Conversion/ImportVerilog/basic.sv

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3196,3 +3196,20 @@ function void TimeFormat();
31963196
// CHECK-NEXT: moore.builtin.display [[CONCAT3]]
31973197
$display("%4t", $time());
31983198
endfunction
3199+
3200+
// CHECK-LABEL: func.func private @StructCreateConversion(
3201+
// CHECK-SAME: [[ARRAY:%.+]]: !moore.array<8 x l8>
3202+
// CHECK-SAME: [[IMM:%.+]]: !moore.l64
3203+
function void StructCreateConversion (logic [7:0][7:0] array, logic [63:0] immediate);
3204+
3205+
typedef struct packed {
3206+
logic [63:0] structField;
3207+
} testStruct;
3208+
3209+
// CHECK: [[TS:%.+]] = moore.struct_create [[IMM]] : !moore.l64 -> struct<{structField: l64}>
3210+
testStruct ts = '{structField: immediate};
3211+
// CHECK: [[CAST:%.+]] = moore.packed_to_sbv [[ARRAY]] : array<8 x l8>
3212+
// CHECK-NEXT: [[TS2:%.+]] = moore.struct_create [[CAST]] : !moore.l64 -> struct<{structField: l64}>
3213+
testStruct ts2 = '{structField: array};
3214+
3215+
endfunction

0 commit comments

Comments
 (0)