[PATCH] D152063: [AVR] Support left-rotations

Tue Jun 6 02:14:52 PDT 2023

benshi001 added a comment.

In D152063#4395330 <https://reviews.llvm.org/D152063#4395330>, @aykevl wrote:

> Sorry I only see this now, after it has been merged.
>
>> Curiously, `fshl.u16` (and larger types) work, similarly as all fshrs, which I think is related to the fact that `ROLBRd` is hard-coded to require `GPR8:$zero` in AVRInstrInfo.td.
>>
>> Now, I'm not really sure why it's been done this way and so I'm not 100% sure my approach to solving this problem is correct as well - but adjusting `ROLBRd` to work similarly as `RORBRd` seems to do the job.
>
> There is a reason, namely that the instruction uses `R1` and now doesn't say so in InstrInfo. I have added the use of `R1` in D117426 <https://reviews.llvm.org/D117426>. With the use removed, interrupts may be miscompiled because `R1` might not be saved/cleared/restored inside an interrupt when a ROLB pseudo instruction is used inside an interrupt.
>
> In other words, the zero register dependency is required for correctness and this patch will lead to miscompilation in some edge cases.
>
> Here is an example that miscompiles on current trunk: https://godbolt.org/z/6ha5rGqY3 (`r1` is used but not saved/cleared).

I have

1. made a fix, though inefficient, but correct
2. recover the test of avr-tiny.

I may need more time to find a proper fix to resolve both the crash and the incorrect generated ISR, but at least this solution is correct.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152063/new/

https://reviews.llvm.org/D152063