<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;
mso-fareast-language:EN-US;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-GB" link="blue" vlink="purple" style="word-wrap:break-word">
<div class="WordSection1">
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">Hi Reid,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"><br>
Sorry for the long delay in getting back to you. I've been thinking about this problem a lot and I have some thoughts on the existing ideas, and have come up with another partial solution.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"># Thoughts on existing ideas<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">Currently I prefer the idea you proposed earlier in this thread (the "Coffee Chat" method, which I summarise later) to the "Bugzilla" method [1] based on the idea that there's less chance of us generating
wrong debug info, because - as you mentioned - most optimisation passes won't need to worry about updating any debug info when messing with stores.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">Additionally, regarding tooling for the Bugzilla method, you said:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">> All I had in mind was something like:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">> - add a pass that finds all "local variable stores" (stores to a static alloca used by a dbg.declare), and inserts markers (llvm.donothing?) carrying the local variable metadata after every such
store<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">> - after optimization, check that every marker is preceded by either a store to a local variable of a dbg.value for the same local variable. Any marker with no dbg.value or store before it is a
bug, unless the variable is available nowhere (I forget how that is represented)<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">This itself sounds similar to the Coffee Chat method implementation, which makes the Bugzilla method less appealing IMO since we'd almost be implementing both solutions. Summarising the version of
the Coffee Chat method from this email [2] as I understand it:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">00. Remove LowerDbgDeclare.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">01. Frontends emit a new kind of debug intrinsic (dbg.assign?) after every store, tracking both the stored value and store destination.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">02. When mem2reg promotes allocas it converts dbg.assign(value, dest) -> dbg.value(value).<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">03. If any dbg.assign remain when we get to ISel then we know the alloca is still used for at least some of the variable's lifetime. We must choose to convert remaining dbg.assign(value, dest) intrinsics
to either:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> A) DBG_VALUE value<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> B) DBG_VALUE dest, DIExpression(DW_OP_deref))<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> where B is preferable if we know the dest address holds the correct value.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">To be able to choose correctly in step 03 we need a way of knowing if a specific store has been deleted or moved. To keep the desirable property of allowing optimisation passes to remain ignorant
of that requirement this needs to be handled automatically in the background. This would involve linking the store instruction to the dbg.assign, and I believe we can (and should) take advantage of existing infrastructure to do this. For instance, we could
change the store instruction to return a special DebugToken type value which dbg.assign intrinsics take as an argument. If the store is deleted, the store-token argument gets RAUW(undef) as per D80264, and then in step 03 we simply look to see if that argument
is undef or not. We can also reason about movement of the store relative to the position of the dbg.assign safely in step 03 because we know exactly which store the dbg.assign is associated with.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">Here is an example showing what that process looks like with DSE:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> === IR snippet from the frontend ===<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> %dest = alloca i32<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> %one = ...<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> %two = ...<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> %token.first = store i32 %one, i32* %dest<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> dbg.assign(%token.first, %one, %dest, "my_var", DIExpression())<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> %token.second = store i32 &two, i32* %dest<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> dbg.assign(%token.second, %two, %dest, "my_var", DIExpression())<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> === DSE the first store ===<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> %dest = alloca i32<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> %one = ...<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> %two = ...<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> ; ### First store DSE'd, RAUW(undef)<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> dbg.assign(undef, %one, %dest, "my_var", DIExpression())<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> %token.second = store i32 &two, i32* %dest<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> dbg.assign(%token.second, %two, %dest, "my_var", DIExpression())<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> === Lower to MIR ===<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> ...<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> DBG_VALUE %one, "my_var", DIExpression() ; Use value componnent because the first dbg.assign argument is undef.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> MOV32mr %dest, %two<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> DBG_VALUE %dest, "my_var", DIExpression(DW_OP_deref) ; Use dest component + deref because the first dbg.assign argument is not undef.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">Finally, imagine that %one also gets DCE'd after the store using it is removed. Its metadata use in the dbg.assign will get RAUW(undef), and when we lower to MIR we’ll insert a DBG_VALUE $noreg just
like we would for a dbg.value(undef, …), correctly indicting that there is no location for the variable available here.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"># An alternative idea<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">I also hoped to get your opinion on an idea for an alternative solution. I was trying to come up with something that would involve less work than these other ideas. It works in a similar way to your
Coffee Chat idea, but doesn't require that link back to stores. The idea is basically to use stores to track assignments to alloca variables, and solve the DSE problem by inserting a new intrinsic dbg.store(value, dest) every time a store is deleted.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">When mem2reg promotes allocas it converts the dbg.declare to dbg.values as per usual, and also inserts dbg.values after each dbg.store using the alloca. Later, if the alloca hasn't been promoted,
we can determine whether a dbg.declare using it is still valid for the entire lifetime of the variable by looking for dbg.stores using the alloca. If there are none then no stores have been removed and we know the stack location is valid. Otherwise, we convert
the dbg.stores to dbg.values and insert dbg.value+deref after the remaining (real) stores.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">00. Remove LowerDbgDeclare.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">01. Keep dbg.declare and dbg.value semantics as they are now.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">02. When a store is deleted insert a dbg.store like this:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> store value, dest -> dbg.store(value, dest, DIExpression())<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">03. mem2reg keeps its existing pipeline for dbg.declare -> dbg.value when promoting, with this addition:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> 04. For each dbg.store(value, dest, expr) using the alloca:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> 05. Insert a dbg.value(value, var, expr) before the dbg.store. var taken from the dbg.declare.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> 06. Delete all dbg.store(_, dest, _) using the alloca.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">07. For each dbg.declare(dest, var, expr) that exists when we get to ISel:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> 08. If there are no dbg.store(_, dest) using dest, emit frame-index variable.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> 09. Else for each dbg.store(value, dest, expr) using dest:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> 10. Insert a DBG_VALUE(value, var, expr) before the dbg.store.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> 11. For each (real) store to dest:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"> 12. Insert DBG_VALUE(dest, var, DIExpression(DW_OP_deref)) before the store.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">This solves the DSE problem, and gives us better location coverage than LowerDbgDeclare for the same reasons that the Coffee Chat and Bugzilla methods do: we'd be end up with dbg.value+deref after
stores, and dbg.values after removed dead stores, instead of what we get now which is dbg.values after both stores and removed stores.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">The advantage of this method is that it shouldn't involve too much implementation work. The biggest downside is that it doesn't account for code motion moving stores, which is a regression from LowerDbgDeclare
and is a problem that the Coffee Chat method doesn't suffer from. If both dbg.stores and (real) stores represent assignments to alloca variables, moving stores around causes the program location of the assignments to differ from those in the source code. Unfortunately,
I don't think this solution is viable unless we can adjust it to account for that. What do you think?<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">Many thanks,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">Orlando<o:p></o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="mso-fareast-language:EN-US">[1] Bugzilla method:
<a href="https://bugs.llvm.org/show_bug.cgi?id=34136#c25">https://bugs.llvm.org/show_bug.cgi?id=34136#c25</a>
<br>
[2] Coffee Chat method: <a href="https://groups.google.com/g/llvm-dev/c/HX8HL1f3sw0/m/9ylo8-CnDAAJ">
https://groups.google.com/g/llvm-dev/c/HX8HL1f3sw0/m/9ylo8-CnDAAJ</a><br>
<br>
</span><o:p></o:p></p>
<div>
<div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</div>
</div>
</body>
</html>