[PATCH] D76201: [TargetLowering] Only demand a rotation's modulo amount bits

Simon Pilgrim via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 16 08:42:14 PDT 2020


RKSimon added inline comments.


================
Comment at: llvm/test/CodeGen/AVR/rot.ll:7
 define i8 @rol8(i8 %val, i8 %amt) {
-  ; CHECK:      andi r22, 7
-
----------------
aykevl wrote:
> If I'm reading this IR, correctly, it does a rotate left just as the name implies.
> I think the transformation here is correct in that it will still produce the same output, but without the mask the rotate may take a lot longer. I think the AVR assembly could be written as the following pseudocode (assume 8-bit unsigned integers everywhere):
> 
> ```
> // r24 = val
> // r22 = amt
> def rol8(r24, r22):
>     r22 &= 7           // this mask is removed by this patch
>     if r22 == 0:
>         return r24     // return value (LBB0_2)
>     while 1:
>         r24 = (r24 << 1) | (r24 >> 7) // rotate r24 left by 1 (lsl, adc)
>         r22 -= 1
>         if r22 == 0:   // brne .LBB0_1
>             return r24 // .LBB0_2, ret
> ```
> 
> So if you're calling `rol8(x, 200)` it will take 200 iterations to rotate a number.
I think this is a fault in the AVR backend - ROTL/ROTR/FSHL/FSHR all assume modulo amounts, so AVRTargetLowering::LowerShifts should mask accordingly. I'll add this to the patch shortly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76201





More information about the llvm-commits mailing list