[DebugInfo] Handle multiple variable location operands in IR

[StephenTozer]

Summary

Following from the previous patch in this series which enables DIArgLists with a single location operand in debug intrinsics, this patch updates the various IR passes to correctly handle multiple location operands.

Compared to the previous 2 patches, this one has the most actual logic, rather than simply being an interface or data change. This patch does not allow salvageDebugInfo to produce DIArgLists by salvaging instructions with a non-constant second operand; it also does not update almost any of CodeGen. Other than that, it should cover every IR pass.

Most of the changes are relatively straightforward, as we simply extend code that operated on a single debug value to operate on the list of debug values in the style of any_of, all_of, for_each, etc. We also have replaced setOperand(0, ...) with replaceVariableLocationOp, which requires us to pass the variable that is being replaced. In places where this value isn't readily available, we have to track the old value through to the point where it gets replaced.

One of the more complicated cases is insertDebugValuesForPHIs; the comments have been updated to explain the new implementation, but I imagine it'll take some effort to confirm that the implementation does what it says on the tin. Also, a question that I don't have a definitive answer on yet: how do we handle moving dbg.values that depend on multiple instructions? It's quite possible there's no way to preserve many of them, because when we move them to follow a sunk or hoisted instruction we risk losing the other instructions. placeDbgValues simply gives up for dbg.values with multiple location operands, as it isn't particularly reliable even in the single op case. As currently stands, I believe that the new insertDebugValuesForPHIs will always produce valid dbg.values, but am interested to find out if anyone sees an error with the logic or has an example where it fails.

[aprantl]

This mostly LGTM.

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

1197

That's how :-)

1277

nit: remove redundant {}?

[StephenTozer]

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

1277

The braces in this case are following the coding standards, specifically following the examples "Use braces for the outer if since the nested for is braced" and "Use braces on the outer block because there are more than two levels of nesting."

[djtodoro]

mostly lgtm, some initinal comments inline, thanks!

llvm/lib/CodeGen/CodeGenPrepare.cpp

2768

Can we add a comment describing what we use this for?

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

1225

So here we will be using the new metadata by salvaging dbg.values with multiple SSA values?

llvm/lib/Transforms/Utils/CodeExtractor.cpp

1474

Can we refactor this comments? :)

llvm/lib/Transforms/Utils/Debugify.cpp

263

Should this go into the Verifier, since we do not use this in the SalvageDebugInfo (yet) ?

llvm/lib/Transforms/Utils/LCSSA.cpp

226

This should be different (nfc) change.

[StephenTozer]

Rebase onto recent master, no functional changes from rebase. Minor changes to findDbgUsers/findDbgValues to handle DIArgLists in which the same value appears more than once.

[aprantl]

llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp

261–263

clang-format?

[StephenTozer]

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

1225

Yes, this isn't currently implemented in the patch stack, but should be implemented later. It's definitely out of scope for this patch, regardless.

[StephenTozer]

Rebased onto recent master.

[hans]

This caused non-deterministic output from the compiler, see https://bugs.chromium.org/p/chromium/issues/detail?id=1189090#c8 for a reproducer. (Usually this is due to non-deterministic iteration order for hash-based data structures such as DenseMap)

I've reverted in 01ac6d1587e8613ba4278786e8341f8b492ac941 until it can be fixed.

[StephenTozer]

In D88232#2631602, @hans wrote:

This caused non-deterministic output from the compiler, see https://bugs.chromium.org/p/chromium/issues/detail?id=1189090#c8 for a reproducer. (Usually this is due to non-deterministic iteration order for hash-based data structures such as DenseMap)

I've reverted in 01ac6d1587e8613ba4278786e8341f8b492ac941 until it can be fixed.

Resubmitted, should be fixed now - instructions were being inserted while iterating through a map, the map type has been changed from SmallDenseMap to MapVector now, so there should no longer be any non-determinism.

llvm/include/llvm/IR/DebugInfoMetadata.h

public:
   /// the same fragment.
   static DIExpression *appendToStack(const DIExpression *Expr,
                                      ArrayRef<uint64_t> Ops);
 
   /// Modify \p Expr by appending the given list of \p Ops to each instance of
   /// the operand `DW_OP_LLVM_arg, \p ArgNo`. This is used to modify a
   /// specific location used by \p Expr, such as when salvaging that location.
   static DIExpression *appendOpsToArg(const DIExpression *Expr,
-                                      ArrayRef<uint64_t> Ops, uint64_t ArgNo);
+                                      ArrayRef<uint64_t> Ops, unsigned ArgNo,
+                                      bool StackValue = false);
 
   /// Create a DIExpression to describe one part of an aggregate variable that
   /// is fragmented across multiple Values. The DW_OP_LLVM_fragment operation
   /// will be appended to the elements of \c Expr. If \c Expr already contains
   /// a \c DW_OP_LLVM_fragment \c OffsetInBits is interpreted as an offset
   /// into the existing fragment.
   ///
   /// \param OffsetInBits Offset of the piece in bits.

llvm/include/llvm/Transforms/Utils/Local.h

 /// Returns true if any debug users were updated.
 /// Mark undef if salvaging cannot be completed.
 void salvageDebugInfoForDbgValues(Instruction &I,
                                   ArrayRef<DbgVariableIntrinsic *> Insns);
 
 /// Given an instruction \p I and DIExpression \p DIExpr operating on it, write
 /// the effects of \p I into the returned DIExpression, or return nullptr if
 /// it cannot be salvaged. \p StackVal: whether DW_OP_stack_value should be
-/// appended to the expression.
+/// appended to the expression. \p LocNo: the index of the location operand to
+/// which \p I applies, should be 0 for debug info without a DIArgList.
 DIExpression *salvageDebugInfoImpl(Instruction &I, DIExpression *DIExpr,
-                                   bool StackVal);
+                                   bool StackVal, unsigned LocNo);
 
 /// Point debug users of \p From to \p To or salvage them. Use this function
 /// only when replacing all uses of \p From with \p To, with a guarantee that
 /// \p From is going to be deleted.
 ///
 /// Follow these rules to prevent use-before-def of \p To:
 ///   . If \p To is a linked Instruction, set \p DomPoint to \p To.
 ///   . If \p To is an unlinked Instruction, set \p DomPoint to the Instruction

llvm/lib/CodeGen/CodeGenPrepare.cpp

struct InstructionAndIdx {
           : Inst(Inst), Idx(Idx) {}
     };
 
     /// Keep track of the original uses (pair Instruction, Index).
     SmallVector<InstructionAndIdx, 4> OriginalUses;
     /// Keep track of the debug users.
     SmallVector<DbgValueInst *, 1> DbgValues;
 
+    Value *New;
+
     using use_iterator = SmallVectorImpl<InstructionAndIdx>::iterator;
 
   public:
     /// Replace all the use of \p Inst by \p New.
-    UsesReplacer(Instruction *Inst, Value *New) : TypePromotionAction(Inst) {
+    UsesReplacer(Instruction *Inst, Value *New)
+        : TypePromotionAction(Inst), New(New) {
       LLVM_DEBUG(dbgs() << "Do: UsersReplacer: " << *Inst << " with " << *New
                         << "\n");
       // Record the original uses.
       for (Use &U : Inst->uses()) {
         Instruction *UserI = cast<Instruction>(U.getUser());
         OriginalUses.push_back(InstructionAndIdx(UserI, U.getOperandNo()));
       }
       // Record the debug uses separately. They are not in the instruction's
void undo() override {
            UseIt != EndIt; ++UseIt) {
         UseIt->Inst->setOperand(UseIt->Idx, Inst);
       }
       // RAUW has replaced all original uses with references to the new value,
       // including the debug uses. Since we are undoing the replacements,
       // the original debug uses must also be reinstated to maintain the
       // correctness and utility of debug value instructions.
       for (auto *DVI : DbgValues)
-        DVI->replaceVariableLocationOp(DVI->getVariableLocationOp(0), Inst);
+        DVI->replaceVariableLocationOp(New, Inst);
     }
   };
 
   /// Remove an instruction from the IR.
   class InstructionRemover : public TypePromotionAction {
     /// Original position of the instruction.
     InsertionHandler Inserter;
 

 
 // Some CGP optimizations may move or alter what's computed in a block. Check
 // whether a dbg.value intrinsic could be pointed at a more appropriate operand.
 bool CodeGenPrepare::fixupDbgValue(Instruction *I) {
   assert(isa<DbgValueInst>(I));
   DbgValueInst &DVI = *cast<DbgValueInst>(I);
 
   // Does this dbg.value refer to a sunk address calculation?
-  Value *Location = DVI.getVariableLocationOp(0);
+  bool AnyChange = false;
+  for (Value *Location : DVI.getValues()) {
     WeakTrackingVH SunkAddrVH = SunkAddrs[Location];
     Value *SunkAddr = SunkAddrVH.pointsToAliveValue() ? SunkAddrVH : nullptr;
     if (SunkAddr) {
       // Point dbg.value at locally computed address, which should give the best
-    // opportunity to be accurately lowered. This update may change the type of
-    // pointer being referred to; however this makes no difference to debugging
-    // information, and we can't generate bitcasts that may affect codegen.
+      // opportunity to be accurately lowered. This update may change the type
+      // of pointer being referred to; however this makes no difference to
+      // debugging information, and we can't generate bitcasts that may affect
+      // codegen.
       DVI.replaceVariableLocationOp(Location, SunkAddr);
-    return true;
+      AnyChange = true;
     }
-  return false;
+  }
+  return AnyChange;
 }
 
 // A llvm.dbg.value may be using a value before its definition, due to
 // optimizations in this pass and others. Scan for such dbg.values, and rescue
 // them by moving the dbg.value to immediately after the value definition.
 // FIXME: Ideally this should never be necessary, and this has the potential
 // to re-order dbg.value intrinsics.
 bool CodeGenPrepare::placeDbgValues(Function &F) {
   bool MadeChange = false;
   DominatorTree DT(F);
 
   for (BasicBlock &BB : F) {
     for (BasicBlock::iterator BI = BB.begin(), BE = BB.end(); BI != BE;) {
       Instruction *Insn = &*BI++;
       DbgValueInst *DVI = dyn_cast<DbgValueInst>(Insn);
       if (!DVI)
         continue;
 
-      Instruction *VI = dyn_cast_or_null<Instruction>(DVI->getValue());
-
-      if (!VI || VI->isTerminator())
+      SmallVector<Instruction *, 4> VIs;
+      for (Value *V : DVI->getValues())
+        if (Instruction *VI = dyn_cast_or_null<Instruction>(V))
+          VIs.push_back(VI);
+
+      // This DVI may depend on multiple instructions, complicating any
+      // potential sink. This block takes the defensive approach, opting to
+      // "undef" the DVI if it has more than one instruction and any of them do
+      // not dominate DVI.
+      for (Instruction *VI : VIs) {
+        if (VI->isTerminator())
           continue;
 
         // If VI is a phi in a block with an EHPad terminator, we can't insert
         // after it.
         if (isa<PHINode>(VI) && VI->getParent()->getTerminator()->isEHPad())
           continue;
 
         // If the defining instruction dominates the dbg.value, we do not need
         // to move the dbg.value.
         if (DT.dominates(VI, DVI))
           continue;
 
+        // If we depend on multiple instructions and any of them doesn't
+        // dominate this DVI, we probably can't salvage it: moving it to
+        // after any of the instructions could cause us to lose the others.
+        if (VIs.size() > 1) {
+          LLVM_DEBUG(
+              dbgs()
+              << "Unable to find valid location for Debug Value, undefing:\n"
+              << *DVI);
+          DVI->setUndef();
+          break;
+        }
+
         LLVM_DEBUG(dbgs() << "Moving Debug Value before :\n"
                           << *DVI << ' ' << *VI);
         DVI->removeFromParent();
         if (isa<PHINode>(VI))
           DVI->insertBefore(&*VI->getParent()->getFirstInsertionPt());
         else
           DVI->insertAfter(VI);
         MadeChange = true;
         ++NumDbgValueMoved;
       }
     }
+  }
   return MadeChange;
 }
 
 /// Scale down both weights to fit into uint32_t.
 static void scaleWeights(uint64_t &NewTrue, uint64_t &NewFalse) {
   uint64_t NewMax = (NewTrue > NewFalse) ? NewTrue : NewFalse;
   uint32_t Scale = (NewMax / std::numeric_limits<uint32_t>::max()) + 1;
   NewTrue = NewTrue / Scale;

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

void SelectionDAGBuilder::salvageUnresolvedDbgValue(DanglingDebugInfo &DDI) {
   if (handleDebugValue(V, Var, Expr, DL, InstDL, SDOrder))
     return;
 
   // Attempt to salvage back through as many instructions as possible. Bail if
   // a non-instruction is seen, such as a constant expression or global
   // variable. FIXME: Further work could recover those too.
   while (isa<Instruction>(V)) {
     Instruction &VAsInst = *cast<Instruction>(V);
-    DIExpression *NewExpr = salvageDebugInfoImpl(VAsInst, Expr, StackValue);
+    // Temporary "0", awaiting real implementation.
+    DIExpression *NewExpr = salvageDebugInfoImpl(VAsInst, Expr, StackValue, 0);
 
     // If we cannot salvage any further, and haven't yet found a suitable debug
     // expression, bail out.
     if (!NewExpr)
       break;
 
     // New value and expr now represent this debuginfo.
     V = VAsInst.getOperand(0);

llvm/lib/IR/DebugInfoMetadata.cpp

DIExpression *DIExpression::prepend(const DIExpression *Expr, uint8_t Flags,
   bool StackValue = Flags & DIExpression::StackValue;
   bool EntryValue = Flags & DIExpression::EntryValue;
 
   return prependOpcodes(Expr, Ops, StackValue, EntryValue);
 }
 
 DIExpression *DIExpression::appendOpsToArg(const DIExpression *Expr,
                                            ArrayRef<uint64_t> Ops,
-                                           uint64_t ArgNo) {
+                                           unsigned ArgNo, bool StackValue) {
   assert(Expr && "Can't add ops to this expression");
 
-  SmallVector<uint64_t, 8> NewOps;
+  // Handle non-variadic intrinsics by prepending the opcodes.
+  if (!any_of(Expr->expr_ops(),
+              [](auto Op) { return Op.getOp() == dwarf::DW_OP_LLVM_arg; })) {
+    assert(ArgNo == 0 &&
+           "Location Index must be 0 for a non-variadic expression.");
+    SmallVector<uint64_t, 8> NewOps(Ops.begin(), Ops.end());
+    return DIExpression::prependOpcodes(Expr, NewOps, StackValue);
+  }
 
+  SmallVector<uint64_t, 8> NewOps;
   for (auto Op : Expr->expr_ops()) {
     Op.appendToVector(NewOps);
     if (Op.getOp() == dwarf::DW_OP_LLVM_arg && Op.getArg(0) == ArgNo)
       NewOps.insert(NewOps.end(), Ops.begin(), Ops.end());
   }
 
   return DIExpression::get(Expr->getContext(), NewOps);
 }

llvm/lib/Target/AArch64/AArch64StackTagging.cpp

for (BasicBlock::iterator IT = BB.begin(); IT != BB.end(); ++IT) {
       Instruction *I = &*IT;
       if (auto *AI = dyn_cast<AllocaInst>(I)) {
         Allocas[AI].AI = AI;
         Allocas[AI].OldAI = AI;
         continue;
       }
 
       if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(I)) {
-        if (auto *AI =
-                dyn_cast_or_null<AllocaInst>(DVI->getVariableLocationOp(0))) {
+        for (Value *V : DVI->getVariableLocationOps())
+          if (auto *AI = dyn_cast_or_null<AllocaInst>(V))
             Allocas[AI].DbgVariableIntrinsics.push_back(DVI);
-        }
         continue;
       }
 
       auto *II = dyn_cast<IntrinsicInst>(I);
       if (II && (II->getIntrinsicID() == Intrinsic::lifetime_start ||
                  II->getIntrinsicID() == Intrinsic::lifetime_end)) {
         AllocaInst *AI = findAllocaForValue(II->getArgOperand(1));
         if (!AI) {

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

static bool TryToSinkInstruction(Instruction *I, BasicBlock *DestBlock) {
   // Also sink all related debug uses from the source basic block. Otherwise we
   // get debug use before the def. Attempt to salvage debug uses first, to
   // maximise the range variables have location for. If we cannot salvage, then
   // mark the location undef: we know it was supposed to receive a new location
   // here, but that computation has been sunk.
   SmallVector<DbgVariableIntrinsic *, 2> DbgUsers;
   findDbgUsers(DbgUsers, I);
 
-  // Update the arguments of a dbg.declare instruction, so that it
-  // does not point into a sunk instruction.
-  auto updateDbgDeclare = [&I](DbgVariableIntrinsic *DII) {
-    if (!isa<DbgDeclareInst>(DII))
-      return false;
-
-    if (isa<CastInst>(I))
-      DII->replaceVariableLocationOp(DII->getVariableLocationOp(0),
-                                     I->getOperand(0));
-    return true;
-  };
-
   SmallVector<DbgVariableIntrinsic *, 2> DIIClones;
   for (auto User : DbgUsers) {
     // A dbg.declare instruction should not be cloned, since there can only be
     // one per variable fragment. It should be left in the original place
     // because the sunk instruction is not an alloca (otherwise we could not be
     // here).
-    if (User->getParent() != SrcBlock || updateDbgDeclare(User))
+    if (User->getParent() != SrcBlock || isa<DbgDeclareInst>(User))
       continue;
 
     DIIClones.emplace_back(cast<DbgVariableIntrinsic>(User->clone()));
+    if (isa<DbgDeclareInst>(User) && isa<CastInst>(I))
+      DIIClones.back()->replaceVariableLocationOp(I, I->getOperand(0));
     LLVM_DEBUG(dbgs() << "CLONE: " << *DIIClones.back() << '\n');
   }
 
   // Perform salvaging without the clones, then sink the clones.
   if (!DIIClones.empty()) {
     salvageDebugInfoForDbgValues(*I, DbgUsers);
     for (auto &DIIClone : DIIClones) {
       DIIClone->insertBefore(&*InsertPos);

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

for (auto &Inst : BB) {
             AllocasToInstrument.push_back(AI);
           continue;
         }
 
       if (isa<ReturnInst>(Inst) || isa<ResumeInst>(Inst) ||
           isa<CleanupReturnInst>(Inst))
         RetVec.push_back(&Inst);
 
-      if (auto *DDI = dyn_cast<DbgVariableIntrinsic>(&Inst))
-        if (auto *Alloca =
-                dyn_cast_or_null<AllocaInst>(DDI->getVariableLocationOp(0)))
-          AllocaDbgMap[Alloca].push_back(DDI);
+      if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&Inst))
+        for (Value *V : DVI->getVariableLocationOps())
+          if (auto *Alloca = dyn_cast_or_null<AllocaInst>(V))
+            AllocaDbgMap[Alloca].push_back(DVI);
 
       if (InstrumentLandingPads && isa<LandingPadInst>(Inst))
         LandingPadVec.push_back(&Inst);
 
       getInterestingMemoryOperands(&Inst, OperandsToInstrument);
 
       if (MemIntrinsic *MI = dyn_cast<MemIntrinsic>(&Inst))
         IntrinToInstrument.push_back(MI);
if (Size != AlignedSize) {
       NewAI->copyMetadata(*AI);
       auto *Bitcast = new BitCastInst(NewAI, AI->getType(), "", AI);
       AI->replaceAllUsesWith(Bitcast);
       AllocaToPaddedAllocaMap[AI] = NewAI;
     }
   }
 
   if (!AllocaToPaddedAllocaMap.empty()) {
-    for (auto &BB : F)
-      for (auto &Inst : BB)
-        if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&Inst))
-          if (auto *AI =
-                  dyn_cast_or_null<AllocaInst>(DVI->getVariableLocationOp(0)))
+    for (auto &BB : F) {
+      for (auto &Inst : BB) {
+        if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&Inst)) {
+          for (Value *V : DVI->getVariableLocationOps()) {
+            if (auto *AI = dyn_cast_or_null<AllocaInst>(V)) {
               if (auto *NewAI = AllocaToPaddedAllocaMap.lookup(AI))
-              DVI->replaceVariableLocationOp(AI, NewAI);
+                DVI->replaceVariableLocationOp(V, NewAI);
+            }
+          }
+        }
+      }
       for (auto &P : AllocaToPaddedAllocaMap)
         P.first->eraseFromParent();
     }
+  }
 
   // If we split the entry block, move any allocas that were originally in the
   // entry block back into the entry block so that they aren't treated as
   // dynamic allocas.
   if (EntryIRB.GetInsertBlock() != &F.getEntryBlock()) {
     InsertPt = &*F.getEntryBlock().begin();
     for (auto II = EntryIRB.GetInsertBlock()->begin(),
               IE = EntryIRB.GetInsertBlock()->end();

llvm/lib/Transforms/Scalar/ADCE.cpp

for (Instruction &I : instructions(F)) {
       if (auto *DII = dyn_cast<DbgVariableIntrinsic>(&I)) {
         // Check if the scope of this variable location is alive.
         if (AliveScopes.count(DII->getDebugLoc()->getScope()))
           continue;
 
         // If intrinsic is pointing at a live SSA value, there may be an
         // earlier optimization bug: if we know the location of the variable,
         // why isn't the scope of the location alive?
-        if (Value *V = DII->getVariableLocationOp(0))
-          if (Instruction *II = dyn_cast<Instruction>(V))
-            if (isLive(II))
+        for (Value *V : DII->getVariableLocationOps()) {
+          if (Instruction *II = dyn_cast<Instruction>(V)) {
+            if (isLive(II)) {
               dbgs() << "Dropping debug info for " << *DII << "\n";
+              break;
+            }
+          }
+        }
       }
     }
   });
 
   // The inverse of the live set is the dead set.  These are those instructions
   // that have no side effects and do not influence the control flow or return
   // value of the function, and may therefore be deleted safely.
   // NOTE: We reuse the Worklist vector here for memory efficiency.

llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp

 }
 
 bool SpeculativeExecutionPass::considerHoistingFromTo(
     BasicBlock &FromBlock, BasicBlock &ToBlock) {
   SmallPtrSet<const Instruction *, 8> NotHoisted;
   const auto AllPrecedingUsesFromBlockHoisted = [&NotHoisted](const User *U) {
     // Debug variable has special operand to check it's not hoisted.
     if (const auto *DVI = dyn_cast<DbgVariableIntrinsic>(U)) {
-      if (const auto *I =
-              dyn_cast_or_null<Instruction>(DVI->getVariableLocationOp(0)))
+      return all_of(DVI->getVariableLocationOps(), [&NotHoisted](Value *V) {
+        if (const auto *I = dyn_cast_or_null<Instruction>(V)) {
           if (NotHoisted.count(I) == 0)
             return true;
+        }
         return false;
+      });
     }
 
     // Usially debug label instrinsic corresponds to label in LLVM IR. In these
     // cases we should not move it here.
     // TODO: Possible special processing needed to detect it is related to a
     // hoisted instruction.
     if (isa<DbgLabelInst>(U))
       return false;

llvm/lib/Transforms/Utils/BasicBlockUtils.cpp

 ///
 /// then the instruction marked with (*) can be removed. Variable "x" is already
 /// described as being mapped to the SSA value X1.
 ///
 /// Possible improvements:
 /// - Keep track of non-overlapping fragments.
 static bool removeRedundantDbgInstrsUsingForwardScan(BasicBlock *BB) {
   SmallVector<DbgValueInst *, 8> ToBeRemoved;
-  DenseMap<DebugVariable, std::pair<Value *, DIExpression *> > VariableMap;
+  DenseMap<DebugVariable, std::pair<SmallVector<Value *, 4>, DIExpression *>>
+      VariableMap;
   for (auto &I : *BB) {
     if (DbgValueInst *DVI = dyn_cast<DbgValueInst>(&I)) {
       DebugVariable Key(DVI->getVariable(),
                         NoneType(),
                         DVI->getDebugLoc()->getInlinedAt());
       auto VMI = VariableMap.find(Key);
       // Update the map if we found a new value/expression describing the
       // variable, or if the variable wasn't mapped already.
-      if (VMI == VariableMap.end() ||
-          VMI->second.first != DVI->getValue() ||
+      if (VMI == VariableMap.end() || VMI->second.first != DVI->getValues() ||
           VMI->second.second != DVI->getExpression()) {
-        VariableMap[Key] = { DVI->getValue(), DVI->getExpression() };
+        VariableMap[Key] = {DVI->getValues(), DVI->getExpression()};
         continue;
       }
       // Found an identical mapping. Remember the instruction for later removal.
       ToBeRemoved.push_back(DVI);
     }
   }
 
   for (auto &Instr : ToBeRemoved)

llvm/lib/Transforms/Utils/CodeExtractor.cpp

if (auto *DLI = dyn_cast<DbgLabelInst>(&I)) {
       DINode *&NewLabel = RemappedMetadata[OldLabel];
       if (!NewLabel)
         NewLabel = DILabel::get(Ctx, NewSP, OldLabel->getName(),
                                 OldLabel->getFile(), OldLabel->getLine());
       DLI->setArgOperand(0, MetadataAsValue::get(Ctx, NewLabel));
       continue;
     }
 
-    // If the location isn't a constant or an instruction, delete the
-    // intrinsic.
-    auto *DVI = cast<DbgVariableIntrinsic>(DII);
-    Value *Location = DVI->getVariableLocationOp(0);
+    auto IsInvalidLocation = [&NewFunc](Value *Location) {
+      // Location is invalid if...
+      // ...it isn't a constant or an instruction.
       if (!Location ||
-        (!isa<Constant>(Location) && !isa<Instruction>(Location))) {
-      DebugIntrinsicsToDelete.push_back(DVI);
-      continue;
-    }
+          (!isa<Constant>(Location) && !isa<Instruction>(Location)))
+        return true;
 
-    // If the variable location is an instruction but isn't in the new
-    // function, delete the intrinsic.
+      // ...it is an instruction but isn't in the new function.
       Instruction *LocationInst = dyn_cast<Instruction>(Location);
-    if (LocationInst && LocationInst->getFunction() != &NewFunc) {
+      return LocationInst && LocationInst->getFunction() != &NewFunc;
+    };
+    auto *DVI = cast<DbgVariableIntrinsic>(DII);
+    // If any of the used locations are invalid, delete the intrinsic.
+    if (any_of(DVI->getVariableLocationOps(), IsInvalidLocation)) {
       DebugIntrinsicsToDelete.push_back(DVI);
       continue;
     }
 
     // Point the intrinsic to a fresh variable within the new function.
     DILocalVariable *OldVar = DVI->getVariable();
     DINode *&NewVar = RemappedMetadata[OldVar];
     if (!NewVar)

llvm/lib/Transforms/Utils/Debugify.cpp

 namespace {
 /// Return true if a mis-sized diagnostic is issued for \p DVI.
 bool diagnoseMisSizedDbgValue(Module &M, DbgValueInst *DVI) {
   // The size of a dbg.value's value operand should match the size of the
   // variable it corresponds to.
   //
   // TODO: This, along with a check for non-null value operands, should be
   // promoted to verifier failures.
-  Value *V = DVI->getValue();
-  if (!V)
-    return false;
 
   // For now, don't try to interpret anything more complicated than an empty
   // DIExpression. Eventually we should try to handle OP_deref and fragments.
   if (DVI->getExpression()->getNumElements())
     return false;
 
+  auto Vs = DVI->getValues();
+  assert(Vs.size() == 1 &&
+         "expected single value for Debug Value with empty DIExpression");
+  Value *V = Vs[0];
+  if (!V)
+    return false;
+
   Type *Ty = V->getType();
   uint64_t ValueOperandSize = getAllocSizeInBits(M, Ty);
   Optional<uint64_t> DbgVarSize = DVI->getFragmentSizeInBits();
   if (!ValueOperandSize || !DbgVarSize)
     return false;
 
   bool HasBadSize = false;
   if (Ty->isIntegerTy()) {

llvm/lib/Transforms/Utils/LCSSA.cpp

for (Use *UseToRewrite : UsesToRewrite) {
       // Otherwise, do full PHI insertion.
       SSAUpdate.RewriteUse(*UseToRewrite);
     }
 
     SmallVector<DbgValueInst *, 4> DbgValues;
     llvm::findDbgValues(DbgValues, I);
 
     // Update pre-existing debug value uses that reside outside the loop.
-    auto &Ctx = I->getContext();
     for (auto DVI : DbgValues) {
       BasicBlock *UserBB = DVI->getParent();
       if (InstBB == UserBB || L->contains(UserBB))
         continue;
       // We currently only handle debug values residing in blocks that were
       // traversed while rewriting the uses. If we inserted just a single PHI,
       // we will handle all relevant debug values.
       Value *V = AddedPHIs.size() == 1 ? AddedPHIs[0]

llvm/lib/Transforms/Utils/Local.cpp

bool llvm::wouldInstructionBeTriviallyDead(Instruction *I,
   // We don't want debug info removed by anything this general, unless
   // debug info is empty.
   if (DbgDeclareInst *DDI = dyn_cast<DbgDeclareInst>(I)) {
     if (DDI->getAddress())
       return false;
     return true;
   }
   if (DbgValueInst *DVI = dyn_cast<DbgValueInst>(I)) {
-    if (DVI->getValue())
+    if (DVI->hasArgList() || DVI->getValue(0))
       return false;
     return true;
   }
   if (DbgLabelInst *DLI = dyn_cast<DbgLabelInst>(I)) {
     if (DLI->getLabel())
       return false;
     return true;
   }
static bool PhiHasDebugValue(DILocalVariable *DIVar,
                              DIExpression *DIExpr,
                              PHINode *APN) {
   // Since we can't guarantee that the original dbg.declare instrinsic
   // is removed by LowerDbgDeclare(), we need to make sure that we are
   // not inserting the same dbg.value intrinsic over and over.
   SmallVector<DbgValueInst *, 1> DbgValues;
   findDbgValues(DbgValues, APN);
   for (auto *DVI : DbgValues) {
-    assert(DVI->getValue() == APN);
+    assert(is_contained(DVI->getValues(), APN));
     if ((DVI->getVariable() == DIVar) && (DVI->getExpression() == DIExpr))
       return true;
   }
   return false;
 }
 
 /// Check if the alloc size of \p ValTy is large enough to cover the variable
 /// (or fragment of the variable) described by \p DII.
 ///
 /// This is primarily intended as a helper for the different
 /// ConvertDebugDeclareToDebugValue functions. The dbg.declare/dbg.addr that is
 /// converted describes an alloca'd variable, so we need to use the
 /// alloc size of the value when doing the comparison. E.g. an i1 value will be
 /// identified as covering an n-bit fragment, if the store size of i1 is at
 /// least n bits.
 static bool valueCoversEntireFragment(Type *ValTy, DbgVariableIntrinsic *DII) {
   const DataLayout &DL = DII->getModule()->getDataLayout();
   uint64_t ValueSize = DL.getTypeAllocSizeInBits(ValTy);
   if (auto FragmentSize = DII->getFragmentSizeInBits())
     return ValueSize >= *FragmentSize;
   // We can't always calculate the size of the DI variable (e.g. if it is a
   // VLA). Try to use the size of the alloca that the dbg intrinsic describes
   // intead.
-  if (DII->isAddressOfVariable())
-    if (auto *AI = dyn_cast_or_null<AllocaInst>(DII->getVariableLocationOp(0)))
+  if (DII->isAddressOfVariable()) {
+    // DII should have exactly 1 location when it is an address.
+    assert(DII->getNumVariableLocationOps() == 1 &&
+           "address of variable must have exactly 1 location operand.");
+    if (auto *AI =
+            dyn_cast_or_null<AllocaInst>(DII->getVariableLocationOps()[0]))
       if (auto FragmentSize = AI->getAllocationSizeInBits(DL))
         return ValueSize >= *FragmentSize;
+  }
   // Could not determine size of variable. Conservatively return false.
   return false;
 }
 
 /// Produce a DebugLoc to use for each dbg.declare/inst pair that are promoted
 /// to a dbg.value. Because no machine insts can come from debug intrinsics,
 /// only the scope and inlinedAt is significant. Zero line numbers are used in
 /// case this DebugLoc leaks into any adjacent instructions.
void llvm::insertDebugValuesForPHIs(BasicBlock *BB,
   assert(BB && "No BasicBlock to clone dbg.value(s) from.");
   if (InsertedPHIs.size() == 0)
     return;
 
   // Map existing PHI nodes to their dbg.values.
   ValueToValueMapTy DbgValueMap;
   for (auto &I : *BB) {
     if (auto DbgII = dyn_cast<DbgVariableIntrinsic>(&I)) {
-      if (auto *Loc =
-              dyn_cast_or_null<PHINode>(DbgII->getVariableLocationOp(0)))
+      for (Value *V : DbgII->getVariableLocationOps())
+        if (auto *Loc = dyn_cast_or_null<PHINode>(V))
           DbgValueMap.insert({Loc, DbgII});
     }
   }
   if (DbgValueMap.size() == 0)
     return;
 
+  // Map a pair of the destination BB and old dbg.value to the new dbg.value,
+  // so that if a dbg.value is being rewritten to use more than one of the
+  // inserted PHIs in the same destination BB, we can update the same dbg.value
+  // with all the new PHIs instead of creating one copy for each.
+  SmallDenseMap<std::pair<BasicBlock *, DbgVariableIntrinsic *>,
+                DbgVariableIntrinsic *>
+      NewDbgValueMap;
   // Then iterate through the new PHIs and look to see if they use one of the
-  // previously mapped PHIs. If so, insert a new dbg.value intrinsic that will
-  // propagate the info through the new PHI.
+  // previously mapped PHIs. If so, create a new dbg.value intrinsic that will
+  // propagate the info through the new PHI. If we use more than one new PHI in
+  // a single destination BB with the same old dbg.value, merge the updates so
+  // that we get a single new dbg.value with all the new PHIs.
   for (auto PHI : InsertedPHIs) {
     BasicBlock *Parent = PHI->getParent();
     // Avoid inserting an intrinsic into an EH block.
     if (Parent->getFirstNonPHI()->isEHPad())
       continue;
     for (auto VI : PHI->operand_values()) {
       auto V = DbgValueMap.find(VI);
       if (V != DbgValueMap.end()) {
         auto *DbgII = cast<DbgVariableIntrinsic>(V->second);
-        DbgVariableIntrinsic *NewDbgII =
-            cast<DbgVariableIntrinsic>(DbgII->clone());
+        auto NewDI = NewDbgValueMap.find({Parent, DbgII});
+        if (NewDI == NewDbgValueMap.end()) {
+          auto *NewDbgII = cast<DbgVariableIntrinsic>(DbgII->clone());
+          NewDI = NewDbgValueMap.insert({{Parent, DbgII}, NewDbgII}).first;
+        }
+        DbgVariableIntrinsic *NewDbgII = NewDI->second;
+        // If PHI contains VI as an operand more than once, we may have already
+        // replaced it in NewDbgII; confirm that it is present.
+        if (is_contained(NewDbgII->getVariableLocationOps(), VI))
           NewDbgII->replaceVariableLocationOp(VI, PHI);
+      }
+    }
+  }
+  // Insert thew new dbg.values into their destination blocks.
+  for (auto DI : NewDbgValueMap) {
+    BasicBlock *Parent = DI.first.first;
+    auto *NewDbgII = DI.second;
     auto InsertionPt = Parent->getFirstInsertionPt();
     assert(InsertionPt != Parent->end() && "Ill-formed basic block");
     NewDbgII->insertBefore(&*InsertionPt);
   }
 }
-  }
-}
 
 /// Finds all intrinsics declaring local variables as living in the memory that
 /// 'V' points to. This may include a mix of dbg.declare and
 /// dbg.addr intrinsics.
 TinyPtrVector<DbgVariableIntrinsic *> llvm::FindDbgAddrUses(Value *V) {
   // This function is hot. Check whether the value has any metadata to avoid a
   // DenseMap lookup.
   if (!V->isUsedByMetadata())
TinyPtrVector<DbgDeclareInst *> llvm::FindDbgDeclareUses(Value *V) {
   return DDIs;
 }
 
 void llvm::findDbgValues(SmallVectorImpl<DbgValueInst *> &DbgValues, Value *V) {
   // This function is hot. Check whether the value has any metadata to avoid a
   // DenseMap lookup.
   if (!V->isUsedByMetadata())
     return;
-  if (auto *L = LocalAsMetadata::getIfExists(V))
-    if (auto *MDV = MetadataAsValue::getIfExists(V->getContext(), L))
+  if (auto *L = LocalAsMetadata::getIfExists(V)) {
+    if (auto *MDV = MetadataAsValue::getIfExists(V->getContext(), L)) {
       for (User *U : MDV->users())
         if (DbgValueInst *DVI = dyn_cast<DbgValueInst>(U))
           DbgValues.push_back(DVI);
     }
+    for (Metadata *AL : L->getAllArgListUsers()) {
+      if (auto *MDV = MetadataAsValue::getIfExists(V->getContext(), AL)) {
+        for (User *U : MDV->users())
+          if (DbgValueInst *DII = dyn_cast<DbgValueInst>(U))
+            DbgValues.push_back(DII);
+      }
+    }
+  }
+}
 
 void llvm::findDbgUsers(SmallVectorImpl<DbgVariableIntrinsic *> &DbgUsers,
                         Value *V) {
   // This function is hot. Check whether the value has any metadata to avoid a
   // DenseMap lookup.
   if (!V->isUsedByMetadata())
     return;
-  if (auto *L = LocalAsMetadata::getIfExists(V))
-    if (auto *MDV = MetadataAsValue::getIfExists(V->getContext(), L))
+  if (auto *L = LocalAsMetadata::getIfExists(V)) {
+    if (auto *MDV = MetadataAsValue::getIfExists(V->getContext(), L)) {
       for (User *U : MDV->users())
         if (DbgVariableIntrinsic *DII = dyn_cast<DbgVariableIntrinsic>(U))
           DbgUsers.push_back(DII);
     }
+    for (Metadata *AL : L->getAllArgListUsers()) {
+      if (auto *MDV = MetadataAsValue::getIfExists(V->getContext(), AL)) {
+        for (User *U : MDV->users())
+          if (DbgVariableIntrinsic *DII = dyn_cast<DbgVariableIntrinsic>(U))
+            DbgUsers.push_back(DII);
+      }
+    }
+  }
+}
 
 bool llvm::replaceDbgDeclare(Value *Address, Value *NewAddress,
                              DIBuilder &Builder, uint8_t DIExprFlags,
                              int Offset) {
   auto DbgAddrs = FindDbgAddrUses(Address);
   for (DbgVariableIntrinsic *DII : DbgAddrs) {
     DebugLoc Loc = DII->getDebugLoc();
     auto *DIVar = DII->getVariable();
void llvm::salvageDebugInfoForDbgValues(
     Instruction &I, ArrayRef<DbgVariableIntrinsic *> DbgUsers) {
   bool Salvaged = false;
 
   for (auto *DII : DbgUsers) {
     // Do not add DW_OP_stack_value for DbgDeclare and DbgAddr, because they
     // are implicitly pointing out the value as a DWARF memory location
     // description.
     bool StackValue = isa<DbgValueInst>(DII);
+    auto DIILocation = DII->getVariableLocationOps();
+    assert(
+        is_contained(DIILocation, &I) &&
+        "DbgVariableIntrinsic must use salvaged instruction as its location");
+    unsigned LocNo = std::distance(DIILocation.begin(), find(DIILocation, &I));
 
     DIExpression *DIExpr =
-        salvageDebugInfoImpl(I, DII->getExpression(), StackValue);
+        salvageDebugInfoImpl(I, DII->getExpression(), StackValue, LocNo);
 
     // salvageDebugInfoImpl should fail on examining the first element of
     // DbgUsers, or none of them.
     if (!DIExpr)
       break;
 
     DII->replaceVariableLocationOp(&I, I.getOperand(0));
     DII->setExpression(DIExpr);
     LLVM_DEBUG(dbgs() << "SALVAGE: " << *DII << '\n');
     Salvaged = true;
   }
 
   if (Salvaged)
     return;
 
   for (auto *DII : DbgUsers) {
     Value *Undef = UndefValue::get(I.getType());
     DII->replaceVariableLocationOp(&I, Undef);
   }
 }
 
 DIExpression *llvm::salvageDebugInfoImpl(Instruction &I,
                                          DIExpression *SrcDIExpr,
-                                         bool WithStackValue) {
+                                         bool WithStackValue, unsigned LocNo) {
   auto &M = *I.getModule();
   auto &DL = M.getDataLayout();
 
   // Apply a vector of opcodes to the source DIExpression.
   auto doSalvage = [&](SmallVectorImpl<uint64_t> &Ops) -> DIExpression * {
     DIExpression *DIExpr = SrcDIExpr;
     if (!Ops.empty()) {
-      DIExpr = DIExpression::prependOpcodes(DIExpr, Ops, WithStackValue);
+      DIExpr = DIExpression::appendOpsToArg(DIExpr, Ops, LocNo, WithStackValue);
     }
     return DIExpr;
   };
 
   // Apply the given offset to the source DIExpression.
   auto applyOffset = [&](uint64_t Offset) -> DIExpression * {
     SmallVector<uint64_t, 8> Ops;
     DIExpression::appendOffset(Ops, Offset);

llvm/lib/Transforms/Utils/LoopRotationUtils.cpp

do {
     for (; PHINode *PN = dyn_cast<PHINode>(I); ++I)
       InsertNewValueIntoMap(ValueMap, PN,
                             PN->getIncomingValueForBlock(OrigPreheader));
 
     // For the rest of the instructions, either hoist to the OrigPreheader if
     // possible or create a clone in the OldPreHeader if not.
     Instruction *LoopEntryBranch = OrigPreheader->getTerminator();
 
-    // Record all debug intrinsics preceding LoopEntryBranch to avoid duplication.
+    // Record all debug intrinsics preceding LoopEntryBranch to avoid
+    // duplication.
     using DbgIntrinsicHash =
-      std::pair<std::pair<Value *, DILocalVariable *>, DIExpression *>;
+        std::pair<std::pair<hash_code, DILocalVariable *>, DIExpression *>;
     auto makeHash = [](DbgVariableIntrinsic *D) -> DbgIntrinsicHash {
-      return {{D->getVariableLocationOp(0), D->getVariable()},
+      auto VarLocOps = D->getVariableLocationOps();
+      return {{hash_combine_range(VarLocOps.begin(), VarLocOps.end()),
+               D->getVariable()},
               D->getExpression()};
     };
     SmallDenseSet<DbgIntrinsicHash, 8> DbgIntrinsics;
     for (auto I = std::next(OrigPreheader->rbegin()), E = OrigPreheader->rend();
          I != E; ++I) {
       if (auto *DII = dyn_cast<DbgVariableIntrinsic>(&*I))
         DbgIntrinsics.insert(makeHash(DII));
       else

llvm/unittests/IR/DebugInfoTest.cpp

)");
   Instruction &I = *M->getFunction("f")->getEntryBlock().getFirstNonPHI();
 
   // Find the dbg.value using %b.
   SmallVector<DbgValueInst *, 1> DVIs;
   findDbgValues(DVIs, &I);
 
   // Delete %b. The dbg.value should now point to undef.
   I.eraseFromParent();
-  EXPECT_TRUE(isa<UndefValue>(DVIs[0]->getValue()));
+  EXPECT_EQ(DVIs[0]->getNumVariableLocationOps(), 1);
+  EXPECT_TRUE(isa<UndefValue>(DVIs[0]->getValue(0)));
 }
 
 } // end namespace

llvm/unittests/Transforms/Utils/LocalTest.cpp

void SetUp() override {
 
     auto *GV = M->getNamedValue("f");
     ASSERT_TRUE(GV);
     F = dyn_cast<Function>(GV);
     ASSERT_TRUE(F);
   }
 
   bool doesDebugValueDescribeX(const DbgValueInst &DI) {
-    const auto &CI = *cast<ConstantInt>(DI.getValue());
+    if (DI.getNumVariableLocationOps() != 1)
+      return false;
+    const auto &CI = *cast<ConstantInt>(DI.getValue(0));
     if (CI.isZero())
       return DI.getExpression()->getElements().equals(
           {dwarf::DW_OP_plus_uconst, 1, dwarf::DW_OP_stack_value});
     else if (CI.isOneValue())
       return DI.getExpression()->getElements().empty();
     return false;
   }
 
   bool doesDebugValueDescribeY(const DbgValueInst &DI) {
-    const auto &CI = *cast<ConstantInt>(DI.getValue());
+    if (DI.getNumVariableLocationOps() != 1)
+      return false;
+    const auto &CI = *cast<ConstantInt>(DI.getValues(0));
     if (CI.isZero())
       return DI.getExpression()->getElements().equals(
           {dwarf::DW_OP_plus_uconst, 1, dwarf::DW_OP_plus_uconst, 2,
            dwarf::DW_OP_stack_value});
     else if (CI.isOneValue())
       return DI.getExpression()->getElements().equals(
           {dwarf::DW_OP_plus_uconst, 2, dwarf::DW_OP_stack_value});
     return false;
TEST(Local, ReplaceAllDbgUsesWith) {
   EXPECT_TRUE(any_of(DDbgVals, [](DbgVariableIntrinsic *DII) {
     return isa<DbgDeclareInst>(DII);
   }));
 
   // Introduce a use-before-def. Check that the dbg.value for %a is salvaged.
   EXPECT_TRUE(replaceAllDbgUsesWith(A, F_, F_, DT));
 
   auto *ADbgVal = cast<DbgValueInst>(A.getNextNode());
-  EXPECT_EQ(ConstantInt::get(A.getType(), 0),
-            ADbgVal->getVariableLocationOp(0));
+  EXPECT_EQ(ADbgVal->getNumVariableLocationOps(), 1);
+  EXPECT_EQ(ConstantInt::get(A.getType(), 0), ADbgVal->getValues(0));
 
   // Introduce a use-before-def. Check that the dbg.values for %f become undef.
   EXPECT_TRUE(replaceAllDbgUsesWith(F_, G, G, DT));
 
   auto *FDbgVal = cast<DbgValueInst>(F_.getNextNode());
-  EXPECT_TRUE(isa<UndefValue>(FDbgVal->getVariableLocationOp(0)));
+  EXPECT_EQ(FDbgVal->getNumVariableLocationOps(), 1);
+  EXPECT_TRUE(FDbgVal->isUndef());
 
   SmallVector<DbgValueInst *, 1> FDbgVals;
   findDbgValues(FDbgVals, &F_);
   EXPECT_EQ(0U, FDbgVals.size());
 
   // Simulate i32 -> i64 conversion to test sign-extension. Here are some
   // interesting cases to handle:
   //  1) debug user has empty DIExpression