Re: [patch match.pd c c++]: Ignore results of 'shorten_compare' and move missing patterns in match.pd

[Jeff Law <law at redhat dot com>]

On 09/08/2015 05:17 AM, Kai Tietz wrote:
> Hi,
>
> This patch is the first part of obsoleting 'shorten_compare' function
> for folding.
> It adjusts the uses of 'shorten_compare' to ignore folding returned by
> it, and adds
> missing pattterns to match.pd to allow full bootstrap of C/C++ without
> regressions.
> Due we are using 'shorten_compare' for some diagnostic we can't simply
> remove it.  So if this patch gets approved, the next step will be to
> rename the function to something like 'check_compare', and adjust its
> arguments and inner logic to reflect that we don't modify
> arguments/expression anymore within that function.
>
> Bootstrap just show 2 regressions within gcc.dg testsuite due patterns
> matched are folded more early by forward-propagation.  I adjusted
> them, and added them to patch, too.
>
> I did regression-testing for x86_64-unknown-linux-gnu.
>
> ChangeLog
>
> 2015-09-08  Kai Tietz  <ktietz@redhat.com>
>
>      * match.pd: Add missing patterns from shorten_compare.
>      * c/c-typeck.c (build_binary_op): Discard foldings of shorten_compare.
>      * cp/typeck.c (cp_build_binary_op): Likewise.
>
> 2015-09-08  Kai Tietz  <ktietz@redhat.com>
>
>      * gcc.dg/tree-ssa/vrp23.c: Adjust testcase to reflect that
>      pattern is matching now already within forward-propagation pass.
>      * gcc.dg/tree-ssa/vrp24.c: Likewise.
So for the new patterns, I would have expected testcases to ensure 
they're getting used.

The fact that we're not regressing with the front-end specific 
shortening disabled like this is probably more of an artifact of lack of 
testing of this feature than anything.

In *theory* one ought to be able to look at the dumps or .s files before 
after this patch for a blob of tests and see that nothing significant 
has changed.  Unfortunately, so much changes that it's hard to evaluate 
if this patch is a step forward or a step back.

I wonder if we'd do better to first add new match.pd patterns, one at a 
time, with tests, and evaluating them along the way by looking at the 
dumps or .s files across many systems.  Then when we think we've got the 
right set, then look at what happens to those dumps/.s files if we make 
the changes so that shorten_compare really just emits warnings.

My worry is that we get part way through the conversion and end up with 
the match.pd patterns without actually getting shorten_compare cleaned 
up and turned into just a warning generator.

jeff