[Mesa-dev] i965 implementation of the ARB_shader_image_load_store built-ins. (v2)

[Matt Turner]

On Tue, May 5, 2015 at 2:17 PM, Francisco Jerez <currojerez at riseup.net> wrote:
> Kenneth Graunke <kenneth at whitecape.org> writes:
>> I like the idea of the builder refactor - having a mechanism for emitting
>> code at arbitrary points makes a ton of sense.  But I'm uncomfortable
>> with how much code is added - and duplicated from the existing visitor
>> code - given that fs_visitor is refactored to use it.
>>
>> I agree with Jason that we should discuss our goal - what things should
>> look like - but frankly, I think we should hold off on large scale
>> refactoring until after we delete the GLSL IR backend.  There will be
>> much less code to reorganize.
>>
> I agree, I wasn't planning on migrating the rest of the compiler
> back-end to the builder framework until that happens.

We talked about this on IRC yesterday -- I find it difficult to review
some of these patches adding large amounts of new infrastructure
that's only used in other in this series.

Basically, the problem is that you're adding (1) a large amount of new
infrastructure, and (2) a large amount of tiling calculation/format
conversion code and neither can be independently evaluated. If the new
infrastructure was proposed and we could see how it was useful in the
existing compiler, we could evaluate it. If the new tiling
calculation/format conversion code used existing compiler
infrastructure, we could evaluate it.

Combining the two in the same series though makes it difficult. It
seems like we'd be buying into a transition to the new builder without
having seen what the transition looks like ("[switching to the new
infrastructure] will be undertaken by a separate series, as it
involves major back-end surgery").

That said, I think people are okay with the builder, in principle at
least. Splitting the visitor classes certainly seems like a good idea.
In an ideal world, we'd transition the backend over to your new
builder, and then add the new image_load_store bits on top of that.
Since we want to delete brw_*_visitor.cpp before we transition to your
new builder and we're not planning to delete brw_*_visitor.cpp until
after 10.6, then this can't happen in 10.6 unless we change something.

>> It may also make sense to land Skylake support first - I believe we can
>> use typed surface messages in most places, which is a lot easier.  We
>> could land that, then add the additional complexity to support BDW/HSW.

I'd like to reiterate this point. The platform we have to support this
extension on is Gen9, and Gen9's typed messages support nearly all of
the formats required.

If we enable Gen9, we'll have more time to figure out how to do the
rest of the infrastructure changes. Is doing Gen9 first feasible?