00:00 karolherbst: I guess?
00:00 jenatali: But you can't, and it's probably good that you can't
00:00 karolherbst: :D
00:00 karolherbst: now I thinking about making &dev.clc_nir; work, but that's ugly
00:00 karolherbst: but seriously.. why doesn't it work? :D
00:01 curro: karolherbst: yes i don't think that will work sorry
00:01 karolherbst: the compiler accepts "spirv_options.clc_shader = dev.clc_nir.operator->().get();"
00:01 karolherbst: ...
00:03 karolherbst: ehhh
00:03 curro: karolherbst: in your code you're returning a basic_lazy from operator->() instead of a pointer as it's supposed to
00:03 karolherbst: and T operator*() returns the shared_ptr...
00:03 karolherbst: curro: shared_ptr is legal for operator-> afaik
00:04 curro: but that's not what you're returning
00:04 karolherbst: well. I do
00:04 karolherbst: or not?
00:04 airlied: karolherbst: okay pushed things to clc_wip branch
00:04 karolherbst: I just added an operator* having the same body
00:04 karolherbst: and I get my shared_ptr
00:04 karolherbst: I think -> is just special and it follows nested object?
00:05 curro: yeah, technically operator* should compile but i find it kind of questionable honestly, because lazy isn't supposed to behave like a pointer
00:05 karolherbst: yeah...
00:05 curro: i'd rather have a function call operator if you ask me :P
00:05 karolherbst: oh well...
00:06 curro: i mean, i'd take a patch adding an operator* if it maps to operator* applied to the underlying object
00:06 karolherbst: yeah... I guess it could make sense to have some enable_if magic to go through an internal wrapper or something
00:07 airlied: seems to be compiling libclc on firat link now
00:07 karolherbst: airlied: where did you push to? :D
00:08 airlied: https://gitlab.freedesktop.org/airlied/mesa/-/commits/clc-wip
00:08 airlied: karolherbst: ^
00:09 airlied: 3 patches that I'd probably just put into jentali MR
00:09 airlied: jenatali MR
00:11 karolherbst: curro: "If a user-defined operator-> is provided, the operator-> is called again on the value that it returns, recursively, until an operator-> is reached that returns a plain pointer. After that, built-in semantics are applied to that pointer."
00:11 karolherbst: ....
00:11 jenatali: ....
00:11 karolherbst: mystery solved
00:11 jenatali: That's...
00:11 jenatali: WOw
00:11 curro: ahh, yeah
00:12 curro: so your patch was correct after all, it just won't work on a nir_shader
00:12 karolherbst: it doesn't work if you just want the damn plain pointer :p
00:12 anholt: jekstrand: got caught in refactoring the 64-bit vectorization of ntt today using the filter thing. I think I'm down to one regression.
00:12 karolherbst: but..
00:12 karolherbst: ehh.. no but
00:12 anholt:calls it quits for the day while piglit runs
00:13 karolherbst: I mean...
00:13 karolherbst: that semantic is indeed useful to just not have to deal with all the wrappers
00:13 karolherbst: just annoying in this case...
00:14 karolherbst: airlied: 0.45s now :)
00:14 karolherbst: it still feels like we could improve the linking phase
00:14 karolherbst: mhhh
00:14 karolherbst: airlied: what's your perf call?
00:15 airlied: karolherbst: just doing perf record --call-graph dwarf clinfo
00:15 airlied: needs symbols to be sensible though
00:16 karolherbst: right
00:16 karolherbst: already compiling :D
00:17 karolherbst: curro: I guess we could add another operator* for wrapped pointer types... mhhh.. but then the template magic might kill everbody reading the code
00:18 karolherbst: sane people stop reading if they see std::enable_if :D
00:19 airlied: now that I remember two lines of C++ I should go back to contributing to SYCL spec
00:19 curro: karolherbst: *shrug*, i'd be fine with an operator()() or a get() method
00:20 karolherbst: if I use magic, I want to use the full one :p
00:21 curro: lol
00:21 karolherbst: ohh.. I have a stupid idea :D
00:21 karolherbst: but that will work, I am sure
00:23 airlied: karolherbst: btw if you even vaguely happy wit those patches, I'll squash them into the clover libclc patch on the MR I think
00:25 airlied: karolherbst: I suppose if there was a disk cache for all shaders it could delay loading it even longer
00:26 karolherbst: I guess
00:26 jenatali: airlied: I'd probably keep the util patch separate, but the other two look good to squash for me
00:26 karolherbst: anyway.. most of the time is spend in libclang.. mhhh
00:26 airlied: karolherbst: embedded profile ftw :-P
00:28 karolherbst: airlied: I like hotspot btw to view perf files :p
00:29 karolherbst: the flame graph makes things easy to spot
00:30 karolherbst: but why is it that expensive to parse the source code? :D
00:30 karolherbst: oh well...
00:30 karolherbst: I guess if we start caching clcs as well clinfo will be for free
00:31 karolherbst: 75% of the time is spend on compiling this stupid kernel for workgroup sizes?
00:31 karolherbst: oh well..
00:32 airlied: the nir cache file is 600k
00:32 karolherbst: oh well
00:32 airlied: karolherbst: we could make clinfo use spirv :-P
00:35 airlied: probably not too messy to add moar disk cache
00:35 airlied: but you'd still have clang
00:35 airlied: since you'd need to cache clang->spirv I suppose
00:43 karolherbst: yeah...
00:47 airlied: jenatali: going to push it now
00:48 airlied: jenatali: okay done
00:48 jenatali: Cool
00:49 airlied:goes back to his dungeon
00:51 jenatali: airlied: Appreciate the help :)
00:51 airlied:will forget all c++ things by close of business :-P
01:11 karolherbst: ahhh my C++ fails me..
01:12 karolherbst: 5 years ago I would have succeeded
01:13 airlied: I think there was a week when I worked on triSYCL where I actaully started to feel like a c++ developer
01:26 karolherbst: ohhh.. now I am close
01:27 karolherbst: airlied: "const nir_shader *tmp = dev.clc_nir;" compiles but the conversion declaration is a bit messy :D
01:38 curro: karolherbst: ist es nicht unglaublich spät bei dir? :P
01:39 karolherbst: :D
07:29 airlied: karolherbst: built spirv-llvm-translator for rawhide, I'll likely get to packagse for f32/f33 next week
07:29 EdB: ah nice :)
10:54 daniels: MrCooper: hm, https://gitlab.freedesktop.org/mesa/mesa/-/pipelines/195789 is running on master, and has a warning about not using `workflow:rules`
10:55 daniels: MrCooper: is it deliberate that we run on master on .gitlab-ci* changes to get new container images maybe?
10:55 MrCooper: don't think that warning's relevant for us, we've created multiple pipelines for a long time :)
10:56 MrCooper: right, exactly
10:57 MrCooper: pre-merge pipeline has the same warning: https://gitlab.freedesktop.org/mesa/mesa/-/pipelines/195778
11:19 _julian: hi, anyone around having seen GL output using v3d drivers on Raspberry Pi 4 being corrupted like seen here: https://www.raspberrypi.org/forums/viewtopic.php?t=267480
11:20 _julian: I do experience this behavior on a yocto-built system. Replacing parts of the system with Raspbian components indicated that the issue is triggered by Xorg. If I start Xorg from a raspbian chroot, glxgears works just fine, running from my normal rootfs
11:21 _julian: I aligned Xorg versions between both systems, but couldn't make it work so far - and I'm actually quite puzzled about what would cause this kind of corruption
11:21 _julian: and why the X server itself would trigger this
11:23 Vanfanel: emersion: is it theorically possible to scale a 640x480 buffer to a 1360x768 PRIMARY plane? I am trying to so so by setting the PRIMARY PLANE props like this: SRC_W 640<<16; SRC_H 480<<16; SRC_X 0; SRC_Y 0; CRTC_W 1360; CRTC_H 768; CRTC_X 0: CRTC_Y 0; ...But the result is that I can only see upper part of the buffer, stretched horizontally.
11:23 Vanfanel: emersion: so is a PRIMARY plane even suitable for this kind of arbitrary scaling at all or an OVERLAY plane is needed?
11:25 vsyrjala: sounds like a broken driver
11:26 Vanfanel: pq: I could also use some of your input as always :)
11:26 emersion: some drivers can do it, some drivers can't
11:26 emersion: you can find out with test-only atomic commits
11:26 emersion: but the result you're seeing here sure sounds like a broken driver
11:26 Vanfanel: emersion: the problem is that I see the same in AMDGPU and VC4...
11:27 pq: broken driver
11:27 emersion: maybe they both ignore SRC_* props?
11:27 emersion: or CRTC_* props
11:28 pq: just make sure you don't have other bugs in your code
11:28 emersion: would be nice to get a drm_info log, to make sure you're setting things up properly
11:39 Vanfanel: emersion: found the error... I was using total video mode size for the buffer size, instead of the window size (640x480 in this test)
11:40 Vanfanel: pq: of course in the end it was ME :D
11:40 emersion: ah, that makes sense
11:41 Vanfanel: thanks guys, for listening me and giving me feedback. Without your reading and words, I would not have thought of the solution
11:41 Vanfanel: thanks, really :)
11:43 pq: heh
12:51 Vanfanel: emersion: According to your excellent database, in https://drmdb.emersion.fr/properties/4008636142/CRTC_W it seems that CRTC_W/H range is 0,INT32_MAX. So is "int32_t" better suited than "int" or "Duint32_t" for CRTC_W/H props?
12:52 emersion: hm, maybe
12:52 emersion: props are 64-bit values
12:52 emersion: (you've probably noticed you need to convert to uint64)
12:52 Vanfanel: emersion: but if the range is 0-INT32_MAX, then only positive values make sense, right?
12:53 emersion: e.g. CRTC_X has range [INT32_MIN, INT32_MAX]
12:53 emersion: using a signed value may prevent some weird underflow issues, if you're doing substractions for instance
12:53 emersion: (make them easier to debug, not prevent them, actually)
12:54 emersion: yeah, so i'd probably use int32_t here, but i don't think it's that of a big deal
12:56 pq: CRTC_X can also be negative, e.g. for a cursor plane part off screen, I believe
12:56 emersion: yes
12:57 Vanfanel: emersion, pq: what about uint32_t for CRTC_W/H, SRC_W/H, and int32_t for CRTC_X/Y, SRC_X/Y ?
12:58 emersion: that's fine, but sometimes uint is a footgun
12:59 emersion: notable example: for (unsigned i = 42; i > 0; i--)
12:59 emersion: that's what i mean by "may prevent some weird underflow issues, if you're doing substractions for instance"
12:59 pq: you mean i >= 0?
12:59 emersion: er, yes
13:00 emersion: i > 0 works fine
13:00 emersion: i >= 0 doesn't
13:01 emersion: but yeah, uint32 would be fine (note, uint32_max > int32_max)
13:01 pq: C has rules for implicit integer promotion when you compute with signed and unsigned types. Sometimes it is what you want, sometimes it's not. And to know if it's right, you have to know the C rules. That's a lot to remember and consider.
13:03 Vanfanel: pq, emersion : Then it seems better to just use uint32_t and just forget about it...
13:03 Vanfanel: uint32_t for all
13:03 emersion: no no :P
13:03 emersion: uint32 isn't suitable for x/y
13:04 Vanfanel: ah, well, yes, negative coords
13:04 Vanfanel: sorry
13:04 emersion: int32 for everybody would be safer
13:04 emersion: and won't trip on someone trying to use negative coords
13:05 Vanfanel: emersion: ok, thanks
13:05 emersion: i hope the kernel returns a failure if you try to set a negative CRTC_X
13:05 emersion: err
13:05 emersion: i hope the kernel returns a failure if you try to set a negative CRTC_W
13:05 emersion: hm CRTC_W is an unsigned range
13:06 emersion: i suppose you'd hit the [int32_max, uint32_max] part
13:07 emersion: or rather somewhere in [int32_max, uint64_max]
13:50 DPA: With "struct hash_table" in mesa, is there a way to use it with a pair of file descriptors as key?
14:26 seanpaul: mlankhorst, mripard: any interest in "[PATCH v6 00/14] drm/trace: Mirror DRM debug logs to tracefs"? it's been around the list for a while, but hasn't really gone anywhere
14:27 seanpaul: (not sure what Thomas' nick is)
14:32 mripard: seanpaul: I've followed that with attention, but I don't really feel like I know ftrace enough to tell if the patches are sane or no
14:32 mripard: *not
14:32 mripard: but if they are, I'm all for it
14:33 seanpaul: no promises :-)
15:43 sravn_: seanpaul: Thomas is known as tzimmerman, but offline atm
15:54 jekstrand: jenatali, bbrezillon: I think I'm finally getting close to having the alignments branch passing in GL/Vulkan and working ok for CL.
15:55 jenatali: \o/
15:55 jekstrand: Some of the "add alignments to types" stuff was breaking Vulkan
15:57 bbrezillon: jekstrand: yay!
15:57 jekstrand: bbrezillon: One of the changes I had to make was to say that scalars never have explicit alignments
15:58 jekstrand: I may still need to tweak some stuff for matrices. Still thinking about that.
15:58 bbrezillon: makes sense
15:58 bbrezillon: though I don't understand why it breaks when you add an explicit alignment to those
15:58 jekstrand: Because a vector type doesn't have a concept of the scalar type which is its child
15:59 jekstrand: So when we do glsl_get_scalar_type() on it, we can't return an explicitly created thing; we have to start making assumptions.
15:59 jekstrand: Same for matrices and rows/columns
16:00 jekstrand: But I think I can sorth that one out by saying "matrices are like arrays of columns; the alignment of the matrix type is the same as its column type"
16:00 bbrezillon: but you still need an explicit alignment on vectors, right?
16:00 jekstrand: Yes
16:01 jekstrand: I can make the matrx -> column vector work by assuming matrices and their columns have the same alignment like arrays.
16:01 jekstrand: But making vector -> element work with an actual alignment requires more assumptions and I'm not sure it works.
16:08 jekstrand: Matrices don't affect CL, though, fortunately.
16:08 _julian: :q
16:09 bbrezillon: yep, but vectors do :)
16:09 jekstrand: Yeah
16:09 jekstrand: I'm also mentally debating if we want to add some mechanism for Vulkan drivers to add alignment information.
16:09 jekstrand: SPIR-V doesn't provide it but the driver might be able to based on what features are supported and/or enabled.
16:09 bbrezillon: I'll have to look at what you've done 'cause I don't quite get why you call get_scalar_type() to get the explicit alignment of a vector
16:10 jekstrand: bbrezillon: Where do I do that?
16:12 jekstrand: bbrezillon: That sounds very much like a bug I fixed recently. :)
16:12 bbrezillon: not saying you do that
16:13 bbrezillon: just trying to understand what the problem is with vectors that have an explicit alignment :)
16:13 jekstrand: They're fine
16:13 jekstrand: the problem is scalars having explicit alignment
16:15 jekstrand: If I have a i32vec4 with a 16B alignment and someone calls get_scalar_type(), what should the alignment of the scalar be?
16:15 bbrezillon: hm, it's the same constructor for matrices, vectors and scalars?
16:15 jekstrand: It is
16:16 jekstrand: It's not like SPIR-V types where matrices are a composite of vectors which are a composite of scalars
16:16 bbrezillon: ah, forgot that scalars were mixed with vectors and matrices
16:17 jekstrand: If we just assume scalars are always naturally aligned, everything works out ok.
16:17 jenatali: jekstrand: Probably worth asserting that vector explicit alignment is a multiple of the corresponding scalar alignment?
16:17 bbrezillon: can't we just make sure explicit_alignment is 0 in that case
16:17 jenatali: Should always either be 1, N, or 4 (for vec3)
16:17 bbrezillon: :)
16:17 jekstrand: jenatali: Sure. That seems reasonable.
16:37 jekstrand: pendingchaos: Any reason to not merge your availability/visibility series?
16:41 yshui`: I was wondering how the mesa xcb-only EGL implementation could work with dri2. Since GLX needs to use XESetWireToEvent/EventToWire when running on dri2.
16:42 jenatali: jekstrand: Address modes for images are all messed up :D
16:43 yshui`: looks like it just pessimistically mark the surface as invalid?
16:43 pendingchaos: jekstrand: !6090? I don't think so
16:43 pendingchaos: I'll run CTS and then merge it
16:44 jekstrand: pendingchaos: Thanks! I gave you reviews on everything except the tests which I acked.
16:49 jenatali: jekstrand: The derefs type for images/samplers are all done using the address format for vtn_variable_mode_function. When they're ready to be used as input to a tex/intrinsic, they're cast to nir_var_uniform derefs
16:49 jenatali: jekstrand: But since the StorageClass for them in SPIR-V is UniformConstant, that'll always be a non-trivial cast, since the deref would either be constant/ubo/global (depending on which patches you have)
16:50 jenatali: At least for CL - for Vulkan, UniformConstant ends up mapping to uniform
16:52 jekstrand: jenatali: Unltimately, I think we have two options:
16:52 jekstrand: 1) Make NIR treat shader_in for kernels like uniform
16:52 jekstrand: 2) Make spirv_to_nir use nir_var_uniform for kernel inputs
16:53 jenatali: Right
16:55 jenatali: Alright, let me play around with this some more
16:55 jenatali: I think 2) is probably the right approach
16:56 jenatali: I'm just getting myself back up to speed after looking at other things all week
16:56 jekstrand: Fair enough. :)
16:58 jenatali: jekstrand: Remind me, what's the status of your mem_constant series?
16:59 jekstrand: jenatali: I need to rework some things
16:59 jekstrand: jenatali: I'm trying to get alignments and packed in good shape first.
16:59 jenatali: Ok, no worries
16:59 jenatali: Was just wondering since it was related
17:00 jekstrand: Yeah
17:00 jekstrand: I'm hoping to get both landed early next week and then revive generic
17:12 jekstrand: pendingchaos: Looks like your unit tests for alignments are causing CI heartburn
17:12 jekstrand: pendingchaos: https://gitlab.freedesktop.org/mesa/mesa/-/jobs/4308131
17:12 jekstrand: pendingchaos: It's the usual "C++ is stupid and doesn't allow designated initializers" issue.
17:13 jekstrand: pendingchaos: Also, we aparently don't support big-endian eiter: https://gitlab.freedesktop.org/mesa/mesa/-/jobs/4308144
17:14 jekstrand: pendingchaos: For that one, I'm inclined to just not run the CTS tests
17:14 jekstrand: Unsupported extension: LSLGdts.054.
17:14 jenatali: Amazing
17:18 karolherbst: :D the heck
17:19 karolherbst: why am I laughing.. I might have to deal with such issues in the future...
17:19 jekstrand: It's not hard to fix, we've just never had to bother.
17:20 dcbaker[m]: jekstrand: c++20 has c designated initializers
17:20 jekstrand: dcbaker[m]: At least there's one good thing about 2020!
17:20 jekstrand: Too bad we won't be able to rely on it until 2030. :-(
17:21 karolherbst: dcbaker[m]: cool, +1 for requiring c++20 then :p
17:21 dcbaker[m]: I think you're being optimistic about 2030
17:21 karolherbst: hey.. we are at c++14 already
17:22 dcbaker[m]: Because llvm requires it
17:22 karolherbst: and because msvc cares about c++
17:23 dcbaker[m]: Msvc 2019 has some c11 support, actually
17:23 karolherbst: oh wow
17:23 karolherbst: "some" :p
17:23 karolherbst: that's the problem
17:24 dcbaker[m]: It's a WIP afaiu
17:24 karolherbst: I mean.. they always had some support for c11
17:24 karolherbst: I don't think they will care unless c++ requires those features :p
17:25 dcbaker[m]: But also, I think people still need msvc 2015 and 2017 to work, so...
17:40 jekstrand: pendingchaos: I think I have a patch to fix bit-endian
17:41 jekstrand: It's been on the todo list of pointless things we should probably implement in the SPIR-V parser for a while. :-/
17:51 anholt: daniels: 3rd windows fail in a row https://gitlab.freedesktop.org/mesa/mesa/-/jobs/4310498
17:53 pendingchaos: jekstrand: "not run the CTS tests": do you mean not run the src/compiler/spirv/ tests on big endian?
18:04 anholt: I just found out: for people who have had trouble with tab completing names starting with "a" in hexchat picking someone arbitrary, /set completion_amount 1 will give you a list of names instead.
18:07 daniels: anholt: oosh :( filing MR to disable now. thanks for letting me know!
18:07 jekstrand: pendingchaos: That's an option
18:09 jekstrand: pendingchaos: I think I've got big-endian just about fixed for strings
18:09 jekstrand: pendingchaos: Sadly, it takes a CI run in GitLab for me to actually test it. :-/
18:11 daniels: anholt: !6495 filed, but about to disappear into the tube and then out for the night - can you please shepherd it through if required?
18:15 jekstrand: pendingchaos: Looks like my latest version works on s390x
18:17 jekstrand:is probably going to get a phoronix article about adding big-endian support to the SPIR-V parser with lots of speculation about Intel's future graphics plans.
18:22 ccr: :D
18:24 ccr: do not forget about people wondering/hoping that if the changes could magically make their pre-broadwell gpus work with Vulkan.
18:24 ccr: in the comments
18:25 chrisf: jekstrand: did you find a BE SPIRV module in the wild?
18:26 jekstrand: chrisf: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6472/diffs?commit_id=c5ceb6f09ba1c36dcc4715d68f41a8fe0d8a0ae4
18:27 jekstrand: chrisf: Means we'll likely going to be the only driver stack capable of properly parsing BE SPIR-V. 🤣😭
18:33 chrisf: i think we'd probably be ok on swiftshader. we throw it through spirv-tools first and it makes a decent attempt at dealing with endianness
18:39 jekstrand: Is anyone testing that decent attempt? :-P
18:43 mareko: can somebody please review this NIR optimizations? https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6283
18:55 chrisf: jekstrand: no :)
18:59 jenatali: jekstrand: Switching shader_in => uniform means running lower_explicit_io on uniform, which means that images have to be lowered away from derefs before that, since images are uniforms, otherwise the deref gets turned into a memory location
18:59 jenatali: Yet another rework of our backend, where we let image deref intrinsics straight into the nir->dxil step
19:00 jekstrand: jenatali: 😭
19:02 jenatali: jekstrand: It's not too bad though
19:06 anholt: MrCooper: since you understand the pipeline rules better, on https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6495 it looks like the #195948 pipeline didn't get triggered (I just clicked it) and marge might have been waiting for i.? didn't you have stuff set up to try to get one pipeline in the MR for a given commit?
19:08 anholt: marge says:
19:08 anholt: 2020-08-28 18:10:27,430 WARNING No pipeline listed for b8f2d71c4045e1ddb630c2f989e4b397b5b97099 on branch ci-windows-reenable
19:08 anholt: 2020-08-28 18:10:27,430 WARNING Suspicious CI status: None
19:08 anholt: 2020-08-28 18:10:48,054 WARNING Suspicious CI status: 'created'
19:08 anholt: 2020-08-28 18:10:58,192 WARNING Suspicious CI status: 'manual'
19:08 anholt: then looped printing the manual line
19:18 anholt: daniels: thanks, btw. and shepherding now done.
19:58 jekstrand: Uh...
19:58 jekstrand: ERROR: Uploading artifacts to coordinator... error error=couldn't execute POST against https://gitlab-ci-packet-new-dfs.freedesktop.org/api/v4/jobs/4312844/artifacts?artifact_format=zip&artifact_type=archive: Post https://gitlab-ci-packet-new-dfs.freedesktop.org/api/v4/jobs/4312844/artifacts?artifact_format=zip&artifact_type=archive: dial tcp: lookup gitlab-ci-packet-new-dfs.freedesktop.org on
19:58 jekstrand: 147.75.207.208:53: server misbehaving id=4312844 token=Gsu-DJht
20:02 jenatali: jekstrand: Managed to get CLOn12 working (images and all) using uniform for kernel inputs
20:02 jekstrand: \o/
20:02 jenatali: It's not too bad, didn't actually require many changes to core nir
20:22 ajax: ugh it is absolutely impossible to work on the glx client library without wanting to rewrite everything
20:31 jenatali: jekstrand: It was actually even less than I thought... for some reason I thought SPIR-V's StorageClassInput would get used, but no, it's completely vtn that produces shader_in out of nowhere
20:31 jenatali: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6497
20:32 dcbaker[m]: ajax: next year's xdc had a talk for you about it :)
20:33 ajax: dcbaker[m]: mmm?
20:34 dcbaker[m]: Wow I butchered that, lol
20:35 dcbaker[m]: Next year's xdc has a slot for you to talk about rewriting the glx client library
20:37 ajax: heh, sure why not
20:39 anholt: I wish capturing little videos on the desktop was easy, because I just made the best looking glxgears bug I've ever done.
20:40 DrNick: ctrl-shift-alt-r
20:40 ajax: yeah, that's whole desktop though
20:40 ajax: single window would be nice
20:41 ccr: "recordmydesktop" can be specified a window id at least
20:48 jekstrand: jenatali, bbrezillon: Dropped the WIP from my alignments MR. I think I'm pretty happy with it now.
20:49 jenatali: jekstrand: Cool, I'll probably get a chance to try it out on Monday
20:49 jekstrand: Sounds good.
20:49 jekstrand: cmarcelo: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6472
20:50 jekstrand: cmarcelo: You're one of the best equipped people to review
20:50 jekstrand: cmarcelo: Heads up: The first dozen or so are from pendingchaos and in a different MR.
21:37 jenatali: jekstrand: I think the CL images bits are good at this point
21:37 jekstrand: \o/
21:37 jekstrand: Is Gitlab struggling for anyone else?
21:38 jekstrand:is having internet troubles
21:38 jenatali: There's one outstanding comment you left, if you wouldn't mind reading my rebuttal? And if you're okay with it I think it's good to go
21:38 jenatali: Working fine for me...
21:39 jekstrand: restarted firefox. THat seems to have fixed it
21:54 jekstrand: jenatali: images still looks good to me. I gave it one more skim.
21:54 jekstrand: jenatali: There's a part of me that thinks that the uniform change should land first, logically.
21:55 jekstrand: But since clover doesn't do images, it probably doesn't matter.
21:55 jekstrand: jenatali: The one thing I don't like is the way constant samplers work.
21:55 jekstrand: jenatali: Not the variable; that's fine.
21:55 jekstrand: There's some serious vtn auto-conversion magic happening in there. It works but it bothers me a bit.
21:58 jekstrand: I don't know if it makes me happy that vtn is that flexible or scared that it's going to break.
21:59 tanty: mmm ... maybe already mentioned or I'm doing something wrong but it seems the "meson-testing" CI job is now generating 0bytes files for several of the driver under /install/lib/dri (?)
22:00 tanty: "kms_swrast_dri.so", "radeonsi_dri.so", "virtio_gpu_dri.so"
22:00 tanty: did something change in the CI or in meson recently?
22:04 jenatali: jekstrand: Yep, agreed on all points :)
22:05 jenatali: Your "vtn auto-conversion magic" is around the fact that we're using pointer-to-function-local-variable as the glsl type for the deref?
22:06 jenatali: jekstrand: Before the uniform change, we had a pass to rewrite modes on images/samplers to uniform so that the uniform deref_cast vtn inserts became trivial and got removed; but with the uniform change that whole thing goes away and normal opts simplify the deref chain for us
22:10 jekstrand: jenatali: Yeah, when the sampler is accessed, vtn_get_sampler calls vtn_get_nir_ssa -> vtn_get_ssa which -> vtn_pointer_to_ssa -> vtn_pointer_to_deref -> vtn_nir_deref_pointer_dereference which constructs a nir_deref_type_var for you.
22:12 jenatali: Yep. I'm not sure I'm seeing a problem though, isn't that just like any other pointer to a variable?
22:13 jekstrand: jenatali: It is
22:13 jekstrand: jenatali: But every other thing that produces an OpTypeSampler produces a vtn_value_type_ssa not a vtn_value_type_pointer.
22:13 jekstrand: But it totally works because vtn auto-converts like mad.
22:14 jenatali: Oh, sure
22:14 jekstrand: I think it's fine
22:14 jekstrand: It just makes me a tad nervous
22:17 jenatali: jekstrand: Makes sense
22:18 jenatali: Anything in particular I should wait for before Marge? More review, or the uniforms patch? Or should I just go for it
22:19 jekstrand: jenatali: Maybe the uniforms patch.
22:19 jekstrand: jenatali: But you can probably marge now
22:19 jenatali: Eh, no particular rush, I'll wait for that one to go first
22:20 jekstrand: ok
22:20 jenatali: I've been too fast at least twice already, I'll play it safer this time :)
22:24 jekstrand: heh
22:24 jekstrand: You'll get the hang of it.
22:25 jekstrand: A bunch of the CL stuff went from sitting around going nowhere to suddenly landing rapid-fire.
22:30 jenatali: Yeah, it's pretty exciting :)
22:30 jekstrand: jenatali: If you're bored and want something to do while you wait, you can review deref alignments. :-P
22:31 jenatali: :P
22:31 jenatali: I've already skimmed it, I should give it a proper review
23:08 jenatali: jekstrand: (or anyone I guess?) I don't entirely understand the point of the array_wildcard deref type... I think I just don't understand what it's supposed to do
23:26 imirkin: anholt: https://cgit.freedesktop.org/mesa/mesa/commit/?id=ca73c3bc596eae86aaeb03d0064568e8c0540e07 -- what was the warning? it looks like unreachable is for _truly_ unreachable code, no? whereas a bug could easily cause this to get reached, leading to essentially an infinite loop?
23:27 imirkin: [actually that unreachable may be accurate for the first 2 things that got fixed up]
23:27 imirkin: [so i'm mostly looking at the last one]
23:52 anholt: imirkin: are you saying that your assert(0) should have been reachable other than through a code bug elsewhere that you were asserting against? because you go on to use the undefined subOp value, so you've gone from undefined behavior to undefined behavior.
23:53 imirkin: anholt: no, i mean a code bug elsewhere
23:53 imirkin: but one is a slightly wrong shader, another is an application hang
23:54 imirkin: i fully agree that the code *should* not be reached, assuming everything is working as expected
23:54 anholt: right. and if you want to be more defensive than that, then assert() is not the way -- the unreachable is the same assert plus silencing the compiler warning.
23:56 imirkin: anholt: unreachable is an infinite loop after the assert
23:56 imirkin: (potentially)
23:56 imirkin: so if you're in opt mode, application hangs
23:57 imirkin: in debug mode, yeah, it's the same, you hit the assert either way
23:57 anholt: so you're concerned that you had some limited scope of undefined behavior and we may have expanded the scope of undefined behavior?
23:58 imirkin: the undefined behavior previously was "shader is wrong". the undefined bheavior now is "application hangs"
23:58 imirkin: in a perfect world, the undefined thing never happens of course :) and the assert helps us locate those issues.
23:59 imirkin: [and a sufficiently wrong shader can lead to hangs too, of course.]