Re: [Mesa-dev] [PATCH 4/4] intel/compiler: Add scheduler deps for instructions that implicitly read g0

[Mark Janes]

Ian Romanick  writes:

> On 04/23/2018 09:19 AM, Mark Janes wrote:
>> I enabled these tests, and could not make them fail in CI.
>
> Can I consider that a Tested-by?

Sorry, I should have been more explicit.  Yes, tested-by.

>> This bug may also be related:
>> 
>> https://bugs.freedesktop.org/show_bug.cgi?id=95009
>
> If none of these tests fail with this patch, I'm included to close them
> with a note to re-open if the failures return.
>
> I also plan to nominate this for stable.  I forgot to do that when I
> sent the patch originally.
>
>> Ian Romanick  writes:
>> 
>>> From: Ian Romanick 
>>>
>>> Otherwise the scheduler can move the writes after the reads.
>>>
>>> Signed-off-by: Ian Romanick 
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95012
>>> Cc: Mark Janes 
>>> Cc: Clayton A Craft 
>>> ---
>>> I'd really like to have this run on the CI with the tests from bugzill
>>> #95012 re-enabled.  I don't know how to do this on my own.  This series
>>> is also available at:
>>>
>>> https://cgit.freedesktop.org/~idr/mesa/log/?h=bug-95012.
>>>
>>>  src/intel/compiler/brw_ir_vec4.h | 25 
>>> 
>>>  src/intel/compiler/brw_schedule_instructions.cpp |  3 +++
>>>  2 files changed, 28 insertions(+)
>>>
>>> diff --git a/src/intel/compiler/brw_ir_vec4.h 
>>> b/src/intel/compiler/brw_ir_vec4.h
>>> index 95c5119c6c0..e401d8b4d16 100644
>>> --- a/src/intel/compiler/brw_ir_vec4.h
>>> +++ b/src/intel/compiler/brw_ir_vec4.h
>>> @@ -334,6 +334,31 @@ public:
>>>opcode != BRW_OPCODE_IF &&
>>>opcode != BRW_OPCODE_WHILE));
>>> }
>>> +
>>> +   bool reads_g0_implicitly() const
>>> +   {
>>> +  switch (opcode) {
>>> +  case SHADER_OPCODE_TEX:
>>> +  case SHADER_OPCODE_TXL:
>>> +  case SHADER_OPCODE_TXD:
>>> +  case SHADER_OPCODE_TXF:
>>> +  case SHADER_OPCODE_TXF_CMS_W:
>>> +  case SHADER_OPCODE_TXF_CMS:
>>> +  case SHADER_OPCODE_TXF_MCS:
>>> +  case SHADER_OPCODE_TXS:
>>> +  case SHADER_OPCODE_TG4:
>>> +  case SHADER_OPCODE_TG4_OFFSET:
>>> +  case SHADER_OPCODE_SAMPLEINFO:
>>> +  case VS_OPCODE_PULL_CONSTANT_LOAD:
>>> +  case GS_OPCODE_SET_PRIMITIVE_ID:
>>> +  case GS_OPCODE_GET_INSTANCE_ID:
>>> +  case SHADER_OPCODE_GEN4_SCRATCH_READ:
>>> +  case SHADER_OPCODE_GEN4_SCRATCH_WRITE:
>>> + return true;
>>> +  default:
>>> + return false;
>>> +  }
>>> +   }
>>>  };
>>>  
>>>  /**
>>> diff --git a/src/intel/compiler/brw_schedule_instructions.cpp 
>>> b/src/intel/compiler/brw_schedule_instructions.cpp
>>> index fa85045de74..f817142a8b5 100644
>>> --- a/src/intel/compiler/brw_schedule_instructions.cpp
>>> +++ b/src/intel/compiler/brw_schedule_instructions.cpp
>>> @@ -1267,6 +1267,9 @@ vec4_instruction_scheduler::calculate_deps()
>>>   }
>>>}
>>>  
>>> +  if (inst->reads_g0_implicitly())
>>> + add_dep(last_fixed_grf_write, n);
>>> +
>>>if (!inst->is_send_from_grf()) {
>>>   for (int i = 0; i < inst->mlen; i++) {
>>>  /* It looks like the MRF regs are released in the send
>>> -- 
>>> 2.14.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/4] intel/compiler: Add scheduler deps for instructions that implicitly read g0

[Ian Romanick]

On 04/23/2018 09:19 AM, Mark Janes wrote:
> I enabled these tests, and could not make them fail in CI.

Can I consider that a Tested-by?

> This bug may also be related:
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=95009

If none of these tests fail with this patch, I'm included to close them
with a note to re-open if the failures return.

I also plan to nominate this for stable.  I forgot to do that when I
sent the patch originally.

> Ian Romanick  writes:
> 
>> From: Ian Romanick 
>>
>> Otherwise the scheduler can move the writes after the reads.
>>
>> Signed-off-by: Ian Romanick 
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95012
>> Cc: Mark Janes 
>> Cc: Clayton A Craft 
>> ---
>> I'd really like to have this run on the CI with the tests from bugzill
>> #95012 re-enabled.  I don't know how to do this on my own.  This series
>> is also available at:
>>
>> https://cgit.freedesktop.org/~idr/mesa/log/?h=bug-95012.
>>
>>  src/intel/compiler/brw_ir_vec4.h | 25 
>> 
>>  src/intel/compiler/brw_schedule_instructions.cpp |  3 +++
>>  2 files changed, 28 insertions(+)
>>
>> diff --git a/src/intel/compiler/brw_ir_vec4.h 
>> b/src/intel/compiler/brw_ir_vec4.h
>> index 95c5119c6c0..e401d8b4d16 100644
>> --- a/src/intel/compiler/brw_ir_vec4.h
>> +++ b/src/intel/compiler/brw_ir_vec4.h
>> @@ -334,6 +334,31 @@ public:
>>opcode != BRW_OPCODE_IF &&
>>opcode != BRW_OPCODE_WHILE));
>> }
>> +
>> +   bool reads_g0_implicitly() const
>> +   {
>> +  switch (opcode) {
>> +  case SHADER_OPCODE_TEX:
>> +  case SHADER_OPCODE_TXL:
>> +  case SHADER_OPCODE_TXD:
>> +  case SHADER_OPCODE_TXF:
>> +  case SHADER_OPCODE_TXF_CMS_W:
>> +  case SHADER_OPCODE_TXF_CMS:
>> +  case SHADER_OPCODE_TXF_MCS:
>> +  case SHADER_OPCODE_TXS:
>> +  case SHADER_OPCODE_TG4:
>> +  case SHADER_OPCODE_TG4_OFFSET:
>> +  case SHADER_OPCODE_SAMPLEINFO:
>> +  case VS_OPCODE_PULL_CONSTANT_LOAD:
>> +  case GS_OPCODE_SET_PRIMITIVE_ID:
>> +  case GS_OPCODE_GET_INSTANCE_ID:
>> +  case SHADER_OPCODE_GEN4_SCRATCH_READ:
>> +  case SHADER_OPCODE_GEN4_SCRATCH_WRITE:
>> + return true;
>> +  default:
>> + return false;
>> +  }
>> +   }
>>  };
>>  
>>  /**
>> diff --git a/src/intel/compiler/brw_schedule_instructions.cpp 
>> b/src/intel/compiler/brw_schedule_instructions.cpp
>> index fa85045de74..f817142a8b5 100644
>> --- a/src/intel/compiler/brw_schedule_instructions.cpp
>> +++ b/src/intel/compiler/brw_schedule_instructions.cpp
>> @@ -1267,6 +1267,9 @@ vec4_instruction_scheduler::calculate_deps()
>>   }
>>}
>>  
>> +  if (inst->reads_g0_implicitly())
>> + add_dep(last_fixed_grf_write, n);
>> +
>>if (!inst->is_send_from_grf()) {
>>   for (int i = 0; i < inst->mlen; i++) {
>>  /* It looks like the MRF regs are released in the send
>> -- 
>> 2.14.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/4] intel/compiler: Add scheduler deps for instructions that implicitly read g0

[Mark Janes]

I enabled these tests, and could not make them fail in CI.

This bug may also be related:

https://bugs.freedesktop.org/show_bug.cgi?id=95009

Ian Romanick  writes:

> From: Ian Romanick 
>
> Otherwise the scheduler can move the writes after the reads.
>
> Signed-off-by: Ian Romanick 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95012
> Cc: Mark Janes 
> Cc: Clayton A Craft 
> ---
> I'd really like to have this run on the CI with the tests from bugzill
> #95012 re-enabled.  I don't know how to do this on my own.  This series
> is also available at:
>
> https://cgit.freedesktop.org/~idr/mesa/log/?h=bug-95012.
>
>  src/intel/compiler/brw_ir_vec4.h | 25 
> 
>  src/intel/compiler/brw_schedule_instructions.cpp |  3 +++
>  2 files changed, 28 insertions(+)
>
> diff --git a/src/intel/compiler/brw_ir_vec4.h 
> b/src/intel/compiler/brw_ir_vec4.h
> index 95c5119c6c0..e401d8b4d16 100644
> --- a/src/intel/compiler/brw_ir_vec4.h
> +++ b/src/intel/compiler/brw_ir_vec4.h
> @@ -334,6 +334,31 @@ public:
>opcode != BRW_OPCODE_IF &&
>opcode != BRW_OPCODE_WHILE));
> }
> +
> +   bool reads_g0_implicitly() const
> +   {
> +  switch (opcode) {
> +  case SHADER_OPCODE_TEX:
> +  case SHADER_OPCODE_TXL:
> +  case SHADER_OPCODE_TXD:
> +  case SHADER_OPCODE_TXF:
> +  case SHADER_OPCODE_TXF_CMS_W:
> +  case SHADER_OPCODE_TXF_CMS:
> +  case SHADER_OPCODE_TXF_MCS:
> +  case SHADER_OPCODE_TXS:
> +  case SHADER_OPCODE_TG4:
> +  case SHADER_OPCODE_TG4_OFFSET:
> +  case SHADER_OPCODE_SAMPLEINFO:
> +  case VS_OPCODE_PULL_CONSTANT_LOAD:
> +  case GS_OPCODE_SET_PRIMITIVE_ID:
> +  case GS_OPCODE_GET_INSTANCE_ID:
> +  case SHADER_OPCODE_GEN4_SCRATCH_READ:
> +  case SHADER_OPCODE_GEN4_SCRATCH_WRITE:
> + return true;
> +  default:
> + return false;
> +  }
> +   }
>  };
>  
>  /**
> diff --git a/src/intel/compiler/brw_schedule_instructions.cpp 
> b/src/intel/compiler/brw_schedule_instructions.cpp
> index fa85045de74..f817142a8b5 100644
> --- a/src/intel/compiler/brw_schedule_instructions.cpp
> +++ b/src/intel/compiler/brw_schedule_instructions.cpp
> @@ -1267,6 +1267,9 @@ vec4_instruction_scheduler::calculate_deps()
>   }
>}
>  
> +  if (inst->reads_g0_implicitly())
> + add_dep(last_fixed_grf_write, n);
> +
>if (!inst->is_send_from_grf()) {
>   for (int i = 0; i < inst->mlen; i++) {
>  /* It looks like the MRF regs are released in the send
> -- 
> 2.14.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/4] intel/compiler: Add scheduler deps for instructions that implicitly read g0

[Jason Ekstrand]

If Ken's good with duct tape then so am I.  Rb.

On April 20, 2018 17:54:00 Kenneth Graunke  wrote:


On Friday, April 20, 2018 5:01:17 PM PDT Jason Ekstrand wrote:
> On Fri, Apr 20, 2018 at 4:30 PM, Ian Romanick  wrote:
>
> > On 04/20/2018 11:56 AM, Jason Ekstrand wrote:
> > > Ugh... I was actually thinking about this the other day.  How did you
> > > come up with your list of instructions?  Is there some algorithmic way
> > > to determine it such as sends with no header?
> >
> > I just looked through the generator for pseudo-ops that emit explicit
> > reads of g0.  Since it was a manual process, I may have missed some. :(
> > I don't know of a way to automate this, but... it may be possible.  Not
> > sure.
> >
>
> We ran into a similar issue with Dota 2 on Vulkan which was fixed by
> ff4726077d86800d33520581f154a27dac408588
>
> For vec4, this may be a reasonable solution.  I don't like list of
> instructions in general but I'm a bit more ok with duct tape in vec4.
> Adding matt & Ken in case they have opinions.

Duct tape in vec4 seems good to me.  Patch is:
Reviewed-by: Kenneth Graunke 

In the scalar backend, we're trying to move away from these implicit
things happening in the generator, and just make them visible to the
visitor (like Jason did).

I suspect that just about everything that turns into a SEND message
will need g0 for FFTID - the shared functions need the fixed-function
thread ID to know where to send the return message.  It might be passed
as side-band though and not actually read as part of g0...so we probably
can't do anything that simple...

But as I said, R-b.  I certainly wouldn't advocate spending more time
in vec4 than you have to.




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/4] intel/compiler: Add scheduler deps for instructions that implicitly read g0

[Kenneth Graunke]

On Friday, April 20, 2018 5:01:17 PM PDT Jason Ekstrand wrote:
> On Fri, Apr 20, 2018 at 4:30 PM, Ian Romanick  wrote:
> 
> > On 04/20/2018 11:56 AM, Jason Ekstrand wrote:
> > > Ugh... I was actually thinking about this the other day.  How did you
> > > come up with your list of instructions?  Is there some algorithmic way
> > > to determine it such as sends with no header?
> >
> > I just looked through the generator for pseudo-ops that emit explicit
> > reads of g0.  Since it was a manual process, I may have missed some. :(
> > I don't know of a way to automate this, but... it may be possible.  Not
> > sure.
> >
> 
> We ran into a similar issue with Dota 2 on Vulkan which was fixed by
> ff4726077d86800d33520581f154a27dac408588
> 
> For vec4, this may be a reasonable solution.  I don't like list of
> instructions in general but I'm a bit more ok with duct tape in vec4.
> Adding matt & Ken in case they have opinions.

Duct tape in vec4 seems good to me.  Patch is:
Reviewed-by: Kenneth Graunke 

In the scalar backend, we're trying to move away from these implicit
things happening in the generator, and just make them visible to the
visitor (like Jason did).

I suspect that just about everything that turns into a SEND message
will need g0 for FFTID - the shared functions need the fixed-function
thread ID to know where to send the return message.  It might be passed
as side-band though and not actually read as part of g0...so we probably
can't do anything that simple...

But as I said, R-b.  I certainly wouldn't advocate spending more time
in vec4 than you have to.


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/4] intel/compiler: Add scheduler deps for instructions that implicitly read g0

[Jason Ekstrand]

On Fri, Apr 20, 2018 at 4:30 PM, Ian Romanick  wrote:

> On 04/20/2018 11:56 AM, Jason Ekstrand wrote:
> > Ugh... I was actually thinking about this the other day.  How did you
> > come up with your list of instructions?  Is there some algorithmic way
> > to determine it such as sends with no header?
>
> I just looked through the generator for pseudo-ops that emit explicit
> reads of g0.  Since it was a manual process, I may have missed some. :(
> I don't know of a way to automate this, but... it may be possible.  Not
> sure.
>

We ran into a similar issue with Dota 2 on Vulkan which was fixed by
ff4726077d86800d33520581f154a27dac408588

For vec4, this may be a reasonable solution.  I don't like list of
instructions in general but I'm a bit more ok with duct tape in vec4.
Adding matt & Ken in case they have opinions.


> > On April 20, 2018 08:52:50 "Ian Romanick"  wrote:
> >
> >> From: Ian Romanick 
> >>
> >> Otherwise the scheduler can move the writes after the reads.
> >>
> >> Signed-off-by: Ian Romanick 
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95012
> >> Cc: Mark Janes 
> >> Cc: Clayton A Craft 
> >> ---
> >> I'd really like to have this run on the CI with the tests from bugzill
> >> #95012 re-enabled.  I don't know how to do this on my own.  This series
> >> is also available at:
> >>
> >>https://cgit.freedesktop.org/~idr/mesa/log/?h=bug-95012.
> >>
> >> src/intel/compiler/brw_ir_vec4.h | 25
> >> 
> >> src/intel/compiler/brw_schedule_instructions.cpp |  3 +++
> >> 2 files changed, 28 insertions(+)
> >>
> >> diff --git a/src/intel/compiler/brw_ir_vec4.h
> >> b/src/intel/compiler/brw_ir_vec4.h
> >> index 95c5119c6c0..e401d8b4d16 100644
> >> --- a/src/intel/compiler/brw_ir_vec4.h
> >> +++ b/src/intel/compiler/brw_ir_vec4.h
> >> @@ -334,6 +334,31 @@ public:
> >>   opcode != BRW_OPCODE_IF &&
> >>   opcode != BRW_OPCODE_WHILE));
> >>}
> >> +
> >> +   bool reads_g0_implicitly() const
> >> +   {
> >> +  switch (opcode) {
> >> +  case SHADER_OPCODE_TEX:
> >> +  case SHADER_OPCODE_TXL:
> >> +  case SHADER_OPCODE_TXD:
> >> +  case SHADER_OPCODE_TXF:
> >> +  case SHADER_OPCODE_TXF_CMS_W:
> >> +  case SHADER_OPCODE_TXF_CMS:
> >> +  case SHADER_OPCODE_TXF_MCS:
> >> +  case SHADER_OPCODE_TXS:
> >> +  case SHADER_OPCODE_TG4:
> >> +  case SHADER_OPCODE_TG4_OFFSET:
> >> +  case SHADER_OPCODE_SAMPLEINFO:
> >> +  case VS_OPCODE_PULL_CONSTANT_LOAD:
> >> +  case GS_OPCODE_SET_PRIMITIVE_ID:
> >> +  case GS_OPCODE_GET_INSTANCE_ID:
> >> +  case SHADER_OPCODE_GEN4_SCRATCH_READ:
> >> +  case SHADER_OPCODE_GEN4_SCRATCH_WRITE:
> >> + return true;
> >> +  default:
> >> + return false;
> >> +  }
> >> +   }
> >> };
> >>
> >> /**
> >> diff --git a/src/intel/compiler/brw_schedule_instructions.cpp
> >> b/src/intel/compiler/brw_schedule_instructions.cpp
> >> index fa85045de74..f817142a8b5 100644
> >> --- a/src/intel/compiler/brw_schedule_instructions.cpp
> >> +++ b/src/intel/compiler/brw_schedule_instructions.cpp
> >> @@ -1267,6 +1267,9 @@ vec4_instruction_scheduler::calculate_deps()
> >>  }
> >>   }
> >>
> >> +  if (inst->reads_g0_implicitly())
> >> + add_dep(last_fixed_grf_write, n);
> >> +
> >>   if (!inst->is_send_from_grf()) {
> >>  for (int i = 0; i < inst->mlen; i++) {
> >> /* It looks like the MRF regs are released in the send
> >> --
> >> 2.14.3
> >>
> >> ___
> >> 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

Re: [Mesa-dev] [PATCH 4/4] intel/compiler: Add scheduler deps for instructions that implicitly read g0

[Ian Romanick]

On 04/20/2018 11:56 AM, Jason Ekstrand wrote:
> Ugh... I was actually thinking about this the other day.  How did you
> come up with your list of instructions?  Is there some algorithmic way
> to determine it such as sends with no header?

I just looked through the generator for pseudo-ops that emit explicit
reads of g0.  Since it was a manual process, I may have missed some. :(
I don't know of a way to automate this, but... it may be possible.  Not
sure.

> On April 20, 2018 08:52:50 "Ian Romanick"  wrote:
> 
>> From: Ian Romanick 
>>
>> Otherwise the scheduler can move the writes after the reads.
>>
>> Signed-off-by: Ian Romanick 
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95012
>> Cc: Mark Janes 
>> Cc: Clayton A Craft 
>> ---
>> I'd really like to have this run on the CI with the tests from bugzill
>> #95012 re-enabled.  I don't know how to do this on my own.  This series
>> is also available at:
>>
>>    https://cgit.freedesktop.org/~idr/mesa/log/?h=bug-95012.
>>
>> src/intel/compiler/brw_ir_vec4.h | 25
>> 
>> src/intel/compiler/brw_schedule_instructions.cpp |  3 +++
>> 2 files changed, 28 insertions(+)
>>
>> diff --git a/src/intel/compiler/brw_ir_vec4.h
>> b/src/intel/compiler/brw_ir_vec4.h
>> index 95c5119c6c0..e401d8b4d16 100644
>> --- a/src/intel/compiler/brw_ir_vec4.h
>> +++ b/src/intel/compiler/brw_ir_vec4.h
>> @@ -334,6 +334,31 @@ public:
>>   opcode != BRW_OPCODE_IF &&
>>   opcode != BRW_OPCODE_WHILE));
>>    }
>> +
>> +   bool reads_g0_implicitly() const
>> +   {
>> +  switch (opcode) {
>> +  case SHADER_OPCODE_TEX:
>> +  case SHADER_OPCODE_TXL:
>> +  case SHADER_OPCODE_TXD:
>> +  case SHADER_OPCODE_TXF:
>> +  case SHADER_OPCODE_TXF_CMS_W:
>> +  case SHADER_OPCODE_TXF_CMS:
>> +  case SHADER_OPCODE_TXF_MCS:
>> +  case SHADER_OPCODE_TXS:
>> +  case SHADER_OPCODE_TG4:
>> +  case SHADER_OPCODE_TG4_OFFSET:
>> +  case SHADER_OPCODE_SAMPLEINFO:
>> +  case VS_OPCODE_PULL_CONSTANT_LOAD:
>> +  case GS_OPCODE_SET_PRIMITIVE_ID:
>> +  case GS_OPCODE_GET_INSTANCE_ID:
>> +  case SHADER_OPCODE_GEN4_SCRATCH_READ:
>> +  case SHADER_OPCODE_GEN4_SCRATCH_WRITE:
>> + return true;
>> +  default:
>> + return false;
>> +  }
>> +   }
>> };
>>
>> /**
>> diff --git a/src/intel/compiler/brw_schedule_instructions.cpp
>> b/src/intel/compiler/brw_schedule_instructions.cpp
>> index fa85045de74..f817142a8b5 100644
>> --- a/src/intel/compiler/brw_schedule_instructions.cpp
>> +++ b/src/intel/compiler/brw_schedule_instructions.cpp
>> @@ -1267,6 +1267,9 @@ vec4_instruction_scheduler::calculate_deps()
>>  }
>>   }
>>
>> +  if (inst->reads_g0_implicitly())
>> + add_dep(last_fixed_grf_write, n);
>> +
>>   if (!inst->is_send_from_grf()) {
>>  for (int i = 0; i < inst->mlen; i++) {
>>     /* It looks like the MRF regs are released in the send
>> -- 
>> 2.14.3
>>
>> ___
>> 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

Re: [Mesa-dev] [PATCH 4/4] intel/compiler: Add scheduler deps for instructions that implicitly read g0

[Jason Ekstrand]

Ugh... I was actually thinking about this the other day.  How did you come 
up with your list of instructions?  Is there some algorithmic way to 
determine it such as sends with no header?


On April 20, 2018 08:52:50 "Ian Romanick"  wrote:


From: Ian Romanick 

Otherwise the scheduler can move the writes after the reads.

Signed-off-by: Ian Romanick 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95012
Cc: Mark Janes 
Cc: Clayton A Craft 
---
I'd really like to have this run on the CI with the tests from bugzill
#95012 re-enabled.  I don't know how to do this on my own.  This series
is also available at:

   https://cgit.freedesktop.org/~idr/mesa/log/?h=bug-95012.

src/intel/compiler/brw_ir_vec4.h | 25 
src/intel/compiler/brw_schedule_instructions.cpp |  3 +++
2 files changed, 28 insertions(+)

diff --git a/src/intel/compiler/brw_ir_vec4.h 
b/src/intel/compiler/brw_ir_vec4.h

index 95c5119c6c0..e401d8b4d16 100644
--- a/src/intel/compiler/brw_ir_vec4.h
+++ b/src/intel/compiler/brw_ir_vec4.h
@@ -334,6 +334,31 @@ public:
  opcode != BRW_OPCODE_IF &&
  opcode != BRW_OPCODE_WHILE));
   }
+
+   bool reads_g0_implicitly() const
+   {
+  switch (opcode) {
+  case SHADER_OPCODE_TEX:
+  case SHADER_OPCODE_TXL:
+  case SHADER_OPCODE_TXD:
+  case SHADER_OPCODE_TXF:
+  case SHADER_OPCODE_TXF_CMS_W:
+  case SHADER_OPCODE_TXF_CMS:
+  case SHADER_OPCODE_TXF_MCS:
+  case SHADER_OPCODE_TXS:
+  case SHADER_OPCODE_TG4:
+  case SHADER_OPCODE_TG4_OFFSET:
+  case SHADER_OPCODE_SAMPLEINFO:
+  case VS_OPCODE_PULL_CONSTANT_LOAD:
+  case GS_OPCODE_SET_PRIMITIVE_ID:
+  case GS_OPCODE_GET_INSTANCE_ID:
+  case SHADER_OPCODE_GEN4_SCRATCH_READ:
+  case SHADER_OPCODE_GEN4_SCRATCH_WRITE:
+ return true;
+  default:
+ return false;
+  }
+   }
};

/**
diff --git a/src/intel/compiler/brw_schedule_instructions.cpp 
b/src/intel/compiler/brw_schedule_instructions.cpp

index fa85045de74..f817142a8b5 100644
--- a/src/intel/compiler/brw_schedule_instructions.cpp
+++ b/src/intel/compiler/brw_schedule_instructions.cpp
@@ -1267,6 +1267,9 @@ vec4_instruction_scheduler::calculate_deps()
 }
  }

+  if (inst->reads_g0_implicitly())
+ add_dep(last_fixed_grf_write, n);
+
  if (!inst->is_send_from_grf()) {
 for (int i = 0; i < inst->mlen; i++) {
/* It looks like the MRF regs are released in the send
--
2.14.3

___
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

[Mesa-dev] [PATCH 4/4] intel/compiler: Add scheduler deps for instructions that implicitly read g0

[Ian Romanick]

From: Ian Romanick 

Otherwise the scheduler can move the writes after the reads.

Signed-off-by: Ian Romanick 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95012
Cc: Mark Janes 
Cc: Clayton A Craft 
---
I'd really like to have this run on the CI with the tests from bugzill
#95012 re-enabled.  I don't know how to do this on my own.  This series
is also available at:

https://cgit.freedesktop.org/~idr/mesa/log/?h=bug-95012.

 src/intel/compiler/brw_ir_vec4.h | 25 
 src/intel/compiler/brw_schedule_instructions.cpp |  3 +++
 2 files changed, 28 insertions(+)

diff --git a/src/intel/compiler/brw_ir_vec4.h b/src/intel/compiler/brw_ir_vec4.h
index 95c5119c6c0..e401d8b4d16 100644
--- a/src/intel/compiler/brw_ir_vec4.h
+++ b/src/intel/compiler/brw_ir_vec4.h
@@ -334,6 +334,31 @@ public:
   opcode != BRW_OPCODE_IF &&
   opcode != BRW_OPCODE_WHILE));
}
+
+   bool reads_g0_implicitly() const
+   {
+  switch (opcode) {
+  case SHADER_OPCODE_TEX:
+  case SHADER_OPCODE_TXL:
+  case SHADER_OPCODE_TXD:
+  case SHADER_OPCODE_TXF:
+  case SHADER_OPCODE_TXF_CMS_W:
+  case SHADER_OPCODE_TXF_CMS:
+  case SHADER_OPCODE_TXF_MCS:
+  case SHADER_OPCODE_TXS:
+  case SHADER_OPCODE_TG4:
+  case SHADER_OPCODE_TG4_OFFSET:
+  case SHADER_OPCODE_SAMPLEINFO:
+  case VS_OPCODE_PULL_CONSTANT_LOAD:
+  case GS_OPCODE_SET_PRIMITIVE_ID:
+  case GS_OPCODE_GET_INSTANCE_ID:
+  case SHADER_OPCODE_GEN4_SCRATCH_READ:
+  case SHADER_OPCODE_GEN4_SCRATCH_WRITE:
+ return true;
+  default:
+ return false;
+  }
+   }
 };
 
 /**
diff --git a/src/intel/compiler/brw_schedule_instructions.cpp 
b/src/intel/compiler/brw_schedule_instructions.cpp
index fa85045de74..f817142a8b5 100644
--- a/src/intel/compiler/brw_schedule_instructions.cpp
+++ b/src/intel/compiler/brw_schedule_instructions.cpp
@@ -1267,6 +1267,9 @@ vec4_instruction_scheduler::calculate_deps()
  }
   }
 
+  if (inst->reads_g0_implicitly())
+ add_dep(last_fixed_grf_write, n);
+
   if (!inst->is_send_from_grf()) {
  for (int i = 0; i < inst->mlen; i++) {
 /* It looks like the MRF regs are released in the send
-- 
2.14.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev