[PATCH][AArch64][GCC 6] PR target/79041: Correct -mpc-relative-literal-loads logic in aarch64_classify_symbol

[Yvan Roux]

Hi all,

On 16 January 2017 at 16:34, Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> wrote:
>
> On 13/01/17 16:35, James Greenhalgh wrote:
>>
>> On Wed, Jan 11, 2017 at 04:32:45PM +0000, Kyrill Tkachov wrote:
>>>
>>> Hi all,
>>>
>>> In this PR we generated ADRP/ADD instructions with :lo12: relocations on
>>> symbols even though -mpc-relative-literal-loads is used. This is due to
>>> the
>>> confusing double-negative logic of the nopcrelative_literal_loads aarch64
>>> variable and its relation to the aarch64_nopcrelative_literal_loads
>>> global
>>> variable in the GCC 6 branch.
>>>
>>> Wilco fixed this on trunk as part of a larger patch (r237607) and parts
>>> of
>>> that patch were backported, but other parts weren't and that patch now
>>> doesn't apply cleanly to the branch.
>>
>> As I commented to Jakub at the time he made the first partial backport,
>> I'd much rather just see all of Wilco's patch backported. We're not on
>> the verge of a 6 release now, so please just backport Wilco's patch (as
>> should have been done all along if this had been correctly identified as
>> a fix rather than a clean-up).
>
>
> Unfortunately simply backporting the patch does not fix this PR.
> The aarch64_classify_symbol changes do not have the desired effect
> and the :lo12: relocations are generated.
> I'll look into it, but I believe that would require a bigger change than
> this one-liner.

Here is the backport of Wilco's patch (r237607) along with Kyrill's
one (r244643, which removed the remaining occurences of
aarch64_nopcrelative_literal_loads).  To fix the issue the original
patch has to be modified, to keep aarch64_pcrelative_literal_loads
test for large models in aarch64_classify_symbol.

On trunk and gcc-7-branch the :lo12: relocations are not generated
because of Wilco's fix for pr78733 (r243456 and 243486), but my
understanding is that the bug is still present since compiling
gcc.target/aarch64/pr78733.c with -mcmodel=large brings back the
:lo12: relocations (I'll submit a patch to add the test back if my
understanding is correct).

Cross built and regtested without issue for aarch64-linux-gnu,
aarch64-none-elf and aarch64_be-none-elf targets

OK for gcc-6-branch ?

Thanks
Yvan

gcc/ChangeLog
2017-06-22  Yvan Roux  <yvan.roux@linaroi.org>

        PR target/79041
        Backport from mainline
        2016-06-20  Wilco Dijkstra  <wdijkstr@arm.com>

        * config/aarch64/aarch64.opt
        (mpc-relative-literal-loads): Rename internal option name.
        * config/aarch64/aarch64.c
        (aarch64_nopcrelative_literal_loads): Rename to
        aarch64_pcrelative_literal_loads.
        (aarch64_expand_mov_immediate): Likewise.
        (aarch64_secondary_reload): Likewise.
        (aarch64_can_use_per_function_literal_pools_p): Likewise.
        (aarch64_classify_symbol): Likewise.
        (aarch64_override_options_after_change_1): Rename and simplify logic.

        2016-01-19  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

        * config/aarch64/aarch64-protos.h(aarch64_nopcrelative_literal_loads):
        Delete.
        * config/aarch64/aarch64.md
        (aarch64_reload_movcp<GPF_TF:mode><P:mode>): Delete reference to
        aarch64_nopcrelative_literal_loads.
        (aarch64_reload_movcp<VALL:mode><P:mode>): Likewise.

gcc/testsuite/ChangeLog
2017-06-22  Yvan Roux  <yvan.roux@linaroi.org>

        PR target/79041
        * gcc.target/aarch64/pr79041.c: New test.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr79041.patch
Type: text/x-patch
Size: 7661 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20170622/f9c8a01c/attachment.bin>