Page MenuHomePhabricator

[DAGCombiner] Add node to the worklist in topological order in parallelizeChainedStores

Authored by deadalnix on Aug 23 2019, 8:29 AM.

Diff Detail

Event Timeline

deadalnix created this revision.Aug 23 2019, 8:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 23 2019, 8:29 AM
lebedev.ri added inline comments.Aug 23 2019, 11:00 AM
20665 ↗(On Diff #216863)

Outdated comment?

Fix outdated comment

fhahn added a subscriber: fhahn.Aug 24 2019, 11:45 AM

Can you add a test case, where this triggers?

Because the order in which nodes are traversed is not reliable, it is very hard to come up with a test for this. Ultimately, this is part of a larger body of work that intend to ensure nodes are processed in topological order by DAGCombiner. Even if they do not change the codegen, doing these changes are useful as they reduce the pressure on the logic that maintains the topological order.

RKSimon accepted this revision.Aug 27 2019, 5:44 AM

LGTM, I agree that creating a robust test is going to be tricky and probably not worth it.

This revision is now accepted and ready to land.Aug 27 2019, 5:44 AM
This revision was automatically updated to reflect the committed changes.