RFR(M): 8016697: PPC64 (part 5): Use stubs to implement safefetch

Volker Simonis volker.simonis at gmail.com
Wed Jul 10 08:29:26 PDT 2013 

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 ppc-aix-port-dev mailing list