Skip to content

Commit

Permalink
Fix for Mirror() PUREPASCAL implementation not producing a perfect tr…
Browse files Browse the repository at this point in the history 
…iangle "wave".

Mirror() assembler implementation used wrong (and thus uninitialized) register.
MirrorPow2 still broken.
Updated Mirror and MirrorPow2 unit tests.
Refs #291
  • Loading branch information
Anders Melander committed Mar 23, 2024
1 parent c51fcf8 commit a322add
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 61 deletions.
124 changes: 69 additions & 55 deletions Source/GR32_LowLevel.pas
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ interface
// Clamp function restricts value to [0..255] range
//
//------------------------------------------------------------------------------
function Clamp(const Value: Integer): Integer; overload; inline;
function Clamp(const Value: Integer): Integer; overload; {$IFDEF USENATIVECODE} inline; {$ENDIF}


//------------------------------------------------------------------------------
Expand Down Expand Up @@ -1181,13 +1181,14 @@ procedure WrapMem(var Value: Single; Max: Cardinal);

//------------------------------------------------------------------------------

function DivMod(Dividend, Divisor: Integer; out Remainder: Integer): Integer;
{$IFDEF USENATIVECODE}
function DivMod(Dividend, Divisor: Integer; out Remainder: Integer): Integer;
begin
Remainder := Dividend mod Divisor;
Result := Dividend div Divisor;
end;
{$ELSE}
{$IFDEF FPC} assembler; nostackframe; {$ENDIF}
function DivMod(Dividend, Divisor: Integer; out Remainder: Integer): Integer; {$IFDEF FPC} assembler; nostackframe; {$ENDIF}
asm
{$IFDEF TARGET_x86}
PUSH EBX
Expand All @@ -1207,8 +1208,8 @@ function DivMod(Dividend, Divisor: Integer; out Remainder: Integer): Integer;
MOV [RCX],EDX
POP RBX
{$ENDIF}
{$ENDIF}
end;
{$ENDIF}

//------------------------------------------------------------------------------

Expand All @@ -1224,49 +1225,76 @@ function WrapPow2(Value, Min, Max: Integer): Integer; overload;
Result := (Value - Min) and (Max - Min) + Min;
end;

//------------------------------------------------------------------------------

function GetOptimalWrap(Max: Integer): TWrapProc; overload;
begin
if (Max >= 0) and IsPowerOf2(Max + 1) then
Result := WrapPow2
else
Result := Wrap;
end;

//------------------------------------------------------------------------------

function GetOptimalWrap(Min, Max: Integer): TWrapProcEx; overload;
begin
if (Min >= 0) and (Max >= Min) and IsPowerOf2(Max - Min + 1) then
Result := WrapPow2
else
Result := Wrap;
end;


//------------------------------------------------------------------------------
//
// Mirror
//
//------------------------------------------------------------------------------
function Mirror(Value, Max: Integer): Integer;
{$IFDEF USENATIVECODE}
function Mirror(Value, Max: Integer): Integer;
var
DivResult: Integer;
Quotient: Integer;
begin
if Value < 0 then
begin
DivResult := DivMod(Value - Max, Max + 1, Result);
Inc(Result, Max);
end
else
DivResult := DivMod(Value, Max + 1, Result);
Value := -Value;
Quotient := DivMod(Value, Max, Result);

if Odd(DivResult) then
if ((Quotient and 1) = 1) then
Result := Max - Result;
end;
{$ELSE}
{$IFDEF FPC} assembler; nostackframe; {$ENDIF}
function Mirror(Value, Max: Integer): Integer; {$IFDEF FPC} assembler; nostackframe; {$ENDIF}
asm
{$IFDEF TARGET_x64}
MOV EAX,ECX
MOV ECX,R8D
MOV EAX, ECX // Value
{$ENDIF}
TEST EAX,EAX
JNL @@1
// EAX: Value
// EDX: Max

// Value := Abs(Value)
TEST EAX, EAX
JNL @@Positive
NEG EAX
@@1:
MOV ECX,EDX

@@Positive:
MOV ECX, EDX
// Value := Int64(Value)
CDQ
// Quotient := DivMod(Value, Max, Remainder)
IDIV ECX
TEST EAX,1
MOV EAX,EDX
// If not Odd(Quotient) then
TEST EAX, 1
// Result := Remainder
MOV EAX, EDX
JZ @Exit
// else
// Result := Max - Remainder
NEG EAX
ADD EAX,ECX
ADD EAX, ECX
@Exit:
{$ENDIF}
end;
{$ENDIF}

//------------------------------------------------------------------------------

Expand All @@ -1284,56 +1312,39 @@ function Mirror(Value, Min, Max: Integer): Integer;
DivResult := DivMod(Value - Min, Max - Min + 1, Result);
Inc(Result, Min);
end;
if Odd(DivResult) then Result := Max + Min - Result;
if Odd(DivResult) then
Result := Max + Min - Result;
end;

//------------------------------------------------------------------------------

function MirrorPow2(Value, Max: Integer): Integer; overload;
begin
if Value and (Max + 1) = 0 then
Result := Value and Max
if Value < 0 then
Value := -Value;

if ((Value and (Max + 1)) = 0) then
Result := (Value ) and Max
else
Result := Max - Value and Max;
Result := (Max - Value + Max) and Max;
end;

//------------------------------------------------------------------------------

function MirrorPow2(Value, Min, Max: Integer): Integer; overload;
begin
Value := Value - Min;
if Value < 0 then
Value := -Value;
Result := Max - Min;

if Value and (Result + 1) = 0 then
Result := Min + Value and Result
if ((Value and (Result + 1)) = 0) then
Result := (Min + Value) and Result
else
Result := Max - Value and Result;
Result := (Max - Value) and Result;
end;


//------------------------------------------------------------------------------
//
// Clamp/Wrap/Mirror
//
//------------------------------------------------------------------------------
function GetOptimalWrap(Max: Integer): TWrapProc; overload;
begin
if (Max >= 0) and IsPowerOf2(Max + 1) then
Result := WrapPow2
else
Result := Wrap;
end;

//------------------------------------------------------------------------------

function GetOptimalWrap(Min, Max: Integer): TWrapProcEx; overload;
begin
if (Min >= 0) and (Max >= Min) and IsPowerOf2(Max - Min + 1) then
Result := WrapPow2
else
Result := Wrap;
end;

//------------------------------------------------------------------------------

function GetOptimalMirror(Max: Integer): TWrapProc; overload;
Expand All @@ -1355,7 +1366,10 @@ function GetOptimalMirror(Min, Max: Integer): TWrapProcEx; overload;
end;

//------------------------------------------------------------------------------

//
// Clamp/Wrap/Mirror
//
//------------------------------------------------------------------------------
function GetWrapProc(WrapMode: TWrapMode): TWrapProc; overload;
begin
case WrapMode of
Expand Down
59 changes: 53 additions & 6 deletions Tests/LowLevel Test/TestGR32_LowLevel.pas
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ TTestLowLevel = class abstract(TBindingTestCase)
procedure TestWrapMinMax;
procedure TestWrapPow2;
procedure TestMirror;
// TODO : procedure TestMirrorMinMax;
procedure TestMirrorPow2;
procedure TestSAR;
end;

Expand Down Expand Up @@ -611,12 +613,57 @@ procedure TTestLowLevel.TestMinMax;

procedure TTestLowLevel.TestMirror;
begin
CheckEquals(50, Mirror(50, 100));
CheckEquals(51, Mirror(150, 100));
CheckEquals(50, Mirror(149, 99));
CheckEquals(64, MirrorPow2(64, 127));
CheckEquals(63, MirrorPow2(192, 127));
CheckEquals(95, MirrorPow2(160, 127));
// Edge cases
CheckEquals( 0, Mirror( 0, 100));
CheckEquals(100, Mirror(100, 100));
CheckEquals( 1, Mirror( 1, 100));
CheckEquals( 99, Mirror( 99, 100));
CheckEquals( 99, Mirror(101, 100));
CheckEquals( 1, Mirror( -1, 100));

CheckEquals( 50, Mirror(150, 100));
CheckEquals( 0, Mirror(200, 100));
CheckEquals( 50, Mirror(250, 100));
CheckEquals(100, Mirror(300, 100));

CheckEquals( 49, Mirror(149, 99));
end;

procedure TTestLowLevel.TestMirrorPow2;
var
Bit: integer;
Max: integer;
Value: integer;
Expected: integer;
Actual: integer;
begin
for Bit := 1 to 15 do
begin
Max := (1 shl Bit)-1;

// Edge cases
CheckEquals( 0, MirrorPow2( 0, Max));
CheckEquals(Max, MirrorPow2(Max, Max));
CheckEquals( 1, MirrorPow2( 1, Max));
CheckEquals(Max-1, MirrorPow2(Max-1, Max));
if (Max > 1) then
CheckEquals(Max-1, MirrorPow2(Max+1, Max));
CheckEquals( 1, MirrorPow2( -1, Max));

// Note: We're using Mirror to validate MirrorPow2 so we're assuming that Mirror isn't broken...
for Value := 0 to 3*Max do
begin
// Positive values
Expected := Mirror(Value, Max);
Actual := MirrorPow2(Value, Max);
CheckEquals(Expected, Actual, Format('MirrorPow2(%d, %d)', [Value, Max]));

// Negative values
Expected := Mirror(-Value, Max);
Actual := MirrorPow2(-Value, Max);
CheckEquals(Expected, Actual, Format('MirrorPow2(%d, %d)', [Value, Max]));
end;
end;
end;

procedure TTestLowLevel.TestMoveLongword;
Expand Down

0 comments on commit a322add

Please sign in to comment.