Re: Deque code cleanup and optimizations
On 5/10/19 3:38 PM, Jonathan Wakely wrote: This seems generally OK, but ... On Fri, 10 May 2019, 05:59 François Dumont wrote: I remove several _M_map != nullptr checks cause in current implementation it can't be null. I have several patches following this one to support it but in this case we will be using a different code path. You can't remove those checks. If _M_map can ever be null now or in the future, then we need the checks. Otherwise code compiled today would break if passed a deque compiled with a future GCC that allows the map to be null. I'm curious how you plan to support it though, I don't think it's possible without an ABI break. No miracle, I plan to propose it only in versioned namespace mode, it does break ABI. But in this implementation those current checks are useless, I'll double check. (_Deque_base::_Deque_impl::_M_move_impl()): Remove _M_impl._M_map check. _M_move_impl and the constructor that calls it can be removed completely, because https://cplusplus.github.io/LWG/issue2593 means that the same allocator can still be used after moving from it. That function only exists to handle the case where an allocator changes value after being moved from. Good, I'll cleanup that and propose a new patch. François
std::vector code cleanup fixes optimizations
Hi This is the patch on vector to: - Optimize sizeof in Versioned namespace mode. We could go one step further by removing _M_p from _M_finish and just transform it into an offset but it is a little bit more impacting for the code. - Implement the swap optimization already done on main std::vector template class. - Fix move constructor so that it is noexcept no matter allocator move constructor noexcept qualification - Optimize move constructor with allocator when allocator type is always equal. - Use shortcuts in C++11 by skipping the _M_XXX_dispatch methods. Those are now defined only in pre-C++11 mode, I can't see any abi issue in doing so. * include/bits/stl_bvector.h [_GLIBCXX_INLINE_VERSION](_Bvector_impl_data::_M_start): Define as _Bit_type*. (_Bvector_impl_data(const _Bvector_impl_data&)): Default. (_Bvector_impl_data(_Bvector_impl_data&&)): Delegate to latter. (_Bvector_impl_data::operator=(const _Bvector_impl_data&)): Default. (_Bvector_impl_data::_M_move_data(_Bvector_impl_data&&)): Use latter. (_Bvector_impl_data::_M_reset()): Likewise. (_Bvector_impl_data::_M_swap_data): New. (_Bvector_impl::_Bvector_impl(_Bvector_impl&&)): Implement explicitely. (_Bvector_impl::_Bvector_impl(_Bit_alloc_type&&, _Bvector_impl&&)): New. (_Bvector_base::_Bvector_base(_Bvector_base&&, const allocator_type&)): New, use latter. (vector::vector(vector&&, const allocator_type&, true_type)): New, use latter. (vector::vector(vector&&, const allocator_type&, false_type)): New. (vector::vector(vector&&, const allocator_type&)): Use latters. (vector::vector(const vector&, const allocator_type&)): Adapt. [__cplusplus >= 201103](vector::vector(_InputIt, _InputIt, const allocator_type&)): Use _M_initialize_range. (vector::operator[](size_type)): Use iterator operator[]. (vector::operator[](size_type) const): Use const_iterator operator[]. (vector::swap(vector&)): Adapt. (vector::_M_initialize(size_type)): Add assertions on allocators. Use _M_swap_data. [__cplusplus >= 201103](vector::insert(const_iterator, _InputIt, _InputIt)): Use _M_insert_range. [__cplusplus >= 201103](vector::_M_initialize_dispatch): Remove. [__cplusplus >= 201103](vector::_M_insert_dispatch): Remove. * testsuite/23_containers/vector/bool/allocator/swap.cc: Adapt. * testsuite/23_containers/vector/bool/cons/noexcept_move_construct.cc: Add check. Tested under Linux x86_64, normal and debug modes. Ok to commit ? François diff --git a/libstdc++-v3/include/bits/stl_bvector.h b/libstdc++-v3/include/bits/stl_bvector.h index 280d40f60c5..d6fcc2d848d 100644 --- a/libstdc++-v3/include/bits/stl_bvector.h +++ b/libstdc++-v3/include/bits/stl_bvector.h @@ -441,7 +441,16 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER struct _Bvector_impl_data { +#if !_GLIBCXX_INLINE_VERSION _Bit_iterator _M_start; +#else + // We don't need the offset field for the start, it's always zero. + struct { + _Bit_type* _M_p; + // Allow assignment from iterators (assume offset is zero): + void operator=(_Bit_iterator __it) { _M_p = __it._M_p; } + } _M_start; +#endif _Bit_iterator _M_finish; _Bit_pointer _M_end_of_storage; @@ -450,33 +459,38 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER { } #if __cplusplus >= 201103L + _Bvector_impl_data(const _Bvector_impl_data&) = default; + _Bvector_impl_data& + operator=(const _Bvector_impl_data&) = default; + _Bvector_impl_data(_Bvector_impl_data&& __x) noexcept - : _M_start(__x._M_start), _M_finish(__x._M_finish) - , _M_end_of_storage(__x._M_end_of_storage) + : _Bvector_impl_data(__x) { __x._M_reset(); } void _M_move_data(_Bvector_impl_data&& __x) noexcept { - this->_M_start = __x._M_start; - this->_M_finish = __x._M_finish; - this->_M_end_of_storage = __x._M_end_of_storage; + *this = __x; __x._M_reset(); } #endif void _M_reset() _GLIBCXX_NOEXCEPT + { *this = _Bvector_impl_data(); } + + void + _M_swap_data(_Bvector_impl_data& __x) _GLIBCXX_NOEXCEPT { - _M_start = _M_finish = _Bit_iterator(); - _M_end_of_storage = _Bit_pointer(); + // Do not use std::swap(_M_start, __x._M_start), etc as it loses + // information used by TBAA. + std::swap(*this, __x); } }; struct _Bvector_impl : public _Bit_alloc_type, public _Bvector_impl_data { - public: _Bvector_impl() _GLIBCXX_NOEXCEPT_IF( is_nothrow_default_constructible<_Bit_alloc_type>::value) : _Bit_alloc_type() @@ -487,7 +501,15 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER { } #if __cplusplus >= 201103L - _Bvector_impl(_Bvector_impl&&) = default; + // Not defaulted, to enforce noexcept(true) even when + // !is_nothrow_move_constructible<_Bit_alloc_type>. + _Bvector_impl(_Bvector_impl&& __x) noexcept + : _Bit_alloc_type(std::move(__x)), _Bvector_impl_data(std::move(__x)) + { } + + _Bvector_impl(_Bit_alloc_type&& __a, _Bvector_impl&& __x) noexcept + :
Re: [PATCH 0/2] Future warnings not treated as errors
On Tue, Mar 19, 2019 at 10:09 PM Alex Henrie wrote: > > On Tue, Mar 19, 2019 at 11:03 AM Martin Liška wrote: > > > > What's Joseph telling is that right now the patch can't be approved (and > > installed). > > It's due to fact that we are close to the release in stage4: > > https://www.gnu.org/software/gcc/develop.html#stage4 > > > > So that a proper review will happen in couple of weeks. > > > > Thanks for understanding, > > Sure. I look forward to hearing back from you guys in a couple of weeks. Now that GCC 9.1 has been released, would you please merge these two patches? -Alex
[PATCH v2 2/3] Add predict_doloop_p target hook
From: Kewen Lin Previous version link for background: https://gcc.gnu.org/ml/gcc-patches/2019-04/msg00912.html This hook is to predict whether one loop in gimple will be transformed to low-overhead loop later in RTL, and designed to be called in ivopts. "Since the low-overhead loop optimize transformation is based on RTL, some of those checks are hard to be imitated on gimple, so it's not possible to predict the current loop will be transformed exactly in middle-end. But if we can have most loop predicted precisely, it would be helpful. It highly depends on target hook fine tuning. It's acceptable to have some loops which can be transformed to low-overhead loop but we don't catch. But we should try our best to avoid to predict some loop as low-overhead loop but which isn't." Bootstrapped and regression testing passed on powerpc64le. Is it ok for trunk? gcc/ChangeLog 2019-05-13 Kewen Lin PR middle-end/80791 * target.def (predict_doloop_p): New hook. * targhooks.h (default_predict_doloop_p): New declaration. * targhooks.c (default_predict_doloop_p): New function. * doc/tm.texi.in (TARGET_PREDICT_DOLOOP_P): New hook. * doc/tm.texi: Regenerate. * config/rs6000/rs6000.c (invalid_insn_for_doloop_p): New function. (costly_iter_for_doloop_p): Likewise. (rs6000_predict_doloop_p): Likewise. (TARGET_PREDICT_DOLOOP_P): New macro. --- gcc/config/rs6000/rs6000.c | 174 - gcc/doc/tm.texi| 8 +++ gcc/doc/tm.texi.in | 2 + gcc/target.def | 9 +++ gcc/targhooks.c| 13 gcc/targhooks.h| 1 + 6 files changed, 206 insertions(+), 1 deletion(-) diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index a21f4f7..1c1c8eb 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -83,6 +83,9 @@ #include "tree-ssa-propagate.h" #include "tree-vrp.h" #include "tree-ssanames.h" +#include "tree-ssa-loop-niter.h" +#include "tree-cfg.h" +#include "tree-scalar-evolution.h" /* This file should be included last. */ #include "target-def.h" @@ -1914,6 +1917,9 @@ static const struct attribute_spec rs6000_attribute_table[] = #undef TARGET_CAN_USE_DOLOOP_P #define TARGET_CAN_USE_DOLOOP_P can_use_doloop_if_innermost +#undef TARGET_PREDICT_DOLOOP_P +#define TARGET_PREDICT_DOLOOP_P rs6000_predict_doloop_p + #undef TARGET_ATOMIC_ASSIGN_EXPAND_FENV #define TARGET_ATOMIC_ASSIGN_EXPAND_FENV rs6000_atomic_assign_expand_fenv @@ -39436,7 +39442,173 @@ rs6000_mangle_decl_assembler_name (tree decl, tree id) return id; } - +/* Check whether there are some instructions preventing doloop transformation + inside loop body, mainly for instructions which are possible to kill CTR. + + Return true if some invalid insn exits, otherwise return false. */ + +static bool +invalid_insn_for_doloop_p (struct loop *loop) +{ + basic_block *body = get_loop_body (loop); + gimple_stmt_iterator gsi; + + for (unsigned i = 0; i < loop->num_nodes; i++) +for (gsi = gsi_start_bb (body[i]); !gsi_end_p (gsi); gsi_next ()) + { + gimple *stmt = gsi_stmt (gsi); + if (is_gimple_call (stmt) && !gimple_call_internal_p (stmt) + && !is_inexpensive_builtin (gimple_call_fndecl (stmt))) + { + if (dump_file && (dump_flags & TDF_DETAILS)) + fprintf (dump_file, + "predict doloop failure due to finding call.\n"); + return true; + } + if (computed_goto_p (stmt)) + { + if (dump_file && (dump_flags & TDF_DETAILS)) + fprintf (dump_file, "predict doloop failure due to" + "finding computed jump.\n"); + return true; + } + /* TODO: Now this hook is expected to be called in ivopts, which is + before switchlower1/switchlower2. Checking for SWITCH at this point + will eliminate some good candidates. But since there are only a few + cases having _a_ switch statement in the innermost loop, it can be a low + priority enhancement. */ + + if (gimple_code (stmt) == GIMPLE_SWITCH) + { + if (dump_file && (dump_flags & TDF_DETAILS)) + fprintf (dump_file, + "predict doloop failure due to finding switch.\n"); + return true; + } + } + + return false; +} + +/* Check whether number of iteration computation is too costly for doloop + transformation. It expands the gimple sequence to equivalent RTL insn + sequence, then evaluate the cost. + + Return true if it's costly, otherwise return false. */ + +static bool +costly_iter_for_doloop_p (struct loop *loop, tree niters) +{ + tree type = TREE_TYPE (niters); + unsigned cost = 0, i; + tree obj; + bool speed = optimize_loop_for_speed_p (loop); + int regno = LAST_VIRTUAL_REGISTER + 1; + vec tvec; +
[PATCH v2 3/3] Consider doloop cmp use in ivopts
From: Kewen Lin Previous version link for background: https://gcc.gnu.org/ml/gcc-patches/2019-04/msg00912.html Firstly, it's to call predict_doloop_p hook to check this loop will be transformed to doloop or not, if yes, find the expected comp iv use and its dependent original iv, set the iv candidate as bind_cand of the group. In following candidate selection process, we will bypass the group with bind_cand, since we don't want to affect global decision making for an iv use which will be eliminated eventually. At the time of iv candidate set finalization, we will fill the cost pair for the group with bind_cand. If the bind_cand is already in the final set, then just use it. Otherwise, we can check whether one of existing final set is better and fill with that if so. Bootstrapped and regression testing passed on powerpc64le. Is it ok for trunk? gcc/ChangeLog 2019-05-14 Kewen Lin PR middle-end/80791 * tree-ssa-loop-ivopts.c (tree_ssa_iv_optimize_loop): Call predict_doloop_p hook and bind_cand_for_doloop_uses. (bind_cand_for_doloop_uses): New function. (find_optimal_iv_set): Call handle_groups_with_bind_cand. (handle_groups_with_bind_cand): New function. (record_group): Init bind_cand. (set_group_iv_cost): Consider bind_cand group. (iv_ca_dump): Add dump for bind_cand. (try_add_cand_for): Bypass bind_cand group. (iv_ca_extend): Likewise. (iv_ca_narrow): Likewise. (iv_ca_replace): Likewise. gcc/testsuite/ChangeLog 2019-05-14 Kewen Lin PR middle-end/80791 * gcc.dg/tree-ssa/ivopts-lt.c : Adjust. --- gcc/testsuite/gcc.dg/tree-ssa/ivopts-lt.c | 7 +- gcc/tree-ssa-loop-ivopts.c| 155 +- 2 files changed, 156 insertions(+), 6 deletions(-) diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ivopts-lt.c b/gcc/testsuite/gcc.dg/tree-ssa/ivopts-lt.c index 171c85a..f61288c 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/ivopts-lt.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/ivopts-lt.c @@ -17,6 +17,7 @@ f1 (char *p, uintptr_t i, uintptr_t n) while (i < n); } -/* { dg-final { scan-tree-dump-times "PHI" 1 "ivopts" } } */ -/* { dg-final { scan-tree-dump-times "PHI vuses; }; @@ -1592,6 +1594,7 @@ record_group (struct ivopts_data *data, enum use_type type) group->type = type; group->related_cands = BITMAP_ALLOC (NULL); group->vuses.create (1); + group->bind_cand = NULL; data->vgroups.safe_push (group); return group; @@ -3612,7 +3615,9 @@ set_group_iv_cost (struct ivopts_data *data, { unsigned i, s; - if (cost.infinite_cost_p ()) + gcc_assert (cand); + /* For the group with bind_cand, make it always have cost pair. */ + if (cost.infinite_cost_p () && group->bind_cand != cand) { BITMAP_FREE (inv_vars); BITMAP_FREE (inv_exprs); @@ -6067,7 +6072,8 @@ iv_ca_dump (struct ivopts_data *data, FILE *file, struct iv_ca *ivs) group->id, cp->cand->id, cp->cost.cost, cp->cost.complexity); else - fprintf (file, " group:%d --> ??\n", group->id); + fprintf (file, " group:%d --> ?? %s\n", group->id, +group->bind_cand ? "(bind)" : ""); } const char *pref = ""; @@ -6110,6 +6116,9 @@ iv_ca_extend (struct ivopts_data *data, struct iv_ca *ivs, for (i = 0; i < ivs->upto; i++) { group = data->vgroups[i]; + /* Ignore groups binded with some cand. */ + if (group->bind_cand) + continue; old_cp = iv_ca_cand_for_group (ivs, group); if (old_cp @@ -6165,7 +6174,9 @@ iv_ca_narrow (struct ivopts_data *data, struct iv_ca *ivs, for (i = 0; i < data->vgroups.length (); i++) { group = data->vgroups[i]; - + /* Ignore groups binded with some cand. */ + if (group->bind_cand) + continue; old_cp = iv_ca_cand_for_group (ivs, group); if (old_cp->cand != cand) continue; @@ -6348,6 +6359,9 @@ iv_ca_replace (struct ivopts_data *data, struct iv_ca *ivs, for (j = 0; j < ivs->upto; j++) { struct iv_group *group = data->vgroups[j]; + /* Ignore groups binded with some cand. */ + if (group->bind_cand) + continue; old_cp = iv_ca_cand_for_group (ivs, group); if (old_cp->cand != cand) @@ -6406,6 +6420,15 @@ try_add_cand_for (struct ivopts_data *data, struct iv_ca *ivs, struct iv_ca_delta *best_delta = NULL, *act_delta; struct cost_pair *cp; + /* Bypass the candidate selection for the groups with bind_cand, but need + to keep upto up to date, to avoid the count of visited groups becomes + inconsistent in futher handlings. */ + if (group->bind_cand) +{ + ivs->upto++; + return true; +} + iv_ca_add_group (data, ivs, group); best_cost = iv_ca_cost (ivs); cp = iv_ca_cand_for_group (ivs, group); @@ -6635,6 +6658,59 @@ find_optimal_iv_set_1 (struct ivopts_data *data,
[PATCH v2 1/3] Move prepare_decl_rtl to expr.[ch] as extern
From: Kewen Lin Previous version link: https://gcc.gnu.org/ml/gcc-patches/2019-04/msg00912.html This is a NFC (no functional change) patch. Ivopts has some codes to expand gimple to RTL seq, but before call expanding, we should call a preparation funciton prepare_decl_rtl. This patch is to change it and its dependents to non-static, can be shared with other passes. Bootstrapped and regression testing passed on powerpc64le. Is OK for trunk? gcc/ChangeLog 2019-05-13 Kewen Lin PR middle-end/80791 * expr.c (produce_memory_decl_rtl): New function. (prepare_decl_rtl): Likewise. * expr.h (produce_memory_decl_rtl): New declaration. (prepare_decl_rtl): Likewise. * tree-ssa-loop-ivopts.c (produce_memory_decl_rtl): Remove. (prepare_decl_rtl): Likewise. (computation_cost): Updated to call refactored prepare_decl_rtl. --- gcc/expr.c | 91 + gcc/expr.h | 16 +++- gcc/tree-ssa-loop-ivopts.c | 93 ++ 3 files changed, 110 insertions(+), 90 deletions(-) diff --git a/gcc/expr.c b/gcc/expr.c index 9ff5e5f..1f2ad45 100644 --- a/gcc/expr.c +++ b/gcc/expr.c @@ -12539,3 +12539,94 @@ int_expr_size (tree exp) return tree_to_shwi (size); } + +/* Produce DECL_RTL for object obj so it looks like it is stored in memory. */ + +rtx +produce_memory_decl_rtl (tree obj, int *regno) +{ + addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (obj)); + machine_mode address_mode = targetm.addr_space.address_mode (as); + rtx x; + + gcc_assert (obj); + if (TREE_STATIC (obj) || DECL_EXTERNAL (obj)) +{ + const char *name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (obj)); + x = gen_rtx_SYMBOL_REF (address_mode, name); + SET_SYMBOL_REF_DECL (x, obj); + x = gen_rtx_MEM (DECL_MODE (obj), x); + set_mem_addr_space (x, as); + targetm.encode_section_info (obj, x, true); +} + else +{ + x = gen_raw_REG (address_mode, (*regno)++); + x = gen_rtx_MEM (DECL_MODE (obj), x); + set_mem_addr_space (x, as); +} + + return x; +} + +/* Prepares decl_rtl for variables referred in *EXPR_P. Callback for + walk_tree. DATA contains the actual fake register number. */ + +tree +prepare_decl_rtl (tree *expr_p, int *ws, void *data) +{ + tree obj = NULL_TREE; + rtx x = NULL_RTX; + decl_rtl_data *info = (decl_rtl_data *) data; + int *regno = info->regno; + vec *treevec = info->treevec; + + switch (TREE_CODE (*expr_p)) +{ +case ADDR_EXPR: + for (expr_p = _OPERAND (*expr_p, 0); handled_component_p (*expr_p); + expr_p = _OPERAND (*expr_p, 0)) + continue; + obj = *expr_p; + if (DECL_P (obj) && HAS_RTL_P (obj) && !DECL_RTL_SET_P (obj)) + x = produce_memory_decl_rtl (obj, regno); + break; + +case SSA_NAME: + *ws = 0; + obj = SSA_NAME_VAR (*expr_p); + /* Defer handling of anonymous SSA_NAMEs to the expander. */ + if (!obj) + return NULL_TREE; + if (!DECL_RTL_SET_P (obj)) + x = gen_raw_REG (DECL_MODE (obj), (*regno)++); + break; + +case VAR_DECL: +case PARM_DECL: +case RESULT_DECL: + *ws = 0; + obj = *expr_p; + + if (DECL_RTL_SET_P (obj)) + break; + + if (DECL_MODE (obj) == BLKmode) + x = produce_memory_decl_rtl (obj, regno); + else + x = gen_raw_REG (DECL_MODE (obj), (*regno)++); + + break; + +default: + break; +} + + if (x) +{ + treevec->safe_push (obj); + SET_DECL_RTL (obj, x); +} + + return NULL_TREE; +} diff --git a/gcc/expr.h b/gcc/expr.h index 17c3962..b1894a6b 100644 --- a/gcc/expr.h +++ b/gcc/expr.h @@ -53,7 +53,21 @@ typedef struct separate_ops tree type; tree op0, op1, op2; } *sepops; - + +/* This structure is used to pass information to tree walker function + prepare_decl_rtl. */ +typedef struct data_for_decl_rtl +{ + int *regno; + vec *treevec; +} decl_rtl_data; + +/* Produce decl_rtl for object so it looks like it is stored in memory. */ +rtx produce_memory_decl_rtl (tree, int *); + +/* Prepares decl_rtl for variables referred. Callback for walk_tree. */ +tree prepare_decl_rtl (tree *, int *, void *); + /* This is run during target initialization to set up which modes can be used directly in memory and to initialize the block move optab. */ extern void init_expr_target (void); diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c index a44b4cb..885c8e8 100644 --- a/gcc/tree-ssa-loop-ivopts.c +++ b/gcc/tree-ssa-loop-ivopts.c @@ -3687,94 +3687,6 @@ get_group_iv_cost (struct ivopts_data *data, struct iv_group *group, return NULL; } -/* Produce DECL_RTL for object obj so it looks like it is stored in memory. */ -static rtx -produce_memory_decl_rtl (tree obj, int *regno) -{ - addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (obj)); - machine_mode address_mode =
[V2] malloc cannot alias preexisting pointers
Here is a version of the patch with a cheaper test, and an extra testcase for Martin. (I kept the tree-ssa-loop-niter.c part although I am not using it anymore) 2019-05-15 Marc Glisse gcc/ * tree-ssa-loop-niter.c (stmt_dominates_stmt_p): Handle NULL basic block. * tree-ssa-alias.c (quick_stmt_dominates_stmt_p): New function. (ptr_derefs_may_alias_p): Detect malloc and an older pointer. gcc/testsuite/ * g++.dg/tree-ssa/ldist-2.C: New file. * gcc.dg/alias-17.c: New file. -- Marc GlisseIndex: gcc/testsuite/g++.dg/tree-ssa/ldist-2.C === --- gcc/testsuite/g++.dg/tree-ssa/ldist-2.C (nonexistent) +++ gcc/testsuite/g++.dg/tree-ssa/ldist-2.C (working copy) @@ -0,0 +1,14 @@ +// { dg-do compile { target c++11 } } +// { dg-options "-O3 -fdump-tree-optimized" } + +#include +#include +#include +// Remove those 2 inlines once gcc knows that new/delete are special +inline void* operator new(std::size_t n){return malloc(n);} +inline void operator delete(void*p){free(p);} +typedef std::unique_ptr T; +typedef std::vector V; +void f(V){v.reserve(1024);} + +/* { dg-final { scan-tree-dump "memcpy" "optimized" } } */ Index: gcc/testsuite/gcc.dg/alias-17.c === --- gcc/testsuite/gcc.dg/alias-17.c (nonexistent) +++ gcc/testsuite/gcc.dg/alias-17.c (working copy) @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ + +__attribute__((malloc)) void*mymalloc(void); +int g; +int*f(int**p){ + int*q=*p; + if(!q)return 0; // new basic block, because the optimization is weak + int*r=mymalloc(); + *q=1; + *r=2; + g=*q; + return r; +} + +/* { dg-final { scan-tree-dump "g = 1;" "optimized"} } */ Index: gcc/tree-ssa-alias.c === --- gcc/tree-ssa-alias.c (revision 271135) +++ gcc/tree-ssa-alias.c (working copy) @@ -118,20 +118,42 @@ dump_alias_stats (FILE *s) + alias_stats.ref_maybe_used_by_call_p_may_alias); fprintf (s, " call_may_clobber_ref_p: " HOST_WIDE_INT_PRINT_DEC" disambiguations, " HOST_WIDE_INT_PRINT_DEC" queries\n", alias_stats.call_may_clobber_ref_p_no_alias, alias_stats.call_may_clobber_ref_p_no_alias + alias_stats.call_may_clobber_ref_p_may_alias); dump_alias_stats_in_alias_c (s); } +/* A faster, weaker version of stmt_dominates_stmt_p. */ + +static bool +quick_stmt_dominates_stmt_p (gimple *s1, gimple *s2) +{ + basic_block bb1 = gimple_bb (s1), bb2 = gimple_bb (s2); + + if (!bb1 + || s1 == s2) +return true; + + if (!bb2) +return false; + + if (bb1 == bb2) +/* Punt. Except in a few passes that maintain UIDs, the only way to answer + is to walk through statements, which is too expensive. */ +return false; + + return dominated_by_p (CDI_DOMINATORS, bb2, bb1); +} + /* Return true, if dereferencing PTR may alias with a global variable. */ bool ptr_deref_may_alias_global_p (tree ptr) { struct ptr_info_def *pi; /* If we end up with a pointer constant here that may point to global memory. */ @@ -284,20 +306,39 @@ ptr_derefs_may_alias_p (tree ptr1, tree || !POINTER_TYPE_P (TREE_TYPE (ptr1)) || !POINTER_TYPE_P (TREE_TYPE (ptr2))) return true; /* We may end up with two empty points-to solutions for two same pointers. In this case we still want to say both pointers alias, so shortcut that here. */ if (ptr1 == ptr2) return true; + /* Memory returned by malloc cannot alias with a pre-existing pointer. + This is extremely crude, the order between the statements may be quite + arbitrary. */ + if (in_gimple_form && dom_info_available_p (cfun, CDI_DOMINATORS)) +{ + gimple *def1 = SSA_NAME_DEF_STMT (ptr1); + gimple *def2 = SSA_NAME_DEF_STMT (ptr2); + tree decl; + if ((is_gimple_call (def2) + && (decl = gimple_call_fndecl (def2)) + && DECL_IS_MALLOC (decl) + && quick_stmt_dominates_stmt_p (def1, def2)) + || (is_gimple_call (def1) + && (decl = gimple_call_fndecl (def1)) + && DECL_IS_MALLOC (decl) + && quick_stmt_dominates_stmt_p (def2, def1))) + return false; +} + /* If we do not have useful points-to information for either pointer we cannot disambiguate anything else. */ pi1 = SSA_NAME_PTR_INFO (ptr1); pi2 = SSA_NAME_PTR_INFO (ptr2); if (!pi1 || !pi2) return true; /* ??? This does not use TBAA to prune decls from the intersection that not both pointers may access. */ return pt_solutions_intersect (>pt, >pt); Index: gcc/tree-ssa-loop-niter.c === --- gcc/tree-ssa-loop-niter.c (revision 271135) +++ gcc/tree-ssa-loop-niter.c (working copy) @@ -4433,20 +4433,23 @@ stmt_dominates_stmt_p (gimple *s1, gimpl if (gimple_code (s1) == GIMPLE_PHI)
Re: [PATCH] Fix aarch64 exception handling (PR c++/59813)
On Mon, May 13, 2019 at 04:29:21PM -0600, Jeff Law wrote: > > That is a serious misunderstanding of what __builtin_eh_return does. > > For the most part, __builtin_eh_return works by forcing all call clobbered > > registers to stack, having that described in the unwind info and having the > > unwinding code modify that saved info when needed. The adjustment of the > > stack is done only on approx. half of the targets where the destination > > stack pointer should be derived from CFA instead of being restored with the > > rest of the call saved registers. You can't do this in assembly, it needs > > to be done in the same function that saves the state etc. > So why not just reject tail calls when we see __builtin_eh_return. It's > not ideal, but that feels better than the currently posted hack. The problem as has been said in this thread is that crtl->calls_eh_return flag is only set during expansion, so if it is tested when deciding if we should allow the tail call or not, it might be (and in the case of _Unwind_Resume_or_Rethrow it is) too late, because we first expand the call that could be tail call and only afterwards expand the __builtin_eh_return and set crtl->calls_eh_return. In https://gcc.gnu.org/ml/gcc-patches/2019-05/msg00484.html I've posted a patch that would set it earlier (or it could be set during gimplification and propagated during inlining, would need to be in cfun->calls_eh_return instead of crtl->calls_eh_return) and then targets for which we do not want to bother with it or where it is not beneficial to have tail calls in functions that call __builtin_eh_return (as I said in the thread, e.g. on x86_64-linux it is both smaller and faster), the targets which we expect to fail currently or even have a proof of that can just punt. Jakub
Re: [PATCH] Fix aarch64 exception handling (PR c++/59813)
On 5/13/19 2:09 PM, Jakub Jelinek wrote: > On Mon, May 13, 2019 at 07:57:30PM +, Wilco Dijkstra wrote: >> Hi, >> >>> I think my worry would be all the targets that support tail calls, but >>> which we can't easily test. ie, we put in the hack, but when do we >>> know it can be removed? >> >> Well it would be easy to make a runnable torture test. I'm not sure >> how easy it would be to do it using a scan-assembler test given the >> stack pointer adjustment may not use a standard pattern (and the >> SP has many different names). >> >>> Is there any property of the code that we can look at instead of a hack >>> like this? >> >> We could simpify __builtin_eh_return and implement it as a libgcc >> function or automatically generated thunk. Basically all you need is >> tailcall a 2-instruction helper which adjusts the stack and then jumps >> to the given function pointer. This way you don't need the complex >> special cases and optimization disables for eh_return functions. > > That is a serious misunderstanding of what __builtin_eh_return does. > For the most part, __builtin_eh_return works by forcing all call clobbered > registers to stack, having that described in the unwind info and having the > unwinding code modify that saved info when needed. The adjustment of the > stack is done only on approx. half of the targets where the destination > stack pointer should be derived from CFA instead of being restored with the > rest of the call saved registers. You can't do this in assembly, it needs > to be done in the same function that saves the state etc. So why not just reject tail calls when we see __builtin_eh_return. It's not ideal, but that feels better than the currently posted hack. jeff
Re: [PATCH] Fix aarch64 exception handling (PR c++/59813)
Hi Jakub, >> We could simpify __builtin_eh_return and implement it as a libgcc >> function or automatically generated thunk. Basically all you need is >> tailcall a 2-instruction helper which adjusts the stack and then jumps >> to the given function pointer. This way you don't need the complex >> special cases and optimization disables for eh_return functions. > > That is a serious misunderstanding of what __builtin_eh_return does. And this is exactly the issue - it's undocumented/untested. There isn't a well-defined interface that makes it clear which part of the feature is implemented by the midend and which part must be done by each backend. > For the most part, __builtin_eh_return works by forcing all call clobbered > registers to stack, having that described in the unwind info and having the > unwinding code modify that saved info when needed. Sure, and that's target independent. > The adjustment of the > stack is done only on approx. half of the targets where the destination > stack pointer should be derived from CFA instead of being restored with the > rest of the call saved registers. You can't do this in assembly, it needs > to be done in the same function that saves the state etc. No, it can be done by a simple 2-instruction thunk. Targets which save the stack pointer don't require it since they restore SP already. You could also enter the unwinder via a small assembler veneer which saves the callee-save registers and stack pointer (a bit like setjmp), and the unwinder will work just fine. Once you decide what the specification is, the implementation becomes really trivial. But without it, every target is just guessing what it should implement. Wilco
C++ PATCH for CWG 2096, constraints on literal unions
This patch implements CWG 2096 which relaxes the constraints for literal unions. [basic.types]/p10 now says: A type is a literal type if... -- it is a union, at least one of its non-static data members is of non-volatile literal type, [...] check_field_decls is called with CLASSTYPE_LITERAL_P set to true, so we can re-set it after we've processed all fields. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2019-05-13 Marek Polacek CWG 2096 - constraints on literal unions. * class.c (check_field_decls): Initialize booleans directly. A union is literal if at least one of its non-static data members is of non-volatile literal type. * g++.dg/cpp0x/literal-type1.C: New test. diff --git gcc/cp/class.c gcc/cp/class.c index a4cdd9e..ed885a5a2c1 100644 --- gcc/cp/class.c +++ gcc/cp/class.c @@ -3403,18 +3403,19 @@ check_field_decls (tree t, tree *access_decls, { tree *field; tree *next; - bool has_pointers; - bool any_default_members; int cant_pack = 0; int field_access = -1; /* Assume there are no access declarations. */ *access_decls = NULL_TREE; /* Assume this class has no pointer members. */ - has_pointers = false; + bool has_pointers = false; /* Assume none of the members of this class have default initializations. */ - any_default_members = false; + bool any_default_members = false; + /* Assume none of the non-static data members are of non-volatile literal + type. */ + bool found_nv_literal_p = false; for (field = _FIELDS (t); *field; field = next) { @@ -3498,13 +3499,19 @@ check_field_decls (tree t, tree *access_decls, if (TREE_PRIVATE (x) || TREE_PROTECTED (x)) CLASSTYPE_NON_AGGREGATE (t) = 1; - /* If at least one non-static data member is non-literal, the whole - class becomes non-literal. Per Core/1453, volatile non-static -data members and base classes are also not allowed. + /* If it is not a union and at least one non-static data member is +non-literal, the whole class becomes non-literal. Per Core/1453, +volatile non-static data members and base classes are also not allowed. +If it is a union, we might set CLASSTYPE_LITERAL_P after we've seen all +members. Note: if the type is incomplete we will complain later on. */ - if (COMPLETE_TYPE_P (type) - && (!literal_type_p (type) || CP_TYPE_VOLATILE_P (type))) -CLASSTYPE_LITERAL_P (t) = false; + if (COMPLETE_TYPE_P (type)) + { + if (!literal_type_p (type) || CP_TYPE_VOLATILE_P (type)) + CLASSTYPE_LITERAL_P (t) = false; + else + found_nv_literal_p = true; + } /* A standard-layout class is a class that: ... @@ -3677,6 +3684,11 @@ check_field_decls (tree t, tree *access_decls, "field %q#D with same name as class", x); } + /* Per CWG 2096, a type is a literal type if it is a union, and at least + one of its non-static data members is of non-volatile literal type. */ + if (TREE_CODE (t) == UNION_TYPE && found_nv_literal_p) +CLASSTYPE_LITERAL_P (t) = true; + /* Effective C++ rule 11: if a class has dynamic memory held by pointers, it should also define a copy constructor and an assignment operator to implement the correct copy semantic (deep vs shallow, etc.). As it is diff --git gcc/testsuite/g++.dg/cpp0x/literal-type1.C gcc/testsuite/g++.dg/cpp0x/literal-type1.C new file mode 100644 index 000..7b5d4288923 --- /dev/null +++ gcc/testsuite/g++.dg/cpp0x/literal-type1.C @@ -0,0 +1,54 @@ +// CWG 2096 - constraints on literal unions. +// { dg-do compile { target c++11 } } + +struct literal { }; +typedef volatile int nonliteral_v; +struct nonliteral { + nonliteral() {} +}; + +union U { + literal l; + nonliteral n; + + constexpr U() : l{} {} +}; + +constexpr U u{}; + +union U2 { + nonliteral n; + literal l; + + constexpr U2() : l{} {} +}; + +constexpr U2 u2{}; + +union U3 { // { dg-message "not literal" } + nonliteral_v n; // { dg-message "volatile type" } + + constexpr U3() : n{} {} +}; + +constexpr U3 u3{}; // { dg-error "not literal" } + +union U4 { + nonliteral n; + nonliteral_v n2; + literal l; + nonliteral n3; + + constexpr U4() : l{} {} +}; + +constexpr U4 u4{}; + +union U5 { + nonliteral_v n; + literal l; + + constexpr U5() : n{} {} +}; + +constexpr U5 u5{};
[C++ PATCH] Use releasing_vec more broadly.
The releasing_vec class is useful anywhere we want to release_tree_vector on exit, particularly in the presence of multiple returns. Tested x86_64-pc-linux-gnu, applying to trunk. * cp-tree.h (struct releasing_vec): Replace get_ref method with operator&. (vec_safe_push, vec_safe_reserve, vec_safe_length, vec_safe_splice): Forwarding functions for releasing_vec. (release_tree_vector): Declare but don't define. * call.c (build_op_delete_call, build_temp, call_copy_ctor) (perform_direct_initialization_if_possible): Use releasing_vec. * constexpr.c (cxx_eval_vec_init_1, cxx_eval_store_expression): Likewise. * cp-gimplify.c (cp_fold): Likewise. * cvt.c (force_rvalue, ocp_convert): Likewise. * decl.c (get_tuple_decomp_init): Likewise. * except.c (build_throw): Likewise. * init.c (perform_member_init, expand_default_init): Likewise. * method.c (do_build_copy_assign, locate_fn_flags): Likewise. * parser.c (cp_parser_userdef_char_literal) (cp_parser_userdef_numeric_literal) (cp_parser_userdef_string_literal) (cp_parser_perform_range_for_lookup) (cp_parser_range_for_member_function, cp_parser_omp_for_loop) (cp_parser_omp_for_loop_init): Likewise. * pt.c (tsubst_copy_and_build, do_class_deduction): Likewise. * semantics.c (calculate_direct_bases, calculate_bases) (finish_omp_barrier, finish_omp_flush, finish_omp_taskwait) (finish_omp_taskyield, finish_omp_cancel) (finish_omp_cancellation_point): Likewise. * tree.c (build_vec_init_elt, strip_typedefs, strip_typedefs_expr) (build_min_non_dep_op_overload): Likewise. * typeck.c (build_function_call_vec, cp_build_function_call_nary) (cp_build_modify_expr): Likewise. * typeck2.c (build_functional_cast): Likewise. --- gcc/cp/cp-tree.h | 16 +--- gcc/cp/call.c| 13 - gcc/cp/constexpr.c | 11 --- gcc/cp/cp-gimplify.c | 4 +--- gcc/cp/cvt.c | 6 ++ gcc/cp/decl.c| 2 +- gcc/cp/except.c | 7 ++- gcc/cp/init.c| 6 ++ gcc/cp/method.c | 8 ++-- gcc/cp/parser.c | 35 +-- gcc/cp/pt.c | 32 +++- gcc/cp/semantics.c | 31 ++- gcc/cp/tree.c| 17 + gcc/cp/typeck.c | 11 +++ gcc/cp/typeck2.c | 4 +--- gcc/cp/ChangeLog | 35 +++ 16 files changed, 101 insertions(+), 137 deletions(-) diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 7fb25a93e44..827b471aa80 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -890,15 +890,25 @@ struct releasing_vec vec_t *operator-> () const { return v; } vec_t *get() const { return v; } operator vec_t *() const { return v; } - tree& operator[] (unsigned i) const { return (*v)[i]; } + vec_t ** operator& () { return } - /* Necessary for use with vec** and vec*& interfaces. */ - vec_t *_ref () { return v; } + /* Breaks pointer/value consistency for convenience. */ + tree& operator[] (unsigned i) const { return (*v)[i]; } ~releasing_vec() { release_tree_vector (v); } private: vec_t *v; }; +/* Forwarding functions for vec_safe_* that might reallocate. */ +inline tree* vec_safe_push (releasing_vec& r, const tree CXX_MEM_STAT_INFO) +{ return vec_safe_push (*, t PASS_MEM_STAT); } +inline bool vec_safe_reserve (releasing_vec& r, unsigned n, bool e = false CXX_MEM_STAT_INFO) +{ return vec_safe_reserve (*, n, e PASS_MEM_STAT); } +inline unsigned vec_safe_length (releasing_vec ) +{ return r->length(); } +inline void vec_safe_splice (releasing_vec , vec *p CXX_MEM_STAT_INFO) +{ vec_safe_splice (*, p PASS_MEM_STAT); } +void release_tree_vector (releasing_vec &); // cause link error struct GTY(()) tree_template_decl { struct tree_decl_common common; diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 2329c4cb43e..00cb3993471 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -6671,7 +6671,7 @@ build_op_delete_call (enum tree_code code, tree addr, tree size, } tree ret; - vec *args = make_tree_vector (); + releasing_vec args; args->quick_push (addr); if (destroying) args->quick_push (destroying); @@ -6683,7 +6683,6 @@ build_op_delete_call (enum tree_code code, tree addr, tree size, args->quick_push (al); } ret = cp_build_function_call_vec (fn, , complain); - release_tree_vector (args); return ret; } } @@ -6787,7 +6786,6 @@ build_temp (tree expr, tree type, int flags, diagnostic_t *diagnostic_kind, tsubst_flags_t complain) { int savew, savee; - vec *args; *diagnostic_kind = DK_UNSPECIFIED; @@ -6801,10 +6799,9 @@ build_temp (tree expr, tree
Re: Remove obsolete Solaris 10 libgo support
Hi Ian, > Thanks. The patch to mksysinfo.sh wasn't quite right. This was the > patch that I committed. that was the only part I'd added at the very last minute without testing it. I'd already noticed the mess myself earlier this evening when it broke my own bootstraps. Sorry for the mess and thanks. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: Remove obsolete Solaris 10 libgo support
From: Rainer Orth > Here's the libgo part of the Solaris 10 removal patch. I wonder if the > cleanup is worthwhile given the small size of the patch. > > Bootstrapped without regressions on i386-pc-solaris2.11 and > sparc-sun-solaris2.11 together with the main patch. Thanks. The patch to mksysinfo.sh wasn't quite right. This was the patch that I committed. Ian Index: gcc/go/gofrontend/MERGE === --- gcc/go/gofrontend/MERGE (revision 271088) +++ gcc/go/gofrontend/MERGE (working copy) @@ -1,4 +1,4 @@ -5c2c4743980556c041561533ef31762f524737ca +3f015e128bf6d1d9279f3d43e26f60f0927019cb The first line of this file holds the git revision number of the last merge done from the gofrontend repository. Index: libgo/configure.ac === --- libgo/configure.ac (revision 270877) +++ libgo/configure.ac (working copy) @@ -397,7 +397,7 @@ case "$target" in # msghdr in . OSCFLAGS="$OSCFLAGS -D_XOPEN_SOURCE=500" ;; -*-*-solaris2.1[[01]]) +*-*-solaris2.*) # Solaris 10+ needs this so struct msghdr gets the msg_control # etc. fields in (_XPG4_2). _XOPEN_SOURCE=600 as # above doesn't work with C99. Index: libgo/go/runtime/signal_gccgo.go === --- libgo/go/runtime/signal_gccgo.go(revision 270877) +++ libgo/go/runtime/signal_gccgo.go(working copy) @@ -60,11 +60,6 @@ type sigctxt struct { } func (c *sigctxt) sigcode() uint64 { - if c.info == nil { - // This can happen on Solaris 10. We don't know the - // code, just avoid a misleading value. - return _SI_USER + 1 - } return uint64(c.info.si_code) } Index: libgo/mksysinfo.sh === --- libgo/mksysinfo.sh (revision 270877) +++ libgo/mksysinfo.sh (working copy) @@ -735,13 +735,9 @@ if ! grep "const EAI_OVERFLOW " ${OUT} > fi # The passwd struct. -# Force uid and gid from int32 to uint32 for consistency; they are -# int32 on Solaris 10 but uint32 everywhere else including Solaris 11. grep '^type _passwd ' gen-sysinfo.go | \ sed -e 's/_passwd/Passwd/' \ -e 's/ pw_/ Pw_/g' \ - -e 's/ Pw_uid int32/ Pw_uid uint32/' \ - -e 's/ Pw_gid int32/ Pw_gid uint32/' \ >> ${OUT} # The group struct.
Re: [PATCH] Improve API docs for and
On 13/05/19 13:11 +0100, Jonathan Wakely wrote: On 13/05/19 14:02 +0200, Christophe Lyon wrote: Hi Jonathan, On Fri, 10 May 2019 at 23:41, Jonathan Wakely wrote: More Doxygenation. Tested powerpc64le-linux. Committed to trunk. You've probably noticed by now, but this introduced a regression (!): FAIL: g++.dg/cpp0x/Wattributes1.C -std=c++14 (test for warnings, line 125) Erm, yes, let's pretend I'd run the compiler testsuite and noticed by now, shall we? :-) This is because you added a line to libstdc++-v3/libsupc++/new and the warning now appears at line 126 I can certainly patch the test and replace 125 with 126 in it, I'm just not sure want to keep this fragile dependency? Yeah, I hate dg-error line numbers that refer into some arbitrary line of a header. I'll take care of it, thanks for pointing it out. Fixed at r271133.
[PATCH] PR libstdc++/90454.cc path construction from void*
Make the filesystem::path constructors SFINAE away for void* arguments, instead of giving an error due to iterator_traits::reference. PR libstdc++/90454.cc path construction from void* * include/bits/fs_path.h (path::_Path): Use remove_pointer so that pointers to void are rejected as well as void. * include/experimental/bits/fs_path.h (path::_Path): Likewise. * testsuite/27_io/filesystem/path/construct/80762.cc: Also check pointers to void. * testsuite/experimental/filesystem/path/construct/80762.cc: Likewise. Tested powerpc64le-linux and x86_64-linux, committed to trunk. commit 8f5ae78f657a521d506fd1d1ce6d70520461deb0 Author: Jonathan Wakely Date: Mon May 13 20:53:09 2019 +0100 PR libstdc++/90454.cc path construction from void* Make the filesystem::path constructors SFINAE away for void* arguments, instead of giving an error due to iterator_traits::reference. PR libstdc++/90454.cc path construction from void* * include/bits/fs_path.h (path::_Path): Use remove_pointer so that pointers to void are rejected as well as void. * include/experimental/bits/fs_path.h (path::_Path): Likewise. * testsuite/27_io/filesystem/path/construct/80762.cc: Also check pointers to void. * testsuite/experimental/filesystem/path/construct/80762.cc: Likewise. diff --git a/libstdc++-v3/include/bits/fs_path.h b/libstdc++-v3/include/bits/fs_path.h index d1ad11a60c4..cec35614b42 100644 --- a/libstdc++-v3/include/bits/fs_path.h +++ b/libstdc++-v3/include/bits/fs_path.h @@ -115,7 +115,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 template using _Path = typename std::enable_if<__and_<__not_, path>>, - __not_>, + __not_>>, __constructible_from<_Tp1, _Tp2>>::value, path>::type; diff --git a/libstdc++-v3/include/experimental/bits/fs_path.h b/libstdc++-v3/include/experimental/bits/fs_path.h index f81f33ca161..588f06822be 100644 --- a/libstdc++-v3/include/experimental/bits/fs_path.h +++ b/libstdc++-v3/include/experimental/bits/fs_path.h @@ -128,11 +128,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 : decltype(__is_path_src(std::declval<_Source>(), 0)) { }; -template +template::type, +typename _Tp1_noptr = typename remove_pointer<_Tp1>::type> using _Path = typename - std::enable_if<__and_<__not_::type, -path>>, - __not_>, + std::enable_if<__and_<__not_>, + __not_>, __constructible_from<_Tp1, _Tp2>>::value, path>::type; diff --git a/libstdc++-v3/testsuite/27_io/filesystem/path/construct/80762.cc b/libstdc++-v3/testsuite/27_io/filesystem/path/construct/80762.cc index 71cdaa9d969..e22e3deb56e 100644 --- a/libstdc++-v3/testsuite/27_io/filesystem/path/construct/80762.cc +++ b/libstdc++-v3/testsuite/27_io/filesystem/path/construct/80762.cc @@ -22,8 +22,18 @@ using std::filesystem::path; +// PR libstdc++/80762.cc static_assert( !std::is_constructible_v ); static_assert( !std::is_constructible_v ); static_assert( !std::is_constructible_v ); static_assert( !std::is_constructible_v ); static_assert( !std::is_constructible_v ); + +// PR libstdc++/90454.cc +static_assert( !std::is_constructible_v ); +static_assert( !std::is_constructible_v ); +static_assert( !std::is_constructible_v ); +static_assert( !std::is_constructible_v ); +static_assert( !std::is_constructible_v ); +static_assert( !std::is_constructible_v ); +static_assert( !std::is_constructible_v ); diff --git a/libstdc++-v3/testsuite/experimental/filesystem/path/construct/80762.cc b/libstdc++-v3/testsuite/experimental/filesystem/path/construct/80762.cc index fa4a64feb3e..98dadabcffb 100644 --- a/libstdc++-v3/testsuite/experimental/filesystem/path/construct/80762.cc +++ b/libstdc++-v3/testsuite/experimental/filesystem/path/construct/80762.cc @@ -22,8 +22,18 @@ using std::experimental::filesystem::path; +// PR libstdc++/80762.cc static_assert( !std::is_constructible::value, "" ); static_assert( !std::is_constructible::value, "" ); static_assert( !std::is_constructible::value, "" ); static_assert( !std::is_constructible::value, "" ); static_assert( !std::is_constructible::value, "" ); + +// PR libstdc++/90454.cc +static_assert( !std::is_constructible::value, "" ); +static_assert( !std::is_constructible::value, "" ); +static_assert( !std::is_constructible::value, "" ); +static_assert( !std::is_constructible::value, "" ); +static_assert( !std::is_constructible::value, "" ); +static_assert( !std::is_constructible::value, "" ); +static_assert( !std::is_constructible::value, "" );
[PATCH] Small markup changes to PBDS docs
* doc/xml/manual/policy_data_structures.xml: Comment out stray elements. Fix formatting of bibliography references. Committed to trunk. commit 97987cf2b2e7549e3c864e257f7e9f07c05db95a Author: Jonathan Wakely Date: Mon May 13 17:24:56 2019 +0100 Small markup changes to PBDS docs * doc/xml/manual/policy_data_structures.xml: Comment out stray elements. Fix formatting of bibliography references. diff --git a/libstdc++-v3/doc/xml/manual/policy_data_structures.xml b/libstdc++-v3/doc/xml/manual/policy_data_structures.xml index 0f19be315a8..107018043ae 100644 --- a/libstdc++-v3/doc/xml/manual/policy_data_structures.xml +++ b/libstdc++-v3/doc/xml/manual/policy_data_structures.xml @@ -458,10 +458,10 @@ priority-queues container), there is a possible need for different types of iterators for self-organizing containers: the iterator concept seems overloaded to mean two different - things (in some cases). XXX + things (in some cases). @@ -3415,7 +3415,9 @@ Αmin ≤ (number of stored elements) / (hash-table size) ≤ - Αmaxload factor min max + Αmax + + Collision-check policies work in the opposite direction of load-check policies. They focus on keeping the number of diff --git a/libstdc++-v3/doc/xml/manual/test_policy_data_structures.xml b/libstdc++-v3/doc/xml/manual/test_policy_data_structures.xml index 27d6c1df084..ea35efa22b9 100644 --- a/libstdc++-v3/doc/xml/manual/test_policy_data_structures.xml +++ b/libstdc++-v3/doc/xml/manual/test_policy_data_structures.xml @@ -2513,7 +2513,7 @@ This test inserts a number of values with keys from an arbitrary - text ([ wickland96thirty ]) into a container + text ([wickland96thirty]) into a container using insert . It measures the average time for insert as a function of the number of values inserted. @@ -3156,7 +3156,7 @@ This test inserts a number of values with keys from an - arbitrary text ([ wickland96thirty ]) into + arbitrary text ([wickland96thirty]) into a container, then performs a series of finds using find. It is different than Tree-Based and Trie-Based Text find Find Timing Test in the @@ -3846,7 +3846,7 @@ This test inserts a number of pairs into a container. The first item of each pair is a string from an arbitrary text - [wickland96thirty], and + ([wickland96thirty]), and the second is a uniform i.i.d.integer. The container is a "multimap" - it considers the first member of each pair as a primary key, and the second member of each pair as a secondary @@ -4310,7 +4310,7 @@ This test inserts a number of pairs into a container. The first item of each pair is a string from an arbitrary text - [wickland96thirty], and + ([wickland96thirty]), and the second is a uniform integer. The container is a "multimap" - it considers the first member of each pair as a primary key, and the second member of each pair as a secondary @@ -4775,7 +4775,7 @@ This test inserts a number of pairs into a container. The first item of each pair is a string from an arbitrary text - [wickland96thirty], and + ([wickland96thirty]), and the second is a uniform integer. The container is a "multimap" - it considers the first member of each pair as a primary key, and the second member of each pair as a secondary @@ -5242,7 +5242,7 @@ This test inserts a number of pairs into a container. The first item of each pair is a string from an arbitrary text - [wickland96thirty], and + ([wickland96thirty]), and the second is a uniform integer. The container is a "multimap" - it considers the first member of each pair as a primary key, and the second member of each pair as a secondary @@ -5708,7 +5708,7 @@ This test inserts a number of pairs into a container. The first item of each pair is a string from an arbitrary text - [wickland96thirty], and + ([wickland96thirty]), and the second is a uniform integer. The container is a "multimap" - it considers the first member of each pair as a primary key, and the second member of each pair as a secondary @@ -6168,7 +6168,7 @@ This test inserts a number of pairs into a container. The first item of each pair is a string from an arbitrary text - [wickland96thirty], and + ([wickland96thirty]), and the second is a uniform integer. The container is a "multimap" - it considers the first member of each pair as a
Re: [PATCH] Fix aarch64 exception handling (PR c++/59813)
On Mon, May 13, 2019 at 07:57:30PM +, Wilco Dijkstra wrote: > Hi, > > > I think my worry would be all the targets that support tail calls, but > > which we can't easily test. ie, we put in the hack, but when do we > > know it can be removed? > > Well it would be easy to make a runnable torture test. I'm not sure > how easy it would be to do it using a scan-assembler test given the > stack pointer adjustment may not use a standard pattern (and the > SP has many different names). > > > Is there any property of the code that we can look at instead of a hack > > like this? > > We could simpify __builtin_eh_return and implement it as a libgcc > function or automatically generated thunk. Basically all you need is > tailcall a 2-instruction helper which adjusts the stack and then jumps > to the given function pointer. This way you don't need the complex > special cases and optimization disables for eh_return functions. That is a serious misunderstanding of what __builtin_eh_return does. For the most part, __builtin_eh_return works by forcing all call clobbered registers to stack, having that described in the unwind info and having the unwinding code modify that saved info when needed. The adjustment of the stack is done only on approx. half of the targets where the destination stack pointer should be derived from CFA instead of being restored with the rest of the call saved registers. You can't do this in assembly, it needs to be done in the same function that saves the state etc. Jakub
Re: [PATCH] Fix aarch64 exception handling (PR c++/59813)
Hi, > I think my worry would be all the targets that support tail calls, but > which we can't easily test. ie, we put in the hack, but when do we > know it can be removed? Well it would be easy to make a runnable torture test. I'm not sure how easy it would be to do it using a scan-assembler test given the stack pointer adjustment may not use a standard pattern (and the SP has many different names). > Is there any property of the code that we can look at instead of a hack > like this? We could simpify __builtin_eh_return and implement it as a libgcc function or automatically generated thunk. Basically all you need is tailcall a 2-instruction helper which adjusts the stack and then jumps to the given function pointer. This way you don't need the complex special cases and optimization disables for eh_return functions. Or drop support for eh_return altogether since its one and only use seems to be for unwinding (which already requires target specific code). Wilco
Re: [PATCH] Fix testsuite regression caused by r271077
On 5/13/19 1:55 PM, Jonathan Wakely wrote: > * g++.dg/cpp0x/Wattributes1.C: Adjust dg-error line number to fix > regression, by matching a note on any line. > * g++.dg/cpp0x/Wattributes2.C: Add another copy that checks the > correct line number is matched without depending on a library header. > > Tested powerpc64le-linux, OK for trunk? > OK jeff
[PATCH] Fix testsuite regression caused by r271077
* g++.dg/cpp0x/Wattributes1.C: Adjust dg-error line number to fix regression, by matching a note on any line. * g++.dg/cpp0x/Wattributes2.C: Add another copy that checks the correct line number is matched without depending on a library header. Tested powerpc64le-linux, OK for trunk? commit d6f2a60080d43fe2c881bc0898f45c834ab29886 Author: Jonathan Wakely Date: Mon May 13 17:28:49 2019 +0100 Fix testsuite regression caused by r271077 * g++.dg/cpp0x/Wattributes1.C: Adjust dg-error line number to fix regression, by matching a note on any line. * g++.dg/cpp0x/Wattributes2.C: Add another copy that checks the correct line number is matched without depending on a library header. diff --git a/gcc/testsuite/g++.dg/cpp0x/Wattributes1.C b/gcc/testsuite/g++.dg/cpp0x/Wattributes1.C index b1c48d42349..2223936fba3 100644 --- a/gcc/testsuite/g++.dg/cpp0x/Wattributes1.C +++ b/gcc/testsuite/g++.dg/cpp0x/Wattributes1.C @@ -5,4 +5,4 @@ #include __attribute__((visibility("hidden")))void*operator new(std::size_t); // { dg-warning "visibility attribute ignored" } -// { dg-message "previous declaration" "" { target *-*-* } 125 } +// { dg-message "previous declaration" "" { target *-*-* } 0 } diff --git a/gcc/testsuite/g++.dg/cpp0x/Wattributes2.C b/gcc/testsuite/g++.dg/cpp0x/Wattributes2.C new file mode 100644 index 000..f37b1f010ca --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/Wattributes2.C @@ -0,0 +1,32 @@ +// PR c++/60373 +// { dg-do compile { target c++11 } } +// { dg-require-visibility "" } + +#pragma GCC visibility push(default) +namespace std +{ + using size_t = decltype(sizeof(0)); + struct nothrow_t { }; +} + +void* operator new(std::size_t) + __attribute__((__externally_visible__)); +void* operator new[](std::size_t) + __attribute__((__externally_visible__)); +void* operator new(std::size_t, const std::nothrow_t&) noexcept + __attribute__((__externally_visible__, __malloc__)); +void* operator new[](std::size_t, const std::nothrow_t&) noexcept + __attribute__((__externally_visible__, __malloc__)); +void operator delete(void*) noexcept + __attribute__((__externally_visible__)); +void operator delete[](void*) noexcept + __attribute__((__externally_visible__)); +void operator delete(void*, const std::nothrow_t&) noexcept + __attribute__((__externally_visible__)); +void operator delete[](void*, const std::nothrow_t&) noexcept + __attribute__((__externally_visible__)); +#pragma GCC visibility pop + +__attribute__((visibility("hidden")))void*operator new(std::size_t); // { dg-warning "visibility attribute ignored" } + +// { dg-message "previous declaration" "" { target *-*-* } 12 }
Re: [PATCH] Fix aarch64 exception handling (PR c++/59813)
> On 13 May 2019, at 20:10, Jakub Jelinek wrote: > > On Mon, May 13, 2019 at 12:52:54PM -0600, Jeff Law wrote: >> On 5/10/19 9:17 AM, Jakub Jelinek wrote: >>> On Fri, May 10, 2019 at 01:46:07PM +, Wilco Dijkstra wrote: And looking at the generated code, emitting a tailcall for this case is actually a de-optimization since the large eh_return epilog must be repeated for every tailcall. >>> >>> On x86_64, the code is shorter with the tail call rather than without. >>> >>> That said, here is actually tested workaround until targets are fixed. >>> Richard or Jeff, do we want this workaround? >>> >>> I don't see why we would need extra testsuite coverage for this, given the >>> number of FAILs or bootstrap issues on targets that are broken. >>> >>> 2019-05-10 Jakub Jelinek >>> >>> PR c++/59813 >>> * unwind.inc (_Unwind_Resume_or_Rethrow): Add optimize attribute >>> to temporarily avoid tail calls in the function. >> I think my worry would be all the targets that support tail calls, but >> which we can't easily test. ie, we put in the hack, but when do we >> know it can be removed? >> >> Is there any property of the code that we can look at instead of a hack >> like this? > > We can test crtl->calls_eh_return if if make sure it is computed early > (before expansion), or we could just compute it in the ok_for_sibcall > hook if we add caching of that per function (check if any bb that doesn't > have successors and is still in GIMPLE IL ends with __builtin_eh_return > call), if not already crtl->calls_eh_return. > > I think only targets that define EH_RETURN_STACKADJ_RTX are problematic, and > from those only those that do not use eh_return_internal patterns (e.g. i386 > or bfin or riscv have special eh_return_internal patterns that expand > epilogue with not just true/false for sibcall/for_sibcall, but either a 3 > state argument or two bools whether it is sibcall/normal/eh_return). > Out of these, aarch64 and rs6000 have been fixed (though, the latter maybe > not for powerpc-darwin yet). no, it’s quite a lot more involved to fix that - I would favour the solution above - just punting when calls_eh_return is true in rs6000_ok_for_sibcall, until there’s a chance to try and remove (or improve) the save_world code. At least that’s also a simple immediate fix for any other affected target. Iain > alpha, arc, cr16, csky, frv, m32c, m68k, mmix, msp430, nds32, > nios2, or1k, pa, sh, tilegx, tilepro, visium, xtensa > might be still broken. > > Some of those might be even harder to fix (e.g. alpha, pa, mmix to name a > few), because they don't even bother to pass the sibcall/for_sibcall > argument to epilogue expansion. > > Jakub
Re: [PATCH] Fix aarch64 exception handling (PR c++/59813)
On Mon, May 13, 2019 at 12:52:54PM -0600, Jeff Law wrote: > On 5/10/19 9:17 AM, Jakub Jelinek wrote: > > On Fri, May 10, 2019 at 01:46:07PM +, Wilco Dijkstra wrote: > >> And looking at the generated code, emitting a tailcall for this case is > >> actually a > >> de-optimization since the large eh_return epilog must be repeated for every > >> tailcall. > > > > On x86_64, the code is shorter with the tail call rather than without. > > > > That said, here is actually tested workaround until targets are fixed. > > Richard or Jeff, do we want this workaround? > > > > I don't see why we would need extra testsuite coverage for this, given the > > number of FAILs or bootstrap issues on targets that are broken. > > > > 2019-05-10 Jakub Jelinek > > > > PR c++/59813 > > * unwind.inc (_Unwind_Resume_or_Rethrow): Add optimize attribute > > to temporarily avoid tail calls in the function. > I think my worry would be all the targets that support tail calls, but > which we can't easily test. ie, we put in the hack, but when do we > know it can be removed? > > Is there any property of the code that we can look at instead of a hack > like this? We can test crtl->calls_eh_return if if make sure it is computed early (before expansion), or we could just compute it in the ok_for_sibcall hook if we add caching of that per function (check if any bb that doesn't have successors and is still in GIMPLE IL ends with __builtin_eh_return call), if not already crtl->calls_eh_return. I think only targets that define EH_RETURN_STACKADJ_RTX are problematic, and from those only those that do not use eh_return_internal patterns (e.g. i386 or bfin or riscv have special eh_return_internal patterns that expand epilogue with not just true/false for sibcall/for_sibcall, but either a 3 state argument or two bools whether it is sibcall/normal/eh_return). Out of these, aarch64 and rs6000 have been fixed (though, the latter maybe not for powerpc-darwin yet). alpha, arc, cr16, csky, frv, m32c, m68k, mmix, msp430, nds32, nios2, or1k, pa, sh, tilegx, tilepro, visium, xtensa might be still broken. Some of those might be even harder to fix (e.g. alpha, pa, mmix to name a few), because they don't even bother to pass the sibcall/for_sibcall argument to epilogue expansion. Jakub
Re: [PATCH] [MIPS] Fix PR/target 90357 20080502-1.c -O0 start with r269880
On 5/6/19 2:34 AM, Paul Hua wrote: > The attached patch fix pr90357, bootstraped and regressed test on > mips64el-linux-gnu target. > Ok for commit ? > > > 0001-PATCH-MIPS-Skip-forward-src-into-next-insn-when-the-.patch > > From a3db8763ee8460a5f63c567d58624a985f9924ce Mon Sep 17 00:00:00 2001 > From: Chenghua Xu > Date: Mon, 6 May 2019 16:14:56 +0800 > Subject: [PATCH] [PATCH,MIPS] Skip forward src into next insn when the SRC reg > is dead. > > PR target/90357 > gcc/ > * config/mips/mips.c (mips_split_move): Skip forward SRC into > next insn when the SRC reg is dead. OK. Sorry for the breakage. jeff
Re: [PATCH] Fix aarch64 exception handling (PR c++/59813)
On 5/10/19 9:17 AM, Jakub Jelinek wrote: > On Fri, May 10, 2019 at 01:46:07PM +, Wilco Dijkstra wrote: >> And looking at the generated code, emitting a tailcall for this case is >> actually a >> de-optimization since the large eh_return epilog must be repeated for every >> tailcall. > > On x86_64, the code is shorter with the tail call rather than without. > > That said, here is actually tested workaround until targets are fixed. > Richard or Jeff, do we want this workaround? > > I don't see why we would need extra testsuite coverage for this, given the > number of FAILs or bootstrap issues on targets that are broken. > > 2019-05-10 Jakub Jelinek > > PR c++/59813 > * unwind.inc (_Unwind_Resume_or_Rethrow): Add optimize attribute > to temporarily avoid tail calls in the function. I think my worry would be all the targets that support tail calls, but which we can't easily test. ie, we put in the hack, but when do we know it can be removed? Is there any property of the code that we can look at instead of a hack like this? jeff
Re: malloc cannot alias preexisting pointers
On 5/13/19 11:37 AM, Marc Glisse wrote: On Mon, 13 May 2019, Martin Sebor wrote: On 5/11/19 5:33 PM, Marc Glisse wrote: Hello, this patch lets gcc know that if a pointer existed before the call to malloc, the result of malloc cannot alias it. This is a bit ad hoc and fragile. A small improvement would be, when the 2 statements are in the same bb but in the wrong order, to check if there is any statement in between that might prevent from reordering them. But that's more complicated, and the patch as it is already does help. I.e., given the description of attribute malloc: the pointer P returned by the function cannot alias any other pointer valid when the function returns, and moreover no pointers to valid objects occur in any storage addressed by P. doesn't the same guarantee hold for other functions declared with the attribute (so that the same optimization could be applied to them as well)? The patch tests DECL_IS_MALLOC, not BUILT_IN_MALLOC. From the failures I got with earlier versions, that seems to include alloca at least, so I would expect it also includes any function with the attribute. Very good. May I suggest to add a test case to verify that? Martin
Re: [RFC] SHIFT_COUNT_TRUNCATED and shift_truncation_mask
On 5/13/19 1:32 AM, Richard Sandiford wrote: > > IMO we should only consider deprecating the macro. I'm not sure how > much practical effect that will have though. cc0 was deprecated ages > ago but we're a still a long way from getting rid of it. :-) Yea, but (in theory) removing SHIFT_COUNT_TRUNCATED from a target ought to be easier than removing cc0. jeff
Re: malloc cannot alias preexisting pointers
On Mon, 13 May 2019, Martin Sebor wrote: On 5/11/19 5:33 PM, Marc Glisse wrote: Hello, this patch lets gcc know that if a pointer existed before the call to malloc, the result of malloc cannot alias it. This is a bit ad hoc and fragile. A small improvement would be, when the 2 statements are in the same bb but in the wrong order, to check if there is any statement in between that might prevent from reordering them. But that's more complicated, and the patch as it is already does help. I.e., given the description of attribute malloc: the pointer P returned by the function cannot alias any other pointer valid when the function returns, and moreover no pointers to valid objects occur in any storage addressed by P. doesn't the same guarantee hold for other functions declared with the attribute (so that the same optimization could be applied to them as well)? The patch tests DECL_IS_MALLOC, not BUILT_IN_MALLOC. From the failures I got with earlier versions, that seems to include alloca at least, so I would expect it also includes any function with the attribute. -- Marc Glisse
Re: malloc cannot alias preexisting pointers
On 5/13/19 10:49 AM, Jakub Jelinek wrote: On Mon, May 13, 2019 at 10:36:00AM -0600, Martin Sebor wrote: On 5/11/19 5:33 PM, Marc Glisse wrote: Hello, this patch lets gcc know that if a pointer existed before the call to malloc, the result of malloc cannot alias it. This is a bit ad hoc and fragile. A small improvement would be, when the 2 statements are in the same bb but in the wrong order, to check if there is any statement in between that might prevent from reordering them. But that's more complicated, and the patch as it is already does help. I.e., given the description of attribute malloc: the pointer P returned by the function cannot alias any other pointer valid when the function returns, and moreover no pointers to valid objects occur in any storage addressed by P. doesn't the same guarantee hold for other functions declared with the attribute (so that the same optimization could be applied to them as well)? Doesn't realloc have also the malloc attribute, but the return value can alias preexisting pointers (if no reallocation occurs, just the allocation is extended)? Realloc doesn't have the malloc attribute. Martin
Re: Recent combine change causing regressions
On Mon, May 13, 2019 at 09:38:00AM -0500, Segher Boessenkool wrote: > I tested on 30-something targets (all *-linux), and only mips64 regressed > a little, everything else improved. I meant alpha, btw. 0.08% code size growth. And the biggest winner is i386, with a similar shrink. Most targets shrink a tiny bit, only a few stay the same size, and even fewer grow. Segher
Re: malloc cannot alias preexisting pointers
On Mon, May 13, 2019 at 10:36:00AM -0600, Martin Sebor wrote: > On 5/11/19 5:33 PM, Marc Glisse wrote: > > Hello, > > > > this patch lets gcc know that if a pointer existed before the call to > > malloc, the result of malloc cannot alias it. This is a bit ad hoc and > > fragile. A small improvement would be, when the 2 statements are in the > > same bb but in the wrong order, to check if there is any statement in > > between that might prevent from reordering them. But that's more > > complicated, and the patch as it is already does help. > > I.e., given the description of attribute malloc: > > the pointer P returned by the function cannot alias any other > pointer valid when the function returns, and moreover no pointers > to valid objects occur in any storage addressed by P. > > doesn't the same guarantee hold for other functions declared with > the attribute (so that the same optimization could be applied to > them as well)? Doesn't realloc have also the malloc attribute, but the return value can alias preexisting pointers (if no reallocation occurs, just the allocation is extended)? Jakub
Re: Remove obsolete Solaris 10 support
From: Rainer Orth Date: Mon, May 13, 2019 at 4:39 AM > With the GCC 9.1 release out of the door, this patch implements the > removal of Solaris 10 support that had been obsoleted in GCC 9. ... > The vast majority of the patch falls squarely under my Solaris > maintainership. I'm uncertain about the libbacktrace/configure.ac bit > and will submit the libgo part separately. The libbacktrace change looks fine to me. Ian
Re: malloc cannot alias preexisting pointers
On 5/11/19 5:33 PM, Marc Glisse wrote: Hello, this patch lets gcc know that if a pointer existed before the call to malloc, the result of malloc cannot alias it. This is a bit ad hoc and fragile. A small improvement would be, when the 2 statements are in the same bb but in the wrong order, to check if there is any statement in between that might prevent from reordering them. But that's more complicated, and the patch as it is already does help. I.e., given the description of attribute malloc: the pointer P returned by the function cannot alias any other pointer valid when the function returns, and moreover no pointers to valid objects occur in any storage addressed by P. doesn't the same guarantee hold for other functions declared with the attribute (so that the same optimization could be applied to them as well)? Martin I expect people may not like the call to a function from tree-ssa-loop-niter too much, but it is convenient. And if someone improves it, they will likely have to rewrite something not quite equivalent to stmt_dominates_stmt_p. The testcase uses libstdc++ quite a bit. I thought about putting it in the libstdc++ testsuite, but it does not know scan-tree-dump, and in the assembly it may be too late, memcpy may get expanded to something target-specific. So it is in g++.dg. I could write a more artificial testcase, but the behavior of std::vector is really what I want to test... Bootstrap+regtest on x86_64-pc-linux-gnu. 2019-05-13 Marc Glisse gcc/ * tree-ssa-loop-niter.c (stmt_dominates_stmt_p): Handle NULL basic block. * tree-ssa-alias.c: Include tree-ssa-loop-niter.h. (ptr_derefs_may_alias_p): Detect malloc and an older pointer. gcc/testsuite/ * g++.dg/tree-ssa/ldist-2.C: New file.
Re: Recent combine change causing regressions
On Mon, May 13, 2019 at 11:41:58PM +0900, Oleg Endo wrote: > On Mon, 2019-05-13 at 09:38 -0500, Segher Boessenkool wrote: > > On Mon, May 13, 2019 at 08:27:15AM -0600, Jeff Law wrote: > > > Tests that now fail, but worked before (3 tests): > > > > > > gcc.target/sh/pr51244-11.c scan-assembler-not subc|and|bra > > > gcc.target/sh/pr51244-11.c scan-assembler-times bf\t0f 1 > > > gcc.target/sh/pr51244-11.c scan-assembler-times bt\t0f 1 > Hmmm .. on SH3 TARGET_ZDCBRANCH should be off, afair. The test explicitly uses O1 -mzdcbranch . Segher
[PATCH] A couple of driver cleanups
I'm applying this obvious patch. Two hunks deal with inappropriate line breaks, and the first changes the baroque: var = (thing) ? thing : var; into the much clearer if (thing) var = thing; nathan -- Nathan Sidwell 2019-05-13 Nathan Sidwell gcc.c * (execute): Simplify cond-expr into if. Reformat command. (run_attempt): Reformat line break. Index: gcc.c === --- gcc.c (revision 271130) +++ gcc.c (working copy) @@ -3068,7 +3068,8 @@ execute (void) if (!wrapper_string) { string = find_a_file (_prefixes, commands[0].prog, X_OK, false); - commands[0].argv[0] = (string) ? string : commands[0].argv[0]; + if (string) + commands[0].argv[0] = string; } for (n_commands = 1, i = 0; argbuf.iterate (i, ); i++) @@ -3077,8 +3078,7 @@ execute (void) #if defined (__MSDOS__) || defined (OS2) || defined (VMS) fatal_error (input_location, "%<-pipe%> not supported"); #endif - argbuf[i] = 0; /* Termination of - command args. */ + argbuf[i] = 0; /* Termination of command args. */ commands[n_commands].prog = argbuf[i + 1]; commands[n_commands].argv = &(argbuf.address ())[i + 1]; @@ -6926,8 +6926,8 @@ run_attempt (const char **new_argv, cons fatal_error (input_location, "pex_init failed: %m"); errmsg = pex_run (pex, pex_flags, new_argv[0], - CONST_CAST2 (char *const *, const char **, _argv[1]), out_temp, - err_temp, ); + CONST_CAST2 (char *const *, const char **, _argv[1]), + out_temp, err_temp, ); if (errmsg != NULL) { errno = err;
Re: Recent combine change causing regressions
On Mon, May 13, 2019 at 08:27:15AM -0600, Jeff Law wrote: > > sh3-linux-gnu and sh3eb-linux-gnu: > > > Tests that now fail, but worked before (3 tests): > > gcc.target/sh/pr51244-11.c scan-assembler-not subc|and|bra > gcc.target/sh/pr51244-11.c scan-assembler-times bf\t0f 1 > gcc.target/sh/pr51244-11.c scan-assembler-times bt\t0f 1 --- pr51244-11.s.OK 2019-05-13 15:25:02.283142995 + +++ pr51244-11.s.BAD2019-05-13 15:23:25.875758477 + @@ -12,9 +12,9 @@ mov r4,r0 mov.l @(12,r4),r1 tst r1,r1 - bf 0f - mov #0,r0 -0: + subcr1,r1 + not r1,r1 + and r1,r0 .L2: rts nop @@ -29,9 +29,8 @@ mov r4,r0 mov.l @(12,r4),r1 tst r1,r1 - bt 0f - mov #0,r0 -0: + subcr1,r1 + and r1,r0 .L4: rts nop is the whole diff, fwiw. Segher
Re: Recent change breaking C++ tests
On 5/13/19 9:08 AM, Jonathan Wakely wrote: > On 13/05/19 08:56 -0600, Jeff Law wrote: >> Tester started failing Wattributes1.C on various targets after this >> change: >> >> >>> commit de3f1d9aabb765f78d127696ff9dd0a83b268aa2 (HEAD, refs/bisect/bad) >>> Author: redi >>> Date: Fri May 10 21:41:11 2019 + >>> >>> Improve API docs for and >>> >>> * include/bits/shared_ptr.h: Improve docs. >>> * include/bits/shared_ptr_base.h: Likewise. >>> * include/bits/stl_uninitialized.h: Likewise. >>> * include/bits/unique_ptr.h: Likewise. >>> * libsupc++/new: Likewise. >>> >>> git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@271077 >>> 138bc75d-0d04-0410-961f-82ee72b054a4 >> >> x86_64 native: >> >>> Running target unix >>> FAIL: g++.dg/cpp0x/Wattributes1.C -std=c++14 (test for warnings, >>> line 125) >>> FAIL: g++.dg/cpp0x/Wattributes1.C -std=c++17 (test for warnings, >>> line 125) >>> FAIL: g++.dg/guality/pr55665.C -O2 line 23 p == 40 >> >> >> Grubbing through the logs shows: >> >> >>> In file included from >>> /home/law/gcc-testing/gcc/gcc/testsuite/g++.dg/cpp0x/Wattributes1.C:5:^M >>> /home/law/gcc-testing/gcc/libstdc++-v3/libsupc++/new:126:26: note: >>> previous declaration of 'void* operator new(std::size_t)'^M >> >> Looking at the test: >> >>> // PR c++/60373 >>> // { dg-do compile { target c++11 } } >>> // { dg-require-visibility "" } >>> >>> #include >>> __attribute__((visibility("hidden")))void*operator new(std::size_t); >>> // { dg-warning "visibility attribute ignored" } >>> >>> // { dg-message "previous declaration" "" { target *-*-* } 125 } >>> ~ >> >> >> It looks like we are expecting an error on line 125 that's now occuring >> on 126. Or am I totally off-base here? I'll avoid ranting on whether >> or not it is wise to test for a line # in a header file outside the >> test :-) > > Yeah I have a patch coming. We discussed it on IRC earlier. Sounds good (clearly I'm going through regressions in the tester this morning :-) jeff
[PATCH] Fix rs6000 exception handling (PR target/90418)
Following the same pattern as the AArch64 patch, this patch fixes the PPC32 BE and AIX EH failures after the sibcall changes. This does not address the Darwin failures because of SAVE_WORLD. Bootstrapped on powerpc-ibm-aix7.2.0.0, powerpc64le-unknown-linux-gnu, powerpc64-unknown-linux-gnu, powerpc-unknown-linux-gnu Approved by Segher and me. Thanks, David * config/rs6000/rs6000.c (rs6000_emit_epilogue): Don't load EH data registers in sibcall epilogues. Don't add EH_RETURN_STACKADJ_RTX to sp in sibcall epilogues. Index: rs6000.c === --- rs6000.c(revision 27) +++ rs6000.c(working copy) @@ -28423,7 +28423,7 @@ rs6000_emit_epilogue (int sibcall) restore_saved_lr (0, exit_func); /* Load exception handler data registers, if needed. */ - if (crtl->calls_eh_return) + if (!sibcall && crtl->calls_eh_return) { unsigned int i, regno; @@ -28614,7 +28614,7 @@ rs6000_emit_epilogue (int sibcall) RTX_FRAME_RELATED_P (insn) = 1; } - if (crtl->calls_eh_return) + if (!sibcall && crtl->calls_eh_return) { rtx sa = EH_RETURN_STACKADJ_RTX; emit_insn (gen_add3_insn (sp_reg_rtx, sp_reg_rtx, sa));
Re: Recent change breaking C++ tests
On 13/05/19 08:56 -0600, Jeff Law wrote: Tester started failing Wattributes1.C on various targets after this change: commit de3f1d9aabb765f78d127696ff9dd0a83b268aa2 (HEAD, refs/bisect/bad) Author: redi Date: Fri May 10 21:41:11 2019 + Improve API docs for and * include/bits/shared_ptr.h: Improve docs. * include/bits/shared_ptr_base.h: Likewise. * include/bits/stl_uninitialized.h: Likewise. * include/bits/unique_ptr.h: Likewise. * libsupc++/new: Likewise. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@271077 138bc75d-0d04-0410-961f-82ee72b054a4 x86_64 native: Running target unix FAIL: g++.dg/cpp0x/Wattributes1.C -std=c++14 (test for warnings, line 125) FAIL: g++.dg/cpp0x/Wattributes1.C -std=c++17 (test for warnings, line 125) FAIL: g++.dg/guality/pr55665.C -O2 line 23 p == 40 Grubbing through the logs shows: In file included from /home/law/gcc-testing/gcc/gcc/testsuite/g++.dg/cpp0x/Wattributes1.C:5:^M /home/law/gcc-testing/gcc/libstdc++-v3/libsupc++/new:126:26: note: previous declaration of 'void* operator new(std::size_t)'^M Looking at the test: // PR c++/60373 // { dg-do compile { target c++11 } } // { dg-require-visibility "" } #include __attribute__((visibility("hidden")))void*operator new(std::size_t); // { dg-warning "visibility attribute ignored" } // { dg-message "previous declaration" "" { target *-*-* } 125 } ~ It looks like we are expecting an error on line 125 that's now occuring on 126. Or am I totally off-base here? I'll avoid ranting on whether or not it is wise to test for a line # in a header file outside the test :-) Yeah I have a patch coming. We discussed it on IRC earlier.
[PATCH] Hollerith constant documentation
The Hollerith constants support section in the gfortran manual stated that they can be used with the ASSIGN statement. This is not the case as a compilation error occurs and in any event the value of any such constant would be larger than the largest valid label (9). I've added some extra examples along with other minor changes. I do not have commit privileges so someone else will have to commit this: For the change log: Mark Eggleston * gfortran.texi: Remove reference to the ASSIGN statement, capitalise complex, state that padding is with spaces and modify the Hollerith constant examples. -- https://www.codethink.co.uk/privacy.html >From 8f26259d673708241abcdb57d3f218bd7ebbeb0f Mon Sep 17 00:00:00 2001 From: Mark Eggleston Date: Mon, 13 May 2019 15:30:07 +0100 Subject: [PATCH] Update documentation for Hollerith Constants --- gcc/fortran/gfortran.texi | 20 +++- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi index e2a1d7ccfab..c887e7d1a42 100644 --- a/gcc/fortran/gfortran.texi +++ b/gcc/fortran/gfortran.texi @@ -1923,13 +1923,13 @@ in I/O operations. @cindex Hollerith constants GNU Fortran supports Hollerith constants in assignments, function -arguments, and @code{DATA} and @code{ASSIGN} statements. A Hollerith -constant is written as a string of characters preceded by an integer -constant indicating the character count, and the letter @code{H} or -@code{h}, and stored in bytewise fashion in a numeric (@code{INTEGER}, -@code{REAL}, or @code{complex}) or @code{LOGICAL} variable. The -constant will be padded or truncated to fit the size of the variable in -which it is stored. +arguments, and @code{DATA} statements. A Hollerith constant is written +as a string of characters preceded by an integer constant indicating the +character count, and the letter @code{H} or @code{h}, and stored in +bytewise fashion in a numeric (@code{INTEGER}, @code{REAL}, or +@code{COMPLEX}) or @code{LOGICAL} variable. The constant will be padded +with spaces or truncated to fit the size of the variable in which it is +stored. Examples of valid uses of Hollerith constants: @smallexample @@ -1939,11 +1939,13 @@ Examples of valid uses of Hollerith constants: call foo (4h abc) @end smallexample -Invalid Hollerith constants examples: +Examples of Hollerith constants: @smallexample integer*4 a + a = 0H ! Invalid, at least one character is needed. + a = 4HAB12 ! Valid a = 8H12345678 ! Valid, but the Hollerith constant will be truncated. - a = 0H ! At least one character is needed. + a = 3Hxyz ! Valid, but the Hollerith constant will be padded. @end smallexample In general, Hollerith constants were used to provide a rudimentary -- 2.11.0
Recent change breaking C++ tests
Tester started failing Wattributes1.C on various targets after this change: > commit de3f1d9aabb765f78d127696ff9dd0a83b268aa2 (HEAD, refs/bisect/bad) > Author: redi > Date: Fri May 10 21:41:11 2019 + > > Improve API docs for and > > * include/bits/shared_ptr.h: Improve docs. > * include/bits/shared_ptr_base.h: Likewise. > * include/bits/stl_uninitialized.h: Likewise. > * include/bits/unique_ptr.h: Likewise. > * libsupc++/new: Likewise. > > git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@271077 > 138bc75d-0d04-0410-961f-82ee72b054a4 x86_64 native: > Running target unix > FAIL: g++.dg/cpp0x/Wattributes1.C -std=c++14 (test for warnings, line 125) > FAIL: g++.dg/cpp0x/Wattributes1.C -std=c++17 (test for warnings, line 125) > FAIL: g++.dg/guality/pr55665.C -O2 line 23 p == 40 Grubbing through the logs shows: > In file included from > /home/law/gcc-testing/gcc/gcc/testsuite/g++.dg/cpp0x/Wattributes1.C:5:^M > /home/law/gcc-testing/gcc/libstdc++-v3/libsupc++/new:126:26: note: previous > declaration of 'void* operator new(std::size_t)'^M Looking at the test: > // PR c++/60373 > // { dg-do compile { target c++11 } } > // { dg-require-visibility "" } > > #include > __attribute__((visibility("hidden")))void*operator new(std::size_t); // { > dg-warning "visibility attribute ignored" } > > // { dg-message "previous declaration" "" { target *-*-* } 125 } > ~ It looks like we are expecting an error on line 125 that's now occuring on 126. Or am I totally off-base here? I'll avoid ranting on whether or not it is wise to test for a line # in a header file outside the test :-) jeff
Re: Recent combine change causing regressions
On 5/13/19 8:38 AM, Segher Boessenkool wrote: > On Mon, May 13, 2019 at 08:27:15AM -0600, Jeff Law wrote: >> sh3-linux-gnu and sh3eb-linux-gnu: > > I test sh2 and sh4, but not sh3 :-) > >> Tests that now fail, but worked before (3 tests): >> >> gcc.target/sh/pr51244-11.c scan-assembler-not subc|and|bra >> gcc.target/sh/pr51244-11.c scan-assembler-times bf\t0f 1 >> gcc.target/sh/pr51244-11.c scan-assembler-times bt\t0f 1 >> >> Previously we'd match this pattern: >> >> (define_insn "*cset_zero" >> [(set (match_operand:SI 0 "arith_reg_dest" "=r") >> (if_then_else:SI (match_operand:SI 1 "cbranch_treg_value") >> (match_operand:SI 2 "arith_reg_operand" "0") >> (const_int 0)))] >> "TARGET_SH1 && TARGET_ZDCBRANCH" >> >> After your change we no longer try to do so. >> >> I really don't care about the SH port. But isn't this really a symptom >> of a larger problem. Namely that by not generating if-then-else you've >> hosed every target that implements conditional moves via if-then-else >> constructs? > > I tested on 30-something targets (all *-linux), and only mips64 regressed > a little, everything else improved. So the current tuning is better than > what it was before. No doubt it can be improved though! > > This is only if-then-else for a single bit, fwiw. So are other targets still generating conditional moves? If so the fix may ultimately be to rewrite that pattern in the SH backend. > > I'll build some sh3-linux if I find a cycle or two. Thanks. It does reproduce with a cross. In fact, you just need the compiler -- you don't need an assembler, binutils, headers, etc :-) jeff
Re: Recent combine change causing regressions
On Mon, 2019-05-13 at 09:38 -0500, Segher Boessenkool wrote: > On Mon, May 13, 2019 at 08:27:15AM -0600, Jeff Law wrote: > > sh3-linux-gnu and sh3eb-linux-gnu: > > I test sh2 and sh4, but not sh3 :-) > > > Tests that now fail, but worked before (3 tests): > > > > gcc.target/sh/pr51244-11.c scan-assembler-not subc|and|bra > > gcc.target/sh/pr51244-11.c scan-assembler-times bf\t0f 1 > > gcc.target/sh/pr51244-11.c scan-assembler-times bt\t0f 1 > > > > Previously we'd match this pattern: > > > > (define_insn "*cset_zero" > > [(set (match_operand:SI 0 "arith_reg_dest" "=r") > > (if_then_else:SI (match_operand:SI 1 "cbranch_treg_value") > > (match_operand:SI 2 "arith_reg_operand" > > "0") > > (const_int 0)))] > > "TARGET_SH1 && TARGET_ZDCBRANCH" > > > > After your change we no longer try to do so. > > > > I really don't care about the SH port. But isn't this really a > > symptom > > of a larger problem. Namely that by not generating if-then-else > > you've > > hosed every target that implements conditional moves via if-then- > > else > > constructs? > > I tested on 30-something targets (all *-linux), and only mips64 > regressed > a little, everything else improved. So the current tuning is better > than > what it was before. No doubt it can be improved though! > > This is only if-then-else for a single bit, fwiw. > > I'll build some sh3-linux if I find a cycle or two. > Hmmm .. on SH3 TARGET_ZDCBRANCH should be off, afair. What would be the alternative now for the if-then-else? Cheers, Oleg
Re: Recent combine change causing regressions
On Mon, May 13, 2019 at 08:27:15AM -0600, Jeff Law wrote: > sh3-linux-gnu and sh3eb-linux-gnu: I test sh2 and sh4, but not sh3 :-) > Tests that now fail, but worked before (3 tests): > > gcc.target/sh/pr51244-11.c scan-assembler-not subc|and|bra > gcc.target/sh/pr51244-11.c scan-assembler-times bf\t0f 1 > gcc.target/sh/pr51244-11.c scan-assembler-times bt\t0f 1 > > Previously we'd match this pattern: > > (define_insn "*cset_zero" > [(set (match_operand:SI 0 "arith_reg_dest" "=r") > (if_then_else:SI (match_operand:SI 1 "cbranch_treg_value") > (match_operand:SI 2 "arith_reg_operand" "0") > (const_int 0)))] > "TARGET_SH1 && TARGET_ZDCBRANCH" > > After your change we no longer try to do so. > > I really don't care about the SH port. But isn't this really a symptom > of a larger problem. Namely that by not generating if-then-else you've > hosed every target that implements conditional moves via if-then-else > constructs? I tested on 30-something targets (all *-linux), and only mips64 regressed a little, everything else improved. So the current tuning is better than what it was before. No doubt it can be improved though! This is only if-then-else for a single bit, fwiw. I'll build some sh3-linux if I find a cycle or two. Segher
Recent combine change causing regressions
sh3-linux-gnu and sh3eb-linux-gnu: Tests that now fail, but worked before (3 tests): gcc.target/sh/pr51244-11.c scan-assembler-not subc|and|bra gcc.target/sh/pr51244-11.c scan-assembler-times bf\t0f 1 gcc.target/sh/pr51244-11.c scan-assembler-times bt\t0f 1 Previously we'd match this pattern: (define_insn "*cset_zero" [(set (match_operand:SI 0 "arith_reg_dest" "=r") (if_then_else:SI (match_operand:SI 1 "cbranch_treg_value") (match_operand:SI 2 "arith_reg_operand" "0") (const_int 0)))] "TARGET_SH1 && TARGET_ZDCBRANCH" After your change we no longer try to do so. I really don't care about the SH port. But isn't this really a symptom of a larger problem. Namely that by not generating if-then-else you've hosed every target that implements conditional moves via if-then-else constructs? Jeff
Re: malloc cannot alias preexisting pointers
On Mon, 13 May 2019, Richard Biener wrote: On Sun, May 12, 2019 at 2:51 PM Marc Glisse wrote: On Sun, 12 May 2019, Richard Sandiford wrote: Marc Glisse writes: Hello, this patch lets gcc know that if a pointer existed before the call to malloc, the result of malloc cannot alias it. This is a bit ad hoc and fragile. A small improvement would be, when the 2 statements are in the same bb but in the wrong order, to check if there is any statement in between that might prevent from reordering them. But that's more complicated, and the patch as it is already does help. I expect people may not like the call to a function from tree-ssa-loop-niter too much, but it is convenient. And if someone improves it, they will likely have to rewrite something not quite equivalent to stmt_dominates_stmt_p. It has linear complexity for statements in the same block though. Ah, true. I should anyway test that the second statement is malloc (cheaper) before checking for domination, but even then that could be used to create a quadratic behavior :-( I also think the oracle queries shouldn't encompany such expensive pieces... Well, it is only expensive because finding the order of statements in a basic block is expensive. Would it be better if it only did this check if we are in one of a very limited set of passes where statements have reliable UIDs? Maybe that's not enough passes, loop-distribution uses UIDs but I didn't check if it uses them in a way compatible with this use... What if I only check basic-block domination and punt when the statements are in the same basic block? That seems cheap enough, and would already work for the vector testcase. (reassoc_stmt_dominates_stmt_p avoids that, but relies on uids being up-to-date.) This function is called from all over the place. Unless there is a simple test to check if uids are safe to use (reassoc_in_progress?), that's going to be a problem. I find it surprising that this information is so hard to get to... Stopping stmt_dominates_stmt_p after walking 128 statements doesn't feel like a great solution. But without controlling the pass where this happens, I don't see any good solution. It would be great if that non-aliasing property could be recorded in the points-to information, then I could set it from a pass I control, but I somehow got the impression that it wouldn't work. Maybe I should try to understand PTA better to make sure. In princple PTA should know the aliasing cannot happen but possibly the info is either lost or the IL is too obfuscated at the point it gets computed. (hello C++...) We don't need much obfuscation for this, a simple C program int g; int*f(int**p){ int*q=*p; int*r=__builtin_malloc(4); *q=1; *r=2; g=*q; return r; } gives q_4 = *p_3(D); r_6 = __builtin_malloc (4); *q_4 = 1; *r_6 = 2; _1 = *q_4; g = _1; return r_6; where we clearly don't manage to prove that q and r don't alias. So at ldist time I see # PT = null { D.47989 } (escaped, escaped heap) # ALIGN = 8, MISALIGN = 0 # USE = nonlocal null { D.47989 } (escaped, escaped heap) # CLB = nonlocal null { D.47989 } (escaped, escaped heap) _27 = malloc (8192); good. The interesting loop is the following where the allocation PTA is good and _4 intersect because they include ESCAPED. This is because _27 itself escapes and PTA not being flow-sensitive. I don't see an easy way to improve things here but dislike the stmt walking very much. That is, the proper solution is to re-write PTA in some way. I am trying to imagine what that might look like. I imagine that instead of having a single "escaped" set, we would have multiple escaped sets at different points in the function (at most one per VDEF?). Then _4 would only contain escaped3 while heap5 (*_27) only appears in escaped9 and later? It may be tricky to keep a linear-ish complexity with anything more powerful than the current PTA. I don't know what LLVM is doing, but they seem to manage. -- Marc Glisse
Re: [PATCH, OpenACC, libgomp, v6, stage1] Async-rework update
On 2019/2/26 9:51 PM, Thomas Schwinge wrote: On Tue, 26 Feb 2019 01:49:09 +0800, Chung-Lin Tang wrote: I have incorporated all your patches you've included in the last mail (with some modifications, though pretty minor I think). OK, thanks, that's good for next GCC development stage 1 as far as I'm concerned, and Tom has already approved the libgomp 'nvptx' plugin changes, and I suppose you've addressed Jakub's requests -- so you're good to go!:-) Thanks Thomas, just committed. I also remembered to add your Reviewed-By tag in the commit log. Thanks, Chung-Lin
Re: [PATCH][RFC] Come up with TARGET_HAS_FAST_MEMPCPY_ROUTINE (PR middle-end/90263).
On Mon, May 13, 2019 at 12:14:37PM +0200, Martin Liška wrote: > On 5/10/19 11:21 AM, Jakub Jelinek wrote: > > On Fri, May 10, 2019 at 11:04:12AM +0200, Martin Liška wrote: > >> --- a/gcc/config/i386/i386.h > >> +++ b/gcc/config/i386/i386.h > >> @@ -1906,6 +1906,9 @@ typedef struct ix86_args { > >> > >> #define CLEAR_RATIO(speed) ((speed) ? MIN (6, ix86_cost->move_ratio) : 2) > >> > >> +/* C library provides fast implementation of mempcpy function. */ > >> +#define TARGET_HAS_FAST_MEMPCPY_ROUTINE 1 > >> + > > > > 1) we shouldn't be adding further target macros, but target hooks > > Done. > > > 2) I don't think this is a property of the x86 target, but of x86 glibc, > >so you should set it on x86 glibc only (i.e. i?86/x86_64 linux and hurd > >when using glibc, not newlib, nor bionic/android, nor uclibc, nor musl) > > I've implemented the in i386.c with DEFAULT_LIBC == LIBC_GLIBC. Hope it's > correct? No, that would be correct only in the rare SINGLE_LIBC configurations. Either you can do #ifdef OPTION_GLIBC return OPTION_GLIBC; #else return false; #endif or define something in config/linux.h (or .[ch]) that you can then use in i386.c. Jakub
Re: [PATCH][RFC] Come up with TARGET_HAS_FAST_MEMPCPY_ROUTINE (PR middle-end/90263).
Hi Martin, +++ b/gcc/testsuite/gcc.c-torture/pr90263.c @@ -0,0 +1,9 @@ +/* PR middle-end/90263 */ + +int *f (int *p, int *q, long n) +{ + return __builtin_mempcpy (p, q, n); +} + +/* { dg-final { scan-assembler "mempcpy" { target { i?86-*-* x86_64-*-* } } } } */ +/* { dg-final { scan-assembler "memcpy" { target { ! { i?86-*-* x86_64-*-* } } } } } */ This should be gcc/testsuite/gcc.c-torture/compile/pr90263.c, otherwise it is ignored. The scan-assembler would also need to check for GLIBC unless you always skip it on x86 (we don't seem to have a check_effective_target_glibc, maybe it's worth adding?). Wilco
Re: [PATCH GCC]Simplify if-then-else in slsr cand_vect
On Mon, May 13, 2019 at 1:39 PM bin.cheng wrote: > > Hi, > While working on other PR, I noticed that we can save lots of if-then-else in > accessing > cand_vec by placing an additional NULL element at front of it. > > Bootstrap and test on x86_64. Is it OK? Nice. OK. Richard. > Thanks, > bin > > 2019-05-13 Bin Cheng > > * gimple-ssa-strength-reduction.c (lookup_cand): Adjust index by 1. > (alloc_cand_and_find_basis): Ditto. > (backtrace_base_for_ref, create_mul_ssa_cand): Remove if-then-else. > (create_mul_imm_cand, create_add_ssa_cand): Ditto. > (create_add_imm_cand, slsr_process_cast): Ditto. > (slsr_process_copy, replace_mult_candidate): Ditto. > (replace_rhs_if_not_dup, replace_one_candidate): Ditto. > (dump_cand_vec, analyze_candidates_and_replace): Skip NULL element. > (pass_strength_reduction::execute): Init the first NULL element.
Re: [PATCH] Improve API docs for and
On 13/05/19 14:02 +0200, Christophe Lyon wrote: Hi Jonathan, On Fri, 10 May 2019 at 23:41, Jonathan Wakely wrote: More Doxygenation. Tested powerpc64le-linux. Committed to trunk. You've probably noticed by now, but this introduced a regression (!): FAIL: g++.dg/cpp0x/Wattributes1.C -std=c++14 (test for warnings, line 125) Erm, yes, let's pretend I'd run the compiler testsuite and noticed by now, shall we? :-) This is because you added a line to libstdc++-v3/libsupc++/new and the warning now appears at line 126 I can certainly patch the test and replace 125 with 126 in it, I'm just not sure want to keep this fragile dependency? Yeah, I hate dg-error line numbers that refer into some arbitrary line of a header. I'll take care of it, thanks for pointing it out.
Re: [PATCH] Improve API docs for and
Hi Jonathan, On Fri, 10 May 2019 at 23:41, Jonathan Wakely wrote: > > More Doxygenation. > > Tested powerpc64le-linux. Committed to trunk. > > You've probably noticed by now, but this introduced a regression (!): FAIL: g++.dg/cpp0x/Wattributes1.C -std=c++14 (test for warnings, line 125) This is because you added a line to libstdc++-v3/libsupc++/new and the warning now appears at line 126 I can certainly patch the test and replace 125 with 126 in it, I'm just not sure want to keep this fragile dependency? Christophe
[wwwdocs] Document PR libstdc++/68210 (LWG DR 206) changes
Some notes for /gcc-9/porting_to.html (I haven't seen this change cause any issues in practice, but it doesn't hurt to document it). Committed to CVS. Index: htdocs/gcc-9/porting_to.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-9/porting_to.html,v retrieving revision 1.4 diff -u -r1.4 porting_to.html --- htdocs/gcc-9/porting_to.html 17 Apr 2019 18:32:41 - 1.4 +++ htdocs/gcc-9/porting_to.html 13 May 2019 11:55:25 - @@ -111,9 +111,51 @@ } - + + + operator new(size_t, nothrow_t) calls + operator new(size_t) + + + +GCC 9 implements the requirement introduced in the C++ 2011 standard that +the nothrow version of operator new calls the +ordinary, throwing version of operator new (and catches any +exception and instead returns a null pointer). +This was changed by https://wg21.link/lwg206;>DR 206 to ensure +that the various forms of operator new do not become decoupled +if a user only replaces the ordinary operator new. + + + +Code that only replaces one version of operator new and expects +the other versions to be unaffected might change behaviour when using GCC 9. + + + +If your program uses a replacement operator new(size_t, nothrow_t) +then it must also replace operator new(size_t) and +operator delete(void*), and ensure memory obtained from the +nothrow version of new can be freed by the ordinary +version of operator delete. + + + +The simplest solution is to only replace the ordinary +operator new(size_t) and +operator delete(void*) +functions, and the replaced versions will be used by all of +operator new(size_t, nothrow_t), +operator new[](size_t) and +operator new[](size_t, nothrow_t) +and the corresponding operator delete functions. +To support types with extended alignment you may also need to replace +operator new(size_t, std::align_val_t) and +operator delete(void*, std::align_val_t) +(which will then be used by the nothrow and array forms for +extended alignments). +
Re: [PATCH] [aarch64] Introduce flags for SVE2.
Sorry for the slow reply. Matthew Malcomson writes: > @@ -326,16 +326,18 @@ int opt_ext_cmp (const void* a, const void* b) > turns on as a dependency. As an example +dotprod turns on FL_DOTPROD > and > FL_SIMD. As such the set of bits represented by this option is > {FL_DOTPROD, FL_SIMD}. */ > - unsigned long total_flags_a = opt_a->flag_canonical & opt_a->flags_on; > - unsigned long total_flags_b = opt_b->flag_canonical & opt_b->flags_on; > + uint64_t total_flags_a = opt_a->flag_canonical & opt_a->flags_on; > + uint64_t total_flags_b = opt_b->flag_canonical & opt_b->flags_on; >int popcnt_a = popcount_hwi ((HOST_WIDE_INT)total_flags_a); >int popcnt_b = popcount_hwi ((HOST_WIDE_INT)total_flags_b); >int order = popcnt_b - popcnt_a; > >/* If they have the same amount of bits set, give it a more > - deterministic ordering by using the value of the bits themselves. */ > + deterministic ordering by using the value of the bits themselves. > + Since the value of the bits themselves can be larger than that > + representable by an integer, we manually truncate the value. */ >if (order == 0) > -return total_flags_b - total_flags_a; > +return (total_flags_b - total_flags_a) & INT_MAX; This means that we return 1 if the flags differ in the low 31 bits and 0 otherwise. I think we should use: return total_flags_a < total_flags_b ? 1 : -1; > diff --git a/gcc/config/aarch64/aarch64-option-extensions.def > b/gcc/config/aarch64/aarch64-option-extensions.def > index > 53dcd03590d2e4eebac83f03039c442fca7f5d5d..e4f56ac5fa08464d67750455ee6476a0e4ce1735 > 100644 > --- a/gcc/config/aarch64/aarch64-option-extensions.def > +++ b/gcc/config/aarch64/aarch64-option-extensions.def > @@ -57,17 +57,20 @@ > > /* Enabling "fp" just enables "fp". > Disabling "fp" also disables "simd", "crypto", "fp16", "aes", "sha2", > - "sha3", sm3/sm4 and "sve". */ > -AARCH64_OPT_EXTENSION("fp", AARCH64_FL_FP, 0, AARCH64_FL_SIMD | > AARCH64_FL_CRYPTO | AARCH64_FL_F16 | AARCH64_FL_AES | AARCH64_FL_SHA2 | > AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE, false, "fp") > + "sha3", sm3/sm4, "sve", "sve2-sm4", "sve2-sha3", "sve2-aes", "bitperm" and > + "sve2". */ > +AARCH64_OPT_EXTENSION("fp", AARCH64_FL_FP, 0, AARCH64_FL_SIMD | > AARCH64_FL_CRYPTO | AARCH64_FL_F16 | AARCH64_FL_AES | AARCH64_FL_SHA2 | > AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE | AARCH64_FL_SVE2 | > AARCH64_FL_SVE2AES | AARCH64_FL_SVE2SHA3 | AARCH64_FL_SVE2SM4 | > AARCH64_FL_SVE2BITPERM, false, "fp") > > /* Enabling "simd" also enables "fp". > Disabling "simd" also disables "crypto", "dotprod", "aes", "sha2", "sha3", > - "sm3/sm4" and "sve". */ > -AARCH64_OPT_EXTENSION("simd", AARCH64_FL_SIMD, AARCH64_FL_FP, > AARCH64_FL_CRYPTO | AARCH64_FL_DOTPROD | AARCH64_FL_AES | AARCH64_FL_SHA2 | > AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE, false, "asimd") > + "sm3/sm4", "sve", "sve2-sm4", "sve2-sha3", "sve2-aes", "bitperm" and > "sve2". > + */ > +AARCH64_OPT_EXTENSION("simd", AARCH64_FL_SIMD, AARCH64_FL_FP, > AARCH64_FL_CRYPTO | AARCH64_FL_DOTPROD | AARCH64_FL_AES | AARCH64_FL_SHA2 | > AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE | AARCH64_FL_SVE2 | > AARCH64_FL_SVE2AES | AARCH64_FL_SVE2SHA3 | AARCH64_FL_SVE2SM4 | > AARCH64_FL_SVE2BITPERM, false, "asimd") > > /* Enabling "crypto" also enables "fp", "simd", "aes" and "sha2". > - Disabling "crypto" disables "crypto", "aes", "sha2", "sha3" and > "sm3/sm4". */ > -AARCH64_OPT_EXTENSION("crypto", AARCH64_FL_CRYPTO, AARCH64_FL_FP | > AARCH64_FL_SIMD | AARCH64_FL_AES | AARCH64_FL_SHA2, AARCH64_FL_AES | > AARCH64_FL_SHA2 |AARCH64_FL_SHA3 | AARCH64_FL_SM4, true, "aes pmull sha1 > sha2") > + Disabling "crypto" disables "crypto", "aes", "sha2", "sha3" and "sm3/sm4", > + "sve2-aes", "sve2-sm4", "sve2-sha3". */ > +AARCH64_OPT_EXTENSION("crypto", AARCH64_FL_CRYPTO, AARCH64_FL_FP | > AARCH64_FL_SIMD | AARCH64_FL_AES | AARCH64_FL_SHA2, AARCH64_FL_AES | > AARCH64_FL_SHA2 | AARCH64_FL_SHA3 | AARCH64_FL_SM4 | AARCH64_FL_SVE2AES | > AARCH64_FL_SVE2SHA3 | AARCH64_FL_SVE2SM4, true, "aes pmull sha1 sha2") > > /* Enabling or disabling "crc" only changes "crc". */ > AARCH64_OPT_EXTENSION("crc", AARCH64_FL_CRC, 0, 0, false, "crc32") > @@ -76,8 +79,9 @@ AARCH64_OPT_EXTENSION("crc", AARCH64_FL_CRC, 0, 0, false, > "crc32") > AARCH64_OPT_EXTENSION("lse", AARCH64_FL_LSE, 0, 0, false, "atomics") > > /* Enabling "fp16" also enables "fp". > - Disabling "fp16" disables "fp16", "fp16fml" and "sve". */ > -AARCH64_OPT_EXTENSION("fp16", AARCH64_FL_F16, AARCH64_FL_FP, > AARCH64_FL_F16FML | AARCH64_FL_SVE, false, "fphp asimdhp") > + Disabling "fp16" disables "fp16", "fp16fml", "sve", "sve2-sm4", > "sve2-sha3", > + "sve2-aes", "bitperm" and "sve2". */ > +AARCH64_OPT_EXTENSION("fp16", AARCH64_FL_F16, AARCH64_FL_FP, > AARCH64_FL_F16FML | AARCH64_FL_SVE | AARCH64_FL_SVE2 | AARCH64_FL_SVE2AES | >
Re: Remove obsolete Solaris 10 support
Hi Jonathan, > On 13/05/19 13:39 +0200, Rainer Orth wrote: >> libstdc++-v3: >> * config/os/solaris/solaris2.10: Move to ... >> * config/os/solaris: ... this. >> * configure.host (os_include_dir): Adapt. >> (abi_baseline_pair): Remove Solaris 10 handling. >> * config/abi/post/i386-solaris2.10: Remove. >> * config/abi/post/sparc-solaris2.10: Remove. >> * config/abi/post/i386-solaris2.11: Rename to ... >> * config/abi/post/i386-solaris: ... this. >> * config/abi/post/sparc-solaris2.11: Rename to ... >> * config/abi/post/sparc-solaris: ... this. >> >> * libsupc++/new_opa.cc [_GLIBCXX_HAVE_MEMALIGN] [__sun__]: Remove >> workaround. > > It's not important, but the actual condition here uses __sun not > __sun__. thanks for catching this. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Remove obsolete Solaris 10 libgo support
Here's the libgo part of the Solaris 10 removal patch. I wonder if the cleanup is worthwhile given the small size of the patch. Bootstrapped without regressions on i386-pc-solaris2.11 and sparc-sun-solaris2.11 together with the main patch. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University 2019-05-11 Rainer Orth libgo: * configure.ac (OSCFLAGS): Simplify Solaris triplet. * configure: Regenerate. * mksysinfo.sh (Pw_uid, Pw_gid): Remove Solaris 10 workaround. * go/runtime/signal_gccgo.go (sigcode): Remove Solaris 10 workaround. # HG changeset patch # Parent 51af0604e57ec839b47bfaff79f7d1877a70f6a1 Remove obsolete Solaris 10 libgo support diff --git a/libgo/configure.ac b/libgo/configure.ac --- a/libgo/configure.ac +++ b/libgo/configure.ac @@ -397,7 +397,7 @@ case "$target" in # msghdr in . OSCFLAGS="$OSCFLAGS -D_XOPEN_SOURCE=500" ;; -*-*-solaris2.1[[01]]) +*-*-solaris2.*) # Solaris 10+ needs this so struct msghdr gets the msg_control # etc. fields in (_XPG4_2). _XOPEN_SOURCE=600 as # above doesn't work with C99. diff --git a/libgo/go/runtime/signal_gccgo.go b/libgo/go/runtime/signal_gccgo.go --- a/libgo/go/runtime/signal_gccgo.go +++ b/libgo/go/runtime/signal_gccgo.go @@ -60,11 +60,6 @@ type sigctxt struct { } func (c *sigctxt) sigcode() uint64 { - if c.info == nil { - // This can happen on Solaris 10. We don't know the - // code, just avoid a misleading value. - return _SI_USER + 1 - } return uint64(c.info.si_code) } diff --git a/libgo/mksysinfo.sh b/libgo/mksysinfo.sh --- a/libgo/mksysinfo.sh +++ b/libgo/mksysinfo.sh @@ -734,16 +734,6 @@ if ! grep "const EAI_OVERFLOW " ${OUT} > echo "const EAI_OVERFLOW = 0" >> ${OUT} fi -# The passwd struct. -# Force uid and gid from int32 to uint32 for consistency; they are -# int32 on Solaris 10 but uint32 everywhere else including Solaris 11. -grep '^type _passwd ' gen-sysinfo.go | \ -sed -e 's/_passwd/Passwd/' \ - -e 's/ pw_/ Pw_/g' \ - -e 's/ Pw_uid int32/ Pw_uid uint32/' \ - -e 's/ Pw_gid int32/ Pw_gid uint32/' \ ->> ${OUT} - # The group struct. grep '^type _group ' gen-sysinfo.go | \ sed -e 's/_group/Group/' \
Re: Remove obsolete Solaris 10 support
On 13/05/19 13:39 +0200, Rainer Orth wrote: libstdc++-v3: * config/os/solaris/solaris2.10: Move to ... * config/os/solaris: ... this. * configure.host (os_include_dir): Adapt. (abi_baseline_pair): Remove Solaris 10 handling. * config/abi/post/i386-solaris2.10: Remove. * config/abi/post/sparc-solaris2.10: Remove. * config/abi/post/i386-solaris2.11: Rename to ... * config/abi/post/i386-solaris: ... this. * config/abi/post/sparc-solaris2.11: Rename to ... * config/abi/post/sparc-solaris: ... this. * libsupc++/new_opa.cc [_GLIBCXX_HAVE_MEMALIGN] [__sun__]: Remove workaround. It's not important, but the actual condition here uses __sun not __sun__. The libstdc++ parts are of course welcome!
[PATCH] Handle VIEW_CONVERT_EXPR in SLP vectorization
Another pick from the branch. Bootstrapped / tested on x86_64-unknown-linux-gnu, applied. Richard. >From 7a2cf96fb0846d1fde52e11f9c1501f1ae70a94d Mon Sep 17 00:00:00 2001 From: Richard Guenther Date: Mon, 21 Jan 2019 16:15:02 +0100 Subject: [PATCH] add-slp-vce-support * tree-vect-slp.c (vect_get_and_check_slp_defs): Handle VIEW_CONVERT_EXPR. (vect_build_slp_tree_1): Likewise. diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c index 2a1e5b83e53..52c7b47d809 100644 --- a/gcc/tree-vect-slp.c +++ b/gcc/tree-vect-slp.c @@ -333,7 +333,7 @@ vect_get_and_check_slp_defs (vec_info *vinfo, unsigned char *swap, number_of_oprnds = gimple_num_ops (stmt) - 1; /* Swap can only be done for cond_expr if asked to, otherwise we could result in different comparison code to the first stmt. */ - if (gimple_assign_rhs_code (stmt) == COND_EXPR + if (code == COND_EXPR && COMPARISON_CLASS_P (gimple_assign_rhs1 (stmt))) { first_op_cond = true; @@ -364,6 +364,8 @@ again: } else oprnd = gimple_op (stmt_info->stmt, first_op_idx + (swapped ? !i : i)); + if (TREE_CODE (oprnd) == VIEW_CONVERT_EXPR) + oprnd = TREE_OPERAND (oprnd, 0); oprnd_info = (*oprnds_info)[i]; @@ -907,6 +909,7 @@ vect_build_slp_tree_1 (unsigned char *swap, && TREE_CODE_CLASS (rhs_code) != tcc_unary && TREE_CODE_CLASS (rhs_code) != tcc_expression && TREE_CODE_CLASS (rhs_code) != tcc_comparison + && rhs_code != VIEW_CONVERT_EXPR && rhs_code != CALL_EXPR) { if (dump_enabled_p ())
Remove obsolete Solaris 10 support
With the GCC 9.1 release out of the door, this patch implements the removal of Solaris 10 support that had been obsoleted in GCC 9. The patch is mostly straightforward: * removing Solaris 10-only code and documentation, * simplifying configure triplets, using *-*-solaris2* everywhere since this effectively means *-*-solaris2.11* now, and * simplifying the v3 Solaris baselines and configuration since there's now only a single one left. (Just FTR, I've omitted the baseline renames and removals from the patch below to avoid blowing it up to insane sizes.) Bootstrapped without regressions on i386-pc-solaris2.11 and sparc-sun-solaris2.11 and tested on i386-pc-solaris2.10 to make sure the configuration is properly rejected. I've also inspected the resulting gccinstall.info and gccinstall.pdf. The vast majority of the patch falls squarely under my Solaris maintainership. I'm uncertain about the libbacktrace/configure.ac bit and will submit the libgo part separately. There are primarily two areas left with Solaris 10 references in them: * fixincludes/inclhack.def: I'll have to check in great detail which of the fixes apply to Solaris 10 only and can thus be removed. * libgcc/config/sparc/sol2-unwind.h: Again, it's difficult to tell which cases only applied to Solaris 10 and can go. I'll give maintainers a day or two to comment before committing. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University 2019-05-10 Rainer Orth libstdc++-v3: * config/os/solaris/solaris2.10: Move to ... * config/os/solaris: ... this. * configure.host (os_include_dir): Adapt. (abi_baseline_pair): Remove Solaris 10 handling. * config/abi/post/i386-solaris2.10: Remove. * config/abi/post/sparc-solaris2.10: Remove. * config/abi/post/i386-solaris2.11: Rename to ... * config/abi/post/i386-solaris: ... this. * config/abi/post/sparc-solaris2.11: Rename to ... * config/abi/post/sparc-solaris: ... this. * libsupc++/new_opa.cc [_GLIBCXX_HAVE_MEMALIGN] [__sun__]: Remove workaround. * testsuite/ext/enc_filebuf/char/13598.cc: Remove *-*-solaris2.10 xfail. libsanitizer: * configure.ac (have_dl_iterate_phdr): Remove *-*-solaris2.10* handling. * configure: Regenerate. libgcc: * config.host: Simplify various *-*-solaris2.1[0-9]* to *-*-solaris2*. * configure.ac: Likewise. * configure: Regenerate. * config/i386/sol2-unwind.h (x86_fallback_frame_state): Remove Solaris 10 and Solaris 11 < snv_125 handling. libbacktrace: * configure.ac (have_dl_iterate_phdr): Remove *-*-solaris2.10* handling. * configure: Regenerate. gcc/testsuite: * gcc.dg/atomic/c11-atomic-exec-4.c: Simplify triplet to *-*-solaris2*. * gcc.dg/atomic/c11-atomic-exec-5.c: Likewise. * gcc.dg/c99-math-double-1.c: Likewise. * gcc.dg/c99-math-float-1.c: Likewise. * gcc.dg/c99-math-long-double-1.c: Likewise. * gcc.misc-tests/linkage.exp: Simplify triplet to x86_64-*-solaris2*. * gcc.target/i386/mcount_pic.c: Remove *-*-solaris2.10* && !gld xfail. * gcc.target/i386/pr63620.c: Likewise. * lib/target-supports.exp (check_sse_os_support_available): Remove Solaris 9/x86 workaround. gcc: * config.gcc: Move *-*-solaris2.10* from obsolete configurations to unsupported ones. Simplify x86_64-*-solaris2.1[0-9]* to x86_64-*-solaris2*. * config.host: Likewise. * config/i386/sol2.h (ASM_COMMENT_START): Remove. * config/sparc/driver-sparc.c (host_detect_local_cpu) [__sun__ && __svr4__]: Remove "brand" fallback. [!KSTAT_DATA_STRING]: Remove. * configure.ac (gcc_cv_ld_hidden): Simplify *-*-solaris2.1[0-9]* to *-*-solaris2*. (comdat_group): Likewise. (set_have_as_tls): Likewise. (gcc_cv_target_dl_iterate_phdr): Likewise. (gcc_cv_as_shf_merge): Remove Solaris 10/x86 workaround. (gcc_cv_ld_aligned_shf_merge): Remove Solaris 10/SPARC workaround. * configure: Regenerate. * doc/install.texi: Simplify Solaris target triplets. (Specific, i?86-*-solaris2*): Remove Solaris 10 references. (Specific, *-*-solaris2*): Document Solaris 10 removal. Remove Solaris 10 references. Remove obsolete Solaris bug reference. (Specific, sparc-sun-solaris2.10): Remove. # HG changeset patch # Parent 166b07563e3885f04c3f8095926e6c33092f7349 Remove obsolete Solaris 10 support diff --git a/gcc/config.gcc b/gcc/config.gcc --- a/gcc/config.gcc +++ b/gcc/config.gcc @@ -247,8 +247,7 @@ md_file= # Obsolete configurations. case ${target} in - *-*-solaris2.10* \ - |
[PATCH GCC]Simplify if-then-else in slsr cand_vect
Hi, While working on other PR, I noticed that we can save lots of if-then-else in accessing cand_vec by placing an additional NULL element at front of it. Bootstrap and test on x86_64. Is it OK? Thanks, bin 2019-05-13 Bin Cheng * gimple-ssa-strength-reduction.c (lookup_cand): Adjust index by 1. (alloc_cand_and_find_basis): Ditto. (backtrace_base_for_ref, create_mul_ssa_cand): Remove if-then-else. (create_mul_imm_cand, create_add_ssa_cand): Ditto. (create_add_imm_cand, slsr_process_cast): Ditto. (slsr_process_copy, replace_mult_candidate): Ditto. (replace_rhs_if_not_dup, replace_one_candidate): Ditto. (dump_cand_vec, analyze_candidates_and_replace): Skip NULL element. (pass_strength_reduction::execute): Init the first NULL element. 0001-Simplify-cand_vec-access-in-slsr.patch Description: Binary data
[PATCH] Another PRE insertion speedup
The following switches PRE insertion from a dominator walk to a RPO walk which should reduce the number of iterations because we know AVAIL_OUT will be up-to-date for all predecessors (apart from backedges). I have a more elaborate patch but that somehow fails bootstrap comparison so I'm doing the changes incrementally. Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. Richard. 2019-05-13 Richard Biener PR tree-optimization/90316 * tree-ssa-pre.c (insert_aux): Fold into ... (insert): ... this function. Use a RPO walk to reduce the number of required iterations. Index: gcc/tree-ssa-pre.c === --- gcc/tree-ssa-pre.c (revision 271117) +++ gcc/tree-ssa-pre.c (working copy) @@ -3601,92 +3601,80 @@ do_hoist_insertion (basic_block block) return new_stuff; } -/* Do a dominator walk on the control flow graph, and insert computations - of values as necessary for PRE and hoisting. */ - -static bool -insert_aux (basic_block block, bool do_pre, bool do_hoist) -{ - basic_block son; - bool new_stuff = false; - - if (block) -{ - basic_block dom; - dom = get_immediate_dominator (CDI_DOMINATORS, block); - if (dom) - { - unsigned i; - bitmap_iterator bi; - bitmap_set_t newset; - - /* First, update the AVAIL_OUT set with anything we may have -inserted higher up in the dominator tree. */ - newset = NEW_SETS (dom); - if (newset) - { - /* Note that we need to value_replace both NEW_SETS, and -AVAIL_OUT. For both the case of NEW_SETS, the value may be -represented by some non-simple expression here that we want -to replace it with. */ - FOR_EACH_EXPR_ID_IN_SET (newset, i, bi) - { - pre_expr expr = expression_for_id (i); - bitmap_value_replace_in_set (NEW_SETS (block), expr); - bitmap_value_replace_in_set (AVAIL_OUT (block), expr); - } - } - - /* Insert expressions for partial redundancies. */ - if (do_pre && !single_pred_p (block)) - { - new_stuff |= do_pre_regular_insertion (block, dom); - if (do_partial_partial) - new_stuff |= do_pre_partial_partial_insertion (block, dom); - } - - /* Insert expressions for hoisting. */ - if (do_hoist && EDGE_COUNT (block->succs) >= 2) - new_stuff |= do_hoist_insertion (block); - } -} - for (son = first_dom_son (CDI_DOMINATORS, block); - son; - son = next_dom_son (CDI_DOMINATORS, son)) -{ - new_stuff |= insert_aux (son, do_pre, do_hoist); -} - - return new_stuff; -} - /* Perform insertion of partially redundant and hoistable values. */ static void insert (void) { - bool new_stuff = true; basic_block bb; - int num_iterations = 0; FOR_ALL_BB_FN (bb, cfun) NEW_SETS (bb) = bitmap_set_new (); - while (new_stuff) + int *rpo = XNEWVEC (int, n_basic_blocks_for_fn (cfun)); + int rpo_num = pre_and_rev_post_order_compute (NULL, rpo, false); + + int num_iterations = 0; + bool changed; + do { num_iterations++; if (dump_file && dump_flags & TDF_DETAILS) fprintf (dump_file, "Starting insert iteration %d\n", num_iterations); - new_stuff = insert_aux (ENTRY_BLOCK_PTR_FOR_FN (cfun), flag_tree_pre, - flag_code_hoisting); + + changed = false; + for (int idx = 0; idx < rpo_num; ++idx) + { + basic_block block = BASIC_BLOCK_FOR_FN (cfun, rpo[idx]); + basic_block dom = get_immediate_dominator (CDI_DOMINATORS, block); + if (dom) + { + unsigned i; + bitmap_iterator bi; + bitmap_set_t newset; + + /* First, update the AVAIL_OUT set with anything we may have +inserted higher up in the dominator tree. */ + newset = NEW_SETS (dom); + if (newset) + { + /* Note that we need to value_replace both NEW_SETS, and +AVAIL_OUT. For both the case of NEW_SETS, the value may be +represented by some non-simple expression here that we want +to replace it with. */ + FOR_EACH_EXPR_ID_IN_SET (newset, i, bi) + { + pre_expr expr = expression_for_id (i); + bitmap_value_replace_in_set (NEW_SETS (block), expr); + bitmap_value_replace_in_set (AVAIL_OUT (block), expr); + } + } + + /* Insert expressions for partial redundancies. */ + if (flag_tree_pre && !single_pred_p (block)) + { + changed |= do_pre_regular_insertion (block, dom);
Re: [PATCH][OBVIOUS] Fix wrong usage of dump_printf_loc (PR tree-optimization/90416).
On 5/13/19 1:19 PM, Richard Sandiford wrote: > Martin Liška writes: >> Hi. >> >> dump_printf_loc is a variadic function and the usafe in tree-vect-stmts.c >> is wrongly passing 2nd part of the string format as a first variadic >> argument. >> That's why I saw such a strange crashes. >> >> I'm going to install the patch as soon as it finishes tests. > > Good catch! Mind applying to gcc-9-branch too? Thanks. I'm going to backport that! Martin > > Thanks, > Richard > >> >> Thanks, >> Martin >> >> gcc/ChangeLog: >> >> 2019-05-13 Martin Liska >> >> PR tree-optimization/90416 >> * tree-vect-stmts.c (vect_check_load_store_mask): Concatenate >> string instead of passing the second part as va_arg argument. >> --- >> gcc/tree-vect-stmts.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> >> diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c >> index ced4264722c..4ed60808a65 100644 >> --- a/gcc/tree-vect-stmts.c >> +++ b/gcc/tree-vect-stmts.c >> @@ -2592,7 +2592,7 @@ vect_check_load_store_mask (stmt_vec_info stmt_info, >> tree mask, >> { >>if (dump_enabled_p ()) >> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, >> - "vector mask type %T", >> + "vector mask type %T" >> " does not match vector data type %T.\n", >> mask_vectype, vectype); >>
Re: [PATCH][OBVIOUS] Fix wrong usage of dump_printf_loc (PR tree-optimization/90416).
Martin Liška writes: > Hi. > > dump_printf_loc is a variadic function and the usafe in tree-vect-stmts.c > is wrongly passing 2nd part of the string format as a first variadic argument. > That's why I saw such a strange crashes. > > I'm going to install the patch as soon as it finishes tests. Good catch! Mind applying to gcc-9-branch too? Thanks, Richard > > Thanks, > Martin > > gcc/ChangeLog: > > 2019-05-13 Martin Liska > > PR tree-optimization/90416 > * tree-vect-stmts.c (vect_check_load_store_mask): Concatenate > string instead of passing the second part as va_arg argument. > --- > gcc/tree-vect-stmts.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > > diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c > index ced4264722c..4ed60808a65 100644 > --- a/gcc/tree-vect-stmts.c > +++ b/gcc/tree-vect-stmts.c > @@ -2592,7 +2592,7 @@ vect_check_load_store_mask (stmt_vec_info stmt_info, > tree mask, > { >if (dump_enabled_p ()) > dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, > - "vector mask type %T", > + "vector mask type %T" >" does not match vector data type %T.\n", >mask_vectype, vectype); >
Re: [PATCH][AArch64] Emit TARGET_DOTPROD-specific sequence for sadv16qi
Hi Richard, On 5/9/19 9:06 AM, Richard Sandiford wrote: Kyrill Tkachov writes: +;; Helper expander for aarch64_abd_3 to save the callers +;; the hassle of constructing the other arm of the MINUS. +(define_expand "abd_3" + [(use (match_operand:VDQ_BHSI 0 "register_operand")) + (USMAX:VDQ_BHSI (match_operand:VDQ_BHSI 1 "register_operand") + (match_operand:VDQ_BHSI 2 "register_operand"))] + "TARGET_SIMD" + { +rtx other_arm + = gen_rtx_ (mode, operands[1], operands[2]); +emit_insn (gen_aarch64_abd_3 (operands[0], operands[1], + operands[2], other_arm)); Should be indented to the innermost "(" instead. LGTM otherwise, but an AArch6 maintainer should have the final say. Thanks. After your recent r271107 I've updated the patch and this helper pattern is no longer necessary. This version is shorter and has been bootstrapped and tested on aarch64-none-linux-gnu. Thanks, Kyrill 2019-13-05 Kyrylo Tkachov * config/aarch64/iterators.md (MAX_OPP): New code attr. * config/aarch64/aarch64-simd.md (*aarch64_abd_3): Rename to... (aarch64_abd_3): ... This. (sadv16qi): Add TARGET_DOTPROD expansion. 2019-13-05 Kyrylo Tkachov * gcc.target/aarch64/ssadv16qi.c: Add +nodotprod to pragma. * gcc.target/aarch64/usadv16qi.c: Likewise. * gcc.target/aarch64/ssadv16qi-dotprod.c: New test. * gcc.target/aarch64/usadv16qi-dotprod.c: Likewise. Thanks, Richard diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md index 2b99225d8d352d64b7f8f879a9d73eead66d2969..4b5772be7f4f99d6e9fcd0c56e9c6a668649d7d7 100644 --- a/gcc/config/aarch64/aarch64-simd.md +++ b/gcc/config/aarch64/aarch64-simd.md @@ -710,7 +710,7 @@ ;; So (ABS:QI (minus:QI 64 -128)) == (ABS:QI (192 or -64 signed)) == 64. ;; Whereas SABD would return 192 (-64 signed) on the above example. ;; Use MINUS ([us]max (op1, op2), [us]min (op1, op2)) instead. -(define_insn "*aarch64_abd_3" +(define_insn "aarch64_abd_3" [(set (match_operand:VDQ_BHSI 0 "register_operand" "=w") (minus:VDQ_BHSI (USMAX:VDQ_BHSI @@ -764,7 +764,16 @@ ;; UABAL tmp.8h, op1.16b, op2.16b ;; UADALP op3.4s, tmp.8h ;; MOV op0, op3 // should be eliminated in later passes. -;; The signed version just uses the signed variants of the above instructions. +;; +;; For TARGET_DOTPROD we do: +;; MOV tmp1.16b, #1 // Can be CSE'd and hoisted out of loops. +;; UABD tmp2.16b, op1.16b, op2.16b +;; UDOT op3.4s, tmp2.16b, tmp1.16b +;; MOV op0, op3 // RA will tie the operands of UDOT appropriately. +;; +;; The signed version just uses the signed variants of the above instructions +;; but for TARGET_DOTPROD still emits a UDOT as the absolute difference is +;; unsigned. (define_expand "sadv16qi" [(use (match_operand:V4SI 0 "register_operand")) @@ -773,6 +782,15 @@ (use (match_operand:V4SI 3 "register_operand"))] "TARGET_SIMD" { +if (TARGET_DOTPROD) + { + rtx ones = force_reg (V16QImode, CONST1_RTX (V16QImode)); + rtx abd = gen_reg_rtx (V16QImode); + emit_insn (gen_aarch64_abdv16qi_3 (abd, operands[1], operands[2])); + emit_insn (gen_aarch64_udotv16qi (operands[0], operands[3], + abd, ones)); + DONE; + } rtx reduc = gen_reg_rtx (V8HImode); emit_insn (gen_aarch64_abdl2v16qi_3 (reduc, operands[1], operands[2])); diff --git a/gcc/testsuite/gcc.target/aarch64/ssadv16qi-dotprod.c b/gcc/testsuite/gcc.target/aarch64/ssadv16qi-dotprod.c new file mode 100644 index ..08b6831cfbee2c44cf6a33f91986e2953c622148 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/ssadv16qi-dotprod.c @@ -0,0 +1,31 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target arm_v8_2a_dotprod_neon_ok } */ +/* { dg-add-options arm_v8_2a_dotprod_neon } */ +/* { dg-additional-options "-O3" } */ + +#pragma GCC target "+nosve" + +#define N 1024 + +signed char pix1[N], pix2[N]; + +int foo (void) +{ + int i_sum = 0; + int i; + + for (i = 0; i < N; i++) +i_sum += __builtin_abs (pix1[i] - pix2[i]); + + return i_sum; +} + +/* { dg-final { scan-assembler-not {\tsshll\t} } } */ +/* { dg-final { scan-assembler-not {\tsshll2\t} } } */ +/* { dg-final { scan-assembler-not {\tssubl\t} } } */ +/* { dg-final { scan-assembler-not {\tssubl2\t} } } */ +/* { dg-final { scan-assembler-not {\tabs\t} } } */ + +/* { dg-final { scan-assembler {\tsabd\t} } } */ +/* { dg-final { scan-assembler {\tudot\t} } } */ + diff --git a/gcc/testsuite/gcc.target/aarch64/ssadv16qi.c b/gcc/testsuite/gcc.target/aarch64/ssadv16qi.c index 40b28843616e84df137210b45ec16abed2a37c75..85a867a113013f560bfd0a3142805b9c95ad8c5a 100644 --- a/gcc/testsuite/gcc.target/aarch64/ssadv16qi.c +++ b/gcc/testsuite/gcc.target/aarch64/ssadv16qi.c @@ -1,7 +1,7 @@ /* { dg-do compile } */ /* { dg-options "-O3" } */ -#pragma GCC target "+nosve" +#pragma GCC target "+nosve+nodotprod" #define N 1024 diff --git a/gcc/testsuite/gcc.target/aarch64/usadv16qi-dotprod.c
[wwwdocs] Add DOCTYPE and lang attribute
This makes /gcc-10/changes.html valid HTML5. Committed to CVS. Index: htdocs/gcc-10/changes.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-10/changes.html,v retrieving revision 1.2 diff -u -r1.2 changes.html --- htdocs/gcc-10/changes.html 13 May 2019 10:49:50 - 1.2 +++ htdocs/gcc-10/changes.html 13 May 2019 10:59:49 - @@ -1,4 +1,5 @@ - + + GCC 10 Release Series Changes, New Features, and Fixes
Re: Mention lerp and midpoint in the libstdc++ status.
On 10/05/19 23:12 -0400, Ed Smith-Rowland via libstdc++ wrote: No complete patch because we could just accumulate a few of these. Thanks. I've committed it to trunk and gcc-9-branch, because I'm probably going to be regenerating the HTML again soon. Index: doc/xml/manual/status_cxx2020.xml === --- doc/xml/manual/status_cxx2020.xml (revision 271076) +++ doc/xml/manual/status_cxx2020.xml (working copy) @@ -934,6 +934,17 @@ +Well-behaved interpolation for numbers and pointers + +http://www.w3.org/1999/xlink; xlink:href="http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p0811r3.html;> + P0811R3 + + + 9.1 + + + + Missing feature test macros
Re: [PATCH] Remove Profile Mode, deprecated since GCC 7.1
On 11/05/19 20:56 +0100, Jonathan Wakely wrote: The Profile Mode extension is not used by anybody, nor maintained by anybody. The containers do not support the full API specified in recent standards, and so enabling Profile Mode is not source compatible with much modern C++ code. The heuristics that would check the profile information and make useful suggestions never materialized, so it isn't useful. It should be removed. Committed to trunk, added to the release notes with this patch committed to CVS. Index: htdocs/gcc-10/changes.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-10/changes.html,v retrieving revision 1.1 diff -u -r1.1 changes.html --- htdocs/gcc-10/changes.html 25 Apr 2019 18:12:29 - 1.1 +++ htdocs/gcc-10/changes.html 13 May 2019 10:49:40 - @@ -26,7 +26,10 @@ Caveats - ... + +The deprecated Profile Mode and array_allocator extensions +have been removed from libstdc++. +
Re: [PATCH][GCC][AArch64] Vectorise __builtin_signbit on aarch64
Hi all, Vectorise __builtin_signbit (v2sf, v4sf) with unsigned shift right vector instruction. Bootstrapped and tested on aarch64-none-linux-gnu. Assembly output for: $ aarch64-elf-gcc -S -O3 signbitv2sf.c -dp Before patch: foo: ldp w2, w1, [x1]// 37 [c=0 l=4] *load_pair_zero_extendsidi2_aarch64/0 and w2, w2, -2147483648 // 8[c=4 l=4] andsi3/1 and w1, w1, -2147483648 // 12 [c=4 l=4] andsi3/1 stp w2, w1, [x0]// 38 [c=0 l=4] store_pair_sw_sisi/0 ret // 32 [c=0 l=4] *do_return After patch: foo: ldr d0, [x1]// 7[c=8 l=4] *aarch64_simd_movv2sf/0 ushrv0.2s, v0.2s, 31// 8[c=12 l=4] aarch64_simd_lshrv2si str d0, [x0]// 9[c=4 l=4] *aarch64_simd_movv2si/2 ret // 28 [c=0 l=4] *do_return Assembly output for: $ aarch64-elf-gcc -S -O3 signbitv4sf.c -dp Before patch: foo: adrpx3, in // 38 [c=4 l=4] *movdi_aarch64/12 adrpx2, out // 41 [c=4 l=4] *movdi_aarch64/12 add x3, x3, :lo12:in// 40 [c=4 l=4] add_losym_di add x2, x2, :lo12:out // 43 [c=4 l=4] add_losym_di mov x0, 0 // 3[c=4 l=4] *movdi_aarch64/3 .p2align 3,,7 .L2: ldr w1, [x3, x0]// 10 [c=16 l=4] *zero_extendsidi2_aarch64/1 and w1, w1, -2147483648 // 11 [c=4 l=4] andsi3/1 str w1, [x2, x0]// 16 [c=4 l=4] *movsi_aarch64/8 add x0, x0, 4 // 17 [c=4 l=4] *adddi3_aarch64/0 cmp x0, 4096// 19 [c=4 l=4] cmpdi/1 bne .L2 // 20 [c=4 l=4] condjump ret // 51 [c=0 l=4] \*do_return After patch: foo: adrpx2, in // 37 [c=4 l=4] *movdi_aarch64/12 adrpx1, out // 40 [c=4 l=4] *movdi_aarch64/12 add x2, x2, :lo12:in// 39 [c=4 l=4] add_losym_di add x1, x1, :lo12:out // 42 [c=4 l=4] add_losym_di mov x0, 0 // 3[c=4 l=4] *movdi_aarch64/3 .p2align 3,,7 .L2: ldr q0, [x2, x0]// 10 [c=8 l=4] *aarch64_simd_movv4sf/0 ushrv0.4s, v0.4s, 31// 11 [c=12 l=4] aarch64_simd_lshrv4si str q0, [x1, x0]// 15 [c=4 l=4] *aarch64_simd_movv4si/2 add x0, x0, 16 // 16 [c=4 l=4] *adddi3_aarch64/0 cmp x0, 4096// 18 [c=4 l=4] cmpdi/1 bne .L2 // 19 [c=4 l=4] condjump ret // 50 [c=0 l=4] *do_return OK for Trunk ? Thanks, Przemyslaw gcc/ChangeLog: 2019-05-13 Przemyslaw Wirkus * internal-fn.def (SIGNBIT): New. * config/aarch64/aarch64-simd.md (signbitv2sf2): New expand defined. (signbitv4sf2): Likewise. gcc/testsuite/ChangeLog: 2019-05-13 Przemyslaw Wirkus * gcc.target/aarch64/signbitv4sf.c: New test. * gcc.target/aarch64/signbitv2sf.c: New test. diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md index e3852c5d182b70978d7603225fce55c0b8ee2894..8f7227327cb960fb34c7b88e1bf283f8f17a3be9 100644 --- a/gcc/config/aarch64/aarch64-simd.md +++ b/gcc/config/aarch64/aarch64-simd.md @@ -935,6 +935,21 @@ [(set_attr "type" "neon_ins")] ) +(define_expand "signbit2" + [(use (match_operand: 0 "register_operand")) + (use (match_operand:VDQSF 1 "register_operand"))] + "TARGET_SIMD" +{ + int shift_amount = GET_MODE_UNIT_BITSIZE (mode) - 1; + rtx shift_vector = aarch64_simd_gen_const_vector_dup (mode, +shift_amount); + operands[1] = lowpart_subreg (mode, operands[1], mode); + + emit_insn (gen_aarch64_simd_lshr (operands[0], operands[1], + shift_vector)); + DONE; +}) + (define_insn "aarch64_simd_lshr" [(set (match_operand:VDQ_I 0 "register_operand" "=w") (lshiftrt:VDQ_I (match_operand:VDQ_I 1 "register_operand" "w") diff --git a/gcc/internal-fn.def b/gcc/internal-fn.def index e370eaa84767839c827b6ebd0c86303bcc36fa54..016301a58d83d7128817824d7c7ef92825c7e03e 100644 --- a/gcc/internal-fn.def +++ b/gcc/internal-fn.def @@ -217,6 +217,7 @@ DEF_INTERNAL_FLT_FN (LOG10, ECF_CONST, log10, unary) DEF_INTERNAL_FLT_FN (LOG1P, ECF_CONST, log1p, unary) DEF_INTERNAL_FLT_FN (LOG2, ECF_CONST, log2, unary) DEF_INTERNAL_FLT_FN (LOGB, ECF_CONST, logb, unary) +DEF_INTERNAL_FLT_FN (SIGNBIT, ECF_CONST, signbit, unary) DEF_INTERNAL_FLT_FN (SIGNIFICAND, ECF_CONST, significand, unary) DEF_INTERNAL_FLT_FN (SIN, ECF_CONST, sin, unary) DEF_INTERNAL_FLT_FN (SINH, ECF_CONST, sinh, unary) diff --git a/gcc/testsuite/gcc.target/aarch64/signbitv2sf.c b/gcc/testsuite/gcc.target/aarch64/signbitv2sf.c new file mode 100644 index ..2587bfedd538f30a018cf827ea57cd583b2fa084 --- /dev/null +++
Re: [PATCH] Eliminates phi on branch for CMP result
On Mon, 13 May 2019, Segher Boessenkool wrote: > On Sun, May 12, 2019 at 10:07:28PM -0500, Jiufu Guo wrote: > > To elimiate the instructions which moving result of CMP to GPR, I'm > > wondering maybe we could do a combine(or a peephole) after bbro to let > > the condition jump directly using the result of CMP. > > It will have to be a peephole. And peepholes are machine-specific. > > You will also not get any further optimisations in those blocks, that way; > not combine etc. > > It really should be done on gimple, not rtl. Agreed.
Re: [PATCH] Eliminates phi on branch for CMP result
On Sun, May 12, 2019 at 10:07:28PM -0500, Jiufu Guo wrote: > To elimiate the instructions which moving result of CMP to GPR, I'm > wondering maybe we could do a combine(or a peephole) after bbro to let > the condition jump directly using the result of CMP. It will have to be a peephole. And peepholes are machine-specific. You will also not get any further optimisations in those blocks, that way; not combine etc. It really should be done on gimple, not rtl. Segher
Re: [DWARF] dwarf2out cleanups
On Fri, May 10, 2019 at 2:40 PM Nathan Sidwell wrote: > > When poking at some dwarf bugs, which were ultimately fixed by Richard, > I made a couple of cleanups. The first two are pretty obvious comment > clarification, but the last fragment is a more invasive control flow > change. In that case we essentially repeat an inline-static-var check > in two places. I've replaced that with a bool to control whether we > should splice. That at least helped me understand the code better, and > should give the optimizer better visibility to simplify the generated > control flow. > > tested on x86_64-linux, ok? OK. > nathan > > -- > Nathan Sidwell
[PATCH][OBVIOUS] Fix wrong usage of dump_printf_loc (PR tree-optimization/90416).
Hi. dump_printf_loc is a variadic function and the usafe in tree-vect-stmts.c is wrongly passing 2nd part of the string format as a first variadic argument. That's why I saw such a strange crashes. I'm going to install the patch as soon as it finishes tests. Thanks, Martin gcc/ChangeLog: 2019-05-13 Martin Liska PR tree-optimization/90416 * tree-vect-stmts.c (vect_check_load_store_mask): Concatenate string instead of passing the second part as va_arg argument. --- gcc/tree-vect-stmts.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c index ced4264722c..4ed60808a65 100644 --- a/gcc/tree-vect-stmts.c +++ b/gcc/tree-vect-stmts.c @@ -2592,7 +2592,7 @@ vect_check_load_store_mask (stmt_vec_info stmt_info, tree mask, { if (dump_enabled_p ()) dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, - "vector mask type %T", + "vector mask type %T" " does not match vector data type %T.\n", mask_vectype, vectype);
[PATCH, i386]: --enable-frame-pointer for cygwin and mingw
On Thu, May 9, 2019 at 11:55 AM Thomas Schwinge wrote: > On Sun, 10 Feb 2019 20:51:39 +0100, Uros Bizjak wrote: > > On Fri, Feb 8, 2019 at 1:24 PM Uros Bizjak wrote: > > > Attached patch fixes --enable-frame-pointer handling [...] > > ACK. > > > Please note that this fix will re-enable frame pointer for all targets > > but linux* or darwin[[8912]]. However, since builds for e.g. cygwin > > and mingw survived just well without frame pointers in the mean time, > > we should probably list these targets as targets without frame > > pointers by default. > > I agree, this would cause the least surprise, to simply keep the previous > default of '--disable-frame-pointer'. Until such a global change is > agreed on, and made... I plan to commit the attached patch later today. 2019-05-13 Uroš Bizjak PR target/89221 * configure.ac (--enable-frame-pointer): Disable by default for cygwin and mingw. * configure: Regenerate. Uros. diff --git a/gcc/configure b/gcc/configure index 947d263a6174..3cab176e5018 100755 --- a/gcc/configure +++ b/gcc/configure @@ -12197,7 +12197,7 @@ if test "${enable_frame_pointer+set}" = set; then : else case $target_os in -linux* | gnu* | darwin[8912]*) +linux* | gnu* | darwin[8912]* | cygwin* | mingw*) # Enable -fomit-frame-pointer by default for these systems with DWARF2. enable_frame_pointer=no ;; diff --git a/gcc/configure.ac b/gcc/configure.ac index bfcdf526e446..264f36fb78a8 100644 --- a/gcc/configure.ac +++ b/gcc/configure.ac @@ -1884,7 +1884,7 @@ AC_ARG_ENABLE(frame-pointer, [enable -fno-omit-frame-pointer by default for x86])], [], [ case $target_os in -linux* | gnu* | darwin[[8912]]*) +linux* | gnu* | darwin[[8912]]* | cygwin* | mingw*) # Enable -fomit-frame-pointer by default for these systems with DWARF2. enable_frame_pointer=no ;;
Re: malloc cannot alias preexisting pointers
On Sun, May 12, 2019 at 2:51 PM Marc Glisse wrote: > > On Sun, 12 May 2019, Richard Sandiford wrote: > > > Marc Glisse writes: > >> Hello, > >> > >> this patch lets gcc know that if a pointer existed before the call to > >> malloc, the result of malloc cannot alias it. This is a bit ad hoc and > >> fragile. A small improvement would be, when the 2 statements are in the > >> same bb but in the wrong order, to check if there is any statement in > >> between that might prevent from reordering them. But that's more > >> complicated, and the patch as it is already does help. > >> > >> I expect people may not like the call to a function from > >> tree-ssa-loop-niter too much, but it is convenient. And if someone > >> improves it, they will likely have to rewrite something not quite > >> equivalent to stmt_dominates_stmt_p. > > > > It has linear complexity for statements in the same block though. > > Ah, true. I should anyway test that the second statement is malloc > (cheaper) before checking for domination, but even then that could be used > to create a quadratic behavior :-( I also think the oracle queries shouldn't encompany such expensive pieces... > > (reassoc_stmt_dominates_stmt_p avoids that, but relies on uids > > being up-to-date.) > > This function is called from all over the place. Unless there is a simple > test to check if uids are safe to use (reassoc_in_progress?), that's going > to be a problem. I find it surprising that this information is so hard to > get to... Stopping stmt_dominates_stmt_p after walking 128 statements > doesn't feel like a great solution. But without controlling the pass where > this happens, I don't see any good solution. It would be great if that > non-aliasing property could be recorded in the points-to information, then > I could set it from a pass I control, but I somehow got the impression > that it wouldn't work. Maybe I should try to understand PTA better to make > sure. In princple PTA should know the aliasing cannot happen but possibly the info is either lost or the IL is too obfuscated at the point it gets computed. (hello C++...) So at ldist time I see # PT = null { D.47989 } (escaped, escaped heap) # ALIGN = 8, MISALIGN = 0 # USE = nonlocal null { D.47989 } (escaped, escaped heap) # CLB = nonlocal null { D.47989 } (escaped, escaped heap) _27 = malloc (8192); good. The interesting loop is the following where the allocation PTA is good and _4 intersect because they include ESCAPED. This is because _27 itself escapes and PTA not being flow-sensitive. I don't see an easy way to improve things here but dislike the stmt walking very much. That is, the proper solution is to re-write PTA in some way. [local count: 2866890688]: # PT = nonlocal escaped null # __first_13 = PHI <_4(12), __first_23(13)> # PT = null { D.47989 } (escaped, escaped heap) # ALIGN = 8, MISALIGN = 0 # __cur_12 = PHI <_27(12), __cur_24(13)> # PT = nonlocal escaped null _20 = MEM[(int * const &)__first_13 clique 2 base 0]; MEM[(int * &)__first_13 clique 2 base 0] = 0B; MEM[(struct &)__cur_12 clique 3 base 1] ={v} {CLOBBER}; MEM[(struct _Head_base *)__cur_12 clique 3 base 1]._M_head_impl = _20; # PT = nonlocal escaped null _22 = MEM[(int * &)__first_13]; if (_22 != 0B) goto ; [53.47%] else goto ; [46.53%] [local count: 1333964241]: goto ; [100.00%] [local count: 1532926450]: *_22 ={v} {CLOBBER}; # USE = nonlocal null { D.47989 } (escaped, escaped heap) # CLB = nonlocal null { D.47989 } (escaped, escaped heap) operator delete (_22, 4); [local count: 2866890691]: MEM[(struct &)__first_13] ={v} {CLOBBER}; # PT = nonlocal escaped __first_23 = __first_13 + 8; # PT = { D.47989 } (escaped, escaped heap) # ALIGN = 8, MISALIGN = 0 __cur_24 = __cur_12 + 8; if (_9 == __first_23) goto ; [11.00%] else goto ; [89.00%] [local count: 2551532717]: goto ; [100.00%] > -- > Marc Glisse
Re: [PATCH][RFC] Come up with TARGET_HAS_FAST_MEMPCPY_ROUTINE (PR middle-end/90263).
On 5/10/19 11:21 AM, Jakub Jelinek wrote: > On Fri, May 10, 2019 at 11:04:12AM +0200, Martin Liška wrote: >> --- a/gcc/config/i386/i386.h >> +++ b/gcc/config/i386/i386.h >> @@ -1906,6 +1906,9 @@ typedef struct ix86_args { >> >> #define CLEAR_RATIO(speed) ((speed) ? MIN (6, ix86_cost->move_ratio) : 2) >> >> +/* C library provides fast implementation of mempcpy function. */ >> +#define TARGET_HAS_FAST_MEMPCPY_ROUTINE 1 >> + > > 1) we shouldn't be adding further target macros, but target hooks Done. > 2) I don't think this is a property of the x86 target, but of x86 glibc, >so you should set it on x86 glibc only (i.e. i?86/x86_64 linux and hurd >when using glibc, not newlib, nor bionic/android, nor uclibc, nor musl) I've implemented the in i386.c with DEFAULT_LIBC == LIBC_GLIBC. Hope it's correct? > >> --- a/gcc/testsuite/gcc.c-torture/execute/builtins/mempcpy.c >> +++ b/gcc/testsuite/gcc.c-torture/execute/builtins/mempcpy.c >> @@ -56,9 +56,8 @@ main_test (void) >>if (__builtin_mempcpy (p, "ABCDE", 6) != p + 6 || memcmp (p, "ABCDE", 6)) >> abort (); >> >> - /* If the result of mempcpy is ignored, gcc should use memcpy. >> - This should be optimized always, so set inside_main again. */ >> - inside_main = 1; >> + /* Set inside main in order to not abort because of usafe of mempcpy. */ >> + inside_main = 0; >>mempcpy (p + 5, s3, 1); >>if (memcmp (p, "ABCDEFg", 8)) >> abort (); > > Why this? Do you mean mempcpy is called here, even when the lhs is unused? > We should be calling memcpy in that case. Yes, that's a stupid mistake. I'm attaching updated version of the patch that works fine in x86_64-linux-gnu. Martin > > Jakub > >From 05b36e6a04e061b46638ec6f23ad2027f033caf9 Mon Sep 17 00:00:00 2001 From: marxin Date: Mon, 29 Apr 2019 13:46:25 +0200 Subject: [PATCH] Come up with has_fast_mempcpy_routine hook (PR middle-end/90263). gcc/ChangeLog: 2019-05-13 Martin Liska PR middle-end/90263 * builtins.c (expand_builtin_memory_copy_args): When having a target with fast mempcpy implementation do now use memcpy. * config/i386/i386.c (glibc_has_fast_mempcpy_routine): New. (TARGET_HAS_FAST_MEMPCPY_ROUTINE): Likewise. * doc/tm.texi: Likewise. * doc/tm.texi.in: Likewise. * target.def: Likewise. * expr.c (emit_block_move_hints): Add 2 new arguments. * expr.h (emit_block_move_hints): Bail out when libcall to memcpy would be used. gcc/testsuite/ChangeLog: 2019-05-13 Martin Liska PR middle-end/90263 * gcc.c-torture/pr90263.c: New test. --- gcc/builtins.c| 17 +++-- gcc/config/i386/i386.c| 10 ++ gcc/doc/tm.texi | 4 gcc/doc/tm.texi.in| 2 ++ gcc/expr.c| 13 - gcc/expr.h| 4 +++- gcc/target.def| 6 ++ gcc/testsuite/gcc.c-torture/pr90263.c | 9 + 8 files changed, 61 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/gcc.c-torture/pr90263.c diff --git a/gcc/builtins.c b/gcc/builtins.c index d37d73fc4a0..06b19b62b43 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -3839,6 +3839,8 @@ expand_builtin_memory_copy_args (tree dest, tree src, tree len, unsigned HOST_WIDE_INT max_size; unsigned HOST_WIDE_INT probable_max_size; + bool is_move_done; + /* If DEST is not a pointer type, call the normal function. */ if (dest_align == 0) return NULL_RTX; @@ -3888,11 +3890,22 @@ expand_builtin_memory_copy_args (tree dest, tree src, tree len, if (CALL_EXPR_TAILCALL (exp) && (retmode == RETURN_BEGIN || target == const0_rtx)) method = BLOCK_OP_TAILCALL; - if (retmode == RETURN_END && target != const0_rtx) + bool use_mempcpy_call = (targetm.has_fast_mempcpy_routine () + && retmode == RETURN_END + && target != const0_rtx); + if (use_mempcpy_call) method = BLOCK_OP_NO_LIBCALL_RET; dest_addr = emit_block_move_hints (dest_mem, src_mem, len_rtx, method, expected_align, expected_size, - min_size, max_size, probable_max_size); + min_size, max_size, probable_max_size, + use_mempcpy_call, _move_done); + + /* Bail out when a mempcpy call would be expanded as libcall and when + we have a target that provides a fast implementation + of mempcpy routine. */ + if (!is_move_done) +return NULL_RTX; + if (dest_addr == pc_rtx) return NULL_RTX; diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index cc0ae3fcfd3..a0fc9786568 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -23056,6 +23056,16 @@ ix86_run_selftests (void) #define TARGET_GET_MULTILIB_ABI_NAME \ ix86_get_multilib_abi_name +#if DEFAULT_LIBC == LIBC_GLIBC +static bool glibc_has_fast_mempcpy_routine (void) +{ + return true; +} + +#undef TARGET_HAS_FAST_MEMPCPY_ROUTINE +#define TARGET_HAS_FAST_MEMPCPY_ROUTINE glibc_has_fast_mempcpy_routine
[PATCH] PR fortran/89100 Default widths for i, f and g format specifiers in format strings
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89100 see comment 4 Please can someone commit the attached patch as I do not have commit rights. Change logs: For gcc/fortran Jeff Law Mark Eggleston PR fortran/89100 * gfortran.texi: Add Default widths for F, G and I format descriptors to Extensions section. * invoke.texi: Add -fdec-format-defaults * io.c (check_format): Use default widths for i, f and g when flag_dec_format_defaults is enabled. * lang.opt: Add new option. * options.c (set_dec_flags): Add SET_BITFLAG for flag_dec_format_defaults. For gcc/testsuite Mark Eggleston PR fortran/89100 * gfortran.dg/fmt_f_default_field_width_1.f90: New test. * gfortran.dg/fmt_f_default_field_width_2.f90: New test. * gfortran.dg/fmt_f_default_field_width_3.f90: New test. * gfortran.dg/fmt_g_default_field_width_1.f90: New test. * gfortran.dg/fmt_g_default_field_width_2.f90: New test. * gfortran.dg/fmt_g_default_field_width_3.f90: New test. * gfortran.dg/fmt_i_default_field_width_1.f90: New test. * gfortran.dg/fmt_i_default_field_width_2.f90: New test. * gfortran.dg/fmt_i_default_field_width_3.f90: New test. For libgfortran Jeff Law PR fortran/89100 * io/format.c (parse_format_list): set default width when the IOPARM_DT_DEC_EXT flag is set for i, f and g. * io/io.h: add default_width_for_integer, default_width_for_float and default_precision_for_float. * io/write.c (write_boz): extra parameter giving length of data corresponding to the type's kind. (write_b): pass data length as extra parameter in calls to write_boz. (write_o): pass data length as extra parameter in calls to write_boz. (write_z): pass data length as extra parameter in calls to write_boz. (size_from_kind): also set size is default width is set. * io/write_float.def (build_float_string): new paramter inserted before result parameter. If default width use values passed instead of the values in fnode. (FORMAT_FLOAT): macro modified to check for default width and calls to build_float_string to pass in default width. (get_float_string): set width and precision to defaults when needed. -- https://www.codethink.co.uk/privacy.html >From f952b060a6d5de506022026eef13c86692c23c12 Mon Sep 17 00:00:00 2001 From: law Date: Thu, 10 May 2018 11:48:34 +0100 Subject: [PATCH 1/6] Default widths for i, f and g format specifiers in format strings. Enabled using -fdec. The behaviour is modelled on the Oracle Fortran compiler. At the time of writing, the details were available at this URL: https://docs.oracle.com/cd/E19957-01/805-4939/6j4m0vnc3/index.html#z4000743746d Addition by Mark Eggleston : Use -fdec-format-defaults to enable this feature. Also enabled using -fdec. --- gcc/fortran/gfortran.texi | 17 gcc/fortran/invoke.texi| 25 ++- gcc/fortran/io.c | 31 -- gcc/fortran/lang.opt | 4 ++ gcc/fortran/options.c | 1 + .../gfortran.dg/fmt_f_default_field_width_1.f90| 40 + .../gfortran.dg/fmt_f_default_field_width_2.f90| 43 +++ .../gfortran.dg/fmt_f_default_field_width_3.f90| 30 + .../gfortran.dg/fmt_g_default_field_width_1.f90| 45 +++ .../gfortran.dg/fmt_g_default_field_width_2.f90| 48 + .../gfortran.dg/fmt_g_default_field_width_3.f90| 33 ++ .../gfortran.dg/fmt_i_default_field_width_1.f90| 40 + .../gfortran.dg/fmt_i_default_field_width_2.f90| 44 +++ .../gfortran.dg/fmt_i_default_field_width_3.f90| 37 libgfortran/io/format.c| 35 +++ libgfortran/io/io.h| 50 ++ libgfortran/io/read.c | 6 +++ libgfortran/io/write.c | 22 ++ libgfortran/io/write_float.def | 37 +--- 19 files changed, 560 insertions(+), 28 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/fmt_f_default_field_width_1.f90 create mode 100644 gcc/testsuite/gfortran.dg/fmt_f_default_field_width_2.f90 create mode 100644 gcc/testsuite/gfortran.dg/fmt_f_default_field_width_3.f90 create mode 100644 gcc/testsuite/gfortran.dg/fmt_g_default_field_width_1.f90 create mode 100644 gcc/testsuite/gfortran.dg/fmt_g_default_field_width_2.f90 create mode 100644 gcc/testsuite/gfortran.dg/fmt_g_default_field_width_3.f90 create mode 100644 gcc/testsuite/gfortran.dg/fmt_i_default_field_width_1.f90 create mode 100644 gcc/testsuite/gfortran.dg/fmt_i_default_field_width_2.f90 create mode 100644 gcc/testsuite/gfortran.dg/fmt_i_default_field_width_3.f90 diff --git a/gcc/fortran/gfortran.texi
Re: [ada, build] Avoid cp -p failures during Ada make install
> > No, this is not OK. > > > > I'd rather keep the simple current logic and either stick to cp -p, or > > use a proper $(INSTALL_whatever) as done elsewhere rather than adding more > > kludges. > > how do you mean, `proper $(INSTALL_whatever)'? Using e.g. INSTALL_DATA from configure. > I've run the cp -p under strace, which shows > > fgetxattr(3, "system.posix_acl_access", 0x7fff96829fd0, 132) = -1 ENODATA (No > data available) > fstat(3, {st_mode=S_IFREG|0644, st_size=35368, ...}) = 0 > fsetxattr(4, "system.posix_acl_access", > "\2\0\0\0\1\0\6\0\377\377\377\377\4\0\4\0\377\377\377\377 > \0\4\0\377\377\377\377", 28, 0) = -1 EOPNOTSUPP (Operation not supported) > > i.e. it tries to determine an extended attribute, is told there's none, > tries to set that none on the destination and chokes if that doesn't > work. Seems pretty insane to me. > > This is cp from coreutils 8.30 on Fedora 29, btw., not same ancient > prehistoric software. Did someone file a bug report there BTW? Arno
Re: [PATCH][PR89341]Fix ICE on function definition with weakref/alias attributes attached
在 2019/5/13 下午5:02, Jakub Jelinek 写道: On Mon, May 13, 2019 at 04:53:52PM +0800, JunMa wrote: According to gnu document of function attributes, neither weakref nor alias could be attached to a function defined in current translation unit. Although GCC checks the attributes under some circumstances, it still fails on some cases and even causes ICE. This patch checks whether alias/weakref attribute attaches on a function declaration which has body, and removes the attribute later. This also avoid the ICE. Bootstrapped/regtested on x86_64-linux, ok for GCC10? 2019-03-26 Jun Ma PR89341 * cgraphunit.c (handle_alias_pairs): Check whether alias attribute attaches on a function declaration which has body. gcc/testsuite/ChangeLog 2019-03-26 Jun Ma PR89341 * gcc.dg/attr-alias-6.c: New test. * gcc.dg/attr-weakref-5.c: Likewise. Hi Jakub Since you are owner of this part, so I add you to cc list. Ping? I'm not the maintainer of the callgraph, Honza is: callgraph Jan Hubicka Hi Jakub Thanks for your reminder. Honza, Ping? Regards JunMa Jakub
Re: Fwd: [Patch] Fix assembler invocation on sparc-sun-solaris2
Hi Bence, > I tried building a cross-compiler for `sparc-sun-solaris2`, but the > resulting GCC executable wouldn't compile anything, always failing > with: "as: unrecognized option '-m32'". After extensive research, I > found that `as` takes `--32`/`--64`, instead of `-m32`/`-m64`. The > patched GCC now does compile. this patch is wrong: gcc on Solaris assumes the native assembler by default, which *does* accept -m32 and -m64. You can see it in the snippet you patched: > diff --git a/gcc/config/sparc/sol2.h b/gcc/config/sparc/sol2.h > index 93404757054..d1d0bb1f99d 100644 > - --- a/gcc/config/sparc/sol2.h > +++ b/gcc/config/sparc/sol2.h > @@ -63,9 +63,9 @@ along with GCC; see the file COPYING3. If not see > > #ifndef USE_GAS ^^^ > #undef ASM_ARCH32_SPEC > - -#define ASM_ARCH32_SPEC "-m32" > +#define ASM_ARCH32_SPEC "--32" > #undef ASM_ARCH64_SPEC > - -#define ASM_ARCH64_SPEC "-m64" > +#define ASM_ARCH64_SPEC "--64" > #endif Obviously in a cross configuration you cannot use as, but need to specify --with-as= --with-gnu-as. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [ada, build] Avoid cp -p failures during Ada make install
Hi Arnaud, >> Tested on x86_64-pc-linux-gnu installing both to a local filesystem and >> an NFSv3 filesystem. >> >> Ok for mainline (and the gcc-9 and gcc-8 branches eventually)? > > No, this is not OK. > > I'd rather keep the simple current logic and either stick to cp -p, or > use a proper $(INSTALL_whatever) as done elsewhere rather than adding more > kludges. how do you mean, `proper $(INSTALL_whatever)'? I've run the cp -p under strace, which shows fgetxattr(3, "system.posix_acl_access", 0x7fff96829fd0, 132) = -1 ENODATA (No data available) fstat(3, {st_mode=S_IFREG|0644, st_size=35368, ...}) = 0 fsetxattr(4, "system.posix_acl_access", "\2\0\0\0\1\0\6\0\377\377\377\377\4\0\4\0\377\377\377\377 \0\4\0\377\377\377\377", 28, 0) = -1 EOPNOTSUPP (Operation not supported) i.e. it tries to determine an extended attribute, is told there's none, tries to set that none on the destination and chokes if that doesn't work. Seems pretty insane to me. This is cp from coreutils 8.30 on Fedora 29, btw., not same ancient prehistoric software. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH][PR89341]Fix ICE on function definition with weakref/alias attributes attached
On Mon, May 13, 2019 at 04:53:52PM +0800, JunMa wrote: > > According to gnu document of function attributes, neither weakref nor > > alias > > could be attached to a function defined in current translation unit. > > Although GCC checks the attributes under some circumstances, it still > > fails > > on some cases and even causes ICE. > > > > This patch checks whether alias/weakref attribute attaches on a function > > declaration which has body, and removes the attribute later. > > This also avoid the ICE. > > > > Bootstrapped/regtested on x86_64-linux, ok for GCC10? > > > > 2019-03-26 Jun Ma > > > > PR89341 > > * cgraphunit.c (handle_alias_pairs): Check whether alias attribute > > attaches > > on a function declaration which has body. > > > > gcc/testsuite/ChangeLog > > > > 2019-03-26 Jun Ma > > > > PR89341 > > * gcc.dg/attr-alias-6.c: New test. > > * gcc.dg/attr-weakref-5.c: Likewise. > Hi Jakub > > Since you are owner of this part, so I add you to cc list. > Ping? I'm not the maintainer of the callgraph, Honza is: callgraph Jan Hubicka Jakub
Re: [PATCH][PR89341]Fix ICE on function definition with weakref/alias attributes attached
在 2019/3/26 下午7:40, JunMa 写道: Hi According to gnu document of function attributes, neither weakref nor alias could be attached to a function defined in current translation unit. Although GCC checks the attributes under some circumstances, it still fails on some cases and even causes ICE. This patch checks whether alias/weakref attribute attaches on a function declaration which has body, and removes the attribute later. This also avoid the ICE. Bootstrapped/regtested on x86_64-linux, ok for GCC10? Regards JunMa gcc/ChangeLog 2019-03-26 Jun Ma PR89341 * cgraphunit.c (handle_alias_pairs): Check whether alias attribute attaches on a function declaration which has body. gcc/testsuite/ChangeLog 2019-03-26 Jun Ma PR89341 * gcc.dg/attr-alias-6.c: New test. * gcc.dg/attr-weakref-5.c: Likewise. Hi Jakub Since you are owner of this part, so I add you to cc list. Ping? Regards JunMa
Re: [PATCH][RFC] Sanitize equals and hash functions in hash-tables.
On 11/8/18 9:56 AM, Martin Liška wrote: > On 11/7/18 11:23 PM, Jeff Law wrote: >> On 10/30/18 6:28 AM, Martin Liška wrote: >>> On 10/30/18 11:03 AM, Jakub Jelinek wrote: On Mon, Oct 29, 2018 at 04:14:21PM +0100, Martin Liška wrote: > +hashtab_chk_error () > +{ > + fprintf (stderr, "hash table checking failed: " > +"equal operator returns true for a pair " > +"of values with a different hash value"); BTW, either use internal_error here, or at least if using fprintf terminate with \n, in your recent mail I saw: ...different hash valueduring RTL pass: vartrack ^^ >>> Sure, fixed in attached patch. >>> >>> Martin >>> > + gcc_unreachable (); > +} Jakub >>> >>> 0001-Sanitize-equals-and-hash-functions-in-hash-tables.patch >>> >>> From 0d9c979c845580a98767b83c099053d36eb49bb9 Mon Sep 17 00:00:00 2001 >>> From: marxin >>> Date: Mon, 29 Oct 2018 09:38:21 +0100 >>> Subject: [PATCH] Sanitize equals and hash functions in hash-tables. >>> >>> --- >>> gcc/hash-table.h | 40 +++- >>> 1 file changed, 39 insertions(+), 1 deletion(-) >>> >>> diff --git a/gcc/hash-table.h b/gcc/hash-table.h >>> index bd83345c7b8..694eedfc4be 100644 >>> --- a/gcc/hash-table.h >>> +++ b/gcc/hash-table.h >>> @@ -503,6 +503,7 @@ private: >>> >>>value_type *alloc_entries (size_t n CXX_MEM_STAT_INFO) const; >>>value_type *find_empty_slot_for_expand (hashval_t); >>> + void verify (const compare_type , hashval_t hash); >>>bool too_empty_p (unsigned int); >>>void expand (); >>>static bool is_deleted (value_type ) >>> @@ -882,8 +883,12 @@ hash_table >>>if (insert == INSERT && m_size * 3 <= m_n_elements * 4) >>> expand (); >>> >>> - m_searches++; >>> +#if ENABLE_EXTRA_CHECKING >>> +if (insert == INSERT) >>> + verify (comparable, hash); >>> +#endif >>> >>> + m_searches++; >>>value_type *first_deleted_slot = NULL; >>>hashval_t index = hash_table_mod1 (hash, m_size_prime_index); >>>hashval_t hash2 = hash_table_mod2 (hash, m_size_prime_index); >>> @@ -930,6 +935,39 @@ hash_table >>>return _entries[index]; >>> } >>> >>> +#if ENABLE_EXTRA_CHECKING >>> + >>> +/* Report a hash table checking error. */ >>> + >>> +ATTRIBUTE_NORETURN ATTRIBUTE_COLD >>> +static void >>> +hashtab_chk_error () >>> +{ >>> + fprintf (stderr, "hash table checking failed: " >>> + "equal operator returns true for a pair " >>> + "of values with a different hash value\n"); >>> + gcc_unreachable (); >>> +} >> I think an internal_error here is probably still better than a simple >> fprintf, even if the fprintf is terminated with a \n :-) > > Fully agree with that, but I see a lot of build errors when using > internal_error. > >> >> The question then becomes can we bootstrap with this stuff enabled and >> if not, are we likely to soon? It'd be a shame to put it into >> EXTRA_CHECKING, but then not be able to really use EXTRA_CHECKING >> because we've got too many bugs to fix. > > Unfortunately it's blocked with these 2 PRs: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87845 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87847 Hi. I've just added one more PR: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90450 I'm sending updated version of the patch that provides a disablement for the 3 PRs with a new function disable_sanitize_eq_and_hash. With that I can bootstrap and finish tests. However, I've done that with a patch limits maximal number of checks: diff --git a/gcc/hash-table.h b/gcc/hash-table.h index dc24fea6405..57564914e31 100644 --- a/gcc/hash-table.h +++ b/gcc/hash-table.h @@ -1027,7 +1027,7 @@ void hash_table ::verify (const compare_type , hashval_t hash) { - for (size_t i = 0; i < m_size; i++) + for (size_t i = 0; i < MIN (m_size, 1000); i++) { value_type *entry = _entries[i]; if (!is_empty (*entry) && !is_deleted (*entry) Without that it would be probably terribly slow. Moreover, one probably does not want that with an extra checking, but with an extra-extra checking. Ideas about where to enable it? Would it be possible to add the sanitization with the aforementioned disablement? Thanks, Martin > > I'm fine with having the patch in in next stage1 after the problems will > be fixed. > > Martin > >> >>> + >>> +/* Verify that all existing elements in th hash table which are >> s/th/the/ >> >> >> Jeff >> > >From fee775b5f4443e6eaeb4028e9a8922ea4ec8703f Mon Sep 17 00:00:00 2001 From: marxin Date: Mon, 13 May 2019 07:16:22 +0200 Subject: [PATCH] Enable sanitization for hash tables. --- gcc/cp/pt.c| 4 gcc/cselib.c | 2 ++ gcc/hash-table.h | 53 -- gcc/tree-ssa-loop-im.c | 2 ++ 4 files changed, 59 insertions(+), 2 deletions(-) diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index d6976e08690..db9c953e3dd 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@
Re: [RFC] SHIFT_COUNT_TRUNCATED and shift_truncation_mask
Robin Dapp writes: >>> Bit tests on x86 also truncate [1], if the bit base operand specifies >>> a register, and we don't use BT with a memory location as a bit base. >>> I don't know what is referred with "(real or pretended) bit field >>> operations" in the documentation for SHIFT_COUNT_TRUNCATED: >>> >>> However, on some machines, such as the 80386 and the 680x0, >>> truncation only applies to shift operations and not the (real or >>> pretended) bit-field operations. Define 'SHIFT_COUNT_TRUNCATED' to >>> >>> Vector shifts don't truncate on x86, so x86 probably shares the same >>> destiny with MIPS. Maybe a machine mode argument could be passed to >>> SHIFT_COUNT_TRUNCATED to distinguish modes that truncate from modes >>> that don't. >> >> But IL semantic differences based on mode is even worse. What happens >> if STV then substitues a vector register/op but you previously optimized >> with the assumption the count would be truncated since the shift was SImode? >> >> IMHO a recipie for desaster. > > Would a shift_truncation_mask (rtx op, machine_mode mode) as replacement > for SHIFT_COUNT_TRUNCATED and the old hook make more sense than just > relying on the mode? That would be a way to alleviate the first counter > argument to SHIFT_COUNT_TRUNCATED ("not flexible enough"). Richard's > argument about the premature optimization however, I can agree on and I > don't see a way to avoid this. Doesn't it apply to other middle-end > optimizations as well, though, and is handled via additional flags that > prohibit selected follow-up optimizations? > > The problem with my proposed unifying is that the hook is/will be called > by RTL as well as tree passes and would need to have an argument type > that can hold all of their respective shift-like operations in order to > return a useful mask. My local version uses an rtx that only contains > an rtx_code which is filled in an ad-hoc fashion for the tree passes. > > That said, I don't intend to pursue the proposal given the opinions so > far - after all it would only save a few lines in our backend and some > others. I would find it useful to more clearly document the current > methods as deprecated since they are apparently not useful for any of > the major targets. The hook and macro do different things though. The hook just describes the behaviour of named shift patterns like ashl3. The only reason we have it is so that target-independent code can rely on certain behaviour when using these patterns (via optabs) to generate new shift instructions. In other words, the hook only applies to a specific target interface and doesn't change the meaning of the IL. By giving the guarantee that the shift optabs use a certain mask, the target has to ensure somehow that the instructions generated by the optabs really do honour that mask. But the target has to do that using the existing rtl semantics; the hook doesn't change what the shift rtx codes mean. Major targets can and do use that, and it helps expand generate more efficient code for doubleword operations (which was the original reason for adding it). So I think the hook is still useful and shouldn't be deprecated. IMO we should only consider deprecating the macro. I'm not sure how much practical effect that will have though. cc0 was deprecated ages ago but we're a still a long way from getting rid of it. :-) Thanks, Richard