[Mesa-dev] [PATCH 88/88] i965: handle 32bit and 64bit version of shader cache objects

[Timothy Arceri]

On Wed, 2016-09-28 at 11:42 -0700, Kenneth Graunke wrote:
> On Wednesday, September 28, 2016 9:00:50 AM PDT Eric Anholt wrote:
> > 
> > Timothy Arceri <timothy.arceri at collabora.com> writes:
> > 
> > > 
> > > On Sun, 2016-09-25 at 19:51 -0700, Kenneth Graunke wrote:
> > > > 
> > > > On Saturday, September 24, 2016 3:26:09 PM PDT Timothy Arceri
> > > > wrote:
> > > > > 
> > > > > 
> > > > > Pointers will have different lengths so we simple create a
> > > > > different
> > > > > sha1 for each platform.
> > > > 
> > > > I don't understand...you're putting pointers in the blob?  How
> > > > can
> > > > that
> > > > work?  You obviously can't reload things from the cache as
> > > > actual
> > > > pointers, so you've got to be doing some kind of remapping from
> > > > a
> > > > "pointer" in the blob to a real pointer.
> > > > 
> > > > At that point, can't you just make the blob always use a
> > > > uint64_t for
> > > > pointers, and just pad with zeroes on 32-bit systems?  Then we
> > > > could
> > > > use the same copy for both...
> > > 
> > > We probably could but we are currently using intptr_t for
> > > storing/restoring pointers and ptrdiff_t during remapping as we
> > > probably should be. Thoughts?
> > 
> > Use consistent uint64_t on disk and remap to 32/64 on the way back
> > in,
> > imo.
> 
> I agree...we should use explicitly sized types for anything on disk,
> rather than ones which can vary based on architecture.

Hi Ken/Eric,

The intention of this patch was to try avoid the whole problem of
varying sizes based on architecture. I guess it could still cause
problems if a compiler was to redefine sizes or someone compiled Mesa
with a different compiler that has different sizes but these scenarios
seem unlikely.

I've made the changes based on your initial suggestion but I've hit
more complications and I'm starting to feel like this patch is a
simpler and safer way to go.

When remapping the uniform remap tables for example I was making use of
the size_of(gl_uniform_storage) where gl_uniform_storage itself
contains pointers that vary in size on different platforms. This can be
solved by storing an offest that is a multiple of
size_of(gl_uniform_storage) itself but it becomes more complicated when
restoring something like the parameter lists which contain pointers to
different types. I'm feeling like doing things this way opens us up to
the possiblity of future bugs that users would find difficult to report
accurately for little gain.

Remap table restore code: 

   for (unsigned i = 0; i < prog->NumUniformRemapTable; i++) {
      uint64_t uni_ptr = blob_read_uint64(metadata);
      if (uni_ptr == (uint64_t) INACTIVE_UNIFORM_EXPLICIT_LOCATION ||
          uni_ptr == (uint64_t) NULL) {
         prog->UniformRemapTable[i] = (gl_uniform_storage *) uni_ptr;
      } else {
         intptr_t uni_offset =
            (uni_ptr - uni_store_base) / sizeof(gl_uniform_storage);
         prog->UniformRemapTable[i] = prog->UniformStorage + uni_offset;
      }
   }

Then there is there is the other problem of writing/restoring whole
structs for example:

   blob_copy_bytes(metadata, (uint8_t *) ltf->Buffers,
                   sizeof(struct gl_transform_feedback_buffer) *
                      MAX_FEEDBACK_BUFFERS);

Mostly this shouldn't be a problem but since we don't use uint32_t etc
everywhere it's not guaranteed to be so.

If you guys still think this is the best way to go I'll rework the
remap table restore code. I just thought I'd get more feedback before
going too far forward.

Thanks,
Tim


> 
> --Ken
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev