[cfe-dev] improved vector bool/pixel support

[Anton Yartsev]

On 23.06.2010 10:04, Chris Lattner wrote:
> On Jun 18, 2010, at 6:46 PM, Anton Yartsev wrote:
>    
>>> My understanding is that 'vector bool' and 'vector pixel' are exclusive.  If so, the argument to getVectorType should be an enum instead of 3 bools (altivec, ispixel, isbool).  Can you refactor it to take an enum?
>>>
>>> -Chris
>>>
>>>
>>>        
>> Hi Chris,
>>
>> Yes, 'bool' and 'pixel' are not allowed to be placed together, but they are of a slightly different nature - 'pixel' is a type specifier and 'bool' is not, type specifier should be specified after 'vector bool'.
>> Sending you the fresh copy of the original patch and the refactored version with enum used.
>>      
> I applied the patch with several minor changes (e.g. fitting in 80 columns) here: r106619.  Note that the testcases didn't pass with the test applied.  I tweaked them to make them work, please verify that the test changes are what you intend.  Having the diagnostics print __pixel instead of pixel seems dubious.   Also, please make sure that future patches pass tests before sending in the patch.
>
>    
Sent unverified test versions, sorry.
> However, though I applied the patch I don't think that this hunk is right:
>
> @@ -1529,16 +1529,19 @@
>     // 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() || IsAltiVec || IsPixel) {
> -    Canonical = getVectorType(getCanonicalType(vecType),
> -      NumElts, false, false);
> +  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
> +    Canonical = getVectorType(getCanonicalType(vecType), NumElts,
> +      VectorType::NotAltiVec);
>
>
> The canonical type for the vector should not change its underlying structure, it should just remove "sugar".   Doug, if you could, I'd appreciate it if you could look at this and see if there is a better way to get the vector type compatibility stuff that this code seems to be going for.  I'm also ok with temporarily removing this hunk until a proper solution is found, please advise.
>
> -Chris
>
>    
Eliminated changes to the canonical type, added compatibility checking 
instead - patch is attached. Verified changed tests and ensured that 
patch have not affected common test results.

-- 
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/20100703/97104d95/attachment.ksh>