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

[BEAM] Runtime initialization of static variables #112

Closed
wants to merge 1 commit into from

Conversation

japaric
Copy link
Member

@japaric japaric commented Apr 14, 2019

Please read #111 first, if you haven't already.

Rendered discussion document


"Why is the memory clobber required?" If the compiler reorders `X.write` to
after the `asm!` block `INTERRUPT0` could observe `X` in an uninitialized state.

Copy link
Contributor

@gnzlbg gnzlbg Apr 15, 2019

Choose a reason for hiding this comment

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

You might be slightly better off rewriting this example to use a core::mem::black_box(X.as_mut_ptr()) that has some guaranteed semantics, but AFAICT that would be an intrinsic that's a nop in the virtual machine, but somehow is guaranteed to disable some optimizations. Which optimizations exactly is a question that would need answering.

Otherwise, it feels that modeling this would require a Rust memory model that understands inline assembly.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have not been actively following that RFC but I recall reading that using black_box for memory safety was discouraged ("it should only be used for benchmarks", is what I recall). Is that no longer the case?

Copy link
Contributor

Choose a reason for hiding this comment

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

There is no RFC for a core::mem::black_box operation with guaranteed semantics. Somebody would need to write one. There is an RFC for a core::hint::black_box intrinsics open, but that is not what I was suggesting to use here.

Since inline assembly is not a part of the Rust language (as seen in the other threads, it's not even clear what volatile/memory as clobbers do / should do in Rust), I don't think discussing anything related to inline assembly is a good use of this working group's time at this point.


**Claim**: the compiler fence is sufficient to prevent misoptimization provided
that `ORDERING` is any of: `Ordering::SeqCst` `Ordering::AcqRel` or
`Ordering::Release`.
Copy link
Contributor

@gnzlbg gnzlbg Apr 15, 2019

Choose a reason for hiding this comment

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

Can the interrupt be executed before X.write(false) ? EDIT: I suppose the volatile read / writes to the mask prevent that?


From the RFC (rust-lang/rfcs#888) these were intended to be equivalent to C11's atomic_signal_fence, so something like this should work if Rust inherits C11 memory model.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can the interrupt be executed before X.write(false) ?

No, the handler won't run while the interrupt is masked or while interrupts are globally disabled. The volatile writes ensure that INTERRUPT0 is masked before interrupts are globally enabled.

an `X` in an uninitialized state. Is it allowed to do that?

- If yes, what additional constraints are required to prevent misoptimization?
Should `X` be initialized using a volatile write?
Copy link

@comex comex Apr 15, 2019

Choose a reason for hiding this comment

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

It doesn't need to be. Just for the record:

Copy link
Member

@RalfJung RalfJung May 5, 2019

Choose a reason for hiding this comment

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

If you did use a volatile write and a volatile read, it would remove the need for the memory clobber or compiler_fence,

I am not sure about this. Volatile accesses are ordered with respect to each other, but here you assume they are also ordered wrt. whatever will make it so that "interrupts can only happen after this point".

Using a volatile write but not a volatile read would be incorrect per Rust docs: "Memory accessed with read_volatile or write_volatile should not be accessed with non-volatile operations." I believe it would be legal in terms of LLVM semantics, though.

That's just a "should". It basically just means "that's likely incorrect and you should re-think what you are doing".

@RalfJung
Copy link
Member

Closing due to inactivity, see #169 for tracking.

@RalfJung RalfJung closed this Jul 18, 2019
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