[code-reflection] RFR: OpWriter block naming align with block indexes

[Paul Sandoz]

On Fri, 24 May 2024 14:12:28 GMT, Adam Sotona <asotona at openjdk.org> wrote:

> I propose to change OpWriter block naming conventions to be aligned with block indexes.
> 
> It is very hard to debug when there is no connection between debugged body and available prints provided by OpWriter.
> This patch composes block name as composition of block indexes from super-parent-body block index down the actual block index. With fallback to the `block__<counter>` In case the block is not a part of a body.
> 
> This is and example of such print:
> 
>     %0 : java.util.function.IntUnaryOperator = lambda @loc="88:31" (%1 : int)int -> {
>         %2 : Var<int> = var %1 @"i" @loc="88:31";
>         %3 : int = var.load %2 @loc="89:17";
>         %4 : int = constant @"0" @loc="89:21";
>         %5 : boolean = gt %3 %4 @loc="89:17";
>         cbranch %5 ^block_0_1 ^block_0_2;
>       
>       ^block_0_1:
>         %6 : java.util.function.IntUnaryOperator = lambda @loc="90:38" (%7 : int)int -> {
>             %8 : Var<int> = var %7 @"ii" @loc="90:38";
>             %9 : int = var.load %8 @loc="91:25";
>             %10 : int = constant @"0" @loc="91:30";
>             %11 : boolean = lt %9 %10 @loc="91:25";
>             cbranch %11 ^block_0_1_1 ^block_0_1_2;
>           
>           ^block_0_1_1:
>             %12 : int = constant @"2" @loc="92:32";
>             %13 : int = var.load %8 @loc="92:36";
>             %14 : int = mul %12 %13 @loc="92:32";
>             %15 : int = constant @"1" @loc="92:41";
>             %16 : int = sub %14 %15 @loc="92:32";
>             return %16 @loc="92:25";
>           
>           ^block_0_1_2:
>             %17 : int = var.load %8 @loc="94:32";
>             return %17 @loc="94:25";
>           
>           ^block_0_1_3:
>             return @loc="90:38";
>         };
>         %18 : Var<java.util.function.IntUnaryOperator> = var %6 @"o" @loc="90:17";
>         %19 : java.util.function.IntUnaryOperator = var.load %18 @loc="97:24";
>         %20 : int = constant @"2" @loc="97:37";
>         %21 : int = var.load %2 @loc="97:41";
>         %22 : int = mul %20 %21 @loc="97:37";
>         %23 : int = invoke %19 %22 @"java.util.function.IntUnaryOperator::applyAsInt(int)int" @loc="97:24";
>         return %23 @loc="97:17";
>       
>       ^block_0_2:
>         %24 : java.util.function.IntUnaryOperator = lambda @loc="99:38" (%25 : int)int -> {
>             %26 : Var<int> = var %25 @"ii" @loc="99:38";
>             %27 : int = var.load %26 @loc="100:25";
>             %28 : int = constant @"0" @loc="100:30";
>             %29 : boolean = lt %27 %28 @loc="100:25";
>             cbranch %29 ^block_0_2_1 ^block_0_2_2;
>   ...

src/java.base/share/classes/java/lang/reflect/code/writer/OpWriter.java line 58:

> 56: 
> 57:         private String blockId(Block b) {
> 58:             if (b.index() < 0) return "__" + blockOrdinal++;

This should always be true `b.parentBody().blocks().indexOf(this) == b.index()` - a block's index should never be negative. Are you observing such a case?

src/java.base/share/classes/java/lang/reflect/code/writer/OpWriter.java line 61:

> 59:             Op po;
> 60:             Block pb;
> 61:             return (((po = b.parentBody().parentOp()) != null

The expression `b.parentBody().parentOp()` should never return a `null` value, so i think we can simplify to something like:

String prefix = b.parentBody().parentOp().parentBlock() instanceof Block pb
        ? blockId(pb) 
        : "";
return prefix + "_" + b.index();


But perhaps there is a simpler way. A block has to be encountered before any descendant blocks. If `b` has an ancestor block `p`, then the name of `b` is `name(p) + "_" + b.index()`, otherwise the name of `b` is `"block_" + b.index()`.

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

PR Review Comment: https://git.openjdk.org/babylon/pull/91#discussion_r1614120569
PR Review Comment: https://git.openjdk.org/babylon/pull/91#discussion_r1614119304