6805: Decompiled conditionals are hard to read

[philpem]

Describe the bug Decompiled conditionals involving the BLS and BHS/BCC branches produce hard-to-read decompilations.

To Reproduce Disassemble and decompile the following code bytes using the 6805 core.

            0487 5c              INCX
            0488 a3 03           CPX        #0x3
            048a 23 ef           BLS        LAB_047b
            048c e6 59           LDA        cablePktHdr,i
            048e a3 07           CPX        #0x7
            0490 23 f3           BLS        LAB_0485
            0492 cc 03 d1        JMP        ResetTimerAndGoAround

Note that the conditionals are hard to read:

              i = i + 1;
              if ((bool)((i < 3) + (i == 3))) break;
              cVar1 = *(char *)(i + 0x59);
              if (!(bool)((i < 7) + (i == 7))) goto ResetTimerAndGoAround;

Expected behavior More readable comparison code, e.g.:

              if (i <= 3) break;
              cVar1 = *(char *)(i + 0x59);
              if (!(i <= 7) goto ResetTimerAndGoAround;

Or even better for the last one:

              if (i > 7) goto ResetTimerAndGoAround;

It would also be nice if the array accesses could be switched to use the C array-dereference form, but the pointer form is arguably correct:

//             048c e6 59           LDA        cablePktHdr,i
// becomes...
              cVar1 = *(char *)(i + 0x59);
// but is easier to read as -->
              cVar1 = cablePktHdr[i];

Environment (please complete the following information):

• OS: Linux (Mint 21.3 Virginia)
• Java Version: 17.0.10
• Ghidra Version: 11.1 DEV
• Ghidra Origin: Git build from master as of 2024-04-26 09:33 BST

Additional context This may happen on other processor modules, but I've mainly seen it on the 6805 one.

[GhidorahRex]

You can try replacing `local tmp = C + Z; with local tmp = C || Z; in the BLS definition and see if that helps. Normally we wouldn't use addition for conditionals. That seems like an odd syntax to me and the decompiler doesn't like it as much. Similar for the other conditionals.

[philpem]

@GhidorahRex - I can confirm this fixes the issue. CMP #4, BLS foo decompiles as (a < 5) - which is correct, and probably better for a for-loop than a <= (LE) comparison.

I've applied that patch to my local build - I can submit a PR if you like but it's only a couple of lines (one for the BLS instruction and the same change in BHS/BCC)

[GhidorahRex]

Glad it worked for you! I've already got a change ready to go :-)