diff mbox

Add .def file for public target instructions

Message ID 878ub6rhmn.fsf@e105548-lin.cambridge.arm.com
State New
Headers show

Commit Message

Richard Sandiford June 26, 2015, 8:36 a.m. UTC
Markus Trippelsdorf <markus@trippelsdorf.de> writes:
> On 2015.06.23 at 19:41 +0100, Richard Sandiford wrote:
>> 
>> gcc/
>> 	* Makefile.in (TARGET_DEF): Add target-insns.def.
>> 	(.PRECIOUS, simple_rtl_generated_h): Add insn-target-def.h.
>> 	(build/gentarget-def.o): New rule.
>> 	(genprogrtl): Add target-def.
>> 	* target-insns.def, gentarget-def.c: New files.
>> 	* target.def: Add targetm.have_* and targetm.gen_* hooks,
>> 	based on the contents of target-insns.def.
>> 	* defaults.h (HAVE_simple_return, gen_simple_return): Delete.
>> 	(HAVE_return, gen_return): Delete.
>> 	* target-def.h: Include insn-target-def.h.
>> 	* cfgrtl.c (force_nonfallthru_and_redirect): Use targetm interface
>> 	instead of direct calls.  Rely on them to do the appropriate assertions.
>> 	* function.c (gen_return_pattern): Likewise.  Return an rtx_insn *.
>> 	(convert_jumps_to_returns): Use targetm interface instead of
>> 	direct calls.
>> 	(thread_prologue_and_epilogue_insns): Likewise.
>> 	* reorg.c (find_end_label, dbr_schedule): Likewise.
>> 	* shrink-wrap.h (SHRINK_WRAPPING_ENABLED): Likewise.
>> 	* shrink-wrap.c (convert_to_simple_return): Likewise.
>> 	(try_shrink_wrapping): Use SHRINK_WRAPPING_ENABLED.
>
> The patch breaks bootstrap on ppc64le. During libgcc configuration:

Yeah, sorry about that.  The problem is due to confusion about whether
the generator should emit a barrier or not.  After the final instruction
in a sequence, it should always be up to the caller to insert any
necessary barriers, just like it is for define_insns, and for define_expands
implemented in C++ with DONE.

I'm testing the following patch.

Thanks,
Richard


gcc/
	* rtl.h (emit): Add an optional boolean parameter to control
	whether barriers are emitted.
	* emit-rtl.c (emit): Likewise.
	* gensupport.c (get_emit_function): Return null rather than "emit".
	* genemit.c (gen_emit_seq): Handle the null return value.
	Don't emit barriers after the final instruction in the sequence.
	* gentarget-def.c (main): Don't emit barriers after the instruction.

Comments

Richard Sandiford June 26, 2015, 10:10 a.m. UTC | #1
Richard Sandiford <richard.sandiford@arm.com> writes:
> Markus Trippelsdorf <markus@trippelsdorf.de> writes:
>> On 2015.06.23 at 19:41 +0100, Richard Sandiford wrote:
>>> 
>>> gcc/
>>> 	* Makefile.in (TARGET_DEF): Add target-insns.def.
>>> 	(.PRECIOUS, simple_rtl_generated_h): Add insn-target-def.h.
>>> 	(build/gentarget-def.o): New rule.
>>> 	(genprogrtl): Add target-def.
>>> 	* target-insns.def, gentarget-def.c: New files.
>>> 	* target.def: Add targetm.have_* and targetm.gen_* hooks,
>>> 	based on the contents of target-insns.def.
>>> 	* defaults.h (HAVE_simple_return, gen_simple_return): Delete.
>>> 	(HAVE_return, gen_return): Delete.
>>> 	* target-def.h: Include insn-target-def.h.
>>> 	* cfgrtl.c (force_nonfallthru_and_redirect): Use targetm interface
>>> 	instead of direct calls.  Rely on them to do the appropriate assertions.
>>> 	* function.c (gen_return_pattern): Likewise.  Return an rtx_insn *.
>>> 	(convert_jumps_to_returns): Use targetm interface instead of
>>> 	direct calls.
>>> 	(thread_prologue_and_epilogue_insns): Likewise.
>>> 	* reorg.c (find_end_label, dbr_schedule): Likewise.
>>> 	* shrink-wrap.h (SHRINK_WRAPPING_ENABLED): Likewise.
>>> 	* shrink-wrap.c (convert_to_simple_return): Likewise.
>>> 	(try_shrink_wrapping): Use SHRINK_WRAPPING_ENABLED.
>>
>> The patch breaks bootstrap on ppc64le. During libgcc configuration:
>
> Yeah, sorry about that.  The problem is due to confusion about whether
> the generator should emit a barrier or not.  After the final instruction
> in a sequence, it should always be up to the caller to insert any
> necessary barriers, just like it is for define_insns, and for define_expands
> implemented in C++ with DONE.
>
> I'm testing the following patch.

Committed after bootstrapping & regression-testing on x86_64-linux-gnu
and testing that Anton's testcase worked on ppc64le.  (The emit-rtl.c
part seemed obvious given the generator changes.)

Sorry again for the breakage.

Thanks,
Richard

> gcc/
> 	* rtl.h (emit): Add an optional boolean parameter to control
> 	whether barriers are emitted.
> 	* emit-rtl.c (emit): Likewise.
> 	* gensupport.c (get_emit_function): Return null rather than "emit".
> 	* genemit.c (gen_emit_seq): Handle the null return value.
> 	Don't emit barriers after the final instruction in the sequence.
> 	* gentarget-def.c (main): Don't emit barriers after the instruction.
>
> Index: gcc/rtl.h
> ===================================================================
> --- gcc/rtl.h	2015-06-26 09:09:10.128602409 +0100
> +++ gcc/rtl.h	2015-06-26 09:09:10.316600233 +0100
> @@ -3494,7 +3494,7 @@ extern void add_insn (rtx_insn *);
>  extern void add_insn_before (rtx, rtx, basic_block);
>  extern void add_insn_after (rtx, rtx, basic_block);
>  extern void remove_insn (rtx);
> -extern rtx_insn *emit (rtx);
> +extern rtx_insn *emit (rtx, bool = true);
>  extern void emit_insn_at_entry (rtx);
>  extern rtx gen_lowpart_SUBREG (machine_mode, rtx);
>  extern rtx gen_const_mem (machine_mode, rtx);
> Index: gcc/emit-rtl.c
> ===================================================================
> --- gcc/emit-rtl.c	2015-06-26 09:09:10.124602455 +0100
> +++ gcc/emit-rtl.c	2015-06-26 09:09:10.316600233 +0100
> @@ -5303,11 +5303,14 @@ set_dst_reg_note (rtx insn, enum reg_not
>    return NULL_RTX;
>  }
>  
> -/* Emit the rtl pattern X as an appropriate kind of insn.
> +/* Emit the rtl pattern X as an appropriate kind of insn.  Also emit a
> +   following barrier if the instruction needs one and if ALLOW_BARRIER_P
> +   is true.
> +
>     If X is a label, it is simply added into the insn chain.  */
>  
>  rtx_insn *
> -emit (rtx x)
> +emit (rtx x, bool allow_barrier_p)
>  {
>    enum rtx_code code = classify_insn (x);
>  
> @@ -5320,7 +5323,8 @@ emit (rtx x)
>      case  JUMP_INSN:
>        {
>  	rtx_insn *insn = emit_jump_insn (x);
> -	if (any_uncondjump_p (insn) || GET_CODE (x) == RETURN)
> +	if (allow_barrier_p
> +	    && (any_uncondjump_p (insn) || GET_CODE (x) == RETURN))
>  	  return emit_barrier ();
>  	return insn;
>        }
> Index: gcc/gensupport.c
> ===================================================================
> --- gcc/gensupport.c	2015-06-26 09:09:10.124602455 +0100
> +++ gcc/gensupport.c	2015-06-26 09:09:10.316600233 +0100
> @@ -2983,7 +2983,9 @@ get_pattern_stats (struct pattern_stats
>  				       stats->max_scratch_opno)) + 1;
>  }
>  
> -/* Return the emit_* function that should be used for pattern X.  */
> +/* Return the emit_* function that should be used for pattern X, or NULL
> +   if we can't pick a particular type at compile time and should instead
> +   fall back to "emit".  */
>  
>  const char *
>  get_emit_function (rtx x)
> @@ -3000,7 +3002,7 @@ get_emit_function (rtx x)
>        return "emit_jump_insn";
>  
>      case UNKNOWN:
> -      return "emit";
> +      return NULL;
>  
>      default:
>        gcc_unreachable ();
> Index: gcc/genemit.c
> ===================================================================
> --- gcc/genemit.c	2015-06-26 09:09:10.124602455 +0100
> +++ gcc/genemit.c	2015-06-26 09:09:10.316600233 +0100
> @@ -277,12 +277,22 @@ gen_emit_seq (rtvec vec, char *used)
>  {
>    for (int i = 0, len = GET_NUM_ELEM (vec); i < len; ++i)
>      {
> +      bool last_p = (i == len - 1);
>        rtx next = RTVEC_ELT (vec, i);
> -      printf ("  %s (", get_emit_function (next));
> -      gen_exp (next, DEFINE_EXPAND, used);
> -      printf (");\n");
> -      if (needs_barrier_p (next))
> -	printf ("  emit_barrier ();");
> +      if (const char *name = get_emit_function (next))
> +	{
> +	  printf ("  %s (", name);
> +	  gen_exp (next, DEFINE_EXPAND, used);
> +	  printf (");\n");
> +	  if (!last_p && needs_barrier_p (next))
> +	    printf ("  emit_barrier ();");
> +	}
> +      else
> +	{
> +	  printf ("  emit (");
> +	  gen_exp (next, DEFINE_EXPAND, used);
> +	  printf (", %s);\n", last_p ? "false" : "true");
> +	}
>      }
>  }
>  
> Index: gcc/gentarget-def.c
> ===================================================================
> --- gcc/gentarget-def.c	2015-06-26 09:09:10.124602455 +0100
> +++ gcc/gentarget-def.c	2015-06-26 09:09:10.316600233 +0100
> @@ -245,7 +245,7 @@ main (int argc, char **argv)
>    printf ("  if (rtx_insn *insn = dyn_cast <rtx_insn *> (x))\n");
>    printf ("    return insn;\n");
>    printf ("  start_sequence ();\n");
> -  printf ("  emit (x);\n");
> +  printf ("  emit (x, false);\n");
>    printf ("  rtx_insn *res = get_insns ();\n");
>    printf ("  end_sequence ();\n");
>    printf ("  return res;\n");
diff mbox

Patch

Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h	2015-06-26 09:09:10.128602409 +0100
+++ gcc/rtl.h	2015-06-26 09:09:10.316600233 +0100
@@ -3494,7 +3494,7 @@  extern void add_insn (rtx_insn *);
 extern void add_insn_before (rtx, rtx, basic_block);
 extern void add_insn_after (rtx, rtx, basic_block);
 extern void remove_insn (rtx);
-extern rtx_insn *emit (rtx);
+extern rtx_insn *emit (rtx, bool = true);
 extern void emit_insn_at_entry (rtx);
 extern rtx gen_lowpart_SUBREG (machine_mode, rtx);
 extern rtx gen_const_mem (machine_mode, rtx);
Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c	2015-06-26 09:09:10.124602455 +0100
+++ gcc/emit-rtl.c	2015-06-26 09:09:10.316600233 +0100
@@ -5303,11 +5303,14 @@  set_dst_reg_note (rtx insn, enum reg_not
   return NULL_RTX;
 }
 
-/* Emit the rtl pattern X as an appropriate kind of insn.
+/* Emit the rtl pattern X as an appropriate kind of insn.  Also emit a
+   following barrier if the instruction needs one and if ALLOW_BARRIER_P
+   is true.
+
    If X is a label, it is simply added into the insn chain.  */
 
 rtx_insn *
-emit (rtx x)
+emit (rtx x, bool allow_barrier_p)
 {
   enum rtx_code code = classify_insn (x);
 
@@ -5320,7 +5323,8 @@  emit (rtx x)
     case  JUMP_INSN:
       {
 	rtx_insn *insn = emit_jump_insn (x);
-	if (any_uncondjump_p (insn) || GET_CODE (x) == RETURN)
+	if (allow_barrier_p
+	    && (any_uncondjump_p (insn) || GET_CODE (x) == RETURN))
 	  return emit_barrier ();
 	return insn;
       }
Index: gcc/gensupport.c
===================================================================
--- gcc/gensupport.c	2015-06-26 09:09:10.124602455 +0100
+++ gcc/gensupport.c	2015-06-26 09:09:10.316600233 +0100
@@ -2983,7 +2983,9 @@  get_pattern_stats (struct pattern_stats
 				       stats->max_scratch_opno)) + 1;
 }
 
-/* Return the emit_* function that should be used for pattern X.  */
+/* Return the emit_* function that should be used for pattern X, or NULL
+   if we can't pick a particular type at compile time and should instead
+   fall back to "emit".  */
 
 const char *
 get_emit_function (rtx x)
@@ -3000,7 +3002,7 @@  get_emit_function (rtx x)
       return "emit_jump_insn";
 
     case UNKNOWN:
-      return "emit";
+      return NULL;
 
     default:
       gcc_unreachable ();
Index: gcc/genemit.c
===================================================================
--- gcc/genemit.c	2015-06-26 09:09:10.124602455 +0100
+++ gcc/genemit.c	2015-06-26 09:09:10.316600233 +0100
@@ -277,12 +277,22 @@  gen_emit_seq (rtvec vec, char *used)
 {
   for (int i = 0, len = GET_NUM_ELEM (vec); i < len; ++i)
     {
+      bool last_p = (i == len - 1);
       rtx next = RTVEC_ELT (vec, i);
-      printf ("  %s (", get_emit_function (next));
-      gen_exp (next, DEFINE_EXPAND, used);
-      printf (");\n");
-      if (needs_barrier_p (next))
-	printf ("  emit_barrier ();");
+      if (const char *name = get_emit_function (next))
+	{
+	  printf ("  %s (", name);
+	  gen_exp (next, DEFINE_EXPAND, used);
+	  printf (");\n");
+	  if (!last_p && needs_barrier_p (next))
+	    printf ("  emit_barrier ();");
+	}
+      else
+	{
+	  printf ("  emit (");
+	  gen_exp (next, DEFINE_EXPAND, used);
+	  printf (", %s);\n", last_p ? "false" : "true");
+	}
     }
 }
 
Index: gcc/gentarget-def.c
===================================================================
--- gcc/gentarget-def.c	2015-06-26 09:09:10.124602455 +0100
+++ gcc/gentarget-def.c	2015-06-26 09:09:10.316600233 +0100
@@ -245,7 +245,7 @@  main (int argc, char **argv)
   printf ("  if (rtx_insn *insn = dyn_cast <rtx_insn *> (x))\n");
   printf ("    return insn;\n");
   printf ("  start_sequence ();\n");
-  printf ("  emit (x);\n");
+  printf ("  emit (x, false);\n");
   printf ("  rtx_insn *res = get_insns ();\n");
   printf ("  end_sequence ();\n");
   printf ("  return res;\n");