[Beignet] [PATCH] do not serialize zero image/sampler info into binary

[Guo, Yejun]

I see, thanks.


Thanks
Yejun


-----Original Message-----
From: Zhigang Gong [mailto:zhigang.gong at linux.intel.com] 
Sent: Friday, May 09, 2014 10:31 AM
To: Guo, Yejun
Cc: beignet at lists.freedesktop.org
Subject: Re: [Beignet] [PATCH] do not serialize zero image/sampler info into binary

Not typo, that two maps always have the same count of elements.

On Fri, May 09, 2014 at 03:10:04AM +0000, Guo, Yejun wrote:
> Good change Zhigang, just to confirm that this is your typo? 
> 
> > +
> > +    bool empty() {
> > +      return (regMap.size() + indexMap.size() == 0 ? true : false);
> > +    }
> > +
>   better to use:     bool empty() const { return regMap.empty(); }
> 
> the correct is:  return regMap.empty() && indexMap.empty();
> 
> Thanks
> Yejun
> 
> 
> -----Original Message-----
> From: Zhigang Gong [mailto:zhigang.gong at linux.intel.com]
> Sent: Friday, May 09, 2014 10:22 AM
> To: Guo, Yejun
> Cc: beignet at lists.freedesktop.org
> Subject: Re: [Beignet] [PATCH] do not serialize zero image/sampler 
> info into binary
> 
> I agree to remove those data if there is no image/sampler exist.
> 
> Some minor comments as below, I will change it directly and push latter.
> Thanks for the patch.
> 
> On Wed, May 07, 2014 at 03:34:37AM +0800, Guo Yejun wrote:
> > if there is no image/sampler used in kernel source, it is not 
> > necessary to serialize the zero image/sampler info into kernel binary.
> > 
> > Signed-off-by: Guo Yejun <yejun.guo at intel.com>
> > ---
> >  backend/src/backend/program.cpp | 8 ++++++--  
> > backend/src/backend/program.hpp | 4 ++--
> >  backend/src/ir/image.hpp        | 5 +++++
> >  backend/src/ir/sampler.hpp      | 4 ++++
> >  4 files changed, 17 insertions(+), 4 deletions(-)
> > 
> > diff --git a/backend/src/backend/program.cpp 
> > b/backend/src/backend/program.cpp index 79622e0..bdc7d34 100644
> > --- a/backend/src/backend/program.cpp
> > +++ b/backend/src/backend/program.cpp
> > @@ -267,7 +267,7 @@ namespace gbe {
> >      OUT_UPDATE_SZ(compileWgSize[1]);
> >      OUT_UPDATE_SZ(compileWgSize[2]);
> >      /* samplers. */
> > -    if (samplerSet) {
> > +    if (!samplerSet->empty()) {   //samplerSet is always valid, allocated in Function::Function
> >        has_samplerset = 1;
> >        OUT_UPDATE_SZ(has_samplerset);
> >        size_t sz = samplerSet->serializeToBin(outs); @@ -280,7 
> > +280,7 @@ namespace gbe {
> >      }
> >  
> >      /* images. */
> > -    if (imageSet) {
> > +    if (!imageSet->empty()) {   //imageSet is always valid, allocated in Function::Function
> >        has_imageset = 1;
> >        OUT_UPDATE_SZ(has_imageset);
> >        size_t sz = imageSet->serializeToBin(outs); @@ -369,6 +369,8 
> > @@ namespace gbe {
> >  
> >        total_size += sz;
> >      }
> > +    else
> > +      samplerSet = NULL;
> >  
> >      IN_UPDATE_SZ(has_imageset);
> >      if (has_imageset) {
> > @@ -380,6 +382,8 @@ namespace gbe {
> >  
> >        total_size += sz;
> >      }
> > +    else
> > +      imageSet = NULL;
> >  
> >      IN_UPDATE_SZ(code_size);
> >      if (code_size) {
> > diff --git a/backend/src/backend/program.hpp 
> > b/backend/src/backend/program.hpp index 0e01256..fe945a6 100644
> > --- a/backend/src/backend/program.hpp
> > +++ b/backend/src/backend/program.hpp
> > @@ -125,7 +125,7 @@ namespace gbe {
> >        samplerSet = from;
> >      }
> >      /*! Get defined sampler size */
> > -    size_t getSamplerSize(void) const { return samplerSet->getDataSize(); }
> > +    size_t getSamplerSize(void) const { return (samplerSet == NULL 
> > + ? 0 : samplerSet->getDataSize()); }
> >      /*! Get defined sampler value array */
> >      void getSamplerData(uint32_t *samplers) const { samplerSet->getData(samplers); }
> >      /*! Set image set. */
> > @@ -145,7 +145,7 @@ namespace gbe {
> >         wg_sz[2] = compileWgSize[2];
> >      }
> >      /*! Get defined image size */
> > -    size_t getImageSize(void) const { return imageSet->getDataSize(); }
> > +    size_t getImageSize(void) const { return (imageSet == NULL ? 0 
> > + : imageSet->getDataSize()); }
> >      /*! Get defined image value array */
> >      void getImageData(ImageInfo *images) const { 
> > imageSet->getData(images); }
> >  
> > diff --git a/backend/src/ir/image.hpp b/backend/src/ir/image.hpp 
> > index cf388d4..4060324 100644
> > --- a/backend/src/ir/image.hpp
> > +++ b/backend/src/ir/image.hpp
> > @@ -61,6 +61,11 @@ namespace ir {
> >      void operator = (const ImageSet& other) {
> >        regMap.insert(other.regMap.begin(), other.regMap.end());
> >      }
> > +
> > +    bool empty() {
> > +      return (regMap.size() + indexMap.size() == 0 ? true : false);
> > +    }
> > +
>   better to use:     bool empty() const { return regMap.empty(); }
> 
> >      ImageSet(const ImageSet& other) : regMap(other.regMap.begin(), other.regMap.end()) { }
> >      ImageSet() {}
> >      ~ImageSet();
> > diff --git a/backend/src/ir/sampler.hpp b/backend/src/ir/sampler.hpp 
> > index dd1f3b6..0a30097 100644
> > --- a/backend/src/ir/sampler.hpp
> > +++ b/backend/src/ir/sampler.hpp
> > @@ -56,6 +56,10 @@ namespace ir {
> >        samplerMap.insert(other.samplerMap.begin(), other.samplerMap.end());
> >      }
> >  
> > +    bool empty() {
> > +      return (samplerMap.size() == 0 ? true : false);
> > +    }
> > +
>   better to use:   bool empty() const { return samplerMap.empty(); }
> >      SamplerSet(const SamplerSet& other) : samplerMap(other.samplerMap.begin(), other.samplerMap.end()) { }
> >      SamplerSet() {}
> >  
> > --
> > 1.8.3.2
> > 
> > _______________________________________________
> > Beignet mailing list
> > Beignet at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/beignet