This is an archive of the discontinued LLVM Phabricator instance.

[X86] Don't use fsin/fcos/fsincos instructions ever

Authored by craig.topper on Aug 4 2017, 5:10 PM.



Previously we would use these instructions if sse was disabled and fastmath was enabled.

As mentioned in D28335, this is a bad idea.

Diff Detail


Event Timeline

craig.topper created this revision.Aug 4 2017, 5:10 PM
DavidKreitzer edited edge metadata.Aug 9 2017, 11:43 AM

Just a minor question on the test changes. Otherwise, this LGTM.

124 ↗(On Diff #109839)

Was moving the check for 'ret' here intentional?

craig.topper added inline comments.Aug 9 2017, 11:45 AM
124 ↗(On Diff #109839)

I think all I did was copy the GNU_SINCOS check lines from above and changed them to say GNU_SINCOS_FASTMATH. I hadn't noticed the difference in the ret.

craig.topper added inline comments.Aug 11 2017, 10:41 AM
124 ↗(On Diff #109839)

Do you want me to add it back or is it ok like this?

DavidKreitzer accepted this revision.Aug 11 2017, 11:14 AM
DavidKreitzer added a subscriber: zvi.

Adding Zvi as an FYI, but I think this is fine to land.

124 ↗(On Diff #109839)

As far as I'm concerned it's okay either way.

This revision is now accepted and ready to land.Aug 11 2017, 11:14 AM
This revision was automatically updated to reflect the committed changes.