Skip to content

Conversation

@cowardsa
Copy link
Contributor

To verify correctness of transformations involving datapath operations, include a lowering from datapath to SMT. Will later be integrated into circt-lec to enable verification of these operators. Each operator satisfied a contract, rather than providing a precise semantics for every operator. This is because the datapath operators return values in redundant number representations, meaning there are many valid implementations.

For example:

%0:2 = datapath.compress %a, %b, %c : i8 [3 -> 2]

Will be verified by introducing free variables for each return value (%0#0, %0#1) then asserting that the sum of the associated free variables is equal to the sum of the inputs: assert(%0#0 + %0#1 == %a + %b + %c).

Whilst this is encoded as an assert it really represents an assumption that must be satisfied by a valid implementation of datapath.compress.

Copy link
Contributor

@fabianschuiki fabianschuiki left a comment

Choose a reason for hiding this comment

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

Really cool! Just a few minor test regex nits.


namespace circt {

/// Get the HW to SMT conversion patterns.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Get the HW to SMT conversion patterns.
/// Get the Datapath to SMT conversion patterns.

Comment on lines 52 to 58
// Create free variables
SmallVector<Value, 2> newResults;
for (Value result : results) {
auto declareFunOp = rewriter.create<smt::DeclareFunOp>(
op.getLoc(), typeConverter->convertType(result.getType()));
newResults.push_back(declareFunOp.getResult());
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Very neat 🥳

//CHECK-NEXT: %[[ADD012:.+]] = smt.bv.add %[[ADD01]], %[[PP2]] : !smt.bv<3>
//CHECK-NEXT: %[[P:.+]] = smt.eq %[[MUL]], %[[ADD012]] : !smt.bv<3>
//CHECK-NEXT: smt.assert %[[P]]
//CHECK-NEXT: hw.output %[[PP0_BV:.+]], %[[PP1_BV:.+]], %[[PP2_BV:.+]] : i3, i3, i3
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//CHECK-NEXT: hw.output %[[PP0_BV:.+]], %[[PP1_BV:.+]], %[[PP2_BV:.+]] : i3, i3, i3
//CHECK-NEXT: hw.output %[[PP0_BV]], %[[PP1_BV]], %[[PP2_BV]] : i3, i3, i3

// CHECK-NEXT: %[[COMP0:.+]] = smt.declare_fun : !smt.bv<4>
// CHECK-NEXT: %[[COMP0_BV:.+]] = builtin.unrealized_conversion_cast %[[COMP0]] : !smt.bv<4> to i4
// CHECK-NEXT: %[[COMP1:.+]] = smt.declare_fun : !smt.bv<4>
// CHECK-NEXT: %[[COMP1_BV:.+]] = builtin.unrealized_conversion_cast %7 : !smt.bv<4> to i4
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// CHECK-NEXT: %[[COMP1_BV:.+]] = builtin.unrealized_conversion_cast %7 : !smt.bv<4> to i4
// CHECK-NEXT: %[[COMP1_BV:.+]] = builtin.unrealized_conversion_cast %[[COMP1]] : !smt.bv<4> to i4

Comment on lines 27 to 32
//CHECK-NEXT: %[[PP0:.+]] = smt.declare_fun : !smt.bv<3>
//CHECK-NEXT: %[[PP0_BV:.+]] = builtin.unrealized_conversion_cast %3 : !smt.bv<3> to i3
//CHECK-NEXT: %[[PP1:.+]] = smt.declare_fun : !smt.bv<3>
//CHECK-NEXT: %[[PP1_BV:.+]] = builtin.unrealized_conversion_cast %5 : !smt.bv<3> to i3
//CHECK-NEXT: %[[PP2:.+]] = smt.declare_fun : !smt.bv<3>
//CHECK-NEXT: %[[PP2_BV:.+]] = builtin.unrealized_conversion_cast %7 : !smt.bv<3> to i3
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//CHECK-NEXT: %[[PP0:.+]] = smt.declare_fun : !smt.bv<3>
//CHECK-NEXT: %[[PP0_BV:.+]] = builtin.unrealized_conversion_cast %3 : !smt.bv<3> to i3
//CHECK-NEXT: %[[PP1:.+]] = smt.declare_fun : !smt.bv<3>
//CHECK-NEXT: %[[PP1_BV:.+]] = builtin.unrealized_conversion_cast %5 : !smt.bv<3> to i3
//CHECK-NEXT: %[[PP2:.+]] = smt.declare_fun : !smt.bv<3>
//CHECK-NEXT: %[[PP2_BV:.+]] = builtin.unrealized_conversion_cast %7 : !smt.bv<3> to i3
//CHECK-NEXT: %[[PP0:.+]] = smt.declare_fun : !smt.bv<3>
//CHECK-NEXT: %[[PP0_BV:.+]] = builtin.unrealized_conversion_cast %[[PP0]] : !smt.bv<3> to i3
//CHECK-NEXT: %[[PP1:.+]] = smt.declare_fun : !smt.bv<3>
//CHECK-NEXT: %[[PP1_BV:.+]] = builtin.unrealized_conversion_cast %[[PP1]] : !smt.bv<3> to i3
//CHECK-NEXT: %[[PP2:.+]] = smt.declare_fun : !smt.bv<3>
//CHECK-NEXT: %[[PP2_BV:.+]] = builtin.unrealized_conversion_cast %[[PP2]] : !smt.bv<3> to i3

#include "circt/Conversion/DatapathToSMT.h"
#include "circt/Conversion/HWToSMT.h"
#include "circt/Dialect/Datapath/DatapathOps.h"
#include "mlir/Dialect/Func/IR/FuncOps.h"
Copy link
Member

Choose a reason for hiding this comment

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

Maybe FuncOp is not necessary?

def ConvertDatapathToSMT : Pass<"convert-datapath-to-smt"> {
let summary = "Convert datapath ops to SMT ops";
let dependentDialects = [
"mlir::smt::SMTDialect", "hw::HWDialect", "mlir::func::FuncDialect"
Copy link
Member

Choose a reason for hiding this comment

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

hw and func would be not necessary

Copy link
Member

@uenoku uenoku left a comment

Choose a reason for hiding this comment

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

LGTM

rewriter.create<smt::BVAddOp>(op.getLoc(), operandRunner, operand);

// Create free variables
SmallVector<Value, 2> newResults;
Copy link
Member

Choose a reason for hiding this comment

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

nit:

Suggested change
SmallVector<Value, 2> newResults;
SmallVector<Value, 2> newResults;
newResults.reserve(results.size());

@cowardsa
Copy link
Contributor Author

Comments addresssed and ready to merge when someone gets a chance please!

@fabianschuiki fabianschuiki merged commit 097604c into llvm:main Jul 11, 2025
7 checks passed
@fabianschuiki
Copy link
Contributor

Awesome thanks! Landed 👍

@cowardsa cowardsa deleted the coward/datapath_to_smt_init branch July 18, 2025 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants