On 08/22/2018 04:34 PM, Jeff Law wrote:
On 08/22/2018 11:22 AM, Bernd Edlinger wrote:
On 08/22/18 18:05, Martin Sebor wrote:
On 08/21/2018 10:05 PM, Bernd Edlinger wrote:
On 08/22/18 00:25, Jeff Law wrote:
On 08/21/2018 02:43 AM, Richard Biener wrote:
On Mon, 20 Aug 2018, Bernd Edlinger wrote:
[ snip. ]
Yes, I found some peanuts on my way.
For instance this fix for PR middle-end/86121 survives bootstrap on
it's own, and fixes one xfail.
Is it OK for trunk?
Yes, that's OK for trunk.
Agreed. Seems like a nice independent bugfix and I don't think it
adversely affects anything else under current discussion. In fact, not
folding here makes it easier to warn about incorrect code elsewhere.
Btw, I don't think we want sth like
flag_assume_zero_terminated_char_arrays or even make it default at
-Ofast.
Yes, I agree. Is there a consensus about this?
Well, it's my own opinion of course. Show me a benchmark that
improves with -fassume-zero-terminated-char-arrays. Certainly
for security reasons it sounds a dangerous thing (and the documentation
needs a more thorough description of what it really means).
I certainly don't want to see a flag. We've already got way too many;
adding another for marginal behavior just seems wrong.
If yes, I go ahead and remove that option again.
BTW: I needed this option in a few test cases, that insist in checking the
optimizer to eliminate stuff, based on the VRP info. (6 +/-1 or so).
Any example?
But we can as well remove those test cases.
Bernd, if there are specific tests that you want to see removed, we
should discuss them.
The test cases are:
gcc.dg/strlenopt-36.c
There are plenty of valid test cases in this test. For example:
extern char a7[7];
if (strlen (a7) >= 7) // fold to false
abort ();
Even if we wanted to accommodate common definitions the array
declarations could be changed to static and the tests would
be useful:
static char a7[7];
There is no valid program where the if condition could be true.
gcc.dg/strlenopt-40.c
There are even more completely uncontroversial test cases here,
such as:
if (strlen (i < 0 ? "123" : "4321") > 4) // fold to false
abort ();
I see, the trouble is that the test case mixes valid cases with
cases that depend on type info in GIMPLE.
I believe Martin's point is that there are tests within those files that
are still valid. We don't want to zap the entire test unless all the
subtests are invalid. We need to look at each sub-test and determine if
it's valid or not.
Right. And if these changes extend to sprintf as I expect will
be the case there will be many more adjustments to make to those
tests. Those tests are quite delicate.
As I think Jeff already implied, I would really prefer to tackle
this work myself, both to make sure that it's done without
compromising existing warnings, and that the future enhancements
we have planned in these areas are made possible without too much
churn.
I expect to be able to get to it after I get back from Cauldron.
Martin
