diff mbox

Record missing equivalence

Message ID 5150A0A2.5020400@redhat.com
State New
Headers show

Commit Message

Jeff Law March 25, 2013, 7:08 p.m. UTC
On 03/21/2013 03:44 AM, Richard Biener wrote:
>> +         /* If LHS is an SSA_NAME and RHS is a constant and LHS was set
>> +            via a widening type conversion, then we may be able to record
>> +            additional equivalences.  */
>> +         if (lhs
>> +             && TREE_CODE (lhs) == SSA_NAME
>> +             && is_gimple_constant (rhs))
>> +           {
>> +             gimple defstmt = SSA_NAME_DEF_STMT (lhs);
>> +
>> +             if (defstmt
>> +                 && is_gimple_assign (defstmt)
>> +                 && CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (defstmt)))
>> +               {
>> +                 tree old_rhs = gimple_assign_rhs1 (defstmt);
>> +                 tree newval = fold_convert (TREE_TYPE (old_rhs), rhs);
>
> You want to delay that folding and creating of a new tree node until after ...
>
>> +
>> +                 /* If this was a widening conversion and if RHS is
>> converted
>> +                    to the type of OLD_RHS and has the same value, then we
>> +                    can record an equivalence between OLD_RHS and the
>> +                    converted representation of RHS.  */
>> +                 if ((TYPE_PRECISION (TREE_TYPE (lhs))
>> +                      > TYPE_PRECISION (TREE_TYPE (old_rhs)))
>
> ... this check.
>
>> +                     && operand_equal_p (rhs, newval, 0))
>
> If you'd restricted yourself to handling INTEGER_CSTs then using
> int_fits_type_p (rhs, TREE_TYPE (lhs)) would have been enough to check.
>
> And operand_equal_p will never return for non-equal typed non-INTEGER_CSTs
> anyway ...
Agreed.  Addressed via the attached patch which was committed after a 
bootstrap and regression test on x86_64-unknown-linux-gnu.

Comments

H.J. Lu March 26, 2013, 12:41 a.m. UTC | #1
On Mon, Mar 25, 2013 at 12:08 PM, Jeff Law <law@redhat.com> wrote:
> On 03/21/2013 03:44 AM, Richard Biener wrote:
>>>
>>> +         /* If LHS is an SSA_NAME and RHS is a constant and LHS was set
>>> +            via a widening type conversion, then we may be able to
>>> record
>>> +            additional equivalences.  */
>>> +         if (lhs
>>> +             && TREE_CODE (lhs) == SSA_NAME
>>> +             && is_gimple_constant (rhs))
>>> +           {
>>> +             gimple defstmt = SSA_NAME_DEF_STMT (lhs);
>>> +
>>> +             if (defstmt
>>> +                 && is_gimple_assign (defstmt)
>>> +                 && CONVERT_EXPR_CODE_P (gimple_assign_rhs_code
>>> (defstmt)))
>>> +               {
>>> +                 tree old_rhs = gimple_assign_rhs1 (defstmt);
>>> +                 tree newval = fold_convert (TREE_TYPE (old_rhs), rhs);
>>
>>
>> You want to delay that folding and creating of a new tree node until after
>> ...
>>
>>> +
>>> +                 /* If this was a widening conversion and if RHS is
>>> converted
>>> +                    to the type of OLD_RHS and has the same value, then
>>> we
>>> +                    can record an equivalence between OLD_RHS and the
>>> +                    converted representation of RHS.  */
>>> +                 if ((TYPE_PRECISION (TREE_TYPE (lhs))
>>> +                      > TYPE_PRECISION (TREE_TYPE (old_rhs)))
>>
>>
>> ... this check.
>>
>>> +                     && operand_equal_p (rhs, newval, 0))
>>
>>
>> If you'd restricted yourself to handling INTEGER_CSTs then using
>> int_fits_type_p (rhs, TREE_TYPE (lhs)) would have been enough to check.
>>
>> And operand_equal_p will never return for non-equal typed non-INTEGER_CSTs
>> anyway ...
>
> Agreed.  Addressed via the attached patch which was committed after a
> bootstrap and regression test on x86_64-unknown-linux-gnu.
>
>
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 9bdf1e5..9db0629 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,9 @@
> +2013-03-25  Jeff Law  <law@redhat.com>
> +
> +       * tree-ssa-dom.c (record_equivalences_from_incoming_edge): Rework
> +       slightly to avoid creating and folding useless trees.  Simplify
> +       slightly by restricting to INTEGER_CSTs and using int_fits_type_p.
> +
>  2013-03-25  Uros Bizjak  <ubizjak@gmail.com>
>

This breaks the bootstrap on Linux/x86:

http://gcc.gnu.org/ml/gcc-regression/2013-03/msg00148.html

../../../../../src-trunk/libstdc++-v3/src/c++98/mt_allocator.cc: In
member function 'std::size_t
__gnu_cxx::__pool<true>::_M_get_thread_id()':
../../../../../src-trunk/libstdc++-v3/src/c++98/mt_allocator.cc:620:3:
internal compiler error: tree check: expected integer_type or
enumeral_type or boolean_type or real_type or fixed_point_type, have
pointer_type in int_fits_type_p, at tree.c:8325
   __pool<true>::_M_get_thread_id()
   ^
0x8ad2cef tree_check_failed(tree_node const*, char const*, int, char
const*, ...)
	../../src-trunk/gcc/tree.c:8947
0x81ba464 tree_check5(tree_node const*, char const*, int, char const*,
tree_code, tree_code, tree_code, tree_code, tree_code)
	../../src-trunk/gcc/tree.h:3987
0x8ad058d int_fits_type_p(tree_node const*, tree_node const*)
	../../src-trunk/gcc/tree.c:8325
0x897363b record_equivalences_from_incoming_edge
	../../src-trunk/gcc/tree-ssa-dom.c:1156
0x8977b5e dom_opt_enter_block
	../../src-trunk/gcc/tree-ssa-dom.c:1769
0x8dccddd walk_dominator_tree(dom_walk_data*, basic_block_def*)
	../../src-trunk/gcc/domwalk.c:210
0x8972b9c tree_ssa_dominator_optimize
	../../src-trunk/gcc/tree-ssa-dom.c:762
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.
Jeff Law March 26, 2013, 2:12 a.m. UTC | #2
On 03/25/2013 06:41 PM, H.J. Lu wrote:
>
> This breaks the bootstrap on Linux/x86:
>
> http://gcc.gnu.org/ml/gcc-regression/2013-03/msg00148.html
>
> ../../../../../src-trunk/libstdc++-v3/src/c++98/mt_allocator.cc: In
> member function 'std::size_t
> __gnu_cxx::__pool<true>::_M_get_thread_id()':
> ../../../../../src-trunk/libstdc++-v3/src/c++98/mt_allocator.cc:620:3:
> internal compiler error: tree check: expected integer_type or
> enumeral_type or boolean_type or real_type or fixed_point_type, have
> pointer_type in int_fits_type_p, at tree.c:8325
>     __pool<true>::_M_get_thread_id()
That looks exactly like something I already fixed.  Let me make sure I 
didn't post/checkin the wrong version of the patch.

jeff
Jeff Law March 26, 2013, 4 a.m. UTC | #3
On 03/25/2013 06:41 PM, H.J. Lu wrote:

> This breaks the bootstrap on Linux/x86:
>
> http://gcc.gnu.org/ml/gcc-regression/2013-03/msg00148.html
>
> ../../../../../src-trunk/libstdc++-v3/src/c++98/mt_allocator.cc: In
> member function 'std::size_t
> __gnu_cxx::__pool<true>::_M_get_thread_id()':
> ../../../../../src-trunk/libstdc++-v3/src/c++98/mt_allocator.cc:620:3:
> internal compiler error: tree check: expected integer_type or
> enumeral_type or boolean_type or real_type or fixed_point_type, have
> pointer_type in int_fits_type_p, at tree.c:8325
>     __pool<true>::_M_get_thread_id()
Definitely the wrong version of the patch.  It's missing the 
INTEGRAL_TYPE_P test that was added to fix this exact problem.

This is the delta to get to the version that should have been checked in.
* tree-ssa-dom.c (record_equivalences_from_incoming_edge): Add missing
	check for INTEGRAL_TYPE_P that was missing due to checking in wrong
	version of prior patch.

*** ../../GIT/gcc/gcc/tree-ssa-dom.c	Mon Mar 25 13:03:11 2013
--- tree-ssa-dom.c	Thu Mar 21 07:28:51 2013
*************** record_equivalences_from_incoming_edge (
*** 1153,1159 ****
  
  		  /* If the constant is in the range of the type of OLD_RHS,
  		     then convert the constant and record the equivalence.  */
! 		  if (int_fits_type_p (rhs, TREE_TYPE (old_rhs)))
  		    {
  		      tree newval = fold_convert (TREE_TYPE (old_rhs), rhs);
  		      record_equality (old_rhs, newval);
--- 1153,1160 ----
  
  		  /* If the constant is in the range of the type of OLD_RHS,
  		     then convert the constant and record the equivalence.  */
! 		  if (INTEGRAL_TYPE_P (TREE_TYPE (old_rhs))
! 		      && int_fits_type_p (rhs, TREE_TYPE (old_rhs)))
  		    {
  		      tree newval = fold_convert (TREE_TYPE (old_rhs), rhs);
  		      record_equality (old_rhs, newval);
diff mbox

Patch

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 9bdf1e5..9db0629 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@ 
+2013-03-25  Jeff Law  <law@redhat.com>
+
+	* tree-ssa-dom.c (record_equivalences_from_incoming_edge): Rework
+	slightly to avoid creating and folding useless trees.  Simplify
+	slightly by restricting to INTEGER_CSTs and using int_fits_type_p.
+
 2013-03-25  Uros Bizjak  <ubizjak@gmail.com>
 
 	* config/i386/i386.md (*zero_extendsidi2): Merge with
diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
index 57b814c..a71c6dc 100644
--- a/gcc/tree-ssa-dom.c
+++ b/gcc/tree-ssa-dom.c
@@ -1135,12 +1135,13 @@  record_equivalences_from_incoming_edge (basic_block bb)
 	  if (lhs)
 	    record_equality (lhs, rhs);
 
-	  /* If LHS is an SSA_NAME and RHS is a constant and LHS was set
-	     via a widening type conversion, then we may be able to record
+	  /* If LHS is an SSA_NAME and RHS is a constant integer and LHS was
+	     set via a widening type conversion, then we may be able to record
 	     additional equivalences.  */
 	  if (lhs
 	      && TREE_CODE (lhs) == SSA_NAME
-	      && is_gimple_constant (rhs))
+	      && is_gimple_constant (rhs)
+	      && TREE_CODE (rhs) == INTEGER_CST)
 	    {
 	      gimple defstmt = SSA_NAME_DEF_STMT (lhs);
 
@@ -1149,16 +1150,14 @@  record_equivalences_from_incoming_edge (basic_block bb)
 		  && CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (defstmt)))
 		{
 		  tree old_rhs = gimple_assign_rhs1 (defstmt);
-		  tree newval = fold_convert (TREE_TYPE (old_rhs), rhs);
-
-		  /* If this was a widening conversion and if RHS is converted
-		     to the type of OLD_RHS and has the same value, then we
-		     can record an equivalence between OLD_RHS and the
-		     converted representation of RHS.  */
-		  if ((TYPE_PRECISION (TREE_TYPE (lhs))
-		       > TYPE_PRECISION (TREE_TYPE (old_rhs)))
-		      && operand_equal_p (rhs, newval, 0))
-		    record_equality (old_rhs, newval);
+
+		  /* If the constant is in the range of the type of OLD_RHS,
+		     then convert the constant and record the equivalence.  */
+		  if (int_fits_type_p (rhs, TREE_TYPE (old_rhs)))
+		    {
+		      tree newval = fold_convert (TREE_TYPE (old_rhs), rhs);
+		      record_equality (old_rhs, newval);
+		    }
 		}
 	    }