Skip to content

[KeyInstr] Don't propagate source atoms to new uncond br in splitBasicBlock #139070

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 8, 2025

Conversation

OCHyams
Copy link
Contributor

@OCHyams OCHyams commented May 8, 2025

splitBasicBlock inserts an unconditional branch in the "before" block to the "after" block. It copies the DebugLoc from the split point. Prevent it copying the source location atom.

Add unittest.

RFC: https://discourse.llvm.org/t/rfc-improving-is-stmt-placement-for-better-interactive-debugging/82668

…cBlock

splitBasicBlock inserts an unconditional branch in the "before" block to the
"after" block. It copies the DebugLoc from the split point. Prevent it copying
the source location atom.

Add unittest.

RFC: https://discourse.llvm.org/t/rfc-improving-is-stmt-placement-for-better-interactive-debugging/82668
@OCHyams OCHyams requested a review from jmorse May 8, 2025 12:34
@llvmbot llvmbot added the llvm:ir label May 8, 2025
@llvmbot
Copy link
Member

llvmbot commented May 8, 2025

@llvm/pr-subscribers-llvm-ir

Author: Orlando Cazalet-Hyams (OCHyams)

Changes

splitBasicBlock inserts an unconditional branch in the "before" block to the "after" block. It copies the DebugLoc from the split point. Prevent it copying the source location atom.

Add unittest.

RFC: https://discourse.llvm.org/t/rfc-improving-is-stmt-placement-for-better-interactive-debugging/82668


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

2 Files Affected:

  • (modified) llvm/lib/IR/BasicBlock.cpp (+2-2)
  • (modified) llvm/unittests/IR/BasicBlockDbgInfoTest.cpp (+69)
diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index b4c16447bc686..aea07f4731e7d 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -612,7 +612,7 @@ BasicBlock *BasicBlock::splitBasicBlock(iterator I, const Twine &BBName,
                                        this->getNextNode());
 
   // Save DebugLoc of split point before invalidating iterator.
-  DebugLoc Loc = I->getStableDebugLoc();
+  DebugLoc Loc = I->getStableDebugLoc()->getWithoutAtom();
   // Move all of the specified instructions from the original basic block into
   // the new basic block.
   New->splice(New->end(), this, I, end());
@@ -641,7 +641,7 @@ BasicBlock *BasicBlock::splitBasicBlockBefore(iterator I, const Twine &BBName) {
 
   BasicBlock *New = BasicBlock::Create(getContext(), BBName, getParent(), this);
   // Save DebugLoc of split point before invalidating iterator.
-  DebugLoc Loc = I->getDebugLoc();
+  DebugLoc Loc = I->getDebugLoc()->getWithoutAtom();
   // Move all of the specified instructions from the original basic block into
   // the new basic block.
   New->splice(New->end(), this, begin(), I);
diff --git a/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp b/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
index 5607c633b7a88..cb1a56a26f063 100644
--- a/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
+++ b/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
@@ -149,6 +149,75 @@ TEST(BasicBlockDbgInfoTest, SplitBasicBlockBefore) {
   ASSERT_TRUE(I2->hasDbgRecords());
 }
 
+TEST(BasicBlockDbgInfoTest, DropSourceAtomOnSplit) {
+  LLVMContext C;
+  std::unique_ptr<Module> M = parseIR(C, R"---(
+    define dso_local void @func() !dbg !10 {
+      %1 = alloca i32, align 4
+      ret void, !dbg !DILocation(line: 3, column: 2, scope: !10, atomGroup: 1, atomRank: 1)
+    }
+
+    !llvm.dbg.cu = !{!0}
+    !llvm.module.flags = !{!2, !3}
+
+    !0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "dummy", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+    !1 = !DIFile(filename: "dummy", directory: "dummy")
+    !2 = !{i32 7, !"Dwarf Version", i32 5}
+    !3 = !{i32 2, !"Debug Info Version", i32 3}
+    !10 = distinct !DISubprogram(name: "func", scope: !1, file: !1, line: 1, type: !11, scopeLine: 1, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !13)
+    !11 = !DISubroutineType(types: !12)
+    !12 = !{null}
+    !13 = !{}
+    !14 = !DILocalVariable(name: "a", scope: !10, file: !1, line: 2, type: !15)
+    !15 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  )---");
+  ASSERT_TRUE(M);
+
+  Function *F = M->getFunction("func");
+
+  // Test splitBasicBlockBefore.
+  {
+    BasicBlock &BB = F->back();
+    // Split at `ret void`.
+    BasicBlock *Before =
+        BB.splitBasicBlockBefore(std::prev(BB.end(), 1), "before");
+    const DebugLoc &BrToAfterDL = Before->getTerminator()->getDebugLoc();
+    ASSERT_TRUE(BrToAfterDL);
+    EXPECT_EQ(BrToAfterDL->getAtomGroup(), 0u);
+
+    BasicBlock *After = Before->getSingleSuccessor();
+    ASSERT_TRUE(After);
+    const DebugLoc &OrigTerminatorDL = After->getTerminator()->getDebugLoc();
+    ASSERT_TRUE(OrigTerminatorDL);
+#ifdef EXPERIMENTAL_KEY_INSTRUCTIONS
+    EXPECT_EQ(OrigTerminatorDL->getAtomGroup(), 1u);
+#else
+    EXPECT_EQ(OrigTerminatorDL->getAtomGroup(), 0u);
+#endif
+  }
+
+  // Test splitBasicBlock.
+  {
+    BasicBlock &BB = F->back();
+    // Split at `ret void`.
+    BasicBlock *After = BB.splitBasicBlock(std::prev(BB.end(), 1), "before");
+
+    const DebugLoc &OrigTerminatorDL = After->getTerminator()->getDebugLoc();
+    ASSERT_TRUE(OrigTerminatorDL);
+#ifdef EXPERIMENTAL_KEY_INSTRUCTIONS
+    EXPECT_EQ(OrigTerminatorDL->getAtomGroup(), 1u);
+#else
+    EXPECT_EQ(OrigTerminatorDL->getAtomGroup(), 0u);
+#endif
+
+    BasicBlock *Before = After->getSinglePredecessor();
+    ASSERT_TRUE(Before);
+    const DebugLoc &BrToAfterDL = Before->getTerminator()->getDebugLoc();
+    ASSERT_TRUE(BrToAfterDL);
+    EXPECT_EQ(BrToAfterDL->getAtomGroup(), 0u);
+  }
+}
+
 TEST(BasicBlockDbgInfoTest, MarkerOperations) {
   LLVMContext C;
   std::unique_ptr<Module> M = parseIR(C, R"(

@OCHyams OCHyams merged commit d2fe889 into llvm:main May 8, 2025
9 of 12 checks passed
OCHyams added a commit that referenced this pull request May 8, 2025
OCHyams added a commit to OCHyams/llvm-project that referenced this pull request May 9, 2025
…cBlock (llvm#139070)

splitBasicBlock inserts an unconditional branch in the "before" block to
the "after" block. It copies the DebugLoc from the split point. Prevent
it copying the source location atom.

Add unittest.

RFC:
https://discourse.llvm.org/t/rfc-improving-is-stmt-placement-for-better-interactive-debugging/82668
OCHyams added a commit to OCHyams/llvm-project that referenced this pull request May 9, 2025
OCHyams added a commit to OCHyams/llvm-project that referenced this pull request May 9, 2025
…cBlock (llvm#139070)

splitBasicBlock inserts an unconditional branch in the "before" block to
the "after" block. It copies the DebugLoc from the split point. Prevent
it copying the source location atom.

Add unittest.

RFC:
https://discourse.llvm.org/t/rfc-improving-is-stmt-placement-for-better-interactive-debugging/82668
OCHyams added a commit to OCHyams/llvm-project that referenced this pull request May 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants