diff mbox series

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 | expand

Commit Message

Aaron Sawdey Feb. 13, 2019, 11:08 p.m. UTC 
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.

Comments

Richard Biener Feb. 14, 2019, 9:37 a.m. UTC | #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
>
diff mbox series

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;
 }