<div dir="auto">You are right so I would like to apologize you.<div dir="auto"><br></div><div dir="auto">If you revert it, I would be fine too. I do my best to do/fix things in this patch anyway :)</div></div><br><div class="gmail_quote"><div dir="ltr">Dňa pi 18. 5. 2018, 0:09 Friedman, Eli <<a href="mailto:efriedma@codeaurora.org">efriedma@codeaurora.org</a>> napísal(a):<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  
    
  
  <div text="#000000" bgcolor="#FFFFFF">
    <div class="m_-4218521709411879700moz-cite-prefix">On 5/17/2018 2:51 PM, Eric Christopher
      wrote:<br>
    </div>
    <blockquote type="cite">
      <div dir="ltr"><br>
        <br>
        <div class="gmail_quote">
          <div dir="ltr">On Wed, May 16, 2018 at 3:09 PM Friedman, Eli
            via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" rel="noreferrer">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">On
            5/16/2018 2:54 PM, Benjamin Kramer via llvm-commits wrote:<br>
            > This triggered a really annoying miscompile (see
            r332531), and it's even<br>
            > present in the test case. Please double check your
            tests, especially when<br>
            > using update_test_checks.py.<br>
            ><br>
            > Also you want to get these things reviewed by someone
            who's actually<br>
            > familiar with the LLVM before they land.<br>
            <br>
            I looked at the patch multiple times, and never spotted this
            issue. <br>
            Granted, maybe I should have looked a little more carefully
            at that part <br>
            of the patch (I was more focused on the actual transform).<br>
          </blockquote>
          <div><br>
          </div>
          <div>Probably would have helped here if you'd ack'd the patch
            yourself. The only approval on the patch is an anonymous
            person with no llvm history (as far as I can tell).</div>
          <div><br>
          </div>
          <div>-eric <br>
          </div>
        </div>
      </div>
    </blockquote>
    <p>The timeline was something like this: I reviewed it on May 8, and
      gave it sort of tentative LGTM, pending the discussion on
      llvmdev.  The discussion happened on llvmdev, David posted a
      slightly updated path in response to that discussion, and I meant
      to review that, but it sort of sank to the bottom of my inbox, and
      David merged on the 15th (?) without a review on the updated
      version.  (I really shouldn't have let it sit that long, but you
      know how reviews tend to pile up.)<br>
    </p>
    <p>David shouldn't have merged it without an explicit ack from me or
      another qualified reviewer, but I don't think we need to revert at
      this point.<br>
    </p>
    <p>-Eli<br>
    </p>
    <pre class="m_-4218521709411879700moz-signature" cols="72">-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project</pre>
  </div>

</blockquote></div>