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

[David Holmes]

Hi Bob,

On 24/09/2018 1:34 PM, Bob Vandette wrote:
> David,
> 
> I assume that you are going to push this change independent of the ARM code removal JEP, right?

Right - unless you get there first. :) I'm unlikely to push this week.

David

> I just changed the JEP to “Propose to Target” and will merge any updates with my patch once
> it is “Targeted”.
> 
> 
> Bob.
> 
>> On Sep 23, 2018, at 12:26 PM, David Holmes <David.Holmes at oracle.com> 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
>