[Mesa-dev] [PATCH 1/4] nir/instr_set: Add an allow_loads field

Iago Toral itoral at igalia.com
Tue Oct 27 23:46:18 PDT 2015


On Tue, 2015-10-27 at 14:33 +0200, Pohjolainen, Topi wrote:
> On Tue, Oct 27, 2015 at 10:28:58AM +0100, Iago Toral Quiroga wrote:
> > We need this so we can configure different behaviors for passes that
> > cannot deal with side-effectful instructions (CSE) and passes that can
> > (we will add a load-combine pass shortly).
> > 
> > For now, when allow_loads is true, we let the instruction set rewrite
> > SSBO loads.
> > ---
> >  src/glsl/nir/nir_instr_set.c | 51 ++++++++++++++++++++++++++++----------------
> >  src/glsl/nir/nir_instr_set.h | 20 ++++++++++++-----
> >  src/glsl/nir/nir_opt_cse.c   |  4 ++--
> >  3 files changed, 50 insertions(+), 25 deletions(-)
> > 
> > diff --git a/src/glsl/nir/nir_instr_set.c b/src/glsl/nir/nir_instr_set.c
> > index d3f939f..583618f 100644
> > --- a/src/glsl/nir/nir_instr_set.c
> > +++ b/src/glsl/nir/nir_instr_set.c
> > @@ -398,6 +398,13 @@ dest_is_ssa(nir_dest *dest, void *data)
> >     return dest->is_ssa;
> >  }
> >  
> > +static bool
> > +is_load(nir_intrinsic_instr *instr)
> > +{
> > +   return instr->intrinsic == nir_intrinsic_load_ssbo ||
> > +          instr->intrinsic == nir_intrinsic_load_ssbo_indirect;
> > +}
> > +
> >  /* This function determines if uses of an instruction can safely be rewritten
> >   * to use another identical instruction instead. Note that this function must
> >   * be kept in sync with hash_instr() and nir_instrs_equal() -- only
> > @@ -406,7 +413,7 @@ dest_is_ssa(nir_dest *dest, void *data)
> >   */
> >  
> >  static bool
> > -instr_can_rewrite(nir_instr *instr)
> > +instr_can_rewrite(nir_instr *instr, bool allow_loads)
> >  {
> >     /* We only handle SSA. */
> >     if (!nir_foreach_dest(instr, dest_is_ssa, NULL) ||
> > @@ -428,11 +435,15 @@ instr_can_rewrite(nir_instr *instr)
> >        return true;
> >     }
> >     case nir_instr_type_intrinsic: {
> > +      nir_intrinsic_instr *intrinsic = nir_instr_as_intrinsic(instr);
> >        const nir_intrinsic_info *info =
> > -         &nir_intrinsic_infos[nir_instr_as_intrinsic(instr)->intrinsic];
> > -      return (info->flags & NIR_INTRINSIC_CAN_ELIMINATE) &&
> > -             (info->flags & NIR_INTRINSIC_CAN_REORDER) &&
> > -             info->num_variables == 0; /* not implemented yet */
> > +         &nir_intrinsic_infos[intrinsic->intrinsic];
> > +      bool can_eliminate_and_reorder =
> > +         (info->flags & NIR_INTRINSIC_CAN_ELIMINATE) &&
> > +         (info->flags & NIR_INTRINSIC_CAN_REORDER) &&
> > +         info->num_variables == 0; /* not implemented yet */
> > +      return can_eliminate_and_reorder ?
> > +         true: allow_loads && is_load(intrinsic);
> 
> Isn't this just?
> 
>          return can_eliminate_and_reorder ||
>                 (allow_loads && is_load(intrinsic));
> 
> Received: from fanzine.local.igalia.com ([192.168.10.13] helo=fanzine.igalia.com)
> 	by mail.igalia.com with esmtps 
> 	(Cipher TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim)
> 	id 1Zr3SP-0002rb-7Q
> 	for <itoral at igalia.com>; Tue, 27 Oct 2015 13:34:29 +0100
> Received: from mga14.intel.com ([192.55.52.115])
> 	by fanzine.igalia.com with esmtp (Exim)
> 	id 1Zr3SO-0001hB-Rd
> 	for <itoral at igalia.com>; Tue, 27 Oct 2015 13:34:29 +0100
> Received: from fmsmga002.fm.intel.com ([10.253.24.26])
>   by fmsmga103.fm.intel.com with ESMTP; 27 Oct 2015 05:33:51 -0700
> X-ExtLoop1: 1
> X-IronPort-AV: E=Sophos;i="5.20,205,1444719600"; 
>    d="scan'208";a="836522023"
> Received: from kgoijens-mobl5.ger.corp.intel.com (HELO nelli) ([10.252.24.134])
>   by fmsmga002.fm.intel.com with ESMTP; 27 Oct 2015 05:33:50 -0700
> Date: Tue, 27 Oct 2015 14:33:50 +0200
> From: "Pohjolainen, Topi" <topi.pohjolainen at intel.com>
> To: Iago Toral Quiroga <itoral at igalia.com>
> Cc: mesa-dev at lists.freedesktop.org
> Subject: Re: [Mesa-dev] [PATCH 1/4] nir/instr_set: Add an allow_loads field
> Message-ID: <20151027123349.GB2575 at nelli.ger.corp.intel.com>
> References: <1445938141-28845-1-git-send-email-itoral at igalia.com>
>  <1445938141-28845-2-git-send-email-itoral at igalia.com>
> MIME-Version: 1.0
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> In-Reply-To: <1445938141-28845-2-git-send-email-itoral at igalia.com>
> User-Agent: Mutt/1.5.23 (2014-03-12)
> 
> On Tue, Oct 27, 2015 at 10:28:58AM +0100, Iago Toral Quiroga wrote:
> > We need this so we can configure different behaviors for passes that
> > cannot deal with side-effectful instructions (CSE) and passes that can
> > (we will add a load-combine pass shortly).
> > 
> > For now, when allow_loads is true, we let the instruction set rewrite
> > SSBO loads.
> > ---
> >  src/glsl/nir/nir_instr_set.c | 51 ++++++++++++++++++++++++++++----------------
> >  src/glsl/nir/nir_instr_set.h | 20 ++++++++++++-----
> >  src/glsl/nir/nir_opt_cse.c   |  4 ++--
> >  3 files changed, 50 insertions(+), 25 deletions(-)
> > 
> > diff --git a/src/glsl/nir/nir_instr_set.c b/src/glsl/nir/nir_instr_set.c
> > index d3f939f..583618f 100644
> > --- a/src/glsl/nir/nir_instr_set.c
> > +++ b/src/glsl/nir/nir_instr_set.c
> > @@ -398,6 +398,13 @@ dest_is_ssa(nir_dest *dest, void *data)
> >     return dest->is_ssa;
> >  }
> >  
> > +static bool
> > +is_load(nir_intrinsic_instr *instr)
> > +{
> > +   return instr->intrinsic == nir_intrinsic_load_ssbo ||
> > +          instr->intrinsic == nir_intrinsic_load_ssbo_indirect;
> > +}
> > +
> >  /* This function determines if uses of an instruction can safely be rewritten
> >   * to use another identical instruction instead. Note that this function must
> >   * be kept in sync with hash_instr() and nir_instrs_equal() -- only
> > @@ -406,7 +413,7 @@ dest_is_ssa(nir_dest *dest, void *data)
> >   */
> >  
> >  static bool
> > -instr_can_rewrite(nir_instr *instr)
> > +instr_can_rewrite(nir_instr *instr, bool allow_loads)
> >  {
> >     /* We only handle SSA. */
> >     if (!nir_foreach_dest(instr, dest_is_ssa, NULL) ||
> > @@ -428,11 +435,15 @@ instr_can_rewrite(nir_instr *instr)
> >        return true;
> >     }
> >     case nir_instr_type_intrinsic: {
> > +      nir_intrinsic_instr *intrinsic = nir_instr_as_intrinsic(instr);
> >        const nir_intrinsic_info *info =
> > -         &nir_intrinsic_infos[nir_instr_as_intrinsic(instr)->intrinsic];
> > -      return (info->flags & NIR_INTRINSIC_CAN_ELIMINATE) &&
> > -             (info->flags & NIR_INTRINSIC_CAN_REORDER) &&
> > -             info->num_variables == 0; /* not implemented yet */
> > +         &nir_intrinsic_infos[intrinsic->intrinsic];
> > +      bool can_eliminate_and_reorder =
> > +         (info->flags & NIR_INTRINSIC_CAN_ELIMINATE) &&
> > +         (info->flags & NIR_INTRINSIC_CAN_REORDER) &&
> > +         info->num_variables == 0; /* not implemented yet */
> > +      return can_eliminate_and_reorder ?
> > +         true: allow_loads && is_load(intrinsic);
> 
> Isn't this just?
> 
>          return can_eliminate_and_reorder ||
>                 (allow_loads && is_load(intrinsic));

Right, changed locally. Thanks!

Iago




More information about the mesa-dev mailing list