On Mon, 29 Sep 2025, Jakub Jelinek wrote:
> On Sat, Sep 27, 2025 at 03:21:17PM +0100, Jason Merrill wrote:
> > The Wstringop-overflow testcase change seems like an instance of a
> > preexisting bug; C++98-only xfails seem to come and go in this testcase.
> > Initially I thought this was because in C++11 and above we throw on
> > allocation size multiplication overflow, but in C++98 we saturate--but
> > -fno-exceptions makes us saturate in C++11, and we still only get the
> > warning in C++98. The tree dumps look the same until cddce1, at which point
> > we get this difference:
> >
> > (C++11) Assume loop 3 to be finite: it has an exit and -ffinite-loops is on
> > or loop was previously finite.
> > (C++98) Estimating # of iterations of loop 3
> > cannot prove finiteness of loop 3
> >
> > And the dumps diverge significantly from that point. It seems rather
> > unfortunate that the optimizers aren't able to prove finiteness of loops
> > produced by build_vec_init; how can we improve that?
>
> This is quite unfortunate testcase.
>
> The C++98 to C++11 difference on it is that for C++11 and above we default
> to -ffinite-loops and for C++98 we don't. That seems reasonable to me,
> only C++11 and later have something which resembles current [intro.progress]
> in some way, I don't see any spots mentioning forward progress in C++98.
>
> On the C++ FE side, the big question is if emitting the {CLOBBER(bob)}
> clobbers is useful in all cases for array new expressions.
>
> There are 3 cases.
>
> One is if the whole size is constant.
> IMHO it is much better in that case to emit a single bob clobber covering
> the whole array first before the loop (build_vec_init), using a MEM_REF
> with say the right array type. I think this would be beneficial both
> for the case where there is some non-trivial constructor or when there is
> nothing else.
>
> Another case is when the whole allocated size is variable but there is
> non-trivial constructor for the elements.
> We can't emit a VLA MEM_REF CLOBBER before the loop, and if not anything
> else, the {CLOBBER(bob)} in the loop will at least help the non-trivial
> ctor right after it in the loop.
>
> The last case is non-constant allocation size and trivial (vacuous?)
> construction. This is what happens in the testcase, and to me it seems
> at least currently the middle-end can't make any use of it and just
> drops it in cddce1 pass, without anything benefiting from the clobber
> (this is before complete unrolling which I think would be the only
> way to make some use of it, but we are talking about non-constant
> allocation sizes, so unlikely that would happen anyway unless after inlining
> we figure out the size is now constant).
>
> And this particular case is what happens in the testcase.
>
> For C++11 (or C++98 with -ffinite-loops) we figure out that after removing
> the inner loop which did have just CLOBBERs on the inner array elements
> we have a loop which has no side-effects in the body and is not a trivial
> infinite loop, has a conditional exit, and we just throw it away completely.
> In the testcase, we have for that particular line new char[r_0_1][2]
> and evrp 2 passes earlier figures out that r_0_1 is in the [0, 1] range.
>
> Before evrp we have the loop nest like:
> <bb 17> :
> *_15 ={v} {CLOBBER(bob)};
> _102 = _16 + -1;
> _103 = _15 + 1;
>
> <bb 18> :
> # _15 = PHI <_13(20), _103(17)>
> # _16 = PHI <1(20), _102(17)>
> if (_16 >= 0)
> goto <bb 17>; [INV]
> else
> goto <bb 19>; [INV]
>
> <bb 19> :
> _99 = _14 + -1;
> _100 = _13 + 2;
>
> <bb 20> :
> # _13 = PHI <_63(16), _100(19)>
> # _14 = PHI <_41(16), _99(19)>
> if (_14 >= 0)
> goto <bb 18>; [INV]
> else
> goto <bb 21>; [INV]
> Now, evrp figures out that _14 has [-1, 0] range (it is r_0_1 - 1) and
> changes the
> # _14 = PHI <_41(16), _99(19)>
> if (_14 >= 0)
> to
> # _14 = PHI <_41(14), -1(17)>
> if (_14 == 0)
> lines in there to, which is reasonable. We still have a loop which iterates
> clearly 0 or 1 times and does nothing otherwise but we don't optimixe it
> away just yet.
>
> cddce1 then probably just uses the finite_p flag on loops (so only C++11)
> and doesn't actually try to compute number of iterations and remove "empty"
> loops with constant small upper bound of iterations, and we end up with
> # RANGE [irange] long int [-1, 0]
> _9 = (long int) _1;
> ...
> <bb 9> [local count: 1073741824]:
> # RANGE [irange] long int [-1, 0]
> # _4 = PHI <_9(8), -1(14)>
> if (_4 == 0)
> goto <bb 14>; [89.00%]
> else
> goto <bb 10>; [11.00%]
>
> <bb 14> [local count: 955630224]:
> goto <bb 9>; [100.00%]
> kind of dummy loops which don't do anything for quite a lot of passes.
> And finally jump threading comes and because one function contains e.g. 4
> or more of these one after another in the test (
> void test_strcpy_new_char_array (size_t n)
> {
> size_t r_0_1 = UR (0, 1);
>
> T (S (0), new char[r_0_1][1]);
> T (S (1), new char[r_0_1][1]); // { dg-warning "\\\[-Wstringop-overflow"
> }
> T (S (1), new char[r_0_1][2]);
> T (S (2), new char[r_0_1][2]); // { dg-warning "\\\[-Wstringop-overflow"
> }
> }
> still reproduces it), it threads it by making separate paths for the r_0_1
> == 0 and r_0_1 == 1 and guess the false positive is then on dead code or
> something.
>
> Anyway, on the C++ FE side I'd really recommend (but can be done
> incrementally of course) to emit one larger clobber for all the whole array
> if it has constant sizes and not emit clobbers at least for now at all if
> there is trivial constructor for the elements and we'd emit the clobbers in
> a loop.
>
> And on the middle-end side, the question is if either cddce1 should compute
> number of iterations (upper bound) for loops with empty bodies which aren't
> finite_p and optimize them away too, or if some other pass later on when
> number of iterations is already computed shouldn't repeat that.
The finite_loop_p query should already do this (if scev_initialized (),
which it is, for CD-DCE).
Richard.
> Jakub
>
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)