Skip to content

Commit aad9446

Browse files
committed
Fix SmallArray with size 0
The original breakage comes from SmallBase<T, Dimv> a couple commits ago. When I removed the Dimv dependence, it made the base class of every SmallArray<T> the same no matter the T, which resulted in a padding bug. But removing the base class entirely makes sizeof(SmallArray)=0 for size 0, which is an inconvenience. For example, one can't create a std::vector of such objects. Options are to 1) use a special object for size 0, or reuse std::array, which does that; or to 2) add a member or a base class that'll take space only when the size is 0. But now I know why. * ra/small.hh (SmallArray): As stated. * test/sizeof.cc: Add test. Elsewhere use SmallArray::data() in a few places instead of .cp directly.
1 parent 045d244 commit aad9446

File tree

4 files changed

+34
-35
lines changed

4 files changed

+34
-35
lines changed

ra/small.hh

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -493,24 +493,17 @@ template <class T, int N> using extvector __attribute__((ext_vector_type(N))) =
493493
template <class T, int N> using extvector __attribute__((vector_size(N*sizeof(T)))) = T;
494494
#endif
495495

496-
template <class Z, class ... T> constexpr static bool equal_to_any = (std::is_same_v<Z, T> || ...);
496+
template <class T, size_t N> constexpr size_t align_req = alignof(T[N]);
497+
template <class Z, class ... T> constexpr static bool equals_any = (std::is_same_v<Z, T> || ...);
497498
template <class T, size_t N>
498-
consteval size_t
499-
align_req()
500-
{
501-
if constexpr (equal_to_any<T, char, unsigned char, short, unsigned short, int, unsigned int,
502-
long, unsigned long, long long, unsigned long long, float, double
503-
> && 0<N && 0==(N & (N-1))) {
504-
return alignof(extvector<T, N>);
505-
} else {
506-
return alignof(T[N]);
507-
}
508-
}
499+
requires (equals_any<T, char, unsigned char, short, unsigned short, int, unsigned int, long, unsigned long,
500+
long long, unsigned long long, float, double> && 0<N && 0==(N & (N-1)))
501+
constexpr size_t align_req<T, N> = alignof(extvector<T, N>);
509502

510503
template <class T, class Dimv, class ... nested_args>
511504
struct
512505
#if RA_OPT_SMALLVECTOR==1
513-
alignas(align_req<T, std::apply([](auto ... i) { return (i.len * ... * 1); }, Dimv::value)>())
506+
alignas(align_req<T, std::apply([](auto ... i) { return (i.len * ... * 1); }, Dimv::value)>)
514507
#endif
515508
SmallArray<T, Dimv, std::tuple<nested_args ...>>
516509
{
@@ -521,14 +514,16 @@ SmallArray<T, Dimv, std::tuple<nested_args ...>>
521514
constexpr static dim_t step(int k) { return dimv[k].step; }
522515
consteval static dim_t size() { return std::apply([](auto ... i) { return (i.len * ... * 1); }, dimv); }
523516

524-
T cp[size()]; // std::array avoids 0, but why would I
517+
T cp[size()];
518+
[[no_unique_address]] struct {} prevent_zero_size; // or reuse std::array
519+
constexpr auto data(this auto && self) { return self.cp; }
525520

526521
using View = ViewSmall<T *, Dimv>;
527522
using ViewConst = ViewSmall<T const *, Dimv>;
528-
constexpr View view() { return View(cp); }
529-
constexpr ViewConst view() const { return ViewConst(cp); }
523+
constexpr View view() { return View(data()); }
524+
constexpr ViewConst view() const { return ViewConst(data()); }
530525
constexpr operator View () { return View(cp); }
531-
constexpr operator ViewConst () const { return ViewConst(cp); }
526+
constexpr operator ViewConst () const { return ViewConst(data()); }
532527

533528
constexpr SmallArray() {}
534529
constexpr SmallArray(ra::none_t) {}
@@ -558,7 +553,6 @@ SmallArray<T, Dimv, std::tuple<nested_args ...>>
558553
#undef ASSIGNOPS
559554

560555
constexpr decltype(auto) back(this auto && self) { return RA_FWD(self).view().back(); }
561-
constexpr auto data(this auto && self) { return self.view().data(); }
562556
constexpr decltype(auto) operator()(this auto && self, auto && ... a) { return RA_FWD(self).view()(RA_FWD(a) ...); }
563557
constexpr decltype(auto) operator[](this auto && self, auto && ... a) { return RA_FWD(self).view()(RA_FWD(a) ...); }
564558
constexpr decltype(auto) at(this auto && self, auto && i) { return RA_FWD(self).view().at(RA_FWD(i)); }

test/sizeof.cc

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// -*- mode: c++; coding: utf-8 -*-
2-
// ra-ra/test - What to expect of sizeof... no checks.
2+
// ra-ra/test - What to expect of sizeof...
33

44
// (c) Daniel Llorens - 2023
55
// This library is free software; you can redistribute it and/or modify it under
@@ -16,7 +16,7 @@ struct noargx {}; // Small uses noarg for its own
1616
struct sample1
1717
{
1818
uint8_t a[16];
19-
[[no_unique_address]] ra::Small<int, 0> b; // adds actually 0, but I don't rely on this atm
19+
[[no_unique_address]] ra::Small<int, 0> b; // 0 or 1, don't rely on this
2020
};
2121

2222
struct sample2
@@ -64,6 +64,7 @@ int main()
6464
cout << sizeof(ra::Small<ra::none_t, 0>) << endl;
6565
cout << sizeof(ra::Small<noargx, 0>) << endl;
6666
// the following tests were broken at some point because Small had a fixed empty base class, so nesting Smalls created multiples of these which added useless padding. Not a fan of the rule but w/e - the base class wasn't doing all that much.
67+
// See
6768
{
6869
using nest = ra::Small<ra::Small<int, 2>, 2>;
6970
cout << "nest " << sizeof(nest) << " " << alignof(nest) << endl;
@@ -102,5 +103,21 @@ int main()
102103
cout << sizeof(sample5) << endl;
103104
cout << sizeof(sample6) << endl;
104105
}
106+
tr.section("sizeof for Small");
107+
{
108+
tr.info("sizeof(ra::Small<double>)").test_eq(sizeof(double), sizeof(ra::Small<double>));
109+
tr.info("sizeof(ra::Small<double, 1>)").test_eq(sizeof(double), sizeof(ra::Small<double, 1>));
110+
tr.info("sizeof(ra::Small<double, 2>)").test_eq(2*sizeof(double), sizeof(ra::Small<double, 2>));
111+
// don't rely on these.
112+
tr.skip().info("sizeof(ra::Small<double, 0>)?").test_eq(0u, sizeof(ra::Small<double, 0>));
113+
tr.skip().info("sizeof(ra::Small<double, 0>)?").test_eq(sizeof(double), sizeof(ra::Small<double, 0>));
114+
}
115+
// this needs some EBO grease in SmallArray, or using std::array as member instead of T[].
116+
tr.section("Small<T, 0> are allocatable");
117+
{
118+
using Zero = ra::Small<int, 0>;
119+
ra::Big<Zero, 1> a(0, ra::none);
120+
tr.test_eq(0u, a.size());
121+
}
105122
return tr.summary();
106123
}

test/small-0.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -535,14 +535,14 @@ int main()
535535
{
536536
ra::Small<int, 3, 4> a = ra::_0 - ra::_1;
537537
auto vn = a.view();
538-
tr.test_seq(a.cp, vn.cp);
538+
tr.test_seq(a.data(), vn.cp);
539539
static_assert(std::is_same_v<int *, decltype(vn.cp)>);
540540
auto vc = std::as_const(a).view();
541-
tr.test_seq(a.cp, vc.cp);
541+
tr.test_seq(a.data(), vc.cp);
542542
static_assert(std::is_same_v<int const *, decltype(vc.cp)>);
543543
// needs ViewSmall::ViewConst conversion op
544544
decltype(vc) vk = vn;
545-
tr.test_seq(a.cp, vk.cp);
545+
tr.test_seq(a.data(), vk.cp);
546546
static_assert(std::is_same_v<int const *, decltype(vk.cp)>);
547547
}
548548
// ra::shape / ra::size are static for Small types

test/small-1.cc

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -47,18 +47,6 @@ int main()
4747
x3() = transpose(y3(), ilist_t<1, 0>{});
4848
tr.info("transpose copy").test_eq(x3, ra::_0 - ra::_1);
4949
}
50-
tr.section("sizeof");
51-
{
52-
// These all static, but show the numbers if there's an error.
53-
tr.info("sizeof(ra::Small<double>)")
54-
.test_eq(sizeof(double), sizeof(ra::Small<double>));
55-
tr.info("sizeof(ra::Small<double, 0>)")
56-
.test(sizeof(double)==sizeof(ra::Small<double, 0>) || 0==sizeof(ra::Small<double, 0>)); // don't rely on either.
57-
tr.info("sizeof(ra::Small<double, 1>)")
58-
.test_eq(sizeof(double), sizeof(ra::Small<double, 1>));
59-
tr.info("sizeof(ra::Small<double, 2>)")
60-
.test_eq(2*sizeof(double), sizeof(ra::Small<double, 2>));
61-
}
6250
tr.section("internal fields");
6351
{
6452
{

0 commit comments

Comments
 (0)