diff --git a/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h b/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h --- a/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h +++ b/llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h @@ -1248,7 +1248,7 @@ for (auto *DeadMI : DeadInsts) { LLVM_DEBUG(dbgs() << *DeadMI << "Is dead, eagerly deleting\n"); WrapperObserver.erasingInstr(*DeadMI); - DeadMI->eraseFromParentAndMarkDBGValuesForRemoval(); + DeadMI->eraseFromParent(); } DeadInsts.clear(); } diff --git a/llvm/include/llvm/CodeGen/MachineInstr.h b/llvm/include/llvm/CodeGen/MachineInstr.h --- a/llvm/include/llvm/CodeGen/MachineInstr.h +++ b/llvm/include/llvm/CodeGen/MachineInstr.h @@ -1173,12 +1173,6 @@ /// eraseFromBundle() to erase individual bundled instructions. void eraseFromParent(); - /// Unlink 'this' from the containing basic block and delete it. - /// - /// For all definitions mark their uses in DBG_VALUE nodes - /// as undefined. Otherwise like eraseFromParent(). - void eraseFromParentAndMarkDBGValuesForRemoval(); - /// Unlink 'this' form its basic block and delete it. /// /// If the instruction is part of a bundle, the other instructions in the diff --git a/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp b/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp --- a/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp +++ b/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp @@ -147,9 +147,9 @@ if (isDead(MI)) { LLVM_DEBUG(dbgs() << "DeadMachineInstructionElim: DELETING: " << *MI); // It is possible that some DBG_VALUE instructions refer to this - // instruction. They get marked as undef and will be deleted - // in the live debug variable analysis. - MI->eraseFromParentAndMarkDBGValuesForRemoval(); + // instruction. They will be deleted in the live debug variable + // analysis. + MI->eraseFromParent(); AnyChanges = true; ++NumDeletes; continue; diff --git a/llvm/lib/CodeGen/GlobalISel/Combiner.cpp b/llvm/lib/CodeGen/GlobalISel/Combiner.cpp --- a/llvm/lib/CodeGen/GlobalISel/Combiner.cpp +++ b/llvm/lib/CodeGen/GlobalISel/Combiner.cpp @@ -136,7 +136,7 @@ // Erase dead insts before even adding to the list. if (isTriviallyDead(*CurMI, *MRI)) { LLVM_DEBUG(dbgs() << *CurMI << "Is dead; erasing.\n"); - CurMI->eraseFromParentAndMarkDBGValuesForRemoval(); + CurMI->eraseFromParent(); continue; } WorkList.deferred_insert(CurMI); diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp --- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp +++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp @@ -1541,8 +1541,8 @@ Builder.buildInstr(MatchInfo.Logic->getOpcode(), {Dest}, {Shift1, Shift2}); // These were one use so it's safe to remove them. - MatchInfo.Shift2->eraseFromParentAndMarkDBGValuesForRemoval(); - MatchInfo.Logic->eraseFromParentAndMarkDBGValuesForRemoval(); + MatchInfo.Shift2->eraseFromParent(); + MatchInfo.Logic->eraseFromParent(); MI.eraseFromParent(); } diff --git a/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp b/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp --- a/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp +++ b/llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp @@ -163,7 +163,7 @@ // If so, erase it. if (isTriviallyDead(MI, MRI)) { LLVM_DEBUG(dbgs() << "Is dead; erasing.\n"); - MI.eraseFromParentAndMarkDBGValuesForRemoval(); + MI.eraseFromParent(); continue; } @@ -261,6 +261,11 @@ continue; const TargetRegisterClass *RC = MRI.getRegClassOrNull(VReg); + + // Debug value instruction is permitted to use invalid vregs. + if (!RC && MI->isDebugValue()) + continue; + if (!RC) { reportGISelFailure(MF, TPC, MORE, "gisel-select", "VReg has no regclass after selection", *MI); diff --git a/llvm/lib/CodeGen/GlobalISel/Utils.cpp b/llvm/lib/CodeGen/GlobalISel/Utils.cpp --- a/llvm/lib/CodeGen/GlobalISel/Utils.cpp +++ b/llvm/lib/CodeGen/GlobalISel/Utils.cpp @@ -1171,25 +1171,6 @@ llvm::shouldOptimizeForSize(MBB.getBasicBlock(), PSI, BFI); } -/// These artifacts generally don't have any debug users because they don't -/// directly originate from IR instructions, but instead usually from -/// legalization. Avoiding checking for debug users improves compile time. -/// Note that truncates or extends aren't included because they have IR -/// counterparts which can have debug users after translation. -static bool shouldSkipDbgValueFor(MachineInstr &MI) { - switch (MI.getOpcode()) { - case TargetOpcode::G_UNMERGE_VALUES: - case TargetOpcode::G_MERGE_VALUES: - case TargetOpcode::G_CONCAT_VECTORS: - case TargetOpcode::G_BUILD_VECTOR: - case TargetOpcode::G_EXTRACT: - case TargetOpcode::G_INSERT: - return true; - default: - return false; - } -} - void llvm::saveUsesAndErase(MachineInstr &MI, MachineRegisterInfo &MRI, LostDebugLocObserver *LocObserver, SmallInstListTy &DeadInstChain) { @@ -1199,10 +1180,7 @@ } LLVM_DEBUG(dbgs() << MI << "Is dead; erasing.\n"); DeadInstChain.remove(&MI); - if (shouldSkipDbgValueFor(MI)) - MI.eraseFromParent(); - else - MI.eraseFromParentAndMarkDBGValuesForRemoval(); + MI.eraseFromParent(); if (LocObserver) LocObserver->checkpoint(false); } diff --git a/llvm/lib/CodeGen/MachineCombiner.cpp b/llvm/lib/CodeGen/MachineCombiner.cpp --- a/llvm/lib/CodeGen/MachineCombiner.cpp +++ b/llvm/lib/CodeGen/MachineCombiner.cpp @@ -485,7 +485,7 @@ MBB->insert((MachineBasicBlock::iterator)&MI, InstrPtr); for (auto *InstrPtr : DelInstrs) { - InstrPtr->eraseFromParentAndMarkDBGValuesForRemoval(); + InstrPtr->eraseFromParent(); // Erase all LiveRegs defined by the removed instruction for (auto I = RegUnits.begin(); I != RegUnits.end(); ) { if (I->MI == InstrPtr) diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp --- a/llvm/lib/CodeGen/MachineInstr.cpp +++ b/llvm/lib/CodeGen/MachineInstr.cpp @@ -682,26 +682,6 @@ getParent()->erase(this); } -void MachineInstr::eraseFromParentAndMarkDBGValuesForRemoval() { - assert(getParent() && "Not embedded in a basic block!"); - MachineBasicBlock *MBB = getParent(); - MachineFunction *MF = MBB->getParent(); - assert(MF && "Not embedded in a function!"); - - MachineInstr *MI = (MachineInstr *)this; - MachineRegisterInfo &MRI = MF->getRegInfo(); - - for (const MachineOperand &MO : MI->operands()) { - if (!MO.isReg() || !MO.isDef()) - continue; - Register Reg = MO.getReg(); - if (!Reg.isVirtual()) - continue; - MRI.markUsesInDebugValueAsUndef(Reg); - } - MI->eraseFromParent(); -} - void MachineInstr::eraseFromBundle() { assert(getParent() && "Not embedded in a basic block!"); getParent()->erase_instr(this); diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp --- a/llvm/lib/CodeGen/MachineVerifier.cpp +++ b/llvm/lib/CodeGen/MachineVerifier.cpp @@ -1986,41 +1986,48 @@ if (MO->isUndef()) report("Generic virtual register use cannot be undef", MO, MONum); - // If we're post-Select, we can't have gvregs anymore. - if (isFunctionSelected) { - report("Generic virtual register invalid in a Selected function", - MO, MONum); - return; - } + // Debug value instruction is permitted to use invalid vregs. + // This is a performance measure to skip the overhead of immediately + // pruning unused debug operands. The final undef substitution occurs + // in LDVImpl::handleDebugValue. + if (!MO->isUse() || !MI->isDebugValue()) { + // If we're post-Select, we can't have gvregs anymore. + if (isFunctionSelected) { + report("Generic virtual register invalid in a Selected function", + MO, MONum); + return; + } - // The gvreg must have a type and it must not have a SubIdx. - LLT Ty = MRI->getType(Reg); - if (!Ty.isValid()) { - report("Generic virtual register must have a valid type", MO, - MONum); - return; - } + // The gvreg must have a type and it must not have a SubIdx. + LLT Ty = MRI->getType(Reg); + if (!Ty.isValid()) { + report("Generic virtual register must have a valid type", MO, + MONum); + return; + } - const RegisterBank *RegBank = MRI->getRegBankOrNull(Reg); + const RegisterBank *RegBank = MRI->getRegBankOrNull(Reg); - // If we're post-RegBankSelect, the gvreg must have a bank. - if (!RegBank && isFunctionRegBankSelected) { - report("Generic virtual register must have a bank in a " - "RegBankSelected function", - MO, MONum); - return; - } + // If we're post-RegBankSelect, the gvreg must have a bank. + if (!RegBank && isFunctionRegBankSelected) { + report("Generic virtual register must have a bank in a " + "RegBankSelected function", + MO, MONum); + return; + } - // Make sure the register fits into its register bank if any. - if (RegBank && Ty.isValid() && - RegBank->getSize() < Ty.getSizeInBits()) { - report("Register bank is too small for virtual register", MO, - MONum); - errs() << "Register bank " << RegBank->getName() << " too small(" - << RegBank->getSize() << ") to fit " << Ty.getSizeInBits() - << "-bits\n"; - return; + // Make sure the register fits into its register bank if any. + if (RegBank && Ty.isValid() && + RegBank->getSize() < Ty.getSizeInBits()) { + report("Register bank is too small for virtual register", MO, + MONum); + errs() << "Register bank " << RegBank->getName() << " too small(" + << RegBank->getSize() << ") to fit " << Ty.getSizeInBits() + << "-bits\n"; + return; + } } + if (SubIdx) { report("Generic virtual register does not allow subregister index", MO, MONum); diff --git a/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp b/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp --- a/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp +++ b/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp @@ -1628,7 +1628,7 @@ // Erase the REG_SEQUENCE eagerly, unless we followed a chain of COPY users, // in which case we can erase them all later in runOnMachineFunction. if (MRI->use_nodbg_empty(MI.getOperand(0).getReg())) - MI.eraseFromParentAndMarkDBGValuesForRemoval(); + MI.eraseFromParent(); return true; } @@ -1829,7 +1829,7 @@ while (MRI->use_nodbg_empty(InstToErase->getOperand(0).getReg())) { auto &SrcOp = InstToErase->getOperand(1); auto SrcReg = SrcOp.isReg() ? SrcOp.getReg() : Register(); - InstToErase->eraseFromParentAndMarkDBGValuesForRemoval(); + InstToErase->eraseFromParent(); InstToErase = nullptr; if (!SrcReg || SrcReg.isPhysical()) break; @@ -1839,7 +1839,7 @@ } if (InstToErase && InstToErase->isRegSequence() && MRI->use_nodbg_empty(InstToErase->getOperand(0).getReg())) - InstToErase->eraseFromParentAndMarkDBGValuesForRemoval(); + InstToErase->eraseFromParent(); } } return true; diff --git a/llvm/lib/Target/NVPTX/NVPTXPeephole.cpp b/llvm/lib/Target/NVPTX/NVPTXPeephole.cpp --- a/llvm/lib/Target/NVPTX/NVPTXPeephole.cpp +++ b/llvm/lib/Target/NVPTX/NVPTXPeephole.cpp @@ -126,9 +126,9 @@ // Check if MRI has only one non dbg use, which is Root if (MRI.hasOneNonDBGUse(Prev.getOperand(0).getReg())) { - Prev.eraseFromParentAndMarkDBGValuesForRemoval(); + Prev.eraseFromParent(); } - Root.eraseFromParentAndMarkDBGValuesForRemoval(); + Root.eraseFromParent(); } bool NVPTXPeephole::runOnMachineFunction(MachineFunction &MF) { @@ -157,7 +157,7 @@ const auto &MRI = MF.getRegInfo(); if (MRI.use_empty(NRI->getFrameRegister(MF))) { if (auto MI = MRI.getUniqueVRegDef(NRI->getFrameRegister(MF))) { - MI->eraseFromParentAndMarkDBGValuesForRemoval(); + MI->eraseFromParent(); } } diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/select-dbg-value.mir b/llvm/test/CodeGen/AArch64/GlobalISel/select-dbg-value.mir --- a/llvm/test/CodeGen/AArch64/GlobalISel/select-dbg-value.mir +++ b/llvm/test/CodeGen/AArch64/GlobalISel/select-dbg-value.mir @@ -62,7 +62,7 @@ liveins: $w0 ; CHECK-LABEL: name: test_dbg_value_dead ; CHECK-NOT: COPY - ; CHECK: DBG_VALUE $noreg, $noreg, !7, !DIExpression(), debug-location !9 + ; CHECK: DBG_VALUE %0:gpr, $noreg, !7, !DIExpression(), debug-location !9 %0:gpr(s32) = COPY $w0 DBG_VALUE %0(s32), $noreg, !7, !DIExpression(), debug-location !9 ... diff --git a/llvm/test/CodeGen/AMDGPU/fold-readlane.mir b/llvm/test/CodeGen/AMDGPU/fold-readlane.mir --- a/llvm/test/CodeGen/AMDGPU/fold-readlane.mir +++ b/llvm/test/CodeGen/AMDGPU/fold-readlane.mir @@ -14,7 +14,7 @@ # GCN-LABEL: name: fold-imm-readfirstlane-dbgvalue{{$}} # GCN: %1:sreg_32_xm0 = S_MOV_B32 123 -# GCN: DBG_VALUE $noreg, 0, 0 +# GCN: DBG_VALUE %0:vgpr_32, 0, 0 --- name: fold-imm-readfirstlane-dbgvalue tracksRegLiveness: true diff --git a/llvm/test/CodeGen/X86/GlobalISel/x86-calllowering-dbg-trunc.ll b/llvm/test/CodeGen/X86/GlobalISel/x86-calllowering-dbg-trunc.ll new file mode 100644 --- /dev/null +++ b/llvm/test/CodeGen/X86/GlobalISel/x86-calllowering-dbg-trunc.ll @@ -0,0 +1,57 @@ +; RUN: llc -mtriple=i386-linux-gnu -global-isel -verify-machineinstrs < %s -o - | FileCheck %s --check-prefix=ALL + +; This file is the output of clang -g -O2 +; int test_dbg_trunc(unsigned long long a) { return a; } +; +; The intent of this check is to ensure the DBG_VALUE use of G_MERGE_VALUES is undef'd when the legalizer erases it. + +; ModuleID = 'x86-calllowering-dbg-trunc.c' +source_filename = "x86-calllowering-dbg-trunc.c" +target datalayout = "e-m:e-p:32:32-p270:32:32-p271:32:32-p272:64:64-f64:32:64-f80:32-n8:16:32-S128" +target triple = "i386" + +; Function Attrs: mustprogress nofree norecurse nosync nounwind readnone uwtable willreturn +define dso_local i32 @test_dbg_trunc(i64 %a) local_unnamed_addr #0 !dbg !9 { +; ALL-LABEL: test_dbg_trunc: +; ALL: # %bb.0: # %entry +; ALL: pushl %ebp +; ALL: movl %esp, %ebp +; ALL: movl 8(%ebp), %eax +; ALL: #DEBUG_VALUE: test_dbg_trunc:a <- undef +; ALL: popl %ebp +; ALL: retl +entry: + call void @llvm.dbg.value(metadata i64 %a, metadata !15, metadata !DIExpression()), !dbg !16 + %conv = trunc i64 %a to i32, !dbg !17 + ret i32 %conv, !dbg !18 +} + +; Function Attrs: nofree nosync nounwind readnone speculatable willreturn +declare void @llvm.dbg.value(metadata, metadata, metadata) #1 + +attributes #0 = { mustprogress nofree norecurse nosync nounwind readnone uwtable willreturn "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="pentium4" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" } +attributes #1 = { nofree nosync nounwind readnone speculatable willreturn } + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!2, !3, !4, !5, !6, !7} +!llvm.ident = !{!8} + +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 14.0.0 (https://github.com/llvm/llvm-project ...)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None) +!1 = !DIFile(filename: "x86-calllowering-dbg-trunc.c", directory: "/tmp") +!2 = !{i32 1, !"NumRegisterParameters", i32 0} +!3 = !{i32 7, !"Dwarf Version", i32 4} +!4 = !{i32 2, !"Debug Info Version", i32 3} +!5 = !{i32 1, !"wchar_size", i32 4} +!6 = !{i32 7, !"uwtable", i32 1} +!7 = !{i32 7, !"frame-pointer", i32 2} +!8 = !{!"clang version 14.0.0 (https://github.com/llvm/llvm-project ...)"} +!9 = distinct !DISubprogram(name: "test_dbg_trunc", scope: !1, file: !1, line: 1, type: !10, scopeLine: 1, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !14) +!10 = !DISubroutineType(types: !11) +!11 = !{!12, !13} +!12 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) +!13 = !DIBasicType(name: "unsigned long long", size: 64, encoding: DW_ATE_unsigned) +!14 = !{!15} +!15 = !DILocalVariable(name: "a", arg: 1, scope: !9, file: !1, line: 1, type: !13) +!16 = !DILocation(line: 0, scope: !9) +!17 = !DILocation(line: 1, column: 51, scope: !9) +!18 = !DILocation(line: 1, column: 44, scope: !9)