Re: [patch] rename local _C2 identifiers in stl map header files

[Olivier Hainque]

> On 9 Jan 2020, at 21:01, Jonathan Wakely  wrote:
> 
>> 
>> 2020-01-09  Olivier Hainque  
>> 
>>  libstdc++-v3/
>>  * doc/xml/manual/appendix_contributing.xml: Document _C2
>>  as a reserved identifier, by VxWorks.
>>  * include/bits/stl_map.h: Rename _C2 template typenames as _Cmp2.
>>  * include/bits/stl_multimap.h: Likewise.
> 
> OK for trunk, thanks!

Great, thanks :-)

[PATCH] Fix PR93216

[Richard Biener]

Committed.

Richard.

2010-01-10  Richard Biener  

PR testsuite/93216
* gcc.dg/optimize-bswaphi-1.c: Split previously added
case into a LE and BE variant.

Index: gcc/testsuite/gcc.dg/optimize-bswaphi-1.c
===
--- gcc/testsuite/gcc.dg/optimize-bswaphi-1.c   (revision 280046)
+++ gcc/testsuite/gcc.dg/optimize-bswaphi-1.c   (working copy)
@@ -55,10 +55,17 @@ swap16 (HItype in)
 }
 
 unsigned short
-get_unaligned_16 (unsigned char *p)
+get_unaligned_16_le (unsigned char *p)
 {
   return p[0] | (p[1] << 8);
 }
 
+unsigned short
+get_unaligned_16_be (unsigned char *p)
+{
+  return p[1] | (p[0] << 8);
+}
+
+
 /* { dg-final { scan-tree-dump-times "16 bit load in target endianness found 
at" 4 "bswap" } } */
-/* { dg-final { scan-tree-dump-times "16 bit bswap implementation found at" 4 
"bswap" } } */
+/* { dg-final { scan-tree-dump-times "16 bit bswap implementation found at" 5 
"bswap" } } */

[PING][PATCH] optimize costly division in rtx_cost

[Alexander Monakov]

Ping.

On Sun, 5 Jan 2020, Alexander Monakov wrote:

> Hi,
> 
> I noticed there's a costly signed 64-bit division in rtx_cost on x86 as well 
> as
> any other target where UNITS_PER_WORD is implemented like TARGET_64BIT ? 8 : 
> 4.
> It's also evident that rtx_cost does redundant work for a SET rtx argument.
> 
> Obviously the variable named 'factor' rarely exceeds 1, so in the majority of
> cases it can be computed with a well-predictable branch rather than a 
> division.
> 
> This patch makes rtx_cost do the division only in case mode is wider than
> UNITS_PER_WORD, and also moves a test for a SET up front to avoid redundancy.
> No functional change.
> 
> Bootstrapped on x86_64, ok for trunk?
> 
> To illustrate the improvement this buys, for tramp3d -O2 compilation, I got
> 
> before:
>73887675319  instructions:u
> 
>72438432200  cycles:u
>  924298569  idq.ms_uops:u
>   102603799255  uops_dispatched.thread:u
> 
> after:
>73888371724  instructions:u
> 
>72386986612  cycles:u
>  802744775  idq.ms_uops:u
>   102096987220  uops_dispatched.thread:u
> 
> (this is on Sandybridge, idq.ms_uops are uops going via the microcode 
> sequencer,
> so the unneeded division is responsible for a good fraction of them)
> 
>   * rtlanal.c (rtx_cost): Handle a SET up front. Avoid division if the
>   mode is not wider than UNITS_PER_WORD.
> 
> diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
> index 9a7afccefb8..c7ab86e228b 100644
> --- a/gcc/rtlanal.c
> +++ b/gcc/rtlanal.c
> @@ -4207,18 +4207,23 @@ rtx_cost (rtx x, machine_mode mode, enum rtx_code 
> outer_code,
>const char *fmt;
>int total;
>int factor;
> +  unsigned mode_size;
>  
>if (x == 0)
>  return 0;
>  
> -  if (GET_MODE (x) != VOIDmode)
> +  if (GET_CODE (x) == SET)
> +/* A SET doesn't have a mode, so let's look at the SET_DEST to get
> +   the mode for the factor.  */
> +mode = GET_MODE (SET_DEST (x));
> +  else if (GET_MODE (x) != VOIDmode)
>  mode = GET_MODE (x);
>  
> +  mode_size = estimated_poly_value (GET_MODE_SIZE (mode));
> +
>/* A size N times larger than UNITS_PER_WORD likely needs N times as
>   many insns, taking N times as long.  */
> -  factor = estimated_poly_value (GET_MODE_SIZE (mode)) / UNITS_PER_WORD;
> -  if (factor == 0)
> -factor = 1;
> +  factor = mode_size > UNITS_PER_WORD ? mode_size / UNITS_PER_WORD : 1;
>  
>/* Compute the default costs of certain things.
>   Note that targetm.rtx_costs can override the defaults.  */
> @@ -4243,14 +4248,6 @@ rtx_cost (rtx x, machine_mode mode, enum rtx_code 
> outer_code,
>/* Used in combine.c as a marker.  */
>total = 0;
>break;
> -case SET:
> -  /* A SET doesn't have a mode, so let's look at the SET_DEST to get
> -  the mode for the factor.  */
> -  mode = GET_MODE (SET_DEST (x));
> -  factor = estimated_poly_value (GET_MODE_SIZE (mode)) / UNITS_PER_WORD;
> -  if (factor == 0)
> - factor = 1;
> -  /* FALLTHRU */
>  default:
>total = factor * COSTS_N_INSNS (1);
>  }
> 

[PATCH] Fix quadraticness in EH sink_clobbers, PR93199

[Richard Biener]

The following patch addresses the quadraticness when sinking
clobbers across a long chain of "empty" EH landing pads one
pad at a time, walking the chain of clobbers that becomes longer
and longer repeatedly.  The idea is simply to record sunk clobbers
off-IL to avoid the need to re-analyze them.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

This shifts the quadraticness elsewhere (empty LP cleanup,
patch for that testing).

OK?

Thanks,
Richard.

2020-01-10  Richard Biener  

PR middle-end/93199
* tree-eh.c (sink_clobbers): Move clobbers to out-of-IL
sequences to avoid walking them again for secondary opportunities.
(pass_lower_eh_dispatch::execute): Instead actually insert
them here.

Index: gcc/tree-eh.c
===
--- gcc/tree-eh.c   (revision 280096)
+++ gcc/tree-eh.c   (working copy)
@@ -3550,11 +3550,15 @@ optimize_clobbers (basic_block bb)
 }
 
 /* Try to sink var = {v} {CLOBBER} stmts followed just by
-   internal throw to successor BB.  If FOUND_OPPORTUNITY is not NULL
-   then do not perform the optimization but set *FOUND_OPPORTUNITY to true.  */
+   internal throw to successor BB.
+   SUNK, if not NULL, is an array of sequences indexed by basic-block
+   index to sink to and to pick up sinking opportunities from.
+   If FOUND_OPPORTUNITY is not NULL then do not perform the optimization
+   but set *FOUND_OPPORTUNITY to true.  */
 
 static int
-sink_clobbers (basic_block bb, bool *found_opportunity = NULL)
+sink_clobbers (basic_block bb,
+  gimple_seq *sunk = NULL, bool *found_opportunity = NULL)
 {
   edge e;
   edge_iterator ei;
@@ -3589,7 +3593,7 @@ sink_clobbers (basic_block bb, bool *fou
return 0;
   any_clobbers = true;
 }
-  if (!any_clobbers)
+  if (!any_clobbers && (!sunk || gimple_seq_empty_p (sunk[bb->index])))
 return 0;
 
   /* If this was a dry run, tell it we found clobbers to sink.  */
@@ -3618,7 +3622,10 @@ sink_clobbers (basic_block bb, bool *fou
 
   gimple *first_sunk = NULL;
   gimple *last_sunk = NULL;
-  dgsi = gsi_after_labels (succbb);
+  if (sunk)
+dgsi = gsi_start (sunk[succbb->index]);
+  else
+dgsi = gsi_after_labels (succbb);
   gsi = gsi_last_bb (bb);
   for (gsi_prev (&gsi); !gsi_end_p (gsi); gsi_prev (&gsi))
 {
@@ -3653,6 +3660,14 @@ sink_clobbers (basic_block bb, bool *fou
first_sunk = stmt;
   last_sunk = stmt;
 }
+  if (sunk && !gimple_seq_empty_p (sunk[bb->index]))
+{
+  if (!first_sunk)
+   first_sunk = gsi_stmt (gsi_last (sunk[bb->index]));
+  last_sunk = gsi_stmt (gsi_start (sunk[bb->index]));
+  gsi_insert_seq_before (&dgsi, sunk[bb->index], GSI_NEW_STMT);
+  sunk[bb->index] = NULL;
+}
   if (first_sunk)
 {
   /* Adjust virtual operands if we sunk across a virtual PHI.  */
@@ -3892,7 +3907,7 @@ pass_lower_eh_dispatch::execute (functio
  if (stmt_can_throw_external (fun, last))
optimize_clobbers (bb);
  else if (!any_resx_to_process)
-   sink_clobbers (bb, &any_resx_to_process);
+   sink_clobbers (bb, NULL, &any_resx_to_process);
}
 }
   if (redirected)
@@ -3908,6 +3923,7 @@ pass_lower_eh_dispatch::execute (functio
 and unreachable block removal.  */
   int *rpo = XNEWVEC  (int, n_basic_blocks_for_fn (fun));
   int rpo_n = pre_and_rev_post_order_compute_fn (fun, NULL, rpo, false);
+  gimple_seq *sunk = XCNEWVEC (gimple_seq, last_basic_block_for_fn (fun));
   for (int i = 0; i < rpo_n; ++i)
{
  bb = BASIC_BLOCK_FOR_FN (fun, rpo[i]);
@@ -3915,9 +3931,17 @@ pass_lower_eh_dispatch::execute (functio
  if (last
  && gimple_code (last) == GIMPLE_RESX
  && !stmt_can_throw_external (fun, last))
-   flags |= sink_clobbers (bb);
+   flags |= sink_clobbers (bb, sunk);
+ /* If there were any clobbers sunk into this BB, insert them now.  */
+ if (!gimple_seq_empty_p (sunk[bb->index]))
+   {
+ gimple_stmt_iterator gsi = gsi_after_labels (bb);
+ gsi_insert_seq_before (&gsi, sunk[bb->index], GSI_NEW_STMT);
+ sunk[bb->index] = NULL;
+   }
}
   free (rpo);
+  free (sunk);
 }
 
   return flags;

Re: [PATCH] Fix quadraticness in EH sink_clobbers, PR93199

[Jakub Jelinek]

On Fri, Jan 10, 2020 at 10:47:32AM +0100, Richard Biener wrote:
> 
> The following patch addresses the quadraticness when sinking
> clobbers across a long chain of "empty" EH landing pads one
> pad at a time, walking the chain of clobbers that becomes longer
> and longer repeatedly.  The idea is simply to record sunk clobbers
> off-IL to avoid the need to re-analyze them.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
> 
> This shifts the quadraticness elsewhere (empty LP cleanup,
> patch for that testing).
> 
> OK?
> 
> Thanks,
> Richard.
> 
> 2020-01-10  Richard Biener  
> 
>   PR middle-end/93199
>   * tree-eh.c (sink_clobbers): Move clobbers to out-of-IL
>   sequences to avoid walking them again for secondary opportunities.
>   (pass_lower_eh_dispatch::execute): Instead actually insert
>   them here.

LGTM, thanks.

Jakub

[PING][PATCH] Fix documentation of -mpoke-function-name ARM option

[Jérémy Lefaure]

Hello,

Ping for https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01081.html.

Thank you,
Jérémy

On Sun, Dec 15, 2019 at 07:20:26PM +0100, Jérémy Lefaure wrote:
> Hi!
> 
> Since in ARM state the value of PC is the address of the current
> instruction plus 8 bytes, the code inspecting the value of PC stored at
> FP + 0 should look at location PC - 16 : PC - 8 points to the stmfd
> instruction, PC - 16 points two words before, where the top 8 bits are
> set.
> 
> gcc/
> 2019-12-14  Jérémy Lefaure 
> 
>   * config/arm/arm.c (-mpoke-function-name): Fix documentation in comment.
>   * doc/invoke.texi (-mpoke-function-name): Fix documentation.
> 

[PATCH] Fix PR93199, quadraticness in cleanup_all_empty_eh

[Richard Biener]

The following manages to avoid high EH indegree of landing pads
during the sequence of cleaning up empty EH with a chain of those.
By walking the landing pads in reverse order we mimic walking of
the EH tree depth-first (which I am too lazy to write...).  It
looks like EH build assures that this is effectively the same.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

OK?

With this the RedHat bugzilla testcase is down to a "flat" profile:

 parser function body   :  14.60 ( 17%)   1.77 ( 31%)  16.37 ( 
18%)  720798 kB ( 23%)
 tree eh:  14.80 ( 17%)   0.06 (  1%)  14.87 ( 
16%)  246315 kB (  8%)
 integrated RA  :  13.08 ( 15%)   0.30 (  5%)  13.40 ( 
15%)  227799 kB (  7%)
 TOTAL  :  85.07  5.68 90.77
3183334 kB

and memory usage is down as well to 4GB (it's actually this very patch
that helps here for reasons I have not investigated - maybe we never
shrink some EH data structures, who knows).

Thanks,
Richard.

2020-01-10  Richard Biener  

PR middle-end/93199
* tree-eh.c (redirect_eh_edge_1): Avoid some work if possible.
(cleanup_all_empty_eh): Walk landing pads in reverse order to
avoid quadraticness.

Index: gcc/tree-eh.c
===
--- gcc/tree-eh.c   (revision 280097)
+++ gcc/tree-eh.c   (working copy)
@@ -2310,7 +2310,7 @@ redirect_eh_edge_1 (edge edge_in, basic_
   old_lp = get_eh_landing_pad_from_number (old_lp_nr);
 
   throw_stmt = last_stmt (edge_in->src);
-  gcc_assert (lookup_stmt_eh_lp (throw_stmt) == old_lp_nr);
+  gcc_checking_assert (lookup_stmt_eh_lp (throw_stmt) == old_lp_nr);
 
   new_label = gimple_block_label (new_bb);
 
@@ -4307,9 +4307,10 @@ cleanup_empty_eh_merge_phis (basic_block
|  | EH
<..>
  which CFG verification would choke on.  See PR45172 and PR51089.  */
-  FOR_EACH_EDGE (e, ei, old_bb->preds)
-if (find_edge (e->src, new_bb))
-  return false;
+  if (!single_pred_p (new_bb))
+FOR_EACH_EDGE (e, ei, old_bb->preds)
+  if (find_edge (e->src, new_bb))
+   return false;
 
   FOR_EACH_EDGE (e, ei, old_bb->preds)
 redirect_edge_var_map_clear (e);
@@ -4698,9 +4699,15 @@ cleanup_all_empty_eh (void)
   eh_landing_pad lp;
   int i;
 
-  for (i = 1; vec_safe_iterate (cfun->eh->lp_array, i, &lp); ++i)
-if (lp)
-  changed |= cleanup_empty_eh (lp);
+  /* Ideally we'd walk the region tree and process LPs inner to outer
+ to avoid quadraticness in EH redirection.  Walking the LP array
+ in reverse seems to be an approximation of that.  */
+  for (i = vec_safe_length (cfun->eh->lp_array) - 1; i >= 1; --i)
+{
+  lp = (*cfun->eh->lp_array)[i];
+  if (lp)
+   changed |= cleanup_empty_eh (lp);
+}
 
   return changed;
 }

Re: [PATCH] Fix PR93199, quadraticness in cleanup_all_empty_eh

[Jakub Jelinek]

On Fri, Jan 10, 2020 at 11:23:34AM +0100, Richard Biener wrote:
> 
> The following manages to avoid high EH indegree of landing pads
> during the sequence of cleaning up empty EH with a chain of those.
> By walking the landing pads in reverse order we mimic walking of
> the EH tree depth-first (which I am too lazy to write...).  It
> looks like EH build assures that this is effectively the same.
> 
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> 
> OK?
> 
> With this the RedHat bugzilla testcase is down to a "flat" profile:
> 
>  parser function body   :  14.60 ( 17%)   1.77 ( 31%)  16.37 ( 
> 18%)  720798 kB ( 23%)
>  tree eh:  14.80 ( 17%)   0.06 (  1%)  14.87 ( 
> 16%)  246315 kB (  8%)
>  integrated RA  :  13.08 ( 15%)   0.30 (  5%)  13.40 ( 
> 15%)  227799 kB (  7%)
>  TOTAL  :  85.07  5.68 90.77  
>   
> 3183334 kB
> 
> and memory usage is down as well to 4GB (it's actually this very patch
> that helps here for reasons I have not investigated - maybe we never
> shrink some EH data structures, who knows).
> 
> Thanks,
> Richard.
> 
> 2020-01-10  Richard Biener  
> 
>   PR middle-end/93199
>   * tree-eh.c (redirect_eh_edge_1): Avoid some work if possible.
>   (cleanup_all_empty_eh): Walk landing pads in reverse order to
>   avoid quadraticness.

LGTM.

Jakub

Re: [wwwdocs] Git transition - how to access private user and vendor branches

[Richard Earnshaw (lists)]

On 09/01/2020 16:50, Richard Earnshaw (lists) wrote:
Given the proposed intention to use non-standard refspecs for private 
and vendor branches I've written some notes on how to use these.


It would be helpful if someone could do some experiments to ensure that 
what I've written works properly for all versions of git, not just the 
one I have installed locally.


R.


A minor tweak after testing it myself pushing to the 
gcc-reposurgeon-8.git trial



R.
diff --git a/htdocs/svnwrite.html b/htdocs/svnwrite.html
index a1346be1..1b06c495 100644
--- a/htdocs/svnwrite.html
+++ b/htdocs/svnwrite.html
@@ -25,6 +25,7 @@ maintainers and significant developers.
   Checking in a change
   Example check-in session
   Creating and using branches
+  Private user and Vendor branches
   Tips&Tricks around your account
 
 
@@ -407,6 +408,73 @@ svn cp svn+ssh://username@gcc.gnu.org/svn/gcc/trunk \
 do not copy the entire set of ChangeLog entries.  Just use something
 like "Merge from mainline" or similar.
 
+
+Private user and vendor branches
+
+The GCC git repository is used by many people and the branch and tag
+namespace would become very polluted if all branches lived at the
+top-level naming space.  To help minimise the amount of data that
+needs to be fetched the git repository on gcc.gnu.org contains a
+number of private user and vendor branches that developers use to
+share their work.  These are not pulled by default, but simple
+configuration steps can give access to them.
+
+
+  Private user branches live
+in refs/users/username/heads with tags
+in refs/users/username/tags.
+  Vendor branches live
+in refs/vendors/vendor-name/heads with tags
+in refs/vendors/vendor-name/tags.
+
+
+To fetch any of these you will need to add a fetch refspec to your
+configuration.  For example, to fetch all the IBM vendor branches add to
+your default upstream pull
+
+
+git config --add remote.origin.fetch "+refs/vendors/IBM/heads/*:refs/remotes/origin/IBM/*"
+git config --add remote.origin.fetch "+refs/vendors/IBM/tags/*:refs/tags/IBM/*"
+
+
+this will cause git pull to fetch all the additional
+branches and make them available locally
+under remotes/origin/IBM and will also add any tags under
+the sub-namespace IBM.
+
+Setting up a tracking branch for one of the upstream vendor branches
+is slightly more complicated as git branch
+--set-upstream-to does not work properly.  However, it is
+possible to configure the branch information directly.  First, check
+out the branch you want to track, for example, to check out the
+arm-9-branch use something like:
+
+
+git checkout -b ARM/arm-9-branch origin/ARM/arm-9-branch
+
+
+then change the merge property for the branch to corectly identify the
+upstream source
+
+
+git config branch.ARM/arm-9-branch.merge refs/vendors/ARM/heads/arm-9-branch
+git config branch.ARM/arm-9-branch.remote origin
+
+
+Pull operations will now correctly track the upstream branch.
+
+It is also possible to set up push operations so that local changes will be pushed to the private namespace.  For example, if you mirror your own private git information with
+
+
+git config --add remote.origin.fetch "+refs/users/my-userid/heads/*:refs/remotes/origin/me/*"
+
+
+then you can also add
+
+git config --add remote.origin.push "+refs/heads/me/*:refs/users/my-userid/heads/*"
+
+and then any push from a branch that begins with me/ will be pushed to the private area.
+
 
 Tips&Tricks around your account
 

Re: [PATCH v7] Missed function specialization + partial devirtualization

[Jan Hubicka]

> > I think you are also not removing the common_target and
> > common_target_probability from cgraph_edge.
> > There is now code in ipa-utils merging the histograms. You will need to
> > update that to your representation. It should not be hard - it either
> > copies all the values from target function or merges them with given
> > probability scales.
> 
> This seems a bit tricky for me.  ipa-icf runs after ipa-profile, the reason 
> why 
> remove  is they are never used in
> later passes during previous discussion.

There is also merging done from lto-symtab.  If you have COMDAT inline
which is late inlined at -fprofile-generate time you will end up with
multiple profiles for it and they needs to be merged.  This is done
before IPA passes including ipa-profile.
> 
> I added a new variable target_prob to save it in direct edge when calling
> make_speculative, since the target_prob is assigned in ipa-profile, so no
> need to stream out and in through LTO like speculative_id.
> 
> Even though, scaling in ipa-icf with multiple direct edges is more complicate 
> then
> single direct edge, need double circulation to find each src to dst edge map 
> and do
> scaling one by one. 
> 
> BTW, the breaking patch r279373 "Fix merging of common traget info." only 
> mentioned
> firefox but doesn't include a testcase, I tried to construct some test
> cases, but fail to cover all the circumstance (I just implemented the N:N map 
> yet, 
> please refer to the code in below patch.), do you have cases other than 
> firefox to
> cover it?  Or how to test it as r279373 required?  As this piece of code is 
> not quite
> related to this patch, can we just leave it for future refine?  Thanks :)

You need testcase with a comdat that is used & inlined in two units,
contains indirect call and build with -fno-early-inlining.

However we can handle this incrementally.

>   2019-12-26  Xiong Hu Luo  
> 
>   PR ipa/69678
>   * cgraph.c (symbol_table::create_edge): Init speculative_id and
>   target_prob.
>   (cgraph_edge::make_speculative): Add param for setting speculative_id
>   and target_prob.
>   (cgraph_edge::speculative_call_info): Update comments and find reference
>   by speculative_id for multiple indirect targets.
>   (cgraph_edge::resolve_speculation): Decrease the speculations
>   for indirect edge, drop it's speculative if not direct target
>   left. Update comments.
>   (cgraph_edge::redirect_call_stmt_to_callee): Likewise.
>   (cgraph_node::dump): Print num_speculative_call_targets.
>   (cgraph_node::verify_node): Don't report error if speculative
>   edge not include statement.
>   (cgraph_edge::num_speculative_call_targets_p): New function.
>   * cgraph.h (int common_target_id): Remove.
>   (int common_target_probability): Remove.
>   (num_speculative_call_targets): New variable.
>   (make_speculative): Add param for setting speculative_id.
>   (cgraph_edge::num_speculative_call_targets_p): New declare.
>   (target_prob): New variable.
>   (speculative_id): New variable.
>   * ipa-fnsummary.c (analyze_function_body): Create and duplicate
> call summaries for multiple speculative call targets.
>   * cgraphclones.c (cgraph_node::create_clone): Clone speculative_id.
>   * ipa-profile.c (struct speculative_call_target): New struct.
>   (class speculative_call_summary): New class.
>   (class speculative_call_summaries): New class.
>   (call_sums): New variable.
>   (ipa_profile_generate_summary): Generate indirect multiple targets 
> summaries.
>   (ipa_profile_write_edge_summary): New function.
>   (ipa_profile_write_summary): Stream out indirect multiple targets 
> summaries.
>   (ipa_profile_dump_all_summaries): New function.
>   (ipa_profile_read_edge_summary): New function.
>   (ipa_profile_read_summary_section): New function.
>   (ipa_profile_read_summary): Stream in indirect multiple targets 
> summaries.
>   (ipa_profile): Generate num_speculative_call_targets from
>   profile summaries.
>   * ipa-ref.h (speculative_id): New variable.
>   * ipa-utils.c (ipa_merge_profiles): Update with target_prob.
>   * lto-cgraph.c (lto_output_edge): Remove indirect common_target_id and
>   common_target_probability.   Stream out speculative_id and
>   num_speculative_call_targets.
>   (input_edge): Likewise.
>   * predict.c (dump_prediction): Remove edges count assert to be
>   precise.
>   * symtab.c (symtab_node::create_reference): Init speculative_id.
>   (symtab_node::clone_references): Clone speculative_id.
>   (symtab_node::clone_referring): Clone speculative_id.
>   (symtab_node::clone_reference): Clone speculative_id.
>   (symtab_node::clear_stmts_in_references): Clear speculative_id.
>   * tree-inline.c (copy_bb): Duplicate all the speculative edges
>   if indirect call contains multiple speculative tar

Re: [PATCH] Relax invalidation of TOP N counters in PGO.

[Martin Liška]

On 1/9/20 10:51 AM, Jan Hubicka wrote:

On 1/8/20 3:05 PM, Jan Hubicka wrote:




I would still preffer invalidation before streaming (which is fully
deterministic) and possibly have option


Do you mean __gcov_merge_topn?


I suggest we do the following:

   - have non-deterministic and deterministic version of TOPN counter
 and a flag chosing between deterministic and non-deterministic
 instrumentation.
   - in non-deterministic version do merging dropping least frequent
 values as done with your patch.


Good.


   - in deterministic version do the following

 in __gcov_merge_topn
 1) prune out all values in measured counts with associated counter
less than total_count/N (where N is number of values we track)


Can you please explain me how precisely is this prune useful?


The motivation is to keep everything deterministic.  At the end of run
we have deterministic set of counters with deterministic values and
after this step we again have deterministic set, just smaller (since we
removed useless ones).

The the idea was to produce union of all the sets from all the runs
(invalidating ocunter at the overflow) which is again deterministic by
distributivity of union operation, so it does not matter in what order
we perform it.


Yes.




 2) prune out all values in read counter same way
 3) merge the values ignoring any with resulting counter being
less than merged_total_count/N and invalidating whole counter if
there are too many suriving ones
 4) before using profiles in GCC prune out all vlaues with associated
 counter less than total_count/N (they are not useful anyway).


However it seems that I missed a problem here. With step 2 and 4 the
merge is not distributive.

I added 2 and 4 trying to work around the fact that we have no
convenient place to do pruning if merging is not performed (since merge
functions are not called at all).  I think we need to invent such place
- just do that on the begginign of dump_one_gcov.


Hm, anyway, stage3 closes today and it's definitely next stage1 material.



 Here total_count can be simply sum of all N counters, it would be
 better if it was total number of runs, but that is not available at
 gcov_merge time unless we introduce extra counter for misses.

 Observation is that since we never kick out value because counter is
 full (but still rather invalidate whole counter) this remains
 deterministics. Hopefully prunning out useless small values will
 prevent the Firefox-like examples from triggering too often.
 Definitly this is stronger than simply invalidating all counters
 where number of runs != sum of all counts which is other
 deterministics variant.

 2) and 4) is only needed since we currently have no convenient place
 to prune counters prior streaming if merging is not done at all.
 If we invent one we could skip that step.


Does it make sense to you?

I was also wondering: with TOPN it would be nice to have the property
that target with at greater 1/(TOPN+1) probability gets into the counter,
but this guaranteed only for N=1.
For N>1 one can populate N-1 counters with not too important values
and only then start putting in frequent values be sure that they fight
and be sure that the frequent target is always fight between themself
for the last slot (exploiting the "lets decrease the least counter
heuristics")

Like for N=3

X X Y Y Z W Z W Z W Z W  ..

here first two counters will get occupied by X and Y with counts 2 and
the third counter will end up with W. Z will be missed even if it has
limit 0.5 and both X and Y probability 0 in the limit.

What about increasing value of every couner by N on hit and decreasing
all by 1 on miss?  It will make sure that counts having frequency less
than 1/N will be dropped making space for those with higher frequencies.


I like this approach. It looks smart. So then we'll divide all counter values
by N, right?


Yes.


Note that we used to have the Google's profiling code for TOP N:
https://github.com/gcc-mirror/gcc/blob/gcc-7-branch/libgcc/libgcov-profiler.c#L179


It seems to not implement the logic to decrease counts on mismatch.
Instead if counter is full the value is entered to the least frequently
used entry.  This clearly can degreade.  For N=3 one can do
X X Y Y Z W Z W Z W Z W ..
where Z and W will keep kicking off each other.

So they seem to wait when this grows to 3000 and take low values out of
histogram.

I do not see much advantage in this scheme except that the counts are
not decreased and this they more closely corresponds to actual
frequencies of the target (still not precisely since the target might
have been kicked out earlier).


Yep, I just wanted to mention that approach ;) Not saying it's the best one.

Martin



Honza


It's somehow counting number of evictions and if it reaches a threshold, then 
low
counter values are evicted.

Martin


This should also improve behaviour W

Re: [PATCH] Relax invalidation of TOP N counters in PGO.

[Jan Hubicka]

> > > >  counter less than total_count/N (they are not useful anyway).
> > 
> > However it seems that I missed a problem here. With step 2 and 4 the
> > merge is not distributive.
> > 
> > I added 2 and 4 trying to work around the fact that we have no
> > convenient place to do pruning if merging is not performed (since merge
> > functions are not called at all).  I think we need to invent such place
> > - just do that on the begginign of dump_one_gcov.
> 
> Hm, anyway, stage3 closes today and it's definitely next stage1 material.

It is a regression, so we can still fix it in stage4 :)
Honza

[PATCH][wwwdocs] GCC 10 changes.html for arm and aarch64

[Kyrill Tkachov]

Hi all,

This patch adds initial entries for notable features that went in to GCC 
10 on the arm and aarch64 front.
The list is by no means complete so if you'd like your contribution 
called please shout or post a follow-up patch.

It is, nevertheless, a decent start at the relevant sections in changes.html

Thanks,
Kyrill

commit b539d38b322883ed5aa6563ac879af6a5ebabd96
Author: Kyrylo Tkachov 
Date:   Thu Nov 7 17:58:45 2019 +

[arm/aarch64] GCC 10 changes.html

diff --git a/htdocs/gcc-10/changes.html b/htdocs/gcc-10/changes.html
index d6108269..8f498017 100644
--- a/htdocs/gcc-10/changes.html
+++ b/htdocs/gcc-10/changes.html
@@ -322,17 +322,102 @@ a work-in-progress.
 
 New Targets and Target Specific Improvements
 
-
+AArch64 & arm
+
+  The AArch64 and arm ports now support condition flag output constraints
+  in inline assembly, as indicated by the __GCC_ASM_FLAG_OUTPUTS__.
+  On arm this feature is only available for A32 and T32 targets.
+  Please refer to the documentation for more details. 
+
+
+AArch64
+
+   The -mbranch-protection=pac-ret option now accepts the
+  optional argument +b-key extension to perform return address
+  signing with the B-key instead of the A-key.
+  
+  The Transactional Memory Extension is now supported through ACLE
+  intrinsics.  It can be enabled through the +tme option
+  extension (for example, -march=armv8.5-a+tme).
+  
+  Initial autovectorization support for SVE2 has been added and can be
+  enabled through the   +sve2 option extension (for example,
+  -march=armv8.5-a+sve2).  Additional extensions can be enabled
+  through +sve2-sm4, +sve2=aes,
+  +sve2-sha3, +sve2-bitperm.
+  
+   A number of features from the Armv8.5-a are now supported through ACLE
+  intrinsics.  These include:
+
+	The random number instructions that can be enabled
+	through the (already present in GCC 9.1) +rng option
+	extension.
+	Floating-point intrinsics to round to integer instructions from
+	Armv8.5-a when targeting -march=armv8.5-a or later.
+	Memory Tagging Extension intrinsics enabled through the
+	+memtag option extension.
+
+  
+   The option -moutline-atomics has been added to aid
+  deployment of the Large System Extensions (LSE) on GNU/Linux systems built
+  with a baseline architecture targeting Armv8-A.  When the option is
+  specified code is emitted to detect the presence of LSE instructions at
+  runtime and use them for standard atomic operations.
+  For more information please refer to the documentation.
+  
+  
+   Support has been added for the following processors
+   (GCC identifiers in parentheses):
+   
+ Arm Cortex-A77 (cortex-a77).
+	 Arm Cortex-A76AE (cortex-a76ae).
+	 Arm Cortex-A65 (cortex-a65).
+	 Arm Cortex-A65AE (cortex-a65ae).
+	 Arm Cortex-A34 (cortex-a34).
+   
+   The GCC identifiers can be used
+   as arguments to the -mcpu or -mtune options,
+   for example: -mcpu=cortex-a77 or
+   -mtune=cortex-a65ae or as arguments to the equivalent target
+   attributes and pragmas.
+  
+
 
 
 
-ARM
+arm
 
   Support for the FDPIC ABI has been added. It uses 64-bit
   function descriptors to represent pointers to functions, and enables
   code sharing on MMU-less systems. The corresponding target triple is
   arm-uclinuxfdpiceabi, and the C library is uclibc-ng.
   
+  Support has been added for the Arm EABI on NetBSD through the
+  arm*-*-netbsdelf-*eabi* triplet.
+  
+  The handling of 64-bit integer operations has been significantly reworked
+  and improved leading to improved performance and reduced stack usage when using
+  64-bit integral data types.  The option -mneon-for-64bits is now
+  deprecated and will be removed in a future release.
+  
+   Support has been added for the following processors
+   (GCC identifiers in parentheses):
+   
+ Arm Cortex-A77 (cortex-a77).
+	 Arm Cortex-A76AE (cortex-a76ae).
+	 Arm Cortex-M35P (cortex-m35p).
+   
+   The GCC identifiers can be used
+   as arguments to the -mcpu or -mtune options,
+   for example: -mcpu=cortex-a77 or
+   -mtune=cortex-m35p.
+  
+  Support has been extended for the ACLE
+  https://developer.arm.com/docs/101028/0009/data-processing-intrinsics";>
+  data-processing intrinsics to include 32-bit SIMD, saturating arithmetic,
+  16-bit multiplication and other related intrinsics aimed at DSP algorithm
+  optimization.
+   
 
 
 

Re: [ PATCH ] [ C++ ] Implementing P0767 - deprecate POD

[Christophe Lyon]

Hi,

On Thu, 9 Jan 2020 at 22:21, Jonathan Wakely  wrote:
>
> On 09/01/20 19:57 +, Jonathan Wakely wrote:
> >I'll commit the attached patch after more testing.
>
> And this follow-up to fix some fallout.
>

I have noticed:
FAIL: g++:g++.dg/cpp0x/std-layout1.C  -std=c++2a (test for excess errors)

was that follow-up intended to fix this too?

[PATCH][OBVIOUS] Fix wrong parenthesis in inliner.

[Martin Liška]

Hi.

This restores parenthesis to before r280040.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
And fixes the issue for ppc64le-linux-gnu.

Thanks,
Martin

gcc/ChangeLog:

2020-01-10  Martin Liska  

PR ipa/93217
* ipa-inline-analysis.c (offline_size): Make proper parenthesis
encapsulation that was there before r280040.
---
 gcc/ipa-inline-analysis.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/gcc/ipa-inline-analysis.c b/gcc/ipa-inline-analysis.c
index 3c71aab4ecb..148efbc09ef 100644
--- a/gcc/ipa-inline-analysis.c
+++ b/gcc/ipa-inline-analysis.c
@@ -457,7 +457,7 @@ offline_size (struct cgraph_node *node, ipa_size_summary *info)
 	   && node->can_remove_if_no_direct_calls_p ())
 	{
 	  int prob = opt_for_fn (node->decl, param_comdat_sharing_probability);
-	  return info->size * (100 - prob + 50) / 100;
+	  return (info->size * (100 - prob) + 50) / 100;
 	}
 }
   return 0;

[wwwdocs] Add AVR news.

[Georg-Johann Lay]

Added the following change to the v10 changes site.

Johann


diff --git a/htdocs/gcc-10/changes.html b/htdocs/gcc-10/changes.html
index d6108269..7d96bc66 100644
--- a/htdocs/gcc-10/changes.html
+++ b/htdocs/gcc-10/changes.html
@@ -334,7 +334,54 @@ a work-in-progress.
   arm-uclinuxfdpiceabi, and the C library is uclibc-ng.
   
 
-
+
+AVR
+
+  Support for the XMEGA-like devices
+
+  ATtiny202, ATtiny204, ATtiny402, ATtiny404, ATtiny406, ATtiny804,
+  ATtiny806, ATtiny807, ATtiny1604, ATtiny1606, ATtiny1607, ATmega808,
+  ATmega809, ATmega1608, ATmega1609, ATmega3208, ATmega3209,
+  ATmega4808, ATmega4809
+
+has been added.
+  
+  
+A new command line option -nodevicespecs has been added.
+It allows to provide a custom device-specs file by means of
+
+   avr-gcc -nodevicespecs -specs=my-spec-file 
+
+and without the need to provide options -B and
+-mmcu=.
+See
+href="https://gcc.gnu.org/onlinedocs/gcc/AVR-Options.html#index-nodevicespecs";>AVR

+  command line options for details.
+This feature is also available in v9.3+ and v8.4+.
+  
+  
+New command line options -mdouble=[32,64] and
+-mlong-double=[32,64] have been added.  They allow
+to chose the size (in bits) of the double and
+long double types, respectively.  Whether or not the
+mentioned layouts are available, whether the options act
+as a multilib option, and what is the default for either option
+is controlled by the new
+https://gcc.gnu.org/install/configure.html#avr";>AVR configure
+  options
+--with-double= and --with-long-double=.
+  
+  
+A new configure option --with-libf7= has been added.
+It controls to which level avr-libgcc provides 64-bit floating point
+support by means of
+https://gcc.gnu.org/wiki/avr-gcc#LibF7";>LibF7.
+  
+  
+A new configure option --with-double-comparison= has been
+added. It's unlikely you need to set this option by hand.
+  
+

 

[PATCH] Micro-optimize store-merging terminate-all-aliasing-chains

[Richard Biener]

This caches alias info avoiding repeated (expensive) 
get_ref_base_and_extent.  It doesn't address the unlimited quadraticness
in this function the PR93199 testcase runs into.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Richard.

2020-01-10  Richard Biener  

* gimple-ssa-store-merging
(pass_store_merging::terminate_all_aliasing_chains): Cache alias info.

Index: gcc/gimple-ssa-store-merging.c
===
--- gcc/gimple-ssa-store-merging.c  (revision 280096)
+++ gcc/gimple-ssa-store-merging.c  (working copy)
@@ -2219,6 +2219,8 @@ pass_store_merging::terminate_all_aliasi
 return false;
 
   tree store_lhs = gimple_store_p (stmt) ? gimple_get_lhs (stmt) : NULL_TREE;
+  ao_ref store_lhs_ref;
+  ao_ref_init (&store_lhs_ref, store_lhs);
   for (imm_store_chain_info *next = m_stores_head, *cur = next; cur; cur = 
next)
 {
   next = cur->next;
@@ -2233,9 +2235,12 @@ pass_store_merging::terminate_all_aliasi
   FOR_EACH_VEC_ELT (cur->m_store_info, i, info)
{
  tree lhs = gimple_assign_lhs (info->stmt);
- if (ref_maybe_used_by_stmt_p (stmt, lhs)
- || stmt_may_clobber_ref_p (stmt, lhs)
- || (store_lhs && refs_output_dependent_p (store_lhs, lhs)))
+ ao_ref lhs_ref;
+ ao_ref_init (&lhs_ref, lhs);
+ if (ref_maybe_used_by_stmt_p (stmt, &lhs_ref)
+ || stmt_may_clobber_ref_p_1 (stmt, &lhs_ref)
+ || (store_lhs && refs_may_alias_p_1 (&store_lhs_ref,
+  &lhs_ref, false)))
{
  if (dump_file && (dump_flags & TDF_DETAILS))
{

Fix gather/scatter check when updating a vector epilogue loop

[Richard Sandiford]

update_epilogue_loop_vinfo applies SSA renmaing to the DR_REF of a
gather or scatter, so that vect_check_gather_scatter continues to work.
However, we sometimes also rely on vect_check_gather_scatter when
using gathers and scatters to implement strided accesses.

This showed up on existing tests when testing with fixed-length
-msve-vector-bits=128.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  Maybe verging
on the obvious, but: OK to install?

Richard


2020-01-10  Richard Sandiford  

gcc/
* tree-vect-loop.c (update_epilogue_loop_vinfo): Update DR_REF
for any type of gather or scatter, including strided accesses.

Index: gcc/tree-vect-loop.c
===
--- gcc/tree-vect-loop.c2020-01-06 12:58:08.877844402 +
+++ gcc/tree-vect-loop.c2020-01-10 12:37:58.683829420 +
@@ -8458,7 +8458,7 @@ update_epilogue_loop_vinfo (class loop *
 updated offset we set using ADVANCE.  Instead we have to make sure the
 reference in the data references point to the corresponding copy of
 the original in the epilogue.  */
-  if (STMT_VINFO_GATHER_SCATTER_P (stmt_vinfo))
+  if (STMT_VINFO_MEMORY_ACCESS_TYPE (stmt_vinfo) == VMAT_GATHER_SCATTER)
{
  DR_REF (dr)
= simplify_replace_tree (DR_REF (dr), NULL_TREE, NULL_TREE,

Use get_related_vectype_for_scalar_type for reduction indices

[Richard Sandiford]

The related_vector_mode series missed this case in
vect_create_epilog_for_reduction, where we want to create the
unsigned integer equivalent of another vector.  Without it we
could mix SVE and Advanced SIMD vectors in the same operation.

This showed up on existing tests when testing with fixed-length
-msve-vector-bits=128.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Richard


2020-01-10  Richard Sandiford  

gcc/
* tree-vect-loop.c (vect_create_epilog_for_reduction): Use
get_related_vectype_for_scalar_type rather than build_vector_type
to create the index type for a conditional reduction.

Index: gcc/tree-vect-loop.c
===
--- gcc/tree-vect-loop.c2020-01-10 12:37:58.683829420 +
+++ gcc/tree-vect-loop.c2020-01-10 12:44:20.021356977 +
@@ -4566,8 +4566,9 @@ vect_create_epilog_for_reduction (stmt_v
   int scalar_precision
= GET_MODE_PRECISION (SCALAR_TYPE_MODE (TREE_TYPE (vectype)));
   tree cr_index_scalar_type = make_unsigned_type (scalar_precision);
-  tree cr_index_vector_type = build_vector_type
-   (cr_index_scalar_type, TYPE_VECTOR_SUBPARTS (vectype));
+  tree cr_index_vector_type = get_related_vectype_for_scalar_type
+   (TYPE_MODE (vectype), cr_index_scalar_type,
+TYPE_VECTOR_SUBPARTS (vectype));
 
   /* First we create a simple vector induction variable which starts
 with the values {1,2,3,...} (SERIES_VECT) and increments by the

[AArch64] Require aarch64_sve256_hw for a 256-bit SVE test

[Richard Sandiford]

One of the SVE run tests was specific to 256-bit SVE but tried to
run for all SVE lengths.

Tested on aarch64-linux-gnu and applied as r280104.

Richard


2020-01-10  Richard Sandiford  

gcc/testsuite/
* gcc.target/aarch64/sve/index_1_run.c: Require aarch64_sve256_hw
rather than aarch64_sve_hw.

Index: gcc/testsuite/gcc.target/aarch64/sve/index_1_run.c
===
--- gcc/testsuite/gcc.target/aarch64/sve/index_1_run.c  2019-03-08 
18:14:29.788994704 +
+++ gcc/testsuite/gcc.target/aarch64/sve/index_1_run.c  2020-01-10 
12:49:32.627330534 +
@@ -1,4 +1,4 @@
-/* { dg-do run { target aarch64_sve_hw } } */
+/* { dg-do run { target aarch64_sve256_hw } } */
 /* { dg-options "-O2 -ftree-vectorize -msve-vector-bits=256" } */
 
 #include "index_1.c"

Re: [PATCH] Fix a bug that propagation in recursive function uses wrong aggregate lattice (PR ipa/93084)

[Martin Jambor]

Hi,

On Thu, Jan 09 2020, Christophe Lyon wrote:
> On Tue, 7 Jan 2020 at 14:18, Martin Jambor  wrote:
>>
>> Hi,
>>
>> On Fri, Jan 03 2020, Feng Xue OS wrote:
>> > When checking a self-recursively generated value for aggregate jump
>> > function, wrong aggregate lattice was used, which will cause infinite
>> > constant propagation. This patch is composed to fix this issue.
>> >
>> > 2020-01-03  Feng Xue  
>> >
>> > PR ipa/93084
>> > * ipa-cp.c (self_recursively_generated_p): Find matched aggregate
>> > lattice for a value to check.
>> > (propagate_vals_across_arith_jfunc): Add an assertion to ensure
>> > finite propagation in self-recursive scc.
>>
>> as far as I am concerned, the patch looks good.
>
> Hi,
> The new test introduced by this patch (gcc.dg/ipa/ipa-clone-3.c) fails
> on arm, powerpc and mips according to gcc-testresults.
> FAIL: gcc.dg/ipa/ipa-clone-3.c scan-ipa-dump-times cp "Creating a
> specialized node of recur_fn/[0-9]*\\." 8
>
> Can you have a look?
>

thank you, next time please also consider filing a bug with the exact
architecture triplets/quadruplets where it fails.  I started with a look
at aarch64-suse-linux and then at ppc64le-redhat-linux but apparently
those were not the arms and powerpcs I was looking for.

After building a cross-compiler for armv8l-unknown-linux-gnueabihf I can
see the failure, aggregate jump functions do not get built for:

  main ()
  { 
struct V v;
  
 [local count: 1073741824]:
v = *.LC0;
_4 = recur_fn (&v);
return _4;
  }

which is how the release_ssa dump looks like for this target, even
though it might actually be simpler than for the supported and more
common form:

  main ()
  { 
struct V v;
  
 [local count: 1073741824]:
v.f0 = 1;
v.f1 = 3;
_5 = recur_fn (&v);
return _5;
  }

The different form stems from the fact that can_move_by_pieces returns
false in gimplify_init_constructor.  I'll put this on my TODO list.

Meanwhile, I' about to commit this which should fix the testcase for now
after lightly testing it with make -k check-gcc RUNTESTFLAGS="ipa.exp".

Thanks,

Martin


2020-01-10  Martin Jambor  

* gcc.dg/ipa/ipa-clone-3.c: Replace struct initializer with
piecemeal initialization.
---
 gcc/testsuite/gcc.dg/ipa/ipa-clone-3.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-clone-3.c 
b/gcc/testsuite/gcc.dg/ipa/ipa-clone-3.c
index 18d29bdd0b3..a73cb8b63fc 100644
--- a/gcc/testsuite/gcc.dg/ipa/ipa-clone-3.c
+++ b/gcc/testsuite/gcc.dg/ipa/ipa-clone-3.c
@@ -34,8 +34,10 @@ int recur_fn (struct V * __restrict v)
 
 int main ()
 {
-  struct V v = {1, 3};
+  struct V v;
 
+  v.f0 = 1;
+  v.f1 = 3;
   return recur_fn (&v);
 }
 
-- 
2.24.1

Some local customization enhancements when using git

[Richard Earnshaw (lists)]

This patch is intended to help with folks setting up a git work 
environment for use with GCC following the transition to git.  It 
currently does a couple of things.


1) Add an alias 'svn-rev' to git so that you can look up a legacy commit 
by its svn revision number.  This enables you to type

git svn-rev 1234
and git will show the commit log entry relating to SVN r1234.

2) Sets up tracking information for the user's private name area in the 
git repo.  It tries to figure out some sensible answers to the data it 
needs, but allows the user to override the values.  It then creates the 
fetch and push entries that are needed for tracking the extra refs. 
This implements one part of the recommendations that I've proposed in 
svnwrite.html for dealing with private branches.


It should be possible to run the script more than once and for it to 
DTRT.  If you change your answers the configuration should be correctly 
updated.


2020-01-10  Richard Earnshaw  

* gcc-git-customization: New file.

diff --git a/contrib/gcc-git-customization b/contrib/gcc-git-customization
new file mode 100755
index 000..7f1a13bdf79
--- /dev/null
+++ b/contrib/gcc-git-customization
@@ -0,0 +1,54 @@
+#!/bin/sh
+
+# Script to add some local git customizations suitable for working
+# with the GCC git repository
+
+ask () {
+question=$1
+default=$2
+var=$3
+echo -n $question "["$default"]? "
+read answer
+if [ "x$answer" = "x" ]
+then
+	eval $var=$default
+else
+	eval $var=$answer
+fi
+}
+
+# Add a git command to find the git commit equivalent to legacy SVN revision NNN
+git config alias.svn-rev '!f() { rev=$1; shift; git log --all --grep="From-SVN: r$rev\\b" "${@}"; } ; f'
+
+upstream=`git config --get "gcc-config.upstream"`
+if [ "x$upstream" = "x" ]
+then
+upstream="origin"
+fi
+ask "Local name for upstream repository" "origin" upstream
+git config "gcc-config.upstream" "$upstream"
+
+remote_id=`git config --get "gcc-config.user"`
+if [ "x$remote_id" = "x" ]
+then
+# See if the url specifies the remote user name.
+url=`git config --get "remote.$upstream.url"`
+if [ "x$url" = "x" ]
+then
+	# This is a pure guess, but for many people it might be OK.
+	remote_id=`whoami`
+else
+	remote_id=`echo $url | sed -r "s|^.*ssh://(.+)@gcc.gnu.org.*$|\1|"`
+	if [ x$remote_id = x$url ]
+	then
+	remote_id=`whoami`
+	fi
+fi
+fi
+ask "Account name on gcc.gnu.org" $remote_id remote_id
+git config "gcc-config.user" "$remote_id"
+
+echo "Setting up tracking for private namespace $remote_id in remotes/$upstream/me"
+git config --replace-all "remote.${upstream}.fetch" "+refs/users/${remote_id}/heads/*:refs/remotes/${upstream}/me/*" ":refs/remotes/${upstream}/me/"
+git config --replace-all "remote.${upstream}.fetch" "+refs/users/${remote_id}/tags/*:refs/tags/me/*" ":refs/tags/me/"
+git config --replace-all "remote.${upstream}.push" "+refs/heads/me/*:refs/users/${remote_id}/heads/*" "^\+refs/heads/me/"

Re: Some local customization enhancements when using git

[Richard Biener]

On Fri, Jan 10, 2020 at 2:23 PM Richard Earnshaw (lists)
 wrote:
>
> This patch is intended to help with folks setting up a git work
> environment for use with GCC following the transition to git.  It
> currently does a couple of things.
>
> 1) Add an alias 'svn-rev' to git so that you can look up a legacy commit
> by its svn revision number.  This enables you to type
> git svn-rev 1234
> and git will show the commit log entry relating to SVN r1234.
>
> 2) Sets up tracking information for the user's private name area in the
> git repo.  It tries to figure out some sensible answers to the data it
> needs, but allows the user to override the values.  It then creates the
> fetch and push entries that are needed for tracking the extra refs.
> This implements one part of the recommendations that I've proposed in
> svnwrite.html for dealing with private branches.
>
> It should be possible to run the script more than once and for it to
> DTRT.  If you change your answers the configuration should be correctly
> updated.

I assume the script is invoked from a clone of the new (final) repo.  Does it
need to be cloned in any special way?

Richard.

> 2020-01-10  Richard Earnshaw  
>
> * gcc-git-customization: New file.
>

Re: Fix gather/scatter check when updating a vector epilogue loop

[Richard Biener]

On Fri, Jan 10, 2020 at 1:39 PM Richard Sandiford
 wrote:
>
> update_epilogue_loop_vinfo applies SSA renmaing to the DR_REF of a
> gather or scatter, so that vect_check_gather_scatter continues to work.
> However, we sometimes also rely on vect_check_gather_scatter when
> using gathers and scatters to implement strided accesses.
>
> This showed up on existing tests when testing with fixed-length
> -msve-vector-bits=128.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  Maybe verging
> on the obvious, but: OK to install?

OK.

> Richard
>
>
> 2020-01-10  Richard Sandiford  
>
> gcc/
> * tree-vect-loop.c (update_epilogue_loop_vinfo): Update DR_REF
> for any type of gather or scatter, including strided accesses.
>
> Index: gcc/tree-vect-loop.c
> ===
> --- gcc/tree-vect-loop.c2020-01-06 12:58:08.877844402 +
> +++ gcc/tree-vect-loop.c2020-01-10 12:37:58.683829420 +
> @@ -8458,7 +8458,7 @@ update_epilogue_loop_vinfo (class loop *
>  updated offset we set using ADVANCE.  Instead we have to make sure 
> the
>  reference in the data references point to the corresponding copy of
>  the original in the epilogue.  */
> -  if (STMT_VINFO_GATHER_SCATTER_P (stmt_vinfo))
> +  if (STMT_VINFO_MEMORY_ACCESS_TYPE (stmt_vinfo) == VMAT_GATHER_SCATTER)
> {
>   DR_REF (dr)
> = simplify_replace_tree (DR_REF (dr), NULL_TREE, NULL_TREE,

Re: Use get_related_vectype_for_scalar_type for reduction indices

[Richard Biener]

On Fri, Jan 10, 2020 at 1:45 PM Richard Sandiford
 wrote:
>
> The related_vector_mode series missed this case in
> vect_create_epilog_for_reduction, where we want to create the
> unsigned integer equivalent of another vector.  Without it we
> could mix SVE and Advanced SIMD vectors in the same operation.
>
> This showed up on existing tests when testing with fixed-length
> -msve-vector-bits=128.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

OK.

Richard.

> Richard
>
>
> 2020-01-10  Richard Sandiford  
>
> gcc/
> * tree-vect-loop.c (vect_create_epilog_for_reduction): Use
> get_related_vectype_for_scalar_type rather than build_vector_type
> to create the index type for a conditional reduction.
>
> Index: gcc/tree-vect-loop.c
> ===
> --- gcc/tree-vect-loop.c2020-01-10 12:37:58.683829420 +
> +++ gcc/tree-vect-loop.c2020-01-10 12:44:20.021356977 +
> @@ -4566,8 +4566,9 @@ vect_create_epilog_for_reduction (stmt_v
>int scalar_precision
> = GET_MODE_PRECISION (SCALAR_TYPE_MODE (TREE_TYPE (vectype)));
>tree cr_index_scalar_type = make_unsigned_type (scalar_precision);
> -  tree cr_index_vector_type = build_vector_type
> -   (cr_index_scalar_type, TYPE_VECTOR_SUBPARTS (vectype));
> +  tree cr_index_vector_type = get_related_vectype_for_scalar_type
> +   (TYPE_MODE (vectype), cr_index_scalar_type,
> +TYPE_VECTOR_SUBPARTS (vectype));
>
>/* First we create a simple vector induction variable which starts
>  with the values {1,2,3,...} (SERIES_VECT) and increments by the

Re: libgo patch committed: Compile examples in _test packages

[Rainer Orth]

Hi Ian,

> This libgo patch compiles examples in _test packages.  Previously if
> the only names defined by _test packages were examples, the gotest
> script would emit an incorrect _testmain.go file.  I worked around
> that by marking the example_test.go files +build ignored.  This CL
> changes the gotest script to handle this case correctly, and removes
> the now-unnecessary build tags.  Bootstrapped and ran Go testsuite on
> x86_64-pc-linux-gnu.  Committed to mainline.

this patch breaks quite a number of libgo tests on Solaris:

FAIL: bufio
/vol/gcc/src/hg/trunk/local/libgo/testsuite/gotest[541]: local: not found [No 
such file or directory]

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

[PATCH][vect] PR92429: do not fold when updating epilogue statements

[Andre Vieira (lists)]

Hi,

This patch addresses the problem reported in PR92429.  When creating an 
epilogue for vectorization we have to replace the SSA_NAMEs in the 
PATTERN_DEF_SEQs and RELATED_STMTs of the epilogue's loop_vec_infos. 
When doing this we were using simplify_replace_tree which always folds 
the replacement.  This may lead to a different tree-node than the one 
which was analyzed in vect_loop_analyze.  In turn the new tree-node may 
require a different vectorization than the one we had prepared for which 
caused the ICE in question.


This patch adds a parameter to the simplify_replace_tree function such 
that we can optionally disable folding as this patch does in 
update_epilogue_loop_vinfo.


Bootstrapped and regression tested on x86_64.

Is this OK for trunk?

gcc/ChangeLog:
2020-01-10  Andre Vieira  

PR tree-optimization/92429
* tree-ssa-loop-niter.h (simplify_replace_tree): Add parameter.
* tree-ssa-loop-niter.c (simplify_replace_tree): Add parameter
to control folding.
* tree-vect-loop.c (update_epilogue_vinfo): Do not fold when
replacing tree.

gcc/testsuite/ChangeLog:
2020-01-10  Andre Vieira  

PR tree-optimization/92429
* gcc.dg/vect/pr92429.c
diff --git a/gcc/testsuite/gcc.dg/vect/pr92429.c b/gcc/testsuite/gcc.dg/vect/pr92429.c
new file mode 100644
index ..7885cd6e4c2639f553a68edcbd70203d1b7b13bb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/pr92429.c
@@ -0,0 +1,14 @@
+/* PR92429 */
+/* { dg-do compile } */
+/* { dg-additional-options "-O1 -fno-tree-fre" } */
+/* { dg-additional-options "-mavx2" { target { x86_64-*-* i?86-*-* } } } */
+
+void
+as (int *gl, int k1)
+{
+  while (k1 < 1)
+{
+  gl[k1] = gl[k1] * gl[k1] / 2;
+  ++k1;
+}
+}
diff --git a/gcc/tree-ssa-loop-niter.h b/gcc/tree-ssa-loop-niter.h
index 621e2c2e28dfbeda279687dcae78e9c4fbe7f559..eb8d1579479cf6df66f9486c7ce6f9ac20a02e2a 100644
--- a/gcc/tree-ssa-loop-niter.h
+++ b/gcc/tree-ssa-loop-niter.h
@@ -58,7 +58,7 @@ extern void free_numbers_of_iterations_estimates (class loop *);
 extern void free_numbers_of_iterations_estimates (function *);
 extern tree simplify_replace_tree (tree, tree,
    tree, tree (*)(tree, void *) = NULL,
-   void * = NULL);
+   void * = NULL, bool do_fold = true);
 extern void substitute_in_loop_info (class loop *, tree, tree);
 
 #endif /* GCC_TREE_SSA_LOOP_NITER_H */
diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c
index 4d5e04945118880d0c0cfb00d9c5841cc58e46f5..6e6df0bfdb898de948c917640ff488b19cf7911f 100644
--- a/gcc/tree-ssa-loop-niter.c
+++ b/gcc/tree-ssa-loop-niter.c
@@ -1934,7 +1934,8 @@ number_of_iterations_cond (class loop *loop,
 
 tree
 simplify_replace_tree (tree expr, tree old, tree new_tree,
-		   tree (*valueize) (tree, void*), void *context)
+		   tree (*valueize) (tree, void*), void *context,
+		   bool do_fold)
 {
   unsigned i, n;
   tree ret = NULL_TREE, e, se;
@@ -1966,7 +1967,7 @@ simplify_replace_tree (tree expr, tree old, tree new_tree,
   for (i = 0; i < n; i++)
 {
   e = TREE_OPERAND (expr, i);
-  se = simplify_replace_tree (e, old, new_tree, valueize, context);
+  se = simplify_replace_tree (e, old, new_tree, valueize, context, do_fold);
   if (e == se)
 	continue;
 
@@ -1976,7 +1977,7 @@ simplify_replace_tree (tree expr, tree old, tree new_tree,
   TREE_OPERAND (ret, i) = se;
 }
 
-  return (ret ? fold (ret) : expr);
+  return (ret ? (do_fold ? fold (ret) : ret) : expr);
 }
 
 /* Expand definitions of ssa names in EXPR as long as they are simple
diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
index cf9b7bcb9c41d678b326b41a4167f1e9c00cc5a4..f30e104e9958c0ebaf170004747ff436b1c1d855 100644
--- a/gcc/tree-vect-loop.c
+++ b/gcc/tree-vect-loop.c
@@ -8440,8 +8440,13 @@ update_epilogue_loop_vinfo (class loop *epilogue, tree advance,
 	gimple_set_op (stmt, j, *new_op);
 	  else
 	{
+	  /* PR92429: The last argument of simplify_replace_tree disables
+		 folding when replacing arguments.  This is required as
+		 otherwise you might end up with different statements than the
+		 ones analyzed in vect_loop_analyze, leading to different
+		 vectorization.  */
 	  op = simplify_replace_tree (op, NULL_TREE, NULL_TREE,
- &find_in_mapping, &mapping);
+	  &find_in_mapping, &mapping, false);
 	  gimple_set_op (stmt, j, op);
 	}
 	}

Re: [ PATCH ] [ C++ ] Implementing P0767 - deprecate POD

[Jonathan Wakely]

On 10/01/20 13:25 +0100, Christophe Lyon wrote:

Hi,

On Thu, 9 Jan 2020 at 22:21, Jonathan Wakely  wrote:


On 09/01/20 19:57 +, Jonathan Wakely wrote:
>I'll commit the attached patch after more testing.

And this follow-up to fix some fallout.



I have noticed:
FAIL: g++:g++.dg/cpp0x/std-layout1.C  -std=c++2a (test for excess errors)

was that follow-up intended to fix this too?


No, I didn't touch the gcc tests.

This fixes it.

Jason, OK for trunk?



commit ffee4a19afa0f3c78378c1db9d5170d2feb156b6
Author: Jonathan Wakely 
Date:   Fri Jan 10 13:04:09 2020 +

Fix g++ testsuite failure caused by std::is_pod deprecation

* g++.dg/cpp0x/std-layout1.C: Use -Wno-deprecated-declarations for
C++20, due to std::is_pod being deprecated.

diff --git a/gcc/testsuite/g++.dg/cpp0x/std-layout1.C b/gcc/testsuite/g++.dg/cpp0x/std-layout1.C
index 09273c5f1c6..36ae170afcb 100644
--- a/gcc/testsuite/g++.dg/cpp0x/std-layout1.C
+++ b/gcc/testsuite/g++.dg/cpp0x/std-layout1.C
@@ -1,4 +1,5 @@
 // { dg-do compile { target c++11 } }
+// { dg-additional-options "-Wno-deprecated-declarations" { target c++2a } }
 
 // [basic.types]/10:
 // Scalar types, standard-layout class types (Clause 9), arrays of such

Re: Fwd: [C++ coroutines 3/7, v2] Front end parsing and transforms.

[Nathan Sidwell]

[for some reason something dropped the emails on their first reposting, 
it took me a while to realize I'd missed something]


On 1/10/20 7:29 AM, Iain Sandoe wrote:


I was able to address all the comments that you made without finding any
show-stoppers.  In addition to your comments, I’ve had one or two privately
(and likewise a few bug reports) these have been addressed in the revised
patch.


Thanks!

There are a few nits and some trivial inconsistencies.  The larger 
issues are:


1) we keep doing name lookup for the traits and handle template.  This 
is unnecessary, but not wrong.  It can be improved later.


2) you use 'struct __X' names in a lot of places.  Why the __?

3) there's an uninitialized read concerning ggc_alloc and the hash 
table.  See comments.  Needs fixing.


4) I don't think it's not PCH safe.  See comments.  I think it's an easy 
but tedious fix.  I guess we can't break it yet?


Otherwise ok.

nathan


--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c



@@ -4583,6 +4584,14 @@ build_new_function_call (tree fn, vec 
**args,
   result = build_over_call (cand, flags, complain);
 }
 
+  if (flag_coroutines

+  && result
+  && result != error_mark_node
+  && TREE_CODE (result) == CALL_EXPR


FWIW you don't need the error_mark_node test in this case, because 
that'll fail the TREE_CODE test anyway.  Change at your discretion.



--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -7752,6 +7752,12 @@ potential_constant_expression_1 (tree t, bool want_rval, 
bool strict, bool now,
 case ANNOTATE_EXPR:
   return RECUR (TREE_OPERAND (t, 0), rval);
 
+/* coroutine await expressions are not.  */


Capital 'Coroutine'.


diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc




+/* Get the existing coroutine_info for FN_DECL, or insert a new one if the
+   entry does not yet exist.  */
+
+coroutine_info *
+get_or_insert_coroutine_info (tree fn_decl)
+{
+  gcc_checking_assert (coroutine_info_table != NULL);
+
+  coroutine_info **slot = coroutine_info_table->find_slot_with_hash
+(fn_decl, coroutine_info_hasher::hash (fn_decl), INSERT);
+
+  if (*slot == NULL)
+{
+  *slot = new (ggc_alloc ()) coroutine_info ();


How is the coroutine_info initialized?  ggc_alloc returns non-zero 
initialized storage?  ggc_cleared_alloc return zeroed storage?



+/* Trees we only need to set up once.  */
+
+static tree void_coro_handle_type;


All these static tree variables are not captured by GC, because there's 
no GTY marker on them.  As it happens, you're ok because the trees are 
all reachable via some other path (identifiers are in the identifier 
hash for instance).


It does mean this is incompatible with PCH though -- if the PCH'd header 
causes coroutines to be inited, things will probably go wrong, or at 
least behave strangely.


I have noticed PCH can easily bitrot, so don't feel bad.



+static tree
+find_coro_traits_template_class (tree fndecl, location_t kw)



+  tree traits_class
+= lookup_template_class (coro_traits_identifier, targ,
+/* in_decl */ NULL_TREE,
+/* context */ std_node,
+/* entering scope */ false, tf_warning_or_error);


You're still always doing name lookup in the coro_traits.  As I 
mentioned before, you can cache that lookup, and simply pass the 
TEMPLATE_DECL into lookup_template_class.  lookup_template_class is 
somewhat misnamed -- it's instantiating a template, and might involve 
namelookup.  Is there a problem with doing that?



+/* [coroutine.handle] */
+
+static tree
+find_coro_handle_type (location_t kw, tree promise_type)
+{
+  /* So now build up a type list for the template, one entry, the promise.  */
+  tree targ = make_tree_vec (1);
+  TREE_VEC_ELT (targ, 0) = promise_type;
+  tree handle_type
+= lookup_template_class (coro_handle_identifier, targ,
+/* in_decl */ NULL_TREE,
+/* context */ std_node,
+/* entering scope */ false, tf_warning_or_error);


... and the same here.
Doing the lookup once in init_coroutines could give a more informative 
user diagnostic 'hey, user, you need these two templates to proceed'



+/* Look for the promise_type in the instantiated.  */

in the instantiated $what?  [traits]

Needs a comment ...


+static bool
+coro_promise_type_found_p (tree fndecl, location_t loc)
+{



+  coroutine_info *coro_info = get_or_insert_coroutine_info (fndecl);
+  /* Without this, we cannot really proceed.  */
+  gcc_checking_assert (coro_info);
+
+  /* If we don't already have a current promise type, try to look it up.  */
+  if (coro_info->promise_type == NULL_TREE)


Yeah, I think you're reading uninitialized state here.  See above 
ggc_alloc comment.




+static tree
+lookup_promise_method (tree fndecl, tree member_id, location_t loc,
+  bool musthave)
+{
+  tree promise = get_coroutine_promise_type (fndecl);
+  tree p

Re: Some local customization enhancements when using git

[Richard Earnshaw (lists)]

On 10/01/2020 13:29, Richard Biener wrote:

On Fri, Jan 10, 2020 at 2:23 PM Richard Earnshaw (lists)
 wrote:


This patch is intended to help with folks setting up a git work
environment for use with GCC following the transition to git.  It
currently does a couple of things.

1) Add an alias 'svn-rev' to git so that you can look up a legacy commit
by its svn revision number.  This enables you to type
git svn-rev 1234
and git will show the commit log entry relating to SVN r1234.

2) Sets up tracking information for the user's private name area in the
git repo.  It tries to figure out some sensible answers to the data it
needs, but allows the user to override the values.  It then creates the
fetch and push entries that are needed for tracking the extra refs.
This implements one part of the recommendations that I've proposed in
svnwrite.html for dealing with private branches.

It should be possible to run the script more than once and for it to
DTRT.  If you change your answers the configuration should be correctly
updated.


I assume the script is invoked from a clone of the new (final) repo.  Does it
need to be cloned in any special way?

Richard.


2020-01-10  Richard Earnshaw  

 * gcc-git-customization: New file.



You can run the script after doing a clone.  It adds the bits that a 
normal clone would not.  I don't know if there's any way of hooking git 
to run this automatically, but I'm not sure if that would be a good idea 
anyway, since that would break non-interactive cloning.


R.

[PATCH] Optimize reads from multiple elts in fold_ctor_reference (PR tree-optimization/93210)

[Jakub Jelinek]

Hi!

The C++ testcase shows we aren't able to determine constant when loading
from
const union U { struct V { int a, b; } c; long long d; } u = { { 1, 2 } };
u.d, but since your patch in the summer can handle
const union V { int a[2]; long long d; } v = { { 1, 2 } };
v.d.

I have noticed dwarf2out.c already has native_encode_initializer that can
deal with extracting stuff from CONSTRUCTORs (recursively), the following
patch just moves it over next to native_encode_expr and adjusts it to match
the native_encode_expr argument standards, i.e. ptr == NULL for dry-run,
off -1 when the whole object needs to be extracted and fitted into ptr
through ptr + len - 1, and then other off values to extract base + off
up to base + off + len - 1 bytes from the ctor into ptr up to ptr + len - 1
and added to the toplevel fold_ctor_reference this native_encode_initializer
as fallback if other ways failed.  E.g. we can't ATM extract RECORD_TYPEs
etc. through native_interpret_expr, while fold_{,non}array_ctor_reference
can extract them, but only if it doesn't overlap multiple initializers.
I;ve done it in the toplevel fold_ctor_reference, not recursive calls,
as I think that would imply exponential compile time behavior.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2020-01-10  Jakub Jelinek  

PR tree-optimization/93210
* fold-const.h (native_encode_initializer,
can_native_interpret_type_p): Declare.
* fold-const.c (native_encode_string): Fix up handling with off != -1,
simplify.
(native_encode_initializer): New function, moved from dwarf2out.c.
Adjust to native_encode_expr compatible arguments, including dry-run
and partial extraction modes.  Don't handle STRING_CST.
(can_native_interpret_type_p): No longer static.
* gimple-fold.c (fold_ctor_reference): For native_encode_expr, verify
offset / BITS_PER_UNIT fits into int and don't call it if
can_native_interpret_type_p fails.  If suboff is NULL and for
CONSTRUCTOR fold_{,non}array_ctor_reference returns NULL, retry with
native_encode_initializer.
(fold_const_aggregate_ref_1): Formatting fix.
* dwarf2out.c (native_encode_initializer): Moved to fold-const.c.
(tree_add_const_value_attribute): Adjust caller.

* gcc.dg/pr93210.c: New test.
* g++.dg/opt/pr93210.C: New test.

--- gcc/fold-const.h.jj 2020-01-01 12:15:53.773522669 +0100
+++ gcc/fold-const.h2020-01-10 11:33:37.516681975 +0100
@@ -26,7 +26,10 @@ extern int folding_initializer;
 
 /* Convert between trees and native memory representation.  */
 extern int native_encode_expr (const_tree, unsigned char *, int, int off = -1);
+extern int native_encode_initializer (tree, unsigned char *, int,
+ int off = -1);
 extern tree native_interpret_expr (tree, const unsigned char *, int);
+extern bool can_native_interpret_type_p (tree);
 
 /* Fold constants as much as possible in an expression.
Returns the simplified expression.
--- gcc/fold-const.c.jj 2020-01-01 12:15:52.894535959 +0100
+++ gcc/fold-const.c2020-01-10 12:47:13.637310535 +0100
@@ -7837,9 +7837,10 @@ native_encode_string (const_tree expr, u
 return 0;
   if (off == -1)
 off = 0;
+  len = MIN (total_bytes - off, len);
   if (ptr == NULL)
 /* Dry run.  */;
-  else if (TREE_STRING_LENGTH (expr) - off < MIN (total_bytes, len))
+  else
 {
   int written = 0;
   if (off < TREE_STRING_LENGTH (expr))
@@ -7847,12 +7848,9 @@ native_encode_string (const_tree expr, u
  written = MIN (len, TREE_STRING_LENGTH (expr) - off);
  memcpy (ptr, TREE_STRING_POINTER (expr) + off, written);
}
-  memset (ptr + written, 0,
- MIN (total_bytes - written, len - written));
+  memset (ptr + written, 0, len - written);
 }
-  else
-memcpy (ptr, TREE_STRING_POINTER (expr) + off, MIN (total_bytes, len));
-  return MIN (total_bytes - off, len);
+  return len;
 }
 
 
@@ -7895,6 +7893,213 @@ native_encode_expr (const_tree expr, uns
 }
 }
 
+/* Similar to native_encode_expr, but also handle CONSTRUCTORs, VCEs,
+   NON_LVALUE_EXPRs and nops.  */
+
+int
+native_encode_initializer (tree init, unsigned char *ptr, int len,
+  int off)
+{
+  /* We don't support starting at negative offset and -1 is special.  */
+  if (off < -1 || init == NULL_TREE)
+return 0;
+
+  STRIP_NOPS (init);
+  switch (TREE_CODE (init))
+{
+case VIEW_CONVERT_EXPR:
+case NON_LVALUE_EXPR:
+  return native_encode_initializer (TREE_OPERAND (init, 0), ptr, len, off);
+default:
+  return native_encode_expr (init, ptr, len, off);
+case CONSTRUCTOR:
+  tree type = TREE_TYPE (init);
+  HOST_WIDE_INT total_bytes = int_size_in_bytes (type);
+  if (total_bytes < 0)
+   return 0;
+  if ((off == -1 && total_bytes > len) || off >= total_bytes)
+   return 0;
+  int o = off 

Re: Some local customization enhancements when using git

[Richard Earnshaw (lists)]

On 10/01/2020 14:01, Richard Earnshaw (lists) wrote:

On 10/01/2020 13:29, Richard Biener wrote:

On Fri, Jan 10, 2020 at 2:23 PM Richard Earnshaw (lists)
 wrote:


This patch is intended to help with folks setting up a git work
environment for use with GCC following the transition to git.  It
currently does a couple of things.

1) Add an alias 'svn-rev' to git so that you can look up a legacy commit
by its svn revision number.  This enables you to type
git svn-rev 1234
and git will show the commit log entry relating to SVN r1234.

2) Sets up tracking information for the user's private name area in the
git repo.  It tries to figure out some sensible answers to the data it
needs, but allows the user to override the values.  It then creates the
fetch and push entries that are needed for tracking the extra refs.
This implements one part of the recommendations that I've proposed in
svnwrite.html for dealing with private branches.

It should be possible to run the script more than once and for it to
DTRT.  If you change your answers the configuration should be correctly
updated.


I assume the script is invoked from a clone of the new (final) repo.  
Does it

need to be cloned in any special way?

Richard.


2020-01-10  Richard Earnshaw  

 * gcc-git-customization: New file.



You can run the script after doing a clone.  It adds the bits that a 
normal clone would not.  I don't know if there's any way of hooking git 
to run this automatically, but I'm not sure if that would be a good idea 
anyway, since that would break non-interactive cloning.


R.


The expected using is:

$ git clone git+svn://{id}@gcc.gnu.org/git/gcc
$ cd gcc
$ contrib/gcc-git-customization
# Now pull the additional refs
$ git fetch

R.

[PATCH] allow nul-over-nul elimination only for single-byte stores (PR 93213)

[Martin Sebor]

The multi-byte store enhancement to the strlen optimization checked
sometime last summer didn't take care to prevent the nul-over-nul
store elimination of multi-byte assignments.  This made it possible
for subsequent multi-byte stores of fewer nuls to cause prior larger
stores to be eliminated.  The latent bug was exposed by a small
unrelated change last December that had masked it until then.

The attached patch makes sure that only single-byte nul stores are
considered as candidates for this elimination.  I will commit it
next week if there are no concerns or suggestions.

Martin
PR tree-optimization/93213 - wrong code with -Og -foptimize-strlen

gcc/testsuite/ChangeLog:

	PR tree-optimization/93213
	* gcc.c-torture/execute/pr93213.c: New test.

gcc/ChangeLog:

	PR tree-optimization/93213
	* tree-ssa-strlen.c (handle_store): Only allow single-byte nul-over-nul
	stores to be eliminated.

Index: gcc/testsuite/gcc.c-torture/execute/pr93213.c
===
--- gcc/testsuite/gcc.c-torture/execute/pr93213.c	(nonexistent)
+++ gcc/testsuite/gcc.c-torture/execute/pr93213.c	(working copy)
@@ -0,0 +1,45 @@
+/* PR tree-optimization/93213 - wrong code on a multibyte store with
+   -Og -foptimize-strlen
+   { dg-additional-options "-Og -foptimize-strlen" } */
+
+typedef unsigned short u16;
+typedef unsigned int u32;
+typedef unsigned __int128 u128;
+
+static inline u128
+foo (u16 u16_1, u32 u32_1, u128 u128_1)
+{
+  u128 u128_0 = 0;
+  u128_1 -= __builtin_mul_overflow (u32_1, u16_1, &u32_1);
+  __builtin_memmove (&u16_1, &u128_0, 2);
+  __builtin_memmove (&u16_1, &u128_1, 1);
+  return u16_1;
+}
+
+__attribute__ ((noipa)) void
+bar (void)
+{
+  char a[] = { 1, 2 };
+  const char b[] = { 0, 0 };
+  const char c[] = { 2 };
+  __builtin_memcpy (a, b, 2);
+  // The above is transformed into
+  //   MEM  [(char * {ref-all})&a] = 0;
+  // which was then dropped because of the non-nul store below.
+  __builtin_memcpy (a, c, 1);
+
+  volatile char *p = a;
+  if (p[0] != 2 || p[1] != 0)
+__builtin_abort ();
+}
+
+int
+main (void)
+{
+  u16 x = foo (-1, -1, 0);
+  if (x != 0xff)
+__builtin_abort ();
+
+  bar ();
+  return 0;
+}
Index: gcc/tree-ssa-strlen.c
===
--- gcc/tree-ssa-strlen.c	(revision 280100)
+++ gcc/tree-ssa-strlen.c	(working copy)
@@ -5239,10 +5239,10 @@ handle_store (gimple_stmt_iterator *gsi, bool *zer
 	}
 }
 
-  if (si != NULL && offset == 0 && storing_all_zeros_p)
+  if (si != NULL && offset == 0 && storing_all_zeros_p && lenrange[2] == 1)
 {
-  /* Allow adjust_last_stmt to remove it if the stored '\0'
-	 is immediately overwritten.  */
+  /* For single-byte stores only, allow adjust_last_stmt to remove
+	 the statement if the stored '\0' is immediately overwritten.  */
   laststmt.stmt = stmt;
   laststmt.len = build_int_cst (size_type_node, 1);
   laststmt.stridx = si->idx;

[PATCH] [amdgcn] Remove dependency on stdint.h in libgcc

[Kwok Cheung Yeung]

The patch for sub-word atomics support added an include of stdint.h for the 
definition of uintptr_h, but this can result in GCC compilation failing if the 
stdint.h header has not been installed (from newlib in the case of AMD GCN).


I have fixed this by removing the stdint.h include and replacing uintptr_t with 
__UINTPTR_TYPE__, which is a predefined macro in the C preprocessor.


Okay for trunk?

Kwok

2020-01-10  Kwok Cheung Yeung  

libgcc/
* config/gcn/atomic.c: Remove include of stdint.h.
(__sync_val_compare_and_swap_##SIZE): Replace uintptr_t with
__UINTPTR_TYPE__.
---
 libgcc/config/gcn/atomic.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/libgcc/config/gcn/atomic.c b/libgcc/config/gcn/atomic.c
index 214c9a5..adceb02 100644
--- a/libgcc/config/gcn/atomic.c
+++ b/libgcc/config/gcn/atomic.c
@@ -22,15 +22,14 @@
.  */

 #include 
-#include 

 #define __SYNC_SUBWORD_COMPARE_AND_SWAP(TYPE, SIZE) \
 \
 TYPE\
 __sync_val_compare_and_swap_##SIZE (TYPE *ptr, TYPE oldval, TYPE newval) \
 {   \
-  unsigned int *wordptr = (unsigned int *)((uintptr_t) ptr & ~3UL);
 \
-  int shift = ((uintptr_t) ptr & 3UL) * 8;  \
+  unsigned int *wordptr = (unsigned int *)((__UINTPTR_TYPE__ ) ptr & ~3UL);  \
+  int shift = ((__UINTPTR_TYPE__ ) ptr & 3UL) * 8;  \
   unsigned int valmask = (1 << (SIZE * 8)) - 1;  \
   unsigned int wordmask = ~(valmask << shift);\
   unsigned int oldword = *wordptr;  \
--
2.8.1

Re: Some local customization enhancements when using git

[Richard Earnshaw (lists)]

On 10/01/2020 13:23, Richard Earnshaw (lists) wrote:
This patch is intended to help with folks setting up a git work 
environment for use with GCC following the transition to git.  It 
currently does a couple of things.


1) Add an alias 'svn-rev' to git so that you can look up a legacy commit 
by its svn revision number.  This enables you to type

git svn-rev 1234
and git will show the commit log entry relating to SVN r1234.

2) Sets up tracking information for the user's private name area in the 
git repo.  It tries to figure out some sensible answers to the data it 
needs, but allows the user to override the values.  It then creates the 
fetch and push entries that are needed for tracking the extra refs. This 
implements one part of the recommendations that I've proposed in 
svnwrite.html for dealing with private branches.


It should be possible to run the script more than once and for it to 
DTRT.  If you change your answers the configuration should be correctly 
updated.


2020-01-10  Richard Earnshaw  

 * gcc-git-customization: New file.



Updated to add better support for diff-ing .md files.

R.
diff --git a/contrib/gcc-git-customization b/contrib/gcc-git-customization
new file mode 100755
index 000..fd3c560f3ae
--- /dev/null
+++ b/contrib/gcc-git-customization
@@ -0,0 +1,59 @@
+#!/bin/sh
+
+# Script to add some local git customizations suitable for working
+# with the GCC git repository
+
+ask () {
+question=$1
+default=$2
+var=$3
+echo -n $question "["$default"]? "
+read answer
+if [ "x$answer" = "x" ]
+then
+	eval $var=$default
+else
+	eval $var=$answer
+fi
+}
+
+# Add a git command to find the git commit equivalent to legacy SVN revision NNN
+git config alias.svn-rev '!f() { rev=$1; shift; git log --all --grep="From-SVN: r$rev\\b" "${@}"; } ; f'
+
+# Make diff on MD files uses "(define" as a function marker.
+# Use this in conjunction with a .gitattributes file containing
+# *.mddiff=md
+git config diff.md.xfuncname '^\(define.*$'
+
+upstream=`git config --get "gcc-config.upstream"`
+if [ "x$upstream" = "x" ]
+then
+upstream="origin"
+fi
+ask "Local name for upstream repository" "origin" upstream
+git config "gcc-config.upstream" "$upstream"
+
+remote_id=`git config --get "gcc-config.user"`
+if [ "x$remote_id" = "x" ]
+then
+# See if the url specifies the remote user name.
+url=`git config --get "remote.$upstream.url"`
+if [ "x$url" = "x" ]
+then
+	# This is a pure guess, but for many people it might be OK.
+	remote_id=`whoami`
+else
+	remote_id=`echo $url | sed -r "s|^.*ssh://(.+)@gcc.gnu.org.*$|\1|"`
+	if [ x$remote_id = x$url ]
+	then
+	remote_id=`whoami`
+	fi
+fi
+fi
+ask "Account name on gcc.gnu.org" $remote_id remote_id
+git config "gcc-config.user" "$remote_id"
+
+echo "Setting up tracking for private namespace $remote_id in remotes/$upstream/me"
+git config --replace-all "remote.${upstream}.fetch" "+refs/users/${remote_id}/heads/*:refs/remotes/${upstream}/me/*" ":refs/remotes/${upstream}/me/"
+git config --replace-all "remote.${upstream}.fetch" "+refs/users/${remote_id}/tags/*:refs/tags/me/*" ":refs/tags/me/"
+git config --replace-all "remote.${upstream}.push" "+refs/heads/me/*:refs/users/${remote_id}/heads/*" "^\+refs/heads/me/"

Go patch committed: Permit duplicate methods from embedded interfaces

[Ian Lance Taylor]

This patch to the Go frontend permits duplicate methods from embedded
interfaces.  This is a language change for Go 1.14.  Bootstrapped and
ran Go testsuite on x86_64-pc-linux-gnu.  Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 280085)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-92ee4c2e295fc760105f187f6ea6dc65c81fa892
+98c4c21b52afd6384f9364527bd7f5f9a1c752cf
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/types.cc
===
--- gcc/go/gofrontend/types.cc  (revision 279848)
+++ gcc/go/gofrontend/types.cc  (working copy)
@@ -8943,8 +8943,12 @@ Interface_type::finalize_methods()
  continue;
}
 
+  const Typed_identifier_list* imethods = it->parse_methods_;
+  if (imethods == NULL)
+   continue;
+
   Named_type* nt = t->named_type();
-  if (nt != NULL && it->parse_methods_ != NULL)
+  if (nt != NULL)
{
  std::vector::const_iterator q;
  for (q = seen.begin(); q != seen.end(); ++q)
@@ -8960,22 +8964,26 @@ Interface_type::finalize_methods()
  seen.push_back(nt);
}
 
-  const Typed_identifier_list* imethods = it->parse_methods_;
-  if (imethods == NULL)
-   continue;
   for (Typed_identifier_list::const_iterator q = imethods->begin();
   q != imethods->end();
   ++q)
{
  if (q->name().empty())
inherit.push_back(*q);
- else if (this->find_method(q->name()) == NULL)
-   this->all_methods_->push_back(Typed_identifier(q->name(),
-  q->type(), tl));
  else
-   go_error_at(tl, "inherited method %qs is ambiguous",
-Gogo::message_name(q->name()).c_str());
+   {
+ const Typed_identifier* oldm = this->find_method(q->name());
+ if (oldm == NULL)
+   this->all_methods_->push_back(Typed_identifier(q->name(),
+  q->type(), tl));
+ else if (!Type::are_identical(q->type(), oldm->type(),
+   Type::COMPARE_TAGS, NULL))
+   go_error_at(tl, "duplicate method %qs",
+   Gogo::message_name(q->name()).c_str());
+   }
}
+
+  seen.pop_back();
 }
 
   if (!this->all_methods_->empty())
Index: gcc/testsuite/go.test/test/fixedbugs/bug211.go
===
--- gcc/testsuite/go.test/test/fixedbugs/bug211.go  (revision 279815)
+++ gcc/testsuite/go.test/test/fixedbugs/bug211.go  (nonexistent)
@@ -1,14 +0,0 @@
-// errorcheck
-
-// Copyright 2009 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package main
-
-type R interface { duplicate() }
-type S interface { duplicate() }
-type T interface { R; S }  // ERROR "duplicate"
-
-func main() {
-}
Index: gcc/testsuite/go.test/test/fixedbugs/bug251.go
===
--- gcc/testsuite/go.test/test/fixedbugs/bug251.go  (revision 279815)
+++ gcc/testsuite/go.test/test/fixedbugs/bug251.go  (working copy)
@@ -1,18 +1,18 @@
 // errorcheck
 
-// Copyright 2010 The Go Authors.  All rights reserved.
+// Copyright 2010 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
 package main
 
-type I1 interface {
+type I1 interface { // GC_ERROR "invalid recursive type"
m() I2
I2 // GCCGO_ERROR "loop|interface"
 }
 
 type I2 interface {
-   I1 // ERROR "loop|interface"
+   I1 // GCCGO_ERROR "loop|interface"
 }
 
 

[PATCH][arm][backport] arm: fix v[78]-r multilibs when configured with --with-multlib-list=aprofile

[Przemyslaw Wirkus]

Hi,
When gcc for Arm is configured with --with-multilib-list=aprofile a
misplaced endif directive in the makefile was causing the arm->thumb
mapping for multilibs to be omitted from the reuse rules.  This
resulted in the default multilib being picked rather than the thumb2
opimized version.

gcc/ChangeLog:
2020-01-10  Przemyslaw Wirkus  

Backport from trunk
* config/arm/t-multilib: Use arm->thumb multilib reuse rules
on a-profile.

Ok for gcc-9-branch?

kind regards
Przemyslaw Wirkusdiff --git a/gcc/config/arm/t-multilib b/gcc/config/arm/t-multilib
index 
dc97c8f09fb0b7f53520432e1a174adfce1bf6af..d5ee537193f2416909516c563b9848a79dabb1bf
 100644
--- a/gcc/config/arm/t-multilib
+++ b/gcc/config/arm/t-multilib
@@ -185,6 +185,8 @@ MULTILIB_MATCHES+= march?armv7=march?armv8.5-a
 MULTILIB_MATCHES   += $(foreach ARCH, $(v8_5_a_simd_variants), \
 march?armv7+fp=march?armv8.5-a$(ARCH))
 
+endif  # Not APROFILE.
+
 # Use Thumb libraries for everything.
 
 MULTILIB_REUSE += 
mthumb/march.armv7/mfloat-abi.soft=marm/march.armv7/mfloat-abi.soft
@@ -198,4 +200,3 @@ MULTILIB_REUSE  += $(foreach MODE, arm thumb, \
 $(foreach ARCH, armv7, \
   
mthumb/march.$(ARCH)/mfloat-abi.soft=m$(MODE)/march.$(ARCH)/mfloat-abi.softfp))
 
-endif  # Not APROFILE.

[PATCH][arm] [backport] arm: Fix rmprofile multilibs when architecture includes +mp or +sec (PR target/93188)

[Przemyslaw Wirkus]

Hi,
When only the rmprofile multilibs are built, compiling for armv7-a
should select the generic v7 multilibs.  This used to work before +sec
and +mp were added to the architecture options but it was broken by
that update.  This patch fixes those variants and adds some tests to
ensure that they remain fixed ;-)

gcc/ChangeLog:
2020-01-08  Przemyslaw Wirkus  

Backport from trunk
PR target/93188
* config/arm/t-multilib (MULTILIB_MATCHES): Add rules to match
armv7-a{+mp,+sec,+mp+sec} to appropriate armv7 multilib variants
when only building rm-profile multilibs.

gcc/testsuite/ChangeLog:
2020-01-08  Przemyslaw Wirkus  

Backport from trunk
* gcc.target/arm/multilib.exp: Add new tests for rm-profile only.

Ok for gcc-9-branch?

kind regards
Przemyslaw Wirkus
diff --git a/gcc/config/arm/t-multilib b/gcc/config/arm/t-multilib
index 
dc97c8f09fb0b7f53520432e1a174adfce1bf6af..182df6e94133d01500de24f511b61beb80152a0e
 100644
--- a/gcc/config/arm/t-multilib
+++ b/gcc/config/arm/t-multilib
@@ -132,10 +132,19 @@ MULTILIB_MATCHES  += 
march?armv7-r+fp.sp=march?armv8-r+crc+fp.sp
 
 ifeq (,$(HAS_APROFILE))
 # Map all v7-a
+
 MULTILIB_MATCHES   += march?armv7=march?armv7-a
+
+MULTILIB_MATCHES   += $(foreach ARCH, $(v7_a_arch_variants), \
+march?armv7=march?armv7-a$(ARCH))
+
 MULTILIB_MATCHES   += $(foreach ARCH, $(v7_a_nosimd_variants) 
$(v7_a_simd_variants), \
 march?armv7+fp=march?armv7-a$(ARCH))
 
+MULTILIB_MATCHES   += $(foreach ARCHVAR, $(v7_a_arch_variants), \
+$(foreach ARCH, $(v7_a_nosimd_variants) 
$(v7_a_simd_variants), \
+  march?armv7+fp=march?armv7-a$(ARCHVAR)$(ARCH)))
+
 MULTILIB_MATCHES   += march?armv7=march?armv7ve
 
 # ARMv7ve FP/SIMD variants: map down to v7+fp
diff --git a/gcc/testsuite/gcc.target/arm/multilib.exp 
b/gcc/testsuite/gcc.target/arm/multilib.exp
index 
dcea829965eb15e372401e6389df5a1403393ecb..2df3f9c3117432299ad6ce8746babd5ff4abe405
 100644
--- a/gcc/testsuite/gcc.target/arm/multilib.exp
+++ b/gcc/testsuite/gcc.target/arm/multilib.exp
@@ -434,6 +434,22 @@ if {[multilib_config "aprofile"] } {
check_multi_dir $opts $dir
 }
 }
+if {[multilib_config "rmprofile"] && ![multilib_config "aprofile"]} {
+foreach {opts dir} {
+   {-mcpu=cortex-a9 -mfpu=auto -mfloat-abi=soft} "thumb/v7/nofp"
+   {-mcpu=cortex-a8 -mfpu=auto -mfloat-abi=softfp} "thumb/v7+fp/softfp"
+   {-mcpu=cortex-a5 -mfpu=auto -mfloat-abi=hard} "thumb/v7+fp/hard"
+   {-mcpu=cortex-a53 -mfpu=auto -mfloat-abi=hard} "thumb/v7+fp/hard"
+   {-march=armv7-a+fp -mfpu=auto -mfloat-abi=softfp} "thumb/v7+fp/softfp"
+   {-march=armv7-a+fp -mfpu=auto -mfloat-abi=soft} "thumb/v7/nofp"
+   {-march=armv7-a+mp+simd -mfpu=auto -mfloat-abi=softfp} 
"thumb/v7+fp/softfp"
+   {-march=armv7-a -mfpu=vfpv4 -mfloat-abi=hard} "thumb/v7+fp/hard"
+   {-march=armv7-a+fp -mfpu=auto -mfloat-abi=hard} "thumb/v7+fp/hard"
+   {-march=armv7-a -mfpu=vfpv4 -mfloat-abi=soft} "thumb/v7/nofp"
+} {
+   check_multi_dir $opts $dir
+}
+}
 if {[multilib_config "rmprofile"] } {
 foreach {opts dir} {
{-mcpu=cortex-m0 -mfpu=auto -mfloat-abi=soft} "thumb/v6-m/nofp"

Re: [PATCH] [amdgcn] Remove dependency on stdint.h in libgcc

[Andrew Stubbs]

On 10/01/2020 14:21, Kwok Cheung Yeung wrote:
The patch for sub-word atomics support added an include of stdint.h for 
the definition of uintptr_h, but this can result in GCC compilation 
failing if the stdint.h header has not been installed (from newlib in 
the case of AMD GCN).


I have fixed this by removing the stdint.h include and replacing 
uintptr_t with __UINTPTR_TYPE__, which is a predefined macro in the C 
preprocessor.


Okay for trunk?

Kwok

2020-01-10  Kwok Cheung Yeung  

 libgcc/
 * config/gcn/atomic.c: Remove include of stdint.h.
 (__sync_val_compare_and_swap_##SIZE): Replace uintptr_t with
 __UINTPTR_TYPE__.
---
  libgcc/config/gcn/atomic.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/libgcc/config/gcn/atomic.c b/libgcc/config/gcn/atomic.c
index 214c9a5..adceb02 100644
--- a/libgcc/config/gcn/atomic.c
+++ b/libgcc/config/gcn/atomic.c
@@ -22,15 +22,14 @@
     .  */

  #include 
-#include 

  #define __SYNC_SUBWORD_COMPARE_AND_SWAP(TYPE, SIZE) \
   \
  TYPE \
  __sync_val_compare_and_swap_##SIZE (TYPE *ptr, TYPE oldval, TYPE 
newval) \

  { \
-  unsigned int *wordptr = (unsigned int *)((uintptr_t) ptr & 
~3UL); \

-  int shift = ((uintptr_t) ptr & 3UL) * 8; \
+  unsigned int *wordptr = (unsigned int *)((__UINTPTR_TYPE__ ) ptr & 
~3UL);  \

+  int shift = ((__UINTPTR_TYPE__ ) ptr & 3UL) * 8; \
    unsigned int valmask = (1 << (SIZE * 8)) - 1;    \
    unsigned int wordmask = ~(valmask << shift); \
    unsigned int oldword = *wordptr; \


OK, thanks.

Andrew

Fix type mismatch in SLPed constructors

[Richard Sandiford]

Having the "same" vector types with different modes means that we can
end up vectorising a constructor with a different mode from the lhs.
This patch adds a VIEW_CONVERT_EXPR in that case.

This showed up on existing tests when testing with fixed-length
-msve-vector-bits=128.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Richard


2020-01-10  Richard Sandiford  

gcc/
* tree-vect-slp.c (vectorize_slp_instance_root_stmt): Use a
VIEW_CONVERT_EXPR if the vectorized constructor has a diffeent
type from the lhs.

Index: gcc/tree-vect-slp.c
===
--- gcc/tree-vect-slp.c 2020-01-06 17:59:49.126752227 +
+++ gcc/tree-vect-slp.c 2020-01-10 15:01:13.780190287 +
@@ -4306,6 +4306,10 @@ vectorize_slp_instance_root_stmt (slp_tr
{
  tree vect_lhs = gimple_get_lhs (child_stmt_info->stmt);
  tree root_lhs = gimple_get_lhs (instance->root_stmt->stmt);
+ if (!useless_type_conversion_p (TREE_TYPE (root_lhs),
+ TREE_TYPE (vect_lhs)))
+   vect_lhs = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (root_lhs),
+  vect_lhs);
  rstmt = gimple_build_assign (root_lhs, vect_lhs);
  break;
}

contrib: script to setup git to pull a vendors branches

[Richard Earnshaw (lists)]

This simple script is intended to setup a new git configuration to pull 
the branches and tags for a specific vendor.  This should simplify some 
of the steps needed for working with a vendor's branches.


* git-fetch-vendor.sh: New file.

R.
diff --git a/contrib/git-fetch-vendor.sh b/contrib/git-fetch-vendor.sh
new file mode 100755
index 000..5e1b1f0a854
--- /dev/null
+++ b/contrib/git-fetch-vendor.sh
@@ -0,0 +1,22 @@
+#!/bin/sh
+
+if [ $# != 1 ]
+then
+echo "Usage: $0 "
+exit 1
+fi
+
+vendor=$1
+upstream=`git config --get "gcc-config.upstream"`
+if [ x"$upstream" = x ]
+then
+echo "Config gcc-config.upstream not set, run contrib/gcc-git-customization"
+exit 1
+fi
+
+echo "setting up git to fetch vendor ${vendor} to remotes/${upstream}/${vendor}"
+
+git config --replace-all "remote.${upstream}.fetch" "+refs/vendors/${vendor}/heads/*:refs/remotes/${upstream}/${vendor}/*" ":refs/remotes/${upstream}/${vendor}/"
+git config --replace-all "remote.${upstream}.fetch" "+refs/vendors/${vendor}/tags/*:refs/tags/${vendor}/*" ":refs/tags/${vendor}/"
+git config --replace-all "remote.${upstream}.push" "+refs/heads/${vendor}/*:refs/vendors/${vendor}/heads/*" "^\+refs/heads/${vendor}/"
+git fetch

[committed][AArch64] Tighten mode checks in aarch64_builtin_vectorized_function

[Richard Sandiford]

aarch64_builtin_vectorized_function checked vectors based on the
number of elements and the element mode.  This doesn't interact
well with fixed-length 128-bit SVE, where SVE modes can have those
same properties.  (And we can't just use the built-ins for SVE because
the types use a different ABI.  SVE handles this kind of thing using
optabs instead.)

Tested on aarch64-linux-gnu and applied as 280114.

Richard


2020-01-10  Richard Sandiford  

gcc/
* config/aarch64/aarch64-builtins.c
(aarch64_builtin_vectorized_function): Check for specific vector modes,
rather than checking the number of elements and the element mode.

Index: gcc/config/aarch64/aarch64-builtins.c
===
--- gcc/config/aarch64/aarch64-builtins.c   2020-01-06 12:58:16.461794517 
+
+++ gcc/config/aarch64/aarch64-builtins.c   2020-01-10 15:05:11.486649966 
+
@@ -1942,17 +1942,13 @@ aarch64_builtin_vectorized_function (uns
 tree type_in)
 {
   machine_mode in_mode, out_mode;
-  unsigned HOST_WIDE_INT in_n, out_n;
 
   if (TREE_CODE (type_out) != VECTOR_TYPE
   || TREE_CODE (type_in) != VECTOR_TYPE)
 return NULL_TREE;
 
-  out_mode = TYPE_MODE (TREE_TYPE (type_out));
-  in_mode = TYPE_MODE (TREE_TYPE (type_in));
-  if (!TYPE_VECTOR_SUBPARTS (type_out).is_constant (&out_n)
-  || !TYPE_VECTOR_SUBPARTS (type_in).is_constant (&in_n))
-return NULL_TREE;
+  out_mode = TYPE_MODE (type_out);
+  in_mode = TYPE_MODE (type_in);
 
 #undef AARCH64_CHECK_BUILTIN_MODE
 #define AARCH64_CHECK_BUILTIN_MODE(C, N) 1
@@ -1968,8 +1964,7 @@ #define AARCH64_FIND_FRINT_VARIANT(N) \
 {
 #undef AARCH64_CHECK_BUILTIN_MODE
 #define AARCH64_CHECK_BUILTIN_MODE(C, N) \
-  (out_mode == N##Fmode && out_n == C \
-   && in_mode == N##Fmode && in_n == C)
+  (out_mode == V##C##N##Fmode && in_mode == V##C##N##Fmode)
 CASE_CFN_FLOOR:
   return AARCH64_FIND_FRINT_VARIANT (floor);
 CASE_CFN_CEIL:
@@ -1984,8 +1979,7 @@ #define AARCH64_CHECK_BUILTIN_MODE(C, N)
   return AARCH64_FIND_FRINT_VARIANT (sqrt);
 #undef AARCH64_CHECK_BUILTIN_MODE
 #define AARCH64_CHECK_BUILTIN_MODE(C, N) \
-  (out_mode == SImode && out_n == C \
-   && in_mode == N##Imode && in_n == C)
+  (out_mode == V##C##SImode && in_mode == V##C##N##Imode)
 CASE_CFN_CLZ:
   {
if (AARCH64_CHECK_BUILTIN_MODE (4, S))
@@ -2002,8 +1996,7 @@ #define AARCH64_CHECK_BUILTIN_MODE(C, N)
   }
 #undef AARCH64_CHECK_BUILTIN_MODE
 #define AARCH64_CHECK_BUILTIN_MODE(C, N) \
-  (out_mode == N##Imode && out_n == C \
-   && in_mode == N##Fmode && in_n == C)
+  (out_mode == V##C##N##Imode && in_mode == V##C##N##Fmode)
 CASE_CFN_IFLOOR:
 CASE_CFN_LFLOOR:
 CASE_CFN_LLFLOOR:

Re: copy/copy_backward/fill/fill_n/equal rework

[Jonathan Wakely]

On 10/12/19 15:19 +, Jonathan Wakely wrote:

On 09/12/19 10:32 +0100, François Dumont wrote:
After completing this work and running more tests I realized that 
the declaration of algos was still not ideal.


So here is another version where algos are not re-declare in 
stl_deque.h, I rather include stl_algobase.h in deque.tcc. The 
problem was spotted but another patch I am going to submit 
afterward.


OK for trunk (with a suitable ChangeLog).

Thanks for persisting with this, sorry it took so long.


A small fix for C++98 compat, I'll commit this to trunk shortly.


commit a9b209bd8032a36ad0a02d15358e5c5d699df9fa
Author: Jonathan Wakely 
Date:   Fri Jan 10 15:02:07 2020 +

libstdc++: Fix testcase for C++98 compatibility

* testsuite/25_algorithms/equal/deque_iterators/1.cc: Don't use C++11
initialization syntax.

diff --git a/libstdc++-v3/testsuite/25_algorithms/equal/deque_iterators/1.cc b/libstdc++-v3/testsuite/25_algorithms/equal/deque_iterators/1.cc
index be60db813cc..82bef051542 100644
--- a/libstdc++-v3/testsuite/25_algorithms/equal/deque_iterators/1.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/equal/deque_iterators/1.cc
@@ -104,7 +104,7 @@ void test05()
 {
   using namespace std;
 
-  int a[] { 0, 1, 2, 3, 4 };
+  int a[] = { 0, 1, 2, 3, 4 };
   deque > d1(a, a + 5);
   deque > d2(a, a + 5);
 

[PATCH] libstdc++: Make istreambuf_iterator base class consistent (PR92285)

[Jonathan Wakely]

Since LWG 445 was implemented for GCC 4.7, the std::iterator base class
of std::istreambuf_iterator changes type depending on the -std mode
used. This creates an ABI incompatibility between different -std modes.

This change ensures the base class always has the same type. This makes
layout for C++98 compatible with the current -std=gnu++14 default, but
no longer compatible with C++98 code from previous releases. In practice
this is unlikely to cause real problems, because it only affects the
layout of types with two std::iterator base classes, one of which comes
from std::istreambuf_iterator. Such types are expected to be vanishingly
rare.

PR libstdc++/92285
* include/bits/streambuf_iterator.h (istreambuf_iterator): Make type
of base class independent of __cplusplus value.
[__cplusplus < 201103L] (istreambuf_iterator::reference): Override the
type defined in the base class
* testsuite/24_iterators/istreambuf_iterator/92285.cc: New test.
* testsuite/24_iterators/istreambuf_iterator/requirements/
base_classes.cc: Adjust expected base class for C++98.

Tested powerpc64le-linux, committed to trunk.


commit e4e3920f4e834b1a6c6b58bdb282947d3a08bc8c
Author: Jonathan Wakely 
Date:   Fri Jan 10 12:02:07 2020 +

libstdc++: Make istreambuf_iterator base class consistent (PR92285)

Since LWG 445 was implemented for GCC 4.7, the std::iterator base class
of std::istreambuf_iterator changes type depending on the -std mode
used. This creates an ABI incompatibility between different -std modes.

This change ensures the base class always has the same type. This makes
layout for C++98 compatible with the current -std=gnu++14 default, but
no longer compatible with C++98 code from previous releases. In practice
this is unlikely to cause real problems, because it only affects the
layout of types with two std::iterator base classes, one of which comes
from std::istreambuf_iterator. Such types are expected to be vanishingly
rare.

PR libstdc++/92285
* include/bits/streambuf_iterator.h (istreambuf_iterator): Make type
of base class independent of __cplusplus value.
[__cplusplus < 201103L] (istreambuf_iterator::reference): Override 
the
type defined in the base class
* testsuite/24_iterators/istreambuf_iterator/92285.cc: New test.
* testsuite/24_iterators/istreambuf_iterator/requirements/
base_classes.cc: Adjust expected base class for C++98.

diff --git a/libstdc++-v3/include/bits/streambuf_iterator.h 
b/libstdc++-v3/include/bits/streambuf_iterator.h
index 67dc3861aa6..fe612f39fe9 100644
--- a/libstdc++-v3/include/bits/streambuf_iterator.h
+++ b/libstdc++-v3/include/bits/streambuf_iterator.h
@@ -49,23 +49,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template
 class istreambuf_iterator
 : public iterator= 201103L
-// LWG 445.
- _CharT>
-#else
- _CharT&>
-#endif
+ _CharT*, _CharT>
 {
 public:
   // Types:
   //@{
   /// Public typedefs
-#if __cplusplus > 201703L
+#if __cplusplus < 201103L
+  typedef _CharT& reference; // Changed to _CharT by LWG 445
+#elif __cplusplus > 201703L
   // _GLIBCXX_RESOLVE_LIB_DEFECTS
   // 3188. istreambuf_iterator::pointer should not be unspecified
   using pointer = void;
 #endif
+
   typedef _CharT   char_type;
   typedef _Traits  traits_type;
   typedef typename _Traits::int_type   int_type;
diff --git a/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/92285.cc 
b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/92285.cc
new file mode 100644
index 000..68b484151d5
--- /dev/null
+++ b/libstdc++-v3/testsuite/24_iterators/istreambuf_iterator/92285.cc
@@ -0,0 +1,51 @@
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+#include 
+#include 
+#include 
+
+// PR libstdc++/92285
+// See https://gcc.gnu.org/ml/libstdc++/2019-10/msg00129.html
+
+typedef std::input_iterator_tag category;
+typedef std::char_traits::off_type off_type;
+typedef std::iterator good;
+typedef 

Re: libgo patch committed: Compile examples in _test packages

[Ian Lance Taylor]

On Fri, Jan 10, 2020 at 5:40 AM Rainer Orth  
wrote:
>
> > This libgo patch compiles examples in _test packages.  Previously if
> > the only names defined by _test packages were examples, the gotest
> > script would emit an incorrect _testmain.go file.  I worked around
> > that by marking the example_test.go files +build ignored.  This CL
> > changes the gotest script to handle this case correctly, and removes
> > the now-unnecessary build tags.  Bootstrapped and ran Go testsuite on
> > x86_64-pc-linux-gnu.  Committed to mainline.
>
> this patch breaks quite a number of libgo tests on Solaris:
>
> FAIL: bufio
> /vol/gcc/src/hg/trunk/local/libgo/testsuite/gotest[541]: local: not found [No 
> such file or directory]

Whoops, sorry.  Fixed like so, tested and committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 280109)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-98c4c21b52afd6384f9364527bd7f5f9a1c752cf
+a69ad9c7d1b45edcf8062a07d3a3c9b6838c04f8
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/testsuite/gotest
===
--- libgo/testsuite/gotest  (revision 280109)
+++ libgo/testsuite/gotest  (working copy)
@@ -538,8 +538,7 @@ symtogo() {
 # Takes an example name and puts any output into the file example.txt.
 # It strips comment markers but does not otherwise change the output.
 exampleoutput() {
-local n=$(testname $1)
-local f
+n=$(testname $1)
 for f in $gofiles $xgofiles; do
if ! grep "^func $n(" $f >/dev/null 2>&1; then
continue

Re: [PATCH 05/41] Add -fdiagnostics-nn-line-numbers

[Jeff Law]

On Wed, 2020-01-08 at 23:35 -0500, David Malcolm wrote:
> On Wed, 2020-01-08 at 21:17 -0700, Jeff Law wrote:
> > On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> > > I may be able to self-approve this.  It's used by the
> > > diagnostic_path
> > > patch, and by the analyzer test suite.  Perhaps better to make
> > > undocumeted, or do it via a DejaGnu pruning directive, but I wanted
> > > to get v5 of the kit posted.
> > > 
> > > This patch implements -fdiagnostics-nn-line-numbers, a new option
> > > which makes diagnostic_show_locus print "NN" rather than specific
> > > line numbers when printing the left margin.
> > > 
> > > This is intended purely to make it easier to write certain kinds of
> > > DejaGnu test; various integration tests for diagnostic paths later
> > > in the patch kit make use of it.
> > > 
> > > gcc/ChangeLog:
> > >   * common.opt (fdiagnostics-nn-line-numbers): New option.
> > >   * diagnostic-show-locus.c
> > > (layout::m_use_nn_for_line_numbers_p):
> > >   New field.
> > >   (layout::layout): Initialize it.
> > >   (layout::calculate_linenum_width): Use it when computing
> > >   m_linenum_width.
> > >   (layout::print_source_line): Implement printing "NN" rather
> > > than
> > >   the line number.
> > >   (selftest::test_line_numbers_multiline_range): Add a test of
> > > "NN"
> > >   printing.
> > >   * diagnostic.c (diagnostic_initialize): Initialize
> > >   use_nn_for_line_numbers_p.
> > >   (num_digits): Add "use_nn_p" param.
> > >   (selftest::test_num_digits): Add a test for use_nn_p==true.
> > >   * diagnostic.h (diagnostic_context::use_nn_for_line_numbers_p):
> > >   New field.
> > >   (num_digits): Add optional "use_nn_p" param.
> > >   * doc/invoke.texi (-fdiagnostics-nn-line-numbers): New option.
> > >   * dwarf2out.c (gen_producer_string): Ignore
> > >   OPT_fdiagnostics_nn_line_numbers.
> > >   * lto-wrapper.c (merge_and_complain): Handle
> > >   OPT_fdiagnostics_nn_line_numbers.
> > >   (append_compiler_options): Likewise.
> > >   (append_diag_options): Likewise.
> > >   * opts.c (common_handle_option): Likewise.
> > >   * toplev.c (general_init): Initialize
> > >   global_dc->use_nn_for_line_numbers_p.
> > Reminds me a lot of the option to not print insn numbers and certain
> > addresses in RTL dumps -- which makes comparing them easier.
> > 
> > OK
> > jeff
> 
> Thanks.  I've actually reworked my working copy to use a DejaGnu-based
> postprocessing approach instead:
>   https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00398.html
> which avoids adding an option.
> 
> Does this latter approach look OK?  (and is the other patch OK?)
After looking at it, I prefer the dejagnu approach.  Not because I like
expect/tcl implementations of *anything* but because it doesn't pollute
the actual compiler with a feature that's really just to make life
easier for the dejagnu testsuite.

jeff
> 

Re: [PATCH] testsuite: add lib/nn-line-numbers.exp

[Jeff Law]

On Wed, 2020-01-08 at 17:07 -0500, David Malcolm wrote:
> (replying to my own "[PATCH 05/41] Add -fdiagnostics-nn-line-numbers"
> with a followup that does it at the DejaGnu level rather than as a
> test-only option)
> 
> On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> > I may be able to self-approve this.  It's used by the diagnostic_path
> > patch, and by the analyzer test suite.  Perhaps better to make
> > undocumeted, or do it via a DejaGnu pruning directive, but I wanted
> > to get v5 of the kit posted.
> > 
> > This patch implements -fdiagnostics-nn-line-numbers, a new option
> > which makes diagnostic_show_locus print "NN" rather than specific
> > line numbers when printing the left margin.
> > 
> > This is intended purely to make it easier to write certain kinds of
> > DejaGnu test; various integration tests for diagnostic paths later
> > in the patch kit make use of it.
> > 
> > gcc/ChangeLog:
> > * common.opt (fdiagnostics-nn-line-numbers): New option.
> > * diagnostic-show-locus.c
> > (layout::m_use_nn_for_line_numbers_p):
> > New field.
> > (layout::layout): Initialize it.
> > (layout::calculate_linenum_width): Use it when computing
> > m_linenum_width.
> > (layout::print_source_line): Implement printing "NN" rather
> > than
> > the line number.
> > (selftest::test_line_numbers_multiline_range): Add a test of
> > "NN"
> > printing.
> > * diagnostic.c (diagnostic_initialize): Initialize
> > use_nn_for_line_numbers_p.
> > (num_digits): Add "use_nn_p" param.
> > (selftest::test_num_digits): Add a test for use_nn_p==true.
> > * diagnostic.h (diagnostic_context::use_nn_for_line_numbers_p):
> > New field.
> > (num_digits): Add optional "use_nn_p" param.
> > * doc/invoke.texi (-fdiagnostics-nn-line-numbers): New option.
> > * dwarf2out.c (gen_producer_string): Ignore
> > OPT_fdiagnostics_nn_line_numbers.
> > * lto-wrapper.c (merge_and_complain): Handle
> > OPT_fdiagnostics_nn_line_numbers.
> > (append_compiler_options): Likewise.
> > (append_diag_options): Likewise.
> > * opts.c (common_handle_option): Likewise.
> > * toplev.c (general_init): Initialize
> > global_dc->use_nn_for_line_numbers_p.
> 
> Here's an alterative patch to the above that replaces the
> "-fdiagnostics-nn-line-numbers" option in earlier versions of the
> analyzer patch kit, by doing it at the DejaGnu level instead.
> 
> This patch adds support for obscuring the line numbers printed in the
> left-hand margin when printing the source code, converting them to NN,
> e.g from:
> 
>   7111 |   if (!(flags & 0x0001)) {
>|  ^
>|  |
>|  (1) following 'true' branch...
>   7112 |
> 
> to:
> 
>NN  |   if (!(flags & 0x0001)) {
>|  ^
>|  |
>|  (1) following 'true' branch...
>NN  |
> 
> This is useful in followup patches e.g. when testing how interprocedural
> paths are printed using multiline.exp, to avoid depending on precise line
> numbers.
> 
> I'm testing this now (but it seems to be a working, drop-in replacement
> for the option in the parts of the patch kit I've tested with it).
> 
> Examples of use can be seen in the analyzer test suite:
>   https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00320.html
> (search for -fdiagnostics-nn-line-numbers and dg-begin-multiline-output
> there to get the idea)
> 
> OK for trunk assuming the other testing looks good?
> 
> gcc/testsuite/ChangeLog:
>   * lib/gcc-dg.exp (cleanup-after-saved-dg-test): Reset global
>   nn_line_numbers_enabled.
>   * lib/nn-line-numbers.exp: New file.
>   * lib/prune.exp: Load nn-line-numbers.exp.
>   (prune_gcc_output): Call maybe-handle-nn-line-numbers.
OK
jeff
> 

Re: [PATCH 06/41] Add diagnostic paths

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> I believe I can self-approve this with my "diagnostic messages"
> maintainer hat on.  It has dependencies on the auto_delete_vec
> and the -fdiagnostics-nn-line-numbers patches.
> 
> Changed in v5:
> - updated copyright years to include 2020
> 
> Changed in v4:
> - Add support for paths for signal-handlers:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00215.html
> - Fix comment
> 
> Changed in v3:
> - Fixup for rebase (r278634): c-format.c: get_pointer_to_named_type -> 
> get_named_type
> https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00530.html
> 
> Changed in v2:
> - Fixup for rebase (r277284) for json::number -> json::integer_number
> https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02035.html
> 
> This patch adds support for associating a "diagnostic_path" with a
> diagnostic: a sequence of events predicted by the compiler that leads to
> the problem occurring, with their locations in the user's source,
> text descriptions, and stack information (for handling interprocedural
> paths).
> 
> For example, the following (hypothetical) error has a 3-event
> intraprocedural path:
> 
> test.c: In function 'demo':
> test.c:29:5: error: passing NULL as argument 1 to 'PyList_Append' which
>   requires a non-NULL parameter
>29 | PyList_Append(list, item);
>   | ^
>   'demo': events 1-3
>  |
>  |   25 |   list = PyList_New(0);
>  |  |  ^
>  |  |  |
>  |  |  (1) when 'PyList_New' fails, returning NULL
>  |   26 |
>  |   27 |   for (i = 0; i < count; i++) {
>  |  |   ~~~
>  |  |   |
>  |  |   (2) when 'i < count'
>  |   28 | item = PyLong_FromLong(random());
>  |   29 | PyList_Append(list, item);
>  |  | ~
>  |  | |
>  |  | (3) when calling 'PyList_Append', passing NULL from (1) as 
> argument 1
>  |
> 
> The patch adds a new "%@" format code for printing event IDs, so that
> in the above, the description of event (3) mentions event (1), showing
> the user where the bogus NULL value comes from (the event IDs are
> colorized to draw the user's attention to them).
> 
> There is a separation between data vs presentation: the above shows how
> the diagnostic-printing code has consolidated the path into a single run
> of events, since all the events are near each other and within the same
> function; more complicated examples (such as interprocedural paths)
> might be printed as multiple runs of events.
> 
> Examples of how interprocedural paths are printed can be seen in the
> test suite (which uses a plugin to exercise the code without relying
> on specific warnings using this functionality).
> 
> Other output formats include
> - JSON,
> - printing each event as a separate "note", and
> - to not emit paths.
> 
> (I have a separate script that can generate HTML from the JSON, but HTML
> is not my speciality; help from a web front-end expert to make it look
> good would be appreciated).
> 
> gcc/ChangeLog:
>   * Makefile.in (OBJS): Add tree-diagnostic-path.o.
>   * common.opt (fdiagnostics-path-format=): New option.
>   (diagnostic_path_format): New enum.
>   (fdiagnostics-show-path-depths): New option.
>   * coretypes.h (diagnostic_event_id_t): New forward decl.
>   * diagnostic-color.c (color_dict): Add "path".
>   * diagnostic-event-id.h: New file.
>   * diagnostic-format-json.cc (json_from_expanded_location): Make
>   non-static.
>   (json_end_diagnostic): Call context->make_json_for_path if it
>   exists and the diagnostic has a path.
>   (diagnostic_output_format_init): Clear context->print_path.
>   * diagnostic-path.h: New file.
>   * diagnostic-show-locus.c (colorizer::set_range): Special-case
>   when printing a run of events in a diagnostic_path so that they
>   all get the same color.
>   (layout::m_diagnostic_path_p): New field.
>   (layout::layout): Initialize it.
>   (layout::print_any_labels): Don't colorize the label text for an
>   event in a diagnostic_path.
>   (gcc_rich_location::add_location_if_nearby): Add
>   "restrict_to_current_line_spans" and "label" params.  Pass the
>   former to layout.maybe_add_location_range; pass the latter
>   when calling add_range.
>   * diagnostic.c: Include "diagnostic-path.h".
>   (diagnostic_initialize): Initialize context->path_format and
>   context->show_path_depths.
>   (diagnostic_show_any_path): New function.
>   (diagnostic_path::interprocedural_p): New function.
>   (diagnostic_report_diagnostic): Call diagnostic_show_any_path.
>   (simple_diagnostic_path::num_events): New function.
>   (simple_diagnostic_path::get_event): New function.
>   (simple_diagnostic_path::add_event): New function.
>   (simple_diagnostic_event::simple_diagnostic_even

Re: [PATCH 08/41] timevar.def: add TVs for analyzer

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Needs review.
> 
> This takes the place of the auto_client_timevar code from v1 of the kit:
>   https://gcc.gnu.org/ml/gcc-patches/2019-11/msg01519.html
> 
> gcc/ChangeLog:
>   * timevar.def (TV_ANALYZER): New timevar.
>   (TV_ANALYZER_SUPERGRAPH): Likewise.
>   (TV_ANALYZER_STATE_PURGE): Likewise.
>   (TV_ANALYZER_PLAN): Likewise.
>   (TV_ANALYZER_SCC): Likewise.
>   (TV_ANALYZER_WORKLIST): Likewise.
>   (TV_ANALYZER_DUMP): Likewise.
>   (TV_ANALYZER_DIAGNOSTICS): Likewise.
>   (TV_ANALYZER_SHORTEST_PATHS): Likewise.
OK
jeff
> 

Re: [PATCH 09/41] analyzer: add ChangeLog

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> New in v5.  Creating this file means all the ChangeLog entries in
> gcc/analyzer are now for this file, rather than for gcc/ChangeLog.
> 
> gcc/analyzer/ChangeLog:
>   * ChangeLog: New file.
Not sure if we have a policy here, but given the level of separation
for the analyzer vs the rest of the compiler, a separate ChangeLog
seems reasonable.

jeff
> 

Re: [PATCH 10/41] analyzer: changes to configure.ac

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Unchanged since v4; needs review.
> 
> This patch adds a configuration option to disable building the analyzer.
> It is built by default (but off by default at compile-time).
> 
> gcc/ChangeLog:
>   * configure.ac (--disable-analyzer, ENABLE_ANALYZER): New option.
>   (gccdepdir): Also create depdir for "analyzer" subdir.
OK
jeff
> 

Re: [PATCH 12/41] analyzer: new files: analyzer-selftests.{cc|h}

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Jeff approved the v1 version of this patch here:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00497.html
> I believe the subsequent changes are obvious enough to be self-approvable.
> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> 
> Changed in v4:
> - Remove include of gcc-plugin.h
> - call run_analyzer_selftests directly, rather than via plugin
>   registration; wrap the analyzer selftests in #if ENABLE_ANALYZER
> - fixup for moves of digraph.cc and tristate.cc from gcc/analyzer to gcc
> 
> gcc/analyzer/ChangeLog:
>   * analyzer-selftests.cc: New file.
>   * analyzer-selftests.h: New file.
While I also think the changes are obvious enough to self-approve, I'll
go ahead and explicitly ACK this one as well.

jeff
> 

Re: [PATCH 11/41] analyzer: add new files to Makefile.in

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Unchanged since v4; needs review
> 
> gcc/ChangeLog:
>   * Makefile.in (lang_opt_files): Add analyzer.opt.
>   (ANALYZER_OBJS): New.
>   (OBJS): Add digraph.o, graphviz.o, tristate.o and ANALYZER_OBJS.
OK
jeff
> 

Re: [PATCH 14/41] analyzer: logging support

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Needs review.
> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> 
> Changed in v4:
> - wrap with #if ENABLE_ANALYZER
> - add DISABLE_COPY_AND_ASSIGN
> 
> This patch adds a logging framework to the analyzer which handles
> hierarchical messages (showing the nested structure of the calls).
> 
> This code is largely based on that in the "jit" subdirectory (with
> a few changes).  An alternative would be to generalize that code
> and move it to the gcc parent directory.
> 
> gcc/analyzer/ChangeLog:
>   * analyzer-logging.cc: New file.
>   * analyzer-logging.h: New file.
If you see a good opportunity to [re]factor, combine, etc with the bits
in JIT, that's fine. But I don't think that should be a requirement to
go forward.

OK

jeff
> 

Re: [PATCH 16/41] analyzer: new files: graphviz.{cc|h}

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Needs review
> 
> Changed in v5:
> - updated copyright years to include 2020
> 
> Changed in v3:
> - https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02461.html
> - moved from gcc/analyzer to gcc
> 
> This patch adds a simple wrapper class to make it easier to
> write human-readable .dot files.
> 
> gcc/ChangeLog:
>   * graphviz.cc: New file.
>   * graphviz.h: New file.
OK
jeff
> 

Re: [PATCH 15/41] analyzer: new file: analyzer-pass.cc and pass registration

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Needs review.
> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> 
> Changed in v4:
> - Remove include of gcc-plugin.h, reworking includes accordingly.
> - Use TV_ANALYZER rather than TV_NONE for the pass
> - Add a gate function to the pass
> - Move the check for #if ENABLE_ANALYZER from the driver to the
>   pass's execute vfunc
> - Expose the pass via make_pass_analyzer, rather than via the plugin
>   pass registration mechanism
> 
> Changed in v3:
> - added to passes.def and tree-pass.h
> 
> gcc/analyzer/ChangeLog:
>   * analyzer-pass.cc: New file.
> 
> gcc/ChangeLog:
>   * passes.def (pass_analyzer): Add before
>   pass_ipa_whole_program_visibility.
>   * tree-pass.h (make_pass_analyzer): New decl.
OK
jeff
> 

Re: [PATCH 17/41] analyzer: new files: digraph.{cc|h} and shortest-paths.h

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Jeff semi-approved an earlier version of this here:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00502.html
> 
> Changed in v5:
> - updated copyright years to include 2020
> 
> Changed in v4:
>   - Moved from gcc/analyzer to gcc, renaming selftests accordingly
>   - Remove  comments
>   - Replace auto_client_timevar with TV_ANALYZER_SHORTEST_PATHS
> 
> This patch adds template classes for directed graphs, their nodes
> and edges, and for finding the shortest path through such a graph.
> 
> gcc/ChangeLog:
>   * digraph.cc: New file.
>   * digraph.h: New file.
>   * shortest-paths.h: New file.
OK.  ANd thanks for making these generally available.

jeff
> 

Re: [PATCH 19/41] analyzer: new files: analyzer.{cc|h}

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Jeff reviewed an earlier version of this here:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00503.html
> My response:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00809.html
> I have followup patches that implement the function_set idea.
> 
> TODO:
>   I haven't yet addressed the is_setjmp_call_p/is_longjmp_call_p concerns
> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> 
> Changed in v4:
> - Remove include of gcc-plugin.h, reworking includes accordingly.
> - Wrap everything in #if ENABLE_ANALYZER
> - Remove /// comment lines
> - Update is_named_call_p to support function pointers:
> https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00178.html
> 
> gcc/analyzer/ChangeLog:
>   * analyzer.cc: New file.
>   * analyzer.h: New file.
So there's a lot more ways to write setjmp.  Some don't even look like
setjmp.  sigsetjmp, savectx, vfork, getcontext all have setjmp like
qualities (returning twice).  They may have an underscore prefix IIRC.

So if what you're really looking for is functions that may return
twice, I think that's the canonical list :-)

jeff

Re: [PATCH 20/41] analyzer: new files: tristate.{cc|h}

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Jeff semi-approved an earlier version of the patch here:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00504.html
> 
> msebor had some observations here:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00644.html
>   TODO: investigate making operator==/!= be non-members
> 
> Changed in v5:
> - updated copyright years to include 2020
> 
> Changed in v4:
> - moved from gcc/analyzer to gcc
> 
> gcc/ChangeLog:
>   * tristate.cc: New file.
>   * tristate.h: New file.
OK
jeff
> 

Re: [PATCH 23/41] analyzer: new files: pending-diagnostic.{cc|h}

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Needs review.
> 
> There was some discussion of the v1 version of this here:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00536.html
> in terms of whether this could be moved up to the "gcc" subdir
> (not without a lot of extra work).
> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> 
> Changed in v4:
> - Remove include of gcc-plugin.h, reworking includes accordingly.
> - Wrap everything in #if ENABLE_ANALYZER
> - Capture const state_change_event& in evdesc_state_change:
> https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00218.html
> 
> This patch adds classes used by the analyzer for handling its diagnostics
> (queueing them, deduplicating them, precision-of-wording hooks).
> 
> gcc/analyzer/ChangeLog:
>   * pending-diagnostic.cc: New file.
>   * pending-diagnostic.h: New file.
I realize we're not pulling this up into the generic code right now,
but I can see a push to do that not too far out in the future.

OK for the trunk, but be aware we'll likely be looking to move this
code in gcc-11.

jeff

Re: [PATCH 26/41] analyzer: new file: sm-file.cc

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Jeff reviewed the v1 version of this patch here:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00507.html
> > I note this seems somewhat incomplete -- which is fine given my
> > recommendation was to focus on the double-free analyzer.  The biggest
> > question is do we want to include this in the first iteration?  Perhaps
> > as an example that others can flesh out to capture the missing stuff
> > (like operations on released FD or file pointers?)
> > 
> > The similarities with double-free, use-after-free are significant.  But
> > I hesitate to suggest trying to generaize and merge them at this point.
> 
> The failures here are likely to be in terms of false negatives rather
> than false positives, as it shares so much infrastructure with
> sm-malloc.cc (there are enough differences though that trying to merging
> feels like more pain that it's worth).
> 
> I have some followups on the branch that fix some of these.
> 
> Or we can leave this out of the initial release if need be.
> Or disable it by default, and require the user to opt-in using
> -fanalyzer-checker=file (though that would complicate the docs).
> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> 
> Changed in v4:
> - Remove include of gcc-plugin.h, reworking includes accordingly.
> - Wrap everything in #if ENABLE_ANALYZER
> - Remove /// comment lines
> - Rework on_leak vfunc:
> https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02028.html
> - Rework for changes to is_named_call_p, resolving function pointers:
>https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00178.html
> 
> This patch adds a state machine checker for stdio's FILE stream API.
> 
> gcc/analyzer/ChangeLog:
>   * sm-file.cc: New file.
I think we ought to go ahead and include it.  No need for someone who
wants to write/finish a FILE */FD analyzer to have to recreate what
you've already done.

jeff
> 

Re: [PATCH 27/41] analyzer: new file: sm-pattern-test.cc

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Jeff approved the v1 version of this patch here:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00508.html
> and the subsequent changes are obvious in my view.
> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> 
> Changed in v4:
> - Remove include of gcc-plugin.h, reworking includes accordingly.
> - Wrap everything in #if ENABLE_ANALYZER
> - Remove /// comment lines
> - Rework on_leak vfunc:
> https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02028.html
> 
> This patch adds a custom state machine checker intended purely for DejaGnu
> testing of the sm "machinery".
> 
> gcc/analyzer/ChangeLog:
>   * sm-pattern-test.cc: New file.
OK
jeff
> 

Re: [PATCH 28/41] analyzer: new file: sm-sensitive.cc

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Jeff reviewed the v1 version of this patch here:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00509.html
> > Given it's not ready for production, fine.  Presumably one of the areas
> > for improvement is a better answer to the "what constitutes exposure"
> > question ;-)
> I have followup work using function_set that could flesh this out
> a bit, but this one isn't going to be "mature" for GCC 10; see
> discussion in cover letter.
> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> 
> Changed in v4:
> - Remove include of gcc-plugin.h, reworking includes accordingly.
> - Wrap everything in #if ENABLE_ANALYZER
> - Remove /// comment lines
> - Rework on_leak vfunc:
> https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02028.html
> - Rework for changes to is_named_call_p, resolving function pointers:
>https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00178.html
> - Implement precision-of-wording vfuncs
> 
> This patch adds a state machine checker for tracking exposure of
> sensitive data (e.g. writing passwords to log files).
> 
> This checker isn't ready for production, and is presented as a
> proof-of-concept of the sm-based approach.
> 
> gcc/analyzer/ChangeLog:
>   * sm-sensitive.cc: New file.
I don't think anything has materially changed.  So OK given this isn't
really considered production ready, but provides a starting point for
someone to work in this space.

jeff
> 

Re: [PATCH 29/41] analyzer: new file: sm-signal.cc

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Needs review.
> 
> Although the comments say "experimental", there are followups (using
> function_set) which make this much more production-ready than the
> other sm-*.cc (other than malloc).
> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> 
> New in v4; part of:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00214.html
> with various fixups
> 
> gcc/analyzer/ChangeLog:
>   * sm-signal.cc: New file.
OK.  Extending this one seems pretty trivial -- it's just checking for
specific functions we know aren't safe, right?

jeff
> 

[PATCH] libiberty, include: add bsearch_r

[Nick Alcock]

libctf wants a bsearch that takes a void * arg pointer to avoid a
nonportable use of __thread.

bsearch_r is required, not optional, at this point because as far as I
can see this obvious-sounding function is not implemented by anyone's
libc.  We can easily move it to AC_LIBOBJ later if it proves necessary
to do so.

include/
* libiberty.h (bsearch_r): New.
libiberty/
* bsearch_r.c: New file.
* Makefile.in (CFILES): Add bsearch_r.c.
(REQUIRED_OFILES): Add bsearch_r.o.
* functions.texi: Regenerate.
---
 include/libiberty.h  |  7 +++
 libiberty/Makefile.in| 12 +-
 libiberty/bsearch_r.c| 93 
 libiberty/functions.texi | 17 
 4 files changed, 128 insertions(+), 1 deletion(-)
 create mode 100644 libiberty/bsearch_r.c

diff --git a/include/libiberty.h b/include/libiberty.h
index 141cb886a85..0bb5b81d4ac 100644
--- a/include/libiberty.h
+++ b/include/libiberty.h
@@ -641,6 +641,13 @@ extern int pexecute (const char *, char * const *, const 
char *,
 
 extern int pwait (int, int *, int);
 
+/* Like bsearch, but takes and passes on an argument like qsort_r.  */
+
+extern void *bsearch_r (register const void *, const void *,
+   size_t, register size_t,
+   register int (*)(const void *, const void *, void *),
+   void *);
+
 #if defined(HAVE_DECL_ASPRINTF) && !HAVE_DECL_ASPRINTF
 /* Like sprintf but provides a pointer to malloc'd storage, which must
be freed by the caller.  */
diff --git a/libiberty/Makefile.in b/libiberty/Makefile.in
index fe738d0db43..53d522d4090 100644
--- a/libiberty/Makefile.in
+++ b/libiberty/Makefile.in
@@ -123,7 +123,7 @@ COMPILE.c = $(CC) -c @DEFS@ $(CFLAGS) $(CPPFLAGS) -I. 
-I$(INCDIR) \
 # CONFIGURED_OFILES and funcs in configure.ac.  Also run "make maint-deps"
 # to build the new rules.
 CFILES = alloca.c argv.c asprintf.c atexit.c   \
-   basename.c bcmp.c bcopy.c bsearch.c bzero.c \
+   basename.c bcmp.c bcopy.c bsearch.c bsearch_r.c bzero.c \
calloc.c choose-temp.c clock.c concat.c cp-demangle.c   \
 cp-demint.c cplus-dem.c crc32.c\
d-demangle.c dwarfnames.c dyn-string.c  \
@@ -167,6 +167,7 @@ REQUIRED_OFILES =   
\
./regex.$(objext) ./cplus-dem.$(objext) ./cp-demangle.$(objext) \
./md5.$(objext) ./sha1.$(objext) ./alloca.$(objext) \
./argv.$(objext)\
+   ./bsearch_r.$(objext)   \
./choose-temp.$(objext) ./concat.$(objext)  \
./cp-demint.$(objext) ./crc32.$(objext) ./d-demangle.$(objext)  \
./dwarfnames.$(objext) ./dyn-string.$(objext)   \
@@ -598,6 +599,15 @@ $(CONFIGURED_OFILES): stamp-picdir stamp-noasandir
else true; fi
$(COMPILE.c) $(srcdir)/bsearch.c $(OUTPUT_OPTION)
 
+./bsearch_r.$(objext): $(srcdir)/bsearch_r.c config.h $(INCDIR)/ansidecl.h
+   if [ x"$(PICFLAG)" != x ]; then \
+ $(COMPILE.c) $(PICFLAG) $(srcdir)/bsearch_r.c -o pic/$@; \
+   else true; fi
+   if [ x"$(NOASANFLAG)" != x ]; then \
+ $(COMPILE.c) $(PICFLAG) $(NOASANFLAG) $(srcdir)/bsearch_r.c -o 
noasan/$@; \
+   else true; fi
+   $(COMPILE.c) $(srcdir)/bsearch_r.c $(OUTPUT_OPTION)
+
 ./bzero.$(objext): $(srcdir)/bzero.c
if [ x"$(PICFLAG)" != x ]; then \
  $(COMPILE.c) $(PICFLAG) $(srcdir)/bzero.c -o pic/$@; \
diff --git a/libiberty/bsearch_r.c b/libiberty/bsearch_r.c
new file mode 100644
index 000..79ebae9b0be
--- /dev/null
+++ b/libiberty/bsearch_r.c
@@ -0,0 +1,93 @@
+/*
+ * Copyright (c) 1990 Regents of the University of California.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ * 3. [rescinded 22 July 1999]
+ * 4. Neither the name of the University nor the names of its contributors
+ *may be used to endorse or promote products derived from this software
+ *without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS 

Re: [PATCH 30/41] analyzer: new file: sm-taint.cc

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Initial comments by Jeff here:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00510.html
> This checker isn't ready for production yet, so the discussion in the
> cover letter applies here.
> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> 
> Changed in v4:
> - Remove include of gcc-plugin.h, reworking includes accordingly.
> - Wrap everything in #if ENABLE_ANALYZER
> - Remove /// comment lines
> - Rework on_leak vfunc:
> https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02028.html
> - Rework for changes to is_named_call_p, resolving function pointers:
>https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00178.html
> 
> This patch adds a state machine checker for tracking "taint",
> where data potentially under an attacker's control is used for
> things like array indices without sanitization (CWE-129).
> 
> This checker isn't ready for production, and is presented as a
> proof-of-concept of the sm-based approach.
> 
> gcc/analyzer/ChangeLog:
>   * sm-taint.cc: New file.
OK.  I think there's all kinds of things we can and will do with taint
analysis over the longer term.   Seems like having this as a starting
point for those interested in the area would be a good thing.

jeff
> 

Re: [PATCH 32/41] analyzer: new files: call-string.{cc|h}

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Jeff approved the v1 version of the patch here:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00806.html
> and the followups count as obvious in my opinion.
> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> 
> Changed in v4:
> - Remove include of gcc-plugin.h, reworking includes accordingly.
> - Wrap everything in #if ENABLE_ANALYZER
> - Add call_string::validate, part of:
> https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02517.html
> 
> This patch adds call_string, a class for representing the
> call stacks at a program_point, so that we can ensure that
> paths through the code are interprocedurally valid.
> 
> gcc/analyzer/ChangeLog:
>   * call-string.cc: New file.
>   * call-string.h: New file.
This is still OK :-)

jeff
> 

Re: [PATCH 31/41] analyzer: new files: analysis-plan.{cc|h}

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Jeff approved ("No concerns here") the v1 version of this patch here:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00511.html
> and the subsequent changes fall under the "obvious" rule in my
> opinion.
> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> 
> Changed in v4:
> - Remove include of gcc-plugin.h, reworking includes accordingly.
> - Wrap everything in #if ENABLE_ANALYZER
> - Use TV_ANALYZER_PLAN rather than an auto_client_timevar.
> - Update for new param API.
> - Add DISABLE_COPY_AND_ASSIGN (analysis_plan);
> 
> This patch adds an analysis_plan class, which encapsulate decisions about
> how the analysis should happen (e.g. the order in which functions should
> be traversed).
> 
> gcc/analyzer/ChangeLog:
>   * analysis-plan.cc: New file.
>   * analysis-plan.h: New file.
OK
jeff
> 

Re: [PATCH 33/41] analyzer: new files: program-point.{cc|h}

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Jeff approved the v1 version of the patch here:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00811.html
> (modulo hash_map issues), and the followups count as obvious in my
> opinion.
> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> 
> Changed in v4:
> - Remove include of gcc-plugin.h, reworking includes accordingly.
> - Wrap everything in #if ENABLE_ANALYZER
> - Remove /// comment lines
> - Add support for more validation, part of:
> https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02517.html
> - Rework logging to avoid exploded_graph multiple-inheritance (moving
>   log_user base to a member)
> - Port to new param API
> 
> This patch introduces function_point and program_point, classes
> for tracking locations within the program (the latter adding
> a call_string for tracking interprocedural location).
> 
> gcc/analyzer/ChangeLog:
>   * program-point.cc: New file.
>   * program-point.h: New file.
Still OK as well.
jeff
> 

Re: [PATCH 35/41] analyzer: new file: exploded-graph.h

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Jeff's initial review of v1 of this patch:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00813.html
> I've addressed most of the issues he raised there.
> TODO: should some structs be classes?
> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> 
> Changed in v4:
> - Remove /// comment lines
> - Don't use multiple inheritance, instead adding a log_user member.
> - Add more validation, part of:
> https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02517.html
> - Generalize rewind_info_t to exploded_edge::custom_info_t
> https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00219.html
> - Add DISABLE_COPY_AND_ASSIGN (exploded_node);
> - Add DISABLE_COPY_AND_ASSIGN (exploded_edge);
> - Add DISABLE_COPY_AND_ASSIGN (exploded_graph);
> 
> This patch adds exploded_graph and related classes, for managing
> exploring paths through the user's code as a directed graph
> of  pairs.
> 
> gcc/analyzer/ChangeLog:
>   * exploded-graph.h: New file.
So I'm not going to insist on the struct/class change.  It was
something I wanted you self-review and consider.  If you end up
changing them, consider that pre-approved.

jeff 
> 

Re: [PATCH] libstdcxx: Update ctype_base.h from NetBSD upstream

[Jonathan Wakely]

On 07/01/20 12:44 -0800, Jason Thorpe wrote:



On Jan 7, 2020, at 7:43 AM, Jonathan Wakely  wrote:

For Jason and Krister's benefit, that last comment was referring to
an earlier suggestion to not try to support old NetBSD releases, see
https://gcc.gnu.org/ml/libstdc++/2020-01/msg00026.html


I think we need the netbsd target maintainers (CC'd) to decide whether
GCC should still support older releases or drop that support for GCC
10. Usually I'd say we need a period of deprecation, but if GCC
doesn't currently build on NetBSD then maybe that's unnecessary.


The affected NetBSD versions are NetBSD 6 and earlier, which are EOL from the 
NetBSD perspective, so I think this is OK.


So is this patch OK then?

Could somebody please test it on NetBSD? (Ideally on the oldest
supported release, but anything is better than nothing).

This differs from the patches posted by using _CTYPE_BL for the
isblank class, which seems better than using _CTYPE_S.


commit 67c5a9a8c7e689bb2208d12e5459c1e207cbbd4c
Author: Jonathan Wakely 
Date:   Fri Jan 10 16:01:19 2020 +

libstdc++: std::ctype fixes for recent versions of NetBSD

This removes support for EOL versions of NetBSD and syncs the
definitions with patches from NetBSD upstream.

2020-01-10  Kai-Uwe Eckhardt  
Matthew Bauer  
Jonathan Wakely  

PR bootstrap/64271 (partial)
* config/os/bsd/netbsd/ctype_base.h (ctype_base::mask): Change type
to unsigned short.
(ctype_base::alpha, ctype_base::digit, ctype_base::xdigit)
(ctype_base::print, ctype_base::graph, ctype_base::alnum): Sync
definitions with NetBSD upstream.
(ctype_base::blank): Use _CTYPE_BL.
* config/os/bsd/netbsd/ctype_configure_char.cc (_C_ctype_): Remove
Declaration.
(ctype::classic_table): Use _C_ctype_tab_ instead of _C_ctype_.
(ctype::do_toupper, ctype::do_tolower): Cast char
parameters to unsigned char.
* config/os/bsd/netbsd/ctype_inline.h (ctype::is): Likewise.

diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
index 340bd8e3d4e..28bf232ba83 100644
--- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
+++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
@@ -43,35 +43,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 // NB: Offsets into ctype::_M_table force a particular size
 // on the mask type. Because of this, we don't use an enum.
-typedef unsigned char  	mask;
+typedef unsigned short  	mask;
 
-#ifndef _CTYPE_U
-static const mask upper	= _U;
-static const mask lower 	= _L;
-static const mask alpha 	= _U | _L;
-static const mask digit 	= _N;
-static const mask xdigit 	= _N | _X;
-static const mask space 	= _S;
-static const mask print 	= _P | _U | _L | _N | _B;
-static const mask graph 	= _P | _U | _L | _N;
-static const mask cntrl 	= _C;
-static const mask punct 	= _P;
-static const mask alnum 	= _U | _L | _N;
-#else
-static const mask upper	= _CTYPE_U;
-static const mask lower 	= _CTYPE_L;
-static const mask alpha 	= _CTYPE_U | _CTYPE_L;
-static const mask digit 	= _CTYPE_N;
-static const mask xdigit 	= _CTYPE_N | _CTYPE_X;
-static const mask space 	= _CTYPE_S;
-static const mask print 	= _CTYPE_P | _CTYPE_U | _CTYPE_L | _CTYPE_N | _CTYPE_B;
-static const mask graph 	= _CTYPE_P | _CTYPE_U | _CTYPE_L | _CTYPE_N;
-static const mask cntrl 	= _CTYPE_C;
-static const mask punct 	= _CTYPE_P;
-static const mask alnum 	= _CTYPE_U | _CTYPE_L | _CTYPE_N;
-#endif
+static const mask upper	= _CTYPE_U;
+static const mask lower	= _CTYPE_L;
+static const mask alpha	= _CTYPE_A;
+static const mask digit	= _CTYPE_D;
+static const mask xdigit	= _CTYPE_X;
+static const mask space	= _CTYPE_S;
+static const mask print	= _CTYPE_R;
+static const mask graph	= _CTYPE_G;
+static const mask cntrl	= _CTYPE_C;
+static const mask punct	= _CTYPE_P;
+static const mask alnum	= _CTYPE_A | _CTYPE_D;
 #if __cplusplus >= 201103L
-static const mask blank 	= space;
+static const mask blank	= _CTYPE_BL;
 #endif
   };
 
diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
index 0680bb350c3..86f7828517d 100644
--- a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
+++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
@@ -38,11 +38,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 // Information as gleaned from /usr/include/ctype.h
 
-  extern "C" const u_int8_t _C_ctype_[];
-
   const ctype_base::mask*
   ctype::classic_table() throw()
-  { return _C_ctype_ + 1; }
+  { return _C_ctype_tab_ + 1; }
 
   ctype::ctype(__c_locale, const mask* __table, bool __del,
 		 size_t __refs)
@@ -69,14 +67,14 @@ _GLIBCXX_BEGIN_NAME

Re: [PATCH 36/41] analyzer: new files: state-purge.{cc|h}

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Jeff approved the v1 version of the patch here:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00815.html
> (with one item that I've addressed in v5), and the followups count as
> obvious in my opinion.
> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> - kill the debugging leftover identified by Jeff
> 
> Changed in v4:
> - Remove include of gcc-plugin.h, reworking includes accordingly.
> - Wrap everything in #if ENABLE_ANALYZER
> - Remove /// comment lines
> - Use TV_ANALYZER_STATE_PURGE rather than an auto_client_timevar
> - Fix .dot output:
> https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02461.html
> - Add DISABLE_COPY_AND_ASSIGN (state_purge_map);
> 
> This patch adds classes for tracking what state can be safely purged
> at any given point in the program.
> 
> gcc/analyzer/ChangeLog:
>   * state-purge.cc: New file.
>   * state-purge.h: New file.
OK
jeff
> 

Re: [PATCH 40/41] gdbinit.in: add break-on-saved-diagnostic

[Jeff Law]

On Wed, 2020-01-08 at 04:03 -0500, David Malcolm wrote:
> Needs review (or potentially falls under the "obvious" rule, at a
> stretch).
> 
> This patch adds a "break-on-saved-diagnostic" command to gdbinit.in,
> useful for debugging when a diagnostic is queued by the analyzer.
> 
> gcc/ChangeLog:
>   * gdbinit.in (break-on-saved-diagnostic): New command.
OK
jeff
> 

Re: [PATCH 41/41] analyzer: test suite

[Jeff Law]

On Wed, 2020-01-08 at 04:03 -0500, David Malcolm wrote:
> Needs review.
> 
> Changed in v5:
> - updated for removal of analyzer-specific builtins:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01310.html
> 
> Changed in v4:
> - more tests, including a test for .dot output and an LTO test
> - update for change from "--analyzer" to "-fanalyzer"
> 
> This patch adds the testsuite for the analyzer.
> 
> gcc/testsuite/ChangeLog:
>   * gcc.dg/analyzer/CVE-2005-1689-minimal.c: New test.
>   * gcc.dg/analyzer/abort.c: New test.
>   * gcc.dg/analyzer/alloca-leak.c: New test.
>   * gcc.dg/analyzer/analyzer-decls.h: New header.
>   * gcc.dg/analyzer/analyzer-verbosity-0.c: New test.
>   * gcc.dg/analyzer/analyzer-verbosity-1.c: New test.
>   * gcc.dg/analyzer/analyzer-verbosity-2.c: New test.
>   * gcc.dg/analyzer/analyzer.exp: New suite.
>   * gcc.dg/analyzer/attribute-nonnull.c: New test.
>   * gcc.dg/analyzer/call-summaries-1.c: New test.
>   * gcc.dg/analyzer/conditionals-2.c: New test.
>   * gcc.dg/analyzer/conditionals-3.c: New test.
>   * gcc.dg/analyzer/conditionals-notrans.c: New test.
>   * gcc.dg/analyzer/conditionals-trans.c: New test.
>   * gcc.dg/analyzer/data-model-1.c: New test.
>   * gcc.dg/analyzer/data-model-2.c: New test.
>   * gcc.dg/analyzer/data-model-3.c: New test.
>   * gcc.dg/analyzer/data-model-4.c: New test.
>   * gcc.dg/analyzer/data-model-5.c: New test.
>   * gcc.dg/analyzer/data-model-5b.c: New test.
>   * gcc.dg/analyzer/data-model-5c.c: New test.
>   * gcc.dg/analyzer/data-model-5d.c: New test.
>   * gcc.dg/analyzer/data-model-6.c: New test.
>   * gcc.dg/analyzer/data-model-7.c: New test.
>   * gcc.dg/analyzer/data-model-8.c: New test.
>   * gcc.dg/analyzer/data-model-9.c: New test.
>   * gcc.dg/analyzer/data-model-11.c: New test.
>   * gcc.dg/analyzer/data-model-12.c: New test.
>   * gcc.dg/analyzer/data-model-13.c: New test.
>   * gcc.dg/analyzer/data-model-14.c: New test.
>   * gcc.dg/analyzer/data-model-15.c: New test.
>   * gcc.dg/analyzer/data-model-16.c: New test.
>   * gcc.dg/analyzer/data-model-17.c: New test.
>   * gcc.dg/analyzer/data-model-18.c: New test.
>   * gcc.dg/analyzer/data-model-19.c: New test.
>   * gcc.dg/analyzer/data-model-path-1.c: New test.
>   * gcc.dg/analyzer/disabling.c: New test.
>   * gcc.dg/analyzer/dot-output.c: New test.
>   * gcc.dg/analyzer/double-free-lto-1-a.c: New test.
>   * gcc.dg/analyzer/double-free-lto-1-b.c: New test.
>   * gcc.dg/analyzer/double-free-lto-1.h: New header.
>   * gcc.dg/analyzer/equivalence.c: New test.
>   * gcc.dg/analyzer/explode-1.c: New test.
>   * gcc.dg/analyzer/explode-2.c: New test.
>   * gcc.dg/analyzer/factorial.c: New test.
>   * gcc.dg/analyzer/fibonacci.c: New test.
>   * gcc.dg/analyzer/fields.c: New test.
>   * gcc.dg/analyzer/file-1.c: New test.
>   * gcc.dg/analyzer/file-2.c: New test.
>   * gcc.dg/analyzer/function-ptr-1.c: New test.
>   * gcc.dg/analyzer/function-ptr-2.c: New test.
>   * gcc.dg/analyzer/function-ptr-3.c: New test.
>   * gcc.dg/analyzer/gzio-2.c: New test.
>   * gcc.dg/analyzer/gzio-3.c: New test.
>   * gcc.dg/analyzer/gzio-3a.c: New test.
>   * gcc.dg/analyzer/gzio.c: New test.
>   * gcc.dg/analyzer/infinite-recursion.c: New test.
>   * gcc.dg/analyzer/loop-2.c: New test.
>   * gcc.dg/analyzer/loop-2a.c: New test.
>   * gcc.dg/analyzer/loop-3.c: New test.
>   * gcc.dg/analyzer/loop-4.c: New test.
>   * gcc.dg/analyzer/loop.c: New test.
>   * gcc.dg/analyzer/malloc-1.c: New test.
>   * gcc.dg/analyzer/malloc-2.c: New test.
>   * gcc.dg/analyzer/malloc-3.c: New test.
>   * gcc.dg/analyzer/malloc-callbacks.c: New test.
>   * gcc.dg/analyzer/malloc-dce.c: New test.
>   * gcc.dg/analyzer/malloc-dedupe-1.c: New test.
>   * gcc.dg/analyzer/malloc-ipa-1.c: New test.
>   * gcc.dg/analyzer/malloc-ipa-10.c: New test.
>   * gcc.dg/analyzer/malloc-ipa-11.c: New test.
>   * gcc.dg/analyzer/malloc-ipa-12.c: New test.
>   * gcc.dg/analyzer/malloc-ipa-13.c: New test.
>   * gcc.dg/analyzer/malloc-ipa-2.c: New test.
>   * gcc.dg/analyzer/malloc-ipa-3.c: New test.
>   * gcc.dg/analyzer/malloc-ipa-4.c: New test.
>   * gcc.dg/analyzer/malloc-ipa-5.c: New test.
>   * gcc.dg/analyzer/malloc-ipa-6.c: New test.
>   * gcc.dg/analyzer/malloc-ipa-7.c: New test.
>   * gcc.dg/analyzer/malloc-ipa-8-double-free.c: New test.
>   * gcc.dg/analyzer/malloc-ipa-8-lto-a.c: New test.
>   * gcc.dg/analyzer/malloc-ipa-8-lto-b.c: New test.
>   * gcc.dg/analyzer/malloc-ipa-8-lto-c.c: New test.
>   * gcc.dg/analyzer/malloc-ipa-8-lto.h: New test.
>   * gcc.dg/analyzer/malloc-ipa-8-unchecked.c: New test.
>   * gcc.dg/analyzer/malloc-ipa-9.c: New test.
>   * gcc.dg/analyzer/malloc-macro-in

Re: [Patch 0/X] HWASAN v3

[Kyrill Tkachov]

On 1/8/20 11:26 AM, Matthew Malcomson wrote:

Hi everyone,

I'm writing this email to summarise & publicise the state of this patch
series, especially the difficulties around approval for GCC 10 mentioned
on IRC.


The main obstacle seems to be that no maintainer feels they have enough
knowledge about hwasan and justification that it's worthwhile to approve
the patch series.

Similarly, Martin has given a review of the parts of the code he can
(thanks!), but doesn't feel he can do a deep review of the code related
to the RTL hooks and stack expansion -- hence that part is as yet not
reviewed in-depth.



The questions around justification raised on IRC are mainly that it
seems like a proof-of-concept for MTE rather than a stand-alone useable
sanitizer.  Especially since in the GNU world hwasan instrumented code
is not really ready for production since we can only use the
less-"interceptor ABI" rather than the "platform ABI".  This restriction
is because there is no version of glibc with the required modifications
to provide the "platform ABI".

(n.b. that since https://reviews.llvm.org/D69574 the code-generation for
these ABI's is the same).


 From my perspective the reasons that make HWASAN useful in itself are:

1) Much less memory usage.

 From a back-of-the-envelope calculation based on the hwasan paper's
table of memory overhead from over-alignment
https://arxiv.org/pdf/1802.09517.pdf I guess hwasan instrumented code
has an overhead of about 1.1x (~4% from overalignment and ~6.25% from
shadow memory), while asan seems to have an overhead somewhere in the
range 1.5x - 3x.

Maybe there's some data out there comparing total overheads that I
haven't found? (I'd appreciate a reference if anyone has that info).



2) Available on more architectures that MTE.

HWASAN only requires TBI, which is a feature of all AArch64 machines,
while MTE will be an optional extension and only available on certain
architectures.


3) This enables using hwasan in the kernel.

While instrumented user-space applications will be using the
"interceptor ABI" and hence are likely not production-quality, the
biggest aim of implementing hwasan in GCC is to allow building the Linux
kernel with tag-based sanitization using GCC.

Instrumented kernel code uses hooks in the kernel itself, so this ABI
distinction is no longer relevant, and this sanitizer should produce a
production-quality kernel binary.




I'm hoping I can find a maintainer willing to review and ACK this patch
series -- especially with stage3 coming to a close soon.  If there's
anything else I could do to help get someone willing up-to-speed then
please just ask.



FWIW I've reviewed the aarch64 parts over the lifetime of the patch 
series and I am okay with them.


Given the reviews of the sanitiser, library and aarch64 backend 
components, and the data at


https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00387.html

how can we move forward with commit approval ? Is this something a 
global reviewer can help with, Jeff ? :)


Thanks,

Kyrill





Cheers,
Matthew



On 07/01/2020 15:14, Martin Liška wrote:
> On 12/12/19 4:18 PM, Matthew Malcomson wrote:
>
> Hello.
>
> I've just sent few comments that are related to the v3 of the patch set.
> Based on the HWASAN (limited) knowledge the patch seems reasonable 
to me.

> I haven't looked much at the newly introduced RTL-hooks.
> But these seems to me isolated to the aarch64 port.
>
> I can also verify that the patchset works on my aarch64 linux 
machine and

> hwasan.exp and asan.exp tests succeed.
>
>> I haven't gotten ASAN_MARK to print as HWASAN_MARK when using memory
>> tagging,
>> since I'm not sure the way I found to implement this would be
>> acceptable.  The
>> inlined patch below works but it requires a special declaration
>> instead of just
>> an ~#include~.
>
> Knowing that, I would not bother with the printing of HWASAN_MARK.
>
> Thanks for the series,
> Martin

Re: [GCC][PATCH][Aarch64] Add Bfloat16_t scalar type, vector types and machine modes to Aarch64 back-end [2/2]

[Stam Markianos-Wright]

On 1/9/20 4:13 PM, Stam Markianos-Wright wrote:
> 
> 
> On 1/9/20 4:07 PM, Richard Sandiford wrote:
>> Stam Markianos-Wright  writes:
>>> diff --git a/gcc/testsuite/g++.target/aarch64/bfloat_cpp_typecheck.C 
>>> b/gcc/testsuite/g++.target/aarch64/bfloat_cpp_typecheck.C
>>> new file mode 100644
>>> index 000..55cbb0b0ef7
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.target/aarch64/bfloat_cpp_typecheck.C
>>> @@ -0,0 +1,14 @@
>>> +/* { dg-do assemble { target { aarch64*-*-* } } } */
>>> +/* { dg-require-effective-target arm_v8_2a_bf16_neon_ok } */
>>> +/* { dg-add-options arm_v8_2a_bf16_neon }  */
>>> +/* { dg-additional-options "-O3 --save-temps" } */
>>> +
>>> +#include 
>>> +
>>> +void foo (void)
>>> +{
>>> +  bfloat16_t (); /* { dg-error {invalid conversion to type 'bfloat16_t'} 
>>> "" 
>>> {target *-*-*} } */
>>
>> The "" {target *-*-*} stuff isn't needed: that's just for when the test
>> depends on a target selector or if you need to specify a line number
>> (which comes after the target).

Removed them.

> 
> Ah ok cool. I just had something that worked and was just doing ctrl+c ctrl+v 
> everywhere!
> 
>>
>> Same for the rest of the patch.
>>
>>> +  bfloat16_t a = bfloat16_t(); /* { dg-error {invalid conversion to type 
>>> 'bfloat16_t'} "" {target *-*-*} } */
>>
>> Why's this one an error?  Looks like it should be OK.  Do we build
>> bfloat16_t() as a conversion from a zero integer?
>>
> Yea that's exactly what it looked like when I went into the debugging! But 
> will 
> investigate a bit further and see if I can fix it for the next revision.
> 

Changed this to dg-bogus with an XFAIL for the purposes of this patch in Stage 
3 :)

> Thank you so much for the help in getting these fixed :D
> 
> Cheers,
> Stam
> 
>> Looks good otherwise, thanks, but I think we should try to support
>> the line above if we can.
>>
>> Richard
>>
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index ebd3f6cf45bc0b5118c4c39e323e6380d64c885e..ce410ddf5515407a4680e186b04c6b6a40ae2562 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -21760,6 +21760,55 @@ aarch64_stack_protect_guard (void)
   return NULL_TREE;
 }
 
+/* Return the diagnostic message string if conversion from FROMTYPE to
+   TOTYPE is not allowed, NULL otherwise.  */
+
+static const char *
+aarch64_invalid_conversion (const_tree fromtype, const_tree totype)
+{
+  if (element_mode (fromtype) != element_mode (totype))
+{
+  /* Do no allow conversions to/from BFmode scalar types.  */
+  if (TYPE_MODE (fromtype) == BFmode)
+	return N_("invalid conversion from type %");
+  if (TYPE_MODE (totype) == BFmode)
+	return N_("invalid conversion to type %");
+}
+
+  /* Conversion allowed.  */
+  return NULL;
+}
+
+/* Return the diagnostic message string if the unary operation OP is
+   not permitted on TYPE, NULL otherwise.  */
+
+static const char *
+aarch64_invalid_unary_op (int op, const_tree type)
+{
+  /* Reject all single-operand operations on BFmode except for &.  */
+  if (element_mode (type) == BFmode && op != ADDR_EXPR)
+return N_("operation not permitted on type %");
+
+  /* Operation allowed.  */
+  return NULL;
+}
+
+/* Return the diagnostic message string if the binary operation OP is
+   not permitted on TYPE1 and TYPE2, NULL otherwise.  */
+
+static const char *
+aarch64_invalid_binary_op (int op ATTRIBUTE_UNUSED, const_tree type1,
+			   const_tree type2)
+{
+  /* Reject all 2-operand operations on BFmode.  */
+  if (element_mode (type1) == BFmode
+  || element_mode (type2) == BFmode)
+return N_("operation not permitted on type %");
+
+  /* Operation allowed.  */
+  return NULL;
+}
+
 /* Implement TARGET_ASM_FILE_END for AArch64.  This adds the AArch64 GNU NOTE
section at the end if needed.  */
 #define GNU_PROPERTY_AARCH64_FEATURE_1_AND	0xc000
@@ -22010,6 +22059,15 @@ aarch64_libgcc_floating_mode_supported_p
 #undef TARGET_MANGLE_TYPE
 #define TARGET_MANGLE_TYPE aarch64_mangle_type
 
+#undef TARGET_INVALID_CONVERSION
+#define TARGET_INVALID_CONVERSION aarch64_invalid_conversion
+
+#undef TARGET_INVALID_UNARY_OP
+#define TARGET_INVALID_UNARY_OP aarch64_invalid_unary_op
+
+#undef TARGET_INVALID_BINARY_OP
+#define TARGET_INVALID_BINARY_OP aarch64_invalid_binary_op
+
 #undef TARGET_VERIFY_TYPE_CONTEXT
 #define TARGET_VERIFY_TYPE_CONTEXT aarch64_verify_type_context
 
diff --git a/gcc/testsuite/g++.target/aarch64/bfloat_cpp_typecheck.C b/gcc/testsuite/g++.target/aarch64/bfloat_cpp_typecheck.C
new file mode 100644
index ..0a04cfb18e567ae0eec88da8ea37922434c60080
--- /dev/null
+++ b/gcc/testsuite/g++.target/aarch64/bfloat_cpp_typecheck.C
@@ -0,0 +1,14 @@
+/* { dg-do assemble { target { aarch64*-*-* } } } */
+/* { dg-require-effective-target arm_v8_2a_bf16_neon_ok } */
+/* { dg-add-options arm_v8_2a_bf16_neon }  */
+/* { dg-additional-options "-O3 --save-temps" } */
+
+#include 
+
+void foo (void)
+{
+  bfloat16_t (); /*

Re: [PATCH 07/41] Add ordered_hash_map

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Needs review.  This is used in many places in the analyzer.
> msebor made some comments about the v1 version of this patch here:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00231.html
> 
> Changed in v5:
> - updated copyright years to include 2020
> 
> This patch adds an ordered_hash_map template, which is similar to
> hash_map, but preserves insertion order.
> 
> gcc/ChangeLog:
>   * Makefile.in (OBJS): Add ordered-hash-map-tests.o.
>   * ordered-hash-map-tests.cc: New file.
>   * ordered-hash-map.h: New file.
>   * selftest-run-tests.c (selftest::run_tests): Call
>   selftest::ordered_hash_map_tests_cc_tests.
>   * selftest.h (selftest::ordered_hash_map_tests_cc_tests): New
>   decl.
There's nothing inherent in the data that would preclude us from using
a standard container (ie, nothing with embedded GC based on our meeting
Tuesday).  But there isn't an existing standard container with the
right properties (based on email between you and Jon).  Right?

Do you need a private assignment operator?

Jeff

> 

Re: [GCC][PATCH][Aarch64] Add Bfloat16_t scalar type, vector types and machine modes to Aarch64 back-end [2/2]

[Richard Sandiford]

Stam Markianos-Wright  writes:
> On 1/9/20 4:13 PM, Stam Markianos-Wright wrote:
>> On 1/9/20 4:07 PM, Richard Sandiford wrote:
>>> Stam Markianos-Wright  writes:
 diff --git a/gcc/testsuite/g++.target/aarch64/bfloat_cpp_typecheck.C 
 b/gcc/testsuite/g++.target/aarch64/bfloat_cpp_typecheck.C
 new file mode 100644
 index 000..55cbb0b0ef7
 --- /dev/null
 +++ b/gcc/testsuite/g++.target/aarch64/bfloat_cpp_typecheck.C
 @@ -0,0 +1,14 @@
 +/* { dg-do assemble { target { aarch64*-*-* } } } */
 +/* { dg-require-effective-target arm_v8_2a_bf16_neon_ok } */
 +/* { dg-add-options arm_v8_2a_bf16_neon }  */
 +/* { dg-additional-options "-O3 --save-temps" } */
 +
 +#include 
 +
 +void foo (void)
 +{
 +  bfloat16_t (); /* { dg-error {invalid conversion to type 'bfloat16_t'} 
 "" 
 {target *-*-*} } */
>>>
>>> The "" {target *-*-*} stuff isn't needed: that's just for when the test
>>> depends on a target selector or if you need to specify a line number
>>> (which comes after the target).
>
> Removed them.
>
>> 
>> Ah ok cool. I just had something that worked and was just doing ctrl+c 
>> ctrl+v 
>> everywhere!
>> 
>>>
>>> Same for the rest of the patch.
>>>
 +  bfloat16_t a = bfloat16_t(); /* { dg-error {invalid conversion to type 
 'bfloat16_t'} "" {target *-*-*} } */
>>>
>>> Why's this one an error?  Looks like it should be OK.  Do we build
>>> bfloat16_t() as a conversion from a zero integer?
>>>
>> Yea that's exactly what it looked like when I went into the debugging! But 
>> will 
>> investigate a bit further and see if I can fix it for the next revision.
>> 
>
> Changed this to dg-bogus with an XFAIL for the purposes of this patch in 
> Stage 3 :)

Yeah.  Like we discussed off-list, we'd need to change the target hook
to do this properly.  (And if we do change the target hook, it would be
good to make it output the errors itself, like we discussed upthread.)
Something for GCC 11 perhaps...

> diff --git a/gcc/testsuite/g++.target/aarch64/bfloat_cpp_typecheck.C 
> b/gcc/testsuite/g++.target/aarch64/bfloat_cpp_typecheck.C
> new file mode 100644
> index 
> ..0a04cfb18e567ae0eec88da8ea37922434c60080
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/aarch64/bfloat_cpp_typecheck.C
> @@ -0,0 +1,14 @@
> +/* { dg-do assemble { target { aarch64*-*-* } } } */
> +/* { dg-require-effective-target arm_v8_2a_bf16_neon_ok } */
> +/* { dg-add-options arm_v8_2a_bf16_neon }  */
> +/* { dg-additional-options "-O3 --save-temps" } */
> +
> +#include 
> +
> +void foo (void)
> +{
> +  bfloat16_t (); /* { dg-bogus {invalid conversion to type 'bfloat16_t'} "" 
> { xfail *-*-* } } */
> +  bfloat16_t a = bfloat16_t(); /* { dg-error {invalid conversion to type 
> 'bfloat16_t'} } */

This should be a dg-bogus too.

OK with that change, thanks.

Richard

Re: [PATCH 07/41] Add ordered_hash_map

[David Malcolm]

On Fri, 2020-01-10 at 09:22 -0700, Jeff Law wrote:
> On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> > Needs review.  This is used in many places in the analyzer.
> > msebor made some comments about the v1 version of this patch here:
> >   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00231.html
> > 
> > Changed in v5:
> > - updated copyright years to include 2020
> > 
> > This patch adds an ordered_hash_map template, which is similar to
> > hash_map, but preserves insertion order.
> > 
> > gcc/ChangeLog:
> > * Makefile.in (OBJS): Add ordered-hash-map-tests.o.
> > * ordered-hash-map-tests.cc: New file.
> > * ordered-hash-map.h: New file.
> > * selftest-run-tests.c (selftest::run_tests): Call
> > selftest::ordered_hash_map_tests_cc_tests.
> > * selftest.h (selftest::ordered_hash_map_tests_cc_tests): New
> > decl.
> There's nothing inherent in the data that would preclude us from
> using
> a standard container (ie, nothing with embedded GC based on our
> meeting
> Tuesday).

Correct: this doesn't support our GC, but doesn't need to for the uses
I'm making of it.

>   But there isn't an existing standard container with the
> right properties (based on email between you and Jon).  Right?

Correct; std::map uses Key ordering to build a red-black tree; I'm
using insertion ordering (not Key ordering), to get more deterministic
results in the face of ptr values that can change from under me.  It's
analogous to Python's OrderedDict, fwiw.

> Do you need a private assignment operator?

I added that in:
  https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00518.html
to ensure that we don't erroneously use the compiler-generated one;
that's the latest version of this patch.

Dave

[committed][AArch64] Fix reversed vcond_mask invocation in aarch64_evpc_sel

[Richard Sandiford]

aarch64_evpc_sel (new in GCC 10) got the true and false vectors
the wrong way round, leading to execution failures with fixed-length
128-bit SVE.

Now that the ACLE types are in trunk, it's much easier to match
the exact asm sequence for a permute.

Tested on aarch64-linux-gnu and applied as r280121.

Richard


2020-01-10  Richard Sandiford  

gcc/
* config/aarch64/aarch64.c (aarch64_evpc_sel): Fix gen_vcond_mask
invocation.

gcc/testsuite/
* gcc.target/aarch64/sve/sel_1.c: Use SVE types for the arguments and
return values.  Use check-function-bodies instead of scan-assembler.
* gcc.target/aarch64/sve/sel_2.c: Likewise
* gcc.target/aarch64/sve/sel_3.c: Likewise.

Index: gcc/config/aarch64/aarch64.c
===
--- gcc/config/aarch64/aarch64.c2020-01-09 16:26:39.658925053 +
+++ gcc/config/aarch64/aarch64.c2020-01-10 16:30:07.989668567 +
@@ -19449,6 +19449,7 @@ aarch64_evpc_sel (struct expand_vec_perm
 
   machine_mode pred_mode = aarch64_sve_pred_mode (vmode);
 
+  /* Build a predicate that is true when op0 elements should be used.  */
   rtx_vector_builder builder (pred_mode, n_patterns, 2);
   for (int i = 0; i < n_patterns * 2; i++)
 {
@@ -19459,7 +19460,8 @@ aarch64_evpc_sel (struct expand_vec_perm
 
   rtx const_vec = builder.build ();
   rtx pred = force_reg (pred_mode, const_vec);
-  emit_insn (gen_vcond_mask (vmode, vmode, d->target, d->op1, d->op0, pred));
+  /* TARGET = PRED ? OP0 : OP1.  */
+  emit_insn (gen_vcond_mask (vmode, vmode, d->target, d->op0, d->op1, pred));
   return true;
 }
 
Index: gcc/testsuite/gcc.target/aarch64/sve/sel_1.c
===
--- gcc/testsuite/gcc.target/aarch64/sve/sel_1.c2019-08-25 
19:10:33.442172145 +0100
+++ gcc/testsuite/gcc.target/aarch64/sve/sel_1.c2020-01-10 
16:30:07.989668567 +
@@ -1,5 +1,6 @@
 /* { dg-do assemble { target aarch64_asm_sve_ok } } */
 /* { dg-options "-O2 -msve-vector-bits=256 --save-temps" } */
+/* { dg-final { check-function-bodies "**" "" } } */
 
 #include 
 
@@ -13,15 +14,14 @@ #define MASK_32 { 0, 33, 2, 35, 4, 37,
 
 #define INDEX_32 vnx16qi
 
-#define PERMUTE(type, nunits)  \
-type permute_##type (type x, type y)   \
-{  \
-  return __builtin_shuffle (x, y, (INDEX_##nunits) MASK_##nunits); \
+/*
+** permute:
+** ptrue   (p[0-7])\.h, vl16
+** sel z0\.b, \1, z0\.b, z1\.b
+** ret
+*/
+__SVInt8_t
+permute (__SVInt8_t x, __SVInt8_t y)
+{
+  return __builtin_shuffle ((vnx16qi) x, (vnx16qi) y, (vnx16qi) MASK_32);
 }
-
-PERMUTE(vnx16qi, 32)
-
-/* { dg-final { scan-assembler-not {\ttbl\t} } } */
-
-/* { dg-final { scan-assembler-times {\tsel\tz[0-9]+\.b, p[0-9]+, z[0-9]+\.b, 
z[0-9]+\.b\n} 1 } } */
-/* { dg-final { scan-assembler-times {\tptrue\tp[0-9]+\.h, vl16\n} 1 } } */
Index: gcc/testsuite/gcc.target/aarch64/sve/sel_2.c
===
--- gcc/testsuite/gcc.target/aarch64/sve/sel_2.c2019-08-25 
19:10:33.442172145 +0100
+++ gcc/testsuite/gcc.target/aarch64/sve/sel_2.c2020-01-10 
16:30:07.989668567 +
@@ -1,14 +1,13 @@
 /* { dg-do assemble { target aarch64_asm_sve_ok } } */
 /* { dg-options "-O2 -msve-vector-bits=256 --save-temps" } */
+/* { dg-final { check-function-bodies "**" "" } } */
 
 #include 
 
 typedef int8_t vnx16qi __attribute__((vector_size (32)));
 typedef int16_t vnx8hi __attribute__((vector_size (32)));
-typedef int32_t vnx4si __attribute__((vector_size (32)));
 
 typedef _Float16 vnx8hf __attribute__((vector_size (32)));
-typedef float vnx4sf __attribute__((vector_size (32)));
 
 /* Predicate vector: 1 0 0 0 ... */
 
@@ -20,22 +19,39 @@ #define MASK_32 { 0, 33, 34, 35, 4, 37,
 
 #define MASK_16{0, 17, 2, 19, 4, 21, 6, 23, 8, 25, 10, 27, 12, 
29, 14, 31}
 
-#define INDEX_32 vnx16qi
-#define INDEX_16 vnx8hi
-
-#define PERMUTE(type, nunits)  \
-type permute_##type (type x, type y)   \
-{  \
-  return __builtin_shuffle (x, y, (INDEX_##nunits) MASK_##nunits); \
+/*
+** permute_vnx16qi:
+** ptrue   (p[0-7])\.s, vl8
+** sel z0\.b, \1, z0\.b, z1\.b
+** ret
+*/
+__SVInt8_t
+permute_vnx16qi (__SVInt8_t x, __SVInt8_t y)
+{
+  return __builtin_shuffle ((vnx16qi) x, (vnx16qi) y, (vnx16qi) MASK_32);
 }
 
-PERMUTE(vnx16qi, 32)
-PERMUTE(vnx8hi, 16)
-PERMUTE(vnx8hf, 16)
-
-/* { dg-final { scan-assembler-not {\ttbl\t} } } */
-
-/* { dg-final { scan-assembler-times {\tsel\tz[0-9]+\.b, p[0-9]+, z[0-9]+\.b, 
z[0-9]+\.b\n} 1 } } */
-/* { dg-final { scan-assembler-times {\tsel\tz[0-9]+\.h, p[0-9]+, z[0-9]+\.h, 
z[0-9]+\.h\

[OpenACC] bump version for 2.6 plus libgomp.texi update — document acc_attach/acc_detach, acc_*_async, acc_*_finalize(_async) functions

[Tobias Burnus]

I believe except for bugs and known omissions (e.g. PR93225+93226), the 
GCC 10 trunk implementation is complete – and the version number can be 
bumped from 2.0 (alias 201306) to OpenACC 2.6 (alias 201711).


That's what this patch does (i.e. applying the previously mentioned OG9 
patch).


It also includes the previous patch, i.e. the addition of the missing 
acc_*_async and acc_*_finalize prototypes.


Additionally, I added the missing documentation for 
acc_attach/acc_detach. — And I did not include the following wording, 
which the OG9 patch added: "This list has not yet been updated for the 
OpenACC specification in version 2.6."


OK for the trunk?

Tobias

PS: Build succeeded but regtesting is still ongoing.

On 1/8/20 5:07 PM, Tobias Burnus wrote:
When looking at libgomp.texi the other day, I saw that the acc_*_async 
variants and the acc_*_finalize functions of OpenACC 2.5 were not 
documented.


Hence, this patch adds them. Those are part of OpenACC 2.5, hence, I 
updated the @ref (but referenced to OpenACC 2.6 instead).


Possible variants:
(a) update all acc_* calls to OpenACC 2.6 @refs
(b) defer updating the @ref until the OpenACC version is bumped from 
2.0 (alias 201306) to OpenACC 2.6 (alias 201711). [Cf. OG9 branch's 
7a22697197b85931d9fda66e8b0f75171ea13b43]
(c) Independent of the @ref: write the variable-type declarations for 
Fortran en bloc after all the "subroutine" as they are the same – 
especially useful for acc_copyout* which has 8 variants. That's how 
OpenACC 2.7's spec does it.


Regarding (c): If one goes for that change, does one keep the 
"INTERFACE" string in the table for each "subroutine" line? And what 
do to about the variable-declaration lines? Adding a single 
"ARGUMENTS" before the first of those (i.e. in the "a" line)?


Comments, suggestions, approval?

Tobias

2020-01-10  Julian Brown  
	Tobias Burnus  

	gcc/c-family/
	* c-cppbuiltin.c (c_cpp_builtins): Update _OPENACC define to 201711.
 
	gcc/doc/
	* invoke.texi: Update mention of OpenACC version to 2.6.

	gcc/fortran/
	* cpp.c (cpp_define_builtins): Update _OPENACC define to 201711.
	* gfortran.texi: Update mentions of OpenACC version to 2.6.
	* intrinsic.texi: Likewise.

	gcc/testsuite/
	* c-c++-common/cpp/openacc-define-3.c: Update expected value for
	_OPENACC define.
	* gfortran.dg/openacc-define-3.f90: Likewise.

	libgomp/
	* libgomp.texi (OpenACC Runtime Library Routines): Document *_async
	and *_finalize variants; document acc_attach and acc_detach; update
	references from OpenACC 2.0 to 2.6.
	* openacc.f90 (openacc_version): Update to 201711.
	* openacc_lib.h (openacc_version): Update to 201711.
	* testsuite/libgomp.oacc-fortran/openacc_version-1.f: Update expected
	openacc_version to 201711.
	* testsuite/libgomp.oacc-fortran/openacc_version-2.f90: Likewise.


 gcc/c-family/c-cppbuiltin.c|   2 +-
 gcc/doc/invoke.texi|   2 +-
 gcc/fortran/cpp.c  |   2 +-
 gcc/fortran/gfortran.texi  |   8 +-
 gcc/fortran/intrinsic.texi |   6 +-
 gcc/testsuite/c-c++-common/cpp/openacc-define-3.c  |   2 +-
 gcc/testsuite/gfortran.dg/openacc-define-3.f90 |   2 +-
 libgomp/libgomp.texi   | 257 +++--
 libgomp/openacc.f90|   2 +-
 libgomp/openacc_lib.h  |   2 +-
 .../libgomp.oacc-fortran/openacc_version-1.f   |   2 +-
 .../libgomp.oacc-fortran/openacc_version-2.f90 |   2 +-
 12 files changed, 206 insertions(+), 83 deletions(-)

diff --git a/gcc/c-family/c-cppbuiltin.c b/gcc/c-family/c-cppbuiltin.c
index cb869415167..e3fd9f9b52a 100644
--- a/gcc/c-family/c-cppbuiltin.c
+++ b/gcc/c-family/c-cppbuiltin.c
@@ -1439,7 +1439,7 @@ c_cpp_builtins (cpp_reader *pfile)
 cpp_define (pfile, "__SSP__=1");
 
   if (flag_openacc)
-cpp_define (pfile, "_OPENACC=201306");
+cpp_define (pfile, "_OPENACC=201711");
 
   if (flag_openmp)
 cpp_define (pfile, "_OPENMP=201511");
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 53df4b1fdf9..70abcbabec7 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -2188,7 +2188,7 @@ freestanding and hosted environments.
 Enable handling of OpenACC directives @code{#pragma acc} in C/C++ and
 @code{!$acc} in Fortran.  When @option{-fopenacc} is specified, the
 compiler generates accelerated code according to the OpenACC Application
-Programming Interface v2.0 @w{@uref{https://www.openacc.org}}.  This option
+Programming Interface v2.6 @w{@uref{https://www.openacc.org}}.  This option
 implies @option{-pthread}, and thus is only supported on targets that
 have support for @option{-pthread}.
 
diff --git a/gcc/fortran/cpp.c b/gcc/fortran/cpp.c
index 8ab46986ae9..dcde5576cd5 100644
--- a/gcc/fortran/cpp.c
+++ b/gcc/fortran/cpp.c
@@ -166,7 +166,7 @@ cpp_define_builtins (cpp_reader *pfile)
   cpp_define (pfile, "_LANGUAGE_FORTRAN=1");
 

Re: [PATCH 13/41] analyzer: command-line options

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Needs review.  msebor expressed some concerns in an earlier version
> of the patch here:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00233.html
> re overlap with existing functions, and very long names.
> For the former, they all have a "-Wanalyzer-" prefix to
> distinguish them, and for the latter, I prefer the precision
> of the longer names, but tastes may vary.
> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> - fix a stray reference to plugins
> 
> Changed in v4:
> - Renamed gcc/analyzer/plugin.opt to gcc/analyzer/analyzer.opt
> 
> - Change option from -analyzer to -fanalyzer, changed it from
>   Driver to Common.
> 
> - Various commits on 2019-11-12 including r278083 through r278087
>   reimplemented parameter-handling in terms of options, so that
>   params are defined in params.opt rather than params.def.
> 
>   This patch adds the params for the analyzer to analyzer.opt,
>   replacing the patch:
> [PATCH 22/49] analyzer: params.def: new parameters
>   https://gcc.gnu.org/ml/gcc-patches/2019-11/msg01520.html
>   from the original version of the patch kit.
> 
> - Added -Wanalyzer-unsafe-call-within-signal-handler from
> https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00214.html
> 
> This patch contains the command-line options for the analyzer.
> 
> gcc/analyzer/ChangeLog:
>   * analyzer.opt: New file.
> 
> gcc/ChangeLog:
>   * common.opt (-fanalyzer): New driver option.
So I like having the diagnostics in a different namespace.  THat's
inherently going to make their names longer.  But yea, the names are a
mouthfull.   I also think that with the analyzer support still being
preliminary that we have degrees of freedom to rename options, so I
don't think that the lengths of the options should be a major blocker
to moving this work forward.

WRT the overlap with existing warnings.  I think there is going to be
degrees of overlap.  The question we're going to have to continually
evaluate is where any particular warning belongs.

We're still early, but I suspect a significant factor in that decision
will be the degree of property tracking across calls/returns that's
needed to have a useful warning.  ISTM that once we have to track info
across calls, then we're into the static analyzer and the higher
overhead that's inherent in that style.

So I'm inclined to ACK, knowing that we may end up coming back and
renaming options related to the analyzer and that we're likely going to
have cases where there is some degree of overlap between the analyzer
and the older diagnostics.

jeff
> 

Re: [PATCH 29/41] analyzer: new file: sm-signal.cc

[David Malcolm]

On Fri, 2020-01-10 at 09:01 -0700, Jeff Law wrote:
> On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> > Needs review.
> > 
> > Although the comments say "experimental", there are followups
> > (using
> > function_set) which make this much more production-ready than the
> > other sm-*.cc (other than malloc).
> > 
> > Changed in v5:
> > - update ChangeLog path
> > - updated copyright years to include 2020
> > 
> > New in v4; part of:
> >   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00214.html
> > with various fixups
> > 
> > gcc/analyzer/ChangeLog:
> > * sm-signal.cc: New file.
> OK. 

Thanks.

> Extending this one seems pretty trivial -- it's just checking for
> specific functions we know aren't safe, right?

Indeed, and I've done some followup work on this in the branch:

  "[PATCH 0/4] analyzer: add class function_set and use in various places"
 https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01422.html

(plus I have experimental code in my working copy that can load such
sets on demand from data files, to avoid baking them into the analyzer,
or relying on attributes in old header files)

Dave

Re: [PATCH] testsuite: add lib/nn-line-numbers.exp

[David Malcolm]

On Fri, 2020-01-10 at 08:35 -0700, Jeff Law wrote:
> On Wed, 2020-01-08 at 17:07 -0500, David Malcolm wrote:
> > (replying to my own "[PATCH 05/41] Add -fdiagnostics-nn-line-
> > numbers"
> > with a followup that does it at the DejaGnu level rather than as a
> > test-only option)
> > 
> > On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> > > I may be able to self-approve this.  It's used by the
> > > diagnostic_path
> > > patch, and by the analyzer test suite.  Perhaps better to make
> > > undocumeted, or do it via a DejaGnu pruning directive, but I
> > > wanted
> > > to get v5 of the kit posted.
> > > 
> > > This patch implements -fdiagnostics-nn-line-numbers, a new option
> > > which makes diagnostic_show_locus print "NN" rather than specific
> > > line numbers when printing the left margin.
> > > 
> > > This is intended purely to make it easier to write certain kinds
> > > of
> > > DejaGnu test; various integration tests for diagnostic paths
> > > later
> > > in the patch kit make use of it.
> > > 
> > > gcc/ChangeLog:
> > >   * common.opt (fdiagnostics-nn-line-numbers): New option.
> > >   * diagnostic-show-locus.c
> > > (layout::m_use_nn_for_line_numbers_p):
> > >   New field.
> > >   (layout::layout): Initialize it.
> > >   (layout::calculate_linenum_width): Use it when computing
> > >   m_linenum_width.
> > >   (layout::print_source_line): Implement printing "NN" rather
> > > than
> > >   the line number.
> > >   (selftest::test_line_numbers_multiline_range): Add a test of
> > > "NN"
> > >   printing.
> > >   * diagnostic.c (diagnostic_initialize): Initialize
> > >   use_nn_for_line_numbers_p.
> > >   (num_digits): Add "use_nn_p" param.
> > >   (selftest::test_num_digits): Add a test for use_nn_p==true.
> > >   * diagnostic.h (diagnostic_context::use_nn_for_line_numbers_p):
> > >   New field.
> > >   (num_digits): Add optional "use_nn_p" param.
> > >   * doc/invoke.texi (-fdiagnostics-nn-line-numbers): New option.
> > >   * dwarf2out.c (gen_producer_string): Ignore
> > >   OPT_fdiagnostics_nn_line_numbers.
> > >   * lto-wrapper.c (merge_and_complain): Handle
> > >   OPT_fdiagnostics_nn_line_numbers.
> > >   (append_compiler_options): Likewise.
> > >   (append_diag_options): Likewise.
> > >   * opts.c (common_handle_option): Likewise.
> > >   * toplev.c (general_init): Initialize
> > >   global_dc->use_nn_for_line_numbers_p.
> > 
> > Here's an alterative patch to the above that replaces the
> > "-fdiagnostics-nn-line-numbers" option in earlier versions of the
> > analyzer patch kit, by doing it at the DejaGnu level instead.
> > 
> > This patch adds support for obscuring the line numbers printed in
> > the
> > left-hand margin when printing the source code, converting them to
> > NN,
> > e.g from:
> > 
> >   7111 |   if (!(flags & 0x0001)) {
> >|  ^
> >|  |
> >|  (1) following 'true' branch...
> >   7112 |
> > 
> > to:
> > 
> >NN  |   if (!(flags & 0x0001)) {
> >|  ^
> >|  |
> >|  (1) following 'true' branch...
> >NN  |
> > 
> > This is useful in followup patches e.g. when testing how
> > interprocedural
> > paths are printed using multiline.exp, to avoid depending on
> > precise line
> > numbers.
> > 
> > I'm testing this now (but it seems to be a working, drop-in
> > replacement
> > for the option in the parts of the patch kit I've tested with it).
> > 
> > Examples of use can be seen in the analyzer test suite:
> >   https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00320.html
> > (search for -fdiagnostics-nn-line-numbers and dg-begin-multiline-
> > output
> > there to get the idea)
> > 
> > OK for trunk assuming the other testing looks good?
> > 
> > gcc/testsuite/ChangeLog:
> > * lib/gcc-dg.exp (cleanup-after-saved-dg-test): Reset global
> > nn_line_numbers_enabled.
> > * lib/nn-line-numbers.exp: New file.
> > * lib/prune.exp: Load nn-line-numbers.exp.
> > (prune_gcc_output): Call maybe-handle-nn-line-numbers.
> OK
> jeff

Thanks.  It turned out that the above patch didn't work; as noted in:
  [PATCH] testsuite: add dg-enable-nn-line-numbers (v6)
https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00517.html
it turns out that the non-gcc parts of the testsuite that use prune.exp
need to jump through some hoops due to Tcl lib path issues needing to
preload libs by hacking in calls to load_gcc_lib, and at least one
(libgo.exp) is a mirrored copy that we're not the canonical copy of.

Given that this is meant to be used in conjunction with multiline.exp
the simplest fix was to move the code into multiline.exp, which I did
in the latter patch.

Presumably that latter version is also OK?  (it has the advantage of
actually successfully running...)

Thanks
Dave

Re: [PATCH 24/41] analyzer: new files: sm.{cc|h}

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> The v1 version of this patch was reviewed by Jeff here:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00805.html
> TODO: looks like I still need to act on some of his comments there
> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> 
> Changed in v4:
> - Remove include of gcc-plugin.h, reworking includes accordingly.
> - Wrap everything in #if ENABLE_ANALYZER
> - Remove /// comment lines
> - Add call to make_signal_state_machine:
> https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00214.html
> - Rework on_leak vfunc:
> https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02028.html
> - Add DISABLE_COPY_AND_ASSIGN to state_machine
> - Add support for global states and custom transitions:
> https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00217.html
> 
> This patch adds a "state_machine" base class for describing
> API checkers in terms of state machine transitions.  Followup
> patches use this to add specific API checkers.
> 
> gcc/analyzer/ChangeLog:
>   * sm.cc: New file.
>   * sm.h: New file.
So I think my original review comments stand.  Once those are
addressed, this is fine for the trunk.
jeff
> 

Re: [PATCH][arm][backport] arm: fix v[78]-r multilibs when configured with --with-multlib-list=aprofile

[Richard Earnshaw (lists)]

On 10/01/2020 14:30, Przemyslaw Wirkus wrote:

Hi,
When gcc for Arm is configured with --with-multilib-list=aprofile a
misplaced endif directive in the makefile was causing the arm->thumb
mapping for multilibs to be omitted from the reuse rules.  This
resulted in the default multilib being picked rather than the thumb2
opimized version.

gcc/ChangeLog:
2020-01-10  Przemyslaw Wirkus  

Backport from trunk
* config/arm/t-multilib: Use arm->thumb multilib reuse rules
on a-profile.

Ok for gcc-9-branch?

kind regards
Przemyslaw Wirkus



Pushed.

R.

Re: [PATCH][arm] [backport] arm: Fix rmprofile multilibs when architecture includes +mp or +sec (PR target/93188)

[Richard Earnshaw (lists)]

On 10/01/2020 14:31, Przemyslaw Wirkus wrote:

Hi,
When only the rmprofile multilibs are built, compiling for armv7-a
should select the generic v7 multilibs.  This used to work before +sec
and +mp were added to the architecture options but it was broken by
that update.  This patch fixes those variants and adds some tests to
ensure that they remain fixed ;-)

gcc/ChangeLog:
2020-01-08  Przemyslaw Wirkus  

Backport from trunk
PR target/93188
* config/arm/t-multilib (MULTILIB_MATCHES): Add rules to match
armv7-a{+mp,+sec,+mp+sec} to appropriate armv7 multilib variants
when only building rm-profile multilibs.

gcc/testsuite/ChangeLog:
2020-01-08  Przemyslaw Wirkus  

Backport from trunk
* gcc.target/arm/multilib.exp: Add new tests for rm-profile only.

Ok for gcc-9-branch?

kind regards
Przemyslaw Wirkus



Pushed.

Re: [PATCH 07/41] Add ordered_hash_map

[Jeff Law]

On Fri, 2020-01-10 at 11:30 -0500, David Malcolm wrote:
> On Fri, 2020-01-10 at 09:22 -0700, Jeff Law wrote:
> > On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> > > Needs review.  This is used in many places in the analyzer.
> > > msebor made some comments about the v1 version of this patch here:
> > >   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00231.html
> > > 
> > > Changed in v5:
> > > - updated copyright years to include 2020
> > > 
> > > This patch adds an ordered_hash_map template, which is similar to
> > > hash_map, but preserves insertion order.
> > > 
> > > gcc/ChangeLog:
> > >   * Makefile.in (OBJS): Add ordered-hash-map-tests.o.
> > >   * ordered-hash-map-tests.cc: New file.
> > >   * ordered-hash-map.h: New file.
> > >   * selftest-run-tests.c (selftest::run_tests): Call
> > >   selftest::ordered_hash_map_tests_cc_tests.
> > >   * selftest.h (selftest::ordered_hash_map_tests_cc_tests): New
> > >   decl.
> > There's nothing inherent in the data that would preclude us from
> > using
> > a standard container (ie, nothing with embedded GC based on our
> > meeting
> > Tuesday).
> 
> Correct: this doesn't support our GC, but doesn't need to for the uses
> I'm making of it.
> 
> >   But there isn't an existing standard container with the
> > right properties (based on email between you and Jon).  Right?
> 
> Correct; std::map uses Key ordering to build a red-black tree; I'm
> using insertion ordering (not Key ordering), to get more deterministic
> results in the face of ptr values that can change from under me.  It's
> analogous to Python's OrderedDict, fwiw.
> 
> > Do you need a private assignment operator?
> 
> I added that in:
>   https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00518.html
> to ensure that we don't erroneously use the compiler-generated one;
> that's the latest version of this patch.
I must have missed it when rescanning the code.  Thanks for pointing it
out.

OK for the trunk.

jeff

Re: [PATCH 25/41] analyzer: new files: sm-malloc.cc and sm-malloc.dot

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Needs review.
> 
> Re the v1 version of this patch Jeff asked in:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00506.html
> > This goes well beyond what we were originally targeting -- which begs
> > the question, what's the state of the other checkers in here?
> Jeff: I thought I had responded to that by discussing the other sm-*.cc
> files but I now realize you may have been referring to the warnings
> other than double-free within sm-malloc.cc.  The warnings within
> sm-malloc.cc are in pretty good shape, as is the warning in
> sm-signal.cc.  Everything else is a lot less mature.  (If we had
> to pick a subset of warnings for the initial release, I'd pick
> everything in sm-malloc.cc plus sm-signal.cc)
> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> 
> Changed in v4:
> - Remove include of gcc-plugin.h, reworking includes accordingly.
> - Wrap everything in #if ENABLE_ANALYZER
> - Remove /// comment lines
> - Rework on_leak vfunc:
> https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02028.html
> - Rework for changes to is_named_call_p, resolving function pointers:
>https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00178.html
> - Support the "__builtin_"-prefixed spellings of malloc, calloc and free
> - Add malloc.dot
> 
> This patch adds a state machine checker for malloc/free.
> 
> gcc/analyzer/ChangeLog:
>   * sm-malloc.cc: New file.
>   * sm-malloc.dot: New file.
I don't immediately recall if I was referring to the other checkers in
the total patchkit or those within sm-malloc.

I think my original point about someone using this as a template for
other checkers still stands -- probably because sm-malloc appears to be
the most complete/complex.  Of course that's what I told you to focus
on, so that's not a real surprise.

It's OK for the trunk IMHO.
jeff

Re: [PATCH 39/41] analyzer: new files: diagnostic-manager.{cc|h}

[Jeff Law]

On Wed, 2020-01-08 at 04:03 -0500, David Malcolm wrote:
> Needs review.  Jeff reviewed the v1 version of the patch here:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00818.html
> requesting a function to be split up, which I did in v4.
> See the URLs below for notes on the other changes.
Just an FYI, thanks for those pointers to the earlier discussions. 
They're really handy for refreshing my memory.


> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> 
> Changed in v4:
> - Remove include of gcc-plugin.h, reworking includes accordingly.
> - Wrap everything in #if ENABLE_ANALYZER
> - Remove /// comment lines
> - Add custom events:
> https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00213.html
> - Generalize rewind_info_t to exploded_edge::custom_info_t
> https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00219.html
> - Add support for global state:
> https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00217.html
> - Show rewind destination for leaks due to longjmp
> https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02029.html
> - Split diagnostic_manager::prune_path into subroutines
> - Add DISABLE_COPY_AND_ASSIGN (saved_diagnostic);
> 
> This patch adds diagnostic_manager and related support classes for
> saving, deduplicating, and emitting analyzer diagnostics.
> 
> gcc/analyzer/ChangeLog:
>   * diagnostic-manager.cc: New file.
>   * diagnostic-manager.h: New file.
OK
jeff

Re: [PATCH 19/41] analyzer: new files: analyzer.{cc|h}

[David Malcolm]

On Fri, 2020-01-10 at 08:53 -0700, Jeff Law wrote:
> On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> > Jeff reviewed an earlier version of this here:
> >   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00503.html
> > My response:
> >   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00809.html
> > I have followup patches that implement the function_set idea.
> > 
> > TODO:
> >   I haven't yet addressed the is_setjmp_call_p/is_longjmp_call_p
> > concerns
> > 
> > Changed in v5:
> > - update ChangeLog path
> > - updated copyright years to include 2020
> > 
> > Changed in v4:
> > - Remove include of gcc-plugin.h, reworking includes accordingly.
> > - Wrap everything in #if ENABLE_ANALYZER
> > - Remove /// comment lines
> > - Update is_named_call_p to support function pointers:
> > https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00178.html
> > 
> > gcc/analyzer/ChangeLog:
> > * analyzer.cc: New file.
> > * analyzer.h: New file.
> So there's a lot more ways to write setjmp.  Some don't even look
> like
> setjmp.  sigsetjmp, savectx, vfork, getcontext all have setjmp like
> qualities (returning twice).  They may have an underscore prefix
> IIRC.
> 
> So if what you're really looking for is functions that may return
> twice, I think that's the canonical list :-)

Thanks - but I don't think "returns twice" is exactly what's needed
here.

I think the needs of code generation/optimization are slightly
different from that of the analyzer here.

I'm special-casing setjmp/longjmp, which have very particular behavior.
Beyond the "returns twice" property the analyzer has special-case
knowledge of things like:
  "the return value from the 2nd return from setjmp is that passed to
longjmp, unless 0 was passed, in which case return 1"

That holds for sigsetjmp/siglongjmp, but not for the other functions,
which have their own behaviors (e.g. getcontext/setcontext can fail and
set errno).

So from the analyzer's point-of-view, those various other functions are
*not* setjmp/longjmp; they are different returns-twice functions.

I picked setjmp/longjmp as being high-priority to model correctly. 
Some of the others in the list you give are rather obscure e.g. savectx
appears to be Solaris-specific and I have no great desire to track down
Solaris documentation and figure out what its behaviors are meant to
be.

So I think this code could be generalized to sigsetjmp/siglongjmp
fairly easily using the setjmp/longjmp logic.  For the other returns-
twice functions, the analyzer's not handling the 2nd return will mean
it will silently fail to explore execution paths involving the 2nd
return, which seems acceptable to me (otherwise there could be state
assumptions we fail to inject, leading to false warnings on paths that
are actually-infeasible).

Dave

Re: [PATCH 38/41] analyzer: new files: checker-path.{cc|h}

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Jeff approved the v1 version of the patch here:
>   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00820.html
> There are some non-trivial changes in the followups (see the URLs
> below).
> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> 
> Changed in v4:
> - Remove include of gcc-plugin.h, reworking includes accordingly.
> - Wrap everything in #if ENABLE_ANALYZER
> - Remove /// comment lines
> - Add custom events:
> https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00213.html
> - Add support for global state:
> https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00217.html
> - start_cfg_edge_event::maybe_describe_condition: special-case the description
>   of edges based on the result of strcmp
> - Generalize rewind_info_t to exploded_edge::custom_info_t
> https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00219.html
> - Add checker_path::debug
> https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02033.html
> 
> This patch adds a family of classes for representing paths of events
> for analyzer diagnostics.
> 
> gcc/analyzer/ChangeLog:
>   * checker-path.cc: New file.
>   * checker-path.h: New file.
OK.

jeff
> ---
> 

[committed] fix a logic error in attribute access validation (PR 93132)

[Martin Sebor]

The attached patch corrects a logic error in the validation
of the new attribute access where the code incorrectly expects
the human readable representation of the attribute to match
the terse internal representation of the positional argument.

Committed in ra280124 after bootstrapping it and running
the testsuite with no regressions.

Martin
PR c/93132 - bogus `attribute((access))' warning when size-index is specified

gcc/c-family/ChangeLog:

	PR c/93132
	* c-attribs.c (append_access_attrs): Validate against the translated
	access string rather than the human-readable representation.

gcc/testsuite/ChangeLog:

	PR c/93132
	* gcc.dg/attr-access-read-only-2.c: New test.

Index: gcc/c-family/c-attribs.c
===
--- gcc/c-family/c-attribs.c	(revision 280108)
+++ gcc/c-family/c-attribs.c	(working copy)
@@ -3970,14 +3970,15 @@ append_access_attrs (tree t, tree attrs, const cha
 	  return NULL_TREE;
 	}
 
-	  if (n2 && strncmp (attrstr + n1 + 1, pos + n1, n2))
+	  if (n2 && strncmp (attrspec + n1 + 1, pos + n1, n2))
 	{
 	  /* Mismatch in the value of the size argument.  */
 	  auto_diagnostic_group d;
 	  if (warning (OPT_Wattributes,
-			   "attribute %qs mismatch positional argument "
+			   "attribute %qs mismatched positional argument "
 			   "values %i and %i",
-			   attrstr, atoi (attrstr + n1 + 1), atoi (pos + n1))
+			   attrstr, atoi (attrspec + n1 + 1) + 1,
+			   atoi (pos + n1) + 1)
 		  && DECL_P (t))
 		inform (DECL_SOURCE_LOCATION (t),
 			"previous declaration here");
Index: gcc/testsuite/gcc.dg/attr-access-read-only-2.c
===
--- gcc/testsuite/gcc.dg/attr-access-read-only-2.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/attr-access-read-only-2.c	(working copy)
@@ -0,0 +1,16 @@
+/* PR c/93132 - bogus 'attribute((access))' warning when size-index
+   is specified
+   { dg-do compile }
+   { dg-options "-Wall" } */
+
+void __attribute__ ((access (read_only, 1, 5)))
+f (void*, int, int, int, int);   // { dg-message "previous declaration" }
+
+void __attribute__ ((access (read_only, 1, 3)))
+f (void*, int, int, int, int);   // { dg-warning "attribute 'access\\\(read_only, 1, 3\\\)' mismatched positional argument values 3 and 5" }
+
+void __attribute__ ((access (read_only, 1, 4)))
+f (void*, int, int, int, int);   // { dg-warning "attribute 'access\\\(read_only, 1, 4\\\)' mismatched positional argument values 4 and 5" }
+
+void __attribute__ ((access (read_only, 1, 5)))
+f (void*, int, int, int, int);

Re: [PATCH 35/41] analyzer: new file: exploded-graph.h

[David Malcolm]

On Fri, 2020-01-10 at 09:10 -0700, Jeff Law wrote:
> On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> > Jeff's initial review of v1 of this patch:
> >   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00813.html
> > I've addressed most of the issues he raised there.
> > TODO: should some structs be classes?
> > 
> > Changed in v5:
> > - update ChangeLog path
> > - updated copyright years to include 2020
> > 
> > Changed in v4:
> > - Remove /// comment lines
> > - Don't use multiple inheritance, instead adding a log_user member.
> > - Add more validation, part of:
> > https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02517.html
> > - Generalize rewind_info_t to exploded_edge::custom_info_t
> > https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00219.html
> > - Add DISABLE_COPY_AND_ASSIGN (exploded_node);
> > - Add DISABLE_COPY_AND_ASSIGN (exploded_edge);
> > - Add DISABLE_COPY_AND_ASSIGN (exploded_graph);
> > 
> > This patch adds exploded_graph and related classes, for managing
> > exploring paths through the user's code as a directed graph
> > of  pairs.
> > 
> > gcc/analyzer/ChangeLog:
> > * exploded-graph.h: New file.
> So I'm not going to insist on the struct/class change.  It was
> something I wanted you self-review and consider.  If you end up
> changing them, consider that pre-approved.

Jeff: sorry, just to clarify, is that an "OK" for the patch?
(given how much needed fixing in the earlier version).

Thanks
Dave

Re: [PATCH 35/41] analyzer: new file: exploded-graph.h

[Jeff Law]

On Fri, 2020-01-10 at 12:18 -0500, David Malcolm wrote:
> On Fri, 2020-01-10 at 09:10 -0700, Jeff Law wrote:
> > On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> > > Jeff's initial review of v1 of this patch:
> > >   https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00813.html
> > > I've addressed most of the issues he raised there.
> > > TODO: should some structs be classes?
> > > 
> > > Changed in v5:
> > > - update ChangeLog path
> > > - updated copyright years to include 2020
> > > 
> > > Changed in v4:
> > > - Remove /// comment lines
> > > - Don't use multiple inheritance, instead adding a log_user member.
> > > - Add more validation, part of:
> > > https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02517.html
> > > - Generalize rewind_info_t to exploded_edge::custom_info_t
> > > https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00219.html
> > > - Add DISABLE_COPY_AND_ASSIGN (exploded_node);
> > > - Add DISABLE_COPY_AND_ASSIGN (exploded_edge);
> > > - Add DISABLE_COPY_AND_ASSIGN (exploded_graph);
> > > 
> > > This patch adds exploded_graph and related classes, for managing
> > > exploring paths through the user's code as a directed graph
> > > of  pairs.
> > > 
> > > gcc/analyzer/ChangeLog:
> > >   * exploded-graph.h: New file.
> > So I'm not going to insist on the struct/class change.  It was
> > something I wanted you self-review and consider.  If you end up
> > changing them, consider that pre-approved.
> 
> Jeff: sorry, just to clarify, is that an "OK" for the patch?
> (given how much needed fixing in the earlier version).
Yes, the patch is OK.  Sorry I wasn't clear.

jeff
> 

Re: [PATCH 18/41] analyzer: new files: supergraph.{cc|h}

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Needs review.
> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> 
> Changed in v4:
> - Rework includes to avoid gcc-plugin.h
> - Wrap everything with #if ENABLE_ANALYZER
> - Replace auto_client_timevar with TV_ANALYZER_SUPERGRAPH
> - Fix .dot output
>   https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02461.html
> - Update for move of digraph.h
> 
> This patch adds a "supergraph" class that combines CFGs and callgraph into
> one directed graph, along with "supernode" and "superedge" classes.
> 
> gcc/analyzer/ChangeLog:
>   * supergraph.cc: New file.
>   * supergraph.h: New file.
So in the back of my mind has always been how else can we use the
infrastructure you're designing & implementing.  I'd always hoped the
supergraph in particular would prove useful and it may.  But I just
realized something as I was looking at the implementation.

It appears that you copy phis/statements from basic blocks into the
supernodes.  That has important implications.  For example, we don't
have mechanisms to keep the two views in sync.  So if we need the
supergraph for something, we're probably going to need to rebuild it.

I don't think that implies we need to do anything now, just an
important restriction we need to keep in mind if we want to re-use some
of this stuff later.

I don't see any EH handling mechansisms.  I realize we haven't focused
on C++ and thus EH hasn't been the top of our minds, but are we going
to have to handle that at some point?


> diff --git a/gcc/analyzer/supergraph.h b/gcc/analyzer/supergraph.h
> new file mode 100644
> index ..81ea3ac08198
> --- /dev/null
> +++ b/gcc/analyzer/supergraph.h
> @@ -0,0 +1,564 @@
> +/* "Supergraph" classes that combine CFGs and callgraph into one digraph.
> +   Copyright (C) 2019-2020 Free Software Foundation, Inc.
> +   Contributed by David Malcolm .
> +
> +This file is part of GCC.
> +
> +GCC is free software; you can redistribute it and/or modify it
> +under the terms of the GNU General Public License as published by
> +the Free Software Foundation; either version 3, or (at your option)
> +any later version.
> +
> +GCC is distributed in the hope that it will be useful, but
> +WITHOUT ANY WARRANTY; without even the implied warranty of
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +General Public License for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with GCC; see the file COPYING3.  If not see
> +;.  */
> +
> +#ifndef GCC_ANALYZER_SUPERGRAPH_H
> +#define GCC_ANALYZER_SUPERGRAPH_H
> +
> +#include "ordered-hash-map.h"
> +#include "options.h"
> +#include "cgraph.h"
> +#include "function.h"
> +#include "cfg.h"
> +#include "basic-block.h"
> +#include "gimple.h"
> +#include "gimple-iterator.h"
> +#include "digraph.h"
Ugh.  Policy is to avoid doing this kind of thing.  Instead the
includes are supposed to be in the .cc files.  This can be fixed
without going through another review cycle.

OK with the #includes fixed.

jeff

Re: [PATCH 34/41] analyzer: new files: program-state.{cc|h}

[Jeff Law]

On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> Needs review.
> 
> Changed in v5:
> - update ChangeLog path
> - updated copyright years to include 2020
> 
> Changed in v4:
> - Remove include of gcc-plugin.h, reworking includes accordingly.
> - Wrap everything in #if ENABLE_ANALYZER
> - Remove /// comment lines
> - Add support for global state:
>   - https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00217.html
> - Rework logging to avoid exploded_graph multiple-inheritance (moving
>   log_user base to a member)
> 
> This patch introduces classes for tracking the state at a particular
> path of analysis.
> 
> gcc/analyzer/ChangeLog:
>   * program-state.cc: New file.
>   * program-state.h: New file.
> 
> 
> 
> 
> 
> diff --git a/gcc/analyzer/program-state.h b/gcc/analyzer/program-state.h
> new file mode 100644
> index ..be83d32b8833
> --- /dev/null
> +++ b/gcc/analyzer/program-state.h
> @@ -0,0 +1,365 @@
> +/* Classes for representing the state of interest at a given path of 
> analysis.
> +   Copyright (C) 2019-2020 Free Software Foundation, Inc.
> +   Contributed by David Malcolm .
> +
> +This file is part of GCC.
> +
> +GCC is free software; you can redistribute it and/or modify it
> +under the terms of the GNU General Public License as published by
> +the Free Software Foundation; either version 3, or (at your option)
> +any later version.
> +
> +GCC is distributed in the hope that it will be useful, but
> +WITHOUT ANY WARRANTY; without even the implied warranty of
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +General Public License for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with GCC; see the file COPYING3.  If not see
> +;.  */
> +
> +#ifndef GCC_ANALYZER_PROGRAM_STATE_H
> +#define GCC_ANALYZER_PROGRAM_STATE_H
> +
> +#include "analyzer/sm.h"
> +#include "analyzer/region-model.h"
Not as bad as the previously reviewed patch.  But let's see if we can
get those out of here and into the appropriate source files.

OK with the #includes moved to the right places.

jeff

Re: [PATCH 18/41] analyzer: new files: supergraph.{cc|h}

[David Malcolm]

On Fri, 2020-01-10 at 10:24 -0700, Jeff Law wrote:
> On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> > Needs review.
> > 
> > Changed in v5:
> > - update ChangeLog path
> > - updated copyright years to include 2020
> > 
> > Changed in v4:
> > - Rework includes to avoid gcc-plugin.h
> > - Wrap everything with #if ENABLE_ANALYZER
> > - Replace auto_client_timevar with TV_ANALYZER_SUPERGRAPH
> > - Fix .dot output
> >   https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02461.html
> > - Update for move of digraph.h
> > 
> > This patch adds a "supergraph" class that combines CFGs and
> > callgraph into
> > one directed graph, along with "supernode" and "superedge" classes.
> > 
> > gcc/analyzer/ChangeLog:
> > * supergraph.cc: New file.
> > * supergraph.h: New file.
> So in the back of my mind has always been how else can we use the
> infrastructure you're designing & implementing.  I'd always hoped the
> supergraph in particular would prove useful and it may.  But I just
> realized something as I was looking at the implementation.
> 
> It appears that you copy phis/statements from basic blocks into the
> supernodes.  That has important implications.  For example, we don't
> have mechanisms to keep the two views in sync.  So if we need the
> supergraph for something, we're probably going to need to rebuild it.
> 
> I don't think that implies we need to do anything now, just an
> important restriction we need to keep in mind if we want to re-use
> some
> of this stuff later.

Right - the analyzer pass builds a representation that's useful for it,
does a bunch of things to it, then tears it down; there's no
interaction with changes to the IR - but for the analyzer there doesn't
need to be.

> I don't see any EH handling mechansisms.  I realize we haven't
> focused
> on C++ and thus EH hasn't been the top of our minds, but are we going
> to have to handle that at some point?

Fair point.  The scope of the analyzer is C only right now, though I
think the only place I've documented that is in the internal docs,
where the Limitations section has "Only for C so far".  But I want to
support C++ in future releases.

> > diff --git a/gcc/analyzer/supergraph.h b/gcc/analyzer/supergraph.h
> > new file mode 100644
> > index ..81ea3ac08198
> > --- /dev/null
> > +++ b/gcc/analyzer/supergraph.h
> > @@ -0,0 +1,564 @@
> > +/* "Supergraph" classes that combine CFGs and callgraph into one
> > digraph.
> > +   Copyright (C) 2019-2020 Free Software Foundation, Inc.
> > +   Contributed by David Malcolm .
> > +
> > +This file is part of GCC.
> > +
> > +GCC is free software; you can redistribute it and/or modify it
> > +under the terms of the GNU General Public License as published by
> > +the Free Software Foundation; either version 3, or (at your
> > option)
> > +any later version.
> > +
> > +GCC is distributed in the hope that it will be useful, but
> > +WITHOUT ANY WARRANTY; without even the implied warranty of
> > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +General Public License for more details.
> > +
> > +You should have received a copy of the GNU General Public License
> > +along with GCC; see the file COPYING3.  If not see
> > +;;.  */
> > +
> > +#ifndef GCC_ANALYZER_SUPERGRAPH_H
> > +#define GCC_ANALYZER_SUPERGRAPH_H
> > +
> > +#include "ordered-hash-map.h"
> > +#include "options.h"
> > +#include "cgraph.h"
> > +#include "function.h"
> > +#include "cfg.h"
> > +#include "basic-block.h"
> > +#include "gimple.h"
> > +#include "gimple-iterator.h"
> > +#include "digraph.h"
> Ugh.  Policy is to avoid doing this kind of thing.  Instead the
> includes are supposed to be in the .cc files.  


FWIW, I've never understood the benefit of this policy; it feels like a
violation of the DRY principle, and I always find myself wondering
things like "what do I need to include before I can include gimple.h?".


But time is short; I'll copy and paste the includes.

> This can be fixed
> without going through another review cycle.
> 
> OK with the #includes fixed.

Thanks; will do.

> jeff
> 

Re: [PATCH 18/41] analyzer: new files: supergraph.{cc|h}

[Jeff Law]

On Fri, 2020-01-10 at 12:40 -0500, David Malcolm wrote:
> > 

> On Fri, 2020-01-10 at 10:24 -0700, Jeff Law wrote:
> > I don't see any EH handling mechansisms.  I realize we haven't
> > focused
> > on C++ and thus EH hasn't been the top of our minds, but are we going
> > to have to handle that at some point?
> 
> Fair point.  The scope of the analyzer is C only right now, though I
> think the only place I've documented that is in the internal docs,
> where the Limitations section has "Only for C so far".  But I want to
> support C++ in future releases.
I'd probably have to review the low level EH implementation internals. 
Worst case it's just  more edges where it's not immediately obvious
when/why/how those edges get traversed.


> 
> > > diff --git a/gcc/analyzer/supergraph.h b/gcc/analyzer/supergraph.h
> > > new file mode 100644
> > > index ..81ea3ac08198
> > > --- /dev/null
> > > +++ b/gcc/analyzer/supergraph.h
> > > @@ -0,0 +1,564 @@
> > > +/* "Supergraph" classes that combine CFGs and callgraph into one
> > > digraph.
> > > +   Copyright (C) 2019-2020 Free Software Foundation, Inc.
> > > +   Contributed by David Malcolm .
> > > +
> > > +This file is part of GCC.
> > > +
> > > +GCC is free software; you can redistribute it and/or modify it
> > > +under the terms of the GNU General Public License as published by
> > > +the Free Software Foundation; either version 3, or (at your
> > > option)
> > > +any later version.
> > > +
> > > +GCC is distributed in the hope that it will be useful, but
> > > +WITHOUT ANY WARRANTY; without even the implied warranty of
> > > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > > +General Public License for more details.
> > > +
> > > +You should have received a copy of the GNU General Public License
> > > +along with GCC; see the file COPYING3.  If not see
> > > +;;;.  */
> > > +
> > > +#ifndef GCC_ANALYZER_SUPERGRAPH_H
> > > +#define GCC_ANALYZER_SUPERGRAPH_H
> > > +
> > > +#include "ordered-hash-map.h"
> > > +#include "options.h"
> > > +#include "cgraph.h"
> > > +#include "function.h"
> > > +#include "cfg.h"
> > > +#include "basic-block.h"
> > > +#include "gimple.h"
> > > +#include "gimple-iterator.h"
> > > +#include "digraph.h"
> > Ugh.  Policy is to avoid doing this kind of thing.  Instead the
> > includes are supposed to be in the .cc files.  
> 
> 
> FWIW, I've never understood the benefit of this policy; it feels like a
> violation of the DRY principle, and I always find myself wondering
> things like "what do I need to include before I can include gimple.h?".
> 
> 
> But time is short; I'll copy and paste the includes.

It was to help us untangle the headers, which was effectively done a
few cycles ago.  What needs to happen now is a re-introduction of
*sane* inclusions.But that keeps getting pushed out.

Jeff

Re: [PATCH] testsuite: add lib/nn-line-numbers.exp

[Jeff Law]

On Fri, 2020-01-10 at 11:44 -0500, David Malcolm wrote:
> On Fri, 2020-01-10 at 08:35 -0700, Jeff Law wrote:
> > On Wed, 2020-01-08 at 17:07 -0500, David Malcolm wrote:
> > > (replying to my own "[PATCH 05/41] Add -fdiagnostics-nn-line-
> > > numbers"
> > > with a followup that does it at the DejaGnu level rather than as a
> > > test-only option)
> > > 
> > > On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote:
> > > > I may be able to self-approve this.  It's used by the
> > > > diagnostic_path
> > > > patch, and by the analyzer test suite.  Perhaps better to make
> > > > undocumeted, or do it via a DejaGnu pruning directive, but I
> > > > wanted
> > > > to get v5 of the kit posted.
> > > > 
> > > > This patch implements -fdiagnostics-nn-line-numbers, a new option
> > > > which makes diagnostic_show_locus print "NN" rather than specific
> > > > line numbers when printing the left margin.
> > > > 
> > > > This is intended purely to make it easier to write certain kinds
> > > > of
> > > > DejaGnu test; various integration tests for diagnostic paths
> > > > later
> > > > in the patch kit make use of it.
> > > > 
> > > > gcc/ChangeLog:
> > > > * common.opt (fdiagnostics-nn-line-numbers): New option.
> > > > * diagnostic-show-locus.c
> > > > (layout::m_use_nn_for_line_numbers_p):
> > > > New field.
> > > > (layout::layout): Initialize it.
> > > > (layout::calculate_linenum_width): Use it when computing
> > > > m_linenum_width.
> > > > (layout::print_source_line): Implement printing "NN" rather
> > > > than
> > > > the line number.
> > > > (selftest::test_line_numbers_multiline_range): Add a test of
> > > > "NN"
> > > > printing.
> > > > * diagnostic.c (diagnostic_initialize): Initialize
> > > > use_nn_for_line_numbers_p.
> > > > (num_digits): Add "use_nn_p" param.
> > > > (selftest::test_num_digits): Add a test for use_nn_p==true.
> > > > * diagnostic.h (diagnostic_context::use_nn_for_line_numbers_p):
> > > > New field.
> > > > (num_digits): Add optional "use_nn_p" param.
> > > > * doc/invoke.texi (-fdiagnostics-nn-line-numbers): New option.
> > > > * dwarf2out.c (gen_producer_string): Ignore
> > > > OPT_fdiagnostics_nn_line_numbers.
> > > > * lto-wrapper.c (merge_and_complain): Handle
> > > > OPT_fdiagnostics_nn_line_numbers.
> > > > (append_compiler_options): Likewise.
> > > > (append_diag_options): Likewise.
> > > > * opts.c (common_handle_option): Likewise.
> > > > * toplev.c (general_init): Initialize
> > > > global_dc->use_nn_for_line_numbers_p.
> > > 
> > > Here's an alterative patch to the above that replaces the
> > > "-fdiagnostics-nn-line-numbers" option in earlier versions of the
> > > analyzer patch kit, by doing it at the DejaGnu level instead.
> > > 
> > > This patch adds support for obscuring the line numbers printed in
> > > the
> > > left-hand margin when printing the source code, converting them to
> > > NN,
> > > e.g from:
> > > 
> > >   7111 |   if (!(flags & 0x0001)) {
> > >|  ^
> > >|  |
> > >|  (1) following 'true' branch...
> > >   7112 |
> > > 
> > > to:
> > > 
> > >NN  |   if (!(flags & 0x0001)) {
> > >|  ^
> > >|  |
> > >|  (1) following 'true' branch...
> > >NN  |
> > > 
> > > This is useful in followup patches e.g. when testing how
> > > interprocedural
> > > paths are printed using multiline.exp, to avoid depending on
> > > precise line
> > > numbers.
> > > 
> > > I'm testing this now (but it seems to be a working, drop-in
> > > replacement
> > > for the option in the parts of the patch kit I've tested with it).
> > > 
> > > Examples of use can be seen in the analyzer test suite:
> > >   https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00320.html
> > > (search for -fdiagnostics-nn-line-numbers and dg-begin-multiline-
> > > output
> > > there to get the idea)
> > > 
> > > OK for trunk assuming the other testing looks good?
> > > 
> > > gcc/testsuite/ChangeLog:
> > >   * lib/gcc-dg.exp (cleanup-after-saved-dg-test): Reset global
> > >   nn_line_numbers_enabled.
> > >   * lib/nn-line-numbers.exp: New file.
> > >   * lib/prune.exp: Load nn-line-numbers.exp.
> > >   (prune_gcc_output): Call maybe-handle-nn-line-numbers.
> > OK
> > jeff
> 
> Thanks.  It turned out that the above patch didn't work; as noted in:
>   [PATCH] testsuite: add dg-enable-nn-line-numbers (v6)
> https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00517.html
> it turns out that the non-gcc parts of the testsuite that use prune.exp
> need to jump through some hoops due to Tcl lib path issues needing to
> preload libs by hacking in calls to load_gcc_lib, and at least one
> (libgo.exp) is a mirrored copy that we're not the canonical copy of.
> 
> Given that this is meant to be used in conjunction with multiline.exp
> the simple