[PATCH] D59981: [SelectionDAGBuilder] Flush PendingExports before creating INLINEASM_BR node for asm goto.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:7924
+    // the inlineasm_br node.
+    Chain = getControlRoot();
+  }
----------------
There's currently a call to getControlRoot() in visitCallBr.  If I'm understanding correctly, the reason we would want to call it here, instead, is to make sure the relevant copytoreg nodes are scheduled before the  INLINEASM_BR, instead of after it?

I guess the reason this isn't leading to practical problems is that in most cases, a CopyToReg doesn't actually correspond to any instruction?  And in cases where it does, we get lucky and schedule it before the INLINEASM_BR even though though there's no edge that would enforce it?

If you can't construct a case where the end result of isel is actually different, it might make sense to construct a testcase that checks `-debug-only=isel` or something like that; not usually a fan of that, but it might be the best alternative here.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:8292
   // Only Update Root if inline assembly has a memory effect.
-  if (ResultValues.empty() || HasSideEffect || !OutChains.empty())
+  if (ResultValues.empty() || HasSideEffect || !OutChains.empty() || IsCallBr)
     DAG.setRoot(Chain);
----------------
Does this have any practical effect?  Nothing can actually use the root after this point, I think.  Not a big deal either way, though.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59981