[DebugInfo][Reassociate] Propagate source loc when negating mul factor

[SLTozer]

As part of RemoveFactorFromExpression, we attempt to remove a factor from a mul/fmul expression; this may involve generating new instructions, e.g. to negate the result if the factor was negative in the original expression. When this happens, the new instructions should have a DebugLoc set from the instruction that the factored expression is being used to compute.

Found using #107279.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-debuginfo

Author: Stephen Tozer (SLTozer)

Changes

As part of RemoveFactorFromExpression, we attempt to remove a factor from a mul/fmul expression; this may involve generating new instructions, e.g. to negate the result if the factor was negative in the original expression. When this happens, the new instructions should have a DebugLoc set from the instruction that the factored expression is being used to compute.

Found using #107279.

---

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

3 Files Affected:

• (modified) llvm/include/llvm/Transforms/Scalar/Reassociate.h (+1-1)
• (modified) llvm/lib/Transforms/Scalar/Reassociate.cpp (+9-3)
• (added) llvm/test/Transforms/Reassociate/debugloc-factoring-neg.ll (+40)

diff --git a/llvm/include/llvm/Transforms/Scalar/Reassociate.h b/llvm/include/llvm/Transforms/Scalar/Reassociate.h
index 3b2d2b83ced62..6d56961a71019 100644
--- a/llvm/include/llvm/Transforms/Scalar/Reassociate.h
+++ b/llvm/include/llvm/Transforms/Scalar/Reassociate.h
@@ -133,7 +133,7 @@ class ReassociatePass : public PassInfoMixin<ReassociatePass> {
                                  SmallVectorImpl<reassociate::Factor> &Factors);
   Value *OptimizeMul(BinaryOperator *I,
                      SmallVectorImpl<reassociate::ValueEntry> &Ops);
-  Value *RemoveFactorFromExpression(Value *V, Value *Factor);
+  Value *RemoveFactorFromExpression(Value *V, Value *Factor, DebugLoc DL);
   void EraseInst(Instruction *I);
   void RecursivelyEraseDeadInsts(Instruction *I, OrderedSet &Insts);
   void OptimizeInst(Instruction *I);
diff --git a/llvm/lib/Transforms/Scalar/Reassociate.cpp b/llvm/lib/Transforms/Scalar/Reassociate.cpp
index f9aef064641d8..99dcd3182038d 100644
--- a/llvm/lib/Transforms/Scalar/Reassociate.cpp
+++ b/llvm/lib/Transforms/Scalar/Reassociate.cpp
@@ -1098,7 +1098,10 @@ static Value *EmitAddTreeOfValues(BasicBlock::iterator It,
 /// If V is an expression tree that is a multiplication sequence,
 /// and if this sequence contains a multiply by Factor,
 /// remove Factor from the tree and return the new tree.
-Value *ReassociatePass::RemoveFactorFromExpression(Value *V, Value *Factor) {
+/// If new instructions are inserted to generate this tree, DL should be used
+/// as the DebugLoc for these instructions.
+Value *ReassociatePass::RemoveFactorFromExpression(Value *V, Value *Factor,
+                                                   DebugLoc DL) {
   BinaryOperator *BO = isReassociableOp(V, Instruction::Mul, Instruction::FMul);
   if (!BO)
     return nullptr;
@@ -1162,8 +1165,10 @@ Value *ReassociatePass::RemoveFactorFromExpression(Value *V, Value *Factor) {
     V = BO;
   }
 
-  if (NeedsNegate)
+  if (NeedsNegate) {
     V = CreateNeg(V, "neg", InsertPt, BO);
+    cast<Instruction>(V)->setDebugLoc(DL);
+  }
 
   return V;
 }
@@ -1664,7 +1669,8 @@ Value *ReassociatePass::OptimizeAdd(Instruction *I,
       if (!BOp)
         continue;
 
-      if (Value *V = RemoveFactorFromExpression(Ops[i].Op, MaxOccVal)) {
+      if (Value *V = RemoveFactorFromExpression(Ops[i].Op, MaxOccVal,
+                                                I->getDebugLoc())) {
         // The factorized operand may occur several times.  Convert them all in
         // one fell swoop.
         for (unsigned j = Ops.size(); j != i;) {
diff --git a/llvm/test/Transforms/Reassociate/debugloc-factoring-neg.ll b/llvm/test/Transforms/Reassociate/debugloc-factoring-neg.ll
new file mode 100644
index 0000000000000..cb1b79ed8d2ce
--- /dev/null
+++ b/llvm/test/Transforms/Reassociate/debugloc-factoring-neg.ll
@@ -0,0 +1,40 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -p=reassociate < %s | FileCheck %s
+
+define i32 @foo(i64 %0, i64 %1) {
+; CHECK-LABEL: define i32 @foo(
+; CHECK-SAME: i64 [[TMP0:%.*]], i64 [[TMP1:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[NEG:%.*]] = sub i64 0, [[TMP1]], !dbg [[DBG3:![0-9]+]]
+; CHECK-NEXT:    [[REASS_ADD:%.*]] = add i64 [[NEG]], [[TMP0]], !dbg [[DBG3]]
+; CHECK-NEXT:    [[REASS_MUL:%.*]] = mul i64 [[REASS_ADD]], 1000, !dbg [[DBG3]]
+; CHECK-NEXT:    store i64 [[REASS_MUL]], ptr null, align 8
+; CHECK-NEXT:    ret i32 0
+;
+entry:
+  %mul1026 = mul i64 %0, 1000
+  %add1028 = or i64 %mul1026, 0
+  %mul1029 = mul i64 %1, 1000
+  %sub1032 = sub i64 %add1028, %mul1029, !dbg !4
+  store i64 %sub1032, ptr null, align 8
+  ret i32 0
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang version 20.0.0git")
+!1 = !DIFile(filename: "test.c", directory: "/tmp")
+!2 = !{}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !DILocation(line: 10, column: 53, scope: !5)
+!5 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 550, type: !7, scopeLine: 557, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
+!7 = distinct !DISubroutineType(types: !2)
+;.
+; CHECK: [[META0:![0-9]+]] = distinct !DICompileUnit(language: DW_LANG_C11, file: [[META1:![0-9]+]], producer: "{{.*}}clang version {{.*}}", isOptimized: false, runtimeVersion: 0, emissionKind: NoDebug)
+; CHECK: [[META1]] = !DIFile(filename: "test.c", directory: {{.*}})
+; CHECK: [[DBG3]] = !DILocation(line: 10, column: 53, scope: [[META4:![0-9]+]])
+; CHECK: [[META4]] = distinct !DISubprogram(name: "foo", scope: [[META1]], file: [[META1]], line: 550, type: [[META5:![0-9]+]], scopeLine: 557, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: [[META0]], retainedNodes: [[META6:![0-9]+]])
+; CHECK: [[META5]] = distinct !DISubroutineType(types: [[META6]])
+; CHECK: [[META6]] = !{}
+;.

[OCHyams]

LGTM

[OCHyams]

Given the name of the test, maybe we don't need it, but I wonder if throwing in a quick comment might help reduce the chance of an incorrect update_test_checks in the future?