[PATCH] D47369: [DebugInfo][ScheduleDAGInstrs] Prevent scheduler from reordering DBG_VALUE instructions through their clobbering MIs

[Andrew Zhogin via Phabricator via llvm-commits]

andrew.zhogin marked 7 inline comments as done.
andrew.zhogin added inline comments.


================
Comment at: lib/CodeGen/ScheduleDAGInstrs.cpp:76-78
+static cl::opt<bool> EnableDbgValueReattach("enable-dbg-value-reattach",
+    cl::Hidden, cl::init(true),
+    cl::desc("Enable dbg_value reattachment to the nearest clobbering MI"));
----------------
dberris wrote:
> Is there value in keeping this hidden under a flag? What is the risk of just doing this by default?
I use this flag in test to verify that the problem exists with "-enable-dbg-value-reattach=false", but not with "-enable-dbg-value-reattach=true".
This flag is enabled by default.


================
Comment at: lib/CodeGen/ScheduleDAGInstrs.cpp:701
+  assert(MI.isDebugValue());
+  assert(MI.getNumOperands() == 4);
+  // If location of variable is described using a register (directly or
----------------
dberris wrote:
> I must be missing something, but why is this assertion required here?
Just to verify correct DBG_VALUE instruction. This code was taken from DbgValueHistoryCalculator.


================
Comment at: lib/CodeGen/ScheduleDAGInstrs.cpp:718-720
+    if (unsigned reg1 = isDescribedByReg(MI))
+      if (unsigned reg2 = isDescribedByReg(dbgMI))
+      return reg1 == reg2;
----------------
dberris wrote:
> Is this a formatting error, is there something else missing here to catch the case where `reg1 != 0 && reg2 == 0`?
Formatting error fixed.  

> reg1 != 0 && reg2 == 0
It means "not clobbering", handled by final "return false;".



https://reviews.llvm.org/D47369