[Mesa-dev] [PATCH 18/22] i965/fs: Add infrastructure for generating CSEL instructions.
Kenneth Graunke
kenneth at whitecape.org
Mon Mar 5 22:24:40 UTC 2018
On Monday, March 5, 2018 11:17:29 AM PST Ian Romanick wrote:
> On 02/28/2018 12:58 AM, Samuel Iglesias Gonsálvez wrote:
> > On 24/02/18 00:56, Ian Romanick wrote:
> >> From: Kenneth Graunke <kenneth at whitecape.org>
> >>
> >> v2 (idr): Don't allow CSEL with a non-float src2.
> >>
> >> v3 (idr): Add CSEL to fs_inst::flags_written. Suggested by Matt.
> >>
> >> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> >> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> >> ---
> >> src/intel/compiler/brw_eu.h | 1 +
> >> src/intel/compiler/brw_eu_emit.c | 1 +
> >> src/intel/compiler/brw_fs.cpp | 1 +
> >> src/intel/compiler/brw_fs_builder.h | 22 +++++++++++++++++++++-
> >> src/intel/compiler/brw_fs_generator.cpp | 5 +++++
> >> 5 files changed, 29 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/intel/compiler/brw_fs_generator.cpp b/src/intel/compiler/brw_fs_generator.cpp
> >> index cd5be05..9157367 100644
> >> --- a/src/intel/compiler/brw_fs_generator.cpp
> >> +++ b/src/intel/compiler/brw_fs_generator.cpp
> >> @@ -1826,6 +1826,11 @@ fs_generator::generate_code(const cfg_t *cfg, int dispatch_width)
> >> case BRW_OPCODE_SEL:
> >> brw_SEL(p, dst, src[0], src[1]);
> >> break;
> >> + case BRW_OPCODE_CSEL:
> >> + brw_set_default_access_mode(p, BRW_ALIGN_16);
> >> + brw_CSEL(p, dst, src[0], src[1], src[2]);
> >> + brw_set_default_access_mode(p, BRW_ALIGN_1);
> >
> > Setting the access mode to ALIGN_1 is not needed, right? It will
> > continue with the next instruction in the loop that sets ALIGN1 at the
> > beginning of the loop.
>
> This was in Ken's original patch, so I'm not 100% sure. I've seen this
> pattern around generating some other instructions, so my gut tells me
> that it's done like this to be explicit and more defensive.
Yeah, I don't think it's strictly necessary, but it's probably a good
idea regardless, just to be defensive. Most instructions should be
align1, and we just want to emit this one instruction in align16.
A more conservative approach still would be to push/set_default/pop.
I think when I wrote this, I was planning on eliminating push/pop, but
that never happened (and probably won't at this point).
I think it's fine as is, or I'd be fine with push/set/pop.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180305/517bc929/attachment.sig>
More information about the mesa-dev
mailing list