RFR: 8169529: AArch64: Revert old JDK-8167595 changes after JDK-8159035 fix is pushed
rahul.v.raghavan at oracle.com
Fri Nov 18 08:52:01 UTC 2016
Please review updated webrev -
hope new changes covers all earlier comments.
> -----Original Message-----
> From: Rahul Raghavan
> Sent: Thursday, November 17, 2016 8:39 PM
> To: Andrew Dinn; Tobias Hartmann; Andrew Haley; aarch64-port-dev at openjdk.java.net
> Cc: Vladimir Kozlov; hotspot-compiler-dev at openjdk.java.net
> Subject: RE: RFR: 8169529: AArch64: Revert old JDK-8167595 changes after JDK-8159035 fix is pushed
> Sorry I missed that JDK-8159035 was Oracle closed bug.
> Thank you Tobias for forwarding the open changeset link
> and thanks for all review comments.
> I will check the pointers from Andrew Dinn and will submit revised webrev for review.
> - Rahul
> > -----Original Message-----
> > From: Andrew Dinn [mailto:adinn at redhat.com]
> > Sent: Thursday, November 17, 2016 6:14 PM
> > To: Tobias Hartmann; Andrew Haley; Rahul Raghavan; ningsheng.jian at linaro.org; Vladimir Kozlov; aarch64-port-
> > dev at openjdk.java.net
> > Cc: hotspot-compiler-dev at openjdk.java.net
> > Subject: Re: RFR: 8169529: AArch64: Revert old JDK-8167595 changes after JDK-8159035 fix is pushed
> > On 17/11/16 11:57, Tobias Hartmann wrote:
> > > this looks good to me. Here are the changes for JDK-8159035 (should have been reviewed in the open):
> > > http://hg.openjdk.java.net/jdk9/hs/jdk/rev/0e98c765ce9b
> > Thanks, Tobias.
> > I understand that a zero check is no longer needed but I am not sure it
> > is correct to revert the AArch64 change. For example, at line 2820+ the
> > following change is being backed out
> > - __ subw(len_reg, len_reg, 16);
> > - __ cbnzw(len_reg, L_aes_loop);
> > + __ sub(len_reg, len_reg, 16);
> > + __ cbnz(len_reg, L_aes_loop);
> > I think the use of subw and cbnzw was one of Andrew's mods to the
> > originally proposed patch. It ensures that the contents of len_reg are
> > correctly treated as a 32-bit int. Similarly, the proposed removal of
> > the zero test at line 2759 once again risks treating the 32 bit length
> > as a 64 bit quantity
> > - __ subsw(rscratch2, len_reg, zr);
> > - __ br(Assembler::LE, _);
> > + __ mov(rscratch2, len_reg);
> > I think that should revert to a movw (of course this still makes the
> > change to add label L_finish redundant).
> > So, rather than completely /revert/ the change I think it needs
> > /correcting/ just to remove the zero check.
> > regards,
> > Andrew Dinn
> > -----------
> > Senior Principal Software Engineer
> > Red Hat UK Ltd
> > Registered in England and Wales under Company Registration No. 03798903
> > Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
More information about the hotspot-compiler-dev