[PATCH] Don't cross-jump in between frame related and non-frame related insns (PR target/80102, take 4)

[Jakub Jelinek <jakub at redhat dot com>]

Hi!

On Sat, Mar 25, 2017 at 07:45:53AM +0100, Jakub Jelinek wrote:
> On Fri, Mar 24, 2017 at 06:37:46PM -0500, Segher Boessenkool wrote:
> > On Fri, Mar 24, 2017 at 08:36:16PM +0100, Jakub Jelinek wrote:
> > > +	  /* Skip over reg notes not related to CFI information.  */
> > > +	  while (n1)
> > > +	    {
> > > +	      for (i = 0; i < ARRAY_SIZE (cfa_note_kinds) - 1; i++)
> > > +		if (REG_NOTE_KIND (n1) == cfa_note_kinds[i])
> > > +		  break;
> > > +	      if (i != ARRAY_SIZE (cfa_note_kinds))
> > > +		break;
> > > +	      n1 = XEXP (n1, 1);
> > > +	    }
> > 
> > Maybe factor out reg_note_is_cfa_note and/or insns_have_identical_cfa_notes
> > functions?
> 
> Well, for the reg_note_is_cfa_note, actually it might be better to just
> have a const bool array indexed by the note enum instead of the loop, I'll
> implement it later.  And yes, I can outline insns_have_identical_cfa_notes.

Here is updated patch that does that, bootstrapped/regtested on
powerpc64le-linux, ok for trunk?

2017-03-26  Jakub Jelinek  <jakub@redhat.com>

	PR target/80102
	* reg-notes.def (REG_CFA_NOTE): Define.  Use it for CFA related
	notes.
	* cfgcleanup.c (reg_note_cfa_p): New array.
	(insns_have_identical_cfa_notes): New function.
	(old_insns_match_p): Don't cross-jump in between /f
	and non-/f instructions.  If both i1 and i2 are frame related,
	verify all CFA notes, their order and content.

	* g++.dg/opt/pr80102.C: New test.

--- gcc/reg-notes.def.jj	2017-03-24 22:42:29.306844194 +0100
+++ gcc/reg-notes.def	2017-03-26 19:57:22.220346563 +0200
@@ -20,10 +20,16 @@ along with GCC; see the file COPYING3.
 /* This file defines all the codes that may appear on individual
    EXPR_LIST, INSN_LIST and INT_LIST rtxes in the REG_NOTES chain of an insn.
    The codes are stored in the mode field of the rtx.  Source files
-   define DEF_REG_NOTE appropriately before including this file.  */
+   define DEF_REG_NOTE appropriately before including this file.
+
+   CFA related notes meant for RTX_FRAME_RELATED_P instructions
+   should be declared with REG_CFA_NOTE macro instead of REG_NOTE.  */
 
 /* Shorthand.  */
 #define REG_NOTE(NAME) DEF_REG_NOTE (REG_##NAME)
+#ifndef REG_CFA_NOTE
+# define REG_CFA_NOTE(NAME) REG_NOTE (NAME)
+#endif
 
 /* REG_DEP_TRUE is used in scheduler dependencies lists to represent a
    read-after-write dependency (i.e. a true data dependency).  This is
@@ -112,7 +118,7 @@ REG_NOTE (BR_PRED)
 /* Attached to insns that are RTX_FRAME_RELATED_P, but are too complex
    for DWARF to interpret what they imply.  The attached rtx is used
    instead of intuition.  */
-REG_NOTE (FRAME_RELATED_EXPR)
+REG_CFA_NOTE (FRAME_RELATED_EXPR)
 
 /* Attached to insns that are RTX_FRAME_RELATED_P, but are too complex
    for FRAME_RELATED_EXPR intuition.  The insn's first pattern must be
@@ -122,7 +128,7 @@ REG_NOTE (FRAME_RELATED_EXPR)
    with a base register and a constant offset.  In the most complicated
    cases, this will result in a DW_CFA_def_cfa_expression with the rtx
    expression rendered in a dwarf location expression.  */
-REG_NOTE (CFA_DEF_CFA)
+REG_CFA_NOTE (CFA_DEF_CFA)
 
 /* Attached to insns that are RTX_FRAME_RELATED_P, but are too complex
    for FRAME_RELATED_EXPR intuition.  This note adjusts the expression
@@ -130,57 +136,57 @@ REG_NOTE (CFA_DEF_CFA)
    expression, relative to the old CFA expression.  This rtx must be of
    the form (SET new-cfa-reg (PLUS old-cfa-reg const_int)).  If the note
    rtx is NULL, we use the first SET of the insn.  */
-REG_NOTE (CFA_ADJUST_CFA)
+REG_CFA_NOTE (CFA_ADJUST_CFA)
 
 /* Similar to FRAME_RELATED_EXPR, with the additional information that
    this is a save to memory, i.e. will result in DW_CFA_offset or the
    like.  The pattern or the insn should be a simple store relative to
    the CFA.  */
-REG_NOTE (CFA_OFFSET)
+REG_CFA_NOTE (CFA_OFFSET)
 
 /* Similar to FRAME_RELATED_EXPR, with the additional information that this
    is a save to a register, i.e. will result in DW_CFA_register.  The insn
    or the pattern should be simple reg-reg move.  */
-REG_NOTE (CFA_REGISTER)
+REG_CFA_NOTE (CFA_REGISTER)
 
 /* Attached to insns that are RTX_FRAME_RELATED_P, but are too complex
    for FRAME_RELATED_EXPR intuition.  This is a save to memory, i.e. will
    result in a DW_CFA_expression.  The pattern or the insn should be a
    store of a register to an arbitrary (non-validated) memory address.  */
-REG_NOTE (CFA_EXPRESSION)
+REG_CFA_NOTE (CFA_EXPRESSION)
 
 /* Attached to insns that are RTX_FRAME_RELATED_P, but are too complex
    for FRAME_RELATED_EXPR intuition.  The DWARF expression computes the value of
    the given register.  */
-REG_NOTE (CFA_VAL_EXPRESSION)
+REG_CFA_NOTE (CFA_VAL_EXPRESSION)
 
 /* Attached to insns that are RTX_FRAME_RELATED_P, with the information
    that this is a restore operation, i.e. will result in DW_CFA_restore
    or the like.  Either the attached rtx, or the destination of the insn's
    first pattern is the register to be restored.  */
-REG_NOTE (CFA_RESTORE)
+REG_CFA_NOTE (CFA_RESTORE)
 
 /* Attached to insns that are RTX_FRAME_RELATED_P, marks insn that sets
    vDRAP from DRAP.  If vDRAP is a register, vdrap_reg is initalized
    to the argument, if it is a MEM, it is ignored.  */
-REG_NOTE (CFA_SET_VDRAP)
+REG_CFA_NOTE (CFA_SET_VDRAP)
 
 /* Attached to insns that are RTX_FRAME_RELATED_P, indicating a window
    save operation, i.e. will result in a DW_CFA_GNU_window_save.
    The argument is ignored.  */
-REG_NOTE (CFA_WINDOW_SAVE)
+REG_CFA_NOTE (CFA_WINDOW_SAVE)
 
 /* Attached to insns that are RTX_FRAME_RELATED_P, marks the insn as
    requiring that all queued information should be flushed *before* insn,
    regardless of what is visible in the rtl.  The argument is ignored.
    This is normally used for a call instruction which is not exposed to
    the rest of the compiler as a CALL_INSN.  */
-REG_NOTE (CFA_FLUSH_QUEUE)
+REG_CFA_NOTE (CFA_FLUSH_QUEUE)
 
 /* Attached to insns that are RTX_FRAME_RELATED_P, toggling the mangling status
    of return address.  Currently it's only used by AArch64.  The argument is
    ignored.  */
-REG_NOTE (CFA_TOGGLE_RA_MANGLE)
+REG_CFA_NOTE (CFA_TOGGLE_RA_MANGLE)
 
 /* Indicates what exception region an INSN belongs in.  This is used
    to indicate what region to which a call may throw.  REGION 0
--- gcc/cfgcleanup.c.jj	2017-03-24 22:42:29.624840140 +0100
+++ gcc/cfgcleanup.c	2017-03-26 20:16:19.825581466 +0200
@@ -1111,6 +1111,48 @@ merge_dir (enum replace_direction a, enu
   return dir_none;
 }
 
+/* Array of flags indexed by reg note kind, true if the given
+   reg note is CFA related.  */
+static const bool reg_note_cfa_p[] = {
+#undef REG_CFA_NOTE
+#define DEF_REG_NOTE(NAME) false,
+#define REG_CFA_NOTE(NAME) true,
+#include "reg-notes.def"
+#undef REG_CFA_NOTE
+#undef DEF_REG_NOTE
+  false
+};
+
+/* Return true if I1 and I2 have identical CFA notes (the same order
+   and equivalent content).  */
+
+static bool
+insns_have_identical_cfa_notes (rtx_insn *i1, rtx_insn *i2)
+{
+  rtx n1, n2;
+  for (n1 = REG_NOTES (i1), n2 = REG_NOTES (i2); ;
+       n1 = XEXP (n1, 1), n2 = XEXP (n2, 1))
+    {
+      /* Skip over reg notes not related to CFI information.  */
+      while (n1 && !reg_note_cfa_p[REG_NOTE_KIND (n1)])
+	n1 = XEXP (n1, 1);
+      while (n2 && !reg_note_cfa_p[REG_NOTE_KIND (n2)])
+	n2 = XEXP (n2, 1);
+      if (n1 == NULL_RTX && n2 == NULL_RTX)
+	return true;
+      if (n1 == NULL_RTX || n2 == NULL_RTX)
+	return false;
+      if (XEXP (n1, 0) == XEXP (n2, 0))
+	;
+      else if (XEXP (n1, 0) == NULL_RTX || XEXP (n2, 0) == NULL_RTX)
+	return false;
+      else if (!(reload_completed
+		 ? rtx_renumbered_equal_p (XEXP (n1, 0), XEXP (n2, 0))
+		 : rtx_equal_p (XEXP (n1, 0), XEXP (n2, 0))))
+	return false;
+    }
+}
+
 /* Examine I1 and I2 and return:
    - dir_forward if I1 can be replaced by I2, or
    - dir_backward if I2 can be replaced by I1, or
@@ -1149,6 +1191,11 @@ old_insns_match_p (int mode ATTRIBUTE_UN
   else if (p1 || p2)
     return dir_none;
 
+  /* Do not allow cross-jumping between frame related insns and other
+     insns.  */
+  if (RTX_FRAME_RELATED_P (i1) != RTX_FRAME_RELATED_P (i2))
+    return dir_none;
+
   p1 = PATTERN (i1);
   p2 = PATTERN (i2);
 
@@ -1207,6 +1254,11 @@ old_insns_match_p (int mode ATTRIBUTE_UN
 	}
     }
 
+  /* If both i1 and i2 are frame related, verify all the CFA notes
+     in the same order and with the same content.  */
+  if (RTX_FRAME_RELATED_P (i1) && !insns_have_identical_cfa_notes (i1, i2))
+    return dir_none;
+
 #ifdef STACK_REGS
   /* If cross_jump_death_matters is not 0, the insn's mode
      indicates whether or not the insn contains any stack-like
--- gcc/testsuite/g++.dg/opt/pr80102.C.jj	2017-03-26 19:57:22.221346550 +0200
+++ gcc/testsuite/g++.dg/opt/pr80102.C	2017-03-26 19:57:22.221346550 +0200
@@ -0,0 +1,14 @@
+// PR target/80102
+// { dg-do compile }
+// { dg-options "-fnon-call-exceptions -Os" }
+// { dg-additional-options "-mminimal-toc" { target { powerpc*-*-* && lp64 } } }
+
+struct B { float a; B (float c) { for (int g; g < c;) ++a; } };
+struct D { D (B); };
+
+int
+main ()
+{
+  B (1.0);
+  D e (0.0), f (1.0);
+}


	Jakub