| 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) ();
+}
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.