[PATCH] D122959: [ARM] Only update the successor edges for immediate predecessors of PrologueMBB.

[John Brawn via Phabricator via llvm-commits]

john.brawn added a comment.

I don't know much about this specific bit of code, but a comment in ReplaceUsesOfBlockWith mentions that it's expected that 'Old' is a successor of 'this' so the fix looks reasonable. However, could you also add a test for this?



================
Comment at: llvm/lib/Target/ARM/ARMFrameLowering.cpp:2573-2578
+    // Only update for immediate predecessors.
+    if (!PrologueMBB.isPredecessor(MBB))
+      continue;
     // Replace the edges to PrologueMBB by edges to the sequences
     // we are about to add.
     MBB->ReplaceUsesOfBlockWith(&PrologueMBB, AddedBlocks[0]);
----------------
I think this would be a bit clearer if rewritten as

```
  if (MBB->isSuccessor(&PrologueMBB))
    MBB->ReplaceUsesOfBlockWith(&PrologueMBB, AddedBlocks[0]);
```
i.e. the test and action are both MBB->thing(&PrologueMBB).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122959