Neotron-Compute / Neotron-Pico-BIOS

BIOS for the Neotron Pico
GNU General Public License v3.0
15 stars 5 forks source link

pio::InstructionOperands::encode() is const since pio 0.2.1 #29

Closed jannic closed 1 year ago

thejpster commented 1 year ago

Do the binaries stay the same?

jannic commented 1 year ago

Good question, I only tested that VGA still works, but of course I can also compare the binaries. I'll have a look later today.

jannic commented 1 year ago

The binaries do change, but it looks like that's just the rust compiler being really bad at keeping changes local. Different address offsets all over the place, and looking for any meaningful differences is too much work.

Instead, I temporarily changed the code to:

         let command = if raise_irq { 
             // This command sets IRQ 0 
             let i = pio::InstructionOperands::IRQ { 
                 clear: false, 
                 wait: false, 
                 index: 0, 
                 relative: false, 
             } 
             .encode(); 
             assert!(i == 0xc000); 
             i 
         } else { 
             // This command is a no-op (it moves Y into Y) 
             let i = pio::InstructionOperands::MOV { 
                 destination: pio::MovDestination::Y, 
                 op: pio::MovOperation::None, 
                 source: pio::MovSource::Y, 
             } 
             .encode(); 
             assert!(i == 0xa042); 
             i 
         } as u32; 

That ensures that the encode functions actually return the same values as were used previously.

If I change the expected values in the assert statements, compiling fails because those asserts are checked at compile time:

error[E0080]: could not evaluate static initializer
    --> src/vga/mod.rs:1305:4
     |
1305 |             assert!(i == 0xbad1);
     |             ^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: i == 0xbad1', src/vga/mod.rs:1305:13

So I'm quite sure that this patch doesn't change the calculated values.

thejpster commented 1 year ago

Works for me. Thanks!