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

[Shuxin Yang]

I apologize for the chaos.  I thought now that you are reviewing the 
change,  if I did something wrong, we will spot the problem before too 
late.
This bug report has high priority, I hope you have sometime to review it 
this week.

Although the patch is not tiny, large percentage of the change is about 
the worklist container and testing cases. The rest change is quite
straightforward.

On 11/13/12 12:21 AM, Duncan Sands wrote:
> Hi Evan,
>
>> 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.
>
> I told Shuxin Yang that I would take a look at it this week.  I was 
> disappointed
> to see it was applied anyway, without review.
>
> Ciao, Duncan.
>
>>
>> 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
>>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits