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

Mon Jul 1 16:17:00 PDT 2013

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.****
>>>>>>
>>>>
>>>
>>
>