On Nov 26, 2013, at 1:34 AM, Richard Biener <richard.guent...@gmail.com> wrote: > On Mon, Nov 25, 2013 at 9:05 PM, Richard Sandiford > <rdsandif...@googlemail.com> wrote: >> Jason Merrill <ja...@redhat.com> writes: >>> On 11/23/2013 02:20 PM, Mike Stump wrote: >>>> @@ -2605,8 +2606,7 @@ cp_tree_equal (tree t1, tree t2) >>>> switch (code1) >>>> { >>>> case INTEGER_CST: >>>> - return TREE_INT_CST_LOW (t1) == TREE_INT_CST_LOW (t2) >>>> - && TREE_INT_CST_HIGH (t1) == TREE_INT_CST_HIGH (t2); >>>> + return wi::to_widest (t1) == wi::to_widest (t2); >>> >>> Why not use wi::eq_p like you do in the C front end? >> >> Thanks for noticing the difference. I think c_tree_equal should change >> to use to_widest too. >> >> wi::eq_p (t1, t2) asserts that t1 and t2 are the same precision and >> ignores signedness; it just tests whether they are the same bitstring. >> wi::to_widest (t1) == wi::to_widest (t2) compares them as logical numbers, >> taking sign into account and allowing different types. I think that's >> what the original TREE_INT_CST_LOW and TREE_INT_CST_HIGH tests did too. > > Though in this case (comparing two INTEGER_CSTs) it would be better > to use a tree abstraction - thus tree_int_cst_equal. It saves us from > making the decision on what to map this in wide-int to multiple times.
Seems like a good idea to me: Index: cp/tree.c =================================================================== --- cp/tree.c (revision 206183) +++ cp/tree.c (working copy) @@ -2606,7 +2606,7 @@ cp_tree_equal (tree t1, tree t2) switch (code1) { case INTEGER_CST: - return wi::to_widest (t1) == wi::to_widest (t2); + return tree_int_cst_equal (t1, t2); case REAL_CST: return REAL_VALUES_EQUAL (TREE_REAL_CST (t1), TREE_REAL_CST (t2)); Jason, are the C++ patches with this change to them Ok?