[PATCH] D93798: [CSKY 4/n] Add basic CSKYAsmParser and CSKYInstPrinter

Tue Dec 29 19:16:01 PST 2020

zixuan-wu added a comment.

In D93798#2473722 <https://reviews.llvm.org/D93798#2473722>, @rengolin wrote:

> In D93798#2473365 <https://reviews.llvm.org/D93798#2473365>, @zixuan-wu wrote:
>
>> I don't think too many patches on the fly is good to upstream because they influence each other and need change on the way of review.
>
> @MaskRay is right. Even though they do affect each other and you have to do a bit more work in refactoring them, it's a lot easier to see he effect of the early decisions on your patches on the later stages.
>
> So far in CSKY, I have been looking at stubs and they don't look "wrong" but they also don't tell me anything about the future decisions.
>
> You could probably compare the amount of work you do initially on a multi-patch approach with a similar amount with post-merge fixes and local refactoring.
>
> The m64k is a good example on the benefits of having such approach, as we have already avoided some mistakes from happening in the first place.

I think it's early-refactoring issue. We should assume the current patch is good in current codebase context, or we will re-review many times.
We can proposal a rough whole picture patch as reference at first to make sure the direction is right, but every small patch is changing during the review process and only suitable for newest codebase at the moment of upstream.

For example, the development process is A->B->C.  We can/should not make the structure of B is good to C as we lookup ahead  the future patch(C), because B is in codebase of A.
What's more, if we have D following C (C->D), the early review of D is meaningless as B is changing and the change of C is much more. It's much more burden to consider every context (A,B,C,D) for both author and reviewer.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93798/new/

https://reviews.llvm.org/D93798