> On Jul 26, 2021, at 11:09 AM, Martin Jambor <mjam...@suse.cz> wrote:
>
> Hi,
>
>>> So you either need to change build_access_from_expr like I described in
>>> my email
>>
>> Is the following the change you suggested previously:
>>
>> [opc@qinzhao-ol8u3-x86 gcc]$ git diff tree-sra.c
>> diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
>> index d1280e5f8848..c2597b705169 100644
>> --- a/gcc/tree-sra.c
>> +++ b/gcc/tree-sra.c
>> @@ -1195,8 +1195,10 @@ build_access_from_expr (tree expr, gimple *stmt, bool
>> write)
>> {
>> /* This means the aggregate is accesses as a whole in a way other than
>> an
>> assign statement and thus cannot be removed even if we had a scalar
>> - replacement for everything. */
>> - if (cannot_scalarize_away_bitmap)
>> + replacement for everything. However, when the STMT is a call to
>> + DEFERRED_INIT, we should not set this bit. */
>> + if (cannot_scalarize_away_bitmap
>> + && !gimple_call_internal_p (stmt, IFN_DEFERRED_INIT))
>> bitmap_set_bit (cannot_scalarize_away_bitmap, DECL_UID
>> (access->base));
>> return true;
>> }
>>
>
> Yes, although I think that the one I wrote today is more efficient as it
> tests for IFN_DEFERRED_INIT only if we already know stmt is a call and
> also philosophically more correct as the test is performed only for the
> LHS of the statement (I don't think either reason matters much in
> practice, though).
Okay, I will use the patch you wrote today instead.
>
>>
>>> or add the following to your patch, which is probably slightly
>>> mor efficient (but it has been only very mildly tested).
>>>
>>>
>>> diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
>>> index d1280e5f884..602b0fb3a2d 100644
>>> --- a/gcc/tree-sra.c
>>> +++ b/gcc/tree-sra.c
>>> @@ -1395,7 +1395,12 @@ scan_function (void)
>>>
>>> t = gimple_call_lhs (stmt);
>>> if (t && !disqualify_if_bad_bb_terminating_stmt (stmt, t, NULL))
>>> - ret |= build_access_from_expr (t, stmt, true);
>>> + {
>>> + if (gimple_call_internal_p (stmt, IFN_DEFERRED_INIT))
>>> + ret |= !!build_access_from_expr_1 (t, stmt, true);
>>> + else
>>> + ret |= build_access_from_expr (t, stmt, true);
>>> + }
>>> break;
>>
>> Thanks for the patch, but I don’t quite understand the above change:
>>
>> When the call is IFN_DEFERRED_INIT, you used build_access_from_expr_1
>> instead of build_access_from_expr to avoid setting
>> “cannot_scalarize_away_bitmap” bit.
>> But why adding “!” To this call?
>
> Note that it is a double !! which is basically a fancy way to convert a
> test for non-NULL to a bool. It is equivalent to:
>
> + if (gimple_call_internal_p (stmt, IFN_DEFERRED_INIT))
> + ret |= (build_access_from_expr_1 (t, stmt, true) != NULL);
> + else
> + ret |= build_access_from_expr (t, stmt, true);
>
> use whichever variant you like better. But the !! trick is used
> throughout the gcc source already.
Okay, I see. thanks.
Qing
>
> Martin
