[2/2] analyzer: avoid use of fold_build2

Message ID 20200131011918.26107-3-dmalcolm@redhat.com
State New
Headers show
Series
  • [1/2] analyzer: further fixes for comparisons between uncomparable types (PR 93450)
Related show

Commit Message

David Malcolm Jan. 31, 2020, 1:19 a.m.
Various places in the analyzer use fold_build2, test the result, then
discard it.  It's more efficient to use fold_binary, which avoids
building and GC-ing a redundant tree for the cases where folding fails.

gcc/analyzer/ChangeLog:
	* constraint-manager.cc (range::constrained_to_single_element):
	Replace fold_build2 with fold_binary.  Remove unnecessary newline.
	(constraint_manager::get_or_add_equiv_class): Replace fold_build2
	with fold_binary in two places, and remove out-of-date comment.
	(constraint_manager::eval_condition): Replace fold_build2 with
	fold_binary.
	* region-model.cc (constant_svalue::eval_condition): Likewise.
	(region_model::on_assignment): Likewise.
---
 gcc/analyzer/constraint-manager.cc | 15 ++++++---------
 gcc/analyzer/region-model.cc       |  6 +++---
 2 files changed, 9 insertions(+), 12 deletions(-)

-- 
2.21.0

Comments

Jakub Jelinek Jan. 31, 2020, 9:32 a.m. | #1
On Thu, Jan 30, 2020 at 08:19:18PM -0500, David Malcolm wrote:
> gcc/analyzer/ChangeLog:

> 	* constraint-manager.cc (range::constrained_to_single_element):

> 	Replace fold_build2 with fold_binary.  Remove unnecessary newline.

> 	(constraint_manager::get_or_add_equiv_class): Replace fold_build2

> 	with fold_binary in two places, and remove out-of-date comment.

> 	(constraint_manager::eval_condition): Replace fold_build2 with

> 	fold_binary.

> 	* region-model.cc (constant_svalue::eval_condition): Likewise.

> 	(region_model::on_assignment): Likewise.


LGTM.

	Jakub
Andrew Pinski Jan. 31, 2020, 10:04 a.m. | #2
On Thu, Jan 30, 2020 at 5:19 PM David Malcolm <dmalcolm@redhat.com> wrote:
>

> Various places in the analyzer use fold_build2, test the result, then

> discard it.  It's more efficient to use fold_binary, which avoids

> building and GC-ing a redundant tree for the cases where folding fails.


If these are all true integer constants, then you might want to use
tree_int_cst_compare instead of even using fold_binary/fold_build2.
Also if you are doing equal but always constant (but not always
integer ones), you could use simple_cst_equal instead.

Thanks,
Andrew Pinski

>

> gcc/analyzer/ChangeLog:

>         * constraint-manager.cc (range::constrained_to_single_element):

>         Replace fold_build2 with fold_binary.  Remove unnecessary newline.

>         (constraint_manager::get_or_add_equiv_class): Replace fold_build2

>         with fold_binary in two places, and remove out-of-date comment.

>         (constraint_manager::eval_condition): Replace fold_build2 with

>         fold_binary.

>         * region-model.cc (constant_svalue::eval_condition): Likewise.

>         (region_model::on_assignment): Likewise.

> ---

>  gcc/analyzer/constraint-manager.cc | 15 ++++++---------

>  gcc/analyzer/region-model.cc       |  6 +++---

>  2 files changed, 9 insertions(+), 12 deletions(-)

>

> diff --git a/gcc/analyzer/constraint-manager.cc b/gcc/analyzer/constraint-manager.cc

> index f3e31ee0830..4d138188856 100644

> --- a/gcc/analyzer/constraint-manager.cc

> +++ b/gcc/analyzer/constraint-manager.cc

> @@ -145,10 +145,9 @@ range::constrained_to_single_element (tree *out)

>    m_upper_bound.ensure_closed (true);

>

>    // Are they equal?

> -  tree comparison

> -    = fold_build2 (EQ_EXPR, boolean_type_node,

> -                  m_lower_bound.m_constant,

> -                  m_upper_bound.m_constant);

> +  tree comparison = fold_binary (EQ_EXPR, boolean_type_node,

> +                                m_lower_bound.m_constant,

> +                                m_upper_bound.m_constant);

>    if (comparison == boolean_true_node)

>      {

>        *out = m_lower_bound.m_constant;

> @@ -930,7 +929,7 @@ constraint_manager::get_or_add_equiv_class (svalue_id sid)

>        FOR_EACH_VEC_ELT (m_equiv_classes, i, ec)

>         if (ec->m_constant)

>           {

> -           tree eq = fold_build2 (EQ_EXPR, boolean_type_node,

> +           tree eq = fold_binary (EQ_EXPR, boolean_type_node,

>                                    cst, ec->m_constant);

>             if (eq == boolean_true_node)

>               {

> @@ -967,10 +966,8 @@ constraint_manager::get_or_add_equiv_class (svalue_id sid)

>                  Determine the direction of the inequality, and record that

>                  fact.  */

>               tree lt

> -               = fold_build2 (LT_EXPR, boolean_type_node,

> +               = fold_binary (LT_EXPR, boolean_type_node,

>                                new_ec->m_constant, other_ec.m_constant);

> -             //gcc_assert (lt == boolean_true_node || lt == boolean_false_node);

> -             // not true for int vs float comparisons

>               if (lt == boolean_true_node)

>                 add_constraint_internal (new_id, CONSTRAINT_LT, other_id);

>               else if (lt == boolean_false_node)

> @@ -1016,7 +1013,7 @@ constraint_manager::eval_condition (equiv_class_id lhs_ec,

>    if (lhs_const && rhs_const)

>      {

>        tree comparison

> -       = fold_build2 (op, boolean_type_node, lhs_const, rhs_const);

> +       = fold_binary (op, boolean_type_node, lhs_const, rhs_const);

>        if (comparison == boolean_true_node)

>         return tristate (tristate::TS_TRUE);

>        if (comparison == boolean_false_node)

> diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc

> index b546114bfd5..95d002f9c28 100644

> --- a/gcc/analyzer/region-model.cc

> +++ b/gcc/analyzer/region-model.cc

> @@ -670,7 +670,7 @@ constant_svalue::eval_condition (constant_svalue *lhs,

>    if (types_compatible_p (TREE_TYPE (lhs_const), TREE_TYPE (rhs_const)))

>      {

>        tree comparison

> -       = fold_build2 (op, boolean_type_node, lhs_const, rhs_const);

> +       = fold_binary (op, boolean_type_node, lhs_const, rhs_const);

>        if (comparison == boolean_true_node)

>         return tristate (tristate::TS_TRUE);

>        if (comparison == boolean_false_node)

> @@ -4070,9 +4070,9 @@ region_model::on_assignment (const gassign *assign, region_model_context *ctxt)

>         if (tree rhs1_cst = maybe_get_constant (rhs1_sid))

>           if (tree rhs2_cst = maybe_get_constant (rhs2_sid))

>             {

> -             tree result = fold_build2 (op, TREE_TYPE (lhs),

> +             tree result = fold_binary (op, TREE_TYPE (lhs),

>                                          rhs1_cst, rhs2_cst);

> -             if (CONSTANT_CLASS_P (result))

> +             if (result && CONSTANT_CLASS_P (result))

>                 {

>                   svalue_id result_sid

>                     = get_or_create_constant_svalue (result);

> --

> 2.21.0

>

Patch

diff --git a/gcc/analyzer/constraint-manager.cc b/gcc/analyzer/constraint-manager.cc
index f3e31ee0830..4d138188856 100644
--- a/gcc/analyzer/constraint-manager.cc
+++ b/gcc/analyzer/constraint-manager.cc
@@ -145,10 +145,9 @@  range::constrained_to_single_element (tree *out)
   m_upper_bound.ensure_closed (true);
 
   // Are they equal?
-  tree comparison
-    = fold_build2 (EQ_EXPR, boolean_type_node,
-		   m_lower_bound.m_constant,
-		   m_upper_bound.m_constant);
+  tree comparison = fold_binary (EQ_EXPR, boolean_type_node,
+				 m_lower_bound.m_constant,
+				 m_upper_bound.m_constant);
   if (comparison == boolean_true_node)
     {
       *out = m_lower_bound.m_constant;
@@ -930,7 +929,7 @@  constraint_manager::get_or_add_equiv_class (svalue_id sid)
       FOR_EACH_VEC_ELT (m_equiv_classes, i, ec)
 	if (ec->m_constant)
 	  {
-	    tree eq = fold_build2 (EQ_EXPR, boolean_type_node,
+	    tree eq = fold_binary (EQ_EXPR, boolean_type_node,
 				   cst, ec->m_constant);
 	    if (eq == boolean_true_node)
 	      {
@@ -967,10 +966,8 @@  constraint_manager::get_or_add_equiv_class (svalue_id sid)
 		 Determine the direction of the inequality, and record that
 		 fact.  */
 	      tree lt
-		= fold_build2 (LT_EXPR, boolean_type_node,
+		= fold_binary (LT_EXPR, boolean_type_node,
 			       new_ec->m_constant, other_ec.m_constant);
-	      //gcc_assert (lt == boolean_true_node || lt == boolean_false_node);
-	      // not true for int vs float comparisons
 	      if (lt == boolean_true_node)
 		add_constraint_internal (new_id, CONSTRAINT_LT, other_id);
 	      else if (lt == boolean_false_node)
@@ -1016,7 +1013,7 @@  constraint_manager::eval_condition (equiv_class_id lhs_ec,
   if (lhs_const && rhs_const)
     {
       tree comparison
-	= fold_build2 (op, boolean_type_node, lhs_const, rhs_const);
+	= fold_binary (op, boolean_type_node, lhs_const, rhs_const);
       if (comparison == boolean_true_node)
 	return tristate (tristate::TS_TRUE);
       if (comparison == boolean_false_node)
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index b546114bfd5..95d002f9c28 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -670,7 +670,7 @@  constant_svalue::eval_condition (constant_svalue *lhs,
   if (types_compatible_p (TREE_TYPE (lhs_const), TREE_TYPE (rhs_const)))
     {
       tree comparison
-	= fold_build2 (op, boolean_type_node, lhs_const, rhs_const);
+	= fold_binary (op, boolean_type_node, lhs_const, rhs_const);
       if (comparison == boolean_true_node)
 	return tristate (tristate::TS_TRUE);
       if (comparison == boolean_false_node)
@@ -4070,9 +4070,9 @@  region_model::on_assignment (const gassign *assign, region_model_context *ctxt)
 	if (tree rhs1_cst = maybe_get_constant (rhs1_sid))
 	  if (tree rhs2_cst = maybe_get_constant (rhs2_sid))
 	    {
-	      tree result = fold_build2 (op, TREE_TYPE (lhs),
+	      tree result = fold_binary (op, TREE_TYPE (lhs),
 					 rhs1_cst, rhs2_cst);
-	      if (CONSTANT_CLASS_P (result))
+	      if (result && CONSTANT_CLASS_P (result))
 		{
 		  svalue_id result_sid
 		    = get_or_create_constant_svalue (result);