On Tue, Jun 9, 2015 at 4:05 AM, Neil Roberts <n...@linux.intel.com> wrote: > Both patches look good to me and I can confirm they make the Piglit > tests pass on Skylake. > > Reviewed-by: Neil Roberts <n...@linux.intel.com> > > My original assumption of the problem was that the implied writes from > the SCRATCH_WRITE instruction aren't taken into account when calculating > the liveliness of the registers and I didn't realise that in the > particular case of the bug report this doesn't matter because the > register for the EOT message is effectively hard-coded to clash with the > message register used for spilling. I'm still not totally convinced that > there's not still a problem where a register can be allocated to clash > with the scratch write register because nothing in > fs_live_variables::setup_def_use takes into account the implied MRF > writes.
As far as liveness analysis is concerned, those are just MRF registers. I think liveness either ignores them or just treats them as a single block. I don't know the liveness code that well to be honest. The only place when the fact that the MRFs are virtual matters is in register allocation. Implied MRF writes are taken into account in setup_mrf_hack_interference. We figure out what MRFs are used and then mark them as conflicting with *all* of the VGRFs. We also force them to be placed in the correct physical register using ra_set_node_reg. The register allocator is then smart enough to not place any other VGRF values into one of those registers. The problem is that we were also forcing the message for the final FB write and we were forcing it to overlap. > However if that is the case and I haven't just misunderstood the > code then it is a separate problem and we'd still need this patch > anyway. > > I was thinking it's a bit risky to just use a register that is lower > than the lowest used MRF register because it could be possible for a > message to use a really low register number and then this would end up > using a negative register number. However in practice on Gen7+ it looks > like there's hardly anything left that is using the fake MRF and we're > unlikely to add more so it probably doesn't really matter. The maximum length of a message is 16 and we have at most 16 virtual MRFs so the message will start no earlier than g96. I think it's fine as long as it's in the upper half or something like that. Thanks for reviewing! --Jason _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
