[PATCH] D88823: [CodeGen][TailDuplicator] Don't duplicate blocks with INLINEASM_BR

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added inline comments.


================
Comment at: llvm/lib/CodeGen/TailDuplicator.cpp:630-634
+    // Copying a block with an INLINEASM_BR instruction may result in a PHI node
+    // on the indirect path. We assume that all values used on the indirect path
+    // dominates all paths into the indirect block, i.e. don't have PHI nodes.
+    // FIXME: This may be too restrictive. Perhaps should restrict only if a
+    //        value in the current block isn't used on the indirect path.
----------------
I'd update this to say something to the effect of:

```
TailDuplicator::appendCopies() will erroneously place COPYs post INLINEASM_BR instructions
after 4b0aa5724fea, demonstrating the same bug that was fixed in findPHICopyInsertPoint() in
f7a53d82c090.
FIXME: use findPHICopyInsertPoint() in TailDuplicator::appendCopies() rather than
MachineBasicBlock::getFirstTerminator() to find the correct insertion point for the
COPY when replacing PHIs.
```

With that comment block updated, the commit message updated, and [the altered test case](https://reviews.llvm.org/D88823#2313499) from @jyknight , we'd be all set to land this and close out the clang-11 release.  Then we have plenty of time to follow up on the FIXME for clang-12. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88823