[llvm-commits] [llvm-gcc-4.2] r54240 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp

[Mon P Wang]

Hi Matthijs,

Thanks for fixing my mistake.  To answer your question, it is doing  
something useless and potentially bad.  Assume the incoming pointer  
has an address space qualifier, we will throw it away with the bit  
cast and then call the intrinsic function that has be customized for  
the address space which would do an implicit cast to the address  
space.  I don't know what gcc does in this case but it could throw an  
error that implicit cast between address spaces are not allowed (it  
probably should if doesn't know the relationship between the address  
spaces).  What the code should do is to preserve the address space  
with something like

   const PointerType* PtrTy = cast<PointerType>(C[0]->getType());
   const Type *OrigTy = PtrTy->getElementType();
   const Type* Ty[2];
   Ty[0] = OrigTy;
   if (isa<PointerType>(Ty[0]))
     Ty[0] = TD.getIntPtrType();
   Ty[1] = PtrTy;
   C[0] = Builder.CreateBitCast(C[0], PointerType::get(Ty[0],
                                                      PtrTy- 
 >getAddressSpace()));
   C[1] = Builder.CreateIntCast(C[1], Ty[0], "cast");
   Result =
     Builder.CreateCall(Intrinsic::getDeclaration(TheModule, id, Ty, 2),
                        C, C + 2);

I'm not sure why we need the bit cast in the first place though if  
Ty[0] was not changed.   I'll send out a patch with this as I am doing  
some cleanup that Duncan suggested.

-- Mon Ping



On Jul 31, 2008, at 3:00 AM, Matthijs Kooijman wrote:

>> +    if (isa<PointerType>(Ty[0]))
>> +      Ty[0] = TD.getIntPtrType();
>> +    Ty[1] = C[0]->getType();
>> +
>> +    C[0] = Builder.CreateBitCast(C[0],  
>> PointerType::getUnqual(Ty[0]));
>> +    C[1] = Builder.CreateIntCast(C[1], Ty[0], "cast");
>> +    C[2] = Builder.CreateIntCast(C[2], Ty[0], "cast");
>>
>>     Result =
>>       Builder.CreateCall(Intrinsic::getDeclaration(TheModule,
>>                                                     
>> Intrinsic::atomic_cmp_swap,
>> -                                                   &Ty, 1),
>> +                                                   Ty, 2),
>>       C, C + 3);
>
> I was going to write a piece here about the weirdness of this code,  
> but
> halfway through this I suddenly saw what it was doing. Actually  
> makes a lot of
> sense once you understand it (and realize that getIntPtrType()  
> doesn't return
> a pointer-to-int type, but an int-that-fits-a-pointer type).
>
> One thing I am still wondering about: Doesn't the above code throw  
> away the
> address space qualifer of C[0], if any? Preserving that was the  
> orginal idea
> behind these changes, right? Or don't these builtin gcc function  
> support
> different address spaces?