[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-compiler-dev
Subject:    Re: RFR(s): 8205582: PPC64: RTM: Fix counter for aborts on nested transactions
From:       Gustavo Romero <gromero () linux ! vnet ! ibm ! com>
Date:       2018-06-27 15:26:02
Message-ID: 1dee4694-2136-9869-3f31-bb78b5dcb28e () linux ! vnet ! ibm ! com
[Download RAW message or body]

Hi Martin,

On 06/25/2018 01:31 PM, Doerr, Martin wrote:
> Hi Gustavo,
> 
> thanks for addressing this issue.

Thanks lot for reviewing.


> I think it would be better to ignore bit 63 in the macroAssembler code and use the \
> definition from the spec in assembler_ppc.hpp. Somebody may want to use the \
> definition for other purposes.

Sure.


> I wonder if Assembler::tm_trans_cf | Assembler::tm_non_trans_cf would be a better \
> match for x86's description for tm_failure_bit[2]. It's also a little unfortunate \
> to print the same bit twice as tm_failure_bit[4].

Absolutely. Matching (Assembler::tm_trans_cf | Assembler::tm_non_trans_cf)
is the most correct way to address it on Power (even if jtreg tests don't
seem to stress the tm_trans_cf cases).

It's indeed unfortunate to generate an asm match rule for tm_failure_bit[4]
twice also. I'm fixing that right now:

The first idea that come up to me is to iterate over the state bits rather
than over the counters. Since it was first designed on Intel counters and
abort_status match 1-by-1 perfectly on x64, so iterating over counters is
just fine. But on Power we can have n (state bits)-to-1 (counter), like
(tm_trans_cf | Assembler::tm_non_trans_cf) -> 1 (abort type 2 counter).
Iterating over state bits also avoids the unfortunate tm_failure_bit[4]
double hit. I'm thinking to use a matrix like the following to track these
relations:


   // RTM counters                             confl.,  aborts, nests, footprint // \
bits in TEXASR  bool tm_counter[][counter::NUM_COUNTER] = {{ true ,  false , false, \
                false },  // bit conflict_tm
                                              { true ,  false , false, false },  // \
                bit conflict_non_tm
                                              { false,  true  , false, false },  // \
                bit abort
                                              { false,  false , false, true  },  // \
                bit footprint
                                              { true ,  false , true , false }}; // \
bit nested


That way we can expand and adapt the relations 'state bits vs counters vs
state bit size (when there is more than 1 bit to match)' easily.


Best regards,
Gustavo
  
> Best regards,
> Martin
> 
> 
> -----Original Message-----
> From: Gustavo Romero [mailto:gromero@linux.vnet.ibm.com]
> Sent: Montag, 25. Juni 2018 10:19
> To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com>; Doerr, Martin \
>                 <martin.doerr@sap.com>; hotspot-compiler-dev@openjdk.java.net
> Cc: ppc-aix-port-dev@openjdk.java.net
> Subject: RFR(s): 8205582: PPC64: RTM: Fix counter for aborts on nested transactions
> 
> Hi,
> 
> Could the following change be reviewed please?
> 
> bug   : https://bugs.openjdk.java.net/browse/JDK-8205582
> webrev: http://cr.openjdk.java.net/~gromero/8205582/v1/
> 
> It fixes the RTM counter for nested aborts (rtm lock aborts type 5) by
> extracting and checking bits in the Transactional Level field of TEXASR
> register.
> 
> It also fixes the memory conflict counter (rtm lock aborts type 2). Power TM
> status register supports two bits to inform two different types of memory
> conflict between threads: non-transactional and transactional. According to how
> the jtreg RTM tests are designed the memory conflict counter counts
> non-transactional conflicts: on TestPrintPreciseRTMLockingStatistics a RTM lock
> is held on a static variable while another thread without any synchronization
> (non-trasactional) tries to modify the same variable. Hence that small
> adjustment satisfies the TestPrintPreciseRTMLockingStatistics making it pass on
> Power. The memory conflict counter is not used in any other place besides by the
> RTM precise statistics (no decision is made by the JVM based on that amount).
> 
> This change partially fixes some failures in
> compiler/rtm/print/TestPrintPreciseRTMLockingStatistics.java regarding the
> nested and memory conflict abort counters. The remaining issue will be fixed by
> aborting on calling JNI (next RFR).
> 
> 
> Thank you and best regards,
> Gustavo
> 


[prev in list] [next in list] [prev in thread] [next in thread]