[code-reflection] RFR: Testing BytecodeLift and BytecodeGenerator roundtrip stability [v4]

[Paul Sandoz]

On Thu, 15 Feb 2024 16:14:22 GMT, Adam Sotona <asotona at openjdk.org> wrote:

>> Hi,
>> I've added `TestLiftSmallCorpus::testDoubleRoundtripStability` to test stability of the `BytecodeLift` and `BytecodeGenerator` roundtrip on JDK classes (aka "Small Corpus"). 
>> 
>> The test takes ~40k methods from JDK classes and tries to lift them from bytecode, generate, lift again and generate again. Only about 4300 methods pass the roundtrip now, because `BytecodeLift` and `BytecodeGenerator` are still under construction.
>> 
>> The test then compares "normalized" bytecode from the first round and the second round and fails with printed details if differ. The test actually contains a non-zero error threshold before failure. This threshold can be manually adjusted when working on `BytecodeLift` and `BytecodeGenerator` improvements.
>> This approach allows to incrementally fix `BytecodeLift` and `BytecodeGenerator` roundtrip, as well as to extend their coverage.
>> 
>> This patch also includes several fixes of bugs discovered during work on `TestLiftSmallCorpus`:
>> - conditional branches are unwound to avoid growing number of blocks during the round trip
>> - conditional branches are inverted to fix roundtrip stability
>> - partial fix of block parameters construction
>> - partial fix of the locals overrides
>> - aggressive local variable slots overrides in `BytecodeGenerator` are temporary disabled until `BytecodeLift` is ready to support it 
>> - fixed redundant block argument assignements of unused variables in `BytecodeGenerator`
>> - fixed storing of values into invalid slots
>> 
>> Partial fix means it significantly reduced the error threshold (in order of magnitudes), however there are still some cases failing.
>> 
>> I would like to integrate this patch, because it introduces a tool to allow subsequent incremental progress.
>> 
>> Please review.
>> 
>> Thank you,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with one additional commit since the last revision:
> 
>   more constrained TestLiftSmallCorpus

Marked as reviewed by psandoz (Lead).

Very good, ~10% passing is a good start, esp. considering there is no support for lowering and lifting of lambdas.

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

PR Review: https://git.openjdk.org/babylon/pull/24#pullrequestreview-1883635472
PR Comment: https://git.openjdk.org/babylon/pull/24#issuecomment-1947039975