[DAG] Remove isVectorClearMaskLegal() check from vector_build dagcombine

[mkuper]

Summary

This check currently doesn't seem to do anything useful on any in-tree target, and I don't think it makes sense to keep it:
On non-x86, it always evaluates to false, so we never hit the code path that creates the shuffle with zero.
On x86, it just forwards to isShuffleMaskLegal().

It's possible we should be checking isShuffleMaskLegal() for all the shuffles we generate here, but (a) that should not be x86-specific, and (b) I don't see any reason for it to be specific to shuffles with zero.

(Not deleting isVectorClearMaskLegal() itself, since there's one more DAGCombine that uses it.)

[uweigand]

I don't really have any comment on the code changes themselves, but the SystemZ testcase change is fine with me.

[RKSimon]

I agree that the UsesZeroVector test shouldn't be there but we should probably use a TLI.isShuffleMaskLegal(Mask, InVT1) test and not just blindly combine to a shuffle.

[mkuper]

In D24625#545635, @RKSimon wrote:

I agree that the UsesZeroVector test shouldn't be there but we should probably use a TLI.isShuffleMaskLegal(Mask, InVT1) test and not just blindly combine to a shuffle.

We already blindly combine to a shuffle in all the other cases, the "blend with 0" case was the only one that had a check. I'm not saying that's necessarily what we should be doing, but I'm not sure I really want to change that behavior right now.
(My main motivation for removing the check is wanting to regularize this before committing D24683, because special-casing zero there would have been much uglier. )

Anyway, I'll try and see if it affects any of our tests.

[mkuper]

In D24625#545669, @mkuper wrote:

Anyway, I'll try and see if it affects any of our tests.

I'm not seeing any change in LIT tests from adding the mask legality check for any shuffle - either here or in D24683.
I'd really prefer to make the smaller change here and remove the check for the zero case.

[mkuper]

ping

[RKSimon]

LGTM

[mkuper]

Thanks, Simon!

llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

if (VectorMask[i] == 1) {
       Mask[i] = ExtIndex;
       continue;
     }
 
     assert(VectorMask[i] == 2 && "Expected input to be from second vector");
     Mask[i] = Vec2Offset + ExtIndex;
   }
 
-  // Avoid introducing illegal shuffles with zero.
-  // TODO: This doesn't actually do anything smart at the moment.
-  // We should either delete this, or check legality for all the shuffles
-  // we create.
-  if (UsesZeroVector && !TLI.isVectorClearMaskLegal(Mask, VT))
-    return SDValue();
-
   // The type the input vectors may have changed above.
   InVT1 = VecIn1.getValueType();
 
   // If we already have a VecIn2, it should have the same type as VecIn1.
   // If we don't, get an undef/zero vector of the appropriate type.
   VecIn2 = getRightHandValue(DAG, dl, VecIn2, InVT1, UsesZeroVector);
   assert(InVT1 == VecIn2.getValueType() && "Unexpected second input type.");
 

llvm/trunk/test/CodeGen/AMDGPU/r600-export-fix.ll

 ; RUN: llc < %s -march=r600 -mcpu=cedar | FileCheck %s
 
 ;CHECK: EXPORT T{{[0-9]}}.XYZW
 ;CHECK: EXPORT T{{[0-9]}}.0000
 ;CHECK: EXPORT T{{[0-9]}}.0000
-;CHECK: EXPORT T{{[0-9]}}.0XYZ
+;CHECK: EXPORT T{{[0-9]}}.0YZW
 ;CHECK: EXPORT T{{[0-9]}}.XYZW
-;CHECK: EXPORT T{{[0-9]}}.YZ00
+;CHECK: EXPORT T{{[0-9]}}.XY00
 ;CHECK: EXPORT T{{[0-9]}}.0000
 ;CHECK: EXPORT T{{[0-9]}}.0000
 
 
 define amdgpu_vs void @main(<4 x float> inreg %reg0, <4 x float> inreg %reg1) {
 main_body:
   %0 = extractelement <4 x float> %reg1, i32 0
   %1 = extractelement <4 x float> %reg1, i32 1

llvm/trunk/test/CodeGen/SystemZ/vec-perm-13.ll

 ; CHECK-CODE: br %r14
 
 ; CHECK-VECTOR: .space  1                                        
 ; CHECK-VECTOR-NEXT: .space  1                                        
 ; CHECK-VECTOR-NEXT: .space  1                                        
 ; CHECK-VECTOR-NEXT: .space  1                                        
 ; CHECK-VECTOR-NEXT: .byte   6
 ; CHECK-VECTOR-NEXT: .byte   7
-; CHECK-VECTOR-NEXT: .byte   16
-; CHECK-VECTOR-NEXT: .byte   17
+; CHECK-VECTOR-NEXT: .byte   22
+; CHECK-VECTOR-NEXT: .byte   23
 ; CHECK-VECTOR-NEXT: .space  1                                        
 ; CHECK-VECTOR-NEXT: .space  1                                        
 ; CHECK-VECTOR-NEXT: .space  1                                        
 ; CHECK-VECTOR-NEXT: .space  1                                        
 ; CHECK-VECTOR-NEXT: .space  1                                        
 ; CHECK-VECTOR-NEXT: .space  1                                        
 ; CHECK-VECTOR-NEXT: .space  1                                        
 ; CHECK-VECTOR-NEXT: .space  1                                        
 
   %elt = extractelement <4 x i16> %x, i32 3
   %vec1 = insertelement <4 x i16> undef, i16 %elt, i32 2
   %vec2 = insertelement <4 x i16> %vec1, i16 0, i32 3
   ret <4 x i16> %vec2
 }