Skip to content

Fix panic in p_block when all control-point live values are globals#2167

Open
Pavel-Durov wants to merge 7 commits into
ykjit:masterfrom
Pavel-Durov:fix-no-arg-trace-when-all-globals
Open

Fix panic in p_block when all control-point live values are globals#2167
Pavel-Durov wants to merge 7 commits into
ykjit:masterfrom
Pavel-Durov:fix-no-arg-trace-when-all-globals

Conversation

@Pavel-Durov
Copy link
Copy Markdown
Contributor

This is something came up while ykifying the yksompp interpreter - there are a lot of static and global varibles.

When all live values at a control point are globals, their addresses are baked into the IR and nothing needs passing via registers or stack. This produces a trace block with no Arg instructions, which previously caused an unconditional panic in p_block.

This fix make yksompp ~20% faster on AWFY List 150 4.

When all live values at a control point are globals, their addresses are
baked into the IR and nothing needs passing via registers or stack. This
produces a trace block with no Arg instructions, which previously caused
an unconditional panic in p_block.
@@ -0,0 +1,43 @@
// Run-time:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICS this test doesn't prove it's testing what it claims. I think we would need to have IR in here and check that there are no arg instructions.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the current test all interpreter state (g_pc, g_mt, i) are set as globals, so the control point safepoint records no live locals:

  bb0:
    %0_0: ptr = alloca {0: i64}, 1, 8
    %0_1: ptr = call malloc(1000000i64)
    %0_2: ptr = ptr_add %0_1, 16
    *@shadowstack_head = %0_1
    *@shadowstack_0 = %0_2
    %0_5: ptr = ptr_add %0_1, 0
    *%0_5 = 0i32
    br bb1
...
  bb7:
    # guard_body_no_args.c:34: yk_mt_control_point(g_mt, &i);
    %7_0: ptr = load @g_mt
    call llvm.experimental.patchpoint.void(0i64, 13i32, __ykrt_control_point, 3i32, %7_0, @i, 0i64) [safepoint: 0i64, ()]
    # guard_body_no_args.c:35: debug_fn();
    br bb8

In Contrast, if g_pc is moved from a global to a local declared inside main it assigns it a shadow stack slot and it appears as a live variable:

-long g_pc = 0;
+
 YkMT *g_mt = NULL;
 YkLocation i;

   i = yk_location_new();
+  long g_pc = 0;

Produced IR:

  bb0:
    %0_0: ptr = alloca {0: i64}, 1, 8
    %0_1: ptr = call malloc(1000000i64)
    %0_2: ptr = ptr_add %0_1, 16
    *@shadowstack_head = %0_1
    *@shadowstack_0 = %0_2
    %0_5: ptr = ptr_add %0_1, 0
    %0_6: ptr = ptr_add %0_1, 8  ; shadow stack slot for g_pc
    *%0_5 = 0i32
    br bb1
  ...
  bb8:
    # guard_body_no_args.c:35: yk_mt_control_point(g_mt, &i);
    %8_0: ptr = load @g_mt
    call llvm.experimental.patchpoint.void(0i64, 13i32, __ykrt_control_point, 3i32, %8_0, @i, 0i64, %0_6) [safepoint: 0i64, (%0_6)]
    # guard_body_no_args.c:36: debug_fn();
    br bb9

%0_6 is live at the control point and yk emits an Arg instruction.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I mean is: the C code might show this but the lang_tester test doesn't prove this. We'd need YKD_LOG_IR=jit and then guarantee that has no args in it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok got it!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated 0699fb1

@ltratt
Copy link
Copy Markdown
Contributor

ltratt commented May 13, 2026

Maybe I'm missing out on something here, but when I look in yksompp at Interpreter::start" (i.e. the function containing YK_DISPATCH_TRAMPOLINE) I see lots of local variables. That said, the fact that the YK_DISPATCH_TRAMPOLINEis at the end of that function might be a factor, but because of thegoto`s, the control flow is disguised, so I'm not very confident about that.

Put another way: I get the problem this PR is fixing, but I don't quite understand how it occurs in practise.

@Pavel-Durov
Copy link
Copy Markdown
Contributor Author

Pavel-Durov commented May 13, 2026

The control point block in yksompp looks like this:

  bb346:
    %346_0: ptr = phi bb9 -> %9_0, bb0 -> %0_3
    %346_1: ptr = load @_ZN8Universe5yk_mtE
    %346_2: ptr = ptr_add %346_0, 112
    %346_3: ptr = load %346_2
    %346_4: i64 = load @_ZN11Interpreter19bytecodeIndexGlobalE
    %346_5: ptr = ptr_add %346_3, 0 + (%346_4 * 8)
    call llvm.experimental.patchpoint.void(0i64, 13i32, __ykrt_control_point, 3i32, %346_1, %346_5, 0i64, %0_0) [safepoint: 0i64, (%0_0)]
    br bb347

It has one live varible - %0_0
Which is printBytecodes, set always to false so its probably constant folded?

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

Successfully merging this pull request may close these issues.

2 participants