[PATCH GCC][1/2]Feed bound computation to folder in loop split

Richard Sandiford richard.sandiford@linaro.org
Wed Jul 26 09:45:00 GMT 2017


Marc Glisse <marc.glisse@inria.fr> writes:
> On Wed, 26 Jul 2017, Richard Sandiford wrote:
>> Richard Biener <richard.guenther@gmail.com> writes:
>>> On Tue, Jul 25, 2017 at 7:45 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>> On Tue, 25 Jul 2017, Richard Biener wrote:
>>>>
>>>>>> I think we need Richard to say what the intent is for the valueization
>>>>>> function. It is used both to stop looking at defining stmt if the return
>>>>>> is
>>>>>> NULL, and to replace/optimize one SSA_NAME with another, but currently it
>>>>>> seems hard to prevent looking at the defining statement without
>>>>>> preventing
>>>>>> from looking at the SSA_NAME at all.
>>>>>
>>>>>
>>>>> Yeah, this semantic overloading is an issue.  For gimple_build we have
>>>>> nothing
>>>>> to "valueize" but we only use it to tell genmatch that it may not look at
>>>>> the
>>>>> SSA_NAME_DEF_STMT.
>>>>>
>>>>>> I guess we'll need a fix in genmatch...
>>>>>
>>>>>
>>>>> I'll have a look tomorrow.
>>>>
>>>>
>>>> My impression yesterday was that we could replace the current do_valueize
>>>> wrapper by 2 wrappers (without touching the valueize callbacks):
>>>> - may_check_def_stmt, which returns a bool corresponding to the current
>>>> do_valueize != NULL_TREE
>>>> - maybe_valueize, which tries to valueize, but if it gets a NULL_TREE, it
>>>> returns its argument unchanged.
>>>>
>>>> Not very confident about it though.
>>>
>>> Note I've been there in the past (twice I think) but always ran into existing
>>> latent issues.  So hopefully we've resolved those, I'm testing the following
>>> simplified variant of what I had back in time.
>>>
>>> It'll produce
>>>
>>>   switch (TREE_CODE (op0))
>>>     {
>>>     case SSA_NAME:
>>>       if (gimple *def_stmt = get_def (valueize, op0))
>>>         {
>>>           if (gassign *def = dyn_cast <gassign *> (def_stmt))
>>>             switch (gimple_assign_rhs_code (def))
>>>               {
>>>               case MINUS_EXPR:
>>>                 {
>>>                   tree o20 = gimple_assign_rhs1 (def);
>>>                   o20 = do_valueize (valueize, o20);
>>>                   tree o21 = gimple_assign_rhs2 (def);
>>>                   o21 = do_valueize (valueize, o21);
>>>                   if (op1 == o21 || (operand_equal_p (op1, o21, 0) &&
>>> types_match (op1, o21)))
>>>                     {
>>>
>>> which also indents less which is nice.
>>>
>>> Bootstrap/regtest running on x86_64-unknown-linux-gnu.
>>>
>>> Richard.
>>>
>>> 2017-07-26  Richard Biener  <rguenther@suse.de>
>>>
>>>         * gimple-match-head.c (do_valueize): Return OP if valueize
>>>         returns NULL_TREE.
>>>         (get_def): New helper to get at the def stmt of a SSA name
>>>         if valueize allows.
>>>         * genmatch.c (dt_node::gen_kids_1): Use get_def instead of
>>>         do_valueize to get at the def stmt.
>>>         (dt_operand::gen_gimple_expr): Simplify do_valueize calls.
>>>
>>>
>>>> --
>>>> Marc Glisse
>>>
>>> 2017-07-26  Richard Biener  <rguenther@suse.de>
>>>
>>> 	* gimple-match-head.c (do_valueize): Return OP if valueize
>>> 	returns NULL_TREE.
>>> 	(get_def): New helper to get at the def stmt of a SSA name
>>> 	if valueize allows.
>>> 	* genmatch.c (dt_node::gen_kids_1): Use get_def instead of
>>> 	do_valueize to get at the def stmt.
>>> 	(dt_operand::gen_gimple_expr): Simplify do_valueize calls.
>>>
>>> Index: gcc/gimple-match-head.c
>>> ===================================================================
>>> --- gcc/gimple-match-head.c	(revision 250518)
>>> +++ gcc/gimple-match-head.c	(working copy)
>>> @@ -779,10 +779,25 @@ inline tree
>>>  do_valueize (tree (*valueize)(tree), tree op)
>>>  {
>>>    if (valueize && TREE_CODE (op) == SSA_NAME)
>>> -    return valueize (op);
>>> +    {
>>> +      tree tem = valueize (op);
>>> +      if (tem)
>>> +	return tem;
>>> +    }
>>>    return op;
>>>  }
>>>
>>> +/* Helper for the autogenerated code, get at the definition of NAME when
>>> +   VALUEIZE allows that.  */
>>> +
>>> +inline gimple *
>>> +get_def (tree (*valueize)(tree), tree name)
>>> +{
>>> +  if (valueize && ! valueize (name))
>>> +    return NULL;
>>> +  return SSA_NAME_DEF_STMT (name);
>>> +}
>>
>> I realise this is preexisting, but why do we ignore the value returned
>> by valueize, even if it's different from NAME?
>
> My impression is that we expect it has already been valueized.

But in that case, why do we try to valueize it again?  It feels like
we're conflating two things.

Richard



More information about the Gcc-patches mailing list