[indvars] Missing variables at Og #88270
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
https://bugs.llvm.org/show_bug.cgi?id=51735 llvm#51077 In the given test case: 4 ... 5 void bar() { 6 int End = 777; 7 int Index = 27; 8 char Var = 1; 9 for (; Index < End; ++Index) 10 ; 11 nop(Index); 12 } 13 ... Missing local variable 'Index' after loop 'Induction Variable Elimination'. When adding a breakpoint at line 11, LLDB does not have information on the variable. But it has info on 'Var' and 'End'.
https://bugs.llvm.org/show_bug.cgi?id=51735 llvm#51077 In the given test case: 4 ... 5 void bar() { 6 int End = 777; 7 int Index = 27; 8 char Var = 1; 9 for (; Index < End; ++Index) 10 ; 11 nop(Index); 12 } 13 ... Missing local variable 'Index' after loop 'Induction Variable Elimination'. When adding a breakpoint at line 11, LLDB does not have information on the variable. But it has info on 'Var' and 'End'. - Moved the new logic to 'rewriteLoopExitValues'. - Removed from the test cases, the unrelated functions: 'nop' and 'main'. - Use 'isa<SCEVConstant>'.
https://bugs.llvm.org/show_bug.cgi?id=51735 llvm#51077 In the given test case: 4 ... 5 void bar() { 6 int End = 777; 7 int Index = 27; 8 char Var = 1; 9 for (; Index < End; ++Index) 10 ; 11 nop(); 12 } 13 ... Missing local variable 'Index' after loop 'Induction Variable Elimination'. When adding a breakpoint at line 11, LLDB does not have information on the variable. But it has info on 'Var' and 'End'. Address reviewers comments. - Early exit to simplify the logic. - Avoid inserting the same instruction in multiple blocks. - Skip debug-users with variadic variable locations. - Change some comments to improve readability. - Add code to clone and move the debug value. - Modify second test case to include multiple exit blocks.
https://bugs.llvm.org/show_bug.cgi?id=51735 llvm#51077 In the given test case: 4 ... 5 void bar() { 6 int End = 777; 7 int Index = 27; 8 char Var = 1; 9 for (; Index < End; ++Index) 10 ; 11 nop(); 12 } 13 ... Missing local variable 'Index' after loop 'Induction Variable Elimination'. When adding a breakpoint at line 11, LLDB does not have information on the variable. But it has info on 'Var' and 'End'. Address reviewers comments. - Early exit to simplify the logic. - Avoid inserting the same instruction in multiple blocks. - Skip debug-users with variadic variable locations. - Change some comments to improve readability. - Add code to clone and move the debug value. - Modify second test case to include multiple exit blocks. Addressed the upstream feedback in relation to: - Each exit block has its own exit value. - Separate the debug values for incoming and exit values.
https://bugs.llvm.org/show_bug.cgi?id=51735 llvm#51077 Fix clang-format found issues in updated patch.
https://bugs.llvm.org/show_bug.cgi?id=51735 llvm#51077 Addressed the upstream feedback in relation to: - Remove the introduced 'ExitBlock' field. - Update some comments.
https://bugs.llvm.org/show_bug.cgi?id=51735 llvm#51077 After internal discussion with @jmorse, it was decided to split the work between the 'indvars' and the 'loop-deletion' passes. 1) passes="loop(indvars)" - 'indvars' transformation is fired: the 'rewriteLoopExitValues' will rewrite the collected PNs with the exit values. - 'indvars' transformation is not fired: If the loop can be deleted, we preserve the induction variable information to be used by 'loop-deletion' if that pass will be executed. 2) passes="loop(indvars,loop-deletion)" If the loop is deleted in 'deleteDeadLoop' and there is a valid exit block, we use any collected values by 'indvars' to updated the exit values. Added extra tests to cover the following cases: ... char Var = 1; for (; Index < End; ++Index) if (Index == 666) ++Var; ... and ... char Var = 1; for (; Index < End; ++Index) if (Index == 666) Var = 555; ... Modified but otherwise unused variable 'Var' in a loop that gets deleted.
https://bugs.llvm.org/show_bug.cgi?id=51735 llvm#51077 Address @SLTozer comments. - Use captured variables for SSA values. - Use new insert logic calls. - Remove not required 'CHECK-NOT'.
https://bugs.llvm.org/show_bug.cgi?id=51735 llvm#51077 Address @SLTozer comments. - Don't use any names for not used SSA values.
https://bugs.llvm.org/show_bug.cgi?id=51735 llvm#51077 Address @nikic comments about layering violation. - Move code to 'LoopInfo.h'.
CarlosAlbertoEnciso
requested review from
nikic,
jryans,
jmorse,
adrian-prantl,
SLTozer,
felipepiovezan,
OCHyams,
dwblaikie,
pogo59 and
djtodoro
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-debuginfo Author: Carlos Alberto Enciso (CarlosAlbertoEnciso) Changeshttps://bugs.llvm.org/show_bug.cgi?id=51735 In the given test case: Missing local variable Patch is 31.82 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/88270.diff 7 Files Affected:
diff --git a/llvm/include/llvm/Analysis/LoopInfo.h b/llvm/include/llvm/Analysis/LoopInfo.h
index 52084630560c55..072ca4ee479923 100644
--- a/llvm/include/llvm/Analysis/LoopInfo.h
+++ b/llvm/include/llvm/Analysis/LoopInfo.h
@@ -18,6 +18,8 @@
#include "llvm/IR/CFG.h"
#include "llvm/IR/Instructions.h"
#include "llvm/IR/PassManager.h"
+#include "llvm/IR/IntrinsicInst.h"
+#include "llvm/IR/ValueHandle.h"
#include "llvm/Pass.h"
#include "llvm/Support/GenericLoopInfo.h"
#include <algorithm>
@@ -392,6 +394,21 @@ class LLVM_EXTERNAL_VISIBILITY Loop : public LoopBase<BasicBlock, Loop> {
return "<unnamed loop>";
}
+ /// Preserve the induction variable exit value and its debug users by the
+ /// 'indvars' pass if the loop can deleted. Those debug users will be used
+ /// by the 'loop-delete' pass.
+ void preserveDebugInductionVariableInfo(
+ Value *FinalValue, SmallVector<DbgVariableIntrinsic *> DbgUsers) {
+ IndVarFinalValue = FinalValue;
+ for (DbgVariableIntrinsic *DebugUser : DbgUsers)
+ IndVarDebugUsers.push_back(DebugUser);
+ }
+
+ Value *getDebugInductionVariableFinalValue() { return IndVarFinalValue; }
+ SmallVector<WeakVH> &getDebugInductionVariableDebugUsers() {
+ return IndVarDebugUsers;
+ }
+
private:
Loop() = default;
@@ -399,6 +416,13 @@ class LLVM_EXTERNAL_VISIBILITY Loop : public LoopBase<BasicBlock, Loop> {
friend class LoopBase<BasicBlock, Loop>;
explicit Loop(BasicBlock *BB) : LoopBase<BasicBlock, Loop>(BB) {}
~Loop() = default;
+
+ // Induction variable exit value and its debug users, preserved by the
+ // 'indvars' pass, when it detects that the loop can be deleted and the
+ // there are no PHIs to be rewritten.
+ // For now, we only preserve single induction variables.
+ Value *IndVarFinalValue = nullptr;
+ SmallVector<WeakVH> IndVarDebugUsers;
};
// Implementation in Support/GenericLoopInfoImpl.h
diff --git a/llvm/include/llvm/Transforms/Utils/LoopUtils.h b/llvm/include/llvm/Transforms/Utils/LoopUtils.h
index 187ace3a0cbedf..736f4a630c7b3e 100644
--- a/llvm/include/llvm/Transforms/Utils/LoopUtils.h
+++ b/llvm/include/llvm/Transforms/Utils/LoopUtils.h
@@ -477,6 +477,12 @@ int rewriteLoopExitValues(Loop *L, LoopInfo *LI, TargetLibraryInfo *TLI,
ReplaceExitVal ReplaceExitValue,
SmallVector<WeakTrackingVH, 16> &DeadInsts);
+/// Assign exit values to variables that use this loop variable during the loop.
+void addDebugValuesToIncomingValue(BasicBlock *Successor, Value *IndVar,
+ PHINode *PN);
+void addDebugValuesToLoopVariable(BasicBlock *Successor, Value *ExitValue,
+ PHINode *PN);
+
/// Set weights for \p UnrolledLoop and \p RemainderLoop based on weights for
/// \p OrigLoop and the following distribution of \p OrigLoop iteration among \p
/// UnrolledLoop and \p RemainderLoop. \p UnrolledLoop receives weights that
diff --git a/llvm/lib/Transforms/Utils/LoopUtils.cpp b/llvm/lib/Transforms/Utils/LoopUtils.cpp
index 002bc90c9b5677..1ffd390ea5accc 100644
--- a/llvm/lib/Transforms/Utils/LoopUtils.cpp
+++ b/llvm/lib/Transforms/Utils/LoopUtils.cpp
@@ -31,6 +31,7 @@
#include "llvm/Analysis/ScalarEvolutionAliasAnalysis.h"
#include "llvm/Analysis/ScalarEvolutionExpressions.h"
#include "llvm/IR/DIBuilder.h"
+#include "llvm/IR/DebugInfo.h"
#include "llvm/IR/Dominators.h"
#include "llvm/IR/Instructions.h"
#include "llvm/IR/IntrinsicInst.h"
@@ -607,6 +608,17 @@ void llvm::deleteDeadLoop(Loop *L, DominatorTree *DT, ScalarEvolution *SE,
llvm::SmallVector<DPValue *, 4> DeadDPValues;
if (ExitBlock) {
+ if (ExitBlock->phis().empty()) {
+ // As the loop is deleted, replace the debug users with the preserved
+ // induction variable final value recorded by the 'indvar' pass.
+ Value *FinalValue = L->getDebugInductionVariableFinalValue();
+ SmallVector<WeakVH> &DbgUsers = L->getDebugInductionVariableDebugUsers();
+ for (WeakVH &DebugUser : DbgUsers)
+ if (DebugUser)
+ cast<DbgVariableIntrinsic>(DebugUser)->replaceVariableLocationOp(
+ 0u, FinalValue);
+ }
+
// Given LCSSA form is satisfied, we should not have users of instructions
// within the dead loop outside of the loop. However, LCSSA doesn't take
// unreachable uses into account. We handle them here.
@@ -1412,6 +1424,36 @@ static bool checkIsIndPhi(PHINode *Phi, Loop *L, ScalarEvolution *SE,
return InductionDescriptor::isInductionPHI(Phi, L, SE, ID);
}
+void llvm::addDebugValuesToIncomingValue(BasicBlock *Successor, Value *IndVar,
+ PHINode *PN) {
+ SmallVector<DbgVariableIntrinsic *> DbgUsers;
+ findDbgUsers(DbgUsers, IndVar);
+ for (auto *DebugUser : DbgUsers) {
+ // Skip debug-users with variadic variable locations; they will not,
+ // get updated, which is fine as that is the existing behaviour.
+ if (DebugUser->hasArgList())
+ continue;
+ auto *Cloned = cast<DbgVariableIntrinsic>(DebugUser->clone());
+ Cloned->replaceVariableLocationOp(0u, PN);
+ Cloned->insertBefore(*Successor, Successor->getFirstNonPHIIt());
+ }
+}
+
+void llvm::addDebugValuesToLoopVariable(BasicBlock *Successor, Value *ExitValue,
+ PHINode *PN) {
+ SmallVector<DbgVariableIntrinsic *> DbgUsers;
+ findDbgUsers(DbgUsers, PN);
+ for (auto *DebugUser : DbgUsers) {
+ // Skip debug-users with variadic variable locations; they will not,
+ // get updated, which is fine as that is the existing behaviour.
+ if (DebugUser->hasArgList())
+ continue;
+ auto *Cloned = cast<DbgVariableIntrinsic>(DebugUser->clone());
+ Cloned->replaceVariableLocationOp(0u, ExitValue);
+ Cloned->insertBefore(*Successor, Successor->getFirstNonPHIIt());
+ }
+}
+
int llvm::rewriteLoopExitValues(Loop *L, LoopInfo *LI, TargetLibraryInfo *TLI,
ScalarEvolution *SE,
const TargetTransformInfo *TTI,
@@ -1553,6 +1595,10 @@ int llvm::rewriteLoopExitValues(Loop *L, LoopInfo *LI, TargetLibraryInfo *TLI,
(isa<PHINode>(Inst) || isa<LandingPadInst>(Inst)) ?
&*Inst->getParent()->getFirstInsertionPt() : Inst;
RewritePhiSet.emplace_back(PN, i, ExitValue, InsertPt, HighCost);
+
+ // Add debug values for the candidate PHINode incoming value.
+ if (BasicBlock *Successor = ExitBB->getSingleSuccessor())
+ addDebugValuesToIncomingValue(Successor, PN->getIncomingValue(i), PN);
}
}
}
@@ -1611,11 +1657,30 @@ int llvm::rewriteLoopExitValues(Loop *L, LoopInfo *LI, TargetLibraryInfo *TLI,
// Replace PN with ExitVal if that is legal and does not break LCSSA.
if (PN->getNumIncomingValues() == 1 &&
LI->replacementPreservesLCSSAForm(PN, ExitVal)) {
+ addDebugValuesToLoopVariable(PN->getParent(), ExitVal, PN);
PN->replaceAllUsesWith(ExitVal);
PN->eraseFromParent();
}
}
+ // If the loop can be deleted and there are no PHIs to be rewritten (there
+ // are no loop live-out values), record debug variables corresponding to the
+ // induction variable with their constant exit-values. Those values will be
+ // inserted by the 'deletion loop' logic.
+ if (LoopCanBeDel && RewritePhiSet.empty()) {
+ if (auto *IndVar = L->getInductionVariable(*SE)) {
+ const SCEV *PNSCEV = SE->getSCEVAtScope(IndVar, L->getParentLoop());
+ if (auto *Const = dyn_cast<SCEVConstant>(PNSCEV)) {
+ Value *FinalIVValue = Const->getValue();
+ if (L->getUniqueExitBlock()) {
+ SmallVector<DbgVariableIntrinsic *> DbgUsers;
+ findDbgUsers(DbgUsers, IndVar);
+ L->preserveDebugInductionVariableInfo(FinalIVValue, DbgUsers);
+ }
+ }
+ }
+ }
+
// The insertion point instruction may have been deleted; clear it out
// so that the rewriter doesn't trip over it later.
Rewriter.clearInsertPoint();
diff --git a/llvm/test/Transforms/IndVarSimplify/pr51735-1.ll b/llvm/test/Transforms/IndVarSimplify/pr51735-1.ll
new file mode 100644
index 00000000000000..356217985fed11
--- /dev/null
+++ b/llvm/test/Transforms/IndVarSimplify/pr51735-1.ll
@@ -0,0 +1,129 @@
+; RUN: opt -passes="loop(indvars)" \
+; RUN: --experimental-debuginfo-iterators=false -S -o - < %s | \
+; RUN: FileCheck --check-prefix=CHECK %s
+; RUN: opt -passes="loop(indvars,loop-deletion)" \
+; RUN: --experimental-debuginfo-iterators=false -S -o - < %s | \
+; RUN: FileCheck --check-prefix=CHECK %s
+
+; Make sure that when we delete the loop, that the variable Index has
+; the 777 value.
+
+; As this test case does fire the 'indvars' transformation, the debug values
+; are added to the 'for.end' exit block. No debug values are preserved by the
+; pass to be used by the 'loop-deletion' pass.
+
+; CHECK: for.cond:
+; CHECK: call void @llvm.dbg.value(metadata i32 %[[SSA_INDEX_0:.+]], metadata ![[DBG:[0-9]+]], {{.*}}
+
+; CHECK: for.extra:
+; CHECK: %[[SSA_CALL_0:.+]] = call noundef i32 @"?nop@@YAHH@Z"(i32 noundef %[[SSA_INDEX_0]]), {{.*}}
+; CHECK: br i1 %[[SSA_CMP_0:.+]], label %for.cond, label %if.else, {{.*}}
+
+; CHECK: if.then:
+; CHECK: call void @llvm.dbg.value(metadata i32 777, metadata ![[DBG]], {{.*}}
+; CHECK: call void @llvm.dbg.value(metadata i32 %[[SSA_VAR_1:.+]], metadata ![[VAR:[0-9]+]], {{.*}}
+; CHECK: br label %for.end, {{.*}}
+
+; CHECK: if.else:
+; CHECK: call void @llvm.dbg.value(metadata i32 %[[SSA_VAR_2:.+]], metadata ![[VAR:[0-9]+]], {{.*}}
+; CHECK: br label %for.end, {{.*}}
+
+; CHECK: for.end:
+; CHECK: call void @llvm.dbg.value(metadata i32 777, metadata ![[DBG]], {{.*}}
+
+; CHECK-DAG: ![[DBG]] = !DILocalVariable(name: "Index"{{.*}})
+; CHECK-DAG: ![[VAR]] = !DILocalVariable(name: "Var"{{.*}})
+
+define dso_local noundef i32 @"?nop@@YAHH@Z"(i32 noundef %Param) !dbg !11 {
+entry:
+ %Param.addr = alloca i32, align 4
+ store i32 %Param, ptr %Param.addr, align 4
+ call void @llvm.dbg.declare(metadata ptr %Param.addr, metadata !32, metadata !DIExpression()), !dbg !35
+ ret i32 0, !dbg !36
+}
+
+define dso_local void @_Z3barv() local_unnamed_addr #1 !dbg !12 {
+entry:
+ call void @llvm.dbg.value(metadata i32 777, metadata !16, metadata !DIExpression()), !dbg !17
+ call void @llvm.dbg.value(metadata i32 27, metadata !18, metadata !DIExpression()), !dbg !17
+ call void @llvm.dbg.value(metadata i32 1, metadata !19, metadata !DIExpression()), !dbg !17
+ call void @llvm.dbg.value(metadata i32 1, metadata !30, metadata !DIExpression()), !dbg !17
+ br label %for.cond, !dbg !20
+
+for.cond: ; preds = %for.cond, %entry
+ %Index.0 = phi i32 [ 27, %entry ], [ %inc, %for.extra ], !dbg !17
+ call void @llvm.dbg.value(metadata i32 %Index.0, metadata !18, metadata !DIExpression()), !dbg !17
+ %cmp = icmp ult i32 %Index.0, 777, !dbg !21
+ %inc = add nuw nsw i32 %Index.0, 1, !dbg !24
+ call void @llvm.dbg.value(metadata i32 %inc, metadata !18, metadata !DIExpression()), !dbg !17
+ br i1 %cmp, label %for.extra, label %if.then, !dbg !25, !llvm.loop !26
+
+for.extra:
+ %call.0 = call noundef i32 @"?nop@@YAHH@Z"(i32 noundef %Index.0), !dbg !21
+ %cmp.0 = icmp ult i32 %Index.0, %call.0, !dbg !21
+ br i1 %cmp.0, label %for.cond, label %if.else, !dbg !25, !llvm.loop !26
+
+if.then: ; preds = %for.cond
+ %Var.1 = add nsw i32 %Index.0, 1, !dbg !20
+ call void @llvm.dbg.value(metadata i32 %Var.1, metadata !19, metadata !DIExpression()), !dbg !20
+ br label %for.end, !dbg !20
+
+if.else:
+ %Var.2 = add nsw i32 %Index.0, 2, !dbg !20
+ call void @llvm.dbg.value(metadata i32 %Var.2, metadata !19, metadata !DIExpression()), !dbg !20
+ br label %for.end, !dbg !20
+
+for.end: ; preds = %if.else, %if.then
+ %Zeta.0 = phi i32 [ %Var.1, %if.then ], [ %Var.2, %if.else ], !dbg !20
+ call void @llvm.dbg.value(metadata i32 %Zeta.0, metadata !30, metadata !DIExpression()), !dbg !20
+ %Var.3 = add nsw i32 %Index.0, 1, !dbg !20
+ call void @llvm.dbg.value(metadata i32 %Var.3, metadata !19, metadata !DIExpression()), !dbg !20
+ %call = call noundef i32 @"?nop@@YAHH@Z"(i32 noundef %Index.0), !dbg !37
+ ret void, !dbg !29
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+declare void @llvm.dbg.declare(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8}
+!llvm.ident = !{!9}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "test.cpp", directory: "")
+!2 = !{i32 7, !"Dwarf Version", i32 5}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 8, !"PIC Level", i32 2}
+!6 = !{i32 7, !"PIE Level", i32 2}
+!7 = !{i32 7, !"uwtable", i32 2}
+!8 = !{i32 7, !"frame-pointer", i32 2}
+!9 = !{!"clang version 18.0.0"}
+!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!11 = distinct !DISubprogram(name: "nop", linkageName: "?nop@@YAHH@Z", scope: !1, file: !1, line: 1, type: !33, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !31)
+!12 = distinct !DISubprogram(name: "bar", linkageName: "_Z3barv", scope: !1, file: !1, line: 5, type: !13, scopeLine: 5, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !15)
+!13 = !DISubroutineType(types: !14)
+!14 = !{null}
+!15 = !{}
+!16 = !DILocalVariable(name: "End", scope: !12, file: !1, line: 6, type: !10)
+!17 = !DILocation(line: 0, scope: !12)
+!18 = !DILocalVariable(name: "Index", scope: !12, file: !1, line: 7, type: !10)
+!19 = !DILocalVariable(name: "Var", scope: !12, file: !1, line: 8, type: !10)
+!20 = !DILocation(line: 9, column: 3, scope: !12)
+!21 = !DILocation(line: 9, column: 16, scope: !22)
+!22 = distinct !DILexicalBlock(scope: !23, file: !1, line: 9, column: 3)
+!23 = distinct !DILexicalBlock(scope: !12, file: !1, line: 9, column: 3)
+!24 = !DILocation(line: 9, column: 23, scope: !22)
+!25 = !DILocation(line: 9, column: 3, scope: !23)
+!26 = distinct !{!26, !25, !27, !28}
+!27 = !DILocation(line: 10, column: 5, scope: !23)
+!28 = !{!"llvm.loop.mustprogress"}
+!29 = !DILocation(line: 12, column: 1, scope: !12)
+!30 = !DILocalVariable(name: "Zeta", scope: !12, file: !1, line: 8, type: !10)
+!31 = !{!32}
+!32 = !DILocalVariable(name: "Param", arg: 1, scope: !11, file: !1, line: 1, type: !10)
+!33 = !DISubroutineType(types: !34)
+!34 = !{!10, !10}
+!35 = !DILocation(line: 1, scope: !11)
+!36 = !DILocation(line: 2, scope: !11)
+!37 = !DILocation(line: 20, scope: !12)
diff --git a/llvm/test/Transforms/IndVarSimplify/pr51735-2.ll b/llvm/test/Transforms/IndVarSimplify/pr51735-2.ll
new file mode 100644
index 00000000000000..58cc9932e04ca8
--- /dev/null
+++ b/llvm/test/Transforms/IndVarSimplify/pr51735-2.ll
@@ -0,0 +1,128 @@
+; RUN: opt -passes="loop(indvars)" \
+; RUN: --experimental-debuginfo-iterators=false -S -o - < %s | \
+; RUN: FileCheck --implicit-check-not="call void @llvm.dbg" \
+; RUN: --check-prefix=ALL-CHECK --check-prefix=PRE-CHECK %s
+; RUN: opt -passes="loop(indvars,loop-deletion)" \
+; RUN: --experimental-debuginfo-iterators=false -S -o - < %s | \
+; RUN: FileCheck --implicit-check-not="call void @llvm.dbg" \
+; RUN: --check-prefix=ALL-CHECK --check-prefix=POST-CHECK %s
+
+; Check what happens to a modified but otherwise unused variable in a loop
+; that gets deleted. The assignment in the loop is 'forgotten' by LLVM and
+; doesn't appear in the debugging information. This behaviour is suboptimal,
+; but we want to know if it changes
+
+; For all cases, LLDB shows
+; Var = <no location, value may have been optimized out>
+
+; 1 __attribute__((optnone)) int nop() {
+; 2 return 0;
+; 3 }
+; 4
+; 5 void bar() {
+; 6 int End = 777;
+; 7 int Index = 27;
+; 8 char Var = 1;
+; 9 for (; Index < End; ++Index) {
+; 10 if (Index == 666) {
+; 11 ++Var;
+; 12 }
+; 13 }
+; 14 nop();
+; 15 }
+
+; ALL-CHECK: entry:
+; ALL-CHECK: call void @llvm.dbg.value(metadata i32 1, metadata ![[DBG:[0-9]+]], {{.*}}
+
+; Only the 'indvars' pass is executed.
+; PRE-CHECK: for.cond:
+; PRE-CHECK: call void @llvm.dbg.value(metadata i32 %[[SSA_VAR_0:.+]], metadata ![[DBG]], {{.*}}
+; PRE-CHECK: call void @llvm.dbg.value(metadata !DIArgList{{.*}}
+
+; PRE-CHECK: for.body:
+; PRE-CHECK: {{.*}} = icmp eq i32 %[[SSA_INDEX_0:.+]], 666
+; PRE-CHECK: {{.*}} = add nsw i32 %[[SSA_VAR_0]], 1
+; PRE-CHECK: {{.*}} = select i1 {{.*}}, i32 {{.*}}, i32 %[[SSA_VAR_0]]
+; PRE-CHECK: call void @llvm.dbg.value(metadata i32 {{.*}}, metadata ![[DBG]], {{.*}}
+; PRE-CHECK: br label %for.cond
+
+; PRE-CHECK: for.end:
+; PRE-CHECK: ret void
+; PRE-CHECK-DAG: ![[DBG]] = !DILocalVariable(name: "Var"{{.*}})
+
+; The 'indvars' and 'loop-deletion' passes are executed.
+; POST-CHECK: for.end:
+; POST-CHECK: call void @llvm.dbg.value(metadata i32 undef, metadata ![[DBG:[0-9]+]], {{.*}}
+; POST-CHECK: ret void
+; POST-CHECK-DAG: ![[DBG]] = !DILocalVariable(name: "Var"{{.*}})
+
+define dso_local void @_Z3barv() local_unnamed_addr !dbg !18 {
+entry:
+ call void @llvm.dbg.value(metadata i32 1, metadata !24, metadata !DIExpression()), !dbg !22
+ br label %for.cond, !dbg !25
+
+for.cond: ; preds = %for.cond, %entry
+ %Index.0 = phi i32 [ 27, %entry ], [ %inc2, %for.body ], !dbg !22
+ %Var.0 = phi i32 [ 1, %entry ], [ %spec.select, %for.body ], !dbg !22
+ call void @llvm.dbg.value(metadata i32 %Var.0, metadata !24, metadata !DIExpression()), !dbg !22
+ %cmp = icmp ult i32 %Index.0, 777, !dbg !26
+ call void @llvm.dbg.value(metadata !DIArgList(i32 poison, i32 %Index.0), metadata !24, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_constu, 666, DW_OP_eq, DW_OP_LLVM_convert, 1, DW_ATE_unsigned, DW_OP_LLVM_convert, 32, DW_ATE_unsigned, DW_OP_plus, DW_OP_stack_value)), !dbg !22
+ %inc2 = add nuw nsw i32 %Index.0, 1, !dbg !29
+ br i1 %cmp, label %for.body, label %for.end, !dbg !30, !llvm.loop !31
+
+for.body: ; preds = %for.cond
+ %cmp1 = icmp eq i32 %Index.0, 666, !dbg !30
+ %inc = add nsw i32 %Var.0, 1
+ %spec.select = select i1 %cmp1, i32 %inc, i32 %Var.0, !dbg !32
+ call void @llvm.dbg.value(metadata i32 %spec.select, metadata !24, metadata !DIExpression()), !dbg !22
+ br label %for.cond, !dbg !34, !llvm.loop !35
+
+for.end: ; preds = %for.cond
+ ret void, !dbg !35
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8}
+!llvm.ident = !{!9}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "test-b.cpp", directory: "")
+!2 = !{i32 7, !"Dwarf Version", i32 5}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 8, !"PIC Level", i32 2}
+!6 = !{i32 7, !"PIE Level", i32 2}
+!7 = !{i32 7, !"uwtable", i32 2}
+!8 = !{i32 7, !"frame-pointer", i32 2}
+!9 = !{!"clang version 19.0.0"}
+!10 = distinct !DISubprogram(name: "nop", linkageName: "_Z3nopi", scope: !1, file: !1, line: 1, type: !11, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !14)
+!11 = !DISubroutineType(types: !12)
+!12 = !{!13, !13}
+!13 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!14 = !{}
+!15 = !DILocalVariable(name: "Param", arg: 1, scope: !10, file: !1, line: 1, type: !13)
+!16 = !DILocation(line: 1, column: 38, scope: !10)
+!17 = !DILocation(line: 2, column: 3, scope: !10)
+!18 = distinct !DISubprogram(name: "bar", linkageName: "_Z3barv", scope: !1, file: !1, line: 5, type: !19, scopeLine: 5, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !14)
+!19 = !DISubroutineType(types: !20)
+!20 = !{null}
+!22 = !DILocation(line: 0, scope: !18)
+!24 = !DILocalVariable(name: "Var", scope: !18, file: !1, line: 8, type: !13)
+!25 = !DILocation(line: 9, column...
[truncated]
|
|
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
https://bugs.llvm.org/show_bug.cgi?id=51735 llvm#51077 Fix Clang-format issue.
OCHyams
reviewed
Apr 15, 2024
| /// 'indvars' pass if the loop can deleted. Those debug users will be used | ||
| /// by the 'loop-delete' pass. | ||
| void preserveDebugInductionVariableInfo( | ||
| Value *FinalValue, SmallVector<DbgVariableIntrinsic *> DbgUsers) { |
There was a problem hiding this comment.
I think it makes sense to delete the DbgUsers parameter and do the findDbgUsers call from within this function?
(If you disagree, please update DbgUsers to be a const SmallVectorImpl reference or ArrayRef)
There was a problem hiding this comment.
To avoid adding a dependency on DebugInfo.h, making DbgUsers to be const SmallVectorImpl reference.
| if (L->getUniqueExitBlock()) { | ||
| SmallVector<DbgVariableIntrinsic *> DbgUsers; | ||
| findDbgUsers(DbgUsers, IndVar); | ||
| L->preserveDebugInductionVariableInfo(FinalIVValue, DbgUsers); |
There was a problem hiding this comment.
Is there a reason that we can't insert a dbg.value in the exit block right now, here, rather than waiting for loop-deletion to delete the loop?
There was a problem hiding this comment.
@OCHyams Sorry for my delay in answering your question.
// If the loop can be deleted and there are no PHIs to be rewritten (there
// are no loop live-out values), record debug variables corresponding to the
// induction variable with their constant exit-values. Those values will be
// inserted by the 'deletion loop' logic.
if (LoopCanBeDel && RewritePhiSet.empty()) {
...
if (L->getUniqueExitBlock()) {
...
}
}
The RewritePhiSet.empty() indicates that the indvars pass has not being triggered (No PHIs to be rewritten); therefore no changes should be made. That was raised on early feedback by @nikic. And as we know that the loop can be deleted, we just collect the information and pass it to the loop-deletion pass.
There was a problem hiding this comment.
TBH I still think the whole design here makes little sense. I think this case should either just be Won't Fix, or LoopDeletion needs to compute the exit value, rather than relying on some complex interaction with IndVars (which is not the only user of LoopDeletion!)
There was a problem hiding this comment.
@nikic Patch already reverted.
If I understand correctly, your main concern is the interaction between IndVars and LoopDeletion passes.
What I would suggest, is to divide the patch into 2 independent ones:
- If the
IndVarspass is triggered add the missing debug values. LoopDeletionto compute the exit value.
With no interaction between those 2 passes.
Your feedback would be highly valuable.
The main discussion is here: If you prefer, I can elaborate more if is not clear. |
Thanks, I'd missed that this is a re-posting of #69920, which already had approval. My other small inline comment still stands though (the "surface level" one). |
OCHyams
approved these changes
Apr 25, 2024
https://bugs.llvm.org/show_bug.cgi?id=51735 llvm#51077 Address @OCHyams comments about DbgUsers type. - Converted to 'const SmallVectorImpl reference'.
|
@OCHyams Thanks for your review. |
nikic
reviewed
May 22, 2024
| #include "llvm/IR/PassManager.h" | ||
| #include "llvm/IR/ValueHandle.h" |
There was a problem hiding this comment.
Adding these includes to a core header like LoopInfo.h adds significant compile-time overhead.
|
@nikic Thanks for your valuable feedback. |
CarlosAlbertoEnciso
added a commit
that referenced
this pull request
May 22, 2024
This reverts commit 89e1f77. #88270 (comment) #88270 (comment) Main concerns from @nikic are the interaction between the 'IndVars' and 'LoopDeletion' passes, increasing build times and adding extra complexity.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://bugs.llvm.org/show_bug.cgi?id=51735
#51077
In the given test case:
Missing local variable
Indexafter loopInduction Variable Elimination.When adding a breakpoint at line
11, LLDB does not have informationon the variable. But it has info on
VarandEnd.