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

[Boris Ulasevich]

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.

> 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.

> 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.

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.

> 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