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.g...@linux.intel.com] Sent: Friday, May 09, 2014 10:22 AM To: Guo, Yejun Cc: beignet@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....@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/beignet _______________________________________________ Beignet mailing list Beignet@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/beignet