00:01robmur01: yeah, until Android exclusively ships bleeding-edge mainline kernels, that would require vendors to upstream drivers 2-3 years before they've even designed the SoC ;)
00:03robmur01: typical vendors are writing brand-new code against GKI branches based on 5.10 or 5.15 (which may eventually get forward-ported to mainline and thrown over the wall if we're lucky)
00:48CounterPillow: I was about to sarcastically describe why they might not bother with upstreaming but then I remembered I should not start flamewars with people who might work for companies I eventually seek employment at.
00:56karolherbst: jenatali: uhh... I have to figure out from a spir-v if `-cl-single-precision-constant` was set... I think...
00:56jenatali: Oh?
00:56jenatali: Why?
00:56karolherbst: to set treat_doubles_as_floats correctly, no?
00:57jenatali: Nah, we can just delete that flag
00:57karolherbst: ohhh
00:57karolherbst: yeah, fair enough :)
00:57karolherbst: that makes everything easier :D
00:57jenatali: If cl-single-precision-constant was set, then you won't see doubles in the printf string unless they were really doubles
00:57karolherbst: right...
00:57karolherbst: and if it's in the spirv, the user is asking for it anyway...
00:57jenatali: Otherwise, clang does the C thing of promoting floats to double because double is the "native" floating point type
00:58karolherbst: yeah.. I'm more concerned about spirv as the input, not clc
00:58karolherbst: as there it's not in our control how it's compiled
00:59karolherbst: so I wonder if we want to keep that option anyway, but only depend on if fp64 support is enabled for the device or not?
00:59karolherbst: and then we can force fp32 for spir-vs
00:59jenatali: Ah yeah I see what you're saying, that makes sense
01:01karolherbst: I'll just push `-cl-single-precision-constant` to the frontend and see how that works out :)
01:01jenatali: Yeah, that's what I've done
01:07karolherbst: mhh...
01:07karolherbst: it's not using fp64 constants by default...?
01:07karolherbst: was there some condition for it?
01:08karolherbst: ehh wait...
01:08karolherbst: it's 32 bit in the spirv, but 64 bit in the LLVM...
01:14karolherbst: mhhhhh
01:14karolherbst: interesting...
01:14karolherbst: jenatali: looks like llvm just handles that for us now 🙃
01:15karolherbst: unless I pass "+cl_khr_fp64" or "+__opencl_c_fp64" to clang the constants all end up as fp32 ones in the spirv...
01:15karolherbst: but I'm also seeing "call spir_func i32 (i8 addrspace(2)*, ...) @printf(i8 addrspace(2)* noundef getelementptr inbounds ([8 x i8], [8 x i8] addrspace(2)* @.str, i64 0, i64 0), float noundef 0x4024B0F280000000)"...
01:17karolherbst: yeah.. it becomes "call spir_func i32 (i8 addrspace(2)*, ...) @printf(i8 addrspace(2)* noundef getelementptr inbounds ([8 x i8], [8 x i8] addrspace(2)* @.str, i64 0, i64 0), double noundef 1.034560e+01)" with enabled fp64
01:18karolherbst: ... that also explains why I have seen regressions in my spirv based runs and not the clc ones...
01:19karolherbst: I think that changed in llvm-16 actually...
01:22karolherbst: jenatali: https://reviews.llvm.org/D24235 mhhh
01:22karolherbst: like that's 7 years ago...
01:24karolherbst: ohhhhhhhhhhhhhh
01:24karolherbst: I know
01:24karolherbst: I've fixed this ....
01:24karolherbst: I suspect my work on forcing enabled features made us not claim fp64 support and all that
01:24karolherbst: so we don't run into this with llvm-14+
01:25jenatali: Oh cool
01:25karolherbst: I guess I will only handle the spir-v part then :D
01:25karolherbst: but yeah.. that explains it then
01:26jenatali: karolherbst: I added that when I was still using 12 I think
01:26karolherbst: yeah...
01:26karolherbst: that's why I was confused because that llvm check is there since llvm-8?
01:26karolherbst: ehh
01:26karolherbst: llvm-4 even
01:27karolherbst: could also been that `-USPIRV` and `-USPIR` part...
01:27karolherbst: as that would otherwise enable rando extes
01:27karolherbst: *exts
01:28karolherbst: apparently not cl_khr_fp64
01:30karolherbst: jenatali: anyway.. the reason I ran into the 64 bit arg mess was because of `%s` args as they are 64 bit anyway ....
01:30jenatali: Right
01:30karolherbst: and then encode ids like... 0x1de
01:33karolherbst: maybe I really just fix the callbacks and move on...
01:33karolherbst: oh well.. I'll decide it tomorrow then
02:54gfxstrand: karolherbst: Did you test the alignment patches or just read them over?
06:26tomeu: how do people figure out what specific test might have caused a GPU hang when it only happens under high concurrency inside deqp-runner?
06:26tomeu: anholt_: maybe you have some tricks?
09:16karolherbst: gfxstrand: I've ran them, just that they didn't do anything for the issue I was seeing as I was also hitting an LLVM bug. But I can run the CL CTS with those and see if anything break if you want
10:42karolherbst: jenatali: actually... for spir-v the printf double thing is also not a problem, because the only valid way of doing is to have a spir-v requiring "OpCapability Float64", which isn't valid to pass to the runtime if it doesn't support cl_khr_fp64 :D
10:42karolherbst: I think I'd rather fix my spirv testing here...
10:42karolherbst: and we can indeed drop that option
10:49karolherbst: uhh.. why is `-cl-single-precision-constant` ignored on the clang CLI...
10:53karolherbst: needs to be -Xclang -cl-ext=-all ...
11:11karolherbst: yeah....
11:11karolherbst: -cl-single-precision-constant doesn't work at all...
11:12karolherbst: at least now how we are using clang...
11:12karolherbst: *sigh*
11:12karolherbst: not in the mood of debugging this honestly :D
11:23karolherbst: oh actually.. it works in libclang.. I forgot to switch to online compilation.. oops
11:23karolherbst: ohh it doens't.. I forgot to enable fp64.. uhhh
11:24karolherbst: whatever...
11:24karolherbst: I spending too much time on printf...
11:24karolherbst: *I'm
11:27dj-death: karolherbst: speaking of printf
11:27dj-death: karolherbst: you have any opinion on those changes: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26505
11:28karolherbst: looks good
11:30karolherbst: I'll leave comments on the MR
11:31dj-death: karolherbst: thanks
11:31dj-death: karolherbst: I think possibly the only controversial thing is to have the printfs on all shader stages
11:31karolherbst: yeah
11:31dj-death: in the serialization
11:31karolherbst: that's where I'll leave a comment :D
11:31dj-death: I can probably workaround that for my case
11:32karolherbst: nah
11:32karolherbst: I have a better idea
11:32dj-death: thanks a lot
11:36karolherbst: done
11:45karolherbst: jenatali: treat_doubles_as_floats seems to be broken, as the printf code still parses it as 8 bytes and generates a wrong value...
11:45karolherbst: so we'd need to update the printf info here
11:45karolherbst: but anyway.. I think it's safe to remove
11:46karolherbst: just need to fix clang to respect -cl-single-precision-constant or figure out what we'd need to change or something...
11:46karolherbst: (in case we do support fp64)
12:12jenatali: karolherbst: Right... I think we'd need it as a separate pass then so it can be before lowering the printf struct to explicit types
12:13jenatali: So yeah, just delete it I think
12:43karolherbst: gfxstrand: no regressions at least...
12:44karolherbst: jenatali: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26541
12:46jenatali: karolherbst: Tag me in it so I get an email so I remember to review it later, it's not even 5am here yet
12:46karolherbst: fair
12:47karolherbst: good night then :P
12:48karolherbst: done
13:04mripard: donaldrobson, frankbinns: ack for https://lore.kernel.org/all/20231204073231.1164163-1-arnd@kernel.org/ ?
13:06frankbinns: mripard: ack
13:08mripard: frankbinns: merged, thanks
13:09frankbinns: I had totally missed that one so thanks for the ping
13:52gfxstrand: karolherbst: Okay, cool.
13:52gfxstrand: karolherbst: Did you verify it fixed the kernel you were looking at?
13:54karolherbst: sadly it doesn't as it only has one shared variable (so the location is 0 anyway) and the align_mul=1 comes from LLVM being silly :'(
13:54karolherbst: I just noticed that we don't handle the Alignment thing on variables while looking into it
13:56karolherbst: but I could probably come up with a test to verify
13:59karolherbst: gfxstrand: btw, I need something like this: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26541/diffs?commit_id=40bab1d34c0701df71cf4b421fd5c22eea6ce6b7
13:59karolherbst: wondering if you have a better idea :)
13:59karolherbst: (and yes, it should loop over the mov as it could be a mov chain)
14:00karolherbst: this is needed with opaque pointers and we ending up with some movs along the way
14:26jenatali: karolherbst: Run copy prop before whatever pass was blowing up?
14:27karolherbst: that's in vtn
14:27karolherbst: `vtn_add_printf_string`
14:28karolherbst: so.. no :)
14:28jenatali: Ohh
14:35gfxstrand: karolherbst: Ugh... I don't want to think about printf. :sob:
14:35karolherbst: gfxstrand: it's your lucky day, it's not a printf specific issue :P
14:35karolherbst: just if you have movs in your deref chain
14:35karolherbst: and you are still in vtn
14:35gfxstrand: karolherbst, eric_engestrom: Is there a reason why the gitlab CI's rustfmt is only running on lib.rs or app.rs?
14:36karolherbst: gfxstrand: this pulls in the entire project
14:36karolherbst: otherwise you'd run it on all files multiple times
14:36gfxstrand: karolherbst: Only if you have a lib.rs. :P
14:36karolherbst: what's your file called then?
14:36gfxstrand: nak.rs
14:36karolherbst: mhh.. could rename to lib.rs 🙃
14:37karolherbst: but I guess...
14:37gfxstrand: Yeah, I probably should.
14:37karolherbst: you kinda want to have that to be able to structurize your code, but yeah...
14:37karolherbst: worst case we just add custom file names
14:37gfxstrand: Yeah, I can rename to lib.rs
14:37karolherbst: `lib.rs` and `app.rs` is just what most rust projects use and it's a cargo default
14:38karolherbst: ehh
14:38karolherbst: lib.rs and main.rs
14:38karolherbst: I should fix the app.rs one...
14:38eric_engestrom: yeah it's trivial to add nak.rs
14:38eric_engestrom: app.rs should be gone, unless I missed something somewhere?
14:39karolherbst: yeah.. it should be main.rs instead
14:39karolherbst: but we can also remove it
14:39karolherbst: and we could also add nak.rs
14:41eric_engestrom: gfxstrand: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26543
14:42gfxstrand: eric_engestrom: I think I'm going to just restructure nak a bit
14:42eric_engestrom: ack
14:43karolherbst: I have a handy git alias btw: rustfmt = ! git ls-files */{lib,main}.rs | xargs rustfmt
14:43karolherbst: (which I would have to update for nak... 🙃)
14:43eric_engestrom: gfxstrand: I'll drop this MR then, but now you know which line to edit when you have your new filename :)
14:45eric_engestrom: karolherbst: if you have `shopt -s globstar` in your shell you can simply do `rustfmt **/lib.rs` :)
14:47karolherbst: but I just want to do that on git tracked files tho :D
14:47karolherbst: otherwise it might pull in generated files and such
14:47karolherbst: but yeah...
14:49eric_engestrom: well, in ci it's run on `src/**/lib.rs`, if you do the same locally that will not include generated files unless you put your builddir in src/ :P
15:16gfxstrand: karolherbst, eric_engestrom: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26546
15:16gfxstrand: I think that should do the trick
15:17gfxstrand: I would actually like an ACK or two on that MR. Just to make sure I didn't do anything exceptionally stupid.
15:17gfxstrand: Maybe look at the meson a bit.
15:26karolherbst: you don't necessarily have to split it up on a crate level
15:26karolherbst: that reminds me.. I wanted to move to rust.proc_macro at some point :)
15:27karolherbst: but yeah.. I think they are fine
15:28gfxstrand: karolherbst: Yeah, I know it's not necessary. But, if my crates are going to be lib.rs..
15:28MrCooper: hmm, does MR Label Maker not know about zink yet?
15:28gfxstrand: Is MR Label Maker working?
15:28karolherbst: it should be fixed now
15:28karolherbst: but it was broken for a while
15:28gfxstrand: Okay
15:28gfxstrand: I was trying to get it to label my dot product MR
15:29gfxstrand: But whaver. I think I found them all manually
15:29karolherbst: yeah... we've debugged the server a few hours ago
15:29gfxstrand: Ah
15:30MrCooper: thanks
20:57DemiMarie: mattst88: sorry about that.
20:58DemiMarie: robmur01: I had not considered that Android doesn’t ship bleeding-edge kernels. That explains so much!
20:59DemiMarie: I’m a bit salty about Android using proprietary drivers because it means no virtio-GPU native context support, which means no decently secure GPU acceleration in pKVM VMs.
22:27lumag: mripard, ack/r-b for the https://lore.kernel.org/all/CAA8EJpr3hNdB02avXrY+PQGGSjJTm4YT3Hct1OwsLuNQN_H9Xw@mail.gmail.com/ ? I'd like to apply it.
23:52mareko: DemiMarie: Android has virtio
23:53mareko: also native context