Skip to content

Commit b8720f2

Browse files
author
Github Executorch
committed
Add c10::irange to ExecuTorch
Pull Request resolved: #8554 irange is a header-only utility that is both more readable than the usual way to write for loops and also solves -Wsign-compare issues. I've previously vetted that it generates the same assembly as a regular for loop. ghstack-source-id: 267193301 @exported-using-ghexport Differential Revision: [D69817195](https://our.internmc.facebook.com/intern/diff/D69817195/)
1 parent cc3974f commit b8720f2

File tree

4 files changed

+130
-1
lines changed

4 files changed

+130
-1
lines changed

runtime/core/portable_type/c10/c10/targets.bzl

+1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ def define_common_targets():
2626
"util/TypeSafeSignMath.h",
2727
"util/bit_cast.h",
2828
"util/floating_point_utils.h",
29+
"util/irange.h",
2930
],
3031
exported_preprocessor_flags = [
3132
# NOTE: If we define C10_EMBEDDED to prevent Half and
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
// Copyright 2004-present Facebook. All Rights Reserved.
2+
3+
#pragma once
4+
5+
#include <c10/util/TypeSafeSignMath.h>
6+
7+
#include <algorithm>
8+
#include <cstddef>
9+
#include <iterator>
10+
#include <type_traits>
11+
12+
namespace c10 {
13+
14+
namespace detail {
15+
16+
template <
17+
typename I,
18+
bool one_sided = false,
19+
std::enable_if_t<std::is_integral_v<I>, int> = 0>
20+
struct integer_iterator {
21+
using iterator_category = std::input_iterator_tag;
22+
using value_type = I;
23+
using difference_type = std::ptrdiff_t;
24+
using pointer = I*;
25+
using reference = I&;
26+
27+
explicit integer_iterator(I value) : value(value) {}
28+
29+
I operator*() const {
30+
return value;
31+
}
32+
33+
I const* operator->() const {
34+
return &value;
35+
}
36+
37+
integer_iterator& operator++() {
38+
++value;
39+
return *this;
40+
}
41+
42+
integer_iterator operator++(int) {
43+
const auto copy = *this;
44+
++*this;
45+
return copy;
46+
}
47+
48+
bool operator==(const integer_iterator& other) const {
49+
if constexpr (one_sided) {
50+
// Range-for loops' end test is `begin != end`, not `begin <
51+
// end`. To handle `c10::irange(n)` where n < 0 (which should be
52+
// empty), we just make `begin != end` fail whenever `end` is
53+
// negative.
54+
return is_negative(other.value) || value == other.value;
55+
} else {
56+
return value == other.value;
57+
}
58+
// Suppress "warning: missing return statement at end of non-void function"
59+
// which Nvidia's Robert Crovella confirms is an NVCC compiler error
60+
// here https://stackoverflow.com/a/64561686/752843 on 2020-10-27
61+
// `__builtin_unreachable();` would be best here, but it's not
62+
// available with all compilers. So we instead return an arbitrary
63+
// value trusting that this line will, in fact, never be reached.
64+
return false; // Horrible hack
65+
}
66+
67+
bool operator!=(const integer_iterator& other) const {
68+
return !(*this == other);
69+
}
70+
71+
protected:
72+
I value;
73+
};
74+
75+
} // namespace detail
76+
77+
template <
78+
typename I,
79+
bool one_sided = false,
80+
std::enable_if_t<std::is_integral_v<I>, bool> = true>
81+
struct integer_range {
82+
public:
83+
integer_range(I begin, I end) : begin_(begin), end_(end) {}
84+
using iterator = detail::integer_iterator<I, one_sided>;
85+
iterator begin() const {
86+
return begin_;
87+
}
88+
iterator end() const {
89+
return end_;
90+
}
91+
92+
private:
93+
iterator begin_;
94+
iterator end_;
95+
};
96+
97+
/// Creates an integer range for the half-open interval [begin, end)
98+
/// If end<=begin, then the range is empty.
99+
/// The range has the type of the `end` integer; `begin` integer is
100+
/// cast to this type.
101+
template <
102+
typename Integer1,
103+
typename Integer2,
104+
std::enable_if_t<std::is_integral_v<Integer1>, bool> = true,
105+
std::enable_if_t<std::is_integral_v<Integer2>, bool> = true>
106+
integer_range<Integer2> irange(Integer1 begin, Integer2 end) {
107+
// If end<=begin then the range is empty; we can achieve this effect by
108+
// choosing the larger of {begin, end} as the loop terminator
109+
return {
110+
static_cast<Integer2>(begin),
111+
std::max(static_cast<Integer2>(begin), end)};
112+
}
113+
114+
/// Creates an integer range for the half-open interval [0, end)
115+
/// If end<=begin, then the range is empty
116+
template <
117+
typename Integer,
118+
std::enable_if_t<std::is_integral_v<Integer>, bool> = true>
119+
integer_range<Integer, true> irange(Integer end) {
120+
return {Integer(), end};
121+
}
122+
123+
} // namespace c10

runtime/core/portable_type/targets.bzl

+3
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ def define_common_targets():
2828
"//executorch/runtime/core/exec_aten/...",
2929
"//executorch/runtime/core/portable_type/test/...",
3030
],
31+
deps = [
32+
"//executorch/runtime/core/portable_type/c10/c10:c10",
33+
],
3134
exported_deps = [
3235
":scalar_type",
3336
"//executorch/runtime/core:core",

runtime/core/portable_type/tensor_impl.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
#include <algorithm>
1212
#include <cstdint>
1313

14+
#include <c10/util/irange.h>
15+
1416
#include <executorch/runtime/core/exec_aten/util/dim_order_util.h>
1517
#include <executorch/runtime/core/exec_aten/util/scalar_type_util.h>
1618
#include <executorch/runtime/core/exec_aten/util/tensor_shape_to_c_string.h>
@@ -30,7 +32,7 @@ ssize_t compute_numel(const TensorImpl::SizesType* sizes, ssize_t dim) {
3032
dim == 0 || sizes != nullptr,
3133
"Sizes must be provided for non-scalar tensors");
3234
ssize_t numel = 1; // Zero-dimensional tensors (scalars) have numel == 1.
33-
for (ssize_t i = 0; i < dim; ++i) {
35+
for (const auto i : c10::irange(dim)) {
3436
ET_CHECK_MSG(
3537
sizes[i] >= 0,
3638
"Size must be non-negative, got %d at dimension %zd",

0 commit comments

Comments
 (0)