<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Sat, Mar 26, 2016 at 3:00 PM, Rob Clark <span dir="ltr"><<a href="mailto:robdclark@gmail.com" target="_blank">robdclark@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>On Sat, Mar 26, 2016 at 5:43 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> wrote:<br>
> On Sat, Mar 26, 2016 at 8:22 AM, Rob Clark <<a href="mailto:robdclark@gmail.com" target="_blank">robdclark@gmail.com</a>> wrote:<br>
>> btw, I do remember for lower_io I wanted the mode to be a bitmask so<br>
>> we could more easily deal w/ multiple modes in the same pass, but that<br>
>> exceeded # of bits somewhere or other.  I suppose maybe nir_var_all is<br>
>> good enough..  offhand I can't think of a scenario where we want to<br>
>> lower some but not all..<br>
><br>
><br>
> I'd rather do that too.  I don't remember why we didn't before (too lazy to<br>
> search the mailing list).  It shouldn't overflow a uint32_t, there are only<br>
> 9 modes last I checked.  We may add more, but it'll be a while before we add<br>
> 22 more.<br>
<br>
</div></div>I dug it up, and the issue is in nir_variable_data:<br>
<br>
      nir_variable_mode mode:5;<br>
<br>
ofc we could just give more bits to that and be done...<br></blockquote><div><br></div><div>Adding connor to the Cc in case he has an opinion...<br><br></div><div>I think that specifying multiple modes is something that will happen often enough that we should just standardize it.  I see three different possibilities here:<br><br></div><div> 1) nir_lower_io takes a nir_var_all enum value which is -1 and indicates "everything"<br></div><div> 2) nir_lower_indirect_derefs takes a uint32_t mode_mask which is expected to be a bunch of (1 << mode) ORd together<br></div><div> 3) We could just convert each of the mode enum values to be a single bit and support ORing them together<br></div><div> <br></div><div>(1) is pretty terrible since it only allows one or all and doesn't allow you to specify a non-trivial subset.  (2) allows for enum confusion but is a fairly standard pattern in mesa today so maybe not too bad.  (3) requires more bits when you only want to specify one enum (meh), requires that we validate that only one bit is set on a variable, and will be confused with the common pattern of (2) in mesa.  On the other hand (3) prints nicely in gdb and looks nice when you write it.<br><br></div><div>All told, I don't like (1) at all and have a mild preference for (2) over (3).  However, I could be convinced of (3) should others have a strong inclination towards it.<br><br></div><div>Regardless of what we choose, I'll update this patch and submit patches to fix up nir_lower_io and nir_lower_indirect_derefs to match.<br><br></div><div>--Jason<br></div><div><br></div></div></div></div>