Adjust Yk tests to handle Mem2Reg Changes

[nmdis1999]

This PR aims to fix yk test cases so that we can check the right IR when enabling mem2reg pass. There is another test case arithmetic.newgc.c which fails at tuntime with error:

thread '<unnamed>' panicked at ykrt/src/compile/jitc_yk/aot_ir.rs:502:13:
explicit panic

here is the reduced test case which reporduce this error:

#include <yk_testing.h>
void main(int argc) {
  YkMT *mt = yk_mt_new(NULL);
  YkLocation loc = yk_location_new();
  while (4) {
    yk_mt_control_point(mt, &loc);
    fprintf(stderr, "", argc);
  }
}

[ltratt]

@ptersilie @vext01 Any ideas what this function should do instead of panic?

    /// Return the `InstID` of a local variable operand. Panics if called on other kinds of
    /// operands.
    pub(crate) fn to_inst_id(&self) -> InstID {
        let Self::LocalVariable(iid) = self else {
            panic!()
        };
        iid.clone()
    }

[ptersilie]

@ptersilie @vext01 Any ideas what this function should do instead of panic?

    /// Return the `InstID` of a local variable operand. Panics if called on other kinds of
    /// operands.
    pub(crate) fn to_inst_id(&self) -> InstID {
        let Self::LocalVariable(iid) = self else {
            panic!()
        };
        iid.clone()
    }

It makes no sense to call to_inst_id on anything that isn't an instruction. So panic seems like the right choice here? Are we seeing this panic? I noticed some constant propagation happening in the changes, so it could be related to that: we assume something is a function, but it's a constant instead. We should print self here to see why type we are getting here.

[ltratt]

Are we seeing this panic?

Yes, see the opening message to this PR, which shows the panic occurring. I agree that it's quite probably because something is becoming a constant. My guess is that addressing that isn't rocket science, and anything we can turn from a variable into a constant at AOT is a bonus!

[vext01]

@ptersilie @vext01 Any ideas what this function should do instead of panic?

I think panic is correct here.

The function is designed to return the instruction that defines the local variable "referenced" by the operand. Instructions only define local variables, so it makes no sense to call this on any other kind of operand.

[ltratt]

I'm confused: it sounds like we're saying "panic is the right thing" as if we're right and the LLVM IR is wrong? Can we not think about how we might potentially be able to alter some part of the system to do The Right Thing? After all, what this PR is really showing is "when you turn on optimisations, LLVM IR is of a different shape than we've previously considered". I'm not saying we should necessarily fix it right now, but maybe we can give @nmdis1999 a hint of what to look at?

[vext01]

The LLVM IR is unlikely to be incorrect. It's more likely a lowering bug.

To understand better, I think you'd need to look at the LLVM IR and our IR side by side. You might want to put print statements in the Yk IR serialiser to see which paths are taken and how certain LLVM constructs are lowered.

[ltratt]

It's more likely a lowering bug.

Aha!

@nmdis1999 do you think you can follow Edd's debugging suggestion? It sounds like it's probably a bug in ykllvm: the good news is that you have a nice, small, test case to use to hunt the problem down.

[nmdis1999]

Printing self gives following output:

add 5
sub 3
mul 12
add 4
sub 2
mul 9
add 3
sub 1
mul 6
add 2
sub 0
mul 3
exit
[ykrt/src/compile/jitc_yk/aot_ir.rs:501:40] self = LocalVariable(
    InstID {
        func_idx: FuncIdx(
            2,
        ),
        bb_idx: BBlockIdx(
            0,
        ),
        inst_idx: InstIdx(
            0,
        ),
    },
)
[ykrt/src/compile/jitc_yk/aot_ir.rs:501:40] self = LocalVariable(
    InstID {
        func_idx: FuncIdx(
            2,
        ),
        bb_idx: BBlockIdx(
            1,
        ),
        inst_idx: InstIdx(
            2,
        ),
    },
)
[ykrt/src/compile/jitc_yk/aot_ir.rs:501:40] self = LocalVariable(
    InstID {
        func_idx: FuncIdx(
            2,
        ),
        bb_idx: BBlockIdx(
            0,
        ),
        inst_idx: InstIdx(
            4,
        ),
    },
)
[ykrt/src/compile/jitc_yk/aot_ir.rs:501:40] self = LocalVariable(
    InstID {
        func_idx: FuncIdx(
            2,
        ),
        bb_idx: BBlockIdx(
            0,
        ),
        inst_idx: InstIdx(
            3,
        ),
    },
)
[ykrt/src/compile/jitc_yk/aot_ir.rs:501:40] self = LocalVariable(
    InstID {
        func_idx: FuncIdx(
            2,
        ),
        bb_idx: BBlockIdx(
            0,
        ),
        inst_idx: InstIdx(
            1,
        ),
    },
)
[ykrt/src/compile/jitc_yk/aot_ir.rs:501:40] self = Arg {
    func_idx: FuncIdx(
        2,
    ),
    arg_idx: ArgIdx(
        0,
    ),
}

I have narrowed down this problem, it seems that something goes wrong when we use yk_mt_drop function in this program. So, fwiu the crash only happens after the loop has been processed. @vext01 @ltratt @ptersilie

[ltratt]

@nmdis1999 Hang on, originally you got a crash in aot_ir.rs but now you're saying the crash happens at runtime? [Note: yk_mt_drop is not recognised as a special function by any part of ykllvm/yk.]

[nmdis1999]

I did get crash in aot_ir.rs I mean that the I see the output of for loop on stdout and then a crash. Sorry for yk_mt_drop line, I think I misunderstood how it works.

[ptersilie]

What's the crash? Does it have an error/line number? Looks like it might be related to Arg which we might not deal with properly at the moment.

[nmdis1999]

What's the crash? Does it have an error/line number? Looks like it might be related to Arg which we might not deal with properly at the moment.

@ptersilie Here:thread '' panicked at ykrt/src/compile/jitc_yk/aot_ir.rs:502:13: explicit panic

And yes, it is due to an Arg I have added the dbg print for that line in above comment.

[nmdis1999]

@ptersilie aot IR:

# IR format version: 0
# Num funcs: 14
# Num consts: 9
# Num global decls: 6
# Num types: 21
global_decl @shadowstack_0
global_decl @stderr
global_decl @.str
global_decl @.str.1
global_decl @.str.2
global_decl @.str.3

func foo(%arg0: i32) -> i32 {
  bb0:
    %0_0: ptr = load @shadowstack_0
    br bb1
  bb1:
    %1_0: i32 = add %arg0, 3i32
    br bb3
  bb2:
    ret %1_0
  bb3:
    br bb2
}

func llvm.dbg.value(%arg0: ?ty<metadata>, %arg1: ?ty<metadata>, %arg2: ?ty<metadata>);

func main(%arg0: i32, %arg1: ptr) -> i32 {
  bb0:
    %0_0: ptr = alloca {0: i32, 64: ptr, 128: ptr, 192: ptr, 256: ptr}, 1, 8
    %0_1: ptr = call malloc(1000000i64)
    *@shadowstack_0 = %0_1
    %0_3: ptr = alloca {0: i64}, 1, 8
    %0_4: ptr = ptr_add %0_1, 0
    br bb1
  bb1:
    %1_0: ptr = ptr_add %0_1, 16
    *@shadowstack_0 = %1_0
    %1_2: ptr = call yk_mt_new(0x0)
    br bb2
  bb2:
    *@shadowstack_0 = %0_1
    %2_1: ptr = ptr_add %0_1, 16
    *@shadowstack_0 = %2_1
    call yk_mt_hot_threshold_set(%1_2, 0i32)
    br bb3
  bb3:
    *@shadowstack_0 = %0_1
    %3_1: ptr = ptr_add %0_1, 16
    *@shadowstack_0 = %3_1
    %3_3: i64 = call yk_location_new()
    br bb4
  bb4:
    *@shadowstack_0 = %0_1
    *%0_3 = %3_3
    *%0_4 = 2i32
    br bb5
  bb5:
    %5_0: i32 = load %0_4
    br bb6
  bb6:
    br bb7
  bb7:
    %7_0: i32 = load %0_4
    %7_1: i1 = sgt %7_0, 0i32
    condbr %7_1, bb8, bb13 [safepoint: 0i64, (%arg0, %0_0, %0_1, %0_3, %0_4, %1_2, %7_1)]
  bb8:
    %8_0: ptr = ptr_add %0_1, 16
    *@shadowstack_0 = %8_0
    %8_2: ptr = ptr_add %0_0, 0
    *%8_2 = %arg0
    %8_4: ptr = ptr_add %0_0, 8
    *%8_4 = %0_1
    %8_6: ptr = ptr_add %0_0, 16
    *%8_6 = %0_3
    %8_8: ptr = ptr_add %0_0, 24
    *%8_8 = %0_4
    %8_10: ptr = ptr_add %0_0, 32
    *%8_10 = %1_2
    %8_12: ptr = call llvm.frameaddress.p0(0i32)
    call __ykrt_control_point(%1_2, %0_3, %0_0, %8_12)
    br bb9
  bb9:
    *@shadowstack_0 = %0_1
    %9_1: i32 = load %0_4
    %9_2: i32 = add %9_1, %arg0
    %9_3: i32 = sub %9_1, %arg0
    %9_4: i32 = mul %9_1, %arg0
    %9_5: i32 = mul %9_4, 3i32
    %9_6: ptr = load @stderr
    %9_7: ptr = ptr_add %0_1, 16
    *@shadowstack_0 = %9_7
    %9_9: i32 = call fprintf(%9_6, @.str, %9_2)
    br bb10
  bb10:
    *@shadowstack_0 = %0_1
    %10_1: ptr = load @stderr
    %10_2: ptr = ptr_add %0_1, 16
    *@shadowstack_0 = %10_2
    %10_4: i32 = call fprintf(%10_1, @.str.1, %9_3)
    br bb11
  bb11:
    *@shadowstack_0 = %0_1
    %11_1: ptr = load @stderr
    %11_2: ptr = ptr_add %0_1, 16
    *@shadowstack_0 = %11_2
    %11_4: i32 = call fprintf(%11_1, @.str.2, %9_5)
    br bb12
  bb12:
    *@shadowstack_0 = %0_1
    %12_1: i32 = load %0_4
    %12_2: i32 = add %12_1, -1i32
    *%0_4 = %12_2
    br bb7
  bb13:
    %13_0: ptr = load @stderr
    %13_1: ptr = ptr_add %0_1, 16
    *@shadowstack_0 = %13_1
    %13_3: i64 = call fwrite(@.str.3, 5i64, 1i64, %13_0)
    br bb14
  bb14:
    *@shadowstack_0 = %0_1
    br bb16
  bb15:
    ret 0i32
  bb16:
    br bb15
}

func yk_mt_new(%arg0: ptr) -> ptr;

func yk_mt_hot_threshold_set(%arg0: ptr, %arg1: i32);

func llvm.dbg.declare(%arg0: ?ty<metadata>, %arg1: ?ty<metadata>, %arg2: ?ty<metadata>);

func yk_location_new() -> i64;

func yk_mt_control_point(%arg0: ptr, %arg1: ptr);

func fprintf(%arg0: ptr, %arg1: ptr, ...) -> i32;

func fwrite(%arg0: ptr, %arg1: i64, %arg2: i64, %arg3: ptr) -> i64;

func malloc(%arg0: i64) -> ptr;

func __ykrt_control_point(%arg0: ptr, %arg1: ptr, %arg2: ptr, %arg3: ptr);

func llvm.frameaddress.p0(%arg0: i32) -> ptr;

func llvm.experimental.stackmap(%arg0: i64, %arg1: i32, ...);

[nmdis1999]

ready for review @ltratt @vext01