RFR: JDK-8203157: Object equals abstraction for BarrierSetAssembler

[Roman Kennke]

jdk/submit came back with unstable (see below). Can somebody with access
look what's going on?

Thanks, Roman

Build Details: 2018-06-12-1147472.roman.source
0 Failed Tests
Mach5 Tasks Results Summary

    PASSED: 62
    KILLED: 0
    FAILED: 0
    UNABLE_TO_RUN: 11
    EXECUTED_WITH_FAILURE: 2
    NA: 0
    Build

        2 Not run
            build_jdk_linux-linux-x64-debug-linux-x64-build-1 error
while building, return value: 2
            build_jdk_linux-linux-x64-open-debug-linux-x64-build-3 error
while building, return value: 2

    Test

        11 Not run

tier1-debug-jdk_open_test_hotspot_jtreg_tier1_common-linux-x64-debug-24
Dependency task failed:
mach5...d_jdk_linux-linux-x64-debug-linux-x64-build-1

tier1-debug-jdk_open_test_hotspot_jtreg_tier1_compiler_1-linux-x64-debug-27
Dependency task failed:
mach5...d_jdk_linux-linux-x64-debug-linux-x64-build-1

tier1-debug-jdk_open_test_hotspot_jtreg_tier1_compiler_2-linux-x64-debug-30
Dependency task failed:
mach5...d_jdk_linux-linux-x64-debug-linux-x64-build-1

tier1-debug-jdk_open_test_hotspot_jtreg_tier1_compiler_3-linux-x64-debug-33
Dependency task failed:
mach5...d_jdk_linux-linux-x64-debug-linux-x64-build-1

tier1-debug-jdk_open_test_hotspot_jtreg_tier1_compiler_not_xcomp-linux-x64-debug-36
Dependency task failed:
mach5...d_jdk_linux-linux-x64-debug-linux-x64-build-1

tier1-debug-jdk_open_test_hotspot_jtreg_tier1_gc_1-linux-x64-debug-39
Dependency task failed:
mach5...d_jdk_linux-linux-x64-debug-linux-x64-build-1

tier1-debug-jdk_open_test_hotspot_jtreg_tier1_gc_2-linux-x64-debug-42
Dependency task failed:
mach5...d_jdk_linux-linux-x64-debug-linux-x64-build-1

tier1-debug-jdk_open_test_hotspot_jtreg_tier1_gc_gcbasher-linux-x64-debug-45
Dependency task failed:
mach5...d_jdk_linux-linux-x64-debug-linux-x64-build-1

tier1-debug-jdk_open_test_hotspot_jtreg_tier1_gc_gcold-linux-x64-debug-48
Dependency task failed:
mach5...d_jdk_linux-linux-x64-debug-linux-x64-build-1

tier1-debug-jdk_open_test_hotspot_jtreg_tier1_runtime-linux-x64-debug-51
Dependency task failed:
mach5...d_jdk_linux-linux-x64-debug-linux-x64-build-1
            See all 11...


> On 06/12/2018 10:23 AM, Roman Kennke wrote:
>> Am 12.06.2018 um 11:11 schrieb Andrew Haley:
>>> On 06/11/2018 08:17 PM, Roman Kennke wrote:
>>>> Am 11.06.2018 um 19:11 schrieb Andrew Haley:
>>>>> On 06/11/2018 04:56 PM, Andrew Haley wrote:
>>>>>> On 06/08/2018 09:17 PM, Roman Kennke wrote:
>>>>>>> Why is it better? And how would I do that? It sounds like a fairly
>>>>>>> complex undertaking for a special case. Notice that if the oop doesn't
>>>>>>> qualify as immediate operand (quite likely for an oop?) it used to be
>>>>>>> moved into rscratch1 anyway a few lines below.
>>>>>>
>>>>>> Sorry for the slow reply.  I'm looking now.
>>>>>
>>>>> OK.  The problem is that this is a very bad code smell:
>>>>>
>>>>>       case T_ARRAY:
>>>>>         jobject2reg(opr2->as_constant_ptr()->as_jobject(), rscratch1);
>>>>>         __ cmpoop(reg1, rscratch1);
>>>>>
>>>>> I can't tell that this is correct.  rscratch1 is used by assembler
>>>>> macros, and I don't know if some other GC (e.g. ZGC) might need to use
>>>>> rscratch1 inside cmpoop.  The risk here is obvious.  The Right Thing
>>>>> to do IMO is to generate a scratch register for pointer comparisons.
>>>>>
>>>>> Unless, I guess, we know that cmpoop never ever needs a scratch
>>>>> register for any forseeable garbage collector.
>>>>>
>>>>
>>>> I do know that Shenandoah does not require a tmp reg. I also do know
>>>> that no other collector currently needs equals-barriers at all.
>>>
>>> So cmpoop() is literally useless.  It does nothing except add a layer
>>> of obfuscation in the name of some possible future collector.
>>
>> The layer of abstraction is needed by Shenandoah. We need special
>> handling for comparing oops. It is certainly not useless. Or are we
>> talking about different issues?
> 
> Ah, okay.  I'm looking at ShenandoahBarrierSetAssembler::obj_equals()
> and I see that it actually has a side effect on its operands rather
> than using scratch registers.  Ewww.  I get it now.
> 
> OK, I withdraw my objection.  It's very confusing code to read, but
> it is what it is.
>