RFR (M): 8188764: Obsolete AssumeMP and then remove all support for non-MP builds

[David Holmes]

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