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: [patch] ccp changes


Zdenek Dvorak wrote on 11/08/06 08:46:

> 1) Store-ccp puts UNKNOWN_VAL value in the lattice.  However, there does
>    not seem to be any reason for that -- if the values of virtual
>    operands at entry are initialized to VARYING and the remaining virtual
>    ssa names to UNDEFINED, everything works just fine.  This patch
>    removes UNKNOWN_VAL.
>
Thanks.  I've been meaning to try this for a while now.

> 2) likely_value returns VARYING whenever there is a VARYING operand in
>    the statement; this is different from what the comment before it
>    claims, and it also is wrong (or at least overly conservative) --
>    e.g., 0 * VARYING = 0, so we do not want to give up immediatelly when
>    one of the operands is VARYING.  This patch makes likely_value
>    behave as described in the comment (i.e., return UNDEFINED if there
>    is at least one undefined operand, CONSTANT if there are no operands
>    or at least one of the operands is constant, and VARYING otherwise).
> 3) In ccp_initialize, if a phi node has VARYING argument, we mark it
>    as DONT_SIMULATE_AGAIN.  This is wrong, since we do not know whether
>    the corresponding edge will be executable or not.  The patch removes
>    the code responsible for this.
>
Excellent.

> 	PR tree-optimization/29738
> 	* tree-ssa-ccp.c: Remove UNKNOWN_VAL from comments.
> 	(ccp_lattice_t): Remove UNKNOWN_VAL.
> 	(dump_lattice_value, ccp_lattice_meet, ccp_visit_phi_node):
> 	Do not handle UNKNOWN_VAL.
> 	(get_default_value): Set initial value of virtual operands to
> 	VARYING.
> 	(get_value): Always use get_default_value on uninitialized
> 	operands.
> 	(set_value_varying, surely_varying_stmt_p): New functions.
> 	(set_lattice_value): Do not pass argument to get_value.
> 	Do not handle UNKNOWN_VAL.
> 	(likely_value): Follow the semantics described in the comment.
> 	(ccp_initialize): Use surely_varying_stmt_p.  Do not mark
> 	phi nodes DONT_SIMULATE_AGAIN.
> 	(ccp_fold): Do not pass argument to get_value.
> 	(fold_const_aggregate_ref, visit_assignment): Ditto.  Do not
> 	handle UNKNOWN_VAL.
>
> 	* gcc.dg/tree-ssa/ssa-ccp-14.c: New test.
>
OK with some revisions below:

>   static prop_value_t
>   get_default_value (tree var)
>   {
>     tree sym = SSA_NAME_VAR (var);
> !   prop_value_t val = { UNINITIALIZED, NULL_TREE, NULL_TREE };
>
>     if (!do_store_ccp && !is_gimple_reg (var))
>       {
> --- 312,328 ----
>      4- Variables defined by statements other than assignments and PHI
>         nodes are considered VARYING.
>
> !    5- Initial values of variables that are not GIMPLE registers are
> !       considered VARYING.  */
>
>   static prop_value_t
>   get_default_value (tree var)
>   {
>     tree sym = SSA_NAME_VAR (var);
> !   prop_value_t val;
> !
> !   val.value = NULL_TREE;
> !   val.mem_ref = NULL_TREE;
>
I'd rather have the other initializer.  Easier for the compiler
to check if you missed an assignment.

> *************** ccp_fold (tree stmt)
> *** 969,975 ****
>     return rhs;
>   }
>
> -
>   /* Return the tree representing the element referenced by T if T is an
>      ARRAY_REF or COMPONENT_REF into constant aggregates.  Return
>      NULL_TREE otherwise.  */
>
No.  There should be two empty lines between function bodies.

> *************** evaluate_stmt (tree stmt)
> *** 1162,1168 ****
> return val;
> }
>
> -
> /* Visit the assignment statement STMT. Set the value of its LHS to the
> value computed by the RHS and store LHS in *OUTPUT_P. If STMT
> creates virtual definitions, set the value of each new name to that
>
Likewise.



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