On 03/21/2017 02:21 PM, Jakub Jelinek wrote:
On Tue, Mar 21, 2017 at 06:53:34PM +0100, Jakub Jelinek wrote:
On Tue, Mar 21, 2017 at 08:41:43AM +0100, Jakub Jelinek wrote:
On Tue, Mar 21, 2017 at 08:38:20AM +1000, Richard Henderson wrote:
On 03/21/2017 07:15 AM, Jakub Jelinek wrote:
Not really sure what we should do if both i1 and i2 are frame related, shall
we check for each of the CFA reg notes if they are available and equal?
Or punt if either of the insns is frame related?

I would punt if either is frame related.

Ok, I'll test then the following patch and gather some statistic on how
often we trigger this.

The statistics I've gathered unfortunately shows that at least on
powerpc64le-linux it is very important to not give up if both i1 and i2
are frame related and have rtx_equal_p notes.
I've set on unpatched old_insns_match_p flags when returning non-dir_none
and checked those flags in the various callers of these when about to
successfully perform cross-jumping, head-merging etc.
With /f vs. non-/f, the only 3 hits were on the new pr80102.C testcase
during powerpc64le-linux bootstrap/regtest, but /f vs. /f there were
167601 hits.

Here is updated patch which allows cross-jumping of RTX_FRAME_RELATED_P
insns, as long as they have the same CFA related notes on them (same order
if more than one).

Bootstrapped/regtested on powerpc64le-linux, ok for trunk?

2017-03-21  Jakub Jelinek  <ja...@redhat.com>

        PR target/80102
        * cfgcleanup.c (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.
Presumably this didn't ICE at some point in the past, so it's a regression? (it's not marked as such in the BZ).



--- gcc/cfgcleanup.c.jj 2017-03-21 07:56:55.711233924 +0100
+++ gcc/cfgcleanup.c    2017-03-21 19:20:40.517746664 +0100
@@ -1149,6 +1149,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 +1212,58 @@ 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))
+    {
+      static enum reg_note cfa_note_kinds[] = {
+       REG_FRAME_RELATED_EXPR, REG_CFA_DEF_CFA, REG_CFA_ADJUST_CFA,
+       REG_CFA_OFFSET, REG_CFA_REGISTER, REG_CFA_EXPRESSION,
+       REG_CFA_VAL_EXPRESSION, REG_CFA_RESTORE, REG_CFA_SET_VDRAP,
+       REG_CFA_TOGGLE_RA_MANGLE, REG_CFA_WINDOW_SAVE, REG_CFA_FLUSH_QUEUE
+      };
ISTM this could get out of date very easily. Is there a clean way to generate the array of cfa notes as we build up the notes from reg-notes.def?


Jeff

Reply via email to