On Fri, 25 Sep 2020 13:13:44 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

Can you please update the JBS issue to accurately describe what the underlying 
cause is. It incorreclty states that it
is 8-byte vs 16-byte aligment.

I'd prefer that you added someting like VM.hasAOT(). This will fix the problem 
for other CPU ports that may be in a
similar situation and also ensure correctness when configure a build with 
`--disable-aot`.

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

Changes requested by cjplummer (Reviewer).

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

Reply via email to