Jordan Justen <jordan.l.jus...@intel.com> writes:

> On 2015-10-13 22:49:08, Iago Toral wrote:
>> On Tue, 2015-10-13 at 09:44 -0700, Jordan Justen wrote:
>> > On 2015-10-13 05:17:37, Francisco Jerez wrote:
>> > > Iago Toral Quiroga <ito...@igalia.com> writes:
>> > > 
>> > > > This fixes the following test:
>> > > >
>> > > > [require]
>> > > > GL >= 3.3
>> > > > GLSL >= 3.30
>> > > > GL_ARB_shader_storage_buffer_object
>> > > >
>> > > > [fragment shader]
>> > > > #version 330
>> > > > #extension GL_ARB_shader_storage_buffer_object: require
>> > > >
>> > > > buffer SSBO {
>> > > >     mat4 sm4;
>> > > > };
>> > > >
>> > > >
>> > > > mat4 um4;
>> > > >
>> > > > void main() {
>> > > >     sm4 *= um4;
>> > > 
>> > > This is using the value of "um4", which is never assigned, so liveness
>> > > analysis will correctly extend its live interval up to the start of the
>> > > block.
>> > 
>> > This test was derived by simplifying a CTS test case.
>> > 
>> > Anyway, I'm not sure what happened on the way to the commit message,
>> > but um4 should be a uniform.
>> > 
>> > http://sprunge.us/cEUe
>> 
>> Oh yes, that was me playing around with the example. The patches also
>> fix the uniform version. Jordan, can you verify if this fixes the CTS
>> test case?
>
> Unfortunately, no. The CTS case has some control flow. I had removed
> it to minimize the test case.
>
> Here is a small shader_test that has control flow and still fails to
> compile with your patches:
>
> http://sprunge.us/LIjA
>
>> In any case, since Curro is working on a more general fix for this
>> stuff, I guess you'd rather wait for his patches...
>
> It depends how long we'd have to wait. :) Anyway, since we don't have
> a short-term fix anyhow, let's wait to see what curro has to say.
>
Assuming that at least some of the scalar writes in the shader are being
introduced by emit_uniformize(), an alternative hack that might get the
shader to compile for the moment would be to change emit_uniformize() to
emit a full SIMD-width BROADCAST instruction instead of a scalar one
(see attachment) -- Which is pretty useless in principle because only
the first component will ever be used but it might keep dataflow
analysis from getting confused.

> -Jordan

diff --git a/src/mesa/drivers/dri/i965/brw_fs_builder.h b/src/mesa/drivers/dri/i965/brw_fs_builder.h
index df10a9d..5db0acf 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_builder.h
+++ b/src/mesa/drivers/dri/i965/brw_fs_builder.h
@@ -392,12 +392,12 @@ namespace brw {
       {
          const fs_builder ubld = exec_all();
          const dst_reg chan_index = component(vgrf(BRW_REGISTER_TYPE_UD), 0);
-         const dst_reg dst = component(vgrf(src.type), 0);
+         const dst_reg dst = vgrf(src.type);
 
          ubld.emit(SHADER_OPCODE_FIND_LIVE_CHANNEL, chan_index);
          ubld.emit(SHADER_OPCODE_BROADCAST, dst, src, chan_index);
 
-         return src_reg(dst);
+         return component(src_reg(dst), 0);
       }
 
       /**

Attachment: signature.asc
Description: PGP signature

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to