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: [tree-ssa] COND_EXPR lowering.


In message <20031024162721.GA17804@atrey.karlin.mff.cuni.cz>, Zdenek Dvorak wri
tes:
 >> > + 
 >> > + 	  slow = e->dest;
 >> > + 	  set_slow = 0;
 >> > + 
 >> > + 	  last = e->dest->succ;
 >> > + 	  for (dest = e->dest->succ->dest;
 >> > + 	       tree_forwarder_block_p (dest);
 >> > + 	       last = dest->succ,
 >> > + 	       dest = dest->succ->dest,
 >> > + 	       set_slow ^= 1)
 >> > + 	    {
 >> > + 	      /* Infinite loop detected.  */
 >> > + 	      if (slow == dest)
 >> > + 		break;
 >> > + 	      if (set_slow)
 >> > + 		slow = slow->succ->dest;
 >> > + 
 >> > + 	      if (dest->succ->dest == EXIT_BLOCK_PTR)
 >> > + 		break;
 >> > + 	    }
 >> > + 
 >> > + 	  if (dest == e->dest)
 >> > + 	    continue;
 >> > + 	      
 >> 
 >> So if we do detect and infinite loop, we might still redirect the edges
 >> anyway? Does that actually serve a purpose? It looks like we'd be
 >> threading into an arbitary point in the cycle?
 >
 >Yes we are.  Neither feature nor bug, just it is easier not to handle it
 >specially when it does not have to be.
This was actually some of the code I really didn't like in Zdenek's
threader.  It's far from obvious what that code is doing.   I really
don't like the coding style for the "for" loop either.  This is naturally
a recursive problem, so unless there's a performance issue, let's use
recursion.

The way it detects loops is also far from obvious.  I think it's far
more straightforward to note which blocks you've forwarded through and
not revisit the same block more than once.  

 >> The only other thing that I think Diego agreed with (yes? no?) is that
 >> we probably ought to set the BB for the 2 goto's on the arms of the
 >> COND_EXPR. Yeah, they aren't real stmt's, but there is no reason someone
 >> couldn't look at them as real stmts.. ie, someone doing path following
 >> may want to process the 2 arms exactly like they process a GOTO, so we
 >> ought to make them behave like a GOTO stmt for consistancy, so we ought
 >> to set their BB.
 >
 >I don't want to do it.  Every change of the statement would than have to
 >take care of setting it, which would be a source of unnecessary errors. 
Err, why again aren't the arms real statements?  I thought we had decided
to go ahead and leave them as real statements with their associated basic
blocks.

 >> ie, we could see something like:
 >> if (TREE_CODE (expr) == GOTO_EXPR
 >>   follow_goto (expr)
 >> else
 >> if (TREE_CODE (expr) == COND_EXPR)
 >>   {
 >>     follow_goto (COND_EXPR_THEN (expr));
 >>     follow_goto (COND_EXPR_ELSE (expr));
 >>   }
 >> where follow_goto wants to treat it just like a stmt, and may ask for
 >> the BB...
 >
 >Why the hell should we do something that stupid? This is what cfg is
 >here for.
You can't always use the CFG for everything.

Jeff



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