[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-build-dev
Subject:    Re: [PATCH v2] Upgrade arm-sflt minimum architecture to ARMv5TE
From:       David Holmes <david.holmes () oracle ! com>
Date:       2018-11-29 3:57:36
Message-ID: c093c9bd-f950-8342-c356-b2c0dc2b8158 () oracle ! com
[Download RAW message or body]

Hi Jakub,

I've filed:

https://bugs.openjdk.java.net/browse/JDK-8214465

for this issue.

I will sponsor.

David

On 29/11/2018 8:12 am, David Holmes wrote:
> Hi Jakub,
> 
> On 29/11/2018 7:17 am, Jakub Vaněk wrote:
>> Hi David,
>>
>> On 2018-11-26 at 17:36 +1000, David Holmes wrote:
>>> Hi Jakub,
>>>
>>> On 26/11/2018 1:53 am, Jakub Vaněk wrote:
>>>> Hi,
>>>>
>>>> This patch raises the minimum architectural level for ARM CPUs to
>>>> ARMv5TE. It is done through changing the -march flag in the current
>>>> CPU-specific CFLAGS and ASFLAGS.
>>>>
>>>> This patch depends on "Append assembler flags on ARM targets" patch
>>>> for
>>>> ASFLAGS handling.
>>>>
>>>> Reason for this change: assembler code in linux_arm_32.s uses PLD
>>>> instructions. ARM ISA manual mentions this: "This instruction is
>>>> available in E variants of ARM architecture v5 and above." (thanks
>>>> to
>>>> David Holmes for noticing this).
>>>
>>> FYI I did a bit of digging and we tried to make this change back in
>>> 2010
>>> and ran into problems:
>>>
>>> "It appears that gcc assumes that if you choose the armv5te
>>> architecture
>>> option, it can use the "E" instruction extensions which provide 64
>>> bit
>>> load and stores to 4 byte aligned addresses. I tried two different
>>> compiler versions 4.1.2 and 4.2.3 and they both had the same
>>> problem.
>>> The <hardware> does support ldrd/strd but they do not support 4 byte
>>> aligned operations. The addresses must be 8 byte aligned."
>>
>>> How things stand today I can not say.
>>
>> I have searched for this and indeed, this bug wasn't fixed until
>> recently: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82445
> 
> That's not quite the issue we encountered as we had buggy hardware. 
> Interesting that they changed the compiler to require 8 byte alignment.
> 
>> For our own use, it seems that Ubuntu gcc7 is patched, whether Debian
>> gcc6 isn't. I think that the best solution is to keep armv5t in CFLAGS
>> and only raise armv5te in ASFLAGS, where it is actually needed. This
>> way GCC still generates older non-buggy code and the assembler works
>> with pld. I have tested this configuration and it compiles
>> successfully.
> 
> That sounds quite reasonable and minimizes the chance of incidental 
> changes through gcc code generation.
> 
> This counts as one review. Hopefully Erik will be able to scan this one 
> too. Again either he can sponsor or I can if he doesn't. :)
> 
> Cheers,
> David
> 
>>
>> https://ci.adoptopenjdk.net/view/ev3dev/job/openjdk12_build_ev3_linux/40/
>>
>> Thanks,
>>
>> Jakub
>>
>>>
>>> Cheers,
>>> David
>>
>> # HG changeset patch
>> # User Jakub Vaněk <linuxtardis@gmail.com>
>> # Date 1543252130 -3600
>> #           Mon Nov 26 18:08:50 2018 +0100
>> # Node ID bfd9e7032cfdc8c814ddaf4bb471a014fbbf1a89
>> # Parent   d2db3b32d5614029f2b1af2731c0d9ec10d88767
>> Fix undefined pld instruction on arm-sflt
>>
>> diff --git a/make/autoconf/flags.m4 b/make/autoconf/flags.m4
>> --- a/make/autoconf/flags.m4
>> +++ b/make/autoconf/flags.m4
>> @@ -46,6 +46,11 @@
>>            AC_MSG_CHECKING([for ABI profle])
>>            AC_MSG_RESULT([$OPENJDK_TARGET_ABI_PROFILE])
>> +       # --- Arm-sflt CFLAGS and ASFLAGS ---
>> +       # Armv5te is required for assembler, because pld insn used in 
>> arm32 hotspot is only in v5E and above.
>> +       # However, there is also a GCC bug which generates unaligned 
>> strd/ldrd instructions on armv5te:
>> +       # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82445, and it was 
>> fixed only quite recently.
>> +       # The resulting compromise is to enable v5TE for assembler and 
>> let GCC generate code for v5T.
>>            if test "x$OPENJDK_TARGET_ABI_PROFILE" = xarm-vfp-sflt; then
>>                ARM_FLOAT_TYPE=vfp-sflt
>>                ARM_ARCH_TYPE_FLAGS='-march=armv7-a -mthumb'
>> @@ -57,11 +62,11 @@
>>            elif test "x$OPENJDK_TARGET_ABI_PROFILE" = xarm-sflt; then
>>                ARM_FLOAT_TYPE=sflt
>>                ARM_ARCH_TYPE_FLAGS='-march=armv5t -marm'
>> -           ARM_ARCH_TYPE_ASFLAGS='-march=armv5t'
>> +           ARM_ARCH_TYPE_ASFLAGS='-march=armv5te'
>>            elif test "x$OPENJDK_TARGET_ABI_PROFILE" = xarmv5-vfp-sflt; then
>>                ARM_FLOAT_TYPE=vfp-sflt
>>                ARM_ARCH_TYPE_FLAGS='-march=armv5t -marm'
>> -           ARM_ARCH_TYPE_ASFLAGS='-march=armv5t'
>> +           ARM_ARCH_TYPE_ASFLAGS='-march=armv5te'
>>            elif test "x$OPENJDK_TARGET_ABI_PROFILE" = xarmv6-vfp-hflt; then
>>                ARM_FLOAT_TYPE=vfp-hflt
>>                ARM_ARCH_TYPE_FLAGS='-march=armv6 -marm'
>>
[prev in list] [next in list] [prev in thread] [next in thread]