[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-compiler-dev
Subject:    RE: RFR(L) 8159976: PPC64: Add missing intrinsics for sub-word atomics
From:       "Lindenmaier, Goetz" <goetz.lindenmaier () sap ! com>
Date:       2016-06-23 12:34:41
Message-ID: 5cb0f14a982248c0b596c8306d20bb41 () DEWDFE13DE09 ! global ! corp ! sap
[Download RAW message or body]

Hi Martin, 

thanks for doing these fixes. Looks perfect now.

Best regards,
  Goetz.

> -----Original Message-----
> From: Doerr, Martin
> Sent: Donnerstag, 23. Juni 2016 14:14
> To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com>; hotspot-compiler-
> dev@openjdk.java.net; Aleksey Shipilev (aleksey.shipilev@oracle.com)
> <aleksey.shipilev@oracle.com>
> Subject: RE: RFR(L) 8159976: PPC64: Add missing intrinsics for sub-word
> atomics
> 
> Hi Aleksey and Götz,
> 
> thank you very much for reviewing.
> 
> @Götz: I have added comments as you requested. I also changed the
> compareAndSwap effects such that the result has a TEMP_DEF effect. This
> avoids the jump instruction and the variable length.
> 
> New webrev is here:
> http://cr.openjdk.java.net/~mdoerr/8159976_PPC64_subword_atomics/we
> brev.01/
> 
> Best regards,
> Martin
> 
> -----Original Message-----
> From: Lindenmaier, Goetz
> Sent: Donnerstag, 23. Juni 2016 11:30
> To: Doerr, Martin <martin.doerr@sap.com>; hotspot-compiler-
> dev@openjdk.java.net
> Subject: RE: RFR(L) 8159976: PPC64: Add missing intrinsics for sub-word
> atomics
> 
> Hi Martin,
> 
> I had a look at your changes.  Thanks for doing all this, it's quite
> a piece of code!
> 
> It took me a while to understand how you distinguish for older
> processor types ... I matched "instruction_type" on the size handled
> by the instruction right away, but not that you then use it to decide
> that you have to emulate the instructions for small sizes.
> 
> Maybe you could !has_lqarx() or at least comment the if statement.
> 
> Also, you properly sign extend the 1/2 byte values that are loaded with the
> Zero-extending l?arx instructions.  Is this correct for Java chars?
> Or do these need to be handled unsigned?
> 
> In the ad file, you say:
> // Variable size: instruction count smaller if regs are disjoint.
> Don't you want to enforce that by adding effect "TEMP_DEF dst"?
> Especially as you save a branch in that case.  In some match rules
> this should hold already ad there is the TEMP_DEF effect specified.
> 
> Besides that the code looks good.
> 
> Best regards,
>   Goetz.
> 
> 
> > -----Original Message-----
> > From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-
> > bounces@openjdk.java.net] On Behalf Of Doerr, Martin
> > Sent: Mittwoch, 22. Juni 2016 13:06
> > To: hotspot-compiler-dev@openjdk.java.net
> > Subject: RFR(L) 8159976: PPC64: Add missing intrinsics for sub-word atomics
> >
> > Hi,
> >
> >
> >
> > after JDK-8157726 was pushed, the JVM supports intrinsics for sub-word
> > atomics.
> >
> >
> >
> > I have adapted the PPC64 implementation:
> >
> >
> http://cr.openjdk.java.net/~mdoerr/8159976_PPC64_subword_atomics/we
> > brev.00/
> >
> <http://cr.openjdk.java.net/~mdoerr/8159976_PPC64_subword_atomics/w
> > ebrev.00/>
> >
> >
> >
> > The change also removes old String intrinsics which are no longer needed
> > (since the new ones are available).
> >
> >
> >
> > Please review.
> >
> >
> >
> > Best regards,
> >
> > Martin
> >
> >

[prev in list] [next in list] [prev in thread] [next in thread]