[Mesa-dev] [PATCH 00/11] i965/nir: Do texture rectangle lowering in NIR
Jason Ekstrand
jason at jlekstrand.net
Sat Nov 14 10:00:38 PST 2015
On Sat, Nov 14, 2015 at 9:44 AM, Rob Clark <robdclark at gmail.com> wrote:
> On Sat, Nov 14, 2015 at 12:30 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>> On Sat, Nov 14, 2015 at 8:58 AM, Rob Clark <robdclark at gmail.com> wrote:
>>> On Sat, Nov 14, 2015 at 11:01 AM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>>>> On Thu, Nov 12, 2015 at 7:30 AM, Iago Toral <itoral at igalia.com> wrote:
>>>>> On Thu, 2015-11-12 at 16:23 +0100, Iago Toral wrote:
>>>>>> Patches 1-4 are,
>>>>>> Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>
>>>>>>
>>>>>> Patch 5 seems to be missing.
>>>>
>>>> If it helps to calm reviewer's minds, I ran patches 1-5 with this patch on top:
>>>>
>>>> http://cgit.freedesktop.org/~jekstrand/mesa/commit/?h=wip/nir-clone
>>>>
>>>> Zero regressions in piglit, dEQP, and the CTS.
>>>
>>> imho, please push something like this to master, w/ perhaps an env-var
>>> switch (ofc just for debug builds).. this way we can work nir_clone
>>> testing into normal CI test cycle, and protect against future
>>> difficult-to-track-down breakage
>>
>> I thought about doing that but it didn't really work very well with
>> patch 6. Also, by the time we get to patch 7, it's getting tested
>> pretty well. About the only thing that doesn't get tested there is
>> registers. I'm not opposed to adding support for testing it in CI,
>> but I don't want to dirty up an API to do so if it can be avoided.
>> Would you be ok with cloning in a few key places?
>
> Well, I prefer testing between each stage.. it's a little brute-force,
> but it ensures we don't miss something that only appears between
> certain stages, now or in the future. The few-key-places approach is
> certainly better than nothing.
>
> I guess the 'dirty up an API' bit referred to returning nir_shader's?
> I don't think that is *that* horrible a price to pay..
I'm more concerned about the fact that you get out a pointer that may
or may not be the one you passed in and we may or may not have deleted
the one you passed in and, to make things better, if you accidentally
ignore the return value it will work fine unless INTEL_NIR_CLONE is
enabled. I think we're going to find ourselves breaking the nir_clone
testing code more often than breaking nir_clone.
--Jason
> BR,
> -R
>
>> --Jason
>>
>>> (and you can even pre-emptively slap my r-b on that, since I'm happy
>>> however that is accomplished..)
>>>
>>> BR,
>>> -R
>>>
>>>>> Oh never mind, I've just seen your reply to the thread pointing to the
>>>>> repository.
>>>>>
>>>>> Iago
>>>>>
>>>>>> Iago
>>>>>>
>>>>>> On Wed, 2015-11-11 at 17:23 -0800, Jason Ekstrand wrote:
>>>>>> > On older hardware (Iron Lake and below), we can't support texture rectangle
>>>>>> > natively. Sandy Bridge through Haswell can support it but don't support
>>>>>> > the GL_CLAMP wrap mode natively. It isn't until Broadwell that GL_CLAMP is
>>>>>> > supported together with GL_TEXTURE_RECTANGLE in hardware. In the cases
>>>>>> > where it isn't supported, we have to fake it by dividing by the texture
>>>>>> > size.
>>>>>> >
>>>>>> > Previously, we had a rescale_texcoord function added a uniform to hold the
>>>>>> > texture coordinate and used that to rescale/clamp the texture coordinates.
>>>>>> > For a while now, nir_lower_tex has been able to lower texture rectangle to
>>>>>> > a textureSize and a regular texture2D operation. This series makes i965
>>>>>> > use the nir_lower_tex path instead. Incidentally, this fixes texture
>>>>>> > rectangle support in vertex and geometry shaders on Haswell and below.
>>>>>> > (The backend lowering was only ever done in the FS backend.)
>>>>>> >
>>>>>> > Since this is the first time we're doing any sort of shader variants in
>>>>>> > NIR, the first several passes add the infastructure to do so. Two of these
>>>>>> > patches are from Ken, two are from Rob, and one (nir_clone itself) is my
>>>>>> > rendition but heavily based on what Rob did only with less hashing.
>>>>>> >
>>>>>> > Jason Ekstrand (7):
>>>>>> > nir: support to clone shaders
>>>>>> > i965/nir: Split shader optimization and lowering into three satages
>>>>>> > i965: Move postprocess_nir to codegen time
>>>>>> > nir/lower_tex: Report progress
>>>>>> > nir/lower_tex: Set the dest_type for txs instructions
>>>>>> > i965/fs: Don't allow SINT32 as a return type for resinfo
>>>>>> > i965: Use nir_lower_tex for texture coordinate lowering
>>>>>> >
>>>>>> > Kenneth Graunke (2):
>>>>>> > i965/nir: Add OPT() and OPT_V() macros for invoking NIR passes.
>>>>>> > i965/nir: Validate that NIR passes call nir_metadata_preserve().
>>>>>> >
>>>>>> > Rob Clark (2):
>>>>>> > nir: remove nir_variable::max_ifc_array_access
>>>>>> > nir: add array length field
>>>>>> >
>>>>>> > src/glsl/Makefile.sources | 1 +
>>>>>> > src/glsl/nir/glsl_to_nir.cpp | 14 +-
>>>>>> > src/glsl/nir/nir.c | 8 +
>>>>>> > src/glsl/nir/nir.h | 27 +-
>>>>>> > src/glsl/nir/nir_clone.c | 671 ++++++++++++++++++++++
>>>>>> > src/glsl/nir/nir_lower_tex.c | 20 +-
>>>>>> > src/glsl/nir/nir_metadata.c | 36 ++
>>>>>> > src/mesa/drivers/dri/i965/brw_fs.cpp | 13 +-
>>>>>> > src/mesa/drivers/dri/i965/brw_fs.h | 3 -
>>>>>> > src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 10 +-
>>>>>> > src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 4 +-
>>>>>> > src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 125 ----
>>>>>> > src/mesa/drivers/dri/i965/brw_nir.c | 268 +++++----
>>>>>> > src/mesa/drivers/dri/i965/brw_nir.h | 15 +
>>>>>> > src/mesa/drivers/dri/i965/brw_vec4.cpp | 7 +-
>>>>>> > src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp | 8 +-
>>>>>> > 16 files changed, 966 insertions(+), 264 deletions(-)
>>>>>> > create mode 100644 src/glsl/nir/nir_clone.c
>>>>>> >
>>>>>>
>>>>>
>>>>>
>>>> _______________________________________________
>>>> mesa-dev mailing list
>>>> mesa-dev at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list