This is an archive of the discontinued LLVM Phabricator instance.

[mlir] GreedyPatternRewriteDriver: Do not enqueue operands of deleted ops
AbandonedPublic

Authored by springerm on Feb 23 2023, 6:24 AM.

Details

Summary

Only ops that were newly inserted or changed should be enqueued.

Patterns can match based on their operands but not based on the uses of their results.

Depends On: D144257

Diff Detail

Event Timeline

springerm created this revision.Feb 23 2023, 6:24 AM
springerm requested review of this revision.Feb 23 2023, 6:24 AM

This change is needed for D144193. Otherwise, we may enqueue deleted operations when removing an op op such as:

"op"() {
 // graph region
  %a = "foo"(%b)
  %b = "foo"(%a)
}

The alternative would be to immediately erase an op after triggering notifyOperationRemoved. I.e., a walk over the op that removes nested ops one-by-one and triggers notifications. This would be less efficient than what we are doing now: a walk over an operation to send notifications, followed by a single Operation::erase (that also drops all nested ops).

springerm edited the summary of this revision. (Show Details)Feb 23 2023, 6:30 AM
springerm added a reviewer: Mogball.
springerm updated this revision to Diff 499840.Feb 23 2023, 6:44 AM

remove dead code

springerm planned changes to this revision.Feb 23 2023, 7:12 AM

One problem with this approach is that it increases the number of iterations needed for convergence.