[DebugInfo] Invalidate debug info in ReassociatePass::RewriteExprTree

[bjope]

Summary

When Reassociate is rewriting an expression tree it may
reuse old binary expression nodes, for new expressions.
Whenever an expression node is reused, but with a non-trivial
change in the result, we need to invalidate any debug info
that is associated with the node.

If for example rewriting

x = mul a, b
y = mul c, x

into

x = mul c, b
y = mul a, x

we still get the same result for 'y', but 'x' is a new expression.
All debug info referring to 'x' must be invalidated (marked as
optimized out) since we no longer calculate the expected value.

As a side-effect this patch avoid (at least some) problems where
reassociate could end up creating IR with debug-use before def.
Earlier the dbg.value nodes where left untouched in the IR, while
the reused binary nodes where sinked to just before the root node
of the rewritten expression tree. See PR27273 for more info about
such problems.

[aprantl]

lib/Transforms/Scalar/Reassociate.cpp

785 ↗

(On Diff #143603)

has -> have

786 ↗

(On Diff #143603)

infor -> info

791 ↗

(On Diff #143603)

I think it would be better use use and undef instead of a nullptr here. Otherwise the intrinsic may will be deleted and doesn't serve as an end point of the live range opened by the previous intrinsic describing the same variable any more.

[bjope]

lib/Transforms/Scalar/Reassociate.cpp

791 ↗

(On Diff #143603)

Not sure if I understand what you mean by "use use and undef instead of nullptr".

I tried to find some place in the code where a dbg.value is invalidated (and not only erased) like this, to find out how to do it. But could not find any pattern to reuse. Do you know if we do this kind of invalidation somewhere already (maybe there is an existing helper similar to salvageDebugInfo that can be used)?

[aprantl]

lib/Transforms/Scalar/Reassociate.cpp

791 ↗

(On Diff #143603)

Sorry this was supposed to say: use an %undef instead of a nullptr, i.e.:

DII->setOperand(UndefValue::get(...), MetadataAsValue::get(DII->getContext(), nullptr));

[bjope]

lib/Transforms/Scalar/Reassociate.cpp

791 ↗

(On Diff #143603)

I thought the first was op-number, but maybe it is possible to do something like DII->setOperand(0, UndefValue::get(...)); ? But I think the operand also has to be metadata.

The resulting IR from my original proposal is basically

`call void @llvm.dbg.value(metadata !2, ...`

which is what Reassociate produce in other situations where it does rewrites without reusing existing instructions (when the original def is erased). Not sure exactly how the metadata is cleared in such situations (if it is for example is done as part of replaceAllUsesWith or eraseFromParent). I could not find anything explicit in Reassociate.cpp.

[aprantl]

lib/Transforms/Scalar/Reassociate.cpp

791 ↗

(On Diff #143603)

Yes the first argument is the operand number, so it should be something like: DII->setOperand(0, MetadataAsValue::get(DII->getContext(), ValueAsMetadata::get(UndefValue::get(...))))

791 ↗

(On Diff #143603)

see DIBuilder.cpp / getDbgIntrinsicValueImpl().

[bjope]

Thanks Adrian! Updated according to your suggestion and this should be more correct.

[aprantl]

Awesome!

llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp

 #include "llvm/IR/CFG.h"
 #include "llvm/IR/Constant.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Instructions.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Operator.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/IR/PatternMatch.h"
 #include "llvm/IR/Type.h"
 #include "llvm/IR/User.h"
 #include "llvm/IR/Value.h"
 #include "llvm/IR/ValueHandle.h"
 #include "llvm/Pass.h"
do {
         FastMathFlags Flags = I->getFastMathFlags();
         ExpressionChanged->clearSubclassOptionalData();
         ExpressionChanged->setFastMathFlags(Flags);
       } else
         ExpressionChanged->clearSubclassOptionalData();
 
       if (ExpressionChanged == I)
         break;
+
+      // Discard any debug info related to the expressions that has changed (we
+      // can leave debug infor related to the root, since the result of the
+      // expression tree should be the same even after reassociation).
+      SmallVector<DbgInfoIntrinsic *, 1> DbgUsers;
+      findDbgUsers(DbgUsers, ExpressionChanged);
+      for (auto *DII : DbgUsers) {
+        Value *Undef = UndefValue::get(ExpressionChanged->getType());
+        DII->setOperand(0, MetadataAsValue::get(DII->getContext(),
+                                                ValueAsMetadata::get(Undef)));
+      }
+
       ExpressionChanged->moveBefore(I);
       ExpressionChanged = cast<BinaryOperator>(*ExpressionChanged->user_begin());
     } while (true);
 
   // Throw away any left over nodes from the original expression.
   for (unsigned i = 0, e = NodesToRewrite.size(); i != e; ++i)
     RedoInsts.insert(NodesToRewrite[i]);
 }

llvm/trunk/test/Transforms/Reassociate/reassociate_dbgvalue_discard.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -reassociate -S -o - | FileCheck %s
+
+; After reassociation m1 and m2 aren't calculated as m1=c*a and m2=c*b any longer.
+; So let's verify that the dbg.value nodes for m1 and m3 are invalidated.
+
+source_filename = "reassociate_dbgvalue_discard.c"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define dso_local i32 @test1(i32 %a, i32 %b, i32 %c, i32 %d) local_unnamed_addr #0 !dbg !7 {
+; CHECK-LABEL: @test1(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    call void @llvm.dbg.value(metadata i32 undef, metadata !16, metadata !DIExpression()), !dbg !20
+; CHECK-NEXT:    call void @llvm.dbg.value(metadata i32 undef, metadata !17, metadata !DIExpression()), !dbg !21
+; CHECK-NEXT:    [[M1:%.*]] = mul i32 [[D:%.*]], [[C:%.*]], !dbg !22
+; CHECK-NEXT:    [[M3:%.*]] = mul i32 [[M1]], [[A:%.*]], !dbg !23
+; CHECK-NEXT:    call void @llvm.dbg.value(metadata i32 [[M3]], metadata !18, metadata !DIExpression()), !dbg !24
+; CHECK-NEXT:    [[M2:%.*]] = mul i32 [[D]], [[C]], !dbg !25
+; CHECK-NEXT:    [[M4:%.*]] = mul i32 [[M2]], [[B:%.*]], !dbg !26
+; CHECK-NEXT:    call void @llvm.dbg.value(metadata i32 [[M4]], metadata !19, metadata !DIExpression()), !dbg !27
+; CHECK-NEXT:    [[RES:%.*]] = xor i32 [[M3]], [[M4]]
+; CHECK-NEXT:    ret i32 [[RES]], !dbg !28
+;
+entry:
+  %m1 = mul i32 %c, %a, !dbg !24
+  call void @llvm.dbg.value(metadata i32 %m1, metadata !16, metadata !DIExpression()), !dbg !25
+  %m2 = mul i32 %c, %b, !dbg !26
+  call void @llvm.dbg.value(metadata i32 %m2, metadata !17, metadata !DIExpression()), !dbg !27
+  %m3 = mul i32 %m1, %d, !dbg !28
+  call void @llvm.dbg.value(metadata i32 %m3, metadata !18, metadata !DIExpression()), !dbg !29
+  %m4 = mul i32 %m2, %d, !dbg !30
+  call void @llvm.dbg.value(metadata i32 %m4, metadata !19, metadata !DIExpression()), !dbg !31
+  %res = xor i32 %m3, %m4
+  ret i32 %res, !dbg !32
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+attributes #0 = { nounwind readnone uwtable }
+attributes #1 = { nounwind readnone speculatable }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 7.0.0 (trunk 330596) (llvm/trunk 330594)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "reassociate_dbgvalue_discard.c", directory: "")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 7.0.0 (trunk 330596) (llvm/trunk 330594)"}
+!7 = distinct !DISubprogram(name: "test1", scope: !1, file: !1, line: 3, type: !8, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !11)
+!8 = !DISubroutineType(types: !9)
+!9 = !{!10, !10, !10, !10, !10}
+!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!11 = !{!12, !13, !14, !15, !16, !17, !18, !19}
+!12 = !DILocalVariable(name: "a", arg: 1, scope: !7, file: !1, line: 3, type: !10)
+!13 = !DILocalVariable(name: "b", arg: 2, scope: !7, file: !1, line: 3, type: !10)
+!14 = !DILocalVariable(name: "c", arg: 3, scope: !7, file: !1, line: 3, type: !10)
+!15 = !DILocalVariable(name: "d", arg: 4, scope: !7, file: !1, line: 3, type: !10)
+!16 = !DILocalVariable(name: "t1", scope: !7, file: !1, line: 4, type: !10)
+!17 = !DILocalVariable(name: "t2", scope: !7, file: !1, line: 5, type: !10)
+!18 = !DILocalVariable(name: "t3", scope: !7, file: !1, line: 6, type: !10)
+!19 = !DILocalVariable(name: "t4", scope: !7, file: !1, line: 7, type: !10)
+!20 = !DILocation(line: 3, column: 15, scope: !7)
+!21 = !DILocation(line: 3, column: 22, scope: !7)
+!22 = !DILocation(line: 3, column: 29, scope: !7)
+!23 = !DILocation(line: 3, column: 36, scope: !7)
+!24 = !DILocation(line: 4, column: 14, scope: !7)
+!25 = !DILocation(line: 4, column: 7, scope: !7)
+!26 = !DILocation(line: 5, column: 14, scope: !7)
+!27 = !DILocation(line: 5, column: 7, scope: !7)
+!28 = !DILocation(line: 6, column: 15, scope: !7)
+!29 = !DILocation(line: 6, column: 7, scope: !7)
+!30 = !DILocation(line: 7, column: 15, scope: !7)
+!31 = !DILocation(line: 7, column: 7, scope: !7)
+!32 = !DILocation(line: 8, column: 3, scope: !7)