Remove remnants of the trace inputs struct.

[ptersilie]

Even thought the trace inputs struct was no longer used, it still existed and was passed into the control point. This commit removes any remnants of the former trace inputs struct and adjusts the code (in particular the inline assembly) accordingly.

Depends on: https://github.com/ykjit/ykllvm/pull/189

[ptersilie]

I've measured the speed up, which is between 1.02x and 1.6x:

test	before	after	speedup
db.lua	194ms	185ms	1.05
bigloop.lua	4.31s	4.22s	1.02
gc.lua	525ms	485ms	1.08
sort.lua	1.53s	955ms	1.6

I think the huge differences in speedups are because this change mostly affects the runtime of the interpreter. So tests where the interpreter runs more get bigger gains.

[ltratt]

Deletes code, speeds things up -- excellent!

[ltratt]

Huh though this row makes no esnse:

sort.lua    1.53    955     1.6

Indeed, the units in the table overall seem weird? Is it seconds or milliseconds or ... ?

[ptersilie]

Sorry, I hadn't finished formatting the table. Refresh the page please.

[ltratt]

OK, I get it. Next time could you use the same units for each cell in each column? It'll stop my mind exploding!

[ptersilie]

I just pasted this from my vim window where I crudely pasted the numbers that time gave me. :laughing: I'll try to format them better next time.

[ptersilie]

Needs https://github.com/ykjit/ykllvm/pull/189

[ltratt]

@ptersilie Is the ykllvm commit in this PR correct?

[ptersilie]

Whoops. I forgot I rebased ykllvm after pushing this PR. Fixed.

[ptersilie]

Added clippy suggestion. Ok to squash?

[ltratt]

Please squash.

[ptersilie]

Squashed.

[ltratt]

This diff is needed for --release mode:

- //     switch %{{9_1}}, bb{{bb14}}, [299 -> bb{{bb10}}, 298 -> bb{{bb12}}] [safepoint: 0i64, (%{{0_0}}, %{{0_1}}, %{{0_3}}, %{{0_4}}, %{{0_5}})]
+ //     switch %{{9_1}}, bb{{bb14}}, [299 -> bb{{bb10}}, 298 -> bb{{bb12}}] [safepoint: 1i64, (%{{0_0}}, %{{0_1}}, %{{0_3}}, %{{0_4}}, %{{0_5}})]

i.e. changing 0i64 to 1i64 but... why? In non-release mode 0i64 is correct. I'm not sure I know why the value has changed.

[ptersilie]

Which test is this? If I run cargo test --release I can't reproduce this. Silly me. It's in the log. Will look into it.

[ptersilie]

Hah! Our stackmaps insertion pass is non-deterministic: https://github.com/ykjit/ykllvm/blob/main/llvm/lib/Transforms/Yk/StackMaps.cpp#L50

I'm surprised we haven't noticed this sooner. Anyway, the order of the IDs doesn't really matter, so we have two choices:

1. Change the std::map to the C++ equivalent of an IndexMap.
2. Make sure tests don't try to pattern match the stackmap ID.

[ltratt]

I presume this is the same thing Jake ran into? It sounds to me like (1) is the right thing to do, if nothing else because it'll make debugging easier. But for this PR, right now, we can do (2) while we wait for (1) to land in ykllvm.

[ptersilie]

I didn't know Jake ran into something similar. I agree (1) would be the right fix so we can't run into this by accident any more. Will have to look if there's an IndexMap in C++.

I pushed a temporary fix to get this PR in. Ok to squash?

[ltratt]

Please squash.

[We can easily make a "custom" indexmap, because it's basically a combo of HashMap<T, usize> where the usize is an index into a Vec.]

[ptersilie]

Squashed.