JEP 175 - Review comments
Vladimir Kozlov
vladimir.kozlov at oracle.com
Wed Jun 12 10:18:38 PDT 2013
Thanks, Goetz
I am perfectly fine with such granularity and I can start generating
RFEs. I filed first:
8016476: PPC64 (part 1): reenable CORE build
Are you sure that 0001_fix_core_build.patch is complete? I can't build
it on my Mac:
gnumake[4]: *** No rule to make target `dtrace_stuff'. Stop.
gnumake[3]: *** [dtrace_stuff] Error 2
gnumake[2]: *** [debugcore] Error 2
gnumake[1]: *** [generic_buildcore] Error 2
gnumake: *** [debugcore] Error 2
And on SPARC:
src/share/vm/runtime/globals.hpp", line 170: Error: Multiple declaration
for pd_InlineSmallCode.
And you should used debugcore instead of jvmgcore:
+all_debugcore: jvmgcore docs export_debug
I want your changes be perfect from the start ;)
And I need to discuss with our embedded group about
0002_PPC_defines.patch because it affects them. It may take time.
Thanks,
Vladimir
On 6/12/13 8:39 AM, Lindenmaier, Goetz wrote:
> Hi Vladimir,
>
> yes, the plan is that each is self contained. When I set up the patches I
> built and jckecked each. When I updated them I only built them selectively,
> so there might be minor issues. I had planned to assure this once I make
> webrevs from the patches.
>
> Some of them might apply in any order, but others depend on previous ones,
> e.g., 0009 containing the ppc files will not work without the changes before.
>
> The patches work with hs25-b34.
>
> Please tell me if I can do anything to ease your reviewing.
>
> Ah, I just saw your mail about closed code ... I tried to keep
> changes necessary to other platform code to a minimum, but also tried
> to avoid strange workarounds. Therefore I for example did change 0002
> renaming the PPC defines, see the comment there.
>
> If you agree with the granularity of the changes, it would be great if you
> could generate bug-ids for them. Maybe at least for the changes
> up to 0016?
>
> Best regards
> Goetz.
>
>
>
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Mittwoch, 12. Juni 2013 17:03
> To: Lindenmaier, Goetz
> Cc: ppc-aix-port-dev at openjdk.java.net; hotspot-dev at openjdk.java.net; Volker Simonis; Azeem Jiva; Chris Plummer
> Subject: Re: JEP 175 - Review comments
>
> Thank you, Goetz.
>
> Can I review just 1 patch (for example, 1 from first 1-9), merge it with jdk8 and build? Or I should do review all 1-9
> patches and merge them together into jdk8 to be able build? In short, is each patch self-contain?
>
> Thanks,
> Vladimir
>
> On 6/12/13 7: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