[Mesa-dev] [PATCH 03/25] threads: update for late C11 changes

[Brian Paul]

On 10/25/2017 10:55 PM, Jose Fonseca wrote:
> On 23/10/17 20:15, Emil Velikov wrote:
>> Hi Nicolai,
>>
>> On 22 October 2017 at 20:07, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>>> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>>>
>>> C11 threads were changed to use struct timespec instead of xtime, and
>>> thrd_sleep got a second argument.
>>>
>> As xtime was replaced with timespec there's a couple of odd bits in
>> the code.
>>
>>
>>> See http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1554.htm and
>>> http://en.cppreference.com/w/c/thread/{thrd_sleep,cnd_timedwait,mtx_timedlock}
>>>
>>>
>>> Note that cnd_timedwait is spec'd to be relative to TIME_UTC /
>>> CLOCK_REALTIME.
>>>
>>> Cc: Jose Fonseca <jfonseca at vmware.com>
>>> ---
>>>   include/c11/threads.h           | 11 -----------
>>>   include/c11/threads_posix.h     | 39
>>> +++++++++++++++------------------------
>>>   include/c11/threads_win32.h     | 37
>>> +++++++++++++++++++------------------
>>>   src/egl/drivers/dri2/egl_dri2.c | 24 +++++++++++++-----------
>>>   4 files changed, 47 insertions(+), 64 deletions(-)
>>>
>>> diff --git a/include/c11/threads.h b/include/c11/threads.h
>>> index 573348d8091..3c3f23a8ab8 100644
>>> --- a/include/c11/threads.h
>>> +++ b/include/c11/threads.h
>>> @@ -23,42 +23,31 @@
>>>    * FITNESS FOR A PARTICULAR PURPOSE, TITLE AND NON-INFRINGEMENT. IN
>>> NO EVENT
>>>    * SHALL THE COPYRIGHT HOLDERS OR ANYONE DISTRIBUTING THE SOFTWARE
>>> BE LIABLE
>>>    * FOR ANY DAMAGES OR OTHER LIABILITY, WHETHER IN CONTRACT, TORT OR
>>> OTHERWISE,
>>>    * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
>>> USE OR OTHER
>>>    * DEALINGS IN THE SOFTWARE.
>>>    */
>>>   #ifndef EMULATED_THREADS_H_INCLUDED_
>>>   #define EMULATED_THREADS_H_INCLUDED_
>>>
>>>   #include <time.h>
>>> -#ifdef _MSC_VER
>>> -#include <thr/xtimec.h>  // for xtime
>>> -#endif
>>>
>>>   #ifndef TIME_UTC
>>>   #define TIME_UTC 1
>>>   #endif
>>>
>>>   #include "c99_compat.h" /* for `inline` */
>>>
>>>   /*---------------------------- types ----------------------------*/
>>>   typedef void (*tss_dtor_t)(void*);
>>>   typedef int (*thrd_start_t)(void*);
>>>
>>> -#ifndef _MSC_VER
>>> -struct xtime {
>>> -    time_t sec;
>>> -    long nsec;
>>> -};
>>> -typedef struct xtime xtime;
>>> -#endif
>>> -
>>
>> We don't have a fall-back declaration of the struct, yet we use it
>> below and provide a timespec_get() implementation.
>> I'd imagine you haven't tested this on Windows (hence Jose in CC)?
>>
>> Quick search suggests that MSVC 2015 was the first one that introduces
>> the struct and timespec_get.
>>
>> If we're safe as-is, please add a comment with some details - I'd
>> imagine Jose had better knowledge in the area.
>>
>>
>>>   /*-------------------- 7.25.7 Time functions --------------------*/
>>>   // 7.25.6.1
>>> +#if 0
>> I'd just drop the hunk mentioning that timespec_get() is part of time.h
>>
>>
>> Thank
>> Emil
>
> If there's doubt, I suggest testing Visual Studio with AppVeyor by
> pushing the changes as a feature branch to FDO's git -- I believe that
> should trigger an AppVeyor build.  (Push to a github repos hooked into
> Appveyor, depending on what people are more confortable with.)
>
> Alternatively reach out to Brian or Roland.

I applied this patch to today's ToT and got some compiler errors:


scons: Building targets ...
cl /Fobuild\windows-x86-debug\gallium\tests\unit\pipe_barrier_test.obj 
/c src\ga
llium\tests\unit\pipe_barrier_test.c /nologo /Od /Oi /Oy- /W3 /wd4018 
/wd4056 /w
d4244 /wd4267 /wd4305 /wd4351 /wd4756 /wd4800 /wd4996 /arch:SSE2 /MTd 
-FIinttype
s.h /D__STDC_CONSTANT_MACROS /D__STDC_FORMAT_MACROS 
/D__STDC_LIMIT_MACROS /DHAVE
_NO_AUTOCONF /DDEBUG /DWIN32 /D_WINDOWS /D_WIN32_WINNT=0x0601 
/DWINVER=0x0601 /D
VC_EXTRALEAN /D_USE_MATH_DEFINES /D_CRT_SECURE_NO_WARNINGS 
/D_CRT_SECURE_NO_DEPR
ECATE /D_SCL_SECURE_NO_WARNINGS /D_SCL_SECURE_NO_DEPRECATE 
/D_ALLOW_KEYWORD_MACR
OS /D_HAS_EXCEPTIONS=0 /D_DEBUG /DPIPE_SUBSYSTEM_WINDOWS_USER 
/DPACKAGE_VERSION=
\"17.4.0-devel\" 
/DPACKAGE_BUGREPORT=\"https://bugs.freedesktop.org/enter_bug.cg
i?product=Mesa\" /Iinclude /Isrc\gallium\include /Isrc\gallium\auxiliary 
/Isrc\g
allium\drivers /Isrc\gallium\winsys /Ibuild\windows-x86-debug /Isrc 
/Isrc /Ibuil
d\windows-x86-debug\gallium\tests\unit\indices 
/Isrc\gallium\tests\unit\indices
/Ibuild\windows-x86-debug\gallium\tests\unit\util 
/Isrc\gallium\tests\unit\util
/Z7
pipe_barrier_test.c
c:\users\brian\projects\mesa\include\c11\threads_win32.h(389): error 
C2065: 'ts'
: undeclared identifier
c:\users\brian\projects\mesa\include\c11\threads_win32.h(389): warning 
C4047: 'f
unction': 'const timespec *' differs in levels of indirection from 'int'
c:\users\brian\projects\mesa\include\c11\threads_win32.h(389): warning 
C4024: 'i
mpl_cond_do_wait': different types for formal and actual parameter 3
c:\users\brian\projects\mesa\include\c11\threads_win32.h(639): error 
C2084: func
tion 'int timespec_get(timespec *const ,const int)' already has a body
C:\Program Files\Windows Kits\10\include\10.0.10240.0\ucrt\time.h(539): 
note: se
e previous definition of 'timespec_get'
scons: *** 
[build\windows-x86-debug\gallium\tests\unit\pipe_barrier_test.obj] Er
ror 2
scons: building terminated because of errors.


I don't have time to investigate/fix.  Maybe you can, Nicolai.

-Brian