From: Jakub Jelinek Date: Fri, 11 Oct 2019 07:36:07 +0000 (+0200) Subject: re PR c++/91987 (-fstrict-eval-order issues) X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=1a37b6d9a7e57c71b1bfe449ebd275eb408117fb;p=gcc.git re PR c++/91987 (-fstrict-eval-order issues) PR c++/91987 cp/ * decl2.c (grok_array_decl): For -fstrong-eval-order, when array ref operands have been swapped and at least one operand has side-effects, revert the swapping before calling build_array_ref. * typeck.c (cp_build_array_ref): For non-ARRAY_TYPE array ref with side-effects on the index operand, if -fstrong-eval-order use save_expr around the array operand. (cp_build_binary_op): For shifts with side-effects in the second operand, wrap first operand into SAVE_EXPR and evaluate it before the shift. * semantics.c (handle_omp_array_sections_1): Temporarily disable flag_strong_eval_order during OMP_CLAUSE_REDUCTION array section processing. * cp-gimplify.c (gimplify_to_rvalue): New function. (cp_gimplify_expr): Use it. testsuite/ * g++.dg/cpp1z/eval-order6.C: New test. * g++.dg/cpp1z/eval-order7.C: New test. * g++.dg/cpp1z/eval-order8.C: New test. * c-c++-common/gomp/pr91987.c: New test. From-SVN: r276860 --- diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index 2e3b7f7dffd..95f683103e0 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,21 @@ +2019-10-11 Jakub Jelinek + + PR c++/91987 + * decl2.c (grok_array_decl): For -fstrong-eval-order, when array ref + operands have been swapped and at least one operand has side-effects, + revert the swapping before calling build_array_ref. + * typeck.c (cp_build_array_ref): For non-ARRAY_TYPE array ref with + side-effects on the index operand, if -fstrong-eval-order use + save_expr around the array operand. + (cp_build_binary_op): For shifts with side-effects in the second + operand, wrap first operand into SAVE_EXPR and evaluate it before + the shift. + * semantics.c (handle_omp_array_sections_1): Temporarily disable + flag_strong_eval_order during OMP_CLAUSE_REDUCTION array section + processing. + * cp-gimplify.c (gimplify_to_rvalue): New function. + (cp_gimplify_expr): Use it. + 2019-10-10 Marek Polacek * typeck.c (comp_ptr_ttypes_real): Change the return type to bool. diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c index 65453929d31..154fa70ec06 100644 --- a/gcc/cp/cp-gimplify.c +++ b/gcc/cp/cp-gimplify.c @@ -638,6 +638,22 @@ lvalue_has_side_effects (tree e) return TREE_SIDE_EFFECTS (e); } +/* Gimplify *EXPR_P as rvalue into an expression that can't be modified + by expressions with side-effects in other operands. */ + +static enum gimplify_status +gimplify_to_rvalue (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, + bool (*gimple_test_f) (tree)) +{ + enum gimplify_status t + = gimplify_expr (expr_p, pre_p, post_p, gimple_test_f, fb_rvalue); + if (t == GS_ERROR) + return GS_ERROR; + else if (is_gimple_variable (*expr_p) && TREE_CODE (*expr_p) != SSA_NAME) + *expr_p = get_initialized_tmp_var (*expr_p, pre_p, NULL); + return t; +} + /* Do C++-specific gimplification. Args are as for gimplify_expr. */ int @@ -823,15 +839,10 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p) && cp_get_callee_fndecl_nofold (*expr_p) == NULL_TREE) { enum gimplify_status t - = gimplify_expr (&CALL_EXPR_FN (*expr_p), pre_p, NULL, - is_gimple_call_addr, fb_rvalue); + = gimplify_to_rvalue (&CALL_EXPR_FN (*expr_p), pre_p, NULL, + is_gimple_call_addr); if (t == GS_ERROR) ret = GS_ERROR; - else if (is_gimple_variable (CALL_EXPR_FN (*expr_p)) - && TREE_CODE (CALL_EXPR_FN (*expr_p)) != SSA_NAME) - CALL_EXPR_FN (*expr_p) - = get_initialized_tmp_var (CALL_EXPR_FN (*expr_p), pre_p, - NULL); } if (!CALL_EXPR_FN (*expr_p)) /* Internal function call. */; diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c index a28e7762a34..ee198cdf5ce 100644 --- a/gcc/cp/decl2.c +++ b/gcc/cp/decl2.c @@ -405,6 +405,7 @@ grok_array_decl (location_t loc, tree array_expr, tree index_exp, else { tree p1, p2, i1, i2; + bool swapped = false; /* Otherwise, create an ARRAY_REF for a pointer or array type. It is a little-known fact that, if `a' is an array and `i' is @@ -431,7 +432,7 @@ grok_array_decl (location_t loc, tree array_expr, tree index_exp, if (p1 && i2) array_expr = p1, index_exp = i2; else if (i1 && p2) - array_expr = p2, index_exp = i1; + swapped = true, array_expr = p2, index_exp = i1; else { error_at (loc, "invalid types %<%T[%T]%> for array subscript", @@ -447,7 +448,12 @@ grok_array_decl (location_t loc, tree array_expr, tree index_exp, else array_expr = mark_lvalue_use_nonread (array_expr); index_exp = mark_rvalue_use (index_exp); - expr = build_array_ref (input_location, array_expr, index_exp); + if (swapped + && flag_strong_eval_order == 2 + && (TREE_SIDE_EFFECTS (array_expr) || TREE_SIDE_EFFECTS (index_exp))) + expr = build_array_ref (input_location, index_exp, array_expr); + else + expr = build_array_ref (input_location, array_expr, index_exp); } if (processing_template_decl && expr != error_mark_node) { diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index 1839013fbbc..f93bb934850 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -5060,6 +5060,15 @@ handle_omp_array_sections_1 (tree c, tree t, vec &types, TREE_PURPOSE (t) = lb; low_bound = lb; } + /* Temporarily disable -fstrong-eval-order for array reductions. + The SAVE_EXPR and COMPOUND_EXPR added if low_bound has side-effects + is something the middle-end can't cope with and more importantly, + it needs to be the actual base variable that is privatized, not some + temporary assigned previous value of it. That, together with OpenMP + saying how many times the side-effects are evaluated is unspecified, + makes int *a, *b; ... reduction(+:a[a = b, 3:10]) really unspecified. */ + warning_sentinel s (flag_strong_eval_order, + OMP_CLAUSE_CODE (c) == OMP_CLAUSE_REDUCTION); ret = grok_array_decl (OMP_CLAUSE_LOCATION (c), ret, low_bound, false); return ret; } diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index 38664ecd177..74ff8857fab 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -3560,6 +3560,10 @@ cp_build_array_ref (location_t loc, tree array, tree idx, { tree ar = cp_default_conversion (array, complain); tree ind = cp_default_conversion (idx, complain); + tree first = NULL_TREE; + + if (flag_strong_eval_order == 2 && TREE_SIDE_EFFECTS (ind)) + ar = first = save_expr (ar); /* Put the integer in IND to simplify error checking. */ if (TREE_CODE (TREE_TYPE (ar)) == INTEGER_TYPE) @@ -3583,11 +3587,10 @@ cp_build_array_ref (location_t loc, tree array, tree idx, warn_array_subscript_with_type_char (loc, idx); - ret = cp_build_indirect_ref (cp_build_binary_op (input_location, - PLUS_EXPR, ar, ind, - complain), - RO_ARRAY_INDEXING, - complain); + ret = cp_build_binary_op (input_location, PLUS_EXPR, ar, ind, complain); + if (first) + ret = build2_loc (loc, COMPOUND_EXPR, TREE_TYPE (ret), first, ret); + ret = cp_build_indirect_ref (ret, RO_ARRAY_INDEXING, complain); protected_set_expr_location (ret, loc); if (non_lvalue) ret = non_lvalue_loc (loc, ret); @@ -5574,6 +5577,17 @@ cp_build_binary_op (const op_location_t &location, if (build_type == NULL_TREE) build_type = result_type; + if (doing_shift + && flag_strong_eval_order == 2 + && TREE_SIDE_EFFECTS (op1) + && !processing_template_decl) + { + /* In C++17, in both op0 << op1 and op0 >> op1 op0 is sequenced before + op1, so if op1 has side-effects, use SAVE_EXPR around op0. */ + op0 = cp_save_expr (op0); + instrument_expr = op0; + } + if (sanitize_flags_p ((SANITIZE_SHIFT | SANITIZE_DIVIDE | SANITIZE_FLOAT_DIVIDE)) && current_function_decl != NULL_TREE @@ -5585,6 +5599,7 @@ cp_build_binary_op (const op_location_t &location, op1 = cp_save_expr (op1); op0 = fold_non_dependent_expr (op0, complain); op1 = fold_non_dependent_expr (op1, complain); + tree instrument_expr1 = NULL_TREE; if (doing_div_or_mod && sanitize_flags_p (SANITIZE_DIVIDE | SANITIZE_FLOAT_DIVIDE)) { @@ -5597,10 +5612,15 @@ cp_build_binary_op (const op_location_t &location, cop0 = cp_convert (orig_type, op0, complain); if (TREE_TYPE (cop1) != orig_type) cop1 = cp_convert (orig_type, op1, complain); - instrument_expr = ubsan_instrument_division (location, cop0, cop1); + instrument_expr1 = ubsan_instrument_division (location, cop0, cop1); } else if (doing_shift && sanitize_flags_p (SANITIZE_SHIFT)) - instrument_expr = ubsan_instrument_shift (location, code, op0, op1); + instrument_expr1 = ubsan_instrument_shift (location, code, op0, op1); + if (instrument_expr != NULL) + instrument_expr = add_stmt_to_compound (instrument_expr, + instrument_expr1); + else + instrument_expr = instrument_expr1; } result = build2_loc (location, resultcode, build_type, op0, op1); diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 76e634e1761..0f55eecfff4 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,11 @@ +2019-10-11 Jakub Jelinek + + PR c++/91987 + * g++.dg/cpp1z/eval-order6.C: New test. + * g++.dg/cpp1z/eval-order7.C: New test. + * g++.dg/cpp1z/eval-order8.C: New test. + * c-c++-common/gomp/pr91987.c: New test. + 2019-10-10 Joseph Myers * gcc.dg/c11-float-dfp-1.c, gcc.dg/c2x-float-no-dfp-1.c, diff --git a/gcc/testsuite/c-c++-common/gomp/pr91987.c b/gcc/testsuite/c-c++-common/gomp/pr91987.c new file mode 100644 index 00000000000..516bb8091fd --- /dev/null +++ b/gcc/testsuite/c-c++-common/gomp/pr91987.c @@ -0,0 +1,26 @@ +/* PR c++/91987 */ + +int bar (void); +void baz (int *); +#pragma omp declare target to (baz) + +void +foo (int *a, int (*b)[10][10]) +{ + #pragma omp target map(a[bar ()]) + baz (a); + #pragma omp target map(a[bar ():1]) + baz (a); + #pragma omp target map(a[10:bar ()]) + baz (a); + #pragma omp task depend(inout:a[10:bar ()]) + baz (a); + #pragma omp task depend(inout:a[10:bar ()]) + baz (a); + #pragma omp parallel reduction(+:a[bar ():2]) + baz (a); + #pragma omp parallel reduction(+:a[2:bar ()]) + baz (a); + #pragma omp parallel reduction(+:b[bar ():2][bar ():10][bar ():10]) + baz (a); +} diff --git a/gcc/testsuite/g++.dg/cpp1z/eval-order6.C b/gcc/testsuite/g++.dg/cpp1z/eval-order6.C new file mode 100644 index 00000000000..c8fb0b6dc9c --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1z/eval-order6.C @@ -0,0 +1,20 @@ +// PR c++/91987 +// { dg-do run } +// { dg-options "-fstrong-eval-order" } + +int +foo () +{ + int x = 5; + int r = x << (x = 3, 2); + if (x != 3) + __builtin_abort (); + return r; +} + +int +main () +{ + if (foo () != (5 << 2)) + __builtin_abort (); +} diff --git a/gcc/testsuite/g++.dg/cpp1z/eval-order7.C b/gcc/testsuite/g++.dg/cpp1z/eval-order7.C new file mode 100644 index 00000000000..c6f3cc25027 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1z/eval-order7.C @@ -0,0 +1,23 @@ +// PR c++/91987 +// { dg-do run } +// { dg-options "-fstrong-eval-order" } + +int a[4] = { 1, 2, 3, 4 }; +int b[4] = { 5, 6, 7, 8 }; + +int +foo () +{ + int *x = a; + int r = x[(x = b, 3)]; + if (x != b) + __builtin_abort (); + return r; +} + +int +main () +{ + if (foo () != 4) + __builtin_abort (); +} diff --git a/gcc/testsuite/g++.dg/cpp1z/eval-order8.C b/gcc/testsuite/g++.dg/cpp1z/eval-order8.C new file mode 100644 index 00000000000..a8a4240f1b3 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1z/eval-order8.C @@ -0,0 +1,20 @@ +// PR c++/91987 +// { dg-do run } +// { dg-options "-fstrong-eval-order" } + +int a[4] = { 1, 2, 3, 4 }; +int b; + +int +main () +{ + int *x = a; + b = 1; + int r = (b = 4, x)[(b *= 2, 3)]; + if (b != 8 || r != 4) + __builtin_abort (); + b = 1; + r = (b = 3, 2)[(b *= 2, x)]; + if (b != 6 || r != 3) + __builtin_abort (); +}