Hi, On 22 June 2017 at 20:42, Yvan Roux <yvan.r...@linaro.org> wrote: > Hi all, > > On 16 January 2017 at 16:34, Kyrill Tkachov <kyrylo.tkac...@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 ?
Sorry for the fast ping, but since a 6.4 rc1 is planned for tomorrow, do you think that this fix can be part of it ? > Thanks > Yvan > > gcc/ChangeLog > 2017-06-22 Yvan Roux <yvan.r...@linaroi.org> > > PR target/79041 > Backport from mainline > 2016-06-20 Wilco Dijkstra <wdijk...@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.tkac...@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.r...@linaroi.org> > > PR target/79041 > * gcc.target/aarch64/pr79041.c: New test.
