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: [tuples][patch] Convert pass_ccp and pass_store_ccp to tuples


On Tue, Mar 4, 2008 at 1:46 PM, Diego Novillo <dnovillo@google.com> wrote:

>  But it doesn't matter in the case of PHI nodes, because those are
>  *always* simulated.

I still don't see how this affects whether the phi node is simulated or not.
The flag in question exists to prevent us from pushing duplicate entries
onto the worklist.

>  Mainline does not have this code, so in theory you should be able to
>  reproduce the bug there.  There's nothing in the tuples code that should
>  require changes in the propagation logic.

As I understand it, mainline is failing to initialize the
STMT_IN_SSA_EDGE_WORKLIST
flag for phi nodes, and the initial attempt to add a phi node to the
worklist is accessing
an undefined value.  Here is the logic in add_ssa_edge:

/* We have just defined a new value for VAR.  If IS_VARYING is true,
   add all immediate uses of VAR to VARYING_SSA_EDGES, otherwise add
   them to INTERESTING_SSA_EDGES.  */

static void
add_ssa_edge (tree var, bool is_varying)
{
  imm_use_iterator iter;
  use_operand_p use_p;

  FOR_EACH_IMM_USE_FAST (use_p, iter, var)
    {
      tree use_stmt = USE_STMT (use_p);

      if (!DONT_SIMULATE_AGAIN (use_stmt)
	  && !STMT_IN_SSA_EDGE_WORKLIST (use_stmt))
	{
	  STMT_IN_SSA_EDGE_WORKLIST (use_stmt) = 1;
	  if (is_varying)
	    VEC_safe_push (tree, gc, varying_ssa_edges, use_stmt);
	  else
	    VEC_safe_push (tree, gc, interesting_ssa_edges, use_stmt);
	}
    }
}

>  So, if this is needed in tuples, it should also be needed in mainline.
>  Can you send me a test case?

Mainline doesn't actually manifest a failure, though I could attempt
to show that the
uninitialized flags are fortuitiously zero, or forcibly scramble them
prior to invoking
the pass.

Given the way add_ssa_edge uses the flag, I can't see how it is
correct not to clear
it for phi nodes in ssa_prop_init if it is deemed necessary to clear
it for normal statements.
Both can contain uses of a def, and need to be added to the worklist
when an interesting
value is propagated to the def, and this is only done via add_ssa_edge.

--Bill


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