Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions include/circt/Dialect/OM/Evaluator/Evaluator.h
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,13 @@ class Evaluator {
/// A worklist that tracks values which needs to be fully evaluated.
std::queue<ObjectKey> worklist;

/// A queue of pending property assertions to be evaluated after the worklist
/// is fully drained. Each entry is a (PropertyAssertOp, ActualParameters)
/// pair. Property assertions are deferred because their operands may be
/// ReferenceValues that are not yet resolved when the class body is first
/// processed.
std::queue<std::pair<PropertyAssertOp, ActualParameters>> pendingAsserts;

/// Evaluator value storage. Return an evaluator value for the given
/// instantiation context (a pair of Value and parameters).
DenseMap<ObjectKey, std::shared_ptr<evaluator::EvaluatorValue>> objects;
Expand Down
24 changes: 19 additions & 5 deletions lib/Dialect/OM/Evaluator/Evaluator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -308,11 +308,13 @@ circt::om::Evaluator::evaluateObjectInstance(StringAttr className,
fields[cast<StringAttr>(name)] = result.value();
}

// Evaluate property assertions.
LLVM_DEBUG(dbgs() << "asserts:\n");
for (auto assertOp : cls.getOps<PropertyAssertOp>())
if (failed(evaluatePropertyAssert(assertOp, actualParams)))
return failure();
// Defer property assertions until after the worklist is drained, so that
// all ReferenceValues are fully resolved before we try to inspect them.
LLVM_DEBUG(dbgs() << "queuing asserts:\n");
for (auto assertOp : cls.getOps<PropertyAssertOp>()) {
LLVM_DEBUG(dbgs(1) << "- " << assertOp << "\n");
pendingAsserts.push({assertOp, actualParams});
}

// If the there is an instance, we must update the object value.
LLVM_DEBUG(dbgs() << "object value:\n");
Expand Down Expand Up @@ -392,6 +394,18 @@ circt::om::Evaluator::instantiate(
worklist.push({value, args});
}

// Now that all values are fully resolved, evaluate the deferred property
// assertions.
LLVM_DEBUG(dbgs() << "asserts:\n");
bool assertFailed = false;
while (!pendingAsserts.empty()) {
auto [assertOp, assertParams] = pendingAsserts.front();
pendingAsserts.pop();
assertFailed |= failed(evaluatePropertyAssert(assertOp, assertParams));
}
if (assertFailed)
return failure();

auto &object = result.value();
// Finalize the value. This will eliminate intermidiate ReferenceValue used as
// a placeholder in the initialization.
Expand Down
26 changes: 26 additions & 0 deletions unittests/Dialect/OM/Evaluator/EvaluatorTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1836,6 +1836,25 @@ om.class @SubfieldFalse() -> () {
om.property_assert %false, "input must be true true" : i1
om.class.fields
}

// Test 11: Test that asserts that need values to flow through objects work.
// This was originally a bug where asserts were evaluated too early.
//
// See: https://github.com/llvm/circt/issues/10264
om.class @Domain(%in: !om.string) -> (out: !om.string) {
om.class.fields %in : !om.string
}
om.class @ChainedDomainAssert(%basepath: !om.frozenbasepath) -> () {
%0 = om.constant "A" : !om.string
%1 = om.object @Domain(%0) : (!om.string) -> !om.class.type<@Domain>
%2 = om.object.field %1, [@out] : (!om.class.type<@Domain>) -> !om.string
%3 = om.object @Domain(%2) : (!om.string) -> !om.class.type<@Domain>
%4 = om.object.field %3, [@out] : (!om.class.type<@Domain>) -> !om.string
%5 = om.constant "B" : !om.string
%6 = om.prop.eq %4, %5 : !om.string
om.property_assert %6, "hello" : i1
om.class.fields
}
)MLIR";

DialectRegistry registry;
Expand Down Expand Up @@ -1878,6 +1897,13 @@ om.class @SubfieldFalse() -> () {

ASSERT_TRUE(failed(
evaluator.instantiate(StringAttr::get(&context, "SubfieldFalse"), {})));

// Test 11: Two asserts on a value flowing through chained object instances.
// Both assertions fail; the evaluator must drain the worklist before
// evaluating either assert (i.e. the fix for the null-ReferenceValue crash).
auto basepath = std::make_shared<evaluator::BasePathValue>(&context);
ASSERT_TRUE(failed(evaluator.instantiate(
StringAttr::get(&context, "ChainedDomainAssert"), {basepath})));
}

TEST(EvaluatorTests, PropEqTests) {
Expand Down
Loading