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.
  • Loading branch information
@CarlosAlbertoEnciso
CarlosAlbertoEnciso committed Mar 13, 2024
1 parent a6260d4 commit a27cef0
Show file tree
Hide file tree
Showing 5 changed files with 159 additions and 95 deletions.
4 changes: 4 additions & 0 deletions llvm/include/llvm/Transforms/Utils/LoopUtils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,10 @@ 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 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
/// UnrolledLoop and \p RemainderLoop. \p UnrolledLoop receives weights that
Expand Down
1 change: 0 additions & 1 deletion llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
#include "llvm/IR/ConstantRange.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/DataLayout.h"
#include "llvm/IR/DebugInfo.h"
#include "llvm/IR/DerivedTypes.h"
#include "llvm/IR/Dominators.h"
#include "llvm/IR/Function.h"
Expand Down
81 changes: 56 additions & 25 deletions llvm/lib/Transforms/Utils/LoopUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1413,6 +1413,36 @@ static bool checkIsIndPhi(PHINode *Phi, Loop *L, ScalarEvolution *SE,
return InductionDescriptor::isInductionPHI(Phi, L, SE, ID);
}

void llvm::addDebugValuesToLoopVariable(Loop *L, 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());
}
}
}

void llvm::addDebugValuesToLoopVariable(Loop *L, ScalarEvolution *SE,
PHINode *PN) {
if (!PN)
return;
const SCEV *PNSCEV = SE->getSCEVAtScope(PN, L->getParentLoop());
if (auto *Const = dyn_cast<SCEVConstant>(PNSCEV)) {
Value *FinalIVValue = Const->getValue();
addDebugValuesToLoopVariable(L, FinalIVValue, PN);
}
}

int llvm::rewriteLoopExitValues(Loop *L, LoopInfo *LI, TargetLibraryInfo *TLI,
ScalarEvolution *SE,
const TargetTransformInfo *TTI,
Expand Down Expand Up @@ -1554,6 +1584,21 @@ 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 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());
}
}
}
}
}
}
Expand Down Expand Up @@ -1612,39 +1657,25 @@ 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);
PN->replaceAllUsesWith(ExitVal);
PN->eraseFromParent();
}
}

// If there are no PHIs to be rewritten then there are no loop live-out
// values, try to rewrite variables corresponding to the induction variable
// with their constant exit-values if we computed any. Otherwise debug-info
// will completely forget that this loop happened.
if (RewritePhiSet.empty()) {
// The loop exit value has been updated; insert the debug location
// for the given the induction variable with its final value.
addDebugValuesToLoopVariable(L, SE, L->getInductionVariable(*SE));
}

// The insertion point instruction may have been deleted; clear it out
// so that the rewriter doesn't trip over it later.
Rewriter.clearInsertPoint();

// The loop exit values have been updated; insert the debug location
// for the induction variable with its final value.
if (PHINode *IndVar = L->getInductionVariable(*SE)) {
const SCEV *IndVarSCEV = SE->getSCEVAtScope(IndVar, L->getParentLoop());
if (isa<SCEVConstant>(IndVarSCEV)) {
Value *FinalIVValue = cast<SCEVConstant>(IndVarSCEV)->getValue();
SmallVector<DbgVariableIntrinsic *> DbgUsers;
SmallVector<DbgVariableIntrinsic *> DbgUsersCloned;
findDbgUsers(DbgUsers, IndVar);
for (auto &DebugUser : DbgUsers) {
auto *Cloned = cast<DbgVariableIntrinsic>(DebugUser->clone());
Cloned->replaceVariableLocationOp(static_cast<unsigned>(0),
FinalIVValue);
DbgUsersCloned.push_back(Cloned);
}

SmallVector<BasicBlock *> ExitBlocks;
L->getExitBlocks(ExitBlocks);
for (BasicBlock *Exit : ExitBlocks)
for (auto &DebugUser : DbgUsersCloned)
DebugUser->insertBefore(Exit->getFirstNonPHI());
}
}

return NumReplaced;
}

Expand Down
155 changes: 93 additions & 62 deletions llvm/test/Transforms/LoopSimplify/pr51735-1.ll
View file
Original file line number Diff line number Diff line change
@@ -1,54 +1,78 @@
; RUN: opt -passes=indvars -S -o - < %s | FileCheck %s

; 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'.

; 1 __attribute__((optnone)) int nop(int Param) {
; 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 ;
; 11 nop(Index);
; 12 }
; 13
; 14 int main () {
; 15 bar();
; 16 }

; CHECK: for.cond: {{.*}}
; CHECK: call void @llvm.dbg.value(metadata i32 %Index.{{[0-9]+}}, metadata ![[DBG:[0-9]+]], {{.*}}
; CHECK: call void @llvm.dbg.value(metadata i32 %inc, metadata ![[DBG:[0-9]+]], {{.*}}
; CHECK: for.end: {{.*}}
; CHECK: call void @llvm.dbg.value(metadata i32 777, metadata ![[DBG:[0-9]+]], {{.*}}
; Make sure that when we delete the loop, that the variable Index has
; the 777 value.

; CHECK: for.cond:
; CHECK: call void @llvm.dbg.value(metadata i32 %Index.0, metadata ![[DBG:[0-9]+]], {{.*}}

; CHECK: for.extra:
; CHECK: %call.0 = call noundef i32 @"?nop@@YAHH@Z"(i32 noundef %Index.0), {{.*}}
; CHECK: br i1 %cmp.0, label %for.cond, label %if.else, {{.*}}

; CHECK: if.then:
; CHECK: call void @llvm.dbg.value(metadata i32 777, metadata ![[DBG]], {{.*}}
; CHECK: br label %for.end, {{.*}}

; CHECK: if.else:
; CHECK: call void @llvm.dbg.value(metadata i32 777, metadata ![[DBG]], {{.*}}
; 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"{{.*}})

define dso_local void @_Z3barv() local_unnamed_addr #2 !dbg !18 {
define dso_local noundef i32 @"?nop@@YAHH@Z"(i32 noundef %Param) !dbg !11 {
entry:
call void @llvm.dbg.value(metadata i32 777, metadata !21, metadata !DIExpression()), !dbg !22
call void @llvm.dbg.value(metadata i32 27, metadata !23, metadata !DIExpression()), !dbg !22
call void @llvm.dbg.value(metadata i32 1, metadata !24, metadata !DIExpression()), !dbg !22
br label %for.cond, !dbg !25
%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.cond ], !dbg !22
call void @llvm.dbg.value(metadata i32 %Index.0, metadata !23, metadata !DIExpression()), !dbg !22
%cmp = icmp ult i32 %Index.0, 777, !dbg !26
%inc = add nuw nsw i32 %Index.0, 1, !dbg !29
call void @llvm.dbg.value(metadata i32 %inc, metadata !23, metadata !DIExpression()), !dbg !22
br i1 %cmp, label %for.cond, label %for.end, !dbg !30, !llvm.loop !31

for.end: ; preds = %for.cond
%Index.0.lcssa = phi i32 [ %Index.0, %for.cond ], !dbg !22
ret void, !dbg !35
%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}
Expand All @@ -64,24 +88,31 @@ declare void @llvm.dbg.value(metadata, metadata, metadata)
!7 = !{i32 7, !"uwtable", i32 2}
!8 = !{i32 7, !"frame-pointer", i32 2}
!9 = !{!"clang version 18.0.0"}
!12 = !{!13, !13}
!13 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
!14 = !{}
!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}
!21 = !DILocalVariable(name: "End", scope: !18, file: !1, line: 6, type: !13)
!22 = !DILocation(line: 0, scope: !18)
!23 = !DILocalVariable(name: "Index", scope: !18, file: !1, line: 7, type: !13)
!24 = !DILocalVariable(name: "Var", scope: !18, file: !1, line: 8, type: !13)
!25 = !DILocation(line: 9, column: 3, scope: !18)
!26 = !DILocation(line: 9, column: 16, scope: !27)
!27 = distinct !DILexicalBlock(scope: !28, file: !1, line: 9, column: 3)
!28 = distinct !DILexicalBlock(scope: !18, file: !1, line: 9, column: 3)
!29 = !DILocation(line: 9, column: 23, scope: !27)
!30 = !DILocation(line: 9, column: 3, scope: !28)
!31 = distinct !{!31, !30, !32, !33}
!32 = !DILocation(line: 10, column: 5, scope: !28)
!33 = !{!"llvm.loop.mustprogress"}
!34 = !DILocation(line: 11, column: 3, scope: !18)
!35 = !DILocation(line: 12, column: 1, scope: !18)
!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)
13 changes: 6 additions & 7 deletions llvm/test/Transforms/LoopSimplify/pr51735.ll
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
; RUN: opt -passes=indvars -S -o - < %s | FileCheck %s

; 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'.
; Make sure that when we delete the loop in the code below, that the variable
; Index has the 777 value.

; 1 __attribute__((optnone)) int nop() {
; 2 return 0;
Expand All @@ -21,12 +20,12 @@
; 15 bar();
; 16 }

; CHECK: for.cond: {{.*}}
; CHECK: call void @llvm.dbg.value(metadata i32 poison, metadata ![[DBG:[0-9]+]], {{.*}}
; CHECK: for.cond:
; CHECK: call void @llvm.dbg.value(metadata i32 poison, metadata ![[DBG:[0-9]+]], {{.*}}
; CHECK: call void @llvm.dbg.value(metadata i32 poison, metadata ![[DBG]], {{.*}}
; CHECK: br i1 false, label %for.cond, label %for.end, {{.*}}
; CHECK: for.end: {{.*}}
; CHECK: call void @llvm.dbg.value(metadata i32 777, metadata ![[DBG:[0-9]+]], {{.*}}
; CHECK: for.end:
; CHECK: call void @llvm.dbg.value(metadata i32 777, metadata ![[DBG]], {{.*}}
; CHECK-DAG: ![[DBG]] = !DILocalVariable(name: "Index"{{.*}})

define dso_local void @_Z3barv() local_unnamed_addr #1 !dbg !15 {
Expand Down

0 comments on commit a27cef0

Please sign in to comment.