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

[Stuart Monteith]

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.

BR,
   Stuart

On 18 August 2017 at 10:07, Andrew Dinn <adinn at redhat.com> wrote:
> On 16/08/17 18:34, Stuart Monteith wrote:
>> Ok,
>>   The assert I added to g1_write_barrier_post was tripping - we were
>> passing rscratch1 and using it locally. I could just change it to
>> rscratch2, but if we are passing temporary registers as parameters,
>> I'd argue we shouldn't be using rscratch1, etc, hardcoded in those
>> routines. In other platforms we are passing more than two temporary
>> registers. I'll have a look at a more extensive change.
>
> First, regarding the asserts:
>
> As you say, there is no need for to separate the cases to make two
> different calls to assert_different_registers. However, it is nice that
> the current code recognises the possibility that obj == noreg, albeit at
> the cost of some branching. You could maybe use this:
>
>   assert_different_registers(obj, pre_val, tmp, rscratch1);
>   assert(pre_val != noreg &&  tmp != noreg, "expecting a register");
>
> Second, the rscratch1 conflict in g1_write_barrier_post is best solved
> by changing the calling code in templateTable_aarch64.cpp to pass
> rscratch2 (that is the only call) i.e.
>
>         } else {
>           // G1 barrier needs uncompressed oop for region cross check.
>           Register new_val = val;
>           if (UseCompressedOops) {
> -             new_val = rscratch1;
> +             new_val = rscratch2;
>             __ mov(new_val, val);
>           }
>           __ store_heap_oop(Address(r3, 0), val);
>           __ g1_write_barrier_post(r3 /* store_adr */,
>                                    new_val /* new_val */,
>                                    rthread /* thread */,
>                                    r10 /* tmp */,
>                                    r1 /* tmp2 */);
>         }
>
> That should be enough to allow a similar assert to be used in
> g1_write_barrier_post. In this latter case obj cannot be noreg so you
> need to include that in the second assert for g1_write_barrier_post.
>
> 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