Re: [patch ping] C6X unwinding/exception handling

Ping for libstdc++ bits?

(Tested on arm-linux-gnueabi and c6x-elf, lightly tested on i686-linux)

http://gcc.gnu.org/ml/libstdc++/2011-08/msg00013.html

> C6X uses an unwinding/exception handling echeme very similar to that
> defined by the ARM EABI.  The core of the unwinder is the same, so I've
> pulled it out into a common file.
> 
> Other than the obvious target specific bits, the main compiler visible
> difference is that the C6X assembler generates the unwinding tables from
> DWARF .cfi directives, rather than the separate set of directives used by
> the ARM assembler.
> 
> The libstdc++ changes probably deserve a bit of explanation. The ttype_base
> field was clearly used in an early draft of the ARM EABI, and the current
> ARM definition is a compatible subset of that used by C6X.
> _GLIBCXX_OVERRIDE_TTYPE_ENCODING is an unfortunate hack because when doing
> the ARM implementation I failed to realise ttype_encoding was the same
> thing as R_ARM_TARGET2.  We now have a lot of ARM binaries floating around
> with that field set incorrectly, so it's either this or an ABI bump.
> 
> Ok?
> 
> Paul
> 
> 2011-08-04  Paul Brook  <paul <at> codesourcery.com>
> ...
> 

Re: [patch ping] C6X unwinding/exception handling

On 1 September 2011 12:07, Paul Brook wrote:
> Ping for libstdc++ bits?

If it really is that or an ABI bump, then I guess we want that 

I'd feel more comfortable with another maintainer approving it though,
I'm not competent to review the EH code.

> (Tested on arm-linux-gnueabi and c6x-elf, lightly tested on i686-linux)
>
> http://gcc.gnu.org/ml/libstdc++/2011-08/msg00013.html
>
>> C6X uses an unwinding/exception handling echeme very similar to that
>> defined by the ARM EABI.  The core of the unwinder is the same, so I've
>> pulled it out into a common file.
>>
>> Other than the obvious target specific bits, the main compiler visible
>> difference is that the C6X assembler generates the unwinding tables from
>> DWARF .cfi directives, rather than the separate set of directives used by
>> the ARM assembler.
>>
>> The libstdc++ changes probably deserve a bit of explanation. The ttype_base
>> field was clearly used in an early draft of the ARM EABI, and the current
>> ARM definition is a compatible subset of that used by C6X.
>> _GLIBCXX_OVERRIDE_TTYPE_ENCODING is an unfortunate hack because when doing
>> the ARM implementation I failed to realise ttype_encoding was the same
>> thing as R_ARM_TARGET2.  We now have a lot of ARM binaries floating around
>> with that field set incorrectly, so it's either this or an ABI bump.
>>
>> Ok?

Re: hash policy patch

Hi,
>     After some additional time spent on hashtable I prefer to do this 
> new proposal. It fixes
> - Yet some issues with hash policy that was still able to give a 
> bucket count for which load_factor == max_load_factor, Standard says 
> load_factor < max_load_factor. Note that in fact the refactoring to 
> generalize use of _M_next_bkt had indeed a side effect which is that 
> when looking for instance for 11.5 lower_bound was returning 13 but 
> now that it is casted to integer it will return 11. Not only that 
> reason now _M_next_bkt takes an optional __strict bool parameter 
> signaling if the returned value shall be not only not shorter but even 
> larger.
> - In hashtable implementation I removed usages of std::max that was 
> potentially leaving the hashtable in an inconsistent state with a hash 
> policy next resize value not matching the current bucket count. It was 
> not really a bug because the next resize value was updated on the next 
> insertion but at the cost of a useless floating point operation.
> - I deal with allocation failure directly in _M_rehash method to avoid 
> introducing new try/catch blocks. I also reset hash policy next resize 
> value when the container is emptied on a hash functor exception.
> - In __rehash_policy I only commit the new hash policy instance if the 
> rehash operation succeeded. The associated test change_load_factor.cc 
> requires exception support, is there already a way to detect it or I 
> need to add a new dg-require-exceptions dejaGnu macro ?
>
Today, I had time to look a bit more into these issues. I think we 
should handle one change at a time. About the first one above, I don't 
like the new __strict parameter, looks like we are going through this 
complication only because we are refactoring to use _M_next_bkt, because 
otherwise, if I understand correctly, we are not really incorrect, since 

testsuite allocator patch

Hi

     A small patch for a small issue detected while playing with 
tracker_allocator. I considered that allocation count should represent 
the number of allocated bytes and not the number of requested bytes.

2011-09-01  François Dumont <frs.dumont <at> gmail.com>

     * testsuite/util/testsuite_allocator.h 
(tracker_allocator_counter::allocate): Update
     allocation count only if allocation succeeded.

Tested under linux x86_64.

François

Re: hash policy patch

On 09/01/2011 08:36 PM, Paolo Carlini wrote:
> Hi,
> Today, I had time to look a bit more into these issues. I think we
> should handle one change at a time. About the first one above, I don't
> like the new __strict parameter, looks like we are going through this
> complication only because we are refactoring to use _M_next_bkt, because
> otherwise, if I understand correctly, we are not really incorrect, since
> we are talking about something like *strict* equality of *floating*
> point quantities, by itself something badly defined (indeed, carefully,
> the standard talks about "keeping the load factor below this number",
> using plain English, not a formula).
>
> I think we can delay point 2.
>
> For points 3 and 4 above, I would like to see separate patches and
> separate testcases. Is it possible?
>
> Also, please be more descriptive in the ChangeLog entry, saying which
> specific functions are touched. Then, splitting the big patch will also
> help clarifying the rationale behind each smaller one.
>
> Paolo.
>
Hi Paolo

     Thanks for the feedback, I agree that splitting the patch will be 
better. I was also not really proud of the __strict parameter. First 
step tomorrow I hope.

François

[v3] Fix libstdc++/50268

Hi,

tested x86_64-linux multilib, will go in 4_6-branch too.

See audit trail for details: in short when std::bitset has been updated 
for constexpr, thus the constructor from unsigned long long marked as 
such, the call to _M_do_sanitize in the body was inadvertently 
completely removed without replacing it with something else compatible 
with constexpr.

Thanks,
Paolo.

/////////////////////

2011-09-02  Paolo Carlini  <paolo.carlini <at> oracle.com>
	    Marc Glisse  <marc.glisse <at> normalesup.org>

	PR libstdc++/50268
	* include/std/bitset (struct _Sanitize_val): Add.
	(bitset<>::bitset(unsigned long long)): Fix.
	* testsuite/23_containers/bitset/cons/50268.cc: New.

Index: include/std/bitset
===================================================================
--- include/std/bitset	(revision 178444)
+++ include/std/bitset	(working copy)
 <at>  <at>  -52,11 +52,13  <at>  <at> 

Re: testsuite allocator patch

On 09/01/2011 10:20 PM, François Dumont wrote:
> 2011-09-01  François Dumont <frs.dumont <at> gmail.com>
>
>     * testsuite/util/testsuite_allocator.h 
> (tracker_allocator_counter::allocate): Update
>     allocation count only if allocation succeeded.
Applied as attached, I took the occasion to remove some spaces.

Careful with the 80 columns limit, for ChangeLog entries too. Also, I 
used your email per MAINTAINERS, if you want to change it in the 
ChangeLog entries, please change it in the MAINTAINERS file first.

Thanks,
Paolo.

////////////////////////

2011-09-02  François Dumont  <fdumont <at> gcc.gnu.org>

	* testsuite/util/testsuite_allocator.h (tracker_allocator_counter::
	allocate): Update allocation count only if allocation succeeded.

Index: testsuite/util/testsuite_allocator.h
===================================================================
--- testsuite/util/testsuite_allocator.h	(revision 178444)
+++ testsuite/util/testsuite_allocator.h	(working copy)
 <at>  <at>  -37,14 +37,15  <at>  <at> 
   {

Re: testsuite allocator patch

On 09/02/2011 06:01 PM, Paolo Carlini wrote:
> On 09/01/2011 10:20 PM, François Dumont wrote:
>> 2011-09-01  François Dumont <frs.dumont <at> gmail.com>
>>
>>     * testsuite/util/testsuite_allocator.h 
>> (tracker_allocator_counter::allocate): Update
>>     allocation count only if allocation succeeded.
> Applied as attached, I took the occasion to remove some spaces.
>
> Careful with the 80 columns limit, for ChangeLog entries too. Also, I 
> used your email per MAINTAINERS, if you want to change it in the 
> ChangeLog entries, please change it in the MAINTAINERS file first.
>
> Thanks,
> Paolo.
>
> ////////////////////////
Thanks

     I didn't know that the ChangeLog email had to match the MAINTAINERS 
entry, from now on I will use the gcc email.

Regards

[v3] libstdc++/50257

Hi,

tested x86_64-linux, committed to mainline. See audit trail for details...

Thanks,
Paolo.

/////////////////////

2011-09-06  Paolo Carlini  <paolo.carlini <at> oracle.com>

	PR libstdc++/50257
	* include/bits/hashtable_policy.h (_Prime_rehash_policy::
   	_M_next_bkt): Optimize for small argument.

Index: include/bits/hashtable_policy.h
===================================================================
--- include/bits/hashtable_policy.h	(revision 178574)
+++ include/bits/hashtable_policy.h	(working copy)
 <at>  <at>  -427,8 +427,15  <at>  <at> 
   _Prime_rehash_policy::
   _M_next_bkt(std::size_t __n) const
   {
-    const unsigned long __p = *std::lower_bound(__prime_list, __prime_list
-						+ _S_n_primes, __n);
+    // Optimize lookups involving the first elements of __prime_list.
+    // (useful to speed-up, eg, constructors)
+    static const unsigned char __fastbkt[12]

Re: hash policy patch

Hi,
>     Thanks for the feedback, I agree that splitting the patch will be 
> better. I was also not really proud of the __strict parameter.
what about doing simply something like the attached for this part of the 
work? Lightly tested...

Paolo.

///////////////////

Index: include/bits/hashtable_policy.h
===================================================================
--- include/bits/hashtable_policy.h	(revision 178581)
+++ include/bits/hashtable_policy.h	(working copy)
 <at>  <at>  -446,14 +446,7  <at>  <at> 
   inline std::size_t
   _Prime_rehash_policy::
   _M_bkt_for_elements(std::size_t __n) const
-  {
-    const float __min_bkts = __n / _M_max_load_factor;
-    const unsigned long __p = *std::lower_bound(__prime_list, __prime_list
-						+ _S_n_primes, __min_bkts);
-    _M_next_resize =
-      static_cast<std::size_t>(__builtin_floor(__p * _M_max_load_factor));
-    return __p;
-  }
+  { return _M_next_bkt(__builtin_ceil(__n / _M_max_load_factor)); }

   // Finds the smallest prime p such that alpha p > __n_elt + __n_ins.