From c9163855dd8b582f75921e01e903c3c27a57663d Mon Sep 17 00:00:00 2001 From: David Stenberg Date: Tue, 16 Oct 2018 08:06:48 +0000 Subject: [PATCH] [DebugInfo][LCSSA] Rewrite pre-existing debug values outside loop Summary: Extend LCSSA so that debug values outside loops are rewritten to use the PHI nodes that the pass creates. This fixes PR39019. In that case, we ran LCSSA on a loop that was later on vectorized, which left us with something like this: for.cond.cleanup: %add.lcssa = phi i32 [ %add, %for.body ], [ %34, %middle.block ] call void @llvm.dbg.value(metadata i32 %add, ret i32 %add.lcssa for.body: %add = [...] br i1 %exitcond, label %for.cond.cleanup, label %for.body which later resulted in the debug.value becoming undef when removing the scalar loop (and the location would have probably been wrong for the vectorized case otherwise). As we now may need to query the AvailableVals cache more than once for a basic block, FindAvailableVals() in SSAUpdaterImpl is changed so that it updates the cache for blocks that we do not create a PHI node for, regardless of the block's number of predecessors. The debug value in the attached IR reproducer would not be properly rewritten without this. Debug values residing in blocks where we have not inserted any PHI nodes are currently left as-is by this patch. I'm not sure what should be done with those uses. Reviewers: mattd, aprantl, vsk, probinson Reviewed By: mattd, aprantl Subscribers: jmorse, gbedwell, JDevlieghere, llvm-commits Differential Revision: https://reviews.llvm.org/D53130 llvm-svn: 344589 --- .../llvm/Transforms/Utils/SSAUpdater.h | 4 ++ .../llvm/Transforms/Utils/SSAUpdaterImpl.h | 7 +- llvm/lib/Transforms/Utils/LCSSA.cpp | 16 +++++ llvm/lib/Transforms/Utils/SSAUpdater.cpp | 5 ++ .../LCSSA/rewrite-existing-dbg-values.ll | 69 +++++++++++++++++++ 5 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 llvm/test/Transforms/LCSSA/rewrite-existing-dbg-values.ll diff --git a/llvm/include/llvm/Transforms/Utils/SSAUpdater.h b/llvm/include/llvm/Transforms/Utils/SSAUpdater.h index 4a7911662990..d02607acbbb5 100644 --- a/llvm/include/llvm/Transforms/Utils/SSAUpdater.h +++ b/llvm/include/llvm/Transforms/Utils/SSAUpdater.h @@ -76,6 +76,10 @@ public: /// block. bool HasValueForBlock(BasicBlock *BB) const; + /// Return the value for the specified block if the SSAUpdater has one, + /// otherwise return nullptr. + Value *FindValueForBlock(BasicBlock *BB) const; + /// Construct SSA form, materializing a value that is live at the end /// of the specified block. Value *GetValueAtEndOfBlock(BasicBlock *BB); diff --git a/llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h b/llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h index b7649ba88334..cab0f3e71575 100644 --- a/llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h +++ b/llvm/include/llvm/Transforms/Utils/SSAUpdaterImpl.h @@ -357,10 +357,9 @@ public: BBInfo *Info = *I; if (Info->DefBB != Info) { - // Record the available value at join nodes to speed up subsequent - // uses of this SSAUpdater for the same value. - if (Info->NumPreds > 1) - (*AvailableVals)[Info->BB] = Info->DefBB->AvailableVal; + // Record the available value to speed up subsequent uses of this + // SSAUpdater for the same value. + (*AvailableVals)[Info->BB] = Info->DefBB->AvailableVal; continue; } diff --git a/llvm/lib/Transforms/Utils/LCSSA.cpp b/llvm/lib/Transforms/Utils/LCSSA.cpp index a1f8e7484bcf..53d444b309d5 100644 --- a/llvm/lib/Transforms/Utils/LCSSA.cpp +++ b/llvm/lib/Transforms/Utils/LCSSA.cpp @@ -41,6 +41,7 @@ #include "llvm/IR/Dominators.h" #include "llvm/IR/Function.h" #include "llvm/IR/Instructions.h" +#include "llvm/IR/IntrinsicInst.h" #include "llvm/IR/PredIteratorCache.h" #include "llvm/Pass.h" #include "llvm/Transforms/Utils.h" @@ -201,6 +202,21 @@ bool llvm::formLCSSAForInstructions(SmallVectorImpl &Worklist, SSAUpdate.RewriteUse(*UseToRewrite); } + SmallVector DbgValues; + llvm::findDbgValues(DbgValues, I); + + // Update pre-existing debug value uses that reside outside the loop. + auto &Ctx = I->getContext(); + for (auto DVI : DbgValues) { + BasicBlock *UserBB = DVI->getParent(); + if (InstBB == UserBB || L->contains(UserBB)) + continue; + // We currently only handle debug values residing in blocks where we have + // inserted a PHI instruction. + if (Value *V = SSAUpdate.FindValueForBlock(UserBB)) + DVI->setOperand(0, MetadataAsValue::get(Ctx, ValueAsMetadata::get(V))); + } + // SSAUpdater might have inserted phi-nodes inside other loops. We'll need // to post-process them to keep LCSSA form. for (PHINode *InsertedPN : InsertedPHIs) { diff --git a/llvm/lib/Transforms/Utils/SSAUpdater.cpp b/llvm/lib/Transforms/Utils/SSAUpdater.cpp index 4a1fd8d571aa..9e5fb0e7172d 100644 --- a/llvm/lib/Transforms/Utils/SSAUpdater.cpp +++ b/llvm/lib/Transforms/Utils/SSAUpdater.cpp @@ -64,6 +64,11 @@ bool SSAUpdater::HasValueForBlock(BasicBlock *BB) const { return getAvailableVals(AV).count(BB); } +Value *SSAUpdater::FindValueForBlock(BasicBlock *BB) const { + AvailableValsTy::iterator AVI = getAvailableVals(AV).find(BB); + return (AVI != getAvailableVals(AV).end()) ? AVI->second : nullptr; +} + void SSAUpdater::AddAvailableValue(BasicBlock *BB, Value *V) { assert(ProtoType && "Need to initialize SSAUpdater"); assert(ProtoType == V->getType() && diff --git a/llvm/test/Transforms/LCSSA/rewrite-existing-dbg-values.ll b/llvm/test/Transforms/LCSSA/rewrite-existing-dbg-values.ll new file mode 100644 index 000000000000..563a75f407f0 --- /dev/null +++ b/llvm/test/Transforms/LCSSA/rewrite-existing-dbg-values.ll @@ -0,0 +1,69 @@ +; RUN: opt -S -lcssa < %s | FileCheck %s + +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +; Reproducer for PR39019. +; +; Verify that the llvm.dbg.value in the %for.cond.cleanup2 block is rewritten +; to use the PHI node for %add that is created by LCSSA. + +; CHECK-LABEL: for.cond.cleanup2: +; CHECK-NEXT: [[PN:%[^ ]*]] = phi i32 [ %add.lcssa, %for.cond.cleanup1 ] +; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 [[PN]], metadata [[VAR:![0-9]+]], metadata !DIExpression()) +; CHECK-NEXT: call void @bar(i32 [[PN]]) + +; CHECK-LABEL: for.body: +; CHECK: %add = add nsw i32 0, 2 +; CHECK: call void @llvm.dbg.value(metadata i32 %add, metadata [[VAR]], metadata !DIExpression()) + +; CHECK: [[VAR]] = !DILocalVariable(name: "sum", + +; Function Attrs: nounwind +define void @foo() #0 !dbg !6 { +entry: + br label %for.cond.preheader, !dbg !12 + +for.cond.preheader: ; preds = %for.cond.cleanup1, %entry + br label %for.body, !dbg !12 + +for.cond.cleanup2: ; preds = %for.cond.cleanup1 + call void @llvm.dbg.value(metadata i32 %add, metadata !9, metadata !DIExpression()), !dbg !12 + tail call void @bar(i32 %add) #0, !dbg !12 + ret void, !dbg !12 + +for.cond.cleanup1: ; preds = %for.body + br i1 false, label %for.cond.preheader, label %for.cond.cleanup2, !dbg !12 + +for.body: ; preds = %for.body, %for.cond.preheader + %add = add nsw i32 0, 2, !dbg !12 + call void @llvm.dbg.value(metadata i32 %add, metadata !9, metadata !DIExpression()), !dbg !12 + br i1 false, label %for.body, label %for.cond.cleanup1, !dbg !12 +} + +; Function Attrs: nounwind +declare void @bar(i32) #0 + +; Function Attrs: nounwind readnone speculatable +declare void @llvm.dbg.value(metadata, metadata, metadata) #1 + +attributes #0 = { nounwind } +attributes #1 = { nounwind readnone speculatable } + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!3, !4} +!llvm.ident = !{!5} + +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 8.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, globals: !2, nameTableKind: None) +!1 = !DIFile(filename: "foo.c", directory: "/") +!2 = !{} +!3 = !{i32 2, !"Dwarf Version", i32 4} +!4 = !{i32 2, !"Debug Info Version", i32 3} +!5 = !{!"clang version 8.0.0"} +!6 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 10, type: !7, isLocal: false, isDefinition: true, scopeLine: 10, isOptimized: true, unit: !0, retainedNodes: !8) +!7 = !DISubroutineType(types: !2) +!8 = !{!9} +!9 = !DILocalVariable(name: "sum", scope: !10, file: !1, line: 11, type: !11) +!10 = !DILexicalBlockFile(scope: !6, file: !1, discriminator: 0) +!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) +!12 = !DILocation(line: 0, scope: !10)