glsl: Make ir_variable::num_state_slots and ir_variable::state_slots private
authorIan Romanick <ian.d.romanick@intel.com>
Thu, 15 May 2014 02:47:28 +0000 (19:47 -0700)
committerIan Romanick <ian.d.romanick@intel.com>
Tue, 30 Sep 2014 20:34:42 +0000 (13:34 -0700)
Also move num_state_slots inside ir_variable_data for better packing.

The payoff for this will come in a few more patches.

No change Valgrind massif results for a trimmed apitrace of dota2.

Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Matt Turner <mattst88@gmail.com>
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
src/glsl/builtin_variables.cpp
src/glsl/ir.h
src/glsl/ir_clone.cpp
src/glsl/ir_validate.cpp
src/glsl/linker.cpp
src/mesa/drivers/dri/i965/brw_fs.cpp
src/mesa/drivers/dri/i965/brw_shader.cpp
src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
src/mesa/program/ir_to_mesa.cpp
src/mesa/state_tracker/st_glsl_to_tgsi.cpp

index ddfdc1227c9350101223f57a88fc07d369246d17..c36d19831f8a4816a7cdaa95c715b1d764482fcc 100644 (file)
@@ -478,12 +478,9 @@ builtin_variable_generator::add_uniform(const glsl_type *type,
       &_mesa_builtin_uniform_desc[i];
 
    const unsigned array_count = type->is_array() ? type->length : 1;
-   uni->num_state_slots = array_count * statevar->num_elements;
 
    ir_state_slot *slots =
-      ralloc_array(uni, ir_state_slot, uni->num_state_slots);
-
-   uni->state_slots = slots;
+      uni->allocate_state_slots(array_count * statevar->num_elements);
 
    for (unsigned a = 0; a < array_count; a++) {
       for (unsigned j = 0; j < statevar->num_elements; j++) {
index 6b953ee0563aff2d98aeb73ecf0d4a2d7edcce9f..722df0baa1357b1a6d59e1d20a3393862973cc4f 100644 (file)
@@ -537,6 +537,37 @@ public:
       return this->max_ifc_array_access;
    }
 
+   inline unsigned get_num_state_slots() const
+   {
+      return this->data._num_state_slots;
+   }
+
+   inline void set_num_state_slots(unsigned n)
+   {
+      this->data._num_state_slots = n;
+   }
+
+   inline ir_state_slot *get_state_slots()
+   {
+      return this->state_slots;
+   }
+
+   inline const ir_state_slot *get_state_slots() const
+   {
+      return this->state_slots;
+   }
+
+   inline ir_state_slot *allocate_state_slots(unsigned n)
+   {
+      this->state_slots = ralloc_array(this, ir_state_slot, n);
+      this->data._num_state_slots = 0;
+
+      if (this->state_slots != NULL)
+         this->data._num_state_slots = n;
+
+      return this->state_slots;
+   }
+
    /**
     * Enable emitting extension warnings for this variable
     */
@@ -734,6 +765,10 @@ public:
       /** Image internal format if specified explicitly, otherwise GL_NONE. */
       uint16_t image_format;
 
+   private:
+      unsigned _num_state_slots;    /**< Number of state slots used */
+
+   public:
       /**
        * Storage location of the base of this variable
        *
@@ -786,22 +821,6 @@ public:
       friend class ir_variable;
    } data;
 
-   /**
-    * Built-in state that backs this uniform
-    *
-    * Once set at variable creation, \c state_slots must remain invariant.
-    * This is because, ideally, this array would be shared by all clones of
-    * this variable in the IR tree.  In other words, we'd really like for it
-    * to be a fly-weight.
-    *
-    * If the variable is not a uniform, \c num_state_slots will be zero and
-    * \c state_slots will be \c NULL.
-    */
-   /*@{*/
-   unsigned num_state_slots;    /**< Number of state slots used */
-   ir_state_slot *state_slots;  /**< State descriptors. */
-   /*@}*/
-
    /**
     * Value assigned in the initializer of a variable declared "const"
     */
@@ -833,6 +852,16 @@ private:
     */
    unsigned *max_ifc_array_access;
 
+   /**
+    * Built-in state that backs this uniform
+    *
+    * Once set at variable creation, \c state_slots must remain invariant.
+    *
+    * If the variable is not a uniform, \c _num_state_slots will be zero and
+    * \c state_slots will be \c NULL.
+    */
+   ir_state_slot *state_slots;
+
    /**
     * For variables that are in an interface block or are an instance of an
     * interface block, this is the \c GLSL_TYPE_INTERFACE type for that block.
index 86c0e3166d50eb2c614e59ce527cb35f0b944dc7..64019fe68aee0cacf7d874efb3096e4ef9b8924e 100644 (file)
@@ -53,15 +53,10 @@ ir_variable::clone(void *mem_ctx, struct hash_table *ht) const
 
    memcpy(&var->data, &this->data, sizeof(var->data));
 
-   var->num_state_slots = this->num_state_slots;
-   if (this->state_slots) {
-      /* FINISHME: This really wants to use something like talloc_reference, but
-       * FINISHME: ralloc doesn't have any similar function.
-       */
-      var->state_slots = ralloc_array(var, ir_state_slot,
-                                     this->num_state_slots);
-      memcpy(var->state_slots, this->state_slots,
-            sizeof(this->state_slots[0]) * var->num_state_slots);
+   if (this->get_state_slots()) {
+      ir_state_slot *s = var->allocate_state_slots(this->get_num_state_slots());
+      memcpy(s, this->get_state_slots(),
+             sizeof(s[0]) * var->get_num_state_slots());
    }
 
    if (this->constant_value)
index 96dd7bd9599d9b3196868dd5bc52b8362458640a..51598629117f317c55fdc7b04983e11f1c7536fd 100644 (file)
@@ -707,7 +707,7 @@ ir_validate::visit(ir_variable *ir)
 
    if (ir->data.mode == ir_var_uniform
        && strncmp(ir->name, "gl_", 3) == 0
-       && ir->state_slots == NULL) {
+       && ir->get_state_slots() == NULL) {
       printf("built-in uniform has no state\n");
       ir->print();
       abort();
index 2fece74d5529837f118a0741d058a13bf4b848aa..dbcc5b4578f634dc57478b2ffdd03602a7cc6b33 100644 (file)
@@ -1832,9 +1832,10 @@ update_array_sizes(struct gl_shader_program *prog)
             * Determine the number of slots per array element by dividing by
             * the old (total) size.
             */
-           if (var->num_state_slots > 0) {
-              var->num_state_slots = (size + 1)
-                 * (var->num_state_slots / var->type->length);
+            const unsigned num_slots = var->get_num_state_slots();
+           if (num_slots > 0) {
+              var->set_num_state_slots((size + 1)
+                                        * (num_slots / var->type->length));
            }
 
            var->type = glsl_type::get_array_instance(var->type->fields.array,
index 6ed3e95ef6ee71c1e43ebe5ed215c349927d1ea5..ab4ee34b41cb5786ea402b6348bf2fa8a7155b21 100644 (file)
@@ -1177,10 +1177,10 @@ fs_visitor::setup_uniform_values(ir_variable *ir)
 void
 fs_visitor::setup_builtin_uniform_values(ir_variable *ir)
 {
-   const ir_state_slot *const slots = ir->state_slots;
-   assert(ir->state_slots != NULL);
+   const ir_state_slot *const slots = ir->get_state_slots();
+   assert(slots != NULL);
 
-   for (unsigned int i = 0; i < ir->num_state_slots; i++) {
+   for (unsigned int i = 0; i < ir->get_num_state_slots(); i++) {
       /* This state reference has already been setup by ir_to_mesa, but we'll
        * get the same index back here.
        */
index 4f58f2865e1e6b0b78e858ab932966326d9417ab..6e17beb16c4f7b6002370939f19b6603d8a10ddc 100644 (file)
@@ -220,10 +220,10 @@ brw_link_shader(struct gl_context *ctx, struct gl_shader_program *shProg)
             || (strncmp(var->name, "gl_", 3) != 0))
            continue;
 
-        const ir_state_slot *const slots = var->state_slots;
-        assert(var->state_slots != NULL);
+        const ir_state_slot *const slots = var->get_state_slots();
+        assert(slots != NULL);
 
-        for (unsigned int i = 0; i < var->num_state_slots; i++) {
+        for (unsigned int i = 0; i < var->get_num_state_slots(); i++) {
            _mesa_add_state_reference(prog->Parameters,
                                      (gl_state_index *) slots[i].tokens);
         }
index f03cf4f1f67cbef437533fe753c1d3e95fa1f559..ef923ddcc03bbcf8dc9beac5f5586baed63f1dce 100644 (file)
@@ -745,10 +745,10 @@ vec4_visitor::setup_uniform_clipplane_values()
 void
 vec4_visitor::setup_builtin_uniform_values(ir_variable *ir)
 {
-   const ir_state_slot *const slots = ir->state_slots;
-   assert(ir->state_slots != NULL);
+   const ir_state_slot *const slots = ir->get_state_slots();
+   assert(slots != NULL);
 
-   for (unsigned int i = 0; i < ir->num_state_slots; i++) {
+   for (unsigned int i = 0; i < ir->get_num_state_slots(); i++) {
       /* This state reference has already been setup by ir_to_mesa,
        * but we'll get the same index back here.  We can reference
        * ParameterValues directly, since unlike brw_fs.cpp, we never
index 293fe34b9e427c9b00b207d5d47dd348b880a5a1..b3e04d7b60584cce810acd60df335bbde8a5564e 100644 (file)
@@ -682,8 +682,8 @@ ir_to_mesa_visitor::visit(ir_variable *ir)
 
    if (ir->data.mode == ir_var_uniform && strncmp(ir->name, "gl_", 3) == 0) {
       unsigned int i;
-      const ir_state_slot *const slots = ir->state_slots;
-      assert(ir->state_slots != NULL);
+      const ir_state_slot *const slots = ir->get_state_slots();
+      assert(slots != NULL);
 
       /* Check if this statevar's setup in the STATE file exactly
        * matches how we'll want to reference it as a
@@ -691,7 +691,7 @@ ir_to_mesa_visitor::visit(ir_variable *ir)
        * temporary storage and hope that it'll get copy-propagated
        * out.
        */
-      for (i = 0; i < ir->num_state_slots; i++) {
+      for (i = 0; i < ir->get_num_state_slots(); i++) {
         if (slots[i].swizzle != SWIZZLE_XYZW) {
            break;
         }
@@ -699,7 +699,7 @@ ir_to_mesa_visitor::visit(ir_variable *ir)
 
       variable_storage *storage;
       dst_reg dst;
-      if (i == ir->num_state_slots) {
+      if (i == ir->get_num_state_slots()) {
         /* We'll set the index later. */
         storage = new(mem_ctx) variable_storage(ir, PROGRAM_STATE_VAR, -1);
         this->variables.push_tail(storage);
@@ -710,7 +710,7 @@ ir_to_mesa_visitor::visit(ir_variable *ir)
          * of the type.  However, this had better match the number of state
          * elements that we're going to copy into the new temporary.
          */
-        assert((int) ir->num_state_slots == type_size(ir->type));
+        assert((int) ir->get_num_state_slots() == type_size(ir->type));
 
         storage = new(mem_ctx) variable_storage(ir, PROGRAM_TEMPORARY,
                                                 this->next_temp);
@@ -721,7 +721,7 @@ ir_to_mesa_visitor::visit(ir_variable *ir)
       }
 
 
-      for (unsigned int i = 0; i < ir->num_state_slots; i++) {
+      for (unsigned int i = 0; i < ir->get_num_state_slots(); i++) {
         int index = _mesa_add_state_reference(this->prog->Parameters,
                                               (gl_state_index *)slots[i].tokens);
 
@@ -741,7 +741,7 @@ ir_to_mesa_visitor::visit(ir_variable *ir)
       }
 
       if (storage->file == PROGRAM_TEMPORARY &&
-         dst.index != storage->index + (int) ir->num_state_slots) {
+         dst.index != storage->index + (int) ir->get_num_state_slots()) {
         linker_error(this->shader_program,
                      "failed to load builtin uniform `%s' "
                      "(%d/%d regs loaded)\n",
index 57f43a67c5d010485b590cacc8734dbf52d3afcf..a0da9f6353c0c35ae810f048464948c1d835d90c 100644 (file)
@@ -1072,8 +1072,8 @@ glsl_to_tgsi_visitor::visit(ir_variable *ir)
 
    if (ir->data.mode == ir_var_uniform && strncmp(ir->name, "gl_", 3) == 0) {
       unsigned int i;
-      const ir_state_slot *const slots = ir->state_slots;
-      assert(ir->state_slots != NULL);
+      const ir_state_slot *const slots = ir->get_state_slots();
+      assert(slots != NULL);
 
       /* Check if this statevar's setup in the STATE file exactly
        * matches how we'll want to reference it as a
@@ -1081,7 +1081,7 @@ glsl_to_tgsi_visitor::visit(ir_variable *ir)
        * temporary storage and hope that it'll get copy-propagated
        * out.
        */
-      for (i = 0; i < ir->num_state_slots; i++) {
+      for (i = 0; i < ir->get_num_state_slots(); i++) {
          if (slots[i].swizzle != SWIZZLE_XYZW) {
             break;
          }
@@ -1089,7 +1089,7 @@ glsl_to_tgsi_visitor::visit(ir_variable *ir)
 
       variable_storage *storage;
       st_dst_reg dst;
-      if (i == ir->num_state_slots) {
+      if (i == ir->get_num_state_slots()) {
          /* We'll set the index later. */
          storage = new(mem_ctx) variable_storage(ir, PROGRAM_STATE_VAR, -1);
          this->variables.push_tail(storage);
@@ -1100,7 +1100,7 @@ glsl_to_tgsi_visitor::visit(ir_variable *ir)
           * of the type.  However, this had better match the number of state
           * elements that we're going to copy into the new temporary.
           */
-         assert((int) ir->num_state_slots == type_size(ir->type));
+         assert((int) ir->get_num_state_slots() == type_size(ir->type));
 
          dst = st_dst_reg(get_temp(ir->type));
 
@@ -1110,7 +1110,7 @@ glsl_to_tgsi_visitor::visit(ir_variable *ir)
       }
 
 
-      for (unsigned int i = 0; i < ir->num_state_slots; i++) {
+      for (unsigned int i = 0; i < ir->get_num_state_slots(); i++) {
          int index = _mesa_add_state_reference(this->prog->Parameters,
                                               (gl_state_index *)slots[i].tokens);
 
@@ -1135,7 +1135,7 @@ glsl_to_tgsi_visitor::visit(ir_variable *ir)
       }
 
       if (storage->file == PROGRAM_TEMPORARY &&
-          dst.index != storage->index + (int) ir->num_state_slots) {
+          dst.index != storage->index + (int) ir->get_num_state_slots()) {
          fail_link(this->shader_program,
                   "failed to load builtin uniform `%s'  (%d/%d regs loaded)\n",
                   ir->name, dst.index - storage->index,