View Issue Details

IDProjectCategoryView StatusLast Update
0037093FPCCompilerpublic2020-05-22 09:12
ReporterNoName Assigned To 
PrioritynormalSeverityminorReproducibilityalways
Status newResolutionopen 
Product Version3.3.1 
Summary0037093: Add atomic intrinsics like Delphi
Descriptionhttp://docwiki.embarcadero.com/Libraries/Rio/en/System.AtomicCmpExchange
http://docwiki.embarcadero.com/Libraries/Rio/en/System.AtomicDecrement
http://docwiki.embarcadero.com/Libraries/Rio/en/System.AtomicExchange
http://docwiki.embarcadero.com/Libraries/Rio/en/System.AtomicIncrement
TagsNo tags attached.
Fixed in Revision
FPCOldBugId
FPCTarget
Attached Files

Activities

Thaddy de Koning

2020-05-17 06:51

reporter   ~0122870

We already have these intrinsics, but they can maybe aliased.
https://www.freepascal.org/docs-html/rtl/system/interlockedcompareexchange.html etc.
These are already cross-platform

Thaddy de Koning

2020-05-18 08:41

reporter   ~0122898

Last edited: 2020-05-18 09:14

View 6 revisions

[edit and tested]

{$macro on}
  {$define AtomicIncrement:=interlockedIncrement}
  {$define AtomicDecrement:=interlockedDecrement}
  {$define AtomicExchange:=interlockedExchange}
  {$define AtomicCompareExchange:=InterlockedCompareExchange}
  {$define AtomicExchangeAdd:=interlockedExchangeAdd} // not sure Delphi knows this
// tests
var
  P:longint = 0;
  R:longint = 0;
begin
  P:=atomicincrement(P);
  P:=atomicdecrement(P);
  P:=atomicexchange(P,R);
  P:=AtomicCompareExchange(P,R,0);
  P:=AtomicExchangeAdd(P,R);
end.

NoName

2020-05-19 00:21

reporter   ~0122919

Maybe provide a patch so that it gets added? :)

Thaddy de Koning

2020-05-19 07:12

reporter   ~0122922

I don't know if macro's are acceptable in the system unit.

Sven Barth

2020-05-19 09:27

manager   ~0122923

No, they are not. Also they won't work cross unit.

Thaddy de Koning

2020-05-19 10:08

reporter   ~0122925

Ok. will alias them proper.

Thaddy de Koning

2020-05-20 08:20

reporter   ~0122946

Last edited: 2020-05-20 09:41

View 4 revisions

Am I right to assume that given:
function InterlockedIncrement (var Target: longint) : longint; public name 'FPC_INTERLOCKEDINCREMENT';
I can simply do this:
function AtomicIncrement (var Target: longint) : longint; external name 'FPC_INTERLOCKEDINCREMENT';

That would make the patch easy and I can simply add the atomic name everywhere in systemh.inc

This works for my test.
{$assertions on}
{ delphi compatibility aliases }
function AtomicIncrement (var Target: longint) : longint; external name 'FPC_INTERLOCKEDINCREMENT';
function AtomicDecrement (var Target: longint) : longint; external name 'FPC_INTERLOCKEDDECREMENT';
function AtomicExchange (var Target: longint;Source : longint) : longint; external name 'FPC_INTERLOCKEDEXCHANGE';
function AtomicExchangeAdd (var Target: longint;Source : longint) : longint; external name 'FPC_INTERLOCKEDEXCHANGEADD';
function AtomicCmpExchange(var Target: longint; NewValue: longint; Comperand: longint): longint; external name 'FPC_INTERLOCKEDCOMPAREEXCHANGE';
// etc for all of the overloads.
var
  a:longint = 0;
  b:longint = 100;
begin
  a:=AtomicIncrement(a);
  assert(a = 1);
  a:=AtomicDecrement(a);
  assert(a = 0);
  b:=AtomicExchange(a,b);
  Assert((a =100) and (b = 0));
  b:=AtomicExchangeAdd(a,b);
  Assert((a =100) and (b = 100));
  b:=0;a:=100;
  b:=AtomicCmpExchange(a,b,0);
  Assert((a =100) and (b = 100));
end.

Sven Barth

2020-05-20 10:01

manager   ~0122949

Technically yes, however if you look at Delphi's documentation that NoName had linked there are more overloads.

Thaddy de Koning

2020-05-20 18:28

reporter   ~0122960

I am aware of that, but this gets me started. (I have to think how to implement the out parameters.
Note this is specific to Delphi for mobile. As per docs.

Bi0T1N

2020-05-20 20:42

reporter   ~0122962

Why not like this?


function AtomicCmpExchange(var Target: Integer; NewValue: Integer; Comparand: Integer; out Succeeded: Boolean): Integer; overload;
begin
  Result := InterlockedCompareExchange(Target, NewValue, Comparand);
  Succeeded := (Result = Comparand);
end;


I've used this approach for the TInterlocked class and it seems to work (at least for the few tests I have).
However, adding the function aliases would be appreciated by me - could use them in my class then.

Thaddy de Koning

2020-05-21 08:40

reporter   ~0122973

Last edited: 2020-05-21 08:55

View 4 revisions

I like to keep it clean and as close as possible to what's already there.
The actual public/external as is used in systemh.inc is more efficient and prevents duplicate code.
That combination will not grow code in any way, it is a straight alias.
So the way I approach it is 80% mere aliasing and - as Sven explained - there need to be some 20% extra, because some is missing.
The 80% is finished, the 20% not. Therefor I think I make two patches. Also note my remark that in Delphi it is strictly for mobile.
It is not that straightforward. Examine systemh.inc for yourself. Then re-examine the Delphi docs.

Sven Barth

2020-05-21 15:18

manager   ~0122981

It's not strictly mobile, because Delphi on Windows supports them as well. And it seems that they're providing intrinsics for any platform, which is why the descriptions look so strange (e.g. the untyped non-var parameter Decrement).

Thaddy de Koning

2020-05-21 15:40

reporter   ~0122983

I stick to the easy part, Sven. In 24 hours...

NoName

2020-05-21 18:14

reporter   ~0122985

Sven Barth is right. It's available/used for all platforms even if the docu says mobile compilers only...don't trust Embarcadero documentation ;)

NoName

2020-05-21 18:29

reporter   ~0122986

Probably should also implement the other Interlocked* functions:
- InterlockedAdd
- InterlockedAnd*
- InterlockedOr*
- InterlockedXor*
Source: https://docs.microsoft.com/en-us/windows/win32/api/winnt/nf-winnt-interlockedxor

instead of only the Delphi ones above. Then everything could be simply aliased!

NoName

2020-05-21 18:54

reporter   ~0122988

https://gitlab.gnome.org/GNOME/glib/-/blob/master/glib/gatomic.c

NoName

2020-05-21 19:01

reporter   ~0122989

from the Windows docu:
#include <stdio.h>
#include <intrin.h>

#pragma intrinsic(_InterlockedAnd)

int main()
{
        long data1 = 0xFF00FF00;
        long data2 = 0x00FFFF00;
        long retval;
        retval = _InterlockedAnd(&data1, data2);
        printf_s("0x%x 0x%x 0x%x", data1, data2, retval);
}

retval = _InterlockedAnd(&data1, data2); compiles to:
        lea rax, QWORD PTR data1$[rsp]
        mov QWORD PTR tv67[rsp], rax
        mov rax, QWORD PTR tv67[rsp]
        prefetchw BYTE PTR [rax]
        mov rcx, QWORD PTR tv67[rsp]
        mov eax, DWORD PTR [rcx]
$LL3@main:
        mov edx, eax
        and edx, DWORD PTR data2$[rsp]
        mov rcx, QWORD PTR tv67[rsp]
        mov rcx, QWORD PTR tv67[rsp]
        lock cmpxchg DWORD PTR [rcx], edx
        jne SHORT $LL3@main
        mov DWORD PTR retval$[rsp], eax

https://godbolt.org/z/QwdPiM

Thaddy de Koning

2020-05-22 09:12

reporter   ~0123001

I can't use that, not cross platform.
I am making progress, but your non-delphi suggestions would have to wait: feature request as opposed to Delphi compatibility.
Suggested patch for 80% mentioned will be added here today

Issue History

Date Modified Username Field Change
2020-05-17 01:44 NoName New Issue
2020-05-17 06:51 Thaddy de Koning Note Added: 0122870
2020-05-18 08:41 Thaddy de Koning Note Added: 0122898
2020-05-18 09:03 Thaddy de Koning Note Edited: 0122898 View Revisions
2020-05-18 09:05 Thaddy de Koning Note Edited: 0122898 View Revisions
2020-05-18 09:06 Thaddy de Koning Note Edited: 0122898 View Revisions
2020-05-18 09:10 Thaddy de Koning Note Edited: 0122898 View Revisions
2020-05-18 09:14 Thaddy de Koning Note Edited: 0122898 View Revisions
2020-05-19 00:21 NoName Note Added: 0122919
2020-05-19 07:12 Thaddy de Koning Note Added: 0122922
2020-05-19 09:27 Sven Barth Note Added: 0122923
2020-05-19 10:08 Thaddy de Koning Note Added: 0122925
2020-05-20 08:20 Thaddy de Koning Note Added: 0122946
2020-05-20 08:23 Thaddy de Koning Note Edited: 0122946 View Revisions
2020-05-20 09:34 Thaddy de Koning Note Edited: 0122946 View Revisions
2020-05-20 09:41 Thaddy de Koning Note Edited: 0122946 View Revisions
2020-05-20 10:01 Sven Barth Note Added: 0122949
2020-05-20 18:28 Thaddy de Koning Note Added: 0122960
2020-05-20 20:42 Bi0T1N Note Added: 0122962
2020-05-21 08:40 Thaddy de Koning Note Added: 0122973
2020-05-21 08:41 Thaddy de Koning Note Edited: 0122973 View Revisions
2020-05-21 08:53 Thaddy de Koning Note Edited: 0122973 View Revisions
2020-05-21 08:55 Thaddy de Koning Note Edited: 0122973 View Revisions
2020-05-21 15:18 Sven Barth Note Added: 0122981
2020-05-21 15:40 Thaddy de Koning Note Added: 0122983
2020-05-21 18:14 NoName Note Added: 0122985
2020-05-21 18:29 NoName Note Added: 0122986
2020-05-21 18:54 NoName Note Added: 0122988
2020-05-21 19:01 NoName Note Added: 0122989
2020-05-22 09:12 Thaddy de Koning Note Added: 0123001