PoC Heap and random number generator implementations

[reinvantveer]

It compiles! Unfortunately, it doesn't work yet. I missed some stuff somewhere, don't quite know where yet. But the megacoinflip app doesn't flip coins unfortunately.

I isolated the problem to the part where it uses the Vec to store the coin flipping. It doesn't push (or pop?):

flipped.push(random_number & 1);
let heads_or_tails_tile_idx = flipped.pop().unwrap() + 1;

doesn't work yet. But I think I can use a hand in debugging this.

I'm a little unsure on how the heap() function from megadrive_sys works. How does it know where the bottom of the heap is, and the size of the heap??? I can't spot any initialization values for these parameters. I see it's in share/ldscripts/megadrive.x.

TODO:

• Move heap and allocator into a new megadrive-alloc crate
• Move rng module into a megadrive-util (???) crate. Suggestions for a better crate name appreciated
• Recompile the alloc crate to have the compiler provide it for m68k-unknown-linux-gnu target
• Replace crate no_mutex with either RefCell or Rc<RefCell> implementation
• Refactor Heap::init() to use the unicorn heap() method

[reinvantveer]

For some very interesting effects: do

        let coin = random_number & 1;
        flipped.push(coin);

        let heads_or_tails_tile_idx = match flipped.pop() {
            None => {coin + 1}
            Some(pushed) => {pushed + 1}
        };

This results in some kind of machine code dump from LLVM

# In Register Scavenger
# Machine code for function main: NoPHIs, TracksLiveness, NoVRegs, TiedOpsRewritten
Frame Objects:
  fi#0: size=12, align=4, at location [SP-52]
... [lots of dumping]
- operand 1:   implicit killed $ccr
LLVM ERROR: Found 1 machine code errors.
error: could not compile `megacoinflip`
...

[ricky26]

I think if you use statics instead of consts where you need mutability, this won't be needed.

[reinvantveer]

The trouble is that the const being returned is in a const fn

impl HoleList {
    pub const fn empty() -> HoleList {
        HoleList { ... }

that is rather tricky to refactor. It doesn't want to return a statically defined HoleList. So this would imply re-writing Philipp Oppermann's heap implementation I fear.

[ricky26]

You can populate statics from const fns too.

[reinvantveer]

This one is a little beyond me, I'm afraid:

error[E0658]: mutable references are not allowed in constant functions
  --> libs/megadrive-alloc/src/hole.rs:20:23
   |
20 |                 next: None,
   |                       ^^^^
   |
   = note: see issue #57349 <https://github.com/rust-lang/rust/issues/57349> for more information
   = help: add `#![feature(const_mut_refs)]` to the crate attributes to enable

I suspect this has to do with the mutable member next in the Hole struct

pub struct Hole {
    size: usize,
    next: Option<&'static mut Hole>,
}

But I'm stumped beyond this. Can you perhaps point me in the direction of a possible solution on this?

[parasyte]

You can workaround this by lazily initializing the static allocator instead of using const functions.

This is easy with UnsafeCell<Option<T>>: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=15c46e35b8e65885057048ce53e47e10 This example just assumes that Alloc::init() will not be interrupted.

[ricky26]

# In Register Scavenger
# Machine code for function main: NoPHIs, TracksLiveness, NoVRegs, TiedOpsRewritten
Frame Objects:
  fi#0: size=12, align=4, at location [SP-52]
... [lots of dumping]
- operand 1:   implicit killed $ccr
LLVM ERROR: Found 1 machine code errors.
error: could not compile `megacoinflip`
...

Oh, that's not good news. I did make some changes into how the CCR was used as a quick hack around the current codegen (it was 'forgetting' conditionals). I'm not sure when I'll find time to look into this. 😢

[reinvantveer]

Oh, that's not good news. I did make some changes into how the CCR was used as a quick hack around the current codegen (it was 'forgetting' conditionals). I'm not sure when I'll find time to look into this.

I understand. So this would be the "condition code register" and it's been disabled in the LLVM version we use? Is that correct?
Perhaps you can point me to the associated commits you made? I can't make promises on understanding any of it, but I'm just curious.

Ah I see it's in ricky26/llvm-project@d7a0d7c

[reinvantveer]

Oh, that's not good news. I did make some changes into how the CCR was used as a quick hack around the current codegen (it was 'forgetting' conditionals). I'm not sure when I'll find time to look into this.

I understand. So this would be the "condition code register" and it's been disabled in the LLVM version we use? Is that correct?
Perhaps you can point me to the associated commits you made? I can't make promises on understanding any of it, but I'm just curious.

Ah I see it's in ricky26/llvm-project@d7a0d7c

I think we can let this go for now, we can work around this. I've opened a new issue in #8

[ricky26]

Oh, that's not good news. I did make some changes into how the CCR was used as a quick hack around the current codegen (it was 'forgetting' conditionals). I'm not sure when I'll find time to look into this.

I understand. So this would be the "condition code register" and it's been disabled in the LLVM version we use? Is that correct?
Perhaps you can point me to the associated commits you made? I can't make promises on understanding any of it, but I'm just curious.

Ah I see it's in ricky26/llvm-project@d7a0d7c

It's not that the register has been disabled. The 'implicit killed' means that the register has been clobbered at that instruction (overwritten, effectively). It's a compiler bug - it shouldn't've been overwritten.

[ricky26]

Sorry for taking so long to look at this again.

I want to make some time to try and figure out what's going on with the invalid codegen but I'm not sure I'll be able to find the time soon. I'd also really like to figure out what the core issue with CCR codegen is and try and get that upstreamed instead.

[reinvantveer]

Sorry for taking so long to look at this again.

I want to make some time to try and figure out what's going on with the invalid codegen but I'm not sure I'll be able to find the time soon. I'd also really like to figure out what the core issue with CCR codegen is and try and get that upstreamed instead.

Thanks ever so much for mentoring me through this. It's quite the journey!
The megacoinflip still isn't flipping though, unfortunately. I'm kinda missing the debugging tools to figure this one out, there's no convenient stack trace or panic info to see what's missing or going wrong... This one needs a very close read, or some more debug tools like a RetroArch/BlastEm gdb session or something, or maybe even a panic handler that shows some debug info on screen.

[ricky26]

I've been using BlastEm's built-in gdb prompt for a lot of the testing I've done. Generally I've had to view a disassembly of my outputs along-side though since the built-in prompt doesn't read debug information - so not for the feint of heart!

[reinvantveer]

I've been using BlastEm's built-in gdb prompt for a lot of the testing I've done. Generally I've had to view a disassembly of my outputs along-side though since the built-in prompt doesn't read debug information - so not for the feint of heart!

Hmmm I'm going to see whether I can cook up something slightly less involved than that, having looked at a GDB debug session only once before. Also this reminds me: some people do amazing things with debugging hardware - you may possible have seen https://hackaday.io/project/1507-usb-megadrive-devkit as an open hardware/software project using the GDB remote serial debug interface, which totally blew my mind.

[parasyte]

This can potentially introduce Undefined Behavior.

• RefCell::borrow_mut() will panic at runtime if it is already borrowed: https://doc.rust-lang.org/core/cell/struct.RefCell.html#method.borrow_mut
• GlobalAlloc is not allowed to panic: https://doc.rust-lang.org/core/alloc/trait.GlobalAlloc.html#safety

The panic can occur with re-entrant allocations or unfortunately timed interrupt handlers which need to allocate. These cases are unlikely, but it is better to use UnsafeCell directly and add a critical section to protect against interrupts.

It is not enough to just say "re-entrant allocations and interrupt handlers that allocate are Undefined Behavior" because you know someone is going to do it anyway, and more importantly these things do not have to be Undefined Behavior.

[reinvantveer]

I managed to write a compiling allocator based on unsafe cell interior mutability, but unfortunately this now runs into #4. I stored this in a separate branch of my fork, in https://github.com/reinvantveer/rust-mega-drive/tree/unsafe_cell_linked_list_allocator (diff in reinvantveer#1) but I'm not too confident that the 32-bit multiplication issue can be resolved. I simply lack the LLVM experience to have any notion on how to make that work, or if it even can.

I'm going to try if this happens with a simple bump allocator as you showed in the playground example, and hope that this will result in at least a minimalistic heap implementation. Otherwise, we'll just have to go heapless 😁

[reinvantveer]

I'm going to close this one, I'll look into a heap allocator at some point in time again, but I made the mistake of pushing all of my commits to my default branch instead of a feature branch. I'm going to save my work in a branch and pull my main branch even with the main branch here again to clean things up.

Maybe there's a way to implement some kind of interior mutable type without the 32-bit mutliplication problem popping up, but at the moment I'm not seeing it unfortunately.