[llvm] d6b6880 - Streamline the API of salvageDebugInfoImpl (NFC)
Adrian Prantl via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 10 15:21:27 PDT 2021
Author: Adrian Prantl
Date: 2021-08-10T15:21:18-07:00
New Revision: d6b6880172f295211dd9139a68ba2f42add53ba9
URL: https://github.com/llvm/llvm-project/commit/d6b6880172f295211dd9139a68ba2f42add53ba9
DIFF: https://github.com/llvm/llvm-project/commit/d6b6880172f295211dd9139a68ba2f42add53ba9.diff
LOG: Streamline the API of salvageDebugInfoImpl (NFC)
This patch refactors / simplifies salvageDebugInfoImpl(). The goal
here is to simplify the implementation of coro::salvageDebugInfo() in
a followup patch.
1. Change the return value to I.getOperand(0). Currently users of
salvageDebugInfoImpl() assume that the first operand is
I.getOperand(0). This patch makes this information explicit. A
nice side-effect of this change is that it allows us to salvage
expressions such as add i8 1, %a in the future.
2. Factor out the creation of a DIExpression and return an array of
DIExpression operations instead. This change allows users that
call salvageDebugInfoImpl() in a loop to avoid the costly
creation of temporary DIExpressions and to defer the creation of
a DIExpression until the end.
This patch does not change any functionality.
rdar://80227769
Differential Revision: https://reviews.llvm.org/D107383
Added:
Modified:
llvm/include/llvm/Transforms/Utils/Local.h
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/lib/Transforms/Coroutines/CoroFrame.cpp
llvm/lib/Transforms/Utils/Local.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h
index 0102aa9ef3cc1..97686d7d5f2fa 100644
--- a/llvm/include/llvm/Transforms/Utils/Local.h
+++ b/llvm/include/llvm/Transforms/Utils/Local.h
@@ -292,14 +292,30 @@ void salvageDebugInfo(Instruction &I);
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. \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, unsigned LocNo,
- SmallVectorImpl<Value *> &AdditionalValues);
+/// Given an instruction \p I and DIExpression \p DIExpr operating on
+/// it, append the effects of \p I to the DIExpression operand list
+/// \p Ops, or return \p nullptr if it cannot be salvaged.
+/// \p CurrentLocOps is the number of SSA values referenced by the
+/// incoming \p Ops. \return the first non-constant operand
+/// implicitly referred to by Ops. If \p I references more than one
+/// non-constant operand, any additional operands are added to
+/// \p AdditionalValues.
+///
+/// \example
+////
+/// I = add %a, i32 1
+///
+/// Return = %a
+/// Ops = llvm::dwarf::DW_OP_lit1 llvm::dwarf::DW_OP_add
+///
+/// I = add %a, %b
+///
+/// Return = %a
+/// Ops = llvm::dwarf::DW_OP_LLVM_arg0 llvm::dwarf::DW_OP_add
+/// AdditionalValues = %b
+Value *salvageDebugInfoImpl(Instruction &I, uint64_t CurrentLocOps,
+ SmallVectorImpl<uint64_t> &Ops,
+ SmallVectorImpl<Value *> &AdditionalValues);
/// 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
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index e0e4eed3e76a1..514b215eeb707 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -1281,21 +1281,23 @@ void SelectionDAGBuilder::salvageUnresolvedDbgValue(DanglingDebugInfo &DDI) {
while (isa<Instruction>(V)) {
Instruction &VAsInst = *cast<Instruction>(V);
// Temporary "0", awaiting real implementation.
+ SmallVector<uint64_t, 16> Ops;
SmallVector<Value *, 4> AdditionalValues;
- DIExpression *SalvagedExpr =
- salvageDebugInfoImpl(VAsInst, Expr, StackValue, 0, AdditionalValues);
-
+ V = salvageDebugInfoImpl(VAsInst, Expr->getNumLocationOperands(), Ops,
+ AdditionalValues);
// If we cannot salvage any further, and haven't yet found a suitable debug
// expression, bail out.
+ if (!V)
+ break;
+
// TODO: If AdditionalValues isn't empty, then the salvage can only be
// represented with a DBG_VALUE_LIST, so we give up. When we have support
// here for variadic dbg_values, remove that condition.
- if (!SalvagedExpr || !AdditionalValues.empty())
+ if (!AdditionalValues.empty())
break;
// New value and expr now represent this debuginfo.
- V = VAsInst.getOperand(0);
- Expr = SalvagedExpr;
+ Expr = DIExpression::appendOpsToArg(Expr, Ops, 0, StackValue);
// Some kind of simplification occurred: check whether the operand of the
// salvaged debug expression can be encoded in this DAG.
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index 422d33ee96e55..921b83ba715ea 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -2529,16 +2529,18 @@ void coro::salvageDebugInfo(
} else if (auto *StInst = dyn_cast<StoreInst>(Storage)) {
Storage = StInst->getOperand(0);
} else if (auto *GEPInst = dyn_cast<GetElementPtrInst>(Storage)) {
- SmallVector<Value *> AdditionalValues;
- DIExpression *SalvagedExpr = llvm::salvageDebugInfoImpl(
- *GEPInst, Expr,
- /*WithStackValue=*/false, 0, AdditionalValues);
+ SmallVector<uint64_t, 16> Ops;
+ SmallVector<Value *, 0> AdditionalValues;
+ Storage = llvm::salvageDebugInfoImpl(
+ *GEPInst, Expr ? Expr->getNumLocationOperands() : 0, Ops,
+ AdditionalValues);
+ if (!Storage)
+ break;
// Debug declares cannot currently handle additional location
// operands.
- if (!SalvagedExpr || !AdditionalValues.empty())
+ if (!AdditionalValues.empty())
break;
- Expr = SalvagedExpr;
- Storage = GEPInst->getOperand(0);
+ Expr = DIExpression::appendOpsToArg(Expr, Ops, 0, /*StackValue*/ false);
} else if (auto *BCInst = dyn_cast<llvm::BitCastInst>(Storage))
Storage = BCInst->getOperand(0);
else
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index d03d76f57ca17..590600eec26c4 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -1752,20 +1752,26 @@ void llvm::salvageDebugInfoForDbgValues(
// must be updated in the DIExpression and potentially have additional
// values added; thus we call salvageDebugInfoImpl for each `I` instance in
// DIILocation.
+ Value *Op0 = nullptr;
DIExpression *SalvagedExpr = DII->getExpression();
auto LocItr = find(DIILocation, &I);
while (SalvagedExpr && LocItr != DIILocation.end()) {
+ SmallVector<uint64_t, 16> Ops;
unsigned LocNo = std::distance(DIILocation.begin(), LocItr);
- SalvagedExpr = salvageDebugInfoImpl(I, SalvagedExpr, StackValue, LocNo,
- AdditionalValues);
+ uint64_t CurrentLocOps = SalvagedExpr->getNumLocationOperands();
+ Op0 = salvageDebugInfoImpl(I, CurrentLocOps, Ops, AdditionalValues);
+ if (!Op0)
+ break;
+ SalvagedExpr =
+ DIExpression::appendOpsToArg(SalvagedExpr, Ops, LocNo, StackValue);
LocItr = std::find(++LocItr, DIILocation.end(), &I);
}
// salvageDebugInfoImpl should fail on examining the first element of
// DbgUsers, or none of them.
- if (!SalvagedExpr)
+ if (!Op0)
break;
- DII->replaceVariableLocationOp(&I, I.getOperand(0));
+ DII->replaceVariableLocationOp(&I, Op0);
if (AdditionalValues.empty()) {
DII->setExpression(SalvagedExpr);
} else if (isa<DbgValueInst>(DII) &&
@@ -1793,16 +1799,16 @@ void llvm::salvageDebugInfoForDbgValues(
}
}
-bool getSalvageOpsForGEP(GetElementPtrInst *GEP, const DataLayout &DL,
- uint64_t CurrentLocOps,
- SmallVectorImpl<uint64_t> &Opcodes,
- SmallVectorImpl<Value *> &AdditionalValues) {
+Value *getSalvageOpsForGEP(GetElementPtrInst *GEP, const DataLayout &DL,
+ uint64_t CurrentLocOps,
+ SmallVectorImpl<uint64_t> &Opcodes,
+ SmallVectorImpl<Value *> &AdditionalValues) {
unsigned BitWidth = DL.getIndexSizeInBits(GEP->getPointerAddressSpace());
// Rewrite a GEP into a DIExpression.
MapVector<Value *, APInt> VariableOffsets;
APInt ConstantOffset(BitWidth, 0);
if (!GEP->collectOffset(DL, BitWidth, VariableOffsets, ConstantOffset))
- return false;
+ return nullptr;
if (!VariableOffsets.empty() && !CurrentLocOps) {
Opcodes.insert(Opcodes.begin(), {dwarf::DW_OP_LLVM_arg, 0});
CurrentLocOps = 1;
@@ -1816,7 +1822,7 @@ bool getSalvageOpsForGEP(GetElementPtrInst *GEP, const DataLayout &DL,
dwarf::DW_OP_plus});
}
DIExpression::appendOffset(Opcodes, ConstantOffset.getSExtValue());
- return true;
+ return GEP->getOperand(0);
}
uint64_t getDwarfOpForBinOp(Instruction::BinaryOps Opcode) {
@@ -1849,14 +1855,14 @@ uint64_t getDwarfOpForBinOp(Instruction::BinaryOps Opcode) {
}
}
-bool getSalvageOpsForBinOp(BinaryOperator *BI, uint64_t CurrentLocOps,
- SmallVectorImpl<uint64_t> &Opcodes,
- SmallVectorImpl<Value *> &AdditionalValues) {
+Value *getSalvageOpsForBinOp(BinaryOperator *BI, uint64_t CurrentLocOps,
+ SmallVectorImpl<uint64_t> &Opcodes,
+ SmallVectorImpl<Value *> &AdditionalValues) {
// Handle binary operations with constant integer operands as a special case.
auto *ConstInt = dyn_cast<ConstantInt>(BI->getOperand(1));
// Values wider than 64 bits cannot be represented within a DIExpression.
if (ConstInt && ConstInt->getBitWidth() > 64)
- return false;
+ return nullptr;
Instruction::BinaryOps BinOpcode = BI->getOpcode();
// Push any Constant Int operand onto the expression stack.
@@ -1867,7 +1873,7 @@ bool getSalvageOpsForBinOp(BinaryOperator *BI, uint64_t CurrentLocOps,
if (BinOpcode == Instruction::Add || BinOpcode == Instruction::Sub) {
uint64_t Offset = BinOpcode == Instruction::Add ? Val : -int64_t(Val);
DIExpression::appendOffset(Opcodes, Offset);
- return true;
+ return BI->getOperand(0);
}
Opcodes.append({dwarf::DW_OP_constu, Val});
} else {
@@ -1883,39 +1889,23 @@ bool getSalvageOpsForBinOp(BinaryOperator *BI, uint64_t CurrentLocOps,
// representation in a DIExpression.
uint64_t DwarfBinOp = getDwarfOpForBinOp(BinOpcode);
if (!DwarfBinOp)
- return false;
+ return nullptr;
Opcodes.push_back(DwarfBinOp);
-
- return true;
+ return BI->getOperand(0);
}
-DIExpression *
-llvm::salvageDebugInfoImpl(Instruction &I, DIExpression *SrcDIExpr,
- bool WithStackValue, unsigned LocNo,
- SmallVectorImpl<Value *> &AdditionalValues) {
- uint64_t CurrentLocOps = SrcDIExpr->getNumLocationOperands();
+Value *llvm::salvageDebugInfoImpl(Instruction &I, uint64_t CurrentLocOps,
+ SmallVectorImpl<uint64_t> &Ops,
+ SmallVectorImpl<Value *> &AdditionalValues) {
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::appendOpsToArg(DIExpr, Ops, LocNo, WithStackValue);
- }
- return DIExpr;
- };
-
- // initializer-list helper for applying operators to the source DIExpression.
- auto applyOps = [&](ArrayRef<uint64_t> Opcodes) {
- SmallVector<uint64_t, 8> Ops(Opcodes.begin(), Opcodes.end());
- return doSalvage(Ops);
- };
-
if (auto *CI = dyn_cast<CastInst>(&I)) {
+ Value *FromValue = CI->getOperand(0);
// No-op casts are irrelevant for debug info.
- if (CI->isNoopCast(DL))
- return SrcDIExpr;
+ if (CI->isNoopCast(DL)) {
+ return FromValue;
+ }
Type *Type = CI->getType();
// Casts other than Trunc, SExt, or ZExt to scalar types cannot be salvaged.
@@ -1923,21 +1913,19 @@ llvm::salvageDebugInfoImpl(Instruction &I, DIExpression *SrcDIExpr,
!(isa<TruncInst>(&I) || isa<SExtInst>(&I) || isa<ZExtInst>(&I)))
return nullptr;
- Value *FromValue = CI->getOperand(0);
unsigned FromTypeBitSize = FromValue->getType()->getScalarSizeInBits();
unsigned ToTypeBitSize = Type->getScalarSizeInBits();
- return applyOps(DIExpression::getExtOps(FromTypeBitSize, ToTypeBitSize,
- isa<SExtInst>(&I)));
+ auto ExtOps = DIExpression::getExtOps(FromTypeBitSize, ToTypeBitSize,
+ isa<SExtInst>(&I));
+ Ops.append(ExtOps.begin(), ExtOps.end());
+ return FromValue;
}
- SmallVector<uint64_t, 8> Ops;
- if (auto *GEP = dyn_cast<GetElementPtrInst>(&I)) {
- if (getSalvageOpsForGEP(GEP, DL, CurrentLocOps, Ops, AdditionalValues))
- return doSalvage(Ops);
- } else if (auto *BI = dyn_cast<BinaryOperator>(&I)) {
- if (getSalvageOpsForBinOp(BI, CurrentLocOps, Ops, AdditionalValues))
- return doSalvage(Ops);
+ if (auto *GEP = dyn_cast<GetElementPtrInst>(&I))
+ return getSalvageOpsForGEP(GEP, DL, CurrentLocOps, Ops, AdditionalValues);
+ else if (auto *BI = dyn_cast<BinaryOperator>(&I)) {
+ return getSalvageOpsForBinOp(BI, CurrentLocOps, Ops, AdditionalValues);
}
// *Not* to do: we should not attempt to salvage load instructions,
// because the validity and lifetime of a dbg.value containing
More information about the llvm-commits
mailing list