On 2018-Jul-26, at 10:21 AM, John Baldwin <jhb at FreeBSD.org> wrote: > 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. Some of my research was when I did not have FreeBSD available and was over the web and I did not cross check all of it later. I apparently looked at some older source at some point. I now see what you report for the #if test. Sorry for the confusion. However, there is in /usr/src/sys/sys/cdefs.h : /* * Testing against Clang-specific extensions. */ #ifndef __has_attribute #define __has_attribute(x) 0 #endif #ifndef __has_extension #define __has_extension __has_feature #endif #ifndef __has_feature #define __has_feature(x) 0 #endif so if those are clang specific then: #if !defined(__cplusplus) && !__has_extension(c_atomic) && \ !__has_extension(cxx_atomic) will test for C code: !0 && !0 && !0 and then the code will select to: #define _Atomic(T) struct { T volatile __val; } https://clang.llvm.org/docs/LanguageExtensions.html (for clang 7) lists: QUOTE __has_feature and __has_extension These function-like macros take a single identifier argument that is the name of a feature. __has_feature evaluates to 1 if the feature is both supported by Clang and standardized in the current language standard or 0 if not (but see below), while __has_extension evaluates to 1 if the feature is supported by Clang in the current language (either as a language extension or a standard language feature) or 0 if not. They can be used like this: #ifndef __has_feature // Optional of course. #define __has_feature(x) 0 // Compatibility with non-clang compilers. #endif #ifndef __has_extension #define __has_extension __has_feature // Compatibility with pre-3.0 compilers. #endif END QUOTE so I think that: #define _Atomic(T) struct { T volatile __val; } is being done. === Mark Millard marklmi at yahoo.com ( dsl-only.net went away in early 2018-Mar)Received on Thu Jul 26 2018 - 15:56:06 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:41:17 UTC