[PATCH] D154785: [AVR] Expand all non-8-bit shifts

[Ben Shi via Phabricator via llvm-commits]

benshi001 added inline comments.


================
Comment at: llvm/lib/Target/AVR/AVRShiftExpand.cpp:54
       continue;
-    if (I.getType() != Type::getInt32Ty(Ctx))
-      // Only expand plain i32 types.
+    if (I.getType() == Type::getInt8Ty(Ctx))
+      // Only expand non-8-bit shifts, since 8-bit-shifts are expanded directly
----------------
I suggest we also exclude int16, since we have already defined pseudo instructions for all int16 shifts, such as

```
def Asr16 : ShiftPseudo<(outs DREGS
                         : $dst),
                        (ins DREGS
                         : $src, GPR8
                         : $cnt),
                        "# Asr16 PSEUDO", [(set i16
                                            : $dst, (AVRasrLoop i16
                                                     : $src, i8
                                                     : $cnt))]>;
```

Could you please have a try, that if we change to
```
if (I.getType() == Type::getInt8Ty(Ctx) || I.getType() == Type::getInt16Ty(Ctx))
```
can it also fix your issues in rust ?


================
Comment at: llvm/test/CodeGen/AVR/shift-expand.ll:91
 
-; This function isn't either, although perhaps it should.
 define i24 @shl24(i24 %value, i24 %amount) addrspace(1) {
 ; CHECK-LABEL: @shl24(
----------------
It would be better to also add `lshr` and `ashr` for `i40` and `i24`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154785