[PowerPC] Remove extend between shift and and

[nemanjai]

Summary

The SDAG will sometimes insert an extend between the shift and an and (immediate) even though the immediate is narrower than the narrow size. This does not allow us to produce a rotate instruction (such as rlwinm).
This patch just adds a combine to move the extend onto the and.

[lei]

LGTM

[amyk]

LGTM.

llvm/test/CodeGen/PowerPC/and-extend-combine.ll

3

Is an AIX run line also necessary?

[stefanp]

This patch makes sense to me. I just had one nit and one idea.

edit:
Sorry, I'm being difficult. :)

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

15505

It may be possible to also make this safe for sign extend.
At this point you are using

if(Imm >= maxUIntN(NarrowVT.getSizeInBits()))
  break;

to check if the immediate fits in the not yet extended range.
In the SIGN_EXTEND case you can do this by using

if (Op1.getOpcode() == ISD::SIGN_EXTEND && Imm >= maxUIntN(NarrowVT.getSizeInBits() - 1))
      break;

When you subtract 1 from the range you know that the sign bit on whatever we extend is going to be zero (because it is for the constant and we are doing an AND) so the sign extend becomes a zero extend and everything is fine.

I'm not 100% about this idea but it might work.

15512

nit:

NarrowOp.getOpcode()

is used a lot in this if statement. You may want to pull it out.

unsigned NarrowOpcode = NarrowOp.getOpcode();
if (NarrowOpcode != ISD::SHL && ...

[nemanjai]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

15505

I think this may be correct, but I can't envision a situation where we sign-extend a value and then AND it with a narrower value. The SDAG should always convert such "don't care" extends to ANY_EXTENDs.

15512

Makes sense. Not sure why I didn't do this.

llvm/test/CodeGen/PowerPC/and-extend-combine.ll

3

I don't think there is anything to be gained by adding it. The code is completely independent of the platform or even subtarget.

[stefanp]

I think this patch looks good.

I'm going to approve it and the nit that I had:

unsigned NarrowOpcode = NarrowOp.getOpcode();
if (NarrowOpcode != ISD::SHL && ...

can be addressed on commit.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

15505

Fair enough. Don't worry about it!

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

   else if (shouldInlineQuadwordAtomics())
     setMaxAtomicSizeInBitsSupported(128);
   else
     setMaxAtomicSizeInBitsSupported(64);
 
   setStackPointerRegisterToSaveRestore(isPPC64 ? PPC::X1 : PPC::R1);
 
   // We have target-specific dag combine patterns for the following nodes:
-  setTargetDAGCombine({ISD::ADD, ISD::SHL, ISD::SRA, ISD::SRL, ISD::MUL,
-                       ISD::FMA, ISD::SINT_TO_FP, ISD::BUILD_VECTOR});
+  setTargetDAGCombine({ISD::AND, ISD::ADD, ISD::SHL, ISD::SRA, ISD::SRL,
+                       ISD::MUL, ISD::FMA, ISD::SINT_TO_FP, ISD::BUILD_VECTOR});
   if (Subtarget.hasFPCVT())
     setTargetDAGCombine(ISD::UINT_TO_FP);
   setTargetDAGCombine({ISD::LOAD, ISD::STORE, ISD::BR_CC});
   if (Subtarget.useCRBits())
     setTargetDAGCombine(ISD::BRCOND);
   setTargetDAGCombine({ISD::BSWAP, ISD::INTRINSIC_WO_CHAIN,
                        ISD::INTRINSIC_W_CHAIN, ISD::INTRINSIC_VOID});
 

 SDValue PPCTargetLowering::PerformDAGCombine(SDNode *N,
                                              DAGCombinerInfo &DCI) const {
   SelectionDAG &DAG = DCI.DAG;
   SDLoc dl(N);
   switch (N->getOpcode()) {
   default: break;
   case ISD::ADD:
     return combineADD(N, DCI);
+  case ISD::AND: {
+    // We don't want (and (zext (shift...)), C) if C fits in the width of the
+    // original input as that will prevent us from selecting optimal rotates.
+    SDValue Op1 = N->getOperand(0);
+    SDValue Op2 = N->getOperand(1);
+    if ((Op1.getOpcode() != ISD::ZERO_EXTEND &&
+         Op1.getOpcode() != ISD::ANY_EXTEND) ||
+        !isa<ConstantSDNode>(Op2))
+      break;
+    SDValue NarrowOp = Op1.getOperand(0);
+    unsigned NarrowOpcode = NarrowOp.getOpcode();
+    if (NarrowOpcode != ISD::SHL && NarrowOpcode != ISD::SRL &&
+        NarrowOpcode != ISD::ROTL && NarrowOpcode != ISD::ROTR &&
+        NarrowOpcode != ISD::FSHL && NarrowOpcode != ISD::FSHR)
+      break;
+
+    uint64_t Imm = cast<ConstantSDNode>(Op2)->getZExtValue();
+    EVT NarrowVT = NarrowOp.getValueType();
+    // Make sure that the constant is narrow enough to fit in the narrow type.
+    if (Imm >= maxUIntN(NarrowVT.getSizeInBits()))
+      break;
+    SDValue ConstOp = DAG.getConstant(Imm, dl, NarrowVT);
+    SDValue NarrowAnd = DAG.getNode(ISD::AND, dl, NarrowVT, NarrowOp, ConstOp);
+    return DAG.getAnyExtOrTrunc(NarrowAnd, dl, N->getValueType(0));
+  }
   case ISD::SHL:
     return combineSHL(N, DCI);
   case ISD::SRA:
     return combineSRA(N, DCI);
   case ISD::SRL:
     return combineSRL(N, DCI);
   case ISD::MUL:
     return combineMUL(N, DCI);

llvm/test/CodeGen/PowerPC/and-extend-combine.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 2
+; RUN: llc < %s -mtriple=powerpc64le-unknown-unknown -ppc-asm-full-reg-names \
+; RUN:   -mcpu=pwr8 -verify-machineinstrs | FileCheck %s
+define dso_local ptr @foo(i32 noundef zeroext %arg, ptr nocapture noundef readonly %arg1, ptr noundef writeonly %arg2) local_unnamed_addr {
+; CHECK-LABEL: foo:
+; CHECK:       # %bb.0: # %bb
+; CHECK-NEXT:    rlwinm r3, r3, 31, 17, 28
+; CHECK-NEXT:    ldx r4, r4, r3
+; CHECK-NEXT:    clrldi r3, r4, 56
+; CHECK-NEXT:    add r3, r5, r3
+; CHECK-NEXT:    std r4, 0(r5)
+; CHECK-NEXT:    blr
+bb:
+  %i = lshr i32 %arg, 1
+  %i3 = and i32 %i, 32760
+  %i4 = zext i32 %i3 to i64
+  %i5 = getelementptr inbounds i8, ptr %arg1, i64 %i4
+  %i6 = load i64, ptr %i5, align 8
+  %i7 = and i64 %i6, 255
+  store i64 %i6, ptr %arg2, align 8
+  %i8 = getelementptr inbounds i8, ptr %arg2, i64 %i7
+  ret ptr %i8
+}