Re: [committed] analyzer: avoid using in malloc-1.c

[Martin Sebor]

On 1/30/20 5:15 PM, David Malcolm wrote:

This test assumes that memset and strlen have been marked with
__attribute__((nonnull)), which isn't necessarily the case for an
arbitrary .


I sometimes find it useful to run tests with a cross-compiler.  Those
that include standard library headers that GCC itself doesn't provide
fail to build, so I've been also avoiding including headers for this
reason (in addition to it causing problems due to the system headers
sometimes containing things that makes the tests unreliable).

Martin


This likely explains these failures:
   FAIL: gcc.dg/analyzer/malloc-1.c  (test for warnings, line 417)
   FAIL: gcc.dg/analyzer/malloc-1.c  (test for warnings, line 418)
   FAIL: gcc.dg/analyzer/malloc-1.c  (test for warnings, line 425)
   FAIL: gcc.dg/analyzer/malloc-1.c  (test for warnings, line 429)
seen in https://gcc.gnu.org/ml/gcc-testresults/2020-01/msg01608.html
on x86_64-apple-darwin18.

Fix it by using the __builtin_ forms.

Successfully regrtested on x86_64-pc-linux-gnu.
Committed to master as 3e990d795405b370dc5315da59ce809750173312.

gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/malloc-1.c: Remove include of .
Use __builtin_ forms of memset and strlen throughout.
---
  gcc/testsuite/gcc.dg/analyzer/malloc-1.c | 17 -
  1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-1.c 
b/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
index e2e279bd7fd..c13170560af 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
@@ -1,6 +1,5 @@
  #include 
  #include 
-#include 
  
  extern int foo (void);

  extern int bar (void);
@@ -71,7 +70,7 @@ void test_7 (void)
void *ptr = malloc(4096);
if (!ptr)
  return;
-  memset(ptr, 0, 4096);
+  __builtin_memset(ptr, 0, 4096);
free(ptr);
  }
  
@@ -80,7 +79,7 @@ void *test_8 (void)

void *ptr = malloc(4096);
if (!ptr)
  return NULL;
-  memset(ptr, 0, 4096);
+  __builtin_memset(ptr, 0, 4096);
return ptr;
/* This needs phi nodes to affect equivalence classes, or we get a false 
report
   of a leak.  */
@@ -398,7 +397,7 @@ int test_35 (void)
void *ptr = malloc(4096);
if (!ptr)
  return -1;
-  memset(ptr, 0, 4096);
+  __builtin_memset(ptr, 0, 4096);
free(ptr);
return 0;
  }
@@ -408,14 +407,14 @@ void test_36 (void)
void *ptr = malloc(4096);
if (!ptr)
  return;
-  memset(ptr, 0, 4096);
+  __builtin_memset(ptr, 0, 4096);
free(ptr);
  }
  
  void *test_37a (void)

  {
void *ptr = malloc(4096); /* { dg-message "this call could return NULL" } */
-  memset(ptr, 0, 4096); /* { dg-warning "use of possibly-NULL 'ptr' where non-null 
expected" } */
+  __builtin_memset(ptr, 0, 4096); /* { dg-warning "use of possibly-NULL 'ptr' where 
non-null expected" } */
return ptr;
  }
  
@@ -424,9 +423,9 @@ int test_37b (void)

void *p = malloc(4096);
void *q = malloc(4096); /* { dg-message "this call could return NULL" } */
if (p) {
-memset(p, 0, 4096); /* Not a bug: checked */
+__builtin_memset(p, 0, 4096); /* Not a bug: checked */
} else {
-memset(q, 0, 4096); /* { dg-warning "use of possibly-NULL 'q' where non-null 
expected" } */
+__builtin_memset(q, 0, 4096); /* { dg-warning "use of possibly-NULL 'q' where 
non-null expected" } */
}
free(p);
free(q);
@@ -579,7 +578,7 @@ int test_47 (void)
  int retval = maybe_alloc (); /* this might write to "p".  */
  if (retval)
return (retval);
-p_size = strlen(p); /* { dg-bogus "non-null expected" } */
+p_size = __builtin_strlen(p); /* { dg-bogus "non-null expected" } */
  free (p);
}
return p_size;

[PATCH] calls.c: refactor special_function_p for use by analyzer (v2)

[David Malcolm]

On Tue, 2020-01-28 at 08:36 +0100, Jakub Jelinek wrote:
> On Mon, Jan 27, 2020 at 09:09:37PM -0500, David Malcolm wrote:
> > > Please see calls.c (special_function_p), you should treat
> > > certainly
> > > also sigsetjmp as a setjmp call, and similarly to
> > > special_function_p,
> > > skip over _ or __ prefixes before the setjmp or sigsetjmp name.
> > > Similarly for longjmp/siglongjmp.
> > 
> > This patch refactors some code in special_function_p that checks
> > for
> > the function being sane to match by name, splitting it out into a
> > new
> > maybe_special_function_p, and using it it two places in the
> > analyzer.
> > 
> > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu;
> > OK for master?
> 
> Not sure it is worth it to factor out the
>   DECL_NAME (fndecl)
>   && (DECL_CONTEXT (fndecl) == NULL_TREE
>   || TREE_CODE (DECL_CONTEXT (fndecl)) ==
> TRANSLATION_UNIT_DECL)
>   && TREE_PUBLIC (fndecl)
> check, that seems like simple enough that it could be duplicated.
> And, even if there is a strong reason not to, at least it ought to be
> defined inline in the header, not everyone will use LTO and without
> LTO it
> will need to be an out of line call.

What's motivating this is that I found a second place in the analyzer
that should be doing this check [1], which means it's needed in three
places in the code (calls.c, and the two places in the analyzer).

Here's an updated version of the patch which puts it inline in the
header.  It does change the ordering of the checks in calls.c, in
that the status quo has a very early reject on
   IDENTIFIER_LENGTH (name_decl) <= 11
which seems like a micro-optimization for calls.c that would break
things for the general case.  I don't know how significant moving that
later is.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.

OK for master?  Or, given it's stage 4, should I simply add the new
helper function to the analyzer and leave a copy in calls.c, as a
way of fixing the analyzer bug w/o touching calls.c?

Thanks
Dave

[1] in function_set which is used e.g. for capturing
"all known async-signal-unsafe functions".

> Ack on removing the fndecl && check from special_function_p, the
> callers
> ensure it is non-NULL already, and even if they didn't, after the if
> (fndecl
> && ...) guarded if there is unconditional dereferencing of fndecl.
> 
>   Jakub


This patch refactors some code in special_function_p that checks for
the function being sane to match by name, splitting it out into a new
maybe_special_function_p, and using it it two places in the analyzer.

gcc/analyzer/ChangeLog:
* analyzer.cc (is_named_call_p): Replace tests for fndecl being
extern at file scope and having a non-NULL DECL_NAME with a call
to maybe_special_function_p.
* function-set.cc (function_set::contains_decl_p): Add call to
maybe_special_function_p.

gcc/ChangeLog:
* calls.c (special_function_p): Split out the check for DECL_NAME
being non-NULL and fndecl being extern at file scope into a
new maybe_special_function_p and call it.  Drop check for fndecl
being non-NULL that was after a usage of DECL_NAME (fndecl).
* tree.h (maybe_special_function_p): New inline function.
---
 gcc/analyzer/analyzer.cc | 10 +-
 gcc/analyzer/function-set.cc |  2 ++
 gcc/calls.c  | 14 ++
 gcc/tree.h   | 25 +
 4 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/gcc/analyzer/analyzer.cc b/gcc/analyzer/analyzer.cc
index 1b5e4c9ecf8..5cf745ea632 100644
--- a/gcc/analyzer/analyzer.cc
+++ b/gcc/analyzer/analyzer.cc
@@ -65,18 +65,10 @@ is_named_call_p (tree fndecl, const char *funcname)
   gcc_assert (fndecl);
   gcc_assert (funcname);
 
-  /* Exclude functions not at the file scope, or not `extern',
- since they are not the magic functions we would otherwise
- think they are.  */
-  if (!((DECL_CONTEXT (fndecl) == NULL_TREE
-|| TREE_CODE (DECL_CONTEXT (fndecl)) == TRANSLATION_UNIT_DECL)
-   && TREE_PUBLIC (fndecl)))
+  if (!maybe_special_function_p (fndecl))
 return false;
 
   tree identifier = DECL_NAME (fndecl);
-  if (identifier == NULL)
-return false;
-
   const char *name = IDENTIFIER_POINTER (identifier);
   const char *tname = name;
 
diff --git a/gcc/analyzer/function-set.cc b/gcc/analyzer/function-set.cc
index 6ed15ae95ad..1b6b5d9f9c1 100644
--- a/gcc/analyzer/function-set.cc
+++ b/gcc/analyzer/function-set.cc
@@ -59,6 +59,8 @@ bool
 function_set::contains_decl_p (tree fndecl) const
 {
   gcc_assert (fndecl && DECL_P (fndecl));
+  if (!maybe_special_function_p (fndecl))
+return false;
   return contains_name_p (IDENTIFIER_POINTER (DECL_NAME (fndecl)));
 }
 
diff --git a/gcc/calls.c b/gcc/calls.c
index 1336f49ea5e..d1c53171176 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -586,18 +586,8 @@ special_function_p (const_tree fndecl, int flags)
 {
   tree 

[PATCH 2/2] analyzer: avoid use of fold_build2

[David Malcolm]

Various places in the analyzer use fold_build2, test the result, then
discard it.  It's more efficient to use fold_binary, which avoids
building and GC-ing a redundant tree for the cases where folding fails.

gcc/analyzer/ChangeLog:
* constraint-manager.cc (range::constrained_to_single_element):
Replace fold_build2 with fold_binary.  Remove unnecessary newline.
(constraint_manager::get_or_add_equiv_class): Replace fold_build2
with fold_binary in two places, and remove out-of-date comment.
(constraint_manager::eval_condition): Replace fold_build2 with
fold_binary.
* region-model.cc (constant_svalue::eval_condition): Likewise.
(region_model::on_assignment): Likewise.
---
 gcc/analyzer/constraint-manager.cc | 15 ++-
 gcc/analyzer/region-model.cc   |  6 +++---
 2 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/gcc/analyzer/constraint-manager.cc 
b/gcc/analyzer/constraint-manager.cc
index f3e31ee0830..4d138188856 100644
--- a/gcc/analyzer/constraint-manager.cc
+++ b/gcc/analyzer/constraint-manager.cc
@@ -145,10 +145,9 @@ range::constrained_to_single_element (tree *out)
   m_upper_bound.ensure_closed (true);
 
   // Are they equal?
-  tree comparison
-= fold_build2 (EQ_EXPR, boolean_type_node,
-  m_lower_bound.m_constant,
-  m_upper_bound.m_constant);
+  tree comparison = fold_binary (EQ_EXPR, boolean_type_node,
+m_lower_bound.m_constant,
+m_upper_bound.m_constant);
   if (comparison == boolean_true_node)
 {
   *out = m_lower_bound.m_constant;
@@ -930,7 +929,7 @@ constraint_manager::get_or_add_equiv_class (svalue_id sid)
   FOR_EACH_VEC_ELT (m_equiv_classes, i, ec)
if (ec->m_constant)
  {
-   tree eq = fold_build2 (EQ_EXPR, boolean_type_node,
+   tree eq = fold_binary (EQ_EXPR, boolean_type_node,
   cst, ec->m_constant);
if (eq == boolean_true_node)
  {
@@ -967,10 +966,8 @@ constraint_manager::get_or_add_equiv_class (svalue_id sid)
 Determine the direction of the inequality, and record that
 fact.  */
  tree lt
-   = fold_build2 (LT_EXPR, boolean_type_node,
+   = fold_binary (LT_EXPR, boolean_type_node,
   new_ec->m_constant, other_ec.m_constant);
- //gcc_assert (lt == boolean_true_node || lt == 
boolean_false_node);
- // not true for int vs float comparisons
  if (lt == boolean_true_node)
add_constraint_internal (new_id, CONSTRAINT_LT, other_id);
  else if (lt == boolean_false_node)
@@ -1016,7 +1013,7 @@ constraint_manager::eval_condition (equiv_class_id lhs_ec,
   if (lhs_const && rhs_const)
 {
   tree comparison
-   = fold_build2 (op, boolean_type_node, lhs_const, rhs_const);
+   = fold_binary (op, boolean_type_node, lhs_const, rhs_const);
   if (comparison == boolean_true_node)
return tristate (tristate::TS_TRUE);
   if (comparison == boolean_false_node)
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index b546114bfd5..95d002f9c28 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -670,7 +670,7 @@ constant_svalue::eval_condition (constant_svalue *lhs,
   if (types_compatible_p (TREE_TYPE (lhs_const), TREE_TYPE (rhs_const)))
 {
   tree comparison
-   = fold_build2 (op, boolean_type_node, lhs_const, rhs_const);
+   = fold_binary (op, boolean_type_node, lhs_const, rhs_const);
   if (comparison == boolean_true_node)
return tristate (tristate::TS_TRUE);
   if (comparison == boolean_false_node)
@@ -4070,9 +4070,9 @@ region_model::on_assignment (const gassign *assign, 
region_model_context *ctxt)
if (tree rhs1_cst = maybe_get_constant (rhs1_sid))
  if (tree rhs2_cst = maybe_get_constant (rhs2_sid))
{
- tree result = fold_build2 (op, TREE_TYPE (lhs),
+ tree result = fold_binary (op, TREE_TYPE (lhs),
 rhs1_cst, rhs2_cst);
- if (CONSTANT_CLASS_P (result))
+ if (result && CONSTANT_CLASS_P (result))
{
  svalue_id result_sid
= get_or_create_constant_svalue (result);
-- 
2.21.0

[PATCH 1/2] analyzer: further fixes for comparisons between uncomparable types (PR 93450)

[David Malcolm]

gcc/analyzer/ChangeLog:
PR analyzer/93450
* constraint-manager.cc
(constraint_manager::get_or_add_equiv_class): Only compare constants
if their types are compatible.
* region-model.cc (constant_svalue::eval_condition): Replace check
for identical types with call to types_compatible_p.
---
 gcc/analyzer/constraint-manager.cc | 4 +++-
 gcc/analyzer/region-model.cc   | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/gcc/analyzer/constraint-manager.cc 
b/gcc/analyzer/constraint-manager.cc
index 777bd1b13c9..f3e31ee0830 100644
--- a/gcc/analyzer/constraint-manager.cc
+++ b/gcc/analyzer/constraint-manager.cc
@@ -958,7 +958,9 @@ constraint_manager::get_or_add_equiv_class (svalue_id sid)
   other_id.m_idx++)
{
  const equiv_class _ec = other_id.get_obj (*this);
- if (other_ec.m_constant)
+ if (other_ec.m_constant
+ && types_compatible_p (TREE_TYPE (new_ec->m_constant),
+TREE_TYPE (other_ec.m_constant)))
{
  /* If we have two ECs, both with constants, the constants must be
 non-equal (or they would be in the same EC).
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index a15088a2e3c..b546114bfd5 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -667,7 +667,7 @@ constant_svalue::eval_condition (constant_svalue *lhs,
   gcc_assert (CONSTANT_CLASS_P (rhs_const));
 
   /* Check for comparable types.  */
-  if (TREE_TYPE (lhs_const) == TREE_TYPE (rhs_const))
+  if (types_compatible_p (TREE_TYPE (lhs_const), TREE_TYPE (rhs_const)))
 {
   tree comparison
= fold_build2 (op, boolean_type_node, lhs_const, rhs_const);
-- 
2.21.0

Re: [committed] analyzer: avoid comparisons between uncomparable types (PR 93450)

[David Malcolm]

On Thu, 2020-01-30 at 15:36 +0100, Jakub Jelinek wrote:
> On Thu, Jan 30, 2020 at 09:27:33AM -0500, David Malcolm wrote:
> > --- a/gcc/analyzer/region-model.cc
> > +++ b/gcc/analyzer/region-model.cc
> > @@ -666,12 +666,16 @@ constant_svalue::eval_condition
> > (constant_svalue *lhs,
> >gcc_assert (CONSTANT_CLASS_P (lhs_const));
> >gcc_assert (CONSTANT_CLASS_P (rhs_const));
> >  
> > -  tree comparison
> > -= fold_build2 (op, boolean_type_node, lhs_const, rhs_const);
> > -  if (comparison == boolean_true_node)
> > -return tristate (tristate::TS_TRUE);
> > -  if (comparison == boolean_false_node)
> > -return tristate (tristate::TS_FALSE);
> > +  /* Check for comparable types.  */
> > +  if (TREE_TYPE (lhs_const) == TREE_TYPE (rhs_const))
> 
> Isn't the analyzer invoked on GIMPLE?  In GIMPLE, pointer equality of
> types
> is often not guaranteed and the compiler generally doesn't care about
> that,
> what we care about is whether the two types are compatible
> (types_compatible_p).  E.g. if originally both types were say long
> int,
> but on lp64 one of the arguments was cast from long long int to long
> int,
> you can end up with one operand long int and the other long long int;
> they have the same precision etc., so it is ok to compare them.

Thanks.  In patch 1 of the attached I've replaced the pointer comparison
with a call to types_compatible_p, and added a call to
types_compatible_p to guard a place where constants were being compared.

> > +{
> > +  tree comparison
> > +   = fold_build2 (op, boolean_type_node, lhs_const, rhs_const);
> > +  if (comparison == boolean_true_node)
> > +   return tristate (tristate::TS_TRUE);
> > +  if (comparison == boolean_false_node)
> > +   return tristate (tristate::TS_FALSE);
> 
> This seems to be a waste of compile time memory.  fold_build2 is
> essentially
>   tem = fold_binary_loc (loc, code, type, op0, op1);
>   if (!tem)
> tem = build2_loc (loc, code, type, op0, op1 PASS_MEM_STAT);
> but as you only care if the result is boolean_true_node or
> boolean_false_node, the build2_loc is unnecessary.  So, just use
> fold_binary instead?  If it returns NULL_TREE, it won't compare equal
> to
> either and will fall through to the TS_UNKNOWN case.

Thanks.  I did some grepping and found that I'd made this mistake in
six places in the analyzer.  Sorry about that.  Patch 2 should fix all
of them.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.

OK for master?

David Malcolm (2):
  analyzer: further fixes for comparisons between uncomparable types (PR
93450)
  analyzer: avoid use of fold_build2

 gcc/analyzer/constraint-manager.cc | 19 +--
 gcc/analyzer/region-model.cc   |  8 
 2 files changed, 13 insertions(+), 14 deletions(-)

-- 
2.21.0

[committed] analyzer: add extrinsic_state::dump

[David Malcolm]

I found this useful whilst investigating a bug.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Committed to master as ebe9174e940c94e99cd688a05309833ae64a998b.

gcc/analyzer/ChangeLog:
* program-state.cc (extrinsic_state::dump_to_pp): New.
(extrinsic_state::dump_to_file): New.
(extrinsic_state::dump): New.
* program-state.h (extrinsic_state::dump_to_pp): New decl.
(extrinsic_state::dump_to_file): New decl.
(extrinsic_state::dump): New decl.
* sm.cc: Include "pretty-print.h".
(state_machine::dump_to_pp): New.
* sm.h (state_machine::dump_to_pp): New decl.
---
 gcc/analyzer/program-state.cc | 38 +++
 gcc/analyzer/program-state.h  |  4 
 gcc/analyzer/sm.cc| 12 +++
 gcc/analyzer/sm.h |  2 ++
 4 files changed, 56 insertions(+)

diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index ead62a5d423..4c0b9a8bfa0 100644
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -59,6 +59,44 @@ along with GCC; see the file COPYING3.  If not see
 
 namespace ana {
 
+/* class extrinsic_state.  */
+
+/* Dump a multiline representation of this state to PP.  */
+
+void
+extrinsic_state::dump_to_pp (pretty_printer *pp) const
+{
+  pp_printf (pp, "extrinsic_state: %i checker(s)\n", get_num_checkers ());
+  unsigned i;
+  state_machine *checker;
+  FOR_EACH_VEC_ELT (m_checkers, i, checker)
+{
+  pp_printf (pp, "m_checkers[%i]: %qs\n", i, checker->get_name ());
+  checker->dump_to_pp (pp);
+}
+}
+
+/* Dump a multiline representation of this state to OUTF.  */
+
+void
+extrinsic_state::dump_to_file (FILE *outf) const
+{
+  pretty_printer pp;
+  if (outf == stderr)
+pp_show_color () = pp_show_color (global_dc->printer);
+  pp.buffer->stream = outf;
+  dump_to_pp ();
+  pp_flush ();
+}
+
+/* Dump a multiline representation of this state to stderr.  */
+
+DEBUG_FUNCTION void
+extrinsic_state::dump () const
+{
+  dump_to_file (stderr);
+}
+
 /* class sm_state_map.  */
 
 /* sm_state_map's ctor.  */
diff --git a/gcc/analyzer/program-state.h b/gcc/analyzer/program-state.h
index a052c6e8026..d2badb1a2ed 100644
--- a/gcc/analyzer/program-state.h
+++ b/gcc/analyzer/program-state.h
@@ -45,6 +45,10 @@ public:
 
   unsigned get_num_checkers () const { return m_checkers.length (); }
 
+  void dump_to_pp (pretty_printer *pp) const;
+  void dump_to_file (FILE *outf) const;
+  void dump () const;
+
 private:
   /* The state machines.  */
   auto_delete_vec  _checkers;
diff --git a/gcc/analyzer/sm.cc b/gcc/analyzer/sm.cc
index 74fd17033ff..e94c691c16c 100644
--- a/gcc/analyzer/sm.cc
+++ b/gcc/analyzer/sm.cc
@@ -28,6 +28,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "options.h"
 #include "function.h"
 #include "diagnostic-core.h"
+#include "pretty-print.h"
 #include "analyzer/analyzer.h"
 #include "analyzer/analyzer-logging.h"
 #include "analyzer/sm.h"
@@ -91,6 +92,17 @@ state_machine::validate (state_t s) const
   gcc_assert (s < m_state_names.length ());
 }
 
+/* Dump a multiline representation of this state machine to PP.  */
+
+void
+state_machine::dump_to_pp (pretty_printer *pp) const
+{
+  unsigned i;
+  const char *name;
+  FOR_EACH_VEC_ELT (m_state_names, i, name)
+pp_printf (pp, "  state %i: %qs\n", i, name);
+}
+
 /* Create instances of the various state machines, each using LOGGER,
and populate OUT with them.  */
 
diff --git a/gcc/analyzer/sm.h b/gcc/analyzer/sm.h
index 25163d7a7b1..3e8f4b6891d 100644
--- a/gcc/analyzer/sm.h
+++ b/gcc/analyzer/sm.h
@@ -80,6 +80,8 @@ public:
 
   void validate (state_t s) const;
 
+  void dump_to_pp (pretty_printer *pp) const;
+
 protected:
   state_t add_state (const char *name);
 
-- 
2.21.0

Re: [PATCH] gcc: Add new configure options to allow static libraries to be selected

[Andrew Burgess]

Here's a cleaned up version of the previous patch I posted.  If Iain
reports this fixes the regressions he saw then I will push this.

All feedback welcome.

Thanks,
Andrew

---

>From 876996580d64d31407357787fc5df7bd5699b2c0 Mon Sep 17 00:00:00 2001
From: Andrew Burgess 
Date: Thu, 30 Jan 2020 12:18:13 +
Subject: [PATCH] Fixes after recent configure changes relating to static
 libraries

This commit:

  commit e7c26e04b2dd6266d62d5a5825ff7eb44d1cf14e (tjteru/master)
  Date:   Wed Jan 22 14:54:26 2020 +

  gcc: Add new configure options to allow static libraries to be selected

contains a couple of issues.  First I failed to correctly regenerate
all of the configure files it should have done.  Second, there was a
mistake in lib-link.m4, one of the conditions didn't use pure sh
syntax, I wrote this:

  if x$lib_type = xauto || x$lib_type = xshared; then

When I should have written this:

  if test "x$lib_type" = "xauto" || test "x$lib_type" = "xshared"; then

These issues were raised on the mailing list in these messages:

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

config/ChangeLog:

* lib-link.m4 (AC_LIB_LINKFLAGS_BODY): Update shell syntax.

gcc/ChangeLog:

* configure: Regenerate.

intl/ChangeLog:

* configure: Regenerate.

libcpp/ChangeLog:

* configure: Regenerate.

libstdc++-v3/ChangeLog:

* configure: Regenerate.
---
 config/ChangeLog   |  4 
 config/lib-link.m4 |  2 +-
 gcc/ChangeLog  |  4 
 gcc/configure  | 29 +++--
 intl/ChangeLog |  4 
 intl/configure | 58 +++---
 libcpp/ChangeLog   |  4 
 libcpp/configure   |  2 +-
 libstdc++-v3/ChangeLog |  4 
 libstdc++-v3/configure |  2 +-
 10 files changed, 92 insertions(+), 21 deletions(-)

diff --git a/config/lib-link.m4 b/config/lib-link.m4
index 662192e0a07..20e281fd323 100644
--- a/config/lib-link.m4
+++ b/config/lib-link.m4
@@ -492,7 +492,7 @@ AC_DEFUN([AC_LIB_LINKFLAGS_BODY],
 dnl known to the linker and runtime loader. (All the system
 dnl directories known to the linker should also be known to the
 dnl runtime loader, otherwise the system is severely 
misconfigured.)
-if x$lib_type = xauto || x$lib_type = xshared; then
+if test "x$lib_type" = "xauto" || test "x$lib_type" = "xshared"; 
then
   LIB[]NAME="${LIB[]NAME}${LIB[]NAME:+ }-l$name"
   LTLIB[]NAME="${LTLIB[]NAME}${LTLIB[]NAME:+ }-l$name"
 else
diff --git a/gcc/configure b/gcc/configure
index e2c8fc71772..4c2c5991c0e 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -974,6 +974,7 @@ with_zstd_include
 with_zstd_lib
 enable_rpath
 with_libiconv_prefix
+with_libiconv_type
 enable_sjlj_exceptions
 with_gcc_major_version_only
 enable_secureplt
@@ -1811,6 +1812,7 @@ Optional Packages:
   --with-gnu-ld   assume the C compiler uses GNU ld default=no
   --with-libiconv-prefix[=DIR]  search for libiconv in DIR/include and DIR/lib
   --without-libiconv-prefix don't search for libiconv in includedir and 
libdir
+  --with-libiconv-type=TYPE type of library to search for 
(auto/static/shared)
   --with-gcc-major-version-only
   use only GCC major number in filesystem paths
   --with-pic  try to use only PIC/non-PIC objects [default=use
@@ -10730,6 +10732,16 @@ if test "${with_libiconv_prefix+set}" = set; then :
 
 fi
 
+
+# Check whether --with-libiconv-type was given.
+if test "${with_libiconv_type+set}" = set; then :
+  withval=$with_libiconv_type;  with_libiconv_type=$withval
+else
+   with_libiconv_type=auto
+fi
+
+  lib_type=`eval echo \$with_libiconv_type`
+
   LIBICONV=
   LTLIBICONV=
   INCICONV=
@@ -10767,13 +10779,13 @@ fi
   found_so=
   found_a=
   if test $use_additional = yes; then
-if test -n "$shlibext" && test -f 
"$additional_libdir/lib$name.$shlibext"; then
+if test -n "$shlibext" && test -f 
"$additional_libdir/lib$name.$shlibext" && test x$lib_type != xstatic; then
   found_dir="$additional_libdir"
   found_so="$additional_libdir/lib$name.$shlibext"
   if test -f "$additional_libdir/lib$name.la"; then
 found_la="$additional_libdir/lib$name.la"
   fi
-else
+elif test x$lib_type != xshared; then
   if test -f "$additional_libdir/lib$name.$libext"; then
 found_dir="$additional_libdir"
 found_a="$additional_libdir/lib$name.$libext"
@@ -10797,13 +10809,13 @@ fi
   case "$x" in
 -L*)
   dir=`echo "X$x" | sed -e 's/^X-L//'`
-  if test -n "$shlibext" && test -f "$dir/lib$name.$shlibext"; 
then
+  if test -n "$shlibext" && test -f 

[committed] analyzer: make extrinsic_state field private

[David Malcolm]

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Committed to master as ebe9174e940c94e99cd688a05309833ae64a998b.

gcc/analyzer/ChangeLog:
* diagnostic-manager.cc (for_each_state_change): Use
extrinsic_state::get_num_checkers rather than accessing m_checkers
directly.
* program-state.cc (program_state::program_state): Likewise.
* program-state.h (extrinsic_state::m_checkers): Make private.
---
 gcc/analyzer/diagnostic-manager.cc | 6 +++---
 gcc/analyzer/program-state.cc  | 4 ++--
 gcc/analyzer/program-state.h   | 1 +
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/gcc/analyzer/diagnostic-manager.cc 
b/gcc/analyzer/diagnostic-manager.cc
index eb1fa05533e..023fda9fa7c 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -684,10 +684,10 @@ for_each_state_change (const program_state _state,
   state_change_visitor *visitor)
 {
   gcc_assert (src_state.m_checker_states.length ()
- == ext_state.m_checkers.length ());
+ == ext_state.get_num_checkers ());
   gcc_assert (dst_state.m_checker_states.length ()
- == ext_state.m_checkers.length ());
-  for (unsigned i = 0; i < ext_state.m_checkers.length (); i++)
+ == ext_state.get_num_checkers ());
+  for (unsigned i = 0; i < ext_state.get_num_checkers (); i++)
 {
   const state_machine  = ext_state.get_sm (i);
   const sm_state_map _smap = *src_state.m_checker_states[i];
diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index f41f105ce6b..ead62a5d423 100644
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -535,9 +535,9 @@ sm_state_map::validate (const state_machine ,
 
 program_state::program_state (const extrinsic_state _state)
 : m_region_model (new region_model ()),
-  m_checker_states (ext_state.m_checkers.length ())
+  m_checker_states (ext_state.get_num_checkers ())
 {
-  int num_states = ext_state.m_checkers.length ();
+  int num_states = ext_state.get_num_checkers ();
   for (int i = 0; i < num_states; i++)
 m_checker_states.quick_push (new sm_state_map ());
 }
diff --git a/gcc/analyzer/program-state.h b/gcc/analyzer/program-state.h
index 0a4e35f3d5d..a052c6e8026 100644
--- a/gcc/analyzer/program-state.h
+++ b/gcc/analyzer/program-state.h
@@ -45,6 +45,7 @@ public:
 
   unsigned get_num_checkers () const { return m_checkers.length (); }
 
+private:
   /* The state machines.  */
   auto_delete_vec  _checkers;
 };
-- 
2.21.0

[rfc PATCH] rs6000: Updated constraint documentation

[Segher Boessenkool]

This is my current work-in-progress version.  There still are rough
edges, and not much is done for the output modifiers yet, but it should
be in much better shape wrt the user manual now.  The internals manual
also is a bit better I think.

md.texi is not automatically kept in synch with constraints.md (let
alone generated from it), so the two diverged.  I tried to correct
that, too.

Please let me know if you have any ideas how to improve it further, or
if I did something terribly wrong, or anything else.  Thanks,


Segher

---
 gcc/config/rs6000/constraints.md | 159 +++--
 gcc/doc/md.texi  | 188 +++
 2 files changed, 182 insertions(+), 165 deletions(-)

diff --git a/gcc/config/rs6000/constraints.md b/gcc/config/rs6000/constraints.md
index 398c894..bafc22a 100644
--- a/gcc/config/rs6000/constraints.md
+++ b/gcc/config/rs6000/constraints.md
@@ -21,192 +21,214 @@
 
 ;; Register constraints
 
-(define_register_constraint "f" "rs6000_constraints[RS6000_CONSTRAINT_f]"
-  "@internal")
-
-(define_register_constraint "d" "rs6000_constraints[RS6000_CONSTRAINT_d]"
-  "@internal")
+; Actually defined in common.md:
+; (define_register_constraint "r" "GENERAL_REGS"
+;   "A general purpose register (GPR), @code{r0}@dots{}@code{r31}.")
 
 (define_register_constraint "b" "BASE_REGS"
-  "@internal")
+  "A base register.  Like @code{r}, but @code{r0} is not allowed, so
+   @code{r1}@dots{}@code{r31}.")
 
-(define_register_constraint "h" "SPECIAL_REGS"
-  "@internal")
+(define_register_constraint "f" "rs6000_constraints[RS6000_CONSTRAINT_f]"
+  "A floating point register (FPR), @code{f0}@dots{}@code{f31}.")
 
-(define_register_constraint "c" "CTR_REGS"
-  "@internal")
-
-(define_register_constraint "l" "LINK_REGS"
-  "@internal")
+(define_register_constraint "d" "rs6000_constraints[RS6000_CONSTRAINT_d]"
+  "A floating point register.  This is the same as @code{f} nowadays;
+   historically @code{f} was for single-precision and @code{d} was for
+   double-precision floating point.")
 
 (define_register_constraint "v" "ALTIVEC_REGS"
-  "@internal")
+  "An Altivec vector register (VR), @code{v0}@dots{}@code{v31}.")
+
+(define_register_constraint "wa" "rs6000_constraints[RS6000_CONSTRAINT_wa]"
+  "A VSX register (VSR), @code{vs0}@dots{}@code{vs63}.  Either a @code{d}
+   or a @code{v} register.")
+
+(define_register_constraint "h" "SPECIAL_REGS"
+  "@internal @code{vrsave}, @code{ctr}, or @code{lr}.")
+
+(define_register_constraint "c" "CTR_REGS"
+  "The count register, @code{ctr}.")
+
+(define_register_constraint "l" "LINK_REGS"
+  "The link register, @code{lr}.")
 
 (define_register_constraint "x" "CR0_REGS"
-  "@internal")
+  "Condition register field 0, @code{cr0}.")
 
 (define_register_constraint "y" "CR_REGS"
-  "@internal")
+  "Any condition register field, @code{cr0}@dots{}@code{cr7}.")
 
 (define_register_constraint "z" "CA_REGS"
-  "@internal")
-
-;; Use w as a prefix to add VSX modes
-;; any VSX register
-(define_register_constraint "wa" "rs6000_constraints[RS6000_CONSTRAINT_wa]"
-  "Any VSX register if the -mvsx option was used or NO_REGS.")
+  "@internal The carry bit, @code{XER[CA]}.")
 
 ;; NOTE: For compatibility, "wc" is reserved to represent individual CR bits.
 ;; It is currently used for that purpose in LLVM.
 
 (define_register_constraint "we" "rs6000_constraints[RS6000_CONSTRAINT_we]"
-  "VSX register if the -mpower9-vector -m64 options were used or NO_REGS.")
+  "@internal VSX register if the -mpower9-vector -m64 options were used
+   or NO_REGS.")
 
 ;; NO_REGs register constraint, used to merge mov{sd,sf}, since movsd can use
 ;; direct move directly, and movsf can't to move between the register sets.
 ;; There is a mode_attr that resolves to wa for SDmode and wn for SFmode
-(define_register_constraint "wn" "NO_REGS" "No register (NO_REGS).")
+(define_register_constraint "wn" "NO_REGS"
+  "@internal No register (NO_REGS).")
 
 (define_register_constraint "wr" "rs6000_constraints[RS6000_CONSTRAINT_wr]"
-  "General purpose register if 64-bit instructions are enabled or NO_REGS.")
+  "@internal General purpose register if 64-bit instructions are enabled
+   or NO_REGS.")
 
 (define_register_constraint "wx" "rs6000_constraints[RS6000_CONSTRAINT_wx]"
-  "Floating point register if the STFIWX instruction is enabled or NO_REGS.")
+  "@internal Floating point register if the STFIWX instruction is enabled
+   or NO_REGS.")
 
 (define_register_constraint "wA" "rs6000_constraints[RS6000_CONSTRAINT_wA]"
-  "BASE_REGS if 64-bit instructions are enabled or NO_REGS.")
+  "@internal BASE_REGS if 64-bit instructions are enabled or NO_REGS.")
 
 ;; wB needs ISA 2.07 VUPKHSW
 (define_constraint "wB"
-  "Signed 5-bit constant integer that can be loaded into an altivec register."
+  "@internal Signed 5-bit constant integer that can be loaded into an
+   Altivec register."
   (and (match_code "const_int")
-   (and (match_test 

[committed] analyzer: avoid using in malloc-1.c

[David Malcolm]

This test assumes that memset and strlen have been marked with
__attribute__((nonnull)), which isn't necessarily the case for an
arbitrary .  This likely explains these failures:
  FAIL: gcc.dg/analyzer/malloc-1.c  (test for warnings, line 417)
  FAIL: gcc.dg/analyzer/malloc-1.c  (test for warnings, line 418)
  FAIL: gcc.dg/analyzer/malloc-1.c  (test for warnings, line 425)
  FAIL: gcc.dg/analyzer/malloc-1.c  (test for warnings, line 429)
seen in https://gcc.gnu.org/ml/gcc-testresults/2020-01/msg01608.html
on x86_64-apple-darwin18.

Fix it by using the __builtin_ forms.

Successfully regrtested on x86_64-pc-linux-gnu.
Committed to master as 3e990d795405b370dc5315da59ce809750173312.

gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/malloc-1.c: Remove include of .
Use __builtin_ forms of memset and strlen throughout.
---
 gcc/testsuite/gcc.dg/analyzer/malloc-1.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-1.c 
b/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
index e2e279bd7fd..c13170560af 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
@@ -1,6 +1,5 @@
 #include 
 #include 
-#include 
 
 extern int foo (void);
 extern int bar (void);
@@ -71,7 +70,7 @@ void test_7 (void)
   void *ptr = malloc(4096);
   if (!ptr)
 return;
-  memset(ptr, 0, 4096);
+  __builtin_memset(ptr, 0, 4096);
   free(ptr);
 }
 
@@ -80,7 +79,7 @@ void *test_8 (void)
   void *ptr = malloc(4096);
   if (!ptr)
 return NULL;
-  memset(ptr, 0, 4096);
+  __builtin_memset(ptr, 0, 4096);
   return ptr;
   /* This needs phi nodes to affect equivalence classes, or we get a false 
report
  of a leak.  */
@@ -398,7 +397,7 @@ int test_35 (void)
   void *ptr = malloc(4096);
   if (!ptr)
 return -1;
-  memset(ptr, 0, 4096);
+  __builtin_memset(ptr, 0, 4096);
   free(ptr);
   return 0;
 }
@@ -408,14 +407,14 @@ void test_36 (void)
   void *ptr = malloc(4096);
   if (!ptr)
 return;
-  memset(ptr, 0, 4096);
+  __builtin_memset(ptr, 0, 4096);
   free(ptr);
 }
 
 void *test_37a (void)
 {
   void *ptr = malloc(4096); /* { dg-message "this call could return NULL" } */
-  memset(ptr, 0, 4096); /* { dg-warning "use of possibly-NULL 'ptr' where 
non-null expected" } */
+  __builtin_memset(ptr, 0, 4096); /* { dg-warning "use of possibly-NULL 'ptr' 
where non-null expected" } */
   return ptr;
 }
 
@@ -424,9 +423,9 @@ int test_37b (void)
   void *p = malloc(4096);
   void *q = malloc(4096); /* { dg-message "this call could return NULL" } */
   if (p) {
-memset(p, 0, 4096); /* Not a bug: checked */
+__builtin_memset(p, 0, 4096); /* Not a bug: checked */
   } else {
-memset(q, 0, 4096); /* { dg-warning "use of possibly-NULL 'q' where 
non-null expected" } */
+__builtin_memset(q, 0, 4096); /* { dg-warning "use of possibly-NULL 'q' 
where non-null expected" } */
   }
   free(p);
   free(q);
@@ -579,7 +578,7 @@ int test_47 (void)
 int retval = maybe_alloc (); /* this might write to "p".  */
 if (retval)
   return (retval);
-p_size = strlen(p); /* { dg-bogus "non-null expected" } */
+p_size = __builtin_strlen(p); /* { dg-bogus "non-null expected" } */
 free (p);
   }
   return p_size;
-- 
2.21.0

[committed] analyzer: convert conditionals-2.c to a torture test

[David Malcolm]

Successfully regrtested on x86_64-pc-linux-gnu.
Committed to master as e34ad101a4338eab41e38e624f2c7178d0b83d24.

gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/conditionals-2.c: Move to...
* gcc.dg/analyzer/torture/conditionals-2.c: ...here, converting
to a torture test.  Remove redundant include.
---
 .../gcc.dg/analyzer/{ => torture}/conditionals-2.c  | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)
 rename gcc/testsuite/gcc.dg/analyzer/{ => torture}/conditionals-2.c (87%)

diff --git a/gcc/testsuite/gcc.dg/analyzer/conditionals-2.c 
b/gcc/testsuite/gcc.dg/analyzer/torture/conditionals-2.c
similarity index 87%
rename from gcc/testsuite/gcc.dg/analyzer/conditionals-2.c
rename to gcc/testsuite/gcc.dg/analyzer/torture/conditionals-2.c
index 6f291f4861b..5580d22ab92 100644
--- a/gcc/testsuite/gcc.dg/analyzer/conditionals-2.c
+++ b/gcc/testsuite/gcc.dg/analyzer/torture/conditionals-2.c
@@ -1,8 +1,6 @@
-// TODO: run this test case at every optimization level
-/* { dg-additional-options "-O2" } */
+/* { dg-skip-if "" { *-*-* } { "-fno-fat-lto-objects" } { "" } } */
 
-#include 
-#include "analyzer-decls.h"
+#include "../analyzer-decls.h"
 
 #define Z_NULL 0
 
-- 
2.21.0

[committed] analyzer: fix ICE in __builtin_isnan (PR 93356)

[David Malcolm]

PR analyzer/93356 reports an ICE handling __builtin_isnan due to a
failing assertion:
  674 gcc_assert (lhs_ec_id != rhs_ec_id);
with op=UNORDERED_EXPR.
when attempting to add an UNORDERED_EXPR constraint.

This is an overzealous assertion, but underlying it are various forms of
sloppiness regarding NaN within the analyzer:

  (a) the assumption in the constraint_manager that equivalence classes
  are reflexive (X == X), which isn't the case for NaN.

  (b) Hardcoding the "honor_nans" param to false when calling
  invert_tree_comparison throughout the analyzer.

  (c) Ignoring ORDERED_EXPR, UNORDERED_EXPR, and the UN-prefixed
  comparison codes.

I wrote a patch for this which tracks the NaN-ness of floating-point
values and uses this to address all of the above.

However, to minimize changes in gcc 10 stage 4, here's a simpler patch
which rejects attempts to query or add constraints on floating-point
values, instead treating any floating-point comparison as "unknown", and
silently dropping the constraints at edges.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Committed to master as r10-6361-ge978955dd720d5cc0e5141a1e9b943a3cc41.

gcc/analyzer/ChangeLog:
PR analyzer/93356
* region-model.cc (region_model::eval_condition): In both
overloads, bail out immediately on floating-point types.
(region_model::eval_condition_without_cm): Likewise.
(region_model::add_constraint): Likewise.

gcc/testsuite/ChangeLog:
PR analyzer/93356
* gcc.dg/analyzer/conditionals-notrans.c (test_float_selfcmp):
Add.
* gcc.dg/analyzer/conditionals-trans.c: Mark floating point
comparison test as failing.
(test_float_selfcmp): Add.
* gcc.dg/analyzer/data-model-1.c: Mark floating point comparison
tests as failing.
* gcc.dg/analyzer/torture/pr93356.c: New test.

gcc/ChangeLog:
PR analyzer/93356
* doc/analyzer.texi (Limitations): Note that constraints on
floating-point values are currently ignored.
---
 gcc/analyzer/region-model.cc  | 25 +++
 gcc/doc/analyzer.texi |  2 ++
 .../gcc.dg/analyzer/conditionals-notrans.c|  6 +
 .../gcc.dg/analyzer/conditionals-trans.c  |  9 ++-
 gcc/testsuite/gcc.dg/analyzer/data-model-1.c  |  9 ---
 .../gcc.dg/analyzer/torture/pr93356.c |  6 +
 6 files changed, 53 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/torture/pr93356.c

diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index c838454a1eb..a15088a2e3c 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -5144,6 +5144,15 @@ region_model::eval_condition (svalue_id lhs_sid,
  enum tree_code op,
  svalue_id rhs_sid) const
 {
+  svalue *lhs = get_svalue (lhs_sid);
+  svalue *rhs = get_svalue (rhs_sid);
+
+  /* For now, make no attempt to capture constraints on floating-point
+ values.  */
+  if ((lhs->get_type () && FLOAT_TYPE_P (lhs->get_type ()))
+  || (rhs->get_type () && FLOAT_TYPE_P (rhs->get_type (
+return tristate::unknown ();
+
   tristate ts = eval_condition_without_cm (lhs_sid, op, rhs_sid);
 
   if (ts.is_known ())
@@ -5173,6 +5182,12 @@ region_model::eval_condition_without_cm (svalue_id 
lhs_sid,
   /* See what we know based on the values.  */
   if (lhs && rhs)
 {
+  /* For now, make no attempt to capture constraints on floating-point
+values.  */
+  if ((lhs->get_type () && FLOAT_TYPE_P (lhs->get_type ()))
+ || (rhs->get_type () && FLOAT_TYPE_P (rhs->get_type (
+   return tristate::unknown ();
+
   if (lhs == rhs)
{
  /* If we have the same svalue, then we have equality
@@ -5252,6 +5267,11 @@ bool
 region_model::add_constraint (tree lhs, enum tree_code op, tree rhs,
  region_model_context *ctxt)
 {
+  /* For now, make no attempt to capture constraints on floating-point
+ values.  */
+  if (FLOAT_TYPE_P (TREE_TYPE (lhs)) || FLOAT_TYPE_P (TREE_TYPE (rhs)))
+return true;
+
   svalue_id lhs_sid = get_rvalue (lhs, ctxt);
   svalue_id rhs_sid = get_rvalue (rhs, ctxt);
 
@@ -5385,6 +5405,11 @@ region_model::eval_condition (tree lhs,
  tree rhs,
  region_model_context *ctxt)
 {
+  /* For now, make no attempt to model constraints on floating-point
+ values.  */
+  if (FLOAT_TYPE_P (TREE_TYPE (lhs)) || FLOAT_TYPE_P (TREE_TYPE (rhs)))
+return tristate::unknown ();
+
   return eval_condition (get_rvalue (lhs, ctxt), op, get_rvalue (rhs, ctxt));
 }
 
diff --git a/gcc/doc/analyzer.texi b/gcc/doc/analyzer.texi
index 81acdd8998b..1fe4bcefd1b 100644
--- a/gcc/doc/analyzer.texi
+++ b/gcc/doc/analyzer.texi
@@ -390,6 +390,8 @@ Lack of function pointer analysis
 @item
 The constraint-handling code assumes 

[Patch] Inline optimization for tanh(x)/sinh(x) -> 1.0/cosh(x)

[Vitor Guidi]

Hi.

This patch adds a new optimization to avoid the redundant calculation
tanh(x)/sinh(x) by replacing it for 1.0/cosh(x), for all cases where x is a
double, a long double or a float.

There should be no need for numerical stability testing, since the division
of the two functions only adds numerical noise. The correctness of the
operation is justified by the definition of tanh(x) = sinh(x)/cosh(x). If
you think it is wise to write a test, please let me know.

As far as testing goes, I ran a check-gcc test under Ubuntu 19.04 by adding
the test in tanhbysinh.c and found no issues.

Best regards,

Vitor.

in gcc/ChangeLog:
2020-08-28  Vitor Guidi 

* match.pd: New substitution rule for tanh(x)/sinh(x) ->
1.0/cosh(x).

in gcc/testsuite/ChangeLog:
2020-08-28  Vitor Guidi 

* gcc.dg/tanhbysinh.c (new): Verify if simplification is applied.

> diff --git gcc/match.pd gcc/match.pd
> index 5fee394e7af..3933fcaf9fa 100644
> --- gcc/match.pd
> +++ gcc/match.pd
> @@ -5069,6 +5069,11 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>(rdiv (SINH:s @0) (COSH:s @0))
> (TANH @0))
>
> + /* Simplify tanh (x) / sinh (x) -> 1.0 / cosh (x). */
> + (simplify
> +   (rdiv (TANH @0) (SINH @0))
> +   (rdiv {build_one_cst (type);} (COSH @0)))
> +
>   /* Simplify cos(x) / sin(x) -> 1 / tan(x). */
>   (simplify
>(rdiv (COS:s @0) (SIN:s @0))
> diff --git gcc/testsuite/gcc.dg/tanhbysinh.c
gcc/testsuite/gcc.dg/tanhbysinh.c
> new file mode 100644
> index 000..fde72c2f93b
> --- /dev/null
> +++ gcc/testsuite/gcc.dg/tanhbysinh.c
> @@ -0,0 +1,40 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Ofast -fdump-tree-optimized" } */
> +
> +extern float sinhf (float);
> +extern float tanhf (float);
> +extern double sinh (double);
> +extern double tanh (double);
> +extern long double sinhl (long double);
> +extern long double tanhl (long double);
> +
> +double __attribute__ ((noinline))
> +tanhbysinh_ (double x)
> +{
> +return tanh (x) / sinh (x);
> +}
> +
> +float __attribute__ ((noinline))
> +tanhbysinhf_ (float x)
> +{
> +return tanhf (x) / sinhf (x);
> +}
> +
> +long double __attribute__ ((noinline))
> +tanhbysinhl_ (long double x)
> +{
> +return tanhl (x) / sinhl (x);
> +}
> +
> +
> +/* There must be no calls to sinh or atanh */
> +/* There must be calls to cosh */
> +/* {dg-final { scan-tree-dump-not "sinh " "optimized" } } */
> +/* {dg-final { scan-tree-dump-not "tanh " "optimized" }} */
> +/* {dg-final { scan-tree-dump-not "sinhf " "optimized" } } */
> +/* {dg-final { scan-tree-dump-not "tanhf " "optimized" }} */
> +/* {dg-final { scan-tree-dump-not "sinhl " "optimized" } } */
> +/* {dg-final { scan-tree-dump-not "tanhl " "optimized" }} */
> +/* { dg-final { scan-tree-dump "cosh " "optimized" } } */
> +/* { dg-final { scan-tree-dump "coshf " "optimized" } } */
> +/* { dg-final { scan-tree-dump "coshl " "optimized" } } */
> \ No newline at end of file

[PATCH] RISC-V: Fix combined tree builds.

[Jim Wilson]

The RISC-V toolchain doesn't support leb128 because of linker relaxation
to reduce code size.  This prevents us from computing the leb128 size of a
value at compile time.  So do a configure time gas feature check regardless
of gas version.

The libiconv configure change comes from the recent config/lib-link.m4
patch.  The gcc configure wasn't rebuilt after this change as this was
intended for library configure files.

Tested with riscv32-elf and arm-eabi combined tree builds.  The riscv build
fails without the patch and works with the patch where HAVE_AS_LEB128 is now
false.  The arm build still works and still has HAVE_AS_LEB128 true.

OK?

Jim

gcc/
PR target/91602
* configure.ac (HAVE_AS_LEB128): Delete gas version number in
gcc_GAS_CHECK_FEATURE call.
* configure: Regenerated.
---
 gcc/configure| 39 +++
 gcc/configure.ac |  5 -
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/gcc/configure b/gcc/configure
index e2c8fc71772..2f57fbf3223 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -974,6 +974,7 @@ with_zstd_include
 with_zstd_lib
 enable_rpath
 with_libiconv_prefix
+with_libiconv_type
 enable_sjlj_exceptions
 with_gcc_major_version_only
 enable_secureplt
@@ -1811,6 +1812,7 @@ Optional Packages:
   --with-gnu-ld   assume the C compiler uses GNU ld default=no
   --with-libiconv-prefix[=DIR]  search for libiconv in DIR/include and DIR/lib
   --without-libiconv-prefix don't search for libiconv in includedir and 
libdir
+  --with-libiconv-type=TYPE type of library to search for 
(auto/static/shared)
   --with-gcc-major-version-only
   use only GCC major number in filesystem paths
   --with-pic  try to use only PIC/non-PIC objects [default=use
@@ -10730,6 +10732,16 @@ if test "${with_libiconv_prefix+set}" = set; then :
 
 fi
 
+
+# Check whether --with-libiconv-type was given.
+if test "${with_libiconv_type+set}" = set; then :
+  withval=$with_libiconv_type;  with_libiconv_type=$withval
+else
+   with_libiconv_type=auto
+fi
+
+  lib_type=`eval echo \$with_libiconv_type`
+
   LIBICONV=
   LTLIBICONV=
   INCICONV=
@@ -10767,13 +10779,13 @@ fi
   found_so=
   found_a=
   if test $use_additional = yes; then
-if test -n "$shlibext" && test -f 
"$additional_libdir/lib$name.$shlibext"; then
+if test -n "$shlibext" && test -f 
"$additional_libdir/lib$name.$shlibext" && test x$lib_type != xstatic; then
   found_dir="$additional_libdir"
   found_so="$additional_libdir/lib$name.$shlibext"
   if test -f "$additional_libdir/lib$name.la"; then
 found_la="$additional_libdir/lib$name.la"
   fi
-else
+elif test x$lib_type != xshared; then
   if test -f "$additional_libdir/lib$name.$libext"; then
 found_dir="$additional_libdir"
 found_a="$additional_libdir/lib$name.$libext"
@@ -10797,13 +10809,13 @@ fi
   case "$x" in
 -L*)
   dir=`echo "X$x" | sed -e 's/^X-L//'`
-  if test -n "$shlibext" && test -f "$dir/lib$name.$shlibext"; 
then
+  if test -n "$shlibext" && test -f "$dir/lib$name.$shlibext" 
&& test x$lib_type != xstatic; then
 found_dir="$dir"
 found_so="$dir/lib$name.$shlibext"
 if test -f "$dir/lib$name.la"; then
   found_la="$dir/lib$name.la"
 fi
-  else
+  elif test x$lib_type != xshared; then
 if test -f "$dir/lib$name.$libext"; then
   found_dir="$dir"
   found_a="$dir/lib$name.$libext"
@@ -11031,8 +11043,13 @@ fi
   done
 fi
   else
-
LIBICONV="${LIBICONV}${LIBICONV:+ }-l$name"
-LTLIBICONV="${LTLIBICONV}${LTLIBICONV:+ }-l$name"
+if x$lib_type = 
xauto || x$lib_type = xshared; then
+  LIBICONV="${LIBICONV}${LIBICONV:+ }-l$name"
+  LTLIBICONV="${LTLIBICONV}${LTLIBICONV:+ }-l$name"
+else
+  LIBICONV="${LIBICONV}${LIBICONV:+ }-l:lib$name.$libext"
+  LTLIBICONV="${LTLIBICONV}${LTLIBICONV:+ }-l:lib$name.$libext"
+fi
   fi
 fi
   fi
@@ -23695,18 +23712,16 @@ _ACEOF
 
 
 # Check if we have .[us]leb128, and support symbol arithmetic with it.
+# Targets with aggressive linker relaxation to reduce code size may not be
+# able to support this regardless of gas version, so we don't pass a gas
+# version to force a configure time gas feature check.  RISC-V is an example.
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for .sleb128 and 
.uleb128" >&5
 $as_echo_n 

Re: [PR93488] [OpenACC] ICE in type-cast 'async', 'wait' clauses

[Andrew Stubbs]

On 29/01/2020 12:30, Thomas Schwinge wrote:

As can be seen in the code a few lines below, the very same problem also
exists for the 'wait' clause; it seems reasonable to fix both at the same
time.  This is not a recent regression, but a user-visible valid-code ICE
that has existed "forever"; I filed  "ICE in
type-cast 'async', 'wait' clauses" for tracking.  This problem is similar
to the OpenMP 'device' clause  "ICE in
verify_gimple_in_cfg, at tree-cfg.c:5212"; I suggest we also use
'force_gimple_operand_gsi' instead of manually doing your suggested
'create_tmp_var', 'gimple_build_assign', 'gsi_insert_before'.  Include a
test case that covers all relevant code paths; I've attached a test case
to the PR, but I've not verified whether "that covers *all* relevant code
paths".  This should then be backported to all GCC release branches; I
can easily test the backports for you, if you're not already set up to do
such testing.


How's this?

Andrew
Normalize GOACC_parallel_keyed async and wait parameters

2020-01-30  Andrew Stubbs  
	Thomas Schwinge  

	PR middle-end/93488

	gcc/
	* omp-expand.c (expand_omp_target): Use force_gimple_operand_gsi on
	t_async and the wait arguments.

diff --git a/gcc/omp-expand.c b/gcc/omp-expand.c
index cd423ad799e..ec4baf46965 100644
--- a/gcc/omp-expand.c
+++ b/gcc/omp-expand.c
@@ -8418,7 +8418,9 @@ expand_omp_target (struct omp_region *region)
 	  i_async));
 	  }
 	if (t_async)
-	  args.safe_push (t_async);
+	  args.safe_push (force_gimple_operand_gsi (, t_async, true,
+		NULL_TREE, true,
+		GSI_SAME_STMT));
 
 	/* Save the argument index, and ... */
 	unsigned t_wait_idx = args.length ();
@@ -8431,9 +8433,12 @@ expand_omp_target (struct omp_region *region)
 	for (; c; c = OMP_CLAUSE_CHAIN (c))
 	  if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_WAIT)
 	{
-	  args.safe_push (fold_convert_loc (OMP_CLAUSE_LOCATION (c),
-		integer_type_node,
-		OMP_CLAUSE_WAIT_EXPR (c)));
+	  tree arg = fold_convert_loc (OMP_CLAUSE_LOCATION (c),
+	   integer_type_node,
+	   OMP_CLAUSE_WAIT_EXPR (c));
+	  arg = force_gimple_operand_gsi (, arg, true, NULL_TREE, true,
+	  GSI_SAME_STMT);
+	  args.safe_push (arg);
 	  num_waits++;
 	}
 
diff --git a/gcc/testsuite/c-c++-common/goacc/pr93488.c b/gcc/testsuite/c-c++-common/goacc/pr93488.c
new file mode 100644
index 000..6fddad919d2
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/goacc/pr93488.c
@@ -0,0 +1,22 @@
+/* PR middle-end/93488
+ 
+   Ensure that wait and async arguments can be cast to the correct type
+   without breaking gimple verification.  */
+
+void test()
+{
+  /* int */ unsigned char a = 1;
+  /* int */ unsigned char w = 1;
+
+#pragma acc parallel wait(w) async(a)
+  ;
+#pragma acc kernels wait(w) async(a)
+  ;
+#pragma acc serial wait(w) async(a)
+  ;
+  int data = 0;
+#pragma acc enter data wait(w) async(a) create(data)
+#pragma acc update wait(w) async(a) device(data)
+#pragma acc exit data wait(w) async(a) delete(data)
+#pragma acc wait(w) async(a)
+}

[PATCH] driver: Fix typos in options descriptions

[Lewis Hyatt]

Hello-

While working on adding a new option, I noticed there are some options
(e.g. -fdiagnostics-format) that use the two-column form of the option
description, but separate the first column with space(s) rather than a
tab. This results in the help output looking a bit redundant, since the
option name is repeated:

$ for f in common optimizers params target warnings; do gcc --help=$f; done |
  grep -E '^[ ]*(-[^ ]*)[ ]+\1' | sort

  -Walloc-size-larger-than=   -Walloc-size-larger-than= Warn for calls 
to allocation functions that attempt to allocate objects larger than
  -Walloc-zero-Walloc-zero Warn for calls to allocation 
functions that specify zero bytes.
  -Wno-alloc-size-larger-than -Wno-alloc-size-larger-than Disable 
Walloc-size-larger-than= warning.  Equivalent to Walloc-size-larger-
  -Wno-alloca-larger-than -Wno-alloca-larger-than Disable 
Walloca-larger-than= warning.  Equivalent to Walloca-larger-than= or 
larger.
  -Wno-vla-larger-than-Wno-vla-larger-than Disable Wvla-larger-than= 
warning.  Equivalent to Wvla-larger-than= or larger.  Same as
  -fdebug-prefix-map= -fdebug-prefix-map== Map one directory 
name to another in debug information.
  -fdiagnostics-format=   -fdiagnostics-format=[text|json] Select output 
format.
  -fdisable-  -fdisable-[tree|rtl|ipa]-=range1+range2 
disables an optimization pass.
  -fenable-   -fenable-[tree|rtl|ipa]-=range1+range2 
enables an optimization pass.
  -ffile-prefix-map=  -ffile-prefix-map== Map one directory 
name to another in compilation result.
  -finstrument-functions-exclude-file-list= 
-finstrument-functions-exclude-file-list=filename,...  Do not instrument 
functions listed in files.
  -finstrument-functions-exclude-function-list= 
-finstrument-functions-exclude-function-list=name,...  Do not instrument listed 
functions.
  -foffload-abi=  -foffload-abi=[lp64|ilp32] Set the ABI to use 
in an offload compiler.
  -foffload=  -foffload==  Specify offloading 
targets and options for them.

Is this something that would be desirable to change for GCC 10? Attached
patch would do so, and the output would become instead:

  -Walloc-size-larger-than= Warn for calls to allocation functions that 
attempt to allocate objects larger than the specified number of bytes.
  -Walloc-zeroWarn for calls to allocation functions that 
specify zero bytes.
  -Wno-alloc-size-larger-than Disable Walloc-size-larger-than= warning.  
Equivalent to Walloc-size-larger-than= or larger.  Same as 
-Walloc-size-larger-than=.
  -Wno-alloca-larger-than Disable Walloca-larger-than= warning.  Equivalent 
to Walloca-larger-than= or larger.  Same as -Walloca-larger-than=.
  -Wno-vla-larger-thanDisable Wvla-larger-than= warning.  Equivalent to 
Wvla-larger-than= or larger.  Same as -Wvla-larger-than=.
  -fdebug-prefix-map== Map one directory name to another in debug 
information.
  -fdiagnostics-format=[text|json] Select output format.
  -fdisable-[tree|rtl|ipa]-=range1+range2 Disable an optimization pass.
  -fenable-[tree|rtl|ipa]-=range1+range2 Enable an optimization pass.
  -ffile-prefix-map== Map one directory name to another in 
compilation result.
  -finstrument-functions-exclude-file-list=filename,... Do not instrument 
functions listed in files.
  -finstrument-functions-exclude-function-list=name,... Do not instrument 
listed functions.
  -foffload-abi=[lp64|ilp32]  Set the ABI to use in an offload compiler.
  -foffload== Specify offloading targets and options for them.


For the most part it is just changing spaces to tabs, although in some cases
the two-column form was redundant so I removed the option name from the
beginning of the description instead.

Bootstrapped and regtested on x86-64 linux also. One test failed because it
was explicitly looking for the old output; modified that test too.

-Lewis
gcc/ChangeLog:

2020-01-30  Lewis Hyatt  

* common.opt: Avoid redundancy in the help text.
* config/arc/arc.opt: Likewise.
* config/cr16/cr16.opt: Likewise.

gcc/c-family/ChangeLog:

2020-01-30  Lewis Hyatt  

* c.opt: Avoid redundancy in the help text.

gcc/fortran/ChangeLog:

2020-01-30  Lewis Hyatt  

* lang.opt: Avoid redundancy in the help text.

gcc/testsuite/ChangeLog:

2020-01-30  Lewis Hyatt  

* gcc.misc-tests/help.exp: Adapt to new output for
-Walloc-size-larger-than= option.
commit 6d98063c67de48928b48783dc54eca3b4e64a4e1
Author: Lewis Hyatt 
Date:   Thu Jan 30 12:18:13 2020 -0500

driver: Fix redundant descriptions in options

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 814ed17f7c4..fd760ee9aea 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -309,16 +309,16 @@ Warn on any use of alloca.
 
 Walloc-size-larger-than=
 C ObjC C++ LTO ObjC++ Var(warn_alloc_size_limit) Joined Host_Wide_Int ByteSize Warning Init(HOST_WIDE_INT_MAX)

Re: [PATCH] Fix PowerPC -fstack-clash-protection -mprefixed-addr ICE (PR target/93122)

[Jakub Jelinek]

On Thu, Jan 30, 2020 at 05:14:08PM +0100, Jakub Jelinek wrote:
> On Wed, Jan 22, 2020 at 03:24:54PM +0100, Jakub Jelinek wrote:
> > > It looks like your patch will pessimise code in some cases as well, btw?
> > 
> > No, it will solely turn previous wrong-codes into something that works
> > (== cases where gen_addr3_insn would previously fail).
> > The 1)+2) variant could even improve code, as gen_addr3_insn could succeed
> > even if we currently don't call it (perhaps generate more than one insn,
> > but it still might be better than forcing the constant into register and
> > then performing add that way).
> 
> Here is what I meant as the alternative, i.e. don't check any predicates,
> just gen_add3_insn, if that fails, force rs into register and retry.
> And, add REG_FRAME_RELATED_EXPR note always when we haven't emitted a single
> insn that has rtl exactly matching what we'd add the REG_FRAME_RELATED_EXPR
> with (in that case, dwarf2cfi.c is able to figure it out by itself, no need
> to waste compile time memory).
> 
> Ok for trunk if it passes bootstrap/regtest?

Successfully bootstrapped/regtested on powerpc64{,le}-linux.

> 2020-01-30  Jakub Jelinek  
> 
>   PR target/93122
>   * config/rs6000/rs6000-logue.c
>   (rs6000_emit_probe_stack_range_stack_clash): Always use gen_add3_insn,
>   if it fails, move rs into end_addr and retry.  Add
>   REG_FRAME_RELATED_EXPR note whenever it returns more than one insn or
>   the insn pattern doesn't describe well what exactly happens to
>   dwarf2cfi.c.
> 
>   * gcc.target/powerpc/pr93122.c: New test.

Jakub

[committed] cgraph: Avoid creating multiple *.localalias aliases with the same name [PR93384]

[Jakub Jelinek]

Hi!

The following testcase FAILs on powerpc64le-linux with assembler errors, as we
emit a call to bar.localalias, then .set bar.localalias, bar twice and then
another call to bar.localalias.  The problem is that bar.localalias can be 
created
at various stages and e.g. ipa-pure-const can slightly adjust the original decl,
so that the existing bar.localalias isn't considered usable (different
flags_from_decl_or_type).  In that case, we'd create another bar.localalias, 
which
clashes with the existing name.

Fixed by retrying with another name if it is already present.  The various 
localalias
aliases shouldn't be that many, from different partitions they would be lto_priv
suffixed and in most cases they would already have the same 
type/flags/attributes.

Bootstrapped/regtested on powerpc64{,le}-linux, acked by Honza on IRC,
committed to trunk.

2020-01-30  Jakub Jelinek  

PR lto/93384
* symtab.c (symtab_node::noninterposable_alias): If localalias
already exists, but is not usable, append numbers after it until
a unique name is found.  Formatting fix.

* gcc.dg/lto/pr93384_0.c: New test.
* gcc.dg/lto/pr93384_1.c: New file.

--- gcc/symtab.c.jj 2020-01-14 09:23:17.648790352 +0100
+++ gcc/symtab.c2020-01-30 18:14:00.702919471 +0100
@@ -1864,7 +1864,7 @@ symtab_node::noninterposable_alias (void
   symtab_node *node = ultimate_alias_target ();
   gcc_assert (!node->alias && !node->weakref);
   node->call_for_symbol_and_aliases (symtab_node::noninterposable_alias,
-  (void *)_node, true);
+(void *)_node, true);
   if (new_node)
 return new_node;
 
@@ -1875,7 +1875,17 @@ symtab_node::noninterposable_alias (void
   /* Otherwise create a new one.  */
   new_decl = copy_node (node->decl);
   DECL_DLLIMPORT_P (new_decl) = 0;
-  DECL_NAME (new_decl) = clone_function_name (node->decl, "localalias");
+  tree name = clone_function_name (node->decl, "localalias");
+  if (!flag_wpa)
+{
+  unsigned long num = 0;
+  /* In the rare case we already have a localalias, but the above
+node->call_for_symbol_and_aliases call didn't find any suitable,
+iterate until we find one not used yet.  */
+  while (symtab_node::get_for_asmname (name))
+   name = clone_function_name (node->decl, "localalias", num++);
+}
+  DECL_NAME (new_decl) = name;
   if (TREE_CODE (new_decl) == FUNCTION_DECL)
 DECL_STRUCT_FUNCTION (new_decl) = NULL;
   DECL_INITIAL (new_decl) = NULL;
--- gcc/testsuite/gcc.dg/lto/pr93384_0.c.jj 2020-01-30 18:18:31.985868078 
+0100
+++ gcc/testsuite/gcc.dg/lto/pr93384_0.c2020-01-30 18:06:44.784429548 
+0100
@@ -0,0 +1,12 @@
+/* PR lto/93384 */
+/* { dg-lto-do link } */
+/* { dg-require-effective-target fpic } */
+/* { dg-require-effective-target shared } */
+/* { dg-lto-options { { -O2 -flto -ffat-lto-objects -fpic 
-fno-semantic-interposition } } } */
+/* { dg-extra-ld-options { -shared -flto-partition=none } } */
+
+void bar (void);
+__attribute__((noipa)) void quux (int x) { if (x == 5) bar (); }
+__attribute__((noipa, noreturn)) void foo (void) { while (1) ; }
+__attribute__((noinline)) void bar (void) { asm (""); quux (7); foo (); }
+void baz (int x) { if (x) bar (); }
--- gcc/testsuite/gcc.dg/lto/pr93384_1.c.jj 2020-01-30 18:18:35.094821645 
+0100
+++ gcc/testsuite/gcc.dg/lto/pr93384_1.c2020-01-30 17:58:58.061373326 
+0100
@@ -0,0 +1,2 @@
+extern void bar (void);
+void qux (int x) { if (!x) bar (); }


Jakub

[committed] combine: Punt on out of range rotate counts [PR93505]

[Jakub Jelinek]

Hi!

What happens on this testcase is with the out of bounds rotate we get:
Trying 13 -> 16:
   13: r129:SI=r132:DI#0<-<0x20
   REG_DEAD r132:DI
   16: r123:DI=r129:SI<0
   REG_DEAD r129:SI
Successfully matched this instruction:
(set (reg/v:DI 123 [  ])
 (const_int 0 [0]))
during combine.  So, perhaps we could also change simplify-rtx.c to punt
if it is out of bounds rather than trying to optimize anything.
Or, but probably GCC11 material, if we decide that ROTATE/ROTATERT doesn't
have out of bounds counts or introduce targetm.rotate_truncation_mask,
we should truncate the argument instead of punting.
Punting is better for backports though.

Bootstrapped/regtested on powerpc64{,le}-linux, preapproved by Segher in the
PR, committed to trunk.

2020-01-30  Jakub Jelinek  

PR middle-end/93505
* combine.c (simplify_comparison) : Punt on out of range
rotate counts.

* gcc.c-torture/compile/pr93505.c: New test.

--- gcc/combine.c.jj2020-01-12 11:54:36.222416290 +0100
+++ gcc/combine.c   2020-01-30 15:08:13.607205111 +0100
@@ -12410,7 +12410,8 @@ simplify_comparison (enum rtx_code code,
 bit.  This will be converted into a ZERO_EXTRACT.  */
  if (const_op == 0 && sign_bit_comparison_p
  && CONST_INT_P (XEXP (op0, 1))
- && mode_width <= HOST_BITS_PER_WIDE_INT)
+ && mode_width <= HOST_BITS_PER_WIDE_INT
+ && UINTVAL (XEXP (op0, 1)) < mode_width)
{
  op0 = simplify_and_const_int (NULL_RTX, mode, XEXP (op0, 0),
(HOST_WIDE_INT_1U
--- gcc/testsuite/gcc.c-torture/compile/pr93505.c.jj2020-01-30 
15:23:19.919634317 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr93505.c   2020-01-30 
15:22:59.537939346 +0100
@@ -0,0 +1,15 @@
+/* PR middle-end/93505 */
+
+unsigned a;
+
+unsigned
+foo (unsigned x)
+{
+  unsigned int y = 32 - __builtin_bswap64 (-a);
+  /* This would be UB (x << 32) at runtime.  Ensure we don't
+ invoke UB in the compiler because of that (visible with
+ bootstrap-ubsan).  */
+  x = x << y | x >> (-y & 31);
+  x >>= 31;
+  return x;
+}


Jakub

[COMMITTED] c++: Fix -Wtype-limits in templates.

[Jason Merrill]

When instantiating a template tsubst_copy_and_build suppresses -Wtype-limits
warnings about e.g. == always being false because it might not always be
false for an instantiation with other template arguments.  But we should
warn if the operands don't depend on template arguments.

I also tried giving these warnings during parsing of the template, but that had
issues with NON_DEPENDENT_EXPR that I don't feel comfortable messing with in
stage 4.

Tested x86_64-pc-linux-gnu, applying to trunk.

PR c++/82521
* pt.c (tsubst_copy_and_build) [EQ_EXPR]: Only suppress warnings if
the expression was dependent before substitution.
---
 gcc/cp/pt.c   | 12 
 gcc/testsuite/g++.dg/warn/Wtype-limits3.C | 13 +
 2 files changed, 21 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/Wtype-limits3.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 416ff63ca3c..40ff3c3a089 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -19279,10 +19279,14 @@ tsubst_copy_and_build (tree t,
 case MEMBER_REF:
 case DOTSTAR_EXPR:
   {
-   warning_sentinel s1(warn_type_limits);
-   warning_sentinel s2(warn_div_by_zero);
-   warning_sentinel s3(warn_logical_op);
-   warning_sentinel s4(warn_tautological_compare);
+   /* If T was type-dependent, suppress warnings that depend on the range
+  of the types involved.  */
+   bool was_dep = uses_template_parms (t);
+   warning_sentinel s1(warn_type_limits, was_dep);
+   warning_sentinel s2(warn_div_by_zero, was_dep);
+   warning_sentinel s3(warn_logical_op, was_dep);
+   warning_sentinel s4(warn_tautological_compare, was_dep);
+
tree op0 = RECUR (TREE_OPERAND (t, 0));
tree op1 = RECUR (TREE_OPERAND (t, 1));
tree r = build_x_binary_op
diff --git a/gcc/testsuite/g++.dg/warn/Wtype-limits3.C 
b/gcc/testsuite/g++.dg/warn/Wtype-limits3.C
new file mode 100644
index 000..b9059ac488e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wtype-limits3.C
@@ -0,0 +1,13 @@
+// PR c++/82521
+// { dg-additional-options "-Wtype-limits" }
+
+template 
+const char * g(const unsigned char value)
+{
+  return value == -1 ? "-1" : "no"; // { dg-warning "always false" }
+}
+
+int main()
+{
+  g(-1);
+}

base-commit: b0e9b18ed432c4b7cb9a4b963b65911b4c103cbe
-- 
2.18.1

Re: [PATCH] simplify-rtx: Extend (truncate (*extract ...)) fold [PR87763]

[Richard Sandiford]

Jeff Law  writes:
> On Wed, 2020-01-29 at 19:18 +, Richard Sandiford wrote:
>> Andreas Schwab  writes:
>> > On Jan 27 2020, Richard Sandiford wrote:
>> > 
>> > >  * simplify-rtx.c (simplify_truncation): Extend sign/zero_extract
>> > >  simplification to handle subregs as well as bare regs.
>> > 
>> > That breaks gcc.target/m68k/pr39726.c
>> 
>> Gah.  Jeff pointed out off-list that it also broke
>> gcc.target/sh/pr64345-1.c on sh3-linux-gnu.  It didn't look like either
>> of them would be fixable in an acceptably non-invasive and unhacky way,
>> so I've reverted the patch "for now".
> I would have considered letting those two targets regress those tests
> to move forward on 87763.  aarch64 is (IMHO) more important than the sh
> and m68k combined ;-)  It also seems to me that your patch generates
> better RTL and that we could claim that a port that regresses on code
> quality needs its port maintainer to step in and fix the port.
>
> WRT the m68k issue I'd think it could be fixed by twiddling
> cbranchsi4_btst_reg_insn_1 to accept a mode iterator on the
> zero_extract and making some minor adjustments in its output code. 
> Something like the attached.  I haven't tested it in any real way and
> haven't really thought about whether or not it does the right thing for
> a MEM operand.

It just feels like this is breaking the contract with extv and extzv,
where all *_extracts are supposed to be in the mode that those patterns
accept.  My i386 patch was doing that too TBH, I was just being
optimistic given that QImode was already handled by that pattern. :-)

So IMO my patch has too many knock-on effects for it to be suitable for
stage 4.  While we have make_extraction, we probably have to leave this
kind of expression untouched.

For AArch64 I was planning on adding a new pattern to match the
(subreg:SI (zero_extract:DI ...)) -- as one of the comments in the
PR suggested -- but with the subreg matched via subreg_lowpart_operator,
to make it endian-safe.  This is similar in spirit to the new i686
popcount patterns.

Thanks,
Richard

>
> I'd be surprised if the SH fix wasn't similar, but I know almost
> nothing about the SH implementation.
>
> Jeff

Re: [patch, fortan] PR87103 - [OOP] ICE in gfc_new_symbol() due to overlong symbol name

[Andrew Benson]

Thanks Bernhard - this is now committed:

https://gcc.gnu.org/g:004ac7b780308dc899e565b887c7def0a6e100f2

On Thursday, January 30, 2020 5:27:55 PM PST Bernhard Reutner-Fischer wrote:
> On 29 January 2020 21:19:52 CET, Andrew Benson  
wrote:
> >I think this patch is still waiting to be applied. I checked that it
> >applies
> >against trunk (with line offsets) and reg tests cleanly and posted an
> >updated
> >version (diff'd against current trunk) at:
> >
> >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87103#c7
> >
> >I'm happy to go ahead and commit this if Bernhard is ok with me doing
> >so.
> 
> Please go ahead and push it.
> Many thanks in advance and sorry for the delay!
> thanks,
> 
> >-Andrew
> >
> >On Wednesday, August 28, 2019 9:00:36 PM PST Bernhard Reutner-Fischer
> >
> >wrote:
> >> On Fri, 23 Aug 2019 17:17:37 -0700
> >> 
> >> Andrew Benson  wrote:
> >> > This PR is still open - I re-tested the patch on the current trunk.
> >
> >The
> >
> >> > patch still applies with some line offsets (I've attached the
> >
> >updated
> >
> >> > patch) and regtests cleanly. It would be very helpful to me to get
> >
> >this
> >
> >> > patch committed if possible.
> >> 
> >> I think Jerry ACKed the patch back then. I'll try to find the time to
> >> commit it maybe during one of the coming weekends unless someone else
> >> beats me to it..
> >> 
> >> Thanks for the reminder!
> >> Bernhard
> >> 
> >> > Thanks,
> >> > Andrew
> >> > 
> >> > On Wednesday, September 5, 2018 12:35:04 PM PDT Bernhard
> >
> >Reutner-Fischer
> >
> >> > wrote:
> >> > > On Wed, 5 Sep 2018 at 03:30, Jerry DeLisle
> >
> >
> >
> >wrote:
> >> > > > On 09/04/2018 10:43 AM, Bernhard Reutner-Fischer wrote:
> >> > > > > On Tue, 4 Sep 2018 at 18:43, Andrew Benson
> >> > > > > 
> >> > 
> >> > wrote:
> >> > > > >> As suggested by Janus, PR87103 is easily fixed by the
> >
> >attached
> >
> >> > > > >> patch
> >> > > > >> which
> >> > > > >> increases GFC_MAX_SYMBOL_LEN to 76 (sufficient to hold the
> >
> >maximum
> >
> >> > > > >> allowed F08 symbol length of 63, plus a null terminator,
> >
> >plus the
> >
> >> > > > >> "__tmp_class_" prefix).> >
> >> > > > > 
> >> > > > > This is so much wrong.
> >> > > > > Note that this will be fixed properly by the changes
> >
> >contained in
> >
> >> > > > > the
> >
> >https://gcc.gnu.org/git/?p=gcc.git;a=shortlog;h=refs/heads/aldot/for
> >
> >> > > > > tran
> >> > > > > -fe-stringpool branch.
> >> > > > > There we keep the GFC_MAX_SYMBOL_LEN at 63 proper but use an
> >> > > > > internal
> >> > > > > buffer double that size which in turn is sufficient to hold
> >
> >all
> >
> >> > > > > compiler-generated identifiers.
> >> > > > > See gfc_get_string() even in current TOT.
> >> > > > > 
> >> > > > > Maybe we should bite the bullet and start to merge the
> >
> >stringpool
> >
> >> > > > > changes now instead of this hack?
> >> > > > 
> >> > > > It all makes sense to me, please proceed. (my 2 cents worth)
> >> > > 
> >> > > Ok so i will reread the fortran-fe-stringpool series and submit
> >
> >it
> >
> >> > > here for review.
> >> > > 
> >> > > Let's return to the issue at hand for a moment, though.
> >> > > I tested the attached alternate fix on top of the
> >> > > fortran-fe-stringpool branch where it fixes PR87103.
> >> > > Maybe somebody has spare cycles to test it on top of current
> >
> >trunk?
> >
> >> > > thanks,
> >> > > 
> >> > > [PATCH,FORTRAN] PR87103: Remove max symbol length check from
> >> > > gfc_new_symbol
> >> > > 
> >> > > gfc_match_name does check for too long names already. Since
> >> > > gfc_new_symbol is also called for symbols with internal names
> >
> >containing
> >
> >> > > compiler-generated prefixes, these internal names can easily
> >
> >exceed the
> >
> >> > > max_identifier_length mandated by the standard.
> >> > > 
> >> > > gcc/fortran/ChangeLog
> >> > > 
> >> > > 2018-09-04  Bernhard Reutner-Fischer  
> >> > > 
> >> > > PR fortran/87103
> >> > > * expr.c (gfc_check_conformance): Check vsnprintf for truncation.
> >> > > * iresolve.c (gfc_get_string): Likewise.
> >> > > * symbol.c (gfc_new_symbol): Remove check for maximum symbol
> >> > > name length.  Remove redundant 0 setting of new calloc()ed
> >> > > gfc_symbol.


-- 

* Andrew Benson: http://users.obs.carnegiescience.edu/abenson/contact.html

* Galacticus: https://github.com/galacticusorg/galacticus

Re: [RFA] [c/88660] Fix bogus set-but-unused warning

[Joseph Myers]

On Wed, 29 Jan 2020, Jeff Law wrote:

> In the last major change in this code was ~5 years ago and twiddled the
> handling of the switch expression to call convert_lvalue_to_rvalue.
> 
> The last argument to that function indicates whether or not we should
> mark the switch expression as a use of the object.  We're currently
> passing in "false" so the object doesn't get marked and we get the
> bogus warning.
> 
> The obvious fix is to pass in "true", which is what the proposed patch
> does.  If there's a reason we can't/shouldn't do that in this case we
> could call mark_exp_read on the switch expression at some point other
> point.
> 
> Bootstrapped and regression tested on x86_64.  OK for the trunk?

I think the code passes false simply because it was a straight conversion 
from previous code that (probably erroneously) didn't call mark_exp_read.  
The patch is OK.

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

Re: [GCC][BUG][Aarch64][ARM] (PR93300) Fix ICE due to BFmode placement in GET_MODES_WIDER chain.

[Stam Markianos-Wright]

On 1/30/20 10:01 AM, Richard Sandiford wrote:

Stam Markianos-Wright  writes:

On 1/29/20 12:42 PM, Richard Sandiford wrote:

Stam Markianos-Wright  writes:

Hi all,

This fixes:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93300

Genmodes.c was generating the "wider_mode" chain as follows:

HF -> BF -> SF - > DF -> TF -> VOID

This caused issues in some rare cases where conversion between modes
was needed, such as the above PR93300 where BFmode was being picked up
as a valid mode for:

optabs.c:prepare_float_lib_cmp

which then led to the ICE at expr.c:convert_mode_scalar.


Hi Richard,



Can you go into more details about why this chain was a problem?
Naively, it's the one I'd have expected: HF should certainly have
priority over BF,


Is that because functionally it looks like genmodes puts things in reverse
alphabetical order if all else is equal? (If I'm reading the comment about
MODE_RANDOM, MODE_CC correctly)


but BF coming before SF doesn't seem unusual in itself.

I'm not saying the patch is wrong.  It just wasn't clear why it was
right either.


Yes, I see what you mean. I'll go through my thought process here:

In investigating the ICE PR93300 I found that the diversion from pre-bf16
behaviour was specifically at `optabs.c:prepare_float_lib_cmp`, where a
`FOR_EACH_MODE_FROM (mode, orig_mode)` is used to then go off and generate
library calls for conversions.

This was then being caught further down by the gcc_assert at expr.c:325 where
GET_MODE_PRECISION (from_mode) was equal to GET_MODE_PRECISION (to_mode) because
it was trying to emit a HF->BF conversion libcall as `bl __extendhfbf2` (which
is what happened if i removed the gcc_assert at expr.c:325)

With BFmode being a target-defined mode, I didn't want to add something like `if
(mode != BFmode)` to specifically exclude BFmode from being selected for this.
(and there's nothing different between HFmode and BFmode here to allow me to
make this distinction?)

Also I couldn't find anywhere where the target back-end is not consulted for a
"is this supported: yes/no" between the `FOR_EACH_MODE_FROM` loop and the
libcall being created later on as __extendhfbf2.


Yeah, prepare_float_lib_cmp just checks for libfuncs rather than
calling target hooks directly.  The libfuncs themselves are under
the control of the target though.

By default we assume all float modes have associated libfuncs.
It's then up to the target to remove functions that don't exist
(or redirect them to other functions).  So I think we need to remove
BFmode libfuncs in arm_init_libfuncs in the same way as we currently
do for HFmode.

I guess we should also nullify the conversion libfuncs for BFmode,
not just the arithmetic and comparison ones.


Ahhh now this works, thank you for the suggestion!

I was aware of arm_init_libfuncs, but I had not realised that returning NULL 
would have the desired effect for us, in this case. So I have essentially rolled 
back the whole previous version of the patch and done this in the new diff.
It seems to have fixed the ICE and I am currently in the process of regression 
testing!


Thank you!
Stam



Thanks,
Richard


Finally, because we really don't want __bf16 to be in the same "conversion rank"
as standard float modes for things like automatic promotion, this seemed like a
reasonable solution to that problem :)

Let me know of your thoughts!

Cheers,
Stam


diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index c47fc232f39..18055d4a75e 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -2643,6 +2643,30 @@ arm_init_libfuncs (void)
 default:
   break;
 }
+
+  /* For all possible libcalls in BFmode, return NULL.  */
+  /* Conversions.  */
+  set_conv_libfunc (trunc_optab, BFmode, HFmode, (NULL));
+  set_conv_libfunc (sext_optab, HFmode, BFmode, (NULL));
+  set_conv_libfunc (trunc_optab, BFmode, SFmode, (NULL));
+  set_conv_libfunc (sext_optab, SFmode, BFmode, (NULL));
+  set_conv_libfunc (trunc_optab, BFmode, DFmode, (NULL));
+  set_conv_libfunc (sext_optab, DFmode, BFmode, (NULL));
+
+  /* Arithmetic.  */
+  set_optab_libfunc (add_optab, BFmode, NULL);
+  set_optab_libfunc (sdiv_optab, BFmode, NULL);
+  set_optab_libfunc (smul_optab, BFmode, NULL);
+  set_optab_libfunc (neg_optab, BFmode, NULL);
+  set_optab_libfunc (sub_optab, BFmode, NULL);
+
+  /* Comparisons.  */
+  set_optab_libfunc (eq_optab, BFmode, NULL);
+  set_optab_libfunc (ne_optab, BFmode, NULL);
+  set_optab_libfunc (lt_optab, BFmode, NULL);
+  set_optab_libfunc (le_optab, BFmode, NULL);
+  set_optab_libfunc (ge_optab, BFmode, NULL);
+  set_optab_libfunc (gt_optab, BFmode, NULL);
+  set_optab_libfunc (unord_optab, BFmode, NULL);
 
   /* Use names prefixed with __gnu_ for fixed-point helper functions.  */
   {

Re: [PATCH] simplify-rtx: Extend (truncate (*extract ...)) fold [PR87763]

[Jeff Law]

On Thu, 2020-01-30 at 18:27 +0100, Jakub Jelinek wrote:
> On Thu, Jan 30, 2020 at 10:23:35AM -0700, Jeff Law wrote:
> > diff --git a/gcc/config/m68k/m68k.md b/gcc/config/m68k/m68k.md
> > index 8e35357ea23..78c4cbe4753 100644
> > --- a/gcc/config/m68k/m68k.md
> > +++ b/gcc/config/m68k/m68k.md
> > @@ -644,12 +644,12 @@
> >return m68k_output_branch_integer (code);
> >  })
> >  
> > -(define_insn "cbranchsi4_btst_reg_insn_1"
> > +(define_insn "cbranch4_btst_reg_insn_1"
> >[(set (pc)
> > (if_then_else (match_operator 0 "equality_comparison_operator"
> > -  [(zero_extract:SI (match_operand:SI 1 
> > "nonimmediate_operand" "do,dQ")
> > -(const_int 1)
> > -(match_operand:SI 2 
> > "const_int_operand" "n,n"))
> > +  [(zero_extract:I (match_operand:I 1 
> > "nonimmediate_operand" "do,dQ")
> > +   (const_int 1)
> > +   (match_operand:I 2 "const_int_operand" 
> > "n,n"))
> > (const_int 0)])
> >   (label_ref (match_operand 3 ""))
> >   (pc)))]
> > @@ -665,8 +665,9 @@
> >  }
> >else
> >  {
> > -  operands[2] = GEN_INT (31 - INTVAL (operands[2]));
> > -  code = m68k_output_btst (operands[2], operands[1], code, 31);
> > +  operands[2] = GEN_INT (GET_MODE_BITSIZE (GET_MODE (operands[1]))
> > +- INTVAL (operands[2]) - 1);
> > +  code = m68k_output_btst (operands[2], operands[1], code, 
> > GET_MODE_BITSIZE (GET_MODE (operands[1])) - 1);
> 
> s/GET_MODE (operands[1])/mode/g ?
> Also, the last line is too long.
It's not meant for inclusion as-is, but to show how we might fix this
and allow us to move forward on 87763.
Jeff

Re: [PATCH] simplify-rtx: Extend (truncate (*extract ...)) fold [PR87763]

[Jakub Jelinek]

On Thu, Jan 30, 2020 at 10:23:35AM -0700, Jeff Law wrote:
> diff --git a/gcc/config/m68k/m68k.md b/gcc/config/m68k/m68k.md
> index 8e35357ea23..78c4cbe4753 100644
> --- a/gcc/config/m68k/m68k.md
> +++ b/gcc/config/m68k/m68k.md
> @@ -644,12 +644,12 @@
>return m68k_output_branch_integer (code);
>  })
>  
> -(define_insn "cbranchsi4_btst_reg_insn_1"
> +(define_insn "cbranch4_btst_reg_insn_1"
>[(set (pc)
>   (if_then_else (match_operator 0 "equality_comparison_operator"
> -[(zero_extract:SI (match_operand:SI 1 
> "nonimmediate_operand" "do,dQ")
> -  (const_int 1)
> -  (match_operand:SI 2 
> "const_int_operand" "n,n"))
> +[(zero_extract:I (match_operand:I 1 
> "nonimmediate_operand" "do,dQ")
> + (const_int 1)
> + (match_operand:I 2 "const_int_operand" 
> "n,n"))
>   (const_int 0)])
> (label_ref (match_operand 3 ""))
> (pc)))]
> @@ -665,8 +665,9 @@
>  }
>else
>  {
> -  operands[2] = GEN_INT (31 - INTVAL (operands[2]));
> -  code = m68k_output_btst (operands[2], operands[1], code, 31);
> +  operands[2] = GEN_INT (GET_MODE_BITSIZE (GET_MODE (operands[1]))
> +  - INTVAL (operands[2]) - 1);
> +  code = m68k_output_btst (operands[2], operands[1], code, 
> GET_MODE_BITSIZE (GET_MODE (operands[1])) - 1);

s/GET_MODE (operands[1])/mode/g ?
Also, the last line is too long.

Jakub

Re: [PATCH] simplify-rtx: Extend (truncate (*extract ...)) fold [PR87763]

[Jeff Law]

On Wed, 2020-01-29 at 19:18 +, Richard Sandiford wrote:
> Andreas Schwab  writes:
> > On Jan 27 2020, Richard Sandiford wrote:
> > 
> > >   * simplify-rtx.c (simplify_truncation): Extend sign/zero_extract
> > >   simplification to handle subregs as well as bare regs.
> > 
> > That breaks gcc.target/m68k/pr39726.c
> 
> Gah.  Jeff pointed out off-list that it also broke
> gcc.target/sh/pr64345-1.c on sh3-linux-gnu.  It didn't look like either
> of them would be fixable in an acceptably non-invasive and unhacky way,
> so I've reverted the patch "for now".
I would have considered letting those two targets regress those tests
to move forward on 87763.  aarch64 is (IMHO) more important than the sh
and m68k combined ;-)  It also seems to me that your patch generates
better RTL and that we could claim that a port that regresses on code
quality needs its port maintainer to step in and fix the port.

WRT the m68k issue I'd think it could be fixed by twiddling
cbranchsi4_btst_reg_insn_1 to accept a mode iterator on the
zero_extract and making some minor adjustments in its output code. 
Something like the attached.  I haven't tested it in any real way and
haven't really thought about whether or not it does the right thing for
a MEM operand.

I'd be surprised if the SH fix wasn't similar, but I know almost
nothing about the SH implementation.

Jeff
diff --git a/gcc/config/m68k/m68k.md b/gcc/config/m68k/m68k.md
index 8e35357ea23..78c4cbe4753 100644
--- a/gcc/config/m68k/m68k.md
+++ b/gcc/config/m68k/m68k.md
@@ -644,12 +644,12 @@
   return m68k_output_branch_integer (code);
 })
 
-(define_insn "cbranchsi4_btst_reg_insn_1"
+(define_insn "cbranch4_btst_reg_insn_1"
   [(set (pc)
 	(if_then_else (match_operator 0 "equality_comparison_operator"
-		   [(zero_extract:SI (match_operand:SI 1 "nonimmediate_operand" "do,dQ")
-	 (const_int 1)
-	 (match_operand:SI 2 "const_int_operand" "n,n"))
+		   [(zero_extract:I (match_operand:I 1 "nonimmediate_operand" "do,dQ")
+	(const_int 1)
+	(match_operand:I 2 "const_int_operand" "n,n"))
 			(const_int 0)])
 		  (label_ref (match_operand 3 ""))
 		  (pc)))]
@@ -665,8 +665,9 @@
 }
   else
 {
-  operands[2] = GEN_INT (31 - INTVAL (operands[2]));
-  code = m68k_output_btst (operands[2], operands[1], code, 31);
+  operands[2] = GEN_INT (GET_MODE_BITSIZE (GET_MODE (operands[1]))
+			 - INTVAL (operands[2]) - 1);
+  code = m68k_output_btst (operands[2], operands[1], code, GET_MODE_BITSIZE (GET_MODE (operands[1])) - 1);
 }
   return m68k_output_branch_integer (code);
 }

Re: [PATCH] doc: clarify the situation with pointer arithmetic

[Uecker, Martin]

Am Donnerstag, den 30.01.2020, 16:50 + schrieb Michael Matz:
> Hi,
> 
> On Thu, 30 Jan 2020, Uecker, Martin wrote:
> 
> > > guarantees face serious implementation difficulties I think
> > > so the only alternative to PVNI (which I think is implementable
> > > but at a optimization opportunity cost) is one that makes
> > > two pointers with the same value always have the same
> > > provenance (and otherwise make the behavior undefined).
> > 
> > This would need to come with precise rules about
> > when the occurance of two such pointers is UB,
> > e.g. comparisons of such pointers, or that
> > two such pointers are cast to int in the same
> > execution.
> > 
> > The mere existance of such pointers should be
> > quite common and should not already be UB.
> > 
> > But I am uncomfortable with the idea that
> > comparison of pointers is always allowed except
> > for some special case which then is UB. This
> > might cause are and very difficult to find bugs.
> 
> As Richi said, the comparison itself wouldn't be UB, all comparisons would 
> be allowed.  But _if_ the pointers compare equal, they must have same (or 
> overlapping) provenance (i.e. when they have not, then _that_ is UB).

Sorry, I still don't get it.  In the following example,

int a[1];
int b[1];

it is often the case that [1] and [0] compare equal
because they have the same address but the pointer
have different provenance.  

Or does there need to be an actual evaluation of a comparison
operations? In this case, I do not see the difference to what
I said.


Best,
Martin

> > > > Others proposed to make the result of the comparison unspecified, 
> > > > but I think this does not help.
> > > 
> > > Indeed.  It's not unspecified, it's known to evaluate to false. I 
> > > think there's existing wording in the standard that allows it to 
> > > evaluate to true for pointers one-after-the-object, that would need to 
> > > be changed of course.
> > 
> > The problem is that if the comparison if not optimized
> > and the pointers have the same address, then it would
> > evaluate to true at run-time. If I understand correctly,
> > you somehow want to make this case be UB, but I haven't
> > quite understood how (if it is not the comparison of such
> > pointers that invokes UB).
> 
> By saying something like "if two pointers compare equal they must have the 
> same provenance, otherwise the behaviour is undefined".

> (I don't know if this definition would or would not help with the problems 
> PVNI poses to compilers).
> 
> 
> Ciao,
> Michael.

Re: [PATCH] doc: clarify the situation with pointer arithmetic

[Michael Matz]

Hi,

On Thu, 30 Jan 2020, Michael Matz wrote:

> > and the pointers have the same address, then it would evaluate to true 
> > at run-time. If I understand correctly, you somehow want to make this 
> > case be UB, but I haven't quite understood how (if it is not the 
> > comparison of such pointers that invokes UB).
> 
> By saying something like "if two pointers compare equal they must have 
> the same provenance, otherwise the behaviour is undefined".
> 
> (I don't know if this definition would or would not help with the 
> problems PVNI poses to compilers).

Or, actually I know at least one case.  The problem with allowing 
value-equivalent pointers to have non-overlapping provenance is the 
following: many of the compiler optimizations are based on as-if rules.  
Now, if it's very easy for users to detect certain situations, that means 
that the as-if rules can't be invoked as often.  In this specific 
instance, if the user writes a program where the compiler would optimize 
mem accesses based on non-overlapping provenance (e.g. a stored value is 
propagated downwards over a store of different provenance), and then 
somewhere else also compares these non-overlapping pointers for equality, 
and then, if they are equal prints out "hah! invalid optimization 
detected", and the outcome of the comparison of non-overlapping pointers 
weren't left unspecified, then that's the reason why the compiler would 
have to globally disable the first optimization (at least when it can't 
prove that there aren't any such comparisons).  Ideally we don't want that 
:)


Ciao,
Michael.

Re: [PATCH] gcc: Add new configure options to allow static libraries to be selected

[Andrew Burgess]

* Iain Sandoe  [2020-01-30 15:21:08 +]:

> >> Can you clarify why there’s no need to match the configury changes in 
> >> libcpp
> >> / gcc / libstdc++ ?
> > 
> > This is the same issue that Tobias pointed out, and was a result of me
> > incorrectly trying to regenerate the configure files.
> 
> .. but it seems that the combination of the two things produced the 
> configuration
> mismatch ..
> 
> > Let me know if this resolves the problems you're seeing and if it does
> > I'll prepare it for submission.
> 
> I did a smoke test on Darwin19 and the amended patch allowed bootstrap to
>  complete. ICONV is seen in both libcpp and gcc (that’s not an exhaustive 
> check
> and the testsuite is still running).
> 
> I’ll add this to my overnight runs - “overnight” means results later tomorrow 
> for the
> slower targets - but it looks encouraging.

Thanks, for checking this so quickly.

I'll prepare this as a proper patch and post this again later tonight
for review, but if your results are good tomorrow then I'll go ahead
and push this.

Thanks,
Andrew

[committed, amdgcn] Add LTGT support

[Andrew Stubbs]

This patch adds support for the LTGT FP comparison operator that was 
previously missing from the backend, and apparently not optional (unlike 
UNLE etc).


It wasn't that we couldn't have it (also unlike UNLE), we just didn't.

Besides just correcting an omission, this also fixes an ICE in testcase 
gcc.dg/pr81228.c.


Andrew
Add LTGT operator support for amdgcn

Fixes ICE in testcase gcc.dg/pr81228.c

2020-01-30  Andrew Stubbs  

	gcc/
	* config/gcn/gcn.c (print_operand): Handle LTGT.
	* config/gcn/predicates.md (gcn_fp_compare_operator): Allow ltgt.

diff --git a/gcc/config/gcn/gcn.c b/gcc/config/gcn/gcn.c
index c78df1d5e3e..a39e9f3fbd6 100644
--- a/gcc/config/gcn/gcn.c
+++ b/gcc/config/gcn/gcn.c
@@ -5929,6 +5929,9 @@ print_operand (FILE *file, rtx x, int code)
 	  case UNORDERED:
 	s = "_u_";
 	break;
+	  case LTGT:
+	s = "_lg_";
+	break;
 	  default:
 	output_operand_lossage ("invalid %%xn code");
 	return;
diff --git a/gcc/config/gcn/predicates.md b/gcc/config/gcn/predicates.md
index 2f904b1f131..7bf763a4ba5 100644
--- a/gcc/config/gcn/predicates.md
+++ b/gcc/config/gcn/predicates.md
@@ -165,7 +165,7 @@
   (match_code "eq,ne,gt,ge,lt,le,gtu,geu,ltu,leu"))
 
 (define_predicate "gcn_fp_compare_operator"
-  (match_code "eq,ne,gt,ge,lt,le,gtu,geu,ltu,leu,ordered,unordered"))
+  (match_code "eq,ne,gt,ge,lt,le,gtu,geu,ltu,leu,ordered,unordered,ltgt"))
 
 (define_predicate "unary_operator"
   (match_code "not,popcount"))

Re: [PATCH] doc: clarify the situation with pointer arithmetic

[Michael Matz]

Hi,

On Thu, 30 Jan 2020, Uecker, Martin wrote:

> > guarantees face serious implementation difficulties I think
> > so the only alternative to PVNI (which I think is implementable
> > but at a optimization opportunity cost) is one that makes
> > two pointers with the same value always have the same
> > provenance (and otherwise make the behavior undefined).
> 
> This would need to come with precise rules about
> when the occurance of two such pointers is UB,
> e.g. comparisons of such pointers, or that
> two such pointers are cast to int in the same
> execution.
> 
> The mere existance of such pointers should be
> quite common and should not already be UB.
> 
> But I am uncomfortable with the idea that
> comparison of pointers is always allowed except
> for some special case which then is UB. This
> might cause are and very difficult to find bugs.

As Richi said, the comparison itself wouldn't be UB, all comparisons would 
be allowed.  But _if_ the pointers compare equal, they must have same (or 
overlapping) provenance (i.e. when they have not, then _that_ is UB).

> > > Others proposed to make the result of the comparison unspecified, 
> > > but I think this does not help.
> > 
> > Indeed.  It's not unspecified, it's known to evaluate to false. I 
> > think there's existing wording in the standard that allows it to 
> > evaluate to true for pointers one-after-the-object, that would need to 
> > be changed of course.
> 
> The problem is that if the comparison if not optimized
> and the pointers have the same address, then it would
> evaluate to true at run-time. If I understand correctly,
> you somehow want to make this case be UB, but I haven't
> quite understood how (if it is not the comparison of such
> pointers that invokes UB).

By saying something like "if two pointers compare equal they must have the 
same provenance, otherwise the behaviour is undefined".

(I don't know if this definition would or would not help with the problems 
PVNI poses to compilers).


Ciao,
Michael.

Re: [PATCH] Add OpenACC acc_get_property support for AMD GCN

[Andrew Stubbs]

On 30/01/2020 16:08, Thomas Schwinge wrote:

Hi!

Andrew and Frederik, thanks for your emails reminding/educating me about
'snprintf' as well as this HSA fixed-size buffer API.  There doesn't
happen to be something available in the HSA API available so that we
could use 'sizeof [something]' instead of hard-coding '64' etc.?


No, not at present; hsa_agent_get_info_fn is a somewhat generic 
interface that takes an enum and returns a void*. The return type is 
written in the documentation: 
https://rocm-documentation.readthedocs.io/en/latest/ROCm_API_References/ROCr-API.html#rocr-api


However, we don't use the official ROCm header files, because 
dependencies and licenses, so we could invent our own typedefs in hsa.h, 
if we chose. I don't see that doing so would be worth the effort now, or 
maintenance burden later.


I'll let Frederik explain his implementation decisions.

Andrew

[committed] Tweak ssa-dse-26.c test for c6x after recent SRA changes

[Jeff Law]

The recent SRA changes triggered a testsuite regression on the c6x port
for ssa-dse-26.c.

There was always a degree of difference between the targets, for 
constraint_expr_equal.  Depending on target characteristics there may
or may not be DSE opportunities in this function.

More importantly is constraint_equal.  For most targets we remove an
aggregate assignment to "x" and "y".  We could probably do better than
we are, but that would require further improvements to DSE and perhaps 
one of the redundancy eliminators such as FRE or DOM or twiddles in the
SRA heuristics.

On the c6 we actually remove aggregate stores to "a" and "b" which are
arguments.  Perhaps the differences are related to how structs are
passed accounts for this difference.  Anyway, on the c6x we fully
scalarize the "x" and "y" objects, so there's really nothing for DSE to
do with them.

So this change to the test makes the original pattern we scanned for
apply to everything but the c6x and adds a separate pattern that's only
used on the c6x.  That new pattern checks for the opportunities in both
functions.

Committing to the trunk.  I'm also watching for other targets that may
need similar handling as a result of the SRA work.

Jeff


diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 0247d2cd083..269adb29e94 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2020-01-30  Jeff Law  
 
 	PR middle-end/92323
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c
index 836a8092ab9..8abc28baccb 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-26.c
@@ -30,6 +30,13 @@ constraint_equal (struct constraint a, struct constraint b)
 && constraint_expr_equal (a.rhs, b.rhs);
 }
 
-/* { dg-final { scan-tree-dump-times "Deleted dead store: x = " 1 "dse1" } } */
-/* { dg-final { scan-tree-dump-times "Deleted dead store: y = " 1 "dse1" } } */
+/* Most targets should be using this test.  */
+/* { dg-final { scan-tree-dump-times "Deleted dead store: x = " 1 "dse1" { target { ! tic6x-*-* } } } } */
+/* { dg-final { scan-tree-dump-times "Deleted dead store: y = " 1 "dse1" { target { ! tic6x-*-* } } } } */
+
+/* The c6x port generates significantly different gimple which
+   changes the SRA and DSE decisions.   Verify we remove all
+   dead stores.  */
+/* { dg-final { scan-tree-dump-times "Deleted dead store: \[ax\].. = " 2 "dse1" { target tic6x-*-* } } } */
+/* { dg-final { scan-tree-dump-times "Deleted dead store: \[by\].. = " 2 "dse1" { target tic6x-*-* } } } */
 

Re: [patch, fortan] PR87103 - [OOP] ICE in gfc_new_symbol() due to overlong symbol name

[Bernhard Reutner-Fischer]

On 29 January 2020 21:19:52 CET, Andrew Benson  
wrote:
>I think this patch is still waiting to be applied. I checked that it
>applies 
>against trunk (with line offsets) and reg tests cleanly and posted an
>updated 
>version (diff'd against current trunk) at:
>
>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87103#c7
>
>I'm happy to go ahead and commit this if Bernhard is ok with me doing
>so.

Please go ahead and push it.
Many thanks in advance and sorry for the delay!
thanks,
>
>-Andrew
>
>On Wednesday, August 28, 2019 9:00:36 PM PST Bernhard Reutner-Fischer
>wrote:
>> On Fri, 23 Aug 2019 17:17:37 -0700
>> 
>> Andrew Benson  wrote:
>> > This PR is still open - I re-tested the patch on the current trunk.
>The
>> > patch still applies with some line offsets (I've attached the
>updated
>> > patch) and regtests cleanly. It would be very helpful to me to get
>this
>> > patch committed if possible.
>> 
>> I think Jerry ACKed the patch back then. I'll try to find the time to
>> commit it maybe during one of the coming weekends unless someone else
>> beats me to it..
>> 
>> Thanks for the reminder!
>> Bernhard
>> 
>> > Thanks,
>> > Andrew
>> > 
>> > On Wednesday, September 5, 2018 12:35:04 PM PDT Bernhard
>Reutner-Fischer
>> > 
>> > wrote:
>> > > On Wed, 5 Sep 2018 at 03:30, Jerry DeLisle
> 
>wrote:
>> > > > On 09/04/2018 10:43 AM, Bernhard Reutner-Fischer wrote:
>> > > > > On Tue, 4 Sep 2018 at 18:43, Andrew Benson
>> > > > > 
>> > 
>> > wrote:
>> > > > >> As suggested by Janus, PR87103 is easily fixed by the
>attached
>> > > > >> patch
>> > > > >> which
>> > > > >> increases GFC_MAX_SYMBOL_LEN to 76 (sufficient to hold the
>maximum
>> > > > >> allowed F08 symbol length of 63, plus a null terminator,
>plus the
>> > > > >> "__tmp_class_" prefix).> >
>> > > > > 
>> > > > > This is so much wrong.
>> > > > > Note that this will be fixed properly by the changes
>contained in
>> > > > > the
>> > > > >
>https://gcc.gnu.org/git/?p=gcc.git;a=shortlog;h=refs/heads/aldot/for
>> > > > > tran
>> > > > > -fe-stringpool branch.
>> > > > > There we keep the GFC_MAX_SYMBOL_LEN at 63 proper but use an
>> > > > > internal
>> > > > > buffer double that size which in turn is sufficient to hold
>all
>> > > > > compiler-generated identifiers.
>> > > > > See gfc_get_string() even in current TOT.
>> > > > > 
>> > > > > Maybe we should bite the bullet and start to merge the
>stringpool
>> > > > > changes now instead of this hack?
>> > > > 
>> > > > It all makes sense to me, please proceed. (my 2 cents worth)
>> > > 
>> > > Ok so i will reread the fortran-fe-stringpool series and submit
>it
>> > > here for review.
>> > > 
>> > > Let's return to the issue at hand for a moment, though.
>> > > I tested the attached alternate fix on top of the
>> > > fortran-fe-stringpool branch where it fixes PR87103.
>> > > Maybe somebody has spare cycles to test it on top of current
>trunk?
>> > > 
>> > > thanks,
>> > > 
>> > > [PATCH,FORTRAN] PR87103: Remove max symbol length check from
>> > > gfc_new_symbol
>> > > 
>> > > gfc_match_name does check for too long names already. Since
>> > > gfc_new_symbol is also called for symbols with internal names
>containing
>> > > compiler-generated prefixes, these internal names can easily
>exceed the
>> > > max_identifier_length mandated by the standard.
>> > > 
>> > > gcc/fortran/ChangeLog
>> > > 
>> > > 2018-09-04  Bernhard Reutner-Fischer  
>> > > 
>> > > PR fortran/87103
>> > > * expr.c (gfc_check_conformance): Check vsnprintf for truncation.
>> > > * iresolve.c (gfc_get_string): Likewise.
>> > > * symbol.c (gfc_new_symbol): Remove check for maximum symbol
>> > > name length.  Remove redundant 0 setting of new calloc()ed
>> > > gfc_symbol.

Re: [PATCH] Fix PowerPC -fstack-clash-protection -mprefixed-addr ICE (PR target/93122)

[Jakub Jelinek]

On Wed, Jan 22, 2020 at 03:24:54PM +0100, Jakub Jelinek wrote:
> > It looks like your patch will pessimise code in some cases as well, btw?
> 
> No, it will solely turn previous wrong-codes into something that works
> (== cases where gen_addr3_insn would previously fail).
> The 1)+2) variant could even improve code, as gen_addr3_insn could succeed
> even if we currently don't call it (perhaps generate more than one insn,
> but it still might be better than forcing the constant into register and
> then performing add that way).

Here is what I meant as the alternative, i.e. don't check any predicates,
just gen_add3_insn, if that fails, force rs into register and retry.
And, add REG_FRAME_RELATED_EXPR note always when we haven't emitted a single
insn that has rtl exactly matching what we'd add the REG_FRAME_RELATED_EXPR
with (in that case, dwarf2cfi.c is able to figure it out by itself, no need
to waste compile time memory).

Ok for trunk if it passes bootstrap/regtest?

2020-01-30  Jakub Jelinek  

PR target/93122
* config/rs6000/rs6000-logue.c
(rs6000_emit_probe_stack_range_stack_clash): Always use gen_add3_insn,
if it fails, move rs into end_addr and retry.  Add
REG_FRAME_RELATED_EXPR note whenever it returns more than one insn or
the insn pattern doesn't describe well what exactly happens to
dwarf2cfi.c.

* gcc.target/powerpc/pr93122.c: New test.

--- gcc/config/rs6000/rs6000-logue.c.jj 2020-01-12 11:54:36.395413680 +0100
+++ gcc/config/rs6000/rs6000-logue.c2020-01-30 16:54:28.646943559 +0100
@@ -1604,20 +1604,32 @@ rs6000_emit_probe_stack_range_stack_clas
   rtx end_addr
= copy_reg ? gen_rtx_REG (Pmode, 0) : gen_rtx_REG (Pmode, 12);
   rtx rs = GEN_INT (-rounded_size);
-  rtx_insn *insn;
-  if (add_operand (rs, Pmode))
-   insn = emit_insn (gen_add3_insn (end_addr, stack_pointer_rtx, rs));
-  else
+  rtx_insn *insn = gen_add3_insn (end_addr, stack_pointer_rtx, rs);
+  if (insn == NULL)
+   {
+ emit_move_insn (end_addr, rs);
+ insn = gen_add3_insn (end_addr, end_addr, stack_pointer_rtx);
+ gcc_assert (insn);
+   }
+  rtx set;
+  if (!NONJUMP_INSN_P (insn)
+ || NEXT_INSN (insn)
+ || (set = single_set (insn)) == NULL_RTX
+ || SET_DEST (set) != end_addr
+ || GET_CODE (SET_SRC (set)) != PLUS
+ || XEXP (SET_SRC (set), 0) != stack_pointer_rtx
+ || XEXP (SET_SRC (set), 1) != rs)
{
- emit_move_insn (end_addr, GEN_INT (-rounded_size));
- insn = emit_insn (gen_add3_insn (end_addr, end_addr,
-  stack_pointer_rtx));
- /* Describe the effect of INSN to the CFI engine.  */
+ insn = emit_insn (insn);
+ /* Describe the effect of INSN to the CFI engine, unless it
+is a single insn that describes it itself.  */
  add_reg_note (insn, REG_FRAME_RELATED_EXPR,
gen_rtx_SET (end_addr,
 gen_rtx_PLUS (Pmode, stack_pointer_rtx,
   rs)));
}
+  else
+   insn = emit_insn (insn);
   RTX_FRAME_RELATED_P (insn) = 1;
 
   /* Emit the loop.  */
--- gcc/testsuite/gcc.target/powerpc/pr93122.c.jj   2020-01-30 
16:42:14.255927311 +0100
+++ gcc/testsuite/gcc.target/powerpc/pr93122.c  2020-01-30 16:42:14.255927311 
+0100
@@ -0,0 +1,12 @@
+/* PR target/93122 */
+/* { dg-do compile { target lp64 } } */
+/* { dg-options "-fstack-clash-protection -mprefixed-addr -mfuture" } */
+
+void bar (char *);
+
+void
+foo (void)
+{
+  char s[4294967296];
+  bar (s);
+}


Jakub

gcov: reduce code quality loss by reproducible topn merging [PR92924]

[Jan Hubicka]

Hi,
this patch implements more careful merging of topn values (tracking regression
we got by the reproducible profiling work).  Instead of giving up on the
counter on the overflow we use sign bits to track
 1) if there are some vlaues lost
 2) if given target was not having probability at least 1/TOPN in some run.
This makes it possible to trust more profiled values and also consumer can
decide whether he wants reproducibility at all.

Bootstrapped/regtested x86_64-linux. The patch makes small improvement to
profile precision but it will make it easy to implement command line option
turning off the profile reproducibility.  It turns out that it reduces
the precision of profile info quite a lot in practice.  In particular with
nonreproducible profiling we speculatively inline 26% more indirect calls
that without in cc1plus.

Martin, I welcome your opinion on the patch :)

lto-profile-bootstrapped/regtested x86_64-linux.  I am going to test
this on Firefox and clang and gather updated logs.

2020-01-30  Jan Hubicka  

PR ipa/92924
* value-prof.c (dump_histogram_value): Update dumping.
(stream_out_histogram_value): Do not check that values are positive for
TOPN
(get_nth_most_common_value): Update comment and handling of sign bits.

libgcc/ChangeLog:

2020-01-30  Jan Hubicka  

PR ipa/92924
* libgcov-merge.c (merge_topn_values_set): Do not invalidate whole
counter when it is too full, but track info in signs of counts.

diff --git a/gcc/value-prof.c b/gcc/value-prof.c
index f0456c8e93d..e906bd49848 100644
--- a/gcc/value-prof.c
+++ b/gcc/value-prof.c
@@ -265,13 +265,17 @@ dump_histogram_value (FILE *dump_file, histogram_value 
hist)
? "Top N value counter" : "Indirect call counter"));
  if (hist->hvalue.counters)
{
- fprintf (dump_file, " all: %" PRId64 ", values: ",
-  (int64_t) hist->hvalue.counters[0]);
+ fprintf (dump_file, " all: %" PRId64 "%s, values: ",
+  abs ((int64_t) hist->hvalue.counters[0]),
+  hist->hvalue.counters[0] < 0
+  ? " (values missing)": "");
  for (unsigned i = 0; i < GCOV_TOPN_VALUES; i++)
{
- fprintf (dump_file, "[%" PRId64 ":%" PRId64 "]",
+ fprintf (dump_file, "[%" PRId64 ":%" PRId64 "%s]",
   (int64_t) hist->hvalue.counters[2 * i + 1],
-  (int64_t) hist->hvalue.counters[2 * i + 2]);
+  (int64_t) abs (hist->hvalue.counters[2 * i + 2]),
+  hist->hvalue.counters[2 * i + 2] < 0
+  ? " (unreproducible)" : "");
  if (i != GCOV_TOPN_VALUES - 1)
fprintf (dump_file, ", ");
}
@@ -330,7 +334,7 @@ stream_out_histogram_value (struct output_block *ob, 
histogram_value hist)
   /* When user uses an unsigned type with a big value, constant converted
 to gcov_type (a signed type) can be negative.  */
   gcov_type value = hist->hvalue.counters[i];
-  if (hist->type == HIST_TYPE_TOPN_VALUES && i > 0)
+  if (hist->type == HIST_TYPE_TOPN_VALUES)
;
   else
gcc_assert (value >= 0);
@@ -719,26 +723,47 @@ gimple_divmod_fixed_value (gassign *stmt, tree value, 
profile_probability prob,
 
 /* Return the n-th value count of TOPN_VALUE histogram.  If
there's a value, return true and set VALUE and COUNT
-   arguments.  */
+   arguments.
+
+   Counters have the following meanings.
+
+   abs (counters[0]) is the number of executions
+   for i in 0 ... TOPN-1
+ counters[2 * i + 1] is target
+ abs (counters[2 * i + 2]) is corresponding hitrate counter.
+
+   Value of counters[0] negative when counter became
+   full during merging and some values are lost.
+
+   Value of counters[2 * i + 2] is negative if there was run when the
+   corresponding targt was not having probability 1/4.
+
+   If both counters[0] and counters[2 * i + 2] are negatie we do not know the
+   precise hitrate count of the target in case order of merges is not fixed
+   (as with parallel profiledbootstrap).
+  */
 
 bool
 get_nth_most_common_value (gimple *stmt, const char *counter_type,
   histogram_value hist, gcov_type *value,
   gcov_type *count, gcov_type *all, unsigned n)
 {
-  if (hist->hvalue.counters[2] == -1)
-return false;
-
   gcc_assert (n < GCOV_TOPN_VALUES);
 
   *count = 0;
   *value = 0;
 
-  gcov_type read_all = hist->hvalue.counters[0];
+  gcov_type read_all = abs (hist->hvalue.counters[0]);
 
   gcov_type v = hist->hvalue.counters[2 * n + 1];
   gcov_type c = hist->hvalue.counters[2 * n + 2];
 
+  /* Negative value marks that target is not necessarily reproducible
+ for parallel merging.  */
+  if (c < 0 && hist->hvalue.counters[0] < 0)
+return false;
+  c = abs (c);
+
   /* 

Re: [PATCH] Add OpenACC acc_get_property support for AMD GCN

[Thomas Schwinge]

Hi!

Andrew and Frederik, thanks for your emails reminding/educating me about
'snprintf' as well as this HSA fixed-size buffer API.  There doesn't
happen to be something available in the HSA API available so that we
could use 'sizeof [something]' instead of hard-coding '64' etc.?


I understand correctly that the only reason for:

On 2020-01-29T10:52:57+0100, "Harwath, Frederik"  
wrote:
>   * testsuite/libgomp.oacc-c-c++-common/acc_get_property-aux.c
>   (expect_device_properties): Split function into ...
>   (expect_device_string_properties): ... this new function ...
>   (expect_device_memory): ... and this new function.

... this split is that we can't test 'expect_device_memory' here:

>   * testsuite/libgomp.oacc-c-c++-common/acc_get_property-gcn.c:
>   Add test.

..., because that one doesn't (re-)implement the 'acc_property_memory'
interface?

> --- a/libgomp/plugin/plugin-gcn.c
> +++ b/libgomp/plugin/plugin-gcn.c

> @@ -4115,12 +4141,37 @@ GOMP_OFFLOAD_openacc_async_dev2host (int device, void 
> *dst, const void *src,
>  union goacc_property_value
>  GOMP_OFFLOAD_openacc_get_property (int device, enum goacc_property prop)
>  {
> [...]
> +  switch (prop)
> +{
> +case GOACC_PROPERTY_FREE_MEMORY:
> +  /* Not supported. */
> +  break;

(OK, can be added later when somebody feels like doing that.)

> +case GOACC_PROPERTY_MEMORY:
> +  {
> + size_t size;
> + hsa_region_t region = agent->data_region;
> + hsa_status_t status =
> +   hsa_fns.hsa_region_get_info_fn (region, HSA_REGION_INFO_SIZE, );
> + if (status == HSA_STATUS_SUCCESS)
> +   propval.val = size;
> + break;
> +  }
> [...]
>  }

Here we got 'acc_property_memory' implemented, but not here:

> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-gcn.c
> @@ -0,0 +1,132 @@
> +/* Test the `acc_get_property' and `acc_get_property_string' library
> +   functions on amdgcn devices by comparing property values with
> +   those obtained through the HSA API. */
> +/* { dg-additional-sources acc_get_property-aux.c } */
> +/* { dg-additional-options "-ldl" } */
> +/* { dg-do run { target openacc_amdgcn_accel_selected } } */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#ifndef __cplusplus
> +typedef int bool;
> +#endif
> +#include 
> +
> +
> +void expect_device_string_properties (acc_device_t dev_type, int dev_num,
> +   const char* expected_vendor,
> +   const char* expected_name,
> +   const char* expected_driver);
> +
> +hsa_status_t (*hsa_agent_get_info_fn) (hsa_agent_t agent,
> +hsa_agent_info_t attribute,
> +void *value);
> +hsa_status_t (*hsa_system_get_info_fn) (hsa_system_info_t attribute,
> + void *value);
> +hsa_status_t (*hsa_iterate_agents_fn)
> +(hsa_status_t (*callback)(hsa_agent_t agent, void *data), void *data);
> +hsa_status_t (*hsa_init_fn) (void);
> +
> +char* support_cpu_devices;
> +
> +void test_setup ()
> +{
> +  char* env_runtime;
> +  char* hsa_runtime_lib;
> +  void *handle;
> +
> +#define DLSYM_FN(function)   \
> +  function##_fn = (typeof(function##_fn))dlsym (handle, #function);  \
> +  if (function##_fn == NULL) \
> +{
> \
> +  fprintf (stderr, "Could not get symbol " #function ".\n"); \
> +  abort ();  \
> +}
> +
> +  env_runtime = getenv ("HSA_RUNTIME_LIB");
> +  hsa_runtime_lib = env_runtime ? env_runtime : (char*)"libhsa-runtime64.so";
> +
> +  handle = dlopen (hsa_runtime_lib, RTLD_LAZY);
> +  if (!handle)
> +{
> +  fprintf (stderr, "Could not load %s.\n", hsa_runtime_lib);
> +  abort ();
> +}
> +
> +  DLSYM_FN (hsa_agent_get_info)
> +  DLSYM_FN (hsa_system_get_info)
> +  DLSYM_FN (hsa_iterate_agents)
> +  DLSYM_FN (hsa_init)
> +
> +  hsa_init_fn ();
> +
> +  support_cpu_devices = getenv ("GCN_SUPPORT_CPU_DEVICES");
> +}
> +
> +static hsa_status_t check_agent_properties (hsa_agent_t agent, void 
> *dev_num_arg)
> +{
> +
> +  char name[64];
> +  char vendor_name[64];
> +  uint16_t minor;
> +  uint16_t major;
> +  char driver[60];
> +
> +  hsa_status_t status;
> +  hsa_device_type_t device_type;
> +  int* dev_num = (int*)dev_num_arg;
> +
> +#define AGENT_GET_INFO(info_type, val)   \
> +  status = hsa_agent_get_info_fn (agent, info_type, );   \
> +  if (status != HSA_STATUS_SUCCESS)  \
> +{\
> +  fprintf (stderr, "Failed to obtain " #info_type ".\n");\
> +  abort (); 

Make OpenACC 'acc_get_property' with 'acc_device_current' work (was: [PATCH] Add OpenACC 2.6 `acc_get_property' support)

[Thomas Schwinge]

Hi!

On 2020-01-10T23:52:11+0100, I wrote:
> On 2019-12-21T23:02:38+0100, I wrote:
>> On 2019-12-20T17:46:57+0100, "Harwath, Frederik"  
>> wrote:
 > --- a/include/gomp-constants.h
 > +++ b/include/gomp-constants.h
>
 > +#define GOMP_DEVICE_CURRENT -3
>
 Should this actually get value '-1' instead of '-3'?  Or, is the OpenACC
 'acc_device_t' code already paying special attention to negative values
 '-1', '-2'?  (I don't think so.)
>
 | Also, 'acc_device_current' is a libgomp-internal thing (doesn't interface
 | with the compiler proper), so strictly speaking 'GOMP_DEVICE_CURRENT'
 | isn't needed in 'include/gomp-constants.h'.  But probably still a good
 | idea to list it there, in this canonical place, to keep the several lists
 | of device types coherent.
>
 I still wonder about that...  ;-)
>
>> I still think that 'GOMP_DEVICE_CURENT' should get value '-1' (and
>> probably be rename 'GOACC_DEVICE_CURRENT' to make more obvious that it's
>> not related to the 'GOMP_DEVICE_*' ones), but we shall have a look at
>> that later (before GCC 10 release); that's libgomp/OpenACC-internal,
>> doesn't affect anything else.
>
> That's still pending.  Recently,
>  "Missing definition
> for acc_device_current" got filed; let's (also/first) watch/wait what
> comes out of that.

(That's still pending, but) notwithstanding the specific value we'll use
eventually, the 'acc_device_current' interface should work already now.

..., but I noticed that we don't have any test cases for that (so by that
definition, it must be broken).  The curious guy that I am sometimes ;-)
I gave that a try, and... "of course"... it doesn't work.  Please review
the attached (Tobias the Fortran test cases, please), and test with AMD
GCN offloading.  If approving this patch, please respond with
"Reviewed-by: NAME " so that your effort will be recorded in the
commit log, see .


Grüße
 Thomas


From 5ce3725cf160f086e99c01e73c26a0bf5654f5b6 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge 
Date: Wed, 29 Jan 2020 22:11:15 +0100
Subject: [PATCH] Make OpenACC 'acc_get_property' with 'acc_device_current'
 work

	libgomp/
	* oacc-init.c (acc_get_property, acc_get_property_string): Allow
	'acc_device_current'.
	* openacc.f90 (module openacc): Export 'acc_device_current'.
	* testsuite/libgomp.oacc-c-c++-common/acc_get_property-aux.c
	(expect_device_memory): Rename to...
	(expect_device_memory_properties): ... this.  Make 'static'.
	(expect_device_string_properties): Rename to...
	(expect_device_non_memory_properties): ... this.  Adjust all
	users.
	* testsuite/libgomp.oacc-c-c++-common/acc_get_property-aux.h: New
	file.
	* testsuite/libgomp.oacc-c-c++-common/acc_get_property-aux.c: Use it.
	* testsuite/libgomp.oacc-c-c++-common/acc_get_property-gcn.c:
	Likewise.
	* testsuite/libgomp.oacc-c-c++-common/acc_get_property-host.c:
	Likewise.
	* testsuite/libgomp.oacc-c-c++-common/acc_get_property-nvptx.c:
	Likewise.
	* testsuite/libgomp.oacc-c-c++-common/acc_get_property-gcn.c: Add
	some more testing.
	* testsuite/libgomp.oacc-c-c++-common/acc_get_property-host.c:
	Likewise.
	* testsuite/libgomp.oacc-c-c++-common/acc_get_property-nvptx.c:
	Likewise.
	* testsuite/libgomp.oacc-c-c++-common/acc_get_property.c:
	Likewise.
	* testsuite/libgomp.oacc-fortran/acc_get_property.f90: Likewise.
	* testsuite/libgomp.oacc-fortran/acc_get_property-aux.f90: New
	file.
	* testsuite/libgomp.oacc-fortran/acc_get_property-host.F90: New
	file.
---
 libgomp/oacc-init.c   |   8 +-
 libgomp/openacc.f90   |   1 +
 .../acc_get_property-aux.c|  76 ++---
 .../acc_get_property-aux.h|  14 +++
 .../acc_get_property-gcn.c|  26 +++--
 .../acc_get_property-host.c   |  26 +++--
 .../acc_get_property-nvptx.c  |  27 +++--
 .../acc_get_property.c|  16 ++-
 .../acc_get_property-aux.f90  | 102 ++
 .../acc_get_property-host.F90 |  31 ++
 .../libgomp.oacc-fortran/acc_get_property.f90 |  15 ++-
 11 files changed, 274 insertions(+), 68 deletions(-)
 create mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-aux.h
 create mode 100644 libgomp/testsuite/libgomp.oacc-fortran/acc_get_property-aux.f90
 create mode 100644 libgomp/testsuite/libgomp.oacc-fortran/acc_get_property-host.F90

diff --git a/libgomp/oacc-init.c b/libgomp/oacc-init.c
index ef12b4c16d01..c28c0f689ba2 100644
--- a/libgomp/oacc-init.c
+++ b/libgomp/oacc-init.c
@@ -796,7 +796,9 @@ get_property_any (int ord, acc_device_t d, acc_device_property_t prop)
 size_t
 acc_get_property (int ord, acc_device_t d, acc_device_property_t prop)
 {
-  if (!known_device_type_p (d))
+  if (d == acc_device_current)
+; /* Allowed only for 'acc_get_property', 

Re: [PATCH] OpenACC reference count consistency checking

[Thomas Schwinge]

Hi Julian!

Notwithstanding the open question about how to implement this checking in
libgomp in a non-intrusive (performance-wise) yet maintainable (avoid
'#if 0') way, I have two more questions.


Is there a specific reason why this checking isn't also enabled for
libgomp OpenMP 'target' entry points?


Can you please explain (textually?) how this checking (design per your
textual description below) is working in context of mixed OpenACC
structured ("S") and dynamic ("D") reference counts?  For example:

// S: 0, D: 0

#pragma acc enter data copyin ([data]) // copyin; S: 0, D: 1

acc_copyin ([data]) // no-op; S: 0, D: 2

#pragma acc data copyout ([data]) // no-op; S: 1, D: 2
  {
acc_create ([data]) // no-op; S: 1, D: 3

#pragma acc data create ([data]) // no-op; S: 2, D: 3
  {
#pragma acc parallel copyout ([data]) // no-op; S: 3, D: 3
  {
  } // no-op; S: 2, D: 3

acc_delete_finalize ([data]) // no-op; S: 2, D: 0

acc_create ([data]) // no-op; S: 2, D: 1
  } // no-op; S: 1, D: 1

#pragma acc exit data delete ([data]) // no-op; S: 1, D: 0
  } // copyout; S: 0, D: 0

assert (!acc_is_present ([data]));

(Haven't compiled but I'm reasonably sure that the nesting and my manual
"[action]; [S], [D]" annotations are correct.  But please verify, if
course.)


Grüße
 Thomas


On 2018-11-30T03:50:24-0800, Julian Brown  wrote:
> The model used for checking is as follows.
>
>  1. Each splay tree key that references a target memory descriptor
> increases that descriptor's refcount by 1.
>
>  2. Each variable listed in a target memory descriptor that links back to a
> splay tree key increases that key's refcount by 1. Each target memory
> descriptor's variable list is counted only once, even if multiple
> splay tree keys point to it (via their "tgt" field).
>
>  3. Additional ("real") target memory descriptors may be present
> representing data mapped through "acc data" or "acc parallel/kernels"
> blocks.  These descriptors have their refcount bumped, and the
> variables linked through such blocks have their refcounts bumped also
> (again, with "once only" semantics).
>
>  4. Asynchronous operations "artificially" bump the reference counts for
> referenced target memory descriptors (but *not* for linked
> variables/splay tree keys), in order to delay freeing mapped device
> memory until the asynchronous operation has completed.  We model this,
> for checking purposes only, using an off-side linked list.
>
>  5. "Virtual" reference counts ("virtual_refcount") cannot be checked
> purely statically, so we add the incoming value to each key's
> statically-determined reference count ("refcount_chk"), and make
> sure that the total matches the incoming reference count ("refcount").
>
> Thanks,
>
> Julian
>
> ChangeLog
>
>   libgomp/
>   * libgomp.h (RC_CHECKING): New macro, disabled by default, guarding all
>   hunks in this patch.
>   (target_mem_desc): Add forward declaration.
>   (async_tgt_use): New struct.
>   (target_mem_desc): Add refcount_chk, mark fields.
>   (acc_dispatch_t): Add tgt_uses, au_lock fields.
>   (dump_tgt, gomp_rc_check): Add prototypes.
>   * oacc-async (goacc_async_unmap_tgt): Add refcount self-check code.
>   (goacc_async_copyout_unmap_vars): Likewise.
>   (goacc_remove_var_async): Likewise.
>   * oacc-parallel.c (GOACC_parallel_keyed_internal): Add refcount
>   self-check code.
>   (GOACC_data_start, GOACC_data_end, GOACC_enter_exit_data): Likewise.
>   * target.c (stdio.h): Include.
>   (dump_tgt, rc_check_clear, rc_check_count, rc_check_verify)
>   (gomp_rc_check): New functions to consistency-check reference counts.
>   (gomp_target_init): Initialise self-check-related device fields.
> ---
>  libgomp/libgomp.h   |   31 +++
>  libgomp/oacc-async.c|   46 +++
>  libgomp/oacc-parallel.c |   33 
>  libgomp/target.c|  199 
> +++
>  4 files changed, 309 insertions(+), 0 deletions(-)
>
> diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
> index df49c1b..24cbddd 100644
> --- a/libgomp/libgomp.h
> +++ b/libgomp/libgomp.h
> @@ -874,9 +874,26 @@ struct target_var_desc {
>uintptr_t length;
>  };
>  
> +/* Uncomment to enable reference-count consistency checking (for development
> +   use only).  */
> +/*#define RC_CHECKING 1*/
> +
> +#ifdef RC_CHECKING
> +struct target_mem_desc;
> +
> +struct async_tgt_use {
> +  struct target_mem_desc *tgt;
> +  struct async_tgt_use *next;
> +};
> +#endif
> +
>  struct target_mem_desc {
>/* Reference count.  */
>uintptr_t refcount;
> +#ifdef RC_CHECKING
> +  uintptr_t refcount_chk;
> +  bool mark;
> +#endif
>/* All the splay nodes allocated together.  */
>splay_tree_node array;
>

Re: [PATCH] gcc: Add new configure options to allow static libraries to be selected

[Iain Sandoe]

Hi Andrew,

Andrew Burgess  wrote:

> * Iain Sandoe  [2020-01-28 10:34:52 +]:

>> * Before the patch, libcpp and gcc configury finds this and they agree on 
>> the availability of ICONV (#define HAVE_ICONV 1).
>> 
>> * After the patch libcpp no longer thinks iconv is available, but gcc 
>> continues to find it - and that, I think, leads to it attempting tests for 
>> which libcpp has not been configured.
> 
> This must be a bug, right?  With no changes to the configure scripts
> both components should continue to find libiconv after this patch.
> Even with the failure to update gcc/configure correctly (see below)
> libcpp should continue to find the exact same libraries as before.

not exactly sure which component you think has the bug in this case…

>> Can you clarify why there’s no need to match the configury changes in libcpp
>> / gcc / libstdc++ ?
> 
> This is the same issue that Tobias pointed out, and was a result of me
> incorrectly trying to regenerate the configure files.

.. but it seems that the combination of the two things produced the 
configuration
mismatch ..

> Let me know if this resolves the problems you're seeing and if it does
> I'll prepare it for submission.

I did a smoke test on Darwin19 and the amended patch allowed bootstrap to
 complete. ICONV is seen in both libcpp and gcc (that’s not an exhaustive check
and the testsuite is still running).

I’ll add this to my overnight runs - “overnight” means results later tomorrow 
for the
slower targets - but it looks encouraging.

thanks,
Iain
> 
> 
> diff --git a/config/lib-link.m4 b/config/lib-link.m4
> index 662192e0a07..20e281fd323 100644
> --- a/config/lib-link.m4
> +++ b/config/lib-link.m4
> @@ -492,7 +492,7 @@ AC_DEFUN([AC_LIB_LINKFLAGS_BODY],
> dnl known to the linker and runtime loader. (All the system
> dnl directories known to the linker should also be known to the
> dnl runtime loader, otherwise the system is severely 
> misconfigured.)
> -if x$lib_type = xauto || x$lib_type = xshared; then
> +if test "x$lib_type" = "xauto" || test "x$lib_type" = "xshared"; 
> then
>   LIB[]NAME="${LIB[]NAME}${LIB[]NAME:+ }-l$name"
>   LTLIB[]NAME="${LTLIB[]NAME}${LTLIB[]NAME:+ }-l$name"
> else
> diff --git a/gcc/configure b/gcc/configure
> index e2c8fc71772..4c2c5991c0e 100755
> --- a/gcc/configure
> +++ b/gcc/configure
> @@ -974,6 +974,7 @@ with_zstd_include
> with_zstd_lib
> enable_rpath
> with_libiconv_prefix
> +with_libiconv_type
> enable_sjlj_exceptions
> with_gcc_major_version_only
> enable_secureplt
> @@ -1811,6 +1812,7 @@ Optional Packages:
>   --with-gnu-ld   assume the C compiler uses GNU ld default=no
>   --with-libiconv-prefix[=DIR]  search for libiconv in DIR/include and DIR/lib
>   --without-libiconv-prefix don't search for libiconv in includedir and 
> libdir
> +  --with-libiconv-type=TYPE type of library to search for 
> (auto/static/shared)
>   --with-gcc-major-version-only
>   use only GCC major number in filesystem paths
>   --with-pic  try to use only PIC/non-PIC objects [default=use
> @@ -10730,6 +10732,16 @@ if test "${with_libiconv_prefix+set}" = set; then :
> 
> fi
> 
> +
> +# Check whether --with-libiconv-type was given.
> +if test "${with_libiconv_type+set}" = set; then :
> +  withval=$with_libiconv_type;  with_libiconv_type=$withval
> +else
> +   with_libiconv_type=auto
> +fi
> +
> +  lib_type=`eval echo \$with_libiconv_type`
> +
>   LIBICONV=
>   LTLIBICONV=
>   INCICONV=
> @@ -10767,13 +10779,13 @@ fi
>   found_so=
>   found_a=
>   if test $use_additional = yes; then
> -if test -n "$shlibext" && test -f 
> "$additional_libdir/lib$name.$shlibext"; then
> +if test -n "$shlibext" && test -f 
> "$additional_libdir/lib$name.$shlibext" && test x$lib_type != xstatic; then
>   found_dir="$additional_libdir"
>   found_so="$additional_libdir/lib$name.$shlibext"
>   if test -f "$additional_libdir/lib$name.la"; then
> found_la="$additional_libdir/lib$name.la"
>   fi
> -else
> +elif test x$lib_type != xshared; then
>   if test -f "$additional_libdir/lib$name.$libext"; then
> found_dir="$additional_libdir"
> found_a="$additional_libdir/lib$name.$libext"
> @@ -10797,13 +10809,13 @@ fi
>   case "$x" in
> -L*)
>   dir=`echo "X$x" | sed -e 's/^X-L//'`
> -  if test -n "$shlibext" && test -f 
> "$dir/lib$name.$shlibext"; then
> +  if test -n "$shlibext" && test -f 
> "$dir/lib$name.$shlibext" && test x$lib_type != xstatic; then
> found_dir="$dir"
> found_so="$dir/lib$name.$shlibext"
> if test -f "$dir/lib$name.la"; then
>   

Re: [GCC][PATCH][AArch32] ACLE intrinsics bfloat16 vmmla and vfma for AArch32 AdvSIMD

[Kyrill Tkachov]

Hi Delia,


On 1/28/20 4:44 PM, Delia Burduv wrote:

Ping.

*From:* Delia Burduv 
*Sent:* 22 January 2020 17:26
*To:* gcc-patches@gcc.gnu.org 
*Cc:* ni...@redhat.com ; Richard Earnshaw 
; Ramana Radhakrishnan 
; Kyrylo Tkachov 
*Subject:* Re: [GCC][PATCH][AArch32] ACLE intrinsics bfloat16 vmmla 
and vfma for AArch32 AdvSIMD

Ping.

I have read Richard Sandiford's comments on the AArch64 patches and I
will apply what is relevant to this patch as well. Particularly, I will
change the tests to use the exact input and output registers and I will
change the types of the rtl patterns.



Please send the updated patches so that someone can commit them for you 
once they're reviewed.


Thanks,

Kyrill




On 12/20/19 6:44 PM, Delia Burduv wrote:
> This patch adds the ARMv8.6 ACLE intrinsics for vmmla, vfmab and vfmat
> as part of the BFloat16 extension.
> (https://developer.arm.com/docs/101028/latest.)
> The intrinsics are declared in arm_neon.h and the RTL patterns are
> defined in neon.md.
> Two new tests are added to check assembler output and lane indices.
>
> This patch depends on the Arm back-end patche.
> (https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01448.html)
>
> Tested for regression on arm-none-eabi and armeb-none-eabi. I don't 
have

> commit rights, so if this is ok can someone please commit it for me?
>
> gcc/ChangeLog:
>
> 2019-11-12  Delia Burduv 
>
>  * config/arm/arm_neon.h (vbfmmlaq_f32): New.
>    (vbfmlalbq_f32): New.
>    (vbfmlaltq_f32): New.
>    (vbfmlalbq_lane_f32): New.
>    (vbfmlaltq_lane_f32): New.
>      (vbfmlalbq_laneq_f32): New.
>    (vbfmlaltq_laneq_f32): New.
>  * config/arm/arm_neon_builtins.def (vbfmmla): New.
>    (vbfmab): New.
>    (vbfmat): New.
>    (vbfmab_lane): New.
>    (vbfmat_lane): New.
>    (vbfmab_laneq): New.
>    (vbfmat_laneq): New.
>   * config/arm/iterators.md (BF_MA): New int iterator.
>    (bt): New int attribute.
>    (VQXBF): Copy of VQX with V8BF.
>    (V_HALF): Added V8BF.
>    * config/arm/neon.md (neon_vbfmmlav8hi): New insn.
>    (neon_vbfmav8hi): New insn.
>    (neon_vbfma_lanev8hi): New insn.
>    (neon_vbfma_laneqv8hi): New expand.
>    (neon_vget_high): Changed iterator to VQXBF.
>  * config/arm/unspecs.md (UNSPEC_BFMMLA): New UNSPEC.
>    (UNSPEC_BFMAB): New UNSPEC.
>    (UNSPEC_BFMAT): New UNSPEC.
>
> 2019-11-12  Delia Burduv 
>
>      * gcc.target/arm/simd/bf16_ma_1.c: New test.
>      * gcc.target/arm/simd/bf16_ma_2.c: New test.
>      * gcc.target/arm/simd/bf16_mmla_1.c: New test.

Re: [PING][PATCH][GCC][ARM] Arm generates out of range conditional branches in Thumb2 (PR91816)

[Kyrill Tkachov]

On 1/30/20 2:42 PM, Stam Markianos-Wright wrote:



On 1/28/20 10:35 AM, Kyrill Tkachov wrote:

Hi Stam,

On 1/8/20 3:18 PM, Stam Markianos-Wright wrote:


On 12/10/19 5:03 PM, Kyrill Tkachov wrote:

Hi Stam,

On 11/15/19 5:26 PM, Stam Markianos-Wright wrote:

Pinging with more correct maintainers this time :)

Also would need to backport to gcc7,8,9, but need to get this 
approved

first!


Sorry for the delay.

Same here now! Sorry totally forget about this in the lead up to Xmas!

Done the changes marked below and also removed the unnecessary extra 
#defines

from the test.



This is ok with a nit on the testcase...


diff --git a/gcc/testsuite/gcc.target/arm/pr91816.c 
b/gcc/testsuite/gcc.target/arm/pr91816.c

new file mode 100644
index 
..757c897e9c0db32709227b3fdf1b4a8033428232

--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr91816.c
@@ -0,0 +1,61 @@
+/* { dg-do compile } */
+/* { dg-options "-march=armv7-a -mthumb -mfpu=vfpv3-d16" }  */
+int printf(const char *, ...);
+

I think this needs a couple of effective target checks like 
arm_hard_vfp_ok and arm_thumb2_ok. See other tests in gcc.target/arm 
that add -mthumb to the options.


Hmm, looking back at this now, is there any reason why it can't just be:

/* { dg-do compile } */
/* { dg-require-effective-target arm_thumb2_ok } */
/* { dg-additional-options "-mthumb" }  */

were we don't override the march or fpu options at all, but just use 
`require-effective-target arm_thumb2_ok` to make sure that thumb2 is 
supported?


The attached new diff does just that.



Works for me, there are plenty of configurations run with fpu that it 
should get the right coverage.


Ok (make sure commit the updated, if needed, ChangeLog as well)

Thanks!

Kyrill



Cheers :)

Stam.



Thanks,
Kyrill

[PATCH, v3] coroutines: Fix ICE on invalid (PR93458).

[Iain Sandoe]

Hi Nathan,

Nathan Sidwell  wrote:

> On 1/29/20 10:39 AM, Iain Sandoe wrote:
>> Hi Nathan,

>> +  /* If we are missing fundmental information, such as the traits, then 
>> don't
>> + emit this for every keyword in a TU.  This particular error is per TU
>> + so we don't need to keep the indicator per function.  */
>> +  static bool traits_error_emitted = false;
> 
> You can of course move this into the if's block scope.
done.

>> +  if (traits_decl == error_mark_node)
>> +{
>> +  if (!traits_error_emitted)
>> +error_at (kw, "cannot find % template");
> 
> Give the name you were looking for:
>   "%<%E::%E%> ...", std_node, coro_traits_identifier

I had the “complain” flag on to the lookup so that it was emitting an error with
that information.

however. ….
> also, what if you find something, but it's not a type template?

… I’ve switched the complain off on lookup_qualified_name and now check for
a type template.

I took the liberty of repeating this treatment in the coroutine handle lookup 
code
(new testcases attached).

so the errors now look like:

"cannot find a valid coroutine traits template using 'std::coroutine_traits’”

and

"cannot find a valid coroutine handle class template using 
'std::coroutine_handle’”

.. but open to tweaking them if there could be bette wording.

>>/* Coroutine traits template.  */
>>coro_traits_templ = find_coro_traits_template_decl (loc);
>> -  gcc_checking_assert (coro_traits_templ != NULL);
>> +  if (coro_traits_templ == NULL_TREE
>> +  || coro_traits_templ == error_mark_node)
>> +return false;
> 
> ISTM that find_coro_traits_template_decl should be returning exactly one of 
> NULL_TREE of error_mark_node on failure.  Its users don't particularly care 
> why it failed (not found vs found ambiguous/not template).

fair enough, settled for NULL_TREE.
> +  /* Save the coroutine data on the side to avoid the overhead on every

>> +  if (!coro_info->coro_ret_type_error_emitted)
>> +error_at (DECL_SOURCE_LOCATION (fndecl), "a coroutine must have a"
>> +  " class or struct return type");
> 
> Perhaps something like "coroutine return type %qT is not a class"?  I.e. show 
> them the type.
> (structs are classes, there's no need to say 'class or struct’)

yes, that’s nicer, done.

>> +  coro_info->coro_ret_type_error_emitted = true;
>> +  return false;
>> +}
>> +
>>tree templ_class = instantiate_coro_traits (fndecl, loc);
>>  /* Find the promise type for that.  */
>> @@ -422,7 +454,7 @@ coro_promise_type_found_p (tree fndecl, location_t loc)
>>/* Try to find the handle type for the promise.  */
>>tree handle_type =
>>  instantiate_coro_handle_for_promise_type (loc, coro_info->promise_type);
>> -  if (handle_type == NULL_TREE)
>> +  if (handle_type == NULL_TREE || handle_type == error_mark_node)
> 
> similar to coro_traits_template_decl.

gave this a similar treatment.

As below with those changes and additional test cases,
OK / tweak error messages?
thanks
Iain.

Since coroutine-ness is discovered lazily, we encounter the diagnostics
during each keyword parse.  We were not handling the case where a user code
failed to include fundamental information (e.g. the traits) in a graceful
manner.

Once we've emitted an error for this level of fail, then we suppress
additional copies (otherwise the same thing will be reported for every
coroutine keyword seen).

gcc/cp/ChangeLog:

2020-01-30  Iain Sandoe  

* coroutines.cc (struct coroutine_info): Add a bool flag to note
that we emitted an error for a bad function return type.
(get_coroutine_info): Tolerate an unset info table in case of
missing traits.
(find_coro_traits_template_decl): In case of error or if we didn't
find a type template, note we emitted the error and suppress
duplicates.
(find_coro_handle_template_decl): Likewise.
(instantiate_coro_traits): Only check for error_mark_node in the
return from lookup_qualified_name.
(coro_promise_type_found_p): Reorder initialization so that we check
for the traits and their usability before allocation of the info
table.  Check for a suitable return type and emit a diagnostic for
here instead of relying on the lookup machinery.  This allows the
error to have a better location, and means we can suppress multiple
copies.
(coro_function_valid_p): Re-check for a valid promise (and thus the
traits) before proceeding.  Tolerate missing info as a fatal error.

gcc/testsuite/ChangeLog:

2020-01-30  Iain Sandoe  

* g++.dg/coroutines/pr93458-1-missing-traits.C: New test.
* g++.dg/coroutines/pr93458-2-bad-traits.C: New test.
* g++.dg/coroutines/pr93458-3-missing-handle.C: New test.
* g++.dg/coroutines/pr93458-4-bad-coro-handle.C: New test.
* 

Re: [PING][PATCH][GCC][ARM] Arm generates out of range conditional branches in Thumb2 (PR91816)

[Stam Markianos-Wright]

On 1/28/20 10:35 AM, Kyrill Tkachov wrote:

Hi Stam,

On 1/8/20 3:18 PM, Stam Markianos-Wright wrote:


On 12/10/19 5:03 PM, Kyrill Tkachov wrote:

Hi Stam,

On 11/15/19 5:26 PM, Stam Markianos-Wright wrote:

Pinging with more correct maintainers this time :)

Also would need to backport to gcc7,8,9, but need to get this approved
first!


Sorry for the delay.

Same here now! Sorry totally forget about this in the lead up to Xmas!

Done the changes marked below and also removed the unnecessary extra #defines
from the test.



This is ok with a nit on the testcase...


diff --git a/gcc/testsuite/gcc.target/arm/pr91816.c 
b/gcc/testsuite/gcc.target/arm/pr91816.c

new file mode 100644
index 
..757c897e9c0db32709227b3fdf1b4a8033428232

--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr91816.c
@@ -0,0 +1,61 @@
+/* { dg-do compile } */
+/* { dg-options "-march=armv7-a -mthumb -mfpu=vfpv3-d16" }  */
+int printf(const char *, ...);
+

I think this needs a couple of effective target checks like arm_hard_vfp_ok and 
arm_thumb2_ok. See other tests in gcc.target/arm that add -mthumb to the options.


Hmm, looking back at this now, is there any reason why it can't just be:

/* { dg-do compile } */
/* { dg-require-effective-target arm_thumb2_ok } */
/* { dg-additional-options "-mthumb" }  */

were we don't override the march or fpu options at all, but just use 
`require-effective-target arm_thumb2_ok` to make sure that thumb2 is supported?


The attached new diff does just that.

Cheers :)

Stam.



Thanks,
Kyrill



diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index 7c4b1003844..8895becc639 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -576,4 +576,6 @@ void arm_parse_option_features (sbitmap, const 
cpu_arch_option *,
 
 void arm_initialize_isa (sbitmap, const enum isa_feature *);
 
+const char * arm_gen_far_branch (rtx *, int, const char * , const char *);
+
 #endif /* ! GCC_ARM_PROTOS_H */
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 07231d722b9..ee5de169f3e 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -32626,6 +32626,40 @@ arm_run_selftests (void)
 }
 } /* Namespace selftest.  */
 
+
+/* Generate code to enable conditional branches in functions over 1 MiB.
+   Parameters are:
+ operands: is the operands list of the asm insn (see arm_cond_branch or
+   arm_cond_branch_reversed).
+ pos_label: is an index into the operands array where operands[pos_label] 
is
+   the asm label of the final jump destination.
+ dest: is a string which is used to generate the asm label of the 
intermediate
+   destination
+   branch_format: is a string denoting the intermediate branch format, e.g.
+ "beq", "bne", etc.  */
+
+const char *
+arm_gen_far_branch (rtx * operands, int pos_label, const char * dest,
+   const char * branch_format)
+{
+  rtx_code_label * tmp_label = gen_label_rtx ();
+  char label_buf[256];
+  char buffer[128];
+  ASM_GENERATE_INTERNAL_LABEL (label_buf, dest , \
+   CODE_LABEL_NUMBER (tmp_label));
+  const char *label_ptr = arm_strip_name_encoding (label_buf);
+  rtx dest_label = operands[pos_label];
+  operands[pos_label] = tmp_label;
+
+  snprintf (buffer, sizeof (buffer), "%s%s", branch_format , label_ptr);
+  output_asm_insn (buffer, operands);
+
+  snprintf (buffer, sizeof (buffer), "b\t%%l0%d\n%s:", pos_label, label_ptr);
+  operands[pos_label] = dest_label;
+  output_asm_insn (buffer, operands);
+  return "";
+}
+
 #undef TARGET_RUN_TARGET_SELFTESTS
 #define TARGET_RUN_TARGET_SELFTESTS selftest::arm_run_selftests
 #endif /* CHECKING_P */
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index f89a2d412df..fb1d4547e5c 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -7546,9 +7546,15 @@
 ;; And for backward branches we have 
 ;;   (neg_range - neg_base_offs + pc_offs) = (neg_range - (-2 or -4) + 4).
 ;;
+;; In 16-bit Thumb these ranges are:
 ;; For a 'b'   pos_range = 2046, neg_range = -2048 giving (-2040->2048).
 ;; For a 'b' pos_range = 254,  neg_range = -256  giving (-250 ->256).
 
+;; In 32-bit Thumb these ranges are:
+;; For a 'b'   +/- 16MB is not checked for.
+;; For a 'b' pos_range = 1048574,  neg_range = -1048576  giving
+;; (-1048568 -> 1048576).
+
 (define_expand "cbranchsi4"
   [(set (pc) (if_then_else
  (match_operator 0 "expandable_comparison_operator"
@@ -7721,23 +7727,50 @@
  (label_ref (match_operand 0 "" ""))
  (pc)))]
   "TARGET_32BIT"
-  "*
-  if (arm_ccfsm_state == 1 || arm_ccfsm_state == 2)
+  {
+if (arm_ccfsm_state == 1 || arm_ccfsm_state == 2)
 {
   arm_ccfsm_state += 2;
-  return \"\";
+  return "";
 }
-  return \"b%d1\\t%l0\";
-  "
+switch (get_attr_length (insn))
+  {
+   case 2: /* Thumb2 16-bit b{cond}.  */
+   case 4: /* Thumb2 32-bit b{cond} or A32 b{cond}.  

Re: [committed] analyzer: avoid comparisons between uncomparable types (PR 93450)

[Jakub Jelinek]

On Thu, Jan 30, 2020 at 09:27:33AM -0500, David Malcolm wrote:
> --- a/gcc/analyzer/region-model.cc
> +++ b/gcc/analyzer/region-model.cc
> @@ -666,12 +666,16 @@ constant_svalue::eval_condition (constant_svalue *lhs,
>gcc_assert (CONSTANT_CLASS_P (lhs_const));
>gcc_assert (CONSTANT_CLASS_P (rhs_const));
>  
> -  tree comparison
> -= fold_build2 (op, boolean_type_node, lhs_const, rhs_const);
> -  if (comparison == boolean_true_node)
> -return tristate (tristate::TS_TRUE);
> -  if (comparison == boolean_false_node)
> -return tristate (tristate::TS_FALSE);
> +  /* Check for comparable types.  */
> +  if (TREE_TYPE (lhs_const) == TREE_TYPE (rhs_const))

Isn't the analyzer invoked on GIMPLE?  In GIMPLE, pointer equality of types
is often not guaranteed and the compiler generally doesn't care about that,
what we care about is whether the two types are compatible
(types_compatible_p).  E.g. if originally both types were say long int,
but on lp64 one of the arguments was cast from long long int to long int,
you can end up with one operand long int and the other long long int;
they have the same precision etc., so it is ok to compare them.

> +{
> +  tree comparison
> + = fold_build2 (op, boolean_type_node, lhs_const, rhs_const);
> +  if (comparison == boolean_true_node)
> + return tristate (tristate::TS_TRUE);
> +  if (comparison == boolean_false_node)
> + return tristate (tristate::TS_FALSE);

This seems to be a waste of compile time memory.  fold_build2 is essentially
  tem = fold_binary_loc (loc, code, type, op0, op1);
  if (!tem)
tem = build2_loc (loc, code, type, op0, op1 PASS_MEM_STAT);
but as you only care if the result is boolean_true_node or
boolean_false_node, the build2_loc is unnecessary.  So, just use
fold_binary instead?  If it returns NULL_TREE, it won't compare equal to
either and will fall through to the TS_UNKNOWN case.
> +}
>return tristate::TS_UNKNOWN;
>  }

Jakub

[committed] analyzer: avoid comparisons between uncomparable types (PR 93450)

[David Malcolm]

PR analyzer/93450 reports an ICE trying to fold an EQ_EXPR comparison
of (int)0 with (float)Inf.

Most comparisons inside the analyzer come from gimple conditions, for
which the necessary casts have already been added.

This one is done inside constant_svalue::eval_condition as part of
purging sm-state for an unknown function call, and fails to check
the types being compared, leading to the ICE.

sm_state_map::set_state calls region_model::eval_condition_without_cm in
order to handle pointer equality (so that e.g. (void *) and (foo *)
transition together), which leads to this code generating a bogus query
to see if the two constants are equal.

This patch fixes the ICE in two ways:

- It avoids generating comparisons within
  constant_svalue::eval_condition unless the types are equal (thus for
  constants, but not for pointer values, which are handled by
  region_svalue).

- It updates sm_state_map::set_state to bail immediately if the new
  state is the same as the old one, thus avoiding the above for the
  common case where an svalue_id has no sm-state (such as for the int
  and float constants in the reproducer), for which the above becomes a
  no-op.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Committed to master as r10-6351-gd177c49cd31131c8cededb216da30877d8a3856d.

gcc/analyzer/ChangeLog:
PR analyzer/93450
* program-state.cc (sm_state_map::set_state): For the overload
taking an svalue_id, bail out if the set_state on the ec does
nothing.  Convert the latter's return type from void to bool,
returning true if anything changed.
(sm_state_map::impl_set_state): Convert the return type from void
to bool, returning true if the state changed.
* program-state.h (sm_state_map::set_state): Convert return type
from void to bool.
(sm_state_map::impl_set_state): Likewise.
* region-model.cc (constant_svalue::eval_condition): Only call
fold_build2 if the types are the same.

gcc/testsuite/ChangeLog:
PR analyzer/93450
* gcc.dg/analyzer/torture/pr93450.c: New test.
---
 gcc/analyzer/program-state.cc | 23 +++--
 gcc/analyzer/program-state.h  |  4 +--
 gcc/analyzer/region-model.cc  | 16 +++-
 .../gcc.dg/analyzer/torture/pr93450.c | 25 +++
 4 files changed, 53 insertions(+), 15 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/torture/pr93450.c

diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index a9e300fba0f..f41f105ce6b 100644
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -259,7 +259,8 @@ sm_state_map::set_state (region_model *model,
   if (model == NULL)
 return;
   equiv_class  = model->get_constraints ()->get_equiv_class (sid);
-  set_state (ec, state, origin);
+  if (!set_state (ec, state, origin))
+return;
 
   /* Also do it for all svalues that are equal via non-cm, so that
  e.g. (void *) and (foo *) transition together.  */
@@ -276,34 +277,42 @@ sm_state_map::set_state (region_model *model,
 }
 
 /* Set the state of EC to STATE, recording that the state came from
-   ORIGIN.  */
+   ORIGIN.
+   Return true if any states of svalue_ids within EC changed.  */
 
-void
+bool
 sm_state_map::set_state (const equiv_class ,
 state_machine::state_t state,
 svalue_id origin)
 {
   int i;
   svalue_id *sid;
+  bool any_changed = false;
   FOR_EACH_VEC_ELT (ec.m_vars, i, sid)
-impl_set_state (*sid, state, origin);
+any_changed |= impl_set_state (*sid, state, origin);
+  return any_changed;
 }
 
-/* Set state of PV to STATE, bypassing equivalence classes.  */
+/* Set state of SID to STATE, bypassing equivalence classes.
+   Return true if the state changed.  */
 
-void
+bool
 sm_state_map::impl_set_state (svalue_id sid, state_machine::state_t state,
  svalue_id origin)
 {
+  if (get_state (sid) == state)
+return false;
+
   /* Special-case state 0 as the default value.  */
   if (state == 0)
 {
   if (m_map.get (sid))
m_map.remove (sid);
-  return;
+  return true;
 }
   gcc_assert (!sid.null_p ());
   m_map.put (sid, entry_t (state, origin));
+  return true;
 }
 
 /* Set the "global" state within this state map to STATE.  */
diff --git a/gcc/analyzer/program-state.h b/gcc/analyzer/program-state.h
index adc71a4eda2..0a4e35f3d5d 100644
--- a/gcc/analyzer/program-state.h
+++ b/gcc/analyzer/program-state.h
@@ -161,10 +161,10 @@ public:
  svalue_id sid,
  state_machine::state_t state,
  svalue_id origin);
-  void set_state (const equiv_class ,
+  bool set_state (const equiv_class ,
  state_machine::state_t state,
  svalue_id origin);
-  void impl_set_state (svalue_id sid,
+  bool impl_set_state (svalue_id sid,
   

Re: [PATCH] doc: clarify the situation with pointer arithmetic

[Uecker, Martin]

Am Donnerstag, den 30.01.2020, 09:30 +0100 schrieb Richard Biener:
> On Wed, Jan 29, 2020 at 3:00 PM Uecker, Martin
>  wrote:

...

> > > I guess I'd me much more happy if PVNI said that when
> > > an integer is converted to a pointer and the integer
> > > is value-equivalent to pointers { p1, p2, ... } then
> > > the provenance of the resulting pointer is
> > > that of p1 (or p2, ... which is semantically equivalent)
> > 
> > (if the provenance is the same)
> > 
> > > and when two pointers p1 and p2 are
> > > value-equivalent and their provenance is not the
> > > same then the behavior is undefined.
> > 
> > I see. Then here..
> > 
> > int a[3];
> > int b[3];
> > 
> > (uintptr_t)[0]; // b also exposed
> > int *p = (int*)(uintptr_t)[3];
> > 
> > ..the behavior is undefined because the
> > two pointers have identical addresses
> > but different provenance.
> > 
> > I agree, from a compiler writer's point-of-view
> > this would be a good solution. But to a programmer,
> > this would be quite difficult to explain.
> > The preference of the working group was that the casts
> > should just work in all cases and do what the
> > programmer intended, even if this prevents some
> > optimization. But I will see that this is
> > added to the list of options under consideration.
> > 
> > 
> > PVNI-ae-ud assigns the provenance of an
> > exposed object at the address. If there
> > are two possible objects (as in the example
> > above), the pointer could point to both but
> > then has to be used consistently only with
> > only one object. Essentially, we want the
> > pointer to have exactly one provenance but
> > we might delay the decision. The idea is
> > that a compiler might figure out the correct
> > provenance later, e.g. by observing accesses.
> 
> I thought about alternatives to PVNI and implementation
> consequences.  But all different kind of must-behave-like-this
> guarantees face serious implementation difficulties I think
> so the only alternative to PVNI (which I think is implementable
> but at a optimization opportunity cost) is one that makes
> two pointers with the same value always have the same
> provenance (and otherwise make the behavior undefined).

This would need to come with precise rules about
when the occurance of two such pointers is UB,
e.g. comparisons of such pointers, or that
two such pointers are cast to int in the same
execution.

The mere existance of such pointers should be
quite common and should not already be UB.

But I am uncomfortable with the idea that
comparison of pointers is always allowed except
for some special case which then is UB. This
might cause are and very difficult to find bugs.

> > It is possible to formulate
> > some conditions about when a pointer converted
> > from an integer could get assigned the
> > points-to-set of a value-equivalent pointer:
> > 
> > 1) using knowledge about object location in
> > memory: If there is no adjacent object which
> > was exposed, one can conclude that the
> > provenance is the object at this address.
> 
> Usually at the point compilers want to know objects
> are not laid out.  So what compilers do is simply
> say the user cannot possibly know so it can
> choose at will (even if later object layout disagrees).

The compiler is free to choose at will. But in
my opinion, it then has to stick with its choice.

Otherwise, this leads to really abstract and
confusing semantics. The wording of the standard
also implies that UB is based on actual behavior.

> > 2) based on offsets: If the pointer points
> > in the middle of an object, there is also
> > no ambiguity.
> 
> The difficulty here lies in the requirement of
> exact offset tracking which makes (some?)
> points-to implementations prohibitly expensive.
> But yes, sure.

Yes, but perhaps there are some low-hanging fruit
where it is easy to determine.

> > 3) a mix of both, to differentiate objects
> > before and after in memory.
> > 
> > 
> > >  That is,
> > > 
> > > int a, b;
> > > int  *p =  + 1;
> > > int *q = 
> > > if (p == q)
> > >   ... undefined ...
> > 
> > We considered making the comparison undefined in the
> > specific situation where one of the pointer is one-after
> > pointer and the other a pointer to the beginning of a
> > different object. This would solve the problems with
> > conditional equivalences.
> 
> Note my proposal doesn't make the comparison undefined
> but the case where both are equivalent cannot be reached
> at runtime without invoking undefined behavior.  That means
> we can optimize the comparison based on provenance
> where p points to a and q points to b.

Sorry, I did not get this. What are the exact conditions
for UB?

> > Others proposed to make the result of the comparison
> > unspecified, but I think this does not help.
> 
> Indeed.  It's not unspecified, it's known to evaluate to false.
> I think there's existing wording in the standard that
> allows it to evaluate to true for pointers one-after-the-object,
> that would need to be changed 

Re: [PATCH, ivopts] Fix fast-math-pr55281.c ICE

[Andrew Stubbs]

On 30/01/2020 13:49, Richard Biener wrote:

On Thu, Jan 30, 2020 at 2:04 PM Bin.Cheng  wrote:


On Thu, Jan 30, 2020 at 8:53 PM Andrew Stubbs  wrote:


On 29/01/2020 08:24, Richard Biener wrote:

On Tue, Jan 28, 2020 at 5:53 PM Andrew Stubbs  wrote:


This patch fixes an ICE compiling fast-math-pr55281.c for amdgcn.

The problem is that an "iv" is created in which both base and step are
pointer types,


How did you get a POINTER_TYPE step?  That's where the issue lies
I think.


It can come from "find_inv_vars_cb":

set_iv (idata, op, op, build_int_cst (TREE_TYPE (op), 0), true);


This is recording invariant with zero step.  It seems we are using
wrong type building the zero-step.  How about detecting that op has
pointer type and using integer type here?


that sounds like a good idea.


How about this?

I've only tested it on the one testcase, so far, but it works for that.

OK to commit (following a full test)?

Andrew
Fix fast-math-pr55281.c ICE v2

2020-01-30  Andrew Stubbs  

	gcc/
	* tree-ssa-loop-ivopts.c (get_iv): Use sizetype for zero-step.
	(find_inv_vars_cb): Likewise.

diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c
index a21f3077e74..1ce6d8b372b 100644
--- a/gcc/tree-ssa-loop-ivopts.c
+++ b/gcc/tree-ssa-loop-ivopts.c
@@ -1246,7 +1246,11 @@ get_iv (struct ivopts_data *data, tree var)
 
   if (!bb
 	  || !flow_bb_inside_loop_p (data->current_loop, bb))
-	set_iv (data, var, var, build_int_cst (type, 0), true);
+	{
+	  if (POINTER_TYPE_P (type))
+	type = sizetype;
+	  set_iv (data, var, var, build_int_cst (type, 0), true);
+	}
 }
 
   return name_info (data, var)->iv;
@@ -2990,7 +2994,10 @@ find_inv_vars_cb (tree *expr_p, int *ws ATTRIBUTE_UNUSED, void *data)
 
   if (!bb || !flow_bb_inside_loop_p (idata->current_loop, bb))
 	{
-	  set_iv (idata, op, op, build_int_cst (TREE_TYPE (op), 0), true);
+	  tree steptype = TREE_TYPE (op);
+	  if (POINTER_TYPE_P (steptype))
+	steptype = sizetype;
+	  set_iv (idata, op, op, build_int_cst (steptype, 0), true);
 	  record_invariant (idata, op, false);
 	}
 }

Re: [PATCH, ivopts] Fix fast-math-pr55281.c ICE

[Richard Biener]

On Thu, Jan 30, 2020 at 2:04 PM Bin.Cheng  wrote:
>
> On Thu, Jan 30, 2020 at 8:53 PM Andrew Stubbs  wrote:
> >
> > On 29/01/2020 08:24, Richard Biener wrote:
> > > On Tue, Jan 28, 2020 at 5:53 PM Andrew Stubbs  
> > > wrote:
> > >>
> > >> This patch fixes an ICE compiling fast-math-pr55281.c for amdgcn.
> > >>
> > >> The problem is that an "iv" is created in which both base and step are
> > >> pointer types,
> > >
> > > How did you get a POINTER_TYPE step?  That's where the issue lies
> > > I think.
> >
> > It can come from "find_inv_vars_cb":
> >
> >set_iv (idata, op, op, build_int_cst (TREE_TYPE (op), 0), true);
>
> This is recording invariant with zero step.  It seems we are using
> wrong type building the zero-step.  How about detecting that op has
> pointer type and using integer type here?

that sounds like a good idea.

> Thanks,
> bin
> >
> > whenever "op" has a pointer type.
> >
> > Similarly for "get_iv":
> >
> >set_iv (data, var, var, build_int_cst (type, 0), true);
> >
> > whenever "var" has a pointer type.
> >
> > In this particular case, I traced the origin back to the second one, I
> > think (but it's somewhat hard to unpick).
> >
> > I could change one or both of those, but I don't understand enough about
> > the consequences of that to be sure it's the right thing to do. I can
> > confirm that converting the step at this point does appear to have the
> > desired effect in this instance.
> >
> > At least at the point of writing it to gimple I can determine what is
> > definitely malformed.
> >
> > Andrew

Re: [RFC] [c-family] PR92867 - Add returns_arg attribute

[Richard Biener]

On Thu, Jan 30, 2020 at 11:49 AM Prathamesh Kulkarni
 wrote:
>
> On Wed, 29 Jan 2020 at 14:38, Richard Biener  
> wrote:
> >
> > On Tue, Jan 28, 2020 at 1:02 PM Jakub Jelinek  wrote:
> > >
> > > On Tue, Jan 28, 2020 at 05:09:36PM +0530, Prathamesh Kulkarni wrote:
> > > > On Tue, 28 Jan 2020 at 17:00, Jakub Jelinek  wrote:
> > > > >
> > > > > On Tue, Jan 28, 2020 at 04:56:59PM +0530, Prathamesh Kulkarni wrote:
> > > > > > Thanks for the suggestions. In the attached patch I bumped up value 
> > > > > > of
> > > > > > ERF_RETURNS_ARG_MASK
> > > > > > to UINT_MAX >> 2, and use highest two bits for ERF_NOALIAS and 
> > > > > > ERF_RETURNS_ARG.
> > > > > > And use fn spec "Z" to store the argument number in fn-spec 
> > > > > > format.
> > > > > > Does that look OK ?
> > > > >
> > > > > No.
> > > > >
> > > > > +#define ERF_RETURN_ARG_MASK(UINT_MAX >> 2)
> > > > >
> > > > >  /* Nonzero if the return value is equal to the argument number
> > > > > flags & ERF_RETURN_ARG_MASK.  */
> > > > > -#define ERF_RETURNS_ARG(1 << 2)
> > > > > +#define ERF_RETURNS_ARG(1 << (BITS_PER_WORD - 2))
> > > > >
> > > > > How is size of host int related to BITS_PER_WORD?  Not to mention that
> > > > > if BITS_PER_WORD is 64 and host int is 32-bit, 1 << (64 - 2) is UB.
> > > > Oops sorry. I should have used HOST_BITS_PER_INT.
> > > > I assume that'd be correct ?
> > >
> > > It still wouldn't, 1 << (HOST_BITS_PER_INT - 1) is negative number, you'd
> > > need either 1U and verify all ERF_* flags uses, or avoid using the sign 
> > > bit.
> > > The patch has other issues, you don't verify that the argnum fits into
> > > the bits (doesn't overflow into the other ERF_* bits), in
> > > +  char *s = (char *) xmalloc (sizeof (char) * BITS_PER_WORD);
> > > +  s[0] = 'Z';
> > > +  sprintf (s + 1, "%lu", argnum);
> > > 1) sizeof (char) is 1 by definition
> > > 2) it is pointless to allocate it and then deallocate (just use automatic
> > > array)
> > > 3) it is unclear how is BITS_PER_WORD related to the length of decimal
> > > encoded string + Z char + terminating '\0'.  The usual way is for unsigned
> > > numbers to estimate number of digits by counting 3 digits per each 8 bits,
> > > in your case of course + 2.
> > >
> > > More importantly, the "fn spec" attribute isn't used just in
> > > gimple_call_return_flags, but also in e.g. gimple_call_arg_flags which
> > > assumes that the return stuff in there is a single char and the reaming
> > > chars are for argument descriptions, or in decl_return_flags which you
> > > haven't modified.
> > >
> > > I must say I fail to see the point in trying to glue this together into 
> > > the
> > > "fn spec" argument so incompatibly, why can't we handle the fn spec with 
> > > its
> > > 1-4 returns_arg and returns_arg attribute with arbitrary position
> > > side-by-side?
> >
> > Yeah, I wouldn't have added "fn spec" for "returns_arg" but kept the
> > query interface thus access it via gimple_call_return_flags and use
> > ERF_*.  For the flags adjustment just up the maximum argument
> > to 1<<15 then the argument number is also nicely aligned, no need
> > to do fancy limiting that depends on the host.  For too large
> > argument numbers just warn the attribute is ignored.  I'd say even
> > a max of 255 is sane just the existing limit is a bit too low.
> Hi,
> Thanks for the suggestions. In the attached patch, I use TREE_VALUE
> (attr) to store/retrieve
> arbitrary argument position, and have bumped ERF_RETURNS_ARG_MASK to 0x3fff.
> Does it look OK ?

+  warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+ "%qE attribute ignored on a function returning %qT.",
+ name, rettype);

no punctuation in diagnostics (trailing '.'), also elsewhere in the patch.

+  tree fndecl = gimple_call_fndecl (stmt);
+  attr = lookup_attribute ("returns_arg", DECL_ATTRIBUTES (fndecl));
+  if (attr)
+{
+  unsigned argnum = tree_to_uhwi (TREE_VALUE (TREE_VALUE (attr)));
+  return ERF_RETURNS_ARG | argnum;

please verify argnum against ERF_ARG_MASK.

+  tree attr = lookup_attribute ("returns_arg", DECL_ATTRIBUTES (decl));
+  if (attr)
+TREE_VALUE (attr) = build_int_cst (unsigned_type_node, argnum);
+  else
+DECL_ATTRIBUTES (decl)
+  = tree_cons (get_identifier ("returns_arg"),
+  build_int_cst (unsigned_type_node, argnum),
+ DECL_ATTRIBUTES (decl));
+  return NULL_TREE;

what's this for?  for *no_add_attrs = false you get the attribute
added by the caller.
Also other positional_argument callers overwrite TREE_VALUE with the result
from the function.

> Thanks,
> Prathamesh
> >
> > Richard.
> >
> > > Jakub
> > >

Re: [PATCH] gcc: Add new configure options to allow static libraries to be selected

[Andrew Burgess]

* Iain Sandoe  [2020-01-28 10:34:52 +]:

> Hi Andrew,
> 
> Andrew Burgess  wrote:
> 
> > * Jeff Law  [2020-01-22 13:52:27 -0700]:
> > 
> > > On Wed, 2020-01-22 at 15:39 +, Andrew Burgess wrote:
> > > > The motivation behind this change is to make it easier for a user to
> > > > link against static libraries on a target where dynamic libraries are
> > > > the default library type (for example GNU/Linux).
> > > > 
> > > > Further, my motivation is really for linking libraries into GDB,
> > > > however, the binutils-gdb/config/ directory is a copy of gcc/config/
> > > > so changes for GDB need to be approved by the GCC project first.
> > > > 
> > > > After making this change in the gcc/config/ directory I've run
> > > > autoreconf on all of the configure scripts in the GCC tree and a
> > > > couple have been updated, so I'll use one of these to describe what my
> > > > change does.
> > > > 
> > > > Consider libcpp, this library links against libiconv.  Currently if
> > > > the user builds on a system with both static and dynamic libiconv
> > > > installed then autotools will pick up the dynamic libiconv by
> > > > default.  This is almost certainly the right thing to do.
> > > > 
> > > > However, if the user wants to link against static libiconv then things
> > > > are a little harder, they could remove the dynamic libiconv from their
> > > > system, but this is probably a bad idea (other things might depend on
> > > > that library), or the user can build their own version of libiconv,
> > > > install it into a unique prefix, and then configure gcc using the
> > > > --with-libiconv-prefix=DIR flag.  This works fine, but is somewhat
> > > > annoying, the static library available, I just can't get autotools to
> > > > use it.
> > > > 
> > > > My change then adds a new flag --with-libiconv-type=TYPE, where type
> > > > is either auto, static, or shared.  The default auto, ensures we keep
> > > > the existing behaviour unchanged.
> > > > 
> > > > If the user configures with --with-libiconv-type=static then the
> > > > configure script will ignore any dynamic libiconv it finds, and will
> > > > only look for a static libiconv, if no static libiconv is found then
> > > > the configure will continue as though there is no libiconv at all
> > > > available.
> > > > 
> > > > Similarly a user can specify --with-libiconv-type=shared and force the
> > > > use of shared libiconv, any static libiconv will be ignored.
> > > > 
> > > > As I've implemented this change within the AC_LIB_LINKFLAGS_BODY macro
> > > > then only libraries configured using the AC_LIB_LINKFLAGS or
> > > > AC_LIB_HAVE_LINKFLAGS macros will gain the new configure flag.
> > > > 
> > > > If this is accepted into GCC then there will be follow on patches for
> > > > binutils and GDB to regenerate some configure scripts in those
> > > > projects.
> > > > 
> > > > For GCC only two configure scripts needed updated after this commit,
> > > > libcpp and libstdc++-v3, both of which link against libiconv.
> 
> This kinda surprises me, gcc/ also configures for iconv
> 
> > > > config/ChangeLog:
> > > > 
> > > > * lib-link.m4 (AC_LIB_LINKFLAGS_BODY): Add new
> > > > --with-libXXX-type=... option.  Use this to guide the selection 
> > > > of
> > > > either a shared library or a static library.
> > > > 
> > > > libcpp/ChangeLog:
> > > > 
> > > > * configure: Regnerate.
> > > > 
> > > > libstdc++-v3/ChangeLog:
> > > > 
> > > > * configure: Regnerate.
> > > s/Regnerate/Regenerate/
> > > 
> > > This isn't strictly a regression bugfix.  But given the nature of these
> > > files I think we probably need to be a bit more lax and allow safe
> > > changes so that downstream uses can move forward independent of the gcc
> > > development and release schedule.
> > > 
> > > So, OK.
> > 
> > Thanks for the flexibility.  Now pushed.
> 
> this (r10-6269,
> https://gcc.gnu.org/g:e7c26e04b2dd6266d62d5a5825ff7eb44d1cf14e) causes or
> exposes a problem which breaks bootstrap on all Darwin platforms I tried.
> 
> Bootstrap fails stage1 self-check with:
> cc1: internal compiler error: in on_diagnostic, at input.c:2182

First, massive apologies for breaking this.  I'm aware that Jeff bent
the rules to allow me to commit this patch, so I feel terrible for
breaking things as a result.

> 
> * AFAICT, this is caused by self-test attempting to do something that libcpp
> was not configured to support.
> 
> * All viable Darwin platforms have libiconv installed (but Darwin’s /lib is
> /usr/lib; this might well apply to other BSD derivatives too).
> 
>  * Before the patch, libcpp and gcc configury finds this and they agree on 
> the availability of ICONV (#define HAVE_ICONV 1).
> 
>  * After the patch libcpp no longer thinks iconv is available, but gcc 
> continues to find it - and that, I think, leads to it attempting tests for 
> which libcpp has not been configured.
>

This must be a bug, right?  With no changes to the configure scripts
both 

Re: [PR47785] COLLECT_AS_OPTIONS

[Richard Biener]

On Thu, Jan 30, 2020 at 5:31 AM Prathamesh Kulkarni
 wrote:
>
> On Tue, 28 Jan 2020 at 17:17, Richard Biener  
> wrote:
> >
> > On Fri, Jan 24, 2020 at 7:04 AM Prathamesh Kulkarni
> >  wrote:
> > >
> > > On Mon, 20 Jan 2020 at 15:44, Richard Biener  
> > > wrote:
> > > >
> > > > On Wed, Jan 8, 2020 at 11:20 AM Prathamesh Kulkarni
> > > >  wrote:
> > > > >
> > > > > On Tue, 5 Nov 2019 at 17:38, Richard Biener 
> > > > >  wrote:
> > > > > >
> > > > > > On Tue, Nov 5, 2019 at 12:17 AM Kugan Vivekanandarajah
> > > > > >  wrote:
> > > > > > >
> > > > > > > Hi,
> > > > > > > Thanks for the review.
> > > > > > >
> > > > > > > On Tue, 5 Nov 2019 at 03:57, H.J. Lu  wrote:
> > > > > > > >
> > > > > > > > On Sun, Nov 3, 2019 at 6:45 PM Kugan Vivekanandarajah
> > > > > > > >  wrote:
> > > > > > > > >
> > > > > > > > > Thanks for the reviews.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > On Sat, 2 Nov 2019 at 02:49, H.J. Lu  
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > On Thu, Oct 31, 2019 at 6:33 PM Kugan Vivekanandarajah
> > > > > > > > > >  wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Wed, 30 Oct 2019 at 03:11, H.J. Lu 
> > > > > > > > > > >  wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > On Sun, Oct 27, 2019 at 6:33 PM Kugan Vivekanandarajah
> > > > > > > > > > > >  wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > Hi Richard,
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thanks for the review.
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Wed, 23 Oct 2019 at 23:07, Richard Biener 
> > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Mon, Oct 21, 2019 at 10:04 AM Kugan 
> > > > > > > > > > > > > > Vivekanandarajah
> > > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Hi Richard,
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Thanks for the pointers.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > On Fri, 11 Oct 2019 at 22:33, Richard Biener 
> > > > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > On Fri, Oct 11, 2019 at 6:15 AM Kugan 
> > > > > > > > > > > > > > > > Vivekanandarajah
> > > > > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > Hi Richard,
> > > > > > > > > > > > > > > > > Thanks for the review.
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > On Wed, 2 Oct 2019 at 20:41, Richard Biener 
> > > > > > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > On Wed, Oct 2, 2019 at 10:39 AM Kugan 
> > > > > > > > > > > > > > > > > > Vivekanandarajah
> > > > > > > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > Hi,
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > As mentioned in the PR, attached patch 
> > > > > > > > > > > > > > > > > > > adds COLLECT_AS_OPTIONS for
> > > > > > > > > > > > > > > > > > > passing assembler options specified with 
> > > > > > > > > > > > > > > > > > > -Wa, to the link-time driver.
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > The proposed solution only works for 
> > > > > > > > > > > > > > > > > > > uniform -Wa options across all
> > > > > > > > > > > > > > > > > > > TUs. As mentioned by Richard Biener, 
> > > > > > > > > > > > > > > > > > > supporting non-uniform -Wa flags
> > > > > > > > > > > > > > > > > > > would require either adjusting 
> > > > > > > > > > > > > > > > > > > partitioning according to flags or
> > > > > > > > > > > > > > > > > > > emitting multiple object files  from a 
> > > > > > > > > > > > > > > > > > > single LTRANS CU. We could
> > > > > > > > > > > > > > > > > > > consider this as a follow up.
> > > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > > Bootstrapped and regression tests on  
> > > > > > > > > > > > > > > > > > > arm-linux-gcc. Is this OK for trunk?
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > While it works for your simple cases it is 
> > > > > > > > > > > > > > > > > > unlikely to work in practice since
> > > > > > > > > > > > > > > > > > your implementation needs the assembler 
> > > > > > > > > > > > > > > > > > options be present at the link
> > > > > > > > > > > > > > > > > > command line.  I agree that this might be 
> > > > > > > > > > > > > > > > > > the way for people to go when
> > > > > > > > > > > > > > > > > > they face the issue but then it needs to be 
> > > > > > > > > > > > > > > > > > documented somewhere
> > > > > > > > > > > > > > > > > > in the manual.
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > That is, with COLLECT_AS_OPTION (why 
> > > > > > > > > > > > > > > > > > singular?  I'd expected
> > > > > > > > > > > > > > > > > > 

[PATCH] dump CTORs properly wrapped with _Literal with -gimple

[Richard Biener]

This wraps { ... } in _Literal (type) for consumption by the GIMPLE FE.

Bootstrap & regtest ongoing on x86_64-unknown-linux-gnu, will push
to trunk since it eases debugging.

Richard.

2020-01-30  Richard Biener  

* tree-pretty-print.c (dump_generic_node): Wrap VECTOR_CST
and CONSTRUCTOR in _Literal (type) with TDF_GIMPLE.
---
 gcc/tree-pretty-print.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
index fe2e62b31ba..7de8c7b0cb7 100644
--- a/gcc/tree-pretty-print.c
+++ b/gcc/tree-pretty-print.c
@@ -2078,6 +2078,12 @@ dump_generic_node (pretty_printer *pp, tree node, int 
spc, dump_flags_t flags,
 case VECTOR_CST:
   {
unsigned i;
+   if (flags & TDF_GIMPLE)
+ {
+   pp_string (pp, "_Literal (");
+   dump_generic_node (pp, TREE_TYPE (node), spc, flags, false);
+   pp_string (pp, ") ");
+ }
pp_string (pp, "{ ");
unsigned HOST_WIDE_INT nunits;
if (!VECTOR_CST_NELTS (node).is_constant ())
@@ -2315,6 +2321,12 @@ dump_generic_node (pretty_printer *pp, tree node, int 
spc, dump_flags_t flags,
bool is_struct_init = false;
bool is_array_init = false;
widest_int curidx;
+   if (flags & TDF_GIMPLE)
+ {
+   pp_string (pp, "_Literal (");
+   dump_generic_node (pp, TREE_TYPE (node), spc, flags, false);
+   pp_string (pp, ") ");
+ }
pp_left_brace (pp);
if (TREE_CLOBBER_P (node))
  pp_string (pp, "CLOBBER");
-- 
2.16.4

[PATCH] tree-optimization/93508 - make VN translate through _chk and valueize length

[Richard Biener]

Value-numbering failed to handle __builtin_{memcpy,memset,...}_chk
variants when removing abstraction and also failed to use the
value-numbering lattice when requiring the length argument of the
call to be constant.

Bootstrapped and tested on x86_64-unknown-linux-gnu, queued for GCC 11
unless somebody things we want this right now.

Richard.

2020-01-30  Richard Biener  

PR tree-optimization/93508
* tree-ssa-sccvn.c (vn_reference_lookup_3): Handle _CHK like
non-_CHK variants.  Valueize their length arguments.

* gcc.dg/tree-ssa/ssa-fre-85.c: New testcase.
---
 gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-85.c | 13 +
 gcc/tree-ssa-sccvn.c   | 19 +++
 2 files changed, 28 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-85.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-85.c 
b/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-85.c
new file mode 100644
index 000..6dace16ecbd
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-85.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-fre1" } */
+
+unsigned int foo(unsigned int x, int *p)
+{
+  unsigned int src = x;
+  unsigned int dst;
+  *p = sizeof (unsigned int);
+  __builtin___memcpy_chk (, , *p, 16);
+  return dst;
+}
+
+/* { dg-final { scan-tree-dump "return x" "fre1" } } */
diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c
index 6e0b2202157..632211f9887 100644
--- a/gcc/tree-ssa-sccvn.c
+++ b/gcc/tree-ssa-sccvn.c
@@ -2377,14 +2377,17 @@ vn_reference_lookup_3 (ao_ref *ref, tree vuse, void 
*data_,
  from that definition.
  1) Memset.  */
   if (is_gimple_reg_type (vr->type)
-  && gimple_call_builtin_p (def_stmt, BUILT_IN_MEMSET)
+  && (gimple_call_builtin_p (def_stmt, BUILT_IN_MEMSET)
+ || gimple_call_builtin_p (def_stmt, BUILT_IN_MEMSET_CHK))
   && (integer_zerop (gimple_call_arg (def_stmt, 1))
  || ((TREE_CODE (gimple_call_arg (def_stmt, 1)) == INTEGER_CST
   || (INTEGRAL_TYPE_P (vr->type) && known_eq (ref->size, 8)))
  && CHAR_BIT == 8 && BITS_PER_UNIT == 8
  && offset.is_constant ()
  && offseti % BITS_PER_UNIT == 0))
-  && poly_int_tree_p (gimple_call_arg (def_stmt, 2))
+  && (poly_int_tree_p (gimple_call_arg (def_stmt, 2))
+ || (TREE_CODE (gimple_call_arg (def_stmt, 2)) == SSA_NAME
+ && poly_int_tree_p (SSA_VAL (gimple_call_arg (def_stmt, 2)
   && (TREE_CODE (gimple_call_arg (def_stmt, 0)) == ADDR_EXPR
  || TREE_CODE (gimple_call_arg (def_stmt, 0)) == SSA_NAME))
 {
@@ -2444,6 +2447,8 @@ vn_reference_lookup_3 (ao_ref *ref, tree vuse, void 
*data_,
   else
return (void *)-1;
   tree len = gimple_call_arg (def_stmt, 2);
+  if (TREE_CODE (len) == SSA_NAME)
+   len = SSA_VAL (len);
   HOST_WIDE_INT leni, offset2i, offseti;
   /* Sometimes the above trickery is smarter than alias analysis.  Take
  advantage of that.  */
@@ -2925,13 +2930,19 @@ vn_reference_lookup_3 (ao_ref *ref, tree vuse, void 
*data_,
   && is_gimple_reg_type (vr->type)
   /* ???  Handle BCOPY as well.  */
   && (gimple_call_builtin_p (def_stmt, BUILT_IN_MEMCPY)
+  || gimple_call_builtin_p (def_stmt, BUILT_IN_MEMCPY_CHK)
   || gimple_call_builtin_p (def_stmt, BUILT_IN_MEMPCPY)
-  || gimple_call_builtin_p (def_stmt, BUILT_IN_MEMMOVE))
+  || gimple_call_builtin_p (def_stmt, BUILT_IN_MEMPCPY_CHK)
+  || gimple_call_builtin_p (def_stmt, BUILT_IN_MEMMOVE)
+  || gimple_call_builtin_p (def_stmt, BUILT_IN_MEMMOVE_CHK))
   && (TREE_CODE (gimple_call_arg (def_stmt, 0)) == ADDR_EXPR
   || TREE_CODE (gimple_call_arg (def_stmt, 0)) == SSA_NAME)
   && (TREE_CODE (gimple_call_arg (def_stmt, 1)) == ADDR_EXPR
   || TREE_CODE (gimple_call_arg (def_stmt, 1)) == SSA_NAME)
-  && poly_int_tree_p (gimple_call_arg (def_stmt, 2), _size)
+  && (poly_int_tree_p (gimple_call_arg (def_stmt, 2), _size)
+  || (TREE_CODE (gimple_call_arg (def_stmt, 2)) == SSA_NAME
+  && poly_int_tree_p (SSA_VAL (gimple_call_arg (def_stmt, 2)),
+  _size)))
   /* Handling this is more complicated, give up for now.  */
   && data->partial_defs.is_empty ())
 {
-- 
2.16.4

Re: [PATCH, ivopts] Fix fast-math-pr55281.c ICE

[Bin.Cheng]

On Thu, Jan 30, 2020 at 8:53 PM Andrew Stubbs  wrote:
>
> On 29/01/2020 08:24, Richard Biener wrote:
> > On Tue, Jan 28, 2020 at 5:53 PM Andrew Stubbs  wrote:
> >>
> >> This patch fixes an ICE compiling fast-math-pr55281.c for amdgcn.
> >>
> >> The problem is that an "iv" is created in which both base and step are
> >> pointer types,
> >
> > How did you get a POINTER_TYPE step?  That's where the issue lies
> > I think.
>
> It can come from "find_inv_vars_cb":
>
>set_iv (idata, op, op, build_int_cst (TREE_TYPE (op), 0), true);

This is recording invariant with zero step.  It seems we are using
wrong type building the zero-step.  How about detecting that op has
pointer type and using integer type here?

Thanks,
bin
>
> whenever "op" has a pointer type.
>
> Similarly for "get_iv":
>
>set_iv (data, var, var, build_int_cst (type, 0), true);
>
> whenever "var" has a pointer type.
>
> In this particular case, I traced the origin back to the second one, I
> think (but it's somewhat hard to unpick).
>
> I could change one or both of those, but I don't understand enough about
> the consequences of that to be sure it's the right thing to do. I can
> confirm that converting the step at this point does appear to have the
> desired effect in this instance.
>
> At least at the point of writing it to gimple I can determine what is
> definitely malformed.
>
> Andrew

Re: [PATCH, ivopts] Fix fast-math-pr55281.c ICE

[Andrew Stubbs]

On 29/01/2020 08:24, Richard Biener wrote:

On Tue, Jan 28, 2020 at 5:53 PM Andrew Stubbs  wrote:


This patch fixes an ICE compiling fast-math-pr55281.c for amdgcn.

The problem is that an "iv" is created in which both base and step are
pointer types,


How did you get a POINTER_TYPE step?  That's where the issue lies
I think.


It can come from "find_inv_vars_cb":

  set_iv (idata, op, op, build_int_cst (TREE_TYPE (op), 0), true);

whenever "op" has a pointer type.

Similarly for "get_iv":

  set_iv (data, var, var, build_int_cst (type, 0), true);

whenever "var" has a pointer type.

In this particular case, I traced the origin back to the second one, I 
think (but it's somewhat hard to unpick).


I could change one or both of those, but I don't understand enough about 
the consequences of that to be sure it's the right thing to do. I can 
confirm that converting the step at this point does appear to have the 
desired effect in this instance.


At least at the point of writing it to gimple I can determine what is 
definitely malformed.


Andrew

[committed] Fix ICE in pa_elf_select_rtx_section

[John David Anglin]

An ICE was noticed in pa_elf_select_rtx_section building googletest on 
hppa-unknown-linux-gnu.
This change fixes the problem.  It puts function pointer rtx's without a DECL 
in .data.rel.ro.local.

Tested on hppa-unknown-linux-gnu.  Committed to trunk and gcc-9 branch.

Dave

2020-01-30  John David Anglin  

* config/pa/pa.c (pa_elf_select_rtx_section): Place function pointers
without a DECL in .data.rel.ro.local.

diff --git a/gcc/config/pa/pa.c b/gcc/config/pa/pa.c
index fb7e2ee110f..24b88304637 100644
--- a/gcc/config/pa/pa.c
+++ b/gcc/config/pa/pa.c
@@ -9852,7 +9852,7 @@ pa_elf_select_rtx_section (machine_mode mode, rtx x,
 {
   tree decl = SYMBOL_REF_DECL (x);

-  if (DECL_P (decl) && DECL_COMDAT_GROUP (decl))
+  if (!decl || (DECL_P (decl) && DECL_COMDAT_GROUP (decl)))
return get_named_section (NULL, ".data.rel.ro.local", 1);
 }

Re: [PATCH, v2] coroutines: Fix ICE on invalid (PR93458).

[Nathan Sidwell]

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

Hi Nathan,

Nathan Sidwell  wrote:



Made the function type error recorded per function too.

OK now?


Still some things to address ...


diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index e8a6a4033f6..3ad80699ca0 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc



@@ -257,9 +260,15 @@ find_coro_traits_template_decl (location_t kw)
  {
tree traits_decl = lookup_qualified_name (std_node, coro_traits_identifier,
0, true);
-  if (traits_decl == NULL_TREE || traits_decl == error_mark_node)
-{
-  error_at (kw, "cannot find % template");
+  /* If we are missing fundmental information, such as the traits, then don't
+ emit this for every keyword in a TU.  This particular error is per TU
+ so we don't need to keep the indicator per function.  */
+  static bool traits_error_emitted = false;


You can of course move this into the if's block scope.


+  if (traits_decl == error_mark_node)
+{
+  if (!traits_error_emitted)
+   error_at (kw, "cannot find % template");


Give the name you were looking for:
   "%<%E::%E%> ...", std_node, coro_traits_identifier
also, what if you find something, but it's not a type template?


/* Coroutine traits template.  */
coro_traits_templ = find_coro_traits_template_decl (loc);
-  gcc_checking_assert (coro_traits_templ != NULL);
+  if (coro_traits_templ == NULL_TREE
+ || coro_traits_templ == error_mark_node)
+   return false;


ISTM that find_coro_traits_template_decl should be returning exactly one 
of NULL_TREE of error_mark_node on failure.  Its users don't 
particularly care why it failed (not found vs found ambiguous/not template).


 +  /* Save the coroutine data on the side to avoid the overhead on every

+ function decl tree.  */
+
coroutine_info *coro_info = get_or_insert_coroutine_info (fndecl);
/* Without this, we cannot really proceed.  */
gcc_checking_assert (coro_info);
@@ -407,6 +427,18 @@ coro_promise_type_found_p (tree fndecl, location_t loc)
  {
/* Get the coroutine traits template class instance for the function
 signature we have - coroutine_traits   */
+  if (!CLASS_TYPE_P (TREE_TYPE (TREE_TYPE (fndecl
+   {
+ /* It makes more sense to show the function header for this, even
+though we will have encountered it when processing a keyword.
+Only emit the error once, not for every keyword we encounter.  */
+ if (!coro_info->coro_ret_type_error_emitted)
+   error_at (DECL_SOURCE_LOCATION (fndecl), "a coroutine must have a"
+ " class or struct return type");


Perhaps something like "coroutine return type %qT is not a class"?  I.e. 
show them the type.

(structs are classes, there's no need to say 'class or struct')



+ coro_info->coro_ret_type_error_emitted = true;
+ return false;
+   }
+
tree templ_class = instantiate_coro_traits (fndecl, loc);
  
/* Find the promise type for that.  */

@@ -422,7 +454,7 @@ coro_promise_type_found_p (tree fndecl, location_t loc)
/* Try to find the handle type for the promise.  */
tree handle_type =
instantiate_coro_handle_for_promise_type (loc, coro_info->promise_type);
-  if (handle_type == NULL_TREE)
+  if (handle_type == NULL_TREE || handle_type == error_mark_node)


similar to coro_traits_template_decl.


--
Nathan Sidwell

Re: [PATCH] fortran: Fix up ISO_Fortran_binding_15.f90 failures [PR92123]

[Paul Richard Thomas]

Hi All,

Thank you for taking care of that, Jakub. I have been overwhelmed by
daytime work since my last posting and, in addition, have yet to set
up a git workflow. This situation is likely to continue for at least
another 3-4 months. I will release all the PRs assigned to me, except
those associated with PDTs for which I do not think there will be any
other takers.

With regard to the other fn spec attributes: I think that we can
assume that they are OK, or at least acceptable, since I did not find
any similar PRs that might implicate a similar problem.

Cheers

Paul

On Thu, 30 Jan 2020 at 07:37, Richard Biener  wrote:
>
> On Thu, 30 Jan 2020, Jakub Jelinek wrote:
>
> > Hi!
> >
> > This is something that has been discussed already a few months ago, but
> > seems to have stalled.  Here is Paul's patch from the PR except for the
> > TREE_STATIC hunk which is wrong, and does the most conservative fn spec
> > tweak for the problematic two builtins we are aware of (to repeat what is in
> > the PR, both .wR and .ww are wrong for these builtins that transform one
> > layout of an descriptor to another one; while the first pointer is properly
> > marked that we only store to what it points to, from the second pointer
> > we copy and reshuffle the content and store into the first one; if there
> > wouldn't be any pointers, ".wr" would be just fine, but as there is a
> > pointer and that pointer is copied to the area pointed by first argument,
> > the pointer effectively leaks that way, so we e.g. can't optimize stores
> > into what the data pointer in the descriptor points to).  I haven't
> > analyzed other fn spec attributes in the FE, but think it is better to
> > fix at least this one we have analyzed.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> OK.
>
> Thanks,
> Richard.
>
> > 2020-01-30  Paul Thomas  
> >   Jakub Jelinek  
> >
> >   PR fortran/92123
> >   * trans-decl.c (gfc_get_symbol_decl): Call gfc_defer_symbol_init for
> >   CFI descs.
> >   (gfc_build_builtin_function_decls): Use ".w." instead of ".ww" or 
> > ".wR"
> >   for gfor_fndecl_{cfi_to_gfc,gfc_to_cfi}.
> >   (convert_CFI_desc): Handle references to CFI descriptors.
> >
> > --- gcc/fortran/trans-decl.c.jj   2020-01-12 11:54:36.600410587 +0100
> > +++ gcc/fortran/trans-decl.c  2020-01-29 10:54:36.771077452 +0100
> > @@ -1552,6 +1552,9 @@ gfc_get_symbol_decl (gfc_symbol * sym)
> >sym->ts.u.cl->backend_decl = build_fold_indirect_ref 
> > (sym->ts.u.cl->backend_decl);
> >  }
> >
> > +  if (is_CFI_desc (sym, NULL))
> > +gfc_defer_symbol_init (sym);
> > +
> >fun_or_res = byref && (sym->attr.result
> >|| (sym->attr.function && sym->ts.deferred));
> >if ((sym->attr.dummy && ! sym->attr.function) || fun_or_res)
> > @@ -3763,12 +3766,17 @@ gfc_build_builtin_function_decls (void)
> >   get_identifier (PREFIX("internal_unpack")), ".wR",
> >   void_type_node, 2, pvoid_type_node, pvoid_type_node);
> >
> > +  /* These two builtins write into what the first argument points to and
> > + read from what the second argument points to, but we can't use R
> > + for that, because the directly pointed structure contains a pointer
> > + which is copied into the descriptor pointed by the first argument,
> > + effectively escaping that way.  See PR92123.  */
> >gfor_fndecl_cfi_to_gfc = gfc_build_library_function_decl_with_spec (
> > - get_identifier (PREFIX("cfi_desc_to_gfc_desc")), ".ww",
> > + get_identifier (PREFIX("cfi_desc_to_gfc_desc")), ".w.",
> >   void_type_node, 2, pvoid_type_node, ppvoid_type_node);
> >
> >gfor_fndecl_gfc_to_cfi = gfc_build_library_function_decl_with_spec (
> > - get_identifier (PREFIX("gfc_desc_to_cfi_desc")), ".wR",
> > + get_identifier (PREFIX("gfc_desc_to_cfi_desc")), ".w.",
> >   void_type_node, 2, ppvoid_type_node, pvoid_type_node);
> >
> >gfor_fndecl_associated = gfc_build_library_function_decl_with_spec (
> > @@ -4398,6 +4406,8 @@ convert_CFI_desc (gfc_wrapped_block * bl
> >   while CFI_desc is the descriptor itself.  */
> >if (DECL_LANG_SPECIFIC (sym->backend_decl))
> >  CFI_desc = GFC_DECL_SAVED_DESCRIPTOR (sym->backend_decl);
> > +  else if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (TREE_TYPE 
> > (sym->backend_decl
> > +CFI_desc = sym->backend_decl;
> >else
> >  CFI_desc = NULL;
> >
> >
> >   Jakub
> >
> >
>
> --
> Richard Biener 
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)



-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein

Re: [PATCH] arm: Fix uaddvdi4 expander [PR93494]

[Richard Sandiford]

Jakub Jelinek  writes:
> Hi!
>
> uaddvdi4 expander has an optimization for the low 32-bits of the 2nd input
> operand known to be 0.  Unfortunately, in that case it only emits copying of
> the low 32 bits to the low 32 bits of the destination, but doesn't emit the
> addition with overflow detection for the high 64 bits.
> Well, to be precise, it emits it, but into an RTL sequence returned by
> gen_uaddvsi4, but that sequence isn't emitted anywhere.
>
> Fixed thusly, bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk?
>
> 2020-01-30  Jakub Jelinek  
>
>   PR target/93494
>   * config/arm/arm.md (uaddvdi4): Actually emit what gen_uaddvsi4
>   returned.
>
>   * gcc.c-torture/execute/pr93494.c: New test.

OK, thanks.

Richard

>
> --- gcc/config/arm/arm.md.jj  2020-01-18 09:39:51.614204056 +0100
> +++ gcc/config/arm/arm.md 2020-01-29 19:56:15.700835823 +0100
> @@ -721,7 +721,7 @@ (define_expand "uaddvdi4"
>if (!arm_add_operand (hi_op2, SImode))
>   hi_op2 = force_reg (SImode, hi_op2);
>  
> -  gen_uaddvsi4 (hi_result, hi_op1, hi_op2, operands[3]);
> +  emit_insn (gen_uaddvsi4 (hi_result, hi_op1, hi_op2, operands[3]));
>  }
>else
>  {
> --- gcc/testsuite/gcc.c-torture/execute/pr93494.c.jj  2020-01-29 
> 20:01:26.849233488 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/pr93494.c 2020-01-29 
> 20:00:59.391639624 +0100
> @@ -0,0 +1,13 @@
> +/* PR target/93494 */
> +
> +unsigned short a;
> +
> +int
> +main ()
> +{
> +  register unsigned long long y = 0;
> +  int x = __builtin_add_overflow (y, 0ULL, );
> +  if (x || a)
> +__builtin_abort ();
> +  return 0;
> +}
>
>   Jakub

Re: [RFC] [c-family] PR92867 - Add returns_arg attribute

[Prathamesh Kulkarni]

On Wed, 29 Jan 2020 at 14:38, Richard Biener  wrote:
>
> On Tue, Jan 28, 2020 at 1:02 PM Jakub Jelinek  wrote:
> >
> > On Tue, Jan 28, 2020 at 05:09:36PM +0530, Prathamesh Kulkarni wrote:
> > > On Tue, 28 Jan 2020 at 17:00, Jakub Jelinek  wrote:
> > > >
> > > > On Tue, Jan 28, 2020 at 04:56:59PM +0530, Prathamesh Kulkarni wrote:
> > > > > Thanks for the suggestions. In the attached patch I bumped up value of
> > > > > ERF_RETURNS_ARG_MASK
> > > > > to UINT_MAX >> 2, and use highest two bits for ERF_NOALIAS and 
> > > > > ERF_RETURNS_ARG.
> > > > > And use fn spec "Z" to store the argument number in fn-spec 
> > > > > format.
> > > > > Does that look OK ?
> > > >
> > > > No.
> > > >
> > > > +#define ERF_RETURN_ARG_MASK(UINT_MAX >> 2)
> > > >
> > > >  /* Nonzero if the return value is equal to the argument number
> > > > flags & ERF_RETURN_ARG_MASK.  */
> > > > -#define ERF_RETURNS_ARG(1 << 2)
> > > > +#define ERF_RETURNS_ARG(1 << (BITS_PER_WORD - 2))
> > > >
> > > > How is size of host int related to BITS_PER_WORD?  Not to mention that
> > > > if BITS_PER_WORD is 64 and host int is 32-bit, 1 << (64 - 2) is UB.
> > > Oops sorry. I should have used HOST_BITS_PER_INT.
> > > I assume that'd be correct ?
> >
> > It still wouldn't, 1 << (HOST_BITS_PER_INT - 1) is negative number, you'd
> > need either 1U and verify all ERF_* flags uses, or avoid using the sign bit.
> > The patch has other issues, you don't verify that the argnum fits into
> > the bits (doesn't overflow into the other ERF_* bits), in
> > +  char *s = (char *) xmalloc (sizeof (char) * BITS_PER_WORD);
> > +  s[0] = 'Z';
> > +  sprintf (s + 1, "%lu", argnum);
> > 1) sizeof (char) is 1 by definition
> > 2) it is pointless to allocate it and then deallocate (just use automatic
> > array)
> > 3) it is unclear how is BITS_PER_WORD related to the length of decimal
> > encoded string + Z char + terminating '\0'.  The usual way is for unsigned
> > numbers to estimate number of digits by counting 3 digits per each 8 bits,
> > in your case of course + 2.
> >
> > More importantly, the "fn spec" attribute isn't used just in
> > gimple_call_return_flags, but also in e.g. gimple_call_arg_flags which
> > assumes that the return stuff in there is a single char and the reaming
> > chars are for argument descriptions, or in decl_return_flags which you
> > haven't modified.
> >
> > I must say I fail to see the point in trying to glue this together into the
> > "fn spec" argument so incompatibly, why can't we handle the fn spec with its
> > 1-4 returns_arg and returns_arg attribute with arbitrary position
> > side-by-side?
>
> Yeah, I wouldn't have added "fn spec" for "returns_arg" but kept the
> query interface thus access it via gimple_call_return_flags and use
> ERF_*.  For the flags adjustment just up the maximum argument
> to 1<<15 then the argument number is also nicely aligned, no need
> to do fancy limiting that depends on the host.  For too large
> argument numbers just warn the attribute is ignored.  I'd say even
> a max of 255 is sane just the existing limit is a bit too low.
Hi,
Thanks for the suggestions. In the attached patch, I use TREE_VALUE
(attr) to store/retrieve
arbitrary argument position, and have bumped ERF_RETURNS_ARG_MASK to 0x3fff.
Does it look OK ?

Thanks,
Prathamesh
>
> Richard.
>
> > Jakub
> >
diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index dc9579c5c60..c6d5bbd1d7a 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -150,6 +150,7 @@ static tree handle_designated_init_attribute (tree *, tree, tree, int, bool *);
 static tree handle_patchable_function_entry_attribute (tree *, tree, tree,
 		   int, bool *);
 static tree handle_copy_attribute (tree *, tree, tree, int, bool *);
+static tree handle_returns_arg_attribute (tree *, tree, tree, int, bool *);
 
 /* Helper to define attribute exclusions.  */
 #define ATTR_EXCL(name, function, type, variable)	\
@@ -484,6 +485,8 @@ const struct attribute_spec c_common_attribute_table[] =
 			  handle_noinit_attribute, attr_noinit_exclusions },
   { "access",		  1, 3, false, true, true, false,
 			  handle_access_attribute, NULL },
+  { "returns_arg",	  1, 1, true, false, false, false,
+			  handle_returns_arg_attribute, NULL },
   { NULL, 0, 0, false, false, false, false, NULL, NULL }
 };
 
@@ -4603,6 +4606,60 @@ handle_patchable_function_entry_attribute (tree *, tree name, tree args,
   return NULL_TREE;
 }
 
+/* Handle a "returns_arg" attribute; arguments as in
+   struct attribute_spec.handler.  */
+
+static tree
+handle_returns_arg_attribute (tree *node, tree name, tree args,
+			  int, bool *no_add_attrs)
+{
+  tree decl = *node;
+  tree rettype = TREE_TYPE (decl);
+
+  if (TREE_CODE (rettype) == METHOD_TYPE
+  || TREE_CODE (rettype) == FUNCTION_TYPE)
+rettype = TREE_TYPE (rettype);
+
+  if (VOID_TYPE_P (rettype))
+{
+  warning_at 

Re: [GCC][BUG][Aarch64][ARM] (PR93300) Fix ICE due to BFmode placement in GET_MODES_WIDER chain.

[Richard Sandiford]

Stam Markianos-Wright  writes:
> On 1/29/20 12:42 PM, Richard Sandiford wrote:
>> Stam Markianos-Wright  writes:
>>> Hi all,
>>>
>>> This fixes:
>>>
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93300
>>>
>>> Genmodes.c was generating the "wider_mode" chain as follows:
>>>
>>> HF -> BF -> SF - > DF -> TF -> VOID
>>>
>>> This caused issues in some rare cases where conversion between modes
>>> was needed, such as the above PR93300 where BFmode was being picked up
>>> as a valid mode for:
>>>
>>> optabs.c:prepare_float_lib_cmp
>>>
>>> which then led to the ICE at expr.c:convert_mode_scalar.
>
> Hi Richard,
>
>> 
>> Can you go into more details about why this chain was a problem?
>> Naively, it's the one I'd have expected: HF should certainly have
>> priority over BF,
>
> Is that because functionally it looks like genmodes puts things in reverse 
> alphabetical order if all else is equal? (If I'm reading the comment about 
> MODE_RANDOM, MODE_CC correctly)
>
>> but BF coming before SF doesn't seem unusual in itself.
>> 
>> I'm not saying the patch is wrong.  It just wasn't clear why it was
>> right either.
>> 
> Yes, I see what you mean. I'll go through my thought process here:
>
> In investigating the ICE PR93300 I found that the diversion from pre-bf16 
> behaviour was specifically at `optabs.c:prepare_float_lib_cmp`, where a 
> `FOR_EACH_MODE_FROM (mode, orig_mode)` is used to then go off and generate 
> library calls for conversions.
>
> This was then being caught further down by the gcc_assert at expr.c:325 where 
> GET_MODE_PRECISION (from_mode) was equal to GET_MODE_PRECISION (to_mode) 
> because 
> it was trying to emit a HF->BF conversion libcall as `bl __extendhfbf2` 
> (which 
> is what happened if i removed the gcc_assert at expr.c:325)
>
> With BFmode being a target-defined mode, I didn't want to add something like 
> `if 
> (mode != BFmode)` to specifically exclude BFmode from being selected for 
> this. 
> (and there's nothing different between HFmode and BFmode here to allow me to 
> make this distinction?)
>
> Also I couldn't find anywhere where the target back-end is not consulted for 
> a 
> "is this supported: yes/no" between the `FOR_EACH_MODE_FROM` loop and the 
> libcall being created later on as __extendhfbf2.

Yeah, prepare_float_lib_cmp just checks for libfuncs rather than
calling target hooks directly.  The libfuncs themselves are under
the control of the target though.

By default we assume all float modes have associated libfuncs.
It's then up to the target to remove functions that don't exist
(or redirect them to other functions).  So I think we need to remove
BFmode libfuncs in arm_init_libfuncs in the same way as we currently
do for HFmode.

I guess we should also nullify the conversion libfuncs for BFmode,
not just the arithmetic and comparison ones.

Thanks,
Richard

> Finally, because we really don't want __bf16 to be in the same "conversion 
> rank" 
> as standard float modes for things like automatic promotion, this seemed like 
> a 
> reasonable solution to that problem :)
>
> Let me know of your thoughts!
>
> Cheers,
> Stam

Re: [Patch] [libgomp, build] Skip plugin-{gcn,hsa} for (-m)x32 (PR bootstrap/93409)

[Jakub Jelinek]

On Thu, Jan 30, 2020 at 09:47:22AM +, Andrew Stubbs wrote:
> > By "that much", I mean that while the host vs. offloading target interfaces
> > should be ok due to structure layout done in the host compiler and then
> > streamed to the offloading compiler, there is the problem when the
> > offloaded code interfaces using structures with code natively compiled for
> > the offloading target (newlib), so say calling stat or similar functions
> > wouldn't work well.  I'm afraid it won't work well in either offloading
> > model though, in the GCC way struct stat in the offloaded code will be
> > simply the host struct stat with field from there and corresponding
> > structure layout, while in native offloading code probably both different
> > fields and different structure layout, while in the LLVM model I'd assume
> > the offloading code will use the offloading target struct stat, most likely
> > with structure layout from the host.
> 
> Stat is not implemented, nor is any other OS call apart from "write", and
> that's only valid for stdout and stderr (aliased to stdout). It's not that
> semi-hosting could not be implemented; it just hasn't been and there's no
> real use case for it (besides making the testsuite happier). If it were to
> be implemented, however, then indeed x86_64 would be significantly simpler
> than ia32, in many cases.

stat was just a random example that first came to my mind, obviously stat
doesn't make much sense in offloading code, but perhaps other functions
that take address of a structure (or reference to it) and are implemented in
the C library (or say libgfortran or libgomp) could.  In gomp it is e.g.
omp_*_lock, though both omp_lock_t and omp_nest_lock_t just contain a single
char array member and all that matters is the @OMP_LOCK_SIZE@ and
@OMP_NEST_LOCK_SIZE@ matches in between host and offloading target (would be
enough if offloading target has it smaller).

Jakub

Re: [Patch] [libgomp, build] Skip plugin-{gcn,hsa} for (-m)x32 (PR bootstrap/93409)

[Andrew Stubbs]

On 30/01/2020 09:20, Jakub Jelinek wrote:

On Fri, Jan 24, 2020 at 03:59:28PM +0100, Tobias Burnus wrote:

As reported in PR93409, the build of libgomp/plugin/plugin-gcn.c fails with
a bunch of error messages when building with
--with-multilib-list=m32,m64,mx32

The reason is that the GCN plugin assumes 64bit pointers. As with HSA, the
build is only enabled for x86-64 and "-m32" is excluded. — However, it seems
as if it makes sense to exclude also "-mx32".

This patch was tested with -m32/-m64 multilib as I do not have a -mx32
setup.


I don't have any working -mx32 setup around nor any supported GCN offloading
hw around, so can't test anything, thus just a general comment.

In the way LLVM implements offloading, two (or more) separate compilations
starting with preprocessing etc., it is essential to have exactly the same
structure layout in both at least for things, through which the host and
offloading code are interfacing, so one needs say support for offloading
target XYZ do structure layout of ABC host ABI.

The way we implement it in GCC is different, for the structure layout
we perform them pre-IPA in the host compilation, and for offloading
therefore it only matters that the host and offloading target agree on
basics (fundamental types having the same size), the exact structure layout
details shouldn't matter that much.  I don't see the gcn target having
ADJUST_FIELD_ALIGN and all the quirks of the ia32 ABI e.g. with alignment of
long long or double in the structures anyway, yet it seems to be supported
for -m32 (ia32) code, right?  So, I don't see a fundamental reason why
-mx32, which is an ilp32 target like -m32, shouldn't be supported.


-m32 is not supported. Not in the business sense, anyway. It's never 
been tested, no effort has been made to make it work, and "long int" is 
always 64-bit (as are pointers).



By "that much", I mean that while the host vs. offloading target interfaces
should be ok due to structure layout done in the host compiler and then
streamed to the offloading compiler, there is the problem when the
offloaded code interfaces using structures with code natively compiled for
the offloading target (newlib), so say calling stat or similar functions
wouldn't work well.  I'm afraid it won't work well in either offloading
model though, in the GCC way struct stat in the offloaded code will be
simply the host struct stat with field from there and corresponding
structure layout, while in native offloading code probably both different
fields and different structure layout, while in the LLVM model I'd assume
the offloading code will use the offloading target struct stat, most likely
with structure layout from the host.


Stat is not implemented, nor is any other OS call apart from "write", 
and that's only valid for stdout and stderr (aliased to stdout). It's 
not that semi-hosting could not be implemented; it just hasn't been and 
there's no real use case for it (besides making the testsuite happier). 
If it were to be implemented, however, then indeed x86_64 would be 
significantly simpler than ia32, in many cases.



But looking at the patch, we already disable the plugin for ia32 (-m32), so
I'm fine with your patch.  If it is ever enabled for ia32, it should be
enabled for -mx32 too.

Thus ok for trunk.

Jakub

Re: [Patch] [libgomp, build] Skip plugin-{gcn,hsa} for (-m)x32 (PR bootstrap/93409)

[Jakub Jelinek]

On Fri, Jan 24, 2020 at 03:59:28PM +0100, Tobias Burnus wrote:
> As reported in PR93409, the build of libgomp/plugin/plugin-gcn.c fails with
> a bunch of error messages when building with
> --with-multilib-list=m32,m64,mx32
> 
> The reason is that the GCN plugin assumes 64bit pointers. As with HSA, the
> build is only enabled for x86-64 and "-m32" is excluded. — However, it seems
> as if it makes sense to exclude also "-mx32".
> 
> This patch was tested with -m32/-m64 multilib as I do not have a -mx32
> setup.

I don't have any working -mx32 setup around nor any supported GCN offloading
hw around, so can't test anything, thus just a general comment.

In the way LLVM implements offloading, two (or more) separate compilations
starting with preprocessing etc., it is essential to have exactly the same
structure layout in both at least for things, through which the host and
offloading code are interfacing, so one needs say support for offloading
target XYZ do structure layout of ABC host ABI.

The way we implement it in GCC is different, for the structure layout
we perform them pre-IPA in the host compilation, and for offloading
therefore it only matters that the host and offloading target agree on
basics (fundamental types having the same size), the exact structure layout
details shouldn't matter that much.  I don't see the gcn target having
ADJUST_FIELD_ALIGN and all the quirks of the ia32 ABI e.g. with alignment of
long long or double in the structures anyway, yet it seems to be supported
for -m32 (ia32) code, right?  So, I don't see a fundamental reason why
-mx32, which is an ilp32 target like -m32, shouldn't be supported.

By "that much", I mean that while the host vs. offloading target interfaces
should be ok due to structure layout done in the host compiler and then
streamed to the offloading compiler, there is the problem when the
offloaded code interfaces using structures with code natively compiled for
the offloading target (newlib), so say calling stat or similar functions
wouldn't work well.  I'm afraid it won't work well in either offloading
model though, in the GCC way struct stat in the offloaded code will be
simply the host struct stat with field from there and corresponding
structure layout, while in native offloading code probably both different
fields and different structure layout, while in the LLVM model I'd assume
the offloading code will use the offloading target struct stat, most likely
with structure layout from the host.

But looking at the patch, we already disable the plugin for ia32 (-m32), so
I'm fine with your patch.  If it is ever enabled for ia32, it should be
enabled for -mx32 too.

Thus ok for trunk.

Jakub

[PATCH] arm: Fix uaddvdi4 expander [PR93494]

[Jakub Jelinek]

Hi!

uaddvdi4 expander has an optimization for the low 32-bits of the 2nd input
operand known to be 0.  Unfortunately, in that case it only emits copying of
the low 32 bits to the low 32 bits of the destination, but doesn't emit the
addition with overflow detection for the high 64 bits.
Well, to be precise, it emits it, but into an RTL sequence returned by
gen_uaddvsi4, but that sequence isn't emitted anywhere.

Fixed thusly, bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk?

2020-01-30  Jakub Jelinek  

PR target/93494
* config/arm/arm.md (uaddvdi4): Actually emit what gen_uaddvsi4
returned.

* gcc.c-torture/execute/pr93494.c: New test.

--- gcc/config/arm/arm.md.jj2020-01-18 09:39:51.614204056 +0100
+++ gcc/config/arm/arm.md   2020-01-29 19:56:15.700835823 +0100
@@ -721,7 +721,7 @@ (define_expand "uaddvdi4"
   if (!arm_add_operand (hi_op2, SImode))
hi_op2 = force_reg (SImode, hi_op2);
 
-  gen_uaddvsi4 (hi_result, hi_op1, hi_op2, operands[3]);
+  emit_insn (gen_uaddvsi4 (hi_result, hi_op1, hi_op2, operands[3]));
 }
   else
 {
--- gcc/testsuite/gcc.c-torture/execute/pr93494.c.jj2020-01-29 
20:01:26.849233488 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr93494.c   2020-01-29 
20:00:59.391639624 +0100
@@ -0,0 +1,13 @@
+/* PR target/93494 */
+
+unsigned short a;
+
+int
+main ()
+{
+  register unsigned long long y = 0;
+  int x = __builtin_add_overflow (y, 0ULL, );
+  if (x || a)
+__builtin_abort ();
+  return 0;
+}

Jakub

Re: [PATCH] doc: clarify the situation with pointer arithmetic

[Richard Biener]

On Wed, Jan 29, 2020 at 3:00 PM Uecker, Martin
 wrote:
>
> Am Mittwoch, den 29.01.2020, 09:45 +0100 schrieb Richard Biener:
> > On Tue, Jan 28, 2020 at 1:24 PM Uecker, Martin
> >  wrote:
>
> > > >
> > > > Note for the current PTA implementation there's almost no cases we can
> > > > handle conservatively enough.  Consider the simple
> > > >
> > > >  int a[4];
> > > >  int *p = [1];
> > > >  uintptr_t pi = (uintptr_t)p;
> > > >  pi += 4;
> > > >  int *q = (int *)pi;
> > > >
> > > > our PTA knows that p points to a (but not the exact offset), same for pi
> > > > (the cast doesn't change the value).  Now you add 4 - this could lead
> > > > you outside of 'a' so the points-to set becomes 'a and anything'.
> > >
> > > PVNI would say that 'a' gets exposed in the first case and
> > > then 'q' can point to all exposed objects because of the
> > > second cast.
> > >
> > > A correct and conservative implementation would do this:
> > > In the PTA you would need to mark the address of a escaped
> > > and then later assign a conservative points-to set (all
> > > escaped objects) to q.
> >
> > I see (I'm asking all these questions to see what implementing -fpta-pnvi
> > would mean).
>
> Thank you for looking into this.
>
> > That might be a feasible implementation route.  How
> > does "escaped" as used in your answer apply when doing inter-procedural
> > analysis?  I guess we would need to assume points-to sets include
> > automatic variables that escaped in the caller.
>
> Yes. Is this not something the compiler assumes in general?
>
> If you have code like this:
>
> int pi = foo(p);
> int *q = bar(pi);
>
> where 'foo' and 'bar' are unknown to the compiler, it
> should have the same  semantics with regard to PTA
> as needed for the casts in PVNI.
>
> (except that one knows that q and p have the same
> value which could be exploited for optimization)
>
> > > Yes, this limits optimizations, but I do not think this is
> > > terrible. (optimizations could be re-enabled with a compiler
> > > option)
> >
> > We'll see.
> >
> > > > I'm also not sure what PVNI does to
> > > >
> > > >  int a[4];
> > > >  int *p = [1];
> > > >  p += 10;
> > > >  uintptr_t pi = (uintptr_t)p;
> > > >  p = (int *)pi;
> > > >
> > > > we assume that p points to a even after p += 10 (but it of course points
> > > > outside of the object - obvious here, but not necessarily in more
> > > > obfuscated cases).
> > >
> > > This is UB because a pointer to outside of the object is formed.
> > >
> > > > Now, can we assume pi points to a?  The cast isn't value-changing.  Do 
> > > > we have
> > > > to assume (int *)pi points to anything?  So, is
> > > >
> > > >  p = (int *)(uintptr_t)p;
> > > >
> > > > something like "laundering" a pointer?  We don't preserve such simple 
> > > > round-trip
> > > > casts since they are value-preserving.  Are they provenance preserving?
> > >
> > > Yes, such a pair would be "laundering" as it allows 'p' to then
> > > point to any exposed object provenance-wise.
> > >
> > > For such casts the FE would maybe add a marker. Maybe a calls
> > > to builtin functions 'builtin_expose(a)' and 'builting_bless'.
> > > (having those builtins would be interesting on its own, btw).
> >
> > Uh, ok.
>
> For testing, I implemented builtin_expose by adding
> in 'find_func_aliases_for_builtin_call'
>
>case BUILT_IN_ESCAPE:
>{
>   make_escape_constraint (gimple_call_arg (t, 0));
>   return true;
>}
>
> which seems to work, but I wasn't sure about
> the other function.
>
> There are concurrent algorithms which revive dead pointers.
> They might also need explicit control to work around provenance
> issues.
>
> > > Having said this, some optimizations could still be allowed using
> > > the "as-if" rule and other lines of reasoning. Specifally, PVNI
> > > states that 'p' gets assigned the provenance of the object the
> > > integer values is the address of. So if the compiler can proof
> > > that the address belongs to certain objects it can reassign the
> > > points-to set to the new 'p'. Only if there is ambiguity between
> > > which objects the address belongs to, the reasoning needs to
> > > be more conservative.
> > >
> > > For example:
> > > int a[3];
> > > int b[3];
> > >
> > >  // b also exposed
>
> Correction: this should be:
>
> (intptr_t)
>
> as it is the cast to integers and not just the address-taken
> operation that marks the object exposed.
>
> > > int *p = (int*)(uintptr_t)[3];
> > >
> > > Here, p could point to the one-after address of 'a' or the
> > > first element of 'b'. (but only because 'b' was also exposed).
> > >
> > > If the compiler can prove that something like this can not
> > > happen (e.g. by considering offsets), it can still do some
> > > tracking of points-to sets.
> >
> > That's probably the very case that we'll get wrong since
> > we definitely won't be able to reliably preserve these
> > kind of laundering points...
>
> Maybe the FE could add the builtins ?
>
> > I guess they 

Re: [PATCH 2/2][MIPS][RFC] Emit .note.GNU-stack for hard-float linux targets.

[Dragan Mladjenovic]

On 29.01.2020. 22:57, Dragan Mladjenovic wrote:


On 29.01.2020. 12:06, Tobias Burnus wrote:

Hi Dragan,

I think your committed patch was incomplete – at least I see the
following bits when running --enable-maintainer-mode (see attachment,
line numbers wrong as I edited my changes out).

Can you re-check?

(The other change in gcc/configure seems to be due to Andrew Burgess's
e7c26e04b2dd6266d62d5a5825ff7eb44d1cf14e )

Tobias

PS: The following was committed as
54b3d52c3cca836c7c4c08cc9c02eda6c096372a

On 1/23/20 11:58 AM, Dragan Mladjenovic wrote: […]

2019-08-05  Dragan Mladjenovic  

  * config.in: Regenerated.
  * config/mips/linux.h (NEED_INDICATE_EXEC_STACK): Define
to 1
  for TARGET_LIBC_GNUSTACK.
  * configure: Regenerated.
  * configure.ac: Define TARGET_LIBC_GNUSTACK if glibc
version is
  found 2.31 or greater.


Thank you for letting me know.

Your change looks fine. How should I handle this? Do I just commit the
portion related to my change or perhaps you wish to commit everything
altogether?


Ok, I now see that that e7c26e04b2dd6266d62d5a5825ff7eb44d1cf14e 
mentions that files will be regenerated later. So I went ahead and

committed this as e0332517f900c7947f03c15fd27e7f71ace98629.

Best regards,
Dragan

[committed, obvious] Regenerate configure for 54b3d52

[Dragan Mladjenovic]

From: Dragan Mladjenovic 

Commit 54b3d52 ("Emit .note.GNU-stack for hard-float linux targets.")
was missing generated files.  Add them now.

gcc/ChangeLog:

2020-01-30  Dragan Mladjenovic  

* config.in: Regenerated.
* configure: Regenerated.
---
 gcc/ChangeLog |  5 +
 gcc/config.in |  6 ++
 gcc/configure | 21 +++--
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index c9fbdac..a36e732 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,8 @@
+2020-01-30  Dragan Mladjenovic  
+
+   * config.in: Regenerated.
+   * configure: Regenerated.
+
 2020-01-29  Tobias Burnus  
 
PR bootstrap/93409
diff --git a/gcc/config.in b/gcc/config.in
index ec5c46a..1110492 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -2185,6 +2185,12 @@
 #endif
 
 
+/* Define if your target C Library properly handles PT_GNU_STACK */
+#ifndef USED_FOR_TARGET
+#undef TARGET_LIBC_GNUSTACK
+#endif
+
+
 /* Define if your target C Library provides the AT_HWCAP value in the TCB */
 #ifndef USED_FOR_TARGET
 #undef TARGET_LIBC_PROVIDES_HWCAP_IN_TCB
diff --git a/gcc/configure b/gcc/configure
index 490fe6a..e2c8fc7 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -18957,7 +18957,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 18960 "configure"
+#line 18977 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -19063,7 +19063,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 19066 "configure"
+#line 19083 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -29800,6 +29800,23 @@ $as_echo "#define TARGET_LIBC_PROVIDES_HWCAP_IN_TCB 1" 
>>confdefs.h
 
 fi
 
+# Check if the target LIBC handles PT_GNU_STACK.
+gcc_cv_libc_gnustack=unknown
+case "$target" in
+  mips*-*-linux*)
+
+if test $glibc_version_major -gt 2 \
+  || ( test $glibc_version_major -eq 2 && test $glibc_version_minor -ge 31 ); 
then :
+  gcc_cv_libc_gnustack=yes
+fi
+;;
+esac
+if test x$gcc_cv_libc_gnustack = xyes; then
+
+$as_echo "#define TARGET_LIBC_GNUSTACK 1" >>confdefs.h
+
+fi
+
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking dl_iterate_phdr in target C 
library" >&5
 $as_echo_n "checking dl_iterate_phdr in target C library... " >&6; }
 gcc_cv_target_dl_iterate_phdr=unknown
-- 
1.9.1