[Mesa-dev] [PATCH 09/19] i965/fs: Add SHADER_OPCODE_LOAD_PAYLOAD.

[Pohjolainen, Topi]

On Wed, May 28, 2014 at 10:06:03AM -0700, Matt Turner wrote:
> On Wed, May 28, 2014 at 7:43 AM, Pohjolainen, Topi
> <topi.pohjolainen at intel.com> wrote:
> > On Tue, May 27, 2014 at 06:47:40PM -0700, Matt Turner wrote:
> >> Will be used to simplify the handling of large virtual GRFs in SSA form.
> >> ---
> >>  src/mesa/drivers/dri/i965/brw_defines.h        |  2 ++
> >>  src/mesa/drivers/dri/i965/brw_fs.cpp           | 10 ++++++++++
> >>  src/mesa/drivers/dri/i965/brw_fs.h             |  2 ++
> >>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp |  3 +++
> >>  src/mesa/drivers/dri/i965/brw_shader.cpp       |  3 +++
> >>  5 files changed, 20 insertions(+)
> >>
> >> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h
> >> index c38e447..34467e9 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> >> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> >> @@ -797,6 +797,8 @@ enum opcode {
> >>     SHADER_OPCODE_TG4,
> >>     SHADER_OPCODE_TG4_OFFSET,
> >>
> >> +   SHADER_OPCODE_LOAD_PAYLOAD,
> >> +
> >>     SHADER_OPCODE_SHADER_TIME_ADD,
> >>
> >>     SHADER_OPCODE_UNTYPED_ATOMIC,
> >> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> >> index c86cb42..0856b6b 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> >> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> >> @@ -241,6 +241,16 @@ fs_visitor::CMP(fs_reg dst, fs_reg src0, fs_reg src1, uint32_t condition)
> >>     return inst;
> >>  }
> >>
> >> +fs_inst *
> >> +fs_visitor::LOAD_PAYLOAD(const fs_reg &dst, fs_reg *src, int sources)
> >> +{
> >> +   fs_inst *inst = new(mem_ctx) fs_inst(SHADER_OPCODE_LOAD_PAYLOAD, dst, src,
> >> +                                        sources);
> >> +   inst->regs_written = sources;
> >> +
> >> +   return inst;
> >> +}
> >> +
> >>  exec_list
> >>  fs_visitor::VARYING_PULL_CONSTANT_LOAD(const fs_reg &dst,
> >>                                         const fs_reg &surf_index,
> >> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> >> index 527c3f3..d0e459c 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> >> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> >> @@ -338,6 +338,8 @@ public:
> >>                                          fs_inst *end,
> >>                                          const fs_reg &reg);
> >>
> >> +   fs_inst *LOAD_PAYLOAD(const fs_reg &dst, fs_reg *src, int sources);
> >> +
> >>     exec_list VARYING_PULL_CONSTANT_LOAD(const fs_reg &dst,
> >>                                          const fs_reg &surf_index,
> >>                                          const fs_reg &varying_offset,
> >> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> >> index a5be0ec..26b963b 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> >> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> >> @@ -1713,6 +1713,9 @@ fs_generator::generate_code(exec_list *instructions,
> >>           generate_discard_jump(inst);
> >>           break;
> >>
> >> +      case SHADER_OPCODE_LOAD_PAYLOAD:
> >> +         break;
> >> +
> >
> > Did I misunderstand but wasn't the idea to always lower this in the visitor
> > before generator? If that is the case then shouldn't this be dropped and let
> > the default case to issue an error?
> 
> I'll add an assert here confirming that it's never hit. Using the
> default case would yield an "Unsupported opcode" error message.

That makes sense. On my behalf it is then:

Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>