JEP 175 - Review comments
David Holmes
david.holmes at oracle.com
Tue Jun 18 19:36:56 PDT 2013
Just for the record ... hotspot folk please note this thread is a
continuation of a discussion that started on the ppc-aix-port-dev list:
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-June/000525.html
and of course there is a lot of discussion on that list if you need
further details/background on things.
David
On 13/06/2013 12:44 AM, Lindenmaier, Goetz wrote:
> Hi,
>
> With my recent changes I removed some of the problems Vladimir mentioned.
>
> I also added the patches queue I maintain into our jdk8/hotspot repository,
> at hotspot/ppc_patches.
> Applied to the staging hotspot directory, the linuxppc and aixppc hotspots
> can be built.
>
> The queue contains the changes proposed by me before, with minor changes due
> to recent development:
>
> 1-9 linuxppc C-interpreter port (In our plan milestone M2.1)
> 11-15 aixppc C-interpreter port (In our plan milestone M2.2)
> 101-107 C-interpreter improvements
> 111-122 ppc C2 compiler port leading to a vm rudimentarily working
> 200-217 C2 compiler fixes, extensions etc needed for a stable and
> performant ppc port.
> Altogether currently 49 changes.
>
> Our plan was to propose the changes in the order of the queue for
> review. I'm happy to create webrevs for any of them.
>
> Vladimir, maybe the queue simplifies reviewing the port, as the changes
> are more complete. They include all later improvements by fixes or
> adaptions in merge changes.
> For why and where I renamed PPC to PPC32 see the second change in the
> queue.
>
> Best regards,
> Goetz.
>
>
> PS: This can be used as the invokedynamic repository:
> hg clone http://hg.openjdk.java.net/ppc-aix-port/jdk8/hotspot ppc-hotspot
> hg clone http://hg.openjdk.java.net/ppc-aix-port/stage/hotspot stage-hotspot
> cd stage-hotspot
> ln -s .../ppc-hotspot/ppc_patches/ .hg/patches
> hg qpush -a
>
>
>
>
>
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Dienstag, 11. Juni 2013 18:34
> To: Lindenmaier, Goetz
> Cc: Volker Simonis; Azeem Jiva; Chris Plummer
> Subject: Re: JEP 175 - Review comments
>
> Here is result of my first attempt to build/test ppc changes together
> with our closed sources.
>
> Small problems:
>
> src/share/vm/memory/allocation.hpp:209: Trailing whitespace
> src/share/vm/memory/allocation.inline.hpp:121: Trailing whitespace
> src/share/vm/opto/escape.cpp:2207: Trailing whitespace
> src/share/vm/memory/allocation.hpp:212: Trailing whitespace
> src/share/vm/opto/escape.cpp:2214: Trailing whitespace
>
> Build on MacOS:
>
> src/share/vm/opto/callGenerator.cpp: In member function 'virtual
> JVMState* VirtualCallGenerator::generate(JVMState*)':
> src/share/vm/opto/callGenerator.cpp:204: error:
> 'zero_page_read_protected' is not a member of 'os'
>
> agent/src/os/bsd/MacosxDebuggerLocal.m:54:2: error: #error UNSUPPORTED_ARCH
> agent/src/os/bsd/MacosxDebuggerLocal.m:167:4: error: #error UNSUPPORTED_ARCH
> agent/src/os/bsd/MacosxDebuggerLocal.m:591:2: error: #error UNSUPPORTED_ARCH
>
>
> We have several conflict with closed sources builds:
>
> src/share/vm/utilities/elfSymbolTable.cpp: In member function 'bool
> ElfSymbolTable::lookup(unsigned char*, int*, int*, int*)':
> src/share/vm/utilities/elfSymbolTable.cpp:97: error: comparison between
> signed and unsigned integer expressions
> src/share/vm/utilities/elfSymbolTable.cpp:124: error: comparison between
> signed and unsigned integer expressions
>
> error: no 'oopDesc** frame::interpreter_frame_mirror_addr() const'
> member function declared in class 'frame'
>
> error: no matching function for call to
> 'SharedRuntime::c_calling_convention(BasicType*&, VMRegPair*&, uint&)'
>
> I think it is due to changes like next:
>
> -#ifdef PPC
> +#ifdef PPC32
> oop* interpreter_frame_mirror_addr() const;
>
>
> Next is easy fix in our closed sources but it requires efforts from our
> side:
>
> src/share/vm/prims/methodHandles.cpp: In static member function 'static
> void MethodHandles::generate_adapters()':
> src/share/vm/prims/methodHandles.cpp:68: error: 'adapter_code_size'
> cannot be used as a function
> src/share/vm/prims/methodHandles.cpp:70: error: 'adapter_code_size'
> cannot be used as a function
>
>
> I would suggest to do such shared changes which affects different builds
> later after initial push.
>
>
> Thanks,
> Vladimir
>
> On 6/7/13 3:20 AM, Lindenmaier, Goetz wrote:
>> Hi Vladimir,
>>
>> I updated the repo to jdk8-b92. Our nightly tests built and
>> tested it successfully. In case you experience any problems
>> please tell me the details so I can fix them.
>>
>> Best regards,
>> Goetz.
>>
>> http://cr.openjdk.java.net/~simonis/ppc-aix-port/index.html
>>
>>
>> -----Original Message-----
>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>> Sent: Dienstag, 4. Juni 2013 20:58
>> To: Simonis, Volker
>> Cc: Iris Clark; Wintergerst, Michael; Lindenmaier, Goetz; Bernard Traversat; Jeannette Hung; Azeem Jiva; David Therkelsen; Mikael Vidstedt; Neil Richards; Steve Poole; luchsh at cn.ibm.com; Tim Ellison; Alan Bateman
>> Subject: Re: JEP 175 - Review comments
>>
>> Volker,
>>
>> Can you or someone update
>> http://hg.openjdk.java.net/ppc-aix-port/jdk8/hotspot to match latest
>> sources in http://hg.openjdk.java.net/jdk8/jdk8/hotspot?
>> I just tried to merge them and build Hotspot on x86 without success.
>>
>> Thanks,
>> Vladimir
>>
>> On 6/4/13 7:04 AM, Simonis, Volker wrote:
>>>
>>> We intentionally used 'porters-dev' rather than'ppc-aix-port-dev' in the
>>> beginning to address a broader audience for the initial discussions.
>>>
>>> But I'm happy to change that back to 'ppc-aix-port-dev' now.
>>>
>>> Regards,
>>> Volker
>>>
>>>
>>> ------------------------------------------------------------------------
>>> *From:* Iris Clark [iris.clark at oracle.com]
>>> *Sent:* Tuesday, June 04, 2013 3:50 PM
>>> *To:* Simonis, Volker; Wintergerst, Michael; Lindenmaier, Goetz; Bernard
>>> Traversat; Jeannette Hung; Azeem Jiva; David Therkelsen; Mikael
>>> Vidstedt; Neil Richards; Steve Poole; luchsh at cn.ibm.com; Tim Ellison;
>>> iris.clark at oracle.com
>>> *Cc:* Alan Bateman; Vladimir Kozlov
>>> *Subject:* RE: JEP 175 - Review comments
>>>
>>> Hi, Volker.
>>>
>>> Sorry about this, one more thing about the JEP...
>>>
>>> I think that the "Discussion" list probably needs to be updated to be
>>> your Project's mailing list (ppc-aix-port-dev). Right now it's listed
>>> as porters-dev.
>>>
>>> Thanks,
>>>
>>> iris
>>>
>>> *From:*Simonis, Volker [mailto:volker.simonis at sap.com]
>>> *Sent:* Tuesday, June 04, 2013 12:12 AM
>>> *To:* Iris Clark; Wintergerst, Michael; Lindenmaier, Goetz; Bernard
>>> Traversat; Jeannette Hung; Azeem Jiva; David Therkelsen; Mikael
>>> Vidstedt; Neil Richards; Steve Poole; luchsh at cn.ibm.com; Tim Ellison
>>> *Cc:* Alan Bateman; Vladimir Kozlov
>>> *Subject:* RE: JEP 175 - Review comments
>>>
>>> Hi Iris,
>>>
>>> you're right, the title is too clumsy - I just couldn't come up with
>>> something better yesterday in the evening.
>>>
>>> "PowerPC/AIX Port" sounds good to me. If nobody complains, I'll take it.
>>>
>>> Thanks,
>>> Volker
>>>
>>> ------------------------------------------------------------------------
>>>
>>> *From:*Iris Clark [iris.clark at oracle.com]
>>> *Sent:* Tuesday, June 04, 2013 2:57 AM
>>> *To:* Simonis, Volker; Wintergerst, Michael; Lindenmaier, Goetz; Bernard
>>> Traversat; Jeannette Hung; Azeem Jiva; David Therkelsen; Mikael
>>> Vidstedt; Neil Richards; Steve Poole; luchsh at cn.ibm.com
>>> <mailto:luchsh at cn.ibm.com>; Tim Ellison
>>> *Cc:* Alan Bateman; Vladimir Kozlov; iris.clark at oracle.com
>>> <mailto:iris.clark at oracle.com>
>>> *Subject:* RE: JEP 175 - Review comments
>>>
>>> Hi, Volker.
>>>
>>> I've just updated the JEP according to your suggestions.
>>>
>>> I'm not a Reviewer however, I think that the phrase "OpenJDK master
>>> repositories" in the revised title is not ideal:
>>>
>>> --- a/jep-175.md Mon May 27 23:22:51 2013 +0400
>>>
>>> +++ b/jep-175.md Mon Jun 03 18:51:18 2013 +0200
>>>
>>> @@ -1,5 +1,5 @@
>>>
>>> JEP: 175
>>>
>>> -Title: Integrate PowerPC/AIX Port into JDK 8
>>>
>>> +Title: Integrate the PowerPC/AIX Port into the OpenJDK master repositories
>>>
>>> Author: Volker Simonis
>>>
>>> Organization: SAP AG
>>>
>>> Created: 2013/1/11
>>>
>>> Thinking out loud, what about just "PowerPC/AIX Port"? JEPs are all
>>> about adding features to JDK Release Projects. There are lots of
>>> examples of JEPs [1] which don't begin with verbs, e.g. "133: Unicode
>>> 6.2", "148: Small VM", "172: DocLint", etc. The JEP itself contains
>>> additional details.
>>>
>>> Perhaps others have suggestions?
>>>
>>> Am I right with my assumption that the targeted release will still be
>>> JDK 8 (or 9/8u respectively) but that the targeted release will be set
>>> in the "Release" header field of the JEP by the OpenJDK Lead (as
>>> specified in the JEP specification)?
>>>
>>> Yes. Once that field is populated, the value will appear in the JEP
>>> index [1], see the third column.
>>>
>>> Thanks,
>>>
>>> Iris
>>>
>>> [1]: http://openjdk.java.net/jeps/0
>>>
>>> *From:*Simonis, Volker [mailto:volker.simonis at sap.com]
>>> *Sent:* Monday, June 03, 2013 10:10 AM
>>> *To:* Iris Clark; Wintergerst, Michael; Lindenmaier, Goetz; Bernard
>>> Traversat; Jeannette Hung; Azeem Jiva; David Therkelsen; Mikael
>>> Vidstedt; Neil Richards; Steve Poole; luchsh at cn.ibm.com
>>> <mailto:luchsh at cn.ibm.com>; Tim Ellison
>>> *Cc:* Alan Bateman; Vladimir Kozlov
>>> *Subject:* RE: JEP 175 - Review comments
>>>
>>> Hi,
>>>
>>> I've just updated the JEP according to your suggestions. Please find
>>> the new version attached to this mail (I haven't checked the new version
>>> in until now to give everybody a chance to comment on the changes).
>>>
>>> Am I right with my assumption that the targeted release will still be
>>> JDK 8 (or 9/8u respectively) but that the targeted release will be set
>>> in the "Release" header field of the JEP by the OpenJDK Lead (as
>>> specified in the JEP specification)?
>>>
>>> In addition to the changes proposed by you I've added the contents of
>>> the "Approach" section from Azeems "PPCAIX plan" to the "Description"
>>> section of the JEP. I've also added links to the new "PowerPC/AIX Port
>>> Integration Plan" [2] of our "PowerPC/AIX Port OpenJDK Wiki Space" [3]
>>> to the JEP.
>>>
>>> The "PowerPC/AIX Port Integration Plan" in the Wiki is intended to hold
>>> Azeems "PPCAIX plan" document.
>>>
>>> @Iris: could you please somehow arrange to give Azeem editing rights to
>>> that page?
>>>
>>> @Azeem: could you please be so kind to past the contents of the "PPCAIX
>>> plan" into that page (once you have the appropriate rights)? I saw that
>>> the document is created from an Atlassian Confluence Wiki anyway and in
>>> my unlimited naivety I imagine this could be a simple copy-and-paste
>>> operation:) If that doesn't work so easily, please let me know how I
>>> could help.
>>>
>>> If there are no objections I plan to checkin the new version of the JEP
>>> tomorrow after our telephone call.
>>>
>>> Thank you and best regards,
>>> Volker
>>>
>>> [2]: https://wiki.openjdk.java.net/pages/viewpage.action?pageId=13729959
>>> [3]: https://wiki.openjdk.java.net/display/PPCAIXPort
>>>
>>> ------------------------------------------------------------------------
>>>
>>> *From:*Iris Clark [iris.clark at oracle.com]
>>> *Sent:* Friday, May 31, 2013 8:48 PM
>>> *To:* Wintergerst, Michael; Simonis, Volker; Lindenmaier, Goetz; Bernard
>>> Traversat; Jeannette Hung; Azeem Jiva; David Therkelsen; Mikael
>>> Vidstedt; Neil Richards; Steve Poole; luchsh at cn.ibm.com
>>> <mailto:luchsh at cn.ibm.com>; Tim Ellison
>>> *Cc:* iris.clark at oracle.com <mailto:iris.clark at oracle.com>; Alan
>>> Bateman; Vladimir Kozlov
>>> *Subject:* JEP 175 - Review comments
>>>
>>> Hi, Volker.
>>>
>>> JEP 175: Integrate PowerPC/AIX Port into JDK 8
>>>
>>> http://openjdk.java.net/jeps/175
>>>
>>> We're actively working to get your JEP to Funded. We had a few comments:
>>>
>>> -Recommend that "8" be removed from the JEP title, etc.
>>>
>>> -Recommend that the first Motivation bullet clearly indicate that it is
>>> only covering PPC/AIX.
>>>
>>> -Recommend that the second Motivation bullet be modified to make it
>>> clear that it applies to Hotspot only.
>>>
>>> (Instructions for editing the JEP may be found in the "Mechanics"
>>> section at the bottom of JEP 1 [1].)
>>>
>>> Vladimir Kozlov (VM) and Alan Bateman (Core Libraries) are lined up to
>>> be the JEP's reviewers. Once they're satisfied with your
>>> changes/feedback they'll add themselves to the JEP's "Reviewed-by" line.
>>>
>>> Thanks,
>>>
>>> Iris Clark
>>>
>>> [1]: http://openjdk.java.net/jeps/1
>>>
More information about the ppc-aix-port-dev
mailing list