This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Fix builtin folding


Hi,
this problem kept me walking in circles for a few days...

I do have memcpy conversion patches in queue that do convert
memcpy(a,b,sizeof(*a)) into *a=*b,a in most circmuatens I've ported
to new Jakub's implementation that does this transformation for special cases.

What goes wrong is that when memcpy return value is ignored, IGNORED argument
is passed to memcpy folder. Jakub's implementation is cureful enought so the
",a" statement is stripped and memcpy is folded to an "*a=*b" assignment that
is all right.  However the tree-ssa-ccp calls convert_to_gimple_builtin on the
result that for some reason again expect every builtin to have used return
value and copy the result of last MODIFY_EXPR into temporary variable (this
time into newly constructed structure).  This create random gimple sharing on
"*a" that later leads to misoptimization rewriting value of "a" for Java's
lex.c compiled with some specific versions of bison that resulted in
misoptimizations of java testsuite reproducing on our automated tester but not
on my machine.

I've fixed the random sharing in internal_get_tmp_var as I think it is bad idea
overall, I also teached convert_to_gimple_builtin to ignore return value (it
leads to awful code including the temporary structure to copy into and one
(void *)0; statement for some reason) and also noticed that fold_builtin is
constructing NOP_EXPR to cast the result even when the return value is ignored.
This NOP_EXPR seems promptly stripped off on each use of fold_builtin with
nonzero value of IGNORE.

I must admit it is not quite clear to me why the NOP_EXPR with warnings
disabled is unconditionally generated - perhaps it is just to avoid warning on
code produced by such a crazy scheme as above....

I will send the memcpy bits in separate patch once full testing converge (I
know it now passes bootstrap+regtest)

:ADDPATCH middle-end:
Bootstrapped/regtested i686-linux, OK?

Honza

2006-10-26  Jan Hubicka  <jh@suse.cz>
	* builtins.c (fold_builtin): Don't generate NOP_EXPR that is going
	to be thrown away soon when IGNORE is set.
	* tree-ssa-ccp.c (convert_to_gimple_builtin): Add IGNORE argument
	indicating when return value shall not be computed.
	* gimplify.c (internal_get_tmp_var): Avoid random tree sharing.
Index: builtins.c
===================================================================
*** builtins.c	(revision 118067)
--- builtins.c	(working copy)
*************** tree
*** 9273,9279 ****
  fold_builtin (tree fndecl, tree arglist, bool ignore)
  {
    tree exp = fold_builtin_1 (fndecl, arglist, ignore);
!   if (exp)
      {
        exp = build1 (NOP_EXPR, TREE_TYPE (exp), exp);
        TREE_NO_WARNING (exp) = 1;
--- 9259,9265 ----
  fold_builtin (tree fndecl, tree arglist, bool ignore)
  {
    tree exp = fold_builtin_1 (fndecl, arglist, ignore);
!   if (exp && !ignore)
      {
        exp = build1 (NOP_EXPR, TREE_TYPE (exp), exp);
        TREE_NO_WARNING (exp) = 1;
Index: tree-ssa-ccp.c
===================================================================
*** tree-ssa-ccp.c	(revision 118067)
--- tree-ssa-ccp.c	(working copy)
*************** fold_stmt_inplace (tree stmt)
*** 2465,2481 ****
  
  /* Convert EXPR into a GIMPLE value suitable for substitution on the
     RHS of an assignment.  Insert the necessary statements before
!    iterator *SI_P.  */
  
  static tree
! convert_to_gimple_builtin (block_stmt_iterator *si_p, tree expr)
  {
    tree_stmt_iterator ti;
    tree stmt = bsi_stmt (*si_p);
    tree tmp, stmts = NULL;
  
    push_gimplify_context ();
!   tmp = get_initialized_tmp_var (expr, &stmts, NULL);
    pop_gimplify_context (NULL);
  
    if (EXPR_HAS_LOCATION (stmt))
--- 2465,2488 ----
  
  /* Convert EXPR into a GIMPLE value suitable for substitution on the
     RHS of an assignment.  Insert the necessary statements before
!    iterator *SI_P. 
!    When IGNORE is set, don't worry about the return value.  */
  
  static tree
! convert_to_gimple_builtin (block_stmt_iterator *si_p, tree expr, bool ignore)
  {
    tree_stmt_iterator ti;
    tree stmt = bsi_stmt (*si_p);
    tree tmp, stmts = NULL;
  
    push_gimplify_context ();
!   if (ignore)
!     {
!       tmp = NULL;
!       gimplify_and_add (expr, &stmts);
!     }
!   else
!     tmp = get_initialized_tmp_var (expr, &stmts, NULL);
    pop_gimplify_context (NULL);
  
    if (EXPR_HAS_LOCATION (stmt))
*************** execute_fold_all_builtins (void)
*** 2551,2557 ****
  
  	  if (!set_rhs (stmtp, result))
  	    {
! 	      result = convert_to_gimple_builtin (&i, result);
  	      if (result)
  		{
  		  bool ok = set_rhs (stmtp, result);
--- 2558,2566 ----
  
  	  if (!set_rhs (stmtp, result))
  	    {
! 	      result = convert_to_gimple_builtin (&i, result,
! 			      			  TREE_CODE (old_stmt)
! 						  != MODIFY_EXPR);
  	      if (result)
  		{
  		  bool ok = set_rhs (stmtp, result);
Index: gimplify.c
===================================================================
*** gimplify.c	(revision 118067)
--- gimplify.c	(working copy)
*************** internal_get_tmp_var (tree val, tree *pr
*** 609,615 ****
    if (TREE_CODE (TREE_TYPE (t)) == COMPLEX_TYPE)
      DECL_COMPLEX_GIMPLE_REG_P (t) = 1;
  
!   mod = build2 (INIT_EXPR, TREE_TYPE (t), t, val);
  
    if (EXPR_HAS_LOCATION (val))
      SET_EXPR_LOCUS (mod, EXPR_LOCUS (val));
--- 609,615 ----
    if (TREE_CODE (TREE_TYPE (t)) == COMPLEX_TYPE)
      DECL_COMPLEX_GIMPLE_REG_P (t) = 1;
  
!   mod = build2 (INIT_EXPR, TREE_TYPE (t), t, unshare_expr (val));
  
    if (EXPR_HAS_LOCATION (val))
      SET_EXPR_LOCUS (mod, EXPR_LOCUS (val));


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]