[PATCH v2] Upgrade arm-sflt minimum architecture to ARMv5TE

[David Holmes]

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 at 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'
>