On 26/07/17 17:30, Roman Kennke wrote:
Thanks for the analysis and proposal.
I find the bitwise magic a bit too magical:bool
Well personal mileage does vary ...
Why cast to long? Isn't intptr_t basically the same, but more explicitely pointer-sized?
Sure, if the left shift right shift directly on an intptr_t does the right thing then use it instead.
How does shifting left then right 'make sure they are set again' ? Do we want them all set? Then simply loffset |= 0xffff << 48; should do it, no?
Yes, we want them all set so the result becomes a proper negative 2's complement number. We know the 55th bit is 1 so left shift and arithmetic right shift by 8 will ensure bits 56-63 are set. Why not use an or? Because loading the constant would take more instructions than lsh, rsh.
Also, it seems we want to enclose this in if (UseShenandoahGC) Oops, yes! I had that in the version I started writing into the original reply but then lost it when I transcribed between posts and re-edited!
I think we actually need: if (UseShenandoahGC && (hi == 0x00ffL || hi == 0xffffL)) { ... return ((loffset + adj) != 0); } else { return true; }
And if not, then we fall through. Is this the intention?
No, see above. We only continue to the range check when we have a positive top 16 bits because this is just a normal user space pointer which requires a normal pointer comparison against the heap.
Does AArch64 define _LP64 ?
Yes, it does.
I don't much like littering shared code with #ifdef SOMEARCH... Maybe this stuff should be moved to src/cpu somewhere? Preferably, yes. In practice, however, there are a few such occurrences in shared code. My concern was not to relocate the ifdef but just to make it clearer what was happening on AArch64 and why.
regards, Andrew Dinn -----------