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

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 2 inline comments as done.
fhahn added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp:1488
+  if (!Interferences.empty())
+    Topo.fixOrder();
   // All candidates are delayed due to live physical reg dependencies.
----------------
efriedma wrote:
> fhahn wrote:
> > efriedma wrote:
> > > Where exactly do we query the topological order relative to this call to fixOrder()?  Do we need to fixOrder() inside the loop?
> > We only query it in the loop below (WillCreateCycle call). In case we add new predecessors, we also exit the loop. And we only enter the loop, if there are some interferences. Maybe it's worth adding a comment here?
> Probably worth adding a comment, yes.
> 
> By the way, how frequently do we hit this case in practice?
> By the way, how frequently do we hit this case in practice?

I haven't looked at this call here in isolation, but I gather statistics on the impact of the number of InitDAGTopologicalSorting and AddPred calls.

Number of InitDAGTopologicalSorting calls for CTMark, X86, -O1
                                                                     patch          base
CTMark/kimwitu++/kc.test                      25671.0    46740.0
CTMark/Bullet/bullet.test                        16228.0    36873.0
CTMark/mafft/pairlocalalign.test            14290.0    28695.0
CTMark/lencod/lencod.test                     19006.0    37880.0
CTMark/sqlite3/sqlite3.test                     22447.0    49409.0
CTMark/ClamAV/clamscan.test               23087.0.   48369.0
CTMark/7zip/7zip-benchmark.test         41045.0    98737.0
CTMark/tramp3d-v4/tramp3d-v4.test   22057.0    49214.0
CTMark/SPASS/SPASS.test                      19596.0   40652.0
CTMark/consumer-typeset/consumer-typeset.test 16567.0 33208.0

Number of AddPred calls for CTMark, X86, -O1
                                                                        patch       base
CTMark/kimwitu++/kc.test                        2645.0    2702.0
CTMark/Bullet/bullet.test                          3619.0    3643.0
CTMark/mafft/pairlocalalign.test              3427.0    3455.0
CTMark/lencod/lencod.test                       3902.0    4108.0
CTMark/sqlite3/sqlite3.test                       3147.0    3179.0
CTMark/ClamAV/clamscan.test                 3764.0    3821.0
CTMark/7zip/7zip-benchmark.test           7308.0    7393.0
CTMark/tramp3d-v4/tramp3d-v4.test     4311.0    4315.0
CTMark/SPASS/SPASS.test                       4512.0    4577.0
CTMark/consumer-typeset/consumer-typeset.test 3695.0    3734.0

Also, the latest version of the patch improves compile-time on CTMark X86, -O1 a bit more: negative diff means a reduction in compile time.
Program                                                                   diff
 test-suite...ark/tramp3d-v4/tramp3d-v4.test     -0.6%
 test-suite :: CTMark/Bullet/bullet.test                -0.4%
 test-suite...:: CTMark/sqlite3/sqlite3.test            -0.4%
 test-suite :: CTMark/kimwitu++/kc.test               -0.4%
 test-suite...TMark/7zip/7zip-benchmark.test      -0.3%
 test-suite :: CTMark/lencod/lencod.test               -0.3%
 test-suite :: CTMark/SPASS/SPASS.test               -0.3%
 test-suite...-typeset/consumer-typeset.test         -0.3%
 test-suite...:: CTMark/ClamAV/clamscan.test       -0.2%
 test-suite...Mark/mafft/pairlocalalign.test             0.0%
 Geomean difference                                                -0.3%


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60125