[PATCH] Peephole optimization causes different code generation at "-O2 -g" and "-O2".

[Manman Ren]

A few nits:
+        unsigned FoldedReg = FoldAsLoadDefReg; // Need this below
--> please reword your comments to explain why we need to copy it to
another variable.

+
+          // Mark any DBG_VALUE that uses FoldReg as undef (but don't
+          // delete it.)  Code copied from DeadMachineInstructionElim.
+          MachineRegisterInfo::use_iterator nextI;
+          for (MachineRegisterInfo::use_iterator I =
MRI->use_begin(FoldedReg),
+               E = MRI->use_end(); I!=E; I=nextI) {
+            nextI = llvm::next(I);  // I is invalidated by the setReg
+            MachineOperand& Use = I.getOperand();
+            MachineInstr *UseMI = Use.getParent();
+            if (UseMI->isDebugValue())
+                UseMI->getOperand(0).setReg(0U);
+          }
--> is it possible to write a helper function to update DBG_VALUEs that use
a removed register instead of copying the code?

Remove !tbaa MDNodes from the testing cases if they do not affect the
correctness?

Thanks,
Manman


On Thu, Feb 13, 2014 at 3:38 PM, Katya Romanova <
Katya_Romanova at playstation.sony.com> wrote:

> kromanova added you to the CC list for the revision "Peephole optimization
> causes different code generation at "-O2 -g" and "-O2".".
>
> Fix for BZ #18590: http://llvm.org/bugs/show_bug.cgi?id=18590
>
> 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.
>
> The fix is to ignore DBG_VALUE's during the optimization, and undef a
> DBG_VALUE that references a vreg that gets removed.
>
> 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:
>
> - rewrite the debug info (this was during inlining to change the vreg)
> - delete it (happens in tail duplication)
> - undef's it, by setting register to %noreg (in eliminate dead insns)
>
> 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.
>
> 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.
>
> In the end this fix requires 3 simple changes:
>
> - skip DBG_VALUE's while iterating over instructions
> - only consider NonDBG uses when checking whether a def has a single use
> - undef any DBG_VALUE's that use the vreg getting folded.
>
> Fix by Trevor Smigiel.
>
>
> http://llvm-reviews.chandlerc.com/D2728
>
> Files:
>   test/CodeGen/X86/dbg-changes-codegen-1.ll
>   test/CodeGen/X86/dbg-changes-codegen-2.ll
>   lib/CodeGen/PeepholeOptimizer.cpp
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140213/7c228ded/attachment.html>