On Fri, 2009-09-11 at 10:37 -0700, Ian Romanick wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Eric Anholt wrote:
> > Module: Mesa
> > Branch: master
> > Commit: 7c0152fbaeb21ab423a9de339b85c54d1713432b
> > URL:    
> > http://cgit.freedesktop.org/mesa/mesa/commit/?id=7c0152fbaeb21ab423a9de339b85c54d1713432b
> > 
> > Author: Eric Anholt <[email protected]>
> > Date:   Thu Sep 10 09:44:30 2009 -0700
> > 
> > i965: Enable loops in the VS.
> > 
> > Passes piglit glsl-vs-loop testcase.
> > 
> > Bug #20171
> 
> I see this went into master.  Is this also a candidate for 7.5 or 7.6?
> If so, it should have gone there first.  I can take care of the
> cherry-picking and merging while you're on vacation.

For the record, I hate that development model.  It means that I have to
first identify the earliest branch a change could apply to before
working.  Or I have to do a dance more complicated than just
cherry-picking after the fact to land my change after I develop it on
master.  It hurts the idle, "how hard will this little thing be"
experimenting that I do a lot.  And given the number of commits of
merges with conflicts we're seeing, I'm doubting that the merges forward
are seeing much testing.

I much prefer periodically analyzing my area and cherry-picking many
patches after they've had a chance to settle, regression testing the
batch all at once, and giving stable something that works.

> > ---
> > 
> >  src/mesa/drivers/dri/i965/brw_vs_emit.c |   53 
> > ++++++++++++++++++++++---------
> >  1 files changed, 38 insertions(+), 15 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_vs_emit.c 
> > b/src/mesa/drivers/dri/i965/brw_vs_emit.c
> > index 584fdbd..1638ef8 100644
> > --- a/src/mesa/drivers/dri/i965/brw_vs_emit.c
> > +++ b/src/mesa/drivers/dri/i965/brw_vs_emit.c
> > @@ -1270,9 +1270,27 @@ post_vs_emit( struct brw_vs_compile *c,
> >  }
> >  
> >  static uint32_t
> > -get_predicate(uint32_t swizzle)
> > +get_predicate(const struct prog_instruction *inst)
> >  {
> > -   switch (swizzle) {
> > +   if (inst->DstReg.CondMask == COND_TR)
> > +      return BRW_PREDICATE_NONE;
> > +
> > +   /* All of GLSL only produces predicates for COND_NE and one channel per
> > +    * vector.  Fail badly if someone starts doing something else, as it 
> > might
> > +    * mean infinite looping or something.
> > +    *
> > +    * We'd like to support all the condition codes, but our hardware 
> > doesn't
> > +    * quite match the Mesa IR, which is modeled after the NV extensions.  
> > For
> > +    * those, the instruction may update the condition codes or not, then 
> > any
> > +    * later instruction may use one of those condition codes.  For gen4, 
> > the
> > +    * instruction may update the flags register based on one of the 
> > condition
> > +    * codes output by the instruction, and then further instructions may
> > +    * predicate on that.  We can probably support this, but it won't
> > +    * necessarily be easy.
> > +    */
> > +   assert(inst->DstReg.CondMask == COND_NE);
> 
> Is there a way we could fall back to software TNL here instead of asserting?

Nothing uses any other CC yet, so no need afaict.

For the NV stuff, I need to look at what instructions can update CCs
(everything?) and look more into what can use CCs (not much, it looks
like?).  If we just scan forward and compute which CC we need to
produce, will that be good enough?  Do we save off temps and recompute a
different CC from the same update if someone uses it?  Or, have the CC
updating instruction produce all of them and save the flags off to temps
for later use?

Some things, like the CAL implementation, will be broken in the presence
of CCs because it does jumps with no execution masking, so we'll take
the path for two verts at a time even if only one passes.  I'm kind of
thinking of just dumping them back to single program flow in that case.

> > +
> > +   switch (inst->DstReg.CondSwizzle) {
> >     case SWIZZLE_XXXX:
> >        return BRW_PREDICATE_ALIGN16_REPLICATE_X;
> >     case SWIZZLE_YYYY:
> > @@ -1282,7 +1300,8 @@ get_predicate(uint32_t swizzle)
> >     case SWIZZLE_WWWW:
> >        return BRW_PREDICATE_ALIGN16_REPLICATE_W;
> >     default:
> > -      _mesa_problem(NULL, "Unexpected predicate: 0x%08x\n", swizzle);
> > +      _mesa_problem(NULL, "Unexpected predicate: 0x%08x\n",
> > +               inst->DstReg.CondMask);
> >        return BRW_PREDICATE_NORMAL;
> >     }
> >  }
> > @@ -1294,6 +1313,7 @@ void brw_vs_emit(struct brw_vs_compile *c )
> >  #define MAX_IF_DEPTH 32
> >  #define MAX_LOOP_DEPTH 32
> >     struct brw_compile *p = &c->func;
> > +   struct brw_context *brw = p->brw;
> >     const GLuint nr_insns = c->vp->program.Base.NumInstructions;
> >     GLuint insn, if_depth = 0, loop_depth = 0;
> >     GLuint end_offset = 0;
> > @@ -1492,8 +1512,8 @@ void brw_vs_emit(struct brw_vs_compile *c )
> >        case OPCODE_IF:
> >      assert(if_depth < MAX_IF_DEPTH);
> >      if_inst[if_depth] = brw_IF(p, BRW_EXECUTE_8);
> > -    if_inst[if_depth]->header.predicate_control =
> > -       get_predicate(inst->DstReg.CondSwizzle);
> > +    /* Note that brw_IF smashes the predicate_control field. */
> > +    if_inst[if_depth]->header.predicate_control = get_predicate(inst);
> >      if_depth++;
> >      break;
> >        case OPCODE_ELSE:
> > @@ -1503,45 +1523,48 @@ void brw_vs_emit(struct brw_vs_compile *c )
> >           assert(if_depth > 0);
> >      brw_ENDIF(p, if_inst[--if_depth]);
> >      break;                 
> > -#if 0
> >        case OPCODE_BGNLOOP:
> >           loop_inst[loop_depth++] = brw_DO(p, BRW_EXECUTE_8);
> >           break;
> >        case OPCODE_BRK:
> > +    brw_set_predicate_control(p, get_predicate(inst));
> >           brw_BREAK(p);
> > -         brw_set_predicate_control(p, BRW_PREDICATE_NONE);
> > +    brw_set_predicate_control(p, BRW_PREDICATE_NONE);
> >           break;
> >        case OPCODE_CONT:
> > +    brw_set_predicate_control(p, get_predicate(inst));
> 
> Whitespace problems?

the original code is incorrectly using 8 spaces.

-- 
Eric Anholt
[email protected]                         [email protected]


Attachment: signature.asc
Description: This is a digitally signed message part

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Mesa3d-dev mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Reply via email to