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) | expand |
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
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 >
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);