[cfe-commits] [PATCH][Review request] correct initialization of AltiVec vectors

[Anton Yartsev]

Thanks for the review!
> Index: lib/CodeGen/CGExprConstant.cpp
>
> ===================================================================
> --- lib/CodeGen/CGExprConstant.cpp	(revision 124983)
> +++ lib/CodeGen/CGExprConstant.cpp	(working copy)
> @@ -907,12 +907,29 @@
>
>         llvm::SmallVector<llvm::Constant *, 4>  Inits;
>         unsigned NumElts = Result.Val.getVectorLength();
>
> -      for (unsigned i = 0; i != NumElts; ++i) {
> -        APValue&Elt = Result.Val.getVectorElt(i);
> -        if (Elt.isInt())
> -          Inits.push_back(llvm::ConstantInt::get(VMContext, Elt.getInt()));
> -        else
> -          Inits.push_back(llvm::ConstantFP::get(VMContext, Elt.getFloat()));
> +      if (Context.getLangOptions().AltiVec&&
> +          isa<CStyleCastExpr>(E)&&
> +          dyn_cast<CStyleCastExpr>(E)->getCastKind() == CK_VectorSplat) {
>
> Why check for a C-style cast here? Why not just check for a (general) CastExpr, which would also cover C-style casts (e.g., static_cast) and implicit casts?
The goal of this 'if' is to detect an AltiVec vector initialization with 
a single literal and I know that it should be exactly CStyleCastExpr:
Index: lib/Sema/SemaExpr.cpp
===================================================================
.....
+      if (PE->getNumExprs() == 1) {
.....
+        return BuildCStyleCastExpr(LParenLoc, TInfo, RParenLoc, Literal);
+      }
> Also, one little nit: if you've already done an isa<T>(X), you can just cast<T>(X) rather than dyn_cast<T>(X), and save ourselves some checking.
corrected!

Can I commit the patch?

-- 
Anton