[llvm-commits] [llvm] r167740 - in /llvm/trunk: lib/Transforms/Scalar/Reassociate.cpp test/Transforms/Reassociate/mul_neg.ll test/Transforms/Reassociate/multistep.ll

Evan Cheng evan.cheng at apple.com
Mon Nov 12 19:12:25 PST 2012


On Nov 12, 2012, at 7:00 PM, Chandler Carruth <chandlerc at google.com> wrote:

> On Mon, Nov 12, 2012 at 6:55 PM, Evan Cheng <evan.cheng at apple.com> wrote:
>> 
>> On Nov 12, 2012, at 3:51 PM, Chris Lattner <clattner at apple.com> wrote:
>> 
>>> 
>>> On Nov 12, 2012, at 1:08 PM, Evan Cheng <evan.cheng at apple.com> wrote:
>>> 
>>>> Hi Eli,
>>>> 
>>>> The approach seems sound to me. Since no one has raised design concerns at this point and the fact this is a a bug fix for a crash, I think it's appropriate for review after commit.
>>> 
>>> This patch isn't "obvious", and Shuxin is still new to this part of the code.  Unless it's been reviewed it shouldn't go in.
>> 
>> The patch was submitted for review back on 11/5 and there were some discussions on 11/6 and 11/7. There were no additional comments since then. Should it continue to sit in review limbo?
> 
> The standard response, and what everyone else in this position does
> (and there are *many*...) is to ping, asking for an update.

Which is why I said the approach seems fine to me (and I looked at the patch and commented on it back on 11/6). Since no one has any concrete suggestion it can be taken as LGTM. If anyone has other suggestions, they can be done after commit.

Evan



More information about the llvm-commits mailing list