[PATCH V2] Allow increment/decrement operators on integer vectors in OpenCL

[Peter N Lewis]

On 05/09/2013, at 12:30 , "Song, Ruiling" <ruiling.song at intel.com> wrote:
> Yes, make clean code is always good. What about below? Patch attached.

LGTM,
   Peter.

> 
> Index: lib/Sema/SemaExpr.cpp
> ===================================================================
> --- lib/Sema/SemaExpr.cpp (revision 190028)
> +++ lib/Sema/SemaExpr.cpp (working copy)
> @@ -8400,6 +8400,9 @@
>                                           IsInc, IsPrefix);
>   } else if (S.getLangOpts().AltiVec && ResType->isVectorType()) {
>     // OK! ( C/C++ Language Extensions for CBEA(Version 2.6) 10.3 )
> +  } else if(S.getLangOpts().OpenCL && ResType->isVectorType() &&
> +            ResType->getAs<VectorType>()->getElementType()->isIntegerType()) {
> +    // OpenCL V1.2 6.3 says dec/inc ops operate on integer vector types.
>   } else {
>     S.Diag(OpLoc, diag::err_typecheck_illegal_increment_decrement)
>       << ResType << int(IsInc) << Op->getSourceRange();
> 
> Ruiling
> -----Original Message-----
> From: Peter N Lewis [mailto:peter at stairways.com.au] 
> Sent: Thursday, September 05, 2013 11:58 AM
> To: cfe-commits at cs.uiuc.edu
> Cc: Song, Ruiling
> Subject: Re: [PATCH V2] Allow increment/decrement operators on integer vectors in OpenCL
> 
> On 05/09/2013, at 11:10 , "Song, Ruiling" <ruiling.song at intel.com> wrote:
>> This is a patch to allow increment decrement operators on integer vectors in OpenCL. Version 2 add a unit test.
>> Please help review. Thanks!
> 
> Would it be better to avoid the duplicate of the error diagnostic?  Something like:
> 
> --- lib/Sema/SemaExpr.cpp	(revision 190019)
> +++ lib/Sema/SemaExpr.cpp	(working copy)
> @@ -8401,9 +8401,19 @@
>   } else if (S.getLangOpts().AltiVec && ResType->isVectorType()) {
>     // 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();
> -    return QualType();
> +    bool bad = true;
> +    if(S.getLangOpts().OpenCL && ResType->isVectorType()) {
> +      // OpenCL V1.2 6.3 says dec/inc ops operate on integer vector types.
> +      QualType ElemTy = ResType->getAs<VectorType>()->getElementType();
> +      if(ElemTy->isIntegerType()) {
> +        bad = false;
> +      }
> +    }
> +    if (bad) {
> +      S.Diag(OpLoc, diag::err_typecheck_illegal_increment_decrement)
> +        << ResType << int(IsInc) << Op->getSourceRange();
> +      return QualType();
> +    }
>   }
>   // At this point, we know we have a real, complex or pointer type.
>   // Now make sure the operand is a modifiable lvalue.
> 
> It adds a boolean, I can't see an easy way to avoid that, but avoids the duplicated code of the diagnostic and return value.
>   Peter.
> 
> -- 
> Keyboard Maestro 6.2 now out - control Mail, reveal a file, format AppleScripts and more.
> 
> Keyboard Maestro <http://www.keyboardmaestro.com/> Macros for your Mac
> <http://www.stairways.com/>           <http://download.keyboardmaestro.com/>
> 
> <Allow_increment_decrement_ops_on_integer_vectors_in_OpenCL.diff>

-- 
Keyboard Maestro 6.2 now out - control Mail, reveal a file, format AppleScripts and more.

Keyboard Maestro <http://www.keyboardmaestro.com/> Macros for your Mac
<http://www.stairways.com/>           <http://download.keyboardmaestro.com/>