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

[John Rose]

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.01_inc/
> 
>> 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