Re: [PATCH] use zero as the lower bound for a signed-unsigned range (PR 79327)
On Fri, Feb 03, 2017 at 05:39:21PM +0100, Jakub Jelinek wrote: > Say in the http://gcc.gnu.org/ml/gcc-patches/2017-02/msg00234.html > patch, you would with my patch need just the tree_digits part, > and then the very last hunk in gimple-ssa-sprintf.c (with > likely_adjust && > removed). Because you do the adjustments only if !res.knownrange > and in that case you know argmin/argmax are actually dirtype's min/max, > so 0 must be in the range. You've committed the patch unnecessarily complicated, see above. The following gives the same testsuite result. dirtype is one of the standard {un,}signed {char,short,int,long,long long} types, all of them have 0 in their ranges. For VR_RANGE we almost always set res.knownrange to true: /* Set KNOWNRANGE if the argument is in a known subrange of the directive's type (KNOWNRANGE may be reset below). */ res.knownrange = (!tree_int_cst_equal (TYPE_MIN_VALUE (dirtype), argmin) || !tree_int_cst_equal (TYPE_MAX_VALUE (dirtype), argmax)); (the exception is in case that range clearly has to include zero), and reset it only if adjust_range_for_overflow returned true, which means it also set the range to TYPE_M{IN,AX}_VALUE (dirtype) and again includes zero. So IMNSHO likely_adjust in what you've committed is always true when you use it and thus just a useless computation and something to make the code harder to understand. Even if you don't trust this, with the ranges in argmin/argmax, it is IMHO undesirable to set it differently at the different code paths, if you want to check whether the final range includes zero and at least one another value, just do - if (likely_adjust && maybebase && base != 10) + if ((tree_int_cst_sgn (argmin) < 0 || tree_int_cst_sgn (argmax) > 0) && maybebase && base != 10) Though, it is useless both for the above reason and for the reason that you actually do something only: if (res.range.min == 1) res.range.likely += base == 8 ? 1 : 2; else if (res.range.min == 2 && base == 16 && (dir.width[0] == 2 || dir.prec[0] == 2)) ++res.range.likely; where if the range doesn't include zero, you would never get res.range.min of 1 and for base == 16 also not 2. 2017-02-04 Jakub Jelinek PR tree-optimization/79327 * gimple-ssa-sprintf.c (format_integer): Remove likely_adjust variable, its initialization and use. --- gcc/gimple-ssa-sprintf.c.jj 2017-02-04 08:43:12.0 +0100 +++ gcc/gimple-ssa-sprintf.c2017-02-04 08:45:33.173709580 +0100 @@ -1232,10 +1232,6 @@ format_integer (const directive &dir, tr of the format string by returning [-1, -1]. */ return fmtresult (); - /* True if the LIKELY counter should be adjusted upward from the MIN - counter to account for arguments with unknown values. */ - bool likely_adjust = false; - fmtresult res; /* Using either the range the non-constant argument is in, or its @@ -1265,14 +1261,6 @@ format_integer (const directive &dir, tr res.argmin = argmin; res.argmax = argmax; - - /* Set the adjustment for an argument whose range includes -zero since that doesn't include the octal or hexadecimal -base prefix. */ - wide_int wzero = wi::zero (wi::get_precision (min)); - if (wi::le_p (min, wzero, SIGNED) - && !wi::neg_p (max)) - likely_adjust = true; } else if (range_type == VR_ANTI_RANGE) { @@ -1307,11 +1295,6 @@ format_integer (const directive &dir, tr if (!argmin) { - /* Set the adjustment for an argument whose range includes -zero since that doesn't include the octal or hexadecimal -base prefix. */ - likely_adjust = true; - if (TREE_CODE (argtype) == POINTER_TYPE) { argmin = build_int_cst (pointer_sized_int_node, 0); @@ -1371,7 +1354,7 @@ format_integer (const directive &dir, tr else { res.range.likely = res.range.min; - if (likely_adjust && maybebase && base != 10) + if (maybebase && base != 10) { if (res.range.min == 1) res.range.likely += base == 8 ? 1 : 2; --- gcc/testsuite/ChangeLog.jj 2017-02-04 08:43:14.0 +0100 +++ gcc/testsuite/ChangeLog 2017-02-04 08:48:14.930627979 +0100 @@ -20,8 +20,8 @@ PR tree-optimization/79327 * gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Adjust. - * gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-13.c: New test. - * gcc/testsuite/gcc.dg/tree-ssa/pr79327-2.c: Ditto. + * gcc.dg/tree-ssa/builtin-sprintf-warn-13.c: New test. + * gcc.dg/tree-ssa/pr79327-2.c: Ditto. 2017-02-03 Jakub Jelinek Martin Sebor @@ -306,7 +306,7 @@ 2017-01-27 Vladimir Makarov PR tree-optimization/71374 - * testsuite/gcc.target/i386/pr71374.c: New. + * gcc.target/i386/p
Re: [wwwdocs] Uncomment link to gcc-7/porting_to.html
On Fri, 3 Feb 2017, David Malcolm wrote: > I took the liberty of committing this, under the "obvious" rule. Thanks! No liberty, no need to quote the "obvious" rule, just be my guest making changes like this (and beyond). :-) Gerald
Re: [PATCH][AArch64] Accept more addressing modes for PRFM
> On Feb 3, 2017, at 8:12 PM, Kyrill Tkachov > wrote: > > Hi all, > > While evaluating Maxim's SW prefetch patches [1] I noticed that the aarch64 > prefetch pattern is > overly restrictive in its address operand. It only accepts simple register > addressing modes. > In fact, the PRFM instruction accepts almost all modes that a normal 64-bit > LDR supports. > The restriction in the pattern leads to explicit address calculation code to > be emitted which we could avoid. Thanks for this fix, I'll test it on my hardware. I've reviewed your patch and it looks OK to me. > > This patch relaxes the restrictions on the prefetch define_insn. It creates a > predicate and constraint that > allow the full addressing modes that PRFM allows. Thus for the testcase in > the patch (adapted from one of the existing > __builtin_prefetch tests in the testsuite) we can generate a: > prfmPLDL1STRM, [x1, 8] > > instead of the current > prfmPLDL1STRM, [x1] > with an explicit increment of x1 by 8 in a separate instruction. > > I've removed the %a output modifier in the output template and wrapped the > address operand into a DImode MEM before > passing it down to aarch64_print_operand. > > This is because operand 0 is an address operand rather than a memory operand > and thus doesn't have a mode associated > with it. When processing the 'a' output modifier the code in final.c will > call TARGET_PRINT_OPERAND_ADDRESS with a VOIDmode > argument. This will ICE on aarch64 because we need a mode for the memory in > order for aarch64_classify_address to work > correctly. Rather than overriding the VOIDmode in > aarch64_print_operand_address I decided to instead create the DImode > MEM in the "prefetch" output template and treat it as a normal 64-bit memory > address, which at the point of assembly output > is what it is anyway. I agree that it is cleaner to convert operand of prefetch to DImode just before printing out to assembly. There is little to be gained in relaxing asserts in aarch64_print_operand_address. > > With this patch I see a reduction in instruction count in the SPEC2006 > benchmarks when SW prefetching is enabled on top > of Maxim's patchset because fewer address calculation instructions are > emitted due to the use of the more expressive > addressing modes. It also fixes a performance regression that I observed in > 410.bwaves from Maxim's patches on Cortex-A72. > I'll be running a full set of benchmarks to evaluate this further, but I > think this is the right thing to do. > > Bootstrapped and tested on aarch64-none-linux-gnu. > > Maxim, do you want to try this on top of your patches on your hardware to see > if it helps with the regressions you mentioned? Sure. -- Maxim Kuvyrkov www.linaro.org
[GIMPLE FE] Avoid ICE with __builtin_abs
Hi, The following test-case ICE's with -fgimple: int __GIMPLE foo(int a) { int t1; t1_1 = __builtin_abs (a); return t1_1; } gimplefe-2.c:4:3: internal compiler error: in get_callee_fndecl, at tree.c:9500 t1_1 = __builtin_abs (a); ^~~~ 0xe96e8d get_callee_fndecl(tree_node const*) ../../gcc/gcc/tree.c:9500 0x924d75 gimple_build_call_from_tree(tree_node*) ../../gcc/gcc/gimple.c:351 0x6c86b3 c_parser_gimple_statement ../../gcc/gcc/c/gimple-parser.c:393 0x6c86b3 c_parser_gimple_compound_statement ../../gcc/gcc/c/gimple-parser.c:216 0x6c86b3 c_parser_parse_gimple_body(c_parser*) ../../gcc/gcc/c/gimple-parser.c:93 0x6b04f1 c_parser_declaration_or_fndef ../../gcc/gcc/c/c-parser.c:2081 0x6b883b c_parser_external_declaration ../../gcc/gcc/c/c-parser.c:1464 0x6b92a1 c_parser_translation_unit ../../gcc/gcc/c/c-parser.c:1344 0x6b92a1 c_parse_file() ../../gcc/gcc/c/c-parser.c:18141 0x717832 c_common_parse_file() ../../gcc/gcc/c-family/c-opts.c:1102 This happens because __builtin_abs(a) gets folded to > and get_callee_fndecl expects CALL_EXPR. The attached patch tries to fix the issue by building gimple_assign with appropriate subcode for functions that get folded to expression instead of trying to build it as a function-call. Is it OK to commit after bootstrap+test ? Thanks, Prathamesh 2017-02-04 Prathamesh Kulkarni * gimple-pretty-print.c (dump_unary_rhs): Change dump format for ABS_EXPR for gimple dump. * tree-pretty-print.c (dump_generic_node): Likewise. c/ * gimple-parser.c (c_parser_gimple_statement): Check if rhs.value is CALL_EXPR and build gimple_assign if it isn't. testsuite/ * gcc.dg/gimplefe-23.c: New test-case. diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c index 7feb6d0..4e2ae37 100644 --- a/gcc/c/gimple-parser.c +++ b/gcc/c/gimple-parser.c @@ -401,10 +401,22 @@ c_parser_gimple_statement (c_parser *parser, gimple_seq *seq) rhs = c_parser_gimple_unary_expression (parser); if (rhs.value != error_mark_node) { - gimple *call = gimple_build_call_from_tree (rhs.value); - gimple_call_set_lhs (call, lhs.value); - gimple_seq_add_stmt (seq, call); - gimple_set_location (call, loc); + if (TREE_CODE (rhs.value) == CALL_EXPR) + { + gimple *call = gimple_build_call_from_tree (rhs.value); + gimple_call_set_lhs (call, lhs.value); + gimple_seq_add_stmt (seq, call); + gimple_set_location (call, loc); + } + else + { + tree value = tree_strip_nop_conversions (rhs.value); + gcc_assert (get_gimple_rhs_class (TREE_CODE (value)) + != GIMPLE_INVALID_RHS); + gassign *assign = gimple_build_assign (lhs.value, value); + gimple_seq_add_stmt (seq, assign); + gimple_set_location (assign, loc); + } } return; } diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c index 91c839d..f773d85 100644 --- a/gcc/gimple-pretty-print.c +++ b/gcc/gimple-pretty-print.c @@ -323,9 +323,18 @@ dump_unary_rhs (pretty_printer *buffer, gassign *gs, int spc, int flags) break; case ABS_EXPR: - pp_string (buffer, "ABS_EXPR <"); - dump_generic_node (buffer, rhs, spc, flags, false); - pp_greater (buffer); + if (flags & TDF_GIMPLE) + { + pp_string (buffer, "__builtin_abs ("); + dump_generic_node (buffer, rhs, spc, flags, false); + pp_right_paren (buffer); + } + else + { + pp_string (buffer, "ABS_EXPR <"); + dump_generic_node (buffer, rhs, spc, flags, false); + pp_greater (buffer); + } break; default: diff --git a/gcc/testsuite/gcc.dg/gimplefe-23.c b/gcc/testsuite/gcc.dg/gimplefe-23.c new file mode 100644 index 000..1448030 --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-23.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O -fgimple -fdump-tree-ssa-gimple" } */ + +int __GIMPLE foo(int a) +{ + int t1; + t1_1 = __builtin_abs (a); + return t1_1; +} + +/* { dg-final { scan-tree-dump "__builtin_abs" "ssa" } } */ diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c index d823c2e..f1a5bdd 100644 --- a/gcc/tree-pretty-print.c +++ b/gcc/tree-pretty-print.c @@ -2441,9 +2441,18 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, int flags, break; case ABS_EXPR: - pp_string (pp, "ABS_EXPR <"); - dump_generic_node (pp, TREE_OPERAND (node, 0), spc, flags, false); - pp_greater (pp); + if (flags & TDF_GIMPLE) + { + pp_string (pp, "__builtin_abs ("); + dump_generic_node (pp, TREE_OPERAND (node, 0), spc, flags, false); + pp_right_paren (pp); + } + else + { + pp_string (pp, "ABS_EXPR <"); + dump_generic_node (pp, TREE_OPERAND (node, 0), spc, flags, false); + p
[PATCH, Fortran, pr79230, v1] [7 Regression] [OOP] Run time error: double free or corruption
Hi all, attached patch fixes a regression when a polymorphic pointer component was present. The results was a double free. The attached patch fixes this, by not caring about freeing pointer components as part of gfortran's memory management, i.e., the programmer has to take care about freeing/disassociating the pointer using a finalizer, as is IMO the intention of the Fortran standard. Bootstrapped and regtested ok on x86_64-linux/f25. Ok for trunk (current or next, haven't monitored whether commits are still allowed)? Regards, Andre -- Andre Vehreschild * Email: vehre ad gmx dot de gcc/fortran/ChangeLog: 2017-02-04 Andre Vehreschild PR fortran/79230 * trans-array.c (structure_alloc_comps): Ignore pointer components when freeing structures. gcc/testsuite/ChangeLog: 2017-02-04 Andre Vehreschild PR fortran/79230 * gfortran.dg/der_ptr_component_2.f90: New test. diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c index a3aab8e..d0dfc26 100644 --- a/gcc/fortran/trans-array.c +++ b/gcc/fortran/trans-array.c @@ -8220,9 +8220,17 @@ structure_alloc_comps (gfc_symbol * der_type, tree decl, /* Shortcut to get the attributes of the component. */ if (c->ts.type == BT_CLASS) - attr = &CLASS_DATA (c)->attr; + { + attr = &CLASS_DATA (c)->attr; + if (attr->class_pointer) + continue; + } else - attr = &c->attr; + { + attr = &c->attr; + if (attr->pointer) + continue; + } if ((c->ts.type == BT_DERIVED && !c->attr.pointer) || (c->ts.type == BT_CLASS && !CLASS_DATA (c)->attr.class_pointer)) diff --git a/gcc/testsuite/gfortran.dg/der_ptr_component_2.f90 b/gcc/testsuite/gfortran.dg/der_ptr_component_2.f90 new file mode 100644 index 000..4eb0869 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/der_ptr_component_2.f90 @@ -0,0 +1,30 @@ +! { dg-do run } +! +! Freeing the width_data lead to double free. This testcase tests that +! pr79230 is fixed now. + +program main_ut + implicit none + + type :: data_t + character, allocatable :: c1 + end type + + type :: t1_t + character, allocatable :: c2 + class(data_t), pointer :: width_data + end type + + call evaluator + +contains + + subroutine evaluator +type(data_t), target :: par_real +type(t1_t) :: field +field%width_data => par_real + end subroutine + +end + +
Re: [PATCH GCC 6/9]Simplify control flow graph for vectorized loop
> Hi, > This is the main patch improving control flow graph for vectorized loop. It > generally rewrites loop peeling stuff in vectorizer. As described in patch, > for a typical loop to be vectorized like: > >preheader: > LOOP: >header_bb: >loop_body >if (exit_loop_cond) goto exit_bb >elsegoto header_bb >exit_bb: > > This patch peels prolog and epilog from the loop, adds guards skipping PROLOG > and EPILOG for various conditions. As a result, the changed CFG would look > like: > >guard_bb_1: >if (prefer_scalar_loop) goto merge_bb_1 >elsegoto guard_bb_2 > >guard_bb_2: > if (skip_prolog) goto merge_bb_2 > else goto prolog_preheader > >prolog_preheader: > PROLOG: >prolog_header_bb: >prolog_body >if (exit_prolog_cond) goto prolog_exit_bb >else goto prolog_header_bb >prolog_exit_bb: > >merge_bb_2: > >vector_preheader: > VECTOR LOOP: >vector_header_bb: >vector_body >if (exit_vector_cond) goto vector_exit_bb >else goto vector_header_bb >vector_exit_bb: > >guard_bb_3: >if (skip_epilog) goto merge_bb_3 >else goto epilog_preheader > >merge_bb_1: > >epilog_preheader: > EPILOG: >epilog_header_bb: >epilog_body >if (exit_epilog_cond) goto merge_bb_3 >else goto epilog_header_bb > >merge_bb_3: > > Bin, this patch seems to miss profile updating, so we get mismatches even for quite simple loops https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79347 I am bit lost in how the code is organized - it does insert multiple guards to the preheader edge? Honza
[PATCH, Fortran, pr78958, v1] Unallocated memory access after SOURCE-ALLOCATEing unlimited polymorphic object
Hi all, attached patch takes the _len-component of unlimited polymorphic objects into account, when source-ALLOCATEing an unlimited polymorphic object. The testcase for this gcc/testsuite/gfortran.dg/alloc_comp_class_5.f03 with valgrind/sanitizer. I therefore added no additional testcase. Bootstraps and regtests ok on x86_64-linux/f25 and as reported in the PR on 32bit-hpux. Ok for trunk (when?)? Regards, Andre -- Andre Vehreschild * Email: vehre ad gmx dot de gcc/fortran/ChangeLog: 2017-02-04 Andre Vehreschild PR fortran/78958 * trans-stmt.c (gfc_trans_allocate): Add the multiplying the _len component of unlimited polymorphic objects when source-allocating. diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c index ad4b737..61e597f 100644 --- a/gcc/fortran/trans-stmt.c +++ b/gcc/fortran/trans-stmt.c @@ -6009,14 +6009,21 @@ gfc_trans_allocate (gfc_code * code) needs to be provided, which is done most of the time by the pre-evaluation step. */ nelems = NULL_TREE; - if (expr3_len && code->expr3->ts.type == BT_CHARACTER) - /* When al is an array, then the element size for each element - in the array is needed, which is the product of the len and - esize for char arrays. */ - tmp = fold_build2_loc (input_location, MULT_EXPR, - TREE_TYPE (expr3_esize), expr3_esize, - fold_convert (TREE_TYPE (expr3_esize), - expr3_len)); + if (expr3_len && (code->expr3->ts.type == BT_CHARACTER + || code->expr3->ts.type == BT_CLASS)) + { + /* When al is an array, then the element size for each element + in the array is needed, which is the product of the len and + esize for char arrays. For unlimited polymorphics len can be + zero, therefore take the maximum of len and one. */ + tmp = fold_build2_loc (input_location, MAX_EXPR, + TREE_TYPE (expr3_len), + expr3_len, fold_convert (TREE_TYPE (expr3_len), + integer_one_node)); + tmp = fold_build2_loc (input_location, MULT_EXPR, + TREE_TYPE (expr3_esize), expr3_esize, + fold_convert (TREE_TYPE (expr3_esize), tmp)); + } else tmp = expr3_esize; if (!gfc_array_allocate (&se, expr, stat, errmsg, errlen,
[PATCH, Fortran, pr79335, v1] [7 Regression] Conditional jump or move depends on uninitialised in value get_scalar_to_descriptor_type(tree_node*, symbol_attribute) (trans-expr.c:53)
Hi all, attached patch fixes the access of uninitialized data. Thanks Martin for analyzing this so far. The symbol_attributes on the stack was used without initializing it from the symbol's attributes. Bootstraps and regstests ok on x86_64-linux-gnu/f25. Ok for trunk? Regards, Andre -- Andre Vehreschild * Email: vehre ad gmx dot de gcc/fortran/ChangeLog: 2017-02-04 Andre Vehreschild PR fortran/79335 * trans-decl.c (generate_coarray_sym_init): Retrieve the symbol's attributes before using them. diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c index 3e54e80..41b36a5 100644 --- a/gcc/fortran/trans-decl.c +++ b/gcc/fortran/trans-decl.c @@ -5128,6 +5128,16 @@ generate_coarray_sym_init (gfc_symbol *sym) else reg_type = GFC_CAF_COARRAY_STATIC; + /* Compile the symbol attribute. */ + if (sym->ts.type == BT_CLASS) +{ + attr = CLASS_DATA (sym)->attr; + /* The pointer attribute is always set on classes, overwrite it with the + class_pointer attribute, which denotes the pointer for classes. */ + attr.pointer = attr.class_pointer; +} + else +attr = sym->attr; gfc_init_se (&se, NULL); desc = gfc_conv_scalar_to_descriptor (&se, decl, attr); gfc_add_block_to_block (&caf_init_block, &se.pre);
Re: [PATCH] Fix __atomic to not implement atomic loads with CAS.
> This patch fixes the __atomic builtins to not implement supposedly > lock-free atomic loads based on just a compare-and-swap operation. > … Commit r245098 caused New failures: FAIL: libgomp.c/atomic-2.c (test for excess errors) FAIL: libgomp.c/atomic-4.c (test for excess errors) FAIL: libgomp.c/atomic-5.c (test for excess errors) FAIL: libgomp.c/pr48591.c (test for excess errors) FAIL: libgomp.c/pr64868.c (test for excess errors) FAIL: libgomp.c++/pr64868.C (test for excess errors) (see https://gcc.gnu.org/ml/gcc-regression/2017-02/msg00011.html). The error is Undefined symbols for architecture x86_64: "___atomic_load_16", referenced from: TIA, Dominique
[RFA][PR tree-optimization/79095] [PATCH 1/4] Improve ranges for MINUS_EXPR and EXACT_DIV_EXPR
This is the first of a 4 part series to address the issues around 79095. This patch addresses improvements in determining ranges of binary expressions in three ways. First if we are otherwise unable to find a range for the result of a MINUS_EXPR, if we know the arguments are not equal, then we know the resultant range is ~[0,0]. Second, for EXACT_DIV_EXPR, if the numerator has the range ~[0,0], then resultant range is currently [TYPE_MIN/DENOM,TYPE_MAX/DENOM]. That is rarely a useful range. A resultant range of ~[0,0] is actually more useful since it often tells us something important about the difference of two pointers. Finally, when vrp2 discovers an updated range for an object that had a range discovered by vrp1, if the new range is ~[0,0], prefer that new range in some cases. This is needed to avoid losing the newly discovered ~[0,0] range for EXACT_DIV_EXPR. Bootstrapped and regression tested with the other patches in this series. OK for the trunk? Jeff * tree-vrp.c (extract_range_from_binary_expr): For EXACT_DIV_EXPR, if the numerator has the range ~[0,0] make the resultant range ~[0,0]. For MINUS_EXPR with no derived range, if the operands are known to be not equal, then the resulting range is ~[0,0]. (intersect_ranges): In some cases prefer ~[0,0]. commit b7baf46ab62e28d2dbc22e9dcd4404926d59df18 Author: Jeff Law Date: Fri Feb 3 15:45:58 2017 -0500 Improved ranges diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c index b429217..3338d8b 100644 --- a/gcc/tree-vrp.c +++ b/gcc/tree-vrp.c @@ -3298,6 +3298,37 @@ extract_range_from_binary_expr (value_range *vr, extract_range_from_binary_expr_1 (vr, code, expr_type, &n_vr0, &vr1); } + + /* EXACT_DIV_EXPR is typically used for pointer subtraction; + as a result a ~[0,0] may be better than what has already + been computed. + + In particular if numerator has the range ~[0,0], then the + result range is going to be something like + [MININT/DIVISOR,MAXINT/DIVISOR], which is rarely useful. + + So instead make the result range ~[0,0]. */ + if (code == EXACT_DIV_EXPR + && TREE_CODE (op0) == SSA_NAME + && vr0.type == VR_ANTI_RANGE + && vr0.min == vr0.max + && integer_zerop (vr0.min)) +set_value_range_to_nonnull (vr, TREE_TYPE (op0)); + + /* If we didn't derive a range for MINUS_EXPR, and + op1's range is ~[op0,op0] or vice-versa, then we + can derive a non-null range. This happens often for + pointer subtraction. */ + if (vr->type == VR_VARYING + && code == MINUS_EXPR + && TREE_CODE (op0) == SSA_NAME + && ((vr0.type == VR_ANTI_RANGE + && symbolic_range_based_on_p (&vr0, op1) + && vr0.min == vr0.max) + || (vr1.type == VR_ANTI_RANGE + && symbolic_range_based_on_p (&vr1, op0) + && vr1.min == vr1.max))) + set_value_range_to_nonnull (vr, TREE_TYPE (op0)); } /* Extract range information from a unary operation CODE based on @@ -8620,6 +8651,12 @@ intersect_ranges (enum value_range_type *vr0type, else if (vrp_val_is_min (vr1min) && vrp_val_is_max (vr1max)) ; + /* Choose the anti-range if it is ~[0,0], that range is special +enough to special case. */ + else if (*vr0type == VR_ANTI_RANGE + && *vr0min == *vr0max + && integer_zerop (*vr0min)) + ; /* Else choose the range. */ else {
[RFA] [PR tree-optimization/79095][PATCH 2/4] Add infrastructure to detect overflow checks
This is the second in the 4 part series to address 79095. This patch introduces a new function into tree-vrp.c to allow for the detection of overflow checks of the form A OP A + CST (for unsigned/wrapping A). This is implemented by first checking for A OP B, we then conditionally walk the ASSERT_EXPR chain for B to produce B'. We then look at the defining statement for B or B' to see if it has the form B = X + CST or B' = X + CST respectively. Then we conditionally walk the ASSERT_EXPR chain for A to see if it resolves to X at any point. There have been cases where no walking was necessary to show that X resolves to A. Other cases have required walking part or the entire ASSERT_EXPR chain. We do not walk during propagation, but do walk during folding/simplification. At this point we have an overflow check of the appropriate form. We compute an updated constant so that we can check for overflow with expressions like A > 0xfffe or A <= 0 Those are particularly interesting forms as they collapse into equality tests (next patch). The code supports other forms, but they're not as useful because they don't end up generating equality tests or allow for constant propagation. Bootstrapped and regression tested with the other patches in this series. OK for the trunk? * tree-vrp.c (overflow_comparison_p): New function. diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c index 3338d8b..6459c71 100644 --- a/gcc/tree-vrp.c +++ b/gcc/tree-vrp.c @@ -5189,6 +5189,94 @@ masked_increment (const wide_int &val_in, const wide_int &mask, return val ^ sgnbit; } +/* OP0 CODE OP1 is a comparison. Examine the comparison and potentially + OP1's defining statement to see if it ultimately has the form + OP0 CODE (OP0 PLUS INTEGER_CST) + + If so, return TRUE indicating this is an overflow test and store into + *NEW_CST an updated constant that can be used in a narrowed range test. + + REVERSED indicates if the comparison was originally: + + OP1 CODE' OP0. + + This affects how we build the updated constant. */ + +static bool +overflow_comparison_p (enum tree_code code, tree op0, tree op1, + bool follow_assert_exprs, bool reversed, tree *new_cst) +{ + /* See if this is a relational operation between two SSA_NAMES with + unsigned, overflow wrapping values. If so, check it more deeply. */ + if ((code == LT_EXPR || code == LE_EXPR + || code == GE_EXPR || code == GT_EXPR) + && TREE_CODE (op0) == SSA_NAME + && TREE_CODE (op1) == SSA_NAME + && INTEGRAL_TYPE_P (TREE_TYPE (op0)) + && TYPE_UNSIGNED (TREE_TYPE (op0)) + && TYPE_OVERFLOW_WRAPS (TREE_TYPE (op0))) +{ + gimple *op1_def = SSA_NAME_DEF_STMT (op1); + + /* If requested, follow any ASSERT_EXPRs backwards for OP1. */ + if (follow_assert_exprs) + { + while (gimple_assign_single_p (op1_def) +&& TREE_CODE (gimple_assign_rhs1 (op1_def)) == ASSERT_EXPR) + { + op1 = TREE_OPERAND (gimple_assign_rhs1 (op1_def), 0); + if (TREE_CODE (op1) != SSA_NAME) + break; + op1_def = SSA_NAME_DEF_STMT (op1); + } + } + + /* Now look at the defining statement of OP1 to see if it adds +or subtracts a nonzero constant from another operand. */ + if (op1_def + && is_gimple_assign (op1_def) + && gimple_assign_rhs_code (op1_def) == PLUS_EXPR + && TREE_CODE (gimple_assign_rhs2 (op1_def)) == INTEGER_CST + && wi::ne_p (gimple_assign_rhs2 (op1_def), 0)) + { + tree target = gimple_assign_rhs1 (op1_def); + + /* If requested, follow ASSERT_EXPRs backwards for op0 looking +for one where TARGET appears on the RHS. */ + if (follow_assert_exprs) + { + /* Now see if that "other operand" is op0, following the chain +of ASSERT_EXPRs if necessary. */ + gimple *op0_def = SSA_NAME_DEF_STMT (op0); + while (op0 != target +&& gimple_assign_single_p (op0_def) +&& TREE_CODE (gimple_assign_rhs1 (op0_def)) == ASSERT_EXPR) + { + op0 = TREE_OPERAND (gimple_assign_rhs1 (op0_def), 0); + if (TREE_CODE (op0) != SSA_NAME) + break; + op0_def = SSA_NAME_DEF_STMT (op0); + } + } + + /* If we did not find our target SSA_NAME, then this is not +an overflow test. */ + if (op0 != target) + return false; + + tree type = TREE_TYPE (op0); + wide_int max = wi::max_value (TYPE_PRECISION (type), UNSIGNED); + HOST_WIDE_INT inc = TREE_INT_CST_LOW (gimple_assign_rhs2 (op1_def)); + if (reversed) + *new_cst = wide_int_to_tree (type, max + inc); + else + *new_cst = wide_int_to_tree (type, max - inc); + return true; +
[RFA][PR tree-optimization/79095][PATCH 4/4] Tests
These are the tests. The tests in g++.dg start with a reduced test from Martin (pr79095-1.C) that includes a size check. With the size != 0 check this testcase should not issue any warnings as the path that turns into a __builtin_memset should get eliminated. pr79095-2.C is the same test, but without the size != test. This test should trigger an "exceeds maximum object size" warning. pr79095-3.C is the original test from the BZ, but with a proper size check on the vector. This test should not produce a warning. pr79095-4.C is the original test from the BZ, but without a size check on the vector. This should produce *one* warning (trunk currently generates 3). We verify that there's just one __builtin_memset by the time VRP2 is done and verify that we do get the desired warning. pr79095-5.C is another test from Martin which should not produce a warning. gcc-torture/execute/arith-1.c is updated to test a few more cases. This was mis-compiled at some point during patch development and at that time I added the additional tsts. gcc.dg/tree-ssa/pr79095.c is a new test to verify that VRP can propagate constants generated on the true/false arms of an overflow test and that constants to _not_ propagate into the wrong arm of the conditional. These were included in the bootstrap & regression testing of the prior patches. All the tests pass with the prior patches installed. OK for the trunk? * g++.dg/pr79095-1.C: New test * g++.dg/pr79095-2.C: New test * g++.dg/pr79095-3.C: New test * g++.dg/pr79095-4.C: New test * g++.dg/pr79095-5.C: New test * gcc.c-torture/execute/arith-1.c: Update with more cases. * gcc.dg/tree-ssa/pr79095-1.c: New test. diff --git a/gcc/testsuite/g++.dg/pr79095-1.C b/gcc/testsuite/g++.dg/pr79095-1.C new file mode 100644 index 000..4b8043c --- /dev/null +++ b/gcc/testsuite/g++.dg/pr79095-1.C @@ -0,0 +1,40 @@ +/* { dg-do compile } */ +/* { dg-options "-Wall -O3" } */ + +typedef long unsigned int size_t; + +inline void +fill (int *p, size_t n, int) +{ + while (n--) +*p++ = 0; +} + +struct B +{ + int* p0, *p1, *p2; + + size_t size () const { +return size_t (p1 - p0); + } + + void resize (size_t n) { +if (n > size()) + append (n - size()); + } + + void append (size_t n) + { +if (size_t (p2 - p1) >= n) { + fill (p1, n, 0); +} + } +}; + +void foo (B &b) +{ + if (b.size () != 0) +b.resize (b.size () - 1); +} + + diff --git a/gcc/testsuite/g++.dg/pr79095-2.C b/gcc/testsuite/g++.dg/pr79095-2.C new file mode 100644 index 000..9dabc7e --- /dev/null +++ b/gcc/testsuite/g++.dg/pr79095-2.C @@ -0,0 +1,46 @@ +/* { dg-do compile } */ +/* { dg-options "-Wall -O3" } */ + +typedef long unsigned int size_t; + +inline void +fill (int *p, size_t n, int) +{ + while (n--) +*p++ = 0; +} + +struct B +{ + int* p0, *p1, *p2; + + size_t size () const { +return size_t (p1 - p0); + } + + void resize (size_t n) { +if (n > size()) + append (n - size()); + } + + void append (size_t n) + { +if (size_t (p2 - p1) >= n) { + fill (p1, n, 0); +} + } +}; + +void foo (B &b) +{ +b.resize (b.size () - 1); +} + +/* If b.size() == 0, then the argument to b.resize is -1U (it overflowed). + This will result calling "fill" which turns into a memset with a bogus + length argument. We want to make sure we warn, which multiple + things. First the ldist pass converted the loop into a memset, + cprop and simplifications made the length a constant and the static + analysis pass determines it's a bogus size to pass to memset. */ +/* { dg-warning "exceeds maximum object size" "" { target *-*-* } 0 } */ + diff --git a/gcc/testsuite/g++.dg/pr79095-3.C b/gcc/testsuite/g++.dg/pr79095-3.C new file mode 100644 index 000..28c8a37 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr79095-3.C @@ -0,0 +1,17 @@ +/* { dg-do compile } */ +/* { dg-options "-Wall -O3" } */ + +#include + +void foo(std::vector &v); + +void vtest() +{ + std::vector v; + foo (v); + if (v.size() > 0) + { +v.resize (v.size()-1); + } +} + diff --git a/gcc/testsuite/g++.dg/pr79095-4.C b/gcc/testsuite/g++.dg/pr79095-4.C new file mode 100644 index 000..df55025 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr79095-4.C @@ -0,0 +1,26 @@ +/* { dg-do compile } */ +/* { dg-options "-Wall -O3 -fdump-tree-vrp2" } */ + +#include + +void foo(std::vector &v); + +void vtest() +{ + std::vector v; + foo (v); + { +v.resize (v.size()-1); + } +} + +/* As written this testcase should trigger a warning. We overflow to -1U + if v.size() == 0 in foo(). This results in bogus calls to memset. + + The number of clearing loops in the IL can vary depending on the C++ + mode used for the test. But by the end of VRP2, there should be a single + clearing loop left and it should be using memcpy. */ +/* { dg-final { scan-tree-dump-times "__builtin_memset \\(_\[0-9\]+, 0, \[0-9\]+\
[RFA] [PR tree-optimization/79095][PATCH 3/4] Improve ASSERT_EXPRs and simplification of overflow tests
This is the actual optimization piece of the patchseries and uses the overflow test detection function in patch #2. First, when we detect an overflow test, we register additional ASSERT_EXPRs for the given name. So instead of an ASSERT_EXPR for an expression like A < B, we get an assert like A > 0xfffe or A <= 0. Additionally, during propagation and folding, if we are presented with an overflow test which collapses into an equality test, we will simplify the test into an equality check (without changing the IL). So A + 1 < A would turn into A == -1 or A + 1 > A turns into A != -1. There's a corresponding equivalent for A - 1 < A and A - 1 > A. The net result is the new ASSERT_EXPRs and simplified tests allow VRP to eliminate more paths through the CFG and improve its constant propagation capabilities. Examples can be found in the next patch which has the tests. Bootstrapped and regression tested with the other patches in this series. OK for the trunk? * tree-vrp.c (register_edge_assert_for_2): Register additional asserts fif NAME is used in an overflow test. (vrp_evaluate_conditional_warnv_with_ops): If the ops represent an overflow check that can be expressed as an equality test, then adjust ops to be that equality test. diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c index 6459c71..8d78646 100644 --- a/gcc/tree-vrp.c +++ b/gcc/tree-vrp.c @@ -5299,7 +5298,19 @@ register_edge_assert_for_2 (tree name, edge e, gimple_stmt_iterator bsi, /* Only register an ASSERT_EXPR if NAME was found in the sub-graph reachable from E. */ if (live_on_edge (e, name)) -register_new_assert_for (name, name, comp_code, val, NULL, e, bsi); +{ + tree x; + if (overflow_comparison_p (comp_code, name, val, false, false, &x) + || overflow_comparison_p (swap_tree_comparison (comp_code), val, name, + false, true, &x)) + { + enum tree_code new_code + = ((comp_code == GT_EXPR || comp_code == GE_EXPR) + ? GT_EXPR : LE_EXPR); + register_new_assert_for (name, name, new_code, x, NULL, e, bsi); + } + register_new_assert_for (name, name, comp_code, val, NULL, e, bsi); +} /* In the case of NAME <= CST and NAME being defined as NAME = (unsigned) NAME2 + CST2 we can assert NAME2 >= -CST2 @@ -7658,6 +7669,60 @@ vrp_evaluate_conditional_warnv_with_ops (enum tree_code code, tree op0, && !POINTER_TYPE_P (TREE_TYPE (op0))) return NULL_TREE; + /* If OP0 CODE OP1 is an overflow comparison, if it can be expressed + as a simple equality test, then prefer that over its current form + for evaluation. + + An overflow test which collapses to an equality test can always be + expressed as a comparison of one argument against zero. Overflow + occurs when the chosen argument is zero and does not occur if the + chosen argument is not zero. */ + tree x; + if (overflow_comparison_p (code, op0, op1, use_equiv_p, false, &x)) +{ + wide_int max = wi::max_value (TYPE_PRECISION (TREE_TYPE (op0)), UNSIGNED); + /* B = A - 1; if (A < B) -> B = A - 1; if (A == 0) + B = A - 1; if (A > B) -> B = A - 1; if (A != 0) */ + if (integer_zerop (x)) + { + op1 = x; + code = (code == LT_EXPR || code == LE_EXPR) ? EQ_EXPR : NE_EXPR; + } + /* B = A + 1; if (A > B) -> B = A + 1; if (B == 0) + B = A + 1; if (A < B) -> B = A + 1; if (B != 0) */ + else if (wi::eq_p (x, max - 1)) + { + op0 = op1; + op1 = wide_int_to_tree (TREE_TYPE (op0), 0); + code = (code == GT_EXPR || code == GE_EXPR) ? EQ_EXPR : NE_EXPR; + } +} + else if (overflow_comparison_p (swap_tree_comparison (code), + op1, op0, use_equiv_p, true, &x)) +{ + /* X holds the value if we wanted to generate an overflow check +for the comparison using OP1. But we're actually going to +test against OP0 and we're always going to use an equality +test, so the constants for detection below are different +than the constant we pass into vrp_evaluate_... */ + wide_int max = wi::max_value (TYPE_PRECISION (TREE_TYPE (op0)), UNSIGNED); + /* B = A - 1; if (B > A) -> B = A - 1; if (A == 0) + B = A - 1; if (B < A) -> B = A - 1; if (A != 0) */ + if (wi::eq_p (x, max - 1)) + { + op0 = op1; + op1 = wide_int_to_tree (TREE_TYPE (op0), 0); + code = (code == GT_EXPR || code == GE_EXPR) ? EQ_EXPR : NE_EXPR; + } + /* B = A + 1; if (B < A) -> B = A + 1; if (B == 0) + B = A + 1; if (B > A) -> B = A + 1; if (B != 0) */ + else if (integer_zerop (x)) + { + op1 = x; + code = (code == LT_EXPR || code == LE_EXPR) ? EQ_EXPR : NE_EXPR; + } +} + if ((ret = vrp_evaluate_conditional_warnv_with_ops_using
New French PO file for 'gcc' (version 7.1-b20170101)
Hello, gentle maintainer. This is a message from the Translation Project robot. A revised PO file for textual domain 'gcc' has been submitted by the French team of translators. The file is available at: http://translationproject.org/latest/gcc/fr.po (This file, 'gcc-7.1-b20170101.fr.po', has just now been sent to you in a separate email.) All other PO files for your package are available in: http://translationproject.org/latest/gcc/ Please consider including all of these in your next release, whether official or a pretest. Whenever you have a new distribution with a new version number ready, containing a newer POT file, please send the URL of that distribution tarball to the address below. The tarball may be just a pretest or a snapshot, it does not even have to compile. It is just used by the translators when they need some extra translation context. The following HTML page has been updated: http://translationproject.org/domain/gcc.html If any question arises, please contact the translation coordinator. Thank you for all your work, The Translation Project robot, in the name of your translation coordinator.
Re: [PATCH, Fortran, pr79230, v1] [7 Regression] [OOP] Run time error: double free or corruption
Hi Andre, This looks fine to me - OK for trunk. Thanks Paul On 4 February 2017 at 11:59, Andre Vehreschild wrote: > Hi all, > > attached patch fixes a regression when a polymorphic pointer component was > present. The results was a double free. The attached patch fixes this, by not > caring about freeing pointer components as part of gfortran's memory > management, > i.e., the programmer has to take care about freeing/disassociating the pointer > using a finalizer, as is IMO the intention of the Fortran standard. > > Bootstrapped and regtested ok on x86_64-linux/f25. Ok for trunk (current or > next, haven't monitored whether commits are still allowed)? > > Regards, > Andre > -- > Andre Vehreschild * Email: vehre ad gmx dot de -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [PATCH, Fortran, pr78958, v1] Unallocated memory access after SOURCE-ALLOCATEing unlimited polymorphic object
Hi Andre, This looks to be OK for trunk. Thanks for the patch. Paul On 4 February 2017 at 13:11, Andre Vehreschild wrote: > Hi all, > > attached patch takes the _len-component of unlimited polymorphic objects into > account, when source-ALLOCATEing an unlimited polymorphic object. The testcase > for this gcc/testsuite/gfortran.dg/alloc_comp_class_5.f03 with > valgrind/sanitizer. I therefore added no additional testcase. > > Bootstraps and regtests ok on x86_64-linux/f25 and as reported in the PR on > 32bit-hpux. Ok for trunk (when?)? > > Regards, > Andre > -- > Andre Vehreschild * Email: vehre ad gmx dot de -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [PATCH, Fortran, pr79335, v1] [7 Regression] Conditional jump or move depends on uninitialised in value get_scalar_to_descriptor_type(tree_node*, symbol_attribute) (trans-expr.c:53)
Hi Andre, This is getting to be a bit monotonous :-) OK for trunk. Thanks for the three patches. Paul PS Are you in a position to have a stab at PR79344? This would go a long way to sorting out Juergen's test suite. Also, this must be the third time that I have been involved in breaking iso_varying_string and I think that others have too :-( We must start running the testsuite on a regular basis. Unfortunately, it lacks a harness to run all the tests automatically. PPS I am some days away yet from recovering from my move from France to the UK and so would defer to you if you can come up with a fix. On 4 February 2017 at 13:39, Andre Vehreschild wrote: > Hi all, > > attached patch fixes the access of uninitialized data. Thanks Martin for > analyzing this so far. The symbol_attributes on the stack was used > without initializing it from the symbol's attributes. > > Bootstraps and regstests ok on x86_64-linux-gnu/f25. Ok for trunk? > > Regards, > Andre > -- > Andre Vehreschild * Email: vehre ad gmx dot de -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
[PATCH] avoid overwriting minimum length of unknown strings (PR 79376)
There's a "thinko" in the get_range_strlen() function that computes the range of possible string lengths for a character pointer that may point to an array holding a string of unknown length and a string literal. The bug lets the function return the length of the string as the lower bound (and the size of the array -1 as the upper bound), which isn't correct. The lower bound in this case must be zero because the array could be the empty string. The attached patch corrects this error. Martin gcc/ChangeLog: * gimple-fold.c (get_range_strlen): Set the minimum length to zero. gcc/testsuite/ChangeLog: * gcc.dg/tree-ssa/builtin-sprintf-warn-14.c: New test. * gcc.dg/tree-ssa/pr79376.c: New test. Index: gcc/gimple-fold.c === --- gcc/gimple-fold.c (revision 245172) +++ gcc/gimple-fold.c (working copy) @@ -1244,8 +1244,9 @@ get_range_strlen (tree arg, tree length[2], bitmap return false; val = fold_build2 (MINUS_EXPR, TREE_TYPE (val), val, integer_one_node); - /* Avoid using the array size as the minimum. */ - minlen = NULL; + /* Set the minimum size to zero since the string in + the array could have zero length. */ + *minlen = integer_zero_node; } } Index: gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-14.c === --- gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-14.c (revision 0) +++ gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-14.c (working copy) @@ -0,0 +1,210 @@ +/* PR middle-end/79376 - wrong lower bound with %s and non-constant + strings in -Wformat-overflow + { dg-do compile } + { dg-options "-O2 -Wall -Wformat-overflow=1 -ftrack-macro-expansion=0" } */ + +typedef __SIZE_TYPE__ size_t; + +#define INT_MAX __INT_MAX__ +#define INT_MIN (-INT_MAX - 1) + +/* When debugging, define LINE to the line number of the test case to exercise + and avoid exercising any of the others. The buffer and objsize macros + below make use of LINE to avoid warnings for other lines. */ +#ifndef LINE +# define LINE 0 +#endif + +extern int int_value (void); + +int int_range (int min, int max) +{ + int n = int_value (); + return n < min || max < n ? min : n; +} + +const char* +choose_string (const char *s1, const char *s2, const char *s3) +{ + int i = int_value (); + return i < 0 ? s1 : 0 < i ? s3 : s2; +} + +void sink (char*, char*); + +int dummy_sprintf (char*, const char*, ...); + +char buffer [256]; +extern char *ptr; + +const char s0[] = ""; +const char s1[] = "1"; +const char s2[] = "12"; +const char s3[] = "123"; +const char s4[] = "1234"; +const char s5[] = "12345"; +const char s6[] = "123456"; +const char s7[] = "1234567"; +const char s8[] = "12345678"; +const char s9[] = "123456789"; +extern const char sx[]; +extern const char sy[]; + +/* Evaluate to an array of SIZE characters when non-negative, or to + a pointer to an unknown object otherwise. */ +#define buffer(size) \ + ((0 <= size) ? buffer + sizeof buffer - (size) : ptr) + +/* Helper to expand function to either __builtin_f or dummy_f to + make debugging GCC easy. */ +#define FUNC(f) \ + ((!LINE || LINE == __LINE__) ? __builtin_ ## f : dummy_ ## f) + +/* Macro to verify that calls to __builtin_sprintf (i.e., with no size + argument) issue diagnostics by correctly determining the size of + the destination buffer. */ +#define T(size, ...) \ + (FUNC (sprintf) (buffer (size), __VA_ARGS__), \ + sink (buffer, ptr)) + +/* Return a value in the range [MIN, MAX]. */ +#define R(min, max) int_range (min, max) + +/* Return one of the strings S1, S2, S3. */ +#define S(s1, s2, s3) choose_string (s1, s2, s3) + +struct S { + char a1[1]; + char a2[2]; + char a3[3]; + char a4[4]; + char a5[5]; + char ax[]; +}; + +void test_strings (struct S *s) +{ + T (0, "%-s", S (s->a1, s->a1, s1)); /* { dg-warning "writing up to 1 byte" } */ + T (0, "%-s", S (s->a1, s->a2, s1)); /* { dg-warning "writing up to 1 byte" } */ + T (0, "%-s", S (s->a2, s->a1, s1)); /* { dg-warning "writing up to 1 byte" } */ + + T (0, "%-s", S (s->a1, s1, s->a1)); /* { dg-warning "writing up to 1 byte" } */ + T (0, "%-s", S (s->a1, s1, s->a2)); /* { dg-warning "writing up to 1 byte" } */ + T (0, "%-s", S (s->a2, s1, s->a1)); /* { dg-warning "writing up to 1 byte" } */ + + T (0, "%-s", S (s1, s->a1, s1)); /* { dg-warning "writing up to 1 byte" } */ + T (0, "%-s", S (s1, s->a1, s1)); /* { dg-warning "writing up to 1 byte" } */ + T (0, "%-s", S (s1, s->a2, s1)); /* { dg-warning "writing up to 1 byte" } */ + + T (0, "%-s", S (s->a1, s->a1, s2)); /* { dg-warning "writing up to 2 bytes" } */ + T (0, "%-s", S (s->a1, s->a2, s2)); /* { dg-warning "writing up to 2 bytes" } */ + T (0, "%-s", S (s->a2, s->a1, s2)); /* { dg-warning "writing up to 2 bytes" } */ + + T (0, "%-s", S (s->a1, s2, s->a1)); /* { dg-warning "writing up t
Re: [PATCH] Fix improper combiner optimization of mixed mode vector/scalar expressions (PR 79365)
Thanks Segher. I will address your comments and repost for GCC 8. > What is this for? It isn't triggered by the testcase in the PR. It was triggered by our strcmp code, but I didn't have a simple test case for it. I will try to generate one. Walter
Re: [PATCH, Fortran, pr79335, v1] [7 Regression] Conditional jump or move depends on uninitialised in value get_scalar_to_descriptor_type(tree_node*, symbol_attribute) (trans-expr.c:53)
Hi Paul, > PS Are you in a position to have a stab at PR79344? This would go a > long way to sorting out Juergen's test suite. Also, this must be the > third time that I have been involved in breaking iso_varying_string > and I think that others have too :-( We must start running the > testsuite on a regular basis. Unfortunately, it lacks a harness to run > all the tests automatically. Currently executing the testsuite with a patched compiler. > PPS I am some days away yet from recovering from my move from France > to the UK and so would defer to you if you can come up with a fix. Fix present, waiting for testsuite to pass. - Andre > > On 4 February 2017 at 13:39, Andre Vehreschild wrote: > > Hi all, > > > > attached patch fixes the access of uninitialized data. Thanks Martin for > > analyzing this so far. The symbol_attributes on the stack was used > > without initializing it from the symbol's attributes. > > > > Bootstraps and regstests ok on x86_64-linux-gnu/f25. Ok for trunk? > > > > Regards, > > Andre > > -- > > Andre Vehreschild * Email: vehre ad gmx dot de > > > -- Andre Vehreschild * Kreuzherrenstr. 8 * 52062 Aachen Tel.: +49 241 9291018 * Email: ve...@gmx.de
Re: [PATCH] Fix __atomic to not implement atomic loads with CAS.
On Sat, Feb 04, 2017 at 03:28:42PM +0100, Dominique d'Humières wrote: > > This patch fixes the __atomic builtins to not implement supposedly > > lock-free atomic loads based on just a compare-and-swap operation. > > … > > Commit r245098 caused > > New failures: > FAIL: libgomp.c/atomic-2.c (test for excess errors) > FAIL: libgomp.c/atomic-4.c (test for excess errors) > FAIL: libgomp.c/atomic-5.c (test for excess errors) > FAIL: libgomp.c/pr48591.c (test for excess errors) > FAIL: libgomp.c/pr64868.c (test for excess errors) > FAIL: libgomp.c++/pr64868.C (test for excess errors) > > (see https://gcc.gnu.org/ml/gcc-regression/2017-02/msg00011.html). The error > is > > Undefined symbols for architecture x86_64: > "___atomic_load_16", referenced from: Known, on my todo list. Jakub
[PATCH, Fortran, pr79344, v1] [7 Regression] segmentation faults and run-time errors
Hi all, attached patch fixes the issue of losing the data in the SOURCE= expression of an ALLOCATE() when the source-expression is just a simple variable. The issue was that internally a temporary variable was created, whose components were freed afterwards. Now the components are only freed on temporary objects, i.e., when the source-expression is not an EXPR_VARIABLE, e.g. an EXPR_STRUCTURE or EXPR_FUNCTION. Bootstraps and regtests ok on x86_64-linux/f25. Ok for trunk? Regards, Andre -- Andre Vehreschild * Email: vehre ad gmx dot de gcc/fortran/ChangeLog: 2017-02-04 Andre Vehreschild PR fortran/79344 * trans-stmt.c (gfc_trans_allocate): Only deallocate the components of the temporary, when a new object was created for the temporary. Not when it is just an alias to an existing object. gcc/testsuite/ChangeLog: 2017-02-04 Andre Vehreschild * gfortran.dg/allocate_with_source_24.f90: New test. diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c index 61e597f..773ca70 100644 --- a/gcc/fortran/trans-stmt.c +++ b/gcc/fortran/trans-stmt.c @@ -5572,7 +5572,8 @@ gfc_trans_allocate (gfc_code * code) expression. */ if (code->expr3) { - bool vtab_needed = false, temp_var_needed = false; + bool vtab_needed = false, temp_var_needed = false, + temp_obj_created = false; is_coarray = gfc_is_coarray (code->expr3); @@ -5645,7 +5646,7 @@ gfc_trans_allocate (gfc_code * code) code->expr3->ts, false, true, false, false); - temp_var_needed = !VAR_P (se.expr); + temp_obj_created = temp_var_needed = !VAR_P (se.expr); } gfc_add_block_to_block (&block, &se.pre); gfc_add_block_to_block (&post, &se.post); @@ -5714,11 +5715,12 @@ gfc_trans_allocate (gfc_code * code) } /* Deallocate any allocatable components in expressions that use a - temporary, i.e. are not of expr-type EXPR_VARIABLE or force the - use of a temporary, after the assignment of expr3 is completed. */ + temporary object, i.e. are not a simple alias of to an EXPR_VARIABLE. + E.g. temporaries of a function call need freeing of their components + here. */ if ((code->expr3->ts.type == BT_DERIVED || code->expr3->ts.type == BT_CLASS) - && (code->expr3->expr_type != EXPR_VARIABLE || temp_var_needed) + && (code->expr3->expr_type != EXPR_VARIABLE || temp_obj_created) && code->expr3->ts.u.derived->attr.alloc_comp) { tmp = gfc_deallocate_alloc_comp (code->expr3->ts.u.derived, diff --git a/gcc/testsuite/gfortran.dg/allocate_with_source_24.f90 b/gcc/testsuite/gfortran.dg/allocate_with_source_24.f90 new file mode 100644 index 000..ec11d7a --- /dev/null +++ b/gcc/testsuite/gfortran.dg/allocate_with_source_24.f90 @@ -0,0 +1,134 @@ +! { dg-do run } +! +! Test that the temporary in a sourced-ALLOCATE is not freeed. +! PR fortran/79344 +! Contributed by Juergen Reuter + +module iso_varying_string + implicit none + + type, public :: varying_string + private + character(LEN=1), dimension(:), allocatable :: chars + end type varying_string + + interface assignment(=) + module procedure op_assign_VS_CH + end interface assignment(=) + + interface operator(/=) + module procedure op_not_equal_VS_CA + end interface operator(/=) + + interface len + module procedure len_ + end interface len + + interface var_str + module procedure var_str_ + end interface var_str + + public :: assignment(=) + public :: operator(/=) + public :: len + + private :: op_assign_VS_CH + private :: op_not_equal_VS_CA + private :: char_auto + private :: len_ + private :: var_str_ + +contains + + elemental function len_ (string) result (length) +type(varying_string), intent(in) :: string +integer :: length +if(ALLOCATED(string%chars)) then + length = SIZE(string%chars) +else + length = 0 +endif + end function len_ + + elemental subroutine op_assign_VS_CH (var, exp) +type(varying_string), intent(out) :: var +character(LEN=*), intent(in) :: exp +var = var_str(exp) + end subroutine op_assign_VS_CH + + pure function op_not_equal_VS_CA (var, exp) result(res) +type(varying_string), intent(in) :: var +character(LEN=*), intent(in) :: exp +logical :: res +integer :: i +res = .true. +if (len(exp) /= size(var%chars)) return +do i = 1, size(var%chars) + if (var%chars(i) /= exp(i:i)) return +end do +res = .false. + end function op_not_equal_VS_CA + + pure function char_auto (string) result (char_string) +type(varying_string), intent(in) :: string +character(LEN=len(string)) :: char_string +integer :: i_char +forall(i_char = 1:len(string)) + char_string(i_char:i_char) = string%chars(i_char) +end forall + end function char_auto + + elemental function var_str_ (char) result (string) +character(LEN=*), in
Re: [Patch][ARM,AArch64] more poly64 intrinsics and tests
On Wed, Jan 11, 2017 at 11:13:07AM +0100, Christophe Lyon wrote: > Ping? > > James, I'm not sure whether your comment was a request for a new > version of my patch or just FYI? Sorry that this was unclear. I was looking for a new version of the patch covering this comment. Otherwise we just have debt to go fix it in future. With the suggested change, the AArch64 parts of this patch are OK - adding missing intrinsics is very safe (even in Stage 4). Please post an updated patch, and give Richard and Marcus a reasonable amount of tiume to object to taking the patch this late. (and you need an AArch32 OK too). Thanks, James > > > On 3 January 2017 at 16:47, Christophe Lyon > wrote: > > Ping? > > > > > > On 14 December 2016 at 23:09, Christophe Lyon > > wrote: > >> On 14 December 2016 at 17:55, James Greenhalgh > >> wrote: > >>> On Mon, Dec 12, 2016 at 05:03:31PM +0100, Christophe Lyon wrote: > Hi, > > After the recent update from Tamar, I noticed a few discrepancies > between ARM and AArch64 regarding a few poly64 intrinsics. > > This patch: > - adds vtst_p64 and vtstq_p64 to AArch64's arm_neon.h > - adds vgetq_lane_p64, vset_lane_p64 and vsetq_lane_p64 to ARM's > arm_neon.h > ( vget_lane_p64 was already there) > - adds the corresponding tests, and moves the vget_lane_p64 ones out > of the #ifdef __aarch64__ zone. > > Cross-tested on arm* and aarch64* targets. > > OK? > >>> > >>> The AArch64 parts of this look fine to me, but I do have one question on > >>> your inline assembly implementation for vtstq_p64: > >>> > +__extension__ extern __inline uint64x2_t > +__attribute__ ((__always_inline__, __gnu_inline__, __artificial__)) > +vtstq_p64 (poly64x2_t a, poly64x2_t b) > +{ > + uint64x2_t result; > + __asm__ ("cmtst %0.2d, %1.2d, %2.2d" > + : "=w"(result) > + : "w"(a), "w"(b) > + : /* No clobbers */); > + return result; > +} > + > >>> > >>> Why can this not be written as many of the other vtstq intrinsics are; > >>> e.g.: > >>> > >>>__extension__ extern __inline uint64x2_t > >>> __attribute__ ((__always_inline__, __gnu_inline__, __artificial__)) > >>> vtstq_p64 (poly64x2_t __a, poly64x2_t __b) > >>> { > >>> return (uint64x2_t) uint64x2_t) __a) & ((uint64x2_t) __b)) > >>> != __AARCH64_INT64_C (0)); > >>> } > >>> > >> > >> I don't know, I just followed the pattern used for vtstq_p8 and vtstq_p16 > >> just above... > >> > >> > >>> Thanks, > >>> James > >>> > gcc/ChangeLog: > > 2016-12-12 Christophe Lyon > > * config/aarch64/arm_neon.h (vtst_p64): New. > (vtstq_p64): New. > * config/arm/arm_neon.h (vgetq_lane_p64): New. > (vset_lane_p64): New. > (vsetq_lane_p64): New. > > gcc/testsuite/ChangeLog: > > 2016-12-12 Christophe Lyon > > * gcc.target/aarch64/advsimd-intrinsics/p64_p128.c > (vget_lane_expected, vset_lane_expected, vtst_expected_poly64x1): > New. > (vmov_n_expected0, vmov_n_expected1, vmov_n_expected2) > (expected_vld_st2_0, expected_vld_st2_1, expected_vld_st3_0) > (expected_vld_st3_1, expected_vld_st3_2, expected_vld_st4_0) > (expected_vld_st4_1, expected_vld_st4_2, expected_vld_st4_3) > (vtst_expected_poly64x2): Move to aarch64-only section. > (vget_lane_p64, vgetq_lane_p64, vset_lane_p64, vsetq_lane_p64) > (vtst_p64, vtstq_p64): New tests. > > >>> > >>>