[PATCH] D149393: [AMDGPU][IGLP] Parameterize the SchedGroup processing / linking in Solver

[Austin Kerbow via Phabricator via llvm-commits]

kerbowa added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUIGroupLP.cpp:395
   for (auto &SyncPipeline : BestPipeline) {
-    auto I = SyncPipeline.rbegin();
-    auto E = SyncPipeline.rend();
-    for (; I != E; ++I) {
-      auto &GroupA = *I;
-      for (auto J = std::next(I); J != E; ++J) {
-        auto &GroupB = *J;
+    for (int I = 0; I < (int)SyncPipeline.size(); I++) {
+      int Idx = ProcessDirection == Direction::BOTTOM_UP
----------------
jrbyrnes wrote:
> kerbowa wrote:
> > Are we able to still use iterators here?
> > auto I = IsBottomUp ? SyncPipeline.rbegin() : SyncPipeline.begin();
> > auto E = IsBottomUp ? SyncPipeline.rend() : SyncPipeline.end();
> This doesn't work out of the box. SmallVector::iterator (T *) isn't compatible with SmallVector::reverse_iterator (std::reverse_iterator<T*>). I can template this code based on the iterator, but I thought the current implementation was the cleanest way to do it (unfortunately). We can switch to ranges in c++20. Please correct me if I'm missing something.
Oh right. Could you use two different for_each?
f = loop(GroupA.link(GroupB)
if (IsBottomUp)
  for_each(rbegin(), rend(), f)
else
  for_each(begin(), end(), f)



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149393