05:01 clever: mripard: making some decent progress now, pixelclock was gated behind setting a source in CM_DPICTL (oops), and hsynv/vsync gated behind setting a source in the PV control, bits 3:2
05:02 clever: now i need to configure the HVS, and feed it image data...
08:02 MrCooper: tomeu: https://gitlab.freedesktop.org/mesa/mesa/-/jobs/4522727 looks like dEQP succeeded, but the scaffolding somehow failed to recognize that correctly and timed out?
10:30 pcercuei: mripard, hi, you're around?
10:33 pcercuei: mripard, I have two commits in drm-misc-fixes (1a21e5b930e8 and 3b5b005ef7d9) that I need in drm-misc-next, I can't push other patches there until they are merged. What should I do, can I just cherry-pick them there?
10:42 mripard: were they in rc5 ?
10:43 pcercuei: yes
10:45 mripard: airlied: danvet: can you pull rc5 into drm-next so that we can backmerge it into drm-misc-next ?
10:45 mripard: pcercuei needs it to apply some patches
11:35 airlied: mripard: will try and remeber for tmrw if danvet hasnt dine it
11:51 mripard: airlied: great, thanks :)
14:04 danvet: mripard, meeting now here, just came back from work-out
14:04 danvet: will try and do it afterwards
14:33 agrisis: hello, is there anyway to disable vsync using drm/kms?
14:35 emersion: no
14:35 emersion: the program using kms decides whether to do vsync or not
14:36 agrisis: emersion: do you know what call to kms might control that?
14:36 agrisis: emersion: I'm not terribly familiar with kms/drm but I have control over the code
14:36 emersion: the way the program uses kms controls that
14:37 agrisis: is it configurable via drmModeSetCrtc?
14:42 pq: agrisis, it is configurable by flags for drmModePageFlip() and drmModeAtomiCommit() in theory. Not all hardware supports it though.
14:43 pq: agrisis, it's also possible to not use double-buffering, but then you may see more artifacts on screen than simply tearing.
14:44 agrisis: pq: thank you, I'm actually mostly interested in disabling vsync temporarily to help with benchmarking my app
14:44 pq: agrisis, if you're benchmarking, then don't post to KMS at all.
14:44 agrisis: pq: ha, fair enough =)
14:45 agrisis: pq: I'm also doing some perf analysis to understand the kernel driver itself
14:50 MrCooper: agrisis: with a Mesa OpenGL driver, setting the environment variable vblank_mode=0 should disable vsync
14:53 agrisis: MrCooper: I'm only on 2D non-gl, but thanks
15:40 danvet: pcercuei, mripard airlied backmerge pushed
16:19 mripard: danvet: thanks!
16:19 mripard: I just pushed it to drm-misc-next
18:51 jekstrand: jenatali, karolherbst: What's the story with OpCopyMemorySized?
18:52 jenatali: jekstrand: We added a memcpy intrinsic
18:52 jenatali: I think bbrezillon has a MR for it out?
18:52 jekstrand: Oh, ok.
18:52 jekstrand: jenatali: I thought it required CLC or something
18:52 jenatali: Though it was based on the original alignment/packed MR, so it probably needs some reworking to deal with using deref alignments instead
18:52 jekstrand: Sure
18:53 jenatali: jekstrand: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6357
18:56 pcercuei: danvet, thanks
19:00 jekstrand: jenatali: Yeah.... It needs a rebase real bad....
19:00 jenatali: :)
19:02 jekstrand:is going to rebase
19:02 jekstrand: jenatali: Do you know of a good memcpy test off-hand?
19:03 jenatali: jekstrand: I'm not aware of one in the CTS, IIRC LLVM only generates the memcpy intrinsic in the context of struct->struct copies
19:03 jenatali: jekstrand: Here's the one we wrote for it: https://gitlab.freedesktop.org/kusma/mesa/-/blob/msclc-d3d12/src/microsoft/clc/clc_compiler_test.cpp#L38
19:04 airlied: we should put one in piglit
19:04 jekstrand: Yeah, we very badly should
19:04 jenatali: That's a good idea
19:04 jekstrand: I can't believe the CTS doesn't test memcpy. That seems like kind-of a failing......
19:04 karolherbst: I think the CTS emits memcpy, but I also don't know where ..
19:06 airlied:only had sycl code that used memcpy when I looked at it initialy
19:09 jenatali: jekstrand: CL C doesn't actually have a memcpy function
19:09 jenatali: jekstrand: The SPIR-V test suite for CL might have kernels that specifically have that instruction?
19:09 jekstrand: jenatali: Right... So we have this whole intrinsic with all this capability and the only thing that uses it is LLVM internals. :-(
19:10 jenatali: :D
19:10 jekstrand:hates LLVM
19:15 airlied: at least we know we won't get insane memcpys?
19:16 jenatali: airlied: Unless someone's writing SPIR-V directly? :)
19:17 karolherbst: jekstrand: I'd be willing to drop llvm in clover for the nir path, if somebody writes me a CLC parser :p
19:18 karolherbst: jenatali: well.. with 2.1 you are required to consume spirv anyway
19:18 jenatali: Right
19:18 karolherbst: so yeah.. I guess some could do that
19:19 karolherbst: and I guess 99% of the spirvs come from llvm
19:19 karolherbst: and I bet 50% optimize the hell out of it
19:22 jekstrand: jenatali, karolherbst, airlied, bbrezillon: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6713
19:23 jekstrand: I --reset-author on it because rebasing and updating the SPIR-V bits was a massive change. Only about 50% of airlied's original code is still there.
19:23 jekstrand: jenatali: How would you feel about having nir_lower_io handle it?
19:23 airlied: feel free to burn down the other 50% :-P
19:23 jenatali: jekstrand: Handle it how?
19:24 jekstrand: jenatali: Emit a bunch of load/store_deref up to vec4 sized if possible.
19:24 karolherbst: airlied: with sycl we got those byte casts + copy things, no?
19:24 jenatali: jekstrand: Sure, that sound fine
19:24 airlied: karolherbst: yeah if memory serves, there was cast + memcpy
19:24 jekstrand: jenatali: Actually, just emit a bunch of load/store. Not with derefs
19:24 jenatali: We wrote a dedicated pass for it, since lumping it into lower_explicit_io seems like it'd want to go all the way to the appropriate dest type (e.g. load/store_global)
19:24 jekstrand: 'cause it's nir_lower_io
19:25 jenatali: jekstrand: The complexity is that the loads and stores can be different types
19:25 jenatali: That's why we did it in two passes: first lower to load/store_deref, then lower the two deref halves
19:25 karolherbst: jekstrand: I am wondering if we want to try to let load propagation and opt_deref try to deal with them as much as possible.. but yeah, I guess lower_io could turn them into vecs in case something can't be optimized away
19:25 jekstrand: jenatali: Once it's bare offset-based pointers, the type doesn't matter. :)
19:25 jekstrand: karolherbst: We do
19:26 jenatali: jekstrand: Type matters for local
19:26 jekstrand: karolherbst: I think opt_copy_prop_vars will want to handle it when we can
19:26 jekstrand: jenatali: Oh, really?
19:26 karolherbst: yeah
19:26 jenatali: jekstrand: You can memcpy to/from local, yeah
19:26 jenatali: We implement local mem completely different from global/constant
19:26 jekstrand: jenatali: Why does the type matter more there?
19:26 jekstrand: jenatali: Ah
19:26 jenatali: Actually, and private/temp
19:26 jekstrand: jenatali: Well, we can have a generic "lower to some type" pass too
19:27 jekstrand: If that makes life easier
19:27 jekstrand: Where that type is "array of uint8_t" if needed
19:27 jekstrand: But bigger if we have a constant size
19:27 jenatali: jekstrand: Oh, sorry, when I meant type, I meant address space
19:27 jekstrand: jenatali: Oh, ok
19:27 jekstrand: jenatali: Do you still use nir_lower_io for local?
19:27 jenatali: jekstrand: Yeah, but with different address modes
19:27 jekstrand: Oh, I see....
19:28 jekstrand: We only have one address mode
19:28 jekstrand: I get the problem now
19:28 jenatali: :)
19:28 jekstrand: jenatali: Is that pass something that's reasonably generic that I can steal instead of rewriting?
19:28 jekstrand: Not that it sounds all that hard to write
19:28 jenatali: jekstrand: Yes, except it does byte-by-byte copies
19:28 jenatali: So you probably want to rewrite it ;)
19:28 jekstrand: Ok, then. I'll rewrite it
19:30 jenatali: jekstrand: If you did want to reference it at least: https://gitlab.freedesktop.org/kusma/mesa/-/blob/msclc-d3d12/src/microsoft/compiler/dxil_nir.c#L1011
19:33 jekstrand: jenatali: I'll at least starty by copy+paste
19:45 jekstrand: jenatali: Are you going to throw a fit if you see a u32vec2 or u32vec4 that isn't 8/16-byte aligned?
19:45 jenatali: jekstrand: We have a lowering pass we run to split loads/stores that are under-aligned
19:46 jenatali: Our min alignment is 4 for everything except kernel inputs, where we use a datatype that has a 16 byte min alignment
19:46 karolherbst: jekstrand: I suspect with packed structs that's something we might actually run into
19:46 jenatali: Or with LLVM optimizations
19:46 karolherbst: or that...
19:47 jenatali: Yeah, I wrote the pass because LLVM optimizations were doing loop unrolling and giving us 2-byte stores with 1-byte alignment. But yeah packed structs can do it too
19:47 jenatali: So, we need to handle it
19:47 karolherbst:wished spir-v to be more restricting and demands alignment for OpCopyMemorySized
19:47 karolherbst: oh well...
19:48 jekstrand: jenatali: My plan is to base the cast type on the memcpy size and trust you to sort out alignments
19:48 jenatali: jekstrand: What if the memcpy size isn't a compile-time constant?
19:49 jekstrand: jenatali: In that case, we fall back to bytes
19:49 jekstrand: Nothing else we can do
19:49 jekstrand: Unless we really want to go nuts with the runtime decisions
19:49 jenatali: jekstrand: You can potentially have alignments that we can apply to the derefs
19:50 karolherbst: jekstrand: couldn't we just use the alignment when lowering the copy?
19:50 jenatali: But yeah, either way that sounds better than we do today
19:51 jenatali: Eh, I guess you could theoretically have e.g. 4-byte alignment for the pointer, but 1-byte-aligned sizes?
19:51 jenatali: I don't see anything that says you couldn't do that
19:51 jekstrand: karolherbst: I don't know what you mean
19:52 karolherbst: jekstrand: instead of looking at the size, we could just look at whatever alignment we have on the deref, no?
19:52 karolherbst: and use that as the element size
19:56 jekstrand: karolherbst: I want vec4 copies for 4-byte-aligned things. :)
19:56 karolherbst: ehh
19:56 karolherbst: yeah.. should work fine.. it's not like I do anything special with vectors anyway yet :p
20:24 jekstrand: Time to figure out how to write OpenCL piglit tests....
20:25 jekstrand: jenatali, airlied, karolherbst: Just pushed the MR again. Now it has a lowering pass (which I haven't tested!). It emits as many vec4 copies as possible and then smaller sizes as needed.
20:26 jekstrand: If we wanted to make it really fancy, we could do something with alignments and try to emit small stuff to get up to a nice alignment and then big stuff and then small again.
20:26 jekstrand: I'm not that motivated. :)
20:26 karolherbst: yeah...
20:26 karolherbst: I saw code like that once
20:26 jenatali: Yeah that'd work if you have alignment offsets
20:26 karolherbst: maybe we want that to be a gsoc project :p
20:26 jekstrand: The memcpy implementaiton for x86 in glibc is... impressive.
20:28 karolherbst: naive as most are, I bet they think a high perf memcpy is like 5 loc :p
20:30 airlied: rep movsb; :-P
20:30 karolherbst: yeah.. well ;p
20:30 jenatali: jekstrand: I think this is the case, but just checking - if src or dest derefs have alignments, then the casts in the lowering pass will implicitly inherit them, right?
20:31 jekstrand: jenatali: Yup
20:31 jenatali: Awesome
20:31 jekstrand: jenatali: Casts only overwrite alignments if align_mul > 0
20:35 jekstrand: airlied: How does one run CL piglits? :P
20:36 airlied: ./bin/cl-program-tester
20:36 jekstrand: Ah, thanks!
20:36 jekstrand: I found something that does a thing with structs
20:36 jekstrand: I wonder if it memcpys :)
20:36 airlied: ./bin/cl-program-tester tests/cl/program/execute/image-read-2d.cl
20:36 jekstrand: airlied: Images are so 2 weeks ago. :P
20:37 airlied: there are also generated_tests/cl
20:37 airlied: but I'd guess they won't be memcpys
20:38 jekstrand: I'm not sure how to force a memcpy
20:38 jekstrand: I guess I could make vtn emit one when it could have emitted something simpler
20:38 airlied: copying a struct should do it I thihnk
20:38 airlied: ot maybe copying one struct out of an array
20:38 jenatali: jekstrand: Copy/paste the test I sent you earlier
20:39 airlied: indirect copies a struct
20:44 jekstrand: Ugh... Piglit doesn't build:
20:44 jekstrand: framework.exceptions.PiglitFatalError: A test has already been assigned the name: program@execute@builtin@builtin-float-mix-1.0.generated
20:45 jekstrand: Let's try cleaning...
20:45 jekstrand: Nope
20:46 airlied: rm -rf generated_tests/cl mayb
20:46 jekstrand: Yeah
20:46 jekstrand: Looks like
20:46 airlied: though youy only need make bin/cl-program-tester :-P
20:47 jekstrand: sure
20:47 jekstrand: But it's good to actually rebuild piglit every couple months. :P
20:49 jekstrand: <built-in>:1:10: fatal error: 'opencl-c.h' file not found
20:49 jenatali: jekstrand: Pretty sure Clover wants to pull that from your LLVM installation
20:50 airlied: yeah clover should be gbetting that from clang
20:50 jekstrand: Yeah, I'm just very confused why it's suddenly not working
20:50 jenatali: Did you upgrade LLVM?
20:50 jekstrand: no
20:50 airlied: fedora might have messed up llvm vs clang recently
20:51 jekstrand: Hrm...
20:51 jekstrand: It's broken for more than piglit
20:51 jekstrand: I've got one in /usr/lib64/clang/10.0.0/include/opencl-c.h
20:52 airlied: yeah youve' likely got the llvm clan screwup then
20:52 airlied: sudo dnf upgrade --enablerepo=updates-testing clang llvm
20:53 jekstrand: Why do I need updates-testing?
20:53 airlied: because it's still screwed up
20:53 jekstrand: Ok....
20:53 airlied: and tom went on pto, trying to chase it down
20:53 jekstrand:installs
20:54 jekstrand:rebuilds
20:55 jekstrand: Yup, that fixed it. THanks!
20:55 jekstrand: Woo! Got a memcpy!
20:57 airlied: ah it was kiscked a few hours ago hopefully be fixed tomorrow then
21:01 jekstrand: Ok, I've now proven that at least one test passes on the branch. :)
21:01 airlied: jekstrand: and the test uses memcpy? :-P
21:01 jekstrand: airlied: Yes. :P
21:02 jekstrand: And it no longer OOMs compiling because I forgot to increment my offset variable. :)
21:04 jenatali: :)
21:38 jekstrand: jenatali: Have you ever come across OpTypeOpaque?
21:38 jenatali: jekstrand: Uh... don't think so
21:38 jekstrand: It's supposidly a struct type where you don't know the body
21:38 jekstrand: I guess it always has to be cast?
21:39 jenatali: Did you managed to get a kernel with that in it?
21:40 jekstrand: No, I misread
21:40 jekstrand: It's missing frexp
21:40 bnieuwenhuizen: jekstrand: isn't that a forward declaration to avoid recursive ptr types?
21:40 jekstrand: *sigh*
21:40 jekstrand: bnieuwenhuizen: I thought that's what OpTypeForwardPointer was for. :P
21:40 jenatali: jekstrand: frexp is in libclc
21:40 jekstrand: jenatali: Yeah....
21:40 airlied: not sure the translator ever produces it
21:40 bnieuwenhuizen: yup, was confused
21:41 jekstrand: jenatali: I guess we need to get libclc landed...
21:41 airlied: though there is a test in the translator for it
21:41 jekstrand: Either that or I need to implement frexp in vtn
21:41 jenatali: jekstrand: Yeah
21:42 jenatali: karolherbst: Did you still have blocking concerns for libclc that haven't been addressed?
21:42 airlied: looks like it just comes from llvm IR opaque type
21:43 jekstrand: airlied: Probably. Most of the OpenCL SPIR-V stupid comes from "Let's copy+paste from LLVM"
21:43 jenatali: Hm... I wonder if I can get a kernel with that
21:43 jekstrand: Most of the Vulkan SPIR-V supid comes from "Let's copy+paste from GLSL"
21:43 airlied: "Opaque structure types are used to represent named structure types that do not have a body specified. This corresponds (for example) to the C notion of a forward declared structure."
21:46 airlied:does think even if karolherbst has concerns we are better dealing with them in tree :-P
21:47 jenatali: jekstrand: Yeah I can get an OpTypeOpaque from CLC through the translator
21:47 jenatali: struct opaque;
21:47 jenatali: __kernel void main_test(__global struct opaque* inout)
21:47 jenatali: {
21:47 jenatali: *(__global uint*)inout = 5;
21:47 jenatali: }
21:48 jekstrand: We should probably implement it then. :)
21:48 jenatali: Probably :)
21:48 HdkR: Woof, the XDC talks schedule page is very confusing for determining times. You can click "View Schedule (UTC+2)" on the side, but depending on the Indico settings the time inside the talk itself will show UTC+2 or local timezone without a timezone indicator
21:52 Lyude: samuelig: ^
22:01 jenatali: jekstrand: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6715
22:01 jekstrand: jenatali: Looks like I need events now. :)
22:01 jenatali: Yep, that's part of the libclc MR
22:01 jenatali: Though it's a pretty small patch if you just want to pick it before that lands
22:02 jekstrand: I'll probably pick it and see how much furhter I can get in this shader
22:02 jenatali: jekstrand: Unless you actually want the async functions, which I hooked up via libclc :P
22:06 jenatali: jekstrand: Are there helpers for extracting string literals?
22:07 jekstrand: jenatali: You mean, like vtn_string_literal()?
22:07 jenatali: Yeah, like that, thanks
22:07 jekstrand: jenatali: Now with big-endian support. :)
22:07 jenatali: Heh, right
22:08 jekstrand: jenatali: What do you need string literals for? Improving printf?
22:08 jenatali: jekstrand: Your comment about opaque types. The opaque struct name is a string literal
22:09 jekstrand: jenatali: Ah, right
22:12 karolherbst: jekstrand: it's fine now. I still would like to figure out a nice way of dealing with the CL precision, but that's not blocking clc
22:17 jenatali: jekstrand, karolherbst, airlied: Should we go ahead and Marge then?
22:17 karolherbst: yeah, fine by me
22:17 karolherbst: there is a bug with libclc in mesa, but that's not new
22:17 karolherbst: and.. annoying to fix anyway
22:18 jenatali: ?
22:18 karolherbst: soo,.. the way we search for the opencl-c.h file
22:18 jenatali: Ohh
22:18 karolherbst: yeah..
22:18 karolherbst: I try to get it worked out this week then if we merge clc
22:18 karolherbst: now it's like super broken :)
22:19 jenatali: karolherbst: Did you see jekstrand and airlied discussing above? Not sure if that's what you're talking about or something else
22:19 karolherbst: ahh yeah
22:19 karolherbst: jekstrand: we search for the file with the llvm version
22:19 karolherbst: but now we have llvm-10.0.1 but clang 10.0.0
22:20 karolherbst: airlied: it's not a llvm bug
22:20 karolherbst: it is a mesa bug
22:21 karolherbst: we can of course paper over it, but the point is we hard code the path
22:21 karolherbst: and that's just bad
22:22 karolherbst: so if you have mesa compiled against 10.0.0, and your system upgrades to 10.0.1, we have to rebuild mesa :)
22:22 karolherbst: it's annoying
22:23 jenatali: Yeah that's a pretty big bug :P nothing to do with libclc really though :)
22:23 airlied: karolherbst: the only reason we sse it is because fedora packaging screwed up
22:23 karolherbst: no
22:23 airlied: karolherbst: but yes we should avoid hardcoding it
22:23 karolherbst: the thing is, we need to rebuild mesa whenever we upgrade clang
22:23 airlied: oh yes the system packages needs a rebuild
22:23 karolherbst: that's a packaging bug
22:23 karolherbst: _but_
22:23 airlied: the problem in fedora at the moment is llvm and clang are out of sync
22:23 karolherbst: mesa uses the llvm version to find the file
22:23 karolherbst: not clangs version
22:23 airlied: so even when you rebuild it scerws up
22:24 karolherbst: it is still a mesa bug :p
22:24 karolherbst: but maybe those packages shouldn't be out of sync anyway
22:24 karolherbst: but that's a different story
22:25 airlied: I don't think you should ever see llvm and clang diverged versions in the wild
22:25 airlied: unless someone screwed up the packaging
22:26 karolherbst: ahhh
22:26 karolherbst: IncludeDefaultHeader was the thing..
22:26 karolherbst: let's see
22:27 karolherbst: ehhh
22:27 karolherbst: wasn't there an MR to bump the llvm version req?
22:29 karolherbst: ohh, it actually exists in 3.9 anyway
22:29 karolherbst: nice
22:30 jekstrand: Uh oh.... deqp-vk: ../src/compiler/nir/nir_lower_goto_ifs.c:220: route_to: Assertion `!target->successors[0]' failed.
22:31 anholt: tomeu: is there some post-processing before the tracie dashboard updates after a traces job completes?
22:31 jenatali: jekstrand: Is that running VK with unstructured CFG?
22:36 jekstrand: jenatali: No, it's coming in as a CL kernel
22:37 jekstrand: Probably one that LLVM has helpfully optimized for me. :-\
22:37 jenatali: :(
22:37 jenatali: jekstrand: Didn't you turn off LLVM optimizations?
22:38 jekstrand: jenatali: It's SPIR-V that someone else is giving me. :-(
22:38 jekstrand: I think this might be an unreachable block
22:38 jenatali: jekstrand: Oh no. Good luck
22:38 jenatali: Ugh I really want to get our code on upstream so we can start benefiting from fixes like this without having to cherry-pick
22:40 jenatali: Huh... somehow I'm missing r-b's on libclc patches. I thought everything had them at this point...
22:40 jekstrand: jenatali: What's missing them?
22:41 jenatali: A surprising number
22:42 karolherbst: ehhh...
22:42 jenatali: I'll re-skim the comments on the MR, see if I just missed adding them
22:42 karolherbst: now I can't get rid of "c.getHeaderSearchOpts().ResourceDir = CLANG_RESOURCE_DIR;" ...
22:43 karolherbst: isn't there some "default include path" thing as well? ..
22:47 jenatali: I thought there was
22:48 karolherbst: mhh.. "clang -print-resource-dir" tells us, but that only works at compile time
22:50 karolherbst: sometimes I get the feeling that llvm doesn't do the sane things by default on purpose ...
22:51 jekstrand: Of course, the shader which fails to structurize has 52 blocks....
22:51 karolherbst: nice
22:51 karolherbst: lol...
22:52 karolherbst: if (ResourceDir.empty()) ResourceDir = CompilerInvocation::GetResourcesPath(argv[0], MainAddr)
22:52 karolherbst: yeah...
22:52 karolherbst: sure
22:52 karolherbst: why the executable path matter here is something somebody still needs to explain to me
22:52 jenatali: karolherbst: Looks like there should be a CLANG_RESOURCE_DIR that should be set when building clang
22:52 karolherbst: jenatali: but we don't build clang :p
22:53 karolherbst: of course we can tell everybody to rebuild mesa on a clang update
22:53 karolherbst: and if we get asked why we say: not our fault, it's llvm being stupid
22:53 jenatali: karolherbst: You can always do what we did and embed it ;)
22:53 karolherbst: but somehow I don't want to force everybody to rebuild based on stupid reasons
22:53 karolherbst: ....
22:53 karolherbst: sure..
22:53 karolherbst: everybody will love us for this
22:53 jenatali: :P I'm kidding, mostly
22:54 karolherbst: ohh wait..
22:54 karolherbst: actually
22:54 karolherbst: CLANG_RESOURCE_DIR might be a good pointer.. let's see
22:55 jekstrand: We could always invoke llvm-config....
22:55 karolherbst: nope
22:56 karolherbst: "clang -print-resource-dir" :p
22:56 karolherbst: it's not like they have no interface to get it
22:56 karolherbst: they just have a super stupid one
22:56 jenatali: karolherbst: Ah... the CLANG_RESOURCE_DIR is a *relative* path from the current executing binary
22:56 karolherbst: std::string CompilerInvocation::GetResourcesPath(const char *Argv0, void *MainAddr)
22:57 karolherbst: why the hell does it even have args...
22:57 karolherbst: jenatali: yeah.. it's super stupid
22:57 karolherbst: I bet there is some stupid corner case somebody needed to get working
22:57 karolherbst: :p
22:58 jenatali: Copy/paste all of clang to a new location without rebuilding it?
22:58 jenatali: That's not really a corner case...
23:00 karolherbst: yeah... I guess that kind of makes sense if you build some tooling
23:00 FLHerne: > of course we can tell everybody to rebuild mesa on a clang update < we tried that in KDevelop, people (and distros) kept not doing that and filing bugs :-(
23:00 karolherbst: FLHerne: yeah well...
23:01 karolherbst: seems like everybody suffers from using llvm :p
23:01 karolherbst: FLHerne: do you have any solution for this clang resource dir ?
23:01 karolherbst: uhm.. ResourcePath
23:01 karolherbst: but at least you are an executable...
23:01 karolherbst: we are not
23:02 karolherbst: I am wondering if we just hand in the absolute path to clang.. but that also sucks
23:03 karolherbst: I'll ask on IRC and see what I get...
23:04 FLHerne: https://phabricator.kde.org/D17858
23:05 karolherbst: mhhhh...
23:05 FLHerne: The fun thing was when distros built KDevelop against a different LLVM version than Mesa
23:06 karolherbst: right..
23:06 FLHerne: Then people used it with radeonsi, linked both versions, and it crashed horribly
23:06 jekstrand: So this structurizer case that fails: It's two loops. Not nested. WTH?
23:07 jenatali: jekstrand: Can you share the source/spirv? I'm just curious, I'm not really sure I can do anything for the structurizer :)
23:07 karolherbst: FLHerne: so you just replace the version with whatever the runtime has
23:07 karolherbst: that kind of does sound like a sane solution
23:08 jekstrand: jenatali: If I mangle it, probably
23:08 jekstrand: They're both while loops and there is zero code between them.
23:08 jekstrand: I suspect that's why
23:08 jenatali: Ah
23:09 karolherbst: mhh
23:09 karolherbst: jekstrand: maybe the structurizer is fine, but the nir is just a big garbage?
23:09 karolherbst: *bit
23:10 karolherbst: nir tends to require blocks between nodes I think
23:12 jekstrand: karolherbst: The structurizer *should* be able to sort that out. That's it's job.
23:12 karolherbst: right..
23:12 karolherbst: but I guess it fails at validate, right?
23:12 jekstrand: I need to whiteboard the CFG
23:12 jekstrand: No, it validates fine pre-structurizing
23:12 karolherbst: yeah.. I mean after
23:13 jekstrand: It doesn't finish structurizing. The structurizer internally asserts
23:13 karolherbst: ohh.. right..
23:13 karolherbst: mhh
23:13 FLHerne: karolherbst: FWIW, `clangVersion()` is https://invent.kde.org/kdevelop/kdevelop/-/blob/master/plugins/clang/duchain/clanghelpers.cpp#L358
23:13 FLHerne: Which, now I look at it, is pretty horrible...
23:13 FLHerne:wonders if there's a better way
23:13 karolherbst: ...
23:14 karolherbst: mhhh
23:14 karolherbst: that just sucks
23:14 karolherbst: heh.. there is a getClangFullVersion
23:15 karolherbst: maybe there is also a getClangShortVersion
23:15 karolherbst: heh.. yeah
23:15 karolherbst: guess what
23:15 karolherbst: no
23:15 FLHerne: karolherbst: We also have this https://invent.kde.org/volden/kdevelop/commit/bc13f955f3f087f2c6c5b6e8ad299f3ca6bb1142
23:15 FLHerne: Which, I mean, aargh
23:16 FLHerne: (note that *also* uses clangVersion())
23:16 karolherbst: wait...
23:16 karolherbst: they have a "CLANG_VERSION_STRING"...
23:16 karolherbst: maybe there is a simple function returning this thing
23:16 karolherbst: maybe
23:17 FLHerne: That would be nice
23:17 FLHerne: I think our min version at the time was 3.8 or something, it might have been added since
23:18 karolherbst: yeah.. lol
23:18 karolherbst: clang -dumpversion
23:18 karolherbst: ....
23:18 karolherbst: FLHerne: Driver::GetResourcesPath
23:18 karolherbst: this is close to what we need actually
23:18 FLHerne: Are you sure that will work with git versions etc?
23:19 FLHerne: (the dumpversion, I assume `GetResourcesPath` is what it says on the tin)
23:19 karolherbst: Driver::GetResourcesPath does some path magic
23:19 karolherbst: but I am wondering what happens if we just add "" as args
23:21 FLHerne: Oh, that's in the C++ API, which I can't use for reasons :p
23:21 FLHerne: Might work for you though
23:23 karolherbst: mhhhh
23:23 karolherbst: "lib64/clang/10.0.0"
23:23 karolherbst: how does that even make sense
23:24 karolherbst: mhh
23:24 karolherbst: "/usr/bin/clang", "" -> /usr/lib64/clang/10.0.0
23:24 karolherbst: mhhhh
23:24 karolherbst: I guess that's a bit better?
23:25 karolherbst: but what if it returns a relative path?
23:27 FLHerne: karolherbst: I mean, our silly regex hack does *work* despite being ugly
23:28 karolherbst: it kind of feels like that working around llvm is the norm :p
23:28 FLHerne: Or at least, it's been in there for years with people trying all kinds of strange clang versions, and no-one's complained about it
23:28 karolherbst: FLHerne: I guess somebody could have added an API to print the short version
23:29 karolherbst: uhhh
23:29 karolherbst: I have an ugly hack
23:32 jekstrand: jenatali, karolherbst: Here's the CFG https://photos.app.goo.gl/6ZucgpBswhc6v9UdA
23:32 jekstrand: LLVM definitely had some fun with that one
23:32 karolherbst: looks hardly complicated :p
23:33 karolherbst: jekstrand: you might want to draw the 18 -> 6 line inside the graph
23:33 karolherbst: then no lines are crossing each other
23:33 karolherbst: :D
23:33 jekstrand: karolherbst: Sure... Because that's the biggest problem that graph has. :P
23:34 jenatali: :O
23:34 jenatali: jekstrand: Where's the assert popping up?
23:34 jekstrand: deqp-vk: ../src/compiler/nir/nir_lower_goto_ifs.c:220: route_to: Assertion `!target->successors[0]' failed.
23:35 karolherbst: uff...
23:35 karolherbst: what a mes
23:35 karolherbst: s
23:35 jekstrand: target = 13
23:35 jenatali: Indeed
23:35 karolherbst: I guess the clc code makes more sense than that CFG
23:36 jekstrand: Honestly, if I re-drew it, I could make it look alright. It's pretty clear that 1, 2, 4, 5, 7, and 11 make the first loop
23:36 karolherbst: yeah
23:36 jekstrand: Then 6, 8, 14, 15, 16, and 17 make a second
23:36 karolherbst: I mean, it's kind of clear what the control flow is, it's just.. mhh
23:37 jekstrand: There's definitely some stuff in the structurizing code that I've found sketchy
23:37 jekstrand: I think I know roughly what's going wrong
23:37 karolherbst: that 9 block is annoying
23:37 jekstrand: Not really, 9 is the break block for the second loop
23:37 jekstrand: Well, sort-of
23:37 jekstrand: Yeah, 9 is annoying
23:37 karolherbst: :p
23:38 jekstrand: It doesn't look much like the source
23:38 karolherbst: I guess llvm inserted that 17 -> 9 path
23:38 karolherbst: for... whatever reason
23:38 karolherbst: huh.. wait
23:38 jekstrand: It looks like LLVM CSEd a whole block somewhere
23:39 karolherbst: how different are 19 and 9\?
23:39 karolherbst: I could imagine one is empty
23:39 jekstrand: The source is just "while (x < y) { ptr = calculation(x); if (*ptr == 0) break; do_a_thing(); } while (z < w) { ptr = calculation(z); if (*ptr == 0) break; do_a_different_thing(); }"
23:40 karolherbst: heh...
23:40 karolherbst: that makes 19 empty indeed
23:41 jekstrand: Yeah, 19 is empty and 9 has no side-effects
23:41 jekstrand: 9 just makes SSA copies of some things
23:41 jekstrand: I don't get why 17 goes to 9
23:41 karolherbst: yeah...
23:41 jekstrand: But, anyway, it's late and I need to make supper
23:41 karolherbst: llvm is obviously doing something silly
23:42 karolherbst: maybe we should open bug reports about that stuff
23:42 karolherbst: :D
23:42 karolherbst: it's clearly more complicated than it needs to be
23:47 jekstrand: karolherbst: We should also fix our structurizer. We claim to be able to consume effectively arbitrary control-flow. Clearly, we can't.
23:47 karolherbst: looks like it
23:47 karolherbst: but I kind of expected there to be a few places it messes up
23:48 jekstrand: Yeah, there's some problems with the approach
23:48 jekstrand: I'm not 100% how to solve them. I'll have to look tomorrow. Hopefully, I can page it back in without too much effort.
23:49 jekstrand: I already fixed one bug where blocks were "leaking" through its frontiers
23:53 jekstrand: I expect I just need to expand more "leave the thing" sets