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)

3 files changed, 357 insertions, 63 deletions

diff --git a/src/mesa/shader/atifragshader.c b/src/mesa/shader/atifragshader.c
index d8df8f518b..ffd46b1095 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 82ac4746b6..def441e6f7 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 0ccc741dd0..e721198136 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);