[Ada] Fix PR ada/71817
Tested on PowerPC/VxWorks, applied on the mainline. 2016-07-11 Eric Botcazou PR ada/71817 * adaint.c (__gnat_is_read_accessible_file): Add parentheses. (__gnat_is_write_accessible_file): Likewise. -- Eric BotcazouIndex: adaint.c === --- adaint.c (revision 238156) +++ adaint.c (working copy) @@ -1924,7 +1924,7 @@ __gnat_is_read_accessible_file (char *na #elif defined (__vxworks) int fd; - if (fd = open (name, O_RDONLY, 0) < 0) + if ((fd = open (name, O_RDONLY, 0)) < 0) return 0; close (fd); return 1; @@ -1997,7 +1997,7 @@ __gnat_is_write_accessible_file (char *n #elif defined (__vxworks) int fd; - if (fd = open (name, O_WRONLY, 0) < 0) + if ((fd = open (name, O_WRONLY, 0)) < 0) return 0; close (fd); return 1;
[PATCH] Introduce new param: AVG_LOOP_NITER
Hello.
During investigation of an IVOPTs issue, I noticed that tree-ssa-loop-ivopts.c
uses a hard-coded
constant AVG_LOOP_NITER. Very similar constant can be seen in cfgloopanal.c.
Thus, I've changed
that to a new param.
Apart from that, I would like to change the default value to 10.
As numbers of SPEC CPU2006 show:
Loop count: 18236
avg. # of iter: 11908.46
median # of iter: 10.00
avg. (1% cutoff) # of iter: 1003.87
avg. (5% cutoff) # of iter: 449.78
avg. (10% cutoff) # of iter: 209.46
avg. (20% cutoff) # of iter: 62.24
avg. (30% cutoff) # of iter: 42.69
Even though the average number of loop iteration is quite high, let's start to
increase it
conservatively to 10. Benchmark results are within a noise level (-Ofast runs
faster by 0.47%).
Patch survives regression tests and bootstraps on x86_64-linux-gnu.
Ready for trunk?
Thanks,
Martin
>From 604637de550a9d23b8a509cdbf493a7212a186f0 Mon Sep 17 00:00:00 2001
From: marxin
Date: Tue, 21 Jun 2016 14:52:31 +0200
Subject: [PATCH] Introduce new param: AVG_LOOP_NITER
gcc/ChangeLog:
2016-06-21 Martin Liska
* params.def: Add avg-loop niter.
* tree-ssa-loop-ivopts.c (avg_loop_niter): Use the param.
* cfgloopanal.c (expected_loop_iterations_unbounded): Likewise.
---
gcc/cfgloopanal.c | 9 -
gcc/params.def | 5 +
gcc/tree-ssa-loop-ivopts.c | 7 +++
3 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/gcc/cfgloopanal.c b/gcc/cfgloopanal.c
index c163986..2739f44 100644
--- a/gcc/cfgloopanal.c
+++ b/gcc/cfgloopanal.c
@@ -241,10 +241,9 @@ expected_loop_iterations_unbounded (const struct loop *loop,
if (read_profile_p)
*read_profile_p = false;
- /* Average loop rolls about 3 times. If we have no profile at all, it is
- best we can do. */
+ /* If we have no profile at all, use AVG_LOOP_NITER. */
if (profile_status_for_fn (cfun) == PROFILE_ABSENT)
-expected = 3;
+expected = PARAM_VALUE (PARAM_AVG_LOOP_NITER);
else if (loop->latch->count || loop->header->count)
{
gcov_type count_in, count_latch;
@@ -282,9 +281,9 @@ expected_loop_iterations_unbounded (const struct loop *loop,
if (freq_in == 0)
{
- /* If we have no profile at all, expect 3 iterations. */
+ /* If we have no profile at all, use AVG_LOOP_NITER iterations. */
if (!freq_latch)
- expected = 3;
+ expected = PARAM_VALUE (PARAM_AVG_LOOP_NITER);
else
expected = freq_latch * 2;
}
diff --git a/gcc/params.def b/gcc/params.def
index 894b7f3..b86d592 100644
--- a/gcc/params.def
+++ b/gcc/params.def
@@ -527,6 +527,11 @@ DEFPARAM(PARAM_IV_ALWAYS_PRUNE_CAND_SET_BOUND,
"If number of candidates in the set is smaller, we always try to remove unused ivs during its optimization.",
10, 0, 0)
+DEFPARAM(PARAM_AVG_LOOP_NITER,
+ "avg-loop-niter",
+ "Average number of iterations of a loop.",
+ 10, 1, 0)
+
DEFPARAM(PARAM_SCEV_MAX_EXPR_SIZE,
"scev-max-expr-size",
"Bound on size of expressions used in the scalar evolutions analyzer.",
diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c
index 6b403f5..d401c41 100644
--- a/gcc/tree-ssa-loop-ivopts.c
+++ b/gcc/tree-ssa-loop-ivopts.c
@@ -115,8 +115,6 @@ along with GCC; see the file COPYING3. If not see
/* The infinite cost. */
#define INFTY 1000
-#define AVG_LOOP_NITER(LOOP) 5
-
/* Returns the expected number of loop iterations for LOOP.
The average trip count is computed from profile data if it
exists. */
@@ -128,8 +126,9 @@ avg_loop_niter (struct loop *loop)
if (niter == -1)
{
niter = likely_max_stmt_executions_int (loop);
- if (niter == -1 || niter > AVG_LOOP_NITER (loop))
- return AVG_LOOP_NITER (loop);
+
+ if (niter == -1 || niter > PARAM_VALUE (PARAM_AVG_LOOP_NITER))
+ return PARAM_VALUE (PARAM_AVG_LOOP_NITER);
}
return niter;
--
2.8.4
[Ada] Small tweak in LTO mode
This make sure the TREE_READONLY flag is set consistently on the definition
and the references of public entities in LTO mode.
Tested on x86_64-suse-linux, applied on the mainline.
2016-07-11 Eric Botcazou
* gcc-interface/trans.c (add_decl_expr): Minor tweak.
* gcc-interface/utils.c (create_var_decl): For an external variable,
also clear TREE_READONLY in LTO mode if the initializer is not a valid
constant and set DECL_READONLY_ONCE_ELAB instead.
--
Eric BotcazouIndex: gcc-interface/utils.c
===
--- gcc-interface/utils.c (revision 323148)
+++ gcc-interface/utils.c (revision 323149)
@@ -2430,8 +2430,9 @@ create_var_decl (tree name, tree asm_nam
and may be used for scalars in general but not for aggregates. */
tree var_decl
= build_decl (input_location,
- (constant_p && const_decl_allowed_p
- && !AGGREGATE_TYPE_P (type)) ? CONST_DECL : VAR_DECL,
+ (constant_p
+ && const_decl_allowed_p
+ && !AGGREGATE_TYPE_P (type) ? CONST_DECL : VAR_DECL),
name, type);
/* Detect constants created by the front-end to hold 'reference to function
@@ -2456,9 +2457,20 @@ create_var_decl (tree name, tree asm_nam
constant initialization and save any variable elaborations for the
elaboration routine. If we are just annotating types, throw away the
initialization if it isn't a constant. */
- if ((extern_flag && !constant_p)
+ if ((extern_flag && init && !constant_p)
|| (type_annotate_only && init && !TREE_CONSTANT (init)))
-init = NULL_TREE;
+{
+ init = NULL_TREE;
+
+ /* In LTO mode, also clear TREE_READONLY the same way add_decl_expr
+ would do it if the initializer was not thrown away here, as the
+ WPA phase requires a consistent view across compilation units. */
+ if (const_flag && flag_generate_lto)
+ {
+ const_flag = false;
+ DECL_READONLY_ONCE_ELAB (var_decl) = 1;
+ }
+}
/* At the global level, a non-constant initializer generates elaboration
statements. Check that such statements are allowed, that is to say,
Index: gcc-interface/trans.c
===
--- gcc-interface/trans.c (revision 323148)
+++ gcc-interface/trans.c (revision 323149)
@@ -8014,7 +8014,7 @@ void
add_decl_expr (tree gnu_decl, Entity_Id gnat_entity)
{
tree type = TREE_TYPE (gnu_decl);
- tree gnu_stmt, gnu_init, t;
+ tree gnu_stmt, gnu_init;
/* If this is a variable that Gigi is to ignore, we may have been given
an ERROR_MARK. So test for it. We also might have been given a
@@ -8061,15 +8061,6 @@ add_decl_expr (tree gnu_decl, Entity_Id
&& !initializer_constant_valid_p (gnu_init,
TREE_TYPE (gnu_init)
{
- /* If GNU_DECL has a padded type, convert it to the unpadded
- type so the assignment is done properly. */
- if (TYPE_IS_PADDING_P (type))
- t = convert (TREE_TYPE (TYPE_FIELDS (type)), gnu_decl);
- else
- t = gnu_decl;
-
- gnu_stmt = build_binary_op (INIT_EXPR, NULL_TREE, t, gnu_init);
-
DECL_INITIAL (gnu_decl) = NULL_TREE;
if (TREE_READONLY (gnu_decl))
{
@@ -8077,6 +8068,12 @@ add_decl_expr (tree gnu_decl, Entity_Id
DECL_READONLY_ONCE_ELAB (gnu_decl) = 1;
}
+ /* If GNU_DECL has a padded type, convert it to the unpadded
+ type so the assignment is done properly. */
+ if (TYPE_IS_PADDING_P (type))
+ gnu_decl = convert (TREE_TYPE (TYPE_FIELDS (type)), gnu_decl);
+
+ gnu_stmt = build_binary_op (INIT_EXPR, NULL_TREE, gnu_decl, gnu_init);
add_stmt_with_node (gnu_stmt, gnat_entity);
}
}
Re: [PATCH/AARCH64] Add rtx_costs routine for vulcan.
On Fri, Jul 08, 2016 at 04:01:33PM +0530, Virendra Pathak wrote: > Hi James, > > Please find the patch after taking care of your comments. > > > > Did you see those patches, and did you consider whether there would be a > > benefit to doing the same for Vulcan? > In our simulation environment, we did not observe any performance gain > for specfp2006. > However, we did it to keep the cost strategy same as cortexa-57/53. > > Please review and merge to trunk. Hi Virendra, This patch is OK. Julian has commit rights and is on the ChangeLog, so I'll let him commit the patch on your behalf. Thanks, James > gcc/ChangeLog: > > Virendra Pathak > Julian Brown > > * config/aarch64/aarch64-cores.def: Update vulcan COSTS. > * config/aarch64/aarch64-cost-tables.h > (vulcan_extra_costs): New variable. > * config/aarch64/aarch64.c > (vulcan_addrcost_table): Likewise. > (vulcan_regmove_cost): Likewise. > (vulcan_vector_cost): Likewise. > (vulcan_branch_cost): Likewise. > (vulcan_tunings): Likewise.
Re: [PATCH, rs6000] Fix PR target/71733, ICE with -mcpu=power9 -mno-vsx
On Mon, Jul 11, 2016 at 04:07:11PM +0930, Alan Modra wrote: > I believe all the r alternatives in altivec_mov should be > disparaged with "?". That certainly makes sense. Okay for trunk if tested (also test with LRA, if you can). Thanks! Segher
Re: [PATCH/AARCH64] Add rtx_costs routine for vulcan.
Hi James, > This patch is OK. Thanks for the review. > Julian has commit rights and is on the ChangeLog, so I'll let him > commit the patch on your behalf. Julian, Kindly commit the patch to the trunk. Thanks. with regards, Virendra Pathak On Mon, Jul 11, 2016 at 2:28 PM, James Greenhalgh wrote: > On Fri, Jul 08, 2016 at 04:01:33PM +0530, Virendra Pathak wrote: >> Hi James, >> >> Please find the patch after taking care of your comments. >> >> >> > Did you see those patches, and did you consider whether there would be a >> > benefit to doing the same for Vulcan? >> In our simulation environment, we did not observe any performance gain >> for specfp2006. >> However, we did it to keep the cost strategy same as cortexa-57/53. >> >> Please review and merge to trunk. > > Hi Virendra, > > This patch is OK. > > Julian has commit rights and is on the ChangeLog, so I'll let him > commit the patch on your behalf. > > Thanks, > James > >> gcc/ChangeLog: >> >> Virendra Pathak >> Julian Brown >> >> * config/aarch64/aarch64-cores.def: Update vulcan COSTS. >> * config/aarch64/aarch64-cost-tables.h >> (vulcan_extra_costs): New variable. >> * config/aarch64/aarch64.c >> (vulcan_addrcost_table): Likewise. >> (vulcan_regmove_cost): Likewise. >> (vulcan_vector_cost): Likewise. >> (vulcan_branch_cost): Likewise. >> (vulcan_tunings): Likewise. >
Re: RFA: new pass to warn on questionable uses of alloca() and VLAs
On 07/10/2016 07:41 PM, Manuel López-Ibáñez wrote: +Walloca +LangEnabledBy(C ObjC C++ ObjC++) +; in common.opt + +Walloca= +LangEnabledBy(C ObjC C++ ObjC++) +; in common.opt + I'm not sure what you think the above means, but this is an invalid use of LangEnabledBy(). (It would be nice if the .awk scripts were able to catch it and give an error.) I was following the practice for -Warray-bounds in c-family/c.opt: Warray-bounds LangEnabledBy(C ObjC C++ ObjC++,Wall) ; in common.opt Warray-bounds= LangEnabledBy(C ObjC C++ ObjC++,Wall,1,0) ; in common.opt ...as well as the generic version in common.opt: Warray-bounds Common Var(warn_array_bounds) Warning Warn if an array is accessed out of bounds. Warray-bounds= Common Joined RejectNegative UInteger Var(warn_array_bounds) Warning Warn if an array is accessed out of bounds. I *thought* you defined the option in common.opt, and narrowed the language variants in c-family/c.opt. ?? +;; warn_vla == 0 for -Wno-vla +;; warn_vla == -1 for nothing passed. +;; warn_vla == -2 for -Wvla passed +;; warn_vla == NUM for -Wvla=NUM Wvla C ObjC C++ ObjC++ Var(warn_vla) Init(-1) Warning Warn if a variable length array is used. +Wvla= +C ObjC C++ ObjC++ Var(warn_vla) Warning Joined RejectNegative UInteger +-Wvla= Warn on unbounded uses of variable-length arrays, and +warn on bounded uses of variable-length arrays whose bound can be +larger than bytes. + This overloading of warn_vla seems confusing (as shown by all the places that require updating). Why not call it Wvla-larger-than= and use a different warn_vla_larger_than variable? We already have -Wlarger-than= and -Wframe-larger-than=. Hey, I'm all for different options. It would definitely be easier for me :). I wast trying really hard to use -Wvla= and keep the current -Wvla meaning the same. Though I see your point about using -Wvla* but using different variables for representing -Wvla and -Wvla=blah. The easiest thing would be: -Wvla: Current behavior (warn on everything). -Wvla-larger-than=: Warn on unbounded VLA and bounded VLAs > . -Walloca: Warn on every use of alloca (analogous to -Wvla). -Walloca-larger-than=: Warn on unbounded alloca and bounded allocas > . This seems straightforward and avoids all the overloading you see. Would this be ok with everyone? Using warn_vla_larger_than (even if you wish to keep -Wvla= as the option name) as a variable distinct from warn_vla would make things simpler: -Wvla => warn_vla == 1 -Wno-vla => warn_vla == 0 -Wvla=N => warn_vla_larger_than == N (where N == 0 means disable). If you wish that -Wno-vla implies -Wvla=0 then you'll need to handle that manually. I don't think we have support for that in the .opt files. But that seems far less complex than having a single shared Var(). Cheers, Manuel. Thanks. Aldy
[PATCH, PR ipa/71633] Fix inlining into thunks
Hi,
Currently when we expand thunk in inliner we assume its body
has a single call. This is wrong for cases when thunk is
instrumented. It means we might try to continue inlining
for wrong edge. This simple patch fixes it.
Bootstrapped and regtested on x86_64-unknown-linux-gnu.
OK for trunk?
Thanks,
Ilya
--
gcc/
2016-07-11 Ilya Enkovich
PR ipa/71633
* ipa-inline-transform.c (inline_call): Support
instrumented thunks.
gcc/testsuite/
2016-07-11 Ilya Enkovich
PR ipa/71633
* g++.dg/pr71633.C: New test.
diff --git a/gcc/ipa-inline-transform.c b/gcc/ipa-inline-transform.c
index 9ac1efc..a4ae305 100644
--- a/gcc/ipa-inline-transform.c
+++ b/gcc/ipa-inline-transform.c
@@ -319,10 +319,14 @@ inline_call (struct cgraph_edge *e, bool update_original,
to = to->global.inlined_to;
if (to->thunk.thunk_p)
{
+ struct cgraph_node *target = to->callees->callee;
if (in_lto_p)
to->get_untransformed_body ();
to->expand_thunk (false, true);
- e = to->callees;
+ /* When thunk is instrumented we may have multiple callees. */
+ for (e = to->callees; e && e->callee != target; e = e->next_callee)
+ ;
+ gcc_assert (e);
}
diff --git a/gcc/testsuite/g++.dg/pr71633.C b/gcc/testsuite/g++.dg/pr71633.C
new file mode 100644
index 000..bb69bbb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr71633.C
@@ -0,0 +1,28 @@
+/* PR71633 */
+// { dg-do compile { target i?86-*-* x86_64-*-* } }
+/* { dg-options "-fcheck-pointer-bounds -mmpx -O2" } */
+
+class c1
+{
+ virtual void fn1 ();
+};
+
+class c2
+{
+ virtual int *fn2 () const;
+};
+
+class c3 : c1, c2
+{
+ int *fn2 () const;
+ int *fn3 (int) const;
+};
+
+int *c3::fn2 () const
+{
+}
+
+int *c3::fn3 (int p) const
+{
+ return fn3 (p);
+}
Re: [PATCH] rs6000: Make the ctr* patterns allow ints in vector regs (PR71763)
On Fri, Jul 08, 2016 at 04:17:36AM -0500, Segher Boessenkool wrote: > Maybe just UNSPEC_BDZ? UNSPEC_DOLOOP? Committed revision 238207, using UNSPEC_DOLOOP. -- Alan Modra Australia Development Lab, IBM
Re: [PATCH] rs6000: Make the ctr* patterns allow ints in vector regs (PR71763)
Should we backport this? At least Alan's UNSPEC_DOLOOP part? - David On Mon, Jul 11, 2016 at 6:30 AM, Alan Modra wrote: > On Fri, Jul 08, 2016 at 04:17:36AM -0500, Segher Boessenkool wrote: >> Maybe just UNSPEC_BDZ? UNSPEC_DOLOOP? > > Committed revision 238207, using UNSPEC_DOLOOP. > > -- > Alan Modra > Australia Development Lab, IBM
New libstdc++ testsuite failures on AIX
Jason, I believe that one of the patches in your recent C++ series introduced some new libstdc++ testsuite failures on AIX. With r238197 the problem exists and with r238169 (after the libstdc++ changes but before your C++ patches) the failures are not present. Note that this was tested with Richi's patch reverted. FAIL: 23_containers/list/debug/insert4_neg.cc (test for excess errors) Excess errors: /tmp/20160708/powerpc-ibm-aix7.1.0.0/libstdc++-v3/include/debug/formatter.h:387:7: error: __gnu_debug::_Error_formatter& __gnu_debug::_Error_formatter::_M_iterator(const _Iterator&, const char*) [with _Iterator = __gnu_debug::_Safe_iterator , std::__debug::list >] causes a section type conflict with __gnu_debug::_Error_formatter& __gnu_debug::_Error_formatter::_M_iterator(const _Iterator&, const char*) [with _Iterator = __gnu_debug::_Safe_iterator, std::__debug::list >] FAIL: 23_containers/deque/debug/insert4_neg.cc (test for excess errors) Excess errors: /tmp/20160708/powerpc-ibm-aix7.1.0.0/libstdc++-v3/include/debug/formatter.h:387:7: error: __gnu_debug::_Error_formatter& __gnu_debug::_Error_formatter::_M_iterator(const _Iterator&, const char*) [with _Iterator = __gnu_debug::_Safe_iterator , std::__debug::deque >] causes a section type conflict with __gnu_debug::_Error_formatter& __gnu_debug::_Error_formatter::_M_iterator(const _Iterator&, const char*) [with _Iterator = __gnu_debug::_Safe_iterator, std::__debug::deque >] Thanks, David
Re: [PATCH] rs6000: Make the ctr* patterns allow ints in vector regs (PR71763)
On Mon, Jul 11, 2016 at 06:54:44AM -0400, David Edelsohn wrote: > Should we backport this? At least Alan's UNSPEC_DOLOOP part? The *wi is a bugfix; I'll backport it, just like the *d (already did that one, 6 and 5; do we want 4.9 as well?) Alan's last patch would be good to have as well, it is arguably a bugfix, and it avoids most of the problematic cases. Segher
Re: [PATCH v5] Allocate constant size dynamic stack space in the prologue
On Thu, Jul 07, 2016 at 12:57:16PM +0100, Dominik Vogt wrote:
> On Wed, Jul 06, 2016 at 02:01:06PM +0200, Bernd Schmidt wrote:
> > There's one thing I don't quite understand and which seems to have
> > changed since v1:
> >
> > On 07/04/2016 02:19 PM, Dominik Vogt wrote:
> > >@@ -1099,8 +1101,10 @@ expand_stack_vars (bool (*pred) (size_t), struct
> > >stack_vars_data *data)
> > >
> > > /* If there were any, allocate space. */
> > > if (large_size > 0)
> > >- large_base = allocate_dynamic_stack_space (GEN_INT (large_size), 0,
> > >- large_align, true);
> > >+ {
> > >+large_allocsize = GEN_INT (large_size);
> > >+get_dynamic_stack_size (&large_allocsize, 0, large_align, NULL);
> > >+ }
> > > }
> > >
> > > for (si = 0; si < n; ++si)
> > >@@ -1186,6 +1190,19 @@ expand_stack_vars (bool (*pred) (size_t), struct
> > >stack_vars_data *data)
> > > /* Large alignment is only processed in the last pass. */
> > > if (pred)
> > > continue;
> > >+
> > >+if (large_allocsize && ! large_allocation_done)
> > >+ {
> > >+/* Allocate space the virtual stack vars area in the
> > >+ prologue. */
> > >+HOST_WIDE_INT loffset;
> > >+
> > >+loffset = alloc_stack_frame_space
> > >+ (INTVAL (large_allocsize),
> > >+ PREFERRED_STACK_BOUNDARY / BITS_PER_UNIT);
> > >+large_base = get_dynamic_stack_base (loffset, large_align);
> > >+large_allocation_done = true;
> > >+ }
> > > gcc_assert (large_base != NULL);
> > >
> >
> > Why is this code split between the two places here?
>
> This is a bugfix from v1 to v2.
> I think I had to move this code to the second loop so that the
> space for dynamically aligned variables is allocated at the "end"
> of the space for stack variables. I cannot remember what the bug
> was, but maybe it was that the variables with fixed and static
> alignment ended up at the same address.
>
> Anyway, now that the new allocation strategy is used
> unconditionally, it should be possible to move the
> get_dynamic_stack_size call down to the second loop and simplify
> the code somewhat. I'll look into that.
Version 5 with some code moved from the first loop to the second.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
Patch ping
Hi! I'd like to ping PR71716 - fix hang with long double atomics http://gcc.gnu.org/ml/gcc-patches/2016-07/msg00045.html Thanks Jakub
Re: [PATCH, rs6000] Fix PR target/71733, ICE with -mcpu=power9 -mno-vsx
Alan Modra wrote: > The reason this fails is that no alternative in altivec_mov > exactly matches. Ideally reload would choose the Z,v alternative > (cost 9) and you'd get an address reload for the mem to make it match > the "Z" constraint. Instead reload chooses the Y,r alternative (cost > 8) as best. > > That's a bad choice. "Y" matches the r3+16 mem so no reloads needed > for the mem, however the altivec reg needs to be reloaded to gprs. > Without direct moves from altivec to gpr the reload needs to go via > mem, so you get an altivec store to stack plus gpr load from stack. > The store to stack has the same form as the original store. Oops, no > progress. > > Even if direct moves were available from altivec regs to gprs, the Y,r > alternative would still be a bad choice. > > I believe all the r alternatives in altivec_mov should be > disparaged with "?". I agree that this makes sense just from a code quality perspective. However, there still seems to be another underlying problem: reload should never generate invalid instructions. (Playing with '?' to fix *inefficient* instructions is fine, but we shouldn't rely on '?' to fix *invalid* instructions.) If reload -for whatever reason- wants to emit a vr->gpr move, then it should generate a valid instruction sequence to do so, via secondary reloads if necessary. I think it would be a good idea to investigate why that isn't happening in this test case. [ There is a chance that the underlying bug will reappear in a different context, even after the '?' change is applied. ] Bye, Ulrich -- Dr. Ulrich Weigand GNU/Linux compilers and toolchain ulrich.weig...@de.ibm.com
Re: [patch] Fix type merging deficiency during WPA
> So sth like
>
> Index: gcc/lto-streamer-out.c
> ===
> --- gcc/lto-streamer-out.c (revision 238039)
> +++ gcc/lto-streamer-out.c (working copy)
> @@ -996,7 +996,7 @@ hash_tree (struct streamer_tree_cache_d
>else
> hstate.add_flag (TREE_NO_WARNING (t));
>hstate.add_flag (TREE_NOTHROW (t));
> - hstate.add_flag (TREE_STATIC (t));
> + //hstate.add_flag (TREE_STATIC (t));
>hstate.add_flag (TREE_PROTECTED (t));
>hstate.add_flag (TREE_DEPRECATED (t));
>if (code != TREE_BINFO)
> @@ -1050,7 +1050,7 @@ hash_tree (struct streamer_tree_cache_d
>hstate.add_flag (DECL_ARTIFICIAL (t));
>hstate.add_flag (DECL_USER_ALIGN (t));
>hstate.add_flag (DECL_PRESERVE_P (t));
> - hstate.add_flag (DECL_EXTERNAL (t));
> + //hstate.add_flag (DECL_EXTERNAL (t));
>hstate.add_flag (DECL_GIMPLE_REG_P (t));
>hstate.commit_flag ();
>hstate.add_int (DECL_ALIGN (t));
> Index: gcc/lto/lto.c
> ===
> --- gcc/lto/lto.c (revision 238039)
> +++ gcc/lto/lto.c (working copy)
> @@ -1263,7 +1263,8 @@ compare_tree_sccs_1 (tree t1, tree t2, t
> tree t1_ = (E1), t2_ = (E2); \
> if (t1_ != t2_ \
> && (!t1_ || !t2_ \
> - || !TREE_VISITED (t2_) \
> + || (!TREE_VISITED (t2_) \
> + && !same_decls (t1_, t2_)) \
>
> || (!TREE_ASM_WRITTEN (t2_) \
>
> && !compare_tree_sccs_1 (t1_, t2_, map \
>return false; \
>
> plus the magic new function same_decls which would check if the trees are
> var/function decls with the same assembler name. The function can't use
> the cgraph as that is not yet read in unfortunately - I still think we
> should do that so we can see the prevailing nodes early.
OK, thanks for the hint, the attached patch where the call to same_decls is
placed differently works on the testcase (modulo the gigi change I posted
earlier today) but not on the original code... It looks like the hash value
computed in lto-streamer-out.c is stricter than the hash value computed for
canonical types, I'm trying to pinpoint the difference.
* lto-streamer-out.c (hash_tree): Do not add the TREE_STATIC and
DECL_EXTERNAL flags for variable or function declarations.
lto/
* lto.c (same_global_decl_p): New predicate.
(compare_tree_sccs_1): Use it in the comparison of pointer fields.
(walk_simple_constant_arithmetic): New function.
(LTO_SET_PREVAIL): Use it to fix up DECL nodes in simple expressions.
--
Eric BotcazouIndex: lto-streamer-out.c
===
--- lto-streamer-out.c (revision 238156)
+++ lto-streamer-out.c (working copy)
@@ -996,7 +996,9 @@ hash_tree (struct streamer_tree_cache_d
else
hstate.add_flag (TREE_NO_WARNING (t));
hstate.add_flag (TREE_NOTHROW (t));
- hstate.add_flag (TREE_STATIC (t));
+ /* We want to unify DECL nodes in pointer fields of global types. */
+ if (!(VAR_OR_FUNCTION_DECL_P (t)))
+hstate.add_flag (TREE_STATIC (t));
hstate.add_flag (TREE_PROTECTED (t));
hstate.add_flag (TREE_DEPRECATED (t));
if (code != TREE_BINFO)
@@ -1050,7 +1052,9 @@ hash_tree (struct streamer_tree_cache_d
hstate.add_flag (DECL_ARTIFICIAL (t));
hstate.add_flag (DECL_USER_ALIGN (t));
hstate.add_flag (DECL_PRESERVE_P (t));
- hstate.add_flag (DECL_EXTERNAL (t));
+ /* We want to unify DECL nodes in pointer fields of global types. */
+ if (!(VAR_OR_FUNCTION_DECL_P (t)))
+ hstate.add_flag (DECL_EXTERNAL (t));
hstate.add_flag (DECL_GIMPLE_REG_P (t));
hstate.commit_flag ();
hstate.add_int (DECL_ALIGN (t));
Index: lto/lto.c
===
--- lto/lto.c (revision 238156)
+++ lto/lto.c (working copy)
@@ -970,6 +970,27 @@ static unsigned long num_sccs_merged;
static unsigned long num_scc_compares;
static unsigned long num_scc_compare_collisions;
+/* Return true if T1 and T2 represent the same global declaration.
+
+ We need to unify declaration nodes in pointer fields of global types
+ so as to unify the types themselves before the declarations are merged. */
+
+static inline bool
+same_global_decl_p (tree t1, tree t2)
+{
+ if (TREE_CODE (t1) != TREE_CODE (t2) || !VAR_OR_FUNCTION_DECL_P (t1))
+return false;
+
+ if (!(TREE_PUBLIC (t1) || DECL_EXTERNAL (t1)))
+return false;
+
+ if (!(TREE_PUBLIC (t2) || DECL_EXTERNAL (t2)))
+return false;
+
+ return symbol_table::assembler_names_equal_p
+ (IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (t1)),
+ IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (t2)));
+}
/* Compare the two entries T1 and T2 of two SCCs that are possibly equal,
recursing through in-SCC tree edges. Returns true if the SCCs entered
@@ -1262,6 +1283,7 @@ compare_tree_sccs_1 (tree
[PATCH] Fix PR71816
The following patch fixes PR71816.
Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.
Richard.
2016-07-11 Richard Biener
PR tree-optimization/71816
* tree-ssa-pre.c (compute_avail): Adjust alignment of ref rather
than replacing all of its operands.
* gcc.dg/torture/pr71816.c: New testcase.
Index: gcc/tree-ssa-pre.c
===
*** gcc/tree-ssa-pre.c (revision 238206)
--- gcc/tree-ssa-pre.c (working copy)
*** compute_avail (void)
*** 3791,3803
|| ref1->opcode == MEM_REF)
&& (TYPE_ALIGN (ref1->type)
> TYPE_ALIGN (ref2->type)))
! {
! ref->operands.release ();
! ref->operands = operands;
! ref1 = ref2;
! }
! else
! operands.release ();
/* TBAA behavior is an obvious part so make sure
that the hashtable one covers this as well
by adjusting the ref alias set and its base. */
--- 3791,3799
|| ref1->opcode == MEM_REF)
&& (TYPE_ALIGN (ref1->type)
> TYPE_ALIGN (ref2->type)))
! ref1->type
! = build_aligned_type (ref1->type,
! TYPE_ALIGN (ref2->type));
/* TBAA behavior is an obvious part so make sure
that the hashtable one covers this as well
by adjusting the ref alias set and its base. */
*** compute_avail (void)
*** 3824,3829
--- 3820,3826
ref1->op2 = fold_convert (ptr_type_node,
ref1->op2);
}
+ operands.release ();
result = pre_expr_pool.allocate ();
result->kind = REFERENCE;
Index: gcc/testsuite/gcc.dg/torture/pr71816.c
===
*** gcc/testsuite/gcc.dg/torture/pr71816.c (revision 0)
--- gcc/testsuite/gcc.dg/torture/pr71816.c (working copy)
***
*** 0
--- 1,21
+ /* { dg-do compile } */
+
+ void *ext2fs_resize_mem_p;
+ struct ext2_icount_el {
+ int ino;
+ } * insert_icount_el_icount_1;
+ int insert_icount_el_icount, insert_icount_el_new_size;
+ void *memcpy();
+ void *realloc();
+ int ext2fs_resize_mem(void *p1) {
+ int size = 0;
+ memcpy(&ext2fs_resize_mem_p, p1, sizeof(ext2fs_resize_mem_p));
+ realloc(&ext2fs_resize_mem_p, size);
+ return 0;
+ }
+ struct ext2_icount_el *insert_icount_el() {
+ if (insert_icount_el_icount)
+ insert_icount_el_new_size = insert_icount_el_icount_1[0].ino;
+ ext2fs_resize_mem(&insert_icount_el_icount_1);
+ return 0;
+ }
Re: [PATCH, vec-tails 01/10] New compiler options
Ping
2016-06-16 16:42 GMT+03:00 Ilya Enkovich :
> On 20 May 14:40, Ilya Enkovich wrote:
>> > Can you make all these --params then? I think to be useful to users we'd
>> > want
>> > them to be loop pragmas rather than options.
>>
>> OK, I'll change it to params. I didn't think about control via
>> pragmas but will do now.
>>
>> Thanks,
>> Ilya
>>
>> >
>> > Richard.
>> >
>
> Hi,
>
> Here is a set of params to be used instead of new flags. Does this set looks
> OK?
> I still use new option for cost model for convenient soct model enum re-use.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2016-06-16 Ilya Enkovich
>
> * common.opt (fvect-epilogue-cost-model=): New.
> * params.def (PARAM_VECT_EPILOGUES_COMBINE): New.
> (PARAM_VECT_EPILOGUES_MASK): New.
> (PARAM_VECT_EPILOGUES_NOMASK): New.
> (PARAM_VECT_SHORT_LOOPS): New.
> * doc/invoke.texi (-fvect-epilogue-cost-model): New.
>
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index fccd4b5..10cd75b 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -2582,6 +2582,10 @@ fsimd-cost-model=
> Common Joined RejectNegative Enum(vect_cost_model) Var(flag_simd_cost_model)
> Init(VECT_COST_MODEL_UNLIMITED) Optimization
> Specifies the vectorization cost model for code marked with a simd directive.
>
> +fvect-epilogue-cost-model=
> +Common Joined RejectNegative Enum(vect_cost_model)
> Var(flag_vect_epilogue_cost_model) Init(VECT_COST_MODEL_DEFAULT) Optimization
> +Specifies the cost model for epilogue vectorization.
> +
> Enum
> Name(vect_cost_model) Type(enum vect_cost_model) UnknownError(unknown
> vectorizer cost model %qs)
>
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index ce162a0..ecbd7ce 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -7638,6 +7638,14 @@ or Cilk Plus simd directive. The @var{model} argument
> should be one of
> have the same meaning as described in @option{-fvect-cost-model} and by
> default a cost model defined with @option{-fvect-cost-model} is used.
>
> +@item -fvect-epilogue-cost-model=@var{model}
> +@opindex fvect-epilogue-cost-model
> +Alter the cost model used for vectorization of loop epilogues. The
> +@var{model} argument should be one of @samp{unlimited}, @samp{dynamic},
> +@samp{cheap}. All values of @var{model} have the same meaning as
> +described in @option{-fvect-cost-model} and by default @samp{dynamic}
> +cost model is used.
> +
> @item -ftree-vrp
> @opindex ftree-vrp
> Perform Value Range Propagation on trees. This is similar to the
> diff --git a/gcc/params.def b/gcc/params.def
> index 62a1e40..3bac68c 100644
> --- a/gcc/params.def
> +++ b/gcc/params.def
> @@ -1220,6 +1220,28 @@ DEFPARAM (PARAM_MAX_SPECULATIVE_DEVIRT_MAYDEFS,
> "Maximum number of may-defs visited when devirtualizing "
> "speculatively", 50, 0, 0)
>
> +DEFPARAM (PARAM_VECT_EPILOGUES_COMBINE,
> + "vect-epilogues-combine",
> + "Enable loop epilogue vectorization by combining it with "
> + "vectorized loop body.",
> + 0, 0, 1)
> +
> +DEFPARAM (PARAM_VECT_EPILOGUES_MASK,
> + "vect-epilogues-mask",
> + "Enable loop epilogue vectorization using the same vector "
> + "size and masking.",
> + 0, 0, 1)
> +
> +DEFPARAM (PARAM_VECT_EPILOGUES_NOMASK,
> + "vect-epilogues-nomask",
> + "Enable loop epilogue vectorization using smaller vector size.",
> + 0, 0, 1)
> +
> +DEFPARAM (PARAM_VECT_SHORT_LOOPS,
> + "vect-short-loops",
> + "Enable vectorization of low trip count loops using masking.",
> + 0, 0, 1)
> +
> /*
>
> Local variables:
Re: [PATCH, vec-tails 04/10] Add masking cost
Ping
2016-06-22 17:13 GMT+03:00 Ilya Enkovich :
> On 16 Jun 00:16, Jeff Law wrote:
>> On 05/19/2016 01:40 PM, Ilya Enkovich wrote:
>> >Hi,
>> >
>> >This patch extends vectorizer cost model to include masking cost by
>> >adding new cost model locations and new target hook to compute
>> >masking cost.
>> >
>> >Thanks,
>> >Ilya
>> >--
>> >gcc/
>> >
>> >2016-05-19 Ilya Enkovich
>> >
>> > * config/i386/i386.c (ix86_init_cost): Extend costs array.
>> > (ix86_add_stmt_masking_cost): New.
>> > (ix86_finish_cost): Add masking_prologue_cost and masking_body_cost
>> > args.
>> > (TARGET_VECTORIZE_ADD_STMT_MASKING_COST): New.
>> > * config/i386/i386.h (TARGET_INCREASE_MASK_STORE_COST): New.
>> > * config/i386/x86-tune.def (X86_TUNE_INCREASE_MASK_STORE_COST): New.
>> > * config/rs6000/rs6000.c (_rs6000_cost_data): Extend cost array.
>> > (rs6000_init_cost): Initialize new cost elements.
>> > (rs6000_finish_cost): Add masking_prologue_cost and masking_body_cost.
>> > * config/spu/spu.c (spu_init_cost): Extend costs array.
>> > (spu_finish_cost): Add masking_prologue_cost and masking_body_cost
>> > args.
>> > * doc/tm.texi.in (TARGET_VECTORIZE_ADD_STMT_MASKING_COST): New.
>> > * doc/tm.texi: Regenerated.
>> > * target.def (add_stmt_masking_cost): New.
>> > (finish_cost): Add masking_prologue_cost and masking_body_cost args.
>> > * target.h (enum vect_cost_for_stmt): Add vector_mask_load and
>> > vector_mask_store.
>> > (enum vect_cost_model_location): Add vect_masking_prologue
>> > and vect_masking_body.
>> > * targhooks.c (default_builtin_vectorization_cost): Support
>> > vector_mask_load and vector_mask_store.
>> > (default_init_cost): Extend costs array.
>> > (default_add_stmt_masking_cost): New.
>> > (default_finish_cost): Add masking_prologue_cost and masking_body_cost
>> > args.
>> > * targhooks.h (default_add_stmt_masking_cost): New.
>> > * tree-vect-loop.c (vect_estimate_min_profitable_iters): Adjust
>> > finish_cost call.
>> > * tree-vect-slp.c (vect_bb_vectorization_profitable_p): Likewise.
>> > * tree-vectorizer.h (add_stmt_masking_cost): New.
>> > (finish_cost): Add masking_prologue_cost and masking_body_cost args.
>> >
>> >
>> >diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
>> >index 9f62089..6c2c364 100644
>> >--- a/gcc/config/i386/i386.c
>> >+++ b/gcc/config/i386/i386.c
>> >@@ -53932,8 +53932,12 @@ ix86_spill_class (reg_class_t rclass, machine_mode
>> >mode)
>> > static void *
>> > ix86_init_cost (struct loop *)
>> > {
>> >- unsigned *cost = XNEWVEC (unsigned, 3);
>> >- cost[vect_prologue] = cost[vect_body] = cost[vect_epilogue] = 0;
>> >+ unsigned *cost = XNEWVEC (unsigned, 5);
>> >+ cost[vect_prologue] = 0;
>> >+ cost[vect_body] = 0;
>> >+ cost[vect_epilogue] = 0;
>> >+ cost[vect_masking_prologue] = 0;
>> >+ cost[vect_masking_body] = 0;
>> > return cost;
>> Trivial nit -- no need or desire to use whitespace to line up the
>> initializers. It looks like others may have done this in the duplicated
>> instances of finish_cost. But we shouldn't propagate that mistake into the
>> init_cost hooks ;-)
>>
>>
>> @@ -1115,8 +1117,12 @@ default_get_mask_mode (unsigned nunits, unsigned
>> vector_size)
>> > void *
>> > default_init_cost (struct loop *loop_info ATTRIBUTE_UNUSED)
>> > {
>> >- unsigned *cost = XNEWVEC (unsigned, 3);
>> >- cost[vect_prologue] = cost[vect_body] = cost[vect_epilogue] = 0;
>> >+ unsigned *cost = XNEWVEC (unsigned, 5);
>> >+ cost[vect_prologue] = 0;
>> >+ cost[vect_body] = 0;
>> >+ cost[vect_epilogue] = 0;
>> >+ cost[vect_masking_prologue] = 0;
>> >+ cost[vect_masking_body] = 0;
>> > return cost;
>> Here too. There's others. I won't point them all out. Please double check
>> for this nit in any added code. You don't have to go back and fix existing
>> problems of this nature.
>>
>> I don't see anything here I really object to -- Richi and I may disagree on
>> the compute-costs once in a single scan vs restarting the scan. If Richi
>> feels strongly about restarting for some reason, I'll defer to him -- he's
>> done more work in the vectorizer than myself.
>>
>> I'd suggest taking another stab at the docs for the hooks based on Richi's
>> question about whether or not the hook returns the cost of hte masked
>> statement or the cost of masking the statement.
>>
>> jeff
>
> Thanks for review. Here is an updated version with initializers and
> documentation fixed.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2016-05-22 Ilya Enkovich
>
> * config/i386/i386.c (ix86_init_cost): Extend costs array.
> (ix86_add_stmt_masking_cost): New.
> (ix86_finish_cost): Add masking_prologue_cost and masking_body_cost
> args.
> (TARGET_VECTORIZE_ADD_STMT_MASKING_COST): New.
> * config/i386/i386.h (TARGET_INCREASE_MASK_STORE_COST): New.
> * config/i386/x86-tune.def (X86_TUNE_INCREASE_MA
Re: [PATCH, vec-tails 03/10] Support epilogues vectorization with no masking
Ping
2016-06-24 10:40 GMT+03:00 Ilya Enkovich :
> On 17 Jun 10:46, Jeff Law wrote:
>> On 06/17/2016 08:33 AM, Ilya Enkovich wrote:
>> >>
>> >>Hmm, there seems to be a level of indirection I'm missing here. We're
>> >>smuggling LOOP_VINFO_ORIG_LOOP_INFO around in loop->aux. Ewww. I thought
>> >>the whole point of LOOP_VINFO_ORIG_LOOP_INFO was to smuggle the VINFO from
>> >>the original loop to the vectorized epilogue. What am I missing? Rather
>> >>than smuggling around in the aux field, is there some inherent reason why
>> >>we
>> >>can't just copy the info from the original loop directly into
>> >>LOOP_VINFO_ORIG_LOOP_INFO for the vectorized epilogue?
>> >
>> >LOOP_VINFO_ORIG_LOOP_INFO is used for several things:
>> > - mark this loop as epilogue
>> > - get VF of original loop (required for both mask and nomask modes)
>> > - get decision about epilogue masking
>> >
>> >That's all. When epilogue is created it has no LOOP_VINFO. Also when we
>> >vectorize loop we create and destroy its LOOP_VINFO multiple times. When
>> >loop has LOOP_VINFO loop->aux points to it and original LOOP_VINFO is in
>> >LOOP_VINFO_ORIG_LOOP_INFO. When Loop has no LOOP_VINFO associated I have no
>> >place to bind it with the original loop and therefore I use vacant loop->aux
>> >for that. Any other way to bind epilogue with its original loop would work
>> >as well. I just chose loop->aux to avoid new fields and data structures.
>> I was starting to draw the conclusion that the smuggling in the aux field
>> was for cases when there was no LOOP_VINFO. But was rather late at night
>> and I didn't follow that idea through the code. THanks for clarifying.
>>
>>
>> >>
>> >>And something just occurred to me -- is there some inherent reason why SLP
>> >>doesn't vectorize the epilogue, particularly for the cases where we can
>> >>vectorize the epilogue using smaller vectors? Sorry if you've already
>> >>answered this somewhere or it's a dumb question.
>> >
>> >IIUC this may happen only if we unroll epilogue into a single BB which
>> >happens
>> >only when epilogue iterations count is known. Right?
>> Probably. The need to make sure the epilogue is unrolled probably makes
>> this a non-starter.
>>
>> I have a soft spot for SLP as I stumbled on the idea while rewriting a
>> presentation in the wee hours of the morning for the next day. Essentially
>> it was a "poor man's" vectorizer that could be done for dramatically less
>> engineering cost than a traditional vectorizer. The MIT paper outlining the
>> same ideas came out a couple years later...
>>
>>
>> >>+ /* Add new loop to a processing queue. To make it easier
>> >>>+ to match loop and its epilogue vectorization in dumps
>> >>>+ put new loop as the next loop to process. */
>> >>>+ if (new_loop)
>> >>>+ {
>> >>>+ loops.safe_insert (i + 1, new_loop->num);
>> >>>+ vect_loops_num = number_of_loops (cfun);
>> >>>+ }
>> >>>+
>> >>
>> >>So just to be clear, the only reason to do this is for dumps -- other than
>> >>processing the loop before it's epilogue, there's no other inherently
>> >>necessary ordering of the loops, right?
>> >
>> >Right, I don't see other reasons to do it.
>> Perfect. Thanks for confirming.
>>
>> jeff
>>
>
> Hi,
>
> Here is an updated version with disabled alias checks for loop epilogues.
> Instead of calling vect_analyze_data_ref_dependence I just use VF of the
> original loop as MAX_VF for epilogue.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2016-05-24 Ilya Enkovich
>
> * tree-if-conv.c (tree_if_conversion): Make public.
> * tree-if-conv.h: New file.
> * tree-vect-data-refs.c (vect_analyze_data_ref_dependences) Avoid
> dynamic alias checks for epilogues.
> (vect_enhance_data_refs_alignment): Don't try to enhance alignment
> for epilogues.
> * tree-vect-loop-manip.c (vect_do_peeling_for_loop_bound): Return
> created loop.
> * tree-vect-loop.c: include tree-if-conv.h.
> (destroy_loop_vec_info): Preserve LOOP_VINFO_ORIG_LOOP_INFO in
> loop->aux.
> (vect_analyze_loop_form): Init LOOP_VINFO_ORIG_LOOP_INFO and reset
> loop->aux.
> (vect_analyze_loop): Reset loop->aux.
> (vect_transform_loop): Check if created epilogue should be returned
> for further vectorization. If-convert epilogue if required.
> * tree-vectorizer.c (vectorize_loops): Add a queue of loops to
> process and insert vectorized loop epilogues into this queue.
> * tree-vectorizer.h (vect_do_peeling_for_loop_bound): Return created
> loop.
> (vect_transform_loop): Return created loop.
>
>
> diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c
> index 5914a78..b790ca9 100644
> --- a/gcc/tree-if-conv.c
> +++ b/gcc/tree-if-conv.c
> @@ -2808,7 +2808,7 @@ ifcvt_local_dce (basic_block bb)
> profitability analysis. Returns non-zero todo flags when something
> chang
Re: [PATCH, vec-tails 08/10] Support loop epilogue masking and low trip count loop vectorization
Ping
2016-06-24 14:46 GMT+03:00 Ilya Enkovich :
> On 16 Jun 18:52, Ilya Enkovich wrote:
>> 2016-06-15 15:00 GMT+03:00 Richard Biener :
>> > On Thu, May 19, 2016 at 9:46 PM, Ilya Enkovich
>> > wrote:
>> >> Hi,
>> >>
>> >> This patch enables vectorization of loop epilogues and low trip count
>> >> loops using masking.
>> >
>> > I wonder why we have the epilogue masking restriction with respect to
>> > the original vectorization factor - shouldn't this simply be handled by
>> > vectorizing the epilogue? First trying the original VF (requires masking
>> > and is equivalent to low-tripcount loop vectorization), then if that is not
>> > profitable iterate to smaller VFs? [yes, ideally we'd be able to compare
>> > cost for vectorization with different VFs and choose the best VF]
>>
>> When main loop is vectorized using some VF we compute epilogue masking
>> profitability and generate epilogue to be vectorized and masked using exactly
>> the same VF. In ideal case we never fail to vectorize epilogue because we
>> check that it can be masked. Unfortunately we may loose some info
>> when generating
>> a loop copy (e.g. scev info is lost) and therefore may fail to
>> vectorize epilogue.
>>
>> I expect that if we loose some info and thus fail to vectorize for a
>> specified VF
>> (for which the main loop was successfully vectorized) then we are going to
>> fail
>> to vectorize for other vector sizes too. Actually I'd prefer to try
>> the only vector
>> size for vectorization with masking to save compilation time.
>>
>> Thanks,
>> Ilya
>>
>> >
>> > Thanks,
>> > Richard.
>> >
>> >> Thanks,
>> >> Ilya
>
> Hi,
>
> Here is an updated version. It allows vectorization with a smaller vector
> size
> in case we fail to vectorize with masking.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2016-05-24 Ilya Enkovich
>
> * dbgcnt.def (vect_tail_mask): New.
> * tree-vect-loop.c (vect_analyze_loop_2): Support masked loop
> epilogues and low trip count loops.
> (vect_get_known_peeling_cost): Ignore scalar epilogue cost for
> loops we are going to mask.
> (vect_estimate_min_profitable_iters): Support masked loop
> epilogues and low trip count loops.
> * tree-vectorizer.c (vectorize_loops): Add a message for a case
> when loop epilogue can't be vectorized.
>
>
> diff --git a/gcc/dbgcnt.def b/gcc/dbgcnt.def
> index 73c2966..5aad1d7 100644
> --- a/gcc/dbgcnt.def
> +++ b/gcc/dbgcnt.def
> @@ -193,4 +193,5 @@ DEBUG_COUNTER (tree_sra)
> DEBUG_COUNTER (vect_loop)
> DEBUG_COUNTER (vect_slp)
> DEBUG_COUNTER (vect_tail_combine)
> +DEBUG_COUNTER (vect_tail_mask)
> DEBUG_COUNTER (dom_unreachable_edges)
> diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
> index 95dfda9..78a6754 100644
> --- a/gcc/tree-vect-loop.c
> +++ b/gcc/tree-vect-loop.c
> @@ -2205,7 +2205,7 @@ vect_analyze_loop_2 (loop_vec_info loop_vinfo, bool
> &fatal)
>int saved_vectorization_factor = LOOP_VINFO_VECT_FACTOR (loop_vinfo);
>HOST_WIDE_INT estimated_niter;
>unsigned th;
> - int min_scalar_loop_bound;
> + int min_scalar_loop_bound = 0;
>
>/* Check the SLP opportunities in the loop, analyze and build SLP trees.
> */
>ok = vect_analyze_slp (loop_vinfo, n_stmts);
> @@ -2230,6 +2230,45 @@ start_over:
>unsigned vectorization_factor = LOOP_VINFO_VECT_FACTOR (loop_vinfo);
>gcc_assert (vectorization_factor != 0);
>
> + /* For now we mask loop epilogue using the same VF since it was used
> + for cost estimations and it should be easier for reduction
> + optimization. */
> + if (LOOP_VINFO_EPILOGUE_P (loop_vinfo)
> + && LOOP_VINFO_ORIG_MASK_EPILOGUE (loop_vinfo)
> + && LOOP_VINFO_ORIG_VECT_FACTOR (loop_vinfo) !=
> (int)vectorization_factor)
> +{
> + /* If we couldn't vectorize epilogue with masking then we may still
> +try to vectorize it with a smaller vector size. */
> + if (LOOP_VINFO_ORIG_VECT_FACTOR (loop_vinfo) >
> (int)vectorization_factor
> + && flag_tree_vectorize_epilogues & VECT_EPILOGUE_NOMASK)
> + {
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> +"couldn't mask loop epilogue; trying to
> vectorize "
> +"using a smaller vector.\n");
> + LOOP_VINFO_ORIG_MASK_EPILOGUE (loop_vinfo) = false;
> + LOOP_VINFO_NEED_MASKING (loop_vinfo) = false;
> + }
> + else
> + {
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> +"not vectorized: VF for loop epilogue doesn't "
> +"match original loop VF.\n");
> + return false;
> + }
> +}
> +
> + if (LOOP_VINFO_EPILOGUE_P (loop_vinfo)
> + && !LOOP_VINFO_ORIG_MASK_EPILOGUE (loop_vinfo)
> + && LOOP_VINFO_ORIG_VECT_FACTOR (loop_vinfo) <=
> (int)vectorization_factor)
> +{
> + if
Re: [PATCH, vec-tails 05/10] Check if loop can be masked
Ping
2016-06-23 12:54 GMT+03:00 Ilya Enkovich :
> On 22 Jun 11:42, Jeff Law wrote:
>> On 06/22/2016 10:09 AM, Ilya Enkovich wrote:
>>
>> >>Given the common structure & duplication I can't help but wonder if a
>> >>single
>> >>function should be used for widening/narrowing. Ultimately can't you swap
>> >>mask_elems/req_elems and always go narrower to wider (using a different
>> >>optab for the two different cases)?
>> >
>> >I think we can't always go in narrower to wider direction because widening
>> >uses two optabs wand also because the way insn_data is checked.
>> OK. Thanks for considering.
>>
>> >>
>> >>I'm guessing Richi's comment about what tree type you're looking at refers
>> >>to this and similar instances. Doesn't this give you the type of the
>> >>number
>> >>of iterations rather than the type of the iteration variable itself?
>> >>
>> >>
>> >
>> >Since I build vector IV by myself and use to compare with NITERS I
>> >feel it's safe to
>> >use type of NITERS. Do you expect NITERS and IV types differ?
>> Since you're comparing to NITERS, it sounds like you've got it right and
>> that Richi and I have it wrong.
>>
>> It's less a question of whether or not we expect NITERS and IV to have
>> different types, but more a realization that there's nothing that inherently
>> says they have to be the same. THey probably are the same most of the time,
>> but I don't think that's something we can or should necessarily depend on.
>>
>>
>>
>> >>>@@ -1791,6 +1870,20 @@ vectorizable_mask_load_store (gimple *stmt,
>> >>>gimple_stmt_iterator *gsi,
>> >>> && !useless_type_conversion_p (vectype, rhs_vectype)))
>> >>> return false;
>> >>>
>> >>>+ if (LOOP_VINFO_CAN_BE_MASKED (loop_vinfo))
>> >>>+{
>> >>>+ /* Check that mask conjuction is supported. */
>> >>>+ optab tab;
>> >>>+ tab = optab_for_tree_code (BIT_AND_EXPR, vectype, optab_default);
>> >>>+ if (!tab || optab_handler (tab, TYPE_MODE (vectype)) ==
>> >>>CODE_FOR_nothing)
>> >>>+ {
>> >>>+ if (dump_enabled_p ())
>> >>>+ dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>> >>>+"cannot be masked: unsupported mask
>> >>>operation\n");
>> >>>+ LOOP_VINFO_CAN_BE_MASKED (loop_vinfo) = false;
>> >>>+ }
>> >>>+}
>> >>
>> >>Should the optab querying be in optab-query.c?
>> >
>> >We always directly call optab_handler for simple operations. There are
>> >dozens
>> >of such calls in vectorizer.
>> OK. I would look favorably on a change to move those queries out into
>> optabs-query as a separate patch.
>>
>> >
>> >We don't embed masking capabilities into vectorizer.
>> >
>> >Actually we don't depend on masking capabilities so much. We have to mask
>> >loads and stores and use can_mask_load_store for that which uses existing
>> >optab
>> >query. We also require masking for reductions and use VEC_COND for that
>> >(and use existing expand_vec_cond_expr_p). Other checks are to check if we
>> >can build required masks. So we actually don't expose any new processor
>> >masking capabilities to GIMPLE. I.e. all this works on targets with no
>> >rich masking capabilities. E.g. we can mask loops for quite old SSE
>> >targets.
>> OK. I think the key here is that load/store masking already exists and the
>> others are either VEC_COND or checking if we can build the mask rather than
>> can the operation be masked. THanks for clarifying.
>> jeff
>
> Here is an updated version with less typos and more comments.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2016-05-23 Ilya Enkovich
>
> * tree-vect-loop.c: Include insn-config.h and recog.h.
> (vect_check_required_masks_widening): New.
> (vect_check_required_masks_narrowing): New.
> (vect_get_masking_iv_elems): New.
> (vect_get_masking_iv_type): New.
> (vect_get_extreme_masks): New.
> (vect_check_required_masks): New.
> (vect_analyze_loop_operations): Add vect_check_required_masks
> call to compute LOOP_VINFO_CAN_BE_MASKED.
> (vect_analyze_loop_2): Initialize LOOP_VINFO_CAN_BE_MASKED and
> LOOP_VINFO_NEED_MASKING before starting over.
> (vectorizable_reduction): Compute LOOP_VINFO_CAN_BE_MASKED and
> masking cost.
> * tree-vect-stmts.c (can_mask_load_store): New.
> (vect_model_load_masking_cost): New.
> (vect_model_store_masking_cost): New.
> (vect_model_simple_masking_cost): New.
> (vectorizable_mask_load_store): Compute LOOP_VINFO_CAN_BE_MASKED
> and masking cost.
> (vectorizable_simd_clone_call): Likewise.
> (vectorizable_store): Likewise.
> (vectorizable_load): Likewise.
> (vect_stmt_should_be_masked_for_epilogue): New.
> (vect_add_required_mask_for_stmt): New.
> (vect_analyze_stmt): Compute LOOP_VINFO_CAN_BE_MASKED.
> * tree-vectorizer.h (vect_model_load_masking_cost): New.
> (vect_model_st
Re: [PATCH, vec-tails 07/10] Support loop epilogue combining
Ping 2016-06-28 15:24 GMT+03:00 Ilya Enkovich : > On 16 Jun 10:54, Jeff Law wrote: >> On 05/19/2016 01:44 PM, Ilya Enkovich wrote: >> >Hi, >> > >> >This patch introduces support for loop epilogue combining. This includes >> >support in cost estimation and all required changes required to mask >> >vectorized loop. >> > >> >Thanks, >> >Ilya >> >-- >> >gcc/ >> > >> >2016-05-19 Ilya Enkovich >> > >> > * dbgcnt.def (vect_tail_combine): New. >> > * params.def (PARAM_VECT_COST_INCREASE_COMBINE_THRESHOLD): New. >> > * tree-vect-data-refs.c (vect_get_new_ssa_name): Support vect_mask_var. >> > * tree-vect-loop-manip.c (slpeel_tree_peel_loop_to_edge): Support >> > epilogue combined with loop body. >> > (vect_do_peeling_for_loop_bound): Likewise. >> > * tree-vect-loop.c Include alias.h and dbgcnt.h. >> > (vect_estimate_min_profitable_iters): Add >> > ret_min_profitable_combine_niters >> > arg, compute number of iterations for which loop epilogue combining is >> > profitable. >> > (vect_generate_tmps_on_preheader): Support combined apilogue. >> > (vect_gen_ivs_for_masking): New. >> > (vect_get_mask_index_for_elems): New. >> > (vect_get_mask_index_for_type): New. >> > (vect_gen_loop_masks): New. >> > (vect_mask_reduction_stmt): New. >> > (vect_mask_mask_load_store_stmt): New. >> > (vect_mask_load_store_stmt): New. >> > (vect_combine_loop_epilogue): New. >> > (vect_transform_loop): Support combined apilogue. >> > >> > >> >diff --git a/gcc/tree-vect-loop-manip.c b/gcc/tree-vect-loop-manip.c >> >index fab5879..b3c0668 100644 >> >--- a/gcc/tree-vect-loop-manip.c >> >+++ b/gcc/tree-vect-loop-manip.c >> >@@ -1464,11 +1469,20 @@ slpeel_tree_peel_loop_to_edge (struct loop *loop, >> >struct loop *scalar_loop, >> > bb_between_loops = new_exit_bb; >> > bb_after_second_loop = split_edge (single_exit (second_loop)); >> > >> >- pre_condition = >> >-fold_build2 (EQ_EXPR, boolean_type_node, *first_niters, niters); >> >- skip_e = slpeel_add_loop_guard (bb_between_loops, pre_condition, NULL, >> >- bb_after_second_loop, >> >bb_before_first_loop, >> >- inverse_probability >> >(second_guard_probability)); >> >+ if (skip_second_after_first) >> >+/* We can just redirect edge from bb_between_loops to >> >+ bb_after_second_loop but we have many code assuming >> >+ we have a guard after the first loop. So just make >> >+ always taken condtion. */ >> >+pre_condition = fold_build2 (EQ_EXPR, boolean_type_node, >> >integer_zero_node, >> >+ integer_zero_node); >> This isn't ideal, but I don't think it's that big of an issue. >> >> >@@ -1758,8 +1772,10 @@ vect_do_peeling_for_loop_bound (loop_vec_info >> >loop_vinfo, >> > basic_block preheader; >> > int loop_num; >> > int max_iter; >> >+ int bound2; >> > tree cond_expr = NULL_TREE; >> > gimple_seq cond_expr_stmt_list = NULL; >> >+ bool combine = LOOP_VINFO_COMBINE_EPILOGUE (loop_vinfo); >> > >> > if (dump_enabled_p ()) >> > dump_printf_loc (MSG_NOTE, vect_location, >> >@@ -1769,12 +1785,13 @@ vect_do_peeling_for_loop_bound (loop_vec_info >> >loop_vinfo, >> > >> > loop_num = loop->num; >> > >> >+ bound2 = combine ? th : LOOP_VINFO_VECT_FACTOR (loop_vinfo); >> Can you document what the TH parameter is to the various routines that use >> it in tree-vect-loop-manip.c? I realize you didn't add it, but it would >> help anyone looking at this code in the future to know it's the threshold of >> iterations for vectorization without having to find it in other function >> comment headers ;-) >> >> That's pre-approved to go in immediately :-) >> >> >@@ -1803,7 +1820,11 @@ vect_do_peeling_for_loop_bound (loop_vec_info >> >loop_vinfo, >> > max_iter = (LOOP_VINFO_PEELING_FOR_GAPS (loop_vinfo) >> > ? LOOP_VINFO_VECT_FACTOR (loop_vinfo) * 2 >> > : LOOP_VINFO_VECT_FACTOR (loop_vinfo)) - 2; >> >- if (check_profitability) >> >+ /* When epilogue is combined only profitability >> >+ treshold matters. */ >> s/treshold/threshold/ >> >> >> >> > static void >> > vect_estimate_min_profitable_iters (loop_vec_info loop_vinfo, >> > int *ret_min_profitable_niters, >> >-int *ret_min_profitable_estimate) >> >+int *ret_min_profitable_estimate, >> >+int *ret_min_profitable_combine_niters) >> I'm torn a bit here. There's all kinds of things missing/incomplete in the >> function comments throughout the vectorizer. And in some cases, like this >> one, the parameters are largely self-documenting. But we've also got coding >> standards that we'd like to adhere to. >> >> I don't think it's fair to require you to fix all these issues in the >> vectorizer (though if you wanted to, I'd fully support those an independent >> cleanups). >> >> Perhaps
[PATCH] PR target/71801: Don't convert TImode in debug insn
When converting V1TImode register in debug insn, check if it has been
converted to TImode already.
Tested on x86-64. OK for trunk?
H.J.
---
gcc/
PR target/71801
* config/i386/i386.c (timode_scalar_chain::fix_debug_reg_uses):
Don't convert TImode in debug insn.
gcc/testsuite/
PR target/71801
* gcc.target/i386/pr71801.c: New test.
---
gcc/config/i386/i386.c | 3 +++
gcc/testsuite/gcc.target/i386/pr71801.c | 22 ++
2 files changed, 25 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/i386/pr71801.c
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 9eaf414..d190bef 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -3814,6 +3814,9 @@ timode_scalar_chain::fix_debug_reg_uses (rtx reg)
continue;
gcc_assert (GET_CODE (val) == VAR_LOCATION);
rtx loc = PAT_VAR_LOCATION_LOC (val);
+ /* It may have been converted to TImode already. */
+ if (GET_MODE (loc) == TImode)
+ continue;
gcc_assert (REG_P (loc)
&& GET_MODE (loc) == V1TImode);
/* Convert V1TImode register, which has been updated by a SET
diff --git a/gcc/testsuite/gcc.target/i386/pr71801.c
b/gcc/testsuite/gcc.target/i386/pr71801.c
new file mode 100644
index 000..6c87522
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr71801.c
@@ -0,0 +1,22 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -g" } */
+
+struct {
+ char uuid[16];
+} c;
+struct {
+ int s_uuid[6];
+} a, b;
+int bar (void);
+static int get_label_uuid(char *p1) {
+ __builtin_memcpy(p1, a.s_uuid, sizeof(a));
+ if (bar())
+__builtin_memcpy(p1, b.s_uuid, sizeof(b));
+ return 0;
+}
+void uuidcache_addentry(char *p1) { __builtin_memcpy(&c, p1, sizeof(c)); }
+void uuidcache_init() {
+ char d[1];
+ get_label_uuid(d);
+ uuidcache_addentry(d);
+}
--
Re: [PATCH] Support running the selftests under valgrind
On 07/08/2016 01:46 PM, David Malcolm wrote: This patch adds a new phony target to gcc/Makefile.in to make it easy to run the selftests under valgrind, via "make selftest-valgrind". This phony target isn't a dependency of anything; it's purely for convenience (it takes about 4-5 seconds on my box). Doing so uncovered a few leaks in the selftest suite, which the patch also fixes, so that it runs cleanly under valgrind (on x86_64-pc-linux-gnu, configured with --enable-valgrind-annotations, at least). Successfully bootstrapped®rtested on x86_64-pc-linux-gnu. Manually verified that the valgrind output is "clean" on x86_64-pc-linux-gnu [1]. OK for trunk? [1]: HEAP SUMMARY: in use at exit: 1,203,983 bytes in 2,114 blocks total heap usage: 4,545 allocs, 2,431 frees, 3,212,841 bytes allocated LEAK SUMMARY: definitely lost: 0 bytes in 0 blocks indirectly lost: 0 bytes in 0 blocks possibly lost: 0 bytes in 0 blocks still reachable: 1,203,983 bytes in 2,114 blocks suppressed: 0 bytes in 0 blocks Reachable blocks (those to which a pointer was found) are not shown. To see them, rerun with: --leak-check=full --show-leak-kinds=all For counts of detected and suppressed errors, rerun with: -v ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2) gcc/ChangeLog: * Makefile.in (selftest-valgrind): New phony target. * function-tests.c (selftest::build_cfg): Delete pass instances created by the test. (selftest::convert_to_ssa): Likewise. (selftest::test_expansion_to_rtl): Likewise. * tree-cfg.c (selftest::test_linear_chain): Release dominator vectors. (selftest::test_diamond): Likewise. OK. jeff
Re: RFA: new pass to warn on questionable uses of alloca() and VLAs
On 11 July 2016 at 11:10, Aldy Hernandez wrote: > On 07/10/2016 07:41 PM, Manuel López-Ibáñez wrote: >>> >>> +Walloca >>> +LangEnabledBy(C ObjC C++ ObjC++) >>> +; in common.opt >>> + >>> +Walloca= >>> +LangEnabledBy(C ObjC C++ ObjC++) >>> +; in common.opt >>> + >> >> >> I'm not sure what you think the above means, but this is an invalid use >> of LangEnabledBy(). (It would be nice if the .awk scripts were able to >> catch it and give an error.) > > > I was following the practice for -Warray-bounds in c-family/c.opt: > > Warray-bounds > LangEnabledBy(C ObjC C++ ObjC++,Wall) > ; in common.opt > > Warray-bounds= > LangEnabledBy(C ObjC C++ ObjC++,Wall,1,0) > ; in common.opt The ones you quoted mean: For that list of languages, -Warray-bounds is enabled by -Wall. https://gcc.gnu.org/onlinedocs/gccint/Option-properties.html#Option-properties But the entries that you added do not specify an option. It should give an error by the *.awk scripts that parse .opt files. I'm actually not sure what the scripts are generating in your case. > I *thought* you defined the option in common.opt, and narrowed the language > variants in c-family/c.opt. ?? No, options that are language specific just need to be defined for the respective FEs using the respective language flags: Wvla is an example of that. It doesn't appear in common.opt. Adding language flags to a common option is just redundant (again, this is not what LangEnabledBy is doing). Cheers, Manuel.
Re: RFA: new pass to warn on questionable uses of alloca() and VLAs
On 07/08/2016 05:48 AM, Aldy Hernandez wrote:
[New thread now that I actually have a tested patch :)].
I think detecting potentially problematic uses of alloca would
be useful, especially when done in an intelligent way like in
your patch (as opposed to simply diagnosing every call to
the function regardless of the value of its argument). At
the same time, it seems that an even more reliable solution
than pointing out potentially unsafe calls to the function
and relying on users to modify their code to use malloc for
large/unbounded allocations would be to let GCC do it for
them automatically (i.e., in response to some other option,
emit a call to malloc instead, and insert a call to free when
appropriate).
As Jeff said, we were thinking the other way around: notice a malloced
area that doesn't escape and replace it with a call to alloca. But all
this is beyond the scope of this patch.
Definitely out of scope for this set of changes.BUt it's part of my
evil plan to squash explicit alloca calls while still allowing its use
when it's provably safe. But we're not in that world yet.
I found the "warning: unbounded use of alloca" misleading when
a call to the function was, in fact, bounded but to a limit
that's greater than alloca-max-size as in the program below:
I have added various levels of granularity for the warning, along with
appropriately different messages:
// Possible problematic uses of alloca.
enum alloca_type {
// Alloca argument is within known bounds that are appropriate.
ALLOCA_OK,
// Alloca argument is KNOWN to have a value that is too large.
ALLOCA_BOUND_DEFINITELY_LARGE,
// Alloca argument may be too large.
ALLOCA_BOUND_MAYBE_LARGE,
// Alloca argument is bounded but of an indeterminate size.
ALLOCA_BOUND_UNKNOWN,
// Alloca argument was casted from a signed integer.
ALLOCA_CAST_FROM_SIGNED,
// Alloca appears in a loop.
ALLOCA_IN_LOOP,
// Alloca call is unbounded. That is, there is no controlling
// predicate for its argument.
ALLOCA_UNBOUNDED
};
Of course, there are plenty of cases where we can't get the exact
diagnosis (due to the limitations on our range info) and we fall back to
ALLOCA_UNBOUNDED or ALLOCA_BOUND_MAYBE_LARGE. In practice, I'm
wondering whether we should lump everything into 2-3 warnings instead of
trying so hard to get the exact reason for the problematic use of
alloca. (More details on upcoming changes to range info further down.)
I'll trust your judgment on this -- I kind of like the more precise
warnings -- if for no other reason than it makes it easier for someone
looking at legacy code (think glibc) to see why GCC determined their
alloca call was unsafe.
void f (void*);
void g (int n)
{
void *p;
if (n < 4096)
p = __builtin_alloca (n);
else
p = __builtin_malloc (n);
f (p);
}
t.C: In function ‘g’:
t.C:7:7: warning: unbounded use of alloca [-Walloca]
p = __builtin_alloca (n);
Well, in this particular case you are using a signed int, so n < 4096
can cause the value passed to alloca to be rather large in the case of
n < 0.
Right. And if the bad guys can get control of "N" in this case, they
can pass in a negative value and perform a "stack shift" -- essentially
moving the stack so that it points somewhere else more "interesting" in
memory.
This patch depends on range information, which is less than stellar past
the VRP pass. To get better range information, we'd have to incorporate
this somehow into the VRP pass and use the ASSERT_EXPRs therein. And
still, VRP gets awfully confused with PHI merge points. Andrew Macleod
is working on a new fancy range info infrastructure that should
eliminate a lot of the false positives we may get and/or help us
diagnose a wider range of cases. Hence the reason that I'm not bending
over backwards to incorporate this pass into tree-vrp.c.
Right. Getting better range information is important in so many ways.
FYI, I have a few XFAILed tests in this patch, to make sure we don't
lose track of possible improvements (which in this case should be
handled by better range info). What's the blessed way of doing this?
Are adding new XFAILed tests acceptable?
I'd go with a new XFAIL'd test.
Jeff
Re: RFA: new pass to warn on questionable uses of alloca() and VLAs
On 07/10/2016 04:09 PM, Martin Sebor wrote: On 07/08/2016 05:48 AM, Aldy Hernandez wrote: I've played with the patch a bit over the weekend and have a few comments and suggestions (I hope you won't regret encouraging me :) I like the consistency between -Walloca and -Wvla! (And despite the volume of my remaining comments, the rest of the patch too! 1) Optimization. Without -O2 GCC prints: sorry, unimplemented: -Walloca ignored without -O2 It seems that a warning would be more appropriate here than a hard error, but the checker could, and I would say should, be made available (in a limited form) without optimization because -Walloca with no argument doesn't rely on it. I suspect in this form, -Walloca is probably mainly going to be useful as a mechanism to enforce a "thou shall not use alloca" policy, though not much more beyond that. :-) Which would be fine with me -- the difference is, we'd be able to back up my "programmers can't correctly use alloca" rant from several years ago with compiler analysis showing why each particular alloca was unsafe. 2) When passed an argument of a signed type, GCC prints warning: cast from signed type in alloca even though there is no explicit cast in the code. It may not be obvious why the conversion is a problem in this context. I would suggest to rephrase the warning along the lines of -Wsign-conversion which prints: conversion to ‘long unsigned int’ from ‘int’ may change the sign of the result and add why it's a potential problem. Perhaps something like: argument to alloca may be too large due to conversion from 'int to 'long unsigned int' I like Martin's much better. 3) I wonder if the warning should also detect alloca calls with a zero argument and VLAs of zero size. They are both likely to be programmer errors. (Although it seems that that would need to be done earlier than in the alloca pass.) Seems like Aldy ought to add this as a testcase, even if it's XFAIL'd for now. 4) I wasn't able to trigger the -Wvla=N warning with VLAs used in loops even though VRP provides the range of the value: Similarly for the others in your message. Jeff
Re: [RFC] Convert TYPE_ALIGN_OK into an TYPE_LANG_FLAG
On 07/10/2016 09:32 AM, Bernd Edlinger wrote: On 07/10/16 10:23, Eric Botcazou wrote: I expect it still has an effect due to the same reason as the DECL_OFFSET_ALIGN thing - memory reference expansion doesn't gather information from the full reference but tries to re-cover it from the pieces returned from get_inner_reference. So I guess we need to fix that first. The situation is a bit different though, because the main (and single?) case for which TYPE_ALIGN_OK had been necessary in the middle-end was made obsolete by a gigi change at some point. And indeed extensive testing of Bernd's patch on a strict-alignment platform didn't reveal any issue. So no objection to the patch on principle by me. Thank you, Eric. So following Jeff's suggestion, here is an updated patch that allocates a new lang_flag_7 out of the spare bits in tree_type_common. The only difference to the previous version is in tree-core.h where tree_type_common is declared, and print-tree.c where the new flag is printed. Boot-strapped and reg-tested on armv7-linux-gneabihf. OK for trunk? OK. jeff
Re: [PATCH] Introduce new param: AVG_LOOP_NITER
On 07/11/2016 01:55 AM, Martin Liška wrote: Hello. During investigation of an IVOPTs issue, I noticed that tree-ssa-loop-ivopts.c uses a hard-coded constant AVG_LOOP_NITER. Very similar constant can be seen in cfgloopanal.c. Thus, I've changed that to a new param. Apart from that, I would like to change the default value to 10. As numbers of SPEC CPU2006 show: Loop count: 18236 avg. # of iter: 11908.46 median # of iter: 10.00 avg. (1% cutoff) # of iter: 1003.87 avg. (5% cutoff) # of iter: 449.78 avg. (10% cutoff) # of iter: 209.46 avg. (20% cutoff) # of iter: 62.24 avg. (30% cutoff) # of iter: 42.69 Even though the average number of loop iteration is quite high, let's start to increase it conservatively to 10. Benchmark results are within a noise level (-Ofast runs faster by 0.47%). Patch survives regression tests and bootstraps on x86_64-linux-gnu. Ready for trunk? Don't you need a corresponding change to invoke.texi? OK with that change. jeff
Re: [PING] Re: [PATCH] input.c: add lexing selftests and a test matrix for line_table states
On 07/08/2016 09:00 AM, David Malcolm wrote: Ping. I believe I need review of the selftest.h change; the rest I think I can self-approve, if need be. https://gcc.gnu.org/ml/gcc-patches/2016-06/msg01340.html OK. jeff
Re: [PATCH] Fix Fortran DO loop fallback
On 07/08/2016 08:26 AM, Martin Liška wrote: Hello Following patch fixes fallout caused by the patch set: https://gcc.gnu.org/ml/gcc-regression/2016-07/msg00097.html Ready after it finished regression tests? Thanks, Martin 0001-Fix-Fortran-DO-loop-fallback.patch From c5dd7ad62f795cce560c7f1bb8767b7ed9298d8a Mon Sep 17 00:00:00 2001 From: marxin Date: Fri, 8 Jul 2016 15:51:54 +0200 Subject: [PATCH] Fix Fortran DO loop fallback gcc/testsuite/ChangeLog: 2016-07-08 Martin Liska * gfortran.dg/ldist-1.f90: Update expected dump scan. * gfortran.dg/pr42108.f90: Likewise. * gfortran.dg/vect/pr62283.f: Likewise. Shouldn't ldist-1.f90 be scan-tree-dump-not? It seems like you change it from that just last week, but it wasn't mentioned in the ChangeLog. OK with that change. jeff
Re: [RFC, v2] Test coverage for --param boundary values
On 07/08/2016 06:47 AM, Martin Liška wrote: Hi. This is my second attempt of the patch where I generate all tests on fly. Firstly, params-options.h is used to generate a list of options in form of: "predictable-branch-outcome"=2,0,50 "inline-min-speedup"=10,0,0 "max-inline-insns-single"=400,0,0 ... The list is then loaded in params.ext which triggers dg-runtest. I've also swapped the tested source file to a part of bzip2 compression algorithm. Now the testing runs 1m20s (w/ -O3) and 0m58s (w/ -O2). Hope it's much better? Martin 0001-Add-tests-that-test-boundary-values-of-params-v2.patch From f84ce7be4a998089541fb4512e19f54a4ec25cf6 Mon Sep 17 00:00:00 2001 From: marxin Date: Fri, 8 Jul 2016 10:59:24 +0200 Subject: [PATCH] Add tests that test boundary values of params gcc/ChangeLog: 2016-07-08 Martin Liska * Makefile.in: Append rule for params-options.h. * params-options.h: New file. gcc/testsuite/ChangeLog: 2016-07-08 Martin Liska * gcc.dg/params/blocksort-part.c: New test. * gcc.dg/params/params.exp: New file. Seems reasonable to me -- OK for the trunk, but please be on the lookout for these tests failing on other targets. jeff
Re: [PATCH][expr.c] PR middle-end/71700: zero-extend sub-word value when widening constructor element
On 05/07/16 13:57, Kyrill Tkachov wrote:
Hi Bernd,
On 04/07/16 19:02, Bernd Schmidt wrote:
On 07/01/2016 11:18 AM, Kyrill Tkachov wrote:
In this arm wrong-code PR the struct assignment goes wrong when
expanding constructor elements to a register destination
when the constructor elements are signed bitfields less than a word wide.
In this testcase we're intialising a struct with a 16-bit signed
bitfield to -1 followed by a 1-bit bitfield to 0.
Before it starts storing the elements it zeroes out the register.
The code in store_constructor extends the first field to word size
because it appears at the beginning of a word.
It sign-extends the -1 to word size. However, when it later tries to
store the 0 to bitposition 16 it has some logic
to avoid redundant zeroing since the destination was originally cleared,
so it doesn't emit the zero store.
But the previous sign-extended -1 took up the whole word, so the
position of the second bitfield contains a set bit.
This patch fixes the problem by zeroing out the bits of the widened
field that did not appear in the original value,
so that we can safely avoid storing the second zero in the constructor.
[...]
Bootstrapped and tested on arm, aarch64, x86_64 though the codepath is
gated on WORD_REGISTER_OPERATIONS I didn't
expect any effect on aarch64 and x86_64 anyway.
So - that code path starts with this comment:
/* If this initializes a field that is smaller than a
word, at the start of a word, try to widen it to a full
word. This special case allows us to output C++ member
function initializations in a form that the optimizers
can understand. */
Doesn't your patch completely defeat the purpose of this? Would you get
better/identical code by just deleting this block? It seems unfortunate to have
two different code generation approaches like this.
It would be interesting to know the effects of your patch, and the effects of removing this code entirely, on generated code. Try to find the motivating C++ member function example perhaps? Maybe another possibility is to ensure this
doesn't happen if the value would be interpreted as signed.
Doing some archaeology shows this code was added back in 1998 with no testcase
(r22567) so I'd have to do more digging.
My interpretation of that comment was that for WORD_REGISTER_OPERATIONS targets
it's more beneficial to have word-size
operations, so the expansion code tries to emit as many of the operations in
word_mode as it safely can.
With my patch it still emits a word_mode operation, it's just that the
immediate that is moved in word_mode has it's
top bits zeroed out instead of sign-extended.
I'll build SPEC2006 on arm (a WORD_REGISTER_OPERATIONS target) and inspect the
assembly to see if I see any interesting
effects, but I suspect there won't be much change.
Ok, I found a bit of time to investigate.
On SPEC2006 I didn't see a difference in codegen with or without that whole
block
(or with and without this patch).
The differences in codegen can be observed on the testcase in the patch.
Currently (without this patch) we generate the wrong-code:
main:
strdr3, lr, [sp, #-8]!
movwr3, #:lower16:d
mov r2, #-1
movtr3, #:upper16:d
str r2, [r3]
bl abort
With this patch that performs the zero extension on the loaded immediate but
still
widens the operation to word_mode we generate (the correct):
main:
strdr3, lr, [sp, #-8]!
movwr3, #:lower16:d
movwr2, #65535
movtr3, #:upper16:d
movr0, #0
strr2, [r3]
cbnzr0, .L5
pop{r3, pc}
.L5:
blabort
If I remove that whole block of code we end up emitting a short operation with
a bitfield
move operation, so we generate:
main:
strdr3, lr, [sp, #-8]!
movr2, #0
movr1, #-1
movwr3, #:lower16:d
bfir2, r1, #0, #16
movtr3, #:upper16:d
movr0, #0
strr2, [r3]
cbnzr0, .L5
pop{r3, pc}
.L5:
blabort
which is correct, but suboptimal compared to the codegen with this patch.
Based on that, I think that code block is a useful optimisation, we just need
to take care with immediates.
What do you think?
Thanks,
Kyrill
Re: [patch] Fix DWARF type output for non-default address spaces
On 07/07/2016 09:06 PM, James Bowman wrote: FT32 makes use of multiple address spaces. When trying to inspect objects in GDB, GDB was treating them as a straight "const". The cause seems to be in GCC DWARF2 output. This output is handled in gcc/gcc/dwarf2out.c, where modified_type_die() checks that TYPE has qualifiers CV_QUALS. However while TYPE has ADDR_SPACE qualifiers, the modified_type_die() explicitly discards the ADDR_SPACE qualifiers. This patch retains the ADDR_SPACE qualifiers as modified_type_die() outputs the DWARF type tree. This allows the types to match, and correct type information for the object is emitted. OK to commit? [gcc] 2016-07-07 James Bowman * gcc/dwarf2out.c (modified_type_die): Retain ADDR_SPACE qualifiers. (add_type_attribute) likewise. Normally any change in this part of the compiler would have to go through a bootstrap and regression test (ie, compare the result of make-check before/after your patch with no regressions). But nothing in a bootstrap is really going to exercise address space qualifiers. So I think it should be sufficient to confirm that you've built either your FT32 target or some other target with this patch. Once you've confirmed that the compiler still builds, this is OK for the trunk. jeff
Re: [PATCH] PR target/71801: Don't convert TImode in debug insn
On Mon, Jul 11, 2016 at 3:48 PM, H.J. Lu wrote:
> When converting V1TImode register in debug insn, check if it has been
> converted to TImode already.
>
> Tested on x86-64. OK for trunk?
I was looking to use simplify_subreg here, but it is too complicated
for the known purpose.
> H.J.
> ---
> gcc/
>
> PR target/71801
> * config/i386/i386.c (timode_scalar_chain::fix_debug_reg_uses):
> Don't convert TImode in debug insn.
>
> gcc/testsuite/
>
> PR target/71801
> * gcc.target/i386/pr71801.c: New test.
OK for mainline.
Thanks,
Uros.
> ---
> gcc/config/i386/i386.c | 3 +++
> gcc/testsuite/gcc.target/i386/pr71801.c | 22 ++
> 2 files changed, 25 insertions(+)
> create mode 100644 gcc/testsuite/gcc.target/i386/pr71801.c
>
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 9eaf414..d190bef 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -3814,6 +3814,9 @@ timode_scalar_chain::fix_debug_reg_uses (rtx reg)
> continue;
> gcc_assert (GET_CODE (val) == VAR_LOCATION);
> rtx loc = PAT_VAR_LOCATION_LOC (val);
> + /* It may have been converted to TImode already. */
> + if (GET_MODE (loc) == TImode)
> + continue;
> gcc_assert (REG_P (loc)
> && GET_MODE (loc) == V1TImode);
> /* Convert V1TImode register, which has been updated by a SET
> diff --git a/gcc/testsuite/gcc.target/i386/pr71801.c
> b/gcc/testsuite/gcc.target/i386/pr71801.c
> new file mode 100644
> index 000..6c87522
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr71801.c
> @@ -0,0 +1,22 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -g" } */
> +
> +struct {
> + char uuid[16];
> +} c;
> +struct {
> + int s_uuid[6];
> +} a, b;
> +int bar (void);
> +static int get_label_uuid(char *p1) {
> + __builtin_memcpy(p1, a.s_uuid, sizeof(a));
> + if (bar())
> +__builtin_memcpy(p1, b.s_uuid, sizeof(b));
> + return 0;
> +}
> +void uuidcache_addentry(char *p1) { __builtin_memcpy(&c, p1, sizeof(c)); }
> +void uuidcache_init() {
> + char d[1];
> + get_label_uuid(d);
> + uuidcache_addentry(d);
> +}
> --
Re: [patch] Fix type merging deficiency during WPA
Hi,
Sorry for jumping in late, only now I had chance to read through the whole
discussion.
I was looking into similar problem some time ago.
> Index: lto-streamer-out.c
> ===
> --- lto-streamer-out.c(revision 238156)
> +++ lto-streamer-out.c(working copy)
> @@ -996,7 +996,9 @@ hash_tree (struct streamer_tree_cache_d
>else
> hstate.add_flag (TREE_NO_WARNING (t));
>hstate.add_flag (TREE_NOTHROW (t));
> - hstate.add_flag (TREE_STATIC (t));
> + /* We want to unify DECL nodes in pointer fields of global types. */
> + if (!(VAR_OR_FUNCTION_DECL_P (t)))
> +hstate.add_flag (TREE_STATIC (t));
>hstate.add_flag (TREE_PROTECTED (t));
>hstate.add_flag (TREE_DEPRECATED (t));
>if (code != TREE_BINFO)
> @@ -1050,7 +1052,9 @@ hash_tree (struct streamer_tree_cache_d
>hstate.add_flag (DECL_ARTIFICIAL (t));
>hstate.add_flag (DECL_USER_ALIGN (t));
>hstate.add_flag (DECL_PRESERVE_P (t));
> - hstate.add_flag (DECL_EXTERNAL (t));
> + /* We want to unify DECL nodes in pointer fields of global types. */
> + if (!(VAR_OR_FUNCTION_DECL_P (t)))
> + hstate.add_flag (DECL_EXTERNAL (t));
It is fine to merge decls across static and external flags, but I am not sure
this is
a safe solution to the problem. In C it is perfectly normal to have one decl
more specified
or with different attributes. Like:
extern int a __attribute__ ((warning("bar"));
int a=7 __attribute__ ((warning("foo")));
which must prevent merging otherwise the warnings will go out wrong way. In GCC
6 timeframe we decided to live with duplicated declarations and added support
for syntactic aliases. Tree merging should be optional WRT correctness, so I
think
bug is in the canonical type calculation:
/* For array types hash the domain bounds and the string flag. */
if (TREE_CODE (type) == ARRAY_TYPE && TYPE_DOMAIN (type))
{
hstate.add_int (TYPE_STRING_FLAG (type));
/* OMP lowering can introduce error_mark_node in place of
random local decls in types. */
if (TYPE_MIN_VALUE (TYPE_DOMAIN (type)) != error_mark_node)
inchash::add_expr (TYPE_MIN_VALUE (TYPE_DOMAIN (type)), hstate);
if (TYPE_MAX_VALUE (TYPE_DOMAIN (type)) != error_mark_node)
inchash::add_expr (TYPE_MAX_VALUE (TYPE_DOMAIN (type)), hstate);
}
which boils down to:
if (tclass == tcc_declaration)
{
/* DECL's have a unique ID */
hstate.add_wide_int (DECL_UID (t));
}
(in asd_expr)
It is bit ugly, but I think for static/external/public decls we need to use
assembler name here (as we can't rely on symtab to be around all the time)
which will result in unstability WRT symbol renaming and also give false
positives for static symbols. But even for static symbols it is not guaranteed
they are not duplicated. It may be possible to use combination of assembler
name and translation unit that is available from symtab node, but not all decls
will have it defined.
Honza
Re: RFA: new pass to warn on questionable uses of alloca() and VLAs
Hey, I'm all for different options. It would definitely be easier for
me :). I wast trying really hard to use -Wvla= and keep the current
-Wvla meaning the same. Though I see your point about using -Wvla* but
using different variables for representing -Wvla and -Wvla=blah.
The easiest thing would be:
-Wvla: Current behavior (warn on everything).
-Wvla-larger-than=: Warn on unbounded VLA and bounded VLAs > .
-Walloca: Warn on every use of alloca (analogous to -Wvla).
-Walloca-larger-than=: Warn on unbounded alloca and bounded allocas
> .
This seems straightforward and avoids all the overloading you see. Would
this be ok with everyone?
I like the consistency.
Given the common root with -Wlarger-than=N, what do envision the effect
of setting -Wlarger-than=N to be on the two new options?
FWIW, I tend to view -Wlarger-than= and certainly -Wframe-larger-than=
not warning on alloca larger than N as a defect. It could be fixed by
simply hooking -Walloca-larger-than= up to it.
void f (void*);
void g (void)
{
void *p = __builtin_alloca (1024);
f (p);
}
Martin
Re: [PATCH] Fix Fortran DO loop fallback
> On Jul 11, 2016, at 7:44 AM, Jeff Law wrote: > > On 07/08/2016 08:26 AM, Martin Liška wrote: >> Hello >> >> Following patch fixes fallout caused by the patch set: >> https://gcc.gnu.org/ml/gcc-regression/2016-07/msg00097.html >> >> Ready after it finished regression tests? >> Thanks, >> Martin >> >> >> 0001-Fix-Fortran-DO-loop-fallback.patch >> >> >> From c5dd7ad62f795cce560c7f1bb8767b7ed9298d8a Mon Sep 17 00:00:00 2001 >> From: marxin >> Date: Fri, 8 Jul 2016 15:51:54 +0200 >> Subject: [PATCH] Fix Fortran DO loop fallback >> >> gcc/testsuite/ChangeLog: >> >> 2016-07-08 Martin Liska >> >> * gfortran.dg/ldist-1.f90: Update expected dump scan. >> * gfortran.dg/pr42108.f90: Likewise. >> * gfortran.dg/vect/pr62283.f: Likewise. > Shouldn't ldist-1.f90 be scan-tree-dump-not? It seems like you change it > from that just last week, but it wasn't mentioned in the ChangeLog. > > OK with that change. A gentle reminder, fortran patches should include the fortran list...
Fix loop_only_exit_p
Hi,
while looking into loop code I noticed that loop_only_exit_p seems overly
simplistic. The purpose of this predicate is to decide whether the loop
must terminate by the exit given (so for example it is known that the number
of iteration test in that particular exit won't overflow because undefined
effect will happen then).
The code checks for calls with side effects, but it misses the fact that loop
can exit by an external EH (with -fnon-call-exceptions) or by an asm statement.
Exactly the same problem is being solved by the gcov code where it is necessary
which tries to verify that given the first statement of bb is executed, the
control flow will continue by one of the outgoing edges (which is necesssary
to make Kirhoff law applicable).
This patch thus unifies the logic and also fixes probably ages long bug about
ignoring external EH in profiling code.
Bootstrapped/regtested x86_64-linux, seems sane?
Honza
* gimple.h (stmt_can_terminate_bb_p): New function.
* tree-cfg.c (need_fake_edge_p): Rename to ...
(stmt_can_terminate_bb_p): ... this; return true if stmt can
throw external; handle const and pure calls.
* tree-ssa-loop-niter.c (loop_only_exit_p): Use it.
Index: gimple.h
===
--- gimple.h(revision 238191)
+++ gimple.h(working copy)
@@ -1526,6 +1526,7 @@ extern void gimple_seq_set_location (gim
extern void gimple_seq_discard (gimple_seq);
extern void maybe_remove_unused_call_args (struct function *, gimple *);
extern bool gimple_inexpensive_call_p (gcall *);
+extern bool stmt_can_terminate_bb_p (gimple *);
/* Formal (expression) temporary table handling: multiple occurrences of
the same scalar expression are evaluated into the same temporary. */
Index: tree-cfg.c
===
--- tree-cfg.c (revision 238191)
+++ tree-cfg.c (working copy)
@@ -7919,15 +7919,20 @@ gimple_block_ends_with_condjump_p (const
}
-/* Return true if we need to add fake edge to exit at statement T.
- Helper function for gimple_flow_call_edges_add. */
+/* Return true if statement T may terminate execution of BB in ways not
+ explicitly represtented in the CFG. */
-static bool
-need_fake_edge_p (gimple *t)
+bool
+stmt_can_terminate_bb_p (gimple *t)
{
tree fndecl = NULL_TREE;
int call_flags = 0;
+ /* Eh exception not handled internally terminates execution of the whole
+ function. */
+ if (stmt_can_throw_external (t))
+return true;
+
/* NORETURN and LONGJMP calls already have an edge to exit.
CONST and PURE calls do not need one.
We don't currently check for CONST and PURE here, although
@@ -7960,6 +7965,13 @@ need_fake_edge_p (gimple *t)
edge e;
basic_block bb;
+ if (call_flags & (ECF_PURE | ECF_CONST)
+ && !(call_flags & ECF_LOOPING_CONST_OR_PURE))
+ return false;
+
+ /* Function call may do longjmp, terminate program or do other things.
+Special case noreturn that have non-abnormal edges out as in this case
+the fact is sufficiently represented by lack of edges out of T. */
if (!(call_flags & ECF_NORETURN))
return true;
@@ -8024,7 +8036,7 @@ gimple_flow_call_edges_add (sbitmap bloc
if (!gsi_end_p (gsi))
t = gsi_stmt (gsi);
- if (t && need_fake_edge_p (t))
+ if (t && stmt_can_terminate_bb_p (t))
{
edge e;
@@ -8059,7 +8071,7 @@ gimple_flow_call_edges_add (sbitmap bloc
do
{
stmt = gsi_stmt (gsi);
- if (need_fake_edge_p (stmt))
+ if (stmt_can_terminate_bb_p (stmt))
{
edge e;
Index: tree-ssa-loop-niter.c
===
--- tree-ssa-loop-niter.c (revision 238191)
+++ tree-ssa-loop-niter.c (working copy)
@@ -2159,7 +2159,6 @@ loop_only_exit_p (const struct loop *loo
basic_block *body;
gimple_stmt_iterator bsi;
unsigned i;
- gimple *call;
if (exit != single_exit (loop))
return false;
@@ -2168,17 +2167,8 @@ loop_only_exit_p (const struct loop *loo
for (i = 0; i < loop->num_nodes; i++)
{
for (bsi = gsi_start_bb (body[i]); !gsi_end_p (bsi); gsi_next (&bsi))
- {
- call = gsi_stmt (bsi);
- if (gimple_code (call) != GIMPLE_CALL)
- continue;
-
- if (gimple_has_side_effects (call))
- {
- free (body);
- return false;
- }
- }
+ if (stmt_can_terminate_bb_p (gsi_stmt (bsi)))
+ return true;
}
free (body);
if (EDGE_COUNT (bb->succs) > 1)
Re: RFA: new pass to warn on questionable uses of alloca() and VLAs
On 07/11/2016 11:08 AM, Martin Sebor wrote: Hey, I'm all for different options. It would definitely be easier for me :). I wast trying really hard to use -Wvla= and keep the current -Wvla meaning the same. Though I see your point about using -Wvla* but using different variables for representing -Wvla and -Wvla=blah. The easiest thing would be: -Wvla: Current behavior (warn on everything). -Wvla-larger-than=: Warn on unbounded VLA and bounded VLAs > . -Walloca: Warn on every use of alloca (analogous to -Wvla). -Walloca-larger-than=: Warn on unbounded alloca and bounded allocas > . This seems straightforward and avoids all the overloading you see. Would this be ok with everyone? I like the consistency. Given the common root with -Wlarger-than=N, what do envision the effect of setting -Wlarger-than=N to be on the two new options? FWIW, I tend to view -Wlarger-than= and certainly -Wframe-larger-than= not warning on alloca larger than N as a defect. It could be fixed by simply hooking -Walloca-larger-than= up to it. I'm not touching any of these independent options. -Wvla-larger-than= and -Walloca-larger-than= will be implemented independently of whatever -Wlarger-than= and -Wframe-larger-than=. Any problems with these last two options can be treated indpendently (PRs). Aldy
[PATCHv2][ARM] -mpure-code option for ARM
On 07/07/16 13:30, mickael guene wrote: > Hi Andre, > > Another feedback on your purecode patch. > You have to disable casesi pattern since then it will > generate wrong code with -mpure-code option. > Indeed it will generate an 'adr rx, .Lx' (aka > 'subs rx, PC, #offset') which will not work in our > case since 'Lx' label is put in an .rodata section. > So offset value is unknown and can be impossible > to encode correctly. > > Regards > Mickael > > On 06/30/2016 04:32 PM, Andre Vieira (lists) wrote: >> Hello, >> >> This patch adds the -mpure-code option for ARM. This option ensures >> functions are put into sections that contain only code and no data. To >> ensure this throughout compilation we give these sections the ARM >> processor-specific ELF section attribute "SHF_ARM_PURECODE". This option >> is only supported for non-pic code for armv7-m targets. >> >> This patch introduces a new target hook 'TARGET_ASM_ELF_FLAGS_NUMERIC'. >> This target hook enables a target to use the numeric value for elf >> section attributes rather than their alphabetical representation. If >> TARGET_ASM_ELF_FLAGS_NUMERIC returns TRUE, the existing >> 'default_elf_asm_named_section', will print the numeric value of the >> section attributes for the current section. This target hook has two >> parameters: >> unsigned int FLAGS, the input parameter that tells the function the >> current section's attributes; >> unsigned int *NUM, used to pass down the numerical representation of the >> section's attributes. >> >> The default implementation for TARGET_ASM_ELF_FLAGS_NUMERIC will return >> false, so existing behavior is not changed. >> >> Bootstrapped and tested for arm-none-linux-gnueabihf. Further tested for >> arm-none-eabi with a Cortex-M3 target. >> >> >> gcc/ChangeLog: >> 2016-06-30 Andre Vieira >> Terry Guo >> >> * target.def (elf_flags_numeric): New target hook. >> * targhooks.h (default_asm_elf_flags_numeric): New. >> * varasm.c (default_asm_elf_flags_numeric): New. >> (default_elf_asm_named_section): Use new target hook. >> * config/arm/arm.opt (mpure-code): New. >> * config/arm/arm.h (SECTION_ARM_PURECODE): New. >> * config/arm/arm.c (arm_asm_init_sections): Add section >> attribute to default text section if -mpure-code. >> (arm_option_check_internal): Diagnose use of option with >> non supported targets and/or options. >> (arm_asm_elf_flags_numeric): New. >> (arm_function_section): New. >> (arm_elf_section_type_flags): New. >> * config/arm/elf.h (JUMP_TABLES_IN_TEXT_SECTION): Disable >> for -mpure-code. >> * gcc/doc/texi (TARGET_ASM_ELF_FLAGS_NUMERIC): New. >> * gcc/doc/texi.in (TARGET_ASM_ELF_FLAGS_NUMERIC): Likewise. >> >> >> >> gcc/testsuite/ChangeLog: >> 2016-06-30 Andre Vieira >> Terry Guo >> >> * gcc.target/arm/pure-code/ffunction-sections.c: New. >> * gcc.target/arm/pure-code/no-literal-pool.c: New. >> * gcc.target/arm/pure-code/pure-code.exp: New. >> > Hi Sandra, Mickael, Thank you for your comments. I changed the description of -mpure-code in invoke.texi to better reflect the error message you get wrt supported targets. As for the target hook description, I hope the text is clearer now. Let me know if you think it needs further explanation. I also fixed the double '%' in the text string for unnamed text sections and disabled the casesi pattern. I duplicated the original casesi test 'gcc/testsuite/gcc.c-torture/compile/pr46934.c' for pure-code to make sure the casesi was disabled and other patterns were selected instead. Reran regressions for pure-code.exp for Cortex-M3. Cheers, Andre gcc/ChangeLog: 2016-07-11 Andre Vieira Terry Guo * target.def (elf_flags_numeric): New target hook. * hooks.c (hook_uint_uintp_false): New generic hook. * varasm.c (default_elf_asm_named_section): Use new target hook. * config/arm/arm.opt (mpure-code): New. * config/arm/arm.h (SECTION_ARM_PURECODE): New. * config/arm/arm.c (arm_asm_init_sections): Add section attribute to default text section if -mpure-code. (arm_option_check_internal): Diagnose use of option with non supported targets and/or options. (arm_asm_elf_flags_numeric): New. (arm_function_section): New. (arm_elf_section_type_flags): New. * config/arm/elf.h (JUMP_TABLES_IN_TEXT_SECTION): Disable for -mpure-code. * config/arm/arm.md (casesi): Disable pattern for -mpure-code. * gcc/doc/texi (TARGET_ASM_ELF_FLAGS_NUMERIC): New. * gcc/doc/texi.in (TARGET_ASM_ELF_FLAGS_NUMERIC): Likewise. gcc/testsuite/ChangeLog: 2016-07-11 Andre Vieira Terry Guo * gcc.target/arm/pure-code/ffunction-sections.c: New. * gcc.target/arm/pure-code/no-literal-pool.c: New. *
Small C++ PATCH to remove dead code
This code in store_parm_decls never fired, because we were checking
VOID_TYPE_P on the PARM_DECL itself rather than its type. And we
already check for void parms in grokparms, so there's no point in
doing it here.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit 7bc6f4c24e2e853ff18bd61f116e02d3b9fe7b6f
Author: Jason Merrill
Date: Mon Jul 11 01:38:35 2016 -0400
* decl.c (store_parm_decls): Remove check for void parm.
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index c86a131..09bb767 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -14376,13 +14376,7 @@ store_parm_decls (tree current_function_parms)
{
next = DECL_CHAIN (parm);
if (TREE_CODE (parm) == PARM_DECL)
- {
- if (DECL_NAME (parm) == NULL_TREE
- || !VOID_TYPE_P (parm))
- pushdecl (parm);
- else
- error ("parameter %qD declared void", parm);
- }
+ pushdecl (parm);
else
{
/* If we find an enum constant or a type tag,
Re: [PATCHv2, ARM, libgcc] New aeabi_idiv function for armv6-m
On 06/07/16 11:52, Andre Vieira (lists) wrote:
> On 01/07/16 14:40, Ramana Radhakrishnan wrote:
>>
>>
>> On 13/10/15 18:01, Andre Vieira wrote:
>>> This patch ports the aeabi_idiv routine from Linaro Cortex-Strings
>>> (https://git.linaro.org/toolchain/cortex-strings.git), which was
>>> contributed by ARM under Free BSD license.
>>>
>>> The new aeabi_idiv routine is used to replace the one in
>>> libgcc/config/arm/lib1funcs.S. This replacement happens within the Thumb1
>>> wrapper. The new routine is under LGPLv3 license.
>>
>> This is not under LGPLv3 . It is under GPLv3 with the runtime library
>> exception license, there's a difference. Assuming your licensing expectation
>> is ok read on for more of a review.
>>
>>>
>>> The main advantage of this version is that it can improve the performance
>>> of the aeabi_idiv function for Thumb1. This solution will also increase the
>>> code size. So it will only be used if __OPTIMIZE_SIZE__ is not defined.
>>>
>>> Make check passed for armv6-m.
>>>
>>> libgcc/ChangeLog:
>>> 2015-08-10 Hale Wang
>>> Andre Vieira
>>>
>>> * config/arm/lib1funcs.S: Add new wrapper.
>>>
>>> 0001-integer-division.patch
>>>
>>>
>>> From 832a3d6af6f06399f70b5a4ac3727d55960c93b7 Mon Sep 17 00:00:00 2001
>>> From: Andre Simoes Dias Vieira
>>> Date: Fri, 21 Aug 2015 14:23:28 +0100
>>> Subject: [PATCH] new wrapper idivmod
>>>
>>> ---
>>> libgcc/config/arm/lib1funcs.S | 250
>>> --
>>> 1 file changed, 217 insertions(+), 33 deletions(-)
>>>
>>> diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S
>>> index
>>> 252efcbd5385cc58a5ce1e48c6816d36a6f4c797..c9e544114590da8cde88382bea0f67206e593816
>>> 100644
>>> --- a/libgcc/config/arm/lib1funcs.S
>>> +++ b/libgcc/config/arm/lib1funcs.S
>>> @@ -306,34 +306,12 @@ LSYM(Lend_fde):
>>> #ifdef __ARM_EABI__
>>> .macro THUMB_LDIV0 name signed
>>> #if defined(__ARM_ARCH_6M__)
>>> - .ifc \signed, unsigned
>>> - cmp r0, #0
>>> - beq 1f
>>> - mov r0, #0
>>> - mvn r0, r0 @ 0x
>>> -1:
>>> - .else
>>> - cmp r0, #0
>>> - beq 2f
>>> - blt 3f
>>> +
>>> + push{r0, lr}
>>> mov r0, #0
>>> - mvn r0, r0
>>> - lsr r0, r0, #1 @ 0x7fff
>>> - b 2f
>>> -3: mov r0, #0x80
>>> - lsl r0, r0, #24 @ 0x8000
>>> -2:
>>> - .endif
>>> - push{r0, r1, r2}
>>> - ldr r0, 4f
>>> - adr r1, 4f
>>> - add r0, r1
>>> - str r0, [sp, #8]
>>> - @ We know we are not on armv4t, so pop pc is safe.
>>> - pop {r0, r1, pc}
>>> - .align 2
>>> -4:
>>> - .word __aeabi_idiv0 - 4b
>>> + bl SYM(__aeabi_idiv0)
>>> + pop {r1, pc}
>>> +
>>
>> I'd still retain the comment about pop pc here because there's often a
>> misconception of merging armv4t and armv6m code.
>>
>>> #elif defined(__thumb2__)
>>> .syntax unified
>>> .ifc \signed, unsigned
>>> @@ -945,7 +923,170 @@ LSYM(Lover7):
>>> add dividend, work
>>>.endif
>>> LSYM(Lgot_result):
>>> -.endm
>>> +.endm
>>> +
>>> +#if defined(__prefer_thumb__) && !defined(__OPTIMIZE_SIZE__)
>>> +/* If performance is preferred, the following functions are provided. */
>>> +
>>
>> Comment above #if please and also check elsewhere in patch.
>>
>>> +/* Branch to div(n), and jump to label if curbit is lo than divisior. */
>>> +.macro BranchToDiv n, label
>>> + lsr curbit, dividend, \n
>>> + cmp curbit, divisor
>>> + blo \label
>>> +.endm
>>> +
>>> +/* Body of div(n). Shift the divisor in n bits and compare the divisor
>>> + and dividend. Update the dividend as the substruction result. */
>>> +.macro DoDiv n
>>> + lsr curbit, dividend, \n
>>> + cmp curbit, divisor
>>> + bcc 1f
>>> + lsl curbit, divisor, \n
>>> + sub dividend, dividend, curbit
>>> +
>>> +1: adc result, result
>>> +.endm
>>> +
>>> +/* The body of division with positive divisor. Unless the divisor is very
>>> + big, shift it up in multiples of four bits, since this is the amount of
>>> + unwinding in the main division loop. Continue shifting until the
>>> divisor
>>> + is larger than the dividend. */
>>> +.macro THUMB1_Div_Positive
>>> + mov result, #0
>>> + BranchToDiv #1, LSYM(Lthumb1_div1)
>>> + BranchToDiv #4, LSYM(Lthumb1_div4)
>>> + BranchToDiv #8, LSYM(Lthumb1_div8)
>>> + BranchToDiv #12, LSYM(Lthumb1_div12)
>>> + BranchToDiv #16, LSYM(Lthumb1_div16)
>>> +LSYM(Lthumb1_div_large_positive):
>>> + mov result, #0xff
>>> + lsl divisor, divisor, #8
>>> + rev result, result
>>> + lsr curbit, dividend, #16
>>> + cmp curbit, divisor
>>> + blo 1f
>>> + asr result, #8
>>> + lsl divisor, divisor, #8
>>> + beq LSYM(Ldivbyzero_waypoint)
>>> +
>>> +1: lsr curbit, dividend, #12
>>> + cmp curbit, divisor
>>> + blo LSYM(Lthumb1_div12)
>>> + b LSYM(Lthumb1_div1
Re: [PATCH][expr.c] PR middle-end/71700: zero-extend sub-word value when widening constructor element
On 07/11/2016 04:52 PM, Kyrill Tkachov wrote: Based on that, I think that code block is a useful optimisation, we just need to take care with immediates. What do you think? Yeah, I think the patch is ok. Bernd
Re: RFA: new pass to warn on questionable uses of alloca() and VLAs
On 07/11/2016 09:40 AM, Aldy Hernandez wrote: On 07/11/2016 11:08 AM, Martin Sebor wrote: Hey, I'm all for different options. It would definitely be easier for me :). I wast trying really hard to use -Wvla= and keep the current -Wvla meaning the same. Though I see your point about using -Wvla* but using different variables for representing -Wvla and -Wvla=blah. The easiest thing would be: -Wvla: Current behavior (warn on everything). -Wvla-larger-than=: Warn on unbounded VLA and bounded VLAs > . -Walloca: Warn on every use of alloca (analogous to -Wvla). -Walloca-larger-than=: Warn on unbounded alloca and bounded allocas > . This seems straightforward and avoids all the overloading you see. Would this be ok with everyone? I like the consistency. Given the common root with -Wlarger-than=N, what do envision the effect of setting -Wlarger-than=N to be on the two new options? FWIW, I tend to view -Wlarger-than= and certainly -Wframe-larger-than= not warning on alloca larger than N as a defect. It could be fixed by simply hooking -Walloca-larger-than= up to it. I'm not touching any of these independent options. -Wvla-larger-than= and -Walloca-larger-than= will be implemented independently of whatever -Wlarger-than= and -Wframe-larger-than=. Any problems with these last two options can be treated indpendently (PRs). Strictly speaking there is no defect in -Wframe-larger-than= because it's documented not to warn for alloca or VLAs. I asked because it seems like an unnecessary (and IMO undesirable) limitation that could be easily removed as part of this patch. Not removing it, OTOH, and providing a separate solution, feels like highlighting the limitation. With the new options, users interested in detecting all forms of excessive stack allocation will be able to do so but not using a single option. Instead, they will need to use all three. Martin
[arm-embedded] [PATCH, libgcc/ARM 1a/6] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions
We've decided to apply the following patch to ARM/embedded-6-branch. Best regards, Thomas -- Forwarded Message -- Subject: Re: [PATCH, libgcc/ARM 1a/6] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions Date: Wednesday 06 July 2016, 17:21:24 From: Ramana Radhakrishnan To: Thomas Preudhomme CC: gcc-patches On Fri, Jun 17, 2016 at 6:21 PM, Thomas Preudhomme wrote: > On Wednesday 01 June 2016 10:00:52 Ramana Radhakrishnan wrote: >> Please fix up the macros, post back and redo the test. Otherwise this >> is ok from a quick read. > > What about the updated patch in attachment? As for the original patch, I've > checked that code generation does not change for a number of combinations of > ISAs (ARM/Thumb), optimization levels (Os/O2), and architectures (armv4, > armv4t, armv5, armv5t, armv5te, armv6, armv6j, armv6k, armv6s-m, armv6kz, > armv6t2, armv6z, armv6zk, armv7, armv7-a, armv7e-m, armv7-m, armv7-r, armv7ve, > armv8-a, armv8-a+crc, iwmmxt and iwmmxt2). > > Note, I renumbered this patch 1a to not make the numbering of other patches > look strange. The CLZ part is now in patch 1b/7. > > ChangeLog entries are now as follow: > > > *** gcc/ChangeLog *** > > 2016-05-23 Thomas Preud'homme > > * config/arm/elf.h: Use __ARM_ARCH_ISA_THUMB and __ARM_ARCH_ISA_ARM to > decide whether to prevent some libgcc routines being included for some > multilibs rather than __ARM_ARCH_6M__ and add comment to indicate the > link between this condition and the one in > libgcc/config/arm/lib1func.S. > > > *** gcc/testsuite/ChangeLog *** > > 2015-11-10 Thomas Preud'homme > > * lib/target-supports.exp (check_effective_target_arm_cortex_m): Use > __ARM_ARCH_ISA_ARM to test for Cortex-M devices. > > > *** libgcc/ChangeLog *** > > 2016-06-01 Thomas Preud'homme > > * config/arm/bpabi-v6m.S: Clarify what architectures is the > implementation suitable for. > * config/arm/lib1funcs.S (__prefer_thumb__): Define among other cases > for all Thumb-1 only targets. > (NOT_ISA_TARGET_32BIT): Define for Thumb-1 only targets. > (THUMB_LDIV0): Test for NOT_ISA_TARGET_32BIT rather than > __ARM_ARCH_6M__. > (EQUIV): Likewise. > (ARM_FUNC_ALIAS): Likewise. > (umodsi3): Add check to __ARM_ARCH_ISA_THUMB != 1 to guard the idiv > version. > (modsi3): Likewise. > (clzsi2): Test for NOT_ISA_TARGET_32BIT rather than __ARM_ARCH_6M__. > (clzdi2): Likewise. > (ctzsi2): Likewise. > (L_interwork_call_via_rX): Test for __ARM_ARCH_ISA_ARM rather than > __ARM_ARCH_6M__ in guard for checking whether it is defined. > (final includes): Test for NOT_ISA_TARGET_32BIT rather than > __ARM_ARCH_6M__ and add comment to indicate the connection between > this condition and the one in gcc/config/arm/elf.h. > * config/arm/libunwind.S: Test for __ARM_ARCH_ISA_THUMB and > __ARM_ARCH_ISA_ARM rather than __ARM_ARCH_6M__. > * config/arm/t-softfp: Likewise. > > > Best regards, > > Thomas OK - thanks for your patience and sorry about the delay. Ramana -
[arm-embedded] [PATCH, libgcc/ARM 1/6] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions
We've decided to apply the following patch to ARM/embedded-6-branch. Best regards, Thomas -- Forwarded Message -- Subject: Re: [PATCH, libgcc/ARM 1/6] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions Date: Wednesday 06 July 2016, 17:20:04 From: Ramana Radhakrishnan To: Thomas Preudhomme CC: gcc-patches On Mon, Jun 27, 2016 at 5:51 PM, Thomas Preudhomme wrote: > Hi Ramana, > > On Wednesday 01 June 2016 10:00:52 Ramana Radhakrishnan wrote: >> >> From here down to >> >> > -#if ((__ARM_ARCH__ > 5) && !defined(__ARM_ARCH_6M__)) \ >> > -|| defined(__ARM_ARCH_5E__) || defined(__ARM_ARCH_5TE__) \ >> > -|| defined(__ARM_ARCH_5TEJ__) >> > -#define HAVE_ARM_CLZ 1 >> > -#endif >> > - >> > >> > #ifdef L_clzsi2 >> > >> > -#if defined(__ARM_ARCH_6M__) >> > +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 >> > >> > FUNC_START clzsi2 >> > >> > mov r1, #28 >> > mov r3, #1 >> > >> > @@ -1544,7 +1538,7 @@ FUNC_START clzsi2 >> > >> > FUNC_END clzsi2 >> > >> > #else >> > ARM_FUNC_START clzsi2 >> > >> > -# if defined(HAVE_ARM_CLZ) >> > +# if defined(__ARM_FEATURE_CLZ) >> > >> > clz r0, r0 >> > RET >> > >> > # else >> > >> > @@ -1568,15 +1562,15 @@ ARM_FUNC_START clzsi2 >> > >> > .align 2 >> > 1: >> > .byte 4, 3, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0 >> > >> > -# endif /* !HAVE_ARM_CLZ */ >> > +# endif /* !__ARM_FEATURE_CLZ */ >> > >> > FUNC_END clzsi2 >> > >> > #endif >> > #endif /* L_clzsi2 */ >> > >> > #ifdef L_clzdi2 >> > >> > -#if !defined(HAVE_ARM_CLZ) >> > +#if !defined(__ARM_FEATURE_CLZ) >> >> here should be it's own little patchlet and can go in separately. > > The patch in attachment changes the CLZ availability check in libgcc to test > ISA supported and architecture version rather than encode a specific list of > architectures. __ARM_FEATURE_CLZ is not used because its value depends on what > mode the user is targeting but only the architecture support matters in this > case. Indeed, the code using CLZ is written in assembler and uses mnemonics > available both in ARM and Thumb mode so only CLZ availability in one of the > mode matters. > > This change was split out from [PATCH, GCC, ARM 1/7] Fix Thumb-1 only == > ARMv6-M & Thumb-2 only == ARMv7-M assumptions. > > ChangeLog entry is as follows: > > *** libgcc/ChangeLog *** > > 2016-06-16 Thomas Preud'homme > > * config/arm/lib1funcs.S (HAVE_ARM_CLZ): Define for ARMv6* or later > and ARMv5t* rather than for a fixed list of architectures. > > Looking for code generation change accross a number of combinations of ISAs > (ARM/Thumb), optimization levels (Os/O2), and architectures (armv4, armv4t, > armv5, armv5t, armv5te, armv6, armv6j, armv6k, armv6s-m, armv6kz, armv6t2, > armv6z, armv6zk, armv7, armv7-a, armv7e-m, armv7-m, armv7-r, armv7ve, armv8- a, > armv8-a+crc, iwmmxt and iwmmxt2) shows that only ARMv5T is impacted (uses CLZ > now). This is expected because currently HAVE_ARM_CLZ is not defined for this > architecture while the ARMv7-a/ARMv7-R Architecture Reference Manual [1] > states that all ARMv5T* architectures have CLZ. ARMv5E should also be impacted > (not using CLZ anymore) but testing it is difficult since current binutils does > not support ARMv5E. > > [1] Document ARM DDI0406C in http://infocenter.arm.com > > Best regards, > > Thomas OK. Ramana -
[arm-embedded] [PATCH, ARM 2/7, ping1] Add support for ARMv8-M
We've decided to apply the following patch to ARM/embedded-6-branch. Best regards, Thomas -- Forwarded Message -- Subject: Re: [PATCH, ARM 2/7, ping1] Add support for ARMv8-M Date: Thursday 07 July 2016, 09:57:08 From: Thomas Preudhomme To: Kyrill Tkachov CC: Richard Earnshaw , Ramana Radhakrishnan , gcc-patches@gcc.gnu.org On Wednesday 25 May 2016 14:32:54 Kyrill Tkachov wrote: > Hi Thomas, > > On 25/05/16 14:26, Thomas Preudhomme wrote: > > On Thursday 19 May 2016 17:59:26 Kyrill Tkachov wrote: > >> Hi Thomas, > > > > Hi Kyrill, > > > > Please find an updated patch in attachment. ChangeLog entries are now as > > follow: > > > > *** gcc/ChangeLog *** > > > > 2016-05-23 Thomas Preud'homme > > > > * config/arm/arm-arches.def (armv8-m.base): Define new > > architecture. > > (armv8-m.main): Likewise. > > (armv8-m.main+dsp): Likewise > > Full stop after "Likewise". > This patch is ok once the others are approved. Now that prerequisites patches have been approved, I've committed it with the following corrected ChangeLog entries: *** gcc/ChangeLog *** 2016-05-23 Thomas Preud'homme * config/arm/arm-arches.def (armv8-m.base): Define new architecture. (armv8-m.main): Likewise. (armv8-m.main+dsp): Likewise. * config/arm/arm-protos.h (FL_FOR_ARCH8M_BASE): Define. (FL_FOR_ARCH8M_MAIN): Likewise. * config/arm/arm-tables.opt: Regenerate. * config/arm/bpabi.h: Add armv8-m.base, armv8-m.main and armv8-m.main+dsp to BE8_LINK_SPEC. * config/arm/arm.h (TARGET_HAVE_LDACQ): Exclude ARMv8-M. (enum base_architecture): Add BASE_ARCH_8M_BASE and BASE_ARCH_8M_MAIN. * config/arm/arm.c (arm_arch_name): Increase size to work with ARMv8-M Baseline and Mainline. (arm_option_override_internal): Also disable arm_restrict_it when !arm_arch_notm. Update comment for -munaligned-access to also cover ARMv8-M Baseline. (arm_file_start): Increase buffer size for printing architecture name. * doc/invoke.texi: Document architectures armv8-m.base, armv8-m.main and armv8-m.main+dsp. (mno-unaligned-access): Clarify that this is disabled by default for ARMv8-M Baseline architectures as well. *** gcc/testsuite/ChangeLog *** 2015-11-10 Thomas Preud'homme * lib/target-supports.exp: Generate add_options_for_arm_arch_FUNC and check_effective_target_arm_arch_FUNC_multilib for ARMv8-M Baseline and ARMv8-M Mainline architectures. *** libgcc/ChangeLog *** 2015-11-10 Thomas Preud'homme * config/arm/lib1funcs.S (__ARM_ARCH__): Define to 8 for ARMv8-M. Thanks. Best regards, Thomas -
[arm-embedded] [PATCH, ARM 4/7, ping1] Factor out MOVW/MOVT availability and desirability checks
We've decided to apply the following patch to ARM/embedded-6-branch. Best regards, Thomas -- Forwarded Message -- Subject: Re: [PATCH, ARM 4/7, ping1] Factor out MOVW/MOVT availability and desirability checks Date: Thursday 07 July 2016, 09:59:53 From: Thomas Preudhomme To: Kyrill Tkachov CC: gcc-patches@gcc.gnu.org, Richard Earnshaw , Ramana Radhakrishnan On Friday 20 May 2016 13:41:30 Kyrill Tkachov wrote: > Hi Thomas, > > On 19/05/16 17:10, Thomas Preudhomme wrote: > > On Wednesday 18 May 2016 11:47:47 Kyrill Tkachov wrote: > >> Hi Thomas, > > > > Hi Kyrill, > > > > Please find below the updated patch and associated ChangeLog entry. > > > > *** gcc/ChangeLog *** > > > > 2016-05-18 Thomas Preud'homme > > > > * config/arm/arm.h (TARGET_USE_MOVT): Check MOVT/MOVW > > availability > > with TARGET_HAVE_MOVT. > > (TARGET_HAVE_MOVT): Define. > > * config/arm/arm.c (const_ok_for_op): Check MOVT/MOVW > > availability with TARGET_HAVE_MOVT. > > * config/arm/arm.md (arm_movt): Use TARGET_HAVE_MOVT to check > > movt > > availability. > > (addsi splitter): Use TARGET_THUMB && TARGET_HAVE_MOVT rather > > than > > TARGET_THUMB2. > > (symbol_refs movsi splitter): Remove TARGET_32BIT check. > > (arm_movtas_ze): Use TARGET_HAVE_MOVT to check movt availability. > > * config/arm/constraints.md (define_constraint "j"): Use > > TARGET_HAVE_MOVT to check movt availability. > > Please use capitalised MOVW/MOVT consistently in the ChangeLog. > Ok with a fixed ChangeLog. Committed with the following ChangeLog entry: 2016-05-18 Thomas Preud'homme * config/arm/arm.h (TARGET_USE_MOVT): Check MOVT/MOVW availability with TARGET_HAVE_MOVT. (TARGET_HAVE_MOVT): Define. * config/arm/arm.c (const_ok_for_op): Check MOVT/MOVW availability with TARGET_HAVE_MOVT. * config/arm/arm.md (arm_movt): Use TARGET_HAVE_MOVT to check MOVT availability. (addsi splitter): Use TARGET_THUMB && TARGET_HAVE_MOVT rather than TARGET_THUMB2. (symbol_refs movsi splitter): Remove TARGET_32BIT check. (arm_movtas_ze): Use TARGET_HAVE_MOVT to check MOVT availability. * config/arm/constraints.md (define_constraint "j"): Use TARGET_HAVE_MOVT to check MOVT availability. Thanks. Best regards, Thomas -
[arm-embedded] [PATCH, ARM 3/7, ping1] Fix indentation of FL_FOR_ARCH* definition after adding support for ARMv8-M
We've decided to apply the following patch to ARM/embedded-6-branch. Best regards, Thomas -- Forwarded Message -- Subject: Re: [PATCH, ARM 3/7, ping1] Fix indentation of FL_FOR_ARCH* definition after adding support for ARMv8-M Date: Wednesday 18 May 2016, 14:47:16 From: Kyrill Tkachov To: Thomas Preudhomme CC: ramana.radhakrish...@arm.com, richard.earns...@arm.com, gcc- patc...@gcc.gnu.org On 18/05/16 14:45, Thomas Preudhomme wrote: > On Wednesday 18 May 2016 11:30:43 Kyrill Tkachov wrote: >> Hi Thomas, >> >> On 17/05/16 11:10, Thomas Preudhomme wrote: >>> Ping? >>> >>> *** gcc/ChangeLog *** >>> >>> 2015-11-06 Thomas Preud'homme >>> >>> * config/arm/arm-protos.h: Reindent FL_FOR_* macro definitions. >>> >>> diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h >>> index >>> 63235cb63acf3e676fac5b61e1195081efd64075..f437d0d8baa5534f9519dd28cd2c4ac5 >>> 2d48685c 100644 >>> --- a/gcc/config/arm/arm-protos.h >>> +++ b/gcc/config/arm/arm-protos.h >>> @@ -395,30 +395,31 @@ extern bool arm_is_constant_pool_ref (rtx); >>> >>>#define FL_TUNE (FL_WBUF | FL_VFPV2 | FL_STRONG | FL_LDSCHED \ >>> >>> | FL_CO_PROC) >>> >>> -#define FL_FOR_ARCH2 FL_NOTM >>> -#define FL_FOR_ARCH3 (FL_FOR_ARCH2 | FL_MODE32) >>> -#define FL_FOR_ARCH3M (FL_FOR_ARCH3 | FL_ARCH3M) >>> -#define FL_FOR_ARCH4 (FL_FOR_ARCH3M | FL_ARCH4) >>> -#define FL_FOR_ARCH4T (FL_FOR_ARCH4 | FL_THUMB) >>> -#define FL_FOR_ARCH5 (FL_FOR_ARCH4 | FL_ARCH5) >>> -#define FL_FOR_ARCH5T (FL_FOR_ARCH5 | FL_THUMB) >>> -#define FL_FOR_ARCH5E (FL_FOR_ARCH5 | FL_ARCH5E) >>> -#define FL_FOR_ARCH5TE (FL_FOR_ARCH5E | FL_THUMB) >>> -#define FL_FOR_ARCH5TEJFL_FOR_ARCH5TE >>> -#define FL_FOR_ARCH6 (FL_FOR_ARCH5TE | FL_ARCH6) >>> -#define FL_FOR_ARCH6J FL_FOR_ARCH6 >>> -#define FL_FOR_ARCH6K (FL_FOR_ARCH6 | FL_ARCH6K) >>> -#define FL_FOR_ARCH6Z FL_FOR_ARCH6 >>> -#define FL_FOR_ARCH6KZ (FL_FOR_ARCH6K | FL_ARCH6KZ) >>> -#define FL_FOR_ARCH6T2 (FL_FOR_ARCH6 | FL_THUMB2) >>> -#define FL_FOR_ARCH6M (FL_FOR_ARCH6 & ~FL_NOTM) >>> -#define FL_FOR_ARCH7 ((FL_FOR_ARCH6T2 & ~FL_NOTM) | FL_ARCH7) >>> -#define FL_FOR_ARCH7A (FL_FOR_ARCH7 | FL_NOTM | FL_ARCH6K) >>> -#define FL_FOR_ARCH7VE (FL_FOR_ARCH7A | FL_THUMB_DIV | FL_ARM_DIV) >>> -#define FL_FOR_ARCH7R (FL_FOR_ARCH7A | FL_THUMB_DIV) >>> -#define FL_FOR_ARCH7M (FL_FOR_ARCH7 | FL_THUMB_DIV) >>> -#define FL_FOR_ARCH7EM (FL_FOR_ARCH7M | FL_ARCH7EM) >>> -#define FL_FOR_ARCH8A (FL_FOR_ARCH7VE | FL_ARCH8) >>> +#define FL_FOR_ARCH2 FL_NOTM >>> +#define FL_FOR_ARCH3 (FL_FOR_ARCH2 | FL_MODE32) >>> +#define FL_FOR_ARCH3M (FL_FOR_ARCH3 | FL_ARCH3M) >>> +#define FL_FOR_ARCH4 (FL_FOR_ARCH3M | FL_ARCH4) >>> +#define FL_FOR_ARCH4T (FL_FOR_ARCH4 | FL_THUMB) >>> +#define FL_FOR_ARCH5 (FL_FOR_ARCH4 | FL_ARCH5) >>> +#define FL_FOR_ARCH5T (FL_FOR_ARCH5 | FL_THUMB) >>> +#define FL_FOR_ARCH5E (FL_FOR_ARCH5 | FL_ARCH5E) >>> +#define FL_FOR_ARCH5TE (FL_FOR_ARCH5E | FL_THUMB) >>> +#define FL_FOR_ARCH5TEJFL_FOR_ARCH5TE >> This one looks misindented. >> Ok with that fixed once the prerequisites are approved. > It is in the patch but not in the result. If you remove the + in the patch for > the last two lines you'll see that they are perfectly aligned. Ah ok, thanks. The patch is ok then once the prerequisites are approved. Kyrill > Best regards, > > Thomas > -
Re: [patch] Fix type merging deficiency during WPA
> It is fine to merge decls across static and external flags, but I am not
> sure this is a safe solution to the problem. In C it is perfectly normal to
> have one decl more specified or with different attributes. Like:
>
> extern int a __attribute__ ((warning("bar"));
>
> int a=7 __attribute__ ((warning("foo")));
>
> which must prevent merging otherwise the warnings will go out wrong way. In
> GCC 6 timeframe we decided to live with duplicated declarations and added
> support for syntactic aliases. Tree merging should be optional WRT
> correctness, so I think bug is in the canonical type calculation:
>
> /* For array types hash the domain bounds and the string flag. */
> if (TREE_CODE (type) == ARRAY_TYPE && TYPE_DOMAIN (type))
> {
> hstate.add_int (TYPE_STRING_FLAG (type));
> /* OMP lowering can introduce error_mark_node in place of
> random local decls in types. */
> if (TYPE_MIN_VALUE (TYPE_DOMAIN (type)) != error_mark_node)
> inchash::add_expr (TYPE_MIN_VALUE (TYPE_DOMAIN (type)), hstate);
> if (TYPE_MAX_VALUE (TYPE_DOMAIN (type)) != error_mark_node)
> inchash::add_expr (TYPE_MAX_VALUE (TYPE_DOMAIN (type)), hstate);
> }
>
> which boils down to:
>
> if (tclass == tcc_declaration)
> {
> /* DECL's have a unique ID */
> hstate.add_wide_int (DECL_UID (t));
> }
>
> (in asd_expr)
>
> It is bit ugly, but I think for static/external/public decls we need to use
> assembler name here (as we can't rely on symtab to be around all the time)
> which will result in unstability WRT symbol renaming and also give false
> positives for static symbols. But even for static symbols it is not
> guaranteed they are not duplicated. It may be possible to use combination
> of assembler name and translation unit that is available from symtab node,
> but not all decls will have it defined.
So something akin to what I initially proposed?
https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00182.html
And, as mentioned in my previous message, I'm not sure that we would be able
to merge all the global types with the unification scheme, it is too strict.
--
Eric Botcazou
[C++ PATCH] Fix ICE with SIZEOF_EXPR in default arg (PR c++/71822)
Hi!
For SIZEOF_EXPR, we rely on cp_fold to fold it.
But, for VEC_INIT_EXPR initialization, we actually just genericize it
without ever folding the expressions, so e.g. if the ctor has default args
and some complicated expressions in there, they will never be cp_folded.
This is the only place that calls cp_genericize_tree other than when the
whole function is genericized, the fix just adds similar folding of the
expression that cp_fold_function does.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/6.2?
2016-07-11 Jakub Jelinek
PR c++/71822
* cp-gimplify.c (cp_gimplify_expr) : Recursively
fold *expr_p before genericizing it.
* g++.dg/template/defarg21.C: New test.
--- gcc/cp/cp-gimplify.c.jj 2016-07-11 11:14:28.0 +0200
+++ gcc/cp/cp-gimplify.c2016-07-11 11:24:30.554083084 +0200
@@ -621,6 +621,8 @@ cp_gimplify_expr (tree *expr_p, gimple_s
init, VEC_INIT_EXPR_VALUE_INIT (*expr_p),
from_array,
tf_warning_or_error);
+ hash_set pset;
+ cp_walk_tree (expr_p, cp_fold_r, &pset, NULL);
cp_genericize_tree (expr_p);
ret = GS_OK;
input_location = loc;
--- gcc/testsuite/g++.dg/template/defarg21.C.jj 2016-07-11 11:32:34.262266398
+0200
+++ gcc/testsuite/g++.dg/template/defarg21.C2016-07-11 11:31:21.0
+0200
@@ -0,0 +1,21 @@
+// PR c++/71822
+// { dg-do compile }
+
+int bar (int);
+
+template
+struct A
+{
+ explicit A (int x = bar (sizeof (T)));
+};
+
+struct B
+{
+ A b[2];
+};
+
+void
+baz ()
+{
+ B b;
+}
Jakub
[PATCH] Fix SLP vectorization ICE (PR tree-optimization/71823)
Hi!
vect_get_vec_defs handles only one or two arguments, for ternary
ops like fma vectorizable_operation calls vect_get_vec_defs on the
first two arguments, and then did
vec_oprnds2.create (1);
vec_oprnds2.quick_push (vect_get_vec_def_for_operand (op2,
stmt));
which is what vect_get_vec_defs does, but only for loop vectorization.
The following patch uses vect_get_vec_defs again so that it does what it
did before for loop vectorization, but additionally handles properly
slp vectorization.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/6.2?
2016-07-11 Jakub Jelinek
PR tree-optimization/71823
* tree-vect-stmts.c (vectorizable_operation): Use vect_get_vec_defs
to get vec_oprnds2 from op2.
* gcc.dg/vect/pr71823.c: New test.
--- gcc/tree-vect-stmts.c.jj2016-07-07 20:40:43.0 +0200
+++ gcc/tree-vect-stmts.c 2016-07-11 12:05:52.501823209 +0200
@@ -5362,11 +5362,8 @@ vectorizable_operation (gimple *stmt, gi
vect_get_vec_defs (op0, NULL_TREE, stmt, &vec_oprnds0, NULL,
slp_node, -1);
if (op_type == ternary_op)
- {
- vec_oprnds2.create (1);
- vec_oprnds2.quick_push (vect_get_vec_def_for_operand (op2,
- stmt));
- }
+ vect_get_vec_defs (op2, NULL_TREE, stmt, &vec_oprnds2, NULL,
+ slp_node, -1);
}
else
{
--- gcc/testsuite/gcc.dg/vect/pr71823.c.jj 2016-07-11 12:07:41.687485519
+0200
+++ gcc/testsuite/gcc.dg/vect/pr71823.c 2016-07-11 12:08:42.449741939 +0200
@@ -0,0 +1,14 @@
+/* PR tree-optimization/71823 */
+/* { dg-do compile } */
+/* { dg-additional-options "-mfma" { target i?86-*-* x86_64-*-* } } */
+
+float a[4], b[4];
+
+int
+main ()
+{
+ int i;
+ for (i = 0; i < 4; ++i)
+b[i] = __builtin_fma (1024.0f, 1024.0f, a[i]);
+ return 0;
+}
Jakub
[C++ PATCH] Fix error recovery in tsubst_baselink (PR c++/71826)
Hi!
Most of the spots in tsubst_baselink that actually access baselink after
it has been assigned lookup_fnfields () test that it is a BASELINK_P, except
one - the BASELINK_OPTYPE update. lookup_fnfields can return
error_mark_node though, perhaps something else too. The patch just follows
what the surrounding code does.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2016-07-11 Jakub Jelinek
PR c++/71826
* pt.c (tsubst_baselink): Only set BASELINK_OPTYPE for BASELINK_P.
* g++.dg/template/pr71826.C: New test.
--- gcc/cp/pt.c.jj 2016-07-11 11:14:28.0 +0200
+++ gcc/cp/pt.c 2016-07-11 12:30:45.939554745 +0200
@@ -13734,7 +13734,8 @@ tsubst_baselink (tree baselink, tree obj
BASELINK_FUNCTIONS (baselink),
template_args);
/* Update the conversion operator type. */
-BASELINK_OPTYPE (baselink) = optype;
+if (BASELINK_P (baselink))
+ BASELINK_OPTYPE (baselink) = optype;
if (!object_type)
object_type = current_class_type;
--- gcc/testsuite/g++.dg/template/pr71826.C.jj 2016-07-11 12:34:51.406568756
+0200
+++ gcc/testsuite/g++.dg/template/pr71826.C 2016-07-11 12:33:35.0
+0200
@@ -0,0 +1,17 @@
+// PR c++/71826
+// { dg-do compile }
+
+template struct A { int i; }; // { dg-message "note" }
+struct B { void i () {} }; // { dg-message "note" }
+template struct C : A , B
+{
+ void f () { i (); } // { dg-error "is ambiguous" }
+};
+
+int
+main ()
+{
+ C c;
+ c.f ();
+ return 0;
+}
Jakub
[PATCH, rs6000, libgcc] Implement temporary solution for __divkc3 and __mulkc3
Hi,
It was recently brought to my attention that glibc needs access to complex
multiply and divide for IEEE-128 floating-point in GCC 6.2 in order to move
ahead with the library implementation work. This patch enables this support
using only target-specific changes to avoid any possible effect on other
targets. This is not the correct long-term approach, and I am working on a
patch that instead makes use of the common infrastructure. The plan is to
use the current patch for GCC 6, and replace it with the other approach in
GCC 7 shortly.
Thus this patch copies the common code for complex multiply and divide out
of libgcc2.c into separate Power-specific files, and specializes it for
the KC type. It adds a couple of straightforward tests to verify that the
approach works. I've tested the code generated for these tests on a POWER9
simulator as well as a POWER8 machine.
Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no regressions.
I've also asked the glibc team to verify that this serves their requirements.
Is this ok for trunk, and for gcc-6-branch after a short burn-in period?
Thanks,
Bill
[libgcc]
2016-07-11 Bill Schmidt
* config/rs6000/_divkc3.c: New.
* config/rs6000/_mulkc3.c: New.
* config/rs6000/quad-float128.h: Define TFtype; declare _mulkc3
and _divkc3.
* config/rs6000/t-float128: Add _mulkc3 and _divkc3 to
fp128_ppc_funcs.
[gcc/testsuite]
2016-07-11 Bill Schmidt
* gcc.target/powerpc/divkc3-1.c: New.
* gcc.target/powerpc/mulkc3-1.c: New.
Index: gcc/testsuite/gcc.target/powerpc/divkc3-1.c
===
--- gcc/testsuite/gcc.target/powerpc/divkc3-1.c (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/divkc3-1.c (working copy)
@@ -0,0 +1,22 @@
+/* { dg-do run { target { powerpc64*-*-* && vsx_hw } } } */
+/* { dg-options "-mfloat128 -mvsx" } */
+
+void abort ();
+
+typedef __complex float __cfloat128 __attribute__((mode(KC)));
+
+__cfloat128 divide (__cfloat128 x, __cfloat128 y)
+{
+ return x / y;
+}
+
+__cfloat128 z, a;
+
+int main ()
+{
+ z = divide (5.0q + 5.0jq, 2.0q + 1.0jq);
+ a = 3.0q + 1.0jq;
+ if (z != a)
+abort ();
+ return 0;
+}
Index: gcc/testsuite/gcc.target/powerpc/mulkc3-1.c
===
--- gcc/testsuite/gcc.target/powerpc/mulkc3-1.c (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/mulkc3-1.c (working copy)
@@ -0,0 +1,22 @@
+/* { dg-do run { target { powerpc64*-*-* && vsx_hw } } } */
+/* { dg-options "-mfloat128 -mvsx" } */
+
+void abort ();
+
+typedef __complex float __cfloat128 __attribute__((mode(KC)));
+
+__cfloat128 multiply (__cfloat128 x, __cfloat128 y)
+{
+ return x * y;
+}
+
+__cfloat128 z, a;
+
+int main ()
+{
+ z = multiply (2.0q + 1.0jq, 3.0q + 1.0jq);
+ a = 5.0q + 5.0jq;
+ if (z != a)
+abort ();
+ return 0;
+}
Index: libgcc/config/rs6000/_divkc3.c
===
--- libgcc/config/rs6000/_divkc3.c (revision 0)
+++ libgcc/config/rs6000/_divkc3.c (working copy)
@@ -0,0 +1,64 @@
+typedef float KFtype __attribute__ ((mode (KF)));
+typedef __complex float KCtype __attribute__ ((mode (KC)));
+
+#define COPYSIGN(x,y) __builtin_copysignq (x, y)
+#define INFINITY __builtin_infq ()
+#define FABS __builtin_fabsq
+#define isnan __builtin_isnan
+#define isinf __builtin_isinf
+#define isfinite __builtin_isfinite
+
+KCtype
+__divkc3 (KFtype a, KFtype b, KFtype c, KFtype d)
+{
+ KFtype denom, ratio, x, y;
+ KCtype res;
+
+ /* ??? We can get better behavior from logarithmic scaling instead of
+ the division. But that would mean starting to link libgcc against
+ libm. We could implement something akin to ldexp/frexp as gcc builtins
+ fairly easily... */
+ if (FABS (c) < FABS (d))
+{
+ ratio = c / d;
+ denom = (c * ratio) + d;
+ x = ((a * ratio) + b) / denom;
+ y = ((b * ratio) - a) / denom;
+}
+ else
+{
+ ratio = d / c;
+ denom = (d * ratio) + c;
+ x = ((b * ratio) + a) / denom;
+ y = (b - (a * ratio)) / denom;
+}
+
+ /* Recover infinities and zeros that computed as NaN+iNaN; the only cases
+ are nonzero/zero, infinite/finite, and finite/infinite. */
+ if (isnan (x) && isnan (y))
+{
+ if (c == 0.0 && d == 0.0 && (!isnan (a) || !isnan (b)))
+ {
+ x = COPYSIGN (INFINITY, c) * a;
+ y = COPYSIGN (INFINITY, c) * b;
+ }
+ else if ((isinf (a) || isinf (b)) && isfinite (c) && isfinite (d))
+ {
+ a = COPYSIGN (isinf (a) ? 1 : 0, a);
+ b = COPYSIGN (isinf (b) ? 1 : 0, b);
+ x = INFINITY * (a * c + b * d);
+ y = INFINITY * (b * c - a * d);
+ }
+ else if ((isinf (c) || isinf (d)) && isfinite (a) && isfinite (b))
+ {
+ c = COPYSIGN (isinf (c) ? 1 : 0, c);
+ d = COPYSIGN (isinf (d) ? 1 : 0, d);
+ x = 0.0 * (a * c + b * d
Implement -Wswitch-fallthrough
The switch fallthrough has been widely considered a design defect in C, a
misfeature or, to use Marshall Cline's definition, evil. The overwhelming
majority of the time you don't want to fall through to the next case, but it is
easy to forget to "break" at the end of the case, making this far too error
prone. Yet GCC (and probably other compilers, too) doesn't have the ability to
warn in this case. A feature request for such warning was opened back in 2002,
but it's mostly been untouched since. But then the [[fallthrough]] attribute
was
approved for C++17 [1], and that's what has got me to do all this.
The following series attempts to implement the fabled -Wswitch-fallthrough
warning. The warning would be quite easy to implement were it not for control
statements, nested scopes, gotos, and such. I took great pains to try to
handle all of that before I succeeded in getting all the pieces in place. So
GCC ought to do the right thing for e.g.:
switch (n)
{
case 1:
if (n > 20)
break;
else if (n > 10)
{
foo (9);
__builtin_fallthrough ();
}
else
foo (8); // warn here
case 2:;
}
Since approximately 3% of fall throughs are desirable, I added a new builtin,
__builtin_fallthrough, that prevents the warning from occurring. It can only
be used in a switch; the compiler will issue an error otherwise. The new C++
attribute can then be implemented in terms of this builtin. There was an idea
floating around about not warning for /* FALLTHRU */ etc. comments but I
rejected this after I'd spent time thinking about it: the preprocessor can't
know that we're in a switch statement so we might reject valid programs, the
"falls through" comments vary wildly, occur even in if-statements, etc. The
warning doesn't warn when the block is empty, e.g. on case 1: case 2: ... The
warning does, inevitably, trigger on Duff's devices. Too bad. I guess I'd
suggest using #pragma GCC warning then. (I think Perl uses the "continue"
keyword for exactly this purpose -- an interesting idea...)
After I'd completed the warning, I kicked off a bootstrap so as to add various
gcc_fallthrough calls. There were a good amount of them, as expected. I also
grepped various FALLTHRU/Falls through/...fall thru.../... comments in config/
and added gcc_fallthroughs to make it easier for people. Wherever I wasn't
sure that the gcc_fallthrough was appropriate, I added an 'XXX' comment. This
must not be relied upon as I don't know most of the compiler code. The same
goes for relevant libraries where I introduced various gomp_fallthrough
macros conditional on __GNUC__ (I'd tried to use a configure check but then
decided I won't put up with all the vagaries of autoconf).
This patch is accompanied by more than 2000 lines of new tests to get the
warning covered though I'm sure folks will come up with other test cases
that I hadn't considered (hi Martin S. ;).
This warning is enabled by default for C/C++. I was more inclined to put this
into -Wall, but our common.opt machinery doesn't seem to allow that (ugh!).
The clang version I have installed doesn't have this warning so I couldn't
compare my implementation with others.
I plan to plunge into the C++ [[fallthrough]] thingie after this core part is
dealt with.
Since the patch is too large, I split it into various parts, the core and then
various __builtin_fallthrough additions.
This patch has been tested on powerpc64le-unknown-linux-gnu, aarch64-linux-gnu,
x86_64-redhat-linux, and also on powerpc-ibm-aix7.1.0.0, though due to PR71816
I was unable to bootstrap.
Thanks,
[1] http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0188r0.pdf
Marek
[PATCH, i386]: Hoist common code in x86_64{,_zext}_immediate_operand predicates
No functional changes.
2016-07-11 Uros Bizjak
* config/i386/predicates.md (x86_64_immediate_operand) :
Hoist common subexpressions.
(x86_64_zext_immediate_operand) : Ditto.
Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
Committed to mainline SVN.
Uros.
diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md
index 2c4cfe6..6854c37 100644
--- a/gcc/config/i386/predicates.md
+++ b/gcc/config/i386/predicates.md
@@ -205,7 +205,11 @@
return false;
if (!CONST_INT_P (op2))
return false;
+
offset = trunc_int_for_mode (INTVAL (op2), DImode);
+ if (trunc_int_for_mode (offset, SImode) != offset)
+ return false;
+
switch (GET_CODE (op1))
{
case SYMBOL_REF:
@@ -224,16 +228,14 @@
if ((ix86_cmodel == CM_SMALL
|| (ix86_cmodel == CM_MEDIUM
&& !SYMBOL_REF_FAR_ADDR_P (op1)))
- && offset < 16*1024*1024
- && trunc_int_for_mode (offset, SImode) == offset)
+ && offset < 16*1024*1024)
return true;
/* For CM_KERNEL we know that all object resist in the
negative half of 32bits address space. We may not
accept negative offsets, since they may be just off
and we may accept pretty large positive ones. */
if (ix86_cmodel == CM_KERNEL
- && offset > 0
- && trunc_int_for_mode (offset, SImode) == offset)
+ && offset > 0)
return true;
break;
@@ -241,12 +243,10 @@
/* These conditions are similar to SYMBOL_REF ones, just the
constraints for code models differ. */
if ((ix86_cmodel == CM_SMALL || ix86_cmodel == CM_MEDIUM)
- && offset < 16*1024*1024
- && trunc_int_for_mode (offset, SImode) == offset)
+ && offset < 16*1024*1024)
return true;
if (ix86_cmodel == CM_KERNEL
- && offset > 0
- && trunc_int_for_mode (offset, SImode) == offset)
+ && offset > 0)
return true;
break;
@@ -255,8 +255,7 @@
{
case UNSPEC_DTPOFF:
case UNSPEC_NTPOFF:
- if (trunc_int_for_mode (offset, SImode) == offset)
- return true;
+ return true;
}
break;
@@ -307,9 +306,17 @@
{
rtx op1 = XEXP (XEXP (op, 0), 0);
rtx op2 = XEXP (XEXP (op, 0), 1);
+ HOST_WIDE_INT offset;
if (ix86_cmodel == CM_LARGE)
return false;
+ if (!CONST_INT_P (op2))
+ return false;
+
+ offset = trunc_int_for_mode (INTVAL (op2), DImode);
+ if (trunc_int_for_mode (offset, SImode) != offset)
+ return false;
+
switch (GET_CODE (op1))
{
case SYMBOL_REF:
@@ -328,9 +335,7 @@
if ((ix86_cmodel == CM_SMALL
|| (ix86_cmodel == CM_MEDIUM
&& !SYMBOL_REF_FAR_ADDR_P (op1)))
- && CONST_INT_P (op2)
- && trunc_int_for_mode (INTVAL (op2), DImode) > -0x1
- && trunc_int_for_mode (INTVAL (op2), SImode) == INTVAL (op2))
+ && offset > -0x1)
return true;
/* ??? For the kernel, we may accept adjustment of
-0x1000, since we know that it will just convert
@@ -342,9 +347,7 @@
/* These conditions are similar to SYMBOL_REF ones, just the
constraints for code models differ. */
if ((ix86_cmodel == CM_SMALL || ix86_cmodel == CM_MEDIUM)
- && CONST_INT_P (op2)
- && trunc_int_for_mode (INTVAL (op2), DImode) > -0x1
- && trunc_int_for_mode (INTVAL (op2), SImode) == INTVAL (op2))
+ && offset > -0x1)
return true;
break;
Re: Implement -Wswitch-fallthrough: core
2016-07-11 Marek Polacek
PR c/7652
* builtins.c (expand_builtin): Handle BUILT_IN_FALLTHROUGH.
* builtins.def: Add BUILT_IN_FALLTHROUGH.
* common.opt (Wswitch-fallthrough): New option.
* doc/extend.texi: Document __builtin_fallthrough.
* doc/invoke.texi: Document -Wswitch-fallthrough.
* gimple-low.c: Include "diagnostic-core.h" and "gimple-walk.h".
(purge_builtin_fallthrough_r): New function.
(purge_builtin_fallthrough): Likewise.
(lower_stmt): Call purge_builtin_fallthrough.
* gimplify.c (struct label_entry): New struct.
(find_label_entry): New function.
(last_stmt_in_scope): Likewise.
(warn_switch_fallthrough_r): Likewise.
(maybe_warn_switch_fallthrough): Likewise.
(gimplify_switch_expr): Call maybe_warn_switch_fallthrough.
(gimplify_label_expr): New function.
(gimplify_case_label_expr): Set location.
(gimplify_expr): Call gimplify_label_expr.
* system.h (gcc_fallthrough): New macro.
* c-c++-common/Wswitch-fallthrough-1.c: New test.
* c-c++-common/Wswitch-fallthrough-10.c: New test.
* c-c++-common/Wswitch-fallthrough-11.c: New test.
* c-c++-common/Wswitch-fallthrough-12.c: New test.
* c-c++-common/Wswitch-fallthrough-2.c: New test.
* c-c++-common/Wswitch-fallthrough-3.c: New test.
* c-c++-common/Wswitch-fallthrough-4.c: New test.
* c-c++-common/Wswitch-fallthrough-5.c: New test.
* c-c++-common/Wswitch-fallthrough-6.c: New test.
* c-c++-common/Wswitch-fallthrough-7.c: New test.
* c-c++-common/Wswitch-fallthrough-8.c: New test.
* c-c++-common/Wswitch-fallthrough-9.c: New test.
* c-c++-common/builtin-fallthrough-1.c: New test.
* c-c++-common/builtin-fallthrough-2.c: New test.
diff --git gcc/gcc/builtins.c gcc/gcc/builtins.c
index 1465c60..c462bc5 100644
--- gcc/gcc/builtins.c
+++ gcc/gcc/builtins.c
@@ -6885,6 +6889,11 @@ expand_builtin (tree exp, rtx target, rtx subtarget,
machine_mode mode,
folding. */
break;
+case BUILT_IN_FALLTHROUGH:
+ /* We should have gotten rid of this during gimplification. */
+ error ("%Kinvalid use of %<__builtin_fallthrough ()%>", exp);
+ return const0_rtx;
+
default: /* just do library call, if unknown builtin */
break;
}
diff --git gcc/gcc/builtins.def gcc/gcc/builtins.def
index 33a7626..c75f022 100644
--- gcc/gcc/builtins.def
+++ gcc/gcc/builtins.def
@@ -851,6 +851,7 @@ DEF_GCC_BUILTIN(BUILT_IN_VA_ARG_PACK,
"va_arg_pack", BT_FN_INT, ATTR_PUR
DEF_GCC_BUILTIN(BUILT_IN_VA_ARG_PACK_LEN, "va_arg_pack_len",
BT_FN_INT, ATTR_PURE_NOTHROW_LEAF_LIST)
DEF_EXT_LIB_BUILTIN(BUILT_IN__EXIT, "_exit", BT_FN_VOID_INT,
ATTR_NORETURN_NOTHROW_LEAF_LIST)
DEF_C99_BUILTIN(BUILT_IN__EXIT2, "_Exit", BT_FN_VOID_INT,
ATTR_NORETURN_NOTHROW_LEAF_LIST)
+DEF_GCC_BUILTIN (BUILT_IN_FALLTHROUGH, "fallthrough",
BT_FN_VOID, ATTR_NOTHROW_LEAF_LIST)
/* Implementing nested functions. */
DEF_BUILTIN_STUB (BUILT_IN_INIT_TRAMPOLINE, "__builtin_init_trampoline")
diff --git gcc/gcc/common.opt gcc/gcc/common.opt
index a7c5125..741da8d 100644
--- gcc/gcc/common.opt
+++ gcc/gcc/common.opt
@@ -707,6 +707,10 @@ Wsuggest-final-methods
Common Var(warn_suggest_final_methods) Warning
Warn about C++ virtual methods where adding final keyword would improve code
quality.
+Wswitch-fallthrough
+Common Var(warn_switch_fallthrough) Warning Init(1)
+Warn when a switch case falls through.
+
Wswitch-unreachable
Common Var(warn_switch_unreachable) Warning Init(1)
Warn about statements between switch's controlling expression and the first
diff --git gcc/gcc/doc/extend.texi gcc/gcc/doc/extend.texi
index 5b9e617..5fc129d 100644
--- gcc/gcc/doc/extend.texi
+++ gcc/gcc/doc/extend.texi
@@ -11127,6 +11127,25 @@ if (__builtin_expect (ptr != NULL, 1))
when testing pointer or floating-point values.
@end deftypefn
+@deftypefn {Built-in Function} void __builtin_fallthrough (void)
+When the @option{-Wswitch-fallthrough} command-line options is enabled, the
+compiler warns when a switch case statement falls through. To suppress
+this warning in a case where the fall through is desirable, it is possible
+to use this function as in the below:
+
+@smallexample
+switch (cond)
+ @{
+ case 1:
+bar (0);
+/* FALLTHRU */
+__builtin_fallthrough ();
+ default:
+@dots{}
+ @}
+@end smallexample
+@end deftypefn
+
@deftypefn {Built-in Function} void __builtin_trap (void)
This function causes the program to exit abnormally. GCC implements
this function by using a target-dependent mechanism (such as
diff --git gcc/gcc/doc/invoke.texi gcc/gcc/doc/invoke.texi
index ba44951..0244643 100644
--- gcc/gcc/doc/invoke.texi
+++ gcc/gcc/doc/invoke.texi
@@ -300,7 +300,7 @@ Objective-C and Objective-C++ Dialects}.
-Wsuggest-final-types @gol -Wsuggest-final-metho
Re: Implement -Wswitch-fallthrough: c/
2016-07-11 Marek Polacek PR c/7652 * c-array-notation.c (expand_array_notations): Add gcc_fallthrough. * c-decl.c (pop_scope): Likewise. (grokdeclarator): Likewise. (get_parm_info): Likewise. * c-objc-common.c (c_tree_printer): Likewise. * c-parser.c (c_parser_external_declaration): Likewise. (c_parser_declspecs): Likewise. (c_parser_statement_after_labels): Likewise. (c_parser_postfix_expression): Likewise. (c_parser_check_literal_zero): Likewise. (c_parser_omp_variable_list): Likewise. (c_parser_omp_for_loop): Likewise. * c-typeck.c (composite_type): Likewise. (build_unary_op): Likewise. (c_mark_addressable): Likewise. (build_binary_op): Likewise. (c_finish_omp_clauses): Likewise. diff --git gcc/gcc/c/c-array-notation.c gcc/gcc/c/c-array-notation.c index c7cf66a..cf91b21 100644 --- gcc/gcc/c/c-array-notation.c +++ gcc/gcc/c/c-array-notation.c @@ -1303,6 +1303,7 @@ expand_array_notations (tree *tp, int *walk_subtrees, void *) A[x:y]; Replace those with just void zero node. */ *tp = void_node; + gcc_fallthrough (); default: break; } diff --git gcc/gcc/c/c-decl.c gcc/gcc/c/c-decl.c index 8b966fe..d3d3bda 100644 --- gcc/gcc/c/c-decl.c +++ gcc/gcc/c/c-decl.c @@ -1284,6 +1284,7 @@ pop_scope (void) } /* Fall through. */ + gcc_fallthrough (); case TYPE_DECL: case CONST_DECL: common_symbol: @@ -1329,6 +1330,7 @@ pop_scope (void) } /* Fall through. */ + gcc_fallthrough (); /* Parameters go in DECL_ARGUMENTS, not BLOCK_VARS, and have already been put there by store_parm_decls. Unused- parameter warnings are handled by function.c. @@ -5564,6 +5566,7 @@ grokdeclarator (const struct c_declarator *declarator, case cdk_array: loc = decl->id_loc; /* FALL THRU. */ + gcc_fallthrough (); case cdk_function: case cdk_pointer: @@ -7250,6 +7253,7 @@ get_parm_info (bool ellipsis, tree expr) DECL_CHAIN (decl) = others; others = decl; /* fall through */ + gcc_fallthrough (); case ERROR_MARK: set_shadowed: diff --git gcc/gcc/c/c-objc-common.c gcc/gcc/c/c-objc-common.c index 20dc024..21c0e2c 100644 --- gcc/gcc/c/c-objc-common.c +++ gcc/gcc/c/c-objc-common.c @@ -112,6 +112,7 @@ c_tree_printer (pretty_printer *pp, text_info *text, const char *spec, } } /* FALLTHRU */ + gcc_fallthrough (); case 'F': if (DECL_NAME (t)) diff --git gcc/gcc/c/c-parser.c gcc/gcc/c/c-parser.c index 1a50dea..0a33728 100644 --- gcc/gcc/c/c-parser.c +++ gcc/gcc/c/c-parser.c @@ -1546,6 +1546,7 @@ c_parser_external_declaration (c_parser *parser) } /* Else fall through, and yield a syntax error trying to parse as a declaration or function definition. */ + gcc_fallthrough (); default: decl_or_fndef: /* A declaration or a function definition (or, in Objective-C, @@ -2517,6 +2518,7 @@ c_parser_declspecs (c_parser *parser, struct c_declspecs *specs, if (!auto_type_ok) goto out; /* Fall through. */ + gcc_fallthrough (); case RID_UNSIGNED: case RID_LONG: case RID_SHORT: @@ -5339,6 +5341,7 @@ c_parser_statement_after_labels (c_parser *parser, bool *if_p, default: expr_stmt: stmt = c_finish_expr_stmt (loc, c_parser_expression_conv (parser).value); + gcc_fallthrough (); expect_semicolon: c_parser_skip_until_found (parser, CPP_SEMICOLON, "expected %<;%>"); break; @@ -8174,6 +8177,7 @@ c_parser_postfix_expression (c_parser *parser) break; } /* Else fall through to report error. */ + gcc_fallthrough (); default: c_parser_error (parser, "expected expression"); expr.set_error (); @@ -8578,6 +8582,7 @@ c_parser_check_literal_zero (c_parser *parser, unsigned *literal_zero_mask, && (c_parser_peek_2nd_token (parser)->type == CPP_COMMA || c_parser_peek_2nd_token (parser)->type == CPP_CLOSE_PAREN)) *literal_zero_mask |= 1U << idx; + gcc_fallthrough (); default: break; } @@ -10672,6 +10677,7 @@ c_parser_omp_variable_list (c_parser *parser, break; } /* FALLTHROUGH */ + gcc_fallthrough (); case OMP_CLAUSE_MAP: case OMP_CLAUSE_FROM: case OMP_CLAUSE_TO: @@ -10693,6 +10699,7 @@ c_parser_omp_variable_list (c_parser *parser, t = build_component_ref (op_loc, t, ident, comp_loc); } /* FALLTHROUGH */ + gcc_fallthrough (); case OMP_CLAUSE_DEPEND: case OMP_CLAUSE_REDUCTION: w
Re: Implement -Wswitch-fallthrough: c-family/
2016-07-11 Marek Polacek
PR c/7652
* c-ada-spec.c (print_ada_macros): Add gcc_fallthrough.
(to_ada_name): Likewise.
(dump_generic_ada_node): Likewise.
(dump_nested_type): Likewise.
* c-common.c (warn_if_unused_value): Likewise.
(sizeof_pointer_memaccess_warning): Likewise.
(c_common_truthvalue_conversion): Likewise.
(handle_tm_attribute): Likewise.
(c_cpp_error): Likewise.
(resolve_overloaded_builtin): Likewise.
(scalar_to_vector): Likewise.
* c-gimplify.c (c_gimplify_expr): Likewise.
* c-lex.c (c_lex_with_flags): Likewise.
(lex_string): Likewise.
* c-omp.c (c_finish_omp_for): Likewise.
* c-opts.c (c_common_handle_option): Likewise.
* c-pragma.c (handle_pragma_pack): Likewise.
* c-pretty-print.c (c_pretty_printer::postfix_expression): Likewise.
* cilk.c (extract_free_variables): Likewise.
diff --git gcc/gcc/c-family/c-ada-spec.c gcc/gcc/c-family/c-ada-spec.c
index e33fdff..04a918f 100644
--- gcc/gcc/c-family/c-ada-spec.c
+++ gcc/gcc/c-family/c-ada-spec.c
@@ -288,6 +288,7 @@ print_ada_macros (pretty_printer *pp, cpp_hashnode
**macros, int max_ada_macros)
break;
}
/* fallthrough */
+ gcc_fallthrough ();
case CPP_RSHIFT:
case CPP_COMPL:
@@ -1182,6 +1183,7 @@ to_ada_name (const char *name, int *space_found)
if (len2 && s[len2 - 1] == '_')
s[len2++] = 'u';
/* fall through */
+ gcc_fallthrough ();
default:
s[len2++] = name[j];
@@ -1862,6 +1864,8 @@ dump_generic_ada_node (pretty_printer *buffer, tree node,
tree type, int spc,
case TREE_BINFO:
dump_generic_ada_node
(buffer, BINFO_TYPE (node), type, spc, limited_access, name_only);
+ /* XXX Really fallthru? */
+ gcc_fallthrough ();
case TREE_VEC:
pp_string (buffer, "--- unexpected node: TREE_VEC");
@@ -2585,6 +2589,7 @@ dump_nested_type (pretty_printer *buffer, tree field,
tree t, tree parent,
pp_string (buffer, ");");
}
}
+ gcc_fallthrough ();
default:
break;
diff --git gcc/gcc/c-family/c-common.c gcc/gcc/c-family/c-common.c
index 936ddfb..0f5f65a 100644
--- gcc/gcc/c-family/c-common.c
+++ gcc/gcc/c-family/c-common.c
@@ -1506,6 +1506,7 @@ warn_if_unused_value (const_tree exp, location_t locus)
goto restart;
}
/* Fall through. */
+ gcc_fallthrough ();
default:
/* Referencing a volatile value is a side effect, so don't warn. */
@@ -1518,6 +1519,7 @@ warn_if_unused_value (const_tree exp, location_t locus)
but front ends may define such. */
if (EXPRESSION_CLASS_P (exp) && TREE_OPERAND_LENGTH (exp) == 0)
return false;
+ gcc_fallthrough ();
warn:
return warning_at (locus, OPT_Wunused_value, "value computed is not
used");
@@ -1629,6 +1631,7 @@ sizeof_pointer_memaccess_warning (location_t
*sizeof_arg_loc, tree callee,
case BUILT_IN_STRNCASECMP:
cmp = true;
/* FALLTHRU */
+ gcc_fallthrough ();
case BUILT_IN_STRNCPY:
case BUILT_IN_STRNCPY_CHK:
case BUILT_IN_STRNCAT:
@@ -1637,6 +1640,7 @@ sizeof_pointer_memaccess_warning (location_t
*sizeof_arg_loc, tree callee,
case BUILT_IN_STPNCPY_CHK:
strop = true;
/* FALLTHRU */
+ gcc_fallthrough ();
case BUILT_IN_MEMCPY:
case BUILT_IN_MEMCPY_CHK:
case BUILT_IN_MEMMOVE:
@@ -4468,6 +4472,7 @@ c_common_truthvalue_conversion (location_t location, tree
expr)
case FUNCTION_DECL:
expr = build_unary_op (location, ADDR_EXPR, expr, 0);
/* Fall through. */
+ gcc_fallthrough ();
case ADDR_EXPR:
{
@@ -8739,6 +8744,7 @@ handle_tm_attribute (tree *node, tree name, tree args,
if (tm_attr_to_mask (name) & ~(TM_ATTR_SAFE | TM_ATTR_CALLABLE))
goto ignored;
/* FALLTHRU */
+ gcc_fallthrough ();
case FUNCTION_TYPE:
case METHOD_TYPE:
@@ -8782,11 +8788,13 @@ handle_tm_attribute (tree *node, tree name, tree args,
}
}
/* FALLTHRU */
+ gcc_fallthrough ();
default:
/* If a function is next, pass it on to be tried next. */
if (flags & (int) ATTR_FLAG_FUNCTION_NEXT)
return tree_cons (name, args, NULL);
+ gcc_fallthrough ();
ignored:
warning (OPT_Wattributes, "%qE attribute ignored", name);
@@ -10253,6 +10261,7 @@ c_cpp_error (cpp_reader *pfile ATTRIBUTE_UNUSED, int
level, int reason,
return false;
global_dc->dc_warn_system_headers = 1;
/* Fall through. */
+ gcc_fallthrough ();
case CPP_DL_WARNING:
if (flag_no_output)
return false;
@@ -11450,6 +11459,7 @@ resolve_overloaded_builtin (location_t loc, tree
function,
{
fetch_op = false;
Re: Implement -Wswitch-fallthrough: cp/
2016-07-11 Marek Polacek PR c/7652 * call.c (add_builtin_candidate): Add gcc_fallthrough. (add_builtin_candidates): Likewise. (build_integral_nontype_arg_conv): Likewise. (build_new_op_1): Likewise. (convert_like_real): Likewise. * cfns.h (libc_name::hash): Likewise. * class.c (fixed_type_or_null): Likewise. (instantiate_type): Likewise. * constexpr.c (cxx_eval_constant_expression): Likewise. (potential_constant_expression_1): Likewise. * cp-gimplify.c (cp_gimplify_expr): Likewise. (cp_genericize_r): Likewise. (cp_fold): Likewise. * cp-ubsan.c (cp_ubsan_check_member_access_r): Likewise. * cvt.c (build_expr_type_conversion): Likewise. * cxx-pretty-print.c (pp_cxx_unqualified_id): Likewise. (pp_cxx_qualified_id): Likewise. (cxx_pretty_printer::constant): Likewise. (cxx_pretty_printer::primary_expression): Likewise. (cxx_pretty_printer::unary_expression): Likewise. (pp_cxx_pm_expression): Likewise. (cxx_pretty_printer::expression): Likewise. (cxx_pretty_printer::function_specifier): Likewise. (cxx_pretty_printer::declaration_specifiers): Likewise. (pp_cxx_type_specifier_seq): Likewise. (pp_cxx_ptr_operator): Likewise. * decl.c (grokdeclarator): Likewise. * decl2.c (coerce_new_type): Likewise. (coerce_delete_type): Likewise. * dump.c (cp_dump_tree): Likewise. * error.c (dump_type): Likewise. (dump_type_prefix): Likewise. (dump_type_suffix): Likewise. (dump_decl): Likewise. (dump_expr): Likewise. * expr.c (cplus_expand_constant): Likewise. * mangle.c (write_type): Likewise. (write_expression): Likewise. * method.c (synthesized_method_walk): Likewise. * name-lookup.c (begin_scope): Likewise. (arg_assoc_type): Likewise. * parser.c (cp_lexer_print_token): Likewise. (cp_parser_skip_to_end_of_statement): Likewise. (cp_parser_primary_expression): Likewise. (cp_parser_id_expression): Likewise. (cp_parser_unqualified_id): Likewise. (cp_parser_unary_expression): Likewise. (cp_parser_builtin_offsetof): Likewise. (cp_parser_jump_statement): Likewise. (cp_parser_storage_class_specifier_opt): Likewise. (cp_parser_operator): Likewise. (cp_parser_type_specifier): Likewise. (cp_parser_skip_to_end_of_template_parameter_list): Likewise. (cp_parser_cache_defarg): Likewise. (cp_parser_objc_declaration): Likewise. (cp_parser_omp_var_list_no_open): Likewise. (cp_parser_omp_atomic): Likewise. (cp_parser_omp_for_cond): Likewise. * pt.c (check_explicit_specialization): Likewise. (find_parameter_packs_r): Likewise. (coerce_template_template_parm): Likewise. (for_each_template_parm_r): Likewise. (tsubst_aggr_type): Likewise. (tsubst_copy): Likewise. (tsubst_omp_clauses): Likewise. (tsubst_omp_for_iterator): Likewise. (tsubst_expr): Likewise. (tsubst_copy_and_build): Likewise. (unify): Likewise. (value_dependent_expression_p): Likewise. * rtti.c (build_dynamic_cast_1): Likewise. (involves_incomplete_p): Likewise. * gcc/cp/semantics.c (finish_omp_clauses): Likewise. (finish_decltype_type): Likewise. * tree.c (lvalue_kind): Likewise. (strip_typedefs_expr): Likewise. (no_linkage_check): Likewise. (bot_manip): Likewise. * typeck.c (structural_comptypes): Likewise. (is_bitfield_expr_with_lowered_type): Likewise. (cp_build_binary_op): Likewise. (cp_build_addr_expr_1): Likewise. (cxx_mark_addressable): Likewise. (cp_build_modify_expr): Likewise. * typeck2.c (split_nonconstant_init_1): Likewise. diff --git gcc/gcc/cp/call.c gcc/gcc/cp/call.c index d77092b..4e8d89b 100644 --- gcc/gcc/cp/call.c +++ gcc/gcc/cp/call.c @@ -2418,6 +2418,7 @@ add_builtin_candidate (struct z_candidate **candidates, enum tree_code code, case PREDECREMENT_EXPR: if (TREE_CODE (type1) == BOOLEAN_TYPE) return; + gcc_fallthrough (); case POSTINCREMENT_EXPR: case PREINCREMENT_EXPR: if (ARITHMETIC_TYPE_P (type1) || TYPE_PTROB_P (type1)) @@ -2454,6 +2455,7 @@ add_builtin_candidate (struct z_candidate **candidates, enum tree_code code, case UNARY_PLUS_EXPR: /* unary + */ if (TYPE_PTR_P (type1)) break; + gcc_fallthrough (); case NEGATE_EXPR: if (ARITHMETIC_TYPE_P (type1)) break; @@ -2539,6 +2541,8 @@ add_builtin_candidate (struct z_candidate **candidates, enum tree_code code, type2 = ptrdiff_type_node; break; } + /* XXX Really fallthru? */ + gcc_fallthrough (); case MULT
Re: Implement -Wswitch-fallthrough: fortran/
2016-07-11 Marek Polacek
PR c/7652
* arith.c (eval_intrinsic): Add gcc_fallthrough.
* array.c (gfc_ref_dimen_size): Likewise.
(gfc_array_dimen_size): Likewise.
* cpp.c (gfc_cpp_handle_option): Likewise.
(cb_cpp_error): Likewise.
* decl.c (match_implicit_range): Likewise.
(match_attr_spec): Likewise.
* dependency.c (gfc_dep_resolver): Likewise.
* dump-parse-tree.c (show_code_node): Likewise.
* expr.c (simplify_const_ref): Likewise.
* frontend-passes.c (optimize_op): Likewise.
(gfc_expr_walker): Likewise.
(gfc_code_walker): Likewise.
* gfortranspec.c: Likewise.
* interface.c (gfc_check_operator_interface): Likewise.
* io.c (format_lex): Likewise.
* openmp.c (resolve_omp_clauses): Likewise.
* parse.c (next_fixed): Likewise.
(parse_select_block): Likewise.
(parse_select_type_block): Likewise.
(parse_executable): Likewise.
* primary.c (match_boz_constant): Likewise.
(match_arg_list_function): Likewise.
(gfc_match_rvalue): Likewise.
(match_variable): Likewise.
* resolve.c (resolve_operator): Likewise.
(resolve_ref): Likewise.
(fixup_charlen): Likewise.
(resolve_allocate_expr): Likewise.
(gfc_resolve_code): Likewise.
(build_default_init_expr): Likewise.
* target-memory.c (gfc_target_interpret_expr): Likewise.
* trans-array.c (gfc_array_allocate): Likewise.
* trans-expr.c (flatten_array_ctors_without_strlen): Likewise.
(gfc_conv_power_op): Likewise.
(gfc_conv_expr_op): Likewise.
* trans-intrinsic.c (conv_caf_vector_subscript): Likewise.
(conv_intrinsic_cobound): Likewise.
(gfc_conv_intrinsic_len): Likewise.
* trans-io.c (gfc_build_st_parameter): Likewise.
(transfer_expr): Likewise.
* trans-stmt.c (gfc_trans_where_2): Likewise.
diff --git gcc/gcc/fortran/arith.c gcc/gcc/fortran/arith.c
index 47a5504..a56b480 100644
--- gcc/gcc/fortran/arith.c
+++ gcc/gcc/fortran/arith.c
@@ -1504,6 +1504,7 @@ eval_intrinsic (gfc_intrinsic_op op,
}
/* Fall through */
+gcc_fallthrough ();
case INTRINSIC_EQ:
case INTRINSIC_EQ_OS:
case INTRINSIC_NE:
@@ -1523,6 +1524,7 @@ eval_intrinsic (gfc_intrinsic_op op,
/* Fall through */
/* Numeric binary */
+gcc_fallthrough ();
case INTRINSIC_PLUS:
case INTRINSIC_MINUS:
case INTRINSIC_TIMES:
diff --git gcc/gcc/fortran/array.c gcc/gcc/fortran/array.c
index 03c8b17..d020709 100644
--- gcc/gcc/fortran/array.c
+++ gcc/gcc/fortran/array.c
@@ -2323,6 +2323,7 @@ gfc_ref_dimen_size (gfc_array_ref *ar, int dimen, mpz_t
*result, mpz_t *end)
mpz_mul (*end, *end, stride);
mpz_add (*end, *end, lower);
}
+ gcc_fallthrough ();
cleanup:
mpz_clear (upper);
@@ -2432,6 +2433,7 @@ gfc_array_dimen_size (gfc_expr *array, int dimen, mpz_t
*result)
}
/* Fall through */
+ gcc_fallthrough ();
default:
if (array->shape == NULL)
return false;
diff --git gcc/gcc/fortran/cpp.c gcc/gcc/fortran/cpp.c
index 8ac8092..df81cbc 100644
--- gcc/gcc/fortran/cpp.c
+++ gcc/gcc/fortran/cpp.c
@@ -390,6 +390,7 @@ gfc_cpp_handle_option (size_t scode, const char *arg, int
value ATTRIBUTE_UNUSED
case OPT_MM:
gfc_cpp_option.deps_skip_system = 1;
/* fall through */
+ gcc_fallthrough ();
case OPT_M:
gfc_cpp_option.deps = 1;
@@ -398,6 +399,7 @@ gfc_cpp_handle_option (size_t scode, const char *arg, int
value ATTRIBUTE_UNUSED
case OPT_MMD:
gfc_cpp_option.deps_skip_system = 1;
/* fall through */
+ gcc_fallthrough ();
case OPT_MD:
gfc_cpp_option.deps = 1;
@@ -1037,6 +1039,7 @@ cb_cpp_error (cpp_reader *pfile ATTRIBUTE_UNUSED, int
level, int reason,
case CPP_DL_WARNING_SYSHDR:
global_dc->dc_warn_system_headers = 1;
/* Fall through. */
+ gcc_fallthrough ();
case CPP_DL_WARNING:
dlevel = DK_WARNING;
break;
diff --git gcc/gcc/fortran/decl.c gcc/gcc/fortran/decl.c
index 1b62833..3d6b7e4 100644
--- gcc/gcc/fortran/decl.c
+++ gcc/gcc/fortran/decl.c
@@ -3486,6 +3486,7 @@ match_implicit_range (void)
{
case ')':
inner = 0;/* Fall through. */
+ gcc_fallthrough ();
case ',':
c2 = c1;
@@ -3911,6 +3912,7 @@ match_attr_spec (void)
d = DECL_CODIMENSION;
break;
}
+ gcc_fallthrough ();
case 'n':
if (match_string_p ("tiguous"))
{
diff --git gcc/gcc/fortran/dependency.c gcc/gcc/fortran/dependency.c
index f117de0..207825e 100644
--- gcc/gcc/fortran/dependency.c
+++ gcc/gcc/fortran/dependency.c
@@ -2199,6 +2199,7 @@ gfc_dep_resolver (gfc_ref *lref, gfc_ref *r
Re: Implement -Wswitch-fallthrough: gcc/
2016-07-11 Marek Polacek PR c/7652 * Makefile.in (insn-attrtab.o-warn, insn-dfatab.o-warn, insn-latencytab.o-warn, insn-output.o-warn, insn-emit.o-warn): Add -Wno-switch-fallthrough. * alias.c (find_base_value): Add gcc_fallthrough. (find_base_term): Likewise. * asan.c (get_mem_refs_of_builtin_call): Likewise. * auto-inc-dec.c (attempt_change): Likewise. * builtins.c (expand_builtin_int_roundingfn_2): Likewise. (fold_builtin_arith_overflow): Likewise. (expand_builtin): Likewise. * cfgexpand.c (expand_gimple_stmt_1): Likewise. (expand_debug_expr): Likewise. * cfgrtl.c (duplicate_insn_chain): Likewise. * combine.c (find_split_point): Likewise. (expand_compound_operation): Likewise. (make_compound_operation): Likewise. (canon_reg_for_combine): Likewise. (force_to_mode): Likewise. (try_widen_shift_mode): Likewise. (simplify_shift_const_1): Likewise. (simplify_compare_const): Likewise. (simplify_comparison): Likewise. * convert.c (convert_to_real_1): Likewise. (convert_to_integer_1): Likewise. * cse.c (fold_rtx): Likewise. (cse_process_notes_1): Likewise. * cselib.c (cselib_expand_value_rtx_1): Likewise. * dbxout.c (dbxout_expand_expr): Likewise. (dbxout_symbol): Likewise. * df-scan.c (df_notes_rescan): Likewise. (df_uses_record): Likewise. * dojump.c (do_jump): Likewise. * dwarf2cfi.c (output_cfi_directive): Likewise. * dwarf2out.c (output_loc_operands): Likewise. (print_dw_val): Likewise. (copy_dwarf_procs_ref_in_attrs): Likewise. (value_format): Likewise. (mem_loc_descriptor): Likewise. (loc_descriptor): Likewise. (resolve_args_picking_1): Likewise. (loc_list_from_tree_1): Likewise. (add_const_value_attribute): Likewise. (rtl_for_decl_init): Likewise. (add_bound_info): Likewise. (gen_formal_parameter_die): Likewise. (output_macinfo): Likewise. (mark_base_types): Likewise. (hash_loc_operands): Likewise. (compare_loc_operands): Likewise. * expmed.c (expand_divmod): Likewise. (make_tree): Likewise. * expr.c (safe_from_p): Likewise. (expand_expr_addr_expr_1): Likewise. (expand_expr_real_2): Likewise. (expand_expr_real_1): Likewise. * final.c (output_alternate_entry_point): Likewise. (output_addr_const): Likewise. * fold-const.c (negate_expr_p): Likewise. (fold_negate_expr): Likewise. (const_binop): Likewise. (fold_convert_loc): Likewise. (operand_equal_p): Likewise. (fold_truth_not_expr): Likewise. (make_range_step): Likewise. (merge_ranges): Likewise. (fold_cond_expr_with_comparison): Likewise. (extract_muldiv_1): Likewise. (fold_binary_loc): Likewise. (contains_label_1): Likewise. (multiple_of_p): Likewise. * gcc.c (driver_handle_option): Likewise. (do_spec_1): Likewise. * gcov-tool.c (process_args): Likewise. * gcse.c (want_to_gcse_p): Likewise. * genattrtab.c (check_attr_test): Likewise. (check_attr_value): Likewise. (make_canonical): Likewise. (write_test_expr): Likewise. * genconfig.c (walk_insn_part): Likewise. * gengtype-parse.c (direct_declarator): Likewise. * gengtype.c (dbgprint_count_type_at): Likewise. * genmodes.c: Likewise. * genpreds.c (validate_exp): Likewise. (needs_variable): Likewise. * genrecog.c (find_operand): Likewise. (find_matching_operand): Likewise. (match_pattern_2): Likewise. * gensupport.c (process_rtx): Likewise. (get_alternatives_number): Likewise. (collect_insn_data): Likewise. (alter_predicate_for_insn): Likewise. (subst_dup): Likewise. (read_md_rtx): Likewise. * gimple-fold.c (gimple_fold_builtin_fputs): Likewise. (fold_stmt_1): Likewise. (and_comparisons_1): Likewise. (or_comparisons_1): Likewise. (fold_const_aggregate_ref_1): Likewise. * gimple-pretty-print.c (dump_gimple_assign): Likewise. * gimple-ssa-backprop.c (backprop::process_assign_use): Likewise. * gimple-ssa-strength-reduction.c (find_candidates_dom_walker::before_dom_children): Likewise. * gimple-streamer-in.c (input_gimple_stmt): Likewise. * gimple-streamer-out.c (output_gimple_stmt): Likewise. * gimple-walk.c (walk_gimple_stmt): Likewise. * gimple.c (gimple_copy): Likewise. * gimplify.c (warn_switch_unreachable_r): Likewise. (gimple_boolify): Likewise. (gimplify_modify_expr_rhs): Likewise. (gimplify_addr_expr): Likewise. (omp_de
Re: Implement -Wswitch-fallthrough: aarch64 + arm
2016-07-11 Marek Polacek
PR c/7652
* config/aarch64/aarch64-builtins.c (aarch64_simd_expand_args): Add
gcc_fallthrough.
* config/aarch64/aarch64-simd.md: Likewise.
* config/aarch64/aarch64.c (aarch64_expand_mov_immediate): Likewise.
(aarch64_print_operand): Likewise.
(aarch64_rtx_costs): Likewise.
(aarch64_expand_compare_and_swap): Likewise.
(aarch64_gen_atomic_ldop): Likewise.
(aarch64_split_atomic_op): Likewise.
(aarch64_expand_vec_perm_const): Likewise.
* config/aarch64/predicates.md: Likewise.
* config/arm/arm-builtins.c (arm_expand_neon_args): Likewise.
* config/arm/arm.c (const_ok_for_op): Likewise.
(arm_rtx_costs_1): Likewise.
(thumb1_size_rtx_costs): Likewise.
(arm_size_rtx_costs): Likewise.
(arm_new_rtx_costs): Likewise.
(thumb2_reorg): Likewise.
(output_move_double): Likewise.
(output_move_neon): Likewise.
(arm_print_operand): Likewise.
(arm_expand_compare_and_swap): Likewise.
(arm_split_atomic_op): Likewise.
(arm_expand_vec_perm_const): Likewise.
* config/arm/neon.md: Likewise.
diff --git gcc/gcc/config/aarch64/aarch64-builtins.c
gcc/gcc/config/aarch64/aarch64-builtins.c
index 6b90b2a..fe37ea2 100644
--- gcc/gcc/config/aarch64/aarch64-builtins.c
+++ gcc/gcc/config/aarch64/aarch64-builtins.c
@@ -999,6 +999,7 @@ aarch64_simd_expand_args (rtx target, int icode, int
have_retval,
}
/* Fall through - if the lane index isn't a constant then
the next case will error. */
+ gcc_fallthrough ();
case SIMD_ARG_CONSTANT:
constant_arg:
if (!(*insn_data[icode].operand[opc].predicate)
diff --git gcc/gcc/config/aarch64/aarch64-simd.md
gcc/gcc/config/aarch64/aarch64-simd.md
index a19d171..110a070 100644
--- gcc/gcc/config/aarch64/aarch64-simd.md
+++ gcc/gcc/config/aarch64/aarch64-simd.md
@@ -2328,6 +2328,7 @@
if (operands[5] == CONST0_RTX (mode))
break;
/* Fall through, as may need to load into register. */
+ gcc_fallthrough ();
default:
if (!REG_P (operands[5]))
operands[5] = force_reg (mode, operands[5]);
@@ -2430,6 +2431,7 @@
break;
}
/* Fall through. */
+ gcc_fallthrough ();
default:
if (!REG_P (operands[5]))
operands[5] = force_reg (mode, operands[5]);
@@ -2441,6 +2443,7 @@
case UNLT:
inverse = 1;
/* Fall through. */
+ gcc_fallthrough ();
case GE:
case UNGE:
case ORDERED:
@@ -2452,6 +2455,7 @@
case UNLE:
inverse = 1;
/* Fall through. */
+ gcc_fallthrough ();
case GT:
case UNGT:
base_comparison = gen_aarch64_cmgt;
@@ -2545,6 +2549,7 @@
Swapping the operands to BSL will give the UNORDERED case. */
swap_bsl_operands = 1;
/* Fall through. */
+ gcc_fallthrough ();
case ORDERED:
emit_insn (gen_aarch64_cmgt (tmp, operands[4], operands[5]));
emit_insn (gen_aarch64_cmge (mask, operands[5], operands[4]));
diff --git gcc/gcc/config/aarch64/aarch64.c gcc/gcc/config/aarch64/aarch64.c
index 512ef10..3ecf244 100644
--- gcc/gcc/config/aarch64/aarch64.c
+++ gcc/gcc/config/aarch64/aarch64.c
@@ -1833,6 +1833,7 @@ aarch64_expand_mov_immediate (rtx dest, rtx imm)
return;
}
/* FALLTHRU */
+ gcc_fallthrough ();
case SYMBOL_SMALL_ABSOLUTE:
case SYMBOL_TINY_ABSOLUTE:
@@ -4541,6 +4542,7 @@ aarch64_print_operand (FILE *f, rtx x, int code)
break;
}
/* Fall through. */
+ gcc_fallthrough ();
default:
output_operand_lossage ("Unsupported operand for code '%c'", code);
@@ -4713,6 +4715,7 @@ aarch64_print_operand (FILE *f, rtx x, int code)
}
/* Fall through */
+ gcc_fallthrough ();
case 0:
/* Print a normal operand, if it's a general register, then we
@@ -6192,6 +6195,7 @@ aarch64_rtx_costs (rtx x, machine_mode mode, int outer
ATTRIBUTE_UNUSED,
*cost += rtx_cost (SUBREG_REG (op0), VOIDmode, SET, 0, speed);
/* Fall through. */
+ gcc_fallthrough ();
case REG:
/* The cost is one per vector-register copied. */
if (VECTOR_MODE_P (GET_MODE (op0)) && REG_P (op1))
@@ -6685,6 +6689,7 @@ cost_plus:
return true;
}
/* Fall through. */
+gcc_fallthrough ();
case XOR:
case AND:
cost_logic:
@@ -7081,6 +7086,7 @@ cost_plus:
}
/* Fall-through. */
+gcc_fallthrough ();
case UMOD:
if (speed)
{
@@ -7356,6 +7362,7 @@ cost_plus:
}
/* Fall through. */
+ gcc_fallthrough ();
default:
break;
}
@@ -11422,6 +11429,7 @@ aarch64_expand_compare_and_swap (rtx operands[])
rval = gen_reg_rtx (SImode);
o
Re: Implement -Wswitch-fallthrough: go
2016-07-11 Marek Polacek
PR c/7652
* go-system.h (go_fallthrough): New macro.
* gofrontend/escape.cc (Escape_analysis_assign::statement): Use it.
(Escape_analysis_assign::assign): Likewise.
* gofrontend/expressions.cc
(Binary_expression::do_get_backend): Likewise.
* gofrontend/lex.cc (Lex::next_token): Likewise.
diff --git gcc/gcc/go/go-system.h gcc/gcc/go/go-system.h
index cb7e745..e1b6a70 100644
--- gcc/gcc/go/go-system.h
+++ gcc/gcc/go/go-system.h
@@ -138,4 +138,7 @@ struct hash
// When using gcc, go_unreachable is just gcc_unreachable.
#define go_unreachable() gcc_unreachable()
+// When using gcc, go_fallthrough is just gcc_fallthrough.
+#define go_fallthrough() gcc_fallthrough()
+
#endif // !defined(GO_SYSTEM_H)
diff --git gcc/gcc/go/gofrontend/escape.cc gcc/gcc/go/gofrontend/escape.cc
index 7a55818..f3d09e5 100644
--- gcc/gcc/go/gofrontend/escape.cc
+++ gcc/gcc/go/gofrontend/escape.cc
@@ -729,6 +729,7 @@ Escape_analysis_assign::statement(Block*, size_t*,
Statement* s)
if (this->context_->loop_depth() == 1)
break;
// fallthrough
+ go_fallthrough ();
case Statement::STATEMENT_GO:
{
@@ -1483,6 +1484,7 @@ Escape_analysis_assign::assign(Node* dst, Node* src)
// A non-pointer can't escape from a struct.
if (!e->type()->has_pointer())
break;
+ go_fallthrough ();
}
case Expression::EXPRESSION_CONVERSION:
diff --git gcc/gcc/go/gofrontend/expressions.cc
gcc/gcc/go/gofrontend/expressions.cc
index 5f7e4c9..a13c997 100644
--- gcc/gcc/go/gofrontend/expressions.cc
+++ gcc/gcc/go/gofrontend/expressions.cc
@@ -5745,6 +5745,7 @@ Binary_expression::do_get_backend(Translate_context*
context)
case OPERATOR_DIV:
if (left_type->float_type() != NULL || left_type->complex_type() != NULL)
break;
+ go_fallthrough ();
case OPERATOR_MOD:
is_idiv_op = true;
break;
@@ -5754,6 +5755,7 @@ Binary_expression::do_get_backend(Translate_context*
context)
break;
case OPERATOR_BITCLEAR:
this->right_ = Expression::make_unary(OPERATOR_XOR, this->right_, loc);
+ go_fallthrough ();
case OPERATOR_AND:
break;
default:
diff --git gcc/gcc/go/gofrontend/lex.cc gcc/gcc/go/gofrontend/lex.cc
index 34a0811..4defad5 100644
--- gcc/gcc/go/gofrontend/lex.cc
+++ gcc/gcc/go/gofrontend/lex.cc
@@ -674,6 +674,7 @@ Lex::next_token()
}
}
// Fall through.
+ go_fallthrough ();
case '|':
case '=':
case '!':
Re: Implement -Wswitch-fallthrough: i386
2016-07-11 Marek Polacek PR c/7652 * config/i386/driver-i386.c (decode_caches_intel): Likewise. (detect_caches_cpuid4): Likewise. * config/i386/i386-c.c (ix86_target_macros_internal): Likewise. * config/i386/i386.c (function_arg_advance_32): Likewise. (function_arg_32): Likewise. (ix86_gimplify_va_arg): Likewise. (standard_sse_constant_opcode): Likewise. (ix86_decompose_address): Likewise. (ix86_legitimate_constant_p): Likewise. (legitimate_pic_operand_p): Likewise. (legitimate_pic_address_disp_p): Likewise. (ix86_legitimate_address_p): Likewise. (output_pic_addr_const): Likewise. (print_reg): Likewise. (ix86_print_operand): Likewise. (ix86_expand_move): Likewise. (ix86_build_const_vector): Likewise. (ix86_match_ccmode): Likewise. (ix86_expand_branch): Likewise. (ix86_prepare_sse_fp_compare_args): Likewise. (ix86_expand_int_sse_cmp): Likewise. (ix86_adjust_cost): Likewise. (ix86_sched_init_global): Likewise. (ix86_expand_multi_arg_builtin): Likewise. (ix86_expand_args_builtin): Likewise. (ix86_expand_round_builtin): Likewise. (ix86_expand_special_args_builtin): Likewise. (ix86_expand_builtin): Likewise. (avx_vpermilp_parallel): Likewise. (ix86_rtx_costs): Likewise. (ix86_expand_vector_init_duplicate): Likewise. (ix86_expand_vector_init_one_nonzero): Likewise. (ix86_expand_vector_init_one_var): Likewise. (ix86_expand_vector_init_interleave): Likewise. (ix86_expand_vector_init_general): Likewise. (ix86_expand_vector_extract): Likewise. (expand_vec_perm_blend): Likewise. (canonicalize_perm): Likewise. (ix86_expand_vecop_qihi): Likewise. (ix86_preferred_simd_mode): Likewise. * config/i386/sse.md: Likewise. diff --git gcc/gcc/config/i386/driver-i386.c gcc/gcc/config/i386/driver-i386.c index 22a8f28..a84a326 100644 --- gcc/gcc/config/i386/driver-i386.c +++ gcc/gcc/config/i386/driver-i386.c @@ -238,6 +238,7 @@ decode_caches_intel (unsigned reg, bool xeon_mp, break; case 0x87: level2->sizekb = 1024; level2->assoc = 8; level2->line = 64; + gcc_fallthrough (); default: break; @@ -326,6 +327,7 @@ detect_caches_cpuid4 (struct cache_desc *level1, struct cache_desc *level2, * cache->line * sets) / 1024; } } + gcc_fallthrough (); default: break; } diff --git gcc/gcc/config/i386/i386-c.c gcc/gcc/config/i386/i386-c.c index f93a09d..8e5087d 100644 --- gcc/gcc/config/i386/i386-c.c +++ gcc/gcc/config/i386/i386-c.c @@ -210,6 +210,7 @@ ix86_target_macros_internal (HOST_WIDE_INT isa_flag, case '3': def_or_undef (parse_in, "__tune_pentium3__"); /* FALLTHRU */ + gcc_fallthrough (); case '2': def_or_undef (parse_in, "__tune_pentium2__"); break; diff --git gcc/gcc/config/i386/i386.c gcc/gcc/config/i386/i386.c index 9eaf414..f7e9845 100644 --- gcc/gcc/config/i386/i386.c +++ gcc/gcc/config/i386/i386.c @@ -9284,6 +9284,7 @@ function_arg_advance_32 (CUMULATIVE_ARGS *cum, machine_mode mode, if (bytes < 0) break; /* FALLTHRU */ + gcc_fallthrough (); case DImode: case SImode: @@ -9312,12 +9313,14 @@ pass_in_reg: error_p = 1; if (cum->float_in_sse < 2) break; + gcc_fallthrough (); case SFmode: if (cum->float_in_sse == -1) error_p = 1; if (cum->float_in_sse < 1) break; /* FALLTHRU */ + gcc_fallthrough (); case V8SFmode: case V8SImode: @@ -9547,6 +9550,7 @@ function_arg_32 (CUMULATIVE_ARGS *cum, machine_mode mode, if (bytes < 0) break; /* FALLTHRU */ + gcc_fallthrough (); case DImode: case SImode: case HImode: @@ -9579,12 +9583,14 @@ pass_in_reg: error_p = 1; if (cum->float_in_sse < 2) break; + gcc_fallthrough (); case SFmode: if (cum->float_in_sse == -1) error_p = 1; if (cum->float_in_sse < 1) break; /* FALLTHRU */ + gcc_fallthrough (); case TImode: /* In 32bit, we pass TImode in xmm registers. */ case V16QImode: @@ -10984,6 +10990,7 @@ ix86_gimplify_va_arg (tree valist, tree type, gimple_seq *pre_p, container = NULL; break; } + gcc_fallthrough (); default: container = construct_container (nat_mode, TYPE_MODE (type), @@ -11495,6 +11502,7 @@ standard_sse_constant_opcode (rtx_insn *insn, rtx x) case MODE_V8SF: gcc_assert (TARGET_AVX2); /* FALLTHRU */ + gcc_fallthrough (); case MODE_TI: case MODE_V2DF: case MODE_V4SF: @@ -14911,6 +14919,7 @@ ix86_de
Re: Implement -Wswitch-fallthrough: java
2016-07-11 Marek Polacek
PR c/7652
* expr.c (java_truthvalue_conversion): Add gcc_fallthrough.
(type_assertion_hasher::hash): Likewise.
* jcf-dump.c (print_constant): Likewise.
* jcf-io.c (verify_constant_pool): Likewise.
* typeck.c (promote_type): Likewise.
diff --git gcc/gcc/java/expr.c gcc/gcc/java/expr.c
index 2b856e5..d824b6c 100644
--- gcc/gcc/java/expr.c
+++ gcc/gcc/java/expr.c
@@ -192,6 +192,7 @@ java_truthvalue_conversion (tree expr)
>= TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (expr, 0
return java_truthvalue_conversion (TREE_OPERAND (expr, 0));
/* fall through to default */
+ gcc_fallthrough ();
default:
return fold_build2 (NE_EXPR, boolean_type_node,
@@ -410,11 +411,13 @@ type_assertion_hasher::hash (type_assertion *k_p)
hash = iterative_hash (&TYPE_UID (k_p->op2), sizeof TYPE_UID (k_p->op2),
hash);
/* Fall through. */
+ gcc_fallthrough ();
case JV_ASSERT_IS_INSTANTIABLE:
hash = iterative_hash (&TYPE_UID (k_p->op1), sizeof TYPE_UID (k_p->op1),
hash);
/* Fall through. */
+ gcc_fallthrough ();
case JV_ASSERT_END_OF_TABLE:
break;
diff --git gcc/gcc/java/jcf-dump.c gcc/gcc/java/jcf-dump.c
index 1331b55..d8733ed 100644
--- gcc/gcc/java/jcf-dump.c
+++ gcc/gcc/java/jcf-dump.c
@@ -926,6 +926,8 @@ print_constant (FILE *out, JCF *jcf, int index, int
verbosity)
if (verbosity > 0)
fprintf (out, "Fieldref: %ld=", (long) JPOOL_USHORT2 (jcf, index));
print_constant (out, jcf, JPOOL_USHORT2 (jcf, index), 0);
+ /* XXX Really fallthru? */
+ gcc_fallthrough ();
case 5:
case 6:
case 7:
diff --git gcc/gcc/java/jcf-io.c gcc/gcc/java/jcf-io.c
index a560db7..91d9048 100644
--- gcc/gcc/java/jcf-io.c
+++ gcc/gcc/java/jcf-io.c
@@ -472,6 +472,7 @@ verify_constant_pool (JCF *jcf)
|| JPOOL_TAG (jcf, n) != CONSTANT_Utf8)
return i;
/* ... fall through ... */
+ gcc_fallthrough ();
case CONSTANT_Class:
case CONSTANT_String:
n = JPOOL_USHORT1 (jcf, i);
diff --git gcc/gcc/java/typeck.c gcc/gcc/java/typeck.c
index d2e3db6..f7c5da4 100644
--- gcc/gcc/java/typeck.c
+++ gcc/gcc/java/typeck.c
@@ -332,6 +332,7 @@ promote_type (tree type)
case INTEGER_TYPE:
if (type == char_type_node)
return promoted_char_type_node;
+ gcc_fallthrough ();
handle_int:
if (TYPE_PRECISION (type) < TYPE_PRECISION (int_type_node))
{
@@ -342,6 +343,7 @@ promote_type (tree type)
return int_type_node;
}
/* ... else fall through ... */
+ gcc_fallthrough ();
default:
return type;
}
Re: Implement -Wswitch-fallthrough: libatomic
2016-07-11 Marek Polacek PR c/7652 * libatomic_i.h (libat_fallthrough): New macro. * gcas.c (libat_compare_exchange): Use libat_fallthrough. * gexch.c (libat_exchange): Likewise. * glfree.c (libat_is_lock_free): Likewise. * gload.c (libat_load): Likewise. * gstore.c (libat_store): Likewise. diff --git gcc/libatomic/gcas.c gcc/libatomic/gcas.c index 6a5025d..224df8e 100644 --- gcc/libatomic/gcas.c +++ gcc/libatomic/gcas.c @@ -92,8 +92,8 @@ libat_compare_exchange (size_t n, void *mptr, void *eptr, void *dptr, case 8:EXACT(8); goto L16; case 16: EXACT(16); break; -case 3: L4:LARGER(4); /* FALLTHRU */ -case 5 ... 7: L8: LARGER(8); /* FALLTHRU */ +case 3: L4:LARGER(4); /* FALLTHRU */ libat_fallthrough (); +case 5 ... 7: L8: LARGER(8); /* FALLTHRU */ libat_fallthrough (); case 9 ... 15: L16:LARGER(16); break; Lsucc: diff --git gcc/libatomic/gexch.c gcc/libatomic/gexch.c index fddb794..876ae87 100644 --- gcc/libatomic/gexch.c +++ gcc/libatomic/gexch.c @@ -116,8 +116,8 @@ libat_exchange (size_t n, void *mptr, void *vptr, void *rptr, int smodel) case 8:EXACT(8); goto L16; case 16: EXACT(16); break; -case 3: L4:LARGER(4); /* FALLTHRU */ -case 5 ... 7: L8: LARGER(8); /* FALLTHRU */ +case 3: L4:LARGER(4); /* FALLTHRU */ libat_fallthrough (); +case 5 ... 7: L8: LARGER(8); /* FALLTHRU */ libat_fallthrough (); case 9 ... 15: L16:LARGER(16); break; Lfinish: diff --git gcc/libatomic/glfree.c gcc/libatomic/glfree.c index fd1b3a7..3bb09de 100644 --- gcc/libatomic/glfree.c +++ gcc/libatomic/glfree.c @@ -56,8 +56,8 @@ libat_is_lock_free (size_t n, void *ptr) case 8:EXACT(8); goto L16; case 16: EXACT(16); break; -case 3: L4:LARGER(4); /* FALLTHRU */ -case 5 ... 7: L8: LARGER(8); /* FALLTHRU */ +case 3: L4:LARGER(4); /* FALLTHRU */ libat_fallthrough (); +case 5 ... 7: L8: LARGER(8); /* FALLTHRU */ libat_fallthrough (); case 9 ... 15: L16:LARGER(16); break; } diff --git gcc/libatomic/gload.c gcc/libatomic/gload.c index 64a0dc5..b4cc61c 100644 --- gcc/libatomic/gload.c +++ gcc/libatomic/gload.c @@ -79,8 +79,8 @@ libat_load (size_t n, void *mptr, void *rptr, int smodel) case 8:EXACT(8); goto L16; case 16: EXACT(16); break; -case 3: L4:LARGER(4); /* FALLTHRU */ -case 5 ... 7: L8: LARGER(8); /* FALLTHRU */ +case 3: L4:LARGER(4); /* FALLTHRU */ libat_fallthrough (); +case 5 ... 7: L8: LARGER(8); /* FALLTHRU */ libat_fallthrough (); case 9 ... 15: L16:LARGER(16); break; Lfinish: diff --git gcc/libatomic/gstore.c gcc/libatomic/gstore.c index af1d060..90a7430 100644 --- gcc/libatomic/gstore.c +++ gcc/libatomic/gstore.c @@ -91,8 +91,8 @@ libat_store (size_t n, void *mptr, void *vptr, int smodel) case 8:EXACT(8); goto L16; case 16: EXACT(16); break; -case 3: L4:LARGER(4); /* FALLTHRU */ -case 5 ... 7: L8: LARGER(8); /* FALLTHRU */ +case 3: L4:LARGER(4); /* FALLTHRU */ libat_fallthrough (); +case 5 ... 7: L8: LARGER(8); /* FALLTHRU */ libat_fallthrough (); case 9 ... 15: L16:LARGER(16); break; } diff --git gcc/libatomic/libatomic_i.h gcc/libatomic/libatomic_i.h index 9206a8e..0faa103 100644 --- gcc/libatomic/libatomic_i.h +++ gcc/libatomic/libatomic_i.h @@ -200,6 +200,12 @@ void libat_unlock_n (void *ptr, size_t n); # define DECLARE_1(RET,NAME,ARGS) RET C2(libat_,NAME) ARGS MAN(NAME) #endif +#if __GNUC__ >= 7 +# define libat_fallthrough() __builtin_fallthrough () +#else +# define libat_fallthrough() +#endif + /* Prefix to use when calling internal, possibly ifunc'ed functions. */ #if HAVE_IFUNC # define local_ ifunc_
Re: Implement -Wswitch-fallthrough: libcpp
2016-07-11 Marek Polacek
PR c/7652
* internal.h (CPP_FALLTHRU): Define.
* expr.c (_cpp_parse_expr): Use CPP_FALLTHRU.
* lex.c (search_line_fast): Likewise.
(lex_raw_string): Likewise.
(_cpp_lex_direct): Likewise.
(cpp_token_val_index): Likewise.
* macro.c (parse_params): Likewise.
* pch.c (write_macdef): Likewise.
(count_defs): Likewise.
(write_defs): Likewise.
diff --git gcc/libcpp/expr.c gcc/libcpp/expr.c
index 5cdca6f..361e157 100644
--- gcc/libcpp/expr.c
+++ gcc/libcpp/expr.c
@@ -1311,6 +1311,7 @@ _cpp_parse_expr (cpp_reader *pfile, bool is_if)
pfile->state.skip_eval++;
else
pfile->state.skip_eval--;
+ CPP_FALLTHRU ();
default:
break;
}
diff --git gcc/libcpp/internal.h gcc/libcpp/internal.h
index ca2b498..32aba40 100644
--- gcc/libcpp/internal.h
+++ gcc/libcpp/internal.h
@@ -78,6 +78,12 @@ struct cset_converter
efficiency, and partly to limit runaway recursion. */
#define CPP_STACK_MAX 200
+#if __GNUC__ >= 7
+# define CPP_FALLTHRU() __builtin_fallthrough ()
+#else
+# define CPP_FALLTHRU()
+#endif
+
/* Host alignment handling. */
struct dummy
{
diff --git gcc/libcpp/lex.c gcc/libcpp/lex.c
index 236418d..ed884a4 100644
--- gcc/libcpp/lex.c
+++ gcc/libcpp/lex.c
@@ -610,6 +610,7 @@ search_line_fast (const uchar *s, const uchar *end
ATTRIBUTE_UNUSED)
if (l != 0)
break;
s += sizeof(unsigned long);
+ CPP_FALLTHRU ();
case 2:
l = u.l[i++];
if (l != 0)
@@ -1574,6 +1575,7 @@ lex_raw_string (cpp_reader *pfile, cpp_token *token,
const uchar *base,
BUF_APPEND (base, cur - base);
base = cur;
BUF_APPEND ("\\", 1);
+ CPP_FALLTHRU ();
after_backslash:
if (note->type == ' ')
{
@@ -2415,6 +2417,7 @@ _cpp_lex_direct (cpp_reader *pfile)
}
}
/* Fall through. */
+ CPP_FALLTHRU ();
case '_':
case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
@@ -2717,6 +2720,8 @@ _cpp_lex_direct (cpp_reader *pfile)
}
buffer->cur++;
}
+ /* XXX Really fallthru? */
+ CPP_FALLTHRU ();
default:
create_literal (pfile, result, buffer->cur - 1, 1, CPP_OTHER);
@@ -3323,6 +3328,7 @@ cpp_token_val_index (const cpp_token *tok)
else if (tok->type == CPP_PRAGMA)
return CPP_TOKEN_FLD_PRAGMA;
/* else fall through */
+ CPP_FALLTHRU ();
default:
return CPP_TOKEN_FLD_NONE;
}
diff --git gcc/libcpp/macro.c gcc/libcpp/macro.c
index a3b8348..c2c0de4 100644
--- gcc/libcpp/macro.c
+++ gcc/libcpp/macro.c
@@ -2886,6 +2886,7 @@ parse_params (cpp_reader *pfile, cpp_macro *macro)
return true;
/* Fall through to pick up the error. */
+ CPP_FALLTHRU ();
case CPP_COMMA:
if (!prev_ident)
{
@@ -2937,6 +2938,7 @@ parse_params (cpp_reader *pfile, cpp_macro *macro)
if (token->type == CPP_CLOSE_PAREN)
return true;
/* Fall through. */
+ CPP_FALLTHRU ();
case CPP_EOF:
cpp_error (pfile, CPP_DL_ERROR, "missing ')' in macro parameter
list");
diff --git gcc/libcpp/pch.c gcc/libcpp/pch.c
index aa5ed6b..31f8bc5 100644
--- gcc/libcpp/pch.c
+++ gcc/libcpp/pch.c
@@ -55,6 +55,7 @@ write_macdef (cpp_reader *pfile, cpp_hashnode *hn, void
*file_p)
case NT_VOID:
if (! (hn->flags & NODE_POISONED))
return 1;
+ CPP_FALLTHRU ();
case NT_MACRO:
if ((hn->flags & NODE_BUILTIN)
@@ -231,6 +232,7 @@ count_defs (cpp_reader *pfile ATTRIBUTE_UNUSED,
cpp_hashnode *hn, void *ss_p)
return 1;
/* else fall through. */
+ CPP_FALLTHRU ();
case NT_VOID:
{
@@ -270,6 +272,7 @@ write_defs (cpp_reader *pfile ATTRIBUTE_UNUSED,
cpp_hashnode *hn, void *ss_p)
return 1;
/* else fall through. */
+ CPP_FALLTHRU ();
case NT_VOID:
{
Re: Implement -Wswitch-fallthrough: libgcc
2016-07-11 Marek Polacek
PR c/7652
* soft-fp/soft-fp.h (_FP_FALLTHRU): Define.
* soft-fp/op-common.h: Use it.
diff --git gcc/libgcc/soft-fp/op-common.h gcc/libgcc/soft-fp/op-common.h
index 080ef0e..6691f50 100644
--- gcc/libgcc/soft-fp/op-common.h
+++ gcc/libgcc/soft-fp/op-common.h
@@ -898,6 +898,7 @@
case _FP_CLS_COMBINE (FP_CLS_NAN, FP_CLS_INF): \
case _FP_CLS_COMBINE (FP_CLS_NAN, FP_CLS_ZERO): \
R##_s = X##_s;\
+ _FP_FALLTHRU (); \
\
case _FP_CLS_COMBINE (FP_CLS_INF, FP_CLS_INF): \
case _FP_CLS_COMBINE (FP_CLS_INF, FP_CLS_NORMAL): \
@@ -911,6 +912,7 @@
case _FP_CLS_COMBINE (FP_CLS_INF, FP_CLS_NAN): \
case _FP_CLS_COMBINE (FP_CLS_ZERO, FP_CLS_NAN): \
R##_s = Y##_s;\
+ _FP_FALLTHRU (); \
\
case _FP_CLS_COMBINE (FP_CLS_NORMAL, FP_CLS_INF): \
case _FP_CLS_COMBINE (FP_CLS_NORMAL, FP_CLS_ZERO): \
@@ -1198,6 +1200,7 @@
\
case _FP_CLS_COMBINE (FP_CLS_NORMAL, FP_CLS_ZERO): \
FP_SET_EXCEPTION (FP_EX_DIVZERO); \
+ _FP_FALLTHRU (); \
case _FP_CLS_COMBINE (FP_CLS_INF, FP_CLS_ZERO): \
case _FP_CLS_COMBINE (FP_CLS_INF, FP_CLS_NORMAL): \
R##_c = FP_CLS_INF; \
diff --git gcc/libgcc/soft-fp/soft-fp.h gcc/libgcc/soft-fp/soft-fp.h
index 3b39336..999b145 100644
--- gcc/libgcc/soft-fp/soft-fp.h
+++ gcc/libgcc/soft-fp/soft-fp.h
@@ -71,6 +71,12 @@
[!!sizeof (struct { int __error_if_negative: (expr) ? 2 : -1; })]
#endif
+#if (defined __GNUC__ && __GNUC__ >= 7)
+# define _FP_FALLTHRU() __builtin_fallthrough ()
+#else
+# define _FP_FALLTHRU()
+#endif
+
/* In the Linux kernel, some architectures have a single function that
uses different kinds of unpacking and packing depending on the
instruction being emulated, meaning it is not readily visible to
Re: Implement -Wswitch-fallthrough: libgomp
2016-07-11 Marek Polacek PR c/7652 * libgomp.h (gomp_fallthrough): Define. * oacc-init.c (resolve_device): Use gomp_fallthrough. * testsuite/libgomp.c++/cancel-parallel-2.C: Add __builtin_fallthrough. * testsuite/libgomp.c/cancel-parallel-2.c: Add __builtin_fallthrough. diff --git gcc/libgomp/libgomp.h gcc/libgomp/libgomp.h index 7b2671b..c0c5e60 100644 --- gcc/libgomp/libgomp.h +++ gcc/libgomp/libgomp.h @@ -1080,6 +1080,12 @@ extern int gomp_test_nest_lock_25 (omp_nest_lock_25_t *) __GOMP_NOTHROW; # define ialias_call(fn) fn #endif +#if __GNUC__ >= 7 +# define gomp_fallthrough() __builtin_fallthrough () +#else +# define gomp_fallthrough() +#endif + /* Helper function for priority_node_to_task() and task_to_priority_node(). diff --git gcc/libgomp/oacc-init.c gcc/libgomp/oacc-init.c index f2325ad..70308fc 100644 --- gcc/libgomp/oacc-init.c +++ gcc/libgomp/oacc-init.c @@ -144,6 +144,7 @@ resolve_device (acc_device_t d, bool fail_is_error) d = acc_device_not_host; } /* FALLTHROUGH */ + gomp_fallthrough (); case acc_device_not_host: /* Find the first available device after acc_device_not_host. */ diff --git gcc/libgomp/testsuite/libgomp.c++/cancel-parallel-2.C gcc/libgomp/testsuite/libgomp.c++/cancel-parallel-2.C index 23b8caa..f793f8a 100644 --- gcc/libgomp/testsuite/libgomp.c++/cancel-parallel-2.C +++ gcc/libgomp/testsuite/libgomp.c++/cancel-parallel-2.C @@ -31,6 +31,7 @@ foo (int *x) case 2: usleep (1000); /* FALLTHRU */ + __builtin_fallthrough (); case 1:; #pragma omp cancellation point parallel break; diff --git gcc/libgomp/testsuite/libgomp.c/cancel-parallel-2.c gcc/libgomp/testsuite/libgomp.c/cancel-parallel-2.c index 20adb55..054a96e 100644 --- gcc/libgomp/testsuite/libgomp.c/cancel-parallel-2.c +++ gcc/libgomp/testsuite/libgomp.c/cancel-parallel-2.c @@ -27,6 +27,7 @@ foo (int *x) case 2: usleep (1000); /* FALLTHRU */ + __builtin_fallthrough (); case 1:; #pragma omp cancellation point parallel break;
Re: Implement -Wswitch-fallthrough: libiberty
2016-07-11 Marek Polacek
PR c/7652
* cp-demangle.c (d_print_comp_inner): Use D_FALLTHRU.
(d_print_mod): Likewise.
* cplus-dem.c (demangle_signature): Likewise.
(demangle_fund_type): Likewise.
(do_hpacc_template_const_value): Likewise.
* d-demangle.c (dlang_value): Likewise.
* hashtab.c (iterative_hash): Likewise.
* regex.c (FALLTHRU): Define.
(byte_re_match_2_internal): Use FALLTHRU.
diff --git gcc/libiberty/cp-demangle.c gcc/libiberty/cp-demangle.c
index 09d6469..6bd8e65 100644
--- gcc/libiberty/cp-demangle.c
+++ gcc/libiberty/cp-demangle.c
@@ -4824,6 +4824,7 @@ d_print_comp_inner (struct d_print_info *dpi, int options,
mod_inner = d_left (sub);
}
/* Fall through. */
+ D_FALLTHRU ();
case DEMANGLE_COMPONENT_RESTRICT_THIS:
case DEMANGLE_COMPONENT_VOLATILE_THIS:
@@ -5634,11 +5635,13 @@ d_print_mod (struct d_print_info *dpi, int options,
case DEMANGLE_COMPONENT_REFERENCE_THIS:
/* For the ref-qualifier, put a space before the &. */
d_append_char (dpi, ' ');
+ D_FALLTHRU ();
case DEMANGLE_COMPONENT_REFERENCE:
d_append_char (dpi, '&');
return;
case DEMANGLE_COMPONENT_RVALUE_REFERENCE_THIS:
d_append_char (dpi, ' ');
+ D_FALLTHRU ();
case DEMANGLE_COMPONENT_RVALUE_REFERENCE:
d_append_string (dpi, "&&");
return;
diff --git gcc/libiberty/cplus-dem.c gcc/libiberty/cplus-dem.c
index d04c32a..97aa2c7 100644
--- gcc/libiberty/cplus-dem.c
+++ gcc/libiberty/cplus-dem.c
@@ -1642,6 +1642,7 @@ demangle_signature (struct work_stuff *work,
else
/* fall through */
{;}
+ D_FALLTHRU ();
default:
if (AUTO_DEMANGLING || GNU_DEMANGLING)
@@ -3992,6 +3993,8 @@ demangle_fund_type (struct work_stuff *work,
success = 0;
break;
}
+ /* XXX Really fallthru? */
+ D_FALLTHRU ();
case 'I':
(*mangled)++;
if (**mangled == '_')
@@ -4086,7 +4089,8 @@ do_hpacc_template_const_value (struct work_stuff *work
ATTRIBUTE_UNUSED,
{
case 'N':
string_append (result, "-");
-/* fall through */
+ /* fall through */
+ D_FALLTHRU ();
case 'P':
(*mangled)++;
break;
diff --git gcc/libiberty/d-demangle.c gcc/libiberty/d-demangle.c
index 4ad90a6..e7d39e4 100644
--- gcc/libiberty/d-demangle.c
+++ gcc/libiberty/d-demangle.c
@@ -1244,6 +1244,7 @@ dlang_value (string *decl, const char *mangled, const
char *name, char type)
if (*mangled < '0' || *mangled > '9')
return NULL;
/* Fall through */
+ D_FALLTHRU ();
case '0': case '1': case '2': case '3': case '4':
case '5': case '6': case '7': case '8': case '9':
mangled = dlang_parse_integer (decl, mangled, type);
diff --git gcc/libiberty/hashtab.c gcc/libiberty/hashtab.c
index 04607ea..cd8d8c5 100644
--- gcc/libiberty/hashtab.c
+++ gcc/libiberty/hashtab.c
@@ -962,18 +962,18 @@ iterative_hash (const PTR k_in /* the key */,
c += length;
switch(len) /* all the case statements fall through */
{
-case 11: c+=((hashval_t)k[10]<<24);
-case 10: c+=((hashval_t)k[9]<<16);
-case 9 : c+=((hashval_t)k[8]<<8);
+case 11: c+=((hashval_t)k[10]<<24); D_FALLTHRU ();
+case 10: c+=((hashval_t)k[9]<<16); D_FALLTHRU ();
+case 9 : c+=((hashval_t)k[8]<<8); D_FALLTHRU ();
/* the first byte of c is reserved for the length */
-case 8 : b+=((hashval_t)k[7]<<24);
-case 7 : b+=((hashval_t)k[6]<<16);
-case 6 : b+=((hashval_t)k[5]<<8);
-case 5 : b+=k[4];
-case 4 : a+=((hashval_t)k[3]<<24);
-case 3 : a+=((hashval_t)k[2]<<16);
-case 2 : a+=((hashval_t)k[1]<<8);
-case 1 : a+=k[0];
+case 8 : b+=((hashval_t)k[7]<<24); D_FALLTHRU ();
+case 7 : b+=((hashval_t)k[6]<<16); D_FALLTHRU ();
+case 6 : b+=((hashval_t)k[5]<<8); D_FALLTHRU ();
+case 5 : b+=k[4]; D_FALLTHRU ();
+case 4 : a+=((hashval_t)k[3]<<24); D_FALLTHRU ();
+case 3 : a+=((hashval_t)k[2]<<16); D_FALLTHRU ();
+case 2 : a+=((hashval_t)k[1]<<8); D_FALLTHRU ();
+case 1 : a+=k[0]; D_FALLTHRU ();
/* case 0: nothing left to add */
}
mix(a,b,c);
diff --git gcc/libiberty/regex.c gcc/libiberty/regex.c
index 9ffc3f4..c8e5ea9 100644
--- gcc/libiberty/regex.c
+++ gcc/libiberty/regex.c
@@ -1368,6 +1368,12 @@ static const char *re_error_msgid[] =
#endif /* INSIDE_RECURSION */
+#if __GNUC__ >= 7
+# define FALLTHRU() __builtin_fallthrough ()
+#else
+# define FALLTHRU()
+#endif
+
#ifndef DEFINED_ONCE
/* Avoiding alloca during matching, to placate r_alloc. */
@@ -2493,6 +2499,7 @@ PREFIX(regex_compile) (const char *ARG_PREFIX(pattern),
if ((syntax & RE_BK_PLUS_QM)
|| (syntax & RE_LIMITED_OPS))
goto normal_char;
+ FALLTHRU ();
handle_plus:
case '*':
/* If there is no previous p
Re: Implement -Wswitch-fallthrough: libgo
2016-07-11 Marek Polacek PR c/7652 * runtime/heapdump.c (dumpefacetypes): Add break. diff --git gcc/libgo/runtime/heapdump.c gcc/libgo/runtime/heapdump.c index d0cfb01..1ce8aa2 100644 --- gcc/libgo/runtime/heapdump.c +++ gcc/libgo/runtime/heapdump.c @@ -766,6 +766,7 @@ dumpefacetypes(void *obj __attribute__ ((unused)), uintptr size, const Type *typ for(i = 0; i <= size - type->__size; i += type->__size) //playgcprog(i, (uintptr*)type->gc + 1, dumpeface_callback, obj); break; + break; case TypeInfo_Chan: if(type->__size == 0) // channels may have zero-sized objects in them break;
Re: Implement -Wswitch-fallthrough: libstdc++
2016-07-11 Marek Polacek
PR c/7652
* libsupc++/hash_bytes.cc: Use __builtin_fallthrough.
diff --git gcc/libstdc++-v3/libsupc++/hash_bytes.cc
gcc/libstdc++-v3/libsupc++/hash_bytes.cc
index 2e5bbfa..818331f 100644
--- gcc/libstdc++-v3/libsupc++/hash_bytes.cc
+++ gcc/libstdc++-v3/libsupc++/hash_bytes.cc
@@ -95,8 +95,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{
case 3:
hash ^= static_cast(buf[2]) << 16;
+ __builtin_fallthrough ();
case 2:
hash ^= static_cast(buf[1]) << 8;
+ __builtin_fallthrough ();
case 1:
hash ^= static_cast(buf[0]);
hash *= m;
Re: Implement -Wswitch-fallthrough: lto
2016-07-11 Marek Polacek
PR c/7652
* lto-plugin.c (lto_fallthrough): Define.
(parse_table_entry): Use it.
diff --git gcc/lto-plugin/lto-plugin.c gcc/lto-plugin/lto-plugin.c
index 51afc52..ffdf54a 100644
--- gcc/lto-plugin/lto-plugin.c
+++ gcc/lto-plugin/lto-plugin.c
@@ -77,6 +77,12 @@ along with this program; see the file COPYING3. If not see
# define O_BINARY 0
#endif
+#if __GNUC__ >= 7
+# define lto_fallthrough() __builtin_fallthrough ()
+#else
+# define lto_fallthrough()
+#endif
+
/* Segment name for LTO sections. This is only used for Mach-O.
FIXME: This needs to be kept in sync with darwin.c. */
@@ -254,6 +260,7 @@ parse_table_entry (char *p, struct ld_plugin_symbol *entry,
break;
}
/* FALL-THROUGH. */
+lto_fallthrough ();
case ss_uscore:
entry->name = concat ("_", p, NULL);
break;
Re: Implement -Wswitch-fallthrough: objc
2016-07-11 Marek Polacek
PR c/7652
* objc-encoding.c (encode_type): Add gcc_fallthrough.
diff --git gcc/gcc/objc/objc-encoding.c gcc/gcc/objc/objc-encoding.c
index 41ac6a4..cc64e1b 100644
--- gcc/gcc/objc/objc-encoding.c
+++ gcc/gcc/objc/objc-encoding.c
@@ -622,6 +622,7 @@ encode_type (tree type, int curtype, int format)
}
/* Else, they are encoded exactly like the integer type that is
used by the compiler to store them. */
+ gcc_fallthrough ();
case INTEGER_TYPE:
{
char c;
Re: Implement -Wswitch-fallthrough: other archs
2016-07-11 Marek Polacek PR c/7652 * config/alpha/alpha.c (alpha_rtx_costs): Likewise. (alpha_legitimate_constant_p): Likewise. (alpha_emit_setcc): Likewise. (alpha_emit_xfloating_libcall): Likewise. (alpha_function_value_1): Likewise. (alpha_fold_builtin): Likewise. * config/alpha/predicates.md: Likewise. * config/arc/arc.c (arc_init): Likewise. (arc_print_operand): Likewise. (arc_rtx_costs): Likewise. (arc_legitimate_constant_p): Likewise. (arc_get_insn_variants): Likewise. (arc_ifcvt): Likewise. (arc_expand_atomic_op): Likewise. * config/arc/predicates.md: Likewise. * config/avr/avr-log.c (avr_log_vadump): Likewise. * config/avr/avr.c (ashrqi3_out): Likewise. (ashrhi3_out): Likewise. (avr_out_ashrpsi3): Likewise. (ashrsi3_out): Likewise. (avr_out_lshrpsi3): Likewise. (avr_rtx_costs_1): Likewise. * config/bfin/bfin.c (print_operand): Likewise. (bfin_rtx_costs): Likewise. * config/c6x/c6x.c (c6x_expand_compare): Likewise. (c6x_mem_operand): Likewise. (c6x_legitimate_address_p_1): Likewise. (count_unit_reqs): Likewise. (c6x_registers_update): Likewise. * config/c6x/c6x.h: Likewise. * config/c6x/predicates.md: Likewise. * config/cr16/cr16.c (cr16_address_cost): Likewise. (cr16_print_operand_address): Likewise. * config/cris/cris.c (cris_print_operand): Likewise. (cris_rtx_costs): Likewise. * config/epiphany/epiphany.c (epiphany_rtx_costs): Likewise. (epiphany_address_cost): Likewise. (epiphany_print_operand): Likewise. (epiphany_mode_needed): Likewise. (epiphany_mode_entry_exit): Likewise. (epiphany_insert_mode_switch_use): Likewise. * config/fr30/fr30.c (fr30_print_operand): Likewise. * config/frv/frv.c (frv_print_operand): Likewise. (frv_legitimate_address_p_1): Likewise. (frv_emit_movsi): Likewise. (frv_rtx_costs): Likewise. * config/h8300/h8300.c (h8300_print_operand): Likewise. (get_shift_alg): Likewise. (output_a_shift): Likewise. (compute_a_shift_length): Likewise. (compute_a_shift_cc): Likewise. * config/ia64/ia64.c (ia64_expand_vecint_compare): Likewise. (ia64_expand_atomic_op): Likewise. (ia64_print_operand): Likewise. (ia64_rtx_costs): Likewise. (group_barrier_needed): Likewise. (ia64_expand_vec_perm_const): Likewise. * config/ia64/predicates.md: Likewise. * config/iq2000/iq2000.c (iq2000_address_cost): Likewise. * config/lm32/lm32.c (lm32_rtx_costs): Likewise. * config/m32r/m32r.c (m32r_rtx_costs): Likewise. (m32r_print_operand): Likewise. * config/m68k/m68k.c (m68k_sched_md_init_global): Likewise. * config/m68k/m68k.h: Likewise. * config/mips/mips.c (mips_symbolic_constant_p): Likewise. (mips_symbol_insns_1): Likewise. (mips_const_insns): Likewise. (mips_set_reg_reg_cost): Likewise. (mips_rtx_costs): Likewise. (mips_output_order_conditional_branch): Likewise. (mips_expand_vec_perm_const): Likewise. * config/mmix/predicates.md: Likewise. * config/mn10300/mn10300.c (mn10300_print_operand): Likewise. (mn10300_rtx_costs): Likewise. (extract_bundle): Likewise. * config/mn10300/mn10300.md: Likewise. * config/msp430/msp430.c (msp430_initial_elimination_offset): Likewise. (msp430_print_operand): Likewise. * config/nds32/nds32-cost.c: Likewise. * config/nios2/nios2.c (nios2_legitimate_address_p): Likewise. (nios2_print_operand): Likewise. * config/pa/pa.c (hppa_rtx_costs): Likewise. * config/pdp11/pdp11.c (pdp11_rtx_costs): Likewise. * config/rl78/rl78-real.md: Likewise. * config/rl78/rl78.c (characterize_address): Likewise. (rl78_initial_elimination_offset): Likewise. * config/rx/rx.c (rx_print_operand_address): Likewise. (rx_print_operand): Likewise. (rx_option_override): Likewise. * config/s390/s390.c (s390_select_ccmode): Likewise. (s390_expand_atomic): Likewise. * config/sh/sh.c (sh_print_operand): Likewise. (output_branch): Likewise. (sh_rtx_costs): Likewise. * config/sh/sh.md: Likewise. * config/sparc/sparc.c (sparc_rtx_costs): Likewise. (sparc_emit_membar_for_model): Likewise. * config/spu/spu.c (classify_immediate): Likewise. * config/tilegx/tilegx.c (tilegx_emit_cc_test): Likewise. (tilegx_print_operand): Likewise. * config/tilepro/tilepro.c (tilepro_emit_cc_test): Likewise. (tilepro_print_operand): Likewise. * config/v850/v850.c (v850_print_operand): Likewise. * co
Re: Implement -Wswitch-fallthrough: rs6000
2016-07-11 Marek Polacek
PR c/7652
* config/rs6000/rs6000.c (rs6000_builtin_vectorized_libmass): Likewise.
(rs6000_legitimate_offset_address_p): Likewise.
(rs6000_emit_move): Likewise.
(altivec_expand_ld_builtin): Likewise.
(altivec_expand_st_builtin): Likewise.
(rs6000_emit_vector_compare_inner): Likewise.
(rs6000_adjust_cost): Likewise.
(insn_must_be_first_in_group): Likewise.
(rs6000_handle_altivec_attribute): Likewise.
(rs6000_rtx_costs): Likewise.
(altivec_expand_vec_perm_const): Likewise.
(rtx_is_swappable_p): Likewise.
* config/rs6000/rs6000.md: Likewise.
diff --git gcc/gcc/config/rs6000/rs6000.c gcc/gcc/config/rs6000/rs6000.c
index dd77e1b..a41d6be 100644
--- gcc/gcc/config/rs6000/rs6000.c
+++ gcc/gcc/config/rs6000/rs6000.c
@@ -5459,6 +5459,7 @@ rs6000_builtin_vectorized_libmass (combined_fn fn, tree
type_out,
CASE_CFN_POW:
n_args = 2;
/* fall through */
+ gcc_fallthrough ();
CASE_CFN_ACOS:
CASE_CFN_ACOSH:
@@ -7675,6 +7676,7 @@ rs6000_legitimate_offset_address_p (machine_mode mode,
rtx x,
return (SPE_CONST_OFFSET_OK (offset)
&& SPE_CONST_OFFSET_OK (offset + 8));
/* fall through */
+ gcc_fallthrough ();
case TDmode:
case TImode:
@@ -9778,6 +9780,7 @@ rs6000_emit_move (rtx dest, rtx source, machine_mode mode)
if (FLOAT128_2REG_P (mode))
rs6000_eliminate_indexed_memrefs (operands);
/* fall through */
+ gcc_fallthrough ();
case DFmode:
case DDmode:
@@ -14398,6 +14401,8 @@ altivec_expand_ld_builtin (tree exp, rtx target, bool
*expandedp)
break;
case ALTIVEC_BUILTIN_LD_INTERNAL_2di:
icode = CODE_FOR_vector_altivec_load_v2di;
+ /* XXX Really? */
+ gcc_fallthrough ();
case ALTIVEC_BUILTIN_LD_INTERNAL_1ti:
icode = CODE_FOR_vector_altivec_load_v1ti;
break;
@@ -14459,6 +14464,8 @@ altivec_expand_st_builtin (tree exp, rtx target
ATTRIBUTE_UNUSED,
break;
case ALTIVEC_BUILTIN_ST_INTERNAL_2di:
icode = CODE_FOR_vector_altivec_store_v2di;
+ /* XXX Really? */
+ gcc_fallthrough ();
case ALTIVEC_BUILTIN_ST_INTERNAL_1ti:
icode = CODE_FOR_vector_altivec_store_v1ti;
break;
@@ -20961,6 +20968,7 @@ print_operand (FILE *file, rtx x, int code)
return;
fputc (',', file);
/* FALLTHRU */
+ gcc_fallthrough ();
case 'R':
/* X is a CR register. Print the mask for `mtcrf'. */
@@ -22532,6 +22540,7 @@ rs6000_emit_vector_compare_inner (enum rtx_code code,
rtx op0, rtx op1)
case GE:
if (GET_MODE_CLASS (mode) == MODE_VECTOR_INT)
return NULL_RTX;
+ gcc_fallthrough ();
case EQ:
case GT:
@@ -30191,6 +30200,7 @@ rs6000_adjust_cost (rtx_insn *insn, rtx link, rtx_insn
*dep_insn, int cost)
&& (INSN_CODE (dep_insn) >= 0)
&& (get_attr_type (dep_insn) == TYPE_MFFGPR))
return 2;
+ gcc_fallthrough ();
default:
break;
@@ -30227,6 +30237,7 @@ rs6000_adjust_cost (rtx_insn *insn, rtx link, rtx_insn
*dep_insn, int cost)
}
}
/* Fall through, no cost for output dependency. */
+ gcc_fallthrough ();
case REG_DEP_ANTI:
/* Anti dependency; DEP_INSN reads a register that INSN writes some
@@ -31373,6 +31384,7 @@ insn_must_be_first_in_group (rtx_insn *insn)
case PROCESSOR_POWER5:
if (is_cracked_insn (insn))
return true;
+ gcc_fallthrough ();
case PROCESSOR_POWER4:
if (is_microcoded_insn (insn))
return true;
@@ -32290,6 +32302,7 @@ rs6000_handle_altivec_attribute (tree *node,
case V4SImode: case V8HImode: case V16QImode: case V4SFmode:
case V2DImode: case V2DFmode:
result = type;
+ gcc_fallthrough ();
default: break;
}
break;
@@ -32300,6 +32313,7 @@ rs6000_handle_altivec_attribute (tree *node,
case SImode: case V4SImode: result = bool_V4SI_type_node; break;
case HImode: case V8HImode: result = bool_V8HI_type_node; break;
case QImode: case V16QImode: result = bool_V16QI_type_node;
+ gcc_fallthrough ();
default: break;
}
break;
@@ -32307,6 +32321,7 @@ rs6000_handle_altivec_attribute (tree *node,
switch (mode)
{
case V8HImode: result = pixel_V8HI_type_node;
+ gcc_fallthrough ();
default: break;
}
default: break;
@@ -33912,6 +33927,7 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int
outer_code,
return true;
}
/* FALLTHRU */
+ gcc_fallthrough ();
case CONST_DOUBLE:
case CONST_WIDE_INT:
@@ -33973,6 +33989,7 @@ rs6000_rtx_costs (rtx x, machine_mode mode, int
outer_code,
return false;
}
/* FALLTHRU */
+ gcc_fallthrough ();
case UDIV:
Re: Implement -Wswitch-fallthrough
On Mon, Jul 11, 2016 at 12:43 PM, Marek Polacek wrote:
> The switch fallthrough has been widely considered a design defect in C, a
> misfeature or, to use Marshall Cline's definition, evil. The overwhelming
> majority of the time you don't want to fall through to the next case, but it
> is
> easy to forget to "break" at the end of the case, making this far too error
> prone. Yet GCC (and probably other compilers, too) doesn't have the ability
> to
> warn in this case. A feature request for such warning was opened back in
> 2002,
> but it's mostly been untouched since. But then the [[fallthrough]] attribute
> was
> approved for C++17 [1], and that's what has got me to do all this.
>
> The following series attempts to implement the fabled -Wswitch-fallthrough
> warning. The warning would be quite easy to implement were it not for control
> statements, nested scopes, gotos, and such. I took great pains to try to
> handle all of that before I succeeded in getting all the pieces in place. So
> GCC ought to do the right thing for e.g.:
>
> switch (n)
> {
> case 1:
> if (n > 20)
> break;
> else if (n > 10)
> {
> foo (9);
> __builtin_fallthrough ();
> }
> else
> foo (8); // warn here
> case 2:;
> }
>
> Since approximately 3% of fall throughs are desirable, I added a new builtin,
> __builtin_fallthrough, that prevents the warning from occurring. It can only
> be used in a switch; the compiler will issue an error otherwise. The new C++
> attribute can then be implemented in terms of this builtin. There was an idea
> floating around about not warning for /* FALLTHRU */ etc. comments but I
> rejected this after I'd spent time thinking about it: the preprocessor can't
> know that we're in a switch statement so we might reject valid programs, the
> "falls through" comments vary wildly, occur even in if-statements, etc. The
> warning doesn't warn when the block is empty, e.g. on case 1: case 2: ... The
> warning does, inevitably, trigger on Duff's devices. Too bad. I guess I'd
> suggest using #pragma GCC warning then. (I think Perl uses the "continue"
> keyword for exactly this purpose -- an interesting idea...)
>
> After I'd completed the warning, I kicked off a bootstrap so as to add various
> gcc_fallthrough calls. There were a good amount of them, as expected. I also
> grepped various FALLTHRU/Falls through/...fall thru.../... comments in config/
> and added gcc_fallthroughs to make it easier for people. Wherever I wasn't
> sure that the gcc_fallthrough was appropriate, I added an 'XXX' comment. This
> must not be relied upon as I don't know most of the compiler code. The same
> goes for relevant libraries where I introduced various gomp_fallthrough
> macros conditional on __GNUC__ (I'd tried to use a configure check but then
> decided I won't put up with all the vagaries of autoconf).
>
> This patch is accompanied by more than 2000 lines of new tests to get the
> warning covered though I'm sure folks will come up with other test cases
> that I hadn't considered (hi Martin S. ;).
>
> This warning is enabled by default for C/C++. I was more inclined to put this
> into -Wall, but our common.opt machinery doesn't seem to allow that (ugh!).
> The clang version I have installed doesn't have this warning so I couldn't
> compare my implementation with others.
I don't like it being default turned on or even part of -Wall. As
evidence all of the patches that comes after this is the main reason
why.
Thanks,
Andrew
>
> I plan to plunge into the C++ [[fallthrough]] thingie after this core part is
> dealt with.
>
> Since the patch is too large, I split it into various parts, the core and then
> various __builtin_fallthrough additions.
>
> This patch has been tested on powerpc64le-unknown-linux-gnu,
> aarch64-linux-gnu,
> x86_64-redhat-linux, and also on powerpc-ibm-aix7.1.0.0, though due to PR71816
> I was unable to bootstrap.
>
> Thanks,
>
> [1] http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0188r0.pdf
>
> Marek
Re: Implement -Wswitch-fallthrough: testsuite
2016-07-11 Marek Polacek
PR c/7652
* c-c++-common/Wswitch-unreachable-1.c: Add __builtin_fallthrough.
* c-c++-common/pr44832.c: Likewise.
* g++.dg/lto/20090128_0.C: Likewise.
* g++.dg/opt/pr14029.C: Likewise.
* g++.dg/opt/pr45412.C: Likewise.
* g++.dg/opt/pr56999.C: Likewise.
* g++.dg/pr48484.C: Likewise.
* g++.dg/pr57662.C: Likewise.
* g++.dg/torture/pr64565.C: Likewise.
* gcc.dg/pr20017.c: Likewise.
* gcc.dg/pr27531-1.c: Likewise.
* gcc.dg/switch-1.c: Likewise.
* gcc.dg/torture/pr64284.c: Likewise.
* c-c++-common/Wmisleading-indentation-2.c: Use -Wno-switch-fallthrough.
* g++.dg/opt/eh5.C: Likewise.
* g++.dg/torture/pr49615.C: Likewise.
* g++.dg/warn/sequence-pt-pr17880.C: Likewise.
* gcc.dg/sequence-pt-pr17880.c: Likewise.
* gcc.dg/Wjump-misses-init-2.c: Likewise.
* gcc.dg/c99-vla-jump-5.c: Likewise.
* gcc.dg/duff-1.c: Likewise.
* gcc.dg/duff-2.c: Likewise.
* gcc.dg/duff-3.c: Likewise.
* gcc.dg/duff-4.c: Likewise.
* gcc.dg/ipa/pr57539.c: Likewise.
* gcc.dg/noncompile/920923-1.c: Likewise.
* gcc.dg/pr42388.c: Likewise.
* gcc.dg/pr45055.c: Likewise.
* gcc.dg/pr52139.c: Likewise.
* gcc.dg/pr53881-2.c: Likewise.
* gcc.dg/pr53887.c: Likewise.
* gcc.dg/pr55150-2.c: Likewise.
* gcc.dg/pr59418.c: Likewise.
* gcc.dg/pr65658.c: Likewise.
* gcc.dg/pr71084.c: Likewise.
* gcc.dg/torture/pr38948.c: Likewise.
* gcc.dg/tree-ssa/20030814-2.c: Likewise.
* gcc.dg/tree-ssa/20030814-3.c: Likewise.
* gcc.dg/tree-ssa/inline-10.c: Likewise.
* gcc.dg/tree-ssa/ssa-dom-thread-9.c: Likewise.
* gcc.dg/tree-ssa/ssa-fre-10.c: Likewise.
* gcc.target/aarch64/pic-symrefplus.c: Likewise.
* gcc.target/i386/avx2-vperm2i128-2.c: Likewise.
* gcc.target/i386/pr46939.c: Likewise.
diff --git gcc/gcc/testsuite/c-c++-common/Wmisleading-indentation-2.c
gcc/gcc/testsuite/c-c++-common/Wmisleading-indentation-2.c
index b4ee700..b00ccc2 100644
--- gcc/gcc/testsuite/c-c++-common/Wmisleading-indentation-2.c
+++ gcc/gcc/testsuite/c-c++-common/Wmisleading-indentation-2.c
@@ -1,4 +1,4 @@
-/* { dg-options "-Wmisleading-indentation" } */
+/* { dg-options "-Wmisleading-indentation -Wno-switch-fallthrough" } */
/* { dg-do compile } */
/* Based on get_attr_athlon_decode from the generated insn-attrtab.c
diff --git gcc/gcc/testsuite/c-c++-common/Wswitch-unreachable-1.c
gcc/gcc/testsuite/c-c++-common/Wswitch-unreachable-1.c
index ee6ecc1..26e085d 100644
--- gcc/gcc/testsuite/c-c++-common/Wswitch-unreachable-1.c
+++ gcc/gcc/testsuite/c-c++-common/Wswitch-unreachable-1.c
@@ -108,6 +108,7 @@ X:
{
L:
j = 16;
+ __builtin_fallthrough ();
default:
if (j < 5)
goto L;
diff --git gcc/gcc/testsuite/c-c++-common/pr44832.c
gcc/gcc/testsuite/c-c++-common/pr44832.c
index b57e525..a8d0b4d 100644
--- gcc/gcc/testsuite/c-c++-common/pr44832.c
+++ gcc/gcc/testsuite/c-c++-common/pr44832.c
@@ -93,6 +93,7 @@ half:
case V16QImode:
if (!((ix86_isa_flags & (1 << 19)) != 0))
break;
+ __builtin_fallthrough ();
case V8HImode:
if (!((ix86_isa_flags & (1 << 17)) != 0))
diff --git gcc/gcc/testsuite/g++.dg/lto/20090128_0.C
gcc/gcc/testsuite/g++.dg/lto/20090128_0.C
index d03cfc6..c639d92 100644
--- gcc/gcc/testsuite/g++.dg/lto/20090128_0.C
+++ gcc/gcc/testsuite/g++.dg/lto/20090128_0.C
@@ -79,6 +79,7 @@ Class1::f3 (const char *src, char *dst, const char *end)
{
case KXYYZ:
*dst = '\0';
+ __builtin_fallthrough ();
case KXFI9:
if (!f2 (p9t42, &src, &dst, end))
;
diff --git gcc/gcc/testsuite/g++.dg/opt/eh5.C gcc/gcc/testsuite/g++.dg/opt/eh5.C
index 3557ab2..92580ac 100644
--- gcc/gcc/testsuite/g++.dg/opt/eh5.C
+++ gcc/gcc/testsuite/g++.dg/opt/eh5.C
@@ -1,6 +1,6 @@
// PR 41377
// { dg-do compile }
-// { dg-options "-O3" }
+// { dg-options "-O3 -Wno-switch-fallthrough" }
struct A
{
diff --git gcc/gcc/testsuite/g++.dg/opt/pr14029.C
gcc/gcc/testsuite/g++.dg/opt/pr14029.C
index 1673edf..5a67b17 100644
--- gcc/gcc/testsuite/g++.dg/opt/pr14029.C
+++ gcc/gcc/testsuite/g++.dg/opt/pr14029.C
@@ -28,6 +28,7 @@ Iterator find_7(Iterator first, Iterator last)
case 1:
if (*first.ptr == 7) return first;
++first;
+__builtin_fallthrough ();
case 0:
default:
return last;
diff --git gcc/gcc/testsuite/g++.dg/opt/pr45412.C
gcc/gcc/testsuite/g++.dg/opt/pr45412.C
index e374f52..4259c7c 100644
--- gcc/gcc/testsuite/g++.dg/opt/pr45412.C
+++ gcc/gcc/testsuite/g++.dg/opt/pr45412.C
@@ -18,6 +18,7 @@ S::vm ()
{
case 0:
bar ();
+ __builtin_fallthrough ();
case 1:
delete this;
}
diff --git gcc/gcc/testsuite/g++.dg/opt/p
Re: Implement -Wswitch-fallthrough
Marek Polacek writes: > > This warning is enabled by default for C/C++. That's purely evil. So have to put your non standard builtin all over standards compliant code just to shut off a warning that is likely common. And you're not even supporting the classic lint style comment either. I think it's ok to make this a separate option, but even putting it in -Wall would annoy far too many people (including me) for good reasons. Making it default is just totally wrong. -Andi
Re: Implement -Wswitch-fallthrough
On Mon, Jul 11, 2016 at 12:59:51PM -0700, Andrew Pinski wrote: > I don't like it being default turned on or even part of -Wall. As > evidence all of the patches that comes after this is the main reason > why. Ok, moving this to -Wextra is rather trivial. Marek
Re: Implement -Wswitch-fallthrough
On Mon, Jul 11, 2016 at 1:01 PM, Marek Polacek wrote: > On Mon, Jul 11, 2016 at 12:59:51PM -0700, Andrew Pinski wrote: >> I don't like it being default turned on or even part of -Wall. As >> evidence all of the patches that comes after this is the main reason >> why. > > Ok, moving this to -Wextra is rather trivial. Even -Wextra is not useful either. If anything maybe -Wc++1z-compat :). > > Marek
Re: Implement -Wswitch-fallthrough
On Mon, Jul 11, 2016 at 01:01:05PM -0700, Andi Kleen wrote: > Marek Polacek writes: > > > > This warning is enabled by default for C/C++. > > That's purely evil. So have to put your non standard builtin all over > standards compliant code just to shut off a warning that is likely > common. And you're not even supporting the classic lint style comment either. I explained why supporting the classic lint style comment wouldn't fly. > I think it's ok to make this a separate option, but even putting it in > -Wall would annoy far too many people (including me) for good reasons. > Making it default is just totally wrong. Ok. As I mentioned elsewhere, moving this to -Wextra (or even out of -Wall/-Wextra) is a one liner change. Marek
Re: [libiberty][PATCH] Avoid zero-length VLAs.
Ping?
(I suspect I should have added a libiberty maintainer to cc in the first place.)
On Mon, Jun 13, 2016 at 9:05 AM, Brooks Moses wrote:
> Zero-length variable-length-arrays are not allowed in standard C99,
> and perhaps more importantly, they cause ASAN to complain. (See,
> e.g., https://gcc.gnu.org/ml/gcc-patches/2013-09/msg00917.html.)
>
> With this patch, the libiberty tests, including demangler-fuzzer, are
> ASAN-clean.
>
> - Brooks
>
>
>
> libiberty/ChangeLog
> --- a/libiberty/ChangeLog
> +++ b/libiberty/ChangeLog
> @@ -1,3 +1,8 @@
> +2016-06-12 Brooks Moses
> +
> + * cp-demangle.c (cplus_demangle_print_callback): Avoid zero-length
> + VLAs.
> +
> 2016-05-31 Alan Modra
>
> * xmemdup.c (xmemdup): Use xmalloc rather than xcalloc.
> libiberty/cp-demangle.c
> --- a/libiberty/cp-demangle.c
> +++ b/libiberty/cp-demangle.c
> @@ -4120,8 +4120,10 @@
>
>{
> #ifdef CP_DYNAMIC_ARRAYS
> -__extension__ struct d_saved_scope scopes[dpi.num_saved_scopes];
> -__extension__ struct d_print_template temps[dpi.num_copy_templates];
> +__extension__ struct d_saved_scope scopes[(dpi.num_saved_scopes > 0)
> + ? dpi.num_saved_scopes : 1];
> +__extension__ struct d_print_template temps[(dpi.num_copy_templates > 0)
> + ? dpi.num_copy_templates : 1];
>
> dpi.saved_scopes = scopes;
> dpi.copy_templates = temps;
[PATCH], PowerPC support to enable -mlra and/or -mfloat128
These configuration switches will allow the PowerPC GCC developers to switch defaults in the compiler to debug the code, before making the decision to flip the default permanently. In the future, when the defaults have been changed, these configuration options would allow developers to go back to the previous versions without modifying the code using the --disable- form. The first option is --enable-lra, which changes the compiler so that the default is to use the LRA register allocator instead of the older RELOAD allocator. The PowerPC team would like to switch the default, but there is a critical bug in LRA that must be fixed before we can change the default: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69847 The second option is --enable-float128, which changes the compiler so that the default for VSX systems is to enable the __float128 keyword to allow users access to the IEEE 128-bit floating point implementation without having to use the keyword. Both of these switches are debug switches, and are not meant to be used by non-developers. The --enable-lra swich causes the following tests to fail: * testsuite/gcc.target/powerpc/bool3-p7.c * testsuite/gcc.target/powerpc/bool3-p8.c See bug 71846 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71846) for more details. The --enable-float128 switch causes libquadmath and libstdc++ to fail because we do not yet have enough of the support in the compiler to allow these libraries to build. It is our intention, that we will use the --enable-float128 option and work on getting the libraries fixed. If I build just a C compiler and disable building libquadmath, there are no regressions in the C tests with __float128 enabled or disabled. Can I check these options into the trunk as non-default options? -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
Re: [PATCH] Fix SLP vectorization ICE (PR tree-optimization/71823)
On July 11, 2016 9:28:43 PM GMT+02:00, Jakub Jelinek wrote:
>Hi!
>
>vect_get_vec_defs handles only one or two arguments, for ternary
>ops like fma vectorizable_operation calls vect_get_vec_defs on the
>first two arguments, and then did
> vec_oprnds2.create (1);
> vec_oprnds2.quick_push (vect_get_vec_def_for_operand (op2,
>stmt));
>which is what vect_get_vec_defs does, but only for loop vectorization.
>The following patch uses vect_get_vec_defs again so that it does what
>it
>did before for loop vectorization, but additionally handles properly
>slp vectorization.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
>trunk/6.2?
OK.
Thanks,
Richard.
>2016-07-11 Jakub Jelinek
>
> PR tree-optimization/71823
> * tree-vect-stmts.c (vectorizable_operation): Use vect_get_vec_defs
> to get vec_oprnds2 from op2.
>
> * gcc.dg/vect/pr71823.c: New test.
>
>--- gcc/tree-vect-stmts.c.jj 2016-07-07 20:40:43.0 +0200
>+++ gcc/tree-vect-stmts.c 2016-07-11 12:05:52.501823209 +0200
>@@ -5362,11 +5362,8 @@ vectorizable_operation (gimple *stmt, gi
> vect_get_vec_defs (op0, NULL_TREE, stmt, &vec_oprnds0, NULL,
> slp_node, -1);
> if (op_type == ternary_op)
>- {
>-vec_oprnds2.create (1);
>-vec_oprnds2.quick_push (vect_get_vec_def_for_operand (op2,
>- stmt));
>- }
>+ vect_get_vec_defs (op2, NULL_TREE, stmt, &vec_oprnds2, NULL,
>+ slp_node, -1);
> }
> else
> {
>--- gcc/testsuite/gcc.dg/vect/pr71823.c.jj 2016-07-11
>12:07:41.687485519 +0200
>+++ gcc/testsuite/gcc.dg/vect/pr71823.c2016-07-11 12:08:42.449741939
>+0200
>@@ -0,0 +1,14 @@
>+/* PR tree-optimization/71823 */
>+/* { dg-do compile } */
>+/* { dg-additional-options "-mfma" { target i?86-*-* x86_64-*-* } } */
>+
>+float a[4], b[4];
>+
>+int
>+main ()
>+{
>+ int i;
>+ for (i = 0; i < 4; ++i)
>+b[i] = __builtin_fma (1024.0f, 1024.0f, a[i]);
>+ return 0;
>+}
>
> Jakub
Re: Implement -Wswitch-fallthrough
> After I'd completed the warning, I kicked off a bootstrap so as to add > various gcc_fallthrough calls. There were a good amount of them, as > expected. I also grepped various FALLTHRU/Falls through/...fall > thru.../... comments in config/ and added gcc_fallthroughs to make it > easier for people. Wherever I wasn't sure that the gcc_fallthrough was > appropriate, I added an 'XXX' comment. This must not be relied upon as I > don't know most of the compiler code. The same goes for relevant libraries > where I introduced various gomp_fallthrough macros conditional on __GNUC__ > (I'd tried to use a configure check but then decided I won't put up with > all the vagaries of autoconf). Do we really want to clutter up the entire tree like that? The result is particularly ugly IMO. Just add -Wno-switch-fallthrough somewhere I'd say. -- Eric Botcazou
[C++ PATCH] Fix lval {REAL,IMAG}PART_EXPR constexpr evaluation (PR c++/71828)
Hi!
REALPART_EXPR and IMAGPART_EXPR are handled like unary expressions, even
though they are references. For !lval that makes no difference, but for
lval it means we can get ADDR_EXPR of INTEGER_CST etc., or trying to store
into an INTEGER_CST.
Fixed by doing roughly what we do for other references like COMPONENT_REF,
ARRAY_REF etc. in that case.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/6.2?
2016-07-11 Jakub Jelinek
PR c++/71828
* constexpr.c (cxx_eval_constant_expression) :
For lval don't use cxx_eval_unary_expression and instead recurse
and if needed rebuild the reference.
* g++.dg/cpp0x/constexpr-71828.C: New test.
--- gcc/cp/constexpr.c.jj 2016-07-11 11:14:28.0 +0200
+++ gcc/cp/constexpr.c 2016-07-11 13:30:17.333065119 +0200
@@ -3790,6 +3790,19 @@ cxx_eval_constant_expression (const cons
case REALPART_EXPR:
case IMAGPART_EXPR:
+ if (lval)
+ {
+ r = cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 0), lval,
+ non_constant_p, overflow_p);
+ if (r == error_mark_node)
+ ;
+ else if (r == TREE_OPERAND (t, 0))
+ r = t;
+ else
+ r = fold_build1 (TREE_CODE (t), TREE_TYPE (t), r);
+ break;
+ }
+ /* FALLTHRU */
case CONJ_EXPR:
case FIX_TRUNC_EXPR:
case FLOAT_EXPR:
--- gcc/testsuite/g++.dg/cpp0x/constexpr-71828.C.jj 2016-07-11
13:39:31.635423827 +0200
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-71828.C2016-07-11
13:39:02.0 +0200
@@ -0,0 +1,5 @@
+// PR c++/71828
+// { dg-do compile { target c++11 } }
+
+constexpr _Complex int a { 1, 2 };
+static_assert (& __imag a != &__real a, "");
Jakub
Re: Implement -Wswitch-fallthrough
On Mon, Jul 11, 2016 at 10:08:02PM +0200, Eric Botcazou wrote: > > After I'd completed the warning, I kicked off a bootstrap so as to add > > various gcc_fallthrough calls. There were a good amount of them, as > > expected. I also grepped various FALLTHRU/Falls through/...fall > > thru.../... comments in config/ and added gcc_fallthroughs to make it > > easier for people. Wherever I wasn't sure that the gcc_fallthrough was > > appropriate, I added an 'XXX' comment. This must not be relied upon as I > > don't know most of the compiler code. The same goes for relevant libraries > > where I introduced various gomp_fallthrough macros conditional on __GNUC__ > > (I'd tried to use a configure check but then decided I won't put up with > > all the vagaries of autoconf). > > Do we really want to clutter up the entire tree like that? The result is > particularly ugly IMO. Just add -Wno-switch-fallthrough somewhere I'd say. Well, we already have the /* FALLTHRU */ etc. comments in most of the places. Perhaps if we replace those comments just with GCC_FALLTHRU, a macro that expands to __builtin_fallthrough (); for GCC >= 7, or to [[fallthrough]] for C++17, or nothing? IMHO it doesn't make sense to keep both the gomp_fallthrough (); macro and /* FALLTHROUGH */ etc. comments in the source. Jakub
[C++ PATCH] Fix diagnostics ICE (PR c++/71835)
Hi!
add_conv_candidate creates cand->fn which is not a FUNCTION_DECL, but
some type, all spots in convert_like_real assume that if fn is non-NULL, it
is some decl. Just a couple of lines above this hunk we even have a comment
reminding us on cand->fn not having to be a decl:
/* Since cand->fn will be a type, not a function, for a conversion
function, we must be careful not to unconditionally look at
DECL_NAME here. */
So, this patch uses just convert_like instead of convert_like_with_context
if cand->fn is not a decl (or shall I test for TREE_CODE (cand->fn) ==
FUNCTION_DECL instead?).
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/6.2?
2016-07-11 Jakub Jelinek
PR c++/71835
* call.c (build_op_call_1): Use convert_like_with_context only
if cand->fn is a decl.
* g++.dg/conversion/ambig3.C: New test.
--- gcc/cp/call.c.jj2016-07-11 11:14:28.0 +0200
+++ gcc/cp/call.c 2016-07-11 14:42:44.466186449 +0200
@@ -4406,8 +4406,11 @@ build_op_call_1 (tree obj, vecconvs[0], obj, cand->fn, -1,
- complain);
+ if (DECL_P (cand->fn))
+ obj = convert_like_with_context (cand->convs[0], obj, cand->fn,
+-1, complain);
+ else
+ obj = convert_like (cand->convs[0], obj, complain);
obj = convert_from_reference (obj);
result = cp_build_function_call_vec (obj, args, complain);
}
--- gcc/testsuite/g++.dg/conversion/ambig3.C.jj 2016-07-11 14:47:26.668826075
+0200
+++ gcc/testsuite/g++.dg/conversion/ambig3.C2016-07-11 14:46:49.0
+0200
@@ -0,0 +1,13 @@
+// PR c++/71835
+// { dg-do compile }
+
+typedef void T (int);
+struct A { operator T * (); }; // { dg-message "candidate" }
+struct B { operator T * (); }; // { dg-message "candidate" }
+struct C : A, B {} c;
+
+void
+foo ()
+{
+ c (0); // { dg-error "is ambiguous" }
+}
Jakub
Re: [C++ PATCH] Fix ICE with PTRMEM_CST (PR c++/70869)
On Thu, Jul 07, 2016 at 03:06:55PM -0400, Jason Merrill wrote:
> On Thu, Jul 7, 2016 at 2:23 PM, Jakub Jelinek wrote:
> > On Thu, Jul 07, 2016 at 12:32:02PM -0400, Jason Merrill wrote:
> >> Hmm, I wonder if walk_tree_1 should walk into DECL_EXPR like it does into
> >> BIND_EXPR_VARS. But your patch is OK.
> >
> > Well, walk_tree_1 does walk into DECL_EXPR, but cp_genericize_r says
> > *walk_subtrees on the VAR_DECL inside of it.
> > When walking BIND_EXPR_VARS, it doesn't walk the vars themselves, but
> > /* Walk the DECL_INITIAL and DECL_SIZE. We don't want to walk
> >into declarations that are just mentioned, rather than
> >declared; they don't really belong to this part of the tree.
> >And, we can see cycles: the initializer for a declaration
> >can refer to the declaration itself. */
> > WALK_SUBTREE (DECL_INITIAL (decl));
> > WALK_SUBTREE (DECL_SIZE (decl));
> > WALK_SUBTREE (DECL_SIZE_UNIT (decl));
> > Do you mean walk_tree_1 should walk DECL_INITIAL/DECL_SIZE/DECL_SIZE_UNIT
> > of the var mentioned in the DECL_EXPR? Then for many vars (which are both
> > mentioned in BIND_EXPR_VARS and in DECL_EXPR) it would walk them twice.
>
> Yes, that's what I meant. Or perhaps since this is a C++ FE issue,
> cp_walk_subtrees should walk those fields for artificial variables.
I've already committed the patch, given your "But your patch is OK." above.
But the following works too, bootstrapped/regtested on x86_64-linux and
i686-linux, ok for trunk?
If yes, do you want the combined diff from both patches on the 6.2 branch
too, or just the earlier patch?
2016-07-11 Jakub Jelinek
PR c++/70869
PR c++/71054
* cp-gimplify.c (cp_genericize_r): Revert the 2016-07-07 change.
* tree.c (cp_walk_subtrees): For DECL_EXPR on DECL_ARTIFICIAL
non-static VAR_DECL, walk the decl's DECL_INITIAL, DECL_SIZE and
DECL_SIZE_UNIT.
--- gcc/cp/cp-gimplify.c.jj 2016-07-11 11:24:30.554083084 +0200
+++ gcc/cp/cp-gimplify.c2016-07-11 15:21:30.459546129 +0200
@@ -1351,15 +1351,7 @@ cp_genericize_r (tree *stmt_p, int *walk
{
tree d = DECL_EXPR_DECL (stmt);
if (TREE_CODE (d) == VAR_DECL)
- {
- gcc_assert (CP_DECL_THREAD_LOCAL_P (d) == DECL_THREAD_LOCAL_P (d));
- /* User var initializers should be genericized during containing
-BIND_EXPR genericization when walk_tree walks DECL_INITIAL
-of BIND_EXPR_VARS. Artificial temporaries might not be
-mentioned there though, so walk them now. */
- if (DECL_ARTIFICIAL (d) && !TREE_STATIC (d) && DECL_INITIAL (d))
- cp_walk_tree (&DECL_INITIAL (d), cp_genericize_r, data, NULL);
- }
+ gcc_assert (CP_DECL_THREAD_LOCAL_P (d) == DECL_THREAD_LOCAL_P (d));
}
else if (TREE_CODE (stmt) == OMP_PARALLEL
|| TREE_CODE (stmt) == OMP_TASK
--- gcc/cp/tree.c.jj2016-07-11 11:14:28.0 +0200
+++ gcc/cp/tree.c 2016-07-11 15:30:38.635047697 +0200
@@ -4075,6 +4075,22 @@ cp_walk_subtrees (tree *tp, int *walk_su
*walk_subtrees_p = 0;
break;
+case DECL_EXPR:
+ /* User variables should be mentioned in BIND_EXPR_VARS
+and their initializers and sizes walked when walking
+the containing BIND_EXPR. Compiler temporaries are
+handled here. */
+ if (VAR_P (TREE_OPERAND (*tp, 0))
+ && DECL_ARTIFICIAL (TREE_OPERAND (*tp, 0))
+ && !TREE_STATIC (TREE_OPERAND (*tp, 0)))
+ {
+ tree decl = TREE_OPERAND (*tp, 0);
+ WALK_SUBTREE (DECL_INITIAL (decl));
+ WALK_SUBTREE (DECL_SIZE (decl));
+ WALK_SUBTREE (DECL_SIZE_UNIT (decl));
+ }
+ break;
+
default:
return NULL_TREE;
}
Jakub
Re: Implement -Wswitch-fallthrough
> I explained why supporting the classic lint style comment wouldn't fly. Not convincing, it worked fine for 30+ years of lints. -Andi -- a...@linux.intel.com -- Speaking for myself only.
Re: Implement -Wswitch-fallthrough
On Mon, Jul 11, 2016 at 10:11:52PM +0200, Jakub Jelinek wrote: > On Mon, Jul 11, 2016 at 10:08:02PM +0200, Eric Botcazou wrote: > > > After I'd completed the warning, I kicked off a bootstrap so as to add > > > various gcc_fallthrough calls. There were a good amount of them, as > > > expected. I also grepped various FALLTHRU/Falls through/...fall > > > thru.../... comments in config/ and added gcc_fallthroughs to make it > > > easier for people. Wherever I wasn't sure that the gcc_fallthrough was > > > appropriate, I added an 'XXX' comment. This must not be relied upon as I > > > don't know most of the compiler code. The same goes for relevant > > > libraries > > > where I introduced various gomp_fallthrough macros conditional on __GNUC__ > > > (I'd tried to use a configure check but then decided I won't put up with > > > all the vagaries of autoconf). > > > > Do we really want to clutter up the entire tree like that? The result is > > particularly ugly IMO. Just add -Wno-switch-fallthrough somewhere I'd say. I think that if *we* refuse to use __builtin_fallthrough, then we can't expect users to use it. > Well, we already have the /* FALLTHRU */ etc. comments in most of the > places. Perhaps if we replace those comments just with GCC_FALLTHRU, > a macro that expands to __builtin_fallthrough (); for GCC >= 7, or > to [[fallthrough]] for C++17, or nothing? > IMHO it doesn't make sense to keep both the gomp_fallthrough (); macro and > /* FALLTHROUGH */ etc. comments in the source. Yea, that's a possibility, though sometimes the comment explains why the fall through is desirable. Marek
Re: Implement -Wswitch-fallthrough
On Mon, Jul 11, 2016 at 01:18:02PM -0700, Andi Kleen wrote: > > I explained why supporting the classic lint style comment wouldn't fly. > > Not convincing, it worked fine for 30+ years of lints. So how can the compiler handle /* Never ever fall through here */ ? Marek
Re: Implement -Wswitch-fallthrough
On Mon, Jul 11, 2016 at 3:43 PM, Marek Polacek wrote: > But then the [[fallthrough]] attribute was > approved for C++17 [1], and that's what has got me to do all this. > ... > I added a new builtin, > __builtin_fallthrough, that prevents the warning from occurring. It can only > be used in a switch; the compiler will issue an error otherwise. The new C++ > attribute can then be implemented in terms of this builtin. This is a stupid question I'm sure, but if C++ can have the attribute version, why can't C have __attribute__((fallthrough)) ?
Re: Implement -Wswitch-fallthrough
> I think that if *we* refuse to use __builtin_fallthrough, then we can't > expect users to use it. No disagreement, but this doesn't change my mind on the thing... IMO there ought to be a simple & standard way of silencing the warning if it is enabled by default, otherwise this will very likely badly backfire. -- Eric Botcazou
Re: Implement -Wswitch-fallthrough
On Mon, Jul 11, 2016 at 10:23:30PM +0200, Marek Polacek wrote: > On Mon, Jul 11, 2016 at 01:18:02PM -0700, Andi Kleen wrote: > > > I explained why supporting the classic lint style comment wouldn't fly. > > > > Not convincing, it worked fine for 30+ years of lints. > > So how can the compiler handle > /* Never ever fall through here */ > ? It can't. But it perhaps could handle a couple of documented most common comment styles, perhaps only if followed by break token, and turn say /* FALLTHROUGH */ break (and say: /* FALL THROUGH */ /* FALLTHRU */ /* FALL THRU */ /*-fallthrough*/ /* Fallthrough */ /* Fall through */ /* Fallthru */ /* Fall thru */ /* fallthrough */ /* fall through */ /* fallthru */ /* fall thru */ // FALLTHROUGH // FALL THROUGH // FALLTHRU // FALL THRU //-fallthrough // Fallthrough // Fall through // Fallthru // Fall thru // fallthrough // fall through // fallthru // fall thru ) into: #pragma GCC fallthrough break and make sure that if the pragma appears outside of switch or say in if (...) #pragma GCC fallthrough break and similar spots, then it is actually ignored, rather than affecting the parsed code. Supporting also __builtin_fallthrough (); and [[fallthrough]] is desirable, the transformation of the above comments into the pragma should be controllable by some separate switch? Jakub
Re: Implement -Wswitch-fallthrough: rs6000
I'm curious about this. In the process of developing a code analysis
tool, I found some GCC code that was, basically:
switch (v) {
case 0:
if (e) {
do_something();
} else {
case 1:
do_something_else();
}
}
Does this patch mean that the above got fixed? I mean, if you're
going to fret over linguistic tags to make falling through explicit,
it would seem the above code is pretty sore-thumby, yes?
