16:31 karolherbst: I wish we wouldn't have tests only failing if we run piglit in parallel :/
16:31 karolherbst: randomly failing
19:17 pmoreau: karolherbst: Do you have (or remember) examples of why Connor's patch is needed? Was it something we were triggering with the U64 addresses?
19:17 karolherbst: yeah... RA kind of relies on merges/splits to be there for all values
19:17 karolherbst: but
19:18 karolherbst: or... wait, let me try to remember which case exactly broke
19:18 karolherbst: pmoreau: it was something stupid like no movs/merges within phi nodes
19:19 pmoreau: Yeah, it feels like there were some phi nodes involved in the story as well.
19:22 karolherbst: pmoreau: something like that: https://gist.githubusercontent.com/karolherbst/c58aa8ff07db82f672f4ec4092075a30/raw/cac2fa69a941b9696bf727377af89429b209b177/gistfile1.txt
19:22 karolherbst: I know I ran into issues having that around
19:23 pmoreau: Interesting, I didn’t know you could pass 2 "64-bit regs" to a st.128 instead of just one "128-bit reg"
19:24 HdkR: bweh?
19:25 pmoreau: HdkR: Look at Karol's link; unless it is something he wrote on the fly rather than taken from a compiled program.
19:26 HdkR: Is that just a hint to the RA later that it must be a consecutive vec4 register?
19:27 HdkR: Because uh...You can't do that
19:27 pmoreau: Maybe? Though I would have expected to have a `merge` instead, whose result is fed to the store.
19:28 HdkR: Sure, me too
19:38 karolherbst: HdkR: why not?
19:38 karolherbst: HdkR: it ends up being a quad op
19:38 karolherbst: this is pre RA
19:38 karolherbst: *reg
19:38 karolherbst: so RA takes those two double regs and merges them into a quad reg
19:39 HdkR: karolherbst: Yea, that was my first assumption that it was a hint to RA that it needed to be a 128bit reg
19:39 HdkR: Just looks a bit strange in that representation
19:40 karolherbst: sure
19:40 HdkR: As pmoreau said, I would have assumed to see another merge and only 1 register as an argument passed to the store :)
19:40 karolherbst: pmoreau: anyway, I am sure mesa master can't handle those cases inside RA
19:40 HdkR: Same thing, just represented differently
19:41 karolherbst: HdkR: ahhh
19:41 karolherbst: yeah, we don't do that
19:41 karolherbst: afaik
19:41 karolherbst: might be smarter to have done it this way though
19:42 karolherbst: HdkR: we have the same situation with texs though where we just put in the scalar regs and let RA merge those
19:42 HdkR: Right
19:43 HdkR: I can see the appeal both ways, no worries, just confusing since I've never looked at the IR before :)
19:43 karolherbst: pmoreau: yeah, I think it was also running issues if you have a (store (merge ) (merge ) ) as well
19:44 HdkR: LLVM GISel has a merge_values op for example. Where it just takes a bunch of same size values and the resulting value is just a <bits>*<N values> sized value, Which creates some disgusting patterns
19:44 karolherbst: :D
19:44 karolherbst: HdkR: I guess you could merge 32 1 bit sized bools together
19:44 HdkR: Aye
19:45 HdkR: and if you're cool enough that merge would then DCE random bits inside of it and you'll have a partially undef'd i32
19:45 karolherbst: nice
19:46 karolherbst: the problem with split/merges are that it is kind of difficult to handle partial uses correctly
19:46 karolherbst: like you compute a 64 bit value, but only use the higher bits
19:47 karolherbst: and because you don't really track through splits/merges, you can't DCE it away
19:47 karolherbst: I am sure we don't
19:47 karolherbst: normally it doesn't matter, except you don't support those 64 bit ops natively and lower those
19:47 HdkR: I think LLVM tracks through the merges and unmerges and will DCE + undef things unused
19:47 karolherbst: yeah.. but that's llvm
19:48 HdkR: I'd have to double check but I've definitely had that problem with SDag before
19:48 HdkR: Yea, LLVM isn't fast :)
19:48 HdkR: Poke the monster
19:50 karolherbst: I guess general purpose compilers always end up being huge monsters
19:50 HdkR: Pretty much
19:50 HdkR: Domain specific is so much nicer isn't it? :)
20:51 pmoreau: karolherbst: Duh, I missed you had updated the patches I had commented on --"
20:51 karolherbst: pmoreau: :D
22:03 pmoreau: karolherbst: Hum, going back through your patch 05 (adding NIR support), and I’m still puzzled by the asymmetry between sp_state_create and cp_state_create regarding cloning or not the NIR stuff. I had some info about it on the old commit, but I can’t find it back.
22:06 karolherbst: pmoreau: huh? both should do the same now
22:06 karolherbst: aka not cloning
22:06 karolherbst: I clone inside nv50_program_translate though
22:07 karolherbst: and nvc0
22:08 karolherbst: pmoreau: 67950da37c58b5d467fcb61d02875043d034dbb4 right?
22:08 pmoreau: Duh, you are right, it’s nv50_program_translate only which is cloning.
22:09 pmoreau: Is it expected that the state_delete is releasing NIR, when it didn’t allocate/clone it?
22:09 karolherbst: I am cloning there because I manipulate the NIR
22:09 karolherbst: pmoreau: we took ownership of the nir, yes
22:09 pmoreau: Right, I remember that you need to clone it there as you change it.
22:10 pmoreau: Ah, that’s true.
22:12 pmoreau: Would you mind adding a comment in (sp|cp)_state_create (for example https://github.com/karolherbst/mesa/commit/67950da37c58b5d467fcb61d02875043d034dbb4#diff-336065b196d4bfd11a37186c536f9affR769) to future me (or others), that we are taking the ownership? O:-)
22:14 karolherbst: sure