<div>r165498.</div><br><div class="gmail_quote">On Tue, Oct 9, 2012 at 12:38 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On Tue, Oct 9, 2012 at 1:13 AM, Alexey Samsonov <span dir="ltr"><<a href="mailto:samsonov@google.com" target="_blank">samsonov@google.com</a>></span> wrote:<br></div></div><div class="gmail_extra">
<div class="gmail_quote"><div><div class="h5">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: samsonov<br>
Date: Tue Oct 9 03:13:15 2012<br>
New Revision: 165490<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=165490&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=165490&view=rev</a><br>
Log:<br>
Fix PR14016.<br>
DeadArgumentElimination pass can replace one LLVM function with another,<br>
invalidating a pointer stored in debug info metadata entry for this function.<br>
To fix this, we collect debug info descriptors for functions before<br>
running a DeadArgumentElimination pass and "patch" pointers in metadata nodes<br>
if we replace a function.<br>
<br>
Added:<br>
llvm/trunk/test/Transforms/DeadArgElim/dbginfo.ll<br>
Modified:<br>
llvm/trunk/include/llvm/DebugInfo.h<br>
llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp<br>
llvm/trunk/lib/VMCore/DebugInfo.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/DebugInfo.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo.h?rev=165490&r1=165489&r2=165490&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo.h?rev=165490&r1=165489&r2=165490&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/DebugInfo.h (original)<br>
+++ llvm/trunk/include/llvm/DebugInfo.h Tue Oct 9 03:13:15 2012<br>
@@ -81,6 +81,7 @@<br>
GlobalVariable *getGlobalVariableField(unsigned Elt) const;<br>
Constant *getConstantField(unsigned Elt) const;<br>
Function *getFunctionField(unsigned Elt) const;<br>
+ void replaceFunctionField(unsigned Elt, Function *F);<br>
<br>
public:<br>
explicit DIDescriptor() : DbgNode(0) {}<br>
@@ -562,6 +563,7 @@<br>
bool describes(const Function *F);<br>
<br>
Function *getFunction() const { return getFunctionField(16); }<br>
+ void replaceFunction(Function *F) { replaceFunctionField(16, F); }<br>
DIArray getTemplateParams() const { return getFieldAs<DIArray>(17); }<br>
DISubprogram getFunctionDeclaration() const {<br>
return getFieldAs<DISubprogram>(18);<br>
<br>
Modified: llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp?rev=165490&r1=165489&r2=165490&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp?rev=165490&r1=165489&r2=165490&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp Tue Oct 9 03:13:15 2012<br>
@@ -21,7 +21,9 @@<br>
#include "llvm/Transforms/IPO.h"<br>
#include "llvm/CallingConv.h"<br>
#include "llvm/Constant.h"<br>
+#include "llvm/DebugInfo.h"<br>
#include "llvm/DerivedTypes.h"<br>
+#include "llvm/DIBuilder.h"<br>
#include "llvm/Instructions.h"<br>
#include "llvm/IntrinsicInst.h"<br>
#include "llvm/LLVMContext.h"<br>
@@ -121,6 +123,15 @@<br>
<br>
typedef SmallVector<RetOrArg, 5> UseVector;<br>
<br>
+ // Map each LLVM function to corresponding metadata with debug info. If<br>
+ // the function is replaced with another one, we should patch the pointer<br>
+ // to LLVM function in metadata.<br>
+ // As the code generation for module is finished (and DIBuilder is<br>
+ // finalized) we assume that subprogram descriptors won't be changed, and<br>
+ // they are stored in map for short duration anyway.<br>
+ typedef std::map<Function*, DISubprogram> FunctionDIMap;<br></blockquote><div><br></div></div></div><div>DenseMap or DenseSmallMap would likely be better here. You're not iterating anywhere...</div><div><div class="h5">
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ FunctionDIMap FunctionDIs;<br>
+<br>
protected:<br>
// DAH uses this to specify a different ID.<br>
explicit DAE(char &ID) : ModulePass(ID) {}<br>
@@ -141,6 +152,7 @@<br>
unsigned RetValNum = 0);<br>
Liveness SurveyUses(const Value *V, UseVector &MaybeLiveUses);<br>
<br>
+ void CollectFunctionDIs(Module &M);<br>
void SurveyFunction(const Function &F);<br>
void MarkValue(const RetOrArg &RA, Liveness L,<br>
const UseVector &MaybeLiveUses);<br>
@@ -180,6 +192,31 @@<br>
ModulePass *llvm::createDeadArgEliminationPass() { return new DAE(); }<br>
ModulePass *llvm::createDeadArgHackingPass() { return new DAH(); }<br>
<br>
+/// CollectFunctionDIs - Map each function in the module to its debug info<br>
+/// descriptor.<br>
+void DAE::CollectFunctionDIs(Module &M) {<br>
+ FunctionDIs.clear();<br>
+<br>
+ for (Module::named_metadata_iterator I = M.named_metadata_begin(),<br>
+ E = M.named_metadata_end(); I != E; ++I) {<br>
+ NamedMDNode &NMD = *I;<br>
+ for (unsigned i = 0, n = NMD.getNumOperands(); i < n; ++i) {<br>
+ MDNode *Node = NMD.getOperand(i);<br>
+ if (DIDescriptor(Node).isCompileUnit()) {<br>
+ DICompileUnit CU(Node);<br>
+ const DIArray &SPs = CU.getSubprograms();<br>
+ for (unsigned i = 0, n = SPs.getNumElements(); i < n; ++i) {<br>
+ DISubprogram SP(SPs.getElement(i));<br>
+ if (SP.Verify()) {<br>
+ if (Function *F = SP.getFunction())<br>
+ FunctionDIs[F] = SP;<br>
+ }<br>
+ }<br>
+ }<br>
+ }<br>
+ }<br></blockquote><div><br></div></div></div><div>This loop is terrifying! ;] A couple of changes here that are pretty common steps to simplify loops in LLVM-land:</div><div><br></div><div>- Don't shadow loop variables in inner loops. You've shadowed both 'i' and 'n', so this happens to work, but a tiny typo would lead to disaster. ;]</div>
<div>- Use 'continue' and inverted conditions to reduce the indentation.</div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+}<br>
+<br>
/// DeleteDeadVarargs - If this is an function that takes a ... list, and if<br>
/// llvm.vastart is never called, the varargs list is dead for the function.<br>
bool DAE::DeleteDeadVarargs(Function &Fn) {<br>
@@ -284,6 +321,11 @@<br>
I2->takeName(I);<br>
}<br>
<br>
+ // Patch the pointer to LLVM function in debug info descriptor.<br>
+ FunctionDIMap::iterator DI = FunctionDIs.find(&Fn);<br>
+ if (DI != FunctionDIs.end())<br>
+ DI->second.replaceFunction(NF);<br>
+<br>
// Finally, nuke the old function.<br>
Fn.eraseFromParent();<br>
return true;<br>
@@ -952,6 +994,11 @@<br>
BB->getInstList().erase(RI);<br>
}<br>
<br>
+ // Patch the pointer to LLVM function in debug info descriptor.<br>
+ FunctionDIMap::iterator DI = FunctionDIs.find(F);<br>
+ if (DI != FunctionDIs.end())<br>
+ DI->second.replaceFunction(NF);<br>
+<br>
// Now that the old function is dead, delete it.<br>
F->eraseFromParent();<br>
<br>
@@ -961,6 +1008,9 @@<br>
bool DAE::runOnModule(Module &M) {<br>
bool Changed = false;<br>
<br>
+ // Collect debug info descriptors for functions.<br>
+ CollectFunctionDIs(M);<br>
+<br>
// First pass: Do a simple check to see if any functions can have their "..."<br>
// removed. We can do this if they never call va_start. This loop cannot be<br>
// fused with the next loop, because deleting a function invalidates<br>
<br>
Modified: llvm/trunk/lib/VMCore/DebugInfo.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/DebugInfo.cpp?rev=165490&r1=165489&r2=165490&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/DebugInfo.cpp?rev=165490&r1=165489&r2=165490&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/VMCore/DebugInfo.cpp (original)<br>
+++ llvm/trunk/lib/VMCore/DebugInfo.cpp Tue Oct 9 03:13:15 2012<br>
@@ -111,6 +111,16 @@<br>
return 0;<br>
}<br>
<br>
+void DIDescriptor::replaceFunctionField(unsigned Elt, Function *F) {<br>
+ if (DbgNode == 0)<br>
+ return;<br>
+<br>
+ if (Elt < DbgNode->getNumOperands()) {<br>
+ MDNode *Node = const_cast<MDNode*>(DbgNode);<br>
+ Node->replaceOperandWith(Elt, F);<br>
+ }<br>
+}<br>
+<br>
unsigned DIVariable::getNumAddrElements() const {<br>
if (getVersion() <= LLVMDebugVersion8)<br>
return DbgNode->getNumOperands()-6;<br>
<br>
Added: llvm/trunk/test/Transforms/DeadArgElim/dbginfo.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/DeadArgElim/dbginfo.ll?rev=165490&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/DeadArgElim/dbginfo.ll?rev=165490&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/DeadArgElim/dbginfo.ll (added)<br>
+++ llvm/trunk/test/Transforms/DeadArgElim/dbginfo.ll Tue Oct 9 03:13:15 2012<br>
@@ -0,0 +1,64 @@<br>
+; RUN: opt %s -deadargelim -S | FileCheck %s<br>
+; PR14016<br>
+<br>
+; Check that debug info metadata for subprograms stores pointers to<br>
+; updated LLVM functions.<br>
+<br>
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"<br>
+target triple = "x86_64-unknown-linux-gnu"<br>
+<br>
+@x = global i32 0, align 4<br>
+<br>
+define void @_Z3runv() uwtable {<br>
+entry:<br>
+ call void @_ZN12_GLOBAL__N_18dead_argEPv(i8* null), !dbg !10<br>
+ call void (...)* @_ZN12_GLOBAL__N_111dead_varargEz(), !dbg !12<br>
+ ret void, !dbg !13<br>
+}<br>
+<br>
+; Argument will be deleted<br>
+define internal void @_ZN12_GLOBAL__N_18dead_argEPv(i8* %foo) nounwind uwtable {<br>
+entry:<br>
+ %0 = load i32* @x, align 4, !dbg !14<br>
+ %inc = add nsw i32 %0, 1, !dbg !14<br>
+ store i32 %inc, i32* @x, align 4, !dbg !14<br>
+ ret void, !dbg !16<br>
+}<br>
+<br>
+; Vararg will be deleted<br>
+define internal void @_ZN12_GLOBAL__N_111dead_varargEz(...) nounwind uwtable {<br>
+entry:<br>
+ %0 = load i32* @x, align 4, !dbg !17<br>
+ %inc = add nsw i32 %0, 1, !dbg !17<br>
+ store i32 %inc, i32* @x, align 4, !dbg !17<br>
+ ret void, !dbg !19<br>
+}<br>
+<br>
+!<a href="http://llvm.dbg.cu" target="_blank">llvm.dbg.cu</a> = !{!0}<br>
+<br>
+!0 = metadata !{i32 786449, i32 0, i32 4, metadata !"test.cc", metadata !"/home/samsonov/tmp/clang-di", metadata !"clang version 3.2 (trunk 165305)", i1 true, i1 false, metadata !"", i32 0, metadata !1, metadata !1, metadata !3, metadata !1} ; [ DW_TAG_compile_unit ] [/home/samsonov/tmp/clang-di/test.cc] [DW_LANG_C_plus_plus]<br>
+!1 = metadata !{metadata !2}<br>
+!2 = metadata !{i32 0}<br>
+!3 = metadata !{metadata !4}<br>
+!4 = metadata !{metadata !5, metadata !8, metadata !9}<br>
+!5 = metadata !{i32 786478, i32 0, metadata !6, metadata !"run", metadata !"run", metadata !"", metadata !6, i32 8, metadata !7, i1 false, i1 true, i32 0, i32 0, null, i32 256, i1 false, void ()* @_Z3runv, null, null, metadata !1, i32 8} ; [ DW_TAG_subprogram ] [line 8] [def] [run]<br>
+!6 = metadata !{i32 786473, metadata !"test.cc", metadata !"/home/samsonov/tmp/clang-di", null} ; [ DW_TAG_file_type ]<br>
+!7 = metadata !{i32 786453, i32 0, metadata !"", i32 0, i32 0, i64 0, i64 0, i64 0, i32 0, null, metadata !2, i32 0, i32 0} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]<br>
+!8 = metadata !{i32 786478, i32 0, metadata !6, metadata !"dead_vararg", metadata !"dead_vararg", metadata !"", metadata !6, i32 5, metadata !7, i1 true, i1 true, i32 0, i32 0, null, i32 256, i1 false, void (...)* @_ZN12_GLOBAL__N_111dead_varargEz, null, null, metadata !1, i32 5} ; [ DW_TAG_subprogram ] [line 5] [local] [def] [dead_vararg]<br>
+<br>
+; CHECK: metadata !"dead_vararg"{{.*}}void ()* @_ZN12_GLOBAL__N_111dead_varargEz<br>
+<br>
+!9 = metadata !{i32 786478, i32 0, metadata !6, metadata !"dead_arg", metadata !"dead_arg", metadata !"", metadata !6, i32 4, metadata !7, i1 true, i1 true, i32 0, i32 0, null, i32 256, i1 false, void (i8*)* @_ZN12_GLOBAL__N_18dead_argEPv, null, null, metadata !1, i32 4} ; [ DW_TAG_subprogram ] [line 4] [local] [def] [dead_arg]<br>
+<br>
+; CHECK: metadata !"dead_arg"{{.*}}void ()* @_ZN12_GLOBAL__N_18dead_argEPv<br>
+<br>
+!10 = metadata !{i32 8, i32 14, metadata !11, null}<br>
+!11 = metadata !{i32 786443, metadata !5, i32 8, i32 12, metadata !6, i32 0} ; [ DW_TAG_lexical_block ] [/home/samsonov/tmp/clang-di/test.cc]<br>
+!12 = metadata !{i32 8, i32 27, metadata !11, null}<br>
+!13 = metadata !{i32 8, i32 42, metadata !11, null}<br>
+!14 = metadata !{i32 4, i32 28, metadata !15, null}<br>
+!15 = metadata !{i32 786443, metadata !9, i32 4, i32 26, metadata !6, i32 2} ; [ DW_TAG_lexical_block ] [/home/samsonov/tmp/clang-di/test.cc]<br>
+!16 = metadata !{i32 4, i32 33, metadata !15, null}<br>
+!17 = metadata !{i32 5, i32 25, metadata !18, null}<br>
+!18 = metadata !{i32 786443, metadata !8, i32 5, i32 23, metadata !6, i32 1} ; [ DW_TAG_lexical_block ] [/home/samsonov/tmp/clang-di/test.cc]<br>
+!19 = metadata !{i32 5, i32 30, metadata !18, null}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">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>
</blockquote></div></div></div><br></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div>Alexey Samsonov, MSK</div><br>