[PATCH] D102269: [ARM] Expand predecessor search to multiple blocks when reverting WhileLoopStarts

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/ARM/MVETPAndVPTOptimisationsPass.cpp:269
+MachineInstr *MVETPAndVPTOptimisations::CheckForLRUseInPredecessors(
+    MachineBasicBlock *PreHeader, MachineInstr *LoopStart) {
+  SmallVector<MachineBasicBlock *> Worklist;
----------------
malharJ wrote:
> Since the function name has changed, this (and also in the function declaration) parameter name
> could also be changed to "Predecessor" ?
No, it's a preheader. Though the naming gets a bit fuzzy. There was a preheader that is split into multiple blocks, which this has to search through. They together will make up the predecessors. And eventually reach the block with the WhileLoopStart (which in other contexts is also called the predecessor.)


================
Comment at: llvm/lib/Target/ARM/MVETPAndVPTOptimisationsPass.cpp:281
-    for (MachineInstr &MI : *PreHeader) {
-      if (MI.isCall()) {
-        // Create a t2DoLoopStart at the end of the preheader.
----------------
malharJ wrote:
> malharJ wrote:
> > Uhm, just trying to understand the original logic ...
> > 
> > but why does a call (in the Preheader) result in reversion of t2WhileLoopStartLR to t2DoLoopStart ?
> What I meant was wouldn't a call (in the preheader/predecessor) be placed before the t2WhileLoopStartLR (since it's a terminator) ... and hence it should not matter if LR get's overwritten by the call ?
This is after the WhileLoopStart, but before the actual loop. There can be other instructions/blocks in that preheader before the loop, including calls and whatnot. The block that contains the WhileLoopStart isn't the same as the block that we put the DoLoopStart in.


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

https://reviews.llvm.org/D102269