diff mbox

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

Message ID CAKdteOZNUYpZZuhNABt0TmjYZ82A4D_ha5KfE20tbvTsexA-1g@mail.gmail.com
State New
Headers show

Commit Message

Christophe Lyon March 14, 2016, 3:34 p.m. UTC
On 10 March 2016 at 14:24, James Greenhalgh <james.greenhalgh@arm.com> wrote:
> On Thu, Mar 10, 2016 at 01:37:50PM +0100, Christophe Lyon wrote:
>> On 10 March 2016 at 12:43, James Greenhalgh <james.greenhalgh@arm.com> wrote:
>> > 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.
>> >
>>
>> OK, here is an updated version.
>
> Thanks.
>
> This is OK for trunk.
>

When GCC is configured to enable the A53 erratum 843419 workaround by default,
this patch caused gcc.target/aarch64/pr63304_1.c to fail.

The attached patch fixes the problem by forcing the use of
-mno-fix-cortex-a53-843419.

OK, or do we prefer not to bother?

Thanks,

Christophe


> James
>
2016-03-14  Christophe Lyon  <christophe.lyon@linaro.org>

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

Comments

Richard Earnshaw (lists) March 18, 2016, 11:41 a.m. UTC | #1
On 14/03/16 15:34, Christophe Lyon wrote:
> On 10 March 2016 at 14:24, James Greenhalgh <james.greenhalgh@arm.com> wrote:
>> On Thu, Mar 10, 2016 at 01:37:50PM +0100, Christophe Lyon wrote:
>>> On 10 March 2016 at 12:43, James Greenhalgh <james.greenhalgh@arm.com> wrote:
>>>> 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.
>>>>
>>>
>>> OK, here is an updated version.
>>
>> Thanks.
>>
>> This is OK for trunk.
>>
> 
> When GCC is configured to enable the A53 erratum 843419 workaround by default,
> this patch caused gcc.target/aarch64/pr63304_1.c to fail.
> 
> The attached patch fixes the problem by forcing the use of
> -mno-fix-cortex-a53-843419.
> 
> OK, or do we prefer not to bother?
> 
> Thanks,
> 
> Christophe
> 
> 
>> James
>>
>>
>> pr70113.log.txt
>>
>>
>> 2016-03-14  Christophe Lyon  <christophe.lyon@linaro.org>
>>
>> 	* gcc.target/aarch64/pr63304_1.c: Add -mno-fix-cortex-a53-843419.
>>

OK.

R.
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.target/aarch64/pr63304_1.c b/gcc/testsuite/gcc.target/aarch64/pr63304_1.c
index fa0fb56..c917f81c 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" } */
+/* { dg-options "-O1 --save-temps -mno-fix-cortex-a53-843419" } */
 #pragma GCC push_options
 #pragma GCC target ("+nothing+simd, cmodel=small")