Skip to content

Commit

Permalink
Fix incorrect range calculation for operator rem
Browse files Browse the repository at this point in the history
acf3116 enhanced the range calculation for `rem`, but at the
same time introduced a bug. For example:

    2 rem X

If the range for `X` was unknown, the compiler would calculate the
range for the remainder as `{2,2}`, while the correct range is
`{0,2}`. This commit corrects the range calculation for the remainder
to ensure that zero is always included in the range.
  • Loading branch information
bjorng committed Aug 31, 2023
1 parent 334589a commit b0926ff
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 4 deletions.
38 changes: 38 additions & 0 deletions erts/emulator/test/small_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -860,6 +860,22 @@ gen_div_function({Name,{A,B}}) ->
put(prevent_div_rem_fusion, Q),
R = X rem Y,
{Q, R};
'@Name@'(any0, fixed, Y) ->
X = _@A@,
Q = X div Y,
R = X rem Y,
{Q, R};
'@Name@'(any1, fixed, Y) ->
X = _@A@,
R = X rem Y,
Q = X div Y,
{Q, R};
'@Name@'(any2, fixed, Y) ->
X = _@A@,
Q = X div Y,
put(prevent_div_rem_fusion, Q),
R = X rem Y,
{Q, R};
'@Name@'(X0, Y0, integer0) ->
Q = X0 div Y0,
R = X0 rem Y0,
Expand Down Expand Up @@ -945,6 +961,22 @@ gen_div_function({Name,{A,B}}) ->
Q = X div Y,
put(prevent_div_rem_fusion, Q),
R = X rem Y,
{Q, R};
'@Name@'(fixed, Y, any0) ->
X = _@A@,
Q = X div Y,
R = X rem Y,
{Q, R};
'@Name@'(fixed, Y, any1) ->
X = _@A@,
R = X rem Y,
Q = X div Y,
{Q, R};
'@Name@'(fixed, Y, any2) ->
X = _@A@,
Q = X div Y,
put(prevent_div_rem_fusion, Q),
R = X rem Y,
{Q, R}. ").


Expand All @@ -969,6 +1001,9 @@ test_division([{Name,{A,B}}|T], Mod) ->
PosRes = F(any0, A, fixed),
PosRes = F(any1, A, fixed),
PosRes = F(any2, A, fixed),
PosRes = F(any0, fixed, B),
PosRes = F(any1, fixed, B),
PosRes = F(any2, fixed, B),

PosRes = F(A, B, integer0),
PosRes = F(A, fixed, integer1),
Expand All @@ -985,6 +1020,9 @@ test_division([{Name,{A,B}}|T], Mod) ->
PosRes = F(A, fixed, any0),
PosRes = F(A, fixed, any1),
PosRes = F(A, fixed, any2),
PosRes = F(fixed, B, any0),
PosRes = F(fixed, B, any1),
PosRes = F(fixed, B, any2),

NegRes = F(integer0, -A, B),
NegRes = F(integer1, -A, fixed),
Expand Down
7 changes: 5 additions & 2 deletions lib/compiler/src/beam_bounds.erl
Original file line number Diff line number Diff line change
Expand Up @@ -327,8 +327,11 @@ rem_bounds({A,B}, _) ->
%% The sign of the remainder is the same as the sign of the
%% left-hand side operand; it does not depend on the sign of the
%% right-hand side operand. Therefore, the range of the remainder
%% is the same as the range of the left-hand side operand.
{A,B};
%% is the range of the left-hand side operand extended to always
%% include zero.
Min = inf_min(0, A),
Max = inf_max(0, B),
normalize({Min,Max});
rem_bounds(_, _) ->
any.

Expand Down
18 changes: 16 additions & 2 deletions lib/compiler/test/beam_bounds_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ division_bounds(_Config) ->
{-50,50} = beam_bounds:bounds('div', {-50,-15}, {-10,'+inf'}),
{-20,20} = beam_bounds:bounds('div', {-20,10}, any),
{-7,7} = beam_bounds:bounds('div', {-5,7}, {'-inf',-1}),
{-42,42} = beam_bounds:bounds('div', {42,42}, any),
{-42,42} = beam_bounds:bounds('div', {-42,-42}, any),

any = beam_bounds:bounds('div', {'-inf',10}, any),
any = beam_bounds:bounds('div', {0,'+inf'}, any),

Expand All @@ -153,15 +156,26 @@ rem_bounds(_Config) ->

{-7,7} = beam_bounds:bounds('rem', {'-inf',10}, {1,8}),
{0,7} = beam_bounds:bounds('rem', {10,'+inf'}, {1,8}),
{0,'+inf'} = beam_bounds:bounds('rem', {17,'+inf'}, any),

{1,10} = beam_bounds:bounds('rem', {1,10}, {'-inf',10}),
{20,'+inf'} = beam_bounds:bounds('rem', {20,'+inf'}, {10,'+inf'}),
{0,10} = beam_bounds:bounds('rem', {1,10}, {'-inf',10}),
{0,'+inf'} = beam_bounds:bounds('rem', {20,'+inf'}, {10,'+inf'}),
{'-inf',10} = beam_bounds:bounds('rem', {'-inf',10}, any),

{-11,10} = beam_bounds:bounds('rem', {-11,10}, {'-inf',89}),
{-11,10} = beam_bounds:bounds('rem', {-11,10}, {7,'+inf'}),
{-11,10} = beam_bounds:bounds('rem', {-11,10}, {'-inf',113}),
{-11,10} = beam_bounds:bounds('rem', {-11,10}, {55,'+inf'}),
{-11,10} = beam_bounds:bounds('rem', {-11,10}, any),

{0,0} = beam_bounds:bounds('rem', {0,0}, any),
{0,1} = beam_bounds:bounds('rem', {1,1}, any),
{0,2} = beam_bounds:bounds('rem', {2,2}, any),
{0,3} = beam_bounds:bounds('rem', {2,3}, any),

{-1,0} = beam_bounds:bounds('rem', {-1,-1}, any),
{-7,0} = beam_bounds:bounds('rem', {-7,-7}, any),
{-6,0} = beam_bounds:bounds('rem', {-6,-4}, any),

ok.

Expand Down

0 comments on commit b0926ff

Please sign in to comment.