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

[Xinliang David Li via llvm-commits]

I am not too wedded to further splitting up D15134 -- what i had is a
general suggestion.

David

On Thu, Dec 3, 2015 at 10:55 AM, Chih-hung Hsieh <chh at google.com> wrote:
> David, I think you have a pretty good picture of the complete patch in
> D11438 now. It seems premature of me to split that patch before I have made
> all your suggested changes. Now I have to test and upload every change with
> both D11438 and D15134.
>
> Could we complete all code review and changes before further split?
> Thanks.
>
>
> On Wed, Dec 2, 2015 at 6:24 PM, Xinliang David Li <davidxl at google.com>
> wrote:
>>
>>
>>
>> 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
>>>> >
>>>> >
>>>> >
>>>
>>>
>>
>