03:57 imirkin: skeggsb: do you know offhand what [RT_WIDTH_OVERRUN] (and SURF_WIDTH_OVERRUN on nv50) are about?
11:52 pmoreau: karolherbst: Thank you for the speedy review and getting marge on it, for that clover/nir patch!
11:52 karolherbst: :)
11:52 karolherbst: well, it was a trivial fix :p
11:55 pmoreau: It was :-D
11:56 pmoreau: I was being a bit bored and wanted to do that for some time, so I started this: https://gitlab.freedesktop.org/pmoreau/mesa/-/commits/track_opencl_features
11:57 karolherbst: cool
11:57 karolherbst: pmoreau: but I think I finished the atomics stuff
11:57 pmoreau: I have put everything as “not started” for now
11:57 karolherbst: pmoreau: on my branch those exts are exposed, no?
11:57 karolherbst: ahh
11:57 karolherbst: I see
11:58 pmoreau: I mean, even cl_khr_icd is marked as “not started”, even if that support has been in for like ages in clover.
11:58 karolherbst: :) true
11:58 karolherbst: we also support cl_khr_fp64
11:59 pmoreau: Just starting by listing everything in that file, and then going through it all and marking things as done.
11:59 karolherbst: pmoreau: maybe we should differentiate between TGSI and NIR
11:59 karolherbst: or maybe not.. dunno
11:59 karolherbst: or do it driver based
11:59 karolherbst: but it's all so broken, so I don't even care all that much anyway
11:59 pmoreau: Driver-based might be the easiest.
11:59 karolherbst: in doubt r600 is as broken as nouveau
12:00 pmoreau: But, it’s a mess, agreed.
12:00 karolherbst: we don't even pass all CL 1.1 tests.. so
12:00 pmoreau: Welp
12:03 karolherbst: well.. hopefully this changes now :)
12:05 pmoreau: :-)
12:09 pmoreau: Since I’m back doing Mesa stuff, I should ask for dev access or at least be able to add labels and such to issues and MRs.
12:12 pmoreau: karolherbst: In the mean time, would you mind adding a “clover” (and maybe “OpenCL” too) tag on this new MR? https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4943 Thanks!
12:13 karolherbst: the use of && is wrong in clover anyway.. ehh
12:15 karolherbst: and what's even the point of advances_by_t :O
12:22 pmoreau: It advances the iterator by the specified amount of steps, so basically `it + n`. Except I guess it will also work with non-random iterators?
12:25 karolherbst: pmoreau: I meant we don't need the class to implement it probably
12:25 karolherbst: it just uses std::advance
12:30 pmoreau: Oh right, yes it could probably be removed.
12:30 karolherbst: that's the feeling I get for most things, and then the reason I see is "ohh, this allows us to use a functor instead of a for loop"
12:34 pmoreau: I’ll look into removing some of those during the week.
12:34 karolherbst: cool
12:34 karolherbst: I always wanted to clean things up and remove pile of useless stuff
12:35 pmoreau: Some cleaning would definitely be nice!
13:25 karolherbst: imirkin, fincs: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4945 if you have some time :)
13:34 imirkin: nice
13:38 karolherbst: ohh "relative luminance" is BT.709 btw..
13:39 karolherbst: and I think I saw the BT.2020 values as well.. mhh
13:39 karolherbst: maybe I should take another look :)
13:40 karolherbst: and not call it liminance but RGB to YcbCr conversion constants :)
13:40 karolherbst: and check all channels
13:40 imirkin: i like the "game engine constants" one :)
13:41 imirkin: "welp, here are some numbers that make no sense, but lots of game use them."
13:41 karolherbst: :)
13:41 karolherbst: yeah.. there are some shaders dumbs in the internet
13:41 karolherbst: and those were hit by tons of unity based games
13:41 karolherbst: :)
13:41 karolherbst: and literally nothing else
13:41 karolherbst: soo yeah :D
13:41 imirkin: 1e-9 didn't make the cut?
13:42 karolherbst: nope
13:42 karolherbst: was hardly used
13:42 karolherbst: we can add it though
13:42 karolherbst: we have enough space left :)
13:43 karolherbst: and I will probably add other constants just to have the sections complete even though they don't matter as much
13:44 karolherbst: but mainly wanted to have an opinion before completing the list
13:44 imirkin: ufff, you do a linear scan of the imms each time?
13:46 karolherbst: what do you mean by linear scan?
13:46 imirkin: imms.find()
13:46 karolherbst: it's a hashed table
13:46 imirkin: oh i see. imms is a map
13:46 imirkin: so then why do you do two lookups?
13:46 imirkin: find returns all you need
13:47 karolherbst: ohh, right it returns an iterator so I can just do *it.. right
13:47 imirkin: it->first / ->second
13:47 karolherbst: ohh, yeah.. :)
13:47 karolherbst: (*it)->second ;)
13:48 pmoreau: Why lookup only once, when you can do it twice! :-p
15:38 pmoreau: Yay, computeinfo now passes for both 1.0 and 1.1 on Tesla! Had to fix the CTS cause it was testing for image formats/flags that were introduced in 1.2 and 2.0. :-D
15:43 karolherbst: :)
15:44 karolherbst: nice
15:44 karolherbst: pmoreau: do you want my awesome cts runner?
15:44 pmoreau: Sure!
15:44 karolherbst: pmoreau: https://gist.github.com/karolherbst/27448325fe88b66b38e990f90bc3c355
15:44 karolherbst: you need to adjust some things, but you get the idea on what it does :p
15:47 pmoreau: Neat, thanks!
15:48 pmoreau: Btw, where do you have the test binaries on your computer after you build the CTS? (And are you building in-source or not?)
15:49 karolherbst: I build inside "build"
15:49 karolherbst: but you just need to adjust CTS_PATH a bit ;)
15:49 pmoreau: Yeah sure, but I mean even the regular CTS runner does not find the binaries for me.
15:50 pmoreau: Cause it’s looking for them under the various test_* directories, but they are all stored in bin/
15:50 karolherbst: I never used that one
15:51 karolherbst: or did they change things?
15:51 karolherbst: mhh
15:51 karolherbst: there are usually subdirectories for each test under test_conformance
16:14 pmoreau: Yes, and I do have those subdirectories, both in source and in build, but the binaries themselves are located (for me at least) under bin in the build folder, and not under one of those subdirectories.
16:16 karolherbst: heh...
16:16 karolherbst: don't have it
16:16 karolherbst: maybe something chagned indeed
16:16 karolherbst: heh.. the master branch doesn't build anyway
16:20 karolherbst: probably my CL headers are too new
16:28 pmoreau: master built fine for me once I updated my CL headers to the latest from OpenCL-Headers' master.
16:47 pmoreau: karolherbst: You might have been annoyed by messages like “Extension 'SPV_KHR_no_integer_wrap_decoration' is not supported.” when running the CTS, right?
16:49 karolherbst: pmoreau: nope
16:49 pmoreau: Ah, good for you then!
16:49 karolherbst: ../test_conformance/computeinfo/extended_versioning.cpp:25:35: error: ‘cl_name_version_khr’ was not declared in this scope
16:49 pmoreau: I realised with now have https://github.com/KhronosGroup/SPIRV-LLVM-Translator/blob/3f0fd561b50424624974c18fa6f95b7b1eb4153d/docs/SPIRVVersionsAndExtensionsHandling.rst
16:49 karolherbst: yay
16:50 pmoreau: And given the API we were using, we were ending up with all SPIR-V extensions allowed, and the max supported SPIR-V version. :-D
16:50 pmoreau: Currently switching to the new API, so that we can specify which SPIR-V version and extensions we want to allow.
16:51 karolherbst: cool
16:51 pmoreau: Weird, I did not have that error. Do you have USE_CL_EXPERIMENTAL set?
16:51 karolherbst: it's OFF
16:51 karolherbst: probably I need to update distributions header or so
16:53 pmoreau: Yeah, maybe. I just cloned OpenCL-Headers and pointed CMake to it (via the CL_INCLUDE_DIR var).
20:09 pmoreau: karolherbst: Arf, it’s not just a matter of replacing `advances_by()` by `std::advance()`, cause the former is used as a lambda function (for example `tuple::map(advances_by(n), its)`).
20:11 pmoreau: Most of the utilities would have to be re-written.
20:17 karolherbst: pmoreau: yeah.. that's the conclusion I usually end up with as well
20:18 karolherbst: pmoreau: I'd focus on replacing util/pointer.hpp first
20:18 pmoreau: So I am not planning on addressing it for that MR, but something to be done separately.
20:19 karolherbst: yeah
20:19 karolherbst: sounds fair
20:19 karolherbst: we could probably get rid of intrusive_ptr and intrusive_ref
20:19 karolherbst: and pseudo_ptr
20:19 pmoreau: Given the names, sounds like a good idea
20:20 karolherbst: pseudo_ptr is probably the easiest one
20:22 karolherbst: mhhh.. that one might be super easy to replace actually...
20:22 karolherbst: let me try somthing
20:34 karolherbst: pmoreau: https://github.com/karolherbst/mesa/commit/63e006fd2655e9f249d722bf9642978f0189f80c
20:34 karolherbst: :)
20:35 karolherbst: that operator bool() thing is just what annoys me a little
20:38 pmoreau: Mmh
20:44 karolherbst: pmoreau: anyway. I expect some other things can be replaced as easily :p
20:45 karolherbst: I'd consider the operator bool to be wrong anyway
20:45 karolherbst: it should probably have been "return x != nullptr;"
20:46 karolherbst: anyway... that part is the only reason this wrapper _makes_ sense to exist
20:46 karolherbst: who doesn't want a pointer which always returns true though
20:58 pmoreau: But `x` is the actual type, not a pointer AFAICT
20:59 karolherbst: ehhhhh
20:59 karolherbst: what
20:59 karolherbst: so it's just a reference
20:59 karolherbst: ehhh
20:59 karolherbst: the hell
20:59 pmoreau: The class description for `pseudo_ptr` does say: “Class that implements the usual pointer interface but in fact contains the object it seems to be pointing to.”
20:59 karolherbst: yeah.. okay
21:00 karolherbst: then it's pointless indeed
21:00 pmoreau: Yeah, it is just a reference.
21:02 pmoreau: I’m guessing it is being used in functions that could take either a pointer or an object, and expect to be able to call operator*() or operator->() on them, regardless of what they are.
21:28 karolherbst: pmoreau: well.. I've fixed the only user of this class :p
22:30 karolherbst: imirkin, pmoreau, skeggsb: anybody of you up for reviewing this https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4580 ?
22:32 karolherbst: ahh crap.. I also need this 64 bit handle bug uff
22:33 imirkin: } while ((start + reserved_size) < 1ull << 40);
22:33 imirkin: why 1 << 40?
22:33 imirkin: address space is 48-bit on pascal+ no?
22:34 karolherbst: yeah... dunno, it was copied over from code glisse wrote back in the days
22:35 karolherbst: I'll ask him
22:37 karolherbst: ehhh
22:37 karolherbst: I messed up the loop anyway
22:37 karolherbst: or maybe that's fine
22:37 karolherbst: I guess if the svm ioctl fails it's no use anyway
22:37 karolherbst: but I should unmap reserved_addr probably
22:51 karolherbst: soo. fixed it