Skip to content

Commit

Permalink
[indvars] Missing variables at Og:
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
CarlosAlbertoEnciso committed Jan 16, 2024
1 parent 7b068fe commit 86eeaa0
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 27 deletions.
5 changes: 4 additions & 1 deletion llvm/include/llvm/Transforms/Utils/LoopUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -469,8 +469,11 @@ int rewriteLoopExitValues(Loop *L, LoopInfo *LI, TargetLibraryInfo *TLI,
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);
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
Expand Down
52 changes: 27 additions & 25 deletions llvm/lib/Transforms/Utils/LoopUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1342,22 +1343,29 @@ 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<DbgVariableIntrinsic *> DbgUsers;
findDbgUsers(DbgUsers, IndVar);
for (auto *DebugUser : DbgUsers) {
auto *Cloned = cast<DbgVariableIntrinsic>(DebugUser->clone());
Cloned->replaceVariableLocationOp(0u, PN);
Cloned->insertBefore(Successor->getFirstNonPHI());
}
}

void llvm::addDebugValuesToLoopVariable(BasicBlock *Successor, Value *ExitValue,
PHINode *PN) {
SmallVector<BasicBlock *> ExitBlocks;
L->getExitBlocks(ExitBlocks);
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;
for (BasicBlock *Exit : ExitBlocks) {
auto *Cloned = cast<DbgVariableIntrinsic>(DebugUser->clone());
Cloned->replaceVariableLocationOp(0u, ExitValue);
Cloned->insertBefore(Exit->getFirstNonPHI());
}
auto *Cloned = cast<DbgVariableIntrinsic>(DebugUser->clone());
Cloned->replaceVariableLocationOp(0u, ExitValue);
Cloned->insertBefore(Successor->getFirstNonPHI());
}
}

Expand All @@ -1368,7 +1376,10 @@ void llvm::addDebugValuesToLoopVariable(Loop *L, ScalarEvolution *SE,
const SCEV *PNSCEV = SE->getSCEVAtScope(PN, L->getParentLoop());
if (auto *Const = dyn_cast<SCEVConstant>(PNSCEV)) {
Value *FinalIVValue = Const->getValue();
addDebugValuesToLoopVariable(L, FinalIVValue, PN);
SmallVector<BasicBlock *> ExitBlocks;
L->getExitBlocks(ExitBlocks);
for (BasicBlock *Exit : ExitBlocks)
addDebugValuesToLoopVariable(Exit, FinalIVValue, PN);
}
}

Expand Down Expand Up @@ -1512,22 +1523,13 @@ int llvm::rewriteLoopExitValues(Loop *L, LoopInfo *LI, TargetLibraryInfo *TLI,
Instruction *InsertPt =
(isa<PHINode>(Inst) || isa<LandingPadInst>(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<DbgVariableIntrinsic *> DbgUsers;
findDbgUsers(DbgUsers, V);
for (auto *DebugUser : DbgUsers) {
auto *Cloned = cast<DbgVariableIntrinsic>(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);
}
}
}
Expand Down Expand Up @@ -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();
}
Expand Down
4 changes: 3 additions & 1 deletion llvm/test/Transforms/LoopSimplify/pr51735-1.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit 86eeaa0

Please sign in to comment.