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

Fri Mar 29 00:30:48 PDT 2019

chandlerc requested changes to this revision.
chandlerc added a comment.
This revision now requires changes to proceed.

While I think we should figure out how to test this, we shouldn't wait to land what seems like an important correctness fix on figuring out how to test. So mostly looking for a particular change below.

Maybe you can use synthetic asserts somewhere here to get `bugpoint` to help you reduce a test case if you have an application that manages to hit it?

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:7915-7917
   // We won't need to flush pending loads if this asm doesn't touch
   // memory and is nonvolatile.
   SDValue Flag, Chain = (HasSideEffect) ? getRoot() : DAG.getRoot();
----------------
While this predicate makes sense for why we would *semantically* need to flush loads, I think we need to flush them for `INLINEASM_BR` even if it doesn't say this because otherwise we will be at risk of disrupting the terminator layout.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59981