[llvm-commits] [PATCH] update getPointerTo to handle multiple address spaces

[Duncan Sands]

Hi Micah,

>>>>
>>>> Once more you are trying to handle the case when the passed in value
>>>> is not a pointer.  It's just wrong, please stop trying to do this.
>>> [Villmow, Micah] There are places where there is no other way to
>> handle this without making the calling code very ugly.
>>
>> I've already fixed this in my own tree.  And an important thing to note
>> is that if you actually read how CastInst::isEliminableCastPair works it
>> is *wrong* to try to pass the intptr type for DstTy here, in fact for
>> correct operation when SrcTy and DstTy are pointers (eg ptrtoint +
>> inttoptr case) you have to pass in the intptr types for both SrcTy *and*
>> DstTy; when MidTy is a pointer (eg: the inttoptr + ptrtoint case) you
>> have to pass in the intptr type for MidTy.  What I'm saying is that by
>> just throwing in a random type without carefully analysing things you
>> are introducing a bug that will result in wrong transforms.
> [Villmow, Micah] I'm not so sure that you need to pass in both the SrcTy and the DstTy.
> I think isEliminableCastPair itself needs to change. IntPtrTy should not be an argument,
> but instead the datalayout should be passed in and the handling of the pointer should
> occur there. Does this sound like a better solution than trying to figure out what
> type is required at the call site?

it's just a question of layering.  Is Instructions.cpp allowed to depend on
DataLayout?  I think it's clear from the design of isEliminableCastPair that
in the past the answer was "no", otherwise why pass in the intptr type rather
than DataLayout itself?  With all the recent changes maybe it is OK now, I'm
not sure.

Ciao, Duncan.

>>
>> You seem to think that you can avoid analysing each and every use of
>> getIntPtrType by doing some tricks inside getIntPtrType.  It just isn't
>> true, there is no magic bullet.  So please stop trying, you are just
>> papering over problems.
>>
>> Ciao, Duncan.
>