guarantee RTMSpinLoopCount > 0

Thu Dec 15 20:53:12 UTC 2016

Hi Martin,

On 15-12-2016 12:50, Doerr, Martin wrote:
> Hi Gustavo,
> 
> thanks for this information.
> 
>> One thing that caught my attention is the use of smt_yield(), because in an internal discussion I've learned that yield, mdoio, and mdoom are not implemented on POWER8 (although listed in the ISA 2.07). The toolchain team suggested an or 31,31,31 instead for setting low priority in that case. But, of course, it's another story and since smt_yield() use is restricted to the RTM feature we can discuss that in another thread (I'll experiment a little on it by myself and report back).
> 
> We didn't find any feasible replacement for x86's pause instruction.

As I understand it setting the PPR register is a correct replacement for the
x86 pause. As per ISA 2.07 smt_yield() would fit indeed as a surrogate for pause,
but it just happens that on POWER it never kicks in so the priority does not
change with or 27,27,27 (yield was removed from PowerPC ISA 3.0 / POWER9 btw).

> "or 31,31,31" is already implemented as "smt_prio_very_low()". But I think it shouldn't be used within a spin loop, right?
> It should rather get executed once before a spin loop with some kind of slow instructions (and reset later) as I understand it.
> Do you agree?

Actually it seems that or 31,31,31 does not take many resources at all being
pretty much a nop. So in theory not a problem to keep that in the middle of the
spin loop, as it is now.

But I prefer to have that executed before the spin loop as you said. I agree
that resetting it to medium later is necessary (by using smt_prio_medium(), for
instance).

> We haven't looked for a more sophisticated solution, yet. E.g. one could generate a signal and call yield in the signal handler.
> But I guess a signal would be too expensive.

Yup, I agree. Also, if setting the PPR is of any help it has to be as we are
reasoning now.

>> Finally, do you know exactly what marks (if anything formal) the transition from an experimental JVM feature (like RTM in general) to a non-experimental one?
> 
> We can decide how long it should be experimental. x86 has already made it a product flag. If it's mature and tested well enough, we can make it available.

Thanks for clarifying.

> The webrev looks good.

Thanks for reviewing the change. I'll start the formal process for reviewing
that on the hs ML.

Best regards,
Gustavo

> Best regards,
> Martin
> 
> 
> -----Original Message-----
> From: Gustavo Romero [mailto:gromero at linux.vnet.ibm.com] 
> Sent: Donnerstag, 15. Dezember 2016 15:05
> To: Doerr, Martin <martin.doerr at sap.com>; ppc-aix-port-dev at openjdk.java.net
> Subject: Re: guarantee RTMSpinLoopCount > 0
> 
> Hi Martin,
> 
> On 13-12-2016 08:38, Doerr, Martin wrote:
>> Btw. RTMSpinLoopCount = 0 leads to the same behavior as RTMSpinLoopCount = 1 on x86. Maybe it's a bug and should better be " movptr(scr_Reg, RTMSpinLoopCount + 1)".
>> I think this was the reason for not supporting the value 0 on PPC64.
>> Checking for "if (RTMSpinLoopCount > 1)" would be more reasonable in my code below.
> 
> hmm, right. Well, anyway I understood that we've decided to follow x64's behavior, so I used RTMSpinLoopCount > 1 as you suggested.
> 
> I also changed a nit on input description: s/box_Reg/owner_addr_Reg/.
> 
> And thought it would be a good idea to not exit abruptly if RTMSpinLoopCount < 0 is passed, so I set RTMSpinLoopCount = 0 issuing a warning to the user in that case.
> 
> One thing that caught my attention is the use of smt_yield(), because in an internal discussion I've learned that yield, mdoio, and mdoom are not implemented on POWER8 (although listed in the ISA 2.07). The toolchain team suggested an or 31,31,31 instead for setting low priority in that case. But, of course, it's another story and since smt_yield() use is restricted to the RTM feature we can discuss that in another thread (I'll experiment a little on it by myself and report back).
> 
> Finally, do you know exactly what marks (if anything formal) the transition from an experimental JVM feature (like RTM in general) to a non-experimental one?
> 
> I filed a bug here: https://bugs.openjdk.java.net/browse/JDK-8171266
> 
> and updated a "draft" webrev here: http://cr.openjdk.java.net/~gromero/8171266/0/
> 
> Thank you.
> 
> 
> Regards,
> Gustavo
> 
> 
>>
>> -----Original Message-----
>> From: ppc-aix-port-dev 
>> [mailto:ppc-aix-port-dev-bounces at openjdk.java.net] On Behalf Of Doerr, 
>> Martin
>> Sent: Dienstag, 13. Dezember 2016 11:21
>> To: Gustavo Romero <gromero at linux.vnet.ibm.com>; 
>> ppc-aix-port-dev at openjdk.java.net
>> Subject: RE: guarantee RTMSpinLoopCount > 0
>>
>> Hi Gustavo,
>>
>> I think it's good to support RTMSpinLoopCount = 0.
>> I'd simply not generate the loop code in this case:
>>
>>  if (RTMSpinLoopCount > 0) {
>>    li(R0, RTMSpinLoopCount);
>>    mtctr(R0);
>>  }
>>
>>  if (RTMSpinLoopCount > 0) {
>>    bdz(retryLabel);
>>    ld(R0, 0, owner_addr_Reg);
>>    cmpdi(CCR0, R0, 0);
>>    bne(CCR0, SpinLoop);
>>  }
>>
>> Generated RTM code is already very large so making it shorter sounds 
>> good :-)
>>
>> Thanks and best regards,
>> Martin
>>
>>
>> -----Original Message-----
>> From: ppc-aix-port-dev 
>> [mailto:ppc-aix-port-dev-bounces at openjdk.java.net] On Behalf Of 
>> Gustavo Romero
>> Sent: Dienstag, 13. Dezember 2016 00:01
>> To: ppc-aix-port-dev at openjdk.java.net
>> Subject: guarantee RTMSpinLoopCount > 0
>>
>> Hi!
>>
>> Jtreg test hotspot/test/compiler/rtm/locking/TestRTMSpinLoopCount.java 
>> is failing on POWER8 due to the guarantee in
>>
>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/88847a1b3718#l21.110
>>
>> hotspot/test/compiler/rtm/locking/TestRTMSpinLoopCount.java uses
>> -XX:RTMSpinLoopCount=0 and so the JVM exit in one occasion through VMError::report_and_die().
>>
>> x64 seems to not have that restriction since it checks for RTMSpinLoopCount less or equal to zero:
>>
>> http://hg.openjdk.java.net/jdk9/hs/hotspot/file/3b4e4dbb855d/src/cpu/x
>> 86/vm/macroAssembler_x86.cpp#l1457
>>
>> Could we do the same check on PPC64, i.e. in
>> http://hg.openjdk.java.net/jdk9/hs/hotspot/file/3b4e4dbb855d/src/cpu/p
>> pc/vm/macroAssembler_ppc.cpp#l2585
>>
>> check for a less or equal on decrement and guarantee(RTMSpinLoopCount >= 0) instead?
>>
>> Thanks.
>>
>> Gustavo
>>
> 