09:51karolherbst: pmoreau: ohh and what was the issue with using the most recent spirvtools version?
09:51karolherbst: uhh, I just didn't update my local branch, meh
09:54karolherbst: pmoreau: how long would it take you to provide a branch with just the clover changes? I encountered some meson related issues and I would like to clean up all the patches if you don't find the time for that
13:38karolherbst: pmoreau: soo, the current spirv to nir stuff totally doesn't like your spv files from that repository with the IL examples
14:33karolherbst: pmoreau: after some dirty hacking: https://gist.githubusercontent.com/karolherbst/a8d5d23b2f652c43ac297a5987a254d9/raw/e7df32d1014a814b9ee0cbd89c31ef8204c7806c/gistfile1.txt
14:35karolherbst: pmoreau: here are also some meson changes you might wanna take over: https://github.com/karolherbst/mesa/commit/2408dab28debc70926fae1878fdf5b62b5ceb662
14:35karolherbst: uhm, carry over
14:35karolherbst: at least some of them
14:57robclark: karolherbst, btw if there is a repo somewhere w/ spv examples, if there are some example(s) which use pointers, could you point me at 'em (no pun intended)..
14:57karolherbst: robclark: shouldn't most of them use pointers like for kernel out parameters?
14:57robclark: I guess I could pretty easily add spirv_to_nir to my ir3 standalone compiler to then have something to work with to add new nir intrinsics plus spriv_to_nir parts
14:58robclark: do they? I haven't really looked yet, but if so then I just need to know where this repo is ;-)
14:58karolherbst: robclark: https://gist.githubusercontent.com/karolherbst/54a5622db6520014a865041b6e85557e/raw/facc7dbbecea15b4986293c19f999ef2cb021ac1/gistfile1.txt
14:58karolherbst: this is from pmoreau examples
14:59robclark: hmm, well in current form I guess that store_var isn't going to go anywhere useful, but I guess you are thinking that should be a store_ptr intrinsic instead
14:59robclark: where is the repo w/ the binary spirv files this came from?
15:00karolherbst: uhm, there are no binary spirv files
15:00karolherbst: they get compiled with khronos llvm-spirv fork
15:00karolherbst: :/ https://phabricator.pmoreau.org/w/mesa/testing_opencl_through_spirv/
15:00robclark: hmm, ok..
15:01karolherbst: ohh wait, that just the howto compile his stuff
15:01robclark: well I guess I can build llvm-spirv fork and then just save the binaries somehow..
15:02karolherbst: I really don't like Phabricator.... somehow I never find the stuff I am looking for
15:03karolherbst: here is the spirv to my example above: https://gist.githubusercontent.com/karolherbst/54a5622db6520014a865041b6e85557e/raw/216dc3eba012857b257aba7fc13eeaae56fbb9a4/gistfile2.txt
15:03karolherbst: I modified the smad24 example
15:03karolherbst: to just do *+
15:04karolherbst: because smad24 is an OpenCL extension op, which spirv_to_nir can't handle yet
15:04robclark: I suppose something like https://phabricator.pmoreau.org/diffusion/SPVTES/browse/master/memory_accesses/g_arg_w_struct_mem_aligned.cl would do the trick..
15:04karolherbst: also this OpEntryPoint Kernel %6 "test" causes troubles
15:04karolherbst: robclark: check the smad24 example
15:04karolherbst: this is super trivial
15:06karolherbst: I am now currently wondering how much work it would be to get everything upstreamed :(
15:06robclark: I think didn't tomeu just make some hack to spirv_to_nir to treat any entry point as "main"?
15:06karolherbst: the opposite
15:06karolherbst: it made spirv2nir eat anything which starts with foo
15:07robclark: (or not specifically look for "main")
15:07karolherbst: well, we just need to parse OpEntryPoint Kernel %6 "test" properly
15:07karolherbst: for example
15:07karolherbst: "test" is just for having a name and super uninteresing
15:07karolherbst: %6 = OpFunction %void None %5
15:07karolherbst: and that's where we have to start the kernel
15:08karolherbst: %5 is just the function type
15:08karolherbst: spirv_to_nir handles most of that already, but it kind of looks for "main" as well
15:09robclark: well, I guess anyways we can keep that hack for now while implementing the pointer support.. either way, need to get setup somehow so that I can actually feed in spirv shaders and see what happens
15:09karolherbst: but that doesn't matter much, because you still call spirv_to_nir and give the name to the "main" function
15:10karolherbst: robclark: use my nouveau_nir_spirv_opencl branch
15:10robclark: well, my laptop doesn't have nv gpu :-P
15:11karolherbst: it can be used with clCreateProgramFromIL
15:11karolherbst: doesn't matter
15:11robclark: but in freedreno/ir3 I have a standalone compiler.. and I guess it wouldn't be too hard to add support for using spirv_to_nir
15:11karolherbst: you just get the spir-v piped to your driver
15:11karolherbst: and then you call spirv_to_nir
15:12karolherbst: this is what I have now for spirv handling in codegen: https://github.com/karolherbst/mesa/blob/2408dab28debc70926fae1878fdf5b62b5ceb662/src/gallium/drivers/nouveau/codegen/nvir_from_spirv.cpp
15:12robclark: sure, but then I have to build clover and things like that.. if at this point I'm just caring about the nir bit and not running on real hw, then I can use standalone compiler on my laptop without plugging in hw ;-)
15:12karolherbst: mhh, true
15:12robclark: btw, why not do the spirv_to_nir call in clover?
15:13robclark: that is kinda what I had in mind for freedreno clover support..
15:13karolherbst: but clovers IR handling is kind of hacky
15:13karolherbst: I dislike a lot of clovers design decisions in this regard
15:13karolherbst: clover in its current form compiles into native emitable binaries
15:14karolherbst: and we would have to clean that up
15:14karolherbst: so that clover can detect what the driver supports and what it gets
15:14karolherbst: like if the input is cl and the driver only supports TGSI/NIR, it should compile from cl to TGSI/NIR through any available path
15:14karolherbst: currently it all lacks that stuff
15:15robclark: hmm, ok.. I kinda assumed you have to deal wth that somehow to add a spirv path.. anyways, I guess those are details I can ignore for now w/ standalone compiler ;-)
15:16karolherbst: well pmoreau took care of this, I never looked into his clover patches, so I went with what gave me the fastest result :)
15:18robclark: fair enough
15:18robclark: maybe tomeu already started on that part already, anyways..
15:18karolherbst: I hope so
15:19karolherbst: but I kind of doubt that
15:19karolherbst: I mean pmoreau wrote most of the stuff for this already
15:19karolherbst: we just need to clean it up
15:19robclark: anyways, I'll wire up spirv_to_nir for standalone compiler and start looking at pointers tomorrow.. (weekend is for other freedreno hacking :-P)
15:20robclark:did manage with some compiler work to make gfxbench alu2 go from ~15fps to 60fps :-P
15:20robclark: although still a bit hacky and needs some cleanup.. but sadly it didn't have a similar boost for manhattan :-(
15:21robclark:needs a better way to track down *which* shaders take the most time.. the apitrace profiling stuff doesn't really work too well on tilers
15:21robclark: alu2 is easy, since there are like 2 shaders :-P
15:21karolherbst: I am now mainly wondering how long the cleaning up and upstreaming work would take :/
15:22karolherbst: robclark: frameretracer ;)
15:22karolherbst: someone just needs to...
15:22robclark: yeah, I need to try again to get that working.. last time I tried (after xdc) I wasn't so successful
15:22karolherbst: yeah, smae here
15:22robclark: as far as upstreaming, I guess things like nir->codegen and new intrinsics could be separated from the hacks and get upstream sooner
15:23karolherbst: I am just talking about getting something like "*out = in1 * in2 + in3;" working
15:23karolherbst: nothing more
15:23karolherbst: so add support for spir-v in clover
15:23karolherbst: and pass that through nir
15:24karolherbst: robclark: I plan to get that nir->codegen stuff merged next week or so
15:24karolherbst: robclark: yeah.. I already have a higher passrate than radeonsi :p
15:25karolherbst: so I kind of see that nir stuff as done already
15:25karolherbst: sure, there will be new things like handling function parameters and so on
15:25karolherbst: but this is more like for the OpenCL stuff
15:25karolherbst: so I am really just wondering how long it might take to get the clover stuff done
15:28robclark: I guess after ptr intrinsics and spriv_to_nir support for it, I'll have a closer look at tomeu's latest branch and get that going on a5xx, and then go from there..
15:28robclark: it is easier for me to see and think about what needs to be done when I can get my hands dirty
15:28karolherbst: yeah, same here
15:28karolherbst: but I was kind of asked to estimate/figure out what needs to be done
15:34robclark: karolherbst, take a guess, multiply by 4?
15:34karolherbst: would be a month then
15:35robclark: take a longer guess :-P
15:35karolherbst: I mean, depends on what should be done
15:35karolherbst: if we ignore clCreateProgramWithSource
15:35karolherbst: then it is fairly somple
15:35karolherbst: the biggest issue is to consume OpenCL kernels
15:36karolherbst: but if we just go with clCreateProgramWithIL and get the spir-v directly, there isn't really much to be done
15:36karolherbst: well sure, only the most basic things would work, but this was kind of the goal here
15:37robclark: I guess it depends on what you are trying to make work.. but I guess as long as we aren't trying to run CTS then *WithIL() seems ok..
15:38karolherbst: first goal is: "something" works
15:42pmoreau: bazzy: I did check (and pinged you back): I do have the “pop” and I do have it even with the NVIDIA driver, so I’m not sure this is a Nouveau issue.
15:43pmoreau: karolherbst: Going through the backlog, but I was going to clean up and send the clover series today.
15:44karolherbst: :) awesome
15:47pmoreau: karolherbst: clover is already detecting what the driver supports (for example TGSI or NATIVE) and take that into account.
15:47karolherbst: pmoreau: with your patches, right?
15:48pmoreau: Even without
15:48karolherbst: no, it didn't
15:48pmoreau: Yes it does
15:48karolherbst: it toally failed for me
15:48karolherbst: it tried to target TGSI
15:48karolherbst: until I changed the preferred ir
15:48pmoreau: one sec
15:48karolherbst: you also changed the preferred IR to spirv ;)
15:48pmoreau: Right, if you advertise TGSI, ir will use that
15:49karolherbst: that's why it is broken
15:49karolherbst: it should pick a useable IR from all supported IRs
15:49pmoreau: No, because Nouveau advertise TGSI as it’s preferred IR
15:49karolherbst: otherwise we get into big big troubles
15:49karolherbst: we can't do that
15:50karolherbst: TGSI stays to be the preferred one
15:50pmoreau: If you have a cap called PREFERRED_IR, why would you not use it?
15:50karolherbst: you should
15:50karolherbst: but clover shouldn't be silly about it
15:50karolherbst: if the rpegerred one is something clover can't do, why should it fail and not try a different one?
15:50pmoreau: clover is not the one being silly, it’s us
15:50karolherbst: preferred should only be taken into account if there are two supported paths
15:51robclark: btw, maybe nouveau could prefer NIR based on some context flag passed when clover creates the pipe_context? A bit of a hack but not that bad of one..
15:51karolherbst: you can change it based on the program type
15:51karolherbst: but OpenGL compute and OpenCL both use the same type
15:51karolherbst: but still
15:51robclark: (ideally we wouldn't have PREFERRED_IR but instead a way to rank IR's in order of preference)
15:51karolherbst: preferred means preferred
15:51karolherbst: not the only one
15:51karolherbst: if supported IR contains SPIR_V and preferred is TGSI
15:51robclark: right, but nouveau prefers TGSI to NIR but perfers NIR to something it doesn't support
15:52karolherbst: why should clover fail and say TGSI is silly?
15:52pmoreau: You can get all supported IRs through one of the cap
15:52robclark: clover shouldn't pretend to support TGSI ;-)
15:52karolherbst: pmoreau: right, but clover doesn't take that into account
15:52pmoreau: It does to some extent (with my patches0
15:52karolherbst: I saw those
15:52karolherbst: but this was the thing I talked about above
15:52pmoreau: The TGSI backend is kinda broken, and got partly fixed, but
15:53karolherbst: clover is a little silly if it comes to choosing the IR to work with
15:53karolherbst: I thought there is like 0 support for TGSI in clover
15:53pmoreau: There is some code to handle TGSI
15:54karolherbst: is that even used?
15:54pmoreau: Well, if the driver advertise TGSI, it should be used ;-)
15:55karolherbst: let me get the error again
15:56pmoreau: For example for program compiling: https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/state_trackers/clover/core/program.cpp#n55
15:56karolherbst: mhh, right
15:58pmoreau: karolherbst: The problem with not supporting the clCreateProgramWithSource upstream, is that I doubt many applications use clCreateProgramWithILKhr or clCreateProgramWithiL.
15:58karolherbst: pmoreau: I am sure none do
15:58karolherbst: because which driver exposes those functions anyway?
15:58karolherbst: on my system it was none
15:58karolherbst: not even icl-ocd
15:58pmoreau: Intel one maybe, I think they have OpenCL 2.1 support
15:59pmoreau: Technically any OpenCL 1.2 driver could advertise the cl_khr_il_program extension, and therefore the clCreateProgramWithILKHR, but I don’t think many do.
15:59karolherbst: I have a pakcage called opencl-headers version 2.1
15:59karolherbst: not even there
15:59karolherbst: the point is, we need headers exposing those functions
16:00pmoreau: clCreateProgramWithIL should be there, cause it is core in 2.1
16:00karolherbst: and if even the official ocl-icd doesn't do that :(
16:00karolherbst: yeah, right
16:00karolherbst: I think it was added to a later version of 2.1 headers
16:00pmoreau: As for clCreateProgramWithILKHR, yeah, that extension was never added to the official headers. I sent a pull request for that
16:01pmoreau: But it hasn’t been merge yet: https://github.com/KhronosGroup/OpenCL-Headers/pull/24
16:01karolherbst: ohh right, clCreateProgramWithIL is in cl.h for 2.2
16:01karolherbst: and 2.1
16:01karolherbst: it is there for me here as well
16:01karolherbst: why didn't the compiler pick it up
16:01pmoreau: SPIR-V was introduced with 2.1, so it has to be there :-D
16:02pmoreau: (And clcreateProgramWithSource was deprecated at the same time IIRC)
16:04karolherbst: :D that's harsh, isn't it?
16:05pmoreau: They deprecated it, but it’s still around, even in 2.2 IIRC
16:05karolherbst: pmoreau: anyway, if TGSI is the preferred IR, clCreateProgramWithIL doesn't work out anymore
16:06pmoreau: Hum, let me check that
16:06karolherbst: `queue.enqueueNDRangeKernel(kernel, cl::NullRange, cl::NDRange(1), cl::NDRange(1), nullptr, nullptr);` failed: CL_INVALID_PROGRAM_EXECUTABLE
16:08pmoreau: I see why, the question is how to solve it
16:08karolherbst: maybe it makes sense to be able to return different values for CAPs depending on the stuff used
16:09karolherbst: like to return different stuff for OpenCL than for OpenGL
16:09karolherbst: or two split GL_COMPUTE and OpenCL
16:09pmoreau: That won’t solve the issue here
16:09karolherbst: but in the end it shouldn't matter
16:09karolherbst: yeah, I was aware, just a more general thought
16:10pmoreau: The linker picks what to do based on the preferred IR. But, now with clCreateProgramWithIL(KHR)?, you can get one program with a certain IR (let’s say TGSI) by doing clCreateProgramWithSource, and try to link with a program created with clCreateProgramWithIL(KHR)?
16:11pmoreau: If we can convert one into the other, then that’s fine. But otherwise, we are asking for troubles.
16:12karolherbst: well we could go with the easiest path
16:12karolherbst: if we can convert one side to the other one, we pick the thing which can be done
16:12karolherbst: but source is source anyway
16:13karolherbst: pmoreau: can an application use clCreateProgramWithIL with two different IRs?
16:13pmoreau: But source got compiled into something
16:14karolherbst: pmoreau: do final compilation at linking time then?
16:14pmoreau: It could: SPIR-V is the only mandatory one, anything else is implementation defined.
16:14karolherbst: mhh I see
16:14pmoreau: But regarding clover, that won’t happen, unless we advertise another IL.
16:14karolherbst: yeah, we have to comple at linking time
16:15karolherbst: we can do a source to llvm thing for withSource regardless
16:15karolherbst: but what to do with the llvm is up to whatever happens until it gets linked
16:15karolherbst: if it gets linked with a spir-v thing, we can either compile spir-v to llvm or compile llvm to spir-v
16:16karolherbst: whatever the driver is able to do
16:16pmoreau: Or we could force the compilation to SPIR-V, link in SPIR-V, and then convert that to whatever afterwards.
16:16karolherbst: we can link in llvm, right?
16:17karolherbst: we do the final compilation at linking time anyway, right?
16:18pmoreau: What do you call by “final compilation”, the generation of the GPU binary?
16:19pmoreau: I think that happens on the first execution of the kernel.
16:20karolherbst: ohh okay
16:20karolherbst: I am just a bit worried about touching the current path we have if we switch over to linking in spir-v
16:21pmoreau: I’ll bring that up in the cover letter of the series, see what curro and tstellar think about it.
16:22karolherbst: okay :)
16:43mupuf: Oh, we got an email from a happy user for Christmas... Only saw it now... http://private.mupuf.org/nouveau_led.mp4
16:44karolherbst: mupuf: you only get a christmas thank you video like that when working on LED stuff, anything else won't do the trick :p
16:45mupuf: yeah, don;t work on hard features, right?
16:45mupuf: instant gratification
16:45mupuf: well, at least, we have one feature not supported by nvidia, that has to count for something
16:46mupuf: make it fun to attract developers... or die trying
16:46mupuf: anyway, it is finally code review time
16:46karolherbst: uhh, what's on the list?
16:46mupuf: first, the easy stuff: Lyude's patchset
16:46mupuf: then yours
16:46mupuf: about time...
16:47mupuf: and then I guess we'll want to work on the next presentation
17:03pmoreau: Awesome video! :-)
18:05rhyskidd: let's get nouveau's advanced LED support onto mesa_matrix :)
18:07karolherbst: we should write an GL extension for it
18:08rhyskidd: so just putting this here: https://paste.fedoraproject.org/paste/5IXTERVQrUSMWC9LLZMPAQ
18:08karolherbst: vec4 gl_LED
18:08karolherbst: rhyskidd: why so many branches though?
18:08rhyskidd: might be duplicative of existing private repo's. just wondering whether the work to dump shaders into shader-db format might be handy for others?
18:09karolherbst: don't you have access to our shader-db?
18:09rhyskidd: do those who have Feral/Valve steam game access want something like this?
18:09rhyskidd: not yet ...
18:09karolherbst: I thought you have access to the vbios one?
18:09rhyskidd: i've got the vbios one
18:16mupuf: rhyskidd: I can add you for our shader-db repo
18:17karolherbst: mupuf: :) I already sent him the info, just access rights are missing
18:18mupuf: done for both the apitraces and shader-db
18:18mupuf: and while at it, I also gave access to Lyude
21:38pmoreau: karolherbst: Got the meson build to work: I just needed to create an “include directory” object rather than pass the path directly.
22:14airlied: oops lag
22:16pmoreau: karolherbst: I added a branch with only the clover patches, and without the compilation from OpenCL to SPIR-V: clover_spirv_only, https://phabricator.pmoreau.org/diffusion/MESA/history/clover_spirv_only/