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

[Paul Sandoz]

On Fri, 23 Aug 2024 15:52:52 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 one additional commit since the last revision:
> 
>   roundtrip stabilization

Overall this looks fine, and looks manageable in terms of complexity. I still need to look more closely. The prior approach served a purpose to move forward, but it looks like we no longer need it. AFAICT there are a few advantages to this approach:
1. There may be a closer correspondence to vars in the lowered and lifted model. We should be able to lower models in pure SSA or not. We lift into vars and if needed a separate SSA transformation can be applied.
2. We don't have to deal with the edge case of try/catch/finally reading and writing variables.

Do you have a sense of where the additional failures in the corpus are occurring? I am hoping that such edge cases do not result in large increase in complexity.

src/java.base/share/classes/java/lang/reflect/code/bytecode/BytecodeLift.java line 235:

> 233:                 case ITEM_DOUBLE -> params.add(JavaType.DOUBLE);
> 234:                 case ITEM_LONG -> params.add(JavaType.LONG);
> 235:                 case ITEM_NULL -> params.add(JavaType.wildcard());

That's a curious change, can you explain you did that?

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

PR Comment: https://git.openjdk.org/babylon/pull/218#issuecomment-2307423100
PR Review Comment: https://git.openjdk.org/babylon/pull/218#discussion_r1729232010