RFR(M): 8016697: PPC64 (part 5): Use stubs to implement safefetch
Lindenmaier, Goetz
goetz.lindenmaier at sap.com
Mon Jul 15 00:41:36 PDT 2013
Hi,
I saw you pushed 5 to hotspot-emb, that's great, thanks a lot.
Best regards,
Goetz.
-----Original Message-----
From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
Sent: Mittwoch, 10. Juli 2013 17:38
To: Volker Simonis
Cc: Lindenmaier, Goetz; hotspot-dev at openjdk.java.net; ppc-aix-port-dev at openjdk.java.net
Subject: Re: RFR(M): 8016697: PPC64 (part 5): Use stubs to implement safefetch
Albert is working on other urgent issues so the work on safefetch is delayed.
Regards,
Vladimir
On 7/10/13 8:29 AM, Volker Simonis wrote:
> Hi Vladimir,
>
> what's the status of this patch?
> Are you still evaluate the required closed source changes?
>
> Regards,
> Volker
>
>
> On Wed, Jul 3, 2013 at 12:17 AM, Vladimir Kozlov
> <vladimir.kozlov at oracle.com> wrote:
>> Thank you, Goetz
>>
>> We are doing review of closed changes. When they are ready I will push.
>>
>> Thanks,
>> Vladimir
>>
>>
>> On 7/2/13 2:47 AM, Lindenmaier, Goetz wrote:
>>>
>>> Hi,
>>>
>>> Sorry for that, I didn't grok the comment. The alignment is a good idea.
>>> Fixed:
>>> http://cr.openjdk.java.net/~goetz/webrevs/8016697-all/
>>>
>>> Best regards,
>>> Goetz.
>>>
>>>
>>> -----Original Message-----
>>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>>> Sent: Dienstag, 2. Juli 2013 01:17
>>> To: Lindenmaier, Goetz
>>> Cc: 'Christian Thalinger'; 'Albert Noll (albert.noll at oracle.com)';
>>> 'ppc-aix-port-dev at openjdk.java.net'; 'hotspot-dev at openjdk.java.net'
>>> Subject: Re: RFR(M): 8016697: PPC64 (part 5): Use stubs to implement
>>> safefetch
>>>
>>> Goetz,
>>>
>>> Why you did not add 'nop' between load and return instructions on sparc?
>>> It was in assembler in .s file. The next comment said we need it:
>>>
>>> !! By convention with the trap handler we ensure there is a non-CTI
>>> !! instruction in the trap shadow.
>>>
>>> Also should we align code in stubs to keep it in one cache line?
>>>
>>> __ align(CodeEntryAlignment);
>>> *entry = __ pc();
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 6/24/13 12:31 PM, Lindenmaier, Goetz wrote:
>>>>
>>>> Hi,
>>>>
>>>> you are right, the check is redundant.
>>>> I removed it and updated the webrev and also based it on the
>>>> recent staging repo:
>>>> http://cr.openjdk.java.net/~goetz/webrevs/8016697-all/
>>>>
>>>> Best regards,
>>>> Goetz.
>>>>
>>>>
>>>> -----Original Message-----
>>>> From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
>>>> Sent: Monday, June 24, 2013 7:48 PM
>>>> To: Lindenmaier, Goetz
>>>> Cc: 'Vladimir Kozlov'; 'ppc-aix-port-dev at openjdk.java.net';
>>>> 'hotspot-dev at openjdk.java.net'
>>>> Subject: Re: RFR(M): 8016697: PPC64 (part 5): Use stubs to implement
>>>> safefetch
>>>>
>>>>
>>>> On Jun 20, 2013, at 7:01 AM, "Lindenmaier, Goetz"
>>>> <goetz.lindenmaier at sap.com> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> I implemented the safefetch stubs for x86 and sparc (was quiet simple).
>>>>> This way I could clean up the #define right away.
>>>>>
>>>>> I tested it thouroughly on
>>>>> x86_64: bsd, nt, linux, solaris
>>>>> x86_32: nt, linux
>>>>> by adding it into our internal VM.
>>>>> Tonight I will get build/test on
>>>>> sparc_64 solaris
>>>>> sparc_32 solaris
>>>>> x86_64 nt, linux
>>>>> with the openjdk ppc port, but I tested that before submitting in a
>>>>> smaller
>>>>> extend, too.
>>>>>
>>>>> Here the webrev:
>>>>> http://cr.openjdk.java.net/~goetz/webrevs/8016697-all/
>>>>
>>>>
>>>> I like this change. It removes a lot of duplication. One comment:
>>>>
>>>> + static bool is_safefetch_fault(address pc) {
>>>> + return pc != NULL &&
>>>> + (pc == _safefetch32_fault_pc ||
>>>> + pc == _safefetchN_fault_pc);
>>>> + }
>>>>
>>>> checks for pc != null. Should we remove the check here?
>>>>
>>>> + if (pc && StubRoutines::is_safefetch_fault(pc)) {
>>>> + set_cont_address(uc,
>>>> address(StubRoutines::continuation_for_safefetch_fault(pc)));
>>>> return true;
>>>> }
>>>>
>>>> -- Chris
>>>>
>>>>>
>>>>> Best regards,
>>>>> Goetz.
>>>>>
>>>>>
>>>>>
>>>>> -----Original Message-----
>>>>> From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
>>>>> Sent: Dienstag, 18. Juni 2013 22:44
>>>>> To: Lindenmaier, Goetz
>>>>> Cc: 'Vladimir Kozlov'; 'ppc-aix-port-dev at openjdk.java.net';
>>>>> 'hotspot-dev at openjdk.java.net'
>>>>> Subject: Re: RFR(M): 8016697: PPC64 (part 5): Use stubs to implement
>>>>> safefetch
>>>>>
>>>>>
>>>>> On Jun 18, 2013, at 1:18 PM, "Lindenmaier, Goetz"
>>>>> <goetz.lindenmaier at sap.com> wrote:
>>>>>
>>>>>> Ok, I will implement it on x86.
>>>>>>
>>>>>> To get a single change, you can give me the sparc patch,
>>>>>> or you extend the webrev once I updated it with the
>>>>>> x86 code.
>>>>>
>>>>>
>>>>> Sounds good. Let me know when it's there.
>>>>>
>>>>> -- Chris
>>>>>
>>>>>> If you prefer, you can also push it to some other repository, it
>>>>>> will end up in the ppc repo in time I guess.
>>>>>>
>>>>>> Best regards,
>>>>>> Goetz.
>>>>>>
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
>>>>>> Sent: Tuesday, June 18, 2013 6:59 PM
>>>>>> To: Lindenmaier, Goetz
>>>>>> Cc: Volker Simonis; Vladimir Kozlov; ppc-aix-port-dev at openjdk.java.net;
>>>>>> hotspot-dev at openjdk.java.net
>>>>>> Subject: Re: RFR(M): 8016697: PPC64 (part 5): Use stubs to implement
>>>>>> safefetch
>>>>>>
>>>>>>
>>>>>> On Jun 18, 2013, at 1:50 AM, "Lindenmaier, Goetz"
>>>>>> <goetz.lindenmaier at sap.com> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> We have it on these platforms:
>>>>>>> ia64 (nt, linux, hp_ux)
>>>>>>> parisc (hp_ux)
>>>>>>> zArch (linux)
>>>>>>> ppc (aix, linux)
>>>>>>>
>>>>>>> I would implement it on x86 & friends, you do it on sparc and wherever
>>>>>>> else you like it?
>>>>>>
>>>>>>
>>>>>> That sounds reasonable. Are we pushing this to the ppc repository
>>>>>> then?
>>>>>>
>>>>>> -- Chris
>>>>>>
>>>>>>>
>>>>>>> Best regards,
>>>>>>> Goetz.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
>>>>>>> Sent: Dienstag, 18. Juni 2013 07:13
>>>>>>> To: Volker Simonis
>>>>>>> Cc: Lindenmaier, Goetz; Vladimir Kozlov;
>>>>>>> ppc-aix-port-dev at openjdk.java.net; hotspot-dev at openjdk.java.net
>>>>>>> Subject: Re: RFR(M): 8016697: PPC64 (part 5): Use stubs to implement
>>>>>>> safefetch
>>>>>>>
>>>>>>>
>>>>>>> On Jun 17, 2013, at 10:08 AM, Volker Simonis
>>>>>>> <volker.simonis at gmail.com> wrote:
>>>>>>>
>>>>>>>> Hi Goetz,
>>>>>>>>
>>>>>>>> I think the change is good and if the other reviewers don't decide to
>>>>>>>> implement it for the current platforms we can push it.
>>>>>>>
>>>>>>>
>>>>>>> I talked with Vladimir about this today and I my opinion we should use
>>>>>>> this stub on all platforms. On which other platforms are you guys having
>>>>>>> this?
>>>>>>>
>>>>>>> -- Chris
>>>>>>>
>>>>>>>>
>>>>>>>> By the way, the makefile changes in ppc64.make will follow in patch 8
>>>>>>>> for
>>>>>>>> linux [1] and patch 14 for aix [2].
>>>>>>>> The implementation of the stubs will be in patch 9 for linux [3] and
>>>>>>>> patch
>>>>>>>> 15 for aix [4] (only the signal handling part).
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Volker
>>>>>>>>
>>>>>>>> [1]
>>>>>>>>
>>>>>>>> http://hg.openjdk.java.net/ppc-aix-port/jdk8/hotspot/file/tip/ppc_patches/0008_linux_ppc_make_changes.patch
>>>>>>>> [2]
>>>>>>>>
>>>>>>>> http://hg.openjdk.java.net/ppc-aix-port/jdk8/hotspot/file/tip/ppc_patches/0014_aix_make_changes.patch
>>>>>>>> [3]
>>>>>>>>
>>>>>>>> http://hg.openjdk.java.net/ppc-aix-port/jdk8/hotspot/file/tip/ppc_patches/0009_linux_ppc_files.patch
>>>>>>>> [4]
>>>>>>>>
>>>>>>>> http://hg.openjdk.java.net/ppc-aix-port/jdk8/hotspot/file/tip/ppc_patches/0015_aix_ppc_files.patch
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Mon, Jun 17, 2013 at 3:55 PM, Lindenmaier, Goetz <
>>>>>>>> goetz.lindenmaier at sap.com> wrote:
>>>>>>>>
>>>>>>>>> Hi,****
>>>>>>>>>
>>>>>>>>> ** **
>>>>>>>>>
>>>>>>>>> the PPC64 port uses stub routines to implement safefetch. We had
>>>>>>>>> and****
>>>>>>>>>
>>>>>>>>> have problems with inline assembly, especially with non-gcc****
>>>>>>>>>
>>>>>>>>> compilers. Stub routines allow to implement safefetch without****
>>>>>>>>>
>>>>>>>>> depending on OS or compiler, as it's the case with the current****
>>>>>>>>>
>>>>>>>>> implementation. This also allows to use a single implementation if
>>>>>>>>> an****
>>>>>>>>>
>>>>>>>>> architecture is supported on several os platforms.****
>>>>>>>>>
>>>>>>>>> ** **
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~goetz/webrevs/8016697-safefetch/****
>>>>>>>>>
>>>>>>>>> ** **
>>>>>>>>>
>>>>>>>>> Currently, we guard the code with ****
>>>>>>>>>
>>>>>>>>> #ifdef SAFEFETCH_STUBS****
>>>>>>>>>
>>>>>>>>> which is set in ppc64.make.****
>>>>>>>>>
>>>>>>>>> ** **
>>>>>>>>>
>>>>>>>>> I could also imagine an implementation that uses a pd_debug flag****
>>>>>>>>>
>>>>>>>>> or a const flag set in os_<os_cpu>.hpp that allows the C-compiler to
>>>>>>>>> ****
>>>>>>>>>
>>>>>>>>> optimize, and writing the code like this:****
>>>>>>>>>
>>>>>>>>> ** **
>>>>>>>>>
>>>>>>>>> extern "C" int pd_SafeFetch32 (int * adr, int errValue) ;****
>>>>>>>>>
>>>>>>>>> extern "C" intptr_t pd_SafeFetchN (intptr_t * adr, intptr_t
>>>>>>>>> errValue) ;***
>>>>>>>>> *
>>>>>>>>>
>>>>>>>>> inline int SafeFetch32(int* adr, int errValue) {****
>>>>>>>>>
>>>>>>>>> if (UseSafefetchStub) {****
>>>>>>>>>
>>>>>>>>> assert(StubRoutines::SafeFetch32_stub(), "stub not yet
>>>>>>>>> generated");***
>>>>>>>>> *
>>>>>>>>>
>>>>>>>>> return StubRoutines::SafeFetch32_stub()(adr, errValue);****
>>>>>>>>>
>>>>>>>>> } else {****
>>>>>>>>>
>>>>>>>>> pd_SafeFetch32(adr, errValue);****
>>>>>>>>>
>>>>>>>>> }****
>>>>>>>>>
>>>>>>>>> }****
>>>>>>>>>
>>>>>>>>> ** **
>>>>>>>>>
>>>>>>>>> Unfortunately this requires ****
>>>>>>>>>
>>>>>>>>> 1) setting the flag on all platforms****
>>>>>>>>>
>>>>>>>>> 2) renaming the safefetch function on all platfoms.****
>>>>>>>>>
>>>>>>>>> ** **
>>>>>>>>>
>>>>>>>>> Actually I would prefer this second solution as it avoids the
>>>>>>>>> preprocessor, ****
>>>>>>>>>
>>>>>>>>> and am happy to edit the sources accordingly if this finds
>>>>>>>>> acceptance.****
>>>>>>>>>
>>>>>>>>> ** **
>>>>>>>>>
>>>>>>>>> For the implementation of a safefetch_stub see****
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> http://hg.openjdk.java.net/ppc-aix-port/jdk8/hotspot/file/fce884e5ba0b/src/cpu/ppc/vm/stubGenerator_ppc.cpp
>>>>>>>>> ****
>>>>>>>>>
>>>>>>>>> ** **
>>>>>>>>>
>>>>>>>>> Could I get a review on this change, please?****
>>>>>>>>>
>>>>>>>>> ** **
>>>>>>>>>
>>>>>>>>> Thanks,****
>>>>>>>>>
>>>>>>>>> Goetz.****
>>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>
More information about the hotspot-dev
mailing list