Make % operator give proper remainders even if one or both operands are negative.

Per the C standards, the % operator should give a remainder after division, such that (a/b)*b + a%b equals a (provided that a/b is representable). As such, the operation of % is defined for cases where either or both of the operands are negative. Since division truncates toward 0, a%b should give a negative result (or 0) in cases where a is negative.

Previously, the % operator was essentially behaving like the "mod" operator in Pascal, which is equivalent for positive operands but not if either operand is negative. It would generally give incorrect results in those cases, or in some cases give compile-time or run-time errors.

This patch addresses both 16-bit and 32-bit signed computations at run time, and operations in constant expressions. The approach at run time is to call existing division routines, which return the correct remainder, except always as a positive number. The generated code checks the sign of the first operand, and if it is negative negates the remainder.

The code generated is somewhat large (especially for the 32-bit case), so it might be sensible to put it in a library function and call that, but for now it's just generated in-line. This avoids introducing a dependency on a new library function, so the generated code remains compatible with older versions of ORCALib (e.g. the GNO one).

Fixes #10.
This commit is contained in:
Stephen Heumann 2018-09-10 17:26:20 -05:00
parent e8d89c9b39
commit 2d43074d5a
4 changed files with 53 additions and 20 deletions

View File

@ -449,10 +449,10 @@
{ value being loaded. }
{ }
{ }
{ pc_mod - integer modulus }
{ pc_uim - unsigned integer modulus }
{ pc_mdl - long modulus }
{ pc_ulm - unsigned long modulus }
{ pc_mod - integer remainder }
{ pc_uim - unsigned integer modulus/remainder }
{ pc_mdl - long remainder }
{ pc_ulm - unsigned long modulus/remainder }
{ }
{ Gen0(pc_mod) cgByte,cgWord }
{ Gen0(pc_uim) cgUByte,cgUWord }
@ -460,9 +460,9 @@
{ Gen0(pc_ulm) cgULong }
{ }
{ The two values on the top of the evaluation stack are }
{ removed and a molulus operation is performed. The result is }
{ placed back on the stack. The result, like the arguments, }
{ is an integer. }
{ removed and the remainder after division is calculated. }
{ The result is placed back on the stack. The result, like }
{ the arguments, is an integer. }
{ }
{ }
{ pc_mpi - integer multiply }

View File

@ -1114,15 +1114,14 @@ var
op1 := op1 div op2;
end;
percentch : begin {%}
if op2 <= 0 then {FIXME: support negative values}
if (op2 = 0) or (not unsigned) then begin
Error(109);
op2 := 1;
end; {if}
if op2 = 0 then begin
Error(109);
op2 := 1;
end; {if}
if unsigned then
op1 := umod(op1,op2)
else
op1 := op1 mod op2;
op1 := op1 - (op1 div op2) * op2;
end;
otherwise: Error(57);
end; {case}

47
Gen.pas
View File

@ -3890,6 +3890,7 @@ procedure GenTree {op: icptr};
var
nd: icptr; {for swapping left/right children}
lab1,lab2: integer; {label numbers}
procedure GenOp (ops, opi: integer);
@ -3900,9 +3901,6 @@ procedure GenTree {op: icptr};
{ ops - stack version of operation }
{ opi - immediate version of operation }
var
lab1: integer; {label number}
begin {GenOp}
if gLong.where = A_X then
GenImplied(m_phx)
@ -3932,6 +3930,8 @@ procedure GenTree {op: icptr};
op^.left := op^.right;
op^.right := nd;
end; {if}
if op^.opcode = pc_mdl then
GenImplied(m_phd); {reserve stack space}
gLong.preference := onStack;
GenTree(op^.left);
if op^.opcode in [pc_blr,pc_blx,pc_bal] then begin
@ -3961,9 +3961,32 @@ procedure GenTree {op: icptr};
pc_dvl: GenCall(43);
pc_mdl: begin
GenCall(44);
GenImplied(m_ply);
lab1 := GenLabel;
lab2 := GenLabel;
{stash high word of dividend (for sign)}
GenNative(m_lda_s, direct, 7, nil, 0);
GenNative(m_sta_s, direct, 9, nil, 0);
GenCall(78); {call ~DIV4}
GenImplied(m_ply); {ignore quotient}
GenImplied(m_ply);
GenImplied(m_pla); {get remainder (always positive or 0)}
GenImplied(m_plx);
GenImplied(m_ply); {if dividend was negative...}
GenNative(m_bpl, relative, lab1, nil, 0);
GenImplied(m_clc); { negate remainder}
GenNative(m_eor_imm, immediate, -1, nil, 0);
GenNative(m_adc_imm, immediate, 1, nil, 0);
GenImplied(m_tay);
GenImplied(m_txa);
GenNative(m_eor_imm, immediate, -1, nil, 0);
GenNative(m_adc_imm, immediate, 0, nil, 0);
GenImplied(m_pha);
GenImplied(m_phy);
GenNative(m_bra, relative, lab2, nil, 0);
GenLab(lab1);
GenImplied(m_phx);
GenImplied(m_pha);
GenLab(lab2);
end;
pc_mpl: GenCall(42);
@ -4175,6 +4198,7 @@ procedure GenTree {op: icptr};
var
opcode: pcodes; {temp storage}
lab1: integer; {label number}
begin {GenDviMod}
if Complex(op^.right) then begin
@ -4194,8 +4218,17 @@ procedure GenTree {op: icptr};
LoadX(op^.right);
end; {else}
opcode := op^.opcode;
if opcode = pc_mod then
GenCall(27)
if opcode = pc_mod then begin
lab1 := GenLabel;
GenImplied(m_pha); {stash away dividend (for sign)}
GenCall(26); {call ~DIV2}
GenImplied(m_txa); {get remainder (always positive or 0)}
GenImplied(m_ply); {if dividend was negative...}
GenNative(m_bpl, relative, lab1, nil, 0);
GenNative(m_eor_imm, immediate, -1, nil, 0); {...negate remainder}
GenImplied(m_ina);
GenLab(lab1);
end {if}
else if opcode = pc_dvi then
GenCall(26)
else {if opcode in [pc_udi,pc_uim] then} begin

View File

@ -1980,6 +1980,7 @@ case callNum of
75: sp := @'~COPYBF';
76: sp := @'~STACKERR'; {CC}
77: sp := @'~LOADSTRUCT'; {CC}
78: sp := @'~DIV4'; {CC}
otherwise:
Error(cge1);
end; {case}