Message ID | CAD57uCdGEc4PH=V4DQpTVuCvTSJx35pvS_yAE_vQGmUYRDrZBA@mail.gmail.com |
---|---|
State | New |
Headers | show |
On 1 May 2015 at 10:11, Yvan Roux <yvan.roux@linaro.org> wrote: > Hi all, > > As described in the thread bellow, there is a link-time workaround for > an erratum (843419) of some early revision of Cortex-A53. Similarly > to what was done for a previous erratum, this patch adds a new > configure-time option --enable-fix-cortex-a53-843419 that pass down > the linker option --fix-cortex-a53-843419. > > I haven't implemented flags to explicitly disable/enable it during > compilation as it is only a linker workaround, but I can do it if you > think it's necessary. Hi Yvan, In the case of the 835769 erratum, providing such GCC configury made (some) sense because the workaround included both a GCC and an LD component. GCC in effect needed the option to control GCC behaviour aswell as LD behaviour. The same is not true of 843419. The net effect of the proposed patch here is to provide a configure option in GCC to change the default behaviour of LD, with no other purpose within GCC itself. This seems rather bizarre to me. I would have thought that if the sole objective is to change the default behaviour of LD, then there should be configure time options on LD, rather than GCC. That said, I can also see that providing consistent behaviour across these various work around configure options and consistent run time options for the user will reduce confusion in the future, both for folks building toolchains and for the folks using them. From this perspective I think it would be better to go with this patch *and* include the flags explicitly such the both of the current workaround have equivalent configury behaviour and have equivalent user flags at run time. Cheers /Marcus > > https://sourceware.org/ml/binutils-cvs/2015-04/msg00012.html > > Is it ok for trunk and/or branches ? > > Thanks, > Yvan > > 2015-05-01 Yvan Roux <yvan.roux@linaro.org> > > * configure.ac: Add --enable-fix-cortex-a53-843419 option. > * configure: Regenerate. > * config/aarch64/aarch64-elf-raw.h (CA53_ERR_843419_SPEC): Define. > (LINK_SPEC): Include CA53_ERR_843419_SPEC. > * config/aarch64/aarch64-linux.h (CA53_ERR_843419_SPEC): Define. > (LINK_SPEC): Include CA53_ERR_843419_SPEC. > * doc/install.texi (aarch64*-*-*): Document > new --enable-fix-cortex-a53-843419 option.
Hi Marcus, (Sorry wanted to cc you in my first mail but seems that gmail prefers Maxim to Marcus ! ;) On 1 May 2015 at 13:11, Marcus Shawcroft <marcus.shawcroft@gmail.com> wrote: > On 1 May 2015 at 10:11, Yvan Roux <yvan.roux@linaro.org> wrote: >> Hi all, >> >> As described in the thread bellow, there is a link-time workaround for >> an erratum (843419) of some early revision of Cortex-A53. Similarly >> to what was done for a previous erratum, this patch adds a new >> configure-time option --enable-fix-cortex-a53-843419 that pass down >> the linker option --fix-cortex-a53-843419. >> >> I haven't implemented flags to explicitly disable/enable it during >> compilation as it is only a linker workaround, but I can do it if you >> think it's necessary. > > Hi Yvan, > > In the case of the 835769 erratum, providing such GCC configury made > (some) sense because the workaround included both a GCC and an LD > component. GCC in effect needed the option to control GCC behaviour > aswell as LD behaviour. The same is not true of 843419. The net effect > of the proposed patch here is to provide a configure option in GCC to > change the default behaviour of LD, with no other purpose within GCC > itself. This seems rather bizarre to me. I would have thought that if > the sole objective is to change the default behaviour of LD, then > there should be configure time options on LD, rather than GCC. Yes indeed. > That said, I can also see that providing consistent behaviour across > these various work around configure options and consistent run time > options for the user will reduce confusion in the future, both for > folks building toolchains and for the folks using them. From this > perspective I think it would be better to go with this patch *and* > include the flags explicitly such the both of the current workaround > have equivalent configury behaviour and have equivalent user flags at > run time. Ok I'll add the -mfix-cortex-a53-843419 flags to be consistent. Cheers Yvan > Cheers > /Marcus > >> >> https://sourceware.org/ml/binutils-cvs/2015-04/msg00012.html >> >> Is it ok for trunk and/or branches ? >> >> Thanks, >> Yvan >> >> 2015-05-01 Yvan Roux <yvan.roux@linaro.org> >> >> * configure.ac: Add --enable-fix-cortex-a53-843419 option. >> * configure: Regenerate. >> * config/aarch64/aarch64-elf-raw.h (CA53_ERR_843419_SPEC): Define. >> (LINK_SPEC): Include CA53_ERR_843419_SPEC. >> * config/aarch64/aarch64-linux.h (CA53_ERR_843419_SPEC): Define. >> (LINK_SPEC): Include CA53_ERR_843419_SPEC. >> * doc/install.texi (aarch64*-*-*): Document >> new --enable-fix-cortex-a53-843419 option.
diff --git a/gcc/config/aarch64/aarch64-elf-raw.h b/gcc/config/aarch64/aarch64-elf-raw.h index ebeeb50..09dcfaf 100644 --- a/gcc/config/aarch64/aarch64-elf-raw.h +++ b/gcc/config/aarch64/aarch64-elf-raw.h @@ -35,10 +35,17 @@ " %{mfix-cortex-a53-835769:--fix-cortex-a53-835769}" #endif +#ifdef TARGET_FIX_ERR_A53_843419_DEFAULT +#define CA53_ERR_843419_SPEC " --fix-cortex-a53-843419 " +#else +#define CA53_ERR_843419_SPEC "" +#endif + #ifndef LINK_SPEC #define LINK_SPEC "%{mbig-endian:-EB} %{mlittle-endian:-EL} -X \ -maarch64elf%{mabi=ilp32*:32}%{mbig-endian:b}" \ - CA53_ERR_835769_SPEC + CA53_ERR_835769_SPEC \ + CA53_ERR_843419_SPEC #endif #endif /* GCC_AARCH64_ELF_RAW_H */ diff --git a/gcc/config/aarch64/aarch64-linux.h b/gcc/config/aarch64/aarch64-linux.h index 9abb252..2219ead 100644 --- a/gcc/config/aarch64/aarch64-linux.h +++ b/gcc/config/aarch64/aarch64-linux.h @@ -49,8 +49,15 @@ " %{mfix-cortex-a53-835769:--fix-cortex-a53-835769}" #endif +#ifdef TARGET_FIX_ERR_A53_843419_DEFAULT +#define CA53_ERR_843419_SPEC " --fix-cortex-a53-843419 " +#else +#define CA53_ERR_843419_SPEC "" +#endif + #define LINK_SPEC LINUX_TARGET_LINK_SPEC \ - CA53_ERR_835769_SPEC + CA53_ERR_835769_SPEC \ + CA53_ERR_843419_SPEC #define GNU_USER_TARGET_MATHFILE_SPEC \ "%{Ofast|ffast-math|funsafe-math-optimizations:crtfastmath.o%s}" diff --git a/gcc/configure b/gcc/configure index 84f58ce..e563e94 100755 --- a/gcc/configure +++ b/gcc/configure @@ -923,6 +923,7 @@ enable_gnu_indirect_function enable_initfini_array enable_comdat enable_fix_cortex_a53_835769 +enable_fix_cortex_a53_843419 with_glibc_version enable_gnu_unique_object enable_linker_build_id @@ -1648,6 +1649,14 @@ Optional Features: disable workaround for AArch64 Cortex-A53 erratum 835769 by default + + --enable-fix-cortex-a53-843419 + enable workaround for AArch64 Cortex-A53 erratum + 843419 by default + --disable-fix-cortex-a53-843419 + disable workaround for AArch64 Cortex-A53 erratum + 843419 by default + --enable-gnu-unique-object enable the use of the @gnu_unique_object ELF extension on glibc systems @@ -18153,7 +18162,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 18156 "configure" +#line 18165 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -18259,7 +18268,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 18262 "configure" +#line 18271 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -24102,6 +24111,25 @@ if test "${enable_fix_cortex_a53_835769+set}" = set; then : fi + # Enable default workaround for AArch64 Cortex-A53 erratum 843419. + # Check whether --enable-fix-cortex-a53-843419 was given. +if test "${enable_fix_cortex_a53_843419+set}" = set; then : + enableval=$enable_fix_cortex_a53_843419; + case $enableval in + yes) + tm_defines="${tm_defines} TARGET_FIX_ERR_A53_843419_DEFAULT=1" + ;; + no) + ;; + *) + as_fn_error "'$enableval' is an invalid value for --enable-fix-cortex-a53-843419.\ + Valid choices are 'yes' and 'no'." "$LINENO" 5 + ;; + + esac + +fi + ;; # All TARGET_ABI_OSF targets. diff --git a/gcc/configure.ac b/gcc/configure.ac index 7fb6131..55fe633 100644 --- a/gcc/configure.ac +++ b/gcc/configure.ac @@ -3592,6 +3592,29 @@ AS_HELP_STRING([--disable-fix-cortex-a53-835769], esac ], []) + # Enable default workaround for AArch64 Cortex-A53 erratum 843419. + AC_ARG_ENABLE(fix-cortex-a53-843419, + [ +AS_HELP_STRING([--enable-fix-cortex-a53-843419], + [enable workaround for AArch64 Cortex-A53 erratum 843419 by default]) +AS_HELP_STRING([--disable-fix-cortex-a53-843419], + [disable workaround for AArch64 Cortex-A53 erratum 843419 by default]) + ], + [ + case $enableval in + yes) + tm_defines="${tm_defines} TARGET_FIX_ERR_A53_843419_DEFAULT=1" + ;; + no) + ;; + *) + AC_MSG_ERROR(['$enableval' is an invalid value for --enable-fix-cortex-a53-843419.\ + Valid choices are 'yes' and 'no'.]) + ;; + + esac + ], + []) ;; # All TARGET_ABI_OSF targets. diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi index 783116e..7810696 100644 --- a/gcc/doc/install.texi +++ b/gcc/doc/install.texi @@ -3415,6 +3415,16 @@ default. The workaround is disabled by default if neither of @option{--enable-fix-cortex-a53-835769} or @option{--disable-fix-cortex-a53-835769} is given at configure time. +To enable a workaround for the Cortex-A53 erratum number 843419 by default +(for all CPUs regardless of -mcpu option given) at configure time use the +@option{--enable-fix-cortex-a53-843419} option. This erratum workaround is +made at link time and enabling it by default in GCC will only pass the +corresponding flag to the linker. Conversely, +@option{--disable-fix-cortex-a53-843419} will disable the workaround by +default. The workaround is disabled by default if neither of +@option{--enable-fix-cortex-a53-843419} or +@option{--disable-fix-cortex-a53-843419} is given at configure time. + @html <hr /> @end html