Treat SEQUENCE specially in mark_jump_label.

Treat SEQUENCE specially in mark_jump_label.

[Bernd Schmidt]

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

I've committed the following patch, which fixes a segfault in the 
Blackfin compiler while building libgcc.

Recently, cfg_layout_finalize has started to call rebuild_jump_labels 
which in turn calls mark_jump_label.  The Blackfin reorg pass uses 
cfg_layout, and mark_jump_label doesn't cope with the SEQUENCE insns we 
generate.

Bootstrapped and regression tested on i686-linux.  I get one additional 
failure:
+FAIL: gcc.dg/noncompile/920923-1.c  -O3 -g  (test for excess errors)
which, from looking at the log, seems to be a dejagnu problem (the 
compiler output appears truncated).


Bernd
-- 
This footer brought to you by insane German lawmakers.
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Registergericht Muenchen HRB 40368
Geschaeftsfuehrer Thomas Wessel, Vincent Roche, Joseph E. McDonough

[-- Attachment #2: jl-sequence.diff --]
[-- Type: text/plain, Size: 798 bytes --]

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 123084)
+++ ChangeLog	(working copy)
@@ -1,3 +1,7 @@
+2007-03-20  Bernd Schmidt  <bernd.schmidt@analog.com>
+
+	* jump.c (mark_jump_labels): Handle SEQUENCE specially.
+
 2007-03-19  Paolo Bonzini  <bonzini@gnu.org>
 
 	PR rtl-optimization/30907
Index: jump.c
===================================================================
--- jump.c	(revision 123084)
+++ jump.c	(working copy)
@@ -992,6 +992,12 @@ mark_jump_label (rtx x, rtx insn, int in
       in_mem = 1;
       break;
 
+    case SEQUENCE:
+      for (i = 0; i < XVECLEN (x, 0); i++)
+	mark_jump_label (PATTERN (XVECEXP (x, 0, i)),
+			 XVECEXP (x, 0, i), 0);
+      return;
+
     case SYMBOL_REF:
       if (!in_mem)
 	return;

Re: Treat SEQUENCE specially in mark_jump_label.

[Steven Bosscher]

On Wednesday 21 March 2007 00:47, Bernd Schmidt wrote:
> I've committed the following patch, which fixes a segfault in the
> Blackfin compiler while building libgcc.
>
> Recently, cfg_layout_finalize has started to call rebuild_jump_labels
> which in turn calls mark_jump_label.  The Blackfin reorg pass uses
> cfg_layout, and mark_jump_label doesn't cope with the SEQUENCE insns we
> generate.
>
> Bootstrapped and regression tested on i686-linux.  I get one additional
> failure:
> +FAIL: gcc.dg/noncompile/920923-1.c  -O3 -g  (test for excess errors)
> which, from looking at the log, seems to be a dejagnu problem (the
> compiler output appears truncated).

This looks like SEQUENCE abuse to me.  SEQUENCE shouldn't be used
anymore, and effort was made to remove it from the compiler entirely,
see e.g. http://gcc.gnu.org/ml/gcc-patches/2002-06/msg00535.html.

You even say in bfin.c that you have to cheat the compiler to make
your SEQUENCE hack work at all:

  /* This is a cheat to avoid emit_insn's special handling of SEQUENCEs.
     Generating a PARALLEL first and changing its code later is the
     easiest way to emit a SEQUENCE insn.  */
  bundle = gen_rtx_PARALLEL (VOIDmode, gen_rtvec (3, slot[0], slot[1], slot[2]));
  emit_insn_before (bundle, slot[0]);
  remove_insn (slot[0]);
  remove_insn (slot[1]);
  remove_insn (slot[2]);
  PUT_CODE (bundle, SEQUENCE);

I see no reason to litter the compiler with code to handle SEQUENCE
again, unless we allow it as a proper citizen again everywhere else,
too.

At the very least you should add a comment in (mark_jump_labels why
this is needed.  Right now it is completely unclear why this is needed
in mark_jump_labels, but not also in init_label_info.  Actually I think
that if this the way we wish to go, then you should add code to handle
SEQUENCE in init_label_info too.

Anyway, I don't like the patch.  Is there really no other way for the
blackfin port to just avoid SEQUENCE completely?

Gr.
Steven

Re: Treat SEQUENCE specially in mark_jump_label.

[Bernd Schmidt]

Steven Bosscher wrote:
> 
> This looks like SEQUENCE abuse to me.  SEQUENCE shouldn't be used
> anymore, and effort was made to remove it from the compiler entirely,

SEQUENCE is still used by reorg.c as far as I can tell.  The Blackfin 
port uses it in a similar manner, generating them just before the final 
pass.

> see e.g. http://gcc.gnu.org/ml/gcc-patches/2002-06/msg00535.html.
> 
> You even say in bfin.c that you have to cheat the compiler to make
> your SEQUENCE hack work at all:

I have to cheat a specific function.  The alternative was to use 
something like reorg's emit_delay_sequence which seemed like overkill.

> Anyway, I don't like the patch.  Is there really no other way for the
> blackfin port to just avoid SEQUENCE completely?

I'm open to suggestions.


Bernd
-- 
This footer brought to you by insane German lawmakers.
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Registergericht Muenchen HRB 40368
Geschaeftsfuehrer Thomas Wessel, Vincent Roche, Joseph E. McDonough