Add QIR Program Format Support to the DDSIM QDMI Device#1766
Conversation
Add qir::jit::Session to encapsulate the LLJIT initialization, QIR runtime symbols registration, and program execution. The idea is to avoid future duplication of JIT setup both in the Runner and in the Device. Update Runner to use qir::jit::Session.
circuits.hpp, error_handling_test.cpp: add QIR_BELL_PAIR_STATIC with an LLVM assembly string of a Bell pair program, and rename MALFORMED_PROGRAM to QASM3_MALFORMED (to be consistent with the other QASM3_ variables). src/qdmi/devices/dd/CMakeLists.txt: add MQT::CoreQIRJIT library dependency. src/qir/jit/CMakeLists.txt: link libraries privately. test/qdmi/devices/dd/CMakeLists.txt: add LLVM native libs dependencies. Device.hpp: add submitQASMProgram and submitQIRProgram APIs. Device.cpp: handle job submissions of a QASM program and of a QIR base module/string separately. Implement submitQIRProgram: for numShots_, reset the runtime, run the program, process results into a bit string, update measurement counts. Device.cpp, test_utils.cpp: the device stores the program into a std::string of a given size, and submitQIRProgram reads exactly that size. No passing +1 bytes or sometimes retrieving -1 bytes is needed in any case. job_parameters_test.cpp: set QDMI_PROGRAM_FORMAT_QIRBASEMODULE and QDMI_PROGRAM_FORMAT_QIRBASESTRING as supported formats. results_sampling_test.cpp: change HistogramKeysAndValuesSumToShots into a test class, and implement two new test fixtures for QIR base modules and strings. These two new tests exercise the submission of a QIR program for a DD device job. Runtime: add getResults API. Session: make run methods const.
This option enables the QIR format support for QDMI. It is defined in the top level CMakeLists.txt, and it is disabled by default. The QDMI DDSim Device library only links to QIR JIT and QIR Runtime libraries if the option is ON. The same applies to the corresponding test library.
Add a 'QIR Support in the QDMI Device' entry. Assisted-by: Claude Opus 4.7 via Claude Code
Leave just one function where both arguments have default values.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
When BUILD_MQT_CORE_MLIR=OFF , find_package(MLIR) is not called, so LLVM CMake macros like llvm_map_components_to_libnames are undefined. The new qir/jit subdirectory used that macro unconditionally and failed to configure. Make BUILD_MQT_CORE_QDMI_WITH_QIR a cmake_dependent_option on BUILD_MQT_CORE_MLIR (forced OFF when MLIR is off). Add the qir/jit subdirectory only when at least one consumer (Runner CLI or QDMI-with-QIR) is enabled. Assisted-by: Claude Opus 4.7 via Claude Code
The HistogramKeysAndValuesSumToShots fixture class in results_sampling_test.cpp was at file scope, triggering clang-tidy's misc-use-anonymous-namespace check in CI (clang-tidy 22). Wrap it in an anonymous namespace to enforce internal linkage, matching the convention used by the ErrorHandling fixture in error_handling_test.cpp. Assisted-by: Claude Opus 4.7 via Claude Code
Signed-off-by: Roberto Turrado Camblor <rturrado@gmail.com>
Remove merge conflict strings.
ystade
left a comment
There was a problem hiding this comment.
Hi @rturrado,
Your contribution already looks great in this state. Thanks for taking part in the unitaryHACK. I am a big fan of the restructuring, i.e., having the JIT interpreter as its own library. This makes the code a lot better structured. It is actually also cool to see that this refactoring comprises the most part of this PR, and the remaining changes are quite local. The biggest action item that remains in my mind is the documentation of the Session class. In particular, I reviewed all files in detail except the session class because the documentation eases the review here a lot. So, in summary, my biggest request are comprehensive docstrings for the Session class. Overall, this contribution looks already very good and is very valuable.
|
@rturrado Just came to say this is a great initial draft! Keep it up 💪🏻 |
Also move comment from Runner's main into Session::run.
Move filtering and sorting of Result pointers to getResults. Add qir::toBitString to convert a map<Result*, bool> to a bit string. Assisted-by: Claude Opus 4.7 via Claude Code
Add an injectable std::ostream pointer member to Runtime, defaulting to std::cout, with getOstream/setOstream/resetOstream accessors. Update __quantum__rt__result_record_output (QIR.cpp) to write through runtime.getOstream() rather than directly to std::cout. Migrate the test fixtures in test_qir_runtime.cpp (QIRRuntimeTest) and results_sampling_test.cpp (HistogramKeysAndValuesSumToShots). Inject a std::ostringstream sink via setOstream in SetUp and restore the default via resetOstream in TearDown. QIRRuntimeTest's previous std::cout.rdbuf swap is gone. The QDMI sampling tests no longer spew thousands of result-record lines to stdout per run. Link MQT::CoreQIRRuntime to the dd-device test target when BUILD_MQT_CORE_QDMI_WITH_QIR is ON, since results_sampling_test.cpp now includes Runtime.hpp directly. Assisted-by: Claude Opus 4.7 via Claude Code
Hi @ystade ! Many thanks for the comments and for the thorough review. I'll go over each of the items in the following days. There are still a few things I want to tackle as part of this PR, on top of whatever you consider necessary:
|
Assisted-by: Claude Opus 4.7 via Claude Code
Add Runtime::recordOutput(Result*) and Runtime::getRecordedOutputs(), backed by a std::string field. __quantum__rt__result_record_output now records the value into recordedOutputs before emitting the QIR-spec "label: 0|1" line, so dynamic QIR programs that release their Results via reference-count decrement no longer lose data before the QDMI device reads it. submitQIRProgram in the dd device consumes runtime.getRecordedOutputs() directly as the histogram key. Remove the now-unused Runtime::getResults() and the qir::toBitString helper. Add QIR_BELL_PAIR_DYNAMIC to circuits.hpp and the matching QIRBaseModuleDynamic and QIRBaseStringDynamic sampling tests. Flip dynamic_qubit_management and dynamic_result_management to i1 true in BellPairDynamic.ll and the inline dynamic string, for spec-compliant module metadata. Assisted-by: Claude Opus 4.7 via Claude Code
Move BellPairStatic.ll, BellPairDynamic.ll, and GHZ4Dynamic.ll from test/qir to test/circuits, so the same files can serve both the qir-{runner,runtime} tests and the QDMI device tests.
Update QIR_FILES_DIR in test/qir/{runner,runtime}/CMakeLists.txt to point at the new location.
Add a QIR_FILES_DIR compile definition to the QDMI device test target.
Switch the QIR tests in results_sampling_test.cpp to load BellPairStatic.ll and BellPairDynamic.ll from that path.
Drop the QIR_BELL_PAIR_STATIC and QIR_BELL_PAIR_DYNAMIC inline strings from circuits.hpp.
Assisted-by: Claude Opus 4.7 via Claude Code
Route `QDMI_PROGRAM_FORMAT_QIRADAPTIVE{MODULE,STRING}` to `submitQIRProgram` and accept them in the program-format gate.
The existing JIT runtime already handles `__quantum__rt__read_result + br i1`, so a `H; measure; conditional-X; measure` Bell pair via classical correction runs end-to-end with no further runtime change.
`bindings/fomac/fomac.cpp` already exposed `QIR_ADAPTIVE_{MODULE,STRING}`, so the Python surface is unchanged.
New `test/circuits/BellPairAdaptive.ll`: produces the same `{"00","11"}` correlation as the entangled Bell pair, but via classical control flow on a measured result, so the same assertion validates all three encodings (Static, Dynamic, Adaptive).
Refactor `results_sampling_test.cpp` into a shared `HistogramTest` fixture plus two QIR subclasses.
`checkHistogram` now asserts both `sum == SHOTS` and the histogram keys are exactly `{"00","11"}`.
The new key-shape check is what proves the adaptive branch actually fires for every shot rather than collapsing to a single value.
Move the adaptive format constants from the unsupported loop to the supported loop in `job_parameters_test.cpp`.
Assisted-by: Claude Opus 4.7 via Claude Code
|
A question for you, guys! The last commits add support for QIR adaptive formats. These formats allow the client to record and output values other than The question would be if we also want to record the other possible values, and if we want to do it as part of this PR. A possible solution would be to change that |
Hm, very good question. I generally believe that for the sake of this PR, it is fine to focus on the things that can reasonably be piped through QDMI as results. Does that make sense? |
|
Thanks for the answer, @burgholzer.
No, not really. I asked Claude today to think about a realistic scenario for using a QIR program in adaptive format, and it came up with a Hamming weight, measuring 3 qubits, and calculating an int weight and a float mean, which I found nice. But, back to my original question, I think that I was understanding wrongly the QIR concept of recording outputs of different types. I believe now that the spec only refers to logging labels and values somewhere (e.g., an output stream). And that, if we decide to use a data structure for keeping the measurements bit string, that is something completely apart. My confusion came from the idea that we were requested by the QIR spec to store bools, and ints, and records, and so on, into that data structure.
The current QIR runner can only record |
|
@rturrado, it is cool to see when the software components that we have, e.g., the runner, become more spec-compliant. In my opinion, to keep the PR focused and avoid overloading it, it should be fine to restrict it to recording results and leave other data types for future Issues/PRs. |
|
@rturrado let me know, when I should review this PR again. I'll try to get to it asap then. |
@ystade Thanks! I think that the only pending points are the coverage errors, so if you want to review the last commits, and maybe check if some unresolved convesations can already be closed, you can do it already. |
|
@rturrado Thanks for the update, I am happy to wait until the coverage is also satisfied. |
|
BTW: Do not worry about the Windows CI errors. They are unrelated to your changes and a consequence of some GitHub runner updates for Windows systems not interacting well with our pre-built LLVM distributions. They will be fixed separately. |
Lift `src/qir/jit/Session.cpp` diff coverage from 76.8% toward the Codecov 90% target by exercising paths the existing tests skipped:
- `LoadModuleFromMemory`: build a `JitSession` from an in-memory IR buffer (the contents of `BellPairStatic.ll`) and run it, covering `loadModuleFromMemory` and the memory-based constructor.
- `MalformedIRThrows`: pass unparseable IR and expect `std::runtime_error`, covering `getThreadSafeModuleOrError`'s null-module branch and the throw in `initialize` when the loader hands back an error.
- `MissingMainThrows`: pass valid IR with no `main` and expect `std::runtime_error`, covering the `lookup("main")` failure branch.
While here, rename the fixture from `JitSessionExecutionMode` to `JitSessionTest` and factor out a `getProgram` helper that mirrors the one in `results_sampling_test.cpp`.
The remaining uncovered lines in `Session.cpp` are Cygwin/MinGW paths, the GDB JIT listener, and throw arms behind LLVM `Expected<T>` failures that cannot be portably forced from a test.
Files:
- test/qir/jit/test_jit_session.cpp: new tests, renamed fixture, and `getProgram` helper.
Assisted-by: Claude Opus 4.7 via Claude Code
The `JitSessionErrors.MissingMainThrows` test added in the previous commit failed on macOS CI.
The host process's `main` symbol is exported through `DynamicLibrarySearchGenerator::GetForCurrentProcess` on macOS but not on Linux.
So the JIT's `lookup("main")` finds the gtest runner's `main` on macOS and the constructor returns without throwing.
On Linux the lookup fails and the throw fires, which is what the test expected.
We do not control which symbols the process generator exposes from the test side, so the test cannot be made portable.
The lost line (the `lookup("main")` failure branch at line 381 of `Session.cpp`) is not worth a platform-specific carve-out.
Files:
- test/qir/jit/test_jit_session.cpp: remove the `MissingMainThrows` test.
Assisted-by: Claude Opus 4.7 via Claude Code
Add `QIRRuntimeTest.PackageResizeWhenEnlargingState`: apply H to qubit address 32, which makes `translateAddresses` call `enlargeState(32)` and pushes `qState.numQubits` past the `dd::Package` default capacity of 32, triggering `qState.dd->resize`. Files: - test/qir/runtime/test_qir_runtime.cpp: new test driving H on qubit index 32. Assisted-by: Claude Opus 4.7 via Claude Code
|
@ystade @burgholzer PR should be ready now for review. Functionality is there. Code coverage is happy. And tests are passing (except those 2 Windows ones that you mentioned we shouldn't worry about for this PR). |
ystade
left a comment
There was a problem hiding this comment.
@rturrado Thanks for this very cool PR extending the QDMI DDSIM device to support QIR in the base and adaptive profile. Your code is very well structured and I like also the few refactoring of the old code base. This is a very valauble contribution.
I have left some minor comments inline. Other then these, you get a green light from my end. I purposefully do not approve the PR yet and leave this to @burgholzer because I do not know whether he also wants to have an eye on the PR before it gets merged.
|
I'll give this a proper review tomorrow. Didn't quite get to it today unfortunately. |
Apply review suggestions across CMake, docs, and Doxygen comments, plus one parameter rename. - CMakeLists.txt: default `BUILD_MQT_CORE_QDMI_DDSIM_WITH_QIR` to `ON` instead of `OFF`, so QIR support is enabled whenever MLIR is. - test/qir/CMakeLists.txt: drop the outer `if(BUILD_MQT_CORE_QIR_RUNNER)` and `if(BUILD_MQT_CORE_QIR_RUNNER OR BUILD_MQT_CORE_QDMI_DDSIM_WITH_QIR)` guards, since the inner `if(TARGET ...)` checks in each child already subsume them. Assisted-by: Claude Opus 4.7 via Claude Code
Replace three identical inline `getProgram` definitions across the QIR JIT and DDSIM QDMI test files with a single shared utility.
- test/qir/helpers/test_utils.{h,c}pp: new `qir_test::getProgram(std::string_view)` that reads a QIR source file from `QIR_FILES_DIR` and returns its contents as a string.
- test/qir/jit/test_jit_session.cpp, test/qdmi/devices/dd/{results_sampling_test.cpp, results_statevector_test.cpp}: drop the local `getProgram` and call `qir_test::getProgram` directly.
- test/CMakeLists.txt: reorder so `qir` is added before `qdmi`, so the qdmi tests can link against `mqt-core-qir-test-utils`.
- test/qir/CMakeLists.txt: add the new `helpers` subdirectory first.
- test/qir/helpers/CMakeLists.txt: new `mqt-core-qir-test-utils` STATIC library, with `test/` on its public include path so consumers include via `#include "qir/helpers/test_utils.hpp"`.
- test/qir/jit/CMakeLists.txt, test/qdmi/devices/dd/CMakeLists.txt: link the helper.
Assisted-by: Claude Opus 4.7 via Claude Code
- include/mqt-core/qdmi/devices/dd/Device.hpp: change `program_` to `std::variant<std::string, std::vector<std::byte>>`. Text formats (QASM2/3, QIR Base/Adaptive String) use the string alternative, binary formats (QIR Base/Adaptive Module) use the byte-vector alternative. - src/qdmi/devices/dd/Device.cpp: write site dispatches on `isTextProgramFormat(format_)`. QIR JIT paths use `std::visit` with a generic lambda so the same lambda extracts `.data()` and `.size()` from either alternative and builds an `llvm::StringRef`. Assisted-by: Claude Opus 4.7 via Claude Code
burgholzer
left a comment
There was a problem hiding this comment.
This looks really clean. Great contribution @rturrado 🙌🏼
I have one small round of feedback that I'd like to see addressed. Afterwards, we can happily merge this 🚀
- Device.cpp: Fix bug at `queryProperty`. The previous `std::get_if<std::string>` guard silently dropped binary payloads. Use ADD_STRING_PROPERTY for the `std::string` alternative and ADD_LIST_PROPERTY for the `std::vector<std::byte>` alternative. - ProgramFormat.hpp: deleted; the predicate is inlined at its two call sites. - index.md: cover Adaptive Profile. - CHANGELOG.md, CMakeLists.txt, index.md: normalize "DDSIM QDMI Device" term. Assisted-by: Claude Opus 4.7 via Claude Code
ystade
left a comment
There was a problem hiding this comment.
@rturrado This PR was truly a very pleasant interaction. This is exactly how we imagine a unitaryHACK contribution to be. Thanks for your contribution!
If you are interested, you are very welcome to contribute to our projects again in the future, either as part of the next unitaryHACK or independently.
@ystade Thanks so much! I really appreciate the words. I can also say it was very pleasant for me. You were very quick to answer my questions and provide reviews, and I found the comments very constructive. Not to mention that for me, being able to work in (and learn from) a C++ codebase in this quantum/LLVM/IR world is kind of an ideal job.
Definitely. I have at least a couple of pending follow-ups: Add QIR Output Schemas Support, and Add QIR Adaptive Profile Support. Do you prefer me to open the PRs directly, or should I create an issue first? |
burgholzer
left a comment
There was a problem hiding this comment.
Wonderful 🙌🏼
Before we merge this, could you please add a comment to the original issue here so that we can assign it to you and you get your bounty? 💸
As for tackling further issues: Typically, it's not a bad idea to create tracking issues. Not an absolute must-have, but it kind of gives us the opportunity to discuss some design decisions before any implementation is done.
🥳
Roger that.
Fair enough! |
Description
First contribution to UnitaryHACK for me! 🎉 And man, I've enjoyed working on this one! 😀
Note
The current PR still does not cover all the functionality proposed in the #1695 issue. In particular, the support for QIR adaptive base modules and strings is not yet implemented. But the current solution is ready for review.
There are two main pieces of work:
MQT::CoreQIRJIT,src/qir/jit). This library contains almost all of the previousRunner.cppcode: LLVM JIT initialization, QIR runtime symbols registration, and program execution. Compared to that previous Runner code, it also lets you parse an IR directly from a string, and not only from a file. The main purpose of the QIR JIT library is to encapsulate the LLVM JIT code so that it can also be used from the QDMI device. The interface is aqir::jit::Sessiontype that can be constructed from an input file or from a byte string (text for LLVM assembly or binary for LLVM bitcode), and then run with or without arguments (e.g., the Runner CLI can run a JIT program with arguments, and the QDMI device without arguments).Device.cpp. By using the MQT Core QIR JIT library, the device can just run a JIT program, get the results, and process them (filter zero and one addresses, sort the addresses, and transform the measurements into a bit string).The new functionality is tested via two tests in
results_sampling_test.cpp, one that exercises the QIR base module path, and the other that exercises the QIR base string path.For this PR I have used Claude Opus 4.7, mainly from a CLion terminal, and for different purposes: from understanding existing code, to drafting code snippets, catching bugs and typos, running clang-format and clang-tidy and proposing fixes for clang-tidy warnings, reviewing wording in commit messages and docs...
Fixes #1695
Checklist
If PR contains AI-assisted content:
Assisted-by: [Model Name] via [Tool Name]footer.