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]

Re: Your change to alloc-pool.c


> 
> On Wed, 28 Jan 2004, Jan Hubicka wrote:
> > > > It's indeed clearly a latent bug.
> > >
> > > I am testing the attached patch.
> > > It solve at least the C testcase you have me.
> 
> Could you provide a bit more explanation of why you believe your
> patch is the correct fix, and that this isn't actually a bug in
> the CFG code, i.e. redirect_edge_and_branch_force?
> 
> I'm particularly confused how between line 4809 of gcse.c, that we
> explicitly ignore abnormal edges, "e->flags & EDGE_COMPLEX", and
> line 4890, where we call "insert_insn_on_edge" with e, we manage
> to trip the assert "(e->flags & EDGE_ABNORMAL) && EDGE_CRITICAL_P (e)".
> 
> Can redirecting an edge change it from normal to abnormal?
> Where are we trying to unify/combine two edges when either edge
> has instructions on.

The edge is not abnormal, but CFG code consistently unify two edges
pointing to same destination and it always happens to be the edge being
redirected to be removed.

Via unification of flag, the edge even may happen to be abnormal, but it
is not what is happening in this case.

The problem is that after unification with other edge going from same
basic block, the code emitting the instruction (if fixed to find proper
edge and not use released datastructure) emits it on both code paths,
not just on the one originally going via old edge.

Of course the bypassing is possible, but we would have to proactivly
create the basic block with instructions we are about to put into split
edge, but that would need more massaging of gcse.c to be able to accept
new basic blocks appearing and disappearing.
> 
> 
> > + 	      edge e2;
> > + 	      for (e2 = e->src->succ; e2; e2 = e2->succ_next)
> > + 		if (e2->dest == dest)
> > + 		  break;
> > + 	      if (e2)
> > + 		dest = NULL;
> 
> Given that you're disabling jump bypassing opportunities, its only
> fitting that you rewrite this as:
> 
>   edge e2;
>   for (e2 = e->src->succ; e2; e2 = e2->succ_next)
>     if (e2->dest == dest)
>     {
>       dest = NULL;
>       break;
>     }
> 
> This has less control flow, and doesn't require jump bypassing to
> clean it up later :>
OK, sure :)
Of course I originally used the e2 to put instruction into, so the code
made more sense...

Honza
> 
> Roger
> --
> 


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