Skip to content

Commit 51534e1

Browse files
swolchokYIWENX14
authored andcommitted
Coerce to true_ctype in tensor_factory (#7856)
* Coerce to true_ctype in tensor_factory (#7856) This should fix the problem where attempts to test bool are often wonky in OSS and fail UBSAN internally; it is undefined behavior to store a value other than 0 or 1 for type bool. * Support Half/BFloat16 in prod operator (#7857) Partial fix for #7748.
1 parent b3e74db commit 51534e1

File tree

3 files changed

+67
-27
lines changed

3 files changed

+67
-27
lines changed

kernels/portable/cpu/op_prod.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ Tensor& prod_out(
3333
ScalarType out_type = out.scalar_type();
3434
constexpr auto name = "prod.int_out";
3535

36-
ET_SWITCH_REALHB_TYPES(in_type, ctx, name, CTYPE_IN, [&] {
37-
ET_SWITCH_REALHB_TYPES(out_type, ctx, name, CTYPE_OUT, [&] {
36+
ET_SWITCH_REALHBBF16_TYPES(in_type, ctx, name, CTYPE_IN, [&] {
37+
ET_SWITCH_REALHBBF16_TYPES(out_type, ctx, name, CTYPE_OUT, [&] {
3838
const auto data_in = in.const_data_ptr<CTYPE_IN>();
3939
auto data_out = out.mutable_data_ptr<CTYPE_OUT>();
4040
data_out[0] = static_cast<CTYPE_OUT>(1);
@@ -73,8 +73,8 @@ Tensor& prod_int_out(
7373
ScalarType out_type = out.scalar_type();
7474
constexpr auto name = "prod.int_out";
7575

76-
ET_SWITCH_REALHB_TYPES(in_type, ctx, name, CTYPE_IN, [&] {
77-
ET_SWITCH_REALHB_TYPES(out_type, ctx, name, CTYPE_OUT, [&] {
76+
ET_SWITCH_REALHBBF16_TYPES(in_type, ctx, name, CTYPE_IN, [&] {
77+
ET_SWITCH_REALHBBF16_TYPES(out_type, ctx, name, CTYPE_OUT, [&] {
7878
CTYPE_OUT* out_data = out.mutable_data_ptr<CTYPE_OUT>();
7979
for (size_t out_ix = 0; out_ix < out.numel(); ++out_ix) {
8080
CTYPE_OUT prod = 1;

kernels/test/op_prod_test.cpp

+40-20
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,24 @@ class OpProdOutTest : public ::testing::Test {
4545
// first.
4646
torch::executor::runtime_init();
4747
}
48+
49+
template <ScalarType DTYPE>
50+
void test_dtype() {
51+
TensorFactory<DTYPE> tf;
52+
TensorFactory<
53+
executorch::runtime::isIntegralType(DTYPE, /*includeBool*/ true)
54+
? ScalarType::Long
55+
: DTYPE>
56+
tf_out;
57+
58+
Tensor self = tf.make({2, 3}, {1, 2, 3, 4, 5, 6});
59+
optional<ScalarType> dtype{};
60+
Tensor out = tf_out.zeros({});
61+
Tensor out_expected =
62+
tf_out.make({}, {DTYPE == ScalarType::Bool ? 1 : 720});
63+
op_prod_out(self, dtype, out);
64+
EXPECT_TENSOR_CLOSE(out, out_expected);
65+
}
4866
};
4967

5068
class OpProdIntOutTest : public ::testing::Test {
@@ -54,30 +72,32 @@ class OpProdIntOutTest : public ::testing::Test {
5472
// first.
5573
torch::executor::runtime_init();
5674
}
57-
};
5875

59-
TEST_F(OpProdOutTest, SmokeTest) {
60-
TensorFactory<ScalarType::Float> tfFloat;
76+
template <ScalarType DTYPE>
77+
void test_dtype() {
78+
TensorFactory<DTYPE> tf;
6179

62-
Tensor self = tfFloat.make({2, 3}, {1, 2, 3, 4, 5, 6});
63-
optional<ScalarType> dtype{};
64-
Tensor out = tfFloat.zeros({});
65-
Tensor out_expected = tfFloat.make({}, {720});
66-
op_prod_out(self, dtype, out);
67-
EXPECT_TENSOR_CLOSE(out, out_expected);
68-
}
80+
Tensor self = tf.make({2, 3}, {1, 2, 3, 4, 5, 6});
81+
int64_t dim = 0;
82+
bool keepdim = false;
83+
optional<ScalarType> dtype{};
84+
Tensor out = tf.zeros({3});
85+
Tensor out_expected = tf.make({3}, {4, 10, 18});
86+
op_prod_int_out(self, dim, keepdim, dtype, out);
87+
EXPECT_TENSOR_CLOSE(out, out_expected);
88+
}
89+
};
6990

70-
TEST_F(OpProdIntOutTest, SmokeTest) {
71-
TensorFactory<ScalarType::Float> tfFloat;
91+
TEST_F(OpProdOutTest, SmokeTest){
92+
#define TEST_ENTRY(ctype, dtype) test_dtype<ScalarType::dtype>();
93+
ET_FORALL_REALHBBF16_TYPES(TEST_ENTRY)
94+
#undef TEST_ENTRY
95+
}
7296

73-
Tensor self = tfFloat.make({2, 3}, {1, 2, 3, 4, 5, 6});
74-
int64_t dim = 0;
75-
bool keepdim = false;
76-
optional<ScalarType> dtype{};
77-
Tensor out = tfFloat.zeros({3});
78-
Tensor out_expected = tfFloat.make({3}, {4, 10, 18});
79-
op_prod_int_out(self, dim, keepdim, dtype, out);
80-
EXPECT_TENSOR_CLOSE(out, out_expected);
97+
TEST_F(OpProdIntOutTest, SmokeTest){
98+
#define TEST_ENTRY(ctype, dtype) test_dtype<ScalarType::dtype>();
99+
ET_FORALL_REALHBBF16_TYPES(TEST_ENTRY)
100+
#undef TEST_ENTRY
81101
}
82102

83103
TEST_F(OpProdIntOutTest, SmokeTestKeepdim) {

runtime/core/exec_aten/testing_util/tensor_factory.h

+23-3
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,10 @@ class TensorFactory {
279279
t = empty_strided(sizes, strides);
280280
}
281281
if (t.nbytes() > 0) {
282-
memcpy(t.template data<true_ctype>(), data.data(), t.nbytes());
282+
std::transform(
283+
data.begin(), data.end(), t.template data<true_ctype>(), [](auto x) {
284+
return static_cast<true_ctype>(x);
285+
});
283286
}
284287
return t;
285288
}
@@ -319,7 +322,10 @@ class TensorFactory {
319322
t = empty_strided(sizes, strides);
320323
}
321324
if (t.nbytes() > 0) {
322-
memcpy(t.template data<true_ctype>(), data.data(), t.nbytes());
325+
std::transform(
326+
data.begin(), data.end(), t.template data<true_ctype>(), [](auto x) {
327+
return static_cast<true_ctype>(x);
328+
});
323329
}
324330
return t;
325331
}
@@ -721,6 +727,13 @@ class TensorFactory {
721727
*/
722728
using ctype = typename internal::ScalarTypeToCppTypeWrapper<DTYPE>::ctype;
723729

730+
/**
731+
* The official C type for the scalar type. Used when accessing elements
732+
* of a constructed Tensor.
733+
*/
734+
using true_ctype =
735+
typename executorch::runtime::ScalarTypeToCppType<DTYPE>::type;
736+
724737
TensorFactory() = default;
725738

726739
/**
@@ -1019,7 +1032,14 @@ class TensorFactory {
10191032
data_.data(),
10201033
dim_order_.data(),
10211034
strides_.data(),
1022-
dynamism) {}
1035+
dynamism) {
1036+
// The only valid values for bool are 0 and 1; coerce!
1037+
if constexpr (std::is_same_v<true_ctype, bool>) {
1038+
for (auto& x : data_) {
1039+
x = static_cast<true_ctype>(x);
1040+
}
1041+
}
1042+
}
10231043

10241044
std::vector<int32_t> sizes_;
10251045
std::vector<ctype> data_;

0 commit comments

Comments
 (0)