diff mbox

[AArch64] Add Cortex-A53 erratum 843419 configure-time option

Message ID CAD57uCdGEc4PH=V4DQpTVuCvTSJx35pvS_yAE_vQGmUYRDrZBA@mail.gmail.com
State New
Headers show

Commit Message

Yvan Roux May 1, 2015, 9:11 a.m. UTC
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.

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.

Comments

Marcus Shawcroft May 1, 2015, 11:11 a.m. UTC | #1
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.
Yvan Roux May 1, 2015, 11:39 a.m. UTC | #2
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 mbox

Patch

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