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

Michihiro Horie HORIE at jp.ibm.com
Tue Jan 16 14:38:36 UTC 2018


Hi Martin,

Thanks a lot for the change. I noticed the guard is needed for Power7.

Webrev:
http://cr.openjdk.java.net/~mhorie/8194861/webrev.05/

Best regards,
--
Michihiro,
IBM Research - Tokyo



From:	"Doerr, Martin" <martin.doerr at sap.com>
To:	Michihiro Horie <HORIE at jp.ibm.com>
Cc:	"Lindenmaier, Goetz" <goetz.lindenmaier at sap.com>,
            "gromero at linux.vnet.ibm.com" <gromero at linux.vnet.ibm.com>,
            "hotspot-dev at openjdk.java.net" <hotspot-dev at openjdk.java.net>,
            "ppc-aix-port-dev at openjdk.java.net"
            <ppc-aix-port-dev at openjdk.java.net>
Date:	2018/01/16 19:26
Subject:	RE: RFR : PPC64 : Need support for VSR spills in ppc.ad



Hi Michihiro,

the change is incorrect for Power7.
I’ll test it with the condition
if (bottom_type()->isa_vect() != NULL && ideal_reg() == Op_VecX) {

(Power7 uses vectors with Op_RegL.)

Best regards,
Martin


From: Michihiro Horie [mailto:HORIE at jp.ibm.com]
Sent: Montag, 15. Januar 2018 14:24
To: Doerr, Martin <martin.doerr at sap.com>
Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>;
gromero at linux.vnet.ibm.com; hotspot-dev at openjdk.java.net;
ppc-aix-port-dev at openjdk.java.net
Subject: RE: RFR : PPC64 : Need support for VSR spills in ppc.ad



Hi Martin,

Thank you for pointing out the assertion mistake. I just remove this line.
I prepared new webrev that includes the change in vm_version_ppc.cpp

Webrev:
http://cr.openjdk.java.net/~mhorie/8194861/webrev.04/

Best regards,
--
Michihiro,
IBM Research - Tokyo

Inactive hide details for "Doerr, Martin" ---2018/01/15 19:44:14---Hi
Michihiro, thanks for adding reg-reg-spills. We will test"Doerr, Martin"
---2018/01/15 19:44:14---Hi Michihiro, thanks for adding reg-reg-spills. We
will test the new version.

From: "Doerr, Martin" <martin.doerr at sap.com>
To: Michihiro Horie <HORIE at jp.ibm.com>, "Lindenmaier, Goetz" <
goetz.lindenmaier at sap.com>
Cc: "gromero at linux.vnet.ibm.com" <gromero at linux.vnet.ibm.com>, "
hotspot-dev at openjdk.java.net" <hotspot-dev at openjdk.java.net>, "
ppc-aix-port-dev at openjdk.java.net" <ppc-aix-port-dev at openjdk.java.net>
Date: 2018/01/15 19:44
Subject: RE: RFR : PPC64 : Need support for VSR spills in ppc.ad




Hi Michihiro,

thanks for adding reg-reg-spills. We will test the new version.
Please note that the following assertion leads to errors:
assert(src_lo_rc == rc_vs || dst_lo_rc == rc_vs, "expected at least src or
dst is vector-scalar class");
It doesn’t fit for mem-mem-spill.

As we’re running out of time for jdk10, I prefer to keep both parts in one
webrev and push it to jdk11 when tests have passed for some time.

Best regards,
Martin


From: Michihiro Horie [mailto:HORIE at jp.ibm.com]
Sent: Samstag, 13. Januar 2018 08:29
To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
Cc: gromero at linux.vnet.ibm.com; hotspot-dev at openjdk.java.net; Doerr, Martin
<martin.doerr at sap.com>; ppc-aix-port-dev at openjdk.java.net
Subject: RE: RFR : PPC64 : Need support for VSR spills in ppc.ad


Hi Martin, Goetz,

Thank you very much for your reviews.

>Please add the bug id to the subject of RFRs. This one is 8194861.
I am sorry for lacking the bug id (, maybe again...)

>The bug could be treated as P3 bug because SuperwordUseVSX is currently
broken and leads to crashes.
I understand, thanks.

>In the MachSpillNode::implementation() implementation,
>why is there no case Reg->Reg? For other register sets,
>this is the MachSpillNode used the most.
Thank you for pointing out the lack of Reg->Reg support. I added necessary
code as well as the fix of comment above enum RC {...} (webrev.03).

>But I’m ok with leaving it switched off for jdk10 and keeping the bug
targeted to jdk11.
>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.)
Sure. I separated the diff in vm_version_ppc.cpp into another webrev below
(webrev.03-2).

Webrevs:
http://cr.openjdk.java.net/~mhorie/8194861/webrev.03/
http://cr.openjdk.java.net/~mhorie/8194861/webrev.03-2/


Best regards,
--
Michihiro,
IBM Research - Tokyo

Inactive hide details for "Lindenmaier, Goetz" ---2018/01/12 20:35:29---Hi
Michihiro,  I had a look at your change. Thanks for "Lindenmaier, Goetz"
---2018/01/12 20:35:29---Hi Michihiro, I had a look at your change. Thanks
for fixing this.

From: "Lindenmaier, Goetz" <goetz.lindenmaier at sap.com>
To: Michihiro Horie <HORIE at jp.ibm.com>, "Doerr, Martin" <
martin.doerr at sap.com>
Cc: "gromero at linux.vnet.ibm.com" <gromero at linux.vnet.ibm.com>, "
hotspot-dev at openjdk.java.net" <hotspot-dev at openjdk.java.net>, "
ppc-aix-port-dev at openjdk.java.net" <ppc-aix-port-dev at openjdk.java.net>
Date: 2018/01/12 20:35
Subject: RE: RFR : PPC64 : Need support for VSR spills in ppc.ad





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:HORIE at jp.ibm.com]
> Sent: Donnerstag, 11. Januar 2018 05:01
> To: Doerr, Martin <martin.doerr at sap.com>
> Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>;
> gromero at linux.vnet.ibm.com; hotspot-dev at openjdk.java.net; ppc-aix-
> port-dev at openjdk.java.net
> 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:
>
https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Emhorie_8194861_webrev.02_&d=DwIFAg&c=jf_iaSHvJObTbx-siA1ZOg&r=oecsIpYF-cifqq2i1JEH0Q&m=lTa5Sy3s2LYYDlIt8knA5E6kEsrEm2Eho-mnJmsSLp8&s=AFEpPhyObo6px4q35vW3Da_9uEIkEVahk1hVC4XP9Xg&e=

>
> 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" <martin.doerr at sap.com>
> To: Michihiro Horie <HORIE at jp.ibm.com>
> Cc: "Lindenmaier, Goetz" <goetz.lindenmaier at sap.com>,
> "gromero at linux.vnet.ibm.com" <gromero at linux.vnet.ibm.com>, "hotspot-
> dev at openjdk.java.net" <hotspot-dev at openjdk.java.net>, "ppc-aix-port-
> dev at openjdk.java.net" <ppc-aix-port-dev at openjdk.java.net>
> 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:HORIE at jp.ibm.com]
> Sent: Mittwoch, 10. Januar 2018 16:29
> To: Doerr, Martin <martin.doerr at sap.com>
> Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>;
> gromero at linux.vnet.ibm.com; hotspot-dev at openjdk.java.net; ppc-aix-
> port-dev at openjdk.java.net
> 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:
https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Emhorie_8194861_webrev.01_&d=DwIFAg&c=jf_iaSHvJObTbx-siA1ZOg&r=oecsIpYF-cifqq2i1JEH0Q&m=lTa5Sy3s2LYYDlIt8knA5E6kEsrEm2Eho-mnJmsSLp8&s=GWnGteyauv7jVkUbm4JcLrHN2YjL74B-K4b5K2QZeSg&e=

> <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" <martin.doerr at sap.com
> <mailto:martin.doerr at sap.com> >
> To: Michihiro Horie <HORIE at jp.ibm.com <mailto:HORIE at jp.ibm.com> >
> Cc: "ppc-aix-port-dev at openjdk.java.net <mailto:ppc-aix-port-
> dev at openjdk.java.net> " <ppc-aix-port-dev at openjdk.java.net <mailto:ppc-
> aix-port-dev at openjdk.java.net> >, "hotspot-dev at openjdk.java.net
> <mailto:hotspot-dev at openjdk.java.net> " <hotspot-dev at openjdk.java.net
> <mailto:hotspot-dev at openjdk.java.net> >, Gustavo Romero
> <gromero at linux.vnet.ibm.com <mailto:gromero at linux.vnet.ibm.com> >,
> "Lindenmaier, Goetz" <goetz.lindenmaier at sap.com
> <mailto:goetz.lindenmaier at sap.com> >
> 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:HORIE at jp.ibm.com
> <mailto:HORIE at jp.ibm.com> ]
> Sent: Mittwoch, 10. Januar 2018 07:10
> To: Doerr, Martin <martin.doerr at sap.com <mailto:martin.doerr at sap.com>
> >
> Cc: ppc-aix-port-dev at openjdk.java.net <mailto:ppc-aix-port-
> dev at openjdk.java.net> ; hotspot-dev at openjdk.java.net <mailto:hotspot-
> dev at openjdk.java.net> ; Gustavo Romero <gromero at linux.vnet.ibm.com
> <mailto:gromero at linux.vnet.ibm.com> >
> 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://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8194861&d=DwIFAg&c=jf_iaSHvJObTbx-siA1ZOg&r=oecsIpYF-cifqq2i1JEH0Q&m=lTa5Sy3s2LYYDlIt8knA5E6kEsrEm2Eho-mnJmsSLp8&s=jFIijhdEUS4C8tqd8bTQaI7VxntFTRCfMbvUBCyl0xs&e=

> <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:
https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Emhorie_8194861_webrev.00&d=DwIFAg&c=jf_iaSHvJObTbx-siA1ZOg&r=oecsIpYF-cifqq2i1JEH0Q&m=lTa5Sy3s2LYYDlIt8knA5E6kEsrEm2Eho-mnJmsSLp8&s=kyzE0n7HVHZuqP6Lh2egbfnIs0uOPZ83VOqQUNAqMqA&e=

> <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
>
>
>


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/attachments/20180116/9b924769/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: graycol.gif
Type: image/gif
Size: 105 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/attachments/20180116/9b924769/graycol-0001.gif>


More information about the ppc-aix-port-dev mailing list