[DebugInfo][GlobalOpt] Preserve source locs for optimized loads #134828
Conversation
This file contains hidden or 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
Some optimizations in globalopt simplify uses of a global value to uses of a generated global bool value; in some cases where this happens, the newly-generated instructions would not have the original source location(s) of the instructions they replaced propagated to them; this patch properly preserves those source locations.
SLTozer
requested review from
jmorse,
adrian-prantl,
OCHyams,
dwblaikie and
CarlosAlbertoEnciso
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-debuginfo Author: Stephen Tozer (SLTozer) ChangesSome optimizations in globalopt simplify uses of a global value to uses of a generated global bool value; in some cases where this happens, the newly-generated instructions would not have the original source location(s) of the instructions they replaced propagated to them; this patch properly preserves those source locations. Found using #107279. Full diff: https://github.com/llvm/llvm-project/pull/134828.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
index 83cc1e5f04f3d..397435a4271a6 100644
--- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -988,6 +988,11 @@ OptimizeGlobalAddressOfAllocation(GlobalVariable *GV, CallInst *CI,
InitBool->getName() + ".val", false, Align(1),
LI->getOrdering(), LI->getSyncScopeID(),
LI->getIterator());
+ // FIXME: Should we use the DebugLoc of the load used by the predicate, or
+ // the predicate? The load seems most appropriate, but there's an argument
+ // that the new load does not represent the old load, but is simply a
+ // component of recomputing the predicate.
+ cast<LoadInst>(LV)->setDebugLoc(LI->getDebugLoc());
InitBoolUsed = true;
switch (ICI->getPredicate()) {
default: llvm_unreachable("Unknown ICmp Predicate!");
@@ -1000,6 +1005,7 @@ OptimizeGlobalAddressOfAllocation(GlobalVariable *GV, CallInst *CI,
case ICmpInst::ICMP_ULE:
case ICmpInst::ICMP_EQ:
LV = BinaryOperator::CreateNot(LV, "notinit", ICI->getIterator());
+ cast<BinaryOperator>(LV)->setDebugLoc(ICI->getDebugLoc());
break;
case ICmpInst::ICMP_NE:
case ICmpInst::ICMP_UGT:
@@ -1276,6 +1282,7 @@ static bool TryToShrinkGlobalToBoolean(GlobalVariable *GV, Constant *OtherVal) {
new LoadInst(NewGV->getValueType(), NewGV, LI->getName() + ".b",
false, Align(1), LI->getOrdering(),
LI->getSyncScopeID(), LI->getIterator());
+ cast<LoadInst>(StoreVal)->setDebugLoc(LI->getDebugLoc());
} else {
assert((isa<CastInst>(StoredVal) || isa<SelectInst>(StoredVal)) &&
"This is not a form that we understand!");
diff --git a/llvm/test/Transforms/GlobalOpt/X86/preserve-load-of-bool-dbgloc.ll b/llvm/test/Transforms/GlobalOpt/X86/preserve-load-of-bool-dbgloc.ll
new file mode 100644
index 0000000000000..4a97a67092906
--- /dev/null
+++ b/llvm/test/Transforms/GlobalOpt/X86/preserve-load-of-bool-dbgloc.ll
@@ -0,0 +1,94 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt < %s -p=globalopt -S | FileCheck %s
+
+;; Test that when we are able to simplify uses of global variables with loads of
+;; newly generated bool values, we transfer debuglocs over correctly.
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@global = internal global i32 0
+@global.1 = internal unnamed_addr global ptr null, align 8
+
+define void @ham() !dbg !7 {
+; CHECK-LABEL: define void @ham(
+; CHECK-SAME: ) local_unnamed_addr !dbg [[DBG4:![0-9]+]] {
+; CHECK-NEXT: [[BB:.*:]]
+; CHECK-NEXT: [[LOAD_B:%.*]] = load i1, ptr @global, align 1, !dbg [[DBG6:![0-9]+]]
+; CHECK-NEXT: [[LOAD_B1:%.*]] = load i1, ptr @global, align 1, !dbg [[DBG6]]
+; CHECK-NEXT: [[LOAD:%.*]] = zext i1 [[LOAD_B1]] to i32, !dbg [[DBG6]]
+; CHECK-NEXT: store i1 [[LOAD_B]], ptr @global, align 1
+; CHECK-NEXT: ret void
+;
+bb:
+ %load = load i32, ptr @global, align 4, !dbg !4
+ store i32 %load, ptr @global, align 4
+ ret void
+}
+
+define void @hoge() {
+; CHECK-LABEL: define void @hoge() local_unnamed_addr {
+; CHECK-NEXT: [[BB:.*:]]
+; CHECK-NEXT: store i1 true, ptr @global, align 1
+; CHECK-NEXT: ret void
+;
+bb:
+ store i32 1, ptr @global, align 4
+ ret void
+}
+
+define void @bar() {
+; CHECK-LABEL: define void @bar() local_unnamed_addr {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: store i1 true, ptr @global.1.init, align 1
+; CHECK-NEXT: ret void
+;
+entry:
+ %call = tail call noalias nonnull dereferenceable(48) ptr @_Znwm(i64 48)
+ store ptr %call, ptr @global.1, align 8
+ ret void
+}
+
+define void @pluto() !dbg !10 {
+; CHECK-LABEL: define void @pluto(
+; CHECK-SAME: ) local_unnamed_addr !dbg [[DBG8:![0-9]+]] {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: [[GLOBAL_1_INIT_VAL:%.*]] = load i1, ptr @global.1.init, align 1, !dbg [[DBG9:![0-9]+]]
+; CHECK-NEXT: [[NOTINIT:%.*]] = xor i1 [[GLOBAL_1_INIT_VAL]], true, !dbg [[DBG10:![0-9]+]]
+; CHECK-NEXT: unreachable
+;
+entry:
+ %0 = load ptr, ptr @global.1, align 8, !dbg !11
+ %.not = icmp eq ptr %0, null, !dbg !12
+ unreachable
+}
+
+declare ptr @_Znwm(i64)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang version 20.0.0git", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !2, globals: !2, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "preserve-load-dbgloc.c", directory: "/tmp")
+!2 = !{}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !DILocation(line: 10, column: 1, scope: !5)
+!5 = distinct !DILexicalBlock(scope: !7, file: !6, line: 1524, column: 3)
+!6 = !DIFile(filename: "preserve-load-dbgloc.c", directory: "/tmp")
+!7 = distinct !DISubprogram(name: "ham", scope: !6, file: !6, line: 10, type: !8, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
+!8 = distinct !DISubroutineType(types: !2)
+!10 = distinct !DISubprogram(name: "pluto", scope: !6, file: !6, line: 20, type: !8, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
+!11 = !DILocation(line: 20, column: 2, scope: !10)
+!12 = !DILocation(line: 21, column: 3, scope: !10)
+;.
+; CHECK: [[META0:![0-9]+]] = distinct !DICompileUnit(language: DW_LANG_C11, file: [[META1:![0-9]+]], producer: "{{.*}}clang version {{.*}}", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: [[META2:![0-9]+]], retainedTypes: [[META2]], globals: [[META2]], splitDebugInlining: false, nameTableKind: None)
+; CHECK: [[META1]] = !DIFile(filename: "preserve-load-dbgloc.c", directory: {{.*}})
+; CHECK: [[META2]] = !{}
+; CHECK: [[DBG4]] = distinct !DISubprogram(name: "ham", scope: [[META1]], file: [[META1]], line: 10, type: [[META5:![0-9]+]], flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: [[META0]], retainedNodes: [[META2]])
+; CHECK: [[META5]] = distinct !DISubroutineType(types: [[META2]])
+; CHECK: [[DBG6]] = !DILocation(line: 10, column: 1, scope: [[META7:![0-9]+]])
+; CHECK: [[META7]] = distinct !DILexicalBlock(scope: [[DBG4]], file: [[META1]], line: 1524, column: 3)
+; CHECK: [[DBG8]] = distinct !DISubprogram(name: "pluto", scope: [[META1]], file: [[META1]], line: 20, type: [[META5]], flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: [[META0]], retainedNodes: [[META2]])
+; CHECK: [[DBG9]] = !DILocation(line: 20, column: 2, scope: [[DBG8]])
+; CHECK: [[DBG10]] = !DILocation(line: 21, column: 3, scope: [[DBG8]])
+;.
|
rnk
reviewed
Apr 16, 2025
| @@ -988,6 +988,11 @@ OptimizeGlobalAddressOfAllocation(GlobalVariable *GV, CallInst *CI, | |||
| InitBool->getName() + ".val", false, Align(1), | |||
| LI->getOrdering(), LI->getSyncScopeID(), | |||
| LI->getIterator()); | |||
| // FIXME: Should we use the DebugLoc of the load used by the predicate, or | |||
There was a problem hiding this comment.
I see new StoreInst above which looks like it does similar straightforward replacement, should we carry over the sloc there?
There was a problem hiding this comment.
It certainly looks like it - I'll find a test case and add it (and any other nearby cases if I spot any).
OCHyams
approved these changes
Apr 23, 2025
| ret void | ||
| } | ||
|
|
||
| define void @hoge() { |
There was a problem hiding this comment.
What's this function testing?
There was a problem hiding this comment.
This function is necessary for GlobalOpt to make the decisions that trigger the bug.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…#134828) Some optimizations in globalopt simplify uses of a global value to uses of a generated global bool value; in some cases where this happens, the newly-generated instructions would not have the original source location(s) of the instructions they replaced propagated to them; this patch properly preserves those source locations. Found using llvm#107279.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…#134828) Some optimizations in globalopt simplify uses of a global value to uses of a generated global bool value; in some cases where this happens, the newly-generated instructions would not have the original source location(s) of the instructions they replaced propagated to them; this patch properly preserves those source locations. Found using llvm#107279.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…#134828) Some optimizations in globalopt simplify uses of a global value to uses of a generated global bool value; in some cases where this happens, the newly-generated instructions would not have the original source location(s) of the instructions they replaced propagated to them; this patch properly preserves those source locations. Found using llvm#107279.
Ankur-0429
pushed a commit
to Ankur-0429/llvm-project
that referenced
this pull request
May 9, 2025
…#134828) Some optimizations in globalopt simplify uses of a global value to uses of a generated global bool value; in some cases where this happens, the newly-generated instructions would not have the original source location(s) of the instructions they replaced propagated to them; this patch properly preserves those source locations. Found using llvm#107279.
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.
Some optimizations in globalopt simplify uses of a global value to uses of a generated global bool value; in some cases where this happens, the newly-generated instructions would not have the original source location(s) of the instructions they replaced propagated to them; this patch properly preserves those source locations.
Found using #107279.