On Thu, 24 Oct 2024 09:22:34 GMT, Thomas Stuefe <stu...@openjdk.org> wrote:
>>> This code causes test errors in
>>> `CompressedClassPointersEncodingScheme.java` on s390 and PPC64. It forces
>>> the shift to `log_cacheline` which is 7 on PPC64 and 9 on s390. The test
>>> passes when we remove "s > log_cacheline && " from the condition below.
>>
>> It's a bit late. We are close to pushing. While it should be harmless to
>> drop below alignment to below cache line size, this would be a change
>> affecting all platforms and would require all tests repeated.
>>
>> PPC/s390 are not targeted by the JEP. There had never been a discussion I am
>> aware of that these platforms have to be clean with +COH. While it's nice
>> that the changes had been contributed, I don't think that test errors on
>> these platforms should hold up pushing this RFE. Therefore, if needed, we
>> should just omit +COH part of the test for PPC/S390.
>>
>> But then, what exactly is the error? If it's just the test assuming that
>> cache line size is log 6, then the test should be fixed for ppc, not hotspot.
>>
>>> In addition, it doesn't fit to the comment which claims we should avoid
>>> shifts larger than the cacheline size. This enforces shifts to be larger
>>> (or equal to) than the cacheline size.
>>
>> ?? The comment is correct. We try to avoid hyper alignment, hence we drop
>> the shift to - if possible - log 2 cache line size. If it's equal to log 2
>> cache line size, we succeeded.
>
>> But then, what exactly is the error? If it's just the test assuming that
>> cache line size is log 6, then the test should be fixed for ppc, not hotspot.
>
> that is the problem, test assumes log2 of 6 for chacheline size
PPC log2 will be `7` (`DEFAULT_CACHE_LINE_SIZE = 128`) and for S390x it will
be `8` (`DEFAULT_CACHE_LINE_SIZE = 256`).
So I guess this change should be fine for now :
diff --git
a/test/hotspot/jtreg/runtime/CompressedOops/CompressedClassPointersEncodingScheme.java
b/test/hotspot/jtreg/runtime/CompressedOops/CompressedClassPointersEncodingScheme.java
index e04e716315a..c1be59e77ab 100644
---
a/test/hotspot/jtreg/runtime/CompressedOops/CompressedClassPointersEncodingScheme.java
+++
b/test/hotspot/jtreg/runtime/CompressedOops/CompressedClassPointersEncodingScheme.java
@@ -108,7 +108,9 @@ public static void main(String[] args) throws Exception {
long ccsSize = 128 * M;
int expectedShift = 6;
- test(forceAddress, true, ccsSize, forceAddress, expectedShift);
+ if (!Platform.isPPC() && !Platform.isS390x()) {
+ test(forceAddress, true, ccsSize, forceAddress, expectedShift);
+ }
ccsSize = 512 * M;
expectedShift = 8;
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1814627120
