[llvm] r203273 - [ADT] Update PointerIntPair to handle pointer types with more than 31 bits free.

[Jordan Rose]

On Mar 7, 2014, at 12:46 , David Blaikie <dblaikie at gmail.com> wrote:

> On Fri, Mar 7, 2014 at 11:57 AM, Jordan Rose <jordan_rose at apple.com> wrote:
>> 
>> On Mar 7, 2014, at 11:32, David Blaikie <dblaikie at gmail.com> wrote:
>> 
>> On Fri, Mar 7, 2014 at 11:19 AM, Jordan Rose <jordan_rose at apple.com> wrote:
>> 
>> Author: jrose
>> Date: Fri Mar  7 13:19:56 2014
>> New Revision: 203273
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=203273&view=rev
>> Log:
>> [ADT] Update PointerIntPair to handle pointer types with more than 31 bits
>> free.
>> 
>> Previously, the assertions in PointerIntPair would try to calculate the
>> value
>> (1 << NumLowBitsAvailable); the inferred type here is 'int', so if there
>> were
>> more than 31 bits available we'd get a shift overflow.
>> 
>> 
>> I'm guessing this was found by the static analyzer? (how/why? the
>> "bits free" would be an template parameter, not a runtime value - and
>> I assume we never even approach 32 bits free... seems sort of like a
>> false positive?)
>> 
>> Or do we really have such a pointer somewhere that's aligned to 4 GB?
>> 
>> 
>> I bet the analyzer could find it if such a path occurred, but the real
>> answer is much more mundane: I've been working on an internal tool that
>> actually has a Fixnum class like the one in the test case, which was
>> originally just being used in a PointerUnion (alongside a real pointer). I
>> tried using the same Fixnum<31> with a PointerIntPair, and Clang gave me a
>> warning (-Wshift-op-overflow, I think).
> 
> Still a little confused - what kind of pointer were you going to fit
> in the PointerIntPair? (& if you couldn't fit a pointer there, why not
> just use the int directly and skip the pair)

It's actually

using Offset = Fixnum<31>;
using OffsetOrRecord = PointerUnion<Offset, Record *>;

// added today
using OffsetAndShouldForce = PointerIntPair<Offset, 1, bool>;

On a 64-bit system this is the same amount of space as std::pair<Offset, bool>, but on a 32-bit system it fits in 32 bits. *shrug* I guess you're right that I should just bit-mangle it myself.

Jordan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140307/48ee9ba2/attachment.html>