[llvm] r332452 - [SimplifyLibcalls] Replace locked IO with unlocked IO

[Friedman, Eli via llvm-commits]

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>