On 07/23/2018 02:18 AM, Jakub Jelinek wrote:
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.
I committed the fix quickly to unblock those whose bootstrap was
failing due the ILP32 warning. That seems like the right thing
to do, certainly preferable to reverting a large patch due to
a trivial oversight.
The fix doesn't do anything different than what the original
approved solution does. It simply applies the same approach
used elsewhere to this instance of the warning.
There are outstanding test failures on ILP32 due to a bug or
limitation in the original approach (I raised bug 86631 for
those yesterday and I will look into those this week).
The patch makes very different behavior by default between 32-bit and 64-bit
targets, that is certainly undesirable.
The difference is unintentional (see bug 86631).
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?);
Warnings about excessive allocation (of any kind) have been
enabled by default for a couple of releases now under
-Walloc-size-larger-than=. As I explained in the patch
description, my goal was not to change the limit under which
they trigger, at least not initially, or the logic that controls
the conditions under which they are issued.
The instance of the warning that showed up in the ILP32 build
is not intended to be enabled by default. (It complains when
the pass is unable to determine whether an alloca call is
bounded). There either were no tests that exercise it in
the LP64 test suite so I missed adding the same condition for
it as for the other instances of it that are not meant to be
enabled by default. The patch I committed yesterday simply
adds that condition.
Martin
