RFR (M): 8188764: Obsolete AssumeMP and then remove all support for non-MP builds
Boris Ulasevich
boris.ulasevich at bell-sw.com
Wed Sep 26 13:47:43 UTC 2018
Hi David,
For this change the most interesting would be to check it with ARMv5,
but I do not have the hardware now. I hope I will get one ARMv5 machine
next week.
For ARMv7 the change is Ok. Almost Ok. There is a minor build issue:
/home/boris/jdk-jdk/src/hotspot/cpu/arm/templateTable_arm.cpp:3724:9:
error: redeclaration of ‘Label notVolatile’
Label notVolatile;
^~~~~~~~~~~
/home/boris/jdk-jdk/src/hotspot/cpu/arm/templateTable_arm.cpp:3492:9:
note: ‘Label notVolatile’ previously declared here
Label notVolatile;
^~~~~~~~~~~
/home/boris/jdk-jdk/src/hotspot/cpu/arm/templateTable_arm.cpp: In static
member function ‘static void TemplateTable::fast_storefield(TosState)’:
/home/boris/jdk-jdk/src/hotspot/cpu/arm/templateTable_arm.cpp:3885:9:
error: redeclaration of ‘Label notVolatile’
Label notVolatile;
^~~~~~~~~~~
/home/boris/jdk-jdk/src/hotspot/cpu/arm/templateTable_arm.cpp:3832:9:
note: ‘Label notVolatile’ previously declared here
Label notVolatile;
^~~~~~~~~~~
regards,
Boris
On 23.09.2018 19:26, David Holmes wrote:
> Hi Boris,
>
> Here is latest webrev with the ARM code updated as well.
>
> http://cr.openjdk.java.net/~dholmes/8188764/webrev/
>
> All is_MP removed except where needed for ARMv5 support, MP specific
> instructions, and controlling of biased locking.
>
> I checked the DMB variants and we only generate for ARMv7 anyway, so no
> issue there.
>
> Would very much appreciate whatever test builds you can do on different
> ARM variants Boris!
>
> Thanks,
> David
>
> On 21/09/2018 9:41 AM, David Holmes wrote:
>> Hi Boris,
>>
>> On 21/09/2018 9:34 AM, Boris Ulasevich wrote:
>>> Hi David,
>>>
>>> On 20.09.2018 18:26, David Holmes wrote:
>>>> Hi Boris,
>>>>
>>>> Thanks for jumping on this. I hope you didn't spend too much time on
>>>> it as I had actually started on the ARM code then decided I didn't
>>>> need to make the changes, so I'd already gone through pretty much
>>>> everything that is needed. My concern is with things like the change in
>>>>
>>>> src/hotspot/cpu/arm/assembler_arm_32.hpp
>>>>
>>>> where you removed the is_MP() check but in fact that needs to remain
>>>> because the instruction pldw is not present on variants of the v7
>>>> architecture without the multi-processor extensions:
>>>
>>> Ok. Good point. Sure, my hasty proposal to get rid of is_MP() check
>>> in arm32 codes was wrong.
>>>
>>> One note. I see there is an ambiguity here: is_MP() is a
>>> multiprocessor system flag (returns true when _processor_count != 1),
>>> but multi-processor extensions is an optional ARMv7 feature which is
>>> not related with processors count, so pldw usage should not be
>>> controlled by is_MP() flag.
>>
>> is_MP can't return true unless there is more than one processor
>> (ignoring the pre-initialization case), so if there's more than one
>> processor there must be a MP supprting architecture.
>>
>>>> http://www.keil.com/support/man/docs/armclang_asm/armclang_asm_pge1425899018492.htm
>>>>
>>>> That makes me wonder whether any of the memory barrier instructions
>>>> may also be missing in some v6/v7 variants as well?
>>>
>>> Nobody complained :) And Reference Manual says DMB/DSB are not optional.
>>
>> I need to double-check we don't define variants like "dmb sy" that are
>> architecture specific.
>>
>>>> Plus I have to account for uniprocessor ARMv5 so need to see whether
>>>> MacroAssembler::membar, for example, must also retain the is_MP()
>>>> check.
>>>
>>> Do you want to delete this check? Processor count check seems quite
>>> natural to see if it has sense to generate memory barriers.
>>
>> That is the general thrust of this change. We assume we always have MP
>> and so always issue memory barriers.
>>
>>> By the way, MacroAssembler::fast_lock have
>>> VM_Version::supports_ldrex() check assertion with message
>>> "unsupported, yet?". Looks like it is not going to work correctly on
>>> multiprocessor ARMv5.
>>
>> The ARM code does not support multiprocessor ARMv5.
>>
>> Thanks,
>> David
>>
>>>> Also in:
>>>>
>>>> src/hotspot/cpu/arm/stubGenerator_arm.cpp
>>>>
>>>> you can't remove the !is_MP() check and related code as that is
>>>> needed for ARMv5 (uniprocessor) support.
>>>>
>>>> Similarly in
>>>>
>>>> src/hotspot/cpu/arm/vm_version_arm_32.cpp
>>>>
>>>> you'll probably still want to disable biased-locking for ARMv5.
>>>>
>>>> Cheers,
>>>> David
>>>>
>>>> On 20/09/2018 3:31 AM, Boris Ulasevich wrote:
>>>>> Hi David,
>>>>>
>>>>> > I have not updated the about-to-be-removed Oracle ARM port code.
>>>>>
>>>>> Ok, but we can do the changes in ARM32 codes leaving ARM64 intact.
>>>>> I would propose the following change - I think we can either add it
>>>>> to your update or to create a separate RFR after your patch and
>>>>> JEP-340 commit:
>>>>>
>>>>> http://cr.openjdk.java.net/~bulasevich/8188764/webrev.00/
>>>>>
>>>>> regards,
>>>>> Boris
>>>>>
>>>>> On 18.09.2018 08:32, David Holmes wrote:
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8188764
>>>>>> Webrev: http://cr.openjdk.java.net/~dholmes/8188764/webrev/
>>>>>>
>>>>>> As previously discussed in the RFC thread:
>>>>>>
>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2018-September/034166.html
>>>>>>
>>>>>>
>>>>>> this change obsoletes the AssumeMP flag and removes the bulk of
>>>>>> the logic that is conditional on os::is_MP() as follows:
>>>>>>
>>>>>> 1. to gate the introduction of MP-specific features, notably
>>>>>> memory barriers
>>>>>>
>>>>>> The is_MP check is removed and we will always issue memory barriers
>>>>>> or pre-pend lock prefix etc.
>>>>>>
>>>>>> 2. to align certain patchable code sequences (method entry, call
>>>>>> sites)
>>>>>>
>>>>>> The is_MP is removed and we always align patchable locations.
>>>>>>
>>>>>> 3. to gate certain optimizations which are questionable on
>>>>>> uniprocessors
>>>>>> (see quicken_jni_functions)
>>>>>>
>>>>>> These are psuedo-memory-barriers where we manufacture a
>>>>>> data-dependency
>>>>>> instead of inserting mfence/lfence (x86 example). These are
>>>>>> treated as
>>>>>> #1 and is_MP is removed.
>>>>>>
>>>>>> 4. to gate optimizations which are desirable on uniprocessors
>>>>>> (mutex/monitor short circuits)
>>>>>>
>>>>>> These are spin controls and so is_MP remains.
>>>>>>
>>>>>> I have not updated the about-to-be-removed Oracle ARM port code.
>>>>>>
>>>>>> Testing:
>>>>>> - Tiers 1 -3 (mach5)
>>>>>>
>>>>>> Performance run TBD.
>>>>>>
>>>>>> Thanks,
>>>>>> David
More information about the hotspot-dev
mailing list