Re: [PATCH] PR libstdc++/83982 fix exception-safety guarantee of std::vector::resize
On Wed, Jun 13, 2018 at 11:43 PM, Jonathan Wakely wrote: > Construct new elements before moving existing ones, so that if a default > constructor throws, the existing elements are not left in a moved-from > state. > > 2018-06-14 Daniel Trebbien > Jonathan Wakely > > PR libstdc++/83982 > * include/bits/vector.tcc (vector::_M_default_append(size_type)): > Default-construct new elements before moving existing ones. > * testsuite/23_containers/vector/capacity/resize/strong_guaran > tee.cc: > New. > > Tested powerpc64le-linux. > > Daniel (CC'd) originally proposed a fix based on the code used in > vector::_M_realloc_insert. This is a slightly simpler fix, possible > because resize only inserts at the end not in the middle. I also wrote > a new testcase which is also valid in C++98, where the lack of move > constructors means the strong exception-safety guarantee is always > met. (Which means the bug is a regression since the default mode > changed to -std=gnu++14 in GCC 6.1). > > I plan to commit this to trunk tomorrow, and backport soon too. > > Thank you, Jonathan. Your simpler patch looks good to me. I have also verified that the strong_guarantee.cc test case fails with g++-8 (Homebrew GCC 8.1.0) macOS AMD64 architecture, and passes when the changes to vector.tcc are applied to /usr/local/include/c++/8.1.0/bits/vector.tcc. Daniel
[PATCH] RISC-V: Add custom RTEMS multilibs
Add multilib variants for -march=rv64imafd, e.g. to support the BOOMv2 core.
Add -mcmodel=medany as a variant of the 64-bit multilibs for RTEMS. The
ratinale for this change is that several existing RISC-V chips map the
RAM at 0x8000. In RTEMS, we do not use virtual memory, so
applications will run at this location which is outside the +-2GiB range
in a 64-bit configuration.
gcc/
* config.gcc (riscv*-*-elf* | riscv*-*-rtems*): Use custom
multilibs for *-*-rtems*.
* config/riscv/t-rtems: New file.
---
gcc/config.gcc | 20 +++-
gcc/config/riscv/t-rtems | 25 +
2 files changed, 36 insertions(+), 9 deletions(-)
create mode 100644 gcc/config/riscv/t-rtems
diff --git a/gcc/config.gcc b/gcc/config.gcc
index c3aecbfe764..25383423721 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -2085,10 +2085,17 @@ riscv*-*-linux*)
;;
riscv*-*-elf* | riscv*-*-rtems*)
tm_file="elfos.h newlib-stdint.h ${tm_file} riscv/elf.h"
- case "x${enable_multilib}" in
- xno) ;;
- xyes) tmake_file="${tmake_file} riscv/t-elf-multilib" ;;
- *) echo "Unknown value for enable_multilib"; exit 1
+ case ${target} in
+ *-*-rtems*)
+ tm_file="${tm_file} rtems.h riscv/rtems.h"
+ tmake_file="${tmake_file} riscv/t-rtems"
+ ;;
+ *)
+ case "x${enable_multilib}" in
+ xno) ;;
+ xyes) tmake_file="${tmake_file} riscv/t-elf-multilib" ;;
+ *) echo "Unknown value for enable_multilib"; exit 1
+ esac
esac
tmake_file="${tmake_file} riscv/t-riscv"
gnu_ld=yes
@@ -2096,11 +2103,6 @@ riscv*-*-elf* | riscv*-*-rtems*)
# Force .init_array support. The configure script cannot always
# automatically detect that GAS supports it, yet we require it.
gcc_cv_initfini_array=yes
- case ${target} in
- riscv*-*-rtems*)
- tm_file="${tm_file} rtems.h riscv/rtems.h"
- ;;
- esac
;;
riscv*-*-freebsd*)
tm_file="${tm_file} elfos.h ${fbsd_tm_file} riscv/freebsd.h"
diff --git a/gcc/config/riscv/t-rtems b/gcc/config/riscv/t-rtems
new file mode 100644
index 000..41f5927fc87
--- /dev/null
+++ b/gcc/config/riscv/t-rtems
@@ -0,0 +1,25 @@
+MULTILIB_OPTIONS =
+MULTILIB_DIRNAMES =
+
+MULTILIB_OPTIONS +=
march=rv32i/march=rv32im/march=rv32imafd/march=rv32iac/march=rv32imac/march=rv32imafc/march=rv64imafd/march=rv64imac/march=rv64imafdc
+MULTILIB_DIRNAMES += rv32i rv32im rv32imafd rv32iac
rv32imac rv32imafc rv64imafd rv64imac rv64imafdc
+
+MULTILIB_OPTIONS +=
mabi=ilp32/mabi=ilp32f/mabi=ilp32d/mabi=lp64/mabi=lp64d
+MULTILIB_DIRNAMES += ilp32 ilp32f ilp32d lp64 lp64d
+
+MULTILIB_OPTIONS += mcmodel=medany
+MULTILIB_DIRNAMES += medany
+
+MULTILIB_REQUIRED =
+MULTILIB_REQUIRED += march=rv32i/mabi=ilp32
+MULTILIB_REQUIRED += march=rv32im/mabi=ilp32
+MULTILIB_REQUIRED += march=rv32imafd/mabi=ilp32d
+MULTILIB_REQUIRED += march=rv32iac/mabi=ilp32
+MULTILIB_REQUIRED += march=rv32imac/mabi=ilp32
+MULTILIB_REQUIRED += march=rv32imafc/mabi=ilp32f
+MULTILIB_REQUIRED += march=rv64imafd/mabi=lp64d
+MULTILIB_REQUIRED += march=rv64imafd/mabi=lp64d/mcmodel=medany
+MULTILIB_REQUIRED += march=rv64imac/mabi=lp64
+MULTILIB_REQUIRED += march=rv64imac/mabi=lp64/mcmodel=medany
+MULTILIB_REQUIRED += march=rv64imafdc/mabi=lp64d
+MULTILIB_REQUIRED += march=rv64imafdc/mabi=lp64d/mcmodel=medany
--
2.13.7
Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections
Committed. Thanks, Roland
[PATCH v2] [aarch64] Add HiSilicon tsv110 CPU support
This patch adds HiSilicon's an mcpu: tsv110, which supports v8_4A.
---
gcc/ChangeLog| 8 +++
gcc/config/aarch64/aarch64-cores.def | 3 +
gcc/config/aarch64/aarch64-cost-tables.h | 103 +++
gcc/config/aarch64/aarch64-tune.md | 2 +-
gcc/config/aarch64/aarch64.c | 80 +++-
gcc/doc/invoke.texi | 2 +-
6 files changed, 195 insertions(+), 3 deletions(-)
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 9c90875..e376714 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,11 @@
+2018-06-12 Shaokun Zhang
+Bo Zhou
+ * config/aarch64/aarch64-cores.def (tsv110): New CPU.
+ * config/aarch64/aarch64-tune.md: Regenerated.
+ * doc/invoke.texi (AArch64 Options/-mtune): Add "tsv110".
+ * config/aarch64/aarch64.c (tsv110_tunings): New tuning table.
+ * config/aarch64/aarch64-cost-tables.h: Add "tsv110" extra costs.
+
2018-06-12 Eric Botcazou
* gcc.c: Document new %@{...} sequence.
diff --git a/gcc/config/aarch64/aarch64-cores.def
b/gcc/config/aarch64/aarch64-cores.def
index e64d831..e6ebf02 100644
--- a/gcc/config/aarch64/aarch64-cores.def
+++ b/gcc/config/aarch64/aarch64-cores.def
@@ -88,6 +88,9 @@ AARCH64_CORE("cortex-a75", cortexa75, cortexa57, 8_2A,
AARCH64_FL_FOR_ARCH8_2
/* ARMv8.4-A Architecture Processors. */
+/* HiSilicon ('H') cores. */
+AARCH64_CORE("tsv110", tsv110,cortexa57,8_4A,
AARCH64_FL_FOR_ARCH8_4 | AARCH64_FL_CRYPTO | AARCH64_FL_F16 | AARCH64_FL_AES |
AARCH64_FL_SHA2, tsv110, 0x48, 0xd01, -1)
+
/* Qualcomm ('Q') cores. */
AARCH64_CORE("saphira", saphira,falkor,8_4A,
AARCH64_FL_FOR_ARCH8_4 | AARCH64_FL_CRYPTO | AARCH64_FL_RCPC, saphira, 0x51,
0xC01, -1)
diff --git a/gcc/config/aarch64/aarch64-cost-tables.h
b/gcc/config/aarch64/aarch64-cost-tables.h
index a455c62..b6890d6 100644
--- a/gcc/config/aarch64/aarch64-cost-tables.h
+++ b/gcc/config/aarch64/aarch64-cost-tables.h
@@ -334,4 +334,107 @@ const struct cpu_cost_table thunderx2t99_extra_costs =
}
};
+const struct cpu_cost_table tsv110_extra_costs =
+{
+ /* ALU */
+ {
+0, /* arith. */
+0, /* logical. */
+0, /* shift. */
+0, /* shift_reg. */
+COSTS_N_INSNS (1), /* arith_shift. */
+COSTS_N_INSNS (1), /* arith_shift_reg. */
+COSTS_N_INSNS (1), /* log_shift. */
+COSTS_N_INSNS (1), /* log_shift_reg. */
+0, /* extend. */
+COSTS_N_INSNS (1), /* extend_arith. */
+0, /* bfi. */
+0, /* bfx. */
+0, /* clz. */
+0,/* rev. */
+0, /* non_exec. */
+true /* non_exec_costs_exec. */
+ },
+ {
+/* MULT SImode */
+{
+ COSTS_N_INSNS (2), /* simple. */
+ COSTS_N_INSNS (2), /* flag_setting. */
+ COSTS_N_INSNS (2), /* extend. */
+ COSTS_N_INSNS (2), /* add. */
+ COSTS_N_INSNS (2), /* extend_add. */
+ COSTS_N_INSNS (11) /* idiv. */
+},
+/* MULT DImode */
+{
+ COSTS_N_INSNS (3), /* simple. */
+ 0, /* flag_setting (N/A). */
+ COSTS_N_INSNS (3), /* extend. */
+ COSTS_N_INSNS (3), /* add. */
+ COSTS_N_INSNS (3), /* extend_add. */
+ COSTS_N_INSNS (19) /* idiv. */
+}
+ },
+ /* LD/ST */
+ {
+COSTS_N_INSNS (3), /* load. */
+COSTS_N_INSNS (4), /* load_sign_extend. */
+COSTS_N_INSNS (3), /* ldrd. */
+COSTS_N_INSNS (3), /* ldm_1st. */
+1, /* ldm_regs_per_insn_1st. */
+2, /* ldm_regs_per_insn_subsequent. */
+COSTS_N_INSNS (4), /* loadf. */
+COSTS_N_INSNS (4), /* loadd. */
+COSTS_N_INSNS (4), /* load_unaligned. */
+0, /* store. */
+0, /* strd. */
+0, /* stm_1st. */
+1, /* stm_regs_per_insn_1st. */
+2, /* stm_regs_per_insn_subsequent. */
+0, /* storef. */
+0, /* stored. */
+COSTS_N_INSNS (1), /* store_unaligned. */
+COSTS_N_INSNS (4), /* loadv. */
+COSTS_N_INSNS (4) /* storev. */
+ },
+ {
+/* FP SFmode */
+{
+ COSTS_N_INSNS (10), /* div. */
+ COSTS_N_INSNS (4), /* mult. */
+ COSTS_N_INSNS (4), /* mult_addsub. */
+ COSTS_N_INSNS (4), /* fma. */
+ COSTS_N_INSNS (4), /* addsub. */
+ COSTS_N_INSNS (1), /* fpconst. */
+ COSTS_N_INSNS (1), /* neg. */
+ COSTS_N_INSNS (1), /* compare. */
+ COSTS_N_INSNS (2), /* widen. */
+
Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections
On Wed, Jun 13, 2018 at 5:52 PM, Roland McGrath wrote: > A direct cherry-pick of the trunk change > (db7548a2771bbf34cf7430712af7ac670b429958 / r259969) applies fine to > today's 8 branch and has no check-gcc regressions on x86_64-linux-gnu. > > OK to commit to 8 branch now? Sure. Ian
Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections
A direct cherry-pick of the trunk change (db7548a2771bbf34cf7430712af7ac670b429958 / r259969) applies fine to today's 8 branch and has no check-gcc regressions on x86_64-linux-gnu. OK to commit to 8 branch now? Thanks, Roland
C++ PATCH for c++/86099, ICE with NSDMI
In this testcase, the compiler is trying to evaluate a
constant-expression which involves a trivial defaulted copy
constructor. My patch for CWG 1581 had us instantiate any function
that appears in a potential constant expression, but that's not what
we want for a trivial constructor like this.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit b71b855538a05716e704d02f636b8820faec3e62
Author: Jason Merrill
Date: Wed Jun 13 17:51:44 2018 -0400
PR c++/86099 - ICE with trivial copy and non-trivial default ctor.
* constexpr.c (instantiate_cx_fn_r): Don't synthesize trivial
constructors.
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 97a338535db..93de6162c04 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -4841,6 +4841,7 @@ instantiate_cx_fn_r (tree *tp, int *walk_subtrees, void */*data*/)
if (TREE_CODE (*tp) == FUNCTION_DECL
&& DECL_DECLARED_CONSTEXPR_P (*tp)
&& !DECL_INITIAL (*tp)
+ && !trivial_fn_p (*tp)
&& DECL_TEMPLOID_INSTANTIATION (*tp))
{
++function_depth;
diff --git a/gcc/testsuite/g++.dg/cpp0x/nsdmi-template18.C b/gcc/testsuite/g++.dg/cpp0x/nsdmi-template18.C
new file mode 100644
index 000..e3520bc6b44
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/nsdmi-template18.C
@@ -0,0 +1,43 @@
+// PR c++/86099
+// { dg-do compile { target c++11 } }
+
+template struct e { static constexpr int c = a; };
+template using d = e;
+template struct aa;
+template struct j;
+template struct j : aa::h {};
+template struct n : d {};
+template l m(int);
+template auto ab() -> decltype(m(0));
+template struct p;
+template struct p : e()))> {};
+template struct r;
+class s;
+template
+struct ac : j, p, q...>> {};
+template struct ae : ac {};
+template struct aa { typedef ad h; };
+template struct w : j>, d> {};
+template struct r {
+ t f;
+ int af;
+ r(r &&) = default;
+};
+template ::c, k>::h> void v(k *);
+template ah aj(ag x, ah, ai) { v(x); return 0; }
+template struct y { typedef int ak; };
+template class z {
+public:
+ template void al();
+};
+template template void z::al() {
+ r *u;
+ typename y::ak a = aj(u, a, int());
+}
+class s {
+ char *an = nullptr;
+};
+void ao() {
+ z b;
+ b.al();
+}
[PATCH] PR libstdc++/83982 fix exception-safety guarantee of std::vector::resize
Construct new elements before moving existing ones, so that if a default
constructor throws, the existing elements are not left in a moved-from
state.
2018-06-14 Daniel Trebbien
Jonathan Wakely
PR libstdc++/83982
* include/bits/vector.tcc (vector::_M_default_append(size_type)):
Default-construct new elements before moving existing ones.
* testsuite/23_containers/vector/capacity/resize/strong_guarantee.cc:
New.
Tested powerpc64le-linux.
Daniel (CC'd) originally proposed a fix based on the code used in
vector::_M_realloc_insert. This is a slightly simpler fix, possible
because resize only inserts at the end not in the middle. I also wrote
a new testcase which is also valid in C++98, where the lack of move
constructors means the strong exception-safety guarantee is always
met. (Which means the bug is a regression since the default mode
changed to -std=gnu++14 in GCC 6.1).
I plan to commit this to trunk tomorrow, and backport soon too.
commit ef19eec2848fecf65ec51199549fdabfafed9927
Author: Jonathan Wakely
Date: Thu Jun 14 00:09:11 2018 +0100
PR libstdc++/83982 fix exception-safety guarantee of std::vector::resize
Construct new elements before moving existing ones, so that if a default
constructor throws, the existing elements are not left in a moved-from
state.
2018-06-14 Daniel Trebbien
Jonathan Wakely
PR libstdc++/83982
* include/bits/vector.tcc (vector::_M_default_append(size_type)):
Default-construct new elements before moving existing ones.
*
testsuite/23_containers/vector/capacity/resize/strong_guarantee.cc:
New.
diff --git a/libstdc++-v3/include/bits/vector.tcc
b/libstdc++-v3/include/bits/vector.tcc
index 1d1ef427b26..86a711713b2 100644
--- a/libstdc++-v3/include/bits/vector.tcc
+++ b/libstdc++-v3/include/bits/vector.tcc
@@ -582,7 +582,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
{
if (__n != 0)
{
- size_type __size = size();
+ const size_type __size = size();
size_type __navail = size_type(this->_M_impl._M_end_of_storage
- this->_M_impl._M_finish);
@@ -601,23 +601,22 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
{
const size_type __len =
_M_check_len(__n, "vector::_M_default_append");
- const size_type __old_size = __size;
pointer __new_start(this->_M_allocate(__len));
- pointer __new_finish(__new_start);
+ pointer __destroy_from = pointer();
__try
{
- __new_finish
- = std::__uninitialized_move_if_noexcept_a
- (this->_M_impl._M_start, this->_M_impl._M_finish,
-__new_start, _M_get_Tp_allocator());
- __new_finish =
- std::__uninitialized_default_n_a(__new_finish, __n,
-_M_get_Tp_allocator());
+ std::__uninitialized_default_n_a(__new_start + __size,
+ __n, _M_get_Tp_allocator());
+ __destroy_from = __new_start + __size;
+ std::__uninitialized_move_if_noexcept_a(
+ this->_M_impl._M_start, this->_M_impl._M_finish,
+ __new_start, _M_get_Tp_allocator());
}
__catch(...)
{
- std::_Destroy(__new_start, __new_finish,
- _M_get_Tp_allocator());
+ if (__destroy_from)
+ std::_Destroy(__destroy_from, __destroy_from + __n,
+ _M_get_Tp_allocator());
_M_deallocate(__new_start, __len);
__throw_exception_again;
}
@@ -628,7 +627,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
this->_M_impl._M_end_of_storage
- this->_M_impl._M_start);
this->_M_impl._M_start = __new_start;
- this->_M_impl._M_finish = __new_finish;
+ this->_M_impl._M_finish = __new_start + __size + __n;
this->_M_impl._M_end_of_storage = __new_start + __len;
}
}
diff --git
a/libstdc++-v3/testsuite/23_containers/vector/capacity/resize/strong_guarantee.cc
b/libstdc++-v3/testsuite/23_containers/vector/capacity/resize/strong_guarantee.cc
new file mode 100644
index 000..b209d76867a
--- /dev/null
+++
b/libstdc++-v3/testsuite/23_containers/vector/capacity/resize/strong_guarantee.cc
@@ -0,0 +1,60 @@
+// Copyright (C) 2018 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
[PATCH] tighten up -Wbuiltin-declaration-mismatch (PR 86125)
The C implementation of the -Wbuiltin-declaration-mismatch
warning relies on TYPE_MODE to detect incompatibilities
between return and argument types in user declarations of
built-in functions. That prevents mistakes like
char* strlen (const char*);
from being detected (where sizeof (char*) == sizeof (size_t)),
while at the same diagnosing similar bugs such as
char* strcmp (const char*, const char*);
where sizeof (char*) != sizeof (int), and always diagnosing
benign declarations like:
void strcpy (char*, const char*)
The attached patch tightens up the detection of incompatible
types so that when -Wextra is set it diagnoses more of these
kinds of problems, including mismatches in qualifiers. (I
added this under -Wextra mainly to avoid the warning with
just -Wall for some of the more benign incompatibilities
like those in const-qualifiers).
This enhancement was prompted by bug 86114. As it is, it
would not have prevented the ICE in that bug because it does
not reject the incompatible redeclaration (I did that to keep
compatibility with prior GCC versions). If there is support
for it, though, I think rejecting all incompatible declarations
would be a better solution. Partly because the middle-end
doesn't seem to fully consider incompatible return types and
so might end up introducing transformations that don't make
sense. And partly because I think at least the C and POSIX
standard built-in functions should be declared with
the types they are specified.
Martin
PR c/86125 - missing -Wbuiltin-declaration-mismatch on a mismatched return type
gcc/c/ChangeLog:
PR c/86125
* c-decl.c (match_builtin_function_types): Add arguments.
(diagnose_mismatched_decls): Diagnose mismatched declarations
of built-ins more strictly.
gcc/testsuite/ChangeLog:
PR c/86125
* gcc.dg/Wbuiltin-declaration-mismatch.c: New test.
Index: gcc/c/c-decl.c
===
--- gcc/c/c-decl.c (revision 261551)
+++ gcc/c/c-decl.c (working copy)
@@ -1630,41 +1630,54 @@ c_bind (location_t loc, tree decl, bool is_global)
/* Subroutine of compare_decls. Allow harmless mismatches in return
and argument types provided that the type modes match. This function
- return a unified type given a suitable match, and 0 otherwise. */
+ returns a unified type given a suitable match, and 0 otherwise. */
static tree
-match_builtin_function_types (tree newtype, tree oldtype)
+match_builtin_function_types (tree newtype, tree oldtype,
+ tree *strict, unsigned *argno)
{
- tree newrettype, oldrettype;
- tree newargs, oldargs;
- tree trytype, tryargs;
-
/* Accept the return type of the new declaration if same modes. */
- oldrettype = TREE_TYPE (oldtype);
- newrettype = TREE_TYPE (newtype);
+ tree oldrettype = TREE_TYPE (oldtype);
+ tree newrettype = TREE_TYPE (newtype);
+ *argno = 0;
+ *strict = NULL_TREE;
+
if (TYPE_MODE (oldrettype) != TYPE_MODE (newrettype))
return NULL_TREE;
- oldargs = TYPE_ARG_TYPES (oldtype);
- newargs = TYPE_ARG_TYPES (newtype);
- tryargs = newargs;
+ if (!comptypes (oldrettype, newrettype))
+*strict = oldrettype;
- while (oldargs || newargs)
+ tree oldargs = TYPE_ARG_TYPES (oldtype);
+ tree newargs = TYPE_ARG_TYPES (newtype);
+ tree tryargs = newargs;
+
+ for (unsigned i = 1; oldargs || newargs; ++i)
{
if (!oldargs
|| !newargs
|| !TREE_VALUE (oldargs)
- || !TREE_VALUE (newargs)
- || TYPE_MODE (TREE_VALUE (oldargs))
- != TYPE_MODE (TREE_VALUE (newargs)))
+ || !TREE_VALUE (newargs))
return NULL_TREE;
+ tree oldtype = TREE_VALUE (oldargs);
+ tree newtype = TREE_VALUE (newargs);
+ if (TYPE_MODE (TREE_VALUE (oldargs))
+ != TYPE_MODE (TREE_VALUE (newargs)))
+ return NULL_TREE;
+
+ if (!*strict && !comptypes (oldtype, newtype))
+ {
+ *argno = i;
+ *strict = oldtype;
+ }
+
oldargs = TREE_CHAIN (oldargs);
newargs = TREE_CHAIN (newargs);
}
- trytype = build_function_type (newrettype, tryargs);
+ tree trytype = build_function_type (newrettype, tryargs);
/* Allow declaration to change transaction_safe attribute. */
tree oldattrs = TYPE_ATTRIBUTES (oldtype);
@@ -1874,10 +1887,19 @@ diagnose_mismatched_decls (tree newdecl, tree oldd
if (TREE_CODE (olddecl) == FUNCTION_DECL
&& DECL_BUILT_IN (olddecl) && !C_DECL_DECLARED_BUILTIN (olddecl))
{
- /* Accept harmless mismatch in function types.
- This is for the ffs and fprintf builtins. */
- tree trytype = match_builtin_function_types (newtype, oldtype);
+ /* Accept "harmless" mismatch in function types such as
+ missing qualifiers or pointer vs same size integer
+ mismatches. This is for the ffs and fprintf builtins.
+ However, with -Wextra in effect, diagnose return and
+ argument types that are incompatible according to
+ language rules. */
+ tree mismatch_expect;
+ unsigned mismatch_argno;
+ tree trytype =
Re: [Committed] PR fortran/86110 -- Avoid NULL pointer dereference
Great! Thanks, Steve. On 13 June 2018 at 20:43, Steve Kargl wrote: > Committed as obviously. > > 2018-06-13 Steven G. Kargl > > PR fortran/86110 > * array.c (gfc_resolve_character_array_constructor): Avoid NULL > pointer dereference. > > 2018-06-13 Steven G. Kargl > > PR fortran/86110 > * gfortran.dg/pr86110.f90: New test. > > -- > Steve -- "If you can't explain it simply, you don't understand it well enough" - Albert Einstein
Re: [PATCH] restore -Wstringop-overflow for checked strcpy/strcat (PR 85259)
On 06/13/2018 02:45 PM, Joseph Myers wrote:
This patch (commit r261518) has broken the build of the glibc testsuite.
I see, for example for aarch64-linux-gnu with build-many-glibcs.py:
In function 'test_strncat',
inlined from 'main' at tester.c:1621:3:
tester.c:490:13: error: 'strncat' accessing 18446744073709551600 or more bytes
at offsets 0 and 0 overlaps 9223372036854775793 bytes at offset
-9223372036854775793 [-Werror=restrict]
check (strncat (buf1 + n2, buf2 + n1, ~((size_t) 0) - n4)
^~
cc1: all warnings being treated as errors
The code in question already has -Wstringop-overflow= and -Warray-bounds
warnings disabled as it's deliberately testing a large size. But a
-Wrestrict warning makes no sense to me at all here. The arguments are
based on two different variables, buf1 and buf2, and regardless of sizes
and offsets I wouldn't expect a -Wrestrict warning when the arguments are
based on different array variables like that.
I couldn't readily reproduce the warning with the Glibc test
but I think it triggers for the same reason as the one below:
extern char a[16], b[16];
void f (void)
{
__builtin_strncat (a, b, __SIZE_MAX__ - 16);
}
In function ‘f’:
warning: ‘__builtin_strncat’ accessing 18446744073709551599 or more
bytes at offsets 0 and 0 overlaps 9223372036854775792 bytes at offset
-9223372036854775792 [-Wrestrict]
There's logic in -Wrestrict that tries to determine the sizes
even distinct objects would need to have in order for an access
to them of a given kind not to overlap. (This code starts on
line 1209 in gimple-ssa-warn-restrict.c.)This is probably
unnecessary when the objects sizes are known (and may even be
redundant when they aren't in some cases since other warnings
might catch that). Then again, the test does some bad things
so it's not surprising (or necessarily bad) that they get
caught by multiple checkers.
What could stand to be improved is the diagnostic. Some of
the offsets in it don't look very meaningful (they may not
be wrong, but they don't help the user make sense out of
the warning).
Martin
Re: [PATCH], PowerPC long double transistion, patch #3
On Wed, Jun 13, 2018 at 05:21:55PM -0400, Michael Meissner wrote: > In addition to the previous patch to aid in transitioning the PowerPC long > double format to IEEE 128-bit, I have some additional patches that are needed. > The previous patch is: > https://gcc.gnu.org/ml/gcc-patches/2018-06/msg00634.html > > This patch prevents re-running the ibm128 and ieee128 initialization functions > when the compiler was handling the clone and target attributes/pragmas. In > particular, the compiler would issue an internal compiler error or segfault > when the complex multiply/divide built-in functions were re-defined. > > Note, the GLIBC library that I used is not fully converted. I'm ignoring the > issues that are due to the library, and just focusing on the compiler support. > > I have done separate bootstraps on a little endian power8 system with the long > double type set to IBM extended and IEEE 128-bit extended. There were no > regressions in using this patch. Can I check it in, and eventually back port > it to GCC 8.2 with the other long double transition patches. With the various library failures, I didn't notice that this patch does break LTO. So I will need to rework this patch so that it fixes both LTO and clone/target attributes. -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
Re: C++ PATCH for c++/86094, wrong calling convention for move-only class
On Wed, Jun 13, 2018 at 5:08 PM, Jakub Jelinek wrote: > > On Wed, Jun 13, 2018 at 04:12:25PM -0400, Jason Merrill wrote: > > On Mon, Jun 11, 2018 at 2:38 PM, Jason Merrill wrote: > > > The fix for 80178 was broken, because I forgot that copy_fn_p is false > > > for move constructors. As a result, the calling convention for a > > > class with a trivial move constructor and deleted copy constructor > > > changed inappropriately. > > > > This patch restores the broken behavior to -fabi-version=12 and adds > > -fabi-version=13 for the fix; people can use -Wabi=12 with GCC 8.2 to > > check for compatibility issues against 8.1, or -Wabi=11 to check > > compatibility with GCC 7. > > > > Tested x86_64-pc-linux-gnu, applying to trunk and 8. Do we want to > > accelerate 8.2 because of this issue? > > I'd like to see the powerpc64le-linux __ieee128 long double support in 8.2 > too, accelerating 8.2 because of this would make that impossible. I'd think we could put that support in an August release regardless of what version number is on that release. Is it important that it be numbered 8.2? Jason
Re: [PATCH], PowerPC long double transistion, patch #4
In addition to the previous patch to aid in transitioning the PowerPC long
double format to IEEE 128-bit, I have some additional patches that are needed.
The previous patch is:
https://gcc.gnu.org/ml/gcc-patches/2018-06/msg00634.html
This patch fixes the power8 implementation of copysign for IEEE 128-bit
floating point. In particular, the way the temporary register was allocated
did not use the normal GCC conventions of using a clobber with match_scratch.
Because the constraint did not include a '&', the temporary register could have
used one of the input registers.
Note, the GLIBC library that I used is not fully converted. I'm ignoring the
issues that are due to the library, and just focusing on the compiler support.
I have done separate bootstraps on a little endian power8 system with the long
double type set to IBM extended and IEEE 128-bit extended. There were no
regressions in using this patch. Can I check it in, and eventually back port
it to GCC 8.2 with the other long double transition patches.
2018-06-13 Michael Meissner
* config/rs6000/rs6000.md (copysign3, IEEE iterator): Rework
copysign of float128 on ISA 2.07 to use an explicit clobber,
instead of passing in a temporary.
(copysign3_soft): Likewise.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
Index: gcc/config/rs6000/rs6000.md
===
--- gcc/config/rs6000/rs6000.md (revision 261512)
+++ gcc/config/rs6000/rs6000.md (working copy)
@@ -14102,11 +14102,8 @@ (define_expand "copysign3"
emit_insn (gen_copysign3_hard (operands[0], operands[1],
operands[2]));
else
-{
- rtx tmp = gen_reg_rtx (mode);
- emit_insn (gen_copysign3_soft (operands[0], operands[1],
- operands[2], tmp));
-}
+emit_insn (gen_copysign3_soft (operands[0], operands[1],
+operands[2]));
DONE;
})
@@ -14125,9 +14122,9 @@ (define_insn "copysign3_soft"
[(set (match_operand:IEEE128 0 "altivec_register_operand" "=v")
(unspec:IEEE128
[(match_operand:IEEE128 1 "altivec_register_operand" "v")
- (match_operand:IEEE128 2 "altivec_register_operand" "v")
- (match_operand:IEEE128 3 "altivec_register_operand" "+v")]
-UNSPEC_COPYSIGN))]
+ (match_operand:IEEE128 2 "altivec_register_operand" "v")]
+UNSPEC_COPYSIGN))
+ (clobber (match_scratch:IEEE128 3 "="))]
"!TARGET_FLOAT128_HW && FLOAT128_IEEE_P (mode)"
"xscpsgndp %x3,%x2,%x1\;xxpermdi %x0,%x3,%x1,1"
[(set_attr "type" "veccomplex")
Re: [PATCH], PowerPC long double transistion, patch #3
In addition to the previous patch to aid in transitioning the PowerPC long
double format to IEEE 128-bit, I have some additional patches that are needed.
The previous patch is:
https://gcc.gnu.org/ml/gcc-patches/2018-06/msg00634.html
This patch prevents re-running the ibm128 and ieee128 initialization functions
when the compiler was handling the clone and target attributes/pragmas. In
particular, the compiler would issue an internal compiler error or segfault
when the complex multiply/divide built-in functions were re-defined.
Note, the GLIBC library that I used is not fully converted. I'm ignoring the
issues that are due to the library, and just focusing on the compiler support.
I have done separate bootstraps on a little endian power8 system with the long
double type set to IBM extended and IEEE 128-bit extended. There were no
regressions in using this patch. Can I check it in, and eventually back port
it to GCC 8.2 with the other long double transition patches.
2018-06-13 Michael Meissner
* config/rs6000/rs6000.c (rs6000_init_libfuncs): Do not re-run the
initialization during clone or target pragma/attribute
processing.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c (revision 261512)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -18000,6 +18000,13 @@ init_float128_ieee (machine_mode mode)
static void
rs6000_init_libfuncs (void)
{
+ static bool done_init = false;
+
+ if (done_init)
+return;
+
+ done_init = true;
+
/* __float128 support. */
if (TARGET_FLOAT128_TYPE)
{
[PATCH], PowerPC long double transistion, patch #2
In addition to the previous patch to aid in transitioning the PowerPC long
double format to IEEE 128-bit, I have some additional patches that are needed.
The previous patch is:
https://gcc.gnu.org/ml/gcc-patches/2018-06/msg00634.html
This fixes some of the PowerPC tests that had implicit assumptions about the
long double format. The fixes involved:
1) Using long double __complex instead of using a KC attribute;
2) Explicitly adding -mabi=ibmlongdouble; (or)
3) Using __ibm128 instead of long double if long double is IEEE 128-bit.
I have done separate bootstraps on a little endian power8 system with the long
double type set to IBM extended and IEEE 128-bit extended. There were no
regressions in using this patch, and it fixes several of the errors when you
use a compiler with long double defaulting to IEEE 128-bit floating point. Can
I check it in, and eventually back port it to GCC 8.2 with the other long
double transition patches.
2018-06-13 Michael Meissner
* gcc.target/powerpc/divkc3-1.c: If long double is IEEE 128-bit,
use __complex long double instead of KC attribute.
* gcc.target/powerpc/mulkc3-1.c: Likewise.
* gcc.target/powerpc/float128-3.c: Add -mabi=ibmlongdouble to
tests that require long double to be IBM extended double.
* gcc.target/powerpc/float128-5.c: Likewise.
* gcc.target/powerpc/float128-mix.c: Likewise.
* gcc.target/powerpc/pack02.c: On systems where long double is
IEEE 128-bit, use __ibm128 instead of long double. Use the ibm128
pack/unpack functions instead of the long double pack/unpack
functions.
* gcc.target/powerpc/pr57150.c: Likewise.
* gcc.target/powerpc/pr60203.c: Likewise.
* gcc.target/powerpc/pr67808.c: Likewise.
* gcc.target/powerpc/pr70117.c: Likewise.
* gcc.target/powerpc/tfmode_off.c: Likewise.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
Index: gcc/testsuite/gcc.target/powerpc/divkc3-1.c
===
--- gcc/testsuite/gcc.target/powerpc/divkc3-1.c (revision 261511)
+++ gcc/testsuite/gcc.target/powerpc/divkc3-1.c (working copy)
@@ -3,7 +3,11 @@
void abort ();
-typedef __complex float __cfloat128 __attribute__((mode(KC)));
+#ifdef __LONG_DOUBLE_IEEE128__
+typedef __complex long double __cfloat128;
+#else
+typedef __complex float __cfloat128 __attribute__((mode(__KC__)));
+#endif
__cfloat128 divide (__cfloat128 x, __cfloat128 y)
{
Index: gcc/testsuite/gcc.target/powerpc/float128-3.c
===
--- gcc/testsuite/gcc.target/powerpc/float128-3.c (revision 261511)
+++ gcc/testsuite/gcc.target/powerpc/float128-3.c (working copy)
@@ -1,6 +1,6 @@
/* { dg-do compile { target { powerpc*-*-linux* } } } */
/* { dg-require-effective-target powerpc_vsx_ok } */
-/* { dg-options "-O2 -mvsx -mno-float128" } */
+/* { dg-options "-O2 -mvsx -mno-float128 -mabi=ibmlongdouble -Wno-psabi" } */
/* Test that we can use #pragma GCC target to enable -mfloat128. */
Index: gcc/testsuite/gcc.target/powerpc/float128-5.c
===
--- gcc/testsuite/gcc.target/powerpc/float128-5.c (revision 261511)
+++ gcc/testsuite/gcc.target/powerpc/float128-5.c (working copy)
@@ -1,6 +1,6 @@
/* { dg-do compile { target { powerpc*-*-linux* && lp64 } } } */
/* { dg-require-effective-target powerpc_p9vector_ok } */
-/* { dg-options "-O2 -mpower9-vector -mno-float128" } */
+/* { dg-options "-O2 -mpower9-vector -mno-float128 -mabi=ibmlongdouble
-Wno-psabi" } */
/* Test that we can use #pragma GCC target to enable -mfloat128 and generate
code on ISA 3.0 for the float128 built-in functions. Lp64 is required
Index: gcc/testsuite/gcc.target/powerpc/float128-mix.c
===
--- gcc/testsuite/gcc.target/powerpc/float128-mix.c (revision 261511)
+++ gcc/testsuite/gcc.target/powerpc/float128-mix.c (working copy)
@@ -1,6 +1,6 @@
/* { dg-do compile { target { powerpc*-*-linux* } } } */
/* { dg-require-effective-target powerpc_vsx_ok } */
-/* { dg-options "-O2 -mvsx" } */
+/* { dg-options "-O2 -mvsx -mabi=ibmlongdouble -Wno-psabi" } */
/* Test to make sure that __float128 and long double cannot be combined
together. */
Index: gcc/testsuite/gcc.target/powerpc/mulkc3-1.c
===
--- gcc/testsuite/gcc.target/powerpc/mulkc3-1.c (revision 261511)
+++ gcc/testsuite/gcc.target/powerpc/mulkc3-1.c (working copy)
@@ -3,7 +3,11 @@
void abort ();
-typedef __complex float __cfloat128 __attribute__((mode(KC)));
+#ifdef __LONG_DOUBLE_IEEE128__
+typedef __complex long double __cfloat128;
+#else
+typedef __complex float __cfloat128
[PATCH], PowerPC long double transistion, patch #1
In addition to the previous patch to aid in transitioning the PowerPC long double format to IEEE 128-bit, I have some additional patches that are needed. The previous patch is: https://gcc.gnu.org/ml/gcc-patches/2018-06/msg00634.html This patch turns off setting the .gnu_attributes when building the libgcc helper functions. I ran into some linker warnings as I built some multilib toolchains, and this turns off those warnings. I have done separate bootstraps on a little endian power8 system with the long double type set to IBM extended and IEEE 128-bit extended. There were no regressions in using this patch. Can I check it in, and eventually back port it to GCC 8.2 with the other long double transition patches? 2018-06-11 Michael Meissner * config/rs6000/t-float128 (FP128_CFLAGS_SW): Compile float128 support modules with -mno-gnu-attribute. * config/rs6000/t-float128-hw (FP128_CFLAGS_HW): Likewise. -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797 Index: libgcc/config/rs6000/t-float128 === --- libgcc/config/rs6000/t-float128 (revision 261338) +++ libgcc/config/rs6000/t-float128 (working copy) @@ -59,7 +59,7 @@ fp128_includes= $(srcdir)/soft-fp/doub # Build the emulator without ISA 3.0 hardware support. FP128_CFLAGS_SW = -Wno-type-limits -mvsx -mfloat128 \ - -mno-float128-hardware \ + -mno-float128-hardware -mno-gnu-attribute \ -I$(srcdir)/soft-fp \ -I$(srcdir)/config/rs6000 \ $(FLOAT128_HW_INSNS) Index: libgcc/config/rs6000/t-float128-hw === --- libgcc/config/rs6000/t-float128-hw (revision 261338) +++ libgcc/config/rs6000/t-float128-hw (working copy) @@ -25,7 +25,7 @@ fp128_sed_hw = -hw # Build the hardware support functions with appropriate hardware support FP128_CFLAGS_HW = -Wno-type-limits -mvsx -mfloat128 \ -mpower8-vector -mpower9-vector \ - -mfloat128-hardware \ + -mfloat128-hardware -mno-gnu-attribute \ -I$(srcdir)/soft-fp \ -I$(srcdir)/config/rs6000 \ $(FLOAT128_HW_INSNS)
Re: C++ PATCH for c++/86094, wrong calling convention for move-only class
On Wed, Jun 13, 2018 at 04:12:25PM -0400, Jason Merrill wrote: > On Mon, Jun 11, 2018 at 2:38 PM, Jason Merrill wrote: > > The fix for 80178 was broken, because I forgot that copy_fn_p is false > > for move constructors. As a result, the calling convention for a > > class with a trivial move constructor and deleted copy constructor > > changed inappropriately. > > This patch restores the broken behavior to -fabi-version=12 and adds > -fabi-version=13 for the fix; people can use -Wabi=12 with GCC 8.2 to > check for compatibility issues against 8.1, or -Wabi=11 to check > compatibility with GCC 7. > > Tested x86_64-pc-linux-gnu, applying to trunk and 8. Do we want to > accelerate 8.2 because of this issue? I'd like to see the powerpc64le-linux __ieee128 long double support in 8.2 too, accelerating 8.2 because of this would make that impossible. Jakub
Re: C++ PATCH for c++/86094, wrong calling convention for move-only class
On Wed, Jun 13, 2018 at 4:12 PM, Jason Merrill wrote:
> On Mon, Jun 11, 2018 at 2:38 PM, Jason Merrill wrote:
>> The fix for 80178 was broken, because I forgot that copy_fn_p is false
>> for move constructors. As a result, the calling convention for a
>> class with a trivial move constructor and deleted copy constructor
>> changed inappropriately.
>
> This patch restores the broken behavior to -fabi-version=12 and adds
> -fabi-version=13 for the fix; people can use -Wabi=12 with GCC 8.2 to
> check for compatibility issues against 8.1, or -Wabi=11 to check
> compatibility with GCC 7.
>
> Tested x86_64-pc-linux-gnu, applying to trunk and 8. Do we want to
> accelerate 8.2 because of this issue?
And one more patch, to suggest -Wabi=11 rather than useless plain -Wabi.
Tested x86_64-pc-linux-gnu, applying to trunk and 8.
commit bf7773e6fa607769b619535e1b22272a36726868
Author: Jason Merrill
Date: Wed Jun 13 16:09:46 2018 -0400
* c-opts.c (c_common_post_options): Warn about useless -Wabi.
(c_common_handle_option) [OPT_Wabi_]: Remove flag_abi_compat_version
handling.
diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
index 107359ec20d..bbcb1bb1a9c 100644
--- a/gcc/c-family/c-opts.c
+++ b/gcc/c-family/c-opts.c
@@ -417,8 +417,6 @@ c_common_handle_option (size_t scode, const char *arg, int value,
value = 2;
}
warn_abi_version = value;
- if (flag_abi_compat_version == -1)
- flag_abi_compat_version = value;
break;
case OPT_fcanonical_system_headers:
@@ -942,7 +940,17 @@ c_common_post_options (const char **pfilename)
{
warn_abi_version = latest_abi_version;
if (flag_abi_version == latest_abi_version)
- flag_abi_compat_version = abi_compat_default;
+ {
+ if (warning (OPT_Wabi, "-Wabi won't warn about anything"))
+ {
+ inform (input_location, "-Wabi warns about differences "
+ "from the most up-to-date ABI, which is also used "
+ "by default");
+ inform (input_location, "use e.g. -Wabi=11 to warn about "
+ "changes from GCC 7");
+ }
+ flag_abi_compat_version = abi_compat_default;
+ }
else
flag_abi_compat_version = latest_abi_version;
}
diff --git a/gcc/testsuite/g++.dg/abi/pr83489.C b/gcc/testsuite/g++.dg/abi/pr83489.C
index aee04eca9d8..b6c82ef16e6 100644
--- a/gcc/testsuite/g++.dg/abi/pr83489.C
+++ b/gcc/testsuite/g++.dg/abi/pr83489.C
@@ -1,5 +1,5 @@
// PR c++/83489
-// { dg-options "-Wabi" }
+// { dg-options "-Wabi=11" }
struct A
{
diff --git a/gcc/testsuite/obj-c++.dg/bitfield-1.mm b/gcc/testsuite/obj-c++.dg/bitfield-1.mm
index a63761904b5..084d2cb0916 100644
--- a/gcc/testsuite/obj-c++.dg/bitfield-1.mm
+++ b/gcc/testsuite/obj-c++.dg/bitfield-1.mm
@@ -5,7 +5,7 @@
superclasses should be removed). */
/* Contributed by Ziemowit Laski . */
/* { dg-do run } */
-/* { dg-options "-Wpadded -Wabi" } */
+/* { dg-options "-Wpadded -Wabi=8" } */
/* Leave blank lines here to keep warnings on the same lines. */
diff --git a/gcc/testsuite/obj-c++.dg/layout-1.mm b/gcc/testsuite/obj-c++.dg/layout-1.mm
index 33879ad17a4..89921236ec3 100644
--- a/gcc/testsuite/obj-c++.dg/layout-1.mm
+++ b/gcc/testsuite/obj-c++.dg/layout-1.mm
@@ -1,7 +1,7 @@
/* Ensure that we do not get bizarre warnings referring to
__attribute__((packed)) or some such. */
/* { dg-do compile } */
-/* { dg-options "-Wpadded -Wpacked -Wabi" } */
+/* { dg-options "-Wpadded -Wpacked -Wabi=8" } */
#include "../objc-obj-c++-shared/TestsuiteObject.h"
RE: [PATCH] MIPS: Add support for P6600
Hi Matthew,
> With one more change to add another comment as below, this is OK to
> commit.
>
> > @@ -18957,7 +19039,10 @@ mips_reorg_process_insns (void)
> > sequence and replace it with the delay slot instruction
> > then the jump to clear the forbidden slot hazard. */
>
> This bit does need the comment extending. Add this:
>
> For the P6600, this optimisation solves the performance penalty associated
> with BALC followed by a delay slot branch. We do not set fs_delay as we
> do not want the full logic of a forbidden slot; the penalty exists only
> against branches not the full class of forbidden slot instructions.
>
> >
> > - if (fs_delay)
> > + if (fs_delay || (TUNE_P6600
> > + && TARGET_CB_MAYBE
> > + && mips_classify_branch_p6600 (insn)
> > + == UC_BALC))
> > {
> > /* Search onwards from the current position looking for
> > a SEQUENCE. We are looking for pipeline hazards
> here
Added and committed as r261570.
Regards,
Robert
Re: [PATCH] restore -Wstringop-overflow for checked strcpy/strcat (PR 85259)
This patch (commit r261518) has broken the build of the glibc testsuite. I see, for example for aarch64-linux-gnu with build-many-glibcs.py: In function 'test_strncat', inlined from 'main' at tester.c:1621:3: tester.c:490:13: error: 'strncat' accessing 18446744073709551600 or more bytes at offsets 0 and 0 overlaps 9223372036854775793 bytes at offset -9223372036854775793 [-Werror=restrict] check (strncat (buf1 + n2, buf2 + n1, ~((size_t) 0) - n4) ^~ cc1: all warnings being treated as errors The code in question already has -Wstringop-overflow= and -Warray-bounds warnings disabled as it's deliberately testing a large size. But a -Wrestrict warning makes no sense to me at all here. The arguments are based on two different variables, buf1 and buf2, and regardless of sizes and offsets I wouldn't expect a -Wrestrict warning when the arguments are based on different array variables like that. -- Joseph S. Myers jos...@codesourcery.com
Re: [patch, fortran] Handling of .and. and .or. expressions
On Mon, Jun 11, 2018 at 09:22:27PM +0200, Thomas Koenig wrote:
>
> Regression-tested (which found one bug in the testsuite).
>
> OK for trunk?
>
I fine with the intent of the patch (see below).
PS: I think there may be some confusion on what IMPURE implies.
> Index: fortran/resolve.c
> ===
> --- fortran/resolve.c (Revision 261388)
> +++ fortran/resolve.c (Arbeitskopie)
> @@ -3807,7 +3807,43 @@ lookup_uop_fuzzy (const char *op, gfc_symtree *uop
>return gfc_closest_fuzzy_match (op, candidates);
> }
>
> +/* Callback finding an impure function as an operand to an .and. or
> + .or. expression. Remember the last function warned about to
> + avoid double warnings when recursing. */
>
> +static int
> +impure_function_callback (gfc_expr **e, int *walk_subtrees ATTRIBUTE_UNUSED,
> + void *data)
> +{
> + gfc_expr *f = *e;
> + const char *name;
> + static gfc_expr *last = NULL;
> + bool *found = (bool *) data;
> +
> + if (f->expr_type == EXPR_FUNCTION)
> +{
> + *found = 1;
Why not true? *found is declared to be bool.
> + if (name)
> + gfc_warning (OPT_Wsurprising, "Impure function %qs at %L "
> + "might not be evaluated", name, >where);
> + else
> + gfc_warning (OPT_Wsurprising, "Impure function at %L "
> + "might not be evaluated", >where);
I think that this can simply be "Function %qs at %L may not be evaluated"
> + /* Some people code which depends on the short-circuiting that
> + Fortran does not provide, such as
The above seems a little muddled. I suggest a shorter comment.
"Some programmers assume that Fortran supports short-circuiting in logical
expression, which can lead to surprising behavior. For example, in the
following
> +
> + if (associated(m) .and. m%t) then
m%t may be evaluated before associated(m).
> + So, warn about this idiom. However, avoid breaking
> + it on purpose. */
> +
> + if (op1->expr_type == EXPR_FUNCTION && op1->value.function.isym
> + && op1->value.function.isym->id == GFC_ISYM_ASSOCIATED)
> + {
> + gfc_expr *e = op1->value.function.actual->expr;
> + gfc_expr *en = op1->value.function.actual->next->expr;
> + if (en == NULL && gfc_check_dependency (e, op2, true))
> + {
> + gfc_warning (OPT_Wsurprising, "%qs function call at %L
> does "
> +"not guard expression at %L", "ASSOCIATED",
> +>where, >where);
> + dont_move = true;
> + }
> + }
> +
> + /* A bit of optimization: Transfer if (f(x) .and. flag)
> + into if (flag .and. f(x)), to save evaluation of a
s/transfer/transform
I would also put quotes around the Fortran code.
> + function. The middle end should be capable of doing
> + this with a TRUTH_AND_EXPR, but it currently does not do
> + so. See PR 85599. */
The rest looks ok, but I'm not sure if this addresses Janus'
concerns.
--
Steve
Re: [patch, fortran] Handling of .and. and .or. expressions
Am 13.06.2018 um 14:20 schrieb Janus Weil: Hi Thomas, the attached patch introduces the following changes: thanks a lot for working on this! If a logical .and. or .or. expression contains a reference to a function which is impure and which also does not behave like a pure function (i.e. does not have the implicit_pure attribute set), it emits a warning with -Wsurprising that the function might not be evaluated. (-Wsurprising is enabled by -Wall). I think you are warning about too many cases here. Warning about an impure function as the *second* operand of the logical operators should be enough, I think. AFAICS there is no danger of the first operand not being evaluated, right? Not necessarily. The middle end, at the moment, may lack this optimization; at least nobody has so far come up with a test case that demonstrates otherwise. On the other hand, people who know the middle end extremely well have indicated that they expect optimiazations to happen with TRUTH_AND_EXPR, so I am far from certaint that this behavior will continue. It special cases the idiom if (associated(m) .and. m%t) which people appear to use. I don't fully understand why you do this, but in any case it should not be necessary if you warn about the second operand only. Well, it is an easy mistake to make, and it is (as above) liable to break at any time. Also ASSOCIATED (and ALLOCATED) are pure, so normally I would not warn on these. And, if there is an expression like func() .and. flag , it reverses the test as an optimization. I really don't like this part. It actually introduces more problems of the sort that we're trying to warn about ... If we actually perform this operation, we will have warned about this before (with -Wsurprising). And of course, the compiler can always reverse the order... Regards Thomas
Go patch committed: Avoid introducing redundant write barriers
This patch to the Go frontend by Than McIntosh fixes the compier to
avoid introducing redundant write barriers. The traversal used by the
write barrier insertion phase can sometimes wind up visiting new
statements inserted during the traversal, which then results in
duplicate / redundant write barrier guards. Example program to
reproduce:
package small
type S struct {
N *S
K int
}
var G *S = {N: nil, K: 101}
This patch changes the traversal code to keep track of statements
already added and avoid processing them again later in the traversal.
This fixes https://golang.org/25867 .
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed
to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 261555)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-1f07926263b6d14edb6abd6a00e6385190d30d0e
+c3ef5bbf4e4271216b6f22621269d458599e8087
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/gogo.cc
===
--- gcc/go/gofrontend/gogo.cc (revision 261521)
+++ gcc/go/gofrontend/gogo.cc (working copy)
@@ -8444,6 +8444,9 @@ Traverse::function_declaration(Named_obj
void
Statement_inserter::insert(Statement* s)
{
+ if (this->statements_added_ != NULL)
+this->statements_added_->insert(s);
+
if (this->block_ != NULL)
{
go_assert(this->pindex_ != NULL);
Index: gcc/go/gofrontend/gogo.h
===
--- gcc/go/gofrontend/gogo.h(revision 261521)
+++ gcc/go/gofrontend/gogo.h(working copy)
@@ -3419,19 +3419,24 @@ class Traverse
class Statement_inserter
{
public:
+ typedef Unordered_set(Statement*) Statements;
+
// Empty constructor.
Statement_inserter()
-: block_(NULL), pindex_(NULL), gogo_(NULL), var_(NULL)
+ : block_(NULL), pindex_(NULL), gogo_(NULL), var_(NULL),
+statements_added_(NULL)
{ }
// Constructor for a statement in a block.
- Statement_inserter(Block* block, size_t *pindex)
-: block_(block), pindex_(pindex), gogo_(NULL), var_(NULL)
+ Statement_inserter(Block* block, size_t *pindex, Statements *added = NULL)
+ : block_(block), pindex_(pindex), gogo_(NULL), var_(NULL),
+statements_added_(added)
{ }
// Constructor for a global variable.
- Statement_inserter(Gogo* gogo, Variable* var)
-: block_(NULL), pindex_(NULL), gogo_(gogo), var_(var)
+ Statement_inserter(Gogo* gogo, Variable* var, Statements *added = NULL)
+ : block_(NULL), pindex_(NULL), gogo_(gogo), var_(var),
+statements_added_(added)
{ go_assert(var->is_global()); }
// We use the default copy constructor and assignment operator.
@@ -3451,6 +3456,8 @@ class Statement_inserter
Gogo* gogo_;
// The global variable, when looking at an initializer expression.
Variable* var_;
+ // If non-null, a set to record new statements inserted (non-owned).
+ Statements* statements_added_;
};
// When translating the gogo IR into the backend data structure, this
Index: gcc/go/gofrontend/wb.cc
===
--- gcc/go/gofrontend/wb.cc (revision 261521)
+++ gcc/go/gofrontend/wb.cc (working copy)
@@ -213,7 +213,7 @@ class Write_barriers : public Traverse
public:
Write_barriers(Gogo* gogo)
: Traverse(traverse_functions | traverse_variables | traverse_statements),
- gogo_(gogo), function_(NULL)
+ gogo_(gogo), function_(NULL), statements_added_()
{ }
int
@@ -230,6 +230,8 @@ class Write_barriers : public Traverse
Gogo* gogo_;
// Current function.
Function* function_;
+ // Statements introduced.
+ Statement_inserter::Statements statements_added_;
};
// Traverse a function. Just record it for later.
@@ -298,9 +300,10 @@ Write_barriers::variable(Named_object* n
Location loc = init->location();
Expression* ref = Expression::make_var_reference(no, loc);
- Statement_inserter inserter(this->gogo_, var);
+ Statement_inserter inserter(this->gogo_, var, >statements_added_);
Statement* s = this->gogo_->assign_with_write_barrier(NULL, NULL, ,
ref, init, loc);
+ this->statements_added_.insert(s);
var->add_preinit_statement(this->gogo_, s);
var->clear_init();
@@ -313,6 +316,9 @@ Write_barriers::variable(Named_object* n
int
Write_barriers::statement(Block* block, size_t* pindex, Statement* s)
{
+ if (this->statements_added_.find(s) != this->statements_added_.end())
+return TRAVERSE_SKIP_COMPONENTS;
+
switch (s->classification())
{
default:
@@ -355,7 +361,7 @@ Write_barriers::statement(Block* block,
Function* function = this->function_;
Location loc = init->location();
- Statement_inserter
Re: libstdc++ bootstrap failure after r261525
Here is the missing part of the can_advance patch.
* include/debug/helper_functions.h
(__gnu_debug::_Safe_iterator<>): Add declaration.
(__can_advance(_Ite, _Size)): New.
(__can_advance(const _Safe_iterator<>&, _Size)): Overload declaration.
* include/debug/functions.h
(__gnu_debug::_Safe_iterator<>): Remove declaration.
* include/debug/stl_iterator.h
(__can_advance(const _Safe_iterator<>&)): New definition.
* include/debug/stl_iterator.h
(__can_advance(const std::reverse_iterator<>&, _Size)): New.
(__can_advance(const std::move_iterator<>&, _Size)): New.
* include/debug/macros.h (__glibcxx_check_can_increment): New.
* include/debug/debug.h (__glibcxx_requires_can_increment): New.
* include/bits/stl_algobase.h (fill_n): Use latter.
* testsuite/25_algorithms/fill_n/2.cc: New.
* testsuite/25_algorithms/fill_n/debug/1_neg.cc: New.
* testsuite/25_algorithms/fill_n/debug/2_neg.cc: New.
* testsuite/25_algorithms/fill_n/debug/3_neg.cc: New.
* testsuite/25_algorithms/fill_n/debug/4_neg.cc: New.
Tested under linux x86_64, normal and debug modes.
Now I understand why applying the patch in the first place failed.
François
On 13/06/2018 15:24, Jonathan Wakely wrote:
On 13/06/18 13:42 +0200, Paolo Carlini wrote:
Hi,
On 13/06/2018 08:11, François Dumont wrote:
On 12/06/2018 23:26, Martin Sebor wrote:
Bootstrap fails in libstdc++ with the following error.
I haven't investigated it much except to take a peek
at recent libstdc++ commits where r261525 looks like it
introduced __glibcxx_check_can_increment_range, so it
seems like it might have something to do with it. I don't
see a __glibcxx_requires_can_increment_range macro defined
anywhere in the patch so maybe it's a typo? Francois, can
you please see what's going on?
/opt/notnfs/msebor/build/gcc-86114/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_algobase.h:452:45:
error: ‘__glibcxx_requires_can_increment_range’ was not declared in
this scope
__glibcxx_requires_can_increment_range(__first, __last,
__result);
Thanks
Martin
.
I had missed the fact that the patch didn't apply correctly on
include/debug/debug.h as stated in the ChangeLog entry.
It is fixed now.
But even after r261537 we are seeing the regressions:
FAIL: g++.dg/warn/pr31246.C -std=gnu++11 (test for excess errors)
FAIL: g++.dg/warn/pr31246.C -std=gnu++14 (test for excess errors)
FAIL: g++.dg/warn/pr31246.C -std=gnu++98 (test for excess errors)
Which definitely are related.
And failing Debug Mode tests:
FAIL: 20_util/hash/84998.cc (test for excess errors)
FAIL: 23_containers/vector/cons/destructible_debug_neg.cc (test for
excess errors)
FAIL: 25_algorithms/binary_search/partitioned.cc (test for excess errors)
FAIL: 25_algorithms/equal_range/partitioned.cc (test for excess errors)
FAIL: 25_algorithms/lexicographical_compare/71545.cc (test for excess
errors)
FAIL: 25_algorithms/lower_bound/partitioned.cc (test for excess errors)
FAIL: 25_algorithms/upper_bound/partitioned.cc (test for excess errors)
FAIL: ext/profile/mutex_extensions_neg.cc (test for excess errors)
The logs show:
/home/jwakely/src/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_algobase.h:452:
error: '__can_advance' is not a member of '__gnu_debug'
/home/jwakely/src/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_algobase.h:484:
error: '__can_advance' is not a member of '__gnu_debug'
/home/jwakely/src/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_algobase.h:629:
error: '__can_advance' is not a member of '__gnu_debug'
/home/jwakely/src/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_algobase.h:664:
error: '__can_advance' is not a member of '__gnu_debug'
diff --git a/libstdc++-v3/include/bits/stl_algobase.h b/libstdc++-v3/include/bits/stl_algobase.h
index 877f32b..022a3f1 100644
--- a/libstdc++-v3/include/bits/stl_algobase.h
+++ b/libstdc++-v3/include/bits/stl_algobase.h
@@ -783,6 +783,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{
// concept requirements
__glibcxx_function_requires(_OutputIteratorConcept<_OI, _Tp>)
+ __glibcxx_requires_can_increment(__first, __n);
return _OI(std::__fill_n_a(std::__niter_base(__first), __n, __value));
}
diff --git a/libstdc++-v3/include/debug/debug.h b/libstdc++-v3/include/debug/debug.h
index d055dcb..f157a6d 100644
--- a/libstdc++-v3/include/debug/debug.h
+++ b/libstdc++-v3/include/debug/debug.h
@@ -62,6 +62,7 @@ namespace __gnu_debug
# define __glibcxx_requires_cond(_Cond,_Msg)
# define __glibcxx_requires_valid_range(_First,_Last)
+# define __glibcxx_requires_can_increment(_First,_Size)
# define __glibcxx_requires_can_increment_range(_First1,_Last1,_First2)
# define __glibcxx_requires_can_decrement_range(_First1,_Last1,_First2)
# define __glibcxx_requires_sorted(_First,_Last)
@@ -88,6 +89,8 @@ namespace __gnu_debug
# define __glibcxx_requires_cond(_Cond,_Msg) _GLIBCXX_DEBUG_VERIFY(_Cond,_Msg)
Re: C++ PATCH for c++/86094, wrong calling convention for move-only class
On Mon, Jun 11, 2018 at 2:38 PM, Jason Merrill wrote:
> The fix for 80178 was broken, because I forgot that copy_fn_p is false
> for move constructors. As a result, the calling convention for a
> class with a trivial move constructor and deleted copy constructor
> changed inappropriately.
This patch restores the broken behavior to -fabi-version=12 and adds
-fabi-version=13 for the fix; people can use -Wabi=12 with GCC 8.2 to
check for compatibility issues against 8.1, or -Wabi=11 to check
compatibility with GCC 7.
Tested x86_64-pc-linux-gnu, applying to trunk and 8. Do we want to
accelerate 8.2 because of this issue?
commit f118d95dd7802a80387982135686be27b3e0dbb7
Author: Jason Merrill
Date: Wed Jun 13 13:35:29 2018 -0400
PR c++/86094 - wrong code with defaulted move ctor.
gcc/c-family/
* c-opts.c (c_common_post_options): Bump the current ABI version to
13. Set warn_abi_version and flag_abi_compat_version to the current
version rather than 0. Fix defaulting flag_abi_compat_version from
warn_abi_version.
gcc/cp/
* class.c (classtype_has_non_deleted_move_ctor): New.
* tree.c (maybe_warn_parm_abi, type_has_nontrivial_copy_init):
Handle v12 breakage.
diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
index ddaaef37e1d..107359ec20d 100644
--- a/gcc/c-family/c-opts.c
+++ b/gcc/c-family/c-opts.c
@@ -915,31 +915,38 @@ c_common_post_options (const char **pfilename)
if (flag_declone_ctor_dtor == -1)
flag_declone_ctor_dtor = optimize_size;
- if (warn_abi_version == -1)
-{
- if (flag_abi_compat_version != -1)
- warn_abi_version = flag_abi_compat_version;
- else
- warn_abi_version = 0;
-}
-
if (flag_abi_compat_version == 1)
{
warning (0, "%<-fabi-compat-version=1%> is not supported, using =2");
flag_abi_compat_version = 2;
}
- else if (flag_abi_compat_version == -1)
+
+ /* Change flag_abi_version to be the actual current ABI level, for the
+ benefit of c_cpp_builtins, and to make comparison simpler. */
+ const int latest_abi_version = 13;
+ /* Generate compatibility aliases for ABI v11 (7.1) by default. */
+ const int abi_compat_default = 11;
+
+#define clamp(X) if (X == 0 || X > latest_abi_version) X = latest_abi_version
+ clamp (flag_abi_version);
+ clamp (warn_abi_version);
+ clamp (flag_abi_compat_version);
+#undef clamp
+
+ /* Default -Wabi= or -fabi-compat-version= from each other. */
+ if (warn_abi_version == -1 && flag_abi_compat_version != -1)
+warn_abi_version = flag_abi_compat_version;
+ else if (flag_abi_compat_version == -1 && warn_abi_version != -1)
+flag_abi_compat_version = warn_abi_version;
+ else if (warn_abi_version == -1 && flag_abi_compat_version == -1)
{
- /* Generate compatibility aliases for ABI v11 (7.1) by default. */
- flag_abi_compat_version
- = (flag_abi_version == 0 ? 11 : 0);
+ warn_abi_version = latest_abi_version;
+ if (flag_abi_version == latest_abi_version)
+ flag_abi_compat_version = abi_compat_default;
+ else
+ flag_abi_compat_version = latest_abi_version;
}
- /* Change flag_abi_version to be the actual current ABI level for the
- benefit of c_cpp_builtins. */
- if (flag_abi_version == 0)
-flag_abi_version = 12;
-
/* By default, enable the new inheriting constructor semantics along with ABI
11. New and old should coexist fine, but it is a change in what
artificial symbols are generated. */
diff --git a/gcc/common.opt b/gcc/common.opt
index 4aebcaf0656..efbeba33819 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -938,7 +938,11 @@ Driver Undocumented
;
; 12: Corrects the calling convention for classes with only deleted copy/move
; constructors and changes passing/returning of empty records.
-; Default in G++ 8.
+; Default in G++ 8.1.
+;
+; 13: Fixes the accidental change in 12 to the calling convention for classes
+; with deleted copy constructor and trivial move constructor.
+; Default in G++ 8.2.
;
; Additional positive integers will be assigned as new versions of
; the ABI become the default version of the ABI.
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index b6e78c6377d..9a397e3856c 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -5175,6 +5175,19 @@ classtype_has_move_assign_or_move_ctor_p (tree t, bool user_p)
return false;
}
+/* True iff T has a move constructor that is not deleted. */
+
+bool
+classtype_has_non_deleted_move_ctor (tree t)
+{
+ if (CLASSTYPE_LAZY_MOVE_CTOR (t))
+lazily_declare_fn (sfk_move_constructor, t);
+ for (ovl_iterator iter (CLASSTYPE_CONSTRUCTORS (t)); iter; ++iter)
+if (move_fn_p (*iter) && !DECL_DELETED_FN (*iter))
+ return true;
+ return false;
+}
+
/* If T, a class, has a user-provided copy constructor, copy assignment
operator, or destructor, returns that function. Otherwise, null. */
diff --git
[PATCH 08/11] testsuite: Mark that PRU has one-cycle jumps
gcc/testsuite/ChangeLog:
2018-06-13 Dimitar Dimitrov
* gcc.dg/tree-ssa/20040204-1.c: XFAIL on pru.
* gcc.dg/tree-ssa/reassoc-33.c: Ditto.
* gcc.dg/tree-ssa/reassoc-34.c: Ditto.
* gcc.dg/tree-ssa/reassoc-35.c: Ditto.
* gcc.dg/tree-ssa/reassoc-36.c: Ditto.
* gcc.dg/tree-ssa/ssa-thread-14.c: Ditto.
Signed-off-by: Dimitar Dimitrov
---
gcc/testsuite/gcc.dg/tree-ssa/20040204-1.c| 2 +-
gcc/testsuite/gcc.dg/tree-ssa/reassoc-33.c| 2 +-
gcc/testsuite/gcc.dg/tree-ssa/reassoc-34.c| 2 +-
gcc/testsuite/gcc.dg/tree-ssa/reassoc-35.c| 2 +-
gcc/testsuite/gcc.dg/tree-ssa/reassoc-36.c| 2 +-
gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-14.c | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/20040204-1.c
b/gcc/testsuite/gcc.dg/tree-ssa/20040204-1.c
index a1237cf839b..06b83029fd3 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/20040204-1.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/20040204-1.c
@@ -33,4 +33,4 @@ void test55 (int x, int y)
that the && should be emitted (based on BRANCH_COST). Fix this
by teaching dom to look through && and register all components
as true. */
-/* { dg-final { scan-tree-dump-times "link_error" 0 "optimized" { xfail { !
"alpha*-*-* arm*-*-* aarch64*-*-* powerpc*-*-* cris-*-* crisv32-*-* hppa*-*-*
i?86-*-* mmix-*-* mips*-*-* m68k*-*-* moxie-*-* nds32*-*-* s390*-*-* sh*-*-*
sparc*-*-* spu-*-* visium-*-* x86_64-*-* riscv*-*-*" } } } } */
+/* { dg-final { scan-tree-dump-times "link_error" 0 "optimized" { xfail { !
"alpha*-*-* arm*-*-* aarch64*-*-* powerpc*-*-* cris-*-* crisv32-*-* hppa*-*-*
i?86-*-* mmix-*-* mips*-*-* m68k*-*-* moxie-*-* nds32*-*-* s390*-*-* sh*-*-*
sparc*-*-* spu-*-* visium-*-* x86_64-*-* riscv*-*-* pru*-*-*" } } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/reassoc-33.c
b/gcc/testsuite/gcc.dg/tree-ssa/reassoc-33.c
index 5572df4ae24..5e1cd1a6fa7 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/reassoc-33.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/reassoc-33.c
@@ -1,4 +1,4 @@
-/* { dg-do run { target { ! "m68k*-*-* mmix*-*-* bfin*-*-* v850*-*-*
moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-*
hppa*-*-* nios2*-*-*"} } } */
+/* { dg-do run { target { ! "m68k*-*-* mmix*-*-* bfin*-*-* v850*-*-*
moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-*
hppa*-*-* nios2*-*-* pru*-*-*"} } } */
/* { dg-options "-O2 -fno-inline -fdump-tree-reassoc1-details" } */
/* { dg-additional-options "-mbranch-cost=2" { target branch_cost } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/reassoc-34.c
b/gcc/testsuite/gcc.dg/tree-ssa/reassoc-34.c
index 9b45f1cd9be..a59df6a7244 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/reassoc-34.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/reassoc-34.c
@@ -1,4 +1,4 @@
-/* { dg-do run { target { ! "m68k*-*-* mmix*-*-* bfin*-*-* v850*-*-*
moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-*
hppa*-*-* nios2*-*-*"} } } */
+/* { dg-do run { target { ! "m68k*-*-* mmix*-*-* bfin*-*-* v850*-*-*
moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-*
hppa*-*-* nios2*-*-* pru*-*-*"} } } */
/* { dg-options "-O2 -fno-inline -fdump-tree-reassoc1-details" } */
/* { dg-additional-options "-mbranch-cost=2" { target branch_cost } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/reassoc-35.c
b/gcc/testsuite/gcc.dg/tree-ssa/reassoc-35.c
index 9ee3abca04e..21239372709 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/reassoc-35.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/reassoc-35.c
@@ -1,4 +1,4 @@
-/* { dg-do run { target { ! "m68k*-*-* mmix*-*-* bfin*-*-* v850*-*-*
moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-*
hppa*-*-* nios2*-*-*"} } } */
+/* { dg-do run { target { ! "m68k*-*-* mmix*-*-* bfin*-*-* v850*-*-*
moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-*
hppa*-*-* nios2*-*-* pru*-*-*"} } } */
/* { dg-options "-O2 -fno-inline -fdump-tree-reassoc1-details" } */
/* { dg-additional-options "-mbranch-cost=2" { target branch_cost } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/reassoc-36.c
b/gcc/testsuite/gcc.dg/tree-ssa/reassoc-36.c
index ac3a04291b7..f8eb0954ae7 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/reassoc-36.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/reassoc-36.c
@@ -1,4 +1,4 @@
-/* { dg-do run { target { ! "m68k*-*-* mmix*-*-* bfin*-*-* v850*-*-*
moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-*
hppa*-*-* nios2*-*-*"} } } */
+/* { dg-do run { target { ! "m68k*-*-* mmix*-*-* bfin*-*-* v850*-*-*
moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-*
hppa*-*-* nios2*-*-* pru*-*-*"} } } */
/* { dg-options "-O2 -fno-inline -fdump-tree-reassoc1-details" } */
/* { dg-additional-options "-mbranch-cost=2" { target branch_cost } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-14.c
b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-14.c
index
[PATCH 07/11] testsuite: Define PRU stack usage
gcc/testsuite/ChangeLog: 2018-06-13 Dimitar Dimitrov * gcc.dg/stack-usage-1.c: Define PRU stack usage. Signed-off-by: Dimitar Dimitrov --- gcc/testsuite/gcc.dg/stack-usage-1.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gcc/testsuite/gcc.dg/stack-usage-1.c b/gcc/testsuite/gcc.dg/stack-usage-1.c index 038bd4ec05c..ffa97da6c32 100644 --- a/gcc/testsuite/gcc.dg/stack-usage-1.c +++ b/gcc/testsuite/gcc.dg/stack-usage-1.c @@ -93,6 +93,8 @@ # define SIZE 254 #elif defined (__nios2__) # define SIZE 252 +#elif defined (__PRU__) +# define SIZE 252 #elif defined (__v850__) #define SIZE 260 #elif defined (__mn10300__) -- 2.11.0
[PATCH 05/11] testsuite: Add check for unsupported TI ABI PRU features to testsuite
Not all C language features are supported when -mabi=ti option is
used for PRU target.
gcc/testsuite/ChangeLog:
2018-06-13 Dimitar Dimitrov
* lib/gcc-dg.exp: Filter unsupported features in PRU's TI ABI mode.
* lib/target-utils.exp: Ditto.
Signed-off-by: Dimitar Dimitrov
---
gcc/testsuite/lib/gcc-dg.exp | 11 +++
gcc/testsuite/lib/target-utils.exp | 8
2 files changed, 19 insertions(+)
diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp
index c5ab850c840..f4976520282 100644
--- a/gcc/testsuite/lib/gcc-dg.exp
+++ b/gcc/testsuite/lib/gcc-dg.exp
@@ -388,6 +388,17 @@ proc gcc-dg-prune { system text } {
return "::unsupported::memory full"
}
+if {[string match "pru-*" $system] && \
+ [string match "*error: function pointers not supported with
-mabi=ti option*" $text]} {
+ # The format here is important. See dg.exp.
+ return "::unsupported::abi"
+}
+if {[string match "pru-*" $system] && \
+ [string match "*error: large return values not supported with
-mabi=ti option*" $text]} {
+ # The format here is important. See dg.exp.
+ return "::unsupported::abi"
+}
+
return $text
}
diff --git a/gcc/testsuite/lib/target-utils.exp
b/gcc/testsuite/lib/target-utils.exp
index 732a1827a02..e288447ef1d 100644
--- a/gcc/testsuite/lib/target-utils.exp
+++ b/gcc/testsuite/lib/target-utils.exp
@@ -44,5 +44,13 @@ proc ${tool}_check_unsupported_p { output } {
[string match "*exceeds local store*" $output] } {
return "memory full"
}
+if {[istarget pru-*] && \
+ [string match "*error: large return values not supported with
-mabi=ti option*" $output]} {
+ return "abi not supported"
+}
+if {[istarget pru-*] && \
+ [string match "*error: function pointers not supported with
-mabi=ti option*" $output]} {
+ return "abi not supported"
+}
return ""
}
--
2.11.0
Re: [PATCH 01/11] Initial TI PRU GCC port
On Wed, 13 Jun 2018, Dimitar Dimitrov wrote:
> + error ("__delay_cycles() only takes constant arguments");
As in documentation, diagnostics should not use () to indicate that a name
refers to a function (as opposed to referring to a call with no
arguments). However, function names, option names and any other literal
source code text in diagnostics, both this and other diagnostics in the
port, should be enclosed in %< and %> to quote them in the diagnostic
output.
> + error ("__delay_cycles only takes non-negative cycle counts.");
> + error ("__delay_cycles is limited to 32-bit loop counts.");
No '.' at end of diagnostics (and use %<__delay_cycles%>, as above).
--
Joseph S. Myers
jos...@codesourcery.com
[Committed] PR fortran/86110 -- Avoid NULL pointer dereference
Committed as obviously.
2018-06-13 Steven G. Kargl
PR fortran/86110
* array.c (gfc_resolve_character_array_constructor): Avoid NULL
pointer dereference.
2018-06-13 Steven G. Kargl
PR fortran/86110
* gfortran.dg/pr86110.f90: New test.
--
Steve
Index: gcc/fortran/array.c
===
--- gcc/fortran/array.c (revision 261521)
+++ gcc/fortran/array.c (working copy)
@@ -2040,7 +2040,9 @@ got_charlen:
gfc_ref *ref;
for (ref = p->expr->ref; ref; ref = ref->next)
if (ref->type == REF_SUBSTRING
+ && ref->u.ss.start
&& ref->u.ss.start->expr_type == EXPR_CONSTANT
+ && ref->u.ss.end
&& ref->u.ss.end->expr_type == EXPR_CONSTANT)
break;
Index: gcc/testsuite/gfortran.dg/pr86110.f90
===
--- gcc/testsuite/gfortran.dg/pr86110.f90 (nonexistent)
+++ gcc/testsuite/gfortran.dg/pr86110.f90 (working copy)
@@ -0,0 +1,7 @@
+! { dg-do compile }
+! PR fortran/86110
+program p
+ character(:), allocatable :: x, y
+ x = 'abc'
+ y = [x(:)] ! { dg-error "Incompatible ranks 0 and 1" }
+end
[PATCH 10/11] Fix LRA to handle multi-word eliminable registers
From: Dimitar Dimitrov
For some targets, Pmode != UNITS_PER_WORD. Take this into account
when marking hard registers as being used.
I tested C and C++ testsuits for x86_64 with and without this
patch. There was no regression, i.e. gcc.sum and g++.sum matched
exactly.
gcc/ChangeLog:
2018-06-13 Dimitar Dimitrov
* lra-eliminations.c (set_ptr_hard_reg_bits): New function.
(update_reg_eliminate): Mark all spanning hw registers.
gcc/testsuite/ChangeLog:
2018-06-13 Dimitar Dimitrov
* gcc.target/pru/lra-framepointer-fragmentation-1.c: New test.
* gcc.target/pru/lra-framepointer-fragmentation-2.c: New test.
Cc: Vladimir Makarov
Cc: Peter Bergner
Cc: Kenneth Zadeck
Cc: Seongbae Park
Signed-off-by: Dimitar Dimitrov
---
gcc/lra-eliminations.c | 14 -
.../pru/lra-framepointer-fragmentation-1.c | 33
.../pru/lra-framepointer-fragmentation-2.c | 61 ++
3 files changed, 106 insertions(+), 2 deletions(-)
create mode 100644
gcc/testsuite/gcc.target/pru/lra-framepointer-fragmentation-1.c
create mode 100644
gcc/testsuite/gcc.target/pru/lra-framepointer-fragmentation-2.c
diff --git a/gcc/lra-eliminations.c b/gcc/lra-eliminations.c
index 21d8d5f8018..566cc2c8248 100644
--- a/gcc/lra-eliminations.c
+++ b/gcc/lra-eliminations.c
@@ -1180,6 +1180,16 @@ spill_pseudos (HARD_REG_SET set)
bitmap_clear (_process);
}
+static void set_ptr_hard_reg_bits (HARD_REG_SET *hard_reg_set, int r)
+{
+ int w;
+
+ for (w = 0; w < GET_MODE_SIZE (Pmode); w += UNITS_PER_WORD, r++)
+{
+ SET_HARD_REG_BIT (*hard_reg_set, r);
+}
+}
+
/* Update all offsets and possibility for elimination on eliminable
registers. Spill pseudos assigned to registers which are
uneliminable, update LRA_NO_ALLOC_REGS and ELIMINABLE_REG_SET. Add
@@ -1264,13 +1274,13 @@ update_reg_eliminate (bitmap insns_with_changed_offsets)
CLEAR_HARD_REG_SET (temp_hard_reg_set);
for (ep = reg_eliminate; ep < _eliminate[NUM_ELIMINABLE_REGS]; ep++)
if (elimination_map[ep->from] == NULL)
- SET_HARD_REG_BIT (temp_hard_reg_set, ep->from);
+ set_ptr_hard_reg_bits (_hard_reg_set, ep->from);
else if (elimination_map[ep->from] == ep)
{
/* Prevent the hard register into which we eliminate from
the usage for pseudos. */
if (ep->from != ep->to)
- SET_HARD_REG_BIT (temp_hard_reg_set, ep->to);
+ set_ptr_hard_reg_bits (_hard_reg_set, ep->to);
if (maybe_ne (ep->previous_offset, ep->offset))
{
bitmap_ior_into (insns_with_changed_offsets,
diff --git a/gcc/testsuite/gcc.target/pru/lra-framepointer-fragmentation-1.c
b/gcc/testsuite/gcc.target/pru/lra-framepointer-fragmentation-1.c
new file mode 100644
index 000..ee1288fc2ae
--- /dev/null
+++ b/gcc/testsuite/gcc.target/pru/lra-framepointer-fragmentation-1.c
@@ -0,0 +1,33 @@
+/* { dg-do assemble } */
+/* { dg-options "-O1 -fno-omit-frame-pointer" } */
+#include
+
+extern uint64_t global;
+
+uint64_t __attribute__((noinline)) test(uint64_t a, uint64_t b,
+ uint64_t c, uint64_t d,
+ uint64_t e, uint64_t f,
+ uint64_t g, uint64_t h)
+{
+ uint64_t l1 = 0x12345678, l2 = 0x87654321, l3 = 1001, l4 = 1002;
+ uint64_t l5 = 1004;
+ uint32_t l6 = 2005;
+ uint8_t c1 = 101, c2 = 102;
+
+ /* The numerous dummy asm input operands create just
+ * enough register pressure to resort to using
+ * FP.b1 (r4.b1).
+ */
+
+ asm ("nop" /* { dg-error "'asm' operand has impossible constraints" } */
+ : "=r" (l1)
+ : "0" (l1), "r" (a), "r"(b),
+ "r"(c), "r"(d), "r"(e), "r"(f),
+ "r"(g), "r"(h), "r"(l2),
+ "r"(c1), "r"(c2),
+ "r"(l3), "r"(l4), "r"(l5), "r"(l6));
+
+ global = a+b+c+d+e+f+g+h + c1+c2 + l2;
+
+ return l1;
+}
diff --git a/gcc/testsuite/gcc.target/pru/lra-framepointer-fragmentation-2.c
b/gcc/testsuite/gcc.target/pru/lra-framepointer-fragmentation-2.c
new file mode 100644
index 000..6c98e9bf13b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/pru/lra-framepointer-fragmentation-2.c
@@ -0,0 +1,61 @@
+/* { dg-do run } */
+/* { dg-options "-O1 -fomit-frame-pointer" } */
+#include
+
+extern void abort (void);
+
+uint64_t global = 5;
+
+uint64_t __attribute__((noinline)) test(uint64_t a, uint64_t b,
+ uint64_t c, uint64_t d,
+ uint64_t e, uint64_t f,
+ uint64_t g, uint64_t h)
+{
+ uint64_t l1 = 0x12345678, l2 = 0x87654321, l3 = 1001, l4 = 1002;
+ uint64_t l5 = 1004;
+ uint32_t l6 = 2005;
+ uint8_t c1 = 101, c2 = 102;
+
+ /* The numerous dummy asm input operands create just
+ * enough register pressure to resort to using FP (r4).
+ */
+
+ asm ("ldi32 %0, 0x11223344\n\t"
+ "add %0, %0, %2\n\t"
+
Re: [RFC][PATCH] Stabilize a few qsort comparison functions
On 06/13/2018 05:50 AM, Franz Sirl wrote: >>> >>> >>> 2018-02-07 Franz Sirl >>> >>> * ira-build.c (object_range_compare_func): Stabilize sort. >>> * tree-sra.c (compare_access_positions): Likewise. >>> * varasm.c (output_object_block_compare): Likewise. >>> * tree-ssa-loop-ivopts.c (group_compare_offset): Likewise. >>> (struct iv_common_cand): New member. >>> (record_common_cand): Initialize new member. >>> (common_cand_cmp): Use new member to stabilize sort. >>> * tree-vrp.c (struct assert_locus): New member. >>> (register_new_assert_for): Initialize new member. >>> (compare_assert_loc): Use new member to stabilize sort. >> This looks pretty reasonable. I don't think you've contributed much >> recently, do you still have write access to the repository? > > Hi Jeff, > > after Alexander Monakov's gcc_qsort changes, this patch is not necessary > anymore. I've verified that with a backport (the 2 patches r260216 and > r260222 applied cleanly) of gcc_qsort to the gcc-8-branch. The resulting > powerpc-eabi crosscompilers produce no more unexpected differences > between a Linux and a Cygwin host. OK Good. > Tested (same like with my patch) by comparing the -fverbose-asm assembly > output on a complete rebuild of the software here. > So, unless someone thinks one of the changes makes sense anyway, this > patch is obsolete. Let's drop then. > > On the repository write access, yes, I don't have one anymore. But > before reactivating that I need to do the legal paperwork, because > unless before when GCC was a strictly private pet project for me, it now > is work related. I already got permission from my company for that, just > need to find some spare time to start the legal stuff. Understood. Good to have you back! jeff
[PATCH 00/11] New backend for the TI PRU processor
Hi, I would like to contribute a port for the TI PRU I/O processor [1], [2]. PRU is naturally 32-bit, but it is presented as 8-bit CPU for the GCC port. This is needed in order to take advantage of the efficient PRU ISA modes for addressing HW subregs [3]. The binutils port has been merged. I will post patches for newlib and simulator. The last two patches are the only ones modifying GCC core code. Logs from running the GCC testsuite using PRU simulator [4] are uploaded here: http://dinux.eu/gnupru/testresults/index.html Note that the listed C ABI failures are due to bugs in the vendor's proprietary toolchain I'm running the test against. GCC produces correct code in those cases. I could not implement full ABI compatibility [5] with the TI proprietary toolchain. One significant reason is that different data and code pointer sizes do not seem to be well accepted [6]. At the very least, it would require major core changes in GCC core. So I have implemented a compromise: * -mabi=gnu the default mode, with full C language support, but with a few deviations from TI ABI. * -mabi=ti mode, where any non-ABI compliant source expression causes a compile error. Regards, Dimitar [1] http://processors.wiki.ti.com/index.php/PRU-ICSS [2] http://beagleboard.org/pru [3] http://gcc.gnu.org/ml/gcc/2017-01/msg00217.html [4] https://github.com/dinuxbg/gnupru/tree/master/testing [5] https://github.com/dinuxbg/gnupru/wiki/ABI [6] http://gcc.gnu.org/ml/gcc/2012-04/msg00870.html Dimitar Dimitrov (11): Initial TI PRU GCC port Initial TI PRU libgcc port testsuite: Add PRU tests testsuite: Add check for overflowed IMEM region to testsuite testsuite: Add check for unsupported TI ABI PRU features to testsuite testsuite: Remove PRU from test cases requiring hosted environment testsuite: Define PRU stack usage testsuite: Mark that PRU has one-cycle jumps testsuite: Mark that PRU uses all function pointer bits Fix LRA to handle multi-word eliminable registers Increase MAX_MAX_OPERANDS limit configure.ac |7 + gcc/common/config/pru/pru-common.c | 36 + gcc/config.gcc |9 + gcc/config/pru/alu-zext.md | 178 ++ gcc/config/pru/constraints.md | 88 + gcc/config/pru/predicates.md | 220 ++ gcc/config/pru/pru-ldst-multiple.ml| 144 + gcc/config/pru/pru-opts.h | 31 + gcc/config/pru/pru-passes.c| 230 ++ gcc/config/pru/pru-pragma.c| 90 + gcc/config/pru/pru-protos.h| 70 + gcc/config/pru/pru.c | 2985 gcc/config/pru/pru.h | 551 gcc/config/pru/pru.md | 905 ++ gcc/config/pru/pru.opt | 53 + gcc/config/pru/t-pru | 31 + gcc/doc/extend.texi| 20 + gcc/doc/invoke.texi| 55 + gcc/doc/md.texi| 22 + gcc/genoutput.c|2 +- gcc/lra-eliminations.c | 14 +- gcc/testsuite/g++.old-deja/g++.abi/ptrmem.C|2 +- gcc/testsuite/gcc.c-torture/execute/20101011-1.c |3 + gcc/testsuite/gcc.dg/20020312-2.c |2 + gcc/testsuite/gcc.dg/stack-usage-1.c |2 + gcc/testsuite/gcc.dg/tree-ssa/20040204-1.c |2 +- gcc/testsuite/gcc.dg/tree-ssa/reassoc-33.c |2 +- gcc/testsuite/gcc.dg/tree-ssa/reassoc-34.c |2 +- gcc/testsuite/gcc.dg/tree-ssa/reassoc-35.c |2 +- gcc/testsuite/gcc.dg/tree-ssa/reassoc-36.c |2 +- gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-14.c |2 +- gcc/testsuite/gcc.target/pru/abi-arg-struct.c | 164 ++ gcc/testsuite/gcc.target/pru/ashiftrt.c| 13 + gcc/testsuite/gcc.target/pru/builtins-1.c | 12 + gcc/testsuite/gcc.target/pru/builtins-error.c |6 + gcc/testsuite/gcc.target/pru/clearbit.c| 13 + gcc/testsuite/gcc.target/pru/loop-asm.c| 19 + gcc/testsuite/gcc.target/pru/loop-dowhile.c| 45 + gcc/testsuite/gcc.target/pru/loop-hi-1.c | 38 + gcc/testsuite/gcc.target/pru/loop-hi-2.c | 17 + gcc/testsuite/gcc.target/pru/loop-qi-1.c | 38 + gcc/testsuite/gcc.target/pru/loop-qi-2.c | 17 + gcc/testsuite/gcc.target/pru/loop-short-1.c| 53 + gcc/testsuite/gcc.target/pru/loop-short-2.c| 21 + gcc/testsuite/gcc.target/pru/loop-si-1.c | 41 + gcc/testsuite/gcc.target/pru/loop-si-2.c | 20 + .../gcc.target/pru/loop-u8_pcrel_overflow.c| 42 + gcc/testsuite/gcc.target/pru/loop-ubyte-1.c
[PATCH 04/11] testsuite: Add check for overflowed IMEM region to testsuite
PRU architecture supports maximum 256k program memory (IMEM). Some GCC
test cases manage to produce executables bigger than that.
gcc/testsuite/ChangeLog:
2018-06-13 Dimitar Dimitrov
* lib/gcc-dg.exp: Bail on region overflow for tiny targets.
* lib/target-utils.exp: Ditto.
* lib/target-supports.exp: Declare PRU target as tiny.
Signed-off-by: Dimitar Dimitrov
---
gcc/testsuite/lib/gcc-dg.exp | 5 +
gcc/testsuite/lib/target-supports.exp | 5 +
gcc/testsuite/lib/target-utils.exp| 4
3 files changed, 14 insertions(+)
diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp
index a15c5d5e2a6..c5ab850c840 100644
--- a/gcc/testsuite/lib/gcc-dg.exp
+++ b/gcc/testsuite/lib/gcc-dg.exp
@@ -375,6 +375,11 @@ proc gcc-dg-prune { system text } {
return "::unsupported::memory full"
}
+if { [regexp "(^|\n)\[^\n\]*: region \[^\n\]* overflowed" $text]
+ && [check_effective_target_tiny] } {
+ return "::unsupported::memory full"
+}
+
# Likewise, if we see ".text exceeds local store range" or
# similar.
if {[string match "spu-*" $system] && \
diff --git a/gcc/testsuite/lib/target-supports.exp
b/gcc/testsuite/lib/target-supports.exp
index b92e54b6ec3..8d5f7e0720c 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -8786,6 +8786,11 @@ proc check_effective_target_tiny {} {
&& [check_effective_target_avr_tiny] } {
set et_target_tiny_saved 1
}
+ # PRU Program Counter is 16-bits, and trampolines are not supported.
+ # Hence directly declare as a tiny target.
+ if [istarget pru-*-*] {
+ set et_target_tiny_saved 1
+ }
}
return $et_target_tiny_saved
diff --git a/gcc/testsuite/lib/target-utils.exp
b/gcc/testsuite/lib/target-utils.exp
index bd39cc5bc79..732a1827a02 100644
--- a/gcc/testsuite/lib/target-utils.exp
+++ b/gcc/testsuite/lib/target-utils.exp
@@ -35,6 +35,10 @@ proc ${tool}_check_unsupported_p { output } {
&& [check_effective_target_tiny] } {
return "memory full"
}
+if { [regexp "(^|\n)\[^\n\]*: region \[^\n\]* overflowed" $output]
+ && [check_effective_target_tiny] } {
+ return "memory full"
+}
if { [istarget spu-*-*] && \
[string match "*exceeds local store*" $output] } {
--
2.11.0
[PATCH 11/11] Increase MAX_MAX_OPERANDS limit
The PRU load/store instructions can access memory with byte
granularity for all 30 of its 32-bit GP registers. Examples:
# Load 17 bytes from address r0[0] into registers r10.b1-r14.b2
lbbo r10.b1, r0, 0, 17
# Load 100 bytes from address r28[0] into registers r0-r25
lbbo r0.b0, r28, 0, 100
The load/store multiple patterns declare all subsequent registers
as distinct operands. Hence the need to increase the limit.
Increase the value to just 60 in order to avoid modifying regrename.c.
2018-06-13 Dimitar Dimitrov
* genoutput.c (MAX_MAX_OPERANDS): Increase to 60.
Signed-off-by: Dimitar Dimitrov
---
gcc/genoutput.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/genoutput.c b/gcc/genoutput.c
index 06456f4400c..d2eb179e813 100644
--- a/gcc/genoutput.c
+++ b/gcc/genoutput.c
@@ -96,7 +96,7 @@ along with GCC; see the file COPYING3. If not see
arbitrary limit, but what machine will have an instruction with
this many operands? */
-#define MAX_MAX_OPERANDS 40
+#define MAX_MAX_OPERANDS 60
static char general_mem[] = { TARGET_MEM_CONSTRAINT, 0 };
--
2.11.0
[PATCH 02/11] Initial TI PRU libgcc port
The floating point support has been borrowed from C6X libgcc port
to help with TI PRU toolchain ABI compatibility.
libgcc/ChangeLog:
2018-06-13 Dimitar Dimitrov
* config.host: Add PRU target.
* config/pru/asri.c: New file.
* config/pru/eqd.c: New file.
* config/pru/eqf.c: New file.
* config/pru/ged.c: New file.
* config/pru/gef.c: New file.
* config/pru/gtd.c: New file.
* config/pru/gtf.c: New file.
* config/pru/led.c: New file.
* config/pru/lef.c: New file.
* config/pru/lib2bitcountHI.c: New file.
* config/pru/lib2divHI.c: New file.
* config/pru/lib2divQI.c: New file.
* config/pru/lib2divSI.c: New file.
* config/pru/libgcc-eabi.ver: New file.
* config/pru/ltd.c: New file.
* config/pru/ltf.c: New file.
* config/pru/mpyll.S: New file.
* config/pru/pru-abi.h: New file.
* config/pru/pru-asm.h: New file.
* config/pru/pru-divmod.h: New file.
* config/pru/sfp-machine.h: New file.
* config/pru/t-pru: New file.
Signed-off-by: Dimitar Dimitrov
---
libgcc/config.host | 7 +++
libgcc/config/pru/asri.c | 33 ++
libgcc/config/pru/eqd.c| 45 +
libgcc/config/pru/eqf.c| 45 +
libgcc/config/pru/ged.c| 45 +
libgcc/config/pru/gef.c| 45 +
libgcc/config/pru/gtd.c| 45 +
libgcc/config/pru/gtf.c| 45 +
libgcc/config/pru/led.c| 45 +
libgcc/config/pru/lef.c| 45 +
libgcc/config/pru/lib2bitcountHI.c | 43 +
libgcc/config/pru/lib2divHI.c | 42 +
libgcc/config/pru/lib2divQI.c | 42 +
libgcc/config/pru/lib2divSI.c | 48 ++
libgcc/config/pru/libgcc-eabi.ver | 88 ++
libgcc/config/pru/ltd.c| 45 +
libgcc/config/pru/ltf.c| 45 +
libgcc/config/pru/mpyll.S | 57 +
libgcc/config/pru/pru-abi.h| 109
libgcc/config/pru/pru-asm.h| 35 +++
libgcc/config/pru/pru-divmod.h | 117 ++
libgcc/config/pru/sfp-machine.h| 125 +
libgcc/config/pru/t-pru| 50 +++
23 files changed, 1246 insertions(+)
create mode 100644 libgcc/config/pru/asri.c
create mode 100644 libgcc/config/pru/eqd.c
create mode 100644 libgcc/config/pru/eqf.c
create mode 100644 libgcc/config/pru/ged.c
create mode 100644 libgcc/config/pru/gef.c
create mode 100644 libgcc/config/pru/gtd.c
create mode 100644 libgcc/config/pru/gtf.c
create mode 100644 libgcc/config/pru/led.c
create mode 100644 libgcc/config/pru/lef.c
create mode 100644 libgcc/config/pru/lib2bitcountHI.c
create mode 100644 libgcc/config/pru/lib2divHI.c
create mode 100644 libgcc/config/pru/lib2divQI.c
create mode 100644 libgcc/config/pru/lib2divSI.c
create mode 100644 libgcc/config/pru/libgcc-eabi.ver
create mode 100644 libgcc/config/pru/ltd.c
create mode 100644 libgcc/config/pru/ltf.c
create mode 100644 libgcc/config/pru/mpyll.S
create mode 100644 libgcc/config/pru/pru-abi.h
create mode 100644 libgcc/config/pru/pru-asm.h
create mode 100644 libgcc/config/pru/pru-divmod.h
create mode 100644 libgcc/config/pru/sfp-machine.h
create mode 100644 libgcc/config/pru/t-pru
diff --git a/libgcc/config.host b/libgcc/config.host
index 18cabaf24f6..ff229c38f0c 100644
--- a/libgcc/config.host
+++ b/libgcc/config.host
@@ -165,6 +165,9 @@ nios2*-*-*)
powerpc*-*-*)
cpu_type=rs6000
;;
+pru-*-*)
+ cpu_type=pru
+ ;;
rs6000*-*-*)
;;
riscv*-*-*)
@@ -1145,6 +1148,10 @@ powerpcle-*-eabi*)
tmake_file="${tmake_file} rs6000/t-ppccomm rs6000/t-crtstuff
t-crtstuff-pic t-fdpbit"
extra_parts="$extra_parts crtbegin.o crtend.o crtbeginS.o crtendS.o
crtbeginT.o ecrti.o ecrtn.o ncrti.o ncrtn.o"
;;
+pru-*-*)
+ tmake_file="${tmake_file} t-softfp-sfdf t-softfp-excl t-softfp
t-gnu-prefix pru/t-pru"
+ tm_file="$tm_file pru/pru-abi.h"
+ ;;
riscv*-*-linux*)
tmake_file="${tmake_file} riscv/t-softfp${host_address} t-softfp
riscv/t-elf riscv/t-elf${host_address}"
extra_parts="$extra_parts crtbegin.o crtend.o crti.o crtn.o crtendS.o
crtbeginT.o"
diff --git a/libgcc/config/pru/asri.c b/libgcc/config/pru/asri.c
new file mode 100644
index 000..591d660f227
--- /dev/null
+++ b/libgcc/config/pru/asri.c
@@ -0,0 +1,33 @@
+/* PRU ABI compatibility functions
+ Arithmetic right shift
+ Copyright (C) 2017-2018 Free Software Foundation, Inc.
+ Contributed by Dimitar Dimitrov
+
+ This file is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+
[PATCH 03/11] testsuite: Add PRU tests
gcc/testsuite/ChangeLog: 2018-06-13 Dimitar Dimitrov * gcc.target/pru/abi-arg-struct.c: New test. * gcc.target/pru/ashiftrt.c: New test. * gcc.target/pru/builtins-1.c: New test. * gcc.target/pru/builtins-error.c: New test. * gcc.target/pru/clearbit.c: New test. * gcc.target/pru/loop-asm.c: New test. * gcc.target/pru/loop-dowhile.c: New test. * gcc.target/pru/loop-hi-1.c: New test. * gcc.target/pru/loop-hi-2.c: New test. * gcc.target/pru/loop-qi-1.c: New test. * gcc.target/pru/loop-qi-2.c: New test. * gcc.target/pru/loop-short-1.c: New test. * gcc.target/pru/loop-short-2.c: New test. * gcc.target/pru/loop-si-1.c: New test. * gcc.target/pru/loop-si-2.c: New test. * gcc.target/pru/loop-u8_pcrel_overflow.c: New test. * gcc.target/pru/loop-ubyte-1.c: New test. * gcc.target/pru/loop-ubyte-2.c: New test. * gcc.target/pru/mabi-ti-1.c: New test. * gcc.target/pru/mabi-ti-2.c: New test. * gcc.target/pru/mabi-ti-3.c: New test. * gcc.target/pru/mabi-ti-4.c: New test. * gcc.target/pru/mabi-ti-5.c: New test. * gcc.target/pru/mabi-ti-6.c: New test. * gcc.target/pru/mabi-ti-7.c: New test. * gcc.target/pru/pr64366.c: New test. * gcc.target/pru/pragma-ctable_entry.c: New test. * gcc.target/pru/pru.exp: New file. * gcc.target/pru/qbbc-1.c: New test. * gcc.target/pru/qbbc-2.c: New test. * gcc.target/pru/qbbc-3.c: New test. * gcc.target/pru/qbbs-1.c: New test. * gcc.target/pru/qbbs-2.c: New test. * gcc.target/pru/setbit.c: New test. * gcc.target/pru/zero_extend-and-hisi.c: New test. * gcc.target/pru/zero_extend-and-qihi.c: New test. * gcc.target/pru/zero_extend-and-qisi.c: New test. * gcc.target/pru/zero_extend-hisi.c: New test. * gcc.target/pru/zero_extend-qihi.c: New test. * gcc.target/pru/zero_extend-qisi.c: New test. * lib/target-supports.exp: Add PRU to feature filters. Signed-off-by: Dimitar Dimitrov --- gcc/testsuite/gcc.target/pru/abi-arg-struct.c | 164 + gcc/testsuite/gcc.target/pru/ashiftrt.c| 13 ++ gcc/testsuite/gcc.target/pru/builtins-1.c | 12 ++ gcc/testsuite/gcc.target/pru/builtins-error.c | 6 + gcc/testsuite/gcc.target/pru/clearbit.c| 13 ++ gcc/testsuite/gcc.target/pru/loop-asm.c| 19 +++ gcc/testsuite/gcc.target/pru/loop-dowhile.c| 45 ++ gcc/testsuite/gcc.target/pru/loop-hi-1.c | 38 + gcc/testsuite/gcc.target/pru/loop-hi-2.c | 17 +++ gcc/testsuite/gcc.target/pru/loop-qi-1.c | 38 + gcc/testsuite/gcc.target/pru/loop-qi-2.c | 17 +++ gcc/testsuite/gcc.target/pru/loop-short-1.c| 53 +++ gcc/testsuite/gcc.target/pru/loop-short-2.c| 21 +++ gcc/testsuite/gcc.target/pru/loop-si-1.c | 41 ++ gcc/testsuite/gcc.target/pru/loop-si-2.c | 20 +++ .../gcc.target/pru/loop-u8_pcrel_overflow.c| 42 ++ gcc/testsuite/gcc.target/pru/loop-ubyte-1.c| 30 gcc/testsuite/gcc.target/pru/loop-ubyte-2.c| 18 +++ gcc/testsuite/gcc.target/pru/mabi-ti-1.c | 10 ++ gcc/testsuite/gcc.target/pru/mabi-ti-2.c | 15 ++ gcc/testsuite/gcc.target/pru/mabi-ti-3.c | 12 ++ gcc/testsuite/gcc.target/pru/mabi-ti-4.c | 14 ++ gcc/testsuite/gcc.target/pru/mabi-ti-5.c | 33 + gcc/testsuite/gcc.target/pru/mabi-ti-6.c | 12 ++ gcc/testsuite/gcc.target/pru/mabi-ti-7.c | 21 +++ gcc/testsuite/gcc.target/pru/pr64366.c | 128 gcc/testsuite/gcc.target/pru/pragma-ctable_entry.c | 22 +++ gcc/testsuite/gcc.target/pru/pru.exp | 41 ++ gcc/testsuite/gcc.target/pru/qbbc-1.c | 29 gcc/testsuite/gcc.target/pru/qbbc-2.c | 15 ++ gcc/testsuite/gcc.target/pru/qbbc-3.c | 15 ++ gcc/testsuite/gcc.target/pru/qbbs-1.c | 29 gcc/testsuite/gcc.target/pru/qbbs-2.c | 15 ++ gcc/testsuite/gcc.target/pru/setbit.c | 13 ++ .../gcc.target/pru/zero_extend-and-hisi.c | 16 ++ .../gcc.target/pru/zero_extend-and-qihi.c | 16 ++ .../gcc.target/pru/zero_extend-and-qisi.c | 16 ++ gcc/testsuite/gcc.target/pru/zero_extend-hisi.c| 43 ++ gcc/testsuite/gcc.target/pru/zero_extend-qihi.c| 43 ++ gcc/testsuite/gcc.target/pru/zero_extend-qisi.c| 43 ++ gcc/testsuite/lib/target-supports.exp | 7 +- 41 files changed, 1213 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gcc.target/pru/abi-arg-struct.c create mode 100644 gcc/testsuite/gcc.target/pru/ashiftrt.c create mode 100644 gcc/testsuite/gcc.target/pru/builtins-1.c create mode
[PATCH 09/11] testsuite: Mark that PRU uses all function pointer bits
gcc/testsuite/ChangeLog: 2018-06-13 Dimitar Dimitrov * g++.old-deja/g++.abi/ptrmem.C: Add PRU to list. Signed-off-by: Dimitar Dimitrov --- gcc/testsuite/g++.old-deja/g++.abi/ptrmem.C | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/testsuite/g++.old-deja/g++.abi/ptrmem.C b/gcc/testsuite/g++.old-deja/g++.abi/ptrmem.C index 341735879c5..bda7960d8a2 100644 --- a/gcc/testsuite/g++.old-deja/g++.abi/ptrmem.C +++ b/gcc/testsuite/g++.old-deja/g++.abi/ptrmem.C @@ -7,7 +7,7 @@ function. However, some platforms use all bits to encode a function pointer. Such platforms use the lowest bit of the delta, that is shifted left by one bit. */ -#if defined __MN10300__ || defined __SH5__ || defined __arm__ || defined __thumb__ || defined __mips__ || defined __aarch64__ +#if defined __MN10300__ || defined __SH5__ || defined __arm__ || defined __thumb__ || defined __mips__ || defined __aarch64__ || defined __PRU__ #define ADJUST_PTRFN(func, virt) ((void (*)())(func)) #define ADJUST_DELTA(delta, virt) (((delta) << 1) + !!(virt)) #else -- 2.11.0
[PATCH 06/11] testsuite: Remove PRU from test cases requiring hosted environment
gcc/testsuite/ChangeLog: 2018-06-13 Dimitar Dimitrov * gcc.c-torture/execute/20101011-1.c: Define DO_TEST to 0 for PRU. * gcc.dg/20020312-2.c: No PIC register for PRU. Signed-off-by: Dimitar Dimitrov --- gcc/testsuite/gcc.c-torture/execute/20101011-1.c | 3 +++ gcc/testsuite/gcc.dg/20020312-2.c| 2 ++ 2 files changed, 5 insertions(+) diff --git a/gcc/testsuite/gcc.c-torture/execute/20101011-1.c b/gcc/testsuite/gcc.c-torture/execute/20101011-1.c index dda49a59852..7f8d14d617c 100644 --- a/gcc/testsuite/gcc.c-torture/execute/20101011-1.c +++ b/gcc/testsuite/gcc.c-torture/execute/20101011-1.c @@ -93,6 +93,9 @@ __aeabi_idiv0 (int return_value) #elif defined (__nvptx__) /* There isn't even a signal function. */ # define DO_TEST 0 +#elif defined (__pru__) +/* There isn't even a signal function. */ +# define DO_TEST 0 #else # define DO_TEST 1 #endif diff --git a/gcc/testsuite/gcc.dg/20020312-2.c b/gcc/testsuite/gcc.dg/20020312-2.c index f5929e0b057..209ef67e263 100644 --- a/gcc/testsuite/gcc.dg/20020312-2.c +++ b/gcc/testsuite/gcc.dg/20020312-2.c @@ -111,6 +111,8 @@ extern void abort (void); /* No pic register. */ #elif defined (__nvptx__) /* No pic register. */ +#elif defined(__PRU__) +/* No pic register. */ #else # error "Modify the test for your target." #endif -- 2.11.0
[gomp5] Support OpenMP 5.0 iterators in depend clauses
Hi!
The following patch adds support for depend clause iterators.
Tested on x86_64-linux, committed to gomp-5_0-branch.
2018-06-13 Jakub Jelinek
* gimplify.c (gimplify_omp_depend): New function.
(gimplify_scan_omp_clauses): Gimplify depend clauses with iterators.
* omp-low.c (lower_depend_clauses): If OMP_CLAUSE_DEPEND_LAST is seen,
assume lowering is done already and return early. Set kind
on artificial depend clause to OMP_CLAUSE_DEPEND_LAST.
* tree-pretty-print.c (dump_omp_iterators): New function.
(dump_omp_clause): Print iterators in depend clauses. Print
__internal__ for OMP_CLAUSE_DEPEND_LAST.
gcc/c/
* c-parser.c (c_parser_omp_iterators): New function.
(c_parser_omp_clause_depend): Parse iterator modifier and handle
iterators.
* c-typeck.c (handle_omp_array_sections): Handle depend clauses
with iterators.
(struct c_find_omp_var_s): New type.
(c_find_omp_var_r): New function.
(c_omp_finish_iterators): New function.
(c_finish_omp_clauses): Handle depend clauses with iterators.
gcc/cp/
* parser.c (cp_parser_omp_iterators): New function.
(cp_parser_omp_clause_depend): Parse iterator modifier and handle
iterators.
* semantics.c (handle_omp_array_sections): Handle depend clauses
with iterators.
(cp_omp_finish_iterators): New function.
(finish_omp_clauses): Handle depend clauses with iterators.
* pt.c (tsubst_omp_clause_decl): Add iterators_cache argument.
Adjust recursive calls. Handle iterators.
(tsubst_omp_clauses): Adjust tsubst_omp_clause_decl callers.
gcc/testsuite/
* c-c++-common/gomp/depend-iterator-1.c: New test.
* c-c++-common/gomp/depend-iterator-2.c: New test.
* g++.dg/gomp/depend-iterator-1.C: New test.
* g++.dg/gomp/depend-iterator-2.C: New test.
libgomp/
* testsuite/libgomp.c-c++-common/depend-iterator-1.c: New test.
* testsuite/libgomp.c++/depend-iterator-1.C: New test.
--- gcc/gimplify.c.jj 2018-06-05 15:00:56.217957674 +0200
+++ gcc/gimplify.c 2018-06-12 11:45:41.817981609 +0200
@@ -7556,6 +7556,368 @@ find_decl_expr (tree *tp, int *walk_subt
return NULL_TREE;
}
+/* If *LIST_P contains any OpenMP depend clauses with iterators,
+ lower all the depend clauses by populating corresponding depend
+ array. Returns 0 if there are no such depend clauses, or
+ 2 if all depend clauses should be removed, 1 otherwise. */
+
+static int
+gimplify_omp_depend (tree *list_p, gimple_seq *pre_p)
+{
+ tree c;
+ gimple *g;
+ size_t n[2] = { 0, 0 };
+ tree counts[2] = { NULL_TREE, NULL_TREE };
+ tree last_iter = NULL_TREE, last_count = NULL_TREE;
+ size_t i;
+ location_t first_loc = UNKNOWN_LOCATION;
+
+ for (c = *list_p; c; c = OMP_CLAUSE_CHAIN (c))
+if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_DEPEND)
+ {
+ switch (OMP_CLAUSE_DEPEND_KIND (c))
+ {
+ case OMP_CLAUSE_DEPEND_IN:
+ i = 0;
+ break;
+ case OMP_CLAUSE_DEPEND_OUT:
+ case OMP_CLAUSE_DEPEND_INOUT:
+ case OMP_CLAUSE_DEPEND_MUTEXINOUTSET:
+ i = 1;
+ break;
+ case OMP_CLAUSE_DEPEND_SOURCE:
+ case OMP_CLAUSE_DEPEND_SINK:
+ continue;
+ default:
+ gcc_unreachable ();
+ }
+ tree t = OMP_CLAUSE_DECL (c);
+ if (first_loc == UNKNOWN_LOCATION)
+ first_loc = OMP_CLAUSE_LOCATION (c);
+ if (TREE_CODE (t) == TREE_LIST
+ && TREE_PURPOSE (t)
+ && TREE_CODE (TREE_PURPOSE (t)) == TREE_VEC)
+ {
+ if (TREE_PURPOSE (t) != last_iter)
+ {
+ tree tcnt = size_one_node;
+ for (tree it = TREE_PURPOSE (t); it; it = TREE_CHAIN (it))
+ {
+ if (gimplify_expr (_VEC_ELT (it, 1), pre_p, NULL,
+ is_gimple_val, fb_rvalue) == GS_ERROR
+ || gimplify_expr (_VEC_ELT (it, 2), pre_p, NULL,
+ is_gimple_val, fb_rvalue) == GS_ERROR
+ || (gimplify_expr (_VEC_ELT (it, 3), pre_p, NULL,
+ is_gimple_val, fb_rvalue)
+ == GS_ERROR))
+ return 2;
+ tree var = TREE_VEC_ELT (it, 0);
+ tree begin = TREE_VEC_ELT (it, 1);
+ tree end = TREE_VEC_ELT (it, 2);
+ tree step = TREE_VEC_ELT (it, 3);
+ tree type = TREE_TYPE (var);
+ tree stype = TREE_TYPE (step);
+ location_t loc = DECL_SOURCE_LOCATION (var);
+ tree endmbegin;
+ /* Compute count for this iterator as
+ step > 0
+ ? (begin < end ? (end - begin + (step - 1)) / step : 0)
+
Re: [PATCH] relax -Wsizeof-pointer-memaccess for strncpy with size of source (PR 85931)
On 06/13/2018 09:16 AM, Jakub Jelinek wrote:
On Wed, Jun 13, 2018 at 08:53:20AM -0600, Martin Sebor wrote:
I'm actually worried about the fold-const.c change and don't understand, why
it has been done at all in the context of this PR.
case SAVE_EXPR:
if (flags & OEP_LEXICOGRAPHIC)
return OP_SAME (0);
return 0;
means it does something different from the state before the patch:
default:
return 0;
only if OEP_LEXICOGRAPHIC is set, which AFAIK is only in the
-Wduplicated-branches warning code, so why it is needed for
-Wstringop-truncation warning is unclear to me.
It lets sizeof_pointer_memaccess_warning detect that sizeof b
refers to the size of the VLA b in cases like the one below
(the sizeof b expression is represented as SAVE_EXPR (n)) and
avoid false positives:
I've missed you've added another operand_equal_p call with
OEP_LEXICOGRAPHIC, but you are using it for something it hasn't been
designed for.
Say if the array sizes are not n, but foo (n) for some
unsigned foo (unsigned). That function can return completely different
values between the two calls, while operand_equal_p with OEP_LEXICOGRAPHIC
will still say they are the same. That is fine for -Wduplicated-branches
it has been designed for.
Or consider
char a[n];
n += 3;
char b[n];
f (a, b);
strncpy (a, b, sizeof b);
f (a);
The lengths are even known not to be equal here, but OEP_LEXICOGRAPHIC
happily says they are the same. On the other hand, you could have:
unsigned n2 = n;
char a[n], b[n2];
and OEP_LEXICOGRAPHIC would say they are not equal, even when they are equal
at runtime.
True. -Wsizeof-pointer-memaccess is not perfect (irrespective
of this change). It warns for "suspicious length parameters
to certain string and memory built-in functions if the argument
uses sizeof." It's unavoidably prone to missing some problems
or flagging non-issues (such as bug 57627).
void f (void*, ...);
void g (unsigned n)
{
char a[n], b[n];
f (a, b);
strncpy (a, b, sizeof b); // bogus -Wsizeof-pointer-memaccess
f (a);
}
Short of moving the SAVE_EXPR logic out of operand_equal_p()
and into sizeof_pointer_memaccess_warning, is there a better
way to determine that? (I would expect the SAVE_EXPR logic
in operand_equal_p() to be useful in other contexts besides
this warning.)
Only -Wduplicated-branches (and now your code too) are using
OEP_LEXICOGRAPHIC, nothing else, so it doesn't make a difference for other
uses. There we only care about SAVE_EXPR pointer equality, if they aren't
pointer equal, they might, but might not evaluate the same.
If you want to be conservative with the warning, then just don't warn if
sizeof doesn't return a constant.
I don't think it's necessary to weaken the warning further.
Since nothing else uses OEP_LEXICOGRAPHIC, do you have any
other concerns about backporting the change to GCC 8?
Martin
libgo patch committed: Dump preinit blocks in AST dumps
This patch to the Go frontend by Than McIntosh includes global
variable preinit blocks in AST dumps. Each preinit block is prefixed
with a comment indicating the variable it is initializing.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed
to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 261549)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-6743db0ed81e313acf66c00a4ed0e2dcaaca2c9f
+1f07926263b6d14edb6abd6a00e6385190d30d0e
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/ast-dump.cc
===
--- gcc/go/gofrontend/ast-dump.cc (revision 261521)
+++ gcc/go/gofrontend/ast-dump.cc (working copy)
@@ -29,7 +29,7 @@ class Ast_dump_traverse_blocks_and_funct
{
public:
Ast_dump_traverse_blocks_and_functions(Ast_dump_context* ast_dump_context)
- : Traverse(traverse_blocks | traverse_functions),
+ : Traverse(traverse_blocks | traverse_functions | traverse_variables),
ast_dump_context_(ast_dump_context)
{ }
@@ -40,6 +40,9 @@ class Ast_dump_traverse_blocks_and_funct
int
function(Named_object*);
+ int
+ variable(Named_object*);
+
private:
Ast_dump_context* ast_dump_context_;
};
@@ -150,6 +153,27 @@ Ast_dump_traverse_blocks_and_functions::
return TRAVERSE_CONTINUE;
}
+// Dump variable preinits
+
+int
+Ast_dump_traverse_blocks_and_functions::variable(Named_object* no)
+{
+ if (!no->is_variable())
+return TRAVERSE_CONTINUE;
+
+ Variable* var = no->var_value();
+ if (var->has_pre_init())
+{
+ this->ast_dump_context_->ostream() << "// preinit block for var "
+ << no->message_name() << "\n";
+ var->preinit()->traverse(this);
+}
+
+ return TRAVERSE_CONTINUE;
+}
+
+
+
// Class Ast_dump_context.
Ast_dump_context::Ast_dump_context(std::ostream* out /* = NULL */,
[PATCH] Decorate string_view members with nonnull attribute
The C++ committee has confirmed that passing a null pointer to the
unary basic_string_view constructor is undefined. This removes the check
from our implementation, and adds the nonnull attribute to warn when the
compiler can detect undefined input.
Any objections to this change?
commit 05d33d556eaabb3c81f4db15b9818c465b359c15
Author: Jonathan Wakely
Date: Wed Jun 13 16:54:47 2018 +0100
Decorate string_view members with nonnull attribute
The C++ committee has confirmed that passing a null pointer to the
unary basic_string_view constructor is undefined. This removes the check
from our implementation, and adds the nonnull attribute to warn when the
compiler can detect undefined input.
* include/std/string_view (basic_string_view(const CharT*)): Remove
check for null pointer and add nonnull attribute.
(compare(const CharT*), compare(size_type, size_type, const CharT*))
(find(const CharT*, size_type), rfind(const CharT*, size_type))
(find_first_of(const CharT*, size_type))
(find_last_of(const CharT*, size_type))
(find_first_not_of(const CharT*, size_type))
(find_last_not_of(const CharT*, size_type)): Add nonnull attribute.
* testsuite/21_strings/basic_string_view/cons/char/nonnull.cc: New.
* testsuite/21_strings/basic_string_view/operations/compare/char/
nonnull.cc: New.
* testsuite/21_strings/basic_string_view/operations/find/char/
nonnull.cc: New.
* testsuite/21_strings/basic_string_view/operations/rfind/char/
nonnull.cc: New.
diff --git a/libstdc++-v3/include/std/string_view
b/libstdc++-v3/include/std/string_view
index 9f39df853e8..4de484a96ba 100644
--- a/libstdc++-v3/include/std/string_view
+++ b/libstdc++-v3/include/std/string_view
@@ -96,8 +96,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
constexpr basic_string_view(const basic_string_view&) noexcept = default;
- constexpr basic_string_view(const _CharT* __str) noexcept
- : _M_len{__str == nullptr ? 0 : traits_type::length(__str)},
+ [[gnu::nonnull]] constexpr
+ basic_string_view(const _CharT* __str) noexcept
+ : _M_len{traits_type::length(__str)},
_M_str{__str}
{ }
@@ -270,11 +271,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return this->substr(__pos1, __n1).compare(__str.substr(__pos2, __n2));
}
- constexpr int
+ [[gnu::nonnull]] constexpr int
compare(const _CharT* __str) const noexcept
{ return this->compare(basic_string_view{__str}); }
- constexpr int
+ [[gnu::nonnull]] constexpr int
compare(size_type __pos1, size_type __n1, const _CharT* __str) const
{ return this->substr(__pos1, __n1).compare(basic_string_view{__str}); }
@@ -296,7 +297,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
constexpr size_type
find(const _CharT* __str, size_type __pos, size_type __n) const noexcept;
- constexpr size_type
+ [[gnu::nonnull]] constexpr size_type
find(const _CharT* __str, size_type __pos = 0) const noexcept
{ return this->find(__str, __pos, traits_type::length(__str)); }
@@ -310,7 +311,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
constexpr size_type
rfind(const _CharT* __str, size_type __pos, size_type __n) const
noexcept;
- constexpr size_type
+ [[gnu::nonnull]] constexpr size_type
rfind(const _CharT* __str, size_type __pos = npos) const noexcept
{ return this->rfind(__str, __pos, traits_type::length(__str)); }
@@ -323,9 +324,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{ return this->find(__c, __pos); }
constexpr size_type
- find_first_of(const _CharT* __str, size_type __pos, size_type __n) const
noexcept;
+ find_first_of(const _CharT* __str, size_type __pos,
+ size_type __n) const noexcept;
- constexpr size_type
+ [[gnu::nonnull]] constexpr size_type
find_first_of(const _CharT* __str, size_type __pos = 0) const noexcept
{ return this->find_first_of(__str, __pos, traits_type::length(__str)); }
@@ -342,7 +344,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
find_last_of(const _CharT* __str, size_type __pos,
size_type __n) const noexcept;
- constexpr size_type
+ [[gnu::nonnull]] constexpr size_type
find_last_of(const _CharT* __str, size_type __pos = npos) const noexcept
{ return this->find_last_of(__str, __pos, traits_type::length(__str)); }
@@ -358,7 +360,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
find_first_not_of(const _CharT* __str,
size_type __pos, size_type __n) const noexcept;
- constexpr size_type
+ [[gnu::nonnull]] constexpr size_type
find_first_not_of(const _CharT* __str, size_type __pos = 0) const
noexcept
{
return this->find_first_not_of(__str, __pos,
@@ -377,7 +379,7 @@
RE: [PATCH] MIPS: Add support for P6600
Robert Suchanek writes:
> As already discussed, the link to the P6600 doesn't
> appear to be referenced on mips.com but reachable
> when searching for 'p6600':
>
> https://www.mips.com/downloads/p6600-multiprocessing-programmers-guide/
Thanks, good spot.
> gcc/ChangeLog:
>
> 2018-06-12 Matthew Fortune
> Prachi Godbole
>
> * config/mips/mips-cpus.def: Define P6600.
> * config/mips/mips-tables.opt: Regenerate.
> * config/mips/mips.c (mips_ucbranch_type): New enum.
> (mips_rtx_cost_data): Add support for P6600.
> (mips_issue_rate): Likewise.
> (mips_multipass_dfa_lookahead): Likewise.
> (mips_avoid_hazard): Likewise.
> (mips_reorg_process_insns): Likewise.
> (mips_classify_branch_p6600): New function.
> * config/mips/mips.h (TUNE_P6600): New define.
> (MIPS_ISA_LEVEL_SPEC): Infer mips64r6 from p6600.
> (ENABLE_LD_ST_PAIRS): Enable load/store bonding for p6600.
> * config/mips/mips.md: Include p6600.md.
> (processor): Add p6600.
> * config/mips/p6600.md: New file.
> * doc/invoke.texi: Add p6600 to supported architectures.
With one more change to add another comment as below, this is OK to
commit.
> @@ -18957,7 +19039,10 @@ mips_reorg_process_insns (void)
>sequence and replace it with the delay slot instruction
>then the jump to clear the forbidden slot hazard. */
This bit does need the comment extending. Add this:
For the P6600, this optimisation solves the performance penalty
associated with BALC followed by a delay slot branch. We do not set
fs_delay as we do not want the full logic of a forbidden slot; the
penalty exists only against branches not the full class of forbidden
slot instructions.
>
> - if (fs_delay)
> + if (fs_delay || (TUNE_P6600
> +&& TARGET_CB_MAYBE
> +&& mips_classify_branch_p6600 (insn)
> + == UC_BALC))
> {
> /* Search onwards from the current position looking
> for
>a SEQUENCE. We are looking for pipeline hazards
here
Thanks,
Matthew
Re: [RFC] New features for multilib control
On 6/13/18, Matthew Fortune wrote: > Hi, > > This patch was developed as part of preparing ever more complex multilib > combinations for the MIPS architecture and aims to solve several problems > in this area. I've attempted to be quite verbose in the description, so > that I can explain how I am using various terms as pretty much everyone > has a different understanding (and I don't claim mine to be 'right' > either). > > The changes aim to: > > 1) Eliminate the fallback multilib > The fallback multilib (top level of 'lib') is often annoying because it is > used for any combination of options that do not match a specific multilib. > Quite often this default multilib is incompatible with the build options > that end up linking against it, leading to bizarre link time messages that > confuse ordinary users. > > 2) Move the default multilib to a subfolder > Having successfully eliminated the fallback multilib it is also true that > it would eliminate the 'default' multilib as well. I.e. the library used > when no relevant user options are supplied. Moving this library to a > subfolder has two benefits. a) it still exists! and b) the location of > this library becomes invariant irrespective of which options are > build-time configured as default. > > 3) Preserve/use invariant paths for multilib variants > A simplistic multilib specification leads to a nested set of folders, > where the top level is the left-most multilib directory and the bottom is > the right most. Introducing a new axis of multilib configuration changes > this path structure leading to the different library configurations to > move around. This is not in itself a problem, as the compiler driver can > always locate the right path for any given build, but it does cause issues > when doing configuration management of binary toolchains. When there are > many different multilibs it is ideal to ship/install only the ones that > are important and if the paths keep changing over time this process is > complex and confusing. This issue is effectively solved by the > MULTI_OSDIRNAMES feature but using it for both sysroot and compiler > internal libraries is even better. > > 4) Support un-released multilib configurations > This one sounds weird but it is quite valuable. When an architecture has > 70+ possible multilib variants but only 40 of them are currently known to > be needed then only build and release 40 variants. However, if it turns > out that another configuration becomes useful then it is often handy to > just build the missing configuration and install it into the pre-existing > release. So, the driver needs to know about all multilibs and their paths > but the build process should only build a subset. > > So, the solution... > > Firstly, be verbose about the MULTILIB_OPTIONS needed for a target. Add > in the default options as well as all the others that may, or could, ever > be supported by the current compiler features. For MIPS supporting > MIPS32r1 onwards it looks like this: > > MULTILIB_OPTIONS = muclibc > mips32/mips32r2/mips32r6/mips64/mips64r2/mips64r6 > mips16/mmicromips mabi=32/mabi=n32/mabi=64 EB/EL msoft-float mnan=2008 > > This does create an enormous matrix of possible configurations so this > has to be trimmed to the valid set using MULTILIB_REQUIRED. Note that > the valid set should include any that you may wish to support even if > you don't want to build/release them. By having the default options in > then this leads to having two copies of the 'default' multilib; one with > the options implicitly set and one with them explicitly set. > > Second, remove the multilib with the implicit default options. This > equates to the '.' multilib entry. To do this use MULTILIB_EXCLUSIONS to > remove the negated form of every MULTILIB_OPTION: > > MULTILIB_EXCLUSIONS = > !muclibc/!mips32/!mips32r2/!mips32r6/!mips64/!mips64r2/!mips64r6/!mips16/!mm > icromips/!mabi=32/!mabi=n32/!mabi=64/!EB/!EL/!msoft-float/!mnan=2008 > > Third, set the MULTILIB_OSDIRNAMES to have an entry for every valid > combination of options and use the '!' modifier. Honestly, I'm not sure > how/why this works but this leads to both the internal library paths and > sysroot paths using the OSDIRNAME instead of the nested tree of > MULTILIB_DIRNAMES. Choose a path for each variant that you will never > change again; I used an encoded form of the configuration for MIPS: > > MULTILIB_OSDIRNAMES = > mips32r6/mabi.32/EB/mnan.2008=!mips-r6-hard$(is_newlib)/lib > > Fourth, deal with the fallout from the config-ml.in logic which handles > the initial 'configure' of a library differently to all of the multilib > configurations. The basic idea is that since the default multilib will now > report that it belongs in a subdirectory then, when configuring the top > level multilib, query the driver for the folder. Later when iterating > through the multilib specs skip the one that matches the default > configuration. Each configuration will be built exactly once and all of >
Re: [PATCH] relax -Wsizeof-pointer-memaccess for strncpy with size of source (PR 85931)
On Wed, Jun 13, 2018 at 08:53:20AM -0600, Martin Sebor wrote:
> > I'm actually worried about the fold-const.c change and don't understand, why
> > it has been done at all in the context of this PR.
> >
> > case SAVE_EXPR:
> > if (flags & OEP_LEXICOGRAPHIC)
> > return OP_SAME (0);
> > return 0;
> >
> > means it does something different from the state before the patch:
> > default:
> > return 0;
> > only if OEP_LEXICOGRAPHIC is set, which AFAIK is only in the
> > -Wduplicated-branches warning code, so why it is needed for
> > -Wstringop-truncation warning is unclear to me.
>
> It lets sizeof_pointer_memaccess_warning detect that sizeof b
> refers to the size of the VLA b in cases like the one below
> (the sizeof b expression is represented as SAVE_EXPR (n)) and
> avoid false positives:
I've missed you've added another operand_equal_p call with
OEP_LEXICOGRAPHIC, but you are using it for something it hasn't been
designed for.
Say if the array sizes are not n, but foo (n) for some
unsigned foo (unsigned). That function can return completely different
values between the two calls, while operand_equal_p with OEP_LEXICOGRAPHIC
will still say they are the same. That is fine for -Wduplicated-branches
it has been designed for.
Or consider
char a[n];
n += 3;
char b[n];
f (a, b);
strncpy (a, b, sizeof b);
f (a);
The lengths are even known not to be equal here, but OEP_LEXICOGRAPHIC
happily says they are the same. On the other hand, you could have:
unsigned n2 = n;
char a[n], b[n2];
and OEP_LEXICOGRAPHIC would say they are not equal, even when they are equal
at runtime.
> void f (void*, ...);
>
> void g (unsigned n)
> {
> char a[n], b[n];
> f (a, b);
> strncpy (a, b, sizeof b); // bogus -Wsizeof-pointer-memaccess
> f (a);
> }
>
> Short of moving the SAVE_EXPR logic out of operand_equal_p()
> and into sizeof_pointer_memaccess_warning, is there a better
> way to determine that? (I would expect the SAVE_EXPR logic
> in operand_equal_p() to be useful in other contexts besides
> this warning.)
Only -Wduplicated-branches (and now your code too) are using
OEP_LEXICOGRAPHIC, nothing else, so it doesn't make a difference for other
uses. There we only care about SAVE_EXPR pointer equality, if they aren't
pointer equal, they might, but might not evaluate the same.
If you want to be conservative with the warning, then just don't warn if
sizeof doesn't return a constant.
Jakub
[PATCH] PR libstdc++/86127 avoid unnecessary allocator conversions
There is no need to use an allocator of the correct value_type when
calling allocator_traits::construct and allocator_traits::destroy. The
existing node allocator can be used, instead of constructing a new
allocator object every time.
There's also no benefit to using __gnu_cxx::__alloc_traits instead of
std::allocator_traits to get the pointer and const_pointer types.
std::forward_list is only available for C++11 and later, when
std::allocator_traits is available too.
PR libstdc++/86127
* include/bits/forward_list.h (_Fwd_list_base::_Tp_alloc_type): Remove
unused typedef.
(_Fwd_list_base::_Node_alloc_traits): Use allocator_traits instead of
__gnu_cxx::__alloc_traits.
(_Fwd_list_base::_M_create_node, _Fwd_list_base::_M_erase_after):
Use node allocator to create and destroy elements.
(forward_list::_Tp_alloc_type): Remove unused typedef.
(forward_list::_Alloc_traits): Use allocator_traits instead of
__gnu_cxx::__alloc_traits.
Tested powerpc64le-linux, committed to trunk.
commit 777e063c0bb527a72539169ea25f06d7ad3de536
Author: Jonathan Wakely
Date: Wed Jun 13 13:54:47 2018 +0100
PR libstdc++/86127 avoid unnecessary allocator conversions
There is no need to use an allocator of the correct value_type when
calling allocator_traits::construct and allocator_traits::destroy. The
existing node allocator can be used, instead of constructing a new
allocator object every time.
There's also no benefit to using __gnu_cxx::__alloc_traits instead of
std::allocator_traits to get the pointer and const_pointer types.
std::forward_list is only available for C++11 and later, when
std::allocator_traits is available too.
PR libstdc++/86127
* include/bits/forward_list.h (_Fwd_list_base::_Tp_alloc_type):
Remove
unused typedef.
(_Fwd_list_base::_Node_alloc_traits): Use allocator_traits instead
of
__gnu_cxx::__alloc_traits.
(_Fwd_list_base::_M_create_node, _Fwd_list_base::_M_erase_after):
Use node allocator to create and destroy elements.
(forward_list::_Tp_alloc_type): Remove unused typedef.
(forward_list::_Alloc_traits): Use allocator_traits instead of
__gnu_cxx::__alloc_traits.
diff --git a/libstdc++-v3/include/bits/forward_list.h
b/libstdc++-v3/include/bits/forward_list.h
index b40256bd104..8c4c074e454 100644
--- a/libstdc++-v3/include/bits/forward_list.h
+++ b/libstdc++-v3/include/bits/forward_list.h
@@ -289,7 +289,6 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
struct _Fwd_list_base
{
protected:
- typedef __alloc_rebind<_Alloc, _Tp>_Tp_alloc_type;
typedef __alloc_rebind<_Alloc, _Fwd_list_node<_Tp>> _Node_alloc_type;
typedef __gnu_cxx::__alloc_traits<_Node_alloc_type> _Node_alloc_traits;
@@ -363,11 +362,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
_Node* __node = this->_M_get_node();
__try
{
- _Tp_alloc_type __a(_M_get_Node_allocator());
- typedef allocator_traits<_Tp_alloc_type> _Alloc_traits;
::new ((void*)__node) _Node;
- _Alloc_traits::construct(__a, __node->_M_valptr(),
- std::forward<_Args>(__args)...);
+ _Node_alloc_traits::construct(_M_get_Node_allocator(),
+ __node->_M_valptr(),
+ std::forward<_Args>(__args)...);
}
__catch(...)
{
@@ -437,10 +435,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
typedef _Fwd_list_base<_Tp, _Alloc> _Base;
typedef _Fwd_list_node<_Tp> _Node;
typedef _Fwd_list_node_base _Node_base;
- typedef typename _Base::_Tp_alloc_type _Tp_alloc_type;
typedef typename _Base::_Node_alloc_type _Node_alloc_type;
typedef typename _Base::_Node_alloc_traits _Node_alloc_traits;
- typedef __gnu_cxx::__alloc_traits<_Tp_alloc_type>_Alloc_traits;
+ typedef allocator_traits<__alloc_rebind<_Alloc, _Tp>>_Alloc_traits;
public:
// types:
diff --git a/libstdc++-v3/include/bits/forward_list.tcc
b/libstdc++-v3/include/bits/forward_list.tcc
index 3e12cd531d4..b41fbbb52f2 100644
--- a/libstdc++-v3/include/bits/forward_list.tcc
+++ b/libstdc++-v3/include/bits/forward_list.tcc
@@ -65,8 +65,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
{
_Node* __curr = static_cast<_Node*>(__pos->_M_next);
__pos->_M_next = __curr->_M_next;
- _Tp_alloc_type __a(_M_get_Node_allocator());
- allocator_traits<_Tp_alloc_type>::destroy(__a, __curr->_M_valptr());
+ _Node_alloc_traits::destroy(_M_get_Node_allocator(),
+ __curr->_M_valptr());
__curr->~_Node();
_M_put_node(__curr);
return
[PATCH] Fix vectorizable_operation destination vector type
This is needed for the ABSU_EXPR changes. It fixes a latent issue
in vect_recog_vector_vector_shift_pattern as well.
Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.
Richard.
2018-06-13 Richard Biener
* tree-vect-patterns.c (vect_recog_vector_vector_shift_pattern):
Properly set vector type of the intermediate stmt.
* tree-vect-stmts.c (vectorizable_operation): The destination
var always has vectype_out type.
diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
index 507c5b94f07..6786ffcd4c6 100644
--- a/gcc/tree-vect-patterns.c
+++ b/gcc/tree-vect-patterns.c
@@ -2185,6 +2185,11 @@ vect_recog_vector_vector_shift_pattern (vec
*stmts,
TYPE_PRECISION (TREE_TYPE (oprnd1)));
def = vect_recog_temp_ssa_var (TREE_TYPE (rhs1), NULL);
def_stmt = gimple_build_assign (def, BIT_AND_EXPR, rhs1, mask);
+ stmt_vec_info new_stmt_info
+ = new_stmt_vec_info (def_stmt, vinfo);
+ set_vinfo_for_stmt (def_stmt, new_stmt_info);
+ STMT_VINFO_VECTYPE (new_stmt_info)
+ = get_vectype_for_scalar_type (TREE_TYPE (rhs1));
new_pattern_def_seq (stmt_vinfo, def_stmt);
}
}
diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
index da4cb0737ed..9f365e31e49 100644
--- a/gcc/tree-vect-stmts.c
+++ b/gcc/tree-vect-stmts.c
@@ -5981,16 +5981,19 @@ vectorizable_operation (gimple *stmt,
gimple_stmt_iterator *gsi,
dump_printf_loc (MSG_NOTE, vect_location,
"transform binary/unary operation.\n");
- /* Handle def. */
- vec_dest = vect_create_destination_var (scalar_dest, vectype);
-
/* POINTER_DIFF_EXPR has pointer arguments which are vectorized as
vectors with unsigned elements, but the result is signed. So, we
need to compute the MINUS_EXPR into vectype temporary and
VIEW_CONVERT_EXPR it into the final vectype_out result. */
tree vec_cvt_dest = NULL_TREE;
if (orig_code == POINTER_DIFF_EXPR)
-vec_cvt_dest = vect_create_destination_var (scalar_dest, vectype_out);
+{
+ vec_dest = vect_create_destination_var (scalar_dest, vectype);
+ vec_cvt_dest = vect_create_destination_var (scalar_dest, vectype_out);
+}
+ /* Handle def. */
+ else
+vec_dest = vect_create_destination_var (scalar_dest, vectype_out);
/* In case the vectorization factor (VF) is bigger than the number
of elements that we can fit in a vectype (nunits), we have to generate
Re: [PATCH] relax -Wsizeof-pointer-memaccess for strncpy with size of source (PR 85931)
On 06/13/2018 01:58 AM, Jakub Jelinek wrote:
On Wed, Jun 13, 2018 at 09:46:28AM +0200, Richard Biener wrote:
gcc/ChangeLog:
PR c/85931
* fold-const.c (operand_equal_p): Handle SAVE_EXPR.
gcc/testsuite/ChangeLog:
PR c/85931
* gcc.dg/Wstringop-truncation-3.c: New test.
OK for the trunk. Richi and Jakub have the final say for the branch.
I'm a bit surprised that you don't just use operand_equal_p for both the
INTEGER_CST and !INTEGER_CST cases when testing dstsz == srcsz
It seemed that since the CST case compared the values of the size
expressions and the other case their lexicographic form they needed
to be different. But operand_equal_p() does value comparison for
constants so the conditional can be simplified by using just it
for both cases. Thanks for the suggestion! I've made that
simplification and committed r261515.
Jakub/Richard, can you please review the commit and let me know
if you have any concerns with backporting it to the GCC 8 branch?
(I will proceed if I don't hear anything this week.)
I'm fine with backporting it.
I'm actually worried about the fold-const.c change and don't understand, why
it has been done at all in the context of this PR.
case SAVE_EXPR:
if (flags & OEP_LEXICOGRAPHIC)
return OP_SAME (0);
return 0;
means it does something different from the state before the patch:
default:
return 0;
only if OEP_LEXICOGRAPHIC is set, which AFAIK is only in the
-Wduplicated-branches warning code, so why it is needed for
-Wstringop-truncation warning is unclear to me.
It lets sizeof_pointer_memaccess_warning detect that sizeof b
refers to the size of the VLA b in cases like the one below
(the sizeof b expression is represented as SAVE_EXPR (n)) and
avoid false positives:
void f (void*, ...);
void g (unsigned n)
{
char a[n], b[n];
f (a, b);
strncpy (a, b, sizeof b); // bogus -Wsizeof-pointer-memaccess
f (a);
}
Short of moving the SAVE_EXPR logic out of operand_equal_p()
and into sizeof_pointer_memaccess_warning, is there a better
way to determine that? (I would expect the SAVE_EXPR logic
in operand_equal_p() to be useful in other contexts besides
this warning.)
Martin
More importantly,
especially -fsanitize=undefined has a tendency of creating trees
which contain two or more uses of the same SAVE_EXPR at each level, and very
deep nesting levels of such SAVE_EXPRs, so if we handle it without checking
for duplicates, it results in exponential compile time complexity.
So, if we really want to handle SAVE_EXPR here, we'd need to create some
cache where we'd remember if in the same toplevel operand_equal_p call we've
already compared two particular SAVE_EXPRs and what was the result.
Jakub
Trivial rl78 build fix
My tester flagged rl78 as failing to build due to a trivial error.
Specifically the rl78 backend was passing integer 0 as the flags
argument to print_rtl_with_bb. The argument should either be omitted
completely (there's an appropriate overload) or use TDF_NONE.
Fixed thusly. Verified the rl78 port builds again. Tester will do some
deeper testing overnight (libgcc, newlib). Installing on the trunk.
Jeff
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 5b15216cdd5..e74509fa3f9 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2018-06-13 Jeff Law
+
+ * config/rl78/rl78.c (move_elim_pass): Use TDF_NONE rather than
+ integer 0 for argument to print_rtl_with_bb.
+ (rl78_reorg): Likewise.
+
2018-06-13 David Malcolm
* config/arc/arc.c (hwloop_optimize): Strengthen local "end_label"
diff --git a/gcc/config/rl78/rl78.c b/gcc/config/rl78/rl78.c
index 8346c9c31e0..03c78fc7549 100644
--- a/gcc/config/rl78/rl78.c
+++ b/gcc/config/rl78/rl78.c
@@ -228,7 +228,7 @@ move_elim_pass (void)
}
if (dump_file)
-print_rtl_with_bb (dump_file, get_insns (), 0);
+print_rtl_with_bb (dump_file, get_insns (), TDF_NONE);
return 0;
}
@@ -4317,7 +4317,7 @@ rl78_reorg (void)
if (dump_file)
{
fprintf (dump_file,
"\nDEVIRT:=AFTER=ALLOC=PHYSICAL=REGISTERS\n");
- print_rtl_with_bb (dump_file, get_insns (), 0);
+ print_rtl_with_bb (dump_file, get_insns (), TDF_NONE);
}
rl78_propogate_register_origins ();
@@ -4326,7 +4326,7 @@ rl78_reorg (void)
if (dump_file)
{
fprintf (dump_file,
"\nDEVIRT:=AFTER=PROPOGATION=\n");
- print_rtl_with_bb (dump_file, get_insns (), 0);
+ print_rtl_with_bb (dump_file, get_insns (), TDF_NONE);
fprintf (dump_file,
"\n==\n");
}
Re: [PATCH] handle OpenMP/OpenACC regions inside Fortran character functions (PR85703)
On Wed, Jun 13, 2018 at 07:06:23AM -0700, Cesar Philippidis wrote: > Character variables in Fortran are slightly different than reals and > integers because they can represent a single character or a string of > them. Consequently, due their nature, the Fortran resolver requires more > information to preform error checking on them. > > PR85703 exposed an ICE when an OpenACC or OpenMP region is nested inside > a Fortran character function. I've isolated the problem down to the > gfc_matching_function not being reset on entry to decode_oacc_directive > or decode_omp_directive. Usually decode_statement resets that variable, > but both OpenACC and OpenMP have their own statement decoders, and > therein lies the problems. The fix is to reset gfc_matching_function > early in those functions. > > Is this OK for trunk and GCC 8? Ok, thanks. > 2018-06-13 Cesar Philippidis > > PR fortran/85703 > > gcc/fortran/ > * parse.c (decode_oacc_directive): Set gfc_matching_function > to false. > (decode_omp_directive): Likewise. > > gcc/testsuite/ > * gfortran.dg/goacc/pr85703.f90: New test. > * gfortran.dg/gomp/pr85703.f90: New test. Jakub
[PATCH] handle OpenMP/OpenACC regions inside Fortran character functions (PR85703)
Character variables in Fortran are slightly different than reals and
integers because they can represent a single character or a string of
them. Consequently, due their nature, the Fortran resolver requires more
information to preform error checking on them.
PR85703 exposed an ICE when an OpenACC or OpenMP region is nested inside
a Fortran character function. I've isolated the problem down to the
gfc_matching_function not being reset on entry to decode_oacc_directive
or decode_omp_directive. Usually decode_statement resets that variable,
but both OpenACC and OpenMP have their own statement decoders, and
therein lies the problems. The fix is to reset gfc_matching_function
early in those functions.
Is this OK for trunk and GCC 8?
Thanks,
Cesar
2018-06-13 Cesar Philippidis
PR fortran/85703
gcc/fortran/
* parse.c (decode_oacc_directive): Set gfc_matching_function
to false.
(decode_omp_directive): Likewise.
gcc/testsuite/
* gfortran.dg/goacc/pr85703.f90: New test.
* gfortran.dg/gomp/pr85703.f90: New test.
>From 8f763ee8aab252d1435cd0f5e32f8773722a69b1 Mon Sep 17 00:00:00 2001
From: Cesar Philippidis
Date: Tue, 12 Jun 2018 10:32:16 -0700
Subject: [PATCH] fix pr85703
---
gcc/fortran/parse.c | 4
gcc/testsuite/gfortran.dg/goacc/pr85703.f90 | 9 +
gcc/testsuite/gfortran.dg/gomp/pr85703.f90 | 8
3 files changed, 21 insertions(+)
create mode 100644 gcc/testsuite/gfortran.dg/goacc/pr85703.f90
create mode 100644 gcc/testsuite/gfortran.dg/gomp/pr85703.f90
diff --git a/gcc/fortran/parse.c b/gcc/fortran/parse.c
index a3693a1..4ce6eb4 100644
--- a/gcc/fortran/parse.c
+++ b/gcc/fortran/parse.c
@@ -624,6 +624,8 @@ decode_oacc_directive (void)
gfc_clear_error (); /* Clear any pending errors. */
gfc_clear_warning (); /* Clear any pending warnings. */
+ gfc_matching_function = false;
+
if (gfc_pure (NULL))
{
gfc_error_now ("OpenACC directives at %C may not appear in PURE "
@@ -795,6 +797,8 @@ decode_omp_directive (void)
gfc_clear_error (); /* Clear any pending errors. */
gfc_clear_warning (); /* Clear any pending warnings. */
+ gfc_matching_function = false;
+
if (gfc_current_state () == COMP_FUNCTION
&& gfc_current_block ()->result->ts.kind == -1)
spec_only = true;
diff --git a/gcc/testsuite/gfortran.dg/goacc/pr85703.f90 b/gcc/testsuite/gfortran.dg/goacc/pr85703.f90
new file mode 100644
index 000..d9de1a3
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/pr85703.f90
@@ -0,0 +1,9 @@
+! PR fortran/85703
+! { dg-do compile }
+
+character function f()
+ !$acc parallel loop reduction(+:a)
+ do i = 1, 4
+ end do
+ !$acc end parallel loop
+end
diff --git a/gcc/testsuite/gfortran.dg/gomp/pr85703.f90 b/gcc/testsuite/gfortran.dg/gomp/pr85703.f90
new file mode 100644
index 000..7ca2b93
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/pr85703.f90
@@ -0,0 +1,8 @@
+! PR fortran/85703
+! { dg-do compile }
+
+character function f()
+ !$omp single
+ !$omp end single
+ f = 'a'
+end
--
2.7.4
[RFC] New features for multilib control
Hi, This patch was developed as part of preparing ever more complex multilib combinations for the MIPS architecture and aims to solve several problems in this area. I've attempted to be quite verbose in the description, so that I can explain how I am using various terms as pretty much everyone has a different understanding (and I don't claim mine to be 'right' either). The changes aim to: 1) Eliminate the fallback multilib The fallback multilib (top level of 'lib') is often annoying because it is used for any combination of options that do not match a specific multilib. Quite often this default multilib is incompatible with the build options that end up linking against it, leading to bizarre link time messages that confuse ordinary users. 2) Move the default multilib to a subfolder Having successfully eliminated the fallback multilib it is also true that it would eliminate the 'default' multilib as well. I.e. the library used when no relevant user options are supplied. Moving this library to a subfolder has two benefits. a) it still exists! and b) the location of this library becomes invariant irrespective of which options are build-time configured as default. 3) Preserve/use invariant paths for multilib variants A simplistic multilib specification leads to a nested set of folders, where the top level is the left-most multilib directory and the bottom is the right most. Introducing a new axis of multilib configuration changes this path structure leading to the different library configurations to move around. This is not in itself a problem, as the compiler driver can always locate the right path for any given build, but it does cause issues when doing configuration management of binary toolchains. When there are many different multilibs it is ideal to ship/install only the ones that are important and if the paths keep changing over time this process is complex and confusing. This issue is effectively solved by the MULTI_OSDIRNAMES feature but using it for both sysroot and compiler internal libraries is even better. 4) Support un-released multilib configurations This one sounds weird but it is quite valuable. When an architecture has 70+ possible multilib variants but only 40 of them are currently known to be needed then only build and release 40 variants. However, if it turns out that another configuration becomes useful then it is often handy to just build the missing configuration and install it into the pre-existing release. So, the driver needs to know about all multilibs and their paths but the build process should only build a subset. So, the solution... Firstly, be verbose about the MULTILIB_OPTIONS needed for a target. Add in the default options as well as all the others that may, or could, ever be supported by the current compiler features. For MIPS supporting MIPS32r1 onwards it looks like this: MULTILIB_OPTIONS = muclibc mips32/mips32r2/mips32r6/mips64/mips64r2/mips64r6 mips16/mmicromips mabi=32/mabi=n32/mabi=64 EB/EL msoft-float mnan=2008 This does create an enormous matrix of possible configurations so this has to be trimmed to the valid set using MULTILIB_REQUIRED. Note that the valid set should include any that you may wish to support even if you don't want to build/release them. By having the default options in then this leads to having two copies of the 'default' multilib; one with the options implicitly set and one with them explicitly set. Second, remove the multilib with the implicit default options. This equates to the '.' multilib entry. To do this use MULTILIB_EXCLUSIONS to remove the negated form of every MULTILIB_OPTION: MULTILIB_EXCLUSIONS = !muclibc/!mips32/!mips32r2/!mips32r6/!mips64/!mips64r2/!mips64r6/!mips16/!mm icromips/!mabi=32/!mabi=n32/!mabi=64/!EB/!EL/!msoft-float/!mnan=2008 Third, set the MULTILIB_OSDIRNAMES to have an entry for every valid combination of options and use the '!' modifier. Honestly, I'm not sure how/why this works but this leads to both the internal library paths and sysroot paths using the OSDIRNAME instead of the nested tree of MULTILIB_DIRNAMES. Choose a path for each variant that you will never change again; I used an encoded form of the configuration for MIPS: MULTILIB_OSDIRNAMES = mips32r6/mabi.32/EB/mnan.2008=!mips-r6-hard$(is_newlib)/lib Fourth, deal with the fallout from the config-ml.in logic which handles the initial 'configure' of a library differently to all of the multilib configurations. The basic idea is that since the default multilib will now report that it belongs in a subdirectory then, when configuring the top level multilib, query the driver for the folder. Later when iterating through the multilib specs skip the one that matches the default configuration. Each configuration will be built exactly once and all of them will be placed in a subfolder leaving the top level install folder bare. Fifth, restrict the set of multilibs that actually get built for any given compiler. This is sort-of a new concept so I added a
[PATCH] update error reporting for OpenACC wait (PR85702)
Prior to this patch, when the Fortran FE detected an invalid argument to
the OpenACC wait directive, it was trying to read the location of the
expression after it checked does not exist to error diagnostics. This
patch resolves that issue by reporting the error using %C.
I'll commit this patch to trunk as obvious.
Cesar
2018-06-13 Cesar Philippidis
PR fortran/85702
gcc/fortran/
* openmp.c (gfc_match_oacc_wait): Use %C to report error location.
gcc/testsuite/
* gfortran.dg/goacc/pr85702.f90: New test.
>From 07022efa1ba4a58fa12c3f8a3b911fba32a5df1b Mon Sep 17 00:00:00 2001
From: Cesar Philippidis
Date: Mon, 11 Jun 2018 14:03:47 -0700
Subject: [PATCH] fix PR85702
---
gcc/fortran/openmp.c| 3 +--
gcc/testsuite/gfortran.dg/goacc/pr85702.f90 | 6 ++
2 files changed, 7 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/goacc/pr85702.f90
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 5c13312..97d6e78 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -2177,8 +2177,7 @@ gfc_match_oacc_wait (void)
{
if (el->expr == NULL)
{
- gfc_error ("Invalid argument to !$ACC WAIT at %L",
- _list->expr->where);
+ gfc_error ("Invalid argument to !$ACC WAIT at %C");
return MATCH_ERROR;
}
diff --git a/gcc/testsuite/gfortran.dg/goacc/pr85702.f90 b/gcc/testsuite/gfortran.dg/goacc/pr85702.f90
new file mode 100644
index 000..7488618
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc/pr85702.f90
@@ -0,0 +1,6 @@
+! PR fortran/85702
+! { dg-do compile }
+
+subroutine s
+ !$acc wait(*) ! { dg-error "Invalid argument to ..ACC WAIT" }
+end
--
2.7.4
libgo patch committed: Update to 1.10.3 release
This patch updates libgo to the 1.10.3 release. Bootstrapped and ran
Go testsuite on x86_64-pc-linux-gnu. Committed to mainline and GCC 8
branch.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 261521)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-bfe3a9b26c8b2e1b9ef34a7232a2d1529e639bbf
+6743db0ed81e313acf66c00a4ed0e2dcaaca2c9f
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: libgo/MERGE
===
--- libgo/MERGE (revision 261521)
+++ libgo/MERGE (working copy)
@@ -1,4 +1,4 @@
-71bdbf431b79dff61944f22c25c7e085ccfc25d5
+fe8a0d12b14108cbe2408b417afcaab722b0727c
The first line of this file holds the git revision number of the
last merge done from the master library sources.
Index: libgo/VERSION
===
--- libgo/VERSION (revision 261521)
+++ libgo/VERSION (working copy)
@@ -1 +1 @@
-go1.10.2
+go1.10.3
Index: libgo/go/cmd/go/internal/get/discovery.go
===
--- libgo/go/cmd/go/internal/get/discovery.go (revision 261521)
+++ libgo/go/cmd/go/internal/get/discovery.go (working copy)
@@ -55,6 +55,13 @@ func parseMetaGoImports(r io.Reader) (im
continue
}
if f := strings.Fields(attrValue(e.Attr, "content")); len(f) ==
3 {
+ // Ignore VCS type "mod", which is new Go modules.
+ // This code is for old go get and must ignore the new
mod lines.
+ // Otherwise matchGoImport will complain about two
+ // different metaImport lines for the same Prefix.
+ if f[1] == "mod" {
+ continue
+ }
imports = append(imports, metaImport{
Prefix: f[0],
VCS: f[1],
Index: libgo/go/cmd/go/internal/get/get.go
===
--- libgo/go/cmd/go/internal/get/get.go (revision 261521)
+++ libgo/go/cmd/go/internal/get/get.go (working copy)
@@ -209,7 +209,7 @@ var downloadRootCache = map[string]bool{
// download runs the download half of the get command
// for the package named by the argument.
func download(arg string, parent *load.Package, stk *load.ImportStack, mode
int) {
- if mode != 0 {
+ if mode != 0 {
// Caller is responsible for expanding vendor paths.
panic("internal error: download mode has useVendor set")
}
@@ -217,7 +217,7 @@ func download(arg string, parent *load.P
if parent == nil {
return load.LoadPackage(path, stk)
}
- return load.LoadImport(path, parent.Dir, parent, stk, nil, mode)
+ return load.LoadImport(path, parent.Dir, parent, stk, nil,
mode|load.ResolveModule)
}
p := load1(arg, mode)
@@ -346,12 +346,12 @@ func download(arg string, parent *load.P
base.Errorf("%s", err)
continue
}
- // If this is a test import, apply vendor lookup now.
- // We cannot pass useVendor to download, because
+ // If this is a test import, apply module and vendor
lookup now.
+ // We cannot pass ResolveImport to download, because
// download does caching based on the value of path,
// so it must be the fully qualified path already.
if i >= len(p.Imports) {
- path = load.VendoredImportPath(p, path)
+ path = load.ResolveImportPath(p, path)
}
download(path, p, stk, 0)
}
Index: libgo/go/cmd/go/internal/get/pkg_test.go
===
--- libgo/go/cmd/go/internal/get/pkg_test.go(revision 261521)
+++ libgo/go/cmd/go/internal/get/pkg_test.go(working copy)
@@ -48,6 +48,20 @@ var parseMetaGoImportsTests = []struct {
},
},
{
+ `https://github.com/rsc/foo/bar;>
+ http://github.com/rsc/baz/quux;>`,
+ []metaImport{
+ {"foo/bar", "git", "https://github.com/rsc/foo/bar"},
+ },
+ },
+ {
+ `http://github.com/rsc/baz/quux;>
+ https://github.com/rsc/foo/bar;>`,
+ []metaImport{
+ {"foo/bar", "git", "https://github.com/rsc/foo/bar"},
+ },
+
Re: [PATCH 1/5] emit-rtl.c: more typesafety
On Tue, 2018-06-12 at 15:23 -0600, Jeff Law wrote: > On 06/12/2018 03:16 PM, David Malcolm wrote: > > On Tue, 2018-06-12 at 14:50 -0600, Jeff Law wrote: > > > > Ultimately these boil down to: > > > > gcc_checking_assert (INSN_P (rt) > > || NOTE_P (rt) > > || JUMP_TABLE_DATA_P (rt) > > || BARRIER_P (rt) > > || LABEL_P (rt)); > > > > (with an "if (rt)" for the safe_a_a variant), and hence they're > > runtime > > checks (and only enabled when #if CHECKING_P). > > Ack. > > > > > > Obviously your testing ought to be sufficient for the > > > former. Mine will help with the latter since it'll build arc, > > > bfin > > > and > > > c6x elf targets through newlib. For sh it'll build a mixture of > > > -elf > > > things through newlib and it'll bootstrap sh4. > > > > Is this testing something that you've already triggered, or does it > > happen upon commit to trunk? > > THe jobs poll the various relevant git servers (gcc, binutils, > newlib, > glibc, linux kernel) once per day . When a change is detected in any > of > the repos a full build is started. > > > > > > (The sh changes are the only aspect of this kit that I'm keen for > > review/testing of, as it needed extra casts; I *think* everywhere I > > added them are places where rtx are already being unconditionally > > treated as instruction nodes, but your eyes/testing would be > > helpful). > > They looked reasonable to me. Thankfully sh is one that we can > actually > bootstrap GCC within a chroot using qemu, so it'll get fairly > extensive > testing. It takes ~8hrs, but that's not too bad :-) Thanks. For reference, I've committed the combination of the patches to trunk as r261547. Dave (I also smoketested the build of arc, since I noticed some changes there since my earlier testing).
Re: [RFC][PATCH] Stabilize a few qsort comparison functions
On Wed, 13 Jun 2018, Franz Sirl wrote: > So, unless someone thinks one of the changes makes sense anyway, this patch is > obsolete. Yeah, I think that in light of gcc_qsort work this patch is completely unnecessary now, and shouldn't be applied. In fact, reversing earlier artificial stabilization changes may be desirable. Alexander
Re: [patch, fortran] Handling of .and. and .or. expressions
Hi Thomas, >the attached patch introduces the following changes: thanks a lot for working on this! >If a logical .and. or .or. expression contains a reference to a >function >which is impure and which also does not behave like a pure function >(i.e. does not have the implicit_pure attribute set), it emits a >warning with -Wsurprising that the function might not be evaluated. >(-Wsurprising is enabled by -Wall). I think you are warning about too many cases here. Warning about an impure function as the *second* operand of the logical operators should be enough, I think. AFAICS there is no danger of the first operand not being evaluated, right? >It special cases the idiom if (associated(m) .and. m%t) which >people appear to use. I don't fully understand why you do this, but in any case it should not be necessary if you warn about the second operand only. >And, if there is an expression like func() .and. flag , it >reverses the test as an optimization. I really don't like this part. It actually introduces more problems of the sort that we're trying to warn about ... Cheers, Janus >2018-06-11 Thomas Koenig > > PR fortran/57160 > PR fortran/85599 > * dump-parse-tree (show_attr): Add handling of implicit_pure. > * resolve.c (impure_function_callback): New function. > (resolve_operator): Call it vial gfc_expr_walker. Special-case > if (associated(m) .and. m%t). If an .and. or .or. expression > has a function or a non-function, exchange the operands. > >2018-06-11 Thomas Koenig > > PR fortran/57160 > PR fortran/85599 > * gfortran.dg/logical_evaluation_1.f90: New test. > * gfortran.dg/alloc_comp_default_init_2.f90: Fix code which > implicitly depends on short-circuiting.
Re: [RFC][PR64946] "abs" vectorization fails for char/short types
On Mon, Jun 11, 2018 at 10:28 AM Kugan Vivekanandarajah
wrote:
>
> Hi Richard,
>
> Thanks for the review and sorry for getting back to you late.
>
> On 4 June 2018 at 18:38, Richard Biener wrote:
> > On Mon, Jun 4, 2018 at 10:18 AM Kugan Vivekanandarajah
> > wrote:
> >>
> >> Hi Richard,
> >>
> >> Thanks for the review.
> >>
> >> On 1 June 2018 at 22:20, Richard Biener wrote:
> >> > On Fri, Jun 1, 2018 at 4:12 AM Kugan Vivekanandarajah
> >> > wrote:
> >> >>
> >> >> Hi Richard,
> >> >>
> >> >> This is the revised patch based on the review and the discussion in
> >> >> https://gcc.gnu.org/ml/gcc/2018-05/msg00179.html.
> >> >>
> >> >> In summary:
> >> >> - I skipped (element_precision (type) < element_precision (TREE_TYPE
> >> >> (@0))) in the match.pd pattern as this would prevent transformation
> >> >> for the case in PR.
> >> >> that is, I am interested in is something like:
> >> >> char t = (char) ABS_EXPR <(int) x>
> >> >> and I want to generate
> >> >> char t = (char) ABSU_EXPR
> >> >>
> >> >> - I also haven't added all the necessary match.pd changes for
> >> >> ABSU_EXPR. I have a patch for that but will submit separately based on
> >> >> this reveiw.
> >> >>
> >> >> - I also tried to add ABSU_EXPRsupport in the places as necessary by
> >> >> grepping for ABS_EXPR.
> >> >>
> >> >> - I also had to add special casing in vectorizer for ABSU_EXP as its
> >> >> result is unsigned type.
> >> >>
> >> >> Is this OK. Patch bootstraps and the regression test is ongoing.
> >> >
> >> > The c/c-typeck.c:build_unary_op change looks unnecessary - the
> >> > C FE should never generate this directly (the c-common one might
> >> > be triggered by early folding I guess).
> >>
> >> The Gimple FE testcase is running into this.
> >
> > Ah, OK then.
> >
> >> >
> >> > @@ -1761,6 +1762,9 @@ const_unop (enum tree_code code, tree type, tree
> >> > arg0)
> >> >if (TREE_CODE (arg0) == INTEGER_CST || TREE_CODE (arg0) ==
> >> > REAL_CST)
> >> > return fold_abs_const (arg0, type);
> >> >break;
> >> > +case ABSU_EXPR:
> >> > + return fold_convert (type, fold_abs_const (arg0,
> >> > + signed_type_for
> >> > (type)));
> >> >
> >> > case CONJ_EXPR:
> >> >
> >> > I think this will get you bogus TREE_OVERFLOW flags set on ABSU
> >> > (-INT_MIN).
> >> >
> >> > I think you want to change fold_abs_const to properly deal with arg0
> >> > being
> >> > signed and type unsigned. That is, sth like
> >> >
> >> > diff --git a/gcc/fold-const.c b/gcc/fold-const.c
> >> > index 6f80f1b1d69..f60f9c77e91 100644
> >> > --- a/gcc/fold-const.c
> >> > +++ b/gcc/fold-const.c
> >> > @@ -13843,18 +13843,19 @@ fold_abs_const (tree arg0, tree type)
> >> >{
> >> > /* If the value is unsigned or non-negative, then the absolute
> >> > value
> >> >is the same as the ordinary value. */
> >> > - if (!wi::neg_p (wi::to_wide (arg0), TYPE_SIGN (type)))
> >> > - t = arg0;
> >> > + wide_int val = wi::to_wide (arg0);
> >> > + bool overflow = false;
> >> > + if (!wi::neg_p (val, TYPE_SIGN (TREE_TYPE (arg0
> >> > + ;
> >> >
> >> > /* If the value is negative, then the absolute value is
> >> >its negation. */
> >> > else
> >> > - {
> >> > - bool overflow;
> >> > - wide_int val = wi::neg (wi::to_wide (arg0), );
> >> > - t = force_fit_type (type, val, -1,
> >> > - overflow | TREE_OVERFLOW (arg0));
> >> > - }
> >> > + wide_int val = wi::neg (val, );
> >> > +
> >> > + /* Force to the destination type, set TREE_OVERFLOW for signed
> >> > + TYPE only. */
> >> > + t = force_fit_type (type, val, 1, overflow | TREE_OVERFLOW
> >> > (arg0));
> >> >}
> >> >break;
> >> >
> >> > and then simply share the const_unop code with ABS_EXPR.
> >>
> >> Done.
> >>
> >> > diff --git a/gcc/match.pd b/gcc/match.pd
> >> > index 14386da..7d7c132 100644
> >> > --- a/gcc/match.pd
> >> > +++ b/gcc/match.pd
> >> > @@ -102,6 +102,14 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >> > (match (nop_convert @0)
> >> > @0)
> >> >
> >> > +(simplify (abs (convert @0))
> >> > + (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
> >> > + && !TYPE_UNSIGNED (TREE_TYPE (@0))
> >> > + && !TYPE_UNSIGNED (type))
> >> > + (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); }
> >> > + (convert (absu:utype @0)
> >> > +
> >> > +
> >> >
> >> > please put a comment before the pattern. I believe there's no
> >> > need to check for !TYPE_UNSIGNED (type). Note this
> >> > also converts abs ((char)int-var) to (char)absu(int-var) which
> >> > doesn't make sense. The original issue you want to address
> >> > here is the case where TYPE_PRECISION of @0 is less than
> >> > the precision of type. That is, you want to remove language
> >> > introduced integer promotion of @0 which only is possible
> >>
Re: [RFC][PATCH] Stabilize a few qsort comparison functions
Am 2018-06-12 um 23:49 schrieb Jeff Law: On 02/07/2018 09:58 AM, Franz Sirl wrote: Hi, this is the result of an attempt to minimize the differences between the compile results of a Linux-based and a Cygwin64-based powerpc-eabi cross toolchain. The method used was: - find the -fverbose-asm assembler files that differ - compile that file again on both platforms with -O2 -g3 -fdump-tree-all-all -fdump-rtl-all -fdump-noaddr - look for the first dump file with differences and check that pass for qsort's - stabilize the compare functions With some help on IRC to better understand the passes and some serious debugging of GCC I came up with this patch. On the tested codebase the differences in the assembler sources are now down to 0. If the various pass maintainers have better ideas on how to stabilize the compare functions, I'll be happy to verify them on the codebase. For the SRA patch I already have an alternate version with an additional ID member. Comments? Bootstrapped on linux-x86_64, no testsuite regressions. Franz Sirl 2018-02-07 Franz Sirl * ira-build.c (object_range_compare_func): Stabilize sort. * tree-sra.c (compare_access_positions): Likewise. * varasm.c (output_object_block_compare): Likewise. * tree-ssa-loop-ivopts.c (group_compare_offset): Likewise. (struct iv_common_cand): New member. (record_common_cand): Initialize new member. (common_cand_cmp): Use new member to stabilize sort. * tree-vrp.c (struct assert_locus): New member. (register_new_assert_for): Initialize new member. (compare_assert_loc): Use new member to stabilize sort. This looks pretty reasonable. I don't think you've contributed much recently, do you still have write access to the repository? Hi Jeff, after Alexander Monakov's gcc_qsort changes, this patch is not necessary anymore. I've verified that with a backport (the 2 patches r260216 and r260222 applied cleanly) of gcc_qsort to the gcc-8-branch. The resulting powerpc-eabi crosscompilers produce no more unexpected differences between a Linux and a Cygwin host. Tested (same like with my patch) by comparing the -fverbose-asm assembly output on a complete rebuild of the software here. So, unless someone thinks one of the changes makes sense anyway, this patch is obsolete. On the repository write access, yes, I don't have one anymore. But before reactivating that I need to do the legal paperwork, because unless before when GCC was a strictly private pet project for me, it now is work related. I already got permission from my company for that, just need to find some spare time to start the legal stuff. Franz
Fix PR target/86048
This is a follow-up to PR target/84763 present on mainline and 8 branch:
https://gcc.gnu.org/ml/gcc-patches/2018-03/msg00418.html
Since i386_pe_seh_cold_init now uses a small pre-allocation when there is
__builtin_{frame,return}_address in the function, it also may need to split
CFI save directives (.seh_savexmm/.seh_savereg) into a first group emitted
after the pre-allocation and the rest emitted after the full allocation.
Tested on x86-64/Windows, applied on the mainline and 8 branch as obvious.
2018-06-13 Eric Botcazou
PR target/86048
* config/i386/winnt.c (i386_pe_seh_cold_init): Do not emit negative
offsets for register save directives. Emit a second batch of save
directives, if need be, when the function accesses prior frames.
2018-06-13 Eric Botcazou
* gcc.target/i386/pr86048.c: New test.
--
Eric BotcazouIndex: config/i386/winnt.c
===
--- config/i386/winnt.c (revision 261473)
+++ config/i386/winnt.c (working copy)
@@ -922,11 +922,14 @@ i386_pe_seh_cold_init (FILE *f, const ch
fprintf (f, "\t.seh_stackalloc\t" HOST_WIDE_INT_PRINT_DEC "\n", offset);
for (int regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
-if (seh->reg_offset[regno] > 0)
+if (seh->reg_offset[regno] > 0 && seh->reg_offset[regno] <= alloc_offset)
{
- fputs ((SSE_REGNO_P (regno) ? "\t.seh_savexmm\t"
- : GENERAL_REGNO_P (regno) ? "\t.seh_savereg\t"
- : (gcc_unreachable (), "")), f);
+ if (SSE_REGNO_P (regno))
+ fputs ("\t.seh_savexmm\t", f);
+ else if (GENERAL_REGNO_P (regno))
+ fputs ("\t.seh_savereg\t", f);
+ else
+ gcc_unreachable ();
print_reg (gen_rtx_REG (DImode, regno), 0, f);
fprintf (f, ", " HOST_WIDE_INT_PRINT_DEC "\n",
alloc_offset - seh->reg_offset[regno]);
@@ -949,6 +952,20 @@ i386_pe_seh_cold_init (FILE *f, const ch
offset = seh->sp_offset - alloc_offset;
if (offset > 0 && offset < SEH_MAX_FRAME_SIZE)
fprintf (f, "\t.seh_stackalloc\t" HOST_WIDE_INT_PRINT_DEC "\n", offset);
+
+ for (int regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
+ if (seh->reg_offset[regno] > alloc_offset)
+ {
+ if (SSE_REGNO_P (regno))
+ fputs ("\t.seh_savexmm\t", f);
+ else if (GENERAL_REGNO_P (regno))
+ fputs ("\t.seh_savereg\t", f);
+ else
+ gcc_unreachable ();
+ print_reg (gen_rtx_REG (DImode, regno), 0, f);
+ fprintf (f, ", " HOST_WIDE_INT_PRINT_DEC "\n",
+ seh->sp_offset - seh->reg_offset[regno]);
+ }
}
fputs ("\t.seh_endprologue\n", f);
/* PR target/86048 */
/* { dg-do assemble } */
/* { dg-options "-O2" } */
/* { dg-require-effective-target return_address } */
extern void abort (void);
void *foo (unsigned int *data, unsigned int len)
{
unsigned int local_data[128];
if (len > 128)
abort ();
for (unsigned int i = 0; i < len; i++)
local_data[i] = data[i] + data[len - 1 - i] * 2;
void *ret = __builtin_frame_address (0);
for (unsigned int i = 0; i < len; i++)
ret = ret + local_data[i] % 8;
__asm__ __volatile__ ("" : : : "%xmm6");
return ret;
}
Re: [PATCH] Use flags_from_decl_or_type in lto_symtab_merge_p (PR ipa/85248).
On Fri, Jun 8, 2018 at 1:10 PM Martin Liška wrote: > > Hi. > > Second follow-up patch uses flags_from_decl_or_type in LTO merging > of declarations. Hope it's more cleaner approach. > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? That doesn't handle the "error" attribute case anymore? We also have attribute_list_equal / attribute_list_contained that could be used here? Richard. > Martin
Add lto-streamer-out dump file
Hi,
this patch adds lto-streamer-out dump file which journal what is streamed
into the output file and why. For example compiling:
#include
main()
{
fprintf (stderr, "Error\n");
}
We get
Creating compressed section .gnu.lto_.profile.3e57b5d44bc7b5b7
Creating output block for icf
Creating compressed section .gnu.lto_.icf.3e57b5d44bc7b5b7
Creating output block for jmpfuncs
Streaming SCC of
Streamingto jmpfuncs
Streamingto jmpfuncs
Encoding indexableas 0
Encoding indexableas 1
Finished SCC of
Streaming SCC of
Finished SCC of
Streaming SCC of
Encoding indexableas 2
Finished SCC of
Creating compressed section .gnu.lto_.jmpfuncs.3e57b5d44bc7b5b7
Creating output block for inline
Creating compressed section .gnu.lto_.inline.3e57b5d44bc7b5b7
Creating compressed section .gnu.lto_.pureconst.3e57b5d44bc7b5b7
Streaming body of main
Creating output block for function_body
Streaming SCC of
Streamingto function_body
Encoding indexableas 0
Encoding indexableas 1
Encoding indexableas 2
Encoding indexableas 0
Finished SCC of
Streaming SCC of
Streamingto function_body
Finished SCC of
Encoding indexableas 3
Streaming gimple stmt stderr.0_1 = stderr;
Streaming SCC of
Streamingto function_body
Encoding indexableas 4
Streamingto function_body
Encoding indexableas 0
Finished SCC of
Streaming gimple stmt fwrite ("Error\n", 1, 6, stderr.0_1);
Streaming SCC of
Streamingto function_body
Encoding indexableas 5
Encoding indexableas 1
Finished SCC of
Streaming SCC of
Streamingto function_body
Streamingto function_body
Encoding indexableas 6
Encoding indexableas 7
Finished SCC of
Streaming SCC of
Encoding indexableas 8
Finished SCC of
Encoding indexableas 9
Streaming gimple stmt return 0;
Creating compressed section .gnu.lto_main.3e57b5d44bc7b5b7
Finished streaming main
Encoding indexableas 0
Encoding indexableas 0
Encoding indexableas 1
Creating compressed section .gnu.lto_.symbol_nodes.3e57b5d44bc7b5b7
Creating compressed section .gnu.lto_.refs.3e57b5d44bc7b5b7
Creating output block for decls
Creating compressed section .gnu.lto_.decls.3e57b5d44bc7b5b7
Outputting global stream
0:
Streaming SCC of
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Finished SCC of
1:
Streaming SCC of
Streamingto decls
Finished SCC of
2:
0:
Streaming SCC of
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streaming
to decls
Streamingto
decls
Streamingto decls
Streamingto decls
Finished SCC of
1:
Streaming SCC of
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Finished SCC of
0:
Streaming SCC of
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
Streamingto decls
[PATCH] [ARC] Add support for HS4x cpus.
From: Claudiu Zissulescu
This patch adds support for two ARCHS variations.
Ok to apply?
Claudiu
gcc/
2017-03-10 Claudiu Zissulescu
* config/arc/arc-arch.h (arc_tune_attr): Add new tune parameters
for ARCHS4x.
* config/arc/arc-cpus.def (hs4x): New cpu.
(hs4xd): Likewise.
* config/arc/arc-tables.opt: Regenerate.
* config/arc/arc.c (arc_sched_issue_rate): New function.
(TARGET_SCHED_ISSUE_RATE): Define.
(TARGET_SCHED_EXPOSED_PIPELINE): Likewise.
* config/arc/arc.md (attr type): Add fpu_fuse, fpu_sdiv, fpu_ddiv,
fpu_cvt.
(attr tune): Add ARCHS4x tune values.
(attr tune_dspmpy): Define.
(*tst): Correct instruction type.
* config/arc/arcHS.md: Don't use this automaton for ARCHS4x cpus.
* config/arc/arcHS4x.md: New file.
* config/arc/fpu.md: Update instruction type attributes.
* config/arc/t-multilib: Regenerate.
---
gcc/config/arc/arc-arch.h | 5 +-
gcc/config/arc/arc-cpus.def | 8 +-
gcc/config/arc/arc-tables.opt | 6 +
gcc/config/arc/arc.c | 19 +++
gcc/config/arc/arc.md | 24 +++-
gcc/config/arc/arcHS.md | 6 +
gcc/config/arc/arcHS4x.md | 221 ++
gcc/config/arc/fpu.md | 16 +--
8 files changed, 289 insertions(+), 16 deletions(-)
create mode 100644 gcc/config/arc/arcHS4x.md
diff --git a/gcc/config/arc/arc-arch.h b/gcc/config/arc/arc-arch.h
index 64866dd529b..01f95946623 100644
--- a/gcc/config/arc/arc-arch.h
+++ b/gcc/config/arc/arc-arch.h
@@ -73,7 +73,10 @@ enum arc_tune_attr
ARC_TUNE_ARC600,
ARC_TUNE_ARC700_4_2_STD,
ARC_TUNE_ARC700_4_2_XMAC,
-ARC_TUNE_CORE_3
+ARC_TUNE_CORE_3,
+ARC_TUNE_ARCHS4X,
+ARC_TUNE_ARCHS4XD,
+ARC_TUNE_ARCHS4XD_SLOW
};
/* CPU specific properties. */
diff --git a/gcc/config/arc/arc-cpus.def b/gcc/config/arc/arc-cpus.def
index 1fce81f6933..4aa422f1a39 100644
--- a/gcc/config/arc/arc-cpus.def
+++ b/gcc/config/arc/arc-cpus.def
@@ -59,10 +59,12 @@ ARC_CPU (archs, hs, FL_MPYOPT_2|FL_DIVREM|FL_LL64,
NONE)
ARC_CPU (hs34, hs, FL_MPYOPT_2, NONE)
ARC_CPU (hs38, hs, FL_MPYOPT_9|FL_DIVREM|FL_LL64, NONE)
ARC_CPU (hs38_linux, hs, FL_MPYOPT_9|FL_DIVREM|FL_LL64|FL_FPU_FPUD_ALL, NONE)
+ARC_CPU (hs4x, hs, FL_MPYOPT_9|FL_DIVREM|FL_LL64, ARCHS4X)
+ARC_CPU (hs4xd, hs, FL_MPYOPT_9|FL_DIVREM|FL_LL64, ARCHS4XD)
-ARC_CPU (arc600, 6xx, FL_BS, ARC600)
-ARC_CPU (arc600_norm,6xx, FL_BS|FL_NORM, ARC600)
-ARC_CPU (arc600_mul64, 6xx, FL_BS|FL_NORM|FL_MUL64, ARC600)
+ARC_CPU (arc600, 6xx, FL_BS, ARC600)
+ARC_CPU (arc600_norm, 6xx, FL_BS|FL_NORM, ARC600)
+ARC_CPU (arc600_mul64,6xx, FL_BS|FL_NORM|FL_MUL64, ARC600)
ARC_CPU (arc600_mul32x16, 6xx, FL_BS|FL_NORM|FL_MUL32x16, ARC600)
ARC_CPU (arc601, 6xx, 0, ARC600)
ARC_CPU (arc601_norm,6xx, FL_NORM, ARC600)
diff --git a/gcc/config/arc/arc-tables.opt b/gcc/config/arc/arc-tables.opt
index 3b17b3de7d5..2afaf5bd83c 100644
--- a/gcc/config/arc/arc-tables.opt
+++ b/gcc/config/arc/arc-tables.opt
@@ -63,6 +63,12 @@ Enum(processor_type) String(hs38) Value(PROCESSOR_hs38)
EnumValue
Enum(processor_type) String(hs38_linux) Value(PROCESSOR_hs38_linux)
+EnumValue
+Enum(processor_type) String(hs4x) Value(PROCESSOR_hs4x)
+
+EnumValue
+Enum(processor_type) String(hs4xd) Value(PROCESSOR_hs4xd)
+
EnumValue
Enum(processor_type) String(arc600) Value(PROCESSOR_arc600)
diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
index 2bedc9af37e..03a2f4223c0 100644
--- a/gcc/config/arc/arc.c
+++ b/gcc/config/arc/arc.c
@@ -483,6 +483,22 @@ arc_autovectorize_vector_sizes (vector_sizes *sizes)
}
}
+
+/* Implements target hook TARGET_SCHED_ISSUE_RATE. */
+static int
+arc_sched_issue_rate (void)
+{
+ switch (arc_tune)
+{
+case TUNE_ARCHS4X:
+case TUNE_ARCHS4XD:
+ return 3;
+default:
+ break;
+}
+ return 1;
+}
+
/* TARGET_PRESERVE_RELOAD_P is still awaiting patch re-evaluation / review. */
static bool arc_preserve_reload_p (rtx in) ATTRIBUTE_UNUSED;
static rtx arc_delegitimize_address (rtx);
@@ -565,6 +581,9 @@ static rtx arc_legitimize_address_0 (rtx, rtx, machine_mode
mode);
#undef TARGET_SCHED_ADJUST_PRIORITY
#define TARGET_SCHED_ADJUST_PRIORITY arc_sched_adjust_priority
+#undef TARGET_SCHED_ISSUE_RATE
+#define TARGET_SCHED_ISSUE_RATE arc_sched_issue_rate
+
#undef TARGET_VECTOR_MODE_SUPPORTED_P
#define TARGET_VECTOR_MODE_SUPPORTED_P arc_vector_mode_supported_p
diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md
index 091f1092bed..5610bab694c 100644
--- a/gcc/config/arc/arc.md
+++ b/gcc/config/arc/arc.md
@@ -82,6 +82,7 @@
(include ("arc700.md"))
(include ("arcEM.md"))
(include ("arcHS.md"))
+(include ("arcHS4x.md"))
;; Predicates
@@ -204,7 +205,7 @@
simd_vcompare, simd_vpermute, simd_vpack, simd_vpack_with_acc,
simd_valign, simd_valign_with_acc, simd_vcontrol,
Re: [PATCH] relax -Wsizeof-pointer-memaccess for strncpy with size of source (PR 85931)
On Wed, Jun 13, 2018 at 10:51:41AM +0200, Richard Biener wrote: > Your concern is for compile-time, not for correctness, right? Yes. > I think that > > /* If ARG0 and ARG1 are the same SAVE_EXPR, they are necessarily equal. > We don't care about side effects in that case because the SAVE_EXPR > takes care of that for us. In all other cases, two expressions are > equal if they have no side effects. If we have two identical > expressions with side effects that should be treated the same due > to the only side effects being identical SAVE_EXPR's, that will > be detected in the recursive calls below. > If we are taking an invariant address of two identical objects > they are necessarily equal as well. */ > if (arg0 == arg1 && ! (flags & OEP_ONLY_CONST) > && (TREE_CODE (arg0) == SAVE_EXPR > || (flags & OEP_MATCH_SIDE_EFFECTS) > || (! TREE_SIDE_EFFECTS (arg0) && ! TREE_SIDE_EFFECTS (arg1 > return 1; > > is supposed to handle == SAVE_EXPRs and thus the hunk should be > conditionalized on arg0 != arg1 or rather for OEP_LEXICOGRAPHIC > always return true for arg0 == arg1? The above handles the arg0 == arg1 case, sure, but that is not what is compared in the -Wduplicated-branches testcases I've posted. There we have different SAVE_EXPRs (one set for the then branch, another for the else branch) which hash the same and we call operand_equal_p on it then, but without some sort of caching (both in the inchash::add_expr SAVE_EXPR case and operand_equal_p OEP_LEXICOGRAPHIC arg0 != arg1 SAVE_EXPR case) we will check the same thing again and again. Jakub
Re: [PATCH 5/6] [ARC] Update fma expansions.
Done. Thank you,
Claudiu
On Tue, Jun 12, 2018 at 10:32 PM, Andrew Burgess
wrote:
> * Claudiu Zissulescu [2018-05-21 13:18:38 +0300]:
>
>> Make sure only one operand has an immediate.
>>
>> OK to apply?
>
> Looks good.
>
> Thanks,
> Andrew
>
>> Claudiu
>>
>> gcc/
>> 2018-03-21 Claudiu Zissulescu
>>
>> * config/arc/fpu.md (fmasf4): Force operand to register.
>> (fnmasf4): Likewise.
>>
>> gcc/testsuite
>> 2018-03-21 Claudiu Zissulescu
>>
>> * gcc.target/arc/fma-1.c: New test.
>> ---
>> gcc/config/arc/fpu.md| 6 ++
>> gcc/testsuite/gcc.target/arc/fma-1.c | 13 +
>> 2 files changed, 19 insertions(+)
>> create mode 100644 gcc/testsuite/gcc.target/arc/fma-1.c
>>
>> diff --git a/gcc/config/arc/fpu.md b/gcc/config/arc/fpu.md
>> index de876cac0c1..9457922667e 100644
>> --- a/gcc/config/arc/fpu.md
>> +++ b/gcc/config/arc/fpu.md
>> @@ -64,6 +64,9 @@
>> tmp = gen_rtx_REG (SFmode, ACCL_REGNO);
>> emit_move_insn (tmp, operands[3]);
>> operands[3] = tmp;
>> + if (!register_operand (operands[1], SFmode)
>> +&& !register_operand (operands[2], SFmode))
>> + operands[2] = force_reg (SFmode, operands[2]);
>> }")
>>
>> (define_expand "fnmasf4"
>> @@ -77,6 +80,9 @@
>> tmp = gen_rtx_REG (SFmode, ACCL_REGNO);
>> emit_move_insn (tmp, operands[3]);
>> operands[3] = tmp;
>> + if (!register_operand (operands[1], SFmode)
>> +&& !register_operand (operands[2], SFmode))
>> + operands[2] = force_reg (SFmode, operands[2]);
>> }")
>>
>> (define_insn "fmasf4_fpu"
>> diff --git a/gcc/testsuite/gcc.target/arc/fma-1.c
>> b/gcc/testsuite/gcc.target/arc/fma-1.c
>> new file mode 100644
>> index 000..c195ad98127
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arc/fma-1.c
>> @@ -0,0 +1,13 @@
>> +/* { dg-do compile } */
>> +/* { dg-skip-if "FPU not available" { arc700 || arc6xx } } */
>> +/* { dg-options "-s -std=gnu11 -O2 -frounding-math -mfpu=fpus_all" } */
>> +
>> +const float a, b = 7.8539818525e01;
>> +
>> +/* Check if the fma operation is generated correctly. */
>> +
>> +int foo (void)
>> +{
>> + return (float)3.0 * b + a;
>> +}
>> +/* { dg-final { scan-assembler "fsmadd" } } */
>> --
>> 2.14.3
>>
Re: [PATCH] relax -Wsizeof-pointer-memaccess for strncpy with size of source (PR 85931)
On Wed, 13 Jun 2018, Jakub Jelinek wrote:
> On Wed, Jun 13, 2018 at 10:22:29AM +0200, Jakub Jelinek wrote:
> > Random testcase for -Wduplicated-branches -fsanitize=shift:
> > int
> > foo (int x, int y)
> > {
> > if (x)
> > y = y << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1
> > >> 1 << 1 >> 1
> > << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1
> > >> 1 << 1 >> 1
> > << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1
> > >> 1 << 1 >> 1
> > << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1
> > >> 1 << 1 >> 1;
> > else
> > y = y << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1
> > >> 1 << 1 >> 1
> > << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1
> > >> 1 << 1 >> 1
> > << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1
> > >> 1 << 1 >> 1
> > << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1
> > >> 1 << 1 >> 1;
> > return y;
> > }
> >
> > Though it seems we have that problem already in inchash::add_expr. In that
> > case perhaps we could have a pointer to a hashmap in inchash::hash objects,
> > clear it in the ctors and destroy/clear in inchash::hash::end (), though we
> > have the add_commutative that has two separate hash objects.
>
> It isn't specific to just -fsanitize=undefined, even without that there are
> cases we can end up with lots of nested SAVE_EXPRs, like
> -Wduplicated-branches:
> int bar (void);
>
> void
> foo (int x, int *y)
> {
> if (x)
> y[0] += (y[1] += (y[2] += (y[3] += (y[4] += (y[5] += (y[6] += (y[7] +=
> (y[8] +=
> (y[9] += (y[10] += (y[11] += (y[12] += (y[13] += (y[14] += (y[15]
> += (y[16] +=
> (y[17] += (y[18] += (y[19] += (y[20] += (y[21] += (y[22] += (y[23]
> += (y[24] +=
> (y[25] += (y[26] += (y[27] += (y[28] += (y[29] += (y[30] += (y[31]
> += (y[32] +=
> (y[33] += (y[34] += (y[35] += (y[36] += (y[37] += (y[38] += (y[39]
> += (y[40] +=
> (y[41] += (y[42] += (y[43] += (y[44] += (y[45] += (y[46] += (y[47]
> += (y[48] +=
> (y[49] += (y[50] += (y[51] += (y[52] += (y[53] += (y[54] += (y[55]
> += (y[56] +=
> (y[57] += (y[58] += (y[59] += (y[60] += (y[61] += (y[62] += (y[63]
> += (y[64] += bar ()
> ;
> else
> y[0] += (y[1] += (y[2] += (y[3] += (y[4] += (y[5] += (y[6] += (y[7] +=
> (y[8] +=
> (y[9] += (y[10] += (y[11] += (y[12] += (y[13] += (y[14] += (y[15]
> += (y[16] +=
> (y[17] += (y[18] += (y[19] += (y[20] += (y[21] += (y[22] += (y[23]
> += (y[24] +=
> (y[25] += (y[26] += (y[27] += (y[28] += (y[29] += (y[30] += (y[31]
> += (y[32] +=
> (y[33] += (y[34] += (y[35] += (y[36] += (y[37] += (y[38] += (y[39]
> += (y[40] +=
> (y[41] += (y[42] += (y[43] += (y[44] += (y[45] += (y[46] += (y[47]
> += (y[48] +=
> (y[49] += (y[50] += (y[51] += (y[52] += (y[53] += (y[54] += (y[55]
> += (y[56] +=
> (y[57] += (y[58] += (y[59] += (y[60] += (y[61] += (y[62] += (y[63]
> += (y[64] += bar ()
> ;
> }
Your concern is for compile-time, not for correctness, right?
I think that
/* If ARG0 and ARG1 are the same SAVE_EXPR, they are necessarily equal.
We don't care about side effects in that case because the SAVE_EXPR
takes care of that for us. In all other cases, two expressions are
equal if they have no side effects. If we have two identical
expressions with side effects that should be treated the same due
to the only side effects being identical SAVE_EXPR's, that will
be detected in the recursive calls below.
If we are taking an invariant address of two identical objects
they are necessarily equal as well. */
if (arg0 == arg1 && ! (flags & OEP_ONLY_CONST)
&& (TREE_CODE (arg0) == SAVE_EXPR
|| (flags & OEP_MATCH_SIDE_EFFECTS)
|| (! TREE_SIDE_EFFECTS (arg0) && ! TREE_SIDE_EFFECTS (arg1
return 1;
is supposed to handle == SAVE_EXPRs and thus the hunk should be
conditionalized on arg0 != arg1 or rather for OEP_LEXICOGRAPHIC
always return true for arg0 == arg1?
I also wondered why this change was necessary for the fix but
it looked safe correctness-wise.
Richard.
Re: [PATCH] relax -Wsizeof-pointer-memaccess for strncpy with size of source (PR 85931)
On Wed, Jun 13, 2018 at 10:22:29AM +0200, Jakub Jelinek wrote:
> Random testcase for -Wduplicated-branches -fsanitize=shift:
> int
> foo (int x, int y)
> {
> if (x)
> y = y << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >>
> 1 << 1 >> 1
> << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >>
> 1 << 1 >> 1
> << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >>
> 1 << 1 >> 1
> << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >>
> 1 << 1 >> 1;
> else
> y = y << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >>
> 1 << 1 >> 1
> << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >>
> 1 << 1 >> 1
> << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >>
> 1 << 1 >> 1
> << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >>
> 1 << 1 >> 1;
> return y;
> }
>
> Though it seems we have that problem already in inchash::add_expr. In that
> case perhaps we could have a pointer to a hashmap in inchash::hash objects,
> clear it in the ctors and destroy/clear in inchash::hash::end (), though we
> have the add_commutative that has two separate hash objects.
It isn't specific to just -fsanitize=undefined, even without that there are
cases we can end up with lots of nested SAVE_EXPRs, like
-Wduplicated-branches:
int bar (void);
void
foo (int x, int *y)
{
if (x)
y[0] += (y[1] += (y[2] += (y[3] += (y[4] += (y[5] += (y[6] += (y[7] +=
(y[8] +=
(y[9] += (y[10] += (y[11] += (y[12] += (y[13] += (y[14] += (y[15]
+= (y[16] +=
(y[17] += (y[18] += (y[19] += (y[20] += (y[21] += (y[22] += (y[23]
+= (y[24] +=
(y[25] += (y[26] += (y[27] += (y[28] += (y[29] += (y[30] += (y[31]
+= (y[32] +=
(y[33] += (y[34] += (y[35] += (y[36] += (y[37] += (y[38] += (y[39]
+= (y[40] +=
(y[41] += (y[42] += (y[43] += (y[44] += (y[45] += (y[46] += (y[47]
+= (y[48] +=
(y[49] += (y[50] += (y[51] += (y[52] += (y[53] += (y[54] += (y[55]
+= (y[56] +=
(y[57] += (y[58] += (y[59] += (y[60] += (y[61] += (y[62] += (y[63]
+= (y[64] += bar ()
;
else
y[0] += (y[1] += (y[2] += (y[3] += (y[4] += (y[5] += (y[6] += (y[7] +=
(y[8] +=
(y[9] += (y[10] += (y[11] += (y[12] += (y[13] += (y[14] += (y[15]
+= (y[16] +=
(y[17] += (y[18] += (y[19] += (y[20] += (y[21] += (y[22] += (y[23]
+= (y[24] +=
(y[25] += (y[26] += (y[27] += (y[28] += (y[29] += (y[30] += (y[31]
+= (y[32] +=
(y[33] += (y[34] += (y[35] += (y[36] += (y[37] += (y[38] += (y[39]
+= (y[40] +=
(y[41] += (y[42] += (y[43] += (y[44] += (y[45] += (y[46] += (y[47]
+= (y[48] +=
(y[49] += (y[50] += (y[51] += (y[52] += (y[53] += (y[54] += (y[55]
+= (y[56] +=
(y[57] += (y[58] += (y[59] += (y[60] += (y[61] += (y[62] += (y[63]
+= (y[64] += bar ()
;
}
Jakub
RE: [PATCH 6/6] [ARC] Reimplement return padding operation for ARC700.
I've included and I updated the patch with your suggestions.
Thank you,
Claudiu
From: Andrew Burgess [andrew.burg...@embecosm.com]
Sent: Tuesday, June 12, 2018 10:10 PM
To: Claudiu Zissulescu
Cc: gcc-patches@gcc.gnu.org; francois.bed...@synopsys.com;
claz...@synopsys.comq; Claudiu Zissulescu
Subject: Re: [PATCH 6/6] [ARC] Reimplement return padding operation for ARC700.
* Claudiu Zissulescu [2018-05-21 13:18:39 +0300]:
> From: Claudiu Zissulescu
>
> For ARC700, adding padding if necessary to avoid a mispredict. A
> return could happen immediately after the function start. A
> call/return and return/return must be 6 bytes apart to avoid
> mispredict.
>
> The old implementation was doing this operation very late in the
> compilation process, and the additional nop instructions and/or
> forcing some other instruction to take their long form was not taken
> into account when generating brcc instructions. Thus, wrong code could
> be generated.
>
> Ok to apply?
This looks fine with a couple of small adjustments, inline below...
> Claudiu
>
> gcc/
> 2017-03-24 Claudiu Zissulescu
>
> * config/arc/arc-protos.h (arc_pad_return): Remove.
> * config/arc/arc.c (machine_function): Remove force_short_suffix
> and size_reason.
> (arc_print_operand): Adjust printing of '&'.
> (arc_verify_short): Remove conditional printing of short suffix.
> (arc_final_prescan_insn): Remove reference to size_reason.
> (pad_return): New function.
> (arc_reorg): Call pad_return.
> (arc_pad_return): Remove.
> (arc_init_machine_status): Remove reference to force_short_suffix.
> * config/arc/arc.md (vunspec): Add VUNSPEC_ARC_BLOCKAGE.
> (attr length): When attribute iscompact is true force to 2
> regardless; in the case of maybe check if we want to force the
> instruction to have 4 bytes length.
> (nopv): Change it to generate 4 byte long nop as well.
> (blockage): New pattern.
> (simple_return): Remove call to arc_pad_return.
> (p_return_i): Likewise.
>
> gcc/testsuite/
> 2017-03-24 Claudiu Zissulescu
>
> * gcc.target/arc/pr9001107555.c: New file.
> ---
> gcc/config/arc/arc-protos.h | 1 -
> gcc/config/arc/arc.c| 156
> +---
> gcc/config/arc/arc.md | 26 +++--
> gcc/testsuite/gcc.target/arc/pr9001107555.c | 38 +++
> 4 files changed, 128 insertions(+), 93 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/arc/pr9001107555.c
>
> diff --git a/gcc/config/arc/arc-protos.h b/gcc/config/arc/arc-protos.h
> index 67f3b4e3226..ce4b6f84749 100644
> --- a/gcc/config/arc/arc-protos.h
> +++ b/gcc/config/arc/arc-protos.h
> @@ -89,7 +89,6 @@ extern void arc_clear_unalign (void);
> extern void arc_toggle_unalign (void);
> extern void split_addsi (rtx *);
> extern void split_subsi (rtx *);
> -extern void arc_pad_return (void);
> extern void arc_split_move (rtx *);
> extern const char *arc_short_long (rtx_insn *insn, const char *, const char
> *);
> extern rtx arc_regno_use_in (unsigned int, rtx);
> diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
> index b1a09d82b72..22f1442a027 100644
> --- a/gcc/config/arc/arc.c
> +++ b/gcc/config/arc/arc.c
> @@ -2648,8 +2648,6 @@ typedef struct GTY (()) machine_function
>struct arc_frame_info frame_info;
>/* To keep track of unalignment caused by short insns. */
>int unalign;
> - int force_short_suffix; /* Used when disgorging return delay slot insns.
> */
> - const char *size_reason;
>struct arc_ccfsm ccfsm_current;
>/* Map from uid to ccfsm state during branch shortening. */
>rtx ccfsm_current_insn;
> @@ -4307,7 +4305,7 @@ arc_print_operand (FILE *file, rtx x, int code)
> }
>break;
> case '&':
> - if (TARGET_ANNOTATE_ALIGN && cfun->machine->size_reason)
> + if (TARGET_ANNOTATE_ALIGN)
> fprintf (file, "; unalign: %d", cfun->machine->unalign);
>return;
> case '+':
> @@ -4980,7 +4978,6 @@ static int
> arc_verify_short (rtx_insn *insn, int, int check_attr)
> {
>enum attr_iscompact iscompact;
> - struct machine_function *machine;
>
>if (check_attr > 0)
> {
> @@ -4988,10 +4985,6 @@ arc_verify_short (rtx_insn *insn, int, int check_attr)
>if (iscompact == ISCOMPACT_FALSE)
> return 0;
> }
> - machine = cfun->machine;
> -
> - if (machine->force_short_suffix >= 0)
> -return machine->force_short_suffix;
>
>return (get_attr_length (insn) & 2) != 0;
> }
> @@ -5030,8 +5023,6 @@ arc_final_prescan_insn (rtx_insn *insn, rtx *opvec
> ATTRIBUTE_UNUSED,
>cfun->machine->prescan_initialized = 1;
> }
>arc_ccfsm_advance (insn, _ccfsm_current);
> -
> - cfun->machine->size_reason = 0;
> }
>
> /* Given FROM and TO register numbers, say whether this elimination is
> allowed.
> @@ -7673,6 +7664,76 @@ jli_call_scan (void)
RE: [PATCH 3/6] [ARC] SYNC instruction is valid on all ARC cores.
Committed. Thank you for your review, Claudiu From: Andrew Burgess [andrew.burg...@embecosm.com] Sent: Tuesday, June 12, 2018 9:31 PM To: Claudiu Zissulescu Cc: gcc-patches@gcc.gnu.org; francois.bed...@synopsys.com; claz...@synopsys.comq; claziss Subject: Re: [PATCH 3/6] [ARC] SYNC instruction is valid on all ARC cores. * Claudiu Zissulescu [2018-05-21 13:18:36 +0300]: > From: claziss > > The sync instruction is part of all ARC architectures. Fix this in the > compiler. > > Ok to apply? Looks good. Thanks, Andrew > Claudiu > > gcc/ > 2017-05-03 Claudiu Zissulescu > > * config/arc/builtins.def (SYNC): SYNC instruction is valid on all > ARC cores. > --- > gcc/config/arc/builtins.def | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gcc/config/arc/builtins.def b/gcc/config/arc/builtins.def > index 349f39fab23..5171ab40842 100644 > --- a/gcc/config/arc/builtins.def > +++ b/gcc/config/arc/builtins.def > @@ -34,7 +34,7 @@ > /* Special builtins. */ > DEF_BUILTIN (NOP,0, void_ftype_void,nothing, 1) > DEF_BUILTIN (RTIE, 0, void_ftype_void,rtie,1) > -DEF_BUILTIN (SYNC, 0, void_ftype_void,sync, > TARGET_ARC700) > +DEF_BUILTIN (SYNC, 0, void_ftype_void,sync,1) > DEF_BUILTIN (BRK,0, void_ftype_void,brk, 1) > DEF_BUILTIN (SWI,0, void_ftype_void,swi, 1) > DEF_BUILTIN (UNIMP_S,0, void_ftype_void,unimp_s, > !TARGET_ARC600_FAMILY) > -- > 2.14.3 >
RE: [PATCH 2/6] [ARC] Allow EX instruction for ARC700 and ARCv2.
Committed. Thank you for your review,
Claudiu
From: Andrew Burgess [andrew.burg...@embecosm.com]
Sent: Tuesday, June 12, 2018 9:31 PM
To: Claudiu Zissulescu
Cc: gcc-patches@gcc.gnu.org; francois.bed...@synopsys.com;
claz...@synopsys.comq; claziss
Subject: Re: [PATCH 2/6] [ARC] Allow EX instruction for ARC700 and ARCv2.
* Claudiu Zissulescu [2018-05-21 13:18:35 +0300]:
> From: claziss
>
> The EX instruction is base line for both architectures. Reflect this in the
> compiler.
>
> OK to apply?
Looks good.
Thanks,
Andrew
> Claudiu
>
> gcc/
> 2017-05-02 Claudiu Zissulescu
>
> * config/arc/arc.c (atomic_exchangesi): EX instruction is default
> for ARC700 and ARCv2.
> ---
> gcc/config/arc/atomic.md | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/config/arc/atomic.md b/gcc/config/arc/atomic.md
> index 13a2e34549c..4a56ca229cc 100644
> --- a/gcc/config/arc/atomic.md
> +++ b/gcc/config/arc/atomic.md
> @@ -120,7 +120,7 @@
> (match_operand:SI 1 "mem_noofs_operand" "")
> (match_operand:SI 2 "register_operand" "")
> (match_operand:SI 3 "const_int_operand" "")]
> - "TARGET_ATOMIC"
> + "TARGET_ARC700 || TARGET_V2"
> {
>enum memmodel model = (enum memmodel) INTVAL (operands[3]);
>
> --
> 2.14.3
>
RE: [PATCH 4/6] [ARC] Define LINK_GCC_C_SEQUENCE_SPEC.
Done. Thank you for your review,
Claudiu
From: Andrew Burgess [andrew.burg...@embecosm.com]
Sent: Tuesday, June 12, 2018 9:32 PM
To: Claudiu Zissulescu
Cc: gcc-patches@gcc.gnu.org; francois.bed...@synopsys.com;
claz...@synopsys.comq; claziss
Subject: Re: [PATCH 4/6] [ARC] Define LINK_GCC_C_SEQUENCE_SPEC.
* Claudiu Zissulescu [2018-05-21 13:18:37 +0300]:
> From: claziss
>
> If no specs file is provided, default to nosys library.
>
> Ok to apply?
Looks good.
Thanks,
Andrew
> Claudiu
>
> gcc/
> 2017-05-03 Claudiu Zissulescu
>
> * config/arc/elf.h (LINK_GCC_C_SEQUENCE_SPEC): Define.
> ---
> gcc/config/arc/elf.h | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/gcc/config/arc/elf.h b/gcc/config/arc/elf.h
> index 43ced3b720f..66ee5b698ea 100644
> --- a/gcc/config/arc/elf.h
> +++ b/gcc/config/arc/elf.h
> @@ -73,3 +73,8 @@ along with GCC; see the file COPYING3. If not see
>
> #undef TARGET_ASM_FILE_END
> #define TARGET_ASM_FILE_END arc_file_end
> +
> +/* If no specs file is enforced, default to nosys libarary. */
> +#undef LINK_GCC_C_SEQUENCE_SPEC
> +#define LINK_GCC_C_SEQUENCE_SPEC \
> + "--start-group %G %{!specs=*:-lc -lnosys} --end-group"
> --
> 2.14.3
>
Re: [PATCH] relax -Wsizeof-pointer-memaccess for strncpy with size of source (PR 85931)
On Wed, Jun 13, 2018 at 09:58:49AM +0200, Jakub Jelinek wrote:
> I'm actually worried about the fold-const.c change and don't understand, why
> it has been done at all in the context of this PR.
>
> case SAVE_EXPR:
> if (flags & OEP_LEXICOGRAPHIC)
> return OP_SAME (0);
> return 0;
>
> means it does something different from the state before the patch:
> default:
> return 0;
> only if OEP_LEXICOGRAPHIC is set, which AFAIK is only in the
> -Wduplicated-branches warning code, so why it is needed for
> -Wstringop-truncation warning is unclear to me. More importantly,
> especially -fsanitize=undefined has a tendency of creating trees
> which contain two or more uses of the same SAVE_EXPR at each level, and very
> deep nesting levels of such SAVE_EXPRs, so if we handle it without checking
> for duplicates, it results in exponential compile time complexity.
> So, if we really want to handle SAVE_EXPR here, we'd need to create some
> cache where we'd remember if in the same toplevel operand_equal_p call we've
> already compared two particular SAVE_EXPRs and what was the result.
Random testcase for -Wduplicated-branches -fsanitize=shift:
int
foo (int x, int y)
{
if (x)
y = y << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
<< 1 >> 1
<< 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
<< 1 >> 1
<< 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
<< 1 >> 1
<< 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
<< 1 >> 1;
else
y = y << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
<< 1 >> 1
<< 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
<< 1 >> 1
<< 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
<< 1 >> 1
<< 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1 << 1 >> 1
<< 1 >> 1;
return y;
}
Though it seems we have that problem already in inchash::add_expr. In that
case perhaps we could have a pointer to a hashmap in inchash::hash objects,
clear it in the ctors and destroy/clear in inchash::hash::end (), though we
have the add_commutative that has two separate hash objects.
Jakub
RE: [PATCH][Aarch64] v2: Arithmetic overflow subv patterns [Patch 3/4]
Updated previous patch: https://gcc.gnu.org/ml/gcc-patches/2018-06/msg00508.html With coding style feedback from Richard Sandiford: (that also apply to this patch) https://gcc.gnu.org/ml/gcc-patches/2018-06/msg00508.html Bootstrapped and tested on aarch64-linux-gnu. Okay for trunk? 2018-05-31 Michael Collison Richard Henderson * config/aarch64/aarch64.md (subv4, usubv4): New patterns. (subti): Handle op1 zero. (subvti4, usub4ti4): New. (*sub3_compare1_imm): New. (sub3_carryinCV): New. (*sub3_carryinCV_z1_z2, *sub3_carryinCV_z1): New. (*sub3_carryinCV_z2, *sub3_carryinCV): New. gnutools-6308-pt3.patch Description: gnutools-6308-pt3.patch
Re: [PATCH] relax -Wsizeof-pointer-memaccess for strncpy with size of source (PR 85931)
On Wed, Jun 13, 2018 at 09:46:28AM +0200, Richard Biener wrote: > > > > gcc/ChangeLog: > > > > > > > > PR c/85931 > > > > * fold-const.c (operand_equal_p): Handle SAVE_EXPR. > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > PR c/85931 > > > > * gcc.dg/Wstringop-truncation-3.c: New test. > > > OK for the trunk. Richi and Jakub have the final say for the branch. > > > > > > I'm a bit surprised that you don't just use operand_equal_p for both the > > > INTEGER_CST and !INTEGER_CST cases when testing dstsz == srcsz > > > > It seemed that since the CST case compared the values of the size > > expressions and the other case their lexicographic form they needed > > to be different. But operand_equal_p() does value comparison for > > constants so the conditional can be simplified by using just it > > for both cases. Thanks for the suggestion! I've made that > > simplification and committed r261515. > > > > Jakub/Richard, can you please review the commit and let me know > > if you have any concerns with backporting it to the GCC 8 branch? > > (I will proceed if I don't hear anything this week.) > > I'm fine with backporting it. I'm actually worried about the fold-const.c change and don't understand, why it has been done at all in the context of this PR. case SAVE_EXPR: if (flags & OEP_LEXICOGRAPHIC) return OP_SAME (0); return 0; means it does something different from the state before the patch: default: return 0; only if OEP_LEXICOGRAPHIC is set, which AFAIK is only in the -Wduplicated-branches warning code, so why it is needed for -Wstringop-truncation warning is unclear to me. More importantly, especially -fsanitize=undefined has a tendency of creating trees which contain two or more uses of the same SAVE_EXPR at each level, and very deep nesting levels of such SAVE_EXPRs, so if we handle it without checking for duplicates, it results in exponential compile time complexity. So, if we really want to handle SAVE_EXPR here, we'd need to create some cache where we'd remember if in the same toplevel operand_equal_p call we've already compared two particular SAVE_EXPRs and what was the result. Jakub
RE: [PATCH][Aarch64] v2: Arithmetic overflow addv patterns [Patch 2/4]
Updated with Richard's style and mismatched mode comments.
Okay for trunk?
-Original Message-
From: Richard Sandiford
Sent: Monday, June 11, 2018 11:47 AM
To: Michael Collison
Cc: James Greenhalgh ; GCC Patches
; nd
Subject: Re: [PATCH][Aarch64] v2: Arithmetic overflow addv patterns [Patch 2/4]
Michael Collison writes:
> +(define_expand "uaddv4"
> + [(match_operand:GPI 0 "register_operand")
> + (match_operand:GPI 1 "register_operand")
> + (match_operand:GPI 2 "register_operand")
> + (label_ref (match_operand 3 "" ""))]
> + ""
> +{
> + emit_insn (gen_add3_compareC (operands[0], operands[1],
> +operands[2]));
> + aarch64_gen_unlikely_cbranch (NE, CC_Cmode, operands[3]);
> +
> + DONE;
> +})
> +
> +
Nit: stray extra line.
> (define_expand "addti3"
>[(set (match_operand:TI 0 "register_operand" "")
> (plus:TI (match_operand:TI 1 "register_operand" "")
> - (match_operand:TI 2 "register_operand" "")))]
> + (match_operand:TI 2 "aarch64_reg_or_imm" "")))]
>""
> {
> - rtx low = gen_reg_rtx (DImode);
> - emit_insn (gen_adddi3_compareC (low, gen_lowpart (DImode, operands[1]),
> - gen_lowpart (DImode, operands[2])));
> + rtx low_dest,op1_low,op2_low,high_dest,op1_high,op2_high;
Spaces after commas (sorry)
[...]
> @@ -1837,10 +1946,70 @@
>[(set_attr "type" "alus_sreg")]
> )
>
> +;; Note that since we're sign-extending, match the immediate in GPI
> +;; rather than in DWI. Since CONST_INT is modeless, this works fine.
> +(define_insn "*add3_compareV_cconly_imm"
> + [(set (reg:CC_V CC_REGNUM)
> + (compare:CC_V
> + (plus:
> + (sign_extend: (match_operand:GPI 0 "register_operand" "r,r"))
> + (match_operand:GPI 1 "aarch64_plus_immediate" "I,J"))
> + (sign_extend: (plus:GPI (match_dup 0) (match_dup 1)]
Real reason for replying is: this is a neat trick, but I think it's only an
accident that genrecog doesn't reject the mode on operand 1.
PLUSes can't have mismatched modes since since rtl doesn't have the sign
information to do a conversion.
IMO we should use a similar structure to the zero_extends:
(plus:
(zero_extend: (match_operand:GPI 0 "register_operand" "r,r"))
(match_operand: 1 "const_scalar_int_operand" ""))
(sign_extend:
(plus:GPI
(match_dup 0)
(match_operand:GPI 2 "aarch64_plus_immediate" "I,J")
but with the simpler check:
INTVAL (operands[1]) == INTVAL (operands[2])
Thanks,
Richard
gnutools-6308-pt2.patch
Description: gnutools-6308-pt2.patch
Re: [PATCH] relax -Wsizeof-pointer-memaccess for strncpy with size of source (PR 85931)
On Tue, 12 Jun 2018, Martin Sebor wrote: > On 06/11/2018 03:57 PM, Jeff Law wrote: > > On 05/29/2018 08:57 PM, Martin Sebor wrote: > > > Warning for a strncpy call whose bound is the same as the size > > > of the source and suggesting to use the size of the source is > > > less than helpful when both sizes are the same, as in: > > > > > > char a[4], b[4]; > > > strncpy (a, b, sizeof b); > > > > > > The attached patch suppresses the -Wsizeof-pointer-memaccess > > > warning for these cases. To do that even for VLAs (in some > > > cases), the patch enhances operand_equal_p() to handle > > > SAVE_EXPR to detect when VLA in sizeof VLA refers to the size > > > of a variable-length array. > > > > > > Is this okay for trunk and GCC 8? > > > > > > Martin > > > > > > gcc-85931.diff > > > > > > > > > PR c/85931 - -Wsizeof-pointer-memaccess for strncpy with size of source > > > > > > gcc/c-family/ChangeLog: > > > > > > PR c/85931 > > > * c-warn.c (sizeof_pointer_memaccess_warning): Avoid warning when > > > sizeof source and destination yields the same value. > > > > > > gcc/ChangeLog: > > > > > > PR c/85931 > > > * fold-const.c (operand_equal_p): Handle SAVE_EXPR. > > > > > > gcc/testsuite/ChangeLog: > > > > > > PR c/85931 > > > * gcc.dg/Wstringop-truncation-3.c: New test. > > OK for the trunk. Richi and Jakub have the final say for the branch. > > > > I'm a bit surprised that you don't just use operand_equal_p for both the > > INTEGER_CST and !INTEGER_CST cases when testing dstsz == srcsz > > It seemed that since the CST case compared the values of the size > expressions and the other case their lexicographic form they needed > to be different. But operand_equal_p() does value comparison for > constants so the conditional can be simplified by using just it > for both cases. Thanks for the suggestion! I've made that > simplification and committed r261515. > > Jakub/Richard, can you please review the commit and let me know > if you have any concerns with backporting it to the GCC 8 branch? > (I will proceed if I don't hear anything this week.) I'm fine with backporting it. Richard.
Re: [PATCH][PR sanitizer/86090] Add checks for lstat and readlink to sanitizer configure.
Hi, On 06/10/2018 09:49 PM, Jakub Jelinek wrote: > On Fri, Jun 08, 2018 at 08:10:40PM +0300, Denis Khalikov wrote: >> From: Denis Khalikov >> Date: Fri, 8 Jun 2018 19:53:01 +0300 >> Subject: [PATCH] PR sanitizer/86090 >> >> * configure.ac: Check for lstat and readlink. >> * configure, config.h.in: Rebuild. > Ok for trunk. Will somebody commit it for you or should I? sorry for the delay, we had public holidays at the beginning of this week. I'll commit this today. -Maxim > > Jakub > > >
Re: [PATCH,MIPS] Fix pr86067 ICE: scal-to-vec1.c:86:1: error: unrecognizable insn with -march=loongson3a
Thanks for your comments, commited as r261538.
Paul Hua
On Tue, Jun 12, 2018 at 8:09 PM, Matthew Fortune wrote:
> Paul Hua writes:
>> The gcc.c-torture/execute/scal-to-vec1.c trigger a gcc ICE bug.
>>
>> It's a mistake in define_expand vec_setv4hi in loongson.md file.
>>
>> 375 (define_expand "vec_setv4hi"
>> 376 [(set (match_operand:V4HI 0 "register_operand" "=f")
>> 377 (unspec:V4HI [(match_operand:V4HI 1 "register_operand" "f")
>> 378 (match_operand:HI 2 "register_operand" "f")
>> 379 (match_operand:SI 3 "const_0_to_3_operand"
>> "")]
>> 380 UNSPEC_LOONGSON_PINSRH))]
>> 381 "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
>> 382 {
>> 383 rtx ext = gen_reg_rtx (SImode);
>> 384 emit_move_insn (ext, gen_lowpart (SImode, operands[1]));
>> 385 operands[1] = ext;
>> 386 })
>>
>> The line 384 gen_lowpart the operands[1], should be gen_lowpart
>> operands[2], cause the operands[2] are HImode.
>>
>>
>> The attached patch fixed this bug.
>>
>> Bootstrapped and reg-tested on mips64el-unknown-linux-gnu.
>> Ok for commit ?
>>
>>
>> ---
>
> Hi Paul,
>
> This looks good, just one issue with the changelog entry. The entry
> would go in the gcc/ChangeLog file and the path is then relative to
> the gcc/ directory. The PR should be referenced as target/:
>
> 2018-03-24 Chenghua Xu
>
> PR target/86076
> * config/mips/loongson.md (vec_setv4hi): Gen_lowpart for
> operands[2] instead of operands[1].
>
> OK to commit, thanks for finding and fixing. This has been broken
> since 2011!
>
> Matthew
>
Re: [RFC PATCH 1/6] vxworks: add target/h/wrn/coreip to the set of system include paths
On 2018-06-12 23:34, Olivier Hainque wrote: > > >> On 12 Jun 2018, at 23:12, Jeff Law wrote: >> >> I'm going to defer to Olivier on all 6 parts of this kit. He knows >> vxworks far better than I. > > No pb. I haven't forgotten about this :) > > As I mentioned on another > thread, exceptions on VxWorks is an intricate nest of > multiple parameters when you consider the possible variations > of kinds of modules you can make (RTP, DKMs, SKMs) combined > with different lines of VxWorks versions (6.7 introduced support > for dwarf eh for example), combined with the various context > in which a toolchain may be used (straight from a command line > or driven by the WindRiver IDEs). > > This include path part alone looks sensible. I'd still like to > evaluate how it fares in a environment where many of there variations > are exercised, and I'm not quite done with this yet. Thanks, I do hope for careful scrutiny. As I said, I'm only working on a VxWorks 5.5 powerpc target, and I obviously don't want to break stuff for other settings, but I can't really test on other combinations than the one I have. I try to break up the changes in small pieces, maybe even smaller than one would usually do, to hopefully get the uncontroversial parts upstream, and then we may just have to maintain some patches in-house if there's no reasonable way to make it work for everybody. Rasmus
