On 7/25/18 6:52 PM, Mark Millard wrote:
>
>
> On 2018-Jul-25, at 2:10 PM, Mark Millard <marklmi at yahoo.com> wrote:
>
>
>
>> On 2018-Jul-25, at 10:09 AM, Mark Millard <marklmi at yahoo.com> wrote:
>>
>>> On 2018-Jul-25, at 8:39 AM, John Baldwin <jhb at freebsd.org> wrote:
>>>
>>>> On 7/24/18 11:39 PM, Mark Millard wrote:
>>>>> On 2018-Jul-24, at 10:32 PM, Mark Millard <marklmi at yahoo.com> wrote:
>>>>>
>>>>>> https://ci.freebsd.org/job/FreeBSD-head-amd64-gcc/6597/consoleText
>>>>>> (head -r336573 after the prior 6596's -r336565 ):
>>>>>>
>>>>>> --- all_subdir_lib/ofed ---
>>>>>> In file included from /workspace/src/contrib/ofed/librdmacm/cma.h:43:0,
>>>>>> from /workspace/src/contrib/ofed/librdmacm/acm.c:42:
>>>>>> /workspace/src/contrib/ofed/librdmacm/cma.h: In function 'fastlock_init':
>>>>>> /workspace/src/contrib/ofed/librdmacm/cma.h:60:2: error: invalid
>>>>>> initializer
>>>>>> atomic_store(&lock->cnt, 0);
>>>>>> ^
>>>>>> In file included from /workspace/src/contrib/ofed/librdmacm/acm.c:42:0:
>>>>>> /workspace/src/contrib/ofed/librdmacm/cma.h: In function
>>>>>> 'fastlock_acquire':
>>>>>> /workspace/src/contrib/ofed/librdmacm/cma.h:68:2: error: operand type
>>>>>> 'struct <anonymous> *' is incompatible with argument 1 of
>>>>>> '__atomic_fetch_add'
>>>>>> if (atomic_fetch_add(&lock->cnt, 1) > 0)
>>>>>> ^~
>>>>>> /workspace/src/contrib/ofed/librdmacm/cma.h: In function
>>>>>> 'fastlock_release':
>>>>>> /workspace/src/contrib/ofed/librdmacm/cma.h:73:2: error: operand type
>>>>>> 'struct <anonymous> *' is incompatible with argument 1 of
>>>>>> '__atomic_fetch_sub'
>>>>>> if (atomic_fetch_sub(&lock->cnt, 1) > 1)
>>>>>> ^~
>>>>>> . . .
>>>>>> --- all_subdir_lib/ofed ---
>>>>>> *** [acm.o] Error code 1
>>>>>>
>>>>>>
>>>>>> https://ci.freebsd.org/job/FreeBSD-head-amd64-gcc/6621/consoleText ( for
>>>>>> -r336700 ) still shows this type of error.
>>>>>
>>>>>
>>>>> [I should have a subject with "head -r336568 through -r336570 . . .".]
>>>>>
>>>>> From what I can tell looking around having something like:
>>>>>
>>>>> if (atomic_fetch_add(&lock->cnt, 1) > 0)
>>>>>
>>>>> involve a __atomic_fetch_add indicates that:
>>>>>
>>>>> /usr/local/lib/gcc/x86_64-unknown-freebsd12.0/6.4.0/include/stdatomic.h
>>>>>
>>>>> was in use instead of FreeBSD's stdatomic.h file.
>>>>>
>>>>> If this is right, then the issue may be tied to head -r335782
>>>>> implicitly changing the order of the include file directory
>>>>> searching for builds via the devel/*-gcc .
>>>>>
>>>>> (I reverted -r335782 in my environment some time ago and have
>>>>> not run into this problem in my context so far.)
>>>>
>>>> C11 atomics should work fine with compiler-provided headers since they
>>>> are a part of the language (and the system stdatomic.h simply attempts
>>>> to mimic the compiler-provided header in case it is missing).
>>>>
>>>> Simple standalone tests of _Atomic(int) with GCC don't trigger those
>>>> failures when using its stdatomic.h, so there is probably something else
>>>> going on with kernel includes being used while building the library,
>>>> etc. The last time we had this issue with stdarg.h it was because a
>>>> header shared between the kernel and userland always used
>>>> '<machine/stdarg.h>'
>>>> which is correct for the kernel but not for userland.
>>>
>>> I did misread the headers. FreeBSD has the likes of:
>>>
>>> #if defined(__CLANG_ATOMICS)
>>> . . .
>>> #define atomic_fetch_add_explicit(object, operand, order)
>>> \
>>> __c11_atomic_fetch_add(object, operand, order)
>>> . . .
>>> #elif defined(__GNUC_ATOMICS)
>>> . . .
>>> #define atomic_fetch_add_explicit(object, operand, order)
>>> \
>>> __atomic_fetch_add(&(object)->__val, operand, order)
>>> . . .
>>> #endif
>>> . . .
>>> #define atomic_fetch_add(object, operand)
>>> \
>>> atomic_fetch_add_explicit(object, operand, memory_order_seq_cst)
>>>
>>> so __atomic_fetch_add would occur.
>>>
>>> But so far I do not see the problem with -r335782 reverted. I last built
>>> -r336693 last night via devel/amd64-gcc (via xtoolchain).
>>>
>>> From what I can tell FreeBSD defines:
>>>
>>> #if !defined(__CLANG_ATOMICS)
>>> #define _Atomic(T) struct { volatile T __val; }
>>> #endif
>>>
>>> and that struct is being used in &(object)->__val is what the
>>> error reports are about. So that would be, for example,
>>>
>>> &(&lock->cnt)->__val
>>>
>>> This would appear to suggest that __val itself had a type meeting:
>>>
>>> operand type struct <anonymous>
>>>
>>> for T in _Atomic(T) .
>>>
>>> (This is independent of just what the issue traces back to: just
>>> the net result on ci.freebsd.org . No claim that you are right
>>> or wrong here. I'll not be looking any more until this afternoon
>>> or night.)
>>
>> Going in a somewhat different direction . . .
>>
>> Looking around I found https://bugs.llvm.org/show_bug.cgi?id=26462
>> which is titled:
>>
>> 26462 – GCC/clang C11 _Atomic incompatibility
>>
>> It appears that the normal source of platform ABI definitions are
>> not explicit/detailed in the area and allow for incompatibilities
>> in this area. clang and gcc made differing choices absent being
>> constrained to match.
>>
>> An example (a powerpc64 context was indicated):
>>
>> struct A16 { char val[16]; };
>> _Atomic struct A16 a16;
>> // GCC:
>> _Static_assert(_Alignof(a16) == 16, "");
>> // Clang:
>> _Static_assert(_Alignof(a16) == 1, "");
>>
>>
>> Non-power-of-2 is a general problem
>> (not a powerpc64 context from what I can
>> tell):
>>
>> struct A3 { char val[3]; };
>> _Atomic struct A3 a3;
>> // GCC:
>> _Static_assert(sizeof(a3) == 3, "");
>> _Static_assert(_Alignof(a3) == 1, "");
>> // Clang:
>> _Static_assert(sizeof(a3) == 4, "");
>> _Static_assert(_Alignof(a3) == 4, "");
>>
>>
>>
>> Comment 6 (by John McCall) is relevant:
>>
>> QUOTE
>> Anyway, while I prefer the Clang rule, the GCC rule is defensible, as are
>> any number of other rules. The important point, however, is that having
>> this discussion is not the right approach to solving this problem. The
>> layout of _Atomic(T) is ABI. ABI rules are not generally determined by
>> compiler implementors making things up as they go along, or at least they
>> shouldn't be. The Darwin ABI for _Atomic is the rule implemented in Clang,
>> which we actually did think about carefully when we adopted it. Other
>> platforms need to make their own call, and it probably shouldn't just be
>> "whatever's implemented in GCC", especially on other platforms where GCC is
>> not the system compiler.
>> END QUOTE
>>
>>
>> (I do nto claim to have proivided all the material that should
>> be read in https://bugs.llvm.org/show_bug.cgi?id=26462 .)
>>
>> It may be that FreeBSD needs to be the source of the ABI definitions
>> involved if clang and gcc freeBSD builds are to be interoperable in
>> this area. But this could mean avoiding builtins?
>>
>> If any of this is inlined and so not behind a more stable interface,
>> it looks like clang and gcc can not be mixed for the same instances
>> of various _Atomic possibilities.
>>
>>
>
>
> Going back to the code being compiled and
> confirming your note:
> ( /usr/src/contrib/ofed/librdmacm/cma.h )
>
> typedef struct {
> sem_t sem;
> _Atomic(int) cnt;
> } fastlock_t;
> . . .
> static inline void fastlock_acquire(fastlock_t *lock)
> {
> if (atomic_fetch_add(&lock->cnt, 1) > 0)
> sem_wait(&lock->sem);
> }
>
> So lock->cnt is an _Atomic(int) , i.e.,
>
> struct { volatile int __val; }
>
> so overall:
>
> typedef struct {
> sem_t sem;
> struct { volatile int __val; } cnt;
> } fastlock_t;
>
>
> for which: atomic_fetch_add(&lock->cnt, 1) has
> for A filled-in in the C11 language official:
>
> atomic_fetch_add (volatile A* obj, M arg)
>
> (generic function) being:
>
> atomic_fetch_add (volatile struct { volatile int __val; }* obj, M arg)
>
> and a direct type-check of that notation for obj would find:
>
> operand type 'struct <anonymous> *'
>
> and that would propagate to GCC's translation to __atomic_fetch_add
> via gcc's stdatomic.h :
>
> #define atomic_fetch_add(PTR, VAL) __atomic_fetch_add ((PTR), (VAL), \
> __ATOMIC_SEQ_CST)
>
> So, yes, it looks like the:
>
> #if !defined(__CLANG_ATOMICS)
> #define _Atomic(T) struct { volatile T __val; }
> #endif
So where are you seeing this btw? In head the conditional is different:
#if !defined(__cplusplus) && !__has_extension(c_atomic) && \
!__has_extension(cxx_atomic)
/*
* No native support for _Atomic(). Place object in structure to prevent
* most forms of direct non-atomic access.
*/
#define _Atomic(T) struct { T volatile __val; }
#endif
And external GCC does support those extensions so should end up not defining
an _Atomic macro, and in fact when I use -E with external GCC it leaves
_Atomic(int) alone:
% x86_64-unknown-freebsd11.2-gcc -E bar.c
...
# 1 "bar.c"
# 1 "/usr/include/sys/cdefs.h" 1 3 4
# 2 "bar.c" 2
# 1 "/usr/local/lib/gcc/x86_64-unknown-freebsd11.2/6.4.0/include/stdatomic.h" 1
3 4
# 29 "/usr/local/lib/gcc/x86_64-unknown-freebsd11.2/6.4.0/include/stdatomic.h"
3 4
# 29 "/usr/local/lib/gcc/x86_64-unknown-freebsd11.2/6.4.0/include/stdatomic.h"
3 4
typedef enum
{
memory_order_relaxed = 0,
memory_order_consume = 1,
memory_order_acquire = 2,
memory_order_release = 3,
memory_order_acq_rel = 4,
memory_order_seq_cst = 5
} memory_order;
typedef _Atomic _Bool atomic_bool;
typedef _Atomic char atomic_char;
...
So cdefs.h isn't overriding _Atomic and should be using the CLANG_ATOMICS case
for external GCC.
--
John Baldwin
_______________________________________________
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
