Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

[libc++] Use _Complex for multiplication and division of complex floating point types
Needs ReviewPublic

Authored by philnik on Jul 14 2023, 10:04 AM.

Details

Reviewers
None
Group Reviewers
Restricted Project
Summary

This significantly simplifies the implementation and improves the codegen. The only downside is that the accuracy can be marginally worse, but that is up to the compiler to decide with this change, which means is can be controlled by compiler flags.

Diff Detail

Event Timeline

philnik created this revision.Jul 14 2023, 10:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 14 2023, 10:04 AM
philnik requested review of this revision.Jul 14 2023, 10:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 14 2023, 10:04 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
philnik updated this revision to Diff 540735.Jul 15 2023, 1:48 PM

Try to fix CI

philnik edited the summary of this revision. (Show Details)Jul 15 2023, 1:59 PM

Can you modify the CI file to run with a limited number of configurations, but enable all platform.

Also fix the non floating point test to be fixed.

That way I can get a better view of the impact of the changes.

libcxx/test/std/numerics/complex.number/complex.member.ops/divide_equal_complex.pass.cpp
29–30

I would like to test with an epsion and not >=.
I think the values used in the test should give an exact result.

Please verify nan and inf etc are tested otherwise test whether they change.

I think this is the right approach to implement floating-point complex. We should let the compiler do the work whenever we can. If the result is "not good enough", then that's something we should take to the compiler folks (probably compiler-rt) instead.

Also, this is how libstdc++ implements their std::complex operations, so it's not like we're doing something crazy here. The need to change tests makes me a tiny bit uncomfortable, but in reality I think the tests should have been written that way from the start -- comparing floating point values for equality is a well known anti pattern.

So this roughly looks good to me but I'd like to see again after updating with the comments and green CI.

libcxx/test/std/numerics/complex.number/complex.member.ops/divide_equal_complex.pass.cpp
29–30

Yeah we should define a function to describe the fact that the value is "equal" (or "within the tolerance") instead of explicitly checking >= and <=.

libcxx/test/std/numerics/complex.number/complex.ops/complex_divide_complex.pass.cpp
15

Is this still required? Applies here and in the other complex tests I guess.

philnik updated this revision to Diff 556803.Thu, Sep 14, 12:15 PM
philnik marked 3 inline comments as done.

Address comments

philnik updated this revision to Diff 556960.Mon, Sep 18, 10:03 AM

Try to fix CI