public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix PR3713
@ 2013-01-16 15:57 Richard Biener
  2013-03-18  8:54 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Richard Biener @ 2013-01-16 15:57 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jan Hubicka


This fixes PR3713 by properly propagating ->has_constants in SCCVN.
With that we are able to simplify (unsigned) Bar & 1 properly.
Only copyprop later turns the call into a direct one though,
so I'm testing the important fact - that Bar is inlined and eliminated
by IPA inlining.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Unless this is somehow a regression (which I doubt) this has to
wait for stage1 (even though it's pretty safe and at most exposes
existing bugs in SCCVN).

Richard.

2013-01-16  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/3713
	* tree-ssa-sccvn.c (visit_copy): Simplify.  Always propagate
	has_constants and expr.
	(stmt_has_constants): Properly valueize SSA names when deciding
	whether the stmt has constants.

	* g++.dg/ipa/devirt-11.C: New testcase.

Index: gcc/tree-ssa-sccvn.c
===================================================================
*** gcc/tree-ssa-sccvn.c	(revision 195240)
--- gcc/tree-ssa-sccvn.c	(working copy)
*************** static tree valueize_expr (tree expr);
*** 2653,2670 ****
  static bool
  visit_copy (tree lhs, tree rhs)
  {
-   /* Follow chains of copies to their destination.  */
-   while (TREE_CODE (rhs) == SSA_NAME
- 	 && SSA_VAL (rhs) != rhs)
-     rhs = SSA_VAL (rhs);
- 
    /* The copy may have a more interesting constant filled expression
       (we don't, since we know our RHS is just an SSA name).  */
!   if (TREE_CODE (rhs) == SSA_NAME)
!     {
!       VN_INFO (lhs)->has_constants = VN_INFO (rhs)->has_constants;
!       VN_INFO (lhs)->expr = VN_INFO (rhs)->expr;
!     }
  
    return set_ssa_val_to (lhs, rhs);
  }
--- 2653,2665 ----
  static bool
  visit_copy (tree lhs, tree rhs)
  {
    /* The copy may have a more interesting constant filled expression
       (we don't, since we know our RHS is just an SSA name).  */
!   VN_INFO (lhs)->has_constants = VN_INFO (rhs)->has_constants;
!   VN_INFO (lhs)->expr = VN_INFO (rhs)->expr;
! 
!   /* And finally valueize.  */
!   rhs = SSA_VAL (rhs);
  
    return set_ssa_val_to (lhs, rhs);
  }
*************** expr_has_constants (tree expr)
*** 3063,3087 ****
  static bool
  stmt_has_constants (gimple stmt)
  {
    if (gimple_code (stmt) != GIMPLE_ASSIGN)
      return false;
  
    switch (get_gimple_rhs_class (gimple_assign_rhs_code (stmt)))
      {
!     case GIMPLE_UNARY_RHS:
!       return is_gimple_min_invariant (gimple_assign_rhs1 (stmt));
  
      case GIMPLE_BINARY_RHS:
!       return (is_gimple_min_invariant (gimple_assign_rhs1 (stmt))
! 	      || is_gimple_min_invariant (gimple_assign_rhs2 (stmt)));
!     case GIMPLE_TERNARY_RHS:
!       return (is_gimple_min_invariant (gimple_assign_rhs1 (stmt))
! 	      || is_gimple_min_invariant (gimple_assign_rhs2 (stmt))
! 	      || is_gimple_min_invariant (gimple_assign_rhs3 (stmt)));
      case GIMPLE_SINGLE_RHS:
        /* Constants inside reference ops are rarely interesting, but
  	 it can take a lot of looking to find them.  */
!       return is_gimple_min_invariant (gimple_assign_rhs1 (stmt));
      default:
        gcc_unreachable ();
      }
--- 3058,3095 ----
  static bool
  stmt_has_constants (gimple stmt)
  {
+   tree tem;
+ 
    if (gimple_code (stmt) != GIMPLE_ASSIGN)
      return false;
  
    switch (get_gimple_rhs_class (gimple_assign_rhs_code (stmt)))
      {
!     case GIMPLE_TERNARY_RHS:
!       tem = gimple_assign_rhs3 (stmt);
!       if (TREE_CODE (tem) == SSA_NAME)
! 	tem = SSA_VAL (tem);
!       if (is_gimple_min_invariant (tem))
! 	return true;
!       /* Fallthru.  */
  
      case GIMPLE_BINARY_RHS:
!       tem = gimple_assign_rhs2 (stmt);
!       if (TREE_CODE (tem) == SSA_NAME)
! 	tem = SSA_VAL (tem);
!       if (is_gimple_min_invariant (tem))
! 	return true;
!       /* Fallthru.  */
! 
      case GIMPLE_SINGLE_RHS:
        /* Constants inside reference ops are rarely interesting, but
  	 it can take a lot of looking to find them.  */
!     case GIMPLE_UNARY_RHS:
!       tem = gimple_assign_rhs1 (stmt);
!       if (TREE_CODE (tem) == SSA_NAME)
! 	tem = SSA_VAL (tem);
!       return is_gimple_min_invariant (tem);
! 
      default:
        gcc_unreachable ();
      }
Index: gcc/testsuite/g++.dg/ipa/devirt-11.C
===================================================================
*** gcc/testsuite/g++.dg/ipa/devirt-11.C	(revision 0)
--- gcc/testsuite/g++.dg/ipa/devirt-11.C	(working copy)
***************
*** 0 ****
--- 1,22 ----
+ // { dg-do compile }
+ // { dg-options "-std=c++11 -O -fdump-ipa-inline" }
+ 
+ class Foo
+ {
+ public:
+   void Bar() const
+     {
+       __builtin_puts ("Howdy!");
+     }
+ };
+ 
+ int main()
+ {
+   Foo x;
+   auto y = &Foo::Bar;
+   (x.*y)();
+   return 0;
+ }
+ 
+ // { dg-final { scan-ipa-dump "Inlined 1 calls, eliminated 1 functions" "inline" } }
+ // { dg-final { cleanup-ipa-dump "inline" } }

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] Fix PR3713
  2013-01-16 15:57 [PATCH] Fix PR3713 Richard Biener
@ 2013-03-18  8:54 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2013-03-18  8:54 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches, Jan Hubicka

On Wed, Jan 16, 2013 at 4:57 PM, Richard Biener <rguenther@suse.de> wrote:
>
> This fixes PR3713 by properly propagating ->has_constants in SCCVN.
> With that we are able to simplify (unsigned) Bar & 1 properly.
> Only copyprop later turns the call into a direct one though,
> so I'm testing the important fact - that Bar is inlined and eliminated
> by IPA inlining.
>
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
>
> Unless this is somehow a regression (which I doubt) this has to
> wait for stage1 (even though it's pretty safe and at most exposes
> existing bugs in SCCVN).

Committed as r196771.

Richard.

> Richard.
>
> 2013-01-16  Richard Biener  <rguenther@suse.de>
>
>         PR tree-optimization/3713
>         * tree-ssa-sccvn.c (visit_copy): Simplify.  Always propagate
>         has_constants and expr.
>         (stmt_has_constants): Properly valueize SSA names when deciding
>         whether the stmt has constants.
>
>         * g++.dg/ipa/devirt-11.C: New testcase.
>
> Index: gcc/tree-ssa-sccvn.c
> ===================================================================
> *** gcc/tree-ssa-sccvn.c        (revision 195240)
> --- gcc/tree-ssa-sccvn.c        (working copy)
> *************** static tree valueize_expr (tree expr);
> *** 2653,2670 ****
>   static bool
>   visit_copy (tree lhs, tree rhs)
>   {
> -   /* Follow chains of copies to their destination.  */
> -   while (TREE_CODE (rhs) == SSA_NAME
> -        && SSA_VAL (rhs) != rhs)
> -     rhs = SSA_VAL (rhs);
> -
>     /* The copy may have a more interesting constant filled expression
>        (we don't, since we know our RHS is just an SSA name).  */
> !   if (TREE_CODE (rhs) == SSA_NAME)
> !     {
> !       VN_INFO (lhs)->has_constants = VN_INFO (rhs)->has_constants;
> !       VN_INFO (lhs)->expr = VN_INFO (rhs)->expr;
> !     }
>
>     return set_ssa_val_to (lhs, rhs);
>   }
> --- 2653,2665 ----
>   static bool
>   visit_copy (tree lhs, tree rhs)
>   {
>     /* The copy may have a more interesting constant filled expression
>        (we don't, since we know our RHS is just an SSA name).  */
> !   VN_INFO (lhs)->has_constants = VN_INFO (rhs)->has_constants;
> !   VN_INFO (lhs)->expr = VN_INFO (rhs)->expr;
> !
> !   /* And finally valueize.  */
> !   rhs = SSA_VAL (rhs);
>
>     return set_ssa_val_to (lhs, rhs);
>   }
> *************** expr_has_constants (tree expr)
> *** 3063,3087 ****
>   static bool
>   stmt_has_constants (gimple stmt)
>   {
>     if (gimple_code (stmt) != GIMPLE_ASSIGN)
>       return false;
>
>     switch (get_gimple_rhs_class (gimple_assign_rhs_code (stmt)))
>       {
> !     case GIMPLE_UNARY_RHS:
> !       return is_gimple_min_invariant (gimple_assign_rhs1 (stmt));
>
>       case GIMPLE_BINARY_RHS:
> !       return (is_gimple_min_invariant (gimple_assign_rhs1 (stmt))
> !             || is_gimple_min_invariant (gimple_assign_rhs2 (stmt)));
> !     case GIMPLE_TERNARY_RHS:
> !       return (is_gimple_min_invariant (gimple_assign_rhs1 (stmt))
> !             || is_gimple_min_invariant (gimple_assign_rhs2 (stmt))
> !             || is_gimple_min_invariant (gimple_assign_rhs3 (stmt)));
>       case GIMPLE_SINGLE_RHS:
>         /* Constants inside reference ops are rarely interesting, but
>          it can take a lot of looking to find them.  */
> !       return is_gimple_min_invariant (gimple_assign_rhs1 (stmt));
>       default:
>         gcc_unreachable ();
>       }
> --- 3058,3095 ----
>   static bool
>   stmt_has_constants (gimple stmt)
>   {
> +   tree tem;
> +
>     if (gimple_code (stmt) != GIMPLE_ASSIGN)
>       return false;
>
>     switch (get_gimple_rhs_class (gimple_assign_rhs_code (stmt)))
>       {
> !     case GIMPLE_TERNARY_RHS:
> !       tem = gimple_assign_rhs3 (stmt);
> !       if (TREE_CODE (tem) == SSA_NAME)
> !       tem = SSA_VAL (tem);
> !       if (is_gimple_min_invariant (tem))
> !       return true;
> !       /* Fallthru.  */
>
>       case GIMPLE_BINARY_RHS:
> !       tem = gimple_assign_rhs2 (stmt);
> !       if (TREE_CODE (tem) == SSA_NAME)
> !       tem = SSA_VAL (tem);
> !       if (is_gimple_min_invariant (tem))
> !       return true;
> !       /* Fallthru.  */
> !
>       case GIMPLE_SINGLE_RHS:
>         /* Constants inside reference ops are rarely interesting, but
>          it can take a lot of looking to find them.  */
> !     case GIMPLE_UNARY_RHS:
> !       tem = gimple_assign_rhs1 (stmt);
> !       if (TREE_CODE (tem) == SSA_NAME)
> !       tem = SSA_VAL (tem);
> !       return is_gimple_min_invariant (tem);
> !
>       default:
>         gcc_unreachable ();
>       }
> Index: gcc/testsuite/g++.dg/ipa/devirt-11.C
> ===================================================================
> *** gcc/testsuite/g++.dg/ipa/devirt-11.C        (revision 0)
> --- gcc/testsuite/g++.dg/ipa/devirt-11.C        (working copy)
> ***************
> *** 0 ****
> --- 1,22 ----
> + // { dg-do compile }
> + // { dg-options "-std=c++11 -O -fdump-ipa-inline" }
> +
> + class Foo
> + {
> + public:
> +   void Bar() const
> +     {
> +       __builtin_puts ("Howdy!");
> +     }
> + };
> +
> + int main()
> + {
> +   Foo x;
> +   auto y = &Foo::Bar;
> +   (x.*y)();
> +   return 0;
> + }
> +
> + // { dg-final { scan-ipa-dump "Inlined 1 calls, eliminated 1 functions" "inline" } }
> + // { dg-final { cleanup-ipa-dump "inline" } }

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2013-03-18  8:54 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-16 15:57 [PATCH] Fix PR3713 Richard Biener
2013-03-18  8:54 ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).