Skip to content

Commit c2e314a

Browse files
benvanikbenvanik
authored
Adding _base variants of all string view int parsing and clean up hip options. (iree-org#21086)
A uint64 version was added awhile back but the rest were missing. C's strtol and co all take a base and this just passes that through. The non-_base methods assume 0 ("interpret from string") as before. Also cleaned up hip driver int parsing code a bit (noticed while touching the string view functions).
1 parent 95cc014 commit c2e314a

File tree

5 files changed

+129
-114
lines changed

5 files changed

+129
-114
lines changed

‎runtime/src/iree/base/string_view.c‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -315,8 +315,9 @@ IREE_API_EXPORT iree_host_size_t iree_string_view_append_to_buffer(
315315
// perform. These _should_ never be on a hot path, though, so this keeps our
316316
// code size small.
317317

318-
IREE_API_EXPORT bool iree_string_view_atoi_int32(iree_string_view_t value,
319-
int32_t* out_value) {
318+
IREE_API_EXPORT bool iree_string_view_atoi_int32_base(iree_string_view_t value,
319+
int base,
320+
int32_t* out_value) {
320321
// Copy to scratch memory with a NUL terminator.
321322
char temp[16] = {0};
322323
if (value.size >= IREE_ARRAYSIZE(temp)) return false;
@@ -335,8 +336,14 @@ IREE_API_EXPORT bool iree_string_view_atoi_int32(iree_string_view_t value,
335336
return parsed_value != 0 || errno == 0;
336337
}
337338

338-
IREE_API_EXPORT bool iree_string_view_atoi_uint32(iree_string_view_t value,
339-
uint32_t* out_value) {
339+
IREE_API_EXPORT bool iree_string_view_atoi_int32(iree_string_view_t value,
340+
int32_t* out_value) {
341+
return iree_string_view_atoi_int32_base(value, /*base=*/0, out_value);
342+
}
343+
344+
IREE_API_EXPORT bool iree_string_view_atoi_uint32_base(iree_string_view_t value,
345+
int base,
346+
uint32_t* out_value) {
340347
// Copy to scratch memory with a NUL terminator.
341348
char temp[16] = {0};
342349
if (value.size >= IREE_ARRAYSIZE(temp)) return false;
@@ -345,15 +352,21 @@ IREE_API_EXPORT bool iree_string_view_atoi_uint32(iree_string_view_t value,
345352
// Attempt to parse.
346353
errno = 0;
347354
char* end = NULL;
348-
unsigned long parsed_value = strtoul(temp, &end, 0);
355+
unsigned long parsed_value = strtoul(temp, &end, base);
349356
if (temp == end) return false;
350357
if (parsed_value == ULONG_MAX && errno == ERANGE) return false;
351358
*out_value = (uint32_t)parsed_value;
352359
return parsed_value != 0 || errno == 0;
353360
}
354361

355-
IREE_API_EXPORT bool iree_string_view_atoi_int64(iree_string_view_t value,
356-
int64_t* out_value) {
362+
IREE_API_EXPORT bool iree_string_view_atoi_uint32(iree_string_view_t value,
363+
uint32_t* out_value) {
364+
return iree_string_view_atoi_uint32_base(value, /*base=*/0, out_value);
365+
}
366+
367+
IREE_API_EXPORT bool iree_string_view_atoi_int64_base(iree_string_view_t value,
368+
int base,
369+
int64_t* out_value) {
357370
// Copy to scratch memory with a NUL terminator.
358371
char temp[32] = {0};
359372
if (value.size >= IREE_ARRAYSIZE(temp)) return false;
@@ -362,7 +375,7 @@ IREE_API_EXPORT bool iree_string_view_atoi_int64(iree_string_view_t value,
362375
// Attempt to parse.
363376
errno = 0;
364377
char* end = NULL;
365-
long long parsed_value = strtoll(temp, &end, 0);
378+
long long parsed_value = strtoll(temp, &end, base);
366379
if (temp == end) return false;
367380
if ((parsed_value == LLONG_MIN || parsed_value == LLONG_MAX) &&
368381
errno == ERANGE) {
@@ -372,6 +385,11 @@ IREE_API_EXPORT bool iree_string_view_atoi_int64(iree_string_view_t value,
372385
return parsed_value != 0 || errno == 0;
373386
}
374387

388+
IREE_API_EXPORT bool iree_string_view_atoi_int64(iree_string_view_t value,
389+
int64_t* out_value) {
390+
return iree_string_view_atoi_int64_base(value, /*base=*/0, out_value);
391+
}
392+
375393
IREE_API_EXPORT bool iree_string_view_atoi_uint64_base(iree_string_view_t value,
376394
int base,
377395
uint64_t* out_value) {
@@ -392,7 +410,7 @@ IREE_API_EXPORT bool iree_string_view_atoi_uint64_base(iree_string_view_t value,
392410

393411
IREE_API_EXPORT bool iree_string_view_atoi_uint64(iree_string_view_t value,
394412
uint64_t* out_value) {
395-
return iree_string_view_atoi_uint64_base(value, 0, out_value);
413+
return iree_string_view_atoi_uint64_base(value, /*base=*/0, out_value);
396414
}
397415

398416
IREE_API_EXPORT bool iree_string_view_atof(iree_string_view_t value,

‎runtime/src/iree/base/string_view.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,10 +232,19 @@ IREE_API_EXPORT iree_host_size_t iree_string_view_append_to_buffer(
232232
iree_string_view_t source_value, iree_string_view_t* target_value,
233233
char* buffer);
234234

235+
IREE_API_EXPORT bool iree_string_view_atoi_int32_base(iree_string_view_t value,
236+
int base,
237+
int32_t* out_value);
235238
IREE_API_EXPORT bool iree_string_view_atoi_int32(iree_string_view_t value,
236239
int32_t* out_value);
240+
IREE_API_EXPORT bool iree_string_view_atoi_uint32_base(iree_string_view_t value,
241+
int base,
242+
uint32_t* out_value);
237243
IREE_API_EXPORT bool iree_string_view_atoi_uint32(iree_string_view_t value,
238244
uint32_t* out_value);
245+
IREE_API_EXPORT bool iree_string_view_atoi_int64_base(iree_string_view_t value,
246+
int base,
247+
int64_t* out_value);
239248
IREE_API_EXPORT bool iree_string_view_atoi_int64(iree_string_view_t value,
240249
int64_t* out_value);
241250
IREE_API_EXPORT bool iree_string_view_atoi_uint64_base(iree_string_view_t value,

‎runtime/src/iree/hal/drivers/hip/api.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ IREE_API_EXPORT void iree_hal_hip_device_params_initialize(
126126
typedef struct iree_hal_hip_driver_options_t {
127127
// The index of the default HIP device to use within the list of available
128128
// devices.
129-
int default_device_index;
129+
int32_t default_device_index;
130130

131131
// List of paths to guide searching for the dynamic libamdhip64.so (or
132132
// amdhip64.dll), which contains the backing HIP runtime library. If this

‎runtime/src/iree/hal/drivers/hip/hip_driver.c‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -347,8 +347,16 @@ static iree_status_t iree_hal_hip_driver_select_default_device(
347347
return status;
348348
}
349349

350-
static const iree_string_view_t key_hip_external_stream =
351-
iree_string_view_literal("hip_external_stream");
350+
static iree_status_t iree_hal_hip_try_parse_uint64_option(
351+
iree_string_view_t name, iree_string_view_t value, uint64_t* out_value) {
352+
if (!iree_string_view_atoi_uint64(value, out_value)) {
353+
return iree_make_status(
354+
IREE_STATUS_FAILED_PRECONDITION,
355+
"option '%*.s' expected a uint64 value, got: '%.*s'", (int)name.size,
356+
name.data, (int)value.size, value.data);
357+
}
358+
return iree_ok_status();
359+
}
352360

353361
static iree_status_t iree_hal_hip_driver_create_device_by_id(
354362
iree_hal_driver_t* base_driver, iree_hal_device_id_t device_id,
@@ -376,15 +384,11 @@ static iree_status_t iree_hal_hip_driver_create_device_by_id(
376384

377385
iree_hal_hip_device_params_t device_params = driver->device_params;
378386
for (iree_host_size_t i = 0; i < param_count; ++i) {
379-
if (iree_string_view_equal(params[i].key, key_hip_external_stream)) {
380-
uint64_t luvalue = 0;
381-
if (!iree_string_view_atoi_uint64(params[i].value, &luvalue)) {
382-
return iree_make_status(
383-
IREE_STATUS_FAILED_PRECONDITION,
384-
"option 'hip_external_stream' expected to be uint64, Got: '%.*s'",
385-
(int)params[i].value.size, params[i].value.data);
386-
}
387-
device_params.external_stream = luvalue;
387+
if (iree_string_view_equal(params[i].key, IREE_SV("hip_external_stream"))) {
388+
IREE_RETURN_AND_END_ZONE_IF_ERROR(
389+
z0,
390+
iree_hal_hip_try_parse_uint64_option(params[i].key, params[i].value,
391+
&device_params.external_stream));
388392
}
389393
}
390394

‎runtime/src/iree/hal/drivers/hip/registration/driver_module.c‎

Lines changed: 77 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -69,76 +69,89 @@ static iree_status_t iree_hal_hip_driver_factory_enumerate(
6969
return iree_ok_status();
7070
}
7171

72-
// Option key constants.
73-
static const iree_string_view_t key_hip_dylib_path =
74-
iree_string_view_literal("hip_dylib_path");
75-
static const iree_string_view_t key_hip_use_streams =
76-
iree_string_view_literal("hip_use_streams");
77-
static const iree_string_view_t key_hip_allow_inline_execution =
78-
iree_string_view_literal("hip_allow_inline_execution");
79-
static const iree_string_view_t key_hip_async_allocations =
80-
iree_string_view_literal("hip_async_allocations");
81-
static const iree_string_view_t key_hip_async_caching =
82-
iree_string_view_literal("hip_async_caching");
83-
static const iree_string_view_t key_hip_tracing =
84-
iree_string_view_literal("hip_tracing");
85-
static const iree_string_view_t key_hip_default_index =
86-
iree_string_view_literal("hip_default_index");
87-
8872
// Parses flags and environment variables into a string pair builder.
8973
static iree_status_t iree_hal_hip_driver_parse_flags(
9074
iree_string_pair_builder_t* builder) {
9175
const iree_flag_string_list_t dylib_path_list = FLAG_hip_dylib_path_list();
9276

9377
// hip_dylib_path
78+
// TODO: make this a single key-value pair (semicolon separated).
79+
// Repeated fields don't work in things like python dictionaries/JSON and we
80+
// want to match the environment variable formatting.
9481
for (iree_host_size_t i = 0; i < dylib_path_list.count; ++i) {
9582
IREE_RETURN_IF_ERROR(iree_string_pair_builder_add(
96-
builder,
97-
iree_make_string_pair(key_hip_dylib_path, dylib_path_list.values[i])));
83+
builder, iree_make_string_pair(IREE_SV("hip_dylib_path"),
84+
dylib_path_list.values[i])));
9885
}
9986

10087
// bool and int flags
10188
IREE_RETURN_IF_ERROR(iree_string_pair_builder_add_int32(
102-
builder, key_hip_use_streams, FLAG_hip_use_streams));
89+
builder, IREE_SV("hip_use_streams"), FLAG_hip_use_streams));
10390
IREE_RETURN_IF_ERROR(iree_string_pair_builder_add_int32(
104-
builder, key_hip_allow_inline_execution,
91+
builder, IREE_SV("hip_allow_inline_execution"),
10592
FLAG_hip_allow_inline_execution));
10693
IREE_RETURN_IF_ERROR(iree_string_pair_builder_add_int32(
107-
builder, key_hip_async_allocations, FLAG_hip_async_allocations));
94+
builder, IREE_SV("hip_async_allocations"), FLAG_hip_async_allocations));
10895
IREE_RETURN_IF_ERROR(iree_string_pair_builder_add_int32(
109-
builder, key_hip_async_caching, FLAG_hip_async_caching));
96+
builder, IREE_SV("hip_async_caching"), FLAG_hip_async_caching));
11097
IREE_RETURN_IF_ERROR(iree_string_pair_builder_add_int32(
111-
builder, key_hip_tracing, FLAG_hip_tracing));
98+
builder, IREE_SV("hip_tracing"), FLAG_hip_tracing));
11299
IREE_RETURN_IF_ERROR(iree_string_pair_builder_add_int32(
113-
builder, key_hip_default_index, FLAG_hip_default_index));
100+
builder, IREE_SV("hip_default_index"), FLAG_hip_default_index));
114101

115102
// If there were no flag-based dylib paths, consult the environment
116103
// variable.
117104
// Read from the IREE_HIP_DYLIB_PATH env var and split by ';' (regardless
118105
// of platform).
106+
//
107+
// TODO: make this a single key-value pair (semicolon separated).
108+
// Repeated fields don't work in things like python dictionaries/JSON and we
109+
// want to match the environment variable formatting.
119110
if (dylib_path_list.count == 0) {
120111
char* raw_dylib_path_env = getenv("IREE_HIP_DYLIB_PATH");
121112
if (raw_dylib_path_env) {
122113
iree_string_view_t dylib_path_env =
123114
iree_make_cstring_view(raw_dylib_path_env);
124115
IREE_RETURN_IF_ERROR(
125116
iree_string_pair_builder_emplace_string(builder, &dylib_path_env));
126-
while (true) {
127-
iree_string_view_t first, rest;
128-
intptr_t index =
129-
iree_string_view_split(dylib_path_env, ';', &first, &rest);
117+
intptr_t split_index = 0;
118+
do {
119+
iree_string_view_t value;
120+
split_index = iree_string_view_split(dylib_path_env, ';', &value,
121+
&dylib_path_env);
130122
IREE_RETURN_IF_ERROR(iree_string_pair_builder_add(
131-
builder, iree_make_string_pair(key_hip_dylib_path, first)));
123+
builder, iree_make_string_pair(IREE_SV("hip_dylib_path"), value)));
124+
} while (split_index != -1);
125+
}
126+
}
132127

133-
if (index < 0) {
134-
break;
135-
}
128+
return iree_ok_status();
129+
}
136130

137-
dylib_path_env = rest;
138-
}
139-
}
131+
static iree_status_t iree_hal_hip_try_parse_int32_option(
132+
iree_string_view_t name, iree_string_view_t value, int32_t* out_value) {
133+
if (!iree_string_view_atoi_int32(value, out_value)) {
134+
return iree_make_status(
135+
IREE_STATUS_FAILED_PRECONDITION,
136+
"option '%*.s' expected an int32 value, got: '%.*s'", (int)name.size,
137+
name.data, (int)value.size, value.data);
140138
}
139+
return iree_ok_status();
140+
}
141141

142+
static iree_status_t iree_hal_hip_try_parse_bool_option(
143+
iree_string_view_t name, iree_string_view_t value, bool* out_value) {
144+
if (iree_string_view_equal(value, IREE_SV("true"))) {
145+
*out_value = 1;
146+
return iree_ok_status();
147+
} else if (iree_string_view_equal(value, IREE_SV("false"))) {
148+
*out_value = 0;
149+
return iree_ok_status();
150+
}
151+
int32_t int_value = 0;
152+
IREE_RETURN_IF_ERROR(
153+
iree_hal_hip_try_parse_int32_option(name, value, &int_value));
154+
*out_value = int_value != 0;
142155
return iree_ok_status();
143156
}
144157

@@ -152,72 +165,43 @@ static iree_status_t iree_hal_hip_driver_populate_options(
152165
for (iree_host_size_t i = 0; i < pairs_size; ++i) {
153166
iree_string_view_t key = pairs[i].key;
154167
iree_string_view_t value = pairs[i].value;
155-
int32_t ivalue;
156-
157-
if (iree_string_view_equal(key, key_hip_dylib_path)) {
168+
if (iree_string_view_equal(key, IREE_SV("hip_dylib_path"))) {
158169
++dylib_path_count;
159-
} else if (iree_string_view_equal(key, key_hip_use_streams)) {
160-
if (!iree_string_view_atoi_int32(value, &ivalue)) {
161-
return iree_make_status(
162-
IREE_STATUS_FAILED_PRECONDITION,
163-
"Option 'hip_use_streams' expected to be int. Got: '%.*s'",
164-
(int)value.size, value.data);
165-
}
170+
} else if (iree_string_view_equal(key, IREE_SV("hip_use_streams"))) {
171+
bool use_streams = false;
172+
IREE_RETURN_IF_ERROR(
173+
iree_hal_hip_try_parse_bool_option(key, value, &use_streams));
166174
device_params->command_buffer_mode =
167-
ivalue ? IREE_HAL_HIP_COMMAND_BUFFER_MODE_STREAM
168-
: IREE_HAL_HIP_COMMAND_BUFFER_MODE_GRAPH;
169-
} else if (iree_string_view_equal(key, key_hip_allow_inline_execution)) {
170-
if (!iree_string_view_atoi_int32(value, &ivalue)) {
171-
return iree_make_status(
172-
IREE_STATUS_FAILED_PRECONDITION,
173-
"Option 'hip_allow_inline_execution' expected to be "
174-
"int. Got: '%.*s'",
175-
(int)value.size, value.data);
176-
}
177-
if (ivalue) {
178-
device_params->allow_inline_execution = ivalue ? true : false;
179-
}
180-
} else if (iree_string_view_equal(key, key_hip_async_allocations)) {
181-
if (!iree_string_view_atoi_int32(value, &ivalue)) {
182-
return iree_make_status(
183-
IREE_STATUS_FAILED_PRECONDITION,
184-
"Option 'hip_async_allocations' expected to be int Got: '%.*s'",
185-
(int)value.size, value.data);
186-
}
187-
device_params->async_allocations = ivalue ? true : false;
188-
} else if (iree_string_view_equal(key, key_hip_async_caching)) {
189-
if (!iree_string_view_atoi_int32(value, &ivalue)) {
190-
return iree_make_status(
191-
IREE_STATUS_FAILED_PRECONDITION,
192-
"Option 'hip_async_caching' expected to be int Got: '%.*s'",
193-
(int)value.size, value.data);
194-
}
195-
device_params->async_caching = ivalue ? true : false;
196-
} else if (iree_string_view_equal(key, key_hip_tracing)) {
197-
if (!iree_string_view_atoi_int32(value, &ivalue)) {
198-
return iree_make_status(
199-
IREE_STATUS_FAILED_PRECONDITION,
200-
"Option 'hip_tracing' expected to be int. Got: '%.*s'",
201-
(int)value.size, value.data);
202-
}
203-
device_params->stream_tracing = ivalue;
204-
} else if (iree_string_view_equal(key, key_hip_default_index)) {
205-
if (!iree_string_view_atoi_int32(value, &ivalue)) {
206-
return iree_make_status(
207-
IREE_STATUS_FAILED_PRECONDITION,
208-
"Option 'hip_default_index' expected to be int. Got: '%.*s'",
209-
(int)value.size, value.data);
210-
;
211-
}
212-
driver_options->default_device_index = ivalue;
175+
use_streams ? IREE_HAL_HIP_COMMAND_BUFFER_MODE_STREAM
176+
: IREE_HAL_HIP_COMMAND_BUFFER_MODE_GRAPH;
177+
} else if (iree_string_view_equal(key,
178+
IREE_SV("hip_allow_inline_execution"))) {
179+
IREE_RETURN_IF_ERROR(iree_hal_hip_try_parse_bool_option(
180+
key, value, &device_params->allow_inline_execution));
181+
} else if (iree_string_view_equal(key, IREE_SV("hip_async_allocations"))) {
182+
IREE_RETURN_IF_ERROR(iree_hal_hip_try_parse_bool_option(
183+
key, value, &device_params->async_allocations));
184+
} else if (iree_string_view_equal(key, IREE_SV("hip_async_caching"))) {
185+
IREE_RETURN_IF_ERROR(iree_hal_hip_try_parse_bool_option(
186+
key, value, &device_params->async_caching));
187+
} else if (iree_string_view_equal(key, IREE_SV("hip_tracing"))) {
188+
IREE_RETURN_IF_ERROR(iree_hal_hip_try_parse_int32_option(
189+
key, value, &device_params->stream_tracing));
190+
} else if (iree_string_view_equal(key, IREE_SV("hip_default_index"))) {
191+
IREE_RETURN_IF_ERROR(iree_hal_hip_try_parse_int32_option(
192+
key, value, &driver_options->default_device_index));
213193
} else {
214194
return iree_make_status(IREE_STATUS_FAILED_PRECONDITION,
215-
"Unrecognized options: %.*s", (int)key.size,
195+
"unrecognized option: '%.*s'", (int)key.size,
216196
key.data);
217197
}
218198
}
219199

220200
// Populate dynamic sized values.
201+
//
202+
// TODO: make this a single key-value pair (semicolon separated).
203+
// Repeated fields don't work in things like python dictionaries/JSON and we
204+
// want to match the environment variable formatting.
221205
if (dylib_path_count > 0) {
222206
IREE_RETURN_IF_ERROR(iree_allocator_malloc(
223207
host_allocator,
@@ -226,7 +210,7 @@ static iree_status_t iree_hal_hip_driver_populate_options(
226210
for (iree_host_size_t i = 0; i < pairs_size; ++i) {
227211
iree_string_view_t key = pairs[i].key;
228212
iree_string_view_t value = pairs[i].value;
229-
if (iree_string_view_equal(key, key_hip_dylib_path)) {
213+
if (iree_string_view_equal(key, IREE_SV("hip_dylib_path"))) {
230214
driver_options->hip_lib_search_paths
231215
[driver_options->hip_lib_search_path_count++] = value;
232216
}

0 commit comments

Comments
 (0)