On Sat, May 26, 2018 at 11:10 AM, Francisco Jerez <curroje...@riseup.net> wrote:
> Jason Ekstrand <ja...@jlekstrand.net> writes: > > > From: Francisco Jerez <curroje...@riseup.net> > > > > When we don't have PLN (gen4 and gen11+), we implement LINTERP as either > > LINE+MAC or a pair of MADs. In both cases, the accumulator is written > > by the first of the two instructions and read by the second. Even > > though the accumulator value isn't actually ever used from a logical > > instruction perspective, it is trashed so we need to make the scheduler > > aware. Otherwise, the scheduler could end up re-ordering instructions > > and putting a LINTERP between another an instruction which writes the > > accumulator and another which tries to use that result. > > > > Cc: mesa-sta...@lists.freedesktop.org > > --- > > src/intel/compiler/brw_shader.cpp | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/src/intel/compiler/brw_shader.cpp b/src/intel/compiler/brw_ > shader.cpp > > index 141b64e..dfd2c5c 100644 > > --- a/src/intel/compiler/brw_shader.cpp > > +++ b/src/intel/compiler/brw_shader.cpp > > @@ -984,7 +984,8 @@ backend_instruction::writes_accumulator_implicitly(const > struct gen_device_info > > return writes_accumulator || > > (devinfo->gen < 6 && > > ((opcode >= BRW_OPCODE_ADD && opcode < BRW_OPCODE_NOP) || > > - (opcode >= FS_OPCODE_DDX_COARSE && opcode <= > FS_OPCODE_LINTERP))); > > + (opcode >= FS_OPCODE_DDX_COARSE && opcode <= > FS_OPCODE_LINTERP))) || > > + (opcode == FS_OPCODE_LINTERP && !devinfo->has_pln); > > The devinfo->has_pln condition is technically inaccurate, because even > SNB will fall back to the non-PLN path which overwrites the accumulator > for certain valid IR, which yeah I'm aware is not *typically* > encountered before this series, I'm pretty sure it's impossible before this series because, without SIMD32, the barycentric coordinates always start at g2 and get incremented by 2 every time. The only other way to get something into the coordinates source of the PLN is with a pixel interpolator message. For that, the register allocator has a workaround to ensure that it's assigned an even register on SNB. One of the early patches in my series replaces the broken gen4.5-6 PLN fall-back (it didn't work in SIMD16 because it assumed the wrong register layout for coordinates) with an assert and Jenkins is just fine with the assert. > but why make things more inaccurate than > the original only to revert back to a devinfo->gen based check in > PATCHv2 33? > See above. > I think I'd squash the last hunk of PATCHv2 33 into this one which would > give you something functionally equivalent to v1 but updated to handle > Gen11 correctly. > > > } > > > > bool > > -- > > 2.5.0.400.gff86faf >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
