From 19cfe14c762bffabdec84718959c5c4a14a09577 Mon Sep 17 00:00:00 2001 From: Jan Wassenberg Date: Fri, 25 Oct 2024 04:13:38 -0700 Subject: [PATCH] Warning fixes (casts) and fix Windows build for aligned_alloc PiperOrigin-RevId: 689734618 --- compression/blob_store.cc | 3 ++- paligemma/BUILD | 1 + paligemma/image.cc | 29 +++++++++++++++++------------ util/allocator.h | 14 +++++++++----- 4 files changed, 29 insertions(+), 18 deletions(-) diff --git a/compression/blob_store.cc b/compression/blob_store.cc index 0ce9996..659946c 100644 --- a/compression/blob_store.cc +++ b/compression/blob_store.cc @@ -275,7 +275,8 @@ BlobError BlobReader::ReadAll(hwy::ThreadPool& pool) { [pfile, &requests, &err](uint64_t i, size_t /*thread*/) { if (!pfile->Read(requests[i].offset, requests[i].size, requests[i].data)) { - fprintf(stderr, "Failed to read blob %zu\n", i); + fprintf(stderr, "Failed to read blob %zu\n", + static_cast(i)); err.test_and_set(); } }); diff --git a/paligemma/BUILD b/paligemma/BUILD index d173526..b19e05e 100644 --- a/paligemma/BUILD +++ b/paligemma/BUILD @@ -14,6 +14,7 @@ cc_library( deps = [ "//compression:io", "@highway//:hwy", + "@highway//:profiler", ], ) diff --git a/paligemma/image.cc b/paligemma/image.cc index 795b5e4..7d0396b 100644 --- a/paligemma/image.cc +++ b/paligemma/image.cc @@ -14,8 +14,8 @@ // limitations under the License. #include "paligemma/image.h" -#include "compression/io.h" +#include #include #include @@ -28,7 +28,10 @@ #include #include +#include "compression/io.h" +#include "hwy/aligned_allocator.h" // hwy::Span #include "hwy/base.h" +#include "hwy/profiler.h" namespace gcpp { namespace { @@ -95,12 +98,12 @@ bool Image::ReadPPM(const std::string& filename) { std::cerr << filename << " does not exist\n"; return false; } - auto content = ReadFileToString(path); + const std::string content = ReadFileToString(path); return ReadPPM(hwy::Span(content.data(), content.size())); } bool Image::ReadPPM(const hwy::Span& buf) { - auto pos = CheckP6Format(buf.cbegin(), buf.cend()); + const char* pos = CheckP6Format(buf.cbegin(), buf.cend()); if (!pos) { std::cerr << "We only support binary PPM (P6)\n"; return false; @@ -134,8 +137,8 @@ bool Image::ReadPPM(const hwy::Span& buf) { return false; } ++pos; - auto data_size = width * height * 3; - if (buf.cend() - pos < data_size) { + const size_t data_size = width * height * 3; + if (buf.cend() - pos < static_cast(data_size)) { std::cerr << "Insufficient data remaining\n"; return false; } @@ -190,23 +193,24 @@ bool Image::WriteBinary(const std::string& filename) const { // We want the N-th patch (of 256) of size kPatchSize x kPatchSize x 3. // Patches are numbered in usual "pixel-order". void Image::GetPatch(size_t patch_num, float* patch) const { + PROFILER_FUNC; constexpr size_t kDataSize = kImageSize * kImageSize * 3; HWY_ASSERT(size() == kDataSize); constexpr size_t kPatchDataSize = kPatchSize * kPatchSize * 3; - int i_offs = patch_num / kNumPatches; - int j_offs = patch_num % kNumPatches; + size_t i_offs = patch_num / kNumPatches; + size_t j_offs = patch_num % kNumPatches; HWY_ASSERT(0 <= i_offs && i_offs < kNumPatches); HWY_ASSERT(0 <= j_offs && j_offs < kNumPatches); i_offs *= kPatchSize; j_offs *= kPatchSize; // This can be made faster, but let's first see whether it matters. const float* image_data = data(); - for (int i = 0; i < kPatchSize; ++i) { - for (int j = 0; j < kPatchSize; ++j) { - for (int k = 0; k < 3; ++k) { - const int patch_index = (i * kPatchSize + j) * 3 + k; + for (size_t i = 0; i < kPatchSize; ++i) { + for (size_t j = 0; j < kPatchSize; ++j) { + for (size_t k = 0; k < 3; ++k) { + const size_t patch_index = (i * kPatchSize + j) * 3 + k; HWY_ASSERT(patch_index < kPatchDataSize); - const int image_index = + const size_t image_index = ((i + i_offs) * kImageSize + (j + j_offs)) * 3 + k; HWY_ASSERT(image_index < kDataSize); patch[patch_index] = image_data[image_index]; @@ -214,4 +218,5 @@ void Image::GetPatch(size_t patch_num, float* patch) const { } } } + } // namespace gcpp diff --git a/util/allocator.h b/util/allocator.h index 386b23f..ca1df45 100644 --- a/util/allocator.h +++ b/util/allocator.h @@ -19,7 +19,7 @@ #include #include -#include // std::aligned_alloc +#include // std::aligned_alloc / _aligned_malloc // IWYU pragma: begin_exports #include "util/threading.h" @@ -140,15 +140,19 @@ class Allocator { } // AlignedFreeUniquePtr has a deleter that can call an arbitrary `free`, but - // with an extra opaque pointer, which we discard via this adapter. + // with an extra opaque pointer, which we discard via `call_free`. +#if defined(__ANDROID_API__) && __ANDROID_API__ < 28 const auto call_free = [](void* ptr, void*) { std::free(ptr); }; -#if !defined(__ANDROID_API__) || __ANDROID_API__ >= 28 - T* p = static_cast(std::aligned_alloc(Alignment(), bytes)); -#else void* mem = nullptr; int err = posix_memalign(&mem, Alignment(), bytes); HWY_ASSERT(err == 0); T* p = static_cast(mem); +#elif HWY_OS_WIN + const auto call_free = [](void* ptr, void*) { _aligned_free(ptr); }; + T* p = static_cast(_aligned_malloc(bytes, Alignment())); +#else + const auto call_free = [](void* ptr, void*) { std::free(ptr); }; + T* p = static_cast(std::aligned_alloc(Alignment(), bytes)); #endif return hwy::AlignedFreeUniquePtr( p, hwy::AlignedFreer(call_free, nullptr));