Re: working C11 thread implementation

[Szabolcs Nagy <nsz@port70.net>]

* Jens Gustedt <jens.gustedt@inria.fr> [2014-08-01 11:55:31 +0200]:
> for anybody interested I attach a first working version, that is
> mildly tested with a relatively big private application that I have
> and that uses C11 threads quite intensively.

nice

> The choices that affect the ABI that I mentioned are values of some
> constants and the sizes of the types that are introduced. These may
> change depending on glibc's choices and also according to future
> developments.
> 
> For the choices of the constants, for this version they are such that
> most wrapper calls result in being tail calls. I verified this by
> looking into the assembler that is produced. As they are now, most of
> these tail functions could also be just provided as weak aliases. We
> chose to implement them as functions such that in case of change of
> the constants we only have to recompile and no other maintenance is
> required.
> 

i'd assume the constants to be right and fix up the
code only in case of later breakage

> +  /* The following list of 9 integer constants makes up for the binary
> +     compatibility of this C thread implementation. You must never
> +     link code against versions of the C library that do not agree
> +     upon these ABI parameters.

nice

> +int cnd_signal(cnd_t * cnd) {
> +	/* In the best of all worlds this is a tail call. */
> +	int ret = __pthread_cond_signal(cnd);
> +	if (thrd_success)
> +		return ret ? thrd_error : thrd_success;
> +	return ret;
> +}

this is a bit weird

i think it's better to just assume thrd_success==0
and static assert it somewhere as a reminder

> +int mtx_init(mtx_t * m, int type)
> +{
> +	*m = (pthread_mutex_t) {
> +		._m_type = ((type&mtx_recursive) ? PTHREAD_MUTEX_RECURSIVE : 0),
> +	};
> +	return 0;
> +}

return thrd_success


> +/* This is needed because not all arch have __pthread_self as a static
> +   symbol. */
>  pthread_t pthread_self()
>  {
>  	return __pthread_self();

...

> +/* This is needed because not all arch have __pthread_self as a static
> +   symbol. */
> +pthread_t thrd_current()
> +{
> +	return __pthread_self();
> +}


i dont understand these comments

you mean that they could be weak aliases otherwise?

> +int thrd_join(thrd_t t, int *res)
> +{
> +	int tmp;
> +	while ((tmp = t->tid)) __timedwait(&t->tid, tmp, 0, 0, 0, 0, 0);
> +	if (res) *res = (int)(intptr_t)t->result;
> +	if (t->map_base) munmap(t->map_base, t->map_size);
> +	return 0;
> +}

thrd_success

> +int tss_set(tss_t k, void *x)
> +{
> +	struct pthread *self = __pthread_self();
> +	/* Avoid unnecessary COW */
> +	if (self->tsd[k] != x) {
> +		self->tsd[k] = x;
> +		self->tsd_used = 1;
> +	}
> +	return 0;
> +}

thrd_success

> +/* Roughly __syscall already returns the right thing: 0 if all went
> +   well or a negative error indication, otherwise. Unfortunately the C
> +   standard foresees the special case of an interrupted call and fixes
> +   that error return to -1 (instead of introducing EINTR). */
> +int thrd_sleep(const struct timespec *req, struct timespec *rem)
> +{
> +  int ret = __syscall(SYS_nanosleep, req, rem);
> +  // compile time comparison, constant propagated
> +  if (EINTR != 1) {
> +    switch (ret) {
> +    case -EINTR: return -1;
> +    case -1: return -EINTR;
> +    }
> +  }
> +  // potentially a tail call
> +  return ret;
> +}

"The thrd_sleep function returns zero if the requested
 time has elapsed, -1 if it has been interrupted by a
 signal, or a negative value if it fails."

this is confusing

(either should not have the name thrd_ or follow
the error enum convention of other thrd_ functions)