[PATCH] D60125: [ScheduleDAGRRList] Recompute topological ordering on demand.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/CodeGen/ScheduleDAG.cpp:716
                                              const SUnit *TargetSU) {
+  assert(!Dirty && Updates.empty() && "Topological order is outdated");
   // If insertion of the edge SU->TargetSU would create a cycle
----------------
fhahn wrote:
> efriedma wrote:
> > How terrible would it be to just call fixOrder from here, instead of making the callers check?  That makes it impossible for callers to mess up, and the check should be cheap.
> > 
> > I guess it also might be possible to add some cheap checks here to avoid calling fixOrder; for example, if TargetSU has no successors.
> I guess the impact would not be too big, in fact that was what I initially had. I'll measure it and get back. I think it would be OK to push responsibility to the caller in a way and the assertion should catch any errors (the message should probably point to fixOrder()).
I gathered numbers with having FixOrder() in IsReachable & WillCreateCycle. The numbers shifted a bit, but we still have roughly the same overall gain. It seems in some cases one approach is slightly better and for some cases the other one. I guess we should go for the safer one for now?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60125/new/

https://reviews.llvm.org/D60125