[10] RFR(S): 8185969: PPC64: Improve VSR support to use up to 64 registers

Doerr, Martin martin.doerr at sap.com
Mon Aug 14 15:04:00 UTC 2017


Tested and pushed with the modification. Note that the range assertion is done in encoding().
Thanks for the contribution and for reviewing.

Best regards,
Martin

-----Original Message-----
From: Lindenmaier, Goetz 
Sent: Montag, 14. August 2017 15:35
To: Gustavo Serra Scalet <gustavo.scalet at eldorado.org.br>; Doerr, Martin <martin.doerr at sap.com>
Cc: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-dev at openjdk.java.net>; hotspot-dev at openjdk.java.net
Subject: RE: [10] RFR(S): 8185969: PPC64: Improve VSR support to use up to 64 registers

Me too :)

> -----Original Message-----
> From: Gustavo Serra Scalet [mailto:gustavo.scalet at eldorado.org.br]
> Sent: Montag, 14. August 2017 15:13
> To: Doerr, Martin <martin.doerr at sap.com>; Lindenmaier, Goetz
> <goetz.lindenmaier at sap.com>
> Cc: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-
> dev at openjdk.java.net>; hotspot-dev at openjdk.java.net
> Subject: RE: [10] RFR(S): 8185969: PPC64: Improve VSR support to use up to 64
> registers
> 
> Hi Martin,
> 
> Yes please. I'm ok with that.
> 
> @Goetz: sorry, I misunderstood. I read your and Martin's comments and mixed
> them up. I got it now about keeping your original "if" and adding and "assert".
> 
> 
> 
> > -----Original Message-----
> > From: Doerr, Martin [mailto:martin.doerr at sap.com]
> > Sent: segunda-feira, 14 de agosto de 2017 09:53
> > To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Gustavo Serra Scalet
> > <gustavo.scalet at eldorado.org.br>
> > Cc: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-
> > dev at openjdk.java.net>; hotspot-dev at openjdk.java.net
> > Subject: RE: [10] RFR(S): 8185969: PPC64: Improve VSR support to use up
> > to 64 registers
> >
> > Hi Gustavo,
> >
> > I can do these minor changes before pushing if you're ok with it.
> >
> > Best regards,
> > Martin
> >
> >
> > -----Original Message-----
> > From: Lindenmaier, Goetz
> > Sent: Montag, 14. August 2017 14:50
> > To: Gustavo Serra Scalet <gustavo.scalet at eldorado.org.br>; Doerr, Martin
> > <martin.doerr at sap.com>
> > Cc: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-
> > dev at openjdk.java.net>; hotspot-dev at openjdk.java.net
> > Subject: RE: [10] RFR(S): 8185969: PPC64: Improve VSR support to use up
> > to 64 registers
> >
> > Hi Gustavo,
> >
> > hmm, I tried to say that noreg is a special value and _not_ an illegal
> > one.
> > You _must_ map vnoreg to vsnoregi and not any other values.
> > You _can_ assert for illegal values:
> > assert (encoding() >=-1 && encoding() <=31, "register encoding out of
> > range")
> >
> > See for example check_klass_subtype_slow_path() on ppc how we use noreg.
> >
> > Please list only first and last year in register_ppc.cpp copyright.
> >
> > Thanks,
> >   Goetz.
> >
> > > -----Original Message-----
> > > From: Gustavo Serra Scalet [mailto:gustavo.scalet at eldorado.org.br]
> > > Sent: Montag, 14. August 2017 14:39
> > > To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Doerr, Martin
> > > <martin.doerr at sap.com>
> > > Cc: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-
> > > dev at openjdk.java.net>; hotspot-dev at openjdk.java.net
> > > Subject: RE: [10] RFR(S): 8185969: PPC64: Improve VSR support to use
> > > up to 64 registers
> > >
> > > Hi Goetz and Martin,
> > >
> > > Thanks for your comments. The webrev was updated accordingly:
> > > https://gut.github.io/openjdk/webrev/JDK-8185969/webrev.02/
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Lindenmaier, Goetz [mailto:goetz.lindenmaier at sap.com]
> > > > Sent: segunda-feira, 14 de agosto de 2017 07:17
> > > > To: Doerr, Martin <martin.doerr at sap.com>; Gustavo Serra Scalet
> > > > <gustavo.scalet at eldorado.org.br>
> > > > Cc: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-
> > > > dev at openjdk.java.net>; hotspot-dev at openjdk.java.net
> > > > Subject: RE: [10] RFR(S): 8185969: PPC64: Improve VSR support to use
> > > > up to 64 registers
> > > >
> > > > Hi Gustavo,
> > > >
> > > > noreg* is meant to be used intentionally. So you should not match
> > > > any illegal values to noreg*.  I would implement it like this:
> > > > if (encoding() == vnoreg->encoding()) { return vsnoregi; } If you
> > > > feel like, you can assert for values < -1 and > 31.
> > > >
> > > > For the copyrights:
> > > > You must adapt the copyright year of the Oracle copyright line in
> > > > any file you edit.  There must always be a trailing comma. I.e., you
> > > > change
> > > >  * Copyright (c) 2000, Oracle and/or its affiliates. All rights
> > > > reserved.
> > > > to
> > > >  * Copyright (c) 2000, 2017, Oracle and/or its affiliates. All
> > > > rights reserved.
> > > > or
> > > >  * Copyright (c) 2000, 2016, Oracle and/or its affiliates. All
> > > > rights reserved.
> > > > to
> > > >  * Copyright (c) 2000, 2017, Oracle and/or its affiliates. All
> > > > rights reserved.
> > > >
> > > > You may add one line of your own copyright. But this is only common
> > > > in new files.  New files usually also get the Oracle copyright line.
> > > > That's why these files have the SAP copyright. You don't need to
> > > > update the SAP copyright :).
> > > >
> > > > Best regards,
> > > >   Goetz.
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Doerr, Martin
> > > > > Sent: Samstag, 12. August 2017 12:01
> > > > > To: Gustavo Serra Scalet <gustavo.scalet at eldorado.org.br>;
> > > > > Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> > > > > Cc: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-
> > > > > dev at openjdk.java.net>; hotspot-dev at openjdk.java.net
> > > > > Subject: RE: [10] RFR(S): 8185969: PPC64: Improve VSR support to
> > > > > use up to 64 registers
> > > > >
> > > > > Hi Gustavo,
> > > > >
> > > > > I think returning vsnoregi is the better choice. If you want to
> > > > > check the upper limit as well, it should be ">=32".
> > > > > Copyright year needs  to be updated in the header (first 2 lines)
> > > > > of the register_ppc files.
> > > > >
> > > > > Thanks and best regards,
> > > > > Martin
> > > > >
> > > > >
> > > > > -----Original Message-----
> > > > > From: Gustavo Serra Scalet [mailto:gustavo.scalet at eldorado.org.br]
> > > > > Sent: Freitag, 11. August 2017 22:03
> > > > > To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Doerr, Martin
> > > > > <martin.doerr at sap.com>
> > > > > Cc: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-
> > > > > dev at openjdk.java.net>; hotspot-dev at openjdk.java.net
> > > > > Subject: RE: [10] RFR(S): 8185969: PPC64: Improve VSR support to
> > > > > use up to 64 registers
> > > > >
> > > > > Hello Goetz,
> > > > >
> > > > > About the noreg: if the VectorRegisterImpl::encoding() is not a
> > > > > value from 0-32 expected, it was an invalid VectorRegister which
> > > > > will, in turn, continue to be invalid as VectorSRegister.
> > > > >
> > > > > Anyway, do you want me to add a verification of "if ((encoding() <
> > > > > 0)
> > > > > ||
> > > > > (encoding() > 32)) return vsnoregi;" before? Or an assert?
> > > > >
> > > > >
> > > > > And about the copyright, where do you mean?
> > > > >
> > > > > Thanks
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Lindenmaier, Goetz [mailto:goetz.lindenmaier at sap.com]
> > > > > > Sent: sexta-feira, 11 de agosto de 2017 11:56
> > > > > > To: Gustavo Serra Scalet <gustavo.scalet at eldorado.org.br>;
> > > > > > Doerr, Martin <martin.doerr at sap.com>
> > > > > > Cc: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-
> > > > > > dev at openjdk.java.net>; hotspot-dev at openjdk.java.net
> > > > > > Subject: RE: [10] RFR(S): 8185969: PPC64: Improve VSR support to
> > > > > > use up to 64 registers
> > > > > >
> > > > > > Hi Gustavo,
> > > > > >
> > > > > > thanks for the new change and posting to the other lists :)
> > > > > >
> > > > > > I appreciate the shorter function. I think you need to check for
> > > > > > noreg and keep that unchanged, though.
> > > > > > Some copyrights are not updated to 2017.
> > > > > > Consider it reviewed if the new function is fixed.
> > > > > >
> > > > > > I posted the webrev on the cr server:
> > > > > > http://cr.openjdk.java.net/~goetz/wr17/8185969-ppcAsm/webrev.01/
> > > > > > and added you as contributor.
> > > > > >
> > > > > > Best regards,
> > > > > >   Goetz.
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-
> > > > > > > bounces at openjdk.java.net] On Behalf Of Gustavo Serra Scalet
> > > > > > > Sent: Freitag, 11. August 2017 15:55
> > > > > > > To: Doerr, Martin <martin.doerr at sap.com>
> > > > > > > Cc: 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-
> > > > > > > dev at openjdk.java.net>; hotspot-dev at openjdk.java.net
> > > > > > > Subject: RE: [10] RFR(S): 8185969: PPC64: Improve VSR support
> > > > > > > to use up to 64 registers
> > > > > > >
> > > > > > > It worked for me Martin. Please check the new webrev:
> > > > > > > https://gut.github.io/openjdk/webrev/JDK-8185969/webrev.01/
> > > > > > >
> > > > > > > Thanks for the review.
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-
> > > > > > > > bounces at openjdk.java.net] On Behalf Of Gustavo Serra Scalet
> > > > > > > > Sent: quinta-feira, 10 de agosto de 2017 14:21
> > > > > > > > To: 'hotspot-compiler-dev at openjdk.java.net'
> > > > > > > > <hotspot-compiler- dev at openjdk.java.net>
> > > > > > > > Subject: FW: [10] RFR(S): 8185969: PPC64: Improve VSR
> > > > > > > > support to use up to 64 registers
> > > > > > > >
> > > > > > > > Hi Martin,
> > > > > > > >
> > > > > > > > Alright. I'll analyze that and update to a new webrev.
> > > > > > > >
> > > > > > > > Thanks
> > > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Doerr, Martin [mailto:martin.doerr at sap.com]
> > > > > > > > Sent: quarta-feira, 9 de agosto de 2017 09:58
> > > > > > > > To: Gustavo Serra Scalet <gustavo.scalet at eldorado.org.br>;
> > > > > > > > ppc-aix-port- dev at openjdk.java.net
> > > > > > > > Subject: RE: [10] RFR(S): 8185969: PPC64: Improve VSR
> > > > > > > > support to use up to 64 registers
> > > > > > > >
> > > > > > > > Hi Gustavo,
> > > > > > > >
> > > > > > > > seems like you're preparing new VSR code :-) Nice change,
> > > > > > > > but please update the copyright messages in the modified
> > files.
> > > > > > > >
> > > > > > > > register_ppc.cpp
> > > > > > > > to_vsr(): I don't like large code for a trivial computation.
> > > > > > > > I'd prefer something like "return
> > > > > > > > as_VectorSRegister(encoding() +
> > > > 32)".
> > > > > > > >
> > > > > > > > I can sponsor this change.
> > > > > > > >
> > > > > > > > Thanks and best regards,
> > > > > > > > Martin
> > > > > > > >
> > > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: ppc-aix-port-dev [mailto:ppc-aix-port-dev-
> > > > > > > > bounces at openjdk.java.net] On Behalf Of Gustavo Serra Scalet
> > > > > > > > Sent: Dienstag, 8. August 2017 22:19
> > > > > > > > To: ppc-aix-port-dev at openjdk.java.net
> > > > > > > > Subject: [10] RFR(S): 8185969: PPC64: Improve VSR support to
> > > > > > > > use up to
> > > > > > > > 64 registers
> > > > > > > >
> > > > > > > > Hi,
> > > > > > > >
> > > > > > > > Could you please review this specific PPC64 change to
> > hotspot?
> > > > > > > > The aim of these changes is to allow a more complete usage
> > > > > > > > of VSR. I make use of these interfaces on an instrinsic that
> > > > > > > > I plan to send next but I decided to separate this change
> > > > > > > > and send it
> > > > first.
> > > > > > > >
> > > > > > > > JBS: https://bugs.openjdk.java.net/browse/JDK-8185969
> > > > > > > > Webrev:
> > > > > > > > https://gut.github.io/openjdk/webrev/JDK-8185969/webrev/
> > > > > > > >
> > > > > > > > Best regards,
> > > > > > > > Gustavo Serra Scalet


More information about the hotspot-compiler-dev mailing list