diff mbox series

[committed,PR,target/83760] Fix several instances of maybe_record_trace_start ICEs on sh

Message ID 3c47469f-c8c0-4daa-aa44-7b38da408727@redhat.com
State New
Headers show
Series [committed,PR,target/83760] Fix several instances of maybe_record_trace_start ICEs on sh | expand

Commit Message

Jeff Law Feb. 13, 2018, 3:09 a.m. UTC
The SH port has a pass which looks for suitable places to put its
constant table.

Ideally it finds a barrier (within a certain range from the first use of
the pool) and shoves the constant pool after that barrier.

Otherwise it'll create jump/barrier/<target> style sequence and insert
the constant pool after the barrier, but before the jump target.


That's all fine and good.  Except that it can confuse the CFI machinery.
 The sequence makes it look like there's live code after the sibling
call.  It's now so clear why this bug came and went semi-randomly across
the various sh targets -- it depends on the sibling call being the insn
which causes the main loop in find_barrier to exit.  So one more or one
less insn, or an insn changing alternative (and thus size) and the bug
goes latent.

Anyway fixing this is pretty simple.  We can just insert the table after
the sibling call (taking care not to separate the call from its
NOTE_INSN_CALL_ARG_LOCATION note).

I went back and verified each of the SH bugs where we had testcases for
this problem were fixed by this patch.  I also verified that the sh-elf
libgcc/newlib targets built after this patch.

Given the sensitivity to precisely what insn causes the loop to exit, I
haven't bothered to add a test to the testsuite.

This would likely be a reasonable backport candidate to gcc-7 if SH
folks are interested.

Installed onto the trunk,

Jeff

ps.  No, I wasn't really looking to fix the SH stuff.  I'm really
chasing that MIPS regression.  Fixing this was just collateral damage.
commit eeace813c643ee2f2c5f37bf984a680be6032a2d
Author: law <law@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Tue Feb 13 03:07:04 2018 +0000

            PR target/83760
            * config/sh/sh.c (find_barrier): Consider a sibling call
            a barrier as well.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@257611 138bc75d-0d04-0410-961f-82ee72b054a4
diff mbox series

Patch

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index d5913d0a7db..a74c8610443 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,9 @@ 
 2018-02-12  Jeff Law  <law@redhat.com>
 
+	PR target/83760
+	* config/sh/sh.c (find_barrier): Consider a sibling call
+	a barrier as well.
+
 	* cse.c (try_back_substitute_reg): Move any REG_ARGS_SIZE note when
 	successfully back substituting a reg.
 
diff --git a/gcc/config/sh/sh.c b/gcc/config/sh/sh.c
index 48e99a3cadf..90d6c733d33 100644
--- a/gcc/config/sh/sh.c
+++ b/gcc/config/sh/sh.c
@@ -5233,10 +5233,22 @@  find_barrier (int num_mova, rtx_insn *mova, rtx_insn *from)
 	 CALL_ARG_LOCATION note.  */
       if (CALL_P (from))
 	{
+	  bool sibcall_p = SIBLING_CALL_P (from);
+
 	  rtx_insn *next = NEXT_INSN (from);
 	  if (next && NOTE_P (next)
 	      && NOTE_KIND (next) == NOTE_INSN_CALL_ARG_LOCATION)
 	    from = next;
+
+	  /* If FROM was a sibling call, then we know that control
+	     will not return.  In fact, we were guaranteed to hit
+	     a barrier before another real insn.
+
+	     The jump around the constant pool is unnecessary.  It
+	     costs space, but more importantly it confuses dwarf2cfi
+	     generation.  */
+	  if (sibcall_p)
+	    return emit_barrier_after (from);
 	}
 
       from = emit_jump_insn_after (gen_jump (label), from);