RFR : PPC64 : Need support for VSR spills in ppc.ad

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR : PPC64 : Need support for VSR spills in ppc.ad

Michihiro Horie

Hi Martin,

Would you review the following change that fixes the SLP for PPC?

In this change, I added support for VSR spills. Also, I fixed how to
specify registers in postalloc_expand for the float constant replication. I
confirmed this change works with JTREG.

Bug: https://bugs.openjdk.java.net/browse/JDK-8194861
webrev: http://cr.openjdk.java.net/~mhorie/8194861/webrev.00/
(I created a webrev under jdk/hs.)

Best regards,
--
Michihiro,
IBM Research - Tokyo
Reply | Threaded
Open this post in threaded view
|

RE: RFR : PPC64 : Need support for VSR spills in ppc.ad

Doerr, Martin
Hi Michihiro,

thanks for implementing it.

I wonder how the content of R19 should get preserved.
Shouldn't repl4F_immF_Ex use a temp register instead of R19?

SuperwordUseVSX is still not activated in vm_version_ppc.cpp. I think we should turn it on with this change (see the TODO).

We will run tests when the R19 question is clarified.

Best regards,
Martin


From: Michihiro Horie [mailto:[hidden email]]
Sent: Mittwoch, 10. Januar 2018 07:10
To: Doerr, Martin <[hidden email]>
Cc: [hidden email]; [hidden email]; Gustavo Romero <[hidden email]>
Subject: RFR : PPC64 : Need support for VSR spills in ppc.ad


Hi Martin,

Would you review the following change that fixes the SLP for PPC?

In this change, I added support for VSR spills. Also, I fixed how to specify registers in postalloc_expand for the float constant replication. I confirmed this change works with JTREG.

Bug: https://bugs.openjdk.java.net/browse/JDK-8194861
webrev: http://cr.openjdk.java.net/~mhorie/8194861/webrev.00/
(I created a webrev under jdk/hs.)

Best regards,
--
Michihiro,
IBM Research - Tokyo
Reply | Threaded
Open this post in threaded view
|

RE: RFR : PPC64 : Need support for VSR spills in ppc.ad

Doerr, Martin
In reply to this post by Michihiro Horie
Hi Michihiro,

> R19 is commented out in bits64_constant_table_base so as not to be used:
Not to be used by MachConstantBaseNode. I don’t see how this should help.

I still suggest to add “iRegLdst tmp” with “effect(TEMP tmp)” to repl4F_immF_Ex.

Best regards,
Martin


From: Michihiro Horie [mailto:[hidden email]]
Sent: Mittwoch, 10. Januar 2018 16:29
To: Doerr, Martin <[hidden email]>
Cc: Lindenmaier, Goetz <[hidden email]>; [hidden email]; [hidden email]; [hidden email]
Subject: RE: RFR : PPC64 : Need support for VSR spills in ppc.ad

Hi Martin,

Thanks a lot for your review.

>I wonder how the content of R19 should get preserved.
R19 is commented out in bits64_constant_table_base so as not to be used:
reg_class bits64_constant_table_base(
       :
  R18_H, R18,
/*R19_H, R19*/
  R20_H, R20,
       :
);

Although bits64_constant_table_base is not directly referred from anywhere, it seems to be used at the following line in ppc.ad:
const RegMask& MachConstantBaseNode::_out_RegMask = BITS64_CONSTANT_TABLE_BASE_mask();
(When I remove the declaration of bits64_constant_table_base, a build error arises at this line telling the lack of the declaration.)

>Shouldn’t repl4F_immF_Ex use a temp register instead of R19?
Thank you for the suggestion, which makes sense very much. I think I can declare temp registers in repl4F_immF_Ex.
I will try this approach if using R19 does not make sense.

I changed vm_version_ppc.cpp:
webrev: http://cr.openjdk.java.net/~mhorie/8194861/webrev.01/

Best regards,
--
Michihiro,
IBM Research - Tokyo


----- Original message -----
From: "Doerr, Martin" <[hidden email]<mailto:[hidden email]>>
To: Michihiro Horie <[hidden email]<mailto:[hidden email]>>
Cc: "[hidden email]<mailto:[hidden email]>" <[hidden email]<mailto:[hidden email]>>, "[hidden email]<mailto:[hidden email]>" <[hidden email]<mailto:[hidden email]>>, Gustavo Romero <[hidden email]<mailto:[hidden email]>>, "Lindenmaier, Goetz" <[hidden email]<mailto:[hidden email]>>
Subject: RE: RFR : PPC64 : Need support for VSR spills in ppc.ad
Date: Wed, Jan 10, 2018 8:17 PM


Hi Michihiro,



thanks for implementing it.



I wonder how the content of R19 should get preserved.

Shouldn’t repl4F_immF_Ex use a temp register instead of R19?



SuperwordUseVSX is still not activated in vm_version_ppc.cpp. I think we should turn it on with this change (see the TODO).



We will run tests when the R19 question is clarified.



Best regards,

Martin





From: Michihiro Horie [mailto:[hidden email]]
Sent: Mittwoch, 10. Januar 2018 07:10
To: Doerr, Martin <[hidden email]<mailto:[hidden email]>>
Cc: [hidden email]<mailto:[hidden email]>; [hidden email]<mailto:[hidden email]>; Gustavo Romero <[hidden email]<mailto:[hidden email]>>
Subject: RFR : PPC64 : Need support for VSR spills in ppc.ad



Hi Martin,

Would you review the following change that fixes the SLP for PPC?

In this change, I added support for VSR spills. Also, I fixed how to specify registers in postalloc_expand for the float constant replication. I confirmed this change works with JTREG.

Bug: https://bugs.openjdk.java.net/browse/JDK-8194861<https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8194861&d=DwMFAg&c=jf_iaSHvJObTbx-siA1ZOg&r=oecsIpYF-cifqq2i1JEH0Q&m=6pCaUQ4ll6S-hRCrDXsSKjl9NrczviVl3vOv0-KnizA&s=HtvmfHT8WviFbLWwFAnJ1KOrzvPiwqleI_inLbbJqTE&e=>
webrev: http://cr.openjdk.java.net/~mhorie/8194861/webrev.00/
(I created a webrev under jdk/hs.)

Best regards,
--
Michihiro,
IBM Research - Tokyo


Reply | Threaded
Open this post in threaded view
|

RE: RFR : PPC64 : Need support for VSR spills in ppc.ad

Lindenmaier, Goetz
Hi Michihiro,

I had a look at your change. Thanks for fixing this.

In the MachSpillNode::implementation() implementation,
why is there no case Reg->Reg? For other register sets,
this is the MachSpillNode used the most.

In case this is pushed to jdk10, I would appreciate to
leave the flag off.  (I.e., push without the change to
vm_version_ppc.cpp.)
Usage of this is probably quite rare, so that uncommon
effects e.g. when spilling to the stack are unlikely
to be catched by the remaining testing before delivery.

Please adapt the comment above enum RC {...}

Besides this, the change looks good!

Best regards,
  Goetz.




> -----Original Message-----
> From: Michihiro Horie [mailto:[hidden email]]
> Sent: Donnerstag, 11. Januar 2018 05:01
> To: Doerr, Martin <[hidden email]>
> Cc: Lindenmaier, Goetz <[hidden email]>;
> [hidden email]; [hidden email]; ppc-aix-
> [hidden email]
> Subject: RE: RFR : PPC64 : Need support for VSR spills in ppc.ad
>
> Hi Martin,
>
> >Not to be used by MachConstantBaseNode.
> Thank you for the confirmation.
>
> >I still suggest to add "iRegLdst tmp" with "effect(TEMP tmp)" to
> repl4F_immF_Ex.
> Sure, I updated code with a tmp register instead of R19:
> http://cr.openjdk.java.net/~mhorie/8194861/webrev.02/
>
> Best regards,
> --
> Michihiro,
> IBM Research - Tokyo
>
> "Doerr, Martin" ---2018/01/11 02:19:30---Hi Michihiro, > R19 is commented
> out in bits64_constant_table_base so as not to be used:
>
> From: "Doerr, Martin" <[hidden email]>
> To: Michihiro Horie <[hidden email]>
> Cc: "Lindenmaier, Goetz" <[hidden email]>,
> "[hidden email]" <[hidden email]>, "hotspot-
> [hidden email]" <[hidden email]>, "ppc-aix-port-
> [hidden email]" <[hidden email]>
> Date: 2018/01/11 02:19
> Subject: RE: RFR : PPC64 : Need support for VSR spills in ppc.ad
>
> ________________________________
>
>
>
>
> Hi Michihiro,
>
> > R19 is commented out in bits64_constant_table_base so as not to be used:
> Not to be used by MachConstantBaseNode. I don't see how this should help.
>
> I still suggest to add "iRegLdst tmp" with "effect(TEMP tmp)" to
> repl4F_immF_Ex.
>
> Best regards,
> Martin
>
>
> From: Michihiro Horie [mailto:[hidden email]]
> Sent: Mittwoch, 10. Januar 2018 16:29
> To: Doerr, Martin <[hidden email]>
> Cc: Lindenmaier, Goetz <[hidden email]>;
> [hidden email]; [hidden email]; ppc-aix-
> [hidden email]
> Subject: RE: RFR : PPC64 : Need support for VSR spills in ppc.ad
>
> Hi Martin,
>
> Thanks a lot for your review.
>
> >I wonder how the content of R19 should get preserved.
> R19 is commented out in bits64_constant_table_base so as not to be used:
> reg_class bits64_constant_table_base(
> :
> R18_H, R18,
> /*R19_H, R19*/
> R20_H, R20,
> :
> );
>
> Although bits64_constant_table_base is not directly referred from
> anywhere, it seems to be used at the following line in ppc.ad:
> const RegMask& MachConstantBaseNode::_out_RegMask =
> BITS64_CONSTANT_TABLE_BASE_mask();
> (When I remove the declaration of bits64_constant_table_base, a build error
> arises at this line telling the lack of the declaration.)
>
> >Shouldn't repl4F_immF_Ex use a temp register instead of R19?
> Thank you for the suggestion, which makes sense very much. I think I can
> declare temp registers in repl4F_immF_Ex.
> I will try this approach if using R19 does not make sense.
>
> I changed vm_version_ppc.cpp:
> webrev: http://cr.openjdk.java.net/~mhorie/8194861/webrev.01/
> <https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__cr.openjdk.java.net_-
> 7Emhorie_8194861_webrev.01_&d=DwMGaQ&c=jf_iaSHvJObTbx-
> siA1ZOg&r=oecsIpYF-
> cifqq2i1JEH0Q&m=SOGOvG8yFchCvPeC1Pr5_BJNIaBdg1G1IXaEeMgwpWI&s
> =kFfDKuiMdVnWKBy0FOjXCTzN430PRKsHQgSCRWkdR8w&e=>
>
> Best regards,
> --
> Michihiro,
> IBM Research - Tokyo
>
>
> ----- Original message -----
> From: "Doerr, Martin" <[hidden email]
> <mailto:[hidden email]> >
> To: Michihiro Horie <[hidden email] <mailto:[hidden email]> >
> Cc: "[hidden email] <mailto:ppc-aix-port-
> [hidden email]> " <[hidden email] <mailto:ppc-
> [hidden email]> >, "[hidden email]
> <mailto:[hidden email]> " <[hidden email]
> <mailto:[hidden email]> >, Gustavo Romero
> <[hidden email] <mailto:[hidden email]> >,
> "Lindenmaier, Goetz" <[hidden email]
> <mailto:[hidden email]> >
> Subject: RE: RFR : PPC64 : Need support for VSR spills in ppc.ad
> Date: Wed, Jan 10, 2018 8:17 PM
>
>
> Hi Michihiro,
>
> thanks for implementing it.
>
> I wonder how the content of R19 should get preserved.
>
> Shouldn't repl4F_immF_Ex use a temp register instead of R19?
>
> SuperwordUseVSX is still not activated in vm_version_ppc.cpp. I think we
> should turn it on with this change (see the TODO).
>
> We will run tests when the R19 question is clarified.
>
> Best regards,
>
> Martin
>
> From: Michihiro Horie [mailto:[hidden email]
> <mailto:[hidden email]> ]
> Sent: Mittwoch, 10. Januar 2018 07:10
> To: Doerr, Martin <[hidden email] <mailto:[hidden email]>
> >
> Cc: [hidden email] <mailto:ppc-aix-port-
> [hidden email]> ; [hidden email] <mailto:hotspot-
> [hidden email]> ; Gustavo Romero <[hidden email]
> <mailto:[hidden email]> >
> Subject: RFR : PPC64 : Need support for VSR spills in ppc.ad
>
> Hi Martin,
>
> Would you review the following change that fixes the SLP for PPC?
>
> In this change, I added support for VSR spills. Also, I fixed how to specify
> registers in postalloc_expand for the float constant replication. I confirmed
> this change works with JTREG.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8194861
> <https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__bugs.openjdk.java.net_browse_JDK-
> 2D8194861&d=DwMFAg&c=jf_iaSHvJObTbx-siA1ZOg&r=oecsIpYF-
> cifqq2i1JEH0Q&m=6pCaUQ4ll6S-hRCrDXsSKjl9NrczviVl3vOv0-
> KnizA&s=HtvmfHT8WviFbLWwFAnJ1KOrzvPiwqleI_inLbbJqTE&e=>
> webrev: http://cr.openjdk.java.net/~mhorie/8194861/webrev.00
> <https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__cr.openjdk.java.net_-
> 7Emhorie_8194861_webrev.00&d=DwMGaQ&c=jf_iaSHvJObTbx-
> siA1ZOg&r=oecsIpYF-
> cifqq2i1JEH0Q&m=SOGOvG8yFchCvPeC1Pr5_BJNIaBdg1G1IXaEeMgwpWI&s
> =RceHIxj8obNFdZExh6_lDeUnogfNv-qGZWuh_RRuHRQ&e=> /
> (I created a webrev under jdk/hs.)
>
> Best regards,
> --
> Michihiro,
> IBM Research - Tokyo
>
>
>