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
This commit is contained in:
Adrian Prantl 2021-08-02 16:59:45 -07:00
parent 56175b2f5c
commit d6b6880172
4 changed files with 79 additions and 71 deletions

View File

@ -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

View File

@ -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.

View File

@ -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

View File

@ -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