[PATCH] D15134: Part 1 to fix x86_64 fp128 calling convention.

[Xinliang David Li via llvm-commits]

On Wed, Dec 2, 2015 at 5:17 PM, Chih-hung Hsieh <chh at google.com> wrote:

> On Wed, Dec 2, 2015 at 3:47 PM, Xinliang David Li <davidxl at google.com>
> wrote:
>
>> On Wed, Dec 2, 2015 at 3:44 PM, Chih-Hung Hsieh <chh at google.com> wrote:
>> > chh added a comment.
>> >
>> > Sean, only the "Fix printOperand to handle null operand" in
>> SelectionDAGDumper.cpp might have some value by itself.
>>
>> If you can submit the smaller patches independently, it is still
>> better to submit them separately  -- it reduces the chances of build
>> breaks.
>>
>>
> One small change is less risky than one large change, but
> it's unclear to me that the total number of broken builds (downtime)
> can be reduced by breaking a 10-file patch into 10 1-file patches.
>
> If there are K errors, they will break builds K times
> if we discover and fix them one by one.
> The hope is to find error before submitting file #2 from the
> regression of submitted file #1. But it seems more likely
> to discover both errors in #1 and #2 when they are submitted together
> and then reverted and fixed together.
>
>
It is a common wisdom to break patch into smaller pieces -- I will try my
best to explain:
1) you don't get all errors at the same time when you submit a large patch
-- some errors may be masked by previous errors
2) The more errors you incur at the same time, the bigger the chances other
submitter's errors leaked in without being caught -- making it hard to
isolate the problems later.
3) Errors in smaller patches are easier to correct -- in many cases you can
fix the problem quicker than rolling it back
4) Smaller patches give other people a chance to peek at the patch and spot
obvious errors missed in the review

Not to mention that smaller patch make it easier for downstream LLVM users
to do merge more easiliy.

Breaking patch into smaller one, you do need to pay a little more cost
up-front, but it is worth it.

(I am not saying you have to break it up in really small granularity -- you
need to use some good judgement here).

David




> -- chh
>
>
>
>
>> David
>>
>>
>> > All other changes are not useful before x86_64 f128 type is configured
>> to fix the calling convention problem.
>> > This part 1 change should have no impact at all before the rest of
>> http://reviews.llvm.org/D11438 is merged in.
>> > Getting this part 1 in first is just to check for any regression.
>> > I think all these changes should go in or be reverted when necessary.
>> > It would be easier to keep track of these changes together, right?
>>
>>
>>
>>
>> >
>> >
>> > http://reviews.llvm.org/D15134
>> >
>> >
>> >
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151202/e9ef534d/attachment.html>