public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Kyrill Tkachov <kyrylo.tkachov@arm.com>
To: GCC Patches <gcc-patches@gcc.gnu.org>
Cc: Marcus Shawcroft <marcus.shawcroft@arm.com>,
	 Richard Earnshaw <Richard.Earnshaw@arm.com>,
	James Greenhalgh <james.greenhalgh@arm.com>
Subject: [PATCH][AArch64] Fix FAIL: gcc.target/aarch64/target_attr_crypto_ice_1.c (internal compiler error)
Date: Wed, 19 Aug 2015 13:43:00 -0000	[thread overview]
Message-ID: <55D4878F.4040803@arm.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 1382 bytes --]

Hi all,

This fixes the ICE exposed by Alexandre's patch (https://gcc.gnu.org/ml/gcc-patches/2015-08/msg00873.html)
The solution I came up with is to re-layout the parameter decls not during expansion time (when RTL has already
been allocated to SSA names) but in TARGET_SET_CURRENT_FUNCTION which is called much earlier before that and is
used when setting cfun. This way we reach expand with the proper vector modes registered for the param decls
and all seems to work ok.

The aarch64-builtins.c workaround that I initially introduced in https://gcc.gnu.org/ml/gcc-patches/2015-07/msg02012.html
are partially reverted (at least the re-laying out parts).

The patch fixes the target_attr_crypto_ice_1.c ICE but I'd like to add a second derived testcase that
tests a different expansion path and it has proved useful in writing this patch.

Bootstrapped and tested on aarch64.

Ok for trunk?

Thanks,
Kyrill

2015-08-19  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * config/aarch64/aarch64.c (aarch64_set_current_function):
     Re-layout any vector parameters have non-simd layout.
     * config/aarch64/aarch64-builtins.c (aarch64_relayout_simd_param):
     Delete.
     (aarch64_simd_expand_args): Delete call to the above.

2015-08-19  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * gcc.target/aarch64/target_attr_crypto_ice_2.c: New test.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: aarch64-target-attr-ice.patch --]
[-- Type: text/x-patch; name=aarch64-target-attr-ice.patch, Size: 3477 bytes --]

commit 94093e43f5bc91f3afa1002f41dcd423e8db3237
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Tue Aug 18 17:02:26 2015 +0100

    [AArch64] Re-layout vector parameter DECLs in TARGET_SET_CURRENT_FUNCTION

diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
index 0f4f2b9..e3a90b5 100644
--- a/gcc/config/aarch64/aarch64-builtins.c
+++ b/gcc/config/aarch64/aarch64-builtins.c
@@ -886,30 +886,6 @@ typedef enum
   SIMD_ARG_STOP
 } builtin_simd_arg;
 
-/* Relayout the decl of a function arg.  Keep the RTL component the same,
-   as varasm.c ICEs.  It doesn't like reinitializing the RTL
-   on PARM decls.  Something like this needs to be done when compiling a
-   file without SIMD and then tagging a function with +simd and using SIMD
-   intrinsics in there.  The types will have been laid out assuming no SIMD,
-   so we want to re-lay them out.  */
-
-static void
-aarch64_relayout_simd_param (tree arg)
-{
-  tree argdecl = arg;
-  if (TREE_CODE (argdecl) == SSA_NAME)
-    argdecl = SSA_NAME_VAR (argdecl);
-
-  if (argdecl
-      && (TREE_CODE (argdecl) == PARM_DECL
-	  || TREE_CODE (argdecl) == VAR_DECL))
-    {
-      rtx rtl = NULL_RTX;
-      rtl = DECL_RTL_IF_SET (argdecl);
-      relayout_decl (argdecl);
-      SET_DECL_RTL (argdecl, rtl);
-    }
-}
 
 static rtx
 aarch64_simd_expand_args (rtx target, int icode, int have_retval,
@@ -940,7 +916,6 @@ aarch64_simd_expand_args (rtx target, int icode, int have_retval,
 	{
 	  tree arg = CALL_EXPR_ARG (exp, opc - have_retval);
 	  enum machine_mode mode = insn_data[icode].operand[opc].mode;
-	  aarch64_relayout_simd_param (arg);
 	  op[opc] = expand_normal (arg);
 
 	  switch (thisarg)
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 217b4d7..b16e511 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -8073,6 +8073,23 @@ aarch64_set_current_function (tree fndecl)
 	      = save_target_globals_default_opts ();
 	}
     }
+
+  if (!fndecl)
+    return;
+
+  /* If we turned on SIMD make sure that any vector parameters are re-laid out
+     so that they use proper vector modes.  */
+  if (TARGET_SIMD)
+    {
+      tree parms = DECL_ARGUMENTS (fndecl);
+      for (; parms && parms != void_list_node; parms = TREE_CHAIN (parms))
+	{
+	  if (TREE_CODE (parms) == PARM_DECL
+	      && VECTOR_TYPE_P (TREE_TYPE (parms))
+	      && DECL_MODE (parms) != TYPE_MODE (TREE_TYPE (parms)))
+	    relayout_decl (parms);
+	}
+    }
 }
 
 /* Enum describing the various ways we can handle attributes.
diff --git a/gcc/testsuite/gcc.target/aarch64/target_attr_crypto_ice_2.c b/gcc/testsuite/gcc.target/aarch64/target_attr_crypto_ice_2.c
new file mode 100644
index 0000000..d6e7b68
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/target_attr_crypto_ice_2.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mcpu=thunderx+nofp" } */
+
+/* Make sure that we don't ICE when dealing with vector parameters
+   in a simd-tagged function within a non-simd translation unit.  */
+
+#pragma GCC push_options
+#pragma GCC target ("+nothing+simd")
+typedef unsigned int __uint32_t;
+typedef __uint32_t uint32_t ;
+typedef __Uint32x4_t uint32x4_t;
+#pragma GCC pop_options
+
+
+__attribute__ ((target ("cpu=cortex-a57")))
+uint32x4_t
+foo (uint32x4_t a, uint32_t b, uint32x4_t c)
+{
+  return c;
+}

             reply	other threads:[~2015-08-19 13:41 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-19 13:43 Kyrill Tkachov [this message]
2015-09-01  8:39 ` Kyrill Tkachov
2015-09-01  9:26 ` James Greenhalgh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55D4878F.4040803@arm.com \
    --to=kyrylo.tkachov@arm.com \
    --cc=Richard.Earnshaw@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=james.greenhalgh@arm.com \
    --cc=marcus.shawcroft@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).