[aarch64-port-dev ] FW: RFR JDK-8194084: Obsolete FastTLABRefill and remove the related code

[White, Derek]

[forwarded]

-----Original Message-----
From: White, Derek 
Sent: Thursday, February 15, 2018 5:55 PM
To: 'JC Beyler' <jcbeyler at google.com>; hotspot-dev at openjdk.java.net
Subject: RE: RFR JDK-8194084: Obsolete FastTLABRefill and remove the related code

Hi JC,

I reviewed the aarch64 code only.

The code looks safe enough, but could be optimized as part of a 2nd CR:

In the fast_new_instance cases, r5 and r19 are saved and restored around the eden allocation. See lines 964, 727, and 731.

- After your change, r5 is no longer needed, so does not need to be saved.
- Perhaps we could use another register beside r19, and not save anything? 
  - Seems like rscratch2 might be free, but it might be bad form to use around so many macros?
- If we do need to save r19, we could push the store to around line 720, and remove the restore at line 731.

I am OK with checking in your patch as is, and optimizing the aarch64 code as a separate CR.

- Derek White

> -----Original Message-----
> From: hotspot-dev [mailto:hotspot-dev-bounces at openjdk.java.net] On 
> Behalf Of JC Beyler
> Sent: Wednesday, February 14, 2018 6:09 PM
> To: hotspot-dev at openjdk.java.net
> Subject: RFR JDK-8194084: Obsolete FastTLABRefill and remove the 
> related code
> 
> Hi all,
> 
> Here is a webrev to do the work mentioned in JDK-8194084
> <https://bugs.openjdk.java.net/browse/JDK-8194084>:
> http://cr.openjdk.java.net/~jcbeyler/8194084/webrev.01/
> 
> It has the parts for each architecture and I can't test a lot of them 
> so I would need a review and test for each :). I think first would be 
> an agreement to the code change itself then test it once everyone agrees on the change ?
> 
> Could I please get some initial reviews on this?
> 
> Basically what this webrev does is follow what the interpreter is saying:
>   - No longer try to do a fast tlab refill
>   - Try eden allocation if contiguous inline allocation is true
>   - Otherwise slowpath
> 
> This is true for all architectures except:
>    - ppc, which doesn't do eden allocations, I just cleaned up the 
> code a bit there to be consistent
>    - s390 that does not do tlab_refill at all, I just removed the dead code there.
> 
> Thanks a lot for your help,
> Jc