On Mon, Jul 3, 2017 at 8:51 AM, Nicolai Hähnle <nhaeh...@gmail.com> wrote:
> On 03.07.2017 02:50, Timothy Arceri wrote:
>>
>> On 03/07/17 10:46, Timothy Arceri wrote:
>>>
>>> This and the previous patch seem a bit hacky to me. It seems it would be
>>> better to either plum the system value all the way through NIR for all
>>> drivers or simply disable GLSLFragCoordIsSysVal when radeonsi is going to be
>>> using the NIR path.
>>
>>
>> Well maybe not disable, but surely we could ignore it.
>
>
> I agree that it's a bit hacky, but having FragCoord as a system value is
> cleaner more logical at least in radeonsi. Since this isn't necessarily true
> for all drivers, I don't think it makes sense to do the "plumbing through
> for all drivers".
>
> Though I could change this patch to add a load_frag_coord NIR intrinsic
> instead, that would be slightly cleaner.
Yeah, I don't know much about this, but if it's cleaner for radeonsi
(and presumably radv too?) for it to be a system value, then sure, add
the plumbing to make it a proper system value in NIR. Of course, other
drivers don't have to make it a system value though.
>
> Cheers,
> Nicolai
>
>
>
>>
>>>
>>> On 27/06/17 00:09, Nicolai Hähnle wrote:
>>>>
>>>> From: Nicolai Hähnle <nicolai.haeh...@amd.com>
>>>>
>>>> Arguably this could convert to a load intrinsic, but other code paths
>>>> already expect this as a fragment shader input.
>>>> ---
>>>> src/compiler/nir/nir_lower_system_values.c | 36
>>>> ++++++++++++++++++++++++++----
>>>> 1 file changed, 32 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/src/compiler/nir/nir_lower_system_values.c
>>>> b/src/compiler/nir/nir_lower_system_values.c
>>>> index 810100a..64a1354 100644
>>>> --- a/src/compiler/nir/nir_lower_system_values.c
>>>> +++ b/src/compiler/nir/nir_lower_system_values.c
>>>> @@ -21,22 +21,46 @@
>>>> * IN THE SOFTWARE.
>>>> *
>>>> * Authors:
>>>> * Connor Abbott (cwabbo...@gmail.com)
>>>> *
>>>> */
>>>> #include "nir.h"
>>>> #include "nir_builder.h"
>>>> +static nir_ssa_def *
>>>> +get_frag_coord(nir_builder *b, nir_shader *shader, nir_variable *orig)
>>>> +{
>>>> + nir_variable *pos = NULL;
>>>> +
>>>> + assert(shader->stage == MESA_SHADER_FRAGMENT);
>>>> +
>>>> + nir_foreach_variable(var, &shader->inputs) {
>>>> + if (var->data.location == VARYING_SLOT_POS) {
>>>> + pos = var;
>>>> + break;
>>>> + }
>>>> + }
>>>> +
>>>> + if (!pos) {
>>>> + pos = nir_variable_create(shader, nir_var_shader_in,
>>>> glsl_vec4_type(), "gl_FragCoord");
>>>> + pos->data.location = VARYING_SLOT_POS;
>>>> + pos->data.pixel_center_integer = orig->data.pixel_center_integer;
>>>> + pos->data.origin_upper_left = orig->data.origin_upper_left;
>>>> + }
>>>> +
>>>> + return nir_load_var(b, pos);
>>>> +}
>>>> +
>>>> static bool
>>>> -convert_block(nir_block *block, nir_builder *b)
>>>> +convert_block(nir_block *block, nir_builder *b, nir_shader *shader)
>>>> {
>>>> bool progress = false;
>>>> nir_foreach_instr_safe(instr, block) {
>>>> if (instr->type != nir_instr_type_intrinsic)
>>>> continue;
>>>> nir_intrinsic_instr *load_var = nir_instr_as_intrinsic(instr);
>>>> if (load_var->intrinsic != nir_intrinsic_load_var)
>>>> @@ -109,59 +133,63 @@ convert_block(nir_block *block, nir_builder *b)
>>>> sysval = nir_load_vertex_id(b);
>>>> }
>>>> break;
>>>> case SYSTEM_VALUE_INSTANCE_INDEX:
>>>> sysval = nir_iadd(b,
>>>> nir_load_instance_id(b),
>>>> nir_load_base_instance(b));
>>>> break;
>>>> + case SYSTEM_VALUE_FRAG_COORD:
>>>> + sysval = get_frag_coord(b, shader, var);
>>>> + break;
>>>> +
>>>> default:
>>>> break;
>>>> }
>>>> if (sysval == NULL) {
>>>> nir_intrinsic_op sysval_op =
>>>> nir_intrinsic_from_system_value(var->data.location);
>>>> sysval = nir_load_system_value(b, sysval_op, 0);
>>>> }
>>>> nir_ssa_def_rewrite_uses(&load_var->dest.ssa,
>>>> nir_src_for_ssa(sysval));
>>>> nir_instr_remove(&load_var->instr);
>>>> progress = true;
>>>> }
>>>> return progress;
>>>> }
>>>> static bool
>>>> -convert_impl(nir_function_impl *impl)
>>>> +convert_impl(nir_function_impl *impl, nir_shader *shader)
>>>> {
>>>> bool progress = false;
>>>> nir_builder builder;
>>>> nir_builder_init(&builder, impl);
>>>> nir_foreach_block(block, impl) {
>>>> - progress |= convert_block(block, &builder);
>>>> + progress |= convert_block(block, &builder, shader);
>>>> }
>>>> nir_metadata_preserve(impl, nir_metadata_block_index |
>>>> nir_metadata_dominance);
>>>> return progress;
>>>> }
>>>> bool
>>>> nir_lower_system_values(nir_shader *shader)
>>>> {
>>>> bool progress = false;
>>>> nir_foreach_function(function, shader) {
>>>> if (function->impl)
>>>> - progress = convert_impl(function->impl) || progress;
>>>> + progress = convert_impl(function->impl, shader) || progress;
>>>> }
>>>> exec_list_make_empty(&shader->system_values);
>>>> return progress;
>>>> }
>>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
>
> --
> Lerne, wie die Welt wirklich ist,
> Aber vergiss niemals, wie sie sein sollte.
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
