Re: C++ PATCH for c++/91346 - Implement C++2a P1668R1, allow unevaluated asm in constexpr

[Jason Merrill]

Let's downgrade the errors in earlier standard modes to pedwarn. Ok with
that change.

On Tue, Aug 6, 2019, 10:28 AM Marek Polacek  wrote:

> This patch implements another C++2a feature, P1668R1: Permit unevaluated
> inline
> asm in constexpr functions.
>
> It's really straightforward so not much to say; we need to allow ASM_EXPRs
> in
> potential_constant_expression_1, but since only unevaluated asm is allowed,
> cxx_eval_constant_expression must give an error when it encounters an
> ASM_EXPR.
>
> I've improved location for "asm", so that the error points to "asm" rather
> than
> to ";".
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2019-08-06  Marek Polacek  
>
> PR c++/91346 - Implement P1668R1, allow unevaluated asm in
> constexpr.
> * constexpr.c (cxx_eval_constant_expression): Handle ASM_EXPR.
> (potential_constant_expression_1) : Allow in C++2a,
> give
> an error otherwise.
> * cp-tree.h (finish_asm_stmt): Adjust.
> * parser.c (cp_parser_asm_definition): Grab the locaion of "asm"
> and
> use it.  Adjust an error message.  Allow asm in C++2a.
> * pt.c (tsubst_expr): Pass a location down to finish_asm_stmt.
> * semantics.c (finish_asm_stmt): New location_t parameter.  Use it.
>
> * g++.dg/cpp2a/inline-asm1.C: New test.
> * g++.dg/cpp2a/inline-asm2.C: New test.
> * g++.dg/cpp1y/constexpr-neg1.C: Adjust dg-error.
>
> diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
> index 36a66337433..d45e65df400 100644
> --- gcc/cp/constexpr.c
> +++ gcc/cp/constexpr.c
> @@ -5289,6 +5289,18 @@ cxx_eval_constant_expression (const constexpr_ctx
> *ctx, tree t,
>r = void_node;
>break;
>
> +case ASM_EXPR:
> +  if (!ctx->quiet)
> +   {
> + error_at (cp_expr_loc_or_input_loc (t),
> +   "inline assembly is not a constant expression");
> + inform (cp_expr_loc_or_input_loc (t),
> + "only unevaluated inline assembly is allowed in a "
> + "% function in C++2a");
> +   }
> +  *non_constant_p = true;
> +  return t;
> +
>  default:
>if (STATEMENT_CODE_P (TREE_CODE (t)))
> {
> @@ -6469,13 +6481,22 @@ potential_constant_expression_1 (tree t, bool
> want_rval, bool strict, bool now,
>/* GCC internal stuff.  */
>  case VA_ARG_EXPR:
>  case TRANSACTION_EXPR:
> -case ASM_EXPR:
>  case AT_ENCODE_EXPR:
>  fail:
>if (flags & tf_error)
> error_at (loc, "expression %qE is not a constant expression", t);
>return false;
>
> +case ASM_EXPR:
> +  if (cxx_dialect >= cxx2a)
> +   /* In C++2a, unevaluated inline assembly is permitted in constexpr
> +  functions.  */
> +   return true;
> +  else if (flags & tf_error)
> +   error_at (loc, "inline assembly is not allowed in % "
> + "functions before C++2a");
> +  return false;
> +
>  case OBJ_TYPE_REF:
>if (cxx_dialect >= cxx2a)
> /* In C++2a virtual calls can be constexpr, don't give up yet.  */
> diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h
> index d4e67cdfd96..72ee1d61e97 100644
> --- gcc/cp/cp-tree.h
> +++ gcc/cp/cp-tree.h
> @@ -7052,8 +7052,8 @@ enum {
>  extern tree begin_compound_stmt(unsigned int);
>
>  extern void finish_compound_stmt   (tree);
> -extern tree finish_asm_stmt(int, tree, tree, tree,
> tree,
> -tree, bool);
> +extern tree finish_asm_stmt(location_t, int, tree,
> tree,
> +tree, tree, tree, bool);
>  extern tree finish_label_stmt  (tree);
>  extern void finish_label_decl  (tree);
>  extern cp_expr finish_parenthesized_expr   (cp_expr);
> diff --git gcc/cp/parser.c gcc/cp/parser.c
> index 79da7b52eb9..2d0e6a0c931 100644
> --- gcc/cp/parser.c
> +++ gcc/cp/parser.c
> @@ -19817,14 +19817,19 @@ cp_parser_asm_definition (cp_parser* parser)
>bool invalid_inputs_p = false;
>bool invalid_outputs_p = false;
>required_token missing = RT_NONE;
> +  location_t asm_loc = cp_lexer_peek_token (parser->lexer)->location;
>
>/* Look for the `asm' keyword.  */
>cp_parser_require_keyword (parser, RID_ASM, RT_ASM);
>
>if (parser->in_function_body
> -  && DECL_DECLARED_CONSTEXPR_P (current_function_decl))
> +  && DECL_DECLARED_CONSTEXPR_P (current_function_decl)
> +  /* In C++2a, unevaluated inline assembly is permitted in constexpr
> +functions.  */
> +  && (cxx_dialect < cxx2a))
>  {
> -  error ("% in % function");
> +  error_at (asm_loc, "% in % function only
> available "
> +   "with %<-std=c++2a%> or %<-std=gnu++2a%>");
>cp_function_chain->invalid_constexpr = true;
>  }
>
> @@ -20032,7 +20037,7 @@ cp_parser_asm_definition (cp_parser* parser)
>/* Create 

C++ PATCH for c++/81429 - wrong parsing of constructor with C++11 attribute

[Marek Polacek]

In this PR, we are wrongly parsing a constructor if its first parameter begins
with a C++11 attribute, e.g.:

  struct S {
S([[maybe_unused]] int a) { }
  };

If the GNU attribute format is used instead, there's no problem.  C++11-style
attribute on a later parameter is fine also.

The problem is in cp_parser_constructor_declarator_p, in particular the code
that checks whether we're dealing with something like "S (f) (int);", which
is not a constructor.  We're checking if what comes after the first '(' is
either ')', "...", of a parameter decl.  A parameter decl can start with the
"attribute" keyword, which cp_lexer_next_token_is_decl_specifier_keyword
recognizes, but a parameter decl can also start with a C++11-style attribute,
which we forgot to realize.

The first test uses -Wunused-parameter in order to check that the attribute
is in effect.

And I also noticed that we don't issue a pedwarn about maybe_unused (C++17
attribute) in C++14: PR 91382.

Bootstrapped/regtested on x86_64-linux, ok for trunk?  I think this should
also go into 9.3.

2019-08-06  Marek Polacek  

PR c++/81429 - wrong parsing of constructor with C++11 attribute.
* parser.c (cp_parser_constructor_declarator_p): Handle the scenario
when a parameter declaration begins with [[attribute]].

* g++.dg/cpp0x/gen-attrs-68.C: New test.
* g++.dg/cpp0x/gen-attrs-69.C: New test.

diff --git gcc/cp/parser.c gcc/cp/parser.c
index 79da7b52eb9..b8996c707b6 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -27855,7 +27855,9 @@ cp_parser_constructor_declarator_p (cp_parser *parser, 
cp_parser_flags flags,
  /* A parameter declaration begins with a decl-specifier,
 which is either the "attribute" keyword, a storage class
 specifier, or (usually) a type-specifier.  */
- && !cp_lexer_next_token_is_decl_specifier_keyword (parser->lexer))
+ && !cp_lexer_next_token_is_decl_specifier_keyword (parser->lexer)
+ /* A parameter declaration can also begin with [[attribute]].  */
+ && !cp_next_tokens_can_be_std_attribute_p (parser))
{
  tree type;
  tree pushed_scope = NULL_TREE;
diff --git gcc/testsuite/g++.dg/cpp0x/gen-attrs-68.C 
gcc/testsuite/g++.dg/cpp0x/gen-attrs-68.C
new file mode 100644
index 000..6bede0659db
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/gen-attrs-68.C
@@ -0,0 +1,40 @@
+// PR c++/81429 - wrong parsing of constructor with C++11 attribute.
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "-Wunused-parameter -Wno-pedantic" }
+
+void fn1([[maybe_unused]] int a) { }
+void fn2(int a [[maybe_unused]]) { }
+void fn3(__attribute__((unused)) int a) { }
+void fn4(int a __attribute__((unused))) { }
+
+struct S1 {
+  S1([[maybe_unused]] int a) { }
+};
+
+struct S2 {
+  S2([[maybe_unused]] int f, [[maybe_unused]] int a) { }
+};
+
+struct S3 {
+  S3(int a [[maybe_unused]]) { }
+};
+
+struct S4 {
+  S4(int f [[maybe_unused]], int a [[maybe_unused]]) { }
+};
+
+struct S5 {
+  S5(__attribute__((unused)) int a) { }
+};
+
+struct S6 {
+  S6(__attribute__((unused)) int f, __attribute__((unused)) int a) { }
+};
+
+struct S7 {
+  S7(int a __attribute__((unused))) { }
+};
+
+struct S8 {
+  S8(int f __attribute__((unused)), int a __attribute__((unused))) { }
+};
diff --git gcc/testsuite/g++.dg/cpp0x/gen-attrs-69.C 
gcc/testsuite/g++.dg/cpp0x/gen-attrs-69.C
new file mode 100644
index 000..43173dbbdf4
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/gen-attrs-69.C
@@ -0,0 +1,40 @@
+// PR c++/81429 - wrong parsing of constructor with C++11 attribute.
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "-Wno-pedantic" }
+
+void fn1([[maybe_unused]] int);
+void fn2(int a [[maybe_unused]]);
+void fn3(__attribute__((unused)) int);
+void fn4(int __attribute__((unused)));
+
+struct S1 {
+  S1([[maybe_unused]] int);
+};
+
+struct S2 {
+  S2([[maybe_unused]] int, [[maybe_unused]] int);
+};
+
+struct S3 {
+  S3(int a [[maybe_unused]]);
+};
+
+struct S4 {
+  S4(int a [[maybe_unused]], int b [[maybe_unused]]);
+};
+
+struct S5 {
+  S5(__attribute__((unused)) int);
+};
+
+struct S6 {
+  S6(__attribute__((unused)) int, __attribute__((unused)) int);
+};
+
+struct S7 {
+  S7(int __attribute__((unused)));
+};
+
+struct S8 {
+  S8(int __attribute__((unused)), int __attribute__((unused)));
+};

Re: [PATCH] Enable GCC support for AVX512_VP2INTERSECT.

[Hongtao Liu]

On Tue, Aug 6, 2019 at 11:02 PM Uros Bizjak  wrote:
>
> On Tue, Aug 6, 2019 at 1:16 PM Rainer Orth  
> wrote:
> >
> > Hi Hongtao,
> >
> > > On Thu, Jun 27, 2019 at 5:38 PM Rainer Orth 
> > >  wrote:
> > >>
> > >> Hi Hongtao,
> > >>
> > >> > On Thu, Jun 27, 2019 at 5:02 PM Rainer Orth
> > >> >  wrote:
> > >> >>
> > >> >> Hi Hongtao,
> > >> >>
> > >> >> >> as usual, the new effective-target keyword needs documenting in
> > >> >> >> sourcebuild.texi.
> > >> >> > Like this?
> > >> >>
> > >> >> a couple of nits: first of all, your mailer seems to replace tabs by a
> > >> >> single space.  Please fix this or attach the patch instead.
> > >> >>
> > >> >> > Index: ChangeLog
> > >> >> > ===
> > >> >> > --- ChangeLog (revision 272668)
> > >> >> > +++ ChangeLog (working copy)
> > >> >> > @@ -1,3 +1,8 @@
> > >> >> > +2019-06-27  Hongtao Liu  
> > >> >> > +
> > >> >> > + * doc/sourcebuild.texi: Document new effective target keyword
> > >> >> > + avx512vp2intersect.
> > >> >>
> > >> >> Please include the sections you're modifying, something like
> > >> >>
> > >> >> * doc/sourcebuild.texi (Effective-Target Keywords, Other
> > >> >> hardware attributes): Document avx512vp2intersect.
> > >> >>
> > >> >> And please don't include the ChangeLog in the patch, but include it in
> > >> >> the mail proper: it won't apply due to date and context changes 
> > >> >> anyway.
> > >> >> Best review https://gcc.gnu.org/contribute.html where this is 
> > >> >> documented
> > >> >> besides other points of patch submission.
> > >> >>
> > >> >> Besides, it's most likely useful to also review the GNU Coding
> > >> >> Standards, too, not only for ChangeLog formatting.
> > >> >>
> > >> >> > Index: testsuite/ChangeLog
> > >> >> > ===
> > >> >> > --- testsuite/ChangeLog (revision 272668)
> > >> >> > +++ testsuite/ChangeLog (working copy)
> > >> >> > @@ -1,3 +1,11 @@
> > >> >> > +2019-06-27  Hongtao Liu  
> > >> >> > +
> > >> >> > + * lib/target-supports.exp: Add
> > >> >> > + check_effective_target_avx512vp2intersect.
> > >> >>
> > >> >> Use
> > >> >>
> > >> >> * lib/target-supports.exp
> > >> >> (check_effective_target_avx512vp2intersect): New proc.
> > >> >>
> > >> >> > + * gcc.target/i386/avx512vp2intersect-2intersect-1b.c: Add
> > >> >> > + dg-require-effective-target avx512vp2intersect.
> > >> >>
> > >> >> Better:
> > >> >>
> > >> >> * gcc.target/i386/avx512vp2intersect-2intersect-1b.c: Require
> > >> >> avx512vp2intersect.
> > >> >>
> > >> >> but that's a matter of preference.
> > >> >>
> > >> >> > Index: testsuite/gcc.target/i386/avx512vp2intersect-2intersect-1b.c
> > >> >> > ===
> > >> >> > --- testsuite/gcc.target/i386/avx512vp2intersect-2intersect-1b.c
> > >> >> > (revision 272668)
> > >> >> > +++ testsuite/gcc.target/i386/avx512vp2intersect-2intersect-1b.c
> > >> >> > (working copy)
> > >> >> > @@ -1,5 +1,6 @@
> > >> >> >  /* { dg-do run } */
> > >> >> >  /* { dg-options "-O2 -mavx512vp2intersect" } */
> > >> >> > +/* { dg-require-effective-target "avx512vp2intersect" } */
> > >> >>
> > >> >> No need to quote avx512vp2intersect here and in the next test.
> > >> >>
> > >> >> Ok with those nits fixed.
> > >> >>
> > >> > Yes, thanks a lot.
> > >> >> Thanks.
> > >> >> Rainer
> > >> >>
> > >> >> --
> > >> >> -
> > >> >> Rainer Orth, Center for Biotechnology, Bielefeld University
> > >> >
> > >> > Ok for trunk?
> > >>
> > >> ENOPATCH
> > > Sorry, Here is the patch.
> > >>
> > >> --
> > >> -
> > >> Rainer Orth, Center for Biotechnology, Bielefeld University
> > >
> > >
> > > Changelog
> > >
> > > gcc/
> > > +2019-06-27  Hongtao Liu  
> > > +
> > > + * doc/sourcebuild.texi (Effective-Target Keywords, Other
> > > + hardware attributes): Document avx512vp2intersect.
> > > +
> > >
> > > gcc/testsuite/
> > > +2019-06-27  Hongtao Liu  
> > > +
> > > + * lib/target-supports.exp
> > > + (check_effective_target_avx512vp2intersect): New proc.
> > > + * gcc.target/i386/avx512vp2intersect-2intersect-1b.c: Add
> > > + dg-require-effective-target avx512vp2intersect.
> > > + * gcc.target/i386/avx512vp2intersect-2intersectvl-1b.c: Ditto.
> > > +
> >
> > unfortunately, the testcases are still not right.  While with gas 2.32
> > they now come up as UNSUPPORTED, I've now tried a mainline bootstrap on
> > i386-pc-solaris2.11 with gas from binutils master.  Doing so, I get
> >
> > +FAIL: gcc.target/i386/avx512vp2intersect-2intersect-1b.c execution test
> > +FAIL: gcc.target/i386/avx512vp2intersect-2intersectvl-1b.c execution test
> >
> > for both 32 and 64-bit, and there are similar results on
> > gcc-testresults for x86_64-pc-linux-gnu.
> >
> > Running one of the testc

Re: [PATCH] RISC-V: Handle g extension in multilib-generator

[Kito Cheng]

Committed as r274156

On Wed, Aug 7, 2019 at 3:37 AM Jim Wilson  wrote:
>
> On Mon, Aug 5, 2019 at 11:56 PM Kito Cheng  wrote:
> > gcc/ChangeLog
> > * gcc/config/riscv/multilib-generator: (canonical_order): Add 'g'.
> > (arch_canonicalize): Support rv32g and rv64g and fix error
> > handling.
>
> Looks good to me.
>
> Jim

Re: [PATCH] Adding _Dependent_ptr type qualifier in C part 1/3

[Martin Sebor]

On 8/6/19 12:26 PM, Akshat Garg wrote:

Many thanks for your feedback on the proposal and the patch.
As you said, the feature should be available only for -std=c2x. I will 
look into this how can it be done.
As you also said, we need some more extensive test coverage. Here is one 
example I tried to make for invalid use of _Dependent_ptr.


/* Test for _Dependent_ptr. _Dependent_ptr qualified pointer 
initialization tests. */

/* { dg-do compile } */
/* { dg-options "-pedantic-errors" } */

typedef __SIZE_TYPE__ size_t;
extern void exit (int);

#define TEST_SIMPLE_DECLARE(TYPE)           \
   do                   \
     {                 \
       TYPE _Dependent_ptr p; /* { dg-error " invalid use of 
'_Dependent_ptr'" } */       \


I don't think wrapping a dg- directive in a macro like this does
quite what you want.  The directive should verify that each of
the invalid declarations below triggers the error.  What it does
instead is verify that at least one of the invalid declarations
fails with the error.  I think it will pass even if only some
are accepted and others rejected.

I would suggest to get rid of the macros and spell out each invalid
declaration separately along with the expected error.

FWIW, the error message is also overly generic.  A better error
would explain why the use is invalid.  E.g.,

  "using '_Dependent_ptr' with 'int' is invalid; the qualifier may only 
be applied to pointer types"


I realize GCC issues the same poor error for 'restrict'.  A better
example to follow here is Clang which prints the much more meaningful:

  error: restrict requires a pointer or reference ('int' is invalid)



     }                                                             \
   while (0)

#define TEST_SIMPLE_DECLARE_VARIABLE()           \
   do                                                               \
     {                                                             \
       TEST_SIMPLE_DECLARE (_Bool);         \
       TEST_SIMPLE_DECLARE (char);           \
       TEST_SIMPLE_DECLARE (signed char);         \
       TEST_SIMPLE_DECLARE (unsigned char);       \
       TEST_SIMPLE_DECLARE (signed short);         \
       TEST_SIMPLE_DECLARE (unsigned short);         \
       TEST_SIMPLE_DECLARE (signed int);           \
       TEST_SIMPLE_DECLARE (unsigned int);         \
       TEST_SIMPLE_DECLARE (signed long);         \
       TEST_SIMPLE_DECLARE (unsigned long);       \
       TEST_SIMPLE_DECLARE (signed long long);         \
       TEST_SIMPLE_DECLARE (unsigned long long);         \
       TEST_SIMPLE_DECLARE (float);         \
       TEST_SIMPLE_DECLARE (double);           \
       TEST_SIMPLE_DECLARE (long double);         \
       TEST_SIMPLE_DECLARE (_Complex float);         \
       TEST_SIMPLE_DECLARE (_Complex double);       \
       TEST_SIMPLE_DECLARE (_Complex long double);       \
      }                 \
   while (0)

static void
test_simple_declare (void)
{
   TEST_SIMPLE_DECLARE_VARIABLE ();
}

int main (void)
{
   test_simple_declare ();
   exit (0);
}
Let me know if this is what you want to see.
For function pointers, I am not sure how this qualifier should behave.


If as an implementer you're not sure then that suggests it should
probably be an error :)

But, as you said, indeed the code for rejecting invalid _Dependent_ptr 
declarations allows the function pointers usage. I have made one example 
as shown, also let me know if this looks correct:


#include "../gcc/gcc/ginclude/stddef.h"
#include "../gcc/gcc/ginclude/stdatomic.h"


I've never seen these sorts of references to GCC's headers in
the test suite.  To include the named headers tests normally
just #include  and #include .



typedef __SIZE_TYPE__ size_t;
extern void abort (void);
extern void exit (int);
extern void *malloc (size_t);
extern int assert ();

_Atomic void (*fp)();
int count = 0;

#define rcu_assign_pointer(p,v) \
   atomic_store_explicit(&(p), (v), memory_order_release);

#define rcu_dereference(p) \
   atomic_load_explicit(&(p), memory_order_consume);

void my_function(){
   count++;
}

void thread0 ()

{

rcu_assign_pointer(fp, &my_function);
}

void thread1 ()
{
void (* _Dependent_ptr p)();
p = rcu_dereference(fp);
if (p)
  (*p)();
}

  I will try to make some new test cases for the conversions between 
different qualifiers, starting with the list provided by Paul and 
looking over the test cases made for other qualifiers and let you guys know.


Of the qualifiers (_Atomic, const, restrict, and volatile) my
question was mainly whether conversions between them and
_Dependent_ptr were meant to be allowed (and would have a meaning).
If not, it would at a minimum suggest they should be rejected.

Similarly, I would want to make sure that conversions between
_Dependent_ptr-qualified and unqualified pointers are valid in
both directions.  It's fine to convert an unqualified pointer
to a pointer to const but not the other way around:

  char *p = 0;
  const char *q = p;   // okay
  char *r = q;   

Re: [EXT] Re: [PATCH 1/3] C++20 constexpr lib part 1/3

[Steve Ellcey]

On Tue, 2019-08-06 at 16:47 -0400, Marek Polacek wrote:
> On Tue, Aug 06, 2019 at 08:30:14PM +, Steve Ellcey wrote:
> > On Tue, 2019-08-06 at 21:04 +0100, Jonathan Wakely wrote:
> > > 
> > > The RAJAPerf code appears to be built with -std=gnu++11 which
> > > means
> > > Ed's patch should make almost no difference at all. 99% of the
> > > patch
> > > has no effect unless compiling with -std=gnu++2a.
> > > 
> > > I don't see any ICE running the libstdc++ testsuite with
> > > -std=gnu++11,
> > > so I have no better suggestion than creating a bugzilla report
> > > for the
> > > C++ front-end, with the preprocessed source of WIP-COUPLE.cpp
> > 
> > I created a preprocessed file.  Unfortunately when I compile that
> > preprocessed file with the same options as the unpreprocessed file,
> > it does not ICE.  I compiled the original file with -save-temps and
> > that compile no longer gives an ICE.  I hate bugs like this.
> 
> Does -fdirectives-only make any difference?
> 
> Marek

Nope.  I also looked at the preprocessed file compiled with and without
this patch and the two preprocessed files are identical.  I am thinking
that this patch is triggering some unrelated latent bug.

Steve Ellcey
sell...@marvell.com

Re: Canonicalization of compares performed as side-effect operations

[Richard Earnshaw (lists)]

On 06/08/2019 18:30, Eric Botcazou wrote:
>> Why is it incorrect?  It's not canonical, sure.  But the cannonical form
>> does NOT describe what the instruction does.
> 
> Yes, you run into this when you try to be clever with the carry.  For the 
> Visium port I kludged around it by using:
> 
>   [(set (reg:CCC R_FLAGS)
>   (compare:CCC (not:I (match_operand:I 0 "register_operand" "r"))
>(const_int -1)))]
> 
> and recognizing the -1 in SELECT_CC_MODE.  Obviously cumbersome though.
> 

Interesting.  Does it work for the general case of a reverse subtract,
which I need to handle as wel?

Re: Canonicalization of compares performed as side-effect operations

[Richard Earnshaw (lists)]

On 06/08/2019 18:39, Uros Bizjak wrote:
> Hello!
> 
>> On Tue, Aug 06, 2019 at 05:49:17PM +0100, Richard Earnshaw (lists) wrote:
>>> On 06/08/2019 17:39, Segher Boessenkool wrote:
>> What's wrong with describing the canonical form in your MD?  You'll need
>> some reversed condition code thingy, but that's it?
>
> It doesn't describe what the instruction does.  The negation has a side
> effect of setting the flags, but the flags are swapped because the
> side-effect comparison is swapped from a normal compare.  As I
> mentioned, SELECT_CC_MODE doesn't help because it can't see the context
> and the comparison just looks 'normal'.

 Sure, and we can work on making combine do what you want, but your existing
 pattern is *incorrect*.  It needs fixing, and probably before we do other
 things.
>>>
>>> Why is it incorrect?  It's not canonical, sure.  But the cannonical form
>>> does NOT describe what the instruction does.
>>
>> More precisely: not having the canonical form of this in your MD is what
>> is incorrect.
> 
> There is TARGET_CANONICALIZE_COMPARISON target hook that can solve the
> issue here. Please see x86, where we canonicalize
> *cmp__i387 via
> ix86_canonicalize_comparison. As said in the comment:
> 
>   /* The order of operands in x87 ficom compare is forced by combine in
>  simplify_comparison () function. Float operator is treated as RTX_OBJ
>  with a precedence over other operators and is always put in the first
>  place. Swap condition and operands to match ficom instruction.  */
> 
> The issue looks similar to me.
> 

That hook can't help as it can't see anything other than the operands of
the compare, and from the operands it looks as though the operands
should be swapped.  To correctly canonicalize this you need the whole
parallel, or some other hint that the operation is a side-effect of
another operation.

R.
> Uros.
> 

Re: [EXT] Re: [PATCH 1/3] C++20 constexpr lib part 1/3

[Marek Polacek]

On Tue, Aug 06, 2019 at 08:30:14PM +, Steve Ellcey wrote:
> On Tue, 2019-08-06 at 21:04 +0100, Jonathan Wakely wrote:
> > 
> > The RAJAPerf code appears to be built with -std=gnu++11 which means
> > Ed's patch should make almost no difference at all. 99% of the patch
> > has no effect unless compiling with -std=gnu++2a.
> > 
> > I don't see any ICE running the libstdc++ testsuite with -std=gnu++11,
> > so I have no better suggestion than creating a bugzilla report for the
> > C++ front-end, with the preprocessed source of WIP-COUPLE.cpp
> 
> I created a preprocessed file.  Unfortunately when I compile that
> preprocessed file with the same options as the unpreprocessed file,
> it does not ICE.  I compiled the original file with -save-temps and
> that compile no longer gives an ICE.  I hate bugs like this.

Does -fdirectives-only make any difference?

Marek

Re: [PATCH 2/9] ifcvt: Use enum instead of transform_name string.

[Richard Sandiford]

Robin Dapp  writes:
> This patch introduces an enum for ifcvt's various noce transformations.
> As the transformation might be queried by the backend, I find it nicer
> to allow checking for a proper type instead of a string comparison.

TBH I think the names of internal ifcvt routines are too level to expose as
an "official" part of the interface.  (OK, like you say, the information's
already there and rogue backends could already check it if they wanted.)

If we were going to have an enum, I think it should be a higher-level
description of what the converted code is going to do, rather than the
name of the routine that's going to do it.

It sounded from the covering note that you might not need this
any more though.

Thanks,
Richard

>
> ---
>  gcc/ifcvt.c | 46 ++--
>  gcc/ifcvt.h | 67 ++---
>  2 files changed, 88 insertions(+), 25 deletions(-)
>
> diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
> index b80dbc43d83..e95ff9ee9b0 100644
> --- a/gcc/ifcvt.c
> +++ b/gcc/ifcvt.c
> @@ -1105,7 +1105,7 @@ noce_try_move (struct noce_if_info *if_info)
> emit_insn_before_setloc (seq, if_info->jump,
>  INSN_LOCATION (if_info->insn_a));
>   }
> -  if_info->transform_name = "noce_try_move";
> +  if_info->transform = ifcvt_transform_noce_try_move;
>return TRUE;
>  }
>return FALSE;
> @@ -1139,7 +1139,7 @@ noce_try_ifelse_collapse (struct noce_if_info * if_info)
>emit_insn_before_setloc (seq, if_info->jump,
> INSN_LOCATION (if_info->insn_a));
>  
> -  if_info->transform_name = "noce_try_ifelse_collapse";
> +  if_info->transform = ifcvt_transform_noce_try_ifelse_collapse;
>return TRUE;
>  }
>  
> @@ -1186,7 +1186,7 @@ noce_try_store_flag (struct noce_if_info *if_info)
>  
>emit_insn_before_setloc (seq, if_info->jump,
>  INSN_LOCATION (if_info->insn_a));
> -  if_info->transform_name = "noce_try_store_flag";
> +  if_info->transform = ifcvt_transform_noce_try_store_flag;
>return TRUE;
>  }
>else
> @@ -1265,7 +1265,7 @@ noce_try_inverse_constants (struct noce_if_info 
> *if_info)
>  
>emit_insn_before_setloc (seq, if_info->jump,
>  INSN_LOCATION (if_info->insn_a));
> -  if_info->transform_name = "noce_try_inverse_constants";
> +  if_info->transform = ifcvt_transform_noce_try_inverse_constants;
>return true;
>  }
>  
> @@ -1485,7 +1485,7 @@ noce_try_store_flag_constants (struct noce_if_info 
> *if_info)
>  
>emit_insn_before_setloc (seq, if_info->jump,
>  INSN_LOCATION (if_info->insn_a));
> -  if_info->transform_name = "noce_try_store_flag_constants";
> +  if_info->transform = ifcvt_transform_noce_try_store_flag_constants;
>  
>return TRUE;
>  }
> @@ -1546,7 +1546,7 @@ noce_try_addcc (struct noce_if_info *if_info)
>  
> emit_insn_before_setloc (seq, if_info->jump,
>  INSN_LOCATION (if_info->insn_a));
> -   if_info->transform_name = "noce_try_addcc";
> +   if_info->transform = ifcvt_transform_noce_try_addcc;
>  
> return TRUE;
>   }
> @@ -1588,7 +1588,7 @@ noce_try_addcc (struct noce_if_info *if_info)
>  
> emit_insn_before_setloc (seq, if_info->jump,
>  INSN_LOCATION (if_info->insn_a));
> -   if_info->transform_name = "noce_try_addcc";
> +   if_info->transform = ifcvt_transform_noce_try_addcc;
> return TRUE;
>   }
> end_sequence ();
> @@ -1639,7 +1639,7 @@ noce_try_store_flag_mask (struct noce_if_info *if_info)
>  
> emit_insn_before_setloc (seq, if_info->jump,
>  INSN_LOCATION (if_info->insn_a));
> -   if_info->transform_name = "noce_try_store_flag_mask";
> +   if_info->transform = ifcvt_transform_noce_try_store_flag_mask;
>  
> return TRUE;
>   }
> @@ -1791,7 +1791,7 @@ noce_try_cmove (struct noce_if_info *if_info)
>  
> emit_insn_before_setloc (seq, if_info->jump,
>  INSN_LOCATION (if_info->insn_a));
> -   if_info->transform_name = "noce_try_cmove";
> +   if_info->transform = ifcvt_transform_noce_try_cmove;
>  
> return TRUE;
>   }
> @@ -1844,7 +1844,7 @@ noce_try_cmove (struct noce_if_info *if_info)
>  
> emit_insn_before_setloc (seq, if_info->jump,
>  INSN_LOCATION (if_info->insn_a));
> -   if_info->transform_name = "noce_try_cmove";
> +   if_info->transform = ifcvt_transform_noce_try_cmove;
> return TRUE;
>   }
> else
> @@ -2286,7 +2286,7 @@ noce_try_cmove_arith (struct noce_if_info *if_info)
>  
>emit_insn_before_setloc (ifcvt_seq, if_info->jump,
>  INSN_LOCATION (if_info->insn_a

Re: [PATCH 8/9] ifcvt: Handle swap-style idioms differently.

[Richard Sandiford]

Robin Dapp  writes:
> A swap-style idiom like
>  tmp = a
>a = b
>b = tmp
> would be transformed like
>  tmp_tmp = cond ? a : tmp
>  tmp_a   = cond ? b : a
>  tmp_b   = cond ? tmp_tmp : b
>  [...]
> including rewiring the first source operand to previous writes (e.g. tmp ->
> tmp_tmp).
>
> The code would recognize this, though, and change the last line to
>  tmp_b   = cond ? a : b.
>
> Without additional temporaries we can now emit the following sequence:
>  tmp = a   // (no condition here)
>  a = cond ? b : a
>  b = cond ? tmp : b
> avoiding any rewiring which would break things now.
>
> check_need_cmovs () finds swap-style idioms and marks the first of the
> three instructions as not needing a cmove.

Looks like a nice optimisation, but could we just test whether the
destination of a set isn't live on exit from the then block?  I think
we could do that on the fly during the main noce_convert_multiple_sets
loop.

Thanks,
Richard

> ---
>  gcc/ifcvt.c | 97 ++---
>  1 file changed, 78 insertions(+), 19 deletions(-)
>
> diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
> index 955f9541f60..09bf443656c 100644
> --- a/gcc/ifcvt.c
> +++ b/gcc/ifcvt.c
> @@ -103,6 +103,7 @@ static rtx_insn *block_has_only_trap (basic_block);
>  static void check_need_temps (basic_block bb,
>hash_map *need_temp,
>rtx cond);
> +static void check_need_cmovs (basic_block bb, hash_map 
> *need_cmov);
>  
>  
>  /* Count the number of non-jump active insns in BB.  */
> @@ -3207,6 +3208,7 @@ noce_convert_multiple_sets (struct noce_if_info 
> *if_info)
>int count = 0;
>  
>hash_map need_temps;
> +  hash_map need_no_cmovs;
>  
>/* If we newly set a CC before a cmov, we might need a temporary
>   even though the compare will be removed by a later pass.  Costing
> @@ -3214,6 +3216,9 @@ noce_convert_multiple_sets (struct noce_if_info 
> *if_info)
>  
>check_need_temps (then_bb, &need_temps, cond);
>  
> +  /* Identify swap-style idioms.  */
> +  check_need_cmovs (then_bb, &need_no_cmovs);
> +
>hash_map temps_created;
>  
>FOR_BB_INSNS (then_bb, insn)
> @@ -3229,10 +3234,8 @@ noce_convert_multiple_sets (struct noce_if_info 
> *if_info)
>rtx new_val = SET_SRC (set);
>rtx old_val = target;
>  
> -  rtx dest = SET_DEST (set);
> -
>rtx temp;
> -  if (need_temps.get (dest))
> +  if (need_temps.get (target))
> {
>   temp = gen_reg_rtx (GET_MODE (target));
>   temps_created.put (target, true);
> @@ -3241,18 +3244,11 @@ noce_convert_multiple_sets (struct noce_if_info 
> *if_info)
> temp = target;
>  
>/* If we were supposed to read from an earlier write in this block,
> -  we've changed the register allocation.  Rewire the read.  While
> -  we are looking, also try to catch a swap idiom.  */
> +  we've changed the register allocation.  Rewire the read.   */
>for (int i = count - 1; i >= 0; --i)
>   if (reg_overlap_mentioned_p (new_val, targets[i]))
> {
> - /* Catch a "swap" style idiom.  */
> - if (find_reg_note (insn, REG_DEAD, new_val) != NULL_RTX)
> -   /* The write to targets[i] is only live until the read
> -  here.  As the condition codes match, we can propagate
> -  the set to here.  */
> -   new_val = SET_SRC (single_set (unmodified_insns[i]));
> - else
> + if (find_reg_note (insn, REG_DEAD, new_val) == NULL_RTX)
> new_val = temporaries[i];
>   break;
> }
> @@ -3324,8 +3320,11 @@ noce_convert_multiple_sets (struct noce_if_info 
> *if_info)
>  
>{
>   start_sequence ();
> - temp_dest1 = noce_emit_cmove (if_info, temp, cond_code,
> - x, y, new_val, old_val, NULL_RTX);
> + if (!need_no_cmovs.get (insn))
> +   temp_dest1 = noce_emit_cmove (if_info, temp, cond_code,
> +   x, y, new_val, old_val, NULL_RTX);
> + else
> +   noce_emit_move_insn (target, new_val);
>  
>   /* If we failed to expand the conditional move, drop out and don't
>  try to continue.  */
> @@ -3346,13 +3345,16 @@ noce_convert_multiple_sets (struct noce_if_info 
> *if_info)
>   end_sequence ();
>}
>  
> - /* Now try emitting one passing a non-canonicalized cc comparison.
> -This allows the backend to emit a cmov directly without additional
> + /* Now try emitting a cmov passing a non-canonicalized cc comparison.
> +This allows backends to emit a cmov directly without additional
>  compare.  */
>{
>   start_sequence ();
> - temp_dest2 = noce_emit_cmove (if_info, target, cond_code,
> - x, y, new_val, old_val, cc_cmp);
> + if (!need_no_cmovs.get (insn))
> +   temp_dest2 = noce_emit_cmove (if_info, target, cond_code,
> +   x, y, new_val, old_val, cc_cmp);
> + else
> +   noce_emit_move_ins

Re: [EXT] Re: [PATCH 1/3] C++20 constexpr lib part 1/3

[Steve Ellcey]

On Tue, 2019-08-06 at 21:04 +0100, Jonathan Wakely wrote:
> 
> The RAJAPerf code appears to be built with -std=gnu++11 which means
> Ed's patch should make almost no difference at all. 99% of the patch
> has no effect unless compiling with -std=gnu++2a.
> 
> I don't see any ICE running the libstdc++ testsuite with -std=gnu++11,
> so I have no better suggestion than creating a bugzilla report for the
> C++ front-end, with the preprocessed source of WIP-COUPLE.cpp

I created a preprocessed file.  Unfortunately when I compile that
preprocessed file with the same options as the unpreprocessed file,
it does not ICE.  I compiled the original file with -save-temps and
that compile no longer gives an ICE.  I hate bugs like this.

Steve Ellcey
sell...@marvell.com

Re: [PATCH 5/9] ifcvt: Allow constants operands in noce_convert_multiple_sets.

[Richard Sandiford]

Robin Dapp  writes:
> This patch checks allows immediate then/else operands for cmovs.
> We rely on,emit_conditional_move returning NULL if something unsupported
> was generated.

It seems like this is making noce_convert_multiple_sets overlap
a lot with cond_move_process_if_block (although that uses CONSTANT_P
instead of CONST_INT_P).  How do they fit together after this patch,
i.e. which cases is each one meant to handle that the other doesn't?

Thanks,
Richard

> Also, minor refactoring is performed.
>
> --
>
> gcc/ChangeLog:
>
> 2018-11-14  Robin Dapp  
>
>   * ifcvt.c (have_const_cmov): New function.
>   (noce_convert_multiple_sets): Allow constants if supported.
>   (bb_ok_for_noce_convert_multiple_sets): Likewise.
>   (check_cond_move_block): Refactor.
> ---
>  gcc/ifcvt.c | 21 +++--
>  1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
> index 55205cac153..99716e5f63c 100644
> --- a/gcc/ifcvt.c
> +++ b/gcc/ifcvt.c
> @@ -3214,7 +3214,9 @@ noce_convert_multiple_sets (struct noce_if_info 
> *if_info)
>we'll end up trying to emit r4:HI = cond ? (r1:SI) : (r3:HI).
>Wrap the two cmove operands into subregs if appropriate to prevent
>that.  */
> -  if (GET_MODE (new_val) != GET_MODE (temp))
> +
> +  if (!CONST_INT_P (new_val)
> + && GET_MODE (new_val) != GET_MODE (temp))
>   {
> machine_mode src_mode = GET_MODE (new_val);
> machine_mode dst_mode = GET_MODE (temp);
> @@ -3225,7 +3227,8 @@ noce_convert_multiple_sets (struct noce_if_info 
> *if_info)
>   }
> new_val = lowpart_subreg (dst_mode, new_val, src_mode);
>   }
> -  if (GET_MODE (old_val) != GET_MODE (temp))
> +  if (!CONST_INT_P (old_val)
> + && GET_MODE (old_val) != GET_MODE (temp))
>   {
> machine_mode src_mode = GET_MODE (old_val);
> machine_mode dst_mode = GET_MODE (temp);
> @@ -3362,9 +3365,9 @@ bb_ok_for_noce_convert_multiple_sets (basic_block 
> test_bb, unsigned *cost)
>if (!REG_P (dest))
>   return false;
>  
> -  if (!(REG_P (src)
> -|| (GET_CODE (src) == SUBREG && REG_P (SUBREG_REG (src))
> -&& subreg_lowpart_p (src
> +  if (!((REG_P (src) || (CONST_INT_P (src)))
> + || (GET_CODE (src) == SUBREG && REG_P (SUBREG_REG (src))
> +   && subreg_lowpart_p (src
>   return false;
>  
>/* Destination must be appropriate for a conditional write.  */
> @@ -3724,7 +3727,7 @@ check_cond_move_block (basic_block bb,
>  {
>rtx set, dest, src;
>  
> -  if (!NONDEBUG_INSN_P (insn) || JUMP_P (insn))
> +  if (!active_insn_p (insn))
>   continue;
>set = single_set (insn);
>if (!set)
> @@ -3740,10 +3743,8 @@ check_cond_move_block (basic_block bb,
>if (!CONSTANT_P (src) && !register_operand (src, VOIDmode))
>   return FALSE;
>  
> -  if (side_effects_p (src) || side_effects_p (dest))
> - return FALSE;
> -
> -  if (may_trap_p (src) || may_trap_p (dest))
> +  /* Check for side effects and trapping.  */
> +  if (!noce_operand_ok (src) || !noce_operand_ok (dest))
>   return FALSE;
>  
>/* Don't try to handle this if the source register was

Re: [PATCH 4/9] ifcvt: Estimate original costs before convert_multiple.

[Richard Sandiford]

Robin Dapp  writes:
> This patch extends bb_ok_for_noce_convert_multiple_sets by a temporary
> cost estimation that can be used by noce_convert_multiple_sets.

I agree it looks like an omission that we didn't do this.  The patch
looks OK to me (maybe independently of the rest?) bar minor things:

> diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
> index 253b8a96c1a..55205cac153 100644
> --- a/gcc/ifcvt.c
> +++ b/gcc/ifcvt.c
> @@ -,11 +,13 @@ noce_convert_multiple_sets (struct noce_if_info 
> *if_info)
> fewer than PARAM_MAX_RTL_IF_CONVERSION_INSNS sets.  */
>  
>  static bool
> -bb_ok_for_noce_convert_multiple_sets (basic_block test_bb)
> +bb_ok_for_noce_convert_multiple_sets (basic_block test_bb, unsigned *cost)

The function comment should document COST.

>  {
>rtx_insn *insn;
>unsigned count = 0;
>unsigned param = PARAM_VALUE (PARAM_MAX_RTL_IF_CONVERSION_INSNS);
> +  bool speed_p = optimize_bb_for_speed_p (test_bb);
> +  unsigned potential_cost = 0;
>  
>FOR_BB_INSNS (test_bb, insn)
>  {
> @@ -3373,9 +3375,14 @@ bb_ok_for_noce_convert_multiple_sets (basic_block 
> test_bb)
>if (!can_conditionally_move_p (GET_MODE (dest)))
>   return false;
>  
> +  rtx sset = single_set (insn);

This is already available as "set", unless I'm missing something.

> +  potential_cost += pattern_cost (sset, speed_p);
> +
>count++;
>  }
>  
> +  *cost += potential_cost;
> +
>/* If we would only put out one conditional move, the other strategies
>   this pass tries are better optimized and will be more appropriate.
>   Some targets want to strictly limit the number of conditional moves
> @@ -3414,11 +3421,15 @@ noce_process_if_block (struct noce_if_info *if_info)
>   ??? For future expansion, further expand the "multiple X" rules.  */
>  
>/* First look for multiple SETS.  */
> +  unsigned int mcost = if_info->original_cost;
> +  unsigned tmp_cost = if_info->original_cost;

Very minor, but it'd be good to be consistent about the choice
between unsigned and unsigned int.　Maybe "old_cost" would be a
better name than "tmp_cost".

>if (!else_bb
>&& HAVE_conditional_move
>&& !HAVE_cc0
> -  && bb_ok_for_noce_convert_multiple_sets (then_bb))
> +  && bb_ok_for_noce_convert_multiple_sets (then_bb, &mcost))
>  {
> +  /* Temporarily set the original costs to what we estimated.  */
> +  if_info->original_cost = mcost;
>if (noce_convert_multiple_sets (if_info))
>   {
> if (dump_file && if_info->transform)
> @@ -3427,6 +3438,8 @@ noce_process_if_block (struct noce_if_info *if_info)
> return TRUE;
>   }
>  }
> +  /* Restore the original costs.  */
> +  if_info->original_cost = tmp_cost;
>  
>bool speed_p = optimize_bb_for_speed_p (test_bb);
>unsigned int then_cost = 0, else_cost = 0;

I guess the save and restore only really need to be done inside the
outer "if".  Not that the performance difference is going to be
noticeable, but maybe it would be a bit clearer to read.

Thanks,
Richard

Re: [PATCH 1/3] C++20 constexpr lib part 1/3

[Jonathan Wakely]

On 06/08/19 15:30 +, Steve Ellcey wrote:

Ed,

I have run into an ICE that I tracked down to this patch:

commit 02fefffe6b78c4c39169206aa40fb53f367ecba8
Author: emsr 
Date:   Thu Aug 1 15:25:42 2019 +

   2019-08-01  Edward Smith-Rowland  <3dw...@verizon.net>

   Implement C++20 p0202 - Add Constexpr Modifiers to Functions
   in  and  Headers.
   Implement C++20 p1023 - constexpr comparison operators for 
std::array.


Before I try to create a smaller test example (the current failure happens
when I build https://github.com/llnl/RAJAPerf.git) I was wondering if you
have already seen this ICE:

/extra/sellcey/raja-build-error/RAJAPerf/src/apps/WIP-COUPLE.cpp: In member 
function 'virtual void rajaperf::apps::COUPLE::runKernel(rajaperf::VariantID)':
/extra/sellcey/raja-build-error/RAJAPerf/src/apps/WIP-COUPLE.cpp:217:1: 
internal compiler error: Segmentation fault


The RAJAPerf code appears to be built with -std=gnu++11 which means
Ed's patch should make almost no difference at all. 99% of the patch
has no effect unless compiling with -std=gnu++2a.

I don't see any ICE running the libstdc++ testsuite with -std=gnu++11,
so I have no better suggestion than creating a bugzilla report for the
C++ front-end, with the preprocessed source of WIP-COUPLE.cpp


 217 | } // end namespace rajaperf
 | ^
0xe81ddf crash_signal
../../gcc/gcc/toplev.c:326
0x968d14 lookup_page_table_entry
../../gcc/gcc/ggc-page.c:632
0x968d14 ggc_set_mark(void const*)
../../gcc/gcc/ggc-page.c:1531
0xbfeadf gt_ggc_mx_symtab_node(void*)
/extra/sellcey/gcc-tot/obj-gcc/gcc/gtype-desc.c:1302
0xd9d2a7 gt_ggc_ma_order
./gt-passes.h:31
0xd9d2a7 gt_ggc_ma_order
./gt-passes.h:26
0xb6f49f ggc_mark_root_tab
../../gcc/gcc/ggc-common.c:77
0xb6f6c3 ggc_mark_roots()
../../gcc/gcc/ggc-common.c:94
0x9696af ggc_collect()
../../gcc/gcc/ggc-page.c:2201
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See  for instructions.

Re: [PATCH 3/9] ifcvt: Only created temporaries as needed.

[Richard Sandiford]

Robin Dapp  writes:
> noce_convert_multiple_sets creates temporaries for the destination of
> every emitted cmov and expects subsequent passes to get rid of them.  This
> does not happen every time and even if the temporaries are removed, code
> generation can be affected adversely.  In this patch, temporaries are
> only created if the destination of a set is used in an emitted condition
> check.

Still digesting the series, but a couple of implementation questions:

>
> ---
>  gcc/ifcvt.c | 54 ++---
>  1 file changed, 51 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
> index e95ff9ee9b0..253b8a96c1a 100644
> --- a/gcc/ifcvt.c
> +++ b/gcc/ifcvt.c
> @@ -99,6 +99,10 @@ static int dead_or_predicable (basic_block, basic_block, 
> basic_block,
>  edge, int);
>  static void noce_emit_move_insn (rtx, rtx);
>  static rtx_insn *block_has_only_trap (basic_block);
> +static void check_need_temps (basic_block bb,
> +  hash_map *need_temp,
> +  rtx cond);
> +
>  
>  /* Count the number of non-jump active insns in BB.  */
>  
> @@ -3145,6 +3149,12 @@ noce_convert_multiple_sets (struct noce_if_info 
> *if_info)
>auto_vec unmodified_insns;
>int count = 0;
>  
> +  hash_map need_temps;
> +
> +  check_need_temps (then_bb, &need_temps, cond);

Is the separate need_temps scan required for correctness?  It looked
like we could test:

  if (reg_overlap_mentioned_p (dest, cond))
...

on-the-fly during the main noce_convert_multiple_sets loop.

> +
> +  hash_map temps_created;
> +
>FOR_BB_INSNS (then_bb, insn)
>  {
>/* Skip over non-insns.  */
> @@ -3155,10 +3165,20 @@ noce_convert_multiple_sets (struct noce_if_info 
> *if_info)
>gcc_checking_assert (set);
>  
>rtx target = SET_DEST (set);
> -  rtx temp = gen_reg_rtx (GET_MODE (target));
>rtx new_val = SET_SRC (set);
>rtx old_val = target;
>  
> +  rtx dest = SET_DEST (set);
> +
> +  rtx temp;
> +  if (need_temps.get (dest))
> +   {
> + temp = gen_reg_rtx (GET_MODE (target));
> + temps_created.put (target, true);
> +   }
> +  else
> +   temp = target;
> +
>/* If we were supposed to read from an earlier write in this block,
>we've changed the register allocation.  Rewire the read.  While
>we are looking, also try to catch a swap idiom.  */
> @@ -3242,8 +3262,8 @@ noce_convert_multiple_sets (struct noce_if_info 
> *if_info)
>  
>/* Now fixup the assignments.  */
>for (int i = 0; i < count; i++)
> -noce_emit_move_insn (targets[i], temporaries[i]);
> -
> +if (temps_created.get(targets[i]) && targets[i] != temporaries[i])
> +  noce_emit_move_insn (targets[i], temporaries[i]);

Would it work to set temporaries[i] to targets[i] whenever a temporary
isn't needed, and avoid temps_created?

Thanks,
Richard

>  
>/* Actually emit the sequence if it isn't too expensive.  */
>rtx_insn *seq = get_insns ();
> @@ -3749,6 +3769,34 @@ check_cond_move_block (basic_block bb,
>return TRUE;
>  }
>  
> +/* Check for which sets we need to emit temporaries to hold the destination 
> of
> +   a conditional move.  */
> +static void
> +check_need_temps (basic_block bb, hash_map *need_temp, rtx cond)
> +{
> +  rtx_insn *insn;
> +
> +  FOR_BB_INSNS (bb, insn)
> +{
> +  rtx set, dest;
> +
> +  if (!active_insn_p (insn))
> + continue;
> +
> +  set = single_set (insn);
> +  if (set == NULL_RTX)
> + continue;
> +
> +  dest = SET_DEST (set);
> +
> +  /* noce_emit_cmove will emit the condition check every time it is 
> called
> + so we need a temp register if the destination is modified.  */
> +  if (reg_overlap_mentioned_p (dest, cond))
> + need_temp->put (dest, true);
> +}
> +}
> +
> +
>  /* Given a basic block BB suitable for conditional move conversion,
> a condition COND, and pointer maps THEN_VALS and ELSE_VALS containing
> the register values depending on COND, emit the insns in the block as

Re: C++ PATCH for c++/91264 - detect modifying const objects in constexpr

[Marek Polacek]

On Tue, Aug 06, 2019 at 09:38:35PM +0200, Paolo Carlini wrote:
> Hi,
> 
> On 31/07/19 21:26, Marek Polacek wrote:
> > +static void
> > +modifying_const_object_error (tree expr, tree obj)
> > +{
> > +  location_t loc = cp_expr_loc_or_loc (expr, input_location);
> 
> Nit: now we have cp_expr_loc_input_loc

Ah yeah, and I was meaning to update it!  Will fix.

Marek

Re: C++ PATCH for c++/91264 - detect modifying const objects in constexpr

[Paolo Carlini]

Hi,

On 31/07/19 21:26, Marek Polacek wrote:

+static void
+modifying_const_object_error (tree expr, tree obj)
+{
+  location_t loc = cp_expr_loc_or_loc (expr, input_location);


Nit: now we have cp_expr_loc_input_loc

Paolo.

Re: [PATCH] RISC-V: Handle g extension in multilib-generator

[Jim Wilson]

On Mon, Aug 5, 2019 at 11:56 PM Kito Cheng  wrote:
> gcc/ChangeLog
> * gcc/config/riscv/multilib-generator: (canonical_order): Add 'g'.
> (arch_canonicalize): Support rv32g and rv64g and fix error
> handling.

Looks good to me.

Jim

Re: [PATCH] PR fortran/42546 -- ALLOCATED has 2 mutually exclusive keywords

[Steve Kargl]

It looks like a backwards compatibility issue.
F95, 13.14.9 ALLOCATED (ARRAY).
F2003, 13.7.9 ALLOCATED (ARRAY) or ALLOCATED (SCALAR)

Thanks for the quick peek.

-- 
steve

On Tue, Aug 06, 2019 at 08:20:28PM +0100, Paul Richard Thomas wrote:
> 
> Who thought of that one in the standard? Uuugh!
> 
> The solution looks good to commit - again as far back as you feel
> inclined to do.
> 
> Regards
> 
> Paul
> 
> On Tue, 6 Aug 2019 at 19:27, Steve Kargl
>  wrote:
> >
> > Ping.
> >
> > On Thu, Aug 01, 2019 at 02:11:39PM -0700, Steve Kargl wrote:
> > > The attached patch fixed the issues raised in the
> > > PR fortran/42546.  Namely, ALLOCATED has two possible
> > > keywords: ALLOCATE(ARRAY=...) or ALLOCATED(SCALAR=...)
> > >
> > > In Tobias' original patch (attached to the PR), he
> > > tried to make both ARRAY and SCALAR options, then
> > > in gfc_check_allocated() appropriate checking was
> > > added.  I started down that road, but intrinsic.c(
> > > sort_actual) got in the way.  Fortunately, the
> > > checking for ARRAY or SCALAR can be special-cased
> > > in sort_actual.  See the patch.
> > >
> > > Regression tested on x86_64-*-freebsd.  OK to commit?
> > >
> > > 2019-08-01  Steven G. Kargl  
> > >
> > >   PR fortran/42546
> > >   * check.c(gfc_check_allocated): Add comment pointing to ...
> > >   * intrinsic.c(sort_actual): ... the checking done here.
> > >
> > > 2019-08-01  Steven G. Kargl  
> > >
> > >   PR fortran/42546
> > >   * gfortran.dg/allocated_1.f90: New test.
> > >   * gfortran.dg/allocated_2.f90: Ditto.
> > >
> > > --
> > > Steve
> >
> > > Index: gcc/fortran/check.c
> > > ===
> > > --- gcc/fortran/check.c   (revision 273950)
> > > +++ gcc/fortran/check.c   (working copy)
> > > @@ -1168,6 +1168,10 @@ gfc_check_all_any (gfc_expr *mask, gfc_expr *dim)
> > >  }
> > >
> > >
> > > +/* Limited checking for ALLOCATED intrinsic.  Additional checking
> > > +   is performed in intrinsic.c(sort_actual), because ALLOCATED
> > > +   has two mutually exclusive non-optional arguments.  */
> > > +
> > >  bool
> > >  gfc_check_allocated (gfc_expr *array)
> > >  {
> > > Index: gcc/fortran/intrinsic.c
> > > ===
> > > --- gcc/fortran/intrinsic.c   (revision 273950)
> > > +++ gcc/fortran/intrinsic.c   (working copy)
> > > @@ -4180,6 +4180,40 @@ sort_actual (const char *name, gfc_actual_arglist 
> > > **ap
> > >if (f == NULL && a == NULL)/* No arguments */
> > >  return true;
> > >
> > > +  /* ALLOCATED has two mutually exclusive keywords, but only one
> > > + can be present at time and neither is optional. */
> > > +  if (strcmp (name, "allocated") == 0 && a->name)
> > > +{
> > > +  if (strcmp (a->name, "scalar") == 0)
> > > + {
> > > +  if (a->next)
> > > + goto whoops;
> > > +   if (a->expr->rank != 0)
> > > + {
> > > +   gfc_error ("Scalar entity required at %L", &a->expr->where);
> > > +   return false;
> > > + }
> > > +  return true;
> > > + }
> > > +  else if (strcmp (a->name, "array") == 0)
> > > + {
> > > +  if (a->next)
> > > + goto whoops;
> > > +   if (a->expr->rank == 0)
> > > + {
> > > +   gfc_error ("Array entity required at %L", &a->expr->where);
> > > +   return false;
> > > + }
> > > +  return true;
> > > + }
> > > +  else
> > > + {
> > > +   gfc_error ("Invalid keyword %qs in %qs intrinsic function at %L",
> > > +  a->name, name, @a->expr->where);
> > > +   return false;
> > > + }
> > > +}
> > > +
> > >for (;;)
> > >  {/* Put the nonkeyword arguments in a 1:1 
> > > correspondence */
> > >if (f == NULL)
> > > @@ -4199,6 +4233,7 @@ sort_actual (const char *name, gfc_actual_arglist 
> > > **ap
> > >if (a == NULL)
> > >  goto do_sort;
> > >
> > > +whoops:
> > >gfc_error ("Too many arguments in call to %qs at %L", name, where);
> > >return false;
> > >
> > > Index: gcc/testsuite/gfortran.dg/allocated_1.f90
> > > ===
> > > --- gcc/testsuite/gfortran.dg/allocated_1.f90 (nonexistent)
> > > +++ gcc/testsuite/gfortran.dg/allocated_1.f90 (working copy)
> > > @@ -0,0 +1,24 @@
> > > +! { dg-do run }
> > > +program foo
> > > +
> > > +   implicit none
> > > +
> > > +   integer, allocatable :: x
> > > +   integer, allocatable :: a(:)
> > > +
> > > +   logical a1, a2
> > > +
> > > +   a1 = allocated(scalar=x)
> > > +   if (a1 .neqv. .false.) stop 1
> > > +   a2 = allocated(array=a)
> > > +   if (a2 .neqv. .false.) stop 2
> > > +
> > > +   allocate(x)
> > > +   allocate(a(2))
> > > +
> > > +   a1 = allocated(scalar=x)
> > > +   if (a1 .neqv. .true.) stop 3
> > > +   a2 = allocated(array=a)
> > > +   if (a2 .neqv. .true.) stop 4
> > > +
> > > +e

Re: [PATCH] PR fortran/42546 -- ALLOCATED has 2 mutually exclusive keywords

[Paul Richard Thomas]

Hi Steve,

Who thought of that one in the standard? Uuugh!

The solution looks good to commit - again as far back as you feel
inclined to do.

Regards

Paul

On Tue, 6 Aug 2019 at 19:27, Steve Kargl
 wrote:
>
> Ping.
>
> On Thu, Aug 01, 2019 at 02:11:39PM -0700, Steve Kargl wrote:
> > The attached patch fixed the issues raised in the
> > PR fortran/42546.  Namely, ALLOCATED has two possible
> > keywords: ALLOCATE(ARRAY=...) or ALLOCATED(SCALAR=...)
> >
> > In Tobias' original patch (attached to the PR), he
> > tried to make both ARRAY and SCALAR options, then
> > in gfc_check_allocated() appropriate checking was
> > added.  I started down that road, but intrinsic.c(
> > sort_actual) got in the way.  Fortunately, the
> > checking for ARRAY or SCALAR can be special-cased
> > in sort_actual.  See the patch.
> >
> > Regression tested on x86_64-*-freebsd.  OK to commit?
> >
> > 2019-08-01  Steven G. Kargl  
> >
> >   PR fortran/42546
> >   * check.c(gfc_check_allocated): Add comment pointing to ...
> >   * intrinsic.c(sort_actual): ... the checking done here.
> >
> > 2019-08-01  Steven G. Kargl  
> >
> >   PR fortran/42546
> >   * gfortran.dg/allocated_1.f90: New test.
> >   * gfortran.dg/allocated_2.f90: Ditto.
> >
> > --
> > Steve
>
> > Index: gcc/fortran/check.c
> > ===
> > --- gcc/fortran/check.c   (revision 273950)
> > +++ gcc/fortran/check.c   (working copy)
> > @@ -1168,6 +1168,10 @@ gfc_check_all_any (gfc_expr *mask, gfc_expr *dim)
> >  }
> >
> >
> > +/* Limited checking for ALLOCATED intrinsic.  Additional checking
> > +   is performed in intrinsic.c(sort_actual), because ALLOCATED
> > +   has two mutually exclusive non-optional arguments.  */
> > +
> >  bool
> >  gfc_check_allocated (gfc_expr *array)
> >  {
> > Index: gcc/fortran/intrinsic.c
> > ===
> > --- gcc/fortran/intrinsic.c   (revision 273950)
> > +++ gcc/fortran/intrinsic.c   (working copy)
> > @@ -4180,6 +4180,40 @@ sort_actual (const char *name, gfc_actual_arglist 
> > **ap
> >if (f == NULL && a == NULL)/* No arguments */
> >  return true;
> >
> > +  /* ALLOCATED has two mutually exclusive keywords, but only one
> > + can be present at time and neither is optional. */
> > +  if (strcmp (name, "allocated") == 0 && a->name)
> > +{
> > +  if (strcmp (a->name, "scalar") == 0)
> > + {
> > +  if (a->next)
> > + goto whoops;
> > +   if (a->expr->rank != 0)
> > + {
> > +   gfc_error ("Scalar entity required at %L", &a->expr->where);
> > +   return false;
> > + }
> > +  return true;
> > + }
> > +  else if (strcmp (a->name, "array") == 0)
> > + {
> > +  if (a->next)
> > + goto whoops;
> > +   if (a->expr->rank == 0)
> > + {
> > +   gfc_error ("Array entity required at %L", &a->expr->where);
> > +   return false;
> > + }
> > +  return true;
> > + }
> > +  else
> > + {
> > +   gfc_error ("Invalid keyword %qs in %qs intrinsic function at %L",
> > +  a->name, name, @a->expr->where);
> > +   return false;
> > + }
> > +}
> > +
> >for (;;)
> >  {/* Put the nonkeyword arguments in a 1:1 
> > correspondence */
> >if (f == NULL)
> > @@ -4199,6 +4233,7 @@ sort_actual (const char *name, gfc_actual_arglist **ap
> >if (a == NULL)
> >  goto do_sort;
> >
> > +whoops:
> >gfc_error ("Too many arguments in call to %qs at %L", name, where);
> >return false;
> >
> > Index: gcc/testsuite/gfortran.dg/allocated_1.f90
> > ===
> > --- gcc/testsuite/gfortran.dg/allocated_1.f90 (nonexistent)
> > +++ gcc/testsuite/gfortran.dg/allocated_1.f90 (working copy)
> > @@ -0,0 +1,24 @@
> > +! { dg-do run }
> > +program foo
> > +
> > +   implicit none
> > +
> > +   integer, allocatable :: x
> > +   integer, allocatable :: a(:)
> > +
> > +   logical a1, a2
> > +
> > +   a1 = allocated(scalar=x)
> > +   if (a1 .neqv. .false.) stop 1
> > +   a2 = allocated(array=a)
> > +   if (a2 .neqv. .false.) stop 2
> > +
> > +   allocate(x)
> > +   allocate(a(2))
> > +
> > +   a1 = allocated(scalar=x)
> > +   if (a1 .neqv. .true.) stop 3
> > +   a2 = allocated(array=a)
> > +   if (a2 .neqv. .true.) stop 4
> > +
> > +end program foo
> > Index: gcc/testsuite/gfortran.dg/allocated_2.f90
> > ===
> > --- gcc/testsuite/gfortran.dg/allocated_2.f90 (nonexistent)
> > +++ gcc/testsuite/gfortran.dg/allocated_2.f90 (working copy)
> > @@ -0,0 +1,16 @@
> > +! { dg-do compile }
> > +program foo
> > +
> > +   implicit none
> > +
> > +   integer, allocatable :: x
> > +   integer, allocatable :: a(:)
> > +
> > +   logical a1, a2
> > +
> > +   a1 = allocated(scalar=a)   ! { dg-error "Scalar

Re: C++ PATCH for c++/91264 - detect modifying const objects in constexpr

[Marek Polacek]

On Mon, Aug 05, 2019 at 03:54:19PM -0400, Jason Merrill wrote:
> On 7/31/19 3:26 PM, Marek Polacek wrote:
> > One of the features of constexpr is that it doesn't allow UB; and such UB 
> > must
> > be detected at compile-time.  So running your code in a context that 
> > requires
> > a constant expression should ensure that the code in question is free of UB.
> > In effect, constexpr can serve as a sanitizer.  E.g. this article describes 
> > in
> > in more detail:
> > 
> > 
> > [dcl.type.cv]p4 says "Any attempt to modify a const object during its 
> > lifetime
> > results in undefined behavior." However, as the article above points out, we
> > aren't detecting that case in constexpr evaluation.
> > 
> > This patch fixes that.  It's not that easy, though, because we have to keep 
> > in
> > mind [class.ctor]p5:
> > "A constructor can be invoked for a const, volatile or const volatile 
> > object.
> > const and volatile semantics are not applied on an object under 
> > construction.
> > They come into effect when the constructor for the most derived object 
> > ends."
> > 
> > I handled this by keeping a hash set which tracks objects under 
> > construction.
> > I considered other options, such as going up call_stack, but that wouldn't
> > work with trivial constructor/op=.  It was also interesting to find out that
> > the definition of TREE_HAS_CONSTRUCTOR says "When appearing in a FIELD_DECL,
> > it means that this field has been duly initialized in its constructor" 
> > though
> > nowhere in the codebase do we set TREE_HAS_CONSTRUCTOR on a FIELD_DECL as 
> > far
> > as I can see.  Unfortunately, using this bit proved useless for my needs 
> > here.
> 
> > Also, be mindful of mutable subobjects.
> > 
> > Does this approach look like an appropriate strategy for tracking objects'
> > construction?
> 
> For scalar objects, we should be able to rely on INIT_EXPR vs. MODIFY_EXPR
> to distinguish between initialization and modification; for class objects, I

This is already true: only class object go into the hash set.

> wonder about setting a flag on the CONSTRUCTOR after initialization is
> complete to indicate that the value is now constant.

But here we're not dealing with CONSTRUCTORs in the gcc sense (i.e. exprs with
TREE_CODE == CONSTRUCTOR).  We have a CALL_EXPR like Y::Y ((struct Y *) &y),
which initializes the object "y".  Setting a flag on the CALL_EXPR or its 
underlying
function decl wouldn't help.  (Also, all 6 TREE_LANG_FLAGs for a CONSTRUCTOR
are used.)

Am I missing something?

Marek

Re: [PATCH] PR fortran/91359 -- A function should return something

[Paul Richard Thomas]

Hi Steve,

That certainly does the trick! OK to commit as far back as you have
the intestinal fortitude for.

Thanks

Paul

On Tue, 6 Aug 2019 at 19:24, Steve Kargl
 wrote:
>
> The spaghetti code in PR fortran/91359 has a few gotos
> to jump to different places.  In doing so, gfortran
> with generate a function that has a naked 'return'.
> Namely,
>
> foo ()
> {
>   logical(kind=4) __result_foo;
>
>   goto __label_02;
>   __label_01:;
>   return; /* <-- THIS IS BAD.  */
>   __label_02:;
>   __result_foo = 0;
>   if (!__result_foo) goto __label_01;
>   L.1:;
>   return __result_foo;
>   return __result_foo;
> }
>
> The attached patch avoids the naked 'return' by
> adding the variable that is constructed from the
> the function name.  Namely,
>
> foo ()
> {
>   logical(kind=4) __result_foo;
>
>   goto __label_02;
>   __label_01:;
>   return __result_foo;/* <-- THIS IS GOOD.  */
>   __label_02:;
>   __result_foo = 0;
>   if (!__result_foo) goto __label_01;
>   L.1:;
>   return __result_foo;
>   return __result_foo;
> }
>
> Regression tested on x86_64-*-freebsd.  OK to commit?
>
> 2019-08-06  Steven G. Kargl  
>
> PR fortran/91359
> * trans-decl.c (gfc_generate_return): Ensure something is returned
> from a function.
>
> 2019-08-06  Steven G. Kargl  
>
> PR fortran/91359
> * gfortran.dg/pr91359_1.f: New test.
> * gfortran.dg/pr91359_2.f: Ditto.
>
> --
> Steve



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

Re: [PATCH] PR fortran/42546 -- ALLOCATED has 2 mutually exclusive keywords

[Steve Kargl]

Ping.

On Thu, Aug 01, 2019 at 02:11:39PM -0700, Steve Kargl wrote:
> The attached patch fixed the issues raised in the
> PR fortran/42546.  Namely, ALLOCATED has two possible
> keywords: ALLOCATE(ARRAY=...) or ALLOCATED(SCALAR=...)
> 
> In Tobias' original patch (attached to the PR), he 
> tried to make both ARRAY and SCALAR options, then 
> in gfc_check_allocated() appropriate checking was
> added.  I started down that road, but intrinsic.c(
> sort_actual) got in the way.  Fortunately, the 
> checking for ARRAY or SCALAR can be special-cased
> in sort_actual.  See the patch.
> 
> Regression tested on x86_64-*-freebsd.  OK to commit?
> 
> 2019-08-01  Steven G. Kargl  
> 
>   PR fortran/42546
>   * check.c(gfc_check_allocated): Add comment pointing to ...
>   * intrinsic.c(sort_actual): ... the checking done here.
>  
> 2019-08-01  Steven G. Kargl  
> 
>   PR fortran/42546
>   * gfortran.dg/allocated_1.f90: New test.
>   * gfortran.dg/allocated_2.f90: Ditto.
> 
> -- 
> Steve

> Index: gcc/fortran/check.c
> ===
> --- gcc/fortran/check.c   (revision 273950)
> +++ gcc/fortran/check.c   (working copy)
> @@ -1168,6 +1168,10 @@ gfc_check_all_any (gfc_expr *mask, gfc_expr *dim)
>  }
>  
>  
> +/* Limited checking for ALLOCATED intrinsic.  Additional checking
> +   is performed in intrinsic.c(sort_actual), because ALLOCATED
> +   has two mutually exclusive non-optional arguments.  */
> +
>  bool
>  gfc_check_allocated (gfc_expr *array)
>  {
> Index: gcc/fortran/intrinsic.c
> ===
> --- gcc/fortran/intrinsic.c   (revision 273950)
> +++ gcc/fortran/intrinsic.c   (working copy)
> @@ -4180,6 +4180,40 @@ sort_actual (const char *name, gfc_actual_arglist **ap
>if (f == NULL && a == NULL)/* No arguments */
>  return true;
>  
> +  /* ALLOCATED has two mutually exclusive keywords, but only one
> + can be present at time and neither is optional. */
> +  if (strcmp (name, "allocated") == 0 && a->name)
> +{
> +  if (strcmp (a->name, "scalar") == 0)
> + {
> +  if (a->next)
> + goto whoops;
> +   if (a->expr->rank != 0)
> + {
> +   gfc_error ("Scalar entity required at %L", &a->expr->where);
> +   return false;
> + }
> +  return true;
> + }
> +  else if (strcmp (a->name, "array") == 0)
> + {
> +  if (a->next)
> + goto whoops;
> +   if (a->expr->rank == 0)
> + {
> +   gfc_error ("Array entity required at %L", &a->expr->where);
> +   return false;
> + }
> +  return true;
> + }
> +  else
> + {
> +   gfc_error ("Invalid keyword %qs in %qs intrinsic function at %L",
> +  a->name, name, @a->expr->where);
> +   return false;
> + }
> +}
> +
>for (;;)
>  {/* Put the nonkeyword arguments in a 1:1 correspondence 
> */
>if (f == NULL)
> @@ -4199,6 +4233,7 @@ sort_actual (const char *name, gfc_actual_arglist **ap
>if (a == NULL)
>  goto do_sort;
>  
> +whoops:
>gfc_error ("Too many arguments in call to %qs at %L", name, where);
>return false;
>  
> Index: gcc/testsuite/gfortran.dg/allocated_1.f90
> ===
> --- gcc/testsuite/gfortran.dg/allocated_1.f90 (nonexistent)
> +++ gcc/testsuite/gfortran.dg/allocated_1.f90 (working copy)
> @@ -0,0 +1,24 @@
> +! { dg-do run }
> +program foo
> +
> +   implicit none
> +
> +   integer, allocatable :: x
> +   integer, allocatable :: a(:)
> +
> +   logical a1, a2
> +
> +   a1 = allocated(scalar=x)
> +   if (a1 .neqv. .false.) stop 1
> +   a2 = allocated(array=a)
> +   if (a2 .neqv. .false.) stop 2
> +
> +   allocate(x)
> +   allocate(a(2))
> +
> +   a1 = allocated(scalar=x)
> +   if (a1 .neqv. .true.) stop 3
> +   a2 = allocated(array=a)
> +   if (a2 .neqv. .true.) stop 4
> +
> +end program foo
> Index: gcc/testsuite/gfortran.dg/allocated_2.f90
> ===
> --- gcc/testsuite/gfortran.dg/allocated_2.f90 (nonexistent)
> +++ gcc/testsuite/gfortran.dg/allocated_2.f90 (working copy)
> @@ -0,0 +1,16 @@
> +! { dg-do compile }
> +program foo
> +
> +   implicit none
> +
> +   integer, allocatable :: x
> +   integer, allocatable :: a(:)
> +
> +   logical a1, a2
> +
> +   a1 = allocated(scalar=a)   ! { dg-error "Scalar entity required" }
> +   a2 = allocated(array=x)! { dg-error "Array entity required" }
> +   a1 = allocated(scalar=x, array=a)   ! { dg-error "Too many arguments" }
> +   a1 = allocated(array=a, scalar=x)   ! { dg-error "Too many arguments" }
> +
> +end program foo


-- 
Steve
20170425 https://www.youtube.com/watch?v=VWUpyCsUKR4
20161221 https://www.youtube.com/watch?v=IbCHE-hONow

Re: [PATCH] Adding _Dependent_ptr type qualifier in C part 1/3

[Akshat Garg]

Many thanks for your feedback on the proposal and the patch.
As you said, the feature should be available only for -std=c2x. I will look
into this how can it be done.
As you also said, we need some more extensive test coverage. Here is one
example I tried to make for invalid use of _Dependent_ptr.

/* Test for _Dependent_ptr. _Dependent_ptr qualified pointer initialization
tests. */
/* { dg-do compile } */
/* { dg-options "-pedantic-errors" } */

typedef __SIZE_TYPE__ size_t;
extern void exit (int);

#define TEST_SIMPLE_DECLARE(TYPE)   \
  do   \
{ \
  TYPE _Dependent_ptr p; /* { dg-error " invalid use of
'_Dependent_ptr'" } */   \
} \
  while (0)

#define TEST_SIMPLE_DECLARE_VARIABLE()   \
  do   \
{ \
  TEST_SIMPLE_DECLARE (_Bool); \
  TEST_SIMPLE_DECLARE (char);   \
  TEST_SIMPLE_DECLARE (signed char); \
  TEST_SIMPLE_DECLARE (unsigned char);   \
  TEST_SIMPLE_DECLARE (signed short); \
  TEST_SIMPLE_DECLARE (unsigned short); \
  TEST_SIMPLE_DECLARE (signed int);   \
  TEST_SIMPLE_DECLARE (unsigned int); \
  TEST_SIMPLE_DECLARE (signed long); \
  TEST_SIMPLE_DECLARE (unsigned long);   \
  TEST_SIMPLE_DECLARE (signed long long); \
  TEST_SIMPLE_DECLARE (unsigned long long); \
  TEST_SIMPLE_DECLARE (float); \
  TEST_SIMPLE_DECLARE (double);   \
  TEST_SIMPLE_DECLARE (long double); \
  TEST_SIMPLE_DECLARE (_Complex float); \
  TEST_SIMPLE_DECLARE (_Complex double);   \
  TEST_SIMPLE_DECLARE (_Complex long double);   \
 } \
  while (0)

static void
test_simple_declare (void)
{
  TEST_SIMPLE_DECLARE_VARIABLE ();
}

int main (void)
{
  test_simple_declare ();
  exit (0);
}
Let me know if this is what you want to see.
For function pointers, I am not sure how this qualifier should behave. But,
as you said, indeed the code for rejecting invalid _Dependent_ptr
declarations allows the function pointers usage. I have made one example as
shown, also let me know if this looks correct:

#include "../gcc/gcc/ginclude/stddef.h"
#include "../gcc/gcc/ginclude/stdatomic.h"

typedef __SIZE_TYPE__ size_t;
extern void abort (void);
extern void exit (int);
extern void *malloc (size_t);
extern int assert ();

_Atomic void (*fp)();
int count = 0;

#define rcu_assign_pointer(p,v) \
  atomic_store_explicit(&(p), (v), memory_order_release);

#define rcu_dereference(p)  \
  atomic_load_explicit(&(p), memory_order_consume);

void my_function(){
  count++;
}

void thread0 ()

{

rcu_assign_pointer(fp, &my_function);
}

void thread1 ()
{
 void (* _Dependent_ptr p)();
p = rcu_dereference(fp);
if (p)
  (*p)();
}

 I will try to make some new test cases for the conversions between
different qualifiers, starting with the list provided by Paul and looking
over the test cases made for other qualifiers and let you guys know.
In example p0190r4_fig10.c, the dg-warning directives are used to suppress
the warnings issues which compiler gives when the assignment is between
incompatible pointer types. These warnings are not our main tests. The main
test is to see if the declarations are defined properly and the statements
with dependency have not changed or reordered (what I think).
rcu_assign_pointer() does not starts the dependency. The dependency starts
from the first assignment like

struct rcutest * _Dependent_ptr p = rcu_dereference(gp);

and the latter references made to variable p should not be reordered or
being replaced by the other variables that may break the user intended
dependency chain. Making p _Dependent_ptr qualified ensures that. Also,
rcu_dereference() is atomic_load_explicit() and rcu_assign_pointer is
atomic_store_explicit.
Kindly, let me know if that makes sense to you.

Akshat

On Sat, Aug 3, 2019 at 8:00 AM Martin Sebor  wrote:

> On 8/1/19 9:26 AM, Paul McKenney wrote:
> > Excellent point, this discussion needs to be made official.
> > Please see attached for an initial draft of a working paper.
> >
> > Thoughts?
>
> The draft is a start but it (obviously) needs a lot of work to cover
> the constraints and semantics so I'll just comment on the patch in
> a bit more detail.
>
> Since the feature is being (or will be) proposed to WG14 and could
> change I would expect it to be only available under -std=c2x and
> disabled otherwise, so that code that makes use of it does so with
> the understanding that it's only experimental.  (I just noticed
> Akshat email with a tweak to the patch.  I'm not sure that issuing
> a pedantic warning and having the name in the implementation namepace
> is enough but others (t

[PATCH] PR fortran/91359 -- A function should return something

[Steve Kargl]

The spaghetti code in PR fortran/91359 has a few gotos
to jump to different places.  In doing so, gfortran 
with generate a function that has a naked 'return'.
Namely,

foo ()
{
  logical(kind=4) __result_foo;

  goto __label_02;
  __label_01:;
  return; /* <-- THIS IS BAD.  */
  __label_02:;
  __result_foo = 0;
  if (!__result_foo) goto __label_01;
  L.1:;
  return __result_foo;
  return __result_foo;
}

The attached patch avoids the naked 'return' by
adding the variable that is constructed from the 
the function name.  Namely,

foo ()
{
  logical(kind=4) __result_foo;

  goto __label_02;
  __label_01:;
  return __result_foo;/* <-- THIS IS GOOD.  */
  __label_02:;
  __result_foo = 0;
  if (!__result_foo) goto __label_01;
  L.1:;
  return __result_foo;
  return __result_foo;
}

Regression tested on x86_64-*-freebsd.  OK to commit?

2019-08-06  Steven G. Kargl  

PR fortran/91359
* trans-decl.c (gfc_generate_return): Ensure something is returned
from a function.

2019-08-06  Steven G. Kargl  

PR fortran/91359
* gfortran.dg/pr91359_1.f: New test.
* gfortran.dg/pr91359_2.f: Ditto.

-- 
Steve
Index: gcc/fortran/trans-decl.c
===
--- gcc/fortran/trans-decl.c	(revision 274121)
+++ gcc/fortran/trans-decl.c	(working copy)
@@ -6449,6 +6449,20 @@ gfc_generate_return (void)
 TREE_TYPE (result), DECL_RESULT (fndecl),
 result);
 	}
+  else
+	{
+	  /* If the function does not have a result variable, result is
+	 NULL_TREE, and a 'return' is generated without a variable.
+	 The following generates a 'return __result_XXX' where XXX is
+	 the function name.  */
+	  if (sym == sym->result && sym->attr.function)
+	{
+	  result = gfc_get_fake_result_decl (sym, 0);
+	  result = fold_build2_loc (input_location, MODIFY_EXPR,
+	TREE_TYPE (result),
+	DECL_RESULT (fndecl), result);
+	}
+	}
 }
 
   return build1_v (RETURN_EXPR, result);
Index: gcc/testsuite/gfortran.dg/pr91359_1.f
===
--- gcc/testsuite/gfortran.dg/pr91359_1.f	(nonexistent)
+++ gcc/testsuite/gfortran.dg/pr91359_1.f	(working copy)
@@ -0,0 +1,17 @@
+! { dg do }
+! PR fortran/91359
+! Orginal code contributed by Brian T. Carcich 
+!
+  logical function zero() result(a)
+ goto 2
+1return
+2a = .false.
+ if (.not.a) goto 1
+ return
+  end
+
+  program test_zero
+ logical zero
+ if (zero()) stop 'FAIL:  zero() returned .TRUE.'
+ stop 'OKAY:  zero() returned .FALSE.'
+  end
Index: gcc/testsuite/gfortran.dg/pr91359_2.f
===
--- gcc/testsuite/gfortran.dg/pr91359_2.f	(nonexistent)
+++ gcc/testsuite/gfortran.dg/pr91359_2.f	(working copy)
@@ -0,0 +1,17 @@
+! { dg do }
+! PR fortran/91359
+! Orginal code contributed by Brian T. Carcich 
+!
+  logical function zero() result(a)
+ goto 2
+1return
+2a = .false.
+ if (.not.a) goto 1
+ return
+  end
+
+  program test_zero
+ logical zero
+ if (zero()) stop 'FAIL:  zero() returned .TRUE.'
+ stop 'OKAY:  zero() returned .FALSE.'
+  end

Re: Canonicalization of compares performed as side-effect operations

[Uros Bizjak]

Hello!

> On Tue, Aug 06, 2019 at 05:49:17PM +0100, Richard Earnshaw (lists) wrote:
>> On 06/08/2019 17:39, Segher Boessenkool wrote:
>> >>>What's wrong with describing the canonical form in your MD?  You'll need
>> >>>some reversed condition code thingy, but that's it?
>> >>
>> >>It doesn't describe what the instruction does.  The negation has a side
>> >>effect of setting the flags, but the flags are swapped because the
>> >>side-effect comparison is swapped from a normal compare.  As I
>> >>mentioned, SELECT_CC_MODE doesn't help because it can't see the context
>> >>and the comparison just looks 'normal'.
>> >
>> >Sure, and we can work on making combine do what you want, but your existing
>> >pattern is *incorrect*.  It needs fixing, and probably before we do other
>> >things.
>>
>> Why is it incorrect?  It's not canonical, sure.  But the cannonical form
>> does NOT describe what the instruction does.
>
> More precisely: not having the canonical form of this in your MD is what
> is incorrect.

There is TARGET_CANONICALIZE_COMPARISON target hook that can solve the
issue here. Please see x86, where we canonicalize
*cmp__i387 via
ix86_canonicalize_comparison. As said in the comment:

  /* The order of operands in x87 ficom compare is forced by combine in
 simplify_comparison () function. Float operator is treated as RTX_OBJ
 with a precedence over other operators and is always put in the first
 place. Swap condition and operands to match ficom instruction.  */

The issue looks similar to me.

Uros.

Re: Canonicalization of compares performed as side-effect operations

[Eric Botcazou]

> Why is it incorrect?  It's not canonical, sure.  But the cannonical form
> does NOT describe what the instruction does.

Yes, you run into this when you try to be clever with the carry.  For the 
Visium port I kludged around it by using:

  [(set (reg:CCC R_FLAGS)
(compare:CCC (not:I (match_operand:I 0 "register_operand" "r"))
 (const_int -1)))]

and recognizing the -1 in SELECT_CC_MODE.  Obviously cumbersome though.

-- 
Eric Botcazou

Re: Canonicalization of compares performed as side-effect operations

[Segher Boessenkool]

On Tue, Aug 06, 2019 at 05:49:17PM +0100, Richard Earnshaw (lists) wrote:
> On 06/08/2019 17:39, Segher Boessenkool wrote:
> >>>What's wrong with describing the canonical form in your MD?  You'll need
> >>>some reversed condition code thingy, but that's it?
> >>
> >>It doesn't describe what the instruction does.  The negation has a side
> >>effect of setting the flags, but the flags are swapped because the
> >>side-effect comparison is swapped from a normal compare.  As I
> >>mentioned, SELECT_CC_MODE doesn't help because it can't see the context
> >>and the comparison just looks 'normal'.
> >
> >Sure, and we can work on making combine do what you want, but your existing
> >pattern is *incorrect*.  It needs fixing, and probably before we do other
> >things.
> 
> Why is it incorrect?  It's not canonical, sure.  But the cannonical form 
> does NOT describe what the instruction does.

More precisely: not having the canonical form of this in your MD is what
is incorrect.


Segher

Re: [PATCH] Detect not-cloned new/delete operators in DCE.

[Martin Jambor]

Hi,

unfortunately I cannot look into the problem now and I don't have my
phone set up to review patches in a sane way, but to answer your
question below...


On Tue, Aug 06 2019, Martin Liška wrote:
> On 8/6/19 2:42 PM, Martin Liška wrote:

...

>> Hm, strange that the ISRA clones don't have n->clone_of set. It's created 
>> here:
>> 
...

>> 
>> @Martin, @Honza: Why do we not set clone_of in this transformation?
>
...node->clone_of is set only while a clone created in the true IPA
stages has no body on its own and shares the body with the
original. These clones form a tree and their clone_of is cleared when
they get a body.  IPA-SRA is not a true IPA pass and the clones it
creates are created with create_clone_with_body (or similarly named)
method which immediately gives them a body, so setting clone_of would be
wrong. (The new IPA-SRA is a true IPA pass and so its clones have phase
when their clone_of is set).

When an IPA-stage clone gets its body (when it is materialized),
node->former_clone_of gets set to the decl (as opposed to cgraph_node)
of the original node and hopefully create_clone_with_body sets it
too. Can you perhaps use that?

> If I'm correct cgraph_node::clone is used for inline clones only?
>

IPA-CP also creates clones, it does so by calling
cgraph_node::create_virtual_clone but that also sets clone_of.

Martin

Re: Canonicalization of compares performed as side-effect operations

[Richard Earnshaw (lists)]

On 06/08/2019 17:39, Segher Boessenkool wrote:

On Tue, Aug 06, 2019 at 05:22:48PM +0100, Richard Earnshaw (lists) wrote:

On 06/08/2019 17:17, Segher Boessenkool wrote:

Hi Richard,

On Tue, Aug 06, 2019 at 04:35:04PM +0100, Richard Earnshaw (lists) wrote:

Arm has an instruction that performs the following operation:

(parallel [
 (set (reg:CC 100 cc)
 (compare:CC (const_int 0 [0])
 (reg:SI 121)))
 (set (reg:SI 113)
 (neg:SI (reg:SI 121)))
 ])

This is simply a reverse subtract from the constant zero, and setting
the condition flags.  It's the low part of a negdi2 expansion.

However, combine will rip this up and try to transform the compare into
'canonical' form, ie

(parallel [
 (set (reg:CC 100 cc)
 (compare:CC (reg:SI 121)
 (const_int 0 [0])))
 (set (reg:SI 113)
 (neg:SI (reg:SI 121)))
 ])

(and obviously swapping the condition on the instruction that uses the
comparison result).

This, of course, doesn't match the behaviour of the instruction and
no-longer matches the pattern in the md file.


It is, however, canonical RTL:

(from md.texi:)

   In addition to algebraic simplifications, following canonicalizations
   are performed:

   @itemize @bullet
   @item
   For commutative and comparison operators, a constant is always made the
   second operand.  If a machine only supports a constant as the second
   operand, only patterns that match a constant in the second operand need
   be supplied.

Putting the constant first is non-canonical RTL and will in general not
match any instructions generated by GCC.


So is there a way to describe this instruction within the compiler, or a
way to stop simplify_set from making this sort of simplification?


What's wrong with describing the canonical form in your MD?  You'll need
some reversed condition code thingy, but that's it?


It doesn't describe what the instruction does.  The negation has a side
effect of setting the flags, but the flags are swapped because the
side-effect comparison is swapped from a normal compare.  As I
mentioned, SELECT_CC_MODE doesn't help because it can't see the context
and the comparison just looks 'normal'.


Sure, and we can work on making combine do what you want, but your existing
pattern is *incorrect*.  It needs fixing, and probably before we do other
things.



Why is it incorrect?  It's not canonical, sure.  But the cannonical form 
does NOT describe what the instruction does.


R.

Re: Canonicalization of compares performed as side-effect operations

[Segher Boessenkool]

On Tue, Aug 06, 2019 at 05:22:48PM +0100, Richard Earnshaw (lists) wrote:
> On 06/08/2019 17:17, Segher Boessenkool wrote:
> >Hi Richard,
> >
> >On Tue, Aug 06, 2019 at 04:35:04PM +0100, Richard Earnshaw (lists) wrote:
> >>Arm has an instruction that performs the following operation:
> >>
> >>(parallel [
> >> (set (reg:CC 100 cc)
> >> (compare:CC (const_int 0 [0])
> >> (reg:SI 121)))
> >> (set (reg:SI 113)
> >> (neg:SI (reg:SI 121)))
> >> ])
> >>
> >>This is simply a reverse subtract from the constant zero, and setting
> >>the condition flags.  It's the low part of a negdi2 expansion.
> >>
> >>However, combine will rip this up and try to transform the compare into
> >>'canonical' form, ie
> >>
> >>(parallel [
> >> (set (reg:CC 100 cc)
> >> (compare:CC (reg:SI 121)
> >> (const_int 0 [0])))
> >> (set (reg:SI 113)
> >> (neg:SI (reg:SI 121)))
> >> ])
> >>
> >>(and obviously swapping the condition on the instruction that uses the
> >>comparison result).
> >>
> >>This, of course, doesn't match the behaviour of the instruction and
> >>no-longer matches the pattern in the md file.
> >
> >It is, however, canonical RTL:
> >
> >(from md.texi:)
> >
> >   In addition to algebraic simplifications, following canonicalizations
> >   are performed:
> >
> >   @itemize @bullet
> >   @item
> >   For commutative and comparison operators, a constant is always made the
> >   second operand.  If a machine only supports a constant as the second
> >   operand, only patterns that match a constant in the second operand need
> >   be supplied.
> >
> >Putting the constant first is non-canonical RTL and will in general not
> >match any instructions generated by GCC.
> >
> >>So is there a way to describe this instruction within the compiler, or a
> >>way to stop simplify_set from making this sort of simplification?
> >
> >What's wrong with describing the canonical form in your MD?  You'll need
> >some reversed condition code thingy, but that's it?
> 
> It doesn't describe what the instruction does.  The negation has a side 
> effect of setting the flags, but the flags are swapped because the 
> side-effect comparison is swapped from a normal compare.  As I 
> mentioned, SELECT_CC_MODE doesn't help because it can't see the context 
> and the comparison just looks 'normal'.

Sure, and we can work on making combine do what you want, but your existing
pattern is *incorrect*.  It needs fixing, and probably before we do other
things.


Segher

Re: Canonicalization of compares performed as side-effect operations

[Richard Earnshaw (lists)]

On 06/08/2019 17:22, Richard Earnshaw (lists) wrote:

On 06/08/2019 17:17, Segher Boessenkool wrote:

Hi Richard,

On Tue, Aug 06, 2019 at 04:35:04PM +0100, Richard Earnshaw (lists) wrote:

Arm has an instruction that performs the following operation:

(parallel [
 (set (reg:CC 100 cc)
 (compare:CC (const_int 0 [0])
 (reg:SI 121)))
 (set (reg:SI 113)
 (neg:SI (reg:SI 121)))
 ])

This is simply a reverse subtract from the constant zero, and setting
the condition flags.  It's the low part of a negdi2 expansion.

However, combine will rip this up and try to transform the compare into
'canonical' form, ie

(parallel [
 (set (reg:CC 100 cc)
 (compare:CC (reg:SI 121)
 (const_int 0 [0])))
 (set (reg:SI 113)
 (neg:SI (reg:SI 121)))
 ])

(and obviously swapping the condition on the instruction that uses the
comparison result).

This, of course, doesn't match the behaviour of the instruction and
no-longer matches the pattern in the md file.


It is, however, canonical RTL:

(from md.texi:)

   In addition to algebraic simplifications, following canonicalizations
   are performed:

   @itemize @bullet
   @item
   For commutative and comparison operators, a constant is always made 
the

   second operand.  If a machine only supports a constant as the second
   operand, only patterns that match a constant in the second operand 
need

   be supplied.

Putting the constant first is non-canonical RTL and will in general not
match any instructions generated by GCC.


So is there a way to describe this instruction within the compiler, or a
way to stop simplify_set from making this sort of simplification?


What's wrong with describing the canonical form in your MD?  You'll need
some reversed condition code thingy, but that's it?


It doesn't describe what the instruction does.  The negation has a side 
effect of setting the flags, but the flags are swapped because the 
side-effect comparison is swapped from a normal compare.  As I 
mentioned, SELECT_CC_MODE doesn't help because it can't see the context 
and the comparison just looks 'normal'.


R.




See, for example, this comment in combine.c:

  /* Many machines that don't use CC0 have insns that can both perform an
 arithmetic operation and set the condition code.  These operations 
will

 be represented as a PARALLEL with the first element of the vector
 being a COMPARE of an arithmetic operation with the constant zero.
 The second element of the vector will set some pseudo to the result
 of the same arithmetic operation.  If we simplify the COMPARE, we 
won't
 match such a pattern and so will generate an extra insn.   Here we 
test

 for this case, where both the comparison and the operation result are
 needed, and make the PARALLEL by just replacing I2DEST in I3SRC with
 I2SRC.  Later we will make the PARALLEL that contains I2.  */

This is exactly the scenario we have here, but despite the comment, this 
is still happening in other places in combine.

Re: Patch ping: C++17 experimental status

[Marek Polacek]

On Tue, Aug 06, 2019 at 12:18:35PM -0400, Jason Merrill wrote:
> Yes, please.

Awesome.  Applied to CVS.

> On Tue, Aug 6, 2019 at 12:03 PM Marek Polacek  wrote:
> >
> > Can Jon's patch go in now?
> > https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00987.html

Marek

Re: Canonicalization of compares performed as side-effect operations

[Richard Earnshaw (lists)]

On 06/08/2019 17:17, Segher Boessenkool wrote:

Hi Richard,

On Tue, Aug 06, 2019 at 04:35:04PM +0100, Richard Earnshaw (lists) wrote:

Arm has an instruction that performs the following operation:

(parallel [
 (set (reg:CC 100 cc)
 (compare:CC (const_int 0 [0])
 (reg:SI 121)))
 (set (reg:SI 113)
 (neg:SI (reg:SI 121)))
 ])

This is simply a reverse subtract from the constant zero, and setting
the condition flags.  It's the low part of a negdi2 expansion.

However, combine will rip this up and try to transform the compare into
'canonical' form, ie

(parallel [
 (set (reg:CC 100 cc)
 (compare:CC (reg:SI 121)
 (const_int 0 [0])))
 (set (reg:SI 113)
 (neg:SI (reg:SI 121)))
 ])

(and obviously swapping the condition on the instruction that uses the
comparison result).

This, of course, doesn't match the behaviour of the instruction and
no-longer matches the pattern in the md file.


It is, however, canonical RTL:

(from md.texi:)

   In addition to algebraic simplifications, following canonicalizations
   are performed:

   @itemize @bullet
   @item
   For commutative and comparison operators, a constant is always made the
   second operand.  If a machine only supports a constant as the second
   operand, only patterns that match a constant in the second operand need
   be supplied.

Putting the constant first is non-canonical RTL and will in general not
match any instructions generated by GCC.


So is there a way to describe this instruction within the compiler, or a
way to stop simplify_set from making this sort of simplification?


What's wrong with describing the canonical form in your MD?  You'll need
some reversed condition code thingy, but that's it?


It doesn't describe what the instruction does.  The negation has a side 
effect of setting the flags, but the flags are swapped because the 
side-effect comparison is swapped from a normal compare.  As I 
mentioned, SELECT_CC_MODE doesn't help because it can't see the context 
and the comparison just looks 'normal'.


R.

Re: Patch ping: C++17 experimental status

[Jason Merrill]

Yes, please.

On Tue, Aug 6, 2019 at 12:03 PM Marek Polacek  wrote:
>
> Can Jon's patch go in now?
> https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00987.html
>
> Marek

Re: [PATCH] Fix file descriptor existence of MinGW.

[Martin Sebor]

On 8/6/19 9:55 AM, Martin Liška wrote:

On 8/6/19 5:35 PM, Martin Sebor wrote:

On 8/6/19 6:04 AM, Martin Liška wrote:

Hi.

The patch is about proper checking of file descriptors on Windows.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.


Is there a way to share the definition of the new function so
it doesn't have to be duplicated?


I would like to, but I'm wondering which header and source file
should I use for it?


I'm not sure.  It's a general purpose function that could go
in some utility library like libiberty but I don't know what
the process for enhancing it is to tell if that's appropriate.


Other than that, I'm wondering if the guard for F_GETFD is
necessary and when (the original code didn't guard its use).


Because the value is not defined on MinGW targets where _get_osfhandle
call must be used ;)


I was unclear -- sorry.  What I meant was: since there is
a check for MinGW, is the check for F_GETFD necessary, or
would this be sufficient and preferable?

  #if defined(_WIN32)
return _get_osfhandle (fd) != -1;
  else
return fcntl (fd, F_GETFD) >= 0;
  #endif

That way it's clear that only two possibilities are expected.

Martin



Martin



Martin




@Pekka: Can you please test it on Windows?

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2019-08-06  Martin Liska  

 PR bootstrap/91352
 * gcc.c (fd_exists): New.
 (driver::detect_jobserver): Use fd_exists.
 * lto-wrapper.c (fd_exists): New.
 (jobserver_active_p): Use fd_exists.
---
   gcc/gcc.c | 19 +--
   gcc/lto-wrapper.c | 19 +--
   2 files changed, 34 insertions(+), 4 deletions(-)

Re: Canonicalization of compares performed as side-effect operations

[Segher Boessenkool]

Hi Richard,

On Tue, Aug 06, 2019 at 04:35:04PM +0100, Richard Earnshaw (lists) wrote:
> Arm has an instruction that performs the following operation:
> 
> (parallel [
> (set (reg:CC 100 cc)
> (compare:CC (const_int 0 [0])
> (reg:SI 121)))
> (set (reg:SI 113)
> (neg:SI (reg:SI 121)))
> ])
> 
> This is simply a reverse subtract from the constant zero, and setting 
> the condition flags.  It's the low part of a negdi2 expansion.
> 
> However, combine will rip this up and try to transform the compare into 
> 'canonical' form, ie
> 
> (parallel [
> (set (reg:CC 100 cc)
> (compare:CC (reg:SI 121)
> (const_int 0 [0])))
> (set (reg:SI 113)
> (neg:SI (reg:SI 121)))
> ])
> 
> (and obviously swapping the condition on the instruction that uses the 
> comparison result).
> 
> This, of course, doesn't match the behaviour of the instruction and 
> no-longer matches the pattern in the md file.

It is, however, canonical RTL:

(from md.texi:)

  In addition to algebraic simplifications, following canonicalizations
  are performed:

  @itemize @bullet
  @item
  For commutative and comparison operators, a constant is always made the
  second operand.  If a machine only supports a constant as the second
  operand, only patterns that match a constant in the second operand need
  be supplied.

Putting the constant first is non-canonical RTL and will in general not
match any instructions generated by GCC.

> So is there a way to describe this instruction within the compiler, or a 
> way to stop simplify_set from making this sort of simplification?

What's wrong with describing the canonical form in your MD?  You'll need
some reversed condition code thingy, but that's it?


Segher

Re: [PATCH][c++] Do not warn about unused macros while processing #pragma GCC optimize

[Martin Sebor]

On 8/5/19 7:53 PM, Piotr H. Dabrowski wrote:

Fixes c++/91318.

libcpp/ChangeLog:

2019-08-06  Piotr Henryk Dabrowski  

PR c++/91318
* include/cpplib.h: Added cpp_define_unused(), 
cpp_define_formatted_unused()
* directives.c: Likewise.

gcc/c-family/ChangeLog:

2019-08-06  Piotr Henryk Dabrowski  

PR c++/91318
* c-cppbuiltin.c: c_cpp_builtins_optimize_pragma(): use 
cpp_define_unused()


This isn't my area so I can't comment on the correctness of
the changes but I would recommend to also add a regression
test case to the test suite along with the fix.

It might be interesting to check whether the macros are defined
the same way in C and in C++, and if they correspond to the effect
of the options set by the pragmas (i.e., "later in the source file"
as documented).

Martin




diff --git a/gcc/c-family/c-cppbuiltin.c b/gcc/c-family/c-cppbuiltin.c
index d389f8ca4a0..47d0cefb85a 100644
--- a/gcc/c-family/c-cppbuiltin.c
+++ b/gcc/c-family/c-cppbuiltin.c
@@ -576,41 +576,41 @@ c_cpp_builtins_optimize_pragma (cpp_reader *pfile, tree 
prev_tree,
/* Other target-independent built-ins determined by command-line
   options.  */
if (!prev->x_optimize_size && cur->x_optimize_size)
-cpp_define (pfile, "__OPTIMIZE_SIZE__");
+cpp_define_unused (pfile, "__OPTIMIZE_SIZE__");
else if (prev->x_optimize_size && !cur->x_optimize_size)
  cpp_undef (pfile, "__OPTIMIZE_SIZE__");
  
if (!prev->x_optimize && cur->x_optimize)

-cpp_define (pfile, "__OPTIMIZE__");
+cpp_define_unused (pfile, "__OPTIMIZE__");
else if (prev->x_optimize && !cur->x_optimize)
  cpp_undef (pfile, "__OPTIMIZE__");
  
prev_fast_math = fast_math_flags_struct_set_p (prev);

cur_fast_math  = fast_math_flags_struct_set_p (cur);
if (!prev_fast_math && cur_fast_math)
-cpp_define (pfile, "__FAST_MATH__");
+cpp_define_unused (pfile, "__FAST_MATH__");
else if (prev_fast_math && !cur_fast_math)
  cpp_undef (pfile, "__FAST_MATH__");
  
if (!prev->x_flag_signaling_nans && cur->x_flag_signaling_nans)

-cpp_define (pfile, "__SUPPORT_SNAN__");
+cpp_define_unused (pfile, "__SUPPORT_SNAN__");
else if (prev->x_flag_signaling_nans && !cur->x_flag_signaling_nans)
  cpp_undef (pfile, "__SUPPORT_SNAN__");
  
if (!prev->x_flag_errno_math && cur->x_flag_errno_math)

  cpp_undef (pfile, "__NO_MATH_ERRNO__");
else if (prev->x_flag_errno_math && !cur->x_flag_errno_math)
-cpp_define (pfile, "__NO_MATH_ERRNO__");
+cpp_define_unused (pfile, "__NO_MATH_ERRNO__");
  
if (!prev->x_flag_finite_math_only && cur->x_flag_finite_math_only)

  {
cpp_undef (pfile, "__FINITE_MATH_ONLY__");
-  cpp_define (pfile, "__FINITE_MATH_ONLY__=1");
+  cpp_define_unused (pfile, "__FINITE_MATH_ONLY__=1");
  }
else if (prev->x_flag_finite_math_only && !cur->x_flag_finite_math_only)
  {
cpp_undef (pfile, "__FINITE_MATH_ONLY__");
-  cpp_define (pfile, "__FINITE_MATH_ONLY__=0");
+  cpp_define_unused (pfile, "__FINITE_MATH_ONLY__=0");
  }
  }
  
diff --git a/libcpp/directives.c b/libcpp/directives.c

index ddf8979d513..9a774c9ed04 100644
--- a/libcpp/directives.c
+++ b/libcpp/directives.c
@@ -2392,6 +2392,15 @@ cpp_define (cpp_reader *pfile, const char *str)
run_directive (pfile, T_DEFINE, buf, count);
  }
  
+/* Like cpp_define, but does not warn about unused macro.  */

+void
+cpp_define_unused (cpp_reader *pfile, const char *str)
+{
+unsigned char warn_unused_macros = CPP_OPTION (pfile, warn_unused_macros);
+CPP_OPTION (pfile, warn_unused_macros) = 0;
+cpp_define (pfile, str);
+CPP_OPTION (pfile, warn_unused_macros) = warn_unused_macros;
+}
  
  /* Use to build macros to be run through cpp_define() as

 described above.
@@ -2411,6 +2420,20 @@ cpp_define_formatted (cpp_reader *pfile, const char 
*fmt, ...)
free (ptr);
  }
  
+/* Like cpp_define_formatted, but does not warn about unused macro.  */

+void
+cpp_define_formatted_unused (cpp_reader *pfile, const char *fmt, ...)
+{
+  char *ptr;
+
+  va_list ap;
+  va_start (ap, fmt);
+  ptr = xvasprintf (fmt, ap);
+  va_end (ap);
+
+  cpp_define_unused (pfile, ptr);
+  free (ptr);
+}
  
  /* Slight variant of the above for use by initialize_builtins.  */

  void
diff --git a/libcpp/include/cpplib.h b/libcpp/include/cpplib.h
index a645f8136a6..8d3f9082601 100644
--- a/libcpp/include/cpplib.h
+++ b/libcpp/include/cpplib.h
@@ -1052,8 +1052,12 @@ extern cppchar_t cpp_host_to_exec_charset (cpp_reader *, 
cppchar_t);
  /* Used to register macros and assertions, perhaps from the command line.
 The text is the same as the command line argument.  */
  extern void cpp_define (cpp_reader *, const char *);
+extern void cpp_define_unused (cpp_reader *, const char *);
  extern void cpp_define_formatted (cpp_reader *pfile,
  const char *fmt, ...) ATTRIBUTE_PRINTF_2;
+extern void cpp_define_formatted_unused (cpp_

Patch ping: C++17 experimental status

[Marek Polacek]

Can Jon's patch go in now?
https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00987.html

Marek

[PATCH] P1651R0 bind_front should not unwrap reference_wrapper

[Jonathan Wakely]

P1651R0 bind_front should not unwrap reference_wrapper
* include/std/functional (bind_front): Don't unwrap reference_wrapper.
* include/std/version (__cpp_lib_bind_front): Update value.
* testsuite/20_util/function_objects/bind_front/1.cc: Fix test for
feature test macro.
* testsuite/20_util/function_objects/bind_front/2.cc: New test.

Tested x86_64-linux, committed to trunk.


commit b4d96c17e488f8bedcfd976e2e9030854211fc46
Author: redi 
Date:   Tue Aug 6 15:57:55 2019 +

P1651R0 bind_front should not unwrap reference_wrapper

P1651R0 bind_front should not unwrap reference_wrapper
* include/std/functional (bind_front): Don't unwrap 
reference_wrapper.
* include/std/version (__cpp_lib_bind_front): Update value.
* testsuite/20_util/function_objects/bind_front/1.cc: Fix test for
feature test macro.
* testsuite/20_util/function_objects/bind_front/2.cc: New test.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@274146 
138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/libstdc++-v3/include/std/functional 
b/libstdc++-v3/include/std/functional
index d610e914b59..30576f23d35 100644
--- a/libstdc++-v3/include/std/functional
+++ b/libstdc++-v3/include/std/functional
@@ -791,7 +791,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 }
 
 #if __cplusplus > 201703L
-#define __cpp_lib_bind_front 201902L
+#define __cpp_lib_bind_front 201907L
 
   template
 struct _Bind_front
@@ -877,7 +877,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   template
 using _Bind_front_t
-  = _Bind_front, unwrap_ref_decay_t<_Args>...>;
+  = _Bind_front, decay_t<_Args>...>;
 
   template
 _Bind_front_t<_Fn, _Args...>
diff --git a/libstdc++-v3/include/std/version b/libstdc++-v3/include/std/version
index bf15a0e8542..9548725d1b7 100644
--- a/libstdc++-v3/include/std/version
+++ b/libstdc++-v3/include/std/version
@@ -151,7 +151,7 @@
 #if __cplusplus > 201703L
 // c++2a
 #define __cpp_lib_atomic_ref 201806L
-#define __cpp_lib_bind_front 201902L
+#define __cpp_lib_bind_front 201907L
 #define __cpp_lib_bounded_array_traits 201902L
 #if __cpp_impl_destroying_delete
 # define __cpp_lib_destroying_delete 201806L
diff --git a/libstdc++-v3/testsuite/20_util/function_objects/bind_front/1.cc 
b/libstdc++-v3/testsuite/20_util/function_objects/bind_front/1.cc
index 8ebc2bab41a..c6cf5cf2baf 100644
--- a/libstdc++-v3/testsuite/20_util/function_objects/bind_front/1.cc
+++ b/libstdc++-v3/testsuite/20_util/function_objects/bind_front/1.cc
@@ -23,7 +23,7 @@
 
 #ifndef __cpp_lib_bind_front
 # error "Feature test macro for bind_front is missing"
-#elif __cpp_lib_bind_front < 201811L
+#elif __cpp_lib_bind_front < 201902L
 # error "Feature test macro for bind_front has wrong value"
 #endif
 
diff --git a/libstdc++-v3/testsuite/20_util/function_objects/bind_front/2.cc 
b/libstdc++-v3/testsuite/20_util/function_objects/bind_front/2.cc
new file mode 100644
index 000..b68cc65f719
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/function_objects/bind_front/2.cc
@@ -0,0 +1,91 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do run { target c++2a } }
+
+#include 
+#include 
+#include 
+#include 
+
+// P1651R0 bind_front should not unwrap reference_wrapper
+
+#ifndef __cpp_lib_bind_front
+# error "Feature test macro for bind_front is missing"
+#elif __cpp_lib_bind_front < 201907L
+# error "Feature test macro for bind_front has wrong value"
+#endif
+
+void functionAcceptingStringView(std::string_view) { }
+
+void
+test01()
+{
+  std::string s;
+  auto fs = std::bind_front(&functionAcceptingStringView, std::string_view(s));
+  fs();
+}
+
+template 
+struct PartialApply {
+PartialApply(F f) : f(f) {}
+F f;
+
+template  decltype(auto) operator()(A const&... a) const {
+if constexpr (std::is_invocable::value) {
+return f(a...);
+} else {
+return bind_front(*this, a...);
+}
+}
+};
+
+void
+test02()
+{
+  struct Thingy { };
+  std::unique_ptr thingy;
+  auto func = [](const std::unique_ptr&, int) {};
+  PartialApply{func}(std::ref(thingy))(10);
+}
+
+void
+test03()
+{
+  std::string str;
+  auto func = [](

Re: [PATCH] Implement "P0631R4 Math Constants" for C++20

[Jonathan Wakely]

On 31/07/19 17:57 -0500, Segher Boessenkool wrote:

On Wed, Jul 31, 2019 at 08:43:50PM +0200, Marc Glisse wrote:

On Wed, 31 Jul 2019, Jonathan Wakely wrote:

>So something like the attached patch.  The glibc  says the
>values I used have enough digits for IEEE quad-precision:
>
>/* The above constants are not adequate for computation using `long
>double's.
> Therefore we provide as an extension constants with similar names as a
> GNU extension.  Provide enough digits for the 128-bit IEEE quad.  */
>
>I don't know if we need more accuracy for IBM double double.

double double has less precision than quad so it should be fine.


The *precision* (as defined in IEEE 754) of double double is much higher
than that of IEEE quad precision float: "the maximum number of
significant digits that can be represented in a format" -- it's just
that most of those bits have to be zero!

Neither double-double is a subset of QP, nor the other way around.  This
is *the* problem we have in rs6000 with these two float formats: GCC
cannot handle if two FP formats are like that.

(The math constants here are fine for double double, of course).


Thanks. I've committed the attached patch then, after testing on
x86_64-linux and powerpc64le-linux.


commit 8c16cb54739973f84f27055c32c7e0cc557a784e
Author: redi 
Date:   Tue Aug 6 15:57:51 2019 +

Specialize std::numbers constants for __float128

* include/std/numbers [!__STRICT_ANSI__ && _GLIBCXX_USE_FLOAT128]
(e_v, log2e_v, log10e_v, pi_v, inv_pi_v, inv_sqrtpi_v, ln2_v, ln10_v)
(sqrt2_v, sqrt3_v, inv_sqrt3, egamma_v, phi_v): Add explicit
specializations for __float128.
* testsuite/26_numerics/numbers/float128.cc: New test.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@274145 138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/libstdc++-v3/include/std/numbers b/libstdc++-v3/include/std/numbers
index b8e38dd8080..314b130fbd5 100644
--- a/libstdc++-v3/include/std/numbers
+++ b/libstdc++-v3/include/std/numbers
@@ -133,6 +133,72 @@ namespace numbers
   inline constexpr double egamma = egamma_v;
   inline constexpr double phi = phi_v;
 
+#if !defined(__STRICT_ANSI__) && defined(_GLIBCXX_USE_FLOAT128)
+  template<>
+inline constexpr __float128 e_v<__float128>
+  = 2.718281828459045235360287471352662498Q;
+
+  /// log_2 e
+  template<>
+inline constexpr __float128 log2e_v<__float128>
+  = 1.442695040888963407359924681001892137Q;
+
+  /// log_10 e
+  template<>
+inline constexpr __float128 log10e_v<__float128>
+  = 0.434294481903251827651128918916605082Q;
+
+  /// pi
+  template<>
+inline constexpr __float128 pi_v<__float128>
+  = 3.141592653589793238462643383279502884Q;
+
+  /// 1/pi
+  template<>
+inline constexpr __float128 inv_pi_v<__float128>
+  = 0.318309886183790671537767526745028724Q;
+
+  /// 1/sqrt(pi)
+  template<>
+inline constexpr __float128 inv_sqrtpi_v<__float128>
+  = 0.564189583547756286948079451560772586Q;
+
+  /// log_e 2
+  template<>
+inline constexpr __float128 ln2_v<__float128>
+  = 0.693147180559945309417232121458176568Q;
+
+  /// log_e 10
+  template<>
+inline constexpr __float128 ln10_v<__float128>
+  = 2.302585092994045684017991454684364208Q;
+
+  /// sqrt(2)
+  template<>
+inline constexpr __float128 sqrt2_v<__float128>
+  = 1.414213562373095048801688724209698079Q;
+
+  /// sqrt(3)
+  template<>
+inline constexpr __float128 sqrt3_v<__float128>
+  = 1.732050807568877293527446341505872367Q;
+
+  /// 1/sqrt(3)
+  template<>
+inline constexpr __float128 inv_sqrt3_v<__float128>
+  = 0.577350269189625764509148780501957456Q;
+
+  /// The Euler-Mascheroni constant
+  template<>
+inline constexpr __float128 egamma_v<__float128>
+  = 0.577215664901532860606512090082402431Q;
+
+  /// The golden ratio, (1+sqrt(5))/2
+  template<>
+inline constexpr __float128 phi_v<__float128>
+  = 1.618033988749894848204586834365638118Q;
+#endif // USE_FLOAT128
+
 } // namespace numbers
 /// @}
 _GLIBCXX_END_NAMESPACE_VERSION
diff --git a/libstdc++-v3/testsuite/26_numerics/numbers/float128.cc b/libstdc++-v3/testsuite/26_numerics/numbers/float128.cc
new file mode 100644
index 000..2d9dafcd482
--- /dev/null
+++ b/libstdc++-v3/testsuite/26_numerics/numbers/float128.cc
@@ -0,0 +1,41 @@
+// Copyright (C) 2019 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received

Re: [PATCH] Fix file descriptor existence of MinGW.

[Martin Liška]

On 8/6/19 5:35 PM, Martin Sebor wrote:
> On 8/6/19 6:04 AM, Martin Liška wrote:
>> Hi.
>>
>> The patch is about proper checking of file descriptors on Windows.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Is there a way to share the definition of the new function so
> it doesn't have to be duplicated?

I would like to, but I'm wondering which header and source file
should I use for it?

> 
> Other than that, I'm wondering if the guard for F_GETFD is
> necessary and when (the original code didn't guard its use).

Because the value is not defined on MinGW targets where _get_osfhandle
call must be used ;)

Martin

> 
> Martin
> 
> 
>>
>> @Pekka: Can you please test it on Windows?
>>
>> Ready to be installed?
>> Thanks,
>> Martin
>>
>> gcc/ChangeLog:
>>
>> 2019-08-06  Martin Liska  
>>
>> PR bootstrap/91352
>> * gcc.c (fd_exists): New.
>> (driver::detect_jobserver): Use fd_exists.
>> * lto-wrapper.c (fd_exists): New.
>> (jobserver_active_p): Use fd_exists.
>> ---
>>   gcc/gcc.c | 19 +--
>>   gcc/lto-wrapper.c | 19 +--
>>   2 files changed, 34 insertions(+), 4 deletions(-)
>>
>>
> 

Re: [PATCH] Detect not-cloned new/delete operators in DCE.

[Marc Glisse]

On Tue, 6 Aug 2019, Martin Liška wrote:


Anyway, I'm sending patch that considers only such new/delete operators
that are not a clone of an original type. That should make the current
DCE code more solid.


DECL_IS_REPLACEABLE_OPERATOR_NEW_P seems to have been replaced with 
DECL_IS_OPERATOR_NEW_P. Is that on purpose?


I like your cleanup of having a single function to decide if this is the 
kind of operator new/delete DCE can handle, but you may have introduced 
long lines in the substitution.


--
Marc Glisse

[PATCH 6/9] Integrate that for IPA ICF.

[Martin Liska]

gcc/ChangeLog:

2019-07-24  Martin Liska  

* ipa-icf-gimple.c (func_checker::hash_operand_valueize): New
function created from compare_operand.
(func_checker::compare_cst_or_decl): Remove handling of
FIELD_DECLs as it's handled in operand_equal_p.
(func_checker::compare_operand): Transform
to func_checker::operand_equal_valueize.
(func_checker::operand_equal_valueize): New.
* ipa-icf-gimple.h (class func_checker): Inherit from
operand_compare.
* ipa-icf.c (sem_function::equals_private): Properly
set push_cfun and pop_cfun.
---
 gcc/ipa-icf-gimple.c | 226 +--
 gcc/ipa-icf-gimple.h |   8 +-
 gcc/ipa-icf.c|   7 +-
 3 files changed, 81 insertions(+), 160 deletions(-)

diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
index 4060c0e8eb3..8448d387428 100644
--- a/gcc/ipa-icf-gimple.c
+++ b/gcc/ipa-icf-gimple.c
@@ -324,6 +324,28 @@ func_checker::compare_memory_operand (tree t1, tree t2)
 /* Function compare for equality given trees T1 and T2 which
can be either a constant or a declaration type.  */
 
+bool
+func_checker::hash_operand_valueize (const_tree arg, inchash::hash &hstate,
+ unsigned int flags)
+{
+  switch (TREE_CODE (arg))
+{
+case FUNCTION_DECL:
+case VAR_DECL:
+case LABEL_DECL:
+case PARM_DECL:
+case RESULT_DECL:
+case CONST_DECL:
+case SSA_NAME:
+  return true;
+
+default:
+  break;
+}
+
+  return false;
+}
+
 bool
 func_checker::compare_cst_or_decl (tree t1, tree t2)
 {
@@ -347,19 +369,6 @@ func_checker::compare_cst_or_decl (tree t1, tree t2)
   return true;
 case VAR_DECL:
   return return_with_debug (compare_variable_decl (t1, t2));
-case FIELD_DECL:
-  {
-	tree offset1 = DECL_FIELD_OFFSET (t1);
-	tree offset2 = DECL_FIELD_OFFSET (t2);
-
-	tree bit_offset1 = DECL_FIELD_BIT_OFFSET (t1);
-	tree bit_offset2 = DECL_FIELD_BIT_OFFSET (t2);
-
-	ret = compare_operand (offset1, offset2)
-	  && compare_operand (bit_offset1, bit_offset2);
-
-	return return_with_debug (ret);
-  }
 case LABEL_DECL:
   {
 	if (t1 == t2)
@@ -383,165 +392,68 @@ func_checker::compare_cst_or_decl (tree t1, tree t2)
 }
 }
 
-/* Function responsible for comparison of various operands T1 and T2.
-   If these components, from functions FUNC1 and FUNC2, are equal, true
-   is returned.  */
-
-bool
-func_checker::compare_operand (tree t1, tree t2)
+int
+func_checker::operand_equal_valueize (const_tree ct1, const_tree ct2,
+  unsigned int)
 {
-  tree x1, x2, y1, y2, z1, z2;
-  bool ret;
-
-  if (!t1 && !t2)
-return true;
-  else if (!t1 || !t2)
-return false;
-
-  tree tt1 = TREE_TYPE (t1);
-  tree tt2 = TREE_TYPE (t2);
-
-  if (!func_checker::compatible_types_p (tt1, tt2))
-return false;
-
-  if (TREE_CODE (t1) != TREE_CODE (t2))
-return return_false ();
+  tree t1 = const_cast  (ct1);
+  tree t2 = const_cast  (ct2);
 
   switch (TREE_CODE (t1))
 {
-case CONSTRUCTOR:
+case FUNCTION_DECL:
+  /* All function decls are in the symbol table and known to match
+	 before we start comparing bodies.  */
+  return true;
+case VAR_DECL:
+  return return_with_debug (compare_variable_decl (t1, t2));
+case LABEL_DECL:
   {
-	unsigned length1 = CONSTRUCTOR_NELTS (t1);
-	unsigned length2 = CONSTRUCTOR_NELTS (t2);
-
-	if (length1 != length2)
-	  return return_false ();
-
-	for (unsigned i = 0; i < length1; i++)
-	  if (!compare_operand (CONSTRUCTOR_ELT (t1, i)->value,
-CONSTRUCTOR_ELT (t2, i)->value))
-	return return_false();
-
-	return true;
+	int *bb1 = m_label_bb_map.get (t1);
+	int *bb2 = m_label_bb_map.get (t2);
+	/* Labels can point to another function (non-local GOTOs).  */
+	return return_with_debug (bb1 != NULL && bb2 != NULL && *bb1 == *bb2);
   }
-case ARRAY_REF:
-case ARRAY_RANGE_REF:
-  /* First argument is the array, second is the index.  */
-  x1 = TREE_OPERAND (t1, 0);
-  x2 = TREE_OPERAND (t2, 0);
-  y1 = TREE_OPERAND (t1, 1);
-  y2 = TREE_OPERAND (t2, 1);
-
-  if (!compare_operand (array_ref_low_bound (t1),
-			array_ref_low_bound (t2)))
-	return return_false_with_msg ("");
-  if (!compare_operand (array_ref_element_size (t1),
-			array_ref_element_size (t2)))
-	return return_false_with_msg ("");
-
-  if (!compare_operand (x1, x2))
-	return return_false_with_msg ("");
-  return compare_operand (y1, y2);
-case MEM_REF:
-  {
-	x1 = TREE_OPERAND (t1, 0);
-	x2 = TREE_OPERAND (t2, 0);
-	y1 = TREE_OPERAND (t1, 1);
-	y2 = TREE_OPERAND (t2, 1);
-
-	/* See if operand is an memory access (the test originate from
-	 gimple_load_p).
-
-	In this case the alias set of the function being replaced must
-	be subset of the alias set of the other function.  At the moment
-	we seek for equivalency classes, so simply require inclussion in
-	both directions.  */
 
-	if (!func_checker::com

[PATCH 1/9] Replace int with boolean in predicate functions.

[Martin Liska]

gcc/ChangeLog:

2019-07-24  Martin Liska  

* fold-const.c (twoval_comparison_p): Replace int
with bool as a return type.
(simple_operand_p): Likewise.
(operand_equal_p): Replace int with bool as a return type.
* fold-const.h (operand_equal_p): Likewise.
---
 gcc/fold-const.c | 148 +++
 gcc/fold-const.h |   2 +-
 2 files changed, 75 insertions(+), 75 deletions(-)

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 716d7397b49..0bd68b5e2d4 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -115,11 +115,11 @@ static tree negate_expr (tree);
 static tree associate_trees (location_t, tree, tree, enum tree_code, tree);
 static enum comparison_code comparison_to_compcode (enum tree_code);
 static enum tree_code compcode_to_comparison (enum comparison_code);
-static int twoval_comparison_p (tree, tree *, tree *);
+static bool twoval_comparison_p (tree, tree *, tree *);
 static tree eval_subst (location_t, tree, tree, tree, tree, tree);
 static tree optimize_bit_field_compare (location_t, enum tree_code,
 	tree, tree, tree);
-static int simple_operand_p (const_tree);
+static bool simple_operand_p (const_tree);
 static bool simple_operand_p_2 (tree);
 static tree range_binop (enum tree_code, tree, tree, int, tree, int);
 static tree range_predecessor (tree);
@@ -2939,7 +2939,7 @@ combine_comparisons (location_t loc,
addresses with TREE_CONSTANT flag set so we know that &var == &var
even if var is volatile.  */
 
-int
+bool
 operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
 {
   /* When checking, verify at the outermost operand_equal_p call that
@@ -2958,10 +2958,10 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
 	  hashval_t h1 = hstate1.end ();
 	  gcc_assert (h0 == h1);
 	}
-	  return 1;
+	  return true;
 	}
   else
-	return 0;
+	return false;
 }
 
   STRIP_ANY_LOCATION_WRAPPER (arg0);
@@ -2971,19 +2971,19 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
   if (TREE_CODE (arg0) == ERROR_MARK || TREE_CODE (arg1) == ERROR_MARK
   || TREE_TYPE (arg0) == error_mark_node
   || TREE_TYPE (arg1) == error_mark_node)
-return 0;
+return false;
 
   /* Similar, if either does not have a type (like a template id),
  they aren't equal.  */
   if (!TREE_TYPE (arg0) || !TREE_TYPE (arg1))
-return 0;
+return false;
 
   /* We cannot consider pointers to different address space equal.  */
   if (POINTER_TYPE_P (TREE_TYPE (arg0))
   && POINTER_TYPE_P (TREE_TYPE (arg1))
   && (TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg0)))
 	  != TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg1)
-return 0;
+return false;
 
   /* Check equality of integer constants before bailing out due to
  precision differences.  */
@@ -3005,13 +3005,13 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
   if (TYPE_UNSIGNED (TREE_TYPE (arg0)) != TYPE_UNSIGNED (TREE_TYPE (arg1))
 	  || POINTER_TYPE_P (TREE_TYPE (arg0))
 			 != POINTER_TYPE_P (TREE_TYPE (arg1)))
-	return 0;
+	return false;
 
   /* If both types don't have the same precision, then it is not safe
 	 to strip NOPs.  */
   if (element_precision (TREE_TYPE (arg0))
 	  != element_precision (TREE_TYPE (arg1)))
-	return 0;
+	return false;
 
   STRIP_NOPS (arg0);
   STRIP_NOPS (arg1);
@@ -3058,17 +3058,17 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
 	  && TREE_CODE (TREE_OPERAND (arg0, 0)) == ADDR_EXPR
 	  && TREE_OPERAND (TREE_OPERAND (arg0, 0), 0) == arg1
 	  && integer_zerop (TREE_OPERAND (arg0, 1)))
-	return 1;
+	return true;
 	  else if (TREE_CODE (arg1) == MEM_REF
 		   && DECL_P (arg0)
 		   && TREE_CODE (TREE_OPERAND (arg1, 0)) == ADDR_EXPR
 		   && TREE_OPERAND (TREE_OPERAND (arg1, 0), 0) == arg0
 		   && integer_zerop (TREE_OPERAND (arg1, 1)))
-	return 1;
-	  return 0;
+	return true;
+	  return false;
 	}
   else
-	return 0;
+	return false;
 }
 
   /* When not checking adddresses, this is needed for conversions and for
@@ -3077,7 +3077,7 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
   || TREE_CODE (TREE_TYPE (arg1)) == ERROR_MARK
   || (TYPE_MODE (TREE_TYPE (arg0)) != TYPE_MODE (TREE_TYPE (arg1))
 	  && !(flags & OEP_ADDRESS_OF)))
-return 0;
+return false;
 
   /* If ARG0 and ARG1 are the same SAVE_EXPR, they are necessarily equal.
  We don't care about side effects in that case because the SAVE_EXPR
@@ -3092,7 +3092,7 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
   && (TREE_CODE (arg0) == SAVE_EXPR
 	  || (flags & OEP_MATCH_SIDE_EFFECTS)
 	  || (! TREE_SIDE_EFFECTS (arg0) && ! TREE_SIDE_EFFECTS (arg1
-return 1;
+return true;
 
   /* Next handle constant cases, those for which we can return 1 even
  if ONLY_CONST is set.  */
@@ -3108,7 +3108,7 @@ op

[PATCH 5/9] Come up with an abstraction.

[Martin Liska]

gcc/ChangeLog:

2019-07-24  Martin Liska  

* fold-const.c (operand_equal_p): Rename to ...
(operand_compare::operand_equal_p): ... this.
(add_expr):  Rename to ...
(operand_compare::hash_operand): ... this.
(operand_compare::operand_equal_valueize): Likewise.
(operand_compare::hash_operand_valueize): Likewise.
* fold-const.h (operand_equal_p): Set default
value for last argument.
(class operand_compare): New.
* tree.c (add_expr): Move content to hash_operand.
---
 gcc/fold-const.c | 346 ++-
 gcc/fold-const.h |  30 +++-
 gcc/tree.c   | 286 ---
 3 files changed, 372 insertions(+), 290 deletions(-)

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 4bcde22ada7..087c450cace 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -2940,7 +2940,8 @@ combine_comparisons (location_t loc,
even if var is volatile.  */
 
 bool
-operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
+operand_compare::operand_equal_p (const_tree arg0, const_tree arg1,
+  unsigned int flags)
 {
   /* When checking, verify at the outermost operand_equal_p call that
  if operand_equal_p returns non-zero then ARG0 and ARG1 has the same
@@ -2952,8 +2953,8 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
 	  if (arg0 != arg1)
 	{
 	  inchash::hash hstate0 (0), hstate1 (0);
-	  inchash::add_expr (arg0, hstate0, flags | OEP_HASH_CHECK);
-	  inchash::add_expr (arg1, hstate1, flags | OEP_HASH_CHECK);
+	  hash_operand (arg0, hstate0, flags | OEP_HASH_CHECK);
+	  hash_operand (arg1, hstate1, flags | OEP_HASH_CHECK);
 	  hashval_t h0 = hstate0.end ();
 	  hashval_t h1 = hstate1.end ();
 	  gcc_assert (h0 == h1);
@@ -3094,6 +3095,12 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
 	  || (! TREE_SIDE_EFFECTS (arg0) && ! TREE_SIDE_EFFECTS (arg1
 return true;
 
+  int val = operand_equal_valueize (arg0, arg1, flags);
+  if (val == 1)
+return 1;
+  if (val == 0)
+return 0;
+
   /* Next handle constant cases, those for which we can return 1 even
  if ONLY_CONST is set.  */
   if (TREE_CONSTANT (arg0) && TREE_CONSTANT (arg1))
@@ -3605,6 +3612,339 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
 
 #undef OP_SAME
 #undef OP_SAME_WITH_NULL
+}
+
+/* Generate a hash value for an expression.  This can be used iteratively
+   by passing a previous result as the HSTATE argument.  */
+
+void
+operand_compare::hash_operand (const_tree t, inchash::hash &hstate,
+			   unsigned int flags)
+{
+  int i;
+  enum tree_code code;
+  enum tree_code_class tclass;
+
+  if (t == NULL_TREE || t == error_mark_node)
+{
+  hstate.merge_hash (0);
+  return;
+}
+
+  STRIP_ANY_LOCATION_WRAPPER (t);
+
+  if (!(flags & OEP_ADDRESS_OF))
+STRIP_NOPS (t);
+
+  code = TREE_CODE (t);
+
+  bool ret = hash_operand_valueize (t, hstate, flags);
+  if (ret)
+return;
+
+  switch (code)
+{
+/* Alas, constants aren't shared, so we can't rely on pointer
+   identity.  */
+case VOID_CST:
+  hstate.merge_hash (0);
+  return;
+case INTEGER_CST:
+  gcc_checking_assert (!(flags & OEP_ADDRESS_OF));
+  for (i = 0; i < TREE_INT_CST_EXT_NUNITS (t); i++)
+	hstate.add_hwi (TREE_INT_CST_ELT (t, i));
+  return;
+case REAL_CST:
+  {
+	unsigned int val2;
+	if (!HONOR_SIGNED_ZEROS (t) && real_zerop (t))
+	  val2 = rvc_zero;
+	else
+	  val2 = real_hash (TREE_REAL_CST_PTR (t));
+	hstate.merge_hash (val2);
+	return;
+  }
+case FIXED_CST:
+  {
+	unsigned int val2 = fixed_hash (TREE_FIXED_CST_PTR (t));
+	hstate.merge_hash (val2);
+	return;
+  }
+case STRING_CST:
+  hstate.add ((const void *) TREE_STRING_POINTER (t),
+		  TREE_STRING_LENGTH (t));
+  return;
+case COMPLEX_CST:
+  hash_operand (TREE_REALPART (t), hstate, flags);
+  hash_operand (TREE_IMAGPART (t), hstate, flags);
+  return;
+case VECTOR_CST:
+  {
+	hstate.add_int (VECTOR_CST_NPATTERNS (t));
+	hstate.add_int (VECTOR_CST_NELTS_PER_PATTERN (t));
+	unsigned int count = vector_cst_encoded_nelts (t);
+	for (unsigned int i = 0; i < count; ++i)
+	  hash_operand (VECTOR_CST_ENCODED_ELT (t, i), hstate, flags);
+	return;
+  }
+case SSA_NAME:
+  /* We can just compare by pointer.  */
+  hstate.add_hwi (SSA_NAME_VERSION (t));
+  return;
+case PLACEHOLDER_EXPR:
+  /* The node itself doesn't matter.  */
+  return;
+case BLOCK:
+case OMP_CLAUSE:
+  /* Ignore.  */
+  return;
+case TREE_LIST:
+  /* A list of expressions, for a CALL_EXPR or as the elements of a
+	 VECTOR_CST.  */
+  for (; t; t = TREE_CHAIN (t))
+	hash_operand (TREE_VALUE (t), hstate, flags);
+  return;
+case CONSTRUCTOR:
+  {
+	unsigned HOST_WIDE_INT idx;
+	tree field, value;
+	flags 

[PATCH 9/9] Remove alias set comparison.

[Martin Liska]

gcc/ChangeLog:

2019-07-24  Martin Liska  

* ipa-icf-gimple.c (func_checker::compatible_types_p):
Do not compare alias sets.  It's handled by operand_equal_p.

gcc/testsuite/ChangeLog:

2019-07-24  Martin Liska  

* c-c++-common/Wstringop-truncation-4.c: Disable IPA ICF.
* gcc.dg/tree-ssa/pr64910-2.c: Likewise.
* gcc.dg/tree-ssa/pr79352.c: Likewise.
* gcc.dg/ipa/ipa-icf-40.c: New test.
---
 gcc/ipa-icf-gimple.c  | 12 ---
 .../c-c++-common/Wstringop-truncation-4.c |  2 +-
 gcc/testsuite/gcc.dg/ipa/ipa-icf-40.c | 32 +++
 gcc/testsuite/gcc.dg/tree-ssa/pr64910-2.c |  2 +-
 gcc/testsuite/gcc.dg/tree-ssa/pr79352.c   |  2 +-
 5 files changed, 35 insertions(+), 15 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-icf-40.c

diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
index 375aadad412..751d5859706 100644
--- a/gcc/ipa-icf-gimple.c
+++ b/gcc/ipa-icf-gimple.c
@@ -31,7 +31,6 @@ along with GCC; see the file COPYING3.  If not see
 #include "cgraph.h"
 #include "data-streamer.h"
 #include "gimple-pretty-print.h"
-#include "alias.h"
 #include "fold-const.h"
 #include "gimple-iterator.h"
 #include "ipa-utils.h"
@@ -209,17 +208,6 @@ func_checker::compatible_types_p (tree t1, tree t2)
   if (!types_compatible_p (t1, t2))
 return return_false_with_msg ("types are not compatible");
 
-  /* We do a lot of unnecesary matching of types that are not being
- accessed and thus do not need to be compatible.  In longer term we should
- remove these checks on all types which are not accessed as memory
- locations.
-
- For time being just avoid calling get_alias_set on types that are not
- having alias sets defined at all.  */
-  if (type_with_alias_set_p (t1) && type_with_alias_set_p (t2)
-  && get_alias_set (t1) != get_alias_set (t2))
-return return_false_with_msg ("alias sets are different");
-
   return true;
 }
 
diff --git a/gcc/testsuite/c-c++-common/Wstringop-truncation-4.c b/gcc/testsuite/c-c++-common/Wstringop-truncation-4.c
index c76f2823daf..15209536add 100644
--- a/gcc/testsuite/c-c++-common/Wstringop-truncation-4.c
+++ b/gcc/testsuite/c-c++-common/Wstringop-truncation-4.c
@@ -3,7 +3,7 @@
Verify that -Wstringop-truncation is issued for uses of arrays and
pointers to qualified forms of characters of all three types.
{ dg-do compile }
-   { dg-options "-O2 -Wall -Wstringop-truncation" } */
+   { dg-options "-O2 -Wall -Wstringop-truncation -fno-ipa-icf" } */
 
 #if __cplusplus
 extern "C"
diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-icf-40.c b/gcc/testsuite/gcc.dg/ipa/ipa-icf-40.c
new file mode 100644
index 000..8d512cbc7d3
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-40.c
@@ -0,0 +1,32 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-ipa-icf"  } */
+
+struct A { int i; char a1[10]; };
+struct B { int i; char a3[30]; };
+struct C { int i; char ax[]; };
+
+static int
+__attribute__((noinline))
+test_array_1 (int i, struct A *a)
+{
+  return __builtin_printf ("%-s\n", a->a1);
+}
+
+static int
+__attribute__((noinline))
+test_array_3 (int i, struct B *b)
+{
+  return __builtin_printf ("%-s\n", b->a3);
+}
+
+struct A a = { 0, "foo" };
+struct B b = { 0, "bar" };
+
+int main()
+{
+  test_array_1 (0, &a);
+  test_array_3 (0, &b);
+  return 0;
+}
+
+/* { dg-final { scan-ipa-dump "Equal symbols: 1" "icf"  } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr64910-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr64910-2.c
index 2e3d6790776..812bfa48825 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr64910-2.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr64910-2.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-reassoc1" } */
+/* { dg-options "-O2 -fdump-tree-reassoc1 -fno-ipa-icf" } */
 
 /* We want to make sure that we reassociate in a way that has the
constant last.  With the constant last, it's more likely to result
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr79352.c b/gcc/testsuite/gcc.dg/tree-ssa/pr79352.c
index 485e2d64cb3..36e195c3a06 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr79352.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr79352.c
@@ -1,7 +1,7 @@
 /* PR tree-optimization/79352 - -fprintf-return-value doesn't handle
flexible-like array members properly
{ dg-do compile }
-   { dg-options "-O2 -fdump-tree-optimized" } */
+   { dg-options "-O2 -fdump-tree-optimized -fno-ipa-icf" } */
 
 struct A { int i; char a1[1]; };
 struct B { int i; char a3[3]; };

[PATCH 7/9] IPA ICF: remove dead code

[Martin Liska]

gcc/ChangeLog:

2019-07-24  Martin Liska  

* ipa-icf-gimple.c (func_checker::compare_ssa_name): Call
compare_operand.
(func_checker::compare_memory_operand): Remove.
(func_checker::compare_cst_or_decl): Remove.
(func_checker::operand_equal_valueize): Do not handle
FIELD_DECL.
(func_checker::compare_gimple_call): Call compare_operand.
(func_checker::compare_gimple_assign): Likewise.
* ipa-icf-gimple.h: Remove compare_cst_or_decl.
* ipa-icf.c (sem_function::icf_handled_component_p): Remove.
* ipa-icf.h (icf_handled_component_p): Remove.
---
 gcc/ipa-icf-gimple.c | 150 ++-
 gcc/ipa-icf-gimple.h |   4 --
 gcc/ipa-icf.c|  11 
 gcc/ipa-icf.h|   3 -
 4 files changed, 6 insertions(+), 162 deletions(-)

diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
index 8448d387428..2d4c5d22534 100644
--- a/gcc/ipa-icf-gimple.c
+++ b/gcc/ipa-icf-gimple.c
@@ -111,13 +111,7 @@ func_checker::compare_ssa_name (tree t1, tree t2)
   tree b1 = SSA_NAME_VAR (t1);
   tree b2 = SSA_NAME_VAR (t2);
 
-  if (b1 == NULL && b2 == NULL)
-	return true;
-
-  if (b1 == NULL || b2 == NULL || TREE_CODE (b1) != TREE_CODE (b2))
-	return return_false ();
-
-  return compare_cst_or_decl (b1, b2);
+  return compare_operand (b1, b2);
 }
 
   return true;
@@ -247,86 +241,12 @@ func_checker::compatible_types_p (tree t1, tree t2)
   return true;
 }
 
-/* Function compare for equality given memory operands T1 and T2.  */
-
-bool
-func_checker::compare_memory_operand (tree t1, tree t2)
-{
-  if (!t1 && !t2)
-return true;
-  else if (!t1 || !t2)
-return false;
-
-  ao_ref r1, r2;
-  ao_ref_init (&r1, t1);
-  ao_ref_init (&r2, t2);
-
-  tree b1 = ao_ref_base (&r1);
-  tree b2 = ao_ref_base (&r2);
-
-  bool source_is_memop = DECL_P (b1) || INDIRECT_REF_P (b1)
-			 || TREE_CODE (b1) == MEM_REF
-			 || TREE_CODE (b1) == TARGET_MEM_REF;
-
-  bool target_is_memop = DECL_P (b2) || INDIRECT_REF_P (b2)
-			 || TREE_CODE (b2) == MEM_REF
-			 || TREE_CODE (b2) == TARGET_MEM_REF;
-
-  /* Compare alias sets for memory operands.  */
-  if (source_is_memop && target_is_memop)
-{
-  if (TREE_THIS_VOLATILE (t1) != TREE_THIS_VOLATILE (t2))
-	return return_false_with_msg ("different operand volatility");
-
-  if (ao_ref_alias_set (&r1) != ao_ref_alias_set (&r2)
-	  || ao_ref_base_alias_set (&r1) != ao_ref_base_alias_set (&r2))
-	return return_false_with_msg ("ao alias sets are different");
-
-  /* We can't simply use get_object_alignment_1 on the full
- reference as for accesses with variable indexes this reports
-	 too conservative alignment.  We also can't use the ao_ref_base
-	 base objects as ao_ref_base happily strips MEM_REFs around
-	 decls even though that may carry alignment info.  */
-  b1 = t1;
-  while (handled_component_p (b1))
-	b1 = TREE_OPERAND (b1, 0);
-  b2 = t2;
-  while (handled_component_p (b2))
-	b2 = TREE_OPERAND (b2, 0);
-  unsigned int align1, align2;
-  unsigned HOST_WIDE_INT tem;
-  get_object_alignment_1 (b1, &align1, &tem);
-  get_object_alignment_1 (b2, &align2, &tem);
-  if (align1 != align2)
-	return return_false_with_msg ("different access alignment");
-
-  /* Similarly we have to compare dependence info where equality
- tells us we are safe (even some unequal values would be safe
-	 but then we have to maintain a map of bases and cliques).  */
-  unsigned short clique1 = 0, base1 = 0, clique2 = 0, base2 = 0;
-  if (TREE_CODE (b1) == MEM_REF)
-	{
-	  clique1 = MR_DEPENDENCE_CLIQUE (b1);
-	  base1 = MR_DEPENDENCE_BASE (b1);
-	}
-  if (TREE_CODE (b2) == MEM_REF)
-	{
-	  clique2 = MR_DEPENDENCE_CLIQUE (b2);
-	  base2 = MR_DEPENDENCE_BASE (b2);
-	}
-  if (clique1 != clique2 || base1 != base2)
-	return return_false_with_msg ("different dependence info");
-}
-
-  return compare_operand (t1, t2);
-}
-
 /* Function compare for equality given trees T1 and T2 which
can be either a constant or a declaration type.  */
 
 bool
-func_checker::hash_operand_valueize (const_tree arg, inchash::hash &hstate,
- unsigned int flags)
+func_checker::hash_operand_valueize (const_tree arg, inchash::hash &,
+ unsigned int)
 {
   switch (TREE_CODE (arg))
 {
@@ -346,52 +266,6 @@ func_checker::hash_operand_valueize (const_tree arg, inchash::hash &hstate,
   return false;
 }
 
-bool
-func_checker::compare_cst_or_decl (tree t1, tree t2)
-{
-  bool ret;
-
-  switch (TREE_CODE (t1))
-{
-case INTEGER_CST:
-case COMPLEX_CST:
-case VECTOR_CST:
-case STRING_CST:
-case REAL_CST:
-  {
-	ret = compatible_types_p (TREE_TYPE (t1), TREE_TYPE (t2))
-	  && operand_equal_p (t1, t2, OEP_ONLY_CONST);
-	return return_with_debug (ret);
-  }
-case FUNCTION_DECL:
-  /* All function decls are in the symbol table and known to match
-	 before we start co

[PATCH 8/9] Remove comparison for polymorphic types.

[Martin Liska]

gcc/ChangeLog:

2019-07-24  Martin Liska  

* ipa-icf-gimple.c (func_checker::func_checker): Do not
initialize m_compare_polymorphic.
(func_checker::compare_decl): Do not compare polymorphic types.
* ipa-icf-gimple.h (m_compare_polymorphic): Remove.
* ipa-icf.c (sem_function::equals_private): Do not call
compare_polymorphic_p.
---
 gcc/ipa-icf-gimple.c | 18 --
 gcc/ipa-icf-gimple.h |  4 
 gcc/ipa-icf.c|  1 -
 3 files changed, 23 deletions(-)

diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
index 2d4c5d22534..375aadad412 100644
--- a/gcc/ipa-icf-gimple.c
+++ b/gcc/ipa-icf-gimple.c
@@ -51,14 +51,12 @@ namespace ipa_icf_gimple {
of declarations that can be skipped.  */
 
 func_checker::func_checker (tree source_func_decl, tree target_func_decl,
-			bool compare_polymorphic,
 			bool ignore_labels,
 			hash_set *ignored_source_nodes,
 			hash_set *ignored_target_nodes)
   : m_source_func_decl (source_func_decl), m_target_func_decl (target_func_decl),
 m_ignored_source_nodes (ignored_source_nodes),
 m_ignored_target_nodes (ignored_target_nodes),
-m_compare_polymorphic (compare_polymorphic),
 m_ignore_labels (ignore_labels)
 {
   function *source_func = DECL_STRUCT_FUNCTION (source_func_decl);
@@ -156,23 +154,7 @@ func_checker::compare_decl (tree t1, tree t2)
   if (!compatible_types_p (TREE_TYPE (t1), TREE_TYPE (t2)))
 return return_false ();
 
-  /* TODO: we are actually too strict here.  We only need to compare if
- T1 can be used in polymorphic call.  */
-  if (TREE_ADDRESSABLE (t1)
-  && m_compare_polymorphic
-  && !compatible_polymorphic_types_p (TREE_TYPE (t1), TREE_TYPE (t2),
-	  false))
-return return_false ();
-
-  if ((t == VAR_DECL || t == PARM_DECL || t == RESULT_DECL)
-  && DECL_BY_REFERENCE (t1)
-  && m_compare_polymorphic
-  && !compatible_polymorphic_types_p (TREE_TYPE (t1), TREE_TYPE (t2),
-	  true))
-return return_false ();
-
   bool existed_p;
-
   tree &slot = m_decl_map.get_or_insert (t1, &existed_p);
   if (existed_p)
 return return_with_debug (slot == t2);
diff --git a/gcc/ipa-icf-gimple.h b/gcc/ipa-icf-gimple.h
index b760b0fdce3..75f2f24ff4a 100644
--- a/gcc/ipa-icf-gimple.h
+++ b/gcc/ipa-icf-gimple.h
@@ -128,7 +128,6 @@ public:
  Similarly, IGNORE_SOURCE_DECLS and IGNORE_TARGET_DECLS are sets
  of declarations that can be skipped.  */
   func_checker (tree source_func_decl, tree target_func_decl,
-		bool compare_polymorphic,
 		bool ignore_labels = false,
 		hash_set *ignored_source_nodes = NULL,
 		hash_set *ignored_target_nodes = NULL);
@@ -258,9 +257,6 @@ private:
   /* Label to basic block index mapping.  */
   hash_map  m_label_bb_map;
 
-  /* Flag if polymorphic comparison should be executed.  */
-  bool m_compare_polymorphic;
-
   /* Flag if ignore labels in comparison.  */
   bool m_ignore_labels;
 
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index 7cac480930b..f6400d48e27 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -845,7 +845,6 @@ sem_function::equals_private (sem_item *item)
 return return_false ();
 
   m_checker = new func_checker (decl, m_compared_func->decl,
-compare_polymorphic_p (),
 false,
 &refs_set,
 &m_compared_func->refs_set);

[PATCH 2/9] operand_equal_p: add support for FIELD_DECL

[Martin Liska]

gcc/ChangeLog:

2019-07-24  Martin Liska  

* fold-const.c (operand_equal_p): Support FIELD_DECL
as well.
* tree.c (add_expr): Hast DECL_FIELD_OFFSET and
DECL_FIELD_BIT_OFFSET for FIELD_DECL.

gcc/testsuite/ChangeLog:

2019-07-24  Martin Liska  

* gcc.dg/vect/vect-35-big-array.c: Vectorize one more loop.
* gcc.dg/vect/vect-35.c: Likewise.
* gcc.dg/pr70740.c: Move from torture and set -O2.
* gfortran.dg/vect/vect-8.f90: Update scanned pattern.
---
 gcc/fold-const.c  | 34 ---
 gcc/testsuite/gcc.dg/{torture => }/pr70740.c  |  3 +-
 gcc/testsuite/gcc.dg/vect/vect-35-big-array.c |  3 +-
 gcc/testsuite/gcc.dg/vect/vect-35.c   |  3 +-
 gcc/testsuite/gfortran.dg/vect/vect-8.f90 |  2 +-
 gcc/tree.c|  4 +++
 6 files changed, 38 insertions(+), 11 deletions(-)
 rename gcc/testsuite/gcc.dg/{torture => }/pr70740.c (77%)

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 0bd68b5e2d4..52414f7729e 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -3462,11 +3462,35 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
 	}
 
 case tcc_declaration:
-  /* Consider __builtin_sqrt equal to sqrt.  */
-  return (TREE_CODE (arg0) == FUNCTION_DECL
-	  && fndecl_built_in_p (arg0) && fndecl_built_in_p (arg1)
-	  && DECL_BUILT_IN_CLASS (arg0) == DECL_BUILT_IN_CLASS (arg1)
-	  && DECL_FUNCTION_CODE (arg0) == DECL_FUNCTION_CODE (arg1));
+  switch (TREE_CODE (arg0))
+	{
+	case FUNCTION_DECL:
+	  /* Consider __builtin_sqrt equal to sqrt.  */
+	  return (fndecl_built_in_p (arg0) && fndecl_built_in_p (arg1)
+		  && DECL_BUILT_IN_CLASS (arg0) == DECL_BUILT_IN_CLASS (arg1)
+		  && DECL_FUNCTION_CODE (arg0) == DECL_FUNCTION_CODE (arg1));
+	case FIELD_DECL:
+	  {
+	tree fo0 = DECL_FIELD_OFFSET (arg0);
+	tree fo1 = DECL_FIELD_OFFSET (arg1);
+	if (fo0 != NULL && fo1 != NULL
+		&& !operand_equal_p (fo0, fo1, OEP_ONLY_CONST))
+	  return false;
+	else if (fo0 != fo1)
+	  return false;
+
+	tree fbo0 = DECL_FIELD_BIT_OFFSET (arg0);
+	tree fbo1 = DECL_FIELD_BIT_OFFSET (arg1);
+	if (fbo0 != NULL && fbo1 != NULL
+		&& !operand_equal_p (fbo0, fbo1, OEP_ONLY_CONST))
+	  return false;
+	else if (fbo0 != fbo1)
+	  return false;
+	return true;
+	  }
+	default:
+	  return false;
+	}
 
 case tcc_exceptional:
   if (TREE_CODE (arg0) == CONSTRUCTOR)
diff --git a/gcc/testsuite/gcc.dg/torture/pr70740.c b/gcc/testsuite/gcc.dg/pr70740.c
similarity index 77%
rename from gcc/testsuite/gcc.dg/torture/pr70740.c
rename to gcc/testsuite/gcc.dg/pr70740.c
index 5bf8e4adc91..186da1b2637 100644
--- a/gcc/testsuite/gcc.dg/torture/pr70740.c
+++ b/gcc/testsuite/gcc.dg/pr70740.c
@@ -1,4 +1,5 @@
 /* { dg-do compile } */
+/* { dg-options "-O2" } */
 
 extern int foo (void);
 extern void *memcpy (void *, const void *, __SIZE_TYPE__);
@@ -32,7 +33,7 @@ baz ()
 e = c.a3;
   else
 e = c.a1;
-  memcpy (d.a, e, 6);
+  memcpy (d.a, e, 6); /* { dg-warning "reading 5 bytes from a region of size 0" } */
   f = bar ();
   memcpy (d.a, f, 1);
 }
diff --git a/gcc/testsuite/gcc.dg/vect/vect-35-big-array.c b/gcc/testsuite/gcc.dg/vect/vect-35-big-array.c
index ca57a10f714..fa356c2c4a2 100644
--- a/gcc/testsuite/gcc.dg/vect/vect-35-big-array.c
+++ b/gcc/testsuite/gcc.dg/vect/vect-35-big-array.c
@@ -47,5 +47,4 @@ int main (void)
 }
 
 
-/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect"  { xfail { ia64-*-* sparc*-*-* } } } } */
-/* { dg-final { scan-tree-dump "can't determine dependence between" "vect" } } */
+/* { dg-final { scan-tree-dump-times "vectorized 2 loops" 1 "vect"  { xfail { ia64-*-* sparc*-*-* } } } } */
diff --git a/gcc/testsuite/gcc.dg/vect/vect-35.c b/gcc/testsuite/gcc.dg/vect/vect-35.c
index 76fe32d68ad..3023c8c714f 100644
--- a/gcc/testsuite/gcc.dg/vect/vect-35.c
+++ b/gcc/testsuite/gcc.dg/vect/vect-35.c
@@ -47,5 +47,4 @@ int main (void)
 } 
 
 
-/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect"  { xfail { ia64-*-* sparc*-*-* } } } } */
-/* { dg-final { scan-tree-dump "can't determine dependence between" "vect" } } */
+/* { dg-final { scan-tree-dump-times "vectorized 2 loops" 1 "vect"  { xfail { ia64-*-* sparc*-*-* } } } } */
diff --git a/gcc/testsuite/gfortran.dg/vect/vect-8.f90 b/gcc/testsuite/gfortran.dg/vect/vect-8.f90
index e26cdf95e51..f83f0d0aa27 100644
--- a/gcc/testsuite/gfortran.dg/vect/vect-8.f90
+++ b/gcc/testsuite/gfortran.dg/vect/vect-8.f90
@@ -704,5 +704,5 @@ CALL track('KERNEL  ')
 RETURN
 END SUBROUTINE kernel
 
-! { dg-final { scan-tree-dump-times "vectorized 22 loops" 1 "vect" { target vect_intdouble_cvt } } }
+! { dg-final { scan-tree-dump-times "vectorized 23 loops" 1 "vect" { target vect_intdouble_cvt } } }
 ! { dg-final { scan-tree-dump-times "vectorized 17 loops" 1 "vect" { target { ! vect_intdouble_cvt } } } }
diff --git a/gcc/tree.c b/gcc/tree.c

[PATCH 0/9] IPA ICF overhaul

[Martin Liska]

Hi.

It's some time I implemented first version of IPA ICF pass. Since that
I experienced more with the GCC internals and now is the right time
to do an overhaul.

Main motivation of changes is to share as many as possible in between
current operand_equal_p and ipa_icf::compare_operand. That's achieved
by a new class operand_compare. That allows use to share and unify
a lot of code. Apart from that I would like to learn current operand_equal_p
to handle new tree types.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests. I also
built Firefox, Godot engine.

Thanks,
Martin

Martin Liska (9):
  Replace int with boolean in predicate functions.
  operand_equal_p: add support for FIELD_DECL
  operand_equal_p: add support for OBJ_TYPE_REF.
  Strengthen alias_ptr_types_compatible_p in LTO mode.
  Come up with an abstraction.
  Integrate that for IPA ICF.
  IPA ICF: remove dead code
  Remove comparison for polymorphic types.
  Remove alias set comparison.

 gcc/alias.c   |   7 +-
 gcc/fold-const.c  | 547 +++---
 gcc/fold-const.h  |  30 +-
 gcc/ipa-icf-gimple.c  | 328 ++-
 gcc/ipa-icf-gimple.h  |  16 +-
 gcc/ipa-icf.c |  19 +-
 gcc/ipa-icf.h |   3 -
 .../c-c++-common/Wstringop-truncation-4.c |   2 +-
 gcc/testsuite/gcc.dg/ipa/ipa-icf-40.c |  32 +
 gcc/testsuite/gcc.dg/{torture => }/pr70740.c  |   3 +-
 gcc/testsuite/gcc.dg/tree-ssa/pr64910-2.c |   2 +-
 gcc/testsuite/gcc.dg/tree-ssa/pr79352.c   |   2 +-
 gcc/testsuite/gcc.dg/vect/vect-35-big-array.c |   3 +-
 gcc/testsuite/gcc.dg/vect/vect-35.c   |   3 +-
 gcc/testsuite/gfortran.dg/vect/vect-8.f90 |   2 +-
 gcc/tree.c| 273 -
 16 files changed, 588 insertions(+), 684 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-icf-40.c
 rename gcc/testsuite/gcc.dg/{torture => }/pr70740.c (77%)

-- 
2.22.0

[PATCH 4/9] Strengthen alias_ptr_types_compatible_p in LTO mode.

[Martin Liska]

gcc/ChangeLog:

2019-07-24  Martin Liska  

* alias.c (alias_ptr_types_compatible_p): Strengten
type comparison in LTO mode.
---
 gcc/alias.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/gcc/alias.c b/gcc/alias.c
index 2755df72907..bae4ddaebaf 100644
--- a/gcc/alias.c
+++ b/gcc/alias.c
@@ -793,8 +793,11 @@ alias_ptr_types_compatible_p (tree t1, tree t2)
   || ref_all_alias_ptr_type_p (t2))
 return false;
 
-  return (TYPE_MAIN_VARIANT (TREE_TYPE (t1))
-	  == TYPE_MAIN_VARIANT (TREE_TYPE (t2)));
+  if (in_lto_p)
+return get_alias_set (t1) == get_alias_set (t2);
+  else
+return (TYPE_MAIN_VARIANT (TREE_TYPE (t1))
+	== TYPE_MAIN_VARIANT (TREE_TYPE (t2)));
 }
 
 /* Create emptry alias set entry.  */

[PATCH 3/9] operand_equal_p: add support for OBJ_TYPE_REF.

[Martin Liska]

gcc/ChangeLog:

2019-07-24  Martin Liska  

* fold-const.c (operand_equal_p): Support OBJ_TYPE_REF.
* tree.c (add_expr): Hash parts of OBJ_TYPE_REF.
---
 gcc/fold-const.c | 21 +
 gcc/tree.c   |  9 +
 2 files changed, 30 insertions(+)

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 52414f7729e..4bcde22ada7 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -3323,6 +3323,27 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
 	  flags &= ~OEP_ADDRESS_OF;
 	  return OP_SAME (1) && OP_SAME (2);
 
+	/* Virtual table call.  */
+	case OBJ_TYPE_REF:
+	  {
+	if (!operand_equal_p (OBJ_TYPE_REF_EXPR (arg0),
+  OBJ_TYPE_REF_EXPR (arg1), flags))
+	  return false;
+	if (virtual_method_call_p (arg0))
+	  {
+		if (tree_to_uhwi (OBJ_TYPE_REF_TOKEN (arg0))
+		!= tree_to_uhwi (OBJ_TYPE_REF_TOKEN (arg1)))
+		  return false;
+		if (!types_same_for_odr (obj_type_ref_class (arg0),
+	 obj_type_ref_class (arg1)))
+		  return false;
+		if (!operand_equal_p (OBJ_TYPE_REF_OBJECT (arg0),
+  OBJ_TYPE_REF_OBJECT (arg1), flags))
+		  return false;
+	  }
+	return true;
+	  }
+
 	default:
 	  return false;
 	}
diff --git a/gcc/tree.c b/gcc/tree.c
index 91ebc9eddc4..2207f644fed 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -8011,6 +8011,15 @@ add_expr (const_tree t, inchash::hash &hstate, unsigned int flags)
 	  inchash::add_expr (TARGET_EXPR_SLOT (t), hstate, flags);
 	  return;
 
+	/* Virtual table call.  */
+	case OBJ_TYPE_REF:
+	  inchash::add_expr (OBJ_TYPE_REF_EXPR (t), hstate, flags);
+	  if (virtual_method_call_p (t))
+		{
+		  inchash::add_expr (OBJ_TYPE_REF_TOKEN (t), hstate, flags);
+		  inchash::add_expr (OBJ_TYPE_REF_OBJECT (t), hstate, flags);
+		}
+	  return;
 	default:
 	  break;
 	}

Re: [PATCH] Fix file descriptor existence of MinGW.

[Martin Sebor]

On 8/6/19 6:04 AM, Martin Liška wrote:

Hi.

The patch is about proper checking of file descriptors on Windows.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.


Is there a way to share the definition of the new function so
it doesn't have to be duplicated?

Other than that, I'm wondering if the guard for F_GETFD is
necessary and when (the original code didn't guard its use).

Martin




@Pekka: Can you please test it on Windows?

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2019-08-06  Martin Liska  

PR bootstrap/91352
* gcc.c (fd_exists): New.
(driver::detect_jobserver): Use fd_exists.
* lto-wrapper.c (fd_exists): New.
(jobserver_active_p): Use fd_exists.
---
  gcc/gcc.c | 19 +--
  gcc/lto-wrapper.c | 19 +--
  2 files changed, 34 insertions(+), 4 deletions(-)

Canonicalization of compares performed as side-effect operations

[Richard Earnshaw (lists)]

Arm has an instruction that performs the following operation:

(parallel [
(set (reg:CC 100 cc)
(compare:CC (const_int 0 [0])
(reg:SI 121)))
(set (reg:SI 113)
(neg:SI (reg:SI 121)))
])

This is simply a reverse subtract from the constant zero, and setting 
the condition flags.  It's the low part of a negdi2 expansion.


However, combine will rip this up and try to transform the compare into 
'canonical' form, ie


(parallel [
(set (reg:CC 100 cc)
(compare:CC (reg:SI 121)
(const_int 0 [0])))
(set (reg:SI 113)
(neg:SI (reg:SI 121)))
])

(and obviously swapping the condition on the instruction that uses the 
comparison result).


This, of course, doesn't match the behaviour of the instruction and 
no-longer matches the pattern in the md file.


SELECT_CC_MODE won't help me: the compare looks just like a simple 
comparison that the machine supports, so it can't return a different 
mode to allow me to fix things up.


I can't just stop the canonicalization: it's done in simplify_set and 
the wider context of the instruction simply isn't available.  If there 
is no wider context (ie the comparison is not part of a parallel), then 
the simplification is probably the right thing to do.


There are comments in combine.c (see for example just above the call to 
target_canonicalize_comparison in try_combine) that suggest that this 
sort of simplification should not be done, but it's clearly not being 
done consistently.


So is there a way to describe this instruction within the compiler, or a 
way to stop simplify_set from making this sort of simplification?  I was 
wondering if we might somehow annotate the compare (perhaps by setting 
the volatile bit) to indicate that the comparison was being done as a 
side-effect of another operation and that therefore swapping was not 
permitted.


R.

[PATCH] Detect not-cloned new/delete operators in DCE.

[Martin Liška]

On 8/6/19 2:42 PM, Martin Liška wrote:
> On 8/5/19 3:46 PM, Marc Glisse wrote:
>> On Mon, 5 Aug 2019, Martin Liška wrote:
>>
>>> You are right. It can really lead to confusion of the DCE.
>>>
>>> What we have is DECL_ABSTRACT_ORIGIN(decl) which we can use to indicate 
>>> operators
>>> that were somehow modified by an IPA optimization.
>>
>> Looks similar to the cgraph_node->clone_of that Richard was talking about. I 
>> have no idea which one would be best.
> 
> 
> Hm, strange that the ISRA clones don't have n->clone_of set. It's created 
> here:
> 
> #0  cgraph_node::create (decl= _ZN1AdlEPvd.isra.0>) at /home/marxin/Programming/gcc/gcc/profile-count.h:751
> #1  0x00bc8342 in cgraph_node::create_version_clone 
> (this=, 
> new_decl=, redirect_callers=..., bbs_to_copy=0x0, 
> suffix=0x1b74427 "isra") at 
> /home/marxin/Programming/gcc/gcc/cgraphclones.c:960
> #2  0x00bc9b9a in cgraph_node::create_version_clone_with_body 
> (this=this@entry=, 
> redirect_callers=redirect_callers@entry=..., tree_map=tree_map@entry=0x0, 
> args_to_skip=args_to_skip@entry=0x0, 
> skip_return=skip_return@entry=false, bbs_to_copy=bbs_to_copy@entry=0x0, 
> new_entry_block=, suffix=, 
> target_attributes=) at 
> /home/marxin/Programming/gcc/gcc/cgraphclones.c:1071
> #3  0x010e4611 in modify_function (adjustments=..., node= * 0x77208000 "operator delete"/64>) at 
> /home/marxin/Programming/gcc/gcc/tree-sra.c:5493
> #4  ipa_early_sra () at /home/marxin/Programming/gcc/gcc/tree-sra.c:5731
> #5  (anonymous namespace)::pass_early_ipa_sra::execute (this=) 
> at /home/marxin/Programming/gcc/gcc/tree-sra.c:5778
> 
> @Martin, @Honza: Why do we not set clone_of in this transformation?

If I'm correct cgraph_node::clone is used for inline clones only?

Anyway, I'm sending patch that considers only such new/delete operators
that are not a clone of an original type. That should make the current
DCE code more solid.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

> 
>>
>>> Do you believe it will be sufficient?
>>
>> In DCE only consider the operator delete that are not clones? (possibly the 
>> same for operator new? I don't know how much we can change the return value 
>> of a function in a clone) I guess that should work, and it wouldn't impact 
>> the common case with default, global operator new/delete.
> 
> I tent to limit that the only cgraph nodes that are not clones. I'm going to 
> prepare a patch for it.
> 
>>
>> An alternative would be to clear the DECL_IS_OPERATOR_DELETE flag when 
>> creating a clone (possibly only if it modified the first parameter?). There 
>> is probably not much information in the fact that a function that doesn't 
>> even take a pointer used to be an operator delete.
>>
>>
>> By the way, I just thought of something, now that we handle class-specific 
>> operator new/delete (but you could do the same with the global replacable 
>> ones as well):
>>
>> #include 
>> int count = 0;
>> struct A {
>>   __attribute__((malloc,noinline))
>>   static void* operator new(unsigned long sz){++count;return ::operator 
>> new(sz);}
>>   static void operator delete(void* ptr){--count;::operator delete(ptr);}
>> };
>> int main(){
>>   delete new A;
>>   printf("%d\n",count);
>> }
>>
>> If we do not inline anything, we can remove the pair and nothing touches 
>> count. If we inline both new and delete, we can then remove the inner pair 
>> instead, count increases and decreases, fine. If we inline only one of them, 
>> and DCE the mismatched pair new/delete, we get something inconsistent (count 
>> is -1).
>>
>> This seems to indicate we should check that the new and delete match 
>> somehow...
>>
> 

>From 94110a47f0c13424d2e39d8f14bcc896a6097bd3 Mon Sep 17 00:00:00 2001
From: Martin Liska 
Date: Tue, 6 Aug 2019 16:14:48 +0200
Subject: [PATCH] Detect not-cloned new/delete operators in DCE.

gcc/ChangeLog:

2019-08-06  Martin Liska  

	* gimple.c (gimple_call_operator_delete_p): Remove.
	* gimple.h (gimple_call_operator_delete_p): Likewise.
	* tree-ssa-dce.c (operator_new_candidate_p): New.
	(operator_delete_candidate_p): Likewise.
	(mark_stmt_if_obviously_necessary): Use operator_new_candidate_p
	and operator_delete_candidate_p in order to detect operators
	that are not not clones.
	(mark_all_reaching_defs_necessary_1): Likewise.
	(propagate_necessity): Likewise.
	(eliminate_unnecessary_stmts): Likewise.
---
 gcc/gimple.c   | 12 --
 gcc/gimple.h   |  1 -
 gcc/tree-ssa-dce.c | 59 ++
 3 files changed, 34 insertions(+), 38 deletions(-)

diff --git a/gcc/gimple.c b/gcc/gimple.c
index 633ef512a19..684b8831b4d 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -2707,18 +2707,6 @@ gimple_builtin_call_types_compatible_p (const gimple *stmt, tree fndecl)
   return true;
 }
 
-/* Return true when STMT is operator delete call.  */
-
-bool
-gimple_call_operator_delete_p (const gcall *stmt

Re: [PATCH 1/3] C++20 constexpr lib part 1/3

[Steve Ellcey]

Ed,

I have run into an ICE that I tracked down to this patch:

commit 02fefffe6b78c4c39169206aa40fb53f367ecba8
Author: emsr 
Date:   Thu Aug 1 15:25:42 2019 +

2019-08-01  Edward Smith-Rowland  <3dw...@verizon.net>

Implement C++20 p0202 - Add Constexpr Modifiers to Functions
in  and  Headers.
Implement C++20 p1023 - constexpr comparison operators for 
std::array.


Before I try to create a smaller test example (the current failure happens
when I build https://github.com/llnl/RAJAPerf.git) I was wondering if you
have already seen this ICE:

/extra/sellcey/raja-build-error/RAJAPerf/src/apps/WIP-COUPLE.cpp: In member 
function 'virtual void rajaperf::apps::COUPLE::runKernel(rajaperf::VariantID)':
/extra/sellcey/raja-build-error/RAJAPerf/src/apps/WIP-COUPLE.cpp:217:1: 
internal compiler error: Segmentation fault
  217 | } // end namespace rajaperf
  | ^
0xe81ddf crash_signal
../../gcc/gcc/toplev.c:326
0x968d14 lookup_page_table_entry
../../gcc/gcc/ggc-page.c:632
0x968d14 ggc_set_mark(void const*)
../../gcc/gcc/ggc-page.c:1531
0xbfeadf gt_ggc_mx_symtab_node(void*)
/extra/sellcey/gcc-tot/obj-gcc/gcc/gtype-desc.c:1302
0xd9d2a7 gt_ggc_ma_order
./gt-passes.h:31
0xd9d2a7 gt_ggc_ma_order
./gt-passes.h:26
0xb6f49f ggc_mark_root_tab
../../gcc/gcc/ggc-common.c:77
0xb6f6c3 ggc_mark_roots()
../../gcc/gcc/ggc-common.c:94
0x9696af ggc_collect()
../../gcc/gcc/ggc-page.c:2201
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See  for instructions.

Re: [PATCH]: Fix PR c++/88095, class template argument deduction for literal operator templates per P0732 for C++2a

[Jason Merrill]

On Tue, Aug 6, 2019 at 11:10 AM Tom Honermann  wrote:
> On 8/5/19 3:05 PM, Jason Merrill wrote:
> > On 8/2/19 9:59 AM, Tom Honermann wrote:
> >> This patch fixes PR c++/88095:
> >> - Bug 88095 - class nontype template parameter UDL string literals
> >> doesn't accepts deduction placeholder
> >> - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88095.
> >>
> >> It also addresses a latent issue; literal operator templates with
> >> template parameter packs of literal class type were previously
> >> accepted.   The patch corrects this and adds a test
> >> (udlit-class-nttp-neg.C).
> >>
> >> In the change to gcc/cp/parser.c, it is not clear to me whether the
> >> 'TREE_CODE (TREE_TYPE (parm)) == TEMPLATE_TYPE_PARM' comparison is
> >> necessary; it might be that 'CLASS_PLACEHOLDER_TEMPLATE' suffices on
> >> its own.
> >
> > template_placeholder_p would be a shorter way to write these, but I
> > think even better would be to just change CLASS_TYPE_P to
> > MAYBE_CLASS_TYPE_P.  I'll make that change and commit the patch, since
> > it looks like you don't have commit access yet.
> Thanks, and correct, I don't have commit access yet (and I'm not sure
> that I should! :) )
> >
> >> If accepted, I'd like to request this change be applied to gcc 9 as
> >> it is needed for one of the char8_t remediation approaches documented
> >> in P1423, and may be helpful for existing code bases impacted by the
> >> char8_t changes adopted via P0482 for C++20.
> >> - https://wg21.link/p1423#emulate
> >
> > Seems reasonable.  It may be too late to make 9.2 at this point, though.
>
> Is there anything I can/should do to request inclusion?

Appeal to Jakub on the 9.2 freeze thread in the gcc mailing list.

Jason

Re: [PATCH]: Fix PR c++/88095, class template argument deduction for literal operator templates per P0732 for C++2a

[Tom Honermann]

On 8/5/19 3:05 PM, Jason Merrill wrote:

On 8/2/19 9:59 AM, Tom Honermann wrote:

This patch fixes PR c++/88095:
- Bug 88095 - class nontype template parameter UDL string literals 
doesn't accepts deduction placeholder

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

It also addresses a latent issue; literal operator templates with 
template parameter packs of literal class type were previously 
accepted.   The patch corrects this and adds a test 
(udlit-class-nttp-neg.C).


In the change to gcc/cp/parser.c, it is not clear to me whether the 
'TREE_CODE (TREE_TYPE (parm)) == TEMPLATE_TYPE_PARM' comparison is 
necessary; it might be that 'CLASS_PLACEHOLDER_TEMPLATE' suffices on 
its own.


template_placeholder_p would be a shorter way to write these, but I 
think even better would be to just change CLASS_TYPE_P to 
MAYBE_CLASS_TYPE_P.  I'll make that change and commit the patch, since 
it looks like you don't have commit access yet.
Thanks, and correct, I don't have commit access yet (and I'm not sure 
that I should! :) )


If accepted, I'd like to request this change be applied to gcc 9 as 
it is needed for one of the char8_t remediation approaches documented 
in P1423, and may be helpful for existing code bases impacted by the 
char8_t changes adopted via P0482 for C++20.

- https://wg21.link/p1423#emulate


Seems reasonable.  It may be too late to make 9.2 at this point, though.


Is there anything I can/should do to request inclusion?

Tom.



Jason

Re: [PATCH V5, rs6000] Support vrotr3 for int vector types

[Segher Boessenkool]

On Tue, Aug 06, 2019 at 10:19:34AM +0800, Kewen.Lin wrote:
> on 2019/8/6 上午5:21, Segher Boessenkool wrote:
> > The other case is if not all shift counts are the same.  I'm not sure
> > we actually care much about this case :-)
> 
> Got it, I think even for the "other" case, the neg operation without masking
> is also safe since the hardware instructions do ignore the unrelated bits.

Oh it is perfectly safe.  Only wondering how well we optimise this :-)


Segher

Re: [PATCH] Enable GCC support for AVX512_VP2INTERSECT.

[Uros Bizjak]

On Tue, Aug 6, 2019 at 1:16 PM Rainer Orth  
wrote:
>
> Hi Hongtao,
>
> > On Thu, Jun 27, 2019 at 5:38 PM Rainer Orth  
> > wrote:
> >>
> >> Hi Hongtao,
> >>
> >> > On Thu, Jun 27, 2019 at 5:02 PM Rainer Orth
> >> >  wrote:
> >> >>
> >> >> Hi Hongtao,
> >> >>
> >> >> >> as usual, the new effective-target keyword needs documenting in
> >> >> >> sourcebuild.texi.
> >> >> > Like this?
> >> >>
> >> >> a couple of nits: first of all, your mailer seems to replace tabs by a
> >> >> single space.  Please fix this or attach the patch instead.
> >> >>
> >> >> > Index: ChangeLog
> >> >> > ===
> >> >> > --- ChangeLog (revision 272668)
> >> >> > +++ ChangeLog (working copy)
> >> >> > @@ -1,3 +1,8 @@
> >> >> > +2019-06-27  Hongtao Liu  
> >> >> > +
> >> >> > + * doc/sourcebuild.texi: Document new effective target keyword
> >> >> > + avx512vp2intersect.
> >> >>
> >> >> Please include the sections you're modifying, something like
> >> >>
> >> >> * doc/sourcebuild.texi (Effective-Target Keywords, Other
> >> >> hardware attributes): Document avx512vp2intersect.
> >> >>
> >> >> And please don't include the ChangeLog in the patch, but include it in
> >> >> the mail proper: it won't apply due to date and context changes anyway.
> >> >> Best review https://gcc.gnu.org/contribute.html where this is documented
> >> >> besides other points of patch submission.
> >> >>
> >> >> Besides, it's most likely useful to also review the GNU Coding
> >> >> Standards, too, not only for ChangeLog formatting.
> >> >>
> >> >> > Index: testsuite/ChangeLog
> >> >> > ===
> >> >> > --- testsuite/ChangeLog (revision 272668)
> >> >> > +++ testsuite/ChangeLog (working copy)
> >> >> > @@ -1,3 +1,11 @@
> >> >> > +2019-06-27  Hongtao Liu  
> >> >> > +
> >> >> > + * lib/target-supports.exp: Add
> >> >> > + check_effective_target_avx512vp2intersect.
> >> >>
> >> >> Use
> >> >>
> >> >> * lib/target-supports.exp
> >> >> (check_effective_target_avx512vp2intersect): New proc.
> >> >>
> >> >> > + * gcc.target/i386/avx512vp2intersect-2intersect-1b.c: Add
> >> >> > + dg-require-effective-target avx512vp2intersect.
> >> >>
> >> >> Better:
> >> >>
> >> >> * gcc.target/i386/avx512vp2intersect-2intersect-1b.c: Require
> >> >> avx512vp2intersect.
> >> >>
> >> >> but that's a matter of preference.
> >> >>
> >> >> > Index: testsuite/gcc.target/i386/avx512vp2intersect-2intersect-1b.c
> >> >> > ===
> >> >> > --- testsuite/gcc.target/i386/avx512vp2intersect-2intersect-1b.c
> >> >> > (revision 272668)
> >> >> > +++ testsuite/gcc.target/i386/avx512vp2intersect-2intersect-1b.c
> >> >> > (working copy)
> >> >> > @@ -1,5 +1,6 @@
> >> >> >  /* { dg-do run } */
> >> >> >  /* { dg-options "-O2 -mavx512vp2intersect" } */
> >> >> > +/* { dg-require-effective-target "avx512vp2intersect" } */
> >> >>
> >> >> No need to quote avx512vp2intersect here and in the next test.
> >> >>
> >> >> Ok with those nits fixed.
> >> >>
> >> > Yes, thanks a lot.
> >> >> Thanks.
> >> >> Rainer
> >> >>
> >> >> --
> >> >> -
> >> >> Rainer Orth, Center for Biotechnology, Bielefeld University
> >> >
> >> > Ok for trunk?
> >>
> >> ENOPATCH
> > Sorry, Here is the patch.
> >>
> >> --
> >> -
> >> Rainer Orth, Center for Biotechnology, Bielefeld University
> >
> >
> > Changelog
> >
> > gcc/
> > +2019-06-27  Hongtao Liu  
> > +
> > + * doc/sourcebuild.texi (Effective-Target Keywords, Other
> > + hardware attributes): Document avx512vp2intersect.
> > +
> >
> > gcc/testsuite/
> > +2019-06-27  Hongtao Liu  
> > +
> > + * lib/target-supports.exp
> > + (check_effective_target_avx512vp2intersect): New proc.
> > + * gcc.target/i386/avx512vp2intersect-2intersect-1b.c: Add
> > + dg-require-effective-target avx512vp2intersect.
> > + * gcc.target/i386/avx512vp2intersect-2intersectvl-1b.c: Ditto.
> > +
>
> unfortunately, the testcases are still not right.  While with gas 2.32
> they now come up as UNSUPPORTED, I've now tried a mainline bootstrap on
> i386-pc-solaris2.11 with gas from binutils master.  Doing so, I get
>
> +FAIL: gcc.target/i386/avx512vp2intersect-2intersect-1b.c execution test
> +FAIL: gcc.target/i386/avx512vp2intersect-2intersectvl-1b.c execution test
>
> for both 32 and 64-bit, and there are similar results on
> gcc-testresults for x86_64-pc-linux-gnu.
>
> Running one of the testcases under gdb shows
>
> Thread 2 received signal SIGILL, Illegal instruction.
> [Switching to Thread 1 (LWP 1)]
> 0x08050d89 in do_test ()
> 1: x/i $pc
> => 0x8050d89 :  (bad)
>
> or with objdump 2.32:
>
>  8050d89:   62 f2 ff 48 68  (bad)
>
> Using objdump from binutils master shows
>
>  8050d89:   62 f2 ff 48 68 05 80

Re: [PATCH] PR fortran/88227 -- Revenge of the BOZ

[Steve Kargl]

On Tue, Aug 06, 2019 at 04:27:46PM +0200, Bernhard Reutner-Fischer wrote:
> Hi Steve,
> 
> I know you already committed this but please let me add a remark or two.
> 
> On Sun, 28 Jul 2019 16:41:02 -0700
> Steve Kargl  wrote:
> 
> > +
> > +  bufp = buf = XCNEWVEC (char, j + 1);
> > +  memset (bufp, 0, j + 1);
> 
> Just cosmetics since it should be optimized away, but the memset is
> redundant, XCNEWVEC aka xcalloc already clears the memory resp.
> allocates cleared memory.
> 

I wasn't sure if XCNEWVEC zeroed memory.  The patch builds
the binary string with pointer arithmetic, and was trying
to prevent bad things from happening if I got that wrong
(which I did once or twice :).

I'll clean this up later. 

-- 
Steve

Patch ping (Re: Fix up -fexcess-precision handling in LTO (was Re: [GCC][middle-end] Add rules to strip away unneeded type casts in expressions (2nd patch)))

[Jakub Jelinek]

Hi!

I'd like to ping the https://gcc.gnu.org/ml/gcc-patches/2019-07/msg01750.html
patch.

Thanks.

On Tue, Jul 30, 2019 at 09:09:11AM +0200, Jakub Jelinek wrote:
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2019-07-30  Jakub Jelinek  
> 
>   PR middle-end/91283
>   * common.opt (fexcess-precision=): Add Optimization flag.  Use
>   flag_excess_precision variable instead of
>   flag_excess_precision_cmdline.
>   * flags.h (class target_flag_state): Remove x_flag_excess_precision
>   member.
>   (flag_excess_precision): Don't define.
>   * langhooks.c (lhd_post_options): Set flag_excess_precision instead of
>   flag_excess_precision_cmdline.  Remove comment.
>   * opts.c (set_fast_math_flags): Use frontend_set_flag_excess_precision
>   and x_flag_excess_precision instead of
>   frontend_set_flag_excess_precision_cmdline and
>   x_flag_excess_precision_cmdline.
>   (fast_math_flags_set_p): Use x_flag_excess_precision instead of
>   x_flag_excess_precision_cmdline.
>   * toplev.c (init_excess_precision): Remove.
>   (lang_dependent_init_target): Don't call it.
> ada/
>   * gcc-interface/misc.c (gnat_post_options): Set flag_excess_precision
>   instead of flag_excess_precision_cmdline.
> brig/
>   * brig-lang.c (brig_langhook_post_options): Set flag_excess_precision
>   instead of flag_excess_precision_cmdline.
> c-family/
>   * c-common.c (c_ts18661_flt_eval_method): Use flag_excess_precision
>   instead of flag_excess_precision_cmdline.
>   * c-cppbuiltin.c (c_cpp_flt_eval_method_iec_559): Likewise.
>   * c-opts.c (c_common_post_options): Likewise.
> d/
>   * d-lang.cc (d_post_options): Set flag_excess_precision instead of
>   flag_excess_precision_cmdline.
> fortran/
>   * options.c (gfc_post_options): Set flag_excess_precision instead of
>   flag_excess_precision_cmdline.  Remove comment.
> go/
>   * go-lang.c (go_langhook_post_options): Set flag_excess_precision
>   instead of flag_excess_precision_cmdline.
> lto/
>   * lto-lang.c (lto_post_options): Set flag_excess_precision instead of
>   flag_excess_precision_cmdline.  Remove comment.

Jakub

Re: Protect tree_to_shwi call in unmodified_param_1

[Richard Sandiford]

Richard Biener  writes:
> On Mon, Aug 5, 2019 at 10:58 AM Richard Sandiford
>  wrote:
>>
>> unmodified_param_1 used tree_to_shwi without first checking
>> tree_fits_shwi_p.  This is needed by the SVE ACLE support and
>> is hard to test independently.
>>
>> Tested on aarch64-linux-gnu, armeb-eabi and x86_64-linux-gnu.
>> OK to install?
>
> Hmm, a better fix would be to use poly_X for *size_p.  There are also
> quite a number of callers with NULL size_p which you unduly
> pessimize for SVE.

OK, you guilt-tripped me into doing it properly. :-)  How does this look?

Tested as before.

Richard


2019-08-06  Richard Sandiford  

gcc/
* data-streamer.h (streamer_write_poly_uint64): Declare.
(streamer_read_poly_uint64): Likewise.
* data-streamer-in.c (streamer_read_poly_uint64): New function.
* data-streamer-out.c (streamer_write_poly_uint64): Likewise.
* ipa-predicate.h (condition::size): Turn into a poly_int64.
(add_condition): Take a poly_int64 size.
* ipa-predicate.c (add_condition): Likewise.
* ipa-prop.h (ipa_load_from_parm_agg): Take a poly_int64 size pointer.
* ipa-prop.c (ipa_load_from_parm_agg): Likewise.
(ipcp_modif_dom_walker::before_dom_children): Update accordingly.
* ipa-fnsummary.c (evaluate_conditions_for_known_args): Handle
condition::size as a poly_int64.
(unmodified_parm_1): Take a poly_int64 size pointer.
(unmodified_parm): Likewise.
(unmodified_parm_or_parm_agg_item): Likewise.
(set_cond_stmt_execution_predicate): Update accordingly.
(set_switch_stmt_execution_predicate): Likewise.
(will_be_nonconstant_expr_predicate): Likewise.
(will_be_nonconstant_predicate): Likewise.
(inline_read_section): Stream condition::size as a poly_int.
(ipa_fn_summary_write): Likewise.

Index: gcc/data-streamer.h
===
--- gcc/data-streamer.h 2019-07-10 19:41:26.375898187 +0100
+++ gcc/data-streamer.h 2019-08-06 15:45:32.908297910 +0100
@@ -53,6 +53,7 @@ HOST_WIDE_INT bp_unpack_var_len_int (str
 void streamer_write_zero (struct output_block *);
 void streamer_write_uhwi (struct output_block *, unsigned HOST_WIDE_INT);
 void streamer_write_hwi (struct output_block *, HOST_WIDE_INT);
+void streamer_write_poly_uint64 (struct output_block *, poly_uint64);
 void streamer_write_gcov_count (struct output_block *, gcov_type);
 void streamer_write_string (struct output_block *, struct lto_output_stream *,
const char *, bool);
@@ -82,6 +83,7 @@ const char *bp_unpack_indexed_string (cl
 const char *bp_unpack_string (class data_in *, struct bitpack_d *);
 unsigned HOST_WIDE_INT streamer_read_uhwi (class lto_input_block *);
 HOST_WIDE_INT streamer_read_hwi (class lto_input_block *);
+poly_uint64 streamer_read_poly_uint64 (class lto_input_block *);
 gcov_type streamer_read_gcov_count (class lto_input_block *);
 wide_int streamer_read_wide_int (class lto_input_block *);
 widest_int streamer_read_widest_int (class lto_input_block *);
Index: gcc/data-streamer-in.c
===
--- gcc/data-streamer-in.c  2019-07-10 19:41:20.147948065 +0100
+++ gcc/data-streamer-in.c  2019-08-06 15:45:32.908297910 +0100
@@ -175,6 +175,17 @@ streamer_read_hwi (class lto_input_block
 }
 }
 
+/* Read a poly_uint64 from IB.  */
+
+poly_uint64
+streamer_read_poly_uint64 (class lto_input_block *ib)
+{
+  poly_uint64 res;
+  for (unsigned int i = 0; i < NUM_POLY_INT_COEFFS; ++i)
+res.coeffs[i] = streamer_read_uhwi (ib);
+  return res;
+}
+
 /* Read gcov_type value from IB.  */
 
 gcov_type
Index: gcc/data-streamer-out.c
===
--- gcc/data-streamer-out.c 2019-03-08 18:14:27.285004225 +
+++ gcc/data-streamer-out.c 2019-08-06 15:45:32.908297910 +0100
@@ -220,6 +220,15 @@ streamer_write_hwi (struct output_block
   streamer_write_hwi_stream (ob->main_stream, work);
 }
 
+/* Write a poly_uint64 value WORK to OB->main_stream.  */
+
+void
+streamer_write_poly_uint64 (struct output_block *ob, poly_uint64 work)
+{
+  for (int i = 0; i < NUM_POLY_INT_COEFFS; ++i)
+streamer_write_uhwi_stream (ob->main_stream, work.coeffs[i]);
+}
+
 /* Write a gcov counter value WORK to OB->main_stream.  */
 
 void
Index: gcc/ipa-predicate.h
===
--- gcc/ipa-predicate.h 2019-07-10 19:41:27.163891877 +0100
+++ gcc/ipa-predicate.h 2019-08-06 15:45:32.908297910 +0100
@@ -31,7 +31,7 @@ struct GTY(()) condition
  loaded.  */
   HOST_WIDE_INT offset;
   /* Size of the access reading the data (or the PARM_DECL SSA_NAME).  */
-  HOST_WIDE_INT size;
+  poly_int64 size;
   tree val;
   int operand_num;
   ENUM_BITFIELD(tree_code) code : 16;
@@ -228,5 +228,5 @@ typedef uint32_t clause_t;
 
 void dump_condition (FILE *f, condition

C++ PATCH for c++/91346 - Implement C++2a P1668R1, allow unevaluated asm in constexpr

[Marek Polacek]

This patch implements another C++2a feature, P1668R1: Permit unevaluated inline
asm in constexpr functions.

It's really straightforward so not much to say; we need to allow ASM_EXPRs in
potential_constant_expression_1, but since only unevaluated asm is allowed,
cxx_eval_constant_expression must give an error when it encounters an ASM_EXPR.

I've improved location for "asm", so that the error points to "asm" rather than
to ";".

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

2019-08-06  Marek Polacek  

PR c++/91346 - Implement P1668R1, allow unevaluated asm in constexpr.
* constexpr.c (cxx_eval_constant_expression): Handle ASM_EXPR.
(potential_constant_expression_1) : Allow in C++2a, give
an error otherwise.
* cp-tree.h (finish_asm_stmt): Adjust.
* parser.c (cp_parser_asm_definition): Grab the locaion of "asm" and
use it.  Adjust an error message.  Allow asm in C++2a.
* pt.c (tsubst_expr): Pass a location down to finish_asm_stmt.
* semantics.c (finish_asm_stmt): New location_t parameter.  Use it.

* g++.dg/cpp2a/inline-asm1.C: New test.
* g++.dg/cpp2a/inline-asm2.C: New test.
* g++.dg/cpp1y/constexpr-neg1.C: Adjust dg-error.

diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
index 36a66337433..d45e65df400 100644
--- gcc/cp/constexpr.c
+++ gcc/cp/constexpr.c
@@ -5289,6 +5289,18 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, 
tree t,
   r = void_node;
   break;
 
+case ASM_EXPR:
+  if (!ctx->quiet)
+   {
+ error_at (cp_expr_loc_or_input_loc (t),
+   "inline assembly is not a constant expression");
+ inform (cp_expr_loc_or_input_loc (t),
+ "only unevaluated inline assembly is allowed in a "
+ "% function in C++2a");
+   }
+  *non_constant_p = true;
+  return t;
+
 default:
   if (STATEMENT_CODE_P (TREE_CODE (t)))
{
@@ -6469,13 +6481,22 @@ potential_constant_expression_1 (tree t, bool 
want_rval, bool strict, bool now,
   /* GCC internal stuff.  */
 case VA_ARG_EXPR:
 case TRANSACTION_EXPR:
-case ASM_EXPR:
 case AT_ENCODE_EXPR:
 fail:
   if (flags & tf_error)
error_at (loc, "expression %qE is not a constant expression", t);
   return false;
 
+case ASM_EXPR:
+  if (cxx_dialect >= cxx2a)
+   /* In C++2a, unevaluated inline assembly is permitted in constexpr
+  functions.  */
+   return true;
+  else if (flags & tf_error)
+   error_at (loc, "inline assembly is not allowed in % "
+ "functions before C++2a");
+  return false;
+
 case OBJ_TYPE_REF:
   if (cxx_dialect >= cxx2a)
/* In C++2a virtual calls can be constexpr, don't give up yet.  */
diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h
index d4e67cdfd96..72ee1d61e97 100644
--- gcc/cp/cp-tree.h
+++ gcc/cp/cp-tree.h
@@ -7052,8 +7052,8 @@ enum {
 extern tree begin_compound_stmt(unsigned int);
 
 extern void finish_compound_stmt   (tree);
-extern tree finish_asm_stmt(int, tree, tree, tree, tree,
-tree, bool);
+extern tree finish_asm_stmt(location_t, int, tree, tree,
+tree, tree, tree, bool);
 extern tree finish_label_stmt  (tree);
 extern void finish_label_decl  (tree);
 extern cp_expr finish_parenthesized_expr   (cp_expr);
diff --git gcc/cp/parser.c gcc/cp/parser.c
index 79da7b52eb9..2d0e6a0c931 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -19817,14 +19817,19 @@ cp_parser_asm_definition (cp_parser* parser)
   bool invalid_inputs_p = false;
   bool invalid_outputs_p = false;
   required_token missing = RT_NONE;
+  location_t asm_loc = cp_lexer_peek_token (parser->lexer)->location;
 
   /* Look for the `asm' keyword.  */
   cp_parser_require_keyword (parser, RID_ASM, RT_ASM);
 
   if (parser->in_function_body
-  && DECL_DECLARED_CONSTEXPR_P (current_function_decl))
+  && DECL_DECLARED_CONSTEXPR_P (current_function_decl)
+  /* In C++2a, unevaluated inline assembly is permitted in constexpr
+functions.  */
+  && (cxx_dialect < cxx2a))
 {
-  error ("% in % function");
+  error_at (asm_loc, "% in % function only available "
+   "with %<-std=c++2a%> or %<-std=gnu++2a%>");
   cp_function_chain->invalid_constexpr = true;
 }
 
@@ -20032,7 +20037,7 @@ cp_parser_asm_definition (cp_parser* parser)
   /* Create the ASM_EXPR.  */
   if (parser->in_function_body)
{
- asm_stmt = finish_asm_stmt (volatile_p, string, outputs,
+ asm_stmt = finish_asm_stmt (asm_loc, volatile_p, string, outputs,
  inputs, clobbers, labels, inline_p);
  /* If the extended syntax was not used, mark the ASM_EXPR.  */
  if (!extended

Re: [PATCH] PR fortran/88227 -- Revenge of the BOZ

[Bernhard Reutner-Fischer]

Hi Steve,

I know you already committed this but please let me add a remark or two.

On Sun, 28 Jul 2019 16:41:02 -0700
Steve Kargl  wrote:

> Index: gcc/fortran/check.c
> ===
> --- gcc/fortran/check.c   (revision 273766)
> +++ gcc/fortran/check.c   (working copy)
> @@ -55,6 +55,7 @@ gfc_invalid_boz (const char *msg, locus *loc)
>  
>  
>  /* Issue an error for an illegal BOZ argument.  */
> +
>  static bool
>  illegal_boz_arg (gfc_expr *x)
>  {
> @@ -101,6 +102,167 @@ is_boz_constant (gfc_expr *a)
>  }
>  
>  
> +/* Convert a octal string into a binary string.  This is used in the
> +   fallback conversion of an octal string to a REAL.  */
> +
> +static char *
> +oct2bin(int nbits, char *oct)
> +{
> +  const char bits[8][5] = {
> +"000", "001", "010", "011", "100", "101", "110", "111"};
> +
> +  char *buf, *bufp;
> +  int i, j, n;
> +
> +  j = nbits + 1;
> +  if (nbits == 64) j++;
> +
> +  bufp = buf = XCNEWVEC (char, j + 1);
> +  memset (bufp, 0, j + 1);

Just cosmetics since it should be optimized away, but the memset is
redundant, XCNEWVEC aka xcalloc already clears the memory resp.
allocates cleared memory.

> +
> +  n = strlen (oct);
> +  for (i = 0; i < n; i++, oct++)
> +{
> +  j = *oct - 48;
> +  strcpy (bufp, &bits[j][0]);
> +  bufp += 3;
> +}
> +
> +  bufp = XCNEWVEC (char, nbits + 1);

Since you strcpy below, a XNEWVEC should suffice, i.e. you don't need
 cleared memory since strcpy also copies the trailing null byte.

> +  if (nbits == 64)
> +strcpy (bufp, buf + 2);
> +  else
> +strcpy (bufp, buf + 1);
> +
> +  free (buf);
> +
> +  return bufp;
> +}
> +
> +
> +/* Convert a hexidecimal string into a binary string.  This is used in the
> +   fallback conversion of a hexidecimal string to a REAL.  */
> +
> +static char *
> +hex2bin(int nbits, char *hex)
> +{
> +  const char bits[16][5] = {
> +"", "0001", "0010", "0011", "0100", "0101", "0110", "0111",
> +"1000", "1001", "1010", "1011", "1100", "1101", "1110", ""};
> +
> +  char *buf, *bufp;
> +  int i, j, n;
> +
> +  bufp = buf = XCNEWVEC (char, nbits + 1);
> +  memset (bufp, 0, nbits + 1);

Like above, memset is redundant.

> +
> +  n = strlen (hex);
> +  for (i = 0; i < n; i++, hex++)
> +{
> +  j = *hex;
> +  if (j > 47 && j < 58)
> + j -= 48;
> +  else if (j > 64 && j < 71)
> + j -= 55;
> +  else if (j > 96 && j < 103)
> + j -= 87;
> +  else
> + gcc_unreachable ();
> +
> +  strcpy (bufp, &bits[j][0]);
> +  bufp += 4;
> +   }
> +
> +   return buf;
> +}
> +
> +
> +/* Fallback conversion of a BOZ string to REAL.  */
> +
> +static void
> +bin2real (gfc_expr *x, int kind)
> +{
> +  char buf[114], *sp;
> +  int b, i, ie, t, w;
> +  bool sgn;
> +  mpz_t em;
> +
> +  i = gfc_validate_kind (BT_REAL, kind, false);
> +  t = gfc_real_kinds[i].digits - 1;
> +
> +  /* Number of bits in the exponent.  */
> +  if (gfc_real_kinds[i].max_exponent == 16384)
> +w = 15;
> +  else if (gfc_real_kinds[i].max_exponent == 1024)
> +w = 11;
> +  else
> +w = 8;
> +
> +  if (x->boz.rdx == 16)
> +sp = hex2bin (gfc_real_kinds[i].mode_precision, x->boz.str);
> +  else if (x->boz.rdx == 8)
> +sp = oct2bin (gfc_real_kinds[i].mode_precision, x->boz.str);
> +  else
> +sp = x->boz.str;
> +
> +  /* Extract sign bit. */
> +  sgn = *sp != '0';
> +
> +  /* Extract biased exponent. */
> +  memset (buf, 0, 114);
> +  strncpy (buf, ++sp, w);
> +  mpz_init (em);
> +  mpz_set_str (em, buf, 2);
> +  ie = mpz_get_si (em);
> +
> +  mpfr_init2 (x->value.real, t + 1);
> +  x->ts.type = BT_REAL;
> +  x->ts.kind = kind;
> +
> +  sp += w;   /* Set to first digit in significand. */
> +  b = (1 << w) - 1;
> +  if ((i == 0 && ie == b) || (i == 1 && ie == b)
> +  || ((i == 2 || i == 3) && ie == b))
> +{
> +  bool zeros = true;
> +  if (i == 2) sp++;
> +  for (; *sp; sp++)
> + {
> +   if (*sp != '0')
> + {
> +   zeros = false;
> +   break;
> + }
> + }
> +
> +  if (zeros)
> + mpfr_set_inf (x->value.real, 1);
> +  else
> + mpfr_set_nan (x->value.real);
> +}
> +  else
> +{
> +  if (i == 2)
> + strncpy (buf, sp, t + 1);
> +  else
> + {
> +   /* Significand with hidden bit. */
> +   buf[0] = '1';
> +   strncpy (&buf[1], sp, t);
> + }
> +
> +  /* Convert to significand to integer. */

/to s/s/to/the/

thanks,

> +  mpz_set_str (em, buf, 2);
> +  ie -= ((1 << (w - 1)) - 1);/* Unbiased exponent. */
> +  mpfr_set_z_2exp (x->value.real, em, ie - t, GFC_RND_MODE);
> +}
> +
> +   if (sgn) mpfr_neg (x->value.real, x->value.real, GFC_RND_MODE);
> +
> +   mpz_clear (em);
> +}
> +
> +
>  /* Fortran 2018 treats a BOZ as simply a string of bits.  gfc_boz2real () 
> converts the string into a REAL of the appropriate kind.  The treatment
> of the sign bit is pro

Re: [ARM/FDPIC v5 03/21] [ARM] FDPIC: Force FDPIC related options unless -mno-fdpic is provided

[Richard Sandiford]

Christophe Lyon  writes:
> On Tue, 16 Jul 2019 at 12:34, Richard Sandiford
>  wrote:
>>
>> Christophe Lyon  writes:
>> > On 22/05/2019 10:45, Christophe Lyon wrote:
>> >> On Wed, 22 May 2019 at 10:39, Szabolcs Nagy  wrote:
>> >>>
>> >>> On 21/05/2019 16:28, Christophe Lyon wrote:
>>  --- a/gcc/config/arm/linux-eabi.h
>>  +++ b/gcc/config/arm/linux-eabi.h
>>  @@ -89,7 +89,7 @@
>>    #define MUSL_DYNAMIC_LINKER_E "%{mbig-endian:eb}"
>>    #endif
>>    #define MUSL_DYNAMIC_LINKER \
>>  -  "/lib/ld-musl-arm" MUSL_DYNAMIC_LINKER_E "%{mfloat-abi=hard:hf}.so.1"
>>  +  "/lib/ld-musl-arm" MUSL_DYNAMIC_LINKER_E
>>  "%{mfloat-abi=hard:hf}%{mfdpic:-fdpic}.so.1"
>> >>>
>> >>> the line break seems wrong (either needs \ or no newline)
>> >>>
>> >> Sorry, that's a mailer artifact.
>> >>
>>  --- a/libsanitizer/configure.tgt
>>  +++ b/libsanitizer/configure.tgt
>>  @@ -45,7 +45,7 @@ case "${target}" in
>>   ;;
>>  sparc*-*-solaris2.11*)
>>   ;;
>>  -  arm*-*-uclinuxfdpiceabi)
>>  +  arm*-*-fdpiceabi)
>> >>>
>> >>> should be *fdpiceabi instead of *-fdpiceabi i think.
>> >>
>> >> Indeed, thanks
>> >> .
>> >>
>> > FWIW, here is the updated patch:
>> > - handles musl -fdpic suffix
>> > - disables sanitizers for arm*-*-fdpiceabi
>> > - does not handle -static in a special way, so using -static produces 
>> > binaries that request the non-existing /usr/lib/ld.so.1, thus effectively 
>> > making -static broken/unsupported (this does lead to a few more FAIL in 
>> > the testsuite)
>> >
>> > The plan is to work -static-pie later, as discussed.
>>
>> Could you make -static without -mno-fdpic an error via a %e spec,
>> so that the failure mode is a bit more user-friendly?
>>
>
> Sure.
> Do you know if there is a way to catch linker options in the specs?
> Would it be possible to still accept -static -Wl,-dynamic-linker XXX ?

Ah, no, don't know a way of doing that.  Maybe the error isn't
feasible after all then (at least not without significant work).

Richard

[C++ PATCH] PR c++/91378 - ICE with noexcept and auto return type.

[Jason Merrill]

Here, since the call to g is not type-dependent, we call mark_used on it to
determine its return type.  This also wants to instantiate the
noexcept-expression.  But since nothing in maybe_instantiate_noexcept was
calling push_to_top_level, we substituted b.i with processing_template_decl
set, so we left it unresolved for later access checking.  As a result, the
type of C::g remained instantiation-dependent, leading to an ICE in
type_dependent_expression_p on the assert that the type of a function
template with no dependent template arguments must be non-dependent.

Tested x86_64-pc-linux-gnu, applying to trunk (and 9 when it unfreezes).

* pt.c (maybe_instantiate_noexcept): push_to_top_level.
---
 gcc/cp/pt.c|  7 ++-
 gcc/testsuite/g++.dg/cpp1y/auto-fn56.C | 19 +++
 gcc/cp/ChangeLog   |  5 +
 3 files changed, 26 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/auto-fn56.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 903e589b663..b71fbaad789 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -24315,12 +24315,11 @@ maybe_instantiate_noexcept (tree fn, tsubst_flags_t 
complain)
}
   else if (push_tinst_level (fn))
{
+ push_to_top_level ();
  push_access_scope (fn);
  push_deferring_access_checks (dk_no_deferred);
  input_location = DECL_SOURCE_LOCATION (fn);
 
- tree save_ccp = current_class_ptr;
- tree save_ccr = current_class_ref;
  /* If needed, set current_class_ptr for the benefit of
 tsubst_copy/PARM_DECL.  */
  tree tdecl = DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (fn));
@@ -24346,9 +24345,6 @@ maybe_instantiate_noexcept (tree fn, tsubst_flags_t 
complain)
/*function_p=*/false,
/*i_c_e_p=*/true);
 
- current_class_ptr = save_ccp;
- current_class_ref = save_ccr;
-
  /* Build up the noexcept-specification.  */
  spec = build_noexcept_spec (noex, tf_warning_or_error);
 
@@ -24358,6 +24354,7 @@ maybe_instantiate_noexcept (tree fn, tsubst_flags_t 
complain)
  pop_deferring_access_checks ();
  pop_access_scope (fn);
  pop_tinst_level ();
+ pop_from_top_level ();
}
   else
spec = noexcept_false_spec;
diff --git a/gcc/testsuite/g++.dg/cpp1y/auto-fn56.C 
b/gcc/testsuite/g++.dg/cpp1y/auto-fn56.C
new file mode 100644
index 000..69bdd9586eb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/auto-fn56.C
@@ -0,0 +1,19 @@
+// PR c++/91378
+// { dg-do compile { target c++14 } }
+
+struct B
+{
+  int i;
+};
+
+struct C
+{
+  template  static auto
+  g(B b) noexcept(noexcept(b.i)) { }
+};
+
+template 
+void h(T t)
+{
+  C::g({});
+}
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index e5419322d6f..9b0a34c98aa 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,8 @@
+2019-08-06  Jason Merrill  
+
+   PR c++/91378 - ICE with noexcept and auto return type.
+   * pt.c (maybe_instantiate_noexcept): push_to_top_level.
+
 2019-08-06  Paolo Carlini  
 
* decl.c (check_array_designated_initializer): Use

base-commit: add510c752d53de29fa01c43034848acfa0775f2
-- 
2.21.0

Re: Don't use integer "FMA" for shifts

[Richard Sandiford]

"Richard Earnshaw (lists)"  writes:
> On 30/07/2019 11:50, Richard Sandiford wrote:
>> Richard Biener  writes:
>>> On Tue, Jul 30, 2019 at 12:04 PM Richard Sandiford
>>>  wrote:

 tree-ssa-math-opts supports FMA optabs for integers as well as
 floating-point types, even though there's no distinction between
 fused and unfused there.  It turns out to be pretty handy for the
 IFN_COND_* handling though, so I don't want to remove it, however
 weird it might seem.

 Instead this patch makes sure that we don't apply it to integer
 multiplications that are actually shifts (but that are represented
 in gimple as multiplications because that's the canonical form).

 This is a preemptive strike.  The test doesn't fail for SVE as-is,
 but would after a later patch.

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

 Richard


 2019-07-30  Richard Sandiford  

 gcc/
  * tree-ssa-math-opts.c (convert_mult_to_fma): Reject integer
  multiplications by a power of 2 or a negative power of 2.

 gcc/testsuite/
  * gcc.dg/vect/vect-cond-arith-8.c: New test.

 Index: gcc/tree-ssa-math-opts.c
 ===
 --- gcc/tree-ssa-math-opts.c2019-07-30 10:51:51.827405171 +0100
 +++ gcc/tree-ssa-math-opts.c2019-07-30 10:52:27.327139149 +0100
 @@ -3074,10 +3074,20 @@ convert_mult_to_fma (gimple *mul_stmt, t
 && flag_fp_contract_mode == FP_CONTRACT_OFF)
   return false;

 -  /* We don't want to do bitfield reduction ops.  */
 -  if (INTEGRAL_TYPE_P (type)
 -  && (!type_has_mode_precision_p (type) || TYPE_OVERFLOW_TRAPS 
 (type)))
 -return false;
 +  if (ANY_INTEGRAL_TYPE_P (type))
 +{
 +  /* We don't want to do bitfield reduction ops.  */
 +  tree itype = INTEGRAL_TYPE_P (type) ? type : TREE_TYPE (type);
>>>
>>> you can use element_type () for this.  But I think it's easier to
>>> leave the existing
>>> test in-place since vector or complex types cannot have non-mode precision
>>> components.
>> 
>> Ah, yeah.  Guess I was over-generalising here :-)
>> 
 +  if (!type_has_mode_precision_p (itype) || TYPE_OVERFLOW_TRAPS 
 (itype))
 +   return false;
 +
 +  /* Don't use FMA for multiplications that are actually shifts.  */
>>>
>>> So I question this - if the FMA can do the shift "for free" and it 
>>> eventually is
>>> same cost/latency as an add why do we want to not allow this (for all 
>>> targets)?
>>> Esp. for vector types I would guess a add plus a shift may be more costly
>>> (not all ISAs implement shifts anyhow).
>> 
>> The shift doesn't really come for free.  By using FMA we're converting
>> the shift by a constant into a general multiplication.
>
> Isn't this just the problem that the madd pattern for aarch64 
> doesn't accept constants that are a power of 2?  So it ends up trying to 
> force the constant into a register unnecessarily.
>
> (define_insn "madd"
>[(set (match_operand:GPI 0 "register_operand" "=r")
>   (plus:GPI (mult:GPI (match_operand:GPI 1 "register_operand" "r")
>   (match_operand:GPI 2 "register_operand" "r"))
> --- This should be reg_or_imm_power2, then a second alternative for the 
> immediate case.
>
> (match_operand:GPI 3 "register_operand" "r")))]
>""

That might be a problem too (e.g. maybe for intrinsics?), but this patch
was only dealing with the way that the "fma4" group of patterns are
used.  The AArch64 port doesn't yet define fma for any integer modes AFAICT,
but I have a patch that adds them for SVE.  (And it now handles shifts
specially, as Richard suggested.)

Thanks,
Richard


>
> R.
>
>> 
>>> Can this be fixed up in the target instead?  (by a splitter or appropriate
>>> expander?)
>> 
>> OK, I'll try to do it that way instead.
>> 
>> Thanks,
>> Richard
>> 
>>>
 +  tree val = VECTOR_TYPE_P (type) ? uniform_vector_p (op2) : op2;
 +  if (val
 + && TREE_CODE (val) == INTEGER_CST
 + && wi::popcount (wi::abs (wi::to_wide (val))) == 1)
 +   return false;
 +}

 /* If the target doesn't support it, don't generate it.  We assume that
if fma isn't available then fms, fnma or fnms are not either.  */
 Index: gcc/testsuite/gcc.dg/vect/vect-cond-arith-8.c
 ===
 --- /dev/null   2019-07-30 08:53:31.317691683 +0100
 +++ gcc/testsuite/gcc.dg/vect/vect-cond-arith-8.c   2019-07-30 
 10:52:27.327139149 +0100
 @@ -0,0 +1,57 @@
 +/* { dg-require-effective-target scalar_all_fma } */
 +/* { dg-additional-options "-fdump-tree-optimized -ffp-contract=fast" } */
 +
 +#include "tree-vect.h"
 +
 +#define N (VECTOR_BITS 

Re: [PATCH] Handle new operators with no arguments in DCE.

[Martin Liška]

On 8/5/19 3:46 PM, Marc Glisse wrote:
> On Mon, 5 Aug 2019, Martin Liška wrote:
> 
>> You are right. It can really lead to confusion of the DCE.
>>
>> What we have is DECL_ABSTRACT_ORIGIN(decl) which we can use to indicate 
>> operators
>> that were somehow modified by an IPA optimization.
> 
> Looks similar to the cgraph_node->clone_of that Richard was talking about. I 
> have no idea which one would be best.


Hm, strange that the ISRA clones don't have n->clone_of set. It's created here:

#0  cgraph_node::create (decl=) at /home/marxin/Programming/gcc/gcc/profile-count.h:751
#1  0x00bc8342 in cgraph_node::create_version_clone (this=, new_decl=, 
redirect_callers=..., bbs_to_copy=0x0, suffix=0x1b74427 "isra") at 
/home/marxin/Programming/gcc/gcc/cgraphclones.c:960
#2  0x00bc9b9a in cgraph_node::create_version_clone_with_body 
(this=this@entry=, 
redirect_callers=redirect_callers@entry=..., tree_map=tree_map@entry=0x0, 
args_to_skip=args_to_skip@entry=0x0, 
skip_return=skip_return@entry=false, bbs_to_copy=bbs_to_copy@entry=0x0, 
new_entry_block=, suffix=, 
target_attributes=) at 
/home/marxin/Programming/gcc/gcc/cgraphclones.c:1071
#3  0x010e4611 in modify_function (adjustments=..., node=) at 
/home/marxin/Programming/gcc/gcc/tree-sra.c:5493
#4  ipa_early_sra () at /home/marxin/Programming/gcc/gcc/tree-sra.c:5731
#5  (anonymous namespace)::pass_early_ipa_sra::execute (this=) 
at /home/marxin/Programming/gcc/gcc/tree-sra.c:5778

@Martin, @Honza: Why do we not set clone_of in this transformation?

> 
>> Do you believe it will be sufficient?
> 
> In DCE only consider the operator delete that are not clones? (possibly the 
> same for operator new? I don't know how much we can change the return value 
> of a function in a clone) I guess that should work, and it wouldn't impact 
> the common case with default, global operator new/delete.

I tent to limit that the only cgraph nodes that are not clones. I'm going to 
prepare a patch for it.

> 
> An alternative would be to clear the DECL_IS_OPERATOR_DELETE flag when 
> creating a clone (possibly only if it modified the first parameter?). There 
> is probably not much information in the fact that a function that doesn't 
> even take a pointer used to be an operator delete.
> 
> 
> By the way, I just thought of something, now that we handle class-specific 
> operator new/delete (but you could do the same with the global replacable 
> ones as well):
> 
> #include 
> int count = 0;
> struct A {
>   __attribute__((malloc,noinline))
>   static void* operator new(unsigned long sz){++count;return ::operator 
> new(sz);}
>   static void operator delete(void* ptr){--count;::operator delete(ptr);}
> };
> int main(){
>   delete new A;
>   printf("%d\n",count);
> }
> 
> If we do not inline anything, we can remove the pair and nothing touches 
> count. If we inline both new and delete, we can then remove the inner pair 
> instead, count increases and decreases, fine. If we inline only one of them, 
> and DCE the mismatched pair new/delete, we get something inconsistent (count 
> is -1).
> 
> This seems to indicate we should check that the new and delete match 
> somehow...
> 

[C++ Patch] Improve start_function and grokmethod locations

[Paolo Carlini]

Hi,

apparently this is now easy to do, likely because a while ago I made 
sure that we consistently have meaningful locations for TYPE_DECLs too.


(I went through grokdeclarator and confirmed that when the third 
argument is FUNCDEF or MEMFUNCDEF it cannot return NULL_TREE)


Tested x86_64-linux.

Thanks, Paolo.




/cp
2019-08-06  Paolo Carlini  

* decl.c (start_function): Use DECL_SOURCE_LOCATION.
(grokmethod): Likewise.

/testsuite
2019-08-06  Paolo Carlini  

* g++.dg/parse/typedef9.C: Test locations too.
Index: cp/decl.c
===
--- cp/decl.c   (revision 274141)
+++ cp/decl.c   (working copy)
@@ -15774,9 +15774,10 @@ start_function (cp_decl_specifier_seq *declspecs,
 return false;
   /* If the declarator is not suitable for a function definition,
  cause a syntax error.  */
-  if (decl1 == NULL_TREE || TREE_CODE (decl1) != FUNCTION_DECL)
+  if (TREE_CODE (decl1) != FUNCTION_DECL)
 {
-  error ("invalid function declaration");
+  error_at (DECL_SOURCE_LOCATION (decl1),
+   "invalid function declaration");
   return false;
 }
 
@@ -16433,9 +16434,10 @@ grokmethod (cp_decl_specifier_seq *declspecs,
   if (fndecl == error_mark_node)
 return error_mark_node;
 
-  if (fndecl == NULL || TREE_CODE (fndecl) != FUNCTION_DECL)
+  if (TREE_CODE (fndecl) != FUNCTION_DECL)
 {
-  error ("invalid member function declaration");
+  error_at (DECL_SOURCE_LOCATION (fndecl),
+   "invalid member function declaration");
   return error_mark_node;
 }
 
Index: testsuite/g++.dg/parse/typedef9.C
===
--- testsuite/g++.dg/parse/typedef9.C   (revision 274140)
+++ testsuite/g++.dg/parse/typedef9.C   (working copy)
@@ -1,8 +1,8 @@
 // PR c++/38794
 // { dg-do compile }
 
-typedef void foo () {} // { dg-error "invalid function declaration" }
+typedef void foo () {} // { dg-error "14:invalid function declaration" }
 struct S
 {
-  typedef int bar (void) { return 0; } // { dg-error "invalid member function 
declaration" }
+  typedef int bar (void) { return 0; }  // { dg-error "15:invalid member 
function declaration" }
 };

[PATCH] Fix file descriptor existence of MinGW.

[Martin Liška]

Hi.

The patch is about proper checking of file descriptors on Windows.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

@Pekka: Can you please test it on Windows?

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2019-08-06  Martin Liska  

PR bootstrap/91352
* gcc.c (fd_exists): New.
(driver::detect_jobserver): Use fd_exists.
* lto-wrapper.c (fd_exists): New.
(jobserver_active_p): Use fd_exists.
---
 gcc/gcc.c | 19 +--
 gcc/lto-wrapper.c | 19 +--
 2 files changed, 34 insertions(+), 4 deletions(-)


diff --git a/gcc/gcc.c b/gcc/gcc.c
index 18a07426290..6bcd989ee30 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -8359,6 +8359,21 @@ driver::final_actions () const
 }
 }
 
+/* Return true when FD file descriptor exists.  */
+
+static bool
+fd_exists (int fd)
+{
+#if defined (F_GETFD)
+  return fcntl (fd, F_GETFD) >= 0;
+#elif defined(_WIN32)
+  HANDLE h = (HANDLE) _get_osfhandle (fd);
+  return h != (HANDLE) -1;
+#else
+  return false;
+#endif
+}
+
 /* Detect whether jobserver is active and working.  If not drop
--jobserver-auth from MAKEFLAGS.  */
 
@@ -8380,8 +8395,8 @@ driver::detect_jobserver () const
 	= (sscanf (n + strlen (needle), "%d,%d", &rfd, &wfd) == 2
 	   && rfd > 0
 	   && wfd > 0
-	   && fcntl (rfd, F_GETFD) >= 0
-	   && fcntl (wfd, F_GETFD) >= 0);
+	   && fd_exists (rfd)
+	   && fd_exists (wfd));
 
 	  /* Drop the jobserver if it's not working now.  */
 	  if (!jobserver)
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index 3414adedd26..065a1d0ad02 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -1217,6 +1217,21 @@ init_num_threads (void)
 
 /* FIXME: once using -std=c11, we can use std::thread::hardware_concurrency.  */
 
+/* Return true when FD file descriptor exists.  */
+
+static bool
+fd_exists (int fd)
+{
+#if defined (F_GETFD)
+  return fcntl (fd, F_GETFD) >= 0;
+#elif defined(_WIN32)
+  HANDLE h = (HANDLE) _get_osfhandle (fd);
+  return h != (HANDLE) -1;
+#else
+  return false;
+#endif
+}
+
 /* Return true when a jobserver is running and can accept a job.  */
 
 static bool
@@ -1237,8 +1252,8 @@ jobserver_active_p (void)
   return (sscanf (n + strlen (needle), "%d,%d", &rfd, &wfd) == 2
 	  && rfd > 0
 	  && wfd > 0
-	  && fcntl (rfd, F_GETFD) >= 0
-	  && fcntl (wfd, F_GETFD) >= 0);
+	  && fd_exists (rfd)
+	  && fd_exists (wfd));
 }
 
 /* Execute gcc. ARGC is the number of arguments. ARGV contains the arguments. */

Re: [PATCH] Enable GCC support for AVX512_VP2INTERSECT.

[Rainer Orth]

Hi Hongtao,

> On Thu, Jun 27, 2019 at 5:38 PM Rainer Orth  
> wrote:
>>
>> Hi Hongtao,
>>
>> > On Thu, Jun 27, 2019 at 5:02 PM Rainer Orth
>> >  wrote:
>> >>
>> >> Hi Hongtao,
>> >>
>> >> >> as usual, the new effective-target keyword needs documenting in
>> >> >> sourcebuild.texi.
>> >> > Like this?
>> >>
>> >> a couple of nits: first of all, your mailer seems to replace tabs by a
>> >> single space.  Please fix this or attach the patch instead.
>> >>
>> >> > Index: ChangeLog
>> >> > ===
>> >> > --- ChangeLog (revision 272668)
>> >> > +++ ChangeLog (working copy)
>> >> > @@ -1,3 +1,8 @@
>> >> > +2019-06-27  Hongtao Liu  
>> >> > +
>> >> > + * doc/sourcebuild.texi: Document new effective target keyword
>> >> > + avx512vp2intersect.
>> >>
>> >> Please include the sections you're modifying, something like
>> >>
>> >> * doc/sourcebuild.texi (Effective-Target Keywords, Other
>> >> hardware attributes): Document avx512vp2intersect.
>> >>
>> >> And please don't include the ChangeLog in the patch, but include it in
>> >> the mail proper: it won't apply due to date and context changes anyway.
>> >> Best review https://gcc.gnu.org/contribute.html where this is documented
>> >> besides other points of patch submission.
>> >>
>> >> Besides, it's most likely useful to also review the GNU Coding
>> >> Standards, too, not only for ChangeLog formatting.
>> >>
>> >> > Index: testsuite/ChangeLog
>> >> > ===
>> >> > --- testsuite/ChangeLog (revision 272668)
>> >> > +++ testsuite/ChangeLog (working copy)
>> >> > @@ -1,3 +1,11 @@
>> >> > +2019-06-27  Hongtao Liu  
>> >> > +
>> >> > + * lib/target-supports.exp: Add
>> >> > + check_effective_target_avx512vp2intersect.
>> >>
>> >> Use
>> >>
>> >> * lib/target-supports.exp
>> >> (check_effective_target_avx512vp2intersect): New proc.
>> >>
>> >> > + * gcc.target/i386/avx512vp2intersect-2intersect-1b.c: Add
>> >> > + dg-require-effective-target avx512vp2intersect.
>> >>
>> >> Better:
>> >>
>> >> * gcc.target/i386/avx512vp2intersect-2intersect-1b.c: Require
>> >> avx512vp2intersect.
>> >>
>> >> but that's a matter of preference.
>> >>
>> >> > Index: testsuite/gcc.target/i386/avx512vp2intersect-2intersect-1b.c
>> >> > ===
>> >> > --- testsuite/gcc.target/i386/avx512vp2intersect-2intersect-1b.c
>> >> > (revision 272668)
>> >> > +++ testsuite/gcc.target/i386/avx512vp2intersect-2intersect-1b.c
>> >> > (working copy)
>> >> > @@ -1,5 +1,6 @@
>> >> >  /* { dg-do run } */
>> >> >  /* { dg-options "-O2 -mavx512vp2intersect" } */
>> >> > +/* { dg-require-effective-target "avx512vp2intersect" } */
>> >>
>> >> No need to quote avx512vp2intersect here and in the next test.
>> >>
>> >> Ok with those nits fixed.
>> >>
>> > Yes, thanks a lot.
>> >> Thanks.
>> >> Rainer
>> >>
>> >> --
>> >> -
>> >> Rainer Orth, Center for Biotechnology, Bielefeld University
>> >
>> > Ok for trunk?
>>
>> ENOPATCH
> Sorry, Here is the patch.
>>
>> --
>> -
>> Rainer Orth, Center for Biotechnology, Bielefeld University
>
>
> Changelog
>
> gcc/
> +2019-06-27  Hongtao Liu  
> +
> + * doc/sourcebuild.texi (Effective-Target Keywords, Other
> + hardware attributes): Document avx512vp2intersect.
> +
>
> gcc/testsuite/
> +2019-06-27  Hongtao Liu  
> +
> + * lib/target-supports.exp
> + (check_effective_target_avx512vp2intersect): New proc.
> + * gcc.target/i386/avx512vp2intersect-2intersect-1b.c: Add
> + dg-require-effective-target avx512vp2intersect.
> + * gcc.target/i386/avx512vp2intersect-2intersectvl-1b.c: Ditto.
> +

unfortunately, the testcases are still not right.  While with gas 2.32
they now come up as UNSUPPORTED, I've now tried a mainline bootstrap on
i386-pc-solaris2.11 with gas from binutils master.  Doing so, I get

+FAIL: gcc.target/i386/avx512vp2intersect-2intersect-1b.c execution test
+FAIL: gcc.target/i386/avx512vp2intersect-2intersectvl-1b.c execution test

for both 32 and 64-bit, and there are similar results on
gcc-testresults for x86_64-pc-linux-gnu.

Running one of the testcases under gdb shows

Thread 2 received signal SIGILL, Illegal instruction.
[Switching to Thread 1 (LWP 1)]
0x08050d89 in do_test ()
1: x/i $pc
=> 0x8050d89 :  (bad)  

or with objdump 2.32:

 8050d89:   62 f2 ff 48 68  (bad)  

Using objdump from binutils master shows

 8050d89:   62 f2 ff 48 68 05 80vp2intersectq 0x8050a80,%zmm0,%k0

Currently, the testcases only check for AVX512F (which the machine in
question supports: Xeon Gold 6132), while they need to check for
AVX512VP2INTERSECT to avoid this.

The following patch does this; tested on i386-pc-solaris2.11 with gas
2.32.51 both 32 and 64-bit where the tests PASS.

Re: [patch][aarch64]: add intrinsics for vld1(q)_x4 and vst1(q)_x4

[Richard Earnshaw (lists)]

On 18/07/2019 18:18, James Greenhalgh wrote:

On Mon, Jun 10, 2019 at 06:21:05PM +0100, Sylvia Taylor wrote:

Greetings,

This patch adds the intrinsic functions for:
- vld1__x4
- vst1__x4
- vld1q__x4
- vst1q__x4

Bootstrapped and tested on aarch64-none-linux-gnu.

Ok for trunk? If yes, I don't have any commit rights, so can someone
please commit it on my behalf.


Hi,

I'm concerned by this strategy for implementing the arm_neon.h builtins:


+__extension__ extern __inline int8x8x4_t
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
+vld1_s8_x4 (const int8_t *__a)
+{
+  union { int8x8x4_t __i; __builtin_aarch64_simd_xi __o; } __au;
+  __au.__o
+= __builtin_aarch64_ld1x4v8qi ((const __builtin_aarch64_simd_qi *) __a);
+  return __au.__i;
+}


As far as I know this is undefined behaviour in C++11. This was the best
resource I could find pointing to the relevant standards paragraphs.

   
https://stackoverflow.com/questions/11373203/accessing-inactive-union-member-and-undefined-behavior

That said, GCC explicitly allows it, so maybe this is fine?

   
https://gcc.gnu.org/onlinedocs/gcc-9.1.0/gcc/Optimize-Options.html#Type-punning

Can anyone from the languages side chime in on whether we're exposing
undefined behaviour (in either C or C++) here?


Yes, this is a GNU extension.  My only question is whether or not this 
can be disabled within GCC if you're trying to check for strict 
standards conformance of your code?  And if so, is there a way of making 
sure that this header still works in that case?  A number of GNU 
extensions can be protected with __extension__ but it's not clear how 
that could be applied in this case.  Perhaps the outer __extension__ on 
the function will already do that.


R.





Thanks,
James





Cheers,
Syl

gcc/ChangeLog:

2019-06-10  Sylvia Taylor  

* config/aarch64/aarch64-simd-builtins.def:
(ld1x4): New.
(st1x4): Likewise.
* config/aarch64/aarch64-simd.md:
(aarch64_ld1x4): New pattern.
(aarch64_st1x4): Likewise.
(aarch64_ld1_x4_): Likewise.
(aarch64_st1_x4_): Likewise.
* config/aarch64/arm_neon.h:
(vld1_s8_x4): New function.
(vld1q_s8_x4): Likewise.
(vld1_s16_x4): Likewise.
(vld1q_s16_x4): Likewise.
(vld1_s32_x4): Likewise.
(vld1q_s32_x4): Likewise.
(vld1_u8_x4): Likewise.
(vld1q_u8_x4): Likewise.
(vld1_u16_x4): Likewise.
(vld1q_u16_x4): Likewise.
(vld1_u32_x4): Likewise.
(vld1q_u32_x4): Likewise.
(vld1_f16_x4): Likewise.
(vld1q_f16_x4): Likewise.
(vld1_f32_x4): Likewise.
(vld1q_f32_x4): Likewise.
(vld1_p8_x4): Likewise.
(vld1q_p8_x4): Likewise.
(vld1_p16_x4): Likewise.
(vld1q_p16_x4): Likewise.
(vld1_s64_x4): Likewise.
(vld1_u64_x4): Likewise.
(vld1_p64_x4): Likewise.
(vld1q_s64_x4): Likewise.
(vld1q_u64_x4): Likewise.
(vld1q_p64_x4): Likewise.
(vld1_f64_x4): Likewise.
(vld1q_f64_x4): Likewise.
(vst1_s8_x4): Likewise.
(vst1q_s8_x4): Likewise.
(vst1_s16_x4): Likewise.
(vst1q_s16_x4): Likewise.
(vst1_s32_x4): Likewise.
(vst1q_s32_x4): Likewise.
(vst1_u8_x4): Likewise.
(vst1q_u8_x4): Likewise.
(vst1_u16_x4): Likewise.
(vst1q_u16_x4): Likewise.
(vst1_u32_x4): Likewise.
(vst1q_u32_x4): Likewise.
(vst1_f16_x4): Likewise.
(vst1q_f16_x4): Likewise.
(vst1_f32_x4): Likewise.
(vst1q_f32_x4): Likewise.
(vst1_p8_x4): Likewise.
(vst1q_p8_x4): Likewise.
(vst1_p16_x4): Likewise.
(vst1q_p16_x4): Likewise.
(vst1_s64_x4): Likewise.
(vst1_u64_x4): Likewise.
(vst1_p64_x4): Likewise.
(vst1q_s64_x4): Likewise.
(vst1q_u64_x4): Likewise.
(vst1q_p64_x4): Likewise.
(vst1_f64_x4): Likewise.
(vst1q_f64_x4): Likewise.

gcc/testsuite/ChangeLog:

2019-06-10  Sylvia Taylor  

* gcc.target/aarch64/advsimd-intrinsics/vld1x4.c: New test.
* gcc.target/aarch64/advsimd-intrinsics/vst1x4.c: New test.

Re: [Contrib PATCH] Add scripts to convert GCC repo from SVN to Git

[Maxim Kuvyrkov]

> On Aug 5, 2019, at 11:24 AM, Maxim Kuvyrkov  wrote:
> 
> 
>> On Aug 2, 2019, at 11:41 AM, Maxim Kuvyrkov  
>> wrote:
>> 
>>> On Aug 1, 2019, at 11:43 PM, Jason Merrill  wrote:
>>> 
> ...
 Unfortunately, current mirror does not and could not account for rewrites 
 of SVN commit log messages.  For trunk the histories of diverge in 2008 
 due to commit message change of r138154.  This is not a single occurrence; 
 I've compared histories only of trunk and gcc-6-branch, and both had 
 commit message change (for gcc-6-branch see r259978).
 
 It's up to the community is to weigh pros and cons of re-using existing 
 GCC mirror as conversion base vs regenerating history from scratch:
 
 Pros of using GCC mirror:
 + No need to rebase public git-only branches
 + No need to rebase private branches
 + No need to rebase current clones, checkouts, work-in-progress trees
 
 Cons of using GCC mirror:
 - Poor author / committer IDs (this breaks patch statistics software)
 - Several commit messages will not be the current "fixed" version
 
 Thoughts?
>>> 
>>> I'm still inclined to stick with the mirror.  I would expect patch
>>> statistics software to be able to be taught about multiple addresses
>>> for the same person.
>> 
>> Patch tracking software breaks on emails like 
>>  , where 
>> 38bc75d-0d04-0410-961f-82ee72b054a4 is not a reasonable domain name.
>> 
>> For completeness, I'll generate and upload a repo based on current mirror 
>> with all branches and tags converted.
> 
> Yeah, this didn't worked as well as I hoped.  Current gcc git mirror has 
> wrong history for branches that followed scenario:
> 1. create $branch from $base at revision N
> 2. commit WORK on $branch
> 3. delete $branch
> 4. create $branch from $base at revision N+M
> 5. rebase WORK on current $branch
> 
> Current mirror connects histories of two versions of $branch, and we get 
> wrong history.  In step (4) instead of plain history of $base we get a commit 
> merging histories of $branch just before deletion and $base at revision N+M.
> 
> There are many branches like this, e.g., branches/gccgo.

I've setup uploads and updates of fully converted GCC history (all branches and 
all tags) in 3 flavors.  These will be updated roughly hourly.

1. https://git-us.linaro.org/people/maxim-kuvyrkov/gcc-pretty.git/
This is a fresh conversion from scratch with "pretty" authors.

2. https://git.linaro.org/people/maxim-kuvyrkov/gcc-mirror.git/
This is a close match to current GCC mirror.  Trunk and gcc-*-branch branches 
are imported from the mirror, and the rest is reconstructed starting from the 
imported branches.

3. https://git-us.linaro.org/people/maxim-kuvyrkov/gcc-raw.git/
This is a fresh conversion from scratch with no author rewrites.

--
Maxim Kuvyrkov
www.linaro.org

[Patch, DWARF] Ignore address spaces for get_nearest_type_subqualifiers

[SenthilKumar.Selvaraj]

Hi,

  r254484 explicitly added previously ignored address space 
  qualifiers, when
  emitting DWARF dies for types.

  Adding address spaces to cv_qual_mask, however, breaks
  nearest_type_subqualifier, which uses the number of bits set 
  (i.e
  popcount_hwi) to find the type variant that has the largest 
  strict subset of a
  given type's qualifiers. Address spaces are simply represented 
  as 16 bits, and
  therefore the number of set bits there has no relevance when 
  finding
  nearest_type_subqualifier. This also causes 
  gcc.target/avr/pr52472.c to ICE.

  This patch fixes that by excluding address space quals from 
  cv_qual_mask.
  Bootstrap and reg test for x86_64-linux in progress. Ok to 
  commit to trunk
  if the tests pass?

Regards
Senthil

gcc/ChangeLog:

2019-08-06  Senthil Kumar Selvaraj 


* dwarf2out.c (modified_type_die): CLEAR_QUAL_ADDR_SPACE from
cv_qual_mask before calling get_nearest_type_subqualifiers.


diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index aa7fd7eb465..157c970d729 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -13295,8 +13295,9 @@ modified_type_die (tree type, int 
cv_quals, bool reverse,
   /* Determine a lesser qualified type that most closely 
   matches
 this one.  Then generate DW_TAG_* entries for the remaining
 qualifiers.  */
+  int nearest_qual_mask = CLEAR_QUAL_ADDR_SPACE 
(cv_qual_mask);
   sub_quals = get_nearest_type_subqualifiers (type, cv_quals,
- cv_qual_mask);
+ nearest_qual_mask);
   if (sub_quals && use_debug_types)
{
  bool needed = false;

[committed] Handle C++ random access iterators in loop construct, other C++ loop construct fixes

[Jakub Jelinek]

Hi!

The following patch fixes handling of C++ class random access iterators
and fixes also other issues found during that with OMP_LOOP C++
genericization and gimplification.

Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.

2019-08-06  Jakub Jelinek  

* tree.h (OMP_CLAUSE_LASTPRIVATE_TASKLOOP_IV): Rename to ...
(OMP_CLAUSE_LASTPRIVATE_LOOP_IV): ... this.  Adjust comment.
* gimplify.c (gimple_add_tmp_var): In SIMD contexts, turn addressable
new vars into GOVD_PRIVATE rather than GOVD_LOCAL.
(gimplify_omp_for): Don't do C++ random access iterator clause
adjustments on combined constructs from OMP_LOOP.  For OMP_LOOP,
don't predetermine the artificial iterator in case of C++ random
access iterators as lastprivate, but private.  For OMP_LOOP, force
bind expr around simd body and force for_pre_body before the
construct.  Use OMP_CLAUSE_LASTPRIVATE_LOOP_IV instead of
OMP_CLAUSE_LASTPRIVATE_TASKLOOP_IV.
(gimplify_omp_loop): Add firstprivate clauses on OMP_PARALLEL for
diff var of C++ random access iterators.  Handle
OMP_CLAUSE_FIRSTPRIVATE.  For OMP_CLAUSE_LASTPRIVATE_LOOP_IV, if
not outermost also add OMP_CLAUSE_FIRSTPRIVATE, and in both cases
clear OMP_CLAUSE_LASTPRIVATE_LOOP_IV on the lastprivate clause
on the OMP_FOR and OMP_DISTRIBUTE constructs if any.
* omp-low.c (lower_rec_input_clauses): For
OMP_CLAUSE_LASTPRIVATE_LOOP_IV on simd copy construct the private
variables instead of default constructing them.
(lower_lastprivate_clauses): Use OMP_CLAUSE_LASTPRIVATE_LOOP_IV
instead of OMP_CLAUSE_LASTPRIVATE_TASKLOOP_IV and move the
is_taskloop_ctx check from the assert to the guarding condition.
gcc/cp/
* parser.c (cp_parser_omp_for_loop): For OMP_LOOP, ignore parallel
clauses and predetermine iterator as lastprivate.
* semantics.c (handle_omp_for_class_iterator): Use
OMP_CLAUSE_LASTPRIVATE_LOOP_IV instead of
OMP_CLAUSE_LASTPRIVATE_TASKLOOP_IV, set it for lastprivate also
on OMP_LOOP construct.  If a clause is missing for class iterator
on OMP_LOOP, add firstprivate clause, and if there is private
clause, turn it into firstprivate too.
(finish_omp_for): Formatting fix.  For OMP_LOOP, adjust
OMP_CLAUSE_LASTPRIVATE_LOOP_IV clause CP_CLAUSE_INFO, so that it
uses copy ctor instead of default ctor.
* cp-gimplify.c (cp_gimplify_expr): Handle OMP_LOOP like
OMP_DISTRIBUTE etc.
(cp_fold_r): Likewise.
(cp_genericize_r): Likewise.
(cxx_omp_finish_clause): Also finish lastprivate clause with
OMP_CLAUSE_LASTPRIVATE_LOOP_IV flag.
* pt.c (tsubst_omp_clauses): Handle OMP_CLAUSE_BIND.
(tsubst_omp_for_iterator): For OMP_LOOP, ignore parallel
clauses and predetermine iterator as lastprivate.
* constexpr.c (potential_constant_expression_1): Handle OMP_LOOP
like OMP_DISTRIBUTE etc.
libgomp/
* testsuite/libgomp.c++/loop-13.C: New test.
* testsuite/libgomp.c++/loop-14.C: New test.
* testsuite/libgomp.c++/loop-15.C: New test.

--- gcc/tree.h.jj   2019-08-05 09:58:05.060531603 +0200
+++ gcc/tree.h  2019-08-05 14:13:40.005834106 +0200
@@ -1517,10 +1517,11 @@ class auto_suppress_location_wrappers
 #define OMP_CLAUSE_LASTPRIVATE_GIMPLE_SEQ(NODE) \
   (OMP_CLAUSE_CHECK (NODE))->omp_clause.gimple_reduction_init
 
-/* True if a LASTPRIVATE clause is for a C++ class IV on taskloop construct
-   (thus should be lastprivate on the outer taskloop and firstprivate on
-   task).  */
-#define OMP_CLAUSE_LASTPRIVATE_TASKLOOP_IV(NODE) \
+/* True if a LASTPRIVATE clause is for a C++ class IV on taskloop or
+   loop construct (thus should be lastprivate on the outer taskloop and
+   firstprivate on task for the taskloop construct and carefully handled
+   for loop construct).  */
+#define OMP_CLAUSE_LASTPRIVATE_LOOP_IV(NODE) \
   TREE_PROTECTED (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_LASTPRIVATE))
 
 /* True if a LASTPRIVATE clause has CONDITIONAL: modifier.  */
--- gcc/gimplify.c.jj   2019-08-05 09:57:55.603676252 +0200
+++ gcc/gimplify.c  2019-08-05 14:15:52.821843830 +0200
@@ -775,14 +775,27 @@ gimple_add_tmp_var (tree tmp)
   if (gimplify_omp_ctxp)
{
  struct gimplify_omp_ctx *ctx = gimplify_omp_ctxp;
+ int flag = GOVD_LOCAL;
  while (ctx
 && (ctx->region_type == ORT_WORKSHARE
 || ctx->region_type == ORT_TASKGROUP
 || ctx->region_type == ORT_SIMD
 || ctx->region_type == ORT_ACC))
-   ctx = ctx->outer_context;
+   {
+ if (ctx->region_type == ORT_SIMD
+ && TREE_ADDRESSABLE (tmp)
+ && !TREE_STATIC (tmp))
+   {
+ if (TREE_CODE (DECL_SIZE_U

Re: [PATCH] Add operator new/delete to cgraph_node::dump.

[Jan Hubicka]

> Hi.
> 
> The patch is about new/delete operator flags that I would
> like to see in cgraph::dump.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Ready to be installed?
OK,
Honza
> Thanks,
> Martin
> 
> gcc/ChangeLog:
> 
> 2019-08-05  Martin Liska  
> 
>   * cgraph.c (cgraph_node::dump): Dump DECL_IS_OPERATOR_NEW_P
>   and DECL_IS_OPERATOR_DELETE_P.
> ---
>  gcc/cgraph.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> 

> diff --git a/gcc/cgraph.c b/gcc/cgraph.c
> index bed407e5766..ed46d81a513 100644
> --- a/gcc/cgraph.c
> +++ b/gcc/cgraph.c
> @@ -2080,6 +2080,11 @@ cgraph_node::dump (FILE *f)
>  fprintf (f, " optimize_size");
>if (parallelized_function)
>  fprintf (f, " parallelized_function");
> +  if (DECL_IS_OPERATOR_NEW_P (decl))
> +fprintf (f, " operator_new");
> +  if (DECL_IS_OPERATOR_DELETE_P (decl))
> +fprintf (f, " operator_delete");
> +
>  
>fprintf (f, "\n");
>  
> 

[PATCH] Add operator new/delete to cgraph_node::dump.

[Martin Liška]

Hi.

The patch is about new/delete operator flags that I would
like to see in cgraph::dump.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2019-08-05  Martin Liska  

* cgraph.c (cgraph_node::dump): Dump DECL_IS_OPERATOR_NEW_P
and DECL_IS_OPERATOR_DELETE_P.
---
 gcc/cgraph.c | 5 +
 1 file changed, 5 insertions(+)


diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index bed407e5766..ed46d81a513 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -2080,6 +2080,11 @@ cgraph_node::dump (FILE *f)
 fprintf (f, " optimize_size");
   if (parallelized_function)
 fprintf (f, " parallelized_function");
+  if (DECL_IS_OPERATOR_NEW_P (decl))
+fprintf (f, " operator_new");
+  if (DECL_IS_OPERATOR_DELETE_P (decl))
+fprintf (f, " operator_delete");
+
 
   fprintf (f, "\n");