Looks like this patch breaks bootstrap.
/home3/wschmidt/gcc/gcc-mainline-base/gcc/tree-ssa-dse.c: In function
'void dse\
_optimize_redundant_stores(gimple*)':
/home3/wschmidt/gcc/gcc-mainline-base/gcc/tree-ssa-dse.c:649:46: error:
ISO C++\
forbids converting a string constant to 'char*' [-Werror=write-strings]
649 | delete_dead_or_redundant_assignment (&gsi, "redundant");
| ^~~~~~~~~~~
/home3/wschmidt/gcc/gcc-mainline-base/gcc/tree-ssa-dse.c:651:40: error:
ISO C++\
forbids converting a string constant to 'char*' [-Werror=write-strings]
651 | delete_dead_or_redundant_call (&gsi, "redundant");
| ^~~~~~~~~~~
/home3/wschmidt/gcc/gcc-mainline-base/gcc/tree-ssa-dse.c: In member
function 'v\
oid dse_dom_walker::dse_optimize_stmt(gimple_stmt_iterator*)':
/home3/wschmidt/gcc/gcc-mainline-base/gcc/tree-ssa-dse.c:979:41: error:
ISO C++\
forbids converting a string constant to 'char*' [-Werror=write-strings]
979 | delete_dead_or_redundant_call (gsi, "dead");
| ^~~~~~
/home3/wschmidt/gcc/gcc-mainline-base/gcc/tree-ssa-dse.c:1007:39: error:
ISO C+\
+ forbids converting a string constant to 'char*' [-Werror=write-strings]
1007 | delete_dead_or_redundant_call (gsi, "dead");
| ^~~~~~
/home3/wschmidt/gcc/gcc-mainline-base/gcc/tree-ssa-dse.c:1066:49: error:
ISO C+\
+ forbids converting a string constant to 'char*' [-Werror=write-strings]
1066 | delete_dead_or_redundant_assignment (gsi, "dead");
| ^~~~~~
Thanks,
Bill
On 6/26/19 1:13 PM, Jeff Law wrote:
> On 6/26/19 5:53 AM, Richard Biener wrote:
>> On Wed, Jun 26, 2019 at 6:17 AM Jeff Law <l...@redhat.com> wrote:
>>> So based on the conversation in the BZ I cobbled together a patch to
>>> extend tree-ssa-dse.c to also detect redundant stores.
>>>
>>> To be clear, given two stores, the first store is dead if the later
>>> store overwrites all the live bytes set by the first store. In this
>>> case we delete the first store. If the first store is partially dead we
>>> may trim it.
>>>
>>> Given two stores, the second store is redundant if it stores _the same
>>> value_ into locations set by the first store. In this case we delete
>>> the second store.
>>>
>>>
>>> We prefer to remove redundant stores over removing dead or trimming
>>> partially dead stores.
>>>
>>> First, if we detect a redundant store, we can always remove it. We may
>>> not always be able to trim a partially dead store. So removing the
>>> redundant store wins in this case.
>>>
>>> But even if the redundant store occurs at the head or tail of the prior
>>> store, removing the redundant store is better than trimming the
>>> partially dead store because we end up with fewer calls to memset with
>>> the same number of total bytes written.
>>>
>>> We only look for redundant stores in a few cases. The first store must
>>> be a memset, empty constructor or calloc call -- ie things which
>>> initialize multiple memory locations to zero. Subsequent stores can
>>> occur via memset, empty constructors or simple memory assignments.
>>>
>>> The chagne to tree-ssa-alias.c deserves a quick note.
>>>
>>> When we're trying to determine if we have a redundant store, we create
>>> an AO_REF for the *second* store, then ask the alias system if the first
>>> store would kill the AO_REF.
>>>
>>> So while normally a calloc wouldn't ever kill anything in normal
>>> execution order, we're not asking about things in execution order. We
>>> really just want to know if the calloc is going to write into the
>>> entirety of the AO_REF of the subsequent store. So we compute the size
>>> of the allocation and we know the destination from the LHS of the calloc
>>> call and everything "just works".
>> I see how stmt_kills_ref_p is convenient here and it's the only
>> ref-must-include-other-ref kind of query the oracle supports right now.
>> Note it is not optimized for your particular case querying the same
>> stmt for multiple refs. It's not refs_must_alias_p that is missing
>> but something stronger ('kills' is also wrong since both refs might
>> be reads), ref_covered_by_ref_p or so. That said, factoring
>> stmt_kills_ref_p might not be so straight-forward for calls since
>> we lack a general ao_ref_init for calls (ao_ref_init_stores_from_call,
>> ao_ref_init_loads_from_call?).
>>
>> So I think the tree-ssa-alias.c change is fine but please put a
>> comment before
>>
>> + if (DECL_FUNCTION_CODE (callee) == BUILT_IN_CALLOC)
>> + {
>> + tree arg0 = gimple_call_arg (stmt, 0);
>>
>> explaining this is used by DSE to detect redundant stores.
> Agreed. I should have done this given I called it out in the email.
>
> Jeff
>
