[PATCH 1/1] Remove redundant definition of srcrootpre
This script has the only occurrence of it, and in this line
it defines and exports it.
---
config-ml.in | 1 -
1 file changed, 1 deletion(-)
diff --git a/config-ml.in b/config-ml.in
index 47f153350..58c80a35c 100644
--- a/config-ml.in
+++ b/config-ml.in
@@ -493,7 +493,6 @@ multi-do:
true; \
else \
rootpre=`${PWD_COMMAND}`/; export rootpre; \
- srcrootpre=`cd $(srcdir); ${PWD_COMMAND}`/; export srcrootpre; \
lib=`echo "$${rootpre}" | sed -e 's,^.*/\([^/][^/]*\)/$$,\1,'`; \
compiler="$(CC)"; \
for i in `$${compiler} --print-multi-lib 2>/dev/null`; do \
--
2.12.2
Re: [PATCH] warn on mem calls modifying objects of non-trivial types (PR 80560)
On 06/04/2017 10:01 PM, Jason Merrill wrote:
On 06/02/2017 05:28 PM, Martin Sebor wrote:
On 05/31/2017 05:34 PM, Jason Merrill wrote:
On 05/27/2017 06:44 PM, Martin Sebor wrote:
+ /* True if the class is trivial and has a trivial non-deleted copy
+ assignment, copy ctor, and default ctor, respectively. The last
+ one isn't used to issue warnings but only to decide what suitable
+ alternatives to offer as replacements for the raw memory
operation. */
+ bool trivial = trivial_type_p (desttype);
This comment seems out of date; there's only one variable here now.
+ /* True if the class is has a non-deleted trivial assignment. Set
s/is//
+ /* True if the class has a (possibly deleted) trivial copy ctor. */
+ bool trivcopy = trivially_copyable_p (desttype);
"True if the class is trivially copyable."
+ if (delassign)
+warnfmt = G_("%qD writing to an object of type %#qT with "
+ "deleted copy assignment");
+ else if (!trivassign)
+warnfmt = G_("%qD writing to an object of type %#qT with "
+ "no trivial copy assignment");
+ else if (!trivial)
+warnfmt = G_("%qD writing to an object of non-trivial "
+ "type %#qT; use assignment instead");
I'd still like the !trivial test to come first in all the memset cases,
!trivcopy in the copy cases.
The tests are in the order they're in to provide as much useful
detail in the diagnostics as necessary to understand the problem
make the suggestion meaningful. To what end you want to change
it?
Mostly I was thinking that whether a class is trivial(ly copyable) is
more to the point, but I guess what you have now is fine.
+static bool
+has_trivial_special_function (tree ctype, special_function_kind sfk,
+ bool *deleted_p)
This seems redundant with type_has_trivial_fn. If that function is
giving the wrong answer for a class where all of the SFK are deleted,
let's fix that, under check_bases_and_members, rather than introduce a
new function. I don't want to call synthesized_method_walk every time
we want to check whether a function is trivial.
A deleted special function can be trivial.
I believe that in the language, the triviality of a deleted function
cannot be determined. But I believe you're right about the behavior of
type_has_trivial_fn, which is why I mentioned changing it.
Maybe I should use a different approach and instead of trying
to see if a function is deleted use trivially_xible to see if
it's usable. That will mean changing the diagnostics from
"with a deleted special function" to "without trivial special
function" but it will avoid calling synthesized_method_walk
while still avoiding giving bogus suggestions.
Actually, this would check for one possible argument type and not
others, so I think it's better to keep looking at the declarations. You
can do that by just looking them up (lookup_fnfields_slot) and iterating
over them, you don't need to call synthesized_method_walk.
Jason
[PATCH, GCC/THUMB1] New define_insn_and_split pattern to optimize out unnecessary uxtb instructions
GCC currently generates unnecessary uxtb instructions when compiling
gcc.target/arm/unsigned-extend-1.c for Thumb1 targets such as the Cortex-M0.
This test case is as follows:
unsigned char foo (unsigned char c)
{
return (c >= '0') && (c <= '9');
}
The assembly output generated with "-mcpu=cortex-m0 -O2":
foo:
mov r3, r0
mov r2, #9
mov r0, #0
sub r3, r3, #48
uxtbr3, r3
cmp r2, r3
adc r0, r0, r0
@ sp needed
uxtbr0, r0
bx lr
The uxtb instructions are supposed to be optimized out at the rtl combine pass,
but the current thumb1_addsi3_addgeu insn pattern is not friendly to the
combine pass. Upon entering the combine pass, we have this rtl:
(insn 6 3 7 2 (set (reg:SI 116)
(plus:SI (reg/v:SI 114 [ c ])
(const_int -48 [0xffd0])))
(insn 7 6 8 2 (set (reg:SI 117)
(zero_extend:SI (subreg:QI (reg:SI 116) 0)))
(insn 8 7 9 2 (set (reg:SI 119)
(const_int 9 [0x9]))
(insn 9 8 10 2 (set (reg:SI 120)
(const_int 0 [0]))
(insn 10 9 12 2 (set (reg:SI 118)
(plus:SI (plus:SI (reg:SI 120)
(reg:SI 120))
(geu:SI (reg:SI 119)
(reg:SI 117
During the rtl combine pass, operand 0 of the first plus operator is
(plus:SI (reg:SI 120) (reg:SI 120)) and this will be optimized to (const_int 0)
as reg 120 is zero after insn 9. The first plus operator will be subsequently
optimized out as it performs addition with zero. Finally, insn 10 is turned
into:
(insn 10 9 12 2 (set (reg:SI 118)
(leu:SI (reg:SI 116) (const_int 9
The optimization to the leu comparator matches our expectation that reg 116
will be used directly instead of reg 117 to eliminate uxtb insn 7.
Unfortunately, this new insn 10 is an illegal rtl for Thumb1. Hence, the
optimization done in the combine pass is undone and the unnecessary uxtb
instruction is kept.
This patch implements a veneer for the insn pattern thumb1_addsi3_addgeu. This
veneer insn pattern looks like the new insn 10 in the above example and can
facilitate the combine pass to optimize out the unnecessary uxtb instructions.
During the rtl expand stage, GIMPLE trees are expanded to this veneer insn
pattern rather than the normal thumb1_addsi3_addgeu. Later on in the split1
stage, this veneer insn pattern will be turned into normal thumb1_addsi3_addgeu
pattern for future code generation.
gcc/ChangeLog:
2017-06-05 Prakhar Bahuguna
* config/arm/arm.md (cstoresi_leu_thumb1): New define_insn_and_split
pattern.
(cstoresi4): Use above new pattern.
testsuite/ChangeLog:
2017-06-05 Prakhar Bahuguna
* gcc.target/arm/thumb1-unsigned-extend-1.c: New file.
Testing done: Ran regression tests, added new test to confirm uxtb instructions
are optimized out for Thumb1 targets.
Okay for stage1?
--
Prakhar Bahuguna
>From c0c5d410536238bf57967cbc50bdc8e7f7f0ab59 Mon Sep 17 00:00:00 2001
From: Prakhar Bahuguna
Date: Fri, 24 Mar 2017 10:48:03 +
Subject: [PATCH] New define_insn_and_split pattern to optimize out unnecessary
uxtb instructions
---
gcc/config/arm/arm.md | 36 ++
.../gcc.target/arm/thumb1-unsigned-extend-1.c | 10 ++
2 files changed, 40 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/arm/thumb1-unsigned-extend-1.c
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index e6e1ac54a85..656ede7c2f5 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -7691,18 +7691,14 @@
case LEU:
op3 = force_reg (SImode, operands[3]);
- scratch = force_reg (SImode, const0_rtx);
- emit_insn (gen_thumb1_addsi3_addgeu (operands[0], scratch, scratch,
- op3, operands[2]));
+ emit_insn (gen_cstoresi_leu_thumb1 (operands[0], operands[2], op3));
break;
case GEU:
op3 = operands[3];
if (!thumb1_cmp_operand (op3, SImode))
op3 = force_reg (SImode, op3);
- scratch = force_reg (SImode, const0_rtx);
- emit_insn (gen_thumb1_addsi3_addgeu (operands[0], scratch, scratch,
- operands[2], op3));
+ emit_insn (gen_cstoresi_leu_thumb1 (operands[0], op3, operands[2]));
break;
case LTU:
@@ -7781,6 +,34 @@
}"
)
+(define_insn_and_split "cstoresi_leu_thumb1"
+ [(set (match_operand:SI 0 "s_register_operand" "=l")
+ (leu:SI (match_operand:SI 1 "s_register_operand" "l")
+ (match_operand:SI 2 "thumb1_cmp_operand" "lI")))]
+ "TARGET_THUMB1"
+ "#"
+ "TARGET_THUMB1 && !reload_completed"
+ [(set (match_dup 3) (const_int 0))
+ (set (match_dup 0)
+ (plus:SI (plus:SI (match_dup 3)
+ (match_dup 3))
+(geu:SI (match_dup 4)
+(match_dup 1]
+ "
+operands[3] = gen_reg_rtx (SImode);
+
Re: [PATCH GCC][4/5]Improve loop distribution to handle hmmer
On Mon, Jun 5, 2017 at 5:21 AM, Kugan Vivekanandarajah wrote: > Hi Bin, > > Thanks for posting the patch. I haven't looked in detail yet but have > couple of quick questions. > > 1. Shouldn’t the run time alias check for versioning happen only when > vectorisation is enabled? You seems to be using the > IFN_LOOP_DIST_ALIAS when vectoring but seems to be versioning > otherwise too. It's to keep behavior consistent when user explicitly specify the option at for example O2 optimization level. It shouldn't matter much since in most cases vectorizer is enabled along with distribution. One case it could be useful is in test cases, i.e, we can test distribution without depending on vectorization. > > 2. As I understand, loop distribution will be mostly beneficial when > enabling transformations succeeds. Like resulting loop being > vectorized. I am wondering if we should only version when the loop’s > control flow is simple that it can be vectorizable. I know that you > are adding the internal function for this to some extend but for some > cases we should be able to say while in loop distribution itself that > the control flow will not result in loop being vectorized. So far distribution is mainly for vectorization, though it could be beneficial to other parallelization optimization. OTOH, cost model on control flow graph only may not be enough, for example, we may still want to distribute case consisting of a vectorizable partition and a complicated CFG partition. One possible change is we may want to restrict the transformation to case with limited number partitions, which is more or less for compilation time concern. That being said, we don't have a cost model now. > > Btw, did you run Spec2006 with this? Any notable changes ? Hmmer. Thanks, bin > > Thanks, > Kugan > > > > > On 2 June 2017 at 21:51, Bin Cheng wrote: >> Hi, >> This is the main patch of the change. It improves loop distribution by >> versioning loop under >> runtime alias check conditions, as well as better partition fusion. As >> described in comments, >> the patch basically implements distribution in the following steps: >> >> 1) Seed partitions with specific type statements. For now we support >> two types seed statements: statement defining variable used outside >> of loop; statement storing to memory. >> 2) Build reduced dependence graph (RDG) for loop to be distributed. >> The vertices (RDG:V) model all statements in the loop and the edges >> (RDG:E) model flow and control dependences between statements. >> 3) Apart from RDG, compute data dependences between memory references. >> 4) Starting from seed statement, build up partition by adding depended >> statements according to RDG's dependence information. Partition is >> classified as parallel type if it can be executed parallelly; or as >> sequential type if it can't. Parallel type partition is further >> classified as different builtin kinds if it can be implemented as >> builtin function calls. >> 5) Build partition dependence graph (PG) based on data dependences. >> The vertices (PG:V) model all partitions and the edges (PG:E) model >> all data dependences between every partitions pair. In general, >> data dependence is either compilation time known or unknown. In C >> family languages, there exists quite amount compilation time unknown >> dependences because of possible alias relation of data references. >> We categorize PG's edge to two types: "true" edge that represents >> compilation time known data dependences; "alias" edge for all other >> data dependences. >> 6) Traverse subgraph of PG as if all "alias" edges don't exist. Merge >> partitions in each strong connected commponent (SCC) correspondingly. >> Build new PG for merged partitions. >> 7) Traverse PG again and this time with both "true" and "alias" edges >> included. We try to break SCCs by removing some edges. Because >> SCCs by "true" edges are all fused in step 6), we can break SCCs >> by removing some "alias" edges. It's NP-hard to choose optimal >> edge set, fortunately simple approximation is good enough for us >> given the small problem scale. >> 8) Collect all data dependences of the removed "alias" edges. Create >> runtime alias checks for collected data dependences. >> 9) Version loop under the condition of runtime alias checks. Given >> loop distribution generally introduces additional overhead, it is >> only useful if vectorization is achieved in distributed loop. We >> version loop with internal function call IFN_LOOP_DIST_ALIAS. If >> no distributed loop can be vectorized, we simply remove distributed >> loops and recover to the original one. >> >> Also, there are some more to improve in the future (wh
Fix profile updating in tree-ssa-loop-im
Hi,
this patch fixes profile update in tree-ssa-loop-im.c. This looks like
quite a noticeable bug because the basic block storing flags ends up with
count 0 and thus is optimized for size/possibly moved offline so perhaps even
backporting to the release branches would make sense.
LSM profuces the following code:
lsm = MEM;
lsm_flag = false;
...
for (...) {
if (foo)
stuff;
else {
lsm = TMP_VAR;
lsm_flag = true;
}
}
if (lsm_flag) <--
MEM = lsm; <--
What was missing was any profile on if (lsm_flag) path at the end.
I don't think one can realistically figure probability of this conditional
(perhaps estimate assuming that if (foo) is independent based on number
of iterations, but that requires computation of powers and seems thus bit
involved and also not necessarily realistic.
Instead I just look for special case where lsm_flag is set always (common
case) or almost never.
Bootstrapped/regtested x86_64-linux, comitted.
Honza
* gcc.dg/tree-prof/cold_partition_label.c: Update template.
* tree-ssa-loop-im.c (execute_sm_if_changed): Add FLAG_BBS parameter;
update profile.
(sm_set_flag_if_changed): Add bbs field.
(execute_sm_if_changed_flag_set): Pass BBS.
(execute_sm): Update.
Index: testsuite/gcc.dg/tree-prof/cold_partition_label.c
===
--- testsuite/gcc.dg/tree-prof/cold_partition_label.c (revision 248862)
+++ testsuite/gcc.dg/tree-prof/cold_partition_label.c (working copy)
@@ -1,7 +1,7 @@
/* Test case to check if function foo gets split and the cold function
gets a label. */
/* { dg-require-effective-target freorder } */
-/* { dg-options "-O2 -freorder-blocks-and-partition -save-temps" } */
+/* { dg-options "-O2 -freorder-blocks-and-partition -save-temps
-fdump-tree-optimized" } */
#define SIZE 1
@@ -39,3 +39,4 @@ main (int argc, char *argv[])
/* { dg-final-use { scan-assembler "foo\[._\]+cold\[\._\]+0" { target
*-*-linux* *-*-gnu* } } } */
/* { dg-final-use { scan-assembler "size\[
\ta-zA-Z0-0\]+foo\[._\]+cold\[\._\]+0" { target *-*-linux* *-*-gnu* } } } */
+/* { dg-final-use { scan-tree-dump-not "Invalid sum" "optimized"} } */
Index: tree-ssa-loop-im.c
===
--- tree-ssa-loop-im.c (revision 248862)
+++ tree-ssa-loop-im.c (working copy)
@@ -1786,7 +1786,8 @@ struct prev_flag_edges {
*/
static void
-execute_sm_if_changed (edge ex, tree mem, tree tmp_var, tree flag)
+execute_sm_if_changed (edge ex, tree mem, tree tmp_var, tree flag,
+ edge preheader, hash_set *flag_bbs)
{
basic_block new_bb, then_bb, old_dest;
bool loop_has_only_one_exit;
@@ -1796,6 +1797,54 @@ execute_sm_if_changed (edge ex, tree mem
struct prev_flag_edges *prev_edges = (struct prev_flag_edges *) ex->aux;
bool irr = ex->flags & EDGE_IRREDUCIBLE_LOOP;
+ int freq_sum = 0;
+ profile_count count_sum = profile_count::zero ();
+ int nbbs = 0, ncount = 0;
+ int flag_probability = -1;
+
+ /* Flag is set in FLAG_BBS. Determine probability that flag will be true
+ at loop exit.
+
+ This code may look fancy, but it can not update profile very realistically
+ because we do not know the probability that flag will be true at given
+ loop exit.
+
+ We look for two interesting extremes
+ - when exit is dominated by block setting the flag, we know it will
+ always be true. This is a common case.
+ - when all blocks setting the flag have very low frequency we know
+ it will likely be false.
+ In all other cases we default to 2/3 for flag being true. */
+
+ for (hash_set::iterator it = flag_bbs->begin ();
+ it != flag_bbs->end (); ++it)
+{
+ freq_sum += (*it)->frequency;
+ if ((*it)->count.initialized_p ())
+ count_sum += (*it)->count, ncount ++;
+ if (dominated_by_p (CDI_DOMINATORS, ex->src, *it))
+flag_probability = REG_BR_PROB_BASE;
[PATCH] Add noexcept to shared_ptr owner comparisons (LWG 2873)
C++17 requires these to be noexcept, and there's no reason not to do
it for earlier standard modes too.
* include/bits/shared_ptr_base.h (__shared_ptr::owner_before)
(__weak_ptr::owner_before, _Sp_owner_less::operator()): Add noexcept
specifiers as per LWG 2873 and LWG 2942.
* testsuite/20_util/owner_less/noexcept.cc: New.
* testsuite/20_util/shared_ptr/observers/owner_before.cc: Test
noexcept guarantees.
* testsuite/20_util/weak_ptr/observers/owner_before.cc: Likewise.
Tested powerpc64le-linux, committed to trunk.
commit 82f522e382dac837924af829a70c61dd3db0ea94
Author: Jonathan Wakely
Date: Mon Jun 5 10:12:22 2017 +0100
Add noexcept to shared_ptr owner comparisons (LWG 2873)
* include/bits/shared_ptr_base.h (__shared_ptr::owner_before)
(__weak_ptr::owner_before, _Sp_owner_less::operator()): Add noexcept
specifiers as per LWG 2873 and LWG 2942.
* testsuite/20_util/owner_less/noexcept.cc: New.
* testsuite/20_util/shared_ptr/observers/owner_before.cc: Test
noexcept guarantees.
* testsuite/20_util/weak_ptr/observers/owner_before.cc: Likewise.
diff --git a/libstdc++-v3/include/bits/shared_ptr_base.h
b/libstdc++-v3/include/bits/shared_ptr_base.h
index b4a5edf..f0916d0 100644
--- a/libstdc++-v3/include/bits/shared_ptr_base.h
+++ b/libstdc++-v3/include/bits/shared_ptr_base.h
@@ -1309,12 +1309,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template
bool
- owner_before(__shared_ptr<_Tp1, _Lp> const& __rhs) const
+ owner_before(__shared_ptr<_Tp1, _Lp> const& __rhs) const noexcept
{ return _M_refcount._M_less(__rhs._M_refcount); }
template
bool
- owner_before(__weak_ptr<_Tp1, _Lp> const& __rhs) const
+ owner_before(__weak_ptr<_Tp1, _Lp> const& __rhs) const noexcept
{ return _M_refcount._M_less(__rhs._M_refcount); }
protected:
@@ -1697,12 +1697,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template
bool
- owner_before(const __shared_ptr<_Tp1, _Lp>& __rhs) const
+ owner_before(const __shared_ptr<_Tp1, _Lp>& __rhs) const noexcept
{ return _M_refcount._M_less(__rhs._M_refcount); }
template
bool
- owner_before(const __weak_ptr<_Tp1, _Lp>& __rhs) const
+ owner_before(const __weak_ptr<_Tp1, _Lp>& __rhs) const noexcept
{ return _M_refcount._M_less(__rhs._M_refcount); }
void
@@ -1747,15 +1747,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
struct _Sp_owner_less : public binary_function<_Tp, _Tp, bool>
{
bool
- operator()(const _Tp& __lhs, const _Tp& __rhs) const
+ operator()(const _Tp& __lhs, const _Tp& __rhs) const noexcept
{ return __lhs.owner_before(__rhs); }
bool
- operator()(const _Tp& __lhs, const _Tp1& __rhs) const
+ operator()(const _Tp& __lhs, const _Tp1& __rhs) const noexcept
{ return __lhs.owner_before(__rhs); }
bool
- operator()(const _Tp1& __lhs, const _Tp& __rhs) const
+ operator()(const _Tp1& __lhs, const _Tp& __rhs) const noexcept
{ return __lhs.owner_before(__rhs); }
};
@@ -1764,7 +1764,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{
template
auto
- operator()(const _Tp& __lhs, const _Up& __rhs) const
+ operator()(const _Tp& __lhs, const _Up& __rhs) const noexcept
-> decltype(__lhs.owner_before(__rhs))
{ return __lhs.owner_before(__rhs); }
diff --git a/libstdc++-v3/testsuite/20_util/owner_less/noexcept.cc
b/libstdc++-v3/testsuite/20_util/owner_less/noexcept.cc
new file mode 100644
index 000..25c9afd
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/owner_less/noexcept.cc
@@ -0,0 +1,40 @@
+// Copyright (C) 2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+//
Re: [PATCH, GCC/ARM, ping] Rename *_compute_save_reg_mask ()
Ping?
Best regards,
Thomas
On 26/05/17 15:19, Thomas Preudhomme wrote:
Hi,
Name of arm_compute_save_reg_mask and thumb1_compute_save_reg_mask
suggest that all register saving computation is done in these function
but these only deal with core registers. The caller
arm_compute_frame_layout () also computes some of the register saving,
eg. for VFP registers in 32bit targets. This commit rename former
functions to refer to core registers saving.
ChangeLog entry is as follows:
*** gcc/ChangeLog ***
2017-05-24 Thomas Preud'homme
* config/arm/arm.c (arm_compute_save_reg_mask): Rename into ...
(arm_compute_save_core_reg_mask): This.
(thumb1_compute_save_reg_mask): Rename into ...
(thumb1_compute_save_core_reg_mask): This.
(arm_compute_save_reg0_reg12_mask): Adapt comment.
(arm_compute_frame_layout): Likewise.
Ok for trunk?
Tested with a successful arm-none-eabi build.
Best regards,
Thomas
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index c6036fe24abe761540374cfed36edcb953703f5b..6a4ab358d5ee5ceef6a3a1cdfa630dece25cd143 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -130,7 +130,7 @@ static void push_minipool_fix (rtx_insn *, HOST_WIDE_INT, rtx *,
static void arm_reorg (void);
static void note_invalid_constants (rtx_insn *, HOST_WIDE_INT, int);
static unsigned long arm_compute_save_reg0_reg12_mask (void);
-static unsigned long arm_compute_save_reg_mask (void);
+static unsigned long arm_compute_save_core_reg_mask (void);
static unsigned long arm_isr_value (tree);
static unsigned long arm_compute_func_type (void);
static tree arm_handle_fndecl_attribute (tree *, tree, tree, int, bool *);
@@ -19030,7 +19030,7 @@ output_ascii_pseudo_op (FILE *stream, const unsigned char *p, int len)
&& reg >= FIRST_HI_REGNUM && reg <= LAST_HI_REGNUM))
/* Compute the register save mask for registers 0 through 12
- inclusive. This code is used by arm_compute_save_reg_mask. */
+ inclusive. This code is used by arm_compute_save_core_reg_mask (). */
static unsigned long
arm_compute_save_reg0_reg12_mask (void)
@@ -19157,12 +19157,12 @@ arm_compute_static_chain_stack_bytes (void)
return 0;
}
-/* Compute a bit mask of which registers need to be
+/* Compute a bit mask of which core registers need to be
saved on the stack for the current function.
This is used by arm_compute_frame_layout, which may add extra registers. */
static unsigned long
-arm_compute_save_reg_mask (void)
+arm_compute_save_core_reg_mask (void)
{
unsigned int save_reg_mask = 0;
unsigned long func_type = arm_current_func_type ();
@@ -19244,10 +19244,10 @@ arm_compute_save_reg_mask (void)
return save_reg_mask;
}
-/* Compute a bit mask of which registers need to be
+/* Compute a bit mask of which core registers need to be
saved on the stack for the current function. */
static unsigned long
-thumb1_compute_save_reg_mask (void)
+thumb1_compute_save_core_reg_mask (void)
{
unsigned long mask;
unsigned reg;
@@ -20783,7 +20783,7 @@ any_sibcall_could_use_r3 (void)
eliminating some of the registers.
The values returned by this function must reflect the behavior
- of arm_expand_prologue() and arm_compute_save_reg_mask().
+ of arm_expand_prologue () and arm_compute_save_core_reg_mask ().
The sign of the number returned reflects the direction of stack
growth, so the values are positive for all eliminations except
@@ -20839,7 +20839,7 @@ arm_compute_frame_layout (void)
{
unsigned int regno;
- offsets->saved_regs_mask = arm_compute_save_reg_mask ();
+ offsets->saved_regs_mask = arm_compute_save_core_reg_mask ();
core_saved = bit_count (offsets->saved_regs_mask) * 4;
saved = core_saved;
@@ -20870,7 +20870,7 @@ arm_compute_frame_layout (void)
}
else /* TARGET_THUMB1 */
{
- offsets->saved_regs_mask = thumb1_compute_save_reg_mask ();
+ offsets->saved_regs_mask = thumb1_compute_save_core_reg_mask ();
core_saved = bit_count (offsets->saved_regs_mask) * 4;
saved = core_saved;
if (TARGET_BACKTRACE)
Re: [PATCH, GCC/ARM, ping] Complete legend for ARM register allocation in arm.h
Ping? Best regards, Thomas On 26/05/17 15:20, Thomas Preudhomme wrote: [resending with gcc-patches this time] Hi, The comment describing ARM register allocation in arm.h contains 3 keys: S, F and *. The legend only explains S and *, this patch remedies that. ChangeLog entry is as follows: *** gcc/ChangeLog *** 2017-05-24 Thomas Preud'homme * config/arm/arm.h: explain F symbol found in description of ARM register allocation in its legend. Is this ok for trunk? Best regards, Thomas diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h index 1821f48ed7ed45ffa5503541426921ec0d0a3736..d0b448d273cb3683f06512fea80ebf65df587ab1 100644 --- a/gcc/config/arm/arm.h +++ b/gcc/config/arm/arm.h @@ -690,7 +690,7 @@ extern int arm_arch_cmse; /* Standard register usage. */ /* Register allocation in ARM Procedure Call Standard - (S - saved over call). + (S - saved over call, F - Frame-related). r0 * argument word/integer result r1-r3 argument word
Re: [PATCH, GCC/ARM] Complete legend for ARM register allocation in arm.h
On 26/05/17 15:20, Thomas Preudhomme wrote: [resending with gcc-patches this time] Hi, The comment describing ARM register allocation in arm.h contains 3 keys: S, F and *. The legend only explains S and *, this patch remedies that. ChangeLog entry is as follows: *** gcc/ChangeLog *** 2017-05-24 Thomas Preud'homme * config/arm/arm.h: explain F symbol found in description of ARM register allocation in its legend. Is this ok for trunk? Ok. Thanks, Kyrill Best regards, Thomas
Re: [PATCH, GCC/ARM] Rename *_compute_save_reg_mask ()
On 26/05/17 15:19, Thomas Preudhomme wrote: Hi, Name of arm_compute_save_reg_mask and thumb1_compute_save_reg_mask suggest that all register saving computation is done in these function but these only deal with core registers. The caller arm_compute_frame_layout () also computes some of the register saving, eg. for VFP registers in 32bit targets. This commit rename former functions to refer to core registers saving. ChangeLog entry is as follows: *** gcc/ChangeLog *** 2017-05-24 Thomas Preud'homme * config/arm/arm.c (arm_compute_save_reg_mask): Rename into ... (arm_compute_save_core_reg_mask): This. (thumb1_compute_save_reg_mask): Rename into ... (thumb1_compute_save_core_reg_mask): This. (arm_compute_save_reg0_reg12_mask): Adapt comment. (arm_compute_frame_layout): Likewise. Ok for trunk? Tested with a successful arm-none-eabi build. Ok if a arm-none-linux-gnueabihf shows no problems. This looks simple enough to me and a useful renaming. Kyrill Best regards, Thomas
Re: [PATCH] gcc::context creation
On Wed, May 24, 2017 at 08:00:30AM -0400, Nathan Sidwell wrote:
> On 05/24/2017 07:57 AM, Jakub Jelinek wrote:
>
> > The numbering is not to differentiate between dumps of the same name, that
> > is done through numbered suffixes of the dump names, but to make the order
> > of passes visible to the user and for sorted file listing to be provided in
> > that order. So, rather than having lang files unnumbered, I think it is
> > better to number them and just have a maximum of lang dumps and if some FE
> > registers fewers, bump m_next_dump accordingly (create a hole).
> > That is pretty much the standard behavior, if some dump is registered, but
> > for whatever reason not dumped, you also get a hole.
>
> oh, ok. Seems fine to me.
Here is a patch to implement that. I chose to keep the 3 dumps predefined
dumps and just tweak their registered number, otherwise the changes would be
bigger. The patch additionally renames TDI_generic to TDI_gimple (to match
the dump name and content) and because we have more than 256 dumps these
days and we register them all even at -O0, the patch avoids 4 unnecessary
reallocations. Ok for trunk if testing passes?
2017-06-05 Jakub Jelinek
* dumpfile.h (enum tree_dump_index): Rename TDI_generic to
TDI_gimple.
(class dump_manager): Add register_dumps method.
* dumpfile.c: Include langhooks.h.
(dump_files): Use 0 instead of 3/4/5 for TDI_{original,gimple,nested}.
(FIRST_AUTO_NUMBERED_DUMP): Decrease to 1.
(FIRST_ME_AUTO_NUMBERED_DUMP): Define.
(dump_manager::dump_register): Start with 512 entries instead of 32.
(dump_manager::register_dumps): New method.
* toplev.c (general_init): Instead of invoking register_dumps
langhook, invoke register_dumps method on the dump manager.
* gimplify.c (gimplify_function_tree): Use TDI_gimple instead of
TDI_generic.
* gimple-parser.c (c_parser_parse_gimple_body): Use TDI_gimple instead
of TDI_generic.
--- gcc/dumpfile.h.jj 2017-06-02 09:01:10.0 +0200
+++ gcc/dumpfile.h 2017-06-05 11:20:07.563008969 +0200
@@ -31,7 +31,7 @@ enum tree_dump_index
TDI_inheritance, /* dump type inheritance graph. */
TDI_clones, /* dump IPA cloning decisions. */
TDI_original,/* dump each function before optimizing
it */
- TDI_generic, /* dump each function after genericizing it */
+ TDI_gimple, /* dump each function after gimplifying it */
TDI_nested, /* dump each function after unnesting it */
TDI_lang_all,/* enable all the language dumps. */
@@ -212,6 +212,11 @@ public:
dump_register (const char *suffix, const char *swtch, const char *glob,
dump_kind dkind, int optgroup_flags, bool take_ownership);
+ /* Allow languages and middle-end to register their dumps before the
+ optimization passes. */
+ void
+ register_dumps ();
+
/* Return the dump_file_info for the given phase. */
struct dump_file_info *
get_dump_file_info (int phase) const;
--- gcc/dumpfile.c.jj 2017-06-02 09:01:10.0 +0200
+++ gcc/dumpfile.c 2017-06-05 11:21:27.381013150 +0200
@@ -27,6 +27,7 @@ along with GCC; see the file COPYING3.
#include "dumpfile.h"
#include "context.h"
#include "tree-cfg.h"
+#include "langhooks.h"
/* If non-NULL, return one past-the-end of the matching SUBPART of
the WHOLE string. */
@@ -59,10 +60,11 @@ static struct dump_file_info dump_files[
DUMP_FILE_INFO (".cgraph", "ipa-cgraph", DK_ipa, 0),
DUMP_FILE_INFO (".type-inheritance", "ipa-type-inheritance", DK_ipa, 0),
DUMP_FILE_INFO (".ipa-clones", "ipa-clones", DK_ipa, 0),
- DUMP_FILE_INFO (".original", "tree-original", DK_tree, 3),
- DUMP_FILE_INFO (".gimple", "tree-gimple", DK_tree, 4),
- DUMP_FILE_INFO (".nested", "tree-nested", DK_tree, 5),
-#define FIRST_AUTO_NUMBERED_DUMP 3
+ DUMP_FILE_INFO (".original", "tree-original", DK_tree, 0),
+ DUMP_FILE_INFO (".gimple", "tree-gimple", DK_tree, 0),
+ DUMP_FILE_INFO (".nested", "tree-nested", DK_tree, 0),
+#define FIRST_AUTO_NUMBERED_DUMP 1
+#define FIRST_ME_AUTO_NUMBERED_DUMP 3
DUMP_FILE_INFO (NULL, "lang-all", DK_lang, 0),
DUMP_FILE_INFO (NULL, "tree-all", DK_tree, 0),
@@ -179,7 +181,7 @@ dump_register (const char *suffix, const
if (count >= m_extra_dump_files_alloced)
{
if (m_extra_dump_files_alloced == 0)
- m_extra_dump_files_alloced = 32;
+ m_extra_dump_files_alloced = 512;
else
m_extra_dump_files_alloced *= 2;
m_extra_dump_files = XRESIZEVEC (struct dump_file_info,
@@ -200,6 +202,25 @@ dump_register (const char *suffix, const
}
+/* Allow languages and middle-end to register their dumps before the
+ optimization passes. */
+void
+gcc::dump_manager::
+register_dumps ()
+{
+ lang_hooks.register_dumps (this);
+ /* If this assert fails, some FE reg
[patch, libfortran] Fix matmul result zeroing for empty arguments
Hello world, the attached patch moves the zeroing of the result variable before the early return. This is done so that the result for zero-sized arguments is still valid. The bug was only in the library version, but I have also added a test case for the inline version to make sure the bug does not suddenly appear there. OK for trunk? Regards Thomas 2017-06-05 Thomas Koenig PR fortran/80975 * m4/matmul_internal.m4: Move zeroing before early return. * generated/matmul_c10.c: Regenerated. * generated/matmul_c16.c: Regenerated. * generated/matmul_c4.c: Regenerated. * generated/matmul_c8.c: Regenerated. * generated/matmul_i1.c: Regenerated. * generated/matmul_i16.c: Regenerated. * generated/matmul_i2.c: Regenerated. * generated/matmul_i4.c: Regenerated. * generated/matmul_i8.c: Regenerated. * generated/matmul_r10.c: Regenerated. * generated/matmul_r16.c: Regenerated. * generated/matmul_r4.c: Regenerated. * generated/matmul_r8.c: Regenerated. * generated/matmulavx128_c10.c: Regenerated. * generated/matmulavx128_c16.c: Regenerated. * generated/matmulavx128_c4.c: Regenerated. * generated/matmulavx128_c8.c: Regenerated. * generated/matmulavx128_i1.c: Regenerated. * generated/matmulavx128_i16.c: Regenerated. * generated/matmulavx128_i2.c: Regenerated. * generated/matmulavx128_i4.c: Regenerated. * generated/matmulavx128_i8.c: Regenerated. * generated/matmulavx128_r10.c: Regenerated. * generated/matmulavx128_r16.c: Regenerated. * generated/matmulavx128_r4.c: Regenerated. Index: generated/matmul_c10.c === --- generated/matmul_c10.c (Revision 248472) +++ generated/matmul_c10.c (Arbeitskopie) @@ -307,6 +307,11 @@ b_offset = 1 + b_dim1; b -= b_offset; + /* Empty c first. */ + for (j=1; j<=n; j++) + for (i=1; i<=m; i++) + c[i + j * c_dim1] = (GFC_COMPLEX_10)0; + /* Early exit if possible */ if (m == 0 || n == 0 || k == 0) return; @@ -319,11 +324,6 @@ t1 = malloc (t1_dim * sizeof(GFC_COMPLEX_10)); - /* Empty c first. */ - for (j=1; j<=n; j++) - for (i=1; i<=m; i++) - c[i + j * c_dim1] = (GFC_COMPLEX_10)0; - /* Start turning the crank. */ i1 = n; for (jj = 1; jj <= i1; jj += 512) @@ -859,6 +859,11 @@ b_offset = 1 + b_dim1; b -= b_offset; + /* Empty c first. */ + for (j=1; j<=n; j++) + for (i=1; i<=m; i++) + c[i + j * c_dim1] = (GFC_COMPLEX_10)0; + /* Early exit if possible */ if (m == 0 || n == 0 || k == 0) return; @@ -871,11 +876,6 @@ t1 = malloc (t1_dim * sizeof(GFC_COMPLEX_10)); - /* Empty c first. */ - for (j=1; j<=n; j++) - for (i=1; i<=m; i++) - c[i + j * c_dim1] = (GFC_COMPLEX_10)0; - /* Start turning the crank. */ i1 = n; for (jj = 1; jj <= i1; jj += 512) @@ -1411,6 +1411,11 @@ b_offset = 1 + b_dim1; b -= b_offset; + /* Empty c first. */ + for (j=1; j<=n; j++) + for (i=1; i<=m; i++) + c[i + j * c_dim1] = (GFC_COMPLEX_10)0; + /* Early exit if possible */ if (m == 0 || n == 0 || k == 0) return; @@ -1423,11 +1428,6 @@ t1 = malloc (t1_dim * sizeof(GFC_COMPLEX_10)); - /* Empty c first. */ - for (j=1; j<=n; j++) - for (i=1; i<=m; i++) - c[i + j * c_dim1] = (GFC_COMPLEX_10)0; - /* Start turning the crank. */ i1 = n; for (jj = 1; jj <= i1; jj += 512) @@ -1977,6 +1977,11 @@ b_offset = 1 + b_dim1; b -= b_offset; + /* Empty c first. */ + for (j=1; j<=n; j++) + for (i=1; i<=m; i++) + c[i + j * c_dim1] = (GFC_COMPLEX_10)0; + /* Early exit if possible */ if (m == 0 || n == 0 || k == 0) return; @@ -1989,11 +1994,6 @@ t1 = malloc (t1_dim * sizeof(GFC_COMPLEX_10)); - /* Empty c first. */ - for (j=1; j<=n; j++) - for (i=1; i<=m; i++) - c[i + j * c_dim1] = (GFC_COMPLEX_10)0; - /* Start turning the crank. */ i1 = n; for (jj = 1; jj <= i1; jj += 512) @@ -2603,6 +2603,11 @@ b_offset = 1 + b_dim1; b -= b_offset; + /* Empty c first. */ + for (j=1; j<=n; j++) + for (i=1; i<=m; i++) + c[i + j * c_dim1] = (GFC_COMPLEX_10)0; + /* Early exit if possible */ if (m == 0 || n == 0 || k == 0) return; @@ -2615,11 +2620,6 @@ t1 = malloc (t1_dim * sizeof(GFC_COMPLEX_10)); - /* Empty c first. */ - for (j=1; j<=n; j++) - for (i=1; i<=m; i++) - c[i + j * c_dim1] = (GFC_COMPLEX_10)0; - /* Start turning the crank. */ i1 = n; for (jj = 1; jj <= i1; jj += 512) Index: generated/matmul_c16.c === --- generated/matmul_c16.c (Revision 248472) +++ generated/matmul_c16
Re: [patch, libfortran] Fix matmul result zeroing for empty arguments
Am 05.06.2017 um 11:52 schrieb Thomas Koenig: OK for trunk? ... and 7.1, where this bug was actually discovered? This is a regerssion. Regards Thomas
RE: [PATCH][x86][PR73350][PR80862]
Hi, 1 is replace 8 spaces with tab suggested by ./check_GNU_style.sh, should I still fix it back? 2,3,4 Done CSE is working, spec 2k6 on skylake-avx512 has same score. PR target/73350,80862 gcc/ * config/i386/subst.md (round): Fix round pattern. * config/i386/i386.c (ix86_erase_embedded_rounding): Fix erasing rounding for the fixed pattern. gcc/testsuite * gcc.target/i386/pr73350.c: New test. Ok for trunk? Thanks, Julia > -Original Message- > From: Kirill Yukhin [mailto:kirill.yuk...@gmail.com] > Sent: Wednesday, May 31, 2017 12:34 PM > To: Koval, Julia > Cc: GCC Patches ; Uros Bizjak > ; Peryt, Sebastian ; > ja...@redhat.com; richard.guent...@gmail.com > Subject: Re: [PATCH][x86][PR73350][PR80862] > > On 31 May 11:38, Kirill Yukhin wrote: > > Hello Julia, > > On 26 May 09:13, Koval, Julia wrote: > > > Hi, > > > This patch fixes these PR's. Ok for trunk? > > > > > > gcc/ > > > * config/i386/subst.md (round): Fix round pattern. > > > * config/i386/i386.c (ix86_erase_embedded_rounding): > > > Fix erasing rounding for the fixed pattern. > > > > > > Thanks, > > > Julia > > > > Let me copy-paste parts of your patch here. > > diff --git a/gcc/config/i386/subst.md b/gcc/config/i386/subst.md > > index 0bc22fd..2e632b9 100644 > > --- a/gcc/config/i386/subst.md > > +++ b/gcc/config/i386/subst.md > > @@ -137,12 +137,12 @@ > > > > (define_subst "round" > >[(set (match_operand:SUBST_A 0) > > -(match_operand:SUBST_A 1))] > > + (match_operand:SUBST_A 1))] > >"TARGET_AVX512F" > > - [(parallel[ > > - (set (match_dup 0) > > - (match_dup 1)) > > - (unspec [(match_operand:SI 2 "const_4_or_8_to_11_operand")] > UNSPEC_EMBEDDED_ROUNDING)])]) > > + [(set (match_dup 0) > > + (unspec:SUBST_A [(match_dup 1) > > + (match_operand:SI 2 "const_4_or_8_to_11_operand")] > > +UNSPEC_EMBEDDED_ROUNDING))]) > > > > (define_subst_attr "round_saeonly_name" "round_saeonly" "" "_round") > > (define_subst_attr "round_saeonly_mask_operand2" "mask" "%r2" "%r4") > > -- > > 2.5.5 > > > > So, you propose to put RC as third argument to the set expression. > > I am not sure that we might use (set ...) in such a way. > Whoops, I was wrong. You are setting w/ SET_SRC as UNSPEC which > which is eliminated conditionally, which is much better to me. > > Few nits: > 1. > diff --git a/gcc/config/i386/subst.md b/gcc/config/i386/subst.md > index 0bc22fd..2e632b9 100644 > --- a/gcc/config/i386/subst.md > +++ b/gcc/config/i386/subst.md > @@ -137,12 +137,12 @@ > > (define_subst "round" >[(set (match_operand:SUBST_A 0) > -(match_operand:SUBST_A 1))] > + (match_operand:SUBST_A 1))] >"TARGET_AVX512F" > Junk. > > 2. Check identation pls > > 3. Mention PR in ChangeLog entry > > 4. Add reg test please > > Overall I like this approach. We must somehow set explicit dependency > between RC and actual op, why not this way? > > Could you pls make sure that CSE is still working for ops w/ identical RC? > > To be paranoid: is it possible to check skylake-avx512 w/ and w/o the patch > on Spec2k6? > > -- > Thanks, K 0001-test_rounding.patch Description: 0001-test_rounding.patch
[PING^2] for three diagnostic-related patches
Hi again, I'd like to re-ping those patches below before they start to bit-rot ;-) Regards, Volker On 16 May, Volker Reichelt wrote: > Hi, > > I'd like to ping the following diagnostic-related patches that might > have slipped through the cracks: > > C++ parser: Fix typos in error messages: > https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00048.html > > Fix-it hints for wrong usage of 'friend' and 'auto': > https://gcc.gnu.org/ml/gcc-patches/2017-04/msg01575.html > > And finally, a follow-up patch: > https://gcc.gnu.org/ml/gcc-patches/2017-04/msg01576.html > > Regards, > Volker
[PATCH] Optimize std::advance for single increments
LWG 2931 suggests a new overload of std::next for the default n==1 case, because it can be simpler for some random access iterators to do ++i than i += 1, e.g. for std::deque::iterator. I've suggested we close that issue as NAD, because we don't need a new overload to do that, and here's the proof. This performs the same optimisation when the argument to __advance is known at compile-time to be +1 or -1, as is the case when called from std::next(i) or std::prev(i). This makes the generated assembly for std::next::iterator>(i) quite a lot shorter. In practice we probably don't need the optimisation in std::advance, only in std::next and std::prev, because it would be very unusual to call std::advance(i, 1) or std::advance(i, -1) when you could just do ++i or --i. But doing it here means we handle the std::next/std::prev cases, and std::advance, and any unusual cases like std::next(i, -1). * include/bits/stl_iterator_base_funcs.h (__advance<_RandomAccessIterator, _Distance>): Optimize for next/prev cases where incrementing or decrementing a single step. Tested powerpc64le-linux, committed to trunk. commit 95e8802ccde0fe86462b28897c78e57c1c0866cc Author: Jonathan Wakely Date: Mon Jun 5 11:13:31 2017 +0100 Optimize std::advance for single increments * include/bits/stl_iterator_base_funcs.h (__advance<_RandomAccessIterator, _Distance>): Optimize for next/prev cases where incrementing or decrementing a single step. diff --git a/libstdc++-v3/include/bits/stl_iterator_base_funcs.h b/libstdc++-v3/include/bits/stl_iterator_base_funcs.h index ce6c3d2..e14a22c 100644 --- a/libstdc++-v3/include/bits/stl_iterator_base_funcs.h +++ b/libstdc++-v3/include/bits/stl_iterator_base_funcs.h @@ -177,7 +177,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // concept requirements __glibcxx_function_requires(_RandomAccessIteratorConcept< _RandomAccessIterator>) - __i += __n; + if (__builtin_constant_p(__n) && __n == 1) + ++__i; + else if (__builtin_constant_p(__n) && __n == -1) + --__i; + else + __i += __n; } /**
Re: [PATCH] Optimize std::advance for single increments
On Mon, 5 Jun 2017, Jonathan Wakely wrote: LWG 2931 suggests a new overload of std::next for the default n==1 case, because it can be simpler for some random access iterators to do ++i than i += 1, e.g. for std::deque::iterator. I've suggested we close that issue as NAD, because we don't need a new overload to do that, and here's the proof. This performs the same optimisation when the argument to __advance is known at compile-time to be +1 or -1, as is the case when called from std::next(i) or std::prev(i). Do you want to optimize the case __n == 0 as well? ;-) -- Marc Glisse
Re: {PATCH] New C++ warning -Wcatch-value
On 15 May, Martin Sebor wrote: >> So how about the following then? I stayed with the catch part and added >> a parameter to the warning to let the user decide on the warnings she/he >> wants to get: -Wcatch-value=n. >> -Wcatch-value=1 only warns for polymorphic classes that are caught by >> value (to avoid slicing), -Wcatch-value=2 warns for all classes that >> are caught by value (to avoid copies). And finally -Wcatch-value=3 >> warns for everything not caught by reference to find typos (like pointer >> instead of reference) and bad coding practices. > > It seems reasonable to me. I'm not too fond of multi-level > warnings since few users take advantage of anything but the > default, but this case is simple and innocuous enough that > I don't think it can do harm. > >> >> Bootstrapped and regtested on x86_64-pc-linux-gnu. >> OK for trunk? >> >> If so, would it make sense to add -Wcatch-value=1 to -Wextra or even -Wall? >> I would do this in a seperate patch, becuase I haven't checked what that >> would mean for the testsuite. > > I can't think of a use case for polymorphic slicing that's not > harmful so unless there is a common one that escapes me, I'd say > -Wall. So that's what I committed after Jason's OK. > What are your thoughts on enhancing the warning to also handle > the rethrow case? > > Also, it seems that a similar warning would be useful even beyond > catch handlers, to help detect slicing when passing arguments to > functions by value. Especially in code that mixes OOP with the > STL (or other template libraries). Have you thought about tackling > that at some point as well? I don't have any plans to handle handle the rethrow case. A general slicing warning would be very nice to have. Actually clang-tidy has one (which is a little buggy, though). Implementing this is over my head, though. I'd rather stick with something less ambitious. > Martin > >> >> Regards, >> Volker
Re: [PATCH] gcc::context creation
On 06/05/2017 05:46 AM, Jakub Jelinek wrote:
Here is a patch to implement that. I chose to keep the 3 dumps predefined
dumps and just tweak their registered number, otherwise the changes would be
bigger.
Yeah, that can wait for another day.
The patch additionally renames TDI_generic to TDI_gimple (to match
the dump name and content) and because we have more than 256 dumps these
days and we register them all even at -O0, the patch avoids 4 unnecessary
reallocations. Ok for trunk if testing passes?
LGTM, two nits below. (I can't approve though)
+/* Allow languages and middle-end to register their dumps before the
+ optimization passes. */
blank line needed here, I think?
+void
+gcc::dump_manager::
+register_dumps ()
+{
+ lang_hooks.register_dumps (this);
+ /* If this assert fails, some FE registered more than
+ FIRST_ME_AUTO_NUMBERED_DUMP - FIRST_AUTO_NUMBERED_DUMP
+ dump files. Bump FIRST_ME_AUTO_NUMBERED_DUMP accordingly. */
+ gcc_assert (m_next_dump <= FIRST_ME_AUTO_NUMBERED_DUMP);
+ if (m_next_dump < FIRST_ME_AUTO_NUMBERED_DUMP)
+m_next_dump = FIRST_ME_AUTO_NUMBERED_DUMP;
Unconditionally set it? there's the protecting assert. It's almost as
if you want 'else gcc_unreachable ()' (with suitable morph of < to <=).
nathan
--
Nathan Sidwell
Re: add VxWorks specific crtstuff implementation files for Ada runtimes
On 06/02/2017 11:58 AM, Olivier Hainque wrote: Hello, 2017-06-02 Olivier Hainque ada/ * vx_crtbegin_auto.c: New file. * vx_crtbegin.c: New file. * vx_crtbegin.inc: New file. * vx_crtend.c: New file. + * Copyright (C) 2016, Free Software Foundation, Inc. Date update? -- Nathan Sidwell
Re: [PING**3] [PATCH] Force use of absolute path names for gcov
On 06/02/2017 10:43 AM, Bernd Edlinger wrote:
On 06/02/17 13:35, Nathan Sidwell wrote:
On 06/01/2017 03:24 PM, Bernd Edlinger wrote:
This is a gcc option that converts relative
path names to absolute ones, so that gcov can
properly merge the line numbers in projects
where different relative path names may refer
to the same source file.
Thanks. From reading the patch though, I didn't grok that intent. The
patch itself suggests gcov simply fails with relative paths and
directories. What you're really trying to do is find the canonical
path, which happens to be absolute. But you're not doing that either --
you're concatenating the relative path to cwd. How is that helping? Is
it when you have a mixture of absolute and relative paths?
Yes, let me explain why I need that.
Recently I wanted to get the line-coverage of an application
I work with. The problem I faced is, that different modules
invoke gcc in different working directories, and there are
things like -I ../include in one place, and -I ../../Core/include
in other places, so ../include/test.h and ../../Core/include/test.h
may or may not refer to the same file. Also several module tests
use a separate folder, and a file like "./Test1.cpp" may actually
exist in several directories, but gcov does not find the sources,
in this scenario and copying everything to a single directory is
not a good solution either.
So in the end I want to run the different module tests etc.
and let gcov collect and merge all the coverage data, and of course
find the right source files for the report, without substantially
changing the application's original make files.
Some other cases:
1) 'bob/../foo.c' and 'baz/../foo.c'?
2) 'bob/foo.c' and 'baz/foo.c' where baz is a symlink to bob?
3) combinations of #2 and #3 such that textual elision of .. gets you to
a different place than resolving symlinks.
Given all that complexity, wouldn't it be better to tell gcov where
relative paths should start? (perhaps encoding in the file?). It does
need access to the source directories.
gcov.c already has a function named canonicalize_name that does exactly
what I need, i.e. elide /./ and collapse ../bob/../foo.c
to ../foo.c and even do something with symbolic links, however my app
does not use any sym-links so I did not really test that part.
But this function works only under the assumption that relative file
names start always from the current working directory.
So what my patch does, is leave absolute file names untouched and
prepend the current working directory to all file names that do
not look like absolute file names. Thus it does not claim to
canonicalize the file name, but only to make it an absolute file
name. But now gcov's canonicalize_name is actually able to
do the rest.
Ok, that makes sense, thanks for the explanation.
+Compile the source files additionally with @option{-fprofile-abs-path}
+to create absolute path names in the @file{.gcno} files. This allows
+@command{gcov} to find the correct sources in projects with multiple
+directories.
I think the second sentence could be better. It's not that the sources
are in different directories, it's that the compiler is invoked with
different working directories. How about
'This allows @command{gcov} to find the correct sources in projects
where compilations occur with different working directories.'
modify as you see fit.
gcov_write_filename (const char *filename)
+{
+ char buf[1024];
+ size_t len;
Ew. (a) bad fixed length (b) bad stack frame bloat. Please use malloc
& MAX_PATH_LEN (or whatever the right #define is).
+ if (profile_abs_path_flag && filename && filename[0]
Can filename ever be null here? Can it ever be the empty string?
+ if (getcwd (buf, sizeof (buf) - len - 1) != NULL)
This is going to getcwd on every file (most likely). Isn't this already
available somewhere?
nathan
--
Nathan Sidwell
Re: [C++ PATCH] lang_decl selector & decomposition
On Wed, May 31, 2017 at 01:16:36PM -0400, Nathan Sidwell wrote: > On 05/31/2017 01:05 PM, Jakub Jelinek wrote: > > > That is weird, that sounds like a bug somewhere? Which testcase is it on? > > I think decomp25 & decomp7 & 8 (sorry, can't recall full name). > I discovered them because my first attempt had an assert that the incoming > base was the same as the stored one, for the already-converted case. That doesn't look like a bug to me. When instantiating structured binding inside of a template, the VAR_DECL is created using copy_decl from the template VAR_DECL: #0 0x769a6521 in memcpy () from /lib64/libc.so.6 #1 0x008ca690 in cxx_dup_lang_specific_decl (node=) at ../../gcc/cp/lex.c:657 #2 0x008ca6ee in copy_decl (decl=) at ../../gcc/cp/lex.c:675 #3 0x009b174b in tsubst_decl (t=, args=, complain=3) at ../../gcc/cp/pt.c:12860 #4 0x009b3796 in tsubst (t=, args=, complain=3, in_decl=) at ../../gcc/cp/pt.c:13376 #5 0x009bf6e8 in tsubst_decomp_names (decl=, pattern_decl=, args=, complain=3, in_decl=, first=0x7fffd648, cnt=0x7fffd654) at ../../gcc/cp/pt.c:15719 #6 0x009c0a5a in tsubst_expr (t=, args=, complain=3, in_decl=, integral_constant_expression_p=false) at ../../gcc/cp/pt.c:15890 #7 0x009bfc0d in tsubst_expr (t=, args=, complain=3, in_decl=, integral_constant_expression_p=false) at ../../gcc/cp/pt.c:15771 #8 0x009c1832 in tsubst_expr (t=, args=, complain=3, in_decl=, integral_constant_expression_p=false) at ../../gcc/cp/pt.c:15998 #9 0x009e2d6d in instantiate_decl (d=, defer_ok=false, expl_inst_class_mem_p=false) at ../../gcc/cp/pt.c:22965 and then finalized later: #0 fit_decomposition_lang_decl (t=, base=) at ../../gcc/cp/lex.c:588 #1 0x0084a584 in cp_finish_decomp (decl=, first=, count=2) at ../../gcc/cp/decl.c:7387 #2 0x009c0a9b in tsubst_expr (t=, args=, complain=3, in_decl=, integral_constant_expression_p=false) at ../../gcc/cp/pt.c:15894 #3 0x009bfc0d in tsubst_expr (t=, args=, complain=3, in_decl=, integral_constant_expression_p=false) at ../../gcc/cp/pt.c:15771 #4 0x009c1832 in tsubst_expr (t=, args=, complain=3, in_decl=, integral_constant_expression_p=false) at ../../gcc/cp/pt.c:15998 #5 0x009e2d6d in instantiate_decl (d=, defer_ok=false, expl_inst_class_mem_p=false) at ../../gcc/cp/pt.c:22965 So unless cxx_dup_lang_specific_decl or copy_decl or tsubst_decl clears the DECL_DECOMP_BASE for the short time before cp_finish_decomp is called, DECL_DECOMP_BASE will be "wrong" for a short time, but it shouldn't matter, nothing should be querying it in between. Setting it to NULL wouldn't be correct either anyway. Jakub
[PATCH] remove incorrect assert
Hi,
here is a tiny patch which removes an assert which I believe is wrong.
I think it is an anomaly as the only callee (determine_max_movement at
gcc/tree-ssa-loop-im.c:749) tests the asserted result against NULL. (If
the assert really were correct then the else statement is redundant
(dead code) at line gcc/tree-ssa-loop-im.c:758).
I think the else is correct and the assert wrong as the gnu modula-2
front end can provoke the assert to fail (thus needing the else statement).
I've run the C regression tests and there are no changes in the results
if the patch is applied.
regards,
Gaius
--- gcc-versionno-orig/gcc/tree-ssa-loop-im.c 2017-06-01 17:06:50.228216946
+0100
+++ gcc-versionno/gcc/tree-ssa-loop-im.c2017-06-01 21:34:55.623992245
+0100
@@ -602,7 +602,6 @@
hash = iterative_hash_expr (*mem, 0);
ref = memory_accesses.refs->find_with_hash (*mem, hash);
- gcc_assert (ref != NULL);
return ref;
}
@@ -2631,5 +2630,3 @@
{
return new pass_lim (ctxt);
}
-
-
Re: [C++ PATCH] lang_decl selector & decomposition
On 06/05/2017 06:28 AM, Jakub Jelinek wrote: So unless cxx_dup_lang_specific_decl or copy_decl or tsubst_decl clears the DECL_DECOMP_BASE for the short time before cp_finish_decomp is called, DECL_DECOMP_BASE will be "wrong" for a short time, but it shouldn't matter, nothing should be querying it in between. Setting it to NULL wouldn't be correct either anyway. Ok, thanks for checking. That also explains why we can meet VAR_DECLs that already have a decomp_lang_decl extension. nathan -- Nathan Sidwell
Re: Containers default initialization
On 04/06/17 22:26 +0200, François Dumont wrote:
Hi
I have eventually adapt the test to all containers and the result
is successful for map/set/unordered_map/unordered_set. It is failing
for deque/list/forward_list/vector/vector.
I even try to change the test to look at the difference between an
explicit call to the default constructor done through the placement
new call and an implicit call done on normal declaration. I wondered
if we would have the same kind of difference we have between a int i;
and a int i(); I tried to set the stack to ~0 before declaring the
instance. I know there is no guarantee on the content of the stack for
the following declaration but do you think it is reliable enough to
commit it ?
Ok to commit the successful tests ?
No, I'm seeing failures for some of these if I add
// { dg-options "-O0" }
Franckly I don't understand the result of those tests. I would
have expect map/set to fail and others to succeed. We might need help
from compiler guys, no ?
I think your tests are just insufficient. With optimisation enabled
(the testsuite uses -O2 by default) the compiler can remove the memset
just before the __aligned_buffer goes out of scope, because it is
unobservable in a correct program. This is similar to the situation
described at https://gcc.gnu.org/gcc-6/porting_to.html#flifetime-dse
If I change the placement new-expressions to use default-init instead
of value-init and use -O0 then I see all four tests FAIL here:
test_type *tmp = ::new(buf._M_addr()) test_type; // not test_type()
VERIFY( tmp->get_allocator().state == 0 ); // FAIL
I haven't looked into why you're seeing failures for other containers,
but I'm definitely seeing the failures that I expected for maps and
sets.
Re: [PATCH] gcc::context creation
On Mon, Jun 05, 2017 at 06:57:47AM -0400, Nathan Sidwell wrote:
> On 06/05/2017 05:46 AM, Jakub Jelinek wrote:
>
> > Here is a patch to implement that. I chose to keep the 3 dumps predefined
> > dumps and just tweak their registered number, otherwise the changes would be
> > bigger.
>
> Yeah, that can wait for another day.
>
> > The patch additionally renames TDI_generic to TDI_gimple (to match
> > the dump name and content) and because we have more than 256 dumps these
> > days and we register them all even at -O0, the patch avoids 4 unnecessary
> > reallocations. Ok for trunk if testing passes?
>
> LGTM, two nits below. (I can't approve though)
>
> > +/* Allow languages and middle-end to register their dumps before the
> > + optimization passes. */
> blank line needed here, I think?
Whether there is a blank between function and its function comment
is something we aren't consistent in, but it seems that in dumpfile.c
there is a blank line, so I'm adjusting.
> > +void
> > +gcc::dump_manager::
> > +register_dumps ()
> > +{
> > + lang_hooks.register_dumps (this);
> > + /* If this assert fails, some FE registered more than
> > + FIRST_ME_AUTO_NUMBERED_DUMP - FIRST_AUTO_NUMBERED_DUMP
> > + dump files. Bump FIRST_ME_AUTO_NUMBERED_DUMP accordingly. */
> > + gcc_assert (m_next_dump <= FIRST_ME_AUTO_NUMBERED_DUMP);
> > + if (m_next_dump < FIRST_ME_AUTO_NUMBERED_DUMP)
> > +m_next_dump = FIRST_ME_AUTO_NUMBERED_DUMP;
>
> Unconditionally set it? there's the protecting assert. It's almost as if
> you want 'else gcc_unreachable ()' (with suitable morph of < to <=).
Unconditional set is fine, else gcc_unreachable (); looks ugly to me.
Here is updated patch:
2017-06-05 Jakub Jelinek
* dumpfile.h (enum tree_dump_index): Rename TDI_generic to
TDI_gimple.
(class dump_manager): Add register_dumps method.
* dumpfile.c: Include langhooks.h.
(dump_files): Use 0 instead of 3/4/5 for TDI_{original,gimple,nested}.
(FIRST_AUTO_NUMBERED_DUMP): Decrease to 1.
(FIRST_ME_AUTO_NUMBERED_DUMP): Define.
(dump_manager::dump_register): Start with 512 entries instead of 32.
(dump_manager::register_dumps): New method.
* toplev.c (general_init): Instead of invoking register_dumps
langhook, invoke register_dumps method on the dump manager.
* gimplify.c (gimplify_function_tree): Use TDI_gimple instead of
TDI_generic.
* gimple-parser.c (c_parser_parse_gimple_body): Use TDI_gimple instead
of TDI_generic.
--- gcc/dumpfile.h.jj 2017-06-02 09:01:10.0 +0200
+++ gcc/dumpfile.h 2017-06-05 11:20:07.563008969 +0200
@@ -31,7 +31,7 @@ enum tree_dump_index
TDI_inheritance, /* dump type inheritance graph. */
TDI_clones, /* dump IPA cloning decisions. */
TDI_original,/* dump each function before optimizing
it */
- TDI_generic, /* dump each function after genericizing it */
+ TDI_gimple, /* dump each function after gimplifying it */
TDI_nested, /* dump each function after unnesting it */
TDI_lang_all,/* enable all the language dumps. */
@@ -212,6 +212,11 @@ public:
dump_register (const char *suffix, const char *swtch, const char *glob,
dump_kind dkind, int optgroup_flags, bool take_ownership);
+ /* Allow languages and middle-end to register their dumps before the
+ optimization passes. */
+ void
+ register_dumps ();
+
/* Return the dump_file_info for the given phase. */
struct dump_file_info *
get_dump_file_info (int phase) const;
--- gcc/dumpfile.c.jj 2017-06-02 09:01:10.0 +0200
+++ gcc/dumpfile.c 2017-06-05 11:21:27.381013150 +0200
@@ -27,6 +27,7 @@ along with GCC; see the file COPYING3.
#include "dumpfile.h"
#include "context.h"
#include "tree-cfg.h"
+#include "langhooks.h"
/* If non-NULL, return one past-the-end of the matching SUBPART of
the WHOLE string. */
@@ -59,10 +60,11 @@ static struct dump_file_info dump_files[
DUMP_FILE_INFO (".cgraph", "ipa-cgraph", DK_ipa, 0),
DUMP_FILE_INFO (".type-inheritance", "ipa-type-inheritance", DK_ipa, 0),
DUMP_FILE_INFO (".ipa-clones", "ipa-clones", DK_ipa, 0),
- DUMP_FILE_INFO (".original", "tree-original", DK_tree, 3),
- DUMP_FILE_INFO (".gimple", "tree-gimple", DK_tree, 4),
- DUMP_FILE_INFO (".nested", "tree-nested", DK_tree, 5),
-#define FIRST_AUTO_NUMBERED_DUMP 3
+ DUMP_FILE_INFO (".original", "tree-original", DK_tree, 0),
+ DUMP_FILE_INFO (".gimple", "tree-gimple", DK_tree, 0),
+ DUMP_FILE_INFO (".nested", "tree-nested", DK_tree, 0),
+#define FIRST_AUTO_NUMBERED_DUMP 1
+#define FIRST_ME_AUTO_NUMBERED_DUMP 3
DUMP_FILE_INFO (NULL, "lang-all", DK_lang, 0),
DUMP_FILE_INFO (NULL, "tree-all", DK_tree, 0),
@@ -179,7 +181,7 @@ dump_register (const char *suffix, const
if (count >= m_extra_dump_files_alloced)
Re: [PATCH] Optimize std::advance for single increments
On 05/06/17 12:47 +0200, Marc Glisse wrote: On Mon, 5 Jun 2017, Jonathan Wakely wrote: LWG 2931 suggests a new overload of std::next for the default n==1 case, because it can be simpler for some random access iterators to do ++i than i += 1, e.g. for std::deque::iterator. I've suggested we close that issue as NAD, because we don't need a new overload to do that, and here's the proof. This performs the same optimisation when the argument to __advance is known at compile-time to be +1 or -1, as is the case when called from std::next(i) or std::prev(i). Do you want to optimize the case __n == 0 as well? ;-) I didn't think of that, but we certainly could. With __builtin_constant_p there's no downside to adding that.
RE: [PATCH,testsuite] Add check_effective_target_rdynamic and use it in g++.dg/lto/pr69589_0.C.
Hi Renlin,
>
> Thanks for fixing this! Do you have plan to backport the fix to gcc-6 branch?
>
I am happy to backport it.
I've rebased the patch on top of the gcc-6 branch and attached it below.
Rainer, is this OK for gcc-6?
Regards,
Toma Tabacu
gcc/ChangeLog:
Backported from mainline
2017-03-09 Toma Tabacu
* doc/sourcebuild.texi (Effective-Target Keywords, Other attributes):
Document rdynamic.
gcc/testsuite/ChangeLog:
Backported from mainline
2017-03-09 Toma Tabacu
* g++.dg/lto/pr69589_0.C: Add dg-require-effective-target for
rdynamic.
* lib/target-supports.exp (check_effective_target_rdynamic):
New proc.
Index: gcc/doc/sourcebuild.texi
===
--- gcc/doc/sourcebuild.texi(revision 248876)
+++ gcc/doc/sourcebuild.texi(working copy)
@@ -1950,6 +1950,9 @@
@item pie
Target supports @option{-pie}, @option{-fpie} and @option{-fPIE}.
+@item rdynamic
+Target supports @option{-rdynamic}.
+
@item section_anchors
Target supports section anchors.
Index: gcc/testsuite/lib/target-supports.exp
===
--- gcc/testsuite/lib/target-supports.exp (revision 248876)
+++ gcc/testsuite/lib/target-supports.exp (working copy)
@@ -1424,6 +1424,14 @@
} "-static"]
}
+# Return 1 if we can use the -rdynamic option, 0 otherwise.
+
+proc check_effective_target_rdynamic { } {
+ return [check_no_compiler_messages rdynamic executable {
+ int main() { return 0; }
+ } "-rdynamic"]
+}
+
# Return 1 if cilk-plus is supported by the target, 0 otherwise.
proc check_effective_target_cilkplus { } {
Index: gcc/testsuite/g++.dg/lto/pr69589_0.C
===
--- gcc/testsuite/g++.dg/lto/pr69589_0.C(revision 248876)
+++ gcc/testsuite/g++.dg/lto/pr69589_0.C(working copy)
@@ -1,6 +1,7 @@
// { dg-lto-do link }
// { dg-lto-options "-O2 -rdynamic" }
// { dg-extra-ld-options "-r -nostdlib" }
+// { dg-require-effective-target rdynamic }
#pragma GCC visibility push(hidden)
struct A { int &operator[] (long); };
template struct B;
[PATCH] Add missing entry for -Wduplicated-branches
Hi, the warning option -Wduplicated-branches added by Marek in r244705 https://gcc.gnu.org/viewcvs?rev=244705&root=gcc&view=rev lacks an entry in the warning list of doc/invoke.texi. The patch below fixes that. OK for trunk? What about the GCC 7 branch? Regards, Volker 2017-06-05 Volker Reichelt * doc/invoke.texi (-Wduplicated-branches): Add to warning list. Index: gcc/doc/invoke.texi === --- gcc/doc/invoke.texi (revision 248863) +++ gcc/doc/invoke.texi (working copy) @@ -273,7 +273,8 @@ -Wno-deprecated -Wno-deprecated-declarations -Wno-designated-init @gol -Wdisabled-optimization @gol -Wno-discarded-qualifiers -Wno-discarded-array-qualifiers @gol --Wno-div-by-zero -Wdouble-promotion -Wduplicated-cond @gol +-Wno-div-by-zero -Wdouble-promotion @gol +-Wduplicated-branches -Wduplicated-cond @gol -Wempty-body -Wenum-compare -Wno-endif-labels -Wexpansion-to-defined @gol -Werror -Werror=* -Wextra-semi -Wfatal-errors @gol -Wfloat-equal -Wformat -Wformat=2 @gol ===
Re: [PATCH] gcc::context creation
On 06/05/2017 07:47 AM, Jakub Jelinek wrote: Whether there is a blank between function and its function comment is something we aren't consistent in, but it seems that in dumpfile.c there is a blank line, so I'm adjusting. Ok (I only know the blank line rule because someone nitted one of my patches) +#define FIRST_AUTO_NUMBERED_DUMP 1 +#define FIRST_ME_AUTO_NUMBERED_DUMP 3 When you commit, could you set this to 4 (at least). Right now you have no headroom for C++ FE changes. And I have a branch with a new lang dump :) nathan -- Nathan Sidwell
Re: [PATCH] gcc::context creation
On Mon, Jun 05, 2017 at 08:46:21AM -0400, Nathan Sidwell wrote: > On 06/05/2017 07:47 AM, Jakub Jelinek wrote: > > > Whether there is a blank between function and its function comment > > is something we aren't consistent in, but it seems that in dumpfile.c > > there is a blank line, so I'm adjusting. > > Ok (I only know the blank line rule because someone nitted one of my > patches) > > > > +#define FIRST_AUTO_NUMBERED_DUMP 1 > > +#define FIRST_ME_AUTO_NUMBERED_DUMP 3 > > When you commit, could you set this to 4 (at least). Right now you have no > headroom for C++ FE changes. And I have a branch with a new lang dump :) It was the intent that there is no unnecessary gap, the difference between those two should be simply the maximum any FE registers. So, on your branch you'd bump it to 4 and on trunk when merging your branch. Jakub
Re: [PATCH] Finish implementing P0426R1 "Constexpr for std::char_traits" for C++17
On 23/04/17 19:33 +0100, Pedro Alves wrote:
On 04/23/2017 06:54 PM, Pedro Alves wrote:
Hi!
As I had suggested in PR c++/80265, here's a patch that uses
__builtin_constant_p to tell whether we can defer to a constexpr
algorithm, which avoids having to wait for compiler support.
Unfortunately I ran out of cycles today to run a full bootstrap/regtest
cycle, but constexpr_functions_c++17.cc passes cleanly on
x86_64 GNU/Linux at least.
If this looks like a reasonable approach, I can maybe try running
full tests on the gcc compile farm next week.
WDYT?
Reading the patch via the list made me spot something
that was incorrect.
- static char_type*
+ static _GLIBCXX17_CONSTEXPR char_type*
assign(char_type* __s, size_t __n, char_type __a)
{
+#if __cplusplus > 201402
+ if (__constant_string_p(__s)
This should probably have been __constant_char_array_p, but now
that I look again at the paper, I see I got myself confused -- this
assign overload is not meant to be constexpr in the first place.
So here's an updated patch that drops that bit. (Same testing
as before.)
Pedro, this is OK for trunk now we're in stage 1. Please go ahead and
commit it - thanks.
It's probably safe for gcc-7-branch too, but let's leave it on trunk
for a while first.
Re: [PATCH] Add missing entry for -Wduplicated-branches
On Mon, Jun 05, 2017 at 02:38:11PM +0200, Volker Reichelt wrote: > Hi, > > the warning option -Wduplicated-branches added by Marek in r244705 > https://gcc.gnu.org/viewcvs?rev=244705&root=gcc&view=rev > lacks an entry in the warning list of doc/invoke.texi. Oops. > The patch below fixes that. > > OK for trunk? > What about the GCC 7 branch? Can't approve, but I think this is obvious, so I'd just go ahead with this. > Regards, > Volker > > 2017-06-05 Volker Reichelt > > * doc/invoke.texi (-Wduplicated-branches): Add to warning list. > > Index: gcc/doc/invoke.texi > === > --- gcc/doc/invoke.texi (revision 248863) > +++ gcc/doc/invoke.texi (working copy) > @@ -273,7 +273,8 @@ > -Wno-deprecated -Wno-deprecated-declarations -Wno-designated-init @gol > -Wdisabled-optimization @gol > -Wno-discarded-qualifiers -Wno-discarded-array-qualifiers @gol > --Wno-div-by-zero -Wdouble-promotion -Wduplicated-cond @gol > +-Wno-div-by-zero -Wdouble-promotion @gol > +-Wduplicated-branches -Wduplicated-cond @gol > -Wempty-body -Wenum-compare -Wno-endif-labels -Wexpansion-to-defined @gol > -Werror -Werror=* -Wextra-semi -Wfatal-errors @gol > -Wfloat-equal -Wformat -Wformat=2 @gol > === > Marek
Re: [PATCH] gcc::context creation
On 06/05/2017 08:50 AM, Jakub Jelinek wrote: It was the intent that there is no unnecessary gap, the difference between those two should be simply the maximum any FE registers. So, on your branch you'd bump it to 4 and on trunk when merging your branch. I can live with that. nathan -- Nathan Sidwell
[Patch, Fortran, OOP] PR 70601: [5/6/7/8 Regression] [OOP] ICE on procedure pointer component call
Hi all,
I have just committed the attached one-line patch to trunk as obvious.
It fixes an ICE-on-valid regression with procedure-pointer components.
I plan to backport it to the release branches soon (if there are no
objections) ...
Cheers,
Janus
Index: gcc/testsuite/gfortran.dg/proc_ptr_comp_50.f90
===
--- gcc/testsuite/gfortran.dg/proc_ptr_comp_50.f90 (nonexistent)
+++ gcc/testsuite/gfortran.dg/proc_ptr_comp_50.f90 (revision 248878)
@@ -0,0 +1,26 @@
+! { dg-do compile }
+!
+! PR 70601: [5/6/7 Regression] [OOP] ICE on procedure pointer component call
+!
+! Contributed by zmi
+
+program test
+ implicit none
+
+ type :: concrete_type
+procedure (run_concrete_type), pointer :: run
+ end type
+
+ type(concrete_type), allocatable :: concrete
+
+ allocate(concrete)
+ concrete % run => run_concrete_type
+ call concrete % run()
+
+contains
+
+ subroutine run_concrete_type(this)
+ class(concrete_type) :: this
+ end subroutine
+
+end
Index: gcc/testsuite/ChangeLog
===
--- gcc/testsuite/ChangeLog (revision 248877)
+++ gcc/testsuite/ChangeLog (revision 248878)
@@ -1,3 +1,8 @@
+2017-06-05 Janus Weil
+
+ PR fortran/70601
+ * gfortran.dg/proc_ptr_comp_50.f90: New test.
+
2017-06-05 Nicolas Koenig
PR fortran/35339
Index: gcc/fortran/trans-expr.c
===
--- gcc/fortran/trans-expr.c(revision 248877)
+++ gcc/fortran/trans-expr.c(revision 248878)
@@ -6132,7 +6132,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol *
after use. This necessitates the creation of a temporary to
hold the result to prevent duplicate calls. */
if (!byref && sym->ts.type != BT_CHARACTER
- && sym->attr.allocatable && !sym->attr.dimension)
+ && sym->attr.allocatable && !sym->attr.dimension && !comp)
{
tmp = gfc_create_var (TREE_TYPE (se->expr), NULL);
gfc_add_modify (&se->pre, tmp, se->expr);
Index: gcc/fortran/ChangeLog
===
--- gcc/fortran/ChangeLog (revision 248877)
+++ gcc/fortran/ChangeLog (revision 248878)
@@ -1,3 +1,9 @@
+2017-06-05 Janus Weil
+
+ PR fortran/70601
+ * trans-expr.c (gfc_conv_procedure_call): Fix detection of allocatable
+ function results.
+
2017-06-05 Nicolas Koenig
PR fortran/35339
Re: RFC: [PATCH] Add warn_if_not_aligned attribute
The new attribute needs documentation. Should the test be in c-c++-common or does this feature not support C++? -- Joseph S. Myers jos...@codesourcery.com
Re: Reorgnanization of profile count maintenance code, part 1
I'm seeing a build failure for mips64-linux-gnu: In file included from /scratch/jmyers/glibc-bot/src/gcc/gcc/config/mips/frame-header-opt.c:35:0: /scratch/jmyers/glibc-bot/src/gcc/gcc/cfg.h:106:64: error: 'profile_count' has not been declared extern void update_bb_profile_for_threading (basic_block, int, profile_count, edge); ^ /scratch/jmyers/glibc-bot/src/gcc/gcc/cfg.h:111:11: error: 'profile_count' has not been declared profile_count, profile_count); ^ /scratch/jmyers/glibc-bot/src/gcc/gcc/cfg.h:111:26: error: 'profile_count' has not been declared profile_count, profile_count); ^ /scratch/jmyers/glibc-bot/src/gcc/gcc/config/mips/t-mips:25: recipe for target 'frame-header-opt.o' failed make[3]: *** [frame-header-opt.o] Error 1 -- Joseph S. Myers jos...@codesourcery.com
Re: Reorgnanization of profile count maintenance code, part 1
Hi, sorry for that. I am testing the obvious change to add #include "profile-count.h" into relevant target files. Honza > I'm seeing a build failure for mips64-linux-gnu: > > In file included from > /scratch/jmyers/glibc-bot/src/gcc/gcc/config/mips/frame-header-opt.c:35:0: > /scratch/jmyers/glibc-bot/src/gcc/gcc/cfg.h:106:64: error: 'profile_count' > has not been declared > extern void update_bb_profile_for_threading (basic_block, int, > profile_count, edge); > ^ > /scratch/jmyers/glibc-bot/src/gcc/gcc/cfg.h:111:11: error: 'profile_count' > has not been declared >profile_count, profile_count); >^ > /scratch/jmyers/glibc-bot/src/gcc/gcc/cfg.h:111:26: error: 'profile_count' > has not been declared >profile_count, profile_count); > ^ > /scratch/jmyers/glibc-bot/src/gcc/gcc/config/mips/t-mips:25: recipe for > target 'frame-header-opt.o' failed > make[3]: *** [frame-header-opt.o] Error 1 > > -- > Joseph S. Myers > jos...@codesourcery.com
Re: [PATCH] C++ parser: Fix typos in error messages
On Tue, 2017-05-02 at 08:05 +0200, Volker Reichelt wrote:
> Hi,
>
> the following patch fixes two typos in error messages of the C++
> parser
> (which have gone unnoticed since GCC 3.4.0).
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu.
> OK for trunk?
> Should this go also to GCC 7.2?
>
> Regards,
> Volker
>
>
> 2017-05-01 Volker Reichelt
>
> * parser.c (cp_parser_base_specifier): Fix typos in error
> messages.
>
> Index: gcc/cp/parser.c
> ===
> --- gcc/cp/parser.c (revision 247445)
> +++ gcc/cp/parser.c (working copy)
> @@ -23716,7 +23716,7 @@
> if (virtual_p && !duplicate_virtual_error_issued_p)
> {
> cp_parser_error (parser,
> - "% specified more than once
> in base-specified");
> + "% specified more than once
> in base-specifier");
> duplicate_virtual_error_issued_p = true;
> }
>
> @@ -23736,7 +23736,7 @@
> && !duplicate_access_error_issued_p)
> {
> cp_parser_error (parser,
> - "more than one access specifier in
> base-specified");
> + "more than one access specifier in
> base-specifier");
> duplicate_access_error_issued_p = true;
> }
>
> ===
OK for both (with my "diagnostic messages" maintainer hat on).
Dave
Re: Reorgnanization of profile count maintenance code, part 1
Hi, I have committed the following fix. Honza Index: ChangeLog === --- ChangeLog (revision 248878) +++ ChangeLog (working copy) @@ -1,5 +1,10 @@ 2017-06-05 Jan Hubicka + * config/mips/frame-header-opt.c: Include profile-count.h. + * config/riscv/riscv.c: Include profile-count.h + +2017-06-05 Jan Hubicka + * tree-ssa-loop-im.c (execute_sm_if_changed): Add FLAG_BBS parameter; update profile. (sm_set_flag_if_changed): Add bbs field. Index: config/mips/frame-header-opt.c === --- config/mips/frame-header-opt.c (revision 248871) +++ config/mips/frame-header-opt.c (working copy) @@ -32,6 +32,7 @@ along with GCC; see the file COPYING3. #include "tree-pass.h" #include "target.h" #include "target-globals.h" +#include "profile-count.h" #include "cfg.h" #include "cgraph.h" #include "function.h" Index: config/riscv/riscv.c === --- config/riscv/riscv.c(revision 248871) +++ config/riscv/riscv.c(working copy) @@ -73,6 +73,7 @@ along with GCC; see the file COPYING3. #include "common/common-target.h" #include "langhooks.h" #include "dominance.h" +#include "profile-count.h" #include "cfg.h" #include "cfgrtl.h" #include "cfganal.h"
Re: [PATCH] add more detail to -Wconversion and -Woverflow (PR 80731)
Hi,
newlib-stdint.h defines UINT32_TYPE as "long unsigned int" for ILP32
targets. This leads such target to warn on "long unsigned int"
conversation rather than "unsigned int". This patch modifies the
dg-warning regexp to allow an option "long " prefix in the warning,
thereby working for LP64 and ILP32 targets.
ChangeLog entry is as follows:
*** gcc/testsuite/ChangeLog ***
2017-06-05 Thomas Preud'homme
* gcc.dg/utf16-4.c: Accept "long unsigned int" as well as
"unsigned int" in conversation warning on line 15.
Is it ok for trunk?
The dg-warning PASSes once this patch is applied.
Best regards,
Thomas
On 04/06/17 23:54, Martin Sebor wrote:
On 06/02/2017 11:11 AM, Renlin Li wrote:
Hi Martin,
I noticed the following failures after your change r248431.
FAIL: c-c++-common/Wfloat-conversion.c -Wc++-compat (test for
warnings, line 42)
FAIL: c-c++-common/Wfloat-conversion.c -Wc++-compat (test for
warnings, line 43)
It happens on arm target which is not a large_long_double target.
The patch here add the missing target selector. After the change, those
test
won't checked in arm target.
Here I have a simple fix to it. Okay to commit?
r248431 wasn't meant to change when any of these warnings fire,
only their text and the amount of detail included in them. The
removal of the { target large_long_double } selector was by
accident.
Please go ahead and commit your fix. Thanks!
gcc/testsuite/ChangeLog:
2017-06-02 Renlin Li
* c-c++-common/Wfloat-conversion.c: Add large_long_double target
selector to related line.
And there is another failure:
FAIL: gcc.dg/utf16-4.c (test for warnings, line 15)
The warning message is slightly different from expected.
utf16-4.c:10:15: warning: character constant too long for its type
utf16-4.c:15:15: warning: conversion from 'long unsigned int' to
'char16_t {aka short unsigned int}' changes value from '410401' to '17185'
The test passes for me now. The initial commit had introduced
bug 80731 but it's recently been fixed. Can you please try
again with the latest sources?
Martin
diff --git a/gcc/testsuite/gcc.dg/utf16-4.c b/gcc/testsuite/gcc.dg/utf16-4.c
index f9ebd61845a15fc657a2f906d3f684768993df44..e2f115ebc813ba380718254db699db8ef96e532b 100644
--- a/gcc/testsuite/gcc.dg/utf16-4.c
+++ b/gcc/testsuite/gcc.dg/utf16-4.c
@@ -12,7 +12,7 @@ char16_t c2 = u'\U00064321'; /* { dg-warning "constant too long" } */
char16_t c3 = 'a';
char16_t c4 = U'a';
char16_t c5 = U'\u2029';
-char16_t c6 = U'\U00064321'; /* { dg-warning "conversion from .unsigned int. to .char16_t {aka short unsigned int}. changes value from .410401. to .17185." } */
+char16_t c6 = U'\U00064321'; /* { dg-warning "conversion from .(long )?unsigned int. to .char16_t {aka short unsigned int}. changes value from .410401. to .17185." } */
char16_t c7 = L'a';
char16_t c8 = L'\u2029';
char16_t c9 = L'\U00064321'; /* { dg-warning "conversion" "" { target { 4byte_wchar_t } } } */
Re: [PATCH] warn on mem calls modifying objects of non-trivial types (PR 80560)
Maybe I should use a different approach and instead of trying
to see if a function is deleted use trivially_xible to see if
it's usable. That will mean changing the diagnostics from
"with a deleted special function" to "without trivial special
function" but it will avoid calling synthesized_method_walk
while still avoiding giving bogus suggestions.
Actually, this would check for one possible argument type and not
others, so I think it's better to keep looking at the declarations. You
can do that by just looking them up (lookup_fnfields_slot) and iterating
over them, you don't need to call synthesized_method_walk.
You mean using trivially_xible might check assignability or copy
constructibility from const T& but not from T& (or the other way
around), and you think both (or perhaps even other forms) should
be considered?
E.g., given:
struct S
{
S& operator= (const S&) = default;
void operator= (S&) = delete;
};
void f (S *d, const S *s)
{
memcpy(d, s, sizeof *d); // don't warn here
}
void g (S *d, S *s)
{
memcpy(d, s, sizeof *d); // but warn here
}
And your suggestion is to iterate over the assignment operator
(and copy ctor) overloads for S looking for one that's trivial,
public, and not deleted?
If that's it, I was thinking of just checking for the const T&
overload (as if by using std::is_trivially_copy_assignable()).
I don't mind trying the approach you suggest. It should be more
accurate. I just want to make sure we're on the same page.
Martin
Re: [PING**3] [PATCH] Force use of absolute path names for gcov
On 06/05/17 13:11, Nathan Sidwell wrote:
> +Compile the source files additionally with @option{-fprofile-abs-path}
> +to create absolute path names in the @file{.gcno} files. This allows
> +@command{gcov} to find the correct sources in projects with multiple
> +directories.
>
> I think the second sentence could be better. It's not that the sources
> are in different directories, it's that the compiler is invoked with
> different working directories. How about
>'This allows @command{gcov} to find the correct sources in projects
> where compilations occur with different working directories.'
>
> modify as you see fit.
>
done.
> gcov_write_filename (const char *filename)
> +{
> + char buf[1024];
> + size_t len;
>
> Ew. (a) bad fixed length (b) bad stack frame bloat. Please use malloc
> & MAX_PATH_LEN (or whatever the right #define is).
>
MAX_PATH is just a portability nightmare and may or may not be
defined or sometimes is called PATH_MAX etc.
But I see getcwd (NULL, 0) is used in gcov-tools already,
and although that is a posix extension, it seems to be safe to use,
because libiberty provides a fall-back.
So I would go for getcwd(NULL, 0) and xrealloc the result buffer
as needed.
> + if (profile_abs_path_flag && filename && filename[0]
>
> Can filename ever be null here? Can it ever be the empty string?
>
I am unable to prove that, if the call from coverage.c where
filename comes directly from expand_location and there are
numerous places like:
if (xloc.file)
chksum = coverage_checksum_string (chksum, xloc.file);
which tells me that xloc.file might be NULL, typically in
case of UNKNOWN_LOCATION.
I am anxious about empty string here because of the code in
#ifdef HAVE_DOS_BASED_FILE_SYSTEM does not work with empty string,
while the original gcov_write_string works fine with NULL string
and empty string.
> + if (getcwd (buf, sizeof (buf) - len - 1) != NULL)
>
> This is going to getcwd on every file (most likely). Isn't this already
> available somewhere?
>
>
No, at least grep does not see any other place, except in gcov-tool.c :(
So how about this new patch?
Is it OK for trunk?
Bernd.
gcc:
2017-04-21 Bernd Edlinger
* doc/invoke.texi: Document the -fprofile-abs-path option.
* common.opt (fprofile-abs-path): New option.
* gcov-io.h (gcov_write_filename): Declare.
* gcov-io.c (gcov_write_filename): New function.
* coverage.c (coverage_begin_function): Use gcov_write_filename.
* profile.c (output_location): Likewise.
gcc/testsuite:
2017-04-21 Bernd Edlinger
* gcc.misc-tests/gcov-1a.c: New test.
Index: gcc/common.opt
===
--- gcc/common.opt (revision 248852)
+++ gcc/common.opt (working copy)
@@ -1969,6 +1969,10 @@ fprofile
Common Report Var(profile_flag)
Enable basic program profiling code.
+fprofile-abs-path
+Common Report Var(profile_abs_path_flag)
+Generate absolute source path names for gcov.
+
fprofile-arcs
Common Report Var(profile_arc_flag)
Insert arc-based program profiling code.
Index: gcc/coverage.c
===
--- gcc/coverage.c (revision 248852)
+++ gcc/coverage.c (working copy)
@@ -663,7 +663,7 @@ coverage_begin_function (unsigned lineno_checksum,
gcov_write_unsigned (cfg_checksum);
gcov_write_string (IDENTIFIER_POINTER
(DECL_ASSEMBLER_NAME (current_function_decl)));
- gcov_write_string (xloc.file);
+ gcov_write_filename (xloc.file);
gcov_write_unsigned (xloc.line);
gcov_write_length (offset);
Index: gcc/doc/invoke.texi
===
--- gcc/doc/invoke.texi (revision 248852)
+++ gcc/doc/invoke.texi (working copy)
@@ -443,6 +443,7 @@ Objective-C and Objective-C++ Dialects}.
@item Program Instrumentation Options
@xref{Instrumentation Options,,Program Instrumentation Options}.
@gccoptlist{-p -pg -fprofile-arcs --coverage -ftest-coverage @gol
+-fprofile-abs-path @gol
-fprofile-dir=@var{path} -fprofile-generate -fprofile-generate=@var{path} @gol
-fsanitize=@var{style} -fsanitize-recover -fsanitize-recover=@var{style} @gol
-fasan-shadow-offset=@var{number} -fsanitize-sections=@var{s1},@var{s2},... @gol
@@ -10694,6 +10695,12 @@ additional @option{-ftest-coverage} option. You d
every source file in a program.
@item
+Compile the source files additionally with @option{-fprofile-abs-path}
+to create absolute path names in the @file{.gcno} files. This allows
+@command{gcov} to find the correct sources in projects where compilations
+occur with different working directories.
+
+@item
Link your object files with @option{-lgcov} or @option{-fprofile-arcs}
(the latter implies the former).
@@ -10737,6 +10744,13 @@ above for a description of @var{auxname} and instr
generate test coverage data. Coverage data matches the source files
more closely if you do not optimize.
[PING^2] re [PATCH v2] C++: Add fix-it hints for -Wold-style-cast
Ping re this patch:
https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00204.html
On Fri, 2017-05-26 at 15:35 -0400, David Malcolm wrote:
> On Wed, 2017-05-03 at 09:51 -0400, David Malcolm wrote:
> > On Thu, 2017-04-27 at 23:03 +0200, Marek Polacek wrote:
> > > On Thu, Apr 27, 2017 at 05:10:24PM -0400, David Malcolm wrote:
> > > > + /* First try const_cast. */
> > > > + trial = build_const_cast (dst_type, orig_expr, 0 /* complain
> > > > */);
> > > > + if (trial != error_mark_node)
> > > > +return "const_cast";
> > > > +
> > > > + /* If that fails, try static_cast. */
> > > > + trial = build_static_cast (dst_type, orig_expr, 0 /*
> > > > complain
> > > > */);
> > > > + if (trial != error_mark_node)
> > > > +return "static_cast";
> > > > +
> > > > + /* Finally, try reinterpret_cast. */
> > > > + trial = build_reinterpret_cast (dst_type, orig_expr, 0 /*
> > > > complain */);
> > > > + if (trial != error_mark_node)
> > > > +return "reinterpret_cast";
> > >
> > > I think you'll want tf_none instead of 0 /* complain */ in these.
> > >
> > > Marek
> >
> > Thanks.
> >
> > Here's an updated version of the patch.
> >
> > Changes since v1:
> > - updated expected fixit-formatting (the new fix-it printer in
> > r247548 handles this properly now)
> > - added new test cases as suggested by Florian
> > - use "tf_none" rather than "0 /* complain */"
> >
> > Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
> >
> > OK for trunk?
> >
> > gcc/cp/ChangeLog:
> > * parser.c (get_cast_suggestion): New function.
> > (maybe_add_cast_fixit): New function.
> > (cp_parser_cast_expression): Capture the location of the
> > closing
> > parenthesis. Call maybe_add_cast_fixit when emitting warnings
> > about old-style casts.
> >
> > gcc/testsuite/ChangeLog:
> > * g++.dg/other/old-style-cast-fixits.C: New test case.
> > ---
> > gcc/cp/parser.c| 93
> > -
> > gcc/testsuite/g++.dg/other/old-style-cast-fixits.C | 95
> > ++
> > 2 files changed, 186 insertions(+), 2 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/other/old-style-cast
> > -fixits.C
> >
> > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> > index 4714bc6..2f83aa9 100644
> > --- a/gcc/cp/parser.c
> > +++ b/gcc/cp/parser.c
> > @@ -8633,6 +8633,85 @@ cp_parser_tokens_start_cast_expression
> > (cp_parser *parser)
> > }
> > }
> >
> > +/* Try to find a legal C++-style cast to DST_TYPE for ORIG_EXPR,
> > trying them
> > + in the order: const_cast, static_cast, reinterpret_cast.
> > +
> > + Don't suggest dynamic_cast.
> > +
> > + Return the first legal cast kind found, or NULL otherwise. */
> > +
> > +static const char *
> > +get_cast_suggestion (tree dst_type, tree orig_expr)
> > +{
> > + tree trial;
> > +
> > + /* Reuse the parser logic by attempting to build the various
> > kinds
> > of
> > + cast, with "complain" disabled.
> > + Identify the first such cast that is valid. */
> > +
> > + /* Don't attempt to run such logic within template processing.
> > */
> > + if (processing_template_decl)
> > +return NULL;
> > +
> > + /* First try const_cast. */
> > + trial = build_const_cast (dst_type, orig_expr, tf_none);
> > + if (trial != error_mark_node)
> > +return "const_cast";
> > +
> > + /* If that fails, try static_cast. */
> > + trial = build_static_cast (dst_type, orig_expr, tf_none);
> > + if (trial != error_mark_node)
> > +return "static_cast";
> > +
> > + /* Finally, try reinterpret_cast. */
> > + trial = build_reinterpret_cast (dst_type, orig_expr, tf_none);
> > + if (trial != error_mark_node)
> > +return "reinterpret_cast";
> > +
> > + /* No such cast possible. */
> > + return NULL;
> > +}
> > +
> > +/* If -Wold-style-cast is enabled, add fix-its to RICHLOC,
> > + suggesting how to convert a C-style cast of the form:
> > +
> > + (DST_TYPE)ORIG_EXPR
> > +
> > + to a C++-style cast.
> > +
> > + The primary range of RICHLOC is asssumed to be that of the
> > original
> > + expression. OPEN_PAREN_LOC and CLOSE_PAREN_LOC give the
> > locations
> > + of the parens in the C-style cast. */
> > +
> > +static void
> > +maybe_add_cast_fixit (rich_location *rich_loc, location_t
> > open_paren_loc,
> > + location_t close_paren_loc, tree orig_expr,
> > + tree dst_type)
> > +{
> > + /* This function is non-trivial, so bail out now if the warning
> > isn't
> > + going to be emitted. */
> > + if (!warn_old_style_cast)
> > +return;
> > +
> > + /* Try to find a legal C++ cast, trying them in order:
> > + const_cast, static_cast, reinterpret_cast. */
> > + const char *cast_suggestion = get_cast_suggestion (dst_type,
> > orig_expr);
> > + if (!cast_suggestion)
> > +return;
> > +
> > + /* Replace the open paren with "CAST_SUGGESTION<". */
> > + pretty_printer pp;
> > + pp_printf (&pp, "%s<", cast_suggestion);
> > +
Re: [PATCH] add more detail to -Wconversion and -Woverflow (PR 80731)
On 06/05/2017 10:05 AM, Thomas Preudhomme wrote:
Hi,
newlib-stdint.h defines UINT32_TYPE as "long unsigned int" for ILP32
targets. This leads such target to warn on "long unsigned int"
conversation rather than "unsigned int". This patch modifies the
dg-warning regexp to allow an option "long " prefix in the warning,
thereby working for LP64 and ILP32 targets.
Ah, that explains it.
ChangeLog entry is as follows:
*** gcc/testsuite/ChangeLog ***
2017-06-05 Thomas Preud'homme
* gcc.dg/utf16-4.c: Accept "long unsigned int" as well as
"unsigned int" in conversation warning on line 15.
Is it ok for trunk?
The dg-warning PASSes once this patch is applied.
Yes, relaxing the test to allow for both underlying char32_t types
makes sense.
Thanks
Martin
Best regards,
Thomas
On 04/06/17 23:54, Martin Sebor wrote:
On 06/02/2017 11:11 AM, Renlin Li wrote:
Hi Martin,
I noticed the following failures after your change r248431.
FAIL: c-c++-common/Wfloat-conversion.c -Wc++-compat (test for
warnings, line 42)
FAIL: c-c++-common/Wfloat-conversion.c -Wc++-compat (test for
warnings, line 43)
It happens on arm target which is not a large_long_double target.
The patch here add the missing target selector. After the change, those
test
won't checked in arm target.
Here I have a simple fix to it. Okay to commit?
r248431 wasn't meant to change when any of these warnings fire,
only their text and the amount of detail included in them. The
removal of the { target large_long_double } selector was by
accident.
Please go ahead and commit your fix. Thanks!
gcc/testsuite/ChangeLog:
2017-06-02 Renlin Li
* c-c++-common/Wfloat-conversion.c: Add large_long_double target
selector to related line.
And there is another failure:
FAIL: gcc.dg/utf16-4.c (test for warnings, line 15)
The warning message is slightly different from expected.
utf16-4.c:10:15: warning: character constant too long for its type
utf16-4.c:15:15: warning: conversion from 'long unsigned int' to
'char16_t {aka short unsigned int}' changes value from '410401' to
'17185'
The test passes for me now. The initial commit had introduced
bug 80731 but it's recently been fixed. Can you please try
again with the latest sources?
Martin
Re: [PATCH] PR libstdc++/80939 Remove unmeetable constexpr specifiers
On 02/06/17 12:19 -0700, Tim Shen wrote: On Fri, Jun 2, 2017 at 6:07 AM, Jonathan Wakely wrote: As the PR points out, we aren't qualifying calls to __ref_cast, and have 'constexpr' on function templates that can never be usable in constant expressions. Apology for the constexpr trolling, but that was not intentional. :) I'm curious why no tests break. Is it because constexpr in a template function is a no-op instead of a hard error, when the function definition is not constexpr? The patch looks good. Committed to trunk (I'll backport it to gcc-7-branch some time too).
More profile updating fixes
Hi,
here are less trivial updates to profile code which I did not bundle into
initial transition. Those are not bugs in old code, just new code needs
to track more.
profile-bootstrapped/regtested x86_64-linux, will commit it shortly.
* cfgexpand.c (expand_gimple_tailcall): Initialize profile of
new edge.
* ipa-inline.c (want_inline_self_recursive_call_p): Watch for missing
profile in callgraph edge.
* profile-count.h (apply_probability): If THIS is 0, then result is 0
(apply_scale): Likewise.
* tree-inline.c (copy_bb, copy_edges_for_bb, copy_cfg_body):
Also scale profile when inlining function with zero profile.
(initialize_cfun): Update exit block profile even when it is zero.
* tree-ssa-threadupdate.c (clear_counts_path): Handle correctly case
when profile is read.
Index: cfgexpand.c
===
--- cfgexpand.c (revision 248871)
+++ cfgexpand.c (working copy)
@@ -3850,8 +3850,8 @@ expand_gimple_tailcall (basic_block bb,
e = make_edge (bb, EXIT_BLOCK_PTR_FOR_FN (cfun), EDGE_ABNORMAL
| EDGE_SIBCALL);
- e->probability += probability;
- e->count += count;
+ e->probability = probability;
+ e->count = count;
BB_END (bb) = last;
update_bb_for_insn (bb);
Index: ipa-inline.c
===
--- ipa-inline.c(revision 248871)
+++ ipa-inline.c(working copy)
@@ -912,7 +912,7 @@ want_inline_self_recursive_call_p (struc
methods. */
else
{
- if (max_count > profile_count::zero ()
+ if (max_count > profile_count::zero () && edge->count.initialized_p ()
&& (edge->count.to_gcov_type () * 100
/ outer_node->count.to_gcov_type ()
<= PARAM_VALUE (PARAM_MIN_INLINE_RECURSIVE_PROBABILITY)))
@@ -920,7 +920,8 @@ want_inline_self_recursive_call_p (struc
reason = "profile of recursive call is too small";
want_inline = false;
}
- else if (max_count == profile_count::zero ()
+ else if ((max_count == profile_count::zero ()
+ || !edge->count.initialized_p ())
&& (edge->frequency * 100 / caller_freq
<= PARAM_VALUE (PARAM_MIN_INLINE_RECURSIVE_PROBABILITY)))
{
Index: profile-count.h
===
--- profile-count.h (revision 248871)
+++ profile-count.h (working copy)
@@ -221,6 +221,8 @@ public:
profile_count apply_probability (int prob) const
{
gcc_checking_assert (prob >= 0 && prob <= REG_BR_PROB_BASE);
+ if (*this == profile_count::zero ())
+ return *this;
if (!initialized_p ())
return profile_count::uninitialized ();
profile_count ret;
@@ -230,6 +232,8 @@ public:
/* Return *THIS * NUM / DEN. */
profile_count apply_scale (int64_t num, int64_t den) const
{
+ if (*this == profile_count::zero ())
+ return *this;
if (!initialized_p ())
return profile_count::uninitialized ();
profile_count ret;
@@ -243,7 +247,7 @@ public:
}
profile_count apply_scale (profile_count num, profile_count den) const
{
- if (*this == profile_count::zero ())
+ if (*this == profile_count::zero () || num == profile_count::zero ())
return profile_count::zero ();
if (!initialized_p () || !num.initialized_p () || !den.initialized_p ())
return profile_count::uninitialized ();
Index: tree-inline.c
===
--- tree-inline.c (revision 248871)
+++ tree-inline.c (working copy)
@@ -1763,7 +1763,8 @@ copy_bb (copy_body_data *id, basic_block
tree decl;
gcov_type freq;
basic_block prev;
- bool scale = num.initialized_p () && den.initialized_p () && den > 0;
+ bool scale = num.initialized_p ()
+ && (den > 0 || num == profile_count::zero ());
/* Search for previous copied basic block. */
prev = bb->prev_bb;
@@ -2211,7 +2212,8 @@ copy_edges_for_bb (basic_block bb, profi
gimple_stmt_iterator si;
int flags;
bool need_debug_cleanup = false;
- bool scale = num.initialized_p () && den.initialized_p () && den > 0;
+ bool scale = num.initialized_p ()
+ && (den > 0 || num == profile_count::zero ());
/* Use the indices from the original blocks to create edges for the
new ones. */
@@ -2472,7 +2474,7 @@ initialize_cfun (tree new_fndecl, tree c
*/
if (ENTRY_BLOCK_PTR_FOR_FN (src_cfun)->count.initialized_p ()
&& count.initialized_p ()
- && ENTRY_BLOCK_PTR_FOR_FN (src_cfun)->count > 0)
+ && ENTRY_BLOCK_PTR_FOR_FN (src_cfun)->count.initialized_p ())
{
ENTRY_BLOCK_PTR_FOR_FN (cfun)->count =
ENTRY_BLOCK_PTR_FOR_FN (src_cfun)->count.apply_scale (count,
@@ -2683,7 +2685,8 @@ copy_cfg_body (copy_body_data * id, prof
profile_c
Re: RFC: [PATCH] Add warn_if_not_aligned attribute
On Mon, Jun 5, 2017 at 8:11 AM, Joseph Myers wrote:
> The new attribute needs documentation. Should the test be in c-c++-common
This feature does support C++. But C++ compiler issues a slightly
different warning at a different location.
> or does this feature not support C++?
>
Here is the updated patch with documentation and a C++ test. This
patch caused a few testsuite failures:
FAIL: gcc.dg/compat/struct-align-1 c_compat_x_tst.o compile
/export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.dg/compat//struct-align-1.h:169:1:
warning: alignment 1 of 'struct B2_m_inner_p_outer' is less than 16
FAIL: g++.dg/torture/pr80334.C -O0 (test for excess errors)
/export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/torture/pr80334.C:4:8:
warning: alignment 1 of 'B' is less than 16
--
H.J.
From b427b82bf9e7e510a1c1d5a91aa8198dd8036232 Mon Sep 17 00:00:00 2001
From: "H.J. Lu"
Date: Fri, 20 Apr 2012 13:49:05 -0700
Subject: [PATCH] Add warn_if_not_aligned attribute
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
__attribute__((warn_if_not_aligned(N))) issues a warning if the field
in a struct or union is not aligned to N:
typedef unsigned long long __u64
__attribute__((aligned(4),warn_if_not_aligned(8)));
struct foo
{
int i1;
int i2;
__u64 x;
};
__u64 is aligned to 4 bytes. But inside struct foo, __u64 should be
aligned at 8 bytes. It is used to define struct foo in such a way that
struct foo has the same layout and x has the same alignment when __u64
is aligned at either 4 or 8 bytes.
Since struct foo is normally aligned to 4 bytes, a warning will be issued:
warning: alignment 4 of ‘struct foo’ is less than 8
Align struct foo to 8 bytes:
struct foo
{
int i1;
int i2;
__u64 x;
} __attribute__((aligned(8)));
silences the warning. It also warns the field with misaligned offset:
struct foo
{
int i1;
int i2;
int i3;
__u64 x;
} __attribute__((aligned(8)));
warning: ‘x’ offset 12 in ‘struct foo’ isn't aligned to 8
The same warning is also issued without the warn_if_not_aligned attribute
for the field with explicitly specified alignment in a packed struct or
union:
struct __attribute__ ((aligned (8))) S8 { char a[8]; };
struct __attribute__ ((packed)) S {
struct S8 s8;
};
warning: alignment 1 of ‘struct S’ is less than 8
gcc/
PR c/53037
* c-decl.c (finish_enum): Also copy TYPE_WARN_IF_NOT_ALIGN.
* print-tree.c (print_node): Support TYPE_WARN_IF_NOT_ALIGN.
* stor-layout.c (handle_warn_if_not_align): New.
(place_union_field): Call handle_warn_if_not_align.
(place_field): Call handle_warn_if_not_align. Copy
TYPE_WARN_IF_NOT_ALIGN.
(finish_builtin_struct): Copy TYPE_WARN_IF_NOT_ALIGN.
(layout_type): Likewise.
* tree.c (build_range_type_1): Likewise.
* tree-core.h (tree_type_common): Add warn_if_not_align. Set
spare to 18.
* tree.h (TYPE_WARN_IF_NOT_ALIGN): New.
(SET_TYPE_WARN_IF_NOT_ALIGN): Likewise.
* c-family/c-attribs.c (handle_warn_if_not_aligned_attribute): New.
(c_common_attribute_table): Add warn_if_not_aligned.
(handle_aligned_attribute): Renamed to ...
(common_handle_aligned_attribute): Remove argument, name, and add
argument, warn_if_not_aligned. Handle warn_if_not_aligned.
(handle_aligned_attribute): New.
* c/c-decl.c (finish_enum): Copy TYPE_WARN_IF_NOT_ALIGN.
* doc/extend.texi: Document warn_if_not_aligned attribute.
gcc/testsuite/
PR c/53037
* g++.dg/pr53037-1.C: New test.
* gcc.dg/pr53037-1.c: Likewise.
---
gcc/c-family/c-attribs.c | 55 +++---
gcc/c/c-decl.c | 2 +
gcc/doc/extend.texi | 50
gcc/print-tree.c | 6 ++-
gcc/stor-layout.c| 45 +
gcc/testsuite/g++.dg/pr53037-1.C | 84
gcc/testsuite/gcc.dg/pr53037-1.c | 84
gcc/tree-core.h | 3 +-
gcc/tree.c | 1 +
gcc/tree.h | 10 +
10 files changed, 331 insertions(+), 9 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/pr53037-1.C
create mode 100644 gcc/testsuite/gcc.dg/pr53037-1.c
diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 695c58c..a76f9f7 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -85,6 +85,8 @@ static tree handle_destructor_attribute (tree *, tree, tree, int, bool *);
static tree handle_mode_attribute (tree *, tree, tree, int, bool *);
static tree handle_section_attribute (tree *, tree, tree, int, bool *);
static tree handle_aligned_attribute (tree *, tree, tree, int, bool *);
+static tree handle_warn_if_not_aligned_attribute (tree *, tree, tree,
+ int, bool *);
static tree handle_weak_attribute (tree *, tree, tree, int, bool *) ;
static tree handle_noplt_attribute (tree *, tree, tree, int, bool *) ;
static tree handle_alias_ifunc_attribute (bool, tree *, tree, tree, bool *);
@@ -
Re: [PATCH] Add missing entry for -Wduplicated-branches
On 5 Jun, Marek Polacek wrote: > On Mon, Jun 05, 2017 at 02:38:11PM +0200, Volker Reichelt wrote: >> Hi, >> >> the warning option -Wduplicated-branches added by Marek in r244705 >> https://gcc.gnu.org/viewcvs?rev=244705&root=gcc&view=rev >> lacks an entry in the warning list of doc/invoke.texi. > > Oops. > >> The patch below fixes that. >> >> OK for trunk? >> What about the GCC 7 branch? > > Can't approve, but I think this is obvious, so I'd just go ahead with this. Seems reasonable. Committed as obvious to trunk and GCC 7 branch. Regards, Volker >> Regards, >> Volker >> >> 2017-06-05 Volker Reichelt >> >> * doc/invoke.texi (-Wduplicated-branches): Add to warning list. >> >> Index: gcc/doc/invoke.texi >> === >> --- gcc/doc/invoke.texi (revision 248863) >> +++ gcc/doc/invoke.texi (working copy) >> @@ -273,7 +273,8 @@ >> -Wno-deprecated -Wno-deprecated-declarations -Wno-designated-init @gol >> -Wdisabled-optimization @gol >> -Wno-discarded-qualifiers -Wno-discarded-array-qualifiers @gol >> --Wno-div-by-zero -Wdouble-promotion -Wduplicated-cond @gol >> +-Wno-div-by-zero -Wdouble-promotion @gol >> +-Wduplicated-branches -Wduplicated-cond @gol >> -Wempty-body -Wenum-compare -Wno-endif-labels -Wexpansion-to-defined @gol >> -Werror -Werror=* -Wextra-semi -Wfatal-errors @gol >> -Wfloat-equal -Wformat -Wformat=2 @gol >> === >> > > Marek
[PATCH] C/C++: fix quoting of "aka" typedef information (PR 62170)
PR 62170 describes a problem with how the quoting in pp_format
interacts with the "aka" information for typedefs in %qT for
the C family of frontends, and also now for %qH and %qI in the
C++ frontend.
Currently for %qT we print e.g.:
‘Py_ssize_t* {aka int*}’
^^ colorized as "quote"
i.e.
‘[START_COLOR]Py_ssize_t* {aka int*}[END_COLOR]’
when we should print:
‘Py_ssize_t*’ {aka ‘int*’}
^^^ colorized as "quote"
i.e.
‘[START_COLOR]Py_ssize_t*[END_COLOR]’ {aka ‘[START_COLOR]int*[END_COLOR]’}
where the opening and closing quote characters and colorization are
currently added by the 'q' handling within pp_format, adding the closing
quote unconditionally after whatever pp_format_decoder prints for 'T'
within "%qT".
This patch fixes the quoting by updating the %T handling in C and C++
and the %H/%I handling in C++ to insert the quoting appropriately.
It converts the "quote" param of the pp_format_decoder callback from
bool to bool *, allowing for the %T and %H/%I handlers to write
false back to it, to avoid printing the closing quote for the cases
like the above where a final trailing closing quote isn't needed.
It introduces pp_begin_quote/pp_end_quote to simplify this. These
take a "bool show_color", rather than using "pp_show_color (pp)"
since cxx_pp's pp_show_color isn't currently initialized (since
cxx_initialize_diagnostics happens before diagnostic_color_init).
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/c/ChangeLog:
PR c++/62170
* c-objc-common.c (c_tree_printer): Convert penultimate param from
bool to bool *. Within '%T' handling, if showing an "aka", use
"quoted" param to add appropriate quoting.
gcc/cp/ChangeLog:
PR c++/62170
* error.c (type_to_string): Add leading comment. Add params
"postprocessed", "quote", and "show_color", using them to fix
quoting of the "aka" for types involving typedefs.
(arg_to_string): Update for new params to type_to_string.
(cxx_format_postprocessor::handle): Likewise.
(cp_printer): Convert penultimate param from bool to bool *.
Update call to type_to_string and calls to
defer_phase_2_of_type_diff.
gcc/fortran/ChangeLog:
PR c++/62170
* error.c (gfc_notify_std): Convert "quoted" param from bool to
bool *.
gcc/ChangeLog:
PR c++/62170
* pretty-print.c (pp_format): Move quoting implementation to
pp_begin_quote and pp_end_quote. Update pp_format_decoder call
to pass address of "quote" local.
(pp_begin_quote): New function.
(pp_end_quote): New function.
* pretty-print.h (printer_fn): Convert penultimate param from bool
to bool *.
(pp_begin_quote): New decl.
(pp_end_quote): New decl.
* tree-diagnostic.c (default_tree_printer): Convert penultimate
param from bool to bool *.
* tree-diagnostic.h (default_tree_printer): Likewise.
gcc/testsuite/ChangeLog:
PR c++/62170
* g++.dg/cpp1z/direct-enum-init1.C: Update expected error messages
to reflect fixes to quoting.
* g++.dg/diagnostic/aka1.C: Likewise.
* g++.dg/diagnostic/aka2.C: New test case.
* g++.dg/parse/error55.C: Update expected error messages to
reflect fixes to quoting.
* g++.dg/warn/pr12242.C: Likewise.
* g++.old-deja/g++.mike/enum1.C: Likewise.
* gcc.dg/diag-aka-1.c: Likewise.
* gcc.dg/diag-aka-2.c: New test case.
* gcc.dg/pr13804-1.c: Update expected error messages to reflect
fixes to quoting.
* gcc.dg/pr56980.c: Likewise.
* gcc.dg/pr65050.c: Likewise.
* gcc.dg/redecl-14.c: Likewise.
* gcc.dg/utf16-4.c Likewise.
* gcc.target/i386/sse-vect-types.c (__m128d): Likewise.
* obj-c++.dg/invalid-type-1.mm: Likewise.
---
gcc/c/c-objc-common.c | 12 +-
gcc/cp/error.c | 92 --
gcc/fortran/error.c| 2 +-
gcc/pretty-print.c | 37 +++-
gcc/pretty-print.h | 5 +-
gcc/testsuite/g++.dg/cpp1z/direct-enum-init1.C | 234 -
gcc/testsuite/g++.dg/diagnostic/aka1.C | 2 +-
gcc/testsuite/g++.dg/diagnostic/aka2.C | 32
gcc/testsuite/g++.dg/parse/error55.C | 2 +-
gcc/testsuite/g++.dg/warn/pr12242.C| 16 +-
gcc/testsuite/g++.old-deja/g++.mike/enum1.C| 2 +-
gcc/testsuite/gcc.dg/diag-aka-1.c | 4 +-
gcc/testsuite/gcc.dg/diag-aka-2.c | 12 ++
gcc/testsuite/gcc.dg/pr13804-1.c | 4 +-
gcc/testsuite/gcc.dg/pr56980.c | 12 +-
gcc/testsuite/gcc.dg/pr65050.c | 8 +-
gcc/testsuite/gcc.dg/redecl-14.c | 2 +-
gcc/testsuite/gcc.dg/utf16-4.c | 2
Re: Reorgnanization of profile count maintenance code, part 1
On Mon, Jun 5, 2017 at 8:37 AM, Jan Hubicka wrote: > Hi, > I have committed the following fix. I seeing the following error while building aarch64-elf: /home/jenkins/workspace/BuildToolchainAARCH64_thunder_elf_upstream/toolchain/scripts/../src/gcc/shrink-wrap.c: In function ‘void handle_simple_exit(edge_def*)’: /home/jenkins/workspace/BuildToolchainAARCH64_thunder_elf_upstream/toolchain/scripts/../src/gcc/shrink-wrap.c:565: error: ‘struct edge_def’ has no member named ‘frequency’ Thanks, Andrew > > Honza > > Index: ChangeLog > === > --- ChangeLog (revision 248878) > +++ ChangeLog (working copy) > @@ -1,5 +1,10 @@ > 2017-06-05 Jan Hubicka > > + * config/mips/frame-header-opt.c: Include profile-count.h. > + * config/riscv/riscv.c: Include profile-count.h > + > +2017-06-05 Jan Hubicka > + > * tree-ssa-loop-im.c (execute_sm_if_changed): Add FLAG_BBS parameter; > update profile. > (sm_set_flag_if_changed): Add bbs field. > Index: config/mips/frame-header-opt.c > === > --- config/mips/frame-header-opt.c (revision 248871) > +++ config/mips/frame-header-opt.c (working copy) > @@ -32,6 +32,7 @@ along with GCC; see the file COPYING3. > #include "tree-pass.h" > #include "target.h" > #include "target-globals.h" > +#include "profile-count.h" > #include "cfg.h" > #include "cgraph.h" > #include "function.h" > Index: config/riscv/riscv.c > === > --- config/riscv/riscv.c(revision 248871) > +++ config/riscv/riscv.c(working copy) > @@ -73,6 +73,7 @@ along with GCC; see the file COPYING3. > #include "common/common-target.h" > #include "langhooks.h" > #include "dominance.h" > +#include "profile-count.h" > #include "cfg.h" > #include "cfgrtl.h" > #include "cfganal.h"
Re: Reorgnanization of profile count maintenance code, part 1
> I seeing the following error while building aarch64-elf: Same thing on darwin!-( TIA Dominique
Re: [PING**3] [PATCH] Force use of absolute path names for gcov
On 06/05/2017 12:31 PM, Bernd Edlinger wrote: So how about this new patch? Is it OK for trunk? Yes thanks. nathan -- Nathan Sidwell
Re: {PATCH] New C++ warning -Wcatch-value
On Mon, Jun 5, 2017 at 6:55 AM, Volker Reichelt wrote: > On 15 May, Martin Sebor wrote: >>> So how about the following then? I stayed with the catch part and added >>> a parameter to the warning to let the user decide on the warnings she/he >>> wants to get: -Wcatch-value=n. >>> -Wcatch-value=1 only warns for polymorphic classes that are caught by >>> value (to avoid slicing), -Wcatch-value=2 warns for all classes that >>> are caught by value (to avoid copies). And finally -Wcatch-value=3 >>> warns for everything not caught by reference to find typos (like pointer >>> instead of reference) and bad coding practices. >> >> It seems reasonable to me. I'm not too fond of multi-level >> warnings since few users take advantage of anything but the >> default, but this case is simple and innocuous enough that >> I don't think it can do harm. >> >>> >>> Bootstrapped and regtested on x86_64-pc-linux-gnu. >>> OK for trunk? >>> >>> If so, would it make sense to add -Wcatch-value=1 to -Wextra or even -Wall? >>> I would do this in a seperate patch, becuase I haven't checked what that >>> would mean for the testsuite. >> >> I can't think of a use case for polymorphic slicing that's not >> harmful so unless there is a common one that escapes me, I'd say >> -Wall. > > So that's what I committed after Jason's OK. > >> What are your thoughts on enhancing the warning to also handle >> the rethrow case? >> >> Also, it seems that a similar warning would be useful even beyond >> catch handlers, to help detect slicing when passing arguments to >> functions by value. Especially in code that mixes OOP with the >> STL (or other template libraries). Have you thought about tackling >> that at some point as well? > > I don't have any plans to handle handle the rethrow case. > > A general slicing warning would be very nice to have. Actually > clang-tidy has one (which is a little buggy, though). > Implementing this is over my head, though. > I'd rather stick with something less ambitious. FWIW I think it would be pretty straightforward to do this in convert_like_real, under the ck_base case. Jason
Re: [PATCH] warn on mem calls modifying objects of non-trivial types (PR 80560)
On 06/05/2017 10:07 AM, Martin Sebor wrote:
Maybe I should use a different approach and instead of trying
to see if a function is deleted use trivially_xible to see if
it's usable. That will mean changing the diagnostics from
"with a deleted special function" to "without trivial special
function" but it will avoid calling synthesized_method_walk
while still avoiding giving bogus suggestions.
Actually, this would check for one possible argument type and not
others, so I think it's better to keep looking at the declarations. You
can do that by just looking them up (lookup_fnfields_slot) and iterating
over them, you don't need to call synthesized_method_walk.
You mean using trivially_xible might check assignability or copy
constructibility from const T& but not from T& (or the other way
around), and you think both (or perhaps even other forms) should
be considered?
E.g., given:
struct S
{
S& operator= (const S&) = default;
void operator= (S&) = delete;
};
void f (S *d, const S *s)
{
memcpy(d, s, sizeof *d); // don't warn here
}
void g (S *d, S *s)
{
memcpy(d, s, sizeof *d); // but warn here
}
And your suggestion is to iterate over the assignment operator
(and copy ctor) overloads for S looking for one that's trivial,
public, and not deleted?
If that's it, I was thinking of just checking for the const T&
overload (as if by using std::is_trivially_copy_assignable()).
I don't mind trying the approach you suggest. It should be more
accurate. I just want to make sure we're on the same page.
Actually, after some more thought and testing the approach I have
a feeling that distinguishing between the two cases above is not
what you meant.
Classes that overload copy assignment or copy ctors on the constness
of the argument are tricky to begin with and using raw memory calls
on them seems suspect and worthy of a warning.
I'm guessing what you meant by "checking for one possible argument
type and not others" is actually checking to make sure all copy
assignment (and copy ctor) overloads are trivial, not jut some,
and at least one of them is accessible. I'll go with that unless
I hear otherwise.
Martin
Re: Reorgnanization of profile count maintenance code, part 1
> > I seeing the following error while building aarch64-elf: > > Same thing on darwin!-( Ooops, I managed to accidentally change my bootstrap tree. I have reverted that change now. Honza > > TIA > > Dominique
Re: More profile updating fixes
Hi,
On 5 June 2017 at 19:36, Jan Hubicka wrote:
> Hi,
> here are less trivial updates to profile code which I did not bundle into
> initial transition. Those are not bugs in old code, just new code needs
> to track more.
>
> profile-bootstrapped/regtested x86_64-linux, will commit it shortly.
> * cfgexpand.c (expand_gimple_tailcall): Initialize profile of
> new edge.
> * ipa-inline.c (want_inline_self_recursive_call_p): Watch for missing
> profile in callgraph edge.
> * profile-count.h (apply_probability): If THIS is 0, then result is 0
> (apply_scale): Likewise.
> * tree-inline.c (copy_bb, copy_edges_for_bb, copy_cfg_body):
> Also scale profile when inlining function with zero profile.
> (initialize_cfun): Update exit block profile even when it is zero.
> * tree-ssa-threadupdate.c (clear_counts_path): Handle correctly case
> when profile is read.
> Index: cfgexpand.c
> ===
> --- cfgexpand.c (revision 248871)
> +++ cfgexpand.c (working copy)
> @@ -3850,8 +3850,8 @@ expand_gimple_tailcall (basic_block bb,
>
>e = make_edge (bb, EXIT_BLOCK_PTR_FOR_FN (cfun), EDGE_ABNORMAL
> | EDGE_SIBCALL);
> - e->probability += probability;
> - e->count += count;
> + e->probability = probability;
> + e->count = count;
>BB_END (bb) = last;
>update_bb_for_insn (bb);
>
> Index: ipa-inline.c
> ===
> --- ipa-inline.c(revision 248871)
> +++ ipa-inline.c(working copy)
> @@ -912,7 +912,7 @@ want_inline_self_recursive_call_p (struc
> methods. */
>else
> {
> - if (max_count > profile_count::zero ()
> + if (max_count > profile_count::zero () && edge->count.initialized_p ()
> && (edge->count.to_gcov_type () * 100
> / outer_node->count.to_gcov_type ()
> <= PARAM_VALUE (PARAM_MIN_INLINE_RECURSIVE_PROBABILITY)))
> @@ -920,7 +920,8 @@ want_inline_self_recursive_call_p (struc
> reason = "profile of recursive call is too small";
> want_inline = false;
> }
> - else if (max_count == profile_count::zero ()
> + else if ((max_count == profile_count::zero ()
> + || !edge->count.initialized_p ())
>&& (edge->frequency * 100 / caller_freq
><= PARAM_VALUE (PARAM_MIN_INLINE_RECURSIVE_PROBABILITY)))
> {
> Index: profile-count.h
> ===
> --- profile-count.h (revision 248871)
> +++ profile-count.h (working copy)
> @@ -221,6 +221,8 @@ public:
>profile_count apply_probability (int prob) const
> {
>gcc_checking_assert (prob >= 0 && prob <= REG_BR_PROB_BASE);
> + if (*this == profile_count::zero ())
> + return *this;
>if (!initialized_p ())
> return profile_count::uninitialized ();
>profile_count ret;
> @@ -230,6 +232,8 @@ public:
>/* Return *THIS * NUM / DEN. */
>profile_count apply_scale (int64_t num, int64_t den) const
> {
> + if (*this == profile_count::zero ())
> + return *this;
>if (!initialized_p ())
> return profile_count::uninitialized ();
>profile_count ret;
> @@ -243,7 +247,7 @@ public:
> }
>profile_count apply_scale (profile_count num, profile_count den) const
> {
> - if (*this == profile_count::zero ())
> + if (*this == profile_count::zero () || num == profile_count::zero ())
> return profile_count::zero ();
>if (!initialized_p () || !num.initialized_p () || !den.initialized_p
> ())
> return profile_count::uninitialized ();
> Index: tree-inline.c
> ===
> --- tree-inline.c (revision 248871)
> +++ tree-inline.c (working copy)
> @@ -1763,7 +1763,8 @@ copy_bb (copy_body_data *id, basic_block
>tree decl;
>gcov_type freq;
>basic_block prev;
> - bool scale = num.initialized_p () && den.initialized_p () && den > 0;
> + bool scale = num.initialized_p ()
> + && (den > 0 || num == profile_count::zero ());
>
>/* Search for previous copied basic block. */
>prev = bb->prev_bb;
> @@ -2211,7 +2212,8 @@ copy_edges_for_bb (basic_block bb, profi
>gimple_stmt_iterator si;
>int flags;
>bool need_debug_cleanup = false;
> - bool scale = num.initialized_p () && den.initialized_p () && den > 0;
> + bool scale = num.initialized_p ()
> + && (den > 0 || num == profile_count::zero ());
>
>/* Use the indices from the original blocks to create edges for the
> new ones. */
> @@ -2472,7 +2474,7 @@ initialize_cfun (tree new_fndecl, tree c
> */
>if (ENTRY_BLOCK_PTR_FOR_FN (src_cfun)->count.initialized_p ()
>&& count.initialized_p ()
> - && ENTRY_BLOCK_PTR_FOR_FN (src_cfun)->co
Re: [Patch, fortran] PR35339 Optimize implied do loops in io statements
With all the style fixes committed as r248877. Thanks for the review. Nicolas On 06/03/2017 06:25 PM, Jerry DeLisle wrote: On 06/03/2017 06:48 AM, Nicolas Koenig wrote: Hello everyone, here is a version of the patch that includes a workaround for PR 80960. I have also included a separate test case for the failure that Dominique detected. The style issues should be fixed. Regression-tested. OK for trunk? Yes, OK. Thanks for the work. Jerry
[committed] libcpp: add callback for comment-handling
On Fri, 2017-05-12 at 12:47 -0600, Jeff Law wrote: > On 05/02/2017 01:08 PM, David Malcolm wrote: > > Currently the C/C++ frontends discard comments when parsing. > > It's possible to set up libcpp to capture comments as tokens, > > by setting CPP_OPTION (pfile, discard_comments) to false), > > and this can be enabled using the -C command line option (see > > also -CC), but c-family/c-lex.c then discards any CPP_COMMENT > > tokens it sees, so they're not seen by the frontend parser. > > > > The following patch adds an (optional) callback to libcpp > > for handling comments, giving the comment content, and the > > location it was seen at. This approach allows arbitrary > > logic to be wired up to comments, and avoids having to > > copy the comment content to a new buffer (which the CPP_COMMENT > > approach does). > > > > This could be used by plugins to chain up on the callback > > e.g. to parse specially-formatted comments, e.g. for > > documentation generation, or e.g. for GObject introspection > > annotations [1]. > > > > As a proof of concept, the patch uses this to add a spellchecker > > for comments. It uses the Enchant meta-library: > > https://abiword.github.io/enchant/ > > (essentially a wrapper around 8 different spellchecking libraries). > > I didn't bother with the autotool detection for enchant, and > > just hacked it in for now. > > > > Example output: > > > > test.c:3:46: warning: spellcheck_word: "evaulate" > > When NONCONST_PRED is false the code will evaulate to constant > > and > >^~~~ > > test.c:3:46: note: suggestion: "evaluate" > > When NONCONST_PRED is false the code will evaulate to constant > > and > >^~~~ > >evaluate > > test.c:3:46: note: suggestion: "ululate" > > When NONCONST_PRED is false the code will evaulate to constant > > and > >^~~~ > >ululate > > test.c:3:46: note: suggestion: "elevate" > > When NONCONST_PRED is false the code will evaulate to constant > > and > >^~~~ > >elevate > > > > License-wise, Enchant is LGPL 2.1 "or (at your option) any > > later version." with a special exception to allow non-LGPL > > spellchecking providers (e.g. to allow linking against an > > OS-provided spellchecker). > > > > Various FIXMEs are present (e.g. hardcoded "en_US" for the > > language to spellcheck against). > > Also, the spellchecker has a lot of false positives e.g. > > it doesn't grok URLs (and thus complains when it seens them); > > similar for DejaGnu directives etc. > > > > Does enchant seem like a reasonable dependency for the compiler? > > (it pulls in libpthread.so.0, libglib-2.0.so.0, libgmodule > > -2.0.so.0). > > Or would this be better pursued as a plugin? (if so, I'd > > prefer the plugin to live in the source tree as an example, > > rather than out-of-tree). > > > > Unrelated to spellchecking, I also added two new options: > > -Wfixme and -Wtodo, for warning when comments containing > > "FIXME" or "TODO" are encountered. > > I use such comments a lot during development. I thought some > > people might want a warning about them (I tend to just use grep > > though). [TODO: document these in invoke.texi, add test cases] > > > > Thoughts? Does any of this sound useful? > > > > [not yet bootstrapped; as noted above, I haven't yet done > > the autoconf stuff for handling Enchant] > > > > [1] > > https://wiki.gnome.org/Projects/GObjectIntrospection/Annotations > > > > gcc/ChangeLog: > > * Makefile.in (LIBS): Hack in -lenchant for now. > > (OBJS): Add spellcheck-enchant.o. > > * common.opt (Wfixme): New option. > > (Wtodo): New option. > > * spellcheck-enchant.c: New file. > > * spellcheck-enchant.h: New file. > > > > gcc/c-family/ChangeLog: > > * c-lex.c: Include spellcheck-enchant.h. > > (init_c_lex): Wire up spellcheck_enchant_check_comment to the > > comment callback. > > * c-opts.c: Include spellcheck-enchant.h. > > (c_common_post_options): Call spellcheck_enchant_init. > > (c_common_finish): Call spellcheck_enchant_finish. > > > > libcpp/ChangeLog: > > * include/cpplib.h (struct cpp_callbacks): Add "comment" > > callback. > > * lex.c (_cpp_lex_direct): Call the comment callback if non > > -NULL. > enchant seems a bit out of the sweet spot, particular just to catch > mis-spellings in comments. But it might make an interesting plugin. > > IIRC from our meeting earlier this week, you had another use case > that > might have been more compelling, but I can't remember what it was. (FWIW I think we were chatting about the GObjectIntrospection annotations use-case) > I do like the ability to at least capture the comments
Re: [PATCH] Add attribute((target_clone(...))) to PowerPC
Jan Hubicka recently changed the calling sequence to init_lowered_empty_function, and my patch doesn't build any more. I cloned the changes made in the x86_64 port and committed the following change (subversion id 248902) so that the rs6000 build should build once again. 2017-06-05 Michael Meissner * config/rs6000/rs6000.c (make_resolver_func): Update init_lowered_empty_function call. Index: gcc/config/rs6000/rs6000.c === --- gcc/config/rs6000/rs6000.c (revision 248901) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -40493,7 +40493,8 @@ make_resolver_func (const tree default_d gimplify_function_tree (decl); push_cfun (DECL_STRUCT_FUNCTION (decl)); - *empty_bb = init_lowered_empty_function (decl, false, 0); + *empty_bb = init_lowered_empty_function (decl, false, + profile_count::uninitialized ()); cgraph_node::add_new_function (decl, true); symtab->call_cgraph_insertion_hooks (cgraph_node::get_create (decl)); -- 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, libfortran] Fix matmul result zeroing for empty arguments
On 06/05/2017 02:52 AM, Thomas Koenig wrote: > Hello world, > > the attached patch moves the zeroing of the result variable before the > early return. This is done so that the result for zero-sized arguments > is still valid. > > The bug was only in the library version, but I have also added a test > case for the inline version to make sure the bug does not suddenly > appear there. > > OK for trunk? Yes OK for Trunk and 7 Thanks, Jerry
Re: [PATCH][SPARC] PR target/80968 Prevent stack loads in return delay slot.
> That seems to work as well, following is going through a testsuite > run right now: > > > [PATCH] sparc: Fix stack references in return delay slot. > > gcc/ > > PR target/80968 > * config/sparc/sparc.c (sparc_expand_prologue): Emit frame > blockage if function uses alloca. Probably worth applying on all active branches I'd think. -- Eric Botcazou
Fix PR rtl-optimization/80474
This is a regression present on the 6 branch for MIPS (and latent on the 7
branch and mainline). The reorg pass generates wrong code for the attached
testcase with a combination of options under the form of a double lo_sum(sym)
instruction for a single hi(sum).
The pass starts from a convoluted CFG with 4 instances of the same pair:
set $16,hi(sym) (1)
set $16,lo_sum($16,sym) (2)
and generates wrong code after 12 steps: first, the 4 (1) instructions are
moved into delay slots, then one of them is stolen and moved into another
slot, then a second one is deleted, then a third one is also stolen, and then
the first stolen one is deleted. Then 3 (2) instructions are moved into the
same delay slots (again empty) as the (1) and, finally, one of them is stolen.
All this dance considerably changes the live ranges of the $16 register, in
particular the deletion of 2 (1) instructions. The strategy of reorg is not
to update the liveness info, but to leave markers instead so that it can be
recomputed locally. But there is a hitch: it doesn't leave a marker when
/* Ignore if this was in a delay slot and it came from the target of
a branch. */
if (INSN_FROM_TARGET_P (insn))
return;
This exception has been there since the dawn of time and I can guess what kind
of reasoning led to it, but it's probably valid only for simple situations and
not for the kind of big transformations described above.
Lifting it fixes the wrong code because it leaves the necessary markers when
instructions that were stolen are then deleted. Surprisingly(?) enough, it
doesn't seem to have much effect outside this case (e.g. 0 changes for the
entire compile.exp testsuite at -O2 on SPARC and virtually same cc1 binaries).
Tested on SPARC/Solaris, objections to applying it on mainline,7 & 6 branches?
2017-06-06 Eric Botcazou
PR rtl-optimization/80474
* reorg.c (update_block): Do not ignore instructions in a delay slot.
--
Eric Botcazouint b;
static int a(char *h) { return 5; }
typedef enum { c } d;
typedef char *(*f)(unsigned char, unsigned char, d, unsigned, unsigned, char *,
unsigned);
typedef struct {
char **g;
f *i;
} j;
char *k(unsigned char, unsigned char, d, unsigned, unsigned, char *, unsigned);
char *m(unsigned char, unsigned char, d, unsigned, unsigned, char *, unsigned);
char *badFn(unsigned char,
unsigned char, d,
unsigned, unsigned,
char *, unsigned);
char *n(unsigned char, unsigned char, d, unsigned, unsigned, char *, unsigned);
char *o(unsigned char, unsigned char, d, unsigned, unsigned, char *, unsigned);
void InitEvent(j *h, unsigned p, int q, char *r, f s, char w) {
int e = 0;
for (; e < q; e++) {
h->i[p] = s;
h->g[p] = r;
}
}
f t;
void u(j *h, unsigned p) {
unsigned ac = p & (65536 | 2097152) ? 1 : 8;
if (p & (65536 | 2097152 | 524288))
if (ac == 1) {
j v = *h, aa = v;
aa.i[3] = n;
}
if (p & (8192 | 65536 | 2097152))
if (p & 524288)
InitEvent(h, p & 65536, 1, "", 0, 1);
if (ac == 1)
InitEvent(h, p, 1, "", 0, 1);
if (p)
if (ac == 1)
t = 0;
if (p & 8192)
t = 0;
if (p & (8192 | 65536 | 2097152))
InitEvent(h, 0, 1, "", 0, 1);
if (p)
InitEvent(h, 0, 1, "", 0, 1);
if (p & 2097152)
t = 0;
if (p & 8)
t = 0;
if (p & (8192 | 65536 | 2097152 | 524288))
if (ac == 1)
InitEvent(h, 0, 1, "", 0, 1);
if (p & 8192)
InitEvent(h, 0, 1, "OUTRESEG_DRAM_DATA_ECC_CORRECTED_1", m, 1);
if (p & (8192 | 65536 | 2097152 | 524288)) {
if (ac == 1)
InitEvent(h, p & 524288 ? 0 : p & 8192 ?: p & 2097152 ?: 5, 1, "",
badFn, 1);
else
InitEvent(h, 0, 1, "",
badFn, 1);
}
if (p & 8192)
InitEvent(h, 0, 1, "OUTRESEG_DRAM_DATA_ECC_UNCORRECTABLE_1",
badFn, 1);
if (p & (8192 | 65536 | 2097152 | 524288))
if (ac == 1)
InitEvent(h, 0, 1, "OUTRESEG_COLOUR_MARKING_OVERWRITE", k, 1);
if (p & (65536 | 2097152 | 524288))
if (ac == 1)
InitEvent(h, 0, 0, "", 0, 1);
if (p & 65536)
if (p & (65536 | 2097152))
InitEvent(h, 5, 0, "", 0, 1);
if (p & 524288)
b = a("");
InitEvent(h, p * (p & 2097152), 1,
"DMFE_QUAD_RBUF_RBUF_UNCORRECTED_ECC", o, 1);
}
char *l(char h, char p, d q, unsigned r, unsigned s, char *w, unsigned x) {
return w;
}
char *ak(char h, char p, d q, unsigned r, unsigned s, char *w, unsigned x) {
return w;
}
Index: reorg.c
===
--- reorg.c (revision 248571)
+++ reorg.c (working copy)
@@ -1697,9 +1697,8 @@ own_thread_p (rtx thread, rtx label, int
}
/* Called when INSN is being moved from a location near the target of a jump.
- We leave a marker of the form (use (
Re: [PATCH][SPARC] PR target/80968 Prevent stack loads in return delay slot.
From: Eric Botcazou Date: Tue, 06 Jun 2017 00:02:06 +0200 >> That seems to work as well, following is going through a testsuite >> run right now: >> >> >> [PATCH] sparc: Fix stack references in return delay slot. >> >> gcc/ >> >> PR target/80968 >> * config/sparc/sparc.c (sparc_expand_prologue): Emit frame >> blockage if function uses alloca. > > Probably worth applying on all active branches I'd think. I agree, this is a really nasty bug. I'll do that after some more testing. Eric, thanks for reviewing and your advice to use frame blockage!
Re: [PATCH] warn on mem calls modifying objects of non-trivial types (PR 80560)
On 06/05/2017 01:13 PM, Martin Sebor wrote:
On 06/05/2017 10:07 AM, Martin Sebor wrote:
Maybe I should use a different approach and instead of trying
to see if a function is deleted use trivially_xible to see if
it's usable. That will mean changing the diagnostics from
"with a deleted special function" to "without trivial special
function" but it will avoid calling synthesized_method_walk
while still avoiding giving bogus suggestions.
Actually, this would check for one possible argument type and not
others, so I think it's better to keep looking at the declarations. You
can do that by just looking them up (lookup_fnfields_slot) and iterating
over them, you don't need to call synthesized_method_walk.
You mean using trivially_xible might check assignability or copy
constructibility from const T& but not from T& (or the other way
around), and you think both (or perhaps even other forms) should
be considered?
E.g., given:
struct S
{
S& operator= (const S&) = default;
void operator= (S&) = delete;
};
void f (S *d, const S *s)
{
memcpy(d, s, sizeof *d); // don't warn here
}
void g (S *d, S *s)
{
memcpy(d, s, sizeof *d); // but warn here
}
And your suggestion is to iterate over the assignment operator
(and copy ctor) overloads for S looking for one that's trivial,
public, and not deleted?
If that's it, I was thinking of just checking for the const T&
overload (as if by using std::is_trivially_copy_assignable()).
I don't mind trying the approach you suggest. It should be more
accurate. I just want to make sure we're on the same page.
Actually, after some more thought and testing the approach I have
a feeling that distinguishing between the two cases above is not
what you meant.
Classes that overload copy assignment or copy ctors on the constness
of the argument are tricky to begin with and using raw memory calls
on them seems suspect and worthy of a warning.
I'm guessing what you meant by "checking for one possible argument
type and not others" is actually checking to make sure all copy
assignment (and copy ctor) overloads are trivial, not jut some,
and at least one of them is accessible. I'll go with that unless
I hear otherwise.
Attached is an update that implements this simplified approach.
Martin
PR c++/80560 - warn on undefined memory operations involving non-trivial types
gcc/c-family/ChangeLog:
PR c++/80560
* c.opt (-Wclass-memaccess): New option.
gcc/cp/ChangeLog:
PR c++/80560
* call.c (first_non_public_field, maybe_warn_class_memaccess): New
functions.
(has_trivial_copy_assign_p, has_trivial_copy_p): Ditto.
(build_cxx_call): Call maybe_warn_class_memaccess.
gcc/ChangeLog:
PR c++/80560
* dumpfile.c (dump_register): Avoid calling memset to initialize
a class with a default ctor.
* gcc.c (struct compiler): Remove const qualification.
* genattrtab.c (gen_insn_reserv): Replace memset with initialization.
* hash-table.h: Ditto.
* ipa-cp.c (allocate_and_init_ipcp_value): Replace memset with
assignment.
* ipa-prop.c (ipa_free_edge_args_substructures): Ditto.
* omp-low.c (lower_omp_ordered_clauses): Replace memset with
default ctor.
* params.h (struct param_info): Make struct members non-const.
* tree-switch-conversion.c (emit_case_bit_tests): Replace memset
with default initialization.
* vec.h (vec_copy_construct, vec_default_construct): New helper
functions.
(vec::copy, vec::splice, vec::reserve): Replace memcpy
with vec_copy_construct.
(vect::quick_grow_cleared): Replace memset with default ctor.
(vect::vec_safe_grow_cleared, vec_safe_grow_cleared): Same.
* doc/invoke.texi (-Wclass-memaccess): Document.
libcpp/ChangeLog:
PR c++/80560
* line-map.c (line_maps::~line_maps): Avoid calling htab_delete
with a null pointer.
(linemap_init): Avoid calling memset on an object of a non-trivial
type.
libitm/ChangeLog:
PR c++/80560
* beginend.cc (GTM::gtm_thread::rollback): Avoid calling memset
on an object of a non-trivial type.
(GTM::gtm_transaction_cp::commit): Use assignment instead of memcpy
to copy an object.
* method-ml.cc (orec_iterator::reinit): Avoid -Wclass-memaccess.
gcc/testsuite/ChangeLog:
PR c++/80560
* g++.dg/Wclass-memaccess.C: New test.
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 9ad2f6e..f777b8f 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -792,6 +792,10 @@ Wnon-template-friend
C++ ObjC++ Var(warn_nontemplate_friend) Init(1) Warning
Warn when non-templatized friend functions are declared within a template.
+Wclass-memaccess
+C++ ObjC++ Var(warn_class_memaccess) Warning LangEnabledBy(C++ ObjC++, Wall)
+Warn for unsafe raw memory writes to objects of class types.
+
Wnon-virtual-dtor
C++ ObjC++ Var(warn_nonvdtor) Warning LangEnabledBy(C++ ObjC++,Weffc++)
Warn about non-virtual destructors.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index e348f29..df83688 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -8153,6 +8153,376 @@ build_over_call (struct z_can
[PING #3] [PATCH] handle enumerated types in -Wformat-overflow (PR 80397)
Jeff, I suspect you're still busy but I'm Looking for approval to commit the nearly trivial patch below: https://gcc.gnu.org/ml/gcc-patches/2017-04/msg00547.html Martin On 05/22/2017 08:19 PM, Martin Sebor wrote: Ping: https://gcc.gnu.org/ml/gcc-patches/2017-04/msg00547.html On 05/15/2017 09:01 PM, Martin Sebor wrote: Ping: Jeff, is this patch approved? https://gcc.gnu.org/ml/gcc-patches/2017-04/msg00547.html On 05/08/2017 08:38 PM, Martin Sebor wrote: On 04/28/2017 12:35 PM, Jeff Law wrote: On 04/26/2017 11:05 AM, Martin Sebor wrote: On 04/24/2017 03:35 PM, Jeff Law wrote: On 04/11/2017 12:57 PM, Martin Sebor wrote: In a review of my fix for bug 80364 Jakub pointed out that to determine whether an argument to an integer directive is of an integer type the gimple-ssa-sprintf pass tests the type code for equality to INTEGER_TYPE when it should instead be using INTEGRAL_TYPE_P(). This has the effect of the pass being unable to use the available range of arguments of enumerated types, resulting in both false positives and false negatives, and in some cases, in emitting suboptimal code. The attached patch replaces those tests with INTEGRAL_TYPE_P(). Since this is not a regression I submit it for GCC 8. You might consider using POINTER_TYPE_P as well. You mean rather than (TREE_CODE (type) == POINTER_TYPE)? Those I believe are vestiges of the %p handling that was removed sometime last year, and (unless you are recommending I remove them as part of this patch) should probably be removed during the next cleanup.Yes, that can be a follow-up cleanup. For the future, if you find yourself writing something like TREE_CODE (TREE_TYPE (x)) == POINTER_TYPE, you're usually going to be better off using POINTER_TYPE_P (TREE_TYPE (x)). That allows the code to work with C++ references as well as C pointers. I'll keep it in mind, thanks. Should I take this as approval of the patch as is or are there some changes you'd like me to make? Martin
Re: Reorgnanization of profile count maintenance code, part 1
On Thu, Jun 1, 2017 at 4:35 AM, Jan Hubicka wrote: > Index: profile.c > === > --- profile.c (revision 248684) > +++ profile.c (working copy) > @@ -67,6 +67,10 @@ along with GCC; see the file COPYING3. > > #include "profile.h" > > +/* Map from BBs/edges to gcov counters. */ > +vec bb_gcov_counts; > +hash_map edge_gcov_counts; This completely breaks the compiler with --enable-gather-detailed-mem-stats; edge_gcov_counts gets initialized before hash_table_usage in hash-table.c, and so when the constructor for edge_gcov_counts calls hash_table_usage.register_descriptor, m_map is null and we get a SEGV. Jason
[PING] [PING] Make the OpenACC C++ acc_on_device wrapper "always inline"
Hi!
Ping.
On Tue, 30 May 2017 14:35:29 +0200, I wrote:
> Ping.
>
> On Tue, 23 May 2017 17:31:11 +0200, I wrote:
> > On Thu, 29 Oct 2015 17:22:46 -0700, Nathan Sidwell wrote:
> > > acc_on_device and it's builtin had a conflict. The function formally
> > > takes an
> > > enum argument, but the builtin takes an int -- primarily to avoid the
> > > compiler
> > > having to generate the enum type internally.
> > >
> > > This works fine for C, where the external declaration of the function
> > > (in
> > > openacc.h) matches up with the builtin, and we optimize the builtin as
> > > expected.
> > >
> > > It fails for C++ where the builtin doesn't match the declaration in the
> > > header.
> > > We end up with emitting a call to acc_on_device, which is resolved by
> > > libgomp. Unfortunately that means we fail to optimize. [...]
> >
> > > [Nathan's trunk r229562] leaves things unchanged for C -- declare a
> > > function with an enum arg.
> > > But for C++ we the extern "C" declaration takes an int -- and therefore
> > > matches the builtin. We insert an inline wrapper that takes an enum
> > > argument.
> > > Because of C++'s overload resolution both the wrapper and the int-taking
> > > declaration can have the same source name.
> >
> > > --- libgomp/openacc.h (revision 229535)
> > > +++ libgomp/openacc.h (working copy)
> >
> > > -int acc_on_device (acc_device_t) __GOACC_NOTHROW;
> > > +#ifdef __cplusplus
> > > +int acc_on_device (int __arg) __GOACC_NOTHROW;
> > > +#else
> > > +int acc_on_device (acc_device_t __arg) __GOACC_NOTHROW;
> > > +#endif
> >
> > > #ifdef __cplusplus
> > > }
> > > +
> > > +/* Forwarding function with correctly typed arg. */
> > > +
> > > +inline int acc_on_device (acc_device_t __arg) __GOACC_NOTHROW
> > > +{
> > > + return acc_on_device ((int) __arg);
> > > +}
> > > #endif
> >
> > > --- libgomp/testsuite/libgomp.oacc-c-c++-common/acc-on-device.c
> > > (revision 0)
> > > +++ libgomp/testsuite/libgomp.oacc-c-c++-common/acc-on-device.c
> > > (working copy)
> > > @@ -0,0 +1,12 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-additional-options "-O2" } */
> > > +
> > > +#include
> > > +
> > > +int Foo (acc_device_t x)
> > > +{
> > > + return acc_on_device (x);
> > > +}
> > > +
> > > +/* { dg-final { scan-assembler-not "acc_on_device" } } */
> >
> > As a user, I'd expect that when compiling such code with "-O0" instead of
> > "-O2", but adding "__attribute__ ((optimize ("O2")))" to "Foo", that I'd
> > then get "acc_on_device" expanded as a builtin, and no calls to the
> > "acc_on_device library function. In C++ that is currently not working,
> > because the "Forwarding function with correctly typed arg" (cited above)
> > doesn't "inherit" that "optimize" attribute. Making that one "always
> > inline" resolves the problem. Also I cleaned up and extended testing
> > some more. OK for trunk?
> >
> > commit 9cc3a384c17e9f692f7864c604d2e2f9fbf0bac9
> > Author: Thomas Schwinge
> > Date: Tue May 23 13:21:14 2017 +0200
> >
> > Make the OpenACC C++ acc_on_device wrapper "always inline"
> >
> > libgomp/
> > * openacc.h [__cplusplus] (acc_on_device): Mark as "always
> > inline".
> > * testsuite/libgomp.oacc-c-c++-common/acc-on-device-2.c: Remove
> > file; test cases already present...
> > * testsuite/libgomp.oacc-c-c++-common/acc_on_device-1.c: ... in
> > this file. Update.
> > * testsuite/libgomp.oacc-c-c++-common/acc-on-device.c: Remove
> > file; test cases now present...
> > * testsuite/libgomp.oacc-c-c++-common/acc_on_device-2.c: ... in
> > this new file.
> > * testsuite/libgomp.oacc-c-c++-common/parallel-dims.c: Update.
> > ---
> > libgomp/openacc.h | 3 +-
> > .../libgomp.oacc-c-c++-common/acc-on-device-2.c| 22 -
> > .../libgomp.oacc-c-c++-common/acc-on-device.c | 12 ---
> > .../libgomp.oacc-c-c++-common/acc_on_device-1.c| 38
> > +-
> > .../libgomp.oacc-c-c++-common/acc_on_device-2.c| 21
> > .../libgomp.oacc-c-c++-common/parallel-dims.c | 14
> > 6 files changed, 52 insertions(+), 58 deletions(-)
> >
> > diff --git libgomp/openacc.h libgomp/openacc.h
> > index 137e2c1..266f559 100644
> > --- libgomp/openacc.h
> > +++ libgomp/openacc.h
> > @@ -121,7 +121,8 @@ int acc_set_cuda_stream (int, void *) __GOACC_NOTHROW;
> > /* Forwarding function with correctly typed arg. */
> >
> > #pragma acc routine seq
> > -inline int acc_on_device (acc_device_t __arg) __GOACC_NOTHROW
> > +inline __attribute__ ((__always_inline__)) int
> > +acc_on_device (acc_device_t __arg) __GOACC_NOTHROW
> > {
> >return acc_on_device ((int) __arg);
> > }
> > diff --git libgomp/testsuite/libgomp.oacc-c-c++-common/acc-on-device-2.c
> > libgomp/testsuite/libgomp.oacc-c-c++-common/acc-on-de
Re: [PATCH] Initialize live_switch_vars for SWITCH_BODY == STATEMENT_LIST (PR sanitizer/80879).
PING^1 On 05/26/2017 01:05 PM, Martin Liška wrote: Hello. Unfortunately I guarded use-after-scope to track live switch variables just to BIND_EXPR. However the bind expression can be included in a STATEMENT_LIST. That enables proper tracking and fixes the test added. Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. Ready to be installed? Martin
Re: [PING] [PING] Make the OpenACC C++ acc_on_device wrapper "always inline"
On Tue, Jun 06, 2017 at 08:35:40AM +0200, Thomas Schwinge wrote: > > > commit 9cc3a384c17e9f692f7864c604d2e2f9fbf0bac9 > > > Author: Thomas Schwinge > > > Date: Tue May 23 13:21:14 2017 +0200 > > > > > > Make the OpenACC C++ acc_on_device wrapper "always inline" > > > > > > libgomp/ > > > * openacc.h [__cplusplus] (acc_on_device): Mark as "always > > > inline". > > > * testsuite/libgomp.oacc-c-c++-common/acc-on-device-2.c: > > > Remove > > > file; test cases already present... > > > * testsuite/libgomp.oacc-c-c++-common/acc_on_device-1.c: ... > > > in > > > this file. Update. > > > * testsuite/libgomp.oacc-c-c++-common/acc-on-device.c: Remove > > > file; test cases now present... > > > * testsuite/libgomp.oacc-c-c++-common/acc_on_device-2.c: ... > > > in > > > this new file. > > > * testsuite/libgomp.oacc-c-c++-common/parallel-dims.c: Update. I don't like this very much. Can't you instead just turn the builtin into BT_FN_INT_VAR and diagnose during folding if it has no or 2+ arguments or if the argument is not type compatible with int? Jakub
Re: [PATCH] Fix multi-versioning issues (PR ipa/80732).
PING^1
On 05/25/2017 12:05 PM, Martin Liška wrote:
Hello.
Following patch tries to resolve following 2 issues:
a) When one takes address of a function that uses target_clones attribute,
default implementation is always returned.
b) Using dlsym("foo") should work and thus the resolver function should
use the default name. Because of that, default implementation must be
renamed.
Unfortunately, we currently do not support redirection of ipa_refs, thus
walk_tree is needed to resolve that. Hopefully there should not be any
different IPA_REF that needs to be handled.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Martin
