0005319: %r11 clobbered by Lswitch in Windows AMD64 native-code compilation

(0006055)
frisch (developer)
2011-07-22 16:40

Suggested fix:

--- ocaml/asmcomp/amd64/emit_nt.mlp (revision 36936)
+++ ocaml/asmcomp/amd64/emit_nt.mlp (working copy)
@@ -600,8 +600,17 @@
     | Lswitch jumptbl ->
         let lbl = new_label() in
         if !pic_code then begin
+ if i.arg.(0).loc = Reg 9 then begin
+ ` sal r11, 3\n`;
+ ` push r11\n`;
+ ` lea r11, {emit_label lbl}\n`;
+ ` add r11, QWORD PTR [rsp]\n`;
+ ` add rsp, 8\n`;
+ ` jmp QWORD PTR [r11]\n`;
+ end else begin
           ` lea r11, {emit_label lbl}\n`;
           ` jmp QWORD PTR [r11+{emit_reg i.arg.(0)}*8]\n`
+ end
         end else begin
           ` jmp QWORD PTR [{emit_reg i.arg.(0)}*8 + {emit_label lbl}]\n`
         end;

(0006056)
xleroy (administrator)
2011-07-23 10:14

Well spotted. I would prefer to use the same fix as in emit.mlp/proc.ml (namely, two temp regs that are destroyed across Lswitch) than to use your suggested fix, for two reasons: 1- trying to keep the two emitters in sync, and 2- avoiding pushes and pops if at all possible. I can do the backporting of the fix; can you do the testing?

(0006057)
frisch (developer)
2011-07-23 12:05

Of course we can do some testing! But if one changes the register selection, the bug would probably not appear at the same place anyway, so it will be difficult to draw any conclusion. (We have a large code base compiled and running with Win64, and the bug only appeared once after several months.)

(0006128)
xleroy (administrator)
2011-09-18 09:58

Tentative fix committed in 3.12 branch and in trunk. Still not tested yet.

Issue History
Date Modified	Username	Field	Change
2011-07-22 16:09	frisch	New Issue
2011-07-22 16:15	frisch	Relationship added	related to 0004424
2011-07-22 16:15	frisch	Description Updated
2011-07-22 16:40	frisch	Note Added: 0006055
2011-07-23 10:14	xleroy	Note Added: 0006056
2011-07-23 10:14	xleroy	Assigned To	=> xleroy
2011-07-23 10:14	xleroy	Status	new => confirmed
2011-07-23 12:05	frisch	Note Added: 0006057
2011-09-18 09:58	xleroy	Note Added: 0006128
2011-09-18 09:58	xleroy	Status	confirmed => resolved
2011-09-18 09:58	xleroy	Resolution	open => fixed
2011-09-18 09:58	xleroy	Fixed in Version	=> 3.12.1+dev
2012-09-25 20:07	xleroy	Status	resolved => closed

Notes
(0006055) frisch (developer) 2011-07-22 16:40	Suggested fix: --- ocaml/asmcomp/amd64/emit_nt.mlp (revision 36936) +++ ocaml/asmcomp/amd64/emit_nt.mlp (working copy) @@ -600,8 +600,17 @@ \| Lswitch jumptbl -> let lbl = new_label() in if !pic_code then begin + if i.arg.(0).loc = Reg 9 then begin + ` sal r11, 3\n`; + ` push r11\n`; + ` lea r11, {emit_label lbl}\n`; + ` add r11, QWORD PTR [rsp]\n`; + ` add rsp, 8\n`; + ` jmp QWORD PTR [r11]\n`; + end else begin ` lea r11, {emit_label lbl}\n`; ` jmp QWORD PTR [r11+{emit_reg i.arg.(0)}8]\n` + end end else begin ` jmp QWORD PTR [{emit_reg i.arg.(0)}8 + {emit_label lbl}]\n` end;

(0006056) xleroy (administrator) 2011-07-23 10:14	Well spotted. I would prefer to use the same fix as in emit.mlp/proc.ml (namely, two temp regs that are destroyed across Lswitch) than to use your suggested fix, for two reasons: 1- trying to keep the two emitters in sync, and 2- avoiding pushes and pops if at all possible. I can do the backporting of the fix; can you do the testing?

(0006057) frisch (developer) 2011-07-23 12:05	Of course we can do some testing! But if one changes the register selection, the bug would probably not appear at the same place anyway, so it will be difficult to draw any conclusion. (We have a large code base compiled and running with Win64, and the bug only appeared once after several months.)

(0006128) xleroy (administrator) 2011-09-18 09:58	Tentative fix committed in 3.12 branch and in trunk. Still not tested yet.