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

Mon Aug 14 12:50:23 UTC 2017

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