8183232: Avoid resolving method_kind in AbstractInterpreter::can_be_compiled

[Nils Eliasson]

Since you included hotspot-compiler-dev:

Reviewed +1.

Compiling intrinsics stand-alone should be a part of the future work in 
JDK-8183232.

Best regards,

Nils


On 2017-06-30 14:27, Doerr, Martin wrote:
> Hi Claes,
>
> you can consider my mail as review. It'd only be nice to improve comments a little bit. The intention behind can_be_compiled() is not really clear from the comments.
> I'd vote for pushing your change as it is and rewrite can_be_compiled() in the larger scope of JDK-8183232.
>
> Best regards,
> Martin
>
>
> -----Original Message-----
> From: Claes Redestad [mailto:claes.redestad at oracle.com]
> Sent: Freitag, 30. Juni 2017 14:13
> To: Doerr, Martin <martin.doerr at sap.com>; hotspot-runtime-dev at openjdk.java.net; 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-dev at openjdk.java.net>
> Subject: Re: 8183232: Avoid resolving method_kind in AbstractInterpreter::can_be_compiled
>
> Hi Martin,
>
> On 06/30/2017 12:14 PM, Doerr, Martin wrote:
>> Hi Claes,
>>
>> adding hotspot-compiler mailing list since it is JIT related.
>>
>> First of all, thanks for addressing this performance issue. I think it makes sense to avoid method resolution (method_kind()).
>>
>> I'm basically ok with the change, but I'd like to share some thoughts.
> thanks for reviewing(?)!
>
>> I think the intention of can_be_compiled() is to prevent compilation of floating point related methods in a stand-alone fashion. The C2 compiler wouldn't use intrinsics in this case. When e.g. C2 inlines the same method into some caller, the intrinsics will be used and may result in a slightly different floating point computation as the stand-alone compiled method (for non-strict math). So the purpose is to enforce consistent computation between interpreter and all compiled methods.
> Right.
>
>> Your current proposal just disables stand-alone computation for all math intrinsics. This still serves the purpose. I guess it wouldn't even really hurt, to prevent stand-alone compilation of intrinsics in general.
>> But it seems like the design goal was to prevent stand-alone compilation of exactly these functions, which have special compiler intrinsics. This is platform dependent.
>> I'm not requesting a change, but I just want to mention that the change kind of breaks this design.
> Shall I count this as a vote in favor of keeping the platform specific
> implementations of can_be_compiled? I have no strong opinion against
> reverting back to that, just that there'll be more places to change
> going forwards. It just seemed to serve little practical purpose on the
> set of platforms I've tested right now.
>
> /Claes
>
>> Please also note, that there's already an open bug to improve math intrinsics related logic:
>> https://bugs.openjdk.java.net/browse/JDK-8170693
>> Maybe can_be_compiled() can get replaced in that context.
>>
>> Best regards,
>> Martin
>>
>>
>> -----Original Message-----
>> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-bounces at openjdk.java.net] On Behalf Of Claes Redestad
>> Sent: Donnerstag, 29. Juni 2017 18:13
>> To: hotspot-runtime-dev at openjdk.java.net
>> Subject: RFR: 8183232: Avoid resolving method_kind in AbstractInterpreter::can_be_compiled
>>
>> Hi all,
>>
>> it appears profitable for startup to simplify
>> AbstractInterpreter::can_be_compiled
>> to use methodHandle->intrinsic_id() rather than take a detour through
>> method_kind.
>>
>> webrev: http://cr.openjdk.java.net/~redestad/8183232/hotspot.00/
>> bug: https://bugs.openjdk.java.net/browse/JDK-8183232
>>
>> Note: Having platform-dependent implementations for can_be_compiled
>> seems excessive,
>> since if the intrinsic isn't implemented the corresponding case will
>> never trigger anyway.
>> No regression can be detected on platforms that doesn't have certain
>> intrinsics, such as
>> SPARC, but if there's preference I can revert to having
>> platform-dependent methods.
>>
>> Testing: JPRT
>>
>> Thanks!
>>
>> /Claes