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