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

[Chandler Carruth]

On Mon, Nov 12, 2012 at 7:19 PM, Shuxin Yang <shuxin.llvm at gmail.com> wrote:
> I hope each pass has more than one owner. In case one owner is busy, the
> other one could response in
> reasonable short period of time.

Certainly; and a few different folks have tried to respond to your patch. =]

> During the dev-meeting, I heard quite a few complaint about the pailful
> "pings".

Curious, I have never been sad to get pings. Anyways, the way to fix
is to get more reviewers.

> If the code review take more than one week, it basically means the author
> who make the change need
> to re-test from scratch. This cost is reasonable for big change. it is way
> to high for tiny to medium change.
>
> It is also make thing hard for incremental change

Sure, but I wouldn't expect review to be so prolonged for every patch.
=] This patch introduced a lot of new code to attack a tricky problem,
and sometimes that takes a lot longer to understand and figure out in
review than simpler patches. Also, there are some tricky to understand
concepts that can draw out the time it takes to write (or review!) a
patch. For example, the subtleties of lifetime semantics in LLVM can
be quite tricky. =]

Finally, this review took place contemporaneously with the developer's
meeting which is a big drain on the active reviewers' time.

I don't think you need to be too concerned here about the long-term
productivity impact on you, but I do think we should figure out the
*right* fix, as we are not quite there.