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