View Issue Details

IDProjectCategoryView StatusLast Update
0037139FPCCompilerpublic2020-06-07 22:50
ReporterBi0T1N Assigned To 
PrioritynormalSeverityminorReproducibilityalways
Status newResolutionopen 
Product Version3.3.1 
Summary0037139: Add InterlockedAdd compiler intrinsic
DescriptionThe InterlockedAdd intrinsic should perform an atomic addition operation with the specified values.
It works like the InterlockedIncrement/InterlockedDecrement function, just with a second operand - the number instead of using hardcoded 1/-1 in the assembler.

Additionally InterlockedIncrement/InterlockedDecrement could be replaced by InterlockedAdd.
Additional Informationhttps://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html#g_t_005f_005fatomic-Builtins (__atomic_add_fetch )
https://docs.microsoft.com/en-us/windows/win32/api/winnt/nf-winnt-interlockedadd
TagsNo tags attached.
Fixed in Revision
FPCOldBugId
FPCTarget
Attached Files

Activities

Thaddy de Koning

2020-05-25 09:14

reporter   ~0123055

Last edited: 2020-05-25 09:15

View 2 revisions

This is being worked on. This intrinsic is already there, but we can add an alias.
First I will do the Atomic vs Interlocked aliases. Then will evaluate what is still missing.
This one is already there in several forms. As you can examine in systemh.inc.

Jonas Maebe

2020-05-25 11:38

manager   ~0123057

There is already InterlockedExchangeAdd which does that.

Bi0T1N

2020-05-29 17:39

reporter   ~0123129

Last edited: 2020-05-29 17:39

View 2 revisions

InterlockedExchangeAdd doesn't do exactly the same as it returns the previous value while InterlockedAdd returns the result of the operation (=new value).
Following the GCC naming convention InterlockedExchangeAdd does fetch + add while InterlockedAdd does add + fetch.


function InterlockedAdd(var Target: LongInt; Source: LongInt):LongInt;
var
  OldValue: LongInt;
begin
  OldValue := InterlockedExchangeAdd(Target, Source);
  Result := OldValue + Source;
end;


but calling InterlockedAdd with 1/-1 would be the same as InterlockedIncrement/InterlockedDecrement and thus they are obsolete once implemented.

Jonas Maebe

2020-05-29 21:52

manager   ~0123133

We generally try to be compatible with Delphi code rather than gcc, and I don't see any reason why we would want to break backward compatibility by removing InterlockedIncrement/Decrement. I really don't see the advantage of adding these intrinsics, as it's not like the semantics of InterlockedAdd are better than those of InterlockedExchangeAdd (nor worse, for that matter).

Sven Barth

2020-05-30 11:29

manager   ~0123144

Plus the System.Interlocked* functions are usually implemented in assembly and it might be that a platform can handle Increment/Decrement using a more efficient instruction sequence than an arbitrary Add.

Bi0T1N

2020-05-30 21:39

reporter   ~0123150

Last edited: 2020-05-30 22:13

View 4 revisions

Actually it's needed for Delphi compatibility - they just changed their naming convention and named it AtomicIncrement.
However, I think it could be added in system.inc with the implementation I gave above as it needs less changes than changing the existing implementations of InterlockedIncrement/InterlockedDecrement to accept a second argument.
And I didn't checked all assembly implementations but all I've seen are using hardcoded 1/-1 and that's why I said InterlockedIncrement/InterlockedDecrement could also be replaced with the new function which accepts the value as a second parameter. I didn't said it has to be replaced.

Bi0T1N

2020-06-07 22:50

reporter   ~0123320

Please find attached a patch which adds InterlockedAdd and AtomicIncrement (Delphi compatibility). If only Delphi compatibility is desired I could also provide a patch for that.
01-Add_InterlockedAdd_and_AtomicIncrement.patch (6,392 bytes)   
diff --git rtl/inc/system.inc rtl/inc/system.inc
index f4489ceb98..a9b648c972 100644
--- rtl/inc/system.inc
+++ rtl/inc/system.inc
@@ -869,6 +869,68 @@ function InterLockedCompareExchangePointer (var Target: pointer; NewValue: point
 {$endif FPC_SYSTEM_HAS_EXPLICIT_INTERLOCKED_POINTER}
 {$endif FPC_HAS_EXPLICIT_INTERLOCKED_POINTER}
 
+{$ifdef cpu16}
+function InterlockedAdd (var Target: smallint; Increment: smallint): smallint;
+var
+  OldValue: smallint;
+begin
+  OldValue := InterlockedExchangeAdd(Target, Increment);
+  Result := OldValue + Increment;
+end;
+
+function InterlockedAdd (var Target: word; Increment: word): word;
+var
+  OldValue: word;
+begin
+  OldValue := InterlockedExchangeAdd(Target, Increment);
+  Result := OldValue + Increment;
+end;
+{$endif cpu16}
+
+function InterlockedAdd (var Target: longint; Increment: longint): longint;
+var
+  OldValue: longint;
+begin
+  OldValue := InterlockedExchangeAdd(Target, Increment);
+  Result := OldValue + Increment;
+end;
+
+function AtomicIncrement (var Target: longint; Increment: longint): longint;
+begin
+  Result := InterlockedAdd(Target, Increment);
+end;
+
+function InterlockedAdd (var Target: cardinal; Increment: cardinal): cardinal;
+var
+  OldValue: cardinal;
+begin
+  OldValue := InterlockedExchangeAdd(Target, Increment);
+  Result := OldValue + Increment;
+end;
+
+{$ifdef cpu64}
+function InterlockedAdd (var Target: int64; Increment: int64): int64;
+var
+  OldValue: int64;
+begin
+  OldValue := InterlockedExchangeAdd64(Target, Increment);
+  Result := OldValue + Increment;
+end;
+
+function AtomicIncrement (var Target: int64; Increment: int64): int64;
+begin
+  Result := InterlockedAdd(Target, Increment);
+end;
+
+function InterlockedAdd (var Target: qword; Increment: qword): qword;
+var
+  OldValue: qword;
+begin
+  OldValue := InterlockedExchangeAdd64(Target, Increment);
+  Result := OldValue + Increment;
+end;
+{$endif cpu64}
+
 procedure fpc_objecterror; compilerproc;
 begin
   HandleErrorAddrFrameInd(210,get_pc_addr,get_frame);
diff --git rtl/inc/systemh.inc rtl/inc/systemh.inc
index d455c4517c..6773afd865 100644
--- rtl/inc/systemh.inc
+++ rtl/inc/systemh.inc
@@ -1492,18 +1492,23 @@ function InterlockedDecrement (var Target: smallint) : smallint; public name 'FP
 function InterlockedExchange (var Target: smallint;Source : smallint) : smallint; public name 'FPC_INTERLOCKEDEXCHANGE16';
 function InterlockedExchangeAdd (var Target: smallint;Source : smallint) : smallint; public name 'FPC_INTERLOCKEDEXCHANGEADD16';
 function InterlockedCompareExchange(var Target: smallint; NewValue: smallint; Comperand: smallint): smallint; public name 'FPC_INTERLOCKEDCOMPAREEXCHANGE16';
+function InterlockedAdd (var Target: smallint; Increment: smallint): smallint;
 {$endif cpu16}
 function InterlockedIncrement (var Target: longint) : longint; public name 'FPC_INTERLOCKEDINCREMENT';
 function InterlockedDecrement (var Target: longint) : longint; public name 'FPC_INTERLOCKEDDECREMENT';
 function InterlockedExchange (var Target: longint;Source : longint) : longint; public name 'FPC_INTERLOCKEDEXCHANGE';
 function InterlockedExchangeAdd (var Target: longint;Source : longint) : longint; public name 'FPC_INTERLOCKEDEXCHANGEADD';
 function InterlockedCompareExchange(var Target: longint; NewValue: longint; Comperand: longint): longint; public name 'FPC_INTERLOCKEDCOMPAREEXCHANGE';
+function InterlockedAdd (var Target: longint; Increment: longint): longint;
+function AtomicIncrement (var Target: longint; Increment: longint = 1): longint;
 {$ifdef cpu64}
 function InterlockedIncrement64 (var Target: int64) : int64; public name 'FPC_INTERLOCKEDINCREMENT64';
 function InterlockedDecrement64 (var Target: int64) : int64; public name 'FPC_INTERLOCKEDDECREMENT64';
 function InterlockedExchange64 (var Target: int64;Source : int64) : int64; public name 'FPC_INTERLOCKEDEXCHANGE64';
 function InterlockedExchangeAdd64 (var Target: int64;Source : int64) : int64; public name 'FPC_INTERLOCKEDEXCHANGEADD64';
 function InterlockedCompareExchange64(var Target: int64; NewValue: int64; Comperand: int64): int64; public name 'FPC_INTERLOCKEDCOMPAREEXCHANGE64';
+function InterlockedAdd (var Target: int64; Increment: int64): int64;
+function AtomicIncrement (var Target: int64; Increment: int64 = 1): int64;
 {$endif cpu64}
 { Pointer overloads }
 {$if defined(FPC_HAS_EXPLICIT_INTERLOCKED_POINTER)}
@@ -1539,18 +1544,21 @@ function InterlockedDecrement (var Target: word) : word; external name 'FPC_INTE
 function InterlockedExchange (var Target: word;Source : word) : word; external name 'FPC_INTERLOCKEDEXCHANGE16';
 function InterlockedExchangeAdd (var Target: word;Source : word) : word; external name 'FPC_INTERLOCKEDEXCHANGEADD16';
 function InterlockedCompareExchange(var Target: word; NewValue: word; Comperand: word): word; external name 'FPC_INTERLOCKEDCOMPAREEXCHANGE16';
+function InterlockedAdd (var Target: word; Increment: word): word;
 {$endif cpu16}
 function InterlockedIncrement (var Target: cardinal) : cardinal; external name 'FPC_INTERLOCKEDINCREMENT';
 function InterlockedDecrement (var Target: cardinal) : cardinal; external name 'FPC_INTERLOCKEDDECREMENT';
 function InterlockedExchange (var Target: cardinal;Source : cardinal) : cardinal; external name 'FPC_INTERLOCKEDEXCHANGE';
 function InterlockedExchangeAdd (var Target: cardinal;Source : cardinal) : cardinal; external name 'FPC_INTERLOCKEDEXCHANGEADD';
 function InterlockedCompareExchange(var Target: cardinal; NewValue: cardinal; Comperand: cardinal): cardinal; external name 'FPC_INTERLOCKEDCOMPAREEXCHANGE';
+function InterlockedAdd (var Target: cardinal; Increment: cardinal): cardinal;
 {$ifdef cpu64}
 function InterlockedIncrement64 (var Target: qword) : qword; external name 'FPC_INTERLOCKEDINCREMENT64';
 function InterlockedDecrement64 (var Target: qword) : qword; external name 'FPC_INTERLOCKEDDECREMENT64';
 function InterlockedExchange64 (var Target: qword;Source : qword) : qword; external name 'FPC_INTERLOCKEDEXCHANGE64';
 function InterlockedExchangeAdd64 (var Target: qword;Source : qword) : qword; external name 'FPC_INTERLOCKEDEXCHANGEADD64';
 function InterlockedCompareExchange64(var Target: qword; NewValue: qword; Comperand: qword): int64; external name 'FPC_INTERLOCKEDCOMPAREEXCHANGE64';
+function InterlockedAdd (var Target: qword; Increment: qword): qword;
 {$endif cpu64}
 
 

Issue History

Date Modified Username Field Change
2020-05-25 01:36 Bi0T1N New Issue
2020-05-25 09:14 Thaddy de Koning Note Added: 0123055
2020-05-25 09:15 Thaddy de Koning Note Edited: 0123055 View Revisions
2020-05-25 11:38 Jonas Maebe Note Added: 0123057
2020-05-29 17:39 Bi0T1N Note Added: 0123129
2020-05-29 17:39 Bi0T1N Note Edited: 0123129 View Revisions
2020-05-29 21:52 Jonas Maebe Note Added: 0123133
2020-05-30 11:29 Sven Barth Note Added: 0123144
2020-05-30 21:39 Bi0T1N Note Added: 0123150
2020-05-30 22:06 Bi0T1N Note Edited: 0123150 View Revisions
2020-05-30 22:10 Bi0T1N Note Edited: 0123150 View Revisions
2020-05-30 22:13 Bi0T1N Note Edited: 0123150 View Revisions
2020-06-07 22:50 Bi0T1N Note Added: 0123320
2020-06-07 22:50 Bi0T1N File Added: 01-Add_InterlockedAdd_and_AtomicIncrement.patch