[patch] simplifyCFG for review

Chandler Carruth chandlerc at google.com
Fri Jul 26 23:49:09 PDT 2013


On Fri, Jul 26, 2013 at 11:32 PM, Ye, Mei <Mei.Ye at amd.com> wrote:

> Hi Chandler****
>
> ** **
>
> Is there a protocol (guideline) on code review process?   I have done my
> due diligence on resolving all issues from code reviewers.   And I believe
> I did not mis-interpret any of their opinions.
>

Some, but not much. But you didn't do anything wrong here, and I don't have
a problem with anything you or Tom did -- it clearly was reviewed.

The reason I asked for a tiny part of it to be undone is because it doesn't
do what you or the reviewers seemed to think it does, and could cause
significant problems. There's nothing wrong here, we revert stuff all the
time when it doesn't work out. =]

  It will be more constructive and productive if you gave your opinions
> during the code review process.
>

We can't all review all the patches. This patch seemed to be getting good
reviews, and so I didn't look at it.

I try to read almost all of the actual commits (I still miss some to my
regret), and noticed it then. I've tried hard not to nit pick or otherwise
complain about the details of the patch even though I might have in
standard code review because I *didn't* do the review, and so I have to
leave that to others.

However, when I or anyone else notices fundamental problems, before or
after a commit, it is critical to bring them up and discuss them. I've
written code that 4 and 5 people reviewed and thought was correct, but one
careful reviewer noted something that all of us had looked past and missed.
This is what keeps the code quality of LLVM high.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130726/eee96e41/attachment.html>


More information about the llvm-commits mailing list