RFR: JDK-8203157: Object equals abstraction for BarrierSetAssembler

[Roman Kennke]

Ok, interesting.

I have that patch, but it builds+tests fine for me.
How has this slipped through mach5 (pre-commit) in the first place?

Anyway, I'll wait for the fix, and then retry.

Thanks, Roman


> Hi,
> 
>   the issue is (or actually the fix) https://bugs.openjdk.java.net/brow
> se/JDK-8204861 .
> 
> Thanks,
>   Thomas
> 
> On Tue, 2018-06-12 at 14:59 +0200, Roman Kennke wrote:
>> 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.
>>>
>>
>>
>