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 <ito...@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 <ito...@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.pohjolai...@intel.com>
> To: Iago Toral Quiroga <ito...@igalia.com>
> Cc: mesa-dev@lists.freedesktop.org
> Subject: Re: [Mesa-dev] [PATCH 1/4] nir/instr_set: Add an allow_loads field
> Message-ID: <20151027123349.gb2...@nelli.ger.corp.intel.com>
> References: <1445938141-28845-1-git-send-email-ito...@igalia.com>
> <1445938141-28845-2-git-send-email-ito...@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-ito...@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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
