[PATCH] Remove __gcov_merge_delta (PR bootstrap/77749)
Hello.
Following patch removes forgotten hunks connected to removal of
__gcov_merge_delta counter.
I'm running make profiledbootstrap, ready to install the patch after it
finishes?
Thanks,
Martin
>From e0123fe1cefe5f5cced239c36ebccd53c38feb25 Mon Sep 17 00:00:00 2001
From: marxin
Date: Tue, 27 Sep 2016 09:38:58 +0200
Subject: [PATCH] Remove __gcov_merge_delta (PR bootstrap/77749)
gcc/ChangeLog:
2016-09-27 Martin Liska
* gcov-counter.def: Remove GCOV_COUNTER_V_DELTA.
libgcc/ChangeLog:
2016-09-27 Martin Liska
* Makefile.in: Remove _gcov_merge_delta.
* libgcov-merge.c (void __gcov_merge_delta): Remove.
* libgcov-util.c (__gcov_delta_counter_op): Remove.
* libgcov.h: Remove declaration of __gcov_merge_delta.
---
gcc/gcov-counter.def | 3 ---
libgcc/Makefile.in | 2 +-
libgcc/libgcov-merge.c | 45 -
libgcc/libgcov-util.c | 17 -
libgcc/libgcov.h | 5 -
5 files changed, 1 insertion(+), 71 deletions(-)
diff --git a/gcc/gcov-counter.def b/gcc/gcov-counter.def
index 64c2673..55d3f0c 100644
--- a/gcc/gcov-counter.def
+++ b/gcc/gcov-counter.def
@@ -38,9 +38,6 @@ DEF_GCOV_COUNTER(GCOV_COUNTER_V_POW2, "pow2", _add)
/* The most common value of expression. */
DEF_GCOV_COUNTER(GCOV_COUNTER_V_SINGLE, "single", _single)
-/* The most common difference between consecutive values of expression. */
-DEF_GCOV_COUNTER(GCOV_COUNTER_V_DELTA, "delta", _delta)
-
/* The most common indirect address. */
DEF_GCOV_COUNTER(GCOV_COUNTER_V_INDIR, "indirect_call", _single)
diff --git a/libgcc/Makefile.in b/libgcc/Makefile.in
index ff3c77f..873cad0 100644
--- a/libgcc/Makefile.in
+++ b/libgcc/Makefile.in
@@ -877,7 +877,7 @@ include $(iterator)
# Build libgcov components.
-LIBGCOV_MERGE = _gcov_merge_add _gcov_merge_single _gcov_merge_delta \
+LIBGCOV_MERGE = _gcov_merge_add _gcov_merge_single \
_gcov_merge_ior _gcov_merge_time_profile _gcov_merge_icall_topn
LIBGCOV_PROFILER = _gcov_interval_profiler\
_gcov_interval_profiler_atomic \
diff --git a/libgcc/libgcov-merge.c b/libgcc/libgcov-merge.c
index 3a8bb2c..fd2f074 100644
--- a/libgcc/libgcov-merge.c
+++ b/libgcc/libgcov-merge.c
@@ -38,11 +38,6 @@ void __gcov_merge_single (gcov_type *counters __attribute__ ((unused)),
unsigned n_counters __attribute__ ((unused))) {}
#endif
-#ifdef L_gcov_merge_delta
-void __gcov_merge_delta (gcov_type *counters __attribute__ ((unused)),
- unsigned n_counters __attribute__ ((unused))) {}
-#endif
-
#else
#ifdef L_gcov_merge_add
@@ -127,46 +122,6 @@ __gcov_merge_single (gcov_type *counters, unsigned n_counters)
}
#endif /* L_gcov_merge_single */
-#ifdef L_gcov_merge_delta
-/* The profile merging function for choosing the most common
- difference between two consecutive evaluations of the value. It is
- given an array COUNTERS of N_COUNTERS old counters and it reads the
- same number of counters from the gcov file. The counters are split
- into 4-tuples where the members of the tuple have meanings:
-
- -- the last value of the measured entity
- -- the stored candidate on the most common difference
- -- counter
- -- total number of evaluations of the value */
-void
-__gcov_merge_delta (gcov_type *counters, unsigned n_counters)
-{
- unsigned i, n_measures;
- gcov_type value, counter, all;
-
- gcc_assert (!(n_counters % 4));
- n_measures = n_counters / 4;
- for (i = 0; i < n_measures; i++, counters += 4)
-{
- /* last = */ gcov_get_counter ();
- value = gcov_get_counter_target ();
- counter = gcov_get_counter ();
- all = gcov_get_counter ();
-
- if (counters[1] == value)
-counters[2] += counter;
- else if (counter > counters[2])
-{
- counters[1] = value;
- counters[2] = counter - counters[2];
-}
- else
-counters[2] -= counter;
- counters[3] += all;
-}
-}
-#endif /* L_gcov_merge_delta */
-
#ifdef L_gcov_merge_icall_topn
/* The profile merging function used for merging indirect call counts
This function is given array COUNTERS of N_COUNTERS old counters and it
diff --git a/libgcc/libgcov-util.c b/libgcc/libgcov-util.c
index 24ee50e..b1d966c 100644
--- a/libgcc/libgcov-util.c
+++ b/libgcc/libgcov-util.c
@@ -720,23 +720,6 @@ __gcov_time_profile_counter_op (gcov_type *counters ATTRIBUTE_UNUSED,
/* Do nothing. */
}
-/* Performaing FN upon delta counters. */
-
-static void
-__gcov_delta_counter_op (gcov_type *counters, unsigned n_counters,
- counter_op_fn fn, void *data1, void *data2)
-{
- unsigned i, n_measures;
-
- gcc_assert (!(n_counters % 4));
- n_measures = n_counters / 4;
- for (i = 0; i < n_measures; i++, counters += 4)
-{
- counters[2] = fn (counters[2], data1, data2);
- counters[3] = fn (counters[3], data1, data2);
-}
-}
-
/* Performing FN upon single counters. */
static void
diff --gi
Re: [PATCH GCC][v2]Simplify alias check code generation in vectorizer
On Mon, Sep 26, 2016 at 8:15 PM, Richard Biener wrote: > On September 26, 2016 5:46:28 PM GMT+02:00, Bernd Edlinger > wrote: >>Hi, >> >>>@@ -2310,7 +2313,8 @@ create_intersect_range_checks_index >>(loop_vec_info loop_vinfo, tree *cond_expr, >>> gcc_assert (TREE_CODE (DR_STEP (dr_a.dr)) == INTEGER_CST); >>> >>> bool neg_step = tree_int_cst_compare (DR_STEP (dr_a.dr), >>size_zero_node) < 0; >>>- unsigned HOST_WIDE_INT abs_step = tree_to_uhwi (DR_STEP (dr_a.dr)); >>>+ unsigned HOST_WIDE_INT abs_step = >>>+absu_hwi (tree_to_shwi (DR_STEP (dr_a.dr))); >>> if (neg_step) >>> abs_step = -abs_step; >> >>Hmmm... >> >>but you must remove the neg_step if you do this. > > The negation, yes. Neg_step is used later as well. Also the '=' in the split line goes to the next line according to coding conventions. Richard. > Richard. > >>Right? >> >> >>Bernd. > >
Re: Implement -Wimplicit-fallthrough (version 9)
Hi Marek, On Sat, 24 Sep 2016, Marek Polacek wrote: > All right. I'll commit the patch on Monday. my thrice a week bootstrap on old (but still "supported") i?86-unknown-freebsd9 broke as follows: cc1plus: error: unrecognized command line option "-Wno-implicit-fallthrough" gmake[3]: *** [Makefile:1102: insn-emit.o] Error 1 gmake[3]: *** Waiting for unfinished jobs gmake[3]: Leaving directory '/scratch/tmp/gerald/OBJ-0927-0731/gcc' gmake[2]: *** [Makefile:4571: all-stage1-gcc] Error 2 gmake[2]: Leaving directory '/scratch/tmp/gerald/OBJ-0927-0731' gmake[1]: *** [Makefile:24240: stage1-bubble] Error 2 (Also filed as https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77751 ) Gerald
[committed] Fix OpenMP ICE with private allocatable array dummy argument (PR fortran/77666)
Hi!
The following testcase ICEs, because we need the outer reference for the
private clause to find out if it is allocated or not and the dimensions,
but while it is provided e.g. for automatic array allocatables or all scalar
allocatables, it isn't provided for dummy array allocatable arguments.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
committed to trunk so far.
2016-09-27 Jakub Jelinek
PR fortran/77666
* trans-openmp.c (gfc_omp_private_outer_ref): Return true even for
references to allocatable arrays.
* gfortran.dg/gomp/pr77666.f90: New test.
--- gcc/fortran/trans-openmp.c.jj 2016-09-13 10:43:58.0 +0200
+++ gcc/fortran/trans-openmp.c 2016-09-26 16:05:33.561074532 +0200
@@ -207,6 +207,9 @@ gfc_omp_private_outer_ref (tree decl)
{
tree type = TREE_TYPE (decl);
+ if (gfc_omp_privatize_by_reference (decl))
+type = TREE_TYPE (type);
+
if (GFC_DESCRIPTOR_TYPE_P (type)
&& GFC_TYPE_ARRAY_AKIND (type) == GFC_ARRAY_ALLOCATABLE)
return true;
@@ -214,9 +217,6 @@ gfc_omp_private_outer_ref (tree decl)
if (GFC_DECL_GET_SCALAR_ALLOCATABLE (decl))
return true;
- if (gfc_omp_privatize_by_reference (decl))
-type = TREE_TYPE (type);
-
if (gfc_has_alloc_comps (type, decl))
return true;
--- gcc/testsuite/gfortran.dg/gomp/pr77666.f90.jj 2016-09-26
16:36:19.548421888 +0200
+++ gcc/testsuite/gfortran.dg/gomp/pr77666.f90 2016-09-26 16:35:56.0
+0200
@@ -0,0 +1,26 @@
+! PR fortran/77666
+! { dg-do compile }
+
+subroutine foo(x)
+ interface
+subroutine baz(x, y)
+ integer, allocatable :: x(:), y
+end subroutine
+ end interface
+ integer, allocatable :: x(:), y
+!$omp parallel private(x, y)
+ call baz (x, y)
+!$omp end parallel
+end
+subroutine bar
+ interface
+subroutine baz(x, y)
+ integer, allocatable :: x(:), y
+end subroutine
+ end interface
+ integer, allocatable :: x(:), y
+ call baz (x, y)
+!$omp parallel private(x, y)
+ call baz (x, y)
+!$omp end parallel
+end
Jakub
Re: Implement -Wimplicit-fallthrough (version 9)
> Seconded. The warning should take into account existing practices instead > of forcing the user to make completely bogus changes to the code (and Ada > should have been tested before the patch was approved). I have a bootstrap failure on x86-64/Linux: /home/eric/svn/gcc/gcc/combine.c: In function 'rtx_code simplify_comparison(rtx_code, rtx_def**, rtx_def**)': /home/eric/svn/gcc/gcc/combine.c:11928:11: error: this statement may fall through [-Werror=implicit-fallthrough] break; ^ /home/eric/svn/gcc/gcc/combine.c:11932:2: note: here case ZERO_EXTEND: ^~~~ /home/eric/svn/gcc/gcc/combine.c:12340:6: error: this statement may fall through [-Werror=implicit-fallthrough] } ^ /home/eric/svn/gcc/gcc/combine.c:12343:2: note: here case LSHIFTRT: ^~~~ /* ... fall through ... */ /* ... fall through ... */ -- Eric Botcazou
[C++ PATCH] Fix -fsanitize=return on small functions (PR c++/77722)
Hi!
As the testcases show, we actually instrument -fsanitize=return only if
the DECL_SAVED_TREE (fndecl) is a BIND_EXPR with STATEMENT_LIST as its
BIND_EXPR_BODY (plus tests that it actually needs such an instrumentation).
The return-{4,5}.C tests show that it isn't always the case, sometimes we
have just a STATEMENT_LIST without BIND_EXPR around it (for empty
functions), sometimes BIND_EXPR_BODY is a single statement, not a
STATEMENT_LIST.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?
2016-09-27 Jakub Jelinek
PR c++/77722
* cp-gimplify.c (cp_ubsan_maybe_instrument_return): Instrument also
functions that have just a STATEMENT_LIST instead of BIND_EXPR, or
BIND_EXPR with some statement rather than STATEMENT_LIST as body.
* g++.dg/ubsan/return-4.C: New test.
* g++.dg/ubsan/return-5.C: New test.
* g++.dg/ubsan/return-6.C: New test.
--- gcc/cp/cp-gimplify.c.jj 2016-08-12 17:33:42.0 +0200
+++ gcc/cp/cp-gimplify.c2016-09-26 09:37:30.679543731 +0200
@@ -1570,14 +1570,22 @@ cp_ubsan_maybe_instrument_return (tree f
}
if (t == NULL_TREE)
return;
- t = DECL_SAVED_TREE (fndecl);
- if (TREE_CODE (t) == BIND_EXPR
- && TREE_CODE (BIND_EXPR_BODY (t)) == STATEMENT_LIST)
+ tree *p = &DECL_SAVED_TREE (fndecl);
+ if (TREE_CODE (*p) == BIND_EXPR)
+p = &BIND_EXPR_BODY (*p);
+ t = ubsan_instrument_return (DECL_SOURCE_LOCATION (fndecl));
+ if (TREE_CODE (*p) == STATEMENT_LIST)
{
- tree_stmt_iterator i = tsi_last (BIND_EXPR_BODY (t));
- t = ubsan_instrument_return (DECL_SOURCE_LOCATION (fndecl));
+ tree_stmt_iterator i = tsi_last (*p);
tsi_link_after (&i, t, TSI_NEW_STMT);
}
+ else
+{
+ tree list = NULL_TREE;
+ append_to_statement_list_force (*p, &list);
+ append_to_statement_list (t, &list);
+ *p = list;
+}
}
void
--- gcc/testsuite/g++.dg/ubsan/return-4.C.jj2016-09-26 09:42:45.306503657
+0200
+++ gcc/testsuite/g++.dg/ubsan/return-4.C 2016-09-26 09:54:08.778728718
+0200
@@ -0,0 +1,18 @@
+// PR c++/77722
+// { dg-do run }
+// { dg-options "-fsanitize=return -w" }
+// { dg-shouldfail "ubsan" }
+
+int
+foo ()
+{
+}
+
+int
+main ()
+{
+ foo ();
+ return 0;
+}
+
+// { dg-output "execution reached the end of a value-returning function
without returning a value" }
--- gcc/testsuite/g++.dg/ubsan/return-5.C.jj2016-09-26 09:53:37.135134983
+0200
+++ gcc/testsuite/g++.dg/ubsan/return-5.C 2016-09-26 09:54:03.273799395
+0200
@@ -0,0 +1,19 @@
+// PR c++/77722
+// { dg-do run }
+// { dg-options "-fsanitize=return -w" }
+// { dg-shouldfail "ubsan" }
+
+int
+foo ()
+{
+ int a = 5;
+}
+
+int
+main ()
+{
+ foo ();
+ return 0;
+}
+
+// { dg-output "execution reached the end of a value-returning function
without returning a value" }
--- gcc/testsuite/g++.dg/ubsan/return-6.C.jj2016-09-26 09:53:40.024097892
+0200
+++ gcc/testsuite/g++.dg/ubsan/return-6.C 2016-09-26 09:54:14.327657477
+0200
@@ -0,0 +1,20 @@
+// PR c++/77722
+// { dg-do run }
+// { dg-options "-fsanitize=return -w" }
+// { dg-shouldfail "ubsan" }
+
+int
+foo ()
+{
+ int a = 5;
+ int b = 5;
+}
+
+int
+main ()
+{
+ foo ();
+ return 0;
+}
+
+// { dg-output "execution reached the end of a value-returning function
without returning a value" }
Jakub
Fix bootstrap failure in combine.c and i386.c due to -Wimplicit-fallthrough
On Tue, Sep 27, 2016 at 09:55:24AM +0200, Eric Botcazou wrote: > > Seconded. The warning should take into account existing practices instead > > of forcing the user to make completely bogus changes to the code (and Ada > > should have been tested before the patch was approved). > > I have a bootstrap failure on x86-64/Linux: > > /home/eric/svn/gcc/gcc/combine.c: In function 'rtx_code > simplify_comparison(rtx_code, rtx_def**, rtx_def**)': > /home/eric/svn/gcc/gcc/combine.c:11928:11: error: this statement may fall > through [-Werror=implicit-fallthrough] > break; >^ > /home/eric/svn/gcc/gcc/combine.c:11932:2: note: here > case ZERO_EXTEND: > ^~~~ > /home/eric/svn/gcc/gcc/combine.c:12340:6: error: this statement may fall > through [-Werror=implicit-fallthrough] > } > ^ > /home/eric/svn/gcc/gcc/combine.c:12343:2: note: here > case LSHIFTRT: > ^~~~ > > /* ... fall through ... */ > > /* ... fall through ... */ > Yeah, I ran into this and another issue in i386.c. Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2016-09-26 Jakub Jelinek * combine.c (simplify_comparison): Add canonical FALLTHROUGH comments. * config/i386/i386.c (ix86_dep_by_shift_count_body): Add FALLTHROUGH comments. Remove break after return. (ix86_fp_compare_code_to_integer, has_dispatch, ix86_simd_clone_usable): Remove break after return. --- gcc/combine.c.jj2016-09-19 16:33:57.0 +0200 +++ gcc/combine.c 2016-09-26 22:15:39.553198973 +0200 @@ -11923,11 +11923,11 @@ simplify_comparison (enum rtx_code code, we can treat the SUBREG as if it were a ZERO_EXTEND. */ if (subreg_lowpart_p (op0) && GET_MODE_PRECISION (GET_MODE (SUBREG_REG (op0))) < mode_width) - /* Fall through */ ; + ; else break; - /* ... fall through ... */ + /* FALLTHROUGH */ case ZERO_EXTEND: mode = GET_MODE (XEXP (op0, 0)); @@ -12339,7 +12339,7 @@ simplify_comparison (enum rtx_code code, continue; } - /* ... fall through ... */ + /* FALLTHROUGH */ case LSHIFTRT: /* If we have (compare (xshiftrt FOO N) (const_int C)) and the low order N bits of FOO are known to be zero, we can do this --- gcc/config/i386/i386.c.jj 2016-09-26 20:22:23.0 +0200 +++ gcc/config/i386/i386.c 2016-09-26 23:15:46.988142242 +0200 @@ -21237,9 +21237,9 @@ ix86_dep_by_shift_count_body (const_rtx if (ix86_dep_by_shift_count_body (XVECEXP (set_body, 0, i), use_body)) return true; + /* FALLTHROUGH */ default: return false; - break; } /* Retrieve shift count of USE_BODY. */ @@ -21253,9 +21253,9 @@ ix86_dep_by_shift_count_body (const_rtx if (ix86_dep_by_shift_count_body (set_body, XVECEXP (use_body, 0, i))) return true; + /* FALLTHROUGH */ default: return false; - break; } if (shift_rtx @@ -22369,19 +22369,14 @@ ix86_fp_compare_code_to_integer (enum rt case ORDERED: case UNORDERED: return code; - break; case UNEQ: return EQ; - break; case UNLT: return LTU; - break; case UNLE: return LEU; - break; case LTGT: return NE; - break; default: return UNKNOWN; } @@ -49500,7 +49495,6 @@ has_dispatch (rtx_insn *insn, int action case IS_DISPATCH_ON: return true; - break; case IS_CMP: return is_cmp (insn); @@ -49958,7 +49952,6 @@ ix86_simd_clone_usable (struct cgraph_no if (!TARGET_AVX) return -1; return TARGET_AVX2 ? 1 : 0; - break; case 'd': if (!TARGET_AVX2) return -1; Jakub
Re: [PATCH] Remove __gcov_merge_delta (PR bootstrap/77749)
On Tue, Sep 27, 2016 at 9:46 AM, Martin Liška wrote: > Hello. > > Following patch removes forgotten hunks connected to removal of > __gcov_merge_delta counter. > I'm running make profiledbootstrap, ready to install the patch after it > finishes? Ok. Richard. > Thanks, > Martin
Re: Fix bootstrap failure in combine.c and i386.c due to -Wimplicit-fallthrough
On Tue, Sep 27, 2016 at 10:01 AM, Jakub Jelinek wrote: > On Tue, Sep 27, 2016 at 09:55:24AM +0200, Eric Botcazou wrote: >> > Seconded. The warning should take into account existing practices instead >> > of forcing the user to make completely bogus changes to the code (and Ada >> > should have been tested before the patch was approved). >> >> I have a bootstrap failure on x86-64/Linux: >> >> /home/eric/svn/gcc/gcc/combine.c: In function 'rtx_code >> simplify_comparison(rtx_code, rtx_def**, rtx_def**)': >> /home/eric/svn/gcc/gcc/combine.c:11928:11: error: this statement may fall >> through [-Werror=implicit-fallthrough] >> break; >>^ >> /home/eric/svn/gcc/gcc/combine.c:11932:2: note: here >> case ZERO_EXTEND: >> ^~~~ >> /home/eric/svn/gcc/gcc/combine.c:12340:6: error: this statement may fall >> through [-Werror=implicit-fallthrough] >> } >> ^ >> /home/eric/svn/gcc/gcc/combine.c:12343:2: note: here >> case LSHIFTRT: >> ^~~~ >> >> /* ... fall through ... */ >> >> /* ... fall through ... */ >> > > Yeah, I ran into this and another issue in i386.c. Fixed thusly, > bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Ok. Richard. > 2016-09-26 Jakub Jelinek > > * combine.c (simplify_comparison): Add canonical FALLTHROUGH comments. > * config/i386/i386.c (ix86_dep_by_shift_count_body): Add FALLTHROUGH > comments. Remove break after return. > (ix86_fp_compare_code_to_integer, has_dispatch, > ix86_simd_clone_usable): Remove break after return. > > --- gcc/combine.c.jj2016-09-19 16:33:57.0 +0200 > +++ gcc/combine.c 2016-09-26 22:15:39.553198973 +0200 > @@ -11923,11 +11923,11 @@ simplify_comparison (enum rtx_code code, > we can treat the SUBREG as if it were a ZERO_EXTEND. */ > if (subreg_lowpart_p (op0) > && GET_MODE_PRECISION (GET_MODE (SUBREG_REG (op0))) < > mode_width) > - /* Fall through */ ; > + ; > else > break; > > - /* ... fall through ... */ > + /* FALLTHROUGH */ > > case ZERO_EXTEND: > mode = GET_MODE (XEXP (op0, 0)); > @@ -12339,7 +12339,7 @@ simplify_comparison (enum rtx_code code, > continue; > } > > - /* ... fall through ... */ > + /* FALLTHROUGH */ > case LSHIFTRT: > /* If we have (compare (xshiftrt FOO N) (const_int C)) and > the low order N bits of FOO are known to be zero, we can do this > --- gcc/config/i386/i386.c.jj 2016-09-26 20:22:23.0 +0200 > +++ gcc/config/i386/i386.c 2016-09-26 23:15:46.988142242 +0200 > @@ -21237,9 +21237,9 @@ ix86_dep_by_shift_count_body (const_rtx > if (ix86_dep_by_shift_count_body (XVECEXP (set_body, 0, i), > use_body)) > return true; > + /* FALLTHROUGH */ > default: >return false; > - break; > } > >/* Retrieve shift count of USE_BODY. */ > @@ -21253,9 +21253,9 @@ ix86_dep_by_shift_count_body (const_rtx > if (ix86_dep_by_shift_count_body (set_body, > XVECEXP (use_body, 0, i))) > return true; > + /* FALLTHROUGH */ > default: >return false; > - break; > } > >if (shift_rtx > @@ -22369,19 +22369,14 @@ ix86_fp_compare_code_to_integer (enum rt > case ORDERED: > case UNORDERED: >return code; > - break; > case UNEQ: >return EQ; > - break; > case UNLT: >return LTU; > - break; > case UNLE: >return LEU; > - break; > case LTGT: >return NE; > - break; > default: >return UNKNOWN; > } > @@ -49500,7 +49495,6 @@ has_dispatch (rtx_insn *insn, int action > >case IS_DISPATCH_ON: > return true; > - break; > >case IS_CMP: > return is_cmp (insn); > @@ -49958,7 +49952,6 @@ ix86_simd_clone_usable (struct cgraph_no >if (!TARGET_AVX) > return -1; >return TARGET_AVX2 ? 1 : 0; > - break; > case 'd': >if (!TARGET_AVX2) > return -1; > > > Jakub
Re: Implement -Wimplicit-fallthrough (version 9)
This breaks building with gcc-4.3. g++ -std=gnu++98 -fno-PIE -c -DUSE_LIBUNWIND_EXCEPTIONS -g -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wwrite-strings -Wcast-qual -Wno-format -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Wno-implicit-fallthrough -DHAVE_CONFIG_H -I. -I. -I../../gcc -I../../gcc/. -I../../gcc/../include -I../../gcc/../libcpp/include -I../../gcc/../libdecnumber -I../../gcc/../libdecnumber/dpd -I../libdecnumber -I../../gcc/../libbacktrace -o insn-emit.o -MT insn-emit.o -MMD -MP -MF ./.deps/insn-emit.TPo insn-emit.c cc1plus: error: unrecognized command line option "-Wno-implicit-fallthrough" make[3]: *** [insn-emit.o] Error 1 Andreas. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 10:03:10AM +0200, Andreas Schwab wrote: > This breaks building with gcc-4.3. > > g++ -std=gnu++98 -fno-PIE -c -DUSE_LIBUNWIND_EXCEPTIONS -g -DIN_GCC > -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall > -Wwrite-strings -Wcast-qual -Wno-format -Wmissing-format-attribute > -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros > -Wno-overlength-strings -Wno-implicit-fallthrough -DHAVE_CONFIG_H -I. -I. > -I../../gcc -I../../gcc/. -I../../gcc/../include > -I../../gcc/../libcpp/include -I../../gcc/../libdecnumber > -I../../gcc/../libdecnumber/dpd -I../libdecnumber -I../../gcc/../libbacktrace > -o insn-emit.o -MT insn-emit.o -MMD -MP -MF ./.deps/insn-emit.TPo > insn-emit.c > cc1plus: error: unrecognized command line option "-Wno-implicit-fallthrough" > make[3]: *** [insn-emit.o] Error 1 Guess it must have been some mistake against the policy that unknown -Wno-* options aren't complained about, unless something else is diagnosed. Anyway, Marek, can you add configure check for that? Jakub
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 08:49:00AM +0200, Eric Botcazou wrote: > > It seems unfortunate that the warning doesn't accept /* ... fall > > through ... */ as a fallthrough comment. > > Seconded. The warning should take into account existing practices instead of > forcing the user to make completely bogus changes to the code (and Ada should > have been tested before the patch was approved). The intent has been that we catch the most common forms, but still require it not to be complete free form. Because, as experience shows, people are extremely creative in these comments, and it is not very good idea to support everything. For ... fall through ... , what is the purpose of those ...s? Jakub
[Patch, fortran] PR69834 - Collision in derived type hashes
Dear All,
The first attempts at fixing this bug were posted to the PR in
February of this year. Since then, real life has intervened and I have
not been able to get back to it until now.
The first patch used the address of the vtable to perform the
switching in SELECT_TYPE. Unfortunately, it failed in submodule_6.f90
and I have not been able to find a way to fix this without breaking
the ABI and having to bump up the module version number.
The second patch uses a string for the switching, which comprises a
concatenation of the type name and the module or procedure name.
Clearly, there is a performance penalty associated with this. My
recent efforts have been focussed on making this version detect
incoming selectors and associates that are use associated with
libraries that were compiled before this patch was applied and the
result is this submission. By the way, I was unable to find a way of
testing this feature as part of the testsuite but have done so 'by
hand'.
If the performance penalty is considered to be a show stopper, I could
develop further the version based on the vtable addresses but will
have to postpone any further work on this for a few weeks.
Otherwise, this patch does bootstrap and regtest on FC21/x86_64 - OK for trunk?
Cheers
Paul
2016-09-27 Paul Thomas
PR fortran/69834
* class.c (get_unique_type_string): Add an extra argument
'icase' that defaults to false but, when true, switches the
order of type name and module or procedure name.
(get_unique_hashed_string): New argument 'icase' switches
bewteen the old form and a new one in which the string length
is limited to GFC_MAX_SYMBOL_LEN and, in case of this limit
being exceeded, the hash string is followed by as much of the
composite name as possible.
(gfc_case_name): New function.
(gfc_find_derived_vtab): Add '_name' field to vtable. This is
initialized by 'get_unique_type_string' with 'icase' true.
(find_intrinsic_vtab): Ditto with initialization performed by a
call to 'gfc_case_name'.
* gfortran.h : Add macro 'gfc_add_name_component' and prototype
for 'gfc_case_name'.
* resolve.c (vtable_old_style): New function to determine if a
use associated vtable is missing the '_name' field.
(resolve_select_type): Call 'vtable_old_style' to determine if
any of the derived types or vtables come from a library that
was compiled before this patch. If this is the case, the old
form of SELECT TYPE is activated, in which the cases are set by
the hash value. Otherwise, the 'unique_type_string' is used.
2016-09-27 Paul Thomas
PR fortran/69834
* gfortran.dg/finalize_21.f90: Remove semi colon from the tree
scan.
* gfortran.dg/select_type_36.f03: New test.
* gfortran.dg/select_type_37.f03: New test.
Index: gcc/fortran/class.c
===
*** gcc/fortran/class.c (revision 240492)
--- gcc/fortran/class.c (working copy)
*** gfc_class_initializer (gfc_typespec *ts,
*** 472,492
containers and vtab symbols. */
static void
! get_unique_type_string (char *string, gfc_symbol *derived)
{
char dt_name[GFC_MAX_SYMBOL_LEN+1];
if (derived->attr.unlimited_polymorphic)
strcpy (dt_name, "STAR");
else
strcpy (dt_name, gfc_dt_upper_string (derived->name));
! if (derived->attr.unlimited_polymorphic)
! sprintf (string, "_%s", dt_name);
! else if (derived->module)
! sprintf (string, "%s_%s", derived->module, dt_name);
! else if (derived->ns->proc_name)
! sprintf (string, "%s_%s", derived->ns->proc_name->name, dt_name);
else
! sprintf (string, "_%s", dt_name);
}
--- 472,508
containers and vtab symbols. */
static void
! get_unique_type_string (char *string, gfc_symbol *derived, bool iscase =
false)
{
char dt_name[GFC_MAX_SYMBOL_LEN+1];
if (derived->attr.unlimited_polymorphic)
strcpy (dt_name, "STAR");
else
strcpy (dt_name, gfc_dt_upper_string (derived->name));
!
! /* The new style SELECT TYPE requires the type name to appear first. */
! if (iscase)
! {
! if (derived->attr.unlimited_polymorphic)
! sprintf (string, "_%s", dt_name);
! else if (derived->module)
! sprintf (string, "%s_%s", dt_name, derived->module);
! else if (derived->ns->proc_name)
! sprintf (string, "%s_%s", dt_name, derived->ns->proc_name->name);
! else
! sprintf (string, "_%s", dt_name);
! }
else
! {
! if (derived->attr.unlimited_polymorphic)
! sprintf (string, "_%s", dt_name);
! else if (derived->module)
! sprintf (string, "%s_%s", derived->module, dt_name);
! else if (derived->ns->proc_name)
! sprintf (string, "%s_%s", derived->ns->proc_name->name, dt_name);
! else
! sprintf (string, "_%s", dt_name);
! }
}
*** get_unique_type_string (char *string, gf
*** 494,512
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 10:13 AM, Jakub Jelinek wrote: > On Tue, Sep 27, 2016 at 10:03:10AM +0200, Andreas Schwab wrote: >> This breaks building with gcc-4.3. >> >> g++ -std=gnu++98 -fno-PIE -c -DUSE_LIBUNWIND_EXCEPTIONS -g -DIN_GCC >> -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall >> -Wwrite-strings -Wcast-qual -Wno-format -Wmissing-format-attribute >> -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros >> -Wno-overlength-strings -Wno-implicit-fallthrough -DHAVE_CONFIG_H -I. -I. >> -I../../gcc -I../../gcc/. -I../../gcc/../include >> -I../../gcc/../libcpp/include -I../../gcc/../libdecnumber >> -I../../gcc/../libdecnumber/dpd -I../libdecnumber >> -I../../gcc/../libbacktrace -o insn-emit.o -MT insn-emit.o -MMD -MP -MF >> ./.deps/insn-emit.TPo insn-emit.c >> cc1plus: error: unrecognized command line option "-Wno-implicit-fallthrough" >> make[3]: *** [insn-emit.o] Error 1 > > Guess it must have been some mistake against the policy that unknown -Wno-* > options aren't complained about, unless something else is diagnosed. > > Anyway, Marek, can you add configure check for that? Existing practice is to instead use -Wno-error rather than warning flags that were not present in those early GCC versions. Richard. > Jakub
Re: [PATCH v3] Optimize strchr to strlen
On Fri, Sep 23, 2016 at 4:10 PM, Oleg Endo wrote: > On Fri, 2016-09-23 at 14:07 +, Wilco Dijkstra wrote: >> After discussion (https://gcc.gnu.org/ml/gcc-patches/2016-09/msg00718 >> .html) >> here is the latest version of the strchr patch. This uses a gimple >> statement for >> the pointer addition so the gsi_prev always points at the strlen >> call. >> >> Optimize strchr (s, 0) to s + strlen (s). strchr (s, 0) appears a >> common >> idiom for finding the end of a string, however it is not a very >> efficient >> way of doing so. Strlen is a much simpler operation which is >> significantly >> faster (eg. on x86 strlen is 50% faster for strings of 8 bytes and >> about >> twice as fast as strchr on strings of 1KB). >> >> OK for commit? > > Please add PR tree-optimization/61056 to the changelog so that it gets > linked in bugzilla. Yes. Also you should strip gcc/ and gcc/testsuite from the filenames in the changelog entry as this entries are for gcc/ChangeLog and gcc/testsuite/ChangeLog. Ok with that changelog changes. Thanks, Richard. > Cheers, > Oleg
Re: Implement -Wimplicit-fallthrough (version 9)
> The intent has been that we catch the most common forms, but still require > it not to be complete free form. Because, as experience shows, people are > extremely creative in these comments, and it is not very good idea to > support everything. For ... fall through ... , what is the purpose of > those ...s? No idea, but it has been there for a while and seems perfectly reasonable. IMO any sentence containing "fall" and "through/thru/etc" on the same line should be accepted, otherwise it's just misplaced pickiness. -- Eric Botcazou
Re: Fix bootstrap failure in combine.c and i386.c due to -Wimplicit-fallthrough
On Tue, Sep 27, 2016 at 10:02:03AM +0200, Richard Biener wrote:
> > ix86_simd_clone_usable): Remove break after return.
And after scripting this a little bit:
#/bin/awk -f
/^[[:blank:]]+return[[:blank:]]/ { L1=$0;
PP=gensub(/^([[:blank:]]+)return.*$/,"\\1","",L1); next; }
/^[[:blank:]]+break/ { if (PP) {
PQ=gensub(/^([[:blank:]]+)break.*$/,"\\1","",$0); if (PP == PQ) { print L1;
print $0 } PP=""; } next }
{ PP=""; }
I found lots of other spots where we have useless break; right after return
stmt. IMHO it is just too confusing to keep them around, ok for trunk if
this passes bootstrap/regtest?
2016-09-27 Jakub Jelinek
* auto-inc-dec.c (try_merge): Remove break after return.
* cselib.c (autoinc_split): Likewise.
* explow.c (promote_mode): Likewise.
* fixed-value.c (fixed_arithmetic): Likewise.
* hsa.c (hsa_internal_fn::get_arity): Likewise.
* rtlanal.c (modified_between_p, modified_in_p): Likewise.
* trans-mem.c (get_attrs_for): Likewise.
* tree-if-conv.c (if_convertible_stmt_p): Likewise.
* tree-vrp.c (simplify_stmt_using_ranges): Likewise.
* config/aarch64/aarch64-builtins.c (aarch64_fold_builtin): Likewise.
* config/aarch64/aarch64.c (aarch64_get_condition_code_1): Likewise.
* config/c6x/c6x.c (c6x_get_unit_specifier): Likewise.
* config/cr16/cr16.c (legitimate_pic_operand_p): Likewise.
* config/cris/cris.c (cris_op_str): Likewise.
* config/mn10300/mn10300.c (cc_flags_for_code): Likewise.
* config/tilepro/tilepro.c (tilepro_emit_setcc_internal_di): Likewise.
c-family/
* c-ada-spec.c (print_ada_declaration): Remove break after return.
objc/
* objc-act.c (continue_class): Remove break after return.
(objc_maybe_printable_name): Likewise.
fortran/
* dependency.c (gfc_dep_compare_expr): Remove break after return.
* frontend-passes.c (optimize_op): Likewise.
* interface.c (gfc_current_interface_head): Likewise.
* symbol.c (check_conflict): Likewise.
* trans-intrinsic.c (build_fix_expr): Likewise.
ada/
* terminals.c (is_gui_app): Remove break after return.
--- gcc/auto-inc-dec.c.jj 2016-01-04 14:55:53.0 +0100
+++ gcc/auto-inc-dec.c 2016-09-27 10:29:15.510619645 +0200
@@ -658,25 +658,21 @@ try_merge (void)
if (dump_file)
fprintf (dump_file, "trying SIMPLE_PRE_INC\n");
return attempt_change (gen_rtx_PRE_INC (reg_mode, inc_reg), inc_reg);
- break;
case SIMPLE_POST_INC:/* size++ */
if (dump_file)
fprintf (dump_file, "trying SIMPLE_POST_INC\n");
return attempt_change (gen_rtx_POST_INC (reg_mode, inc_reg), inc_reg);
- break;
case SIMPLE_PRE_DEC: /* --size */
if (dump_file)
fprintf (dump_file, "trying SIMPLE_PRE_DEC\n");
return attempt_change (gen_rtx_PRE_DEC (reg_mode, inc_reg), inc_reg);
- break;
case SIMPLE_POST_DEC:/* size-- */
if (dump_file)
fprintf (dump_file, "trying SIMPLE_POST_DEC\n");
return attempt_change (gen_rtx_POST_DEC (reg_mode, inc_reg), inc_reg);
- break;
case DISP_PRE: /* ++con */
if (dump_file)
@@ -687,7 +683,6 @@ try_merge (void)
inc_reg,
inc_insn.reg1)),
inc_reg);
- break;
case DISP_POST: /* con++ */
if (dump_file)
@@ -698,7 +693,6 @@ try_merge (void)
inc_reg,
inc_insn.reg1)),
inc_reg);
- break;
case REG_PRE:/* ++reg */
if (dump_file)
@@ -709,7 +703,6 @@ try_merge (void)
inc_reg,
inc_insn.reg1)),
inc_reg);
- break;
case REG_POST:/* reg++ */
if (dump_file)
@@ -720,7 +713,6 @@ try_merge (void)
inc_reg,
inc_insn.reg1)),
inc_reg);
- break;
}
}
--- gcc/cselib.c.jj 2016-08-10 00:21:08.0 +0200
+++ gcc/cselib.c2016-09-27 10:29:42.125282563 +0200
@@ -805,7 +805,6 @@ autoinc_split (rtx x, rtx *off, machine_
*off = GEN_INT (-GET_MODE_SIZE (memmode));
return XEXP (x, 0);
- break;
case PRE_INC:
if (memmode == VOIDmode)
--- gcc/explow.c.jj 2016-09-05 19:27:05.0 +0200
+++ gcc/explow.c2016-09-27 10:30:05.435987326 +0200
@@ -802,7 +802,6 @@ promote_mode (const_tree type ATTRIBUTE_
PROMOTE_MODE (mode, unsignedp, type);
Unbreak bootstrap with GCC 4.3 (PR bootstrap/77751)
GCC 4.3 stupidly errors on this with error: unrecognized command line option "-Wno-implicit-fallthrough" so use -Wno-error instead. Bootstrapped on x86_64-linux, ok for trunk? 2016-09-27 Marek Polacek PR bootstrap/77751 * Makefile.in (insn-attrtab.o-warn, insn-dfatab.o-warn, insn-latencytab.o-warn, insn-output.o-warn, insn-emit.o-warn): Use -Wno-error instead of -Wno-implicit-fallthrough. diff --git gcc/Makefile.in gcc/Makefile.in index e8559cb..ff12908 100644 --- gcc/Makefile.in +++ gcc/Makefile.in @@ -218,11 +218,11 @@ libgcov-merge-tool.o-warn = -Wno-error gimple-match.o-warn = -Wno-unused generic-match.o-warn = -Wno-unused dfp.o-warn = -Wno-strict-aliasing -insn-attrtab.o-warn = -Wno-implicit-fallthrough -insn-dfatab.o-warn = -Wno-implicit-fallthrough -insn-latencytab.o-warn = -Wno-implicit-fallthrough -insn-output.o-warn = -Wno-implicit-fallthrough -insn-emit.o-warn = -Wno-implicit-fallthrough +insn-attrtab.o-warn = -Wno-error +insn-dfatab.o-warn = -Wno-error +insn-latencytab.o-warn = -Wno-error +insn-output.o-warn = -Wno-error +insn-emit.o-warn = -Wno-error # All warnings have to be shut off in stage1 if the compiler used then # isn't gcc; configure determines that. WARN_CFLAGS will be either Marek
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 09:50:17AM +0200, Gerald Pfeifer wrote: > Hi Marek, > > On Sat, 24 Sep 2016, Marek Polacek wrote: > > All right. I'll commit the patch on Monday. > > my thrice a week bootstrap on old (but still "supported") > i?86-unknown-freebsd9 broke as follows: > > cc1plus: error: unrecognized command line option "-Wno-implicit-fallthrough" > gmake[3]: *** [Makefile:1102: insn-emit.o] Error 1 > gmake[3]: *** Waiting for unfinished jobs > gmake[3]: Leaving directory '/scratch/tmp/gerald/OBJ-0927-0731/gcc' > gmake[2]: *** [Makefile:4571: all-stage1-gcc] Error 2 > gmake[2]: Leaving directory '/scratch/tmp/gerald/OBJ-0927-0731' > gmake[1]: *** [Makefile:24240: stage1-bubble] Error 2 > > (Also filed as https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77751 ) Sorry about that. I just sent a fix. Marek
Re: Unbreak bootstrap with GCC 4.3 (PR bootstrap/77751)
On Tue, Sep 27, 2016 at 11:40:19AM +0200, Marek Polacek wrote: > GCC 4.3 stupidly errors on this with > error: unrecognized command line option "-Wno-implicit-fallthrough" > so use -Wno-error instead. I'm not sure I like this, then we'll see even during stage2/stage3 lots of warnings in those files and other kinds of errors will not be reported. And, the surrounding lines show that we already use -Wno-unused or -Wno-strict-aliasing on other files. My preference would be configure test for whether -Wno-implicit-fallthrough works, and using @W_NO_IMPLICIT_FALLTHROUGH@ in there or something similar (or stick that into a make variable and use $(W_NO_IMPLICIT_FALLTHROUGH)). > > Bootstrapped on x86_64-linux, ok for trunk? > > 2016-09-27 Marek Polacek > > PR bootstrap/77751 > * Makefile.in (insn-attrtab.o-warn, insn-dfatab.o-warn, > insn-latencytab.o-warn, insn-output.o-warn, insn-emit.o-warn): Use > -Wno-error instead of -Wno-implicit-fallthrough. > > diff --git gcc/Makefile.in gcc/Makefile.in > index e8559cb..ff12908 100644 > --- gcc/Makefile.in > +++ gcc/Makefile.in > @@ -218,11 +218,11 @@ libgcov-merge-tool.o-warn = -Wno-error > gimple-match.o-warn = -Wno-unused > generic-match.o-warn = -Wno-unused > dfp.o-warn = -Wno-strict-aliasing > -insn-attrtab.o-warn = -Wno-implicit-fallthrough > -insn-dfatab.o-warn = -Wno-implicit-fallthrough > -insn-latencytab.o-warn = -Wno-implicit-fallthrough > -insn-output.o-warn = -Wno-implicit-fallthrough > -insn-emit.o-warn = -Wno-implicit-fallthrough > +insn-attrtab.o-warn = -Wno-error > +insn-dfatab.o-warn = -Wno-error > +insn-latencytab.o-warn = -Wno-error > +insn-output.o-warn = -Wno-error > +insn-emit.o-warn = -Wno-error > > # All warnings have to be shut off in stage1 if the compiler used then > # isn't gcc; configure determines that. WARN_CFLAGS will be either > > Marek Jakub
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 10:03:10AM +0200, Andreas Schwab wrote: > This breaks building with gcc-4.3. > > g++ -std=gnu++98 -fno-PIE -c -DUSE_LIBUNWIND_EXCEPTIONS -g -DIN_GCC > -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall > -Wwrite-strings -Wcast-qual -Wno-format -Wmissing-format-attribute > -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros > -Wno-overlength-strings -Wno-implicit-fallthrough -DHAVE_CONFIG_H -I. -I. > -I../../gcc -I../../gcc/. -I../../gcc/../include > -I../../gcc/../libcpp/include -I../../gcc/../libdecnumber > -I../../gcc/../libdecnumber/dpd -I../libdecnumber -I../../gcc/../libbacktrace > -o insn-emit.o -MT insn-emit.o -MMD -MP -MF ./.deps/insn-emit.TPo > insn-emit.c > cc1plus: error: unrecognized command line option "-Wno-implicit-fallthrough" > make[3]: *** [insn-emit.o] Error 1 You're right, sorry. Should be fixed in a bit. Marek
Re: Unbreak bootstrap with GCC 4.3 (PR bootstrap/77751)
On Sep 27 2016, Jakub Jelinek wrote: > On Tue, Sep 27, 2016 at 11:40:19AM +0200, Marek Polacek wrote: >> GCC 4.3 stupidly errors on this with >> error: unrecognized command line option "-Wno-implicit-fallthrough" >> so use -Wno-error instead. > > I'm not sure I like this, then we'll see even during stage2/stage3 lots of > warnings in those files and other kinds of errors will not be reported. > And, the surrounding lines show that we already use -Wno-unused or > -Wno-strict-aliasing on other files. > > My preference would be configure test for whether -Wno-implicit-fallthrough > works, and using @W_NO_IMPLICIT_FALLTHROUGH@ in there or something similar > (or stick that into a make variable and use $(W_NO_IMPLICIT_FALLTHROUGH)). Perhaps some general construct that suppresses -Wno-foo in stage1 only? Andreas. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
Re: [PATCH] Refactor section/label init for early LTO debug
Hi Richard, > The following patch ports a refactoring of section/label in it from > the early LTO debug work to trunk. For early LTO debug we need to > be able to emit two sets of debug infos into two sets of different > sections - early LTO into .gnu.debuglto_ prefixed sections and > regular (early + late) debug for the FAT part of the object. > > Thus this preparation splits out the section and label generation > from dwarf2out_init moving the text section related stuff to > dwarf2out_assembly_start and the rest to a new function > init_sections_and_labels which is now called only before we start > outputting dwarf (in dwarf2out_finish). It also removes some > dwarf_split_debug_info checks from the macro section name defines > (in the end I'll have up to four variants - regular, regular LTO, > DWO, DWO LTO). > > And it removes an unused function. > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress. I've > also bootstrapped with -O2 -g3 to exercise the .debug_macro path. > > Ok? > > Just noticed that DEBUG_STR_OFFSETS_SECTION needs similar massaging > for its dwarf_split_debug_info condition - will followup as obvious > if this one is approved. this patch introduced many pch assembly comparison failures on Solaris (both sparc and x86, 32 and 64-bit, /bin/as only), like FAIL: gcc.dg/pch/common-1.c -O3 -g assembly comparison FAIL: gcc.dg/pch/common-1.c -O0 -g assembly comparison gcc.log shows this diff: < .long .Letext0 > .long While the .Letext0 label is still in the assembly output, it isn't referenced inside .debug_line, as can be seen in the diff. Unlike the gas case, with as HAVE_AS_DWARF2_DEBUG_LINE isn't defined. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH] Refactor section/label init for early LTO debug
> this patch introduced many pch assembly comparison failures on Solaris > (both sparc and x86, 32 and 64-bit, /bin/as only), like Something on darwin. TIA Dominique
Re: Fix bootstrap failure in combine.c and i386.c due to -Wimplicit-fallthrough
On Tue, Sep 27, 2016 at 11:06 AM, Jakub Jelinek wrote:
> On Tue, Sep 27, 2016 at 10:02:03AM +0200, Richard Biener wrote:
>> > ix86_simd_clone_usable): Remove break after return.
>
> And after scripting this a little bit:
> #/bin/awk -f
> /^[[:blank:]]+return[[:blank:]]/ { L1=$0;
> PP=gensub(/^([[:blank:]]+)return.*$/,"\\1","",L1); next; }
> /^[[:blank:]]+break/ { if (PP) {
> PQ=gensub(/^([[:blank:]]+)break.*$/,"\\1","",$0); if (PP == PQ) { print L1;
> print $0 } PP=""; } next }
> { PP=""; }
>
> I found lots of other spots where we have useless break; right after return
> stmt. IMHO it is just too confusing to keep them around, ok for trunk if
> this passes bootstrap/regtest?
Ok (this counts as obvious changes IMHO).
Richard.
> 2016-09-27 Jakub Jelinek
>
> * auto-inc-dec.c (try_merge): Remove break after return.
> * cselib.c (autoinc_split): Likewise.
> * explow.c (promote_mode): Likewise.
> * fixed-value.c (fixed_arithmetic): Likewise.
> * hsa.c (hsa_internal_fn::get_arity): Likewise.
> * rtlanal.c (modified_between_p, modified_in_p): Likewise.
> * trans-mem.c (get_attrs_for): Likewise.
> * tree-if-conv.c (if_convertible_stmt_p): Likewise.
> * tree-vrp.c (simplify_stmt_using_ranges): Likewise.
> * config/aarch64/aarch64-builtins.c (aarch64_fold_builtin): Likewise.
> * config/aarch64/aarch64.c (aarch64_get_condition_code_1): Likewise.
> * config/c6x/c6x.c (c6x_get_unit_specifier): Likewise.
> * config/cr16/cr16.c (legitimate_pic_operand_p): Likewise.
> * config/cris/cris.c (cris_op_str): Likewise.
> * config/mn10300/mn10300.c (cc_flags_for_code): Likewise.
> * config/tilepro/tilepro.c (tilepro_emit_setcc_internal_di): Likewise.
> c-family/
> * c-ada-spec.c (print_ada_declaration): Remove break after return.
> objc/
> * objc-act.c (continue_class): Remove break after return.
> (objc_maybe_printable_name): Likewise.
> fortran/
> * dependency.c (gfc_dep_compare_expr): Remove break after return.
> * frontend-passes.c (optimize_op): Likewise.
> * interface.c (gfc_current_interface_head): Likewise.
> * symbol.c (check_conflict): Likewise.
> * trans-intrinsic.c (build_fix_expr): Likewise.
> ada/
> * terminals.c (is_gui_app): Remove break after return.
>
> --- gcc/auto-inc-dec.c.jj 2016-01-04 14:55:53.0 +0100
> +++ gcc/auto-inc-dec.c 2016-09-27 10:29:15.510619645 +0200
> @@ -658,25 +658,21 @@ try_merge (void)
>if (dump_file)
> fprintf (dump_file, "trying SIMPLE_PRE_INC\n");
>return attempt_change (gen_rtx_PRE_INC (reg_mode, inc_reg), inc_reg);
> - break;
>
> case SIMPLE_POST_INC:/* size++ */
>if (dump_file)
> fprintf (dump_file, "trying SIMPLE_POST_INC\n");
>return attempt_change (gen_rtx_POST_INC (reg_mode, inc_reg), inc_reg);
> - break;
>
> case SIMPLE_PRE_DEC: /* --size */
>if (dump_file)
> fprintf (dump_file, "trying SIMPLE_PRE_DEC\n");
>return attempt_change (gen_rtx_PRE_DEC (reg_mode, inc_reg), inc_reg);
> - break;
>
> case SIMPLE_POST_DEC:/* size-- */
>if (dump_file)
> fprintf (dump_file, "trying SIMPLE_POST_DEC\n");
>return attempt_change (gen_rtx_POST_DEC (reg_mode, inc_reg), inc_reg);
> - break;
>
> case DISP_PRE: /* ++con */
>if (dump_file)
> @@ -687,7 +683,6 @@ try_merge (void)
>inc_reg,
>
> inc_insn.reg1)),
> inc_reg);
> - break;
>
> case DISP_POST: /* con++ */
>if (dump_file)
> @@ -698,7 +693,6 @@ try_merge (void)
> inc_reg,
>
> inc_insn.reg1)),
> inc_reg);
> - break;
>
> case REG_PRE:/* ++reg */
>if (dump_file)
> @@ -709,7 +703,6 @@ try_merge (void)
>inc_reg,
>
> inc_insn.reg1)),
> inc_reg);
> - break;
>
> case REG_POST:/* reg++ */
>if (dump_file)
> @@ -720,7 +713,6 @@ try_merge (void)
> inc_reg,
>
> inc_insn.reg1)),
> inc_reg);
> - break;
> }
> }
>
> --- gcc/cselib.c.jj 2016-08-10 00:21:08.0 +0200
> +++ gcc/cselib.c2016-09-27 10:29:42.125282563 +0200
> @@ -805,7 +805,6 @@ autoinc_split (rtx x, rtx *off, machine_
>
>*off = GEN_INT (-GET_MODE_SIZE (memmode)
Re: Unbreak bootstrap with GCC 4.3 (PR bootstrap/77751)
On Tue, Sep 27, 2016 at 11:40 AM, Marek Polacek wrote: > GCC 4.3 stupidly errors on this with > error: unrecognized command line option "-Wno-implicit-fallthrough" > so use -Wno-error instead. > > Bootstrapped on x86_64-linux, ok for trunk? Ok for trunk now to unbreak peoples bootstrap. We can improve on this later if we want. Richard. > 2016-09-27 Marek Polacek > > PR bootstrap/77751 > * Makefile.in (insn-attrtab.o-warn, insn-dfatab.o-warn, > insn-latencytab.o-warn, insn-output.o-warn, insn-emit.o-warn): Use > -Wno-error instead of -Wno-implicit-fallthrough. > > diff --git gcc/Makefile.in gcc/Makefile.in > index e8559cb..ff12908 100644 > --- gcc/Makefile.in > +++ gcc/Makefile.in > @@ -218,11 +218,11 @@ libgcov-merge-tool.o-warn = -Wno-error > gimple-match.o-warn = -Wno-unused > generic-match.o-warn = -Wno-unused > dfp.o-warn = -Wno-strict-aliasing > -insn-attrtab.o-warn = -Wno-implicit-fallthrough > -insn-dfatab.o-warn = -Wno-implicit-fallthrough > -insn-latencytab.o-warn = -Wno-implicit-fallthrough > -insn-output.o-warn = -Wno-implicit-fallthrough > -insn-emit.o-warn = -Wno-implicit-fallthrough > +insn-attrtab.o-warn = -Wno-error > +insn-dfatab.o-warn = -Wno-error > +insn-latencytab.o-warn = -Wno-error > +insn-output.o-warn = -Wno-error > +insn-emit.o-warn = -Wno-error > > # All warnings have to be shut off in stage1 if the compiler used then > # isn't gcc; configure determines that. WARN_CFLAGS will be either > > Marek
Re: debug container mutex association
On 26/09/16 22:36 +0200, François Dumont wrote:
Fixed with attached patch.
François
On 26/09/2016 13:56, Andreas Schwab wrote:
FAIL: 23_containers/list/debug/invalidation/4.cc (test for excess errors)
Excess errors:
/daten/aranym/gcc/gcc-20160926/Build/m68k-linux/libstdc++-v3/include/debug/safe_sequence.tcc:89:
error: 'void __gnu_debug::_Safe_iterator_base::_M_detach_single()' is
protected within this context
/daten/aranym/gcc/gcc-20160926/Build/m68k-linux/libstdc++-v3/include/debug/safe_sequence.tcc:112:
error: 'void __gnu_debug::_Safe_iterator_base::_M_detach_single()' is
protected within this context
Andreas.
Index: include/debug/safe_base.h
===
--- include/debug/safe_base.h (revision 240509)
+++ include/debug/safe_base.h (working copy)
@@ -121,11 +121,11 @@
void
_M_detach();
+ public:
/** Likewise, but not thread-safe. */
void
_M_detach_single() throw ();
- public:
/** Determines if we are attached to the given sequence. */
bool
_M_attached_to(const _Safe_sequence_base* __seq) const
Would this be a smaller change, that doesn't make the member
accessible to all code?
--- a/libstdc++-v3/include/debug/safe_base.h
+++ b/libstdc++-v3/include/debug/safe_base.h
@@ -50,6 +50,7 @@ namespace __gnu_debug
class _Safe_iterator_base
{
friend class _Safe_sequence_base;
+template friend class _Safe_sequence;
public:
/** The sequence this iterator references; may be NULL to indicate
Re: Implement -Wimplicit-fallthrough (version 9)
Hi Marek, On 27/09/16 10:44, Marek Polacek wrote: On Tue, Sep 27, 2016 at 10:03:10AM +0200, Andreas Schwab wrote: This breaks building with gcc-4.3. g++ -std=gnu++98 -fno-PIE -c -DUSE_LIBUNWIND_EXCEPTIONS -g -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wwrite-strings -Wcast-qual -Wno-format -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Wno-implicit-fallthrough -DHAVE_CONFIG_H -I. -I. -I../../gcc -I../../gcc/. -I../../gcc/../include -I../../gcc/../libcpp/include -I../../gcc/../libdecnumber -I../../gcc/../libdecnumber/dpd -I../libdecnumber -I../../gcc/../libbacktrace -o insn-emit.o -MT insn-emit.o -MMD -MP -MF ./.deps/insn-emit.TPo insn-emit.c cc1plus: error: unrecognized command line option "-Wno-implicit-fallthrough" make[3]: *** [insn-emit.o] Error 1 You're right, sorry. Should be fixed in a bit. Marek I'm seeing about 4 triggers of this warning in the arm backend (thus breaking bootstrap), mostly due to the fall through comment adding some explanation. For example: $SRC/gcc/config/arm/arm-builtins.c: In function 'rtx_def* arm_expand_neon_args(rtx, machine_mode, int, int, int, tree, builtin_arg*)': $SRC/gcc/config/arm/arm-builtins.c:2155:3: error: this statement may fall through [-Werror=implicit-fallthrough] } ^ $SRC/gcc/config/arm/arm-builtins.c:2159:6: note: here case NEON_ARG_CONSTANT: where the code is: 2156 /* Fall through - if the lane index isn't a constant then 2157 the next case will error. */ 2158 2159 case NEON_ARG_CONSTANT: Is there supposed to be no empty line between the case statement and the comment? Or is the comment only supposed to contain "Fall through"? Thanks, Kyrill
[gomp4] pretty print ifn args
In working on some new code I got sufficiently frustrated to implement pretty
printing on internal function discriminators. We now get:
.data_dep.2 = UNIQUE (OACC_FORK, .data_dep.2, -1);
rather than an obscure raw integer for the first argument.
For the internal fns (I know of) that have a discriminator argument, I define
the codes in appropriate macros, and then expand them to create the enum
definitions. In the pretty printer I examine the first argument and use that to
index into a string array also built from the code macro.
applied to gomp4.
nathan
2016-09-27 Nathan Sidwell
* internal-fn.h (IFN_UNIQUE_CODES, IFN_GOACC_LOOP_CODES,
IFN_GOACC_REDUCTION_CODES): New.
(enum ifn_unique_kind, enum ifn_goacc_loop_kind, enum
ifn_goacc_reduction_kind): Use them.
* gimple-pretty-print.c (dump_gimple_call_args): Decode first arg
of internal functions, when applicable.
Index: gimple-pretty-print.c
===
--- gimple-pretty-print.c (revision 240524)
+++ gimple-pretty-print.c (working copy)
@@ -579,9 +579,63 @@ dump_gimple_return (pretty_printer *buff
static void
dump_gimple_call_args (pretty_printer *buffer, gcall *gs, int flags)
{
- size_t i;
+ size_t i = 0;
- for (i = 0; i < gimple_call_num_args (gs); i++)
+ /* Pretty print first arg to certain internal fns. */
+ if (gimple_call_internal_p (gs))
+{
+ const char *const *enums = NULL;
+ unsigned limit = 0;
+
+ switch (gimple_call_internal_fn (gs))
+ {
+ case IFN_UNIQUE:
+#define DEF(X) #X
+ static const char *const unique_args[] = {IFN_UNIQUE_CODES};
+#undef DEF
+ enums = unique_args;
+
+ limit = ARRAY_SIZE (unique_args);
+ break;
+
+ case IFN_GOACC_LOOP:
+#define DEF(X) #X
+ static const char *const loop_args[] = {IFN_GOACC_LOOP_CODES};
+#undef DEF
+ enums = loop_args;
+ limit = ARRAY_SIZE (loop_args);
+ break;
+
+ case IFN_GOACC_REDUCTION:
+#define DEF(X) #X
+ static const char *const reduction_args[]
+ = {IFN_GOACC_REDUCTION_CODES};
+#undef DEF
+ enums = reduction_args;
+ limit = ARRAY_SIZE (reduction_args);
+ break;
+
+ default:
+ break;
+ }
+ if (limit)
+ {
+ tree arg0 = gimple_call_arg (gs, 0);
+ HOST_WIDE_INT v;
+
+ if (TREE_CODE (arg0) == INTEGER_CST
+ && tree_fits_shwi_p (arg0)
+ && (v = tree_to_shwi (arg0)) >= 0 && v < limit)
+ {
+ i++;
+ pp_string (buffer, enums[v]);
+ if (i < gimple_call_num_args (gs))
+ pp_string (buffer, ", ");
+ }
+ }
+}
+
+ for (; i < gimple_call_num_args (gs); i++)
{
dump_generic_node (buffer, gimple_call_arg (gs, i), 0, flags, false);
if (i < gimple_call_num_args (gs) - 1)
Index: internal-fn.h
===
--- internal-fn.h (revision 240524)
+++ internal-fn.h (working copy)
@@ -20,26 +20,28 @@ along with GCC; see the file COPYING3.
#ifndef GCC_INTERNAL_FN_H
#define GCC_INTERNAL_FN_H
-/* INTEGER_CST values for IFN_UNIQUE function arg-0. */
-enum ifn_unique_kind {
- IFN_UNIQUE_UNSPEC, /* Undifferentiated UNIQUE. */
+/* INTEGER_CST values for IFN_UNIQUE function arg-0.
+
+ UNSPEC: Undifferentiated UNIQUE.
- /* FORK and JOIN mark the points at which OpenACC partitioned
- execution is entered or exited.
- return: data dependency value
- arg-1: data dependency var
- arg-2: INTEGER_CST argument, indicating the axis. */
- IFN_UNIQUE_OACC_FORK,
- IFN_UNIQUE_OACC_JOIN,
-
- /* HEAD_MARK and TAIL_MARK are used to demark the sequence entering
- or leaving partitioned execution.
- return: data dependency value
- arg-1: data dependency var
- arg-2: INTEGER_CST argument, remaining markers in this sequence
- arg-3...: varargs on primary header */
- IFN_UNIQUE_OACC_HEAD_MARK,
- IFN_UNIQUE_OACC_TAIL_MARK
+ FORK and JOIN mark the points at which OpenACC partitioned
+ execution is entered or exited.
+ DEP_VAR = UNIQUE ({FORK,JOIN}, DEP_VAR, AXIS)
+
+ HEAD_MARK and TAIL_MARK are used to demark the sequence entering
+ or leaving partitioned execution.
+ DEP_VAR = UNIQUE ({HEAD,TAIL}_MARK, REMAINING_MARKS, ...PRIMARY_FLAGS)
+
+ The PRIMARY_FLAGS only occur on the first HEAD_MARK of a sequence. */
+#define IFN_UNIQUE_CODES \
+ DEF(UNSPEC), \
+DEF(OACC_FORK), DEF(OACC_JOIN), \
+DEF(OACC_HEAD_MARK), DEF(OACC_TAIL_MARK)
+
+enum ifn_unique_kind {
+#define DEF(X) IFN_UNIQUE_##X
+ IFN_UNIQUE_CODES
+#undef DEF
};
/* INTEGER_CST values for IFN_GOACC_LOOP arg-0. Allows the precise
@@ -59,11 +61,12 @@ enum ifn_unique_kind {
CHUNK_NO - chunk number
MASK - partitioning mask. */
+#define IFN_GOACC_LOOP_CODES \
+ DEF(CHUNKS), DEF(STEP), DEF(OFFSET), DEF(BOUND)
enum ifn_goacc_loop_kind {
- IFN_GOACC_LOOP_CHUNKS, /* Number of chunks. */
- IFN_GOACC_LOOP_STEP,/* Size of each thread's step. */
- IFN_GOACC_LOOP_OFFSET, /* Initial iteration value. */
- IFN_GOACC_LOOP_BOUND
Re: [PATCH] Refactor section/label init for early LTO debug
On Tue, 27 Sep 2016, Rainer Orth wrote:
> Hi Richard,
>
> > The following patch ports a refactoring of section/label in it from
> > the early LTO debug work to trunk. For early LTO debug we need to
> > be able to emit two sets of debug infos into two sets of different
> > sections - early LTO into .gnu.debuglto_ prefixed sections and
> > regular (early + late) debug for the FAT part of the object.
> >
> > Thus this preparation splits out the section and label generation
> > from dwarf2out_init moving the text section related stuff to
> > dwarf2out_assembly_start and the rest to a new function
> > init_sections_and_labels which is now called only before we start
> > outputting dwarf (in dwarf2out_finish). It also removes some
> > dwarf_split_debug_info checks from the macro section name defines
> > (in the end I'll have up to four variants - regular, regular LTO,
> > DWO, DWO LTO).
> >
> > And it removes an unused function.
> >
> > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress. I've
> > also bootstrapped with -O2 -g3 to exercise the .debug_macro path.
> >
> > Ok?
> >
> > Just noticed that DEBUG_STR_OFFSETS_SECTION needs similar massaging
> > for its dwarf_split_debug_info condition - will followup as obvious
> > if this one is approved.
>
> this patch introduced many pch assembly comparison failures on Solaris
> (both sparc and x86, 32 and 64-bit, /bin/as only), like
>
> FAIL: gcc.dg/pch/common-1.c -O3 -g assembly comparison
> FAIL: gcc.dg/pch/common-1.c -O0 -g assembly comparison
>
> gcc.log shows this diff:
>
> < .long .Letext0
> > .long
>
> While the .Letext0 label is still in the assembly output, it isn't
> referenced inside .debug_line, as can be seen in the diff.
Can you check if moving
static void
dwarf2out_assembly_start (void)
{
#ifndef DWARF2_LINENO_DEBUGGING_INFO
ASM_GENERATE_INTERNAL_LABEL (text_section_label, TEXT_SECTION_LABEL, 0);
ASM_GENERATE_INTERNAL_LABEL (text_end_label, TEXT_END_LABEL, 0);
ASM_GENERATE_INTERNAL_LABEL (cold_text_section_label,
COLD_TEXT_SECTION_LABEL, 0);
ASM_GENERATE_INTERNAL_LABEL (cold_end_label, COLD_END_LABEL, 0);
switch_to_section (text_section);
ASM_OUTPUT_LABEL (asm_out_file, text_section_label);
#endif
back to dwarf2out_init helps? Ah! Does
Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 240521)
+++ gcc/dwarf2out.c (working copy)
@@ -25657,14 +25687,6 @@ dwarf2out_init (const char *filename ATT
vec_alloc (macinfo_table, 64);
#endif
- /* Make sure the line number table for .text always exists. */
- text_section_line_info = new_line_info_table ();
- text_section_line_info->end_label = text_end_label;
-
-#ifdef DWARF2_LINENO_DEBUGGING_INFO
- cur_line_info_table = text_section_line_info;
-#endif
-
/* If front-ends already registered a main translation unit but we were
not
ready to perform the association, do this now. */
if (main_translation_unit != NULL_TREE)
@@ -25688,6 +25710,14 @@ dwarf2out_assembly_start (void)
ASM_OUTPUT_LABEL (asm_out_file, text_section_label);
#endif
+ /* Make sure the line number table for .text always exists. */
+ text_section_line_info = new_line_info_table ();
+ text_section_line_info->end_label = text_end_label;
+
+#ifdef DWARF2_LINENO_DEBUGGING_INFO
+ cur_line_info_table = text_section_line_info;
+#endif
+
if (HAVE_GAS_CFI_SECTIONS_DIRECTIVE
&& dwarf2out_do_cfi_asm ()
&& (!(flag_unwind_tables || flag_exceptions)
fix it? Ok if it passes testing for you.
Thanks,
Richard.
Re: Implement -Wimplicit-fallthrough (version 9)
On 2016.09.27 at 10:46 +0200, Eric Botcazou wrote: > > The intent has been that we catch the most common forms, but still require > > it not to be complete free form. Because, as experience shows, people are > > extremely creative in these comments, and it is not very good idea to > > support everything. For ... fall through ... , what is the purpose of > > those ...s? > > No idea, but it has been there for a while and seems perfectly reasonable. > IMO any sentence containing "fall" and "through/thru/etc" on the same line > should be accepted, otherwise it's just misplaced pickiness. +1. Folks will just disable the warning if gcc is not very permissive when paring existing comments. You cannot expect anyone to change perfectly fine fall-through comments just to accommodate an arbitrary gcc style. -- Markus
Re: Unbreak bootstrap with GCC 4.3 (PR bootstrap/77751)
On Tue, Sep 27, 2016 at 12:24:23PM +0200, Richard Biener wrote: > On Tue, Sep 27, 2016 at 11:40 AM, Marek Polacek wrote: > > GCC 4.3 stupidly errors on this with > > error: unrecognized command line option "-Wno-implicit-fallthrough" > > so use -Wno-error instead. > > > > Bootstrapped on x86_64-linux, ok for trunk? > > Ok for trunk now to unbreak peoples bootstrap. We can improve on this > later if we want. Committed. I'll look into the configure test check now. Marek
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 11:32:42AM +0100, Kyrill Tkachov wrote: > where the code is: > 2156 /* Fall through - if the lane index isn't a constant then > 2157 the next case will error. */ > 2158 > 2159 case NEON_ARG_CONSTANT: > > > Is there supposed to be no empty line between the case statement and the > comment? > Or is the comment only supposed to contain "Fall through"? The last comment before case or default keyword (or user label before case/default) has to match one of the following regexps: //-fallthrough$ //@fallthrough@$ //[ \t]*FALL(S | |-)?THR(OUGH|U)\.?[ \t]*$ //[ \t]*Fall(s | |-)?[Tt]hr(ough|u)\.?[ \t]*$ //[ \t]*fall(s | |-)?thr(ough|u)\.?[ \t]*$ /\*-fallthrough\*/ /\*@fallthrough@\*/ /\*[ \t]*FALL(S | |-)?THR(OUGH|U)\.?[ \t]*\*/ /\*[ \t]*Fall(s | |-)?[Tt]hr(ough|u)\.?[ \t]*\*/ /\*[ \t]*fall(s | |-)?thr(ough|u)\.?[ \t]*\*/ So, you could e.g. write: /* If the lane index isn't a constant, then the next case will error. */ /* Fall through. */ but not what you have, free form is not accepted. Jakub
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 12:39:41PM +0200, Markus Trippelsdorf wrote: > On 2016.09.27 at 10:46 +0200, Eric Botcazou wrote: > > > The intent has been that we catch the most common forms, but still require > > > it not to be complete free form. Because, as experience shows, people are > > > extremely creative in these comments, and it is not very good idea to > > > support everything. For ... fall through ... , what is the purpose of > > > those ...s? > > > > No idea, but it has been there for a while and seems perfectly reasonable. > > IMO any sentence containing "fall" and "through/thru/etc" on the same line > > should be accepted, otherwise it's just misplaced pickiness. > > +1. Folks will just disable the warning if gcc is not very permissive > when paring existing comments. You cannot expect anyone to change > perfectly fine fall-through comments just to accommodate an arbitrary > gcc style. The accepted style is already very permissive, we don't allow just one spelling as various lint tools. I'm afraid looking for various cases of fall and through/thru possibly separated by anything and surrounded by anything is IMHO already too much, the compiler shouldn't try to try to grammar analyze the comments on what they actually talk about and whether it might be related to the switch fall through or something completely different. Users should start using [[fallthrough]]; anyway. Jakub
Re: [PATCH] Refactor section/label init for early LTO debug
Hi Richard,
>> this patch introduced many pch assembly comparison failures on Solaris
>> (both sparc and x86, 32 and 64-bit, /bin/as only), like
>>
>> FAIL: gcc.dg/pch/common-1.c -O3 -g assembly comparison
>> FAIL: gcc.dg/pch/common-1.c -O0 -g assembly comparison
>>
>> gcc.log shows this diff:
>>
>> < .long .Letext0
>> > .long
>>
>> While the .Letext0 label is still in the assembly output, it isn't
>> referenced inside .debug_line, as can be seen in the diff.
>
> Can you check if moving
>
> static void
> dwarf2out_assembly_start (void)
> {
> #ifndef DWARF2_LINENO_DEBUGGING_INFO
> ASM_GENERATE_INTERNAL_LABEL (text_section_label, TEXT_SECTION_LABEL, 0);
> ASM_GENERATE_INTERNAL_LABEL (text_end_label, TEXT_END_LABEL, 0);
> ASM_GENERATE_INTERNAL_LABEL (cold_text_section_label,
>COLD_TEXT_SECTION_LABEL, 0);
> ASM_GENERATE_INTERNAL_LABEL (cold_end_label, COLD_END_LABEL, 0);
>
> switch_to_section (text_section);
> ASM_OUTPUT_LABEL (asm_out_file, text_section_label);
> #endif
>
> back to dwarf2out_init helps? Ah! Does
>
> Index: gcc/dwarf2out.c
> ===
> --- gcc/dwarf2out.c (revision 240521)
> +++ gcc/dwarf2out.c (working copy)
> @@ -25657,14 +25687,6 @@ dwarf2out_init (const char *filename ATT
> vec_alloc (macinfo_table, 64);
> #endif
>
> - /* Make sure the line number table for .text always exists. */
> - text_section_line_info = new_line_info_table ();
> - text_section_line_info->end_label = text_end_label;
> -
> -#ifdef DWARF2_LINENO_DEBUGGING_INFO
> - cur_line_info_table = text_section_line_info;
> -#endif
> -
>/* If front-ends already registered a main translation unit but we were
> not
> ready to perform the association, do this now. */
>if (main_translation_unit != NULL_TREE)
> @@ -25688,6 +25710,14 @@ dwarf2out_assembly_start (void)
>ASM_OUTPUT_LABEL (asm_out_file, text_section_label);
> #endif
>
> + /* Make sure the line number table for .text always exists. */
> + text_section_line_info = new_line_info_table ();
> + text_section_line_info->end_label = text_end_label;
> +
> +#ifdef DWARF2_LINENO_DEBUGGING_INFO
> + cur_line_info_table = text_section_line_info;
> +#endif
> +
>if (HAVE_GAS_CFI_SECTIONS_DIRECTIVE
>&& dwarf2out_do_cfi_asm ()
>&& (!(flag_unwind_tables || flag_exceptions)
>
>
> fix it? Ok if it passes testing for you.
testing on a single testcase worked fine. I'll just run a full
bootstrap to make sure everything is fine.
Thanks.
Rainer
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 12:47:50PM +0200, Jakub Jelinek wrote: > On Tue, Sep 27, 2016 at 12:39:41PM +0200, Markus Trippelsdorf wrote: > > On 2016.09.27 at 10:46 +0200, Eric Botcazou wrote: > > > > The intent has been that we catch the most common forms, but still > > > > require > > > > it not to be complete free form. Because, as experience shows, people > > > > are > > > > extremely creative in these comments, and it is not very good idea to > > > > support everything. For ... fall through ... , what is the purpose of > > > > those ...s? > > > > > > No idea, but it has been there for a while and seems perfectly reasonable. > > > IMO any sentence containing "fall" and "through/thru/etc" on the same > > > line > > > should be accepted, otherwise it's just misplaced pickiness. > > > > +1. Folks will just disable the warning if gcc is not very permissive > > when paring existing comments. You cannot expect anyone to change > > perfectly fine fall-through comments just to accommodate an arbitrary > > gcc style. > > The accepted style is already very permissive, we don't allow just one > spelling as various lint tools. I'm afraid looking for various cases of > fall and through/thru possibly separated by anything and surrounded by > anything is IMHO already too much, the compiler shouldn't try to try to > grammar analyze the comments on what they actually talk about and whether it > might be related to the switch fall through or something completely > different. Users should start using [[fallthrough]]; anyway. Oh, forgot, I think allowing /* Fallthrough */ /* arbitrary comment */ case ... might be something we could be also supporting, especially because sometimes users might want to comment on what the following case handle and fallthrough would be just something in between. But IMHO forcing users to use some clear markup style if they don't want to/can't switch to [[fallthrough]]; __attribute__((fallthrough)); or some macro that does that is a good idea. That will certainly increase the chance other compilers could do the same thing, parsing arbitrary stuff is hard to agree on. Jakub
Re: [PATCH, RFC] gcov: dump in a static dtor instead of in an atexit handler
On 09/26/16 11:22, Martin Liška wrote:
Hi.
So the I found reason of inconsistencies, running multiple times -fselftest is
enough to
find that memory allocation related functions can be executed different times.
Small example:
thanks for checking.
@@ -598,6 +598,10 @@ facilities to restrict profile collection to the program
region of
interest. Calling @code{__gcov_reset(void)} will clear all profile counters
to zero, and calling @code{__gcov_dump(void)} will cause the profile
information
collected at that point to be dumped to @file{.gcda} output files.
+By default, every instrumented application calls __gcov_dump function
+via a static destructor with priority equal to 99. That would guarantee
+that all user defined destructors, as well as function handlers registered
+by atexit, would be executed before gcov dump function is executed.
'by default' This suggests there's a non-default behaviour, but I can't see it
nor how to enable it. Perhaps:
"Instrumented applications use a static destructor with priority 99 to invoke
the __gcov_dump function. Thus __gcov_dump is executed after all
user defined static destructors, as well as handlers registered with atexit."
?
nathan
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 12:47:50PM +0200, Jakub Jelinek wrote: > On Tue, Sep 27, 2016 at 12:39:41PM +0200, Markus Trippelsdorf wrote: > > On 2016.09.27 at 10:46 +0200, Eric Botcazou wrote: > > > > The intent has been that we catch the most common forms, but still > > > > require > > > > it not to be complete free form. Because, as experience shows, people > > > > are > > > > extremely creative in these comments, and it is not very good idea to > > > > support everything. For ... fall through ... , what is the purpose of > > > > those ...s? > > > > > > No idea, but it has been there for a while and seems perfectly reasonable. > > > IMO any sentence containing "fall" and "through/thru/etc" on the same > > > line > > > should be accepted, otherwise it's just misplaced pickiness. > > > > +1. Folks will just disable the warning if gcc is not very permissive > > when paring existing comments. You cannot expect anyone to change > > perfectly fine fall-through comments just to accommodate an arbitrary > > gcc style. > > The accepted style is already very permissive, we don't allow just one > spelling as various lint tools. I'm afraid looking for various cases of > fall and through/thru possibly separated by anything and surrounded by > anything is IMHO already too much, the compiler shouldn't try to try to > grammar analyze the comments on what they actually talk about and whether it > might be related to the switch fall through or something completely > different. Users should start using [[fallthrough]]; anyway. I'm thinking perhaps we should also accept /* ... fall through ... */ and /* else fall through */, but accepting any sentence containing "fall" and "through/thru/etc" on the same line would mean that we also accept /* Don't fall through here. */ and that is clearly not desirable. Marek
internal fn pretty printing
In working on some new code I got sufficiently frustrated to implement pretty
printing on internal function discriminators, as I think one of you suggested a
while back. With this patch we get:
.data_dep.2 = UNIQUE (OACC_FORK, .data_dep.2, -1);
rather than an obscure raw integer for the first argument.
For the internal fns (I know of) that have a discriminator argument, I define
the codes in appropriate macros, and then expand them to create the enum
definitions. In the pretty printer I examine the first argument and use that to
index into a string array also built from the code macro.
Ok for trunk? (I've just applied it to gomp4).
nathan
2016-09-27 Nathan Sidwell
* internal-fn.h (IFN_UNIQUE_CODES, IFN_GOACC_LOOP_CODES,
IFN_GOACC_REDUCTION_CODES): New.
(enum ifn_unique_kind, enum ifn_goacc_loop_kind, enum
ifn_goacc_reduction_kind): Use them.
* gimple-pretty-print.c (dump_gimple_call_args): Decode first arg
of internal functions, when applicable.
Index: gimple-pretty-print.c
===
--- gimple-pretty-print.c (revision 240525)
+++ gimple-pretty-print.c (working copy)
@@ -599,9 +599,63 @@ dump_gimple_return (pretty_printer *buff
static void
dump_gimple_call_args (pretty_printer *buffer, gcall *gs, int flags)
{
- size_t i;
+ size_t i = 0;
- for (i = 0; i < gimple_call_num_args (gs); i++)
+ /* Pretty print first arg to certain internal fns. */
+ if (gimple_call_internal_p (gs))
+{
+ const char *const *enums = NULL;
+ unsigned limit = 0;
+
+ switch (gimple_call_internal_fn (gs))
+ {
+ case IFN_UNIQUE:
+#define DEF(X) #X
+ static const char *const unique_args[] = {IFN_UNIQUE_CODES};
+#undef DEF
+ enums = unique_args;
+
+ limit = ARRAY_SIZE (unique_args);
+ break;
+
+ case IFN_GOACC_LOOP:
+#define DEF(X) #X
+ static const char *const loop_args[] = {IFN_GOACC_LOOP_CODES};
+#undef DEF
+ enums = loop_args;
+ limit = ARRAY_SIZE (loop_args);
+ break;
+
+ case IFN_GOACC_REDUCTION:
+#define DEF(X) #X
+ static const char *const reduction_args[]
+ = {IFN_GOACC_REDUCTION_CODES};
+#undef DEF
+ enums = reduction_args;
+ limit = ARRAY_SIZE (reduction_args);
+ break;
+
+ default:
+ break;
+ }
+ if (limit)
+ {
+ tree arg0 = gimple_call_arg (gs, 0);
+ HOST_WIDE_INT v;
+
+ if (TREE_CODE (arg0) == INTEGER_CST
+ && tree_fits_shwi_p (arg0)
+ && (v = tree_to_shwi (arg0)) >= 0 && v < limit)
+ {
+ i++;
+ pp_string (buffer, enums[v]);
+ if (i < gimple_call_num_args (gs))
+ pp_string (buffer, ", ");
+ }
+ }
+}
+
+ for (; i < gimple_call_num_args (gs); i++)
{
dump_generic_node (buffer, gimple_call_arg (gs, i), 0, flags, false);
if (i < gimple_call_num_args (gs) - 1)
Index: internal-fn.h
===
--- internal-fn.h (revision 240525)
+++ internal-fn.h (working copy)
@@ -20,26 +20,28 @@ along with GCC; see the file COPYING3.
#ifndef GCC_INTERNAL_FN_H
#define GCC_INTERNAL_FN_H
-/* INTEGER_CST values for IFN_UNIQUE function arg-0. */
-enum ifn_unique_kind {
- IFN_UNIQUE_UNSPEC, /* Undifferentiated UNIQUE. */
+/* INTEGER_CST values for IFN_UNIQUE function arg-0.
+
+ UNSPEC: Undifferentiated UNIQUE.
- /* FORK and JOIN mark the points at which OpenACC partitioned
- execution is entered or exited.
- return: data dependency value
- arg-1: data dependency var
- arg-2: INTEGER_CST argument, indicating the axis. */
- IFN_UNIQUE_OACC_FORK,
- IFN_UNIQUE_OACC_JOIN,
-
- /* HEAD_MARK and TAIL_MARK are used to demark the sequence entering
- or leaving partitioned execution.
- return: data dependency value
- arg-1: data dependency var
- arg-2: INTEGER_CST argument, remaining markers in this sequence
- arg-3...: varargs on primary header */
- IFN_UNIQUE_OACC_HEAD_MARK,
- IFN_UNIQUE_OACC_TAIL_MARK
+ FORK and JOIN mark the points at which OpenACC partitioned
+ execution is entered or exited.
+ DEP_VAR = UNIQUE ({FORK,JOIN}, DEP_VAR, AXIS)
+
+ HEAD_MARK and TAIL_MARK are used to demark the sequence entering
+ or leaving partitioned execution.
+ DEP_VAR = UNIQUE ({HEAD,TAIL}_MARK, REMAINING_MARKS, ...PRIMARY_FLAGS)
+
+ The PRIMARY_FLAGS only occur on the first HEAD_MARK of a sequence. */
+#define IFN_UNIQUE_CODES \
+ DEF(UNSPEC), \
+DEF(OACC_FORK), DEF(OACC_JOIN), \
+DEF(OACC_HEAD_MARK), DEF(OACC_TAIL_MARK)
+
+enum ifn_unique_kind {
+#define DEF(X) IFN_UNIQUE_##X
+ IFN_UNIQUE_CODES
+#undef DEF
};
/* INTEGER_CST values for IFN_GOACC_LOOP arg-0. Allows the precise
@@ -59,11 +61,12 @@ enum ifn_unique_kind {
CHUNK_NO - chunk number
MASK - partitioning mask. */
+#define IFN_GOACC_LOOP_CODES \
+ DEF(CHUNKS), DEF(STEP), DEF(OFFSET), DEF(BOUND)
enum ifn_goacc_loop_kind {
- IFN_GOACC_LOOP_CHUNKS, /* Number of chunks. */
- IFN_GOACC_LOOP_STEP,/* Size of each thread's step.
Re: [PATCH, Fortran] Extension: COTAN and degree-valued trig intrinsics with -fdec-math
Tobias, Many thanks for the comments. I will adjust the patch according to your advice shortly. - Fritz On Mon, Sep 26, 2016, 11:59 Tobias Burnus wrote: > > Fritz Reese wrote: > > Attached is a patch extending the GNU Fortran front-end to support > > some additional math intrinsics, enabled with a new compile flag > > -fdec-math. The flag adds the COTAN intrinsic (cotangent), as well as > > degree versions of all trigonometric intrinsics (SIND, TAND, ACOSD, > > etc...). > > > > + /* There is no builtin mpc_cot, so compute x = 1 / tan (x). */ > > + val = &result->value.complex; > > + mpc_tan (*val, *val, GFC_MPC_RND_MODE); > > + mpc_div (*val, one, *val, GFC_MPC_RND_MODE); > > The internet remarks: TAN(x) for x -> pi/2 goes to +inf or -inf - while > COTAN(pi/2) == 0. For values near pi/2, using cos(x)/sin(x) should be > numerically better than 1/tan(x). [Cf. mpfr_sin_cos and BUILT_IN_SINCOS.] > > I am not sure how big the prevision difference really is. A quick test > showed results which didn't look to bad: > > implicit none > real(8), volatile :: x > x = 3.14159265358d0/2.0d0 > print '(g0)', cotan(x), cos(x)/sin(x), cotan(x)-cos(x)/sin(x) > end > > yields: > > 0.48965888601467483E-011 > 0.48965888601467475E-011 > 0.80779356694631609E-027 > > Usint N[1/Tan[314159265358/10^11/2],200], Mathematica shows > 4.8966192313216916397514812338... * 10^12. > I am not quite sure whether I should expect that already the 5th digit > differs from the results above. > > > > mpfr_set_d (tmp, 180.0l, GFC_RND_MODE); > > With some fonts the L after 180.0 looks like a one; I was initially > puzzled why one uses not 180 but 180.01. Hence, I'd prefer an "L" instead > of an "l". > > However, the second argument of mpfr_set_d is a "double" and 180.0 is > already a double. Hence, one can even completely get rid of the "l". > > > > Regarding the decimal trigonometric functions, the internet suggests to > use >sin (fmod ((x), 360) * M_PI / 180) > instead of >sin (x * M_PI / 180) > to yield better results for angles which are larger than +/-360 degrees. > > > Cheers, > > Tobias
[PATCH] Fix PR77478
The following backports a correctness fix I slipped in during some
refactoring in the GCC 6 dev phase.
Bootstrap and regtest running on x86_64-unknown-linux-gnu, meanwhile
applied the testcase to trunk and GCC 6.
Richard.
2016-09-27 Richard Biener
PR tree-optimization/77478
* tree-vect-loop-manip.c (vect_gen_niters_for_prolog_loop):
Fix alignment of SSA var used before the alignment prologue.
* gcc.dg/torture/pr77478.c: New testcase.
Index: gcc/tree-vect-loop-manip.c
===
--- gcc/tree-vect-loop-manip.c (revision 240417)
+++ gcc/tree-vect-loop-manip.c (working copy)
@@ -1886,7 +1886,7 @@ vect_gen_niters_for_prolog_loop (loop_ve
gimple_seq new_stmts = NULL;
bool negative = tree_int_cst_compare (DR_STEP (dr), size_zero_node) < 0;
tree offset = negative
- ? size_int (-TYPE_VECTOR_SUBPARTS (vectype) + 1) : NULL_TREE;
+ ? size_int (-TYPE_VECTOR_SUBPARTS (vectype) + 1) : size_zero_node;
tree start_addr = vect_create_addr_base_for_vector_ref (dr_stmt,
&new_stmts, offset, loop);
tree type = unsigned_type_for (TREE_TYPE (start_addr));
Index: gcc/testsuite/gcc.dg/torture/pr77478.c
===
--- gcc/testsuite/gcc.dg/torture/pr77478.c (revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr77478.c (working copy)
@@ -0,0 +1,22 @@
+/* { dg-do run } */
+/* { dg-additional-options "-ffast-math" } */
+
+static const float A[10] = {1};
+
+float
+foo(float *f, int n)
+{
+ int i, j;
+ float a = 0, b = 0;
+ for (i = n/2; i < n; i++)
+a += f[i]*.1f;
+ for (i = n/2, j = 0; i < n; i++, j++)
+b += f[i]*A[j]+a*A[j];
+ return b;
+}
+
+int main()
+{
+ float a[21] = {0};
+ return foo(a+1, 20) + foo(a, 20);
+}
Re: Implement -Wimplicit-fallthrough (version 9)
> The accepted style is already very permissive, we don't allow just one > spelling as various lint tools. Well, it cannot even handle the variations of a single codebase, GCC itself, so I'm afraid very permissive is not exactly the appropriate wording here. Anyway, we'll see whether Bugzilla is flooded or not in the upcoming days. -- Eric Botcazou
Re: [PATCH, RFC] gcov: dump in a static dtor instead of in an atexit handler
On 09/27/2016 12:55 PM, Nathan Sidwell wrote: > > "Instrumented applications use a static destructor with priority 99 to invoke > the __gcov_dump function. Thus __gcov_dump is executed after all > user defined static destructors, as well as handlers registered with atexit." > > ? Hello. I like your wording, I installed the patch as r240529. Martin
Re: [PATCH, RFC] gcov: dump in a static dtor instead of in an atexit handler
On 09/27/16 07:07, Martin Liška wrote: On 09/27/2016 12:55 PM, Nathan Sidwell wrote: "Instrumented applications use a static destructor with priority 99 to invoke the __gcov_dump function. Thus __gcov_dump is executed after all user defined static destructors, as well as handlers registered with atexit." ? Hello. I like your wording, I installed the patch as r240529. thanks!
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 1:06 PM, Eric Botcazou wrote: >> The accepted style is already very permissive, we don't allow just one >> spelling as various lint tools. > > Well, it cannot even handle the variations of a single codebase, GCC itself, > so I'm afraid very permissive is not exactly the appropriate wording here. > Anyway, we'll see whether Bugzilla is flooded or not in the upcoming days. During discussion I already pointed out that people may use non-english variants as well. I've seen a lot of french variable/function names in my academic life for example. Richard. > -- > Eric Botcazou
[Patch, testsuite] Require int32plus for builtin-sprintf-warn-1.c
Hi,
This patch requires int32plus for
gcc.dg/tree-ssa/builtin-sprintf-warn-1.c, as it reports a bunch of
failures for a 16 bit int target like the avr. The "%u" format
specifier tests, for example, use int literals big enough to only fit
in a long int, and this causes unexpected warnings.
Comitted to trunk.
Regards
Senthil
2016-09-27 Senthil Kumar Selvaraj
* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Require int32plus.
PR fortran/77666
Index: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c
===
--- gcc.dg/tree-ssa/builtin-sprintf-warn-1.c(revision 240524)
+++ gcc.dg/tree-ssa/builtin-sprintf-warn-1.c(working copy)
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options "-std=c99 -Wformat -Wformat-length=1
-ftrack-macro-expansion=0" } */
+/* { dg-require-effective-target int32plus } */
/* 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
Re: Implement -Wimplicit-fallthrough (version 9)
On 27/09/16 11:41, Jakub Jelinek wrote: On Tue, Sep 27, 2016 at 11:32:42AM +0100, Kyrill Tkachov wrote: where the code is: 2156 /* Fall through - if the lane index isn't a constant then 2157 the next case will error. */ 2158 2159 case NEON_ARG_CONSTANT: Is there supposed to be no empty line between the case statement and the comment? Or is the comment only supposed to contain "Fall through"? The last comment before case or default keyword (or user label before case/default) has to match one of the following regexps: //-fallthrough$ //@fallthrough@$ //[ \t]*FALL(S | |-)?THR(OUGH|U)\.?[ \t]*$ //[ \t]*Fall(s | |-)?[Tt]hr(ough|u)\.?[ \t]*$ //[ \t]*fall(s | |-)?thr(ough|u)\.?[ \t]*$ /\*-fallthrough\*/ /\*@fallthrough@\*/ /\*[ \t]*FALL(S | |-)?THR(OUGH|U)\.?[ \t]*\*/ /\*[ \t]*Fall(s | |-)?[Tt]hr(ough|u)\.?[ \t]*\*/ /\*[ \t]*fall(s | |-)?thr(ough|u)\.?[ \t]*\*/ So, you could e.g. write: /* If the lane index isn't a constant, then the next case will error. */ /* Fall through. */ but not what you have, free form is not accepted. Thanks. Given the discussion going on about the acceptable comment formats, is it preferable to use comments in the gcc codebase at all, or should I use gcc_fallthrough () (with an explanatory comment if needed)? Kyrill Jakub
[PATCH] Fix PR77745
The following fixes bogus redundant store removal by FRE/PRE and bogus
"store match" detections by VN. This amends the similar PR69776 fix.
Bootstraped on x86_64-unknown-linux-gnu, testing in progress.
2016-09-27 Richard Biener
PR tree-optimization/77745
* tree-ssa-pre.c (eliminate_dom_walker::before_dom_children):
When removing redundant stores make sure to check compatibility
of the TBAA state for downstream accesses.
* tree-ssa-sccvn.c (visit_reference_op_store): Likewise for when
value-numbering virtual operands for store matches.
* g++.dg/torture/pr77745.C: New testcase.
Index: gcc/tree-ssa-pre.c
===
--- gcc/tree-ssa-pre.c (revision 240521)
+++ gcc/tree-ssa-pre.c (working copy)
@@ -4431,26 +4436,34 @@ eliminate_dom_walker::before_dom_childre
&& !is_gimple_reg (gimple_assign_lhs (stmt))
&& (TREE_CODE (gimple_assign_rhs1 (stmt)) == SSA_NAME
|| is_gimple_min_invariant (gimple_assign_rhs1 (stmt
-{
- tree val;
+ {
+ tree val;
tree rhs = gimple_assign_rhs1 (stmt);
- val = vn_reference_lookup (gimple_assign_lhs (stmt),
- gimple_vuse (stmt), VN_WALK, NULL, false);
- if (TREE_CODE (rhs) == SSA_NAME)
-rhs = VN_INFO (rhs)->valnum;
- if (val
- && operand_equal_p (val, rhs, 0))
-{
- if (dump_file && (dump_flags & TDF_DETAILS))
-{
- fprintf (dump_file, "Deleted redundant store ");
- print_gimple_stmt (dump_file, stmt, 0, 0);
-}
+ vn_reference_t vnresult;
+ val = vn_reference_lookup (lhs, gimple_vuse (stmt), VN_WALKREWRITE,
+&vnresult, false);
+ if (TREE_CODE (rhs) == SSA_NAME)
+ rhs = VN_INFO (rhs)->valnum;
+ if (val
+ && operand_equal_p (val, rhs, 0))
+ {
+ /* We can only remove the later store if the former aliases
+at least all accesses the later one does. */
+ alias_set_type set = get_alias_set (lhs);
+ if (vnresult->set == set
+ || alias_set_subset_of (set, vnresult->set))
+ {
+ if (dump_file && (dump_flags & TDF_DETAILS))
+ {
+ fprintf (dump_file, "Deleted redundant store ");
+ print_gimple_stmt (dump_file, stmt, 0, 0);
+ }
- /* Queue stmt for removal. */
- el_to_remove.safe_push (stmt);
- continue;
-}
+ /* Queue stmt for removal. */
+ el_to_remove.safe_push (stmt);
+ continue;
+ }
+ }
}
/* If this is a control statement value numbering left edges
Index: gcc/tree-ssa-sccvn.c
===
--- gcc/tree-ssa-sccvn.c(revision 240521)
+++ gcc/tree-ssa-sccvn.c(working copy)
@@ -3599,13 +3706,21 @@ visit_reference_op_store (tree lhs, tree
Otherwise, the vdefs for the store are used when inserting into
the table, since the store generates a new memory state. */
- result = vn_reference_lookup (lhs, vuse, VN_NOWALK, NULL, false);
-
+ result = vn_reference_lookup (lhs, vuse, VN_NOWALK, &vnresult, false);
if (result)
{
if (TREE_CODE (result) == SSA_NAME)
result = SSA_VAL (result);
resultsame = expressions_equal_p (result, op);
+ if (resultsame)
+ {
+ /* If the TBAA state isn't compatible for downstream reads
+we cannot value-number the VDEFs the same. */
+ alias_set_type set = get_alias_set (lhs);
+ if (vnresult->set != set
+ && ! alias_set_subset_of (set, vnresult->set))
+ resultsame = false;
+ }
}
if ((!result || !resultsame)
Index: gcc/testsuite/g++.dg/torture/pr77745.C
===
--- gcc/testsuite/g++.dg/torture/pr77745.C (revision 0)
+++ gcc/testsuite/g++.dg/torture/pr77745.C (working copy)
@@ -0,0 +1,24 @@
+// { dg-do run }
+
+inline void* operator new(__SIZE_TYPE__, void* __p) noexcept { return __p; }
+
+long foo(char *c1, char *c2)
+{
+ long *p1 = new (c1) long;
+ *p1 = 100;
+ long long *p2 = new (c2) long long;
+ *p2 = 200;
+ long *p3 = new (c2) long;
+ *p3 = 200;
+ return *p1;
+}
+int main()
+{
+ union {
+ char c;
+ long l;
+ long long ll;
+ } c;
+ if (foo(&c.c, &c.c) != 200)
+__builtin_abort();
+}
Re: [PATCH] objc: update documetation and add test-case of constructor/destructor attr.
ping^1, CC'ing objective C maintainers. Thanks, Martin On 08/10/2016 11:11 AM, Martin Liška wrote: > Hi. > > Following patch clarifies usage of ctor and dtor attributes for Objective C. > Patch survives (on x86_64-linux-gnu): > > make -k check-objc RUNTESTFLAGS="execute.exp" > > Ready for trunk? > Thanks, > Martin >
Re: [PATCH] [RTEMS] Always use atomic builtins for libstdc++
On 22/09/16 12:55, Jonathan Wakely wrote: [...] N.B. if you're going to require libatomic for RTEMS then you should check if the preprocessor conditions in libsupc++/exception_ptr.h and libsupc++/eh_ptr.cc are appropriate. If exception_ptr is not currently enabled for RTEMS then you could enable it, since libatomic will ensure the required atomics are available. Thanks for the hint. I added a ticket for RTEMS: http://devel.rtems.org/ticket/2791 -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.hu...@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
[PATCH] Fix PR gcov-profile/46266
Following patch prevents emission of gcno file (notes file) for statements
that do not point to original source file, like:
$ echo "int main(){}" > x.c
In this case the location points to a builtin.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Martin
>From 0b33d288ecad252d00ef22fd72a1b1f12c0642ff Mon Sep 17 00:00:00 2001
From: marxin
Date: Mon, 8 Aug 2016 17:38:07 +0200
Subject: [PATCH] Fix PR gcov-profile/46266
gcc/ChangeLog:
2016-08-12 Martin Liska
PR gcov-profile/46266
* gimple.h (gimple_has_not_reserved_location): New function.
* input.h (RESERVED_LOCATION_P): New macro.
* profile.c (branch_prob): Use RESERVED_LOCATION_P and
gimple_has_not_reserved_location instread of comparison
with UNKNOWN_LOCATION.
---
gcc/gimple.h | 8
gcc/input.h | 2 ++
gcc/profile.c | 9 -
3 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/gcc/gimple.h b/gcc/gimple.h
index 9fad15b..4982aed 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -1792,6 +1792,14 @@ gimple_has_location (const gimple *g)
return LOCATION_LOCUS (gimple_location (g)) != UNKNOWN_LOCATION;
}
+/* Return true if G has a location that is not any from reserved. */
+
+static inline bool
+gimple_has_not_reserved_location (const gimple *g)
+{
+ return !RESERVED_LOCATION_P (gimple_location (g));
+}
+
/* Return the file name of the location of STMT. */
diff --git a/gcc/input.h b/gcc/input.h
index fe80605..6ce0b81 100644
--- a/gcc/input.h
+++ b/gcc/input.h
@@ -61,6 +61,8 @@ extern location_t input_location;
#define LOCATION_BLOCK(LOC) \
((tree) ((IS_ADHOC_LOC (LOC)) ? get_data_from_adhoc_loc (line_table, (LOC)) \
: NULL))
+#define RESERVED_LOCATION_P(LOC) \
+ (LOCATION_LOCUS (LOC) < RESERVED_LOCATION_COUNT)
/* Return a positive value if LOCATION is the locus of a token that is
located in a system header, O otherwise. It returns 1 if LOCATION
diff --git a/gcc/profile.c b/gcc/profile.c
index 791225b..ceb5f29 100644
--- a/gcc/profile.c
+++ b/gcc/profile.c
@@ -1042,7 +1042,7 @@ branch_prob (void)
gsi_prev_nondebug (&gsi))
{
last = gsi_stmt (gsi);
- if (gimple_has_location (last))
+ if (gimple_has_not_reserved_location (last))
break;
}
@@ -1053,7 +1053,7 @@ branch_prob (void)
is not computed twice. */
if (last
&& gimple_has_location (last)
- && LOCATION_LOCUS (e->goto_locus) != UNKNOWN_LOCATION
+ && !RESERVED_LOCATION_P (e->goto_locus)
&& !single_succ_p (bb)
&& (LOCATION_FILE (e->goto_locus)
!= LOCATION_FILE (gimple_location (last))
@@ -1262,15 +1262,14 @@ branch_prob (void)
for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
{
gimple *stmt = gsi_stmt (gsi);
- if (gimple_has_location (stmt))
+ if (gimple_has_not_reserved_location (stmt))
output_location (gimple_filename (stmt), gimple_lineno (stmt),
&offset, bb);
}
/* Notice GOTO expressions eliminated while constructing the CFG. */
if (single_succ_p (bb)
- && LOCATION_LOCUS (single_succ_edge (bb)->goto_locus)
- != UNKNOWN_LOCATION)
+ && !RESERVED_LOCATION_P (single_succ_edge (bb)->goto_locus))
{
expanded_location curr_location
= expand_location (single_succ_edge (bb)->goto_locus);
--
2.9.2
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 12:56:29PM +0200, Marek Polacek wrote: > On Tue, Sep 27, 2016 at 12:47:50PM +0200, Jakub Jelinek wrote: > > On Tue, Sep 27, 2016 at 12:39:41PM +0200, Markus Trippelsdorf wrote: > > > On 2016.09.27 at 10:46 +0200, Eric Botcazou wrote: > > > > > The intent has been that we catch the most common forms, but still > > > > > require > > > > > it not to be complete free form. Because, as experience shows, > > > > > people are > > > > > extremely creative in these comments, and it is not very good idea to > > > > > support everything. For ... fall through ... , what is the purpose of > > > > > those ...s? > > > > > > > > No idea, but it has been there for a while and seems perfectly > > > > reasonable. > > > > IMO any sentence containing "fall" and "through/thru/etc" on the same > > > > line > > > > should be accepted, otherwise it's just misplaced pickiness. > > > > > > +1. Folks will just disable the warning if gcc is not very permissive > > > when paring existing comments. You cannot expect anyone to change > > > perfectly fine fall-through comments just to accommodate an arbitrary > > > gcc style. > > > > The accepted style is already very permissive, we don't allow just one > > spelling as various lint tools. I'm afraid looking for various cases of > > fall and through/thru possibly separated by anything and surrounded by > > anything is IMHO already too much, the compiler shouldn't try to try to > > grammar analyze the comments on what they actually talk about and whether it > > might be related to the switch fall through or something completely > > different. Users should start using [[fallthrough]]; anyway. > > I'm thinking perhaps we should also accept /* ... fall through ... */ > and /* else fall through */, but accepting any sentence containing "fall" and > "through/thru/etc" on the same line would mean that we also accept > /* Don't fall through here. */ and that is clearly not desirable. I think it is important to think in terms of what regexps we still want to match, even when the matching is actually implemented in C, not using regexps. And yes, you list one reason why arbitrary text with fall and through somewhere in it is not a good idea. Another: /* XXX Really fallthru? */ (what we have in pch.c). So, if you want to allow ... fall through ... and else fall through, and perhaps // fall through - some explanation then it might be e.g. //-fallthrough$ //@fallthrough@$ /\*-fallthrough\*/ /\*@fallthrough@\*/ //[ \t.]*(ELSE )?FALL(S | |-)?THR(OUGH|U)[ \t.]*(-.*)?$ //[ \t.]*(Else )?Fall(s | |-)?[Tt]hr(ough|u)[ \t.]*(-.*)?$ //[ \t.]*(else )?fall(s | |-)?thr(ough|u)[ \t.]*(-.*)?$ /\*[ \t.]*(ELSE )?FALL(S | |-)?THR(OUGH|U)[ \t.]*(-.*)?\*/ /\*[ \t.]*(Else )?Fall(s | |-)?[Tt]hr(ough|u)[ \t.]*(-.*)?\*/ /\*[ \t.]*(else )?fall(s | |-)?thr(ough|u)[ \t.]*(-.*)?\*/ where . would match even newlines in the last 3, but $ would always match just end of line? Jakub
Re: [PATCH] Fix PR gcov-profile/46266
On 09/27/16 07:26, Martin Liška wrote:
Following patch prevents emission of gcno file (notes file) for statements
that do not point to original source file, like:
$ echo "int main(){}" > x.c
In this case the location points to a builtin.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
I'm sure this is sensible. Is there a difficulty with a testcase -- what exactly
is the failure mode?
One thing I dislike is negated predicates though -- I think I'd find
if (!gimple_has_reserved_location (gs))
to be more understandable (particularly as that matches the sense of
RESERVED_LOCATION_P.
Richard?
nathan
Re: Implement -Wimplicit-fallthrough (version 9)
On 2016.09.27 at 12:56 +0200, Marek Polacek wrote:
> On Tue, Sep 27, 2016 at 12:47:50PM +0200, Jakub Jelinek wrote:
> > On Tue, Sep 27, 2016 at 12:39:41PM +0200, Markus Trippelsdorf wrote:
> > > On 2016.09.27 at 10:46 +0200, Eric Botcazou wrote:
> > > > > The intent has been that we catch the most common forms, but still
> > > > > require
> > > > > it not to be complete free form. Because, as experience shows,
> > > > > people are
> > > > > extremely creative in these comments, and it is not very good idea to
> > > > > support everything. For ... fall through ... , what is the purpose of
> > > > > those ...s?
> > > >
> > > > No idea, but it has been there for a while and seems perfectly
> > > > reasonable.
> > > > IMO any sentence containing "fall" and "through/thru/etc" on the same
> > > > line
> > > > should be accepted, otherwise it's just misplaced pickiness.
> > >
> > > +1. Folks will just disable the warning if gcc is not very permissive
> > > when paring existing comments. You cannot expect anyone to change
> > > perfectly fine fall-through comments just to accommodate an arbitrary
> > > gcc style.
> >
> > The accepted style is already very permissive, we don't allow just one
> > spelling as various lint tools. I'm afraid looking for various cases of
> > fall and through/thru possibly separated by anything and surrounded by
> > anything is IMHO already too much, the compiler shouldn't try to try to
> > grammar analyze the comments on what they actually talk about and whether it
> > might be related to the switch fall through or something completely
> > different. Users should start using [[fallthrough]]; anyway.
>
> I'm thinking perhaps we should also accept /* ... fall through ... */
> and /* else fall through */, but accepting any sentence containing "fall" and
> "through/thru/etc" on the same line would mean that we also accept
> /* Don't fall through here. */ and that is clearly not desirable.
>
I'm also wondering about the situation where not a single break is used
in all of the cases. It would be best not to warn here.
An example from ffmpeg:
#define LPC1(x) { \
int c = coefs[(x)-1]; \
p0 += MUL(c, s); \
s = smp[i-(x)+1]; \
p1 += MUL(c, s); \
}
static av_always_inline void FUNC(lpc_encode_unrolled)(int32_t *res,
const int32_t *smp, int len, int order,
const int32_t *coefs, int shift, int big)
{
int i;
for (i = order; i < len; i += 2) {
int s = smp[i-order];
sum_type p0 = 0, p1 = 0;
if (big) {
switch (order) {
case 32: LPC1(32)
case 31: LPC1(31)
case 30: LPC1(30)
case 29: LPC1(29)
case 28: LPC1(28)
case 27: LPC1(27)
case 26: LPC1(26)
case 25: LPC1(25)
case 24: LPC1(24)
case 23: LPC1(23)
case 22: LPC1(22)
case 21: LPC1(21)
case 20: LPC1(20)
case 19: LPC1(19)
case 18: LPC1(18)
case 17: LPC1(17)
case 16: LPC1(16)
case 15: LPC1(15)
case 14: LPC1(14)
case 13: LPC1(13)
case 12: LPC1(12)
case 11: LPC1(11)
case 10: LPC1(10)
case 9: LPC1( 9)
LPC1( 8)
LPC1( 7)
LPC1( 6)
LPC1( 5)
LPC1( 4)
LPC1( 3)
LPC1( 2)
LPC1( 1)
}
} else {
switch (order) {
case 8: LPC1( 8)
case 7: LPC1( 7)
case 6: LPC1( 6)
case 5: LPC1( 5)
case 4: LPC1( 4)
case 3: LPC1( 3)
case 2: LPC1( 2)
case 1: LPC1( 1)
}
}
res[i ] = smp[i ] - CLIP(p0 >> shift);
res[i+1] = smp[i+1] - CLIP(p1 >> shift);
}
}
--
Markus
Re: Implement -Wimplicit-fallthrough (version 9)
On 09/27/2016 01:09 PM, Richard Biener wrote: On Tue, Sep 27, 2016 at 1:06 PM, Eric Botcazou wrote: The accepted style is already very permissive, we don't allow just one spelling as various lint tools. Well, it cannot even handle the variations of a single codebase, GCC itself, so I'm afraid very permissive is not exactly the appropriate wording here. Anyway, we'll see whether Bugzilla is flooded or not in the upcoming days. During discussion I already pointed out that people may use non-english variants as well. I've seen a lot of french variable/function names in my academic life for example. Yes, I pointed out the same thing a few weeks ago. The warning does seem to be useful and discover errors, but I worry about the large amount of false positives it produces. Bernd
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 01:31:15PM +0200, Jakub Jelinek wrote: > I think it is important to think in terms of what regexps we still want to > match, even when the matching is actually implemented in C, not using > regexps. And yes, you list one reason why arbitrary text with fall and > through somewhere in it is not a good idea. Another: > /* XXX Really fallthru? */ > (what we have in pch.c). > So, if you want to allow ... fall through ... and else fall through, and > perhaps // fall through - some explanation > then it might be e.g. > //-fallthrough$ > //@fallthrough@$ > /\*-fallthrough\*/ > /\*@fallthrough@\*/ > //[ \t.]*(ELSE )?FALL(S | |-)?THR(OUGH|U)[ \t.]*(-.*)?$ > //[ \t.]*(Else )?Fall(s | |-)?[Tt]hr(ough|u)[ \t.]*(-.*)?$ > //[ \t.]*(else )?fall(s | |-)?thr(ough|u)[ \t.]*(-.*)?$ > /\*[ \t.]*(ELSE )?FALL(S | |-)?THR(OUGH|U)[ \t.]*(-.*)?\*/ > /\*[ \t.]*(Else )?Fall(s | |-)?[Tt]hr(ough|u)[ \t.]*(-.*)?\*/ > /\*[ \t.]*(else )?fall(s | |-)?thr(ough|u)[ \t.]*(-.*)?\*/ > where . would match even newlines in the last 3, > but $ would always match just end of line? Any comment with text ^[^_[:alnum:]]*(else )?fall(s | |-)?thr(ough|u)[^_[:alnum:]]*$ perhaps? Case-insensitive. Or allow any amount of space, or even any interpunction. Just don't allow any alphanumerics except for those exact words, and there won't be many false hits at all. Segher
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 01:47:07PM +0200, Bernd Schmidt wrote: > On 09/27/2016 01:09 PM, Richard Biener wrote: > > On Tue, Sep 27, 2016 at 1:06 PM, Eric Botcazou > > wrote: > > > > The accepted style is already very permissive, we don't allow just one > > > > spelling as various lint tools. > > > > > > Well, it cannot even handle the variations of a single codebase, GCC > > > itself, > > > so I'm afraid very permissive is not exactly the appropriate wording here. > > > Anyway, we'll see whether Bugzilla is flooded or not in the upcoming days. > > > > During discussion I already pointed out that people may use non-english > > variants as well. I've seen a lot of french variable/function names in my > > academic life for example. > > Yes, I pointed out the same thing a few weeks ago. But the C/C++ keywords are all English, too; lint tools only accept English, and so it wouldn't seem unreasonable to only accept English keywords in the comments. And in any case, I don't see how a compiler can be expected to be able to parse non-English languages. Marek
Re: [PATCH RESEND 2/2] Extend -falign-FOO=N to N[,M[,N2[,M2]]]
On 09/26/2016 09:08 PM, Denys Vlasenko wrote:
+@gccoptlist{-faggressive-loop-optimizations @gol
+-falign-functions[=@var{n}[,@var{m},[@var{n}[,@var{m} @gol
+-falign-jumps[=@var{n}[,@var{m}]] @gol
+-falign-labels[=@var{n}[,@var{m}]] -falign-loops[=@var{n}[,@var{m}]] @gol
@itemx -falign-functions=@var{n}
+@itemx -falign-functions=@var{n},@var{m}
@opindex falign-functions
+If @var{m} is not specified, it defaults to @var{n}.
There are inconsistencies here about how many arguments these take.
There's no documentation of what it would mean to have more than two.
The first paragraph seems to imply it's only allowed for
-falign-functions, but the same implementation is used for all three.
+#if defined (__i386__) || defined (__x86_64__)
+ /* Before -falign-foo=N,M,N2,M2 was introduced, x86 had a tweak.
+-falign-functions=N with N > 8 was adding secondary alignment.
+-falign-functions=10 was emitting this before every function:
+ .p2align 4,,9
+ .p2align 3
+Now this behavior (and more) can be explicitly requested:
+-falign-functions=16,10,8
+Retain old behavior if N2 is missing: */
+ else if (a[0].log > 3)
+ a[1].log = 3;
+#endif
Can't have such #ifdef blocks in generic code. To start with, this
changes behaviour based on the host, when you want it to change
depending on the target. If there's no way to detect such a situation
from the x86 backend, such as in the option_override function, then
you'll need a hook.
IIUC the intention for the whole patch is that behaviour is unchanged by
default, but there are additional options for users to choose?
Since it seems this is mostly for x86, maybe Uros should have a say in
whether this patch is a good idea or not. Maybe it would be good to also
pursue the idea of taking function size into account?
Bernd
Re: Implement -Wimplicit-fallthrough (version 9)
On 09/27/2016 01:51 PM, Marek Polacek wrote: But the C/C++ keywords are all English, too; lint tools only accept English, and so it wouldn't seem unreasonable to only accept English keywords in the comments. And in any case, I don't see how a compiler can be expected to be able to parse non-English languages. It isn't. But it can also be reasonably by expected not to warn about things that are valid according to the language specification and are frequently used. Bernd
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 06:51:31AM -0500, Segher Boessenkool wrote: > On Tue, Sep 27, 2016 at 01:31:15PM +0200, Jakub Jelinek wrote: > > I think it is important to think in terms of what regexps we still want to > > match, even when the matching is actually implemented in C, not using > > regexps. And yes, you list one reason why arbitrary text with fall and > > through somewhere in it is not a good idea. Another: > > /* XXX Really fallthru? */ > > (what we have in pch.c). > > So, if you want to allow ... fall through ... and else fall through, and > > perhaps // fall through - some explanation > > then it might be e.g. > > //-fallthrough$ > > //@fallthrough@$ > > /\*-fallthrough\*/ > > /\*@fallthrough@\*/ > > //[ \t.]*(ELSE )?FALL(S | |-)?THR(OUGH|U)[ \t.]*(-.*)?$ > > //[ \t.]*(Else )?Fall(s | |-)?[Tt]hr(ough|u)[ \t.]*(-.*)?$ > > //[ \t.]*(else )?fall(s | |-)?thr(ough|u)[ \t.]*(-.*)?$ > > /\*[ \t.]*(ELSE )?FALL(S | |-)?THR(OUGH|U)[ \t.]*(-.*)?\*/ > > /\*[ \t.]*(Else )?Fall(s | |-)?[Tt]hr(ough|u)[ \t.]*(-.*)?\*/ > > /\*[ \t.]*(else )?fall(s | |-)?thr(ough|u)[ \t.]*(-.*)?\*/ > > where . would match even newlines in the last 3, > > but $ would always match just end of line? > > Any comment with text > > ^[^_[:alnum:]]*(else )?fall(s | |-)?thr(ough|u)[^_[:alnum:]]*$ > > perhaps? Case-insensitive. Or allow any amount of space, or even any > interpunction. Just don't allow any alphanumerics except for those > exact words, and there won't be many false hits at all. Not sure we want to match FaLlS THrouGH, and [^_[:alnum:]]* isn't without a problem either, what if there is hebrew, or chinese, etc. text in there? The matching shouldn't depend on the current locale IMHO, and figuring out what unicode entry points are letters and which are not really isn't easy without that. IMO before changing anything further, we want to gather some statistics what styles are actually used in the wild together with how often they are used, and then for the more common ones decide what is really supportable. Jakub
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 01:55:22PM +0200, Bernd Schmidt wrote: > On 09/27/2016 01:51 PM, Marek Polacek wrote: > > But the C/C++ keywords are all English, too; lint tools only accept English, > > and so it wouldn't seem unreasonable to only accept English keywords in the > > comments. And in any case, I don't see how a compiler can be expected to > > be able to parse non-English languages. > > It isn't. But it can also be reasonably by expected not to warn about things > that are valid according to the language specification and are frequently > used. Ok, but note that the warning is in -Wextra, not enabled by default/-Wall. I'm all for reducing false positives whenever possible and we can improve out comment-parsing heuristics, but I just can't see us handling anything other than English. Marek
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 01:46:08PM +0200, Markus Trippelsdorf wrote: > I'm also wondering about the situation where not a single break is used > in all of the cases. It would be best not to warn here. This is tricky and I'm afraid all I can offer here is to use the diagnostics pragma to suppress the warning for Duff's device-like constructs. Marek
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 12:12:30PM +0100, Kyrill Tkachov wrote: > > On 27/09/16 11:41, Jakub Jelinek wrote: > > On Tue, Sep 27, 2016 at 11:32:42AM +0100, Kyrill Tkachov wrote: > > > where the code is: > > > 2156 /* Fall through - if the lane index isn't a constant > > > then > > > 2157 the next case will error. */ > > > 2158 > > > 2159 case NEON_ARG_CONSTANT: > > > > > > > > > Is there supposed to be no empty line between the case statement and the > > > comment? > > > Or is the comment only supposed to contain "Fall through"? > > The last comment before case or default keyword (or user label before > > case/default) has to match one of the following regexps: > > //-fallthrough$ > > //@fallthrough@$ > > //[ \t]*FALL(S | |-)?THR(OUGH|U)\.?[ \t]*$ > > //[ \t]*Fall(s | |-)?[Tt]hr(ough|u)\.?[ \t]*$ > > //[ \t]*fall(s | |-)?thr(ough|u)\.?[ \t]*$ > > /\*-fallthrough\*/ > > /\*@fallthrough@\*/ > > /\*[ \t]*FALL(S | |-)?THR(OUGH|U)\.?[ \t]*\*/ > > /\*[ \t]*Fall(s | |-)?[Tt]hr(ough|u)\.?[ \t]*\*/ > > /\*[ \t]*fall(s | |-)?thr(ough|u)\.?[ \t]*\*/ > > > > So, you could e.g. write: > > /* If the lane index isn't a constant, then the next case will error. > > */ > > /* Fall through. */ > > but not what you have, free form is not accepted. > Thanks. Given the discussion going on about the acceptable comment formats, > is it preferable to use comments in the gcc codebase at all, or should I > use gcc_fallthrough () (with an explanatory comment if needed)? It's probably that the comments are preferable, but sometimes you can't use them (if e.g. something like CASE_CONVERT or another comment or } follows). Marek
Re: Implement -Wimplicit-fallthrough (version 9)
* Marek Polacek: > On Tue, Sep 27, 2016 at 01:46:08PM +0200, Markus Trippelsdorf wrote: >> I'm also wondering about the situation where not a single break is used >> in all of the cases. It would be best not to warn here. > > This is tricky and I'm afraid all I can offer here is to use the diagnostics > pragma to suppress the warning for Duff's device-like constructs. Would it make sense to apply the fallthrough attribute to the entire switch statement to address such scenarios? Currently, that does not seem supported.
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 02:27:12PM +0200, Florian Weimer wrote: > * Marek Polacek: > > > On Tue, Sep 27, 2016 at 01:46:08PM +0200, Markus Trippelsdorf wrote: > >> I'm also wondering about the situation where not a single break is used > >> in all of the cases. It would be best not to warn here. > > > > This is tricky and I'm afraid all I can offer here is to use the diagnostics > > pragma to suppress the warning for Duff's device-like constructs. > > Would it make sense to apply the fallthrough attribute to the entire > switch statement to address such scenarios? Currently, that does not > seem supported. Where the attribute is allowed or not allowed is currently intentionally derived from where C++17 allows it. Jakub
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 02:27:12PM +0200, Florian Weimer wrote: > * Marek Polacek: > > > On Tue, Sep 27, 2016 at 01:46:08PM +0200, Markus Trippelsdorf wrote: > >> I'm also wondering about the situation where not a single break is used > >> in all of the cases. It would be best not to warn here. > > > > This is tricky and I'm afraid all I can offer here is to use the diagnostics > > pragma to suppress the warning for Duff's device-like constructs. > > Would it make sense to apply the fallthrough attribute to the entire > switch statement to address such scenarios? Currently, that does not > seem supported. I've been thinking about this, too. But I think we'd have to invent a new attribute, e.g. no_warn_fallthrough or so. Marek
Re: [PATCH] Make -Wint-in-bool-context warn on suspicious shift ops
On Sun, Sep 25, 2016 at 3:46 AM, Bernd Edlinger wrote: > This patch makes -Wint-in-bool-context warn on suspicious integer left > shifts, when the integer is signed, which is most likely some kind of > programming error, for instance using "<<" instead of "<". > > The warning is motivated by the fact, that an overflow on integer shift > left is undefined behavior, even if gcc won't optimize the shift based > on the undefined behavior. > > So in absence of undefined behavior the boolean result does not depend > on the shift value, thus the whole shifting is pointless. It's pointless for unsigned integers, too; why not warn for them as well? And why not warn for 0 << 0 and 1 << 0, which are just as pointless? Jason
Re: [Patch, fortran] PR69834 - Collision in derived type hashes
Dear All, After submitting the patch, I did something that I should have done a long time ago: some timing tests :-) I used actual_array_offset_1.f90, which is based on Arjen Markus's implementation of quicksort using unlimited polymorphic entities as carriers of the objects to be sorted. With 10^5 elements in the array and -O3, without the patch, the execution time is 49ms. With the patch it climbs to 315ms Dominique repeated the test with 10^7 elements and got 4.4s before the patch and 46.5 after. In light of this, I withdraw the submission and will concentrate on making the pointer version work in all circumstances with submodules. Best regards Paul On 27 September 2016 at 10:27, Paul Richard Thomas wrote: > Dear All, > > The first attempts at fixing this bug were posted to the PR in > February of this year. Since then, real life has intervened and I have > not been able to get back to it until now. > > The first patch used the address of the vtable to perform the > switching in SELECT_TYPE. Unfortunately, it failed in submodule_6.f90 > and I have not been able to find a way to fix this without breaking > the ABI and having to bump up the module version number. > > The second patch uses a string for the switching, which comprises a > concatenation of the type name and the module or procedure name. > Clearly, there is a performance penalty associated with this. My > recent efforts have been focussed on making this version detect > incoming selectors and associates that are use associated with > libraries that were compiled before this patch was applied and the > result is this submission. By the way, I was unable to find a way of > testing this feature as part of the testsuite but have done so 'by > hand'. > > If the performance penalty is considered to be a show stopper, I could > develop further the version based on the vtable addresses but will > have to postpone any further work on this for a few weeks. > > Otherwise, this patch does bootstrap and regtest on FC21/x86_64 - OK for > trunk? > > Cheers > > Paul > > 2016-09-27 Paul Thomas > > PR fortran/69834 > * class.c (get_unique_type_string): Add an extra argument > 'icase' that defaults to false but, when true, switches the > order of type name and module or procedure name. > (get_unique_hashed_string): New argument 'icase' switches > bewteen the old form and a new one in which the string length > is limited to GFC_MAX_SYMBOL_LEN and, in case of this limit > being exceeded, the hash string is followed by as much of the > composite name as possible. > (gfc_case_name): New function. > (gfc_find_derived_vtab): Add '_name' field to vtable. This is > initialized by 'get_unique_type_string' with 'icase' true. > (find_intrinsic_vtab): Ditto with initialization performed by a > call to 'gfc_case_name'. > * gfortran.h : Add macro 'gfc_add_name_component' and prototype > for 'gfc_case_name'. > * resolve.c (vtable_old_style): New function to determine if a > use associated vtable is missing the '_name' field. > (resolve_select_type): Call 'vtable_old_style' to determine if > any of the derived types or vtables come from a library that > was compiled before this patch. If this is the case, the old > form of SELECT TYPE is activated, in which the cases are set by > the hash value. Otherwise, the 'unique_type_string' is used. > > 2016-09-27 Paul Thomas > > PR fortran/69834 > * gfortran.dg/finalize_21.f90: Remove semi colon from the tree > scan. > * gfortran.dg/select_type_36.f03: New test. > * gfortran.dg/select_type_37.f03: New test. -- The difference between genius and stupidity is; genius has its limits. Albert Einstein
Re: [PATCH] Fix PR gcov-profile/46266
On 09/27/2016 01:32 PM, Nathan Sidwell wrote:
> On 09/27/16 07:26, Martin Liška wrote:
>> Following patch prevents emission of gcno file (notes file) for statements
>> that do not point to original source file, like:
>>
>> $ echo "int main(){}" > x.c
>>
>> In this case the location points to a builtin.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
>
> I'm sure this is sensible. Is there a difficulty with a testcase -- what
> exactly is the failure mode?
Second version of the patch adds validation to gcov.exp, where $result is
scanned for "File ''".
Luckily current test-case hit that verification:
FAIL: gcc.misc-tests/gcov-6.c gcov failed: .gcov should not be created
FAIL: gcc.misc-tests/gcov-7.c gcov failed: .gcov should not be created
>
> One thing I dislike is negated predicates though -- I think I'd find
>if (!gimple_has_reserved_location (gs))
> to be more understandable (particularly as that matches the sense of
> RESERVED_LOCATION_P.
Agree with you, renamed to gimple_has_reserved_location.
Ready with that change?
Martin
>
> Richard?
>
> nathan
>
Re: [PATCH] Fix PR gcov-profile/46266
On 09/27/2016 02:45 PM, Martin Liška wrote:
> On 09/27/2016 01:32 PM, Nathan Sidwell wrote:
>> On 09/27/16 07:26, Martin Liška wrote:
>>> Following patch prevents emission of gcno file (notes file) for statements
>>> that do not point to original source file, like:
>>>
>>> $ echo "int main(){}" > x.c
>>>
>>> In this case the location points to a builtin.
>>>
>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>>
>>> Ready to be installed?
>>
>> I'm sure this is sensible. Is there a difficulty with a testcase -- what
>> exactly is the failure mode?
>
> Second version of the patch adds validation to gcov.exp, where $result is
> scanned for "File ''".
> Luckily current test-case hit that verification:
>
> FAIL: gcc.misc-tests/gcov-6.c gcov failed: .gcov should not be
> created
> FAIL: gcc.misc-tests/gcov-7.c gcov failed: .gcov should not be
> created
>
>>
>> One thing I dislike is negated predicates though -- I think I'd find
>>if (!gimple_has_reserved_location (gs))
>> to be more understandable (particularly as that matches the sense of
>> RESERVED_LOCATION_P.
>
> Agree with you, renamed to gimple_has_reserved_location.
> Ready with that change?
>
> Martin
>
>>
>> Richard?
>>
>> nathan
>>
>
Adding missing patch.
M.
>From a456386416c0d4d5a6acd8755ff81c601af01c9c Mon Sep 17 00:00:00 2001
From: marxin
Date: Mon, 8 Aug 2016 17:38:07 +0200
Subject: [PATCH] Fix PR gcov-profile/46266
gcc/testsuite/ChangeLog:
2016-09-27 Martin Liska
* lib/gcov.exp: Verify that .gcov file is not
considered.
gcc/ChangeLog:
2016-09-27 Martin Liska
* gimple.h (gimple_has_reserved_location): New function.
* input.h (RESERVED_LOCATION_P): New macro.
* profile.c (branch_prob): Use RESERVED_LOCATION_P and
gimple_has_reserved_location instread of comparison
with UNKNOWN_LOCATION.
---
gcc/gimple.h | 8
gcc/input.h| 2 ++
gcc/profile.c | 9 -
gcc/testsuite/lib/gcov.exp | 9 -
4 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/gcc/gimple.h b/gcc/gimple.h
index 9fad15b..f6dab77 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -1792,6 +1792,14 @@ gimple_has_location (const gimple *g)
return LOCATION_LOCUS (gimple_location (g)) != UNKNOWN_LOCATION;
}
+/* Return true if G has a location that is any from reserved. */
+
+static inline bool
+gimple_has_reserved_location (const gimple *g)
+{
+ return RESERVED_LOCATION_P (gimple_location (g));
+}
+
/* Return the file name of the location of STMT. */
diff --git a/gcc/input.h b/gcc/input.h
index fe80605..6ce0b81 100644
--- a/gcc/input.h
+++ b/gcc/input.h
@@ -61,6 +61,8 @@ extern location_t input_location;
#define LOCATION_BLOCK(LOC) \
((tree) ((IS_ADHOC_LOC (LOC)) ? get_data_from_adhoc_loc (line_table, (LOC)) \
: NULL))
+#define RESERVED_LOCATION_P(LOC) \
+ (LOCATION_LOCUS (LOC) < RESERVED_LOCATION_COUNT)
/* Return a positive value if LOCATION is the locus of a token that is
located in a system header, O otherwise. It returns 1 if LOCATION
diff --git a/gcc/profile.c b/gcc/profile.c
index 791225b..3c00077 100644
--- a/gcc/profile.c
+++ b/gcc/profile.c
@@ -1042,7 +1042,7 @@ branch_prob (void)
gsi_prev_nondebug (&gsi))
{
last = gsi_stmt (gsi);
- if (gimple_has_location (last))
+ if (!gimple_has_reserved_location (last))
break;
}
@@ -1053,7 +1053,7 @@ branch_prob (void)
is not computed twice. */
if (last
&& gimple_has_location (last)
- && LOCATION_LOCUS (e->goto_locus) != UNKNOWN_LOCATION
+ && !RESERVED_LOCATION_P (e->goto_locus)
&& !single_succ_p (bb)
&& (LOCATION_FILE (e->goto_locus)
!= LOCATION_FILE (gimple_location (last))
@@ -1262,15 +1262,14 @@ branch_prob (void)
for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
{
gimple *stmt = gsi_stmt (gsi);
- if (gimple_has_location (stmt))
+ if (!gimple_has_reserved_location (stmt))
output_location (gimple_filename (stmt), gimple_lineno (stmt),
&offset, bb);
}
/* Notice GOTO expressions eliminated while constructing the CFG. */
if (single_succ_p (bb)
- && LOCATION_LOCUS (single_succ_edge (bb)->goto_locus)
- != UNKNOWN_LOCATION)
+ && !RESERVED_LOCATION_P (single_succ_edge (bb)->goto_locus))
{
expanded_location curr_location
= expand_location (single_succ_edge (bb)->goto_locus);
diff --git a/gcc/testsuite/lib/gcov.exp b/gcc/testsuite/lib/gcov.exp
index 02bc6b9..91f14e2 100644
--- a/gcc/testsuite/lib/gcov.exp
+++ b/gcc/testsuite/lib/gcov.exp
@@ -364,13 +364,20 @@ proc run-gcov { args } {
return
}
+set builtin_index [string first "File ''" $result]
+if { $builtin_index != -1 } {
+fail "$testname gcov failed: .gcov should not be created"
+clean-gcov $testcase
+return
+}
+
# Get the gcov output file after making sure it exists.
set files [glob
Re: [PATCH] objc: update documetation and add test-case of constructor/destructor attr.
> On 10 Aug 2016, at 19:53, Sandra Loosemore wrote: > > On 08/10/2016 03:11 AM, Martin Liška wrote: >> Hi. >> >> Following patch clarifies usage of ctor and dtor attributes for Objective C. >> Patch survives (on x86_64-linux-gnu): >> >> make -k check-objc RUNTESTFLAGS="execute.exp" >> >> Ready for trunk? > > The documentation fix looks fine, but probably an objc maintainer needs to > confirm that it's not just an accident that the test case works. Appologies; traveling this week so won’t be able to give this proper attention until I get back to the office (Mike might have something to add however). I don’t believe it’s an accident that the test-case works. Objective-C / Objective-C++ are supposed to be supersets of the parent language, and the test case is pure C compiled with an Objective-C FE (so the underlying C should ‘just work’). What I don’t expect to be supported is to try to apply that attribute to any Objective-C entity (but I would like to qualify that statement with some double-checking once i’m back in the office). thanks, Iain
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 01:58:54PM +0200, Jakub Jelinek wrote: > > Any comment with text > > > > ^[^_[:alnum:]]*(else )?fall(s | |-)?thr(ough|u)[^_[:alnum:]]*$ > > > > perhaps? Case-insensitive. Or allow any amount of space, or even any > > interpunction. Just don't allow any alphanumerics except for those > > exact words, and there won't be many false hits at all. > > Not sure we want to match FaLlS THrouGH, Yes it's silly, but would it ever match the wrong thing? > and [^_[:alnum:]]* isn't without a > problem either, what if there is hebrew, or chinese, etc. text in there? I meant in LANG=C, but it would work otherwise, too. Nasty, of course. > The matching shouldn't depend on the current locale IMHO, and figuring out > what > unicode entry points are letters and which are not really isn't easy without > that. Right. > IMO before changing anything further, we want to gather some statistics what > styles are actually used in the wild together with how often they are used, > and then for the more common ones decide what is really supportable. If you do not allow a lot then there will be many false negatives. Segher
Re: [PATCH] Make -Wint-in-bool-context warn on suspicious shift ops
* Jason Merrill:
> On Sun, Sep 25, 2016 at 3:46 AM, Bernd Edlinger
> wrote:
>> This patch makes -Wint-in-bool-context warn on suspicious integer left
>> shifts, when the integer is signed, which is most likely some kind of
>> programming error, for instance using "<<" instead of "<".
>>
>> The warning is motivated by the fact, that an overflow on integer shift
>> left is undefined behavior, even if gcc won't optimize the shift based
>> on the undefined behavior.
>>
>> So in absence of undefined behavior the boolean result does not depend
>> on the shift value, thus the whole shifting is pointless.
>
> It's pointless for unsigned integers, too; why not warn for them as
> well? And why not warn for 0 << 0 and 1 << 0, which are just as
> pointless?
“1 << 0“ is often used in a sequence of flag mask definitions. This
example is from :
| /* Terminal control structure. */
| struct termios
| {
| /* Input modes. */
| tcflag_t c_iflag;
| #define IGNBRK (1 << 0)/* Ignore break condition. */
| #define BRKINT (1 << 1)/* Signal interrupt on break. */
| #define IGNPAR (1 << 2)/* Ignore characters with parity errors. */
| #define PARMRK (1 << 3)/* Mark parity and framing errors. */
“0 << 0” is used in a similar context, to create a zero constant for a
multi-bit subfield of an integer.
This example comes from GDB, in bfd/elf64-alpha.c:
| insn = INSN_ADDQ | (16 << 21) | (0 << 16) | (0 << 0);
Re: [SH][committed] Fix cset_zero pattern regressions
On Sun, 2016-09-25 at 16:06 +0900, Oleg Endo wrote:
>
> This fixes a fallout that actually goes back to 5.0 but went
> unnoticed. The costs for movt and movrt type of insns were not
> correctly reported and ifcvt thus made some bad choices for SH. A
> new cset_zero pattern variant is also required to fix the matching
> for some recent changes in the middle end.
>
> Tested on sh-elf with
> make -k check RUNTESTFLAGS="--target_board=sh-sim\{-m2/-ml,-m2/-mb,-
> m2a/-mb,-m4/-ml,-m4/-mb,-m4a/-ml,-m4a/-mb}"
>
> Committed as r240471.
> Backports to GCC 6 and GCC 5 will follow.
>
While working on the backport, I noticed that I've screwed up the costs
return value. Funny that it had no effect in this case.
Maybe rtx_costs functions should be made to return something like
std::optional to avoid these kind of mistakes. It would also cut
the code by half in most cases.
Anyway, fixed as attached, re-tested as above and committed as r240533.
Cheers,
Oleg
gcc/ChangeLog
PR target/51244
* config/sh/sh.c (sh_rtx_costs): Fix return value of SET of movt and
movrt patterns. Match them before anything else in the SET case.
Index: gcc/config/sh/sh.c
===
--- gcc/config/sh/sh.c (revision 240471)
+++ gcc/config/sh/sh.c (working copy)
@@ -3199,6 +3199,12 @@
vector-move, so we have to provide the correct cost in the number
of move insns to load/store the reg of the mode in question. */
case SET:
+ if (sh_movt_set_dest (x) != NULL || sh_movrt_set_dest (x) != NULL)
+ {
+ *total = COSTS_N_INSNS (1);
+ return true;
+ }
+
if (register_operand (SET_DEST (x), VOIDmode)
&& (register_operand (SET_SRC (x), VOIDmode)
|| satisfies_constraint_Z (SET_SRC (x
@@ -3208,10 +3214,6 @@
/ mov_insn_size (mode, TARGET_SH2A));
return true;
}
-
- if (sh_movt_set_dest (x) != NULL || sh_movrt_set_dest (x) != NULL)
- return COSTS_N_INSNS (1);
-
return false;
/* The cost of a mem access is mainly the cost of the address mode. */
Re: [PATCH] Fix PR gcov-profile/46266
On 09/27/16 08:46, Martin Liška wrote: Second version of the patch adds validation to gcov.exp, where $result is scanned for "File ''". Luckily current test-case hit that verification: FAIL: gcc.misc-tests/gcov-6.c gcov failed: .gcov should not be created FAIL: gcc.misc-tests/gcov-7.c gcov failed: .gcov should not be created thanks. One thing I dislike is negated predicates though -- I think I'd find if (!gimple_has_reserved_location (gs)) to be more understandable (particularly as that matches the sense of RESERVED_LOCATION_P. Agree with you, renamed to gimple_has_reserved_location. Ready with that change? Adding missing patch. I think this needs Richard's approval for the gimple predicates etc. But OK for me. nathan
Add configure check for -Wimplicit-fallthrough
Using -Wno-error where only -Wno-implicit-fallthrough was meant was deemed
to coarse, so this patch attempts to add a configure check for this warnign
and only use -Wno-implicit-fallthrough when appropriate.
Bootstrapped on x86_64-linux and ppc64-linux, ok for trunk?
2016-09-27 Marek Polacek
* Makefile.in (insn-attrtab.o-warn, insn-dfatab.o-warn,
insn-latencytab.o-warn, insn-output.o-warn, insn-emit.o-warn): Use
@W_NO_IMPLICIT_FALLTHROUGH@ instead of -Wno-error.
* configure.ac: Add check for -Wimplicit-fallthrough.
* configure: Regenerate.
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index ff12908..5871a47 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -218,11 +218,11 @@ libgcov-merge-tool.o-warn = -Wno-error
gimple-match.o-warn = -Wno-unused
generic-match.o-warn = -Wno-unused
dfp.o-warn = -Wno-strict-aliasing
-insn-attrtab.o-warn = -Wno-error
-insn-dfatab.o-warn = -Wno-error
-insn-latencytab.o-warn = -Wno-error
-insn-output.o-warn = -Wno-error
-insn-emit.o-warn = -Wno-error
+insn-attrtab.o-warn = @W_NO_IMPLICIT_FALLTHROUGH@
+insn-dfatab.o-warn = @W_NO_IMPLICIT_FALLTHROUGH@
+insn-latencytab.o-warn = @W_NO_IMPLICIT_FALLTHROUGH@
+insn-output.o-warn = @W_NO_IMPLICIT_FALLTHROUGH@
+insn-emit.o-warn = @W_NO_IMPLICIT_FALLTHROUGH@
# All warnings have to be shut off in stage1 if the compiler used then
# isn't gcc; configure determines that. WARN_CFLAGS will be either
diff --git a/gcc/configure b/gcc/configure
index 96eba9e..459f513 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -789,6 +789,7 @@ valgrind_path
TREECHECKING
nocommon_flag
noexception_flags
+W_NO_IMPLICIT_FALLTHROUGH
warn_cxxflags
warn_cflags
c_strict_warn
@@ -7006,6 +7007,34 @@ fi
+# Check whether -Wimplicit-fallthrough works.
+W_NO_IMPLICIT_FALLTHROUGH=
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for -Wimplicit-fallthrough
option" >&5
+$as_echo_n "checking for -Wimplicit-fallthrough option... " >&6; }
+if test "${gcc_cv_implicit_fallthrough+set}" = set; then :
+ $as_echo_n "(cached) " >&6
+else
+ saved_CXXFLAGS="$CXXFLAGS"
+ CXXFLAGS="$CXXFLAGS -Wimplicit-fallthrough"
+ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+int main(void) {return 0;}
+_ACEOF
+if ac_fn_cxx_try_compile "$LINENO"; then :
+ gcc_cv_implicit_fallthrough=yes
+else
+ gcc_cv_implicit_fallthrough=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+ CXXFLAGS="$saved_CXXFLAGS"
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_implicit_fallthrough"
>&5
+$as_echo "$gcc_cv_implicit_fallthrough" >&6; }
+if test "$gcc_cv_implicit_fallthrough" = "yes"; then
+ W_NO_IMPLICIT_FALLTHROUGH="-Wno-implicit-fallthrough"
+fi
+
+
# Disable exceptions and RTTI if building with g++
ac_ext=c
ac_cpp='$CPP $CPPFLAGS'
@@ -18476,7 +18505,7 @@ else
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
-#line 18479 "configure"
+#line 18508 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
@@ -18582,7 +18611,7 @@ else
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
-#line 18585 "configure"
+#line 18614 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 534f22e..b72e52a 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -503,6 +503,21 @@ fi
AC_SUBST(warn_cflags)
AC_SUBST(warn_cxxflags)
+# Check whether -Wimplicit-fallthrough works.
+W_NO_IMPLICIT_FALLTHROUGH=
+AC_CACHE_CHECK([for -Wimplicit-fallthrough option],
+ [gcc_cv_implicit_fallthrough],
+ [saved_CXXFLAGS="$CXXFLAGS"
+ CXXFLAGS="$CXXFLAGS -Wimplicit-fallthrough"
+ AC_COMPILE_IFELSE([int main(void) {return 0;}],
+ [gcc_cv_implicit_fallthrough=yes],
+ [gcc_cv_implicit_fallthrough=no])
+ CXXFLAGS="$saved_CXXFLAGS"])
+if test "$gcc_cv_implicit_fallthrough" = "yes"; then
+ W_NO_IMPLICIT_FALLTHROUGH="-Wno-implicit-fallthrough"
+fi
+AC_SUBST([W_NO_IMPLICIT_FALLTHROUGH])
+
# Disable exceptions and RTTI if building with g++
ACX_PROG_CC_WARNING_OPTS(
m4_quote(m4_do([-fno-exceptions -fno-rtti
-fasynchronous-unwind-tables])),
Marek
Re: [PATCH] Fix PR gcov-profile/46266
On Tue, Sep 27, 2016 at 2:57 PM, Nathan Sidwell wrote: > On 09/27/16 08:46, Martin Liška wrote: > >>> Second version of the patch adds validation to gcov.exp, where $result is >>> scanned for "File ''". >>> Luckily current test-case hit that verification: >>> >>> FAIL: gcc.misc-tests/gcov-6.c gcov failed: .gcov should not be >>> created >>> FAIL: gcc.misc-tests/gcov-7.c gcov failed: .gcov should not be >>> created > > > thanks. > One thing I dislike is negated predicates though -- I think I'd find if (!gimple_has_reserved_location (gs)) to be more understandable (particularly as that matches the sense of RESERVED_LOCATION_P. >>> >>> >>> Agree with you, renamed to gimple_has_reserved_location. >>> Ready with that change? > > >> Adding missing patch. > > > I think this needs Richard's approval for the gimple predicates etc. But OK > for me. Sorry for not chiming in earlier but I'd rather have you use if (RESERVED_LOCATION_P (gimple_location (...))) and not add the gimple_has_not_reserved_location wrapper. That's more in line with the other uses you have. Ok with that change (well, adding the RESERVED_LOCATION_P macro is ok). Richard. > nathan
Re: Add configure check for -Wimplicit-fallthrough
On Tue, Sep 27, 2016 at 3:05 PM, Marek Polacek wrote:
> Using -Wno-error where only -Wno-implicit-fallthrough was meant was deemed
> to coarse, so this patch attempts to add a configure check for this warnign
> and only use -Wno-implicit-fallthrough when appropriate.
>
> Bootstrapped on x86_64-linux and ppc64-linux, ok for trunk?
It looks to me this would hide eventual bugs in .md files by not
issueing the warning?
Richard.
> 2016-09-27 Marek Polacek
>
> * Makefile.in (insn-attrtab.o-warn, insn-dfatab.o-warn,
> insn-latencytab.o-warn, insn-output.o-warn, insn-emit.o-warn): Use
> @W_NO_IMPLICIT_FALLTHROUGH@ instead of -Wno-error.
> * configure.ac: Add check for -Wimplicit-fallthrough.
> * configure: Regenerate.
>
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index ff12908..5871a47 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -218,11 +218,11 @@ libgcov-merge-tool.o-warn = -Wno-error
> gimple-match.o-warn = -Wno-unused
> generic-match.o-warn = -Wno-unused
> dfp.o-warn = -Wno-strict-aliasing
> -insn-attrtab.o-warn = -Wno-error
> -insn-dfatab.o-warn = -Wno-error
> -insn-latencytab.o-warn = -Wno-error
> -insn-output.o-warn = -Wno-error
> -insn-emit.o-warn = -Wno-error
> +insn-attrtab.o-warn = @W_NO_IMPLICIT_FALLTHROUGH@
> +insn-dfatab.o-warn = @W_NO_IMPLICIT_FALLTHROUGH@
> +insn-latencytab.o-warn = @W_NO_IMPLICIT_FALLTHROUGH@
> +insn-output.o-warn = @W_NO_IMPLICIT_FALLTHROUGH@
> +insn-emit.o-warn = @W_NO_IMPLICIT_FALLTHROUGH@
>
> # All warnings have to be shut off in stage1 if the compiler used then
> # isn't gcc; configure determines that. WARN_CFLAGS will be either
> diff --git a/gcc/configure b/gcc/configure
> index 96eba9e..459f513 100755
> --- a/gcc/configure
> +++ b/gcc/configure
> @@ -789,6 +789,7 @@ valgrind_path
> TREECHECKING
> nocommon_flag
> noexception_flags
> +W_NO_IMPLICIT_FALLTHROUGH
> warn_cxxflags
> warn_cflags
> c_strict_warn
> @@ -7006,6 +7007,34 @@ fi
>
>
>
> +# Check whether -Wimplicit-fallthrough works.
> +W_NO_IMPLICIT_FALLTHROUGH=
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for -Wimplicit-fallthrough
> option" >&5
> +$as_echo_n "checking for -Wimplicit-fallthrough option... " >&6; }
> +if test "${gcc_cv_implicit_fallthrough+set}" = set; then :
> + $as_echo_n "(cached) " >&6
> +else
> + saved_CXXFLAGS="$CXXFLAGS"
> + CXXFLAGS="$CXXFLAGS -Wimplicit-fallthrough"
> + cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> +/* end confdefs.h. */
> +int main(void) {return 0;}
> +_ACEOF
> +if ac_fn_cxx_try_compile "$LINENO"; then :
> + gcc_cv_implicit_fallthrough=yes
> +else
> + gcc_cv_implicit_fallthrough=no
> +fi
> +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
> + CXXFLAGS="$saved_CXXFLAGS"
> +fi
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result:
> $gcc_cv_implicit_fallthrough" >&5
> +$as_echo "$gcc_cv_implicit_fallthrough" >&6; }
> +if test "$gcc_cv_implicit_fallthrough" = "yes"; then
> + W_NO_IMPLICIT_FALLTHROUGH="-Wno-implicit-fallthrough"
> +fi
> +
> +
> # Disable exceptions and RTTI if building with g++
> ac_ext=c
> ac_cpp='$CPP $CPPFLAGS'
> @@ -18476,7 +18505,7 @@ else
>lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>lt_status=$lt_dlunknown
>cat > conftest.$ac_ext <<_LT_EOF
> -#line 18479 "configure"
> +#line 18508 "configure"
> #include "confdefs.h"
>
> #if HAVE_DLFCN_H
> @@ -18582,7 +18611,7 @@ else
>lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>lt_status=$lt_dlunknown
>cat > conftest.$ac_ext <<_LT_EOF
> -#line 18585 "configure"
> +#line 18614 "configure"
> #include "confdefs.h"
>
> #if HAVE_DLFCN_H
> diff --git a/gcc/configure.ac b/gcc/configure.ac
> index 534f22e..b72e52a 100644
> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -503,6 +503,21 @@ fi
> AC_SUBST(warn_cflags)
> AC_SUBST(warn_cxxflags)
>
> +# Check whether -Wimplicit-fallthrough works.
> +W_NO_IMPLICIT_FALLTHROUGH=
> +AC_CACHE_CHECK([for -Wimplicit-fallthrough option],
> + [gcc_cv_implicit_fallthrough],
> + [saved_CXXFLAGS="$CXXFLAGS"
> + CXXFLAGS="$CXXFLAGS -Wimplicit-fallthrough"
> + AC_COMPILE_IFELSE([int main(void) {return 0;}],
> + [gcc_cv_implicit_fallthrough=yes],
> + [gcc_cv_implicit_fallthrough=no])
> + CXXFLAGS="$saved_CXXFLAGS"])
> +if test "$gcc_cv_implicit_fallthrough" = "yes"; then
> + W_NO_IMPLICIT_FALLTHROUGH="-Wno-implicit-fallthrough"
> +fi
> +AC_SUBST([W_NO_IMPLICIT_FALLTHROUGH])
> +
> # Disable exceptions and RTTI if building with g++
> ACX_PROG_CC_WARNING_OPTS(
> m4_quote(m4_do([-fno-exceptions -fno-rtti
> -fasynchronous-unwind-tables])),
>
> Marek
Re: Add configure check for -Wimplicit-fallthrough
On Tue, Sep 27, 2016 at 03:05:24PM +0200, Marek Polacek wrote: > Using -Wno-error where only -Wno-implicit-fallthrough was meant was deemed > to coarse, so this patch attempts to add a configure check for this warnign > and only use -Wno-implicit-fallthrough when appropriate. > > Bootstrapped on x86_64-linux and ppc64-linux, ok for trunk? > > 2016-09-27 Marek Polacek > > * Makefile.in (insn-attrtab.o-warn, insn-dfatab.o-warn, > insn-latencytab.o-warn, insn-output.o-warn, insn-emit.o-warn): Use > @W_NO_IMPLICIT_FALLTHROUGH@ instead of -Wno-error. > * configure.ac: Add check for -Wimplicit-fallthrough. > * configure: Regenerate. Shouldn't this use ACX_PROG_CXX_WARNING_OPTS ? Jakub
Re: Add configure check for -Wimplicit-fallthrough
On Tue, Sep 27, 2016 at 03:09:36PM +0200, Richard Biener wrote: > On Tue, Sep 27, 2016 at 3:05 PM, Marek Polacek wrote: > > Using -Wno-error where only -Wno-implicit-fallthrough was meant was deemed > > to coarse, so this patch attempts to add a configure check for this warnign > > and only use -Wno-implicit-fallthrough when appropriate. > > > > Bootstrapped on x86_64-linux and ppc64-linux, ok for trunk? > > It looks to me this would hide eventual bugs in .md files by not > issueing the warning? Guess it depends on what kind of warnings we want to suppress here, if it is something user can control in their *.md files, or something that perhaps changes to the generators could handle (add /* FALLTHRU */ comments or gcc_fallthrough (); in some cases)? > > 2016-09-27 Marek Polacek > > > > * Makefile.in (insn-attrtab.o-warn, insn-dfatab.o-warn, > > insn-latencytab.o-warn, insn-output.o-warn, insn-emit.o-warn): Use > > @W_NO_IMPLICIT_FALLTHROUGH@ instead of -Wno-error. > > * configure.ac: Add check for -Wimplicit-fallthrough. > > * configure: Regenerate. Jakub
Re: [PATCH GCC][v2]Simplify alias check code generation in vectorizer
> Also the '=' in the split line goes to the next line according to
> coding conventions.
fixed, I had only looked at an instance one function above which had it
wrong as well. Also changed comment grammar slightly.
Regards
Robin
--
gcc/ChangeLog:
2016-09-27 Robin Dapp
* tree-vect-loop-manip.c (create_intersect_range_checks_index):
Add tree_fits_shwi_p check.
gcc/testsuite/ChangeLog:
2016-09-27 Robin Dapp
* gcc.dg/vect/pr77724.c: New test.
diff --git a/gcc/testsuite/gcc.dg/vect/pr77724.c b/gcc/testsuite/gcc.dg/vect/pr77724.c
new file mode 100644
index 000..b3411d6
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/pr77724.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+
+int a[81];
+int b, c;
+
+void
+fn1()
+{
+ int d = b;
+ for (; c; --c)
+a[c + d] = a[c];
+}
diff --git a/gcc/tree-vect-loop-manip.c b/gcc/tree-vect-loop-manip.c
index 8203040..a715fd9 100644
--- a/gcc/tree-vect-loop-manip.c
+++ b/gcc/tree-vect-loop-manip.c
@@ -2301,6 +2301,9 @@ create_intersect_range_checks_index (loop_vec_info loop_vinfo, tree *cond_expr,
if (!tree_fits_uhwi_p (dr_a.seg_len) || !tree_fits_uhwi_p (dr_b.seg_len))
return false;
+ if (!tree_fits_shwi_p (DR_STEP (dr_a.dr)))
+return false;
+
if (!operand_equal_p (DR_BASE_OBJECT (dr_a.dr), DR_BASE_OBJECT (dr_b.dr), 0))
return false;
@@ -2310,9 +2313,8 @@ create_intersect_range_checks_index (loop_vec_info loop_vinfo, tree *cond_expr,
gcc_assert (TREE_CODE (DR_STEP (dr_a.dr)) == INTEGER_CST);
bool neg_step = tree_int_cst_compare (DR_STEP (dr_a.dr), size_zero_node) < 0;
- unsigned HOST_WIDE_INT abs_step = tree_to_uhwi (DR_STEP (dr_a.dr));
- if (neg_step)
-abs_step = -abs_step;
+ unsigned HOST_WIDE_INT abs_step
+= absu_hwi (tree_to_shwi (DR_STEP (dr_a.dr)));
unsigned HOST_WIDE_INT seg_len1 = tree_to_uhwi (dr_a.seg_len);
unsigned HOST_WIDE_INT seg_len2 = tree_to_uhwi (dr_b.seg_len);
@@ -2331,7 +2333,7 @@ create_intersect_range_checks_index (loop_vec_info loop_vinfo, tree *cond_expr,
{
tree access1 = DR_ACCESS_FN (dr_a.dr, i);
tree access2 = DR_ACCESS_FN (dr_b.dr, i);
- /* Two index must be the same if they are not scev, or not scev wrto
+ /* Two indices must be the same if they are not scev, or not scev wrto
current loop being vecorized. */
if (TREE_CODE (access1) != POLYNOMIAL_CHREC
|| TREE_CODE (access2) != POLYNOMIAL_CHREC
@@ -2343,7 +2345,7 @@ create_intersect_range_checks_index (loop_vec_info loop_vinfo, tree *cond_expr,
return false;
}
- /* Two index must have the same step. */
+ /* The two indices must have the same step. */
if (!operand_equal_p (CHREC_RIGHT (access1), CHREC_RIGHT (access2), 0))
return false;
[ARM] Fix invalid instructions generated for data movement.
Recently added support for ARMv8.2-A (https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01240.html) included a number of changes to improve data movement, particularly for HI and HF mode values. These included the use of the Thumb-2 instruction MOVW and of the new VMOV.F16 instruction. There are problems with both: the use of MOVW isn't properly guarded so that it can be generated for targets that don't support it and the VMOV.F16 instruction is wrongly marked as being predicable. This patch adds guards to the use of the MOVW so that it is only generated when the target supports Thumb-2 and fixes the predication on the VMOV.F16 instruction. Tested for arm-none-linux-gnueabihf with native bootstrap and make check on ARMv8-A and for arm-none-eabi with cross compiled check-gcc on an ARMv8.2-A emulator. There is one failure on arm-none-linux-gnueabihf, gcc.dg/guality/pr36728-1.c which is due to MOVW not being generated, even for ARMv7-A. The generated code is otherwise correct. I think I understand why MOVW isn't being emitted but it'll take time to test properly. Since this patch is to fix a broken build is it OK to commit it and to fix the poor code-gen in a follow-up patch? Matthew gcc/ 2016-09-27 Matthew Wahab * config/arm/arm.md (*arm_movsi_insn): Add "arch" attribute. * config/arm/vfp.md (*arm_movhi_vfp): Likewise. (*thumb2_movhi_vfp): Likewise. (*arm_movhi_fp16): Remove predication operand from VMOV.F16 template. Expand predicable attribute to mark VMOV.F16 as not predicable. Add "arch" attribute. (*thumb2_movhi_fp16): Likewise. (*arm_movsi_vfp): Break a long line. Add "arch" attribute. (*thumb2_movsi_vfp): Add "arch" attribute. >From bedba58f504ef1f68ee0f90d9e34563b75653ae5 Mon Sep 17 00:00:00 2001 From: Matthew Wahab Date: Mon, 26 Sep 2016 12:05:34 +0100 Subject: [PATCH] [ARM] Fix invalid instructions generated for data movement. Recently added support for ARMv8.2-A (https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01240.html) included a number of changes to improve data movement, particularly for HI and HF mode values. These included the use of the Thumb-2 instruction MOVW and of the new VMOV.F16 instruction. There are problems with both: the use of MOVW isn't properly guarded so that it can be generated for targets that don't support it. The VMOV.F16 instruction is wrongly marked as being predicable. This patch adds guards to the use of the MOVW so that it is only generated when the target supports Thumb-2 and fixes the predication on the VMOV.F16 instruction. Tested for arm-none-linux-gnueabihf with native bootstrap and make check on ARMv8-A and for arm-none-eabi with cross compiled check-gcc on an ARMv8.2-A emulator. There is one failure on arm-none-linux-gnueabihf, gcc.dg/guality/pr36728-1.c which is due to MOVW not being generated, even for ARMv7-A. The generated code is otherwise correct. I think I understand why MOVW isn't being emitted but it'll take time to test properly. Since this patch is to fix a broken build is it OK to commit and to fix the poor code-gen in a follow-up patch? gcc/ 2016-09-27 Matthew Wahab * config/arm/arm.md (*arm_movsi_insn): Add "arch" attribute. * config/arm/vfp.md (*arm_movhi_vfp): Likewise. (*thumb2_movhi_vfP): Likewise. (*arm_movhi_fp16): Remove predication operand from VMOV.F16 template. Expand predicable attribute to mark VMOV.F16 as not predicable. Add "arch" attribute. (*thumb2_movhi_fp16): Likewise. (*arm_movsi_vfp): Break a long line. Add "arch" attribute. (*thum2_movsi_vfp): Add "arch" attribute. --- gcc/config/arm/arm.md | 1 + gcc/config/arm/vfp.md | 18 +- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 411754f..999292b 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -6065,6 +6065,7 @@ [(set_attr "type" "mov_reg,mov_imm,mvn_imm,mov_imm,load1,store1") (set_attr "predicable" "yes") (set_attr "pool_range" "*,*,*,*,4096,*") + (set_attr "arch" "*,*,*,t2,*,*") (set_attr "neg_pool_range" "*,*,*,*,4084,*")] ) diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md index 5d22c34..21eaf48 100644 --- a/gcc/config/arm/vfp.md +++ b/gcc/config/arm/vfp.md @@ -65,6 +65,7 @@ (const_string "f_mcr") (const_string "f_mrc") (const_string "fmov")]) + (set_attr "arch" "*, *, t2, *, *, *, *, *") (set_attr "pool_range" "*, *, *, *, 256, *, *, *") (set_attr "neg_pool_range" "*, *, *, *, 244, *, *, *") (set_attr "length" "4")] @@ -108,6 +109,7 @@ (set_attr "type" "mov_reg, mov_imm, mov_imm, mov_imm, store1, load1,\ f_mcr, f_mrc, fmov") + (set_attr "arch" "*, *, *, t2, *, *, *, *, *") (set_attr "pool_range" "*, *, *, *, *, 4094, *, *, *") (set_attr "neg_pool_range" "*, *, *, *, *, 250, *, *, *") (set_attr "length" "2, 4, 2, 4, 4, 4, 4, 4, 4")] @@ -139,14 +141,14 @@ return "ldrh%?\t%0, %1\t%@ movhi";
Re: [PATCH GCC][v2]Simplify alias check code generation in vectorizer
On Tue, 27 Sep 2016, Robin Dapp wrote: > > Also the '=' in the split line goes to the next line according to > > coding conventions. > > fixed, I had only looked at an instance one function above which had it > wrong as well. Also changed comment grammar slightly. Ok. Thanks, Richard. > > Regards > Robin > > -- > > gcc/ChangeLog: > > 2016-09-27 Robin Dapp > > * tree-vect-loop-manip.c (create_intersect_range_checks_index): > Add tree_fits_shwi_p check. > > > gcc/testsuite/ChangeLog: > > 2016-09-27 Robin Dapp > > * gcc.dg/vect/pr77724.c: New test. > -- Richard Biener SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
Re: [PATCH] Fix PR gcov-profile/46266
On 09/27/2016 03:05 PM, Richard Biener wrote: > On Tue, Sep 27, 2016 at 2:57 PM, Nathan Sidwell wrote: >> On 09/27/16 08:46, Martin Liška wrote: >> Second version of the patch adds validation to gcov.exp, where $result is scanned for "File ''". Luckily current test-case hit that verification: FAIL: gcc.misc-tests/gcov-6.c gcov failed: .gcov should not be created FAIL: gcc.misc-tests/gcov-7.c gcov failed: .gcov should not be created >> >> >> thanks. >> > One thing I dislike is negated predicates though -- I think I'd find >if (!gimple_has_reserved_location (gs)) > to be more understandable (particularly as that matches the sense of > RESERVED_LOCATION_P. Agree with you, renamed to gimple_has_reserved_location. Ready with that change? >> >> >>> Adding missing patch. >> >> >> I think this needs Richard's approval for the gimple predicates etc. But OK >> for me. > > Sorry for not chiming in earlier but I'd rather have you use > > if (RESERVED_LOCATION_P (gimple_location (...))) > > and not add the gimple_has_not_reserved_location wrapper. That's more in line > with the other uses you have. > > Ok with that change (well, adding the RESERVED_LOCATION_P macro is ok). > > Richard. Done that in r240536. Thanks for the review. Martin > >> nathan
Re: Add configure check for -Wimplicit-fallthrough
On Tue, Sep 27, 2016 at 3:13 PM, Jakub Jelinek wrote: > On Tue, Sep 27, 2016 at 03:09:36PM +0200, Richard Biener wrote: >> On Tue, Sep 27, 2016 at 3:05 PM, Marek Polacek wrote: >> > Using -Wno-error where only -Wno-implicit-fallthrough was meant was deemed >> > to coarse, so this patch attempts to add a configure check for this warnign >> > and only use -Wno-implicit-fallthrough when appropriate. >> > >> > Bootstrapped on x86_64-linux and ppc64-linux, ok for trunk? >> >> It looks to me this would hide eventual bugs in .md files by not >> issueing the warning? > > Guess it depends on what kind of warnings we want to suppress here, if it is > something user can control in their *.md files, or something that perhaps > changes to the generators could handle (add /* FALLTHRU */ comments or > gcc_fallthrough (); in some cases)? I just looked at one random one and it was sse.md (repeatedly doing) case MODE_V16SF: gcc_assert (TARGET_AVX512F); case MODE_V8SF: gcc_assert (TARGET_AVX); case MODE_V4SF: gcc_assert (TARGET_SSE); ... Richard. >> > 2016-09-27 Marek Polacek >> > >> > * Makefile.in (insn-attrtab.o-warn, insn-dfatab.o-warn, >> > insn-latencytab.o-warn, insn-output.o-warn, insn-emit.o-warn): Use >> > @W_NO_IMPLICIT_FALLTHROUGH@ instead of -Wno-error. >> > * configure.ac: Add check for -Wimplicit-fallthrough. >> > * configure: Regenerate. > > Jakub
[PATCH][ARM] Fix -Wimplicit-fallthrough warnings
Hi all, This patch adjusts the fallthrough comments in the arm backend to not trigger the -Wimplicit-fallthrough warning which is currently causing arm bootstrap to fail. Bootstrapped and tested on arm-none-linux-gnueabihf. Committing to trunk in the interest of fixing bootstrap. Thanks, Kyrill 2016-09-27 Kyrylo Tkachov * config/arm/arm.c (const_ok_for_op): Use "Fall through" comment form expected by -Wimplicit-fallthrough. (thumb1_size_rtx_costs): Likewise. (thumb2_reorg): Likewise. (tls_mentioned_p): Add "Fall through" comment. (thumb2_reorg): Likewise. * config/arm/arm-builtins.c (arm_expand_neon_args): Use "Fall through" comment form expected by -Wimplicit-fallthrough. diff --git a/gcc/config/arm/arm-builtins.c b/gcc/config/arm/arm-builtins.c index 70bcc07..4fafefc 100644 --- a/gcc/config/arm/arm-builtins.c +++ b/gcc/config/arm/arm-builtins.c @@ -2153,9 +2153,9 @@ arm_expand_neon_args (rtx target, machine_mode map_mode, int fcode, enum machine_mode vmode = mode[argc - 1]; neon_lane_bounds (op[argc], 0, GET_MODE_NUNITS (vmode), exp); } - /* Fall through - if the lane index isn't a constant then - the next case will error. */ - + /* If the lane index isn't a constant then the next + case will error. */ + /* Fall through. */ case NEON_ARG_CONSTANT: constant_arg: if (!(*insn_data[icode].operand[opno].predicate) diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index feb54cb..705fa00 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -4001,8 +4001,7 @@ const_ok_for_op (HOST_WIDE_INT i, enum rtx_code code) && ((i & 0xf000) == 0 || ((-i) & 0xf000) == 0)) return 1; - /* else fall through. */ - + /* Fall through. */ case COMPARE: case EQ: case NE: @@ -9129,7 +9128,7 @@ thumb1_size_rtx_costs (rtx x, enum rtx_code code, enum rtx_code outer) || (GET_CODE (XEXP (x, 1)) == MULT && power_of_two_operand (XEXP (XEXP (x, 1), 1), SImode))) return COSTS_N_INSNS (2); - /* On purpose fall through for normal RTX. */ + /* Fall through. */ case COMPARE: case NEG: case NOT: @@ -13371,6 +13370,7 @@ tls_mentioned_p (rtx x) if (XINT (x, 1) == UNSPEC_TLS) return 1; +/* Fall through. */ default: return 0; } @@ -17568,7 +17568,7 @@ thumb2_reorg (void) test the global flag here. */ if (!optimize_size) break; - /* else fall through. */ + /* Fall through. */ case AND: case IOR: case XOR:
Re: internal fn pretty printing
On 09/27/2016 12:58 PM, Nathan Sidwell wrote: In working on some new code I got sufficiently frustrated to implement pretty printing on internal function discriminators, as I think one of you suggested a while back. With this patch we get: .data_dep.2 = UNIQUE (OACC_FORK, .data_dep.2, -1); rather than an obscure raw integer for the first argument. For the internal fns (I know of) that have a discriminator argument, I define the codes in appropriate macros, and then expand them to create the enum definitions. In the pretty printer I examine the first argument and use that to index into a string array also built from the code macro. Ok for trunk? (I've just applied it to gomp4). Ok. Bernd
Re: [PATCH] Make -Wint-in-bool-context warn on suspicious shift ops
Hi, On Tue, 27 Sep 2016, Jason Merrill wrote: > On Sun, Sep 25, 2016 at 3:46 AM, Bernd Edlinger > wrote: > > This patch makes -Wint-in-bool-context warn on suspicious integer left > > shifts, when the integer is signed, which is most likely some kind of > > programming error, for instance using "<<" instead of "<". > > > > The warning is motivated by the fact, that an overflow on integer shift > > left is undefined behavior, even if gcc won't optimize the shift based > > on the undefined behavior. > > > > So in absence of undefined behavior the boolean result does not depend > > on the shift value, thus the whole shifting is pointless. > > It's pointless for unsigned integers, too; why not warn for them as > well? Um, because left shift on unsigned integers is never undefined, so !!(1u << a) is meaningful and effectively tests if a < CHAR_BITS*sizeof(unsigned) ? Ciao, Michael.
Fix fall through comment in ia64/
Someone would hit this sooner or later. Applying to trunk. 2016-09-27 Marek Polacek * config/ia64/ia64.c (ia64_print_operand): Adjust fall through comment. diff --git gcc/config/ia64/ia64.c gcc/config/ia64/ia64.c index 573872e..d32823a 100644 --- gcc/config/ia64/ia64.c +++ gcc/config/ia64/ia64.c @@ -5550,7 +5550,7 @@ ia64_print_operand (FILE * file, rtx x, int code) case POST_DEC: case POST_MODIFY: x = XEXP (x, 0); - /* ... fall through ... */ + /* fall through */ case REG: fputs (reg_names [REGNO (x)], file); Marek
[PATCH] Last bit from Early LTO debug merge -- move break_out_includes
This is moving break_out_includes (aka -feliminate-dwarf2-dups handling)
to early finish. It requires some massaging with the way we pick
up its results which previously "conveniently" ended up in the
limbo list but after moving to early will be scrapped off by
late finish doing a limbo list flush (where we just drop all CU DIEs
thinking it can only be comp_unit_die () itself). Thus this patch
adds a cu_die_list list for it (much similar to the comdat one we already
have).
Eventually -feliminate-dwarf2-dups might die anyway(?), its testing
coverage is very low and it's declared broken for C++.
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
Ok for trunk?
Thanks,
Richard.
2016-09-27 Richard Biener
* dwarf2out.c (cu_die_list): New global.
(dwarf2out_finish): Walk cu_die_list instead of limbo DIEs. Add
main_comp_unit_die to cu_die_list if we created it.
Move break_out_includes ...
(dwarf2out_early_finish): ... here. Push created CU DIEs onto
the cu_die_list.
diff -u gcc/dwarf2out.c gcc/dwarf2out.c
--- gcc/dwarf2out.c (working copy)
+++ gcc/dwarf2out.c (working copy)
@@ -2866,6 +2866,9 @@
/* A list of type DIEs that have been separated into comdat sections. */
static GTY(()) comdat_type_node *comdat_type_list;
+/* A list of CU DIEs that have been separated. */
+static GTY(()) limbo_die_node *cu_die_list;
+
/* A list of DIEs with a NULL parent waiting to be relocated. */
static GTY(()) limbo_die_node *limbo_die_list;
@@ -27855,11 +27858,6 @@
resolve_addr (comp_unit_die ());
move_marked_base_types ();
- /* Generate separate CUs for each of the include files we've seen.
- They will go into limbo_die_list. */
- if (flag_eliminate_dwarf2_dups)
-break_out_includes (comp_unit_die ());
-
/* Initialize sections and labels used for actual assembler output. */
init_sections_and_labels ();
@@ -27867,7 +27865,7 @@
have children. */
add_sibling_attributes (comp_unit_die ());
limbo_die_node *node;
- for (node = limbo_die_list; node; node = node->next)
+ for (node = cu_die_list; node; node = node->next)
add_sibling_attributes (node->die);
for (ctnode = comdat_type_list; ctnode != NULL; ctnode = ctnode->next)
add_sibling_attributes (ctnode->root_die);
@@ -27876,7 +27874,15 @@
skeleton compile_unit DIE that remains in the .o, while
most attributes go in the DWO compile_unit_die. */
if (dwarf_split_debug_info)
-main_comp_unit_die = gen_compile_unit_die (NULL);
+{
+ limbo_die_node *cu;
+ main_comp_unit_die = gen_compile_unit_die (NULL);
+ cu = limbo_die_list;
+ gcc_assert (cu->die == main_comp_unit_die);
+ limbo_die_list = limbo_die_list->next;
+ cu->next = cu_die_list;
+ cu_die_list = cu;
+}
else
main_comp_unit_die = comp_unit_die ();
@@ -27988,7 +27994,7 @@
/* Output all of the compilation units. We put the main one last so that
the offsets are available to output_pubnames. */
- for (node = limbo_die_list; node; node = node->next)
+ for (node = cu_die_list; node; node = node->next)
output_comp_unit (node->die, 0);
hash_table comdat_type_table (100);
@@ -28217,6 +28223,21 @@
prune_unused_types ();
}
+ /* Generate separate CUs for each of the include files we've seen.
+ They will go into limbo_die_list and from there to cu_die_list. */
+ if (flag_eliminate_dwarf2_dups)
+{
+ gcc_assert (limbo_die_list == NULL);
+ break_out_includes (comp_unit_die ());
+ limbo_die_node *cu;
+ while ((cu = limbo_die_list))
+ {
+ limbo_die_list = cu->next;
+ cu->next = cu_die_list;
+ cu_die_list = cu;
+ }
+}
+
/* The early debug phase is now finished. */
early_dwarf_finished = true;
}
Re: Implement -Wimplicit-fallthrough (version 9)
On 09/27/2016 02:01 PM, Marek Polacek wrote: On Tue, Sep 27, 2016 at 01:55:22PM +0200, Bernd Schmidt wrote: On 09/27/2016 01:51 PM, Marek Polacek wrote: But the C/C++ keywords are all English, too; lint tools only accept English, and so it wouldn't seem unreasonable to only accept English keywords in the comments. And in any case, I don't see how a compiler can be expected to be able to parse non-English languages. It isn't. But it can also be reasonably by expected not to warn about things that are valid according to the language specification and are frequently used. Ok, but note that the warning is in -Wextra, not enabled by default/-Wall. I think it's problematic enough that it needs to be removed from -Wextra as well. The latest ia64 backend patch shows that clearly IMO. Bernd
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 03:48:07PM +0200, Bernd Schmidt wrote: > On 09/27/2016 02:01 PM, Marek Polacek wrote: > >On Tue, Sep 27, 2016 at 01:55:22PM +0200, Bernd Schmidt wrote: > >>On 09/27/2016 01:51 PM, Marek Polacek wrote: > >>>But the C/C++ keywords are all English, too; lint tools only accept > >>>English, > >>>and so it wouldn't seem unreasonable to only accept English keywords in the > >>>comments. And in any case, I don't see how a compiler can be expected to > >>>be able to parse non-English languages. > >> > >>It isn't. But it can also be reasonably by expected not to warn about things > >>that are valid according to the language specification and are frequently > >>used. > > > >Ok, but note that the warning is in -Wextra, not enabled by default/-Wall. > > I think it's problematic enough that it needs to be removed from -Wextra as > well. The latest ia64 backend patch shows that clearly IMO. Just compare that to the number of real bugs the warning found in gcc codebase. It is really worth it for -Wextra. Jakub
Re: [ARM] Fix invalid instructions generated for data movement.
Hi Matthew, On 27/09/16 14:21, Matthew Wahab wrote: Recently added support for ARMv8.2-A (https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01240.html) included a number of changes to improve data movement, particularly for HI and HF mode values. These included the use of the Thumb-2 instruction MOVW and of the new VMOV.F16 instruction. There are problems with both: the use of MOVW isn't properly guarded so that it can be generated for targets that don't support it and the VMOV.F16 instruction is wrongly marked as being predicable. This patch adds guards to the use of the MOVW so that it is only generated when the target supports Thumb-2 and fixes the predication on the VMOV.F16 instruction. Tested for arm-none-linux-gnueabihf with native bootstrap and make check on ARMv8-A and for arm-none-eabi with cross compiled check-gcc on an ARMv8.2-A emulator. There is one failure on arm-none-linux-gnueabihf, gcc.dg/guality/pr36728-1.c which is due to MOVW not being generated, even for ARMv7-A. The generated code is otherwise correct. I think I understand why MOVW isn't being emitted but it'll take time to test properly. Since this patch is to fix a broken build is it OK to commit it and to fix the poor code-gen in a follow-up patch? Matthew gcc/ 2016-09-27 Matthew Wahab * config/arm/arm.md (*arm_movsi_insn): Add "arch" attribute. * config/arm/vfp.md (*arm_movhi_vfp): Likewise. (*thumb2_movhi_vfp): Likewise. (*arm_movhi_fp16): Remove predication operand from VMOV.F16 template. Expand predicable attribute to mark VMOV.F16 as not predicable. Add "arch" attribute. (*thumb2_movhi_fp16): Likewise. (*arm_movsi_vfp): Break a long line. Add "arch" attribute. (*thumb2_movsi_vfp): Add "arch" attribute. diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 411754f..999292b 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -6065,6 +6065,7 @@ [(set_attr "type" "mov_reg,mov_imm,mvn_imm,mov_imm,load1,store1") (set_attr "predicable" "yes") (set_attr "pool_range" "*,*,*,*,4096,*") + (set_attr "arch" "*,*,*,t2,*,*") (set_attr "neg_pool_range" "*,*,*,*,4084,*")] ) The "t2" attribute means that we're currently compiling for Thumb2. What you want is to check that the architecture we're compiling for supports Thumb2, so in this case you want the v6t2 value. In the interest of fixing the build I'll approve this patch as is since it's still correct (just not as general as it could be), but it'd be good to have a follow-up patch to fix this. Thanks for fixing this, Kyrill
Re: Implement -Wimplicit-fallthrough (version 9)
Hi, On Tue, 27 Sep 2016, Jakub Jelinek wrote: > Just compare that to the number of real bugs the warning found in gcc > codebase. It is really worth it for -Wextra. All those bugs would also have been found as well when it had simply accepted /fall.*thr/i anywhere in the preceding comment on one line. But all the recent spelling changes of comments to cater for the strictness exactly shows how misguided that is. The above would accept "Don't fall through" as well. I say: so what? Ciao, Michael.
Fix more fall through comments
More fall through comments.
Applying to trunk.
2016-09-27 Marek Polacek
* config/c6x/c6x.h: Adjust fall through comment.
* config/sh/sh.c (final_prescan_insn): Likewise.
* config/visium/visium.c (visium_expand_int_cstore): Likewise.
(visium_expand_fp_cstore): Likewise.
diff --git gcc/config/c6x/c6x.h gcc/config/c6x/c6x.h
index 3209bf6..fe173ab 100644
--- gcc/config/c6x/c6x.h
+++ gcc/config/c6x/c6x.h
@@ -92,14 +92,14 @@ extern c6x_cpu_t c6x_arch;
\
case C6X_CPU_C64XP: \
builtin_define ("_TMS320C6400_PLUS"); \
- /* ... fall through ... */\
+ /* fall through */\
case C6X_CPU_C64X: \
builtin_define ("_TMS320C6400"); \
break;\
\
case C6X_CPU_C67XP: \
builtin_define ("_TMS320C6700_PLUS"); \
- /* ... fall through ... */\
+ /* fall through */\
case C6X_CPU_C67X: \
builtin_define ("_TMS320C6700"); \
break;\
diff --git gcc/config/sh/sh.c gcc/config/sh/sh.c
index bfa248d..c593421 100644
--- gcc/config/sh/sh.c
+++ gcc/config/sh/sh.c
@@ -6549,7 +6549,7 @@ final_prescan_insn (rtx_insn *insn, rtx *opvec
ATTRIBUTE_UNUSED,
(asm_out_file, "L", CODE_LABEL_NUMBER (XEXP (note, 0)));
break;
}
- /* else FALLTHROUGH */
+ /* FALLTHROUGH */
case CALL:
asm_fprintf (asm_out_file, "\t.uses %LL%d\n",
CODE_LABEL_NUMBER (XEXP (note, 0)));
diff --git gcc/config/visium/visium.c gcc/config/visium/visium.c
index af58f99..3ce79f4 100644
--- gcc/config/visium/visium.c
+++ gcc/config/visium/visium.c
@@ -,7 +,7 @@ visium_expand_int_cstore (rtx *operands, enum
machine_mode mode)
code = reverse_condition (code);
reverse = true;
- /* ... fall through ... */
+ /* fall through */
case LTU:
case GTU:
@@ -2270,7 +2270,7 @@ visium_expand_fp_cstore (rtx *operands,
code = reverse_condition_maybe_unordered (code);
reverse = true;
- /* ... fall through ... */
+ /* fall through */
case LT:
case GT:
Marek
Re: Implement -Wimplicit-fallthrough (version 9)
On Tue, Sep 27, 2016 at 03:53:25PM +0200, Bernd Schmidt wrote: > On 09/27/2016 03:49 PM, Jakub Jelinek wrote: > >On Tue, Sep 27, 2016 at 03:48:07PM +0200, Bernd Schmidt wrote: > >>On 09/27/2016 02:01 PM, Marek Polacek wrote: > >>>On Tue, Sep 27, 2016 at 01:55:22PM +0200, Bernd Schmidt wrote: > On 09/27/2016 01:51 PM, Marek Polacek wrote: > >But the C/C++ keywords are all English, too; lint tools only accept > >English, > >and so it wouldn't seem unreasonable to only accept English keywords in > >the > >comments. And in any case, I don't see how a compiler can be expected to > >be able to parse non-English languages. > > It isn't. But it can also be reasonably by expected not to warn about > things > that are valid according to the language specification and are frequently > used. > >>> > >>>Ok, but note that the warning is in -Wextra, not enabled by default/-Wall. > >> > >>I think it's problematic enough that it needs to be removed from -Wextra as > >>well. The latest ia64 backend patch shows that clearly IMO. > > > >Just compare that to the number of real bugs the warning found in gcc > >codebase. It is really worth it for -Wextra. > > What's the ratio of comments "fixed" to actual bugs found? IMO this is not > something we should inflict on users unasked. We've inflicted on users many other coding style warnings that have a reasonably high chance to reveal real bugs, e.g. -Wmisleading-indentation which is even enabled in -Wall, not just -Wextra. In reality, -Wextra isn't used that often, and the people that use it will most likely benefit from the warning IMNSHO. Jakub
[PATCH] Omit AIX mapping class from VAR_DECL
AIX Assembler uses storage mapping classes to map symbols and CSECTs to XCOFF file sections. References to symbols with a section to be determined in the future are suppose to use [UA] storage mapping class for "unclassified". rs6000.c:rs6000_output_symbol() adds the [DS] decoration for function symbols and recently added the [UA] decoration for extern data. rs6000_output_symbol() both emitted the decoration and also changed the string in the DECL. For data symbols, this causes a failure in some of the GCC hashing and comparison routines. This patch adjusts rs6000_output_symbol() to only modify the DECL for FUNCTION_DECLs not VAR_DECLs. Bootstrapped on powrepc-ibm-aix7.1.0.0 * config/rs6000/rs6000.c (rs6000_output_symbol): Don't modify VAR_DECL string. Index: rs6000.c === --- rs6000.c(revision 240516) +++ rs6000.c(working copy) @@ -30309,7 +30309,10 @@ rs6000_output_symbol_ref (FILE *file, rtx x) (TREE_CODE (decl) == FUNCTION_DECL ? "[DS]" : "[UA]"), NULL); - XSTR (x, 0) = name; + + /* Don't modify name in extern VAR_DECL to include mapping class. */ + if (TREE_CODE (decl) == FUNCTION_DECL) + XSTR (x, 0) = name; } if (VTABLE_NAME_P (name))
Re: [PATCH] Make -Wint-in-bool-context warn on suspicious shift ops
On 09/27/16 14:49, Florian Weimer wrote:
> * Jason Merrill:
>
>> On Sun, Sep 25, 2016 at 3:46 AM, Bernd Edlinger
>> wrote:
>>> This patch makes -Wint-in-bool-context warn on suspicious integer left
>>> shifts, when the integer is signed, which is most likely some kind of
>>> programming error, for instance using "<<" instead of "<".
>>>
>>> The warning is motivated by the fact, that an overflow on integer shift
>>> left is undefined behavior, even if gcc won't optimize the shift based
>>> on the undefined behavior.
>>>
>>> So in absence of undefined behavior the boolean result does not depend
>>> on the shift value, thus the whole shifting is pointless.
>>
>> It's pointless for unsigned integers, too; why not warn for them as
>> well? And why not warn for 0 << 0 and 1 << 0, which are just as
>> pointless?
>
> “1 << 0“ is often used in a sequence of flag mask definitions. This
> example is from :
>
> | /* Terminal control structure. */
> | struct termios
> | {
> | /* Input modes. */
> | tcflag_t c_iflag;
> | #define IGNBRK (1 << 0)/* Ignore break condition. */
> | #define BRKINT (1 << 1)/* Signal interrupt on break. */
> | #define IGNPAR (1 << 2)/* Ignore characters with parity errors. */
> | #define PARMRK (1 << 3)/* Mark parity and framing errors. */
>
> “0 << 0” is used in a similar context, to create a zero constant for a
> multi-bit subfield of an integer.
>
> This example comes from GDB, in bfd/elf64-alpha.c:
>
> | insn = INSN_ADDQ | (16 << 21) | (0 << 16) | (0 << 0);
>
Of course that is not a boolean context, and will not get a warning.
Question is if "if (1 << 0)" is possibly a miss-spelled "if (1 < 0)".
Maybe 1 and 0 come from macro expansion
Bernd.
Re: Implement -Wimplicit-fallthrough (version 9)
On 09/27/2016 03:49 PM, Jakub Jelinek wrote: On Tue, Sep 27, 2016 at 03:48:07PM +0200, Bernd Schmidt wrote: On 09/27/2016 02:01 PM, Marek Polacek wrote: On Tue, Sep 27, 2016 at 01:55:22PM +0200, Bernd Schmidt wrote: On 09/27/2016 01:51 PM, Marek Polacek wrote: But the C/C++ keywords are all English, too; lint tools only accept English, and so it wouldn't seem unreasonable to only accept English keywords in the comments. And in any case, I don't see how a compiler can be expected to be able to parse non-English languages. It isn't. But it can also be reasonably by expected not to warn about things that are valid according to the language specification and are frequently used. Ok, but note that the warning is in -Wextra, not enabled by default/-Wall. I think it's problematic enough that it needs to be removed from -Wextra as well. The latest ia64 backend patch shows that clearly IMO. Just compare that to the number of real bugs the warning found in gcc codebase. It is really worth it for -Wextra. What's the ratio of comments "fixed" to actual bugs found? IMO this is not something we should inflict on users unasked. Bernd
Re: [PATCH, PING] DWARF: process all TYPE_DECL nodes when iterating on scopes
On 09/07/2016 11:30 AM, Richard Biener wrote:
That said, with the idea of early debug in place and thus giving
more responsibility to the frontends I wonder in what order the Ada
FE calls debug_hooks.early_global_decl ()? Maybe it's the middle-end
and it should arrange for a more natural order on function nests. So
I'd appreciate if you can investigate this side of the issue a bit,
that is, simply avoid the bad ordering.
I investigated this track: it’s not the Ada front-end’s calls to the
debug_hooks.early_global_decl that matter, but actually the ones in
cgraphunit.c (symbol_table::finalize_compilation_unit).
The new patch attached tries to fix the call order. Bootstrapping and
regtesting on x86_64-linux were clean except one testcase where the
debug output changed legitimally (matcher updated). Ok to commit? Thank
you in advance!
--
Pierre-Marie de Rodat
>From 2508b2be4d58ae29cc0093688c50fefea15f6934 Mon Sep 17 00:00:00 2001
From: Pierre-Marie de Rodat
Date: Fri, 23 Sep 2016 18:16:07 +0200
Subject: [PATCH] DWARF: fix scoping for descriptions of local types
In Ada, it is possible to have nested subprograms in the following
configuration:
procedure Parent is
type T;
[...]
procedure Child (Value : T) is
begin
[...]
end Child;
begin
[...]
end Parent;
As we currently generate debugging information for Child first before
Parent, the debug info for T appears in global scope since the DIE for
Parent does not exist yet.
First, this patch makes sure nested function cgraph_node's are created
after their parents'. Then, it reverses the iteration on the symbol
table that calls the early_global_decl debug hook so that debug info for
nested functions is generated after their parents'.
gcc/ChangeLog:
* cgraph.h (symbol_table::last_function_with_gimple_body,
symbol_table::previous_function_with_gimple_body): New methods.
(REVERSE_FOR_EACH_FUNCTION_WITH_GIMPLE_BODY): New iteration macro.
* cgraph.c (symbol_table::call_cgraph_duplication_hooks): Adjust so
that nested functions have their cgraph_node created after their
parents'.
* cgraphunit.c (symbol_table::finalize_compilation_unit): Call the
early_global_decl debug hook on last functions in the symtab list (i.e.
older ones first).
gcc/testsuite/
* gcc/testsuite/g++.dg/debug/dwarf2/auto1.C: Adjust pattern to
accomodate new DIEs output order.
---
gcc/cgraph.c | 12 --
gcc/cgraph.h | 40 +++
gcc/cgraphunit.c | 5 ++--
gcc/testsuite/g++.dg/debug/dwarf2/auto1.C | 2 +-
4 files changed, 54 insertions(+), 5 deletions(-)
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index b702a7c..78d1a85 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -492,6 +492,14 @@ symbol_table::call_cgraph_duplication_hooks (cgraph_node *node,
cgraph_node *
cgraph_node::create (tree decl)
{
+ cgraph_node *origin = NULL;
+
+ /* If DECL is local to a function, make sure we have a node for this function
+ first so that the symbol table has parent functions created before their
+ children. This helps debug information generation. */
+ if (DECL_CONTEXT (decl) && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL)
+origin = cgraph_node::get_create (DECL_CONTEXT (decl));
+
cgraph_node *node = symtab->create_empty ();
gcc_assert (TREE_CODE (decl) == FUNCTION_DECL);
@@ -507,9 +515,9 @@ cgraph_node::create (tree decl)
node->register_symbol ();
- if (DECL_CONTEXT (decl) && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL)
+ if (origin != NULL)
{
- node->origin = cgraph_node::get_create (DECL_CONTEXT (decl));
+ node->origin = origin;
node->next_nested = node->origin->nested;
node->origin->nested = node;
}
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index ecafe63..6b1181c 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -2097,6 +2097,13 @@ public:
/* Return next reachable static variable with initializer after NODE. */
inline cgraph_node *next_function_with_gimple_body (cgraph_node *node);
+ /* Return last function with body defined. */
+ cgraph_node *last_function_with_gimple_body (void);
+
+ /* Return previous reachable static variable with initializer before
+ NODE. */
+ inline cgraph_node *previous_function_with_gimple_body (cgraph_node *node);
+
/* Register HOOK to be called with DATA on each removed edge. */
cgraph_edge_hook_list *add_edge_removal_hook (cgraph_edge_hook hook,
void *data);
@@ -2775,6 +2782,36 @@ symbol_table::next_function_with_gimple_body (cgraph_node *node)
return NULL;
}
+/* Return last function with body defined. */
+inline cgraph_node *
+symbol_table::last_function_with_gimple_body (void)
+{
+ cgraph_node *res = NULL;
+ symtab_node *node;
+
+ for (node = nodes; node; node = node->next)
+{
+ cgraph_node *cn = dyn_cast (node);
+ if (cn && cn->has_gimple_body_p
