This is an archive of the discontinued LLVM Phabricator instance.

[Complex] Don't use __div?c3 when building with fast-math.
ClosedPublic

Authored by paulwalker-arm on Nov 21 2017, 5:41 AM.

Diff Detail

Event Timeline

paulwalker-arm created this revision.Nov 21 2017, 5:41 AM
fhahn added a subscriber: fhahn.

Looks good to me, with some nits. However it seems that we do not use the fast math flags anywhere else in Clang codegen, so it would be good to clarify if it is OK to use it here.

lib/CodeGen/CGExprComplex.cpp
765–766

Maybe we should update the comment that we do a similar simplification for floats with fast math enabled as for integers?

773

Would the following structure be slightly easier to read?

if (RHSi) {

if (FMF.isFast()) { simplify } else {libcall}

}

800

This is basically the same as the simplification for integers, unfortunate that we have to duplicate it (because of different instructions).

hfinkel added inline comments.Dec 14 2017, 4:31 PM
lib/CodeGen/CGExprComplex.cpp
773

I'd use CGF.getLangOpts().FastMath (instead of interrogating the implicit state stored in the IR builder).

test/CodeGen/complex-math.c
134

Please check the actual expansion here and below.

Query LangOpts for FastMast rather than IRBuilder and fleshed out the tests.

paulwalker-arm marked 3 inline comments as done.Dec 19 2017, 7:52 AM
paulwalker-arm added inline comments.
lib/CodeGen/CGExprComplex.cpp
773

Probably subjective but in this instance I preferred the look with fewer nested conditionals.

hfinkel accepted this revision.Dec 19 2017, 6:16 PM

LGTM

This revision is now accepted and ready to land.Dec 19 2017, 6:16 PM
fhahn added a comment.Dec 20 2017, 7:06 AM

I'll commit this for you Paul

This revision was automatically updated to reflect the committed changes.