Skip to content

Commit 75f1453

Browse files
aparshin-inteligcbot
authored andcommitted
fix generation of DWARF locations for indirect values
The problem comes from the inconsistency between semantics of <llvm.dbg.value> and location descriptors defined by DWARF. <llvm.dbg.value> describes the actual *value*, of a source variable - on the other hand, DWARF location descriptors focus primarily (with few excepions) on describing the *location* from which the value can be fetched (the location can be an address in register file or memory). Now, DWARF allows for the following location desciptors for the pieces of variables: 1. empty location 2. register location 3. memory location 4. implicit location The situation that was not processed correctly looked like this: ``` call void @llvm.dbg.value(metadata [4096 x i32] addrspace(1)* %0, metadata !1299, metadata !DIExpression(DW_OP_deref)) ``` This means that the actual *value* of the source varible !1299 is located at the address defined by %0. Let's assume %0 is held in a register - then a proper DWARF location descripor should be of "memory location" type which is evaluated to the value held in %0. In general case, the respected location descripor should look like this: ``` 0: DW_OP_reg<N + 16> 1: DW_OP_const1u <offset in GRF N> 2: DW_OP_const1u <size in bits of %0> 3: DW_OP_INTEL_push_bit_piece_stack ``` NOTE_1: Item #3, transforms the original "register location" defined by Item #0 to a "memory location". Such transformations are not possible in the vanilla DWARF spec, thus we had to opt for intel extensions. NOTE_2: It seems that we have some level of ambiguity. For example, I think it is perfectly legal to have this kind of expressions: ``` call void @llvm.dbg.value(metadata i32 %0, metadata !1299, metadata !DIExpression(DW_OP_deref), DW_OP_plus_uconst, 3) ``` Such an expression defines a concrete value. But our current implementaion won't be able to handle this. From dwarf point of view the respected expression should be an implicit value. While in reality our backend will emit a <memory location expression>. I've added some asserts and diagnostics to detect such cases.
1 parent 5d21ee3 commit 75f1453

File tree

5 files changed

+187
-63
lines changed

5 files changed

+187
-63
lines changed

IGC/DebugInfo/DwarfCompileUnit.cpp

Lines changed: 22 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -202,12 +202,6 @@ int64_t CompileUnit::getDefaultLowerBound() const
202202
return -1;
203203
}
204204

205-
static bool HasImplicitLocation(const DbgVariable& var) {
206-
if (auto* dbgInst = var.getDbgInst())
207-
if (auto* expr = dbgInst->getExpression())
208-
return expr->isImplicit();
209-
return false;
210-
}
211205
/// Check whether the DIE for this MDNode can be shared across CUs.
212206
static bool isShareableAcrossCUs(llvm::MDNode* D)
213207
{
@@ -1381,9 +1375,7 @@ void CompileUnit::addSimdLane(IGC::DIEBlock* Block, const DbgVariable& DV,
13811375
{
13821376
// SIMD lane
13831377
const auto* VISAMod = Loc->GetVISAModule();
1384-
auto varSizeInBits = Loc->IsInMemory()
1385-
? Asm->GetPointerSize() * 8
1386-
: DV.getRegisterValueSizeInBits(DD);
1378+
auto varSizeInBits = DV.getRegisterValueSizeInBits(DD);
13871379

13881380
LLVM_DEBUG(dbgs() << " addSimdLane(varSizeInBits: " << varSizeInBits <<
13891381
", simdWidthOffset: " << simdWidthOffset << ", isPacked: " <<
@@ -1586,26 +1578,29 @@ void CompileUnit::addSimdLaneScalar(IGC::DIEBlock* Block, const DbgVariable& DV,
15861578
{
15871579
if (EmitSettings.EnableSIMDLaneDebugging)
15881580
{
1589-
IGC_ASSERT_MESSAGE(lr->isSpill() == false, "Scalar spilled in scratch space");
1581+
IGC_ASSERT_MESSAGE(!lr->isSpill(), "Scalar spilled in scratch space");
15901582

1591-
auto varSizeInBits = Loc->IsInMemory()
1592-
? Asm->GetPointerSize() * 8
1593-
: DV.getRegisterValueSizeInBits(DD);
1583+
auto varSizeInBits = DV.getRegisterValueSizeInBits(DD);
15941584
auto offsetInBits = subRegInBytes * 8;
15951585
IGC_ASSERT(offsetInBits / 8 == subRegInBytes);
15961586

1587+
if (DD->getEmitterSettings().EnableDebugInfoValidation)
1588+
DD->getStreamEmitter().verifyRegisterLocationExpr(DV, *DD);
1589+
15971590
LLVM_DEBUG(dbgs() << " addSimdLaneScalar(varSizeInBits: " <<
15981591
varSizeInBits << ", offsetInBits: " << offsetInBits << ")\n");
1599-
if (isa<llvm::DbgDeclareInst>(DV.getDbgInst()) || HasImplicitLocation(DV))
1592+
if (DV.currentLocationIsMemoryAddress() ||
1593+
DV.currentLocationIsImplicit() ||
1594+
DV.currentLocationIsSimpleIndirectValue())
16001595
{
1601-
// Pointer or an implicit value of a variable
1596+
// Pointer, indirect or an implicit value of a variable
16021597
// Note: in case of implicit value we want to put the value of the
1603-
// bit_piece onto DWARF stack, so implict expression could operatate
1598+
// bit_piece onto DWARF stack, so expression could operate
16041599
// on it.
16051600
addConstantUValue(Block, offsetInBits);
16061601
addConstantUValue(Block, varSizeInBits);
16071602

1608-
LLVM_DEBUG(dbgs() << " value is pointer or an implicit\n");
1603+
LLVM_DEBUG(dbgs() << " value is pointer/indirect or an implicit\n");
16091604
IGC_ASSERT_MESSAGE(varSizeInBits <= 64, "Entries pushed onto DWARF stack are limited to 8 bytes");
16101605

16111606
addUInt(Block, dwarf::DW_FORM_data1, DW_OP_INTEL_push_bit_piece_stack);
@@ -2623,22 +2618,6 @@ IGC::DIEBlock* CompileUnit::buildGeneral(const DbgVariable& var,
26232618
DIEValue* secondHalfOff = nullptr, *skipOff = nullptr;
26242619
unsigned int offsetTaken = 0, offsetNotTaken = 0, offsetEnd = 0;
26252620

2626-
auto EmitExpression = [this](IGC::DIEBlock* Block, const DbgVariable& var) {
2627-
// Emit DIExpression if it exists
2628-
if (auto* dbgInst = var.getDbgInst())
2629-
{
2630-
if (auto* expr = dbgInst->getExpression())
2631-
{
2632-
for (auto elem : expr->getElements())
2633-
{
2634-
auto BF = DIEInteger::BestForm(false, elem);
2635-
addUInt(Block, BF, elem);
2636-
}
2637-
}
2638-
}
2639-
return;
2640-
};
2641-
26422621
LLVM_DEBUG(dbgs() << " building DWARF info for the variable [" << var.getName() << "]\n");
26432622
// locs contains 1 item for SIMD8/SIMD16 kernels describing locations of all channels.
26442623
// locs contains 2 items for SIMD32 kernels. First item has storage mapping for lower 16
@@ -2931,13 +2910,19 @@ IGC::DIEBlock* CompileUnit::buildGeneral(const DbgVariable& var,
29312910
auto offsetInBits = lrToUse.getGRF().subRegNum * 8;
29322911
IGC_ASSERT(offsetInBits / 8 == lrToUse.getGRF().subRegNum);
29332912

2913+
if (DD->getEmitterSettings().EnableDebugInfoValidation)
2914+
DD->getStreamEmitter().verifyRegisterLocationExpr(var, *DD);
2915+
29342916
addRegisterLoc(Block, lrToUse.getGRF().regNum, offset, var.getDbgInst());
29352917

2936-
if (HasImplicitLocation(var))
2918+
if (var.currentLocationIsMemoryAddress() ||
2919+
var.currentLocationIsImplicit() ||
2920+
var.currentLocationIsSimpleIndirectValue())
29372921
{
29382922
addConstantUValue(Block, offsetInBits);
29392923
addConstantUValue(Block, varSizeInBits);
29402924

2925+
LLVM_DEBUG(dbgs() << " value is pointer/indirect or an implicit\n");
29412926
IGC_ASSERT_MESSAGE(varSizeInBits <= 64, "Entries pushed onto DWARF stack are limited to 8 bytes");
29422927
addUInt(Block, dwarf::DW_FORM_data1, DW_OP_INTEL_push_bit_piece_stack);
29432928
}
@@ -2955,7 +2940,7 @@ IGC::DIEBlock* CompileUnit::buildGeneral(const DbgVariable& var,
29552940
// Emit GT-relative location expression
29562941
addGTRelativeLocation(Block, loc);
29572942
addSimdLaneScalar(Block, var, loc, &lrToUse, subReg);
2958-
EmitExpression(Block, var);
2943+
var.emitExpression(this, Block);
29592944

29602945
break;
29612946
}
@@ -3002,9 +2987,7 @@ IGC::DIEBlock* CompileUnit::buildGeneral(const DbgVariable& var,
30022987
unsigned GrfSizeInBits = GrfSizeBytes * 8;
30032988
IGC_ASSERT(GrfSizeInBits <= std::numeric_limits<uint16_t>::max());
30042989

3005-
unsigned varSizeInBits = loc->IsInMemory()
3006-
? Asm->GetPointerSize() * 8
3007-
: var.getRegisterValueSizeInBits(DD);
2990+
unsigned varSizeInBits = var.getRegisterValueSizeInBits(DD);
30082991

30092992
unsigned varSizeInReg = (loc->IsInMemory() && varSizeInBits < 32)
30102993
? 32
@@ -3033,7 +3016,7 @@ IGC::DIEBlock* CompileUnit::buildGeneral(const DbgVariable& var,
30333016
{
30343017
LLVM_DEBUG(dbgs() << " <warning> variable is niether in GRF nor spilled\n");
30353018
}
3036-
EmitExpression(Block, var);
3019+
var.emitExpression(this, Block);
30373020

30383021
}
30393022
firstHalf = false;

IGC/DebugInfo/DwarfDebug.cpp

Lines changed: 81 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,15 @@ See LICENSE.TXT for details.
3131
#include "llvm/IR/InstIterator.h"
3232
#include "llvm/IR/Module.h"
3333
#include "llvm/MC/MCAsmInfo.h"
34+
#include "llvm/MC/MCDwarf.h"
3435
#include "llvm/MC/MCSection.h"
3536
#include "llvm/MC/MCStreamer.h"
3637
#include "llvm/MC/MCSymbol.h"
3738
#include "llvm/Support/Debug.h"
3839
#include "llvm/Support/FormattedStream.h"
3940
#include "llvm/Support/LEB128.h"
4041
#include "llvm/IR/IntrinsicInst.h"
41-
#include "llvm/MC/MCDwarf.h"
42+
#include "llvm/IR/DebugInfoMetadata.h"
4243
#include "common/LLVMWarningsPop.hpp"
4344

4445
#include "DwarfDebug.hpp"
@@ -77,19 +78,12 @@ bool DbgVariable::isBlockByrefVariable() const {
7778
#endif
7879
}
7980

80-
// Currently, only DbgDeclare and DbgValue are supported
81-
static bool IsSupportedDebugInst(const llvm::Instruction* Inst)
82-
{
83-
IGC_ASSERT(Inst);
84-
return dyn_cast<DbgValueInst>(Inst) || dyn_cast<DbgDeclareInst>(Inst);
85-
}
86-
8781
static bool IsDebugInst(const llvm::Instruction* Inst)
8882
{
8983
if (!isa<DbgInfoIntrinsic>(Inst))
9084
return false;
9185
#ifndef NDEBUG
92-
if (!IsSupportedDebugInst(Inst))
86+
if (!DbgVariable::IsSupportedDebugInst(Inst))
9387
{
9488
LLVM_DEBUG(dbgs() << "WARNING! Unsupported DbgInfo Instruction detected:\n";
9589
DbgVariable::dumpDbgInst(Inst));
@@ -100,7 +94,7 @@ static bool IsDebugInst(const llvm::Instruction* Inst)
10094

10195
static const MDNode* GetDebugVariable(const Instruction* Inst)
10296
{
103-
IGC_ASSERT(IsSupportedDebugInst(Inst));
97+
IGC_ASSERT(DbgVariable::IsSupportedDebugInst(Inst));
10498

10599
if (const auto* DclInst = dyn_cast<DbgDeclareInst>(Inst))
106100
return DclInst->getVariable();
@@ -111,6 +105,83 @@ static const MDNode* GetDebugVariable(const Instruction* Inst)
111105
return nullptr;
112106
}
113107

108+
bool DbgVariable::IsSupportedDebugInst(const llvm::Instruction* Inst)
109+
{
110+
IGC_ASSERT(Inst);
111+
return dyn_cast<DbgValueInst>(Inst) || dyn_cast<DbgDeclareInst>(Inst);
112+
}
113+
114+
bool DbgVariable::currentLocationIsImplicit() const
115+
{
116+
const auto* DbgInst = getDbgInst();
117+
if (!DbgInst)
118+
return false;
119+
return DbgInst->getExpression()->isImplicit();
120+
}
121+
122+
bool DbgVariable::currentLocationIsMemoryAddress() const
123+
{
124+
const auto* DbgInst = getDbgInst();
125+
if (!DbgInst)
126+
return false;
127+
return isa<llvm::DbgDeclareInst>(DbgInst);
128+
}
129+
130+
bool DbgVariable::currentLocationIsSimpleIndirectValue() const
131+
{
132+
if (currentLocationIsImplicit())
133+
return false;
134+
135+
const auto* DbgInst = getDbgInst();
136+
if (!isa<llvm::DbgValueInst>(DbgInst))
137+
return false;
138+
auto* Expr = DbgInst->getExpression();
139+
140+
// IMPORTANT: changes here should be in sync with DbgVariable::emitExpression
141+
Value* IRLocation = IGCLLVM::getVariableLocation(DbgInst);
142+
if (!IRLocation->getType()->isPointerTy())
143+
return false;
144+
145+
if (!Expr->startsWithDeref())
146+
return false;
147+
148+
if (!std::all_of(Expr->expr_op_begin(), Expr->expr_op_end(),
149+
[](const auto& DIOp) {
150+
return DIOp.getOp() == dwarf::DW_OP_deref ||
151+
DIOp.getOp() == dwarf::DW_OP_LLVM_fragment;
152+
}))
153+
{
154+
// backout if the expression contains something other than deref/fragment
155+
return false;
156+
}
157+
158+
return true;
159+
}
160+
161+
void DbgVariable::emitExpression(CompileUnit* CU, IGC::DIEBlock* Block) const
162+
{
163+
IGC_ASSERT(CU);
164+
IGC_ASSERT(Block);
165+
166+
const auto* DbgInst = getDbgInst();
167+
if (!DbgInst)
168+
return;
169+
170+
// Indirect values result in emission DWARF location descriptors of
171+
// <memory location> type - the evaluation should result in address,
172+
// thus no need for OP_deref.
173+
// Currently, our dwarf emitters support only "simple indirect" values.
174+
auto Elements = DbgInst->getExpression()->getElements();
175+
if (currentLocationIsSimpleIndirectValue())
176+
Elements = Elements.slice(1); // drop OP_deref
177+
178+
for (auto elem : Elements)
179+
{
180+
auto BF = DIEInteger::BestForm(false, elem);
181+
CU->addUInt(Block, BF, elem);
182+
}
183+
}
184+
114185
/// If this type is derived from a base type then return base type size
115186
/// even if it derived directly or indirectly from Derived Type
116187
uint64_t DbgVariable::getBasicTypeSize(const DICompositeType* Ty) const

IGC/DebugInfo/DwarfDebug.hpp

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,11 @@ namespace IGC
151151
DbgVariable* getAbstractVariable() const { return AbsVar; }
152152

153153
const llvm::DbgVariableIntrinsic* getDbgInst() const { return m_pDbgInst; }
154-
void setDbgInst(const llvm::DbgVariableIntrinsic* pInst) { m_pDbgInst = pInst; }
154+
void setDbgInst(const llvm::DbgVariableIntrinsic* pInst)
155+
{
156+
IGC_ASSERT(pInst && IsSupportedDebugInst(pInst));
157+
m_pDbgInst = pInst;
158+
}
155159

156160
/// If this type is derived from a base type then return base type size
157161
/// even if it derived directly or indirectly from Composite Type
@@ -170,6 +174,25 @@ namespace IGC
170174
/// storage required
171175
unsigned getRegisterValueSizeInBits(const DwarfDebug* DD) const;
172176

177+
bool currentLocationIsImplicit() const;
178+
bool currentLocationIsMemoryAddress() const;
179+
180+
// "Simple Indirect Value" is a dbg.value of the following form:
181+
// llvm.dbg.value(pointer to <whatever>, <nevermind>,
182+
// !DIExpression(DW_OP_deref, [DW_OP_LLVM_fragment]))
183+
// Other types of indirect values are currently not supported.
184+
// For example, if we encounter an expression like this:
185+
// call void @llvm.dbg.value(metadata i32* %<whatever>,
186+
// metadata !<nevermind>,
187+
// metadata !DIExpression(DW_OP_deref), DW_OP_plus_uconst, 3)
188+
// We won't be able to emit a proper expression, due to limitations
189+
// of the current *implementation*. A proper expression should have
190+
// DW_OP_stack_value to form a proper location descriptor needed in
191+
// this context.
192+
bool currentLocationIsSimpleIndirectValue() const;
193+
194+
void emitExpression(CompileUnit* CU, IGC::DIEBlock* Block) const;
195+
173196
// Translate tag to proper Dwarf tag.
174197
llvm::dwarf::Tag getTag() const
175198
{
@@ -202,6 +225,8 @@ namespace IGC
202225

203226
llvm::DIType* getType() const;
204227

228+
229+
static bool IsSupportedDebugInst(const llvm::Instruction* Inst);
205230
void print(llvm::raw_ostream& O, bool NestedAbstract = false) const;
206231
static void printDbgInst(llvm::raw_ostream& O,
207232
const llvm::Instruction* Inst,

IGC/DebugInfo/StreamEmitter.cpp

Lines changed: 42 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -836,30 +836,59 @@ void StreamEmitter::verifyRegisterLocationSize(const IGC::DbgVariable& VarVal,
836836
// TODO: implement some sanity checks
837837
return;
838838
}
839-
std::string ErrMsg;
840-
llvm::raw_string_ostream OS(ErrMsg);
841-
auto PosBeforeChecks = OS.tell();
842-
839+
DiagnosticBuff Diag;
843840
auto DwarfTypeSize = VarVal.getBasicSize(&DD);
844841
if (DwarfTypeSize != ExpectedSize) {
845-
OS << "ValidationFailure [regLocSize] -- DWARF Type Size: " <<
842+
Diag.out() << "ValidationFailure [regLocSize] -- DWARF Type Size: " <<
846843
DwarfTypeSize << ", expected: " << ExpectedSize << "\n";
847844
}
848845
if (ExpectedSize > MaxGRFSpaceInBits) {
849-
OS << "ValidationFailure [GRFSpace] -- Available GRF space: " <<
846+
Diag.out() << "ValidationFailure [GRFSpace] -- Available GRF space: " <<
850847
MaxGRFSpaceInBits << ", while expected value size: " <<
851848
ExpectedSize << "\n";
852849
}
853850

854851
// Dump DbgVariable if errors were reported
855-
if (PosBeforeChecks != OS.tell())
856-
{
857-
VarVal.print(OS);
858-
OS << "==============\n";
859-
OS.str(); // flush raw_string_stream
852+
verificationReport(VarVal, Diag);
853+
}
854+
855+
void StreamEmitter::verifyRegisterLocationExpr(const DbgVariable& DV,
856+
const DwarfDebug& DD)
857+
{
858+
if (!GetEmitterSettings().EnableDebugInfoValidation)
859+
return;
860860

861-
ErrorLog.append(ErrMsg);
862-
LLVM_DEBUG(dbgs() << ErrMsg);
861+
// TODO: add checks for locations other than llvm.dbg.value
862+
if (DV.currentLocationIsMemoryAddress())
863+
return;
864+
865+
auto* DbgInst = DV.getDbgInst();
866+
if (!isa<llvm::DbgValueInst>(DbgInst))
867+
return;
868+
869+
DiagnosticBuff Diag;
870+
if (!DV.currentLocationIsImplicit() &&
871+
!DV.currentLocationIsSimpleIndirectValue())
872+
{
873+
if (DbgInst->getExpression()->isComplex())
874+
{
875+
Diag.out() << "ValidationFailure [UnexpectedComlexExpression]" <<
876+
" for a simple register location\n";
877+
}
863878
}
879+
verificationReport(DV, Diag);
864880
}
865881

882+
void StreamEmitter::verificationReport(const DbgVariable& VarVal,
883+
DiagnosticBuff& Diag)
884+
{
885+
if (Diag.out().tell() == 0)
886+
return;
887+
888+
VarVal.print(Diag.out());
889+
Diag.out() << "==============\n";
890+
const auto& ErrMsg = Diag.out().str();
891+
892+
ErrorLog.append(ErrMsg);
893+
LLVM_DEBUG(dbgs() << ErrMsg);
894+
}

0 commit comments

Comments
 (0)