This is an archive of the discontinued LLVM Phabricator instance.

Use the 'arcp' fast-math-flag when combining repeated FP divisors
ClosedPublic

Authored by spatel on May 12 2015, 10:15 AM.

Details

Summary

This is a usage of the IR-level fast-math-flags now that they are propagated to SDNodes (r237046). This was originally part of D8900.

I'm not sure if we need to maintain the global TargetOptions.UnsafeFPMath checks or if we're now free to start deleting those and rely exclusively on the node-level optimization flags. If the latter, I need to update the corresponding fdiv testcases for PowerPC and Aarch64 to use 'arcp'.

Diff Detail

Repository
rL LLVM

Event Timeline

spatel updated this revision to Diff 25597.May 12 2015, 10:15 AM
spatel retitled this revision from to Use the 'arcp' fast-math-flag when combining repeated FP divisors.
spatel updated this object.
spatel edited the test plan for this revision. (Show Details)
spatel added reviewers: hfinkel, echristo, ab, kariddi, milseman.
spatel added a subscriber: Unknown Object (MLST).

To preserve backward compatibility, I think we should make sure functions using fast math attributes (e.g., "unsafe-fp-math" in test/CodeGen/PowerPC/fdiv-combine.ll) still get optimized after we migrate to modeling those flags at the node-level (using autoupgrade?).

To preserve backward compatibility, I think we should make sure functions using fast math attributes (e.g., "unsafe-fp-math" in test/CodeGen/PowerPC/fdiv-combine.ll) still get optimized after we migrate to modeling those flags at the node-level (using autoupgrade?).

Thanks, Akira. If I understand correctly, I should create this autoupgrade patch first then. That would allow us to remove the Options.UnsafeFPMath checks as we update all of the places in the backend to use the node-level flags.

Is a separate mechanism needed to handle the llc command-line flag "-enable-unsafe-fp-math"?

spatel updated this revision to Diff 38575.Oct 27 2015, 11:38 AM

Sorry for the (5 month!) delay...

After several false starts, FMF on DAG nodes appears to be here for good. This will still be the first use of them to enable an optimization from what I can tell.

Patch rebased.

hfinkel accepted this revision.Oct 27 2015, 11:47 AM
hfinkel edited edge metadata.

This will still be the first use of them to enable an optimization from what I can tell.

:-)

LGTM.

This revision is now accepted and ready to land.Oct 27 2015, 11:47 AM
This revision was automatically updated to reflect the committed changes.