00:00airlied: the chase derefs across casts and write const string to buffer are the bits that probbably have other use cases
00:00airlied: I'm 99% sure I've written a deref var chaser that jumps casts before
00:04curro: jenatali, jekstrand: yeah im okay with things, didnt ask for any conditions for it to be merged. but yeah it would be great to try to deduplicate things some more as follow up
00:04jekstrand: curro: Ok, cool. jenatali filed an MR with all the requested things to do later.
00:06jenatali: curro: Thanks :) I'd love some help pruning the AMD native IR stuff so I don't have to worry about that on Windows, then I can tackle the MSVC stuff and start refactoring more
00:06jenatali: And with that... here goes nothing I guess
00:07kusma: Exciting :-)
00:07curro: good ill take a look when im back home need to grab groceries
00:07jenatali: jekstrand: I added my r-b to your pass rewrite btw, thanks again
00:09jekstrand:reads printf code
00:09jekstrand:sees the struct variable and goes "Wha? Whose rubbish idea was this?!?"
00:09jekstrand:remembers it was his idea....
00:09jenatali: airlied: After !7565 lands, if you want to rebase (or want me to do it) I can start looking at hooking up your new printf ABI to the runtime
00:09jenatali: jekstrand: Have you had a better idea since then? :P
00:10jekstrand: jenatali: Nope. :)
00:10jekstrand: jenatali: But I've got an even more rubbish idea.....
00:10jekstrand: jenatali: We could encode the printf format string in the struct name. :)
00:10jekstrand: i.e. name the struct "printf:<format_str>"
00:11jenatali: jekstrand: I'm not sure if I like that idea or hate it with a passion :P
00:11jekstrand: jenatali: That is the correct response, I believe. :)
00:14kusma: Using the name as arbitrary payload, how could this possibly go wrong?
00:17dcbaker[m]: `printf("rm -rf /")` anyone?
00:25jekstrand: jenatali: I'm really wondering if we don't want nir_instr_type_printf.
00:25jekstrand: jenatali: I really hate that idea
00:25jekstrand: I don't want to add that much to NIR just to support this stupid thing.
00:25jekstrand: But ugh...
00:25jenatali: jekstrand: I think the decision to be made is what's going to hold back NIR the least. This can be an ugly thing off in the corner as long as it doesn't get in the way
00:25jekstrand: It's just so totally alien
00:25jenatali: Yeah, a variadic instruction with tons of args
00:25jekstrand: jenatali: Yeah. The problem is that it's *really* ugly.
00:25jekstrand: We have variadic things. Calls, tex, etc.
00:25jekstrand: And it's not just that it has tons of args. It has multiple types of args.
00:25jenatali: Right, but each one is its own instruction type
00:25jekstrand: In particular, it has int/float args and it has string args and they're totally different things.
00:25jekstrand: It's the string args that are threatening to push me over the edge.
00:25jenatali: And even float is weird in that it probably got converted to double which you almost definitely didn't want/need
00:33airlied: jekstrand: not sure we need the format string in names anyways, since it never goes into nir now, we parse it out at vtn stage, nir only ever really sees the index
00:36airlied: jekstrand: not sure about parsing the format string in vtn, maybe that is the only way to know for sure something is %s and heuristics will get caught out
00:41jekstrand:wishes we could keep using the "return -1" approach. :P
02:27airlied: jenatali, jekstrand : technically I don't think converting %p to strings is violating the spec
02:27airlied: " The argument shall be a pointer to void. The pointer can refer to a memory region in the global, constant, local, private, or generic address space. The value of the pointer is converted to a sequence of printing characters in an implementation-defined manner."
02:27airlied: implemenation defined seems a pretty broad statement :-P
02:50airlied:writes a multi arg printf test and isn't surprised it explodes
02:51Sachiel: explosions make everything more fun
03:03jekstrand: airlied: yeah....
03:24airlied: jekstrand: I might look at parsing the printf str in vtn, I wonder if I can use similiar code as in clover
05:06jekstrand: airlied: You should be able to look for %[.0-9<other stuff>]s
05:50airlied: jekstrand: if you think reviewing my nir code is bad, you don't want to invite me anywhere near a regular expression :-P
05:50airlied:envisages a perl callout :-P
05:53airlied: okay strstr here we come
06:21airlied: oh wow it wasn't horrid
06:33airlied: okay %p works gain
06:35airlied: jekstrand, jenatali : okay now with format string parsing shared with clover
06:54airlied: jekstrand, jenatali : we were lowering printf before explicit sizing, which was why those values were filled out
06:56airlied: and of course moving printf lowering later blows things up
07:17airlied: jekstrand, jenatali : uggh vtn_opencl.c:handle_instr only handles 5 src, which is kinda limiting for printf testing :-P
07:19jekstrand: airlied: I wondered about that.
07:19jekstrand: airlied: printf needs to be able to handle arbitrarily many sources.
07:20airlied: yeah I'll have to fix the handling to malloc above a certain size
07:20airlied: jekstrand: how against the explicit struct sizing in vtn are you?
07:20airlied: because I think changing the pass ordering will have a lot of fallout
07:20jekstrand: airlied: Or make `handle_printf` take opcode and const uint32_t*
07:21airlied: I've pushed a reworked pass ordering to the MR
07:21jekstrand: airlied: Why do we need explicit sizing?
07:21jekstrand: airlied: I don't see how it buys us anything whatsoever.
07:21airlied: jekstrand: the lowering passes uses it
07:21airlied: to work out where in the ssbo to put things
07:22airlied: and that lowering pass happens before we call vars_to_explicit
07:24jekstrand: airlied: Ugh... Yeah...
07:24jekstrand: airlied: This is a problem I've come across with my ANV kernel support for BVH building.
07:24jekstrand: We really want to assign explicit types very early but assign variable locations later after we dead-code a bunch.
07:25airlied: so we should split the pass?
07:26jekstrand: Yeah, we probably should.
07:27jekstrand: I've just not liked any of my attempts at doing so.
07:27jekstrand: What I'm doing now in ANV which is totally gross is running vars_to_explicit once and then, later, stomping shared_size and scratch_size to 0 and running it again.
07:27jekstrand: It's terrible
07:28jekstrand: And I'm very ashamed of what I've done. :)
07:28jekstrand: airlied: Part of what makes this a mess is that we don't have a solid concept of a type's size.
07:29jekstrand: For scalars, vectors, and arrays, it's pretty obvious.
07:29jekstrand: For structs, less so.
07:29jekstrand: We've got glsl_type::explicit_size but I don't really like it.
07:29airlied:adds back explicit as reordering the pases broke a bit
07:29jekstrand: I kind-of want struct types to have an actual size that's baked into the type.
07:30airlied: is that one of those easy for CL, messy for GL type situations
07:30jekstrand: I don't think so
07:31jekstrand: Honestly, we could probably use glsl_type::explicit_size() as-is.
07:31jekstrand: There are just annoying corners
07:31jekstrand: Which structs having an actual size would fix, I think.
07:32jekstrand: In any case, I don't think any of that's really necessary for splitting the type/location assignment in two.
07:32jekstrand: I'm just complaining. :)
07:32airlied:will take a look tomorrow if I can do anything, needs to fix printf multi args as weel
07:33jekstrand: But, generally, we want to make explicit types very close to when we come out of spirv_to_nir and assign locations after optimization.
07:33jekstrand: That's the rub I ran into.
07:33jekstrand: And I think the same roughly applies to the printf situation.
08:34pepp: idr: got it, thanks
09:12pq: When should one use drmModeGetConnector() instead of drmModeGetConnectorCurrent()?
09:14MrCooper: pepp: looks like you accidentally pushed to your optimize_dlist branch
09:15emersion: pq: drmModeGetConnector polls, drmModeGetConnectorCurrent doesn't
09:15emersion: some connectors only show up as "connected" when polled
09:16emersion: (e.g. old VGA, or bad cables where HPD is disabled)
09:16pq: emersion, that I know, but why/when do I want to poll?
09:16pepp: MrCooper: oh right - that's because I also test the 3 commits on top of this branch. Thanks
09:17pq: Should I always poll all VGA type connectors?
09:17emersion: would be nice to have an async API to poll btw
09:17emersion: i wouldn't try to decide whether to poll depending on connector type…
09:17pq: If polling is necessary, does it mean that hotplug event will not come without polling first?
09:18pq: emersion, right, so how I decide whether to poll or not?
09:19pq: I'm trying to figure out if it is safe for weston to stop polling, and when it is safe.
09:19emersion: if you don't poll: when plugging a screen, the kernel won't be aware that the connector is plugged in, and will report it as disconnected
09:19emersion: ^ cc danvet
09:20pq: so, does that mean that on start-up, I should poll everything, but on hotplug events, I should not poll?
09:20emersion: but honestly… i'm starting to wonder whether polling is really a good idea
09:20pq: right ;-)
09:20emersion: if anything, users can run a tool that does the polling, and the kernel should then trigger uevents if necessary
09:21emersion: e.g. drm_info will poll
09:21pq: users can?
09:21pq: polling is not privileged?
09:21emersion: (users used to run `xrandr` to poll)
09:21pq: xrandr tells Xorg to poll, so
09:21emersion: hm, i don't think so, because drm_info is slow :P but haven't checked
09:23pepp: MrCooper: fixed both branches
09:24pq: Doesn't polling on VGA potentially cause a visible glitch on screen? :-)
09:25emersion: reading the kernel source, it doesn't seem like polling is privileged
09:26emersion: drm_mode_getconnector calls fill_modes when count_modes == 0
09:26pq: btw. wasn't there also a sysfs file to trigger a poll?
09:26emersion: drm_helper_probe_single_connector_modes will update the connector's status
09:26danvet: mupuf, if you can pls close the MR for this modesetting hack
09:26emersion: many drivers set fill_modes to drm_helper_probe_single_connector_modes
09:27danvet: pq, poll when the user is fed up and fires up the manual configuration tool
09:27emersion: pq: that does ring a bell
09:27danvet: which is also not perfect, because manual polling might wreck the display
09:27danvet: the kernel should auto-poll for you as much as is reasonable in the background
09:27pq: danvet, ...which weston does not have. Is that the only case where poll is needed?
09:28danvet: any additional stuff we do with full probe is potentially disruptive
09:28danvet: as in, screen flicker
09:29pq: danvet, cool, so for a display server it is totally fine to never poll itself?
09:29mupuf: danvet: done, thanks :)
09:29danvet: pq, hm I think most kms drivers suck at boot-up
09:29danvet: and rely on fbcon doing the first poll
09:29danvet: so if that's not enabled, you'd need to poll when the server starts
09:29danvet: which is nasty, because it's expensive, so fairly big delay at startup :-/
09:29pq: danvet, isn't that a driver bug?
09:30emersion: danvet: any way to make polling async?
09:30pq: right now, weston always polls everything both on start-up and on hotplug events. I'm looking to change that to speed things up.
09:30emersion: "please poll in the background and trigger a uevent if something changes"
09:30pq: emersion, he said kernel already does polling in the background
09:30danvet: pq, well yes
09:31danvet: similar after resume
09:31danvet: the trouble is, as long as many drivers are buggy compositors do full probe
09:31danvet: which means driver bugs are papered over
09:31danvet: well for hw that needs polling
09:31danvet: the remaining gaps boil down to driver bugs
09:31emersion: i'm not too afraid about saying to my users "report kernel bug plz"
09:32pq: danvet, I'm perfectly happy to "break Weston" and blame drivers, if that is justified. :-)
09:32emersion: especially when there's an easy workaround
09:32emersion: so tl;dr never poll and we should be fine?
09:32danvet: I think so
09:32pq: danvet, thanks a lot, I have a contributor who wants to write the weston patch to never poll again.
09:33danvet: emersion, maybe kernel uapi doc in the getconnector struct?
09:33danvet: iirc we have something already
09:33emersion: i remember we have something there
09:34danvet: hm not much kerneldoc for uapi/drm/drm_mode.h
09:34emersion: drm_mode_get_connector has nothing
09:34danvet: documenting struct drm_mode_get_connector
09:34emersion: but i can swear i saw something about it
09:34danvet: plus that file has a lot of malformed kerneldoc
09:35emersion: maybe it ws in the ml
09:35danvet: libdrm docs?
09:36emersion: only drmAvailable, drmHandlEvent and drmModeGetResources are documented there
09:37emersion: that reminds me, the connection enum is magically defined in libdrm...
09:38emersion: oh, no, you're right
09:38emersion: the comments in the header files explain this
09:39emersion: between libdrm headers, libdrm man pages, kernel ioctl structs, and internal kernel functions
09:39emersion: there's a lot of potential for duplicates
09:39emersion: and missing docs
09:41emersion: danvet: related: what should user-space do with DRM_MODE_UNKNOWNCONNECTION?
09:42pq: emersion, the reason I came up with this question orignally is that drmModeGetConnector() has no docs I could find.
09:42pq: and the doc for drmModeGetConnectorCurrent() is not exactly recommending to ditch drmModeGetConnector
09:42danvet: emersion, ah another good one to document I guess
09:43danvet: emersion, autoconfig should try to light up all connected outputs
09:43danvet: if there's none, also try the "unknown"
09:43danvet: if there's none, give up
09:46emersion: ah, interesting
09:47emersion: i really wonder where to document this one, since there's nothing in uapi headers
09:47emersion: i guess i'll need to type the status prop patch
09:48emersion: other random question: thoughts on exposing a "possible crtcs" connector prop, to make it so user-space doesn't need to deal with drmModeEncoder anymore?
09:54danvet: emersion, yeah we need to start there
09:54danvet: the lease stuff has kerneldoc
09:54danvet: emersion, uh make it a wrapper in libdrm maybe?
09:55emersion: ah, yeah, maybe
09:55danvet: emersion, hm if you never probe, maybe we should add that for unknown, autoconfig should try to do a full probe (at least at boot-up or so)
09:55danvet: since unknown happens when we can't do the invasive load balancing stuff that results in flicker
09:55danvet: or maybe that should be the fallback for "no output detected" dunno
09:56danvet: the thing is, the last hw that can do this is like over 10 years old anyway
09:56emersion: what should happen if i got one CONNECTED connector, and one UNKNOWN connector?
09:56danvet: ignore unknown
09:56danvet: most likely it's a ghost
09:56danvet: and so the user would get pissed about half the desktop missing if you have expand mode
09:56danvet: or tiny resolution if you clone
09:57danvet: unknown = "all the probe tests we've done say disconnected, but there's some probes we couldn't do for $reasons"
09:58emersion: can drmModeGetConnector do the missing probes>
09:58danvet: chances of there actually being something connected are very low
09:58danvet: or well, should
09:58emersion: ok cool
09:58danvet: I think there's some oddball cases where even a full GetConnector can give you unknown
09:59danvet: but that's all really old hw anyway
10:00danvet: (some gen2 i915 devices have only 1 crtc but 2 outputs, so if the other output is on we can't use the crtc to do load detection because it's in use)
10:00emersion:uses a gen2 i915 device daily…
10:01danvet: laptop or desktop?
10:01danvet: laptop have 2 pipes iirc
10:01emersion: ok, so i shouldn't have this issue then :P
10:01gitbot: Mesa issue (Merge request) 101 in drm "Document drmModeConnection" [Opened]
10:02emersion: ^ done in libdrm because that's the only place where we can document it…
10:03danvet: emersion, I found it
10:03gitbot: Mesa issue (Merge request) 101 in drm "Document drmModeConnection" [Opened]
10:03danvet: I think would be really good to document the uapi struct a bit better and link to the stuff we have already
10:03emersion: i can't open your file:// urls
10:04danvet: also disappointed in your hacking skills
10:04emersion:needs to practice more
10:05emersion: if any this was uapi…
10:06emersion: should i just copy-pasta this to libdrm?
10:07danvet: I think we should also improve the kerneldoc for the uapi struct
10:08emersion: yeah definitely
10:08emersion:proceeds to copypasta libdrm docs to kernel uapi
10:08danvet: I wonder whether we should vendor libdrm into the kernel
10:09danvet: just for the docs essentially
10:09emersion: what problems will that solve>
10:09danvet: we could document the uapi structs and the libdrm wrappers together
10:10emersion: you mean in the same patch? they would still be duplicated right>
10:10emersion: bleh, can't type interrogation points apparently
10:10danvet: we could cross reference at least somewhat
10:10danvet: for the overview section
10:11danvet: also libdrm is a lot deader than the kernel
10:12emersion: ugh, the "probe" flag is actually implemented as "is modes_ptr NULL?"
10:12danvet: it's cute, isn't it
10:13emersion: it _kind_ of makes sense, but…
10:13danvet: look at the flow for the "query the size of the mode array without probing"
10:18pq: thank you for all that work you're doing :-)
10:36danvet: tzimmermann, btw want igt commit rights or have already?
10:36danvet: maybe ping ivyl or Adrinael if so
10:37danvet: tzimmermann, also maybe you could volunter jonas mark for the review, they seem to care about fbdev
10:40tzimmermann: danvet, i don't have igt commit rights
10:41tzimmermann: for the single patchset, someone could merge it.
10:43tzimmermann: oh, the name is jonas? i though mark
10:46MrCooper: mutter/gnome-shell just started using marge-bot, and immediately disabled direct merging for developers. Is it time for Mesa to do the latter as well?
10:47jadahl: MrCooper: fwiw, there is no way to disable it for all developers, 'maintainers' still can accidentally merge
10:47MrCooper: yeah, that's fine
10:47krh: when was the last time anybody merged manually?
10:47MrCooper: maintainers are supposed to use their powers responsibly :)
10:48MrCooper: krh: 15 hours ago
10:48krh: MrCooper: what for? I guess it's still necessary for disabling broken CI
10:48MrCooper: interfering with another MR Marge was trying to merge
10:50MrCooper: krh: not sure what your last question was referring to
10:50krh: MrCooper: what are the reasons we still do manual commits?
10:51MrCooper: mostly the stable branches
10:51MrCooper: there's normally no reason for master
10:52MrCooper: BTW I didn't say manual commits were still supposed to be used :) Just that maintainers still being able to in theory is fine
10:53krh: MrCooper: right and I just realized to disable broken CI, you just disable it in the gitlab-ci config and then it won't block marge landing that commit :)
11:57pendingchaos: jekstrand: does nir_opt_dce have to work with unstructured control flow?
11:57pendingchaos: I think I can make the pass much faster but I'm unsure if I can assume structured
11:58pendingchaos: karolherbst: ^?
12:00tzimmermann: danvet, may i ask you for a review of my shmem patchset: https://patchwork.freedesktop.org/series/83959/ ?
12:02emersion: eric_engestrom: do i need to do anything wrt. https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/72 or just wait?
12:02gitbot: Mesa issue (Merge request) 72 in drm "man: convert to reStructuredText" [Docs, Opened]
12:38karolherbst: pendingchaos: good question, but why would it matter?
12:46tango_: OT: I get this «Failed to get security report information. Please reload the page or try again later.» when opening MR 72 linked above
12:47emersion: yeah, been getting that as well
12:47emersion: doesn't seem to affect anything
12:48tango_: (fwiw I'd bikeshed in favor of asciidoc rather than rst, but I'm not going to do the work so ignore this message)
12:49emersion: tango_: kernel already uses rst, rst may be also used to generate a doc website
12:49pendingchaos: karolherbst: currently my changes visit the cf nodes and repeats the pass for loops to dce instructions in them correctly
12:50pendingchaos: there are no loop cf nodes if the shader is unstructured, but there might be blocks that need repeating
12:51pendingchaos: maybe I could make my changes a little faster and remove the structured requirement by using nir_block_worklist instead
12:51tango_: emersion: np, as I said it was bikeshedding. I prefer adoc to rst ;-)
12:52karolherbst: pendingchaos: dce kind of feels like this pass which doesn't need a structured shader
12:52emersion: yeah. i prefer scdoc to asciidoc/rst personally, but it's also bikeshedding
12:53karolherbst: the successors/predecessors links are all fine, so you really just need a proper way of iterating through the shader.
13:07pendingchaos: I just remembered, structured control flow is also useful since the pass can know if it might have to visit a block again later
13:07pendingchaos: and only visit instructions once
13:07danvet: tzimmermann, so mripard also asked me for some review and I'm woefully behind on everything, can't you cross review?
13:07karolherbst: pendingchaos: why would it matter for dce in an ssa shader?
13:07pendingchaos: looks like the current version already doesn't work with unstructured? since it uses nir_foreach_block instead of nir_foreach_block_unstructured
13:08karolherbst: ahh yeah, currently most passes won't work
13:08karolherbst: I think jekstrand fixed a hand ful
13:08karolherbst: but yeah
13:09tzimmermann: danvet, what does he want to get reviewed?
13:10karolherbst: the most trivial dce implementation could just run backwards through a shader with safe iterators and throw stuff away, which isn't an output or not referenced
13:10karolherbst: (registers being a bit weird)
13:10karolherbst: (or variables)
13:10pendingchaos: karolherbst: visiting instructions only once is an optimization, it doesn't matter for correctness
13:10pendingchaos: IIRC the difference was significant
13:11karolherbst: sure, but the same you get when you iterate backwards
13:11danvet: tzimmermann, some vc4 patches
13:11danvet: mripard, ^^
13:11karolherbst: and just delete while you iterate
13:11danvet: I did some high level bikesheds on them already, so really just need a read-through + ack
13:12tzimmermann: danvet, mripard, this: https://patchwork.freedesktop.org/series/83865/ ?
13:54robher: pinchartl: Here's what I'm ending up with on the graph schemas looking like: https://pastebin.com/ALA3iKE8
13:57pinchartl: robher: could be worse. it would be nice if the inner $ref could be avoided, but I understand that's a tooling limitation ?
13:57pinchartl: the port-base vs port is a bit annoying :S
13:59robher: pinchartl: It's just how json-schema works.
14:02robher: pinchartl: Essentially '-base' means you have extra properties to define. I worry it will be hard to get right.
14:05pinchartl: getting it right won't be that difficult. making sure it will always be right... I also worry we would miss things during review
14:11mripard: danvet: tzimmermann: https://firstname.lastname@example.org/ would be more important, it's a fix and the series you pointed at depends on it
14:12mripard: https://email@example.com/ would be great as well :)
14:21tzimmermann_: mripard, i'll take a look tomorrow
14:22mripard: tzimmermann_: awesome, thanks
16:35jekstrand: pendingchaos: I had a use for DCE on unstructured at one point but I don't think it's really needed right now.
16:35jekstrand: pendingchaos: If I understand what you're intending, we could probably keep both paths working if we really wanted.
16:51Lyude: Hey - I'm looking at https://gitlab.freedesktop.org/gfx-ci/cibuglog as we're looking at experimenting with this at Red Hat for our own testing needs. I see that we'd need to setup our own postgresql host, but am I also right in assuming we wouldn't be able to get access to the cibug log data that Intel's actual CI uses just from hosting our own instance of the docker image mentioned on there?
16:54ickle: Lyude: better to ask on intel-gfx or intel-gfx-ci
16:54Lyude: oh oops
16:54Lyude: meant to ask there in the first place :P, sorry about that
17:16jenatali: airlied: Mind if I rebase your printf MR now that our CL stack has landed? I'd like to hook it up for us as well, and then I can push that patch to your MR as well
18:58lrusak: would it be possible for me to convert a P010 format to XRGB2101010 through GL_TEXTURE_EXTERNAL_OES ?
19:05lrusak: or how is p010 handled internally in mesa?
19:08lrusak: or is this not possible and it will always convert to an 8bit format?
19:10lrusak: I guess it's only possible by using R16 RG1616 method?
19:14airlied: jenatali: rebase away though it's a bit rough at the moment
19:14emersion: it should be possible to import a P010 buffer as a FBO, then paint it into a XRGB2101010 FBO, lrusak
19:14lrusak: emersion, thanks, that's what I'm looking into now
19:14jenatali: airlied: Not too bad, I left some comments in the MR but it should be functional enough for me :)
19:14emersion: lrusak: it should at least be possible on my AMD GPU
19:15emersion: GL_TEXTURE_EXTERNAL_OES can only be textured from, they can't be rendered to
19:16airlied: I'm going to fix up the few piglit tests I wrote today hopefylly
19:18lrusak: emersion, but if I have an FBO with the correct format can I texture from OES into it and still have a 10bit format?
19:34emersion: the FBO won't change its format on the fly
19:53airlied: jenatali: I was considering changing the non-format string storage into one pointer with the strings concatentated after it
19:53airlied: just to avoid the extra malloc
19:53jenatali: airlied: Sure, seems reasonable
19:53jenatali: Dunno if you saw my comment yet, but they're all leaked right now :)
19:54airlied: I thought I freed them in clover
19:54jenatali: Ah, I missed that
19:55jenatali: I expected to see them ralloc'd or somehow attached to the lifetime of the nir_shader
19:55airlied: clover transfers them to the module after ralloc_free
19:55airlied: I'm kinda thinking a spirv_to_nir output might be better than stashing them in nir_shader
19:57jenatali: airlied: I'd probably still ralloc them with the shader, and have clover ralloc_steal them
19:58jenatali: Eh maybe, I dunno, that's a bunch of extra code, but makes it harder to accidentally leak them, and easier to use-after-free
20:04airlied: jenatali: considering there is only 2 users for this code I'm not sure I care about accidents
20:05airlied: but yeah I might ralloc them
20:15jenatali: Ugh this stupid printf test is so unreliable on Windows...
20:18jenatali: Finally: PASSED 57 of 57 tests.
20:28jenatali: airlied: Double-checking before I stomp on you, objections if I push the branch, rebased with clon12 changes on top?
20:28airlied: jenatali: go for it I'll rebase my local stuff after that
20:30jenatali: airlied: Done - there's one fixup commit in there that should get squashed in but I figured I'd leave that until after someone looks at it
20:44jekstrand:should probably find a barf-bag and read it again
20:44airlied: jekstrand: give it a minute
20:45airlied: just fixing up the instr handling for > 5 sources
21:00daniels: jenatali: \o/
21:01jenatali: daniels: \o/
21:01jenatali: daniels: How's that CI coming? :)
21:17airlied: jekstrand, jenatali : turd polishing complete for now
21:17jenatali: airlied: Great :) will take another look
21:18airlied: I think the printf clover bits have a bug with multiple args but otherwise the vtn/nir bits should be "good" :-P
21:20jenatali: airlied: You added back in explicit offsets into printf struct construction?
21:22airlied: jenatali: yeah reordering the passes wasn't looking like fun
21:22airlied: though maybe it wasn't as buggy as I thought
21:22jenatali: airlied: Cool, I had to do it too, it wasn't pretty..
21:23jenatali: Ended up needing to do vars_to_explicit_types twice for temp, once before eliminating dead variables (while printf struct was there) and once after to properly set the scratch size
21:24airlied: jenatali: yeah I don't think we should do that kind of dance
21:24airlied: I think we should look at jekstrand's ideas of splitting the vars_To_explicit_types pass
21:26jenatali: That's not a bad idea, yeah
21:27airlied: at least I don't think we should complicate the passes we have now just to avoid setting some sizes in the printf code
21:31jekstrand: airlied: That's not the only reason. :)
21:31jekstrand: airlied: opt_deref needs the information in order to eliminate redundent alignment casts.
21:32daniels: jenatali: well, if now knowing several more things that don’t work is good then ... good? :\
21:33daniels: there’s some really weird stuff around global connection reuse that I just don’t understand, and I’m really hoping it’s not yet another instance of Docker not working as well as you’d hope
21:33jenatali: daniels: :(
21:43LiquidAcid: emersion, hey, do mind me asking a question about gamescope? (wlroots related)
21:44emersion: sure, feel free
21:45jenatali: airlied: Updating my last patch on your MR to deal with your metadata updates, FYI
21:46LiquidAcid: emersion, as a wlroots based application, it should honor the WLR_ envvars, right?
21:47emersion: yes, for the parts of wlroots used by gamescope
21:47LiquidAcid: so WLR_DRM_DEVICES should work
21:47emersion: gamescope for instance doesn't use wlroots backends
21:47emersion: so WLR_DRM_* won't work
21:47emersion: wlroots is more of a collection of helpers than a framework
21:47LiquidAcid: ok, that explains why it behaves like i behaves
21:48emersion: so compositors are free to use what they want, and not use what they don't want
21:48LiquidAcid: i was trying to run sway and gamescope in parallel, sway on the igpu, and gamescope on the dgpu, but gamescope didn't exactly use the right device
21:48emersion: gamescope doesn't support multi-gpu
21:48emersion: ah, yeah
21:48emersion: i don't remember how gamescope enumerates GPU devices
21:49emersion: i'd merge a patch adding a flag to select the GPU
21:52LiquidAcid: also judging from the code it seems like there currently isn't any way to select a different connector, but maybe i missed something there
21:55airlied: jenatali: cool, I'm just fixing up the last clover side bugs in the printing
22:18daniels: jenatali: hoping that closing connections instead of RST will be less confusing ... let’s see
22:30jenatali: Hm... I wonder if anybody ever wants Windows.h's stupid min/max macros...
22:30jekstrand: jenatali: If they're stupid then probably no. :P
22:31jenatali: jekstrand: It's literally min and max as macros, that alone is enough to make them stupid
22:32jekstrand: jenatali: Is it at least MIN/MAX and not min/max?
22:33jekstrand: Then I'm going with "no" :)
22:33jenatali: Trying to get Clover building with MSVC, lots of instances of weirdness as a result of those :P
22:33jekstrand: jenatali: #undef min?
22:33jenatali: Disabling it globally didn't blow up for anything I'm building so far, except we have a couple instances in the d3d12 driver that snuck in...
22:33jenatali: -DNOMINMAX on the command line
22:34jekstrand: Yeah, I suspect you can set that option globally when building with MSVC and no one will care.
22:35dcbaker[m]: looks like swr might use it, but they explicitly undef it
22:35dcbaker[m]: oh, no wait
22:35dcbaker[m]: they explicitly set it then undef it at the end of the file
22:36dcbaker[m]: but I'd test with SWR on windows (I don't think we do that in any of our CI)
22:36jenatali: Thanks, I'll see if I can figure out how to do that
22:37dcbaker[m]:sent a long message: < https://matrix.org/_matrix/media/r0/download/matrix.org/dBcihjsAVixaQnKXjsJEiJHR/message.txt >
22:37dcbaker[m]: you need llvm (I presume you have that already) then -Dgallium-drivers=swr
22:37jenatali: Ah, cool
22:41airlied: multi args has some alignment issues, bleh
22:58danvet: tfiga, finally got around to compile vivid and run this with yavta
22:58danvet: didn't go boom
22:59danvet: anything I should check for?
23:01airlied: jenatali: making multiple arg work is kinda messy with alignment
23:01jenatali: airlied: Yeah, that doesn't surprise me
23:01jenatali: Where's the problem you're hitting?
23:02airlied: since at least in clvoer we have a 4 byte headers, then a 4byte format string id
23:02airlied: but we parse off the 4 byte header, memcpy the rest of the buffer in
23:02airlied: which means any alignments after that for 8-bytes are wrong
23:02airlied: I just hacked up 8 byte header + 8 byte format string locally to make sure that works
23:03airlied: granted it won't be compatible with the llvm backend
23:03jenatali: I'm still not following exactly, where's the alignment problem?
23:04airlied: yeah it's not a simple thing to explain
23:04airlied: but the ssbo side and what I work out on the cpu side don't end up the same
23:06airlied: one test ends up with <header 4b> <fmt_str_id 4b> <arg0 4b> <padding> <arg1 8b> <arg2 8b> in the ssbo
23:06airlied: but clover parsing code pull the header 4bytes off and memcpys the rest
23:06airlied: which messes up the cpu side alignments
23:06jenatali: Oh, it assumes the args are packed?
23:06jenatali: Or each arg is 4byte aligned?
23:07airlied: well once it strips off the 4b, the args end up on the wrong alignment wrt 8b
23:07airlied: but fixing just that didn't seem to completely fix it
23:07jenatali: Oh, it tries to align everything to the base of the buffer instead of the base of the struct?
23:08airlied: jenatali: yes that probably explains it better
23:09airlied: not 100% sure if the ssbo writes need to change or if I can work it out in clover
23:09jenatali: I'd expect you could work it out in clover, or you could just not CL-align the struct and only 4-byte-align the struct members
23:11airlied: jenatali: but will that piss off 8-byte ssb stores?
23:11jenatali: CL already requires you to support writing a ulong in a packed struct at a comletely unaligned offset
23:12jenatali: For D3D all our SSBO writes just need to be 4 byte aligned, any smaller than that and it starts to get a bit painful
23:16airlied: jenatali: yeah probably makes sense the to cl align to 4 bytes
23:16airlied: and do same on the cpu side
23:17jenatali: Yeah, that way alignment to the base of the buffer is the same as alignment to the base of the struct, it's simpler to calculate
23:21airlied: I have a test that does two printfs with a bunch of args now that should be good to validate it
23:21jenatali: Great :)
23:37airlied: jenatali: does cl struct size pad the end of the struct?
23:38airlied: the second string I print is disagreeing on where format ends up
23:40jekstrand: airlied: ish :)
23:41airlied: so we might have to make the struct packed
23:43airlied: yah test passes now
23:45airlied: though still one bug left in my printf, missing a space somewhere
23:59jenatali: airlied: Yeah CL struct size is padded to align to the largest member