Request for reviews (S): 6658428: C2 doesn't inline java method if corresponding intrinsic failed to inline

[Tom Rodriguez]

On Feb 22, 2012, at 5:27 AM, Nils Eliasson wrote:

> Thanks Vladimir and Tom for the feedback,
> 
> I agree with removing the out-of-line part.
> 
> Removing the UsePopCountInstruction-guard in library_call also makes it nicer and cleaner, but noting that we in a few rare cases create call generators for intrinsics that always return false.

Actually what I meant was to propagate the appropriate has_match_rule test up into make_vm_intrinsic. So something more like:

  case vmIntrinsics::_bitCount_i:
    if (!Matcher::has_match_rule(Op_PopCountI)) return NULL;
    break;
  case vmIntrinsics::_bitCount_l:
    if (!Matcher::has_match_rule(Op_PopCountL)) return NULL;
    break;
  case vmIntrinsics::_numberOfLeadingZeros_i:
    if (!Matcher::match_rule_supported(Op_CountLeadingZerosI))) return NULL;
    break;
  …

I think that will take into account the various flags properly without having extra ifdefs.

There's a more major restructuring of the intrinsics lurking in all this I think, which I'm not advocating you do as part of this change.  It's always seemed confusing to me that make_vm_intrinsics doesn't include all the intrinsics, and the duplication of the static guards like has_match_rule and other globals flags in make_vm_intrinsics and the actual inliner as well seems wasteful.

The other changes look fine.

tom

> 
> http://cr.openjdk.java.net/~neliasso/6658428.01/webrev.01/
> 
> Skiping reviewboard for now, doesn't seem to work well with mq and cygwin.
> 
> //N
> 
> Tom Rodriguez skrev 2012-02-21 20:05:
>> On Feb 21, 2012, at 2:26 AM, Nils Eliasson wrote:
>> 
>> 
>>> http://cr.openjdk.java.net/~neliasso/6658428.01/webrev.00/
>>> 
>>> 
>>> 6658428: C2 doesn't inline java method if corresponding intrinsic failed to inline.
>>> 
>>> Allow intrinsic inline to fallback to plain java inline case. Changing some intrinsics to not be created when they cant be used and adding some print inline changes. 
>>> 
>> I don't think you need to try inline and out of line separately.  I think the old code forced out of line because that was the only way to suppress the use of the intrinsic.  So remove the try_inline = false line and just add the extra false argument to the existing call.  It should use inline if it's appropriate or out of line if not.
>> 
>> In library_call.cpp I think it would be better to stop using !UsePopCountInstruction to guard those intrinsics and use the appropriate Matcher::has_match_rule for each one.  That should take into account the platform dependent meaning of UsePopCountIntstruction.
>> 
>> tom
>> 
>> 
>>> And I will need someone to submit it.
>>> 
>>> Thanks,
>>> Nils E.
>>> 
>>> 
> 
> 
> -- 
> <oracle_sig_logo.gif>
> Nils Eliasson | Senior Member of Technical Staff
> Oracle Java Platform Group, JVM Engineering
> ORACLE Sweden 
>