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

[Chih-hung Hsieh via llvm-commits]

Sean, could you review the test cases in http://reviews.llvm.org/D11438 too?

New features (or bug fixes) introduced in this patch really should only be
executed when x86_64 f128 type is configured to stay in SSE registers.
We don't turn on that configuration in this partial patch, so all unit
tests in D11438 won't apply. Is there some new test we can add to test
other types for this patch?

After my initial submit of D11438 in Jul 22 2015, I have received virtually
no comment on those unit tests yet. I do feel the risk of test coverage for
both new x86_64 f128 config and existing configurations.

I explained before in patch D11438 comments that a partial change of x86_64
f128 is worse than no change at all, as we would rather have a working
non-gcc compatible x86_64 long double type than a buggy type or crashing
compiler. So a partial commit like this has only benefit to verify partial
regression to non x86_64 f128 types, which has lower possibility and I have
no trouble to revert them all at once.

The main battle will be for Android platform to verify the correctness of
x86_64 f128 type after the configuration is submitted in the follow up
patch.


On Wed, Dec 2, 2015 at 6:21 PM, Sean Silva <chisophugis at gmail.com> wrote:

> silvas added a comment.
>
> In http://reviews.llvm.org/D15134#300978, @chh wrote:
>
> > It would be easier to keep track of these changes together, right?
>
>
> No, it is actually the opposite. The more fine-grained the individual
> commits, the easier it is for a reviewer to review, and the easier it is to
> identify what caused an issue if an issue does show up. Tests are also
> easier to write for fine-grained commits (or, it is easier to identify that
> a fine-grained commit does not change externally visible behavior and so
> doesn't require a test).
>
> Also, the changes you are proposing here seem to fix bugs / introduce new
> functionality. Therefore tests are required.
>
>
> http://reviews.llvm.org/D15134
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151203/5c03caba/attachment.html>