[OpenCL patch] vector logical operations (not, and, or)

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[OpenCL patch] vector logical operations (not, and, or)

Tanya Lattner
This patch adds support (CodeGen and Sema) for logical NOT, logical AND, and logical OR for vector types. It also includes a test case for each operation.

Please review:




Thanks,
Tanya
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

vecLogicalOps.patch (7K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [OpenCL patch] vector logical operations (not, and, or) ** revised **

Tanya Lattner
Attaching a revised version of this patch with a few fixes:




Thanks,
Tanya

On Mar 11, 2011, at 10:44 AM, Tanya Lattner wrote:

> This patch adds support (CodeGen and Sema) for logical NOT, logical AND, and logical OR for vector types. It also includes a test case for each operation.
>
> Please review:
>
> <vecLogicalOps.patch>
>
> Thanks,
> Tanya_______________________________________________
> cfe-dev mailing list
> [hidden email]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

vecLogicalOps.patch (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [OpenCL patch] vector logical operations (not, and, or) ** revised **

Peter Collingbourne
Hi Tanya,

On Fri, Mar 11, 2011 at 04:59:30PM -0800, Tanya Lattner wrote:
> Attaching a revised version of this patch with a few fixes:

Comments below.

> +  const VectorType *VTy = lex->getType()->getAs<VectorType>();
> +  unsigned TypeSize = Context.getTypeSize(VTy->getElementType());
> +  if (TypeSize == Context.getTypeSize(Context.CharTy))
> +    return Context.getExtVectorType(Context.CharTy, VTy->getNumElements());
> +  else if (TypeSize == Context.getTypeSize(Context.ShortTy))
> +    return Context.getExtVectorType(Context.ShortTy, VTy->getNumElements());
> +  else if (TypeSize == Context.getTypeSize(Context.IntTy))
> +    return Context.getExtVectorType(Context.IntTy, VTy->getNumElements());
> +  else if (TypeSize == Context.getTypeSize(Context.Int128Ty))
> +    return Context.getExtVectorType(Context.Int128Ty, VTy->getNumElements());
> +  else if (TypeSize == Context.getTypeSize(Context.LongTy))
> +    return Context.getExtVectorType(Context.LongTy, VTy->getNumElements());
> +
> +  assert(TypeSize == Context.getTypeSize(Context.LongLongTy) &&
> +         "Unhandled vector element size in vector compare");
> +  return Context.getExtVectorType(Context.LongLongTy, VTy->getNumElements());

Isn't this essentially a reimplementation of getSignedVariant?
We should be able to reuse it here.

> -    } else {
> +    } else if (const ExtVectorType *EVT = resultType->getAs<ExtVectorType>()) {
> +      // Handle vectors.
> +      // ExtVector LNot returns the signed version of the input type.
> +      QualType EltTy =  EVT->getElementType();
> +      EltTy = EltTy->getAs<BuiltinType>()->getSignedVariant(Context);

What happens if EltTy is a floating point type?

> +      resultType = Context.getExtVectorType(EltTy, EVT->getNumElements());
> +      break;
> +    }
> +    else {

Style nitpick: should be '} else {'

>        return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr)
>          << resultType << Input->getSourceRange());
>      }
> Index: lib/AST/Type.cpp
> ===================================================================
> --- lib/AST/Type.cpp (revision 127366)
> +++ lib/AST/Type.cpp (working copy)
> @@ -36,6 +36,27 @@
>    return false;
>  }
>  
> +QualType BuiltinType::getSignedVariant(ASTContext& Ctx) const {

This function should be a member of Type so that it can be used on
integer and integer vector types.

> +  assert(isUnsignedInteger()
> +         && "Only unsigned integers have an unsigned variant");

It should also be able to cope with signed integer types, in which case
it would just return the original type back.  The test to use here is
hasIntegerRepresentation(), which is true if this type has an integer
representation of some sort, e.g., it is an integer type or a vector.

> +  switch (BuiltinTypeBits.Kind) {
> +    case Bool: return Ctx.BoolTy;
> +    case Char_U: return Ctx.CharTy;

This case should use Ctx.SignedCharTy (char could be signed or
unsigned, and specifically if we see a Char_U then char is unsigned).

>  Value *ScalarExprEmitter::VisitBinLAnd(const BinaryOperator *E) {
> +  
> +  // If this is vector-typed, just emit two vcmp nodes and a vector and.
> +  if (E->getType()->isVectorType()) {
> +    Value *LHS = Visit(E->getLHS());
> +    Value *RHS = Visit(E->getRHS());
> +    Value *Zero = llvm::ConstantAggregateZero::get(LHS->getType());
> +    LHS = Builder.CreateICmp(llvm::CmpInst::ICMP_NE, LHS, Zero, "cmp");
> +    RHS = Builder.CreateICmp(llvm::CmpInst::ICMP_NE, RHS, Zero, "cmp");
> +    LHS = Builder.CreateSExt(LHS, Zero->getType(), "sext");
> +    RHS = Builder.CreateSExt(RHS, Zero->getType(), "sext");
> +    return Builder.CreateAnd(LHS, RHS);

What about and'ing the results of the two icmp instructions, and sext'ing
that, i.e.:

LHS = Builder.CreateICmp(llvm::CmpInst::ICMP_NE, LHS, Zero, "cmp");
RHS = Builder.CreateICmp(llvm::CmpInst::ICMP_NE, RHS, Zero, "cmp");
Value *LAnd = Builder.CreateAnd(LHS, RHS);
return Builder.CreateSExt(LAnd, Zero->getType(), "sext");

>  Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) {
> +  
> +  // If this is vector-typed, just emit two vcmp nodes and a vector and.
> +  if (E->getType()->isVectorType()) {
> +    Value *LHS = Visit(E->getLHS());
> +    Value *RHS = Visit(E->getRHS());
> +    Value *Zero = llvm::ConstantAggregateZero::get(LHS->getType());
> +    LHS = Builder.CreateICmp(llvm::CmpInst::ICMP_NE, LHS, Zero, "cmp");
> +    RHS = Builder.CreateICmp(llvm::CmpInst::ICMP_NE, RHS, Zero, "cmp");
> +    LHS = Builder.CreateSExt(LHS, Zero->getType(), "sext");
> +    RHS = Builder.CreateSExt(RHS, Zero->getType(), "sext");
> +    return Builder.CreateOr(LHS, RHS);

Same here.

Thanks,
--
Peter
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev