[Mesa-dev] [PATCH 1/2] intel/genxml: Fix decoding of groups with fields smaller than a DWord.

[Kenneth Graunke]

Groups containing fields smaller than a DWord were not being decoded
correctly.  For example:

    <group count="32" start="32" size="4">
      <field name="Vertex Element Enables" start="0" end="3" type="uint"/>
    </group>

gen_field_iterator_next would properly walk over each element of the
array, incrementing group_iter, and calling iter_group_offset_bits()
to advance to the proper DWord.  However, the code to print the actual
values only considered iter->field->start/end, which are 0 and 3 in the
above example.  So it would always fetch bits 3:0 of the current DWord
when printing values, instead of advancing to each element of the array,
printing bits 0-3, 4-7, 8-11, and so on.

To fix this, we add new iter->start/end tracking, which properly
advances for each instance of a group's field.

Caught by Matt Turner while working on 3DSTATE_VF_COMPONENT_PACKING,
with a patch to convert it to use an array of bitfields (the example
above).

This also fixes the decoding of 3DSTATE_SBE's "Attribute Active
Component Format" fields.
---
 src/intel/common/gen_decoder.c | 24 ++++++++++++++----------
 src/intel/common/gen_decoder.h |  2 ++
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c
index 85880143f00..1bf69ac4f94 100644
--- a/src/intel/common/gen_decoder.c
+++ b/src/intel/common/gen_decoder.c
@@ -843,8 +843,12 @@ iter_advance_field(struct gen_field_iterator *iter)
        strncpy(iter->name, iter->field->name, sizeof(iter->name));
    else
       memset(iter->name, 0, sizeof(iter->name));
-   iter->dword = iter_group_offset_bits(iter, iter->group_iter) / 32 +
-      iter->field->start / 32;
+
+   int group_member_offset = iter_group_offset_bits(iter, iter->group_iter);
+
+   iter->start = group_member_offset + iter->field->start;
+   iter->end = group_member_offset + iter->field->end;
+   iter->dword = iter->start / 32;
    iter->struct_desc = NULL;
 
    return true;
@@ -861,7 +865,7 @@ gen_field_iterator_next(struct gen_field_iterator *iter)
    if (!iter_advance_field(iter))
       return false;
 
-   if ((iter->field->end - iter->field->start) > 32)
+   if ((iter->end - iter->start) > 32)
       v.qw = ((uint64_t) iter->p[iter->dword+1] << 32) | iter->p[iter->dword];
    else
       v.qw = iter->p[iter->dword];
@@ -871,13 +875,13 @@ gen_field_iterator_next(struct gen_field_iterator *iter)
    switch (iter->field->type.kind) {
    case GEN_TYPE_UNKNOWN:
    case GEN_TYPE_INT: {
-      uint64_t value = field(v.qw, iter->field->start, iter->field->end);
+      uint64_t value = field(v.qw, iter->start, iter->end);
       snprintf(iter->value, sizeof(iter->value), "%"PRId64, value);
       enum_name = gen_get_enum_name(&iter->field->inline_enum, value);
       break;
    }
    case GEN_TYPE_UINT: {
-      uint64_t value = field(v.qw, iter->field->start, iter->field->end);
+      uint64_t value = field(v.qw, iter->start, iter->end);
       snprintf(iter->value, sizeof(iter->value), "%"PRIu64, value);
       enum_name = gen_get_enum_name(&iter->field->inline_enum, value);
       break;
@@ -886,7 +890,7 @@ gen_field_iterator_next(struct gen_field_iterator *iter)
       const char *true_string =
          iter->print_colors ? "\e[0;35mtrue\e[0m" : "true";
       snprintf(iter->value, sizeof(iter->value), "%s",
-               field(v.qw, iter->field->start, iter->field->end) ?
+               field(v.qw, iter->start, iter->end) ?
                true_string : "false");
       break;
    }
@@ -896,7 +900,7 @@ gen_field_iterator_next(struct gen_field_iterator *iter)
    case GEN_TYPE_ADDRESS:
    case GEN_TYPE_OFFSET:
       snprintf(iter->value, sizeof(iter->value), "0x%08"PRIx64,
-               field_address(v.qw, iter->field->start, iter->field->end));
+               field_address(v.qw, iter->start, iter->end));
       break;
    case GEN_TYPE_STRUCT:
       snprintf(iter->value, sizeof(iter->value), "<struct %s>",
@@ -907,8 +911,8 @@ gen_field_iterator_next(struct gen_field_iterator *iter)
       break;
    case GEN_TYPE_UFIXED:
       snprintf(iter->value, sizeof(iter->value), "%f",
-               (float) field(v.qw, iter->field->start,
-                             iter->field->end) / (1 << iter->field->type.f));
+               (float) field(v.qw, iter->start,
+                             iter->end) / (1 << iter->field->type.f));
       break;
    case GEN_TYPE_SFIXED:
       /* FIXME: Sign extend extracted field. */
@@ -917,7 +921,7 @@ gen_field_iterator_next(struct gen_field_iterator *iter)
    case GEN_TYPE_MBO:
        break;
    case GEN_TYPE_ENUM: {
-      uint64_t value = field(v.qw, iter->field->start, iter->field->end);
+      uint64_t value = field(v.qw, iter->start, iter->end);
       snprintf(iter->value, sizeof(iter->value),
                "%"PRId64, value);
       enum_name = gen_get_enum_name(iter->field->type.gen_enum, value);
diff --git a/src/intel/common/gen_decoder.h b/src/intel/common/gen_decoder.h
index cfc9f2e3f15..12d4551a127 100644
--- a/src/intel/common/gen_decoder.h
+++ b/src/intel/common/gen_decoder.h
@@ -58,6 +58,8 @@ struct gen_field_iterator {
    struct gen_group *struct_desc;
    const uint32_t *p;
    int dword; /**< current field starts at &p[dword] */
+   int start; /**< current field starts at this bit number */
+   int end;   /**< current field ends at this bit number */
 
    int field_iter;
    int group_iter;
-- 
2.14.2