Re: [PATCH 1/3] vec: add exact argument for various grow functions.
On 28/08/2020 15:26, Martin Liška wrote: On 8/28/20 4:24 PM, Andrew Stubbs wrote: Should I just add "true" to fix it? That's enough to make it build. Yes, please and push it as obvious. I've committed the attached. Andrew amdgcn: Update vec_safe_grow_cleared usage An API change broke the amdgcn build. gcc/ChangeLog: * config/gcn/gcn-tree.c (gcn_goacc_get_worker_red_decl): Add "true" parameter to vec_safe_grow_cleared. diff --git a/gcc/config/gcn/gcn-tree.c b/gcc/config/gcn/gcn-tree.c index 4dcb179ba71..a681426139b 100644 --- a/gcc/config/gcn/gcn-tree.c +++ b/gcc/config/gcn/gcn-tree.c @@ -456,7 +456,7 @@ gcn_goacc_get_worker_red_decl (tree type, unsigned offset) varpool_node::finalize_decl (decl); - vec_safe_grow_cleared (machfun->reduc_decls, offset + 1); + vec_safe_grow_cleared (machfun->reduc_decls, offset + 1, true); (*machfun->reduc_decls)[offset] = decl; return decl;
Re: [PATCH 1/3] vec: add exact argument for various grow functions.
On 8/28/20 12:49 AM, Martin Liška wrote: On 8/28/20 1:29 AM, Martin Sebor wrote: With --enable-valgrind-annotations the change to the member function signature in this patch triggers compilation errors during bootstrap: I must confirm I didn't tested the configuration. Feel free to install the patch, it's obvious. I checked it in r11-2925. Martin Thank you, Martin
Re: [PATCH 1/3] vec: add exact argument for various grow functions.
On 8/28/20 4:24 PM, Andrew Stubbs wrote: Should I just add "true" to fix it? That's enough to make it build. Yes, please and push it as obvious. Thanks, Martin
Re: [PATCH 1/3] vec: add exact argument for various grow functions.
On 11/08/2020 12:36, Martin Liška wrote: Hello. All right, I did it in 3 steps: 1) - new exact argument is added (no default value) - I tested the on x86_64-linux-gnu and I build all cross targets. 2) set default value of exact = false 3) change places which calculate its own growth to use the default argument I would like to install first 1) and then wait some time before the rest is installed. This broke the amdgcn-amdhsa build. Should I just add "true" to fix it? That's enough to make it build. Andrew
Re: [PATCH 1/3] vec: add exact argument for various grow functions.
On 8/28/20 1:29 AM, Martin Sebor wrote: With --enable-valgrind-annotations the change to the member function signature in this patch triggers compilation errors during bootstrap: I must confirm I didn't tested the configuration. Feel free to install the patch, it's obvious. Thank you, Martin
Re: [PATCH 1/3] vec: add exact argument for various grow functions.
With --enable-valgrind-annotations the change to the member function
signature in this patch triggers compilation errors during bootstrap:
/src/gcc/trunk/gcc/ggc-common.c: In function ‘void gt_pch_save(FILE*)’:
/src/gcc/trunk/gcc/ggc-common.c:509:33: error: no matching function for
call to ‘vec::safe_grow(size_t&)’
vbits.safe_grow (valid_size);
^
In file included from /src/gcc/trunk/gcc/hash-table.h:248,
from /src/gcc/trunk/gcc/coretypes.h:476,
from /src/gcc/trunk/gcc/ggc-common.c:26:
/src/gcc/trunk/gcc/vec.h:1897:1: note: candidate: ‘void
vec::safe_grow(unsigned int, bool) [with T = char]’
vec::safe_grow (unsigned len, bool exact
MEM_STAT_DECL)
^~~
/src/gcc/trunk/gcc/vec.h:1897:1: note: candidate expects 2 arguments,
1 provided
/src/gcc/trunk/gcc/jit/jit-recording.c: In member function ‘virtual
gcc::jit::recording::string*
gcc::jit::recording::switch_::make_debug_string()’:
/src/gcc/trunk/gcc/jit/jit-recording.c:6313:41: error: no matching
function for call to ‘auto_vec::safe_grow(size_t)’
6313 | cases_str.safe_grow (idx + 1 + len);
| ^
In file included from /src/gcc/trunk/gcc/hash-table.h:248,
from /src/gcc/trunk/gcc/coretypes.h:476,
from /src/gcc/trunk/gcc/jit/jit-recording.c:23:
/src/gcc/trunk/gcc/vec.h:1897:1: note: candidate: ‘void
vec::safe_grow(unsigned int, bool) [with T = char]’
1897 | vec::safe_grow (unsigned len, bool exact
MEM_STAT_DECL)
| ^~~
/src/gcc/trunk/gcc/vec.h:1897:1: note: candidate expects 2 arguments,
1 provided
Bootstrap succeeds with the patch below:
diff --git a/gcc/ggc-common.c b/gcc/ggc-common.c
index 0d528cf455c..d4691cc5448 100644
--- a/gcc/ggc-common.c
+++ b/gcc/ggc-common.c
@@ -506,7 +506,7 @@ gt_pch_save (FILE *f)
if (__builtin_expect (RUNNING_ON_VALGRIND, 0))
{
if (vbits.length () < valid_size)
- vbits.safe_grow (valid_size);
+ vbits.safe_grow (valid_size, true);
get_vbits = VALGRIND_GET_VBITS (state.ptrs[i]->obj,
vbits.address (), valid_size);
if (get_vbits == 3)
diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c
index b73cd76a0a0..a9e6528db69 100644
--- a/gcc/jit/jit-recording.c
+++ b/gcc/jit/jit-recording.c
@@ -6310,7 +6310,7 @@ recording::switch_::make_debug_string ()
{
size_t len = strlen (c->get_debug_string ());
unsigned idx = cases_str.length ();
- cases_str.safe_grow (idx + 1 + len);
+ cases_str.safe_grow (idx + 1 + len, true);
cases_str[idx] = ' ';
memcpy (&(cases_str[idx + 1]),
c->get_debug_string (),
Martin
Re: [PATCH 1/3] vec: add exact argument for various grow functions.
On Tue, 2020-08-11 at 13:36 +0200, Martin Liška wrote: > Hello. > > All right, I did it in 3 steps: > 1) - new exact argument is added (no default value) - I tested the on > x86_64-linux-gnu > and I build all cross targets. > 2) set default value of exact = false > 3) change places which calculate its own growth to use the default argument > > I would like to install first 1) and then wait some time before the rest is > installed. > > Thoughts? Given this is how Richi wanted the series to play out. #1 is fine with me as is the plan for the kit as a whole. Jeff >
Re: [PATCH 1/3] vec: add exact argument for various grow functions.
On 8/12/20 2:28 PM, Martin Liška wrote: I guess Richi can defend his strategy for this Richi? Martin
Re: [PATCH 1/3] vec: add exact argument for various grow functions.
On 8/12/20 6:28 AM, Martin Liška wrote: On 8/11/20 4:58 PM, Martin Sebor wrote: On 8/11/20 5:36 AM, Martin Liška wrote: Hello. All right, I did it in 3 steps: 1) - new exact argument is added (no default value) - I tested the on x86_64-linux-gnu and I build all cross targets. 2) set default value of exact = false 3) change places which calculate its own growth to use the default argument The usual intent of a default argument is to supply a value the function is the most commonly invoked with. But in this case it looks like it's the opposite: most of the callers (hundreds) provide the non-default value (true) and only a handful make use of the default. I feel I must be missing something. What is it? You are right, but Richi wanted to make this transformation in more defensive way. I'm eventually planning to drop the explicit 'true' argument for most of the places except selective scheduling and LTO streaming. If it's just transitional on the way toward the usual approach then that seems fine (although even then I can't say I understand the rationale for going this circuitous route). Thanks Martin I guess Richi can defend his strategy for this ;) ? Martin Martin I would like to install first 1) and then wait some time before the rest is installed. Thoughts? Martin
Re: [PATCH 1/3] vec: add exact argument for various grow functions.
On 8/11/20 4:58 PM, Martin Sebor wrote: On 8/11/20 5:36 AM, Martin Liška wrote: Hello. All right, I did it in 3 steps: 1) - new exact argument is added (no default value) - I tested the on x86_64-linux-gnu and I build all cross targets. 2) set default value of exact = false 3) change places which calculate its own growth to use the default argument The usual intent of a default argument is to supply a value the function is the most commonly invoked with. But in this case it looks like it's the opposite: most of the callers (hundreds) provide the non-default value (true) and only a handful make use of the default. I feel I must be missing something. What is it? You are right, but Richi wanted to make this transformation in more defensive way. I'm eventually planning to drop the explicit 'true' argument for most of the places except selective scheduling and LTO streaming. I guess Richi can defend his strategy for this ;) ? Martin Martin I would like to install first 1) and then wait some time before the rest is installed. Thoughts? Martin
Re: [PATCH 1/3] vec: add exact argument for various grow functions.
On 8/11/20 5:36 AM, Martin Liška wrote: Hello. All right, I did it in 3 steps: 1) - new exact argument is added (no default value) - I tested the on x86_64-linux-gnu and I build all cross targets. 2) set default value of exact = false 3) change places which calculate its own growth to use the default argument The usual intent of a default argument is to supply a value the function is the most commonly invoked with. But in this case it looks like it's the opposite: most of the callers (hundreds) provide the non-default value (true) and only a handful make use of the default. I feel I must be missing something. What is it? Martin I would like to install first 1) and then wait some time before the rest is installed. Thoughts? Martin
[PATCH 1/3] vec: add exact argument for various grow functions.
Hello. All right, I did it in 3 steps: 1) - new exact argument is added (no default value) - I tested the on x86_64-linux-gnu and I build all cross targets. 2) set default value of exact = false 3) change places which calculate its own growth to use the default argument I would like to install first 1) and then wait some time before the rest is installed. Thoughts? Martin >From c659680bd65bdaa749d8c07fc99b45542a872786 Mon Sep 17 00:00:00 2001 From: Martin Liska Date: Mon, 10 Aug 2020 11:11:05 +0200 Subject: [PATCH 1/3] vec: add exact argument for various grow functions. gcc/ada/ChangeLog: * gcc-interface/trans.c (gigi): Set exact argument of a vector growth function to true. (Attribute_to_gnu): Likewise. gcc/ChangeLog: * alias.c (init_alias_analysis): Set exact argument of a vector growth function to true. * calls.c (internal_arg_pointer_based_exp_scan): Likewise. * cfgbuild.c (find_many_sub_basic_blocks): Likewise. * cfgexpand.c (expand_asm_stmt): Likewise. * cfgrtl.c (rtl_create_basic_block): Likewise. * combine.c (combine_split_insns): Likewise. (combine_instructions): Likewise. * config/aarch64/aarch64-sve-builtins.cc (function_expander::add_output_operand): Likewise. (function_expander::add_input_operand): Likewise. (function_expander::add_integer_operand): Likewise. (function_expander::add_address_operand): Likewise. (function_expander::add_fixed_operand): Likewise. * df-core.c (df_worklist_dataflow_doublequeue): Likewise. * dwarf2cfi.c (update_row_reg_save): Likewise. * early-remat.c (early_remat::init_block_info): Likewise. (early_remat::finalize_candidate_indices): Likewise. * except.c (sjlj_build_landing_pads): Likewise. * final.c (compute_alignments): Likewise. (grow_label_align): Likewise. * function.c (temp_slots_at_level): Likewise. * fwprop.c (build_single_def_use_links): Likewise. (update_uses): Likewise. * gcc.c (insert_wrapper): Likewise. * genautomata.c (create_state_ainsn_table): Likewise. (add_vect): Likewise. (output_dead_lock_vect): Likewise. * genmatch.c (capture_info::capture_info): Likewise. (parser::finish_match_operand): Likewise. * genrecog.c (optimize_subroutine_group): Likewise. (merge_pattern_info::merge_pattern_info): Likewise. (merge_into_decision): Likewise. (print_subroutine_start): Likewise. (main): Likewise. * gimple-loop-versioning.cc (loop_versioning::loop_versioning): Likewise. * gimple.c (gimple_set_bb): Likewise. * graphite-isl-ast-to-gimple.c (translate_isl_ast_node_user): Likewise. * haifa-sched.c (sched_extend_luids): Likewise. (extend_h_i_d): Likewise. * insn-addr.h (insn_addresses_new): Likewise. * ipa-cp.c (gather_context_independent_values): Likewise. (find_more_contexts_for_caller_subset): Likewise. * ipa-devirt.c (final_warning_record::grow_type_warnings): Likewise. (ipa_odr_read_section): Likewise. * ipa-fnsummary.c (evaluate_properties_for_edge): Likewise. (ipa_fn_summary_t::duplicate): Likewise. (analyze_function_body): Likewise. (ipa_merge_fn_summary_after_inlining): Likewise. (read_ipa_call_summary): Likewise. * ipa-icf.c (sem_function::bb_dict_test): Likewise. * ipa-prop.c (ipa_alloc_node_params): Likewise. (parm_bb_aa_status_for_bb): Likewise. (ipa_compute_jump_functions_for_edge): Likewise. (ipa_analyze_node): Likewise. (update_jump_functions_after_inlining): Likewise. (ipa_read_edge_info): Likewise. (read_ipcp_transformation_info): Likewise. (ipcp_transform_function): Likewise. * ipa-reference.c (ipa_reference_write_optimization_summary): Likewise. * ipa-split.c (execute_split_functions): Likewise. * ira.c (find_moveable_pseudos): Likewise. * lower-subreg.c (decompose_multiword_subregs): Likewise. * lto-streamer-in.c (input_eh_regions): Likewise. (input_cfg): Likewise. (input_struct_function_base): Likewise. (input_function): Likewise. * modulo-sched.c (set_node_sched_params): Likewise. (extend_node_sched_params): Likewise. (schedule_reg_moves): Likewise. * omp-general.c (omp_construct_simd_compare): Likewise. * passes.c (pass_manager::create_pass_tab): Likewise. (enable_disable_pass): Likewise. * predict.c (determine_unlikely_bbs): Likewise. * profile.c (compute_branch_probabilities): Likewise. * read-rtl-function.c (function_reader::parse_block): Likewise. * read-rtl.c (rtx_reader::read_rtx_code): Likewise. * reg-stack.c (stack_regs_mentioned): Likewise. * regrename.c (regrename_init): Likewise. * rtlanal.c (T>::add_single_to_queue): Likewise. * sched-deps.c (init_deps_data_vector): Likewise. * sel-sched-ir.c (sel_extend_global_bb_info): Likewise. (extend_region_bb_info): Likewise. (extend_insn_data): Likewise. * symtab.c (symtab_node::create_reference): Likewise. * tracer.c (tail_duplicate): Likewise. * trans-mem.c (tm_region_init): Likewise. (get_bb_regions_instrumented): Likewise. * tree-cfg.c (init_empty_tree_cfg_for_function): Likewise. (build_gimple_cfg): Likewise. (create_bb): Likewise. (move_block_to_fn): Likewise. * tree-com
