On Wed, Mar 01, 2017 at 03:03:29PM +0100, Richard Biener wrote:
> One issue with the patch is duplicate warnings as TREE_NO_WARNING
> doesn't work very well on tcc_reference trees which are not
> shared. A followup could use some sort of hash table to mitigate
Can't we use TREE_NO_WARNING on get_base_address instead? That is
shared...
> @@ -2925,14 +2927,22 @@ walk_aliased_vdefs_1 (ao_ref *ref, tree
> if (!*visited)
> *visited = BITMAP_ALLOC (NULL);
> for (i = 0; i < gimple_phi_num_args (def_stmt); ++i)
> - cnt += walk_aliased_vdefs_1 (ref, gimple_phi_arg_def (def_stmt, i),
> - walker, data, visited, 0,
> - function_entry_reached);
> + {
> + int res = walk_aliased_vdefs_1 (ref,
> + gimple_phi_arg_def (def_stmt, i),
> + walker, data, visited, 0,
> + function_entry_reached, limit);
> + if (res == -1)
> + return -1;
> + cnt += res;
> + }
This doesn't look right. The recursive call starts with cnt of 0 again,
so if say you have limit 20 and cnt 10 when you are about to recurse and
that walk does 15 oracle comparisons, it won't return -1, but 15 and
you cnt += 15 to get 25 and never reach the limit, so then you walk perhaps
another 100000 times.
Can't you just pass cnt instead of 0 and then use
if (res == -1)
return -1;
cnt = res;
? Or you'd need to play gaves with decrementing the limit for the recursive
call.
> + /* For limiting the alias walk below we count all
> + vdefs in the function. We then give the alias walk an
> + overall budget (and simply not warn for further stmts).
> + ??? The interface doesn't give us the chance to assign
> + a maximum cost to each walk (and abort the walk if hit). */
I don't understand the last two lines, I thought you've added the ability
to the interface above and you actually use it.
Otherwise it LGTM, yes, we will end up with new warnings, but unless the
alias oracle is wrong, there shouldn't be too many false positives, right?
Jakub
