On Wed, 31 Jan 2018, Jakub Jelinek wrote:
> Hi!
>
> Some spots in the vectorizer create generic COND_EXPRs that in one of the
> branches compute some +/-/* arithmetics. When -ftrapv, the gimplifier
> ICEs on this as it may trap, we can't emit code with multiple basic blocks
> from the APIs and don't want to evaluate it if the guarding condition is
> false.
>
> This patch resolves it by making that arithmetic untrappable (i.e. in
> unsigned types).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Wow, yes - we _do_ have absv patterns. Looks like
rewrite_to_defined_overflow / arith_code_with_undefined_signed_overflow
miss handling of ABS_EXPR. This again reminds me of adding
ABSU_EXPR which would also work nicer for fixing the trapping case.
Anyway, we probably want to postpone this "cleanup" to GCC 9.
I wonder if you actually ran into the case of ABS_EXPR being
passed to rewrite_to_non_trapping_overflow?
Ok for trunk.
Thanks,
Richard.
> 2018-01-31 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/81661
> PR tree-optimization/84117
> * tree-eh.h (rewrite_to_non_trapping_overflow): Declare.
> * tree-eh.c: Include gimplify.h.
> (find_trapping_overflow, replace_trapping_overflow,
> rewrite_to_non_trapping_overflow): New functions.
> * tree-vect-loop.c: Include tree-eh.h.
> (vect_get_loop_niters): Use rewrite_to_non_trapping_overflow.
> * tree-data-ref.c: Include tree-eh.h.
> (get_segment_min_max): Use rewrite_to_non_trapping_overflow.
>
> * gcc.dg/pr81661.c: New test.
> * gfortran.dg/pr84117.f90: New test.
>
> --- gcc/tree-eh.h.jj 2018-01-03 10:19:54.561533862 +0100
> +++ gcc/tree-eh.h 2018-01-31 11:59:36.328182707 +0100
> @@ -37,6 +37,7 @@ extern bool operation_could_trap_helper_
> bool, tree, bool *);
> extern bool operation_could_trap_p (enum tree_code, bool, bool, tree);
> extern bool tree_could_trap_p (tree);
> +extern tree rewrite_to_non_trapping_overflow (tree);
> extern bool stmt_could_throw_p (gimple *);
> extern bool tree_could_throw_p (tree);
> extern bool stmt_can_throw_external (gimple *);
> --- gcc/tree-eh.c.jj 2018-01-03 10:19:56.103534109 +0100
> +++ gcc/tree-eh.c 2018-01-31 16:51:00.726119778 +0100
> @@ -46,6 +46,7 @@ along with GCC; see the file COPYING3.
> #include "stringpool.h"
> #include "attribs.h"
> #include "asan.h"
> +#include "gimplify.h"
>
> /* In some instances a tree and a gimple need to be stored in a same table,
> i.e. in hash tables. This is a structure to do this. */
> @@ -2720,6 +2721,91 @@ tree_could_trap_p (tree expr)
> }
> }
>
> +/* Return non-NULL if there is an integer operation with trapping overflow
> + we can rewrite into non-trapping. Called via walk_tree from
> + rewrite_to_non_trapping_overflow. */
> +
> +static tree
> +find_trapping_overflow (tree *tp, int *walk_subtrees, void *data)
> +{
> + if (EXPR_P (*tp)
> + && !operation_no_trapping_overflow (TREE_TYPE (*tp), TREE_CODE (*tp)))
> + return *tp;
> + if (IS_TYPE_OR_DECL_P (*tp)
> + || (TREE_CODE (*tp) == SAVE_EXPR && data == NULL))
> + *walk_subtrees = 0;
> + return NULL_TREE;
> +}
> +
> +/* Rewrite selected operations into unsigned arithmetics, so that they
> + don't trap on overflow. */
> +
> +static tree
> +replace_trapping_overflow (tree *tp, int *walk_subtrees, void *data)
> +{
> + if (find_trapping_overflow (tp, walk_subtrees, data))
> + {
> + tree type = TREE_TYPE (*tp);
> + tree utype = unsigned_type_for (type);
> + *walk_subtrees = 0;
> + int len = TREE_OPERAND_LENGTH (*tp);
> + for (int i = 0; i < len; ++i)
> + walk_tree (&TREE_OPERAND (*tp, i), replace_trapping_overflow,
> + data, (hash_set<tree> *) data);
> +
> + if (TREE_CODE (*tp) == ABS_EXPR)
> + {
> + tree op = TREE_OPERAND (*tp, 0);
> + op = save_expr (op);
> + /* save_expr skips simple arithmetics, which is undesirable
> + here, if it might trap due to flag_trapv. We need to
> + force a SAVE_EXPR in the COND_EXPR condition, to evaluate
> + it before the comparison. */
> + if (EXPR_P (op)
> + && TREE_CODE (op) != SAVE_EXPR
> + && walk_tree (&op, find_trapping_overflow, NULL, NULL))
> + {
> + op = build1_loc (EXPR_LOCATION (op), SAVE_EXPR, type, op);
> + TREE_SIDE_EFFECTS (op) = 1;
> + }
> + /* Change abs (op) to op < 0 ? -op : op and handle the NEGATE_EXPR
> + like other signed integer trapping operations. */
> + tree cond = fold_build2 (LT_EXPR, boolean_type_node,
> + op, build_int_cst (type, 0));
> + tree neg = fold_build1 (NEGATE_EXPR, utype,
> + fold_convert (utype, op));
> + *tp = fold_build3 (COND_EXPR, type, cond,
> + fold_convert (type, neg), op);
> + }
> + else
> + {
> + TREE_TYPE (*tp) = utype;
> + len = TREE_OPERAND_LENGTH (*tp);
> + for (int i = 0; i < len; ++i)
> + TREE_OPERAND (*tp, i)
> + = fold_convert (utype, TREE_OPERAND (*tp, i));
> + *tp = fold_convert (type, *tp);
> + }
> + }
> + return NULL_TREE;
> +}
> +
> +/* If any subexpression of EXPR can trap due to -ftrapv, rewrite it
> + using unsigned arithmetics to avoid traps in it. */
> +
> +tree
> +rewrite_to_non_trapping_overflow (tree expr)
> +{
> + if (!flag_trapv)
> + return expr;
> + hash_set<tree> pset;
> + if (!walk_tree (&expr, find_trapping_overflow, &pset, &pset))
> + return expr;
> + expr = unshare_expr (expr);
> + pset.empty ();
> + walk_tree (&expr, replace_trapping_overflow, &pset, &pset);
> + return expr;
> +}
>
> /* Helper for stmt_could_throw_p. Return true if STMT (assumed to be a
> an assignment or a conditional) may throw. */
> --- gcc/tree-vect-loop.c.jj 2018-01-19 19:41:53.624549770 +0100
> +++ gcc/tree-vect-loop.c 2018-01-31 12:02:02.642190868 +0100
> @@ -53,6 +53,7 @@ along with GCC; see the file COPYING3.
> #include "internal-fn.h"
> #include "tree-vector-builder.h"
> #include "vec-perm-indices.h"
> +#include "tree-eh.h"
>
> /* Loop Vectorization Pass.
>
> @@ -1063,7 +1064,8 @@ vect_get_loop_niters (struct loop *loop,
> may_be_zero));
> else
> niter = fold_build3 (COND_EXPR, TREE_TYPE (niter), may_be_zero,
> - build_int_cst (TREE_TYPE (niter), 0), niter);
> + build_int_cst (TREE_TYPE (niter), 0),
> + rewrite_to_non_trapping_overflow (niter));
>
> may_be_zero = NULL_TREE;
> }
> --- gcc/tree-data-ref.c.jj 2018-01-14 17:16:55.614836141 +0100
> +++ gcc/tree-data-ref.c 2018-01-31 12:01:04.836187642 +0100
> @@ -98,6 +98,7 @@ along with GCC; see the file COPYING3.
> #include "stringpool.h"
> #include "tree-vrp.h"
> #include "tree-ssanames.h"
> +#include "tree-eh.h"
>
> static struct datadep_stats
> {
> @@ -1790,7 +1791,8 @@ get_segment_min_max (const dr_with_seg_l
> tree addr_base = fold_build_pointer_plus (DR_BASE_ADDRESS (d.dr),
> DR_OFFSET (d.dr));
> addr_base = fold_build_pointer_plus (addr_base, DR_INIT (d.dr));
> - tree seg_len = fold_convert (sizetype, d.seg_len);
> + tree seg_len
> + = fold_convert (sizetype, rewrite_to_non_trapping_overflow (d.seg_len));
>
> tree min_reach = fold_build3 (COND_EXPR, sizetype, neg_step,
> seg_len, size_zero_node);
> --- gcc/testsuite/gcc.dg/pr81661.c.jj 2018-01-31 12:08:20.457211930 +0100
> +++ gcc/testsuite/gcc.dg/pr81661.c 2018-01-31 12:08:03.723210999 +0100
> @@ -0,0 +1,12 @@
> +/* PR tree-optimization/81661 */
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -ftrapv" } */
> +
> +int a, b, c;
> +
> +void
> +foo (void)
> +{
> + while (a + c > b)
> + a--;
> +}
> --- gcc/testsuite/gfortran.dg/pr84117.f90.jj 2018-01-31 12:09:11.305214765
> +0100
> +++ gcc/testsuite/gfortran.dg/pr84117.f90 2018-01-31 12:09:31.827215918
> +0100
> @@ -0,0 +1,7 @@
> +! PR tree-optimization/84117
> +! { dg-do compile }
> +! { dg-options "-O3 -ftrapv" }
> + FUNCTION pw_integral_aa ( cc ) RESULT ( integral_value )
> + COMPLEX(KIND=8), DIMENSION(:), POINTER :: cc
> + integral_value = accurate_sum ( CONJG ( cc (:) ) * cc (:) )
> + END FUNCTION pw_integral_aa
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
