Patchwork Fix PR56131 - gcc.dg/pr56035.c ICEs gcc on sparc-linux

login
register
mail settings
Submitter Tom de Vries
Date Feb. 4, 2013, 5:30 p.m.
Message ID <510FF01D.5040309@mentor.com>
Download mbox | patch
Permalink /patch/218003/
State New
Headers show

Comments

Tom de Vries - Feb. 4, 2013, 5:30 p.m.
Eric,

This patch fixes PR56131.

The problem is that in delete_insn, while deleting an undeletable label (in
other words, transforming a label into a INSN_NOTE_DELETED_LABEL):
- we try to find the bb of a NOTE_INSN_BASIC_BLOCK following the label using
  BLOCK_FOR_INSN (which is NULL) instead of NOTE_BASIC_BLOCK (which is not
  NULL), and
- we don't handle the case that we find a NULL pointer for that bb gracefully.

This comment in insn-notes.def tells us that the bb info could be in either of
the two:
...
/* Record the struct for the following basic block.  Uses
   NOTE_BASIC_BLOCK.  FIXME: Redundant with the basic block pointer
   now included in every insn.  */
INSN_NOTE (BASIC_BLOCK)
...

The patch fixes the problems by:
- using NOTE_BASIC_BLOCK to find the bb of NOTE_INSN_BASIC_BLOCK, and
- explicitly handling the cases that the bb of either the label or the note is
  NULL.

Bootstrapped and reg-tested by Mikael Pettersson on both x86_64-linux and
sparc64-linux.

OK for trunk?

Thanks,
- Tom

2013-02-04  Tom de Vries  <tom@codesourcery.com>

	PR rtl-optimization/56131
	* cfgrtl.c (delete_insn): Use NOTE_BASIC_BLOCK instead of BLOCK_FOR_INSN
	to get the bb of a NOTE_INSN_BASIC_BLOCK.  Handle the case that the bb
	of the label is NULL.  Add comment.
Eric Botcazou - Feb. 5, 2013, 9:02 a.m.
> The problem is that in delete_insn, while deleting an undeletable label (in
> other words, transforming a label into a INSN_NOTE_DELETED_LABEL):
> - we try to find the bb of a NOTE_INSN_BASIC_BLOCK following the label using
> BLOCK_FOR_INSN (which is NULL) instead of NOTE_BASIC_BLOCK (which is not
> NULL), and
> - we don't handle the case that we find a NULL pointer for that bb
> gracefully.

Can the NOTE_BASIC_BLOCK of a NOTE_INSN_BASIC_BLOCK really be NULL?

> This comment in insn-notes.def tells us that the bb info could be in either
> of the two:
> ...
> /* Record the struct for the following basic block.  Uses
>    NOTE_BASIC_BLOCK.  FIXME: Redundant with the basic block pointer
>    now included in every insn.  */
> INSN_NOTE (BASIC_BLOCK)
> ...
> 
> The patch fixes the problems by:
> - using NOTE_BASIC_BLOCK to find the bb of NOTE_INSN_BASIC_BLOCK, and
> - explicitly handling the cases that the bb of either the label or the note
> is NULL.

I don't think that we need to handle the very last case.

> 2013-02-04  Tom de Vries  <tom@codesourcery.com>
> 
> 	PR rtl-optimization/56131
> 	* cfgrtl.c (delete_insn): Use NOTE_BASIC_BLOCK instead of BLOCK_FOR_INSN
> 	to get the bb of a NOTE_INSN_BASIC_BLOCK.  Handle the case that the bb
> 	of the label is NULL.  Add comment.

OK on principle, but remove the handling of the NULL NOTE_BASIC_BLOCK and keep 
everything in a single condition.

Patch

Index: gcc/cfgrtl.c
===================================================================
--- gcc/cfgrtl.c (revision 195240)
+++ gcc/cfgrtl.c (working copy)
@@ -135,7 +135,7 @@  delete_insn (rtx insn)
       if (! can_delete_label_p (insn))
 	{
 	  const char *name = LABEL_NAME (insn);
-	  basic_block bb = BLOCK_FOR_INSN (insn);
+	  basic_block bb = NULL, label_bb = BLOCK_FOR_INSN (insn);
 	  rtx bb_note = NEXT_INSN (insn);
 
 	  really_delete = false;
@@ -143,8 +143,16 @@  delete_insn (rtx insn)
 	  NOTE_KIND (insn) = NOTE_INSN_DELETED_LABEL;
 	  NOTE_DELETED_LABEL_NAME (insn) = name;
 
-	  if (bb_note != NULL_RTX && NOTE_INSN_BASIC_BLOCK_P (bb_note)
-	      && BLOCK_FOR_INSN (bb_note) == bb)
+	  if (bb_note != NULL_RTX
+	      && NOTE_INSN_BASIC_BLOCK_P (bb_note))
+	    bb = NOTE_BASIC_BLOCK (bb_note);
+
+	  /* If the note following the label starts a basic block, and the
+	     label is a member of the same basic block, interchange the two.
+	     If the label is not marked with a bb, assume it's the same bb.  */
+	  if (bb != NULL
+	      && (bb == label_bb
+		  || label_bb == NULL))
 	    {
 	      reorder_insns_nobb (insn, insn, bb_note);
 	      BB_HEAD (bb) = bb_note;