RFR JDK-8194084: Obsolete FastTLABRefill and remove the related code

[JC Beyler]

Hi Derek,

I agree with removing the r5 saving/loading; for r19, I have no idea if
there is another register that could be used instead. I also have no
opinion on using rscratch2 but I agree that it is probably error prone to
do that with the other macros. I'm unfamiliar with aarch64 so if someone
tells me what they want, I can wrap the refactor here too.

Last thing: you say we could move the store down but I don't see how to
remove the line 731 restore because allocate_eden does seem to use it so
should we not always restore it?

Thanks for your help,
Jc


On Thu, Feb 15, 2018 at 2:54 PM, White, Derek <Derek.White at cavium.com>
wrote:

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