<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body dir="auto"><div>Is there going to be any issue with keeping norecurse in the short term, cutting 3.8, then removing that attribute?</div><div id="AppleMailSignature"><br></div><div id="AppleMailSignature">That is, can we even remove an attribute or are we stuck with it forever because we support old bitcode? Guess it depends on whether it can be auto upgraded or not. Just worth a thought if it's worth trying to fix this before a new branch is cut.</div><div id="AppleMailSignature"><br></div><div id="AppleMailSignature">Cheers</div><div id="AppleMailSignature">Pete<br><br>Sent from my iPhone</div><div><br>On Dec 22, 2015, at 7:03 PM, Philip Reames via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br><br></div><blockquote type="cite"><div>
  
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  
  
    I agree w/James perspective here.  In particular, I feel like the
    issue with norecurse is how we chose to specify it and not the
    notion itself or the implementation thereof.  We should raise some
    of the issues on llvm-dev and hammer through them, but I don't think
    we need to revert anything in the meantime.  This doesn't feel like
    a major problem or the fundamentally wrong approach.<br>
    <br>
    Philip<br>
    <br>
    <div class="moz-cite-prefix">On 12/21/2015 11:51 AM, James Molloy
      via llvm-commits wrote:<br>
    </div>
    <blockquote cite="mid:CALCTSA3BpXKn9CgH8nWoiQF5FA2C2r3wmAq0nbkX-y1ERua4YA@mail.gmail.com" type="cite">Hey Chandler,<br>
      <br>
      I'll think this through and give a more reasoned reply in the
      morning. However I would just say that I'm not opposed to an
      alternative implementation that meets the same ends. <br>
      <br>
      I did put a lot of effort into this current solution though and it
      is *significantly* less broken than what was in LLVM before (main
      is non recursive, unconditionally!!). It currently gives
      nontrivial speed ups to workloads we care about, so ripping it all
      out before designing something new (likely taking at least a month
      given the season) isn't massively appealing.<br>
      <br>
      Cheers,<br>
      <br>
      James<br>
      <div class="gmail_quote">
        <div dir="ltr">On Mon, 21 Dec 2015 at 19:40, Chandler Carruth
          <<a moz-do-not-send="true" href="mailto:chandlerc@gmail.com">chandlerc@gmail.com</a>>
          wrote:<br>
        </div>
        <blockquote class="gmail_quote" style="margin:0 0 0
          .8ex;border-left:1px #ccc solid;padding-left:1ex">
          <div dir="ltr">I'm getting the strong indication that
            norecurse isn't actually a good idea, and hasn't been
            hammered through enough.
            <div><br>
            </div>
            <div>It turns out that I also need to build brand new
              infrastructure to keep supporting it.</div>
            <div><br>
            </div>
            <div>I'm ripping it out of this patch clearly, but I am
              wondering whether we want to rip it out entirely and start
              a new discussion on the actual problem James is trying to
              solve and the best way to go about it.</div>
            <div><br>
            </div>
            <div>Note that, for example, if strlen and memcpy can't be
              marked as norecurse (and I agree that they can't in a
              strict sense as it is conforming to implement both of them
              as recursive algorithms) then *any* inference of norecurse
              in LLVM is actually broken today because we may add calls
              to non-norecurse functions (namely memcpy) at a later
              point. Given that, I think the spec for norecurse is too
              narrow to be a terribly useful property. I suspect that
              James will need to use some other strategy to get the
              useful optimizations he's after (which center around
              *main* not being recursive, a very different beast).</div>
          </div>
          <div dir="ltr">
            <div><br>
            </div>
            <div>-Chandler</div>
          </div>
          <br>
          <div class="gmail_quote">
            <div dir="ltr">On Mon, Dec 21, 2015 at 10:09 AM Mehdi Amini
              <<a moz-do-not-send="true" href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>>
              wrote:<br>
            </div>
            <blockquote class="gmail_quote" style="margin:0 0 0
              .8ex;border-left:1px #ccc solid;padding-left:1ex">
              <div dir="auto">
                <div><br>
                  <br>
                  Sent from my iPhone</div>
              </div>
              <div dir="auto">
                <div><br>
                  On Dec 21, 2015, at 12:07 AM, James Molloy <<a moz-do-not-send="true" href="mailto:james@jamesmolloy.co.uk" target="_blank"></a><a class="moz-txt-link-abbreviated" href="mailto:james@jamesmolloy.co.uk">james@jamesmolloy.co.uk</a>>
                  wrote:<br>
                  <br>
                </div>
                <blockquote type="cite">
                  <div>
                    <div dir="ltr">Hi Mehdi,
                      <div><br>
                      </div>
                      <div>> For strlen: you’re right that it is not
                        correct in the absolute, but for what we use the
                        information I’m not sure it matters. We’re
                        interested in the fact that it won’t call back
                        in the user code in any way, so that we can
                        infer the norecurse on the callers.<br>
                      </div>
                      <div><br>
                      </div>
                      <div>I understand where you're coming from, but I
                        think second-guessing how an attribute may be
                        used is a dangerous route to go down.
                        "norecurse" could be used for something else in
                        the future, so unless we have a good reason it
                        would be good not to make assumptions.</div>
                      <div><br>
                      </div>
                    </div>
                  </div>
                </blockquote>
                <div><br>
                </div>
              </div>
              <div dir="auto">
                <div>Agree, but if we go on the "safe way" now it means
                  we should either redefine the norecurse attribute
                  (like the almostreadnone for example) or have another
                  one.</div>
              </div>
              <div dir="auto">
                <div><br>
                </div>
                <div>Mehdi</div>
              </div>
              <div dir="auto">
                <div><br>
                </div>
                <div><br>
                </div>
                <br>
                <blockquote type="cite">
                  <div>
                    <div dir="ltr">
                      <div>James</div>
                    </div>
                    <br>
                    <div class="gmail_quote">
                      <div dir="ltr">On Mon, 21 Dec 2015 at 10:06 James
                        Molloy via llvm-commits <<a moz-do-not-send="true" href="mailto:llvm-commits@lists.llvm.org" target="_blank"></a><a class="moz-txt-link-abbreviated" href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>>
                        wrote:<br>
                      </div>
                      <blockquote class="gmail_quote" style="margin:0 0
                        0 .8ex;border-left:1px #ccc
                        solid;padding-left:1ex">jmolloy accepted this
                        revision.<br>
                        jmolloy added a comment.<br>
                        This revision is now accepted and ready to land.<br>
                        <br>
                        Hi Chandler,<br>
                        <br>
                        Thanks for taking the time to do this.<br>
                        <br>
                        I agree with Philip - while I like that you've
                        added extra norecurse attributes, I don't like
                        that it's smuggled in as part of this patch. I'd
                        highly prefer this patch to be NFC and then a
                        simple update patch adding the norecurse
                        attributes.<br>
                        <br>
                        Apart from that and the specific comment below,
                        it LGTM.<br>
                        <br>
                        Cheers,<br>
                        <br>
                        James<br>
                        <br>
                        <br>
                        ================<br>
                        Comment at:
                        include/llvm/Transforms/IPO/InferFunctionAttrs.h:25<br>
                        @@ +24,3 @@<br>
                        +/// A pass which infers function attributes
                        from the names and signatures of<br>
                        +/// function declarations in a module..<br>
                        +class InferFunctionAttrsPass {<br>
                        ----------------<br>
                        Double period.<br>
                        <br>
                        ================<br>
                        Comment at:
                        include/llvm/Transforms/IPO/InferFunctionAttrs.h:26<br>
                        @@ +25,3 @@<br>
                        +/// function declarations in a module..<br>
                        +class InferFunctionAttrsPass {<br>
                        +public:<br>
                        ----------------<br>
                        Unrelated: Damn, new style passes look so much
                        nicer!<br>
                        <br>
                        <br>
                        <a moz-do-not-send="true" href="http://reviews.llvm.org/D15676" rel="noreferrer" target="_blank">http://reviews.llvm.org/D15676</a><br>
                        <br>
                        <br>
                        <br>
                        _______________________________________________<br>
                        llvm-commits mailing list<br>
                        <a moz-do-not-send="true" href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
                        <a moz-do-not-send="true" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
                      </blockquote>
                    </div>
                  </div>
                </blockquote>
              </div>
            </blockquote>
          </div>
        </blockquote>
      </div>
      <br>
      <fieldset class="mimeAttachmentHeader"></fieldset>
      <br>
      <pre wrap="">_______________________________________________
llvm-commits mailing list
<a class="moz-txt-link-abbreviated" href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>
<a class="moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a>
</pre>
    </blockquote>
    <br>
  

</div></blockquote><blockquote type="cite"><div><span>_______________________________________________</span><br><span>llvm-commits mailing list</span><br><span><a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a></span><br><span><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a></span><br></div></blockquote></body></html>