From 1a470034b195f400dd1711b145cdd7018de6d95d Mon Sep 17 00:00:00 2001 From: Tobias Grosser Date: Mon, 24 Jul 2017 20:49:12 +0200 Subject: [PATCH] Handle error conditions returned by level_before in isl_flow Check the return value of level_before and intermediate_sources and pass on the error the functions potentially return. The function isl_access_info_sort_sources (and referenced functions) is also adapted to propagate error conditions. When isl-0.14-306-g5e596b8c2f started to use the isl schedule tree to detect the order of accesses, it introduced a level_before function that may also return -1, but failed to introduce code for handling such error conditions in the callers. This resulted in unspecified behavior and crashes, in cases where level_before returned a failure, e.g. in case of a compute out. Even though this commit would technically allow users to also return -1 as error value, it does not yet update the user level documentation. It is a pure bugfix. Expanding the user level interface should be a separate discussion and commit. This commit comes without a test case. It is difficult to provide stable test cases, if the error condition that triggers the problem depends on very specific values of the isl compute out. Signed-off-by: Tobias Grosser Signed-off-by: Sven Verdoolaege --- isl_flow.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 63 insertions(+), 6 deletions(-) diff --git a/isl_flow.c b/isl_flow.c index 6e2a9a3a..08d8651e 100644 --- a/isl_flow.c +++ b/isl_flow.c @@ -304,6 +304,13 @@ error: return NULL; } +/* A helper struct carrying the isl_access_info and an error condition. + */ +struct access_sort_info { + isl_access_info *access_info; + int error; +}; + /* Return -n, 0 or n (with n a positive value), depending on whether * the source access identified by p1 should be sorted before, together * or after that identified by p2. @@ -316,10 +323,18 @@ error: * If not, we try to order the two statements based on the description * of the iteration domains. This results in an arbitrary, but fairly * stable ordering. + * + * In case of an error, sort_info.error is set to true and all elements are + * reported to be equal. */ static int access_sort_cmp(const void *p1, const void *p2, void *user) { - isl_access_info *acc = user; + struct access_sort_info *sort_info = user; + isl_access_info *acc = sort_info->access_info; + + if (sort_info->error) + return 0; + const struct isl_labeled_map *i1, *i2; int level1, level2; uint32_t h1, h2; @@ -327,16 +342,24 @@ static int access_sort_cmp(const void *p1, const void *p2, void *user) i2 = (const struct isl_labeled_map *) p2; level1 = acc->level_before(i1->data, i2->data); + if (level1 < 0) + goto error; if (level1 % 2) return -1; level2 = acc->level_before(i2->data, i1->data); + if (level2 < 0) + goto error; if (level2 % 2) return 1; h1 = isl_map_get_hash(i1->map); h2 = isl_map_get_hash(i2->map); return h1 > h2 ? 1 : h1 < h2 ? -1 : 0; +error: + sort_info->error = 1; + return 0; + } /* Sort the must source accesses in their textual order. @@ -344,13 +367,20 @@ static int access_sort_cmp(const void *p1, const void *p2, void *user) static __isl_give isl_access_info *isl_access_info_sort_sources( __isl_take isl_access_info *acc) { + struct access_sort_info sort_info; + + sort_info.access_info = acc; + sort_info.error = 0; + if (!acc) return NULL; if (acc->n_must <= 1) return acc; if (isl_sort(acc->source, acc->n_must, sizeof(struct isl_labeled_map), - access_sort_cmp, acc) < 0) + access_sort_cmp, &sort_info) < 0) + return isl_access_info_free(acc); + if (sort_info.error) return isl_access_info_free(acc); return acc; @@ -674,6 +704,9 @@ static int can_precede_at_level(int shared_level, int target_level) * * If temp_rel[j] is empty, then there can be no improvement and * we return immediately. + * + * This function returns 0 in case it was executed successfully and + * -1 in case of errors during the execution of this function. */ static int intermediate_sources(__isl_keep isl_access_info *acc, struct isl_map **temp_rel, int j, int sink_level) @@ -687,11 +720,15 @@ static int intermediate_sources(__isl_keep isl_access_info *acc, for (k = j - 1; k >= 0; --k) { int plevel, plevel2; plevel = acc->level_before(acc->source[k].data, acc->sink.data); + if (plevel < 0) + return -1; if (!can_precede_at_level(plevel, sink_level)) continue; plevel2 = acc->level_before(acc->source[j].data, acc->source[k].data); + if (plevel2 < 0) + return -1; for (level = sink_level; level <= depth; ++level) { struct isl_map *T; @@ -800,6 +837,8 @@ static __isl_give isl_map *all_intermediate_sources( plevel = acc->level_before(acc->source[k].data, acc->source[acc->n_must + j].data); + if (plevel < 0) + return isl_map_free(map); for (level = sink_level; level <= depth; ++level) { isl_map *T; @@ -859,6 +898,9 @@ static __isl_give isl_flow *compute_mem_based_dependences( isl_map *dep; plevel = acc->level_before(acc->source[i].data, acc->sink.data); + if (plevel < 0) + goto error; + is_before = plevel & 1; plevel >>= 1; @@ -879,6 +921,11 @@ static __isl_give isl_flow *compute_mem_based_dependences( res->must_no_source = mustdo; return res; +error: + isl_set_free(mustdo); + isl_set_free(maydo); + isl_flow_free(res); + return NULL; } /* Compute dependences for the case where there is at least one @@ -964,6 +1011,8 @@ static __isl_give isl_flow *compute_val_based_dependences( plevel = acc->level_before(acc->source[j].data, acc->sink.data); + if (plevel < 0) + goto error; if (!can_precede_at_level(plevel, level)) continue; @@ -971,13 +1020,15 @@ static __isl_give isl_flow *compute_val_based_dependences( must_rel[j] = isl_map_union_disjoint(must_rel[j], T); mustdo = rest; - intermediate_sources(acc, must_rel, j, level); + if (intermediate_sources(acc, must_rel, j, level)) + goto error; T = last_source(acc, maydo, j, level, &rest); may_rel[j] = isl_map_union_disjoint(may_rel[j], T); maydo = rest; - intermediate_sources(acc, may_rel, j, level); + if (intermediate_sources(acc, may_rel, j, level)) + goto error; if (isl_set_plain_is_empty(mustdo) && isl_set_plain_is_empty(maydo)) @@ -988,11 +1039,15 @@ static __isl_give isl_flow *compute_val_based_dependences( plevel = acc->level_before(acc->source[j].data, acc->sink.data); + if (plevel < 0) + goto error; if (!can_precede_at_level(plevel, level)) continue; - intermediate_sources(acc, must_rel, j, level); - intermediate_sources(acc, may_rel, j, level); + if (intermediate_sources(acc, must_rel, j, level)) + goto error; + if (intermediate_sources(acc, may_rel, j, level)) + goto error; } for (j = 0; j < acc->n_may; ++j) { @@ -1002,6 +1057,8 @@ static __isl_give isl_flow *compute_val_based_dependences( plevel = acc->level_before(acc->source[acc->n_must + j].data, acc->sink.data); + if (plevel < 0) + goto error; if (!can_precede_at_level(plevel, level)) continue; -- 2.11.4.GIT