SDK 2.3.0 - pio_encoding still broken

[gmx-pico]

Following this PR #2958 pio_encoding fixes + added PIO_VERSION to platform_defs.h
it fixes #2946 hardware_pio: fix pio_encode_mov(pio_pindirs, ...) on RP2350
but it breaks other opcode encodings.

A simple test:

  printf( "mov osr,x          0x%4X (should be 0xA0E1) pio_osr=0x%1X\n", pio_encode_mov(pio_osr, pio_x), pio_osr );
  printf( "mov pindirs,x      0x%4X (should be 0xA061) pio_pindirs=0x%1X\n", pio_encode_mov(pio_pindirs, pio_x), pio_pindirs );
  printf( "mov pindirs_mov,x  0x%4X (should be 0xA061) pio_pindirs_mov=0x%1X\n", pio_encode_mov(pio_pindirs_mov, pio_x), pio_pindirs_mov );
  printf( "mov exec,x         0x%4X (should be 0xA081) pio_exec=0x%1X\n", pio_encode_mov(pio_exec, pio_x), pio_exec );
  printf( "mov exec_mov,x     0x%4X (should be 0xA081) pio_exec_mov=0x%1X\n", pio_encode_mov(pio_exec_mov, pio_x), pio_exec_mov );
  printf( "mov y,x            0x%4X (should be 0xA041) pio_y=0x%1X\n", pio_encode_mov(pio_y, pio_x), pio_y );

RP2350 SDK 2.3.0 results:

mov osr,x          0xA061 (should be 0xA0E1) pio_osr=0x7          << WRONG
mov pindirs,x      0xA061 (should be 0xA061) pio_pindirs=0x4      << OK
mov pindirs_mov,x  0xA061 (should be 0xA061) pio_pindirs_mov=0x3  << OK
mov exec,x         0xA061 (should be 0xA081) pio_exec=0x7         << WRONG
mov exec_mov,x     0xA061 (should be 0xA081) pio_exec_mov=0x4     << WRONG
mov y,x            0xA041 (should be 0xA041) pio_y=0x2            << OK

For comparison SDK 2.2.0

mov osr,x          0xA0E1 (should be 0xA0E1) pio_osr=0x7          << OK
mov pindirs,x      0xA081 (should be 0xA061) pio_pindirs=0x4      << WRONG
mov exec_mov,x     0xA081 (should be 0xA081) pio_exec_mov=0x4     << OK
mov y,x            0xA041 (should be 0xA041) pio_y=0x2            << OK

There are reports on forum that RP2040 is affected too: https://forums.raspberrypi.com/viewtopic.php?t=399120#p2380376
pio_encode_mov(pio_osr, pio_x) is actually generating mov exec,x

I think the problem arises from overlapping enum fields across different instructions, maybe separate enums (per instruction type) would be easier to manage.

[lurch]

I did a bit of investigation, and surprisingly it works fine in Debug mode (i.e. when #ifndef NDEBUG).

I did a bit more investigation, and tracked down that in Debug mode pio_osr and pio_exec have different integer values, because of the invalid parameter-checking stuff; and so everything works fine. But in Release mode (i.e. when NDEBUG is defined) pio_osr and pio_exec both have the integer value 0x7, and this means that the if (dest == pio_exec) dest = pio_exec_mov; line in pio_encode_mov() is incorrectly translating a pio_osr into a pio_exec_mov.

EDIT: Similarly (when NDEBUG is defined), pio_pindirs and pio_exec_mov both have the integer value 0x4 which means that the if (dest == pio_pindirs) dest = pio_pindirs_mov; line in pio_encode_mov() is incorrectly translating a pio_exec_mov into a pio_pindirs_mov.
And the combination of

    if (dest == pio_exec) dest = pio_exec_mov;
    if (dest == pio_pindirs) dest = pio_pindirs_mov;

means that a pio_exec gets translated intto a pio_exec_mov, and this then gets incorrectly translated into into a pio_pindirs_mov.
(Which in turn also means that pio_osr is getting translated to pio_pindirs_mov, which explains why you're getting three identical "WRONG" encodings in your report above.)

[kilograham]

ack - i guess a testcase is in order!