[PATCH] D79794: Change the INLINEASM_BR MachineInstr to be a non-terminating instruction.

[Bill Wendling via Phabricator via llvm-commits]

void added a comment.

I think we should rename `INLINEASM_BR` to something that doesn't involve the `_BR` bit, since it's no longer a branch.

In general, I'm okay with this approach. (I thought we shouldn't make it a terminator in the first place.)



================
Comment at: llvm/lib/CodeGen/MachineBasicBlock.cpp:281-284
+  for (const_succ_iterator I = succ_begin(), E = succ_end(); I != E; ++I)
+    if ((*I)->isInlineAsmBrIndirectTarget())
+      return true;
+  return false;
----------------
nickdesaulniers wrote:
> ```
> return any_of(successors(), [](MachineBasicBlock* Succ) {
>   return Succ->isInlineAsmBrIndirectTarget(); };
> ```
> 
> or better yet, why not be precise and iterate `terminators()` checking the `getOpcode() == TargetOpcode::INLINEASM_BR`?
It's no longer a terminator, but could be written:

```
return any_of(*this, [](const MachineInstr &MI) {
  return MI.getOpcode() == TargetOpcode::INLINEASM_BR;
}
```


================
Comment at: llvm/lib/CodeGen/MachineBasicBlock.cpp:288
 bool MachineBasicBlock::isLegalToHoistInto() const {
-  if (isReturnBlock() || hasEHPadSuccessor())
+  if (isReturnBlock() || hasEHPadSuccessor() || hasInlineAsmBr())
     return false;
----------------
Is this correct? We should be able to hoist into a block with `INLINEASM_BR` if it's coming from the default target.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp:1031
 
-  // Split after an INLINEASM_BR block with outputs. This allows us to keep the
-  // copy to/from register instructions from being between two terminator
----------------
Yay!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79794