[PATCH] D79670: [AMDGPU] Order pos exports before param exports
Jay Foad via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon May 11 01:34:20 PDT 2020
foad added inline comments.
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUExportClustering.cpp:38
+
+ // Move position exports before other exports while preserving
+ // the order within different export types (pos or other).
----------------
Can you say why it's beneficial to do position exports first?
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUExportClustering.cpp:41
+ unsigned InsertionPoint = 0;
+ for (unsigned Idx = 0, End = Chain.size(); Idx < End; ++Idx) {
+ SUnit *SU = Chain[Idx];
----------------
It's a shame that this sorting is O(n^2) but I guess it's not a problem because the average chain length will be 2?
You could probably do this sorting in a cute way with std::partition_copy if you felt inclined: https://en.cppreference.com/w/cpp/algorithm/partition_copy
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUExportClustering.cpp:82-85
+ // Pass through DAG gathering a list of exports and removing barrier edges
+ // creating dependencies on exports. Freeing exports of successor edges
+ // allows more scheduling freedom, and nothing should be order dependent
+ // on exports. Edges will be added later to order the exports.
----------------
Why are the barrier edges there in the first place? Either the exports can be reordered, so the barrier edges should not be there; or they can't be, so we shouldn't ignore the barrier edges!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79670/new/
https://reviews.llvm.org/D79670
More information about the llvm-commits
mailing list