Re: [PATCH] PR rtl-optimization/88308 Update LABEL_NUSES in move_insn_for_shrink_wrap

[Richard Biener]

On Thu, Feb 14, 2019 at 12:08 AM Aaron Sawdey  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  
>
> * 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.  acsaw...@linux.vnet.ibm.com
> 050-2/C113  (507) 253-7520 home: 507/263-0782
> IBM Linux Technology Center - PPC Toolchain
>

[PATCH] PR rtl-optimization/88308 Update LABEL_NUSES in move_insn_for_shrink_wrap

[Aaron Sawdey]

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  

* 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.  acsaw...@linux.vnet.ibm.com
050-2/C113  (507) 253-7520 home: 507/263-0782
IBM Linux Technology Center - PPC Toolchain