Review Request: Regression Shark fails to JIT in hs20

[Dr Andrew John Hughes]

On 01:43 Thu 21 Apr     , Xerxes Rånby wrote:
> On 2011-04-21 00:38, Dr Andrew John Hughes wrote:
> > On 16:00 Wed 20 Apr     , Xerxes Rånby wrote:
> >> Hi
> >>
> >> Shark JIT fails to work when using HotSpot 20.
> >>
> >> This webrev fix by backporting Shark fixes from OpenJDK bug 7032458.
> >> http://labb.zafena.se/openjdk/pr690-shark-jit-regression-hs20/
> >>
> >> The webrev are made against hsx20
> >> http://hg.openjdk.java.net/hsx/hsx20/master/
> >>
> >> I don't have a bug id for this.
> >>
> >
> > AIUI, commits to hs20 are only allowed by Oracle employees.
> > At least, that's what I've been told in the past.  So this
> > will need to go into the OpenJDK6 HotSpot tree once hs20 is
> > merged.
> I agree its not clear how to file patches for regressions to the hsXX 
> branches.
> I got recommended by Chris Phillips on IRC to send in the patch for 
> review with a long cc list.
> 

I'm just repeating what I was told when I wanted to do the same thing.
I don't agree with it FWIW.

> >
> > If this is a backport of 7032458, why do you need a bug ID?
> 7032458 contains more than one fix (one zero fix, this shark fix and one 
> parts that do not belong in hs20 at all). Preferably i would like a bug 
> ID that handles this Shark regression for the hs20 branch.
> 

I agree with your reasoning.  I complained about this patch at the time:

http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2011-March/005021.html

> >
> > As a side note, the server you are uploading the webrev to does
> > not have mime types set up correctly.  I couldn't view the patch
> > without being asked to download it.
> odd, my server used the correct patch mime type of
> text/x-diff for diff and patch files.
> I have now changed my server to instead use
> text/plain for diff and patch files to ease review.
> 

Thanks.  It works now.
text/x-diff is probably right but both Firefox and Chromium don't seem to think so.
Or rather, they are too stupid to realise they can display it.

> 
>    It is viewable on cr.openjdk.java.net.
> > Is there a reason you aren't using this?
> I do not have an account on that server. Only users with push access to 
> the OpenJDK mercurial server can use the cr.openjdk.java.net server:
> http://openjdk.java.net/guide/codeReview.html
> 

I kinda assumed you had push access as you posted a patch for review...
But I or someone else can easily push it for you once it is approved
and has a bug ID.

jdk6 patches have to be reviewed by Kelly for some reason, otherwise
I could just be the reviewer on this one.  And also, as I mentioned before,
OpenJDK6 needs to have the merge approved first:

http://mail.openjdk.java.net/pipermail/jdk6-dev/2011-April/002392.html

> >
> >> Cheers
> >> Xerxes
> >
> 

-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D  0698 0713 C3ED F586 2A37