Unreachability

[Maurizio Cimadamore]

Paul,
the fix you propose looks (very) good.

Luckily, the alive analyzer is one of the simplest visitors we have in 
Flow - and we also have to run it as part of lambda type-checking. So it 
should be safely reusable, like you did.

Maurizio

On 17/10/2024 00:16, Paul Sandoz wrote:
> Here’s a possible fix:
>
>          CoreOp.FuncOp scanMethod() {
>              scan(body);
>              appendReturnOrUnreachable(body);
>              CoreOp.FuncOp func = CoreOp.func(name.toString(), stack.body);
>              func.setLocation(generateLocation(currentNode, true));
>              return func;
>          }
>
>          private <O extends Op & Op.Terminating> void appendReturnOrUnreachable(JCTree body) {
>              // Append only if an existing terminating operation is not present
>              if (lastOp == null || !(lastOp instanceof Op.Terminating)) {
>                  if (isReachable(body)) {
>                      append(CoreOp._return());
>                  } else {
>                      append(CoreOp.unreachable());
>                  }
>              }
>          }
>
>          boolean isReachable(JCTree node) {
>              return flow.aliveAfter(typeEnvs.get(currentClassSym), node, make);
>          }
>
> Paul.
>
>> On Oct 16, 2024, at 1:18 PM, Paul Sandoz <paul.sandoz at oracle.com> wrote:
>>
>> Hi,
>>
>> Gary encountered an issue when transforming say:
>>
>>     @CodeReflection
>>     public static float f(float input) {
>>         if (input >= 0f) {
>>             return 1f;
>>         } else {
>>             return 0f;
>>         }
>>         // Unreachable
>>     }
>>
>> Compiler produced model:
>>
>> func @"f" @loc="10:5:..." (%0 : float)float -> {
>>     %1 : Var<float> = var %0 @"input" @loc="10:5";
>>     java.if @loc="13:9"
>>         ()boolean -> {
>>             %2 : float = var.load %1 @loc="13:13";
>>             %3 : float = constant @"0.0" @loc="13:22";
>>             %4 : boolean = ge %2 %3 @loc="13:13";
>>             yield %4 @loc="13:9";
>>         }
>>         ()void -> {
>>             %5 : float = constant @"1.0" @loc="14:20";
>>             return %5 @loc="14:13";
>>         }
>>         ()void -> {
>>             %6 : float = constant @"0.0" @loc="16:20";
>>             return %6 @loc="16:13";
>>         };
>>     return @loc="10:5";
>> };
>>
>> Lowered model:
>>
>> func @"f" @loc="10:5:..." (%0 : float)float -> {
>>     %1 : Var<float> = var %0 @"input" @loc="10:5";
>>     %2 : float = var.load %1 @loc="13:13";
>>     %3 : float = constant @"0.0" @loc="13:22";
>>     %4 : boolean = ge %2 %3 @loc="13:13";
>>     cbranch %4 ^block_1 ^block_2;
>>
>>   ^block_1:
>>     %5 : float = constant @"1.0" @loc="14:20";
>>     return %5 @loc="14:13";
>>
>>   ^block_2:
>>     %6 : float = constant @"0.0" @loc="16:20";
>>     return %6 @loc="16:13";
>>
>>   ^block_3:
>>     return @loc="10:5";
>> };
>>
>>
>> The compiler produced model is incorrect placing a return operation when it should be an unreachable operation.
>>
>> There are some reminders in ReflectMethods:
>>
>>         CoreOp.FuncOp scanMethod() {
>>             scan(body);
>>             // @@@ Check if unreachable
>>             appendTerminating(CoreOp::_return);
>>             CoreOp.FuncOp func = CoreOp.func(name.toString(), stack.body);
>>             func.setLocation(generateLocation(currentNode, true));
>>             return func;
>>         }
>>
>> We need to determine if the last statement yields control or not. (It’s easy to see if we lower, as ^block_3 is never referenced.)
>>
>> In this case we can know because there is no return value in scope, but generally we cannot rely upon that.
>>
>> The compiler already checks this, I believe in Flow.AliveAnalyzer. However, it does not add additional information to the attributed tree - it does not need to.
>>
>> Can we reuse the method Flow.aliveAfter? Although I don’t know how to get access to an instance of Env<AttrContext>. Do we get it from TypeEnvs?
>>
>> Paul.