Re: [AARCH64] Disable pc relative literal load irrespective of TARGET_FIX_ERR_A53_84341

[Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>]

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.
>>
>>