Add interpreter emulator for ECS in inliner

[cathyzhyi]

This change adds the infrastructure to emulate the interpreter
execution during estimate code size of target in inliner.

Also the loop looking for callsites in ECS is refactored and moved
to findAndCreateCallsitesFromBytecodes. ECS will be changed to call
this function in a following commit.

issue: #6204
Signed-off-by: Yi Zhang [email protected]

[andrewcraik]

FYI @efferifick since you are also working in this space

[andrewcraik]

What is the lifetime of the CallSites that are allocated? to make the code easier to maintain, I'd almost suggest that the CallSites be allocated in a region held by the interpreter object so that if we wanted to generate this info and throw it away at some later point during the compile we would have that flexibility. Baking use of the compilation heap in for something as big as this seems a bit dirty - it would be better to control when the memory is freed.

[andrewcraik]

Why is all the operand tracking etc done using the stack region rather than a region specifically tied to the lifetime of interpretation so we can free it as soon as the interpreter is done running to keep the footprint lower?

[andrewcraik]

Just marking that I requested changes and further review will be needed once the PR is updated/commented on.

[cathyzhyi]

What is the lifetime of the CallSites that are allocated? to make the code easier to maintain, I'd almost suggest that the CallSites be allocated in a region held by the interpreter object so that if we wanted to generate this info and throw it away at some later point during the compile we would have that flexibility. Baking use of the compilation heap in for something as big as this seems a bit dirty - it would be better to control when the memory is freed.

TheCallsites would be needed for the actually inlining as well so they have been allocated on heap memory in estimiate code size as well as in other places in inliner. Allocating to memory that can be reclaimed when the inlining is finished would be a good idea but is probably out of the scope of this item.

[cathyzhyi]

Why is all the operand tracking etc done using the stack region rather than a region specifically tied to the lifetime of interpretation so we can free it as soon as the interpreter is done running to keep the footprint lower?

Thanks for the suggestion. Allocated new memory region at the beginning of interpretation, namely findAndCreateCallsitesFromBytecodes so that as soon as findAndCreateCallsitesFromBytecodes returns the memories for operand tracking are release.

[andrewcraik]

Allocating to memory that can be reclaimed when the inlining is finished would be a good idea but is probably out of the scope of this item.

Could you create an issue for this? It is definitely something we should look at when we need to shrink the footprint again...

[andrewcraik]

Overall, this is getting close. Is the trace in the base bytecode iterator enough to know which cases etc the interpeter is processing? There is some debug trace in the methods you added, but it doesn't quite seem like enough to follow the whole interpretation - I'm guessing the 'missing' bits come from the base?

[cathyzhyi]

Overall, this is getting close. Is the trace in the base bytecode iterator enough to know which cases etc the interpeter is processing? There is some debug trace in the methods you added, but it doesn't quite seem like enough to follow the whole interpretation - I'm guessing the 'missing' bits come from the base?

There is InterpreterEmulator::dumpStack() that dumps the stack content and the current bytecode. findAndCreateCallsitesFromBytecodescalls this after executing each bytecode.

[cathyzhyi]

Allocating to memory that can be reclaimed when the inlining is finished would be a good idea but is probably out of the scope of this item.

Could you create an issue for this? It is definitely something we should look at when we need to shrink the footprint again...

Created an issue to track the problem. #7950.

[andrewcraik]

Jenkins test sanity all jdk11

[andrewcraik]

the failed job is just infra - the rest of the testing is sufficient to merge this