PR rtl-optimization/88308 Update LABEL_NUSES in move_insn_for_shrink_wrap

Message ID 4ad69547-67df-f989-331c-4111d29fcace@linux.ibm.com
State New
Headers show
Series
  • PR rtl-optimization/88308 Update LABEL_NUSES in move_insn_for_shrink_wrap
Related show

Commit Message

Aaron Sawdey Feb. 13, 2019, 11:08 p.m.
I've tracked pr/88308 down to move_insn_for_shrink_wrap(). This function moves an insn
from one BB to another by copying it and deleting the old one. Unfortunately this does
the LABEL_NUSES count on labels referenced because deleting the old instruction decrements
the count and nothing in this function is incrementing the count.

It just happens that on rs6000 with -m64, force_const_mem() gets called on the address
and that sets LABEL_PRESERVE_P on the label which prevents it from being deleted. For
whatever reason this doesn't happen in a -m32 compilation, and the label and it's associated
jump table data are deleted. This later causes the ICE when the dwarf code tries to look
at the label.

Segher and I came up with 3 possible solutions to this:

1) Don't let move_insn_for_shrink_wrap try to move insns with label_ref in them.
2) Call mark_jump_label() on the copied instruction to fix up the ref counts.
3) Make the function actually move the insn instead of copying/deleting it.

It seemed like option 2 was the best thing for stage 4 as it is not inhibiting anything
and is just doing a fixup of the ref count.

OK for trunk after regtesting on ppc64be (32/64) and x86_64?

Thanks!
   Aaron


2019-02-13  Aaron Sawdey  <acsawdey@linux.ibm.com>

	* shrink-wrap.c (move_insn_for_shrink_wrap): Fix LABEL_NUSES counts
	on copied instruction.




-- 
Aaron Sawdey, Ph.D.  acsawdey@linux.vnet.ibm.com
050-2/C113  (507) 253-7520 home: 507/263-0782
IBM Linux Technology Center - PPC Toolchain

Comments

Richard Biener Feb. 14, 2019, 9:37 a.m. | #1
On Thu, Feb 14, 2019 at 12:08 AM Aaron Sawdey <acsawdey@linux.ibm.com> wrote:
>

> I've tracked pr/88308 down to move_insn_for_shrink_wrap(). This function moves an insn

> from one BB to another by copying it and deleting the old one. Unfortunately this does

> the LABEL_NUSES count on labels referenced because deleting the old instruction decrements

> the count and nothing in this function is incrementing the count.

>

> It just happens that on rs6000 with -m64, force_const_mem() gets called on the address

> and that sets LABEL_PRESERVE_P on the label which prevents it from being deleted. For

> whatever reason this doesn't happen in a -m32 compilation, and the label and it's associated

> jump table data are deleted. This later causes the ICE when the dwarf code tries to look

> at the label.

>

> Segher and I came up with 3 possible solutions to this:

>

> 1) Don't let move_insn_for_shrink_wrap try to move insns with label_ref in them.

> 2) Call mark_jump_label() on the copied instruction to fix up the ref counts.

> 3) Make the function actually move the insn instead of copying/deleting it.

>

> It seemed like option 2 was the best thing for stage 4 as it is not inhibiting anything

> and is just doing a fixup of the ref count.

>

> OK for trunk after regtesting on ppc64be (32/64) and x86_64?


OK.

> Thanks!

>    Aaron

>

>

> 2019-02-13  Aaron Sawdey  <acsawdey@linux.ibm.com>

>

>         * shrink-wrap.c (move_insn_for_shrink_wrap): Fix LABEL_NUSES counts

>         on copied instruction.

>

>

> Index: gcc/shrink-wrap.c

> ===================================================================

> --- gcc/shrink-wrap.c   (revision 268783)

> +++ gcc/shrink-wrap.c   (working copy)

> @@ -414,7 +414,12 @@

>        dead_debug_insert_temp (debug, DF_REF_REGNO (def), insn,

>                               DEBUG_TEMP_BEFORE_WITH_VALUE);

>

> -  emit_insn_after (PATTERN (insn), bb_note (bb));

> +  rtx_insn *insn_copy = emit_insn_after (PATTERN (insn), bb_note (bb));

> +  /* Update the LABEL_NUSES count on any referenced labels. The ideal

> +     solution here would be to actually move the instruction instead

> +     of copying/deleting it as this loses some notations on the

> +     insn.  */

> +  mark_jump_label (PATTERN (insn), insn_copy, 0);

>    delete_insn (insn);

>    return true;

>  }

>

>

> --

> Aaron Sawdey, Ph.D.  acsawdey@linux.vnet.ibm.com

> 050-2/C113  (507) 253-7520 home: 507/263-0782

> IBM Linux Technology Center - PPC Toolchain

>

Patch

Index: gcc/shrink-wrap.c
===================================================================
--- gcc/shrink-wrap.c	(revision 268783)
+++ gcc/shrink-wrap.c	(working copy)
@@ -414,7 +414,12 @@ 
       dead_debug_insert_temp (debug, DF_REF_REGNO (def), insn,
 			      DEBUG_TEMP_BEFORE_WITH_VALUE);

-  emit_insn_after (PATTERN (insn), bb_note (bb));
+  rtx_insn *insn_copy = emit_insn_after (PATTERN (insn), bb_note (bb));
+  /* Update the LABEL_NUSES count on any referenced labels. The ideal
+     solution here would be to actually move the instruction instead
+     of copying/deleting it as this loses some notations on the
+     insn.  */
+  mark_jump_label (PATTERN (insn), insn_copy, 0);
   delete_insn (insn);
   return true;
 }