RFR(L): 8205044: [lworld] Interpreter and compiler support for acmp with value type operands
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.
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:
>> 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.
More information about the valhalla-dev