[ImportVerilog] Fix value domain mismatch for logic/ternary ops

Fix an issue in the mapping of the logical `&&`, `||`, `->`, and `<->`
operators, where the left and right-hand side could have different value
domains in the AST (one `logic`, one `bit`).

Similarly, fix an issue with the `?:` ternary operator where the true
and false expressions could have different but cast-compatible types.
This commit is contained in:
Fabian Schuiki 2024-08-04 12:29:20 -07:00
parent c26d7705c3
commit 7df593d081
No known key found for this signature in database
GPG Key ID: C42F5825FC5275E6
2 changed files with 89 additions and 19 deletions

View File

@ -12,6 +12,7 @@
using namespace circt;
using namespace ImportVerilog;
using moore::Domain;
// NOLINTBEGIN(misc-no-recursion)
namespace {
@ -62,6 +63,18 @@ struct RvalueExprVisitor {
return {};
}
/// Helper function to convert a value to its "truthy" boolean value and
/// convert it to the given domain.
Value convertToBool(Value value, Domain domain) {
value = convertToBool(value);
if (!value)
return {};
auto type = moore::IntType::get(context.getContext(), 1, domain);
if (value.getType() == type)
return value;
return builder.create<moore::ConversionOp>(loc, type, value);
}
// Handle references to the left-hand side of a parent assignment.
Value visit(const slang::ast::LValueReferenceExpression &expr) {
assert(!context.lvalueStack.empty() && "parent assignments push lvalue");
@ -242,6 +255,12 @@ struct RvalueExprVisitor {
if (!rhs)
return {};
// Determine the domain of the result.
Domain domain = Domain::TwoValued;
if (expr.type->isFourState() || expr.left().type->isFourState() ||
expr.right().type->isFourState())
domain = Domain::FourValued;
using slang::ast::BinaryOperator;
switch (expr.op) {
case BinaryOperator::Add:
@ -322,35 +341,45 @@ struct RvalueExprVisitor {
// See IEEE 1800-2017 § 11.4.7 "Logical operators".
case BinaryOperator::LogicalAnd: {
// TODO: This should short-circuit. Put the RHS code into an scf.if.
lhs = convertToBool(lhs);
rhs = convertToBool(rhs);
if (!lhs || !rhs)
// TODO: This should short-circuit. Put the RHS code into a separate
// block.
lhs = convertToBool(lhs, domain);
if (!lhs)
return {};
rhs = convertToBool(rhs, domain);
if (!rhs)
return {};
return builder.create<moore::AndOp>(loc, lhs, rhs);
}
case BinaryOperator::LogicalOr: {
// TODO: This should short-circuit. Put the RHS code into an scf.if.
lhs = convertToBool(lhs);
rhs = convertToBool(rhs);
if (!lhs || !rhs)
// TODO: This should short-circuit. Put the RHS code into a separate
// block.
lhs = convertToBool(lhs, domain);
if (!lhs)
return {};
rhs = convertToBool(rhs, domain);
if (!rhs)
return {};
return builder.create<moore::OrOp>(loc, lhs, rhs);
}
case BinaryOperator::LogicalImplication: {
// `(lhs -> rhs)` equivalent to `(!lhs || rhs)`.
lhs = convertToBool(lhs);
rhs = convertToBool(rhs);
if (!lhs || !rhs)
lhs = convertToBool(lhs, domain);
if (!lhs)
return {};
rhs = convertToBool(rhs, domain);
if (!rhs)
return {};
auto notLHS = builder.create<moore::NotOp>(loc, lhs);
return builder.create<moore::OrOp>(loc, notLHS, rhs);
}
case BinaryOperator::LogicalEquivalence: {
// `(lhs <-> rhs)` equivalent to `(lhs && rhs) || (!lhs && !rhs)`.
lhs = convertToBool(lhs);
rhs = convertToBool(rhs);
if (!lhs || !rhs)
lhs = convertToBool(lhs, domain);
if (!lhs)
return {};
rhs = convertToBool(rhs, domain);
if (!rhs)
return {};
auto notLHS = builder.create<moore::NotOp>(loc, lhs);
auto notRHS = builder.create<moore::NotOp>(loc, rhs);
@ -620,12 +649,20 @@ struct RvalueExprVisitor {
auto type = context.convertType(*expr.type);
// Handle condition.
Value cond = convertToSimpleBitVector(
context.convertRvalueExpression(*expr.conditions.begin()->expr));
cond = convertToBool(cond);
if (!cond)
if (expr.conditions.size() > 1) {
mlir::emitError(loc)
<< "unsupported conditional expression with more than one condition";
return {};
auto conditionalOp = builder.create<moore::ConditionalOp>(loc, type, cond);
}
const auto &cond = expr.conditions[0];
if (cond.pattern) {
mlir::emitError(loc) << "unsupported conditional expression with pattern";
return {};
}
auto value = convertToBool(context.convertRvalueExpression(*cond.expr));
if (!value)
return {};
auto conditionalOp = builder.create<moore::ConditionalOp>(loc, type, value);
// Create blocks for true region and false region.
conditionalOp.getTrueRegion().emplaceBlock();
@ -638,6 +675,8 @@ struct RvalueExprVisitor {
auto trueValue = context.convertRvalueExpression(expr.left());
if (!trueValue)
return {};
if (trueValue.getType() != type)
trueValue = builder.create<moore::ConversionOp>(loc, type, trueValue);
builder.create<moore::YieldOp>(loc, trueValue);
// Handle right expression.
@ -645,6 +684,8 @@ struct RvalueExprVisitor {
auto falseValue = context.convertRvalueExpression(expr.right());
if (!falseValue)
return {};
if (falseValue.getType() != type)
falseValue = builder.create<moore::ConversionOp>(loc, type, falseValue);
builder.create<moore::YieldOp>(loc, falseValue);
return conditionalOp.getResult();

View File

@ -858,6 +858,16 @@ module Expressions;
// CHECK: [[NOT_BOTH:%.+]] = moore.and [[NOT_A]], [[NOT_B]] : i1
// CHECK: moore.or [[BOTH]], [[NOT_BOTH]] : i1
c = a <-> b;
// CHECK: [[TMP:%.+]] = moore.read %x : <i1>
// CHECK: [[Y:%.+]] = moore.read %y : <l1>
// CHECK: [[X:%.+]] = moore.conversion [[TMP]] : !moore.i1 -> !moore.l1
// CHECK: moore.and [[X]], [[Y]] : l1
y = x && y;
// CHECK: [[Y:%.+]] = moore.read %y : <l1>
// CHECK: [[TMP:%.+]] = moore.read %x : <i1>
// CHECK: [[X:%.+]] = moore.conversion [[TMP]] : !moore.i1 -> !moore.l1
// CHECK: moore.and [[Y]], [[X]] : l1
y = y && x;
// CHECK: [[TMP1:%.+]] = moore.read %a
// CHECK: [[TMP2:%.+]] = moore.read %b
@ -1591,3 +1601,22 @@ function void funcArgs2();
funcArgs1(42, x, y, z, w);
// CHECK: return
endfunction
// CHECK-LABEL: func.func private @ConvertConditionalExprsToResultType(
function void ConvertConditionalExprsToResultType(bit [15:0] x, struct packed { bit [15:0] a; } y, bit z);
bit [15:0] r;
// CHECK: moore.conditional %arg2 : i1 -> i16 {
// CHECK: moore.yield %arg0
// CHECK: } {
// CHECK: [[TMP:%.+]] = moore.conversion %arg1
// CHECK: moore.yield [[TMP]]
// CHECK: }
r = z ? x : y;
// CHECK: moore.conditional %arg2 : i1 -> i16 {
// CHECK: [[TMP:%.+]] = moore.conversion %arg1
// CHECK: moore.yield [[TMP]]
// CHECK: } {
// CHECK: moore.yield %arg0
// CHECK: }
r = z ? y : x;
endfunction