01:30karolherbst: imirkin: okay, I understood what GlobalCSE is doing. I think instead of moving the phi around, we could simply add a mov bld.getSSA phi.def to the current bb instead of moving that phi instruction
01:31karolherbst: ohh, we don't need a new SSA value, but simply reuse the def of the phi globalCSE wants to whipe out
01:48karolherbst: fun, that original phi was "phi u32 %r354 %r207 %r207"
01:52karolherbst: inserting this mov causes worse code to be generated
01:57karolherbst: here another attempt to fix that, but it generates worse code. Funny enough for this shader it doesn't matter if GLobalCSE is turned on or not, the produced result after RA is the same, so maybe we could just drop that moving phis around in globalCSE completly? I shall ask shader-db what it thinks about this. https://github.com/karolherbst/mesa/commit/2abf0999dbe1e876c4e58c1541144cda3f5be1ac
02:17technohacker: Hello again, sorry for disturbing, I wanted to know whether OpenCL is supported on NVC0. Wanted to use my GPU in Blender3D
02:20imirkin: not with nouveau.
02:21technohacker: imirkin: Oh ok, thanks
09:20karolherbst: ugh, "precise" is a mess. It looks like it has to be fully analyzed in glsl, because if you assign a result of a function to a precise value, the internal operations of that function don't need to care about precise at all :/
09:20karolherbst: should check how that's handled in nir
09:23karolherbst: yeah, in nir the instruction are tagged as precise, which makes perfectly sense
10:57karolherbst: hakzsam, imirkin: any of you to do a quick review of my tgsi-precise patches today? Never did anything there so I would like to show them to you before posting them on the ML
11:08karolherbst: yay, that flickering is fixed :)
11:08karolherbst: I know there a re a few things missing (like parsing that PRECISE modifier from the tgsi string), but I think it's a good first start: https://github.com/karolherbst/mesa/commits/gallium_precise
11:44karolherbst: okay nice, that other tombRaider issue isn't caused by optimizations, let's look into this one
12:57karolherbst: anybody an idea what can cause something like this? EXT_shader_integer_mix
13:56imirkin: karolherbst: you're on the right track, but there's more to precise.
13:56imirkin: karolherbst: for example, if you have
13:57imirkin: foo = a + b
13:57imirkin: bar = c + a + b + d
13:57imirkin: you can't rewrite it as
13:57imirkin: bar = c + foo + d
13:58imirkin: [because floats suck]
14:04karolherbst: imirkin: I know, but I just wanted to have enough to fix the issue I've encountered
14:04karolherbst: adding more rules is optional for now
14:04karolherbst: important is: 1. tgsi infrastructure 2. fixing that issue. Everything else we can also add with follow ups
14:05imirkin: otoh, we don't have stuff like algebraic rebalancing in nouveau
14:05imirkin: so the above rewrite won't end up happening.
14:05imirkin: so the issue really is when we do inexact algebraic transformations
14:05karolherbst: mhh I think it could happen through some CSE magic by accident
14:05imirkin: which is the float mul+add -> fma
14:06imirkin: and some annoying things involving NaN's
14:06karolherbst: that's also the example written in the spec
14:06imirkin: which i don't think matter
14:06karolherbst: that fma thing
14:07karolherbst: imirkin: but to be honest, that a + b + c = a + c + b thing won't matter for precise as long as we do it _always_
14:08karolherbst: the idea behind precise is, to do the same calculation the same way. Nouveau breaks this, because there is this refcount check
14:08karolherbst: for fma
14:08karolherbst: it's the reason written inside the spec
14:08imirkin: the idea of precise is for the same calculation to come out the same way in different shaders
14:08karolherbst: well, that's what I meant
14:09imirkin: karolherbst: mind handling 'invariant' while you're at it? should be expressible in terms of 'precise'
14:09karolherbst: imirkin: in glsl to nir invariant = precise
14:09karolherbst: so yeah
14:10imirkin: that should account for a handful of CTS fails
14:10karolherbst: yeah, I guess so
14:10karolherbst: maybe I should fix most of those while I work at it
14:11karolherbst: the TGSI is just looking a bit ugly though. Any better idea to express precise? maybe the same way as saturate? https://gist.github.com/karolherbst/8a574602e6be19a4e2292faf112d80be
14:13imirkin: yeah, _PRECISE can work.
14:13imirkin: or maybe at the very end
14:13imirkin: although that may cause confusion for parsing
14:15karolherbst: allthough then we can get OP_SAT_PRECISE, but what ever
14:18karolherbst: _PRECISE version: https://gist.github.com/karolherbst/42d2e26e6f92713bd74d9b0d83950d0a
14:18karolherbst: yeah, I think this looks better
14:23karolherbst: imirkin: any idea about this other issue though?
14:25karolherbst: the thing which is broken is this fire light refraction
14:25karolherbst: this one: https://i.imgur.com/w31cIwa.png
14:26imirkin: uhm ok
14:26karolherbst: this is on intel: https://i.imgur.com/wcAog81.png
14:26karolherbst: and this is on intel before that refraction gets rendered: https://i.imgur.com/Beil0Qe.png
14:27karolherbst: I think they reuse stuff from the prior frame or something like this
14:39swedave: Hello all . I want to try to reclock my nvidia 970 with Noveau driver and mount customs fan to my GPU . I was told that the reclock works but not the fans . How can i reclock it ? Im on Linux mint 18.1 with kernel 4.12.0- rc4
14:40imirkin: karolherbst: care to provide instructions?
14:40imirkin: swedave: it's not straightforward.
14:41imirkin: [have to get the nouveau pmu firmware onto the gpu after the nvidia pmu firmware boots the gpu]
14:41swedave: ok . I thout it was like changing pstate at kernel . Im not good at linux , but i like to try stuff ;)
14:41imirkin: it's not for end users.
14:44karolherbst: well it's not hard, but messy
14:44karolherbst: 1. disable secboot and build 2. load nouveau 3. reclock 4. unload nouveau 5. enable secboot and build 6. load nouveau (and all that without rebooting)
14:45karolherbst: one could automate this with a script on boot though with two nouveau modules
14:45karolherbst: or something fancy like this
14:45swedave: When i updatet my kernel i noticed that the noveau driver with wined3d9 is almost as fast as the Nvidia driver . Graphics is better with noveau and sound . I run my monitor with hdmi .
14:46karolherbst: swedave: yeah well. We were thinking about adding a flag or something for this, because the fans on laptops are controlled by the EC in most cases
14:46imirkin: karolherbst: would this be as easy as adding a "do secboot" module param?
14:46karolherbst: but I got stuck with odd PMU issues and secboot
14:46karolherbst: imirkin: kind of, but I should rather search for the real bug why loading the nouveau PMU image after secboot doesn't work
14:47swedave: ok . I have a homebuild desktop .
14:48swedave: I noticed now that winth the noveau driver , the fans dont spin .
14:49karolherbst: swedave: yes, because we need signed firmware to control those
14:49karolherbst: but we can reclock and set whatever voltage, ironic, isn't it?
14:49swedave: Yeah , Read about it .
14:51swedave: Yeah , Thats ironic . Thats why im thought mounting custom fans and reclock . The nvidia driver with wine sux cause it convert d3dx9 to opengl . World of warcraft is not playable now :( Thats why i thouch to get messy ;)
14:51karolherbst: imirkin: mhh, that TGSI parser doesn't like me :/ I guess I missed a place to set that precise flag
14:53karolherbst: ohh, found it
14:53swedave: karolherbst . Can you email me instrucktions how i can try to get reclock work with my nvidia 970 4gb ?
14:57karolherbst: swedave: first build nouveau from source and get this working ;) you could use a drm-next kernel or 4.11 or so
15:06karolherbst: imirkin: do you think it makes sense to print the precise flag in nv50_ir as well?
15:07imirkin: dunno... maybe at the end (p) or osmething
15:07karolherbst: it may be confusing if you look at the ir and think "huh, that could be optimized"
15:09karolherbst: but I also don't mind it that much, because you will see it inside the TGSI as well
15:10imirkin: btw, your code doesn't deal with a situation like
15:10imirkin: foo = a * b;
15:10imirkin: precise bar = a * b;
15:10imirkin: foo2 = foo + c;
15:10imirkin: precise bar2 = bar + c;
15:11imirkin: [or ... something]
15:11imirkin: basically once you CSE something, you can lose the precise bit
15:11karolherbst: well, glsl does everything
15:11karolherbst: I see
15:11imirkin: so once you CSE, you should copy the precise bit from the removed value if any
15:11karolherbst: yeah, I was thinking about situations where we build new instructions and forget to check for precise flags
15:11karolherbst: but seriously, I would rather concentrate on real issues and CTS fails for now
15:12karolherbst: otherwise I could spend a month reading all codegen to figure out every corner case ;)
15:12imirkin: it's important to get this right.
15:12imirkin: a month well spent imho
15:12karolherbst: sure it is, not saying we shouldn't do it
15:12swedave: Someone needs to get out more ;)
15:12karolherbst: I should have a focus and later on we can fix up all the other parts
15:13imirkin: anyways, difficult to get EVERY case, but we should cover the ones we can think of.
15:13karolherbst: or first the ones which matter ;) second what we can think of and third what ever bugs arrise
15:13karolherbst: fixing the CTS fails may be challanging enough already
15:14imirkin: the CSE case matters.
15:14karolherbst: I know
15:14imirkin: and is easy to fix.
15:14karolherbst: just need a way to run selected test with the CTS somehow
15:16karolherbst: not really looking forward to run every case. who was it with the fancy scrpts to make those tests run under piglit, airlied?
15:18imirkin: those are checked in
15:18imirkin: there are cts_foo profiles
15:19karolherbst: ohh, how do I run those?
15:19imirkin: not easily.
15:19imirkin: it'll take you an hour to work it out the first time
15:19imirkin: but then it'll be easy to reuse.
15:19karolherbst: I could also just run the cTS binaries and select subtests though
15:22swedave: Im gonna let you highiq folks towork on . Im gonna go to the gym . Thats for explaining the situation . I just have to wait for updates then ;( Take care .
15:22swedave: thanks *
15:24karolherbst: imirkin: do you know how to list all tests in the cts?
15:24imirkin: yeah, but it doesn't work
15:24imirkin: they recently added a master file
15:24imirkin: which has all the tests you're supposed to pass
15:24karolherbst: I see
15:26karolherbst: ohhh nice
15:26karolherbst: I can run it for 3.0
15:30karolherbst: now it would be nice to know what exactly went wrong
15:31imirkin: look at TestResults.qpa
15:32imirkin: iirc there may be options you can pass to the runner to get more debug output
15:32karolherbst: it also fails with NV50_PROG_OPTIMIZE=0 :/
15:33karolherbst: it passes on intel though
15:33imirkin: yeah, st/mesa might do some stuff
15:34karolherbst: just wanted to ask how to disable those opts there
15:34imirkin: although i can't imagine what...
15:34imirkin: it does copy prop
15:34imirkin: and dce
15:34imirkin: and various register renumbering
15:34imirkin: but ... none of that should affect results
15:34imirkin: so in combination with disabling all nouveau-side opts, should be fine
15:35imirkin: welll... good luck :)
15:35imirkin: feel free to file a card
15:35imirkin: on the CTS board
15:35imirkin: and include your analysis
15:35karolherbst: it's not nv50_irs fault :p
15:37karolherbst: so how can I disable all st/mesa opts?
15:37imirkin: judicious use of "//"
15:37imirkin: oh wait
15:37imirkin: there are a couple of lame ones in there
15:37imirkin: hold on
15:39imirkin: karolherbst: oh, one more thing - i think for OP_MAD, for precise, nv50_ir_from_tgsi should emit a mul + add instead.
15:39imirkin: or maybe do it in lowering in nvc0
15:39imirkin: (but not for OP_FMA, of course)
15:39karolherbst: only ADDs are tagged as precise in the TGSI
15:39karolherbst: the TGSI looks wrong anyway
15:40karolherbst: but I also don't find the glsl
15:40imirkin:hates that there's both MAD and FMA
15:41imirkin: i wish i could take like ... a year off ... and fix all this shit
15:41karolherbst: imirkin: make a list, somebody might be able to take care of it soon
15:46karolherbst: imirkin: it suceeds now :D
15:46karolherbst: wondering why that is
15:46imirkin: after doing what?
15:46imirkin: commenting the try_emit_mad's?
15:47imirkin: coz they become FMA
15:47imirkin: but in a different shader, the try_emit_mad fails
15:47imirkin: and so it comes out as mul+add
15:47imirkin: hence diff result
15:47karolherbst: but the Precise flag should be set there as well already
15:47karolherbst: so I can disable this st/mesa pass if the instruction is flagged as precise
15:47imirkin: imo we should lower mad -> mul + add in nouveau unconditionally
15:48imirkin: irrespective of precise.
15:48imirkin: and then recombine into mad if possible.
15:48karolherbst: compilers are allowed to do the mul+add=mad opt
15:48imirkin: as an optimization
15:48karolherbst: well sure, but if we split it, we would just opt it again
15:48karolherbst: because the precise flag isn't there
15:49karolherbst: so it's kind of bogus to do that
15:49imirkin: shouldn't there be a precise flag in this case though?
15:49imirkin: in which case it wouldn't get recombined
15:49karolherbst: not really
15:49imirkin: that's a problem.
15:49karolherbst: I just check for precise in that st/mesa pass
15:49karolherbst: it's simple enough
15:50karolherbst: or we could simply remove those peepholes
15:51imirkin: look ... you're not trying to hack your way around things until this one test passes
15:51imirkin: you want to do something that is logically consistent
15:51imirkin: logically, MAD = mul + add; FMA = fma
15:52karolherbst: I know, but if the expression is tagged as precise, st/mesa isn't allowed to do this mad peephole as well
15:52imirkin: should be possible for MAD to have a precise flag
15:52imirkin: sure it is.
15:52imirkin: why not?
15:52imirkin: MAD = mul + add. all's well.
15:52karolherbst: because it's tagged as precise
15:52imirkin: it's nouveau that implements it as fma, which is wrong.
15:53karolherbst: mhh, okay, I see your point
15:53imirkin: which is also why we should disassociate MAD on input
15:53imirkin: and then reassociate as possible.
15:53imirkin: that said, the st/mesa opt is pretty dumb too
15:53imirkin: so wtvr
15:54imirkin: but the st/mesa thing isn't wrong. it's just dumb.
15:54karolherbst: I see
15:54karolherbst: fun thing is, that those mul
15:54karolherbst: +adds aren't tagged as precise in the first place
15:54imirkin: what's the glsl say?
15:55karolherbst: I try to find it
15:55imirkin: MESA_GLSL=dump helps
15:55karolherbst: "precise float result = (p.x*w.x + p.y*w.y) + (p.z*w.z + p.w*w.w);"
15:55karolherbst: all subexpressions need to be tagged as well afaik
15:56imirkin: of course, yea
15:58imirkin: yeah, you need to see how glsl -> nir handles it
15:58imirkin: or ask around
15:58karolherbst: I already checked
15:58karolherbst: and I was sure I do exactly the same thing
15:58imirkin: could be that the nir thing is incomplete and intel just gets lucky
15:59karolherbst: well nouveau also gets lucky
15:59imirkin: or could be that nir has some logic on its side to propagate precise's
15:59karolherbst: mhhh, doubt it
16:01karolherbst: imirkin: what's "hir" again?
16:01imirkin: hierarchical ir visitor
16:02imirkin: [i have no clue]
16:02imirkin: it goes ast -> hir -> ir
16:02imirkin: or something
16:02karolherbst: ahh okay
16:02karolherbst: so unrelated to nir
16:02karolherbst: there is only one line regarding precise in glsl_to_nir
16:03karolherbst: and it's where assignments are checked
16:03karolherbst: but maybe nir has some more information and can analye the entire assignment tree to tag more as precise
16:03karolherbst: and we would need to do that in glsl_to_tgsi as well
16:05imirkin: they might treat the whole ssa chain as precise - i dunno.
16:06imirkin: like i said ... worth asking
17:30karolherbst: imirkin: nice, got it working with a crapy hack :/
17:30karolherbst: the mads are flagged as precise as well though
17:31karolherbst: this is uper ugly: https://github.com/karolherbst/mesa/commit/4e65f8a1e5e97038adf8fd35ef48dacc355da815
17:31karolherbst: especially because I've added state to glsl_to_tgsi_visitor
17:32karolherbst: the lines around "ir->rhs->accept(this);" is the important part
17:34imirkin: not the worst thing in the world
17:36karolherbst: but I think other ways especially if it comes to subtree parsing are much more complex than this
17:37karolherbst: especially because precise has no effect if it's declared out of scope
17:38karolherbst: interesting is stuff like float f = a * b; precise float g = 2*f+a
17:38karolherbst: is the expression a*b also taged as precise?
17:38karolherbst: but I think it's a yes here
17:38imirkin: talk to kayden
17:39imirkin: and/or jekstrand
17:39karolherbst: no, it's a yes
17:39imirkin: i believe one of them did the i965 precise stuff
17:39karolherbst: it's basically in the spec
17:39karolherbst: "ome examples of the use of "precise" include:" part
17:39imirkin: and can share some insights with you about how to properly implement it
17:39karolherbst: the main function there
17:39karolherbst: but I think glsl handles this already
17:40karolherbst: it looked like it looking at the tomb_raider shader output
17:40karolherbst: anyway, now I implement this mad->mul+add split in nv50_ir and then we should be good to go
17:45karolherbst: pass :)
17:47karolherbst: imirkin: what do you think? https://github.com/karolherbst/mesa/commit/1a89b4afc582df4d08aa5086384119da282ac1c5
17:48imirkin: sounds reasonable
17:49karolherbst: would be fun if that other tomb raider bug is fixed now as well
17:50karolherbst: sadly no
17:57karolherbst: 'KHR-GL44.shaders.arrays.constructor.float4_fragment' passes
17:58karolherbst: but causes segfault when 'KHR-GL44.shaders.arrays.constructor.float4_*' is ran
17:58karolherbst: sounds like memory corruption somewhere
18:06imirkin: annoying, right?
18:06imirkin: there are a few of those.
18:06karolherbst: super annoying
18:12karolherbst: but it seems like we pass quite a lot of tests already
18:18karolherbst: this looks interesting: https://gist.github.com/karolherbst/ef7ab8b4b5e5201d6967f104a2664db6
18:20imirkin: oh yeah... i seem to recall debugging that... something gets left in a bin that's not supposed to
18:23karolherbst: imirkin: wanna look at my whole precise series again and then after fixing your comments I post it as RFC on mesa-dev?
18:30imirkin: karolherbst: just post it...
18:42karolherbst: mhhh, I should alter my CC settings :/ sorry
20:06karolherbst: imirkin: ohh, I think I found the error for that memory thing
20:06karolherbst: the same entry is twice inside the list
20:07karolherbst: that's odd though
20:19karolherbst: when I ignore on_flush in nvc0_bufctx_fence and always use the pending bufctx, there is no invalid read.
20:24karolherbst: KHR-GL44.cull_distance.functional is broken on a glsl ir level
20:25karolherbst: but doesn't fail on intel? ...
20:33karolherbst: yeah, that looks wrong: "(declare (location=17 shader_out ) (array float 0) gl_ClipDistance)"
20:39tobijk: karolherbst: it is right cull and clip get narrowed together ;>
20:40tobijk: see lower_distance.cpp or how its called
20:41karolherbst: tobijk: I get an error though
20:41karolherbst: "ir_variable has maximum access out of bounds (1 vs 0) (declare (location=17 shader_in ) (array (array vec4 2) 1) gl_ClipDistanceMESA)"
20:42tobijk: not said its implemented right, just that it is put into one slot
20:42karolherbst: I see
20:45tobijk: karolherbst: mh there was soething with array size=1 and cull, but i cant remember exactly
20:46imirkin: karolherbst: yeah, it's a weird one
20:47imirkin: karolherbst: double-weird is that it passes on radeon :)
20:52karolherbst: and on intel
20:53tobijk: karolherbst: well intel does not use the lowering and gallium
20:54tobijk: two places to get things wrong :/
20:54karolherbst: "Conditional jump or move depends on uninitialised value(s)" :)
20:56karolherbst: tobijk: is this array an implicit_sized_array?
20:58tobijk: should be :>
20:59karolherbst: okay, at least that "!fields[i].implicit_sized_array" is not initialized
21:05tobijk: karolherbst: what bothers me: normaly there should be lower_distance.cpp in your call stack
21:05karolherbst: tobijk: I think I compiled with Ofast
21:05tobijk: validate_ir_tree() is called rigth after the lowering not somewhere later
21:06karolherbst: ohh wait
21:06karolherbst: actually with O0
21:10tobijk: karolherbst: mh the lowering pass looks different to the original one
21:10karolherbst: what do you mean?
21:11tobijk: its a bit different to what i remember :)
21:11karolherbst: ahh I see
21:11karolherbst: well code changes :p
21:20tobijk: karolherbst: am i right, there is no clip size defined?
21:20tobijk: or better it is 0, only cull is used in the test?
21:22tobijk: karolherbst: https://cgit.freedesktop.org/mesa/mesa/tree/src/compiler/glsl/lower_distance.cpp#n674 make this cull_size and give it a test :>
21:30airlied: pretty surs clip size is correct
21:30tobijk: airlied: shouldnt it be cull_size if cli_size is 0?
21:30airlied: its an offset
21:31airlied: where to start the cull dists
21:31tobijk: uhm the offset is after that
21:31airlied: no it isnt
21:31airlied: there are two initialiser paths for the isiotr
21:32airlied: i have vague memories of writing that pass
21:33airlied: only a year ago, how quickly i forget
21:35karolherbst: anyhow, clip_size is 0
21:35karolherbst: okay, makes sense though
21:35tobijk: airlied: hmhm so new_size is 0 and max_array_access ends up as -1
21:36tobijk: not sure if that matters
21:36karolherbst: ohh, when is the glsl shader printed, after it linked or before that?
21:37karolherbst: I was wondering, because I don'T think the shader I've got has anything todo with the error but is a new shader all along
21:38tobijk: karolherbst: which test are you runnig exactly and can i get my hands on it? :D
21:38karolherbst: tobijk: ./glcts --deqp-case='KHR-GL44.cull_distance.functional'
21:39tobijk: well the cts are not for veryone as i remember?
21:39karolherbst: I only have access to the public stuff
21:39karolherbst: tobijk: https://github.com/KhronosGroup/VK-GL-CTS
21:39tobijk: oh i'm out of the loops :>
21:40karolherbst: removed that abort() call now
21:40karolherbst: let's see
21:41airlied: tobijk: new sizr is never 0
21:41karolherbst: ahh, now it makes more sense
21:41airlied: total size has to be > 0
21:44tobijk: airlied: yep oversaw the orig->total_size ~_~
21:45karolherbst: mhhh, it seems like there are multiple tests ran at once
21:50tobijk: hrmpf the build system does not honor my prefix given to configure everywhere :/
21:50tobijk: */usr/bin/install: das Entfernen von '/usr/share/glvnd/egl_vendor.d/50_mesa.json' ist nicht möglich: Keine Berechtigung*
21:51karolherbst: don't install it
21:51karolherbst: why would you?
21:51tobijk: well configure mesa with prefix and just hit make install
21:51tobijk: goes to the prefix path
21:51karolherbst: ohhh mesa
21:51tobijk: well it did in the good old days :-)
21:52karolherbst: maybe that glvnd path is hardcoded?
21:52tobijk: i think the forgot the prefix to include or simething
21:52karolherbst: it's a fixed path
21:53tobijk: jep, still not my favored behavior
21:54karolherbst: tobijk: no, I meant according to spec the path is fixed. It doesn't make sense to install it somewhere else
21:54karolherbst: but yeah
21:54karolherbst: they should honor it
21:54tobijk: mhm damn that spec :/
22:15tobijk: karolherbst: do you run the cts with additional args to work with prime?
22:16karolherbst: tobijk: second X server
22:16karolherbst: tobijk: also you need to force 4.5 450 with nouveau ;)
22:17karolherbst: tobijk: but you can also use DRI_PRIME
22:17karolherbst: you just need to force a higher gl version
22:25tobijk: karolherbst: oh with the test clip and cull sizes are 0 and thus get not lowered at all
22:25karolherbst: there are several though
22:25karolherbst: there are several tests where both values are 0
22:29karolherbst: this should be the shader messing up on nouveua: https://gist.github.com/karolherbst/1a0b741155e8c15cd4b7beaeb9190089
22:39karolherbst: tobijk: shader_test file: https://gist.githubusercontent.com/karolherbst/24f83b4367231268d937c3b03a1d7bbb/raw/c590f4a3d8d7ff4a6770b6e9e5c7bc6c4ea1863c/gistfile1.txt
22:41tobijk: karolherbst: ay thanks
22:47karolherbst: tobijk: do you see how lower_clip_cull_distance isn't called for intel?
22:48karolherbst: imirkin: you will love this
22:52tobijk: karolherbst: yeah as i said, they do not lower :)
22:52karolherbst: I guess the lowering code is a bit faulty then
22:52karolherbst: does it even work on AMD?
22:52tobijk: i only see cull sizes of 0 instead o 8
22:52karolherbst: I would be surprised if it suceeds
22:57karolherbst: tobijk: hum, I think there is something odd
22:58karolherbst: this "in float clipdistance_data;" confuses me
22:59karolherbst: there is no reason why gl_ClipDistanceMESA should be access at index 1, because nothing reads from it
22:59karolherbst: or I am just blind
22:59karolherbst: I can't see why the compiler generates this expression: "(declare (location=17 shader_in ) (array (array vec4 2) 1) gl_ClipDistanceMESA)"
23:00tobijk: mhm im no further :/
23:02karolherbst: tobijk: after removing all ASSIGN_CULL_DISTANCE calls in the geometry shader, it compiles
23:03tobijk: heh well you just removed the error then :) (nothing uses cull, just discard it)
23:04karolherbst: but the error is about clip
23:04karolherbst: not cull
23:04tobijk: no its clipDistanceMesa
23:04tobijk: clipDistance and cullDistance get lowered to that
23:04karolherbst: it's an array for both?
23:05karolherbst: okay, makes sense then
23:05karolherbst: wondering why that array is of size 0 then
23:06tobijk: yep it should be 1
23:07tobijk: (we lower to 2x4)
23:10karolherbst: going to bed sounds like a reasonable plan now :)
23:10tobijk: yep good idea :)
23:10airlied: I'm pretty sure radeon passes that test fine
23:10karolherbst: airlied: but how?
23:11karolherbst: gallium complains if that lowering isn't done
23:11airlied: I'm pretty sure I even wrote the lowering pass on radeon
23:11karolherbst: ohh, so there is special handling for radeon for this?
23:12karolherbst: would you mind to check that?
23:12karolherbst: because it doesn't looks like it should work
23:12airlied: yupo just going to now
23:12karolherbst: maybe somebody just broke something
23:14airlied: might take me a few mins, the machine I nromally run CTS on seem to be dead
23:15tobijk: a quick search did only bring up two commits really: https://cgit.freedesktop.org/mesa/mesa/commit/?id=64e201ab8f08daa2c189ab615a4096daf60c27c5 and https://cgit.freedesktop.org/mesa/mesa/commit/?id=fc707f570f918ab0defd33405c8c82f307196d17
23:16imirkin: it's something about some lowering which is done differently between nouveau and radeon
23:16tobijk: imirkin: and silly me thought it was run unconditionally if you use gallium
23:17karolherbst: it should run on both actually
23:17airlied: is from 17.0.5 just building master
23:18imirkin: i mean, some unrelated lowering
23:18imirkin: like ... e.g. return lowering
23:18karolherbst: ohh, I see
23:18karolherbst: yeah, that might explain it
23:19imirkin: which in turn is triggering a bug in one of the passes that radeon/intel don't trigger
23:20airlied: it's also valgrind clean here
23:21tobijk: airlied: master? so we are back to square 1
23:22airlied: oh debug radeon build shows it here
23:22karolherbst: that un-init read?
23:22airlied: the assert
23:23karolherbst: ohh wait
23:23karolherbst: on nouveau as well
23:23karolherbst: on nouveau there is just a silly test fail: Fail (Vertex is unexpectedly clipped or invisible at gl3cCullDistanceTests.cpp:2437)
23:23karolherbst: on release build
23:23airlied: we pass the test on release build
23:24airlied: let me go see if I can work it out
23:24karolherbst: but at least radeon hits the same assert in the glsl code as well
23:24karolherbst: now the world makes sense again :)
23:25karolherbst: tobijk: fyi, running CTS and piglit on a second X server doesn't bring up those annoying windows all the time ;)
23:26airlied: PIGLIT_NO_WINDOW=1 gets rid of most of them
23:27karolherbst: well sure, but I also don
23:27tobijk: oh piglit gets mature :D
23:27karolherbst: 't like that my main X server gets messed up
23:27karolherbst: sometimes.... it gets weird
23:27karolherbst: especially if a test causes to hang the GPU or stuff like this
23:28tobijk: karolherbst: well then i'm fucked anyway as my external screen is connected to the nvidia card
23:28karolherbst: but I've heard nouveau can recover from faults now
23:29tobijk: karolherbst: actually i saw it revoer from some minor errors just fine
23:29karolherbst: yeah I know it works most of the time, but I still get nouveau to crash real hard
23:32airlied: looks like it's validating the wrong level of array if I had to guess, not sure how max_array_access works for multi-level arrays
23:33tobijk: airlied: you got to the lowering pass with actual sizes for clip and cull in that test?
23:34tobijk: max_array_access was my idea earlier on where i bugged you about, but it looks like it does not even get there
23:34karolherbst: have fun, I am outta here
23:34tobijk: karolherbst: good night
23:34airlied: max_array_acccess is being set wrong
23:36airlied: https://paste.fedoraproject.org/paste/xDQiVUptpzVFmYurfLr2IQ fixes it here
23:39tobijk: airlied: let me do a clean compile
23:47airlied: patch sent to list
23:50tobijk: mh ok then one: one less bug more to go for nouveau