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

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Mon Jun 24 12:31:59 PDT 2013


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