This is an archive of the discontinued LLVM Phabricator instance.

Remove non-necessary canonicalization from the DAG
ClosedPublic

Authored by mehdi_amini on Apr 29 2015, 8:53 PM.

Details

Reviewers
spatel
Summary

r236031 introduced a transformation to reduce the dependency chain
for a sequence of binop instructions.

(fadd N0: (fadd N00: (fadd z, w), N01: y), N1: x) ->
(fadd N00: (fadd z, w), (fadd N1: x, N01: y))

However as part of the implementation, a canonicalization was
introduced that commute chain of fadd to be on the left side:

(fadd (otherop, fadd)) -> (fadd (fadd, otherop))

This canonicalization does not seem required to perform the original
targetted transformation, swapping the operand locally is enough to
achieve this goal as far as I understand. Moreover the DAG is already
slow and the way it was done required a round trip through the
combiner queue and the creation of a temporary node.

Diff Detail

Event Timeline

mehdi_amini retitled this revision from to Remove non-necessary canonicalization from the DAG.
mehdi_amini updated this object.
mehdi_amini edited the test plan for this revision. (Show Details)
mehdi_amini added a reviewer: spatel.
mehdi_amini added a subscriber: Unknown Object (MLST).
spatel accepted this revision.Apr 29 2015, 9:05 PM
spatel edited edge metadata.

LGTM. Thanks!

This revision is now accepted and ready to land.Apr 29 2015, 9:05 PM
spatel closed this revision.May 14 2015, 12:32 PM

Closing; I used the swap suggestion in the new proposal of this patch in D9780.