[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]

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? I vote for review after commit.

Evan

> 
> -Chris
> 
> 
>> 
>> Thanks,
>> 
>> Evan
>> 
>> On Nov 12, 2012, at 11:40 AM, Eli Friedman <eli.friedman at gmail.com> wrote:
>> 
>>> On Mon, Nov 12, 2012 at 11:34 AM, Shuxin Yang <shuxin.llvm at gmail.com> wrote:
>>>> Author: shuxin_yang
>>>> Date: Mon Nov 12 13:34:11 2012
>>>> New Revision: 167740
>>>> 
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=167740&view=rev
>>>> Log:
>>>> This change is to fix rdar://12571717 which is about assertion in Reassociate pass.
>>>> 
>>>> The assertion is trigged when the Reassociater tries to transform expression
>>>>   ... + 2 * n * 3 + 2 * m + ...
>>>> into:
>>>>   ... + 2 * (n*3 + m).
>>>> 
>>>> In the process of the transformation, a helper routine folds the constant 2*3 into 6,
>>>> confusing optimizer which is trying the to eliminate the common factor 2, and cannot
>>>> find 2 any more.
>>>> 
>>>> Review is pending. But I'd like commit first in order to help those who are waiting
>>>> for this fix.
>>> 
>>> No; this is not how our review system works; you're not allowed to
>>> commit without review just because some particular customer needs a
>>> fix.  We have private branches for that sort of thing.
>>> 
>>> -Eli
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>