[committed] Use target-insns.def for prologue & epilogue insns

[committed] Use target-insns.def for prologue & epilogue insns

[Richard Sandiford]

Bootstrapped & regression-tested on x86_64-linux-gnu and aarch64-linux-gnu.
Also tested via config-list.mk.  Committed as preapproved.

Thanks,
Richard


gcc/
	* defaults.h (HAVE_epilogue, gen_epilogue): Delete.
	* target-insns.def (epilogue, prologue, sibcall_prologue): New
	targetm instruction patterns.
	* alias.c (init_alias_analysis): Use them instead of HAVE_*/gen_*
	interface.
	* calls.c (expand_call): Likewise.
	* cfgrtl.c (cfg_layout_finalize): Likewise.
	* df-scan.c (df_get_entry_block_def_set): Likewise.
	(df_get_exit_block_use_set): Likewise.
	* dwarf2cfi.c (pass_dwarf2_frame::gate): Likewise.
	* final.c (final_start_function): Likewise.
	* function.c (thread_prologue_and_epilogue_insns): Likewise.
	(reposition_prologue_and_epilogue_notes): Likewise.
	* reorg.c (find_end_label): Likewise.
	* toplev.c (process_options): Likewise.

Index: gcc/defaults.h
===================================================================
--- gcc/defaults.h	2015-06-30 21:54:23.984536147 +0100
+++ gcc/defaults.h	2015-06-30 21:54:23.972536284 +0100
@@ -1426,16 +1426,6 @@ #define STACK_CHECK_MAX_VAR_SIZE (STACK_
 #define TARGET_VTABLE_USES_DESCRIPTORS 0
 #endif
 
-#ifndef HAVE_epilogue
-#define HAVE_epilogue 0
-static inline rtx
-gen_epilogue ()
-{
-  gcc_unreachable ();
-  return NULL;
-}
-#endif
-
 #ifndef HAVE_mem_thread_fence
 #define HAVE_mem_thread_fence 0
 static inline rtx
Index: gcc/target-insns.def
===================================================================
--- gcc/target-insns.def	2015-06-30 21:54:23.984536147 +0100
+++ gcc/target-insns.def	2015-06-30 21:54:23.976536238 +0100
@@ -30,6 +30,9 @@
    Patterns that take no operands should have a prototype "(void)".
 
    Instructions should be documented in md.texi rather than here.  */
+DEF_TARGET_INSN (canonicalize_funcptr_for_compare, (rtx x0, rtx x1))
+DEF_TARGET_INSN (epilogue, (void))
+DEF_TARGET_INSN (prologue, (void))
 DEF_TARGET_INSN (return, (void))
+DEF_TARGET_INSN (sibcall_epilogue, (void))
 DEF_TARGET_INSN (simple_return, (void))
-DEF_TARGET_INSN (canonicalize_funcptr_for_compare, (rtx x0, rtx x1))
Index: gcc/alias.c
===================================================================
--- gcc/alias.c	2015-06-30 21:54:23.984536147 +0100
+++ gcc/alias.c	2015-06-30 21:54:23.972536284 +0100
@@ -3038,6 +3038,14 @@ init_alias_analysis (void)
   rpo = XNEWVEC (int, n_basic_blocks_for_fn (cfun));
   rpo_cnt = pre_and_rev_post_order_compute (NULL, rpo, false);
 
+  /* The prologue/epilogue insns are not threaded onto the
+     insn chain until after reload has completed.  Thus,
+     there is no sense wasting time checking if INSN is in
+     the prologue/epilogue until after reload has completed.  */
+  bool could_be_prologue_epilogue = ((targetm.have_prologue ()
+				      || targetm.have_epilogue ())
+				     && reload_completed);
+
   pass = 0;
   do
     {
@@ -3076,17 +3084,7 @@ init_alias_analysis (void)
 		{
 		  rtx note, set;
 
-#if defined (HAVE_prologue)
-		  static const bool prologue = true;
-#else
-		  static const bool prologue = false;
-#endif
-
-		  /* The prologue/epilogue insns are not threaded onto the
-		     insn chain until after reload has completed.  Thus,
-		     there is no sense wasting time checking if INSN is in
-		     the prologue/epilogue until after reload has completed.  */
-		  if ((prologue || HAVE_epilogue) && reload_completed
+		  if (could_be_prologue_epilogue
 		      && prologue_epilogue_contains (insn))
 		    continue;
 
Index: gcc/calls.c
===================================================================
--- gcc/calls.c	2015-06-30 21:54:23.984536147 +0100
+++ gcc/calls.c	2015-06-30 21:54:23.972536284 +0100
@@ -2783,13 +2783,8 @@ expand_call (tree exp, rtx target, int i
     try_tail_call = 0;
 
   /*  Rest of purposes for tail call optimizations to fail.  */
-  if (
-#ifdef HAVE_sibcall_epilogue
-      !HAVE_sibcall_epilogue
-#else
-      1
-#endif
-      || !try_tail_call
+  if (!try_tail_call
+      || !targetm.have_sibcall_epilogue ()
       /* Doing sibling call optimization needs some work, since
 	 structure_value_addr can be allocated on the stack.
 	 It does not seem worth the effort since few optimizable
Index: gcc/cfgrtl.c
===================================================================
--- gcc/cfgrtl.c	2015-06-30 21:54:23.984536147 +0100
+++ gcc/cfgrtl.c	2015-06-30 21:54:23.972536284 +0100
@@ -4324,7 +4324,7 @@ cfg_layout_finalize (void)
 #endif
   force_one_exit_fallthru ();
   rtl_register_cfg_hooks ();
-  if (reload_completed && !HAVE_epilogue)
+  if (reload_completed && !targetm.have_epilogue ())
     fixup_fallthru_exit_predecessor ();
   fixup_reorder_chain ();
 
Index: gcc/df-scan.c
===================================================================
--- gcc/df-scan.c	2015-06-30 21:54:23.984536147 +0100
+++ gcc/df-scan.c	2015-06-30 21:54:23.976536238 +0100
@@ -52,13 +52,6 @@ Software Foundation; either version 3, o
 typedef struct df_mw_hardreg *df_mw_hardreg_ptr;
 
 
-#ifndef HAVE_prologue
-#define HAVE_prologue 0
-#endif
-#ifndef HAVE_sibcall_epilogue
-#define HAVE_sibcall_epilogue 0
-#endif
-
 /* The set of hard registers in eliminables[i].from. */
 
 static HARD_REG_SET elim_reg_set;
@@ -3523,7 +3516,7 @@ df_get_entry_block_def_set (bitmap entry
 
   /* Once the prologue has been generated, all of these registers
      should just show up in the first regular block.  */
-  if (HAVE_prologue && epilogue_completed)
+  if (targetm.have_prologue () && epilogue_completed)
     {
       /* Defs for the callee saved registers are inserted so that the
 	 pushes have some defining location.  */
@@ -3701,7 +3694,7 @@ df_get_exit_block_use_set (bitmap exit_b
     if (global_regs[i] || EPILOGUE_USES (i))
       bitmap_set_bit (exit_block_uses, i);
 
-  if (HAVE_epilogue && epilogue_completed)
+  if (targetm.have_epilogue () && epilogue_completed)
     {
       /* Mark all call-saved registers that we actually used.  */
       for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
@@ -3721,7 +3714,7 @@ df_get_exit_block_use_set (bitmap exit_b
       }
 
 #ifdef EH_RETURN_STACKADJ_RTX
-  if ((!HAVE_epilogue || ! epilogue_completed)
+  if ((!targetm.have_epilogue () || ! epilogue_completed)
       && crtl->calls_eh_return)
     {
       rtx tmp = EH_RETURN_STACKADJ_RTX;
@@ -3731,7 +3724,7 @@ df_get_exit_block_use_set (bitmap exit_b
 #endif
 
 #ifdef EH_RETURN_HANDLER_RTX
-  if ((!HAVE_epilogue || ! epilogue_completed)
+  if ((!targetm.have_epilogue () || ! epilogue_completed)
       && crtl->calls_eh_return)
     {
       rtx tmp = EH_RETURN_HANDLER_RTX;
Index: gcc/dwarf2cfi.c
===================================================================
--- gcc/dwarf2cfi.c	2015-06-30 21:54:23.984536147 +0100
+++ gcc/dwarf2cfi.c	2015-06-30 21:54:23.976536238 +0100
@@ -3476,11 +3476,10 @@ const pass_data pass_data_dwarf2_frame =
 bool
 pass_dwarf2_frame::gate (function *)
 {
-#ifndef HAVE_prologue
   /* Targets which still implement the prologue in assembler text
      cannot use the generic dwarf2 unwinding.  */
-  return false;
-#endif
+  if (!targetm.have_prologue ())
+    return false;
 
   /* ??? What to do for UI_TARGET unwinding?  They might be able to benefit
      from the optimized shrink-wrapping annotations that we will compute.
Index: gcc/final.c
===================================================================
--- gcc/final.c	2015-06-30 21:54:23.984536147 +0100
+++ gcc/final.c	2015-06-30 21:54:23.976536238 +0100
@@ -1803,12 +1803,8 @@ final_start_function (rtx_insn *first, F
      if the profiling code comes after the prologue.  */
   if (targetm.profile_before_prologue () && crtl->profile)
     {
-      if (targetm.asm_out.function_prologue
-	  == default_function_pro_epilogue
-#ifdef HAVE_prologue
-	  && HAVE_prologue
-#endif
-	 )
+      if (targetm.asm_out.function_prologue == default_function_pro_epilogue
+	  && targetm.have_prologue ())
 	{
 	  rtx_insn *insn;
 	  for (insn = first; insn; insn = NEXT_INSN (insn))
@@ -1864,9 +1860,7 @@ final_start_function (rtx_insn *first, F
 
   /* If the machine represents the prologue as RTL, the profiling code must
      be emitted when NOTE_INSN_PROLOGUE_END is scanned.  */
-#ifdef HAVE_prologue
-  if (! HAVE_prologue)
-#endif
+  if (! targetm.have_prologue ())
     profile_after_prologue (file);
 }
 
Index: gcc/function.c
===================================================================
--- gcc/function.c	2015-06-30 21:54:23.984536147 +0100
+++ gcc/function.c	2015-06-30 21:54:23.976536238 +0100
@@ -5864,11 +5864,10 @@ thread_prologue_and_epilogue_insns (void
     }
 
   prologue_seq = NULL;
-#ifdef HAVE_prologue
-  if (HAVE_prologue)
+  if (targetm.have_prologue ())
     {
       start_sequence ();
-      rtx_insn *seq = safe_as_a <rtx_insn *> (gen_prologue ());
+      rtx_insn *seq = targetm.gen_prologue ();
       emit_insn (seq);
 
       /* Insert an explicit USE for the frame pointer
@@ -5890,7 +5889,6 @@ thread_prologue_and_epilogue_insns (void
       end_sequence ();
       set_insn_locations (prologue_seq, prologue_location);
     }
-#endif
 
   bitmap_initialize (&bb_flags, &bitmap_default_obstack);
 
@@ -5995,11 +5993,11 @@ thread_prologue_and_epilogue_insns (void
   if (exit_fallthru_edge == NULL)
     goto epilogue_done;
 
-  if (HAVE_epilogue)
+  if (targetm.have_epilogue ())
     {
       start_sequence ();
       epilogue_end = emit_note (NOTE_INSN_EPILOGUE_BEG);
-      rtx_insn *seq = as_a <rtx_insn *> (gen_epilogue ());
+      rtx_insn *seq = targetm.gen_epilogue ();
       if (seq)
 	emit_jump_insn (seq);
 
@@ -6070,7 +6068,6 @@ thread_prologue_and_epilogue_insns (void
     convert_to_simple_return (entry_edge, orig_entry_edge, bb_flags,
 			      returnjump, unconverted_simple_returns);
 
-#ifdef HAVE_sibcall_epilogue
   /* Emit sibling epilogues before any sibling call sites.  */
   for (ei = ei_start (EXIT_BLOCK_PTR_FOR_FN (cfun)->preds); (e =
 							     ei_safe_edge (ei));
@@ -6078,7 +6075,6 @@ thread_prologue_and_epilogue_insns (void
     {
       basic_block bb = e->src;
       rtx_insn *insn = BB_END (bb);
-      rtx ep_seq;
 
       if (!CALL_P (insn)
 	  || ! SIBLING_CALL_P (insn)
@@ -6090,8 +6086,7 @@ thread_prologue_and_epilogue_insns (void
 	  continue;
 	}
 
-      ep_seq = gen_sibcall_epilogue ();
-      if (ep_seq)
+      if (rtx_insn *ep_seq = targetm.gen_sibcall_epilogue ())
 	{
 	  start_sequence ();
 	  emit_note (NOTE_INSN_EPILOGUE_BEG);
@@ -6109,7 +6104,6 @@ thread_prologue_and_epilogue_insns (void
 	}
       ei_next (&ei);
     }
-#endif
 
   if (epilogue_end)
     {
@@ -6143,10 +6137,10 @@ thread_prologue_and_epilogue_insns (void
 void
 reposition_prologue_and_epilogue_notes (void)
 {
-#if ! defined (HAVE_prologue) && ! defined (HAVE_sibcall_epilogue)
-  if (!HAVE_epilogue)
+  if (!targetm.have_prologue ()
+      && !targetm.have_epilogue ()
+      && !targetm.have_sibcall_epilogue ())
     return;
-#endif
 
   /* Since the hash table is created on demand, the fact that it is
      non-null is a signal that it is non-empty.  */
Index: gcc/reorg.c
===================================================================
--- gcc/reorg.c	2015-06-30 21:54:23.984536147 +0100
+++ gcc/reorg.c	2015-06-30 21:54:23.976536238 +0100
@@ -473,7 +473,7 @@ find_end_label (rtx kind)
 	}
       else
 	{
-	  if (HAVE_epilogue && ! targetm.have_return ())
+	  if (targetm.have_epilogue () && ! targetm.have_return ())
 	    /* The RETURN insn has its delay slot filled so we cannot
 	       emit the label just before it.  Since we already have
 	       an epilogue and cannot emit a new RETURN, we cannot
Index: gcc/toplev.c
===================================================================
--- gcc/toplev.c	2015-06-30 21:54:23.984536147 +0100
+++ gcc/toplev.c	2015-06-30 21:54:23.976536238 +0100
@@ -112,10 +112,6 @@ Software Foundation; either version 3, o
 				   declarations for e.g. AIX 4.x.  */
 #endif
 
-#ifndef HAVE_prologue
-#define HAVE_prologue 0
-#endif
-
 #include <new>
 
 static void general_init (const char *, bool);
@@ -1660,7 +1656,7 @@ process_options (void)
 
  /* Do not use IPA optimizations for register allocation if profiler is active
     or port does not emit prologue and epilogue as RTL.  */
-  if (profile_flag || !HAVE_prologue || !HAVE_epilogue)
+  if (profile_flag || !targetm.have_prologue () || !targetm.have_epilogue ())
     flag_ipa_ra = 0;
 
   /* Enable -Werror=coverage-mismatch when -Werror and -Wno-error

Regressions with "[committed] Use target-insns.def for prologue & epilogue insns"

[Hans-Peter Nilsson]

> From: Richard Sandiford <richard.sandiford@arm.com>
> Date: Tue, 30 Jun 2015 22:55:24 +0200

> Bootstrapped & regression-tested on x86_64-linux-gnu and aarch64-linux-gnu.
> Also tested via config-list.mk.  Committed as preapproved.
> 
> Thanks,
> Richard
> 
> 
> gcc/
>         * defaults.h (HAVE_epilogue, gen_epilogue): Delete.
>         * target-insns.def (epilogue, prologue, sibcall_prologue): New
>         targetm instruction patterns.
>         * alias.c (init_alias_analysis): Use them instead of HAVE_*/gen_*
>         interface.
>         * calls.c (expand_call): Likewise.
>         * cfgrtl.c (cfg_layout_finalize): Likewise.
>         * df-scan.c (df_get_entry_block_def_set): Likewise.
>         (df_get_exit_block_use_set): Likewise.
>         * dwarf2cfi.c (pass_dwarf2_frame::gate): Likewise.
>         * final.c (final_start_function): Likewise.
>         * function.c (thread_prologue_and_epilogue_insns): Likewise.
>         (reposition_prologue_and_epilogue_notes): Likewise.
>         * reorg.c (find_end_label): Likewise.
>         * toplev.c (process_options): Likewise.

I think this one -being the most fitting patch in the range
(225190:225210]- caused this regression for cris-elf:

Running /tmp/hpautotest-gcc1/gcc/gcc/testsuite/gcc.target/cris/torture/cris-torture.exp ...
FAIL: gcc.target/cris/torture/no-pro-epi-1.c   -O3 -g  (internal compiler error)
FAIL: gcc.target/cris/torture/no-pro-epi-1.c   -O3 -g  (test for excess errors)

This test checks that the -mno-prologue-epilogue option works,
whose semantics is supposedly self-explanatory.

gcc.log:
Executing on host: /tmp/hpautotest-gcc1/cris-elf/gccobj/gcc/xgcc -B/tmp/hpautotest-gcc1/cris-elf/gccobj/gcc/ /tmp/hpautotest-gcc1/gcc/gcc/testsuite/gcc.target/cris/torture/no-pro-epi-1.c  -fno-diagnostics-show-caret -fdiagnostics-color=never    -O3 -g  -mno-prologue-epilogue -S   -isystem /tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/./newlib/targ-include -isystem /tmp/hpautotest-gcc1/gcc/newlib/libc/include   -o no-pro-epi-1.s    (timeout = 300)
/tmp/hpautotest-gcc1/gcc/gcc/testsuite/gcc.target/cris/torture/no-pro-epi-1.c:4:1: internal compiler error: Segmentation fault
0xa6ba45 crash_signal
	/tmp/hpautotest-gcc1/gcc/gcc/toplev.c:360
0x6b4724 vec<dw_cfi_node*, va_gc, vl_embed>::iterate(unsigned int, dw_cfi_node**) const
	/tmp/hpautotest-gcc1/gcc/gcc/vec.h:755
0x6b4724 output_call_frame_info
	/tmp/hpautotest-gcc1/gcc/gcc/dwarf2out.c:925
0x6b4f36 dwarf2out_frame_finish()
	/tmp/hpautotest-gcc1/gcc/gcc/dwarf2out.c:1177
Please submit a full bug report,

brgds, H-P

Re: Regressions with "[committed] Use target-insns.def for prologue & epilogue insns"

[Richard Sandiford]

Hans-Peter Nilsson <hans-peter.nilsson@axis.com> writes:
>> From: Richard Sandiford <richard.sandiford@arm.com>
>> Date: Tue, 30 Jun 2015 22:55:24 +0200
>
>> Bootstrapped & regression-tested on x86_64-linux-gnu and aarch64-linux-gnu.
>> Also tested via config-list.mk.  Committed as preapproved.
>> 
>> Thanks,
>> Richard
>> 
>> 
>> gcc/
>>         * defaults.h (HAVE_epilogue, gen_epilogue): Delete.
>>         * target-insns.def (epilogue, prologue, sibcall_prologue): New
>>         targetm instruction patterns.
>>         * alias.c (init_alias_analysis): Use them instead of HAVE_*/gen_*
>>         interface.
>>         * calls.c (expand_call): Likewise.
>>         * cfgrtl.c (cfg_layout_finalize): Likewise.
>>         * df-scan.c (df_get_entry_block_def_set): Likewise.
>>         (df_get_exit_block_use_set): Likewise.
>>         * dwarf2cfi.c (pass_dwarf2_frame::gate): Likewise.
>>         * final.c (final_start_function): Likewise.
>>         * function.c (thread_prologue_and_epilogue_insns): Likewise.
>>         (reposition_prologue_and_epilogue_notes): Likewise.
>>         * reorg.c (find_end_label): Likewise.
>>         * toplev.c (process_options): Likewise.
>
> I think this one -being the most fitting patch in the range
> (225190:225210]- caused this regression for cris-elf:
>
> Running
> /tmp/hpautotest-gcc1/gcc/gcc/testsuite/gcc.target/cris/torture/cris-torture.exp
> ...
> FAIL: gcc.target/cris/torture/no-pro-epi-1.c   -O3 -g  (internal compiler error)
> FAIL: gcc.target/cris/torture/no-pro-epi-1.c   -O3 -g  (test for excess errors)
>
> This test checks that the -mno-prologue-epilogue option works,
> whose semantics is supposedly self-explanatory.

Well, yes and no :-)  The crash is coming from the code that outputs
dwarf CFI information.  The code that records this information is skipped
for targets without rtl prologues, with the comment:

  /* Targets which still implement the prologue in assembler text
     cannot use the generic dwarf2 unwinding.  */

That seems accurate.  So what's -mno-prologue-epilogue supposed to do
wrt CFI entries?  Should it output empty entries or none at all?

The first-order reason for the failure is that the code used to be
conditional on #ifndef HAVE_prologue and didn't care what HAVE_prologue
itself evaluated to.  So the condition on the pattern wasn't actually
tested.

Which I suppose leads to the question: does !HAVE_prologue when "prologue"
is defined mean "I know how to output rtl prologues, but the prologue
for this function is empty" or "I'll output the prologue as text rather
than rtl".  I think it logically means the second.  The condition says
whether the pattern can be used; if the pattern can be used but happens
to generate no code then it just outputs no instructions (which is pretty
common for prologues in leaf functions).

The port seems to hedge its bets here.  It has both:

(define_expand "prologue"
  [(const_int 0)]
  "TARGET_PROLOGUE_EPILOGUE"
  "cris_expand_prologue (); DONE;")

and:

void
cris_expand_prologue (void)
{
  [...]
  /* Don't do anything if no prologues or epilogues are wanted.  */
  if (!TARGET_PROLOGUE_EPILOGUE)
    return;

which I guess means that the HAVE_prologue condition wasn't being
consistently tested.  Now that it is: is -mno-prologue-epilogue
just supposed to generate empty prologues and epilogues, as implied
by the cris.c code?  If so then removing the conditions on "prologue"
and "epilogue" should work.  If not, then which of the targetm.have_prologue ()
etc. conditions do you need to be true for -mno-prologue-epilogue?

(You have the distinction of having the only port with conditional
prologue and epilogue patterns. :-))

Thanks,
Richard

Fixed Regressions with "[committed] Use target-insns.def for prologue & epilogue insns"

[Hans-Peter Nilsson]

> From: Richard Sandiford <rdsandiford@googlemail.com>
> Date: Wed, 1 Jul 2015 23:26:59 +0200

> Hans-Peter Nilsson <hans-peter.nilsson@axis.com> writes:
> >> From: Richard Sandiford <richard.sandiford@arm.com>
> >> Date: Tue, 30 Jun 2015 22:55:24 +0200
> >
> >> Bootstrapped & regression-tested on x86_64-linux-gnu and aarch64-linux-gnu.
> >> Also tested via config-list.mk.  Committed as preapproved.
> >> 
> >> Thanks,
> >> Richard
> >> 
> >> 
> >> gcc/
> >>         * defaults.h (HAVE_epilogue, gen_epilogue): Delete.
> >>         * target-insns.def (epilogue, prologue, sibcall_prologue): New
> >>         targetm instruction patterns.
> >>         * alias.c (init_alias_analysis): Use them instead of HAVE_*/gen_*
> >>         interface.
> >>         * calls.c (expand_call): Likewise.
> >>         * cfgrtl.c (cfg_layout_finalize): Likewise.
> >>         * df-scan.c (df_get_entry_block_def_set): Likewise.
> >>         (df_get_exit_block_use_set): Likewise.
> >>         * dwarf2cfi.c (pass_dwarf2_frame::gate): Likewise.
> >>         * final.c (final_start_function): Likewise.
> >>         * function.c (thread_prologue_and_epilogue_insns): Likewise.
> >>         (reposition_prologue_and_epilogue_notes): Likewise.
> >>         * reorg.c (find_end_label): Likewise.
> >>         * toplev.c (process_options): Likewise.
> >
> > I think this one -being the most fitting patch in the range
> > (225190:225210]- caused this regression for cris-elf:
> >
> > Running
> > /tmp/hpautotest-gcc1/gcc/gcc/testsuite/gcc.target/cris/torture/cris-torture.exp
> > ...
> > FAIL: gcc.target/cris/torture/no-pro-epi-1.c   -O3 -g  (internal compiler error)
> > FAIL: gcc.target/cris/torture/no-pro-epi-1.c   -O3 -g  (test for excess errors)
> >
> > This test checks that the -mno-prologue-epilogue option works,
> > whose semantics is supposedly self-explanatory.
> 
> Well, yes and no :-)

Hm...I take that as an affirmation on the regression but perhaps
a "no" to some of the my statements...

>  The crash is coming from the code that outputs
> dwarf CFI information.  The code that records this information is skipped
> for targets without rtl prologues, with the comment:
> 
>   /* Targets which still implement the prologue in assembler text
>      cannot use the generic dwarf2 unwinding.  */
> 
> That seems accurate.  So what's -mno-prologue-epilogue supposed to do
> wrt CFI entries?  Should it output empty entries or none at all?

A big "whatever" on that one.  Debugging and omitting prologue
and epilogue is not something I find reason to spend time on
other than making sure there are no crashes.

> The first-order reason for the failure is that the code used to be
> conditional on #ifndef HAVE_prologue and didn't care what HAVE_prologue
> itself evaluated to.  So the condition on the pattern wasn't actually
> tested.

Not completely true: there was inconsistency between uses of
#ifdef and if (HAVE_prologue).

> Which I suppose leads to the question: does !HAVE_prologue when "prologue"
> is defined mean "I know how to output rtl prologues, but the prologue
> for this function is empty" or "I'll output the prologue as text rather
> than rtl".  I think it logically means the second.

Agreed.

> The condition says
> whether the pattern can be used; if the pattern can be used but happens
> to generate no code then it just outputs no instructions (which is pretty
> common for prologues in leaf functions).
> 
> The port seems to hedge its bets here.  It has both:
> 
> (define_expand "prologue"
>   [(const_int 0)]
>   "TARGET_PROLOGUE_EPILOGUE"
>   "cris_expand_prologue (); DONE;")
> 
> and:
> 
> void
> cris_expand_prologue (void)
> {
>   [...]
>   /* Don't do anything if no prologues or epilogues are wanted.  */
>   if (!TARGET_PROLOGUE_EPILOGUE)
>     return;

Yeah, a visit to the archive supports me thinking this was an
oversight, perhaps caused by the effects of the now fixed
inconsistency.

> which I guess means that the HAVE_prologue condition wasn't being
> consistently tested.  Now that it is: is -mno-prologue-epilogue
> just supposed to generate empty prologues and epilogues, as implied
> by the cris.c code?  If so then removing the conditions on "prologue"
> and "epilogue" should work.  If not, then which of the targetm.have_prologue ()
> etc. conditions do you need to be true for -mno-prologue-epilogue?
> 
> (You have the distinction of having the only port with conditional
> prologue and epilogue patterns. :-))

Not any longer.  Also removed a stale comment.
This committed patch fixes the noted regressions, without
causing further regressions, testing cris-elf in a simulator.

gcc:
	* config/cris/cris.md ("epilogue"): Remove condition.
	("prologue"): Ditto.

Index: config/cris/cris.md
===================================================================
--- config/cris/cris.md	(revision 225286)
+++ config/cris/cris.md	(working copy)
@@ -3518,14 +3518,12 @@ (define_insn "*return_expanded"
 
 (define_expand "prologue"
   [(const_int 0)]
-  "TARGET_PROLOGUE_EPILOGUE"
+  ""
   "cris_expand_prologue (); DONE;")
 
-;; Note that the (return) from the expander itself is always the last
-;; insn in the epilogue.
 (define_expand "epilogue"
   [(const_int 0)]
-  "TARGET_PROLOGUE_EPILOGUE"
+  ""
   "cris_expand_epilogue (); DONE;")
 \f
 ;; Conditional branches.

brgds, H-P

Re: Fixed Regressions with "[committed] Use target-insns.def for prologue & epilogue insns"

[Richard Sandiford]

Hans-Peter Nilsson <hans-peter.nilsson@axis.com> writes:
>> From: Richard Sandiford <rdsandiford@googlemail.com>
>> Date: Wed, 1 Jul 2015 23:26:59 +0200
>
>> Hans-Peter Nilsson <hans-peter.nilsson@axis.com> writes:
>> >> From: Richard Sandiford <richard.sandiford@arm.com>
>> >> Date: Tue, 30 Jun 2015 22:55:24 +0200
>> >
>> >> Bootstrapped & regression-tested on x86_64-linux-gnu and aarch64-linux-gnu.
>> >> Also tested via config-list.mk.  Committed as preapproved.
>> >> 
>> >> Thanks,
>> >> Richard
>> >> 
>> >> 
>> >> gcc/
>> >>         * defaults.h (HAVE_epilogue, gen_epilogue): Delete.
>> >>         * target-insns.def (epilogue, prologue, sibcall_prologue): New
>> >>         targetm instruction patterns.
>> >>         * alias.c (init_alias_analysis): Use them instead of HAVE_*/gen_*
>> >>         interface.
>> >>         * calls.c (expand_call): Likewise.
>> >>         * cfgrtl.c (cfg_layout_finalize): Likewise.
>> >>         * df-scan.c (df_get_entry_block_def_set): Likewise.
>> >>         (df_get_exit_block_use_set): Likewise.
>> >>         * dwarf2cfi.c (pass_dwarf2_frame::gate): Likewise.
>> >>         * final.c (final_start_function): Likewise.
>> >>         * function.c (thread_prologue_and_epilogue_insns): Likewise.
>> >>         (reposition_prologue_and_epilogue_notes): Likewise.
>> >>         * reorg.c (find_end_label): Likewise.
>> >>         * toplev.c (process_options): Likewise.
>> >
>> > I think this one -being the most fitting patch in the range
>> > (225190:225210]- caused this regression for cris-elf:
>> >
>> > Running
>> > /tmp/hpautotest-gcc1/gcc/gcc/testsuite/gcc.target/cris/torture/cris-torture.exp
>> > ...
>> > FAIL: gcc.target/cris/torture/no-pro-epi-1.c -O3 -g (internal
>> > compiler error)
>> > FAIL: gcc.target/cris/torture/no-pro-epi-1.c -O3 -g (test for excess
>> > errors)
>> >
>> > This test checks that the -mno-prologue-epilogue option works,
>> > whose semantics is supposedly self-explanatory.
>> 
>> Well, yes and no :-)
>
> Hm...I take that as an affirmation on the regression but perhaps
> a "no" to some of the my statements...

Just the semantics being self-explanatory.  It wasn't obvious to me
what we were supposed to do with CFI.  "Whatever" works for me though...

>> which I guess means that the HAVE_prologue condition wasn't being
>> consistently tested.  Now that it is: is -mno-prologue-epilogue
>> just supposed to generate empty prologues and epilogues, as implied
>> by the cris.c code?  If so then removing the conditions on "prologue"
>> and "epilogue" should work.  If not, then which of the
>> targetm.have_prologue ()
>> etc. conditions do you need to be true for -mno-prologue-epilogue?
>> 
>> (You have the distinction of having the only port with conditional
>> prologue and epilogue patterns. :-))
>
> Not any longer.  Also removed a stale comment.
> This committed patch fixes the noted regressions, without
> causing further regressions, testing cris-elf in a simulator.
>
> gcc:
> 	* config/cris/cris.md ("epilogue"): Remove condition.
> 	("prologue"): Ditto.

Thanks.

Richard

Re: Fixed Regressions with "[committed] Use target-insns.def for prologue & epilogue insns"

[Hans-Peter Nilsson]

> From: Richard Sandiford <rdsandiford@googlemail.com>
> Date: Thu, 2 Jul 2015 20:58:15 +0200

> Hans-Peter Nilsson <hans-peter.nilsson@axis.com> writes:
> > gcc:
> > 	* config/cris/cris.md ("epilogue"): Remove condition.
> > 	("prologue"): Ditto.
> 
> Thanks.

No, thank *you* for the massive #ifdef HAVE_x -> if (HAVE_x)
cleanup!  Some kind of fallout is practically inevitable.  I
appreciate the initial look even if it's just to punt to the
target maintainer.

brgds, H-P