Skip to content

Commit a441efe

Browse files
bettiobettio
committed
Merge pull request #1895 from pguyot/w42/fix-parameter-passing-bug-x86-64-exhaustion
JIT: fix parameter passing bug when registers were exhausted on x86-64 These changes are made under both the "Apache 2.0" and the "GNU Lesser General Public License 2.1 or later" license terms (dual license). SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later
2 parents 5a33246 + cbff632 commit a441efe

File tree

4 files changed

+135
-7
lines changed

4 files changed

+135
-7
lines changed

‎libs/jit/src/jit_x86_64.erl‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1028,6 +1028,18 @@ replace_reg0([{free, Reg} | T], Reg, Replacement, Acc) ->
10281028
replace_reg0([Other | T], Reg, Replacement, Acc) ->
10291029
replace_reg0(T, Reg, Replacement, [Other | Acc]).
10301030

1031+
% Exchange registers in both Args and ArgsRegs lists
1032+
exchange_reg(Args, ArgsRegs, Reg1, Reg2) ->
1033+
NewArgs = replace_reg(Args, Reg1, Reg2),
1034+
NewArgsRegs = lists:map(
1035+
fun
1036+
(R) when R =:= Reg1 -> Reg2;
1037+
(R) -> R
1038+
end,
1039+
ArgsRegs
1040+
),
1041+
{NewArgs, NewArgsRegs}.
1042+
10311043
set_args0([], [], [], _AvailGP, Acc) ->
10321044
list_to_binary(lists:reverse(Acc));
10331045
set_args0([{free, FreeVal} | ArgsT], ArgsRegs, ParamRegs, AvailGP, Acc) ->
@@ -1056,19 +1068,23 @@ set_args0([Arg | ArgsT], [_ArgReg | ArgsRegs], [?CTX_REG | ParamRegs], AvailGP,
10561068
set_args0(ArgsT, ArgsRegs, ParamRegs, AvailGP, [J | Acc]);
10571069
set_args0(
10581070
[Arg | ArgsT],
1059-
[_ArgReg | ArgsRegs],
1071+
[ArgReg | ArgsRegs],
10601072
[ParamReg | ParamRegs],
1061-
[Avail | AvailGPT] = AvailGP,
1073+
AvailGP,
10621074
Acc
10631075
) ->
1064-
J = set_args1(Arg, ParamReg),
10651076
case lists:member(ParamReg, ArgsRegs) of
10661077
false ->
1078+
% Normal case: ParamReg is free, just move Arg to ParamReg
1079+
J = set_args1(Arg, ParamReg),
10671080
set_args0(ArgsT, ArgsRegs, ParamRegs, AvailGP, [J | Acc]);
10681081
true ->
1069-
I = jit_x86_64_asm:movq(ParamReg, Avail),
1070-
NewArgsT = replace_reg(ArgsT, ParamReg, Avail),
1071-
set_args0(NewArgsT, ArgsRegs, ParamRegs, AvailGPT, [J, I | Acc])
1082+
% ParamReg is occupied by another argument that will go elsewhere
1083+
% Use xchg to swap ArgReg and ParamReg
1084+
% After xchg, the value from Arg (which was in ArgReg) is now in ParamReg
1085+
I = jit_x86_64_asm:xchgq(ArgReg, ParamReg),
1086+
{NewArgsT, NewArgsRegs} = exchange_reg(ArgsT, ArgsRegs, ParamReg, ArgReg),
1087+
set_args0(NewArgsT, NewArgsRegs, ParamRegs, AvailGP, [I | Acc])
10721088
end.
10731089

10741090
set_args1(Reg, Reg) ->

‎libs/jit/src/jit_x86_64_asm.erl‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@
5656
popq/1,
5757
jmpq/1,
5858
retq/0,
59-
cmpb/2
59+
cmpb/2,
60+
xchgq/2
6061
]).
6162

6263
-define(IS_SINT8_T(X), is_integer(X) andalso X >= -128 andalso X =< 127).
@@ -569,3 +570,25 @@ jmpq({Reg}) ->
569570

570571
retq() ->
571572
<<16#C3>>.
573+
574+
%% XCHG r64, r64: Exchange two 64-bit registers
575+
%% Encoding: REX.W + 87 /r
576+
xchgq(rax, rax) ->
577+
% NOP
578+
<<16#90>>;
579+
xchgq(rax, Reg) when is_atom(Reg) ->
580+
% Special short encoding for XCHG rax, r64
581+
% For low registers: REX.W + 0x90 + reg
582+
% For high registers: REX.W + REX.B + 0x90 + reg (need REX.B to access r8-r11)
583+
case x86_64_x_reg(Reg) of
584+
{0, Index} -> <<16#48, (16#90 + Index)>>;
585+
{1, Index} -> <<16#49, (16#90 + Index)>>
586+
end;
587+
xchgq(Reg, rax) when is_atom(Reg) ->
588+
% XCHG is commutative
589+
xchgq(rax, Reg);
590+
xchgq(RegA, RegB) when is_atom(RegA), is_atom(RegB) ->
591+
% General form: REX.W + 87 /r
592+
{REX_R, MODRM_REG} = x86_64_x_reg(RegA),
593+
{REX_B, MODRM_RM} = x86_64_x_reg(RegB),
594+
<<?X86_64_REX(1, REX_R, 0, REX_B), 16#87, 3:2, MODRM_REG:3, MODRM_RM:3>>.

‎tests/libs/jit/jit_x86_64_asm_tests.erl‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -957,3 +957,38 @@ retq_test_() ->
957957
[
958958
?_assertAsmEqual(<<16#c3>>, "retq", jit_x86_64_asm:retq())
959959
].
960+
961+
xchgq_test_() ->
962+
[
963+
?_assertAsmEqual(<<16#90>>, "xchg %rax,%rax", jit_x86_64_asm:xchgq(rax, rax)),
964+
?_assertAsmEqual(<<16#48, 16#91>>, "xchg %rax,%rcx", jit_x86_64_asm:xchgq(rax, rcx)),
965+
?_assertAsmEqual(<<16#48, 16#92>>, "xchg %rax,%rdx", jit_x86_64_asm:xchgq(rax, rdx)),
966+
?_assertAsmEqual(<<16#48, 16#96>>, "xchg %rax,%rsi", jit_x86_64_asm:xchgq(rax, rsi)),
967+
?_assertAsmEqual(<<16#48, 16#97>>, "xchg %rax,%rdi", jit_x86_64_asm:xchgq(rax, rdi)),
968+
?_assertAsmEqual(<<16#49, 16#90>>, "xchg %rax,%r8", jit_x86_64_asm:xchgq(rax, r8)),
969+
?_assertAsmEqual(<<16#49, 16#91>>, "xchg %rax,%r9", jit_x86_64_asm:xchgq(rax, r9)),
970+
?_assertAsmEqual(<<16#49, 16#92>>, "xchg %rax,%r10", jit_x86_64_asm:xchgq(rax, r10)),
971+
?_assertAsmEqual(<<16#49, 16#93>>, "xchg %rax,%r11", jit_x86_64_asm:xchgq(rax, r11)),
972+
973+
% xchg reg, rax - commutative, should use same short encoding
974+
?_assertAsmEqual(<<16#48, 16#91>>, "xchg %rcx,%rax", jit_x86_64_asm:xchgq(rcx, rax)),
975+
?_assertAsmEqual(<<16#48, 16#92>>, "xchg %rdx,%rax", jit_x86_64_asm:xchgq(rdx, rax)),
976+
?_assertAsmEqual(<<16#49, 16#91>>, "xchg %r9,%rax", jit_x86_64_asm:xchgq(r9, rax)),
977+
978+
% xchg reg, reg - general form (REX.W + 0x87 /r)
979+
?_assertAsmEqual(
980+
<<16#48, 16#87, 16#D1>>, "xchg %rdx,%rcx", jit_x86_64_asm:xchgq(rdx, rcx)
981+
),
982+
?_assertAsmEqual(
983+
<<16#48, 16#87, 16#F2>>, "xchg %rsi,%rdx", jit_x86_64_asm:xchgq(rsi, rdx)
984+
),
985+
?_assertAsmEqual(
986+
<<16#4C, 16#87, 16#C1>>, "xchg %r8,%rcx", jit_x86_64_asm:xchgq(r8, rcx)
987+
),
988+
?_assertAsmEqual(
989+
<<16#4D, 16#87, 16#C8>>, "xchg %r9,%r8", jit_x86_64_asm:xchgq(r9, r8)
990+
),
991+
?_assertAsmEqual(
992+
<<16#4D, 16#87, 16#D1>>, "xchg %r10,%r9", jit_x86_64_asm:xchgq(r10, r9)
993+
)
994+
].

‎tests/libs/jit/jit_x86_64_tests.erl‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,60 @@ call_primitive_extended_regs_test() ->
163163
>>,
164164
?assertEqual(dump_to_bin(Dump), Stream).
165165

166+
call_primitive_few_regs_test() ->
167+
State0 = ?BACKEND:new(?JIT_VARIANT_PIC, jit_stream_binary, jit_stream_binary:new(0)),
168+
{State1, rax} = ?BACKEND:move_to_native_register(State0, {x_reg, 0}),
169+
{State2, r11} = ?BACKEND:move_to_native_register(State1, {x_reg, 1}),
170+
{State3, r10} = ?BACKEND:move_to_native_register(State2, {x_reg, 2}),
171+
{State4, r9} = ?BACKEND:move_to_native_register(State3, {x_reg, 3}),
172+
{State5, r8} = ?BACKEND:move_to_native_register(State4, {x_reg, 4}),
173+
{State6, rcx} = ?BACKEND:move_to_native_register(State5, {x_reg, 5}),
174+
175+
CreatedBin = rax,
176+
Offset = r11,
177+
SrcReg = r8,
178+
SizeValue = r9,
179+
FlagsValue = rcx,
180+
181+
{State7, r8} = ?BACKEND:call_primitive(State6, ?PRIM_BITSTRING_INSERT_INTEGER, [
182+
CreatedBin, Offset, {free, SrcReg}, SizeValue, {free, FlagsValue}
183+
]),
184+
Stream = ?BACKEND:stream(State7),
185+
Dump =
186+
<<
187+
" 0: 48 8b 47 30 mov 0x30(%rdi),%rax\n"
188+
" 4: 4c 8b 5f 38 mov 0x38(%rdi),%r11\n"
189+
" 8: 4c 8b 57 40 mov 0x40(%rdi),%r10\n"
190+
" c: 4c 8b 4f 48 mov 0x48(%rdi),%r9\n"
191+
" 10: 4c 8b 47 50 mov 0x50(%rdi),%r8\n"
192+
" 14: 48 8b 4f 58 mov 0x58(%rdi),%rcx\n"
193+
" 18: 57 push %rdi\n"
194+
" 19: 56 push %rsi\n"
195+
" 1a: 52 push %rdx\n"
196+
" 1b: 41 51 push %r9\n"
197+
" 1d: 41 52 push %r10\n"
198+
" 1f: 41 53 push %r11\n"
199+
" 21: 50 push %rax\n"
200+
" 22: 48 8b 92 c8 01 00 00 mov 0x1c8(%rdx),%rdx\n"
201+
" 29: 52 push %rdx\n"
202+
" 2a: 48 89 c7 mov %rax,%rdi\n"
203+
" 2d: 4c 89 de mov %r11,%rsi\n"
204+
" 30: 4c 89 c2 mov %r8,%rdx\n"
205+
" 33: 4c 87 c9 xchg %r9,%rcx\n"
206+
" 36: 4d 89 c8 mov %r9,%r8\n"
207+
" 39: 58 pop %rax\n"
208+
" 3a: ff d0 callq *%rax\n"
209+
" 3c: 49 89 c0 mov %rax,%r8\n"
210+
" 3f: 58 pop %rax\n"
211+
" 40: 41 5b pop %r11\n"
212+
" 42: 41 5a pop %r10\n"
213+
" 44: 41 59 pop %r9\n"
214+
" 46: 5a pop %rdx\n"
215+
" 47: 5e pop %rsi\n"
216+
" 48: 5f pop %rdi"
217+
>>,
218+
?assertEqual(dump_to_bin(Dump), Stream).
219+
166220
call_ext_only_test() ->
167221
State0 = ?BACKEND:new(?JIT_VARIANT_PIC, jit_stream_binary, jit_stream_binary:new(0)),
168222
State1 = ?BACKEND:decrement_reductions_and_maybe_schedule_next(State0),

0 commit comments

Comments
 (0)