[PATCH GCC][1/2]Feed bound computation to folder in loop split
Marc Glisse
marc.glisse@inria.fr
Wed Jul 26 09:38:00 GMT 2017
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.
--
Marc Glisse
More information about the Gcc-patches
mailing list