RFR: JDK-8213371: GC/C2 abstraction and cleanup to handle custom offset for GC memory accesses

Roman Kennke rkennke at redhat.com
Wed Nov 7 12:35:26 UTC 2018 

Any ideas what's up with the mach5 job?

Build Details: 2018-11-07-1047208.roman.source
21 Failed Tests
Test	Tier	Platform	Keywords	Description	Task
runtime/verifier/PrimIntArray.java 	tier1 	macosx-x64-debug 	bug8129895
othervm 	ExitCode: 134 	task
runtime/verifier/popTopTests/PopDupTop.java 	tier1 	macosx-x64-debug
bug8149607 othervm 	ExitCode: 134 	task
runtime/stackMapCheck/StackMapCheck.java 	tier1 	macosx-x64-debug
bug7127066 othervm 	ExitCode: 134 	task
runtime/handlerInTry/LoadHandlerInTry.java 	tier1 	macosx-x64-debug
bug8075118 othervm 	ExitCode: 134 	task
runtime/7116786/Test7116786.java 	tier1 	macosx-x64-debug 	othervm
ExitCode: 134 	task
compiler/types/TestMeetIncompatibleInterfaceArrays.java 	tier1
macosx-x64-debug 	bug8141551 othervm driver 	ExitCode: 134 	task
runtime/logging/ClassInitializationTest.java 	tier1 	macosx-x64-debug
bug8142976 driver 	Crash: Internal Error
...allocation.cpp...assert(~(_allocation_t[0] | allocation_mask) !=
(uintptr_t)this || !is_type_set()) failed: embedded or stack only,
this(...) type ... a[0]=(...) a[1]=(...) 	task
runtime/verifier/PrimIntArray.java 	tier1 	windows-x64-debug 	bug8129895
othervm 	ExitCode: 1 	task
runtime/verifier/popTopTests/PopDupTop.java 	tier1 	windows-x64-debug
bug8149607 othervm 	ExitCode: 1 	task
runtime/stackMapCheck/StackMapCheck.java 	tier1 	windows-x64-debug
bug7127066 othervm 	ExitCode: 1 	task
runtime/handlerInTry/LoadHandlerInTry.java 	tier1 	windows-x64-debug
bug8075118 othervm 	ExitCode: 1 	task
compiler/types/TestMeetIncompatibleInterfaceArrays.java 	tier1
windows-x64-debug 	bug8141551 othervm driver 	ExitCode: 1 	task
runtime/7116786/Test7116786.java 	tier1 	windows-x64-debug 	othervm
ExitCode: 1 	task
runtime/logging/ClassInitializationTest.java 	tier1 	windows-x64-debug
bug8142976 driver 	Crash: Internal Error
...allocation.cpp...assert(~(_allocation_t[0] | allocation_mask) !=
(uintptr_t)this || !is_type_set()) failed: embedded or stack only,
this(...) type ... a[0]=(...) a[1]=(...) 	task
runtime/verifier/PrimIntArray.java 	tier1 	linux-x64-debug 	bug8129895
othervm 	ExitCode: 134 	task
runtime/verifier/popTopTests/PopDupTop.java 	tier1 	linux-x64-debug
bug8149607 othervm 	ExitCode: 134 	task
runtime/stackMapCheck/StackMapCheck.java 	tier1 	linux-x64-debug
bug7127066 othervm 	ExitCode: 134 	task
compiler/types/TestMeetIncompatibleInterfaceArrays.java 	tier1
linux-x64-debug 	bug8141551 othervm driver 	ExitCode: 134 	task
runtime/handlerInTry/LoadHandlerInTry.java 	tier1 	linux-x64-debug
bug8075118 othervm 	ExitCode: 134 	task
runtime/7116786/Test7116786.java 	tier1 	linux-x64-debug 	othervm
ExitCode: 134 	task
runtime/logging/ClassInitializationTest.java 	tier1 	linux-x64-debug
bug8142976 driver 	Crash: Internal Error
...allocation.cpp...assert(~(_allocation_t[0] | allocation_mask) !=
(uintptr_t)this || !is_type_set()) failed: embedded or stack only,
this(...) type ... a[0]=(...) a[1]=(...) 	task
Mach5 Tasks Results Summary

    EXECUTED_WITH_FAILURE: 6
    UNABLE_TO_RUN: 0
    FAILED: 0
    KILLED: 0
    PASSED: 69
    NA: 0
    Test

        6 Executed with failure

tier1-debug-open_test_hotspot_jtreg_tier1_compiler_3-linux-x64-debug-33
Results: total: 178, passed: 177; failed: 1

tier1-debug-open_test_hotspot_jtreg_tier1_compiler_3-macosx-x64-debug-34
Results: total: 178, passed: 177; failed: 1

tier1-debug-open_test_hotspot_jtreg_tier1_compiler_3-windows-x64-debug-35 Results:
total: 178, passed: 177; failed: 1

tier1-debug-open_test_hotspot_jtreg_tier1_runtime-linux-x64-debug-51
Results: total: 435, passed: 429; failed: 6

tier1-debug-open_test_hotspot_jtreg_tier1_runtime-macosx-x64-debug-52
Results: total: 427, passed: 421; failed: 6

tier1-debug-open_test_hotspot_jtreg_tier1_runtime-windows-x64-debug-53
Results: total: 419, passed: 413; failed: 6



Am 06.11.18 um 22:24 schrieb Vladimir Kozlov:
> On 11/6/18 12:09 PM, Roman Kennke wrote:
>> Hi Vladimir,
>>
>> Thanks for reviewing!
>>
>>> Changes in compile.cpp and type.cpp look fine.
>>>
>>> I think in memnode.cpp (uint) casts are used to take into account
>>> OffsetTop and OffsetBot without explicitly checking for them. We don't
>>> expect OffsetTop here - there assert to check that. But 'off' could be
>>> OffsetBot. In line 1706 there is OffsetBot check but not in line 1737:
>>>
>>> http://hg.openjdk.java.net/jdk/jdk/file/0e8084c8cbb7/src/hotspot/share/opto/memnode.cpp#l1737
>>>
>>
>> Ah! I was wondering. So by casting we implicitly accept OffsetBot as
>> off_beyond_header=true. Ok.
>>
>>> I think you need to add explicit check OffsetBot for off_beyond_header.
>>> You did that in type.cpp.
>>
>> Yes. However, I think it's clearer to include "|| off ==
>> Type::OffsetBot" in the second use of off_beyond_header, rather than
>> including it first, and then excluding it in the first use. Like this:
>>
>> Incremental:
>> http://cr.openjdk.java.net/~rkennke/JDK-8213371/webrev.01.diff/
>> Full:
>> http://cr.openjdk.java.net/~rkennke/JDK-8213371/webrev.01/
> 
> Good.
> 
>>
>>
>> I must admit that I don't really understand the usefulness of including
>> OffsetBot in the 2nd clause, because that includes the case where offset
>> is not actually beyond the mark word, iow 'we don't know what the offset
>> is'. Right?
> 
> Bot offset is common case for array access - we have to take it into
> account.
> Accesses to header should not be Bot (except LoadKlass as comment said -
> for which there are checks). Length and markword should have fixed offsets.
> 
> Thanks,
> Vladimir
> 
>>
>> Thanks,
>> Roman
>>
>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 11/5/18 7:34 AM, Roman Kennke wrote:
>>>>    There are a few places in C2 where we need to deal with Shenandoah's
>>>> custom offset to load the forwarding pointer.
>>>>
>>>> This patch:
>>>> - Adds hook and verification to allow GC to flatten/initialize an alias
>>>> type for (e.g.) fwd ptr accesses (btw: the 'to' field in compile.cpp
>>>> around the changed code is unused)
>>>> - in memnode.cpp removes the (unnecessary?) cast from ints to uints
>>>> - in type.cpp, make the offset check more explicit about offset > 0 or
>>>> the two predefined offsets that could be negative.
>>>>
>>>> The latter two are related to the change insofar that they conflict
>>>> with
>>>> Shenandoah's -8 offset, which would require extra handling without
>>>> those
>>>> cleanups. Let me know if you want those changes separate.
>>>>
>>>> Testing: hotspot/jtreg:tier1
>>>>
>>>> Bug:
>>>> https://bugs.openjdk.java.net/browse/JDK-8213371
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~rkennke/JDK-8213371/webrev.00/
>>>>
>>>> Can I please get a review?
>>>>
>>>> Roman
>>>>
>>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20181107/b7b1227c/signature.asc>

More information about the hotspot-gc-dev mailing list