[PATCH] D71175: [DebugInfo][SelectionDAG] Change order while transferring SDDbgValue to another node

Wed Dec 11 09:28:52 PST 2019

jmorse added a comment.

> I found a couple of dozens of cases building clang where this patch causes SelectionDAG::transferDbgValues() to move processed debug value past another debug value of the same variable and I'm going to investigate them in details but it can take some time.
> 
> For now, I think we could conservatively let such debug values be on their places not changing their order (although it doesn't seem necessarily correct) and add corresponding TODO or FIXME note for further investigation.

Yeah, the rest of the code around SelectionDAG doesn't do a good job of keeping location intrinsics in order; I think it's acceptable to suffer a little bit more re-ordering here for an increase in available locations, for now.

> It looks easy to check if a debug value gets moved past any others of the same variable: just to iterate over SDDbgValues and to find one that in-between From and To by their order.

This is potentially expensive, particularly in ASAN builds where there are lots of variable locations. It might be worth benchmarking though in case I'm wrong.

> But there is one issue I faced that seems weird to me. I'd expect that a node should get an order that is greater than orders of its operands, but after DAGCombiner it's not true. I found lots of cases where a node has an order less than its operands. Do you know is it expected behavior?

Ah, that's interesting -- according to comments for the "IROrder" field in SelectionDAGNodes.h, that number can be used to order node output instead of using a scheduler. So I wouldn't expect to see nodes with lesser order numbers. That being said, I'm not a SelectionDAG expert, so there could be some other explanation.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71175