Skip to content

Conversation

SLTozer
Copy link
Contributor

@SLTozer SLTozer commented Sep 9, 2025

In simplifyPartiallyRedundantLoad we may replace a load with a PHI of available values in predecessor blocks. As part of this process, we may need to cast those values, which we do by inserting a new cast at the end of the predecessor. These cast instructions should take their debug location from the load instruction, just as the PHI does; we make an exception if the predecessor does not unconditionally branch to the load's block, as in that case we are not guaranteed to reach the load and must therefore drop its debug location.

Found using #107279.

@llvmbot
Copy link
Member

llvmbot commented Sep 9, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Stephen Tozer (SLTozer)

Changes

In simplifyPartiallyRedundantLoad we may replace a load with a PHI of available values in predecessor blocks. As part of this process, we may need to cast those values, which we do by inserting a new cast at the end of the predecessor. These cast instructions should take their debug location from the load instruction, just as the PHI does; we make an exception if the predecessor does not unconditionally branch to the load's block, as in that case we are not guaranteed to reach the load and must therefore drop its debug location.

Found using #107279.


Full diff: https://github.com/llvm/llvm-project/pull/157683.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/JumpThreading.cpp (+10-1)
  • (added) llvm/test/Transforms/JumpThreading/simplify-partially-redundant-load-debugloc.ll (+88)
diff --git a/llvm/lib/Transforms/Scalar/JumpThreading.cpp b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
index c2a737d8f9a4a..c7d71eb5633ec 100644
--- a/llvm/lib/Transforms/Scalar/JumpThreading.cpp
+++ b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
@@ -1437,9 +1437,18 @@ bool JumpThreadingPass::simplifyPartiallyRedundantLoad(LoadInst *LoadI) {
     // AvailablePreds vector as we go so that all of the PHI entries for this
     // predecessor use the same bitcast.
     Value *&PredV = I->second;
-    if (PredV->getType() != LoadI->getType())
+    if (PredV->getType() != LoadI->getType()) {
       PredV = CastInst::CreateBitOrPointerCast(
           PredV, LoadI->getType(), "", P->getTerminator()->getIterator());
+      // The new cast is producing the value used to replace the load
+      // instruction, so uses the load's debug location. If P does not always
+      // branch to the load BB however then the debug location must be dropped,
+      // as it is hoisted past a conditional branch.
+      DebugLoc DL = P->getTerminator()->getNumSuccessors() == 1
+                        ? LoadI->getDebugLoc()
+                        : DebugLoc::getDropped();
+      cast<CastInst>(PredV)->setDebugLoc(DL);
+    }
 
     PN->addIncoming(PredV, I->first);
   }
diff --git a/llvm/test/Transforms/JumpThreading/simplify-partially-redundant-load-debugloc.ll b/llvm/test/Transforms/JumpThreading/simplify-partially-redundant-load-debugloc.ll
new file mode 100644
index 0000000000000..80f7a53d5e2a3
--- /dev/null
+++ b/llvm/test/Transforms/JumpThreading/simplify-partially-redundant-load-debugloc.ll
@@ -0,0 +1,88 @@
+; RUN: opt -S -passes=jump-threading < %s | FileCheck %s
+
+;; Check that when we simplify the load of %p by replacing it with 0 when coming
+;; from %left, the inttoptr cast instruction is assigned the DILocation from the
+;; load instruction.
+
+; CHECK-LABEL: define void @foo(
+
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[P:%.*]] = alloca i32, align 8
+; CHECK-NEXT:    br i1 {{.*}}, label %[[LEFT:.*]], label %[[RIGHT:.*]]
+
+;; Cast in "left" should have the load's debug location.
+; CHECK:       [[LEFT]]:
+; CHECK-NEXT:    call void @llvm.memset.p0.i64(ptr [[P]], i8 0, i64 8, i1 false)
+; CHECK-NEXT:    [[TMP0:%.*]] = inttoptr i64 0 to ptr, !dbg [[LOAD_LOC:![0-9]+]]
+; CHECK-NEXT:    br label %[[END:.*]]
+
+; CHECK:       [[RIGHT]]:
+; CHECK-NEXT:    br i1 {{.*}}, label %[[RIGHT_LEFT:.*]], label %[[ENDTHREAD_PRE_SPLIT:.*]]
+
+;; Cast in "right.left" should not have a debug location, as we are not
+;; guaranteed to reach the load's original position.
+; CHECK:       [[RIGHT_LEFT]]:
+; CHECK-NEXT:    call void @llvm.memset.p0.i64(ptr [[P]], i8 0, i64 8, i1 false)
+; CHECK-NEXT:    [[TMP1:%.*]] = inttoptr i64 0 to ptr
+; CHECK-NEXT:    br i1 {{.*}}, label %[[END]], label %[[EXIT:.*]]
+
+;; Load in "right.right" should have the load's debug location.
+; CHECK:       [[ENDTHREAD_PRE_SPLIT]]:
+; CHECK-NEXT:    [[DOTPR:%.*]] = load ptr, ptr [[P]], align 8, !dbg [[LOAD_LOC]]
+; CHECK-NEXT:    br label %[[END]], !dbg [[LOAD_LOC]]
+
+;; Finally, the PHI node should also have the load's debug location.
+; CHECK:       [[END]]:
+; CHECK-NEXT:    [[TMP2:%.*]] = phi ptr [ [[DOTPR]], %[[ENDTHREAD_PRE_SPLIT]] ], [ [[TMP1]], %[[RIGHT_LEFT]] ], [ [[TMP0]], %[[LEFT]] ], !dbg [[LOAD_LOC]]
+
+; CHECK: [[LOAD_LOC]] = !DILocation(line: 1, column: 1,
+
+define void @foo(i1 %b, i1 %c, i1 %d) !dbg !5 {
+entry:
+  %p = alloca i32, align 8
+  br i1 %b, label %left, label %right
+
+left:
+  call void @llvm.memset.p0.i64(ptr %p, i8 0, i64 8, i1 false)
+  br label %end
+
+right:
+  br i1 %c, label %right.left, label %right.right
+
+right.left:
+  call void @llvm.memset.p0.i64(ptr %p, i8 0, i64 8, i1 false)
+  br i1 %d, label %end, label %exit
+
+right.right:
+  br label %end
+
+end:
+  %0 = load ptr, ptr %p, align 8, !dbg !8
+  %isnull = icmp eq ptr %0, null
+  br i1 %isnull, label %exit, label %notnull
+
+notnull:
+  br label %exit
+
+exit:
+  ret void
+}
+
+; Function Attrs: nocallback nofree nounwind willreturn memory(argmem: write)
+declare void @llvm.memset.p0.i64(ptr writeonly captures(none), i8, i64, i1 immarg) #0
+
+attributes #0 = { nocallback nofree nounwind willreturn memory(argmem: write) }
+
+!llvm.dbg.cu = !{!0}
+!llvm.debugify = !{!2, !3}
+!llvm.module.flags = !{!4}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
+!1 = !DIFile(filename: "simplify-partially-redundant-load-debugloc.ll", directory: "/")
+!2 = !{i32 13}
+!3 = !{i32 0}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: !0, file: !1, line: 1, type: !6, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+!6 = !DISubroutineType(types: !7)
+!7 = !{}
+!8 = !DILocation(line: 1, column: 1, scope: !5)

In simplifyPartiallyRedundantLoad we may replace a load with a PHI of
available values in predecessor blocks. As part of this process, we may
need to cast those values, which we do by inserting a new cast at the
end of the predecessor. These cast instructions should take their debug
location from the load instruction, just as the PHI does; we make an
exception if the predecessor does not unconditionally branch to the load's
block, as in that case we are not guaranteed to reach the load and must
therefore drop its debug location.
@SLTozer SLTozer force-pushed the jump-threading-redundant-load branch from 3d9f390 to a5b9ff5 Compare September 10, 2025 12:17
@SLTozer SLTozer requested a review from dwblaikie September 10, 2025 12:18
Copy link
Contributor

@OCHyams OCHyams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me, LGTM! The comments in the test are appreciated

@SLTozer SLTozer merged commit 0a69cd4 into llvm:main Sep 10, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants