ChanServ changed the topic of #zink to: official development channel for the mesa3d zink driver || https://docs.mesa3d.org/drivers/zink.html
nsneck has joined #zink
nsneck has quit [Remote host closed the connection]
nsneck has joined #zink
<zmike> baryluk: which glmark case has the vram leak?
<zmike> baryluk: also new wip up which may fix a couple hangs
<baryluk> zmike: It was reproducing easily, but after fixing few unitialized variables, it stopped reproducing...
<baryluk> zmike: it was any case. any benchmark would leak. setting duration to 0.01 second, and letting it sit for few minutes you will see stead grow, no matter the selected benchmark.
<baryluk> ./build/src/glmark2 --data-path $(pwd)/data --benchmark shading:shading=phong:duration=0.001 --run-forever
<baryluk> for example
<baryluk> but, I have few local fixes, which maybe fixed it
<zmike> did
<baryluk> thx
<zmike> I think the other one you're hitting a compiler bug
<zmike> C99 spec requires zero-initialization of structs from {0} as best I can tell
<baryluk> anecdotal and a bit old. but something
<turol> I think "remainder of the aggregate" can be interpreted so the compiler only needs to initialize actual members and not holes
<turol> next gcc will have an option to force-initialize all stack variables
<turol> doesn't help us right now though
<zmike> :/
<zmike> so then I guess you can already guess what I'm going to say next
<baryluk> gcc does not initialize the holes
<baryluk> write proper hasher for cbvi ?
<zmike> because if it's fixed here in just this one place, that's great, but then there's still the rest of the driver...
<baryluk> it does not matter in most other places.
<zmike> there's a lot of stuff that's cached nowadays
<zmike> so evaluating all that would be beneficial
<zmike> frustrating
<zmike> surfaces, framebuffers, renderpasses, pipelines, descriptors, shaders
<baryluk> in kernel they even have a checker for leakage of struct alignment holes to user space
<baryluk> yeah. we need to check all the hash computations.
<baryluk> I can check some.
<zmike> ughhhh
<zmike> according to this though memset isn't even technically enough
<zmike> I hate compilers
<baryluk> well, memset, well. yeah, technically...
<zmike> so really, I think if we want to be Extra Sure then all the hashing functions need to be rewritten to avoid holes on all archs
<zmike> the createinfo is usually stored so the structs can re-create the members, so those have to be kept
<zmike> alternatively...
<baryluk> we can do struct xyzCreatInfo_packed __atribute__((packed)) , then copy relevant fields we want for hashing, then hash that.
<baryluk> it can be even on stack just temporarily. slower, but maybe....
<baryluk> the problem with memset is that is also not optimal.
<baryluk> it will write to some locations twice, once zero, and second time the final value. sure cpus are smart, but these are still wasted cycles.
<baryluk> I bet clang has better memset optimizations. it could probably avoid double writing, while ensuring full initialization. let me check
<zmike> here we go, this is the future we want and need https://godbolt.org/z/qbovcb4Gb
<baryluk> i told you: https://godbolt.org/z/c69ex8Wcs - clang produces identical code with memset and with {0}
<zmike> oof
<baryluk> zmike: you do not need the cast. it changes nothing. memset already casts to void* anyway.
<zmike> I'm just doing a meme
<zmike> I'd bet other mesa components are affected by this
<zmike> might be worth writing a mail to the list to see if anyone wants to come up with a unified solution
<zmike> but for now
<zmike> uhhh
<zmike> 🤔
<baryluk> Well. this function create_bvci is a little bit special, because it allocated one on stack, then returns by value. my guess a big chunk of other places use rzalloc and pass pointer.
<baryluk> i.e. I checked panfrost usage of _mesa_hash_data and that is what they do there.
<zmike> hm could be
<zmike> anyway, it's friday night, so I'm in full problem solving as fast as possible to get on the weekend mode
<zmike> there's a bunch of these types of functions around in zink
<zmike> https://godbolt.org/z/js5q7ch6q what do you think about this
<baryluk> Yeah. I just grepped. and almost half of the calls are in zink
<zmike> haha
<baryluk> zmike: sure. that works too, looks reasonably conformant. not sure about the pointer aliasing, but we would not use struct with pointers as hash input usually.
<baryluk> also the end result is same as with memset(, 0, )
<zmike> is it? should've checked that
<baryluk> byte by byte the same
<zmike> in theory though this should require all compilers to fully zero right?
<zmike> it's an array with no holes
<zmike> what I'd probably do then is make this a macro/inline function so variables can just be declared more simply like ZERO(VkBufferViewCreateInfo, bvci);