[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]

Could you please give a link to this manual ?

[tomtor]

https://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-8331-8-and-16-bit-AVR-Microcontroller-XMEGA-AU_Manual.pdf page 17 SPL description.

It is in every chip full manual in the section describing SPL

[benshi001]

Why restore the interrupt state before writing to SPL ?

[tomtor]

That is not my code. I wondered myself.

[tomtor]

Ah, from SEI in the manual:

Sets the Global Interrupt Enable (I) bit in SREG (Status Register). The instruction following SEI will be executed
before any pending interrupts.

[benshi001]

This two branches have common code, if would be better to be

if (not xmega)
  disable interrupt.
common code;
if (not xmega)
  restore interrupt.

[tomtor]

The second branch is the unmodified original code. I can change it as suggested.

[tomtor]

SPL and SPH order is also reversed, so I do not think this is possible, clear to read? EDIT: we could switch the order in the original code, that should not matter.

[benshi001]

It would be better to use STI.getIORegSPL() instead of fixed value.

[tomtor]

Original code, could change that

[benshi001]

It would be better to check if SPH is available here for devices with a small stack, for example

if (getIORegSPH() >= 0)
  ....

[tomtor]

Original code. EDIT: so the original code is broken for those devices and has always been that way?

[benshi001]

Is it applicable to all upper devices? should xmega3 be excluded ?

[tomtor]

It should apply to all

[benshi001]

Is it applicable to all upper devices? should xmega3 be excluded ?

[tomtor]

Xmega3 is included, all new devices have this.

[benshi001]

A unit test is needed for this change. Please see other tests for pseudo instructions at llvm/test/CodeGen/AVR/pseudo.

[tomtor]

None of those vary architecture?

[benshi001]

Please do not use git push -f when updating a PR, I need to trace if all of my concerns are fixed.

[benshi001]

just use push, more than one commits will be squashed.

[tomtor]

I never used push -f in this PR, but did indeed in earlier ones.

[benshi001]

just use push, more than one commits will be squashed.

[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.