[lworld] VerifyError: Bad type on operand stack, limitation in javac for value type constructors

[Srikanth]

On Monday 16 July 2018 06:45 PM, Srikanth wrote:
> Thanks Paul,
>
> This is a tricky issue indeed, but not a hard limitation.
> I have raised: https://bugs.openjdk.java.net/browse/JDK-8207332 for 
> this problem.

Note: I have pushed the fix here: 
http://hg.openjdk.java.net/valhalla/valhalla/rev/54f37cb89cb3

There was both a lowering problem and a code generation problem.
Given:

     x = y = 1234;

Earlier it was being lowered to:

$value = __WithField($value.x, $value = __WithField($value.y, 1234));

which is bogus.

Now we lower the above chained assignment to:

$value = __WithField($value.x, ($value = __WithField($value.y, 1234)).y);

Now the lowering is good, but it complicates code generation due to side 
effects peculiar to values:

For the outer withfield if we *first* load the value type instance onto 
the operand stack and *then*
compute and load the expression ($value = __WithField($value.y, 
1234)).y, we have trouble: Since the value type instance already loaded 
onto the operand stack is the one being modified by the subexpression
($value = __WithField($value.y, 1234)).y.

So the right code generation is to first compute ($value = 
__WithField($value.y, 1234)).y with the
attendant updates to $value (the factory's return value) and *then* load 
the *modified* value.

Only complication is this leaves the withfield instructions operands in 
the reverse order.

This is now fixed by a dup_x1 or dup_x2 as required, followed by a pop 
to get rid of the redundant value.

This slightly convoluted code generation scheme is needed to ensure 
correctness in all cases.

Perhaps some "optimization" at the bytecode level is possible by 
recognizing side effect free subexpressions.

I am not sure if it is worth following up. I'll do if I hear strong 
opinions in favor.

Thanks!
Srikanth


>
> I will tackle the related but simpler problem against
> https://bugs.openjdk.java.net/browse/JDK-8207341
>
> first and then get to JDK-8207332.
>
> Thanks!
> Srikanth
>
> On Friday 13 July 2018 11:54 PM, Paul Sandoz wrote:
>> Hi,
>>
>> The program below compiles but fails when executed:
>>
>>   $ java -XX:+EnableValhalla A
>> Exception in thread "main" java.lang.VerifyError: Bad type on operand 
>> stack
>> Exception Details:
>>    Location:
>>      A$Point.$makeValue$()LA$Point; @12: i2b
>>    Reason:
>>      Type 'A$Point' (current frame, stack[1]) is not assignable to 
>> integer
>>    Current Frame:
>>      bci: @12
>>      flags: { }
>>      locals: { 'A$Point' }
>>      stack: { 'A$Point', 'A$Point' }
>>    Bytecode:
>>      0000000: cb00 024b 2a2a 03cc 0004 594b 91cc 0003
>>      0000010: 4b2a b0
>>
>>     at A.main(A.java:31)
>>
>>
>> The cause is this constructor:
>>
>>      Point() {
>>          x = y = 0; // This is the cause
>>      }
>>
>> the byte code of which is:
>>
>>    A$Point();
>>      descriptor: ()V
>>      flags: (0x0000)
>>      Code:
>>        stack=1, locals=1, args_size=1
>>           0: aload_0
>>           1: invokespecial #1                  // Method 
>> java/lang/Object."<init>":()V
>>           4: return
>>        LineNumberTable:
>>          line 6: 0
>>
>>
>> If the constructor is updated to the following then things work:
>>
>>      Point() {
>>          x = 0;
>>          y = 0;
>>      }
>>
>>    static A$Point point();
>>      descriptor: ()LA$Point;
>>      flags: (0x0008) ACC_STATIC
>>      Code:
>>        stack=2, locals=0, args_size=0
>>           0: iconst_0
>>           1: iconst_0
>>           2: invokestatic  #6                  // Method 
>> point:(II)LA$Point;
>>           5: areturn
>>
>>
>> There is a limitation in javac's current approach mapping field 
>> assignments to arguments passed to the static factory method.
>>
>> Paul.
>>
>> public class A {
>> static final __ByValue class Point {
>>      final int x;
>>      final int y;
>>
>>      Point() {
>>          x = y = 0; // This is the cause
>>      }
>>
>>      static Point point(int x, int y) {
>>          Point p = __MakeDefault Point();
>>          p = __WithField(p.x, x);
>>          p = __WithField(p.y, y);
>>          return p;
>>      }
>>
>>      @Override
>>      public boolean equals(Object o) {
>>          if (!(o instanceof Point)) return false;
>>          Point op = (Point) o;
>>
>>          return x == op.x && y == op.y;
>>      }
>>
>>      static Point point() {
>>          return point(0, 0);
>>      }
>> }
>>
>> public static void main(String[] args) {
>>    Point p = Point.point();
>> }
>> }
>>
>