[PATCH] D92385: [ARM] Add a pass that re-arranges blocks when there is a backwards WLS branch
Sam Tebbs via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 12 05:59:34 PST 2021
samtebbs marked 3 inline comments as done.
samtebbs added inline comments.
================
Comment at: llvm/lib/Target/ARM/ARMBlockPlacement.cpp:123
+ CanMove = false;
+ break;
+ }
----------------
SjoerdMeijer wrote:
> I am a bit struggling with reading the indentation, but was wondering if we could `return false` here? In that case, do we need `CanMove`?
I don't think we can return as that would stop looking at other loops in the function.
================
Comment at: llvm/lib/Target/ARM/ARMBlockPlacement.cpp:130
+ // An example loop structure where the LoopExit can't be moved, since
+ // bb2's LE will become forwards once bb1 is moved after bb3 bb1: -
+ // LoopExit bb2:
----------------
SjoerdMeijer wrote:
> nit: newline after `bb3` and LoopExit? I guess you want to have:
>
> // bb1: -LoopExit
> // bb2:
> // LE bb1 - Terminator
> // bb3:
> // ...
Ah yes. I blame clang-format.
================
Comment at: llvm/lib/Target/ARM/ARMBlockPlacement.cpp:136
+ // bb4: - Header
+ for (auto It = LoopExit->getIterator(); It != Preheader->getIterator();
+ It++) {
----------------
SjoerdMeijer wrote:
> Have we checked that LoopExit < Preheader?
Yep, on line 85.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92385/new/
https://reviews.llvm.org/D92385
More information about the llvm-commits
mailing list