From aca252feb944973068b6a25a2e51b0426eb57637 Mon Sep 17 00:00:00 2001 From: Vincent Laviron Date: Wed, 14 Dec 2022 16:20:31 +0100 Subject: [PATCH] x86: Force result of Icomp to be in a register (#11808) Also: export `makeregs` from the Reloadgen interface. Fixes: #11803 --- Changes | 6 ++++++ asmcomp/amd64/reload.ml | 15 +++++++++++++-- asmcomp/i386/reload.ml | 12 +++++++++++- asmcomp/reloadgen.ml | 2 +- asmcomp/reloadgen.mli | 1 + 5 files changed, 32 insertions(+), 4 deletions(-) diff --git a/Changes b/Changes index 3adce809b57a..7f9d3ffd934d 100644 --- a/Changes +++ b/Changes @@ -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 ----------------------------- diff --git a/asmcomp/amd64/reload.ml b/asmcomp/amd64/reload.ml index 1f4cadc391b5..9e3cadbe4402 100644 --- a/asmcomp/amd64/reload.ml +++ b/asmcomp/amd64/reload.ml @@ -33,7 +33,7 @@ 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 @@ -41,6 +41,7 @@ open Mach 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 @@ -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) @@ -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 *) diff --git a/asmcomp/i386/reload.ml b/asmcomp/i386/reload.ml index 09497e050751..6a20e887bacd 100644 --- a/asmcomp/i386/reload.ml +++ b/asmcomp/i386/reload.ml @@ -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) @@ -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 *) diff --git a/asmcomp/reloadgen.ml b/asmcomp/reloadgen.ml index d9c707164b2a..c000dd4aeaad 100644 --- a/asmcomp/reloadgen.ml +++ b/asmcomp/reloadgen.ml @@ -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; diff --git a/asmcomp/reloadgen.mli b/asmcomp/reloadgen.mli index 638082f0a718..0cf264c43416 100644 --- a/asmcomp/reloadgen.mli +++ b/asmcomp/reloadgen.mli @@ -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