[code-reflection] RFR: BytecodeLift directly calculating local variables + related BytecodeGenerator fixes [v5]

[Paul Sandoz]

On Mon, 26 Aug 2024 15:58:31 GMT, Adam Sotona <asotona at openjdk.org> wrote:

>> Proposal of `BytecodeLift` implementation skipping intermediate use of `SlotOp` and `SlotSSA` transformation.
>> `LocalsTypeMapper` already handled majority of the variable mapping and its complexity grew to cover more use cases.
>> This patch adds a very simple slots model to `LocalsTypeMapper` to compute variables out of the bytecode directly .
>> `BytecodeLift` now knows all necessary information to emit relevant `VarOp` and `VarAccessOp`  directly.
>> 
>> Stability of `TestSmallCorpus` has slightly degraded, however it is just a temporary regression.
>> 
>> Please review.
>> 
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - reverted wildcard type use in BytecodeLift
>  - Bytecode lift names variables by slot number "slot#XY"
>  - Stabilization of the roundtrip - single-use var allocations deferred

src/java.base/share/classes/java/lang/reflect/code/bytecode/BytecodeGenerator.java line 987:

> 985: 
> 986:     // Checks if the Op.Result is used more than once in operands and block arguments
> 987:     private static boolean moreThanOneUse(Value val) {

Memo to self - we should have something in the API. More generally, liveness analysis of values might help formalize when a value is no longer used and its slot can be reused with another value. I suspect generating bytecode in from models in pure SSA might result use of more locals than actually needed.

src/java.base/share/classes/java/lang/reflect/code/bytecode/LocalsTypeMapper.java line 328:

> 326:             case IncrementInstruction i -> {
> 327:                 Slot v = locals.get(i.slot());
> 328:                 v.writes++;

Why is this required? I likely don't understand the significance of this and how it differs from a store instruction.

-------------

PR Review Comment: https://git.openjdk.org/babylon/pull/218#discussion_r1731489221
PR Review Comment: https://git.openjdk.org/babylon/pull/218#discussion_r1731517616