Skip to content

Commit d0d26f1

Browse files
committed
review fixes: portable sizeof gate, explicit plan-move, alignment intent
- arena_sizeof_invariant_suite: drop platform-specific absolute baselines (328/16/248 were Apple-arm64/libc++ only); keep relative ImplLayoutAllocator == ImplLayoutMaster invariant + monostate static_asserts. - cont_engine: reset arena_plan_ after std::move into op_ so later reads see "no plan" rather than a moved-from optional. - arena_kernels: one-line intent note on trivial kernels' tight packing.
1 parent a034afd commit d0d26f1

3 files changed

Lines changed: 12 additions & 21 deletions

File tree

src/TiledArray/expressions/cont_engine.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,11 @@ class ContEngine : public BinaryEngine<Derived> {
306306
outer_size(left_indices_), outer_size(right_indices_),
307307
total_perm, this->element_nonreturn_op_,
308308
std::move(this->arena_plan_));
309+
// Plan ownership transferred to op_; mark carrier slot empty so any
310+
// later use of arena_plan_ reads as "no plan" rather than moved-from.
311+
if constexpr (!std::is_same_v<arena_plan_storage_t, std::monostate>) {
312+
this->arena_plan_.reset();
313+
}
309314
}
310315
trange_ = ContEngine_::make_trange(outer_perm);
311316
shape_ = ContEngine_::make_shape(outer_perm);
@@ -337,6 +342,11 @@ class ContEngine : public BinaryEngine<Derived> {
337342
outer_size(left_indices_), outer_size(right_indices_),
338343
total_perm, this->element_nonreturn_op_,
339344
std::move(this->arena_plan_));
345+
// Plan ownership transferred to op_; mark carrier slot empty so any
346+
// later use of arena_plan_ reads as "no plan" rather than moved-from.
347+
if constexpr (!std::is_same_v<arena_plan_storage_t, std::monostate>) {
348+
this->arena_plan_.reset();
349+
}
340350
}
341351
trange_ = ContEngine_::make_trange();
342352
shape_ = ContEngine_::make_shape();

src/TiledArray/tensor/arena_kernels.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ OuterTensor arena_trivial_unary(const SrcOuterTensor& src, FillOp&& fill_op) {
4747
auto shape_fn = [&src](std::size_t ord) -> decltype(auto) {
4848
return src.data()[ord].range();
4949
};
50+
// Elementwise kernels: pack tight (no cross-cell GEMM to amortize 128B pad).
5051
ArenaPlan p = plan(N_cells, shape_fn, sizeof(elem_t), alignof(elem_t));
5152
auto arena = std::make_shared<Arena>();
5253
if (p.total_bytes > 0) arena->reserve(p.total_bytes, false);
@@ -80,6 +81,7 @@ OuterTensor arena_trivial_binary(const LeftTensor& left, const RightTensor& righ
8081
auto shape_fn = [&left](std::size_t ord) -> decltype(auto) {
8182
return left.data()[ord].range();
8283
};
84+
// Elementwise kernels: pack tight (no cross-cell GEMM to amortize 128B pad).
8385
ArenaPlan p = plan(N_cells, shape_fn, sizeof(elem_t), alignof(elem_t));
8486
auto arena = std::make_shared<Arena>();
8587
if (p.total_bytes > 0) arena->reserve(p.total_bytes, false);

tests/arena_sizeof_invariant_suite.cpp

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
#include "TiledArray/tensor.h"
44
#include "TiledArray/tensor/arena_einsum.h"
5-
#include "TiledArray/tile_op/contract_reduce.h"
65
#include "TiledArray/util/function.h"
76
#include "tiledarray.h"
87
#include "unit_test_config.h"
@@ -20,10 +19,6 @@ using PlainLeft = TA::Tensor<double>;
2019
using PlainRight = TA::Tensor<double>;
2120
using PlainScalar = double;
2221

23-
using PlainContractReduceBase =
24-
TA::detail::ContractReduceBase<PlainResult, PlainLeft, PlainRight,
25-
PlainScalar>;
26-
2722
using PlainArenaPlanStorage =
2823
TA::detail::arena_plan_storage_t<PlainResult, PlainLeft, PlainRight>;
2924

@@ -47,11 +42,6 @@ struct ImplLayoutAllocator {
4742
TA_NO_UNIQUE_ADDRESS PlainArenaPlanStorage arena_plan_;
4843
};
4944

50-
/// Sizes captured against master; re-baseline if the toolchain changes.
51-
constexpr std::size_t kTensorDoubleSizeMaster = 328;
52-
constexpr std::size_t kContractReduceBaseSizeMaster = 16;
53-
constexpr std::size_t kImplLayoutSizeMaster = 248;
54-
5545
static_assert(std::is_same_v<PlainArenaPlanStorage, std::monostate>,
5646
"plain-tensor arena_plan_storage_t must be std::monostate");
5747

@@ -62,18 +52,7 @@ static_assert(sizeof(ImplLayoutAllocator) == sizeof(ImplLayoutMaster),
6252

6353
BOOST_AUTO_TEST_SUITE(arena_sizeof_invariant_suite, TA_UT_LABEL_SERIAL)
6454

65-
BOOST_AUTO_TEST_CASE(tensor_double_sizeof_matches_master) {
66-
BOOST_CHECK_EQUAL(sizeof(TA::Tensor<double>), kTensorDoubleSizeMaster);
67-
}
68-
69-
BOOST_AUTO_TEST_CASE(contract_reduce_base_sizeof_matches_master) {
70-
BOOST_CHECK_EQUAL(sizeof(PlainContractReduceBase),
71-
kContractReduceBaseSizeMaster);
72-
}
73-
7455
BOOST_AUTO_TEST_CASE(impl_layout_no_unique_address_invariant) {
75-
BOOST_CHECK_EQUAL(sizeof(ImplLayoutMaster), kImplLayoutSizeMaster);
76-
BOOST_CHECK_EQUAL(sizeof(ImplLayoutAllocator), kImplLayoutSizeMaster);
7756
BOOST_CHECK_EQUAL(sizeof(ImplLayoutAllocator), sizeof(ImplLayoutMaster));
7857
}
7958

0 commit comments

Comments
 (0)