<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Feb 16, 2015 at 1:03 PM, Connor Abbott <span dir="ltr"><<a href="mailto:cwabbott0@gmail.com" target="_blank">cwabbott0@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Mon, Feb 16, 2015 at 3:57 PM, Connor Abbott <<a href="mailto:cwabbott0@gmail.com">cwabbott0@gmail.com</a>> wrote:<br>
> Now that we talked about this on the last version of this patch, this<br>
> makes more sense. But I'd like to see something in the commit message<br>
> explaining what the issue was (going too far and casting the exec_node<br>
> embedded in the block to a nir_instr) and how you put the patch that<br>
> changes the behavior of nir_instr_prev() to actually fix this next in<br>
> order to avoid segfaults in the middle of the series.<br>
<br>
</span>Whoops, I forgot... once you do that, you get my r-b for this and the<br>
next patch.<br><div class="HOEnZb"><div class="h5"></div></div></blockquote><div><br></div><div>How about:<br><br></div><div>Right now, the nir_instr_prev function function blindly looks up the previous element in the exec list and casts it to an instruction even if it's the tail sentinel.  The next commit will change this to return null if it's the first instruction.  Making this change first avoids getting a segfault between commits.  The only reason we never noticed is that, thanks to the way things are laid out in nir_block, the casted instruction's type was never parallal_copy.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
><br>
> On Mon, Feb 9, 2015 at 11:24 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
>> ---<br>
>>  src/glsl/nir/nir_from_ssa.c | 2 +-<br>
>>  1 file changed, 1 insertion(+), 1 deletion(-)<br>
>><br>
>> diff --git a/src/glsl/nir/nir_from_ssa.c b/src/glsl/nir/nir_from_ssa.c<br>
>> index 3625237..7c50095 100644<br>
>> --- a/src/glsl/nir/nir_from_ssa.c<br>
>> +++ b/src/glsl/nir/nir_from_ssa.c<br>
>> @@ -272,7 +272,7 @@ get_parallel_copy_at_end_of_block(nir_block *block)<br>
>>     if (last_instr->type == nir_instr_type_jump)<br>
>>        last_instr = nir_instr_prev(last_instr);<br>
>><br>
>> -   if (last_instr->type == nir_instr_type_parallel_copy)<br>
>> +   if (last_instr && last_instr->type == nir_instr_type_parallel_copy)<br>
>>        return nir_instr_as_parallel_copy(last_instr);<br>
>>     else<br>
>>        return NULL;<br>
>> --<br>
>> 2.2.2<br>
>><br>
>> _______________________________________________<br>
>> mesa-dev mailing list<br>
>> <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
>> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</div></div></blockquote></div><br></div></div>