IDE 1.8.6 / 1.8.7 Compilation Issues

I have a new Windows 10 Laptop and I hate it. One thing I have noticed is that things compiled on it are a little off … let me explain.

  • I copied the entire Arduino/libraries directory from my old machine to the new one.
  • I copied my ProjectABE installation from my old machine to the new.
  • I installed a new version of VSCode and the Arduino plugin.
  • I have fetched the latest code of my Blackjack game from GitHub onto both machines.

If I compile a HEX on the old machine and execute it within ProjectABE on either machine it works fines.
If I compile the same HEX on the new machine and execute it within ProjectABE it works on both machines however there are graphics glitches that I cannot explain as shown below:

ArduboyRecording

Note the ellipses that are on the right hand side of the graphics. They appear as vertical lines when in fact the code is rendering a simple pixel using the following code:


    uint8_t i = (this->counter / 15) % 4;

    for (uint8_t j = 0; j < i; j++) {
      
        arduboy.drawPixel(79 + (j * 2), 49);

    }

As I mentioned, I am using the same version of the Arduboy2 library so I am at a loss to explain what is going on.

Do you also use the same Arduino IDE version (or copied a portable version from old to new)?

Ah … my old machine is running 1.8.5 and the new 1.8.6

I have tried compiling the same code on 1.8.6 and 1.8.7 and both exhibit the problem on the new machine.

I then downgraded the machine to 1.8.5 and everything works properly.

I was aware that there were some issue with 1.8.6 but I thought people were unable to compile some programs. I never expected the issue to manifest itself in a subtle graphics error.

The drawPixel() code is assembly - I wonder if this is contributing to the issue? I also noticed that a few fillRect() calls were giving trouble elsewhere in the program and they use drawFastVLine() which in turn calls drawPixel().

I might flag @MLXXXp into this thread as he may know why the drawPixel() function might be programmatic. (I am assuming that the assembler is his code, maybe not).

No, it’s not. The assembly code added to optimise drawPixel() was written by @Dreamer3 (yyyc514 on GitHub). @Pharap fixed a bug and I also made a later fix.

My time is limited right now, so I may not get a chance to look at this problem for a while. If anyone else can look into it, I would appreciate it.

1 Like

actually it’s assembly with somemixed C.

A couple of weeks ago I looked into the drawPixel (and optimized the C bit a bit) as I noticed the compiler didn’t optimize out an unused pointer (didn’t solve that though)

@filmote if you zip up your project I’ll have a go and compare at it.

In case anyones wondering what’s optimized. I got rid of the else case and used a different masking technique (It will be featured in the next homemade package update)

if (color) {
    sBuffer[row_offset] |=   bit;
  } else {
    sBuffer[row_offset] &= ~ bit;
}

To:

  uint8_t data = sBuffer[row_offset] |= bit;
  if (!color) data ^= bit;
  sBuffer[row_offset] = data;

When you’ve got time again and picking up your Arduboy2 library, give me a ping and I’ll sort out all the optimisations I’ve added to simplify things.

2 Likes

Does that need to be sBuffer[row_offset] |= bit?
You’re writing it back anyway so it could just be sBuffer[row_offset] | bit, couldn’t it?

uint8_t data = sBuffer[row_offset] | bit;
if (!color) data ^= bit;
sBuffer[row_offset] = data;

(The compiler will probably optimise the write out at the SSA stage anyway, but better safe than sorry.)

1 Like

Ha! right missed that. Thanks for pointing that out.

1 Like

Its the Blackjack project from here > https://github.com/Press-Play-On-Tape/Blackjack

1 Like

Thanks! and… I found the glitch.

The compiler somehow decides to store bitshift_left array in ram rather then in progmem. But since the drawPixel assembly code reads from progmem, ‘random’ bitmask data is used instead.

I think the problem arises here

I don’t know if it’s an actual compiler glitch or incorrect use of C(++) @Pharap any ideas?

1 Like

Nice pickup … I am glad I could isolate for you!

2 Likes

Well, I can start with some information at least.
PROGMEM is actually implemented as __attribute__((__progmem__)).

It’s plausible that a newer GCC version has affected where the attribute can be placed, but I would hope not because that contradicts the docs.

It certainly sounds like a bug of some description, possibly in GCC rather than the Arduino IDE.

1 Like

I think I may have a nice alternative that may save a few bytes too at the cost of 1 more cycle. Will test it first though

1 Like

Now I think about it, I’m not sure the bsl variable is actually needed anyway.
Registers are values, not references, so copying the value of bitshift_left straight to z should have had the same effect without needing the intermediary variable.

The compiler will start to complain about it being read only and ‘+z’ is used.

I found a little more about the glitch. It looks like the compiler optimized out bitshift_left and uses BIT_SHIFT instead which used in Ardbitmap and stored in ram.

Edit:

Okay my idea works :smiley: the bit conversion code takes the same number of cycles, 1 instruction more but doesn’t need the byte array, saving 6 bytes.

   //bit = 1 << (y & 7)
    "ldi  %[bit], 1                    \n" bit = 1;
    "sbrc %[y], 1                      \n" if (y & _BV(1)) bit = 4;
    "ldi  %[bit], 4                    \n"
    "sbrc %[y], 0                      \n" if (y & _BV(0)) bit = bit << 1;
    "lsl  %[bit]                       \n"
    "sbrc %[y], 2                      \n" if (y & _BV(2)) bit = (bit << 4) | (bit >> 4);
    "swap %[bit]                       \n" 
2 Likes

I am not sure what you mean here but are you saying Ardbutmap is the issue? Or that using Ardbitmao contributes to the issue?

bitshift_left (arduboy2) and BIT_SHIFT (Ardbitmap) both contain the same data. only the 1st is in progmem and the 2nd in ram.

When reversing the code there is only bitshift_left in ram and no BIT_SHIFT at all.

When removing bitshift_left (because of using my alternative code) then after reversing the code there is only BIT_SHIFT in ram

Ok here’s the optimized drawPixel that works fine with 1.8.7 :

void Arduboy2Base::drawPixel(int16_t x, int16_t y, uint8_t color)
{
  #ifdef PIXEL_SAFE_MODE
  if (x < 0 || x > (WIDTH-1) || y < 0 || y > (HEIGHT-1))
  {
    return;
  }
  #endif

  uint16_t row_offset;
  uint8_t bit;

  asm volatile
  (
    // bit = 1 << (y & 7)
    "ldi  %[bit], 1                    \n" //bit = 1;
    "sbrc %[y], 1                      \n" //if (y & _BV(1)) bit = 4;
    "ldi  %[bit], 4                    \n"
    "sbrc %[y], 0                      \n" //if (y & _BV(0)) bit = bit << 1;
    "lsl  %[bit]                       \n"
    "sbrc %[y], 2                      \n" //if (y & _BV(2)) bit = (bit << 4) | (bit >> 4);
    "swap %[bit]                       \n" 
    //row_offset = y / 8 * WIDTH + x;
    "andi %A[y], 0xf8                  \n" // 
    "mul  %[width_offset], %A[y]       \n"
    "movw %[row_offset], r0            \n"
    "clr  __zero_reg__                 \n"
    "add  %A[row_offset], %[x]         \n"
#if WIDTH != 128
    "adc  %B[row_offset], __zero_reg__ \n" // only none 128 width can overflow
#endif
    : [row_offset]   "=&x" (row_offset),   // upper register (ANDI)
      [bit]          "=&d" (bit),          // upper register (LDI)
      [y]            "+d"  (y)             // upper register (ANDI), must be writable
    : [width_offset] "r"   ((uint8_t)(WIDTH/8)),
      [x]            "r"   ((uint8_t)x)
    :
  );
  uint8_t data = sBuffer[row_offset] | bit;
  if (!color) data ^= bit;
  sBuffer[row_offset] = data;
}
4 Likes

Can you create a GitHub PR for this? I’ll then apply it and make a new release.

If you don’t want to do a PR you can raise an issue with the code included. Or, I can just pull it from here. I can then apply your code and credit you.

I’ll create a PR

Edit:

created

1 Like