RFR(L): 8205044: [lworld] Interpreter and compiler support for acmp with value type operands (original) (raw)

John Rose john.r.rose at oracle.com
Thu Jul 12 07:35:47 UTC 2018

I was liking it all the way to the last file, especially the part that went "speculate, speculate, speculate"–truly a sentiment for our times. When I got to the last file, with CmpLNode::Ideal, I noticed that it seems to maybe work correctly for the particular graph shapes created elsewhere, but not considered in isolation. Note that there is no reference to in(2), only in(1)->in([1 or 2]). It can't be correct, in general, to execute the non-null returns, if you didn't inspect in(2). I think you need more pattern-matching here; it's too eager to get to the win as coded.

I reviewed it and approve (heartily) of all of it, except for the above misgiving.

— John

On Jul 11, 2018, at 6:43 AM, Tobias Hartmann <tobias.hartmann at oracle.com> wrote:

Hi Roland, thanks for the review! On 11.07.2018 15:09, Roland Westrelin wrote: In compile.cpp:

4652 // Return constant false because one operand is a non-null value type 4653 return new CmpINode(phase->intcon(0), phase->intcon(1)); there must be a better way to return always false. Yes, here's the incremental webrev: http://cr.openjdk.java.net/~thartmann/8205044/webrev.01inc/ How is the change in callGenerator.cpp related to acmp? It's not related to acmp but I found the problem during testing with this patch. If we late inline a method handle linkTo* call, the return value might be a ValueTypeNode although we are expecting an oop. We need to allocate before we can replace the call. Thanks, Tobias

More information about the valhalla-dev mailing list