Re: [PATCH] D24609: [ARM] Add missing Interlocked intrinsics
compnerd closed this revision. compnerd added a comment. r282447 https://reviews.llvm.org/D24609 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24609: [ARM] Add missing Interlocked intrinsics
mstorsjo added a comment. Ping, can someone commit this one now? https://reviews.llvm.org/D24609 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24609: [ARM] Add missing Interlocked intrinsics
mstorsjo added a comment. Can someone commit this patch now after it has been rebased? https://reviews.llvm.org/D24609 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24609: [ARM] Add missing Interlocked intrinsics
mstorsjo updated the summary for this revision.
mstorsjo updated this revision to Diff 72025.
mstorsjo added a comment.
Rebased on top of the latest master
https://reviews.llvm.org/D24609
Files:
lib/Headers/intrin.h
Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -490,6 +490,23 @@
long _PrevVal = __atomic_fetch_or(_BitBase, 1l << _BitPos, __ATOMIC_SEQ_CST);
return (_PrevVal >> _BitPos) & 1;
}
+#if defined(__arm__) || defined(__aarch64__)
+static __inline__ unsigned char __DEFAULT_FN_ATTRS
+_interlockedbittestandset_acq(long volatile *_BitBase, long _BitPos) {
+ long _PrevVal = __atomic_fetch_or(_BitBase, 1l << _BitPos, __ATOMIC_ACQUIRE);
+ return (_PrevVal >> _BitPos) & 1;
+}
+static __inline__ unsigned char __DEFAULT_FN_ATTRS
+_interlockedbittestandset_nf(long volatile *_BitBase, long _BitPos) {
+ long _PrevVal = __atomic_fetch_or(_BitBase, 1l << _BitPos, __ATOMIC_RELAXED);
+ return (_PrevVal >> _BitPos) & 1;
+}
+static __inline__ unsigned char __DEFAULT_FN_ATTRS
+_interlockedbittestandset_rel(long volatile *_BitBase, long _BitPos) {
+ long _PrevVal = __atomic_fetch_or(_BitBase, 1l << _BitPos, __ATOMIC_RELEASE);
+ return (_PrevVal >> _BitPos) & 1;
+}
+#endif
#ifdef __x86_64__
static __inline__ unsigned char __DEFAULT_FN_ATTRS
_BitScanForward64(unsigned long *_Index, unsigned __int64 _Mask) {
@@ -533,64 +550,521 @@
__atomic_fetch_or(_BitBase, 1ll << _BitPos, __ATOMIC_SEQ_CST);
return (_PrevVal >> _BitPos) & 1;
}
+#endif
/**\
|* Interlocked Exchange Add
\**/
+#if defined(__arm__) || defined(__aarch64__)
+static __inline__ char __DEFAULT_FN_ATTRS
+_InterlockedExchangeAdd8_acq(char volatile *_Addend, char _Value) {
+ return __atomic_fetch_add(_Addend, _Value, __ATOMIC_ACQUIRE);
+}
+static __inline__ char __DEFAULT_FN_ATTRS
+_InterlockedExchangeAdd8_nf(char volatile *_Addend, char _Value) {
+ return __atomic_fetch_add(_Addend, _Value, __ATOMIC_RELAXED);
+}
+static __inline__ char __DEFAULT_FN_ATTRS
+_InterlockedExchangeAdd8_rel(char volatile *_Addend, char _Value) {
+ return __atomic_fetch_add(_Addend, _Value, __ATOMIC_RELAXED);
+}
+static __inline__ short __DEFAULT_FN_ATTRS
+_InterlockedExchangeAdd16_acq(short volatile *_Addend, short _Value) {
+ return __atomic_fetch_add(_Addend, _Value, __ATOMIC_ACQUIRE);
+}
+static __inline__ short __DEFAULT_FN_ATTRS
+_InterlockedExchangeAdd16_nf(short volatile *_Addend, short _Value) {
+ return __atomic_fetch_add(_Addend, _Value, __ATOMIC_RELAXED);
+}
+static __inline__ short __DEFAULT_FN_ATTRS
+_InterlockedExchangeAdd16_rel(short volatile *_Addend, short _Value) {
+ return __atomic_fetch_add(_Addend, _Value, __ATOMIC_RELEASE);
+}
+static __inline__ long __DEFAULT_FN_ATTRS
+_InterlockedExchangeAdd_acq(long volatile *_Addend, long _Value) {
+ return __atomic_fetch_add(_Addend, _Value, __ATOMIC_ACQUIRE);
+}
+static __inline__ long __DEFAULT_FN_ATTRS
+_InterlockedExchangeAdd_nf(long volatile *_Addend, long _Value) {
+ return __atomic_fetch_add(_Addend, _Value, __ATOMIC_RELAXED);
+}
+static __inline__ long __DEFAULT_FN_ATTRS
+_InterlockedExchangeAdd_rel(long volatile *_Addend, long _Value) {
+ return __atomic_fetch_add(_Addend, _Value, __ATOMIC_RELEASE);
+}
+#endif
+#if defined(__x86_64__) || defined(__arm__) || defined(__aarch64__)
static __inline__ __int64 __DEFAULT_FN_ATTRS
_InterlockedExchangeAdd64(__int64 volatile *_Addend, __int64 _Value) {
return __atomic_fetch_add(_Addend, _Value, __ATOMIC_SEQ_CST);
}
+#endif
+#if defined(__arm__) || defined(__aarch64__)
+static __inline__ __int64 __DEFAULT_FN_ATTRS
+_InterlockedExchangeAdd64_acq(__int64 volatile *_Addend, __int64 _Value) {
+ return __atomic_fetch_add(_Addend, _Value, __ATOMIC_ACQUIRE);
+}
+static __inline__ __int64 __DEFAULT_FN_ATTRS
+_InterlockedExchangeAdd64_nf(__int64 volatile *_Addend, __int64 _Value) {
+ return __atomic_fetch_add(_Addend, _Value, __ATOMIC_RELAXED);
+}
+static __inline__ __int64 __DEFAULT_FN_ATTRS
+_InterlockedExchangeAdd64_rel(__int64 volatile *_Addend, __int64 _Value) {
+ return __atomic_fetch_add(_Addend, _Value, __ATOMIC_RELEASE);
+}
+#endif
/**\
|* Interlocked Exchange Sub
\**/
+#if defined(__x86_64__) || defined(__arm__) || defined(__aarch64__)
static __inline__ __int64 __DEFAULT_FN_ATTRS
_InterlockedExchangeSub64(__int64 volatile *_Subend, __int64 _Value) {
return __atomic_fetch_sub(_Subend, _Value, __ATOMIC_SEQ_CST);
}
+#endif
/**\
|* Interlocked Increment
\**/
+#if defined(__ar
Re: [PATCH] D24609: [ARM] Add missing Interlocked intrinsics
compnerd added a comment. Im happy to do so, but, could you please rebase the patch? https://reviews.llvm.org/D24609 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24609: [ARM] Add missing Interlocked intrinsics
mstorsjo added a comment. Thanks! Can you/someone commit it? https://reviews.llvm.org/D24609 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24609: [ARM] Add missing Interlocked intrinsics
compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. That sounds reasonable. https://reviews.llvm.org/D24609 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24609: [ARM] Add missing Interlocked intrinsics
mstorsjo added inline comments.
Comment at: lib/Headers/intrin.h:504
@@ +503,3 @@
+_interlockedbittestandset_acq(long volatile *_BitBase, long _BitPos) {
+ long _PrevVal = __atomic_fetch_or(_BitBase, 1l << _BitPos, __ATOMIC_ACQUIRE);
+ return (_PrevVal >> _BitPos) & 1;
compnerd wrote:
> mstorsjo wrote:
> > compnerd wrote:
> > > Perhaps we should add static asserts that _BitPos is within limits for
> > > the signed shift.
> > Sure, although I guess that also goes for the existing inline functions as
> > well?
> >
> > Which kind of assert would be suitable for that here? As far as I see,
> > static_assert is C++ only, while this header also can be used from C.
> >
> > If I try to add _Static_assert, which is usable in C, I get the following
> > error when compiling:
> >
> > intrin.h:499:18: error:
> > static_assert expression is not an integral constant expression
> > _Static_assert(_BitPos < 32, "_BitPos out of range");
> >
> > This even when I don't actually use the inline function anywhere, just
> > including intrin.h.
> Yeah, we would have to use `_Static_assert`, but that requires C11. It is
> possible to emulate it, but probably not worth the effort. I am worried
> though that we could introduce undefined behavior with an incorrect parameter.
Sure, there's probably such a risk. But this issue already exists - an almost
identical _interlockedbittestandset function already exists in the header - I'm
just adding new copies of it with different atomic semantics (__ATOMIC_ACQUIRE
etc).
So I'd ask if we can deal with that issue separately from this patch.
https://reviews.llvm.org/D24609
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24609: [ARM] Add missing Interlocked intrinsics
compnerd added inline comments.
Comment at: lib/Headers/intrin.h:504
@@ +503,3 @@
+_interlockedbittestandset_acq(long volatile *_BitBase, long _BitPos) {
+ long _PrevVal = __atomic_fetch_or(_BitBase, 1l << _BitPos, __ATOMIC_ACQUIRE);
+ return (_PrevVal >> _BitPos) & 1;
mstorsjo wrote:
> compnerd wrote:
> > Perhaps we should add static asserts that _BitPos is within limits for the
> > signed shift.
> Sure, although I guess that also goes for the existing inline functions as
> well?
>
> Which kind of assert would be suitable for that here? As far as I see,
> static_assert is C++ only, while this header also can be used from C.
>
> If I try to add _Static_assert, which is usable in C, I get the following
> error when compiling:
>
> intrin.h:499:18: error:
> static_assert expression is not an integral constant expression
> _Static_assert(_BitPos < 32, "_BitPos out of range");
>
> This even when I don't actually use the inline function anywhere, just
> including intrin.h.
Yeah, we would have to use `_Static_assert`, but that requires C11. It is
possible to emulate it, but probably not worth the effort. I am worried though
that we could introduce undefined behavior with an incorrect parameter.
https://reviews.llvm.org/D24609
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24609: [ARM] Add missing Interlocked intrinsics
mstorsjo added inline comments.
Comment at: lib/Headers/intrin.h:504
@@ +503,3 @@
+_interlockedbittestandset_acq(long volatile *_BitBase, long _BitPos) {
+ long _PrevVal = __atomic_fetch_or(_BitBase, 1l << _BitPos, __ATOMIC_ACQUIRE);
+ return (_PrevVal >> _BitPos) & 1;
compnerd wrote:
> Perhaps we should add static asserts that _BitPos is within limits for the
> signed shift.
Sure, although I guess that also goes for the existing inline functions as well?
Which kind of assert would be suitable for that here? As far as I see,
static_assert is C++ only, while this header also can be used from C.
If I try to add _Static_assert, which is usable in C, I get the following error
when compiling:
intrin.h:499:18: error:
static_assert expression is not an integral constant expression
_Static_assert(_BitPos < 32, "_BitPos out of range");
This even when I don't actually use the inline function anywhere, just
including intrin.h.
Comment at: lib/Headers/intrin.h:517
@@ -501,1 +516,3 @@
+}
+#endif
#ifdef __x86_64__
compnerd wrote:
> Are the names supposed to be all lower case?
Yes, these ones (for some reason) are all lower case:
https://msdn.microsoft.com/en-us/library/646k06sz.aspx
https://reviews.llvm.org/D24609
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24609: [ARM] Add missing Interlocked intrinsics
compnerd added inline comments.
Comment at: lib/Headers/intrin.h:504
@@ +503,3 @@
+_interlockedbittestandset_acq(long volatile *_BitBase, long _BitPos) {
+ long _PrevVal = __atomic_fetch_or(_BitBase, 1l << _BitPos, __ATOMIC_ACQUIRE);
+ return (_PrevVal >> _BitPos) & 1;
Perhaps we should add static asserts that _BitPos is within limits for the
signed shift.
Comment at: lib/Headers/intrin.h:517
@@ -501,1 +516,3 @@
+}
+#endif
#ifdef __x86_64__
Are the names supposed to be all lower case?
https://reviews.llvm.org/D24609
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
