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 <[email protected]>
# 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'
