<html>
  <head>
    <meta content="text/html; charset=windows-1252"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    sorry, missed that. <br>
    <br>
    Here is the a new one, with Johns suggestion too.<br>
    <br>
    <a
      href="http://cr.openjdk.java.net/%7Eneliasso/6658428.01/webrev.04/">http://cr.openjdk.java.net/~neliasso/6658428.01/webrev.04/</a><br>
    <br>
    //N<br>
    <br>
    Vladimir Kozlov skrev 2012-02-24 18:23:
    <blockquote cite="mid:4F47C785.7040902@oracle.com" type="cite">Looks
      good.
      <br>
      <br>
      I asked before to move msg definition under the following check
      where it is used:
      <br>
      <br>
      + if (PrintIntrinsics || PrintInlining NOT_PRODUCT( ||
      PrintOptoInlining) ) {
      <br>
       if (jvms-&gt;has_method()) {
      <br>
      + const char* msg = is_virtual() ? "failed to inline
      (intrinsic, virtual)" : "failed to inline (intrinsic)";
      <br>
      <br>
      Thanks,
      <br>
      Vladimir
      <br>
      <br>
      On 2/24/12 1:48 AM, Nils Eliasson wrote:
      <br>
      <blockquote type="cite">+1 for restructuring. A common table with
        intrinsic_id, matcher and expander. Perhaps even c1/c2 agnostic.
        <br>
        <br>
        Also noticed that there are no popcount intrinsic for c1. Adding
        that to my learning c1 task list.
        <br>
        <br>
        New webrev here:
        <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~neliasso/6658428.01/webrev.03/">http://cr.openjdk.java.net/~neliasso/6658428.01/webrev.03/</a>
        <br>
<a class="moz-txt-link-rfc2396E" href="http://cr.openjdk.java.net/%7Eneliasso/6658428.01/webrev.03/">&lt;http://cr.openjdk.java.net/%7Eneliasso/6658428.01/webrev.03/&gt;</a>
        <br>
        <br>
        Thanks!
        <br>
        <br>
        //N
        <br>
        <br>
        Tom Rodriguez skrev 2012-02-22 19:36:
        <br>
        <blockquote type="cite">On Feb 22, 2012, at 5:27 AM, Nils
          Eliasson wrote:
          <br>
          <br>
          <blockquote type="cite">Thanks Vladimir and Tom for the
            feedback,
            <br>
            <br>
            I agree with removing the out-of-line part.
            <br>
            <br>
            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.
            <br>
          </blockquote>
          Actually what I meant was to propagate the appropriate
          has_match_rule test up into make_vm_intrinsic. So something
          more like:
          <br>
          <br>
           case vmIntrinsics::_bitCount_i:
          <br>
           if (!Matcher::has_match_rule(Op_PopCountI)) return NULL;
          <br>
           break;
          <br>
           case vmIntrinsics::_bitCount_l:
          <br>
           if (!Matcher::has_match_rule(Op_PopCountL)) return NULL;
          <br>
           break;
          <br>
           case vmIntrinsics::_numberOfLeadingZeros_i:
          <br>
           if
          (!Matcher::match_rule_supported(Op_CountLeadingZerosI)))
          return NULL;
          <br>
           break;
          <br>
           
          <br>
          <br>
          I think that will take into account the various flags properly
          without having extra ifdefs.
          <br>
          <br>
          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.
          <br>
          <br>
          The other changes look fine.
          <br>
          <br>
          tom
          <br>
          <br>
          <blockquote type="cite"><a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~neliasso/6658428.01/webrev.01/">http://cr.openjdk.java.net/~neliasso/6658428.01/webrev.01/</a>
            <br>
            <br>
            Skiping reviewboard for now, doesn't seem to work well with
            mq and cygwin.
            <br>
            <br>
            //N
            <br>
            <br>
            Tom Rodriguez skrev 2012-02-21 20:05:
            <br>
            <blockquote type="cite">On Feb 21, 2012, at 2:26 AM, Nils
              Eliasson wrote:
              <br>
              <br>
              <br>
              <blockquote type="cite"><a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~neliasso/6658428.01/webrev.00/">http://cr.openjdk.java.net/~neliasso/6658428.01/webrev.00/</a>
                <br>
                <br>
                <br>
                6658428: C2 doesn't inline java method if corresponding
                intrinsic failed to inline.
                <br>
                <br>
                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.
                <br>
                <br>
              </blockquote>
              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.
              <br>
              <br>
              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.
              <br>
              <br>
              tom
              <br>
              <br>
              <br>
              <blockquote type="cite">And I will need someone to submit
                it.
                <br>
                <br>
                Thanks,
                <br>
                Nils E.
                <br>
                <br>
                <br>
              </blockquote>
            </blockquote>
            <br>
          </blockquote>
        </blockquote>
        <br>
      </blockquote>
    </blockquote>
    <br>
    <br>
    <div class="moz-signature">-- <br>
      <a href="http://www.oracle.com" target="_blank"><img
          src="cid:part1.09010104.05020501@oracle.com" alt="Oracle"
          border="0" height="26" width="114"></a><br>
      <font color="#666666" face="Verdana, Arial, Helvetica, sans-serif"
        size="1">Nils Eliasson | Senior Member of Technical Staff<br>
        <font color="#ff0000">Oracle</font> Java Platform Group, JVM
        Engineering<br>
        ORACLE Sweden </font><br>
      <br>
      <!-- This signature was generated by the MyDesktop Oracle Business Signature utility version 3.8.8 -->
    </div>
  </body>
</html>