This is an archive of the discontinued LLVM Phabricator instance.

[libc] Move implementations of expf and exp2f from the AOR to src/math.
ClosedPublic

Authored by sivachandra on Apr 29 2020, 4:41 PM.

Diff Detail

Event Timeline

sivachandra created this revision.Apr 29 2020, 4:41 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 29 2020, 4:41 PM

This change is not quite ready but feel free to share any opinions/comments you might have on the changes I am making. I started with moving exp and all its friends but soon ended with a large change which even I was getting confused about. Also, I made a mistake with how LLVM libc results are compared with MPFR results. I have to fix that first before I can add more tests to this patch.

abrachet added inline comments.
libc/src/math/exp2f.cpp
18–20

These are only used once or twice and we just expand these manually?

27

We probably want these to be //

libc/src/math/exp2f.h
2–3

Classic clang-format

11–12

MATH_EXP2F

libc/src/math/exp_utils.cpp
2

Missing comment

libc/src/math/expf.h
10–12

MATH_EXPF

  • Rebase and add tests
  • Address comments from @abrachet

I am mostly done with everything I wanted to do in this patch but I am still leaving it without reviewers as I want to do one more pass tomorrow before I add reviewers. Like with the other math change, I want to keep the general look of the code similar to how it is in the AOR directory. So, I have not made widespread stylistic changes which would change the look and feel of the actual code. That said, as I start moving the double precision functions, it might be more appropriate to consolidate the now dispersed floating point utils into a single, more C++ like util library. I will start doing it in the following patches as I learn more about the double precision implementations.

Added reviewers now. Just pointing out again: I have not done any stylistic changes to the main code. The idea is to keep it as familiar as possible to the AOR developers when they start working with LLVM-libc.

phosek accepted this revision.May 15 2020, 12:15 AM

LGTM

libc/utils/MPFRWrapper/MPFRUtils.h
42

This could be enum class and we could avoid the OP_ prefix but this is fine as well.

This revision is now accepted and ready to land.May 15 2020, 12:15 AM
This revision was automatically updated to reflect the committed changes.