From 86eeaa077ffdd39a05484ef32fb0b0db770c6663 Mon Sep 17 00:00:00 2001 From: Carlos Alberto Enciso Date: Tue, 9 Jan 2024 13:16:01 +0000 Subject: [PATCH] [indvars] Missing variables at Og: https://bugs.llvm.org/show_bug.cgi?id=51735 https://github.com/llvm/llvm-project/issues/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. --- .../include/llvm/Transforms/Utils/LoopUtils.h | 5 +- llvm/lib/Transforms/Utils/LoopUtils.cpp | 52 ++++++++++--------- .../test/Transforms/LoopSimplify/pr51735-1.ll | 4 +- 3 files changed, 34 insertions(+), 27 deletions(-) diff --git a/llvm/include/llvm/Transforms/Utils/LoopUtils.h b/llvm/include/llvm/Transforms/Utils/LoopUtils.h index 26230fe97f6786..3328e9955e1258 100644 --- a/llvm/include/llvm/Transforms/Utils/LoopUtils.h +++ b/llvm/include/llvm/Transforms/Utils/LoopUtils.h @@ -469,8 +469,11 @@ int rewriteLoopExitValues(Loop *L, LoopInfo *LI, TargetLibraryInfo *TLI, SmallVector &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); void addDebugValuesToLoopVariable(Loop *L, ScalarEvolution *SE, PHINode *PN); -void addDebugValuesToLoopVariable(Loop *L, 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 diff --git a/llvm/lib/Transforms/Utils/LoopUtils.cpp b/llvm/lib/Transforms/Utils/LoopUtils.cpp index 5109666500c3f9..45408ff804e007 100644 --- a/llvm/lib/Transforms/Utils/LoopUtils.cpp +++ b/llvm/lib/Transforms/Utils/LoopUtils.cpp @@ -1267,11 +1267,12 @@ struct RewritePhi { const SCEV *ExpansionSCEV; // The SCEV of the incoming value we are rewriting. Instruction *ExpansionPoint; // Where we'd like to expand that SCEV? bool HighCost; // Is this expansion a high-cost? + BasicBlock *ExitBlock; // Exit block for PHI node. RewritePhi(PHINode *P, unsigned I, const SCEV *Val, Instruction *ExpansionPt, - bool H) + bool H, BasicBlock *Exit) : PN(P), Ith(I), ExpansionSCEV(Val), ExpansionPoint(ExpansionPt), - HighCost(H) {} + HighCost(H), ExitBlock(Exit) {} }; // Check whether it is possible to delete the loop after rewriting exit @@ -1342,10 +1343,19 @@ static bool checkIsIndPhi(PHINode *Phi, Loop *L, ScalarEvolution *SE, return InductionDescriptor::isInductionPHI(Phi, L, SE, ID); } -void llvm::addDebugValuesToLoopVariable(Loop *L, Value *ExitValue, +void llvm::addDebugValuesToIncomingValue(BasicBlock *Successor, Value *IndVar, + PHINode *PN) { + SmallVector DbgUsers; + findDbgUsers(DbgUsers, IndVar); + for (auto *DebugUser : DbgUsers) { + auto *Cloned = cast(DebugUser->clone()); + Cloned->replaceVariableLocationOp(0u, PN); + Cloned->insertBefore(Successor->getFirstNonPHI()); + } +} + +void llvm::addDebugValuesToLoopVariable(BasicBlock *Successor, Value *ExitValue, PHINode *PN) { - SmallVector ExitBlocks; - L->getExitBlocks(ExitBlocks); SmallVector DbgUsers; findDbgUsers(DbgUsers, PN); for (auto *DebugUser : DbgUsers) { @@ -1353,11 +1363,9 @@ void llvm::addDebugValuesToLoopVariable(Loop *L, Value *ExitValue, // get updated, which is fine as that is the existing behaviour. if (DebugUser->hasArgList()) continue; - for (BasicBlock *Exit : ExitBlocks) { - auto *Cloned = cast(DebugUser->clone()); - Cloned->replaceVariableLocationOp(0u, ExitValue); - Cloned->insertBefore(Exit->getFirstNonPHI()); - } + auto *Cloned = cast(DebugUser->clone()); + Cloned->replaceVariableLocationOp(0u, ExitValue); + Cloned->insertBefore(Successor->getFirstNonPHI()); } } @@ -1368,7 +1376,10 @@ void llvm::addDebugValuesToLoopVariable(Loop *L, ScalarEvolution *SE, const SCEV *PNSCEV = SE->getSCEVAtScope(PN, L->getParentLoop()); if (auto *Const = dyn_cast(PNSCEV)) { Value *FinalIVValue = Const->getValue(); - addDebugValuesToLoopVariable(L, FinalIVValue, PN); + SmallVector ExitBlocks; + L->getExitBlocks(ExitBlocks); + for (BasicBlock *Exit : ExitBlocks) + addDebugValuesToLoopVariable(Exit, FinalIVValue, PN); } } @@ -1512,22 +1523,13 @@ int llvm::rewriteLoopExitValues(Loop *L, LoopInfo *LI, TargetLibraryInfo *TLI, Instruction *InsertPt = (isa(Inst) || isa(Inst)) ? &*Inst->getParent()->getFirstInsertionPt() : Inst; - RewritePhiSet.emplace_back(PN, i, ExitValue, InsertPt, HighCost); + RewritePhiSet.emplace_back(PN, i, ExitValue, InsertPt, HighCost, ExitBB); // Add debug values if the PN is a induction variable. PHINode *IndVar = L->getInductionVariable(*SE); - for (Value *V : PN->incoming_values()) - if (V == IndVar) { - if (BasicBlock *Successor = ExitBB->getSingleSuccessor()) { - SmallVector DbgUsers; - findDbgUsers(DbgUsers, V); - for (auto *DebugUser : DbgUsers) { - auto *Cloned = cast(DebugUser->clone()); - Cloned->replaceVariableLocationOp(0u, PN); - Cloned->insertBefore(Successor->getFirstNonPHI()); - } - } - } + if (PN->getIncomingValue(i) == IndVar) + if (BasicBlock *Successor = ExitBB->getSingleSuccessor()) + addDebugValuesToIncomingValue(Successor, PN->getIncomingValue(i), PN); } } } @@ -1586,7 +1588,7 @@ 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(L, ExitVal, PN); + addDebugValuesToLoopVariable(Phi.ExitBlock, ExitVal, PN); PN->replaceAllUsesWith(ExitVal); PN->eraseFromParent(); } diff --git a/llvm/test/Transforms/LoopSimplify/pr51735-1.ll b/llvm/test/Transforms/LoopSimplify/pr51735-1.ll index 293553b000c13c..20cadcac47b60f 100644 --- a/llvm/test/Transforms/LoopSimplify/pr51735-1.ll +++ b/llvm/test/Transforms/LoopSimplify/pr51735-1.ll @@ -12,16 +12,18 @@ ; CHECK: if.then: ; CHECK: call void @llvm.dbg.value(metadata i32 777, metadata ![[DBG]], {{.*}} +; CHECK: call void @llvm.dbg.value(metadata i32 %Var.1, metadata ![[VAR:[0-9]+]], {{.*}} ; CHECK: br label %for.end, {{.*}} ; CHECK: if.else: -; CHECK: call void @llvm.dbg.value(metadata i32 777, metadata ![[DBG]], {{.*}} +; CHECK: call void @llvm.dbg.value(metadata i32 %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: