[PATCH] D99774: [LoopUtils] Populate sibling loops in reverse program order on new pass manager

[JinGu Kang via Phabricator via llvm-commits]

jaykang10 created this revision.
jaykang10 added reviewers: aeubanks, asbirlea.
Herald added subscribers: zzheng, hiraditya.
jaykang10 requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

I have seen performance regressions from new pass manager against legacy pass manager. It looks there is a problem with the order of loops on loop pass manager.
The legacy pass manager populates loops as below.

  bool LPPassManager::runOnFunction(Function &F) {
  ...
    // Populate the loop queue in reverse program order. There is no clear need to
    // process sibling loops in either forward or reverse order. There may be some
    // advantage in deleting uses in a later loop before optimizing the
    // definitions in an earlier loop. If we find a clear reason to process in
    // forward order, then a forward variant of LoopPassManager should be created.
    //  
    // Note that LoopInfo::iterator visits loops in reverse program
    // order. Here, reverse_iterator gives us a forward order, and the LoopQueue
    // reverses the order a third time by popping from the back.
    for (Loop *L : reverse(*LI))
      addLoopIntoQueue(L, LQ)

The new loop pass manager populates loops as below.

  // We walk the initial sequence of loops in reverse because we generally want
  // to visit defs before uses and the worklist is LIFO.
  for (Loop *RootL : Loops) {
    assert(PreOrderLoops.empty() && "Must start with an empty preorder walk.");
    assert(PreOrderWorklist.empty() &&
           "Must start with an empty preorder walk worklist.");
    PreOrderWorklist.push_back(RootL);
    do { 
      Loop *L = PreOrderWorklist.pop_back_val();
      PreOrderWorklist.append(L->begin(), L->end());
      PreOrderLoops.push_back(L);
    } while (!PreOrderWorklist.empty());
  
    Worklist.insert(std::move(PreOrderLoops));
    PreOrderLoops.clear();
  }

As you can see, new loop pass manager uses different order of loops with legacy pass manager.

I have seen a worse case with the order of new loop pass manager.

  loop1:
  ...
  br cond %loop1, %bb.exit
  
  bb.exit:
  ...
  br %loop2
  
  loop2:
  ...
  }

The `bb.exit` is exit block for loop1 but it is preheader is for loop2. With new loop pass manager, the bb.exit is updated during all loop optimization on loop1. The updated bb.exit affects loop optimization passes on loop2 and it causes more instructions. If I use the reverse order of loops on new loop pass manager, I can see the redundant instructions are gone. I am not sure which order is better. Can someone let me know why the new pass manager uses different order of loops against legacy pass manager please?


https://reviews.llvm.org/D99774

Files:
  llvm/lib/Transforms/Utils/LoopUtils.cpp
  llvm/test/Analysis/LoopNestAnalysis/infinite.ll
  llvm/test/CodeGen/AArch64/loop-micro-op-buffer-size-t99.ll
  llvm/test/Other/loop-pass-ordering.ll
  llvm/test/Transforms/IndVarSimplify/scev-expander-preserve-lcssa.ll
  llvm/test/Transforms/LoopUnroll/X86/store_cost.ll
  llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D99774.334831.patch
Type: text/x-patch
Size: 6532 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210401/2e8e4adc/attachment.bin>