When doing RA we end up with adding ValueDef references to Values across
all over the shader. This is all fine until we remove the Instruction
defining those Values, which happens when spilling values.
Instead of manipulating the values directly we should just track all
merged in defs in a seperate structure and remove stale references when
an instruction gets deleted in the spiller.
fixes following libasan report:
=================================================================
==612087==ERROR: AddressSanitizer: heap-use-after-free on address
0x6150003ea380 at pc 0x7f1d12142fe9 bp 0x7fffca6fd120 sp 0x7fffca6fd110
READ of size 8 at 0x6150003ea380 thread T0
#0 0x7f1d12142fe8 in nv50_ir::ValueDef::get() const
../src/gallium/drivers/nouveau/codegen/nv50_ir.h:648
#1 0x7f1d12143c02 in nv50_ir::Value::getUniqueInsn() const
../src/gallium/drivers/nouveau/codegen/nv50_ir_inlines.h:229
#2 0x7f1d1221530d in
nv50_ir::RegAlloc::BuildIntervalsPass::addLiveRange(nv50_ir::Value*,
nv50_ir::BasicBlock const*, int)
../src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp:333
#3 0x7f1d1221872e in
nv50_ir::RegAlloc::BuildIntervalsPass::visit(nv50_ir::BasicBlock*)
../src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp:686
#4 0x7f1d1215676c in nv50_ir::Pass::doRun(nv50_ir::Function*, bool, bool)
../src/gallium/drivers/nouveau/codegen/nv50_ir_bb.cpp:495
#5 0x7f1d121563ed in nv50_ir::Pass::run(nv50_ir::Function*, bool, bool)
../src/gallium/drivers/nouveau/codegen/nv50_ir_bb.cpp:477
#6 0x7f1d122262b8 in nv50_ir::RegAlloc::execFunc()
../src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp:1910
#7 0x7f1d122256b0 in nv50_ir::RegAlloc::exec()
../src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp:1849
#8 0x7f1d12226f1e in nv50_ir::Program::registerAllocation()
../src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp:1970
#9 0x7f1d1214092a in nv50_ir_generate_code
../src/gallium/drivers/nouveau/codegen/nv50_ir.cpp:1275
#10 0x7f1d1227461b in nvc0_program_translate
../src/gallium/drivers/nouveau/nvc0/nvc0_program.c:634
#11 0x7f1d12294b21 in nvc0_sp_state_create
../src/gallium/drivers/nouveau/nvc0/nvc0_state.c:620
#12 0x7f1d12294d90 in nvc0_fp_state_create
../src/gallium/drivers/nouveau/nvc0/nvc0_state.c:661
#13 0x7f1d12ad4912 in st_create_fp_variant
../src/mesa/state_tracker/st_program.c:1498
#14 0x7f1d12ad4cd5 in st_get_fp_variant
../src/mesa/state_tracker/st_program.c:1525
#15 0x7f1d12ad8252 in st_precompile_shader_variant
../src/mesa/state_tracker/st_program.c:2053
#16 0x7f1d12c9c851 in st_program_string_notify
../src/mesa/state_tracker/st_cb_program.c:185
#17 0x7f1d12d17731 in st_link_tgsi
../src/mesa/state_tracker/st_glsl_to_tgsi.cpp:7441
#18 0x7f1d12cabaf0 in st_link_shader
../src/mesa/state_tracker/st_glsl_to_ir.cpp:175
#19 0x7f1d127c85ca in _mesa_glsl_link_shader
../src/mesa/program/ir_to_mesa.cpp:3186
#20 0x7f1d1252a9f7 in link_program ../src/mesa/main/shaderapi.c:1285
#21 0x7f1d1252a9f7 in link_program_error ../src/mesa/main/shaderapi.c:1384
#22 0x7f1d1252deb3 in _mesa_LinkProgram ../src/mesa/main/shaderapi.c:1876
#23 0x403e13 in main._omp_fn.0 /home/kherbst/git/shader-db/run.c:926
#24 0x7f1d17b8b4b5 in GOMP_parallel (/lib64/libgomp.so.1+0x124b5)
#25 0x4029e4 in main /home/kherbst/git/shader-db/run.c:765
#26 0x7f1d179b51a2 in __libc_start_main ../csu/libc-start.c:308
#27 0x402d1d in _start (/home/kherbst/git/shader-db/run+0x402d1d)
0x6150003ea380 is located 0 bytes inside of 504-byte region
[0x6150003ea380,0x6150003ea578)
freed by thread T0 here:
#0 0x7f1d17e5d96f in operator delete(void*)
(/usr/lib64/libasan.so.5.0.0+0x11096f)
#1 0x7f1d1214ec0f in
__gnu_cxx::new_allocator<nv50_ir::ValueDef>::deallocate(nv50_ir::ValueDef*,
unsigned long) /usr/include/c++/9/ext/new_allocator.h:128
#2 0x7f1d1214dc00 in
std::allocator_traits<std::allocator<nv50_ir::ValueDef>
>::deallocate(std::allocator<nv50_ir::ValueDef>&, nv50_ir::ValueDef*, unsigned
long) /usr/include/c++/9/bits/alloc_traits.h:470
#3 0x7f1d1214c5fb in std::_Deque_base<nv50_ir::ValueDef,
std::allocator<nv50_ir::ValueDef> >::_M_deallocate_node(nv50_ir::ValueDef*)
/usr/include/c++/9/bits/stl_deque.h:624
#4 0x7f1d121498c4 in std::_Deque_base<nv50_ir::ValueDef,
std::allocator<nv50_ir::ValueDef> >::_M_destroy_nodes(nv50_ir::ValueDef**,
nv50_ir::ValueDef**) /usr/include/c++/9/bits/stl_deque.h:758
#5 0x7f1d1214704d in std::_Deque_base<nv50_ir::ValueDef,
std::allocator<nv50_ir::ValueDef> >::~_Deque_base()
/usr/include/c++/9/bits/stl_deque.h:680
#6 0x7f1d12145371 in std::deque<nv50_ir::ValueDef,
std::allocator<nv50_ir::ValueDef> >::~deque()
/usr/include/c++/9/bits/stl_deque.h:1069
#7 0x7f1d1213bc5b in nv50_ir::Instruction::~Instruction()
../src/gallium/drivers/nouveau/codegen/nv50_ir.cpp:615
#8 0x7f1d1213fb2f in
nv50_ir::Program::releaseInstruction(nv50_ir::Instruction*)
../src/gallium/drivers/nouveau/codegen/nv50_ir.cpp:1148
#9 0x7f1d122250fb in
nv50_ir::SpillCodeInserter::run(std::__cxx11::list<std::pair<nv50_ir::Value*,
nv50_ir::Value*>, std::allocator<std::pair<nv50_ir::Value*, nv50_ir::Value*> >
> const&) ../src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp:1830
#10 0x7f1d12221445 in nv50_ir::GCRA::allocateRegisters(nv50_ir::ArrayList&)
../src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp:1541
#11 0x7f1d122262e9 in nv50_ir::RegAlloc::execFunc()
../src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp:1913
#12 0x7f1d122256b0 in nv50_ir::RegAlloc::exec()
../src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp:1849
#13 0x7f1d12226f1e in nv50_ir::Program::registerAllocation()
../src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp:1970
#14 0x7f1d1214092a in nv50_ir_generate_code
../src/gallium/drivers/nouveau/codegen/nv50_ir.cpp:1275
#15 0x7f1d1227461b in nvc0_program_translate
../src/gallium/drivers/nouveau/nvc0/nvc0_program.c:634
#16 0x7f1d12294b21 in nvc0_sp_state_create
../src/gallium/drivers/nouveau/nvc0/nvc0_state.c:620
#17 0x7f1d12294d90 in nvc0_fp_state_create
../src/gallium/drivers/nouveau/nvc0/nvc0_state.c:661
#18 0x7f1d12ad4912 in st_create_fp_variant
../src/mesa/state_tracker/st_program.c:1498
#19 0x7f1d12ad4cd5 in st_get_fp_variant
../src/mesa/state_tracker/st_program.c:1525
#20 0x7f1d12ad8252 in st_precompile_shader_variant
../src/mesa/state_tracker/st_program.c:2053
#21 0x7f1d12c9c851 in st_program_string_notify
../src/mesa/state_tracker/st_cb_program.c:185
#22 0x7f1d12d17731 in st_link_tgsi
../src/mesa/state_tracker/st_glsl_to_tgsi.cpp:7441
#23 0x7f1d12cabaf0 in st_link_shader
../src/mesa/state_tracker/st_glsl_to_ir.cpp:175
#24 0x7f1d127c85ca in _mesa_glsl_link_shader
../src/mesa/program/ir_to_mesa.cpp:3186
#25 0x7f1d1252a9f7 in link_program ../src/mesa/main/shaderapi.c:1285
#26 0x7f1d1252a9f7 in link_program_error ../src/mesa/main/shaderapi.c:1384
#27 0x7f1d1252deb3 in _mesa_LinkProgram ../src/mesa/main/shaderapi.c:1876
#28 0x403e13 in main._omp_fn.0 /home/kherbst/git/shader-db/run.c:926
previously allocated by thread T0 here:
#0 0x7f1d17e5c9d7 in operator new(unsigned long)
(/usr/lib64/libasan.so.5.0.0+0x10f9d7)
#1 0x7f1d1215046f in
__gnu_cxx::new_allocator<nv50_ir::ValueDef>::allocate(unsigned long, void
const*) /usr/include/c++/9/ext/new_allocator.h:114
#2 0x7f1d1214ebec in
std::allocator_traits<std::allocator<nv50_ir::ValueDef>
>::allocate(std::allocator<nv50_ir::ValueDef>&, unsigned long)
/usr/include/c++/9/bits/alloc_traits.h:444
#3 0x7f1d1214dbd3 in std::_Deque_base<nv50_ir::ValueDef,
std::allocator<nv50_ir::ValueDef> >::_M_allocate_node()
/usr/include/c++/9/bits/stl_deque.h:617
#4 0x7f1d1214c464 in std::_Deque_base<nv50_ir::ValueDef,
std::allocator<nv50_ir::ValueDef> >::_M_create_nodes(nv50_ir::ValueDef**,
nv50_ir::ValueDef**) (/home/kherbst/local/lib64/dri//nouveau_dri.so+0x829464)
#5 0x7f1d121495cd in std::_Deque_base<nv50_ir::ValueDef,
std::allocator<nv50_ir::ValueDef> >::_M_initialize_map(unsigned long)
/usr/include/c++/9/bits/stl_deque.h:716
#6 0x7f1d12146f7d in std::_Deque_base<nv50_ir::ValueDef,
std::allocator<nv50_ir::ValueDef> >::_Deque_base()
/usr/include/c++/9/bits/stl_deque.h:507
#7 0x7f1d1214518d in std::deque<nv50_ir::ValueDef,
std::allocator<nv50_ir::ValueDef> >::deque()
/usr/include/c++/9/bits/stl_deque.h:912
#8 0x7f1d1213b9c9 in nv50_ir::Instruction::Instruction(nv50_ir::Function*,
nv50_ir::operation, nv50_ir::DataType)
../src/gallium/drivers/nouveau/codegen/nv50_ir.cpp:605
#9 0x7f1d1224dd44 in nv50_ir::Function::convertToSSA()
../src/gallium/drivers/nouveau/codegen/nv50_ir_ssa.cpp:385
#10 0x7f1d1224d381 in nv50_ir::Program::convertToSSA()
../src/gallium/drivers/nouveau/codegen/nv50_ir_ssa.cpp:310
#11 0x7f1d121407c0 in nv50_ir_generate_code
../src/gallium/drivers/nouveau/codegen/nv50_ir.cpp:1264
#12 0x7f1d1227461b in nvc0_program_translate
../src/gallium/drivers/nouveau/nvc0/nvc0_program.c:634
#13 0x7f1d12294b21 in nvc0_sp_state_create
../src/gallium/drivers/nouveau/nvc0/nvc0_state.c:620
#14 0x7f1d12294d90 in nvc0_fp_state_create
../src/gallium/drivers/nouveau/nvc0/nvc0_state.c:661
#15 0x7f1d12ad4912 in st_create_fp_variant
../src/mesa/state_tracker/st_program.c:1498
#16 0x7f1d12ad4cd5 in st_get_fp_variant
../src/mesa/state_tracker/st_program.c:1525
#17 0x7f1d12ad8252 in st_precompile_shader_variant
../src/mesa/state_tracker/st_program.c:2053
#18 0x7f1d12c9c851 in st_program_string_notify
../src/mesa/state_tracker/st_cb_program.c:185
#19 0x7f1d12d17731 in st_link_tgsi
../src/mesa/state_tracker/st_glsl_to_tgsi.cpp:7441
#20 0x7f1d12cabaf0 in st_link_shader
../src/mesa/state_tracker/st_glsl_to_ir.cpp:175
#21 0x7f1d127c85ca in _mesa_glsl_link_shader
../src/mesa/program/ir_to_mesa.cpp:3186
#22 0x7f1d1252a9f7 in link_program ../src/mesa/main/shaderapi.c:1285
#23 0x7f1d1252a9f7 in link_program_error ../src/mesa/main/shaderapi.c:1384
#24 0x7f1d1252deb3 in _mesa_LinkProgram ../src/mesa/main/shaderapi.c:1876
#25 0x403e13 in main._omp_fn.0 /home/kherbst/git/shader-db/run.c:926
SUMMARY: AddressSanitizer: heap-use-after-free
../src/gallium/drivers/nouveau/codegen/nv50_ir.h:648 in
nv50_ir::ValueDef::get() const
Shadow bytes around the buggy address:
0x0c2a80075420: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c2a80075430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c2a80075440: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c2a80075450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
0x0c2a80075460: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c2a80075470:[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c2a80075480: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c2a80075490: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c2a800754a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
0x0c2a800754b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c2a800754c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
Shadow gap: cc
==612087==ABORTING
v2: full rework
Signed-off-by: Karol Herbst <kher...@redhat.com>
---
.../drivers/nouveau/codegen/nv50_ir_ra.cpp | 87 ++++++++++++++-----
1 file changed, 66 insertions(+), 21 deletions(-)
diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
b/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
index d6d3e70cce6..9a106eff2d1 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_ra.cpp
@@ -295,10 +295,48 @@ private:
typedef std::pair<Value *, Value *> ValuePair;
+class MergedDefs
+{
+public:
+ std::list<ValueDef *> operator()(Value *val) {
+ std::list<ValueDef *> res;
+ res.insert(res.end(), val->defs.begin(), val->defs.end());
+ res.insert(res.end(), defs[val].begin(), defs[val].end());
+ return res;
+ }
+
+ void add(Value *val, std::list<ValueDef *> &vals) {
+ assert(val);
+ defs[val].insert(defs[val].end(), vals.begin(), vals.end());
+ }
+
+ void remove(Value *val, ValueDef *def) {
+ defs[val].remove(def);
+ }
+
+ void removeDefsOfInstruction(Instruction *insn) {
+ for (int d = 0; insn->defExists(d); ++d) {
+ ValueDef *def = &insn->def(d);
+ defs.erase(def->get());
+ for (auto &p : defs)
+ p.second.remove(def);
+ }
+ }
+
+ void merge() {
+ for (auto &p : defs)
+ p.first->defs.insert(p.first->defs.end(), p.second.begin(),
p.second.end());
+ defs.clear();
+ }
+
+private:
+ std::unordered_map<Value *, std::list<ValueDef *> > defs;
+};
+
class SpillCodeInserter
{
public:
- SpillCodeInserter(Function *fn) : func(fn), stackSize(0), stackBase(0) { }
+ SpillCodeInserter(Function *fn, MergedDefs &mergedDefs) : func(fn),
mergedDefs(mergedDefs), stackSize(0), stackBase(0) { }
bool run(const std::list<ValuePair>&);
@@ -308,6 +346,7 @@ public:
private:
Function *func;
+ MergedDefs &mergedDefs;
struct SpillSlot
{
@@ -708,7 +747,7 @@ RegAlloc::BuildIntervalsPass::visit(BasicBlock *bb)
class GCRA
{
public:
- GCRA(Function *, SpillCodeInserter&);
+ GCRA(Function *, SpillCodeInserter&, MergedDefs&);
~GCRA();
bool allocateRegisters(ArrayList& insns);
@@ -825,6 +864,8 @@ private:
SpillCodeInserter& spill;
std::list<ValuePair> mustSpill;
+
+ MergedDefs &mergedDefs;
};
const GCRA::RelDegree GCRA::relDegree;
@@ -954,12 +995,13 @@ GCRA::coalesceValues(Value *dst, Value *src, bool force)
rep->id, rep->reg.data.id, val->id);
// set join pointer of all values joined with val
- for (ValueDef *def : val->defs)
+ std::list<ValueDef *> defs = mergedDefs(val);
+ for (ValueDef *def : defs)
def->get()->join = rep;
assert(rep->join == rep && val->join == rep);
// add val's definitions to rep and extend the live interval of its RIG node
- rep->defs.insert(rep->defs.end(), val->defs.begin(), val->defs.end());
+ mergedDefs.add(rep, defs);
nRep->livei.unify(nVal->livei);
nRep->degreeLimit = MIN2(nRep->degreeLimit, nVal->degreeLimit);
nRep->maxReg = MIN2(nRep->maxReg, nVal->maxReg);
@@ -1160,10 +1202,11 @@ GCRA::RIG_Node::addRegPreference(RIG_Node *node)
prefRegs.push_back(node);
}
-GCRA::GCRA(Function *fn, SpillCodeInserter& spill) :
+GCRA::GCRA(Function *fn, SpillCodeInserter& spill, MergedDefs& mergedDefs) :
func(fn),
regs(fn->getProgram()->getTarget()),
- spill(spill)
+ spill(spill),
+ mergedDefs(mergedDefs)
{
prog = func->getProgram();
}
@@ -1258,7 +1301,7 @@ GCRA::calculateSpillWeights()
if (!val->noSpill) {
int rc = 0;
- for (ValueDef *def : val->defs)
+ for (ValueDef *def : mergedDefs(val))
rc += def->get()->refCount();
nodes[i].weight =
@@ -1360,15 +1403,15 @@ GCRA::checkInterference(const RIG_Node *node,
Graph::EdgeIterator& ei)
if (intf->reg < 0)
return;
- const LValue *vA = node->getValue();
- const LValue *vB = intf->getValue();
+ LValue *vA = node->getValue();
+ LValue *vB = intf->getValue();
const uint8_t intfMask = ((1 << intf->colors) - 1) << (intf->reg & 7);
if (vA->compound | vB->compound) {
// NOTE: this only works for >aligned< register tuples !
- for (const ValueDef *D : vA->defs) {
- for (const ValueDef *d : vB->defs) {
+ for (const ValueDef *D : mergedDefs(vA)) {
+ for (const ValueDef *d : mergedDefs(vB)) {
const LValue *vD = D->get()->asLValue();
const LValue *vd = d->get()->asLValue();
@@ -1539,6 +1582,7 @@ GCRA::allocateRegisters(ArrayList& insns)
if (prog->dbgFlags & NV50_IR_DEBUG_REG_ALLOC)
func->print();
} else {
+ mergedDefs.merge();
prog->maxGPR = std::max(prog->maxGPR, regs.getMaxAssigned(FILE_GPR));
}
@@ -1564,14 +1608,10 @@ GCRA::cleanup(const bool success)
if (lval->join == lval)
continue;
- if (success) {
+ if (success)
lval->reg.data.id = lval->join->reg.data.id;
- } else {
- for (Value::DefIterator d = lval->defs.begin(); d != lval->defs.end();
- ++d)
- lval->join->defs.remove(*d);
+ else
lval->join = lval;
- }
}
if (success)
@@ -1764,7 +1804,8 @@ SpillCodeInserter::run(const std::list<ValuePair>& lst)
// multiple destinations that all need to be spilled (like OP_SPLIT).
unordered_set<Instruction *> to_del;
- for (Value::DefIterator d = lval->defs.begin(); d != lval->defs.end();
+ std::list<ValueDef *> defs = mergedDefs(lval);
+ for (Value::DefIterator d = defs.begin(); d != defs.end();
++d) {
Value *slot = mem ?
static_cast<Value *>(mem) : new_LValue(func, FILE_GPR);
@@ -1800,6 +1841,7 @@ SpillCodeInserter::run(const std::list<ValuePair>& lst)
assert(defi);
if (defi->isPseudo()) {
+ mergedDefs.remove(lval, *d);
d = lval->defs.erase(d);
--d;
if (slot->reg.file == FILE_MEMORY_LOCAL)
@@ -1812,8 +1854,10 @@ SpillCodeInserter::run(const std::list<ValuePair>& lst)
}
for (unordered_set<Instruction *>::const_iterator it = to_del.begin();
- it != to_del.end(); ++it)
+ it != to_del.end(); ++it) {
+ mergedDefs.removeDefsOfInstruction(*it);
delete_Instruction(func->getProgram(), *it);
+ }
}
// TODO: We're not trying to reuse old slots in a potential next iteration.
@@ -1841,13 +1885,14 @@ RegAlloc::exec()
bool
RegAlloc::execFunc()
{
+ MergedDefs mergedDefs;
InsertConstraintsPass insertConstr;
PhiMovesPass insertPhiMoves;
ArgumentMovesPass insertArgMoves;
BuildIntervalsPass buildIntervals;
- SpillCodeInserter insertSpills(func);
+ SpillCodeInserter insertSpills(func, mergedDefs);
- GCRA gcra(func, insertSpills);
+ GCRA gcra(func, insertSpills, mergedDefs);
unsigned int i, retries;
bool ret;
--
2.24.1
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
