On Thu, 2015-07-30 at 16:27 +0300, Francisco Jerez wrote: > Iago Toral Quiroga <ito...@igalia.com> writes: > > > In theory, GRF array access should have been moved to scratch by the time > > we got here, so this should never happen. A full piglit run forcing > > spilling of all registers seems to confirm this. The FS backend > > does not seem to check for this either. > > --- > > src/mesa/drivers/dri/i965/brw_vec4_reg_allocate.cpp | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_reg_allocate.cpp > > b/src/mesa/drivers/dri/i965/brw_vec4_reg_allocate.cpp > > index a9bf0d8..cff5406 100644 > > --- a/src/mesa/drivers/dri/i965/brw_vec4_reg_allocate.cpp > > +++ b/src/mesa/drivers/dri/i965/brw_vec4_reg_allocate.cpp > > @@ -282,15 +282,13 @@ vec4_visitor::evaluate_spill_costs(float > > *spill_costs, bool *no_spill) > > for (unsigned int i = 0; i < 3; i++) { > > if (inst->src[i].file == GRF) { > > spill_costs[inst->src[i].reg] += loop_scale; > > - if (inst->src[i].reladdr) > > - no_spill[inst->src[i].reg] = true; > > + assert(!inst->src[i].reladdr); > > } > > } > > > > if (inst->dst.file == GRF) { > > spill_costs[inst->dst.reg] += loop_scale; > > - if (inst->dst.reladdr) > > - no_spill[inst->dst.reg] = true; > > I guess the intention behind these checks was to support unlowered > indirect addressing of GRFs sometime in the future. Is there some > reason why you want to remove them?
The only reason was that AFAICT we never let indirect addressing of GRFs get here, so this code is checking for a case that can't happen at the moment and I figured that maybe some forgot to just remove it. But I am okay with it if you think this can serve a purpose later on, I just thought this was dead code. > That said, I don't think there would be any reason to force indirectly > addressed VGRFs not to spill even if we implemented them, because if the > VGRF is larger than one physical register it will already have been > marked as no-spill by the previous loop at the top of this function, and > if they are exactly one physical register the reladdr value is > guaranteed to be zero so the indirect addressing is a no-op. > > How about we get rid of these checks as you have done, but leave no > assertions behind? Ben was working on trying to enable spilling of registers with size > 1, he abandoned that idea for now but I figure that he or someone else may want to do that at some point... so all things considered, maybe we should just drop this patch? > > + assert(!inst->dst.reladdr); > > } > > > > switch (inst->opcode) { > > -- > > 1.9.1 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev