Re: [RFC] Come up with ipa passes introduction in gccint documentation

[Segher Boessenkool]

Hi!

On Mon, Sep 30, 2019 at 09:47:13AM +0800, luoxhu wrote:
> On 2019/9/30 00:17, Segher Boessenkool wrote:
> The updated output will be as below in gccint.pdf, references are valid to 
> jump over
> (suppose should be the same in info?):

> > Did you test this with both "make info" and "make pdf" (and checked the
> > result of those of course :-) )?

The reason I ask is, there often are little mistakes (in markup for example)
that are obvious in either rendered output or in the info reader (but not
(always) in both).  Not just the content, also the way things are laid out,
and what makes a link to what, etc.

> To simplify development, the GCC pass manager differentiates between normal 
> interprocedural
> passes see Section 9.4.2 [All regular IPA passes], page 127, small 
> inter-procedural
> passes see Section 9.4.1 [All small IPA passes], page 127 and late 
> inter-procedural passes see
> Section 9.4.3 [All late IPA passes], page 128. A small inter-procedural pass 
> (SIMPLE_IPA_

It is weird to have a different order here than the order of the actual
sections, for example.

> The reason for "small" is in passes.def, it is used as 
> INSERT_PASSES_AFTER (all_small_ipa_passes).  And in cgraphunit.c:
> ipa_passes (void)
> {
> ... 
>   execute_ipa_pass_list (passes->all_small_ipa_passes);
> ...
> }
> 
> So is it OK to use small here?

I don't mind if it is "small" or "simple", but we probably should use the
same name everywhere in the manual (or, if we use both names, explain that
they are the same thing).

There are many cases where the internal name (in the GCC source code, in
pass names, etc.) is different from the external name (in the manuals, in
option names, etc.)  It is hard to decide what to use in the internals
manual then :-)  Maybe the internal name should just be changed?

> One more thing to worry about is my poor English

It's perfectly good enough that we understand what you are saying :-)

> and I am not familiar with all the
> 30+ IPA passes so not easy to extract exact explanations for them. 

Yeah...  It's a good start to just have stubs for them, I guess?  Makes
it obvious things need to be filled out, so hopefully people will :-)


Segher

Re: [WIP PATCH] add object access attributes (PR 83859)

[Martin Sebor]

On 9/30/19 3:34 PM, Joseph Myers wrote:

On Sun, 29 Sep 2019, Martin Sebor wrote:


PR 83859 asks to expose the same checking that GCC does natively for
built-in calls via a function attribute that associates a pointer
argument with the size argument, such as:


I'll also note that, as mentioned in that bug (but more specifically
covered by the separate bug 50584) it would make sense to have similar
warnings with [static] parameter array declarators.  And that the C2x
charter includes a principle that new interfaces should have the array
size before the array parameter to allow parameters to be declared using
VLA syntax like that, so it's plausible the [static] case will be useful
for more functions in future (although it's up to library implementations
exactly what form they use in their headers, given they have other
considerations such as C++ compatibility).

Hopefully the infrastructure in this patch will facilitate future support
for such diagnostics in the [static] case.


Thanks for the reminder!  I will make sure the infrastructure makes
the VLA checking possible, even if it's not actually implemented for
GCC 10.

Martin

Re: [PATCH] rs6000: Fix PR91275

[Segher Boessenkool]

Hi Bill,

On Mon, Sep 30, 2019 at 08:24:09PM -0500, Bill Schmidt wrote:
> PR91275 observes that __builtin_crypto_vpmsumd fails to work properly
> with -O1 or higher with -mcpu=power8.  That combination spells swap
> optimization.  Indeed, all vpmsum* instructions were being accepted
> as swappable operations.  This is correct for all of them but vpmsumd,
> which creates a 128-bit result.

Yeah, that is obvious once you see it spelled out :-)

> The -std=gnu11 in the testcase is there to avoid errors about long long
> not being accepted with pure ANSI.  The "11" part is arbitrary.  The
> testcase is modified from the original bug report.

That is fine.

> This patch disallows swap optimization in the presence of vpmsumd.
> Bootstrapped and tested on powerpc64le-unknown-linux-gnu.  Is this okay
> for trunk, and for backport to all active branches after an appropriate
> waiting period?

Yes please.  Thanks,


Segher


> 2019-09-30  Bill Schmidt  
> 
>   * config/rs6000/rs6000-p8swap.c (rtx_is_swappable_p): Don't swap
>   vpmsumd.
> 
> [gcc/testsuite]
> 
> 2019-09-30  Bill Schmidt  
> 
>   * gcc.target/powerpc/pr91275.c: New.

build-failure for cris-elf with "[00/32] Support multiple ABIs in the same translation unit"

[Hans-Peter Nilsson]

> From: Richard Sandiford 
> Date: Wed, 11 Sep 2019 21:02:26 +0200

> This series of patches introduces some classes and helpers for handling
> multiple ABIs in the same translation unit.  At the moment "ABI" maans
> specifically the choice of call-clobbered registers
[...]

> The series also makes -fipa-ra work for partially-clobbered registers too.
[...]

My autotester for cris-elf complains about a build-breaking
commit in the revision range (working:breaking) 276299:276359
and a glance at those commits and the error message says the
cause is likely one of your commits.  Relevant part of
build-log, hopefully sufficient:

---
g++ -fno-PIE -c   -g -O2 -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE   
-fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing 
-Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual 
-pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings 
-fno-common  -DHAVE_CONFIG_H -I. -I. -I/x/hpautotest-gcc1/gcc/gcc 
-I/x/hpautotest-gcc1/gcc/gcc/. -I/x/hpautotest-gcc1/gcc/gcc/../include 
-I/x/hpautotest-gcc1/gcc/gcc/../libcpp/include 
-I/x/hpautotest-gcc1/cris-elf/gccobj/./gmp -I/x/hpautotest-gcc1/gcc/gmp 
-I/x/hpautotest-gcc1/cris-elf/gccobj/./mpfr/src 
-I/x/hpautotest-gcc1/gcc/mpfr/src -I/x/hpautotest-gcc1/gcc/mpc/src  
-I/x/hpautotest-gcc1/gcc/gcc/../libdecnumber 
-I/x/hpautotest-gcc1/gcc/gcc/../libdecnumber/dpd -I../libdecnumber 
-I/x/hpautotest-gcc1/gcc/gcc/../libbacktrace   -o caller-save.o -MT 
caller-save.o -MMD -MP -MF ./.deps/caller-save.TPo 
/x/hpautotest-gcc1/gcc/gcc/caller-save.c
In file included from /x/hpautotest-gcc1/gcc/gcc/caller-save.c:31:0:
/x/hpautotest-gcc1/gcc/gcc/caller-save.c: In function 'void init_caller_save()':
/x/hpautotest-gcc1/gcc/gcc/regs.h:195:44: error: cannot convert 'bool' to 
'const predefined_function_abi*' for argument '3' to 'machine_mode 
choose_hard_reg_mode(unsigned int, unsigned int, const 
predefined_function_abi*)'
   choose_hard_reg_mode (REGNO, NREGS, false)
^
/x/hpautotest-gcc1/gcc/gcc/caller-save.c:203:26: note: in expansion of macro 
'HARD_REGNO_CALLER_SAVE_MODE'
  regno_save_mode[i][j] = HARD_REGNO_CALLER_SAVE_MODE (i, j, VOIDmode);
  ^~~
/x/hpautotest-gcc1/gcc/gcc/caller-save.c: In function 'void 
save_call_clobbered_regs()':
/x/hpautotest-gcc1/gcc/gcc/regs.h:195:44: error: cannot convert 'bool' to 
'const predefined_function_abi*' for argument '3' to 'machine_mode 
choose_hard_reg_mode(unsigned int, unsigned int, const 
predefined_function_abi*)'
   choose_hard_reg_mode (REGNO, NREGS, false)
^
/x/hpautotest-gcc1/gcc/gcc/caller-save.c:821:12: note: in expansion of macro 
'HARD_REGNO_CALLER_SAVE_MODE'
 mode = HARD_REGNO_CALLER_SAVE_MODE
^~~
Makefile:1117: recipe for target 'caller-save.o' failed
---

> Also tested by compiling at least one target per CPU directory and
> checking for no new warnings.

(Hmm...  So maybe a host gcc issue?)

My host is x86-64 Debian 9, i.e. gcc-6.3.0.

brgds, H-P

Re: [PATCH, RS6000] Add movmemsi pattern for inline expansion of memmove()

[Segher Boessenkool]

Hi!

On Mon, Sep 30, 2019 at 11:36:31AM -0500, Aaron Sawdey wrote:
> This patch uses the support added in the patch I posted last week for 
> actually doing
> inline expansion of memmove().

> I've also removed the code from expand_block_move() for dealing with
> mode==BLKmode because I don't believe that can happen. The big if construct 
> that
> figures out which size we are going to use has a plain else on it, and every
> clause in it sets mode to something other than BLKmode. So I removed that code
> to simplify things and just left a gcc_assert(mode != BLKmode).

That looks fine, with that assert.

> 2019-09-30  Aaron Sawdey 
> 
>   * config/rs6000/rs6000-protos.h (expand_block_move): Change prototype.
>   * config/rs6000/rs6000-string.c (expand_block_move): Add might_overlap 
> parm.

This line is too long...  80 columns max.

>   * config/rs6000/rs6000.md (movmemsi): Add new pattern.
>   (cpymemsi): Add might_overlap parm to expand_block_move() call.


Okay for trunk.  Thanks!


Segher

[PATCH] rs6000: Fix PR91275

[Bill Schmidt]

Hi,

PR91275 observes that __builtin_crypto_vpmsumd fails to work properly
with -O1 or higher with -mcpu=power8.  That combination spells swap
optimization.  Indeed, all vpmsum* instructions were being accepted
as swappable operations.  This is correct for all of them but vpmsumd,
which creates a 128-bit result.

The -std=gnu11 in the testcase is there to avoid errors about long long
not being accepted with pure ANSI.  The "11" part is arbitrary.  The
testcase is modified from the original bug report.

This patch disallows swap optimization in the presence of vpmsumd.
Bootstrapped and tested on powerpc64le-unknown-linux-gnu.  Is this okay
for trunk, and for backport to all active branches after an appropriate
waiting period?

Thanks,
Bill


[gcc]

2019-09-30  Bill Schmidt  

* config/rs6000/rs6000-p8swap.c (rtx_is_swappable_p): Don't swap
vpmsumd.

[gcc/testsuite]

2019-09-30  Bill Schmidt  

* gcc.target/powerpc/pr91275.c: New.


Index: gcc/config/rs6000/rs6000-p8swap.c
===
--- gcc/config/rs6000/rs6000-p8swap.c   (revision 276360)
+++ gcc/config/rs6000/rs6000-p8swap.c   (working copy)
@@ -791,6 +791,11 @@ rtx_is_swappable_p (rtx op, unsigned int *special)
  case UNSPEC_REDUC_PLUS:
  case UNSPEC_REDUC:
return 1;
+ case UNSPEC_VPMSUM:
+   /* vpmsumd is not swappable, but vpmsum[bhw] are.  */
+   if (GET_MODE (op) == V2DImode)
+ return 0;
+   break;
  }
   }
 
Index: gcc/testsuite/gcc.target/powerpc/pr91275.c

===
--- gcc/testsuite/gcc.target/powerpc/pr91275.c  (nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/pr91275.c  (working copy)
@@ -0,0 +1,21 @@
+/* Test that we generate vpmsumd correctly without a swap error.  */
+
+/* { dg-do run { target { p8vector_hw } } } */
+/* { dg-options "-O2 -std=gnu11" } */
+
+#include 
+
+int main() {
+
+  const unsigned long long r0l = 0x8e7dfceac070e3a0;
+  vector unsigned long long r0 = (vector unsigned long long) {r0l, 0}, v;
+  const vector unsigned long long pd
+= (vector unsigned long) {0xc2LLU << 56, 0};
+
+  v = __builtin_crypto_vpmsumd ((vector unsigned long long) {r0[0], 0}, pd);
+
+  if (v[0] != 0x4000 || v[1] != 0x65bd7ab605a4a8ff)
+__builtin_abort ();
+
+  return 0;
+}

Re: [PATCH v3 4/9] S/390: Do not use signaling vector comparisons on z13

[Segher Boessenkool]

On Mon, Sep 30, 2019 at 03:36:41PM +0200, Ilya Leoshkevich wrote:
> > Am 06.09.2019 um 12:34 schrieb Segher Boessenkool 
> > :
> > Should you handle -fsignaling-nans here as well?
> 
> Do you mean disabling vectorisation of LE/LT/GE/GT/LTGT when
> -fsignaling-nans is in effect?  This makes sense to me.

I meant it sounds like you need to do *something* more than the generic
code does, here.

> I could do that
> here, but wouldn't common code (e.g. expand_vec_cond_expr_p) be a better
> place?

If it can be done there, all the better, sure.

Anyway, this is a separate issue, don't let me distract you :-)


Segher

Re: [PATCH] C testsuite, silence a FreeBSD libc warning

[Segher Boessenkool]

On Mon, Sep 30, 2019 at 07:47:54PM +0200, Jakub Jelinek wrote:
> So, can you try just
>{ dg-prune-output "warning: warning: \[^\n\r\]* possibly used unsafely; 
> consider using" } */
> or if that doesn't work, with .* at start end end?

Or even just

{ dg-prune-output {(?n)warning: warning: .* possibly used unsafely; consider 
using} } */

(?n) means to match single lines only; . and bracket expressions using ^
will not match newlines if you use it.

("warning: " twice, btw?)


Segher

Re: [SVE] PR91532

[Prathamesh Kulkarni]

On Wed, 25 Sep 2019 at 23:44, Richard Biener  wrote:
>
> On Wed, 25 Sep 2019, Prathamesh Kulkarni wrote:
>
> > On Fri, 20 Sep 2019 at 15:20, Jeff Law  wrote:
> > >
> > > On 9/19/19 10:19 AM, Prathamesh Kulkarni wrote:
> > > > Hi,
> > > > For PR91532, the dead store is trivially deleted if we place dse pass
> > > > between ifcvt and vect. Would it be OK to add another instance of dse 
> > > > there ?
> > > > Or should we add an ad-hoc "basic-block dse" sub-pass to ifcvt that
> > > > will clean up the dead store ?
> > > I'd hesitate to add another DSE pass.  If there's one nearby could we
> > > move the existing pass?
> > Well I think the nearest one is just after pass_warn_restrict. Not
> > sure if it's a good
> > idea to move it up from there ?
>
> You'll need it inbetween ifcvt and vect so it would be disabled
> w/o vectorization, so no, that doesn't work.
>
> ifcvt already invokes SEME region value-numbering so if we had
> MESE region DSE it could use that.  Not sure if you feel like
> refactoring DSE to work on regions - it currently uses a DOM
> walk which isn't suited for that.
>
> if-conversion has a little "local" dead predicate compute removal
> thingy (not that I like that), eventually it can be enhanced to
> do the DSE you want?  Eventually it should be moved after the local
> CSE invocation though.
Hi,
Thanks for the suggestions.
For now, would it be OK to do "dse" on loop header in
tree_if_conversion, as in the attached patch ?
The patch does local dse in a new function ifcvt_local_dse instead of
ifcvt_local_dce, because it needed to be done after RPO VN which
eliminates:
Removing dead stmt _ifc__62 = *_55;
and makes the following store dead:
*_55 = _ifc__61;

Thanks,
Prathamesh
>
> Richard.
diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c
index 40ad4c5947a..69dd9d88991 100644
--- a/gcc/tree-if-conv.c
+++ b/gcc/tree-if-conv.c
@@ -120,6 +120,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "fold-const.h"
 #include "tree-ssa-sccvn.h"
 #include "tree-cfgcleanup.h"
+#include "tree-ssa-dse.h"
 
 /* Only handle PHIs with no more arguments unless we are asked to by
simd pragma.  */
@@ -2975,6 +2976,25 @@ ifcvt_local_dce (basic_block bb)
 }
 }
 
+static void
+ifcvt_local_dse (basic_block bb)
+{
+  gimple_stmt_iterator gsi;
+  for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next ())
+{
+  gimple *stmt = gsi_stmt (gsi);
+  if (gimple_store_p (stmt))
+	{
+	  tree lhs = gimple_get_lhs (stmt);
+	  ao_ref write;
+	  ao_ref_init (, lhs);
+	  if (dse_classify_store (, stmt, false, NULL)
+	  == DSE_STORE_DEAD)
+	delete_dead_or_redundant_assignment (, "dead");
+	}
+}
+}
+
 /* If-convert LOOP when it is legal.  For the moment this pass has no
profitability analysis.  Returns non-zero todo flags when something
changed.  */
@@ -3071,6 +3091,7 @@ tree_if_conversion (class loop *loop, vec *preds)
   bitmap_set_bit (exit_bbs, single_exit (loop)->dest->index);
   bitmap_set_bit (exit_bbs, loop->latch->index);
   todo |= do_rpo_vn (cfun, loop_preheader_edge (loop), exit_bbs);
+  ifcvt_local_dse (loop->header);
   BITMAP_FREE (exit_bbs);
 
   todo |= TODO_cleanup_cfg;
diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c
index ba67884a825..0185bcbfce7 100644
--- a/gcc/tree-ssa-dse.c
+++ b/gcc/tree-ssa-dse.c
@@ -36,6 +36,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "params.h"
 #include "alias.h"
 #include "tree-ssa-loop.h"
+#include "tree-ssa-dse.h"
 
 /* This file implements dead store elimination.
 
@@ -76,21 +77,13 @@ along with GCC; see the file COPYING3.  If not see
fact, they are the same transformation applied to different views of
the CFG.  */
 
-static void delete_dead_or_redundant_assignment (gimple_stmt_iterator *, const char *);
+void delete_dead_or_redundant_assignment (gimple_stmt_iterator *, const char *);
 static void delete_dead_or_redundant_call (gimple_stmt_iterator *, const char *);
 
 /* Bitmap of blocks that have had EH statements cleaned.  We should
remove their dead edges eventually.  */
 static bitmap need_eh_cleanup;
 
-/* Return value from dse_classify_store */
-enum dse_store_status
-{
-  DSE_STORE_LIVE,
-  DSE_STORE_MAYBE_PARTIAL_DEAD,
-  DSE_STORE_DEAD
-};
-
 /* STMT is a statement that may write into memory.  Analyze it and
initialize WRITE to describe how STMT affects memory.
 
@@ -662,10 +655,10 @@ dse_optimize_redundant_stores (gimple *stmt)
if only clobber statements influenced the classification result.
Returns the classification.  */
 
-static dse_store_status
+dse_store_status
 dse_classify_store (ao_ref *ref, gimple *stmt,
 		bool byte_tracking_enabled, sbitmap live_bytes,
-		bool *by_clobber_p = NULL)
+		bool *by_clobber_p)
 {
   gimple *temp;
   int cnt = 0;
@@ -901,7 +894,7 @@ delete_dead_or_redundant_call (gimple_stmt_iterator *gsi, const char *type)
 
 /* Delete a dead store at GSI, which is a gimple assignment. */
 
-static void
+void
 

Re: [PATCH] Include netinet/in.h in include/experimental/internet

[Thomas Rodgers]

Looks good to me.

Andreas Tobler writes:

> Hi all,
>
> I'm going to commit the attached patch to trunk.
> It is preapproved by Jonathan. (via #irc)
>
> On FreeBSD the netinet/in.h header is not included by arpa/inet.h, so do the
> include if we have the _GLIBCXX_HAVE_NETINET_IN_H defined.
>
> This makes all the experimental/net/internet test cases pass here.
>
> Any comments?
>
> TIA,
> Andreas
>
> Commit one-liner:
>
> Include netinet/in.h in include/experimental/internet
>
> 2019-09-30  Andreas Tobler  
>
>   * include/experimental/internet: Include netinet/in.h if we have
>   _GLIBCXX_HAVE_NETINET_IN_H defined.
>
> Index: include/experimental/internet
> ===
> --- include/experimental/internet (revision 276342)
> +++ include/experimental/internet (working copy)
> @@ -51,6 +51,9 @@
>  #ifdef _GLIBCXX_HAVE_ARPA_INET_H
>  # include   // inet_ntop
>  #endif
> +#ifdef _GLIBCXX_HAVE_NETINET_IN_H
> +# include  // IPPROTO_IP
> +#endif
>  #ifdef _GLIBCXX_HAVE_NETINET_TCP_H
>  # include // TCP_NODELAY
>  #endif

Go patch committed: Change escape maps to hash tables

[Ian Lance Taylor]

This patch to the Go frontend changes some maps used by the escape
analysis pass to use hash tables instead.  It also changes them to use
just one table lookup, not two.  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 276228)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-10a1671d94ddc0c39f2f4b039e5ea33358f414c0
+07faafda5fbd66a710153814f30d93c91461e7cb
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/escape.cc
===
--- gcc/go/gofrontend/escape.cc (revision 276221)
+++ gcc/go/gofrontend/escape.cc (working copy)
@@ -579,9 +579,9 @@ Node::is_sink() const
   return false;
 }
 
-std::map Node::objects;
-std::map Node::expressions;
-std::map Node::statements;
+Unordered_map(Named_object*, Node*) Node::objects;
+Unordered_map(Expression*, Node*) Node::expressions;
+Unordered_map(Statement*, Node*) Node::statements;
 std::vector Node::indirects;
 
 // Make a object node or return a cached node for this object.
@@ -589,13 +589,12 @@ std::vector Node::indirects;
 Node*
 Node::make_node(Named_object* no)
 {
-  if (Node::objects.find(no) != Node::objects.end())
-return Node::objects[no];
-
-  Node* n = new Node(no);
-  std::pair val(no, n);
-  Node::objects.insert(val);
-  return n;
+  std::pair val(no, NULL);
+  std::pair ins =
+Node::objects.insert(val);
+  if (ins.second)
+ins.first->second = new Node(no);
+  return ins.first->second;
 }
 
 // Make an expression node or return a cached node for this expression.
@@ -603,13 +602,12 @@ Node::make_node(Named_object* no)
 Node*
 Node::make_node(Expression* e)
 {
-  if (Node::expressions.find(e) != Node::expressions.end())
-return Node::expressions[e];
-
-  Node* n = new Node(e);
-  std::pair val(e, n);
-  Node::expressions.insert(val);
-  return n;
+  std::pair val(e, NULL);
+  std::pair ins =
+Node::expressions.insert(val);
+  if (ins.second)
+ins.first->second = new Node(e);
+  return ins.first->second;
 }
 
 // Make a statement node or return a cached node for this statement.
@@ -617,13 +615,12 @@ Node::make_node(Expression* e)
 Node*
 Node::make_node(Statement* s)
 {
-  if (Node::statements.find(s) != Node::statements.end())
-return Node::statements[s];
-
-  Node* n = new Node(s);
-  std::pair val(s, n);
-  Node::statements.insert(val);
-  return n;
+  std::pair val(s, NULL);
+  std::pair ins =
+Node::statements.insert(val);
+  if (ins.second)
+ins.first->second = new Node(s);
+  return ins.first->second;
 }
 
 // Make an indirect node with given child.
@@ -3447,19 +3444,22 @@ Gogo::reclaim_escape_nodes()
 void
 Node::reclaim_nodes()
 {
-  for (std::map::iterator p = Node::objects.begin();
+  for (Unordered_map(Named_object*, Node*)::iterator p =
+Node::objects.begin();
p != Node::objects.end();
++p)
 delete p->second;
   Node::objects.clear();
 
-  for (std::map::iterator p = Node::expressions.begin();
+  for (Unordered_map(Expression*, Node*)::iterator p =
+Node::expressions.begin();
p != Node::expressions.end();
++p)
 delete p->second;
   Node::expressions.clear();
 
-  for (std::map::iterator p = Node::statements.begin();
+  for (Unordered_map(Statement*, Node*)::iterator p =
+Node::statements.begin();
p != Node::statements.end();
++p)
 delete p->second;
Index: gcc/go/gofrontend/escape.h
===
--- gcc/go/gofrontend/escape.h  (revision 276221)
+++ gcc/go/gofrontend/escape.h  (working copy)
@@ -329,9 +329,9 @@ class Node
   Node* child_;
 
   // Cache all the Nodes created via Node::make_node to make the API simpler.
-  static std::map objects;
-  static std::map expressions;
-  static std::map statements;
+  static Unordered_map(Named_object*, Node*) objects;
+  static Unordered_map(Expression*, Node*) expressions;
+  static Unordered_map(Statement*, Node*) statements;
 
   // Collection of all NODE_INDIRECT Nodes, used for reclaiming memory. This
   // is not a cache -- each make_indirect_node will make a fresh Node.

[PATCH] PR fortran/91943 -- More BOZ fallout

[Steve Kargl]

A BOZ cannot be an actual argument in a subroutine or function
reference except those intrinsic function listed in the Fortran
standard.  The attach patch checks for invalid BOZ.  Built and
tested on x86_64-*-freebsd.  OK to commit?

2019-09-30  Steven G. Kargl  

PR fortran/91943
* match.c (gfc_match_call): BOZ cannot be an actual argument in
a subroutine reference.
* resolve.c (resolve_function): BOZ cannot be an actual argument in
a function reference.
 
2019-09-30  Steven G. Kargl  

PR fortran/91943
gfortran.dg/pr91943.f90

-- 
Steve
Index: gcc/fortran/match.c
===
--- gcc/fortran/match.c	(revision 276271)
+++ gcc/fortran/match.c	(working copy)
@@ -4984,6 +4984,16 @@ gfc_match_call (void)
 	goto syntax;
 }
 
+  /* Walk the argument list looking for invalid BOZ.  */
+  for (a = arglist; a; a = a->next)
+if (a->expr && a->expr->ts.type == BT_BOZ)
+  {
+	gfc_error ("A BOZ literal constant at %L cannot appear as an actual "
+		   "argument in a subroutine reference", >expr->where);
+	goto cleanup;
+  }
+
+
   /* If any alternate return labels were found, construct a SELECT
  statement that will jump to the right place.  */
 
Index: gcc/fortran/resolve.c
===
--- gcc/fortran/resolve.c	(revision 276271)
+++ gcc/fortran/resolve.c	(working copy)
@@ -3242,6 +3242,21 @@ resolve_function (gfc_expr *expr)
   if (expr->expr_type != EXPR_FUNCTION)
 return t;
 
+  /* Walk the argument list looking for invalid BOZ.  */
+  if (expr->value.function.esym)
+{
+  gfc_actual_arglist *a;
+
+  for (a = expr->value.function.actual; a; a = a->next)
+	if (a->expr && a->expr->ts.type == BT_BOZ)
+	  {
+	gfc_error ("A BOZ literal constant at %L cannot appear as an "
+			"actual argument in a function reference",
+			>expr->where);
+	return false;
+	  }
+}
+
   temp = need_full_assumed_size;
   need_full_assumed_size = 0;
 
Index: gcc/testsuite/gfortran.dg/pr91943.f90
===
--- gcc/testsuite/gfortran.dg/pr91943.f90	(nonexistent)
+++ gcc/testsuite/gfortran.dg/pr91943.f90	(working copy)
@@ -0,0 +1,7 @@
+! { dg-do compile }
+! PR fortran/91943
+! Code contributed by Gerhard Steinmetz
+program p
+   print *, f(b'1001')  ! { dg-error "cannot appear as an actual argument" }
+   call sub(b'1001')! { dg-error "cannot appear as an actual argument" }
+end

Re: [C] Avoid aka types that just add tags

[Joseph Myers]

On Mon, 30 Sep 2019, Richard Sandiford wrote:

> 2019-09-30  Richard Sandiford  
> 
> gcc/c/
>   * c-objc-common.c (useful_aka_type_p): New function.
>   (print_type): Use it to decide whether an aka type is worth printing.
> 
> gcc/testsuite/
>   * gcc.dg/diag-aka-1.c (T): Turn into a pointer typedef.
>   (foo): Update accordingly.
>   * gcc.dg/diag-aka-4.c: New test.

OK.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [C] Improve diagnostics for vector types

[Joseph Myers]

On Mon, 30 Sep 2019, Richard Sandiford wrote:

> 2019-09-30  Richard Sandiford  
> 
> gcc/c-family/
>   * c-pretty-print.c (pp_c_specifier_qualifier_list): If a vector type
>   has a type name, use it in preference to the __vector syntax.
> 
> gcc/testsuite/
>   * gcc.dg/diag-aka-3.c: New test.
>   * gcc.target/aarch64/diag_aka_1.c: New test.
>   * g++.dg/diagnostic/aka4.C: New test.

OK.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [WIP PATCH] add object access attributes (PR 83859)

[Joseph Myers]

On Sun, 29 Sep 2019, Martin Sebor wrote:

> PR 83859 asks to expose the same checking that GCC does natively for
> built-in calls via a function attribute that associates a pointer
> argument with the size argument, such as:

I'll also note that, as mentioned in that bug (but more specifically 
covered by the separate bug 50584) it would make sense to have similar 
warnings with [static] parameter array declarators.  And that the C2x 
charter includes a principle that new interfaces should have the array 
size before the array parameter to allow parameters to be declared using 
VLA syntax like that, so it's plausible the [static] case will be useful 
for more functions in future (although it's up to library implementations 
exactly what form they use in their headers, given they have other 
considerations such as C++ compatibility).

Hopefully the infrastructure in this patch will facilitate future support 
for such diagnostics in the [static] case.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: Remove clobber_high

[Jeff Law]

On 9/25/19 10:48 AM, Richard Sandiford wrote:
> [This follows on from:
>  https://gcc.gnu.org/ml/gcc-patches/2019-09/msg01466.html]
> 
> The AArch64 SVE tlsdesc patterns were the main motivating reason
> for clobber_high.  It's no longer needed now that the patterns use
> calls instead.
> 
> At the time, one of the possible future uses for clobber_high was for
> asm statements.  However, the current code wouldn't handle that case
> without modification, so I think we might as well remove it for now.
> We can always reapply it in future if it turns out to be useful again.
> 
> [Perhaps we should have a syntax for saying asms clobber the same
> registers as calls, with a syntax for specifying a particular
> ABI where necessary?]
> 
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install
> if the prerequisites are?
> 
> Richard
> 
> 
> 2019-09-25  Richard Sandiford  
> 
> gcc/
>   * rtl.def (CLOBBER_HIGH): Delete.
>   * doc/rtl.texi (clobber_high): Remove documentation.
>   * rtl.h (SET_DEST): Remove CLOBBER_HIGH from the list of codes.
>   (reg_is_clobbered_by_clobber_high): Delete.
>   (gen_hard_reg_clobber_high): Likewise.
>   * alias.c (record_set): Remove CLOBBER_HIGH handling.
>   * cfgexpand.c (expand_gimple_stmt): Likewise.
>   * combine-stack-adj.c (single_set_for_csa): Likewise.
>   * combine.c (find_single_use_1, set_nonzero_bits_and_sign_copies)
>   (can_combine_p, is_parallel_of_n_reg_sets, try_combine)
>   (record_dead_and_set_regs_1, reg_dead_at_p_1): Likewise.
>   * cse.c (invalidate_reg): Remove clobber_high parameter.
>   (invalidate): Update call accordingly.
>   (canonicalize_insn): Remove CLOBBER_HIGH handling.
>   (invalidate_from_clobbers, invalidate_from_sets_and_clobbers)
>   (count_reg_usage, insn_live_p): Likewise.
>   * cselib.h (cselib_invalidate_rtx): Remove sett argument.
>   * cselib.c (cselib_invalidate_regno, cselib_invalidate_rtx): Likewise.
>   (cselib_invalidate_rtx_note_stores): Update call accordingly.
>   (cselib_expand_value_rtx_1): Remove CLOBBER_HIGH handling.
>   (cselib_invalidate_regno, cselib_process_insn): Likewise.
>   * dce.c (deletable_insn_p, mark_nonreg_stores_1): Likewise.
>   (mark_nonreg_stores_2): Likewise.
>   * df-scan.c (df_find_hard_reg_defs, df_uses_record): Likewise.
>   (df_get_call_refs): Likewise.
>   * dwarf2out.c (mem_loc_descriptor): Likewise.
>   * emit-rtl.c (verify_rtx_sharing): Likewise.
>   (copy_insn_1, copy_rtx_if_shared_1): Likewise.
>   (hard_reg_clobbers_high, gen_hard_reg_clobber_high): Delete.
>   * genconfig.c (walk_insn_part): Remove CLOBBER_HIGH handling.
>   * genemit.c (gen_exp, gen_insn): Likewise.
>   * genrecog.c (validate_pattern, remove_clobbers): Likewise.
>   * haifa-sched.c (haifa_classify_rtx): Likewise.
>   * ira-build.c (create_insn_allocnos): Likewise.
>   * ira-costs.c (scan_one_insn): Likewise.
>   * ira.c (equiv_init_movable_p, memref_referenced_p): Likewise.
>   (rtx_moveable_p, interesting_dest_for_shprep): Likewise.
>   * jump.c (mark_jump_label_1): Likewise.
>   * lra-int.h (lra_insn_reg::clobber_high): Delete.
>   * lra-eliminations.c (lra_eliminate_regs_1): Remove CLOBBER_HIGH
>   handling.
>   (mark_not_eliminable): Likewise.
>   * lra-lives.c (process_bb_lives): Likewise.
>   * lra.c (new_insn_reg): Remove clobber_high parameter.
>   (collect_non_operand_hard_regs): Likewise.  Update call to new
>   insn_reg.  Remove CLOBBER_HIGH handling.
>   (lra_set_insn_recog_data): Remove CLOBBER_HIGH handling.  Update call
>   to collect_non_operand_hard_regs.
>   (add_regs_to_insn_regno_info): Remove CLOBBER_HIGH handling.
>   Update call to new_insn_reg.
>   (lra_update_insn_regno_info): Remove CLOBBER_HIGH handling.
>   * postreload.c (reload_cse_simplify, reload_combine_note_use)
>   (move2add_note_store): Likewise.
>   * print-rtl.c (print_pattern): Likewise.
>   * recog.c (store_data_bypass_p_1, store_data_bypass_p): Likewise.
>   (if_test_bypass_p): Likewise.
>   * regcprop.c (kill_clobbered_value, kill_set_value): Likewise.
>   * reginfo.c (reg_scan_mark_refs): Likewise.
>   * reload1.c (maybe_fix_stack_asms, eliminate_regs_1): Likewise.
>   (elimination_effects, mark_not_eliminable, scan_paradoxical_subregs)
>   (forget_old_reloads_1): Likewise.
>   * reorg.c (find_end_label, try_merge_delay_insns, redundant_insn)
>   (own_thread_p, fill_simple_delay_slots, fill_slots_from_thread)
>   (dbr_schedule): Likewise.
>   * resource.c (update_live_status, mark_referenced_resources)
>   (mark_set_resources): Likewise.
>   * rtl.c (copy_rtx): Likewise.
>   * rtlanal.c (reg_referenced_p, set_of_1, single_set_2, noop_move_p)
>   (note_pattern_stores): Likewise.
>   (reg_is_clobbered_by_clobber_high): Delete.
>   * 

Re: [01/32] Add function_abi.{h,cc}

[Jeff Law]

On 9/30/19 3:19 AM, Richard Sandiford wrote:
> Hi Jeff,
> 
> Thanks as always for the reviews :-)
> 
> Jeff Law  writes:
>> On 9/11/19 1:03 PM, Richard Sandiford wrote:
>>> This patch adds new structures and functions for handling
>>> multiple ABIs in a translation unit.  The structures are:
>>>
>>> - predefined_function_abi: describes a static, predefined ABI
>>> - function_abi: describes either a predefined ABI or a local
>>>   variant of one (e.g. taking -fipa-ra into account)
>>>
>>> The patch adds functions for getting the ABI from a given type
>>> or decl; a later patch will also add a function for getting the
>>> ABI of the target of a call insn.
>>>
>>> Although ABIs are about much more than call-clobber/saved choices,
>>> I wanted to keep the name general in case we add more ABI-related
>>> information in future.
>>>
>>>
>>> 2019-09-11  Richard Sandiford  
>>>
>>> gcc/
>>> * Makefile.in (OBJS): Add function-abi.o.
>>> (GTFILES): Add function-abi.h.
>>> * function-abi.cc: New file.
>>> * function-abi.h: Likewise.
>>> * emit-rtl.h (rtl_data::abi): New field.
>>> * function.c: Include function-abi.h.
>>> (prepare_function_start): Initialize crtl->abi.
>>> * read-rtl-function.c: Include regs.h and function-abi.h.
>>> (read_rtl_function_body): Initialize crtl->abi.
>>> (read_rtl_function_body_from_file_range): Likewise.
>>> * reginfo.c: Include function-abi.h.
>>> (init_reg_sets_1): Initialize default_function_abi.
>>> (globalize_reg): Call add_full_reg_clobber for each predefined ABI
>>> when making a register global.
>>> * target-globals.h (this_target_function_abi_info): Declare.
>>> (target_globals::function_abi_info): New field.
>>> (restore_target_globals): Copy it.
>>> * target-globals.c: Include function-abi.h.
>>> (default_target_globals): Initialize the function_abi_info field.
>>> (target_globals): Allocate it.
>>> (save_target_globals): Free it.
>> So no problem with this as-is.  Questions though:
>>
>> 1. Do we need to stream this information for LTO?
> 
> At the moment this is all derived information rather than something we need
> to stream directly.  E.g. the set of available predefined_function_abis
> really only depends on command-line flags.  The mapping from functions
> to predefined_function_abis currently depends only on the function type,
> so streaming the type is enough to recover the ABI too.  function_abi
> additionally depends on RTL stuff that doesn't affect LTO.
OK.  THanks for clarifying.


> 
>> 2. Do we need to support it for the RTL front-end, even if primarily for
>> testing purposes?
> 
> Yeah, I guess it could be useful to be able to pretend that a function
> is defined locally with a certain -fipa-ra clobber set, but I think in
> most cases it'd be possible to do this using:
> 
>   void __attribute__ ((noinline, noclone))
>   callee (void)
>   {
> asm ("" ::: ...regs...);
>   }
> 
> (That's the kind of test I've used in the support for the SVE PCS FWIW,
> not posted yet.)
Your call on whether or not it's worth it.  Given that it's supposed to
be derived info, I won't lose any sleep if it's not available in the RTL
front-end.

jeff

Re: [ping][PR target/85401] initialize the move cost table before using it

[coypu]

On Mon, Sep 30, 2019 at 11:46:24AM -0400, Vladimir Makarov wrote:
> Yes, the patch is mostly ok.  You can commit it into the trunk after
> applying changes mentioned below. If you do not have a write access, let me
> know I'll commit the patch by myself.

I don't have commit access. It would be nice if you committed it :)

> It would be nice to add a small test too.  But it is not obligatory for this
> case as the patch is obvious and it might be hard to create a small test to
> reproduce the bug.

I have the C code that produces this failure. I can creduce it, but I'm
not sure there's a relationship between it and the bug.
Doing unrelated changes (adding instruction scheduling) to vax also hid it.

Is this kind of test still valuable?

Thanks.

[PATCH] Implement C++20 P1023 for __debug::array

[François Dumont]

This is a missing part of C++20 P1023 for __debug::array.

    Implement C++20 p1023 - constexpr comparison operators for 
__debug::array.

    * include/debug/array: Add C++20 constexpr to comparison operators.
    * testsuite/23_containers/array/tuple_interface/get_debug_neg.cc: Adapt
    dg-error line numbers.
    * testsuite/23_containers/array/tuple_interface/
    tuple_element_debug_neg.cc: Likewise.


Tested under Linux x86_64 normal and debug modes.

François

diff --git a/libstdc++-v3/include/debug/array b/libstdc++-v3/include/debug/array
index 2f8eb842eb8..5566a087f9a 100644
--- a/libstdc++-v3/include/debug/array
+++ b/libstdc++-v3/include/debug/array
@@ -234,16 +234,19 @@ namespace __debug
 
   // Array comparisons.
   template
+_GLIBCXX20_CONSTEXPR
 inline bool
 operator==(const array<_Tp, _Nm>& __one, const array<_Tp, _Nm>& __two)
 { return std::equal(__one.begin(), __one.end(), __two.begin()); }
 
   template
+_GLIBCXX20_CONSTEXPR
 inline bool
 operator!=(const array<_Tp, _Nm>& __one, const array<_Tp, _Nm>& __two)
 { return !(__one == __two); }
 
   template
+_GLIBCXX20_CONSTEXPR
 inline bool
 operator<(const array<_Tp, _Nm>& __a, const array<_Tp, _Nm>& __b)
 {
@@ -252,16 +255,19 @@ namespace __debug
 }
 
   template
+_GLIBCXX20_CONSTEXPR
 inline bool
 operator>(const array<_Tp, _Nm>& __one, const array<_Tp, _Nm>& __two)
 { return __two < __one; }
 
   template
+_GLIBCXX20_CONSTEXPR
 inline bool
 operator<=(const array<_Tp, _Nm>& __one, const array<_Tp, _Nm>& __two)
 { return !(__one > __two); }
 
   template
+_GLIBCXX20_CONSTEXPR
 inline bool
 operator>=(const array<_Tp, _Nm>& __one, const array<_Tp, _Nm>& __two)
 { return !(__one < __two); }
diff --git a/libstdc++-v3/testsuite/23_containers/array/tuple_interface/get_debug_neg.cc b/libstdc++-v3/testsuite/23_containers/array/tuple_interface/get_debug_neg.cc
index f9880b4b0f5..3c60a435491 100644
--- a/libstdc++-v3/testsuite/23_containers/array/tuple_interface/get_debug_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/array/tuple_interface/get_debug_neg.cc
@@ -27,6 +27,6 @@ int n1 = std::get<1>(a);
 int n2 = std::get<1>(std::move(a));
 int n3 = std::get<1>(ca);
 
-// { dg-error "static assertion failed" "" { target *-*-* } 288 }
-// { dg-error "static assertion failed" "" { target *-*-* } 297 }
-// { dg-error "static assertion failed" "" { target *-*-* } 305 }
+// { dg-error "static assertion failed" "" { target *-*-* } 294 }
+// { dg-error "static assertion failed" "" { target *-*-* } 303 }
+// { dg-error "static assertion failed" "" { target *-*-* } 311 }
diff --git a/libstdc++-v3/testsuite/23_containers/array/tuple_interface/tuple_element_debug_neg.cc b/libstdc++-v3/testsuite/23_containers/array/tuple_interface/tuple_element_debug_neg.cc
index 10b1681c0c4..a6b44eb57fe 100644
--- a/libstdc++-v3/testsuite/23_containers/array/tuple_interface/tuple_element_debug_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/array/tuple_interface/tuple_element_debug_neg.cc
@@ -22,4 +22,4 @@
 
 typedef std::tuple_element<1, std::array>::type type;
 
-// { dg-error "static assertion failed" "" { target *-*-* } 331 }
+// { dg-error "static assertion failed" "" { target *-*-* } 376 }

Re: [PATCH] Fix algo constexpr tests in Debug mode

[François Dumont]

On 9/30/19 11:03 AM, Jonathan Wakely wrote:

On 28/09/19 23:12 +0200, François Dumont wrote:

Here is what I just commited.


Thanks. In my branch I had a lot more _GLIBCXX20_CONSTEXPR additions
in the debug mode headers. Is it not needed on __check_singular,
__foreign_iterator etc. ?


Yes, I know, I just limited myself to fixing testsuite tests for the 
moment. I'll check if those need it too.




I try to use the asm trick in the _GLIBCXX_DEBUG_VERIFY_COND_AT but 
didn't notice any enhancement. So for now I kept my solution to just 
have a non-constexpr call compiler error.


You won't see any improvement in the testsuite, because the compiler
flags for the testsuite suppress diagnostic notes. But I'd expect it
to give better output for users with the default compiler flags.

Ok, I didn't know, I'll give it another try then outside testsuite.



diff --git a/libstdc++-v3/include/bits/stl_algo.h 
b/libstdc++-v3/include/bits/stl_algo.h

index a672f8b2b39..f25b8b76df6 100644
--- a/libstdc++-v3/include/bits/stl_algo.h
+++ b/libstdc++-v3/include/bits/stl_algo.h
@@ -5054,8 +5054,8 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
   *  @param  __last1   Another iterator.
   *  @param  __last2   Another iterator.
   *  @param  __result  An iterator pointing to the end of the merged 
range.
-   *  @return An iterator pointing to the first element 
not less

-   *  than @e val.
+   *  @return   An output iterator equal to @p __result + (__last1 - 
__first1)

+   *    + (__last2 - __first2).
   *
   *  Merges the ranges @p [__first1,__last1) and @p 
[__first2,__last2) into

   *  the sorted range @p [__result, __result + (__last1-__first1) +
@@ -5102,8 +5102,8 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
   *  @param  __last2   Another iterator.
   *  @param  __result  An iterator pointing to the end of the merged 
range.

   *  @param  __comp    A functor to use for comparisons.
-   *  @return An iterator pointing to the first element "not 
less

-   *  than" @e val.
+   *  @return   An output iterator equal to @p __result + (__last1 - 
__first1)

+   *    + (__last2 - __first2).
   *
   *  Merges the ranges @p [__first1,__last1) and @p 
[__first2,__last2) into

   *  the sorted range @p [__result, __result + (__last1-__first1) +


These changes are fine but should have been a separate, unrelated
commit.


Ok, sorry, I consider that just a comment change was fine.






@@ -157,10 +192,16 @@ namespace __gnu_debug
   *  otherwise.
  */
  template
+    _GLIBCXX20_CONSTEXPR
    inline bool
    __valid_range(_InputIterator __first, _InputIterator __last,
  typename _Distance_traits<_InputIterator>::__type& __dist)
    {
+#ifdef __cpp_lib_is_constant_evaluated
+  if (std::is_constant_evaluated())
+    // Detected by the compiler directly.
+    return true;
+#endif


Should this be using the built-in, not the C++20 function?


In practice it's probably equivalent, because the function is only
going to be constant-evaluated when called from C++20 code, and in
that case the std::is_constant_evaluated() function will be available.



Yes, this is why I did it this way. And moreover it is using std::pair 
move assignment operator which is also C++20 constexpr.




It just seems inconsistent to use the built-in in one place and not in
the other.


It is also the reason why the simple simple __valid_range is not using 
the other anymore.


Maybe once I'll have check all the algo calls I'll find out that this 
one need _GLIBCXX_CONSTEXPR.


I got the sensation that library is being 'constexpr' decorated only 
when needed and when properly Standardise so are the Debug helpers.


François

[committed] diagnostic-show-locus.c: rework handling of multiple labels

[David Malcolm]

This patch improves the handling of large numbers of labels within a
rich_location: previously, overlapping labels could lead to an assertion
failure within layout::print_any_labels.  Also, the labels were printed
in reverse order of insertion into the rich_location.

This patch moves the determination of whether a vertical bar should
be printed for a line_label into the
  'Figure out how many "label lines" we need, and which
   one each label is printed in.'
step of layout::print_any_labels, rather than doing it as the lines
are printed.  It also flips the sort order, so that labels at the
same line/column are printed in order of insertion into the
rich_location.

I haven't run into these issues with our existing diagnostics, but it
affects a patch kit I'm working on that makes more extensive use of
labels.

Successfully bootstrapped on x86_64-pc-linux-gnu.

Committed to trunk as r276371.

gcc/ChangeLog:
* diagnostic-show-locus.c (line_label::line_label): Initialize
m_has_vbar.
(line_label::comparator): Reverse the sort order by m_state_idx,
so that when the list is walked backwards the labels appear in
order of insertion into the rich_location.
(line_label::m_has_vbar): New field.
(layout::print_any_labels): When dealing with multiple labels at
the same line and column, only print vertical bars for the one
with the highest label_line.
(selftest::test_one_liner_labels): Update test for multiple labels
to expect the labels to be in the order of insertion into the
rich_location.  Add a test for many such labels, where the column
numbers are out-of-order relative to the insertion order.
---
 gcc/diagnostic-show-locus.c | 75 +
 1 file changed, 62 insertions(+), 13 deletions(-)

diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index e36c16e..42146c5 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -1416,7 +1416,7 @@ public:
   line_label (int state_idx, int column, label_text text)
   : m_state_idx (state_idx), m_column (column),
 m_text (text), m_length (strlen (text.m_buffer)),
-m_label_line (0)
+m_label_line (0), m_has_vbar (true)
   {}
 
   /* Sorting is primarily by column, then by state index.  */
@@ -1427,7 +1427,10 @@ public:
 int column_cmp = compare (ll1->m_column, ll2->m_column);
 if (column_cmp)
   return column_cmp;
-return compare (ll1->m_state_idx, ll2->m_state_idx);
+/* Order by reverse state index, so that labels are printed
+   in order of insertion into the rich_location when the
+   sorted list is walked backwards.  */
+return -compare (ll1->m_state_idx, ll2->m_state_idx);
   }
 
   int m_state_idx;
@@ -1435,6 +1438,7 @@ public:
   label_text m_text;
   size_t m_length;
   int m_label_line;
+  bool m_has_vbar;
 };
 
 /* Print any labels in this row.  */
@@ -1511,8 +1515,8 @@ layout::print_any_labels (linenum_type row)
foo + bar
^   : label line 0
|   : label line 1
-   label 1 : label line 2
-   label 0 : label line 3.  */
+   label 0 : label line 2
+   label 1 : label line 3.  */
 
   int max_label_line = 1;
   {
@@ -1522,7 +1526,15 @@ layout::print_any_labels (linenum_type row)
   {
/* Would this label "touch" or overlap the next label?  */
if (label->m_column + label->m_length >= (size_t)next_column)
- max_label_line++;
+ {
+   max_label_line++;
+
+   /* If we've already seen labels with the same column, suppress the
+  vertical bar for subsequent ones in this backwards iteration;
+  hence only the one with the highest label_line has m_has_vbar 
set.  */
+   if (label->m_column == next_column)
+ label->m_has_vbar = false;
+ }
 
label->m_label_line = max_label_line;
next_column = label->m_column;
@@ -1533,10 +1545,6 @@ layout::print_any_labels (linenum_type row)
  either a vertical bar ('|') for the labels that are lower down, or the
  labels themselves once we've reached their line.  */
   {
-/* Keep track of in which column we last printed a vertical bar.
-   This allows us to suppress duplicate vertical bars for the case
-   where multiple labels are on one column.  */
-int last_vbar = 0;
 for (int label_line = 0; label_line <= max_label_line; label_line++)
   {
start_annotation_line ();
@@ -1558,14 +1566,13 @@ layout::print_any_labels (linenum_type row)
m_colorizer.set_normal_text ();
column += label->m_length;
  }
-   else if (label->m_column != last_vbar)
+   else if (label->m_has_vbar)
  {
gcc_assert (column <= label->m_column);
move_to_column (, label->m_column, 

Re: [PATCH] libstdc++ testsuite, silence a FreeBSD libm warning

[Jonathan Wakely]

On 30/09/19 21:21 +0200, Andreas Tobler wrote:

On 30.09.19 19:09, Andreas Tobler wrote:

Hi all,

the attached patch tries to silence a warning from the msun of FreeBSD.
When compiling the special_functions testcases, I get excess errors
because of this warning:

warning: tgammal has lower than advertised precision

With the patch, all special_functions test cases pass now.

What do you think?


I learned from the c-side, a patch to prune.exp might be too expensive 
if I only have a couple of test cases involved.


Here the version which only touches the affected test cases.

Which one do you prefer?


I think I prefer this one, although it means we need to remember (or
be reminded) to add it to new testcases that need it.

This one is OK for trunk and any branches you want to backport it to,
thanks.



TIA,
Andreas

2019-09-30  Andreas Tobler  

* testsuite/ext/special_functions/airy_ai/check_nan.cc: Ignore the
FreeBSD warning about lower advertised precision of tgammal.
* testsuite/ext/special_functions/airy_bi/check_nan.cc: Likewise.
* testsuite/special_functions/07_cyl_bessel_i/check_nan.cc: Likewise.
* testsuite/special_functions/08_cyl_bessel_j/check_nan.cc: Likewise.
* testsuite/special_functions/09_cyl_bessel_k/check_nan.cc: Likewise.
* testsuite/special_functions/10_cyl_neumann/check_nan.cc: Likewise.
* testsuite/special_functions/19_sph_bessel/check_nan.cc: Likewise.
* testsuite/special_functions/21_sph_neumann/check_nan.cc: Likewise.
* testsuite/tr1/5_numerical_facilities/special_functions/
08_cyl_bessel_i/check_nan.cc: Likewise.
* testsuite/tr1/5_numerical_facilities/special_functions/
09_cyl_bessel_j/check_nan.cc: Likewise.
* testuite/tr1/5_numerical_facilities/special_functions/
10_cyl_bessel_k/check_nan.cc: Likewise.
* testsuite/tr1/5_numerical_facilities/special_functions/
11_cyl_neumann/check_nan.cc: Likewise.
* testsuite/tr1/5_numerical_facilities/special_functions/
21_sph_bessel/check_nan.cc: Likewise.
* testsuite/tr1/5_numerical_facilities/special_functions/
23_sph_neumann/check_nan.cc: Likewise.

Re: [C++ Patch] Improve grokdeclarator error

[Paolo Carlini]

Hi,

On 30/09/19 20:46, Jason Merrill wrote:

On Mon, Sep 30, 2019 at 6:54 AM Paolo Carlini  wrote:

On 28/09/19 04:05, Jason Merrill wrote:

On 9/26/19 3:39 PM, Paolo Carlini wrote:

+template void foo(  // { dg-error "15:template-id .foo.
used as a declarator" }

That's a strange diagnostic message; there's nothing wrong with using
a template-id as a declarator.  Why are we even calling grokdeclarator
when we hit EOF in the middle of the declarator?

It's indeed a weird situation. Note, by the way, that for

template void foo;

we end up giving the same diagnostic, only, the location was already fine.

Anyway, to explain why I say weird, clang believes it's dealing with an
explicit instantiation:

explicit instantiation of 'foo' does not refer to a function template,
variable template, member function, member class, or static data member

whereas EDG gives:

declaration is incompatible with function template "void foo<>()"

I *think* what we are issuing is closer in spirit to the latter, we
don't get fooled into thinking it's an instantiation and we say that as
a declaration doesn't work either. See what I mean?

Well, it is an explicit instantiation, the problem (as EDG says) is
that the instantiation declaration doesn't match the template.


Definitely it doesn't.




Removing completely
the diagnostic code doesn't seem fine either because we end up with very
confusing wordings like

variable or field declared void

or worse we mention variable templates, depending on the type (I already
mentioned this).

Any thought about my question about why we're calling grokdeclarator
in the first place?


Yes. If you look at cp_parser_explicit_instantiation, clearly 
cp_parser_declarator doesn't return a cp_error_declarator and in such 
cases we always call grokdeclarator. Do you think we should error out 
here instead, much earlier? Currently grokdeclarator does quite a bit of 
work before the diagnostic.


Paolo..

Re: [PATCH] libstdc++ testsuite, silence a FreeBSD libm warning

[Andreas Tobler]

On 30.09.19 19:09, Andreas Tobler wrote:

Hi all,

the attached patch tries to silence a warning from the msun of FreeBSD.
When compiling the special_functions testcases, I get excess errors
because of this warning:

warning: tgammal has lower than advertised precision

With the patch, all special_functions test cases pass now.

What do you think?


I learned from the c-side, a patch to prune.exp might be too expensive 
if I only have a couple of test cases involved.


Here the version which only touches the affected test cases.

Which one do you prefer?

TIA,
Andreas

2019-09-30  Andreas Tobler  

* testsuite/ext/special_functions/airy_ai/check_nan.cc: Ignore the
FreeBSD warning about lower advertised precision of tgammal.
* testsuite/ext/special_functions/airy_bi/check_nan.cc: Likewise.
* testsuite/special_functions/07_cyl_bessel_i/check_nan.cc: Likewise.
* testsuite/special_functions/08_cyl_bessel_j/check_nan.cc: Likewise.
* testsuite/special_functions/09_cyl_bessel_k/check_nan.cc: Likewise.
* testsuite/special_functions/10_cyl_neumann/check_nan.cc: Likewise.
* testsuite/special_functions/19_sph_bessel/check_nan.cc: Likewise.
* testsuite/special_functions/21_sph_neumann/check_nan.cc: Likewise.
* testsuite/tr1/5_numerical_facilities/special_functions/
08_cyl_bessel_i/check_nan.cc: Likewise.
* testsuite/tr1/5_numerical_facilities/special_functions/
09_cyl_bessel_j/check_nan.cc: Likewise.
* testuite/tr1/5_numerical_facilities/special_functions/
10_cyl_bessel_k/check_nan.cc: Likewise.
* testsuite/tr1/5_numerical_facilities/special_functions/
11_cyl_neumann/check_nan.cc: Likewise.
* testsuite/tr1/5_numerical_facilities/special_functions/
21_sph_bessel/check_nan.cc: Likewise.
* testsuite/tr1/5_numerical_facilities/special_functions/
23_sph_neumann/check_nan.cc: Likewise.
Index: ext/special_functions/airy_ai/check_nan.cc
===
--- ext/special_functions/airy_ai/check_nan.cc  (revision 276292)
+++ ext/special_functions/airy_ai/check_nan.cc  (working copy)
@@ -3,6 +3,7 @@
 // { dg-options "-D__STDCPP_WANT_MATH_SPEC_FUNCS__" }
 // { dg-add-options ieee }
 // { dg-skip-if "no extensions in strict dialects" { *-*-* } { "-std=c++*" } }
+// { dg-prune-output ".*warning: tgammal has lower than advertised precision" }
 
 // Copyright (C) 2016-2019 Free Software Foundation, Inc.
 //
Index: ext/special_functions/airy_bi/check_nan.cc
===
--- ext/special_functions/airy_bi/check_nan.cc  (revision 276292)
+++ ext/special_functions/airy_bi/check_nan.cc  (working copy)
@@ -3,6 +3,7 @@
 // { dg-options "-D__STDCPP_WANT_MATH_SPEC_FUNCS__" }
 // { dg-add-options ieee }
 // { dg-skip-if "no extensions in strict dialects" { *-*-* } { "-std=c++*" } }
+// { dg-prune-output ".*warning: tgammal has lower than advertised precision" }
 
 // Copyright (C) 2016-2019 Free Software Foundation, Inc.
 //
Index: special_functions/07_cyl_bessel_i/check_nan.cc
===
--- special_functions/07_cyl_bessel_i/check_nan.cc  (revision 276292)
+++ special_functions/07_cyl_bessel_i/check_nan.cc  (working copy)
@@ -2,6 +2,7 @@
 // { dg-require-c-std "" }
 // { dg-options "-D__STDCPP_WANT_MATH_SPEC_FUNCS__" }
 // { dg-add-options ieee }
+// { dg-prune-output ".*warning: tgammal has lower than advertised precision" }
 
 // Copyright (C) 2016-2019 Free Software Foundation, Inc.
 //
Index: special_functions/08_cyl_bessel_j/check_nan.cc
===
--- special_functions/08_cyl_bessel_j/check_nan.cc  (revision 276292)
+++ special_functions/08_cyl_bessel_j/check_nan.cc  (working copy)
@@ -2,6 +2,7 @@
 // { dg-require-c-std "" }
 // { dg-options "-D__STDCPP_WANT_MATH_SPEC_FUNCS__" }
 // { dg-add-options ieee }
+// { dg-prune-output ".*warning: tgammal has lower than advertised precision" }
 
 // Copyright (C) 2016-2019 Free Software Foundation, Inc.
 //
Index: special_functions/09_cyl_bessel_k/check_nan.cc
===
--- special_functions/09_cyl_bessel_k/check_nan.cc  (revision 276292)
+++ special_functions/09_cyl_bessel_k/check_nan.cc  (working copy)
@@ -2,6 +2,7 @@
 // { dg-require-c-std "" }
 // { dg-options "-D__STDCPP_WANT_MATH_SPEC_FUNCS__" }
 // { dg-add-options ieee }
+// { dg-prune-output ".*warning: tgammal has lower than advertised precision" }
 
 // Copyright (C) 2016-2019 Free Software Foundation, Inc.
 //
Index: special_functions/10_cyl_neumann/check_nan.cc
===
--- special_functions/10_cyl_neumann/check_nan.cc   (revision 276292)
+++ special_functions/10_cyl_neumann/check_nan.cc   (working copy)

Re: [PATCH] C testsuite, silence a FreeBSD libc warning

[Andreas Tobler]

On 30.09.19 20:37, Kamil Rytarowski wrote:

On 30.09.2019 19:47, Jakub Jelinek wrote:

On Mon, Sep 30, 2019 at 07:41:00PM +0200, Andreas Tobler wrote:

--- fprintf-2.c (revision 276292)
+++ fprintf-2.c (working copy)
@@ -1,7 +1,8 @@
  /* Verify that calls to fprintf don't get eliminated even if their
 result on success can be computed at compile time (they can fail).
 The calls can still be transformed into those of other functions.
-   { dg-skip-if "requires io" { freestanding } } */
+   { dg-skip-if "requires io" { freestanding } }
+   { dg-prune-output "(^|\n)(\[^\n\])*warning: warning: \[^\n\]* possibly used 
unsafely; consider using \[^\n\]*\n" } */


I'm worried about that (^|\n) at the start + \n at the end, doesn't it prune
too much then?
Looking at other tests, they dg-prune-output just a few words from a
message, or .*few words.*
So, can you try just
{ dg-prune-output "warning: warning: \[^\n\r\]* possibly used unsafely; consider 
using" } */
or if that doesn't work, with .* at start end end?

Jakub



Please handle the NetBSD specific string too: "warning: tmpnam()
possibly used unsafely, use mkstemp() or mkdtemp()".

https://nxr.netbsd.org/xref/src/lib/libc/stdio/tmpnam.c#52



Ok, I think the attached version should also match these cases. Although 
untested on NetBSD.

Kamil, if you have cycles, would you mind giving it a run? Thanks!
Andreas

Index: fprintf-2.c
===
--- fprintf-2.c (revision 276292)
+++ fprintf-2.c (working copy)
@@ -1,7 +1,8 @@
 /* Verify that calls to fprintf don't get eliminated even if their
result on success can be computed at compile time (they can fail).
The calls can still be transformed into those of other functions.
-   { dg-skip-if "requires io" { freestanding } } */
+   { dg-skip-if "requires io" { freestanding } }
+   { dg-prune-output "warning: warning: \[^\n\r\]* possibly used unsafely" } */
 
 #include 
 #include 
Index: printf-2.c
===
--- printf-2.c  (revision 276292)
+++ printf-2.c  (working copy)
@@ -2,7 +2,8 @@
result on success can be computed at compile time (they can fail).
The calls can still be transformed into those of other functions.
{ dg-require-effective-target unwrapped }
-   { dg-skip-if "requires io" { freestanding } } */
+   { dg-skip-if "requires io" { freestanding } }
+   { dg-prune-output "warning: warning: \[^\n\r\]* possibly used unsafely" } */
 
 #include 
 #include 
Index: user-printf.c
===
--- user-printf.c   (revision 276292)
+++ user-printf.c   (working copy)
@@ -2,7 +2,8 @@
don't get eliminated even if their result on success can be computed at
compile time (they can fail).
{ dg-require-effective-target unwrapped }
-   { dg-skip-if "requires io" { freestanding } } */
+   { dg-skip-if "requires io" { freestanding } }
+   { dg-prune-output "warning: warning: \[^\n\r\]* possibly used unsafely" } */
 
 #include 
 #include 

Re: [C++ Patch] Improve grokdeclarator error

[Jason Merrill]

On Mon, Sep 30, 2019 at 6:54 AM Paolo Carlini  wrote:
>
> On 28/09/19 04:05, Jason Merrill wrote:
> > On 9/26/19 3:39 PM, Paolo Carlini wrote:
> >> +template void foo(  // { dg-error "15:template-id .foo.
> >> used as a declarator" }
> >
> > That's a strange diagnostic message; there's nothing wrong with using
> > a template-id as a declarator.  Why are we even calling grokdeclarator
> > when we hit EOF in the middle of the declarator?
>
> It's indeed a weird situation. Note, by the way, that for
>
> template void foo;
>
> we end up giving the same diagnostic, only, the location was already fine.
>
> Anyway, to explain why I say weird, clang believes it's dealing with an
> explicit instantiation:
>
> explicit instantiation of 'foo' does not refer to a function template,
> variable template, member function, member class, or static data member
>
> whereas EDG gives:
>
> declaration is incompatible with function template "void foo<>()"
>
> I *think* what we are issuing is closer in spirit to the latter, we
> don't get fooled into thinking it's an instantiation and we say that as
> a declaration doesn't work either. See what I mean?

Well, it is an explicit instantiation, the problem (as EDG says) is
that the instantiation declaration doesn't match the template.

> Removing completely
> the diagnostic code doesn't seem fine either because we end up with very
> confusing wordings like
>
> variable or field declared void
>
> or worse we mention variable templates, depending on the type (I already
> mentioned this).

Any thought about my question about why we're calling grokdeclarator
in the first place?

> Thus, all in all, would it make sense to simply issue something closer
> to EDG?

Probably.

Jason

Re: [PATCH] C testsuite, silence a FreeBSD libc warning

[Jakub Jelinek]

On Mon, Sep 30, 2019 at 08:31:23PM +0200, Andreas Tobler wrote:
> Ok, yes it works too with your suggestion.
> 
> Attached.

Ok for trunk.

> One question, doing it per test case is cheaper than in prune.exp? (Where we
> do it for 'all' test cases, needed or not.)

IMHO yes, doing it 20 or how many times vs. just 3 times should make a
difference.

> Index: fprintf-2.c
> ===
> --- fprintf-2.c   (revision 276292)
> +++ fprintf-2.c   (working copy)
> @@ -1,7 +1,8 @@
>  /* Verify that calls to fprintf don't get eliminated even if their
> result on success can be computed at compile time (they can fail).
> The calls can still be transformed into those of other functions.
> -   { dg-skip-if "requires io" { freestanding } } */
> +   { dg-skip-if "requires io" { freestanding } }
> +   { dg-prune-output "warning: warning: \[^\n\r\]* possibly used unsafely; 
> consider using" } */
>  
>  #include 
>  #include 
> Index: printf-2.c
> ===
> --- printf-2.c(revision 276292)
> +++ printf-2.c(working copy)
> @@ -2,7 +2,8 @@
> result on success can be computed at compile time (they can fail).
> The calls can still be transformed into those of other functions.
> { dg-require-effective-target unwrapped }
> -   { dg-skip-if "requires io" { freestanding } } */
> +   { dg-skip-if "requires io" { freestanding } }
> +   { dg-prune-output "warning: warning: \[^\n\r\]* possibly used unsafely; 
> consider using" } */
>  
>  #include 
>  #include 
> Index: user-printf.c
> ===
> --- user-printf.c (revision 276292)
> +++ user-printf.c (working copy)
> @@ -2,7 +2,8 @@
> don't get eliminated even if their result on success can be computed at
> compile time (they can fail).
> { dg-require-effective-target unwrapped }
> -   { dg-skip-if "requires io" { freestanding } } */
> +   { dg-skip-if "requires io" { freestanding } }
> +   { dg-prune-output "warning: warning: \[^\n\r\]* possibly used unsafely; 
> consider using" } */
>  
>  #include 
>  #include 


Jakub

Re: [PATCH] C testsuite, silence a FreeBSD libc warning

[Kamil Rytarowski]

On 30.09.2019 19:47, Jakub Jelinek wrote:
> On Mon, Sep 30, 2019 at 07:41:00PM +0200, Andreas Tobler wrote:
>> --- fprintf-2.c  (revision 276292)
>> +++ fprintf-2.c  (working copy)
>> @@ -1,7 +1,8 @@
>>  /* Verify that calls to fprintf don't get eliminated even if their
>> result on success can be computed at compile time (they can fail).
>> The calls can still be transformed into those of other functions.
>> -   { dg-skip-if "requires io" { freestanding } } */
>> +   { dg-skip-if "requires io" { freestanding } }
>> +   { dg-prune-output "(^|\n)(\[^\n\])*warning: warning: \[^\n\]* possibly 
>> used unsafely; consider using \[^\n\]*\n" } */ 
> 
> I'm worried about that (^|\n) at the start + \n at the end, doesn't it prune
> too much then?
> Looking at other tests, they dg-prune-output just a few words from a
> message, or .*few words.*
> So, can you try just
>{ dg-prune-output "warning: warning: \[^\n\r\]* possibly used unsafely; 
> consider using" } */
> or if that doesn't work, with .* at start end end?
> 
>   Jakub
> 

Please handle the NetBSD specific string too: "warning: tmpnam()
possibly used unsafely, use mkstemp() or mkdtemp()".

https://nxr.netbsd.org/xref/src/lib/libc/stdio/tmpnam.c#52



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] Add some hash_map_safe_* functions like vec_safe_*.

[Jason Merrill]

My comments accidentally got lost.

Several places in the front-end (and elsewhere) use the same lazy
allocation pattern for hash_maps, and this patch replaces them with
hash_map_safe_* functions like vec_safe_*.  They don't provide a way
to specify an initial size, but I don't think that's a significant
issue.

Tested x86_64-pc-linux-gnu.  OK for trunk?

On Mon, Sep 30, 2019 at 2:30 PM Jason Merrill  wrote:
>
> gcc/
> * hash-map.h (default_size): Put in member variable.
> (create_ggc): Use it as default argument.
> (hash_map_maybe_create, hash_map_safe_get)
> (hash_map_safe_get_or_insert, hash_map_safe_put): New fns.
> gcc/cp/
> * constexpr.c (maybe_initialize_fundef_copies_table): Remove.
> (get_fundef_copy): Use hash_map_safe_get_or_insert.
> * cp-objcp-common.c (cp_get_debug_type): Use hash_map_safe_*.
> * decl.c (store_decomp_type): Remove.
> (cp_finish_decomp): Use hash_map_safe_put.
> * init.c (get_nsdmi): Use hash_map_safe_*.
> * pt.c (store_defaulted_ttp, lookup_defaulted_ttp): Remove.
> (add_defaults_to_ttp): Use hash_map_safe_*.
> ---
>  gcc/hash-map.h   | 38 --
>  gcc/cp/constexpr.c   | 14 ++
>  gcc/cp/cp-objcp-common.c |  6 ++
>  gcc/cp/decl.c|  9 +
>  gcc/cp/init.c|  9 ++---
>  gcc/cp/pt.c  | 21 +++--
>  gcc/hash-table.c |  2 +-
>  7 files changed, 47 insertions(+), 52 deletions(-)
>
> diff --git a/gcc/hash-map.h b/gcc/hash-map.h
> index ba20fe79f23..e638f761465 100644
> --- a/gcc/hash-map.h
> +++ b/gcc/hash-map.h
> @@ -128,8 +128,9 @@ class GTY((user)) hash_map
> }
>};
>
> +  static const size_t default_size = 13;
>  public:
> -  explicit hash_map (size_t n = 13, bool ggc = false,
> +  explicit hash_map (size_t n = default_size, bool ggc = false,
>  bool sanitize_eq_and_hash = true,
>  bool gather_mem_stats = GATHER_STATISTICS
>  CXX_MEM_STAT_INFO)
> @@ -146,7 +147,7 @@ public:
>HASH_MAP_ORIGIN PASS_MEM_STAT) {}
>
>/* Create a hash_map in ggc memory.  */
> -  static hash_map *create_ggc (size_t size,
> +  static hash_map *create_ggc (size_t size = default_size,
>bool gather_mem_stats = GATHER_STATISTICS
>CXX_MEM_STAT_INFO)
>  {
> @@ -326,4 +327,37 @@ gt_pch_nx (hash_map *h, gt_pointer_operator op, 
> void *cookie)
>op (>m_table.m_entries, cookie);
>  }
>
> +template
> +inline hash_map *
> +hash_map_maybe_create (hash_map *)
> +{
> +  if (!h)
> +h = h->create_ggc ();
> +  return h;
> +}
> +
> +/* Like h->get, but handles null h.  */
> +template
> +inline V*
> +hash_map_safe_get (hash_map *h, const K& k)
> +{
> +  return h ? h->get (k) : NULL;
> +}
> +
> +/* Like h->get, but handles null h.  */
> +template
> +inline V&
> +hash_map_safe_get_or_insert (hash_map *, const K& k, bool *e = NULL)
> +{
> +  return hash_map_maybe_create (h)->get_or_insert (k, e);
> +}
> +
> +/* Like h->put, but handles null h.  */
> +template
> +inline bool
> +hash_map_safe_put (hash_map *, const K& k, const V& v)
> +{
> +  return hash_map_maybe_create (h)->put (k, v);
> +}
> +
>  #endif
> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> index cb5484f4b72..904b70a9c99 100644
> --- a/gcc/cp/constexpr.c
> +++ b/gcc/cp/constexpr.c
> @@ -1098,15 +1098,6 @@ maybe_initialize_constexpr_call_table (void)
>
>  static GTY(()) hash_map *fundef_copies_table;
>
> -/* Initialize FUNDEF_COPIES_TABLE if it's not initialized.  */
> -
> -static void
> -maybe_initialize_fundef_copies_table ()
> -{
> -  if (fundef_copies_table == NULL)
> -fundef_copies_table = hash_map::create_ggc (101);
> -}
> -
>  /* Reuse a copy or create a new unshared copy of the function FUN.
> Return this copy.  We use a TREE_LIST whose PURPOSE is body, VALUE
> is parms, TYPE is result.  */
> @@ -1114,11 +1105,10 @@ maybe_initialize_fundef_copies_table ()
>  static tree
>  get_fundef_copy (constexpr_fundef *fundef)
>  {
> -  maybe_initialize_fundef_copies_table ();
> -
>tree copy;
>bool existed;
> -  tree *slot = _copies_table->get_or_insert (fundef->decl, );
> +  tree *slot = _map_safe_get_or_insert (fundef_copies_table,
> +fundef->decl, );
>
>if (!existed)
>  {
> diff --git a/gcc/cp/cp-objcp-common.c b/gcc/cp/cp-objcp-common.c
> index 4369a5b5570..652b94230ad 100644
> --- a/gcc/cp/cp-objcp-common.c
> +++ b/gcc/cp/cp-objcp-common.c
> @@ -145,11 +145,9 @@ cp_get_debug_type (const_tree type)
>if (dtype)
>  {
>tree ktype = CONST_CAST_TREE (type);
> -  if (debug_type_map == NULL)
> -   debug_type_map = tree_cache_map::create_ggc (512);
> -  else if (tree *slot = debug_type_map->get (ktype))
> +  if (tree *slot = hash_map_safe_get (debug_type_map, ktype))
> 

Re: [PATCH] C testsuite, silence a FreeBSD libc warning

[Andreas Tobler]

On 30.09.19 19:47, Jakub Jelinek wrote:

On Mon, Sep 30, 2019 at 07:41:00PM +0200, Andreas Tobler wrote:

--- fprintf-2.c (revision 276292)
+++ fprintf-2.c (working copy)
@@ -1,7 +1,8 @@
  /* Verify that calls to fprintf don't get eliminated even if their
 result on success can be computed at compile time (they can fail).
 The calls can still be transformed into those of other functions.
-   { dg-skip-if "requires io" { freestanding } } */
+   { dg-skip-if "requires io" { freestanding } }
+   { dg-prune-output "(^|\n)(\[^\n\])*warning: warning: \[^\n\]* possibly used 
unsafely; consider using \[^\n\]*\n" } */


I'm worried about that (^|\n) at the start + \n at the end, doesn't it prune
too much then?
Looking at other tests, they dg-prune-output just a few words from a
message, or .*few words.*
So, can you try just
{ dg-prune-output "warning: warning: \[^\n\r\]* possibly used unsafely; consider 
using" } */
or if that doesn't work, with .* at start end end?


Ok, yes it works too with your suggestion.

Attached.

One question, doing it per test case is cheaper than in prune.exp? 
(Where we do it for 'all' test cases, needed or not.)


Thanks,
Andreas



Index: fprintf-2.c
===
--- fprintf-2.c (revision 276292)
+++ fprintf-2.c (working copy)
@@ -1,7 +1,8 @@
 /* Verify that calls to fprintf don't get eliminated even if their
result on success can be computed at compile time (they can fail).
The calls can still be transformed into those of other functions.
-   { dg-skip-if "requires io" { freestanding } } */
+   { dg-skip-if "requires io" { freestanding } }
+   { dg-prune-output "warning: warning: \[^\n\r\]* possibly used unsafely; 
consider using" } */
 
 #include 
 #include 
Index: printf-2.c
===
--- printf-2.c  (revision 276292)
+++ printf-2.c  (working copy)
@@ -2,7 +2,8 @@
result on success can be computed at compile time (they can fail).
The calls can still be transformed into those of other functions.
{ dg-require-effective-target unwrapped }
-   { dg-skip-if "requires io" { freestanding } } */
+   { dg-skip-if "requires io" { freestanding } }
+   { dg-prune-output "warning: warning: \[^\n\r\]* possibly used unsafely; 
consider using" } */
 
 #include 
 #include 
Index: user-printf.c
===
--- user-printf.c   (revision 276292)
+++ user-printf.c   (working copy)
@@ -2,7 +2,8 @@
don't get eliminated even if their result on success can be computed at
compile time (they can fail).
{ dg-require-effective-target unwrapped }
-   { dg-skip-if "requires io" { freestanding } } */
+   { dg-skip-if "requires io" { freestanding } }
+   { dg-prune-output "warning: warning: \[^\n\r\]* possibly used unsafely; 
consider using" } */
 
 #include 
 #include 

[PATCH] Add some hash_map_safe_* functions like vec_safe_*.

[Jason Merrill]

gcc/
* hash-map.h (default_size): Put in member variable.
(create_ggc): Use it as default argument.
(hash_map_maybe_create, hash_map_safe_get)
(hash_map_safe_get_or_insert, hash_map_safe_put): New fns.
gcc/cp/
* constexpr.c (maybe_initialize_fundef_copies_table): Remove.
(get_fundef_copy): Use hash_map_safe_get_or_insert.
* cp-objcp-common.c (cp_get_debug_type): Use hash_map_safe_*.
* decl.c (store_decomp_type): Remove.
(cp_finish_decomp): Use hash_map_safe_put.
* init.c (get_nsdmi): Use hash_map_safe_*.
* pt.c (store_defaulted_ttp, lookup_defaulted_ttp): Remove.
(add_defaults_to_ttp): Use hash_map_safe_*.
---
 gcc/hash-map.h   | 38 --
 gcc/cp/constexpr.c   | 14 ++
 gcc/cp/cp-objcp-common.c |  6 ++
 gcc/cp/decl.c|  9 +
 gcc/cp/init.c|  9 ++---
 gcc/cp/pt.c  | 21 +++--
 gcc/hash-table.c |  2 +-
 7 files changed, 47 insertions(+), 52 deletions(-)

diff --git a/gcc/hash-map.h b/gcc/hash-map.h
index ba20fe79f23..e638f761465 100644
--- a/gcc/hash-map.h
+++ b/gcc/hash-map.h
@@ -128,8 +128,9 @@ class GTY((user)) hash_map
}
   };
 
+  static const size_t default_size = 13;
 public:
-  explicit hash_map (size_t n = 13, bool ggc = false,
+  explicit hash_map (size_t n = default_size, bool ggc = false,
 bool sanitize_eq_and_hash = true,
 bool gather_mem_stats = GATHER_STATISTICS
 CXX_MEM_STAT_INFO)
@@ -146,7 +147,7 @@ public:
   HASH_MAP_ORIGIN PASS_MEM_STAT) {}
 
   /* Create a hash_map in ggc memory.  */
-  static hash_map *create_ggc (size_t size,
+  static hash_map *create_ggc (size_t size = default_size,
   bool gather_mem_stats = GATHER_STATISTICS
   CXX_MEM_STAT_INFO)
 {
@@ -326,4 +327,37 @@ gt_pch_nx (hash_map *h, gt_pointer_operator op, 
void *cookie)
   op (>m_table.m_entries, cookie);
 }
 
+template
+inline hash_map *
+hash_map_maybe_create (hash_map *)
+{
+  if (!h)
+h = h->create_ggc ();
+  return h;
+}
+
+/* Like h->get, but handles null h.  */
+template
+inline V*
+hash_map_safe_get (hash_map *h, const K& k)
+{
+  return h ? h->get (k) : NULL;
+}
+
+/* Like h->get, but handles null h.  */
+template
+inline V&
+hash_map_safe_get_or_insert (hash_map *, const K& k, bool *e = NULL)
+{
+  return hash_map_maybe_create (h)->get_or_insert (k, e);
+}
+
+/* Like h->put, but handles null h.  */
+template
+inline bool
+hash_map_safe_put (hash_map *, const K& k, const V& v)
+{
+  return hash_map_maybe_create (h)->put (k, v);
+}
+
 #endif
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index cb5484f4b72..904b70a9c99 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1098,15 +1098,6 @@ maybe_initialize_constexpr_call_table (void)
 
 static GTY(()) hash_map *fundef_copies_table;
 
-/* Initialize FUNDEF_COPIES_TABLE if it's not initialized.  */
-
-static void
-maybe_initialize_fundef_copies_table ()
-{
-  if (fundef_copies_table == NULL)
-fundef_copies_table = hash_map::create_ggc (101);
-}
-
 /* Reuse a copy or create a new unshared copy of the function FUN.
Return this copy.  We use a TREE_LIST whose PURPOSE is body, VALUE
is parms, TYPE is result.  */
@@ -1114,11 +1105,10 @@ maybe_initialize_fundef_copies_table ()
 static tree
 get_fundef_copy (constexpr_fundef *fundef)
 {
-  maybe_initialize_fundef_copies_table ();
-
   tree copy;
   bool existed;
-  tree *slot = _copies_table->get_or_insert (fundef->decl, );
+  tree *slot = _map_safe_get_or_insert (fundef_copies_table,
+fundef->decl, );
 
   if (!existed)
 {
diff --git a/gcc/cp/cp-objcp-common.c b/gcc/cp/cp-objcp-common.c
index 4369a5b5570..652b94230ad 100644
--- a/gcc/cp/cp-objcp-common.c
+++ b/gcc/cp/cp-objcp-common.c
@@ -145,11 +145,9 @@ cp_get_debug_type (const_tree type)
   if (dtype)
 {
   tree ktype = CONST_CAST_TREE (type);
-  if (debug_type_map == NULL)
-   debug_type_map = tree_cache_map::create_ggc (512);
-  else if (tree *slot = debug_type_map->get (ktype))
+  if (tree *slot = hash_map_safe_get (debug_type_map, ktype))
return *slot;
-  debug_type_map->put (ktype, dtype);
+  hash_map_safe_put (debug_type_map, ktype, dtype);
 }
 
   return dtype;
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 67c4521e98c..562fe10bd96 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -7704,13 +7704,6 @@ get_tuple_decomp_init (tree decl, unsigned i)
based on the actual type of the variable, so store it in a hash table.  */
 
 static GTY((cache)) tree_cache_map *decomp_type_table;
-static void
-store_decomp_type (tree v, tree t)
-{
-  if (!decomp_type_table)
-decomp_type_table = tree_cache_map::create_ggc (13);
-  decomp_type_table->put (v, t);
-}
 
 tree
 lookup_decomp_type 

Re: [x86] Cache result of expensive_function_p between frame layouts

[Jan Hubicka]

> ix86_compute_frame_layout sets use_fast_prologue_epilogue if
> the function isn't more expensive than a certain threshold,
> where the threshold depends on the number of saved registers.
> However, the RA is allowed to insert and delete instructions
> as it goes along, which can change whether this threshold is
> crossed or not.
> 
> I hit this with an RA change I'm working on.  Rematerialisation
> was able to remove an instruction and avoid a spill, which happened
> to bring the size of the function below the threshold.  But since
> nothing legitimately frame-related had changed, there was no need for
> the RA to lay out the frame again.  We then failed the final sanity
> check in lra_eliminate.
> 
> Tested on x86_64-linux-gnu.  OK to install?
> 
> Richard
> 
> 
> 2019-09-30  Richard Sandiford  
> 
> gcc/
>   * config/i386/i386.h (ix86_frame::expensive_p): New field.
>   (ix86_frame::expensive_count): Likewise.
>   * config/i386/i386.c (ix86_compute_frame_layout): Make the choice
>   of use_fast_prologue_epilogue robust against incidental changes
>   in function size.
OK,
Honza

Re: [PATCH] ifcvt: improve cost estimation (PR 87047)

[Alexander Monakov]

On Mon, 30 Sep 2019, Alexander Monakov wrote:

> +static unsigned
> +average_cost (unsigned then_cost, unsigned else_cost, edge e)
> +{
> +  return else_cost + e->probability.apply ((int) then_cost - else_cost);

Ugh, I made a wrong last-minute edit here, we want signed cost difference so
the argument to probability.apply should be

  (int) (then_cost - else_cost)

or

  (int) then_cost - (int) else_cost.

The patch I bootstrapped and passed Martin for testing correctly had

  (gcov_type) then_cost - else_cost

(gcov_type is int64).

Alexander

Re: [PATCH] C testsuite, silence a FreeBSD libc warning

[Jakub Jelinek]

On Mon, Sep 30, 2019 at 07:41:00PM +0200, Andreas Tobler wrote:
> --- fprintf-2.c   (revision 276292)
> +++ fprintf-2.c   (working copy)
> @@ -1,7 +1,8 @@
>  /* Verify that calls to fprintf don't get eliminated even if their
> result on success can be computed at compile time (they can fail).
> The calls can still be transformed into those of other functions.
> -   { dg-skip-if "requires io" { freestanding } } */
> +   { dg-skip-if "requires io" { freestanding } }
> +   { dg-prune-output "(^|\n)(\[^\n\])*warning: warning: \[^\n\]* possibly 
> used unsafely; consider using \[^\n\]*\n" } */ 

I'm worried about that (^|\n) at the start + \n at the end, doesn't it prune
too much then?
Looking at other tests, they dg-prune-output just a few words from a
message, or .*few words.*
So, can you try just
   { dg-prune-output "warning: warning: \[^\n\r\]* possibly used unsafely; 
consider using" } */
or if that doesn't work, with .* at start end end?

Jakub

Re: [PATCH] C testsuite, silence a FreeBSD libc warning

[Andreas Tobler]

On 30.09.19 19:20, Jakub Jelinek wrote:

On Mon, Sep 30, 2019 at 07:15:59PM +0200, Andreas Tobler wrote:

Hi all,

the below patch tries to silence a warning from the FreeBSD libc. This
warning currently makes all the test cases fail where we make use of the
tmpnam() function.

---
/usr/local/bin/ld: /tmp//ccBQaYlC.o: in function `main':
printf-2.c:(.text+0x8d): warning: warning: tmpnam() possibly used unsafely;
consider using mkstemp()
---

With this patch, all the *printf* test cases in the gcc part are now
passing.

What dou you think?


Wouldn't it be better to just dg-prune-output it in the 3 affected tests,
perhaps only for affected targets?


Thanks for the feedback!

This way?

TIA,
Andreas


2019-09-30  Andreas Tobler  

* gcc.c-torture/execute/fprintf-2.c: Silence a FreeBSD libc warning.
* gcc.c-torture/execute/printf-2.c: Likewise.
* gcc.c-torture/execute/user-printf.c: Likewise.


Index: fprintf-2.c
===
--- fprintf-2.c (revision 276292)
+++ fprintf-2.c (working copy)
@@ -1,7 +1,8 @@
 /* Verify that calls to fprintf don't get eliminated even if their
result on success can be computed at compile time (they can fail).
The calls can still be transformed into those of other functions.
-   { dg-skip-if "requires io" { freestanding } } */
+   { dg-skip-if "requires io" { freestanding } }
+   { dg-prune-output "(^|\n)(\[^\n\])*warning: warning: \[^\n\]* possibly used 
unsafely; consider using \[^\n\]*\n" } */ 
 
 #include 
 #include 
Index: printf-2.c
===
--- printf-2.c  (revision 276292)
+++ printf-2.c  (working copy)
@@ -2,7 +2,8 @@
result on success can be computed at compile time (they can fail).
The calls can still be transformed into those of other functions.
{ dg-require-effective-target unwrapped }
-   { dg-skip-if "requires io" { freestanding } } */
+   { dg-skip-if "requires io" { freestanding } }
+   { dg-prune-output "(^|\n)(\[^\n\])*warning: warning: \[^\n\]* possibly used 
unsafely; consider using \[^\n\]*\n" } */
 
 #include 
 #include 
Index: user-printf.c
===
--- user-printf.c   (revision 276292)
+++ user-printf.c   (working copy)
@@ -2,7 +2,8 @@
don't get eliminated even if their result on success can be computed at
compile time (they can fail).
{ dg-require-effective-target unwrapped }
-   { dg-skip-if "requires io" { freestanding } } */
+   { dg-skip-if "requires io" { freestanding } }
+   { dg-prune-output "(^|\n)(\[^\n\])*warning: warning: \[^\n\]* possibly used 
unsafely; consider using \[^\n\]*\n" } */
 
 #include 
 #include 

[PATCH] ifcvt: improve cost estimation (PR 87047)

[Alexander Monakov]

Hi,

this patch corrects a problem where our cost estimation in if-conversion is
broken when the 'else' block is missing: we then take the cost of 'then' block,
which leads gcc willing to replace a conditionally-executed block with cost 80
with an unconditionally-executed block with cost 72.

Existing heuristic is clearly discontinuous: if the 'else' block wasn't missing
but instead had cost 1, we'd take the minimum of 1 and 80 and match replacement
cost against that.

A more sensible approach is to take average expected time of the if-then-else
(i.e. add costs of 'then' and 'else' scaled by their probabilities), e.g. in
the above example take 0.5*80 = 40 if we expect the block to be executed about
every other time.

Martin Jambor (thanks!) kindly benchmarked this for me on SPEC 2006 and 2017
(sans 521.wrf_r and 527.cam4_r).  There is a 2% improvement for 403.gcc and
3% for 433.milc, both on Skylake.

Considering also changes in 1..2% range, there might be a 1.5% regression on
526.blender_r on Zen 2, and 1.5% improvements for 520.omnetpp_r, 538.imagick_r
and 549.fotonik3d_r on Skylake.

In the PR Richi notes we sometimes won't get high-quality probability data
for such edges on RTL, but that shouldn't be a reason not to replace a broken
heuristic with a less broken one.

Bootstrapped on x86-64, ok for trunk?  For backports?

Thanks.
Alexander

* ifcvt.c (average_cost): New static function.  Use it...
(noce_process_if_block): ... here.

diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index e0c9522057a..283bb47f51a 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -3358,6 +3358,16 @@ bb_ok_for_noce_convert_multiple_sets (basic_block 
test_bb)
   return count > 1 && count <= param;
 }
 
+/* Compute average of two given costs weighted by relative probabilities
+   of respective basic blocks in an IF-THEN-ELSE.  E is the IF-THEN edge.
+   With P as the probability to take the IF-THEN branch, return
+   P * THEN_COST + (1 - P) * ELSE_COST.  */
+static unsigned
+average_cost (unsigned then_cost, unsigned else_cost, edge e)
+{
+  return else_cost + e->probability.apply ((int) then_cost - else_cost);
+}
+
 /* Given a simple IF-THEN-JOIN or IF-THEN-ELSE-JOIN block, attempt to convert
it without using conditional execution.  Return TRUE if we were successful
at converting the block.  */
@@ -3413,10 +3423,9 @@ noce_process_if_block (struct noce_if_info *if_info)
   _info->else_simple))
 return false;
 
-  if (else_bb == NULL)
-if_info->original_cost += then_cost;
-  else if (speed_p)
-if_info->original_cost += MIN (then_cost, else_cost);
+  if (speed_p)
+if_info->original_cost += average_cost (then_cost, else_cost,
+   find_edge (test_bb, then_bb));
   else
 if_info->original_cost += then_cost + else_cost;
 

Re: [PATCH] C testsuite, silence a FreeBSD libc warning

[Jakub Jelinek]

On Mon, Sep 30, 2019 at 07:15:59PM +0200, Andreas Tobler wrote:
> Hi all,
> 
> the below patch tries to silence a warning from the FreeBSD libc. This
> warning currently makes all the test cases fail where we make use of the
> tmpnam() function.
> 
> ---
> /usr/local/bin/ld: /tmp//ccBQaYlC.o: in function `main':
> printf-2.c:(.text+0x8d): warning: warning: tmpnam() possibly used unsafely;
> consider using mkstemp()
> ---
> 
> With this patch, all the *printf* test cases in the gcc part are now
> passing.
> 
> What dou you think?

Wouldn't it be better to just dg-prune-output it in the 3 affected tests,
perhaps only for affected targets?

Jakub

[PATCH] C testsuite, silence a FreeBSD libc warning

[Andreas Tobler]

Hi all,

the below patch tries to silence a warning from the FreeBSD libc. This 
warning currently makes all the test cases fail where we make use of the 
tmpnam() function.


---
/usr/local/bin/ld: /tmp//ccBQaYlC.o: in function `main':
printf-2.c:(.text+0x8d): warning: warning: tmpnam() possibly used 
unsafely; consider using mkstemp()

---

With this patch, all the *printf* test cases in the gcc part are now 
passing.


What dou you think?

TIA,
Andreas

2019-09-30  Andreas Tobler  

* lib/prune.exp (prune_gcc_output): Silence a FreeBSD libc warning.

Index: lib/prune.exp
===
--- lib/prune.exp   (revision 276342)
+++ lib/prune.exp   (working copy)
@@ -73,6 +73,9 @@
 # Ignore harmless warnings from Xcode 4.0.
 regsub -all "(^|\n)\[^\n\]*ld: warning: could not create compact 
unwind for\[^\n\]*" $text "" text


+# Ignore FreeBSD libc warning
+regsub -all "(^|\n)(\[^\n\])*warning: warning: \[^\n\]* possibly 
used unsafely; consider using \[^\n\]*\n" $text "" text

+
 # Call into multiline.exp to handle any multiline output directives.
 set text [handle-multiline-outputs $text]

[PATCH] libstdc++ testsuite, silence a FreeBSD libm warning

[Andreas Tobler]

Hi all,

the attached patch tries to silence a warning from the msun of FreeBSD.
When compiling the special_functions testcases, I get excess errors 
because of this warning:


warning: tgammal has lower than advertised precision

With the patch, all special_functions test cases pass now.

What do you think?

TIA,
Andreas


2019-09-30  Andreas Tobler  

* testsuite/lib/prune.exp (libstdc++-dg-prune): Ignore the FreeBSD
warning about lower advertised precision of tgammal.


Index: testsuite/lib/prune.exp
===
--- testsuite/lib/prune.exp (revision 276342)
+++ testsuite/lib/prune.exp (working copy)
@@ -66,6 +66,10 @@
 # Ignore harmless warnings from Xcode 4.0.
 regsub -all "(^|\n)\[^\n\]*ld: warning: could not create compact 
unwind for\[^\n\]*" $text "" text


+# Ignore FreeBSD msun warning
+regsub -all "(^|\n)\[^\n]*ld: \[^\n\]* in function \[^\n\]*" $text 
"" text
+regsub -all "(^|\n)\[^\n]*warning: tgammal has lower than 
advertised precision" $text "" text

+
 foreach p $additional_prunes {
if { [string length $p] > 0 } {
# Following regexp matches a complete line containing $p.

[PATCH] Include netinet/in.h in include/experimental/internet

[Andreas Tobler]

Hi all,

I'm going to commit the attached patch to trunk.
It is preapproved by Jonathan. (via #irc)

On FreeBSD the netinet/in.h header is not included by arpa/inet.h, so do 
the include if we have the _GLIBCXX_HAVE_NETINET_IN_H defined.


This makes all the experimental/net/internet test cases pass here.

Any comments?

TIA,
Andreas

Commit one-liner:

Include netinet/in.h in include/experimental/internet

2019-09-30  Andreas Tobler  

* include/experimental/internet: Include netinet/in.h if we have
_GLIBCXX_HAVE_NETINET_IN_H defined.

Index: include/experimental/internet
===
--- include/experimental/internet   (revision 276342)
+++ include/experimental/internet   (working copy)
@@ -51,6 +51,9 @@
 #ifdef _GLIBCXX_HAVE_ARPA_INET_H
 # include   // inet_ntop
 #endif
+#ifdef _GLIBCXX_HAVE_NETINET_IN_H
+# include  // IPPROTO_IP
+#endif
 #ifdef _GLIBCXX_HAVE_NETINET_TCP_H
 # include // TCP_NODELAY
 #endif

Re: [AArch64][SVE] Utilize ASRD instruction for division and remainder

[Richard Sandiford]

Yuliang Wang  writes:
> Thanks for the corrections, updated.
>
> Regards
> Yuliang
>
> (no ChangeLog updates)

Thanks, applied as r276343.

Richard

Re: [AArch64] Allow shrink-wrapping of non-leaf vector PCS functions

[Richard Sandiford]

Richard Sandiford  writes:
> [This follows on from:
>  https://gcc.gnu.org/ml/gcc-patches/2019-09/msg00778.html
>  https://gcc.gnu.org/ml/gcc-patches/2019-09/msg01456.html]
>
> With the function ABI stuff, we can now support shrink-wrapping of
> non-leaf vector PCS functions.  This is particularly useful if the
> vector PCS function calls an ordinary function on an error path,
> since we can then keep the extra saves and restores specific to
> that path too.
>
> Tested on aarch64-linux-gnu.  OK to install?

Now self-approved :-), applied as r276340.

Richard

Re: [AArch64] Make more use of function_abi

[Richard Sandiford]

Richard Sandiford  writes:
> [This follows on from:
>  https://gcc.gnu.org/ml/gcc-patches/2019-09/msg00778.html
>  https://gcc.gnu.org/ml/gcc-patches/2019-09/msg01456.html]
>
> This patch makes more use of the function_abi infrastructure.
> We can then avoid checking specifically for the vector PCS in
> a few places, and can test it more directly otherwise.
>
> Specifically: we no longer need to call df_set_regs_ever_live
> for the extra call-saved registers, since IRA now does that for us.
> We also don't need to handle the vector PCS specially in
> aarch64_epilogue_uses, because DF now marks the registers
> as live on exit.
>
> Tested on aarch64-linux-gnu.  OK to install?
>
> Richard

Now self-approved and applied as r276341.

Richard

Re: [03/32] Add a function for getting the ABI of a call insn target

[Jeff Law]

On 9/30/19 10:32 AM, Richard Sandiford wrote:
> Jeff Law  writes:
>> On 9/25/19 9:38 AM, Richard Sandiford wrote:
>>> Richard Sandiford  writes:
 This patch replaces get_call_reg_set_usage with call_insn_abi,
 which returns the ABI of the target of a call insn.  The ABI's
 full_reg_clobbers corresponds to regs_invalidated_by_call,
 whereas many callers instead passed call_used_or_fixed_regs, i.e.:

   (regs_invalidated_by_call | fixed_reg_set)

 The patch slavishly preserves the "| fixed_reg_set" for these callers;
 later patches will clean this up.
>>>
>>> On reflection, I think insn_callee_abi would be a better name for the
>>> function than call_insn_abi, since it should make it clearer that the
>>> function returns the ABI of the target function.  In future we could
>>> have expr_callee_abi for CALL_EXPRs.
>>>
>>> Also, after Segher's comments for 10/32, I've used "callee_abi" as
>>> the name of temporary variables, instead of just "abi".
>>>
>>> I've made the same change for later patches (except where I've posted
>>> new versions instead), but it didn't seem worth spamming the lists
>>> with that.
>>>
>>> Tested as before.
>>>
>>> Richard
>>>
>>> PS. Ping for the series :-)
>>>
>>>
>>> 2019-09-25  Richard Sandiford  
>>>
>>> gcc/
>>> * target.def (insn_callee_abi): New hook.
>>> (remove_extra_call_preserved_regs): Delete.
>>> * doc/tm.texi.in (TARGET_INSN_CALLEE_ABI): New macro.
>>> (TARGET_REMOVE_EXTRA_CALL_PRESERVED_REGS): Delete.
>>> * doc/tm.texi: Regenerate.
>>> * targhooks.h (default_remove_extra_call_preserved_regs): Delete.
>>> * targhooks.c (default_remove_extra_call_preserved_regs): Delete.
>>> * config/aarch64/aarch64.c (aarch64_simd_call_p): Constify the
>>> insn argument.
>>> (aarch64_remove_extra_call_preserved_regs): Delete.
>>> (aarch64_insn_callee_abi): New function.
>>> (TARGET_REMOVE_EXTRA_CALL_PRESERVED_REGS): Delete.
>>> (TARGET_INSN_CALLEE_ABI): New macro.
>>> * rtl.h (get_call_fndecl): Declare.
>>> (cgraph_rtl_info): Fix formatting.  Tweak comment for
>>> function_used_regs.  Remove function_used_regs_valid.
>>> * rtlanal.c (get_call_fndecl): Moved from final.c
>>> * function-abi.h (insn_callee_abi): Declare.
>>> (target_function_abi_info): Mention insn_callee_abi.
>>> * function-abi.cc (fndecl_abi): Handle flag_ipa_ra in a similar
>>> way to get_call_reg_set_usage did.
>>> (insn_callee_abi): New function.
>>> * regs.h (get_call_reg_set_usage): Delete.
>>> * final.c: Include function-abi.h.
>>> (collect_fn_hard_reg_usage): Add fixed and stack registers to
>>> function_used_regs before the main loop rather than afterwards.
>>> Use insn_callee_abi instead of get_call_reg_set_usage.  Exit early
>>> if function_used_regs ends up not being useful.
>>> (get_call_fndecl): Move to rtlanal.c
>>> (get_call_cgraph_rtl_info, get_call_reg_set_usage): Delete.
>>> * caller-save.c: Include function-abi.h.
>>> (setup_save_areas, save_call_clobbered_regs): Use insn_callee_abi
>>> instead of get_call_reg_set_usage.
>>> * cfgcleanup.c: Include function-abi.h.
>>> (old_insns_match_p): Use insn_callee_abi instead of
>>> get_call_reg_set_usage.
>>> * cgraph.h (cgraph_node::rtl_info): Take a const_tree instead of
>>> a tree.
>>> * cgraph.c (cgraph_node::rtl_info): Likewise.  Initialize
>>> function_used_regs.
>>> * df-scan.c: Include function-abi.h.
>>> (df_get_call_refs): Use insn_callee_abi instead of
>>> get_call_reg_set_usage.
>>> * ira-lives.c: Include function-abi.h.
>>> (process_bb_node_lives): Use insn_callee_abi instead of
>>> get_call_reg_set_usage.
>>> * lra-lives.c: Include function-abi.h.
>>> (process_bb_lives): Use insn_callee_abi instead of
>>> get_call_reg_set_usage.
>>> * postreload.c: Include function-abi.h.
>>> (reload_combine): Use insn_callee_abi instead of
>>> get_call_reg_set_usage.
>>> * regcprop.c: Include function-abi.h.
>>> (copyprop_hardreg_forward_1): Use insn_callee_abi instead of
>>> get_call_reg_set_usage.
>>> * resource.c: Include function-abi.h.
>>> (mark_set_resources, mark_target_live_regs): Use insn_callee_abi
>>> instead of get_call_reg_set_usage.
>>> * var-tracking.c: Include function-abi.h.
>>> (dataflow_set_clear_at_call): Use insn_callee_abi instead of
>>> get_call_reg_set_usage.
>>>
>> OK.  I think that's the whole set, right?
> 
> Yep, all committed now :-)  Thanks again for the reviews.
> 
>> There were a ton of guality issues over the last couple weeks in the
>> tester, but those were somewhat expected due to a prior patchkit.  The
>> affected targets have new baseline results in my tester, so if anything
>> goes nuts as a result of your patches it should be fairly obvious.
> 
> There was certainly an extra guality failure on aarch64-linux-gnu with
> this series, think it was:
> 

[PATCH, RS6000] Add movmemsi pattern for inline expansion of memmove()

[Aaron Sawdey]

This patch uses the support added in the patch I posted last week for actually 
doing
inline expansion of memmove(). I've added a might_overlap parameter to 
expand_block_move()
to tell it when it must make sure to handle overlapping moves. I changed the 
code to
save up the generated rtx for both loads and stores instead of just stores. In 
the
might_overlap==true case, if we get to MAX_MOVE_REG and the move is not done 
yet, then
we bail out and return false. So what this can now do is inline expand any 
memmove()
that can be done in 4 loads followed by 4 stores. It will use lxv/stxv if 
size/alignment
allows, otherwise it will use unaligned integer loads/stores. So it can expand 
most
memmove() up to 32 bytes, and some that are 33-64 bytes if the arguments are 16 
byte
aligned.

I've also removed the code from expand_block_move() for dealing with
mode==BLKmode because I don't believe that can happen. The big if construct that
figures out which size we are going to use has a plain else on it, and every
clause in it sets mode to something other than BLKmode. So I removed that code
to simplify things and just left a gcc_assert(mode != BLKmode).

Regtest in progress on ppc64le (power9), if tests are ok, is this ok for trunk
after the movmem optab patch posted last week is approved?

Thanks!
   Aaron

2019-09-30  Aaron Sawdey 

* config/rs6000/rs6000-protos.h (expand_block_move): Change prototype.
* config/rs6000/rs6000-string.c (expand_block_move): Add might_overlap 
parm.
* config/rs6000/rs6000.md (movmemsi): Add new pattern.
(cpymemsi): Add might_overlap parm to expand_block_move() call.



Index: gcc/config/rs6000/rs6000-protos.h
===
--- gcc/config/rs6000/rs6000-protos.h   (revision 276131)
+++ gcc/config/rs6000/rs6000-protos.h   (working copy)
@@ -69,7 +69,7 @@
 extern void rs6000_generate_float2_double_code (rtx, rtx, rtx);
 extern void rs6000_generate_vsigned2_code (bool, rtx, rtx, rtx);
 extern int expand_block_clear (rtx[]);
-extern int expand_block_move (rtx[]);
+extern int expand_block_move (rtx[], bool);
 extern bool expand_block_compare (rtx[]);
 extern bool expand_strn_compare (rtx[], int);
 extern bool rs6000_is_valid_mask (rtx, int *, int *, machine_mode);
Index: gcc/config/rs6000/rs6000-string.c
===
--- gcc/config/rs6000/rs6000-string.c   (revision 276131)
+++ gcc/config/rs6000/rs6000-string.c   (working copy)
@@ -2719,7 +2719,7 @@
 #define MAX_MOVE_REG 4

 int
-expand_block_move (rtx operands[])
+expand_block_move (rtx operands[], bool might_overlap)
 {
   rtx orig_dest = operands[0];
   rtx orig_src = operands[1];
@@ -2730,6 +2730,7 @@
   int bytes;
   int offset;
   int move_bytes;
+  rtx loads[MAX_MOVE_REG];
   rtx stores[MAX_MOVE_REG];
   int num_reg = 0;

@@ -2817,47 +2818,35 @@
  gen_func.mov = gen_movqi;
}

+  /* Mode is always set to something other than BLKmode by one of the
+cases of the if statement above.  */
+  gcc_assert (mode != BLKmode);
+
   src = adjust_address (orig_src, mode, offset);
   dest = adjust_address (orig_dest, mode, offset);

-  if (mode != BLKmode)
-   {
- rtx tmp_reg = gen_reg_rtx (mode);
+  rtx tmp_reg = gen_reg_rtx (mode);
+
+  loads[num_reg]= (*gen_func.mov) (tmp_reg, src);
+  stores[num_reg++] = (*gen_func.mov) (dest, tmp_reg);

- emit_insn ((*gen_func.mov) (tmp_reg, src));
- stores[num_reg++] = (*gen_func.mov) (dest, tmp_reg);
-   }
+  /* If we didn't succeed in doing it in one pass, we can't do it in the
+might_overlap case.  Bail out and return failure.  */
+  if (might_overlap && num_reg >= MAX_MOVE_REG
+ && bytes > move_bytes)
+   return 0;

-  if (mode == BLKmode || num_reg >= MAX_MOVE_REG || bytes == move_bytes)
+  /* Emit loads and stores saved up.  */
+  if (num_reg >= MAX_MOVE_REG || bytes == move_bytes)
{
  int i;
  for (i = 0; i < num_reg; i++)
+   emit_insn (loads[i]);
+ for (i = 0; i < num_reg; i++)
emit_insn (stores[i]);
  num_reg = 0;
}
-
-  if (mode == BLKmode)
-   {
- /* Move the address into scratch registers.  The movmemsi
-patterns require zero offset.  */
- if (!REG_P (XEXP (src, 0)))
-   {
- rtx src_reg = copy_addr_to_reg (XEXP (src, 0));
- src = replace_equiv_address (src, src_reg);
-   }
- set_mem_size (src, move_bytes);
-
- if (!REG_P (XEXP (dest, 0)))
-   {
- rtx dest_reg = copy_addr_to_reg (XEXP (dest, 0));
- dest = replace_equiv_address (dest, dest_reg);
-   }
- set_mem_size (dest, move_bytes);
-
- emit_insn ((*gen_func.movmemsi) (dest, src,
-  GEN_INT (move_bytes & 31),
-

Re: [03/32] Add a function for getting the ABI of a call insn target

[Richard Sandiford]

Jeff Law  writes:
> On 9/25/19 9:38 AM, Richard Sandiford wrote:
>> Richard Sandiford  writes:
>>> This patch replaces get_call_reg_set_usage with call_insn_abi,
>>> which returns the ABI of the target of a call insn.  The ABI's
>>> full_reg_clobbers corresponds to regs_invalidated_by_call,
>>> whereas many callers instead passed call_used_or_fixed_regs, i.e.:
>>>
>>>   (regs_invalidated_by_call | fixed_reg_set)
>>>
>>> The patch slavishly preserves the "| fixed_reg_set" for these callers;
>>> later patches will clean this up.
>> 
>> On reflection, I think insn_callee_abi would be a better name for the
>> function than call_insn_abi, since it should make it clearer that the
>> function returns the ABI of the target function.  In future we could
>> have expr_callee_abi for CALL_EXPRs.
>> 
>> Also, after Segher's comments for 10/32, I've used "callee_abi" as
>> the name of temporary variables, instead of just "abi".
>> 
>> I've made the same change for later patches (except where I've posted
>> new versions instead), but it didn't seem worth spamming the lists
>> with that.
>> 
>> Tested as before.
>> 
>> Richard
>> 
>> PS. Ping for the series :-)
>> 
>> 
>> 2019-09-25  Richard Sandiford  
>> 
>> gcc/
>>  * target.def (insn_callee_abi): New hook.
>>  (remove_extra_call_preserved_regs): Delete.
>>  * doc/tm.texi.in (TARGET_INSN_CALLEE_ABI): New macro.
>>  (TARGET_REMOVE_EXTRA_CALL_PRESERVED_REGS): Delete.
>>  * doc/tm.texi: Regenerate.
>>  * targhooks.h (default_remove_extra_call_preserved_regs): Delete.
>>  * targhooks.c (default_remove_extra_call_preserved_regs): Delete.
>>  * config/aarch64/aarch64.c (aarch64_simd_call_p): Constify the
>>  insn argument.
>>  (aarch64_remove_extra_call_preserved_regs): Delete.
>>  (aarch64_insn_callee_abi): New function.
>>  (TARGET_REMOVE_EXTRA_CALL_PRESERVED_REGS): Delete.
>>  (TARGET_INSN_CALLEE_ABI): New macro.
>>  * rtl.h (get_call_fndecl): Declare.
>>  (cgraph_rtl_info): Fix formatting.  Tweak comment for
>>  function_used_regs.  Remove function_used_regs_valid.
>>  * rtlanal.c (get_call_fndecl): Moved from final.c
>>  * function-abi.h (insn_callee_abi): Declare.
>>  (target_function_abi_info): Mention insn_callee_abi.
>>  * function-abi.cc (fndecl_abi): Handle flag_ipa_ra in a similar
>>  way to get_call_reg_set_usage did.
>>  (insn_callee_abi): New function.
>>  * regs.h (get_call_reg_set_usage): Delete.
>>  * final.c: Include function-abi.h.
>>  (collect_fn_hard_reg_usage): Add fixed and stack registers to
>>  function_used_regs before the main loop rather than afterwards.
>>  Use insn_callee_abi instead of get_call_reg_set_usage.  Exit early
>>  if function_used_regs ends up not being useful.
>>  (get_call_fndecl): Move to rtlanal.c
>>  (get_call_cgraph_rtl_info, get_call_reg_set_usage): Delete.
>>  * caller-save.c: Include function-abi.h.
>>  (setup_save_areas, save_call_clobbered_regs): Use insn_callee_abi
>>  instead of get_call_reg_set_usage.
>>  * cfgcleanup.c: Include function-abi.h.
>>  (old_insns_match_p): Use insn_callee_abi instead of
>>  get_call_reg_set_usage.
>>  * cgraph.h (cgraph_node::rtl_info): Take a const_tree instead of
>>  a tree.
>>  * cgraph.c (cgraph_node::rtl_info): Likewise.  Initialize
>>  function_used_regs.
>>  * df-scan.c: Include function-abi.h.
>>  (df_get_call_refs): Use insn_callee_abi instead of
>>  get_call_reg_set_usage.
>>  * ira-lives.c: Include function-abi.h.
>>  (process_bb_node_lives): Use insn_callee_abi instead of
>>  get_call_reg_set_usage.
>>  * lra-lives.c: Include function-abi.h.
>>  (process_bb_lives): Use insn_callee_abi instead of
>>  get_call_reg_set_usage.
>>  * postreload.c: Include function-abi.h.
>>  (reload_combine): Use insn_callee_abi instead of
>>  get_call_reg_set_usage.
>>  * regcprop.c: Include function-abi.h.
>>  (copyprop_hardreg_forward_1): Use insn_callee_abi instead of
>>  get_call_reg_set_usage.
>>  * resource.c: Include function-abi.h.
>>  (mark_set_resources, mark_target_live_regs): Use insn_callee_abi
>>  instead of get_call_reg_set_usage.
>>  * var-tracking.c: Include function-abi.h.
>>  (dataflow_set_clear_at_call): Use insn_callee_abi instead of
>>  get_call_reg_set_usage.
>> 
> OK.  I think that's the whole set, right?

Yep, all committed now :-)  Thanks again for the reviews.

> There were a ton of guality issues over the last couple weeks in the
> tester, but those were somewhat expected due to a prior patchkit.  The
> affected targets have new baseline results in my tester, so if anything
> goes nuts as a result of your patches it should be fairly obvious.

There was certainly an extra guality failure on aarch64-linux-gnu with
this series, think it was:

gcc.dg/guality/pr41616-1.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-object

Will be 

Re: [11/32] Remove global call sets: cse.c

[Richard Sandiford]

Jeff Law  writes:
> On 9/25/19 9:57 AM, Richard Sandiford wrote:
>> Richard Sandiford  writes:
>>> Like with the combine.c patch, this one keeps things simple by
>>> invalidating values in partially-clobbered registers, rather than
>>> trying to tell whether the value in a partially-clobbered register
>>> is actually clobbered or not.  Again, this is in principle a bug fix,
>>> but probably never matters in practice.
>> 
>> Similary to the combine patch, I've updated this to avoid the
>> short "abi" name and use a temporary HARD_REG_SET instead.
>> 
>> Richard
>> 
>> 
>> 2019-09-25  Richard Sandiford  
>> 
>> gcc/
>>  * cse.c: Include regs.h and function-abi.h.
>>  (invalidate_for_call): Take the call insn as an argument.
>>  Use insn_callee_abi to get the ABI of the call and invalidate
>>  partially clobbered registers as well as fully clobbered ones.
>>  (cse_insn): Update call accordingly.
>> 
>> Index: gcc/cse.c
>> ===
>> --- gcc/cse.c2019-09-17 15:27:11.338066929 +0100
>> +++ gcc/cse.c2019-09-25 16:55:31.202641509 +0100
>> @@ -42,6 +42,8 @@ Software Foundation; either version 3, o
>>  #include "tree-pass.h"
>>  #include "dbgcnt.h"
>>  #include "rtl-iter.h"
>> +#include "regs.h"
>> +#include "function-abi.h"
>>  
>>  /* The basic idea of common subexpression elimination is to go
>> through the code, keeping a record of expressions that would
>> @@ -566,7 +568,6 @@ static void remove_invalid_subreg_refs (
>>  machine_mode);
>>  static void rehash_using_reg (rtx);
>>  static void invalidate_memory (void);
>> -static void invalidate_for_call (void);
>>  static rtx use_related_value (rtx, struct table_elt *);
>>  
>>  static inline unsigned canon_hash (rtx, machine_mode);
>> @@ -2091,23 +2092,29 @@ rehash_using_reg (rtx x)
>>  }
>>  
>>  /* Remove from the hash table any expression that is a call-clobbered
>> -   register.  Also update their TICK values.  */
>> +   register in INSN.  Also update their TICK values.  */
>>  
>>  static void
>> -invalidate_for_call (void)
>> +invalidate_for_call (rtx_insn *insn)
>>  {
>> -  unsigned int regno, endregno;
>> -  unsigned int i;
>> +  unsigned int regno;
>>unsigned hash;
>>struct table_elt *p, *next;
>>int in_table = 0;
>>hard_reg_set_iterator hrsi;
>>  
>> -  /* Go through all the hard registers.  For each that is clobbered in
>> - a CALL_INSN, remove the register from quantity chains and update
>> +  /* Go through all the hard registers.  For each that might be clobbered
>> + in call insn INSN, remove the register from quantity chains and update
>>   reg_tick if defined.  Also see if any of these registers is currently
>> - in the table.  */
>> -  EXECUTE_IF_SET_IN_HARD_REG_SET (regs_invalidated_by_call, 0, regno, hrsi)
>> + in the table.
>> +
>> + ??? We could be more precise for partially-clobbered registers,
>> + and only invalidate values that actually occupy the clobbered part
>> + of the registers.  It doesn't seem worth the effort though, since
>> + we shouldn't see this situation much before RA.  */
>> +  HARD_REG_SET callee_clobbers
>> += insn_callee_abi (insn).full_and_partial_reg_clobbers ();
>> +  EXECUTE_IF_SET_IN_HARD_REG_SET (callee_clobbers, 0, regno, hrsi)
>>  {
>>delete_reg_equiv (regno);
>>if (REG_TICK (regno) >= 0)
>> @@ -2132,15 +2139,11 @@ invalidate_for_call (void)
>>|| REGNO (p->exp) >= FIRST_PSEUDO_REGISTER)
>>  continue;
>>  
>> -  regno = REGNO (p->exp);
>> -  endregno = END_REGNO (p->exp);
>> -
>> -  for (i = regno; i < endregno; i++)
>> -if (TEST_HARD_REG_BIT (regs_invalidated_by_call, i))
>> -  {
>> -remove_from_table (p, hash);
>> -break;
>> -  }
>> +  /* This must use the same test as above rather than the
>> + more accurate clobbers_reg_p.  */
>> +  if (overlaps_hard_reg_set_p (callee_clobbers, GET_MODE (p->exp),
>> +   REGNO (p->exp)))
> Is it worth putting a forward comment to the earlier test to the later
> one to help ensure they're kept in sync?
>
> OK with or without that comment update.

Yeah, agree that'd be safer.  Here's what I installed.

Thanks,
Richard


2019-09-30  Richard Sandiford  

gcc/
* cse.c: Include regs.h and function-abi.h.
(invalidate_for_call): Take the call insn as an argument.
Use insn_callee_abi to get the ABI of the call and invalidate
partially clobbered registers as well as fully clobbered ones.
(cse_insn): Update call accordingly.

Index: gcc/cse.c
===
--- gcc/cse.c   2019-09-30 17:03:36.0 +0100
+++ gcc/cse.c   2019-09-30 17:03:36.726099589 +0100
@@ -42,6 +42,8 @@ Software Foundation; either version 3, o
 #include "tree-pass.h"
 #include 

Re: [03/32] Add a function for getting the ABI of a call insn target

[Jeff Law]

On 9/25/19 9:38 AM, Richard Sandiford wrote:
> Richard Sandiford  writes:
>> This patch replaces get_call_reg_set_usage with call_insn_abi,
>> which returns the ABI of the target of a call insn.  The ABI's
>> full_reg_clobbers corresponds to regs_invalidated_by_call,
>> whereas many callers instead passed call_used_or_fixed_regs, i.e.:
>>
>>   (regs_invalidated_by_call | fixed_reg_set)
>>
>> The patch slavishly preserves the "| fixed_reg_set" for these callers;
>> later patches will clean this up.
> 
> On reflection, I think insn_callee_abi would be a better name for the
> function than call_insn_abi, since it should make it clearer that the
> function returns the ABI of the target function.  In future we could
> have expr_callee_abi for CALL_EXPRs.
> 
> Also, after Segher's comments for 10/32, I've used "callee_abi" as
> the name of temporary variables, instead of just "abi".
> 
> I've made the same change for later patches (except where I've posted
> new versions instead), but it didn't seem worth spamming the lists
> with that.
> 
> Tested as before.
> 
> Richard
> 
> PS. Ping for the series :-)
> 
> 
> 2019-09-25  Richard Sandiford  
> 
> gcc/
>   * target.def (insn_callee_abi): New hook.
>   (remove_extra_call_preserved_regs): Delete.
>   * doc/tm.texi.in (TARGET_INSN_CALLEE_ABI): New macro.
>   (TARGET_REMOVE_EXTRA_CALL_PRESERVED_REGS): Delete.
>   * doc/tm.texi: Regenerate.
>   * targhooks.h (default_remove_extra_call_preserved_regs): Delete.
>   * targhooks.c (default_remove_extra_call_preserved_regs): Delete.
>   * config/aarch64/aarch64.c (aarch64_simd_call_p): Constify the
>   insn argument.
>   (aarch64_remove_extra_call_preserved_regs): Delete.
>   (aarch64_insn_callee_abi): New function.
>   (TARGET_REMOVE_EXTRA_CALL_PRESERVED_REGS): Delete.
>   (TARGET_INSN_CALLEE_ABI): New macro.
>   * rtl.h (get_call_fndecl): Declare.
>   (cgraph_rtl_info): Fix formatting.  Tweak comment for
>   function_used_regs.  Remove function_used_regs_valid.
>   * rtlanal.c (get_call_fndecl): Moved from final.c
>   * function-abi.h (insn_callee_abi): Declare.
>   (target_function_abi_info): Mention insn_callee_abi.
>   * function-abi.cc (fndecl_abi): Handle flag_ipa_ra in a similar
>   way to get_call_reg_set_usage did.
>   (insn_callee_abi): New function.
>   * regs.h (get_call_reg_set_usage): Delete.
>   * final.c: Include function-abi.h.
>   (collect_fn_hard_reg_usage): Add fixed and stack registers to
>   function_used_regs before the main loop rather than afterwards.
>   Use insn_callee_abi instead of get_call_reg_set_usage.  Exit early
>   if function_used_regs ends up not being useful.
>   (get_call_fndecl): Move to rtlanal.c
>   (get_call_cgraph_rtl_info, get_call_reg_set_usage): Delete.
>   * caller-save.c: Include function-abi.h.
>   (setup_save_areas, save_call_clobbered_regs): Use insn_callee_abi
>   instead of get_call_reg_set_usage.
>   * cfgcleanup.c: Include function-abi.h.
>   (old_insns_match_p): Use insn_callee_abi instead of
>   get_call_reg_set_usage.
>   * cgraph.h (cgraph_node::rtl_info): Take a const_tree instead of
>   a tree.
>   * cgraph.c (cgraph_node::rtl_info): Likewise.  Initialize
>   function_used_regs.
>   * df-scan.c: Include function-abi.h.
>   (df_get_call_refs): Use insn_callee_abi instead of
>   get_call_reg_set_usage.
>   * ira-lives.c: Include function-abi.h.
>   (process_bb_node_lives): Use insn_callee_abi instead of
>   get_call_reg_set_usage.
>   * lra-lives.c: Include function-abi.h.
>   (process_bb_lives): Use insn_callee_abi instead of
>   get_call_reg_set_usage.
>   * postreload.c: Include function-abi.h.
>   (reload_combine): Use insn_callee_abi instead of
>   get_call_reg_set_usage.
>   * regcprop.c: Include function-abi.h.
>   (copyprop_hardreg_forward_1): Use insn_callee_abi instead of
>   get_call_reg_set_usage.
>   * resource.c: Include function-abi.h.
>   (mark_set_resources, mark_target_live_regs): Use insn_callee_abi
>   instead of get_call_reg_set_usage.
>   * var-tracking.c: Include function-abi.h.
>   (dataflow_set_clear_at_call): Use insn_callee_abi instead of
>   get_call_reg_set_usage.
> 
OK.  I think that's the whole set, right?

There were a ton of guality issues over the last couple weeks in the
tester, but those were somewhat expected due to a prior patchkit.  The
affected targets have new baseline results in my tester, so if anything
goes nuts as a result of your patches it should be fairly obvious.

Jeff

Re: [ping][PR target/85401] initialize the move cost table before using it

[Vladimir Makarov]

On 2019-09-28 5:52 a.m., co...@sdf.org wrote:

re-posting, now CC'ing vmakarov who might be the right person to ping
about issues in this file.  apologies for the noise if I'm wrong.

--
This seems to be the way the rest of ira-color.c does it.
I hope it's OK. It does fix the segfault.
Yes, the patch is mostly ok.  You can commit it into the trunk after 
applying changes mentioned below. If you do not have a write access, let 
me know I'll commit the patch by myself.

2019-09-10  Maya Rashish 

PR target/85401
* ira-color.c: (allocno_copy_cost_saving) Call
ira_init_register_move_cost_if_necessary


Wrong format for ChangeLog.  Wrong place for the colon and missed 
period.  It should be


PR target/85401
* ira-color.c (allocno_copy_cost_saving): Call
ira_init_register_move_cost_if_necessary.



diff --git a/gcc/ira-color.c b/gcc/ira-color.c
index 99236994d64..5d721102e19 100644
--- a/gcc/ira-color.c
+++ b/gcc/ira-color.c
@@ -2828,6 +2828,7 @@ allocno_copy_cost_saving (ira_allocno_t allocno, int 
hard_regno)
}
else
gcc_unreachable ();
+  ira_init_register_move_cost_if_necessary(allocno_mode);

Blank should be before the open parenthesis.

cost += cp->freq * ira_register_move_cost[allocno_mode][rclass][rclass];
  }
return cost;


It would be nice to add a small test too.  But it is not obligatory for 
this case as the patch is obvious and it might be hard to create a small 
test to reproduce the bug.

Re: [WIP PATCH] add object access attributes (PR 83859)

[Martin Sebor]

On 9/30/19 1:37 AM, Richard Biener wrote:

On Sun, Sep 29, 2019 at 9:52 PM Martin Sebor  wrote:


-Wstringop-overflow detects a subset of past-the-end read and write
accesses by built-in functions such as memcpy and strcpy.  It relies
on the functions' effects the knowledge of which is hardwired into
GCC.  Although it's possible for users to create wrappers for their
own functions to detect similar problems, it's quite cumbersome and
so only lightly used outside system libraries like Glibc.  Even Glibc
only checks for buffer overflow and not for reading past the end.

PR 83859 asks to expose the same checking that GCC does natively for
built-in calls via a function attribute that associates a pointer
argument with the size argument, such as:

__attribute__((buffer_size (1, 2))) void
f (char* dst, size_t dstsize);

The attached patch is my initial stab at providing this feature by
introducing three new attributes:

* read_only (ptr-argno, size-argno)
* read_only (ptr-argno, size-argno)
* read_write (ptr-argno, size-argno)

As requested, the attributes associate a pointer parameter to
a function with a size parameter.  In addition, they also specify
how the function accesses the object the pointer points to: either
it only reads from it, or it only writes to it, or it does both.

Besides enabling the same buffer overflow detection as for built-in
string functions they also let GCC issue -Wuninitialized warnings
for uninitialized objects passed to read-only functions by reference,
and -Wunused-but-set warnings for objects passed to write-only
functions that are otherwise unused (PR 80806).  The -Wununitialized
part is done. The -Wunused-but-set detection is implemented only in
the C FE and not yet in C++.

Besides the diagnostic improvements above the attributes also open
up optimization opportunities such as DCE.  I'm still working on this
and so it's not yet part of the initial patch.


There's the "fn spec" attribute which you can use for the optimization
part.  Note "fn spec" also likes to know whether the address of the
argument escapes and whether the argument is only dereferenced
directly or also indirectly (when passing a pointer to a struct is
transitively reachable memory through the pointer accessed or not?).


Thanks, I'll look into those.



So you should at least make sure to document the full
semantics of your proposed read_only/write_only/read_write atributes.

I guess that "read_only" means that direct accesses do not write
but the attribute does not constrain indirect accesses?


Correct.  Some other annotation is necessary to constrain those.
A read-only restrict-qualified pointer would do that.  Because
the read-only attribute only applies to const pointers the only
purpose it serves in that combination is the association with
the size parameter.  As in:

  __attribute__ ((read_only (2, 3))) void*
  memcpy (void* restrict, const void *restrict, size_t);

without the pointer-size association the above reduces to this:

  __attribute__ ((read_only)) void*
  memcpy (void* restrict, const void *restrict, size_t);

which should ultimately have the same effect as the plain

  void*
  memcpy (void* restrict, const void *restrict, size_t);

As a future extension (GCC 11 or beyond) I'd like to look into
allowing the read/write attributes on object and mainly subobject
declarations.  There too, read-only should be paired with restrict
to express the same constraint.


Note "fn spec" doesn't offer read/write constraints when not at the
same time constraining escaping since when a pointer escapes
through a call we cannot make any optimization for a positional
read/write constraint since a function can access global memory
(and all escaped pointed-to data ends up as "global memory").
There's no way to tell (via "fn spec") that the function only accesses
memory reachable via function arguments.


In my WIP patch I have a no_side_effect attribute that further
constrains what a function can do.  It's just like pure except
that it lets the function access objects passed to it by reference.



So I'm not sure there's a 1:1 mapping for your desired semantics
to "fn spec" plus your desired semantics may offer no
opportunity for optimization.  Useful would be if read_only
would map to "R" and read_write and write_only would map to "W".


I didn't know about the R and W fnspecs.  Let me look into them
for GCC 11 to see if I can make use of them for the optimization.

Martin



Richard.


I plan to finish the patch for GCC 10 but I don't expect to have
the time to start taking advantage of the attributes for optimization
until GCC 11.

Besides regression testing on x86_64-linux, I also tested the patch
by compiling Binutils/GDB, Glibc, and the Linux kernel with it.  It
found no new problems but caused a handful of -Wunused-but-set-variable
false positives due to an outstanding bug in the C front-end introduced
by the patch that I still need to fix.

Martin

Re: [21/32] Remove global call sets: LRA

[Jeff Law]

On 9/11/19 1:14 PM, Richard Sandiford wrote:
> lra_reg has an actual_call_used_reg_set field that is only used during
> inheritance.  This in turn required a special lra_create_live_ranges
> pass for flag_ipa_ra to set up this field.  This patch instead makes
> the inheritance code do its own live register tracking, using the
> same ABI-mask-and-clobber-set pair as for IRA.
> 
> Tracking ABIs simplifies (and cheapens) the logic in lra-lives.c and
> means we no longer need a separate path for -fipa-ra.  It also means
> we can remove TARGET_RETURN_CALL_WITH_MAX_CLOBBERS.
> 
> The patch also strengthens the sanity check in lra_assigns so that
> we check that reg_renumber is consistent with the whole conflict set,
> not just the call-clobbered registers.
> 
> 
> 2019-09-11  Richard Sandiford  
> 
> gcc/
>   * target.def (return_call_with_max_clobbers): Delete.
>   * doc/tm.texi.in (TARGET_RETURN_CALL_WITH_MAX_CLOBBERS): Delete.
>   * doc/tm.texi: Regenerate.
>   * config/aarch64/aarch64.c (aarch64_return_call_with_max_clobbers)
>   (TARGET_RETURN_CALL_WITH_MAX_CLOBBERS): Delete.
>   * lra-int.h (lra_reg::actual_call_used_reg_set): Delete.
>   (lra_reg::call_insn): Delete.
>   * lra.c: Include function-abi.h.
>   (initialize_lra_reg_info_element): Don't initialize the fields above.
>   (lra): Use crtl->abi to test whether the current function needs to
>   save a register in the prologue.  Remove special pre-inheritance
>   lra_create_live_ranges pass for flag_ipa_ra.
>   * lra-assigns.c: Include function-abi.h
>   (find_hard_regno_for_1): Use crtl->abi to test whether the current
>   function needs to save a register in the prologue.
>   (lra_assign): Assert that registers aren't allocated to a
>   conflicting register, rather than checking only for overlaps
>   with call_used_or_fixed_regs.  Do this even for flag_ipa_ra,
>   and for registers that are not live across a call.
>   * lra-constraints.c (last_call_for_abi): New variable.
>   (full_and_partial_call_clobbers): Likewise.
>   (setup_next_usage_insn): Remove the register from
>   full_and_partial_call_clobbers.
>   (need_for_call_save_p): Use call_clobbered_in_region_p to test
>   whether the register needs a caller save.
>   (need_for_split_p): Use full_and_partial_reg_clobbers instead
>   of call_used_or_fixed_regs.
>   (inherit_in_ebb): Initialize and maintain last_call_for_abi and
>   full_and_partial_call_clobbers.
>   * lra-lives.c (check_pseudos_live_through_calls): Replace
>   last_call_used_reg_set and call_insn arguments with an abi argument.
>   Remove handling of lra_reg::call_insn.  Use function_abi::mode_clobbers
>   as the set of conflicting registers.
>   (calls_have_same_clobbers_p): Delete.
>   (process_bb_lives): Track the ABI of the last call instead of an
>   insn/HARD_REG_SET pair.  Update calls to
>   check_pseudos_live_through_calls.  Use eh_edge_abi to calculate
>   the set of registers that could be clobbered by an EH edge.
>   Include partially-clobbered as well as fully-clobbered registers.
>   (lra_create_live_ranges_1): Don't initialize lra_reg::call_insn.
>   * lra-remat.c: Include function-abi.h.
>   (call_used_regs_arr_len, call_used_regs_arr): Delete.
>   (set_bb_regs): Use call_insn_abi to get the set of call-clobbered
>   registers and bitmap_view to combine them into dead_regs.
>   (call_used_input_regno_present_p): Take a function_abi argument
>   and use it to test whether a register is call-clobbered.
>   (calculate_gen_cands): Use call_insn_abi to get the ABI of the
>   call insn target.  Update tje call to call_used_input_regno_present_p.
>   (do_remat): Likewise.
>   (lra_remat): Remove the initialization of call_used_regs_arr_len
>   and call_used_regs_arr.
OK
jeff

Re: [AArch64] Strengthen aarch64_hard_regno_call_part_clobbered

[Richard Sandiford]

Richard Sandiford  writes:
> The aarch64_vector_pcs handling in aarch64_hard_regno_call_part_clobbered
> checks whether the mode might be bigger than 16 bytes, since on SVE
> targets the (non-SVE) vector PCS only guarantees that the low 16 bytes
> are preserved.  But for multi-register modes, we should instead test
> whether each single-register part might be bigger than 16 bytes.
> (The size is always divided evenly between registers.)
>
> The testcase uses XImode as an example where this helps.
>
> Tested on aarch64-linux-gnu (with and without SVE).  OK to install?
>
> Richard
>
>
> 2019-09-10  Richard Sandiford  
>
> gcc/
>   * config/aarch64/aarch64.c (aarch64_hard_regno_call_part_clobbered):
>   For multi-registers modes, test how big each register part is.
>
> gcc/testsuite/
>   * gcc.target/aarch64/torture/simd-abi-8.c: New test.

I've now applied this as r276305.

Thanks,
Richard

New Spanish PO file for 'gcc' (version 9.1.0)

[Translation Project Robot]

Hello, gentle maintainer.

This is a message from the Translation Project robot.

A revised PO file for textual domain 'gcc' has been submitted
by the Spanish team of translators.  The file is available at:

https://translationproject.org/latest/gcc/es.po

(This file, 'gcc-9.1.0.es.po', has just now been sent to you in
a separate email.)

All other PO files for your package are available in:

https://translationproject.org/latest/gcc/

Please consider including all of these in your next release, whether
official or a pretest.

Whenever you have a new distribution with a new version number ready,
containing a newer POT file, please send the URL of that distribution
tarball to the address below.  The tarball may be just a pretest or a
snapshot, it does not even have to compile.  It is just used by the
translators when they need some extra translation context.

The following HTML page has been updated:

https://translationproject.org/domain/gcc.html

If any question arises, please contact the translation coordinator.

Thank you for all your work,

The Translation Project robot, in the
name of your translation coordinator.

Re: [19/32] Remove global call sets: IRA

[Jeff Law]

On 9/11/19 1:12 PM, Richard Sandiford wrote:
> For -fipa-ra, IRA already keeps track of which specific registers
> are call-clobbered in a region, rather than using global information.
> The patch generalises this so that it tracks which ABIs are used
> by calls in the region.
> 
> We can then use the new ABI descriptors to handle partially-clobbered
> registers in the same way as fully-clobbered registers, without having
> special code for targetm.hard_regno_call_part_clobbered.  This in turn
> makes -fipa-ra work for partially-clobbered registers too.
> 
> A side-effect of allowing multiple ABIs is that we no longer have
> an obvious set of conflicting registers for the self-described
> "fragile hack" in ira-constraints.c.  This code kicks in for
> user-defined registers that aren't live across a call at -O0,
> and it tries to avoid allocating a call-clobbered register to them.
> Here I've used the set of call-clobbered registers in the current
> function's ABI, applying on top of any registers that are clobbered by
> called functions.  This is enough to keep gcc.dg/debug/dwarf2/pr5948.c
> happy.
> 
> The handling of GENERIC_STACK_CHECK in do_reload seemed to have
> a reversed condition:
> 
>   for (int i = 0; i < FIRST_PSEUDO_REGISTER; i++)
>   if (df_regs_ever_live_p (i)
>   && !fixed_regs[i]
>   && call_used_or_fixed_reg_p (i))
> size += UNITS_PER_WORD;
> 
> The final part of the condition counts registers that don't need to be
> saved in the prologue, but I think the opposite was intended.
Agreed.  Given it's just used to emit a diagnostic and that in reality
it's only used for Ada, I'm confident this code isn't getting exercised
in any significant way right now.

> 
> 
> 2019-09-11  Richard Sandiford  
> 
> gcc/
>   * function-abi.h (call_clobbers_in_region): Declare.
>   (call_clobbered_in_region_p): New function.
>   * function-abi.cc (call_clobbers_in_region): Likewise.
>   * ira-int.h: Include function-abi.h.
>   (ira_allocno::crossed_calls_abis): New field.
>   (ALLOCNO_CROSSED_CALLS_ABIS): New macro.
>   (ira_need_caller_save_regs): New function.
>   (ira_need_caller_save_p): Likewise.
>   * ira.c (setup_reg_renumber): Use ira_need_caller_save_p instead
>   of call_used_or_fixed_regs.
>   (do_reload): Use crtl->abi to test whether the current function
>   needs to save a register in the prologue.  Count registers that
>   need to be saved rather than registers that don't.
>   * ira-build.c (create_cap_allocno): Copy ALLOCNO_CROSSED_CALLS_ABIS.
>   Remove unnecessary | from ALLOCNO_CROSSED_CALLS_CLOBBERED_REGS.
>   (propagate_allocno_info): Merge ALLOCNO_CROSSED_CALLS_ABIS too.
>   (propagate_some_info_from_allocno): Likewise.
>   (copy_info_to_removed_store_destinations): Likewise.
>   (ira_flattening): Say that ALLOCNO_CROSSED_CALLS_ABIS and
>   ALLOCNO_CROSSED_CALLS_CLOBBERED_REGS are handled conservatively.
>   (ira_build): Use ira_need_caller_save_regs instead of
>   call_used_or_fixed_regs.
>   * ira-color.c (calculate_saved_nregs): Use crtl->abi to test
>   whether the current function would need to save a register
>   before using it.
>   (calculate_spill_cost): Likewise.
>   (allocno_reload_assign): Use ira_need_caller_save_regs and
>   ira_need_caller_save_p instead of call_used_or_fixed_regs.
>   * ira-conflicts.c (ira_build_conflicts): Use
>   ira_need_caller_save_regs rather than call_used_or_fixed_regs
>   as the set of call-clobbered registers.  Remove the
>   call_used_or_fixed_regs mask from the calculation of
>   temp_hard_reg_set and mask its use instead.  Remove special
>   handling of partially-clobbered registers.
>   * ira-costs.c (ira_tune_allocno_costs): Use ira_need_caller_save_p.
>   * ira-lives.c (process_bb_node_lives): Use mode_clobbers to
>   calculate the set of conflicting registers for calls that
>   can throw.  Record the ABIs of calls in ALLOCNO_CROSSED_CALLS_ABIS.
>   Use full_and_partial_reg_clobbers rather than full_reg_clobbers
>   for the calculation of ALLOCNO_CROSSED_CALLS_CLOBBERED_REGS.
>   Use eh_edge_abi to calculate the set of registers that could
>   be clobbered by an EH edge.  Include partially-clobbered as
>   well as fully-clobbered registers.
OK
jeff

[WIP PATCH] PR71102: Make #pragma GCC error/warning concat strings

[Stephan Bergmann]

(I think this is my first patch submission here, so please bear with me.)

As discussed at  
"_Pragma("GCC warning ...") should concatenate string literals", it 
would be useful if #pragma GCC error/warning behaved like #pragma 
message and consumed multiple (macro-expanded) string literal tokens.


The below patch gets close, but:

* I don't know how to get a location_t that spans all the tokens, see 
the TODO in the patch.  (If that's hard to get right, an alternative 
might be to instead highlight the "error" resp. "warning" token, in the 
same way as #pragma message highlights the "message" token.)


* Additional tokens that are not (ordinary) string literals keep being 
ignored (to be exact, the first such non--string-literal and all 
remaining tokens are ignored).  That's in line with existing behavior 
for those pragmas, where they ignored everything following the first 
(necessarily string-literal) token (but unlike #pragma message, which 
emits "warning: junk at end of '#pragma message' [-Wpragmas]"). 
Something of a corner case happens when such a non--string-literal token 
is the result of macro expansion as in


  #define FOO "2" 3
  #pragma GCC error "1" FOO

emitting a message of "12".  (My naive understanding of how 
cpp_get_token works in do_pragma_warnign_or_error is that the end of the 
#pragma line is represented by a CPP_EOF token.)


* I have left the documentation in gcc/doc/extend.texi alone, as it 
didn't specify any details of how the #pragma message string is parsed, 
either.



diff --git a/gcc/testsuite/c-c++-common/cpp/diagnostic-pragma-1.c 
b/gcc/testsuite/c-c++-common/cpp/diagnostic-pragma-1.c

index 9867c94a8dd..53bae44e210 100644
--- a/gcc/testsuite/c-c++-common/cpp/diagnostic-pragma-1.c
+++ b/gcc/testsuite/c-c++-common/cpp/diagnostic-pragma-1.c
@@ -9,3 +9,6 @@
 char a[CONST1]; // { dg-warning warn-c }
 char b[CONST2]; // { dg-error err-d }

+#define EXPAND "exp-"
+#pragma GCC warning "warn-" EXPAND "e" // { dg-warning warn-exp-e }
+#pragma GCC error "err-" EXPAND "f" // { dg-error err-exp-f }
diff --git a/libcpp/directives.c b/libcpp/directives.c
index 61f1fef9489..4d06a2d2934 100644
--- a/libcpp/directives.c
+++ b/libcpp/directives.c
@@ -1740,19 +1740,49 @@ do_pragma_dependency (cpp_reader *pfile)
 static void
 do_pragma_warning_or_error (cpp_reader *pfile, bool error)
 {
-  const cpp_token *tok = _cpp_lex_token (pfile);
+  size_t count = 0;
+  obstack str_ob;
+  obstack_specify_allocation (_ob, 0, 0, xmalloc, free);
+  location_t loc;
+  location_t loc_end;
+  gcc_assert (pfile->state.prevent_expansion > 0);
+  pfile->state.prevent_expansion--;
+  for (;; ++count)
+{
+  const cpp_token *tok = cpp_get_token (pfile);
+  if (tok->type != CPP_STRING)
+   break;
+  if (count == 0)
+   loc = tok->src_loc;
+  obstack_grow (_ob, >val.str, sizeof (cpp_string));
+  loc_end = tok->src_loc;
+}
+  pfile->state.prevent_expansion++;
+  if (count == 0)
+{
+  cpp_error (pfile, CPP_DL_ERROR, "invalid \"#pragma GCC %s\" 
directive",

+error ? "error" : "warning");
+  return;
+}
+  cpp_string * strs = (cpp_string *) obstack_finish (_ob);
   cpp_string str;
-  if (tok->type != CPP_STRING
-  || !cpp_interpret_string_notranslate (pfile, >val.str, 1, ,
-   CPP_STRING)
-  || str.len == 0)
+  bool combine = cpp_interpret_string_notranslate (pfile, strs, count,
+  , CPP_STRING);
+  obstack_free (_ob, 0);
+  if (!combine || str.len == 0)
 {
   cpp_error (pfile, CPP_DL_ERROR, "invalid \"#pragma GCC %s\" 
directive",

 error ? "error" : "warning");
   return;
 }
-  cpp_error (pfile, error ? CPP_DL_ERROR : CPP_DL_WARNING,
-"%s", str.text);
+  if (count != 1)
+{
+  //TODO: combine loc, loc_end into
+  // loc = COMBINE_LOCATION_DATA (pfile->line_table, ..., ..., NULL);
+  (void)loc_end;
+}
+  cpp_error_at (pfile, error ? CPP_DL_ERROR : CPP_DL_WARNING, loc,
+   "%s", str.text);
   free ((void *)str.text);
 }

Re: [30/32] Remove global call sets: sel-sched.c

[Jeff Law]

On 9/11/19 1:18 PM, Richard Sandiford wrote:
> The main change here is to replace a crosses_call boolean with
> a bitmask of the ABIs used by the crossed calls.  For space reasons,
> I didn't also add a HARD_REG_SET that tracks the set of registers
> that are actually clobbered, which means that this is the one part
> of the series that doesn't benefit from -fipa-ra.  The existing
> FIXME suggests that the current structures aren't the preferred
> way of representing this anyhow, and the pass already makes
> conservative assumptions about call-crossing registers.
> 
> 
> 2019-09-11  Richard Sandiford  
> 
> gcc/
>   * sel-sched-ir.h (_def::crosses_call): Replace with...
>   (_def::crossed_call_abis): ..this new field.
>   (def_list_add): Take a mask of ABIs instead of a crosses_call
>   boolean.
>   * sel-sched-ir.c (def_list_add): Likewise.  Update initialization
>   of _def accordingly.
>   * sel-sched.c: Include function-abi.h.
>   (hard_regs_data::regs_for_call_clobbered): Delete.
>   (reg_rename::crosses_call): Replace with...
>   (reg_rename::crossed_call_abis): ...this new field.
>   (fur_static_params::crosses_call): Replace with...
>   (fur_static_params::crossed_call_abis): ...this new field.
>   (init_regs_for_mode): Don't initialize sel_hrd.regs_for_call_clobbered.
>   (init_hard_regs_data): Use crtl->abi to test which registers the
>   current function would need to save before it uses them.
>   (mark_unavailable_hard_regs): Update handling of call-clobbered
>   registers, using call_clobbers_in_region to find out which registers
>   might be call-clobbered (but without taking -fipa-ra into account
>   for now).  Remove separate handling of partially call-clobbered
>   registers.
>   (verify_target_availability): Use crossed_call_abis instead of
>   crosses_call.
>   (get_spec_check_type_for_insn, find_used_regs): Likewise.
>   (fur_orig_expr_found, fur_on_enter, fur_orig_expr_not_found): Likewise.
> 
OK
jeff

Re: [PATCH v3 2/9] Introduce rtx_alloca, alloca_raw_REG and alloca_rtx_fmt_*

[Ilya Leoshkevich]

> Am 06.09.2019 um 14:40 schrieb Richard Sandiford :
> 
> Ilya Leoshkevich  writes:
>> One of the next patches in series needs to frequently pass short-lived
>> fake rtxes to the back-end in order to test its capabilities.  In order
>> to reduce the load on GC, it is beneficial to allocate these rtxes on
>> stack.
>> 
>> Provide the macro counterparts of gen_* functions required by the next
>> patch in series.
>> 
>> gcc/ChangeLog:
>> 
>> 2019-08-27  Ilya Leoshkevich  
>> 
>>  PR target/77918
>>  * emit-rtl.c (gen_raw_REG): Use init_raw_REG.
>>  (init_raw_REG): New function.
>>  * gengenrtl.c (gendef): Emit init_* functions and alloca_*
>>  macros.
>>  * rtl.c (rtx_alloc_stat_v): Use rtx_init.
>>  * rtl.h (rtx_init): New function.
>>  (rtx_alloca): New function.
>>  (init_raw_REG): New function.
>>  (alloca_raw_REG): New macro.
>> ---
>> gcc/emit-rtl.c  | 15 +---
>> gcc/gengenrtl.c | 64 +
>> gcc/rtl.c   |  7 +-
>> gcc/rtl.h   | 12 ++
>> 4 files changed, 74 insertions(+), 24 deletions(-)
>> 
>> diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
>> index a667cdab94e..ecfa3735bba 100644
>> --- a/gcc/emit-rtl.c
>> +++ b/gcc/emit-rtl.c
>> @@ -466,20 +466,29 @@ set_mode_and_regno (rtx x, machine_mode mode, unsigned 
>> int regno)
>>   set_regno_raw (x, regno, nregs);
>> }
>> 
>> -/* Generate a new REG rtx.  Make sure ORIGINAL_REGNO is set properly, and
>> +/* Initialize a REG rtx.  Make sure ORIGINAL_REGNO is set properly, and
>>don't attempt to share with the various global pieces of rtl (such as
>>frame_pointer_rtx).  */
>> 
>> rtx
>> -gen_raw_REG (machine_mode mode, unsigned int regno)
>> +init_raw_REG (rtx x, machine_mode mode, unsigned int regno)
>> {
>> -  rtx x = rtx_alloc (REG MEM_STAT_INFO);
>>   set_mode_and_regno (x, mode, regno);
>>   REG_ATTRS (x) = NULL;
>>   ORIGINAL_REGNO (x) = regno;
>>   return x;
>> }
>> 
>> +/* Generate a new REG rtx.  */
>> +
>> +rtx
>> +gen_raw_REG (machine_mode mode, unsigned int regno)
>> +{
>> +  rtx x = rtx_alloc (REG MEM_STAT_INFO);
>> +  init_raw_REG (x, mode, regno);
>> +  return x;
>> +}
>> +
> 
> I think we should keep the gen_raw_REG comment the same (including
> the "Make sure..." bit).  The point is to contrast gen_raw_REG with
> gen_rtx_REG, which does various bits of sharing.
> 
> Maybe the comment above init_raw_REG can instead be just:
> 
> /* Initialize a fresh REG rtx with mode MODE and register REGNO.  */
> 
>> [...]
>> @@ -266,16 +261,55 @@ gendef (const char *format)
>> else
>>   printf ("  %s (rt, %d) = arg%d;\n", accessor_from_format (*p), i, j++);
>> 
>> -  puts ("\n  return rt;\n}\n");
>> +  puts ("  return rt;\n}\n");
>> +
>> +  /* Write the definition of the gen function name and the types
>> + of the arguments.  */
>> +
>> +  puts ("static inline rtx");
>> +  printf ("gen_rtx_fmt_%s_stat (RTX_CODE code, machine_mode mode", format);
>> +  for (p = format, i = 0; *p != 0; p++)
>> +if (*p != '0')
>> +  printf (",\n\t%sarg%d", type_from_format (*p), i++);
>> +  puts (" MEM_STAT_DECL)");
>> +
>> +  /* Now write out the body of the function itself, which allocates
>> + the memory and initializes it.  */
>> +  puts ("{");
>> +  puts ("  rtx rt;\n");
>> +
>> +  puts ("  rt = rtx_alloc (code PASS_MEM_STAT);");
>> +  printf ("  init_rtx_fmt_%s (rt, mode", format);
>> +  for (p = format, i = 0; *p != 0; p++)
>> +if (*p != '0')
>> +  printf (", arg%d", i++);
>> +  puts (");\n");
>> +
>> +  puts ("  return rt;\n}\n");
> 
> Might as well make it:
> 
>  printf ("  return init_rtx_fmt_%s (rt, mode", format);
> 
> and drop the separate return, for consistency with the new
> alloca code.
> 
> OK with those changes, thanks.

I've made the changes and committed the patch:

https://gcc.gnu.org/viewcvs/gcc?view=revision=276303

Thanks for the review!

Re: [PATCH v3 5/9] S/390: Implement vcond expander for V1TI,V1TF

[Andreas Krebbel]

On 05.09.19 13:10, Ilya Leoshkevich wrote:
> Currently gcc does not emit wf{c,k}* instructions when comparing long
> double values.  Middle-end actually adds them in the first place, but
> then veclower pass replaces them with floating point register pair
> operations, because the corresponding expander is missing.
> 
> gcc/ChangeLog:
> 
> 2019-08-09  Ilya Leoshkevich  
> 
>   PR target/77918
>   * config/s390/vector.md (vcondv1tiv1tf): New variant of
>   vcond$a$b expander.

Couldn't you just add V1TI to V_HW and V_HW2 instead?

Andreas

> ---
>  gcc/config/s390/vector.md | 15 +++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/gcc/config/s390/vector.md b/gcc/config/s390/vector.md
> index d7a266c5605..ca5ec0dd3b0 100644
> --- a/gcc/config/s390/vector.md
> +++ b/gcc/config/s390/vector.md
> @@ -649,6 +649,21 @@
>DONE;
>  })
>  
> +(define_expand "vcondv1tiv1tf"
> +  [(set (match_operand:V1TI 0 "register_operand" "")
> + (if_then_else:V1TI
> +  (match_operator 3 "vcond_comparison_operator"
> +  [(match_operand:V1TF 4 "register_operand" "")
> +   (match_operand:V1TF 5 "nonmemory_operand" "")])
> +  (match_operand:V1TI 1 "nonmemory_operand" "")
> +  (match_operand:V1TI 2 "nonmemory_operand" "")))]
> +  "TARGET_VXE"
> +{
> +  s390_expand_vcond (operands[0], operands[1], operands[2],
> +  GET_CODE (operands[3]), operands[4], operands[5]);
> +  DONE;
> +})
> +
>  (define_expand "vcondu"
>[(set (match_operand:V_HW 0 "register_operand" "")
>   (if_then_else:V_HW
> 

Re: [PATCH v3 7/9] S/390: Remove code duplication in vec_* comparison expanders

[Andreas Krebbel]

On 05.09.19 13:10, Ilya Leoshkevich wrote:
> s390.md uses a lot of near-identical expanders that perform dispatching
> to other expanders based on operand types. Since the following patch
> would require even more of these, avoid copy-pasting the code by
> generating these expanders using an iterator.
> 
> gcc/ChangeLog:
> 
> 2019-08-09  Ilya Leoshkevich  
> 
>   PR target/77918
>   * config/s390/s390.c (s390_expand_vec_compare): Use
>   gen_vec_cmpordered and gen_vec_cmpunordered.
>   * config/s390/vector.md (vec_cmpuneq, vec_cmpltgt, vec_ordered,
>   vec_unordered): Delete.
>   (vec_ordered): Rename to vec_cmpordered.
>   (vec_unordered): Rename to vec_cmpunordered.
>   (vec_cmp): Generic dispatcher.

VEC_CODE_WITH_COMPLEX_EXPAND is pretty long and doesn't mention that it is 
about compare operators.
Perhaps VEC_CMP_EXPAND or something like this? Btw. could you please mention 
the new iterator in the
changelog.

Ok.

Andreas

> ---
>  gcc/config/s390/s390.c|  4 +--
>  gcc/config/s390/vector.md | 67 +++
>  2 files changed, 13 insertions(+), 58 deletions(-)
> 
> diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
> index 24784266848..f36e12affd2 100644
> --- a/gcc/config/s390/s390.c
> +++ b/gcc/config/s390/s390.c
> @@ -6523,10 +6523,10 @@ s390_expand_vec_compare (rtx target, enum rtx_code 
> cond,
> emit_insn (gen_vec_cmpltgt (target, cmp_op1, cmp_op2));
> return;
>   case ORDERED:
> -   emit_insn (gen_vec_ordered (target, cmp_op1, cmp_op2));
> +   emit_insn (gen_vec_cmpordered (target, cmp_op1, cmp_op2));
> return;
>   case UNORDERED:
> -   emit_insn (gen_vec_unordered (target, cmp_op1, cmp_op2));
> +   emit_insn (gen_vec_cmpunordered (target, cmp_op1, cmp_op2));
> return;
>   default: break;
>   }
> diff --git a/gcc/config/s390/vector.md b/gcc/config/s390/vector.md
> index 1b66b8be61f..a093ae5c565 100644
> --- a/gcc/config/s390/vector.md
> +++ b/gcc/config/s390/vector.md
> @@ -1507,22 +1507,6 @@
>operands[3] = gen_reg_rtx (mode);
>  })
>  
> -(define_expand "vec_cmpuneq"
> -  [(match_operand 0 "register_operand" "")
> -   (match_operand 1 "register_operand" "")
> -   (match_operand 2 "register_operand" "")]
> -  "TARGET_VX"
> -{
> -  if (GET_MODE (operands[1]) == V4SFmode)
> -emit_insn (gen_vec_cmpuneqv4sf (operands[0], operands[1], operands[2]));
> -  else if (GET_MODE (operands[1]) == V2DFmode)
> -emit_insn (gen_vec_cmpuneqv2df (operands[0], operands[1], operands[2]));
> -  else
> -gcc_unreachable ();
> -
> -  DONE;
> -})
> -
>  ; LTGT a <> b -> a > b | b > a
>  (define_expand "vec_cmpltgt"
>[(set (match_operand: 0 "register_operand" "=v")
> @@ -1535,24 +1519,8 @@
>operands[3] = gen_reg_rtx (mode);
>  })
>  
> -(define_expand "vec_cmpltgt"
> -  [(match_operand 0 "register_operand" "")
> -   (match_operand 1 "register_operand" "")
> -   (match_operand 2 "register_operand" "")]
> -  "TARGET_VX"
> -{
> -  if (GET_MODE (operands[1]) == V4SFmode)
> -emit_insn (gen_vec_cmpltgtv4sf (operands[0], operands[1], operands[2]));
> -  else if (GET_MODE (operands[1]) == V2DFmode)
> -emit_insn (gen_vec_cmpltgtv2df (operands[0], operands[1], operands[2]));
> -  else
> -gcc_unreachable ();
> -
> -  DONE;
> -})
> -
>  ; ORDERED (a, b): a >= b | b > a
> -(define_expand "vec_ordered"
> +(define_expand "vec_cmpordered"
>[(set (match_operand:  0 "register_operand" "=v")
>   (ge: (match_operand:VFT 1 "register_operand"  "v")
>(match_operand:VFT 2 "register_operand"  "v")))
> @@ -1563,45 +1531,32 @@
>operands[3] = gen_reg_rtx (mode);
>  })
>  
> -(define_expand "vec_ordered"
> -  [(match_operand 0 "register_operand" "")
> -   (match_operand 1 "register_operand" "")
> -   (match_operand 2 "register_operand" "")]
> -  "TARGET_VX"
> -{
> -  if (GET_MODE (operands[1]) == V4SFmode)
> -emit_insn (gen_vec_orderedv4sf (operands[0], operands[1], operands[2]));
> -  else if (GET_MODE (operands[1]) == V2DFmode)
> -emit_insn (gen_vec_orderedv2df (operands[0], operands[1], operands[2]));
> -  else
> -gcc_unreachable ();
> -
> -  DONE;
> -})
> -
>  ; UNORDERED (a, b): !ORDERED (a, b)
> -(define_expand "vec_unordered"
> +(define_expand "vec_cmpunordered"
>[(match_operand: 0 "register_operand" "=v")
> (match_operand:VFT1 "register_operand" "v")
> (match_operand:VFT2 "register_operand" "v")]
>"TARGET_VX"
>  {
> -  emit_insn (gen_vec_ordered (operands[0], operands[1], operands[2]));
> +  emit_insn (gen_vec_cmpordered (operands[0], operands[1], 
> operands[2]));
>emit_insn (gen_rtx_SET (operands[0],
>gen_rtx_NOT (mode, operands[0])));
>DONE;
>  })
>  
> -(define_expand "vec_unordered"
> +(define_code_iterator VEC_CODE_WITH_COMPLEX_EXPAND
> +  [uneq ltgt ordered unordered])
> +
> +(define_expand "vec_cmp"
>[(match_operand 0 "register_operand" "")
> -   

Re: [PATCH v3 6/9] S/390: Remove code duplication in vec_unordered

[Andreas Krebbel]

On 05.09.19 13:10, Ilya Leoshkevich wrote:
> vec_unordered is vec_ordered plus a negation at the end.
> Reuse vec_unordered logic.
> 
> gcc/ChangeLog:
> 
> 2019-08-13  Ilya Leoshkevich  
> 
>   PR target/77918
>   * config/s390/vector.md (vec_unordered): Call
>   gen_vec_ordered.

Ok. Thanks!

Andreas

[IRA] Handle fully-tied destinations in a similar way to earlyclobbers

[Richard Sandiford]

IRA's make_early_clobber_and_input_conflicts checks for cases in
which an output operand is likely to be an earlyclobber and an input
operand is unlikely to be tieable with it.  If so, the allocno for
the output conflicts with the allocno for the input.  This seems
to work well.

However, a similar situation arises if an output operand is likely
to be tied to one of a set of input operands X and if another input
operand has a different value from all of the operands in X.
E.g. if we have:

  0: "=r, r"
  1: "0, r"
  2: "r, 0"
  3: "r, r"

operand 0 will always be tied to operand 1 or operand 2, so if operand 3
is different from them both, operand 0 acts like an earlyclobber as far
as operand 3 (only) is concerned.  The same is true for operand 2 in:

  0: "=r"
  1: "0"
  2: "r"

In the second example, we'd normally have a copy between operand 1 and
operand 0 if operand 1 dies in the instruction, and so there's rarely
a problem.  But if operand 1 doesn't die in the instruction, operand 0
still acts as an earlyclobber for operand 2 (if different from operand 1),
since in that case LRA must copy operand 1 to operand 0 before the
instruction.

As the existing comment says:

Avoid introducing unnecessary conflicts by checking classes of the
constraints and pseudos because otherwise significant code
degradation is possible for some targets.

I think that's doubly true here.  E.g. it's perfectly reasonable to have
constraints like:

  0: "=r, r"
  1: "0, r"
  2: "r, r"

on targets like s390 that have shorter instructions for tied operands,
but that don't want the size difference to influence RA too much.
We shouldn't treat operand 0 as earlyclobber wrt operand 2 in that case.

This patch therefore treats a normal tied non-earlyclobber output as
being effectively earlyclobber wrt to an input if it is so for *all*
preferred alternatives.

My usual bogo-comparison of gcc.c-torture, gcc.dg and g++.dg
(this time using -Os -fno-schedule-insns{,2}) gives:

Target Tests  Delta   Best  Worst Median
== =  =     = ==
aarch64-linux-gnu  3 -3 -1 -1 -1
aarch64_be-linux-gnu   4 -4 -1 -1 -1
alpha-linux-gnu  136   -190-56 84 -1
arc-elf   31   -172-27  3 -2
arm-linux-gnueabi 59   -996   -136  4 -1
arm-linux-gnueabihf   59   -996   -136  4 -1
bfin-elf  22-31-19  8 -1
bpf-elf  276   -388   -191 12 -1
cris-elf  73 69-18 26 -1
epiphany-elf  58-91-10  2 -1
fr30-elf 123   -156-33 20 -1
h8300-elf150   -426-36 17 -2
hppa64-hp-hpux11.23   39-65-16  1 -1
i686-apple-darwin 93-51-29 26 -1
i686-pc-linux-gnu 43  8-10 27 -1
m32r-elf  68-92-31 14 -1
m68k-linux-gnu   169-65-23 33 -1
mcore-elf 27-29-14  8 -1
mmix  25-75-28  2 -1
mn10300-elf  166 32-46149 -1
moxie-rtems  937   1461  -1649   6000 -1
msp430-elf89  -1364   -835  5 -4
nds32le-elf   34-54-29  2 -1
pdp11252   -458-23 13 -1
powerpc-ibm-aix7.0 3 -4 -2 -1 -1
powerpc64-linux-gnu1 -1 -1 -1 -1
powerpc64le-linux-gnu  3 -3 -1 -1 -1
rl78-elf   4-12 -4 -2 -4
rx-elf59-99-11  2 -1
s390-linux-gnu   115   -117-53 21 -1
s390x-linux-gnu  120-47-25 21 -1
sh-linux-gnu  54-89-31  8 -1
sparc64-linux-gnu 14 -6 -5  4 -1
tilepro-linux-gnu209   -452-55 16 -1
v850-elf  10 18 -2 21 -1
vax-netbsdelf  5 -5 -1 -1 -1
x86_64-darwin 53-62-33  3 -1
x86_64-linux-gnu  52 -8 -8 13 -1
xstormy16-elf144   -814   -541 25 -1
xtensa-elf   578  -2096   -138 15 -1

The eye-watering moxie-rtems +6,000 outlier is from gcc.dg/pr59992.c,
where the same code template is instantiated 10,000 times.  In some
instances the code improves by one instruction, but it regresses by
one instruction in many more.

To emphasise that this is a poor metric (and is just to get a flavour),
most of the {i686,x86_64}-linux-gnu LOC increases happen in frame info
rather than code.  I should try to script that out...

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

Richard


2019-09-30  Richard Sandiford  

gcc/
* ira-lives.c (check_and_make_def_conflict): Handle 

Re: [SVE] PR91532

[Jeff Law]

On 9/30/19 12:49 AM, Richard Biener wrote:
> On Sun, 29 Sep 2019, Jeff Law wrote:
> 
>> On 9/26/19 12:44 AM, Richard Biener wrote:
>>> On Wed, 25 Sep 2019, Prathamesh Kulkarni wrote:
>>>
 On Fri, 20 Sep 2019 at 15:20, Jeff Law  wrote:
>
> On 9/19/19 10:19 AM, Prathamesh Kulkarni wrote:
>> Hi,
>> For PR91532, the dead store is trivially deleted if we place dse pass
>> between ifcvt and vect. Would it be OK to add another instance of dse 
>> there ?
>> Or should we add an ad-hoc "basic-block dse" sub-pass to ifcvt that
>> will clean up the dead store ?
> I'd hesitate to add another DSE pass.  If there's one nearby could we
> move the existing pass?
 Well I think the nearest one is just after pass_warn_restrict. Not
 sure if it's a good
 idea to move it up from there ?
>>>
>>> You'll need it inbetween ifcvt and vect so it would be disabled
>>> w/o vectorization, so no, that doesn't work.
>>>
>>> ifcvt already invokes SEME region value-numbering so if we had
>>> MESE region DSE it could use that.  Not sure if you feel like
>>> refactoring DSE to work on regions - it currently uses a DOM
>>> walk which isn't suited for that.
>>>
>>> if-conversion has a little "local" dead predicate compute removal
>>> thingy (not that I like that), eventually it can be enhanced to
>>> do the DSE you want?  Eventually it should be moved after the local
>>> CSE invocation though.
>> We've speculated for a while that some of these dominator walking
>> optimizers could be extended to handle cases where the caller specifies
>> what blocks are "of interest".  Bin was pretty far down this path for
>> DOM which is probably the most complex.  Maybe that could be resurrected
>> in the context of DSE.
> 
> Note that DSE is way easier than DOM, it's use of a dominator walk
> is probably entirely historical given the fact that we only need to
> walk the factored virtual SSA chain.  So a (SESE) region is
> represented by two virtual SSA names (and for a loop in the ifcvt
> context by its virtual PHI in the loop header).
> 
> The issue with if-conversion and dominators is that if-conversion
> does analysis + transform loop-by-loop which means we at least
> wreck the fast dominator access by doing CFG transforms between
> analyzing separate loops in a function.  The pass is in need of
> factoring to do analysis of all loops upfront and only then
> transform.  The cleanup phase would then be after re-computing
> dominators.
If we're looking at something that is local to a block, then,
conceptually we'd just process the one interesting block.  In theory the
wrecked fast dominator access wouldn't matter.

What's more important in my mind is the state of the SSA graph.

Jeff

[PATCH], V4, patch #4.1: Enable prefixed/pc-rel addressing (revised)

[Michael Meissner]

As we discussed privately on Friday, I had a few nits to patch #4 that came up
after I submitted the patch.  The changes between that patch and this patch are:

1) I needed to change the enum non_prefixed_form to its new spelling in the
revised patch #1 that was committed.

2) In building glibc for 'future', we discovered the stack protector insns did
not use a constraint that prevented the register allocator from creating a
prefixed address.  Even though the predicate did not allow prefixed
instructions, the compiler still generated the address due to the constraint.
The previous patch used 'ZY', but 'Y' now allows prefixed addresses.  I needed
to add a second memory constraint ('eM') that prevents using a prefixed
address.

3) The function rs6000_adjust_vec_address did not have the optimizations I had
in my private branch that allowed folding extracting a constant element from a
vector in memory into a single memory instruction.  I have put those
optimizations into this patch.

4) In the previous patch, I missed setting the prefixed size and non prefixed
size for mov_ppc64 in the insn.  This pattern is used for moving PTImode
in GPR registers (on non-VSX systems, it would move TImode also).  By the time
it gets to final, it will have been split, but it is still useful to get the
sizes correct before the mode is split.

Note, patches #5-8 do not need any modification (other than adjusting the line
numbers, which patch does).  As I stated in other mail, patches #1-3 have been
checked in.

I have done a bootstrap/make check on a little endian power8 system and there
were no regressions.  I have also built both Spec 2017 rate and Spec 2006 CPU
benchmarks for each of -mcpu=power8, -mcpu=power9, and -mcpu=future, and there
were no build failures.

Can I check this into the trunk?

2019-09-30  Michael Meissner  

* config/rs6000/constraints.md (em constraint): New constraint for
non PC-relative memory.
(eM constraint): New constraint for non-prefixed memory.
* config/rs6000/predicates.md (lwa_operand): Allow odd offsets if
we have prefixed addressing.
(non_prefixed_memory): New predicate.
(non_pcrel_memory): New predicate.
(reg_or_non_pcrel_memory): New predicate.
* config/rs6000/rs6000-protos.h (make_memory_non_prefixed): New
declaration.
* config/rs6000/rs6000.c (rs6000_adjust_vec_address): Optimize
PC-relative addresses with constant offsets.  Signal an error if
we have a PC-relative address and a variable offset.  Use the
SIGNED_16BIT_OFFSET_P macro.
(rs6000_split_vec_extract_var): Signal an error if we have a
PC-relative address and a variable offset.
(quad_address_p): Add support for prefixed addresses.
(mem_operand_gpr): Add support for prefixed addresses.
(mem_operand_ds_form): Add support for prefixed addresses.
(rs6000_legitimate_offset_address_p): Add support for prefixed
addresses.
(rs6000_legitimate_address_p): Add support for prefixed
addresses.
(rs6000_mode_dependent_address): Add support for prefixed
addresses.
(rs6000_num_insns): New helper function.
(rs6000_insn_cost): Treat prefixed instructions as having the same
cost as non prefixed instructions, even though the prefixed
instructions are larger.
(make_memory_non_prefixed): New function to make a non-prefixed
memory operand.
* config/rs6000/rs6000.md (mov_64bit_dm): Add support for
prefixed addresses.
(movtd_64bit_nodm): Add support for prefixed addresses.
(mov_ppc64): Add support for prefixed addresses.
(stack_protect_setdi): Convert prefixed addresses to non-prefixed
addresses.  Allow for indexed addressing as well as offsettable.
(stack_protect_testdi): Convert prefixed addresses to non-prefixed
addresses.  Allow for indexed addressing as well as offsettable.
* config/rs6000/vsx.md (vsx_mov_64bit): Add support for
prefixed addresses.
(vsx_extract__var, VSX_D iterator): Do not allow a vector in
memory with a prefixed address to combine with variable offsets.
(vsx_extract_v4sf_var): Do not allow a vector in memory with a
prefixed address to combine with variable offsets.
(vsx_extract__var, VSX_EXTRACT_I iterator): Do not allow a
vector in memory with a prefixed address to combine with variable
offsets.
(vsx_extract__mode_var): Do not allow a vector in
memory with a prefixed address to combine with variable offsets.
* doc/md.texi (PowerPC constraints): Document the 'em' and 'eM'
constraints.

Index: gcc/config/rs6000/constraints.md
===
--- gcc/config/rs6000/constraints.md(revision 276284)
+++ gcc/config/rs6000/constraints.md(working copy)
@@ -210,6 +210,16 @@ 

[x86] Cache result of expensive_function_p between frame layouts

[Richard Sandiford]

ix86_compute_frame_layout sets use_fast_prologue_epilogue if
the function isn't more expensive than a certain threshold,
where the threshold depends on the number of saved registers.
However, the RA is allowed to insert and delete instructions
as it goes along, which can change whether this threshold is
crossed or not.

I hit this with an RA change I'm working on.  Rematerialisation
was able to remove an instruction and avoid a spill, which happened
to bring the size of the function below the threshold.  But since
nothing legitimately frame-related had changed, there was no need for
the RA to lay out the frame again.  We then failed the final sanity
check in lra_eliminate.

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

Richard


2019-09-30  Richard Sandiford  

gcc/
* config/i386/i386.h (ix86_frame::expensive_p): New field.
(ix86_frame::expensive_count): Likewise.
* config/i386/i386.c (ix86_compute_frame_layout): Make the choice
of use_fast_prologue_epilogue robust against incidental changes
in function size.

Index: gcc/config/i386/i386.h
===
--- gcc/config/i386/i386.h  2019-09-26 08:37:44.0 +0100
+++ gcc/config/i386/i386.h  2019-09-30 15:07:51.784114465 +0100
@@ -2643,6 +2643,11 @@ struct GTY(()) ix86_frame
   /* When save_regs_using_mov is set, emit prologue using
  move instead of push instructions.  */
   bool save_regs_using_mov;
+
+  /* Assume without checking that:
+   EXPENSIVE_P = expensive_function_p (EXPENSIVE_COUNT).  */
+  bool expensive_p;
+  int expensive_count;
 };
 
 /* Machine specific frame tracking during prologue/epilogue generation.  All
Index: gcc/config/i386/i386.c
===
--- gcc/config/i386/i386.c  2019-09-26 08:37:44.0 +0100
+++ gcc/config/i386/i386.c  2019-09-30 15:07:51.784114465 +0100
@@ -5876,7 +5876,14 @@ ix86_compute_frame_layout (void)
 case function is known to be outside hot spot (this is known with
 feedback only).  Weight the size of function by number of registers
 to save as it is cheap to use one or two push instructions but very
-slow to use many of them.  */
+slow to use many of them.
+
+Calling this hook multiple times with the same frame requirements
+must produce the same layout, since the RA might otherwise be
+unable to reach a fixed point or might fail its final sanity checks.
+This means that once we've assumed that a function does or doesn't
+have a particular size, we have to stick to that assumption
+regardless of how the function has changed since.  */
   if (count)
count = (count - 1) * FAST_PROLOGUE_INSN_COUNT;
   if (node->frequency < NODE_FREQUENCY_NORMAL
@@ -5884,8 +5891,14 @@ ix86_compute_frame_layout (void)
  && node->frequency < NODE_FREQUENCY_HOT))
m->use_fast_prologue_epilogue = false;
   else
-   m->use_fast_prologue_epilogue
-  = !expensive_function_p (count);
+   {
+ if (count != frame->expensive_count)
+   {
+ frame->expensive_count = count;
+ frame->expensive_p = expensive_function_p (count);
+   }
+ m->use_fast_prologue_epilogue = !frame->expensive_p;
+   }
 }
 
   frame->save_regs_using_mov

Re: [PATCH] libgomp_g.h: Include stdint.h instead of gstdint.h

[Jakub Jelinek]

On Mon, Sep 30, 2019 at 03:45:51PM +0200, Jakub Jelinek wrote:
> > Am 30.09.2019 um 09:25 schrieb Jakub Jelinek:
> > > On Mon, Sep 30, 2019 at 12:03:00AM -0700, Frederik Harwath wrote:
> > >> The patch changes libgomp/libgomp_g.h to include stdint.h instead of the 
> > >> internal gstdint.h. [...]
> > > 
> > > That looks wrong, will make libgomp less portable. [...]
> > >   Jakub
> > 
> > We have discussed this issue with Joseph Myers. Let me quote what Joseph
> > wrote:
> > 
> > "I think including  is appropriate (and, more generally,
> > removing the special configure support for GCC_HEADER_STDINT for
> > anything built only for the target - note that libgcc/gstdint.h has a
> > comment saying it's about libdecnumber portability to *hosts*, not
> > targets, without stdint.h). On any target without stdint.h, GCC should
> > be providing its own; the only targets where GCC does not yet know about
> > target stdint.h types are SymbianOS, LynxOS, QNX, TPF (see GCC bug 448),
> > and I think it's pretty unlikely libgomp would do anything useful for
> > those (and if in fact they do provide stdint.h, there wouldn't be an
> > issue anyway)."
> > 
> > Hence, I think the change will not affect portability negatively.
> 
> Ok for trunk then, I'll test removal of the gstdint.h traces from libgomp as a
> follow-up.

Here is what I'll test on top of your patch.

2019-09-30  Jakub Jelinek  

* configure.ac: Remove GCC_HEADER_STDINT(gstdint.h).
* libgomp.h: Include  instead of "gstdint.h".
* oacc-parallel.c: Don't include "libgomp_g.h".
* plugin/plugin-hsa.c: Include  instead of "gstdint.h".
* plugin/plugin-nvptx.c: Don't include "gstdint.h".
* aclocal.m4: Regenerated.
* config.h.in: Regenerated.
* configure: Regenerated.
* Makefile.in: Regenerated.

--- libgomp/configure.ac.jj 2019-05-30 20:40:50.661683381 +0200
+++ libgomp/configure.ac2019-09-30 15:46:29.585952622 +0200
@@ -171,8 +171,6 @@ AC_HEADER_TIME
 ACX_HEADER_STRING
 AC_CHECK_HEADERS(pthread.h unistd.h semaphore.h sys/loadavg.h sys/sysctl.h 
sys/time.h)
 
-GCC_HEADER_STDINT(gstdint.h)
-
 XPCFLAGS=""
 case "$host" in
   *-*-rtems*)
--- libgomp/libgomp.h.jj2019-09-03 09:42:48.253602133 +0200
+++ libgomp/libgomp.h   2019-09-30 15:49:46.669007187 +0200
@@ -42,7 +42,7 @@
 #endif
 
 #include "config.h"
-#include "gstdint.h"
+#include 
 #include "libgomp-plugin.h"
 #include "gomp-constants.h"
 
--- libgomp/oacc-parallel.c.jj  2019-06-19 10:04:27.782549120 +0200
+++ libgomp/oacc-parallel.c 2019-09-30 15:52:01.894986975 +0200
@@ -28,7 +28,6 @@
 
 #include "openacc.h"
 #include "libgomp.h"
-#include "libgomp_g.h"
 #include "gomp-constants.h"
 #include "oacc-int.h"
 #ifdef HAVE_INTTYPES_H
--- libgomp/plugin/plugin-hsa.c.jj  2019-09-20 12:25:53.260308442 +0200
+++ libgomp/plugin/plugin-hsa.c 2019-09-30 15:52:29.690571715 +0200
@@ -28,7 +28,7 @@
.  */
 
 #include "config.h"
-#include "gstdint.h"
+#include 
 #include 
 #include 
 #include 
--- libgomp/plugin/plugin-nvptx.c.jj2019-05-24 10:59:15.974314095 +0200
+++ libgomp/plugin/plugin-nvptx.c   2019-09-30 15:53:07.609005239 +0200
@@ -34,7 +34,6 @@
 #define _GNU_SOURCE
 #include "openacc.h"
 #include "config.h"
-#include "gstdint.h"
 #include "libgomp-plugin.h"
 #include "oacc-plugin.h"
 #include "gomp-constants.h"
--- libgomp/aclocal.m4.jj   2019-05-30 20:40:50.849680298 +0200
+++ libgomp/aclocal.m4  2019-09-30 15:54:06.136130872 +0200
@@ -1176,7 +1176,6 @@ m4_include([../config/lead-dot.m4])
 m4_include([../config/lthostflags.m4])
 m4_include([../config/multi.m4])
 m4_include([../config/override.m4])
-m4_include([../config/stdint.m4])
 m4_include([../config/tls.m4])
 m4_include([../ltoptions.m4])
 m4_include([../ltsugar.m4])
--- libgomp/config.h.in.jj  2019-05-23 10:43:19.458316966 +0200
+++ libgomp/config.h.in 2019-09-30 15:46:56.708546877 +0200
@@ -183,21 +183,6 @@
 /* Define if all infrastructure, needed for plugins, is supported. */
 #undef PLUGIN_SUPPORT
 
-/* The size of `char', as computed by sizeof. */
-#undef SIZEOF_CHAR
-
-/* The size of `int', as computed by sizeof. */
-#undef SIZEOF_INT
-
-/* The size of `long', as computed by sizeof. */
-#undef SIZEOF_LONG
-
-/* The size of `short', as computed by sizeof. */
-#undef SIZEOF_SHORT
-
-/* The size of `void *', as computed by sizeof. */
-#undef SIZEOF_VOID_P
-
 /* Define to 1 if you have the ANSI C header files. */
 #undef STDC_HEADERS
 
--- libgomp/configure.jj2019-09-29 13:01:28.512867791 +0200
+++ libgomp/configure   2019-09-30 15:53:31.140653688 +0200
@@ -14916,408 +14916,6 @@ fi
 done
 
 
-
-
-inttype_headers=`echo inttypes.h sys/inttypes.h  | sed -e 's/,/ /g'`
-
-acx_cv_header_stdint=stddef.h
-acx_cv_header_stdint_kind="(already complete)"
-for i in stdint.h $inttype_headers; do
-  unset ac_cv_type_uintptr_t
-  unset ac_cv_type_uintmax_t
-  unset ac_cv_type_int_least32_t
-  unset ac_cv_type_int_fast32_t
- 

Re: [PATCH, committed], V4.2, patch #2: Add prefixed insn attribute (revised #2)

[Michael Meissner]

This is the patch that I committed (along with revised patch #1 and #3).  In
addition to the changes suggested, I needed to change the enumeration
non_prefixed_form to the new name.

2019-09-30  Michael Meissner  

* config/rs6000/rs6000-protos.h (prefixed_load_p): New
declaration.
(prefixed_store_p): New declaration.
(prefixed_paddi_p): New declaration.
(rs6000_asm_output_opcode): New declaration.
(rs6000_final_prescan_insn): Move declaration and update calling
signature.
(address_is_prefixed): New helper inline function.
* config/rs6000/rs6000.c (rs6000_emit_move): Support loading
PC-relative addresses.
(reg_to_non_prefixed): New function to identify what the
non-prefixed memory instruction format is for a register.
(prefixed_load_p): New function to identify prefixed loads.
(prefixed_store_p): New function to identify prefixed stores.
(prefixed_paddi_p): New function to identify prefixed load
immediates.
(next_insn_prefixed_p): New static state variable.
(rs6000_final_prescan_insn): New function to determine if an insn
uses a prefixed instruction.
(rs6000_asm_output_opcode): New function to emit 'p' in front of a
prefixed instruction.
* config/rs6000/rs6000.h (FINAL_PRESCAN_INSN): New target hook.
(ASM_OUTPUT_OPCODE): New target hook.
* config/rs6000/rs6000.md (prefixed): New insn attribute for
prefixed instructions.
(prefixed_length): New insn attribute for the size of prefixed
instructions.
(non_prefixed_length): New insn attribute for the size of
non-prefixed instructions.
(pcrel_local_addr): New insn to load up a local PC-relative
address.
(pcrel_extern_addr): New insn to load up an external PC-relative
address.

Index: gcc/config/rs6000/rs6000-protos.h
===
--- gcc/config/rs6000/rs6000-protos.h   (revision 276277)
+++ gcc/config/rs6000/rs6000-protos.h   (working copy)
@@ -189,6 +189,31 @@ enum non_prefixed_form {
 
 extern enum insn_form address_to_insn_form (rtx, machine_mode,
enum non_prefixed_form);
+extern bool prefixed_load_p (rtx_insn *);
+extern bool prefixed_store_p (rtx_insn *);
+extern bool prefixed_paddi_p (rtx_insn *);
+extern void rs6000_asm_output_opcode (FILE *);
+extern void rs6000_final_prescan_insn (rtx_insn *, rtx [], int);
+
+/* Return true if the address can be used for a prefixed load, store, or add
+   immediate instructions that cannot be used with a non-prefixed instruction.
+   For example, using a numeric offset that is not valid for the non-prefixed
+   instruction or a PC-relative reference to a local symbol would return true,
+   but an address with an offset of 64 would not return true.
+
+   References to external PC-relative symbols aren't allowed, because GCC has
+   to load the address into a register and then issue a separate load or
+   store.  */
+
+static inline bool
+address_is_prefixed (rtx addr,
+machine_mode mode,
+enum non_prefixed_form non_prefixed)
+{
+  enum insn_form iform = address_to_insn_form (addr, mode, non_prefixed);
+  return (iform == INSN_FORM_PREFIXED_NUMERIC
+ || iform == INSN_FORM_PCREL_LOCAL);
+}
 #endif /* RTX_CODE */
 
 #ifdef TREE_CODE
@@ -268,8 +293,6 @@ extern void rs6000_d_target_versions (vo
 const char * rs6000_xcoff_strip_dollar (const char *);
 #endif
 
-void rs6000_final_prescan_insn (rtx_insn *, rtx *operand, int num_operands);
-
 extern unsigned char rs6000_class_max_nregs[][LIM_REG_CLASSES];
 extern unsigned char rs6000_hard_regno_nregs[][FIRST_PSEUDO_REGISTER];
 
Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c  (revision 276277)
+++ gcc/config/rs6000/rs6000.c  (working copy)
@@ -9639,6 +9639,14 @@ rs6000_emit_move (rtx dest, rtx source,
  return;
}
 
+  /* Use the default pattern for loading up PC-relative addresses.  */
+  if (TARGET_PCREL && mode == Pmode
+ && pcrel_local_or_external_address (operands[1], Pmode))
+   {
+ emit_insn (gen_rtx_SET (operands[0], operands[1]));
+ return;
+   }
+
   if (DEFAULT_ABI == ABI_V4
  && mode == Pmode && mode == SImode
  && flag_pic == 1 && got_operand (operands[1], mode))
@@ -24714,6 +24722,221 @@ address_to_insn_form (rtx addr,
   return INSN_FORM_BAD;
 }
 
+/* Helper function to take a REG and a MODE and turn it into the non-prefixed
+   instruction format (D/DS/DQ) used for offset memory.  */
+
+static enum non_prefixed_form
+reg_to_non_prefixed (rtx reg, machine_mode mode)
+{
+  /* If it isn't a register, use the defaults.  */
+  if (!REG_P (reg) && !SUBREG_P (reg))
+return NON_PREFIXED_DEFAULT;
+
+  

Re: [PATCH, committed] V4.2, patch #1: Rework prefixed/pc-relative lookup (revised #2)

[Michael Meissner]

This is the reworked version of patch #1 that I committed:

2019-09-30  Michael Meissner  

* config/rs6000/predicates.md (pcrel_address): Delete predicate.
(pcrel_local_address): Replace pcrel_address predicate, use the
new function address_to_insn_form.
(pcrel_external_address): Replace with new implementation using
address_to_insn_form..
(prefixed_mem_operand): Delete predicate which is now unused.
(pcrel_external_mem_operand): Delete predicate which is now
unused.
* config/rs6000/rs6000-protos.h (enum insn_form): New
enumeration.
(enum non_prefixed): New enumeration.
(address_to_insn_form): New declaration.
* config/rs6000/rs6000.c (print_operand_address): Check for either
PC-relative local symbols or PC-relative external symbols.
(mode_supports_prefixed_address_p): Delete, no longer used.
(rs6000_prefixed_address_mode_p): Delete, no longer used.
(address_to_insn_form): New function to decode an address format.

Index: gcc/config/rs6000/predicates.md
===
--- gcc/config/rs6000/predicates.md (revision 276276)
+++ gcc/config/rs6000/predicates.md (working copy)
@@ -1625,82 +1625,7 @@ (define_predicate "small_toc_ref"
   return GET_CODE (op) == UNSPEC && XINT (op, 1) == UNSPEC_TOCREL;
 })
 
-;; Return true if the operand is a pc-relative address.
-(define_predicate "pcrel_address"
-  (match_code "label_ref,symbol_ref,const")
-{
-  if (!rs6000_pcrel_p (cfun))
-return false;
-
-  if (GET_CODE (op) == CONST)
-op = XEXP (op, 0);
-
-  /* Validate offset.  */
-  if (GET_CODE (op) == PLUS)
-{
-  rtx op0 = XEXP (op, 0);
-  rtx op1 = XEXP (op, 1);
-
-  if (!CONST_INT_P (op1) || !SIGNED_34BIT_OFFSET_P (INTVAL (op1)))
-   return false;
-
-  op = op0;
-}
-
-  if (LABEL_REF_P (op))
-return true;
-
-  return (SYMBOL_REF_P (op) && SYMBOL_REF_LOCAL_P (op));
-})
-
-;; Return true if the operand is an external symbol whose address can be loaded
-;; into a register using:
-;; PLD reg,label@pcrel@got
-;;
-;; The linker will either optimize this to either a PADDI if the label is
-;; defined locally in another module or a PLD of the address if the label is
-;; defined in another module.
-
-(define_predicate "pcrel_external_address"
-  (match_code "symbol_ref,const")
-{
-  if (!rs6000_pcrel_p (cfun))
-return false;
-
-  if (GET_CODE (op) == CONST)
-op = XEXP (op, 0);
-
-  /* Validate offset.  */
-  if (GET_CODE (op) == PLUS)
-{
-  rtx op0 = XEXP (op, 0);
-  rtx op1 = XEXP (op, 1);
-
-  if (!CONST_INT_P (op1) || !SIGNED_34BIT_OFFSET_P (INTVAL (op1)))
-   return false;
-
-  op = op0;
-}
-
-  return (SYMBOL_REF_P (op) && !SYMBOL_REF_LOCAL_P (op));
-})
-
-;; Return 1 if op is a prefixed memory operand.
-(define_predicate "prefixed_mem_operand"
-  (match_code "mem")
-{
-  return rs6000_prefixed_address_mode_p (XEXP (op, 0), GET_MODE (op));
-})
-
-;; Return 1 if op is a memory operand to an external variable when we
-;; support pc-relative addressing and the PCREL_OPT relocation to
-;; optimize references to it.
-(define_predicate "pcrel_external_mem_operand"
-  (match_code "mem")
-{
-  return pcrel_external_address (XEXP (op, 0), Pmode);
-})
-
+
 ;; Match the first insn (addis) in fusing the combination of addis and loads to
 ;; GPR registers on power8.
 (define_predicate "fusion_gpr_addis"
@@ -1857,3 +1782,28 @@ (define_predicate "fusion_addis_mem_comb
 
   return 0;
 })
+
+
+;; Return true if the operand is a PC-relative address of a local symbol or a
+;; label that can be used directly in a memory operation.
+(define_predicate "pcrel_local_address"
+  (match_code "label_ref,symbol_ref,const")
+{
+  enum insn_form iform = address_to_insn_form (op, mode, NON_PREFIXED_DEFAULT);
+  return iform == INSN_FORM_PCREL_LOCAL;
+})
+
+;; Return true if the operand is a PC-relative external symbol whose address
+;; can be loaded into a register.
+(define_predicate "pcrel_external_address"
+  (match_code "symbol_ref,const")
+{
+  enum insn_form iform = address_to_insn_form (op, mode, NON_PREFIXED_DEFAULT);
+  return iform == INSN_FORM_PCREL_EXTERNAL;
+})
+
+;; Return true if the address is PC-relative and the symbol is either local or
+;; external.
+(define_predicate "pcrel_local_or_external_address"
+  (ior (match_operand 0 "pcrel_local_address")
+   (match_operand 0 "pcrel_external_address")))
Index: gcc/config/rs6000/rs6000-protos.h
===
--- gcc/config/rs6000/rs6000-protos.h   (revision 276276)
+++ gcc/config/rs6000/rs6000-protos.h   (working copy)
@@ -154,7 +154,41 @@ extern align_flags rs6000_loop_align (rt
 extern void rs6000_split_logical (rtx [], enum rtx_code, bool, bool, bool);
 extern bool rs6000_pcrel_p (struct function *);
 extern bool rs6000_fndecl_pcrel_p (const_tree);
-extern 

Re: [PATCH] libgomp_g.h: Include stdint.h instead of gstdint.h

[Jakub Jelinek]

On Mon, Sep 30, 2019 at 03:42:34PM +0200, Harwath, Frederik wrote:
> 
> Hi Jakub,
> 
> Am 30.09.2019 um 09:25 schrieb Jakub Jelinek:
> > On Mon, Sep 30, 2019 at 12:03:00AM -0700, Frederik Harwath wrote:
> >> The patch changes libgomp/libgomp_g.h to include stdint.h instead of the 
> >> internal gstdint.h. [...]
> > 
> > That looks wrong, will make libgomp less portable. [...]
> > Jakub
> 
> We have discussed this issue with Joseph Myers. Let me quote what Joseph
> wrote:
> 
> "I think including  is appropriate (and, more generally,
> removing the special configure support for GCC_HEADER_STDINT for
> anything built only for the target - note that libgcc/gstdint.h has a
> comment saying it's about libdecnumber portability to *hosts*, not
> targets, without stdint.h). On any target without stdint.h, GCC should
> be providing its own; the only targets where GCC does not yet know about
> target stdint.h types are SymbianOS, LynxOS, QNX, TPF (see GCC bug 448),
> and I think it's pretty unlikely libgomp would do anything useful for
> those (and if in fact they do provide stdint.h, there wouldn't be an
> issue anyway)."
> 
> Hence, I think the change will not affect portability negatively.

Ok for trunk then, I'll test removal of the gstdint.h traces from libgomp as a
follow-up.

Jakub

Re: [PATCH] libgomp_g.h: Include stdint.h instead of gstdint.h

[Harwath, Frederik]

Hi Jakub,

Am 30.09.2019 um 09:25 schrieb Jakub Jelinek:
> On Mon, Sep 30, 2019 at 12:03:00AM -0700, Frederik Harwath wrote:
>> The patch changes libgomp/libgomp_g.h to include stdint.h instead of the 
>> internal gstdint.h. [...]
> 
> That looks wrong, will make libgomp less portable. [...]
>   Jakub

We have discussed this issue with Joseph Myers. Let me quote what Joseph
wrote:

"I think including  is appropriate (and, more generally,
removing the special configure support for GCC_HEADER_STDINT for
anything built only for the target - note that libgcc/gstdint.h has a
comment saying it's about libdecnumber portability to *hosts*, not
targets, without stdint.h). On any target without stdint.h, GCC should
be providing its own; the only targets where GCC does not yet know about
target stdint.h types are SymbianOS, LynxOS, QNX, TPF (see GCC bug 448),
and I think it's pretty unlikely libgomp would do anything useful for
those (and if in fact they do provide stdint.h, there wouldn't be an
issue anyway)."

Hence, I think the change will not affect portability negatively.

Best regards,
Frederik

Re: [PATCH v3 4/9] S/390: Do not use signaling vector comparisons on z13

[Ilya Leoshkevich]

> Am 06.09.2019 um 12:34 schrieb Segher Boessenkool 
> :
> 
> Hi Ilya,
> 
> On Thu, Sep 05, 2019 at 01:10:14PM +0200, Ilya Leoshkevich wrote:
>> z13 supports only non-signaling vector comparisons.  This means we
>> cannot vectorize LT, LE, GT, GE and LTGT when compiling for z13.  Notify
>> middle-end about this using more restrictive operator predicate in
>> vcond.
> 
>> +(define_predicate "vcond_comparison_operator"
>> +  (match_operand 0 "comparison_operator")
>> +{
>> +  if (!HONOR_NANS (GET_MODE (XEXP (op, 0)))
>> +  && !HONOR_NANS (GET_MODE (XEXP (op, 1
>> +return true;
>> +  switch (GET_CODE (op))
>> +{
>> +case LE:
>> +case LT:
>> +case GE:
>> +case GT:
>> +case LTGT:
>> +  /* Signaling vector comparisons are supported only on z14+.  */
>> +  return TARGET_Z14;
>> +default:
>> +  return true;
>> +}
>> +})
> 
> Should you handle -fsignaling-nans here as well?

Do you mean disabling vectorisation of LE/LT/GE/GT/LTGT when
-fsignaling-nans is in effect?  This makes sense to me.  I could do that
here, but wouldn't common code (e.g. expand_vec_cond_expr_p) be a better
place?

Re: [Patch][fortran,omp-low.c] Fix OpenMP's use_device_ptr clause with array descriptors

[Tobias Burnus]

Hi all,

found bug in the run test of the patch (pending review) – fixing it 
still passes (with nvptx):


On 9/27/19 4:52 PM, Tobias Burnus wrote:

libgomp/testsuite/libgomp.fortran/use_device_ptr1.f90

[…]

+  call use_device_ptr_sub(arg2_AA, arg2_BB, arg2_CC, arg2_DD, arg2_EE, 
arg2_FF, AptrA, BptrB, N)

(3rd last line): This should call "...sub2" (i.e. use_device_ptr_sub2) and not 
"…sub" a second time!

Cheers,

Tobias

Re: [patch, libgomp] testsuite remove alloca header

[Jakub Jelinek]

On Mon, Sep 30, 2019 at 03:16:00PM +0200, Andreas Tobler wrote:
> here on FreeBSD we lack the alloca.h header so two test cases fail to
> compile.
> 
> Do the same as I did six years ago for another test case: 'Remove alloca.h
> and replace it with __builtin_alloca"
> 
> Is this ok for trunk?
> 
> TIA,
> Andreas
> 
> 2019-09-30  Andreas Tobler  
> 
>   * testsuite/libgomp.oacc-c-c++-common/loop-default.h: Remove alloca.h
>   inlcude. Replace alloca () with __builtin_alloca ().

s/inlcude/include/

Because the tests are already using GCC specific builtins
(__builtin_goacc_parlevel_id), I think that is ok, they aren't portable
anyway, CCing Thomas to confirm.

>   * testsuite/libgomp.oacc-c-c++-common/loop-dim-default.c: Likewise.

Jakub

[C][C++] Avoid exposing internal details in aka types

[Richard Sandiford]

The current aka diagnostics can sometimes leak internal details that
seem more likely to be distracting than useful.  E.g. on aarch64:

  void f (va_list *va) { *va = 1; }

gives:

  incompatible types when assigning to type ‘va_list’ {aka ‘__va_list’} from 
type ‘int’

where __va_list isn't something the user is expected to know about.
A similar thing happens for C++ on the arm_neon.h-based:

  float x;
  int8x8_t y = x;

which gives:

  cannot convert ‘float’ to ‘int8x8_t’ {aka ‘__Int8x8_t’} in initialization

This is accurate -- and __Int8x8_t is defined by the AArch64 PCS --
but it's not going to be meaningful to most users.

This patch stops the aka code looking through typedefs if all of
the following are true:

(1) the typedef is built into the compiler or comes from a system header

(2) the target of the typedef is anonymous or has a name in the
implementation namespace

(3) the target type is a tag type or vector type, which have in common that:
(a) we print their type names if they have one
(b) what we print for anonymous types isn't all that useful
("struct " etc. for tag types, pseudo-C "__vector(N) T"
for vector types)

The C side does this by recursively looking for the aka type, like the
C++ side already does.  This in turn makes "aka" work for distinct type
copies like __Int8x8_t on aarch64, fixing the ??? in aarch64/diag_aka_1.c.

On the C++ side, strip_typedefs had:

  /* Explicitly get the underlying type, as TYPE_MAIN_VARIANT doesn't
 strip typedefs with attributes.  */
  result = TYPE_MAIN_VARIANT (DECL_ORIGINAL_TYPE (TYPE_NAME (t)));
  result = strip_typedefs (result);

Applying TYPE_MAIN_VARIANT predates the strip_typedefs call, with the
comment originally contrasting with plain:

  result = TYPE_MAIN_VARIANT (t);

But the recursive call to strip_typedefs will apply TYPE_MAIN_VARIANT,
so it doesn't seem necessary to do it here too.  I think there was also
a missing "remove_attributes" argument, since wrapping something in a
typedef shouldn't change which attributes get removed.

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

Richard


2019-09-30  Richard Sandiford  

gcc/c-family/
* c-common.h (user_facing_original_type_p): Declare.
* c-common.c (user_facing_original_type_p): New function.

gcc/c/
* c-objc-common.c (useful_aka_type_p): Replace with...
(get_aka_type): ...this new function.  Given the original type,
decide which aka type to print (if any).  Only look through typedefs
if user_facing_original_type_p.
(print_type): Update accordingly.

gcc/cp/
* cp-tree.h (STF_USER_VISIBLE): New constant.
(strip_typedefs, strip_typedefs_expr): Take a flags argument.
* tree.c (strip_typedefs, strip_typedefs_expr): Likewise,
updating mutual calls accordingly.  When STF_USER_VISIBLE is true,
only look through typedefs if user_facing_original_type_p.
* error.c (dump_template_bindings, type_to_string): Pass
STF_USER_VISIBLE to strip_typedefs.
(dump_type): Likewise, unless pp_c_flag_gnu_v3 is set.

gcc/testsuite/
* g++.dg/diagnostic/aka5.h: New test.
* g++.dg/diagnostic/aka5a.C: Likewise.
* g++.dg/diagnostic/aka5b.C: Likewise.
* g++.target/aarch64/diag_aka_1.C: Likewise.
* gcc.dg/diag-aka-5.h: Likewise.
* gcc.dg/diag-aka-5a.c: Likewise.
* gcc.dg/diag-aka-5b.c: Likewise.
* gcc.target/aarch64/diag_aka_1.c (f): Expect an aka to be printed
for myvec.

Index: gcc/c-family/c-common.h
===
--- gcc/c-family/c-common.h 2019-09-30 13:54:16.0 +0100
+++ gcc/c-family/c-common.h 2019-09-30 14:16:45.002103890 +0100
@@ -1063,6 +1063,7 @@ extern tree builtin_type_for_size (int,
 extern void c_common_mark_addressable_vec (tree);
 
 extern void set_underlying_type (tree);
+extern bool user_facing_original_type_p (const_tree);
 extern void record_types_used_by_current_var_decl (tree);
 extern vec *make_tree_vector (void);
 extern void release_tree_vector (vec *);
Index: gcc/c-family/c-common.c
===
--- gcc/c-family/c-common.c 2019-09-30 13:54:16.0 +0100
+++ gcc/c-family/c-common.c 2019-09-30 14:16:45.002103890 +0100
@@ -7713,6 +7713,55 @@ set_underlying_type (tree x)
 }
 }
 
+/* Return true if it is worth exposing the DECL_ORIGINAL_TYPE of TYPE to
+   the user in diagnostics, false if it would be better to use TYPE itself.
+   TYPE is known to satisfy typedef_variant_p.  */
+
+bool
+user_facing_original_type_p (const_tree type)
+{
+  gcc_assert (typedef_variant_p (type));
+  tree decl = TYPE_NAME (type);
+
+  /* Look through any typedef in "user" code.  */
+  if (!DECL_IN_SYSTEM_HEADER (decl) && !DECL_IS_BUILTIN (decl))
+return true;
+
+  /* If the original type is also 

[C] Avoid aka types that just add tags

[Richard Sandiford]

diag-aka-1.c tests that:

  struct T { int i; } T;
  void *a;
  T *t = a;

produces:

  request for implicit conversion from 'void *' to 'T *' {aka 'struct T *'} ...

But printing an aka for the tag seems a bit redundant when the tag name
is the same as the typedef name.  It's probably not going to be telling
the user anything they don't already know, and can be distracting if "T"
rather than "struct T" is the preferred choice for an exported interface.
This is even more true if the tag is anonymous; e.g.:

  struct { int i; } T;
  void *a;
  T *t = a;

gives:

  request for implicit conversion from 'void *' to 'T *' {aka 'struct 
 *'}

[FWIW, clang also drops the aka for the errors in the test, except for
the function pointer ones.]

Rather than just drop the test above, the patch instead tests for:

  struct T { int i; } *T;

where seeing the tag definitely helps.

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

Richard


2019-09-30  Richard Sandiford  

gcc/c/
* c-objc-common.c (useful_aka_type_p): New function.
(print_type): Use it to decide whether an aka type is worth printing.

gcc/testsuite/
* gcc.dg/diag-aka-1.c (T): Turn into a pointer typedef.
(foo): Update accordingly.
* gcc.dg/diag-aka-4.c: New test.

Index: gcc/c/c-objc-common.c
===
--- gcc/c/c-objc-common.c   2019-09-30 13:54:16.0 +0100
+++ gcc/c/c-objc-common.c   2019-09-30 14:08:26.597671509 +0100
@@ -62,6 +62,73 @@ c_objc_common_init (void)
   return c_common_init ();
 }
 
+/* Return true if it's worth saying that TYPE1 is also known as TYPE2.  */
+
+static bool
+useful_aka_type_p (tree type1, tree type2)
+{
+  if (type1 == type2)
+return false;
+
+  if (type1 == error_mark_node || type2 == error_mark_node)
+return false;
+
+  if (TREE_CODE (type1) != TREE_CODE (type2))
+return true;
+
+  if (typedef_variant_p (type1))
+{
+  /* Saying that "foo" is also known as "struct foo" or
+"struct " is unlikely to be useful, since users of
+structure-like types would already know that they're structures.
+The same applies to unions and enums; in general, printing the
+tag is only useful if it has a different name.  */
+  tree_code code = TREE_CODE (type2);
+  tree id2 = TYPE_IDENTIFIER (type2);
+  if ((code == RECORD_TYPE || code == UNION_TYPE || code == ENUMERAL_TYPE)
+ && (!id2 || TYPE_IDENTIFIER (type1) == id2))
+   return false;
+
+  return true;
+}
+  else
+{
+  switch (TREE_CODE (type1))
+   {
+   case POINTER_TYPE:
+   case REFERENCE_TYPE:
+ return useful_aka_type_p (TREE_TYPE (type1), TREE_TYPE (type2));
+
+   case ARRAY_TYPE:
+ return (useful_aka_type_p (TYPE_DOMAIN (type1), TYPE_DOMAIN (type2))
+ || useful_aka_type_p (TREE_TYPE (type1), TREE_TYPE (type2)));
+
+   case FUNCTION_TYPE:
+ {
+   tree args1 = TYPE_ARG_TYPES (type1);
+   tree args2 = TYPE_ARG_TYPES (type2);
+   while (args1 != args2)
+ {
+   /* Although this shouldn't happen, it seems to wrong to assert
+  for it in a diagnostic routine.  */
+   if (!args1 || args1 == void_type_node)
+ return true;
+   if (!args2 || args2 == void_type_node)
+ return true;
+   if (useful_aka_type_p (TREE_VALUE (args1), TREE_VALUE (args2)))
+ return true;
+   args1 = TREE_CHAIN (args1);
+   args2 = TREE_CHAIN (args2);
+ }
+   return useful_aka_type_p (TREE_TYPE (type1), TREE_TYPE (type2));
+ }
+
+   default:
+ return true;
+   }
+}
+}
+
 /* Print T to CPP.  */
 
 static void
@@ -83,7 +150,7 @@ print_type (c_pretty_printer *cpp, tree
  stripped version.  But sometimes the stripped version looks
  exactly the same, so we don't want it after all.  To avoid
  printing it in that case, we play ugly obstack games.  */
-  if (TYPE_CANONICAL (t) && t != TYPE_CANONICAL (t))
+  if (TYPE_CANONICAL (t) && useful_aka_type_p (t, TYPE_CANONICAL (t)))
 {
   c_pretty_printer cpp2;
   /* Print the stripped version into a temporary printer.  */
Index: gcc/testsuite/gcc.dg/diag-aka-1.c
===
--- gcc/testsuite/gcc.dg/diag-aka-1.c   2019-03-08 18:15:07.100852863 +
+++ gcc/testsuite/gcc.dg/diag-aka-1.c   2019-09-30 14:08:26.597671509 +0100
@@ -2,7 +2,7 @@
 /* { dg-options "-Wc++-compat" } */
 
 typedef struct A { int i; } B;
-typedef struct T { int i; } T;
+typedef struct T { int i; } *T; /* { dg-warning "using 'T' as both a typedef 
and a tag is invalid" } */
 typedef const float TFA;
 typedef TFA TFB;
 typedef TFB TFC;
@@ -24,6 +24,6 @@ bar (B *b, int *i)
 int
 foo (void *a)
 {
-  T *t = a; /* { dg-warning "request for implicit 

[patch, libgomp] testsuite remove alloca header

[Andreas Tobler]

Hi,

here on FreeBSD we lack the alloca.h header so two test cases fail to 
compile.


Do the same as I did six years ago for another test case: 'Remove 
alloca.h and replace it with __builtin_alloca"


Is this ok for trunk?

TIA,
Andreas

2019-09-30  Andreas Tobler  

* testsuite/libgomp.oacc-c-c++-common/loop-default.h: Remove alloca.h
inlcude. Replace alloca () with __builtin_alloca ().
* testsuite/libgomp.oacc-c-c++-common/loop-dim-default.c: Likewise.
Index: testsuite/libgomp.oacc-c-c++-common/loop-dim-default.c
===
--- testsuite/libgomp.oacc-c-c++-common/loop-dim-default.c  (revision 
276292)
+++ testsuite/libgomp.oacc-c-c++-common/loop-dim-default.c  (working copy)
@@ -1,7 +1,6 @@
 /* { dg-additional-options "-fopenacc-dim=16:16" } */
 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -28,9 +27,9 @@
 {
   int exit = 0;
   int ix;
-  int *gangs = (int *)alloca (gp * sizeof (int));
-  int *workers = (int *)alloca (wp * sizeof (int));
-  int *vectors = (int *)alloca (vp * sizeof (int));
+  int *gangs = (int *)__builtin_alloca (gp * sizeof (int));
+  int *workers = (int *)__builtin_alloca (wp * sizeof (int));
+  int *vectors = (int *)__builtin_alloca (vp * sizeof (int));
   int offloaded = 0;
   
   memset (gangs, 0, gp * sizeof (int));
Index: testsuite/libgomp.oacc-c-c++-common/loop-default.h
===
--- testsuite/libgomp.oacc-c-c++-common/loop-default.h  (revision 276292)
+++ testsuite/libgomp.oacc-c-c++-common/loop-default.h  (working copy)
@@ -1,5 +1,4 @@
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -28,9 +27,9 @@
 {
   int exit = 0;
   int ix;
-  int *gangs = (int *)alloca (gp * sizeof (int));
-  int *workers = (int *)alloca (wp * sizeof (int));
-  int *vectors = (int *)alloca (vp * sizeof (int));
+  int *gangs = (int *)__builtin_alloca (gp * sizeof (int));
+  int *workers = (int *)__builtin_alloca (wp * sizeof (int));
+  int *vectors = (int *)__builtin_alloca (vp * sizeof (int));
   int offloaded = 0;
 
   memset (gangs, 0, gp * sizeof (int));

[C] Improve diagnostics for vector types

[Richard Sandiford]

Given the following invalid arm_neon.h-based code:

  float x;
  int8x8_t y = x;

the error message we emit is pretty good:

  incompatible types when initializing type 'int8x8_t' using type 'float'

But convert the types to pointers:

  int8x8_t *ptr = 

and the message becomes:

  initialization of '__vector(8) signed char *' from incompatible pointer type 
'float *'

Although it's reasonably obvious what '__vector(8) signed char *' means,
it isn't valid C or C++ syntax and is quite far from what the user wrote,
so using 'int8x8_t *' would be better.

This patch therefore prints the type name of vectors that have one.
It's still OK to print the __vector syntax as an "aka", although I have
a follow-on patch to tweak this slightly for types defined in system
header files.  The follow-on patch also addresses the ??? in
gcc.target/aarch64/diag_aka_1.c.

The C++ test already passed, but it seemed worth including for
consistency.

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

Richard


2019-09-30  Richard Sandiford  

gcc/c-family/
* c-pretty-print.c (pp_c_specifier_qualifier_list): If a vector type
has a type name, use it in preference to the __vector syntax.

gcc/testsuite/
* gcc.dg/diag-aka-3.c: New test.
* gcc.target/aarch64/diag_aka_1.c: New test.
* g++.dg/diagnostic/aka4.C: New test.

Index: gcc/c-family/c-pretty-print.c
===
--- gcc/c-family/c-pretty-print.c   2019-09-30 13:54:18.387750214 +0100
+++ gcc/c-family/c-pretty-print.c   2019-09-30 14:01:05.904830151 +0100
@@ -470,6 +470,16 @@ pp_c_specifier_qualifier_list (c_pretty_
 ? "_Complex" : "__complex__"));
   else if (code == VECTOR_TYPE)
{
+ /* The syntax we print for vector types isn't real C or C++ syntax,
+so it's better to print the type name if we have one.  */
+ tree name = TYPE_NAME (t);
+ if (!(pp->flags & pp_c_flag_gnu_v3)
+ && name
+ && TREE_CODE (name) == TYPE_DECL)
+   {
+ pp->id_expression (name);
+ break;
+   }
  pp_c_ws_string (pp, "__vector");
  pp_c_left_paren (pp);
  pp_wide_integer (pp, TYPE_VECTOR_SUBPARTS (t));
Index: gcc/testsuite/gcc.dg/diag-aka-3.c
===
--- /dev/null   2019-09-17 11:41:18.176664108 +0100
+++ gcc/testsuite/gcc.dg/diag-aka-3.c   2019-09-30 14:01:05.904830151 +0100
@@ -0,0 +1,9 @@
+typedef unsigned int myvec __attribute__((vector_size (16)));
+
+void f (float x)
+{
+  myvec y = x; /* { dg-error {incompatible types when initializing type 
'myvec' {aka '__vector\([48]\) unsigned int'} using type 'float'} } */
+  myvec *ptr =  /* { dg-error {initialization of 'myvec \*' {aka 
'__vector\([48]\) unsigned int \*'} from incompatible pointer type 'float \*'} 
} */
+  const myvec *const_ptr =  /* { dg-error {initialization of 'const myvec 
\*' {aka 'const __vector\([48]\) unsigned int \*'} from incompatible pointer 
type 'float \*'} } */
+  volatile myvec *volatile_ptr =  /* { dg-error {initialization of 
'volatile myvec \*' {aka 'volatile __vector\([48]\) unsigned int \*'} from 
incompatible pointer type 'float \*'} } */
+}
Index: gcc/testsuite/gcc.target/aarch64/diag_aka_1.c
===
--- /dev/null   2019-09-17 11:41:18.176664108 +0100
+++ gcc/testsuite/gcc.target/aarch64/diag_aka_1.c   2019-09-30 
14:01:05.908830119 +0100
@@ -0,0 +1,14 @@
+#include 
+
+typedef int16x4_t myvec;
+
+void f (float x)
+{
+  __Int8x8_t y1 = x; /* { dg-error {incompatible types when initializing type 
'__Int8x8_t' using type 'float'} } */
+  __Int8x8_t *ptr1 =  /* { dg-error {initialization of '__Int8x8_t \*' from 
incompatible pointer type 'float \*'} } */
+  int8x8_t y2 = x; /* { dg-error {incompatible types when initializing type 
'int8x8_t' using type 'float'} } */
+  int8x8_t *ptr2 =  /* { dg-error {initialization of 'int8x8_t \*' from 
incompatible pointer type 'float \*'} } */
+  /* ??? For these it would be better to print an aka for 'int16x4_t'.  */
+  myvec y3 = x; /* { dg-error {incompatible types when initializing type 
'myvec' using type 'float'} } */
+  myvec *ptr3 =  /* { dg-error {initialization of 'myvec \*' from 
incompatible pointer type 'float \*'} } */
+}
Index: gcc/testsuite/g++.dg/diagnostic/aka4.C
===
--- /dev/null   2019-09-17 11:41:18.176664108 +0100
+++ gcc/testsuite/g++.dg/diagnostic/aka4.C  2019-09-30 14:01:05.904830151 
+0100
@@ -0,0 +1,9 @@
+typedef unsigned int myvec __attribute__((vector_size (16)));
+
+void f (float x)
+{
+  myvec y = x; // { dg-error {cannot convert 'float' to 'myvec' {aka 
'__vector\([48]\) unsigned int'} in initialization} }
+  myvec *ptr =  // { dg-error {cannot convert 'float\*' to 'myvec\*' {aka 

Re: [C] Print ", ..." rather than ", ..." in diagnostics

[Marek Polacek]

On Mon, Sep 30, 2019 at 01:57:49PM +0100, Richard Sandiford wrote:
> pp_separate_with inserts a space after the separator, so there's
> no need to add whitespace before "..." as well.
> 
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  Bordering on obvious,
> but I wasn't 100% sure if this would be the preferred fix or not, so...
> OK to install?

Ok.

> 2019-09-30  Richard Sandiford  
> 
> gcc/c-family/
>   * c-pretty-print.c (pp_c_parameter_type_list): Avoid printing
>   two spaces between a comma and "...".
> 
> gcc/testsuite/
>   * gcc.dg/Wincompatible-pointer-types-1.c (f1): Expect only one
>   space between the comma and "...".
> 
> Index: gcc/c-family/c-pretty-print.c
> ===
> --- gcc/c-family/c-pretty-print.c 2019-03-08 18:14:25.857009653 +
> +++ gcc/c-family/c-pretty-print.c 2019-09-30 13:54:18.387750214 +0100
> @@ -525,7 +525,7 @@ pp_c_parameter_type_list (c_pretty_print
>if (!first && !parms)
>   {
> pp_separate_with (pp, ',');
> -   pp_c_ws_string (pp, "...");
> +   pp_string (pp, "...");
>   }
>  }
>pp_c_right_paren (pp);
> Index: gcc/testsuite/gcc.dg/Wincompatible-pointer-types-1.c
> ===
> --- gcc/testsuite/gcc.dg/Wincompatible-pointer-types-1.c  2019-03-08 
> 18:15:05.048860662 +
> +++ gcc/testsuite/gcc.dg/Wincompatible-pointer-types-1.c  2019-09-30 
> 13:54:18.387750214 +0100
> @@ -7,6 +7,6 @@ void f (int, ...);
>  f1 (void)
>  {
>int (*x) ();
> -  x = f; /* { dg-error "assignment to 'int \\(\\*\\)\\(\\)' from 
> incompatible pointer type 'void \\(\\*\\)\\(int,  \.\.\.\\)'" } */
> +  x = f; /* { dg-error "assignment to 'int \\(\\*\\)\\(\\)' from 
> incompatible pointer type 'void \\(\\*\\)\\(int, \.\.\.\\)'" } */
>return x (1);
>  }

Marek

[C] Print ", ..." rather than ", ..." in diagnostics

[Richard Sandiford]

pp_separate_with inserts a space after the separator, so there's
no need to add whitespace before "..." as well.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  Bordering on obvious,
but I wasn't 100% sure if this would be the preferred fix or not, so...
OK to install?

Richard


2019-09-30  Richard Sandiford  

gcc/c-family/
* c-pretty-print.c (pp_c_parameter_type_list): Avoid printing
two spaces between a comma and "...".

gcc/testsuite/
* gcc.dg/Wincompatible-pointer-types-1.c (f1): Expect only one
space between the comma and "...".

Index: gcc/c-family/c-pretty-print.c
===
--- gcc/c-family/c-pretty-print.c   2019-03-08 18:14:25.857009653 +
+++ gcc/c-family/c-pretty-print.c   2019-09-30 13:54:18.387750214 +0100
@@ -525,7 +525,7 @@ pp_c_parameter_type_list (c_pretty_print
   if (!first && !parms)
{
  pp_separate_with (pp, ',');
- pp_c_ws_string (pp, "...");
+ pp_string (pp, "...");
}
 }
   pp_c_right_paren (pp);
Index: gcc/testsuite/gcc.dg/Wincompatible-pointer-types-1.c
===
--- gcc/testsuite/gcc.dg/Wincompatible-pointer-types-1.c2019-03-08 
18:15:05.048860662 +
+++ gcc/testsuite/gcc.dg/Wincompatible-pointer-types-1.c2019-09-30 
13:54:18.387750214 +0100
@@ -7,6 +7,6 @@ void f (int, ...);
 f1 (void)
 {
   int (*x) ();
-  x = f; /* { dg-error "assignment to 'int \\(\\*\\)\\(\\)' from incompatible 
pointer type 'void \\(\\*\\)\\(int,  \.\.\.\\)'" } */
+  x = f; /* { dg-error "assignment to 'int \\(\\*\\)\\(\\)' from incompatible 
pointer type 'void \\(\\*\\)\\(int, \.\.\.\\)'" } */
   return x (1);
 }

[C++ Patch] More cp_expr_loc_or_input_loc and DECL_SOURCE_LOCATION uses

[Paolo Carlini]

Hi,

a few more in init.c and name-lookup.c, respectively. Tested 
x86_64-linux, as usual.


Thanks, Paolo.

///

/cp
2019-09-30  Paolo Carlini  

* init.c (build_new): Use cp_expr_loc_or_input_loc in two places.
* name-lookup.c (do_pushdecl): Use DECL_SOURCE_LOCATION.
(push_class_level_binding_1): Likewise.
(set_decl_namespace): Likewise.

/testsuite
2019-09-30  Paolo Carlini  

* g++.dg/diagnostic/integral-array-size-1.C: New.
* g++.dg/cpp0x/alias-decl-1.C: Test location(s) too.
* g++.dg/init/new43.C: Likewise.
* g++.dg/lookup/friend12.C: Likewise.
* g++.dg/lookup/pr79766.C: Likewise.
* g++.dg/lookup/pr84375.C: Likewise.
* g++.dg/other/new-size-type.C: Likewise.
Index: cp/init.c
===
--- cp/init.c   (revision 276151)
+++ cp/init.c   (working copy)
@@ -3759,7 +3759,8 @@ build_new (vec **placement, tree type
   if (!build_expr_type_conversion (WANT_INT | WANT_ENUM, nelts, false))
 {
   if (complain & tf_error)
-permerror (input_location, "size in array new must have integral 
type");
+   permerror (cp_expr_loc_or_input_loc (nelts),
+  "size in array new must have integral type");
   else
 return error_mark_node;
 }
@@ -3774,7 +3775,8 @@ build_new (vec **placement, tree type
 less than zero. ... If the expression is a constant expression,
 the program is ill-fomed.  */
   if (TREE_CODE (cst_nelts) == INTEGER_CST
- && !valid_array_size_p (input_location, cst_nelts, NULL_TREE,
+ && !valid_array_size_p (cp_expr_loc_or_input_loc (nelts),
+ cst_nelts, NULL_TREE,
  complain & tf_error))
return error_mark_node;
 
Index: cp/name-lookup.c
===
--- cp/name-lookup.c(revision 276151)
+++ cp/name-lookup.c(working copy)
@@ -3080,8 +3080,9 @@ do_pushdecl (tree decl, bool is_friend)
  /* In a local class, a friend function declaration must
 find a matching decl in the innermost non-class scope.
 [class.friend/11] */
- error ("friend declaration %qD in local class without "
-"prior local declaration", decl);
+ error_at (DECL_SOURCE_LOCATION (decl),
+   "friend declaration %qD in local class without "
+   "prior local declaration", decl);
  /* Don't attempt to push it.  */
  return error_mark_node;
}
@@ -4451,9 +4452,9 @@ push_class_level_binding_1 (tree name, tree x)
   tree scope = context_for_name_lookup (x);
   if (TYPE_P (scope) && same_type_p (scope, current_class_type))
{
- error ("%qD has the same name as the class in which it is "
-"declared",
-x);
+ error_at (DECL_SOURCE_LOCATION (x),
+   "%qD has the same name as the class in which it is "
+   "declared", x);
  return false;
}
 }
@@ -4757,7 +4758,8 @@ set_decl_namespace (tree decl, tree scope, bool fr
   /* Writing "N::i" to declare something directly in "N" is invalid.  */
   if (CP_DECL_CONTEXT (decl) == current_namespace
  && at_namespace_scope_p ())
-   error ("explicit qualification in declaration of %qD", decl);
+   error_at (DECL_SOURCE_LOCATION (decl),
+ "explicit qualification in declaration of %qD", decl);
   return;
 }
 
Index: testsuite/g++.dg/cpp0x/alias-decl-1.C
===
--- testsuite/g++.dg/cpp0x/alias-decl-1.C   (revision 276151)
+++ testsuite/g++.dg/cpp0x/alias-decl-1.C   (working copy)
@@ -12,5 +12,5 @@ template struct Ptr {}; // { dg-error
 
 struct A {
 using A = int;  // { dg-error "11:ISO C\\+\\+ forbids nested type .A." }
-// { dg-error "same name as" "" { target c++11 } .-1 }  
+// { dg-error "11:.using A = int. has the same name as" "" { target c++11 } 
.-1 }  
 };
Index: testsuite/g++.dg/diagnostic/integral-array-size-1.C
===
--- testsuite/g++.dg/diagnostic/integral-array-size-1.C (nonexistent)
+++ testsuite/g++.dg/diagnostic/integral-array-size-1.C (working copy)
@@ -0,0 +1,7 @@
+template
+void foo(T a)
+{
+  new int[a];  // { dg-error "11:size in array new must have integral type" }
+}
+
+template void foo(float);
Index: testsuite/g++.dg/init/new43.C
===
--- testsuite/g++.dg/init/new43.C   (revision 276151)
+++ testsuite/g++.dg/init/new43.C   (working copy)
@@ -30,36 +30,36 @@ void test_literal ()
 B b;
 
 // Verify integer 

[PATCH] Implement LWG 3255 for std::span constructors

[Jonathan Wakely]

Also fix the constraints on span(Container&) and span(const Container&)
constructors so that they aren't used for const spans or const arrays.

* include/std/span (span(element_type(&)[N]))
(span(array&), span(const array&)):
Deduce array element type to allow safe const conversions (LWG 3255).
[!_GLIBCXX_P1394] (span(Container&), span(const Container&)): Use
remove_cv_t on arguments to __is_std_span and __is_std_array.
* testsuite/23_containers/span/lwg3255.cc: New test.

This library issue hasn't been resolved yet, but the proposed
resolution looks right and so I'm implementing it now.

Tested powerpc64le-linux, committed to trunk.


commit f7686704e7edb8b38b4d4726726b97b78ae2da8d
Author: Jonathan Wakely 
Date:   Mon Sep 30 11:48:14 2019 +0100

Implement LWG 3255 for std::span constructors

Also fix the constraints on span(Container&) and span(const Container&)
constructors so that they aren't used for const spans or const arrays.

* include/std/span (span(element_type(&)[N]))
(span(array&), span(const array&)):
Deduce array element type to allow safe const conversions (LWG 
3255).
[!_GLIBCXX_P1394] (span(Container&), span(const Container&)): Use
remove_cv_t on arguments to __is_std_span and __is_std_array.
* testsuite/23_containers/span/lwg3255.cc: New test.

diff --git a/libstdc++-v3/include/std/span b/libstdc++-v3/include/std/span
index 1a0d61c1947..fcec22a6c57 100644
--- a/libstdc++-v3/include/std/span
+++ b/libstdc++-v3/include/std/span
@@ -125,6 +125,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template
using __is_compatible = is_convertible<_Tp(*)[], _Type(*)[]>;
 
+  // _GLIBCXX_RESOLVE_LIB_DEFECTS
+  // 3255. span's array constructor is too strict
+  template>
+   using __is_compatible_array  = __is_compatible<_Tp>;
+
 public:
   // member types
   using value_type = remove_cv_t<_Type>;
@@ -149,9 +156,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   // constructors
 
-  template 
- && (_Extent == dynamic_extent || _Extent == 0)>* = nullptr>
+  template* = nullptr>
constexpr
span() noexcept : _M_extent(0), _M_ptr(nullptr)
{ }
@@ -159,28 +165,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   constexpr
   span(const span&) noexcept = default;
 
-  template*
- = nullptr>
+  template>>
constexpr
-   span(element_type (&__arr)[_ArrayExtent]) noexcept
+   span(_Tp (&__arr)[_ArrayExtent]) noexcept
: span(static_cast(__arr), _ArrayExtent)
{ }
 
-  template*
- = nullptr>
+  template>>
constexpr
-   span(array& __arr) noexcept
-   : span(__arr.data(), _ArrayExtent)
+   span(array<_Tp, _ArrayExtent>& __arr) noexcept
+   : span(static_cast(__arr.data()), _ArrayExtent)
{ }
 
-  template*
- = nullptr>
+  template>>
constexpr
-   span(const array& __arr) noexcept
-   : span(__arr.data(), _ArrayExtent)
+   span(const array<_Tp, _ArrayExtent>& __arr) noexcept
+   : span(static_cast(__arr.data()), _ArrayExtent)
{ }
 
   // NOTE: when the time comes, and P1394 -
@@ -271,8 +274,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 public:
   template,
-   __not_<__detail::__is_std_span<_Container>>,
-   __not_<__detail::__is_std_array<_Container>>,
+   __not_<__detail::__is_std_span>>,
+   __not_<__detail::__is_std_array>>,
__not_>,
__is_compatible_container<_Container>>>
constexpr
@@ -283,8 +286,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   template,
-   __not_<__detail::__is_std_span<_Container>>,
-   __not_<__detail::__is_std_array<_Container>>,
+   __not_<__detail::__is_std_span>>,
+   __not_<__detail::__is_std_array>>,
__not_>,
__is_compatible_container>>
constexpr
diff --git a/libstdc++-v3/testsuite/23_containers/span/lwg3255.cc 
b/libstdc++-v3/testsuite/23_containers/span/lwg3255.cc
new file mode 100644
index 000..638c88101f9
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/span/lwg3255.cc
@@ -0,0 +1,69 @@
+// Copyright (C) 2019 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 

[PATCH] PR libstdc++/77936 remove unused variable

[Jonathan Wakely]

PR libstdc++/77936
* include/parallel/checkers.h (__is_sorted): Remove unused variable.

Committed as obvious.

commit 917359de04e5127ebdbb88d20b891a4140a1d266
Author: Jonathan Wakely 
Date:   Mon Sep 30 11:07:01 2019 +0100

PR libstdc++/77936 remove unused variable

PR libstdc++/77936
* include/parallel/checkers.h (__is_sorted): Remove unused variable.

diff --git a/libstdc++-v3/include/parallel/checkers.h 
b/libstdc++-v3/include/parallel/checkers.h
index f2737c791d6..e325b0ab8b4 100644
--- a/libstdc++-v3/include/parallel/checkers.h
+++ b/libstdc++-v3/include/parallel/checkers.h
@@ -55,7 +55,6 @@ namespace __gnu_parallel
 
   _IIter __current(__begin), __recent(__begin);
 
-  unsigned long long __position = 1;
   for (__current++; __current != __end; __current++)
 {
   if (__comp(*__current, *__recent))
@@ -63,7 +62,6 @@ namespace __gnu_parallel
   return false;
 }
   __recent = __current;
-  __position++;
 }
 
   return true;

Re: [C++ Patch] Improve grokdeclarator error

[Paolo Carlini]

Hi,

On 28/09/19 04:05, Jason Merrill wrote:

On 9/26/19 3:39 PM, Paolo Carlini wrote:
+template void foo(  // { dg-error "15:template-id .foo. 
used as a declarator" }


That's a strange diagnostic message; there's nothing wrong with using 
a template-id as a declarator.  Why are we even calling grokdeclarator 
when we hit EOF in the middle of the declarator?


It's indeed a weird situation. Note, by the way, that for

template void foo;

we end up giving the same diagnostic, only, the location was already fine.

Anyway, to explain why I say weird, clang believes it's dealing with an 
explicit instantiation:


explicit instantiation of 'foo' does not refer to a function template, 
variable template, member function, member class, or static data member


whereas EDG gives:

declaration is incompatible with function template "void foo<>()"

I *think* what we are issuing is closer in spirit to the latter, we 
don't get fooled into thinking it's an instantiation and we say that as 
a declaration doesn't work either. See what I mean? Removing completely 
the diagnostic code doesn't seem fine either because we end up with very 
confusing wordings like


variable or field declared void

or worse we mention variable templates, depending on the type (I already 
mentioned this).


Thus, all in all, would it make sense to simply issue something closer 
to EDG?


Thanks, Paolo.

[PATCH] Dissect vect_create_epilog_for_reduction

[Richard Biener]

This completes the first refactoring by decoupling epilogue creation
(vect_create_epilog_for_reduction) from vectorizing of the "reduction 
stmt" (vectorizable_reduction).

The epilogue is now created when vectorizing the live result of the
reduction stmt, thus through vectorizable_live_operation.

Backedge values of vectorized PHIs are filled when we vectorized
their defs.

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

This was the first step to make the "reduction stmt" not vectorized
from vectorizable_reduction and thus generalizing "reduction stmt".

The next step is probably to split out nested cycle handling though
(first simplify code before turning it upside-down...).

Richard.

2019-09-30  Richard Biener  

* gimple.c (gimple_get_lhs): For PHIs return the result.
* tree-vectorizer.h (vectorizable_live_operation): Also get the
SLP instance as argument.
* tree-vect-loop.c (vect_analyze_loop_operations): Also handle
double-reduction PHIs with vectorizable_lc_phi.
(vect_analyze_loop_operations): Adjust.
(vect_create_epilog_for_reduction): Remove all code not dealing
with reduction LC PHI or epilogue generation.
(vectorizable_live_operation): Call vect_create_epilog_for_reduction
for live stmts of reductions.
* tree-vect-stmts.c (vectorizable_condition): When !for_reduction
do not handle defs that are not vect_internal_def.
(can_vectorize_live_stmts): Adjust.
(vect_analyze_stmt): When the vectorized stmt defined a value
used on backedges adjust the backedge uses of vectorized PHIs.

Index: gcc/gimple.c
===
--- gcc/gimple.c(revision 276292)
+++ gcc/gimple.c(working copy)
@@ -1797,6 +1797,8 @@ gimple_get_lhs (const gimple *stmt)
 return gimple_assign_lhs (stmt);
   else if (code == GIMPLE_CALL)
 return gimple_call_lhs (stmt);
+  else if (code == GIMPLE_PHI)
+return gimple_phi_result (stmt);
   else
 return NULL_TREE;
 }
Index: gcc/tree-vect-loop.c
===
--- gcc/tree-vect-loop.c(revision 276292)
+++ gcc/tree-vect-loop.c(working copy)
@@ -1526,7 +1526,9 @@ vect_analyze_loop_operations (loop_vec_i
  != vect_used_in_outer_by_reduction))
return opt_result::failure_at (phi, "unsupported phi\n");
 
- if (STMT_VINFO_DEF_TYPE (stmt_info) == vect_internal_def
+ if ((STMT_VINFO_DEF_TYPE (stmt_info) == vect_internal_def
+  || (STMT_VINFO_DEF_TYPE (stmt_info)
+  == vect_double_reduction_def))
  && !vectorizable_lc_phi (stmt_info, NULL, NULL))
return opt_result::failure_at (phi, "unsupported phi\n");
 }
@@ -1564,8 +1566,8 @@ vect_analyze_loop_operations (loop_vec_i
  if (ok
  && STMT_VINFO_LIVE_P (stmt_info)
  && !PURE_SLP_STMT (stmt_info))
-   ok = vectorizable_live_operation (stmt_info, NULL, NULL, -1, NULL,
- _vec);
+   ok = vectorizable_live_operation (stmt_info, NULL, NULL, NULL,
+ -1, NULL, _vec);
 
   if (!ok)
return opt_result::failure_at (phi,
@@ -4260,6 +4275,20 @@ vect_create_epilog_for_reduction (stmt_v
  slp_instance slp_node_instance)
 {
   loop_vec_info loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_info);
+  /* For double reductions we need to get at the inner loop reduction
+ stmt which has the meta info attached.  Our stmt_info is that of the
+ loop-closed PHI of the inner loop which we remember as
+ def for the reduction PHI generation.  */
+  bool double_reduc = false;
+  stmt_vec_info rdef_info = stmt_info;
+  if (STMT_VINFO_DEF_TYPE (stmt_info) == vect_double_reduction_def)
+{
+  gcc_assert (!slp_node);
+  double_reduc = true;
+  stmt_info = loop_vinfo->lookup_def (gimple_phi_arg_def
+   (stmt_info->stmt, 0));
+  stmt_info = vect_stmt_to_vectorize (stmt_info);
+}
   gphi *reduc_def_stmt
 = as_a  (STMT_VINFO_REDUC_DEF (vect_orig_stmt (stmt_info))->stmt);
   enum tree_code code = STMT_VINFO_REDUC_CODE (stmt_info);
@@ -4269,20 +4298,6 @@ vect_create_epilog_for_reduction (stmt_v
 neutral_op
   = neutral_op_for_slp_reduction (slp_node_instance->reduc_phis, code,
  REDUC_GROUP_FIRST_ELEMENT (stmt_info));
-  bool double_reduc = false;
-  if (nested_in_vect_loop_p (LOOP_VINFO_LOOP (loop_vinfo), stmt_info))
-{
-  tree def_arg
-   = PHI_ARG_DEF_FROM_EDGE (reduc_def_stmt,
-loop_preheader_edge 
-  (LOOP_VINFO_LOOP (loop_vinfo)->inner));
-  

Re: [PATCH] Automatics in equivalence statements

[Jakub Jelinek]

On Sat, Sep 28, 2019 at 10:33:26PM +0200, Andreas Schwab wrote:
> On Aug 14 2019, Mark Eggleston  wrote:
> 
> >     * gfortran.dg/auto_in_equiv_3.f90: New test.
> 
> This test fails everywhere.

Yes, and _2 on i686-linux at -O0.

To me both testcases are undefined behavior.
E.g. the first one has:
  subroutine suba(option)
integer, intent(in) :: option
integer, automatic :: a
integer :: b
integer :: c
equivalence (a, b)
if (option.eq.0) then
  ! initialise a and c
  a = 9
  c = 99
  if (a.ne.b) stop 1
  if (loc(a).ne.loc(b)) stop 2
else
  ! a should've been overwritten
  if (a.eq.9) stop 3
end if
  end subroutine suba
My understanding is that because a is explicitly automatic and b is automatic 
too
(implicitly), the whole equivalence is automatic, so if you call this
subroutine with non-zero option, you read an uninitialized variable and
compare it to 9.  That can result in anything, .false., .true., disk
formatting, you can't rely on some other routine laying out its automatic
variable at exactly the same spot and overwriting the memory in there, not
to mention that the compiler can easily spot the uninitialized use too.
Similarly in the second test, returning address of an automatic variable
from a function is already something e.g. the C/C++ FEs warn about, because
you really can't do anything useful with that address, it can't be
dereferenced, or even the comparisons to addresses of other automatic
variables that left their scope is wrong.

IMHO if you want to check if a variable is SAVEd or AUTOMATIC, you want to
recurse, either directly or indirectly, pass the address of the variable in
the outer subroutine down to the inner one and compare there, SAVEd
variables need to have the same address, while AUTOMATIC variables where
both the outer and inner variable is at that point still in the scope need
to have the addresses different.

Though, in order to have in Fortran a recursively callable subroutine, one
needs to use RECURSIVE.
So, IMHO we want 4 testcases out of these 2, two dg-do compile only which
will verify the tests compile when mixing automatic with no explicit
save/automatic in equivalence and will -fdump-tree-gimple and scan the
gimple dump to verify there is equiv.\[0-9]* variable which is not static,
and then two runtime testcases like (one with just -fdec-static, one with
also -fno-automatic, though guess it doesn't matter that much, as recursive
already implies that it is automatic).
program test
  integer :: dummy
  integer, parameter :: address = kind(loc(dummy))
  integer(address) :: addr
  addr = 0
  call sub (0, addr)
contains
  recursive subroutine sub (option, addr)
integer, intent(in) :: option
integer(address), intent(in) :: addr
integer, automatic :: a
integer :: b
integer(address) :: c
equivalence (a, b)
if (option.eq.0) then
  a = 9
  if (a.ne.b) stop 1
  if (loc(a).ne.loc(b)) stop 2
  c = loc(a)
  call sub (1, c)
  if (a.ne.9) stop 3
else
  a = 10
  if (a.ne.b) stop 4
  if (loc(a).ne.loc(b)) stop 5
  if (addr.eq.loc(a)) stop 6
end if
  end subroutine sub
end program test

Jakub

Re: [MIPS] GCC: Fix Loongson3 LLSC Errata

[Jiaxun Yang]

On 2018/11/30 下午8:45, Paul Hua wrote:

In some older Loongson3 processors there is a LL/SC errata that can
cause the CPU to deadlock occasionally.  The details are very
complicated. We find a way to work around this errata by a) adding a
sync before ll/lld instruction, b) adding a sync
before branch target that between ll and sc. The assembler do the jobs
'a', gcc do the jobs 'b'.


Hi all,

Any update about this patch?

As this workaround is the last barrier for generic MIPS distros to run 
on Loongson systems.


Btw, I heard from Loongson guys that type A (adding a sync before ll/lld 
instruction) of this errata only exist on pre-3A2000 (GS464) processors 
while type B exist on all processors. Is that true?


If so, then they should have different naming in flags to prevent 
confusing. I saw that you are using "LLTGT" and "LLSYNC" in cpucfg bit 
domin of 3A4000, should we also use that naming in toolchain?



Thanks.

--

Jiaxun Yang


This patch also add a configure options
--with-mips-fix-loongson3-llsc=[yes|no] to enable fix-loongson3-llsc
by config.

>From 16f0fd9e32d2098637dc0eb3e576444c48c43f22 Mon Sep 17 00:00:00 2001
From: Chenghua Xu 
Date: Fri, 30 Nov 2018 19:57:38 +0800
Subject: [PATCH] [MIPS][GCC]  Fix Loongson3 LLSC Errata.

gcc/
* config.gcc (supported_defaults): Add fix-loongson3-llsc
(with_fix_loongson3_llsc): Add validation.
(all_defaults): Add fix-loongson3-llsc.
* config/mips/mips.c (mips_process_sync_loop): Add sync before
branch target that between ll and sc.
* config/mips/mips.h (OPTION_DEFAULT_SPECS): Add a default for
fix-loongson3-llsc.
gcc/config/mips/mips.opt: New option.
* doc/install.texi (--with-fix-loongson3-llsc):Document the new
option.
* doc/invoke.texi (-mfix-loongson3-llsc):Document the new option.

gcc/testsuite/
* gcc.target/mips/fix-loongson3-llsc.c: New test.
* gcc.target/mips/mips.exp (option): Add fix-loongson3-llsc.
---
  gcc/config.gcc| 19 +--
  gcc/config/mips/mips.c| 13 +++--
  gcc/config/mips/mips.h|  4 +++-
  gcc/config/mips/mips.opt  |  4 
  gcc/doc/install.texi  |  4 
  gcc/doc/invoke.texi   |  8 
  .../gcc.target/mips/fix-loongson3-llsc.c  | 10 ++
  gcc/testsuite/gcc.target/mips/mips.exp|  1 +
  8 files changed, 58 insertions(+), 5 deletions(-)
  create mode 100644 gcc/testsuite/gcc.target/mips/fix-loongson3-llsc.c

Re: [PATCH] [MIPS] Fix PR target/91769

[Dragan Mladjenovic]

CC: YunQiang Su

On 25.09.2019. 09:16, Dragan Mladjenovic wrote:
> From: "Dragan Mladjenovic" 
>
> This fixes the issue by checking that addr's base reg is not part of dest
> multiword reg instead just checking the first reg of dest.
>
> gcc/ChangeLog:
>
> 2019-09-25  Dragan Mladjenovic  
>
>   PR target/91769
>   * config/mips/mips.c (mips_split_move): Use reg_overlap_mentioned_p
>   instead of REGNO equality check on addr.reg.
>
> gcc/testsuite/ChangeLog:
>
> 2019-09-25  Dragan Mladjenovic  
>
>   PR target/91769
>   * gcc.target/mips/pr91769.c: New test.
> ---
>
> Hi all,
>
> Is this OK for trunk?
>
> The test case is a bit crude, but I guess that is better than having none.
>
> On top of that, I would like to backport this along with r273174 onto gcc 9 
> branch.
> That should fix BZ91702 and BZ91474 reported against gcc 9.2.
>
> Tested on mips-mti-linux-gnu.
>
> Best regards,
> Dragan
>
>   gcc/config/mips/mips.c  |  2 +-
>   gcc/testsuite/gcc.target/mips/pr91769.c | 19 +++
>   2 files changed, 20 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/gcc.target/mips/pr91769.c
>
> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
> index c682ebd..aa527b4 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -4862,7 +4862,7 @@ mips_split_move (rtx dest, rtx src, enum 
> mips_split_type split_type, rtx insn_)
>   {
> rtx tmp = XEXP (src, 0);
> mips_classify_address (, tmp, GET_MODE (tmp), true);
> -   if (addr.reg && REGNO (addr.reg) != REGNO (dest))
> +   if (addr.reg && !reg_overlap_mentioned_p (dest, addr.reg))
>   validate_change (next, _SRC (set), src, false);
>   }
> else
> diff --git a/gcc/testsuite/gcc.target/mips/pr91769.c 
> b/gcc/testsuite/gcc.target/mips/pr91769.c
> new file mode 100644
> index 000..b856183
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/mips/pr91769.c
> @@ -0,0 +1,19 @@
> +/* PR target/91769 */
> +/* { dg-do compile } */
> +/* { dg-skip-if "naming registers makes this a code quality test" { *-*-* } 
> { "-O0" "-g" } { "" } } */
> +/* { dg-options "-EL -mgp32 -mhard-float" } */
> +
> +NOCOMPRESSION double
> +foo (void)
> +{
> +  register double* pf __asm__ ("$a1");
> +  __asm__ __volatile__ ("":"=r"(pf));
> +  double f = *pf;
> +
> +  if (f != f)
> +f = -f;
> +  return f;
> +}
> +
> +/* { dg-final { scan-assembler-not 
> "lw\t\\\$4,0\\(\\\$5\\)\n\tlw\t\\\$5,4\\(\\\$5\\)\n\tldc1\t\\\$.*,0\\(\\\$5\\)"
>  } } */
> +/* { dg-final { scan-assembler 
> "lw\t\\\$4,0\\(\\\$5\\)\n\tlw\t\\\$5,4\\(\\\$5\\)\n\tmtc1\t\\\$4,\\\$.*\n\tmthc1\t\\\$5,\\\$.*"
>  } } */
>

[PATCH] Do not check call type compatibility when cloning cgraph edges

[Martin Jambor]

Hi,

when looking into PR 70929 and thus looking at what
gimple_check_call_matching_types and gimple_check_call_args (both in
cgraph.c) do, I noticed that they get invoked a lot when cloning cgraph
edges when code is being copied as part of inlining transformation, only
to have their result overwritten by the value from the original edge.
(They also fail a lot during that time because since call redirection has
not taken place yet, the arguments are actually expected not to match.)

The following patch avoids that by adding a simple parameter to various
create_edge methods which indicate that we are cloning and the call
statement should therefore not be examined.  For consistency reasons I
unfortunately also had to change the meaning of the last parameter of
create_indirect_edge but there is only one place where it is called with
value of the parameter specified and its intent has always been to
behave differently when cloning.

Bootstrapped and tested on x86_64-linux.  OK for trunk?

Thanks,

Martin



2019-09-27  Martin Jambor  

* cgraph.c (symbol_table::create_edge): New parameter cloning_p,
do not compute some stuff when set.
(cgraph_node::create_edge): Likewise.
(cgraph_node::create_indirect_edge): Renamed last parameter to
coning_p and flipped its meaning, don't even calculate
inline_failed when set.
* cgraph.h (cgraph_node::create_edge): Add new parameter.
(symbol_tablecreate_edge): Likewise.
(cgraph_node::create_indirect_edge): Rename last parameter, flip
the default value.
* cgraphclones.c (cgraph_edge::clone): Pass true cloning_p to all
call graph edge creating functions.
---
 gcc/cgraph.c   | 48 --
 gcc/cgraph.h   | 12 +++-
 gcc/cgraphclones.c |  6 +++---
 3 files changed, 39 insertions(+), 27 deletions(-)

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 331b363c175..c9eb565a9dd 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -824,12 +824,13 @@ cgraph_edge::set_call_stmt (gcall *new_stmt, bool 
update_speculative)
 
 /* Allocate a cgraph_edge structure and fill it with data according to the
parameters of which only CALLEE can be NULL (when creating an indirect call
-   edge).  */
+   edge).  CLONING_P should be set if properties that are copied from an
+   original edge should not be calculated.  */
 
 cgraph_edge *
 symbol_table::create_edge (cgraph_node *caller, cgraph_node *callee,
   gcall *call_stmt, profile_count count,
-  bool indir_unknown_callee)
+  bool indir_unknown_callee, bool cloning_p)
 {
   cgraph_edge *edge;
 
@@ -862,8 +863,17 @@ symbol_table::create_edge (cgraph_node *caller, 
cgraph_node *callee,
   edge->lto_stmt_uid = 0;
 
   edge->count = count;
-
   edge->call_stmt = call_stmt;
+  edge->indirect_info = NULL;
+  edge->indirect_inlining_edge = 0;
+  edge->speculative = false;
+  edge->indirect_unknown_callee = indir_unknown_callee;
+  if (call_stmt && caller->call_site_hash)
+cgraph_add_edge_to_call_site_hash (edge);
+
+  if (cloning_p)
+return edge;
+
   edge->can_throw_external
 = call_stmt ? stmt_can_throw_external (DECL_STRUCT_FUNCTION (caller->decl),
   call_stmt) : false;
@@ -881,10 +891,6 @@ symbol_table::create_edge (cgraph_node *caller, 
cgraph_node *callee,
   edge->call_stmt_cannot_inline_p = false;
 }
 
-  edge->indirect_info = NULL;
-  edge->indirect_inlining_edge = 0;
-  edge->speculative = false;
-  edge->indirect_unknown_callee = indir_unknown_callee;
   if (opt_for_fn (edge->caller->decl, flag_devirtualize)
   && call_stmt && DECL_STRUCT_FUNCTION (caller->decl))
 edge->in_polymorphic_cdtor
@@ -892,22 +898,23 @@ symbol_table::create_edge (cgraph_node *caller, 
cgraph_node *callee,
  caller->decl);
   else
 edge->in_polymorphic_cdtor = caller->thunk.thunk_p;
-  if (call_stmt && caller->call_site_hash)
-cgraph_add_edge_to_call_site_hash (edge);
 
   return edge;
 }
 
-/* Create edge from a given function to CALLEE in the cgraph.  */
+/* Create edge from a given function to CALLEE in the cgraph.  CLONING_P should
+   be set if properties that are copied from an original edge should not be
+   calculated.  */
 
 cgraph_edge *
 cgraph_node::create_edge (cgraph_node *callee,
- gcall *call_stmt, profile_count count)
+ gcall *call_stmt, profile_count count, bool cloning_p)
 {
   cgraph_edge *edge = symtab->create_edge (this, callee, call_stmt, count,
-  false);
+  false, cloning_p);
 
-  initialize_inline_failed (edge);
+  if (!cloning_p)
+initialize_inline_failed (edge);
 
   edge->next_caller = callee->callers;
   if (callee->callers)
@@ -935,25 +942,28 @@ cgraph_allocate_init_indirect_info 

Re: [01/32] Add function_abi.{h,cc}

[Richard Sandiford]

Hi Jeff,

Thanks as always for the reviews :-)

Jeff Law  writes:
> On 9/11/19 1:03 PM, Richard Sandiford wrote:
>> This patch adds new structures and functions for handling
>> multiple ABIs in a translation unit.  The structures are:
>> 
>> - predefined_function_abi: describes a static, predefined ABI
>> - function_abi: describes either a predefined ABI or a local
>>   variant of one (e.g. taking -fipa-ra into account)
>> 
>> The patch adds functions for getting the ABI from a given type
>> or decl; a later patch will also add a function for getting the
>> ABI of the target of a call insn.
>> 
>> Although ABIs are about much more than call-clobber/saved choices,
>> I wanted to keep the name general in case we add more ABI-related
>> information in future.
>> 
>> 
>> 2019-09-11  Richard Sandiford  
>> 
>> gcc/
>>  * Makefile.in (OBJS): Add function-abi.o.
>>  (GTFILES): Add function-abi.h.
>>  * function-abi.cc: New file.
>>  * function-abi.h: Likewise.
>>  * emit-rtl.h (rtl_data::abi): New field.
>>  * function.c: Include function-abi.h.
>>  (prepare_function_start): Initialize crtl->abi.
>>  * read-rtl-function.c: Include regs.h and function-abi.h.
>>  (read_rtl_function_body): Initialize crtl->abi.
>>  (read_rtl_function_body_from_file_range): Likewise.
>>  * reginfo.c: Include function-abi.h.
>>  (init_reg_sets_1): Initialize default_function_abi.
>>  (globalize_reg): Call add_full_reg_clobber for each predefined ABI
>>  when making a register global.
>>  * target-globals.h (this_target_function_abi_info): Declare.
>>  (target_globals::function_abi_info): New field.
>>  (restore_target_globals): Copy it.
>>  * target-globals.c: Include function-abi.h.
>>  (default_target_globals): Initialize the function_abi_info field.
>>  (target_globals): Allocate it.
>>  (save_target_globals): Free it.
> So no problem with this as-is.  Questions though:
>
> 1. Do we need to stream this information for LTO?

At the moment this is all derived information rather than something we need
to stream directly.  E.g. the set of available predefined_function_abis
really only depends on command-line flags.  The mapping from functions
to predefined_function_abis currently depends only on the function type,
so streaming the type is enough to recover the ABI too.  function_abi
additionally depends on RTL stuff that doesn't affect LTO.

> 2. Do we need to support it for the RTL front-end, even if primarily for
> testing purposes?

Yeah, I guess it could be useful to be able to pretend that a function
is defined locally with a certain -fipa-ra clobber set, but I think in
most cases it'd be possible to do this using:

  void __attribute__ ((noinline, noclone))
  callee (void)
  {
asm ("" ::: ...regs...);
  }

(That's the kind of test I've used in the support for the SVE PCS FWIW,
not posted yet.)

Richard

Re: [PATCH] IPA-CP release transformation summary (PR jit/91928)

[Martin Jambor]

Hi,

On Mon, Sep 30 2019, Andrea Corallo wrote:
> Hi all,
> I'd like to submit this patch.
> It release the ipa cp transformation summary after functions being expanded.
> This is to fix the compiler when used with libgccjit on subsequent
> compilations (every new compilation should have a clean transformation
> summary).

if this is a general problem then I think we should instead add another
hook to class ipa_opt_pass_d to free transformation summary, call it for
all IPA passes at the appropriate time and implement it for IPA-CP. That
way it will work for all IPA passes which might have a transformation
summary.

Martin


>
> Bootstrap on arm64 and X86-64.
>
> Bests
>   Andrea
>
> gcc/ChangeLog
> 2019-??-??  Andrea Corallo  
>
>   * cgraphunit.c (expand_all_functions): Release ipcp_transformation_sum
>   when finished.
>   * ipa-prop.c (ipcp_free_transformation_sum): New function.
>   * ipa-prop.h (ipcp_free_transformation_sum): Add declaration.

Re: [PATCH][gcc] libgccjit: handle long literals in playback::context::new_string_literal

[Andrea Corallo]

Andrea Corallo writes:

> Andrea Corallo writes:
>
>> Hi all,
>> yesterday I've found an interesting bug in libgccjit.
>> Seems we have an hard limitation of 200 characters for literal strings.
>> Attempting to create longer strings lead to ICE during pass_expand
>> while performing a sanity check in get_constant_size.
>>
>> Tracking down the issue seems the code we have was inspired from
>> c-family/c-common.c:c_common_nodes_and_builtins were array_domain_type
>> is actually defined with a size of 200.
>> The comment that follows that point sounded premonitory :) :)
>>
>> /* Make a type for arrays of characters.
>>With luck nothing will ever really depend on the length of this
>>array type.  */
>>
>> At least in the current implementation the type is set by
>> fix_string_type were the actual string length is taken in account.
>>
>> I attach a patch updating the logic accordingly and a new testcase
>>  for that.
>>
>> make check-jit is passing clean.
>>
>> Best Regards
>>   Andrea
>>
>> gcc/jit/ChangeLog
>> 2019-??-??  Andrea Corallo  
>>
>>  * jit-playback.h
>>  (gcc::jit::recording::context m_recording_ctxt): Remove
>>  m_char_array_type_node field.
>>  * jit-playback.c
>>  (playback::context::context) Remove m_char_array_type_node from member
>>  initializer list.
>>  (playback::context::new_string_literal) Fix logic to handle string
>>  length > 200.
>>
>> gcc/testsuite/ChangeLog
>> 2019-??-??  Andrea Corallo  
>>
>>  * jit.dg/all-non-failing-tests.h: Add test-long-string-literal.c.
>>  * jit.dg/test-long-string-literal.c: New testcase.
>> diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
>> index 9eeb2a7..a26b8d3 100644
>> --- a/gcc/jit/jit-playback.c
>> +++ b/gcc/jit/jit-playback.c
>> @@ -88,7 +88,6 @@ playback::context::context (recording::context *ctxt)
>>: log_user (ctxt->get_logger ()),
>>  m_recording_ctxt (ctxt),
>>  m_tempdir (NULL),
>> -m_char_array_type_node (NULL),
>>  m_const_char_ptr (NULL)
>>  {
>>JIT_LOG_SCOPE (get_logger ());
>> @@ -670,9 +669,12 @@ playback::rvalue *
>>  playback::context::
>>  new_string_literal (const char *value)
>>  {
>> -  tree t_str = build_string (strlen (value), value);
>> -  gcc_assert (m_char_array_type_node);
>> -  TREE_TYPE (t_str) = m_char_array_type_node;
>> +  /* Compare with c-family/c-common.c: fix_string_type.  */
>> +  size_t len = strlen (value);
>> +  tree i_type = build_index_type (size_int (len));
>> +  tree a_type = build_array_type (char_type_node, i_type);
>> +  tree t_str = build_string (len, value);
>> +  TREE_TYPE (t_str) = a_type;
>>
>>/* Convert to (const char*), loosely based on
>>   c/c-typeck.c: array_to_pointer_conversion,
>> @@ -2703,10 +2705,6 @@ playback::context::
>>  replay ()
>>  {
>>JIT_LOG_SCOPE (get_logger ());
>> -  /* Adapted from c-common.c:c_common_nodes_and_builtins.  */
>> -  tree array_domain_type = build_index_type (size_int (200));
>> -  m_char_array_type_node
>> -= build_array_type (char_type_node, array_domain_type);
>>
>>m_const_char_ptr
>>  = build_pointer_type (build_qualified_type (char_type_node,
>> diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
>> index d4b148e..801f610 100644
>> --- a/gcc/jit/jit-playback.h
>> +++ b/gcc/jit/jit-playback.h
>> @@ -322,7 +322,6 @@ private:
>>
>>auto_vec m_functions;
>>auto_vec m_globals;
>> -  tree m_char_array_type_node;
>>tree m_const_char_ptr;
>>
>>/* Source location handling.  */
>> diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h 
>> b/gcc/testsuite/jit.dg/all-non-failing-tests.h
>> index 0272e6f8..1b3d561 100644
>> --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
>> +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
>> @@ -220,6 +220,13 @@
>>  #undef create_code
>>  #undef verify_code
>>
>> +/* test-long-string-literal.c */
>> +#define create_code create_code_long_string_literal
>> +#define verify_code verify_code_long_string_literal
>> +#include "test-long-string-literal.c"
>> +#undef create_code
>> +#undef verify_code
>> +
>>  /* test-sum-of-squares.c */
>>  #define create_code create_code_sum_of_squares
>>  #define verify_code verify_code_sum_of_squares
>> diff --git a/gcc/testsuite/jit.dg/test-long-string-literal.c 
>> b/gcc/testsuite/jit.dg/test-long-string-literal.c
>> new file mode 100644
>> index 000..882567c
>> --- /dev/null
>> +++ b/gcc/testsuite/jit.dg/test-long-string-literal.c
>> @@ -0,0 +1,48 @@
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "libgccjit.h"
>> +
>> +#include "harness.h"
>> +
>> +const char very_long_string[] =
>> +  
>> "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"
>> +  
>> "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"
>> +  
>> "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"
>> +  "abcabcabcabcabcabcabcabcabcabca";
>> +
>> +void
>> +create_code (gcc_jit_context *ctxt, void 

[PATCH] IPA-CP release transformation summary (PR jit/91928)

[Andrea Corallo]

Hi all,
I'd like to submit this patch.
It release the ipa cp transformation summary after functions being expanded.
This is to fix the compiler when used with libgccjit on subsequent
compilations (every new compilation should have a clean transformation
summary).

Bootstrap on arm64 and X86-64.

Bests
  Andrea

gcc/ChangeLog
2019-??-??  Andrea Corallo  

* cgraphunit.c (expand_all_functions): Release ipcp_transformation_sum
when finished.
* ipa-prop.c (ipcp_free_transformation_sum): New function.
* ipa-prop.h (ipcp_free_transformation_sum): Add declaration.
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index cb08efe..0251fa4 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -2342,6 +2342,7 @@ expand_all_functions (void)
  profiled_func_count, expanded_func_count);
 
   symtab->process_new_functions ();
+  ipcp_free_transformation_sum ();
   free_gimplify_stack ();
 
   free (order);
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index 30948fb..0ff8085 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -561,6 +561,7 @@ struct GTY(()) ipcp_transformation
 void ipa_set_node_agg_value_chain (struct cgraph_node *node,
    struct ipa_agg_replacement_value *aggvals);
 void ipcp_transformation_initialize (void);
+void ipcp_free_transformation_sum (void);
 
 /* ipa_edge_args stores information related to a callsite and particularly its
arguments.  It can be accessed by the IPA_EDGE_REF macro.  */
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 2f2b070..158dbe7 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -3758,6 +3758,18 @@ ipcp_transformation_initialize (void)
 ipcp_transformation_sum = ipcp_transformation_t::create_ggc (symtab);
 }
 
+/* Release the IPA CP transformation summary.  */
+
+void
+ipcp_free_transformation_sum (void)
+{
+  if (!ipcp_transformation_sum)
+return;
+
+  ipcp_transformation_sum->release ();
+  ipcp_transformation_sum = NULL;
+}
+
 /* Set the aggregate replacements of NODE to be AGGVALS.  */
 
 void

Re: [PATCH] Fix algo constexpr tests in Debug mode

[Jonathan Wakely]

On 28/09/19 23:12 +0200, François Dumont wrote:

Here is what I just commited.


Thanks. In my branch I had a lot more _GLIBCXX20_CONSTEXPR additions
in the debug mode headers. Is it not needed on __check_singular,
__foreign_iterator etc. ?

I try to use the asm trick in the _GLIBCXX_DEBUG_VERIFY_COND_AT but 
didn't notice any enhancement. So for now I kept my solution to just 
have a non-constexpr call compiler error.


You won't see any improvement in the testsuite, because the compiler
flags for the testsuite suppress diagnostic notes. But I'd expect it
to give better output for users with the default compiler flags.



diff --git a/libstdc++-v3/include/bits/stl_algo.h 
b/libstdc++-v3/include/bits/stl_algo.h
index a672f8b2b39..f25b8b76df6 100644
--- a/libstdc++-v3/include/bits/stl_algo.h
+++ b/libstdc++-v3/include/bits/stl_algo.h
@@ -5054,8 +5054,8 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
   *  @param  __last1   Another iterator.
   *  @param  __last2   Another iterator.
   *  @param  __result  An iterator pointing to the end of the merged range.
-   *  @return An iterator pointing to the first element not less
-   *  than @e val.
+   *  @return   An output iterator equal to @p __result + (__last1 - __first1)
+   *+ (__last2 - __first2).
   *
   *  Merges the ranges @p [__first1,__last1) and @p [__first2,__last2) into
   *  the sorted range @p [__result, __result + (__last1-__first1) +
@@ -5102,8 +5102,8 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
   *  @param  __last2   Another iterator.
   *  @param  __result  An iterator pointing to the end of the merged range.
   *  @param  __compA functor to use for comparisons.
-   *  @return An iterator pointing to the first element "not less
-   *  than" @e val.
+   *  @return   An output iterator equal to @p __result + (__last1 - __first1)
+   *+ (__last2 - __first2).
   *
   *  Merges the ranges @p [__first1,__last1) and @p [__first2,__last2) into
   *  the sorted range @p [__result, __result + (__last1-__first1) +


These changes are fine but should have been a separate, unrelated
commit.



@@ -157,10 +192,16 @@ namespace __gnu_debug
   *  otherwise.
  */
  template
+_GLIBCXX20_CONSTEXPR
inline bool
__valid_range(_InputIterator __first, _InputIterator __last,
  typename _Distance_traits<_InputIterator>::__type& __dist)
{
+#ifdef __cpp_lib_is_constant_evaluated
+  if (std::is_constant_evaluated())
+   // Detected by the compiler directly.
+   return true;
+#endif


Should this be using the built-in, not the C++20 function?

In practice it's probably equivalent, because the function is only
going to be constant-evaluated when called from C++20 code, and in
that case the std::is_constant_evaluated() function will be available.
It just seems inconsistent to use the built-in in one place and not in
the other.


  typedef typename std::__is_integer<_InputIterator>::__type _Integral;
  return __valid_range_aux(__first, __last, __dist, _Integral());
}
@@ -180,11 +221,17 @@ namespace __gnu_debug
#endif

  template
+_GLIBCXX_CONSTEXPR
inline bool
__valid_range(_InputIterator __first, _InputIterator __last)
{
-  typename _Distance_traits<_InputIterator>::__type __dist;
-  return __valid_range(__first, __last, __dist);
+#ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
+  if (__builtin_is_constant_evaluated())
+   // Detected by the compiler directly.
+   return true;
+#endif
+  typedef typename std::__is_integer<_InputIterator>::__type _Integral;
+  return __valid_range_aux(__first, __last, _Integral());
}

  template

Ping: [PATCH V4] Extend IPA-CP to support arithmetically-computed value-passing on by-ref argument (PR ipa/91682)

[Feng Xue OS]

Hi Honza & Martin,

   And also hope your comments on this patch. Thanks.

Feng


From: Feng Xue OS 
Sent: Thursday, September 19, 2019 10:30 PM
To: Martin Jambor; Jan Hubicka; gcc-patches@gcc.gnu.org
Subject: [PATCH V4] Extend IPA-CP to support arithmetically-computed 
value-passing on by-ref argument (PR ipa/91682)

Fix a bug on unary/binary operation check.

Feng
---
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 33d52fe5537..f218f1093b8 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1244,23 +1244,23 @@ initialize_node_lattices (struct cgraph_node *node)
   }
 }

-/* Return the result of a (possibly arithmetic) pass through jump function
-   JFUNC on the constant value INPUT.  RES_TYPE is the type of the parameter
-   to which the result is passed.  Return NULL_TREE if that cannot be
-   determined or be considered an interprocedural invariant.  */
+/* Return the result of a (possibly arithmetic) operation on the constant
+   value INPUT.  OPERAND is 2nd operand for binary operation.  RES_TYPE is
+   the type of the parameter to which the result is passed.  Return
+   NULL_TREE if that cannot be determined or be considered an
+   interprocedural invariant.  */

 static tree
-ipa_get_jf_pass_through_result (struct ipa_jump_func *jfunc, tree input,
-   tree res_type)
+ipa_get_jf_arith_result (enum tree_code opcode, tree input, tree operand,
+tree res_type)
 {
   tree res;

-  if (ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR)
+  if (opcode == NOP_EXPR)
 return input;
   if (!is_gimple_ip_invariant (input))
 return NULL_TREE;

-  tree_code opcode = ipa_get_jf_pass_through_operation (jfunc);
   if (!res_type)
 {
   if (TREE_CODE_CLASS (opcode) == tcc_comparison)
@@ -1274,8 +1274,7 @@ ipa_get_jf_pass_through_result (struct ipa_jump_func 
*jfunc, tree input,
   if (TREE_CODE_CLASS (opcode) == tcc_unary)
 res = fold_unary (opcode, res_type, input);
   else
-res = fold_binary (opcode, res_type, input,
-  ipa_get_jf_pass_through_operand (jfunc));
+res = fold_binary (opcode, res_type, input, operand);

   if (res && !is_gimple_ip_invariant (res))
 return NULL_TREE;
@@ -1283,6 +1282,21 @@ ipa_get_jf_pass_through_result (struct ipa_jump_func 
*jfunc, tree input,
   return res;
 }

+/* Return the result of a (possibly arithmetic) pass through jump function
+   JFUNC on the constant value INPUT.  RES_TYPE is the type of the parameter
+   to which the result is passed.  Return NULL_TREE if that cannot be
+   determined or be considered an interprocedural invariant.  */
+
+static tree
+ipa_get_jf_pass_through_result (struct ipa_jump_func *jfunc, tree input,
+   tree res_type)
+{
+  return ipa_get_jf_arith_result (ipa_get_jf_pass_through_operation (jfunc),
+ input,
+ ipa_get_jf_pass_through_operand (jfunc),
+ res_type);
+}
+
 /* Return the result of an ancestor jump function JFUNC on the constant value
INPUT.  Return NULL_TREE if that cannot be determined.  */

@@ -1416,6 +1430,146 @@ ipa_context_from_jfunc (ipa_node_params *info, 
cgraph_edge *cs, int csidx,
   return ctx;
 }

+/* See if NODE is a clone with a known aggregate value at a given OFFSET of a
+   parameter with the given INDEX.  */
+
+static tree
+get_clone_agg_value (struct cgraph_node *node, HOST_WIDE_INT offset,
+int index)
+{
+  struct ipa_agg_replacement_value *aggval;
+
+  aggval = ipa_get_agg_replacements_for_node (node);
+  while (aggval)
+{
+  if (aggval->offset == offset
+ && aggval->index == index)
+   return aggval->value;
+  aggval = aggval->next;
+}
+  return NULL_TREE;
+}
+
+/* Determine whether ITEM, jump function for an aggregate part, evaluates to a
+   single known constant value and if so, return it.  Otherwise return NULL.
+   NODE and INFO describes the caller node or the one it is inlined to, and
+   its related info.  */
+
+static tree
+ipa_agg_value_from_node (class ipa_node_params *info,
+struct cgraph_node *node,
+struct ipa_agg_jf_item *item)
+{
+  tree value = NULL_TREE;
+  int src_idx;
+
+  if (item->offset < 0 || item->jftype == IPA_JF_UNKNOWN)
+return NULL_TREE;
+
+  if (item->jftype == IPA_JF_CONST)
+return item->value.constant;
+
+  gcc_checking_assert (item->jftype == IPA_JF_PASS_THROUGH
+  || item->jftype == IPA_JF_LOAD_AGG);
+
+  src_idx = item->value.pass_through.formal_id;
+
+  if (info->ipcp_orig_node)
+{
+  if (item->jftype == IPA_JF_PASS_THROUGH)
+   value = info->known_csts[src_idx];
+  else
+   value = get_clone_agg_value (node, item->value.load_agg.offset,
+src_idx);
+}
+  else if (info->lattices)
+{
+  class ipcp_param_lattices *src_plats
+   

Ping: [PATCH V4] Generalized predicate/condition for parameter reference in IPA (PR ipa/91088)

[Feng Xue OS]

Hi, Honza & Martin,

  Would you please take some time to review this updated patch? Thanks.

Feng


From: Feng Xue OS 
Sent: Wednesday, September 18, 2019 8:41 PM
To: Jan Hubicka
Cc: Martin Jambor; gcc-patches@gcc.gnu.org
Subject: [PATCH V4] Generalized predicate/condition for parameter reference in 
IPA (PR ipa/91088)

>> +  if (unmodified_parm_or_parm_agg_item (fbi, stmt, expr, index_p, ,
>> + aggpos))
>> + {
>> +  tree type = TREE_TYPE (expr);
>> +
>> +   /* Stop if found bit-field whose size does not equal any natural
>> +  integral type.  */
>> +   if (maybe_ne (tree_to_poly_int64 (TYPE_SIZE (type)), size))
>> + break;

> Why is this needed?
This is to exclude bit-field reference. Actually, the restrict is not required, 
a TODO to remove it. Now I directly check bit-field attribute.

>> - = add_condition (summary, index, size, , this_code,
>> -  unshare_expr_without_location
>> -  (gimple_cond_rhs (last)));
>> + = add_condition (summary, index, type, , this_code,
>> +  gimple_cond_rhs (last), param_ops);

> An why unsharing is no longer needed here?
> It is importnat to avoid anything which reffers to function local blocks
> to get into the global LTO stream.
Do unsharing inside add_condition, since constants in param_ops also need to be 
unshared.

>> +   if (op1.code != op2.code
>> +   || op1.val_is_rhs != op2.val_is_rhs
>> +   || !vrp_operand_equal_p (op1.val, op2.val)

> Why you need vrp_operand_equal_p instead of operand_equal_p here?
op1.val and op2.val might be NULL_TREE.

> Overall the patch looks very reasonable. We may end up with bit more
> general expressions (i.e. supporting arithmetics involving more than one
> operand).
If you means more than one constant operand, I'v changed the patch to support 
ternary operation.

And if you means more than one parameter operand, this will involve much more 
code change in ipa-fnsummary, it's better to let it be another TODO.

> I see you also fixed a lot of typos in comments, please go head and
> commit them separately as obvious.
Removed.

Thank for your comments.
Feng
---
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 0e3693598e7..05b1bb97c6b 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -11943,6 +11943,13 @@ For switch exceeding this limit, IPA-CP will not 
construct cloning cost
 predicate, which is used to estimate cloning benefit, for default case
 of the switch statement.

+@item ipa-max-param-expr-ops
+IPA-CP will analyze conditional statement that references some function
+parameter to estimate benefit for cloning upon certain constant value.
+But if number of operations in a parameter expression exceeds
+@option{ipa-max-param-expr-ops}, the expression is treated as complicated
+one, and is not handled by IPA analysis.
+
 @item lto-partitions
 Specify desired number of partitions produced during WHOPR compilation.
 The number of partitions should exceed the number of CPUs used for compilation.
diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c
index 1bf1806eaf8..c25e3395f59 100644
--- a/gcc/ipa-fnsummary.c
+++ b/gcc/ipa-fnsummary.c
@@ -331,6 +331,8 @@ evaluate_conditions_for_known_args (struct cgraph_node 
*node,
 {
   tree val;
   tree res;
+  int j;
+  struct expr_eval_op *op;

   /* We allow call stmt to have fewer arguments than the callee function
  (especially for K style programs).  So bound check here (we assume
@@ -382,7 +384,7 @@ evaluate_conditions_for_known_args (struct cgraph_node 
*node,
  continue;
}

-  if (maybe_ne (tree_to_poly_int64 (TYPE_SIZE (TREE_TYPE (val))), c->size))
+  if (TYPE_SIZE (c->type) != TYPE_SIZE (TREE_TYPE (val)))
{
  clause |= 1 << (i + predicate::first_dynamic_condition);
  nonspec_clause |= 1 << (i + predicate::first_dynamic_condition);
@@ -394,7 +396,30 @@ evaluate_conditions_for_known_args (struct cgraph_node 
*node,
  continue;
}

-  val = fold_unary (VIEW_CONVERT_EXPR, TREE_TYPE (c->val), val);
+  val = fold_unary (VIEW_CONVERT_EXPR, c->type, val);
+  for (j = 0; vec_safe_iterate (c->param_ops, j, ); j++)
+   {
+ if (!val)
+   break;
+ if (!op->val[0])
+   val = fold_unary (op->code, op->type, val);
+ else if (!op->val[1])
+   val = fold_binary (op->code, op->type,
+  op->index ? op->val[0] : val,
+  op->index ? val : op->val[0]);
+ else if (op->index == 0)
+   val = fold_ternary (op->code, op->type,
+   val, op->val[0], op->val[1]);
+ else if (op->index == 1)
+   val = fold_ternary (op->code, op->type,
+   op->val[0], 

Re: [RFC] Come up with ipa passes introduction in gccint documentation

[Martin Jambor]

Hi,

I agree that we should have a chapter on IPA passes in gccint, just
please note that...

On Sun, Sep 29 2019, Xiong Hu Luo wrote:
> There is no ipa passes introduction in gccint now, is it nessessary to
> add this part as both GIMPLE passes and RTL passes breif intruduction
> already exit in Chapter 9 "Passes and Files of the Compiler" but no
> section for ipa passes?
> If it's OK, this is just a framework, lots of words need be filled into
> each item.
>
> ChangeLog:
>   * doc/lto.texi (IPA): Reference to the IPA passes.
>   * doc/passes.texi (Tree SSA passes): Add node IPA passes.
> ---
>  gcc/doc/lto.texi|  6 ++---
>  gcc/doc/passes.texi | 62 +
>  2 files changed, 65 insertions(+), 3 deletions(-)
>

...

> diff --git a/gcc/doc/passes.texi b/gcc/doc/passes.texi
> index 6edb9a0bfb7..0b6cf73469c 100644
> --- a/gcc/doc/passes.texi
> +++ b/gcc/doc/passes.texi
> @@ -21,6 +21,7 @@ where near complete.
>  * Gimplification pass::  The bits are turned into something we can optimize.
>  * Pass manager:: Sequencing the optimization passes.
>  * Tree SSA passes::  Optimizations on a high-level representation.
> +* IPA passes::   Optimizations on scope of intra-procedual.

...IPA stands for inter (not intra!) procedural analysis.

Martin

Re: [PATCH] FreeBSD PowerPC use secure-plt

[Andreas Tobler]

On 26.09.19 08:13, Andreas Tobler wrote:

Hi Segher,

On 26.09.19 00:49, Segher Boessenkool wrote:


On Wed, Sep 25, 2019 at 10:46:57PM +0200, Andreas Tobler wrote:

--- gcc/config/rs6000/t-freebsd64   (revision 276090)
+++ gcc/config/rs6000/t-freebsd64   (working copy)
@@ -27,3 +27,6 @@
   MULTILIB_EXCEPTIONS =
   MULTILIB_OSDIRNAMES  = ../lib32
   
+SECURE_PLT = $(if $(findstring TARGET_FREEBSD32_SECURE_PLT=1, $(tm_defines)),msecure-plt)

+
+MULTILIB_EXTRA_OPTS += $(SECURE_PLT)


$(findstring) isn't super great, it looks for substrings, so it would
also match "TARGET_FREEBSD32_SECURE_PLT=123"; you can use $(filter) instead?


Thank you for the feedback. Indeed, filter looks better. Testing again :)

And here for completeness the CL.

2019-09-25  Andreas Tobler  

* config.gcc: Use the secure-plt on FreeBSD 13 and upwards for
32-bit PowerPC.
Define TARGET_FREEBSD32_SECURE_PLT for 64-bit PowerPC.
* config/rs6000/t-freebsd64: Make use of the above define and build
the 32-bit libraries with secure-plt.


Looks fine to me either way.


Committed to trunk.

Andreas

Re: [WIP PATCH] add object access attributes (PR 83859)

[Richard Biener]

On Sun, Sep 29, 2019 at 9:52 PM Martin Sebor  wrote:
>
> -Wstringop-overflow detects a subset of past-the-end read and write
> accesses by built-in functions such as memcpy and strcpy.  It relies
> on the functions' effects the knowledge of which is hardwired into
> GCC.  Although it's possible for users to create wrappers for their
> own functions to detect similar problems, it's quite cumbersome and
> so only lightly used outside system libraries like Glibc.  Even Glibc
> only checks for buffer overflow and not for reading past the end.
>
> PR 83859 asks to expose the same checking that GCC does natively for
> built-in calls via a function attribute that associates a pointer
> argument with the size argument, such as:
>
>__attribute__((buffer_size (1, 2))) void
>f (char* dst, size_t dstsize);
>
> The attached patch is my initial stab at providing this feature by
> introducing three new attributes:
>
>* read_only (ptr-argno, size-argno)
>* read_only (ptr-argno, size-argno)
>* read_write (ptr-argno, size-argno)
>
> As requested, the attributes associate a pointer parameter to
> a function with a size parameter.  In addition, they also specify
> how the function accesses the object the pointer points to: either
> it only reads from it, or it only writes to it, or it does both.
>
> Besides enabling the same buffer overflow detection as for built-in
> string functions they also let GCC issue -Wuninitialized warnings
> for uninitialized objects passed to read-only functions by reference,
> and -Wunused-but-set warnings for objects passed to write-only
> functions that are otherwise unused (PR 80806).  The -Wununitialized
> part is done. The -Wunused-but-set detection is implemented only in
> the C FE and not yet in C++.
>
> Besides the diagnostic improvements above the attributes also open
> up optimization opportunities such as DCE.  I'm still working on this
> and so it's not yet part of the initial patch.

There's the "fn spec" attribute which you can use for the optimization
part.  Note "fn spec" also likes to know whether the address of the
argument escapes and whether the argument is only dereferenced
directly or also indirectly (when passing a pointer to a struct is
transitively reachable memory through the pointer accessed or not?).

So you should at least make sure to document the full
semantics of your proposed read_only/write_only/read_write atributes.

I guess that "read_only" means that direct accesses do not write
but the attribute does not constrain indirect accesses?

Note "fn spec" doesn't offer read/write constraints when not at the
same time constraining escaping since when a pointer escapes
through a call we cannot make any optimization for a positional
read/write constraint since a function can access global memory
(and all escaped pointed-to data ends up as "global memory").
There's no way to tell (via "fn spec") that the function only accesses
memory reachable via function arguments.

So I'm not sure there's a 1:1 mapping for your desired semantics
to "fn spec" plus your desired semantics may offer no
opportunity for optimization.  Useful would be if read_only
would map to "R" and read_write and write_only would map to "W".

Richard.

> I plan to finish the patch for GCC 10 but I don't expect to have
> the time to start taking advantage of the attributes for optimization
> until GCC 11.
>
> Besides regression testing on x86_64-linux, I also tested the patch
> by compiling Binutils/GDB, Glibc, and the Linux kernel with it.  It
> found no new problems but caused a handful of -Wunused-but-set-variable
> false positives due to an outstanding bug in the C front-end introduced
> by the patch that I still need to fix.
>
> Martin

Re: [PATCH] libgomp_g.h: Include stdint.h instead of gstdint.h

[Jakub Jelinek]

On Mon, Sep 30, 2019 at 12:03:00AM -0700, Frederik Harwath wrote:
> Hi,
> I am a new member of Mentor's Sourcery Tools Services group and this is the 
> first patch that I am submitting here.
> I do not have write access to the svn repository yet, hence someone would 
> have to merge this patch for me if it gets accepted.
> But I intend to apply for an account soon.
> 
> The patch changes libgomp/libgomp_g.h to include stdint.h instead of the 
> internal gstdint.h. The inclusion of gstdint.h has been
> introduced by GCC trunk r265930, presumably because this introduced uses of 
> uintptr_t. Since gstdint.h is not part of GCC's
> installation, several libgomp test cases fail to compile when running the 
> tests with the installed GCC.
> 
> I have tested the patch with "make check" on x86_64 GNU/Linux.

That looks wrong, will make libgomp less portable.
If anything, it should be something like:
#ifndef GCC_GENERATED_STDINT_H
#include 
#endif
so that if gstdint.h is available and has been included by libgomp.h
already, it doesn't try to include it.

Jakub

Re: [PATCH] Fix up ix86_expand_adjust_ufix_to_sfix_si ICE (PR target/91931)

[Uros Bizjak]

On Mon, Sep 30, 2019 at 9:01 AM Jakub Jelinek  wrote:
>
> Hi!
>
> The recent SUBREG simplification changes aren't as forgiving in the past and
> ICE on invalid constants, in this case CONST_VECTOR:V8SI with 8x CONST_INT
> 0x8000 element, which is invalid, because for SImode it needs to be sign
> extended.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
>
> 2019-09-30  Jakub Jelinek  
>
> PR target/91931
> * config/i386/i386-expand.c (ix86_expand_adjust_ufix_to_sfix_si): Use
> gen_int_mode instead of GEN_INT.
>
> * gcc.target/i386/pr91931.c: New test.

OK.

Thanks,
Uros.

> --- gcc/config/i386/i386-expand.c.jj2019-09-10 20:08:44.931792487 +0200
> +++ gcc/config/i386/i386-expand.c   2019-09-29 13:19:47.122382234 +0200
> @@ -1694,7 +1694,7 @@ ix86_expand_adjust_ufix_to_sfix_si (rtx
>  OPTAB_DIRECT);
>else
>  {
> -  rtx two31 = GEN_INT (HOST_WIDE_INT_1U << 31);
> +  rtx two31 = gen_int_mode (HOST_WIDE_INT_1U << 31, SImode);
>two31 = ix86_build_const_vector (intmode, 1, two31);
>*xorp = expand_simple_binop (intmode, AND,
>gen_lowpart (intmode, tmp[0]),
> --- gcc/testsuite/gcc.target/i386/pr91931.c.jj  2019-09-29 13:22:58.502510721 
> +0200
> +++ gcc/testsuite/gcc.target/i386/pr91931.c 2019-09-29 13:23:36.099946605 
> +0200
> @@ -0,0 +1,5 @@
> +/* PR target/91931 */
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -ftree-loop-vectorize -mavx -mno-avx2" } */
> +
> +#include "../../gcc.dg/vect/pr32216.c"
>
> Jakub

[PATCH] libgomp_g.h: Include stdint.h instead of gstdint.h

[Frederik Harwath]

Hi,
I am a new member of Mentor's Sourcery Tools Services group and this is the 
first patch that I am submitting here.
I do not have write access to the svn repository yet, hence someone would have 
to merge this patch for me if it gets accepted.
But I intend to apply for an account soon.

The patch changes libgomp/libgomp_g.h to include stdint.h instead of the 
internal gstdint.h. The inclusion of gstdint.h has been
introduced by GCC trunk r265930, presumably because this introduced uses of 
uintptr_t. Since gstdint.h is not part of GCC's
installation, several libgomp test cases fail to compile when running the tests 
with the installed GCC.

I have tested the patch with "make check" on x86_64 GNU/Linux.

Best regards,
Frederik

libgomp/ChangeLog:

2019-09-25  Kwok Cheung Yeung  

 * libgomp_g.h: Include stdint.h instead of gstdint.h.

diff --git a/libgomp/libgomp_g.h b/libgomp/libgomp_g.h
index 32a9d8a..dfb55fb 100644
--- a/libgomp/libgomp_g.h
+++ b/libgomp/libgomp_g.h
@@ -31,7 +31,7 @@
 
 #include 
 #include 
-#include "gstdint.h"
+#include 
 
 /* barrier.c */