✨ Record typed compiler target environments in MQT IR - #2215
Conversation
7b25247 to
07352c2
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
07352c2 to
c2c5028
Compare
c2c5028 to
420fbc8
Compare
36b7fbb to
99f2786
Compare
burgholzer
left a comment
There was a problem hiding this comment.
This also looks genuinely good except for one little detail.
99f2786 to
e7350d0
Compare
|
Important Approval pendingCodeRabbit has no unresolved comments, but it has not reviewed the latest commit. Use the checkbox below to review the latest commit. CodeRabbit will approve the changes if it finds no blocking issues.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
e7350d0 to
86d01c1
Compare
Record complete compiler-target facts and one exact payload execution contract as typed module metadata. Assisted-by: GPT-5.6 Sol via Codex
Assisted-by: GPT-5.6 Sol via Codex
Remove duplicate connectivity and native-operation enum definitions. Reuse the generated MQT attribute enums across the C++ target and its materialized MLIR representation. Assisted-by: GPT-5.6 Sol via Codex
86d01c1 to
390e966
Compare
denialhaag
left a comment
There was a problem hiding this comment.
I really like the addition of these attribues! I only have a nitpick and a comment on the attribute descriptions, otherwise this LGTM! I'm already approving now, as neither of my comments is blocking. 😎
There was a problem hiding this comment.
Not important, but I don't think we need the MQT_ prefixes here.
There was a problem hiding this comment.
Agreed. Let's drop them!
| let description = [{ | ||
| Records the exact payload and its effective capabilities. Producers expand | ||
| descriptor baselines before creating this attribute. The capability list | ||
| remains available when optional capability metadata is unknown; | ||
| `optional_capabilities_known` records whether that optional metadata is | ||
| complete. | ||
| }]; |
There was a problem hiding this comment.
While I'm generally a fan of keeping documentation concise, I think (some of) the attributes added here could benefit from some examples in their descriptions, especially because they all end up in the rendered docs. 🤔
There was a problem hiding this comment.
Yeah, fully agree. The initial version of these was even more concise and I already urged the system to be a little more elaborate.
I like the idea of including an explicit example of a small program using that feature as ```mlir ... ``` code
|
@simon1hofmann any feedback on this one? Given its low position in the stack, I'd like to not touch this part more than once anymore. |
simon1hofmann
left a comment
There was a problem hiding this comment.
Also went through this and looks pretty clean as well. Just have one finding that was flagged, probably worth double-checking in your Codex session.
| }]; | ||
| let parameters = (ins "CompilationTargetAttr":$compilation_target, | ||
| "PayloadEnvAttr":$payload_env, | ||
| OptionalParameter<"::mlir::MapAttr">:$extensions); |
There was a problem hiding this comment.
🤖 AI text below 🤖
MQTDialect.td:209 embeds #dlti.map, but MQT does not declare DLTIDialect as dependent. Standard contexts omit it in Programs.cpp:88-92 and mqt-cc.cpp:456-460. Consequently, valid extension-bearing target environments cannot be parsed through QCOProgram or mqt-cc; the latter crashes after parsing fails. The test masks this by registering DLTI explicitly at test_mqt_ir.cpp:51-54.
There was a problem hiding this comment.
Feels like a correct finding. Let's double check and fix this where it is supposed to be fixed.
🤖 AI text below 🤖
Description
Record one complete compiler target environment as typed MQT IR metadata.
mqt.target_envcombines immutable hardware facts with one exact payload descriptor and its effective execution capabilities. Textual MLIR pipelines can now inspect and reproduce the target contract without hidden pass state.The hardware side preserves ordered sites, timing and fidelity data, topology knowledge, and native-operation knowledge without serializing derived routing or synthesis caches. The payload side keeps format ID, semantic version, profile, encoding, extensible capabilities, typed constraints, and whether optional capability metadata is complete.
CompilerTargetremains a context-free value and converts losslessly at the IR boundary.The attribute implements the DLTI query interface for typed MQT values and namespaced extensions. Direct lookup through the canonical
mqt.target_envmodule attribute remains authoritative because a module can contain other DLTI query attributes.This PR is stacked on #2218. It stays independent of the work-in-progress QDMI v1.4 API; a later adapter change can map stable QDMI descriptors and capabilities into this provider-neutral schema.
Checklist
If PR contains AI-assisted content:
🤖 *AI text below* 🤖(titles are exempt).