Index: include/llvm/IR/DebugInfoMetadata.h =================================================================== --- include/llvm/IR/DebugInfoMetadata.h +++ include/llvm/IR/DebugInfoMetadata.h @@ -778,6 +778,12 @@ unsigned getEncoding() const { return Encoding; } + enum class Signedness { Signed, Unsigned }; + + /// Return the signedness of this type, or None if this type is neither + /// signed nor unsigned. + Optional getSignedness() const; + static bool classof(const Metadata *MD) { return MD->getMetadataID() == DIBasicTypeKind; } @@ -2206,6 +2212,14 @@ /// Determines the size of the variable's type. Optional getSizeInBits() const; + /// Return the signedness of this variable's type, or None if this type is + /// neither signed nor unsigned. + Optional getSignedness() const { + if (auto *BT = dyn_cast(getType().resolve())) + return BT->getSignedness(); + return None; + } + StringRef getFilename() const { if (auto *F = getFile()) return F->getFilename(); Index: include/llvm/Transforms/Utils/Local.h =================================================================== --- include/llvm/Transforms/Utils/Local.h +++ include/llvm/Transforms/Utils/Local.h @@ -337,17 +337,9 @@ /// Assuming the value \p From is going to be deleted, insert replacement /// dbg.value intrinsics for each debug user of \p From. The newly-inserted /// dbg.values refer to \p To instead of \p From. Each replacement dbg.value -/// has the same location and variable as the debug user it replaces, has a -/// DIExpression determined by the result of \p RewriteExpr applied to an old -/// debug user of \p From, and is placed before \p InsertBefore. If -/// \p RewriteExpr returns nullptr, no replacement for the specified debug -/// user is emitted. -void insertReplacementDbgValues( - Value &From, Value &To, Instruction &InsertBefore, - function_ref RewriteExpr); - -/// An overload of insertReplacementDbgValues() for the common case where -/// the replacement dbg.values have the same DIExpressions as the originals. +/// has the same location and variable as the debug user it replaces and is +/// placed before \p InsertBefore. \p From must be convertible to \p To using +/// truncation, (sign|zero|fp)-extension, or the identity mapping. void insertReplacementDbgValues(Value &From, Value &To, Instruction &InsertBefore); Index: lib/CodeGen/AsmPrinter/DwarfExpression.cpp =================================================================== --- lib/CodeGen/AsmPrinter/DwarfExpression.cpp +++ lib/CodeGen/AsmPrinter/DwarfExpression.cpp @@ -357,6 +357,9 @@ case dwarf::DW_OP_shl: case dwarf::DW_OP_shr: case dwarf::DW_OP_shra: + case dwarf::DW_OP_lit0: + case dwarf::DW_OP_not: + case dwarf::DW_OP_dup: emitOp(Op->getOp()); break; case dwarf::DW_OP_deref: Index: lib/IR/DebugInfoMetadata.cpp =================================================================== --- lib/IR/DebugInfoMetadata.cpp +++ lib/IR/DebugInfoMetadata.cpp @@ -283,6 +283,19 @@ Ops); } +Optional DIBasicType::getSignedness() const { + switch (getEncoding()) { + case dwarf::DW_ATE_signed: + case dwarf::DW_ATE_signed_char: + return Signedness::Signed; + case dwarf::DW_ATE_unsigned: + case dwarf::DW_ATE_unsigned_char: + return Signedness::Unsigned; + default: + return None; + } +} + DIDerivedType *DIDerivedType::getImpl( LLVMContext &Context, unsigned Tag, MDString *Name, Metadata *File, unsigned Line, Metadata *Scope, Metadata *BaseType, uint64_t SizeInBits, @@ -733,6 +746,9 @@ case dwarf::DW_OP_shra: case dwarf::DW_OP_deref: case dwarf::DW_OP_xderef: + case dwarf::DW_OP_lit0: + case dwarf::DW_OP_not: + case dwarf::DW_OP_dup: break; } } Index: lib/Transforms/InstCombine/InstCombineCasts.cpp =================================================================== --- lib/Transforms/InstCombine/InstCombineCasts.cpp +++ lib/Transforms/InstCombine/InstCombineCasts.cpp @@ -270,9 +270,7 @@ auto *Res = CastInst::Create(NewOpc, CSrc->getOperand(0), Ty); // Replace debug users of the eliminable cast by emitting debug values // which refer to the new cast. - if (Ty->isIntegerTy() || Ty->isPointerTy()) - // TODO: Support floats and vectors (see DW_OP_convert, fragment). - insertReplacementDbgValues(*CSrc, *Res, *std::next(CI.getIterator())); + insertReplacementDbgValues(*CSrc, *Res, *std::next(CI.getIterator())); return Res; } } Index: lib/Transforms/Utils/Local.cpp =================================================================== --- lib/Transforms/Utils/Local.cpp +++ lib/Transforms/Utils/Local.cpp @@ -1704,9 +1704,16 @@ } } -void llvm::insertReplacementDbgValues( +/// A replacement for a dbg.value expression. +using DbgValReplacement = Optional; + +/// Insert replacement dbg.values for \p From which refer to \p To. The +/// DIExpression for new dbg.values is determined by \p RewriteExpr. If there +/// is no expression rewrite for a dbg.value, no replacement for that dbg.value +/// is emitted. +static void replaceDbgValues( Value &From, Value &To, Instruction &InsertBefore, - function_ref RewriteExpr) { + function_ref RewriteExpr) { // Collect all debug users of From. SmallVector Users; findDbgUsers(Users, &From); @@ -1716,21 +1723,99 @@ // Insert a replacement debug value for each old debug user. It's assumed // that the old debug users will be erased later. DIBuilder DIB(*InsertBefore.getModule()); - for (auto *OldDII : Users) - if (DIExpression *Expr = RewriteExpr(*OldDII)) { - auto *I = DIB.insertDbgValueIntrinsic(&To, OldDII->getVariable(), Expr, - OldDII->getDebugLoc().get(), - &InsertBefore); - (void)I; - LLVM_DEBUG(dbgs() << "REPLACE: " << *I << '\n'); - } + SmallVector Replacements; + for (auto *OldDII : Users) { + DbgValReplacement DVR = RewriteExpr(*OldDII); + if (!DVR) + continue; + auto *I = + DIB.insertDbgValueIntrinsic(&To, OldDII->getVariable(), *DVR, + OldDII->getDebugLoc().get(), &InsertBefore); + Replacements.push_back(I); + LLVM_DEBUG(dbgs() << "REPLACE: " << *I << '\n'); + } +} + +/// Check if the debug info representation of values of type \p FromTy and \p +/// ToTy are compatible. +/// +/// Note that Type::canLosslesslyBitCastTo is not suitable here because it +/// allows semantically unequivalent bitcasts, such as <2 x i64> -> <4 x i32>, +/// but does not allow lossless pointer <-> integer conversions. +static bool hasCompatibleDebugInfoRepresentation(const DataLayout &DL, + Type *FromTy, Type *ToTy) { + // Trivially compatible types. + if (FromTy == ToTy) + return true; + + // Handle compatible pointer <-> integer conversions. + return (FromTy->isPointerTy() || ToTy->isPointerTy()) && + !DL.isNonIntegralPointerType(FromTy) && + !DL.isNonIntegralPointerType(ToTy); } void llvm::insertReplacementDbgValues(Value &From, Value &To, Instruction &InsertBefore) { - return llvm::insertReplacementDbgValues( - From, To, InsertBefore, - [](DbgInfoIntrinsic &OldDII) { return OldDII.getExpression(); }); + Type *FromTy = From.getType(); + Type *ToTy = To.getType(); + + auto Identity = [&](DbgInfoIntrinsic &OldDII) -> DbgValReplacement { + return OldDII.getExpression(); + }; + + // The simple case: no conversion between the values is necessary. + const DataLayout &DL = InsertBefore.getModule()->getDataLayout(); + if (hasCompatibleDebugInfoRepresentation(DL, FromTy, ToTy)) { + replaceDbgValues(From, To, InsertBefore, Identity); + return; + } + + // Handle integer-to-integer widening and narrowing. + // FIXME: Use DW_OP_convert when it's available everywhere. + if (FromTy->isIntegerTy() && ToTy->isIntegerTy()) { + uint64_t FromBits = FromTy->getPrimitiveSizeInBits(); + uint64_t ToBits = ToTy->getPrimitiveSizeInBits(); + + // When the width of the result grows, assume that a debugger will only + // access the low `FromBits` bits when inspecting the source variable. + if (FromBits < ToBits) { + replaceDbgValues(From, To, InsertBefore, Identity); + return; + } + + // The width of the result has shrunk. Use sign/zero extension to describe + // the source variable's high bits. + auto SignOrZeroExt = [&](DbgInfoIntrinsic &OldDII) -> DbgValReplacement { + DILocalVariable *Var = OldDII.getVariable(); + + // Without knowing signedness, sign/zero extension isn't possible. + auto Signedness = Var->getSignedness(); + if (!Signedness) + return None; + + bool Signed = *Signedness == DIBasicType::Signedness::Signed; + + if (!Signed) { + // In the unsigned case, assume that a debugger will initialize the high + // bits to 0 and use a no-op conversion. + return Identity(OldDII); + } else { + // In the signed case, the high bits are given by sign extension, i.e: + // (To >> (ToBits - 1)) * ((2 ^ FromBits) - 1) + // Calculate the high bits and OR them together with the low bits. + SmallVector Ops({dwarf::DW_OP_dup, dwarf::DW_OP_constu, + (ToBits - 1), dwarf::DW_OP_shr, + dwarf::DW_OP_lit0, dwarf::DW_OP_not, + dwarf::DW_OP_mul, dwarf::DW_OP_or}); + return DIExpression::prependOpcodes(OldDII.getExpression(), Ops, + DIExpression::WithStackValue); + } + }; + replaceDbgValues(From, To, InsertBefore, SignOrZeroExt); + return; + } + + // TODO: Floating-point conversions, vectors. } unsigned llvm::removeAllNonTerminatorAndEHPadInstructions(BasicBlock *BB) { Index: test/Transforms/InstCombine/cast-set-preserve-signed-dbg-val.ll =================================================================== --- /dev/null +++ test/Transforms/InstCombine/cast-set-preserve-signed-dbg-val.ll @@ -0,0 +1,50 @@ +; RUN: opt -instcombine -S < %s | FileCheck %s + +; CHECK-LABEL: define {{.*}} @test5 +define i16 @test5(i16 %A) !dbg !34 { + ; CHECK: [[and:%.*]] = and i16 %A, 15 + + %B = sext i16 %A to i32, !dbg !40 + call void @llvm.dbg.value(metadata i32 %B, metadata !36, metadata !DIExpression()), !dbg !40 + + %C = and i32 %B, 15, !dbg !41 + call void @llvm.dbg.value(metadata i32 %C, metadata !37, metadata !DIExpression()), !dbg !41 + + ; Preserve the dbg.value for the DCE'd 32-bit 'and'. + ; + ; The high 16 bits of the original 'and' require sign-extending the new 16-bit and: + ; CHECK-NEXT: call void @llvm.dbg.value(metadata i16 [[and]], metadata [[C:![0-9]+]], + ; CHECK-SAME: metadata !DIExpression(DW_OP_dup, DW_OP_constu, 15, DW_OP_shr, DW_OP_lit0, DW_OP_not, DW_OP_mul, DW_OP_or, DW_OP_stack_value) + + %D = trunc i32 %C to i16, !dbg !42 + call void @llvm.dbg.value(metadata i16 %D, metadata !38, metadata !DIExpression()), !dbg !42 + + ; The dbg.value for a truncate should simply point to the result of the 16-bit 'and'. + ; CHECK-NEXT: call void @llvm.dbg.value(metadata i16 [[and]], metadata [[D:![0-9]+]], metadata !DIExpression()) + + ret i16 %D, !dbg !43 + ; CHECK-NEXT: ret i16 [[and]] +} + +declare void @llvm.dbg.value(metadata, metadata, metadata) + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!5} + +!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2) +!1 = !DIFile(filename: "void", directory: "/") +!2 = !{} +!5 = !{i32 2, !"Debug Info Version", i32 3} +!7 = !DISubroutineType(types: !2) +!10 = !DIBasicType(name: "ty32", size: 32, encoding: DW_ATE_signed) +!12 = !DIBasicType(name: "ty8", size: 8, encoding: DW_ATE_signed) +!34 = distinct !DISubprogram(name: "test5", linkageName: "test5", scope: null, file: !1, line: 12, type: !7, isLocal: false, isDefinition: true, scopeLine: 12, isOptimized: true, unit: !0, retainedNodes: !35) +!35 = !{!36, !37, !38} +!36 = !DILocalVariable(name: "B", scope: !34, file: !1, line: 12, type: !10) +!37 = !DILocalVariable(name: "C", scope: !34, file: !1, line: 13, type: !10) +!38 = !DILocalVariable(name: "D", scope: !34, file: !1, line: 14, type: !39) +!39 = !DIBasicType(name: "ty16", size: 16, encoding: DW_ATE_signed) +!40 = !DILocation(line: 12, column: 1, scope: !34) +!41 = !DILocation(line: 13, column: 1, scope: !34) +!42 = !DILocation(line: 14, column: 1, scope: !34) +!43 = !DILocation(line: 15, column: 1, scope: !34) Index: test/Transforms/InstCombine/dce-iterate.ll =================================================================== --- test/Transforms/InstCombine/dce-iterate.ll +++ test/Transforms/InstCombine/dce-iterate.ll @@ -1,9 +1,19 @@ -; RUN: opt < %s -instcombine -S | grep "ret double .sy" +; RUN: opt < %s -debugify -instcombine -S | FileCheck %s +; CHECK-LABEL: define {{.*}} @ScaleObjectAdd define internal double @ScaleObjectAdd(double %sx, double %sy, double %sz) nounwind { entry: +; CHECK: call void @llvm.dbg.value(metadata double %sx, metadata [[sx34:![0-9]+]], metadata !DIExpression()) + %sx34 = bitcast double %sx to i64 ; [#uses=1] %sx3435 = zext i64 %sx34 to i960 ; [#uses=1] + +; The i64 -> i960 zero extension is DCE'd. Check that we preserve debug values +; for the intermediate SSA values. +; +; This is the bitcasted double %sx. +; CHECK: call void @llvm.dbg.value(metadata double %sx, metadata [[sx3435:![0-9]+]], metadata !DIExpression()) + %sy22 = bitcast double %sy to i64 ; [#uses=1] %sy2223 = zext i64 %sy22 to i960 ; [#uses=1] %sy222324 = shl i960 %sy2223, 320 ; [#uses=1] @@ -20,5 +30,9 @@ %e = bitcast i64 %d to double ; [#uses=1] %f = fadd double %b, %e +; CHECK: ret double %sy ret double %e } + +; CHECK: [[sx34]] = !DILocalVariable(name: "1" +; CHECK: [[sx3435]] = !DILocalVariable(name: "2"