[llvm] 855752e - Revert [Debuginfo][LSR] Add salvaging variadic dbg.value intrinsics[2/2]

Chris Jackson via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 27 05:06:57 PDT 2022


Author: Chris Jackson
Date: 2022-04-27T13:06:03+01:00
New Revision: 855752e563ec4d05677c63db3fe3418c2ab62705

URL: https://github.com/llvm/llvm-project/commit/855752e563ec4d05677c63db3fe3418c2ab62705
DIFF: https://github.com/llvm/llvm-project/commit/855752e563ec4d05677c63db3fe3418c2ab62705.diff

LOG: Revert [Debuginfo][LSR] Add salvaging variadic dbg.value intrinsics[2/2]

This reverts commit 8f550368b169b0a3d4ebc3e65efec49ec1e0798a.

Added: 
    

Modified: 
    llvm/include/llvm/Analysis/ScalarEvolution.h
    llvm/lib/Analysis/ScalarEvolution.cpp
    llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp

Removed: 
    llvm/test/Transforms/LoopStrengthReduce/debuginfo-scev-salvage-5.ll


################################################################################
diff  --git a/llvm/include/llvm/Analysis/ScalarEvolution.h b/llvm/include/llvm/Analysis/ScalarEvolution.h
index 950a0d7badcf..7dbe59513f24 100644
--- a/llvm/include/llvm/Analysis/ScalarEvolution.h
+++ b/llvm/include/llvm/Analysis/ScalarEvolution.h
@@ -545,10 +545,6 @@ class ScalarEvolution {
   /// Return true if the SCEV expression contains an undef value.
   bool containsUndefs(const SCEV *S) const;
 
-  /// Return true if the SCEV expression contains a Value that has been
-  /// optimised out and is now a nullptr.
-  bool containsErasedValue(const SCEV *S) const;
-
   /// Return a SCEV expression for the full generality of the specified
   /// expression.
   const SCEV *getSCEV(Value *V);

diff  --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index 42c87359a327..451ebeab06bf 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -12688,15 +12688,6 @@ bool ScalarEvolution::containsUndefs(const SCEV *S) const {
   });
 }
 
-// Return true when S contains a value that is a nullptr.
-bool ScalarEvolution::containsErasedValue(const SCEV *S) const {
-  return SCEVExprContains(S, [](const SCEV *S) {
-    if (const auto *SU = dyn_cast<SCEVUnknown>(S))
-      return SU->getValue() == nullptr;
-    return false;
-  });
-}
-
 /// Return the size of an element read or written by Inst.
 const SCEV *ScalarEvolution::getElementSize(Instruction *Inst) {
   Type *Ty;

diff  --git a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
index 3b7a399c6dcf..cb6bd44861e6 100644
--- a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
@@ -5912,57 +5912,41 @@ void LoopStrengthReduce::getAnalysisUsage(AnalysisUsage &AU) const {
 }
 
 namespace {
-
-/// Enables more convenient iteration over a DWARF expression vector.
-static iterator_range<llvm::DIExpression::expr_op_iterator>
-ToDwarfOpIter(SmallVectorImpl<uint64_t> &Expr) {
-  llvm::DIExpression::expr_op_iterator Begin =
-      llvm::DIExpression::expr_op_iterator(Expr.begin());
-  llvm::DIExpression::expr_op_iterator End =
-      llvm::DIExpression::expr_op_iterator(Expr.end());
-  return {Begin, End};
-}
-
 struct SCEVDbgValueBuilder {
   SCEVDbgValueBuilder() = default;
   SCEVDbgValueBuilder(const SCEVDbgValueBuilder &Base) { clone(Base); }
 
   void clone(const SCEVDbgValueBuilder &Base) {
-    LocationOps = Base.LocationOps;
+    Values = Base.Values;
     Expr = Base.Expr;
   }
 
-  void clear() {
-    LocationOps.clear();
-    Expr.clear();
-  }
-
   /// The DIExpression as we translate the SCEV.
   SmallVector<uint64_t, 6> Expr;
   /// The location ops of the DIExpression.
-  SmallVector<Value *, 2> LocationOps;
+  SmallVector<Value *, 2> Values;
 
   void pushOperator(uint64_t Op) { Expr.push_back(Op); }
   void pushUInt(uint64_t Operand) { Expr.push_back(Operand); }
 
   /// Add a DW_OP_LLVM_arg to the expression, followed by the index of the value
   /// in the set of values referenced by the expression.
-  void pushLocation(llvm::Value *V) {
+  void pushValue(llvm::Value *V) {
     Expr.push_back(llvm::dwarf::DW_OP_LLVM_arg);
-    auto *It = std::find(LocationOps.begin(), LocationOps.end(), V);
+    auto *It = std::find(Values.begin(), Values.end(), V);
     unsigned ArgIndex = 0;
-    if (It != LocationOps.end()) {
-      ArgIndex = std::distance(LocationOps.begin(), It);
+    if (It != Values.end()) {
+      ArgIndex = std::distance(Values.begin(), It);
     } else {
-      ArgIndex = LocationOps.size();
-      LocationOps.push_back(V);
+      ArgIndex = Values.size();
+      Values.push_back(V);
     }
     Expr.push_back(ArgIndex);
   }
 
   void pushValue(const SCEVUnknown *U) {
     llvm::Value *V = cast<SCEVUnknown>(U)->getValue();
-    pushLocation(V);
+    pushValue(V);
   }
 
   bool pushConst(const SCEVConstant *C) {
@@ -5973,12 +5957,6 @@ struct SCEVDbgValueBuilder {
     return true;
   }
 
-  // Iterating the expression as DWARF ops is convenient when updating
-  // DWARF_OP_LLVM_args.
-  iterator_range<llvm::DIExpression::expr_op_iterator> expr_ops() {
-    return ToDwarfOpIter(Expr);
-  }
-
   /// Several SCEV types are sequences of the same arithmetic operator applied
   /// to constants and values that may be extended or truncated.
   bool pushArithmeticExpr(const llvm::SCEVCommutativeExpr *CommExpr,
@@ -6020,7 +5998,7 @@ struct SCEVDbgValueBuilder {
     } else if (const SCEVUnknown *U = dyn_cast<SCEVUnknown>(S)) {
       if (!U->getValue())
         return false;
-      pushLocation(U->getValue());
+      pushValue(U->getValue());
 
     } else if (const SCEVMulExpr *MulRec = dyn_cast<SCEVMulExpr>(S)) {
       Success &= pushArithmeticExpr(MulRec, llvm::dwarf::DW_OP_mul);
@@ -6101,7 +6079,7 @@ struct SCEVDbgValueBuilder {
 
   /// Create an expression that is an offset from a value (usually the IV).
   void createOffsetExpr(int64_t Offset, Value *OffsetValue) {
-    pushLocation(OffsetValue);
+    pushValue(OffsetValue);
     DIExpression::appendOffset(Expr, Offset);
     LLVM_DEBUG(
         dbgs() << "scev-salvage: Generated IV offset expression. Offset: "
@@ -6110,8 +6088,7 @@ struct SCEVDbgValueBuilder {
 
   /// Combine a translation of the SCEV and the IV to create an expression that
   /// recovers a location's value.
-  /// returns true if an expression was created.
-  bool createIterCountExpr(const SCEV *S,
+  void createIterCountExpr(const SCEV *S,
                            const SCEVDbgValueBuilder &IterationCount,
                            ScalarEvolution &SE) {
     // SCEVs for SSA values are most frquently of the form
@@ -6120,25 +6097,22 @@ struct SCEVDbgValueBuilder {
     // SCEVs have not been observed to result in debuginfo-lossy optimisations,
     // so its not expected this point will be reached.
     if (!isa<SCEVAddRecExpr>(S))
-      return false;
+      return;
 
-    LLVM_DEBUG(dbgs() << "scev-salvage: Location to salvage SCEV: " << *S
-                      << '\n');
+    LLVM_DEBUG(dbgs() << "scev-salvage: Value to salvage SCEV: " << *S << '\n');
 
     const auto *Rec = cast<SCEVAddRecExpr>(S);
     if (!Rec->isAffine())
-      return false;
+      return;
 
     if (S->getExpressionSize() > MaxSCEVSalvageExpressionSize)
-      return false;
+      return;
 
     // Initialise a new builder with the iteration count expression. In
     // combination with the value's SCEV this enables recovery.
     clone(IterationCount);
     if (!SCEVToValueExpr(*Rec, SE))
-      return false;
-
-    return true;
+      return;
   }
 
   /// Convert a SCEV of a value to a DIExpression that is pushed onto the
@@ -6170,184 +6144,46 @@ struct SCEVDbgValueBuilder {
     }
     return true;
   }
-
-  // Append the current expression and locations to a location list and an
-  // expression list. Modify the DW_OP_LLVM_arg indexes to account for
-  // the locations already present in the destination list.
-  void appendToVectors(SmallVectorImpl<uint64_t> &DestExpr,
-                       SmallVectorImpl<Value *> &DestLocations) {
-    assert(!DestLocations.empty() &&
-           "Expected the locations vector to contain the IV");
-    // The DWARF_OP_LLVM_arg arguments of the expression being appended must be
-    // modified to account for the locations already in the destination vector.
-    // All builders contain the IV as the first location op.
-    assert(!LocationOps.empty() &&
-           "Expected the location ops to contain the IV.");
-    // DestIndexMap[n] contains the index in DestLocations for the nth
-    // location in this SCEVDbgValueBuilder.
-    SmallVector<uint64_t, 2> DestIndexMap;
-    for (const auto &Op : LocationOps) {
-      auto It = find(DestLocations, Op);
-      if (It != DestLocations.end()) {
-        // Location already exists in DestLocations, reuse existing ArgIndex.
-        DestIndexMap.push_back(std::distance(DestLocations.begin(), It));
-        continue;
-      }
-      // Location is not in DestLocations, add it.
-      DestIndexMap.push_back(DestLocations.size());
-      DestLocations.push_back(Op);
-    }
-
-    for (const auto &Op : expr_ops()) {
-      if (Op.getOp() != dwarf::DW_OP_LLVM_arg) {
-        Op.appendToVector(DestExpr);
-        continue;
-      } 
-
-      DestExpr.push_back(dwarf::DW_OP_LLVM_arg);
-      // `DW_OP_LLVM_arg n` represents the nth LocationOp in this SCEV,
-      // DestIndexMap[n] contains its new index in DestLocations.
-      uint64_t NewIndex = DestIndexMap[Op.getArg(0)];
-      DestExpr.push_back(NewIndex);
-    }
-  }
 };
 
 /// Holds all the required data to salvage a dbg.value using the pre-LSR SCEVs
 /// and DIExpression.
 struct DVIRecoveryRec {
-  DVIRecoveryRec(DbgValueInst *DbgValue)
-      : DVI(DbgValue), Expr(DbgValue->getExpression()),
-        HadLocationArgList(false) {}
-
   DbgValueInst *DVI;
   DIExpression *Expr;
-  bool HadLocationArgList;
-  SmallVector<WeakVH, 2> LocationOps;
-  SmallVector<const llvm::SCEV *, 2> SCEVs;
-  SmallVector<std::unique_ptr<SCEVDbgValueBuilder>, 2> RecoveryExprs;
-
-  void clear() {
-    for (auto &RE : RecoveryExprs)
-      RE.reset();
-    RecoveryExprs.clear();
-  }
-
-  ~DVIRecoveryRec() { clear(); }
+  Metadata *LocationOp;
+  const llvm::SCEV *SCEV;
 };
 } // namespace
 
-/// Returns the total number of DW_OP_llvm_arg operands in the expression.
-/// This helps in determining if a DIArglist is necessary or can be omitted from
-/// the dbg.value.
-static unsigned numLLVMArgOps(SmallVectorImpl<uint64_t> &Expr) {
-  auto expr_ops = ToDwarfOpIter(Expr);
-  unsigned Count = 0;
-  for (auto Op : expr_ops)
-    if (Op.getOp() == dwarf::DW_OP_LLVM_arg)
-      Count++;
-  return Count;
-}
-
-/// Overwrites DVI with the location and Ops as the DIExpression. This will
-/// create an invalid expression if Ops has any dwarf::DW_OP_llvm_arg operands,
-/// because a DIArglist is not created for the first argument of the dbg.value.
-static void updateDVIWithLocation(DbgValueInst &DVI, Value *Location,
-                                  SmallVectorImpl<uint64_t> &Ops) {
-  assert(
-      numLLVMArgOps(Ops) == 0 &&
-      "Expected expression that does not contain any DW_OP_llvm_arg operands.");
-  DVI.setRawLocation(ValueAsMetadata::get(Location));
-  DVI.setExpression(DIExpression::get(DVI.getContext(), Ops));
-}
-
-/// Overwrite DVI with locations placed into a DIArglist.
-static void updateDVIWithLocations(DbgValueInst &DVI,
-                                   SmallVectorImpl<Value *> &Locations,
-                                   SmallVectorImpl<uint64_t> &Ops) {
-  assert(numLLVMArgOps(Ops) != 0 &&
-         "Expected expression that references DIArglist locations using "
-         "DW_OP_llvm_arg operands.");
-  SmallVector<ValueAsMetadata *, 3> MetadataLocs;
-  for (Value *V : Locations)
-    MetadataLocs.push_back(ValueAsMetadata::get(V));
-  auto ValArrayRef = llvm::ArrayRef<llvm::ValueAsMetadata *>(MetadataLocs);
-  DVI.setRawLocation(llvm::DIArgList::get(DVI.getContext(), ValArrayRef));
-  DVI.setExpression(DIExpression::get(DVI.getContext(), Ops));
-}
-
-/// Write the new expression and new location ops for the dbg.value. If possible
-/// reduce the szie of the dbg.value intrinsic by omitting DIArglist. This
-/// can be omitted if:
-/// 1. There is only a single location, refenced by a single DW_OP_llvm_arg.
-/// 2. The DW_OP_LLVM_arg is the first operand in the expression.
+/// Write the new expression and new location ops for the dbg.value. Emit as
+/// short a expression as possible based on checks of the expression length and
+/// number of location ops.
 static void UpdateDbgValueInst(DVIRecoveryRec &DVIRec,
                                SmallVectorImpl<Value *> &NewLocationOps,
                                SmallVectorImpl<uint64_t> &NewExpr) {
-  unsigned NumLLVMArgs = numLLVMArgOps(NewExpr);
-  if (NumLLVMArgs == 0) {
-    // Location assumed to be on the stack.
-    updateDVIWithLocation(*DVIRec.DVI, NewLocationOps[0], NewExpr);
-  } else if (NumLLVMArgs == 1 && NewExpr[0] == dwarf::DW_OP_LLVM_arg) {
-    // There is only a single DW_OP_llvm_arg at the start of the expression,
-    // so it can be omitted along with DIArglist.
-    assert(NewExpr[1] == 0 &&
-           "Lone LLVM_arg in a DIExpression should refer to location-op 0.");
-    llvm::SmallVector<uint64_t, 6> ShortenedOps(llvm::drop_begin(NewExpr, 2));
-    updateDVIWithLocation(*DVIRec.DVI, NewLocationOps[0], ShortenedOps);
-  } else {
-    // Multiple DW_OP_llvm_arg, so DIArgList is strictly necessary.
-    updateDVIWithLocations(*DVIRec.DVI, NewLocationOps, NewExpr);
-  }
-
-  // If the DIExpression was previously empty then add the stack terminator.
-  // Non-empty expressions have only had elements inserted into them and so the
-  // terminator should already be present e.g. stack_value or fragment.
-  DIExpression *SalvageExpr = DVIRec.DVI->getExpression();
-  if (!DVIRec.Expr->isComplex() && SalvageExpr->isComplex()) {
-    SalvageExpr = DIExpression::append(SalvageExpr, {dwarf::DW_OP_stack_value});
-    DVIRec.DVI->setExpression(SalvageExpr);
-  }
-}
-
-/// Cached location ops may be erased during LSR, in which case an undef is
-/// required when restoring from the cache. The type of that location is no
-/// longer available, so just use int8. The undef will be replaced by one or
-/// more locations later when a SCEVDbgValueBuilder selects alternative
-/// locations to use for the salvage.
-static Value *getValueOrUndef(WeakVH &VH, LLVMContext &C) {
-  return (VH) ? VH : UndefValue::get(llvm::Type::getInt8Ty(C));
-}
-
-/// Restore the DVI's pre-LSR arguments. Substitute undef for any erased values.
-static void restorePreTransformState(DVIRecoveryRec &DVIRec) {
-  LLVM_DEBUG(dbgs() << "scev-salvage: restore dbg.value to pre-LSR state\n"
-                    << "scev-salvage: post-LSR: " << *DVIRec.DVI << '\n');
-  assert(DVIRec.Expr && "Expected an expression");
-  DVIRec.DVI->setExpression(DVIRec.Expr);
-
-  // Even a single location-op may be inside a DIArgList and referenced with
-  // DW_OP_LLVM_arg, which is valid only with a DIArgList.
-  if (!DVIRec.HadLocationArgList) {
-    assert(DVIRec.LocationOps.size() == 1 &&
-           "Unexpected number of location ops.");
-    // LSR's unsuccessful salvage attempt may have added DIArgList, which in
-    // this case was not present before, so force the location back to a single
-    // uncontained Value.
-    Value *CachedValue =
-        getValueOrUndef(DVIRec.LocationOps[0], DVIRec.DVI->getContext());
-    DVIRec.DVI->setRawLocation(ValueAsMetadata::get(CachedValue));
+
+  // If there is only a single location op, the {DW_OP_LLVM_arg, 0} sequence
+  // can be omitted from the expression. Also, DIArglist() can be ommitted from
+  // the first argument of the dbg.value.
+  if (NewLocationOps.size() == 1 && NewExpr[0] == dwarf::DW_OP_LLVM_arg &&
+      NewExpr[1] == 0) {
+    DVIRec.DVI->replaceVariableLocationOp(0u, NewLocationOps[0]);
+    llvm::SmallVector<uint64_t, 6> ShortExpr(llvm::drop_begin(NewExpr, 2));
+    auto *FinalExpr = DIExpression::prependOpcodes(DVIRec.Expr, ShortExpr,
+                                                   /*StackValue*/ true);
+    DVIRec.DVI->setExpression(FinalExpr);
   } else {
     SmallVector<ValueAsMetadata *, 3> MetadataLocs;
-    for (WeakVH VH : DVIRec.LocationOps) {
-      Value *CachedValue = getValueOrUndef(VH, DVIRec.DVI->getContext());
-      MetadataLocs.push_back(ValueAsMetadata::get(CachedValue));
-    }
+    for (Value *V : NewLocationOps)
+      MetadataLocs.push_back(ValueAsMetadata::get(V));
     auto ValArrayRef = llvm::ArrayRef<llvm::ValueAsMetadata *>(MetadataLocs);
     DVIRec.DVI->setRawLocation(
         llvm::DIArgList::get(DVIRec.DVI->getContext(), ValArrayRef));
+    auto *FinalExpr =
+        DIExpression::prependOpcodes(DVIRec.Expr, NewExpr, /*StackValue*/ true);
+    DVIRec.DVI->setExpression(FinalExpr);
   }
-  LLVM_DEBUG(dbgs() << "scev-salvage: pre-LSR: " << *DVIRec.DVI << '\n');
 }
 
 static bool SalvageDVI(llvm::Loop *L, ScalarEvolution &SE,
@@ -6357,97 +6193,37 @@ static bool SalvageDVI(llvm::Loop *L, ScalarEvolution &SE,
   if (!DVIRec.DVI->isUndef())
     return false;
 
-  // LSR may have caused several changes to the dbg.value in the failed salvage
-  // attempt. So restore the DIExpression, the location ops and also the
-  // location ops format, which is always DIArglist for multiple ops, but only
-  // sometimes for a single op.
-  restorePreTransformState(DVIRec);
-
-  // LocationOpIndexMap[i] will store the post-LSR location index of
-  // the non-optimised out location at pre-LSR index i.
-  SmallVector<int64_t, 2> LocationOpIndexMap;
-  LocationOpIndexMap.assign(DVIRec.LocationOps.size(), -1);
-  SmallVector<Value *, 2> NewLocationOps;
-  NewLocationOps.push_back(LSRInductionVar);
-
-  for (unsigned i = 0; i < DVIRec.LocationOps.size(); i++) {
-    WeakVH VH = DVIRec.LocationOps[i];
-    // Place the locations not optimised out in the list first, avoiding
-    // inserts later. The map is used to update the DIExpression's
-    // DW_OP_LLVM_arg arguments as the expression is updated.
-    if (VH && !isa<UndefValue>(VH)) {
-      NewLocationOps.push_back(VH);
-      LocationOpIndexMap[i] = NewLocationOps.size() - 1;
-      LLVM_DEBUG(dbgs() << "scev-salvage: Location index " << i
-                        << " now at index " << LocationOpIndexMap[i] << "\n");
-      continue;
-    }
-
-    // It's possible that a value referred to in the SCEV may have been
-    // optimised out by LSR.
-    if (SE.containsErasedValue(DVIRec.SCEVs[i]) ||
-        SE.containsUndefs(DVIRec.SCEVs[i])) {
-      LLVM_DEBUG(dbgs() << "scev-salvage: SCEV for location at index: " << i
-                        << " refers to a location that is now undef or erased. "
-                           "Salvage abandoned.\n");
-      return false;
-    }
-
-    LLVM_DEBUG(dbgs() << "scev-salvage: salvaging location at index " << i
-                      << " with SCEV: " << *DVIRec.SCEVs[i] << "\n");
-
-    DVIRec.RecoveryExprs[i] = std::make_unique<SCEVDbgValueBuilder>();
-    SCEVDbgValueBuilder *SalvageExpr = DVIRec.RecoveryExprs[i].get();
-
-    // Create an offset-based salvage expression if possible, as it requires
-    // less DWARF ops than an iteration count-based expression.
-    if (Optional<APInt> Offset =
-            SE.computeConstantDifference(DVIRec.SCEVs[i], SCEVInductionVar)) {
-      if (Offset.getValue().getMinSignedBits() <= 64)
-        SalvageExpr->createOffsetExpr(Offset.getValue().getSExtValue(),
-                                      LSRInductionVar);
-    } else if (!SalvageExpr->createIterCountExpr(DVIRec.SCEVs[i], IterCountExpr,
-                                                 SE))
+  // Some DVIs that were single location-op when cached are now multi-op,
+  // due to LSR optimisations. However, multi-op salvaging is not yet
+  // supported by SCEV salvaging. The expression applies to a single
+  // location op, so the DIAarglist must be replaced by the original location,
+  // which is now undef.
+  if (DVIRec.DVI->hasArgList()) {
+    if (DVIRec.DVI->getNumVariableLocationOps() == 0 ||
+        !DVIRec.DVI->getVariableLocationOp(0))
       return false;
+    llvm::Type *Ty = DVIRec.DVI->getVariableLocationOp(0)->getType();
+    DVIRec.DVI->setRawLocation(llvm::ValueAsMetadata::get(UndefValue::get(Ty)));
+    DVIRec.DVI->setExpression(DVIRec.Expr);
   }
 
-  // Merge the DbgValueBuilder generated expressions and the original
-  // DIExpression, place the result into an new vector.
-  SmallVector<uint64_t, 3> NewExpr;
-  if (DVIRec.Expr->getNumElements() == 0) {
-    assert(DVIRec.RecoveryExprs.size() == 1 &&
-           "Expected only a single recovery expression for an empty "
-           "DIExpression.");
-    assert(DVIRec.RecoveryExprs[0] &&
-           "Expected a SCEVDbgSalvageBuilder for location 0");
-    SCEVDbgValueBuilder *B = DVIRec.RecoveryExprs[0].get();
-    B->appendToVectors(NewExpr, NewLocationOps);
-  }
-  for (const auto &Op : DVIRec.Expr->expr_ops()) {
-    // Most Ops needn't be updated.
-    if (Op.getOp() != dwarf::DW_OP_LLVM_arg) {
-      Op.appendToVector(NewExpr);
-      continue;
-    }
+  LLVM_DEBUG(dbgs() << "scev-salvage: attempt to salvage: " << *DVIRec.DVI
+                    << '\n');
 
-    uint64_t LocationArgIndex = Op.getArg(0);
-    SCEVDbgValueBuilder *DbgBuilder =
-        DVIRec.RecoveryExprs[LocationArgIndex].get();
-    // The location doesn't have s SCEVDbgValueBuilder, so LSR did not
-    // optimise it away. So just translate the argument to the updated
-    // location index.
-    if (!DbgBuilder) {
-      NewExpr.push_back(dwarf::DW_OP_LLVM_arg);
-      assert(LocationOpIndexMap[Op.getArg(0)] != -1 &&
-             "Expected a positive index for the location-op position.");
-      NewExpr.push_back(LocationOpIndexMap[Op.getArg(0)]);
-      continue;
-    }
-    // The location has a recovery expression.
-    DbgBuilder->appendToVectors(NewExpr, NewLocationOps);
-  }
+  SCEVDbgValueBuilder SalvageExpr;
+
+  // Create an offset-based salvage expression if possible, as it requires
+  // less DWARF ops than an iteration count-based expression.
+  if (Optional<APInt> Offset =
+          SE.computeConstantDifference(DVIRec.SCEV, SCEVInductionVar)) {
+    if (Offset.getValue().getMinSignedBits() <= 64)
+      SalvageExpr.createOffsetExpr(Offset.getValue().getSExtValue(),
+                                   LSRInductionVar);
+  } else
+    SalvageExpr.createIterCountExpr(DVIRec.SCEV, IterCountExpr, SE);
+
+  UpdateDbgValueInst(DVIRec, SalvageExpr.Values, SalvageExpr.Expr);
 
-  UpdateDbgValueInst(DVIRec, NewLocationOps, NewExpr);
   LLVM_DEBUG(dbgs() << "scev-salvage: Updated DVI: " << *DVIRec.DVI << "\n");
   return true;
 }
@@ -6457,7 +6233,7 @@ static bool SalvageDVI(llvm::Loop *L, ScalarEvolution &SE,
 static void
 DbgRewriteSalvageableDVIs(llvm::Loop *L, ScalarEvolution &SE,
                           llvm::PHINode *LSRInductionVar,
-                          SmallVector<DVIRecoveryRec *, 2> &DVIToUpdate) {
+                          SmallVector<DVIRecoveryRec, 2> &DVIToUpdate) {
   if (DVIToUpdate.empty())
     return;
 
@@ -6476,7 +6252,7 @@ DbgRewriteSalvageableDVIs(llvm::Loop *L, ScalarEvolution &SE,
 
     // The iteration count is required to recover location values.
     SCEVDbgValueBuilder IterCountExpr;
-    IterCountExpr.pushLocation(LSRInductionVar);
+    IterCountExpr.pushValue(LSRInductionVar);
     if (!IterCountExpr.SCEVToIterCountExpr(*IVAddRec, SE))
       return;
 
@@ -6484,7 +6260,7 @@ DbgRewriteSalvageableDVIs(llvm::Loop *L, ScalarEvolution &SE,
                       << '\n');
 
     for (auto &DVIRec : DVIToUpdate) {
-      SalvageDVI(L, SE, LSRInductionVar, *DVIRec, SCEVInductionVar,
+      SalvageDVI(L, SE, LSRInductionVar, DVIRec, SCEVInductionVar,
                  IterCountExpr);
     }
   }
@@ -6495,50 +6271,37 @@ DbgRewriteSalvageableDVIs(llvm::Loop *L, ScalarEvolution &SE,
 /// cacheing and salvaging.
 static void
 DbgGatherSalvagableDVI(Loop *L, ScalarEvolution &SE,
-                       SmallVector<DVIRecoveryRec *, 2> &SalvageableDVISCEVs,
+                       SmallVector<DVIRecoveryRec, 2> &SalvageableDVISCEVs,
                        SmallSet<AssertingVH<DbgValueInst>, 2> &DVIHandles) {
   for (auto &B : L->getBlocks()) {
     for (auto &I : *B) {
       auto DVI = dyn_cast<DbgValueInst>(&I);
       if (!DVI)
         continue;
-      // Ensure that if any location op is undef that the dbg.vlue is not
-      // cached.
+
       if (DVI->isUndef())
         continue;
 
-      // Check that the location op SCEVs are suitable for translation to
-      // DIExpression.
-      const auto &HasTranslatableLocationOps =
-          [&](const DbgValueInst *DVI) -> bool {
-        for (const auto LocOp : DVI->location_ops()) {
-          if (!LocOp)
-            return false;
+      if (DVI->hasArgList())
+        continue;
 
-          if (!SE.isSCEVable(LocOp->getType()))
-            return false;
+      if (!DVI->getVariableLocationOp(0) ||
+          !SE.isSCEVable(DVI->getVariableLocationOp(0)->getType()))
+        continue;
 
-          const SCEV *S = SE.getSCEV(LocOp);
-          if (SE.containsUndefs(S))
-            return false;
-        }
-        return true;
-      };
+      // SCEVUnknown wraps an llvm::Value, it does not have a start and stride.
+      // Therefore no translation to DIExpression is performed.
+      const SCEV *S = SE.getSCEV(DVI->getVariableLocationOp(0));
+      if (isa<SCEVUnknown>(S))
+        continue;
 
-      if (!HasTranslatableLocationOps(DVI))
+      // Avoid wasting resources generating an expression containing undef.
+      if (SE.containsUndefs(S))
         continue;
 
-      DVIRecoveryRec *NewRec = new DVIRecoveryRec(DVI);
-      // Each location Op may need a SCEVDbgValueBuilder in order to recover it.
-      // Pre-allocating a vector will enable quick lookups of the builder later
-      // during the salvage.
-      NewRec->RecoveryExprs.resize(DVI->getNumVariableLocationOps());
-      for (const auto LocOp : DVI->location_ops()) {
-        NewRec->SCEVs.push_back(SE.getSCEV(LocOp));
-        NewRec->LocationOps.push_back(LocOp);
-        NewRec->HadLocationArgList = DVI->hasArgList();
-      }
-      SalvageableDVISCEVs.push_back(NewRec);
+      SalvageableDVISCEVs.push_back(
+          {DVI, DVI->getExpression(), DVI->getRawLocation(),
+           SE.getSCEV(DVI->getVariableLocationOp(0))});
       DVIHandles.insert(DVI);
     }
   }
@@ -6587,9 +6350,9 @@ static bool ReduceLoopStrength(Loop *L, IVUsers &IU, ScalarEvolution &SE,
 
   // Debug preservation - before we start removing anything identify which DVI
   // meet the salvageable criteria and store their DIExpression and SCEVs.
-  SmallVector<DVIRecoveryRec *, 2> SalvageableDVIRecords;
+  SmallVector<DVIRecoveryRec, 2> SalvageableDVI;
   SmallSet<AssertingVH<DbgValueInst>, 2> DVIHandles;
-  DbgGatherSalvagableDVI(L, SE, SalvageableDVIRecords, DVIHandles);
+  DbgGatherSalvagableDVI(L, SE, SalvageableDVI, DVIHandles);
 
   bool Changed = false;
   std::unique_ptr<MemorySSAUpdater> MSSAU;
@@ -6637,7 +6400,7 @@ static bool ReduceLoopStrength(Loop *L, IVUsers &IU, ScalarEvolution &SE,
     }
   }
 
-  if (SalvageableDVIRecords.empty())
+  if (SalvageableDVI.empty())
     return Changed;
 
   // Obtain relevant IVs and attempt to rewrite the salvageable DVIs with
@@ -6645,16 +6408,13 @@ static bool ReduceLoopStrength(Loop *L, IVUsers &IU, ScalarEvolution &SE,
   // TODO: Allow for multiple IV references for nested AddRecSCEVs
   for (auto &L : LI) {
     if (llvm::PHINode *IV = GetInductionVariable(*L, SE, Reducer))
-      DbgRewriteSalvageableDVIs(L, SE, IV, SalvageableDVIRecords);
+      DbgRewriteSalvageableDVIs(L, SE, IV, SalvageableDVI);
     else {
       LLVM_DEBUG(dbgs() << "scev-salvage: SCEV salvaging not possible. An IV "
                            "could not be identified.\n");
     }
   }
 
-  for (auto &Rec : SalvageableDVIRecords)
-    Rec->clear();
-  SalvageableDVIRecords.clear();
   DVIHandles.clear();
   return Changed;
 }

diff  --git a/llvm/test/Transforms/LoopStrengthReduce/debuginfo-scev-salvage-5.ll b/llvm/test/Transforms/LoopStrengthReduce/debuginfo-scev-salvage-5.ll
deleted file mode 100644
index a4c0bab91d68..000000000000
--- a/llvm/test/Transforms/LoopStrengthReduce/debuginfo-scev-salvage-5.ll
+++ /dev/null
@@ -1,123 +0,0 @@
-; RUN: opt -S -loop-reduce %s | FileCheck %s
-
-;; Ensure that SCEV-based salvaging in Loop Strength Reduction can salvage
-;; variadic dbg.value intrinsics. Generated from the following C++:
-
-;; clang -S -emit-llvm -Xclang -disable-llvm-passes -g lsr-variadic.cpp -o
-;; Then running 'opt -O2' up until LSR.
-;; void mul_to_addition(unsigned k, unsigned l, unsigned m, unsigned size, unsigned *data) {
-;;     unsigned i = 0;
-;; #pragma clang loop vectorize(disable)
-;;     while (i < size) {
-;;         unsigned comp = (4 * i) + k;
-;;         unsigned comp2 = comp * l;
-;;         unsigned comp3 = comp2 << m;
-;;         data[i] = comp;
-;;         i++;
-;;     }
-;; }
-;; This produces variadic dbg.value intrinsics with location op DIArglists 
-;; of length two and three.
-;; A fourth dbg.value was added artificially by copying a generated dbg.value
-;; and the modifying the position of the optimised-out value in the location
-;; list.
-
-; CHECK: call void @llvm.dbg.value(metadata !DIArgList(i64 %lsr.iv, i32 %k), metadata ![[comp:[0-9]+]], metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_consts, 4, DW_OP_div, DW_OP_consts, 4, DW_OP_mul, DW_OP_LLVM_arg, 1, DW_OP_plus, DW_OP_stack_value))
-; CHECK: call void @llvm.dbg.value(metadata !DIArgList(i64 %lsr.iv, i32 %l, i32 %k), metadata ![[comp2:[0-9]+]], metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_consts, 4, DW_OP_div, DW_OP_consts, 4, DW_OP_mul, DW_OP_LLVM_arg, 2, DW_OP_plus, DW_OP_LLVM_arg, 1, DW_OP_mul, DW_OP_stack_value))
-; CHECK: call void @llvm.dbg.value(metadata !DIArgList(i64 %lsr.iv, i32 %m, i32 %l, i32 %k), metadata ![[comp3:[0-9]+]], metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_consts, 4, DW_OP_div, DW_OP_consts, 4, DW_OP_mul, DW_OP_LLVM_arg, 3, DW_OP_plus, DW_OP_LLVM_arg, 2, DW_OP_mul, DW_OP_LLVM_arg, 1, DW_OP_shl, DW_OP_stack_value))
-; CHECK: call void @llvm.dbg.value(metadata !DIArgList(i64 %lsr.iv, i32 %m, i32 %l, i32 %k), metadata ![[comp3:[0-9]+]], metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_consts, 4, DW_OP_div, DW_OP_consts, 4, DW_OP_mul, DW_OP_LLVM_arg, 3, DW_OP_plus, DW_OP_LLVM_arg, 2, DW_OP_mul, DW_OP_LLVM_arg, 1, DW_OP_shl, DW_OP_stack_value))
-; CHECK: ![[comp]] = !DILocalVariable(name: "comp"
-; CHECK: ![[comp2]] = !DILocalVariable(name: "comp2"
-; CHECK: ![[comp3]] = !DILocalVariable(name: "comp3"
-
-
-target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-unknown-linux-gnu"
-
-define dso_local void @_Z15mul_to_additionjjjjPj(i32 %k, i32 %l, i32 %m, i32 %size, i32* nocapture %data) local_unnamed_addr #0 !dbg !7 {
-entry:
-  call void @llvm.dbg.value(metadata i32 %k, metadata !14, metadata !DIExpression()), !dbg !24
-  call void @llvm.dbg.value(metadata i32 %l, metadata !15, metadata !DIExpression()), !dbg !24
-  call void @llvm.dbg.value(metadata i32 %m, metadata !16, metadata !DIExpression()), !dbg !24
-  call void @llvm.dbg.value(metadata i32 %size, metadata !17, metadata !DIExpression()), !dbg !24
-  call void @llvm.dbg.value(metadata i32* %data, metadata !18, metadata !DIExpression()), !dbg !24
-  call void @llvm.dbg.value(metadata i32 0, metadata !19, metadata !DIExpression()), !dbg !24
-  %cmp9.not = icmp eq i32 %size, 0, !dbg !25
-  br i1 %cmp9.not, label %while.end, label %while.body.preheader, !dbg !26
-
-while.body.preheader:                             ; preds = %entry
-  %wide.trip.count = zext i32 %size to i64, !dbg !25
-  br label %while.body, !dbg !26
-
-while.body:                                       ; preds = %while.body, %while.body.preheader
-  %indvars.iv = phi i64 [ 0, %while.body.preheader ], [ %indvars.iv.next, %while.body ]
-  call void @llvm.dbg.value(metadata i64 %indvars.iv, metadata !19, metadata !DIExpression()), !dbg !24
-  %0 = trunc i64 %indvars.iv to i32, !dbg !27
-  %mul = shl i32 %0, 2, !dbg !27
-  %add = add i32 %mul, %k, !dbg !28
-  call void @llvm.dbg.value(metadata i32 %add, metadata !20, metadata !DIExpression()), !dbg !29
-  call void @llvm.dbg.value(metadata !DIArgList(i32 %add, i32 %l), metadata !22, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_mul, DW_OP_stack_value)), !dbg !29
-  call void @llvm.dbg.value(metadata !DIArgList(i32 %add, i32 %m, i32 %l), metadata !23, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 2, DW_OP_mul, DW_OP_LLVM_arg, 1, DW_OP_shl, DW_OP_stack_value)), !dbg !29
-  call void @llvm.dbg.value(metadata !DIArgList(i32 %m, i32 %add, i32 %l), metadata !23, metadata !DIExpression(DW_OP_LLVM_arg, 1, DW_OP_LLVM_arg, 2, DW_OP_mul, DW_OP_LLVM_arg, 0, DW_OP_shl, DW_OP_stack_value)), !dbg !29
-  %arrayidx = getelementptr inbounds i32, i32* %data, i64 %indvars.iv, !dbg !30
-  store i32 %add, i32* %arrayidx, align 4, !dbg !31
-  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1, !dbg !32
-  call void @llvm.dbg.value(metadata i64 %indvars.iv.next, metadata !19, metadata !DIExpression()), !dbg !24
-  %exitcond = icmp ne i64 %indvars.iv.next, %wide.trip.count, !dbg !25
-  br i1 %exitcond, label %while.body, label %while.end.loopexit, !dbg !26, !llvm.loop !33
-
-while.end.loopexit:                               ; preds = %while.body
-  br label %while.end, !dbg !37
-
-while.end:                                        ; preds = %while.end.loopexit, %entry
-  ret void, !dbg !37
-}
-
-; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
-declare void @llvm.dbg.value(metadata, metadata, metadata)
-
-attributes #0 = { "target-cpu"="x86-64" }
-
-
-!llvm.dbg.cu = !{!0}
-!llvm.module.flags = !{!2, !3, !4, !5}
-!llvm.ident = !{!6}
-
-!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 14.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
-!1 = !DIFile(filename: "variadic2.cpp", directory: "/test")
-!2 = !{i32 7, !"Dwarf Version", i32 4}
-!3 = !{i32 2, !"Debug Info Version", i32 3}
-!4 = !{i32 1, !"wchar_size", i32 4}
-!5 = !{i32 7, !"uwtable", i32 1}
-!6 = !{!"clang version 14.0.0"}
-!7 = distinct !DISubprogram(name: "mul_to_addition", linkageName: "_Z15mul_to_additionjjjjPj", scope: !8, file: !8, line: 1, type: !9, scopeLine: 1, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !13)
-!8 = !DIFile(filename: "./variadic2.cpp", directory: "/test")
-!9 = !DISubroutineType(types: !10)
-!10 = !{null, !11, !11, !11, !11, !12}
-!11 = !DIBasicType(name: "unsigned int", size: 32, encoding: DW_ATE_unsigned)
-!12 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !11, size: 64)
-!13 = !{!14, !15, !16, !17, !18, !19, !20, !22, !23}
-!14 = !DILocalVariable(name: "k", arg: 1, scope: !7, file: !8, line: 1, type: !11)
-!15 = !DILocalVariable(name: "l", arg: 2, scope: !7, file: !8, line: 1, type: !11)
-!16 = !DILocalVariable(name: "m", arg: 3, scope: !7, file: !8, line: 1, type: !11)
-!17 = !DILocalVariable(name: "size", arg: 4, scope: !7, file: !8, line: 1, type: !11)
-!18 = !DILocalVariable(name: "data", arg: 5, scope: !7, file: !8, line: 1, type: !12)
-!19 = !DILocalVariable(name: "i", scope: !7, file: !8, line: 2, type: !11)
-!20 = !DILocalVariable(name: "comp", scope: !21, file: !8, line: 5, type: !11)
-!21 = distinct !DILexicalBlock(scope: !7, file: !8, line: 4, column: 23)
-!22 = !DILocalVariable(name: "comp2", scope: !21, file: !8, line: 6, type: !11)
-!23 = !DILocalVariable(name: "comp3", scope: !21, file: !8, line: 7, type: !11)
-!24 = !DILocation(line: 0, scope: !7)
-!25 = !DILocation(line: 4, column: 15, scope: !7)
-!26 = !DILocation(line: 4, column: 6, scope: !7)
-!27 = !DILocation(line: 5, column: 29, scope: !21)
-!28 = !DILocation(line: 5, column: 34, scope: !21)
-!29 = !DILocation(line: 0, scope: !21)
-!30 = !DILocation(line: 8, column: 10, scope: !21)
-!31 = !DILocation(line: 8, column: 18, scope: !21)
-!32 = !DILocation(line: 9, column: 11, scope: !21)
-!33 = distinct !{!33, !26, !34, !35, !36}
-!34 = !DILocation(line: 10, column: 6, scope: !7)
-!35 = !{!"llvm.loop.mustprogress"}
-!36 = !{!"llvm.loop.vectorize.width", i32 1}
-!37 = !DILocation(line: 11, column: 2, scope: !7)


        


More information about the llvm-commits mailing list