On Fri, 15 Nov 2013, Richard Biener wrote:
> On Fri, 15 Nov 2013, Jakub Jelinek wrote:
>
> > On Fri, Nov 15, 2013 at 02:56:51PM +0100, Richard Biener wrote:
> > > Now that there is (finally :() a wrong-code testcase for the
> > > PR54570 issue we can no longer ignore it (bah). So the following
> > > tries to paper over the fact that object-size sucks and disables
> > > value-numbering of equal addresses the same before that pass
> > > had a chance to finally look at the structure of the addresses.
> > >
> > > To make this "fix" suck less I moved the object-size pass before
> > > the final FRE pass runs which is after IPA inlining and the
> > > propagation of constants and addresses. You won't catch
> > > any "improvements" you'd get by memory CSE opportunities that
> > > IPA inlining exposes, but you cannot have everything here.
> >
> > If it doesn't regress anything in the testsuite, I guess that is ok.
> >
> > > (IMHO object-size would should run during early optimizations)
> >
> > It can't, because inlining and some limited cleanup afterwards is essential
> > for it. Otherwise you'd regress not just for __builtin_object_size (x, 1),
> > which admittedly is problematic since the introduction of MEM_REFs and
> > various other changes, but also for __builtin_object_size (x, 0), which
> > would be much more important.
> >
> > As discussed earlier, perhaps instead of checking cfun->after_inlining
> > you could just introduce a new flag whether cfun contains any
> > __builtin_object_size (x, {1,3}) calls, initialized by the gimplifier,
> > propagated by the inliner and finally cleared again by objsz pass.
>
> But you'd need to pessimistically initialize it because if you inline
> into a function with __builtin_object_size you may not previously
> optimize. You can of course analyze the cgraph to clear it for
> functions that cannot end up being inlined into such function.
> So that's effectively the same as ->after_inlining
> minus losing the optimization that didn't end up with
> __builtin_object_size after that.
>
> Not sure if it's worth all the trouble. Arriving at a better
> design for computing __builtin_object_size would be better ;)
> (not that I have one)
>
> > Of course, if moving objsz earlier seems to work, it could stay where you
> > put it, but the flag could make it clearer why you want to avoid certain
> > optimizations.
>
> Well, all object-size testcases are pretty simplistic right now and
> don't trigger the IPA inliner for example.
>
> > > Bootstrap / regtest pending on x86_64-unknown-linux-gnu.
> > >
> > > Similar candidate for the 4.8 branch.
> >
> > Please wait sufficiently long for trunk issues before you backport.
>
> Of course.
So I had to do some more changes because doing objsz earlier (and
separating it from strlenopt) exposes that the GENERIC folders (ugh)
for strcat mess up points-to info as it folds strcat to
memcpy (SAVE_EXPR <dest + strlen ()>, ...) which adds a new
SSA name for the destination with no alias info associated.
Rather than moving the non-constant builtin foldings to GIMPLE
I chose to make sure to forward the constants in objsz and fold
the stmts there (as PTA is re-run shortly after it).
This then breaks some of the scan-tree-dumps in the strlenopt
testcases because nobody has previously folded the _chk builtins
with -1U size to non-_chk variants, so I have to adjust them
(I didn't want to move the strlenopt pass as well).
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
Any objections?
Thanks,
Richard.
2013-11-18 Richard Biener <[email protected]>
PR tree-optimization/59125
PR tree-optimization/54570
* tree-ssa-sccvn.c (copy_reference_ops_from_ref): When inlining
is not complete do not treat component-references with offset zero
but different fields as equal.
* tree-object-size.c: Include tree-phinodes.h and ssa-iterators.h.
(compute_object_sizes): Apply TLC. Propagate the constant
results into all uses and fold their stmts.
* passes.def (pass_all_optimizations): Move pass_object_sizes
after the first pass_forwprop and before pass_fre.
* gcc.dg/builtin-object-size-8.c: Un-xfail.
* gcc.dg/builtin-object-size-14.c: New testcase.
* gcc.dg/strlenopt-14gf.c: Adjust.
* gcc.dg/strlenopt-1f.c: Likewise.
* gcc.dg/strlenopt-4gf.c: Likewise.
Index: gcc/tree-ssa-sccvn.c
===================================================================
*** gcc/tree-ssa-sccvn.c.orig 2013-11-18 11:20:36.000000000 +0100
--- gcc/tree-ssa-sccvn.c 2013-11-18 11:29:12.300337512 +0100
*************** copy_reference_ops_from_ref (tree ref, v
*** 760,766 ****
}
/* For non-calls, store the information that makes up the address. */
!
while (ref)
{
vn_reference_op_s temp;
--- 760,766 ----
}
/* For non-calls, store the information that makes up the address. */
! tree orig = ref;
while (ref)
{
vn_reference_op_s temp;
*************** copy_reference_ops_from_ref (tree ref, v
*** 810,816 ****
+ tree_to_double_int (bit_offset)
.rshift (BITS_PER_UNIT == 8
? 3 : exact_log2 (BITS_PER_UNIT));
! if (off.fits_shwi ())
temp.off = off.low;
}
}
--- 810,819 ----
+ tree_to_double_int (bit_offset)
.rshift (BITS_PER_UNIT == 8
? 3 : exact_log2 (BITS_PER_UNIT));
! if (off.fits_shwi ()
! && (TREE_CODE (orig) != ADDR_EXPR
! || !off.is_zero ()
! || cfun->after_inlining))
temp.off = off.low;
}
}
Index: gcc/passes.def
===================================================================
*** gcc/passes.def.orig 2013-11-18 11:20:36.000000000 +0100
--- gcc/passes.def 2013-11-18 11:29:12.305337567 +0100
*************** along with GCC; see the file COPYING3.
*** 142,147 ****
--- 142,148 ----
form if possible. */
NEXT_PASS (pass_phiprop);
NEXT_PASS (pass_forwprop);
+ NEXT_PASS (pass_object_sizes);
/* pass_build_alias is a dummy pass that ensures that we
execute TODO_rebuild_alias at this point. */
NEXT_PASS (pass_build_alias);
*************** along with GCC; see the file COPYING3.
*** 185,191 ****
NEXT_PASS (pass_dce);
NEXT_PASS (pass_forwprop);
NEXT_PASS (pass_phiopt);
- NEXT_PASS (pass_object_sizes);
NEXT_PASS (pass_strlen);
NEXT_PASS (pass_ccp);
/* After CCP we rewrite no longer addressed locals into SSA
--- 186,191 ----
Index: gcc/testsuite/gcc.dg/builtin-object-size-8.c
===================================================================
*** gcc/testsuite/gcc.dg/builtin-object-size-8.c.orig 2013-11-18
11:20:36.000000000 +0100
--- gcc/testsuite/gcc.dg/builtin-object-size-8.c 2013-11-18
11:29:12.305337567 +0100
***************
*** 1,4 ****
! /* { dg-do run { xfail *-*-* } } */
/* { dg-options "-O2" } */
typedef __SIZE_TYPE__ size_t;
--- 1,4 ----
! /* { dg-do run } */
/* { dg-options "-O2" } */
typedef __SIZE_TYPE__ size_t;
Index: gcc/testsuite/gcc.dg/builtin-object-size-14.c
===================================================================
*** /dev/null 1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.dg/builtin-object-size-14.c 2013-11-18
11:29:12.306337579 +0100
***************
*** 0 ****
--- 1,28 ----
+ /* { dg-do run } */
+ /* { dg-options "-O2" } */
+
+ extern void abort (void);
+ extern char *strncpy(char *, const char *, __SIZE_TYPE__);
+
+ union u {
+ struct {
+ char vi[8];
+ char pi[16];
+ };
+ char all[8+16+4];
+ };
+
+ void __attribute__((noinline,noclone))
+ f(union u *u)
+ {
+ char vi[8+1];
+ __builtin_strncpy(vi, u->vi, sizeof(u->vi));
+ if (__builtin_object_size (u->all, 1) != -1)
+ abort ();
+ }
+ int main()
+ {
+ union u u;
+ f (&u);
+ return 0;
+ }
Index: gcc/tree-object-size.c
===================================================================
*** gcc/tree-object-size.c.orig 2013-11-18 11:20:36.000000000 +0100
--- gcc/tree-object-size.c 2013-11-18 11:29:12.306337579 +0100
*************** along with GCC; see the file COPYING3.
*** 32,37 ****
--- 32,39 ----
#include "tree-ssanames.h"
#include "tree-pass.h"
#include "tree-ssa-propagate.h"
+ #include "tree-phinodes.h"
+ #include "ssa-iterators.h"
struct object_size_info
{
*************** compute_object_sizes (void)
*** 1207,1222 ****
gimple_stmt_iterator i;
for (i = gsi_start_bb (bb); !gsi_end_p (i); gsi_next (&i))
{
! tree callee, result;
gimple call = gsi_stmt (i);
!
! if (gimple_code (call) != GIMPLE_CALL)
! continue;
!
! callee = gimple_call_fndecl (call);
! if (!callee
! || DECL_BUILT_IN_CLASS (callee) != BUILT_IN_NORMAL
! || DECL_FUNCTION_CODE (callee) != BUILT_IN_OBJECT_SIZE)
continue;
init_object_sizes ();
--- 1209,1217 ----
gimple_stmt_iterator i;
for (i = gsi_start_bb (bb); !gsi_end_p (i); gsi_next (&i))
{
! tree result;
gimple call = gsi_stmt (i);
! if (!gimple_call_builtin_p (call, BUILT_IN_OBJECT_SIZE))
continue;
init_object_sizes ();
*************** compute_object_sizes (void)
*** 1245,1264 ****
continue;
}
if (dump_file && (dump_flags & TDF_DETAILS))
{
fprintf (dump_file, "Simplified\n ");
print_gimple_stmt (dump_file, call, 0, dump_flags);
}
! if (!update_call_from_tree (&i, result))
! gcc_unreachable ();
! if (dump_file && (dump_flags & TDF_DETAILS))
{
! fprintf (dump_file, "to\n ");
! print_gimple_stmt (dump_file, gsi_stmt (i), 0, dump_flags);
! fprintf (dump_file, "\n");
}
}
}
--- 1240,1271 ----
continue;
}
+ gcc_assert (TREE_CODE (result) == INTEGER_CST);
+
if (dump_file && (dump_flags & TDF_DETAILS))
{
fprintf (dump_file, "Simplified\n ");
print_gimple_stmt (dump_file, call, 0, dump_flags);
+ fprintf (dump_file, " to ");
+ print_generic_expr (dump_file, result, 0);
+ fprintf (dump_file, "\n");
}
! tree lhs = gimple_call_lhs (call);
! if (!lhs)
! continue;
! /* Propagate into all uses and fold those stmts. */
! gimple use_stmt;
! imm_use_iterator iter;
! FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs)
{
! use_operand_p use_p;
! FOR_EACH_IMM_USE_ON_STMT (use_p, iter)
! SET_USE (use_p, result);
! gimple_stmt_iterator gsi = gsi_for_stmt (use_stmt);
! if (fold_stmt (&gsi))
! update_stmt (gsi_stmt (gsi));
}
}
}
Index: gcc/testsuite/gcc.dg/strlenopt-14gf.c
===================================================================
*** gcc/testsuite/gcc.dg/strlenopt-14gf.c.orig 2013-06-11 09:32:56.000000000
+0200
--- gcc/testsuite/gcc.dg/strlenopt-14gf.c 2013-11-18 11:33:46.303480430
+0100
***************
*** 11,24 ****
memcpy. */
/* { dg-final { scan-tree-dump-times "strlen \\(" 4 "strlen" } } */
/* { dg-final { scan-tree-dump-times "__memcpy_chk \\(" 0 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "__mempcpy_chk \\(" 2 "strlen" } } */
/* { dg-final { scan-tree-dump-times "__strcpy_chk \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "__strcat_chk \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "strchr \\(" 0 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "__stpcpy_chk \\(" 3 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "memcpy \\(" 0 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "mempcpy \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "strcpy \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "strcat \\(" 0 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "stpcpy \\(" 0 "strlen" } } */
/* { dg-final { cleanup-tree-dump "strlen" } } */
--- 11,24 ----
memcpy. */
/* { dg-final { scan-tree-dump-times "strlen \\(" 4 "strlen" } } */
/* { dg-final { scan-tree-dump-times "__memcpy_chk \\(" 0 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "__mempcpy_chk \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "__strcpy_chk \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "__strcat_chk \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "strchr \\(" 0 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "__stpcpy_chk \\(" 0 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "memcpy \\(" 1 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "mempcpy \\(" 2 "strlen" } } */
/* { dg-final { scan-tree-dump-times "strcpy \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "strcat \\(" 0 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "stpcpy \\(" 2 "strlen" } } */
/* { dg-final { cleanup-tree-dump "strlen" } } */
Index: gcc/testsuite/gcc.dg/strlenopt-1f.c
===================================================================
*** gcc/testsuite/gcc.dg/strlenopt-1f.c.orig 2013-11-15 15:49:12.000000000
+0100
--- gcc/testsuite/gcc.dg/strlenopt-1f.c 2013-11-18 11:30:53.643500139 +0100
***************
*** 6,17 ****
#include "strlenopt-1.c"
/* { dg-final { scan-tree-dump-times "strlen \\(" 2 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "__memcpy_chk \\(" 4 "strlen" } } */
/* { dg-final { scan-tree-dump-times "__strcpy_chk \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "__strcat_chk \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "strchr \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "__stpcpy_chk \\(" 0 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "memcpy \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "strcpy \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "strcat \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "stpcpy \\(" 0 "strlen" } } */
--- 6,17 ----
#include "strlenopt-1.c"
/* { dg-final { scan-tree-dump-times "strlen \\(" 2 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "__memcpy_chk \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "__strcpy_chk \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "__strcat_chk \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "strchr \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "__stpcpy_chk \\(" 0 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "memcpy \\(" 4 "strlen" } } */
/* { dg-final { scan-tree-dump-times "strcpy \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "strcat \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "stpcpy \\(" 0 "strlen" } } */
Index: gcc/testsuite/gcc.dg/strlenopt-4gf.c
===================================================================
*** gcc/testsuite/gcc.dg/strlenopt-4gf.c.orig 2013-06-11 09:32:55.000000000
+0200
--- gcc/testsuite/gcc.dg/strlenopt-4gf.c 2013-11-18 11:31:12.352714788
+0100
***************
*** 7,19 ****
#include "strlenopt-4.c"
/* { dg-final { scan-tree-dump-times "strlen \\(" 1 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "__memcpy_chk \\(" 4 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "__strcpy_chk \\(" 1 "strlen" } } */
/* { dg-final { scan-tree-dump-times "__strcat_chk \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "strchr \\(" 0 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "__stpcpy_chk \\(" 5 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "memcpy \\(" 0 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "strcpy \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "strcat \\(" 0 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "stpcpy \\(" 0 "strlen" } } */
/* { dg-final { cleanup-tree-dump "strlen" } } */
--- 7,19 ----
#include "strlenopt-4.c"
/* { dg-final { scan-tree-dump-times "strlen \\(" 1 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "__memcpy_chk \\(" 0 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "__strcpy_chk \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "__strcat_chk \\(" 0 "strlen" } } */
/* { dg-final { scan-tree-dump-times "strchr \\(" 0 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "__stpcpy_chk \\(" 0 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "memcpy \\(" 4 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "strcpy \\(" 1 "strlen" } } */
/* { dg-final { scan-tree-dump-times "strcat \\(" 0 "strlen" } } */
! /* { dg-final { scan-tree-dump-times "stpcpy \\(" 5 "strlen" } } */
/* { dg-final { cleanup-tree-dump "strlen" } } */
