[llvm-commits] [PATCH][SOPT] Fix assertion in Reassociate

Chandler Carruth chandlerc at google.com
Thu Nov 15 10:51:14 PST 2012


On Thu, Nov 15, 2012 at 10:41 AM, Shuxin Yang <shuxin.llvm at gmail.com> wrote:
>    2) not only fix (you may think it is merely sidestep) the problem.
>       It include the enhancement to the worklist, making it possible to go
> through the worklist in forward order.

I've tried to explain this before -- it is possible to walk worklists
based on SetVector in forward order. We don't need a new abstraction
for this.

>       It also remove the cost the erasing element in the middle of the
> container.
>       It also include the optimization for "t = -a; t2 = t * 7 (where t has
> multiple uses)" => t2 = a * -7"
>
>     How about all this changes?
>
>    Down the load, please let us know if you want to own the Reassociate.cpp
> and have  *EXCLUSIVELY" write-permission or not.

Clearly Duncan doesn't have exclusive write permission -- no one does.
He was just fixing the root cause of the issue, and felt that the root
cause should be fixed (at least initially) rather than avoiding the
crash without fixing the root cause. I think that's a reasonable
stance to take as a reviewer in general.



More information about the llvm-commits mailing list