[Mesa-dev] [PATCH 2/2] gallium: avoid generating loop overhead in case of a non cubemap texture.

[Matt Turner]

On Sun, Jan 8, 2012 at 5:15 PM, Vincent Lejeune <vljn at ovi.com> wrote:
> ---
>  src/mesa/state_tracker/st_cb_texture.c |   82 ++++++++++++++++++++++++--------
>  1 files changed, 62 insertions(+), 20 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c
> index 3cd1b2b..eeb6e3a 100644
> --- a/src/mesa/state_tracker/st_cb_texture.c
> +++ b/src/mesa/state_tracker/st_cb_texture.c
> @@ -1197,6 +1197,60 @@ copy_image_data_to_texture(struct st_context *st,
>    pipe_resource_reference(&stImage->pt, stObj->pt);
>  }
>
> +
> +static void inline
> +import_texture_given_level(struct st_context *st, struct st_texture_object *stObj,const struct pipe_resource *stObjpt,struct gl_texture_image **img, const GLuint level)
> +{
> +      struct st_texture_image *stImage =
> +         st_texture_image(img[level]);
> +
> +       /* Need to import images in main memory or held in other textures.
> +       */
> +      if (stImage && stObjpt != stImage->pt) {
> +         if (level == 0 ||
> +             (stImage->base.Width == u_minify(stObj->width0, level) &&
> +              stImage->base.Height == u_minify(stObj->height0, level) &&
> +              stImage->base.Depth == u_minify(stObj->depth0, level))) {
> +            /* src image fits expected dest mipmap level size */
> +            copy_image_data_to_texture(st, stObj, level, stImage);
> +         }
> +      }
> +}
> +
> +
> +static void inline
> +import_texture(struct st_context *st, struct st_texture_object *stObj, const GLuint face)
> +{
> +   const GLuint baselevel = stObj->base.BaseLevel;
> +   const GLuint levelcount = stObj->lastLevel - baselevel;
> +   const struct pipe_resource* stObjpt = stObj->pt;
> +
> +   struct gl_texture_image **img = &(stObj->base.Image[face][baselevel]);
> +
> +   for (unsigned n = (levelcount + 7) / 8;n>0;n--) {
> +      import_texture_given_level(st,stObj,stObjpt,img,0);

A couple of style nits: spaces after the comma in function arguments,
and spaces after commas and semi-colons in for-loops.

> +      import_texture_given_level(st,stObj,stObjpt,img,1);
> +      import_texture_given_level(st,stObj,stObjpt,img,2);
> +      import_texture_given_level(st,stObj,stObjpt,img,3);
> +      import_texture_given_level(st,stObj,stObjpt,img,4);
> +      import_texture_given_level(st,stObj,stObjpt,img,5);
> +      import_texture_given_level(st,stObj,stObjpt,img,6);
> +      import_texture_given_level(st,stObj,stObjpt,img,7);
> +      img += 8;
> +   }
> +
> +   switch (levelcount % 8) {
> +      case 7: import_texture_given_level(st,stObj,stObjpt,img,6);
> +      case 6: import_texture_given_level(st,stObj,stObjpt,img,5);
> +      case 5: import_texture_given_level(st,stObj,stObjpt,img,4);
> +      case 4: import_texture_given_level(st,stObj,stObjpt,img,3);
> +      case 3: import_texture_given_level(st,stObj,stObjpt,img,2);
> +      case 2: import_texture_given_level(st,stObj,stObjpt,img,1);
> +      case 1: import_texture_given_level(st,stObj,stObjpt,img,0);
> +      case 0: break;
> +   }
> +}
> +
>  static void inline
>  set_mipmap_level(struct st_texture_object *stObj)
>  {
> @@ -1219,8 +1273,6 @@ st_finalize_texture_body(struct gl_context *ctx,
>                struct st_texture_object *stObj)
>  {
>    struct st_context *st = st_context(ctx);
> -   const GLuint nr_faces = (stObj->base.Target == GL_TEXTURE_CUBE_MAP) ? 6 : 1;
> -   GLuint face;
>    struct st_texture_image *firstImage;
>    enum pipe_format firstImageFormat;
>    GLuint ptWidth, ptHeight, ptDepth, ptLayers;
> @@ -1305,25 +1357,15 @@ st_finalize_texture_body(struct gl_context *ctx,
>
>    /* Pull in any images not in the object's texture:
>     */
> -   for (face = 0; face < nr_faces; face++) {
> -      GLuint level;
> -      for (level = stObj->base.BaseLevel; level <= stObj->lastLevel; level++) {
> -         struct st_texture_image *stImage =
> -            st_texture_image(stObj->base.Image[face][level]);
> +   import_texture(st, stObj, 0);
> +   if (stObj->base.Target != GL_TEXTURE_CUBE_MAP)
> +      return GL_TRUE;
>
> -         /* Need to import images in main memory or held in other textures.
> -          */
> -         if (stImage && stObj->pt != stImage->pt) {
> -            if (level == 0 ||
> -                (stImage->base.Width == u_minify(stObj->width0, level) &&
> -                 stImage->base.Height == u_minify(stObj->height0, level) &&
> -                 stImage->base.Depth == u_minify(stObj->depth0, level))) {
> -               /* src image fits expected dest mipmap level size */
> -               copy_image_data_to_texture(st, stObj, level, stImage);
> -            }
> -         }
> -      }
> -   }
> +   import_texture(st, stObj, 1);
> +   import_texture(st, stObj, 2);
> +   import_texture(st, stObj, 3);
> +   import_texture(st, stObj, 4);
> +   import_texture(st, stObj, 5);
>
>    return GL_TRUE;
>  }
> --

Is manually unrolling this significantly better than something like

+   import_texture(st, stObj, 0);
+   if (stObj->base.Target != GL_TEXTURE_CUBE_MAP)
+      return GL_TRUE;

and then beginning the for (face = 0; face < nr_faces; face++) loop
with face = 1 instead of 0?

Matt