(Condy) Folded of local variables are still considered when calculating the max number of local vars of a method

[Vicente Romero]

On 09/03/2018 06:40 AM, Maurizio Cimadamore wrote:
>
>
> On 02/09/18 19:02, Brian Goetz wrote:
>> It sounds like the dead-code elimination is done after we construct 
>> the LV tables?
> I think the issue is that, for javac, the local variable still exists 
> (e.g. there's a LocalItem for it), it is just never loaded, so you 
> never see it in the resulting code. But that means that it will be 
> taken into account when generating local var slots number (**).
>
> In a way, javac doesn't actively perform any dead code elimination - 
> it's the turning of all variable initializer into constants (which are 
> dealt with by ldc) which turns code generation lazy, meaning that bits 
> will only be generated when needed. So, the resulting code gives you 
> the feeling of dead code elimination, but in reality there is no piece 
> of code where javac says: it's ok to ditch this local variable. In 
> other word, DCE is an emergent property of the way in which we lazily 
> generate code.
>
> You can reproduce a similar behavior with this Java program which only 
> involves string constants:
>
> class Test {
>    static foo() {
>        String hello = "Hello!";
>    }
> }
>
> This gives:
>
>
> static void foo();
>     descriptor: ()V
>     flags: ACC_STATIC
>     Code:
>       stack=0, *locals=1,* args_size=0
>          0: return
>       LineNumberTable:
>         line 4: 0
>
>
> (**) I thought that debug info (e.g. LocalVariableTable) would be 
> affected too, but in reality when debug info is enabled, const folding 
> is disabled for var initializer - and the enhanced condy folding 
> support respects that invariant, so nothing to look at there.
>
> In order to get rid of this behavior we would need to look at avoiding 
> the call to code.newLocal in Gen::visitVarDef:
>
> public void visitVarDef(JCVariableDecl tree) {
>         VarSymbol v = tree.sym;
>         code.newLocal(v); // <----------------------
>         if (tree.init != null) {
>             checkStringConstant(tree.init.pos(), v.getConstValue());
>             if (v.getConstValue() == null || varDebugInfo) {
>                 Assert.check(code.isStatementStart());
>                 genExpr(tree.init, v.erasure(types)).load();
>                 items.makeLocalItem(v).store();
>                 Assert.check(code.isStatementStart());
>             }
>         }
>         checkDimension(tree.pos(), v.type);
>     }
>
>
> As that is what's updating the compiler internal state. In principle, 
> if a variable holds a constant value, then javac will always load that 
> constant value instead of loading the variable, see Gen::gnExpr:
>
> if (tree.type.constValue() != null) {
>       // Short circuit any expressions which are constants
>       tree.accept(classReferenceVisitor);
>       checkStringConstant(tree.pos(), tree.type.constValue());
>       result = items.makeImmediateItem(tree.type, 
> tree.type.constValue());
> } else {
>       this.pt = pt;
>       tree.accept(this);
> }
>
> But maybe there are other cases/invariants that would be broken by 
> skipping the Code::newLocal call entirely - this means we'll have to 
> do some experiments with the code in order to rectify this. As you can 
> see, this problem is a general problem with constant folding, and is 
> not something specific to the Amber condy-folding branch (although I 
> agree that the new support magnifies the problem quite a bit).

I will be doing some experiments to check if there are side effects from 
removing the call to Code::newLocal

>
> Cheers
> Maurizio

Vicente

>>
>> On 9/2/2018 1:58 PM, Remi Forax wrote:
>>> Hi all,
>>> while trying to reproduce a bug in ASM using the constant API + 
>>> Intrinsics.ldc, i've spot a discrepancy,
>>> using the follwing example,
>>>
>>> package jdk12;
>>>
>>> import java.lang.constant.ClassDesc;
>>> import java.lang.constant.DirectMethodHandleDesc.Kind;
>>> import java.lang.constant.DynamicConstantDesc;
>>> import java.lang.constant.MethodHandleDesc;
>>> import java.lang.constant.MethodTypeDesc;
>>> import java.lang.invoke.Intrinsics;
>>> import java.lang.invoke.MethodHandles.Lookup;
>>>
>>> public class ConstantDynamicExample {
>>>    public static long primitiveExample() {
>>>      MethodTypeDesc methodDescriptor = 
>>> MethodTypeDesc.ofDescriptor("(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/Class;)J");
>>>      DynamicConstantDesc<Long> dynamicConstant = 
>>> DynamicConstantDesc.of(
>>>          MethodHandleDesc.of(Kind.STATIC, 
>>> ClassDesc.of("jdk12.ConstantDynamicExample"), "bsm", 
>>> methodDescriptor));
>>>      return Intrinsics.ldc(dynamicConstant);
>>>    }
>>>
>>>    private static long bsm(Lookup lookup, String name, Class<?> type) {
>>>      return 3L;
>>>    }
>>>       public static void main(String[] args) {
>>>      System.out.println(primitiveExample());
>>>    }
>>> }
>>>
>>>
>>> Using javap, you can see that the dynamicConstant is folded but the 
>>> number of local variable (locals=2) still count the variables that 
>>> doesn't exist anymore
>>>
>>>   public static long primitiveExample();
>>>      descriptor: ()J
>>>      flags: (0x0009) ACC_PUBLIC, ACC_STATIC
>>>      Code:
>>>        stack=2, locals=2, args_size=0
>>>           0: ldc2_w        #2                  // Dynamic #0:_:J
>>>           3: lreturn
>>>        LineNumberTable:
>>>          line 43: 0
>>>
>>> regards,
>>> Rémi
>>
>