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

[Krzysztof Parzyszek via Phabricator via llvm-commits]

kparzysz added inline comments.


================
Comment at: llvm/lib/Target/Hexagon/BitTracker.cpp:958
+  // FIXME: why do we need to recompute successors info in this function?
+  // Shouldn't successors() be fine as-is?
+  if (B.hasInlineAsmBr())
----------------
It would defeat the purpose of this function.  It calculates the set of possible targets for each branch, given the updated register states (i.e. branch conditions), which can be a proper subset of the set of targets listed in the branch.


================
Comment at: llvm/lib/Target/Hexagon/BitTracker.cpp:959-960
+  // Shouldn't successors() be fine as-is?
+  if (B.hasInlineAsmBr())
+    DefaultToAll = true;
+
----------------
nickdesaulniers wrote:
> ```
> DefaultToAll |= B.hasInlineAsmBr();
> ```
Ok.


================
Comment at: llvm/lib/Target/Hexagon/HexagonConstPropagation.cpp:758
+  // FIXME: why do we need to recompute successors info in this function?
+  // Shouldn't successors() be fine as-is?
+  if (B.hasInlineAsmBr())
----------------
Same reason as in BitTracker.


================
Comment at: llvm/lib/Target/Hexagon/HexagonConstPropagation.cpp:760
+  if (B.hasInlineAsmBr())
+    EvalOk = false;
+
----------------
Ok.


================
Comment at: llvm/lib/Target/Hexagon/HexagonConstPropagation.cpp:817
+// FIXME: what is this function for? It doesn't seem like we ought to need to
+// recompute the successor list from scratch?
 bool MachineConstPropagator::computeBlockSuccessors(const MachineBasicBlock *MB,
----------------
...


================
Comment at: llvm/lib/Target/Hexagon/HexagonConstPropagation.cpp:820
       SetVector<const MachineBasicBlock*> &Targets) {
+  Targets.clear();
+
----------------
Ok.


================
Comment at: llvm/lib/Target/Hexagon/HexagonConstPropagation.cpp:825
+    if (MI.getOpcode() == TargetOpcode::INLINEASM_BR)
+      return false;
     if (MI.isDebugInstr())
----------------
Ok.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79794