[DebugInfo@O2][Dexter] dbg.value's of isel vreg's can be accidentally dropped

[jmorse]

Bugzilla Link	39755
Version	trunk
OS	Linux
Blocks	#38116 #38102
CC	@dwblaikie,@gregbedwell,@CarlosAlbertoEnciso,@pogo59

Extended Description

This bug is spun out of bug 38754 to represent a regression I saw there; using llvm/clang r346686 with options "-O2 -g -fno-inline", and with CodeGenPrepare's placeDbgValues function disabled, the "blah" variable in the code below does not get a location. (It does if placeDbgValues is enabled).

--------8<--------
struct faces {
unsigned bar : 15;
unsigned baz : 15;
unsigned qux : 1;
unsigned wat;
};

void forks(faces *foo, bool cond, int someval) {
foo->wat = foo->bar;
if (someval == 4) {
unsigned blah = foo->bar;
foo->baz = 3;
if (cond)
blah |= 4;
else
blah |= 8;
foo->bar = blah;
}
}

int
main()
{
volatile int foo = 4;
bool cond = foo == 4;
faces spoon;
spoon.bar = 0;
spoon.baz = 0;
forks(&spoon, cond, foo);
return spoon.bar;
}
-------->8--------

The IR for the start of the outer conditional block in the 'forks' function, where !​24 is the DIVariable for "blah":

--------8<--------
if.then: ; preds = %entry
%1 = bitcast %struct.faces* %foo to i32*
call void @​llvm.dbg.value(metadata i32 %bf.clear, metadata !​24, [...])
%bf.clear4 = and i32 %bf.load, -1073741824
call void @​llvm.dbg.value(metadata i32 undef, metadata !​24, [...])
%blah.0 = select i1 %cond, i32 98308, i32 98312
-------->8--------

Some facts:

• The inner condition has been optimised into a single select instruction
• The stores to foo->baz and foo->bar gets merged into a single store
• The read of foo->bar at the start of the function makes the field live
  in the %bf.clear SSA register, the whole memory word lives in %bf.load
• The middle line (the "and" instruction) survives into the object file

With the IR above, I would expect that "blah" would have a location on the "and" instruction, and to then lose it afterwards. The dbg.value(undef) comes from the merged stores getting instcombined with the logic insns, leaving no single value for "blah".

Unfortunately, the first dbg.value dangles because %bf.clear isn't read until a few insns further down the IR BB, and the dbg.value(undef) then causes the first one to get dropped. That feature [0] is to avoid emitting spurious DBG_VALUEs after a new valuation is available; in this circumstance we end up dropping a legitimate dbg.value because we didn't know it was going to be legitimate yet.

IMHO, in the theme of this WIP [1] patch, we need to distinguish what's a read/write of a VReg from general SDNodes. The value being referred to (in a VReg) is already available and the DBG_VALUE could be emitted without generating spurious code, we just don't recognise it.

There's also the matter that the value of "blah" could probably be recovered from the instcombined Value, but is instead marked as undef, however that's probably a different ticket.

[0] https://github.com/llvm-mirror/llvm/blob/5018f6ea8fcd1c655d36a2ae1900e0ccee906b96/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp#L1121
[1] https://reviews.llvm.org/D54715