Matt Turner <matts...@gmail.com> writes:
>> +static fs_reg
>> +get_num_samples_reg(fs_visitor *v)
>> +{
>> + struct gl_program_parameter_list *params = v->prog->Parameters;
>> + static gl_state_index tokens[STATE_LENGTH] = {
>
> I suspect this isn't thread-safe.
Do you mean because the tokens array is static? I don't think this is
written to so I was assuming it would be ok. I should have made it
const. Or did you mean something else?
>> + bld.CMP(bld.null_reg_ud(),
>> + sample_src, i_reg,
>> + BRW_CONDITIONAL_EQ);
>
> I think you might be able to put all of this on one line.
Sadly it doesn't fit. I could probably make it two lines though.
> If you want, you could reverse the order of the loop (that is, iterate
> from num_samples_reg down to 0) and then save the
> opt_cmod_propagation() pass some work and just emit an ADD with a a
> conditional_mod in order to get rid of the CMP instruction.
>
>> + inst = bld.emit(BRW_OPCODE_BREAK);
>> + inst->predicate = BRW_PREDICATE_NORMAL;
>> + bld.emit(BRW_OPCODE_WHILE);
>
> You can actually get rid of the BREAK by predicating the WHILE (and
> setting predicate_inverse on it). There's also a nice
> set_predicate(predicate, inst) function you should use.
>
> If you want to do that, I think it'll take a small amount of work in
> the BRW_OPCODE_WHILE case in brw_cfg.cpp to understand that a
> predicated WHILE has two successors.
That sounds like a good idea. I still have more to learn about the ISA
so I guess this is a good way to do it :)
>> +static bool
>> +find_interpolate_at_sample_in_block(nir_block *block,
>> + void *data)
>> +{
>> + nir_foreach_instr_safe(block, instr) {
>
> I don't think you need _safe here. At least I can't see why it's
> necessary.
Good point, I don't know why I put _safe there.
Thanks for looking at this.
Regards,
- Neil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
