00:05fdobridge: <![NVK Whacker] Echo (she) 🇱🇹> @karolherbst Here's the log of the failed submission: https://pastebin.com/KLPG2SK5
00:05fdobridge: <karolherbst🐧🦀> @asdqueerfromeu mind also sharing the latest stuff in dmesg?
00:09fdobridge: <![NVK Whacker] Echo (she) 🇱🇹> https://pastebin.com/fTBembfG
00:10fdobridge: <gfxstrand> That may cause you real problems, actually.
00:10fdobridge: <gfxstrand> If someone ever writes to that image, it could lead to any number of memory corruptions
00:11fdobridge: <gfxstrand> I wish that assert were a joke we could just remove. 😭
00:11fdobridge: <karolherbst🐧🦀> ohh, so you think the GPU trashes the command buffers?
00:12fdobridge: <karolherbst🐧🦀> that would be.... bad, yes
00:12fdobridge: <gfxstrand> It's certainly possible.
00:12fdobridge: <karolherbst🐧🦀> I have a funky debug idea
00:12fdobridge: <gfxstrand> NAK gets rid of it but GL depends on it.
00:12fdobridge: <karolherbst🐧🦀> snapshot the command buffer and validate its content is the same 🙃
00:12fdobridge: <gfxstrand> Might be another good use for an API flag.
00:13fdobridge: <karolherbst🐧🦀> and whenever it changes, report it
00:13fdobridge: <karolherbst🐧🦀> at least if the command buffer would be corrupted on the CPU side, printing it should produce random garbabe
00:13fdobridge: <karolherbst🐧🦀> *garbage
00:13fdobridge: <karolherbst🐧🦀> but it doesn't
00:13fdobridge: <![NVK Whacker] Echo (she) 🇱🇹> I don't think NAK has proper support for non-compute shaders though yet
00:14fdobridge: <karolherbst🐧🦀> `subc 7 mthd 2664 data 0000007f`....
00:15fdobridge: <karolherbst🐧🦀> we don't use subc 7 at all 🙂
00:15fdobridge: <karolherbst🐧🦀> anyway.. whatever happens, the CPU and GPU disagree on what got submitted
00:17fdobridge: <gfxstrand> @asdqueerfromeu nvk/codegen-11bit-image-hack
00:17fdobridge: <karolherbst🐧🦀> why didn't it ever occur to me to ever verify that the GPU didn't trash the push buffers 🙃
00:18fdobridge: <karolherbst🐧🦀> I should add a runtime flag for gl as well to detect those corruptions
00:19fdobridge: <karolherbst🐧🦀> and track down all the tests hitting that
00:20fdobridge: <![NVK Whacker] Echo (she) 🇱🇹> What issues did you encounter without the codegen part?
00:22fdobridge: <karolherbst🐧🦀> 2d/3d handling in codegen is terrible
00:24fdobridge: <karolherbst🐧🦀> but 2d slices of 3d images are also just pure evil
00:26fdobridge: <gfxstrand> Oh, just that we can't upstream it without totally breaking GL.
00:27fdobridge: <gfxstrand> Look at how NVK does it. 😝
00:27fdobridge: <karolherbst🐧🦀> ~~we could revert that stuff~~
00:27fdobridge: <karolherbst🐧🦀> vulkan has an explicit flag for it
00:28fdobridge: <karolherbst🐧🦀> so yeah, disabling tiling on the z axis and then just binding the slice is a proper way of dealing wiht that in vulkan (I didn't check, but I assume that's what nvk does 😛 )
00:28fdobridge: <karolherbst🐧🦀> and I think I even wrote some of the code? dunno 😄
00:28fdobridge: <gfxstrand> Yup
00:28fdobridge: <karolherbst🐧🦀> in gl it's all madness
00:29fdobridge: <gfxstrand> But also, 3D only have 11 bits of Z and you have an extra 12 bits at the top of the 32 bit handle...
00:29fdobridge: <gfxstrand> You can pull the same stunt without restricting to 11 bits
00:29fdobridge: <karolherbst🐧🦀> fair
00:32fdobridge: <![NVK Whacker] Echo (she) 🇱🇹> I'll have to try DXVK v1.10.3 to see if synchronization2/vulkanMemoryModel is the problem
00:34fdobridge: <gfxstrand> We can about turn on sync2. I just haven't because it increases the test count so much for not really any extra coverage for us.
00:35fdobridge: <![NVK Whacker] Echo (she) 🇱🇹> I remember the CTS tests failing like crazy (not sure for which extension though)
00:37fdobridge: <gfxstrand> That's quite possible. I did say "almost". 😝
00:49fdobridge: <![NVK Whacker] Echo (she) 🇱🇹> But anyway besides the synchr2/memmodel faux enablement and EXT_memory_budget implementation patches I have no more feature patches in my local PKGBUILD 🐸
01:42fdobridge: <airlied> we do submits with 0 no pushes, probably shouldn't have submits with 0 length pushes
01:45fdobridge: <airlied> guess next week will be retarget the MRs week 😛
01:48fdobridge: <gfxstrand> @karolherbst I think your codegen patch is okay
01:48fdobridge: <gfxstrand> We don't. We already filter those out
01:52fdobridge: <gfxstrand> Why am I faulting inside the kernel's memory range?
01:53fdobridge: <gfxstrand> This sounds like not my bug. 😛
01:56fdobridge: <gfxstrand> @airlied Where should I file suspected kernel bugs?
01:58fdobridge: <gfxstrand> Hrm... It fails on the old API
02:00fdobridge: <airlied> the mailing list or maybe the nouvelles issue tracker if you just want a place holder to track them
02:00fdobridge: <airlied> if you suspect they are with the new uapi make sure to point dakr at them
02:18fdobridge: <gfxstrand> I'm going to have to hack on the CTS for this one. 😕
05:29fdobridge: <![NVK Whacker] Echo (she) 🇱🇹> I think you forgot to mark certain features that are restricted to newer NVIDIA GPU architectures as such in the features.txt file (both anv and turnip already do that)
08:07fdobridge: <![NVK Whacker] Echo (she) 🇱🇹> Also should I remove the synchr2/memmodel hacks in the AUR package? 🐸
09:31fdobridge: <karolherbst🐧🦀> https://gitlab.freedesktop.org/drm/nouveau/-/issues
09:40fdobridge: <karolherbst🐧🦀> though if anybody wants to get into nouveau kernel development, there is a good list of things to start with 🙃
09:41fdobridge: <dadschoorse> https://github.com/doitsujin/dxvk/pull/3603 I hope nak will also support ffmaz/fmulz when it gets merged
09:42fdobridge: <karolherbst🐧🦀> isn't there a better way to check?
09:43fdobridge: <dadschoorse> no
09:43fdobridge: <karolherbst🐧🦀> or is it about knowing if it's lowered or not?
09:43fdobridge: <![NVK Whacker] Echo (she) 🇱🇹> This is probably one of the first NVK-specific changes outside of Mesa/kernel
09:43fdobridge: <dadschoorse> it's about knowing if the terrible emulation pattern that dxvk emits will be optimized to something that's fast
09:44fdobridge: <karolherbst🐧🦀> ahh
09:44fdobridge: <karolherbst🐧🦀> so the proper answer here would be a spir-v extension or something?
09:45fdobridge: <dadschoorse> yeah but do you want to put d3d9 garbage into spirv?
09:45fdobridge: <karolherbst🐧🦀> it's one opcode
09:46fdobridge: <karolherbst🐧🦀> there are already other weirdo things in spirv, I think it would be fine 😄
09:47fdobridge: <dadschoorse> it's two opcodes with fma
09:48fdobridge: <karolherbst🐧🦀> could do it an evil way and make it a decoration instead
09:48fdobridge: <dadschoorse> but from past discussions there also was the question if it shouldn't be a per shader mode that allows intel to use their d3d9 hw
09:48fdobridge: <karolherbst🐧🦀> ohh.. right
09:49fdobridge: <karolherbst🐧🦀> so rather an execution mode
09:49fdobridge: <karolherbst🐧🦀> ehh wait
09:49fdobridge: <karolherbst🐧🦀> this other thing
09:50fdobridge: <karolherbst🐧🦀> ahh no, execution mode might be fine
09:50fdobridge: <karolherbst🐧🦀> why did they call it execution model and mode
09:50fdobridge: <karolherbst🐧🦀> it's confusing
09:51fdobridge: <dadschoorse> anyway, just hard coding it in dxvk isn't too terrible imo, it works well for radv
09:51fdobridge: <dadschoorse> the only thing we would maybe gain with a two opcode extension is nvidia prop support
09:51fdobridge: <karolherbst🐧🦀> yeah.. and a more reliable solution
09:51fdobridge: <karolherbst🐧🦀> and intel
09:52fdobridge: <karolherbst🐧🦀> if it's an execution mode isntead
09:52fdobridge: <dadschoorse> the problem with the intel mode is that it affects a lot more opcodes than just mul/fma afaiu
09:52fdobridge: <karolherbst🐧🦀> mhhhh
09:52fdobridge: <karolherbst🐧🦀> would be good to know what exactly
09:53fdobridge: <dadschoorse> I think it also changes things like rcp(0)
09:53fdobridge: <karolherbst🐧🦀> well.. that might be fine even, because rcp is already defined in a wonky way
09:54fdobridge: <dadschoorse> or pow(0,0)
09:54fdobridge: <karolherbst🐧🦀> same for pow 😄
09:54fdobridge: <karolherbst🐧🦀> I could imagine that those return 0 a bit more often than they would normally do, but that's still all within spec limits, no?
09:54fdobridge: <dadschoorse> yeah maybe it's fine but I don't want to be the guy advocating for it in khronos 🐸
09:54fdobridge: <karolherbst🐧🦀> but isn't pow also implemented with rcp and mul?
09:55fdobridge: <dadschoorse> exp and mul
09:55fdobridge: <karolherbst🐧🦀> right..
09:55fdobridge: <karolherbst🐧🦀> well.. fmulz technically
09:55fdobridge: <dadschoorse> but not on all intel hw
09:55fdobridge: <rhed0x> We hit that with Alan wake, didn't we?
09:55fdobridge: <karolherbst🐧🦀> nouveau lowers with fmulz because it violates the spec otherwise
09:55fdobridge: <dadschoorse> I think only on gen12+, they actually had hw pow before
09:58fdobridge: <dadschoorse> I think that was nrm
09:58fdobridge: <rhed0x> Oh, yeah right
09:58fdobridge: <dadschoorse> which uses rsq internally
10:00fdobridge: <dadschoorse> the other question is if accommodating intel is worth it, it sounds like they might drop the d3d9 mode soon
10:00fdobridge: <dadschoorse> maybe they add fmulz instead (I doubt it but it would be cool 🐸)
10:05fdobridge: <karolherbst🐧🦀> I'm annoyed that some parts of spir-v are super poorly speced 😢
10:08fdobridge: <dadschoorse> I'm annoyed that vulkan still has no real fma
10:19fdobridge: <karolherbst🐧🦀> right... that's also terrible 😄
10:20fdobridge: <karolherbst🐧🦀> it will be needed for CL anyway
10:20fdobridge: <karolherbst🐧🦀> that reminds me.. I should work on getting proper ffma support in.. uhhh
10:20fdobridge: <karolherbst🐧🦀> that can of worms
10:20fdobridge: <karolherbst🐧🦀> we'd have to fix nir first
10:21fdobridge: <karolherbst🐧🦀> and add a `fmad` and make `ffma` a real `ffma`
10:21fdobridge: <karolherbst🐧🦀> otherwise it's all pointless
10:21fdobridge: <![NVK Whacker] Echo (she) 🇱🇹> What does `mad` mean in this case?
10:21fdobridge: <karolherbst🐧🦀> unfused
10:21fdobridge: <dadschoorse> muladd
10:22fdobridge: <dadschoorse> umad?
10:22fdobridge: <karolherbst🐧🦀> it's a bit of a pain for opt_algebraic
10:22fdobridge: <karolherbst🐧🦀> and it's a pain on so many levels
10:23fdobridge: <dadschoorse> are there backends that uses ffma for unfused mad atm?
10:23fdobridge: <karolherbst🐧🦀> yes
10:23fdobridge: <karolherbst🐧🦀> nouveau does
10:23fdobridge: <karolherbst🐧🦀> nvidia hardware only has one of them
10:23fdobridge: <karolherbst🐧🦀> never both
10:23fdobridge: <karolherbst🐧🦀> they flipped from fmad to ffma at one point
10:23fdobridge: <dadschoorse> does your compiler not fuse in the backend?
10:24fdobridge: <karolherbst🐧🦀> it does
10:24fdobridge: <karolherbst🐧🦀> well.. unless it's precise
10:24fdobridge: <dadschoorse> then you can just lower ffma on hardware that doesn't have real fma
10:24fdobridge: <dadschoorse> even if the hw has unfused mad?
10:25fdobridge: <karolherbst🐧🦀> good question...
10:25fdobridge: <karolherbst🐧🦀> I think we never bothered?
10:25fdobridge: <karolherbst🐧🦀> or maybe I did?
10:25fdobridge: <karolherbst🐧🦀> it's all a bit funky
10:26fdobridge: <dadschoorse> I think in the end nir to tgsi might be the most problematic backend because tgsi also has the stupid definition of fma as "might be fused"
10:26fdobridge: <karolherbst🐧🦀> yeah..
10:26fdobridge: <karolherbst🐧🦀> the real battle is getting glsl vs cl semantics in place
10:26fdobridge: <karolherbst🐧🦀> in CL fma is fma
10:27fdobridge: <karolherbst🐧🦀> and fmad is whatever
10:28fdobridge: <karolherbst🐧🦀> and then there is `-cl-mad-enable`
10:28fdobridge: <karolherbst🐧🦀> `Allow a * b + c to be replaced by a mad instruction. The mad instruction may compute a * b + c with reduced accuracy in the embedded profile. See the OpenCL C or OpenCL SPIR-V Environment specification for accuracy details. On some hardware the mad instruction may provide better performance than the expanded computation.`
10:29fdobridge: <dadschoorse> that would just mean the mul could be imprecise in NIR I guess
10:29fdobridge: <karolherbst🐧🦀> yeah
10:29fdobridge: <karolherbst🐧🦀> I think we already handle that part
10:29fdobridge: <karolherbst🐧🦀> but ffma not so much
10:29fdobridge: <karolherbst🐧🦀> libclc has actual ffma lowering and we use that so far I think
10:29fdobridge: <karolherbst🐧🦀> I really have to fix that nonsense
10:31fdobridge: <karolherbst🐧🦀> anyway.. supporting it on hardware with just one is simple
10:31fdobridge: <karolherbst🐧🦀> AMD is the weird oddball here
10:31fdobridge: <karolherbst🐧🦀> having hardware with both, where fma is sometimes slower
10:32fdobridge: <dadschoorse> amd has hw with both at the same speed, both but fma is slow and hw with only fma
10:32fdobridge: <karolherbst🐧🦀> yeah
10:32fdobridge: <karolherbst🐧🦀> kinda don't get why there is hardware with both at the same speed 😄
10:32fdobridge: <karolherbst🐧🦀> but whatever
10:32fdobridge: <karolherbst🐧🦀> is there actually a good reason for that?
10:33fdobridge: <dadschoorse> because you can use mad if the mul is precise
10:33fdobridge: <karolherbst🐧🦀> I question the benefits of making the architecture more complex, but maybe it pays off ...
10:33fdobridge: <karolherbst🐧🦀> I just doubt it
10:34fdobridge: <dadschoorse> amd's unfused mad also always flushes denorms to zero, which is fun for fp16
10:34fdobridge: <karolherbst🐧🦀> uhhhh
10:35fdobridge: <karolherbst🐧🦀> yeah, that sounds like fun
10:35fdobridge: <karolherbst🐧🦀> it's already such a corner case, but I also added that precise handling to TGSI because some games hit it 🙃
10:36fdobridge: <dadschoorse> radv had a ton of issues in games when rdna2 was released as the first hw without mad
10:36fdobridge: <dadschoorse> now we treat everything that is used to calculate position in vertex shaders as precise 🐸
10:37fdobridge: <karolherbst🐧🦀> funky
11:19fdobridge: <![NVK Whacker] Echo (she) 🇱🇹> Will nouveau/mesa repository still receive updates? 🐸
11:41fdobridge: <![NVK Whacker] Echo (she) 🇱🇹> But anyway the new uAPI patches managed to apply without any conflicts on 6.4 (I had to fix one mistake though) and they work fine in some games that I've tried (the performance is too low to play them properly so I can't give a proper stability rating)
12:26fdobridge: <conan_kudo> Congratulations on landing NVK!
12:51Ermine: Congrats on merging NVK!
13:25fdobridge: <gfxstrand> Thanks! 🥳
13:56fdobridge: <gfxstrand> Should be able to. As long as it's correct and just a matter of perf if we can reduce the pattern.
14:09fdobridge: <gfxstrand> No. The branch there has been removed. Any still useful MRs should be re-targeted.
14:11fdobridge: <gfxstrand> There's a part of me that's inclined to continue using the issue tracker for stuff like tracking all the misc features just to keep them out of the public eye.
14:16fdobridge: <gfxstrand> Cool. I did successfully get through a run last night after I disabled iwlwifi. 🙃
14:30fdobridge: <![NVK Whacker] Echo (she) 🇱🇹> I can still see nvk/main
14:32fdobridge: <gfxstrand> Probably need to push main and make it the primary branch before we can delete NVK/main
14:36fdobridge: <gfxstrand> There were go
14:38fdobridge: <gfxstrand> Really, I just renamed nvk/main to main.
14:47fdobridge: <![NVK Whacker] Echo (she) 🇱🇹> I'm kind of considering making an Arch kernel package with new uAPI support or an option in linux-tkg to use the new uAPI patches :nouveau:
14:56fdobridge: <gfxstrand> Sure. Hopefully we won't need that for long. Getting something for GSP might be useful, too.
15:07fdobridge: <karolherbst🐧🦀> ohh yeah.. I think we want a kernel with GSP and the new uapi so people can already try things out, but they'll be disappointed with the perf 😄
15:07fdobridge: <karolherbst🐧🦀> @gfxstrand what's the pass called in anv to promote ubos to bound ones? I might look into it next week then.
15:07fdobridge: <gfxstrand> There isn't one
15:08fdobridge: <gfxstrand> Oh, in ANV? It's just part of apply_pipeline_layout
15:08fdobridge: <karolherbst🐧🦀> yeah
15:09fdobridge: <karolherbst🐧🦀> that `try_lower_direct_buffer_intrinsic` stuff?
15:10fdobridge: <gfxstrand> That's part of it but no
15:10fdobridge: <![NVK Whacker] Echo (she) 🇱🇹> Hopefully hwmon and display parts can be eventually hooked up
15:11fdobridge: <gfxstrand> It's a more general part of the pass. It has some heuristics about what gets placed where. It counts uses of each resource and promotes the most commonly used ones to bound.
15:11fdobridge: <karolherbst🐧🦀> right...
15:12fdobridge: <karolherbst🐧🦀> we do however have 18 slots for graphics, so we might as well just use the first n and add some smarter heurestics later if we really care enough
15:12fdobridge: <gfxstrand> But before we can do that we need to fix codegen so it doesn't tweak with cb indices. Right now we're binding the roof descriptors to all the bind points just in case. 🙄
15:12fdobridge: <karolherbst🐧🦀> not sure if it's common enough in vulkan to have more
15:12fdobridge: <karolherbst🐧🦀> uhhh
15:12fdobridge: <karolherbst🐧🦀> I see
15:13fdobridge: <karolherbst🐧🦀> yeah, I can look into that as well while at it
15:13fdobridge: <karolherbst🐧🦀> however
15:13fdobridge: <karolherbst🐧🦀> we could bind the root descriptor at info.io.auxCBSlot and then use that
15:13fdobridge: <karolherbst🐧🦀> and it would be the last and it _should_ mostly work
15:13fdobridge: <karolherbst🐧🦀> but anyway
15:13fdobridge: <karolherbst🐧🦀> I can look into that
15:14fdobridge: <karolherbst🐧🦀> I guess a vulkan driver also won't need `lower_uniforms_to_ubo` actually?
15:14fdobridge: <karolherbst🐧🦀> where do push constants land atm?
15:15fdobridge: <gfxstrand> I think we are? But at least for a time it was tweaking UBOn indices so load_ubo(n, offset) might load from n+1
15:15fdobridge: <gfxstrand> Maybe it isn't anymore?
15:15fdobridge: <gfxstrand> 🤷🏻♀️
15:15fdobridge: <karolherbst🐧🦀> I've ported to `lower_uniforms_to_ubo` at some point
15:15fdobridge: <karolherbst🐧🦀> so codegen itself shouldn't do any n+1 anymore
15:15fdobridge: <gfxstrand> Okay then maybe we're okay
15:15fdobridge: <karolherbst🐧🦀> https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22815
15:16fdobridge: <karolherbst🐧🦀> maybe it was @mhenning 🙃
15:17fdobridge: <karolherbst🐧🦀> anyway.. we have to decide where to put push constants, though I guess the best thing would be to reserve space in the root descriptor and that's probably already done
15:17fdobridge: <karolherbst🐧🦀> so we need 1 ubo (the last?) for driver internal + root descriptor fun, and the other ones are free for ubos
15:17fdobridge: <karolherbst🐧🦀> should make it easier to have it the last one, as then ubos are 0 to n - 1, where n is 7/16/18 depending on the stage/gpu
15:18fdobridge: <karolherbst🐧🦀> *7/15/17
15:18fdobridge: <karolherbst🐧🦀> mhh
15:18fdobridge: <karolherbst🐧🦀> what about shader constants
15:19fdobridge: <karolherbst🐧🦀> maybe 32k from the root descriptor as well? Not sure how big that one can become
15:20fdobridge: <karolherbst🐧🦀> we kinda need to extract indirectly accessed constants from shaders, because perf
15:20fdobridge: <gfxstrand> Uh... We've had push constants since forever.
15:20fdobridge: <karolherbst🐧🦀> like .. all of them
15:20fdobridge: <karolherbst🐧🦀> sure, but where do they go atm? 😄
15:21fdobridge: <gfxstrand> In the root descriptor table.
15:21fdobridge: <karolherbst🐧🦀> okay
15:21fdobridge: <karolherbst🐧🦀> how big is the root descriptor in the worst case?
15:21fdobridge: <karolherbst🐧🦀> or do we have some kind of limit?
15:21fdobridge: <gfxstrand> Like 1K maybe
15:22fdobridge: <karolherbst🐧🦀> ahh, that's not much
15:22fdobridge: <gfxstrand> Maybe 512
15:22fdobridge: <karolherbst🐧🦀> okay, so we can put all that driver stuff into one const buffer
15:22fdobridge: <gfxstrand> Yup
15:22fdobridge: <karolherbst🐧🦀> root descriptor + push constants + nir_shader.constant_data
15:22fdobridge: <gfxstrand> And it already has a bunch of stuff in it.
15:22fdobridge: <karolherbst🐧🦀> okay
15:23fdobridge: <karolherbst🐧🦀> do we already have a pass to lower _all_ indirectly accessed constants to `constant_data`?
15:23fdobridge: <gfxstrand> nvk_cmd_buffer.h
15:23fdobridge: <gfxstrand> IDK what you mean by that.
15:23fdobridge: <karolherbst🐧🦀> like if you have an array of constants in the sahder, and the shader accesses them indirectly
15:23fdobridge: <karolherbst🐧🦀> we need them lowered
15:24fdobridge: <karolherbst🐧🦀> all of them 😄
15:24fdobridge: <gfxstrand> Right. nir_lower_large_constants(), I think
15:24fdobridge: <karolherbst🐧🦀> except we need it for all constants
15:24fdobridge: <karolherbst🐧🦀> of any size
15:24fdobridge: <gfxstrand> It has a threshold
15:24fdobridge: <karolherbst🐧🦀> yeah.. and we need it to be 1
15:25fdobridge: <karolherbst🐧🦀> probably
15:25fdobridge: <karolherbst🐧🦀> codegen lowers it to local mem in all cases
15:25fdobridge: <karolherbst🐧🦀> and that hurts a lot
15:25fdobridge: <gfxstrand> Well, you can bcsel a couple if needed but yeah, pulling them into a UBO.
15:25fdobridge: <karolherbst🐧🦀> yeah..
15:26fdobridge: <karolherbst🐧🦀> I'd rather put them all into an ubo 😄
15:26fdobridge: <karolherbst🐧🦀> it's easier
15:26fdobridge: <karolherbst🐧🦀> so we have about 62k of space in the "driver" ubo, should be plenty
15:30fdobridge: <karolherbst🐧🦀> anyway.. I had a funky idea once to have a pre baked constant table shared across all shaders, so we don't have to reupload buffers all the time, but we can also just pull out all constants as it does improve codegen especially for older gpus.. but that needs to be done in the backend compiler, because not all instructions can load from ubos directly.... anyway, later we should lower all indirectly accessed constants in nir, and d
15:30fdobridge: <karolherbst🐧🦀> but first that ubo stuff
15:31fdobridge: <karolherbst🐧🦀> https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4945
15:31fdobridge: <karolherbst🐧🦀> that's what I've played around in the past
15:37fdobridge: <karolherbst🐧🦀> I also had a pass to pull in in shader constants into the buffer, but some people didn't like the idea of reuploading per shader constants on each bind
16:03dakr: airlied, gfxstrand: sent out the nouveau svm fix (https://lore.kernel.org/dri-devel/20230805160027.88116-1-dakr@redhat.com/T/#u)
16:04DodoGTA: dakr: I'm kind of surprised that mistake wasn't noticed
16:15fdobridge: <gfxstrand> Yeah, and because of how constants work, we can put multiple things in the same cb. IDK how that relates to recycling of the backing storage, though. It can't be infinite so there have to be stalls sometime. I'm still getting used to how Nvidia does things...
16:18karolherbst: nobody builds with SVM enabled :')
16:20fdobridge: <karolherbst🐧🦀> yeah... I think it doesn't make sense to have a shader with a constant buffer of like 8 bytes, and `opt_large_constants` with a big enough threshold should be useful. But then there are other considerations like, if you have constant table with tons of indirect accesses you might want to do it regardless
16:20fdobridge: <karolherbst🐧🦀> but
16:20fdobridge: <karolherbst🐧🦀> we upload the driver constant buffer _anyway_ so we might as well put constants there
16:23fdobridge: <karolherbst🐧🦀> but anyway, nvidia has up to 18 slots and they all exists on hardware afaik
16:24fdobridge: <karolherbst🐧🦀> I'm mostly curious on why compute only has 7 and what they use the space for? maybe it's some huge cache for other things?
16:24fdobridge: <karolherbst🐧🦀> mhh.. apparently ptx has 11 slots..
16:26fdobridge: <karolherbst🐧🦀> ohh interesting.. so apparently each SM has a 4/8kb constant memory cache on top of the backing storage
16:29fdobridge: <gfxstrand> Yeah, if we have 18 slots, we can burn one for shader constants. We just need to make lower_descriptors rewrite lower_constant accordingly.
16:29fdobridge: <karolherbst🐧🦀> well.. we have only 8 in compute, but maybe they changed it recently...
16:29fdobridge: <karolherbst🐧🦀> it's weird that ptx has 11
16:30fdobridge: <karolherbst🐧🦀> maybe it was done for random reasons
16:30fdobridge: <gfxstrand> In ANV, we do a trick with shader relocations to put them at the end of the shader binary and not even have to worry about binding. 🙃
16:31fdobridge: <karolherbst🐧🦀> heh
16:31fdobridge: <gfxstrand> We could go the same with NVK with bindless UBOs
16:31fdobridge: <karolherbst🐧🦀> `NVC7C0_QMDV02_03_CONSTANT_BUFFER_VALID(i) MW((640+(i)*1):(640+(i)*1))`
16:31fdobridge: <karolherbst🐧🦀> `NVC7C0_QMDV02_03_REGISTER_COUNT_V MW(656:648)`
16:31fdobridge: <karolherbst🐧🦀> looks like 8 for compute still
16:32fdobridge: <karolherbst🐧🦀> whatever...
16:32fdobridge: <karolherbst🐧🦀> having 17 in the graphics pipeline is more important 😄
16:32fdobridge: <karolherbst🐧🦀> (+1 for our internal stuff)
16:33fdobridge: <karolherbst🐧🦀> I have to check which arch added two more, but I think it was 2nd gen maxwell
18:40fdobridge: <gfxstrand> I thought reading phoronix comments would be amusing but no... It's just depressing. 😢
18:41fdobridge: <karolherbst🐧🦀> depends on the topic, nvidia is one to avoid
18:41fdobridge: <gfxstrand> Yeah, lots of hatred and fighting
18:42fdobridge: <karolherbst🐧🦀> asahi is also such a topic :/
18:43fdobridge: <karolherbst🐧🦀> sometimes I'm in the mood of "discussing" with trolls there, but often I'm not 🙃
18:47fdobridge: <georgeouzou> I encountered a weird thing. Generated primitive queries are correct if I use the push_sync debug variable. If not, I get some errors on the queries that also use xfb.
18:58fdobridge: <karolherbst🐧🦀> sounds like a sync problem then
19:13fdobridge: <gfxstrand> Ugh... codegen optimizer is screwing up again. 🙄
19:14fdobridge: <gfxstrand> In this case, it's somehow turning a u2u64(load_ubo()) into just loading a 64-bit value.
19:15fdobridge: <gfxstrand> dEQP-VK.binding_model.buffer_device_address.set0.depth1.basessbo.convertuvec2.nostore.multi.std140.comp
19:16fdobridge: <gfxstrand> I guess I could just lower 64-bit arithmetic in NIR
19:16fdobridge: <gfxstrand> Or I could not care
19:16fdobridge: <gfxstrand> But also this is the source of a lot of faults
19:17fdobridge: <mhenning> I'll take a look at that one - it might just be peephole messing up again
19:17fdobridge: <gfxstrand> Thanks!
19:17fdobridge: <karolherbst🐧🦀> creative
19:18fdobridge: <gfxstrand> Yeah, when that value goes into a (x << 4) and then gets added to an address it turns out those top bits not being zero can screw things up. 🙃
19:19fdobridge: <karolherbst🐧🦀> yeah...
19:19fdobridge: <karolherbst🐧🦀> u2u64 is just a move, so it's kinda funky why that even messes things up...
19:19fdobridge: <karolherbst🐧🦀> well.. cvt actually mhh
19:20fdobridge: <karolherbst🐧🦀> and that becomes a merge(src0, 0)
19:20fdobridge: <karolherbst🐧🦀> ohhh....
19:22fdobridge: <karolherbst🐧🦀> I wouldn't be surprised if LoadPropagation messes that up
19:28fdobridge: <gfxstrand> I suspect something like that.
19:29fdobridge: <gfxstrand> Because AFAICT, the second UBO element never gets loaded by the NIR. That load is invented by codegen from whole cloth.
19:35fdobridge: <karolherbst🐧🦀> yeah...
19:36fdobridge: <karolherbst🐧🦀> I suspect it's something not checking if it's a 64 bit thing 🙂
19:36fdobridge: <karolherbst🐧🦀> I think there is still some code in codegen which assumes that only 32 bit values exist
19:37fdobridge: <karolherbst🐧🦀> but I let@mhenning to figure it out 😄 Just wanted to share my wild guess here
19:38fdobridge: <mhenning> yeah, that wouldn't surprise me
19:49fdobridge: <airlied> @karolherbst there are v3 qmds maybe they expanded it
19:49fdobridge: <karolherbst🐧🦀> nah, they didn't. This PTX limit also existed since forever
19:50fdobridge: <karolherbst🐧🦀> I should check what nvidia does if you actually use all 11
19:50fdobridge: <karolherbst🐧🦀> let's see...
20:01fdobridge: <prop_energy_ball> If I want to contribute an ext implementation to NVK, should I be using the tree in mesa/mesa now or nouveau/mesa
20:02fdobridge: <airlied> The former
20:05fdobridge: <mhenning> Which load is it? What generation are you on? On kepler the only ld u64 I see is at ubo 0 offset 0xa0 and that's already 64-bit in the nir
20:08fdobridge: <gfxstrand> Turing
20:09fdobridge: <gfxstrand> It's a little over half-way through the test.
20:09fdobridge: <karolherbst🐧🦀> ```
20:09fdobridge: <karolherbst🐧🦀> /*00f0*/ UMOV UR4, '(cb10) ; /* 0x0000000000047882 */
20:09fdobridge: <karolherbst🐧🦀> /* 0x000fc60000000000 */
20:09fdobridge: <karolherbst🐧🦀> ```
20:09fdobridge: <karolherbst🐧🦀> the hell is this
20:10fdobridge: <karolherbst🐧🦀> they deprecated that banked const buffer feature in ptx 2.2
20:10fdobridge: <karolherbst🐧🦀> and it seems like they live patch the location at runtime 🙃
20:10fdobridge: <gfxstrand> A mov into a uniform register
20:10fdobridge: <karolherbst🐧🦀> what's that `(cb10)` thing tho 😛
20:10fdobridge: <karolherbst🐧🦀> it's not in the encoding
20:11fdobridge: <karolherbst🐧🦀> I'm sure they play weirdo tricks at runtime, like checking what you actually bind on launch time and then live patch the locations of the kernels
20:11fdobridge: <karolherbst🐧🦀> *instructions
20:12fdobridge: <karolherbst🐧🦀> however, I also see stuff like this generated: `IADD3 R0, R0, c[0xe][0x0], R11 ;`
20:12fdobridge: <gfxstrand> In NIR, you'll see `iadd(x, ushl(i2i64(load_ubo(0, n)), 4)` and a load that uses the result. I think it's the only 64-bit shift in the NIR.
20:13fdobridge: <karolherbst🐧🦀> even on 2nd gen kepler I see `MOV R0, c[0xe][0x0];` mhhhh
20:13fdobridge: <karolherbst🐧🦀> this is.... interesting
20:13fdobridge: <karolherbst🐧🦀> maybe there is a way to use those const buffers in compute?
20:13fdobridge: <gfxstrand> 🤷🏻♀️
20:17fdobridge: <karolherbst🐧🦀> mhhh no idea
20:38fdobridge: <mhenning> hmm. I switched to ampere and I still can't reproduce this. The test actually passes for me on ampere, and the only i2i64 I see in the nir is consumed by an ishl, not a ushl
20:39fdobridge: <mhenning> I don't think we set any lowering options differently on ampere vs turing, so I'm puzzled why I have different nir than you
20:41fdobridge: <mhenning> maybe I need a newer cts or something? my cts on this machine is from february
20:49fdobridge: <mhenning> No difference with latest cts
20:52fdobridge: <karolherbst🐧🦀> @gfxstrand I know you'll hate me for it, but I'll need derefs in spir-v constant ops 🙃
20:52fdobridge: <karolherbst🐧🦀> maybe it's a memory corruption thing?
21:02fdobridge: <gfxstrand> Could be ishl. I'm not looking at it right now.
21:02fdobridge: <gfxstrand> ?
21:02fdobridge: <karolherbst🐧🦀> `%17 = OpSpecConstantOp %_ptr_CrossWorkgroup_uchar InBoundsPtrAccessChain %a_var %uint_0 %ulong_1`
21:03fdobridge: <gfxstrand> Oh, right, that... 🙄
21:04fdobridge: <karolherbst🐧🦀> I'm just confused why it's doing a `OpSpecConstantOp` at all.... because there are no spec constants, but maybe I don't fully understand that part of spir-v yet
21:05fdobridge: <karolherbst🐧🦀> https://gist.githubusercontent.com/karolherbst/a60b4d92701f60d0c82cf69ce64a6082/raw/42b2cf3198071c75e71582e3022fdad7c226d01e/gistfile1.txt
21:07fdobridge: <karolherbst🐧🦀> ohh, seems like `OpSpecConstantOp` can't be updated and it's simply the result of the inputs
21:07fdobridge: <gfxstrand> It's horrible. You can, like, initialize global variables with pointers to other global things.
21:07fdobridge: <karolherbst🐧🦀> yeah, I can see that 🙃
21:08fdobridge: <gfxstrand> Full relocation madness
21:08fdobridge: <karolherbst🐧🦀> indeed
21:10fdobridge: <karolherbst🐧🦀> I'm not even sure I want to support that Intiializer/Finalize part, because that's soo poorly speced, I have not even any idea if you can have 100 of those and then in what order they'll have to be executed
21:10fdobridge: <karolherbst🐧🦀> but that part is also deprecated, so maybe I just hope we'll never need it
21:11fdobridge: <gfxstrand> 🤷🏻♀️
21:12fdobridge: <gfxstrand> But, like, you can theoretically size an array with the difference between two global pointers.
21:13fdobridge: <gfxstrand> I think they were trying to do c++ constrexpr but they really flubbed it.
21:13fdobridge: <karolherbst🐧🦀> yeah...
21:13fdobridge: <gfxstrand> IDK what we're going to do to deal with all the insanity
21:13fdobridge: <karolherbst🐧🦀> it's deprecated
21:13fdobridge: <karolherbst🐧🦀> well
21:13fdobridge: <karolherbst🐧🦀> part of it
21:14fdobridge: <karolherbst🐧🦀> Initializers and Finalizers are
21:14fdobridge: <karolherbst🐧🦀> and for the rest? dunno.. care if something needs it?
21:27fdobridge: <gfxstrand> Are you running on the new UAPI or old? For some reason, the fall only reproduces on new. Something to do with higher buffer addresses, I think.
21:28fdobridge: <mhenning> Oh, old. I haven't built a new kernel yet
21:28fdobridge: <karolherbst🐧🦀> ohh right... nouveau usually only sees buffer addresses within the 32 bit range 🙃
21:29fdobridge: <gfxstrand> Yeah, lots of fun bugs you find when you place buffers at the top of the space. There's a reason util_vma_heap defaults to that. 😝
21:29fdobridge: <karolherbst🐧🦀> yeah...
21:30fdobridge: <karolherbst🐧🦀> huh.. isn't then the bug that codegen simply cuts of high bits?
21:30fdobridge: <karolherbst🐧🦀> there... might be places where codegen assumes 32 bit pointers.... I think
21:31fdobridge: <karolherbst🐧🦀> do we even emit the `.E` field?
21:31fdobridge: <karolherbst🐧🦀> *flag
21:31fdobridge: <karolherbst🐧🦀> `.E` makes an address 64 bit instead of 32
21:32fdobridge: <karolherbst🐧🦀> `emitField(72, 1, insn->src(0).getIndirect(0)->getSize() == 8);` mhh. that's `.E` for `OP_LOAD`
21:33fdobridge: <karolherbst🐧🦀> but yeah.. things might assume 32 bit where they shouldn't
21:36fdobridge: <gfxstrand> No, the problem is that codegen adds stuff in. IDK why that only faults on the new API
21:37fdobridge: <gfxstrand> The compile is probably bad either way. The ishl should have 0 in the top 32 bits but it doesn't.
21:46fdobridge: <karolherbst🐧🦀> well.. the only reasonable explanation on why it faults on the new API is, if the high bit are now non 0 and they make it to where they shouldn't. And the shifts are a bit wonky, but it also depends on how they are emited. They are 64 bit operations, always, just the value is split across two registers
21:47fdobridge: <gfxstrand> Actually, I suspect it has to do with where the kernel places context memory.
21:47fdobridge: <gfxstrand> But whatever
21:47fdobridge: <gfxstrand> I kinda don
21:47fdobridge: <gfxstrand> I kinda don't care (edited)
21:50fdobridge: <gfxstrand> Yeah, it's LoadPropagation
21:51fdobridge: <karolherbst🐧🦀> I wouldn't be surprised if there is a "high bits of addresses are 0, therefore it's safe to..." code around somewhere
21:52fdobridge: <karolherbst🐧🦀> `TargetGV100::insnCanLoad` is probably at fault here
21:52fdobridge: <gfxstrand> No, it really is just this sift getting messed up
21:56fdobridge: <gfxstrand> Wait... this is more subtle than I thought
21:58fdobridge: <gfxstrand> Yeah, it's not just load propagation going banannas
22:04fdobridge: <gfxstrand> Maybe 64-bit shuffles can't take immediates?
22:06fdobridge: <karolherbst🐧🦀> they can
22:06fdobridge: <karolherbst🐧🦀> and if not, the hardware would complain in dmesg
22:07fdobridge: <karolherbst🐧🦀> however, it can only be a 32 bit immediate
22:08fdobridge: <karolherbst🐧🦀> could dump the binary and see if nvdisasm agrees
22:15fdobridge: <gfxstrand> I did
22:15fdobridge: <gfxstrand> nvdisasm seems okay
22:18fdobridge: <karolherbst🐧🦀> mhh.. could be something subtle, like pulling 64 bit from a const buffer instead of just 32. It all kinda depends on the instruction they are used in. But can also be just something super subtle, like an instruction writing to a 64 bit reg, even though only 32 was intended. nvdisasm is kinda terrible at showing that
22:18fdobridge: <karolherbst🐧🦀> I... kinda remember having fixed a rando bug like that before...
22:20fdobridge: <karolherbst🐧🦀> I also hope that I actually upstreamed such a fix
22:23fdobridge: <karolherbst🐧🦀> uhhhhmmmm
22:23fdobridge: <karolherbst🐧🦀> what kinda shift do you have there?
22:23fdobridge: <gfxstrand> I've pretty much got it narrowed down
22:26fdobridge: <gfxstrand> Works:
22:26fdobridge: <gfxstrand> ```
22:26fdobridge: <gfxstrand> 10: mov u32 $r8 c0[0x24] (16)
22:26fdobridge: <gfxstrand> ...
22:26fdobridge: <gfxstrand> 68: shf (SUBOP:3) s32 $r3 $r255 31 $r8 (16)
22:26fdobridge: <gfxstrand> ```
22:26fdobridge: <gfxstrand> Doesn't:
22:26fdobridge: <gfxstrand> ```
22:26fdobridge: <gfxstrand> 68: mov u32 $r2 0x0000001f (16)
22:26fdobridge: <gfxstrand> 69: shf (SUBOP:3) s32 $r7 $r255 $r2 c0[0x24] (16)
22:26fdobridge: <gfxstrand> ```
22:26fdobridge: <karolherbst🐧🦀> I should skim through my local git gree more often... I don't even remember having ever written this: https://gitlab.freedesktop.org/karolherbst/mesa/-/commit/4dad0e35cf29d2d28df878cf13b5dba64ac2db79
22:26fdobridge: <gfxstrand> It's the 32-bit shift that's borked
22:27fdobridge: <karolherbst🐧🦀> mhhh
22:27fdobridge: <karolherbst🐧🦀> at first it looks equal...
22:28fdobridge: <karolherbst🐧🦀> mhhh
22:28fdobridge: <karolherbst🐧🦀> it would be weird to pull 64 bits from `c0[0x24]`
22:28fdobridge: <karolherbst🐧🦀> _but_...
22:29fdobridge: <karolherbst🐧🦀> let me read some docs
22:29fdobridge: <gfxstrand> Forget the 32 bits
22:29fdobridge: <gfxstrand> It's just a sign-extend
22:29fdobridge: <gfxstrand> Forget the 64 bits (edited)
22:29fdobridge: <karolherbst🐧🦀> uhhhh
22:29fdobridge: <gfxstrand> But it's ending up with garbage somewhere
22:29fdobridge: <karolherbst🐧🦀> :painpeko:
22:29fdobridge: <karolherbst🐧🦀> mhhh
22:29fdobridge: <karolherbst🐧🦀> yeah well...
22:30fdobridge: <karolherbst🐧🦀> that's kinda funky
22:30fdobridge: <karolherbst🐧🦀> sooo
22:30fdobridge: <karolherbst🐧🦀> shf is kinda weird
22:30fdobridge: <gfxstrand> Yeah, I know
22:30fdobridge: <karolherbst🐧🦀> let's see...
22:31fdobridge: <karolherbst🐧🦀> what's the thing nvdisasm print for that shf?
22:33fdobridge: <gfxstrand> Good:
22:33fdobridge: <gfxstrand> ```
22:33fdobridge: <gfxstrand> /*01e0*/ SHF.R.S32.HI R3, RZ, 0x1f, R8 ; /* 0x0000001fff037819 */
22:33fdobridge: <gfxstrand> ```
22:33fdobridge: <gfxstrand> Bad:
22:33fdobridge: <gfxstrand> ```
22:33fdobridge: <gfxstrand> /*01a0*/ MOV R2, 0x1f ; /* 0x0000001f00027802 */
22:33fdobridge: <gfxstrand> /* 0x000fcc0000000f00 */
22:33fdobridge: <gfxstrand> /*01b0*/ SHF.R.S32.HI R7, RZ, R2, c[0x0][0x24] ; /* 0x00000900ff077619 */
22:33fdobridge: <gfxstrand> /* 0x000fde0000011402 */
22:33fdobridge: <gfxstrand> ```
22:33fdobridge: <karolherbst🐧🦀> okay, so those are at least clamped shifts...
22:34fdobridge: <karolherbst🐧🦀> yeah so that should be `signed(c[0x0][0x24]) >> R2`
22:37fdobridge: <karolherbst🐧🦀> heh..
22:37fdobridge: <karolherbst🐧🦀> I think my docs are buggy
22:38fdobridge: <karolherbst🐧🦀> there are two examples of `SHF.R.S32.HI` and they show a different operation each 🙃
22:38fdobridge: <gfxstrand> heh
22:39fdobridge: <karolherbst🐧🦀> once `signed(c[0x0][0x24]) >> R2` and the other is `signed(c[0x0][0x24]) >> R2 >> 32`
22:39fdobridge: <gfxstrand> right
22:40fdobridge: <karolherbst🐧🦀> I suspect the first should have been `.LO`
22:41fdobridge: <karolherbst🐧🦀> ehh.. or maybe that's fine, because of where the value went
22:42fdobridge: <karolherbst🐧🦀> yeah.. nvm
22:42fdobridge: <karolherbst🐧🦀> it's just weirdly explained
22:42fdobridge: <karolherbst🐧🦀> anyway.. yes, the value would be sign extented
22:42fdobridge: <karolherbst🐧🦀> even if the other reg is RZ
22:43fdobridge: <karolherbst🐧🦀> which kinda makes sense
22:44fdobridge: <gfxstrand> Yeah, lo doesn't help
22:45fdobridge: <karolherbst🐧🦀> dest, low, shift, high is the encoding
22:46fdobridge: <gfxstrand> Right
22:46fdobridge: <karolherbst🐧🦀> I still don't see why the two are different...
22:47fdobridge: <gfxstrand> I suspect it doesn't support all the encodings we think it does
22:47fdobridge: <gfxstrand> But that also seems weird
22:47fdobridge: <karolherbst🐧🦀> nah, the encodings are both valid
22:47fdobridge: <karolherbst🐧🦀> and again, the hardware would shout unless we encode random nonsense
22:47fdobridge: <karolherbst🐧🦀> but then nvdisasm would shout
22:48fdobridge: <karolherbst🐧🦀> It kinda feels like, that the bug might not be that instruction, it just shows because it's writing to a different reg now or something...
22:48fdobridge: <karolherbst🐧🦀> to me both of them, good and bad, look identical
22:49fdobridge: <karolherbst🐧🦀> mhhhhh
22:50fdobridge: <karolherbst🐧🦀> @gfxstrand mind running with `NV50_PROG_SCHED=0` just in case it's something with the sched stuff?
22:50fdobridge: <karolherbst🐧🦀> the last I want is, that it's a bug with the scheduling 🙃
22:51fdobridge: <gfxstrand> Yeah, doesn't help
22:51fdobridge: <karolherbst🐧🦀> I hope you also made sure it recompiled? Not sure if caching is set up already
22:51fdobridge: <gfxstrand> we have no caching
22:51fdobridge: <karolherbst🐧🦀> mhhhhh
22:53fdobridge: <gfxstrand> How has this never shown up?!?
22:53fdobridge: <karolherbst🐧🦀> some of those random weirdo bugs are terrible hard to track down
22:54fdobridge: <karolherbst🐧🦀> but it still feels like it's something else
22:54fdobridge: <karolherbst🐧🦀> sooo
22:54fdobridge: <karolherbst🐧🦀> `c[0x0][0x24]` is aligned for a 4 byte load
22:54fdobridge: <karolherbst🐧🦀> _if_ the shift would load 8 bytes, it wouldn't load them at `c[0x0][0x24]`, but `c[0x0][0x20]` instead
22:55fdobridge: <gfxstrand> The broken shift is the 32-bit one. The whole 64-bit thing was a red herring
22:55fdobridge: <karolherbst🐧🦀> yeah.. but I don't see how it's broken
22:56fdobridge: <gfxstrand> Okay, so this is interesting...
22:56fdobridge: <gfxstrand> handleShifts handles SHL and SHR differently
22:58fdobridge: <karolherbst🐧🦀> yeah, it has to
22:59fdobridge: <karolherbst🐧🦀> the ISA doc states "for 32 bit left shifts, do that:" and that's the code you see
22:59fdobridge: <karolherbst🐧🦀> anyway, I think the shift is alright, and it's something else going wrong
23:00fdobridge: <gfxstrand> Maybe
23:00fdobridge: <gfxstrand> Could be RA, I guess, but it seems pretty stable
23:00fdobridge: <karolherbst🐧🦀> might sharing the good and bad shader?
23:01fdobridge: <karolherbst🐧🦀> *mind
23:02fdobridge: <gfxstrand> Good
23:02fdobridge: <gfxstrand> https://cdn.discordapp.com/attachments/1034184951790305330/1137521073609506877/message.txt
23:02fdobridge: <gfxstrand> Bad
23:02fdobridge: <gfxstrand> https://cdn.discordapp.com/attachments/1034184951790305330/1137521151283822692/message.txt
23:05fdobridge: <karolherbst🐧🦀> ehhh...
23:07fdobridge: <karolherbst🐧🦀> ehh no, that's fine
23:08fdobridge: <gfxstrand> The value in `c[0x0][0x24]` is 1 for whatever it's worth
23:09fdobridge: <gfxstrand> The output I'm seeing seems consistent with it doing `0x1f >> 1` instead of `1 >> 0x1f`
23:09fdobridge: <karolherbst🐧🦀> yeah...
23:11fdobridge: <gfxstrand> But there's plenty of other SHFs with a constbuf argument that seem fine
23:12fdobridge: <gfxstrand> I mean, maybe our dependency tracker is busted?
23:19fdobridge: <karolherbst🐧🦀> okay.. I think I got it..
23:20fdobridge: <karolherbst🐧🦀> mind sharing the codegen ir output? 😄
23:20fdobridge: <karolherbst🐧🦀> but yeah... it's busted
23:20fdobridge: <karolherbst🐧🦀> but in the most annoying way
23:20fdobridge: <karolherbst🐧🦀> in the bad one you have `SHF.L.W.S64 R2, R6, 0x4, RZ ;`
23:20fdobridge: <karolherbst🐧🦀> and `SHF.R.S32.HI R7, RZ, R2, c[0x0][0x24] ;` before that
23:20fdobridge: <karolherbst🐧🦀> but.. who actually reads `R7`?
23:21fdobridge: <karolherbst🐧🦀> can't be that `SHF.L.W.S64 R2, R6, 0x4, RZ ;`, because it uses 32 bit regs, not 64 bit ones
23:21fdobridge: <karolherbst🐧🦀> but besides that?
23:22fdobridge: <gfxstrand> No one
23:22fdobridge: <gfxstrand> But that's a 64-bit shift
23:22fdobridge: <karolherbst🐧🦀> so?
23:22fdobridge: <gfxstrand> Or do 64-bit shifts not actually exist?
23:22fdobridge: <karolherbst🐧🦀> again
23:22fdobridge: <karolherbst🐧🦀> the encoding is: dest, low, shift, high
23:23fdobridge: <gfxstrand> Right, so we need to put both regs in there for a 64-bit shift?
23:23fdobridge: <karolherbst🐧🦀> yes
23:23fdobridge: <karolherbst🐧🦀> I suspect codegen got a 64 bit source
23:23fdobridge: <karolherbst🐧🦀> and never bothered splitting it
23:24fdobridge: <gfxstrand> Yeah, handleShifts doesn't take that into account
23:24fdobridge: <karolherbst🐧🦀> which... with 32 bit addresses doesn't matter one bit 🙃
23:24fdobridge: <karolherbst🐧🦀> or well...
23:24fdobridge: <karolherbst🐧🦀> 64 bit values with their high bits being 0
23:25fdobridge: <gfxstrand> but `c[0x0][0x24] >> 0x1f` should be 0
23:26fdobridge: <karolherbst🐧🦀> the right shift never makes it out of the shader
23:26fdobridge: <karolherbst🐧🦀> it's the left shift following that having a different result
23:26fdobridge: <karolherbst🐧🦀> ehh wait.. shouldn't
23:27fdobridge: <karolherbst🐧🦀> that `IADD3.X R3, R5, R3, RZ, P0, !PT ; ` is wonky
23:27fdobridge: <gfxstrand> But I think maybe I see your point
23:27fdobridge: <karolherbst🐧🦀> that `R3` is `SHF.R.S32.HI R3, RZ, 0x1f, R8 ;` in the good version
23:27fdobridge: <karolherbst🐧🦀> or uhhh..
23:28fdobridge: <karolherbst🐧🦀> SHF.L.W should write R3.. nevermind me
23:29fdobridge: <karolherbst🐧🦀> it's still kinda weird.. all of that
23:29fdobridge: <gfxstrand> Still, your explanation of low and high makes sense
23:29fdobridge: <gfxstrand> I'm just not quite sure how to modify the code
23:29fdobridge: <karolherbst🐧🦀> yeah.. it's definetly one bug
23:29fdobridge: <karolherbst🐧🦀> but there might be another one
23:33fdobridge: <karolherbst🐧🦀> I have to figure out how to use mksplit again..
23:34fdobridge: <karolherbst🐧🦀> totally untested https://gist.github.com/karolherbst/93b137a962334aff0b27d209e4a52c9a
23:36fdobridge: <gfxstrand> I typed something similar and no dice
23:36fdobridge: <gfxstrand> I
23:36fdobridge: <gfxstrand> I've got to go now (edited)
23:36fdobridge: <gfxstrand> We can look more on Monday
23:36fdobridge: <karolherbst🐧🦀> yeah..
23:36fdobridge: <gfxstrand> Or you can play with the test yourself
23:36fdobridge: <karolherbst🐧🦀> I suspect there are more bugs
23:37fdobridge: <karolherbst🐧🦀> did something change at least?
23:40fdobridge: <karolherbst🐧🦀> it _could_ also be, that `.S64` selects the source to be 64 bit for real, but still only writes a 32 bit result
23:40fdobridge: <karolherbst🐧🦀> and then `R3` is whatever random garbage it was before
23:40fdobridge: <karolherbst🐧🦀> maybe it's that instead
23:42fdobridge: <karolherbst🐧🦀> the docs aren't... as great as I wished they were here
23:43fdobridge: <karolherbst🐧🦀> I also think I have a patch for that somewhere....
23:43fdobridge: <karolherbst🐧🦀> yeah.. let's talk on Monday