[AARCH64] Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341

Message ID CAELXzTMS9eHrcQ5SxXq4CuipBX4hgqrcYXapSxnAwanXL0AWqA@mail.gmail.com
State New
Headers show

Commit Message

Kugan Vivekanandarajah June 27, 2017, 1:20 a.m.
https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00614.html  added this
workaround to get kernel building with when TARGET_FIX_ERR_A53_843419
is enabled.

This was added to support building kernel loadable modules. In kernel,
when CONFIG_ARM64_ERRATUM_843419 is selected, the relocation needed
for ADRP/LDR (R_AARCH64_ADR_PREL_PG_HI21 and
R_AARCH64_ADR_PREL_PG_HI21_NC are removed from the kernel to avoid
loading objects with possibly offending sequence). Thus, it could only
support pc relative literal loads.

However, the following patch was posted to kernel to add
-mpc-relative-literal-loads
http://www.spinics.net/lists/arm-kernel/msg476149.html

-mpc-relative-literal-loads is unconditionally added to the kernel
build as can be seen from:
https://github.com/torvalds/linux/blob/master/arch/arm64/Makefile

Therefore this patch removes the hunk so that applications like
SPECcpu2017's 521/621.wrf can be built (with LTO in this case) without
-mno-pc-relative-literal-loads

Bootstrapped and regression tested on aarch64-linux-gnu with no new regressions.

Is this OK for trunk?

Thanks,
Kugan

gcc/testsuite/ChangeLog:

2017-06-27  Kugan Vivekanandarajah  <kuganv@linaro.org>

    * gcc.target/aarch64/pr63304_1.c: Remove-mno-fix-cortex-a53-843419.

gcc/ChangeLog:

2017-06-27  Kugan Vivekanandarajah  <kuganv@linaro.org>

    * config/aarch64/aarch64.c (aarch64_override_options_after_change_1):
    Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341
    for default.

Comments

Ramana Radhakrishnan June 27, 2017, 8:01 a.m. | #1
On 27/06/17 02:20, Kugan Vivekanandarajah wrote:
> https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00614.html  added this

> workaround to get kernel building with when TARGET_FIX_ERR_A53_843419

> is enabled.

> 

> This was added to support building kernel loadable modules. In kernel,

> when CONFIG_ARM64_ERRATUM_843419 is selected, the relocation needed

> for ADRP/LDR (R_AARCH64_ADR_PREL_PG_HI21 and

> R_AARCH64_ADR_PREL_PG_HI21_NC are removed from the kernel to avoid

> loading objects with possibly offending sequence). Thus, it could only

> support pc relative literal loads.

> 

> However, the following patch was posted to kernel to add

> -mpc-relative-literal-loads

> http://www.spinics.net/lists/arm-kernel/msg476149.html

> 

> -mpc-relative-literal-loads is unconditionally added to the kernel

> build as can be seen from:

> https://github.com/torvalds/linux/blob/master/arch/arm64/Makefile

> 

> Therefore this patch removes the hunk so that applications like

> SPECcpu2017's 521/621.wrf can be built (with LTO in this case) without

> -mno-pc-relative-literal-loads


Is that because your compiler has defaulted to 
-mpc-relative-literal-loads because it has the workaround enabled by 
default ? I'm curious as to why others haven't seen this issue.

regards
Ramana
Kugan Vivekanandarajah June 28, 2017, 1:02 a.m. | #2
Hi Ramana,

On 27 June 2017 at 18:01, Ramana Radhakrishnan
<ramana.radhakrishnan@foss.arm.com> wrote:
> On 27/06/17 02:20, Kugan Vivekanandarajah wrote:

>>

>> https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00614.html  added this

>> workaround to get kernel building with when TARGET_FIX_ERR_A53_843419

>> is enabled.

>>

>> This was added to support building kernel loadable modules. In kernel,

>> when CONFIG_ARM64_ERRATUM_843419 is selected, the relocation needed

>> for ADRP/LDR (R_AARCH64_ADR_PREL_PG_HI21 and

>> R_AARCH64_ADR_PREL_PG_HI21_NC are removed from the kernel to avoid

>> loading objects with possibly offending sequence). Thus, it could only

>> support pc relative literal loads.

>>

>> However, the following patch was posted to kernel to add

>> -mpc-relative-literal-loads

>> http://www.spinics.net/lists/arm-kernel/msg476149.html

>>

>> -mpc-relative-literal-loads is unconditionally added to the kernel

>> build as can be seen from:

>> https://github.com/torvalds/linux/blob/master/arch/arm64/Makefile

>>

>> Therefore this patch removes the hunk so that applications like

>> SPECcpu2017's 521/621.wrf can be built (with LTO in this case) without

>> -mno-pc-relative-literal-loads

>

>

> Is that because your compiler has defaulted to -mpc-relative-literal-loads

> because it has the workaround enabled by default ? I'm curious as to why

> others haven't seen this issue.

>


If TARGET_FIX_ERR_A53_843419 is selected, compiler defaults to
-mpc-relative-literal-loads unless we explicitly specify
-mno-pc-relative-literal-loads. Linaro toolchain is built with
TARGET_FIX_ERR_A53_843419.

This linking of TARGET_FIX_ERR_A53_843419 and
-mpc-relative-literal-loads  should now be relaxed since the kernel
explicitly uses -mpc-relative-literal-loads.

This 1MiB issue should be very rarely seen even before you fixed it.

Thanks,
Kugan


> regards

> Ramana
Ramana Radhakrishnan June 28, 2017, 10:06 p.m. | #3
On Wed, Jun 28, 2017 at 2:02 AM, Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
> Hi Ramana,

>

> On 27 June 2017 at 18:01, Ramana Radhakrishnan

> <ramana.radhakrishnan@foss.arm.com> wrote:

>> On 27/06/17 02:20, Kugan Vivekanandarajah wrote:

>>>

>>> https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00614.html  added this

>>> workaround to get kernel building with when TARGET_FIX_ERR_A53_843419

>>> is enabled.

>>>

>>> This was added to support building kernel loadable modules. In kernel,

>>> when CONFIG_ARM64_ERRATUM_843419 is selected, the relocation needed

>>> for ADRP/LDR (R_AARCH64_ADR_PREL_PG_HI21 and

>>> R_AARCH64_ADR_PREL_PG_HI21_NC are removed from the kernel to avoid

>>> loading objects with possibly offending sequence). Thus, it could only

>>> support pc relative literal loads.

>>>

>>> However, the following patch was posted to kernel to add

>>> -mpc-relative-literal-loads

>>> http://www.spinics.net/lists/arm-kernel/msg476149.html

>>>

>>> -mpc-relative-literal-loads is unconditionally added to the kernel

>>> build as can be seen from:

>>> https://github.com/torvalds/linux/blob/master/arch/arm64/Makefile

>>>

>>> Therefore this patch removes the hunk so that applications like

>>> SPECcpu2017's 521/621.wrf can be built (with LTO in this case) without

>>> -mno-pc-relative-literal-loads

>>

>>

>> Is that because your compiler has defaulted to -mpc-relative-literal-loads

>> because it has the workaround enabled by default ? I'm curious as to why

>> others haven't seen this issue.

>>

>

> If TARGET_FIX_ERR_A53_843419 is selected, compiler defaults to

> -mpc-relative-literal-loads unless we explicitly specify

> -mno-pc-relative-literal-loads. Linaro toolchain is built with

> TARGET_FIX_ERR_A53_843419.


That explains why we haven't been hit by this issue in our builds of
SPEC2017 even though I don't think we've done any lto builds
recently.,
>

> This linking of TARGET_FIX_ERR_A53_843419 and

> -mpc-relative-literal-loads  should now be relaxed since the kernel

> explicitly uses -mpc-relative-literal-loads.

>

> This 1MiB issue should be very rarely seen even before you fixed it.

>


This particular issue maybe, but the original patch was put in because
we had a number of users complaining about functions > 1MiB especially
with autogenerated code.

regards
Ramana

> Thanks,

> Kugan

>

>

>> regards

>> Ramana
Kugan Vivekanandarajah July 21, 2017, 10:12 a.m. | #4
Ping ?

Thanks,
Kugan

On 27 June 2017 at 11:20, Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
> https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00614.html  added this

> workaround to get kernel building with when TARGET_FIX_ERR_A53_843419

> is enabled.

>

> This was added to support building kernel loadable modules. In kernel,

> when CONFIG_ARM64_ERRATUM_843419 is selected, the relocation needed

> for ADRP/LDR (R_AARCH64_ADR_PREL_PG_HI21 and

> R_AARCH64_ADR_PREL_PG_HI21_NC are removed from the kernel to avoid

> loading objects with possibly offending sequence). Thus, it could only

> support pc relative literal loads.

>

> However, the following patch was posted to kernel to add

> -mpc-relative-literal-loads

> http://www.spinics.net/lists/arm-kernel/msg476149.html

>

> -mpc-relative-literal-loads is unconditionally added to the kernel

> build as can be seen from:

> https://github.com/torvalds/linux/blob/master/arch/arm64/Makefile

>

> Therefore this patch removes the hunk so that applications like

> SPECcpu2017's 521/621.wrf can be built (with LTO in this case) without

> -mno-pc-relative-literal-loads

>

> Bootstrapped and regression tested on aarch64-linux-gnu with no new regressions.

>

> Is this OK for trunk?

>

> Thanks,

> Kugan

>

> gcc/testsuite/ChangeLog:

>

> 2017-06-27  Kugan Vivekanandarajah  <kuganv@linaro.org>

>

>     * gcc.target/aarch64/pr63304_1.c: Remove-mno-fix-cortex-a53-843419.

>

> gcc/ChangeLog:

>

> 2017-06-27  Kugan Vivekanandarajah  <kuganv@linaro.org>

>

>     * config/aarch64/aarch64.c (aarch64_override_options_after_change_1):

>     Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341

>     for default.

Patch hide | download patch | download mbox

From bf5d8151ad6a83903f51529655e83181bdb67200 Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Thu, 8 Jun 2017 15:51:29 +1000
Subject: [PATCH] Disable pc relative literal load irrespective of
 TARGET_FIX_ERR_A53_84341

---
 gcc/config/aarch64/aarch64.c                 | 11 -----------
 gcc/testsuite/gcc.target/aarch64/pr63304_1.c |  2 +-
 2 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 71f9819..99cfd20 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -8632,17 +8632,6 @@  aarch64_override_options_after_change_1 (struct gcc_options *opts)
   if (opts->x_pcrelative_literal_loads == 1)
     aarch64_pcrelative_literal_loads = true;
 
-  /* This is PR70113. When building the Linux kernel with
-     CONFIG_ARM64_ERRATUM_843419, support for relocations
-     R_AARCH64_ADR_PREL_PG_HI21 and R_AARCH64_ADR_PREL_PG_HI21_NC is
-     removed from the kernel to avoid loading objects with possibly
-     offending sequences.  Without -mpc-relative-literal-loads we would
-     generate such relocations, preventing the kernel build from
-     succeeding.  */
-  if (opts->x_pcrelative_literal_loads == 2
-      && TARGET_FIX_ERR_A53_843419)
-    aarch64_pcrelative_literal_loads = true;
-
   /* In the tiny memory model it makes no sense to disallow PC relative
      literal pool loads.  */
   if (aarch64_cmodel == AARCH64_CMODEL_TINY
diff --git a/gcc/testsuite/gcc.target/aarch64/pr63304_1.c b/gcc/testsuite/gcc.target/aarch64/pr63304_1.c
index c917f81c..fa0fb56 100644
--- a/gcc/testsuite/gcc.target/aarch64/pr63304_1.c
+++ b/gcc/testsuite/gcc.target/aarch64/pr63304_1.c
@@ -1,5 +1,5 @@ 
 /* { dg-do assemble } */
-/* { dg-options "-O1 --save-temps -mno-fix-cortex-a53-843419" } */
+/* { dg-options "-O1 --save-temps" } */
 #pragma GCC push_options
 #pragma GCC target ("+nothing+simd, cmodel=small")
 
-- 
2.7.4