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

Gate spsc under the cas feature (Fixes #271) #272

Closed
wants to merge 1 commit into from
Closed

Gate spsc under the cas feature (Fixes #271) #272

wants to merge 1 commit into from

Conversation

Kriskras99
Copy link

The spsc module should only be built if the cas feature is enabled.
This fixes issue #271.

@TDHolmes
Copy link
Contributor

TDHolmes commented Mar 8, 2022

hmm, but spsc doesn't actually require cas operations. What we really need I think is to make atomic-polyfill a requirement for targets that have no atomic support (i.e., have full_atomic_polyfill cfg set). Does dropping optional = true like below fix it?

[target.riscv32i-unknown-none-elf.dependencies]
 atomic-polyfill = { version = "0.1.4" }

 [target.riscv32imc-unknown-none-elf.dependencies]
 atomic-polyfill = { version = "0.1.4" }

@Kriskras99
Copy link
Author

I would suggest adding an atomic load/store gate and hide it behind that.
See 2a8f38c.
In this way people who don't care for atomic types don't have to pull in 12 extra dependencies.

@Kriskras99
Copy link
Author

I have created an alternative pull request in #273 .
@TDHolmes you're solution also works but I'd like to avoid pulling in atomic_polyfill if I can.

bors bot added a commit that referenced this pull request May 12, 2022
293: unconditionally depend on atomic-polyfill for riscv32i target r=japaric a=japaric

due to a limitation in the llvm backend [1] the `riscv32i-unknown-none-elf` target lacks the `core::sync::atomic` API even though the actual hardware is capable of atomic loads and stores (`fence` instruction). thus, at this point in time, this target needs `atomic-polyfill` for a working SPSC implementation

[1]: knurling-rs/defmt#597 (comment)

fixes #271 
closes #272 
closes #273 

Co-authored-by: Jorge Aparicio <jorge.aparicio@ferrous-systems.com>
bors bot added a commit that referenced this pull request May 12, 2022
293: unconditionally depend on atomic-polyfill for riscv32i target r=japaric a=japaric

due to a limitation in the llvm backend [1] the `riscv32i-unknown-none-elf` target lacks the `core::sync::atomic` API even though the actual hardware is capable of atomic loads and stores (`fence` instruction). thus, at this point in time, this target needs `atomic-polyfill` for a working SPSC implementation

[1]: knurling-rs/defmt#597 (comment)

fixes #271 
closes #272 
closes #273 

Co-authored-by: Jorge Aparicio <jorge.aparicio@ferrous-systems.com>
@bors bors bot closed this in #293 May 12, 2022
@bors bors bot closed this in 02773a5 May 12, 2022
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.

2 participants