<div dir="ltr">On 29 October 2013 00:53, Tapani <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:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000"><div><div class="im">
On 10/28/2013 10:09 PM, Paul Berry wrote:<br>
</div></div><div><div class="h5"><blockquote type="cite"><div dir="ltr"><div><div><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+static uint32_t _unique_id(ir_variable *var)<br>
+{<br>
+ char buffer[256];<br>
+ _mesa_snprintf(buffer, 256, "%s_%p", var->name,
var);<br>
+ return _mesa_str_checksum(buffer);<br>
</blockquote>
<div><br>
</div>
<div>Two problems:<br>
<br>
</div>
<div>1. This is a lot of work to obtain a unique
identifier. The pointer is already unique; we should
just use that.<br>
</div>
<div><br>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
<br></div></div>
I was originally using a pointer but then there's the 32 vs 64bit
address problem. I wanted to avoid usage of pointers.<div class="im"><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div>
<div>
<div class="gmail_extra">
<div class="gmail_quote">
<div>2. There's no guarantee that the checksum of the
string will be unique. Because of the birthday
paradox, checksum collisions are probably going to
happen all the time.<br>
<br>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
<br></div>
I think there is because the pointer address is unique and is used
as part of the string.</div></blockquote><div><br></div><div>Yes, but the checksumming process destroys uniqueness (it has to, because the output of the checksum operation is a 32-bit int, and the input is a string. There are more than 2^32 possible strings, so by the pigeon hole principle at least one checksum must correspond to more than one possible input string). In the case of _mesa_str_checksum() (which is not a particularly good hash from a cryptographic perspective) the collisions are pretty easy to come by. For example, these were the first two collisions I found by writing a quick test program:<br>
<br> printf("checksum of \"%s\" is 0x%x\n", "foo_0000003c",<br> _mesa_str_checksum("foo_0000003c"));<br> printf("checksum of \"%s\" is 0x%x\n", "foo_000001a8",<br>
_mesa_str_checksum("foo_000001a8"));<br><br>checksum of "foo_0000003c" is 0x1360<br>checksum of "foo_000001a8" is 0x1360<br><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><div class="im"><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div>
<div>
<div class="gmail_extra">
<div class="gmail_quote">
<div>Instead of going to a lot of work to generate a
unique id for each ir_variable, I think we should just
serialize the ir_variable pointer address. If we are
trying to use the same binary format for 32-bit and
64-bit builds (see my clarifying question at the top
of this email), we'll need to extend the pointer to 64
bits before serializing it.<br>
<br>
</div>
<div>Alternatively, if you want to keep the unique ID's
small, we'll have to use some hashtable mechanism to
map each variable to its own ID.<br>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
<br></div>
OK, I will verify if this is a problem. For me it does not seem like
a problem as I'm using the address there.<div class="im"><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div>
<div>
<div class="gmail_extra">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+}<br>
+<br>
+<br>
+/* data required to serialize ir_variable */<br>
+struct ir_cache_variable_data<br>
</blockquote>
<div><br>
</div>
<div>As with glsl_type_data, I don't understand why we
need to have this class rather than just adding a
serialize() method to the ir_variable class.<br>
</div>
<div> </div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
<br></div>
This could be a serialize function in ir_cache_serialize.cpp. I did
not want to touch existing ir classes, I think it helps to have the
cache code separated and in one place. This is of course debatable,
every ir instruction could have serialize() and unserialize() to
deal with this.</div></blockquote><div><br></div><div>My chief worry with separating them is that people will forget to update the serialization code when they change the ir class, because the thing they need to update will be far away. This is particularly a problem when adding new members to the class (something we do quite frequently when implementing new GLSL features).<br>
</div></div></div></div>