[PATCH] D100525: [Greedy RA] Add a check to MachineVerifier

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

Seems reasonable



================
Comment at: llvm/lib/CodeGen/MachineVerifier.cpp:2976-2977
+      // Predecessor of landing pad live-out on last call.
+      // If last call is a statepoint and has tied-def it is
+      // relocated gc pointer and its value is alive in landing pad.
+      if (MFI->isEHPad())
----------------
I don't see anything statepoint-specific below, so I'm not sure how the second part of this comment is relevant. The first sentence makes sense: EH pads have a special rule that requires virtual registers to be defined before the last call in each predecessor, rather than before the first terminator.


================
Comment at: llvm/lib/CodeGen/MachineVerifier.cpp:2978-2979
+      // relocated gc pointer and its value is alive in landing pad.
+      if (MFI->isEHPad())
+        for (auto I = Pred->rbegin(), E = Pred->rend(); I != E; ++I)
+          if (I->isCall()) {
----------------
Please use braces when there is an inner compound statement that uses braces. The style guide has an example that supports this interpretation:
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
```
// Use braces for the outer `if` since the nested `for` is braced.
if (isa<FunctionDecl>(D)) {
  for (auto *A : D.attrs()) {
    // In this for loop body, it is necessary that we explain the situation
    // with this surprisingly long comment, forcing braces on the `for` block.
    handleAttr(A);
  }
}
```


================
Comment at: llvm/lib/CodeGen/MachineVerifier.cpp:2981
+          if (I->isCall()) {
+            PEnd = Indexes->getInstructionIndex(*I).getBoundaryIndex();
+            break;
----------------
I believe this is the correct condition.


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

https://reviews.llvm.org/D100525