[CodeGen] Don't cast and use SizeTy instead of Int32Ty when constructing {extract,insert} vector element instructions.

[Bigcheese]

Summary

With this patch the following code is able to be correctly optimized.

#include <x86intrin.h>
#include <immintrin.h>
#include <avxintrin.h>
#include <avx2intrin.h>
#include <cstdio>
#include <cinttypes>

__m128 bss4( const __m128 *ptr, size_t i, size_t j )
{
    float f = ptr[i][j];
    return (__m128) { f, f, f, f };
}

Previously an unneeded trunc + zext would be emitted.

[chandlerc]

This seems like a strict improvement. Is there ever a case where the incoming index size would not be SizeTy? I wonder if we should use its type directly, even if just for clarity, to indicate that the goal is to use whatever integer size we already have.

[Bigcheese]

This patch uses the incoming index's type unless it's creating a constant.

[Bigcheese]

Committed as r209942.

[chandlerc]

Cool, LGTM. Maybe add tests to verify that we use a narrow integer when an incoming index is a narrow integer variable. Submit whenever.

lib/CodeGen/CGBuiltin.cpp

static Value *EmitCommonNeonSISDBuiltinExpr(CodeGenFunction &CGF,
   assert(Int && "Generic code assumes a valid intrinsic");
 
   // Determine the type(s) of this overloaded AArch64 intrinsic.
   const Expr *Arg = E->getArg(0);
   llvm::Type *ArgTy = CGF.ConvertType(Arg->getType());
   Function *F = CGF.LookupNeonLLVMIntrinsic(Int, Modifier, ArgTy, E);
 
   int j = 0;
-  ConstantInt *C0 = ConstantInt::get(CGF.Int32Ty, 0);
+  ConstantInt *C0 = ConstantInt::get(CGF.SizeTy, 0);
   for (Function::const_arg_iterator ai = F->arg_begin(), ae = F->arg_end();
        ai != ae; ++ai, ++j) {
     llvm::Type *ArgTy = ai->getType();
     if (Ops[j]->getType()->getPrimitiveSizeInBits() ==
              ArgTy->getPrimitiveSizeInBits())
       continue;
 
     assert(ArgTy->isVectorTy() && !Ops[j]->getType()->isVectorTy());
Value *CodeGenFunction::EmitCommonNeonBuiltinExpr(
   }
   case NEON::BI__builtin_neon_vld1_dup_v:
   case NEON::BI__builtin_neon_vld1q_dup_v: {
     Value *V = UndefValue::get(Ty);
     Ty = llvm::PointerType::getUnqual(VTy->getElementType());
     Ops[0] = Builder.CreateBitCast(Ops[0], Ty);
     LoadInst *Ld = Builder.CreateLoad(Ops[0]);
     Ld->setAlignment(cast<ConstantInt>(Align)->getZExtValue());
-    llvm::Constant *CI = ConstantInt::get(Int32Ty, 0);
+    llvm::Constant *CI = ConstantInt::get(SizeTy, 0);
     Ops[0] = Builder.CreateInsertElement(V, Ld, CI);
     return EmitNeonSplat(Ops[0], CI);
   }
   case NEON::BI__builtin_neon_vld2_lane_v:
   case NEON::BI__builtin_neon_vld2q_lane_v:
   case NEON::BI__builtin_neon_vld3_lane_v:
   case NEON::BI__builtin_neon_vld3q_lane_v:
   case NEON::BI__builtin_neon_vld4_lane_v:
static Value *EmitAArch64TblBuiltinExpr(CodeGenFunction &CGF, unsigned BuiltinID,
   Function *F = CGF.CGM.getIntrinsic(Int, Ty);
   return CGF.EmitNeonCall(F, Ops, s);
 }
 
 Value *CodeGenFunction::vectorWrapScalar16(Value *Op) {
   llvm::Type *VTy = llvm::VectorType::get(Int16Ty, 4);
   Op = Builder.CreateBitCast(Op, Int16Ty);
   Value *V = UndefValue::get(VTy);
-  llvm::Constant *CI = ConstantInt::get(Int32Ty, 0);
+  llvm::Constant *CI = ConstantInt::get(SizeTy, 0);
   Op = Builder.CreateInsertElement(V, Op, CI);
   return Op;
 }
 
 Value *CodeGenFunction::vectorWrapScalar8(Value *Op) {
   llvm::Type *VTy = llvm::VectorType::get(Int8Ty, 8);
   Op = Builder.CreateBitCast(Op, Int8Ty);
   Value *V = UndefValue::get(VTy);
-  llvm::Constant *CI = ConstantInt::get(Int32Ty, 0);
+  llvm::Constant *CI = ConstantInt::get(SizeTy, 0);
   Op = Builder.CreateInsertElement(V, Op, CI);
   return Op;
 }
 
 Value *CodeGenFunction::
 emitVectorWrappedScalar8Intrinsic(unsigned Int, SmallVectorImpl<Value*> &Ops,
                                   const char *Name) {
   // i8 is not a legal types for AArch64, so we can't just use
   // a normal overloaed intrinsic call for these scalar types. Instead
   // we'll build 64-bit vectors w/ lane zero being our input values and
   // perform the operation on that. The back end can pattern match directly
   // to the scalar instruction.
   Ops[0] = vectorWrapScalar8(Ops[0]);
   Ops[1] = vectorWrapScalar8(Ops[1]);
   llvm::Type *VTy = llvm::VectorType::get(Int8Ty, 8);
   Value *V = EmitNeonCall(CGM.getIntrinsic(Int, VTy), Ops, Name);
-  Constant *CI = ConstantInt::get(Int32Ty, 0);
+  Constant *CI = ConstantInt::get(SizeTy, 0);
   return Builder.CreateExtractElement(V, CI, "lane0");
 }
 
 Value *CodeGenFunction::
 emitVectorWrappedScalar16Intrinsic(unsigned Int, SmallVectorImpl<Value*> &Ops,
                                    const char *Name) {
   // i16 is not a legal types for AArch64, so we can't just use
   // a normal overloaed intrinsic call for these scalar types. Instead
   // we'll build 64-bit vectors w/ lane zero being our input values and
   // perform the operation on that. The back end can pattern match directly
   // to the scalar instruction.
   Ops[0] = vectorWrapScalar16(Ops[0]);
   Ops[1] = vectorWrapScalar16(Ops[1]);
   llvm::Type *VTy = llvm::VectorType::get(Int16Ty, 4);
   Value *V = EmitNeonCall(CGM.getIntrinsic(Int, VTy), Ops, Name);
-  Constant *CI = ConstantInt::get(Int32Ty, 0);
+  Constant *CI = ConstantInt::get(SizeTy, 0);
   return Builder.CreateExtractElement(V, CI, "lane0");
 }
 
 Value *CodeGenFunction::EmitAArch64BuiltinExpr(unsigned BuiltinID,
                                                const CallExpr *E) {
   if (BuiltinID == AArch64::BI__clear_cache) {
     assert(E->getNumArgs() == 2 && "__clear_cache takes 2 arguments");
     const FunctionDecl *FD = E->getDirectCallee();
case NEON::BI__builtin_neon_vcvtd_f64_s64: {
     return Builder.CreateSIToFP(Ops[0], FTy);
   }
   case NEON::BI__builtin_neon_vpaddd_s64: {
     llvm::Type *Ty =
       llvm::VectorType::get(llvm::Type::getInt64Ty(getLLVMContext()), 2);
     Value *Vec = EmitScalarExpr(E->getArg(0));
     // The vector is v2f64, so make sure it's bitcast to that.
     Vec = Builder.CreateBitCast(Vec, Ty, "v2i64");
-    llvm::Value *Idx0 = llvm::ConstantInt::get(Int32Ty, 0);
-    llvm::Value *Idx1 = llvm::ConstantInt::get(Int32Ty, 1);
+    llvm::Value *Idx0 = llvm::ConstantInt::get(SizeTy, 0);
+    llvm::Value *Idx1 = llvm::ConstantInt::get(SizeTy, 1);
     Value *Op0 = Builder.CreateExtractElement(Vec, Idx0, "lane0");
     Value *Op1 = Builder.CreateExtractElement(Vec, Idx1, "lane1");
     // Pairwise addition of a v2f64 into a scalar f64.
     return Builder.CreateAdd(Op0, Op1, "vpaddd");
   }
   case NEON::BI__builtin_neon_vpaddd_f64: {
     llvm::Type *Ty =
       llvm::VectorType::get(llvm::Type::getDoubleTy(getLLVMContext()), 2);
     Value *Vec = EmitScalarExpr(E->getArg(0));
     // The vector is v2f64, so make sure it's bitcast to that.
     Vec = Builder.CreateBitCast(Vec, Ty, "v2f64");
-    llvm::Value *Idx0 = llvm::ConstantInt::get(Int32Ty, 0);
-    llvm::Value *Idx1 = llvm::ConstantInt::get(Int32Ty, 1);
+    llvm::Value *Idx0 = llvm::ConstantInt::get(SizeTy, 0);
+    llvm::Value *Idx1 = llvm::ConstantInt::get(SizeTy, 1);
     Value *Op0 = Builder.CreateExtractElement(Vec, Idx0, "lane0");
     Value *Op1 = Builder.CreateExtractElement(Vec, Idx1, "lane1");
     // Pairwise addition of a v2f64 into a scalar f64.
     return Builder.CreateFAdd(Op0, Op1, "vpaddd");
   }
   case NEON::BI__builtin_neon_vpadds_f32: {
     llvm::Type *Ty =
       llvm::VectorType::get(llvm::Type::getFloatTy(getLLVMContext()), 2);
     Value *Vec = EmitScalarExpr(E->getArg(0));
     // The vector is v2f32, so make sure it's bitcast to that.
     Vec = Builder.CreateBitCast(Vec, Ty, "v2f32");
-    llvm::Value *Idx0 = llvm::ConstantInt::get(Int32Ty, 0);
-    llvm::Value *Idx1 = llvm::ConstantInt::get(Int32Ty, 1);
+    llvm::Value *Idx0 = llvm::ConstantInt::get(SizeTy, 0);
+    llvm::Value *Idx1 = llvm::ConstantInt::get(SizeTy, 1);
     Value *Op0 = Builder.CreateExtractElement(Vec, Idx0, "lane0");
     Value *Op1 = Builder.CreateExtractElement(Vec, Idx1, "lane1");
     // Pairwise addition of a v2f32 into a scalar f32.
     return Builder.CreateFAdd(Op0, Op1, "vpaddd");
   }
   case NEON::BI__builtin_neon_vceqzd_s64:
   case NEON::BI__builtin_neon_vceqzd_f64:
   case NEON::BI__builtin_neon_vceqzs_f32:
Value *CodeGenFunction::EmitAArch64BuiltinExpr(unsigned BuiltinID,
   case NEON::BI__builtin_neon_vqdmlalh_s16:
   case NEON::BI__builtin_neon_vqdmlslh_s16: {
     SmallVector<Value *, 2> ProductOps;
     ProductOps.push_back(vectorWrapScalar16(Ops[1]));
     ProductOps.push_back(vectorWrapScalar16(EmitScalarExpr(E->getArg(2))));
     llvm::Type *VTy = llvm::VectorType::get(Int32Ty, 4);
     Ops[1] = EmitNeonCall(CGM.getIntrinsic(Intrinsic::aarch64_neon_sqdmull, VTy),
                           ProductOps, "vqdmlXl");
-    Constant *CI = ConstantInt::get(Int32Ty, 0);
+    Constant *CI = ConstantInt::get(SizeTy, 0);
     Ops[1] = Builder.CreateExtractElement(Ops[1], CI, "lane0");
 
     unsigned AccumInt = BuiltinID == NEON::BI__builtin_neon_vqdmlalh_s16
                                         ? Intrinsic::aarch64_neon_sqadd
                                         : Intrinsic::aarch64_neon_sqsub;
     return EmitNeonCall(CGM.getIntrinsic(AccumInt, Int32Ty), Ops, "vqdmlXl");
   }
   case NEON::BI__builtin_neon_vqshlud_n_s64: {
case NEON::BI__builtin_neon_vqdmlslh_laneq_s16: {
     Ops[2] = Builder.CreateExtractElement(Ops[2], EmitScalarExpr(E->getArg(3)),
                                           "lane");
     SmallVector<Value *, 2> ProductOps;
     ProductOps.push_back(vectorWrapScalar16(Ops[1]));
     ProductOps.push_back(vectorWrapScalar16(Ops[2]));
     llvm::Type *VTy = llvm::VectorType::get(Int32Ty, 4);
     Ops[1] = EmitNeonCall(CGM.getIntrinsic(Intrinsic::aarch64_neon_sqdmull, VTy),
                           ProductOps, "vqdmlXl");
-    Constant *CI = ConstantInt::get(Int32Ty, 0);
+    Constant *CI = ConstantInt::get(SizeTy, 0);
     Ops[1] = Builder.CreateExtractElement(Ops[1], CI, "lane0");
     Ops.pop_back();
 
     unsigned AccInt = (BuiltinID == NEON::BI__builtin_neon_vqdmlalh_lane_s16 ||
                        BuiltinID == NEON::BI__builtin_neon_vqdmlalh_laneq_s16)
                           ? Intrinsic::aarch64_neon_sqadd
                           : Intrinsic::aarch64_neon_sqsub;
     return EmitNeonCall(CGM.getIntrinsic(AccInt, Int32Ty), Ops, "vqdmlXl");
case NEON::BI__builtin_neon_vld1q_lane_v:
     Ops[0] = Builder.CreateLoad(Ops[0]);
     return Builder.CreateInsertElement(Ops[1], Ops[0], Ops[2], "vld1_lane");
   case NEON::BI__builtin_neon_vld1_dup_v:
   case NEON::BI__builtin_neon_vld1q_dup_v: {
     Value *V = UndefValue::get(Ty);
     Ty = llvm::PointerType::getUnqual(VTy->getElementType());
     Ops[0] = Builder.CreateBitCast(Ops[0], Ty);
     Ops[0] = Builder.CreateLoad(Ops[0]);
-    llvm::Constant *CI = ConstantInt::get(Int32Ty, 0);
+    llvm::Constant *CI = ConstantInt::get(SizeTy, 0);
     Ops[0] = Builder.CreateInsertElement(V, Ops[0], CI);
     return EmitNeonSplat(Ops[0], CI);
   }
   case NEON::BI__builtin_neon_vst1_lane_v:
   case NEON::BI__builtin_neon_vst1q_lane_v:
     Ops[1] = Builder.CreateBitCast(Ops[1], Ty);
     Ops[1] = Builder.CreateExtractElement(Ops[1], Ops[2]);
     Ty = llvm::PointerType::getUnqual(Ops[1]->getType());
case X86::BI__builtin_ia32_storelps: {
     llvm::Type *PtrTy = llvm::PointerType::getUnqual(Int64Ty);
     llvm::Type *VecTy = llvm::VectorType::get(Int64Ty, 2);
 
     // cast val v2i64
     Ops[1] = Builder.CreateBitCast(Ops[1], VecTy, "cast");
 
     // extract (0, 1)
     unsigned Index = BuiltinID == X86::BI__builtin_ia32_storelps ? 0 : 1;
-    llvm::Value *Idx = llvm::ConstantInt::get(Int32Ty, Index);
+    llvm::Value *Idx = llvm::ConstantInt::get(SizeTy, Index);
     Ops[1] = Builder.CreateExtractElement(Ops[1], Idx, "extract");
 
     // cast pointer to i64 & store
     Ops[0] = Builder.CreateBitCast(Ops[0], PtrTy);
     return Builder.CreateStore(Ops[1], Ops[0]);
   }
   case X86::BI__builtin_ia32_palignr: {
     unsigned shiftVal = cast<llvm::ConstantInt>(Ops[2])->getZExtValue();

lib/CodeGen/CGExpr.cpp

RValue CodeGenFunction::EmitLoadOfExtVectorElementLValue(LValue LV) {
 
   const llvm::Constant *Elts = LV.getExtVectorElts();
 
   // If the result of the expression is a non-vector type, we must be extracting
   // a single element.  Just codegen as an extractelement.
   const VectorType *ExprVT = LV.getType()->getAs<VectorType>();
   if (!ExprVT) {
     unsigned InIdx = getAccessedFieldNo(0, Elts);
-    llvm::Value *Elt = llvm::ConstantInt::get(Int32Ty, InIdx);
+    llvm::Value *Elt = llvm::ConstantInt::get(SizeTy, InIdx);
     return RValue::get(Builder.CreateExtractElement(Vec, Elt));
   }
 
   // Always use shuffle vector to try to retain the original program structure
   unsigned NumResultElts = ExprVT->getNumElements();
 
   SmallVector<llvm::Constant*, 4> Mask;
   for (unsigned i = 0; i != NumResultElts; ++i)
if (NumDstElts == NumSrcElts) {
       Vec = Builder.CreateShuffleVector(Vec, ExtSrcVal, MaskV);
     } else {
       // We should never shorten the vector
       llvm_unreachable("unexpected shorten vector length");
     }
   } else {
     // If the Src is a scalar (not a vector) it must be updating one element.
     unsigned InIdx = getAccessedFieldNo(0, Elts);
-    llvm::Value *Elt = llvm::ConstantInt::get(Int32Ty, InIdx);
+    llvm::Value *Elt = llvm::ConstantInt::get(SizeTy, InIdx);
     Vec = Builder.CreateInsertElement(Vec, SrcVal, Elt);
   }
 
   llvm::StoreInst *Store = Builder.CreateStore(Vec, Dst.getExtVectorAddr(),
                                                Dst.isVolatileQualified());
   Store->setAlignment(Dst.getAlignment().getQuantity());
 }
 
if (SanOpts->ArrayBounds)
     EmitBoundsCheck(E, E->getBase(), Idx, IdxTy, Accessed);
 
   // If the base is a vector type, then we are forming a vector element lvalue
   // with this subscript.
   if (E->getBase()->getType()->isVectorType()) {
     // Emit the vector as an lvalue to get its address.
     LValue LHS = EmitLValue(E->getBase());
     assert(LHS.isSimple() && "Can only subscript lvalue vectors here!");
-    Idx = Builder.CreateIntCast(Idx, Int32Ty, IdxSigned, "vidx");
     return LValue::MakeVectorElt(LHS.getAddress(), Idx,
                                  E->getBase()->getType(), LHS.getAlignment());
   }
 
   // Extend or truncate the index type to 32 or 64-bits.
   if (Idx->getType() != IntPtrTy)
     Idx = Builder.CreateIntCast(Idx, IntPtrTy, IdxSigned, "idxprom");
 

lib/CodeGen/CGExprScalar.cpp

if (E->getNumSubExprs() == 2 ||
     // for each elt
     //   n = extract mask i
     //   x = extract val n
     //   newv = insert newv, x, i
     llvm::VectorType *RTy = llvm::VectorType::get(LTy->getElementType(),
                                                   MTy->getNumElements());
     Value* NewV = llvm::UndefValue::get(RTy);
     for (unsigned i = 0, e = MTy->getNumElements(); i != e; ++i) {
-      Value *IIndx = Builder.getInt32(i);
+      Value *IIndx = llvm::ConstantInt::get(CGF.SizeTy, i);
       Value *Indx = Builder.CreateExtractElement(Mask, IIndx, "shuf_idx");
-      Indx = Builder.CreateZExt(Indx, CGF.Int32Ty, "idx_zext");
 
       Value *VExt = Builder.CreateExtractElement(LHS, Indx, "shuf_elt");
       NewV = Builder.CreateInsertElement(NewV, VExt, IIndx, "shuf_ins");
     }
     return NewV;
   }
 
   Value* V1 = CGF.EmitScalarExpr(E->getExpr(0));
Value *ScalarExprEmitter::VisitArraySubscriptExpr(ArraySubscriptExpr *E) {
   // integer value.
   Value *Base = Visit(E->getBase());
   Value *Idx  = Visit(E->getIdx());
   QualType IdxTy = E->getIdx()->getType();
 
   if (CGF.SanOpts->ArrayBounds)
     CGF.EmitBoundsCheck(E, E->getBase(), Idx, IdxTy, /*Accessed*/true);
 
-  bool IdxSigned = IdxTy->isSignedIntegerOrEnumerationType();
-  Idx = Builder.CreateIntCast(Idx, CGF.Int32Ty, IdxSigned, "vecidxcast");
   return Builder.CreateExtractElement(Base, Idx, "vecext");
 }
 
 static llvm::Constant *getMaskElt(llvm::ShuffleVectorInst *SVI, unsigned Idx,
                                   unsigned Off, llvm::Type *I32Ty) {
   int MV = SVI->getMaskValue(Idx);
   if (MV == -1)
     return llvm::UndefValue::get(I32Ty);

test/CodeGen/builtinshufflevector2.c

 // RUN: %clang -emit-llvm -S -o - %s | FileCheck %s
 
 typedef float float4 __attribute__((ext_vector_type(4)));
 typedef unsigned int uint4 __attribute__((ext_vector_type(4)));
 
 // CHECK-LABEL: define void @clang_shufflevector_v_v(
 void clang_shufflevector_v_v( float4* A, float4 x, uint4 mask ) {
 // CHECK: [[MASK:%.*]] = and <4 x i32> {{%.*}}, <i32 3, i32 3, i32 3, i32 3>
-// CHECK: [[I:%.*]] = extractelement <4 x i32> [[MASK]], i32 0
-// CHECK: [[E:%.*]] = extractelement <4 x float> [[X:%.*]], i32 [[I]]
+// CHECK: [[I:%.*]] = extractelement <4 x i32> [[MASK]], i{{[0-9]+}} 0
+// CHECK: [[E:%.*]] = extractelement <4 x float> [[X:%.*]], i{{[0-9]+}} [[I]]
 //
 // Here is where ToT Clang code generation makes a mistake.  
 // It uses [[I]] as the insertion index instead of 0.
 // Similarly on the remaining insertelement.
-// CHECK: [[V:%[a-zA-Z0-9._]+]] = insertelement <4 x float> undef, float [[E]], i32 0
+// CHECK: [[V:%[a-zA-Z0-9._]+]] = insertelement <4 x float> undef, float [[E]], i{{[0-9]+}} 0
 
-// CHECK: [[I:%.*]] = extractelement <4 x i32> [[MASK]], i32 1
-// CHECK: [[E:%.*]] = extractelement <4 x float> [[X]], i32 [[I]]
-// CHECK: [[V2:%.*]] = insertelement <4 x float> [[V]], float [[E]], i32 1
-// CHECK: [[I:%.*]] = extractelement <4 x i32> [[MASK]], i32 2
-// CHECK: [[E:%.*]] = extractelement <4 x float> [[X]], i32 [[I]]
-// CHECK: [[V3:%.*]] = insertelement <4 x float> [[V2]], float [[E]], i32 2
-// CHECK: [[I:%.*]] = extractelement <4 x i32> [[MASK]], i32 3
-// CHECK: [[E:%.*]] = extractelement <4 x float> [[X]], i32 [[I]]
-// CHECK: [[V4:%.*]] = insertelement <4 x float> [[V3]], float [[E]], i32 3
+// CHECK: [[I:%.*]] = extractelement <4 x i32> [[MASK]], i{{[0-9]+}} 1
+// CHECK: [[E:%.*]] = extractelement <4 x float> [[X]], i{{[0-9]+}} [[I]]
+// CHECK: [[V2:%.*]] = insertelement <4 x float> [[V]], float [[E]], i{{[0-9]+}} 1
+// CHECK: [[I:%.*]] = extractelement <4 x i32> [[MASK]], i{{[0-9]+}} 2
+// CHECK: [[E:%.*]] = extractelement <4 x float> [[X]], i{{[0-9]+}} [[I]]
+// CHECK: [[V3:%.*]] = insertelement <4 x float> [[V2]], float [[E]], i{{[0-9]+}} 2
+// CHECK: [[I:%.*]] = extractelement <4 x i32> [[MASK]], i{{[0-9]+}} 3
+// CHECK: [[E:%.*]] = extractelement <4 x float> [[X]], i{{[0-9]+}} [[I]]
+// CHECK: [[V4:%.*]] = insertelement <4 x float> [[V3]], float [[E]], i{{[0-9]+}} 3
 // CHECK: store <4 x float> [[V4]], <4 x float>* {{%.*}},
   *A = __builtin_shufflevector( x, mask );
 }
 
 // CHECK-LABEL: define void @clang_shufflevector_v_v_c(
 void clang_shufflevector_v_v_c( float4* A, float4 x, float4 y) {
 // CHECK: [[V:%.*]] = shufflevector <4 x float> {{%.*}}, <4 x float> {{%.*}}, <4 x i32> <i32 0, i32 4, i32 1, i32 5>
 // CHECK: store <4 x float> [[V]], <4 x float>* {{%.*}}