On Fri, 2 Oct 2020 02:40:16 GMT, Ziviani 
<[email protected]> wrote:

>> TestInstanceKlassSize was failing because, for PowerPC, the following code 
>> (instanceKlass.cpp) always compiles to
>> `return false;` bool InstanceKlass::has_stored_fingerprint() const {
>> #if INCLUDE_AOT
>>   return should_store_fingerprint() || is_shared();
>> #else
>>   return false;
>> #endif
>> }
>> However, in `hasStoredFingerprint()@InstanceKlass.java` the condition 
>> `shouldStoreFingerprint() || isShared();` is
>> always evaluated and may return true (_AFAIK isShared() returns true_). Such 
>> condition adds 8 bytes in the
>> `getSize()@InstanceKlass.java` causing the failure in TestInstanceKlassSize: 
>> public long getSize() { // in number of
>> bytes
>>   ...
>>   if (hasStoredFingerprint()) {
>>     size += 8; // uint64_t
>>   }
>>   return alignSize(size);
>> }
>> Considering these tests are failing for PowerPC only (_based on 
>> ProblemList.txt_), my solution checks if
>> `hasStoredFingerprint()` is running on a PowerPC platform. I decided to go 
>> this way because there is no existing flag
>> informing whether AOT is included or not and creating a new one just to 
>> handle the PowerPC case seems too much.  This
>> patch is an attempt to fix https://bugs.openjdk.java.net/browse/JDK-8230664
>
> Ziviani has refreshed the contents of this pull request, and previous commits 
> have been removed. The incremental views
> will show differences compared to the previous content of the PR.

Looks good.

-------------

Marked as reviewed by cjplummer (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/358

Reply via email to