<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 10/29/2013 03:48 PM, Paul Berry
wrote:<br>
</div>
<blockquote
cite="mid:CA+yLL66eqmp8L5azxrKpFLf5DH_NibW3XwTRcjegG1K-eJkA0g@mail.gmail.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html;
charset=ISO-8859-1">
<div dir="ltr">On 29 October 2013 00:53, Tapani <span dir="ltr"><<a
moz-do-not-send="true" 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>
</div>
</blockquote>
<br>
OK, I'll put the address itself back in use then.<br>
<br>
<blockquote
cite="mid:CA+yLL66eqmp8L5azxrKpFLf5DH_NibW3XwTRcjegG1K-eJkA0g@mail.gmail.com"
type="cite">
<div dir="ltr">
<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">
<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>
</blockquote>
<br>
I understand and I've been worried about this from the
maintainability point of view as well. I've thought to add some
documentation on the ir.h file about this.<br>
<br>
// Tapani<br>
<br>
</body>
</html>