<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Thu, Jan 3, 2019 at 1:37 PM Roland Scheidegger <<a href="mailto:sroland@vmware.com">sroland@vmware.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Am 03.01.19 um 18:58 schrieb Jason Ekstrand: > On Thu, Jan 3, 2019 at 3:39 AM Erik Faye-Lund > <<a href="mailto:erik.faye-lund@collabora.com" target="_blank">erik.faye-lund@collabora.com</a> <mailto:<a href="mailto:erik.faye-lund@collabora.com" target="_blank">erik.faye-lund@collabora.com</a>>> wrote: > > On Wed, 2019-01-02 at 10:16 -0600, Jason Ekstrand wrote: > > On Wed, Jan 2, 2019 at 9:43 AM Ilia Mirkin <<a href="mailto:imirkin@alum.mit.edu" target="_blank">imirkin@alum.mit.edu</a> > <mailto:<a href="mailto:imirkin@alum.mit.edu" target="_blank">imirkin@alum.mit.edu</a>>> > > wrote: > > > Have a look at the first 4 patches in the series from Jonathan > > > Marek > > > to address some of these issues: > > > > > > <a href="https://patchwork.freedesktop.org/series/54295/" rel="noreferrer" target="_blank">https://patchwork.freedesktop.org/series/54295/</a> > <<a href="https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fseries%2F54295%2F&data=02%7C01%7Csroland%40vmware.com%7Cb5a1b18f854a4533274508d671a52647%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636821351481601457&sdata=RPh1ZhwqvjBP8eSmE9D%2BErOVgCcmb4kobJVy%2BpJeyIc%3D&reserved=0" rel="noreferrer" target="_blank">https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fseries%2F54295%2F&data=02%7C01%7Csroland%40vmware.com%7Cb5a1b18f854a4533274508d671a52647%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636821351481601457&sdata=RPh1ZhwqvjBP8eSmE9D%2BErOVgCcmb4kobJVy%2BpJeyIc%3D&reserved=0</a>> > > > > > > Not sure exactly what state that work is in, but I've added > > > Jonathan > > > to CC, perhaps he can provide an update. > > > > > > Cheers, > > > > > > -ilia > > > > > > On Wed, Jan 2, 2019 at 6:28 AM Qiang Yu <<a href="mailto:yuq825@gmail.com" target="_blank">yuq825@gmail.com</a> > <mailto:<a href="mailto:yuq825@gmail.com" target="_blank">yuq825@gmail.com</a>>> wrote: > > > > > > > > Hi guys, > > > > > > > > I found the problem with this test fragment shader when lima > > > development: > > > > uniform int color; > > > > void main() { > > > > if (color > 1) > > > > gl_FragColor = vec4(1.0, 0.0, 0.0, 1); > > > > else > > > > gl_FragColor = vec4(0.0, 1.0, 0.0, 1); > > > > } > > > > > > > > nir_print_shader output: > > > > impl main { > > > > block block_0: > > > > /* preds: */ > > > > vec1 32 ssa_0 = load_const (0x00000001 /* 0.000000 */) > > > > vec4 32 ssa_1 = load_const (0x3f800000 /* 1.000000 */, > > > > 0x00000000 /* 0.000000 */, 0x00000000 /* 0.000000 */, 0x3f800000 > > > /* > > > > 1.000000 */) > > > > vec4 32 ssa_2 = load_const (0x00000000 /* 0.000000 */, > > > > 0x3f800000 /* 1.000000 */, 0x00000000 /* 0.000000 */, 0x3f800000 > > > /* > > > > 1.000000 */) > > > > vec1 32 ssa_3 = load_const (0x00000000 /* 0.000000 */) > > > > vec1 32 ssa_4 = intrinsic load_uniform (ssa_3) (0, 1, 0) > > > /* > > > > base=0 */ /* range=1 */ /* component=0 */ /* color */ > > > > vec1 32 ssa_5 = slt ssa_0, ssa_4 > > > > vec1 32 ssa_6 = fnot ssa_5 > > > > vec4 32 ssa_7 = bcsel ssa_6.xxxx, ssa_2, ssa_1 > > > > intrinsic store_output (ssa_7, ssa_3) (0, 15, 0) /* > > > base=0 */ > > > > /* wrmask=xyzw */ /* component=0 */ /* gl_FragColor */ > > > > /* succs: block_1 */ > > > > block block_1: > > > > } > > > > > > > > ssa0 is not converted to float when glsl to nir. I see > > > glsl_to_nir.cpp > > > > will create flt/ilt/ult > > > > based on source type for gpu support native integer, but for gpu > > > not > > > > support native > > > > integer, just create slt for all source type. And in > > > > nir_lower_constant_initializers, > > > > there's also no type conversion for integer constant. > > > > This is a generally sticky issue. In NIR, we have no concept of > > types on SSA values which has proven perfectly reasonable and > > actually very powerful in a world where integers are supported > > natively. Unfortunately, it causes significant problems for float- > > only architectures. > > I would like to take this chance to say that this untyped SSA-value > choice has lead to issues in both radeon_si (because LLVM values are > typed) and zink (similarly, because SPIR-V values are typed), where we > need to to bitcasts on every access because there's just not enough > information available to emit variables with the right type. > > > I'm not sure if I agree that the two problems are the same or not... > More on that in a bit. > > > It took us a lot of time to realize that the meta-data from the opcodes > doesn't *really* provide this, because the rest of nir doesn't treat > values consistently. In fact, this feels arguably more like buggy > behavior; why do we even have fmov when all of the time the compiler > will emit imovs for floating-point values...? Or why do we have bitcast > > > Why do we have different mov opcodes? Because they have different > behavior in the presence of source/destination modifiers. You likely > don't use those in radeon or Zink but we do use them on Intel. That > being said, I've very seriously considered adding a generic nir_op_mov > which is entirely typeless and doesn't support modifiers at all and make > most of core NIR use that. For that matter, there's no real reason why > we need fmov with modifiers at all when we could we could easily replace > "ssa1 = fmov.sat |x|" with "ssa1 = fsat |x|" or "ssa1 = fabs.sat x". If > we had a generic nir_op_mov to deal with swizzling etc., the only thing > having i/fmov would really accomplish is lowering the number of > different ways you can write "fmov.sat |x|". The only reason I haven't > added this nir_op_mov opcode is because it's a pile of work and anyone > who can't do integers can just implement imov as fmov and it's not a big > deal. > > > I would really love it if we could at least consider making this "we > can just treat floats as ints without bitcasts if we feel like it"- > design optional for the backend somehow. > > I guess the assumption is that bitcasts are for free? They aren't once > you have to emit them and have a back-end remove a bunch of redundant > ones... We should already have all the information to trivially place > casts for backends that care, yet we currently make it hard (unless > your HW/backend happens to be untyped)... > > Is there some way we can perhaps improve this for backends that care? > > > That's an interesting question... > > Before answering that question, let's back up and ask why. From a > hardware perspective, I know of no graphics hardware that puts types on > registers. On Intel, we assign types to sources and destinations of > instructions but outside a specific instruction, a register is just a > bag of bits. I suspect most other hardware is the same. In the > hardware that his particular thread was originally about, you could > argue that they have a type and it's always float. The real issue that > the GLES2 people are having is that constants have a bag of bits value > which may need to be interpreted as integer or float depending on > context which they do not have when lowering. They don't care, for > instance, about the fact that imov or bcsel take integers because they > know that the sources will either be floats or integers that they've > lowered to floats and so they know that the result will be a float. > > The problem you're describing is in converting from NIR to another IR, > not to hardware. In LLVM they made a choice to put types on SSA values > but then to have the actual semantics be based on the instruction > itself. This leads to lots of redundancy in the IR but also lots of > things you can validate which is kind-of nice. Is that redundancy > really useful? In our experience with NIR, we haven't found it to be > other than booleans (now sorted), this one constant issue, and > translating to IRs that have that redundancy. Then why did they add > it? I'm really not sure but it may, at least somewhat, be related to > the fact that they allow arrays and structs in their SSA values and so > need full types. This is another design decision in LLVM which I find > highly questionable. What you're is more-or-less that NIR should carry, > maintain, and validate extra useless information just so we can pass it > on to LLVM which is going to use it for what, exactly? Sorry if I'm > extremely reluctant to make fairly fundamental changes to NIR with no > better reason than "LLVM did it that way". > > There's a second reason why I don't like the idea of putting types on > SSA values: It's extremely deceptive. In SPIR-V you're allowed to do an > OpSConvert with a source that is an unsigned 16-bit integer and a > destination that is an unsigned 32-bit integer. What happens? Your > uint -> uint cast sign extends! Yup.... That's what happens. No joke. > The same is true of signed vs. unsigned division or modulus. The end > result is that the types in SPIR-V are useless and you can't actually > trust them for anything except bit-size and sometimes labelling > something as a float vs. int. This is really a decision of spir-v only though, llvm doesn't have that nonsense (maybe for making it easier to translate to other languages?) - there's just int and float types there, with no distinction between signed and unsigned. </blockquote><div> </div><div>I think with SPIR-V you could probably just pick one and make everything either signed or unsigned. I'm not immediately aware of any opcodes that require one signedness or the other; most just require an integer or require a float. That said, this is SPIR-V so I'm not going to bet money on that... </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> You are quite right though that float vs. int is somewhat redundant too due to the instructions indicating the type. I suppose there might be reasons why there's different types - hw may use different registers for instance (whereas of course noone in their right mind would use different registers for signed vs. unsigned ints), or there might be some real cost of bitcasts (at least bypass delays are common for cpus when transitioning values between int and float execution units). For instance, it makes a difference with x86 sse if you use float or int loads, which otherwise you couldn't indicate directly (llvm can optimize this into the right kind of load nowadays even if you use the wrong kind of variable for the load, e.g. int load then bitcast to float and do some float op will change it into a float load, but this is IIRC actually a pretty new ability, and possibly doesn't happen if you disable enough optimization passes). </blockquote><div> </div><div>Having it for the purpose of register allocation makes sense in the CPU world. In the GPU world, everything is typically designed float-first and I'm not aware of any hardware has separate int and float registers like x86 does. That said, hardware changes over time and it's entirely possible that someone will decide that a heterogeneous register file is a good idea on a GPU. (Technically, most GPUs do have flag regs or accumulators or something but it's not as bad as x86.) Our of curiosity, do you (or anyone else) know if LLVM actually uses them for that purpose? I could see that information getting lost in the back-end and them using something else to choose registers.</div><div> </div><div>--Jason </div></div></div>