[aarch64-port-dev ] RFR (XS) 8229123: Revert build fixes for aarch64/zero

[Andrew Dinn]

Ah, sorry, I think I got the wrong end of the stick here. You are
talking about deleting this from jdk8u-aarch64 in order not to push it
up into jdk8u.

In that case then the direction and weight of inertia is much more
unclear. In theory the status quo derives from the jdk8u upstream code
which mans the conservative approach is not to push this change up into
jdk8u.

However, if anyone has actually been using zero then they I think they
may well have been using a Linux build and, for recent builds, that
means they will likely have been using the jdk8u-aarch64 code.

I'm no longer sure what to do here and would really like Andrew Haley to
provide an answer.

regards,


Andrew Dinn
-----------

On 05/08/2019 11:07, Andrew Dinn wrote:
> On 05/08/2019 09:54, Aleksey Shipilev wrote:
>> RFE:
>>   https://bugs.openjdk.java.net/browse/JDK-8229123
>>
>> Another thing found during 8u-aarch64 webrev scrubbing. Not sure why this thing is needed. Reverting
>> it does not break aarch64/zero cross-compiled build. There is no change like this in any of the
>> upstreams either. Maybe Edward remembers why it is there?
> 
> I ecrtainy don't recognise this code. Maybe Andrew Haley does?
> 
> Is Ed Nevill's name on the change-set? If so he will probably have had a
> good reason to add this. Given the comment explaining why this is needed
> I'm concerned that the error it guards against may only turn up in
> unusual circumstances which might well explain why standard tests
> succeeded after removal. Still ...
> 
>> 8u-aarch64 fix:
>>
>> diff -r 732ebff024df src/share/vm/interpreter/interpreterRuntime.cpp
>> --- a/src/share/vm/interpreter/interpreterRuntime.cpp   Mon Aug 05 09:42:26 2019 +0200
>> +++ b/src/share/vm/interpreter/interpreterRuntime.cpp   Mon Aug 05 10:51:59 2019 +0200
>> @@ -396,37 +396,35 @@
>>    int                handler_bci;
>>    int                current_bci = bci(thread);
>>
>> -#ifndef CC_INTERP
>>    if (thread->frames_to_pop_failed_realloc() > 0) {
>>      // Allocation of scalar replaced object used in this frame
>>      // failed. Unconditionally pop the frame.
>>      thread->dec_frames_to_pop_failed_realloc();
>>      thread->set_vm_result(h_exception());
>>      // If the method is synchronized we already unlocked the monitor
>>      // during deoptimization so the interpreter needs to skip it when
>>      // the frame is popped.
>>      thread->set_do_not_unlock_if_synchronized(true);
>>  #ifdef CC_INTERP
>>      return (address) -1;
>>  #else
>>      return Interpreter::remove_activation_entry();
>>  #endif
>>    }
>> -#endif
>>
>>    // Need to do this check first since when _do_not_unlock_if_synchronized
>>    // is set, we don't want to trigger any classloading which may make calls
>>
>> Testing: aarch64 {server,zero} builds; ad-hoc testing zero build
> ... Yuck! Those ifdefs are a mess which does not inspire confidence.
> Since this is not present and has not yet caused a problem in jdk11u or
> head I can see why it is very tempting to delete it and wait for the
> error to turn up. However, that's just what we *ought not* to do in a
> maintenance release. So, I don't think that's a good enough reason to
> remove it.
> 
> I think the only way to proceed here would be to 1) look into how the
> value returned by thread->frames_to_pop_failed_realloc() can be > 0 and
> see if the logic of this response really makes sense 2) check with Ed
> Nevill why he added the code (assuming it was his) 3) work out if ths
> code really is needed to protect against a potential error and 4) still
> leave it alone unless those 3 prior investigations provide an utterly
> sound reason to delete it. Alternatively, you can claim priority of the
> status quo as force majeure and do nothing. Your choice :-)
> 
> 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
> 

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