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

[Erik Joelsson]

Looks good to me.

/Erik

On 2018-11-28 13:17, 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
>
> 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.
>
>
> 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'
>