On Wed, Jun 13, 2018 at 09:58:49AM +0200, Jakub Jelinek wrote:
> I'm actually worried about the fold-const.c change and don't understand, why
> it has been done at all in the context of this PR.
>
> case SAVE_EXPR:
> if (flags & OEP_LEXICOGRAPHIC)
> return OP_SAME (0);
> return 0;
>
> means it does something different from the state before the patch:
> default:
> return 0;
> only if OEP_LEXICOGRAPHIC is set, which AFAIK is only in the
> -Wduplicated-branches warning code, so why it is needed for
> -Wstringop-truncation warning is unclear to me. More importantly,
> especially -fsanitize=undefined has a tendency of creating trees
> which contain two or more uses of the same SAVE_EXPR at each level, and very
> deep nesting levels of such SAVE_EXPRs, so if we handle it without checking
> for duplicates, it results in exponential compile time complexity.
> So, if we really want to handle SAVE_EXPR here, we'd need to create some
> cache where we'd remember if in the same toplevel operand_equal_p call we've
> already compared two particular SAVE_EXPRs and what was the result.
Random testcase for -Wduplicated-branches -fsanitize=shift:
int
foo (int x, int y)
{
if (x)
y = y << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
<< 1 >> 1
<< 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
<< 1 >> 1
<< 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
<< 1 >> 1
<< 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
<< 1 >> 1;
else
y = y << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
<< 1 >> 1
<< 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
<< 1 >> 1
<< 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
<< 1 >> 1
<< 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
<< 1 >> 1;
return y;
}
Though it seems we have that problem already in inchash::add_expr. In that
case perhaps we could have a pointer to a hashmap in inchash::hash objects,
clear it in the ctors and destroy/clear in inchash::hash::end (), though we
have the add_commutative that has two separate hash objects.
Jakub
