Skip to content

[compute/cker] Reduce duplicated code and Fix wrong shape size. #15158

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chunseoklee merged 2 commits into from
Apr 14, 2025
Merged

[compute/cker] Reduce duplicated code and Fix wrong shape size. #15158

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a99b62a
[compute/cker] Reduce duplicated code and fix wrong shape size.
ragmani Apr 14, 2025
ea488a8
- Reverted the incorrectly replaced resize and fixed some flawed init…
ragmani Apr 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 17 additions & 48 deletions runtime/compute/cker/include/cker/Shape.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,30 +47,12 @@ class Shape
// Constructor that takes a dimension count.
// If dimensions_count <= kMaxSmallSize, it uses a fixed-size array.
// Otherwise, it uses a dynamic vector.
explicit Shape(int dimensions_count) : _size(dimensions_count)
{
if (dimensions_count <= kMaxSmallSize)
{
dims_ = std::array<int32_t, kMaxSmallSize>{};
}
else
{
dims_ = std::vector<int32_t>(dimensions_count);
}
}
explicit Shape(int dimensions_count) : _size(dimensions_count) { initStorage(dimensions_count); }

// Constructor that creates a shape of given size and fills all dimensions with "value".
Shape(int shape_size, int32_t value) : _size(0)
Shape(int shape_size, int32_t value) : _size(shape_size)
Copy link
Copy Markdown
Contributor Author

@ragmani ragmani Apr 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This generated test errors because this constructor did not apply shape_size to _size anywhere.

{
if (shape_size <= kMaxSmallSize)
{
dims_ = std::array<int32_t, kMaxSmallSize>{};
}
else
{
dims_ = std::vector<int32_t>(shape_size);
}

initStorage(shape_size);
for (int i = 0; i < shape_size; ++i)
{
SetDim(i, value);
Expand All @@ -80,16 +62,7 @@ class Shape
// Constructor that creates a shape from an array of dimension data.
Shape(int dimensions_count, const int32_t *dims_data) : _size(0)
{
// Explicitly initialize dims_ based on dimensions_count to avoid uninitialized state.
if (dimensions_count <= kMaxSmallSize)
{
dims_ = std::array<int32_t, kMaxSmallSize>{};
}
else
{
dims_ = std::vector<int32_t>(dimensions_count);
}

initStorage(dimensions_count);
ReplaceWith(dimensions_count, dims_data);
}

Expand All @@ -98,18 +71,7 @@ class Shape
Shape(const std::initializer_list<int> init_list) : _size(0)
{
const auto size = static_cast<int>(std::distance(init_list.begin(), init_list.end()));

// Explicitly initialize dims_ based on the initializer list size to prevent
// "maybe uninitialized" warnings when BuildFrom() is invoked.
if (size <= kMaxSmallSize)
{
dims_ = std::array<int32_t, kMaxSmallSize>{};
}
else
{
dims_ = std::vector<int32_t>(size);
}

initStorage(size);
BuildFrom(init_list);
}

Expand Down Expand Up @@ -207,10 +169,7 @@ class Shape
// initialize dims_ explicitly based on dimensions_count to ensure it is in a valid state.
if (dims_.valueless_by_exception())
{
if (dimensions_count <= kMaxSmallSize)
dims_ = std::array<int32_t, kMaxSmallSize>{};
else
dims_ = std::vector<int32_t>(dimensions_count);
initStorage(dimensions_count);
}

std::vector<int32_t> oldDims;
Expand Down Expand Up @@ -304,21 +263,31 @@ class Shape
bool operator!=(const Shape &comp) const { return !((*this) == comp); }

private:
// Helper function: initialize dims_ storage based on the number of dimensions.
inline void initStorage(int dimensions_count)
{
if (dimensions_count <= kMaxSmallSize)
dims_ = std::array<int32_t, kMaxSmallSize>{};
else
dims_ = std::vector<int32_t>(dimensions_count);
}

// For use only by ExtendedShape(), written to guarantee (return-value) copy
// elision in C++17.
// This creates a shape padded to the desired size with the specified value.
Shape(int new_shape_size, const Shape &shape, int pad_value) : _size(0)
{
assert(new_shape_size >= shape.DimensionsCount());
assert(new_shape_size <= kMaxSmallSize);
Resize(new_shape_size);
initStorage(new_shape_size);
Comment thread
ragmani marked this conversation as resolved.
Outdated
const int size_increase = new_shape_size - shape.DimensionsCount();
for (int i = 0; i < size_increase; ++i)
{
SetDim(i, pad_value);
}
std::memcpy(DimsData() + size_increase, shape.DimsData(),
sizeof(int32_t) * shape.DimensionsCount());
_size = new_shape_size;
}

int32_t _size;
Expand Down