[Mesa-dev] [PATCH 10/11] intel/tools/error: Use do-while for field iterator loops.

Lionel Landwerlin lionel.g.landwerlin at intel.com
Sun Nov 12 12:01:30 UTC 2017


I think I sneaked the exact same fix in 
f5e5ca1e210c2e0f505ea154ca553275157dda73 (bottom of the patch).

Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>

On 12/11/17 08:35, Kenneth Graunke wrote:
> while loops skip the first field of the instruction/structure, which
> is not what the code intended.  It works out because the field we're
> looking for doesn't happen to be first, but we ought to do it right
> regardless.
>
> Found while writing the next patch, where Kernel Start Pointer is
> the first field of INTERFACE_DESCRIPTOR_DATA.
> ---
>   src/intel/tools/aubinator_error_decode.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/src/intel/tools/aubinator_error_decode.c b/src/intel/tools/aubinator_error_decode.c
> index 96b4989936e..09ca7c3a4ab 100644
> --- a/src/intel/tools/aubinator_error_decode.c
> +++ b/src/intel/tools/aubinator_error_decode.c
> @@ -275,7 +275,7 @@ decode(struct gen_spec *spec, struct gen_disasm *disasm,
>            struct gen_field_iterator iter;
>            gen_field_iterator_init(&iter, inst, p, false);
>   
> -         while (gen_field_iterator_next(&iter)) {
> +         do {
>               if (strcmp(iter.name, "Instruction Base Address") == 0) {
>                  uint64_t instr_base_address = strtol(iter.value, NULL, 16);
>                  current_instruction_buffer = NULL;
> @@ -285,7 +285,7 @@ decode(struct gen_spec *spec, struct gen_disasm *disasm,
>                     }
>                  }
>               }
> -         }
> +         } while (gen_field_iterator_next(&iter));
>         } else if (strcmp(inst->name,   "WM_STATE") == 0 ||
>                    strcmp(inst->name, "3DSTATE_PS") == 0 ||
>                    strcmp(inst->name, "3DSTATE_WM") == 0) {
> @@ -294,7 +294,7 @@ decode(struct gen_spec *spec, struct gen_disasm *disasm,
>            uint64_t ksp[3] = {0, 0, 0};
>            bool enabled[3] = {false, false, false};
>   
> -         while (gen_field_iterator_next(&iter)) {
> +         do {
>               if (strncmp(iter.name, "Kernel Start Pointer ",
>                           strlen("Kernel Start Pointer ")) == 0) {
>                  int idx = iter.name[strlen("Kernel Start Pointer ")] - '0';
> @@ -306,7 +306,7 @@ decode(struct gen_spec *spec, struct gen_disasm *disasm,
>               } else if (strcmp(iter.name, "32 Pixel Dispatch Enable") == 0) {
>                  enabled[2] = strcmp(iter.value, "true") == 0;
>               }
> -         }
> +         } while (gen_field_iterator_next(&iter));
>   
>            /* Reorder KSPs to be [8, 16, 32] instead of the hardware order. */
>            if (enabled[0] + enabled[1] + enabled[2] == 1) {
> @@ -353,7 +353,7 @@ decode(struct gen_spec *spec, struct gen_disasm *disasm,
>            bool is_simd8 = false; /* vertex shaders on Gen8+ only */
>            bool is_enabled = true;
>   
> -         while (gen_field_iterator_next(&iter)) {
> +         do {
>               if (strcmp(iter.name, "Kernel Start Pointer") == 0) {
>                  ksp = strtol(iter.value, NULL, 16);
>               } else if (strcmp(iter.name, "SIMD8 Dispatch Enable") == 0) {
> @@ -363,7 +363,7 @@ decode(struct gen_spec *spec, struct gen_disasm *disasm,
>               } else if (strcmp(iter.name, "Enable") == 0) {
>                  is_enabled = strcmp(iter.value, "true") == 0;
>               }
> -         }
> +         } while (gen_field_iterator_next(&iter));
>   
>            const char *type =
>               strcmp(inst->name,   "VS_STATE") == 0 ? "vertex shader" :




More information about the mesa-dev mailing list