[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

6

That's how :-)

7

nit: remove redundant {}?

[StephenTozer]

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

7

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

30

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

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

1

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

llvm/lib/Transforms/Utils/CodeExtractor.cpp

0

Can we refactor this comments? :)

llvm/lib/Transforms/Utils/Debugify.cpp

3

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

llvm/lib/Transforms/Utils/LCSSA.cpp

226 ↗

(On Diff #294062)

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

2–1

clang-format?

[StephenTozer]

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

1

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/IntrinsicInst.h

public:
   /// Get the locations corresponding to the variable referenced by the debug
   /// info intrinsic.  Depending on the intrinsic, this could be the
   /// variable's value or its address.
   iterator_range<location_op_iterator> location_ops() const;
 
   Value *getVariableLocationOp(unsigned OpIdx) const;
 
   void replaceVariableLocationOp(Value *OldValue, Value *NewValue);
+  void replaceVariableLocationOp(unsigned OpIdx, Value *NewValue);
 
   void setVariable(DILocalVariable *NewVar) {
     setArgOperand(1, MetadataAsValue::get(NewVar->getContext(), NewVar));
   }
 
   void setExpression(DIExpression *NewExpr) {
     setArgOperand(2, MetadataAsValue::get(NewExpr->getContext(), NewExpr));
   }

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

 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Analysis/BlockFrequencyInfo.h"
 #include "llvm/Analysis/BranchProbabilityInfo.h"
 #include "llvm/Analysis/ConstantFolding.h"
 #include "llvm/Analysis/InstructionSimplify.h"
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/Analysis/MemoryBuiltins.h"
 #include "llvm/Analysis/ProfileSummaryInfo.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/Analysis/VectorUtils.h"
 #include "llvm/CodeGen/Analysis.h"
 #include "llvm/CodeGen/ISDOpcodes.h"
 #include "llvm/CodeGen/SelectionDAGNodes.h"
 #include "llvm/CodeGen/TargetLowering.h"
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;
 
+    /// Keep track of the new value so that we can undo it by replacing
+    /// instances of the new value with the original value.
+    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 {
       LLVM_DEBUG(dbgs() << "Undo: UsersReplacer: " << *Inst << "\n");
       for (InstructionAndIdx &Use : OriginalUses)
         Use.Inst->setOperand(Use.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

 //===- SelectionDAGBuilder.cpp - Selection-DAG building -------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This implements routines for translating from LLVM IR into SelectionDAG IR.
void SelectionDAGBuilder::salvageUnresolvedDbgValue(DanglingDebugInfo &DDI) {
   if (handleDebugValue(V, Var, Expr, DL, InstDL, SDOrder, /*IsVariadic=*/false))
     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);
void SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I,
   }
   case Intrinsic::dbg_value: {
     const DbgValueInst &DI = cast<DbgValueInst>(I);
     assert(DI.getVariable() && "Missing variable");
 
     DILocalVariable *Variable = DI.getVariable();
     DIExpression *Expression = DI.getExpression();
     dropDanglingDebugInfo(Variable, Expression);
-    SmallVector<Value *> Values(DI.getValues());
+    SmallVector<Value *, 4> Values(DI.getValues());
     if (Values.empty())
       return;
 
     if (std::count(Values.begin(), Values.end(), nullptr))
       return;
 
     bool IsVariadic = DI.hasArgList();
     if (!handleDebugValue(Values, Variable, Expression, dl, DI.getDebugLoc(),

llvm/lib/IR/IntrinsicInst.cpp

void DbgVariableIntrinsic::replaceVariableLocationOp(Value *OldValue,
   }
   SmallVector<ValueAsMetadata *, 4> MDs;
   ValueAsMetadata *NewOperand = getAsMetadata(NewValue);
   for (auto *VMD : Locations)
     MDs.push_back(VMD == *OldIt ? NewOperand : getAsMetadata(VMD));
   setArgOperand(
       0, MetadataAsValue::get(getContext(), DIArgList::get(getContext(), MDs)));
 }
+void DbgVariableIntrinsic::replaceVariableLocationOp(unsigned OpIdx,
+                                                     Value *NewValue) {
+  assert(OpIdx < getNumVariableLocationOps() && "Invalid Operand Index");
+  if (!hasArgList()) {
+    Value *NewOperand = isa<MetadataAsValue>(NewValue)
+                            ? NewValue
+                            : MetadataAsValue::get(
+                                  getContext(), ValueAsMetadata::get(NewValue));
+    return setArgOperand(0, NewOperand);
+  }
+  SmallVector<ValueAsMetadata *, 4> MDs;
+  ValueAsMetadata *NewOperand = getAsMetadata(NewValue);
+  for (unsigned Idx = 0; Idx < getNumVariableLocationOps(); ++Idx)
+    MDs.push_back(Idx == OpIdx ? NewOperand
+                               : getAsMetadata(getVariableLocationOp(Idx)));
+  setArgOperand(
+      0, MetadataAsValue::get(getContext(), DIArgList::get(getContext(), MDs)));
+}
 
 Optional<uint64_t> DbgVariableIntrinsic::getFragmentSizeInBits() const {
   if (auto Fragment = getExpression()->getFragmentInfo())
     return Fragment->SizeInBits;
   return getVariable()->getSizeInBits();
 }
 
 int llvm::Intrinsic::lookupLLVMIntrinsicByName(ArrayRef<const char *> NameTable,

llvm/lib/IR/User.cpp

void User::replaceUsesOfWith(Value *From, Value *To) {
 
   for (unsigned i = 0, E = getNumOperands(); i != E; ++i)
     if (getOperand(i) == From) {  // Is This operand is pointing to oldval?
       // The side effects of this setOperand call include linking to
       // "To", adding "this" to the uses list of To, and
       // most importantly, removing "this" from the use list of "From".
       setOperand(i, To);
     }
+  if (auto DVI = dyn_cast_or_null<DbgVariableIntrinsic>(this)) {
+    if (is_contained(DVI->location_ops(), From))
+      DVI->replaceVariableLocationOp(From, To);
+  }
 }
 
 //===----------------------------------------------------------------------===//
 //                         User allocHungoffUses Implementation
 //===----------------------------------------------------------------------===//
 
 void User::allocHungoffUses(unsigned N, bool IsPhi) {
   assert(HasHungOffUses && "alloc must have hung off uses");

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->location_ops())
+          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/Coroutines/CoroFrame.cpp

if (auto *LdInst = dyn_cast<LoadInst>(Storage)) {
       // effectively drops the *last* DW_OP_deref in the expression.
       if (!OutermostLoad)
         Expr = DIExpression::prepend(Expr, DIExpression::DerefBefore);
       OutermostLoad = false;
     } else if (auto *StInst = dyn_cast<StoreInst>(Storage)) {
       Storage = StInst->getOperand(0);
     } else if (auto *GEPInst = dyn_cast<GetElementPtrInst>(Storage)) {
       Expr = llvm::salvageDebugInfoImpl(*GEPInst, Expr,
-                                        /*WithStackValue=*/false);
+                                        /*WithStackValue=*/false, 0);
       if (!Expr)
         return;
       Storage = GEPInst->getOperand(0);
     } else if (auto *BCInst = dyn_cast<llvm::BitCastInst>(Storage))
       Storage = BCInst->getOperand(0);
     else
       break;
   }

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

static bool TryToSinkInstruction(Instruction *I, BasicBlock *DestBlock) {
   // last appearing debug intrinsic for each given variable.
   SmallVector<DbgVariableIntrinsic *, 2> DbgUsersToSink;
   for (DbgVariableIntrinsic *DVI : DbgUsers)
     if (DVI->getParent() == SrcBlock)
       DbgUsersToSink.push_back(DVI);
   llvm::sort(DbgUsersToSink,
              [](auto *A, auto *B) { return B->comesBefore(A); });
 
-  // Update the arguments of a dbg.declare instruction, so that it
-  // does not point into a sunk instruction.
-  auto updateDbgDeclare = [](DbgVariableIntrinsic *DII) {
-    if (!isa<DbgDeclareInst>(DII))
-      return false;
-
-    return true;
-  };
-
   SmallVector<DbgVariableIntrinsic *, 2> DIIClones;
   SmallSet<DebugVariable, 4> SunkVariables;
   for (auto User : DbgUsersToSink) {
     // 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 (updateDbgDeclare(User))
+    if (isa<DbgDeclareInst>(User))
       continue;
 
     DebugVariable DbgUserVariable =
         DebugVariable(User->getVariable(), User->getExpression(),
                       User->getDebugLoc()->getInlinedAt());
 
     if (!SunkVariables.insert(DbgUserVariable).second)
       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);
     // The clones are in reverse order of original appearance, reverse again to
     // maintain the original order.

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

 //===- HWAddressSanitizer.cpp - detector of uninitialized reads -------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 /// \file
 /// This file is a part of HWAddressSanitizer, an address sanity checker
 /// based on tagged addressing.
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/Instrumentation/HWAddressSanitizer.h"
 #include "llvm/ADT/MapVector.h"
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->location_ops())
+          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->location_ops()) {
+            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()) {

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->location_ops()) {
+          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/LoopStrengthReduce.cpp

void LoopStrengthReduce::getAnalysisUsage(AnalysisUsage &AU) const {
   // twice, since LoopSimplify was invalidated by running ScalarEvolution.
   AU.addRequiredID(LoopSimplifyID);
   AU.addRequired<IVUsersWrapperPass>();
   AU.addPreserved<IVUsersWrapperPass>();
   AU.addRequired<TargetTransformInfoWrapperPass>();
   AU.addPreserved<MemorySSAWrapperPass>();
 }
 
-using EqualValues = SmallVector<std::tuple<WeakVH, int64_t, DIExpression *>, 4>;
-using EqualValuesMap = DenseMap<DbgValueInst *, EqualValues>;
+using EqualValues = SmallVector<std::tuple<WeakVH, int64_t>, 4>;
+using EqualValuesMap =
+    DenseMap<std::pair<DbgValueInst *, unsigned>, EqualValues>;
+using ExpressionMap = DenseMap<DbgValueInst *, DIExpression *>;
 
 static void DbgGatherEqualValues(Loop *L, ScalarEvolution &SE,
-                                 EqualValuesMap &DbgValueToEqualSet) {
+                                 EqualValuesMap &DbgValueToEqualSet,
+                                 ExpressionMap &DbgValueToExpression) {
   for (auto &B : L->getBlocks()) {
     for (auto &I : *B) {
       auto DVI = dyn_cast<DbgValueInst>(&I);
       if (!DVI)
         continue;
-      auto V = DVI->getVariableLocationOp(0);
+      for (unsigned Idx = 0; Idx < DVI->getNumVariableLocationOps(); ++Idx) {
+        // TODO: We can duplicate results if the same arg appears more than
+        // once.
+        Value *V = DVI->getVariableLocationOp(Idx);
         if (!V || !SE.isSCEVable(V->getType()))
           continue;
         auto DbgValueSCEV = SE.getSCEV(V);
         EqualValues EqSet;
         for (PHINode &Phi : L->getHeader()->phis()) {
           if (V->getType() != Phi.getType())
             continue;
           if (!SE.isSCEVable(Phi.getType()))
             continue;
           auto PhiSCEV = SE.getSCEV(&Phi);
           Optional<APInt> Offset =
-                SE.computeConstantDifference(DbgValueSCEV, PhiSCEV);
           if (Offset && Offset->getMinSignedBits() <= 64)
-          EqSet.emplace_back(std::make_tuple(
-              &Phi, Offset.getValue().getSExtValue(), DVI->getExpression()));
+            EqSet.emplace_back(
+                std::make_tuple(&Phi, Offset.getValue().getSExtValue()));
+        }
+        DbgValueToEqualSet[{DVI, Idx}] = std::move(EqSet);
+        DbgValueToExpression[DVI] = DVI->getExpression();
       }
-      DbgValueToEqualSet[DVI] = std::move(EqSet);
     }
   }
 }
 
-static void DbgApplyEqualValues(EqualValuesMap &DbgValueToEqualSet) {
+static void DbgApplyEqualValues(EqualValuesMap &DbgValueToEqualSet,
+                                ExpressionMap &DbgValueToExpression) {
   for (auto A : DbgValueToEqualSet) {
-    auto DVI = A.first;
+    auto DVI = A.first.first;
+    auto Idx = A.first.second;
     // Only update those that are now undef.
-    if (!isa_and_nonnull<UndefValue>(DVI->getVariableLocationOp(0)))
+    if (!isa_and_nonnull<UndefValue>(DVI->getVariableLocationOp(Idx)))
       continue;
     for (auto EV : A.second) {
-      auto V = std::get<WeakVH>(EV);
-      if (!V)
+      auto EVHandle = std::get<WeakVH>(EV);
+      if (!EVHandle)
         continue;
-      auto DbgDIExpr = std::get<DIExpression *>(EV);
+      // The dbg.value may have had its value changed by LSR; refresh it from
+      // the map, but continue to update the mapped expression as it may be
+      // updated multiple times in this function.
+      auto DbgDIExpr = DbgValueToExpression[DVI];
       auto Offset = std::get<int64_t>(EV);
-      DVI->replaceVariableLocationOp(DVI->getVariableLocationOp(0), V);
+      DVI->replaceVariableLocationOp(Idx, EVHandle);
       if (Offset) {
         SmallVector<uint64_t, 8> Ops;
         DIExpression::appendOffset(Ops, Offset);
-        DbgDIExpr = DIExpression::prependOpcodes(DbgDIExpr, Ops, true);
+        DbgDIExpr = DIExpression::appendOpsToArg(DbgDIExpr, Ops, Idx, true);
       }
       DVI->setExpression(DbgDIExpr);
+      DbgValueToExpression[DVI] = DbgDIExpr;
       break;
     }
   }
 }
 
 static bool ReduceLoopStrength(Loop *L, IVUsers &IU, ScalarEvolution &SE,
                                DominatorTree &DT, LoopInfo &LI,
                                const TargetTransformInfo &TTI,
                                AssumptionCache &AC, TargetLibraryInfo &TLI,
                                MemorySSA *MSSA) {
 
   bool Changed = false;
   std::unique_ptr<MemorySSAUpdater> MSSAU;
   if (MSSA)
     MSSAU = std::make_unique<MemorySSAUpdater>(MSSA);
 
   // Run the main LSR transformation.
   Changed |=
       LSRInstance(L, IU, SE, DT, LI, TTI, AC, TLI, MSSAU.get()).getChanged();
 
   // Debug preservation - before we start removing anything create equivalence
   // sets for the llvm.dbg.value intrinsics.
   EqualValuesMap DbgValueToEqualSet;
-  DbgGatherEqualValues(L, SE, DbgValueToEqualSet);
+  ExpressionMap DbgValueToExpression;
+  DbgGatherEqualValues(L, SE, DbgValueToEqualSet, DbgValueToExpression);
 
   // Remove any extra phis created by processing inner loops.
   Changed |= DeleteDeadPHIs(L->getHeader(), &TLI, MSSAU.get());
   if (EnablePhiElim && L->isLoopSimplifyForm()) {
     SmallVector<WeakTrackingVH, 16> DeadInsts;
     const DataLayout &DL = L->getHeader()->getModule()->getDataLayout();
     SCEVExpander Rewriter(SE, DL, "lsr", false);
 #ifndef NDEBUG
     Rewriter.setDebugType(DEBUG_TYPE);
 #endif
     unsigned numFolded = Rewriter.replaceCongruentIVs(L, &DT, DeadInsts, &TTI);
     if (numFolded) {
       Changed = true;
       RecursivelyDeleteTriviallyDeadInstructionsPermissive(DeadInsts, &TLI,
                                                            MSSAU.get());
       DeleteDeadPHIs(L->getHeader(), &TLI, MSSAU.get());
     }
   }
 
-  DbgApplyEqualValues(DbgValueToEqualSet);
+  DbgApplyEqualValues(DbgValueToEqualSet, DbgValueToExpression);
 
   return Changed;
 }
 
 bool LoopStrengthReduce::runOnLoop(Loop *L, LPPassManager & /*LPM*/) {
   if (skipLoop(L))
     return false;
 

llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp

 //===- SpeculativeExecution.cpp ---------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This pass hoists instructions to enable speculative execution on

 }
 
 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->location_ops(), [&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() ||
+      SmallVector<Value *, 4> Values(DVI->getValues());
+      if (VMI == VariableMap.end() || VMI->second.first != Values ||
           VMI->second.second != DVI->getExpression()) {
-        VariableMap[Key] = { DVI->getValue(), DVI->getExpression() };
+        VariableMap[Key] = {Values, 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

 //===- CodeExtractor.cpp - Pull code region into a new function -----------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
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, or is an
+      // instruction but isn't in the new function.
       if (!Location ||
-        (!isa<Constant>(Location) && !isa<Instruction>(Location))) {
-      DebugIntrinsicsToDelete.push_back(DVI);
-      continue;
-    }
-
-    // If the variable location is an instruction but isn't in the new
-    // function, delete the intrinsic.
+          (!isa<Constant>(Location) && !isa<Instruction>(Location)))
+        return true;
       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->location_ops(), 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

 //===- Debugify.cpp - Check debug info preservation in optimizations ------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file In the `synthetic` mode, the `-debugify` attaches synthetic debug info
 /// to everything. It can be used to create targeted tests for debug info
 /// preservation. In addition, when using the `original` mode, it can check

 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;
 
+  Value *V = DVI->getVariableLocationOp(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/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.
static bool valueCoversEntireFragment(Type *ValTy, DbgVariableIntrinsic *DII) {
   if (Optional<uint64_t> FragmentSize = DII->getFragmentSizeInBits()) {
     assert(!ValueSize.isScalable() &&
            "Fragments don't work on scalable types.");
     return ValueSize.getFixedSize() >= *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->getVariableLocationOp(0))) {
       if (Optional<TypeSize> FragmentSize = AI->getAllocationSizeInBits(DL)) {
         assert(ValueSize.isScalable() == FragmentSize->isScalable() &&
                "Both sizes should agree on the scalable flag.");
         return TypeSize::isKnownGE(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->location_ops())
+        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
+        // replaced it in NewDbgII; confirm that it is present.
+        if (is_contained(NewDbgII->location_ops(), VI))
           NewDbgII->replaceVariableLocationOp(VI, PHI);
-        auto InsertionPt = Parent->getFirstInsertionPt();
-        assert(InsertionPt != Parent->end() && "Ill-formed basic block");
-        NewDbgII->insertBefore(&*InsertionPt);
-      }
-    }
-  }
-}
+      }
+    }
+  }
+  // Insert thew new dbg.values into their destination blocks.
+  for (auto DI : NewDbgValueMap) {
+    BasicBlock *Parent = DI.first.first;
+    auto *NewDbgII = DI.second;
 
 /// 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))
+  // TODO: If this value appears multiple times in a DIArgList, we should still
+  // only add the owning DbgValueInst once; use this set to track ArgListUsers.
+  // This behaviour can be removed when we can automatically remove duplicates.
+  SmallPtrSet<DbgValueInst *, 4> EncounteredDbgValues;
+  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 *DVI = dyn_cast<DbgValueInst>(U))
+            if (EncounteredDbgValues.insert(DVI).second)
               DbgValues.push_back(DVI);
       }
+    }
+  }
+}
 
 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))
+  // TODO: If this value appears multiple times in a DIArgList, we should still
+  // only add the owning DbgValueInst once; use this set to track ArgListUsers.
+  // This behaviour can be removed when we can automatically remove duplicates.
+  SmallPtrSet<DbgVariableIntrinsic *, 4> EncounteredDbgValues;
+  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))
+            if (EncounteredDbgValues.insert(DII).second)
+              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->location_ops();
+    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->location_ops();
+      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(), 1u);
+  EXPECT_TRUE(isa<UndefValue>(DVIs[0]->getValue(0)));
 }
 
 TEST(DIBuilder, CreateFortranArrayTypeWithAttributes) {
   LLVMContext Ctx;
   std::unique_ptr<Module> M(new Module("MyModule", Ctx));
   DIBuilder DIB(*M);
 
   DISubrange *Subrange = DIB.getOrCreateSubrange(1,1);