public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, rs6000] Fix PR target/80210: ICE in extract_insn
@ 2017-08-18  6:39 Peter Bergner
  2017-08-19  0:04 ` Segher Boessenkool
  2017-09-08 19:11 ` Andreas Schwab
  0 siblings, 2 replies; 8+ messages in thread
From: Peter Bergner @ 2017-08-18  6:39 UTC (permalink / raw)
  To: GCC Patches; +Cc: Segher Boessenkool, Bill Schmidt, Michael Meissner

PR target/80210 exposes a problem in rs6000_set_current_function() where
is fails to correctly clear the rs6000_previous_fndecl cache correctly.
With the test case, we notice that rs6000_previous_fndecl is set (when it
shouldn't be) and we end up restoring options from it.  In this case,
we end up disabling HW sqrt (because of the pragma) when we earlier
decided we could generate HW sqrts which leads to an ICE.

The current code in rs6000_set_current_function() is kind of a rats nest,
so I threw it out and rewrote it, modeling it after how S390 and i386
handle it, which correctly clears the *_previous_fndecl caches.
It also makes the code much more readable in my view.

This passed bootstrap and regtesting with no regressions and it fixes
the ICE.  Ok for trunk?

This is also broken in GCC 7, GCC 6 and GCC 5.  Ok for those after this
has been on trunk for a little while and assuming testing passes?

Peter


gcc/
	* config/rs6000/rs6000.c (rs6000_activate_target_options): New function.
	(rs6000_set_current_function): Rewrite function to use it.

gcc/testsuite/

	* gcc.target/powerpc/pr80210.c: New test.

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 251171)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -36664,23 +36664,30 @@ rs6000_pragma_target_parse (tree args, t
 /* Remember the last target of rs6000_set_current_function.  */
 static GTY(()) tree rs6000_previous_fndecl;
 
+/* Restore target's globals from NEW_TREE and invalidate the
+   rs6000_previous_fndecl cache.  */
+
+static void
+rs6000_activate_target_options (tree new_tree)
+{
+  cl_target_option_restore (&global_options, TREE_TARGET_OPTION (new_tree));
+  if (TREE_TARGET_GLOBALS (new_tree))
+    restore_target_globals (TREE_TARGET_GLOBALS (new_tree));
+  else if (new_tree == target_option_default_node)
+    restore_target_globals (&default_target_globals);
+  else
+    TREE_TARGET_GLOBALS (new_tree) = save_target_globals_default_opts ();
+  rs6000_previous_fndecl = NULL_TREE;
+}
+
 /* Establish appropriate back-end context for processing the function
    FNDECL.  The argument might be NULL to indicate processing at top
    level, outside of any function scope.  */
 static void
 rs6000_set_current_function (tree fndecl)
 {
-  tree old_tree = (rs6000_previous_fndecl
-		   ? DECL_FUNCTION_SPECIFIC_TARGET (rs6000_previous_fndecl)
-		   : NULL_TREE);
-
-  tree new_tree = (fndecl
-		   ? DECL_FUNCTION_SPECIFIC_TARGET (fndecl)
-		   : NULL_TREE);
-
   if (TARGET_DEBUG_TARGET)
     {
-      bool print_final = false;
       fprintf (stderr, "\n==================== rs6000_set_current_function");
 
       if (fndecl)
@@ -36693,58 +36700,60 @@ rs6000_set_current_function (tree fndecl
 	fprintf (stderr, ", prev_fndecl (%p)", (void *)rs6000_previous_fndecl);
 
       fprintf (stderr, "\n");
+    }
+
+  /* Only change the context if the function changes.  This hook is called
+     several times in the course of compiling a function, and we don't want to
+     slow things down too much or call target_reinit when it isn't safe.  */
+  if (fndecl == rs6000_previous_fndecl)
+    return;
+
+  tree old_tree;
+  if (rs6000_previous_fndecl == NULL_TREE)
+    old_tree = target_option_current_node;
+  else if (DECL_FUNCTION_SPECIFIC_TARGET (rs6000_previous_fndecl))
+    old_tree = DECL_FUNCTION_SPECIFIC_TARGET (rs6000_previous_fndecl);
+  else
+    old_tree = target_option_default_node;
+
+  tree new_tree;
+  if (fndecl == NULL_TREE)
+    {
+      if (old_tree != target_option_current_node)
+	new_tree = target_option_current_node;
+      else
+	new_tree = NULL_TREE;
+    }
+  else
+    {
+      new_tree = DECL_FUNCTION_SPECIFIC_TARGET (fndecl);
+      if (new_tree == NULL_TREE)
+	new_tree = target_option_default_node;
+    }
+
+  if (TARGET_DEBUG_TARGET)
+    {
       if (new_tree)
 	{
 	  fprintf (stderr, "\nnew fndecl target specific options:\n");
 	  debug_tree (new_tree);
-	  print_final = true;
 	}
 
       if (old_tree)
 	{
 	  fprintf (stderr, "\nold fndecl target specific options:\n");
 	  debug_tree (old_tree);
-	  print_final = true;
 	}
 
-      if (print_final)
+      if (old_tree != NULL_TREE || new_tree != NULL_TREE)
 	fprintf (stderr, "--------------------\n");
     }
 
-  /* Only change the context if the function changes.  This hook is called
-     several times in the course of compiling a function, and we don't want to
-     slow things down too much or call target_reinit when it isn't safe.  */
-  if (fndecl && fndecl != rs6000_previous_fndecl)
-    {
-      rs6000_previous_fndecl = fndecl;
-      if (old_tree == new_tree)
-	;
-
-      else if (new_tree && new_tree != target_option_default_node)
-	{
-	  cl_target_option_restore (&global_options,
-				    TREE_TARGET_OPTION (new_tree));
-	  if (TREE_TARGET_GLOBALS (new_tree))
-	    restore_target_globals (TREE_TARGET_GLOBALS (new_tree));
-	  else
-	    TREE_TARGET_GLOBALS (new_tree)
-	      = save_target_globals_default_opts ();
-	}
+  if (new_tree && old_tree != new_tree)
+    rs6000_activate_target_options (new_tree);
 
-      else if (old_tree && old_tree != target_option_default_node)
-	{
-	  new_tree = target_option_current_node;
-	  cl_target_option_restore (&global_options,
-				    TREE_TARGET_OPTION (new_tree));
-	  if (TREE_TARGET_GLOBALS (new_tree))
-	    restore_target_globals (TREE_TARGET_GLOBALS (new_tree));
-	  else if (new_tree == target_option_default_node)
-	    restore_target_globals (&default_target_globals);
-	  else
-	    TREE_TARGET_GLOBALS (new_tree)
-	      = save_target_globals_default_opts ();
-	}
-    }
+  if (fndecl)
+    rs6000_previous_fndecl = fndecl;
 }
 
 \f
Index: gcc/testsuite/gcc.target/powerpc/pr80210.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr80210.c	(nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/pr80210.c	(working copy)
@@ -0,0 +1,10 @@
+/* Test for ICE arising from GCC target pragma.  */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+double
+foo (double a)
+{
+  return __builtin_sqrt (a);
+}
+#pragma GCC target "no-powerpc-gpopt"

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH, rs6000] Fix PR target/80210: ICE in extract_insn
  2017-08-18  6:39 [PATCH, rs6000] Fix PR target/80210: ICE in extract_insn Peter Bergner
@ 2017-08-19  0:04 ` Segher Boessenkool
  2017-08-20  5:12   ` Peter Bergner
  2017-09-08 19:11 ` Andreas Schwab
  1 sibling, 1 reply; 8+ messages in thread
From: Segher Boessenkool @ 2017-08-19  0:04 UTC (permalink / raw)
  To: Peter Bergner; +Cc: GCC Patches, Bill Schmidt, Michael Meissner

On Thu, Aug 17, 2017 at 07:02:14PM -0500, Peter Bergner wrote:
> PR target/80210 exposes a problem in rs6000_set_current_function() where
> is fails to correctly clear the rs6000_previous_fndecl cache correctly.
> With the test case, we notice that rs6000_previous_fndecl is set (when it
> shouldn't be) and we end up restoring options from it.  In this case,
> we end up disabling HW sqrt (because of the pragma) when we earlier
> decided we could generate HW sqrts which leads to an ICE.
> 
> The current code in rs6000_set_current_function() is kind of a rats nest,
> so I threw it out and rewrote it, modeling it after how S390 and i386
> handle it, which correctly clears the *_previous_fndecl caches.
> It also makes the code much more readable in my view.

Yeah, it almost looks easy this way.  Treacherous :-)

> This passed bootstrap and regtesting with no regressions and it fixes
> the ICE.  Ok for trunk?
> 
> This is also broken in GCC 7, GCC 6 and GCC 5.  Ok for those after this
> has been on trunk for a little while and assuming testing passes?

Okay for trunk and all branches.  Thanks!


Segher


> gcc/
> 	* config/rs6000/rs6000.c (rs6000_activate_target_options): New function.
> 	(rs6000_set_current_function): Rewrite function to use it.
> 
> gcc/testsuite/
> 
> 	* gcc.target/powerpc/pr80210.c: New test.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH, rs6000] Fix PR target/80210: ICE in extract_insn
  2017-08-19  0:04 ` Segher Boessenkool
@ 2017-08-20  5:12   ` Peter Bergner
  2017-08-23 15:58     ` Peter Bergner
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Bergner @ 2017-08-20  5:12 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: GCC Patches, Bill Schmidt, Michael Meissner

On 8/18/17 6:27 PM, Segher Boessenkool wrote:
> On Thu, Aug 17, 2017 at 07:02:14PM -0500, Peter Bergner wrote:
>> This is also broken in GCC 7, GCC 6 and GCC 5.  Ok for those after this
>> has been on trunk for a little while and assuming testing passes?
> 
> Okay for trunk and all branches.  Thanks!

Ok, committed to trunk and the back port bootstraps/regtesting
are running.  If they're clean, I'll commit this sometime next
week after the trunk version has baked a little bit.
Thanks!

Peter

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH, rs6000] Fix PR target/80210: ICE in extract_insn
  2017-08-20  5:12   ` Peter Bergner
@ 2017-08-23 15:58     ` Peter Bergner
  0 siblings, 0 replies; 8+ messages in thread
From: Peter Bergner @ 2017-08-23 15:58 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: GCC Patches, Bill Schmidt, Michael Meissner

On 8/18/17 9:19 PM, Peter Bergner wrote:
> On 8/18/17 6:27 PM, Segher Boessenkool wrote:
>> On Thu, Aug 17, 2017 at 07:02:14PM -0500, Peter Bergner wrote:
>>> This is also broken in GCC 7, GCC 6 and GCC 5.  Ok for those after this
>>> has been on trunk for a little while and assuming testing passes?
>>
>> Okay for trunk and all branches.  Thanks!
> 
> Ok, committed to trunk and the back port bootstraps/regtesting
> are running.  If they're clean, I'll commit this sometime next
> week after the trunk version has baked a little bit.

...testing was clean, so now committed to the open release branches.
Thanks.

Peter

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH, rs6000] Fix PR target/80210: ICE in extract_insn
  2017-08-18  6:39 [PATCH, rs6000] Fix PR target/80210: ICE in extract_insn Peter Bergner
  2017-08-19  0:04 ` Segher Boessenkool
@ 2017-09-08 19:11 ` Andreas Schwab
  2017-09-08 22:30   ` Peter Bergner
  1 sibling, 1 reply; 8+ messages in thread
From: Andreas Schwab @ 2017-09-08 19:11 UTC (permalink / raw)
  To: Peter Bergner
  Cc: GCC Patches, Segher Boessenkool, Bill Schmidt, Michael Meissner

Executing on host: /daten/gcc/gcc-20170907/Build/gcc/xgcc -B/daten/gcc/gcc-20170907/Build/gcc/ /daten/gcc/gcc-20170907/gcc/testsuite/gcc.target/powerpc/pr80125.c  -m32    -fno-diagnostics-show-caret -fdiagnostics-color=never   -O2 -maltivec -S -o pr80125.s    (timeout = 300)

FAIL: gcc.target/powerpc/pr80210.c (internal compiler error)
FAIL: gcc.target/powerpc/pr80210.c (test for excess errors)
Excess errors:
/daten/gcc/gcc-20170907/gcc/testsuite/gcc.target/powerpc/pr80210.c:9:1: error: unrecognizable insn:
(insn 6 3 7 2 (set (reg:DF 121 [ <retval> ])
        (sqrt:DF (reg/v:DF 122 [ a ]))) "/daten/gcc/gcc-20170907/gcc/testsuite/gcc.target/powerpc/pr80210.c":8 -1
     (nil))
during RTL pass: vregs
/daten/gcc/gcc-20170907/gcc/testsuite/gcc.target/powerpc/pr80210.c:9:1: internal compiler error: in extract_insn, at recog.c:2306
0x1013f86f _fatal_insn(char const*, rtx_def const*, char const*, int, char const*)
	../../gcc/rtl-error.c:108
0x1013f8b3 _fatal_insn_not_found(rtx_def const*, char const*, int, char const*)
	../../gcc/rtl-error.c:116
0x106926fb extract_insn(rtx_insn*)
	../../gcc/recog.c:2306
0x1043e5f7 instantiate_virtual_regs_in_insn
	../../gcc/function.c:1591
0x1043e5f7 instantiate_virtual_regs
	../../gcc/function.c:1959
0x1043e5f7 execute
	../../gcc/function.c:2008

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH, rs6000] Fix PR target/80210: ICE in extract_insn
  2017-09-08 19:11 ` Andreas Schwab
@ 2017-09-08 22:30   ` Peter Bergner
  2017-09-09  8:45     ` Andreas Schwab
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Bergner @ 2017-09-08 22:30 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: GCC Patches, Segher Boessenkool, Bill Schmidt, Michael Meissner,
	David Edelsohn

On 9/8/17 2:11 PM, Andreas Schwab wrote:
> Executing on host: /daten/gcc/gcc-20170907/Build/gcc/xgcc -B/daten/gcc/gcc-20170907/Build/gcc/ /daten/gcc/gcc-20170907/gcc/testsuite/gcc.target/powerpc/pr80125.c  -m32    -fno-diagnostics-show-caret -fdiagnostics-color=never   -O2 -maltivec -S -o pr80125.s    (timeout = 300)
> 
> FAIL: gcc.target/powerpc/pr80210.c (internal compiler error)
> FAIL: gcc.target/powerpc/pr80210.c (test for excess errors)

David mentioned this to me earlier that he saw the same thing on AIX.
I didn't see this on my BE builds, because my build scripts were
using the --with-cpu=... configure command and using any -mcpu=...
option will work around the bug.  The bug isn't in my patch, since
the ICE existed before my patch, we just didn't have the test case
to notice before.  

The problem is due to a mismatch between TARGET_DEFAULT, which contains
MASK_PPC_GPOPT and the ISA flags for the default "powerpc64" cpu, which
does not contain MASK_PPC_GPOPT and how rs6000_option_override_internal()
decides which one to use.  The failure scenario is:

Early on, we call init_all_optabs() which setups up a table which
describes which patterns that generate some HW insns are "valid".
Before we call init_all_optabs(), rs6000_option_override_internal()
gets called with global_init_p arg set to "true" and we basically
set rs6000_isa_flags to TARGET_DEFAULT.  This is because we do not
have a -mcpu= value nor do we have an "implicit_cpu", which forces
us to use TARGET_DEFAULT.  With this, init_all_optabs() thinks we
can generate a HW sqrt, so it enables generating its pattern.

Later, after we've scanned the entire file, we go to expand our
function into RTL and we reset our compiler options and we end
up calling rs6000_option_override_internal() again, but with
global_init_p arg now false and we encounter this code:

  struct cl_target_option *main_target_opt
    = ((global_init_p || target_option_default_node == NULL)
       ? NULL : TREE_TARGET_OPTION (target_option_default_node));

This ends up setting main_target_opt to a non-NULL value, then:

  ...
  else if (main_target_opt != NULL && main_target_opt->x_rs6000_cpu_index >= 0)
    {
      rs6000_cpu_index = cpu_index = main_target_opt->x_rs6000_cpu_index;
      have_cpu = true;
    }

So now we act as if the user explicitly passed in a -mcpu= option, then:

  ...
  /* If we have a cpu, either through an explicit -mcpu=<xxx> or if the
     compiler was configured with --with-cpu=<xxx>, replace all of the ISA bits
     with those from the cpu, except for options that were explicitly set.  If
     we don't have a cpu, do not override the target bits set in
     TARGET_DEFAULT.  */
  if (have_cpu)
    {
      rs6000_isa_flags &= ~set_masks;
      rs6000_isa_flags |= (processor_target_table[cpu_index].target_enable
                           & set_masks);
    }
  else
    {
      /* If no -mcpu=<xxx>, inherit any default options that were cleared via
         POWERPC_MASKS.  Originally, TARGET_DEFAULT was used to initialize
         target_flags via the TARGET_DEFAULT_TARGET_FLAGS hook.  When we switched
         to using rs6000_isa_flags, we need to do the initialization here.

         If there is a TARGET_DEFAULT, use that.  Otherwise fall back to using
         -mcpu=powerpc, -mcpu=powerpc64, or -mcpu=powerpc64le defaults.  */
      HOST_WIDE_INT flags = ((TARGET_DEFAULT) ? TARGET_DEFAULT
                             : processor_target_table[cpu_index].target_enable);
      rs6000_isa_flags |= (flags & ~rs6000_isa_flags_explicit);
    }

So the first time through here with global_init_p == true, have_cpu is set to
false and we get TARGET_DEFAULT.  The next time we come here, global_init_p
== false and we set have_cpu to true because main_target_opt is non-NULL and
the cpu_index value is set to "powerpc64" (for -m64 compiles) or "powerpc"
(for -m32 compiles).  This causes us to now grab the ISA flags from:

  processor_target_table[cpu_index].target_enable

instead of from TARGET_DEFAULT and neither "powerpc64" or "powerpc" contain
the MASK_PPC_GPOPT flag, which leads us to ICE because the optabs allows us
to generate the HW sqrt pattern, but our ISA flags don't allow it.

This doesn't affect LE builds, because it has a TARGET_DEFAULT value that matches
the "powerpc64l" default masks.  We also enforce passing a -mcpu=power8 option
when the user doesn't explicitly use one, so again, not a problem.

This also doesn't affect --target=powerpc-linux builds or --target=powerpc64-linux
builds that default to 32-bit binaries, because we use a value of TARGET_DEFAULT == 0
(for both -m32 and -m64), so the first time through rs6000_option_override_internal(),
we end up using processor_target_table[cpu_index].target_enable right from the beginning.

The following patch fixes the problem I saw on Linux and bootstraps and regtests
with no regressions on LE and BE (running testsuite in both 32-bit and 64-bit
modes).  I was waiting to submit this until David had a chance to verify this
fixes the problem on AIX.

Peter



Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 251386)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -3993,34 +3993,19 @@ rs6000_option_override_internal (bool gl
     }
   else if (main_target_opt != NULL && main_target_opt->x_rs6000_cpu_index >= 0)
     {
-      rs6000_cpu_index = cpu_index = main_target_opt->x_rs6000_cpu_index;
+      cpu_index = main_target_opt->x_rs6000_cpu_index;
       have_cpu = true;
     }
   else if (implicit_cpu)
     {
-      rs6000_cpu_index = cpu_index = rs6000_cpu_name_lookup (implicit_cpu);
+      cpu_index = rs6000_cpu_name_lookup (implicit_cpu);
       have_cpu = true;
     }
-  else
-    {
-      /* PowerPC 64-bit LE requires at least ISA 2.07.  */
-      const char *default_cpu = ((!TARGET_POWERPC64)
-				 ? "powerpc"
-				 : ((BYTES_BIG_ENDIAN)
-				    ? "powerpc64"
-				    : "powerpc64le"));
-
-      rs6000_cpu_index = cpu_index = rs6000_cpu_name_lookup (default_cpu);
-      have_cpu = false;
-    }
-
-  gcc_assert (cpu_index >= 0);
 
   if (have_cpu)
     {
 #ifndef HAVE_AS_POWER9
-      if (processor_target_table[rs6000_cpu_index].processor 
-	  == PROCESSOR_POWER9)
+      if (processor_target_table[cpu_index].processor == PROCESSOR_POWER9)
 	{
 	  have_cpu = false;
 	  warning (0, "will not generate power9 instructions because "
@@ -4028,8 +4013,7 @@ rs6000_option_override_internal (bool gl
 	}
 #endif
 #ifndef HAVE_AS_POWER8
-      if (processor_target_table[rs6000_cpu_index].processor
-	  == PROCESSOR_POWER8)
+      if (processor_target_table[cpu_index].processor == PROCESSOR_POWER8)
 	{
 	  have_cpu = false;
 	  warning (0, "will not generate power8 instructions because "
@@ -4037,8 +4021,7 @@ rs6000_option_override_internal (bool gl
 	}
 #endif
 #ifndef HAVE_AS_POPCNTD
-      if (processor_target_table[rs6000_cpu_index].processor
-	  == PROCESSOR_POWER7)
+      if (processor_target_table[cpu_index].processor == PROCESSOR_POWER7)
 	{
 	  have_cpu = false;
 	  warning (0, "will not generate power7 instructions because "
@@ -4046,8 +4029,7 @@ rs6000_option_override_internal (bool gl
 	}
 #endif
 #ifndef HAVE_AS_DFP
-      if (processor_target_table[rs6000_cpu_index].processor
-	  == PROCESSOR_POWER6)
+      if (processor_target_table[cpu_index].processor == PROCESSOR_POWER6)
 	{
 	  have_cpu = false;
 	  warning (0, "will not generate power6 instructions because "
@@ -4055,26 +4037,13 @@ rs6000_option_override_internal (bool gl
 	}
 #endif
 #ifndef HAVE_AS_POPCNTB
-      if (processor_target_table[rs6000_cpu_index].processor
-	  == PROCESSOR_POWER5)
+      if (processor_target_table[cpu_index].processor == PROCESSOR_POWER5)
 	{
 	  have_cpu = false;
 	  warning (0, "will not generate power5 instructions because "
 		   "assembler lacks power5 support");
 	}
 #endif
-
-      if (!have_cpu)
-	{
-	  /* PowerPC 64-bit LE requires at least ISA 2.07.  */
-	  const char *default_cpu = (!TARGET_POWERPC64
-				     ? "powerpc"
-				     : (BYTES_BIG_ENDIAN
-					? "powerpc64"
-					: "powerpc64le"));
-
-	  rs6000_cpu_index = cpu_index = rs6000_cpu_name_lookup (default_cpu);
-	}
     }
 
   /* If we have a cpu, either through an explicit -mcpu=<xxx> or if the
@@ -4084,6 +4053,7 @@ rs6000_option_override_internal (bool gl
      TARGET_DEFAULT.  */
   if (have_cpu)
     {
+      rs6000_cpu_index = cpu_index;
       rs6000_isa_flags &= ~set_masks;
       rs6000_isa_flags |= (processor_target_table[cpu_index].target_enable
 			   & set_masks);
@@ -4097,8 +4067,20 @@ rs6000_option_override_internal (bool gl
 
 	 If there is a TARGET_DEFAULT, use that.  Otherwise fall back to using
 	 -mcpu=powerpc, -mcpu=powerpc64, or -mcpu=powerpc64le defaults.  */
-      HOST_WIDE_INT flags = ((TARGET_DEFAULT) ? TARGET_DEFAULT
-			     : processor_target_table[cpu_index].target_enable);
+      HOST_WIDE_INT flags;
+      if (TARGET_DEFAULT)
+	flags = TARGET_DEFAULT;
+      else
+	{
+	  /* PowerPC 64-bit LE requires at least ISA 2.07.  */
+	  const char *default_cpu = ((!TARGET_POWERPC64)
+				     ? "powerpc"
+				     : ((BYTES_BIG_ENDIAN)
+					? "powerpc64"
+					: "powerpc64le"));
+	  cpu_index = rs6000_cpu_name_lookup (default_cpu);
+	  flags = processor_target_table[cpu_index].target_enable;
+	}
       rs6000_isa_flags |= (flags & ~rs6000_isa_flags_explicit);
     }
 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH, rs6000] Fix PR target/80210: ICE in extract_insn
  2017-09-08 22:30   ` Peter Bergner
@ 2017-09-09  8:45     ` Andreas Schwab
  2017-09-12 18:27       ` Peter Bergner
  0 siblings, 1 reply; 8+ messages in thread
From: Andreas Schwab @ 2017-09-09  8:45 UTC (permalink / raw)
  To: Peter Bergner
  Cc: GCC Patches, Segher Boessenkool, Bill Schmidt, Michael Meissner,
	David Edelsohn

On Sep 08 2017, Peter Bergner <bergner@vnet.ibm.com> wrote:

> The following patch fixes the problem I saw on Linux and bootstraps and regtests
> with no regressions on LE and BE (running testsuite in both 32-bit and 64-bit
> modes).  I was waiting to submit this until David had a chance to verify this
> fixes the problem on AIX.

I've verified that this fixes the ICE.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH, rs6000] Fix PR target/80210: ICE in extract_insn
  2017-09-09  8:45     ` Andreas Schwab
@ 2017-09-12 18:27       ` Peter Bergner
  0 siblings, 0 replies; 8+ messages in thread
From: Peter Bergner @ 2017-09-12 18:27 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: GCC Patches, Segher Boessenkool, Bill Schmidt, Michael Meissner,
	David Edelsohn

On 9/9/17 3:44 AM, Andreas Schwab wrote:
> On Sep 08 2017, Peter Bergner <bergner@vnet.ibm.com> wrote:
> 
>> The following patch fixes the problem I saw on Linux and bootstraps and regtests
>> with no regressions on LE and BE (running testsuite in both 32-bit and 64-bit
>> modes).  I was waiting to submit this until David had a chance to verify this
>> fixes the problem on AIX.
> 
> I've verified that this fixes the ICE.

So it seems this is only a partial fix.  If I modify the test case to
move the #pragma to the start of the file, we still ICE with the same
error.

Clearly, we're not disabling the generation of the HW sqrt in the
optabs when we should.  Basically, this is the opposite problem
than the pr80210.c test case.

Peter


bergner@bns:~/gcc/BUGS/PR80210> cat no-sqrt.i
#pragma GCC target "no-powerpc-gpopt"
double
foo (double a)
{
  return __builtin_sqrt (a);
}
bergner@bns:~/gcc/BUGS/PR80210>
/home/bergner/gcc/build/gcc-fsf-mainline-pr80210-64/gcc/xgcc
-B/home/bergner/gcc/build/gcc-fsf-mainline-pr80210-64/gcc -O2 -S no-sqrt.i
no-sqrt.i: In function ‘foo’:
no-sqrt.i:6:1: error: unrecognizable insn:
 }
 ^
(insn 6 3 7 2 (set (reg:DF 121 [ <retval> ])
        (sqrt:DF (reg/v:DF 122 [ a ]))) "no-sqrt.i":5 -1
     (nil))
during RTL pass: vregs
no-sqrt.i:6:1: internal compiler error: in extract_insn, at recog.c:2311

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-09-12 18:27 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-18  6:39 [PATCH, rs6000] Fix PR target/80210: ICE in extract_insn Peter Bergner
2017-08-19  0:04 ` Segher Boessenkool
2017-08-20  5:12   ` Peter Bergner
2017-08-23 15:58     ` Peter Bergner
2017-09-08 19:11 ` Andreas Schwab
2017-09-08 22:30   ` Peter Bergner
2017-09-09  8:45     ` Andreas Schwab
2017-09-12 18:27       ` Peter Bergner

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).