[PATCH] D25318: [DAG] Don't increase SDNodeOrder for dbg.value/declare.

[Mikael Holmén via Phabricator via llvm-commits]

uabelho added a comment.

Glad to see someone else also noticing this problem! :)

We've been running with this patch on our out-of-tree target for two months now without seeing
any problems with it.

I completely agree with the first two comments. I'm not sure what to do with the test case.
Any suggestions? Maybe just use the tests in https://reviews.llvm.org/D27261 instead?



================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:976
 
-  ++SDNodeOrder;
+  // Increase the SDNodeOrder if dealing with a non-debug instruction
+  if (!isa<DbgValueInst>(I) && !isa<DbgDeclareInst>(I))
----------------
aprantl wrote:
> and  a `.` at the end of the comment ;-)
Ok!


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:977
+  // Increase the SDNodeOrder if dealing with a non-debug instruction
+  if (!isa<DbgValueInst>(I) && !isa<DbgDeclareInst>(I))
+    ++SDNodeOrder;
----------------
aprantl wrote:
> fhahn wrote:
> > I think you could use  if (!isa<DbgInfoIntrinsic>(I)) here.
> This would be not just shorter but also more future-proof.
Good point, I agree!


================
Comment at: test/DebugInfo/Generic/selectiondag-order.ll:5
+; llvm.dbg.value in fn1_dbg. The generated instruction should be identical.
+; However currently we just check the first pushq.
+
----------------
aprantl wrote:
> Is there a good reason for this? Does it make the testcase more stable?
> If we only check the first instruction, sh/could we simplify the IR?
I simply didn't know how to write the test in the best way. I would like to check that the generated instructions are completely identical in the two cases since they only differ in a dbg.value in the input.

Now it's been a while since I saw this problem but I think I tried to reduce more but couldn't if I stil lwanted to see the diff in the output. Someone else might of course be more succesful though.


https://reviews.llvm.org/D25318