[IfConversion] Preserve debug info when flipping a branch

[ellishg]

Grab the debug info location of a branch condition before we remove it and insert a flipped condition.

[arsenm]

Testcase?

[llvmbot]

@llvm/pr-subscribers-debuginfo

Author: Ellis Hoag (ellishg)

Changes

Grab the debug info location of a branch condition before we remove it and insert a flipped condition.

---

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

1 Files Affected:

• (modified) llvm/lib/CodeGen/IfConversion.cpp (+7-8)

diff --git a/llvm/lib/CodeGen/IfConversion.cpp b/llvm/lib/CodeGen/IfConversion.cpp
index ba5605ad5cb4a4..bb9ca7edf7844c 100644
--- a/llvm/lib/CodeGen/IfConversion.cpp
+++ b/llvm/lib/CodeGen/IfConversion.cpp
@@ -617,14 +617,13 @@ static MachineBasicBlock *findFalseBlock(MachineBasicBlock *BB,
 /// Reverse the condition of the end of the block branch. Swap block's 'true'
 /// and 'false' successors.
 bool IfConverter::reverseBranchCondition(BBInfo &BBI) const {
-  DebugLoc dl;  // FIXME: this is nowhere
-  if (!TII->reverseBranchCondition(BBI.BrCond)) {
-    TII->removeBranch(*BBI.BB);
-    TII->insertBranch(*BBI.BB, BBI.FalseBB, BBI.TrueBB, BBI.BrCond, dl);
-    std::swap(BBI.TrueBB, BBI.FalseBB);
-    return true;
-  }
-  return false;
+  if (TII->reverseBranchCondition(BBI.BrCond))
+    return false;
+  auto Dl = BBI.BB->findBranchDebugLoc();
+  TII->removeBranch(*BBI.BB);
+  TII->insertBranch(*BBI.BB, BBI.FalseBB, BBI.TrueBB, BBI.BrCond, Dl);
+  std::swap(BBI.TrueBB, BBI.FalseBB);
+  return true;
 }
 
 /// Returns the next block in the function blocks ordering. If it is the end,

[jmorse]

Change looks good, with a test of course

[ellishg]

I can't find a test case where the debug location was incorrect or missing before this change, but it is fairly easy to find a test that executes this code (some existing tests already do). Is this acceptable to land without a test?

[arsenm]

I can't find a test case where the debug location was incorrect or missing before this change

Then what is the point?

[ellishg]

I saw this comment in a few places and I figured it would be an easy improvement.

DebugLoc dl; // FIXME: this is nowhere

[arsenm]

I saw this comment in a few places and I figured it would be an easy improvement.

DebugLoc dl; // FIXME: this is nowhere

Should be easily MIR testable?