<div dir="ltr">A few nits:<div>+ unsigned FoldedReg = FoldAsLoadDefReg; // Need this below<br></div><div>--> please reword your comments to explain why we need to copy it to another variable.</div><div><br></div>
<div><div>+</div><div>+ // Mark any DBG_VALUE that uses FoldReg as undef (but don't </div><div>+ // delete it.) Code copied from DeadMachineInstructionElim.</div><div>+ MachineRegisterInfo::use_iterator nextI;</div>
<div>+ for (MachineRegisterInfo::use_iterator I = MRI->use_begin(FoldedReg),</div><div>+ E = MRI->use_end(); I!=E; I=nextI) {</div><div>+ nextI = llvm::next(I); // I is invalidated by the setReg</div>
<div>+ MachineOperand& Use = I.getOperand();</div><div>+ MachineInstr *UseMI = Use.getParent();</div><div>+ if (UseMI->isDebugValue())</div><div>+ UseMI->getOperand(0).setReg(0U);</div>
<div>+ }</div></div><div>--> is it possible to write a helper function to update DBG_VALUEs that use a removed register instead of copying the code?</div><div><br></div><div>Remove !tbaa MDNodes from the testing cases if they do not affect the correctness?</div>
<div><br></div><div>Thanks,</div><div>Manman</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Feb 13, 2014 at 3:38 PM, Katya Romanova <span dir="ltr"><<a href="mailto:Katya_Romanova@playstation.sony.com" target="_blank">Katya_Romanova@playstation.sony.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">kromanova added you to the CC list for the revision "Peephole optimization causes different code generation at "-O2 -g" and "-O2".".<br>
<br>
Fix for BZ #18590: <a href="http://llvm.org/bugs/show_bug.cgi?id=18590" target="_blank">http://llvm.org/bugs/show_bug.cgi?id=18590</a><br>
<br>
This code review is for the bug fix in peephole optimization that folds a load which defines one vreg into the one and only use of that vreg. With debug info, a DBG_VALUE that referenced the vreg considered to be a use, preventing the optimization.<br>
<br>
The fix is to ignore DBG_VALUE's during the optimization, and undef a DBG_VALUE that references a vreg that gets removed.<br>
<br>
The main issue is how to update the DBG_VALUE properly. I looked at all calls of isDebugValue() to see what is typically done. There is at least one case that does each of the following:<br>
<br>
- rewrite the debug info (this was during inlining to change the vreg)<br>
- delete it (happens in tail duplication)<br>
- undef's it, by setting register to %noreg (in eliminate dead insns)<br>
<br>
Looking at those also showed that most optimization happily just skip any DBG_VALUE while iterating over insns. Which is what I do in this fix.<br>
<br>
I also looked at all calls to MachineInstr::eraseFromParent() to see how other code dealt with DBG_VALUE's when deleting an instruction. In most cases it did nothing. Presumably because the live debug variable pass will clean it up. I tested a fix that does nothing to the DBG_VALUE for this peephole case and it does get set to %noreg by the live debug variable pass.<br>
<br>
In the end this fix requires 3 simple changes:<br>
<br>
- skip DBG_VALUE's while iterating over instructions<br>
- only consider NonDBG uses when checking whether a def has a single use<br>
- undef any DBG_VALUE's that use the vreg getting folded.<br>
<br>
Fix by Trevor Smigiel.<br>
<br>
<br>
<a href="http://llvm-reviews.chandlerc.com/D2728" target="_blank">http://llvm-reviews.chandlerc.com/D2728</a><br>
<br>
Files:<br>
test/CodeGen/X86/dbg-changes-codegen-1.ll<br>
test/CodeGen/X86/dbg-changes-codegen-2.ll<br>
lib/CodeGen/PeepholeOptimizer.cpp<br>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>