On Sun, Jul 22, 2018 at 04:43:17PM -0600, Martin Sebor wrote:
> > > > OK with the nit fixed.  IF you need to update the doc changes as a
> > > > result of the -faligned-* changes, those are pre-approved.
> > > 
> > > 
> > > I had to adjust a few more tests and make a couple of minor
> > > tweaks as I noticed a coupled of test failures that I had
> > > previously missed but the result was didn't show any more
> > > regressions on x86_64.
> > > 
> > > Committed in r262910.
> > > 
> > 
> > This caused:
> > 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86624
> > 
> > This may impact all 32-bit hosts.
> 
> r262923 should have fixed it.

You haven't posted the patch you've committed, nobody has reviewed it and it
caused quite a lot regressions:
make check-gcc check-c++-all RUNTESTFLAGS="--target_board=unix\{-m32,-m64\} 
dg.exp='*alloc* pr42611.c'"
One can revert his own patch or parts of it, but for doing something
completely different you need to seek approval.

The patch makes very different behavior by default between 32-bit and 64-bit
targets, that is certainly undesirable.

And as mentioned in the PR, I'm still arguing we should not make these
warnings on by default (even -Wall/-Wextra is questionable, but by
default?); either you intend not to warn about anything with the default
setting, but then it is a a waste of compile time resources to run the
walloca pass even when it will do nothing, or you do warn about some cases,
but that is something that is undesirable for a code style probabilistic
warning; the compiler is quiet only if one chooses to explicitly (and close
to the uses) guard all alloca calls with some bound; it is a good thing for
codebases that decide to require such coding style, but for others it is
enough that the programmers know it is bounded, or are ok with any sizes as
long as there is enough stuck (say gcc itself, requiring bounding of alloca
makes no sense, we have lots of ways for unbounded recursion that eats stuck
exactly the same way, etc.).  And by using VRP as the base of its decisions,
it can be some check in third-party inline used in unrelated other part of
code that affects whether the warning triggers or not.

        Jakub

Reply via email to