[DAGCombine] Add (sext_in_reg (zext x)) -> (sext x) combine

[RKSimon]

Summary

Handle the case where a sign extension has ended up being split into separate stages (typically to get around vector legal ops) and a zext + sext_in_reg gets inserted.

[andreadb]

Hi Simon,
I only have one question. Otherwise the patch LGMT.

Thanks

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

7130–7148

Your new rule has a lot in common with the folding rules between lines [7130:7138]. You could potentially extend those folding rules to accomodate your extra case (ISD::ZERO_EXTEND). The only difference seems to be the check for EVTBits. All other checks (for legal operations) are identical.

[RKSimon]

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

7130–7148

Thanks Andrea, I did try to merge the two but I the extra EVTBits logic just made it difficult to grok.

[andreadb]

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

7130–7148

Ah I see. I think it is fine then.

LGTM.

Thanks!

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

if (DAG.ComputeNumSignBits(N0) >= VTBits-EVTBits+1)
     return N0;
 
   // fold (sext_in_reg (sext_in_reg x, VT2), VT1) -> (sext_in_reg x, minVT) pt2
   if (N0.getOpcode() == ISD::SIGN_EXTEND_INREG &&
       EVT.bitsLT(cast<VTSDNode>(N0.getOperand(1))->getVT()))
     return DAG.getNode(ISD::SIGN_EXTEND_INREG, SDLoc(N), VT,
                        N0.getOperand(0), N1);
 
   // fold (sext_in_reg (sext x)) -> (sext x)
   // fold (sext_in_reg (aext x)) -> (sext x)
   // if x is small enough.
   if (N0.getOpcode() == ISD::SIGN_EXTEND || N0.getOpcode() == ISD::ANY_EXTEND) {
     SDValue N00 = N0.getOperand(0);
     if (N00.getScalarValueSizeInBits() <= EVTBits &&
         (!LegalOperations || TLI.isOperationLegal(ISD::SIGN_EXTEND, VT)))
       return DAG.getNode(ISD::SIGN_EXTEND, SDLoc(N), VT, N00, N1);
   }
 
+  // fold (sext_in_reg (zext x)) -> (sext x)
+  // iff we are extending the source sign bit.
+  if (N0.getOpcode() == ISD::ZERO_EXTEND) {
+    SDValue N00 = N0.getOperand(0);
+    if (N00.getScalarValueSizeInBits() == EVTBits &&
+        (!LegalOperations || TLI.isOperationLegal(ISD::SIGN_EXTEND, VT)))
+      return DAG.getNode(ISD::SIGN_EXTEND, SDLoc(N), VT, N00, N1);
+  }
+
   // fold (sext_in_reg x) -> (zext_in_reg x) if the sign bit is known zero.
   if (DAG.MaskedValueIsZero(N0, APInt::getBitsSet(VTBits, EVTBits-1, EVTBits)))
     return DAG.getZeroExtendInReg(N0, SDLoc(N), EVT.getScalarType());
 
   // fold operands of sext_in_reg based on knowledge that the top bits are not
   // demanded.
   if (SimplifyDemandedBits(SDValue(N, 0)))
     return SDValue(N, 0);

test/CodeGen/X86/combine-sext-in-reg.ll

 
 ; fold sextinreg(zext(sext)) -> sext
 define <4 x i64> @sextinreg_zext_sext_v16i8_4i64(<16 x i8> %a0) {
 ; SSE-LABEL: sextinreg_zext_sext_v16i8_4i64:
 ; SSE:       # BB#0:
 ; SSE-NEXT:    pmovsxbq %xmm0, %xmm2
 ; SSE-NEXT:    psrld $16, %xmm0
 ; SSE-NEXT:    pmovsxbq %xmm0, %xmm1
-; SSE-NEXT:    psllq $32, %xmm2
-; SSE-NEXT:    pshufd {{.*#+}} xmm0 = xmm2[1,1,3,3]
-; SSE-NEXT:    psrad $31, %xmm2
-; SSE-NEXT:    pblendw {{.*#+}} xmm2 = xmm0[0,1],xmm2[2,3],xmm0[4,5],xmm2[6,7]
-; SSE-NEXT:    psllq $32, %xmm1
-; SSE-NEXT:    pshufd {{.*#+}} xmm0 = xmm1[1,1,3,3]
-; SSE-NEXT:    psrad $31, %xmm1
-; SSE-NEXT:    pblendw {{.*#+}} xmm1 = xmm0[0,1],xmm1[2,3],xmm0[4,5],xmm1[6,7]
 ; SSE-NEXT:    movdqa %xmm2, %xmm0
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: sextinreg_zext_sext_v16i8_4i64:
 ; AVX:       # BB#0:
 ; AVX-NEXT:    vpmovsxbq %xmm0, %ymm0
-; AVX-NEXT:    vpsllq $32, %ymm0, %ymm0
-; AVX-NEXT:    vpsrad $31, %ymm0, %ymm1
-; AVX-NEXT:    vpshufd {{.*#+}} ymm0 = ymm0[1,1,3,3,5,5,7,7]
-; AVX-NEXT:    vpblendd {{.*#+}} ymm0 = ymm0[0],ymm1[1],ymm0[2],ymm1[3],ymm0[4],ymm1[5],ymm0[6],ymm1[7]
 ; AVX-NEXT:    retq
   %1 = shufflevector <16 x i8> %a0, <16 x i8> undef, <4 x i32> <i32 0, i32 1, i32 2, i32 3>
   %2 = sext <4 x i8> %1 to <4 x i32>
   %3 = zext <4 x i32> %2 to <4 x i64>
   %4 = shl <4 x i64> %3, <i64 32, i64 32, i64 32, i64 32>
   %5 = ashr <4 x i64> %4, <i64 32, i64 32, i64 32, i64 32>
   ret <4 x i64> %5
 }