[aarch64-port-dev ] Problem in MacroAssembler::needs_explicit_null_check

[Andrew Dinn]

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
-----------