[DebugInfo][InstrRef] Describe value sizes when spilt to stack

[jmorse]

Summary

With exciting new variable locations from instruction referencing comes exciting new ways for locations to be broken. I've run into a sample where LLVM reduces the size of an enumeration value from 32 bits to a single byte, inserting the relevant DW_OP_convert's. Unfortunately, when the single byte is spilt to the stack we describe it's location correctly, but the fact that it's only one byte is omitted. Consumers faced with a stack location and DW_OP_deref quite naturally load 32 bits from the stack instead of 8 bits, which then corrupts the variable value displayed.

The location can be correctly described with DW_OP_deref_size. This patch seeks out the size of the value and the size of the variable, and uses DW_OP_deref_size if they don't match.

While that's fairly simple, doing this stumbles into the awkward problem of whether locations should be:

• A location expression with an implicit deref,
• A location expression with a DW_OP_stack_value,

and we're forced to commit to one of them at this point, because it affects where the DW_OP_deref{,_size} is placed. There are then several different inputs to location-production that need to be handled. The result is a four-case if/else block, which is unfortunate, but is a gnarly problem that should be designed away at a later date.

Other test changes aside from the one added: one test needs the variable size corrected to avoid spurious changes. Another test with a FIXME is fixed (live-debug-values-restore.mir), the problem was that we have to change the expression produced for a stack slot depending on whether there's already a non-empty DIExpression. For an empty expression, adding "DW_OP_uconst, 8" and becoming a location expression with implicit deref is sufficient. But if there's something else in the expression, we have to explicitly add DW_OP_deref so that the other expression operators can operate on the loaded value. Ugly, but here we are.

[jmorse]

/me squints -- looks like the test doesn't cover the case of expressions with fragments, which I'll revise in later. (The patch has a case for that).

[Orlando]

In D123599#3445402, @jmorse wrote:

/me squints -- looks like the test doesn't cover the case of expressions with fragments, which I'll revise in later. (The patch has a case for that).

Pending the promised additional test, this LGTM with a couple of nits.

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

865

nit: ValueSizeInBits instead?

897

if ValueSize > VariableSize then presumably a standard DW_OP_deref would do the job? (this distinction may only save a few bytes, so may not be worth the additional case, it was just a thought).

904

nit: Any reason to use intermediate variable for the flags here but when it's not done in the other branches?

907

nit: Looks like the else if line is indented one level too few.

llvm/test/DebugInfo/MIR/InstrRef/deref-spills-with-size.mir

169

llvm/test/DebugInfo/MIR/X86/livedebugvalues_load_in_loop.mir

55

What is this change for?

[jmorse]

Stone the crows, of the Dexter tests we have that originally detected this problem, another has found a problem in this patch -- for values defined in a stack slot, InstrRefBasedLDV currently guesses the size [0] because it can't work it out so late in compilation. In a piece of code dealing with vectors:

• The variable is a "vector" class with a single array of 4 f32's, that get promoted and vectorised it seems,
• The vectorised value is subject to a PHI at some point,
• A DBG_INSTR_REF refers to the PHI,
• This becomes a DBG_PHI referring to a stack slot after regalloc,
• The code in [0] guesses that the stack slot contains 64 bits, where in reality it's 128 bits,
• 64 bits is not the size of the variable, so a DW_OP_deref_size and DW_OP_stack_value is used, and we lose half the vector.

I think this can be solved by baking a "value size" into DBG_PHIs that refer to the stack: the register allocator knows the register class of the value spilt. We don't need to know the size of values in stack slots at any time other than when a DBG_PHI refers to it, as that's the only way DBG_INSTR_REFs can refer to stack locations. (Except for fused memory operands, which come with a size anyway...). Anyhoo, that's another patch, and more testing.

[0] https://github.com/llvm/llvm-project/blob/12a2f7494e745eb4c90133ea17cadac3a8eb8d07/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp#L1296

[jmorse]

The aforementioned problems are addressed in D124184, this patch revises the test to cover scenarios where DBG_PHIs refer to the stack, and ensure that the size operand of DBG_PHI is considered.

[jmorse]

Refreshed with a more comprehensive test plan -- see deref-spills-with-size.mir where there's a truth-table (well, kind of) of what inputs should lead to what output expressions, for:

• Different sizes of value on the stack,
• The differing presence of stack values / no stack value,
• Whether the variable is a scalar or a fragment.

It would be nice to design some of this decision making out of DIExpression in the future, but that's for another time. This will want a little more review, as I've made one further change. For fragments with no size mistmatch, we might be able to use a DW_OP_deref and let the consumer work out "DW_OP_piece says 4 bytes, so we should dereference a 4 byte value". However it's better to be explicit and use a DW_OP_deref_size rather than leaving it to interpretation.

[jmorse]

(Addressing inline comments -- note that they've now shifted down a few lines).

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

897

I think in that case we get into questions about endian-ness -- it's alright on little endian I think, but not large. Rather than trying to think about that, IMO it's better to be explicit.

904

Uuuhhhh. No, purely a line wrapping consideration. I can pick one and stick to it if you've got an opinion on which way it should be.

llvm/test/DebugInfo/MIR/X86/livedebugvalues_load_in_loop.mir

55

I usually copy-and-paste metadata from other tests to avoid trimming it all over again, and then join it up with the IR / MIR that I want to test. Usually the size of the variable doesn't matter -- however

[jmorse]

llvm/test/DebugInfo/MIR/X86/livedebugvalues_load_in_loop.mir

55

... for this test it does, and so I've corrected the variable size to be 64 bits, which is how the variable is used below.

[Orlando]

LGTM. It's great to have the comprehensive test spelling out the expectations for all the combinations of fragment / deref / indirect.

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

895–896

nit: if (auto Fragment = Var.getFragment()) more idiomatic?

904

nit: I think adding InBits / InBytes to these variable names would improve readability here

llvm/test/DebugInfo/MIR/InstrRef/deref-spills-with-size.mir

270

338

[jmorse]

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

895–896

Yup, will fold in,

904

Yup, will fold in.

[zequanwu]

Hi, this seems causing crashes on our windows/mac/ios bots: https://bugs.chromium.org/p/chromium/issues/detail?id=1320808

[zequanwu]

The compressed linker repro is 4.3GB, so it's hard to upload.
here is better backtrace. Hope that helps:

Assertion failed: !Expr->isComplex(), file ../../llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp, line 907
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.      Running pass 'Function Pass Manager' on module 'obj/third_party/blink/renderer/platform\platform/open_type_math_support.obj'.
1.      Running pass 'Live DEBUG_VALUE analysis' on function '@"??$GetHarfBuzzMathRecord@Uhb_ot_math_glyph_variant_t@@G@blink@@YA?AV?$Vector@G$0A@VPartitionAllocator@WTF@@@WTF@@PEBVHarfBuzzFace@0@GW4StretchAxis@OpenTypeMathStretchData@0@V?$OnceCallback@$$A6AIPEAUhb_font_t@@IW4hb_direction_t@@IPEAIPEAUhb_ot_math_glyph_variant_t@@@Z@base@@V?$RepeatingCallback@$$A6AGUhb_ot_math_glyph_variant_t@@@Z@7@V?$optional@G@absl@@@Z"'
 #0 0x00007ff79a1f2920 HandleAbort C:\src\llvm-project\llvm\lib\Support\Windows\Signals.inc:418:0
 #1 0x00007ff79da2cf53 raise C:\src\llvm-project\build\debug\minkernel\crts\ucrt\src\appcrt\misc\signal.cpp:547:0
 #2 0x00007ff79da1de3c abort C:\src\llvm-project\build\debug\minkernel\crts\ucrt\src\appcrt\startup\abort.cpp:71:0
 #3 0x00007ff79da1ed79 common_assert_to_stderr_direct C:\src\llvm-project\build\debug\minkernel\crts\ucrt\src\appcrt\startup\assert.cpp:160:0
 #4 0x00007ff79da1f05c common_assert_to_stderr<wchar_t> C:\src\llvm-project\build\debug\minkernel\crts\ucrt\src\appcrt\startup\assert.cpp:179:0
 #5 0x00007ff79da1ebce _wassert C:\src\llvm-project\build\debug\minkernel\crts\ucrt\src\appcrt\startup\assert.cpp:443:0
 #6 0x00007ff79d23175c LiveDebugValues::MLocTracker::emitLoc(class llvm::Optional<class LiveDebugValues::LocIdx>, class llvm::DebugVariable const &, class LiveDebugValues::DbgValueProperties const &) C:\src\llvm-project\llvm\lib\CodeGen\LiveDebugValues\InstrRefBasedImpl.cpp:908:0
 #7 0x00007ff79d256f33 TransferTracker::transferMlocs(class LiveDebugValues::LocIdx, class LiveDebugValues::LocIdx, class llvm::MachineInstrBundleIterator<class llvm::MachineInstr, 0>) C:\src\llvm-project\llvm\lib\CodeGen\LiveDebugValues\InstrRefBasedImpl.cpp:620:0
 #8 0x00007ff79d236839 LiveDebugValues::InstrRefBasedLDV::transferSpillOrRestoreInst::<lambda_0>::operator() C:\src\llvm-project\llvm\lib\CodeGen\LiveDebugValues\InstrRefBasedImpl.cpp:1641:0
 #9 0x00007ff79d23628a LiveDebugValues::InstrRefBasedLDV::transferSpillOrRestoreInst(class llvm::MachineInstr &) C:\src\llvm-project\llvm\lib\CodeGen\LiveDebugValues\InstrRefBasedImpl.cpp:1656:0
#10 0x00007ff79d23755a LiveDebugValues::InstrRefBasedLDV::process(class llvm::MachineInstr &, class std::unique_ptr<class LiveDebugValues::ValueIDNum[], struct std::default_delete<class LiveDebugValues::ValueIDNum[]>> const *, class std::unique_ptr<class LiveDebugValues::ValueIDNum[], struct std::default_delete<class LiveDebugValues::ValueIDNum[]>> const *) C:\src\llvm-project\llvm\lib\CodeGen\LiveDebugValues\InstrRefBasedImpl.cpp:1826:0
#11 0x00007ff79d23e3a4 LiveDebugValues::InstrRefBasedLDV::depthFirstVLocAndEmit::<lambda_0>::operator() C:\src\llvm-project\llvm\lib\CodeGen\LiveDebugValues\InstrRefBasedImpl.cpp:3015:0
#12 0x00007ff79d23e01e LiveDebugValues::InstrRefBasedLDV::depthFirstVLocAndEmit(unsigned int, class llvm::DenseMap<class llvm::LexicalScope const *, class llvm::DILocation const *, struct llvm::DenseMapInfo<class llvm::LexicalScope const *, void>, struct llvm::detail::DenseMapPair<class llvm::LexicalScope const *, class llvm::DILocation const *>> const &, class llvm::DenseMap<class llvm::LexicalScope const *, class llvm::SmallSet<class llvm::DebugVariable, 4, struct std::less<class llvm::DebugVariable>>, struct llvm::DenseMapInfo<class llvm::LexicalScope const *, void>, struct llvm::detail::DenseMapPair<class llvm::LexicalScope const *, class llvm::SmallSet<class llvm::DebugVariable, 4, struct std::less<class llvm::DebugVariable>>>> const &, class llvm::DenseMap<class llvm::LexicalScope const *, class llvm::SmallPtrSet<class llvm::MachineBasicBlock *, 4>, struct llvm::DenseMapInfo<class llvm::LexicalScope const *, void>, struct llvm::detail::DenseMapPair<class llvm::LexicalScope const *, class llvm::SmallPtrSet<class llvm::MachineBasicBlock *, 4>>> &, class llvm::SmallVector<class llvm::SmallVector<struct std::pair<class llvm::DebugVariable, class LiveDebugValues::DbgValue>, 8>, 8> &, class std::unique_ptr<class std::unique_ptr<class LiveDebugValues::ValueIDNum[], struct std::default_delete<class LiveDebugValues::ValueIDNum[]>>[], struct std::default_delete<class std::unique_ptr<class LiveDebugValues::ValueIDNum[], struct std::default_delete<class LiveDebugValues::ValueIDNum[]>>[]>> &, class std::unique_ptr<class std::unique_ptr<class LiveDebugValues::ValueIDNum[], struct std::default_delete<class LiveDebugValues::ValueIDNum[]>>[], struct std::default_delete<class std::unique_ptr<class LiveDebugValues::ValueIDNum[], struct std::default_delete<class LiveDebugValues::ValueIDNum[]>>[]>> &, class llvm::SmallVectorImpl<class LiveDebugValues::VLocTracker> &, class llvm::MachineFunction &, class llvm::DenseMap<class llvm::DebugVariable, unsigned int, struct llvm::DenseMapInfo<class llvm::DebugVariable, void>, struct llvm::detail::DenseMapPair<class llvm::DebugVariable, unsigned int>> &, class llvm::TargetPassConfig const &) C:\src\llvm-project\llvm\lib\CodeGen\LiveDebugValues\InstrRefBasedImpl.cpp:3073:0
#13 0x00007ff79d23f7c9 LiveDebugValues::InstrRefBasedLDV::ExtendRanges(class llvm::MachineFunction &, class llvm::MachineDominatorTree *, class llvm::TargetPassConfig *, unsigned int, unsigned int) C:\src\llvm-project\llvm\lib\CodeGen\LiveDebugValues\InstrRefBasedImpl.cpp:3289:0
#14 0x00007ff79bc5dd02 `anonymous namespace'::LiveDebugValues::runOnMachineFunction C:\src\llvm-project\llvm\lib\CodeGen\LiveDebugValues\LiveDebugValues.cpp:123:0
#15 0x00007ff79a88a70d llvm::MachineFunctionPass::runOnFunction(class llvm::Function &) C:\src\llvm-project\llvm\lib\CodeGen\MachineFunctionPass.cpp:73:0
#16 0x00007ff79ab02e52 llvm::FPPassManager::runOnFunction(class llvm::Function &) C:\src\llvm-project\llvm\lib\IR\LegacyPassManager.cpp:1430:0
#17 0x00007ff79ab0808e llvm::FPPassManager::runOnModule(class llvm::Module &) C:\src\llvm-project\llvm\lib\IR\LegacyPassManager.cpp:1476:0
#18 0x00007ff79ab03810 `anonymous namespace'::MPPassManager::runOnModule C:\src\llvm-project\llvm\lib\IR\LegacyPassManager.cpp:1545:0
#19 0x00007ff79ab032f0 llvm::legacy::PassManagerImpl::run(class llvm::Module &) C:\src\llvm-project\llvm\lib\IR\LegacyPassManager.cpp:535:0
#20 0x00007ff79ab08361 llvm::legacy::PassManager::run(class llvm::Module &) C:\src\llvm-project\llvm\lib\IR\LegacyPassManager.cpp:1672:0
#21 0x00007ff79a4cfda0 codegen C:\src\llvm-project\llvm\lib\LTO\LTOBackend.cpp:392:0
#22 0x00007ff79a4d09ea llvm::lto::thinBackend::<lambda_0>::operator() C:\src\llvm-project\llvm\lib\LTO\LTOBackend.cpp:559:0
#23 0x00007ff79a4d081d llvm::lto::thinBackend(struct llvm::lto::Config const &, unsigned int, class std::function<(unsigned int)>, class llvm::Module &, class llvm::ModuleSummaryIndex const &, class llvm::StringMap<class std::unordered_set<unsigned __int64, struct std::hash<unsigned __int64>, struct std::equal_to<unsigned __int64>, class std::allocator<unsigned __int64>>, class llvm::MallocAllocator> const &, class llvm::DenseMap<unsigned __int64, class llvm::GlobalValueSummary *, struct llvm::DenseMapInfo<unsigned __int64, void>, struct llvm::detail::DenseMapPair<unsigned __int64, class llvm::GlobalValueSummary *>> const &, class llvm::MapVector<class llvm::StringRef, class llvm::BitcodeModule, class llvm::DenseMap<class llvm::StringRef, unsigned int, struct llvm::DenseMapInfo<class llvm::StringRef, void>, struct llvm::detail::DenseMapPair<class llvm::StringRef, unsigned int>>, class std::vector<struct std::pair<class llvm::StringRef, class llvm::BitcodeModule>, class std::allocator<struct std::pair<class llvm::StringRef, class llvm::BitcodeModule>>>> *, class std::vector<unsigned char, class std::allocator<unsigned char>> const &) C:\src\llvm-project\llvm\lib\LTO\LTOBackend.cpp:630:0
#24 0x00007ff799d02c5d `anonymous namespace'::InProcessThinBackend::runThinLTOBackendThread::<lambda_1>::operator() C:\src\llvm-project\llvm\lib\LTO\LTO.cpp:1223:0
#25 0x00007ff799d029aa `anonymous namespace'::InProcessThinBackend::runThinLTOBackendThread C:\src\llvm-project\llvm\lib\LTO\LTO.cpp:1246:0
#26 0x00007ff799d02488 `anonymous namespace'::InProcessThinBackend::start::<lambda_1>::operator() C:\src\llvm-project\llvm\lib\LTO\LTO.cpp:1275:0
#27 0x00007ff799d022b4 std::invoke<`lambda at ../../llvm/lib/LTO/LTO.cpp:1262:9' &,llvm::BitcodeModule &,llvm::ModuleSummaryIndex &,const llvm::StringMap<std::unordered_set<unsigned long long,std::hash<unsigned long long>,std::equal_to<unsigned long long>,std::allocator<unsigned long long> >,llvm::MallocAllocator> &,const llvm::DenseSet<llvm::ValueInfo,llvm::DenseMapInfo<llvm::ValueInfo,void> > &,const std::map<unsigned long long,llvm::GlobalValue::LinkageTypes,std::less<unsigned long long>,std::allocator<std::pair<const unsigned long long,llvm::GlobalValue::LinkageTypes> > > &,const llvm::DenseMap<unsigned long long,llvm::GlobalValueSummary *,llvm::DenseMapInfo<unsigned long long,void>,llvm::detail::DenseMapPair<unsigned long long,llvm::GlobalValueSummary *> > &,llvm::MapVector<llvm::StringRef,llvm::BitcodeModule,llvm::DenseMap<llvm::StringRef,unsigned int,llvm::DenseMapInfo<llvm::StringRef,void>,llvm::detail::DenseMapPair<llvm::StringRef,unsigned int> >,std::vector<std::pair<llvm::StringRef,llvm::BitcodeModule>,std::allocator<std::pair<llvm::StringRef,llvm::BitcodeModule> > > > &> C:\src\llvm-project\chrome-sysroot\VC\Tools\MSVC\14.29.30133\include\type_traits:1595:0
#28 0x00007ff799d021a6 std::_Invoker_ret<std::_Unforced,0>::_Call<`lambda at ../../llvm/lib/LTO/LTO.cpp:1262:9' &,llvm::BitcodeModule &,llvm::ModuleSummaryIndex &,const llvm::StringMap<std::unordered_set<unsigned long long,std::hash<unsigned long long>,std::equal_to<unsigned long long>,std::allocator<unsigned long long> >,llvm::MallocAllocator> &,const llvm::DenseSet<llvm::ValueInfo,llvm::DenseMapInfo<llvm::ValueInfo,void> > &,const std::map<unsigned long long,llvm::GlobalValue::LinkageTypes,std::less<unsigned long long>,std::allocator<std::pair<const unsigned long long,llvm::GlobalValue::LinkageTypes> > > &,const llvm::DenseMap<unsigned long long,llvm::GlobalValueSummary *,llvm::DenseMapInfo<unsigned long long,void>,llvm::detail::DenseMapPair<unsigned long long,llvm::GlobalValueSummary *> > &,llvm::MapVector<llvm::StringRef,llvm::BitcodeModule,llvm::DenseMap<llvm::StringRef,unsigned int,llvm::DenseMapInfo<llvm::StringRef,void>,llvm::detail::DenseMapPair<llvm::StringRef,unsigned int> >,std::vector<std::pair<llvm::StringRef,llvm::BitcodeModule>,std::allocator<std::pair<llvm::StringRef,llvm::BitcodeModule> > > > &> C:\src\llvm-project\chrome-sysroot\VC\Tools\MSVC\14.29.30133\include\functional:763:0
#29 0x00007ff799d02100 std::_Call_binder<std::_Unforced,0,1,2,3,4,5,6,`lambda at ../../llvm/lib/LTO/LTO.cpp:1262:9',std::tuple<llvm::BitcodeModule,std::reference_wrapper<llvm::ModuleSummaryIndex>,std::reference_wrapper<const llvm::StringMap<std::unordered_set<unsigned long long,std::hash<unsigned long long>,std::equal_to<unsigned long long>,std::allocator<unsigned long long> >,llvm::MallocAllocator> >,std::reference_wrapper<const llvm::DenseSet<llvm::ValueInfo,llvm::DenseMapInfo<llvm::ValueInfo,void> > >,std::reference_wrapper<const std::map<unsigned long long,llvm::GlobalValue::LinkageTypes,std::less<unsigned long long>,std::allocator<std::pair<const unsigned long long,llvm::GlobalValue::LinkageTypes> > > >,std::reference_wrapper<const llvm::DenseMap<unsigned long long,llvm::GlobalValueSummary *,llvm::DenseMapInfo<unsigned long long,void>,llvm::detail::DenseMapPair<unsigned long long,llvm::GlobalValueSummary *> > >,std::reference_wrapper<llvm::MapVector<llvm::StringRef,llvm::BitcodeModule,llvm::DenseMap<llvm::StringRef,unsigned int,llvm::DenseMapInfo<llvm::StringRef,void>,llvm::detail::DenseMapPair<llvm::StringRef,unsigned int> >,std::vector<std::pair<llvm::StringRef,llvm::BitcodeModule>,std::allocator<std::pair<llvm::StringRef,llvm::BitcodeModule> > > > > >,std::tuple<> > C:\src\llvm-project\chrome-sysroot\VC\Tools\MSVC\14.29.30133\include\functional:1414:0
#30 0x00007ff799d01f31 std::_Binder<std::_Unforced,`lambda at ../../llvm/lib/LTO/LTO.cpp:1262:9',llvm::BitcodeModule &,std::reference_wrapper<llvm::ModuleSummaryIndex>,std::reference_wrapper<const llvm::StringMap<std::unordered_set<unsigned long long,std::hash<unsigned long long>,std::equal_to<unsigned long long>,std::allocator<unsigned long long> >,llvm::MallocAllocator> >,std::reference_wrapper<const llvm::DenseSet<llvm::ValueInfo,llvm::DenseMapInfo<llvm::ValueInfo,void> > >,std::reference_wrapper<const std::map<unsigned long long,llvm::GlobalValue::LinkageTypes,std::less<unsigned long long>,std::allocator<std::pair<const unsigned long long,llvm::GlobalValue::LinkageTypes> > > >,std::reference_wrapper<const llvm::DenseMap<unsigned long long,llvm::GlobalValueSummary *,llvm::DenseMapInfo<unsigned long long,void>,llvm::detail::DenseMapPair<unsigned long long,llvm::GlobalValueSummary *> > >,std::reference_wrapper<llvm::MapVector<llvm::StringRef,llvm::BitcodeModule,llvm::DenseMap<llvm::StringRef,unsigned int,llvm::DenseMapInfo<llvm::StringRef,void>,llvm::detail::DenseMapPair<llvm::StringRef,unsigned int> >,std::vector<std::pair<llvm::StringRef,llvm::BitcodeModule>,std::allocator<std::pair<llvm::StringRef,llvm::BitcodeModule> > > > > >::operator()<> C:\src\llvm-project\chrome-sysroot\VC\Tools\MSVC\14.29.30133\include\functional:1452:0
#31 0x00007ff799d01ec3 std::invoke<std::_Binder<std::_Unforced,`lambda at ../../llvm/lib/LTO/LTO.cpp:1262:9',llvm::BitcodeModule &,std::reference_wrapper<llvm::ModuleSummaryIndex>,std::reference_wrapper<const llvm::StringMap<std::unordered_set<unsigned long long,std::hash<unsigned long long>,std::equal_to<unsigned long long>,std::allocator<unsigned long long> >,llvm::MallocAllocator> >,std::reference_wrapper<const llvm::DenseSet<llvm::ValueInfo,llvm::DenseMapInfo<llvm::ValueInfo,void> > >,std::reference_wrapper<const std::map<unsigned long long,llvm::GlobalValue::LinkageTypes,std::less<unsigned long long>,std::allocator<std::pair<const unsigned long long,llvm::GlobalValue::LinkageTypes> > > >,std::reference_wrapper<const llvm::DenseMap<unsigned long long,llvm::GlobalValueSummary *,llvm::DenseMapInfo<unsigned long long,void>,llvm::detail::DenseMapPair<unsigned long long,llvm::GlobalValueSummary *> > >,std::reference_wrapper<llvm::MapVector<llvm::StringRef,llvm::BitcodeModule,llvm::DenseMap<llvm::StringRef,unsigned int,llvm::DenseMapInfo<llvm::StringRef,void>,llvm::detail::DenseMapPair<llvm::StringRef,unsigned int> >,std::vector<std::pair<llvm::StringRef,llvm::BitcodeModule>,std::allocator<std::pair<llvm::StringRef,llvm::BitcodeModule> > > > > > &> C:\src\llvm-project\chrome-sysroot\VC\Tools\MSVC\14.29.30133\include\type_traits:1585:0
#32 0x00007ff799d01ea3 std::_Invoker_ret<void,1>::_Call<std::_Binder<std::_Unforced,`lambda at ../../llvm/lib/LTO/LTO.cpp:1262:9',llvm::BitcodeModule &,std::reference_wrapper<llvm::ModuleSummaryIndex>,std::reference_wrapper<const llvm::StringMap<std::unordered_set<unsigned long long,std::hash<unsigned long long>,std::equal_to<unsigned long long>,std::allocator<unsigned long long> >,llvm::MallocAllocator> >,std::reference_wrapper<const llvm::DenseSet<llvm::ValueInfo,llvm::DenseMapInfo<llvm::ValueInfo,void> > >,std::reference_wrapper<const std::map<unsigned long long,llvm::GlobalValue::LinkageTypes,std::less<unsigned long long>,std::allocator<std::pair<const unsigned long long,llvm::GlobalValue::LinkageTypes> > > >,std::reference_wrapper<const llvm::DenseMap<unsigned long long,llvm::GlobalValueSummary *,llvm::DenseMapInfo<unsigned long long,void>,llvm::detail::DenseMapPair<unsigned long long,llvm::GlobalValueSummary *> > >,std::reference_wrapper<llvm::MapVector<llvm::StringRef,llvm::BitcodeModule,llvm::DenseMap<llvm::StringRef,unsigned int,llvm::DenseMapInfo<llvm::StringRef,void>,llvm::detail::DenseMapPair<llvm::StringRef,unsigned int> >,std::vector<std::pair<llvm::StringRef,llvm::BitcodeModule>,std::allocator<std::pair<llvm::StringRef,llvm::BitcodeModule> > > > > > &> C:\src\llvm-project\chrome-sysroot\VC\Tools\MSVC\14.29.30133\include\functional:745:0
#33 0x00007ff799d01c97 std::_Func_impl_no_alloc<std::_Binder<std::_Unforced,`lambda at ../../llvm/lib/LTO/LTO.cpp:1262:9',llvm::BitcodeModule &,std::reference_wrapper<llvm::ModuleSummaryIndex>,std::reference_wrapper<const llvm::StringMap<std::unordered_set<unsigned long long,std::hash<unsigned long long>,std::equal_to<unsigned long long>,std::allocator<unsigned long long> >,llvm::MallocAllocator> >,std::reference_wrapper<const llvm::DenseSet<llvm::ValueInfo,llvm::DenseMapInfo<llvm::ValueInfo,void> > >,std::reference_wrapper<const std::map<unsigned long long,llvm::GlobalValue::LinkageTypes,std::less<unsigned long long>,std::allocator<std::pair<const unsigned long long,llvm::GlobalValue::LinkageTypes> > > >,std::reference_wrapper<const llvm::DenseMap<unsigned long long,llvm::GlobalValueSummary *,llvm::DenseMapInfo<unsigned long long,void>,llvm::detail::DenseMapPair<unsigned long long,llvm::GlobalValueSummary *> > >,std::reference_wrapper<llvm::MapVector<llvm::StringRef,llvm::BitcodeModule,llvm::DenseMap<llvm::StringRef,unsigned int,llvm::DenseMapInfo<llvm::StringRef,void>,llvm::detail::DenseMapPair<llvm::StringRef,unsigned int> >,std::vector<std::pair<llvm::StringRef,llvm::BitcodeModule>,std::allocator<std::pair<llvm::StringRef,llvm::BitcodeModule> > > > > >,void>::_Do_call C:\src\llvm-project\chrome-sysroot\VC\Tools\MSVC\14.29.30133\include\functional:920:0
#34 0x00007ff799b4fd34 std::_Func_class<void>::operator()(void) const C:\src\llvm-project\chrome-sysroot\VC\Tools\MSVC\14.29.30133\include\functional:968:0
#35 0x00007ff799d3a5ec `private: static struct std::pair<class std::function<void __cdecl(void)>, class std::future<void>> __cdecl llvm::ThreadPool::createTaskAndFuture(class std::function<void __cdecl(void)>)'::`1'::<lambda_1>::operator()(void) const C:\src\llvm-project\llvm\include\llvm\Support\ThreadPool.h:95:0
#36 0x00007ff799d3a5c3 std::invoke<class `private: static struct std::pair<class std::function<void __cdecl(void)>, class std::future<void>> __cdecl llvm::ThreadPool::createTaskAndFuture(class std::function<void __cdecl(void)>)'::`1'::<lambda_1> &>(class `private: static struct std::pair<class std::function<void __cdecl(void)>, class std::future<void>> __cdecl llvm::ThreadPool::createTaskAndFuture(class std::function<void __cdecl(void)>)'::`1'::<lambda_1> &) C:\src\llvm-project\chrome-sysroot\VC\Tools\MSVC\14.29.30133\include\type_traits:1585:0
#37 0x00007ff799d3a5a3 std::_Invoker_ret<void, 1>::_Call<class `private: static struct std::pair<class std::function<void __cdecl(void)>, class std::future<void>> __cdecl llvm::ThreadPool::createTaskAndFuture(class std::function<void __cdecl(void)>)'::`1'::<lambda_1> &>(class `private: static struct std::pair<class std::function<void __cdecl(void)>, class std::future<void>> __cdecl llvm::ThreadPool::createTaskAndFuture(class std::function<void __cdecl(void)>)'::`1'::<lambda_1> &) C:\src\llvm-project\chrome-sysroot\VC\Tools\MSVC\14.29.30133\include\functional:745:0
#38 0x00007ff799d3a2f7 std::_Func_impl_no_alloc<class `private: static struct std::pair<class std::function<void __cdecl(void)>, class std::future<void>> __cdecl llvm::ThreadPool::createTaskAndFuture(class std::function<void __cdecl(void)>)'::`1'::<lambda_1>, void>::_Do_call(void) C:\src\llvm-project\chrome-sysroot\VC\Tools\MSVC\14.29.30133\include\functional:920:0
#39 0x00007ff799b4fd34 std::_Func_class<void>::operator()(void) const C:\src\llvm-project\chrome-sysroot\VC\Tools\MSVC\14.29.30133\include\functional:968:0
#40 0x00007ff79a5fba7c llvm::ThreadPool::grow::<lambda_0>::operator() C:\src\llvm-project\llvm\lib\Support\ThreadPool.cpp:59:0
#41 0x00007ff79a5fb93f llvm::thread::Apply<`lambda at ../../llvm/lib/Support/ThreadPool.cpp:37:26'> C:\src\llvm-project\llvm\include\llvm\Support\thread.h:43:0
#42 0x00007ff79a5fb8f7 llvm::thread::GenericThreadProxy<std::tuple<`lambda at ../../llvm/lib/Support/ThreadPool.cpp:37:26'> > C:\src\llvm-project\llvm\include\llvm\Support\thread.h:51:0
#43 0x00007ff79a5fb773 llvm::thread::ThreadProxy<std::tuple<`lambda at ../../llvm/lib/Support/ThreadPool.cpp:37:26'> > C:\src\llvm-project\llvm\include\llvm\Support\thread.h:71:0
#44 0x00007ff79da1569e thread_start<unsigned int (__cdecl*)(void *),1> C:\src\llvm-project\build\debug\minkernel\crts\ucrt\src\appcrt\startup\thread.cpp:97:0
#45 0x00007ffdf8087034 (C:\Windows\System32\KERNEL32.DLL+0x17034)
#46 0x00007ffdf8ce2651 (C:\Windows\SYSTEM32\ntdll.dll+0x52651)

[jmorse]

Thanks for reverting; I think this has uncovered some changes in my assumptions of how some variable locations are described. Would you happen to know whether the source being compiled contains coroutines? There's been some work there lately which might have had an effect.

[jmorse]

To explain my thinking -- coroutines recently started using dbg.addr, and can be salvaged in 0b647fc529, which will create complex DIExpressions (aka anything with a DW_OP_stack_value). I think this might be the first scenario where dbg.addrs are generated and can be salvaged, which then messes up other code I've written in SelectionDAG, that assumes a dbg.addr can be treated as a more specific dbg.declare. If so, the fix is probably to flush the indirectness out of the intrinsics/instructions at SelectionDAG, seeing how everyone wants the IsIndirect flag gone in the long run.

[jmorse]

Thinking about it, actually we can easily support the llvm.dbg.addr with complex-expression scenario, just by weakening the assertion that's firing. The code as written will cope just fine with dbg.addrs that perform a computation, the true problem is when there's a DW_OP_stack_value in the expression. That's what this assertion should have been testing for in the first place. It doesn't make sense for a variable to both be indirect, and computed on the DWARF expression stack.

I've revised the patch to weaken the assertion, and added a test to cover the situation I described, an indirect DBG_VALUE with extra opcodes, but no stack value. There are two dereferences in this situation, and we need to make sure they're performed at the right time.

Paging @Orlando , if you could review the update.

[zequanwu]

A smaller linker repro (133MB) is at https://drive.google.com/file/d/1nMOBkvu2Ii0ja6MHDPRSjDcvkCrH1rZa/view.

[jmorse]

Thanks for the reproducer; while it's mildly hard to interpret what's going on, I think a large block of local stack memory that's the subject of a dbg.declare, is being recorded by a

DBG_VALUE %fixed.stack.0, 0, !123, !DIExpression()

In the entry block, rather than being put in the side-table. That then gets translated by the prolog/epilog pass into an indirect DBG_VALUE with an offset added, which then hits this assertion. Happily, I think this too falls into the category of "the assertion is over-zealous" I described above, and can be handled just by the modifications I've made in the revised patch. Woo.

[Orlando]

The latest change makes sense, LGTM

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h

public:
   LocIdx getSpillMLoc(unsigned SpillID) {
     assert(LocIDToLocIdx[SpillID] != UINT_MAX); // Sentinal for IndexedMap.
     return LocIDToLocIdx[SpillID];
   }
 
   /// Return true if Idx is a spill machine location.
   bool isSpill(LocIdx Idx) const { return LocIdxToLocID[Idx] >= NumRegs; }
 
+  /// How large is this location (aka, how wide is a value defined there?).
+  unsigned getLocSizeInBits(LocIdx L) const {
+    unsigned ID = LocIdxToLocID[L];
+    if (!isSpill(L)) {
+      return TRI.getRegSizeInBits(Register(ID), MF.getRegInfo());
+    } else {
+      // The slot location on the stack is uninteresting, we care about the
+      // position of the value within the slot (which comes with a size).
+      StackSlotPos Pos = locIDToSpillIdx(ID);
+      return Pos.first;
+   }
+  }
+
   MLocIterator begin() { return MLocIterator(LocIdxToIDNum, 0); }
 
   MLocIterator end() {
     return MLocIterator(LocIdxToIDNum, LocIdxToIDNum.size());
   }
 
   /// Return a range over all locations currently tracked.
   iterator_range<MLocIterator> locations() {

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

if (!MLoc) {
     MIB.addReg(0);
     MIB.addReg(0);
   } else if (LocIdxToLocID[*MLoc] >= NumRegs) {
     unsigned LocID = LocIdxToLocID[*MLoc];
     SpillLocationNo SpillID = locIDToSpill(LocID);
     StackSlotPos StackIdx = locIDToSpillIdx(LocID);
     unsigned short Offset = StackIdx.second;
 
+    // The value in this stack slot might not be the same size as the variable
+    // being accessed -- this needs to be encoded in the expression.
+    Optional<unsigned> VariableSize = None;
+    if (Var.getFragment()) {
+      VariableSize = Var.getFragment()->SizeInBits;
+    } else if (auto Size = Var.getVariable()->getSizeInBits()) {
+      VariableSize = *Size;
+    }
+    unsigned ValueSize = getLocSizeInBits(*MLoc);
+
     // TODO: support variables that are located in spill slots, with non-zero
     // offsets from the start of the spill slot. It would require some more
     // complex DIExpression calculations. This doesn't seem to be produced by
     // LLVM right now, so don't try and support it.
     // Accept no-subregister slots and subregisters where the offset is zero.
     // The consumer should already have type information to work out how large
     // the variable is.
     if (Offset == 0) {
       const SpillLoc &Spill = SpillLocs[SpillID.id()];
-      Expr = TRI.prependOffsetExpression(Expr, DIExpression::ApplyOffset,
-                                         Spill.SpillOffset);
       unsigned Base = Spill.SpillBase;
       MIB.addReg(Base);
-      MIB.addImm(0);
 
-      // Being on the stack makes this location indirect; if it was _already_
-      // indirect though, we need to add extra indirection. See this test for
-      // a scenario where this happens:
-      //     llvm/test/DebugInfo/X86/spill-nontrivial-param.ll
-      if (Properties.Indirect) {
-        std::vector<uint64_t> Elts = {dwarf::DW_OP_deref};
-        Expr = DIExpression::append(Expr, Elts);
+      // There are several ways we can dereference things, and several inputs
+      // to consider:
+      // * NRVO variables will appear with IsIndirect set, but should have
+      //   nothing else in their DIExpressions,
+      // * Variables with DW_OP_stack_value in their expr already need an
+      //   explicit dereference of the stack location,
+      // * Values that don't match the variable size need DW_OP_deref_size,
+      // * Everything else can just become a simple location expression.
+
+        // This is something like an NRVO variable, where the pointer has been
+        // spilt to the stack. It should end up being a memory location, with
+        // the pointer to the variable loaded off the stack with a deref:
+        assert(!Expr->isComplex());
+        Expr = TRI.prependOffsetExpression(Expr, DIExpression::ApplyOffset |
+                                           DIExpression::DerefAfter,
+                                           Spill.SpillOffset);
+        MIB.addImm(0);
+      } else if (VariableSize && *VariableSize != ValueSize) {
+        // We're loading a value off the stack that's not the same size as the
+        // variable. Add / subtract stack offset, explicitly deref with a size,
+        // and add DW_OP_stack_value if not already present.
+        assert((ValueSize % 8) == 0);
+        SmallVector<uint64_t, 2> Ops = {dwarf::DW_OP_deref_size, ValueSize / 8};
+        Expr = DIExpression::prependOpcodes(Expr, Ops, true);
+        unsigned Flags = DIExpression::StackValue | DIExpression::ApplyOffset;
+        Expr = TRI.prependOffsetExpression(Expr, Flags, Spill.SpillOffset);
+        MIB.addReg(0);
+    } else if (Expr->isComplex()) {
+        // A variable with no size ambiguity, but with extra elements in it's
+        // expression. Manually dereference the stack location.
+        assert(Expr->isComplex());
+        Expr = TRI.prependOffsetExpression(Expr, DIExpression::ApplyOffset |
+                                           DIExpression::DerefAfter,
+                                           Spill.SpillOffset);
+        MIB.addReg(0);
+      } else {
+        // A plain value that has been spilt to the stack, with no further
+        // context. Request a location expression, marking the DBG_VALUE as
+        // IsIndirect.
+        Expr = TRI.prependOffsetExpression(Expr, DIExpression::ApplyOffset,
+                                           Spill.SpillOffset);
+        MIB.addImm(0);
       }
     } else {
       // This is a stack location with a weird subregister offset: emit an undef
       // DBG_VALUE instead.
       MIB.addReg(0);
       MIB.addReg(0);
     }
   } else {

llvm/test/DebugInfo/MIR/InstrRef/deref-spills-with-size.mir

+# RUN: llc %s -o - -experimental-debug-variable-locations=true \
+# RUN:   -run-pass=livedebugvalues \
+# RUN: | FileCheck %s --implicit-check-not=DBG_VALUE
+# RUN: llc %s -o - -experimental-debug-variable-locations=true \
+# RUN:   -start-before=livedebugvalues -filetype=obj \
+# RUN: | llvm-dwarfdump - | FileCheck %s --check-prefix=DWARF
+#
+# LLVM can produce DIExpressions that convert from one value of arbitrary size
+# to another. This is normally fine, however that means the value for a
+# variable tracked in instruction referencing might not be the same size as the
+# variable itself.
+#
+# This becomes a problem when values move onto the stack and we emit
+# DW_OP_deref: there is no information about how large a value the consumer
+# should load from the stack. The convention today appears to be the size of
+# the variable, and as a result if you have a value that's smaller than the
+# variable size, extra information is loaded onto the DWARF expression stack.
+# This then appears as a false variable value to the consumer.
+#
+# In this test, take a 32 bit variable and point it at an 8 bit register, which
+# is then spilt. Ensure that DW_OP_deref_size is used, rather than a plain
+# DW_OP_deref which is open to interpretation.
+#
+## Capture variable num,
+# CHECK: ![[VARNUM:[0-9]+]] = !DILocalVariable
+
+# Checks for DWARF location list up front:
+# DWARF:      DW_TAG_variable
+# DWARF-NEXT: DW_AT_location
+# DWARF-NEXT: DW_OP_breg0 RAX+0, DW_OP_constu 0xff, DW_OP_and,
+# DWARF-SAME:   DW_OP_convert (0x00000026) "DW_ATE_signed_8",
+# DWARF-SAME:   DW_OP_convert (0x0000002a) "DW_ATE_signed_32", DW_OP_stack_value
+# DWARF-NEXT: DW_OP_breg7 RSP-8, DW_OP_deref_size 0x1,
+# DWARF-SAME:   DW_OP_convert (0x00000026) "DW_ATE_signed_8",
+# DWARF-SAME:   DW_OP_convert (0x0000002a) "DW_ATE_signed_32", DW_OP_stack_value
+## First define-on-stack location.
+# DWARF-NEXT: DW_OP_breg7 RSP-8, DW_OP_deref_size 0x1,
+# DWARF-SAME:   DW_OP_convert (0x00000026) "DW_ATE_signed_8",
+# DWARF-SAME:   DW_OP_convert (0x0000002a) "DW_ATE_signed_32", DW_OP_stack_value
+## Tests with meaningless converts, on 32 bit values.
+# DWARF-NEXT: DW_OP_breg0 RAX+0, DW_OP_constu 0xffffffff, DW_OP_and,
+# DWARF-SAME:   DW_OP_convert (0x0000002a) "DW_ATE_signed_32",
+# DWARF-SAME:   DW_OP_convert (0x0000002a) "DW_ATE_signed_32", DW_OP_stack_value
+# DWARF-NEXT: DW_OP_breg7 RSP-8, DW_OP_deref,
+# DWARF-SAME:   DW_OP_convert (0x0000002a) "DW_ATE_signed_32",
+# DWARF-SAME:   DW_OP_convert (0x0000002a) "DW_ATE_signed_32", DW_OP_stack_value
+# DWARF-NEXT: DW_OP_breg7 RSP-8, DW_OP_deref,
+# DWARF-SAME:   DW_OP_convert (0x0000002a) "DW_ATE_signed_32",
+# DWARF-SAME:   DW_OP_convert (0x0000002a) "DW_ATE_signed_32", DW_OP_stack_value
+## Tests with no DIExpression.
+# DWARF-NEXT: DW_OP_reg0 RAX
+# DWARF-NEXT: DW_OP_breg7 RSP-8, DW_OP_deref_size 0x1, DW_OP_stack_value
+# DWARF-NEXT: DW_OP_breg7 RSP-8, DW_OP_deref_size 0x1, DW_OP_stack_value)
+
+--- |
+  ; ModuleID = 'missingvar.ll'
+  source_filename = "a"
+  target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+  target triple = "x86_64-unknown-linux-gnu"
+  
+  define linkonce_odr void @_ZNSt5dequeIPN4llvm4LoopESaIS2_EE13_M_insert_auxESt15_Deque_iteratorIS2_RS2_PS2_EmRKS2_() local_unnamed_addr align 2 !dbg !3 {
+  entry:
+    call void @llvm.dbg.value(metadata i32 0, metadata !8, metadata !DIExpression()), !dbg !7
+    ret void
+  }
+
+  declare void @llvm.dbg.value(metadata, metadata, metadata)
+  
+  !llvm.module.flags = !{!0, !9}
+  !llvm.dbg.cu = !{!1}
+  
+  !0 = !{i32 2, !"Debug Info Version", i32 3}
+  !1 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !2, producer: "beards", isOptimized: true, runtimeVersion: 4, emissionKind: FullDebug)
+  !2 = !DIFile(filename: "bees.cpp", directory: "")
+  !3 = distinct !DISubprogram(name: "nope", scope: !2, file: !2, line: 1, type: !4, spFlags: DISPFlagDefinition, unit: !1)
+  !4 = !DISubroutineType(types: !5)
+  !5 = !{!6}
+  !6 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !7 = !DILocation(line: 1, scope: !3)
+  !8 = !DILocalVariable(name: "flannel", scope: !3, type: !6)
+  !9 = !{i32 2, !"Dwarf Version", i32 5}
+
+...
+---
+name:            _ZNSt5dequeIPN4llvm4LoopESaIS2_EE13_M_insert_auxESt15_Deque_iteratorIS2_RS2_PS2_EmRKS2_
+alignment:       16
+tracksRegLiveness: true
+liveins:
+  - { reg: '$rdi' }
+  - { reg: '$rsi' }
+  - { reg: '$rdx' }
+frameInfo:
+  stackSize:       48
+  offsetAdjustment: -48
+  maxAlignment:    8
+  maxCallFrameSize: 0
+  cvBytesOfCalleeSavedRegisters: 48
+fixedStack:
+  - { id: 0, type: spill-slot, offset: -56, size: 8, alignment: 8, callee-saved-register: '$rbx' }
+  - { id: 1, type: spill-slot, offset: -48, size: 8, alignment: 16, callee-saved-register: '$r12' }
+  - { id: 2, type: spill-slot, offset: -40, size: 8, alignment: 8, callee-saved-register: '$r13' }
+  - { id: 3, type: spill-slot, offset: -32, size: 8, alignment: 16, callee-saved-register: '$r14' }
+  - { id: 4, type: spill-slot, offset: -24, size: 8, alignment: 8, callee-saved-register: '$r15' }
+  - { id: 5, type: spill-slot, offset: -16, size: 8, alignment: 16, callee-saved-register: '$rbp' }
+stack:
+  - { id: 0, type: spill-slot, offset: -64, size: 8, alignment: 8 }
+machineFunctionInfo: {}
+body:             |
+  bb.0.entry:
+    liveins: $rdi, $rdx, $rsi, $rbp, $r15, $r14, $r13, $r12, $rbx
+  
+    ; CHECK-LABEL: bb.0.entry:
+
+    $al = MOV8ri 0, debug-instr-number 1, debug-location !7
+    DBG_INSTR_REF 1, 0, !8, !DIExpression(DW_OP_LLVM_convert, 8, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value), debug-location !7
+    ; CHECK:      DBG_VALUE $al, $noreg, ![[VARNUM]],
+    ; CHECK-SAME: !DIExpression(DW_OP_LLVM_convert, 8, DW_ATE_signed,
+    ; CHECK-SAME     : DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value)
+
+    MOV8mr $rsp, 1, $noreg, -8, $noreg, renamable $al :: (store 1 into %stack.0)
+    ; Clobber to force variable location onto stack.
+    $al = MOV8ri 0, debug-location !7
+    ; CHECK:      DBG_VALUE $rsp, $noreg, ![[VARNUM]],
+    ; CHECK-SAME: !DIExpression(DW_OP_constu, 8, DW_OP_minus, 
+    ; CHECK-SAME:     DW_OP_deref_size, 1, DW_OP_LLVM_convert, 8, DW_ATE_signed,
+    ; CHECK-SAME:     DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value),
+
+    ; Terminate the current variable location,
+    DBG_VALUE $noreg, $noreg, !8, !DIExpression(), debug-location !7
+    ; CHECK: DBG_VALUE $noreg, $noreg, ![[VARNUM]], !DIExpression()
+
+    ; Try again, but with the value originating on the stack, to ensure that
+    ; we can find its size.
+    INC8m $rsp, 1, $noreg, 4, $noreg, implicit-def dead $eflags, debug-instr-number 2, debug-location !7 :: (store (s8) into %stack.0)
+    DBG_INSTR_REF 2, 1000000, !8, !DIExpression(DW_OP_LLVM_convert, 8, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value), debug-location !7
+    ; CHECK:      DBG_VALUE $rsp, $noreg, ![[VARNUM]],
+    ; CHECK-SAME: !DIExpression(DW_OP_constu, 8, DW_OP_minus,
+    ; CHECK-SAME:     DW_OP_deref_size, 1, DW_OP_LLVM_convert, 8, DW_ATE_signed,
+    ; CHECK-SAME:     DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value),
+
+    ; Just for certainty, let's do it all again but with the correct
+    ; sizes, and ensure they get the right expressions. With a meaningless
+    ; convert from 32 bits to 32 bits baked in.
+
+    $eax = MOV32ri 0, debug-instr-number 3, debug-location !7
+    DBG_INSTR_REF 3, 0, !8, !DIExpression(DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value), debug-location !7
+    ; CHECK:      DBG_VALUE $eax, $noreg, ![[VARNUM]],
+    ; CHECK-SAME: !DIExpression(DW_OP_LLVM_convert, 32, DW_ATE_signed,
+    ; CHECK-SAME:     DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value),
+
+    MOV32mr $rsp, 1, $noreg, -8, $noreg, renamable $eax :: (store 4 into %stack.0)
+    ; Clobber to force variable location onto stack.
+    $eax = MOV32ri 0, debug-location !7
+    ; CHECK:      DBG_VALUE $rsp, $noreg, ![[VARNUM]],
+    ; CHECK-SAME: !DIExpression(DW_OP_constu, 8, DW_OP_minus,
+    ; CHECK-SAME:     DW_OP_deref, DW_OP_LLVM_convert, 32, DW_ATE_signed,
+    ; CHECK-SAME:     DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value),
+
+    DBG_VALUE $noreg, $noreg, !8, !DIExpression(), debug-location !7
+    ; CHECK: DBG_VALUE $noreg, $noreg, ![[VARNUM]], !DIExpression(),
+
+    INC32m $rsp, 1, $noreg, 4, $noreg, implicit-def dead $eflags, debug-instr-number 4, debug-location !7 :: (store (s32) into %stack.0)
+    DBG_INSTR_REF 4, 1000000, !8, !DIExpression(DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value), debug-location !7
+    ; CHECK:      DBG_VALUE $rsp, $noreg, ![[VARNUM]],
+    ; CHECK-SAME: !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_deref,
+    ; CHECK-SAME:     DW_OP_LLVM_convert, 32, DW_ATE_signed,
+    ; CHECK-SAME:     DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value),
+
+    ; Repeat again -- this time with no contents in the DIExpression. This test
+    ; that when we switch between a plain register location and a DWARF location
+    ; expression, we add a DW_OP_stack_value. We can't use a non-stack-value
+    ; location expression, because the difference between the variable size and
+    ; the value size will not be described.
+
+    $al = MOV8ri 0, debug-instr-number 5, debug-location !7
+    DBG_INSTR_REF 5, 0, !8, !DIExpression(), debug-location !7
+    ; CHECK:      DBG_VALUE $al, $noreg, ![[VARNUM]], !DIExpression()
+
+    MOV32mr $rsp, 1, $noreg, -8, $noreg, renamable $eax :: (store 4 into %stack.0)
+    ; Clobber to force variable location onto stack.
+    $eax = MOV32ri 0, debug-location !7
+    ; CHECK:      DBG_VALUE $rsp, $noreg, ![[VARNUM]],
+    ; CHECK-SAME: !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_deref_size, 1, DW_OP_stack_value)
+
+    DBG_VALUE $noreg, $noreg, !8, !DIExpression(), debug-location !7
+    ; CHECK: DBG_VALUE $noreg, $noreg, ![[VARNUM]], !DIExpression(),
+
+    ; We can tell from the memory operand that a single-byte value is defined,
+    ; which should be recognised as "variable size doesn't match value size".
+    INC8m $rsp, 1, $noreg, 4, $noreg, implicit-def dead $eflags, debug-instr-number 6, debug-location !7 :: (store (s8) into %stack.0)
+    DBG_INSTR_REF 6, 1000000, !8, !DIExpression(), debug-location !7
+    ; CHECK:      DBG_VALUE $rsp, $noreg, ![[VARNUM]],
+    ; CHECK-SAME: !DIExpression(DW_OP_constu, 8, DW_OP_minus,
+    ; CHECK-SAME:     DW_OP_deref_size, 1, DW_OP_stack_value),
+
+    RET64
+...

llvm/test/DebugInfo/MIR/X86/live-debug-values-restore.mir

bb.2.if.end:
     $rdi = MOV64ri 0
     RET64 $eax, debug-location !128
 
 ...
 ---
 # This third function tests that complex expressions are spilt, and restored
 # correctly within a basic block.
 
-# FIXME: the spilt location below is wrong, there should be a deref between
-# the spill-offset and the DW_OP_plus_uconst.
-
 # CHECK-LABEL: name: h
 # CHECK-LABEL: bb.0.entry:
 # CHECK:       DBG_VALUE $rdi, $noreg, ![[RVAR]], !DIExpression(DW_OP_plus_uconst, 1)
 # CHECK-LABEL: bb.1.if.then:
 # CHECK:       DBG_VALUE $rdi, $noreg, ![[RVAR]], !DIExpression(DW_OP_plus_uconst, 1)
-# CHECK:       DBG_VALUE $rsp, 0, ![[RVAR]], !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_plus_uconst, 1)
+# CHECK:       DBG_VALUE $rsp, $noreg, ![[RVAR]], !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_deref, DW_OP_plus_uconst, 1)
 # CHECK:       DBG_VALUE $rdi, $noreg, ![[RVAR]], !DIExpression(DW_OP_plus_uconst, 1)
 # CHECK-LABEL: bb.2.if.end:
 # CHECK:       DBG_VALUE $rdi, $noreg, ![[RVAR]], !DIExpression(DW_OP_plus_uconst, 1)
 
 name:            h
 alignment:       16
 tracksRegLiveness: true
 liveins:         

 
 ...
 ---
 # Function four: test that spill restores are detected across block
 # boundaries. The spill has been moved to bb.1, children of which are
 # bb 2 and 3, neither of which modifies the stack loc. The exit block (3)
 # should still be tracking the spill, and restore it on stack load.
 
-# FIXME: this test too contains a broken spill location.
-
 # Summary: loc is in $rdi in bb0, spills to stack in bb1, remains in stack
 # in bb2, starts in stack then loaded in bb3.
 
 # CHECK-LABEL: name: i
 # CHECK-LABEL: bb.0.entry:
 # CHECK:       DBG_VALUE $rdi, $noreg, ![[SVAR]], !DIExpression(DW_OP_plus_uconst, 1)
 # CHECK-LABEL: bb.1.foo:
 # CHECK:       DBG_VALUE $rdi, $noreg, ![[SVAR]], !DIExpression(DW_OP_plus_uconst, 1)
-# CHECK:       DBG_VALUE $rsp, 0, ![[SVAR]], !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_plus_uconst, 1)
+# CHECK:       DBG_VALUE $rsp, $noreg, ![[SVAR]], !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_deref, DW_OP_plus_uconst, 1)
 # CHECK-LABEL: bb.2.if.then:
-# CHECK:       DBG_VALUE $rsp, 0, ![[SVAR]], !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_plus_uconst, 1)
+# CHECK:       DBG_VALUE $rsp, $noreg, ![[SVAR]], !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_deref, DW_OP_plus_uconst, 1)
 # CHECK-LABEL: bb.3.if.end
-# CHECK:       DBG_VALUE $rsp, 0, ![[SVAR]], !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_plus_uconst, 1)
+# CHECK:       DBG_VALUE $rsp, $noreg, ![[SVAR]], !DIExpression(DW_OP_constu, 8, DW_OP_minus, DW_OP_deref, DW_OP_plus_uconst, 1)
 # CHECK:       DBG_VALUE $rdi, $noreg, ![[SVAR]], !DIExpression(DW_OP_plus_uconst, 1)
 name:            i
 alignment:       16
 tracksRegLiveness: true
 liveins:         
   - { reg: '$rdi', virtual-reg: '' }
 frameInfo:       
   stackSize:       48

llvm/test/DebugInfo/MIR/X86/livedebugvalues_load_in_loop.mir

--- |
   !llvm.module.flags = !{!7, !8, !9, !10}
   !llvm.ident = !{!11}
   !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 10.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, globals: !3, debugInfoForProfiling: true, nameTableKind: None)
   !1 = !DIFile(filename: "main.cpp", directory: "F:\")
   !2 = !{}
   !3 = !{!4}
   !4 = !DIGlobalVariableExpression(var: !5, expr: !DIExpression())
   !5 = distinct !DIGlobalVariable(name: "start", scope: !0, file: !1, line: 4, type: !6, isLocal: false, isDefinition: true)
-  !6 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !6 = !DIBasicType(name: "int", size: 64, encoding: DW_ATE_signed)
   !7 = !{i32 2, !"Dwarf Version", i32 4}
   !8 = !{i32 2, !"Debug Info Version", i32 3}
   !9 = !{i32 1, !"wchar_size", i32 2}
   !10 = !{i32 7, !"PIC Level", i32 2}
   !11 = !{!"clang version 10.0.0"}
   !12 = distinct !DISubprogram(name: "bb_to_bb", linkageName: "bb_to_bb", scope: !1, file: !1, line: 6, type: !13, scopeLine: 6, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !15)
   !13 = !DISubroutineType(types: !14)
   !14 = !{!6, !6}