[cfe-commits] [PATCH][Review request] ++/-- for AltiVec vectors

[Douglas Gregor]

On Nov 28, 2010, at 7:56 PM, Anton Yartsev wrote:

> Support of ++/-- for AltiVec vectors according to "C/C++ Language Extensions for CBEA(Version 2.6)" section 10.3

A few comments.

Index: lib/CodeGen/CGExprScalar.cpp
===================================================================
--- lib/CodeGen/CGExprScalar.cpp	(revision 120239)
+++ lib/CodeGen/CGExprScalar.cpp	(working copy)
@@ -1257,10 +1257,11 @@
     // An interesting aspect of this is that increment is always true.
     // Decrement does not have this property.
     NextVal = llvm::ConstantInt::getTrue(VMContext);
-  } else if (isa<llvm::IntegerType>(InVal->getType())) {
+  } else if (isa<llvm::IntegerType>(InVal->getType()) ||
+             ValTy->isVectorType() && ValTy->hasIntegerRepresentation()) {
     NextVal = llvm::ConstantInt::get(InVal->getType(), AmountVal);
-    
-    if (!ValTy->isSignedIntegerType())
+
+    if (!ValTy->hasSignedIntegerRepresentation())
       // Unsigned integer inc is always two's complement.
       NextVal = Builder.CreateAdd(InVal, NextVal, isInc ? "inc" : "dec");
     else {
@@ -1292,6 +1293,8 @@
       NextVal =
       llvm::ConstantFP::get(VMContext,
                             llvm::APFloat(static_cast<double>(AmountVal)));
+    else if (ValTy->isVectorType() && ValTy->hasFloatingRepresentation())
+      NextVal = llvm::ConstantFP::get(InVal->getType(), AmountVal);
     else {
       llvm::APFloat F(static_cast<float>(AmountVal));
       bool ignored;

This looks a bit tangled... please completely separate out the vector case, rather than lumping it in with the integer case.

Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp	(revision 120239)
+++ lib/Sema/SemaExpr.cpp	(working copy)
@@ -6931,6 +6931,8 @@
     if (PR.isInvalid()) return QualType();
     return CheckIncrementDecrementOperand(S, PR.take(), VK, OpLoc,
                                           isInc, isPrefix);
+  } else if (ResType->isVectorType() && S.getLangOptions().AltiVec) {
+    // OK! ( C/C++ Language Extensions for CBEA(Version 2.6) 10.3 )
   } else {
     S.Diag(OpLoc, diag::err_typecheck_illegal_increment_decrement)
       << ResType << int(isInc) << Op->getSourceRange();

Please check S.getLangOptions().AltiVec first, since the isVectorType() check is more expensive.

Could you add some test cases that ensure that ++ and -- on vectors work properly in C++? Things to consider:
	- Test that the result is an lvalue, such that you can assign to it,  assign to its components, etc.
	- Test that ++/-- work in templates, both with an arbitrary type 'T' that gets instantiated to an AltiVec vector, and when we have an AltiVec vector of dependent size (e.g., the size is N, a non-type template parameter of type unsigned).

	- Doug