pioasm: python output for RP2350 mov pindirs incorrect

[magy00]

word() is prepended with part of the mov() that it replaces:

$ cat test.pio
.program test
.pio_version 1
_:  mov pindirs, null
jmp _

$ pioasm -o python test.pio
# -------------------------------------------------- #
# This file is autogenerated by pioasm; do not edit! #
# -------------------------------------------------- #

import rp2
from machine import Pin
# ---- #
# test #
# ---- #

@rp2.asm_pio()
def test():
    wrap_target()
    label("0")
    mov(, null)             word(41059)                           # 0
    jmp("0")                              # 1
    wrap()

Ideally, this could look something like:

import rp2
from machine import Pin
# ---- #
# test #
# ---- #

@rp2.asm_pio()
def test():
    wrap_target()
    label(0)
    word(0xa063)  # mov(pindirs, null)  # 0
    jmp(0)                              # 1
    wrap()

[lurch]

ping @dpgeorge Just out of curiosity, does the PIO support in MicroPython include the new instructions / options added for RP2350? (which I guess would be "nicer" than just emitting word() functions?)

[magy00]

There's a thread in the forum with some hacks to add some of the features:
https://forums.raspberrypi.com/viewtopic.php?t=375677
https://github.com/orgs/micropython/discussions/16487

[dpgeorge]

does the PIO support in MicroPython include the new instructions / options added for RP2350?

No, we don't support the new RP2350 PIO options yet.

[magy00]

Maybe something like the following (completely untested):

diff --git tools/pioasm/python_output.cpp tools/pioasm/python_output.cpp
index bcaa8d6..73ecc4b 100644
--- tools/pioasm/python_output.cpp
+++ tools/pioasm/python_output.cpp
@@ -112,7 +112,7 @@ struct python_output : public output_format {
                 }
                 auto it = jmp_labels.find(i);
                 if (it != jmp_labels.end()) {
-                    fprintf(out, "    label(\"%s\")\n", it->second.c_str());
+                    fprintf(out, "    label(%s)\n", it->second.c_str());
                 }
                 fprintf(out, "    %s # %d\n", disassemble(jmp_labels, inst, program.sideset_bits_including_opt.get(), program.sideset_opt).c_str(), i);
                 if (i == (uint)program.wrap) {
@@ -176,9 +176,9 @@ struct python_output : public output_format {
                     label = it->second;
                 }
                 if (arg1)
-                    op_guts(conditions[arg1] + ", \"" + label +"\"");
+                    op_guts(conditions[arg1] + ", " + label);
                 else
-                    op_guts("\"" + label + "\"");
+                    op_guts(label);
                 break;
             }
             case 0b001: {
@@ -254,8 +254,7 @@ struct python_output : public output_format {
                 uint operation = arg2 >> 3u;
                 if (source.empty() || dest.empty() || operation == 3) {
                     invalid = true;
-                }
-                if (dest == source && (arg1 == 1 || arg2 == 2) && operation == 0) {
+                } else if (dest == source && (arg1 == 1 || arg2 == 2) && operation == 0) {
                     op("nop");
                     op_guts("");
                 } else {
@@ -309,8 +308,9 @@ struct python_output : public output_format {
         }
         if (invalid) {
             op("word");
-            ss << std::hex;
-            op_guts(std::to_string(inst));
+            std::string guts;
+            guts << "0x" << std::hex << std::setfill('0') << std::setw(4) << inst;
+            op_guts(guts);
         }
         uint delay = ((uint) inst >> 8u) & 0x1f;
         ss << std::left << std::setw(9);

[lurch]

I suspect that we need to keep the string-quoting around labels? See the example at https://docs.micropython.org/en/latest/rp2/tutorial/pio.html#an-example

[magy00]

It's documented in the MicroPython docs:

label(label)
Define a label called label at the current location. label can be a string or integer.

The way it's currently implemented, it could be any object (so one could use built-in function next without quotes). The integer doesn't need to match the address, but since it currently does, it would probably be relatively safe to use.