Re: [jira] Created: (STDCXX-1022) [MSVC x86 / optimized] ICE in std::__make_heap()
Farid Zaripov-2 wrote: > >> Did you happen to reduce this to a small test case and send >> it to Microsoft? > > Not yet. It's difficult to create the small test. > >> Do you see the ICE in a specific test (or example), maybe only for a >> specific instantiation of the template, or does it happen regardless >> of the actual types of the template arguments? > > Today I've made some investigations: > > 1. the 25.heap, 25.partial.sort and 25.sort tests from trunk and 4.3.x > branch > are compiled withoud ICE; > > 2. the only difference between preprocessed 25.heap tests from 4.2.x > branch and trunk is > > - > Comparing files 42x.i and trunk.i > * 42x.i > void __declspec (noreturn) > __rw_assert_fail (const char*, const char*, int, const char*) > * trunk.i > void > __rw_assert_fail (const char*, const char*, int, const char*) > * > - > > 3. removing _RWSTD_NORETURN from declaration of the __rw_assert_fail() > eliminates > the ICE in tests from 4.2.x branch; > > Thanks for looking into it! That's too bad about noreturn. It can help the compiler generate better code so it would be a shame to have to remove it. A test case might help us come up with a workaround and get Microsoft to fix their compiler :) > > 4. The __rw_assert_fail() is used in RandomAccessIter methods thus > RW_ASSERT macro; > > So I guess, that MSVC issues ICE when processing "infinite" for-loop > (loop without condition) > with __declspec (noreturn) function inside the loop. > > >> Btw., the reason I ask is so we can come up with a better/cleaner >> workaround, hopefully one that will be as efficient as the original >> code and won't need an #ifdef. > > I've just added condition in for-loop, to make the loop finite. > We can rewrite the loop the same way, as it's implemented in Dinkumware > STL > (it's also fixes the ICE): > > ... > > _EXPORT > template > void __make_heap (_RandomAccessIter __first, _RandomAccessIter __last, > _Compare __comp, _Dist*) > { > _RWSTD_ASSERT_RANGE (__first, __last); > > const _Dist __dist = __last - __first; > > for (_Dist __parent = __dist / 2; 0 < __parent; ) { > --__parent; > __adjust_heap (__first, __parent, __dist, *(__first + __parent), >__comp); > } > } > > This is close to what I had in mind when I saw the #ifdef but is it the same thing? IIUC, in the original version, the loop will execute at least once. In this version, it may not execute at all (i.e., if __parent is zero). Or does it not matter? If not, this would seem like an improvement over the original code irrespective of the ICE, which is exactly what I was hoping for! :) Martin -- View this message in context: http://www.nabble.com/Re%3A--jira--Created%3A-%28STDCXX-1022%29--MSVC-x86---optimized--ICE-in-std%3A%3A__make_heap%28%29-tp20532273p20553378.html Sent from the stdcxx-dev mailing list archive at Nabble.com.
Re: svn commit: r713762 - /stdcxx/branches/4.2.x/include/rw/_config.h
Farid Zaripov-2 wrote: > >> I wonder if it would be worthwhile to give users the ability to >> decide whether to enable TLS in case they don't need LoadLibrary() >> or not. What do you think? > > Hmm. I only can say, that nor MSVC run-time, nor STLport nor boost > libraries are not > using the implicit TLS. > > Currently the TLS variables are used in 3 places only: > - exception's what-buffer; > - table for random number generator; > - buffer for __rw_tmpbuf(). > > Right. But there might be other opportunities for TLS (e.g., in locale or maybe in some of the C++ 0x facilities?) > > We need to check what would be if an exception object is created in one > thread (i.e. thus "throw new exception();") > and after catch() the pointer passed to another thread and there deleted? > > Ouch! Tricky! I hadn't thought of this when I implemented it. I think we either need to get this case to work or disable TLS for exceptions, but we can't have it crash (which, I assume, is what happens in this case?) > > The same issue with __rw_tmpbuf(). What would be if we getting the > temporary buffer in one thread using > get_temporary_buffer(), and releasing it, using return_temporary_buffer(), > in another thread? > > That would also be a problem. In this case, though, I think it would be sufficient to document it as a restriction of the API. IMO, getting this to work would be more trouble than it's worth. Martin -- View this message in context: http://www.nabble.com/Re%3A-svn-commit%3A-r713762stdcxx-branches-4.2.x-include-rw-_config.h-tp20518249p20553291.html Sent from the stdcxx-dev mailing list archive at Nabble.com.
Re: svn commit: r713762 - /stdcxx/branches/4.2.x/include/rw/_config.h
> I wonder if it would be worthwhile to give users the ability to > decide whether to enable TLS in case they don't need LoadLibrary() > or not. What do you think? Hmm. I only can say, that nor MSVC run-time, nor STLport nor boost libraries are not using the implicit TLS. Currently the TLS variables are used in 3 places only: - exception's what-buffer; - table for random number generator; - buffer for __rw_tmpbuf(). We need to check what would be if an exception object is created in one thread (i.e. thus "throw new exception();") and after catch() the pointer passed to another thread and there deleted? The same issue with __rw_tmpbuf(). What would be if we getting the temporary buffer in one thread using get_temporary_buffer(), and releasing it, using return_temporary_buffer(), in another thread? Farid.
Re: [jira] Created: (STDCXX-1022) [MSVC x86 / optimized] ICE in std::__make_heap()
> Did you happen to reduce this to a small test case and send > it to Microsoft? Not yet. It's difficult to create the small test. > Do you see the ICE in a specific test (or example), maybe only for a > specific instantiation of the template, or does it happen regardless > of the actual types of the template arguments? Today I've made some investigations: 1. the 25.heap, 25.partial.sort and 25.sort tests from trunk and 4.3.x branch are compiled withoud ICE; 2. the only difference between preprocessed 25.heap tests from 4.2.x branch and trunk is - Comparing files 42x.i and trunk.i * 42x.i void __declspec (noreturn) __rw_assert_fail (const char*, const char*, int, const char*) * trunk.i void __rw_assert_fail (const char*, const char*, int, const char*) * - 3. removing _RWSTD_NORETURN from declaration of the __rw_assert_fail() eliminates the ICE in tests from 4.2.x branch; 4. The __rw_assert_fail() is used in RandomAccessIter methods thus RW_ASSERT macro; So I guess, that MSVC issues ICE when processing "infinite" for-loop (loop without condition) with __declspec (noreturn) function inside the loop. > Btw., the reason I ask is so we can come up with a better/cleaner > workaround, hopefully one that will be as efficient as the original > code and won't need an #ifdef. I've just added condition in for-loop, to make the loop finite. We can rewrite the loop the same way, as it's implemented in Dinkumware STL (it's also fixes the ICE): --- template inline void _Make_heap(_RanIt _First, _RanIt _Last, _Pr _Pred, _Diff *, _Ty *) { // make nontrivial [_First, _Last) into a heap, using _Pred _Diff _Bottom = _Last - _First; for (_Diff _Hole = _Bottom / 2; 0 < _Hole; ) { // reheap top half, bottom to top --_Hole; std::_Adjust_heap(_First, _Hole, _Bottom, _Ty(*(_First + _Hole)), _Pred); } } --- - _EXPORT template void __make_heap (_RandomAccessIter __first, _RandomAccessIter __last, _Compare __comp, _Dist*) { _RWSTD_ASSERT_RANGE (__first, __last); const _Dist __dist = __last - __first; for (_Dist __parent = __dist / 2; 0 < __parent; ) { --__parent; __adjust_heap (__first, __parent, __dist, *(__first + __parent), __comp); } } - Farid.