Skip to content

Commit 21e9261

Browse files
atmnpatelatmnpatel
committed
[Libomptarget] Experimental Remote Plugin Fixes
D97883 introduced a compile-time error in the experimental remote offloading libomptarget plugin, this patch fixes it and resolves a number of inconsistencies in the plugin as well: 1. Non-functional Asynchronous API 2. Unnecessarily verbose debug printing 3. Misc. code clean ups This is not intended to make any functional changes to the plugin. Differential Revision: https://reviews.llvm.org/D105325
1 parent f239026 commit 21e9261

File tree

9 files changed

+283
-515
lines changed

9 files changed

+283
-515
lines changed

‎openmp/libomptarget/plugins/remote/include/Utils.h

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,20 +47,39 @@ using openmp::libomptarget::remote::TargetBinaryDescription;
4747
using openmp::libomptarget::remote::TargetOffloadEntry;
4848
using openmp::libomptarget::remote::TargetTable;
4949

50-
struct RPCConfig {
50+
struct ClientManagerConfigTy {
5151
std::vector<std::string> ServerAddresses;
5252
uint64_t MaxSize;
5353
uint64_t BlockSize;
54-
RPCConfig() {
55-
ServerAddresses = {"0.0.0.0:50051"};
56-
MaxSize = 1 << 30;
57-
BlockSize = 1 << 20;
54+
int Timeout;
55+
56+
ClientManagerConfigTy()
57+
: ServerAddresses({"0.0.0.0:50051"}), MaxSize(1 << 30),
58+
BlockSize(1 << 20), Timeout(5) {
59+
// TODO: Error handle for incorrect inputs
60+
if (const char *Env = std::getenv("LIBOMPTARGET_RPC_ADDRESS")) {
61+
ServerAddresses.clear();
62+
std::string AddressString = Env;
63+
const std::string Delimiter = ",";
64+
65+
size_t Pos;
66+
std::string Token;
67+
while ((Pos = AddressString.find(Delimiter)) != std::string::npos) {
68+
Token = AddressString.substr(0, Pos);
69+
ServerAddresses.push_back(Token);
70+
AddressString.erase(0, Pos + Delimiter.length());
71+
}
72+
ServerAddresses.push_back(AddressString);
73+
}
74+
if (const char *Env = std::getenv("LIBOMPTARGET_RPC_ALLOCATOR_MAX"))
75+
MaxSize = std::stoi(Env);
76+
if (const char *Env = std::getenv("LIBOMPTARGET_RPC_BLOCK_SIZE"))
77+
BlockSize = std::stoi(Env);
78+
if (const char *Env1 = std::getenv("LIBOMPTARGET_RPC_LATENCY"))
79+
Timeout = std::stoi(Env1);
5880
}
5981
};
6082

61-
/// Helper function to parse common environment variables between client/server
62-
void parseEnvironment(RPCConfig &Config);
63-
6483
/// Loads a target binary description into protobuf.
6584
void loadTargetBinaryDescription(const __tgt_bin_desc *Desc,
6685
TargetBinaryDescription &Request);

‎openmp/libomptarget/plugins/remote/include/openmp.proto

Lines changed: 16 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,18 @@ service RemoteOffload {
1616
rpc InitRequires(I64) returns (I32) {}
1717

1818
rpc LoadBinary(Binary) returns (TargetTable) {}
19-
rpc Synchronize(SynchronizeDevice) returns (I32) {}
2019

2120
rpc DataAlloc(AllocData) returns (Pointer) {}
2221
rpc DataDelete(DeleteData) returns (I32) {}
2322

24-
rpc DataSubmitAsync(stream SubmitDataAsync) returns (I32) {}
25-
rpc DataRetrieveAsync(RetrieveDataAsync) returns (stream Data) {}
23+
rpc DataSubmit(stream SubmitData) returns (I32) {}
24+
rpc DataRetrieve(RetrieveData) returns (stream Data) {}
2625

2726
rpc IsDataExchangeable(DevicePair) returns (I32) {}
28-
rpc DataExchangeAsync(ExchangeDataAsync) returns (I32) {}
27+
rpc DataExchange(ExchangeData) returns (I32) {}
2928

30-
rpc RunTargetRegionAsync(TargetRegionAsync) returns (I32) {}
31-
rpc RunTargetTeamRegionAsync(TargetTeamRegionAsync) returns (I32) {}
29+
rpc RunTargetRegion(TargetRegion) returns (I32) {}
30+
rpc RunTargetTeamRegion(TargetTeamRegion) returns (I32) {}
3231
}
3332

3433
message Null {}
@@ -92,32 +91,25 @@ message TargetBinaryDescription {
9291
uint64 bin_ptr = 5;
9392
}
9493

95-
message SynchronizeDevice {
96-
uint64 queue_ptr = 1;
97-
int32 device_id = 2;
98-
}
99-
10094
message AllocData {
10195
uint64 size = 1;
10296
uint64 hst_ptr = 2;
10397
int32 device_id = 3;
10498
}
10599

106-
message SubmitDataAsync {
100+
message SubmitData {
107101
bytes data = 1;
108102
uint64 hst_ptr = 2;
109103
uint64 tgt_ptr = 3;
110-
uint64 queue_ptr = 4;
111104
uint64 start = 5;
112105
uint64 size = 6;
113106
int32 device_id = 7;
114107
}
115108

116-
message RetrieveDataAsync {
109+
message RetrieveData {
117110
uint64 hst_ptr = 1;
118111
uint64 tgt_ptr = 2;
119112
uint64 size = 3;
120-
uint64 queue_ptr = 4;
121113
int32 device_id = 5;
122114
}
123115

@@ -128,12 +120,11 @@ message Data {
128120
int32 ret = 4;
129121
}
130122

131-
message ExchangeDataAsync {
123+
message ExchangeData {
132124
uint64 src_dev_id = 1;
133125
uint64 src_ptr = 2;
134126
uint64 dst_dev_id = 3;
135127
uint64 dst_ptr = 4;
136-
uint64 queue_ptr = 5;
137128
uint64 size = 6;
138129
}
139130

@@ -142,23 +133,21 @@ message DeleteData {
142133
int32 device_id = 2;
143134
}
144135

145-
message TargetRegionAsync {
136+
message TargetRegion {
146137
repeated uint64 tgt_args = 1;
147138
repeated int64 tgt_offsets = 2;
148139
uint64 tgt_entry_ptr = 3;
149-
uint64 queue_ptr = 4;
150-
int32 device_id = 5;
151-
int32 arg_num = 6;
140+
int32 device_id = 4;
141+
int32 arg_num = 5;
152142
}
153143

154-
message TargetTeamRegionAsync {
144+
message TargetTeamRegion {
155145
repeated uint64 tgt_args = 1;
156146
repeated int64 tgt_offsets = 2;
157147
uint64 tgt_entry_ptr = 3;
158148
uint64 loop_tripcount = 4;
159-
uint64 queue_ptr = 5;
160-
int32 device_id = 6;
161-
int32 arg_num = 7;
162-
int32 team_num = 8;
163-
int32 thread_limit = 9;
149+
int32 device_id = 5;
150+
int32 arg_num = 6;
151+
int32 team_num = 7;
152+
int32 thread_limit = 8;
164153
}

‎openmp/libomptarget/plugins/remote/lib/Utils.cpp

Lines changed: 18 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -14,27 +14,6 @@
1414
#include "omptarget.h"
1515

1616
namespace RemoteOffloading {
17-
void parseEnvironment(RPCConfig &Config) {
18-
// TODO: Error handle for incorrect inputs
19-
if (const char *Env = std::getenv("LIBOMPTARGET_RPC_ADDRESS")) {
20-
Config.ServerAddresses.clear();
21-
std::string AddressString = Env;
22-
const std::string Delimiter = ",";
23-
24-
size_t Pos = 0;
25-
std::string Token;
26-
while ((Pos = AddressString.find(Delimiter)) != std::string::npos) {
27-
Token = AddressString.substr(0, Pos);
28-
Config.ServerAddresses.push_back(Token);
29-
AddressString.erase(0, Pos + Delimiter.length());
30-
}
31-
Config.ServerAddresses.push_back(AddressString);
32-
}
33-
if (const char *Env = std::getenv("LIBOMPTARGET_RPC_ALLOCATOR_MAX"))
34-
Config.MaxSize = std::stoi(Env);
35-
if (const char *Env = std::getenv("LIBOMPTARGET_RPC_BLOCK_SIZE"))
36-
Config.BlockSize = std::stoi(Env);
37-
}
3817

3918
void loadTargetBinaryDescription(const __tgt_bin_desc *Desc,
4019
TargetBinaryDescription &Request) {
@@ -101,10 +80,12 @@ void unloadTargetBinaryDescription(
10180

10281
// Copy Global Offload Entries
10382
__tgt_offload_entry *CurEntry = Desc->HostEntriesBegin;
104-
for (int i = 0; i < Request->entries_size(); i++) {
105-
copyOffloadEntry(Request->entries()[i], CurEntry);
106-
CopiedOffloadEntries[(void *)Request->entry_ptrs()[i]] = CurEntry;
83+
size_t I = 0;
84+
for (auto &Entry : Request->entries()) {
85+
copyOffloadEntry(Entry, CurEntry);
86+
CopiedOffloadEntries[(void *)Request->entry_ptrs()[I]] = CurEntry;
10787
CurEntry++;
88+
I++;
10889
}
10990
Desc->HostEntriesEnd = CurEntry;
11091

@@ -113,29 +94,27 @@ void unloadTargetBinaryDescription(
11394
auto ImageItr = Request->image_ptrs().begin();
11495
for (auto Image : Request->images()) {
11596
// Copy Device Offload Entries
116-
auto *CurEntry = Desc->HostEntriesBegin;
97+
CurEntry = Desc->HostEntriesBegin;
11798
bool Found = false;
11899

119100
if (!Desc->HostEntriesBegin) {
120101
CurImage->EntriesBegin = nullptr;
121102
CurImage->EntriesEnd = nullptr;
122103
}
123104

124-
for (int i = 0; i < Image.entries_size(); i++) {
105+
for (size_t I = 0; I < Image.entries_size(); I++) {
125106
auto TgtEntry =
126-
CopiedOffloadEntries.find((void *)Request->entry_ptrs()[i]);
107+
CopiedOffloadEntries.find((void *)Request->entry_ptrs()[I]);
127108
if (TgtEntry != CopiedOffloadEntries.end()) {
128109
if (!Found)
129110
CurImage->EntriesBegin = CurEntry;
130111

112+
CurImage->EntriesEnd = CurEntry + 1;
131113
Found = true;
132-
if (Found) {
133-
CurImage->EntriesEnd = CurEntry + 1;
134-
}
135114
} else {
136115
Found = false;
137-
copyOffloadEntry(Image.entries()[i], CurEntry);
138-
CopiedOffloadEntries[(void *)(Request->entry_ptrs()[i])] = CurEntry;
116+
copyOffloadEntry(Image.entries()[I], CurEntry);
117+
CopiedOffloadEntries[(void *)(Request->entry_ptrs()[I])] = CurEntry;
139118
}
140119
CurEntry++;
141120
}
@@ -199,10 +178,10 @@ void unloadTargetTable(
199178
Table->EntriesBegin = new __tgt_offload_entry[TableResponse.entries_size()];
200179

201180
auto *CurEntry = Table->EntriesBegin;
202-
for (int i = 0; i < TableResponse.entries_size(); i++) {
203-
copyOffloadEntry(TableResponse.entries()[i], CurEntry);
181+
for (size_t I = 0; I < TableResponse.entries_size(); I++) {
182+
copyOffloadEntry(TableResponse.entries()[I], CurEntry);
204183
HostToRemoteTargetTableMap[CurEntry->addr] =
205-
(void *)TableResponse.entry_ptrs()[i];
184+
(void *)TableResponse.entry_ptrs()[I];
206185
CurEntry++;
207186
}
208187
Table->EntriesEnd = CurEntry;
@@ -292,10 +271,10 @@ void dump(__tgt_target_table *Table) {
292271

293272
void dump(TargetOffloadEntry Entry) {
294273
fprintf(stderr, "Entry: ");
295-
fprintf(stderr, " %s\n", Entry.name().c_str());
296-
fprintf(stderr, " %d\n", Entry.reserved());
297-
fprintf(stderr, " %d\n", Entry.flags());
298-
fprintf(stderr, " %ld\n", Entry.data().size());
274+
fprintf(stderr, " Name: %s\n", Entry.name().c_str());
275+
fprintf(stderr, " Reserved: %d\n", Entry.reserved());
276+
fprintf(stderr, " Flags: %d\n", Entry.flags());
277+
fprintf(stderr, " Size: %ld\n", Entry.data().size());
299278
dump(static_cast<const void *>(Entry.data().data()),
300279
static_cast<const void *>((Entry.data().c_str() + Entry.data().size())));
301280
}

‎openmp/libomptarget/plugins/remote/server/OffloadingServer.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,7 @@ using grpc::ServerBuilder;
2424
std::promise<void> ShutdownPromise;
2525

2626
int main() {
27-
RPCConfig Config;
28-
parseEnvironment(Config);
27+
ClientManagerConfigTy Config;
2928

3029
RemoteOffloadImpl Service(Config.MaxSize, Config.BlockSize);
3130

0 commit comments

Comments
 (0)