Page MenuHomePhabricator

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

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

Diff Detail

Event Timeline

deadalnix created this revision.Fri, Aug 23, 8:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptFri, Aug 23, 8:29 AM
lebedev.ri added inline comments.Fri, Aug 23, 11:00 AM
lib/CodeGen/SelectionDAG/DAGCombiner.cpp
20665 ↗(On Diff #216863)

Outdated comment?

Fix outdated comment

fhahn added a subscriber: fhahn.Sat, Aug 24, 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.Tue, Aug 27, 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.Tue, Aug 27, 5:44 AM
This revision was automatically updated to reflect the committed changes.