[MSP430] Optimize srl/sra in case of A >> (8 + N)

[efriedma]

lib/Target/MSP430/MSP430ISelLowering.cpp

954

I'm confused how the ZERO_EXTEND case works; you aren't even passing the right number of operands for a ZERO_EXTEND. (There is no ZERO_EXTEND_INREG because it's equivalent to an AND with the appropriate immediate. There's a helper function SelectionDAG::getZeroExtendInReg() to generate that for you.)

[asl]

lib/Target/MSP430/MSP430ISelLowering.cpp

954

Oh, you're right. It works here because the type operand for zext is essentially ignored and there is only a single version of i8 => i16 zext to match.

[krisb]

@asl , @efriedma Thanks!

Applied comments and extended condition with 'ShiftAmount == 8'

lib/Target/MSP430/MSP430ISelLowering.cpp

SDValue MSP430TargetLowering::LowerShifts(SDValue Op,
 
   // Expand non-constant shifts to loops:
   if (!isa<ConstantSDNode>(N->getOperand(1)))
     return Op;
 
   uint64_t ShiftAmount = cast<ConstantSDNode>(N->getOperand(1))->getZExtValue();
 
   // Expand the stuff into sequence of shifts.
-  // FIXME: for some shift amounts this might be done better!
-  // E.g.: foo >> (8 + N) => sxt(swpb(foo)) >> N
   SDValue Victim = N->getOperand(0);
 
+  if ((Opc == ISD::SRA || Opc == ISD::SRL) && ShiftAmount > 8) {
+    // E.g.: foo >> (8 + N) => sxt(swpb(foo)) >> N
+    Victim = DAG.getNode(ISD::BSWAP, dl, VT, Victim);
+    unsigned ExtOpc =
+        Opc == ISD::SRA ? ISD::SIGN_EXTEND_INREG : ISD::ZERO_EXTEND;
+    Victim = DAG.getNode(ExtOpc, dl, VT, Victim, DAG.getValueType(MVT::i8));
+    ShiftAmount -= 8;
+  }
+
   if (Opc == ISD::SRL && ShiftAmount) {
     // Emit a special goodness here:
     // srl A, 1 => clrc; rrc A
     Victim = DAG.getNode(MSP430ISD::RRCL, dl, VT, Victim);
     ShiftAmount -= 1;
   }
 
   while (ShiftAmount--)

test/CodeGen/MSP430/shifts.ll

 ; RUN: llc < %s | FileCheck %s
 target datalayout = "e-p:16:8:8-i8:8:8-i16:8:8-i32:8:8-n8:16"
 target triple = "msp430-elf"
 
 define zeroext i8 @lshr8(i8 zeroext %a, i8 zeroext %cnt) nounwind readnone {
 entry:
 ; CHECK-LABEL: lshr8:
+; CHECK: clrc
 ; CHECK: rrc.b
   %shr = lshr i8 %a, %cnt
   ret i8 %shr
 }
 
 define signext i8 @ashr8(i8 signext %a, i8 zeroext %cnt) nounwind readnone {
 entry:
 ; CHECK-LABEL: ashr8:
 ; CHECK: rra.b
   %shr = ashr i8 %a, %cnt
   ret i8 %shr
 }
 
 define zeroext i8 @shl8(i8 zeroext %a, i8 zeroext %cnt) nounwind readnone {
 entry:
 ; CHECK: shl8
 ; CHECK: add.b
   %shl = shl i8 %a, %cnt
   ret i8 %shl
 }
 
 define zeroext i16 @lshr16(i16 zeroext %a, i16 zeroext %cnt) nounwind readnone {
 entry:
 ; CHECK-LABEL: lshr16:
+; CHECK: clrc
 ; CHECK: rrc
   %shr = lshr i16 %a, %cnt
   ret i16 %shr
 }
 
 define signext i16 @ashr16(i16 signext %a, i16 zeroext %cnt) nounwind readnone {
 entry:
 ; CHECK-LABEL: ashr16:
 ; CHECK: rra
   %shr = ashr i16 %a, %cnt
   ret i16 %shr
 }
 
 define zeroext i16 @shl16(i16 zeroext %a, i16 zeroext %cnt) nounwind readnone {
 entry:
 ; CHECK-LABEL: shl16:
 ; CHECK: add
   %shl = shl i16 %a, %cnt
   ret i16 %shl
 }
+
+define i16 @ashr10_i16(i16 %a) #0 {
+entry:
+; CHECK-LABEL: ashr10_i16:
+; CHECK:      swpb  r12
+; CHECK-NEXT: sxt r12
+; CHECK-NEXT: rra r12
+; CHECK-NEXT: rra r12
+  %shr = ashr i16 %a, 10
+  ret i16 %shr
+}
+
+define i16 @lshr10_i16(i16 %a) #0 {
+entry:
+; CHECK-LABEL: lshr10_i16:
+; CHECK:      swpb  r12
+; CHECK-NEXT: mov.b r12, r12
+; CHECK-NEXT: clrc
+; CHECK-NEXT: rrc r12
+; CHECK-NEXT: rra r12
+  %shr = lshr i16 %a, 10
+  ret i16 %shr
+}