This is an archive of the discontinued LLVM Phabricator instance.

[DAG] Set up infrastructure to avoid smart constructor-based dangling nodes
ClosedPublic

Authored by niravd on Feb 11 2019, 12:29 PM.

Details

Summary

Various SelectionDAG non-combine operations (e.g. the getNode smart
constructor and legalization) may leave dangling nodes by applying
optimizations without fully pruning unused result values. This results
in nodes that are never added to the worklist and therefore can not be
pruned.

Add a node inserter for the combiner to make sure such nodes have the
chance of being pruned. This allows a number of additional peephole
optimizations.

Diff Detail

Repository
rL LLVM

Event Timeline

niravd created this revision.Feb 11 2019, 12:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 11 2019, 12:29 PM
niravd updated this revision to Diff 188609.Feb 27 2019, 1:18 PM
niravd edited the summary of this revision. (Show Details)

Rebase after some cleanups. Add comments on regressions test cases. Most of these are exposing PR40881. '
The remain case (vsel-cmp-load.ll) appears to be a missing revisitation some nodes

Could you add a comment to the CL description noting that a bunch of the calls to AddToWorklist in DAGCombiner.cpp that follow a DAG.getNode() are now redundant, and should be cleaned up in the future.

llvm/test/CodeGen/X86/3addr-or.ll
21 ↗(On Diff #188609)

These are all due to PR40881? Please add PR references to all the FIXME comments if possible.

niravd updated this revision to Diff 191160.Mar 18 2019, 1:28 PM

Address James' comments.

jyknight accepted this revision.Mar 25 2019, 12:12 PM
This revision is now accepted and ready to land.Mar 25 2019, 12:12 PM
niravd updated this revision to Diff 192280.Mar 26 2019, 7:53 AM

Rebase to tip before landing.

niravd reopened this revision.Mar 27 2019, 12:54 PM

Initial commit reverted due to Halide compile time explosion.

This revision is now accepted and ready to land.Mar 27 2019, 12:54 PM
niravd updated this revision to Diff 192774.Mar 28 2019, 8:17 PM
niravd retitled this revision from [DAG] Avoid smart constructor-based dangling nodes. to [DAG] Set up infrastructure to avoid smart constructor-based dangling nodes.

Defer Worklist modification in NodeInserter to follow up patch (D58070).

jyknight accepted this revision.Mar 29 2019, 8:35 AM

Looks fine again.

This revision was automatically updated to reflect the committed changes.