On Mon, Aug 24, 2015 at 4:02 PM, Kenneth Graunke <kenn...@whitecape.org> wrote:
> On Wednesday, August 19, 2015 10:45:43 PM Jason Ekstrand wrote:
>> In the i965 backend, we want to be able to "pull apart" the uniforms and
>> push some of them into the shader through a different path. In order to do
>> this effectively, we need to know which variable is actually being referred
>> to by a given uniform load. Previously, it was completely flattened by
>> nir_lower_io which made things difficult. This adds more information to
>> the intrinsic to make this easier for us.
>> ---
>> src/glsl/nir/nir_intrinsics.h | 28 +++++++++++++++++-----------
>> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 2 +-
>> src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 2 +-
>> 3 files changed, 19 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/glsl/nir/nir_intrinsics.h b/src/glsl/nir/nir_intrinsics.h
>> index bc6e6b8..5aa12fa 100644
>> --- a/src/glsl/nir/nir_intrinsics.h
>> +++ b/src/glsl/nir/nir_intrinsics.h
>> @@ -139,11 +139,17 @@ SYSTEM_VALUE(sample_mask_in, 1)
>> SYSTEM_VALUE(invocation_id, 1)
>>
>> /*
>> - * The first and only index is the base address to load from. Indirect
>> - * loads have an additional register input, which is added to the constant
>> - * address to compute the final address to load from. For UBO's (and
>> - * SSBO's), the first source is the (possibly constant) UBO buffer index
>> - * and the indirect (if it exists) is the second source.
>> + * The format of the indices depends on the type of the load. For uniforms,
>> + * the first index is the base address and the second index is an offset
>> that
>> + * should be added to the base address. (This way you can determine in the
>> + * back-end which variable is being accessed even in an array.) For inputs,
>> + * the one and only index corresponds to the attribute slot. UBO loads also
>
> Please drop the new sentence about inputs. Attribute slots only make
> sense for vertex shader inputs, not inputs in general...and it's not
> entirely related to this change.
Part of this change was to actually enumerate all of the possibilities
as opposed to describing them in general. I have to say *something*
about inputs. I guess I could say something about attribute slot or
input offset. Suggestions welcome.
> So, it sounds like TGSI-to-NIR simply ignores the second index and keeps
> working as usual? I suppose that works - it shouldn't break anything,
> at least. Eric and Rob can change it if they want.
Yeah, they should default to 0 and be completely ignorable.
--Jason
>> + * have a single index which is the base address to load from.
>> + *
>> + * UBO loads have a (possibly constant) source which is the UBO buffer
>> index.
>> + * For each type of load, the _indirect variant has one additional source
>> + * (the second in the case of UBO's) that is the is an indirect to be added
>> to
>> + * the constant address or base offset to compute the final offset.
>> *
>> * For vector backends, the address is in terms of one vec4, and so each
>> array
>> * element is +4 scalar components from the previous array element. For
>> scalar
>> @@ -151,14 +157,14 @@ SYSTEM_VALUE(invocation_id, 1)
>> * elements begin immediately after the previous array element.
>> */
>>
>> -#define LOAD(name, extra_srcs, flags) \
>> - INTRINSIC(load_##name, extra_srcs, ARR(1), true, 0, 0, 1, flags) \
>> +#define LOAD(name, extra_srcs, indices, flags) \
>> + INTRINSIC(load_##name, extra_srcs, ARR(1), true, 0, 0, indices, flags) \
>> INTRINSIC(load_##name##_indirect, extra_srcs + 1, ARR(1, 1), \
>> - true, 0, 0, 1, flags)
>> + true, 0, 0, indices, flags)
>>
>> -LOAD(uniform, 0, NIR_INTRINSIC_CAN_ELIMINATE | NIR_INTRINSIC_CAN_REORDER)
>> -LOAD(ubo, 1, NIR_INTRINSIC_CAN_ELIMINATE | NIR_INTRINSIC_CAN_REORDER)
>> -LOAD(input, 0, NIR_INTRINSIC_CAN_ELIMINATE | NIR_INTRINSIC_CAN_REORDER)
>> +LOAD(uniform, 0, 2, NIR_INTRINSIC_CAN_ELIMINATE | NIR_INTRINSIC_CAN_REORDER)
>> +LOAD(ubo, 1, 1, NIR_INTRINSIC_CAN_ELIMINATE | NIR_INTRINSIC_CAN_REORDER)
>> +LOAD(input, 0, 1, NIR_INTRINSIC_CAN_ELIMINATE | NIR_INTRINSIC_CAN_REORDER)
>> /* LOAD(ssbo, 1, 0) */
>>
>> /*
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> index 354c5ee..5b54b95 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> @@ -1470,7 +1470,7 @@ fs_visitor::nir_emit_intrinsic(const fs_builder &bld,
>> nir_intrinsic_instr *instr
>> has_indirect = true;
>> /* fallthrough */
>> case nir_intrinsic_load_uniform: {
>> - unsigned index = instr->const_index[0];
>> + unsigned index = instr->const_index[0] + instr->const_index[1];
>>
>> fs_reg uniform_reg;
>> if (index < num_direct_uniforms) {
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> index 4f689df..9e52229 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> @@ -570,7 +570,7 @@ vec4_visitor::nir_emit_intrinsic(nir_intrinsic_instr
>> *instr)
>> has_indirect = true;
>> /* fallthrough */
>> case nir_intrinsic_load_uniform: {
>> - int uniform = instr->const_index[0];
>> + int uniform = instr->const_index[0] + instr->const_index[1];
>>
>> dest = get_nir_dest(instr->dest);
>>
>>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
