On 06/07/2018 02:57 PM, Jan Hubicka wrote:
>>
>> gcc/ChangeLog:
>>
>> 2018-04-24 Martin Liska <mli...@suse.cz>
>>
>> * ipa-pure-const.c (struct funct_state_d): Do it class instead
>> of struct.
>> (class funct_state_summary_t): New function_summary class.
>> (has_function_state): Remove.
>> (get_function_state): Likewise.
>> (set_function_state): Likewise.
>> (add_new_function): Likewise.
>> (funct_state_summary_t::insert): New function.
>> (duplicate_node_data): Remove.
>> (remove_node_data): Remove.
>> (funct_state_summary_t::duplicate): New function.
>> (register_hooks): Create new funct_state_summaries.
>> (pure_const_generate_summary): Use it.
>> (pure_const_write_summary): Likewise.
>> (pure_const_read_summary): Likewise.
>> (propagate_pure_const): Likewise.
>> (propagate_nothrow): Likewise.
>> (dump_malloc_lattice): Likewise.
>> (propagate_malloc): Likewise.
>> (execute): Do not register hooks, just remove summary
>> instead.
>> (pass_ipa_pure_const::pass_ipa_pure_const): Simplify
>> constructor.
>
> OK with changes below.
> In general, it would be cool to reorg this pass into simple SCC propagation
> template
> because it does same things over and over again (sometimes on slightly
> different graph
> because it has feature to skip uninteresting edges e.g. for nothrow
> propagation).
>> @@ -1485,7 +1439,7 @@ propagate_pure_const (void)
>> int i;
>> struct ipa_ref *ref = NULL;
>>
>> - funct_state w_l = get_function_state (w);
>> + funct_state w_l = funct_state_summaries->get_create (w);
>> if (dump_file && (dump_flags & TDF_DETAILS))
>> fprintf (dump_file, " Visiting %s state:%s looping %i\n",
>> w->dump_name (),
>> @@ -1527,7 +1481,7 @@ propagate_pure_const (void)
>> }
>> if (avail > AVAIL_INTERPOSABLE)
>> {
>> - funct_state y_l = get_function_state (y);
>> + funct_state y_l = funct_state_summaries->get_create (y);
>> if (dump_file && (dump_flags & TDF_DETAILS))
>> {
>> fprintf (dump_file,
>
> The functions are organized in a way that it goes cycle by cycle. First loop
> initializes
> everything in the cycle and in this case you want get_create (where w_l is
> set)
> y_l looks for calls and because it works in SCC order all calls are either in
> same cycle
> or processed. I do not see any guard checking that y_l is initialized and
> because you
> now initialize it to pesimistic state I think it will turn any SCC component
> into non-pure
> now (while it originaly worked by initializing to 0 which is optimistic state
> CONST)
>
> So i would add guard that y is in different SCC component and make code to
> crash
> if get() returns NULL.
> Similarly for all other propagators.
>
>> @@ -1642,7 +1596,7 @@ propagate_pure_const (void)
>> while (w && !can_free)
>> {
>> struct cgraph_edge *e;
>> - funct_state w_l = get_function_state (w);
>> + funct_state w_l = funct_state_summaries->get_create (w);
>>
>> if (w_l->can_free
>> || w->get_availability () == AVAIL_INTERPOSABLE
>> @@ -1657,7 +1611,7 @@ propagate_pure_const (void)
>> e->caller);
>>
>> if (avail > AVAIL_INTERPOSABLE)
>> - can_free = get_function_state (y)->can_free;
>> + can_free = funct_state_summaries->get_create (y)->can_free;
>> else
>> can_free = true;
>> }
>
> Here everything should be computed already.
>
> OK with those changes provides it does not affect generated code.
>
> Honza
>
Hi.
Ok, I'll do that with one additional patch that I'm sending.
Martin
>From 42f122845969bf39ec606a0592d7a511b87c4657 Mon Sep 17 00:00:00 2001
From: marxin <mli...@suse.cz>
Date: Fri, 8 Jun 2018 12:59:34 +0200
Subject: [PATCH] Make ipa-pure-const more strict about summary constrains.
gcc/ChangeLog:
2018-06-08 Martin Liska <mli...@suse.cz>
* ipa-pure-const.c (propagate_pure_const): Use ::get at places
where we expect an existing summary.
---
gcc/ipa-pure-const.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/gcc/ipa-pure-const.c b/gcc/ipa-pure-const.c
index 8c415bc1fc0..d0f9cb8d7f7 100644
--- a/gcc/ipa-pure-const.c
+++ b/gcc/ipa-pure-const.c
@@ -1477,7 +1477,7 @@ propagate_pure_const (void)
}
if (avail > AVAIL_INTERPOSABLE)
{
- funct_state y_l = funct_state_summaries->get_create (y);
+ funct_state y_l = funct_state_summaries->get (y);
if (dump_file && (dump_flags & TDF_DETAILS))
{
fprintf (dump_file,
@@ -1591,7 +1591,7 @@ propagate_pure_const (void)
while (w && !can_free)
{
struct cgraph_edge *e;
- funct_state w_l = funct_state_summaries->get_create (w);
+ funct_state w_l = funct_state_summaries->get (w);
if (w_l->can_free
|| w->get_availability () == AVAIL_INTERPOSABLE
@@ -1606,7 +1606,7 @@ propagate_pure_const (void)
e->caller);
if (avail > AVAIL_INTERPOSABLE)
- can_free = funct_state_summaries->get_create (y)->can_free;
+ can_free = funct_state_summaries->get (y)->can_free;
else
can_free = true;
}
@@ -1619,7 +1619,7 @@ propagate_pure_const (void)
w = node;
while (w)
{
- funct_state w_l = funct_state_summaries->get_create (w);
+ funct_state w_l = funct_state_summaries->get (w);
enum pure_const_state_e this_state = pure_const_state;
bool this_looping = looping;
--
2.17.0
