Skip to content

Commit

Permalink
x86: Force result of Icomp to be in a register (ocaml#11808)
Browse files Browse the repository at this point in the history
Also: export `makeregs` from the Reloadgen interface.

Fixes: ocaml#11803
  • Loading branch information
lthls committed Dec 14, 2022
1 parent 1018602 commit aca252f
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 4 deletions.
6 changes: 6 additions & 0 deletions Changes
Expand Up @@ -3,6 +3,12 @@ OCaml 4.14 maintenance branch

### Bug fixes:

- #11803, #11808: on x86, the destination of an integer comparison must be
a register, it cannot be a stack slot.
(Vincent Laviron, review by Xavier Leroy, report by
Emilio Jesús Gallego Arias)


OCaml 4.14.1
-----------------------------

Expand Down
15 changes: 13 additions & 2 deletions asmcomp/amd64/reload.ml
Expand Up @@ -33,14 +33,15 @@ open Mach
Iload R R R
Istore R R
Iintop(Icomp) R R S
or S S R
or R S R
Iintop(Imul|Idiv|Imod) R R S
Iintop(Imulh) R R S
Iintop(shift) S S R
Iintop(others) R R S
or S S R
Iintop_imm(Iadd, n)/lea R R
Iintop_imm(Imul, n) R R
Iintop_imm(Icomp, n) R S
Iintop_imm(others) S S
Inegf...Idivf R R S
Ifloatofint R S
Expand All @@ -66,7 +67,14 @@ inherit Reloadgen.reload_generic as super

method! reload_operation op arg res =
match op with
| Iintop(Iadd|Isub|Iand|Ior|Ixor|Icomp _|Icheckbound) ->
| Iintop(Iadd|Isub|Iand|Ior|Ixor|Icheckbound) ->
(* One of the two arguments can reside in the stack, but not both *)
if stackp arg.(0) && stackp arg.(1)
then ([|arg.(0); self#makereg arg.(1)|], res)
else (arg, res)
| Iintop(Icomp _) ->
(* The result must be a register (PR#11803) *)
let res = self#makeregs res in
(* One of the two arguments can reside in the stack, but not both *)
if stackp arg.(0) && stackp arg.(1)
then ([|arg.(0); self#makereg arg.(1)|], res)
Expand All @@ -80,6 +88,9 @@ method! reload_operation op arg res =
if stackp arg.(0)
then (let r = self#makereg arg.(0) in ([|r|], [|r|]))
else (arg, res)
| Iintop_imm(Icomp _, _) ->
(* The result must be in a register (PR#11803) *)
(arg, self#makeregs res)
| Iintop(Imulh | Idiv | Imod | Ilsl | Ilsr | Iasr)
| Iintop_imm(_, _) ->
(* The argument(s) and results can be either in register or on stack *)
Expand Down
12 changes: 11 additions & 1 deletion asmcomp/i386/reload.ml
Expand Up @@ -40,7 +40,14 @@ method! makereg r =

method! reload_operation op arg res =
match op with
Iintop(Iadd|Isub|Iand|Ior|Ixor|Icomp _|Icheckbound) ->
Iintop(Iadd|Isub|Iand|Ior|Ixor|Icheckbound) ->
(* One of the two arguments can reside in the stack *)
if stackp arg.(0) && stackp arg.(1)
then ([|arg.(0); self#makereg arg.(1)|], res)
else (arg, res)
| Iintop(Icomp _) ->
(* The result must be a register (PR#11803) *)
let res = self#makeregs res in
(* One of the two arguments can reside in the stack *)
if stackp arg.(0) && stackp arg.(1)
then ([|arg.(0); self#makereg arg.(1)|], res)
Expand All @@ -60,6 +67,9 @@ method! reload_operation op arg res =
if stackp arg.(0)
then let r = self#makereg arg.(0) in ([|r|], [|r|])
else (arg, res)
| Iintop_imm(Icomp _, _) ->
(* The result must be in a register (PR#11803) *)
(arg, self#makeregs res)
| Iintop(Imulh | Ilsl | Ilsr | Iasr) | Iintop_imm(_, _)
| Ifloatofint | Iintoffloat | Ispecific(Ipush) ->
(* The argument(s) can be either in register or on stack *)
Expand Down
2 changes: 1 addition & 1 deletion asmcomp/reloadgen.ml
Expand Up @@ -46,7 +46,7 @@ method makereg r =
newr.spill_cost <- 100000;
newr

method private makeregs rv =
method makeregs rv =
let n = Array.length rv in
let newv = Array.make n Reg.dummy in
for i = 0 to n-1 do newv.(i) <- self#makereg rv.(i) done;
Expand Down
1 change: 1 addition & 0 deletions asmcomp/reloadgen.mli
Expand Up @@ -20,6 +20,7 @@ class reload_generic : object
(* Can be overridden to reflect instructions that can operate
directly on stack locations *)
method makereg : Reg.t -> Reg.t
method makeregs : Reg.t array -> Reg.t array
(* Can be overridden to avoid creating new registers of some class
(i.e. if all "registers" of that class are actually on stack) *)
method fundecl : Mach.fundecl -> int array -> Mach.fundecl * bool
Expand Down

0 comments on commit aca252f

Please sign in to comment.