[Bug middle-end/78016] New: REG_NOTE order is not kept during insn copy

["jiwang at gcc dot gnu.org" <gcc-bugzilla at gcc dot gnu dot org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78016

            Bug ID: 78016
           Summary: REG_NOTE order is not kept during insn copy
           Product: gcc
           Version: 7.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jiwang at gcc dot gnu.org
                CC: ebotcazou at gcc dot gnu.org, jakub at redhat dot com
  Target Milestone: ---

Created attachment 39826
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=39826&action=edit
keep REG_NOTE order during insn copy

I recently noticed when copying insn, GCC is actually not keeping the order of
REG_NOTEs.  This seems OK for some types of REG_NOTEs, for example UNUSED,
NORETURN etc.  But it seems not OK for DWARF annotation REG_NOTEs.

The reason is GCC DEARF module will do sanity check to make sure CFI rules come
from multiply paths are identical.


  A        B
  \       /
   \     /
    \   /
      C

For example, A and B are predecessor of C, then the following code in
"maybe_record_trace_start" in dwarf2cfi.c will do sanity check to make sure
DWARF rules come from A and B will be identical.

      /* We ought to have the same state incoming to a given trace no
         matter how we arrive at the trace.  Anything else means we've
         got some kind of optimization error.  */
      gcc_checking_assert (cfi_row_equal_p (cur_row, ti->beg_row));


As we don't keep the order of REG_NOTE, it's possible the final DWARF rules for
each register won't be identical.

It seems this issue is not exposed because:

  * normally, only prologue and epilogue basic blocks will contain DWARF
    annnocation info.
  * Very few passes (bb-reorder) after pro/epi insertion pass will do insn
copy.
  * DWARF sanity check happens before CFI auto deduction, normally backends
only
    generate simply DWARF info.  So normally we simply mark the insn as
    RTX_FRAME_RELATED_P to let GCC auto-deduct the CFI annocation this happens
    after the sanity check.

I came accorss the ICE when I was playing with some DWARF generation.

  0x98c9f4 maybe_record_trace_start
        ../../gcc-svn/gcc/dwarf2cfi.c:2285
  0x98cd15 create_trace_edges
        ../../gcc-svn/gcc/dwarf2cfi.c:2379
  0x98d59e scan_trace
        ../../gcc-svn/gcc/dwarf2cfi.c:2593
  0x98d686 create_cfi_notes
        ../../gcc-svn/gcc/dwarf2cfi.c:2619
  0x98e172 execute_dwarf2_frame
        ../../gcc-svn/gcc/dwarf2cfi.c:2977
  0x98ee3e execute
        ../../gcc-svn/gcc/dwarf2cfi.c:3457

You can reproduce this ICE by

step 1  (patch aarch64.c to generate DWARF manually)
===
Index: gcc/config/aarch64/aarch64.c
===================================================================
--- gcc/config/aarch64/aarch64.c        (revision 241233)
+++ gcc/config/aarch64/aarch64.c        (working copy)
@@ -2958,9 +2958,26 @@

   insn = emit_insn (aarch64_gen_storewb_pair (mode, stack_pointer_rtx, reg1,
                                              reg2, adjustment));
+#if 0
   RTX_FRAME_RELATED_P (XVECEXP (PATTERN (insn), 0, 2)) = 1;
   RTX_FRAME_RELATED_P (XVECEXP (PATTERN (insn), 0, 1)) = 1;
+#else
+  /* cfi_offset for reg1.  */
+  rtx mem =
+    gen_rtx_MEM (Pmode, plus_constant (Pmode, stack_pointer_rtx,
-adjustment));
+  add_reg_note (insn, REG_CFA_OFFSET, gen_rtx_SET (mem, reg1));
+
+  /* cfi_offset for reg2.  */
+  mem =
+    gen_rtx_MEM (Pmode,
+                plus_constant (Pmode, stack_pointer_rtx, -adjustment + 8));
+  add_reg_note (insn, REG_CFA_OFFSET, gen_rtx_SET (mem, reg2));
+
+  /* .cfi_def_cfa_offset for sp adjustment.  */
+  add_reg_note (insn, REG_CFA_DEF_CFA,
+               plus_constant (Pmode, stack_pointer_rtx, -adjustment));
+  add_reg_note (insn, REG_CFA_OFFSET, gen_rtx_SET (mem, reg2));
+
+  /* .cfi_def_cfa_offset for sp adjustment.  */
+  add_reg_note (insn, REG_CFA_DEF_CFA,
+               plus_constant (Pmode, stack_pointer_rtx, -adjustment));
   RTX_FRAME_RELATED_P (insn) = 1;
+#endif
 }

 static rtx

step 2
===
./configure --target=aarch64-linux --enable-languages=c,c++
make all-gcc -j10
./gcc/cc1 ./gcc/testsuite/gcc.c-torture/execute/20031204-1.c -g -O3 
-funroll-loops -fpeel-loops -ftracer -finline-functions 20031204-1.i
(please remove the #include <string.h> in the head of 20031204-1.c)

I attached a simply fix to keep REG-NOTE order during insn copy.

Any comments?

(The only relevant discussion I can find is at
https://gcc.gnu.org/ml/gcc-patches/2012-01/msg00546.html, where and when
REG_NOTE order was thinking to be not matter)