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

[RFC] initialize singletons at runtime #81

Merged
merged 3 commits into from
Jan 25, 2018
Merged

[RFC] initialize singletons at runtime #81

merged 3 commits into from
Jan 25, 2018

Conversation

japaric
Copy link
Member

@japaric japaric commented Jan 23, 2018

This PR changes how the singletons are initialized. The advantage of initializing a singleton at
runtime is that the initial value is not constrained to only what works in const context. The
disadvantage is that this approach will use up more Flash. With the new approach, singleton!(_: [u8; 1024] = [0; 1024]) will invoke a memcpy at the caller site; with the old approach, the array
would have been initialized (zeroed) during startup.

The following code works with the new approach, but doesn't with the old one.

let x = 0;
let y = singleton!(_: u32 = x);

cc @therealprof @hannobraun

@therealprof
Copy link
Contributor

@japaric Hm, this is weird. I actually don't see a memcpy() but a memclr() when initialising with 0 although curiously it does store the initialisation data in the .data segment, too. It also uses the address where the data is stored so it seems it both clears it and then copies over the 0s from the .data segment in a loop.

Even more curious than that is that with the 0.4.2 code the initialisation data ends up in the .bss segment, rather than .data.

So as I see it the only real overhead is the inclusion of all the __aeabi_mem*() code which may or may not be required anyway; when using local arrays rustc will often include them anyway and then the damage is already done. 😉

One nice thing is that the new version allows to write:

let x: &'static mut [u32; 19] = singleton!(: [u32; 19] = mem::uninitialized()).unwrap();

(without unsafe!)

and that'll also do away with the __aeabi_mem*() calls.

Oh, and if you do something like:

let x: &'static mut [u32; 19] = singleton!(: [u32; 19] =
 [0,1,2,3,4,5,6,7,8,9,1,2,3,4,5,6,7,8,9]).unwrap();

rustc will happily generate lots of assembly calls to generate the values in registers and store them in the right spot, i.e. completely without __aeabi_mem*() calls:

 8000126:       2202            movs    r2, #2
 8000128:       9208            str     r2, [sp, #32]
 800012a:       6002            str     r2, [r0, #0]
 800012c:       2203            movs    r2, #3
 800012e:       9207            str     r2, [sp, #28]
 8000130:       6042            str     r2, [r0, #4]
 8000132:       2204            movs    r2, #4
 8000134:       9206            str     r2, [sp, #24]
 8000136:       6082            str     r2, [r0, #8]

but it will still put the appropriate amount of 0s in the .data segment:

Contents of section .data:
 20000004 00000000 00000000 00000000 00000000  ................
 20000014 00000000 00000000 00000000 00000000  ................
 20000024 00000000 00000000 00000000 00000000  ................
 20000034 00000000 00000000 00000000 00000000  ................
 20000044 00000000 00000000 00000000 00000000  ................

Anywhoo, I like the new version a lot better.

@therealprof
Copy link
Contributor

Oh, and the syntax mentioned above

let x = 0;
let y = singleton!(_: u32 = x);

doesn't work for me:

    |
169 | let y = singleton!(_: u32 = x);
    |                    ^

It works without the underscore though.

@japaric
Copy link
Member Author

japaric commented Jan 23, 2018

I actually don't see a memcpy() but a memclr()

Right, that makes sense. Since the initial values is all zeros.

curiously it does store the initialisation data in the .data segment

UntaggedOption.none should end up in .bss. If it doesn't it could be due to rust-lang/rust#41315.

(without unsafe!)

Err, that's wrong. You should require unsafe. I'll fix that.

@hannobraun
Copy link
Member

Hmm, additional flash usage is a problem, but being able to use non-const values is neat. I have the feeling which I prefer will always depend on the situation.

Anyway, I don't feel strongly about this. If I ever need to save those last few bytes to fit something into flash, I'll figure something out.

@therealprof
Copy link
Contributor

I don't see any additional flash usage compared to other approaches. The .data bug (rust-lang/rust#41315) is independent of this. In fact, the ability to initialise with mem::uninitialized() is a huge advantage of this approach.

@hannobraun
Copy link
Member

@therealprof Everything's great, then. Thanks for the clarification!

@japaric
Copy link
Member Author

japaric commented Jan 25, 2018

OK. Let's land this. Thanks for the feedback!

@homunkulus r+

@homunkulus
Copy link
Contributor

📌 Commit 032af4b has been approved by japaric

@homunkulus
Copy link
Contributor

⌛ Testing commit 032af4b with merge e7618ca...

japaric pushed a commit that referenced this pull request Jan 25, 2018
[RFC] initialize singletons at runtime

This PR changes how the singletons are initialized. The advantage of initializing a singleton at
runtime is that the initial value is not constrained to only what works in const context. The
disadvantage is that this approach will use up more Flash. With the new approach, `singleton!(_:
[u8; 1024] = [0; 1024])` will invoke a memcpy at the caller site; with the old approach, the array
would have been initialized (zeroed) during startup.

The following code works with the new approach, but doesn't with the old one.

``` rust
let x = 0;
let y = singleton!(_: u32 = x);
```

cc @therealprof @hannobraun
@homunkulus
Copy link
Contributor

☀️ Test successful - status-travis
Approved by: japaric
Pushing e7618ca to master...

@homunkulus homunkulus merged commit 032af4b into master Jan 25, 2018
@japaric japaric deleted the singleton branch January 25, 2018 20:40
adamgreig pushed a commit that referenced this pull request Jan 12, 2022
81: Add a __pre_init function to be called at the start of the reset handler r=korken89 a=yodaldevoid

I needed this for a project so I went ahead and rebased and tweaked @jcsoo 's changes from #71. I will admit, I got a little impatient waiting and that also played into why I did this. If either @jcsoo or @japaric have an issue with this PR, please let me know and I will remove it. I apologize for toes that I have stepped on.

Now onto the PR. This is possibly the simplest implementation that is possible. No nightly features were used in this implementation. Also, there is no hand-holding for the end user; if they want to mess with uninitialized statics, that is on them.

If you would like me to add more documentation, please let me know what you would like to see.

Fixes #17 

Co-authored-by: Jonathan Soo <jcsoo@agora.com>
Co-authored-by: Gabriel Smith <ga29smith@gmail.com>
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