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 <kenn...@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 <kenn...@whitecape.org> > >> Signed-off-by: Ian Romanick <ian.d.roman...@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.
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