Folding (a ? b : c) op d
Hello, this patch changes the test deciding whether to fold "op d" with both branches in (a ? b : c) op d. I don't know if the new test is right, it gives what I expect on the new testcase, but I may have missed important cases. Cc: Eric for comments as the author of the previous conditions. Passes bootstrap+testsuite on x86_64-unknown-linux-gnu. 2013-06-29 Marc Glisse PR tree-optimization/57755 gcc/ * fold-const.c (fold_binary_op_with_conditional_arg): Change condition under which the transformation is performed. gcc/testsuite/ * gcc.dg/pr57755.c: New testcase. * gcc.dg/binop-notand1a.c: Remove xfail. * gcc.dg/binop-notand4a.c: Likewise. -- Marc GlisseIndex: fold-const.c === --- fold-const.c(revision 200556) +++ fold-const.c(working copy) @@ -6097,26 +6097,33 @@ constant_boolean_node (bool value, tree given here), it is the second argument. TYPE is the type of the original expression. Return NULL_TREE if no simplification is possible. */ static tree fold_binary_op_with_conditional_arg (location_t loc, enum tree_code code, tree type, tree op0, tree op1, tree cond, tree arg, int cond_first_p) { - tree cond_type = cond_first_p ? TREE_TYPE (op0) : TREE_TYPE (op1); - tree arg_type = cond_first_p ? TREE_TYPE (op1) : TREE_TYPE (op0); + /* ??? This transformation is only worthwhile if we don't have + to wrap ARG in a SAVE_EXPR. */ + if (TREE_SIDE_EFFECTS (arg)) +return NULL_TREE; + + /* Avoid exponential recursion. */ + static int depth = 0; + if (depth > 1) +return NULL_TREE; + tree test, true_value, false_value; tree lhs = NULL_TREE; tree rhs = NULL_TREE; - enum tree_code cond_code = COND_EXPR; if (TREE_CODE (cond) == COND_EXPR || TREE_CODE (cond) == VEC_COND_EXPR) { test = TREE_OPERAND (cond, 0); true_value = TREE_OPERAND (cond, 1); false_value = TREE_OPERAND (cond, 2); /* If this operand throws an expression, then it does not make sense to try to perform a logical or arithmetic operation involving it. */ @@ -6126,54 +6133,49 @@ fold_binary_op_with_conditional_arg (loc rhs = false_value; } else { tree testtype = TREE_TYPE (cond); test = cond; true_value = constant_boolean_node (true, testtype); false_value = constant_boolean_node (false, testtype); } - if (TREE_CODE (TREE_TYPE (test)) == VECTOR_TYPE) -cond_code = VEC_COND_EXPR; - - /* This transformation is only worthwhile if we don't have to wrap ARG - in a SAVE_EXPR and the operation can be simplified without recursing - on at least one of the branches once its pushed inside the COND_EXPR. */ - if (!TREE_CONSTANT (arg) - && (TREE_SIDE_EFFECTS (arg) - || TREE_CODE (arg) == COND_EXPR || TREE_CODE (arg) == VEC_COND_EXPR - || TREE_CONSTANT (true_value) || TREE_CONSTANT (false_value))) -return NULL_TREE; + tree cond_type = cond_first_p ? TREE_TYPE (op0) : TREE_TYPE (op1); + tree arg_type = cond_first_p ? TREE_TYPE (op1) : TREE_TYPE (op0); arg = fold_convert_loc (loc, arg_type, arg); + ++depth; if (lhs == 0) { true_value = fold_convert_loc (loc, cond_type, true_value); if (cond_first_p) lhs = fold_build2_loc (loc, code, type, true_value, arg); else lhs = fold_build2_loc (loc, code, type, arg, true_value); } if (rhs == 0) { false_value = fold_convert_loc (loc, cond_type, false_value); if (cond_first_p) rhs = fold_build2_loc (loc, code, type, false_value, arg); else rhs = fold_build2_loc (loc, code, type, arg, false_value); } + --depth; /* Check that we have simplified at least one of the branches. */ - if (!TREE_CONSTANT (arg) && !TREE_CONSTANT (lhs) && !TREE_CONSTANT (rhs)) + if (!TREE_CONSTANT (lhs) && !TREE_CONSTANT (rhs)) return NULL_TREE; + enum tree_code cond_code = VECTOR_TYPE_P (TREE_TYPE (test)) +? VEC_COND_EXPR : COND_EXPR; return fold_build3_loc (loc, cond_code, type, test, lhs, rhs); } /* Subroutine of fold() that checks for the addition of +/- 0.0. If !NEGATE, return true if ADDEND is +/-0.0 and, for all X of type TYPE, X + ADDEND is the same as X. If NEGATE, return true if X - ADDEND is the same as X. Index: testsuite/gcc.dg/binop-notand1a.c === --- testsuite/gcc.dg/binop-notand1a.c (revision 200556) +++ testsuite/gcc.dg/binop-notand1a.c (working copy) @@ -1,13 +1,14 @@ /* { dg-do compile } */ /* { dg-options "-O2 -fdump-tree-optimized" } */ int foo (char a, unsigned short b) { return (a & !a) | (b & !b); } /* As long as comparisons aren'
Re: [patch] regex_traits implementation
Thank you for response! On Sat, Jun 29, 2013 at 7:08 AM, Paolo Carlini wrote: > Very nice to see progress in this area! Obvious question - I may have missed > past discussions: is your paperwork, the copyright assignment, Ok? > Otherwise, unfortunately, the technical review can't even begin. I've already sent a signed assignment form to FSF and got a response saying that "Your assignment/disclaimer process with the FSF is currently complete; your fully executed PDF will be sent to you in a separate email immediately following this one". Shall I attach the PDF? > PS: The formatting needs work, anyway. Eg: 80 columns; operators, eg ||, > never end a line, begin one, etc) Sorry for those. They're fixed in the updated patch. -- Tim Shen regex_traits.patch Description: Binary data
Re: Unordered container insertion hints
Patch applied, I also needed to update some dg-error line numbers, it was not part of the original patch. The only remaining thing will be to update the doxygen links on the unordered containers to point to the new hint doc. 2013-06-29 François Dumont * include/bits/hashtable_policy.h (_Insert_base): Consider hint in insert methods. * include/bits/hashtable.h: Likewise. * testsuite/23_containers/unordered_multimap/insert/hint.cc: New. * testsuite/performance/23_containers/insert/unordered_multiset_hint.cc: New. * testsuite/23_containers/unordered_set/instantiation_neg.cc: Adjust dg-error line number. * testsuite/23_containers/unordered_set/ not_default_constructible_hash_neg.cc: Likewise. * doc/xml/manual/containers.xml: Document hinting in unordered containers. François On 06/28/2013 01:46 PM, Jonathan Wakely wrote: On 23 June 2013 13:51, Jonathan Wakely wrote: On 19 June 2013 20:56, François Dumont wrote: Still no chance to have a look ? I'll try to finish reviewing it today, thanks for the reminder. Sorry for the delay. The patch is OK, with a few small changes to the new docs: Please change "rational" to "rationale" "can require to update the bucket" should be "can require updating the bucket" "need to compute following node hash code." should be "need to compute the following node's hash code." "won't compute next element hash code" should be "won't compute the next element's hash code" "bench" should be "benchmark" I would say "more harm than good" instead of "more bad than good" Thanks! Index: include/bits/hashtable_policy.h === --- include/bits/hashtable_policy.h (revision 200491) +++ include/bits/hashtable_policy.h (working copy) @@ -612,7 +612,6 @@ using __unique_keys = typename __hashtable_base::__unique_keys; using __ireturn_type = typename __hashtable_base::__ireturn_type; - using __iconv_type = typename __hashtable_base::__iconv_type; __hashtable& _M_conjure_hashtable() @@ -626,8 +625,11 @@ } iterator - insert(const_iterator, const value_type& __v) - { return __iconv_type()(insert(__v)); } + insert(const_iterator __hint, const value_type& __v) + { + __hashtable& __h = _M_conjure_hashtable(); + return __h._M_insert(__hint, __v, __unique_keys()); + } void insert(initializer_list __l) @@ -711,8 +713,11 @@ } iterator - insert(const_iterator, value_type&& __v) - { return insert(std::move(__v)).first; } + insert(const_iterator __hint, value_type&& __v) + { + __hashtable& __h = this->_M_conjure_hashtable(); + return __h._M_insert(__hint, std::move(__v), __unique_keys()); + } }; /// Specialization. @@ -745,9 +750,12 @@ } iterator - insert(const_iterator, value_type&& __v) - { return insert(std::move(__v)); } - }; + insert(const_iterator __hint, value_type&& __v) + { + __hashtable& __h = this->_M_conjure_hashtable(); + return __h._M_insert(__hint, std::move(__v), __unique_keys()); + } +}; /// Specialization. template> iterator - insert(const_iterator, _Pair&& __v) - { return __iconv_type()(insert(std::forward<_Pair>(__v))); } + insert(const_iterator __hint, _Pair&& __v) + { + __hashtable& __h = this->_M_conjure_hashtable(); + return __h._M_emplace(__hint, __unique_keys(), +std::forward<_Pair>(__v)); + } }; /** @@ -1470,10 +1481,6 @@ using __ireturn_type = typename std::conditional<__unique_keys::value, std::pair, iterator>::type; - -using __iconv_type = typename std::conditional<__unique_keys::value, - _Select1st, _Identity - >::type; private: using _EqualEBO = _Hashtable_ebo_helper<0, _Equal>; using _EqualHelper = _Equal_helper<_Key, _Value, _ExtractKey, _Equal, Index: include/bits/hashtable.h === --- include/bits/hashtable.h (revision 200491) +++ include/bits/hashtable.h (working copy) @@ -225,7 +225,6 @@ using __node_base = typename __hashtable_base::__node_base; using __bucket_type = typename __hashtable_base::__bucket_type; using __ireturn_type = typename __hashtable_base::__ireturn_type; - using __iconv_type = typename __hashtable_base::__iconv_type; using __map_base = __detail::_Map_base<_Key, _Value, _Alloc, _ExtractKey, _Equal, _H1, _H2, _Hash, @@ -680,7 +679,8 @@ // Insert node with hash code __code. Take ownership of the node, // deallocate it on exception. iterator - _M_insert_multi_node(__hash_code __code, __node_type* __n); + _M_insert_multi_node(__node_type* __hint, + __hash_code __code, __node_type* __n); template std::pair @@ -688,16 +688,39 @@ template iterator - _M_emplace(std::false_type, _
Re: [patch] regex_traits implementation
On 29 June 2013 08:16, Tim Shen wrote: > Thank you for response! > > On Sat, Jun 29, 2013 at 7:08 AM, Paolo Carlini > wrote: >> Very nice to see progress in this area! Obvious question - I may have missed >> past discussions: is your paperwork, the copyright assignment, Ok? >> Otherwise, unfortunately, the technical review can't even begin. > > I've already sent a signed assignment form to FSF and got a response > saying that "Your assignment/disclaimer process with the FSF is > currently complete; your fully executed PDF will be sent to you in a > separate email immediately following this one". > > Shall I attach the PDF? No need, if the FSF have said you're good to go then that's fine. I'll take a look at the patch later today or tomorrow - thanks very much for working on it.