05:29K900: Hey folks, can we have https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/38151 and https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/39119 picked to stable?
05:29K900: Build fixes for obscure platforms
05:29K900: (ppcbe)
05:29K900: (well the same fix twice, really)
05:55mareko: karolherbst4: buffer_map should implicitly flush the context (if needed) and then wait for all fences from all previously flushed contexts; only unflushed work from other contexts is not visible to buffer_map
08:30eric_engestrom: ajax just pushed a bunch of random changes to a bunch of random non-sense branch names, in the upstream repo (https://gitlab.freedesktop.org/mesa/mesa/activity) 08:30eric_engestrom: do we think his account got compromised?
08:32Kayden: airlied, daniels: ^?
08:32eric_engestrom: I would've message just him but he's not online, so I'm asking everyone if we should take an action like lock his account until he's back to let us know if that was him
08:32pixelcluster: eric_engestrom: those commits have Co-Authored-By: Claude Opus 4.5
08:32pixelcluster: rogue agentic LLM "helper"?
08:32airlied: guess his claude went rogue
08:33eric_engestrom: oh, so "compromised" but in an "intentional" way :/
08:33airlied: I msg'ed him internally
08:33eric_engestrom: airlied: thanks :)
08:36daniels: the branch names actually track just fine if you ignore the suffix
08:39eric_engestrom: "polecat/sinhy", "polecat/chrome", "polecat/rust", "polecat/guzzle" even if I ignore the random string at the end, this still looks like random nonsense to me 😅
08:39eric_engestrom: *shiny
08:40eric_engestrom: I assume no objection that I delete all these branches he just pushed upstream? he can always push again, and we're not supposed to be pushing personal branches upstream but to our forks instead
08:41eric_engestrom: (done)
09:03eric_engestrom: K900: they're trivial so sure, they're backported to 26.0 :)
09:32linkmauve: K900, I don’t like these two fixes at all, PRIu64 should likely be used instead.
09:32linkmauve: Or is there any reason not to use that?
09:33linkmauve: Well, PRIx64 instead.
09:39pixelcluster: linkmauve: the MR mentions that
09:39pixelcluster: PRIx64 is for uint64_t
09:39pixelcluster: this is about __u64
09:40linkmauve: Is there any way for both types to not be equal? If not a cast from __u64 to uint64_t should work on every platform, or is there something I’m missing?
09:40pixelcluster: on platforms I typically use I'd expect those to be the same but who knows, it seems to at least have been considered
09:48K900: linkmauve: This is the case on ppc, at least
09:52linkmauve: K900, do you have a reproducer? I have a ppc laptop running besides me, I’d be extremely surprised if __u64 and uint64_t can’t be converted between each other on that platform.
09:52MrCooper: a cast should be fine though, since both are unsigned and hold 64 bits
09:52linkmauve: ↑ this.
10:01K900: linkmauve: Possibly only ppcbe
10:07linkmauve: I’m also on ppcbe. :)
10:07linkmauve: Both on my G4 laptop and on my Wii.
10:08linkmauve: Do you mean the order of 32-bit halves would be different between __u64 and uint64_t? Big-endian is actually the sensible byte order, so I doubt anyone would mess that up.
10:08K900: Hmm, I wonder if it's just a format-security thing
10:08K900: I know it doesn't build in nixpkgs without those
10:08K900: But I don't actually have ppc64be hardware to test
10:22MrCooper: the two types may be defined as different fundamental types, which can result in warnings
10:23MrCooper: e.g. long unsigned vs long long unsigned
10:25K900: That's what it is I beleive
10:25K900: I can try and get the patch author into this channel
10:59dschuermann: for visibility: https://gitlab.freedesktop.org/mesa/mesa/-/issues/14831 13:56glehmann: dj-death: cmarcelo: can I merge https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/39641 with the weird intel regression, or does someone from your team want to look into it first?
14:01dj-death: let me have a look
14:10dj-death: glehmann: this doesn't look right
14:11dj-death: glehmann: prior to your MR we do 0x80000000 * 0x80000000
14:11dj-death: which gives 0x00000000 as a result
14:12dj-death: but after it's 0x80000000 * 0x00000000 which still gives 0x80000000
14:13glehmann: what does the nir look like?
14:14glehmann: I'm kind of confused, because if I split fma, the result is +0.0 because the cross reduces to src0*src1 - src0*src1. and with fma it's fma(src0, src1, fmul(src0, src1)) on radv, which is fine too
14:15dj-death: 32x2 %4 = @load_ssbo_uniform_block_intel (%3, %1 (0x0)) (access=readonly|reorderable, align_mul=1073741824, align_offset=0, base=0)
14:15dj-death: 32 %5 = fmul %4.y, %1 (0.000000)
14:15dj-death: with your MR ^
14:15dj-death: before :
14:15dj-death: 32x2 %4 = @load_ssbo_uniform_block_intel (%3, %1 (0x0)) (access=readonly|reorderable, align_mul=1073741824, align_offset=0, base=0)
14:15dj-death: 32 %5 = fmul %4.x, %4.y
14:15dj-death: 32 %6 = fneg %5
14:15dj-death: 32 %7 = ffma %4.x, %4.y, %6
14:15glehmann: that is really ood
14:16glehmann: because that broken optimization isn't happening for me on radv
14:16dj-death: do you know which one it is?
14:17glehmann: which optimization you mean? no
14:19dj-death: I wonder if there is a way to print out what optimization is applied
14:20pendingchaos: there's some debug code #if 0'd out in nir_search.c
14:20dj-death: thanks a lot
14:21dj-death: matched: (~fadd (~fneg a) a) -> 0.000000 ssa_6
14:22Kayden: 32x2 %4 = @load_ssbo_uniform_block_intel (%3, %1 (0x0)) (access=readonly|reorderable, align_mu>
14:22Kayden: 32 %5 = fneg %4.x
14:22Kayden: 32 %6 = fadd %4.x, %5
14:22Kayden: 32 %7 = fmul %4.y, %6
14:22Kayden: is what that rule's being applied to
14:23glehmann: the input is already wrong
14:24dj-death: then the culprit is opt_gcm
14:24dj-death: turns :
14:24dj-death: 32 %5 = fneg %4.x
14:24dj-death: 32 %6 = fmul %5, %4.y
14:24dj-death: 32 %7 = fadd %4.x, %5
14:24dj-death: 32 %8 = fmul %4.y, %7
14:24dj-death: into :
14:24dj-death: 32 %5 = fneg %4.x
14:24dj-death: 32 %6 = fadd %4.x, %5
14:24dj-death: 32 %7 = fmul %4.y, %6
14:25Kayden: nope, you can turn off gcm and it still fails
14:26dj-death: errr
14:26dj-death: you're right
14:26dj-death: or it's previous opt_algebraic :
14:26dj-death: matched: (~ffma a b (~fmul a c)) -> (fmul a (fadd b c)) ssa_7
14:29dj-death: 32 %5 = fneg %4.x
14:29dj-death: 32 %6 = fmul %5, %4.y
14:29dj-death: 32 %7 = ffma %4.x, %4.y, %6
14:29dj-death: into:
14:29dj-death: 32 %5 = fneg %4.x
14:29dj-death: 32 %6 = fmul %5, %4.y
14:29dj-death: 32 %7 = fadd %4.x, %5
14:29dj-death: 32 %8 = fmul %4.y, %7
14:29dj-death: but I can't really tell what rule matches this...
14:30Kayden: (('~ffma', a, b, ('fmul(is_used_once)', a, c)), ('fmul', a, ('fadd', b, c))),
14:30Kayden: probably
14:31karolherbst: eric_engestrom, dcbaker: Do you think it would be feasible to add a label for MR that should be backported if one forgot to add tags to commits? Or do we already have something like that?
14:32glehmann: dj-death: thanks, that makes sense
14:32glehmann: but it's also pretty annoying
14:32glehmann: because that pattern is valid
14:33glehmann: so I guess that's just another cts bug
14:33Kayden: karolherbst: you can also open an MR against the staging/26.0 branch
14:33karolherbst: yeah, but that's more work than assigning a label
14:33karolherbst: and a label could be queried via tooling
14:33Kayden: true :)
14:34karolherbst: but we also have still 25.3 going on, so it's already two MRs :D
14:35glehmann: dj-death: can you test if https://gerrit.khronos.org/c/vk-gl-cts/+/19565 fixes this test too?
14:37glehmann: testing special values with non trivial math while allowing reassociation and other transforms is just weird
14:38glehmann: it's also weird that the (('~ffma', a, b, ('fmul(is_used_once)', a, c)), ('fmul', a, ('fadd', b, c))), rule isn't applied for radv
14:38glehmann: we end up with exactly that nir
14:40dj-death: glehmann: the tests list seems to pass
14:40dj-death: s/list/listed/
14:41glehmann: oh I understand why anv is affected, but not radv
14:42glehmann: because you call distribute_src_mods
14:44glehmann: dj-death: to be clear, I meant dEQP-VK.spirv_assembly.instruction.compute.float_controls2.fp32.input_args.cross_testedWithout_NSZ_arg1_minusZero_arg2_minusZero_res_zero_exec with that CL
14:44dj-death: ah...
14:44glehmann: for some reason, it's not in the CL description as affected
14:44glehmann: but it's changed
14:44dj-death: glehmann: yeah it passes
14:45glehmann: okay, then I would say, I add those tests as fails for now, because it's a cts bug and will hopefully be fixed soon
14:45glehmann: does that sound okay for you?
14:49alyssa: glehmann: ack on my end
14:52Kayden: is there more that needs to happen to get the CTS fix landed?
14:54alyssa: Kayden: it just runs on Khronos time
14:57glehmann: I mean, the CL is a day old
14:58Kayden: oh yeah, sure, just wasn't sure if it needed a second ack or something
14:59eric_engestrom: karolherbst: no objection to adding a label as well, if that's something people want to use
14:59eric_engestrom: obviously it's at the MR level and not the commit level so it can only be used when all the commits need to be backported, but that's a reasonable use-case IMO
14:59eric_engestrom: (FYI you can also just tag me in a comment on the MR asking me to pick it up)
14:59Kayden: glehmann: marking the CI fails as expected and landing makes sense to me
15:03dj-death: Kayden: I'll +1 the CL
15:06dj-death: rerun all *float_controls2* tests on ADL/DG2/BMG
15:06dj-death: should be good enough
15:07Kayden: nice
15:45eric_engestrom: thanks mairacanal3 for spotting that ajax pushed another "ai" commit, this one straight to main
15:45eric_engestrom: I've reverted it in https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/39810, and included advice that's probably good for everyone to avoid pushing things upstream
15:49alyssa: eric_engestrom: can we please just ban slop in mesa?
15:49alyssa: I'm tired.
15:50eric_engestrom: alyssa: very strong agree from me, but when it was discussed a few months ago the conclusion was "it would put some devs in awkward positions with their employers" and was left at that
15:50alyssa: yes well
15:51alyssa: some employers are putting us devs in awkward positions
15:51alyssa: so here we are.
15:52alyssa: I'd support adopting the gentoo policy https://wiki.gentoo.org/wiki/Project:Council/AI_policy as has been suggested before
15:53stsquad: one of the motivations for QEMU's AI policy was to give engineers something to point to when they don't utilise AI (at least for QEMU)
15:54stsquad: that said there have been arguments on the list about relaxing the policy to allow modern tools to help with re-factoring and "mechanical" changes
15:54eric_engestrom: +1 to adopting the gentoo policy
15:55glehmann: why is pushing to main possible at all?
15:56alyssa: stsquad: I mean I'm a heavy Coccinelle user, but Coccinelle has never randomly decided to push slop to main, or fabricate MRs full of nonsense that burn through tens of hours of review time & discussion while humans patiently determine and explain why the nonsense is nonsense, or have seriously morally dicey provenance (everything on the gentoo policy and more), all of which are
15:56alyssa: real examples from Mesa in the past year.
15:58cmarcelo: I think that revisiting the policy of pushing to main (we probably still want to have an escape valve but perhaps less users, or something we explicitly set) like glehmann suggested is something we should do to avoid accidents.
16:00alyssa: yes - there are 2 issues here.
16:00cmarcelo: alyssa: eric_engestrom: I'm really curious to see other commits like that, this one seems clearly an accident from actual dev and not the "random noisy contributions from random people" that has been plaguing other projects.
16:02eric_engestrom: re- pushing to main, the one use-case I can think of right now is when a farm is down and you need to push a commit to disable that farm so that MRs can go through, but also that can also simply be done the way I've done it lately of unassigning/reassigning MRs to marge to shuffle the merge queue
16:04stsquad: alyssa: well sure - and I don't think anyone is arguing genai code is pushed un-reviewed or even that the "driving" engineer isn't still responsible for whatever they push for review
16:04eric_engestrom: I'm disabling pushes to main right now, let's see if/when this causes an issue
16:04cmarcelo: eric_engestrom: ok (make sure marge can still "marge" though :) )
16:04eric_engestrom: one setting that gitlab doens't have is to disallow creating new branches, so that people can't push their branches to upstream mesa (like ajax also did 12h ago)
16:04eric_engestrom: cmarcelo: yep, I'm keeping an eye on the next MR to make sure I'm not breaking that ^^
16:04cmarcelo: eric_engestrom: tks :)
16:04cmarcelo: eric_engestrom: I think if ajax fix up his setup, the issues will be gone. I'm seeing this particular case more as an accident.
16:05alyssa: that's a.. generous view of genai
16:05eric_engestrom: oh I agree, but it's an accident that's bound to happen again
16:05eric_engestrom: if not him, someone else
16:05alyssa: I don't see this as about him or his setup, it's the tech
16:06alyssa: gentoo's solution seems entirely reasonable here.
16:06eric_engestrom: (re- pushing to main, gitlab actually has two settings, one for "merge" and one for "push & merge"; I've set the latter to "no-one" while leaving the former to "developers and above")
16:06karolherbst: alyssa: I was starting to draft things here: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/38887 16:06karolherbst: a "while" ago
16:07karolherbst: but if we want to use the gentoo thing that's also good
16:07eric_engestrom: agreed with alyssa, the problem is inherent to that tech, not something that "some future update will fix"
16:08karolherbst: lucky that I can decide what's best for the project and don't have to take into account what my employer wants or doesn't want 🙃 so I don't even feel conflicted participating in that discussion :D
16:09cmarcelo: karolherbst: that patch clarifying the expectations seems reasoanble.
16:09karolherbst: Though in my experience, some people will engage on the topic and get real defensive and we might want to cover that we don't argue about it outside of significant contributors..
16:11karolherbst: anyway, we should decide if we want to go with "every author is 100% and fully and completely responsible for anything they push, duh" which is already the case, or just disallow it outright
16:11cmarcelo: alyssa: I agree I have a more generous view of the new tooling. without more context the Gentoo AI Policy linked seems overkill to me.
16:12karolherbst: though the latter might also been seen as a community/indsutry why example setting, because "mesa banning AI" will for sure cause headlines and will for sure make some employers have "conversations" with some of the contributors/maintainers
16:12karolherbst: s/why/wide
16:12karolherbst: we could practically ban genAI contributions while not being as explicit about it in written form
16:12karolherbst: in order to avoid uncomfortable situations for maitnainers
16:13karolherbst: (it also shows the mess of genAI, that I even bring it up in this way)
16:13cmarcelo: karolherbst: why isn't "commiter is responsible for what it commits" sufficient?
16:13karolherbst: ethical reasons
16:14eric_engestrom: honnestly, I'm in the situation where my company will not like it if we ban "AI" but I'm still in favour of doing it fully and explicitly
16:14karolherbst: yeah... that's why I wouldn't want to do a full ban without having a serious conversation and an ack from almost everybody
16:15karolherbst: we can do the "everybody is responsible" thing first and go further if we want to
16:16karolherbst: but also fucked up that people feel that they will be retaliated against if we really do this...
16:16eric_engestrom: can you create an issue and send an email to mesa-dev@ ?
16:16eric_engestrom: with something like "if we haven't heard convincing arguments against banning, we'll go ahead in a month" so that we don't continue being in limbo for another year+
16:17karolherbst: I think it would be better if somebody else creates that issue :')
16:17eric_engestrom: "everybody is responsible" is already the case, the problem with that is that it's always excused as "this was a mistake, won't happen again"
16:17eric_engestrom: I'm unfortunately in a position where me pushing for that would not go well, but supporting it should be fine™
16:18karolherbst: yeah.. same probably
16:18eric_engestrom: 🙃
16:19karolherbst: well it is written in our business ethics&morals doc that doing any determination for the benefit of upstream isn't a conflict of interest even if it goes against Red Hat interests, sooo on paper I'm good, but I still would like to avoid "conversations" on this topic heh
16:21cmarcelo: eric_engestrom: did someone reached out to the committer and explained "hey, don't push this stuff on main repo"? if the warning was ignored, can we just temporarily disable for an user until his setup gets sorted out? for the problem presented here I do think narrowing down direct push access (marge seems to be a success story anyway) is a good solution.
16:21karolherbst: push access was already disabled, so if marge still works there is nothing to be done, right?
16:22cmarcelo: karolherbst: yes yes