[llvm] r332452 - [SimplifyLibcalls] Replace locked IO with unlocked IO
Friedman, Eli via llvm-commits
llvm-commits at lists.llvm.org
Thu May 17 15:09:07 PDT 2018
On 5/17/2018 2:51 PM, Eric Christopher wrote:
>
>
> On Wed, May 16, 2018 at 3:09 PM Friedman, Eli via llvm-commits
> <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>
> On 5/16/2018 2:54 PM, Benjamin Kramer via llvm-commits wrote:
> > This triggered a really annoying miscompile (see r332531), and
> it's even
> > present in the test case. Please double check your tests,
> especially when
> > using update_test_checks.py.
> >
> > Also you want to get these things reviewed by someone who's actually
> > familiar with the LLVM before they land.
>
> I looked at the patch multiple times, and never spotted this issue.
> Granted, maybe I should have looked a little more carefully at
> that part
> of the patch (I was more focused on the actual transform).
>
>
> 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).
>
> -eric
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.)
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.
-Eli
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180517/5de4cf32/attachment.html>
More information about the llvm-commits
mailing list