[aarch64-port-dev ] jdk1 Fix for hotspot/test/gc/g1/TestJNIWeakG1/TestJNIWeakG1.java

[Andrew Dinn]

Hi Stuart,

On 18/08/17 12:00, Stuart Monteith wrote:
> Hello,
>    Thanks Andrew. I might have gone too far by factoring out rscratch1
> and adding an extra parameter to g1_write_barrier_post.
> This is my current patch:
>  http://cr.openjdk.java.net/~njian/8186325/webrev.00/
> 
> I'll add the extra asserts for testing for !=noreg. If you want I can
> undo adding tmp3 to g1_write_barrier_post.
You don't need to add any extra registers as arguments to
g1_write_barrier_pre and g1_write_barrier_post. You really just need an
assert to ensure that none of the incoming registers clash with those
employed explicitly by the called code or with each other. In the first
case that means:

  assert_different_registers(obj, pre_val, tmp, rscratch1);

In the second case that means:

  assert_different_registers(obj, pre_val, tmp, tmp2, rscratch1);

This is all that is needed to catch wrong caller assumptions.

Although the use of registers internal to the barrier methods is
implicit there is no way this is not going to be found out and corrected
before anything gets shipped. If you want to be extra helpful to future
devs you can add a header comment to the two barrier functions e.g.

/*
 * g1_write_barrier_pre -- write a barrier for store of
 * new_val at store_addr
 *
 * allocates rscratch1
 */

If you look around you will find lots of places in the implementation
where rscratch1 is passed as a tmp register and many more where
rscratch2 or some other register is passed because it is /implicitly/
'known' that rscratch1 is going to be used by the called routine (e.g.
see file interp_masm_aarch64.cpp).

regards,


Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander