-
Notifications
You must be signed in to change notification settings - Fork 93
Replace C++ QIR Runtime with Rust QIR stdlib #1087
Conversation
You may also want to update the .github/CODEOWNERS (some paths get deleted). |
PauliId PauliArgCli; | ||
PauliArgCli = PauliId::PauliId_I; | ||
char PauliArgCli; | ||
PauliArgCli = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Pauli values turn into the "magic numbers". E.g. it stops being evident that in green L37 and L20 the same value is used.
Consider creating some common C header with Pauli values enum, other common stuff, and including that header wherever needed.
@@ -0,0 +1,504 @@ | |||
// Copyright (c) Microsoft Corporation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Nit)
If this file is a result of renaming/moving of some other file, then consider renaming/moving with git mv ..
to preserve the history and minimize the diff (and only see the diff for the lines that changed).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Git rename detection doesn't use git mv
. It works the same if you git rm
and git add
two files with similar contents. The rename is detected at diff time and has a configurable similarity threshold (default 50%); see git diff --find-renames[=<n>]
. So if more than 50% of the file changed, it won't be seen as a rename by default, even if you used git mv
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm done reviewing. Approving with suggestions.
I recommend more approvals.
src/Qir/Tests/Tools/TestCases/FullStateDriverGenerator/UseBoolArg.cpp
Outdated
Show resolved
Hide resolved
Merging with passed e2e builds (see https://github.com/microsoft/qdk/pull/288). Thanks everyone for the review! |
This change removes the C++ implementation for the QIR Runtime and replaces it with a Rust implementation of a QIR stdlib that matches the classical functionality exposed.
Previously, the C++ QIR Runtime was compiled into three dynamic libraries,
Microsoft.Quantum.Qir.Runtime
,Microsoft.Quantum.Qir.QSharp.Foundation
, andMicrosoft.Quantum.Qir.QSharp.Core
, and the "core" library would load the simulator dll at runtime. With this change, these dynamic libraries are no longer shipped, and we instead build a single static library,qir_stdlib
, that is linked into the simulator dynamic library,Microsoft.Quantum.Simulator.Runtime
. This makes the full state simulator a "QIR Backend" which means it exposes the classical and quantum functions necessary to link in and execute a QIR program.To help orient reviews of this rather large change, here are some key landmarks in the code:
stdlib
folder that implements the classical functions described in the QIR spec as a static library written in Rust. This includes unit tests for those functions.qir_stdlib
as a dependency and re-export the QIR classical library support. New QIR-compliant C-API is added to allow QIR programs to link directly against the quantum operations supported by the simulator without the need of an additional shim library.NOTE: e2e builds will fail until a corresponding update is made to the e2e build script logic in order to build the QIR stdlib before the simulator to satisfy dependencies.