Skip to content

Commit 0c7ca82

Browse files
jchodorCompute-Runtime-Automation
authored andcommitted
refactor: Introducing common ext pattern
Signed-off-by: Chodor, Jaroslaw <[email protected]>
1 parent b55347a commit 0c7ca82

File tree

6 files changed

+146
-58
lines changed

6 files changed

+146
-58
lines changed

shared/source/device_binary_format/zebin/zeinfo.h

Lines changed: 3 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "shared/source/device_binary_format/yaml/yaml_parser.h"
1111
#include "shared/source/helpers/non_copyable_or_moveable.h"
1212
#include "shared/source/utilities/const_stringref.h"
13+
#include "shared/source/utilities/mem_lifetime.h"
1314

1415
#include <array>
1516
#include <cstring>
@@ -664,43 +665,9 @@ inline constexpr OffsetT offset = -1;
664665
inline constexpr BtiValueT btiValue = -1;
665666
} // namespace Defaults
666667

667-
struct PayloadArgElasticPtrBaseT;
668-
struct PayloadArgumentBaseT;
669668
struct PayloadArgumentExtT;
670-
PayloadArgumentExtT *allocatePayloadArgumentExt();
671-
void freePayloadArgumentExt(PayloadArgumentExtT *);
672-
void copyPayloadArgumentExt(PayloadArgumentExtT *&, const PayloadArgElasticPtrBaseT &);
673669

674-
struct PayloadArgElasticPtrBaseT {
675-
PayloadArgElasticPtrBaseT() {
676-
pPayArgExt = allocatePayloadArgumentExt();
677-
}
678-
~PayloadArgElasticPtrBaseT() {
679-
freePayloadArgumentExt(pPayArgExt);
680-
}
681-
PayloadArgElasticPtrBaseT(const PayloadArgElasticPtrBaseT &src) {
682-
copyPayloadArgumentExt(pPayArgExt, src);
683-
}
684-
PayloadArgElasticPtrBaseT &operator=(const PayloadArgElasticPtrBaseT &rhs) {
685-
if (this != &rhs) {
686-
copyPayloadArgumentExt(pPayArgExt, rhs);
687-
}
688-
return *this;
689-
}
690-
PayloadArgElasticPtrBaseT(PayloadArgElasticPtrBaseT &&src) noexcept : pPayArgExt(src.pPayArgExt) {
691-
src.pPayArgExt = nullptr;
692-
}
693-
PayloadArgElasticPtrBaseT &operator=(PayloadArgElasticPtrBaseT &&rhs) noexcept {
694-
if (this != &rhs) {
695-
this->pPayArgExt = rhs.pPayArgExt;
696-
rhs.pPayArgExt = nullptr;
697-
}
698-
return *this;
699-
}
700-
PayloadArgumentExtT *pPayArgExt = nullptr;
701-
};
702-
703-
struct PayloadArgumentBaseT : PayloadArgElasticPtrBaseT {
670+
struct PayloadArgumentBaseT {
704671
ArgTypeT argType = argTypeUnknown;
705672
OffsetT offset = Defaults::offset;
706673
SourceOffseT sourceOffset = Defaults::sourceOffset;
@@ -717,6 +684,7 @@ struct PayloadArgumentBaseT : PayloadArgElasticPtrBaseT {
717684
bool imageTransformable = false;
718685
bool isPipe = false;
719686
bool isPtr = false;
687+
Ext<PayloadArgumentExtT> pPayArgExt;
720688
};
721689

722690
} // namespace PayloadArgument

shared/source/device_binary_format/zebin/zeinfo_decoder_ext.cpp

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,18 +32,16 @@ void freeExecEnvExt(ExecutionEnvExt *envExt) {
3232
void populateKernelExecutionEnvironmentExt(KernelDescriptor &dst, const KernelExecutionEnvBaseT &execEnv, const Types::Version &srcZeInfoVersion) {
3333
}
3434

35-
namespace Types::Kernel::PayloadArgument {
36-
PayloadArgumentExtT *allocatePayloadArgumentExt() {
37-
return nullptr;
38-
}
39-
void freePayloadArgumentExt(PayloadArgumentExtT *pPayArgExt) {
40-
}
41-
void copyPayloadArgumentExt(PayloadArgumentExtT *&pPayArgExtOut, const PayloadArgElasticPtrBaseT &src) {
42-
}
43-
} // namespace Types::Kernel::PayloadArgument
44-
4535
DecodeError populateKernelPayloadArgumentExt(NEO::KernelDescriptor &dst, const KernelPayloadArgBaseT &src, std::string &outErrReason) {
4636
return DecodeError::unhandledBinary;
4737
}
4838

4939
} // namespace NEO::Zebin::ZeInfo
40+
41+
template <>
42+
void cloneExt(ExtUniquePtrT<NEO::Zebin::ZeInfo::Types::Kernel::PayloadArgument::PayloadArgumentExtT> &dst, const NEO::Zebin::ZeInfo::Types::Kernel::PayloadArgument::PayloadArgumentExtT &src) {
43+
}
44+
45+
template <>
46+
void destroyExt(NEO::Zebin::ZeInfo::Types::Kernel::PayloadArgument::PayloadArgumentExtT *dst) {
47+
}

shared/source/utilities/mem_lifetime.h

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,32 +13,71 @@ template <typename T>
1313
using ExtUniquePtrT = std::unique_ptr<T, void (*)(T *)>;
1414

1515
template <typename T>
16-
void cloneExt(ExtUniquePtrT<T> &dst, const T *src);
16+
void cloneExt(ExtUniquePtrT<T> &dst, const T &src);
1717

1818
template <typename T>
1919
void destroyExt(T *dst);
2020

21+
namespace Impl {
22+
23+
template <typename ParentT>
24+
struct UniquePtrWrapperOps {
25+
auto operator*() const noexcept(noexcept(std::declval<decltype(ParentT::ptr)>().operator*())) {
26+
return *static_cast<const ParentT *>(this)->ptr;
27+
}
28+
29+
auto operator->() const noexcept {
30+
return static_cast<const ParentT *>(this)->ptr.operator->();
31+
}
32+
33+
explicit operator bool() const noexcept {
34+
return static_cast<bool>(static_cast<const ParentT *>(this)->ptr);
35+
}
36+
37+
template <typename Rhs>
38+
friend bool operator==(const ParentT &lhs, const Rhs &rhs) {
39+
return lhs.ptr == rhs;
40+
}
41+
42+
template <typename Lhs>
43+
friend bool operator==(const Lhs &lhs, const ParentT &rhs) {
44+
return lhs == rhs.ptr;
45+
}
46+
47+
friend bool operator==(const ParentT &lhs, const ParentT &rhs) {
48+
return lhs.ptr == rhs.ptr;
49+
}
50+
};
51+
52+
} // namespace Impl
53+
2154
template <typename T>
22-
struct Ext {
55+
struct Ext : Impl::UniquePtrWrapperOps<Ext<T>> {
2356
Ext(T *ptr) : ptr(ptr, destroyExt) {}
2457
Ext() = default;
2558
Ext(const Ext &rhs) {
26-
cloneExt(this->ptr, rhs.ptr.get());
59+
if (rhs.ptr.get()) {
60+
cloneExt(this->ptr, *rhs.ptr.get());
61+
}
2762
}
2863

2964
Ext &operator=(const Ext &rhs) {
3065
if (this == &rhs) {
3166
return *this;
3267
}
33-
cloneExt(this->ptr, rhs.ptr.get());
68+
if (rhs.ptr.get()) {
69+
cloneExt(this->ptr, *rhs.ptr.get());
70+
} else {
71+
ptr.reset();
72+
}
3473
return *this;
3574
}
3675

3776
ExtUniquePtrT<T> ptr{nullptr, destroyExt};
3877
};
3978

4079
template <typename T>
41-
struct Clonable {
80+
struct Clonable : Impl::UniquePtrWrapperOps<Clonable<Ext<T>>> {
4281
Clonable(T *ptr) : ptr(ptr) {}
4382
Clonable() = default;
4483
Clonable(const Clonable &rhs) {

shared/test/unit_test/device_binary_format/zebin_decoder_exclusive_tests.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,4 @@ TEST(ExtBaseKernelDescriptorAndPayloadArgumentPointers, givenKernelDescriptorAnd
1717

1818
EXPECT_EQ(nullptr, kd.kernelDescriptorExt);
1919
EXPECT_EQ(nullptr, arg.pPayArgExt);
20-
EXPECT_EQ(nullptr, NEO::Zebin::ZeInfo::Types::Kernel::PayloadArgument::allocatePayloadArgumentExt());
2120
}

shared/test/unit_test/device_binary_format/zebin_decoder_tests.cpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2394,11 +2394,7 @@ TEST(BaseKernelDescriptorAndPayloadArgumentPoinetrsExt, givenKernelDescriptorAnd
23942394
NEO::KernelDescriptor kd;
23952395
NEO::Zebin::ZeInfo::KernelPayloadArgBaseT arg;
23962396

2397-
if (nullptr != kd.kernelDescriptorExt) {
2398-
EXPECT_NE(nullptr, arg.pPayArgExt);
2399-
} else {
2400-
EXPECT_EQ(nullptr, arg.pPayArgExt);
2401-
}
2397+
EXPECT_EQ(nullptr, arg.pPayArgExt);
24022398
}
24032399

24042400
TEST(PopulateKernelSourceAttributes, GivenInvalidKernelAttributeWhenPopulatingKernelSourceAttributesThenKernelIsInvalidFlagIsSet) {

shared/test/unit_test/utilities/mem_lifetime_tests.cpp

Lines changed: 90 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ struct TestStruct {
2222
};
2323

2424
template <>
25-
void cloneExt(ExtUniquePtrT<TestStruct> &dst, const TestStruct *src) {
25+
void cloneExt(ExtUniquePtrT<TestStruct> &dst, const TestStruct &src) {
2626
dst.reset(new TestStruct{true});
2727
}
2828

@@ -89,4 +89,92 @@ TEST(Clonable, GivenTypeThenProvidesClonableUniquePtr) {
8989
auto &re = e;
9090
e = re;
9191
EXPECT_EQ(nullptr, e.ptr.get());
92-
}
92+
}
93+
94+
struct PtrOpsMock {
95+
auto operator*() const noexcept {
96+
opCalled |= deref;
97+
return *this;
98+
}
99+
100+
auto operator->() const noexcept {
101+
opCalled |= arrow;
102+
return this;
103+
}
104+
105+
explicit operator bool() const noexcept {
106+
opCalled |= boolCast;
107+
return true;
108+
}
109+
110+
enum OpCalled : uint32_t {
111+
deref = 1 << 0,
112+
arrow = 1 << 1,
113+
boolCast = 1 << 2,
114+
compareAsLhs = 1 << 3,
115+
compareAsRhs = 1 << 4,
116+
compare = 1 << 5,
117+
};
118+
119+
mutable uint32_t opCalled = 0;
120+
};
121+
122+
static bool operator==(const PtrOpsMock &lhs, const void *) {
123+
lhs.opCalled |= PtrOpsMock::compareAsLhs;
124+
return true;
125+
}
126+
127+
static bool operator==(const void *, const PtrOpsMock &rhs) {
128+
rhs.opCalled |= PtrOpsMock::compareAsRhs;
129+
return true;
130+
}
131+
132+
static bool operator==(const PtrOpsMock &lhs, const PtrOpsMock &rhs) {
133+
lhs.opCalled |= PtrOpsMock::compare;
134+
rhs.opCalled |= PtrOpsMock::compare;
135+
return true;
136+
}
137+
138+
TEST(Clonable, WhenUsingUniquePtrWrapperOpsThenForwardsToParentsPtr) {
139+
struct Parent : Impl::UniquePtrWrapperOps<Parent> {
140+
PtrOpsMock ptr;
141+
};
142+
143+
{
144+
Parent parent;
145+
[[maybe_unused]] auto res = *parent;
146+
EXPECT_EQ(PtrOpsMock::deref, parent.ptr.opCalled);
147+
}
148+
149+
{
150+
Parent parent;
151+
[[maybe_unused]] auto res = parent->opCalled;
152+
EXPECT_EQ(PtrOpsMock::arrow, parent.ptr.opCalled);
153+
}
154+
155+
{
156+
Parent parent;
157+
[[maybe_unused]] auto res = static_cast<bool>(parent);
158+
EXPECT_EQ(PtrOpsMock::boolCast, parent.ptr.opCalled);
159+
}
160+
161+
{
162+
Parent parent;
163+
[[maybe_unused]] auto res = parent == nullptr;
164+
EXPECT_EQ(PtrOpsMock::compareAsLhs, parent.ptr.opCalled);
165+
}
166+
167+
{
168+
Parent parent;
169+
[[maybe_unused]] auto res = nullptr == parent;
170+
EXPECT_EQ(PtrOpsMock::compareAsRhs, parent.ptr.opCalled);
171+
}
172+
173+
{
174+
Parent parentA;
175+
Parent parentB;
176+
[[maybe_unused]] auto res = parentA == parentB;
177+
EXPECT_EQ(PtrOpsMock::compare, parentA.ptr.opCalled);
178+
EXPECT_EQ(PtrOpsMock::compare, parentB.ptr.opCalled);
179+
}
180+
}

0 commit comments

Comments
 (0)