[ was: Re: [RFC] ldist: Recognize rawmemchr loop patterns ]
On 1/31/22 16:00, Richard Biener wrote:
I'm running into PR56888 ( https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56888 ) on nvptx due to this, f.i. in gcc/testsuite/gcc.c-torture/execute/builtins/strlen.c, where gcc/testsuite/gcc.c-torture/execute/builtins/lib/strlen.c contains a strlen function, with a strlen loop, which is transformed by pass_loop_distribution into a __builtin_strlen, which is then expanded into a strlen call, creating a self-recursive function. [ And on nvptx, that happens to result in a compilation failure, which is how I found this. ] According to this ( https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56888#c21 ) comment: ... -fno-tree-loop-distribute-patterns is the reliable way to not transform loops into library calls. ... Then should we have something along the lines of: ... $ git diff diff --git a/gcc/tree-loop-distribution.c b/gcc/tree-loop-distribution.c index 6fe59cd56855..9a211d30cd7e 100644 --- a/gcc/tree-loop-distribution.c +++ b/gcc/tree-loop-distribution.c @@ -3683,7 +3683,11 @@ loop_distribution::transform_reduction_loop && TYPE_PRECISION (ptr_type_node) >= 32) || (TYPE_OVERFLOW_UNDEFINED (reduction_var_type) && TYPE_PRECISION (reduction_var_type) <= TYPE_PRECISION (sizetype))) - && builtin_decl_implicit (BUILT_IN_STRLEN)) + && builtin_decl_implicit (BUILT_IN_STRLEN) + && flag_tree_loop_distribute_patterns) generate_strlen_builtin (loop, reduction_var, load_iv.base, reduction_iv.base, loc); else if (direct_optab_handler (rawmemchr_optab, TYPE_MODE (load_type)) ... ? Or is the comment no longer valid?It is still valid - and yes, I think we need to guard it with this flag but please do it in the caller to transform_reduction_loop.
Done. Ok for trunk? Thanks, - Tom
[ldist] Don't add lib calls with -fno-tree-loop-distribute-patterns As mentioned in PR56888 comment 21: ... -fno-tree-loop-distribute-patterns is the reliable way to not transform loops into library calls. ... However, since commit 6f966f06146 ("ldist: Recognize strlen and rawmemchr like loops") a strlen or rawmemchr library call may be introduced by ldist. This caused regressions in testcases gcc.c-torture/execute/builtins/strlen{,-2,-3}.c for nvptx. Fix this by not calling transform_reduction_loop from loop_distribution::execute for -fno-tree-loop-distribute-patterns. Tested regressing test-cases as well as gcc.dg/tree-ssa/ldist-*.c on nvptx. gcc/ChangeLog: 2022-01-31 Tom de Vries <tdevr...@suse.de> * tree-loop-distribution.cc (generate_reduction_builtin_1): Check for -ftree-loop-distribute-patterns. (loop_distribution::execute): Don't call transform_reduction_loop for -fno-tree-loop-distribute-patterns. gcc/testsuite/ChangeLog: 2022-01-31 Tom de Vries <tdevr...@suse.de> * gcc.dg/tree-ssa/ldist-strlen-4.c: New test. --- gcc/testsuite/gcc.dg/tree-ssa/ldist-strlen-4.c | 17 +++++++++++++++++ gcc/tree-loop-distribution.cc | 5 ++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ldist-strlen-4.c b/gcc/testsuite/gcc.dg/tree-ssa/ldist-strlen-4.c new file mode 100644 index 000000000000..eafb37e84fc3 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/ldist-strlen-4.c @@ -0,0 +1,17 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -ftree-loop-distribution -fno-tree-loop-distribute-patterns -fdump-tree-ldist-details" } */ +/* { dg-final { scan-tree-dump-not "generated strlen" "ldist" } } */ + +/* Copied from gcc/testsuite/gcc.c-torture/execute/builtins/lib/strlen.c. */ + +__SIZE_TYPE__ +foo (const char *s) +{ + __SIZE_TYPE__ i; + + i = 0; + while (s[i] != 0) + i++; + + return i; +} diff --git a/gcc/tree-loop-distribution.cc b/gcc/tree-loop-distribution.cc index 6fe59cd56855..c7b428572636 100644 --- a/gcc/tree-loop-distribution.cc +++ b/gcc/tree-loop-distribution.cc @@ -3290,6 +3290,8 @@ generate_reduction_builtin_1 (loop_p loop, gimple_seq &seq, tree reduction_var_old, tree reduction_var_new, const char *info, machine_mode load_mode) { + gcc_assert (flag_tree_loop_distribute_patterns); + /* Place new statements before LOOP. */ gimple_stmt_iterator gsi = gsi_last_bb (loop_preheader_edge (loop)->src); gsi_insert_seq_after (&gsi, seq, GSI_CONTINUE_LINKING); @@ -3773,7 +3775,8 @@ loop_distribution::execute (function *fun) if (niters == NULL_TREE || niters == chrec_dont_know) { datarefs_vec.create (20); - if (transform_reduction_loop (loop)) + if (flag_tree_loop_distribute_patterns + && transform_reduction_loop (loop)) { changed = true; loops_to_be_destroyed.safe_push (loop);