[PATCH] D141487: [LoopUnroll] Directly update DT instead of DTU.

[Florian Hahn via Phabricator via llvm-commits]

fhahn updated this revision to Diff 489405.
fhahn added a comment.

I took a step back and removed the need to add and use a new `updateDominatorTreeUsingPredecessors` helper. I discovered cases in general where we miss updates. While this was not an issue for the LoopUnroll test case, it seems dangerous to expose it.

The current patch switches to a hybrid approach: if there's a single exiting node, it should be sufficient to directly update the immediate dominators of the original exiting block and move them to the first actual exiting block of the unrolled loop. for all other cases, use the DTU.

After D141810 <https://reviews.llvm.org/D141810>, only MergeBlockIntoPredecessor needs udpating, and here the update should also be a straight-forward transfer of immediate dominators. The latest version achieves the same CTMark gains, while it should be much more limited in scope.

In D141487#4051794 <https://reviews.llvm.org/D141487#4051794>, @kuhar wrote:

>> Recalculating the DTs from scratch is much quicker than DTU (~18s), but still noticeable slower than manual updates (~2s).
>
> It would be great if we could detect those cases and teach DTU to trigger this recalculation. Would 18s be an acceptable fix for your use case?

It would be an improvement, but LLVM performs loop-unrolling very aggressively and I think it is very desirable to make sure the code to update the DT here is as fast as possible. The latest version of the patch is even slightly faster.

I also experimented with cut-offs for the DTU based on the number of nodes it needs to traverse in its DFS, which yielded good results for the slow case, but was negative on CTMark :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141487

Files:
  llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
  llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
  llvm/lib/Transforms/Utils/LoopUnroll.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D141487.489405.patch
Type: text/x-patch
Size: 7732 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230115/44ba1b32/attachment.bin>