[LLVMdev] PointerIntPair causing trouble

[Stefanus Du Toit]

Hi Gabor,

On 3-May-09, at 4:57 PM, Gabor Greif wrote:
> Your analysis is perfectly correct. I Was AFK for the last two days,
> so I couldn't
> tell you this same story.

Thanks, glad I was on the right track :).

> The algorithm relies on the fact that the LSBit of the first "pointer"
> in User
> is zero. This is normally the case with VPtrs, which are "normally"
> placed there
> by ABIs. Originally, I just checked that bit verbatim, and later by
> means of
> PointerIntPair. The problem arose when PointerIntPair's layout
> changed.
> I think we could switch back without a loss.
>
> Btw. the alignment of pointers in a 32 bit machine is 4, so I do not
> understand
> how the traits define 3 free bits for them. Maybe sabre can explain
> that change
> which caused the breakage.

Well, that's actually been that way for a long time. What's changed  
recently is that the high available bits are actually used now,  
whereas before, if you only asked for one bit, only the LSB would be  
used.

As the (now inconsistent) comments in the code explain, the traits  
class assumed that pointers (not the pointees, the pointers  
themselves!) got allocated by malloc unless otherwise specialized, and  
that malloc will return 8-byte-aligned pointers. Obviously this is not  
true in general; the trait should probably be using AlignOf anyways,  
since pointers might even be less aligned than that on say 16-bit  
systems. I'm planning to clean this up.

> Is there any acute breakage left?

I don't believe so, at least not under Windows.

> I assume that there will be problems with layouts on compilers with
> non-standard
> ABIs that place the Vptr in a different location, or use other means
> of dispatch.
>
> I am happy to work on this if you now a serious platform that breaks
> my assumption.

I don't know of any. Out of curiosity, do you have any reference that  
guarantees that MSVC will always place a pointer at the start of a User?

As I mentioned I have a patch pending that I'll commit soon that adds  
a bunch of comments and will document this assumption in the code. I  
was also wondering if we could get rid of AugmentedUse. Is there any  
reason we don't just cast the actual end of the list (i.e. the result  
of getImpliedUser) to a PointerIntPair (or a void* passed to it)  
directly? Unless there's some specific reason why AugmentedUse exists,  
it seems to me like it only makes the code more confusing. If you're  
OK with this change I'll make it along with my comment changes. I  
wouldn't mind renaming getImpliedUser to getEndOfUses or something  
like that either...

I also want to add an assert to catch having a non-zero bit in there  
directly when the User is constructed in the future. I'm just not sure  
where to put it yet.

Stefanus

--
Stefanus Du Toit <stefanus.dutoit at rapidmind.com>
   RapidMind Inc.
   phone: +1 519 885 5455 x116 -- fax: +1 519 885 1463