<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Mar 6, 2013 at 1:01 PM, Adrian Prantl <span dir="ltr"><<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div><br>
On Mar 1, 2013, at 10:15 AM, Adrian Prantl <<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>> wrote:<br>
<br>
><br>
> On Feb 28, 2013, at 11:20 AM, John McCall <<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>> wrote:<br>
><br>
>> On Feb 28, 2013, at 11:12 AM, Eric Christopher <<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>> wrote:<br>
>>> On Thu, Feb 28, 2013 at 11:06 AM, John McCall <<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>> wrote:<br>
>>> On Feb 27, 2013, at 3:17 PM, Eric Christopher <<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>> wrote:<br>
>>>> On Wed, Feb 27, 2013 at 11:49 AM, John McCall <<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>> wrote:<br>
>>>> On Feb 27, 2013, at 11:42 AM, Eric Christopher <<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>> wrote:<br>
>>>>> On Wed, Feb 27, 2013 at 11:39 AM, Adrian Prantl <<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>> wrote:<br>
>>>>><br>
>>>>> On Feb 27, 2013, at 11:31 AM, John McCall <<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>> wrote:<br>
>>>>>> Okay, you're saying that the value is actually no longer live at all at this point in the function? It seems reasonable to lose track of the debug info then, although we should be leaving behind a marker in the DWARF that says the value is unavailable.<br>
>>>>>><br>
>>>>>> If we want to make stronger guarantees in -O0 for purposes of debugging — and I think that's reasonable — then throwing the value in an alloca is acceptable.<br>
>>>>><br>
>>>>> To clarify: Are you suggesting to only generate the alloca at -O0, or are you comfortable with it as it is?<br>
>>>>><br>
>>>>> If the value isn't live past that spot I'm more comfortable with dropping the debug info there rather than changing the generated code to keep the value live through the end of the function.<br>
>>>><br>
>>>> Purely out of attachment to the principle that debug info shouldn't change the code?<br>
>>>><br>
>>>><br>
>>>> Pretty much.<br>
>>>><br>
>>>> Not losing information has intrinsic value in a debug build. If we need to emit slightly different code in order to force a value to stay live and thus improve the debugging experience, then so be it.<br>
>>>><br>
>>>><br>
>>>> Agreed that making the experience better is desirable, but it can make debugging a problem more difficult if the code changes when you turn on debugging symbols.<br>
>>><br>
>>> Ah, I see your point; not doing the alloca could slide stack frames around.<br>
>>><br>
>>> Alright, I agree with emitting it in all -O0 builds.<br>
>>><br>
>>> Thought if optimization should fix it then perhaps all builds? :)<br>
>><br>
>> I don't see any point in creating it just for mem2reg to trivially destroy. :)<br>
>><br>
>>> That said I'll remove the objection to the allocas. We'll need to fix the alloca problem at some point, but making poor Adrian do it right now for this bug when we've got other workarounds already in the source base seems a bit mean.<br>
>><br>
>> Well, if the value really isn't live anymore, then I'm not sure what the supposed alloca problem is, other than needing to leave breadcrumbs to say that the value isn't available at this point in the function. We definitely don't want regalloc to be keeping values live just for debug info!<br>
><br>
> FYI: this is what the patch looks like if output the alloca only at -O0.<br>
<br>
<br>
</div></div>Hi John and Eric,<br>
<br>
It seems as if at some point in the conversation everybody agreed with generating allocas at -O0.<br>
If someone can hint me at a better solution I'm happy to look into that as well, but on the other hand I also really would like to close this bug.<br>
Any objections to the -O0-version of the patch?<br></blockquote><div><br></div><div>I'll defer to John for most of this. I'm not hugely happy with it because it means that the base code is papering over the problem even more at higher optimization levels, but... </div>
<div><br></div><div style>That said:</div><div style><br></div><div style><div>+ // Matching the code in EmitParmDecl, depending on optimization level.</div><div>+ llvm::Instruction *Call;</div><div>+ if (CGM.getCodeGenOpts().OptimizationLevel == 0)</div>
<div>+ // Insert an llvm.dbg.declare into the current block.</div><div>+ Call = DBuilder.insertDbgValueIntrinsic(Arg, 0, debugVar,</div><div>+<span class="" style="white-space:pre"> </span> Builder.GetInsertBlock());</div>
<div>+ else</div><div>+ // Insert an llvm.dbg.declare into the current block.</div><div>+ Call = DBuilder.insertDeclare(LocalAddr, debugVar, Builder.GetInsertBlock());</div><div>+ Call->setDebugLoc(llvm::DebugLoc::get(line, column, scope));</div>
<div><br></div><div style>Since you added LocalAddr can't you just check if it's NULL instead of the optimization level?</div><div style><br></div><div style>Also I'd prefer a TODO and a PR to fix this as it's not really the right way to fix variable tracking.</div>
<div style><br></div><div style>+// RUN: %clang -fblocks -S -g -fverbose-asm -triple x86_64-apple-darwin -o - %s | FileCheck %s<br></div><div style><br></div><div style>Please don't have a testcase that requires a backend. Put this part in the backend with the code as a comment for the IR. You could also add it to the debug info tests to make sure you can debug and print out self.</div>
<div style><br></div><div style>-eric</div></div></div></div></div>