add error checking to the GL_ATI_FRAGMENT_SHADER entrypoints. Fix bug with scope of ati fragment shader constants. Fix issues with specifying color/alpha instructions not pair-wise. Change internal representation of the shader (to better fit how the extension works, should make driver implementations simpler, and saves some memory). (still doesn't work correctly with doom3 and swrast, but not worse than before)

5 files changed, 401 insertions, 101 deletions

diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 17ef921738c..a37ba7ce98e 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -1746,6 +1746,7 @@ enum register_file
 struct vp_instruction;
 struct fp_instruction;
 struct atifs_instruction;
+struct atifs_setupinst;
 struct program_parameter_list;
 
 
@@ -1806,10 +1807,18 @@ struct fragment_program
 struct ati_fragment_shader
 {
    struct program Base;
-   struct atifs_instruction *Instructions;
+   struct atifs_instruction *Instructions[2];
+   struct atifs_setupinst *SetupInst[2];
    GLfloat Constants[8][4];
-   GLint NumPasses;
-   GLint cur_pass;
+   GLuint localConstDef;
+   GLubyte numArithInstr[2];
+   GLubyte regsAssigned[2];
+   GLubyte NumPasses;
+   GLubyte cur_pass;
+   GLubyte last_optype;
+   GLboolean interpinp1;
+   GLboolean isValid;
+   GLuint swizzlerq;
 };
 
 /**
@@ -1883,6 +1892,7 @@ struct gl_ati_fragment_shader_state
    GLboolean Enabled;
    GLboolean _Enabled;
    GLboolean Compiling;
+   GLfloat globalConstants[8][4];
    struct atifs_machine Machine;            /* machine state */
    struct ati_fragment_shader *Current;
 };
diff --git a/src/mesa/shader/atifragshader.c b/src/mesa/shader/atifragshader.c
index d8df8f518b6..ffd46b1095c 100644
--- a/src/mesa/shader/atifragshader.c
+++ b/src/mesa/shader/atifragshader.c
@@ -35,9 +35,24 @@
 extern struct program _mesa_DummyProgram;
 
 static void
-new_inst(struct ati_fragment_shader *prog)
+new_arith_inst(struct ati_fragment_shader *prog)
 {
-   prog->Base.NumInstructions++;
+/* set "default" instruction as not all may get defined.
+   there is no specified way to express a nop with ati fragment shaders we use
+   GL_NONE as the op enum and just set some params to 0 - so nothing to do here */
+   prog->numArithInstr[prog->cur_pass >> 1]++;
+}
+
+static void
+new_tex_inst(struct ati_fragment_shader *prog)
+{
+}
+
+static void match_pair_inst(struct ati_fragment_shader *curProg, GLuint optype)
+{
+   if (optype == curProg->last_optype) {
+      curProg->last_optype = 1;
+   }
 }
 
 #if MESA_DEBUG_ATI_FS
@@ -104,6 +119,35 @@ static void debug_op(GLint optype, GLuint arg_count, GLenum op, GLuint dst,
 }
 #endif
 
+static int check_arith_arg(struct ati_fragment_shader *curProg,
+			GLuint optype, GLuint arg, GLuint argRep)
+{
+   GET_CURRENT_CONTEXT(ctx);
+
+   if (((arg < GL_CON_0_ATI) || (arg > GL_CON_7_ATI)) &&
+      ((arg < GL_REG_0_ATI) || (arg > GL_REG_5_ATI)) &&
+      (arg != GL_ZERO) && (arg != GL_ONE) &&
+      (arg != GL_PRIMARY_COLOR_ARB) && (arg != GL_SECONDARY_INTERPOLATOR_ATI)) {
+      _mesa_error(ctx, GL_INVALID_ENUM, "C/AFragmentOpATI(arg)");
+      return 0;
+   }
+   if ((arg == GL_SECONDARY_INTERPOLATOR_ATI) && (((optype == 0) && (argRep == GL_ALPHA)) ||
+      ((optype == 1) && ((arg == GL_ALPHA) || (argRep == GL_NONE))))) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "C/AFragmentOpATI(sec_interp)");
+      return 0;
+   }
+   if ((arg == GL_SECONDARY_INTERPOLATOR_ATI) && (((optype == 0) && (argRep == GL_ALPHA)) ||
+      ((optype == 1) && ((arg == GL_ALPHA) || (argRep == GL_NONE))))) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "C/AFragmentOpATI(sec_interp)");
+      return 0;
+   }
+   if ((curProg->cur_pass == 1) &&
+      ((arg == GL_PRIMARY_COLOR_ARB) || (arg == GL_SECONDARY_INTERPOLATOR_ATI))) {
+      curProg->interpinp1 = GL_TRUE;
+   }
+   return 1;
+}
+
 GLuint GLAPIENTRY
 _mesa_GenFragmentShadersATI(GLuint range)
 {
@@ -111,6 +155,16 @@ _mesa_GenFragmentShadersATI(GLuint range)
    GLuint i;
    GET_CURRENT_CONTEXT(ctx);
 
+   if (range == 0) {
+      _mesa_error(ctx, GL_INVALID_VALUE, "glGenFragmentShadersATI(range)");
+      return 0;
+   }
+
+   if (ctx->ATIFragmentShader.Compiling) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "glGenFragmentShadersATI(insideShader)");
+      return 0;
+   }
+
    first = _mesa_HashFindFreeKeyBlock(ctx->Shared->Programs, range);
    for (i = 0; i < range; i++) {
       _mesa_HashInsert(ctx->Shared->Programs, first + i, &_mesa_DummyProgram);
@@ -126,6 +180,11 @@ _mesa_BindFragmentShaderATI(GLuint id)
    GET_CURRENT_CONTEXT(ctx);
    struct ati_fragment_shader *curProg = ctx->ATIFragmentShader.Current;
 
+   if (ctx->ATIFragmentShader.Compiling) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "glBindFragmentShaderATI(insideShader)");
+      return;
+   }
+
    FLUSH_VERTICES(ctx, _NEW_PROGRAM);
 
    if (curProg->Base.Id == id) {
@@ -173,6 +232,11 @@ _mesa_DeleteFragmentShaderATI(GLuint id)
 {
    GET_CURRENT_CONTEXT(ctx);
 
+   if (ctx->ATIFragmentShader.Compiling) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "glDeleteFragmentShaderATI(insideShader)");
+      return;
+   }
+
    if (id != 0) {
       struct program *prog = (struct program *)
 	 _mesa_HashLookup(ctx->Shared->Programs, id);
@@ -199,7 +263,7 @@ _mesa_DeleteFragmentShaderATI(GLuint id)
       _mesa_HashRemove(ctx->Shared->Programs, id);
       prog->RefCount--;
       if (prog->RefCount <= 0) {
-         ctx->Driver.DeleteProgram(ctx, prog);
+	 ctx->Driver.DeleteProgram(ctx, prog);
       }
 #endif
    }
@@ -208,16 +272,49 @@ _mesa_DeleteFragmentShaderATI(GLuint id)
 void GLAPIENTRY
 _mesa_BeginFragmentShaderATI(void)
 {
+   GLint i;
    GET_CURRENT_CONTEXT(ctx);
 
+   if (ctx->ATIFragmentShader.Compiling) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "glBeginFragmentShaderATI(insideShader)");
+      return;
+   }
+
+   /* if the shader was already defined free instructions and get new ones
+      (or, could use the same mem but would need to reinitialize) */
+   /* no idea if it's allowed to redefine a shader */
+   for (i = 0; i < MAX_NUM_PASSES_ATI; i++) {
+         if (ctx->ATIFragmentShader.Current->Instructions[i])
+            _mesa_free(ctx->ATIFragmentShader.Current->Instructions[i]);
+         if (ctx->ATIFragmentShader.Current->SetupInst[i])
+            _mesa_free(ctx->ATIFragmentShader.Current->SetupInst[i]);
+   }
+
    /* malloc the instructions here - not sure if the best place but its
       a start */
-   ctx->ATIFragmentShader.Current->Instructions =
-      (struct atifs_instruction *)
-      _mesa_calloc(sizeof(struct atifs_instruction) * MAX_NUM_PASSES_ATI *
-		   MAX_NUM_INSTRUCTIONS_PER_PASS_ATI * 2);
+   for (i = 0; i < MAX_NUM_PASSES_ATI; i++) {
+      ctx->ATIFragmentShader.Current->Instructions[i] =
+	 (struct atifs_instruction *)
+	 _mesa_calloc(sizeof(struct atifs_instruction) *
+		   (MAX_NUM_INSTRUCTIONS_PER_PASS_ATI));
+      ctx->ATIFragmentShader.Current->SetupInst[i] =
+	 (struct atifs_setupinst *)
+	 _mesa_calloc(sizeof(struct atifs_setupinst) *
+		   (MAX_NUM_FRAGMENT_REGISTERS_ATI));
+   }
 
+/* can't rely on calloc for initialization as it's possible to redefine a shader (?) */
+   ctx->ATIFragmentShader.Current->localConstDef = 0;
+   ctx->ATIFragmentShader.Current->numArithInstr[0] = 0;
+   ctx->ATIFragmentShader.Current->numArithInstr[1] = 0;
+   ctx->ATIFragmentShader.Current->regsAssigned[0] = 0;
+   ctx->ATIFragmentShader.Current->regsAssigned[1] = 0;
+   ctx->ATIFragmentShader.Current->NumPasses = 0;
    ctx->ATIFragmentShader.Current->cur_pass = 0;
+   ctx->ATIFragmentShader.Current->last_optype = 0;
+   ctx->ATIFragmentShader.Current->interpinp1 = GL_FALSE;
+   ctx->ATIFragmentShader.Current->isValid = GL_FALSE;
+   ctx->ATIFragmentShader.Current->swizzlerq = 0;
    ctx->ATIFragmentShader.Compiling = 1;
 }
 
@@ -225,25 +322,51 @@ void GLAPIENTRY
 _mesa_EndFragmentShaderATI(void)
 {
    GET_CURRENT_CONTEXT(ctx);
-#if MESA_DEBUG_ATI_FS
    struct ati_fragment_shader *curProg = ctx->ATIFragmentShader.Current;
-   GLint i;
+#if MESA_DEBUG_ATI_FS
+   GLint i, j;
 #endif
 
+   if (!ctx->ATIFragmentShader.Compiling) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "glEndFragmentShaderATI(outsideShader)");
+      return;
+   }
+   if (curProg->interpinp1 && (ctx->ATIFragmentShader.Current->cur_pass > 1)) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "glEndFragmentShaderATI(interpinfirstpass)");
+   /* according to spec, DON'T return here */
+   }
+
+   match_pair_inst(curProg, 0);
    ctx->ATIFragmentShader.Compiling = 0;
-   ctx->ATIFragmentShader.Current->NumPasses = ctx->ATIFragmentShader.Current->cur_pass;
+   ctx->ATIFragmentShader.Current->isValid = GL_TRUE;
+   if ((ctx->ATIFragmentShader.Current->cur_pass == 0) ||
+      (ctx->ATIFragmentShader.Current->cur_pass == 2)) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "glEndFragmentShaderATI(noarithinst)");
+   }
+   if (ctx->ATIFragmentShader.Current->cur_pass > 1)
+      ctx->ATIFragmentShader.Current->NumPasses = 2;
+   else ctx->ATIFragmentShader.Current->NumPasses = 1;
    ctx->ATIFragmentShader.Current->cur_pass=0;
 #if MESA_DEBUG_ATI_FS
-   for (i = 0; i < curProg->Base.NumInstructions; i++) {
-      GLuint op0 = curProg->Instructions[i].Opcode[0];
-      GLuint op1 = curProg->Instructions[i].Opcode[1];
-      const char *op0_enum = op0 > 5 ? _mesa_lookup_enum_by_nr(op0) : "0";
-      const char *op1_enum = op1 > 5 ? _mesa_lookup_enum_by_nr(op1) : "0";
-      GLuint count0 = curProg->Instructions[i].ArgCount[0];
-      GLuint count1 = curProg->Instructions[i].ArgCount[1];
-
-      fprintf(stderr, "%2d %04X %s %d %04X %s %d\n", i, op0, op0_enum, count0,
+   for (j = 0; j < MAX_NUM_PASSES_ATI; j++) {
+      for (i = 0; i < MAX_NUM_FRAGMENT_REGISTERS_ATI; i++) {
+	 GLuint op = curProg->SetupInst[j][i].Opcode;
+	 const char *op_enum = op > 5 ? _mesa_lookup_enum_by_nr(op) : "0";
+	 GLuint src = curProg->SetupInst[j][i].src;
+	 GLuint swizzle = curProg->SetupInst[j][i].swizzle;
+	 fprintf(stderr, "%2d %04X %s %d %04X\n", i, op, op_enum, src,
+	      swizzle);
+      }
+      for (i = 0; i < curProg->numArithInstr[j]; i++) {
+	 GLuint op0 = curProg->Instructions[j][i].Opcode[0];
+	 GLuint op1 = curProg->Instructions[j][i].Opcode[1];
+	 const char *op0_enum = op0 > 5 ? _mesa_lookup_enum_by_nr(op0) : "0";
+	 const char *op1_enum = op1 > 5 ? _mesa_lookup_enum_by_nr(op1) : "0";
+	 GLuint count0 = curProg->Instructions[j][i].ArgCount[0];
+	 GLuint count1 = curProg->Instructions[j][i].ArgCount[1];
+	 fprintf(stderr, "%2d %04X %s %d %04X %s %d\n", i, op0, op0_enum, count0,
 	      op1, op1_enum, count1);
+      }
    }
 #endif
 }
@@ -253,28 +376,63 @@ _mesa_PassTexCoordATI(GLuint dst, GLuint coord, GLenum swizzle)
 {
    GET_CURRENT_CONTEXT(ctx);
    struct ati_fragment_shader *curProg = ctx->ATIFragmentShader.Current;
-   GLint ci;
-   struct atifs_instruction *curI;
+   struct atifs_setupinst *curI;
+
+   if (!ctx->ATIFragmentShader.Compiling) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "glPassTexCoordATI(outsideShader)");
+      return;
+   }
 
-   if (ctx->ATIFragmentShader.Current->cur_pass==1)
-     ctx->ATIFragmentShader.Current->cur_pass=2;
+   if (curProg->cur_pass == 1) {
+      match_pair_inst(curProg, 0);
+      curProg->cur_pass = 2;
+   }
+   if ((curProg->cur_pass > 2) ||
+      ((1 << (dst - GL_REG_0_ATI)) & curProg->regsAssigned[curProg->cur_pass >> 1])) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "glPassTexCoord(pass)");
+      return;
+   }
+   if ((dst < GL_REG_0_ATI) || (dst > GL_REG_5_ATI)) {
+      _mesa_error(ctx, GL_INVALID_ENUM, "glPassTexCoordATI(dst)");
+      return;
+   }
+   if (((coord < GL_REG_0_ATI) || (coord > GL_REG_5_ATI)) &&
+       ((coord < GL_TEXTURE0_ARB) || (coord > GL_TEXTURE5_ARB))) {
+   /* is this texture5 or texture7? spec is a bit unclear there */
+      _mesa_error(ctx, GL_INVALID_ENUM, "glPassTexCoordATI(coord)");
+      return;
+   }
+   if ((curProg->cur_pass == 0) && (coord >= GL_REG_0_ATI)) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "glPassTexCoordATI(coord)");
+      return;
+   }
+   if ((swizzle < GL_SWIZZLE_STR_ATI) && (swizzle > GL_SWIZZLE_STQ_DQ_ATI)) {
+      _mesa_error(ctx, GL_INVALID_ENUM, "glPassTexCoordATI(swizzle)");
+      return;
+   }
+   if ((swizzle & 1) && (coord >= GL_REG_0_ATI)) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "glPassTexCoordATI(swizzle)");
+      return;
+   }
+   if (coord <= GL_TEXTURE5) {
+      if ((((curProg->swizzlerq >> (coord * 2)) & 3) != 0) &&
+	   (((swizzle & 1) + 1) != ((curProg->swizzlerq >> (coord * 2)) & 3))) {
+	 _mesa_error(ctx, GL_INVALID_OPERATION, "glPassTexCoordATI(swizzle)");
+	 return;
+      } else {
+	 curProg->swizzlerq |= (((swizzle & 1) + 1) << (coord * 2));
+      }
+   }
 
-   new_inst(curProg);
-   ci = curProg->Base.NumInstructions - 1;
-   /* some validation 
-      if ((swizzle != GL_SWIZZLE_STR_ATI) ||
-      (swizzle != GL_SWIZZLE_STQ_ATI) ||
-      (swizzle != GL_SWIZZLE_STR_DR_ATI) ||
-      (swizzle != GL_SWIZZLE_STQ_DQ_ATI))
-    */
+   curProg->regsAssigned[curProg->cur_pass >> 1] |=  1 << (dst - GL_REG_0_ATI);
+   new_tex_inst(curProg);
 
    /* add the instructions */
-   curI = &curProg->Instructions[ci];
+   curI = &curProg->SetupInst[curProg->cur_pass >> 1][dst - GL_REG_0_ATI];
 
-   curI->Opcode[0] = ATI_FRAGMENT_SHADER_PASS_OP;
-   curI->DstReg[0].Index = dst;
-   curI->SrcReg[0][0].Index = coord;
-   curI->DstReg[0].Swizzle = swizzle;
+   curI->Opcode = ATI_FRAGMENT_SHADER_PASS_OP;
+   curI->src = coord;
+   curI->swizzle = swizzle;
 
 #if MESA_DEBUG_ATI_FS
    _mesa_debug(ctx, "%s(%s, %s, %s)\n", __FUNCTION__,
@@ -288,24 +446,63 @@ _mesa_SampleMapATI(GLuint dst, GLuint interp, GLenum swizzle)
 {
    GET_CURRENT_CONTEXT(ctx);
    struct ati_fragment_shader *curProg = ctx->ATIFragmentShader.Current;
-   GLint ci;
-   struct atifs_instruction *curI;
+   struct atifs_setupinst *curI;
 
-   if (ctx->ATIFragmentShader.Current->cur_pass==1)
-     ctx->ATIFragmentShader.Current->cur_pass=2;
+   if (!ctx->ATIFragmentShader.Compiling) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "glSampleMapATI(outsideShader)");
+      return;
+   }
 
+   if (curProg->cur_pass == 1) {
+      match_pair_inst(curProg, 0);
+      curProg->cur_pass = 2;
+   }
+   if ((curProg->cur_pass > 2) ||
+      ((1 << (dst - GL_REG_0_ATI)) & curProg->regsAssigned[curProg->cur_pass >> 1])) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "glSampleMapATI(pass)");
+      return;
+   }
+   if ((dst < GL_REG_0_ATI) || (dst > GL_REG_5_ATI)) {
+      _mesa_error(ctx, GL_INVALID_ENUM, "glSampleMapATI(dst)");
+      return;
+   }
+   if (((interp < GL_REG_0_ATI) || (interp > GL_REG_5_ATI)) &&
+       ((interp < GL_TEXTURE0_ARB) || (interp > GL_TEXTURE5_ARB))) {
+   /* is this texture5 or texture7? spec is a bit unclear there */
+      _mesa_error(ctx, GL_INVALID_ENUM, "glSampleMapATI(interp)");
+      return;
+   }
+   if ((curProg->cur_pass == 0) && (interp >= GL_REG_0_ATI)) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "glSampleMapATI(interp)");
+      return;
+   }
+   if ((swizzle < GL_SWIZZLE_STR_ATI) && (swizzle > GL_SWIZZLE_STQ_DQ_ATI)) {
+      _mesa_error(ctx, GL_INVALID_ENUM, "glSampleMapATI(swizzle)");
+      return;
+   }
+   if ((swizzle & 1) && (interp >= GL_REG_0_ATI)) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "glSampleMapATI(swizzle)");
+      return;
+   }
+   if (interp <= GL_TEXTURE5) {
+      if ((((curProg->swizzlerq >> (interp * 2)) & 3) != 0) &&
+	   (((swizzle & 1) + 1) != ((curProg->swizzlerq >> (interp * 2)) & 3))) {
+	 _mesa_error(ctx, GL_INVALID_OPERATION, "glSampleMapATI(swizzle)");
+	 return;
+      } else {
+	 curProg->swizzlerq |= (((swizzle & 1) + 1) << (interp * 2));
+      }
+   }
 
-   new_inst(curProg);
+   curProg->regsAssigned[curProg->cur_pass >> 1] |=  1 << (dst - GL_REG_0_ATI);
+   new_tex_inst(curProg);
 
-   ci = curProg->Base.NumInstructions - 1;
    /* add the instructions */
-   curI = &curProg->Instructions[ci];
-
-   curI->Opcode[0] = ATI_FRAGMENT_SHADER_SAMPLE_OP;
-   curI->DstReg[0].Index = dst;
-   curI->DstReg[0].Swizzle = swizzle;
+   curI = &curProg->SetupInst[curProg->cur_pass >> 1][dst - GL_REG_0_ATI];
 
-   curI->SrcReg[0][0].Index = interp;
+   curI->Opcode = ATI_FRAGMENT_SHADER_SAMPLE_OP;
+   curI->src = interp;
+   curI->swizzle = swizzle;
 
 #if MESA_DEBUG_ATI_FS
    _mesa_debug(ctx, "%s(%s, %s, %s)\n", __FUNCTION__,
@@ -325,21 +522,92 @@ _mesa_FragmentOpXATI(GLint optype, GLuint arg_count, GLenum op, GLuint dst,
    struct ati_fragment_shader *curProg = ctx->ATIFragmentShader.Current;
    GLint ci;
    struct atifs_instruction *curI;
+   GLuint modtemp = dstMod & ~GL_SATURATE_BIT_ATI;
 
-   if (ctx->ATIFragmentShader.Current->cur_pass==0)
-     ctx->ATIFragmentShader.Current->cur_pass=1;
+   if (!ctx->ATIFragmentShader.Compiling) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "C/AFragmentOpATI(outsideShader)");
+      return;
+   }
 
-   /* decide whether this is a new instruction or not ... all color instructions are new */
-   if (optype == 0)
-      new_inst(curProg);
+   if (curProg->cur_pass==0)
+      curProg->cur_pass=1;
 
-   ci = curProg->Base.NumInstructions - 1;
+   else if (curProg->cur_pass==2)
+      curProg->cur_pass=3;
+
+   /* decide whether this is a new instruction or not ... all color instructions are new,
+      and alpha instructions might also be new if there was no preceding color inst */
+   if ((optype == 0) || (curProg->last_optype == optype)) {
+      if (curProg->numArithInstr[curProg->cur_pass >> 1] > 7) {
+	 _mesa_error(ctx, GL_INVALID_OPERATION, "C/AFragmentOpATI(instrCount)");
+	 return;
+      }
+      /* easier to do that here slight side effect invalid instr will still be inserted as nops */
+      match_pair_inst(curProg, optype);
+      new_arith_inst(curProg);
+   }
+   curProg->last_optype = optype;
+   ci = curProg->numArithInstr[curProg->cur_pass >> 1] - 1;
 
    /* add the instructions */
-   curI = &curProg->Instructions[ci];
+   curI = &curProg->Instructions[curProg->cur_pass >> 1][ci];
 
-   curI->Opcode[optype] = op;
+   /* error checking */
+   if ((dst < GL_REG_0_ATI) || (dst > GL_REG_5_ATI)) {
+      _mesa_error(ctx, GL_INVALID_ENUM, "C/AFragmentOpATI(dst)");
+      return;
+   }
+   if ((modtemp != GL_NONE) && (modtemp != GL_2X_BIT_ATI) &&
+      (modtemp != GL_4X_BIT_ATI) && (modtemp != GL_8X_BIT_ATI) &&
+      (modtemp != GL_HALF_BIT_ATI) && !(modtemp != GL_QUARTER_BIT_ATI) &&
+      (modtemp != GL_EIGHTH_BIT_ATI)) {
+      _mesa_error(ctx, GL_INVALID_ENUM, "C/AFragmentOpATI(dstMod)%x", modtemp);
+      return;
+   }
+   /* op checking? Actually looks like that's missing in the spec but we'll do it anyway */
+   if (((op < GL_ADD_ATI) || (op > GL_DOT2_ADD_ATI)) && !(op == GL_MOV_ATI)) {
+      _mesa_error(ctx, GL_INVALID_ENUM, "C/AFragmentOpATI(op)");
+      return;
+   }
+   if (optype == 1) {
+      if (((op == GL_DOT2_ADD_ATI) && (curI->Opcode[0] != GL_DOT2_ADD_ATI)) ||
+	 ((op == GL_DOT3_ATI) && (curI->Opcode[0] != GL_DOT3_ATI)) ||
+	 ((op == GL_DOT4_ATI) && (curI->Opcode[0] != GL_DOT4_ATI)) ||
+	 ((op != GL_DOT4_ATI) && (curI->Opcode[0] == GL_DOT4_ATI))) {
+	 _mesa_error(ctx, GL_INVALID_OPERATION, "AFragmentOpATI(op)");
+	 return;
+      }
+   }
+   if ((op == GL_DOT4_ATI) &&
+      (((arg1 == GL_SECONDARY_INTERPOLATOR_ATI) && ((arg1Rep == GL_ALPHA) || (arg1Rep == GL_NONE))) ||
+      (((arg2 == GL_SECONDARY_INTERPOLATOR_ATI) && ((arg2Rep == GL_ALPHA) || (arg2Rep == GL_NONE)))))) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, "C/AFragmentOpATI(sec_interp)");
+   }
+
+   if (!check_arith_arg(curProg, optype, arg1, arg1Rep)) {
+      return;
+   }
+   if (arg2) {
+      if (!check_arith_arg(curProg, optype, arg2, arg2Rep)) {
+	 return;
+      }
+   }
+   if (arg3) {
+      if (!check_arith_arg(curProg, optype, arg3, arg3Rep)) {
+	 return;
+      }
+      if ((arg1 >= GL_CON_0_ATI) && (arg1 <= GL_CON_7_ATI) &&
+	  (arg2 >= GL_CON_0_ATI) && (arg2 <= GL_CON_7_ATI) &&
+	  (arg3 >= GL_CON_0_ATI) && (arg3 <= GL_CON_7_ATI) &&
+	  (arg1 != arg2) && (arg1 != arg3) && (arg2 != arg3)) {
+	 _mesa_error(ctx, GL_INVALID_OPERATION, "C/AFragmentOpATI(3Consts)");
+	 return;
+      }
+   }
+
+   /* all ok - not all fully validated though (e.g. argNMod - spec doesn't say anything) */
 
+   curI->Opcode[optype] = op;
    curI->SrcReg[optype][0].Index = arg1;
    curI->SrcReg[optype][0].argRep = arg1Rep;
    curI->SrcReg[optype][0].argMod = arg1Mod;
@@ -360,7 +628,7 @@ _mesa_FragmentOpXATI(GLint optype, GLuint arg_count, GLenum op, GLuint dst,
    curI->DstReg[optype].Index = dst;
    curI->DstReg[optype].dstMod = dstMod;
    curI->DstReg[optype].dstMask = dstMask;
-   
+
 #if MESA_DEBUG_ATI_FS
    debug_op(optype, arg_count, op, dst, dstMask, dstMod, arg1, arg1Rep, arg1Mod, arg2, arg2Rep, arg2Mod, arg3, arg3Rep, arg3Mod);
 #endif
@@ -432,8 +700,20 @@ void GLAPIENTRY
 _mesa_SetFragmentShaderConstantATI(GLuint dst, const GLfloat * value)
 {
    GET_CURRENT_CONTEXT(ctx);
-   GLuint dstindex = dst - GL_CON_0_ATI;
-   struct ati_fragment_shader *curProg = ctx->ATIFragmentShader.Current;
 
-   COPY_4V(curProg->Constants[dstindex], value);
+   if ((dst < GL_CON_0_ATI) || (dst > GL_CON_7_ATI)) {
+      /* spec says nothing about what should happen here but we can't just segfault...*/
+      _mesa_error(ctx, GL_INVALID_ENUM, "glSetFragmentShaderConstantATI(dst)");
+      return;
+   }
+
+   GLuint dstindex = dst - GL_CON_0_ATI;
+   if (ctx->ATIFragmentShader.Compiling) {
+      struct ati_fragment_shader *curProg = ctx->ATIFragmentShader.Current;
+      COPY_4V(curProg->Constants[dstindex], value);
+      curProg->localConstDef |= 1 << dstindex;
+   }
+   else {
+      COPY_4V(ctx->ATIFragmentShader.globalConstants[dstindex], value);
+   }
 }
diff --git a/src/mesa/shader/atifragshader.h b/src/mesa/shader/atifragshader.h
index 82ac4746b6b..def441e6f7f 100644
--- a/src/mesa/shader/atifragshader.h
+++ b/src/mesa/shader/atifragshader.h
@@ -40,7 +40,7 @@ struct atifragshader_dst_register
 #define ATI_FRAGMENT_SHADER_PASS_OP  2
 #define ATI_FRAGMENT_SHADER_SAMPLE_OP 3
 
-/* two opcodes - one for color/one for alpha - also pass/sample */
+/* two opcodes - one for color/one for alpha */
 /* up to three source registers for most ops */
 struct atifs_instruction
 {
@@ -50,6 +50,14 @@ struct atifs_instruction
    struct atifragshader_dst_register DstReg[2];
 };
 
+/* different from arithmetic shader instruction */
+struct atifs_setupinst
+{
+   GLenum Opcode;
+   GLuint src;
+   GLenum swizzle;
+};
+
 extern GLuint GLAPIENTRY _mesa_GenFragmentShadersATI(GLuint range);
 
 extern void GLAPIENTRY _mesa_BindFragmentShaderATI(GLuint id);
diff --git a/src/mesa/shader/program.c b/src/mesa/shader/program.c
index 0ccc741dd0c..e7211981366 100644
--- a/src/mesa/shader/program.c
+++ b/src/mesa/shader/program.c
@@ -40,6 +40,7 @@
 #include "nvfragprog.h"
 #include "nvvertparse.h"
 #include "nvvertprog.h"
+#include "atifragshader.h"
 
 
 /**********************************************************************/
@@ -324,8 +325,13 @@ _mesa_delete_program(GLcontext *ctx, struct program *prog)
    }
    else if (prog->Target == GL_FRAGMENT_SHADER_ATI) {
       struct ati_fragment_shader *atifs = (struct ati_fragment_shader *)prog;
-      if (atifs->Instructions)
-	 _mesa_free(atifs->Instructions);
+      GLuint i;
+      for (i = 0; i < MAX_NUM_PASSES_ATI; i++) {
+	 if (atifs->Instructions[i])
+	    _mesa_free(atifs->Instructions[i]);
+	 if (atifs->SetupInst[i])
+	    _mesa_free(atifs->SetupInst[i]);
+      }
    }
 
    _mesa_free(prog);
diff --git a/src/mesa/swrast/s_atifragshader.c b/src/mesa/swrast/s_atifragshader.c
index d137b17d210..aeadefc8caa 100644
--- a/src/mesa/swrast/s_atifragshader.c
+++ b/src/mesa/swrast/s_atifragshader.c
@@ -258,18 +258,12 @@ struct ati_fs_opcode_st ati_fs_opcodes[] = {
 
 
 static void
-handle_pass_op(struct atifs_machine *machine, struct atifs_instruction *inst,
-	       const struct sw_span *span, GLuint column)
+handle_pass_op(struct atifs_machine *machine, struct atifs_setupinst *texinst,
+	       const struct sw_span *span, GLuint column, GLuint idx)
 {
-   GLuint idx = inst->DstReg[0].Index - GL_REG_0_ATI;
-   GLuint swizzle = inst->DstReg[0].Swizzle;
-   GLuint pass_tex = inst->SrcReg[0][0].Index;
-
-   /* if we get here after passing pass one then we are starting pass two - backup the registers */
-   if (machine->pass == 1) {
-      finish_pass(machine);
-      machine->pass = 2;
-   }
+   GLuint swizzle = texinst->swizzle;
+   GLuint pass_tex = texinst->src;
+
    if (pass_tex >= GL_TEXTURE0_ARB && pass_tex <= GL_TEXTURE7_ARB) {
       pass_tex -= GL_TEXTURE0_ARB;
       COPY_4V(machine->Registers[idx],
@@ -286,18 +280,11 @@ handle_pass_op(struct atifs_machine *machine, struct atifs_instruction *inst,
 
 static void
 handle_sample_op(GLcontext * ctx, struct atifs_machine *machine,
-		 struct atifs_instruction *inst, const struct sw_span *span,
-		 GLuint column)
+		 struct atifs_setupinst *texinst, const struct sw_span *span,
+		 GLuint column, GLuint idx)
 {
-   GLuint idx = inst->DstReg[0].Index - GL_REG_0_ATI;
-   GLuint swizzle = inst->DstReg[0].Swizzle;
-   GLuint sample_tex = inst->SrcReg[0][0].Index;
-
-   /* if we get here after passing pass one then we are starting pass two - backup the registers */
-   if (machine->pass == 1) {
-      finish_pass(machine);
-      machine->pass = 2;
-   }
+   GLuint swizzle = texinst->swizzle;
+   GLuint sample_tex = texinst->src;
 
    if (sample_tex >= GL_TEXTURE0_ARB && sample_tex <= GL_TEXTURE7_ARB) {
       sample_tex -= GL_TEXTURE0_ARB;
@@ -310,7 +297,6 @@ handle_sample_op(GLcontext * ctx, struct atifs_machine *machine,
       fetch_texel(ctx, machine->Registers[sample_tex], 0, sample_tex,
 		  machine->Registers[idx]);
    }
-
    apply_swizzle(machine, idx, swizzle);
 }
 
@@ -329,24 +315,28 @@ execute_shader(GLcontext * ctx,
 {
    GLuint pc;
    struct atifs_instruction *inst;
+   struct atifs_setupinst *texinst;
    GLint optype;
-   GLint i;
+   GLint i, j, pass;
    GLint dstreg;
    GLfloat src[2][3][4];
    GLfloat zeros[4] = { 0.0, 0.0, 0.0, 0.0 };
    GLfloat ones[4] = { 1.0, 1.0, 1.0, 1.0 };
    GLfloat dst[2][4], *dstp;
 
-   for (pc = 0; pc < shader->Base.NumInstructions; pc++) {
-      inst = &shader->Instructions[pc];
+   for (pass = 0; pass < shader->NumPasses; pass++) {
+      if (pass > 0)
+	 finish_pass(machine);
+      for (j = 0; j < MAX_NUM_FRAGMENT_REGISTERS_ATI; j++) {
+	 texinst = &shader->SetupInst[pass][j];
+	 if (texinst->Opcode == ATI_FRAGMENT_SHADER_PASS_OP)
+	    handle_pass_op(machine, texinst, span, column, j);
+	 else if (texinst->Opcode == ATI_FRAGMENT_SHADER_SAMPLE_OP)
+	    handle_sample_op(ctx, machine, texinst, span, column, j);
+      }
 
-      if (inst->Opcode[0] == ATI_FRAGMENT_SHADER_PASS_OP)
-	 handle_pass_op(machine, inst, span, column);
-      else if (inst->Opcode[0] == ATI_FRAGMENT_SHADER_SAMPLE_OP)
-	 handle_sample_op(ctx, machine, inst, span, column);
-      else {
-	 if (machine->pass == 0)
-	    machine->pass = 1;
+      for (pc = 0; pc < shader->numArithInstr[pass]; pc++) {
+	 inst = &shader->Instructions[pass][pc];
 
 	 /* setup the source registers for color and alpha ops */
 	 for (optype = 0; optype < 2; optype++) {
@@ -356,9 +346,15 @@ execute_shader(GLcontext * ctx,
 	       if (index >= GL_REG_0_ATI && index <= GL_REG_5_ATI)
 		  SETUP_SRC_REG(optype, i,
 				machine->Registers[index - GL_REG_0_ATI]);
-	       else if (index >= GL_CON_0_ATI && index <= GL_CON_7_ATI)
-		  SETUP_SRC_REG(optype, i,
+	       else if (index >= GL_CON_0_ATI && index <= GL_CON_7_ATI) {
+		  if (shader->localConstDef & (1 << (index - GL_CON_0_ATI))) {
+		     SETUP_SRC_REG(optype, i,
 				shader->Constants[index - GL_CON_0_ATI]);
+		  } else {
+		     SETUP_SRC_REG(optype, i,
+				ctx->ATIFragmentShader.globalConstants[index - GL_CON_0_ATI]);
+		  }
+	       }
 	       else if (index == GL_ONE)
 		  SETUP_SRC_REG(optype, i, ones);
 	       else if (index == GL_ZERO)