Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Debug print in yklua can crash trace optimiser. #1598

Open
vext01 opened this issue Feb 10, 2025 · 5 comments
Open

Debug print in yklua can crash trace optimiser. #1598

vext01 opened this issue Feb 10, 2025 · 5 comments
Assignees

Comments

@vext01
Copy link
Contributor

vext01 commented Feb 10, 2025

This is similar, but distinct from #1597.

Using yk pulled just now (806507c), using the test case from #1594 (comment) again, and with this slightly different diff to yklua:

diff --git a/src/lvm.c b/src/lvm.c
index ea28b66..151ee1c 100644
--- a/src/lvm.c
+++ b/src/lvm.c
@@ -27,13 +27,13 @@
 #include "lgc.h"
 #include "lobject.h"
 #include "lopcodes.h"
+#include "lopnames.h"
 #include "lstate.h"
 #include "lstring.h"
 #include "ltable.h"
 #include "ltm.h"
 #include "lvm.h"

-
 /*
 ** By default, use jump tables in the main interpreter loop on gcc
 ** and compatible compilers.
@@ -1225,6 +1225,7 @@ void luaV_execute (lua_State *L, CallInfo *ci) {
     if (GET_OPCODE(i) == OP_FORLOOP || GET_OPCODE(i) == OP_TFORLOOP)
       ykloc = &cl->p->yklocs[pcRel(pc, cl->p)];
     yk_mt_control_point(G(L)->yk_mt, ykloc);
+    fprintf(stderr, "debug_bytecode: pc=%d, opcode=%s\n", pcRel(pc, cl->p), opnames[GET_OPCODE(i)]);
 #endif
     #if 0
       /* low-level line tracing for debugging Lua */

With the optimiser ON I get:

$ YKD_OPT=1 YKD_SERIALISE_COMPILATION=1 ~/research/yklua/src/lua havlak_onefile.lua
...
debug_bytecode: pc=13, opcode=GETFIELD
debug_bytecode: pc=14, opcode=LOADI
debug_bytecode: pc=15, opcode=FORPREP

thread '<unnamed>' panicked at ykrt/src/compile/jitc_yk/opt/mod.rs:592:13:
assertion failed: (inst.expect() && v.to_zero_ext_u8().unwrap() == 1) ||
    (!inst.expect() && v.to_zero_ext_u8().unwrap() == 0)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

With the optimiser OFF, I get #1594, which I'm already investigating.

It's not impossible that something to do with #1594 could be feeding garbage to the trace optimiser. Not sure. If we think that's likely then we can wait for the fix for #1594.

@ltratt
Copy link
Contributor

ltratt commented Feb 10, 2025

With my build with asserts on (I hope you've got debug asserts on!) I get:

debug_bytecode: pc=20, opcode=SETTABLE
debug_bytecode: pc=21, opcode=RETURN0
debug_bytecode: pc=3, opcode=RETURN0

thread '<unnamed>' panicked at ykrt/src/compile/jitc_yk/codegen/x64/deopt.rs:201:17:
assertion `left == right` failed: duplicate deopt with different value
  left: 0
 right: 65735
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
fatal runtime error: failed to initiate panic, error 5

@vext01 vext01 assigned vext01 and unassigned ltratt Feb 10, 2025
@vext01
Copy link
Contributor Author

vext01 commented Feb 10, 2025

Here's the test case reduced further, about half the size:

D = {}
do
	E = 10
	function D:B() end
	function D:E()
		if A then
			for A = 0, E do
			end
		end
	end
end
C = {}
do
	function C.D()
		F = {
			F = D,
		}
		return setmetatable(F, {
			__index = C,
		})
	end
	function C:A()
		self.F:E()
	end
end
G = {}
do
	function G.D(F)
		A = F:F()
		A:A()
		A:A()
	end
end
F = {}
do
	function F.D()
		setmetatable(G, {
			__index = F,
		})
	end
	function F:F()
		if D:B() then
		else
			B = C.D()
		end
		return B
	end
end
A = {}
do
	function A.D()
		F.D()
		F = {}
		return setmetatable(F, {
			__index = A,
		})
	end
	function A:C()
		G.D(G)
	end
	function A:F()
		self:C()
	end
	function A:E()
		do
			do
				for _ = 0, E do
					self:F()
				end
			end
		end
	end
end
B = {}
do
	function B:F()
		A.D():E()
	end
end
B:F()

@vext01
Copy link
Contributor Author

vext01 commented Feb 10, 2025

Looking at the targets of the deopt routine, I can see two different AOT variables wanting to write to rbp-44, but with differing values:

...
debug_bytecode: pc=3, opcode=RETURN0
yk-jit-event: deoptimise: GuardIdx(17)
...
[ykrt/src/compile/jitc_yk/codegen/x64/deopt.rs:208:13] aotvar = LiveVar {
    locs: [
        Indirect(
            6,
            -44,
            4,
        ),
    ],
}
...
[ykrt/src/compile/jitc_yk/codegen/x64/deopt.rs:208:13] aotvar = LiveVar {
    locs: [
        Register(
            0,
            4,
            [
                -44,
            ],
        ),
    ],
}
...
thread '<unnamed>' panicked at ykrt/src/compile/jitc_yk/codegen/x64/deopt.rs:201:17:
assertion `left == right` failed: duplicate deopt -44 with different value
  left: 0
 right: 65735

I wonder if this is a bug in the spillmap stuff again.

@vext01
Copy link
Contributor Author

vext01 commented Feb 10, 2025

Here's some more info on the two variables in question and the instructions that define them:

aotvar=LiveVar { locs: [Indirect(6, -44, 4)] } -> ConstInt { bits: 32, v: 0 }
inst=%16_0: i32 = phi bb15 -> %14_0, bb13 -> 0i32
aotvar=LiveVar { locs: [Register(0, 4, [-44])] } -> Register(GP(RAX))
inst=%16_4: i32 = promote %16_3 [safepoint: 5301i64, (%0_0, %0_2, %0_5, %0_6, %0_7, %0_8, %0_9, %0_10, %0_11, %0_12, %0_13, %0_14, %0_15, %0_16, %0_17, %0_18, %0_19, %0_20, %0_21, %0_22, %0_23, %0_24, %0_25, %0_26, %0_27, %0_28, %0_2, 9, %0_30, %0_31, %0_32, %0_33, %0_34, %0_35, %6_1, %6_3, %6_4, %6_7, %6_8, %12_3, %12_4, %12_5, %13_1, %16_1, %16_0, %16_2, %16_3, %16_4)]

In the context of the aot module:

bb16:
   %16_0: i32 = phi bb15 -> %14_0, bb13 -> 0i32
   %16_1: ptr = phi bb15 -> %15_1, bb13 -> %13_2
   %16_2: ptr = ptr_add %13_1, 4
   %16_3: i32 = load %13_1
   %16_4: i32 = promote %16_3 [safepoint: 5301i64, (%0_0, %0_2, %0_5, %0_6, %0_7, %0_8, %0_9, %0_10, %0_11, %0_12, %0_13, %0_14, %0_15, %0_16, %0_17, %0_18, %0_19, %0_20, %0_21, %0_22, %0_23, %0_24, %0_25, %0_26, %0_27, %0_28, %0_29, %0_30, %0_31, %0_32, %0_33, %0_34, %0_35, %6_1, %6_3, %6_4, %6_7, %6_8, %12_3, %12_4, %12_5, %13_1, %16_1, %16_0, %16_2, %16_3, %16_4)]

@ltratt
Copy link
Contributor

ltratt commented Feb 10, 2025

I assume @ptersilie will be interested in this too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants