RFR(L): 8143211: provide bytecode intrinsics for loop and try/finally executors

[Vladimir Ivanov]

src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java:

+     * Emit bytecode for the loop idiom.
...
+     * loop=Lambda(a0:L,a1:L,a2:L,a3:L,a4:L,a5:L,a6:L,a7:L)=>{
+     *   t8:L=MethodHandles.constant(String.class, "<init types>");
+     *   t9:L=MethodHandle.invokeBasic(a5:L,a7:L);              // box 
the arguments into an Object[]
+     *   t10:L=MethodHandleImpl.loop(a1:L,a2:L,a3:L,a4:L,t9:L); // call 
the loop executor
+     *   t11:L=MethodHandle.invokeBasic(a6:L,t10:L);t11:L}      // 
unbox the result; return the result

Stale LF shape in the comment.

===

-            if (assertStaticType(cls, n))
-                return;  // this cast was already performed
+            assertStaticType(cls, n);

This change completely defeats localClasses purpose. I'd like to 
understand more why the verifier complains about missing checkcast.

Otherwise, looks really good!

Thanks for your patience when addressing my comments :-)

Best regards,
Vladimir Ivanov

On 7/1/16 2:33 PM, Michael Haupt wrote:
> Hi Vladimir,
>
> thank you once more - your comments led to more improvements. The results are at http://cr.openjdk.java.net/~mhaupt/8143211/webrev.02/, and some details are inlined below.
>
>> Am 29.06.2016 um 17:57 schrieb Vladimir Ivanov <vladimir.x.ivanov at oracle.com>:
>> I'd prefer to see loopStateTypes attached to the intrinsic itself. Right now, it's unrelated and you look it up by offset. You can pass it as an additional parameter into MHI.loop instead and access it directly from the Name marked as the loop intrinsic.
>>
>> LFE.noteLoopLocalTypesForm() looks fragile. It assumes the LF being edited contains only loop intrinsic and adds type info as the first non-argument Name. What if you explicitly pass loop intrinsic position into it, check that the Name represents a loop, and substitute loop types argument.
>>
>> Such representation will support multiple loops inside a single LF and allow per-loop editing.
>
> The loop invoker (MHImpl.loop()) now accepts an initial BasicType[] that captures the loop clause types. The loop LF for a method type is initialised to pass null in this place. For each combination of loop clause types, the LambdaFormEditor will now replace this null with the appropriate BasicType[]. This argument is altogether ignored in LambdaForm interpretation mode, but read and used in bytecode generation in InvokerBytecodeGenerator.emitLoop().
>
> Thereby, the loop clause types, which are crucial for boxing-free bytecode generation, are now associated with (rather: stored in) the LambdaForm representing a particular loop state distribution, but they do not consume extra space in the names array comprising the LambdaForm.
>
>> Regarding loopStarts, loopStateStarts, and currentLoop fields in IBG.
>>
>> Since IBG iterates over the LF sequentially, you should be able to get rid of arrays and convert the position where loop state should be stored into a local variable in IBG.generateCustomizedCodeBytes().
>>
>> The only problem is localsMapSize (which is eagerly allocated).
>> Current eager initialization logic looks over-complicated and I think it doesn't work as expected (mapLoopLocalState() overwrites locals which are used by Names following the loop).
>>
>> I'd prefer suggest to see it simplified.
>>
>> You can just do a single pass over the LF being compiled and sum up slots needed to represent loops state. After that, you can initialize them incrementally during compilation when you encounter a loop intrinsic.
>>
>> If you put loop state after locals used for other LF Names, you don't need to adjust local slot indexes for non-loop Names.
>
> InvokerBytecodeGenerator now adopts a solution where localsMap and localClasses are dynamically resized as loops are encountered in the LambdaForm. Thereby, no additional state about local slot allocation for loops needs to be maintained any more.
>
>> Another thought: what if loop state is so large it overflows 65k locals limit? I don't see any checks of clauses array size in MHs.loop().
>
> This is filed as a bug: https://bugs.openjdk.java.net/browse/JDK-8160717.
>
> Best,
>
> Michael
>