Conversation
In the meantime, I'll do a quick review and ask others for review as well, but we can parallelize there. |
|
@mondokm I could not add Dani as a reviewer, but I think it's you two who are the main stakeholders in Btor2Xcfa currently - can either of you please review? Thanks! |
AdamZsofi
left a comment
There was a problem hiding this comment.
I left quite a few small tasks, but overall, I think the frontend is a really nice shape, great work! You are almost there. :D
|
|
||
| override fun getExpr(): Expr<BvType> { | ||
| val newU: BigInteger = u + BigInteger.valueOf(1) | ||
| // val newU: BigInteger = if (u == l) u + BigInteger.valueOf(1) else u |
There was a problem hiding this comment.
remove commented code if not needed
| } | ||
|
|
||
| override fun getStmt(): Stmt { | ||
| TODO("We might not even need this Stmt for states") |
There was a problem hiding this comment.
Is this a comment or a todo? What's the solution? If it's not implemented on purpose, please throw an appropriate exception, not a TODO
| } | ||
|
|
||
| override fun getExpr(): Expr<*> { | ||
| TODO("Not yet implemented") |
There was a problem hiding this comment.
Why not implemented? Can it be? Or exception instead of TODO?
| private val operationVisitor = OperationVisitor() | ||
| private val statefulVisitor = StateVisitor() | ||
|
|
||
| // private val logger = ConsoleLogger(Logger.Level.VERBOSE) |
There was a problem hiding this comment.
Either remove or uncomment. Would be nice to have some verbose level logging, so maybe uncomment, if possible?
There was a problem hiding this comment.
Same for all the other logger calls around here.
| val size = sort.width.toInt() | ||
| val binArray = | ||
| BooleanArray(size) { index -> | ||
| val hexDigit = value[index / 4] |
There was a problem hiding this comment.
These 3 lines are a bit of a magic code - can you maybe leave a short comment on how this formula works here?
| var disable: Boolean = false, | ||
| @Parameter(names = ["--induction-solver", "--ind-solver"], description = "Induction solver name") | ||
| var indSolver: String = "Z3", | ||
| var indSolver: String = "mathsat:5.6.10", |
There was a problem hiding this comment.
Was this changed on purpose? If not, change back, please. :)
| description = "Interpolation solver name", | ||
| ) | ||
| var itpSolver: String = "Z3", | ||
| var itpSolver: String = "mathsat:5.6.10", |
There was a problem hiding this comment.
Was this changed on purpose? If not, change back, please. :)
| data class MddConfig( | ||
| @Parameter(names = ["--solver", "--mdd-solver"], description = "MDD solver name") | ||
| var solver: String = "Z3", | ||
| var solver: String = "mathsat:5.6.10", |
There was a problem hiding this comment.
Was this changed on purpose? If not, change back, please. :)
| data class Ic3Config( | ||
| @Parameter(names = ["--solver", "--mdd-solver"], description = "MDD solver name") | ||
| var solver: String = "Z3", | ||
| var solver: String = "mathsat:5.6.10", |
There was a problem hiding this comment.
Was this changed on purpose? If not, change back, please. :) (And just use the flag when running Theta, isntead of changing it in code?)
| @@ -0,0 +1,92 @@ | |||
| /* | |||
There was a problem hiding this comment.
Please quickly remove the witnesses from this PR to a separate branch, so that half-done code is not merged to master.
| fun visit(node: Btor2Const, param: P): R | ||
| } | ||
|
|
||
| object Btor2Circuit { |
There was a problem hiding this comment.
I think the parsed circuit being a singleton object is not a good pattern, we should instantiate a different object for each parsing.
| } | ||
| } | ||
|
|
||
| // Ehhez a nidhez vezetünk be egy változót, bekötjük |
There was a problem hiding this comment.
Please only add english comments, or remove if this is not important
| val visited = mutableSetOf<XcfaEdge>() | ||
| while (toVisit.isNotEmpty()) { | ||
| val visiting = toVisit.removeFirst() | ||
| val visiting = toVisit.removeAt(0) |
There was a problem hiding this comment.
What is the reason for this change? I see that it was applied to quite a few files
| implementation("com.zaxxer:nuprocess:2.0.5") | ||
| implementation("org.jetbrains.kotlin:kotlin-scripting-jsr223:${Versions.kotlin}") | ||
| implementation(project(mapOf("path" to ":theta-btor2-frontend"))) | ||
| implementation(project(mapOf("path" to ":theta-btor2xcfa"))) |
There was a problem hiding this comment.
Pleae use the format that is used above for both imports:
implementation(project(":theta-btor2-frontend"))
implementation(project(":theta-btor2xcfa"))
🚀 Description
This PR introduces a direct frontend and transformation pipeline for BTOR2 hardware descriptions. By bypassing external intermediate C translation steps (such as
Btor2C), this direct approach avoids the semantic mismatches and structural bloat associated with simulating hardware concurrency in sequential C code.Key Impacts:
🛠️ Key Changes
Frontend & Parsing
Core Logic & Transformation
init), unconstrained inputs (havoc), sequential operations, safety properties (bad), and state transitions (next).Btr2Passesto seamlessly integrate with the existing suite of optimization techniques already provided by Theta..satfiles. If the input is BTOR2, a.satfile is generated matching the BTOR2 witness format.🧪 Testing
.btor2and configuration files to validate the direct transformation workflow and witness generation, mostly for run configurations.read,write), preventing the verification of designs that rely on memory modeling.saddo,uaddo, etc.) and reduction operations (redand,redor,redxor), are not yet supported.badstates). Liveness properties (e.g.,justice,fairness) and other temporal specifications remain unsupported..satwitness generation feature is experimental and needs to be validated bybtor2sim.✅ Checklist
.satfiles