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

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Gustavo Serra Scalet
Hi Martin,

Alright. I'll analyze that and update to a new webrev.

Thanks

-----Original Message-----
From: Doerr, Martin [mailto:[hidden email]]
Sent: quarta-feira, 9 de agosto de 2017 09:58
To: Gustavo Serra Scalet <[hidden email]>; [hidden email]
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:[hidden email]] On Behalf Of Gustavo Serra Scalet
Sent: Dienstag, 8. August 2017 22:19
To: [hidden email]
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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Gustavo Serra Scalet
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-
> [hidden email]] On Behalf Of Gustavo Serra Scalet
> Sent: quinta-feira, 10 de agosto de 2017 14:21
> To: '[hidden email]' <hotspot-compiler-
> [hidden email]>
> 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:[hidden email]]
> Sent: quarta-feira, 9 de agosto de 2017 09:58
> To: Gustavo Serra Scalet <[hidden email]>; ppc-aix-port-
> [hidden email]
> 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-
> [hidden email]] On Behalf Of Gustavo Serra Scalet
> Sent: Dienstag, 8. August 2017 22:19
> To: [hidden email]
> 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Lindenmaier, Goetz
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-
> [hidden email]] On Behalf Of Gustavo Serra Scalet
> Sent: Freitag, 11. August 2017 15:55
> To: Doerr, Martin <[hidden email]>
> Cc: '[hidden email]' <hotspot-compiler-
> [hidden email]>; [hidden email]
> 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-
> > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > Sent: quinta-feira, 10 de agosto de 2017 14:21
> > To: '[hidden email]' <hotspot-compiler-
> > [hidden email]>
> > 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:[hidden email]]
> > Sent: quarta-feira, 9 de agosto de 2017 09:58
> > To: Gustavo Serra Scalet <[hidden email]>; ppc-aix-port-
> > [hidden email]
> > 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-
> > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > Sent: Dienstag, 8. August 2017 22:19
> > To: [hidden email]
> > 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Gustavo Serra Scalet
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:[hidden email]]
> Sent: sexta-feira, 11 de agosto de 2017 11:56
> To: Gustavo Serra Scalet <[hidden email]>; Doerr, Martin
> <[hidden email]>
> Cc: '[hidden email]' <hotspot-compiler-
> [hidden email]>; [hidden email]
> 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-
> > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > Sent: Freitag, 11. August 2017 15:55
> > To: Doerr, Martin <[hidden email]>
> > Cc: '[hidden email]' <hotspot-compiler-
> > [hidden email]>; [hidden email]
> > 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-
> > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > Sent: quinta-feira, 10 de agosto de 2017 14:21
> > > To: '[hidden email]' <hotspot-compiler-
> > > [hidden email]>
> > > 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:[hidden email]]
> > > Sent: quarta-feira, 9 de agosto de 2017 09:58
> > > To: Gustavo Serra Scalet <[hidden email]>;
> > > ppc-aix-port- [hidden email]
> > > 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-
> > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > Sent: Dienstag, 8. August 2017 22:19
> > > To: [hidden email]
> > > 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Doerr, Martin
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:[hidden email]]
Sent: Freitag, 11. August 2017 22:03
To: Lindenmaier, Goetz <[hidden email]>; Doerr, Martin <[hidden email]>
Cc: '[hidden email]' <[hidden email]>; [hidden email]
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:[hidden email]]
> Sent: sexta-feira, 11 de agosto de 2017 11:56
> To: Gustavo Serra Scalet <[hidden email]>; Doerr, Martin
> <[hidden email]>
> Cc: '[hidden email]' <hotspot-compiler-
> [hidden email]>; [hidden email]
> 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-
> > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > Sent: Freitag, 11. August 2017 15:55
> > To: Doerr, Martin <[hidden email]>
> > Cc: '[hidden email]' <hotspot-compiler-
> > [hidden email]>; [hidden email]
> > 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-
> > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > Sent: quinta-feira, 10 de agosto de 2017 14:21
> > > To: '[hidden email]' <hotspot-compiler-
> > > [hidden email]>
> > > 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:[hidden email]]
> > > Sent: quarta-feira, 9 de agosto de 2017 09:58
> > > To: Gustavo Serra Scalet <[hidden email]>;
> > > ppc-aix-port- [hidden email]
> > > 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-
> > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > Sent: Dienstag, 8. August 2017 22:19
> > > To: [hidden email]
> > > 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Lindenmaier, Goetz
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 <[hidden email]>; Lindenmaier,
> Goetz <[hidden email]>
> Cc: '[hidden email]' <hotspot-compiler-
> [hidden email]>; [hidden email]
> 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:[hidden email]]
> Sent: Freitag, 11. August 2017 22:03
> To: Lindenmaier, Goetz <[hidden email]>; Doerr, Martin
> <[hidden email]>
> Cc: '[hidden email]' <hotspot-compiler-
> [hidden email]>; [hidden email]
> 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:[hidden email]]
> > Sent: sexta-feira, 11 de agosto de 2017 11:56
> > To: Gustavo Serra Scalet <[hidden email]>; Doerr, Martin
> > <[hidden email]>
> > Cc: '[hidden email]' <hotspot-compiler-
> > [hidden email]>; [hidden email]
> > 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-
> > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > Sent: Freitag, 11. August 2017 15:55
> > > To: Doerr, Martin <[hidden email]>
> > > Cc: '[hidden email]' <hotspot-compiler-
> > > [hidden email]>; [hidden email]
> > > 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-
> > > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > > Sent: quinta-feira, 10 de agosto de 2017 14:21
> > > > To: '[hidden email]' <hotspot-compiler-
> > > > [hidden email]>
> > > > 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:[hidden email]]
> > > > Sent: quarta-feira, 9 de agosto de 2017 09:58
> > > > To: Gustavo Serra Scalet <[hidden email]>;
> > > > ppc-aix-port- [hidden email]
> > > > 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-
> > > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > > Sent: Dienstag, 8. August 2017 22:19
> > > > To: [hidden email]
> > > > 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Gustavo Serra Scalet
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:[hidden email]]
> Sent: segunda-feira, 14 de agosto de 2017 07:17
> To: Doerr, Martin <[hidden email]>; Gustavo Serra Scalet
> <[hidden email]>
> Cc: '[hidden email]' <hotspot-compiler-
> [hidden email]>; [hidden email]
> 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 <[hidden email]>;
> > Lindenmaier, Goetz <[hidden email]>
> > Cc: '[hidden email]' <hotspot-compiler-
> > [hidden email]>; [hidden email]
> > 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:[hidden email]]
> > Sent: Freitag, 11. August 2017 22:03
> > To: Lindenmaier, Goetz <[hidden email]>; Doerr, Martin
> > <[hidden email]>
> > Cc: '[hidden email]' <hotspot-compiler-
> > [hidden email]>; [hidden email]
> > 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:[hidden email]]
> > > Sent: sexta-feira, 11 de agosto de 2017 11:56
> > > To: Gustavo Serra Scalet <[hidden email]>; Doerr,
> > > Martin <[hidden email]>
> > > Cc: '[hidden email]' <hotspot-compiler-
> > > [hidden email]>; [hidden email]
> > > 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-
> > > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > > Sent: Freitag, 11. August 2017 15:55
> > > > To: Doerr, Martin <[hidden email]>
> > > > Cc: '[hidden email]' <hotspot-compiler-
> > > > [hidden email]>; [hidden email]
> > > > 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-
> > > > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > > > Sent: quinta-feira, 10 de agosto de 2017 14:21
> > > > > To: '[hidden email]' <hotspot-compiler-
> > > > > [hidden email]>
> > > > > 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:[hidden email]]
> > > > > Sent: quarta-feira, 9 de agosto de 2017 09:58
> > > > > To: Gustavo Serra Scalet <[hidden email]>;
> > > > > ppc-aix-port- [hidden email]
> > > > > 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-
> > > > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > > > Sent: Dienstag, 8. August 2017 22:19
> > > > > To: [hidden email]
> > > > > 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Lindenmaier, Goetz
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:[hidden email]]
> Sent: Montag, 14. August 2017 14:39
> To: Lindenmaier, Goetz <[hidden email]>; Doerr, Martin
> <[hidden email]>
> Cc: '[hidden email]' <hotspot-compiler-
> [hidden email]>; [hidden email]
> 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:[hidden email]]
> > Sent: segunda-feira, 14 de agosto de 2017 07:17
> > To: Doerr, Martin <[hidden email]>; Gustavo Serra Scalet
> > <[hidden email]>
> > Cc: '[hidden email]' <hotspot-compiler-
> > [hidden email]>; [hidden email]
> > 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 <[hidden email]>;
> > > Lindenmaier, Goetz <[hidden email]>
> > > Cc: '[hidden email]' <hotspot-compiler-
> > > [hidden email]>; [hidden email]
> > > 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:[hidden email]]
> > > Sent: Freitag, 11. August 2017 22:03
> > > To: Lindenmaier, Goetz <[hidden email]>; Doerr, Martin
> > > <[hidden email]>
> > > Cc: '[hidden email]' <hotspot-compiler-
> > > [hidden email]>; [hidden email]
> > > 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:[hidden email]]
> > > > Sent: sexta-feira, 11 de agosto de 2017 11:56
> > > > To: Gustavo Serra Scalet <[hidden email]>; Doerr,
> > > > Martin <[hidden email]>
> > > > Cc: '[hidden email]' <hotspot-compiler-
> > > > [hidden email]>; [hidden email]
> > > > 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-
> > > > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > > > Sent: Freitag, 11. August 2017 15:55
> > > > > To: Doerr, Martin <[hidden email]>
> > > > > Cc: '[hidden email]' <hotspot-compiler-
> > > > > [hidden email]>; [hidden email]
> > > > > 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-
> > > > > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > > > > Sent: quinta-feira, 10 de agosto de 2017 14:21
> > > > > > To: '[hidden email]' <hotspot-compiler-
> > > > > > [hidden email]>
> > > > > > 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:[hidden email]]
> > > > > > Sent: quarta-feira, 9 de agosto de 2017 09:58
> > > > > > To: Gustavo Serra Scalet <[hidden email]>;
> > > > > > ppc-aix-port- [hidden email]
> > > > > > 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-
> > > > > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > > > > Sent: Dienstag, 8. August 2017 22:19
> > > > > > To: [hidden email]
> > > > > > 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Doerr, Martin
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 <[hidden email]>; Doerr, Martin <[hidden email]>
Cc: '[hidden email]' <[hidden email]>; [hidden email]
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:[hidden email]]
> Sent: Montag, 14. August 2017 14:39
> To: Lindenmaier, Goetz <[hidden email]>; Doerr, Martin
> <[hidden email]>
> Cc: '[hidden email]' <hotspot-compiler-
> [hidden email]>; [hidden email]
> 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:[hidden email]]
> > Sent: segunda-feira, 14 de agosto de 2017 07:17
> > To: Doerr, Martin <[hidden email]>; Gustavo Serra Scalet
> > <[hidden email]>
> > Cc: '[hidden email]' <hotspot-compiler-
> > [hidden email]>; [hidden email]
> > 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 <[hidden email]>;
> > > Lindenmaier, Goetz <[hidden email]>
> > > Cc: '[hidden email]' <hotspot-compiler-
> > > [hidden email]>; [hidden email]
> > > 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:[hidden email]]
> > > Sent: Freitag, 11. August 2017 22:03
> > > To: Lindenmaier, Goetz <[hidden email]>; Doerr, Martin
> > > <[hidden email]>
> > > Cc: '[hidden email]' <hotspot-compiler-
> > > [hidden email]>; [hidden email]
> > > 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:[hidden email]]
> > > > Sent: sexta-feira, 11 de agosto de 2017 11:56
> > > > To: Gustavo Serra Scalet <[hidden email]>; Doerr,
> > > > Martin <[hidden email]>
> > > > Cc: '[hidden email]' <hotspot-compiler-
> > > > [hidden email]>; [hidden email]
> > > > 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-
> > > > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > > > Sent: Freitag, 11. August 2017 15:55
> > > > > To: Doerr, Martin <[hidden email]>
> > > > > Cc: '[hidden email]' <hotspot-compiler-
> > > > > [hidden email]>; [hidden email]
> > > > > 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-
> > > > > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > > > > Sent: quinta-feira, 10 de agosto de 2017 14:21
> > > > > > To: '[hidden email]' <hotspot-compiler-
> > > > > > [hidden email]>
> > > > > > 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:[hidden email]]
> > > > > > Sent: quarta-feira, 9 de agosto de 2017 09:58
> > > > > > To: Gustavo Serra Scalet <[hidden email]>;
> > > > > > ppc-aix-port- [hidden email]
> > > > > > 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-
> > > > > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > > > > Sent: Dienstag, 8. August 2017 22:19
> > > > > > To: [hidden email]
> > > > > > 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Gustavo Serra Scalet
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:[hidden email]]
> Sent: segunda-feira, 14 de agosto de 2017 09:53
> To: Lindenmaier, Goetz <[hidden email]>; Gustavo Serra Scalet
> <[hidden email]>
> Cc: '[hidden email]' <hotspot-compiler-
> [hidden email]>; [hidden email]
> 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 <[hidden email]>; Doerr, Martin
> <[hidden email]>
> Cc: '[hidden email]' <hotspot-compiler-
> [hidden email]>; [hidden email]
> 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:[hidden email]]
> > Sent: Montag, 14. August 2017 14:39
> > To: Lindenmaier, Goetz <[hidden email]>; Doerr, Martin
> > <[hidden email]>
> > Cc: '[hidden email]' <hotspot-compiler-
> > [hidden email]>; [hidden email]
> > 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:[hidden email]]
> > > Sent: segunda-feira, 14 de agosto de 2017 07:17
> > > To: Doerr, Martin <[hidden email]>; Gustavo Serra Scalet
> > > <[hidden email]>
> > > Cc: '[hidden email]' <hotspot-compiler-
> > > [hidden email]>; [hidden email]
> > > 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 <[hidden email]>;
> > > > Lindenmaier, Goetz <[hidden email]>
> > > > Cc: '[hidden email]' <hotspot-compiler-
> > > > [hidden email]>; [hidden email]
> > > > 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:[hidden email]]
> > > > Sent: Freitag, 11. August 2017 22:03
> > > > To: Lindenmaier, Goetz <[hidden email]>; Doerr, Martin
> > > > <[hidden email]>
> > > > Cc: '[hidden email]' <hotspot-compiler-
> > > > [hidden email]>; [hidden email]
> > > > 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:[hidden email]]
> > > > > Sent: sexta-feira, 11 de agosto de 2017 11:56
> > > > > To: Gustavo Serra Scalet <[hidden email]>;
> > > > > Doerr, Martin <[hidden email]>
> > > > > Cc: '[hidden email]' <hotspot-compiler-
> > > > > [hidden email]>; [hidden email]
> > > > > 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-
> > > > > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > > > > Sent: Freitag, 11. August 2017 15:55
> > > > > > To: Doerr, Martin <[hidden email]>
> > > > > > Cc: '[hidden email]' <hotspot-compiler-
> > > > > > [hidden email]>; [hidden email]
> > > > > > 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-
> > > > > > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > > > > > Sent: quinta-feira, 10 de agosto de 2017 14:21
> > > > > > > To: '[hidden email]'
> > > > > > > <hotspot-compiler- [hidden email]>
> > > > > > > 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:[hidden email]]
> > > > > > > Sent: quarta-feira, 9 de agosto de 2017 09:58
> > > > > > > To: Gustavo Serra Scalet <[hidden email]>;
> > > > > > > ppc-aix-port- [hidden email]
> > > > > > > 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-
> > > > > > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > > > > > Sent: Dienstag, 8. August 2017 22:19
> > > > > > > To: [hidden email]
> > > > > > > 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Lindenmaier, Goetz
Me too :)

> -----Original Message-----
> From: Gustavo Serra Scalet [mailto:[hidden email]]
> Sent: Montag, 14. August 2017 15:13
> To: Doerr, Martin <[hidden email]>; Lindenmaier, Goetz
> <[hidden email]>
> Cc: '[hidden email]' <hotspot-compiler-
> [hidden email]>; [hidden email]
> 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:[hidden email]]
> > Sent: segunda-feira, 14 de agosto de 2017 09:53
> > To: Lindenmaier, Goetz <[hidden email]>; Gustavo Serra Scalet
> > <[hidden email]>
> > Cc: '[hidden email]' <hotspot-compiler-
> > [hidden email]>; [hidden email]
> > 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 <[hidden email]>; Doerr, Martin
> > <[hidden email]>
> > Cc: '[hidden email]' <hotspot-compiler-
> > [hidden email]>; [hidden email]
> > 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:[hidden email]]
> > > Sent: Montag, 14. August 2017 14:39
> > > To: Lindenmaier, Goetz <[hidden email]>; Doerr, Martin
> > > <[hidden email]>
> > > Cc: '[hidden email]' <hotspot-compiler-
> > > [hidden email]>; [hidden email]
> > > 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:[hidden email]]
> > > > Sent: segunda-feira, 14 de agosto de 2017 07:17
> > > > To: Doerr, Martin <[hidden email]>; Gustavo Serra Scalet
> > > > <[hidden email]>
> > > > Cc: '[hidden email]' <hotspot-compiler-
> > > > [hidden email]>; [hidden email]
> > > > 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 <[hidden email]>;
> > > > > Lindenmaier, Goetz <[hidden email]>
> > > > > Cc: '[hidden email]' <hotspot-compiler-
> > > > > [hidden email]>; [hidden email]
> > > > > 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:[hidden email]]
> > > > > Sent: Freitag, 11. August 2017 22:03
> > > > > To: Lindenmaier, Goetz <[hidden email]>; Doerr, Martin
> > > > > <[hidden email]>
> > > > > Cc: '[hidden email]' <hotspot-compiler-
> > > > > [hidden email]>; [hidden email]
> > > > > 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:[hidden email]]
> > > > > > Sent: sexta-feira, 11 de agosto de 2017 11:56
> > > > > > To: Gustavo Serra Scalet <[hidden email]>;
> > > > > > Doerr, Martin <[hidden email]>
> > > > > > Cc: '[hidden email]' <hotspot-compiler-
> > > > > > [hidden email]>; [hidden email]
> > > > > > 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-
> > > > > > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > > > > > Sent: Freitag, 11. August 2017 15:55
> > > > > > > To: Doerr, Martin <[hidden email]>
> > > > > > > Cc: '[hidden email]' <hotspot-compiler-
> > > > > > > [hidden email]>; [hidden email]
> > > > > > > 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-
> > > > > > > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > > > > > > Sent: quinta-feira, 10 de agosto de 2017 14:21
> > > > > > > > To: '[hidden email]'
> > > > > > > > <hotspot-compiler- [hidden email]>
> > > > > > > > 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:[hidden email]]
> > > > > > > > Sent: quarta-feira, 9 de agosto de 2017 09:58
> > > > > > > > To: Gustavo Serra Scalet <[hidden email]>;
> > > > > > > > ppc-aix-port- [hidden email]
> > > > > > > > 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-
> > > > > > > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > > > > > > Sent: Dienstag, 8. August 2017 22:19
> > > > > > > > To: [hidden email]
> > > > > > > > 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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Doerr, Martin
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 <[hidden email]>; Doerr, Martin <[hidden email]>
Cc: '[hidden email]' <[hidden email]>; [hidden email]
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:[hidden email]]
> Sent: Montag, 14. August 2017 15:13
> To: Doerr, Martin <[hidden email]>; Lindenmaier, Goetz
> <[hidden email]>
> Cc: '[hidden email]' <hotspot-compiler-
> [hidden email]>; [hidden email]
> 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:[hidden email]]
> > Sent: segunda-feira, 14 de agosto de 2017 09:53
> > To: Lindenmaier, Goetz <[hidden email]>; Gustavo Serra Scalet
> > <[hidden email]>
> > Cc: '[hidden email]' <hotspot-compiler-
> > [hidden email]>; [hidden email]
> > 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 <[hidden email]>; Doerr, Martin
> > <[hidden email]>
> > Cc: '[hidden email]' <hotspot-compiler-
> > [hidden email]>; [hidden email]
> > 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:[hidden email]]
> > > Sent: Montag, 14. August 2017 14:39
> > > To: Lindenmaier, Goetz <[hidden email]>; Doerr, Martin
> > > <[hidden email]>
> > > Cc: '[hidden email]' <hotspot-compiler-
> > > [hidden email]>; [hidden email]
> > > 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:[hidden email]]
> > > > Sent: segunda-feira, 14 de agosto de 2017 07:17
> > > > To: Doerr, Martin <[hidden email]>; Gustavo Serra Scalet
> > > > <[hidden email]>
> > > > Cc: '[hidden email]' <hotspot-compiler-
> > > > [hidden email]>; [hidden email]
> > > > 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 <[hidden email]>;
> > > > > Lindenmaier, Goetz <[hidden email]>
> > > > > Cc: '[hidden email]' <hotspot-compiler-
> > > > > [hidden email]>; [hidden email]
> > > > > 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:[hidden email]]
> > > > > Sent: Freitag, 11. August 2017 22:03
> > > > > To: Lindenmaier, Goetz <[hidden email]>; Doerr, Martin
> > > > > <[hidden email]>
> > > > > Cc: '[hidden email]' <hotspot-compiler-
> > > > > [hidden email]>; [hidden email]
> > > > > 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:[hidden email]]
> > > > > > Sent: sexta-feira, 11 de agosto de 2017 11:56
> > > > > > To: Gustavo Serra Scalet <[hidden email]>;
> > > > > > Doerr, Martin <[hidden email]>
> > > > > > Cc: '[hidden email]' <hotspot-compiler-
> > > > > > [hidden email]>; [hidden email]
> > > > > > 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-
> > > > > > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > > > > > Sent: Freitag, 11. August 2017 15:55
> > > > > > > To: Doerr, Martin <[hidden email]>
> > > > > > > Cc: '[hidden email]' <hotspot-compiler-
> > > > > > > [hidden email]>; [hidden email]
> > > > > > > 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-
> > > > > > > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > > > > > > Sent: quinta-feira, 10 de agosto de 2017 14:21
> > > > > > > > To: '[hidden email]'
> > > > > > > > <hotspot-compiler- [hidden email]>
> > > > > > > > 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:[hidden email]]
> > > > > > > > Sent: quarta-feira, 9 de agosto de 2017 09:58
> > > > > > > > To: Gustavo Serra Scalet <[hidden email]>;
> > > > > > > > ppc-aix-port- [hidden email]
> > > > > > > > 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-
> > > > > > > > [hidden email]] On Behalf Of Gustavo Serra Scalet
> > > > > > > > Sent: Dienstag, 8. August 2017 22:19
> > > > > > > > To: [hidden email]
> > > > > > > > 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
Loading...