[aarch64-port-dev ] RFR: Add support for G1GC

[Andrew Haley]

On 03/18/2014 10:52 AM, Edward Nevill wrote:
> Hi,
> 
> The following patch adds support for G1GC.
> 
> This is disabled by default and only enabled with the -XX:+UseG1GC option.
> 
> I have tested this against JTreg hotspot and it gives no additional failures/errors.
> 
> OK to push?

One or two questions inline:

> --- CUT HERE ---
> exporting patch:
> # HG changeset patch
> # User Edward Nevill edward.nevill at linaro.org
> # Date 1395139429 0
> #      Tue Mar 18 10:43:49 2014 +0000
> # Node ID 53205a277e07e8be32c4592ba0982f7bc3817717
> # Parent  939480aaf1b23f1013de7bca05dd6a2c3cef3430
> Add support for G1GC
> 
> diff -r 939480aaf1b2 -r 53205a277e07 src/cpu/aarch64/vm/aarch64.ad
> --- a/src/cpu/aarch64/vm/aarch64.ad	Tue Mar 11 15:44:21 2014 +0000
> +++ b/src/cpu/aarch64/vm/aarch64.ad	Tue Mar 18 10:43:49 2014 +0000
> @@ -5112,6 +5112,19 @@
>  
>  // Store Instructions
>  
> +// Store CMS card-mark Immediate
> +instruct storeimmCM0(immI0 zero, memory mem)
> +%{
> +  match(Set mem (StoreCM mem zero));
> +
> +  ins_cost(MEMORY_REF_COST);
> +  format %{ "strb zr, $mem\t# byte" %}
> +
> +  ins_encode(aarch64_enc_strb0(mem));

Are you sure this isn't a store release?  I'm not sure.

> @@ -1866,47 +1866,47 @@
>  void LIR_Assembler::logic_op(LIR_Code code, LIR_Opr left, LIR_Opr right, LIR_Opr dst) {
>    
>    assert(left->is_single_cpu() || left->is_double_cpu(), "expect single or double register");
> -  if (left->is_single_cpu()) {
> -    assert (right->is_single_cpu() || right->is_constant(), "single register or constant expected");
> -    if (right->is_constant()
> -	&& Assembler::operand_valid_for_logical_immediate(true, right->as_jint())) {
> -
> -      switch (code) {
> -      case lir_logic_and: __ andw (dst->as_register(), left->as_register(), right->as_jint()); break;
> -      case lir_logic_or:  __ orrw (dst->as_register(), left->as_register(), right->as_jint()); break;
> -      case lir_logic_xor: __ eorw (dst->as_register(), left->as_register(), right->as_jint()); break;
> -      default: ShouldNotReachHere(); break;
> -      }
> -    } else {
> -      switch (code) {
> -      case lir_logic_and: __ andw (dst->as_register(), left->as_register(), right->as_register()); break;
> -      case lir_logic_or:  __ orrw (dst->as_register(), left->as_register(), right->as_register()); break;
> -      case lir_logic_xor: __ eorw (dst->as_register(), left->as_register(), right->as_register()); break;
> -      default: ShouldNotReachHere(); break;
> -      }
> -    }
> -  } else {
> -    assert (right->is_double_cpu() || right->is_constant(), "single register or constant expected");
> -    if (right->is_double_cpu()) {
> -      switch (code) {
> -      case lir_logic_and: __ andr(dst->as_register_lo(), left->as_register_lo(), right->as_register_lo()); break;
> -      case lir_logic_or:  __ orr (dst->as_register_lo(), left->as_register_lo(), right->as_register_lo()); break;
> -      case lir_logic_xor: __ eor (dst->as_register_lo(), left->as_register_lo(), right->as_register_lo()); break;
> -      default:
> -	ShouldNotReachHere();
> -	break;
> -      }
> -    } else {
> -      switch (code) {
> -      case lir_logic_and: __ andr(dst->as_register_lo(), left->as_register_lo(), right->as_jlong()); break;
> -      case lir_logic_or:  __ orr (dst->as_register_lo(), left->as_register_lo(), right->as_jlong()); break;
> -      case lir_logic_xor: __ eor (dst->as_register_lo(), left->as_register_lo(), right->as_jlong()); break;
> -      default:
> -	ShouldNotReachHere();
> -	break;
> -      }
> -    }
> -  }
> +  Register Rleft = left->is_single_cpu() ? left->as_register() :
> +                                           left->as_register_lo();
> +   if (dst->is_single_cpu()) {
> +     Register Rdst = dst->as_register();
> +     if (right->is_constant()) {
> +       switch (code) {
> +         case lir_logic_and: __ andw (Rdst, Rleft, right->as_jint()); break;
> +         case lir_logic_or:  __ orrw (Rdst, Rleft, right->as_jint()); break;
> +         case lir_logic_xor: __ eorw (Rdst, Rleft, right->as_jint()); break;
> +         default: ShouldNotReachHere(); break;
> +       }
> +     } else {
> +       Register Rright = right->is_single_cpu() ? right->as_register() :
> +                                                  right->as_register_lo();
> +       switch (code) {
> +         case lir_logic_and: __ andw (Rdst, Rleft, Rright); break;
> +         case lir_logic_or:  __ orrw (Rdst, Rleft, Rright); break;
> +         case lir_logic_xor: __ eorw (Rdst, Rleft, Rright); break;
> +         default: ShouldNotReachHere(); break;
> +       }
> +     }
> +   } else {
> +     Register Rdst = dst->as_register_lo();
> +     if (right->is_constant()) {
> +       switch (code) {
> +         case lir_logic_and: __ andr (Rdst, Rleft, right->as_jlong()); break;
> +         case lir_logic_or:  __ orr (Rdst, Rleft, right->as_jlong()); break;
> +         case lir_logic_xor: __ eor (Rdst, Rleft, right->as_jlong()); break;
> +         default: ShouldNotReachHere(); break;
> +       }
> +     } else {
> +       Register Rright = right->is_single_cpu() ? right->as_register() :
> +                                                  right->as_register_lo();
> +       switch (code) {
> +         case lir_logic_and: __ andr (Rdst, Rleft, Rright); break;
> +         case lir_logic_or:  __ orr (Rdst, Rleft, Rright); break;
> +         case lir_logic_xor: __ eor (Rdst, Rleft, Rright); break;
> +         default: ShouldNotReachHere(); break;
> +       }
> +     }
> +   }
>  }

What is this rewrite of logic_op for?  Is it part of this patch?

> +	//__ push(r0->bit(1) | r1->bit(1), sp);
> +	__ push(r0->bit(1) | r1->bit(1) | rscratch1->bit(1) | rscratch2->bit(1), sp);

What is the commented-out code for?  Why is this particular set of registers
pushed?

> +  // Calling the runtime using the regular call_VM_leaf mechanism generates
> +  // code (generated by InterpreterMacroAssember::call_VM_leaf_base)
> +  // that checks that the *(ebp+frame::interpreter_frame_last_sp) == NULL.

Does it really?

Otherwise this is OK.

Andrew.