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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

I dont' have an application that managed to hit. I just happened to be trying to understand chains and the optimizations made in selectiondagbuilder and realized I didn't know what I was doing when I created INLINEASM_BR.

I'm not sure PendingLoads really need to be flushed. As long as nothing else between here and the branch flushes the pending loads we should be fine. Their chain outputs will just never be connected to anything. And their data output usages shoudl have already been collected by something ahead of the inlineasm_br or real br. If their data isn't used their dead anyway.

The reason invoke has a call to (void)getRoot() in the EHPad part of lowerInvokable is because at the time that was added the code basically did this for invoke

getControlRoot() to prepare to create the eh_label.
create eh_label and call DAG.setRoot()
getRoot() to create the call.

The problem was that getRoot() on the create call doesn't call DAG.getRoot() unless PendingLoads is empty. If pending loads isn't empty, it expects the root hasn't changed since the loads were created and that the loads themselves used that root in their input and so just creates a token factor of the load chain outputs and sets that as the root. So the call to getRoot to create the call overwrote the setRoot() call made by the eh_label creation causing the eh_label to be disconnected from the graph. Adding the getRoot call in front of the getControlRoot and eh_label creation fixed this by ensuring the PendingLoads was empty when the call was created. The code has been refactored several times since then and now the CallLoweringInfo object is created outside of lowerInvokable and getRoot is called there. So the (void)getRoot() inside lowerInvokable is probably unnecessary now. And the CLI.setChain(getRoot()) that occurs after creating the eh_label can just be CLI.setChain(DAG.getRoot()) since there are no pending loads to flush.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59981