| Message ID | VI1PR0802MB2176E33A1C87830AC120CB86E7F90@VI1PR0802MB2176.eurprd08.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
On Tue, May 23, 2017 at 6:22 PM, Bin Cheng <Bin.Cheng@arm.com> wrote: > Hi, > This patch set factors out runtime alias check code from tree-vect-data-refs.c > and tree-vect-loop-manip.c as general interfaces in tree-data-ref.c. With this > change other optimizers like tree loop distribution could version loop wrto the > runtime alias checks. During this work, I also found current code has issues > with negative DR_STEP. This patch set fixes the issue as tracked in PR80815. > > This is the first patch simply moves compare_tree to tree.c and exposes it. > Bootstrap and test on x86_64 and AArch64, is it OK? I think the name is quite bad for an exported function given for INTEGER_CSTs it doesn't return anything resembling a comparison result. Also (not your fault) it doesn't seem to handle hash collisions nor have a suitable fallback for trees it doesn't handle. I don't have a good suggestion for the name but tree.c exported fns should have higher standards regarding their implementation... Richard. > Thanks, > bin > > 2017-05-22 Bin Cheng <bin.cheng@arm.com> > > * tree-vect-data-refs.c (compare_tree): Move ... > * tree.c (compare_tree): ... to here. > * tree.h (compare_tree): New decalaration.
On Fri, May 26, 2017 at 12:14 PM, Richard Biener <richard.guenther@gmail.com> wrote: > On Tue, May 23, 2017 at 6:22 PM, Bin Cheng <Bin.Cheng@arm.com> wrote: >> Hi, >> This patch set factors out runtime alias check code from tree-vect-data-refs.c >> and tree-vect-loop-manip.c as general interfaces in tree-data-ref.c. With this >> change other optimizers like tree loop distribution could version loop wrto the >> runtime alias checks. During this work, I also found current code has issues >> with negative DR_STEP. This patch set fixes the issue as tracked in PR80815. >> >> This is the first patch simply moves compare_tree to tree.c and exposes it. >> Bootstrap and test on x86_64 and AArch64, is it OK? > > I think the name is quite bad for an exported function given for INTEGER_CSTs > it doesn't return anything resembling a comparison result. Also (not > your fault) > it doesn't seem to handle hash collisions nor have a suitable fallback for > trees it doesn't handle. > > I don't have a good suggestion for the name but tree.c exported fns should > have higher standards regarding their implementation... Hmm, I don't have idea to generalize it for the moment, so OK to rename it to data_ref_compare_tree and move it to tree-data-ref.c? It needs to be external symbol though. Thanks, bin > > Richard. > > > >> Thanks, >> bin >> >> 2017-05-22 Bin Cheng <bin.cheng@arm.com> >> >> * tree-vect-data-refs.c (compare_tree): Move ... >> * tree.c (compare_tree): ... to here. >> * tree.h (compare_tree): New decalaration.
On Fri, May 26, 2017 at 1:30 PM, Bin.Cheng <amker.cheng@gmail.com> wrote: > On Fri, May 26, 2017 at 12:14 PM, Richard Biener > <richard.guenther@gmail.com> wrote: >> On Tue, May 23, 2017 at 6:22 PM, Bin Cheng <Bin.Cheng@arm.com> wrote: >>> Hi, >>> This patch set factors out runtime alias check code from tree-vect-data-refs.c >>> and tree-vect-loop-manip.c as general interfaces in tree-data-ref.c. With this >>> change other optimizers like tree loop distribution could version loop wrto the >>> runtime alias checks. During this work, I also found current code has issues >>> with negative DR_STEP. This patch set fixes the issue as tracked in PR80815. >>> >>> This is the first patch simply moves compare_tree to tree.c and exposes it. >>> Bootstrap and test on x86_64 and AArch64, is it OK? >> >> I think the name is quite bad for an exported function given for INTEGER_CSTs >> it doesn't return anything resembling a comparison result. Also (not >> your fault) >> it doesn't seem to handle hash collisions nor have a suitable fallback for >> trees it doesn't handle. >> >> I don't have a good suggestion for the name but tree.c exported fns should >> have higher standards regarding their implementation... > Hmm, I don't have idea to generalize it for the moment, so OK to > rename it to data_ref_compare_tree and move it to tree-data-ref.c? It > needs to be external symbol though. Works for me. Richard. > Thanks, > bin >> >> Richard. >> >> >> >>> Thanks, >>> bin >>> >>> 2017-05-22 Bin Cheng <bin.cheng@arm.com> >>> >>> * tree-vect-data-refs.c (compare_tree): Move ... >>> * tree.c (compare_tree): ... to here. >>> * tree.h (compare_tree): New decalaration.
diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c index 67cc969..7b835ae 100644 --- a/gcc/tree-vect-data-refs.c +++ b/gcc/tree-vect-data-refs.c @@ -2574,83 +2574,6 @@ vect_analyze_data_ref_access (struct data_reference *dr) return vect_analyze_group_access (dr); } - - -/* A helper function used in the comparator function to sort data - references. T1 and T2 are two data references to be compared. - The function returns -1, 0, or 1. */ - -static int -compare_tree (tree t1, tree t2) -{ - int i, cmp; - enum tree_code code; - char tclass; - - if (t1 == t2) - return 0; - if (t1 == NULL) - return -1; - if (t2 == NULL) - return 1; - - STRIP_NOPS (t1); - STRIP_NOPS (t2); - - if (TREE_CODE (t1) != TREE_CODE (t2)) - return TREE_CODE (t1) < TREE_CODE (t2) ? -1 : 1; - - code = TREE_CODE (t1); - switch (code) - { - /* For const values, we can just use hash values for comparisons. */ - case INTEGER_CST: - case REAL_CST: - case FIXED_CST: - case STRING_CST: - case COMPLEX_CST: - case VECTOR_CST: - { - hashval_t h1 = iterative_hash_expr (t1, 0); - hashval_t h2 = iterative_hash_expr (t2, 0); - if (h1 != h2) - return h1 < h2 ? -1 : 1; - break; - } - - case SSA_NAME: - cmp = compare_tree (SSA_NAME_VAR (t1), SSA_NAME_VAR (t2)); - if (cmp != 0) - return cmp; - - if (SSA_NAME_VERSION (t1) != SSA_NAME_VERSION (t2)) - return SSA_NAME_VERSION (t1) < SSA_NAME_VERSION (t2) ? -1 : 1; - break; - - default: - tclass = TREE_CODE_CLASS (code); - - /* For var-decl, we could compare their UIDs. */ - if (tclass == tcc_declaration) - { - if (DECL_UID (t1) != DECL_UID (t2)) - return DECL_UID (t1) < DECL_UID (t2) ? -1 : 1; - break; - } - - /* For expressions with operands, compare their operands recursively. */ - for (i = TREE_OPERAND_LENGTH (t1) - 1; i >= 0; --i) - { - cmp = compare_tree (TREE_OPERAND (t1, i), TREE_OPERAND (t2, i)); - if (cmp != 0) - return cmp; - } - } - - return 0; -} - - /* Compare two data-references DRA and DRB to group them into chunks suitable for grouping. */ diff --git a/gcc/tree.c b/gcc/tree.c index b76b521..c9b0615 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -7635,6 +7635,80 @@ compare_tree_int (const_tree t, unsigned HOST_WIDE_INT u) return 1; } +/* A helper function computes order between two tree epxressions T1 and T2. + This is used in comparator functions sorting objects based on the order + of tree expressions. The function returns -1, 0, or 1. */ + +int +compare_tree (tree t1, tree t2) +{ + int i, cmp; + enum tree_code code; + char tclass; + + if (t1 == t2) + return 0; + if (t1 == NULL) + return -1; + if (t2 == NULL) + return 1; + + STRIP_NOPS (t1); + STRIP_NOPS (t2); + + if (TREE_CODE (t1) != TREE_CODE (t2)) + return TREE_CODE (t1) < TREE_CODE (t2) ? -1 : 1; + + code = TREE_CODE (t1); + switch (code) + { + /* For const values, we can just use hash values for comparisons. */ + case INTEGER_CST: + case REAL_CST: + case FIXED_CST: + case STRING_CST: + case COMPLEX_CST: + case VECTOR_CST: + { + hashval_t h1 = iterative_hash_expr (t1, 0); + hashval_t h2 = iterative_hash_expr (t2, 0); + if (h1 != h2) + return h1 < h2 ? -1 : 1; + break; + } + + case SSA_NAME: + cmp = compare_tree (SSA_NAME_VAR (t1), SSA_NAME_VAR (t2)); + if (cmp != 0) + return cmp; + + if (SSA_NAME_VERSION (t1) != SSA_NAME_VERSION (t2)) + return SSA_NAME_VERSION (t1) < SSA_NAME_VERSION (t2) ? -1 : 1; + break; + + default: + tclass = TREE_CODE_CLASS (code); + + /* For var-decl, we could compare their UIDs. */ + if (tclass == tcc_declaration) + { + if (DECL_UID (t1) != DECL_UID (t2)) + return DECL_UID (t1) < DECL_UID (t2) ? -1 : 1; + break; + } + + /* For expressions with operands, compare their operands recursively. */ + for (i = TREE_OPERAND_LENGTH (t1) - 1; i >= 0; --i) + { + cmp = compare_tree (TREE_OPERAND (t1, i), TREE_OPERAND (t2, i)); + if (cmp != 0) + return cmp; + } + } + + return 0; +} + /* Return true if SIZE represents a constant size that is in bounds of what the middle-end and the backend accepts (covering not more than half of the address-space). */ diff --git a/gcc/tree.h b/gcc/tree.h index c6e883c..732ae09 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -4819,6 +4819,7 @@ static inline hashval_t iterative_hash_expr(const_tree tree, hashval_t seed) } extern int compare_tree_int (const_tree, unsigned HOST_WIDE_INT); +extern int compare_tree (tree, tree); extern int type_list_equal (const_tree, const_tree); extern int chain_member (const_tree, const_tree); extern void dump_tree_statistics (void);
Hi, This patch set factors out runtime alias check code from tree-vect-data-refs.c and tree-vect-loop-manip.c as general interfaces in tree-data-ref.c. With this change other optimizers like tree loop distribution could version loop wrto the runtime alias checks. During this work, I also found current code has issues with negative DR_STEP. This patch set fixes the issue as tracked in PR80815. This is the first patch simply moves compare_tree to tree.c and exposes it. Bootstrap and test on x86_64 and AArch64, is it OK? Thanks, bin 2017-05-22 Bin Cheng <bin.cheng@arm.com> * tree-vect-data-refs.c (compare_tree): Move ... * tree.c (compare_tree): ... to here. * tree.h (compare_tree): New decalaration. From 2b993c1aaada767cbbda4e73010d48b0b8347ef3 Mon Sep 17 00:00:00 2001 From: Bin Cheng <binche01@e108451-lin.cambridge.arm.com> Date: Fri, 19 May 2017 12:49:38 +0100 Subject: [PATCH 1/6] compare_tree-interface-20170515.txt --- gcc/tree-vect-data-refs.c | 77 ----------------------------------------------- gcc/tree.c | 74 +++++++++++++++++++++++++++++++++++++++++++++ gcc/tree.h | 1 + 3 files changed, 75 insertions(+), 77 deletions(-)