<div dir="ltr">On 5 November 2013 23:45, Tapani Pälli <span dir="ltr"><<a href="mailto:tapani.palli@intel.com" target="_blank">tapani.palli@intel.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><div>
<div>On 11/05/2013 07:36 PM, Paul Berry
wrote:<br>
</div>
</div><blockquote type="cite">
<div dir="ltr"><div>On 1 November 2013 02:16, Tapani Pälli <span dir="ltr"><<a href="mailto:tapani.palli@intel.com" target="_blank">tapani.palli@intel.com</a>></span>
wrote:<br>
</div><div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><br></div></blockquote></div></div></div></blockquote><div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra">
<div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+/**<br>
+ * Function to create an unique string for a ir_variable.
This is<br>
+ * used by variable dereferences to indicate the exact
ir_variable<br>
+ * when deserialization happens.<br>
</blockquote>
<div><br>
</div>
<div>I still don't understand why we don't just use the
pointer for this purpose. It's unique, and it takes up
much less storage than <name>_<decimal
address>.<br>
</div>
<div> </div>
</div>
</div>
</div>
</blockquote>
<br></div>
We need to construct a unique name to refer to when reading so this
is constructed here already. It could be also just a counter like is
used somewhere else (to have assignment_tmp@1, assignment_tmp@2) but
I wanted to keep this code minimal as it's just unique naming.</div></blockquote><div><br></div><div>I'm sorry, there's still something I'm not understanding. Before serialization, the names are not unique. So I don't see why we need to create unique names when reading. The goal should be to reproduce the IR data structure that was saved as closely as possible.<br>
<br>It seems like storing the address of the ir_variable (or some other suitably determined unique numeric id) should be sufficient for this. When we serialize the ir_variable, we write out its unique numeric id. When we serialize an ir_dereference_variable, we write out the unique numeric id of the ir_variable it refers to. When deserializing an ir_variable, we store the mapping from its unique numeric id to its new address in memory in a hash table. When deserializing an ir_dereference_variable, we look up the unique numeric id to find the address of the ir_variable that's already been deserialized.<br>
<br>It doesn't seem to me that uniqueness of names is necessary anywhere in that process. What am I missing?<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000"><div><br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+ for (unsigned i = 0; i < ir->num_state_slots;
i++) {<br>
+
blob.write_int32(&ir->state_slots[i].swizzle);<br>
</blockquote>
<div><br>
</div>
<div>Swizzles are unsigned 8-bit values. This should be
write_uint8.<br>
</div>
<div> </div>
</div>
</div>
</div>
</blockquote>
<br></div>
OK, maybe the struct could also be changed to have only 8 bits
instead of a int then.</div></blockquote><div><br></div><div>I could get behind that change.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><div><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">+ CACHE_DEBUG("write %d
prototypes\n", total);<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+ foreach_list_const(node, shader->ir) {<br>
+ ir_instruction *const inst = (ir_instruction *)
node;<br>
+ if (inst->as_variable())<br>
+ if (save(inst))<br>
+ goto write_errors;<br>
+ }<br>
+<br>
+ foreach_list_const(node, shader->ir) {<br>
+ ir_instruction *const inst = (ir_instruction *)
node;<br>
+ if (inst->as_function())<br>
+ if (save(inst))<br>
+ goto write_errors;<br>
+ }<br>
</blockquote>
<div><br>
</div>
<div>Why is it necessary to save the variables and
instructions first?<br>
</div>
</div>
</div>
</div>
</blockquote>
<br></div>
This is because during parsing we might encounter a call to a
function that does not exist yet, same goes for variable references.
Another way would be to modify the reading side so that it makes 2
passes over the data but I took this way as originally reader did
not use mmap so it was just simpler.<br></div></blockquote><div><br></div><div>Ok, I think you are correct about the functions. But I believe for variables, the ir_variable always appears in the IR before any references to it. Can someone confirm this? (Ken or Ian perhaps?)<br>
</div></div></div></div>