Page MenuHomePhabricator

[DAGCombine] Fold the (fma -x, y, -z) to -(fma x, y, z)
ClosedPublic

Authored by steven.zhang on Jan 6 2020, 7:44 PM.

Details

Summary

This is a positive combination as long as the NEG is NOT free, as we are reducing the number of NEG from two to one.

Diff Detail

Event Timeline

steven.zhang created this revision.Jan 6 2020, 7:44 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 6 2020, 7:44 PM

Unit tests: pass. 61273 tests passed, 0 failed and 736 were skipped.

clang-tidy: pass.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Format the patch.

Jim added a subscriber: Jim.Jan 6 2020, 10:10 PM

Seems reasonable to me, but i'll leave review to someone more FP-familiar.

Wouldn't it be better to make use of the TargetLowering::isNegatibleForFree/getNegatedExpression mechanism?

This is really a good suggestion. I will go with that mechanism, though it seems that, it didn't respect the TLI.isFNegFree() and UnsafeFPMath which didn't make sense. (When it is FNEG opcode, we just return 2 to indicate that it is profitable. What if some target is free for FNEG operations ...)

Use the isNegatibleForFree/getNegatibleExpression mechanism.

RKSimon accepted this revision.Jan 8 2020, 3:47 AM

LGTM - cheers

This revision is now accepted and ready to land.Jan 8 2020, 3:47 AM
xbolva00 added inline comments.
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
12645

It would be better if this function returns enum constant to improve readability.

RKSimon added inline comments.Jan 8 2020, 4:21 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
12645

Yeah, thats been a pet peeve of mine as well for a long time - I'll take a look (might be after the release though)

This revision was automatically updated to reflect the committed changes.