[Mesa-dev] [PATCH 01/10] gallivm: supply correct opcode info to emit functions
sroland at vmware.com
Sun Oct 11 14:15:05 PDT 2015
Am 11.10.2015 um 12:39 schrieb Marek Olšák:
> On Sun, Oct 11, 2015 at 4:22 AM, Roland Scheidegger <sroland at vmware.com> wrote:
>> Am 11.10.2015 um 03:29 schrieb Marek Olšák:
>>> From: Marek Olšák <marek.olsak at amd.com>
>>> This is useful only when emit functions use it.
>>> The new radeonsi min/max opcode implementation requires this.
>>> src/gallium/auxiliary/gallivm/lp_bld_tgsi.c | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c b/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c
>>> index c4ae304..c50d83e 100644
>>> --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c
>>> +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c
>>> @@ -114,12 +114,17 @@ lp_build_emit_llvm(
>>> struct lp_build_emit_data * emit_data)
>>> struct lp_build_tgsi_action * action = &bld_base->op_actions[tgsi_opcode];
>>> + const struct tgsi_opcode_info *old_info = emit_data->info;
>>> /* XXX: Assert that this is a componentwise or replicate instruction */
>>> lp_build_action_set_dst_type(emit_data, bld_base, tgsi_opcode);
>>> emit_data->chan = 0;
>>> + /* Set and restore the opcode info. */
>>> + emit_data->info = tgsi_get_opcode_info(tgsi_opcode);
>>> action->emit(action, bld_base, emit_data);
>>> + emit_data->info = old_info;
>>> return emit_data->output;
>> Could you elaborate why this is necessary? Looks like a hack and I can't
>> see why opcode info would be wrong in the first place. Or if that's
>> never set correctly and you just need it to be able to distinguish
>> min/max later, I'd suggest you shouldn't do that and just use different
> lp_build_emit_llvm_binary and friends don't set the opcode info at all
> and they also don't set the instruction info. This means the emit
> function has no way to tell which opcode is being processed if that
> emit function is used for multiple opcodes.
So why do you need to set the info back after action->emit? If you want
to set that always so that information can be used, looks fine to me.
But if you have to set it back afterwards that screams hack (and I see
no reason for such a hack as you could easily avoid it by just using
different emit actions).
More information about the mesa-dev