Message ID | 5710F670.3010605@foss.arm.com |
---|---|
State | New |
Headers | show |
On 15/04/16 15:10, Kyrill Tkachov wrote: > Hi all, > > This is a repost of Andrew's fix for PR target/64971 that was originally posted at: > https://gcc.gnu.org/ml/gcc-patches/2015-02/msg00502.html > > The only change is that I substituted DImode for Pmode and added a FIXME comment > to remind us to revisit this (see the PR in bugzilla for more info). > > Bootstrapped and tested on aarch64-none-linux-gnu (LP64, I don't have access to a full ILP32 system) > This patch affects only ILP32 codegen so I've run a make check on aarch64-none-elf with /-mabi=ilp32 and nothing regressed. > I think at this stage it's the least risky band-aid. > > Is this ok for trunk at this stage? > > Thanks, > Kyrill > > 2016-04-15 Andrew Pinski <apinski@cavium.com> > Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > PR target/65971 Of course this should say: "PR target/64971". Kyrill > * config/aarch64/aarch64.md (sibcall): Force call > address to be DImode for ILP32. > (sibcall_value): Likewise. > > 2016-04-15 Andrew Pinski <apinski@cavium.com> > > * gcc.c-torture/compile/pr37433-1.c: New testcase.
On 04/15/2016 08:10 AM, Kyrill Tkachov wrote: > Hi all, > > This is a repost of Andrew's fix for PR target/64971 that was originally > posted at: > https://gcc.gnu.org/ml/gcc-patches/2015-02/msg00502.html > > The only change is that I substituted DImode for Pmode and added a FIXME > comment > to remind us to revisit this (see the PR in bugzilla for more info). > > Bootstrapped and tested on aarch64-none-linux-gnu (LP64, I don't have > access to a full ILP32 system) > This patch affects only ILP32 codegen so I've run a make check on > aarch64-none-elf with /-mabi=ilp32 and nothing regressed. > I think at this stage it's the least risky band-aid. > > Is this ok for trunk at this stage? > > Thanks, > Kyrill > > 2016-04-15 Andrew Pinski <apinski@cavium.com> > Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > PR target/65971 > * config/aarch64/aarch64.md (sibcall): Force call > address to be DImode for ILP32. > (sibcall_value): Likewise. > > 2016-04-15 Andrew Pinski <apinski@cavium.com> > > * gcc.c-torture/compile/pr37433-1.c: New testcase. Just a note, gcc-6 has branched, so you'll need to apply to the trunk and the branch once approved. I would suggest that the issues raised by Richard be addressed separately, possibly using a new BZ for tracking purposes. Jeff
On Fri, Apr 15, 2016 at 03:12:58PM +0100, Kyrill Tkachov wrote: > > On 15/04/16 15:10, Kyrill Tkachov wrote: > >Hi all, > > > >This is a repost of Andrew's fix for PR target/64971 that was originally posted at: > >https://gcc.gnu.org/ml/gcc-patches/2015-02/msg00502.html > > > >The only change is that I substituted DImode for Pmode and added a FIXME > >comment to remind us to revisit this (see the PR in bugzilla for more info). > > > >Bootstrapped and tested on aarch64-none-linux-gnu (LP64, I don't have access > >to a full ILP32 system) This patch affects only ILP32 codegen so I've run a > >make check on aarch64-none-elf with /-mabi=ilp32 and nothing regressed. > >I think at this stage it's the least risky band-aid. > > > >Is this ok for trunk at this stage? I hope that we are able to revisit this for GCC 7 with the more complete fixes detailed in the bug report. I've got no objections to the patch as a band-aid step forward for the ILP32 ABI, and the patch is no risk to the LP64 ABI (the code added is very clearly predicated on TARGET_ILP32). As Jeff points out, this will need RM approval to go in to GCC 6. Thanks, James
On 15/04/16 17:27, James Greenhalgh wrote: > On Fri, Apr 15, 2016 at 03:12:58PM +0100, Kyrill Tkachov wrote: >> On 15/04/16 15:10, Kyrill Tkachov wrote: >>> Hi all, >>> >>> This is a repost of Andrew's fix for PR target/64971 that was originally posted at: >>> https://gcc.gnu.org/ml/gcc-patches/2015-02/msg00502.html >>> >>> The only change is that I substituted DImode for Pmode and added a FIXME >>> comment to remind us to revisit this (see the PR in bugzilla for more info). >>> >>> Bootstrapped and tested on aarch64-none-linux-gnu (LP64, I don't have access >>> to a full ILP32 system) This patch affects only ILP32 codegen so I've run a >>> make check on aarch64-none-elf with /-mabi=ilp32 and nothing regressed. >>> I think at this stage it's the least risky band-aid. >>> >>> Is this ok for trunk at this stage? > I hope that we are able to revisit this for GCC 7 with the more complete > fixes detailed in the bug report. > > I've got no objections to the patch as a band-aid step forward for the > ILP32 ABI, and the patch is no risk to the LP64 ABI (the code added is very > clearly predicated on TARGET_ILP32). > > As Jeff points out, this will need RM approval to go in to GCC 6. Sorry for the early ping, but since we're planning for RC2 this week can I apply this to the GCC 6 branch? Thanks, Kyrill > Thanks, > James >
On Wed, 20 Apr 2016, Kyrill Tkachov wrote: > > On 15/04/16 17:27, James Greenhalgh wrote: > > On Fri, Apr 15, 2016 at 03:12:58PM +0100, Kyrill Tkachov wrote: > > > On 15/04/16 15:10, Kyrill Tkachov wrote: > > > > Hi all, > > > > > > > > This is a repost of Andrew's fix for PR target/64971 that was originally > > > > posted at: > > > > https://gcc.gnu.org/ml/gcc-patches/2015-02/msg00502.html > > > > > > > > The only change is that I substituted DImode for Pmode and added a FIXME > > > > comment to remind us to revisit this (see the PR in bugzilla for more > > > > info). > > > > > > > > Bootstrapped and tested on aarch64-none-linux-gnu (LP64, I don't have > > > > access > > > > to a full ILP32 system) This patch affects only ILP32 codegen so I've > > > > run a > > > > make check on aarch64-none-elf with /-mabi=ilp32 and nothing regressed. > > > > I think at this stage it's the least risky band-aid. > > > > > > > > Is this ok for trunk at this stage? > > I hope that we are able to revisit this for GCC 7 with the more complete > > fixes detailed in the bug report. > > > > I've got no objections to the patch as a band-aid step forward for the > > ILP32 ABI, and the patch is no risk to the LP64 ABI (the code added is very > > clearly predicated on TARGET_ILP32). > > > > As Jeff points out, this will need RM approval to go in to GCC 6. > > Sorry for the early ping, but since we're planning for RC2 this week > can I apply this to the GCC 6 branch? Sure. A broken ILP32 aarch64 won't block the release. Thanks, Richard. > Thanks, > Kyrill > > > Thanks, > > James > > > >
commit ff99b9cb21a195fb2b2c0e4d580db2b1e806ec97 Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com> Date: Thu Apr 14 10:52:45 2016 +0100 [AArch64] From Andrew Pinski: Work around PR target/64971 diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index da85a7f..a9e811e 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -855,6 +855,13 @@ (define_expand "sibcall" || aarch64_is_noplt_call_p (callee))) XEXP (operands[0], 0) = force_reg (Pmode, callee); + /* FIXME: This is a band-aid. Need to analyze why expand_expr_addr_expr + is generating an SImode symbol reference. See PR 64971. */ + if (TARGET_ILP32 + && GET_CODE (XEXP (operands[0], 0)) == SYMBOL_REF + && GET_MODE (XEXP (operands[0], 0)) == SImode) + XEXP (operands[0], 0) = convert_memory_address (Pmode, + XEXP (operands[0], 0)); if (operands[2] == NULL_RTX) operands[2] = const0_rtx; @@ -886,6 +893,14 @@ (define_expand "sibcall_value" || aarch64_is_noplt_call_p (callee))) XEXP (operands[1], 0) = force_reg (Pmode, callee); + /* FIXME: This is a band-aid. Need to analyze why expand_expr_addr_expr + is generating an SImode symbol reference. See PR 64971. */ + if (TARGET_ILP32 + && GET_CODE (XEXP (operands[1], 0)) == SYMBOL_REF + && GET_MODE (XEXP (operands[1], 0)) == SImode) + XEXP (operands[1], 0) = convert_memory_address (Pmode, + XEXP (operands[1], 0)); + if (operands[3] == NULL_RTX) operands[3] = const0_rtx; diff --git a/gcc/testsuite/gcc.c-torture/compile/pr37433-1.c b/gcc/testsuite/gcc.c-torture/compile/pr37433-1.c new file mode 100644 index 0000000..322c167 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/pr37433-1.c @@ -0,0 +1,11 @@ +void regex_subst(void) +{ + const void *subst = ""; + (*(void (*)(int))subst) (0); +} + +void foobar (void) +{ + int x; + (*(void (*)(void))&x) (); +}