Folding (a ? b : c) op d

2013-06-29 Thread Marc Glisse

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

2013-06-29 Thread Tim Shen
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

2013-06-29 Thread François Dumont
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

2013-06-29 Thread Jonathan Wakely
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.