RFR(XS): 8234645: ARM32: C1: PatchingStub for field access: not enough bytes

Doerr, Martin martin.doerr at sap.com
Thu Nov 28 11:08:21 UTC 2019


Hi Götz,

thanks for the review. Pushed with updated comments.

Best regards,
Martin


> -----Original Message-----
> From: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> Sent: Donnerstag, 28. November 2019 11:51
> To: Doerr, Martin <martin.doerr at sap.com>;
> christoph.goettschkes at microdoc.com; 'hotspot-compiler-
> dev at openjdk.java.net' <hotspot-compiler-dev at openjdk.java.net>
> Subject: RE: RFR(XS): 8234645: ARM32: C1: PatchingStub for field access: not
> enough bytes
> 
> Hi Martin,
> 
> the change looks good.
> I had to take a second look to grok your comment. Mentioning "2nd"
> is a bit shaky.
> I would prefer if you either move up the real comment to the
> first occurrence in the function, and add comment "// see above" at
> the others.
> 
> Or you say:
> // see comment before patching_epilog for 2nd ldr (str respectively)
> 
> Don't need a webrev for this.
> 
> Best regards,
>   Goetz.
> 
> 
> > -----Original Message-----
> > From: hotspot-compiler-dev <hotspot-compiler-dev-
> > bounces at openjdk.java.net> On Behalf Of Doerr, Martin
> > Sent: Dienstag, 26. November 2019 17:22
> > To: christoph.goettschkes at microdoc.com; 'hotspot-compiler-
> > dev at openjdk.java.net' <hotspot-compiler-dev at openjdk.java.net>
> > Subject: [CAUTION] RFR(XS): 8234645: ARM32: C1: PatchingStub for field
> > access: not enough bytes
> >
> > Hi Christoph,
> >
> > thanks for reporting the bug
> > https://bugs.openjdk.java.net/browse/JDK-8234645
> >
> > Seems like the large offset fix in mem2reg and reg2mem missed the first
> > patching stub in the long/double cases.
> > We should have nop padding there, too (for same reason as for the 2nd
> > patching stub).
> > NativeMovRegMem should always consist of 2 instructions on arm32 in
> order
> > to support larger offsets.
> >
> > Webrev:
> > http://cr.openjdk.java.net/~mdoerr/8234645_arm_padding/webrev.00/
> >
> > May I ask you to test this fix? We don't have arm32 in our testing
> landscape.
> >
> > Best regards,
> > Martin



More information about the hotspot-compiler-dev mailing list