[cfe-dev] improved vector bool/pixel support

[Anton Yartsev]

On 26.07.2010 18:12, Douglas Gregor wrote:
> On Jul 20, 2010, at 8:11 PM, Anton Yartsev wrote:
>
>    
>>      
>>> On Jul 6, 2010, at 1:46 PM, Douglas Gregor wrote:
>>>
>>>
>>>        
>>>>> I'm happy to defer to Doug's opinion here.  However, I thought that the idea of a canonical type was that it represent the structural behavior of the type.  Semantically there should be no difference between the sugared and desugared type.  If "altivec" vectors have more behavior than just how they print, I don't think it's just sugar, right?
>>>>>
>>>>>            
>>>> Do AltiVec vectors have more/different behavior from than GCC vectors? I honestly don't know. If they do, they need their own canonical types and we'll need to introduce appropriate implicit conversions between the two kinds of vectors. If the behavior is the same, then it's just sugar.
>>>>
>>>>          
>>> Yes, they do.  For example,
>>>
>>> my_altivec_vector x = {1} does a splat, not a zero fill.
>>>
>>> -Chris
>>>        
>> As I understand the conclusion is that AltiVec vectors need their own canonical type. So resending you the updated patch for review. The patch gives unique canonical type for AltiVec vectors and adds compatibility checking.
>>      
>
> Index: lib/AST/ASTContext.cpp
>
> ===================================================================
> --- lib/AST/ASTContext.cpp	(revision 108983)
> +++ lib/AST/ASTContext.cpp	(working copy)
> @@ -1570,10 +1570,7 @@
>     // If the element type isn't canonical, this won't be a canonical type either,
>     // so fill in the canonical type field.
>     QualType Canonical;
> -  if (!vecType.isCanonical() || (AltiVecSpec == VectorType::AltiVec)) {
> -    // pass VectorType::NotAltiVec for AltiVecSpec to make AltiVec canonical
> -    // vector type (except 'vector bool ...' and 'vector Pixel') the same as
> -    // the equivalent GCC vector types
> +  if (!vecType.isCanonical()) {
>       Canonical = getVectorType(getCanonicalType(vecType), NumElts,
>         VectorType::NotAltiVec);
>
> @@ -4200,6 +4197,28 @@
>
>            LHS->getNumElements() == RHS->getNumElements();
>   }
>
> +bool ASTContext::areCompatibleVectorTypes(QualType FirstVec,
> +                                          QualType SecondVec) {
> +
> +  assert(FirstVec->isVectorType()&&  SecondVec->isVectorType());
> +
> +  if (hasSameUnqualifiedType(FirstVec, SecondVec))
> +    return true;
> +
> +  // AltiVec vectors types are identical to equivalent GCC vector types
> +  const VectorType *First = FirstVec->getAs<VectorType>();
> +  const VectorType *Second = SecondVec->getAs<VectorType>();
> +  if ((((First->getAltiVecSpecific() == VectorType::AltiVec)&&
> +        (Second->getAltiVecSpecific() == VectorType::NotAltiVec)) ||
> +       ((First->getAltiVecSpecific() == VectorType::NotAltiVec)&&
> +        (Second->getAltiVecSpecific() == VectorType::AltiVec)))&&
> +      (First->getElementType() == Second->getElementType())&&
> +      (First->getNumElements() == Second->getNumElements()))
> +    return true;
> +
> +  return false;
> +}
> +
>
>
> Rather than comparing First->getElementType() == Second->getElementType(), I think you want to check that First->getElementType() and Second->getElementType() are compatible. At the very least, use hasSameType(), which will compare the canonical types of the element types.
>
> Index: lib/CodeGen/CodeGenFunction.h
> ===================================================================
> --- lib/CodeGen/CodeGenFunction.h	(revision 108983)
> +++ lib/CodeGen/CodeGenFunction.h	(working copy)
> @@ -1602,11 +1602,16 @@
>           assert(Arg != ArgEnd&&  "Running over edge of argument list!");
>           QualType ArgType = *I;
>
> -        assert(getContext().getCanonicalType(ArgType.getNonReferenceType()).
> -               getTypePtr() ==
> -               getContext().getCanonicalType(Arg->getType()).getTypePtr()&&
> -               "type mismatch in call argument!");
> -
> +        if (!ArgType.getNonReferenceType()->isVectorType() ||
> +            !Arg->getType()->isVectorType() ||
> +            !getContext().areCompatibleVectorTypes(
> +                                      ArgType.getNonReferenceType(),
> +                                      Arg->getType())) {
> +          assert(getContext().getCanonicalType(ArgType.getNonReferenceType()).
> +                 getTypePtr() ==
> +                 getContext().getCanonicalType(Arg->getType()).getTypePtr()&&
> +                 "type mismatch in call argument!");
> +        }
>           Args.push_back(std::make_pair(EmitCallArg(*Arg, ArgType),
>                                         ArgType));
>         }
>
> This won't work in general. There are many other places where CodeGen requires the canonical types to be equivalent, and the AST itself needs to express that equivalence. The best approach here would be to allow an implicit conversion from AltiVec vector types to the corresponding GCC vector types, and to perform that implicit conversion wherever needed---initialization, assignment, comparison, etc. Essentially, CodeGen should only require one change, to implement the aforementioned implicit conversion (as a simple bitcast), and everything else should work. If it doesn't, then Sema isn't doing its job of introducing the appropriate conversions.
>
> 	- Doug
Added code for an implicit conversion between GCC and AltiVec vectors to 
Sema. Also attached tests with the relaxed vector conversions turned 
off. Please review.

-- 
Anton

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: Altivec_GCC_compat.patch
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20100805/c8759819/attachment.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: test.c
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20100805/c8759819/attachment.c>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: test.cpp
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20100805/c8759819/attachment-0001.ksh>