public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] PR rtl-optimization/88308 Update LABEL_NUSES in move_insn_for_shrink_wrap
@ 2019-02-13 23:08 Aaron Sawdey
  2019-02-14  9:38 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Aaron Sawdey @ 2019-02-13 23:08 UTC (permalink / raw)
  To: gcc-patches, Segher Boessenkool, Bill Schmidt, David Edelsohn,
	ebotcazou, rguenther

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.


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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] PR rtl-optimization/88308 Update LABEL_NUSES in move_insn_for_shrink_wrap
  2019-02-13 23:08 [PATCH] PR rtl-optimization/88308 Update LABEL_NUSES in move_insn_for_shrink_wrap Aaron Sawdey
@ 2019-02-14  9:38 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2019-02-14  9:38 UTC (permalink / raw)
  To: Aaron Sawdey
  Cc: GCC Patches, Segher Boessenkool, Bill Schmidt, David Edelsohn,
	Eric Botcazou, Richard Guenther

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
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-02-14  9:38 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-13 23:08 [PATCH] PR rtl-optimization/88308 Update LABEL_NUSES in move_insn_for_shrink_wrap Aaron Sawdey
2019-02-14  9:38 ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).