On 9/22/21 8:21 AM, Martin Sebor wrote:
On 9/21/21 7:38 PM, Hongtao Liu wrote:
On Mon, Sep 20, 2021 at 4:13 AM Martin Sebor <mse...@gmail.com> wrote:
...
diff --git a/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c
b/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c
index 1d79930cd58..9351f7e7a1a 100644
--- a/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c
+++ b/gcc/testsuite/c-c++-common/Wstringop-overflow-2.c
@@ -1,7 +1,7 @@
/* PR middle-end/91458 - inconsistent warning for writing past
the end
of an array member
{ dg-do compile }
- { dg-options "-O2 -Wall -Wno-array-bounds -fno-ipa-icf" } */
+ { dg-options "-O2 -Wall -Wno-array-bounds -fno-ipa-icf
-fno-tree-vectorize" } */
The testcase is large - what part requires this change? Given the
testcase was added for inconsistent warnings do they now become
inconsistent again as we enable vectorization at -O2?
That said, the testcase adjustments need some explaining - I suppose
you didn't just slap -fno-tree-vectorize to all of those changing
behavior?
void ga1_ (void)
{
a1_.a[0] = 0;
a1_.a[1] = 1; // { dg-warning
"\\\[-Wstringop-overflow" }
a1_.a[2] = 2; // { dg-warning
"\\\[-Wstringop-overflow" }
struct A1 a;
a.a[0] = 0;
a.a[1] = 1; // { dg-warning
"\\\[-Wstringop-overflow" }
a.a[2] = 2; // { dg-warning
"\\\[-Wstringop-overflow" }
sink (&a);
}
It's supposed to be 2 warning for a.a[1] = 1 and a.a[2] = 1 since
there are 2 accesses, but after enabling vectorization, there's only
one access, so one warning is missing which causes the failure.
With the stores vectorized, is the warning on the correct line or
does it point to the first store, the one that's in bounds, as
it does with -O3? The latter would be a regression at -O2.
I would find it preferable to change the test code over disabling
optimizations that are on by default. My concern is that the test
would no longer exercise the default behavior. (The same goes for
the -fno-ipa-icf option.)
Hmm, it's a middle-end test, for some backend, it may not do
vectorization(it depends on TARGET_VECTOR_MODE_SUPPORTED_P and
relative cost model).
Yes, there are quite a few warning tests like that. Their main
purpose is to verify that in common GCC invocations (i.e., without
any special options) warnings are a) issued when expected and b)
not issued when not expected. Otherwise, middle end warnings are
known to have both false positives and false negatives in some
invocations, depending on what optimizations are in effect.
Indiscriminately disabling common optimizations for these large
tests and invoking them under artificial conditions would
compromise this goal and hide the problems.
If enabling vectorization at -O2 causes regressions in the quality
of diagnostics (as the test failure above indicates seems to be
happening) we should investigate these and open bugs for them so
they can be fixed. We can then tweak the specific failing test
cases to avoid the failures until they are fixed.
To expand on the last part: in my tests with -O2 and -O3 the failure
is specific to char stores and doesn't affect stores of larger types
because they're not detected by -Wstringop-overflow. Those accesses
are handled by -Warray-bounds. The former runs as part of the strlen
and after vectorization, while the latter runs in vrp1 and before
vectorization. So the "quick and dirty" solution here, to keep
the warnings on the right lines, might be to move the char store
handling from the strlen pass to vrp1. A more robust solution is
to avoid vectorizing (or merging) out of bounds stores.
Martin
