[AVR] No cli for SPWRITE on XMEGA

[tomtor]

From the XMEGA series manual:
To prevent corruption when updating the stack pointer from software,
a write to SPL will automatically disable interrupts
for up to four instructions or until the next I/O memory write.

This will save 6! instructions in every function with an FP on ALL Xmega devices and it
is compatible with gcc code generation for all those devices. It is also not very modern, because the first devices are from 2013. So this is really significant and it is a small change.

@benshi001 Note that I could not find any existing (SPWRITE) unit tests to which this variant could be added.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[benshi001]

From the XMEGA series manual: To prevent corruption when updating the stack pointer from software, a write to SPL will automatically disable interrupts for up to four instructions or until the next I/O memory write.

This will save 6! instructions in every function with an FP on ALL Xmega devices and it is compatible with gcc code generation for all those devices. It is also not very modern, because the first devices are from 2013. So this is really significant and it is a small change.

@benshi001 Note that I could not find any existing (SPWRITE) unit tests to which this variant could be added.

You are appreciated to give a link about how gcc handles this, would be better to gcc's source code lines.

[tomtor]

I prefer to not read GCC code in general, but regarding gcc output, compile any C fragment with many locals with gcc with eg -mmcu=attiny85 and -mmcu=attiny402 (an xmega cpu) and compare the prologue and epilogue of the different -S outputs.

extern const char fl[];

long loop()
{
  const char *s = fl;
  long sum = 1;
  long sum2 = 1;
  int sums[10];
  for (long l= sum; l > 0; l--) {
    while (*s++) {
      sum += *s;
      sum2 += *s+1;
      sums[l%10]++;
    }
  }
  for (int i=0; i < sizeof(sums)/sizeof(sums[0]); i++)
    sum += sums[i];
  return sum+sum2;
}

avr-gcc -S -Os loop.c -mmcu=attiny85

loop.s:

        in r28,__SP_L__
        in r29,__SP_H__
        sbiw r28,20
        in __tmp_reg__,__SREG__
        cli
        out __SP_H__,r29
        out __SREG__,__tmp_reg__
        out __SP_L__,r28
/* prologue: function */__

versus with -mmcu=attiny402

        in r28,__SP_L__
        in r29,__SP_H__
        sbiw r28,20
        out __SP_L__,r28
        out __SP_H__,r29
/* prologue: function */

Note that llvm allways generates the first variant. The same applies to the epilogue so the total difference is 6 extra instructions.

[benshi001]

To summarize. Please

1. Keep your original code structure is OK, due to the byte order issue.
2. No need to check if SPH is valid.
3. Use getIORegSPL/getIORegSPH to replace fixed 0x3d/0x3e.
4. Supplement a unit test in llvm/test/CodeGen/AVR/pseudo, something like SPWRITE.mir, refer to OUTWARr.mir for how tests are performed on different architectures.

[benshi001]

.

[benshi001]

@tomtor Sorry for my late response. I also work on llvm in my spare time without any payment. I can not serve reviewing jobs in time. For that reason, my two core points are

1. Stability (especially bug fixes) is much more important than optimization. I would rather sub optimized code is generated than broken code. So I quite concern if optimization for newer devices will harm older devices.
2. Fix defects against avr-gcc-7.3 tool chain, arduino still uses gcc-7.3 as its main tool chain.

[benshi001]

For the unit test to be supplemented, a contrast test between avr6 and avrxemaga3 is enough.

[tomtor]

For the unit test to be supplemented, a contrast test between avr6 and avrxemaga3 is enough.

I added avr128db28 and attiny1614. Note that we MUST test for SPH to prevent writing wrong assembly with -1 as destination, so I added that single test.

So we have 3 tests, a classic (atmega328), a tiny without SPH and some modern xmegas/tiny/avr

[tomtor]

@Patryk27 Must some action be taken to include this in Rust (nightly?)

[Patryk27]

@tomtor nah, your changes will be automatically included in the Rust's fork once there's a new LLVM release (I thiink minor releases count, so probably a couple of weeks or so? but I'm not 100% certain on the release cycle).

Sometimes commits are cherry-picked directly into the fork to be included earlier, but that's mostly for bug fixes (rust-lang#184).