diff mbox

[AArch64] Disable pcrelative_literal_loads with fix-cortex-a53-843419

Message ID CAKdteObGkvOihvMW+taP6m4i5XMO5VBHOVSm_Pma1YbNZF5weQ@mail.gmail.com
State New
Headers show

Commit Message

Christophe Lyon Jan. 26, 2016, 2:43 p.m. UTC
With the attachment....


On 26 January 2016 at 15:42, Christophe Lyon <christophe.lyon@linaro.org> wrote:
> Hi,

>

> This is a followup to PR63304.

>

> As discussed in bugzilla, this patch disables pcrelative_literal_loads

> when -mfix-cortex-a53-843419 (or its default configure option) is

> used.

>

> I copied the behavior of -mfix-cortex-a53-835769 (e.g. in

> aarch64_can_inline_p), and I have tested by building the Linux kernel

> using -mfix-cortex-a53-843419 and checked that

> R_AARCH64_ADR_PREL_PG_HI21 relocations are not emitted anymore (under

> CONFIG_ARM64_ERRATUM_843419).

>

> For reference, this is motivated by:

> https://bugs.linaro.org/show_bug.cgi?id=1994

> and further details on Launchpad:

> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1533009

>

> OK for trunk?

>

> Thanks,

>

> Christophe.
2016-01-26  Christophe Lyon  <christophe.lyon@linaro.org>

	* config/aarch64/aarch64.h (TARGET_FIX_ERR_A53_843419_DEFAULT):
	Always define to 0 or 1.
	(TARGET_FIX_ERR_A53_843419): New macro.
	* config/aarch64/aarch64-elf-raw.h
	(TARGET_FIX_ERR_A53_843419_DEFAULT): Update for above changes.
	* config/aarch64/aarch64-linux.h: Likewise.
	* config/aarch64/aarch64.c
	(aarch64_override_options_after_change_1): Do not default
	aarch64_nopcrelative_literal_loads to true if Cortex-A53 erratum
	843419 is on.
	(aarch64_attributes): Handle fix-cortex-a53-843419.
	(aarch64_can_inline_p): Likewise.
	* config/aarch64/aarch64.opt (aarch64_fix_a53_err843419): Save.

Comments

Christophe Lyon Feb. 26, 2016, 12:29 p.m. UTC | #1
Ping?

On 26 January 2016 at 15:43, Christophe Lyon <christophe.lyon@linaro.org> wrote:
> With the attachment....

>

>

> On 26 January 2016 at 15:42, Christophe Lyon <christophe.lyon@linaro.org> wrote:

>> Hi,

>>

>> This is a followup to PR63304.

>>

>> As discussed in bugzilla, this patch disables pcrelative_literal_loads

>> when -mfix-cortex-a53-843419 (or its default configure option) is

>> used.

>>

>> I copied the behavior of -mfix-cortex-a53-835769 (e.g. in

>> aarch64_can_inline_p), and I have tested by building the Linux kernel

>> using -mfix-cortex-a53-843419 and checked that

>> R_AARCH64_ADR_PREL_PG_HI21 relocations are not emitted anymore (under

>> CONFIG_ARM64_ERRATUM_843419).

>>

>> For reference, this is motivated by:

>> https://bugs.linaro.org/show_bug.cgi?id=1994

>> and further details on Launchpad:

>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1533009

>>

>> OK for trunk?

>>

>> Thanks,

>>

>> Christophe.
James Greenhalgh March 10, 2016, 11:43 a.m. UTC | #2
On Tue, Jan 26, 2016 at 03:43:36PM +0100, Christophe Lyon wrote:
> With the attachment....

> 

> 

> On 26 January 2016 at 15:42, Christophe Lyon <christophe.lyon@linaro.org> wrote:

> > Hi,

> >

> > This is a followup to PR63304.

> >

> > As discussed in bugzilla, this patch disables pcrelative_literal_loads

> > when -mfix-cortex-a53-843419 (or its default configure option) is

> > used.

> >

> > I copied the behavior of -mfix-cortex-a53-835769 (e.g. in

> > aarch64_can_inline_p), and I have tested by building the Linux kernel

> > using -mfix-cortex-a53-843419 and checked that

> > R_AARCH64_ADR_PREL_PG_HI21 relocations are not emitted anymore (under

> > CONFIG_ARM64_ERRATUM_843419).

> >

> > For reference, this is motivated by:

> > https://bugs.linaro.org/show_bug.cgi?id=1994

> > and further details on Launchpad:

> > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1533009

> >

> > OK for trunk?


Thanks, this looks like a clear regression from GCC 5 (we can no longer
build the kernel, so this workaround is fine to go in now). Please remember
to add the link to the relevant PR in the ChangeLog.

I'd also really appreciate a nice big comment over this code:

> +  /* If it is not set on the command line, we default to no pc

> +     relative literal loads, unless the workaround for Cortex-A53

> +     erratum 843419 is in effect.  */

> +  if (opts->x_nopcrelative_literal_loads == 2

> +      && !TARGET_FIX_ERR_A53_843419)


Explaining why this is important (i.e. some summary of the discussion
in PR63304 regarding the kernel module loader).

Can you repost with that comment added? I don't have any other objections
to the patch.

Thanks,
James
diff mbox

Patch

diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 8b463c9..ec96ce3 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -179,6 +179,20 @@  extern unsigned aarch64_architecture_version;
   ((aarch64_fix_a53_err835769 == 2)	\
   ? TARGET_FIX_ERR_A53_835769_DEFAULT : aarch64_fix_a53_err835769)
 
+/* Make sure this is always defined so we don't have to check for ifdefs
+   but rather use normal ifs.  */
+#ifndef TARGET_FIX_ERR_A53_843419_DEFAULT
+#define TARGET_FIX_ERR_A53_843419_DEFAULT 0
+#else
+#undef TARGET_FIX_ERR_A53_843419_DEFAULT
+#define TARGET_FIX_ERR_A53_843419_DEFAULT 1
+#endif
+
+/* Apply the workaround for Cortex-A53 erratum 843419.  */
+#define TARGET_FIX_ERR_A53_843419	\
+  ((aarch64_fix_a53_err843419 == 2)	\
+  ? TARGET_FIX_ERR_A53_843419_DEFAULT : aarch64_fix_a53_err843419)
+
 /* ARMv8.1 Adv.SIMD support.  */
 #define TARGET_SIMD_RDMA (TARGET_SIMD && AARCH64_ISA_RDMA)
 
diff --git a/gcc/config/aarch64/aarch64-elf-raw.h b/gcc/config/aarch64/aarch64-elf-raw.h
index 2dcb6d4..9097017 100644
--- a/gcc/config/aarch64/aarch64-elf-raw.h
+++ b/gcc/config/aarch64/aarch64-elf-raw.h
@@ -35,7 +35,7 @@ 
   " %{mfix-cortex-a53-835769:--fix-cortex-a53-835769}"
 #endif
 
-#ifdef TARGET_FIX_ERR_A53_843419_DEFAULT
+#if TARGET_FIX_ERR_A53_843419_DEFAULT
 #define CA53_ERR_843419_SPEC \
   " %{!mno-fix-cortex-a53-843419:--fix-cortex-a53-843419}"
 #else
diff --git a/gcc/config/aarch64/aarch64-linux.h b/gcc/config/aarch64/aarch64-linux.h
index 6064b26..5fcaa59 100644
--- a/gcc/config/aarch64/aarch64-linux.h
+++ b/gcc/config/aarch64/aarch64-linux.h
@@ -53,7 +53,7 @@ 
   " %{mfix-cortex-a53-835769:--fix-cortex-a53-835769}"
 #endif
 
-#ifdef TARGET_FIX_ERR_A53_843419_DEFAULT
+#if TARGET_FIX_ERR_A53_843419_DEFAULT
 #define CA53_ERR_843419_SPEC \
   " %{!mno-fix-cortex-a53-843419:--fix-cortex-a53-843419}"
 #else
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 03bc1b9..3bea61e 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -8062,9 +8062,11 @@  aarch64_override_options_after_change_1 (struct gcc_options *opts)
   if (opts->x_nopcrelative_literal_loads == 1)
     aarch64_nopcrelative_literal_loads = false;
 
-  /* If it is not set on the command line, we default to no
-     pc relative literal loads.  */
-  if (opts->x_nopcrelative_literal_loads == 2)
+  /* If it is not set on the command line, we default to no pc
+     relative literal loads, unless the workaround for Cortex-A53
+     erratum 843419 is in effect.  */
+  if (opts->x_nopcrelative_literal_loads == 2
+      && !TARGET_FIX_ERR_A53_843419)
     aarch64_nopcrelative_literal_loads = true;
 
   /* In the tiny memory model it makes no sense
@@ -8748,6 +8750,8 @@  static const struct aarch64_attribute_info aarch64_attributes[] =
      OPT_mgeneral_regs_only },
   { "fix-cortex-a53-835769", aarch64_attr_bool, true, NULL,
      OPT_mfix_cortex_a53_835769 },
+  { "fix-cortex-a53-843419", aarch64_attr_bool, true, NULL,
+     OPT_mfix_cortex_a53_843419 },
   { "cmodel", aarch64_attr_enum, false, NULL, OPT_mcmodel_ },
   { "strict-align", aarch64_attr_mask, false, NULL, OPT_mstrict_align },
   { "omit-leaf-frame-pointer", aarch64_attr_bool, true, NULL,
@@ -9162,6 +9166,12 @@  aarch64_can_inline_p (tree caller, tree callee)
 	  2, TARGET_FIX_ERR_A53_835769_DEFAULT))
     return false;
 
+  if (!aarch64_tribools_ok_for_inlining_p (
+	  caller_opts->x_aarch64_fix_a53_err843419,
+	  callee_opts->x_aarch64_fix_a53_err843419,
+	  2, TARGET_FIX_ERR_A53_843419))
+    return false;
+
   /* If the user explicitly specified -momit-leaf-frame-pointer for the
      caller and calle and they don't match up, reject inlining.  */
   if (!aarch64_tribools_ok_for_inlining_p (
diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
index 5cbd4cd..2427826 100644
--- a/gcc/config/aarch64/aarch64.opt
+++ b/gcc/config/aarch64/aarch64.opt
@@ -73,7 +73,7 @@  Target Report Var(aarch64_fix_a53_err835769) Init(2) Save
 Workaround for ARM Cortex-A53 Erratum number 835769.
 
 mfix-cortex-a53-843419
-Target Report
+Target Report Var(aarch64_fix_a53_err843419) Init(2) Save
 Workaround for ARM Cortex-A53 Erratum number 843419.
 
 mlittle-endian