JEP 175 - Review comments

Steve Poole spoole at linux.vnet.ibm.com
Tue Jun 11 23:28:12 PDT 2013 

When you say 'closed code' what are we talking about here?  What functionality? 

Just to be clear - fixing up the open code to suit Oracles closed code is not our responsibility and should not be a gate to the merging in of the ppc-aix project.   




 On 11 Jun 2013, at 17:36, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

> Forgot to include ppc-aix mail alias. Which Hotspot mail alias we will use for reviews?
> 
> Thanks,
> Vladimir
> 
> 
> -------- Original Message --------
> Subject: Re: JEP 175 - Review comments
> Date: Tue, 11 Jun 2013 09:33:59 -0700
> From: Vladimir Kozlov <vladimir.kozlov at oracle.com>
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> CC: Volker Simonis <volker.simonis at gmail.com>,  Azeem Jiva <azeem.jiva at oracle.com>, Chris Plummer <chris.plummer at oracle.com>
> 
> 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