* [AARCH64] Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341
@ 2017-06-27 1:20 Kugan Vivekanandarajah
2017-06-27 8:01 ` Ramana Radhakrishnan
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Kugan Vivekanandarajah @ 2017-06-27 1:20 UTC (permalink / raw)
To: gcc-patches, James Greenhalgh, Richard Earnshaw
[-- Attachment #1: Type: text/plain, Size: 1497 bytes --]
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.
[-- Attachment #2: 0001-Disable-pc-relative-literal-load-irrespective-of-TAR.patch --]
[-- Type: text/x-patch, Size: 1971 bytes --]
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [AARCH64] Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341
2017-06-27 1:20 [AARCH64] Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341 Kugan Vivekanandarajah
@ 2017-06-27 8:01 ` Ramana Radhakrishnan
2017-06-28 1:02 ` Kugan Vivekanandarajah
2017-07-21 10:12 ` Kugan Vivekanandarajah
2017-08-29 12:02 ` James Greenhalgh
2 siblings, 1 reply; 13+ messages in thread
From: Ramana Radhakrishnan @ 2017-06-27 8:01 UTC (permalink / raw)
To: Kugan Vivekanandarajah, gcc-patches, James Greenhalgh, Richard Earnshaw
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [AARCH64] Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341
2017-06-27 8:01 ` Ramana Radhakrishnan
@ 2017-06-28 1:02 ` Kugan Vivekanandarajah
2017-06-28 22:06 ` Ramana Radhakrishnan
0 siblings, 1 reply; 13+ messages in thread
From: Kugan Vivekanandarajah @ 2017-06-28 1:02 UTC (permalink / raw)
To: Ramana Radhakrishnan; +Cc: gcc-patches, James Greenhalgh, Richard Earnshaw
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [AARCH64] Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341
2017-06-28 1:02 ` Kugan Vivekanandarajah
@ 2017-06-28 22:06 ` Ramana Radhakrishnan
0 siblings, 0 replies; 13+ messages in thread
From: Ramana Radhakrishnan @ 2017-06-28 22:06 UTC (permalink / raw)
To: Kugan Vivekanandarajah
Cc: Ramana Radhakrishnan, gcc-patches, James Greenhalgh, Richard Earnshaw
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [AARCH64] Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341
2017-06-27 1:20 [AARCH64] Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341 Kugan Vivekanandarajah
2017-06-27 8:01 ` Ramana Radhakrishnan
@ 2017-07-21 10:12 ` Kugan Vivekanandarajah
2017-08-11 10:34 ` Kugan Vivekanandarajah
2017-08-29 12:02 ` James Greenhalgh
2 siblings, 1 reply; 13+ messages in thread
From: Kugan Vivekanandarajah @ 2017-07-21 10:12 UTC (permalink / raw)
To: gcc-patches, James Greenhalgh, Richard Earnshaw
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [AARCH64] Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341
2017-07-21 10:12 ` Kugan Vivekanandarajah
@ 2017-08-11 10:34 ` Kugan Vivekanandarajah
2017-08-29 8:32 ` Kugan Vivekanandarajah
0 siblings, 1 reply; 13+ messages in thread
From: Kugan Vivekanandarajah @ 2017-08-11 10:34 UTC (permalink / raw)
To: gcc-patches, James Greenhalgh, Richard Earnshaw
Ping^2?
Thanks,
Kugan
On 21 July 2017 at 20:12, Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
> 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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [AARCH64] Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341
2017-08-11 10:34 ` Kugan Vivekanandarajah
@ 2017-08-29 8:32 ` Kugan Vivekanandarajah
0 siblings, 0 replies; 13+ messages in thread
From: Kugan Vivekanandarajah @ 2017-08-29 8:32 UTC (permalink / raw)
To: gcc-patches, James Greenhalgh, Richard Earnshaw
ping^3
Thanks,
Kugan
On 11 August 2017 at 16:09, Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
> Ping^2?
>
> Thanks,
> Kugan
>
> On 21 July 2017 at 20:12, Kugan Vivekanandarajah
> <kugan.vivekanandarajah@linaro.org> wrote:
>> 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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [AARCH64] Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341
2017-06-27 1:20 [AARCH64] Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341 Kugan Vivekanandarajah
2017-06-27 8:01 ` Ramana Radhakrishnan
2017-07-21 10:12 ` Kugan Vivekanandarajah
@ 2017-08-29 12:02 ` James Greenhalgh
2017-08-30 9:05 ` Kugan Vivekanandarajah
2 siblings, 1 reply; 13+ messages in thread
From: James Greenhalgh @ 2017-08-29 12:02 UTC (permalink / raw)
To: Kugan Vivekanandarajah; +Cc: gcc-patches, Richard Earnshaw, nd
On Tue, Jun 27, 2017 at 11:20:02AM +1000, 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
>
> Bootstrapped and regression tested on aarch64-linux-gnu with no new regressions.
>
> Is this OK for trunk?
Hi Kugan,
I'm struggling a little to convince myself that this is correct. I think
the argument is that this was a workaround for one very particular issue
with the linux kernel module loader, which hard faults by refusing to
handle these relocations when in a workaround mode for Erratum 843419.
Most of these relocations won't occur because the kernel builds with
-mcmodel=large, but literals would always use a small model style
adrp/load, unless we were using -mpc-relative-literal-loads . So, this
workaround enabled -mpc-relative-literal-loads always when we were in
a workaround mode, thus allowing the kernel loader to continue.
The argument for removing it then, is that with the kernel now always using
-mpc-relative-literal-loads there is no reason for this workaround to stay
in place. The linkers which we will use will apply the workaround if needed.
Testcases and a detailed problem report of the build failures you had seen in
521.wrf would have helped me get closer to understanding this, and made
review substantially easier.
Am I on the right track?
If so, this is OK for trunk. If not, please can you expand on what is going
on in this patch.
Thanks,
James
>
> 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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [AARCH64] Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341
2017-08-29 12:02 ` James Greenhalgh
@ 2017-08-30 9:05 ` Kugan Vivekanandarajah
2018-03-07 1:59 ` Kugan Vivekanandarajah
0 siblings, 1 reply; 13+ messages in thread
From: Kugan Vivekanandarajah @ 2017-08-30 9:05 UTC (permalink / raw)
To: James Greenhalgh; +Cc: gcc-patches, Richard Earnshaw, nd
Hi James,
On 29 August 2017 at 21:31, James Greenhalgh <james.greenhalgh@arm.com> wrote:
> On Tue, Jun 27, 2017 at 11:20:02AM +1000, 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
>>
>> Bootstrapped and regression tested on aarch64-linux-gnu with no new regressions.
>>
>> Is this OK for trunk?
>
> Hi Kugan,
>
> I'm struggling a little to convince myself that this is correct. I think
> the argument is that this was a workaround for one very particular issue
> with the linux kernel module loader, which hard faults by refusing to
> handle these relocations when in a workaround mode for Erratum 843419.
Yes.
> Most of these relocations won't occur because the kernel builds with
> -mcmodel=large, but literals would always use a small model style
> adrp/load, unless we were using -mpc-relative-literal-loads . So, this
> workaround enabled -mpc-relative-literal-loads always when we were in
> a workaround mode, thus allowing the kernel loader to continue.
>
> The argument for removing it then, is that with the kernel now always using
> -mpc-relative-literal-loads there is no reason for this workaround to stay
> in place. The linkers which we will use will apply the workaround if needed.
Yes.
> Testcases and a detailed problem report of the build failures you had seen in
> 521.wrf would have helped me get closer to understanding this, and made
> review substantially easier.
Sorry for not being clear with this. Unfortunately 521.wrf was
showing this error with LTO so I could not reproduce with a small
enough test case.
> Am I on the right track?
>
> If so, this is OK for trunk. If not, please can you expand on what is going
> on in this patch.
Thanks for the review.
Kugan
>
> Thanks,
> James
>
>
>>
>> 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.
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [AARCH64] Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341
2017-08-30 9:05 ` Kugan Vivekanandarajah
@ 2018-03-07 1:59 ` Kugan Vivekanandarajah
2018-03-07 10:35 ` James Greenhalgh
0 siblings, 1 reply; 13+ messages in thread
From: Kugan Vivekanandarajah @ 2018-03-07 1:59 UTC (permalink / raw)
To: James Greenhalgh; +Cc: gcc-patches, Richard Earnshaw, nd
Hi James,
This patch has to be backported to gcc-7 branch as the build error for
521.wrf with LTO is there too (for the same reason). This patch has
been on trunk for some time now. So, is this OK to backport this
patch gcc-7 branch?
Thanks,
Kugan
On 30 August 2017 at 15:19, Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
> Hi James,
>
> On 29 August 2017 at 21:31, James Greenhalgh <james.greenhalgh@arm.com> wrote:
>> On Tue, Jun 27, 2017 at 11:20:02AM +1000, 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
>>>
>>> Bootstrapped and regression tested on aarch64-linux-gnu with no new regressions.
>>>
>>> Is this OK for trunk?
>>
>> Hi Kugan,
>>
>> I'm struggling a little to convince myself that this is correct. I think
>> the argument is that this was a workaround for one very particular issue
>> with the linux kernel module loader, which hard faults by refusing to
>> handle these relocations when in a workaround mode for Erratum 843419.
>
> Yes.
>
>> Most of these relocations won't occur because the kernel builds with
>> -mcmodel=large, but literals would always use a small model style
>> adrp/load, unless we were using -mpc-relative-literal-loads . So, this
>> workaround enabled -mpc-relative-literal-loads always when we were in
>> a workaround mode, thus allowing the kernel loader to continue.
>>
>> The argument for removing it then, is that with the kernel now always using
>> -mpc-relative-literal-loads there is no reason for this workaround to stay
>> in place. The linkers which we will use will apply the workaround if needed.
>
> Yes.
>
>> Testcases and a detailed problem report of the build failures you had seen in
>> 521.wrf would have helped me get closer to understanding this, and made
>> review substantially easier.
>
> Sorry for not being clear with this. Unfortunately 521.wrf was
> showing this error with LTO so I could not reproduce with a small
> enough test case.
>
>> Am I on the right track?
>>
>> If so, this is OK for trunk. If not, please can you expand on what is going
>> on in this patch.
>
> Thanks for the review.
> Kugan
>
>>
>> Thanks,
>> James
>>
>>
>>>
>>> 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.
>>
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [AARCH64] Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341
2018-03-07 1:59 ` Kugan Vivekanandarajah
@ 2018-03-07 10:35 ` James Greenhalgh
0 siblings, 0 replies; 13+ messages in thread
From: James Greenhalgh @ 2018-03-07 10:35 UTC (permalink / raw)
To: Kugan Vivekanandarajah; +Cc: gcc-patches, Richard Earnshaw, nd
On Wed, Mar 07, 2018 at 01:58:40AM +0000, Kugan Vivekanandarajah wrote:
> Hi James,
>
> This patch has to be backported to gcc-7 branch as the build error for
> 521.wrf with LTO is there too (for the same reason). This patch has
> been on trunk for some time now. So, is this OK to backport this
> patch gcc-7 branch?
OK.
Thanks,
James
> On 30 August 2017 at 15:19, Kugan Vivekanandarajah
> <kugan.vivekanandarajah@linaro.org> wrote:
> > Hi James,
> >
> > On 29 August 2017 at 21:31, James Greenhalgh <james.greenhalgh@arm.com> wrote:
> >> On Tue, Jun 27, 2017 at 11:20:02AM +1000, 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
> >>>
> >>> Bootstrapped and regression tested on aarch64-linux-gnu with no new regressions.
> >>>
> >>> Is this OK for trunk?
> >>
> >> Hi Kugan,
> >>
> >> I'm struggling a little to convince myself that this is correct. I think
> >> the argument is that this was a workaround for one very particular issue
> >> with the linux kernel module loader, which hard faults by refusing to
> >> handle these relocations when in a workaround mode for Erratum 843419.
> >
> > Yes.
> >
> >> Most of these relocations won't occur because the kernel builds with
> >> -mcmodel=large, but literals would always use a small model style
> >> adrp/load, unless we were using -mpc-relative-literal-loads . So, this
> >> workaround enabled -mpc-relative-literal-loads always when we were in
> >> a workaround mode, thus allowing the kernel loader to continue.
> >>
> >> The argument for removing it then, is that with the kernel now always using
> >> -mpc-relative-literal-loads there is no reason for this workaround to stay
> >> in place. The linkers which we will use will apply the workaround if needed.
> >
> > Yes.
> >
> >> Testcases and a detailed problem report of the build failures you had seen in
> >> 521.wrf would have helped me get closer to understanding this, and made
> >> review substantially easier.
> >
> > Sorry for not being clear with this. Unfortunately 521.wrf was
> > showing this error with LTO so I could not reproduce with a small
> > enough test case.
> >
> >> Am I on the right track?
> >>
> >> If so, this is OK for trunk. If not, please can you expand on what is going
> >> on in this patch.
> >
> > Thanks for the review.
> > Kugan
> >
> >>
> >> Thanks,
> >> James
> >>
> >>
> >>>
> >>> 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.
> >>
> >>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [AARCH64] Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341
2017-08-11 13:07 Wilco Dijkstra
@ 2017-08-11 13:08 ` Yvan Roux
0 siblings, 0 replies; 13+ messages in thread
From: Yvan Roux @ 2017-08-11 13:08 UTC (permalink / raw)
To: Wilco Dijkstra
Cc: GCC Patches, kugan.vivekanandarajah, nd, James Greenhalgh,
Richard Earnshaw
On 11 August 2017 at 13:50, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
> Kugan wrote:
>> Ping^2?
>
> Could you make sure to either include the patch or provide a link to it?
> (https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01967.html)
>
> I think the patch is fine since avoiding adrp/ldr on literals doesn't really
> help when global variables still use adrp. If you really want to avoid adrp
> and don't care about the overhead, there is -mcmodel=large...
>
> Note once you get the OK, we should consider backporting this with
> https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00386.html.
good point Wilco, I'll take care to add to the backport when it'll be accepted.
> Wilco
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [AARCH64] Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341
@ 2017-08-11 13:07 Wilco Dijkstra
2017-08-11 13:08 ` Yvan Roux
0 siblings, 1 reply; 13+ messages in thread
From: Wilco Dijkstra @ 2017-08-11 13:07 UTC (permalink / raw)
To: GCC Patches, kugan.vivekanandarajah
Cc: nd, James Greenhalgh, Richard Earnshaw
Kugan wrote:
> Ping^2?
Could you make sure to either include the patch or provide a link to it?
(https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01967.html)
I think the patch is fine since avoiding adrp/ldr on literals doesn't really
help when global variables still use adrp. If you really want to avoid adrp
and don't care about the overhead, there is -mcmodel=large...
Note once you get the OK, we should consider backporting this with
https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00386.html.
Wilco
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-03-07 10:35 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-27 1:20 [AARCH64] Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341 Kugan Vivekanandarajah
2017-06-27 8:01 ` Ramana Radhakrishnan
2017-06-28 1:02 ` Kugan Vivekanandarajah
2017-06-28 22:06 ` Ramana Radhakrishnan
2017-07-21 10:12 ` Kugan Vivekanandarajah
2017-08-11 10:34 ` Kugan Vivekanandarajah
2017-08-29 8:32 ` Kugan Vivekanandarajah
2017-08-29 12:02 ` James Greenhalgh
2017-08-30 9:05 ` Kugan Vivekanandarajah
2018-03-07 1:59 ` Kugan Vivekanandarajah
2018-03-07 10:35 ` James Greenhalgh
2017-08-11 13:07 Wilco Dijkstra
2017-08-11 13:08 ` Yvan Roux
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).