thank you, Claes, Vladimir, and Paul, for your reviews. See
http://cr.openjdk.java.net/~mhaupt/8143211/webrev.01/ for the next round.
The difference to the previous version is, roughly, as follows. Rev 01 ...
... honours the possibility of there being more than one loop in a LambdaForm;
InvokerBytecodeGenerator now has extra state summarising characteristics of the
loops, while the concrete types are extracted from the LambdaForm during the
actual compilation step.
... does away with the extra intrinsic data for loops. Instead, the
LambdaFormEditor is used to create (and cache) an appropriate LambdaForm as a
specialisation of a template. This was a very good suggestion from Vladimir
(thanks!), and it makes the code much more tidy in my opinion.
I agree it looks better now. But I think LF structure can be improved
further.
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.
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.
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().
... contains various small fixes that were suggested.
Things I think should be expressed in other issues rather than being
piggybacked on this one:
* Removing IntrinsicMethodHandle.
* Taking care of Transform cache cleanup.
I'm fine with that.
FTR IntrinsicMH removal was just an idea.
From the design perspective, attaching intrinsic info to MethodHandle
doesn't look optimal. But I haven't done any experiments and don't know
how it the code will look.
Best regards,
Vladimir Ivanov
Am 16.06.2016 um 15:17 schrieb Michael Haupt <michael.ha...@oracle.com>:
Dear all,
please review this change.
RFE: https://bugs.openjdk.java.net/browse/JDK-8143211
Webrev: http://cr.openjdk.java.net/~mhaupt/8143211/webrev.00/
The change puts the tryFinally and loop method handle combinator
implementations, which were introduced as part of the JEP 274 effort, on a
LambdaForm basis, which executes in bytecode generating (default) and
LambdaForm interpretation
(-Djava.lang.invoke.MethodHandle.COMPILE_THRESHOLD=-1) modes. It also changes
the output formatting of LambdaForms, introducing a (hopefully) more readable
format.
Thanks,
Michael
