Skip to content

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

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.

Sign up for GitHub

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 2 commits into from
Apr 8, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f430aad
[DebugInfo][Reassociate] When factoring expressions, set DebugLoc for…
SLTozer Oct 30, 2024
1ba836f
Update test case and add comment
SLTozer Apr 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion llvm/include/llvm/Transforms/Scalar/Reassociate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
12 changes: 9 additions & 3 deletions llvm/lib/Transforms/Scalar/Reassociate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;) {
Expand Down
45 changes: 45 additions & 0 deletions llvm/test/Transforms/Reassociate/debugloc-factoring-neg.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
; RUN: opt -S -p=reassociate < %s | FileCheck %s

Copy link
Contributor

Choose a reason for hiding this comment

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

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?

;; When reassociating the expression below, we reduce it to using a single
;; multiply and as part of that we generate a sub that negates one of the
;; original terms; this negation "neg" should use the original expression's
;; source location.

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]]
; 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]] = !{}
;.