This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

fix alpha glibc miscompilation


The problem is complicated.  This does not *entirely* fix the
problem, I think, merely corrects this case; remaining cases
would require exception handling to provoke.

The problem starts with the peephole2 optimizer.  Alpha uses
this pass to break up CALL patterns into four steps: address
load, branch, and two insns to reload the gp.

The peephole pass attempts to handle splitting call patterns
by copying over the REG_EH_REGION notes from the old call to
the new.  Additionally, if (1) insns were emitted after the
call, and (2) the call has an exception edge, then logically
the block must end after the call_insn and subsequent insn
are placed in a new block.

The peephole pass believes that any EDGE_ABNORMAL_CALL edge
indicates something that needs to be handled this way.  And
truely, I can't fault this logic.  Now, prior to the patch
below, we described sibcalls as having an ABCALL edge to the
exit block.  This is where my problems begin.

The failure scenario proceeds as follows.  

  * Begin with a block with at least one normal call, and
    ends with a sibcall.

  * The first call, when split by the peepholer, will likely
    *not* have any EH_REGION notes.  That, plus the ABCALL
    at the end of the block leads the peepholer to decide
    that the first call has an EH edge.

  * The peep2 pass then splits the block after the first 
    call.  This leaves the (pc-relative) gp reload in the
    next block.

  * The bb-reorder pass decides to duplicate the block
    containing the first call.  It is constrained to not
    duplicate the gp-reload, because of various details
    concerning how that relocation is generated.

  * This results in two blocks containing calls, one of
    which then branches to the join block containing the
    gp-reload.  It is this path containing the branch that
    is incorrect -- the gp-reload is pc-relative and is
    fed the return address from the function; the first
    insn of the gp-reload must *immediately* follow the call.

All this goes of course to beg the question of why the 
gp-reload is broken apart from the call at all.  The 
answer of course is better scheduling.  If the second
insn of the gp-reload isn't broken out, we produce a 
for-sure stall.  If the first insn of the gp-reload isn't
broken out, then we havn't represented the fact that one
arithmetic pipeline is occupied.

To some extent this is a pass ordering problem.  If we 
run bb-reorder before calls are split apart, we have no
problems duplicating blocks at will.  If we split first,
then we've limited ourselves to not duplicating any block
that contains a call.

I have no clear idea what to do about that issue.

What this patch does, however, is start at the beginning
and prevent the original block from being split unnecessarily.
Which, for the test case at hand, solves the problem.



r~



        * basic-block.h (EDGE_SIBCALL): New.
        (EDGE_ALL_FLAGS): Update.
        * cfg.c (dump_edge_info): Add sibcall name.
        * cfgbuild.c (make_edges): Use EDGE_SIBCALL.
        * cfgrtl.c (purge_dead_edges): Handle sibcalls.

Index: basic-block.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/basic-block.h,v
retrieving revision 1.173
diff -c -p -d -u -r1.173 basic-block.h
--- basic-block.h	6 Jun 2003 09:24:23 -0000	1.173
+++ basic-block.h	7 Jun 2003 21:27:31 -0000
@@ -151,7 +151,8 @@ typedef struct edge_def {
 #define EDGE_CAN_FALLTHRU	64	/* Candidate for straight line
 					   flow.  */
 #define EDGE_IRREDUCIBLE_LOOP	128	/* Part of irreducible loop.  */
-#define EDGE_ALL_FLAGS		255
+#define EDGE_SIBCALL		256	/* Edge from sibcall to exit.  */
+#define EDGE_ALL_FLAGS		511
 
 #define EDGE_COMPLEX	(EDGE_ABNORMAL | EDGE_ABNORMAL_CALL | EDGE_EH)
 
Index: cfg.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cfg.c,v
retrieving revision 1.44
diff -c -p -d -u -r1.44 cfg.c
--- cfg.c	6 Jun 2003 09:24:23 -0000	1.44
+++ cfg.c	7 Jun 2003 21:27:31 -0000
@@ -652,8 +652,10 @@ dump_edge_info (file, e, do_succ)
 
   if (e->flags)
     {
-      static const char * const bitnames[]
-	= {"fallthru", "ab", "abcall", "eh", "fake", "dfs_back", "can_fallthru","irreducible"};
+      static const char * const bitnames[] = {
+	"fallthru", "ab", "abcall", "eh", "fake", "dfs_back",
+	"can_fallthru", "irreducible", "sibcall"
+      };
       int comma = 0;
       int i, flags = e->flags;
 
Index: cfgbuild.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cfgbuild.c,v
retrieving revision 1.35
diff -c -p -d -u -r1.35 cfgbuild.c
--- cfgbuild.c	24 Apr 2003 15:53:19 -0000	1.35
+++ cfgbuild.c	7 Jun 2003 21:27:31 -0000
@@ -406,8 +406,7 @@ make_edges (label_value_list, min, max, 
 	 worry about EH edges, since we wouldn't have created the sibling call
 	 in the first place.  */
       if (code == CALL_INSN && SIBLING_CALL_P (insn))
-	cached_make_edge (edge_cache, bb, EXIT_BLOCK_PTR,
-		   EDGE_ABNORMAL | EDGE_ABNORMAL_CALL);
+	cached_make_edge (edge_cache, bb, EXIT_BLOCK_PTR, EDGE_SIBCALL);
 
       /* If this is a CALL_INSN, then mark it as reaching the active EH
 	 handler for this CALL_INSN.  If we're handling non-call
Index: cfgrtl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cfgrtl.c,v
retrieving revision 1.77
diff -c -p -d -u -r1.77 cfgrtl.c
--- cfgrtl.c	6 Jun 2003 09:24:25 -0000	1.77
+++ cfgrtl.c	7 Jun 2003 21:27:31 -0000
@@ -2299,6 +2299,19 @@ purge_dead_edges (bb)
 
       return purged;
     }
+  else if (GET_CODE (insn) == CALL_INSN && SIBLING_CALL_P (insn))
+    {
+      /* First, there should not be any EH or ABCALL edges resulting
+	 from non-local gotos and the like.  If there were, we shouldn't
+	 have created the sibcall in the first place.  Second, there
+	 should of course never have been a fallthru edge.  */
+      if (!bb->succ || bb->succ->succ_next)
+	abort ();
+      if (bb->succ->flags != EDGE_SIBCALL)
+	abort ();
+
+      return 0;
+    }
 
   /* If we don't see a jump insn, we don't know exactly why the block would
      have been broken at this point.  Look for a simple, non-fallthru edge,


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]