[PATCH][OBVIOUS] Fix -Wformat-diag in options-save.c
The patch removes bunch of warnings: options-save.c:12004:29: warning: unquoted identifier or keyword ‘global_options’ in format [-Wformat-diag] 12004 | internal_error ("Error: global_options are modified in local context\n"); gcc/ChangeLog: * optc-save-gen.awk: Quote error string. --- gcc/optc-save-gen.awk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/optc-save-gen.awk b/gcc/optc-save-gen.awk index 4a0e5ab64f3..1b010085b75 100644 --- a/gcc/optc-save-gen.awk +++ b/gcc/optc-save-gen.awk @@ -967,7 +967,7 @@ for (i = 0; i < n_opts; i++) { checked_options[name]++ print " if (ptr1->x_" name " != ptr2->x_" name ")" - print "internal_error (\"Error: global_options are modified in local context\\n\");"; + print "internal_error (\"% are modified in local context\");"; } print "}"; -- 2.26.2
[PATCH 4/4 V2] vect: Factor out and rename some functions/macros
on 2020/6/11 上午12:58, Richard Sandiford wrote: > "Kewen.Lin" writes: >> diff --git a/gcc/tree-vect-loop-manip.c b/gcc/tree-vect-loop-manip.c >> index ca68d04a919..1fac5898525 100644 >> --- a/gcc/tree-vect-loop-manip.c >> +++ b/gcc/tree-vect-loop-manip.c >> @@ -420,8 +420,8 @@ vect_set_loop_controls_directly (class loop *loop, >> loop_vec_info loop_vinfo, >> rgroup_controls *rgc, tree niters, >> tree niters_skip, bool might_wrap_p) >> { >> - tree compare_type = LOOP_VINFO_MASK_COMPARE_TYPE (loop_vinfo); >> - tree iv_type = LOOP_VINFO_MASK_IV_TYPE (loop_vinfo); >> + tree compare_type = LOOP_VINFO_COMPARE_TYPE (loop_vinfo); >> + tree iv_type = LOOP_VINFO_IV_TYPE (loop_vinfo); > > How about s/MASK/RGROUP/ instead? COMPARE_TYPE and IV_TYPE sound a bit > too generic, and might give the impression that they're meaningful for > classic full-vector vectorisation too. > >> @@ -748,13 +748,12 @@ vect_set_loop_condition_masked (class loop *loop, >> loop_vec_info loop_vinfo, >> } >> >> /* Like vect_set_loop_condition, but handle the case in which there >> - are no loop masks. */ >> + are no partial vectorization loops. */ > > Maybe: > > … in which the vector loop handles exactly VF scalars per iteration. > >> diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c >> index 7ea75d6d095..b6e96f77f69 100644 >> --- a/gcc/tree-vect-loop.c >> +++ b/gcc/tree-vect-loop.c >> @@ -155,6 +155,7 @@ along with GCC; see the file COPYING3. If not see >> static void vect_estimate_min_profitable_iters (loop_vec_info, int *, int >> *); >> static stmt_vec_info vect_is_simple_reduction (loop_vec_info, stmt_vec_info, >> bool *, bool *); >> +static bool known_niters_smaller_than_vf (loop_vec_info); > > Please instead define the function before its first caller. > Adding “vect_” to the beginning of the name would probably be > more consistent. > >> [...] >> @@ -959,14 +960,41 @@ vect_get_max_nscalars_per_iter (loop_vec_info >> loop_vinfo) >>return res; >> } >> >> +/* Calculate the minimal bits necessary to represent the maximal iteration >> + count of loop with loop_vec_info LOOP_VINFO which is scaling with a given >> + factor FACTOR. */ > > How about: > > /* Calculate the minimum precision necessary to represent: > > MAX_NITERS * FACTOR > >as an unsigned integer, where MAX_NITERS is the maximum number of >loop header iterations for the original scalar form of LOOP_VINFO. */ > >> + >> +static unsigned >> +min_prec_for_max_niters (loop_vec_info loop_vinfo, unsigned int factor) > > Here too I think a “vect_” prefix would be more consistent. > >> +{ >> + class loop *loop = LOOP_VINFO_LOOP (loop_vinfo); >> + >> + /* Get the maximum number of iterations that is representable >> + in the counter type. */ >> + tree ni_type = TREE_TYPE (LOOP_VINFO_NITERSM1 (loop_vinfo)); >> + widest_int max_ni = wi::to_widest (TYPE_MAX_VALUE (ni_type)) + 1; >> + >> + /* Get a more refined estimate for the number of iterations. */ >> + widest_int max_back_edges; >> + if (max_loop_iterations (loop, &max_back_edges)) >> +max_ni = wi::smin (max_ni, max_back_edges + 1); >> + >> + /* Account for factor, in which each bit is replicated N times. */ > > The “, in which each bit …” part no longer makes sense in this generic > context. Probably best just to drop the comment altogether and… > >> + max_ni *= factor; >> + >> + /* Work out how many bits we need to represent the limit. */ >> + unsigned int min_ni_width = wi::min_precision (max_ni, UNSIGNED); >> + >> + return min_ni_width; > > …change this to: > > /* Work out how many bits we need to represent the limit. */ > return wi::min_precision (max_ni * factor, UNSIGNED); > >> [...] >> @@ -6813,8 +6820,8 @@ vectorizable_reduction (loop_vec_info loop_vinfo, >> { >>if (dump_enabled_p ()) >> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location, >> - "can't use a fully-masked loop because no" >> - " conditional operation is available.\n"); >> + "can't use a partial vectorization loop because" >> + " no conditional operation is available.\n"); > > Maybe “can't operate on partial vectors because…”. Same for the > later changes. > >> @@ -9194,12 +9202,13 @@ optimize_mask_stores (class loop *loop) >> } >> >> /* Decide whether it is possible to use a zero-based induction variable >> - when vectorizing LOOP_VINFO with a fully-masked loop. If it is, >> - return the value that the induction variable must be able to hold >> - in order to ensure that the loop ends with an all-false mask. >> + when vectorizing LOOP_VINFO with a partial vectorization loop. If > > Maybe ”…with partial vectors” > >> + it is, return the value that the induction variable must be able to >> + hold in order to ensure tha
Re: [committed] gcc-changelog: Use non-zero exit status on error
On 6/10/20 2:15 PM, Jonathan Wakely wrote: And this is another little patch that just avoids running 'git diff' twice, by using tee(1) to write to $GCC_GIT_DIFF_FILE as a side effect of generating the diff to pipe to mklog.py. Thanks, please install the improvement. Martin
Re: [committed] gcc-changelog: Use non-zero exit status on error
On 6/10/20 2:20 PM, Jonathan Wakely wrote: Oops, this line was left in while I was testing it by amending existing commits! Here's an updated patch without that line. I generally like the suggested idea and I have suggestion to usage of the GCC_FORCE_MKLOG. Right now, we use the env variable only in 'git config alias.gcc-commit-mklog' alias. Wouldn't it be better to only add git config gcc-config.mklog-hook-type [always|smart-amend] and do not support GCC_FORCE_MKLOG=no or GCC_FORCE_MKLOG=smart-amend? Or do you have always GCC_FORCE_MKLOG=1 on (I mean for git commit)? Thanks, Martin
Re: [committed] gcc-changelog: Use non-zero exit status on error
On 6/11/20 9:45 AM, Martin Liška wrote: On 6/10/20 2:15 PM, Jonathan Wakely wrote: And this is another little patch that just avoids running 'git diff' twice, by using tee(1) to write to $GCC_GIT_DIFF_FILE as a side effect of generating the diff to pipe to mklog.py. Thanks, please install the improvement. Martin I've pushed the patch as I'm going to replace my GCC_GIT_DIFF_FILE environment variable with a more feasible 'git config gcc-config.diff-file'. Pushed to master as well. Martin >From 2dd6d9d4029d99c5ad54a21677506bd25e3f7540 Mon Sep 17 00:00:00 2001 From: Martin Liska Date: Thu, 11 Jun 2020 10:02:26 +0200 Subject: [PATCH] prepare-commit-hook: Use gcc-config.diff-file. contrib/ChangeLog: * prepare-commit-msg: Replace ENV variable with a git config value. --- contrib/prepare-commit-msg | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/contrib/prepare-commit-msg b/contrib/prepare-commit-msg index 57bb91747f6..d4971e65092 100755 --- a/contrib/prepare-commit-msg +++ b/contrib/prepare-commit-msg @@ -58,8 +58,9 @@ else fi # Save diff to a file if requested. -if ! [ -z "$GCC_GIT_DIFF_FILE" ]; then -tee="tee $GCC_GIT_DIFF_FILE" +DIFF_FILE = $(git config gcc-config.diff-file) +if ! [ -z "$DIFF_FILE" ]; then +tee="tee $DIFF_FILE" else tee="cat" fi -- 2.26.2
[PATCH] asan: fix RTX emission for ilp32
Hello. There's a patch for ilp32 where we should use Pmode instead of ptr_mode. Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? Thanks, Martin gcc/ChangeLog: PR sanitizer/95634 * asan.c (asan_emit_stack_protection): Fix emission for ilp32 by using Pmode instead of ptr_mode. --- gcc/asan.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc/asan.c b/gcc/asan.c index e015fa3ec9b..5d123a3e8a6 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -1610,8 +1610,8 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb, = (1 << (use_after_return_class + 6)); offset -= GET_MODE_SIZE (ptr_mode); mem = gen_rtx_MEM (ptr_mode, base); - mem = adjust_address (mem, ptr_mode, offset); - rtx addr = gen_reg_rtx (ptr_mode); + mem = adjust_address (mem, Pmode, offset); + rtx addr = gen_reg_rtx (Pmode); emit_move_insn (addr, mem); mem = gen_rtx_MEM (QImode, addr); emit_move_insn (mem, const0_rtx); -- 2.26.2
[PATCH] Fix few -Wformat-diag warnings.
Ready for master? Thanks, Martin gcc/ChangeLog: * cgraphunit.c (process_symver_attribute): Wrap weakref keyword. * dbgcnt.c (dbg_cnt_set_limit_by_index): Do not print extra new line. * lto-wrapper.c (merge_and_complain): Wrap option names. --- gcc/cgraphunit.c | 2 +- gcc/dbgcnt.c | 2 +- gcc/lto-wrapper.c | 12 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c index 01b3f82a4b2..ea9a34bda6f 100644 --- a/gcc/cgraphunit.c +++ b/gcc/cgraphunit.c @@ -762,7 +762,7 @@ process_symver_attribute (symtab_node *n) if (n->weakref) { error_at (DECL_SOURCE_LOCATION (n->decl), - "weakref cannot be versioned"); + "% cannot be versioned"); return; } if (!TREE_PUBLIC (n->decl)) diff --git a/gcc/dbgcnt.c b/gcc/dbgcnt.c index b0dd893ed49..ae98a281d63 100644 --- a/gcc/dbgcnt.c +++ b/gcc/dbgcnt.c @@ -126,7 +126,7 @@ dbg_cnt_set_limit_by_index (enum debug_counter index, const char *name, if (t1.first <= t2.second) { error ("Interval overlap of %<-fdbg-cnt=%s%>: [%u, %u] and " -"[%u, %u]\n", name, t2.first, t2.second, t1.first, t1.second); +"[%u, %u]", name, t2.first, t2.second, t1.first, t1.second); return false; } } diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c index d565b0861f5..8fbca7cabc4 100644 --- a/gcc/lto-wrapper.c +++ b/gcc/lto-wrapper.c @@ -508,24 +508,24 @@ merge_and_complain (struct cl_decoded_option **decoded_options, break; else if (i < *decoded_options_count && j == fdecoded_options_count) { - warning (0, "Extra option to -Xassembler: %s," -" dropping all -Xassembler and -Wa options.", + warning (0, "Extra option to %<-Xassembler%>: %s," +" dropping all %<-Xassembler%> and %<-Wa%> options.", (*decoded_options)[i].arg); xassembler_options_error = true; break; } else if (i == *decoded_options_count && j < fdecoded_options_count) { - warning (0, "Extra option to -Xassembler: %s," -" dropping all -Xassembler and -Wa options.", + warning (0, "Extra option to %<-Xassembler%>: %s," +" dropping all %<-Xassembler%> and %<-Wa%> options.", fdecoded_options[j].arg); xassembler_options_error = true; break; } else if (strcmp ((*decoded_options)[i].arg, fdecoded_options[j].arg)) { - warning (0, "Options to Xassembler do not match: %s, %s," -" dropping all -Xassembler and -Wa options.", + warning (0, "Options to %<-Xassembler%> do not match: %s, %s," +" dropping all %<-Xassembler%> and %<-Wa%> options.", (*decoded_options)[i].arg, fdecoded_options[j].arg); xassembler_options_error = true; break; -- 2.26.2
Re: [committed] gcc-changelog: Use non-zero exit status on error
On 11/06/20 09:54 +0200, Martin Liška wrote: On 6/10/20 2:20 PM, Jonathan Wakely wrote: Oops, this line was left in while I was testing it by amending existing commits! Here's an updated patch without that line. I generally like the suggested idea and I have suggestion to usage of the GCC_FORCE_MKLOG. Right now, we use the env variable only in 'git config alias.gcc-commit-mklog' alias. Ah yes, I forgot about that alias (I don't use it). Wouldn't it be better to only add git config gcc-config.mklog-hook-type [always|smart-amend] and do not support GCC_FORCE_MKLOG=no or GCC_FORCE_MKLOG=smart-amend? Sure, that works for me. Or do you have always GCC_FORCE_MKLOG=1 on (I mean for git commit)? No, I was actually using a modified version of the script that didn't check the env var at all, because I always wanted it to be used. I'll prepare a patch that replaces the env var completely, and also modifies the contrib/gcc-git-customization.sh script to optionally enable the hook. Do we want to drop the gcc-config.gcc-commit-mklog alias now, since it won't do anything different from 'git commit'?
Re: [PATCH] asan: fix RTX emission for ilp32
On Thu, Jun 11, 2020 at 10:12:14AM +0200, Martin Liška wrote: > gcc/ChangeLog: > > PR sanitizer/95634 > * asan.c (asan_emit_stack_protection): Fix emission for ilp32 > by using Pmode instead of ptr_mode. > --- > gcc/asan.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/gcc/asan.c b/gcc/asan.c > index e015fa3ec9b..5d123a3e8a6 100644 > --- a/gcc/asan.c > +++ b/gcc/asan.c > @@ -1610,8 +1610,8 @@ asan_emit_stack_protection (rtx base, rtx pbase, > unsigned int alignb, > = (1 << (use_after_return_class + 6)); > offset -= GET_MODE_SIZE (ptr_mode); > mem = gen_rtx_MEM (ptr_mode, base); > - mem = adjust_address (mem, ptr_mode, offset); > - rtx addr = gen_reg_rtx (ptr_mode); > + mem = adjust_address (mem, Pmode, offset); > + rtx addr = gen_reg_rtx (Pmode); That is not correct. On the architectures where ptr_mode != Pmode, when you are reading a pointer from memory, you want to use ptr_mode, because that is how the pointer is represented in memory. So, it needs to stay: mem = gen_rtx_MEM (ptr_mode, base); mem = adjust_address (mem, ptr_mode, offset); rtx addr = gen_reg_rtx (ptr_mode); emit_move_insn (addr, mem); But, at this point addr is ptr_mode, but you need to convert it into Pmode. addr = convert_memory_address (Pmode, addr); This one will do nothing at all on normal arches where ptr_mode == Pmode, and perform some extension (zero/sign/whatever else the arch needs) otherwise. > mem = gen_rtx_MEM (QImode, addr); > emit_move_insn (mem, const0_rtx); > -- > 2.26.2 Jakub
Re: [stage1][PATCH] Lower VEC_COND_EXPR into internal functions.
On 6/9/20 3:42 PM, Richard Biener wrote: I think we need to fix that before merging. There's updated version of the patch that should handle the EH properly. Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? Thanks, Martin >From fc5a59e8c8887c102bff06e1a537ccfc9d44e3d8 Mon Sep 17 00:00:00 2001 From: Martin Liska Date: Mon, 9 Mar 2020 13:23:03 +0100 Subject: [PATCH] Lower VEC_COND_EXPR into internal functions. gcc/ChangeLog: 2020-03-30 Martin Liska * expr.c (expand_expr_real_2): Put gcc_unreachable, we should reach this path. (do_store_flag): Likewise here. * internal-fn.c (vec_cond_mask_direct): New. (vec_cond_direct): Likewise. (vec_condu_direct): Likewise. (vec_condeq_direct): Likewise. (expand_vect_cond_optab_fn): Move from optabs.c. (expand_vec_cond_optab_fn): New alias. (expand_vec_condu_optab_fn): Likewise. (expand_vec_condeq_optab_fn): Likewise. (expand_vect_cond_mask_optab_fn): Moved from optabs.c. (expand_vec_cond_mask_optab_fn): New alias. (direct_vec_cond_mask_optab_supported_p): New. (direct_vec_cond_optab_supported_p): Likewise. (direct_vec_condu_optab_supported_p): Likewise. (direct_vec_condeq_optab_supported_p): Likewise. * internal-fn.def (VCOND): New new internal optab function. (VCONDU): Likewise. (VCONDEQ): Likewise. (VCOND_MASK): Likewise. * optabs.c (expand_vec_cond_mask_expr): Removed. (expand_vec_cond_expr): Likewise. * optabs.h (expand_vec_cond_expr): Likewise. (vector_compare_rtx): Likewise. * passes.def: Add pass_gimple_isel. * tree-cfg.c (verify_gimple_assign_ternary): Add new GIMPLE check. * tree-pass.h (make_pass_gimple_isel): New. * tree-ssa-forwprop.c (pass_forwprop::execute): Do not forward to already lowered VEC_COND_EXPR. * tree-vect-generic.c (expand_vector_divmod): Expand to SSA_NAME. (expand_vector_condition): Expand tcc_comparison of a VEC_COND_EXPR into a SSA_NAME. (expand_vector_condition): Add new argument. (expand_vector_operations): Likewise. (expand_vector_operations_1): Fix up EH by moving that to vector comparison. * tree-vect-isel.c: New file. gcc/testsuite/ChangeLog: * g++.dg/vect/vec-cond-expr-eh.C: New test. --- gcc/Makefile.in | 2 + gcc/expr.c | 25 +- gcc/internal-fn.c| 89 +++ gcc/internal-fn.def | 5 + gcc/optabs.c | 124 +- gcc/optabs.h | 7 +- gcc/passes.def | 1 + gcc/testsuite/g++.dg/vect/vec-cond-expr-eh.C | 17 ++ gcc/tree-cfg.c | 8 + gcc/tree-pass.h | 1 + gcc/tree-ssa-forwprop.c | 6 + gcc/tree-vect-generic.c | 71 -- gcc/tree-vect-isel.c | 244 +++ 13 files changed, 431 insertions(+), 169 deletions(-) create mode 100644 gcc/testsuite/g++.dg/vect/vec-cond-expr-eh.C create mode 100644 gcc/tree-vect-isel.c diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 4f70c189b9d..4cbb9d23606 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -1631,6 +1631,7 @@ OBJS = \ tree-streamer-out.o \ tree-tailcall.o \ tree-vect-generic.o \ + tree-vect-isel.o \ tree-vect-patterns.o \ tree-vect-data-refs.o \ tree-vect-stmts.o \ @@ -2600,6 +2601,7 @@ GTFILES = $(CPPLIB_H) $(srcdir)/input.h $(srcdir)/coretypes.h \ $(srcdir)/dwarf2cfi.c \ $(srcdir)/dwarf2out.c \ $(srcdir)/tree-vect-generic.c \ + $(srcdir)/tree-vect-isel.c \ $(srcdir)/dojump.c $(srcdir)/emit-rtl.h \ $(srcdir)/emit-rtl.c $(srcdir)/except.h $(srcdir)/explow.c $(srcdir)/expr.c \ $(srcdir)/expr.h \ diff --git a/gcc/expr.c b/gcc/expr.c index ca6b1c1291e..3c68b0d754c 100644 --- a/gcc/expr.c +++ b/gcc/expr.c @@ -9316,17 +9316,8 @@ expand_expr_real_2 (sepops ops, rtx target, machine_mode tmode, if (temp != 0) return temp; - /* For vector MIN , expand it a VEC_COND_EXPR - and similarly for MAX . */ if (VECTOR_TYPE_P (type)) - { - tree t0 = make_tree (type, op0); - tree t1 = make_tree (type, op1); - tree comparison = build2 (code == MIN_EXPR ? LE_EXPR : GE_EXPR, -type, t0, t1); - return expand_vec_cond_expr (type, comparison, t0, t1, - original_target); - } + gcc_unreachable (); /* At this point, a MEM target is no longer useful; we will get better code without it. */ @@ -9915,10 +9906,6 @@ expand_expr_real_2 (sepops ops, rtx target, machine_mode tmode, return temp; } -case VEC_COND_EXPR: - target = expand_vec_cond_expr (type, treeop0, treeop1, treeop2, target); - return target; - case VEC_DUPLICATE_EXPR: op0 = expand_expr (treeop0, NULL_RTX, VOIDmode, modifier); target = expand_vector_broadcast (mode, op0); @@ -12249,8 +12236,7 @@ do_store_flag (sepops ops, rtx target, machine_mode mode) S
Re: [committed] gcc-changelog: Use non-zero exit status on error
On 6/11/20 10:48 AM, Jonathan Wakely wrote: On 11/06/20 09:54 +0200, Martin Liška wrote: On 6/10/20 2:20 PM, Jonathan Wakely wrote: Oops, this line was left in while I was testing it by amending existing commits! Here's an updated patch without that line. I generally like the suggested idea and I have suggestion to usage of the GCC_FORCE_MKLOG. Right now, we use the env variable only in 'git config alias.gcc-commit-mklog' alias. Ah yes, I forgot about that alias (I don't use it). I use it (and I want to distinguish in between it and normal commit command) ;) Wouldn't it be better to only add git config gcc-config.mklog-hook-type [always|smart-amend] and do not support GCC_FORCE_MKLOG=no or GCC_FORCE_MKLOG=smart-amend? Sure, that works for me. Or do you have always GCC_FORCE_MKLOG=1 on (I mean for git commit)? No, I was actually using a modified version of the script that didn't check the env var at all, because I always wanted it to be used. I bet some other people (including me) want to have 2 different commands. I'll prepare a patch that replaces the env var completely, and also modifies the contrib/gcc-git-customization.sh script to optionally enable the hook. I would preserve it for the alias. Martin Do we want to drop the gcc-config.gcc-commit-mklog alias now, since it won't do anything different from 'git commit'?
Re: [PATCH] asan: fix RTX emission for ilp32
On 6/11/20 10:50 AM, Jakub Jelinek wrote: On Thu, Jun 11, 2020 at 10:12:14AM +0200, Martin Liška wrote: gcc/ChangeLog: PR sanitizer/95634 * asan.c (asan_emit_stack_protection): Fix emission for ilp32 by using Pmode instead of ptr_mode. --- gcc/asan.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc/asan.c b/gcc/asan.c index e015fa3ec9b..5d123a3e8a6 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -1610,8 +1610,8 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb, = (1 << (use_after_return_class + 6)); offset -= GET_MODE_SIZE (ptr_mode); mem = gen_rtx_MEM (ptr_mode, base); - mem = adjust_address (mem, ptr_mode, offset); - rtx addr = gen_reg_rtx (ptr_mode); + mem = adjust_address (mem, Pmode, offset); + rtx addr = gen_reg_rtx (Pmode); That is not correct. On the architectures where ptr_mode != Pmode, when you are reading a pointer from memory, you want to use ptr_mode, because that is how the pointer is represented in memory. So, it needs to stay: mem = gen_rtx_MEM (ptr_mode, base); mem = adjust_address (mem, ptr_mode, offset); rtx addr = gen_reg_rtx (ptr_mode); emit_move_insn (addr, mem); But, at this point addr is ptr_mode, but you need to convert it into Pmode. addr = convert_memory_address (Pmode, addr); This one will do nothing at all on normal arches where ptr_mode == Pmode, and perform some extension (zero/sign/whatever else the arch needs) otherwise. Thank you for help, I'm going to push the patch. Martin mem = gen_rtx_MEM (QImode, addr); emit_move_insn (mem, const0_rtx); -- 2.26.2 Jakub
Re: [committed] gcc-changelog: Use non-zero exit status on error
On 11/06/20 10:54 +0200, Martin Liška wrote: On 6/11/20 10:48 AM, Jonathan Wakely wrote: On 11/06/20 09:54 +0200, Martin Liška wrote: On 6/10/20 2:20 PM, Jonathan Wakely wrote: Oops, this line was left in while I was testing it by amending existing commits! Here's an updated patch without that line. I generally like the suggested idea and I have suggestion to usage of the GCC_FORCE_MKLOG. Right now, we use the env variable only in 'git config alias.gcc-commit-mklog' alias. Ah yes, I forgot about that alias (I don't use it). I use it (and I want to distinguish in between it and normal commit command) ;) OK. I'd rather just have one command that always does the right thing, so want the hook to always run. But I can just change my local copy. Wouldn't it be better to only add git config gcc-config.mklog-hook-type [always|smart-amend] and do not support GCC_FORCE_MKLOG=no or GCC_FORCE_MKLOG=smart-amend? The point of GCC_FORCE_MKLOG=no was as an override to disable the hook if you have it enabled in your git config options. If my git config enables the hook, but I want it off for a single commit, I can use the env var to disable it. For your scenario, you want the hook off by default, but enabled when the env var is set (by the gcc-commit-mklog alias). Sure, that works for me. Or do you have always GCC_FORCE_MKLOG=1 on (I mean for git commit)? No, I was actually using a modified version of the script that didn't check the env var at all, because I always wanted it to be used. I bet some other people (including me) want to have 2 different commands. I'll prepare a patch that replaces the env var completely, and also modifies the contrib/gcc-git-customization.sh script to optionally enable the hook. I would preserve it for the alias. OK, I misunderstood what you were suggesting. Is the attached patch what you're asking for? Martin Do we want to drop the gcc-config.gcc-commit-mklog alias now, since it won't do anything different from 'git commit'? commit c0e9ad58b132d01a3d7bb7eaeb981324eb55074a Author: Jonathan Wakely Date: Thu Jun 11 10:22:04 2020 +0100 contrib: Make prepare-commit-msg hook smarter for amends With this change the prepare-commit-msg hook can compare the log of a commit being amended with the staged changes, and not run mklog.py unnecessarily. This is controlled by a git config option, gcc-config.mklog-hook-type. contrib/ChangeLog: * prepare-commit-msg: Use the gcc-config.mklog-hook-type Git config key instead of the GCC_FORCE_MKLOG environment variable. Optionally disable generating a new ChangeLog template for amended commits when the existing log is still OK. diff --git a/contrib/prepare-commit-msg b/contrib/prepare-commit-msg index cc9ba2e6ba1..969847df6f4 100755 --- a/contrib/prepare-commit-msg +++ b/contrib/prepare-commit-msg @@ -49,6 +49,19 @@ elif [ $COMMIT_SOURCE = commit ]; then # otherwise, assume a new commit with -C. if [ $SHA1 = HEAD ]; then cmd="diff --cached HEAD^" + if [ "$(git config gcc-config.mklog-hook-type)" = "smart-amend" ]; then + # Check if the existing message still describes the staged changes. + f=$(mktemp /tmp/git-commit.XX) || exit 1 + git log -1 --pretty=email HEAD > $f + printf '\n---\n\n' >> $f + git $cmd >> $f + if contrib/gcc-changelog/git_email.py "$f" >/dev/null 2>&1; then + # Existing commit message is still OK for amended commit. + rm $f + exit 0 + fi + rm $f + fi else cmd="diff --cached" fi
Re: [committed] gcc-changelog: Use non-zero exit status on error
On 6/11/20 11:47 AM, Jonathan Wakely wrote: Is the attached patch what you're asking for? Yes ;) Martin
[Ada] Remove useless code in Backend_Processing_Possible
The call to Set_Size_Known_At_Compile_Time in Backend_Processing_Possible happened just after querying Size_Known_At_Compile_Time several lines before. Both calls operated on the same type entity, so the call to "Set" routine had no effect and now is removed. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Piotr Trojanek gcc/ada/ * exp_aggr.adb (Backend_Processing_Possible): Remove useless call.--- gcc/ada/exp_aggr.adb +++ gcc/ada/exp_aggr.adb @@ -743,7 +743,6 @@ package body Exp_Aggr is -- Backend processing is possible - Set_Size_Known_At_Compile_Time (Etype (N), True); return True; end Backend_Processing_Possible;
[Ada] AI12-0356 Root_Storage_Pool_With_Subpools & Preelaborable_Init
This Ada 202x AI clarifies that Root_Storage_Pool_With_Subpools and Root_Subpool should have pragma Preelaborable_Initialization. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Arnaud Charlet gcc/ada/ * libgnat/s-stposu.ads (Root_Storage_Pool_With_Subpools, Root_Subpool): Mark with Preelaborable_Initialization.--- gcc/ada/libgnat/s-stposu.ads +++ gcc/ada/libgnat/s-stposu.ads @@ -42,12 +42,14 @@ package System.Storage_Pools.Subpools is type Root_Storage_Pool_With_Subpools is abstract new Root_Storage_Pool with private; + pragma Preelaborable_Initialization (Root_Storage_Pool_With_Subpools); -- The base for all implementations of Storage_Pool_With_Subpools. This -- type is Limited_Controlled by derivation. To use subpools, an access -- type must be associated with an implementation descending from type -- Root_Storage_Pool_With_Subpools. type Root_Subpool is abstract tagged limited private; + pragma Preelaborable_Initialization (Root_Subpool); -- The base for all implementations of Subpool. Objects of this type are -- managed by the pool_with_subpools.
[Ada] Fix unnesting crash with Predicate_Failure/no pred
This patch fixes a bug where if you have a Predicate_Failure aspect on a nested type, but no Predicate, Static_Predicate, or Dynamic_Predicate, the compiler crashes when compiled with assertions enabled. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Bob Duff gcc/ada/ * sem_ch13.adb (Analyze_Aspect_Specifications): Do not set the Has_Predicates flag when the Predicate_Failure aspect is seen. It is legal (but pointless) to use this aspect without a predicate. If we set the flag, we generate a half-baked Predicate procedure, and if that procedure is nested, it causes unnesting to crash.--- gcc/ada/sem_ch13.adb +++ gcc/ada/sem_ch13.adb @@ -2540,8 +2540,6 @@ package body Sem_Ch13 is Expression => Relocate_Node (Expr))), Pragma_Name => Name_Predicate_Failure); - Set_Has_Predicates (E); - -- If the type is private, indicate that its completion -- has a freeze node, because that is the one that will -- be visible at freeze time.
[Ada] Refine type for sorting case-choices tables
Tables with case-choices are sorted by Sort_Case_Table with an insertion sort. Contrary to what comments for the Case_Table_Type says, this routine doesn't use the table element at index 0 as a placeholder. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Piotr Trojanek gcc/ada/ * sem_aggr.adb (Case_Table_Type): Change index type from Nat to Pos.--- gcc/ada/sem_aggr.adb +++ gcc/ada/sem_aggr.adb @@ -85,9 +85,8 @@ package body Sem_Aggr is -- The node of the choice end record; - type Case_Table_Type is array (Nat range <>) of Case_Bounds; - -- Table type used by Check_Case_Choices procedure. Entry zero is not - -- used (reserved for the sort). Real entries start at one. + type Case_Table_Type is array (Pos range <>) of Case_Bounds; + -- Table type used by Check_Case_Choices procedure --- -- Local Subprograms -- @@ -1827,9 +1826,8 @@ package body Sem_Aggr is -- if a choice in an aggregate is a subtype indication these -- denote the lowest and highest values of the subtype -Table : Case_Table_Type (0 .. Case_Table_Size); --- Used to sort all the different choice values. Entry zero is --- reserved for sorting purposes. +Table : Case_Table_Type (1 .. Case_Table_Size); +-- Used to sort all the different choice values Single_Choice : Boolean; -- Set to true every time there is a single discrete choice in a
[Ada] Generate predicate checks for on assignments in records
When an assignment of an allocator the type of which has predicate checks is made inside of a record, GNAT generates a call to the predicate function. However, before this commit, GNAT wouldn't check if the subtype mark had a predicate, which would result in the predicate check function not being called if no temporary was created for the designated object. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Ghjuvan Lacambre gcc/ada/ * exp_ch3.adb (Build_Assignment): Generate predicate check if subtype mark has predicate.--- gcc/ada/exp_ch3.adb +++ gcc/ada/exp_ch3.adb @@ -1998,6 +1998,20 @@ package body Exp_Ch3 is Append (Make_Predicate_Check (Typ, Exp), Res); end if; + if Nkind (Exp) = N_Allocator +and then Nkind (Expression (Exp)) = N_Qualified_Expression + then +declare + Subtype_Entity : constant Entity_Id + := Entity (Subtype_Mark (Expression (Exp))); +begin + if Has_Predicates (Subtype_Entity) then + Append (Make_Predicate_Check + (Subtype_Entity, Expression (Expression (Exp))), Res); + end if; +end; + end if; + return Res; exception
[Ada] Missing accessibility error on object in type conversion
This patch corrects an issue whereby the compiler would incorrectly calculate accessibility levels of objects within type conversions - leading to potentially missing static and dynamic errors. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Justin Squirek gcc/ada/ * sem_util.adb (Expand_N_Attribute_Reference): Use original nodes where required to avoid looking at the expanded tree.--- gcc/ada/sem_util.adb +++ gcc/ada/sem_util.adb @@ -23175,18 +23175,20 @@ package body Sem_Util is -- Local variables - E : Entity_Id; + E: Entity_Id; + Orig_Obj : constant Node_Id := Original_Node (Obj); + Orig_Pre : Node_Id; -- Start of processing for Object_Access_Level begin - if Nkind (Obj) = N_Defining_Identifier -or else Is_Entity_Name (Obj) + if Nkind (Orig_Obj) = N_Defining_Identifier +or else Is_Entity_Name (Orig_Obj) then - if Nkind (Obj) = N_Defining_Identifier then -E := Obj; + if Nkind (Orig_Obj) = N_Defining_Identifier then +E := Orig_Obj; else -E := Entity (Obj); +E := Entity (Orig_Obj); end if; if Is_Prival (E) then @@ -23220,14 +23222,17 @@ package body Sem_Util is return Scope_Depth (Enclosing_Dynamic_Scope (E)); end if; - elsif Nkind_In (Obj, N_Indexed_Component, N_Selected_Component) then - if Is_Access_Type (Etype (Prefix (Obj))) then -return Type_Access_Level (Etype (Prefix (Obj))); + elsif Nkind_In (Orig_Obj, N_Indexed_Component, N_Selected_Component) then + Orig_Pre := Original_Node (Prefix (Orig_Obj)); + + if Is_Access_Type (Etype (Orig_Pre)) then +return Type_Access_Level (Etype (Prefix (Orig_Obj))); else -return Object_Access_Level (Prefix (Obj)); +return Object_Access_Level (Prefix (Orig_Obj)); end if; - elsif Nkind (Obj) = N_Explicit_Dereference then + elsif Nkind (Orig_Obj) = N_Explicit_Dereference then + Orig_Pre := Original_Node (Prefix (Orig_Obj)); -- If the prefix is a selected access discriminant then we make a -- recursive call on the prefix, which will in turn check the level @@ -23239,46 +23244,48 @@ package body Sem_Util is -- otherwise expansion will already have transformed the prefix into -- a temporary. - if Nkind (Prefix (Obj)) = N_Selected_Component - and then Ekind (Etype (Prefix (Obj))) = E_Anonymous_Access_Type + if Nkind (Orig_Pre) = N_Selected_Component + and then Ekind (Etype (Orig_Pre)) = E_Anonymous_Access_Type and then - Ekind (Entity (Selector_Name (Prefix (Obj = E_Discriminant + Ekind (Entity (Selector_Name (Orig_Pre))) = E_Discriminant and then (not Has_Implicit_Dereference -(Entity (Selector_Name (Prefix (Obj +(Entity (Selector_Name (Orig_Pre))) or else Nkind (Parent (Obj)) /= N_Selected_Component) then -return Object_Access_Level (Prefix (Obj)); +return Object_Access_Level (Prefix (Orig_Obj)); -- Detect an interface conversion in the context of a dispatching -- call. Use the original form of the conversion to find the access -- level of the operand. - elsif Is_Interface (Etype (Obj)) - and then Is_Interface_Conversion (Prefix (Obj)) - and then Nkind (Original_Node (Obj)) = N_Type_Conversion + elsif Is_Interface (Etype (Orig_Obj)) + and then Is_Interface_Conversion (Orig_Pre) + and then Nkind (Orig_Obj) = N_Type_Conversion then -return Object_Access_Level (Original_Node (Obj)); +return Object_Access_Level (Orig_Obj); - elsif not Comes_From_Source (Obj) then + elsif not Comes_From_Source (Orig_Obj) then declare - Ref : constant Node_Id := Reference_To (Obj); + Ref : constant Node_Id := Reference_To (Orig_Obj); begin if Present (Ref) then return Object_Access_Level (Ref); else - return Type_Access_Level (Etype (Prefix (Obj))); + return Type_Access_Level (Etype (Prefix (Orig_Obj))); end if; end; else -return Type_Access_Level (Etype (Prefix (Obj))); +return Type_Access_Level (Etype (Prefix (Orig_Obj))); end if; - elsif Nkind_In (Obj, N_Type_Conversion, N_Unchecked_Type_Conversion) then - return Object_Access_Level (Expression (Obj)); + elsif Nkind_In (Orig_Obj, N_Type_Conversion, +N_Unchecked_Type_Conversion) + then + return Object_Access_Level (Expression (O
[Ada] Make Object Specific Dispatch tables constant
Internally generated static dispatch tables are preferably put in ROM, so they are declared as constant where possible. However, the Object Specific Dispatch tables were declared as variables, even though they are initialized with static aggregates (with only literal integers) and are never modified (either by the GNAT runtime nor by the code generated by the GNAT compiler). Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Piotr Trojanek gcc/ada/ * exp_disp.adb (Make_Secondary_DT): Internally generated OSD tables are now constant.--- gcc/ada/exp_disp.adb +++ gcc/ada/exp_disp.adb @@ -4310,6 +4310,7 @@ package body Exp_Disp is Append_To (Result, Make_Object_Declaration (Loc, Defining_Identifier => OSD, +Constant_Present=> True, Object_Definition => Make_Subtype_Indication (Loc, Subtype_Mark =>
[Ada] Simplify iteration over formal parameters for aliasing error
When iterating over pairs of formal parameters we now finish as soon as we find a single problematic pair; previously we continued iteration. This is just a simplification and a compiler performance improvement. Semantics is not affected. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Piotr Trojanek gcc/ada/ * sem_warn.adb (Warn_On_Overlapping_Actuals): Add label to the outer loop and use it in the exit statement.--- gcc/ada/sem_warn.adb +++ gcc/ada/sem_warn.adb @@ -1835,7 +1835,7 @@ package body Sem_Warn is elsif Nkind (Pref) = N_Explicit_Dereference then return True; --- If prefix is itself a component reference or slice check prefix + -- If prefix is itself a component reference or slice check prefix elsif Nkind (Pref) = N_Slice or else Nkind (Pref) = N_Indexed_Component @@ -3707,7 +3707,7 @@ package body Sem_Warn is Warn_Only := True; Form1 := First_Formal (Subp); - while Present (Form1) loop + Set_Warn_Only : while Present (Form1) loop Form2 := Next_Formal (Form1); while Present (Form2) loop if Is_Elementary_Type (Etype (Form1)) @@ -3716,14 +3716,14 @@ package body Sem_Warn is and then Ekind (Form2) /= E_In_Parameter then Warn_Only := False; - exit; + exit Set_Warn_Only; end if; Next_Formal (Form2); end loop; Next_Formal (Form1); - end loop; + end loop Set_Warn_Only; -- Exclude calls rewritten as enumeration literals
[Ada] Remove a dubious optimization for Object Specific Data dispatching
Routines Sem_Aggr.Build_Constrained_Itype and Sem_Ch3.Build_Subtype that create discriminated itypes were originally identical, but now they are subtly different. This patch removes one of their two subtle differences, namely a call to Set_Size_Known_At_Compile_Time that was meant as a very narrow optimization (and was introduced many years ago without a specific reason). This call appears to only matter for aggregates of the Ada.Tags.Object_Specific_Data type, which are part of the secondary dispatch table machinery. Now, instead of relying on this very specific optimization we recognize aggregates of this type as static. This is safe, because such aggregates are only created in the Exp_Disp.Make_DT routine and are only composed from integer literals. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Piotr Trojanek gcc/ada/ * exp_disp.adb: Minor reformatting. * exp_aggr.adb (Is_Static_Dispatch_Table_Aggregate): Recognize aggregates of the Ada.Tags.Object_Specific_Data type as static. * sem_aggr.adb (Check_Static_Discriminated_Subtype): Deconstruct and do not call it from Build_Constrained_Itype.--- gcc/ada/exp_aggr.adb +++ gcc/ada/exp_aggr.adb @@ -7790,6 +7790,9 @@ package body Exp_Aggr is or else Typ = RTE (RE_Tag_Table) or else + (RTE_Available (RE_Object_Specific_Data) + and then Typ = RTE (RE_Object_Specific_Data)) +or else (RTE_Available (RE_Interface_Data) and then Typ = RTE (RE_Interface_Data)) or else --- gcc/ada/exp_disp.adb +++ gcc/ada/exp_disp.adb @@ -4348,7 +4348,7 @@ package body Exp_Disp is Attribute_Name => Name_Alignment))); -- In secondary dispatch tables the Typeinfo component contains --- the address of the Object Specific Data (see a-tags.ads) +-- the address of the Object Specific Data (see a-tags.ads). Append_To (DT_Aggr_List, Make_Attribute_Reference (Loc, --- gcc/ada/sem_aggr.adb +++ gcc/ada/sem_aggr.adb @@ -226,12 +226,6 @@ package body Sem_Aggr is -- misspelling of one of the components of the Assoc_List. This is called -- by Resolve_Aggr_Expr after producing an invalid component error message. - procedure Check_Static_Discriminated_Subtype (T : Entity_Id; V : Node_Id); - -- An optimization: determine whether a discriminated subtype has a static - -- constraint, and contains array components whose length is also static, - -- either because they are constrained by the discriminant, or because the - -- original component bounds are static. - - -- Subprograms used for ARRAY AGGREGATE Processing -- - @@ -722,66 +716,6 @@ package body Sem_Aggr is end if; end Check_Expr_OK_In_Limited_Aggregate; - - -- Check_Static_Discriminated_Subtype -- - - - procedure Check_Static_Discriminated_Subtype (T : Entity_Id; V : Node_Id) is - Disc : constant Entity_Id := First_Discriminant (T); - Comp : Entity_Id; - Ind : Entity_Id; - - begin - if Has_Record_Rep_Clause (T) then - return; - - elsif Present (Next_Discriminant (Disc)) then - return; - - elsif Nkind (V) /= N_Integer_Literal then - return; - end if; - - Comp := First_Component (T); - while Present (Comp) loop - if Is_Scalar_Type (Etype (Comp)) then -null; - - elsif Is_Private_Type (Etype (Comp)) - and then Present (Full_View (Etype (Comp))) - and then Is_Scalar_Type (Full_View (Etype (Comp))) - then -null; - - elsif Is_Array_Type (Etype (Comp)) then -if Is_Bit_Packed_Array (Etype (Comp)) then - return; -end if; - -Ind := First_Index (Etype (Comp)); -while Present (Ind) loop - if Nkind (Ind) /= N_Range - or else Nkind (Low_Bound (Ind)) /= N_Integer_Literal - or else Nkind (High_Bound (Ind)) /= N_Integer_Literal - then - return; - end if; - - Next_Index (Ind); -end loop; - - else -return; - end if; - - Next_Component (Comp); - end loop; - - -- On exit, all components have statically known sizes - - Set_Size_Known_At_Compile_Time (T); - end Check_Static_Discriminated_Subtype; - - -- Is_Others_Aggregate -- - @@ -4509,8 +4443,6 @@ package body Sem_Aggr is Analyze (Subtyp_Decl, Suppress => All_Checks); Set_Etype (N, Def_I
[Ada] Create constrained itypes for nested record aggregates
When resolving a record aggregate with a box as the value of one of its component that is itself of a discriminated record type, this box is replaced with an inner record aggregate. However, while a constrained itype is created for the outer record aggregate (as described in the comment of Resolve_Record_Aggregate, Step 4), a similar itype was never created for the inner record aggregate. This didn't matter for GNAT, but GNATprove assumes that all record aggregates have constrained types (or itypes, where necessary). In theory, GNATprove could be deduce the needed information by itself, but this would require frontend code to be either duplicated (or preferably reused). This patch creates the missing constrained itypes in the frontend to make them available to GNATprove (and to other backends, should they need it). Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Piotr Trojanek gcc/ada/ * sem_aggr.adb (Build_Constrained_Itype): Previously a declare block, now a separate procedure; the only change is that now New_Assoc_List might include components and an others clause, which we ignore (while we deal with discriminants exactly as we did before); extend a ??? comment about how this routine is different from the Build_Subtype (Resolve_Record_Aggregate): Create a constrained itype not just for the outermost record aggregate, but for its inner record aggregates as well.--- gcc/ada/sem_aggr.adb +++ gcc/ada/sem_aggr.adb @@ -3315,6 +3315,29 @@ package body Sem_Aggr is -- part of the enclosing aggregate. Assoc_List provides the discriminant -- associations of the current type or of some enclosing record. + procedure Build_Constrained_Itype +(N : Node_Id; + Typ: Entity_Id; + New_Assoc_List : List_Id); + -- Build a constrained itype for the newly created record aggregate N + -- and set it as a type of N. The itype will have Typ as its base type + -- and will be constrained by the values of discriminants from the + -- component association list New_Assoc_List. + + -- ??? This code used to be pretty much a copy of Sem_Ch3.Build_Subtype, + -- but now those two routines behave differently for types with unknown + -- discriminants. They should really be exported in sem_util or some + -- such and used in sem_ch3 and here rather than have a copy of the + -- code which is a maintenance nightmare. + + -- ??? Performance WARNING. The current implementation creates a new + -- itype for all aggregates whose base type is discriminated. This means + -- that for record aggregates nested inside an array aggregate we will + -- create a new itype for each record aggregate if the array component + -- type has discriminants. For large aggregates this may be a problem. + -- What should be done in this case is to reuse itypes as much as + -- possible. + function Discriminant_Present (Input_Discr : Entity_Id) return Boolean; -- If aggregate N is a regular aggregate this routine will return True. -- Otherwise, if N is an extension aggregate, then Input_Discr denotes @@ -3474,6 +3497,78 @@ package body Sem_Aggr is end loop; end Add_Discriminant_Values; + - + -- Build_Constrained_Itype -- + - + + procedure Build_Constrained_Itype +(N : Node_Id; + Typ: Entity_Id; + New_Assoc_List : List_Id) + is + Constrs : constant List_Id:= New_List; + Loc : constant Source_Ptr := Sloc (N); + Def_Id : Entity_Id; + Indic : Node_Id; + New_Assoc : Node_Id; + Subtyp_Decl : Node_Id; + + begin + New_Assoc := First (New_Assoc_List); + while Present (New_Assoc) loop + +-- There is exactly one choice in the component association (and +-- it is either a discriminant, a component or the others clause). +pragma Assert (List_Length (Choices (New_Assoc)) = 1); + +-- Duplicate expression for the discriminant and put it on the +-- list of constraints for the itype declaration. + +if Is_Entity_Name (First (Choices (New_Assoc))) + and then +Ekind (Entity (First (Choices (New_Assoc = E_Discriminant +then + Append_To (Constrs, Duplicate_Subexpr (Expression (New_Assoc))); +end if; + +Next (New_Assoc); + end loop; + + if Has_Unknown_Discriminants (Typ) + and then Present (Underlying_Record_View (Typ)) + then +Indic := + Make_Subtype_Indication (Loc, +Subtype_Mark => + New_Occurrence_Of (Underlying_Record_View (Ty
[Ada] Refine type of a counter-like variable
A local variable that is used as a counter (which is clear from both its comment and its used) will only be assigned with natural numbers. This is now reflected in its type. Code cleanup only; semantics is unaffected. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Piotr Trojanek gcc/ada/ * sem_aggr.adb (Resolve_Record_Aggregate): Refine type of Others_Box.--- gcc/ada/sem_aggr.adb +++ gcc/ada/sem_aggr.adb @@ -3283,7 +3283,7 @@ package body Sem_Aggr is Box_Node : Node_Id := Empty; Is_Box_Present : Boolean := False; - Others_Box : Integer := 0; + Others_Box : Natural := 0; -- Ada 2005 (AI-287): Variables used in case of default initialization -- to provide a functionality similar to Others_Etype. Box_Present -- indicates that the component takes its default initialization;
[Ada] Avoid "others => <>" association in resolved record aggregates
When resolving record aggregates, frontend was creating "others => <>" association to represent nested components, for example: type T1 (D1 : Boolean := False) is record C1 : Boolean := True; end record; type T2 is record C2 : T1; end record; X2 : T2 := (others => <>); was expanded into: x2 : t2 := (c2 => (d1 => false, others => <>)); Now those components are represented explicitly: x2 : t2 := (c2 => (d1 => false, c1 => <>)); This makes no difference for the compiler (except that heavily nested records aggregates will be resolved into bigger AST), but GNATprove will not need to rediscover which components are covered by the "others => <>" association. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Piotr Trojanek gcc/ada/ * sem_aggr.adb (Add_Association): Add assertion about the formal parameters. (Propagate_Discriminants): Always add an explicit component association, so that an "others => <>" association is never needed.--- gcc/ada/sem_aggr.adb +++ gcc/ada/sem_aggr.adb @@ -3393,6 +3393,8 @@ package body Sem_Aggr is -- If this is a box association the expression is missing, so use the -- Sloc of the aggregate itself for the new association. + pragma Assert (Present (Expr) xor Is_Box_Present); + if Present (Expr) then Loc := Sloc (Expr); else @@ -3804,8 +3806,6 @@ package body Sem_Aggr is is Loc : constant Source_Ptr := Sloc (N); - Needs_Box : Boolean := False; - procedure Process_Component (Comp : Entity_Id); -- Add one component with a box association to the inner aggregate, -- and recurse if component is itself composite. @@ -3834,7 +3834,9 @@ package body Sem_Aggr is Build_Constrained_Itype (New_Aggr, T, Component_Associations (New_Aggr)); else - Needs_Box := True; + Add_Association + (Comp, Empty, Component_Associations (Aggr), + Is_Box_Present => True); end if; end Process_Component; @@ -3885,14 +3887,6 @@ package body Sem_Aggr is Next_Component (Comp); end loop; end if; - - if Needs_Box then -Append_To (Component_Associations (Aggr), - Make_Component_Association (Loc, -Choices => New_List (Make_Others_Choice (Loc)), -Expression => Empty, -Box_Present => True)); - end if; end Propagate_Discriminants; ---
[Ada] Move duplicated routines for building itypes to Sem_Util
Routine Build_Constrained_Itype was created as an exact duplicate Build_Subtype with a comment suggesting that their code should be exported from Sem_Util and reused. Unfortunately, since then both routines diverged and now are subtly different, so reusing is not straightforward. However, it is still better to have them both exported to prevent further duplication. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Piotr Trojanek gcc/ada/ * sem_aggr.adb (Build_Constrained_Itype): Move to Sem_Util. * sem_ch3.adb (Build_Subtype, Inherit_Predicate_Flags): Move... * sem_util.adb (Build_Subtype): Here. Add parameters for references to objects previously declared in enclosing scopes. (Inherit_Predicate_Flags): And here, because it is called by Build_Subtype. * sem_util.ads (Build_Overriding_Spec): Reorder alphabetically. (Build_Subtype): Moved from Sem_Ch3; comments updated. (Build_Constrained_Itype): Moved from Sem_Aggr; comments updated.--- gcc/ada/sem_aggr.adb +++ gcc/ada/sem_aggr.adb @@ -3313,29 +3313,6 @@ package body Sem_Aggr is -- part of the enclosing aggregate. Assoc_List provides the discriminant -- associations of the current type or of some enclosing record. - procedure Build_Constrained_Itype -(N : Node_Id; - Typ: Entity_Id; - New_Assoc_List : List_Id); - -- Build a constrained itype for the newly created record aggregate N - -- and set it as a type of N. The itype will have Typ as its base type - -- and will be constrained by the values of discriminants from the - -- component association list New_Assoc_List. - - -- ??? This code used to be pretty much a copy of Sem_Ch3.Build_Subtype, - -- but now those two routines behave differently for types with unknown - -- discriminants. They should really be exported in sem_util or some - -- such and used in sem_ch3 and here rather than have a copy of the - -- code which is a maintenance nightmare. - - -- ??? Performance WARNING. The current implementation creates a new - -- itype for all aggregates whose base type is discriminated. This means - -- that for record aggregates nested inside an array aggregate we will - -- create a new itype for each record aggregate if the array component - -- type has discriminants. For large aggregates this may be a problem. - -- What should be done in this case is to reuse itypes as much as - -- possible. - function Discriminant_Present (Input_Discr : Entity_Id) return Boolean; -- If aggregate N is a regular aggregate this routine will return True. -- Otherwise, if N is an extension aggregate, then Input_Discr denotes @@ -3495,78 +3472,6 @@ package body Sem_Aggr is end loop; end Add_Discriminant_Values; - - - -- Build_Constrained_Itype -- - - - - procedure Build_Constrained_Itype -(N : Node_Id; - Typ: Entity_Id; - New_Assoc_List : List_Id) - is - Constrs : constant List_Id:= New_List; - Loc : constant Source_Ptr := Sloc (N); - Def_Id : Entity_Id; - Indic : Node_Id; - New_Assoc : Node_Id; - Subtyp_Decl : Node_Id; - - begin - New_Assoc := First (New_Assoc_List); - while Present (New_Assoc) loop - --- There is exactly one choice in the component association (and --- it is either a discriminant, a component or the others clause). -pragma Assert (List_Length (Choices (New_Assoc)) = 1); - --- Duplicate expression for the discriminant and put it on the --- list of constraints for the itype declaration. - -if Is_Entity_Name (First (Choices (New_Assoc))) - and then -Ekind (Entity (First (Choices (New_Assoc = E_Discriminant -then - Append_To (Constrs, Duplicate_Subexpr (Expression (New_Assoc))); -end if; - -Next (New_Assoc); - end loop; - - if Has_Unknown_Discriminants (Typ) - and then Present (Underlying_Record_View (Typ)) - then -Indic := - Make_Subtype_Indication (Loc, -Subtype_Mark => - New_Occurrence_Of (Underlying_Record_View (Typ), Loc), -Constraint => - Make_Index_Or_Discriminant_Constraint (Loc, -Constraints => Constrs)); - else -Indic := - Make_Subtype_Indication (Loc, -Subtype_Mark => - New_Occurrence_Of (Base_Type (Typ), Loc), -Constraint => - Make_Index_Or_Discriminant_Constraint (Loc, -
[Ada] Iterate with procedural version of Next routine where possible
Routine Next is implemented both as a procedure and as a function. The procedure variant seems meant to be used when iterating, e.g.: Next (Decl); because it is more readable than the corresponding functions: Decl := Next (Decl); (and it is inlined anyway, so there is no performance penalty). This patch makes use of the procedure variants where possible; uses of function were detected with: $ grep " \([[:alpha:]_]\+\) := Next (\1)" *.adb Note: procedural version is kept in sem_prag.adb, where it is used to iterate over pragma arguments and it feels more readable there. Semantics is unaffected. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Piotr Trojanek gcc/ada/ * checks.adb, exp_ch7.adb, exp_ch9.adb, exp_smem.adb, lib.adb, nlists.adb, sem.adb, sem_aggr.adb, sem_ch3.adb, sem_ch6.adb, sem_ch8.adb, sem_dim.adb, sem_res.adb, sem_util.adb, sem_warn.adb: Replace uses of Next function with procedure.--- gcc/ada/checks.adb +++ gcc/ada/checks.adb @@ -3531,7 +3531,7 @@ package body Checks is -- Move to next subscript - Sub := Next (Sub); + Next (Sub); end loop; end Apply_Subscript_Validity_Checks; --- gcc/ada/exp_ch7.adb +++ gcc/ada/exp_ch7.adb @@ -2852,7 +2852,7 @@ package body Exp_Ch7 is exit; end if; - Result := Next (Result); + Next (Result); end loop; return Result; @@ -2895,7 +2895,7 @@ package body Exp_Ch7 is if No_Initialization (Decl) then if No (Expression (Last_Init)) then loop - Last_Init := Next (Last_Init); + Next (Last_Init); exit when No (Last_Init); exit when Nkind (Last_Init) = N_Object_Declaration and then Nkind (Expression (Last_Init)) = N_Reference --- gcc/ada/exp_ch9.adb +++ gcc/ada/exp_ch9.adb @@ -2130,7 +2130,7 @@ package body Exp_Ch9 is if Present (First_Formal (Iface_Op)) and then Is_Controlling_Formal (First_Formal (Iface_Op)) then - Iface_Op_Param := Next (Iface_Op_Param); + Next (Iface_Op_Param); end if; Wrapper_Param := First (Wrapper_Params); @@ -2215,7 +2215,7 @@ package body Exp_Ch9 is if Is_Private_Primitive_Subprogram (Subp_Id) and then not Has_Controlling_Result (Subp_Id) then -Formal := Next (Formal); +Next (Formal); end if; while Present (Formal) loop --- gcc/ada/exp_smem.adb +++ gcc/ada/exp_smem.adb @@ -454,7 +454,7 @@ package body Exp_Smem is begin while Next (Nod) /= After loop -Nod := Next (Nod); +Next (Nod); end loop; return Nod; --- gcc/ada/lib.adb +++ gcc/ada/lib.adb @@ -1335,7 +1335,7 @@ package body Lib is and then (Nkind (Context_Item) /= N_With_Clause or else Limited_Present (Context_Item)) loop -Context_Item := Next (Context_Item); +Next (Context_Item); end loop; if Present (Context_Item) then @@ -1359,7 +1359,7 @@ package body Lib is Write_Eol; end if; - Context_Item := Next (Context_Item); + Next (Context_Item); end loop; Outdent; --- gcc/ada/nlists.adb +++ gcc/ada/nlists.adb @@ -243,7 +243,7 @@ package body Nlists is N := F; loop Set_List_Link (N, To); - N := Next (N); + Next (N); exit when No (N); end loop; @@ -530,7 +530,7 @@ package body Nlists is loop Set_List_Link (N, LC); exit when N = L; - N := Next (N); + Next (N); end loop; if Present (Before) then @@ -597,7 +597,7 @@ package body Nlists is loop Set_List_Link (N, LC); exit when N = L; - N := Next (N); + Next (N); end loop; if Present (After) then @@ -699,7 +699,7 @@ package body Nlists is Node := First (List); while Present (Node) loop Result := Result + 1; - Node := Next (Node); + Next (Node); end loop; return Result; @@ -756,7 +756,7 @@ package body Nlists is while Present (E) loop Append (New_Copy (E), NL); -E := Next (E); +Next (E); end loop; return NL; @@ -784,7 +784,7 @@ package body Nlists is Append (New_Copy (E), NL); end if; -E := Next (E); +Next (E); end loop; return NL; @@ -990,7 +990,7 @@ package body
[Ada] Consolidate handling of implicit dereferences into semantic analysis
This consolidates the handling of almost all the implicit dereferences allowed in Ada 95 into the semantic analysis phase of the compiler, and more precisely in the Resolve routine of the front-end. This both means that the generic code handling them in the expander is removed, and that various constructs generated by it are directly built with explicit dereferences when needed. No functional changes, except for swapped error messages in case of problematic dereferences in requeue statements. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Eric Botcazou gcc/ada/ * checks.adb (Build_Discriminant_Checks): Build an explicit dereference when the type is an access type. * exp_atag.adb (Build_CW_Membership): Add explicit dereferences. (Build_Get_Access_Level): Likewise. (Build_Get_Alignment): Likewise. (Build_Inherit_Prims): Likewise. (Build_Get_Transportable): Likewise. (Build_Set_Size_Function): Likewise. * exp_ch3.adb (Build_Offset_To_Top_Function): Likewise. * exp_ch4.adb (Expand_Allocator_Expression): Likewise. (Expand_N_Indexed_Component ): Remove code dealing with implicit dereferences. (Expand_N_Selected_Component): Likewise. (Expand_N_Slice): Likewise. * exp_ch9.adb (Add_Formal_Renamings): Add explicit dereference. (Expand_Accept_Declarations): Likewise. (Build_Simple_Entry_Call): Remove code dealing with implicit dereferences. (Expand_N_Requeue_Statement): Likewise. * exp_disp.adb (Expand_Dispatching_Call): Build an explicit dereference when the controlling type is an access type. * exp_spark.adb (Expand_SPARK_N_Selected_Component): Delete. (Expand_SPARK_N_Slice_Or_Indexed_Component): Likewise. (Expand_SPARK): Do not call them. * sem_ch4.adb (Process_Implicit_Dereference_Prefix): Delete. (Process_Indexed_Component): Call Implicitly_Designated_Type to get the designated type for an implicit dereference. (Analyze_Overloaded_Selected_Component): Do not insert an explicit dereference here. (Analyze_Selected_Component): Likewise. (Analyze_Slice): Call Implicitly_Designated_Type to get the designated type for an implicit dereference. * sem_ch8.adb (Has_Components): New predicate extracted from... (Is_Appropriate_For_Record): ...this. Delete. (Is_Appropriate_For_Entry_Prefix): Likewise. (Analyze_Renamed_Entry): Deal with implicit dereferences. (Find_Selected_Component): Do not insert an explicit dereference here. Call Implicitly_Designated_Type to get the designated type for an implicit dereference. Call Has_Components, Is_Task_Type and Is_Protected_Type directly. Adjust test for error. * sem_res.adb (Resolve_Implicit_Dereference): New procedure. (Resolve_Call): Call Resolve_Indexed_Component last. (Resolve_Entry): Call Resolve_Implicit_Dereference on the prefix. (Resolve_Indexed_Component): Call Implicitly_Designated_Type to get the designated type for an implicit dereference and Resolve_Implicit_Dereference on the prefix at the end. (Resolve_Selected_Component): Likewise. (Resolve_Slice): Likewise. Do not apply access checks here. * sem_util.ads (Implicitly_Designated_Type): Declare. * sem_util.adb (Copy_And_Maybe_Dereference): Simplify. (Implicitly_Designated_Type): New function. (Object_Access_Level): Fix typo. * sem_warn.adb (Check_Unset_Reference): Test Comes_From_Source on the original node. patch.diff.gz Description: application/gzip
[Ada] Skip unnecessary iterations over constraint expressions
When looking for references to discriminants within constraint expressions we now stop once the first such a reference is found. This is just a tiny performance improvement; semantics is unaffected. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Piotr Trojanek gcc/ada/ * sem_ch3.adb (Build_Constrained_Array_Type, Build_Constrained_Discriminated_Type): Skip unnecessary loop iterations.--- gcc/ada/sem_ch3.adb +++ gcc/ada/sem_ch3.adb @@ -13093,7 +13093,7 @@ package body Sem_Ch3 is Scop : Entity_Id; begin - -- if the original access type was not embedded in the enclosing + -- If the original access type was not embedded in the enclosing -- type definition, there is no need to produce a new access -- subtype. In fact every access type with an explicit constraint -- generates an itype whose scope is the enclosing record. @@ -13192,6 +13192,7 @@ package body Sem_Ch3 is Is_Discriminant (Hi_Expr) then Need_To_Create_Itype := True; + exit; end if; Next_Index (Old_Index); @@ -13248,6 +13249,7 @@ package body Sem_Ch3 is if Is_Discriminant (Expr) then Need_To_Create_Itype := True; + exit; -- After expansion of discriminated task types, the value -- of the discriminant may be converted to a run-time type @@ -13259,6 +13261,7 @@ package body Sem_Ch3 is and then Is_Discriminant (Expression (Expr)) then Need_To_Create_Itype := True; + exit; end if; Next_Elmt (Old_Constraint);
[Ada] Additional warnings on overlapping actuals of composite types
This patch refines the handling of warnings on overlapping actuals of composite types when only one of them is writable. Formals of a generic type are excluded, given that the warning will be given on any instance. Uniform treatment of formals and actuals. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Ed Schonberg gcc/ada/ * sem_warn.adb (Warn_On_Overlapping_Actuals): Simplify code, remove inner predicate Is_Covered_Formal, preserve warning for two overlapping composite types when only one is writable, and for two overlapping and writable elementary types.--- gcc/ada/sem_warn.adb +++ gcc/ada/sem_warn.adb @@ -3643,9 +3643,6 @@ package body Sem_Warn is - procedure Warn_On_Overlapping_Actuals (Subp : Entity_Id; N : Node_Id) is - function Is_Covered_Formal (Formal : Node_Id) return Boolean; - -- Return True if Formal is covered by the rule - function Refer_Same_Object (Act1 : Node_Id; Act2 : Node_Id) return Boolean; @@ -3658,19 +3655,6 @@ package body Sem_Warn is -- (RM 6.4.1(6.11/3)) --- - -- Is_Covered_Formal -- - --- - - function Is_Covered_Formal (Formal : Node_Id) return Boolean is - begin - return - Ekind_In (Formal, E_Out_Parameter, E_In_Out_Parameter) - and then (Is_Elementary_Type (Etype (Formal)) -or else Is_Record_Type (Etype (Formal)) -or else Is_Array_Type (Etype (Formal))); - end Is_Covered_Formal; - - --- -- Refer_Same_Object -- --- @@ -3759,137 +3743,182 @@ package body Sem_Warn is Form1 := First_Formal (Subp); Act1 := First_Actual (N); while Present (Form1) and then Present (Act1) loop - if Is_Covered_Formal (Form1) -or else not Is_Elementary_Type (Etype (Act1)) + if Is_Generic_Type (Etype (Act1)) then +return; + end if; + + -- One of the formals must be either (in)-out or composite. + -- The other must be (in)-out. + + if Is_Elementary_Type (Etype (Act1)) + and then Ekind (Form1) = E_In_Parameter then +null; + + else Form2 := First_Formal (Subp); Act2 := First_Actual (N); while Present (Form2) and then Present (Act2) loop if Form1 /= Form2 - and then Is_Covered_Formal (Form2) and then Refer_Same_Object (Act1, Act2) then - -- Guard against previous errors + if Is_Generic_Type (Etype (Act2)) then + return; + end if; - if Error_Posted (N) -or else No (Etype (Act1)) -or else No (Etype (Act2)) - then - null; + -- First case : two writable elementary parameters + -- that overlap. - -- If the actual is a function call in prefix notation, - -- there is no real overlap. + if (Is_Elementary_Type (Etype (Form1)) +and then Is_Elementary_Type (Etype (Form2)) +and then Ekind (Form1) /= E_In_Parameter +and then Ekind (Form2) /= E_In_Parameter) - elsif Nkind (Act2) = N_Function_Call then - null; + -- Second case : two composite parameters that overlap, + -- one of which is writable. - -- If type is not by-copy, assume that aliasing is intended +or else (Is_Composite_Type (Etype (Form1)) + and then Is_Composite_Type (Etype (Form2)) + and then (Ekind (Form1) /= E_In_Parameter + or else Ekind (Form2) /= E_In_Parameter)) - elsif -Present (Underlying_Type (Etype (Form1))) - and then -(Is_By_Reference_Type (Underlying_Type (Etype (Form1))) - or else -Convention (Underlying_Type (Etype (Form1))) = - Convention_Ada_Pass_By_Reference) - then - null; + -- Third case : an elementary writable parameter that + -- overlaps a composite one. - -- Under Ada 2012 we only report warnings on overlapping - -- arrays and record types if switch is set. +or else (Is_Elementary_Type (Etype (Form1)) + and then Ekind (Form1) /= E_In_Parameter + and then Is_Composite_Type (Etype (Form2))) - elsif Ada
[Ada] Put_Image attribute
Work around the fact that Put_Image doesn't work for private types whose full type is real. Make Put_Image_Unknown print out the name of the type. Put_Image is still disabled by default for all types. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Bob Duff gcc/ada/ * exp_put_image.adb (Build_Elementary_Put_Image_Call): If the underlying type is real, call Put_Image_Unknown. (Build_Unknown_Put_Image_Call): Pass the type name to Put_Image_Unknown. * libgnat/s-putima.ads, libgnat/s-putima.adb (Put_Image_Unknown): Add Type_Name parameter. Remove overly-detailed documentation of what it does; better to leave it open.--- gcc/ada/exp_put_image.adb +++ gcc/ada/exp_put_image.adb @@ -27,6 +27,7 @@ with Atree;use Atree; with Debug;use Debug; with Einfo;use Einfo; with Exp_Tss; use Exp_Tss; +with Exp_Util; with Lib; use Lib; with Namet;use Namet; with Nlists; use Nlists; @@ -340,26 +341,34 @@ package body Exp_Put_Image is -- -- Note that this is putting a leading space for reals. + -- ???Work around the fact that Put_Image doesn't work for private + -- types whose full type is real. + + if Is_Real_Type (U_Type) then +return Build_Unknown_Put_Image_Call (N); + end if; + declare Image : constant Node_Id := Make_Attribute_Reference (Loc, Prefix => New_Occurrence_Of (U_Type, Loc), Attribute_Name => Name_Wide_Wide_Image, Expressions => New_List (Relocate_Node (Item))); - begin -return +Put_Call : constant Node_Id := Make_Procedure_Call_Statement (Loc, Name => New_Occurrence_Of (RTE (RE_Put_Wide_Wide_String), Loc), Parameter_Associations => New_List (Relocate_Node (Sink), Image)); + begin +return Put_Call; end; end if; -- Unchecked-convert parameter to the required type (i.e. the type of -- the corresponding parameter), and call the appropriate routine. -- We could use a normal type conversion for scalars, but the - -- "unchecked" is needed for access types. + -- "unchecked" is needed for access and private types. declare Libent : constant Entity_Id := RTE (Lib_RE); @@ -800,7 +809,10 @@ package body Exp_Put_Image is Make_Procedure_Call_Statement (Loc, Name => New_Occurrence_Of (Libent, Loc), Parameter_Associations => New_List ( -Relocate_Node (Sink))); +Relocate_Node (Sink), +Make_String_Literal (Loc, + Exp_Util.Fully_Qualified_Name_String ( +Entity (Prefix (N)), Append_NUL => False; end Build_Unknown_Put_Image_Call; -- --- gcc/ada/libgnat/s-putima.adb +++ gcc/ada/libgnat/s-putima.adb @@ -212,9 +212,11 @@ package body System.Put_Images is Put_7bit (S, ')'); end Record_After; - procedure Put_Image_Unknown (S : in out Sink'Class) is + procedure Put_Image_Unknown (S : in out Sink'Class; Type_Name : String) is begin - Put_UTF_8 (S, "{unknown image}"); + Put_UTF_8 (S, "{"); + Put_String (S, Type_Name); + Put_UTF_8 (S, " object}"); end Put_Image_Unknown; end System.Put_Images; --- gcc/ada/libgnat/s-putima.ads +++ gcc/ada/libgnat/s-putima.ads @@ -86,8 +86,8 @@ package System.Put_Images is procedure Record_Between (S : in out Sink'Class); procedure Record_After (S : in out Sink'Class); - procedure Put_Image_Unknown (S : in out Sink'Class); + procedure Put_Image_Unknown (S : in out Sink'Class; Type_Name : String); -- For Put_Image of types that don't have the attribute, such as type - -- Sink. Prints a canned string. + -- Sink. end System.Put_Images;
[Ada] Refine type of a routine parameter from Node_Id to Entity_Id
Routine Get_Value is only called with its Compon parameter equal to entity ids of components. This is now reflected in the type of this parameter. Code cleanup only; semantics is unaffected. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Piotr Trojanek gcc/ada/ * sem_aggr.adb (Get_Value): Refine type of the Compon parameter.--- gcc/ada/sem_aggr.adb +++ gcc/ada/sem_aggr.adb @@ -3336,7 +3336,7 @@ package body Sem_Aggr is -- of the ancestor. function Get_Value -(Compon : Node_Id; +(Compon : Entity_Id; From : List_Id; Consider_Others_Choice : Boolean := False) return Node_Id; -- Given a record component stored in parameter Compon, this function @@ -3614,7 +3614,7 @@ package body Sem_Aggr is --- function Get_Value -(Compon : Node_Id; +(Compon : Entity_Id; From : List_Id; Consider_Others_Choice : Boolean := False) return Node_Id is
[Ada] Allow specifying volatility refinement aspects for types
Previously, the four aspects Async_Readers, Async_Writers, Effective_Reads, and Effective_Writes could only be specified for volatile variables and for state abstractions. Allow specifying these aspects for volatile types. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Steve Baird gcc/ada/ * contracts.adb (Add_Contract_Item): Support specifying volatility refinement aspects for types. (Analyze_Contracts): Add call to Analyze_Type_Contract in the case of a contract for a type. (Freeze_Contracts): Add call to Analyze_Type_Contract in the case of a contract for a type. (Check_Type_Or_Object_External_Properties): A new procedure which performs the work that needs to be done for both object declarations and types. (Analyze_Object_Contract): Add a call to Check_Type_Or_Object_External_Properties and remove the code in this procedure which did much of the work that is now performed by that call. (Analyze_Type_Contract): Implement this new routine as nothing more than a call to Check_Type_Or_Object_External_Properties. * contracts.ads: Update comment for Add_Contract_To_Item because types can have contracts. Follow (questionable) precedent and declare new routine Analyze_Type_Contract as visible (following example of Analyze_Object_Contract), despite the fact that it is never called from outside of the package where it is declared. * einfo.adb (Contract, Set_Contract): Id argument can be a type; support this case. (Write_Field34_Name): Field name is "contract" for a type. * einfo.ads: Update comment describing Contract attribute. * sem_ch3.adb (Build_Derived_Numeric_Type): Is_Volatile should return same answer for all subtypes of a given type. Thus, when building the base type for something like type Volatile_1_To_10 is range 1 .. 10 with Volatile; that basetype should be marked as being volatile. (Access_Type_Declaration): Add SPARK-specific legality check that the designated type of an access type shall be compatible with respect to volatility with the access type. * sem_ch12.adb (Check_Shared_Variable_Control_Aspects): Add SPARK-specific legality check that an actual type parameter in an instantiation shall be compatible with respect to volatility with the corresponding formal type. * sem_ch13.adb (Analyze_Aspect_Specifications): Perform checks for aspect specs for the 4 volatility refinement aspects that were already being performed for all language-defined aspects. * sem_prag.adb (Analyze_External_Property_In_Decl_Part, Analyze_Pragma): External properties (other than No_Caching) may be specified for a type, including a generic formal type. * sem_util.ads: Declare new subprograms - Async_Readers_Enabled, Async_Writers_Enabled, Effective_Reads, Effective_Writes, and Check_Volatility_Compatibility. * sem_util.adb (Async_Readers_Enabled, Async_Writers_Enabled, Effective_Reads, Effective_Writes): Initial implementation of new functions for querying aspect values. (Check_Volatility_Compatibility): New procedure intended for use in checking all SPARK legality rules of the form "<> shall be compatible with respect to volatility with <>". (Has_Enabled_Property): Update comment because Item_Id can be a type. Change name of nested Variable_Has_Enabled_Property function to Type_Or_Variable_Has_Enabled_Property; add a parameter to that function because recursion may be needed, e.g., in the case of a derived typ). Cope with the case where the argument to Has_Enabled_Property is a type. patch.diff.gz Description: application/gzip
[Ada] Update SPARK RM rule numbers after removing a redundant rule
SPARK RM 7.1.3(8) has been deleted, but there were actually plenty of mistakes in references to rules 7.1.3(X). This patch fixes them in both comments and error messages. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Piotr Trojanek gcc/ada/ * sem_ch4.adb, sem_ch6.adb, sem_res.adb, sem_util.ads: Fix references to SPARK RM 7.1.3 rule numbers.--- gcc/ada/sem_ch4.adb +++ gcc/ada/sem_ch4.adb @@ -665,7 +665,7 @@ package body Sem_Ch4 is -- that outside of spec expressions, otherwise the declaration -- cannot be inserted and analyzed. In such a case, GNATprove -- later rejects the allocator as it is not used here in - -- a non-interfering context (SPARK 4.8(2) and 7.1.3(12)). + -- a non-interfering context (SPARK 4.8(2) and 7.1.3(10)). if Expander_Active or else (GNATprove_Mode and then not In_Spec_Expression) --- gcc/ada/sem_ch6.adb +++ gcc/ada/sem_ch6.adb @@ -11889,7 +11889,7 @@ package body Sem_Ch6 is -- A procedure cannot have an effectively volatile formal -- parameter of mode IN because it behaves as a constant --- (SPARK RM 7.1.3(6)). -- ??? maybe 7.1.3(4) +-- (SPARK RM 7.1.3(4)). elsif Ekind (Scope (Formal)) = E_Procedure and then Ekind (Formal) = E_In_Parameter --- gcc/ada/sem_res.adb +++ gcc/ada/sem_res.adb @@ -3328,7 +3328,7 @@ package body Sem_Res is procedure Flag_Effectively_Volatile_Objects (Expr : Node_Id); -- Emit an error concerning the illegal usage of an effectively volatile - -- object in interfering context (SPARK RM 7.1.3(12)). + -- object in interfering context (SPARK RM 7.1.3(10)). procedure Insert_Default; -- If the actual is missing in a call, insert in the actuals list @@ -3613,7 +3613,7 @@ package body Sem_Res is then Error_Msg_N ("volatile object cannot appear in this context (SPARK " - & "RM 7.1.3(11))", N); + & "RM 7.1.3(10))", N); return Skip; end if; end if; @@ -4739,7 +4739,7 @@ package body Sem_Res is -- An effectively volatile object may act as an actual when the -- corresponding formal is of a non-scalar effectively volatile - -- type (SPARK RM 7.1.3(11)). + -- type (SPARK RM 7.1.3(10)). if not Is_Scalar_Type (Etype (F)) and then Is_Effectively_Volatile (Etype (F)) @@ -4748,7 +4748,7 @@ package body Sem_Res is -- An effectively volatile object may act as an actual in a -- call to an instance of Unchecked_Conversion. - -- (SPARK RM 7.1.3(11)). + -- (SPARK RM 7.1.3(10)). elsif Is_Unchecked_Conversion_Instance (Nam) then null; @@ -4758,7 +4758,7 @@ package body Sem_Res is elsif Is_Effectively_Volatile_Object (A) then Error_Msg_N ("volatile object cannot act as actual in a call (SPARK " - & "RM 7.1.3(11))", A); + & "RM 7.1.3(10))", A); -- Otherwise the actual denotes an expression. Inspect the -- expression and flag each effectively volatile object with @@ -7544,7 +7544,7 @@ package body Sem_Res is -- An effectively volatile object subject to enabled properties -- Async_Writers or Effective_Reads must appear in non-interfering --- context (SPARK RM 7.1.3(12)). +-- context (SPARK RM 7.1.3(10)). if Is_Object (E) and then Is_Effectively_Volatile (E) @@ -7554,7 +7554,7 @@ package body Sem_Res is then SPARK_Msg_N ("volatile object cannot appear in this context " - & "(SPARK RM 7.1.3(12))", N); + & "(SPARK RM 7.1.3(10))", N); end if; -- Check for possible elaboration issues with respect to reads of --- gcc/ada/sem_util.ads +++ gcc/ada/sem_util.ads @@ -1919,7 +1919,7 @@ package Sem_Util is (Context : Node_Id; Obj_Ref : Node_Id) return Boolean; -- Determine whether node Context denotes a "non-interfering context" (as - -- defined in SPARK RM 7.1.3(12)) where volatile reference Obj_Ref can + -- defined in SPARK RM 7.1.3(10)) where volatile reference Obj_Ref can -- safely reside. function Is_Package_Contract_Annotation (Item : Node_Id) return Boolean;
[Ada] Fix wrong access to large bit-packed arrays with reverse SSO
Large bit-packed arrays, i.e. whose size is greater than 64 bits, are implemented under the hood by means of arrays of storage units, the front-end generating the required mask-and-shifts operations to go back and forth between the two representations. These operations depend on the endianness of the bit-packed arrays, which may be different from that of the target if the attribute Scalar_Storage_Order is specified for the bit-packed arrays. But, even though the component type of arrays of storage units is endian neutral, the Scalar_Storage_Order attribute of the bit-packed arrays must be propagated onto them, because it is required if the bit-packed arrays are components of an outer composite type that do not start or do not end on a storage unit boundary. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Eric Botcazou gcc/ada/ * exp_pakd.ads: Add paragraph about scalar storage order. * exp_pakd.adb (Install_PAT): Do not set the scalar storage order of the PAT here but... (Set_PB_Type): ...here instead and... (Create_Packed_Array_Impl_Type): ...here as well. * rtsfind.ads (RE_Id): Add RE_Rev_Packed_Bytes{1,2,4}. (RE_Unit_Table): Likewise. * libgnat/s-unstyp.ads (Rev_Packed_Bytes1): New derived type. (Rev_Packed_Bytes2): Likewise. (Rev_Packed_Bytes4): Likewise.--- gcc/ada/exp_pakd.adb +++ gcc/ada/exp_pakd.adb @@ -501,8 +501,9 @@ package body Exp_Pakd is -- packed array type. It creates the type and installs it as required. procedure Set_PB_Type; - -- Sets PB_Type to Packed_Bytes{1,2,4} as required by the alignment - -- requirements (see documentation in the spec of this package). + -- Set PB_Type to [Rev_]Packed_Bytes{1,2,4} as required by the alignment + -- and the scalar storage order requirements (see documentation in the + -- spec of this package). - -- Install_PAT -- @@ -580,14 +581,6 @@ package body Exp_Pakd is Set_Is_Volatile_Full_Access (PAT, Is_Volatile_Full_Access (Typ)); Set_Treat_As_Volatile (PAT, Treat_As_Volatile(Typ)); - -- For a non-bit-packed array, propagate reverse storage order - -- flag from original base type to packed array base type. - - if not Is_Bit_Packed_Array (Typ) then -Set_Reverse_Storage_Order - (Etype (PAT), Reverse_Storage_Order (Base_Type (Typ))); - end if; - -- We definitely do not want to delay freezing for packed array -- types. This is of particular importance for the itypes that are -- generated for record components depending on discriminants where @@ -616,16 +609,36 @@ package body Exp_Pakd is or else Alignment (Typ) = 1 or else Component_Alignment (Typ) = Calign_Storage_Unit then -PB_Type := RTE (RE_Packed_Bytes1); +if Reverse_Storage_Order (Typ) then + PB_Type := RTE (RE_Rev_Packed_Bytes1); +else + PB_Type := RTE (RE_Packed_Bytes1); +end if; elsif Csize mod 4 /= 0 or else Alignment (Typ) = 2 then -PB_Type := RTE (RE_Packed_Bytes2); +if Reverse_Storage_Order (Typ) then + PB_Type := RTE (RE_Rev_Packed_Bytes2); +else + PB_Type := RTE (RE_Packed_Bytes2); +end if; else -PB_Type := RTE (RE_Packed_Bytes4); +if Reverse_Storage_Order (Typ) then + PB_Type := RTE (RE_Rev_Packed_Bytes4); +else + PB_Type := RTE (RE_Packed_Bytes4); +end if; end if; + + -- The Rev_Packed_Bytes{1,2,4} types cannot be directly declared with + -- the reverse scalar storage order in System.Unsigned_Types because + -- their component type is aliased and the combination would then be + -- flagged as illegal by the compiler. Moreover changing the compiler + -- would not address the bootstrap path issue with earlier versions. + + Set_Reverse_Storage_Order (PB_Type, Reverse_Storage_Order (Typ)); end Set_PB_Type; -- Start of processing for Create_Packed_Array_Impl_Type @@ -797,6 +810,10 @@ package body Exp_Pakd is end; Install_PAT; + + -- Propagate the reverse storage order flag to the base type + + Set_Reverse_Storage_Order (Etype (PAT), Reverse_Storage_Order (Typ)); return; -- Case of bit-packing required for unconstrained array. We create --- gcc/ada/exp_pakd.ads +++ gcc/ada/exp_pakd.ads @@ -86,6 +86,15 @@ package Exp_Pakd is --Packed_Bytes{1,2,4} type is made on the basis of alignment needs as --described above for the unconstrained case. + -- When the packed array (sub)type is specified to have the reverse scalar + -- storage order, the
[Ada] Fix assertion failure on entry call through unchecked conversion
The Safe_Unchecked_Type_Conversion predicate was invoking the Has_Discriminant predicate without checking that the Etype really Is_Type, which is not the case for Standard_Void_Type "returned" by entry calls. No functional changes. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Eric Botcazou gcc/ada/ * exp_util.adb (Safe_Unchecked_Type_Conversion): Add missing Is_Type guard before calling Has_Discriminants on Etype.--- gcc/ada/exp_util.adb +++ gcc/ada/exp_util.adb @@ -12551,13 +12551,10 @@ package body Exp_Util is elsif Nkind (Pexp) = N_Selected_Component and then Prefix (Pexp) = Exp then - if No (Etype (Pexp)) then -return True; - else -return - not Has_Discriminants (Etype (Pexp)) -or else Is_Constrained (Etype (Pexp)); - end if; + return No (Etype (Pexp)) + or else not Is_Type (Etype (Pexp)) + or else not Has_Discriminants (Etype (Pexp)) + or else Is_Constrained (Etype (Pexp)); end if; -- Set the output type, this comes from Etype if it is set, otherwise we
[Ada] Fix missing insertion of explicit dereference in instance
This adjusts the new Resolve_Implicit_Dereference procedure to the cases where nodes in an instance do not have the proper view of a type that was declared as private (a well-known limitation of the current implementation of generic instantiations for types that are only implicitly referenced in the generic code). No functional changes. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Eric Botcazou gcc/ada/ * sem_res.adb (Resolve_Implicit_Dereference): In an instance, reset the type of the prefix if it is private before building the dereference.--- gcc/ada/sem_res.adb +++ gcc/ada/sem_res.adb @@ -8740,6 +8740,17 @@ package body Sem_Res is Desig_Typ : Entity_Id; begin + -- In an instance the proper view may not always be correct for + -- private types, see e.g. Sem_Type.Covers for similar handling. + + if Is_Private_Type (Etype (P)) +and then Present (Full_View (Etype (P))) +and then Is_Access_Type (Full_View (Etype (P))) +and then In_Instance + then + Set_Etype (P, Full_View (Etype (P))); + end if; + if Is_Access_Type (Etype (P)) then Desig_Typ := Implicitly_Designated_Type (Etype (P)); Insert_Explicit_Dereference (P);
[Ada] Put_Image attribute
Work around bug in Put_Image of types in Remote_Types packages. Use the switch -gnatd_z to control enabling of Put_Image. Put_Image is still disabled by default for all types. Tested on x86_64-pc-linux-gnu, committed on trunk 2020-06-11 Bob Duff gcc/ada/ * exp_put_image.adb (Build_Record_Put_Image_Procedure): Remove special processing of protected types, because those are handled by Build_Protected_Put_Image_Call. (Enable_Put_Image): Use the switch -gnatd_z to control enabling of Put_Image. Disable Put_Image for types in Remote_Types packages. * debug.adb: Document -gnatd_z switch. * exp_imgv.adb, libgnat/a-stteou.ads, opt.ads: Minor cleanups.--- gcc/ada/debug.adb +++ gcc/ada/debug.adb @@ -170,7 +170,7 @@ package body Debug is -- d_w -- d_x -- d_y - -- d_z + -- d_z Enable Put_Image -- d_A Stop generation of ALI file -- d_B @@ -993,6 +993,9 @@ package body Debug is -- a call to routine Ada.Synchronous_Task_Control.Suspend_Until_True -- or Ada.Synchronous_Barriers.Wait_For_Release. + -- d_z The Put_Image attribute is a work in progress, and is disabled by + -- default. This enables it. + -- d_A Do not generate ALI files by setting Opt.Disable_ALI_File. -- d_F The compiler encodes the full path from an invocation construct to --- gcc/ada/exp_imgv.adb +++ gcc/ada/exp_imgv.adb @@ -747,7 +747,7 @@ package body Exp_Imgv is --btyp (Value_xx (X)) - -- where btyp is he base type of the prefix + -- where btyp is the base type of the prefix --For types whose root type is Character -- xx = Character --- gcc/ada/exp_put_image.adb +++ gcc/ada/exp_put_image.adb @@ -24,6 +24,7 @@ -- with Atree;use Atree; +with Debug;use Debug; with Einfo;use Einfo; with Exp_Tss; use Exp_Tss; with Lib; use Lib; @@ -323,9 +324,14 @@ package body Exp_Put_Image is -- -- Put_Wide_Wide_String (Sink, U_Type'Wide_Wide_Image (Item)); -- - -- This is a bit of a cheat; we should probably do it the other way - -- around (define '[[Wide_]Wide_]Image in terms of 'Put_Image). But - -- this is expedient for now. We can't do this: + -- It would be more elegant to do it the other way around (define + -- '[[Wide_]Wide_]Image in terms of 'Put_Image). But this is easier + -- to implement, because we already have support for + -- 'Wide_Wide_Image. Furthermore, we don't want to remove the + -- existing support for '[[Wide_]Wide_]Image, because we don't + -- currently plan to support 'Put_Image on restricted runtimes. + + -- We can't do this: -- -- Put_UTF_8 (Sink, U_Type'Image (Item)); -- @@ -689,22 +695,12 @@ package body Exp_Put_Image is Stms : constant List_Id := New_List; Rdef : Node_Id; - Typt : Entity_Id; - Type_Decl : Node_Id; + Type_Decl : constant Node_Id := +Declaration_Node (Base_Type (Underlying_Type (Typ))); -- Start of processing for Build_Record_Put_Image_Procedure begin - -- For the protected type case, use corresponding record - - if Is_Protected_Type (Typ) then - Typt := Corresponding_Record_Type (Typ); - else - Typt := Typ; - end if; - - Type_Decl := Declaration_Node (Base_Type (Underlying_Type (Typt))); - Append_To (Stms, Make_Procedure_Call_Statement (Loc, Name => New_Occurrence_Of (RTE (RE_Record_Before), Loc), @@ -813,7 +809,7 @@ package body Exp_Put_Image is function Enable_Put_Image (T : Entity_Id) return Boolean is begin - if True then -- True to disable for all types. + if not Debug_Flag_Underscore_Z then -- True to disable for all types return False; end if; @@ -832,6 +828,15 @@ package body Exp_Put_Image is -- scalar types are expanded inline. We certainly want to be able to use -- Integer'Put_Image, for example. + -- ???Work around a bug: Put_Image does not work for Remote_Types. + -- We check the containing package, rather than the type itself, because + -- we want to include types in the private part of a Remote_Types + -- package. + + if Is_Remote_Types (Scope (T)) then + return False; + end if; + -- ???Disable Put_Image on type Sink declared in -- Ada.Strings.Text_Output. Note that we can't call Is_RTU on -- Ada_Strings_Text_Output, because it's not known yet (we might be --- gcc/ada/libgnat/a-stteou.ads +++ gcc/ada/libgnat/a-stteou.ads @@ -133,7 +133,7 @@ package Ada.Strings.Text_Output is (UTF_Encoding.Wide_Wide_Strings.Decode (UTF_8_Lines)) = UTF_8_Lines; subtype UTF_8 is UTF_8_Lines with - Predicate => (for all
Re: [PATCH 4/4 V2] vect: Factor out and rename some functions/macros
"Kewen.Lin" writes: > @@ -9195,12 +9222,13 @@ optimize_mask_stores (class loop *loop) > } > > /* Decide whether it is possible to use a zero-based induction variable > - when vectorizing LOOP_VINFO with a fully-masked loop. If it is, > - return the value that the induction variable must be able to hold > - in order to ensure that the loop ends with an all-false mask. > - Return -1 otherwise. */ > + when vectorizing LOOP_VINFO with partial vectors. If it is, return > + the value that the induction variable must be able to hold in order > + to ensure that the loop ends with an all-false rgroup control like > + mask. Return -1 otherwise. */ Maybe: “…in order to ensure that the rgroups eventually have no active vector elements”. > static void > -check_load_store_masking (loop_vec_info loop_vinfo, tree vectype, > - vec_load_store_type vls_type, int group_size, > - vect_memory_access_type memory_access_type, > - gather_scatter_info *gs_info, tree scalar_mask) > +check_load_store_for_partial_vectors ( > + loop_vec_info loop_vinfo, tree vectype, vec_load_store_type vls_type, > + int group_size, vect_memory_access_type memory_access_type, > + gather_scatter_info *gs_info, tree scalar_mask) Think it's more usual in GCC to put the "(" on the same line as the arguments in this situation. OK with those changes, thanks. Richard
[PATCH][RFC] __builtin_shuffle sometimes should produce zip1 rather than TBL (PR82199)
The following patch enables vector permutations optimization by using another vector element size when applicable. It allows usage of simpler instructions in applicable cases. example: #define vector __attribute__((vector_size(16) )) vector float f(vector float a, vector float b) { return __builtin_shuffle (a, b, (vector int){0, 1, 4,5}); } was compiled into: ... adrpx0, .LC0 ldr q2, [x0, #:lo12:.LC0] tbl v0.16b, {v0.16b - v1.16b}, v2.16b ... and after patch: ... zip1v0.2d, v0.2d, v1.2d ... bootstrapped and tested on aarch64-linux-gnu with no regressions This patch was initially introduced by Andrew Pinksi with me being involved later. (I have no write access to repo) Thanks, Dmitrij gcc/ChangeLog: 2020-06-11 Andrew Pinski PR gcc/82199 * gcc/config/aarch64/aarch64.c (aarch64_evpc_reencode): New function gcc/testsuite/ChangeLog: 2020-06-11 Andrew Pinski PR gcc/82199 * gcc.target/aarch64/vdup_n_3.c: New test * gcc.target/aarch64/vzip_1.c: New test * gcc.target/aarch64/vzip_2.c: New test * gcc.target/aarch64/vzip_3.c: New test * gcc.target/aarch64/vzip_4.c: New test Co-Authored-By: Dmitrij Pochepko Thanks, Dmitrij >From 3c9f3fe834811386223755fc58e2ab4a612eefcf Mon Sep 17 00:00:00 2001 From: Dmitrij Pochepko Date: Thu, 11 Jun 2020 14:13:35 +0300 Subject: [PATCH] __builtin_shuffle sometimes should produce zip1 rather than TBL (PR82199) The following patch enables vector permutations optimization by using another vector element size when applicable. It allows usage of simpler instructions in applicable cases. example: vector float f(vector float a, vector float b) { return __builtin_shuffle (a, b, (vector int){0, 1, 4,5}); } was compiled into: ... adrp x0, .LC0 ldr q2, [x0, #:lo12:.LC0] tbl v0.16b, {v0.16b - v1.16b}, v2.16b ... and after patch: ... zip1 v0.2d, v0.2d, v1.2d ... bootstrapped and tested on aarch64-linux-gnu with no regressions gcc/ChangeLog: 2020-06-11 Andrew Pinski PR gcc/82199 * gcc/config/aarch64/aarch64.c (aarch64_evpc_reencode): New function gcc/testsuite/ChangeLog: 2020-06-11 Andrew Pinski PR gcc/82199 * gcc.target/aarch64/vdup_n_3.c: New test * gcc.target/aarch64/vzip_1.c: New test * gcc.target/aarch64/vzip_2.c: New test * gcc.target/aarch64/vzip_3.c: New test * gcc.target/aarch64/vzip_4.c: New test Co-Authored-By: Dmitrij Pochepko --- gcc/config/aarch64/aarch64.c| 81 + gcc/testsuite/gcc.target/aarch64/vdup_n_3.c | 16 ++ gcc/testsuite/gcc.target/aarch64/vzip_1.c | 11 gcc/testsuite/gcc.target/aarch64/vzip_2.c | 12 + gcc/testsuite/gcc.target/aarch64/vzip_3.c | 12 + gcc/testsuite/gcc.target/aarch64/vzip_4.c | 12 + 6 files changed, 144 insertions(+) create mode 100644 gcc/testsuite/gcc.target/aarch64/vdup_n_3.c create mode 100644 gcc/testsuite/gcc.target/aarch64/vzip_1.c create mode 100644 gcc/testsuite/gcc.target/aarch64/vzip_2.c create mode 100644 gcc/testsuite/gcc.target/aarch64/vzip_3.c create mode 100644 gcc/testsuite/gcc.target/aarch64/vzip_4.c diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 973c65a..ab7b39e 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -19889,6 +19889,8 @@ struct expand_vec_perm_d bool testing_p; }; +static bool aarch64_expand_vec_perm_const_1 (struct expand_vec_perm_d *d); + /* Generate a variable permutation. */ static void @@ -20074,6 +20076,83 @@ aarch64_evpc_trn (struct expand_vec_perm_d *d) return true; } +/* Try to re-encode the PERM constant so it use the bigger size up. + This rewrites constants such as {0, 1, 4, 5}/V4SF to {0, 2}/V2DI. + We retry with this new constant with the full suite of patterns. */ +static bool +aarch64_evpc_reencode (struct expand_vec_perm_d *d) +{ + expand_vec_perm_d newd; + unsigned HOST_WIDE_INT nelt; + + if (d->vec_flags != VEC_ADVSIMD) +return false; + + unsigned int encoded_nelts = d->perm.encoding ().encoded_nelts (); + for (unsigned int i = 0; i < encoded_nelts; ++i) +if (!d->perm[i].is_constant ()) + return false; + + /* to_constant is safe since this routine is specific to Advanced SIMD + vectors. */ + nelt = d->perm.length ().to_constant (); + + /* Get the new mode. Always twice the size of the inner + and half the elements. */ + machine_mode new_mode; + switch (d->vmode) +{ +/* 128bit vectors. */ +case E_V4SFmode: +case E_V4SImode: + new_mode = V2DImode; + break; +case E_V8BFmode: +case E_V8HFmode: +case E_V8HImode: + new_mode = V4SImode; + break; +case E_V16QImode: + new_mode = V8HImode; + break; +/* 64bit vectors. */ +case E_V4BFmode: +case E_V4HFmode: +case E_V4HImode: + new_mode = V2SImode; + break; +case E_V8QImode: + ne
[patch] Fix ICE in verify_sra_access_forest
Hi, this fixes an issue with reverse storage order in SRA, which is caught by the built-in verifier: ===GNAT BUG DETECTED==+ | 11.0.0 20200610 (experimental) (x86_64-suse-linux) GCC error:| | in verify_sra_access_forest, at tree-sra.c:2359 gcc_assert (reverse == access->reverse); The problem is that propagate_subaccesses_from_rhs changes the type of an access from aggregate to scalar and, as discussed elsewhere, this must be done with extra care in the presence of reverse storage order. Tested on x86-64/Linux, OK for the mainline? 2020-06-11 Eric Botcazou * tree-sra.c (propagate_subaccesses_from_rhs): When a non-scalar access on the LHS is replaced with a scalar access, propagate the TYPE_REVERSE_STORAGE_ORDER flag of the type of the original access. 2020-06-11 Eric Botcazou * gnat.dg/opt85.ad[sb]: New test. -- Eric Botcazoudiff --git a/gcc/tree-sra.c b/gcc/tree-sra.c index 4793b48f32c..fcba7fbdd31 100644 --- a/gcc/tree-sra.c +++ b/gcc/tree-sra.c @@ -2758,6 +2758,9 @@ propagate_subaccesses_from_rhs (struct access *lacc, struct access *racc) } if (!lacc->first_child && !racc->first_child) { + /* We are about to change the access type from aggregate to scalar, + so we need to put the reverse flag onto the access, if any. */ + const bool reverse = TYPE_REVERSE_STORAGE_ORDER (lacc->type); tree t = lacc->base; lacc->type = racc->type; @@ -2772,9 +2775,12 @@ propagate_subaccesses_from_rhs (struct access *lacc, struct access *racc) lacc->expr = build_ref_for_model (EXPR_LOCATION (lacc->base), lacc->base, lacc->offset, racc, NULL, false); + if (TREE_CODE (lacc->expr) == MEM_REF) + REF_REVERSE_STORAGE_ORDER (lacc->expr) = reverse; lacc->grp_no_warning = true; lacc->grp_same_access_path = false; } + lacc->reverse = reverse; } return ret; } with Ada.Finalization; with Interfaces; with System; package Opt85 is type Data_Type is record Value : Interfaces.Integer_16; end record; for Data_Type use record Value at 0 range 0 .. 15; end record; for Data_Type'Alignment use 1; for Data_Type'Size use 2 * System.Storage_Unit; for Data_Type'Bit_Order use System.High_Order_First; for Data_Type'Scalar_Storage_Order use System.High_Order_First; type Header_Type is array (1 .. 1) of Boolean; type Record_Type is new Ada.Finalization.Controlled with record Header : Header_Type; Data : Data_Type; end record; function Create (Value : Integer) return Record_Type; end Opt85; -- { dg-do compile } -- { dg-options "-O" } package body Opt85 is function Conversion_Of (Value : Integer) return Data_Type is begin return (Value => Interfaces.Integer_16 (Value)); end; function Create (Value : Integer) return Record_Type is Rec : constant Record_Type := (Ada.Finalization.Controlled with Header => (others => False), Data => Conversion_Of (Value)); begin return Rec; end; end Opt85;
RE: [PATCH PR95570] vect: ICE: Segmentation fault in vect_loop_versioning
Hi, > -Original Message- > From: Richard Sandiford [mailto:richard.sandif...@arm.com] > Sent: Thursday, June 11, 2020 12:23 AM > To: Yangfei (Felix) > Cc: gcc-patches@gcc.gnu.org > Subject: Re: [PATCH PR95570] vect: ICE: Segmentation fault in > vect_loop_versioning > > "Yangfei (Felix)" writes: > > Hi, > > > > PR: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95570 > > > > Here, we are doing loop versioning for alignment. The only dr here is a > gather-statter operation: x[start][i]. > > Scalar evolution analysis for this dr failed, so DR_STEP is NULL_TREE, which > leads to the segfault. > > But scatter-gather operation should be filtered out in > vect_enhance_data_refs_alignment. > > There are similar issues in vect_verify_datarefs_alignment, > vect_get_peeling_costs_all_drs and vect_peeling_supportable. > > Proposed patch adds back the necessary tests. Bootstrapped and tested > on aarch64-linux-gnu & x86_64-linux-gnu. > > > > Test coverage: > > Existing tests [1] and newly added test ensures coverage for all the changes > except for the changes in vect_peeling_supportable. > > Currently I don't have a test to cover the changes in > vect_peeling_supportable. Should we keep them? > > Rather than add several instances of the new test, I think it would make > sense to split the (hopefully) correct conditions in > vect_enhance_data_refs_alignment out into a subroutine and use it in the > other sites. Doing that for vect_peeling_supportable would then be > justifiable as a clean-up. OK. > How about something like vect_relevant_for_alignment_p as the name of > the subroutine? Nice name. Does the v2 patch look better? Bootstrapped and tested on aarch64-linux-gnu. Newly added test fail without the fix and pass otherwise. gcc/ +2020-06-11 Felix Yang + + PR tree-optimization/95570 + * tree-vect-data-refs.c (vect_relevant_for_alignment_p): New function. + (vect_verify_datarefs_alignment): Call it to filter out data references + in the loop whose alignment is irrelevant. + (vect_get_peeling_costs_all_drs): Likewise. + (vect_peeling_supportable): Likewise. + (vect_enhance_data_refs_alignment): Likewise. gcc/testsuite/ +2020-06-11 Felix Yang + + PR tree-optimization/95570 + * gcc.dg/vect/pr95570.c: New test. Thanks, Felix pr95570-v2.diff Description: pr95570-v2.diff
[PATCH] git_update_version: add --current argument.
The argument can be useful to update arbitrary branch, the changes are added to git index and user is supposed to make a commit. I'm going to install it if there are no objections. Martin contrib/ChangeLog: * gcc-changelog/git_update_version.py: Add --curent argument. --- contrib/gcc-changelog/git_update_version.py | 106 +++- 1 file changed, 59 insertions(+), 47 deletions(-) diff --git a/contrib/gcc-changelog/git_update_version.py b/contrib/gcc-changelog/git_update_version.py index 6b6ccf68a5e..733a1a0f14a 100755 --- a/contrib/gcc-changelog/git_update_version.py +++ b/contrib/gcc-changelog/git_update_version.py @@ -66,60 +66,72 @@ parser.add_argument('-d', '--dry-mode', help='Generate patch for ChangeLog entries and do it' ' even if DATESTAMP is unchanged; folder argument' ' is expected') +parser.add_argument('-c', '--current', action='store_true', +help='Modify current branch (--push argument is ignored)') args = parser.parse_args() repo = Repo(args.git_path) origin = repo.remotes['origin'] -for ref in origin.refs: -assert ref.name.startswith('origin/') -name = ref.name[len('origin/'):] -if name in active_refs: -if name in repo.branches: -branch = repo.branches[name] + +def update_current_branch(): +commit = repo.head.commit +commit_count = 1 +while commit: +if (commit.author.email == 'gccad...@gcc.gnu.org' +and commit.message.strip() == 'Daily bump.'): +break +commit = commit.parents[0] +commit_count += 1 + +print('%d revisions since last Daily bump' % commit_count) +datestamp_path = os.path.join(args.git_path, 'gcc/DATESTAMP') +if (read_timestamp(datestamp_path) != current_timestamp +or args.dry_mode or args.current): +commits = parse_git_revisions(args.git_path, '%s..HEAD' + % commit.hexsha) +for git_commit in reversed(commits): +prepend_to_changelog_files(repo, args.git_path, git_commit, + not args.dry_mode) +if args.dry_mode: +diff = repo.git.diff('HEAD') +patch = os.path.join(args.dry_mode, + branch.name.split('/')[-1] + '.patch') +with open(patch, 'w+') as f: +f.write(diff) +print('branch diff written to %s' % patch) +repo.git.checkout(force=True) else: -branch = repo.create_head(name, ref).set_tracking_branch(ref) -print('=== Working on: %s ===' % branch, flush=True) -origin.pull(rebase=True) -branch.checkout() -print('branch pulled and checked out') -assert not repo.index.diff(None) -commit = branch.commit -commit_count = 1 -while commit: -if (commit.author.email == 'gccad...@gcc.gnu.org' -and commit.message.strip() == 'Daily bump.'): -break -commit = commit.parents[0] -commit_count += 1 - -print('%d revisions since last Daily bump' % commit_count) -datestamp_path = os.path.join(args.git_path, 'gcc/DATESTAMP') -if (read_timestamp(datestamp_path) != current_timestamp -or args.dry_mode): -commits = parse_git_revisions(args.git_path, '%s..HEAD' - % commit.hexsha) -for git_commit in reversed(commits): -prepend_to_changelog_files(repo, args.git_path, git_commit, - not args.dry_mode) -if args.dry_mode: -diff = repo.git.diff('HEAD') -patch = os.path.join(args.dry_mode, - branch.name.split('/')[-1] + '.patch') -with open(patch, 'w+') as f: -f.write(diff) -print('branch diff written to %s' % patch) -repo.git.checkout(force=True) -else: -# update timestamp -print('DATESTAMP will be changed:') -with open(datestamp_path, 'w+') as f: -f.write(current_timestamp) -repo.git.add(datestamp_path) +# update timestamp +print('DATESTAMP will be changed:') +with open(datestamp_path, 'w+') as f: +f.write(current_timestamp) +repo.git.add(datestamp_path) +if not args.current: repo.index.commit('Daily bump.') if args.push: repo.git.push('origin', branch) print('branch is pushed') -else: -print('DATESTAMP unchanged') -print('branch is done\n', flush=True) +else: +print('DATESTAMP unchanged') + + +if args.current: +
[PATCH] rs6000: skip debug info statements
Hi. Since stmt_vec_info is not generated for debug info statements, these needs to be skipped in rs6000_density_test. Patch can bootstrap on ppc64le-linux-gnu and survives regression tests. Ready to be installed? Thanks, Martin gcc/ChangeLog: PR target/95627 * config/rs6000/rs6000.c (rs6000_density_test): Skip debug statements. --- gcc/config/rs6000/rs6000.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 0bbd06ad1de..00daf979856 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -4987,6 +4987,9 @@ rs6000_density_test (rs6000_cost_data *data) for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) { gimple *stmt = gsi_stmt (gsi); + if (is_gimple_debug (stmt)) + continue; + stmt_vec_info stmt_info = loop_vinfo->lookup_stmt (stmt); if (!STMT_VINFO_RELEVANT_P (stmt_info) -- 2.26.2
(patch] Optimize assignment to volatile aggregate variable
Hi, gimplify_modify_expr_rhs has an optimization whereby the assignment to an aggregate variable from a read-only object with a DECL_INITIAL is optimized into the direct assignment of the DECL_INITIAL, provided that no temporary is created in the process. The optimization is blocked if the read-only object is volatile, which is OK as per the semantics of volatile, but also if the target variable is volatile, on the grounds that the modified assignment might end up being done on a per field basis, which is also OK. But this latter restriction is enforced a priori and there are cases where the modified assignment would be OK, for example if there is only one field or the DECL_INITIAL is equivalent to the empty CONSTRUCTOR, i.e. all zeros. So, in the latter case, the patch changes gimplify_modify_expr_rhs to ask gimplify_init_constructor whether the assignment would be done as a block, which is easy because gimplify_init_constructor knows that it must create a temporary if the LHS is volatile and this would not be the case, so it's just a matter of completing the NOTIFY_TEMP_CREATION mechanism for this case. Tested on x86-64/Linux, OK for the mainline? 2020-06-11 Eric Botcazou * gimplify.c (gimplify_init_constructor) : Declare new ENSURE_SINGLE_ACCESS constant and move variables down. If it is true and all elements are zero, then always clear. Return GS_ERROR if a temporary would be created for it and NOTIFY_TEMP_CREATION is set. (gimplify_modify_expr_rhs) : If the target is volatile but the type is aggregate non-addressable, ask gimplify_init_constructor whether it can generate a single access to the target. 2020-06-11 Eric Botcazou * gnat.dg/aggr30.adb[sb]: New test. -- Eric Botcazouwith Interfaces; package Aggr30 is type Data_Type is array (1 .. 4) of Interfaces.Unsigned_8; type Padding_Type is array (5 .. 4096) of Interfaces.Unsigned_8; type Rec is record Data: Data_Type; Padding : Padding_Type; end record; procedure Init; procedure Init_Volatile; private Instance : Rec; Instance_Volatile : Rec; pragma Volatile (Instance_Volatile); end Aggr30; -- { dg-do compile } -- { dg-options "-fdump-tree-gimple" } package body Aggr30 is Null_Constant : constant Rec := (Data => (others => 0), Padding => (others => 0)); procedure Init is begin Instance := Null_Constant; end; procedure Init_Volatile is begin Instance_Volatile := Null_Constant; end; end Aggr30; -- { dg-final { scan-tree-dump-times "= {}" 2 "gimple"} } diff --git a/gcc/gimplify.c b/gcc/gimplify.c index e14932fafaf..ff2be7b4fc4 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -4865,10 +4865,6 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, case QUAL_UNION_TYPE: case ARRAY_TYPE: { - struct gimplify_init_ctor_preeval_data preeval_data; - HOST_WIDE_INT num_ctor_elements, num_nonzero_elements; - HOST_WIDE_INT num_unique_nonzero_elements; - bool cleared, complete_p, valid_const_initializer; /* Use readonly data for initializers of this or smaller size regardless of the num_nonzero_elements / num_unique_nonzero_elements ratio. */ @@ -4876,6 +4872,17 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, /* If num_nonzero_elements / num_unique_nonzero_elements ratio is smaller than this, use readonly data. */ const int unique_nonzero_ratio = 8; + /* True if a single access of the object must be ensured. This is the + case if the target is volatile, the type is non-addressable and more + than one field need to be assigned. */ + const bool ensure_single_access + = TREE_THIS_VOLATILE (object) + && !TREE_ADDRESSABLE (type) + && vec_safe_length (elts) > 1; + struct gimplify_init_ctor_preeval_data preeval_data; + HOST_WIDE_INT num_ctor_elements, num_nonzero_elements; + HOST_WIDE_INT num_unique_nonzero_elements; + bool cleared, complete_p, valid_const_initializer; /* Aggregate types must lower constructors to initialization of individual elements. The exception is that a CONSTRUCTOR node @@ -4914,6 +4921,7 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, { if (notify_temp_creation) return GS_ERROR; + DECL_INITIAL (object) = ctor; TREE_STATIC (object) = 1; if (!DECL_NAME (object)) @@ -4961,6 +4969,10 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, /* If there are "lots" of zeros, it's more efficient to clear the memory and then set the nonzero elements. */ cleared = true; + else if (ensure_single_access && num_nonzero_elements == 0) + /* If a single access to the target must be ensured and all elements + are zero, then it's optimal to clear whatever their number. */ + cleared = true; else
Re: [PATCH] git_update_version: add --current argument.
On Thu, Jun 11, 2020 at 01:47:30PM +0200, Martin Liška wrote: > The argument can be useful to update arbitrary branch, the changes > are added to git index and user is supposed to make a commit. > > I'm going to install it if there are no objections. > > Martin > > contrib/ChangeLog: > > * gcc-changelog/git_update_version.py: Add --curent argument. LGTM, thanks. Jakub
Re: [Patch, fortran] PR fortran/94022 - Array slices of assumed-size arrays
Hi Jose, Proposed patch to Bug 94022 - Array slices of assumed-size arrays. Patch tested only on x86_64-pc-linux-gnu. Reviewed, regression-tested and commited as r11-1228-g6a07010b774cb5a0b1790b857e69d3d8534eebd2 . Thanks for the patch! Regards Thomas
[PATCH PR94274] fold phi whose incoming args are defined from binary
Hi, This is a experimental fix for pr94274. For if/else structure, when the expressions is binary operation and have a common subexpression, and the opcode is the same, we can fold the merging phi node in tree_ssa_phiopt_worker (ssa-phiopt). It can be optimized to do csel first and then do binary operations. This can eliminate one or more instructions. And this patch is effective for 500.perlbench_r in spec2017. Bootstrap and tested on aarch64/x86_64 Linux platform. No new regression witnessed. Any suggestion? Thanks, Haijian Zhang pr94274-v1.diff Description: pr94274-v1.diff
Re: [PR95416] outputs.exp: skip lto tests when not using linker plugin
Hi Alexandre, > On Jun 9, 2020, Rainer Orth wrote: > >> this is wrong unfortunately: braces are the Tcl equivalent of single >> quotes so you're setting skip_lto to the string inside. > > Aah, thanks. So when $skip_lto is expanded in the ifs, the whole thing > is evaluated, and that's why it works anyway? but it didn't work: when I tried the patch on i386-pc-solaris2.11 with as/ld (i.e. no linker plugin}, the lto tests of outputs.exp were still run, and check_linker_plugin_available wasn't even run at all, what quite confused me at first. As I'd mentioned in the PR, you can easily test an equivalent configuration with gld when configuring gcc with --disable-lto-plugin. >> While this worked for me on i386-pc-solaris2.11 (both with ld, thus >> without the lto plugin, and with gld and the plugin), I wonder if >> there's not a better way than just skipping the lto tests, > > My initial focus was on relieving the pain by removing the symptoms, so > I can then work on the improvements with lower urgency. Fully understood: as I'd mentioned, that's mine as well. We already accumulate all too many testsuite failures on master... > Here's what I'd like to check in for now: > > [PR95416] outputs.exp: skip lto tests when not using linker plugin > > From: Alexandre Oliva > > When the linker plugin is not available, dump outputs in lto runs are > different from those outputs.exp tests expect, so skip them for now. > > Regstrapped on x86_64-linux-gnu. Ok to install? > > > for gcc/testsuite/ChangeLog > > * outputs.exp (skip_lto): Set when missing the linker plugin. Ok, thanks. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [Patch, fortran] PR fortran/52351, 85868 Wrong array section bounds when passing to an intent-in pointer dummy
Hi Jose, Proposed patch to PRs 52351, 85868 Wrong array section bounds when passing to an intent-in pointer dummy. First, thanks for working on this and for this patch. Regarding the patch, there are a few style issues which I fixed for the commit. If you could try to adhere to a few more of them, I'd be obliged :-) Regarding the ChangeLog: That format is rather rigid, see https://gcc.gnu.org/codingconventions.html#ChangeLogs . Also, a script run on the server checks all the ChangeLogs for correct formats. I usually get this right on my second or third attempt :-) Regarding the patch itself: There were a few whitespace issues that could be corrected easily (git diff shows them up bright red). Reviewed, regression-tested and committed as r11-1230-g2ff0f48819c8a7ed5d7c03e2bfc02e5907e2ff1a . Thanks a lot for fixing this! Regards Thomas Here is the ChangeLog of what I committed: Wrong array section bounds when passing to an intent-in pointer dummy. Add code to allow for the creation a new descriptor for array sections with the correct one based indexing. Rework the generated descriptors indexing (hopefully) fixing the wrong offsets generated. gcc/fortran/ChangeLog: 2020-06-11 José Rui Faustino de Sousa PR fortran/52351 PR fortran/85868 * trans-array.c (gfc_conv_expr_descriptor): Enable the creation of a new descriptor with the correct one based indexing for array sections. Rework array descriptor indexing offset calculation. gcc/testsuite/ChangeLog: 2020-06-11 José Rui Faustino de Sousa PR fortran/52351 PR fortran/85868 * gfortran.dg/coarray_lib_comm_1.f90: Adjust match test for the newly generated descriptor. * gfortran.dg/PR85868A.f90: New test. * gfortran.dg/PR85868B.f90: New test.
Re: [PATCH] rs6000: skip debug info statements
On June 11, 2020 1:49:57 PM GMT+02:00, "Martin Liška" wrote: >Hi. > >Since stmt_vec_info is not generated for debug info statements, these >needs to >be skipped in rs6000_density_test. > >Patch can bootstrap on ppc64le-linux-gnu and survives regression tests. > >Ready to be installed? OK. Richard. >Thanks, >Martin > >gcc/ChangeLog: > > PR target/95627 > * config/rs6000/rs6000.c (rs6000_density_test): Skip debug > statements. >--- > gcc/config/rs6000/rs6000.c | 3 +++ > 1 file changed, 3 insertions(+) > >diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c >index 0bbd06ad1de..00daf979856 100644 >--- a/gcc/config/rs6000/rs6000.c >+++ b/gcc/config/rs6000/rs6000.c >@@ -4987,6 +4987,9 @@ rs6000_density_test (rs6000_cost_data *data) > for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) > { > gimple *stmt = gsi_stmt (gsi); >+if (is_gimple_debug (stmt)) >+continue; >+ > stmt_vec_info stmt_info = loop_vinfo->lookup_stmt (stmt); > > if (!STMT_VINFO_RELEVANT_P (stmt_info)
Re: [PATCH] rs6000: skip debug info statements
On Thu, Jun 11, 2020 at 01:49:57PM +0200, Martin Liška wrote: > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c > index 0bbd06ad1de..00daf979856 100644 > --- a/gcc/config/rs6000/rs6000.c > +++ b/gcc/config/rs6000/rs6000.c > @@ -4987,6 +4987,9 @@ rs6000_density_test (rs6000_cost_data *data) >for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) > { > gimple *stmt = gsi_stmt (gsi); > + if (is_gimple_debug (stmt)) > + continue; Formatting, continue; indented too much. Jakub
Re: [Patch, fortran] PR fortran/95331 - Unlimited polymorphic arrays have wrong bounds
Hi Jose, Proposed patch to PR95331 - Unlimited polymorphic arrays have wrong bounds. Patch tested only on x86_64-pc-linux-gnu. reviewed, ChangeLog reformatted, committed as r11-1235-g2ee70f5d161edd99a7af97d166b251bcf83cd91b . Thanks a lot for the patch! Do you have interest in getting write access? Regards Thomas PR95331 - Unlimited polymorphic arrays have wrong bounds. When iterating over a class array use the bounds provided by the transformed descriptor (in sym->backend_decl) instead of the original bounds of the array (in the descriptor passed in the class _data) which are passed in se->expr. The patch partially depends on the patch for PR52351 and PR85868, but does not seems to break anything by itself. gcc/fortran/ChangeLog: 2020-06-11 José Rui Faustino de Sousa PR fortran/95331 * trans-array.c (gfc_conv_array_ref): For class array dummy arguments use the transformed descriptor in sym->backend_decl instead of the original descriptor. gcc/testsuite/ChangeLog: 2020-06-11 José Rui Faustino de Sousa PR fortran/95331 * gfortran.dg/PR95331.f90: New test.
Re: [PATCH, PR fortran/95503] [9/10/11 Regression] ICE in gfc_is_simply_contiguous, at fortran/expr.c:5844
Hi Harald, The following patch fixes an almost obvious ICE in invalid. Regtested on x86_64-pc-linux-gnu. OK for master, and backports to 9/10? OK. Thanks for the patch! Regards Thomas
Re: [patch] Fix ICE in verify_sra_access_forest
Hi, On Thu, Jun 11 2020, Eric Botcazou wrote: > Hi, > > this fixes an issue with reverse storage order in SRA, which is caught by the > built-in verifier: > > ===GNAT BUG DETECTED==+ > | 11.0.0 20200610 (experimental) (x86_64-suse-linux) GCC error:| > | in verify_sra_access_forest, at tree-sra.c:2359 > > gcc_assert (reverse == access->reverse); > > The problem is that propagate_subaccesses_from_rhs changes the type of an > access from aggregate to scalar and, as discussed elsewhere, this must be > done > with extra care in the presence of reverse storage order. > > Tested on x86-64/Linux, OK for the mainline? > > > 2020-06-11 Eric Botcazou > > * tree-sra.c (propagate_subaccesses_from_rhs): When a non-scalar > access on the LHS is replaced with a scalar access, propagate the > TYPE_REVERSE_STORAGE_ORDER flag of the type of the original access. > > > 2020-06-11 Eric Botcazou > > * gnat.dg/opt85.ad[sb]: New test. > > -- > Eric Botcazou > diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c > index 4793b48f32c..fcba7fbdd31 100644 > --- a/gcc/tree-sra.c > +++ b/gcc/tree-sra.c > @@ -2758,6 +2758,9 @@ propagate_subaccesses_from_rhs (struct access *lacc, > struct access *racc) > } >if (!lacc->first_child && !racc->first_child) > { > + /* We are about to change the access type from aggregate to scalar, > + so we need to put the reverse flag onto the access, if any. */ > + const bool reverse = TYPE_REVERSE_STORAGE_ORDER (lacc->type); I am not very good at reasoning about reverse storage order stuff but can it ever happen that this reverse is not the same as racc->reverse? In order for that to happen, we'd have to have an assignment (folded memcpy?) between two aggregates in the original code that, at the same offset within the aggregates, have single field structures with different storage orders. If that has undefined behavior (even for folded memcpy), I guess I am fine with the patch (but I cannot approve it). If not, I'd add a condition that racc->reverse is the same as TYPE_REVERSE_STORAGE_ORDER(lacc->type) to the if guarding all of this. I tried to come up with a (C) testcase for this but just could not trigger even this condition reasonably quickly. Thanks for looking into this, Martin
Re: BRIG FE testsuite: Fix all dump-scans (Was: Re: drop -aux{dir, base}, revamp -dump{dir, base})
Hi Mike, On Tue, Jun 09 2020, Mike Stump wrote: > I think I'd prefer the fix on the other side, if reasonable. I'd give > them some time to see about a fix there before selecting this patch. given Alexandre's email, are you OK with the patch? It essentially manually keeps the input name "rootname" in sync with the output file "rootname" which is the new basis for dump names. As Alexandre noted, the proper fix is probably to teach the testsuite to expect dump names based on outputs by default but I do not want to leave the majority of BRIG testcases broken until that happens. Thanks, Martin > > On Jun 9, 2020, at 5:42 AM, Martin Jambor wrote: [...] >> >> >> I looked into the issue yesterday and decided the simplest fix is >> probably the following. I am going to use my BRIG maintainer hat to >> commit the patch in a day or two unless someone thinks it is a bad idea. >> Tested by running make check-brig on an x86_64-linux. >> >> Martin >> >> >> >> Since Alexandre's revamp of dump files handling in >> r11-627-g1dedc12d186, BRIG FE has been receiving slightly different >> -dumpbase (e.g. smoke_test.brig instead of smoke_test.hsail.brig when >> compiling file smoke_test.hsail.brig) and the testsuite then could not >> find the generated dump files it wanted to scan. I have not really >> looked into why that changed, the easiest fix seems to me to remove >> the hsail part already when generating the binary brig file from the >> textual HSAIL representation. >> >> gcc/testsuite/ChangeLog: >> >> 2020-06-09 Martin Jambor >> >> * lib/brig.exp (brig_target_compile): Strip hsail extension when >> gnerating the name of the binary brig file. >> --- >> gcc/testsuite/lib/brig.exp | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/gcc/testsuite/lib/brig.exp b/gcc/testsuite/lib/brig.exp >> index fbfb1da947a..de47f13e42c 100644 >> --- a/gcc/testsuite/lib/brig.exp >> +++ b/gcc/testsuite/lib/brig.exp >> @@ -29,7 +29,7 @@ proc brig_target_compile { source dest type options } { >> # We cannot assume all inputs are .hsail as the dg machinery >> # calls this for a some c files to check linker plugin support or >> # similar. >> -set brig_source ${tmpdir}/[file tail ${source}].brig >> +set brig_source ${tmpdir}/[file rootname [file tail ${source}]].brig >> exec HSAILasm $source -o ${brig_source} >> set source ${brig_source} >> # Change the testname the .brig. >> -- >> 2.26.2 >>
[PATCH] PR fortran/95544 - ICE in gfc_can_put_var_on_stack, at fortran/trans-decl.c:494
> Gesendet: Montag, 08. Juni 2020 um 22:25 Uhr > Von: "Harald Anlauf" > An: "fortran" , "gcc-patches" > Betreff: [PATCH] PR fortran/95544 - ICE in gfc_can_put_var_on_stack, at > fortran/trans-decl.c:494 OK, now with a brown bag over my head, here comes the patch instead of just the testcase. Thanks to Thomas for pointing that out in private. Harald diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c index 0afb96c0414..148a3269815 100644 --- a/gcc/fortran/check.c +++ b/gcc/fortran/check.c @@ -1431,8 +1431,8 @@ gfc_check_x_yd (gfc_expr *x, gfc_expr *y) return true; } -static bool -invalid_null_arg (gfc_expr *x) +bool +gfc_invalid_null_arg (gfc_expr *x) { if (x->expr_type == EXPR_NULL) { @@ -1451,7 +1451,7 @@ gfc_check_associated (gfc_expr *pointer, gfc_expr *target) int i; bool t; - if (invalid_null_arg (pointer)) + if (gfc_invalid_null_arg (pointer)) return false; attr1 = gfc_expr_attr (pointer); @@ -1477,7 +1477,7 @@ gfc_check_associated (gfc_expr *pointer, gfc_expr *target) if (target == NULL) return true; - if (invalid_null_arg (target)) + if (gfc_invalid_null_arg (target)) return false; if (target->expr_type == EXPR_VARIABLE || target->expr_type == EXPR_FUNCTION) @@ -3374,7 +3374,7 @@ gfc_check_kill_sub (gfc_expr *pid, gfc_expr *sig, gfc_expr *status) bool gfc_check_kind (gfc_expr *x) { - if (invalid_null_arg (x)) + if (gfc_invalid_null_arg (x)) return false; if (gfc_bt_struct (x->ts.type) || x->ts.type == BT_CLASS) @@ -3453,6 +3453,9 @@ gfc_check_len_lentrim (gfc_expr *s, gfc_expr *kind) if (!type_check (s, 0, BT_CHARACTER)) return false; + if (gfc_invalid_null_arg (s)) +return false; + if (!kind_check (kind, 1, BT_INTEGER)) return false; if (kind && !gfc_notify_std (GFC_STD_F2003, "%qs intrinsic " @@ -4138,10 +4141,10 @@ gfc_check_transf_bit_intrins (gfc_actual_arglist *ap) bool gfc_check_merge (gfc_expr *tsource, gfc_expr *fsource, gfc_expr *mask) { - if (invalid_null_arg (tsource)) + if (gfc_invalid_null_arg (tsource)) return false; - if (invalid_null_arg (fsource)) + if (gfc_invalid_null_arg (fsource)) return false; if (!same_type_check (tsource, 0, fsource, 1)) @@ -5061,7 +5064,7 @@ gfc_check_shape (gfc_expr *source, gfc_expr *kind) { gfc_array_ref *ar; - if (invalid_null_arg (source)) + if (gfc_invalid_null_arg (source)) return false; if (source->rank == 0 || source->expr_type != EXPR_VARIABLE) @@ -5146,7 +5149,7 @@ gfc_check_size (gfc_expr *array, gfc_expr *dim, gfc_expr *kind) bool gfc_check_sizeof (gfc_expr *arg) { - if (invalid_null_arg (arg)) + if (gfc_invalid_null_arg (arg)) return false; if (arg->ts.type == BT_PROCEDURE) @@ -5634,7 +5637,7 @@ gfc_check_sngl (gfc_expr *a) bool gfc_check_spread (gfc_expr *source, gfc_expr *dim, gfc_expr *ncopies) { - if (invalid_null_arg (source)) + if (gfc_invalid_null_arg (source)) return false; if (source->rank >= GFC_MAX_DIMENSIONS) @@ -6167,7 +6170,7 @@ gfc_check_transfer (gfc_expr *source, gfc_expr *mold, gfc_expr *size) size_t source_size; size_t result_size; - if (invalid_null_arg (source)) + if (gfc_invalid_null_arg (source)) return false; /* SOURCE shall be a scalar or array of any type. */ @@ -6186,7 +6189,7 @@ gfc_check_transfer (gfc_expr *source, gfc_expr *mold, gfc_expr *size) if (mold->ts.type == BT_BOZ && illegal_boz_arg (mold)) return false; - if (invalid_null_arg (mold)) + if (gfc_invalid_null_arg (mold)) return false; /* MOLD shall be a scalar or array of any type. */ @@ -6412,6 +6415,9 @@ gfc_check_trim (gfc_expr *x) if (!type_check (x, 0, BT_CHARACTER)) return false; + if (gfc_invalid_null_arg (x)) +return false; + if (!scalar_check (x, 0)) return false; diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 0ef7b1b0eff..6d76efb5298 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -3553,6 +3553,7 @@ bool gfc_calculate_transfer_sizes (gfc_expr*, gfc_expr*, gfc_expr*, bool gfc_boz2int (gfc_expr *, int); bool gfc_boz2real (gfc_expr *, int); bool gfc_invalid_boz (const char *, locus *); +bool gfc_invalid_null_arg (gfc_expr *); /* class.c */ diff --git a/gcc/fortran/intrinsic.c b/gcc/fortran/intrinsic.c index 17f5efc6566..95150c8b6ce 100644 --- a/gcc/fortran/intrinsic.c +++ b/gcc/fortran/intrinsic.c @@ -4442,6 +4442,18 @@ check_arglist (gfc_actual_arglist **ap, gfc_intrinsic_sym *sym, return false; } + /* F2018, p. 328: An argument to an intrinsic procedure other than + ASSOCIATED, NULL, or PRESENT shall be a data object. An EXPR_NULL + is not a data object. */ + if (actual->expr->expr_type == EXPR_NULL + && !(strcmp(gfc_current_intrinsic, "associated") == 0 + || strcmp(gfc_current_intrinsic, "null") == 0 + || strcmp(gfc_current_intrinsic, "present") == 0)) + { + gfc_invalid_null_arg (actual->expr); + return false; + } + /* If the formal argument
Re: [patch] Fix ICE in verify_sra_access_forest
> I am not very good at reasoning about reverse storage order stuff but > can it ever happen that this reverse is not the same as racc->reverse? > > In order for that to happen, we'd have to have an assignment (folded > memcpy?) between two aggregates in the original code that, at the same > offset within the aggregates, have single field structures with > different storage orders. If that has undefined behavior (even for > folded memcpy), I guess I am fine with the patch (but I cannot approve > it). If not, I'd add a condition that racc->reverse is the same as > TYPE_REVERSE_STORAGE_ORDER(lacc->type) to the if guarding all of this. You cannot do an assignment between aggregates with opposite storage order, that's rejected by the compiler (both in C and Ada); generally speaking, such types are not compatible, even if they have the same underlying structure. We also block SRA when there is a VIEW_CONVERT_EXPR messing with the storage order, see calls to storage_order_barrier_p. And folding a memcpy between them should be invalid too (see e.g. the ongoing discussion with Richard in an earlier thread). So my understanding is that you don't need the condition. -- Eric Botcazou
Re: [PATCH] diagnostics: Add options to control the column units [PR49973] [PR86904]
On Wed, Jun 10, 2020 at 12:11:00PM -0400, David Malcolm wrote: > Thanks for the patch; sorry about the delay in reviewing it. > > Some high-level review points > > - I like the patch overall > > - This will deserve an item in the release notes > > - I don't like adding "global_tabstop" (I don't like global > variables). Is there nowhere else we can handle this? I believe > there's a cluster of functions in the callgraph that make use of > it; can we simply pass around the tabstop value instead? "tabstop" > seems to have several meanings. If I'm reading the patch correctly > * "tabstop > 0" means to expand tabs so that column numbers are a > multiple of tabstop > * "tabstop == 0" means "don't expand tabs" > * "tabstop < 0" in some places means: use the global_tabstop value > Is it possible to eliminate global_tabstop value? Or is there some > deep reason I'm missing? > > I'll do a more thorough review once that's addressed/resolved (since > eliminating global_tabstop might touch a few places). > Thanks for the feedback! The attached updated patch addresses these concerns. Regarding tabstop, I have removed the new static variable global_tabstop in charset.c. FWIW, the usage of "tabstop" arguments in the various new APIs did previously work a bit more consistently than you described. In all cases "tabstop <= 0" meant to use the default value, otherwise it specified the tabstop to use (with tabstop=1 naturally restoring the old behavior of changing tabs to a single space). In order for libcpp to provide this feature (callers can pass tabstop <= 0 to get a default, and the default can in turn by configured when processing the -ftabstop option), it does need to remember the default, and this has to be a file-level static variable because the routines need to work independent of any cpp_reader instance. (Some frontends don't use libcpp to read their input, for instance.) Anyway, I see the point that this file-level static, being accessible with cpp_set_tabstop() and cpp_get_tabstop(), is effectively just a global variable, so I have removed this feature, which just means that all callers need to pass the tabstop they want to use. I am now rather using the diagnostic_context object to remember the value passed to -ftabstop. The only place this involves global variables is now in c-family/c-indentation.c, where if I understood correctly, the only diagnostic_context available is global_dc, so I am getting the tabstop value from there. Please let me know if there's a better way to handle that? Prior to my patch, the tabstop was obtained from a different global variable (extern cpp_options *cpp_opts), so at least conservation of total globals is maintained. :) Compared to the previous version, this one is a bit longer, since 25 or so call sites had to be modified to know the value of -ftabstop. Most of the churn is in diagnostic-show-locus.c, because there are a fair number of static helper functions and helper classes there, which just needed to receive the diagnostic_context object from their callers. I could have made this simpler by letting the tabstop argument default to something like 8 in all functions that require it... this would remove the need to pass it in all the selftests that are indifferent to it. I figured it would be better to force this argument to be passed, though, or else in the future it may be easy to forget to pass it where it is needed. > Thanks for adding docs; some nits on them: > > > --- a/gcc/doc/invoke.texi > > +++ b/gcc/doc/invoke.texi > > [...snip...] > > > +@item -fdiagnostics-column-unit=@var{UNIT} > > +@opindex fdiagnostics-column-unit > > +Select the units for the column number. This affects traditional > > diagnostics > > +(in the absence of @option{-fno-show-column}), as well as JSON format > > +diagnostics if requested. > > + > > +The default @var{UNIT}, @samp{display}, considers the number of display > > columns > > +occupied by each character. This may be larger than the number of bytes > > +occupied, in the case of tab characters, or it may be smaller, in the case > > of > > +multibyte characters. For example, the UTF-8 character ``@U{03C0}'' > > occupies > > +two bytes and one display column, while the character ``@U{1F642}'' > > occupies > > +four bytes and two display columns. > > This is imprecise. A unicode code point occupies some number of display > columns, > and its *UTF-8 encoding* occupies some number of bytes. > > [and my inner pedant is now thinking: what about combining diacritics? > But I don't think we can ever issue a diagnostic on a diacritic; I > *think* we only ever care about the per-glyph level] > > > +Setting @var{UNIT} to @samp{byte} changes the column number to the > raw byte > > +count in all cases, as was traditionally output by GCC prior to version > > 11.1.0. > > + > > +@item -fdiagnostics-column-origin=@var{ORIGIN} > > +@opindex fdiagnostics-column-origin > > +Select the origin for column numbers, i.e. the col
Re: [PATCH] PR fortran/95544 - ICE in gfc_can_put_var_on_stack, at fortran/trans-decl.c:494
Hi Harald, one remark: Instead of + && !(strcmp(gfc_current_intrinsic, "associated") == 0 + || strcmp(gfc_current_intrinsic, "null") == 0 + || strcmp(gfc_current_intrinsic, "present") == 0)) could you maybe test sym->id ? OK with that change. Best regards, and thanks for the patch Thomas
Re: [PATCH] Fix few -Wformat-diag warnings.
On 6/11/20 2:43 AM, Martin Liška wrote: Ready for master? I've been meaning to do some of this to resolve pr94982. Thanks a lot for taking the lead on it! I'll try to get to it soon! Martin Thanks, Martin gcc/ChangeLog: * cgraphunit.c (process_symver_attribute): Wrap weakref keyword. * dbgcnt.c (dbg_cnt_set_limit_by_index): Do not print extra new line. * lto-wrapper.c (merge_and_complain): Wrap option names. --- gcc/cgraphunit.c | 2 +- gcc/dbgcnt.c | 2 +- gcc/lto-wrapper.c | 12 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c index 01b3f82a4b2..ea9a34bda6f 100644 --- a/gcc/cgraphunit.c +++ b/gcc/cgraphunit.c @@ -762,7 +762,7 @@ process_symver_attribute (symtab_node *n) if (n->weakref) { error_at (DECL_SOURCE_LOCATION (n->decl), - "weakref cannot be versioned"); + "% cannot be versioned"); return; } if (!TREE_PUBLIC (n->decl)) diff --git a/gcc/dbgcnt.c b/gcc/dbgcnt.c index b0dd893ed49..ae98a281d63 100644 --- a/gcc/dbgcnt.c +++ b/gcc/dbgcnt.c @@ -126,7 +126,7 @@ dbg_cnt_set_limit_by_index (enum debug_counter index, const char *name, if (t1.first <= t2.second) { error ("Interval overlap of %<-fdbg-cnt=%s%>: [%u, %u] and " - "[%u, %u]\n", name, t2.first, t2.second, t1.first, t1.second); + "[%u, %u]", name, t2.first, t2.second, t1.first, t1.second); return false; } } diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c index d565b0861f5..8fbca7cabc4 100644 --- a/gcc/lto-wrapper.c +++ b/gcc/lto-wrapper.c @@ -508,24 +508,24 @@ merge_and_complain (struct cl_decoded_option **decoded_options, break; else if (i < *decoded_options_count && j == fdecoded_options_count) { - warning (0, "Extra option to -Xassembler: %s," - " dropping all -Xassembler and -Wa options.", + warning (0, "Extra option to %<-Xassembler%>: %s," + " dropping all %<-Xassembler%> and %<-Wa%> options.", (*decoded_options)[i].arg); xassembler_options_error = true; break; } else if (i == *decoded_options_count && j < fdecoded_options_count) { - warning (0, "Extra option to -Xassembler: %s," - " dropping all -Xassembler and -Wa options.", + warning (0, "Extra option to %<-Xassembler%>: %s," + " dropping all %<-Xassembler%> and %<-Wa%> options.", fdecoded_options[j].arg); xassembler_options_error = true; break; } else if (strcmp ((*decoded_options)[i].arg, fdecoded_options[j].arg)) { - warning (0, "Options to Xassembler do not match: %s, %s," - " dropping all -Xassembler and -Wa options.", + warning (0, "Options to %<-Xassembler%> do not match: %s, %s," + " dropping all %<-Xassembler%> and %<-Wa%> options.", (*decoded_options)[i].arg, fdecoded_options[j].arg); xassembler_options_error = true; break;
[committed] libstdc++: Fix istream::ignore discarding too many chars (PR 94749)
The current code assumes that if the next character in the stream is equal to the delimiter then we stopped because we saw that delimiter, and so discards it. But in the testcase for the PR we stop because we reached the maximum number of characters, and it's coincidence that the next character equals the delimiter. We should not discard the next character in that case. The fix is to check that we haven't discarded __n characters already, instead of checking whether the next character equals __delim. Because we've already checked for EOF, if we haven't discarded __n yet then we know we stopped because we saw the delimiter. On the other hand, if the next character is the delimiter we don't know if that's why we stopped. PR libstdc++/94749 * include/bits/istream.tcc (basic_istream::ignore(streamsize, CharT)): Only discard an extra character if we didn't already reach the maximum number. * src/c++98/istream.cc (istream::ignore(streamsiz, char)) (wistream::ignore(streamsize, wchar_t)): Likewise. * testsuite/27_io/basic_istream/ignore/char/94749.cc: New test. * testsuite/27_io/basic_istream/ignore/wchar_t/94749.cc: New test. Tested powerpc64le-linux, committed to master. commit b32eea9c0c25a03e77170675abc4e4bcab6d2b3b Author: Jonathan Wakely Date: Thu Jun 11 18:41:37 2020 +0100 libstdc++: Fix istream::ignore discarding too many chars (PR 94749) The current code assumes that if the next character in the stream is equal to the delimiter then we stopped because we saw that delimiter, and so discards it. But in the testcase for the PR we stop because we reached the maximum number of characters, and it's coincidence that the next character equals the delimiter. We should not discard the next character in that case. The fix is to check that we haven't discarded __n characters already, instead of checking whether the next character equals __delim. Because we've already checked for EOF, if we haven't discarded __n yet then we know we stopped because we saw the delimiter. On the other hand, if the next character is the delimiter we don't know if that's why we stopped. PR libstdc++/94749 * include/bits/istream.tcc (basic_istream::ignore(streamsize, CharT)): Only discard an extra character if we didn't already reach the maximum number. * src/c++98/istream.cc (istream::ignore(streamsiz, char)) (wistream::ignore(streamsize, wchar_t)): Likewise. * testsuite/27_io/basic_istream/ignore/char/94749.cc: New test. * testsuite/27_io/basic_istream/ignore/wchar_t/94749.cc: New test. diff --git a/libstdc++-v3/include/bits/istream.tcc b/libstdc++-v3/include/bits/istream.tcc index c82da56b9f9..d36374c707f 100644 --- a/libstdc++-v3/include/bits/istream.tcc +++ b/libstdc++-v3/include/bits/istream.tcc @@ -601,11 +601,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION if (traits_type::eq_int_type(__c, __eof)) __err |= ios_base::eofbit; - else if (traits_type::eq_int_type(__c, __delim)) + else if (_M_gcount < __n) // implies __c == __delim { - if (_M_gcount - < __gnu_cxx::__numeric_traits::__max) - ++_M_gcount; + ++_M_gcount; __sb->sbumpc(); } } diff --git a/libstdc++-v3/src/c++98/istream.cc b/libstdc++-v3/src/c++98/istream.cc index 79d829e23b4..d6fee1a0d66 100644 --- a/libstdc++-v3/src/c++98/istream.cc +++ b/libstdc++-v3/src/c++98/istream.cc @@ -171,11 +171,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION if (traits_type::eq_int_type(__c, __eof)) __err |= ios_base::eofbit; - else if (traits_type::eq_int_type(__c, __delim)) + else if (_M_gcount < __n) // implies __c == __delim { - if (_M_gcount - < __gnu_cxx::__numeric_traits::__max) - ++_M_gcount; + ++_M_gcount; __sb->sbumpc(); } } @@ -413,11 +411,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION if (traits_type::eq_int_type(__c, __eof)) __err |= ios_base::eofbit; - else if (traits_type::eq_int_type(__c, __delim)) + else if (_M_gcount < __n) // implies __c == __delim { - if (_M_gcount - < __gnu_cxx::__numeric_traits::__max) - ++_M_gcount; + ++_M_gcount; __sb->sbumpc(); } } diff --git a/libstdc++-v3/testsuite/27_io/basic_istream/ignore/char/94749.cc b/libstdc++-v3/testsuite/27_io/basic_istream/ignore/char/94749.cc new file mode 100644 index 000..03b5286b00e --- /dev/null +++ b/libstdc++-v3/testsuite/27_io/basic_istream/ignore/ch
Re: [PATCH/RFC] How to fix PR95440
On 6/10/20 4:43 PM, Iain Sandoe wrote: Hi Jason, Jason Merrill wrote: On Tue, Jun 9, 2020 at 5:04 AM Iain Sandoe wrote: /* Don't bother reversing an operator with two identical parameters. */ - else if (args->length () == 2 && (flags & LOOKUP_REVERSED)) + else if (args && args->length () == 2 && (flags & LOOKUP_REVERSED)) The usual pattern is to use vec_safe_length here. Similarly, in build_new_method_call_1 I think !user_args->is_empty() should be vec_safe_is_empty (user_args) Those changes are OK. Thanks, What I applied (after regtest on x86_64-linux/darwin and powerpc64-linux) is below. Given that this fixes an IDE-on-valid filed against 10.1, I’d like to backport it for 10.2, is that OK? Yes. thanks Iain coroutines: Make call argument handling more robust [PR95440] build_new_method_call is supposed to be able to handle a null arguments list pointer (when the method has no parms). There were a couple of places where uses of the argument list pointer were not defended against NULL. gcc/cp/ChangeLog: PR c++/95440 * call.c (add_candidates): Use vec_safe_length() for testing the arguments list. (build_new_method_call_1): Use vec_safe_is_empty() when checking for an empty args list. gcc/testsuite/ChangeLog: PR c++/95440 * g++.dg/coroutines/pr95440.C: New test. --- gcc/cp/call.c | 4 +-- gcc/testsuite/g++.dg/coroutines/pr95440.C | 39 +++ 2 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95440.C diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 3c97b9846e2..b99959f76f9 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -5862,7 +5862,7 @@ add_candidates (tree fns, tree first_arg, const vec *args, } /* Don't bother reversing an operator with two identical parameters. */ - else if (args->length () == 2 && (flags & LOOKUP_REVERSED)) + else if (vec_safe_length (args) == 2 && (flags & LOOKUP_REVERSED)) { tree parmlist = TYPE_ARG_TYPES (TREE_TYPE (fn)); if (same_type_p (TREE_VALUE (parmlist), @@ -10263,7 +10263,7 @@ build_new_method_call_1 (tree instance, tree fns, vec **args, && !(flags & LOOKUP_ONLYCONVERTING) && cxx_dialect >= cxx20 && CP_AGGREGATE_TYPE_P (basetype) - && !user_args->is_empty ()) + && !vec_safe_is_empty (user_args)) { /* Create a CONSTRUCTOR from ARGS, e.g. {1, 2} from <1, 2>. */ tree list = build_tree_list_vec (user_args); diff --git a/gcc/testsuite/g++.dg/coroutines/pr95440.C b/gcc/testsuite/g++.dg/coroutines/pr95440.C new file mode 100644 index 000..8542880d1ab --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr95440.C @@ -0,0 +1,39 @@ +#if __has_include() +#include +#else +#include +namespace std { using namespace experimental; } +#endif +#if 0 +struct suspend_n { + const int x; + constexpr suspend_n (int x) : x (x) {} + constexpr static bool await_ready() { return false; } + constexpr static void await_suspend(std::coroutine_handle<>) {} + constexpr static void await_resume() {} +}; +#endif +struct task +{ + struct promise_type + { +auto get_return_object() const { return task{}; } +#if 0 +//static constexpr suspend_n initial_suspend() { return {2}; } +#endif +static constexpr std::suspend_always initial_suspend() { return {}; } +static constexpr std::suspend_never final_suspend() { return {}; } +static constexpr void return_void() {} +static constexpr void unhandled_exception() {} + }; +}; + +task +test_task () +{ + co_await std::suspend_always{}; +} + +auto t = test_task(); + +int main() {}
Re: [PATCH PR95570] vect: ICE: Segmentation fault in vect_loop_versioning
"Yangfei (Felix)" writes: > From 30a0196b0afd45bae9291cfab3fee4ad6b90cbcb Mon Sep 17 00:00:00 2001 > From: Fei Yang > Date: Thu, 11 Jun 2020 19:33:22 +0800 > Subject: [PATCH] vect: Fix an ICE in vect_loop_versioning [PR95570] > > In the test case for PR95570, the only data reference in the loop is a > gather-statter access. Scalar evolution analysis for this data reference > failed, so DR_STEP is NULL_TREE. This leads to the segmentation fault. > We should filter out scatter-gather access in > vect_enhance_data_refs_alignment. Looks good, just a couple of very minor details… > 2020-06-11 Felix Yang > > gcc/ > PR tree-optimization/95570 > * tree-vect-data-refs.c (vect_relevant_for_alignment_p): New function. > (vect_verify_datarefs_alignment): Call it to filter out data > references > in the loop whose alignment is irrelevant. > (vect_get_peeling_costs_all_drs): Likewise. > (vect_peeling_supportable): Likewise. > (vect_enhance_data_refs_alignment): Likewise. > > gcc/testsuite/ > > PR tree-optimization/95570 > * gcc.dg/vect/pr95570.c: New test. > --- > gcc/testsuite/gcc.dg/vect/pr95570.c | 11 > gcc/tree-vect-data-refs.c | 83 - > 2 files changed, 45 insertions(+), 49 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/vect/pr95570.c > > diff --git a/gcc/testsuite/gcc.dg/vect/pr95570.c > b/gcc/testsuite/gcc.dg/vect/pr95570.c > new file mode 100644 > index 000..b9362614004 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/vect/pr95570.c > @@ -0,0 +1,11 @@ > +/* { dg-do compile } */ > +/* { dg-additional-options "-march=armv8.2-a+sve -msve-vector-bits=256 > -mstrict-align -fwrapv" { target aarch64*-*-* } } */ > + > +int x[8][32]; > + > +void > +foo (int start) > +{ > + for (int i = start; i < start + 16; i++) > +x[start][i] = i; > +} > diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c > index 39d5a1b554c..98f6fb76ff9 100644 > --- a/gcc/tree-vect-data-refs.c > +++ b/gcc/tree-vect-data-refs.c > @@ -1129,6 +1129,35 @@ vect_update_misalignment_for_peel (dr_vec_info > *dr_info, >SET_DR_MISALIGNMENT (dr_info, DR_MISALIGNMENT_UNKNOWN); > } > > +/* Return TRUE if alignment is relevant for DR_INFO. */ Just “Return true …“ for new code. TRUE is a hold-over from C days. > +static bool > +vect_relevant_for_alignment_p (dr_vec_info *dr_info) > +{ > + stmt_vec_info stmt_info = dr_info->stmt; > + > + if (!STMT_VINFO_RELEVANT_P (stmt_info)) > +return false; > + > + /* For interleaving, only the alignment of the first access matters. */ > + if (STMT_VINFO_GROUPED_ACCESS (stmt_info) > + && DR_GROUP_FIRST_ELEMENT (stmt_info) != stmt_info) > +return false; > + > + /* For scatter-gather or invariant accesses, alignment is irrelevant > + for them. */ Maybe: /* Scatter-gather and invariant accesses continue to address individual scalars, so vector-level alignment is irrelevant. */ Thanks, Richard
Re: [PATCH] c++: ICE with IMPLICIT_CONV_EXPR in array subscript [PR95508]
On 6/10/20 5:11 PM, Marek Polacek wrote: Since r10-7096 convert_like, when called in a template, creates an IMPLICIT_CONV_EXPR when we're converting to/from array type. In this test, we have e[f], and we're converting f (of type class A) to int, so convert_like in build_new_op_1 created the IMPLICIT_CONV_EXPR that got into cp_build_array_ref which calls maybe_constant_value. My patch above failed to adjust this spot to call fold_non_dependent_expr instead, which can handle codes like I_C_E in a template. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? gcc/cp/ChangeLog: PR c++/95508 * typeck.c (cp_build_array_ref): Call fold_non_dependent_expr instead of maybe_constant_value. gcc/testsuite/ChangeLog: PR c++/95508 * g++.dg/template/conv16.C: New test. --- gcc/cp/typeck.c| 2 +- gcc/testsuite/g++.dg/template/conv16.C | 17 + 2 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/template/conv16.C diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index f01ae656254..07144d4c1fc 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -3565,7 +3565,7 @@ cp_build_array_ref (location_t loc, tree array, tree idx, pointer arithmetic.) */ idx = cp_perform_integral_promotions (idx, complain); - idx = maybe_constant_value (idx); + idx = fold_non_dependent_expr (idx, complain); Hmm, if idx isn't constant this will leave us with non-template trees in the ARRAY_REF. I think what we want is a function that will fold a non-dependent expr to a constant or return the argument. maybe_fold_non_dependent_expr? Jason
Re: [PATCH] PR fortran/95544 - ICE in gfc_can_put_var_on_stack, at fortran/trans-decl.c:494
Hi Thomas, > one remark: Instead of > > + && !(strcmp(gfc_current_intrinsic, "associated") == 0 > + || strcmp(gfc_current_intrinsic, "null") == 0 > + || strcmp(gfc_current_intrinsic, "present") == 0)) > > could you maybe test sym->id ? > > OK with that change. done. See attached for the actual commit. Thanks for the hint and the swift review! Harald diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c index 0afb96c0414..148a3269815 100644 --- a/gcc/fortran/check.c +++ b/gcc/fortran/check.c @@ -1431,8 +1431,8 @@ gfc_check_x_yd (gfc_expr *x, gfc_expr *y) return true; } -static bool -invalid_null_arg (gfc_expr *x) +bool +gfc_invalid_null_arg (gfc_expr *x) { if (x->expr_type == EXPR_NULL) { @@ -1451,7 +1451,7 @@ gfc_check_associated (gfc_expr *pointer, gfc_expr *target) int i; bool t; - if (invalid_null_arg (pointer)) + if (gfc_invalid_null_arg (pointer)) return false; attr1 = gfc_expr_attr (pointer); @@ -1477,7 +1477,7 @@ gfc_check_associated (gfc_expr *pointer, gfc_expr *target) if (target == NULL) return true; - if (invalid_null_arg (target)) + if (gfc_invalid_null_arg (target)) return false; if (target->expr_type == EXPR_VARIABLE || target->expr_type == EXPR_FUNCTION) @@ -3374,7 +3374,7 @@ gfc_check_kill_sub (gfc_expr *pid, gfc_expr *sig, gfc_expr *status) bool gfc_check_kind (gfc_expr *x) { - if (invalid_null_arg (x)) + if (gfc_invalid_null_arg (x)) return false; if (gfc_bt_struct (x->ts.type) || x->ts.type == BT_CLASS) @@ -3453,6 +3453,9 @@ gfc_check_len_lentrim (gfc_expr *s, gfc_expr *kind) if (!type_check (s, 0, BT_CHARACTER)) return false; + if (gfc_invalid_null_arg (s)) +return false; + if (!kind_check (kind, 1, BT_INTEGER)) return false; if (kind && !gfc_notify_std (GFC_STD_F2003, "%qs intrinsic " @@ -4138,10 +4141,10 @@ gfc_check_transf_bit_intrins (gfc_actual_arglist *ap) bool gfc_check_merge (gfc_expr *tsource, gfc_expr *fsource, gfc_expr *mask) { - if (invalid_null_arg (tsource)) + if (gfc_invalid_null_arg (tsource)) return false; - if (invalid_null_arg (fsource)) + if (gfc_invalid_null_arg (fsource)) return false; if (!same_type_check (tsource, 0, fsource, 1)) @@ -5061,7 +5064,7 @@ gfc_check_shape (gfc_expr *source, gfc_expr *kind) { gfc_array_ref *ar; - if (invalid_null_arg (source)) + if (gfc_invalid_null_arg (source)) return false; if (source->rank == 0 || source->expr_type != EXPR_VARIABLE) @@ -5146,7 +5149,7 @@ gfc_check_size (gfc_expr *array, gfc_expr *dim, gfc_expr *kind) bool gfc_check_sizeof (gfc_expr *arg) { - if (invalid_null_arg (arg)) + if (gfc_invalid_null_arg (arg)) return false; if (arg->ts.type == BT_PROCEDURE) @@ -5634,7 +5637,7 @@ gfc_check_sngl (gfc_expr *a) bool gfc_check_spread (gfc_expr *source, gfc_expr *dim, gfc_expr *ncopies) { - if (invalid_null_arg (source)) + if (gfc_invalid_null_arg (source)) return false; if (source->rank >= GFC_MAX_DIMENSIONS) @@ -6167,7 +6170,7 @@ gfc_check_transfer (gfc_expr *source, gfc_expr *mold, gfc_expr *size) size_t source_size; size_t result_size; - if (invalid_null_arg (source)) + if (gfc_invalid_null_arg (source)) return false; /* SOURCE shall be a scalar or array of any type. */ @@ -6186,7 +6189,7 @@ gfc_check_transfer (gfc_expr *source, gfc_expr *mold, gfc_expr *size) if (mold->ts.type == BT_BOZ && illegal_boz_arg (mold)) return false; - if (invalid_null_arg (mold)) + if (gfc_invalid_null_arg (mold)) return false; /* MOLD shall be a scalar or array of any type. */ @@ -6412,6 +6415,9 @@ gfc_check_trim (gfc_expr *x) if (!type_check (x, 0, BT_CHARACTER)) return false; + if (gfc_invalid_null_arg (x)) +return false; + if (!scalar_check (x, 0)) return false; diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 0ef7b1b0eff..6d76efb5298 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -3553,6 +3553,7 @@ bool gfc_calculate_transfer_sizes (gfc_expr*, gfc_expr*, gfc_expr*, bool gfc_boz2int (gfc_expr *, int); bool gfc_boz2real (gfc_expr *, int); bool gfc_invalid_boz (const char *, locus *); +bool gfc_invalid_null_arg (gfc_expr *); /* class.c */ diff --git a/gcc/fortran/intrinsic.c b/gcc/fortran/intrinsic.c index 17f5efc6566..60d91f658bd 100644 --- a/gcc/fortran/intrinsic.c +++ b/gcc/fortran/intrinsic.c @@ -4442,6 +4442,18 @@ check_arglist (gfc_actual_arglist **ap, gfc_intrinsic_sym *sym, return false; } + /* F2018, p. 328: An argument to an intrinsic procedure other than + ASSOCIATED, NULL, or PRESENT shall be a data object. An EXPR_NULL + is not a data object. */ + if (actual->expr->expr_type == EXPR_NULL + && (!(sym->id == GFC_ISYM_ASSOCIATED + || sym->id == GFC_ISYM_NULL + || sym->id == GFC_ISYM_PRESENT))) + { + gfc_invalid_null_arg (actual->expr); + return false; + } + /* If the formal argument is INTENT([IN]OUT), c
Re: [PATCH] rs6000: skip debug info statements
Hi! On Thu, Jun 11, 2020 at 01:49:57PM +0200, Martin Liška wrote: > Since stmt_vec_info is not generated for debug info statements, these needs > to > be skipped in rs6000_density_test. > PR target/95627 > * config/rs6000/rs6000.c (rs6000_density_test): Skip debug > statements. > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c > index 0bbd06ad1de..00daf979856 100644 > --- a/gcc/config/rs6000/rs6000.c > +++ b/gcc/config/rs6000/rs6000.c > @@ -4987,6 +4987,9 @@ rs6000_density_test (rs6000_cost_data *data) >for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) > { > gimple *stmt = gsi_stmt (gsi); > + if (is_gimple_debug (stmt)) > + continue; Yes, this is fine for trunk (with the indent fixed), thanks! Or you could use gsi_next_nondebug? That is a bit neater. Either way is okay. Thanks! Segher
PR fortran/95611 - ICE in access_attr_decl, at fortran/decl.c:9075
Found by Steve. Committed as obvious. Thanks, Harald PR fortran/95611 - ICE in access_attr_decl, at fortran/decl.c:9075 When reporting a duplicate access specification of an operator, refer to the proper symbol. 2020-06-11 Harald Anlauf gcc/fortran/ PR fortran/95611 * decl.c (access_attr_decl): Use correct symbol in error message. Co-Authored-By: Steven G. Kargl diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c index 1c1626d3fa4..c8a98537e87 100644 --- a/gcc/fortran/decl.c +++ b/gcc/fortran/decl.c @@ -9073,7 +9073,7 @@ access_attr_decl (gfc_statement st) else { gfc_error ("Access specification of the .%s. operator at %C " -"has already been specified", sym->name); +"has already been specified", uop->name); goto done; } diff --git a/gcc/testsuite/gfortran.dg/pr95611.f90 b/gcc/testsuite/gfortran.dg/pr95611.f90 new file mode 100644 index 000..b7a54514ca3 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr95611.f90 @@ -0,0 +1,7 @@ +! { dg-do compile } +! PR fortran/95611 - ICE in access_attr_decl, at fortran/decl.c:9075 + +module m + public operator (.a.) + public operator (.a.) ! { dg-error "has already been specified" } +end
[PATCH] middle-end: Optimize (A&C)^(B&C) to (A^B)&C in simplify_rtx.
My apologies in advance for a middle-end patch without a test case. The patch below implements a simple/safe missing transformation in the RTL optimizers, that transforms (A&C)^(B&C) into the equivalent (A^B)&C, when C doesn't side-effect, such as a constant. I originally identified this opportunity in gfortran, where: integer function foo(i) result(res) integer(kind=16), intent(in) :: i res = poppar(i) end function currently on x86_64 with -O2 -march=corei7 gfortran produces: foo_: popcntq (%rdi), %rdx popcntq 8(%rdi), %rax andl$1, %edx andl$1, %eax xorl%edx, %eax ret But with this patch, now produces: foo_: popcntq (%rdi), %rdx popcntq 8(%rdi), %rax xorl%edx, %eax andl$1, %eax ret The equivalent C/C++ testcase is: unsigned int foo(unsigned int x, unsigned int y) { return __builtin_parityll(x) ^ __builtin_parityll(y); } where GCC currently generates: foo:movl%esi, %eax movl%edi, %edi popcntq %rdi, %rdi popcntq %rax, %rax andl$1, %edi andl$1, %eax xorl%edi, %eax ret and with this patch, it instead generates: foo:movl%esi, %eax movl%edi, %edi popcntq %rdi, %rdi popcntq %rax, %rax xorl%edi, %eax andl$1, %eax ret The trouble is I'm just about to submit a patch to improve constant folding of parity in the middle-end's match.pd, which will generate different RTL code sequences for the above two examples. Hopefully, folks agree it's better to have a RTL optimization that's difficult to test, than not perform this simplification at all. The semantics/correctness of this transformation are tested by the run-time tests in gfortran.dg/popcnt_poppar_2.F90 This patch has been tested with "make bootstrap" and "make -k check" on x86_64-pc-linux-gnu with no regressions. If approved, I'd very much appreciate it if someone could commit this change for me. 2020-06-11 Roger Sayle * simplify-rtx.c (simplify_binary_operation_1): Simplify (X & C) ^ (Y & C) to (X ^ Y) & C, when C is simple (i.e. a constant). Thanks very much in advance, Roger -- Roger Sayle NextMove Software Cambridge, UK diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c index 28c2dc6..ccf5f6d 100644 --- a/gcc/simplify-rtx.c +++ b/gcc/simplify-rtx.c @@ -3128,6 +3128,17 @@ simplify_binary_operation_1 (enum rtx_code code, machine_mode mode, mode); } + /* Convert (xor (and A C) (and B C)) into (and (xor A B) C). */ + if (GET_CODE (op0) == AND + && GET_CODE (op1) == AND + && rtx_equal_p (XEXP (op0, 1), XEXP (op1, 1)) + && ! side_effects_p (XEXP (op0, 1))) + return simplify_gen_binary (AND, mode, + simplify_gen_binary (XOR, mode, +XEXP (op0, 0), +XEXP (op1, 0)), + XEXP (op0, 1)); + /* Convert (xor (and A B) B) to (and (not A) B). The latter may correspond to a machine insn or result in further simplifications if B is a constant. */
Re: [PATCH] c++: Fix ICE in check_local_shadow with enum [PR95560]
On 6/10/20 5:11 PM, Marek Polacek wrote: Another indication that perhaps this warning is emitted too early. We crash because same_type_p gets a null type: we have an enumerator without a fixed underlying type and finish_enum_value_list hasn't yet run. So check if the type is null before calling same_type_p. Hmm, I wonder why we use NULL_TREE for the type of uninitialized enumerators in a template; why not give them integer_type_node temporarily? (This is a regression and this fix is suitable for backporting. Delaying the warning would not be suitable to backport.) Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10/9? gcc/cp/ChangeLog: PR c++/95560 * name-lookup.c (check_local_shadow): Check if types are non-null before calling same_type_p. gcc/testsuite/ChangeLog: PR c++/95560 * g++.dg/warn/Wshadow-local-3.C: New test. --- gcc/cp/name-lookup.c| 4 +++- gcc/testsuite/g++.dg/warn/Wshadow-local-3.C | 7 +++ 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wshadow-local-3.C diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c index 2ff85f1cf5e..159c98a67cd 100644 --- a/gcc/cp/name-lookup.c +++ b/gcc/cp/name-lookup.c @@ -2762,7 +2762,9 @@ check_local_shadow (tree decl) enum opt_code warning_code; if (warn_shadow) warning_code = OPT_Wshadow; - else if (same_type_p (TREE_TYPE (old), TREE_TYPE (decl)) + else if ((TREE_TYPE (old) + && TREE_TYPE (decl) + && same_type_p (TREE_TYPE (old), TREE_TYPE (decl))) || TREE_CODE (decl) == TYPE_DECL || TREE_CODE (old) == TYPE_DECL || (!dependent_type_p (TREE_TYPE (decl)) diff --git a/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C new file mode 100644 index 000..fd743eca347 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C @@ -0,0 +1,7 @@ +// PR c++/95560 +// { dg-do compile { target c++11 } } + +template void fn1() { + bool ready; + enum class State { ready }; +} base-commit: 3a391adf7a38780f8d01dbac08a2a143fc80b469
Re: [PATCH] c++: Fix ICE in check_local_shadow with enum [PR95560]
On 6/11/20 3:34 PM, Jason Merrill wrote: On 6/11/20 3:32 PM, Jason Merrill wrote: On 6/10/20 5:11 PM, Marek Polacek wrote: Another indication that perhaps this warning is emitted too early. We crash because same_type_p gets a null type: we have an enumerator without a fixed underlying type and finish_enum_value_list hasn't yet run. So check if the type is null before calling same_type_p. Hmm, I wonder why we use NULL_TREE for the type of uninitialized enumerators in a template; why not give them integer_type_node temporarily? But this patch is OK for 10.2. And 9. (This is a regression and this fix is suitable for backporting. Delaying the warning would not be suitable to backport.) Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10/9? gcc/cp/ChangeLog: PR c++/95560 * name-lookup.c (check_local_shadow): Check if types are non-null before calling same_type_p. gcc/testsuite/ChangeLog: PR c++/95560 * g++.dg/warn/Wshadow-local-3.C: New test. --- gcc/cp/name-lookup.c | 4 +++- gcc/testsuite/g++.dg/warn/Wshadow-local-3.C | 7 +++ 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wshadow-local-3.C diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c index 2ff85f1cf5e..159c98a67cd 100644 --- a/gcc/cp/name-lookup.c +++ b/gcc/cp/name-lookup.c @@ -2762,7 +2762,9 @@ check_local_shadow (tree decl) enum opt_code warning_code; if (warn_shadow) warning_code = OPT_Wshadow; - else if (same_type_p (TREE_TYPE (old), TREE_TYPE (decl)) + else if ((TREE_TYPE (old) + && TREE_TYPE (decl) + && same_type_p (TREE_TYPE (old), TREE_TYPE (decl))) || TREE_CODE (decl) == TYPE_DECL || TREE_CODE (old) == TYPE_DECL || (!dependent_type_p (TREE_TYPE (decl)) diff --git a/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C new file mode 100644 index 000..fd743eca347 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C @@ -0,0 +1,7 @@ +// PR c++/95560 +// { dg-do compile { target c++11 } } + +template void fn1() { + bool ready; + enum class State { ready }; +} base-commit: 3a391adf7a38780f8d01dbac08a2a143fc80b469
Re: [PATCH] c++: Fix ICE in check_local_shadow with enum [PR95560]
On 6/11/20 3:32 PM, Jason Merrill wrote: On 6/10/20 5:11 PM, Marek Polacek wrote: Another indication that perhaps this warning is emitted too early. We crash because same_type_p gets a null type: we have an enumerator without a fixed underlying type and finish_enum_value_list hasn't yet run. So check if the type is null before calling same_type_p. Hmm, I wonder why we use NULL_TREE for the type of uninitialized enumerators in a template; why not give them integer_type_node temporarily? But this patch is OK for 10.2. (This is a regression and this fix is suitable for backporting. Delaying the warning would not be suitable to backport.) Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10/9? gcc/cp/ChangeLog: PR c++/95560 * name-lookup.c (check_local_shadow): Check if types are non-null before calling same_type_p. gcc/testsuite/ChangeLog: PR c++/95560 * g++.dg/warn/Wshadow-local-3.C: New test. --- gcc/cp/name-lookup.c | 4 +++- gcc/testsuite/g++.dg/warn/Wshadow-local-3.C | 7 +++ 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wshadow-local-3.C diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c index 2ff85f1cf5e..159c98a67cd 100644 --- a/gcc/cp/name-lookup.c +++ b/gcc/cp/name-lookup.c @@ -2762,7 +2762,9 @@ check_local_shadow (tree decl) enum opt_code warning_code; if (warn_shadow) warning_code = OPT_Wshadow; - else if (same_type_p (TREE_TYPE (old), TREE_TYPE (decl)) + else if ((TREE_TYPE (old) + && TREE_TYPE (decl) + && same_type_p (TREE_TYPE (old), TREE_TYPE (decl))) || TREE_CODE (decl) == TYPE_DECL || TREE_CODE (old) == TYPE_DECL || (!dependent_type_p (TREE_TYPE (decl)) diff --git a/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C new file mode 100644 index 000..fd743eca347 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C @@ -0,0 +1,7 @@ +// PR c++/95560 +// { dg-do compile { target c++11 } } + +template void fn1() { + bool ready; + enum class State { ready }; +} base-commit: 3a391adf7a38780f8d01dbac08a2a143fc80b469
Re: [RFA] Fix various regressions and kernel build failure due to adjust-alignment issue
On Tue, 9 Jun 2020, Jeff Law via Gcc-patches wrote: > On Tue, 2020-06-09 at 17:26 +0200, Jakub Jelinek wrote: > > On Tue, Jun 09, 2020 at 09:18:25AM -0600, Jeff Law wrote: > > > On Tue, 2020-06-09 at 16:59 +0200, Jakub Jelinek wrote: > > > > On Tue, Jun 09, 2020 at 08:54:47AM -0600, Jeff Law via Gcc-patches > > > > wrote: > > > > > > While technically OK the issue is that it does not solve the x86 > > > > > > issue > > > > > > which with incoming stack alignment < 8 bytes does not perform > > > > > > dynamic re-alignment to align 'long long' variables appropriately. > > > > > > Currently the x86 backend thinks it can fixup by lowering alignment > > > > > > via LOCAL_DECL_ALIGNMENT but this is a latent wrong-code issue > > > > > > because the larger alignment is present in the IL for a long > > > > > > (previously > > > > > > RTL expansion, now adjust-aligment) time and your patch makes that > > > > > > wrong info last forever (or alternatively cause dynamic stack > > > > > > alignment > > > > > > which we do _not_ want to perform here). > > > > > I've never looked at the dynamic realignment stuff -- is there a good > > > > > reason why > > > > > we don't dynamically realign when we've got a local with a requested > > > > > alignment? > > > > > Isn't that a huge oversight in the whole concept of dynamic > > > > > realignment? > > > > > > > > It is quite expensive and long long/double uses are pervasive, so we > > > > don't > > > > want to realign just because of that. If we do dynamic realignment for > > > > other reasons, there is no reason not to align the long long/double > > > Right, but if there's an explicit alignment from the user, don't we need > > > to honor > > > that? > > > > Sure. Do we ignore that? For user alignment we have DECL_USER_ALIGN bit... > I suspect that's what's going on with the kernel build failure. Sounds like you're about to tackle PR84877. Great! :) brgds, H-P
Re: [PATCH] c++: constrained class template friend [PR93467]
On 6/9/20 2:18 PM, Patrick Palka wrote: This fixes two issues in our handling of constrained class template friend declarations. The first issue is that we fail to set the constraints on the injected class template declaration during tsubst_friend_class. The second issue is that the template parameter levels within the parsed constraints of a class template friend declaration are shifted if the enclosing class is a template, and this shift leads to spurious constraint mismatch errors in associate_classtype_constraints if the friend declaration refers to an already declared class template. Passes 'make check-c++', and also verified by building the testsuite of cmcstl2. Does this look OK to commit to master and to the 10.2 branch after a full bootstrap and regtest? OK. gcc/cp/ChangeLog: PR c++/93467 * constraint.cc (associate_classtype_constraints): If there is a discrepancy between the current template depth and the template depth of the original declaration, then adjust the template parameter depth within the current constraints appropriately. * pt.c (tsubst_friend_class): Substitute into and set the constraints on injected declaration. gcc/testsuite/ChangeLog: PR c++/93467 * g++.dg/cpp2a/concepts-friend6.C: New test. * g++.dg/cpp2a/concepts-friend7.C: New test. --- gcc/cp/constraint.cc | 13 + gcc/cp/pt.c | 10 ++ gcc/testsuite/g++.dg/cpp2a/concepts-friend6.C | 19 +++ gcc/testsuite/g++.dg/cpp2a/concepts-friend7.C | 19 +++ 4 files changed, 61 insertions(+) create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend6.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend7.C diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index 92ff283013e..d0da2300ba9 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -1075,6 +1075,19 @@ associate_classtype_constraints (tree type) original declaration. */ if (tree orig_ci = get_constraints (decl)) { + if (int extra_levels = (TMPL_PARMS_DEPTH (current_template_parms) + - TMPL_ARGS_DEPTH (TYPE_TI_ARGS (type + { + /* If there is a discrepancy between the current template depth +and the template depth of the original declaration, then we +must be redeclaring a class template as part of a friend +declaration within another class template. Before matching +constraints, we need to reduce the template parameter level +within the current constraints via substitution. */ + tree outer_gtargs = template_parms_to_args (current_template_parms); + TREE_VEC_LENGTH (outer_gtargs) = extra_levels; + ci = tsubst_constraint_info (ci, outer_gtargs, tf_none, NULL_TREE); + } if (!equivalent_constraints (ci, orig_ci)) { error ("%qT does not match original declaration", type); diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 142392224c6..7fcb3e9f1c5 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -11211,6 +11211,16 @@ tsubst_friend_class (tree friend_tmpl, tree args) DECL_ANTICIPATED (tmpl) = DECL_ANTICIPATED (DECL_TEMPLATE_RESULT (tmpl)) = true; + /* Substitute into and set the constraints on the new declaration. */ + if (tree ci = get_constraints (friend_tmpl)) + { + ++processing_template_decl; + ci = tsubst_constraint_info (ci, args, tf_warning_or_error, + DECL_FRIEND_CONTEXT (friend_tmpl)); + --processing_template_decl; + set_constraints (tmpl, ci); + } + /* Inject this template into the enclosing namspace scope. */ tmpl = pushdecl_namespace_level (tmpl, true); } diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-friend6.C b/gcc/testsuite/g++.dg/cpp2a/concepts-friend6.C new file mode 100644 index 000..11e8313f0ac --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend6.C @@ -0,0 +1,19 @@ +// PR c++/93467 +// { dg-do compile { target c++20 } } + +template requires B + class C; + +template +class S1 +{ + template requires B +friend class ::C; +}; + +template +class S2 +{ + template requires (!B) +friend class ::C; // { dg-error "does not match original declaration" } +}; diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-friend7.C b/gcc/testsuite/g++.dg/cpp2a/concepts-friend7.C new file mode 100644 index 000..1c6893584b1 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend7.C @@ -0,0 +1,19 @@ +// PR c++/93467 +// { dg-do compile { target c++20 } } + +template concept True = true; + +template +struct S1 { +template friend struct S2; // friend declaration for S2 +}; + +S1 s; // instan
[PATCH] coroutines: Update handling and failure for g-r-o-o-a-f [PR95505]
Hi, The reason that the code fails is that (by a somewhat complex implied route), when a user adds a 'get-return-on-alloc-fail’ to their coroutine promise, this implies the use of some content from ; we should not ICE because the user forgot that tho. Jonathan and I were discussing whether there’s a better way than including in to make this less likely to happen. The issue is that neither nor the user’s code overtly use ; one has to know that the standard makes use of it by implication… anyway that’s not the bug, but the background. tested on Linux, and Darwin. OK for master? 10.2? thanks Iain The actual issue is that (in the testcase) std::nothrow is not available. So update the handling of the get-return-on-alloc-fail to include the possibility that std::nothrow might not be available. gcc/cp/ChangeLog: * coroutines.cc (morph_fn_to_coro): Update handling of get-return-object-on-allocation-fail and diagnose missing std::nothrow. gcc/testsuite/ChangeLog: * g++.dg/coroutines/pr95505.C: New test. --- gcc/cp/coroutines.cc | 43 +++ gcc/testsuite/g++.dg/coroutines/pr95505.C | 26 ++ 2 files changed, 47 insertions(+), 22 deletions(-) create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95505.C diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index f0b7e71633d..93f1e5ca30d 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -3913,30 +3913,25 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) tree grooaf = NULL_TREE; tree dummy_promise = build_dummy_object (get_coroutine_promise_type (orig)); - /* We don't require this, so lookup_promise_method can return NULL... */ + /* We don't require this, so lookup_promise_method can return NULL, + but, if the lookup succeeds, then the function must be usable.*/ if (grooaf_meth && BASELINK_P (grooaf_meth)) -{ - /* ... but, if the lookup succeeds, then the function must be -usable. -build_new_method_call () wants a valid pointer to (an empty) args -list in this case. */ - vec *args = make_tree_vector (); - grooaf = build_new_method_call (dummy_promise, grooaf_meth, &args, - NULL_TREE, LOOKUP_NORMAL, NULL, - tf_warning_or_error); - release_tree_vector (args); -} +grooaf = build_new_method_call (dummy_promise, grooaf_meth, NULL, + NULL_TREE, LOOKUP_NORMAL, NULL, + tf_warning_or_error); /* Allocate the frame, this has several possibilities: [dcl.fct.def.coroutine] / 9 (part 1) The allocation function’s name is looked up in the scope of the promise type. It's not a failure for it to be absent see part 4, below. */ + tree nwname = ovl_op_identifier (false, NEW_EXPR); - tree fns = lookup_promise_method (orig, nwname, fn_start, - /*musthave=*/false); tree new_fn = NULL_TREE; - if (fns && BASELINK_P (fns)) + /* We don't require this, so lookup_promise_method can return NULL... */ + if (TYPE_HAS_NEW_OPERATOR (promise_type)) { + tree fns = lookup_promise_method (orig, nwname, fn_start, + /*musthave=*/true); /* [dcl.fct.def.coroutine] / 9 (part 2) If the lookup finds an allocation function in the scope of the promise type, overload resolution is performed on a function call created by @@ -3973,14 +3968,13 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) LOOKUP_NORMAL, &func, tf_none); release_tree_vector (args); - if (!new_fn || new_fn == error_mark_node) + if (new_fn == error_mark_node) { /* [dcl.fct.def.coroutine] / 9 (part 3) If no viable function is found, overload resolution is performed again on a function call created by passing just the amount of space required as an argument of type std::size_t. */ - args = make_tree_vector (); - vec_safe_push (args, resizeable); /* Space needed. */ + args = make_tree_vector_single (resizeable); /* Space needed. */ new_fn = build_new_method_call (dummy_promise, fns, &args, NULL_TREE, LOOKUP_NORMAL, &func, tf_none); @@ -3989,7 +3983,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) /* However, if the initial lookup succeeded, then one of these two options must be available. */ -if (!new_fn || new_fn == error_mark_node) +if (new_fn == error_mark_node) { error_at (fn_start, "%qE is provided by %qT but is not usable with" " the function signature %qD", nwname, promise_type, orig); @@ -3999,7 +3993,7 @@ morph_fn_to_coro (tree orig, tree *resume
Re: [PATCH] c++: Don't allow designated initializers with non-aggregates [PR95369]
On 6/9/20 2:17 PM, Marek Polacek wrote: Another part of 95369 is that we accept designated initializers with non-aggregate types. That seems to be wrong since they're part of aggregate initialization. clang/icc also reject it. (Un)fortunately there are multiple contexts where we can use designated initializers: function-like casts, member list initializers, NTTP, etc. So I've adjusted multiple places in the compiler in order to to detect this case and to provide a nice diagnostic, instead of an ugly raft of errors. Would it work to handle this only in add_list_candidates? Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? gcc/cp/ChangeLog: PR c++/95369 * call.c (implicit_conversion): Return NULL if a designated initializer is used with a non-aggregate. (implicit_conversion_error): Give an error for the case above. * decl.c (check_initializer): Likewise. * init.c (build_aggr_init): Likewise. * semantics.c (finish_compound_literal): Likewise. gcc/testsuite/ChangeLog: PR c++/95369 * g++.dg/cpp2a/desig11.C: Adjust dg-error. * g++.dg/cpp2a/desig16.C: New test. --- gcc/cp/call.c| 11 +++ gcc/cp/decl.c| 9 + gcc/cp/init.c| 10 ++ gcc/cp/semantics.c | 7 +++ gcc/testsuite/g++.dg/cpp2a/desig11.C | 2 +- gcc/testsuite/g++.dg/cpp2a/desig16.C | 28 6 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/desig16.C diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 3c97b9846e2..346fb850f84 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -2020,6 +2020,12 @@ implicit_conversion (tree to, tree from, tree expr, bool c_cast_p, if (is_std_init_list (to) && !CONSTRUCTOR_IS_DESIGNATED_INIT (expr)) return build_list_conv (to, expr, flags, complain); + /* Designated initializers can only be used to initialize an aggregate +because they're part of aggregate initialization. Return NULL here, +implicit_conversion_error will issue an actual error. */ + if (CONSTRUCTOR_IS_DESIGNATED_INIT (expr) && !CP_AGGREGATE_TYPE_P (to)) + return NULL; + /* As an extension, allow list-initialization of _Complex. */ if (TREE_CODE (to) == COMPLEX_TYPE && !CONSTRUCTOR_IS_DESIGNATED_INIT (expr)) @@ -4301,6 +4307,11 @@ implicit_conversion_error (location_t loc, tree type, tree expr) instantiate_type (type, expr, complain); else if (invalid_nonstatic_memfn_p (loc, expr, complain)) /* We gave an error. */; + else if (BRACE_ENCLOSED_INITIALIZER_P (expr) + && CONSTRUCTOR_IS_DESIGNATED_INIT (expr) + && !CP_AGGREGATE_TYPE_P (type)) +error_at (loc, "designated initializers cannot be used with a " + "non-aggregate type %qT", type); else { range_label_for_type_mismatch label (TREE_TYPE (expr), type); diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index b8bd09b37e6..577643a1523 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -6668,6 +6668,15 @@ check_initializer (tree decl, tree init, int flags, vec **cleanups) return NULL_TREE; } } + else if (CONSTRUCTOR_IS_DESIGNATED_INIT (init) + && !CP_AGGREGATE_TYPE_P (type)) + { + error_at (cp_expr_loc_or_loc (init, DECL_SOURCE_LOCATION (decl)), + "designated initializers cannot be used with a " + "non-aggregate type %qT", type); + TREE_TYPE (decl) = error_mark_node; + return NULL_TREE; + } } if (TREE_CODE (decl) == CONST_DECL) diff --git a/gcc/cp/init.c b/gcc/cp/init.c index ef4b3c4dc3c..de261cfe7a6 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -1802,6 +1802,16 @@ build_aggr_init (tree exp, tree init, int flags, tsubst_flags_t complain) TREE_TYPE (init) = itype; return stmt_expr; } + else if (init + && BRACE_ENCLOSED_INITIALIZER_P (init) + && CONSTRUCTOR_IS_DESIGNATED_INIT (init) + && !CP_AGGREGATE_TYPE_P (type)) +{ + if (complain & tf_error) + error_at (init_loc, "designated initializers cannot be used with a " + "non-aggregate type %qT", type); + return error_mark_node; +} if (init && init != void_type_node && TREE_CODE (init) != TREE_LIST diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index 64587c791c6..f25c4ec7110 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -2933,6 +2933,13 @@ finish_compound_literal (tree type, tree compound_literal, if (TYPE_NON_AGGREGATE_CLASS (type)) { + if (CONSTRUCTOR_IS_DESIGNATED_INIT (compound_literal)) + { + if (complain & tf_error) + error ("designated initializers cannot be used with a " + "non-aggregate type %qT", typ
[PATCH] coroutines: Copy attributes to the outlined functions [PR95518]
Hi We had omitted the copying of function attributes (including the 'used' status). Mark the outlined functions as artificial, since they are; some diagnostic processing tests this. tested on Linux and Darwin, OK for master? 10.2? thanks Iain gcc/cp/ChangeLog: PR c++/95518 * coroutines.cc (act_des_fn): Copy function attributes from the user’s function decl onto the outlined helpers. gcc/testsuite/ChangeLog: PR c++/95518 * g++.dg/coroutines/pr95518.C: New test. --- gcc/cp/coroutines.cc | 5 + gcc/testsuite/g++.dg/coroutines/pr95518.C | 27 +++ 2 files changed, 32 insertions(+) create mode 100644 gcc/testsuite/g++.dg/coroutines/pr95518.C diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index 93f1e5ca30d..4f7356e94e5 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -3530,12 +3530,17 @@ act_des_fn (tree orig, tree fn_type, tree coro_frame_ptr, const char* name) tree fn_name = get_fn_local_identifier (orig, name); tree fn = build_lang_decl (FUNCTION_DECL, fn_name, fn_type); DECL_CONTEXT (fn) = DECL_CONTEXT (orig); + DECL_ARTIFICIAL (fn) = true; DECL_INITIAL (fn) = error_mark_node; tree id = get_identifier ("frame_ptr"); tree fp = build_lang_decl (PARM_DECL, id, coro_frame_ptr); DECL_CONTEXT (fp) = fn; DECL_ARG_TYPE (fp) = type_passed_as (coro_frame_ptr); DECL_ARGUMENTS (fn) = fp; + /* Copy used-ness from the original function. */ + TREE_USED (fn) = TREE_USED (orig); + /* Apply attributes from the original fn. */ + DECL_ATTRIBUTES (fn) = copy_list (DECL_ATTRIBUTES (orig)); return fn; } diff --git a/gcc/testsuite/g++.dg/coroutines/pr95518.C b/gcc/testsuite/g++.dg/coroutines/pr95518.C new file mode 100644 index 000..2d7dd049e1b --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr95518.C @@ -0,0 +1,27 @@ +// { dg-additional-options "-O -Wunused-function" } + +#if __has_include () +#include +using namespace std; +#elif defined (__clang__) && __has_include () +#include +namespace std { using namespace experimental; } +#endif + +struct dummy +{ +struct promise_type +{ +dummy get_return_object() const noexcept { return {}; } +std::suspend_never initial_suspend() const noexcept { return {}; } +std::suspend_never final_suspend() const noexcept { return {}; } +void return_void() const noexcept {} +void unhandled_exception() const noexcept {} +}; +int i; // work around #95516 +}; + +[[maybe_unused]] static dummy foo() +{ +co_return; +} -- 2.24.1
Can we use safe-ctype.h in a library?
Hi, I am currently looking at making libgfortran use the TOUPPER et al functions from ctype-safe.h. Adding this in a gfortran header results in lots of Gcc/trunk-bin/x86_64-pc-linux-gnu/./libgfortran/.libs/libgfortran.so: undefined reference to `_sch_istable' errors. So, can we use this, or should we just define this ourselves with something like #define TOUPPER(a) toupper ((unsigned char) (a)) ? Regards Thomas
Re: [PATCH] underline null argument in -Wnonnull (PR c++/86568)
On 6/5/20 3:41 PM, Martin Sebor wrote: + location_t loc += EXPR_HAS_LOCATION (param) ? EXPR_LOCATION (param) : pctx->loc; This could be EXPR_LOC_OR_LOC (param, pctx->loc) + location_t loc = (EXPR_HAS_LOCATION (ptr) + ? EXPR_LOCATION (ptr) : EXPR_LOCATION (exp)); And similarly here. + if (param_num == 0) { + warned = warning_at (loc, OPT_Wnonnull, + "%qs pointer null", "this"); + if (pctx->fndecl) + inform (DECL_SOURCE_LOCATION (pctx->fndecl), + "in a call to non-static member function %qD", + pctx->fndecl); } + else +{ + warned = warning_at (loc, OPT_Wnonnull, + "argument %u null where non-null expected", + (unsigned) param_num); + if (pctx->fndecl) + inform (DECL_SOURCE_LOCATION (pctx->fndecl), + "in a call to function %qD declared %qs", + pctx->fndecl, "nonnull"); +} You need auto_diagnostic_group somewhere. The c-common.c and tree.c changes are OK with these adjustments. I'll leave the optimizer bits to someone else. Jason
Re: [PATCH] avoid false positives due to compute_objsize (PR 95353)
Hi Martin, > The compute_objsize() function started out as a thin wrapper around > compute_builtin_object_size(), but over time developed its own > features to compensate for the other function's limitations (such > as its inability to work with ranges). The interaction of these > features and the limitations has started to become increasingly > problematic as the former function is used in more contexts. > > A complete "fix" for all the problems (as well as some other > limitations) that I'm working on will be more extensive and won't > be appropriate for backports. Until then, the attached patch > cleans up the extensions compute_objsize() has accumulated over > the years to avoid a class of false positives. > > To make the warnings issued based on the results of the function > easier to understand and fix, the patch also adds an informative > message to many instances of -Wstringop-overflow to point to > the object to which the warning refers. This is especially > helpful when the object is referenced by a series of pointer > operations. > > Tested by boostrapping on x86_64-linux and building Binutils/GDB, > Glibc, and the Linux kernel with no new warnings. > > Besides applying it to trunk I'm looking to backport the fix to > GCC 10. it seems you were over-eager in removing xfail's from gcc.dg/builtin-stringop-chk-5.c: on both Solaris/SPARC and x86 (32-bit only) I see +FAIL: gcc.dg/builtin-stringop-chk-5.c (test for excess errors) +FAIL: gcc.dg/builtin-stringop-chk-5.c memcpy into allocated (test for warnings, line 136) +FAIL: gcc.dg/builtin-stringop-chk-5.c memcpy into allocated (test for warnings, line 139) +FAIL: gcc.dg/builtin-stringop-chk-5.c memcpy into allocated (test for warnings, line 142) +FAIL: gcc.dg/builtin-stringop-chk-5.c memcpy into allocated (test for warnings, line 145) +FAIL: gcc.dg/builtin-stringop-chk-5.c memcpy into allocated (test for warnings, line 148) +FAIL: gcc.dg/builtin-stringop-chk-5.c memcpy into allocated (test for warnings, line 151) Excess errors: /vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/builtin-stringop-chk-5.c:136:3: warning: 'memset' writing 4 bytes into a region of size 1 overflows the destination [-Wstringop-overflow=] /vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/builtin-stringop-chk-5.c:139:3: warning: 'memset' writing 4 bytes into a region of size 1 overflows the destination [-Wstringop-overflow=] /vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/builtin-stringop-chk-5.c:142:3: warning: 'memset' writing 3 bytes into a region of size 1 overflows the destination [-Wstringop-overflow=] /vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/builtin-stringop-chk-5.c:145:3: warning: 'memset' writing 3 bytes into a region of size 1 overflows the destination [-Wstringop-overflow=] /vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/builtin-stringop-chk-5.c:148:3: warning: 'memset' writing 2 bytes into a region of size 1 overflows the destination [-Wstringop-overflow=] /vol/gcc/src/hg/master/local/gcc/testsuite/gcc.dg/builtin-stringop-chk-5.c:151:3: warning: 'memset' writing 2 bytes into a region of size 1 overflows the destination [-Wstringop-overflow=] Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH] c++: Don't allow designated initializers with non-aggregates [PR95369]
On Thu, Jun 11, 2020 at 03:51:29PM -0400, Jason Merrill wrote: > On 6/9/20 2:17 PM, Marek Polacek wrote: > > Another part of 95369 is that we accept designated initializers with > > non-aggregate types. That seems to be wrong since they're part of > > aggregate initialization. clang/icc also reject it. > > > > (Un)fortunately there are multiple contexts where we can use designated > > initializers: function-like casts, member list initializers, NTTP, etc. > > So I've adjusted multiple places in the compiler in order to to detect > > this case and to provide a nice diagnostic, instead of an ugly raft of > > errors. > > Would it work to handle this only in add_list_candidates? 'fraid not -- we don't call add_list_candidates at all when compiling desig16.C. Marek
Re: [PATCH] Port libgccjit to Windows.
Hi, On 6/7/20 11:12 PM, JonY wrote: > Ideally, libtool is used so we get libgccjit-0.dll, unfortunately it is > not. So the only way to ABI version the dll would be to use Unix style > soname to mark when an ABI has changed. I tried generating the library as libgccjit-0.dll and naming its import library libgccjit.dll.a. It worked. I understand you prefer the libgccjit-0.dll filename from this comment about libtool. A patch implementing this change is attached. Thanks for your feedback. Nicolas. 0001-Rename-libgccjit.dll-to-libgccjit-0.dll.patch Description: Binary data
Re: [PATCH 1/2] c++: Improve access checking inside templates [PR41437]
On 6/5/20 5:16 PM, Patrick Palka wrote: This patch generalizes our existing functionality for deferring access checking of typedefs when parsing a function or class template to now defer all kinds of access checks until template instantiation time, including member function and member object accesses. Since all access checks eventually go through enforce_access, the main component of this patch is new handling inside enforce_access to defer the current access check if we're inside a template. The bulk of the rest of the patch consists of removing now-unneeded code pertaining to suppressing access checks inside templates or pertaining to typedef-specific access handling. Renamings and other changes with no functional impact have been split off into the followup patch. Great! Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested by building parts of boost, cmcstl2 and other libraries. - && !dependent_type_p (qualifying_type)) + && !dependent_type_p (scope)) This needs a comment. And it occurs to me that if we're only going to check access if scope is non-dependent, we can check that much earlier and avoid the need to guard DECL_NONSTATIC_MEMBER_P. Jason
Re: [PATCH] c++: Don't allow designated initializers with non-aggregates [PR95369]
On 6/11/20 5:28 PM, Marek Polacek wrote: On Thu, Jun 11, 2020 at 03:51:29PM -0400, Jason Merrill wrote: On 6/9/20 2:17 PM, Marek Polacek wrote: Another part of 95369 is that we accept designated initializers with non-aggregate types. That seems to be wrong since they're part of aggregate initialization. clang/icc also reject it. (Un)fortunately there are multiple contexts where we can use designated initializers: function-like casts, member list initializers, NTTP, etc. So I've adjusted multiple places in the compiler in order to to detect this case and to provide a nice diagnostic, instead of an ugly raft of errors. Would it work to handle this only in add_list_candidates? 'fraid not -- we don't call add_list_candidates at all when compiling desig16.C. Hmm, why not? What is turning the CONSTRUCTOR into an argument vec? Jason
Re: [PATCH 2/7 V3] rs6000: lenload/lenstore optab support
Hi! On Wed, Jun 10, 2020 at 08:39:19PM +0800, Kewen.Lin wrote: > +;; Define optab for vector access with length vectorization exploitation. > +(define_expand "lenload" > + [(match_operand:VEC_A 0 "vlogical_operand") > + (match_operand:VEC_A 1 "memory_operand") > + (match_operand:QI 2 "int_reg_operand")] Why this? gpc_reg_operand will just work, no? (Even just register_operand should, but let's not go there today ;-) ) Okay for trunk with that change, or with some explanation. Thanks! Segher
Re: [PATCH v1 1/2][PPC64] [PR88877]
On Tue, Jun 09, 2020 at 02:29:13PM -0600, Jeff Law wrote: > On Sun, 2020-05-24 at 11:22 -0500, Segher Boessenkool wrote: > > OTOH, you don't need to name Tuple at all... It should not *have* a > > constructor, since you declared it as class... But you can just use > > std::tuple here? > > > > > (emit_library_call): Added default arg unsigned_p. > > > (emit_library_call_value): Added default arg unsigned_p. > > > > Yeah, eww. Default arguments have all the problems you had before, > > except now it is hidden and much more surprising. > > > > Those functions really should take rtx_mode_t arguments? > > > > Thanks again for working on this, > ISTM that using std::tuple would be better than defining our own types. Yeah. But as Jakub an Iain said, not using a container type (but a more concrete type, instead) is much better anyway :-) > I'd rather see the argument be explicit rather than using default arguments > too. > While I have ack'd some patches with default arguments, I still don't like > 'em. Default arguments have their place (but it's not here :-) ) > I do like the approach of getting the infrastructure in place without changing > behavior, then having the behavior fix as a distinct change. With Git, commits are easy and cheap, and massaging a patch series into shape is easy and cheap as well. If you develop using Git in the first place (and you should!), you should naturally end up with many patches in your series, and the preparatory patches first (after you reshuffle things a bit, if you are like me and your foresight is severly limited). So you have this separate *anyway* (or should have). Since it helps reviewing a lot, and also later bisecting, it is good to keep it. Segher
Re: [PATCH] Port libgccjit to Windows.
On 6/11/20 10:02 PM, Nicolas Bértolo wrote: > Hi, > > On 6/7/20 11:12 PM, JonY wrote: >> Ideally, libtool is used so we get libgccjit-0.dll, unfortunately it is >> not. So the only way to ABI version the dll would be to use Unix style >> soname to mark when an ABI has changed. > > I tried generating the library as libgccjit-0.dll and naming its import > library > libgccjit.dll.a. It worked. I understand you prefer the libgccjit-0.dll > filename > from this comment about libtool. A patch implementing this change is attached. > > Thanks for your feedback. > > Nicolas. > Thanks for the patch, it looks good to me. I will push this soon if no one else objects. signature.asc Description: OpenPGP digital signature
Re: [PATCH] guard against calls with fewer arguments than the function type implies (PR 95581)
On Tue, Jun 09, 2020 at 09:51:12AM -0600, Martin Sebor wrote: > >I think the backend declaration is wrong, the function only takes > >a void * argument and returns a long. > > Thanks. In his comment on the bug Segher (CC'd) points to > the internals manual that documents the function: > > https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/target.def;h=07059a87caf7cc0237d583124b476ee45ea41ed5;hb=HEAD#l1744 > > (By the way, thanks for the pointer!) > > If I read it right, ihe function f in > the TARGET_VECTORIZE_BUILTIN_MASK_FOR_LOAD documentation is > __builtin_altivec_mask_for_load. Although the manual isn't > unequivocal about this but it does suggest the address addr > the function is given as an argument should be the first > (and presumably only) argument. This matches the call in > the IL where the first argument is a pointer, but not > the function's signature. > > I think the middle end needs to be able to rely on built-in > function types when processing calls: i.e., the types and > numbers of actual arguments in the calls need to match those > of the built-in function type. Otherwise it would have to > validate every call against the function signature and avoid > treating it as a built-in if it doesn't match. There are > parts of the middle end that do that for library built-ins > (because they can be declared in a program with mismatched > signatures) but as we have seen it's error-prone. I don't > think it would be helpful to try to extend this approach to > internal built-ins. > > So I agree that the real problem is the declaration of > the built-in. The problem is that this altivec builtin cannot implement the generic builtin directly. It should *not* be changed, we *do* need to keep this builtin as-is. We just cannot forward the generic builtin to it like this. Why did this work before, and not anymore? That sounds like a missing or broken test? Thanks, Segher
RE: [PATCH PR95570] vect: ICE: Segmentation fault in vect_loop_versioning
Hi, > -Original Message- > From: Richard Sandiford [mailto:richard.sandif...@arm.com] > Sent: Friday, June 12, 2020 2:29 AM > To: Yangfei (Felix) > Cc: gcc-patches@gcc.gnu.org > Subject: Re: [PATCH PR95570] vect: ICE: Segmentation fault in > vect_loop_versioning > > "Yangfei (Felix)" writes: > > From 30a0196b0afd45bae9291cfab3fee4ad6b90cbcb Mon Sep 17 00:00:00 > 2001 > > From: Fei Yang > > Date: Thu, 11 Jun 2020 19:33:22 +0800 > > Subject: [PATCH] vect: Fix an ICE in vect_loop_versioning [PR95570] > > > > In the test case for PR95570, the only data reference in the loop is a > > gather-statter access. Scalar evolution analysis for this data > > reference failed, so DR_STEP is NULL_TREE. This leads to the segmentation > fault. > > We should filter out scatter-gather access in > vect_enhance_data_refs_alignment. > > Looks good, just a couple of very minor details… > > > 2020-06-11 Felix Yang > > > > gcc/ > > PR tree-optimization/95570 > > * tree-vect-data-refs.c (vect_relevant_for_alignment_p): New > function. > > (vect_verify_datarefs_alignment): Call it to filter out data > > references > > in the loop whose alignment is irrelevant. > > (vect_get_peeling_costs_all_drs): Likewise. > > (vect_peeling_supportable): Likewise. > > (vect_enhance_data_refs_alignment): Likewise. > > > > gcc/testsuite/ > > > > PR tree-optimization/95570 > > * gcc.dg/vect/pr95570.c: New test. > > --- > > gcc/testsuite/gcc.dg/vect/pr95570.c | 11 > > gcc/tree-vect-data-refs.c | 83 - > > 2 files changed, 45 insertions(+), 49 deletions(-) create mode > > 100644 gcc/testsuite/gcc.dg/vect/pr95570.c > > > > diff --git a/gcc/testsuite/gcc.dg/vect/pr95570.c > > b/gcc/testsuite/gcc.dg/vect/pr95570.c > > new file mode 100644 > > index 000..b9362614004 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/vect/pr95570.c > > @@ -0,0 +1,11 @@ > > +/* { dg-do compile } */ > > +/* { dg-additional-options "-march=armv8.2-a+sve > > +-msve-vector-bits=256 -mstrict-align -fwrapv" { target aarch64*-*-* } > > +} */ > > + > > +int x[8][32]; > > + > > +void > > +foo (int start) > > +{ > > + for (int i = start; i < start + 16; i++) > > +x[start][i] = i; > > +} > > diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c > > index 39d5a1b554c..98f6fb76ff9 100644 > > --- a/gcc/tree-vect-data-refs.c > > +++ b/gcc/tree-vect-data-refs.c > > @@ -1129,6 +1129,35 @@ vect_update_misalignment_for_peel > (dr_vec_info *dr_info, > >SET_DR_MISALIGNMENT (dr_info, DR_MISALIGNMENT_UNKNOWN); } > > > > +/* Return TRUE if alignment is relevant for DR_INFO. */ > > Just “Return true …“ for new code. TRUE is a hold-over from C days. OK. > > +static bool > > +vect_relevant_for_alignment_p (dr_vec_info *dr_info) { > > + stmt_vec_info stmt_info = dr_info->stmt; > > + > > + if (!STMT_VINFO_RELEVANT_P (stmt_info)) > > +return false; > > + > > + /* For interleaving, only the alignment of the first access > > + matters. */ if (STMT_VINFO_GROUPED_ACCESS (stmt_info) > > + && DR_GROUP_FIRST_ELEMENT (stmt_info) != stmt_info) > > +return false; > > + > > + /* For scatter-gather or invariant accesses, alignment is irrelevant > > + for them. */ > > Maybe: > > /* Scatter-gather and invariant accesses continue to address individual > scalars, so vector-level alignment is irrelevant. */ Much better : - ) Updated accordingly. Also bootstrapped and tested on x86_64-linux-gnu. Thanks, Felix gcc/ +2020-06-12 Felix Yang + + PR tree-optimization/95570 + * tree-vect-data-refs.c (vect_relevant_for_alignment_p): New function. + (vect_verify_datarefs_alignment): Call it to filter out data references + in the loop whose alignment is irrelevant. + (vect_get_peeling_costs_all_drs): Likewise. + (vect_peeling_supportable): Likewise. + (vect_enhance_data_refs_alignment): Likewise. gcc/testsuite/ +2020-06-12 Felix Yang + + PR tree-optimization/95570 + * gcc.dg/vect/pr95570.c: New test. pr95570-v3.diff Description: pr95570-v3.diff
Re: [PATCH 2/7 V3] rs6000: lenload/lenstore optab support
Hi Segher, on 2020/6/12 上午6:55, Segher Boessenkool wrote: > Hi! > > On Wed, Jun 10, 2020 at 08:39:19PM +0800, Kewen.Lin wrote: >> +;; Define optab for vector access with length vectorization exploitation. >> +(define_expand "lenload" >> + [(match_operand:VEC_A 0 "vlogical_operand") >> + (match_operand:VEC_A 1 "memory_operand") >> + (match_operand:QI 2 "int_reg_operand")] > > Why this? gpc_reg_operand will just work, no? (Even just > register_operand should, but let's not go there today ;-) ) > Good question! The existing lxvl requires register_operand, yeah, gpr_reg_operand looks fine too. I was thinking this operand for length would only exist in GPR, int_reg_operand looks more reasonable here? > Okay for trunk with that change, or with some explanation. Thanks! > Thanks! BR, Kewen > > Segher >
Re: [PATCH] Fix few -Wformat-diag warnings.
On Thu, 2020-06-11 at 10:43 +0200, Martin Liška wrote: > Ready for master? > > Thanks, > Martin > > gcc/ChangeLog: > > * cgraphunit.c (process_symver_attribute): Wrap weakref keyword. > * dbgcnt.c (dbg_cnt_set_limit_by_index): Do not print extra new > line. > * lto-wrapper.c (merge_and_complain): Wrap option names. OK. And I think we should consider other changes of similar nature as pre- approved. Just run them through the usual regression testing. Jeff >
Re: collect2.exe errors not pruned
On Wed, 2020-06-10 at 23:41 -0300, Alexandre Oliva wrote: > On May 26, 2020, Alexandre Oliva wrote: > > > On May 19, 2020, Joseph Myers wrote: > > > Allowing a missing executable name is reasonable enough, but I was > > > actually thinking that the messages should print "gcc" or whatever > > > command > > > the user ran in place of "collect2". > > Should we make the regexps '\[^\n\]*', as in so many other pruned > > messages? > > Like this. Regstrapped on x86_64-linux-gnu. Ok to install? > > > match any program name when pruning collect messages > > From: Alexandre Oliva > > When collect* programs have an executable suffix, they may include it > in their outputs. Match them when pruning gcc output, making room for > other program names to print them. > > > for gcc/testsuite/ChangeLog > > * lib/prune.exp (prune_gcc_output): Match any executable name > in collect messages. OK jeff >
Re: [PATCH] x86: Add UNSPECV_PATCHABLE_AREA
On Sat, 2020-05-02 at 04:55 -0700, H.J. Lu wrote: > Currently patchable area is at the wrong place. It is placed immediately > after function label, before both .cfi_startproc and ENDBR. This patch > adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and > changes ENDBR insertion pass to also insert patchable area instruction. > TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY is defined to avoid placing > patchable area before .cfi_startproc and ENDBR. > > OK for master? > > Thanks. > > H.J. > --- > gcc/ > > PR target/93492 > * config/i386/i386-features.c (rest_of_insert_endbranch): > Renamed to ... > (rest_of_insert_endbr_and_patchable_area): Change return type > to void. Add need_endbr and patchable_area_size arguments. > Don't call timevar_push nor timevar_pop. Replace > endbr_queued_at_entrance with insn_queued_at_entrance. Insert > UNSPECV_PATCHABLE_AREA for patchable area. > (pass_data_insert_endbranch): Renamed to ... > (pass_data_insert_endbr_and_patchable_area): This. Change > pass name to endbr_and_patchable_area. > (pass_insert_endbranch): Renamed to ... > (pass_insert_endbr_and_patchable_area): This. Add need_endbr > and patchable_area_size;. > (pass_insert_endbr_and_patchable_area::gate): Set and check > need_endbr and patchable_area_size. > (pass_insert_endbr_and_patchable_area::execute): Call > timevar_push and timevar_pop. Pass need_endbr and > patchable_area_size to rest_of_insert_endbr_and_patchable_area. > (make_pass_insert_endbranch): Renamed to ... > (make_pass_insert_endbr_and_patchable_area): This. > * config/i386/i386-passes.def: Replace pass_insert_endbranch > with pass_insert_endbr_and_patchable_area. > * config/i386/i386-protos.h (ix86_output_patchable_area): New. > (make_pass_insert_endbranch): Renamed to ... > (make_pass_insert_endbr_and_patchable_area): This. > * config/i386/i386.c (ix86_asm_output_function_label): Set > function_label_emitted to true. > (ix86_print_patchable_function_entry): New function. > (ix86_output_patchable_area): Likewise. > (x86_function_profiler): Replace endbr_queued_at_entrance with > insn_queued_at_entrance. Generate ENDBR only for TYPE_ENDBR. > Call ix86_output_patchable_area to generate patchable area if > needed. > (TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY): New. > * i386.h (queued_insn_type): New. > (machine_function): Add function_label_emitted. Replace > endbr_queued_at_entrance with insn_queued_at_entrance. > * config/i386/i386.md (UNSPECV_PATCHABLE_AREA): New. > (patchable_area): New. > > gcc/testsuite/ > > PR target/93492 > * gcc.target/i386/pr93492-1.c: New test. > * gcc.target/i386/pr93492-2.c: Likewise. > * gcc.target/i386/pr93492-3.c: Likewise. > * gcc.target/i386/pr93492-4.c: Likewise. > * gcc.target/i386/pr93492-5.c: Likewise. OK jeff >
Re: [PATCH] Optimize multiplication for V8QI,V16QI,V32QI under TARGET_AVX512BW [target/95488]
On Fri, 2020-06-05 at 13:46 +0800, Hongtao Liu via Gcc-patches wrote: > Hi: > > +/* Optimize vector MUL generation for V8QI, V16QI and V32QI > + under TARGET_AVX512BW. i.e. for v16qi a * b, it has > + > + vpmovzxbw ymm2, xmm0 > + vpmovzxbw ymm3, xmm1 > + vpmullw ymm4, ymm2, ymm3 > + vpmovwb xmm0, ymm4 > + > + it would take less instructions than ix86_expand_vecop_qihi. > + Return true if success. */ > > Bootstrap is ok, regression test on i386/x86-64 backend is ok. > > gcc/ChangeLog: > PR target/95488 > * config/i386/i386-expand.c (ix86_expand_vecmul_qihi): New > function. > * config/i386/i386-protos.h (ix86_expand_vecmul_qihi): Declare. > * config/i386/sse.md (mul3): Drop mask_name since > there's no real vector char multiplication instruction with > mask. Also optimize it under TARGET_AVX512BW. > (mulv8qi3): New expander. > > gcc/testsuite/ChangeLog: > * gcc.target/i386/avx512bw-pr95488-1.c: New test. > * gcc.target/i386/avx512bw-pr95488-2.c: Ditto. > * gcc.target/i386/avx512vl-pr95488-1.c: Ditto. > * gcc.target/i386/avx512vl-pr95488-2.c: Ditto. > OK jeff
[PATCH, RS6000 PR target/94954] Fix wrong codegen for vec_pack_to_short_fp32() builtin
Hi, Fix codegen implementation for the builtin vec_pack_to_short_fp32. Regtests are underway against powerpc64 (power7be,power8le,power9le). (this is a power9 builtin, so should be a noop for most of those). OK for trunk and backports? Thanks -Will [gcc] target pr/94954 * config/rs6000/altivec.h (vec_pack_to_short_fp32): Update. * config/rs6000/altivec.md (UNSPEC_CONVERT_4F32_8F16): New unspec. (convert_4f32_8f16): New define_expand. * config/rs6000/rs6000-builtin.def (convert_4f32_8f16): New builtin define and overload. * config/rs6000/rs6000-call.c (P9V_BUILTIN_VEC_CONVERT_4F32_8F16): New overloaded builtin entry. * config/rs6000/vsx.md (UNSPEC_VSX_XVCVSPHP): New unspec. (vsx_xvcvsphp): New define_insn. [testsuite] * testsuite/gcc.target/powerpc/builtins-1-p9-runnable.c: Update. diff --git a/gcc/config/rs6000/altivec.h b/gcc/config/rs6000/altivec.h index 0a7e8ab..ab10025 100644 --- a/gcc/config/rs6000/altivec.h +++ b/gcc/config/rs6000/altivec.h @@ -431,11 +431,11 @@ /* Vector additions added in ISA 3.0. */ #define vec_first_match_index __builtin_vec_first_match_index #define vec_first_match_or_eos_index __builtin_vec_first_match_or_eos_index #define vec_first_mismatch_index __builtin_vec_first_mismatch_index #define vec_first_mismatch_or_eos_index __builtin_vec_first_mismatch_or_eos_index -#define vec_pack_to_short_fp32 __builtin_vec_convert_4f32_8i16 +#define vec_pack_to_short_fp32 __builtin_vec_convert_4f32_8f16 #define vec_parity_lsbb __builtin_vec_vparity_lsbb #define vec_vctz __builtin_vec_vctz #define vec_cnttz __builtin_vec_vctz #define vec_vctzb __builtin_vec_vctzb #define vec_vctzd __builtin_vec_vctzd diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md index 159f24e..855e7cc 100644 --- a/gcc/config/rs6000/altivec.md +++ b/gcc/config/rs6000/altivec.md @@ -78,10 +78,11 @@ UNSPEC_VUNPACK_HI_SIGN_DIRECT UNSPEC_VUNPACK_LO_SIGN_DIRECT UNSPEC_VUPKHPX UNSPEC_VUPKLPX UNSPEC_CONVERT_4F32_8I16 + UNSPEC_CONVERT_4F32_8F16 UNSPEC_DST UNSPEC_DSTT UNSPEC_DSTST UNSPEC_DSTSTT UNSPEC_LVSL @@ -3215,10 +3216,32 @@ emit_insn (gen_altivec_vctuxs (rtx_tmp_lo, operands[2], const0_rtx)); emit_insn (gen_altivec_vpkswss (operands[0], rtx_tmp_hi, rtx_tmp_lo)); DONE; }) + +;; Convert two vector F32 to packed vector f16. +(define_expand "convert_4f32_8f16" + [(set (match_operand:V8HI 0 "register_operand" "=v") + (unspec:V8HI [(match_operand:V4SF 1 "register_operand" "v") + (match_operand:V4SF 2 "register_operand" "v")] +UNSPEC_CONVERT_4F32_8F16))] + "TARGET_P9_VECTOR" +{ + rtx rtx_tmp_hi = gen_reg_rtx (V4SImode); + rtx rtx_tmp_lo = gen_reg_rtx (V4SImode); + + emit_insn (gen_vsx_xvcvsphp (rtx_tmp_hi, operands[1] )); + emit_insn (gen_vsx_xvcvsphp (rtx_tmp_lo, operands[2] )); + if (BYTES_BIG_ENDIAN) +emit_insn (gen_altivec_vpkuwum (operands[0], rtx_tmp_lo, rtx_tmp_hi)); + else +emit_insn (gen_altivec_vpkuwum (operands[0], rtx_tmp_hi, rtx_tmp_lo)); + DONE; +}) + + ;; Generate ;;xxlxor/vxor SCRATCH0,SCRATCH0,SCRATCH0 ;;vsubu?m SCRATCH2,SCRATCH1,%1 ;;vmaxs? %0,%1,SCRATCH2" (define_expand "abs2" diff --git a/gcc/config/rs6000/rs6000-builtin.def b/gcc/config/rs6000/rs6000-builtin.def index 8b1ddb0..47e9137 100644 --- a/gcc/config/rs6000/rs6000-builtin.def +++ b/gcc/config/rs6000/rs6000-builtin.def @@ -2208,10 +2208,11 @@ BU_P8V_OVERLOAD_3 (VPERMXOR, "vpermxor") /* ISA 3.0 vector overloaded 2-argument functions. */ BU_P9V_AV_2 (VSLV, "vslv", CONST, vslv) BU_P9V_AV_2 (VSRV, "vsrv", CONST, vsrv) BU_P9V_AV_2 (CONVERT_4F32_8I16, "convert_4f32_8i16", CONST, convert_4f32_8i16) +BU_P9V_AV_2 (CONVERT_4F32_8F16, "convert_4f32_8f16", CONST, convert_4f32_8f16) BU_P9V_AV_2 (VFIRSTMATCHINDEX_V16QI, "first_match_index_v16qi", CONST, first_match_index_v16qi) BU_P9V_AV_2 (VFIRSTMATCHINDEX_V8HI, "first_match_index_v8hi", CONST, first_match_index_v8hi) @@ -2238,10 +2239,11 @@ BU_P9V_AV_2 (VFIRSTMISMATCHOREOSINDEX_V4SI, "first_mismatch_or_eos_index_v4si", /* ISA 3.0 vector overloaded 2-argument functions. */ BU_P9V_OVERLOAD_2 (VSLV, "vslv") BU_P9V_OVERLOAD_2 (VSRV, "vsrv") BU_P9V_OVERLOAD_2 (CONVERT_4F32_8I16, "convert_4f32_8i16") +BU_P9V_OVERLOAD_2 (CONVERT_4F32_8F16, "convert_4f32_8f16") /* 2 argument vector functions added in ISA 3.0 (power9). */ BU_P9V_AV_2 (VADUB,"vadub",CONST, vaduv16qi3) BU_P9V_AV_2 (VADUH,"vaduh",CONST, vaduv8hi3) BU_P9V_AV_2 (VADUW,"vaduw",CONST, vaduv4si3) diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c index 0ac8054..9708d7e 100644 --- a/gcc/config/rs6000/rs6000-call.c +++ b/gcc/config/rs6000
[PATCH] middle-end: Parity folding optimizations.
This patch implements several constant folding optimizations for __builtin_parity and friends. We canonicalize popcount(x)&1 as parity(x) in gimple, and potentially convert back again when we expand to RTL. parity(~x) is simplified to parity(x), which is true for all integer modes with an even number of bits. But probably most usefully, parity(x)^parity(y) can be simplified to a parity(x^y), requiring only a single libcall or popcount. This idiom is seen in PR target/44481 and elsewhere in the Linux kernel. This patch has been tested with "make bootstrap" and "make -k check" on x86_64-pc-linux-gnu with no regressions. If approved, I'd very much appreciate it if someone could commit this change for me. 2020-06-12 Roger Sayle * match.pd (popcount(x)&1 -> parity(x)): New simplification. (parity(~x) -> parity(x)): New simplification. (parity(x&1) -> x&1): New simplification. (parity(x)^parity(y) -> parity(x^y)): New simplification. 2020-06-12 Roger Sayle * gcc.dg/fold-parity-1.c: New test. * gcc.dg/fold-parity-2.c: Likewise. * gcc.dg/fold-parity-3.c: Likewise. * gcc.dg/fold-parity-4.c: Likewise. Thanks in advance, Roger -- Roger Sayle NextMove Software Cambridge, UK diff --git a/gcc/match.pd b/gcc/match.pd index 2b8c37e..ddb0650 100644 --- a/gcc/match.pd +++ b/gcc/match.pd @@ -5949,6 +5949,32 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) (cmp (popcount @0) integer_zerop) (rep @0 { build_zero_cst (TREE_TYPE (@0)); } +/* Canonicalize POPCOUNT(x)&1 as PARITY(X). */ +(for popcount (BUILT_IN_POPCOUNT BUILT_IN_POPCOUNTL BUILT_IN_POPCOUNTLL + BUILT_IN_POPCOUNTIMAX) + parity (BUILT_IN_PARITY BUILT_IN_PARITYL BUILT_IN_PARITYLL +BUILT_IN_PARITYIMAX) + (simplify +(bit_and (popcount @0) integer_onep) +(parity @0))) + +/* PARITY simplifications. */ +(for parity (BUILT_IN_PARITY BUILT_IN_PARITYL BUILT_IN_PARITYLL +BUILT_IN_PARITYIMAX) + /* parity(~X) is parity(X). */ + (simplify +(parity (bit_not @0)) +(parity @0)) + /* parity(X&1) is nop_expr(X&1). */ + (simplify +(parity @0) +(if (tree_nonzero_bits (@0) == 1) + (convert @0))) + /* parity(X)^parity(Y) is parity(X^Y). */ + (simplify +(bit_xor (parity:s @0) (parity:s @1)) +(parity (bit_xor @0 @1 + #if GIMPLE /* 64- and 32-bits branchless implementations of popcount are detected: /* { dg-do compile } */ /* { dg-options "-O2 -fdump-tree-original" } */ int foo(unsigned int x) { return __builtin_popcount(x) & 1; } int fool(unsigned long x) { return __builtin_popcountl(x) & 1; } int fooll(unsigned long long x) { return __builtin_popcountll(x) & 1; } /* { dg-final { scan-tree-dump-times "__builtin_popcount" 0 "original" } } */ /* { dg-final { scan-tree-dump-times "__builtin_parity" 3 "original" } } */ /* { dg-do compile } */ /* { dg-options "-O2 -fdump-tree-optimized" } */ int foo(unsigned int x) { return __builtin_parity(~x); } int fool(unsigned long x) { return __builtin_parityl(~x); } int fooll(unsigned long long x) { return __builtin_parityll(~x); } /* { dg-final { scan-tree-dump-times "~" 0 "optimized" } } */ /* { dg-do compile } */ /* { dg-options "-O2 -fdump-tree-optimized" } */ int foo(unsigned int x) { return __builtin_parity(x&1); } int fool(unsigned long x) { return __builtin_parityl(x&1); } int fooll(unsigned long long x) { return __builtin_parityll(x&1); } /* { dg-final { scan-tree-dump-times "__builtin_parity" 0 "optimized" } } */ /* { dg-do compile } */ /* { dg-options "-O2 -fdump-tree-optimized" } */ int foo(unsigned int x, unsigned int y) { return __builtin_parity(x) ^ __builtin_parity(y); } int fool(unsigned long x, unsigned long y) { return __builtin_parityl(x) ^ __builtin_parityl(y); } int fooll(unsigned long long x, unsigned long long y) { return __builtin_parityll(x) ^ __builtin_parityll(y); } /* { dg-final { scan-tree-dump-times "__builtin_parity" 3 "optimized" } } */
[PATCH PR95199] vect: Remove extra variable created for memory reference
Hi, This is a fix for pr95199. In vectorization, vinfo->ivexpr_map is supposed to catch those "IV base and/or step expressions". Just call cse_and_gimplify_to_preheader to handle gathering/scattering to avoid the extra variable. Bootstrap and tested on aarch64/x86_64 Linux platform. No new regression witnessed. Is it ok? Thanks, Kaipeng Zhou 0001-vect-Remove-extra-variable-created-for-memory-refere.patch Description: 0001-vect-Remove-extra-variable-created-for-memory-refere.patch