Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Experimental concurrency support #730

Merged
merged 30 commits into from
May 13, 2021
Merged

Experimental concurrency support #730

merged 30 commits into from
May 13, 2021

Conversation

abakst
Copy link
Contributor

@abakst abakst commented May 4, 2021

The main purpose of this PR is to:

  1. Add crucible-concurrency, a library used to add concurrency support for crucible-based tools. Notes on designs, limitations, future work, etc are included in DesignNotes.md in the crucible-concurrency subdirectory.
  2. Enable some concurrency support in crux-mir using the above. Notes on this implementation are included in Concurrency.md in the crux-mir subdirectory. The changes broadly consist of a new module Mir.Concurrency, as well as some changes to the Rust libraries shipped with the tool (i.e., those under lib).

crucible-concurrency ships with an implementation for crucible-syntax: hence, this PR also includes some minor changes to crucible-syntax to make it a bit more useful as a library, and also to fix a small issue with its entry block construction.

@abakst abakst requested review from spernsteiner and atomb May 4, 2021 21:12
crux-mir/lib/libcore/crucible/concurrency.rs Outdated Show resolved Hide resolved
crucible-concurrency/test/Main.hs Outdated Show resolved Hide resolved
crucible-concurrency/DesignNotes.md Outdated Show resolved Hide resolved
abakst and others added 2 commits May 5, 2021 08:31
Co-authored-by: Ryan Scott <ryan.gl.scott@gmail.com>
Copy link
Contributor

@RyanGlScott RyanGlScott left a comment

Choose a reason for hiding this comment

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

I guess it's the plural of crucible (multiple threads, etc...).

Gotcha. I suppose my confusion was rooted in that I kept trying to interpret "Crucibles" as "multiple instances of Crucible" rather than as a separate name altogether. For whatever reason, I didn't seem to have the same issue with Crux versus Cruces—I guess the use of Spanish/Latin adds its own distinct flair :)

In any case, I appreciate the footnote you've added to the README.

crucible-concurrency/DesignNotes.md Outdated Show resolved Hide resolved
crucible-concurrency/DesignNotes.md Outdated Show resolved Hide resolved
Copy link
Contributor

@atomb atomb left a comment

Choose a reason for hiding this comment

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

This is really an impressive bit of work! I appreciate the design notes and the number of tests and benchmarks you included. I didn't deeply review most of the Haskell code, since a lot of it implements algorithms I'm not very familiar with, but it looks good from what I can tell (though I did notice a few bits of commented-out code that could probably be removed).

crucible-concurrency/src/Cruces/CrucesMain.hs Outdated Show resolved Hide resolved
crucible-concurrency/src/Cruces/CrucesMain.hs Outdated Show resolved Hide resolved
crucible-concurrency/src/Cruces/ExploreCrux.hs Outdated Show resolved Hide resolved
crucible-concurrency/src/Crucibles/CruciblesMain.hs Outdated Show resolved Hide resolved
crucible-concurrency/src/Crucibles/CruciblesMain.hs Outdated Show resolved Hide resolved
crucible-concurrency/src/Crucibles/CruciblesMain.hs Outdated Show resolved Hide resolved
crucible-concurrency/src/Crucibles/DPOR.hs Outdated Show resolved Hide resolved
crucible-concurrency/src/Crucibles/DPOR.hs Outdated Show resolved Hide resolved
const fn new() {
AtomicBool {
//...
id: crucible::concurrency::nonce(), //Not allowed in a const context
Copy link
Contributor

Choose a reason for hiding this comment

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

One option if you need to work around this is to change the field to id: Cell<Option<usize>> and assign a nonce on first use. I'm not sure whether this fits with the requirement that "the same resource has the same name across several executions", since I think it would make the naming depend on what order the first access to each variable happens in, which could depend on what order threads start up in.

Another option is to use a hack to run code on initialization, like we do for Bv: Rust side, Haskell side. The idea would be to define struct Nonce(u64); with an initializer containing a marker value like const NONCE_INIT: Nonce = Nonce(0xffff_ffff_ffff_ffff). Then recognize the nonce type and marker value in transConstVal and replace it with Crucible code to generate a fresh nonce (this might require a custom Crucible instruction). I'm not 100% sure whether this trick would work here, since for Nonce (unlike for Bv) it actually matters how many times the constant gets evaluated.

@spernsteiner
Copy link
Contributor

I only looked at the crux-mir parts, but overall those parts look good. My only real worry is the Index_RefPath case I commented on above.

@abakst abakst merged commit 9d0347a into master May 13, 2021
@abakst abakst deleted the concurrency branch May 13, 2021 16:39
brianhuffman pushed a commit to GaloisInc/saw-script that referenced this pull request May 15, 2021
brianhuffman pushed a commit to GaloisInc/saw-script that referenced this pull request May 15, 2021
brianhuffman pushed a commit to GaloisInc/saw-script that referenced this pull request May 15, 2021
Also add new packages to `cabal.project`.

Also update megaparsec version in freeze files to make
`crucible-syntax` happy.
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.

4 participants