[aarch64-port-dev ] RFR: Optimise addressing of card table byte map base

[Andrew Haley]

On 05/12/2014 02:21 PM, Edward Nevill wrote:
> Hi,
> 
> The following patch optimises the addressing of the card table byte map from
> 
>  movz x, #N
>  movk x, #N << 16
>  movk x, #N << 32
> 
> 
> to
> 
>  adrp x, #N
> 
> The implementation is identical to the previous optimisation of addressing the safepoint polling page.
> 
> The card table byte map is always page aligned (and this is asserted in the code below).

Comments inline.

> 
> --- CUT HERE ---
> # HG changeset patch
> # User Edward Nevill edward.nevill at linaro.org
> # Date 1399900167 -3600
> #      Mon May 12 14:09:27 2014 +0100
> # Node ID 1a6e4b95d2689fc31c1715bf8bec786a7d66161f
> # Parent  6523308f9626004171794372e5577a0f6939b4df
> Optimise addressing of card table byte map base
> 
> diff -r 6523308f9626 -r 1a6e4b95d268 src/cpu/aarch64/vm/aarch64.ad
> --- a/src/cpu/aarch64/vm/aarch64.ad	Mon May 12 13:41:43 2014 +0100
> +++ b/src/cpu/aarch64/vm/aarch64.ad	Mon May 12 14:09:27 2014 +0100
> @@ -2565,6 +2565,15 @@
>      assert(off == 0, "assumed offset == 0");
>    %}
>  
> +  enc_class aarch64_enc_mov_byte_map_base(iRegP dst, immByteMapBase src) %{
> +    MacroAssembler _masm(&cbuf);
> +    address page = (address)$src$$constant;
> +    Register dst_reg = as_Register($dst$$reg);
> +    unsigned long off;
> +    __ adrp(dst_reg, Address(page, relocInfo::poll_type), off);

Is this reloc type correct?  It should be an external address, no?

What is the cost of this insn?

> +    assert(off == 0, "assumed offset == 0");
> +  %}
> +

> diff -r 6523308f9626 -r 1a6e4b95d268 src/cpu/aarch64/vm/macroAssembler_aarch64.cpp
> --- a/src/cpu/aarch64/vm/macroAssembler_aarch64.cpp	Mon May 12 13:41:43 2014 +0100
> +++ b/src/cpu/aarch64/vm/macroAssembler_aarch64.cpp	Mon May 12 14:09:27 2014 +0100
> @@ -94,7 +94,9 @@
>        offset = adr_page - pc_page;
>  
>        unsigned insn2 = ((unsigned*)branch)[1];
> -      if ((address)target == os::get_polling_page()) {
> +      if ((jbyte *)target ==
> +		((CardTableModRefBS*)(Universe::heap()->barrier_set()))->byte_map_base ||
> +          (address)target == os::get_polling_page()) {
>  	assert(offset_lo == 0, "offset must be 0 for polling page");
>        } else if (Instruction_aarch64::extract(insn2, 29, 24) == 0b111001) {
>  	// Load/store register (unsigned immediate)
> @@ -182,7 +184,9 @@
>        uint64_t target_page = ((uint64_t)insn_addr) + offset;
>        target_page &= ((uint64_t)-1) << shift;
>        unsigned insn2 = ((unsigned*)insn_addr)[1];
> -      if ((address)target_page == os::get_polling_page()) {
> +      if ((jbyte *)target_page ==
> +		((CardTableModRefBS*)(Universe::heap()->barrier_set()))->byte_map_base ||
> +          (address)target_page == os::get_polling_page()) {
>  	return (address)target_page;
>        } else if (Instruction_aarch64::extract(insn2, 29, 24) == 0b111001) {
>  	// Load/store register (unsigned immediate)

I'm getting rather unhappy about these special cases in the reloc code.

Andrew.