<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body>
    <font size="4"><font face="monospace">Thanks again for the
        thoughtful feedback.  <br>
      </font></font><br>
    <blockquote type="cite" cite="mid:CAEYHehjY0iz1V_1H-nbtJOXcK2mwmJ=4wS0TSKVv92fzV9AKSw@mail.gmail.com">
      <div dir="ltr">Because of the high density<br>
        of instanceof checks on instruction types, this was the one
        place<br>
        where I wished for shorter class names.<br>
      </div>
    </blockquote>
    <br>
    We did consider a few alternatives to the XxxInstruction convention,
    such as XxxOp.  I'm open to rationalizing names at this point; the
    current conventions were chosen early on to be clear and consistent,
    so we could move on to actually implementing the right idioms, but
    it is reasonable to circle back and question naming (for some
    bounded time) now that the shape of the API is more settled.  <br>
    <br>
    (Irritating confounding factor: if you name the builder methods
    load(), store(), etc, when you get to "return" and "instanceof", you
    have a problem...)<br>
    <br>
    <blockquote type="cite" cite="mid:CAEYHehjY0iz1V_1H-nbtJOXcK2mwmJ=4wS0TSKVv92fzV9AKSw@mail.gmail.com">
      <div dir="ltr">Another huge boon is `block()`.  I call it just
        once in my code base,<br>
        but it reduces the work of managing local slots, their
        lifetimes, and<br>
        their debug information by an enormous amount.<br>
      </div>
    </blockquote>
    <br>
    These were added relatively late, especially the local-variable
    management, and we could especially use more feedback (and test
    coverage!) on that.  There are some missing bits here too, since
    `allocateLocal` doesn't automatically generate LVT/LVTT entries, and
    it should.  Feedback and contribution is welcome here.  <br>
    <br>
    <blockquote type="cite" cite="mid:CAEYHehjY0iz1V_1H-nbtJOXcK2mwmJ=4wS0TSKVv92fzV9AKSw@mail.gmail.com">
      <div dir="ltr">With regard to performance, I can only say that it
        is fast.  </div>
    </blockquote>
    <br>
    I'm sure authors of other frameworks will take that as "fighting
    words", but yes, it is likely to be somewhat surprising to many that
    such an allocation-heavy approach offers this degree of "fast
    enough" for most use cases.  <br>
    <br>
    <blockquote type="cite" cite="mid:CAEYHehjY0iz1V_1H-nbtJOXcK2mwmJ=4wS0TSKVv92fzV9AKSw@mail.gmail.com">
      <div dir="ltr">### Lower than I like<br>
        <br>
        The bulk of API usage stays on a single level of abstraction,
        for<br>
        example working with *Desc entry points instead of *Entry. 
        There are<br>
        rare places where this slipped at little.<br>
      </div>
    </blockquote>
    <br>
    I'm not following what you're getting at here.  Do you mean there
    are gaps where we are not consistent about choices of Desc vs
    Entry?  Or simply that you stuck to the Desc level as a user?  And
    if so, was there friction here?<br>
    <br>
    <blockquote type="cite" cite="mid:CAEYHehjY0iz1V_1H-nbtJOXcK2mwmJ=4wS0TSKVv92fzV9AKSw@mail.gmail.com">
      <div dir="ltr">The New*Array family of instructions gave me a hard
        time.  I had<br>
        forgotten that there are three of those, and there was some
        bumping<br>
        around involved while re-learning this fact.  (Btw, anewarray()
        accepts<br>
        a primitive ClassDesc and converts it into a reference type,
        e.g. "I"<br>
        to "LI;".)  Two of the instructions have no ClassDesc factory,
        which<br>
        meant I had to wrap the family into a intermediate
        representation node<br>
        to carry essentially (ClassDesc,int) downstream to the
        CodeBuilder<br>
        instance.  I wonder if it is worthwhile to move the three under
        an<br>
        umbrella interface, similar to what ConstantInstruction is
        doing.<br>
      </div>
    </blockquote>
    <br>
    Quite possibly.  These instructions gave us a hard time too, because
    they are so irregular in how they work.  It is quite possible that
    there is a simplification here.  The constant instructions have a
    similar abstraction-resistence, and we did go around a few times
    with how much to lump and split there too.  The main constraint is
    that the lowest level modeling should map directly to the classfile
    spec, which means any creativity around merging representations has
    to happen one level up.  <br>
    <br>
    When designing the groupings of instructions, there are a lot of
    forces pulling in different directions.  The main normalizing force
    is that transformation is an emergent property of reading+writing,
    and we want for the no-op transformation to do as little "lifting
    and lowering" work as possible. This is why no XxxInstruction uses
    XxxDesc in its data model; converting from CP entry -> descriptor
    string -> XxxDesc -> descriptor string -> CP entry for a
    no-op transform is expensive and pointless.  We have to worry about
    this because the default representation is a stream of element; if
    the elements are "too far" from what's in the classfile, then the
    lifting and lowering overhead will eat you.  <br>
    <br>
    If you try to merge semi-related instructions like the constant or
    new-array instructions into a common representation, that
    representation will not be the natural representation for some or
    all of the instructions in the group.  When we hit walls like this,
    we fall back to splitting, and modeling the classfile spec directly,
    but there is probably more we can do atop them.  Suggestions
    welcome.  <br>
    <br>
    <blockquote type="cite" cite="mid:CAEYHehjY0iz1V_1H-nbtJOXcK2mwmJ=4wS0TSKVv92fzV9AKSw@mail.gmail.com">
      <div dir="ltr">The one place where I use labelToBci() is
        try/catch/finally.  There is<br>
        the special case of exceptionCatch() failing for an empty
        region, a<br>
        condition that in turn can lead to handler blocks becoming<br>
        unreachable.  For me, the only robust way to deal with this to
        a)<br>
        guard against an empty region by inspecting the bcis and b)<br>
        subsequently omitting the invalid/unreachable parts.<br>
      </div>
    </blockquote>
    <br>
    This raises a good question about how much the library wants to do
    to "fix" questionable bytecode.  We already NOP out unreachable
    bytecode (otherwise the verifier freaks out).  Should we just
    silently drop catch clauses associated with empty try blocks?  (We
    won't know that they are empty until after all the labels are
    resolved, so we can't usually detect this at the point of emitting
    the catch entry.)  What about when, by the time we get to the end of
    generation, a label used in a try-catch, or LVT[T], isn't bound? 
    Should we throw, or just drop the entry?  I suspect one size does
    not fit all here and we have to design some more options-handling. 
    <br>
    <br>
    <blockquote type="cite" cite="mid:CAEYHehjY0iz1V_1H-nbtJOXcK2mwmJ=4wS0TSKVv92fzV9AKSw@mail.gmail.com">
      <div dir="ltr">Another single use only is constantPool(), to go
        from a DMHD instance<br>
        to CodeBuilder's (field|invoke)Instruction.  This was a
        consequence of<br>
        DMHD only providing the lookupDescriptor() as String and not as
        an MTD<br>
        as well.  With hindsight, it may have been better for me to
        recover<br>
        the MTD from the String regardless, and to stay on the level of
        *Desc<br>
        throughout.<br>
      </div>
    </blockquote>
    <br>
    Is there something missing that would bridge that for you?<br>
    <br>
    <blockquote type="cite" cite="mid:CAEYHehjY0iz1V_1H-nbtJOXcK2mwmJ=4wS0TSKVv92fzV9AKSw@mail.gmail.com">
      <div dir="ltr">Finally, is there a way to decide between
        tableswitch and<br>
        lookupswitch?  Lacking something better, I'm trying to emulate
        this<br>
        code here:<br>
        <a href="https://github.com/openjdk/jdk-sandbox/blob/master/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java#L1320" moz-do-not-send="true" class="moz-txt-link-freetext">https://github.com/openjdk/jdk-sandbox/blob/master/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java#L1320</a><br>
      </div>
    </blockquote>
    <br>
    Most compilers use a heuristic based on the size of the two
    alternatives, comparing `(hi-lo) / count` to some density
    threshold.  You're mostly optimizing for bytecode size here, since
    when the JIT gets its hands on it, it has its own heuristics.  <br>
    <br>
    <blockquote type="cite" cite="mid:CAEYHehjY0iz1V_1H-nbtJOXcK2mwmJ=4wS0TSKVv92fzV9AKSw@mail.gmail.com">
      <div dir="ltr">### Lost in translation<br>
        <br>
        One feature I cannot duplicate with Classfile is
        try/catch/finally in<br>
        expression position when the operand stack is not empty.  The
        old<br>
        bytecode generator dealt with this case by unwinding the operand
        stack<br>
        into locals, evaluating the t/c/f, and then rebuilding the
        operand<br>
        stack with the result on top.  But to do this, one needs to know
        what<br>
        the operand stack looks like at the point of the `try`.<br>
      </div>
    </blockquote>
    <br>
    Interesting point.  We do not build stack maps as we go, so we don't
    have our hands readily on the stack state.  However, I could imagine
    an overload of the try-catch builder that would let you feed it a
    TypeKind[], and that would use allocateLocal to automate the
    push/pop logic.  This is something you should be able to build from
    outside the library, too; this would be a good experiment try try. 
    (You'd have to manually compute the stack state.)<br>
    <br>
    Cheers,<br>
    -Brian<br>
  </body>
</html>