[PING][PATCH] constrain one character optimization to one character stores (PR 90989)

2019-07-08 Thread Martin Sebor

Ping: https://gcc.gnu.org/ml/gcc-patches/2019-06/msg01506.html

Jeff (et al.), do you have any outstanding questions/concerns
about the patch?

Martin

On 6/27/19 4:30 PM, Jeff Law wrote:

On 6/27/19 12:40 PM, Richard Biener wrote:

On June 27, 2019 7:04:32 PM GMT+02:00, Jakub Jelinek  wrote:

On Thu, Jun 27, 2019 at 10:58:25AM -0600, Martin Sebor wrote:

The LHS is unsigned short so handle_char_store would not be called
because of the check in the caller.  You would need something like:

   MEM  [(char *)] = { 'a', 'b' };


This is invalid, because the rhs is non-empty CONSTRUCTOR that doesn't
have
VECTOR_TYPE - verify_gimple_assign_single has:
case CONSTRUCTOR:
  if (TREE_CODE (rhs1_type) == VECTOR_TYPE)
...
  else if (CONSTRUCTOR_NELTS (rhs1) != 0)
{
  error ("non-vector %qs with elements", code_name);
  debug_generic_stmt (rhs1);
  return true;
}

But
  MEM  [(char *)] = MEM  [(char *)"ab"];
is valid.  Or = {} would be valid too, even for array stores.


And you can of course write GIMPLE unit tests for the pass using the GIMPLE FE.

Yea.  And this may ultimately be the way we should think about testing
this kind of stuff where GIMPLE might allow something that is impossible
to express in C or C++.

Realistically I don't think any of us can be expected to know the dusty
corners of every language.  But we ought to be able to reason about
what's valid gimple and write tests using the gimple front-end to verify
how a particular construct would be handled.

jeff





Re: [PATCH V4] PR88497 - Extend reassoc for vector bit_field_ref

2019-07-08 Thread Kewen.Lin
Hi Segher,

on 2019/7/9 上午12:32, Segher Boessenkool wrote:
> Hi Kewen,
> 
> On Mon, Jul 08, 2019 at 04:07:00PM +0800, Kewen.Lin wrote:
>> gcc/ChangeLog
> 
> (You have trailing spaces in the changelog, fwiw).
> 

Thanks for catching!

>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr88497-1.c
>> @@ -0,0 +1,60 @@
>> +/* { dg-do run } */
>> +/* { dg-require-effective-target vect_double } */
>> +/* { dg-require-effective-target powerpc_vsx_ok { target { powerpc*-*-* } } 
>> } */
> 
> For "dg-do run" tests, you need "powerpc_vsx_hw".  "_ok" only tests if
> the assembler can handle VSX instructions, not whether the test system
> can run them.  (powerpc_vsx_ok is what you need for "dg-do assemble" or
> "dg-do link" tests.  It also tests if you can use -mvsx, but that doesn't
> do what you might hope it does: you can use -mvsx together with a -mcpu=
> that doesn't support VSX, for example).
> 

Thanks, I will update it.  But sorry that I can't find "powerpc_vsx_hw" but 
"vsx_hw_available".  I guess it's the one you are referring to?  And I happened
to find the vect_double will force powerpc to check vsx_hw_available.

This reminds me I should use sse2 instead of sse2-runtime for case 2-5.

>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr88497-2.c
>> @@ -0,0 +1,37 @@
>> +/* { dg-do compile } */
>> +/* { dg-require-effective-target vect_float } */
>> +/* { dg-require-effective-target powerpc_altivec_ok { target { powerpc*-*-* 
>> } } } */
> 
> This one is fine, and the rest of the tests as well.
> 
> 
> Segher
> 



Re: [PATCH 1/2] Come up with function_decl_type and use it in tree_function_decl.

2019-07-08 Thread Marc Glisse

On Mon, 8 Jul 2019, Martin Liška wrote:

The patch apparently has DECL_IS_OPERATOR_DELETE only on the 
replaceable global deallocation functions, not all delete operators, 
contrary to DECL_IS_OPERATOR_NEW, so the name is misleading. On the 
other hand, those seem to be the ones for which the optimization is 
legal (well, not quite, the rules are in terms of operator new, and I 
am not sure how well operator delete has to match, but close enough).


Are you talking about this location where we set OPERATOR_NEW:
https://github.com/gcc-mirror/gcc/blob/master/gcc/cp/decl.c#L13643
?

That's the only place where we set OPERATOR_NEW flag and not OPERATOR_DELETE.


Yes, I think that's the place.

Again, not setting DECL_IS_OPERATOR_DELETE on local operator delete
seems misleading, but setting it would let us optimize in cases where we
are not really allowed to. Maybe just rename your macro to
DECL_IS_GLOBAL_OPERATOR_DELETE?

--
Marc Glisse


Re: [PATCH 2/3] change class-key of PODs to struct and others to class (PR 61339)

2019-07-08 Thread Martin Sebor

Hopefully with the right patch this time (thanks Jon).

On 7/8/19 4:00 PM, Martin Sebor wrote:

The attached patch changes the class-key of class definitions that
satisfy the requirements on a POD struct to 'struct', and that of
struct definitions that aren't POD to class, according to the GCC
coding convention.  The patch is also prerequisite for GCC being
able to compile cleanly with -Wmismatched-tags.

I made the changes building GCC with -Wstruct-not-pod and
-Wclass-is-pod enabled, scanning the build log for instances
of each warning, and using a script replacing the class-key
as necessary and adjusting the access of the members declared
immediately after the class-head.

Martin




gcc-61339-pod-cleanup.diff.gz
Description: application/gzip


[PATCH] Improve scan_operand_equal_p

2019-07-08 Thread Jakub Jelinek
Hi!

The 4 testcases below weren't vectorized, because while
tree-vect-data-refs.c now allows more forms of simd lane access,
scan_operand_equal_p didn't allow combining them together.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
committed to trunk.

2019-07-08  Jakub Jelinek  

* tree-vect-stmts.c (scan_operand_equal_p): Look through MEM_REF
with SSA_NAME address of POINTER_PLUS_EXPR.  Handle MULT_EXPR
and casts in offset when different, both through gimple stmts
and through trees.  Rewritten using loops to minimize code duplication
for each operand.

* g++.dg/vect/simd-6.cc: Replace xfail with target x86.
* g++.dg/vect/simd-9.cc: Likewise.

* testsuite/libgomp.c++/scan-13.C: Replace xfail with target x86.
* testsuite/libgomp.c++/scan-16.C: Likewise.

--- gcc/tree-vect-stmts.c.jj2019-07-04 09:24:28.595303590 +0200
+++ gcc/tree-vect-stmts.c   2019-07-08 20:59:52.376285636 +0200
@@ -6334,30 +6334,88 @@ get_group_alias_ptr_type (stmt_vec_info
 static bool
 scan_operand_equal_p (tree ref1, tree ref2)
 {
-  machine_mode mode1, mode2;
-  poly_int64 bitsize1, bitsize2, bitpos1, bitpos2;
-  tree offset1, offset2;
-  int unsignedp1, unsignedp2, reversep1, reversep2;
-  int volatilep1 = 0, volatilep2 = 0;
-  tree base1 = get_inner_reference (ref1, , , ,
-   , , ,
-   );
-  tree base2 = get_inner_reference (ref2, , , ,
-   , , ,
-   );
-  if (reversep1 || reversep2 || volatilep1 || volatilep2)
-return false;
-  if (!operand_equal_p (base1, base2, 0))
-return false;
-  if (maybe_ne (bitpos1, 0) || maybe_ne (bitpos2, 0))
-return false;
-  if (maybe_ne (bitsize1, bitsize2))
+  tree ref[2] = { ref1, ref2 };
+  poly_int64 bitsize[2], bitpos[2];
+  tree offset[2], base[2];
+  for (int i = 0; i < 2; ++i)
+{
+  machine_mode mode;
+  int unsignedp, reversep, volatilep = 0;
+  base[i] = get_inner_reference (ref[i], [i], [i],
+[i], , ,
+, );
+  if (reversep || volatilep || maybe_ne (bitpos[i], 0))
+   return false;
+  if (TREE_CODE (base[i]) == MEM_REF
+ && offset[i] == NULL_TREE
+ && TREE_CODE (TREE_OPERAND (base[i], 0)) == SSA_NAME)
+   {
+ gimple *def_stmt = SSA_NAME_DEF_STMT (TREE_OPERAND (base[i], 0));
+ if (is_gimple_assign (def_stmt)
+ && gimple_assign_rhs_code (def_stmt) == POINTER_PLUS_EXPR
+ && TREE_CODE (gimple_assign_rhs1 (def_stmt)) == ADDR_EXPR
+ && TREE_CODE (gimple_assign_rhs2 (def_stmt)) == SSA_NAME)
+   {
+ if (maybe_ne (mem_ref_offset (base[i]), 0))
+   return false;
+ base[i] = TREE_OPERAND (gimple_assign_rhs1 (def_stmt), 0);
+ offset[i] = gimple_assign_rhs2 (def_stmt);
+   }
+   }
+}
+
+  if (!operand_equal_p (base[0], base[1], 0))
 return false;
-  if (offset1 != offset2
-  && (!offset1
- || !offset2
- || !operand_equal_p (offset1, offset2, 0)))
+  if (maybe_ne (bitsize[0], bitsize[1]))
 return false;
+  if (offset[0] != offset[1])
+{
+  if (!offset[0] || !offset[1])
+   return false;
+  if (!operand_equal_p (offset[0], offset[1], 0))
+   {
+ tree step[2];
+ for (int i = 0; i < 2; ++i)
+   {
+ step[i] = integer_one_node;
+ if (TREE_CODE (offset[i]) == SSA_NAME)
+   {
+ gimple *def_stmt = SSA_NAME_DEF_STMT (offset[i]);
+ if (is_gimple_assign (def_stmt)
+ && gimple_assign_rhs_code (def_stmt) == MULT_EXPR
+ && (TREE_CODE (gimple_assign_rhs2 (def_stmt))
+ == INTEGER_CST))
+   {
+ step[i] = gimple_assign_rhs2 (def_stmt);
+ offset[i] = gimple_assign_rhs1 (def_stmt);
+   }
+   }
+ else if (TREE_CODE (offset[i]) == MULT_EXPR)
+   {
+ step[i] = TREE_OPERAND (offset[i], 1);
+ offset[i] = TREE_OPERAND (offset[i], 0);
+   }
+ tree rhs1 = NULL_TREE;
+ if (TREE_CODE (offset[i]) == SSA_NAME)
+   {
+ gimple *def_stmt = SSA_NAME_DEF_STMT (offset[i]);
+ if (gimple_assign_cast_p (def_stmt))
+   rhs1 = gimple_assign_rhs1 (def_stmt);
+   }
+ else if (CONVERT_EXPR_P (offset[i]))
+   rhs1 = TREE_OPERAND (offset[i], 0);
+ if (rhs1
+ && INTEGRAL_TYPE_P (TREE_TYPE (rhs1))
+ && INTEGRAL_TYPE_P (TREE_TYPE (offset[i]))
+ && (TYPE_PRECISION (TREE_TYPE (offset[i]))
+ >= TYPE_PRECISION (TREE_TYPE (rhs1
+

Re: [patch, c++ openmp] Improve diagnostics for unmappable types

2019-07-08 Thread Jakub Jelinek
On Thu, Jul 04, 2019 at 12:44:32PM +0100, Andrew Stubbs wrote:
> On 03/07/2019 18:58, Jason Merrill wrote:
> > OK, thanks.
> 
> Committed.

This broke following testcase.
error_mark_node type isn't really incomplete, it is errorneous, doesn't have
TYPE_MAIN_DECL and we should have diagnosed it earlier, so it makes no sense
to emit extra explanation messages.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
committed to trunk.

2019-07-08  Jakub Jelinek  

PR c++/91110
* decl2.c (cp_omp_mappable_type_1): Don't emit any note for
error_mark_node type.

* g++.dg/gomp/pr91110.C: New test.

--- gcc/cp/decl2.c.jj   2019-07-04 23:39:02.579106113 +0200
+++ gcc/cp/decl2.c  2019-07-08 13:22:52.552898230 +0200
@@ -1416,7 +1416,7 @@ cp_omp_mappable_type_1 (tree type, bool
   /* Mappable type has to be complete.  */
   if (type == error_mark_node || !COMPLETE_TYPE_P (type))
 {
-  if (notes)
+  if (notes && type != error_mark_node)
{
  tree decl = TYPE_MAIN_DECL (type);
  inform ((decl ? DECL_SOURCE_LOCATION (decl) : input_location),
--- gcc/testsuite/g++.dg/gomp/pr91110.C.jj  2019-07-08 13:29:43.803163534 
+0200
+++ gcc/testsuite/g++.dg/gomp/pr91110.C 2019-07-08 13:29:17.550593456 +0200
@@ -0,0 +1,11 @@
+// PR c++/91110
+// { dg-do compile }
+
+void
+foo ()
+{
+  X b[2];  // { dg-error "'X' was not declared in this scope" }
+  b[0] = 1;// { dg-error "'b' was not declared in this scope" }
+  #pragma omp target map(to: b)// { dg-error "'b' does not have a 
mappable type in 'map' clause" }
+  ;
+}


Jakub


Re: [PATCH] Add generic support for "noinit" attribute

2019-07-08 Thread Martin Sebor

On 7/8/19 5:10 AM, Christophe Lyon wrote:

On Sat, 6 Jul 2019 at 19:57, Martin Sebor  wrote:


On 7/4/19 9:27 AM, Christophe Lyon wrote:

Hi,

Similar to what already exists for TI msp430 or in TI compilers for
arm, this patch adds support for the "noinit" attribute.

It is convenient for embedded targets where the user wants to keep the
value of some data when the program is restarted: such variables are
not zero-initialized. It is mostly a helper/shortcut to placing
variables in a dedicated section.

It's probably desirable to add the following chunk to the GNU linker:
diff --git a/ld/emulparams/armelf.sh b/ld/emulparams/armelf.sh
index 272a8bc..9555cec 100644
--- a/ld/emulparams/armelf.sh
+++ b/ld/emulparams/armelf.sh
@@ -10,7 +10,19 @@ OTHER_TEXT_SECTIONS='*(.glue_7t) *(.glue_7)
*(.vfp11_veneer) *(.v4_bx)'
   OTHER_BSS_SYMBOLS="${CREATE_SHLIB+PROVIDE (}__bss_start__ =
.${CREATE_SHLIB+)};"
   OTHER_BSS_END_SYMBOLS="${CREATE_SHLIB+PROVIDE (}_bss_end__ =
.${CREATE_SHLIB+)}; ${CREATE_SHLIB+PROVIDE (}__bss_end__ =
.${CREATE_SHLIB+)};"
   OTHER_END_SYMBOLS="${CREATE_SHLIB+PROVIDE (}__end__ = .${CREATE_SHLIB+)};"
   -OTHER_SECTIONS='.note.gnu.arm.ident 0 : { KEEP (*(.note.gnu.arm.ident)) }'
   +OTHER_SECTIONS='
   +.note.gnu.arm.ident 0 : { KEEP (*(.note.gnu.arm.ident)) }
   +  /* This section contains data that is not initialised during load
   + *or* application reset.  */
   +   .noinit (NOLOAD) :
   +   {
   + . = ALIGN(2);
   + PROVIDE (__noinit_start = .);
   + *(.noinit)
   + . = ALIGN(2);
   + PROVIDE (__noinit_end = .);
   +   }
   +'

so that the noinit section has the "NOLOAD" flag.

I'll submit that part separately to the binutils project if OK.

However, I'm not sure for which other targets (beyond arm), I should
update the linker scripts.

I left the new testcase in gcc.target/arm, guarded by
dg-do run { target { *-*-eabi } }
but since this attribute is now generic, I suspect I should move the
test to some other place. But then, which target selector should I
use?

Finally, I tested on arm-eabi, but not on msp430 for which I do not
have the environment, so advice from msp430 maintainers is
appreciated. Since msp430 does not use the same default helpers as
arm, I left the "noinit" handling code in place, to avoid changing
other generic functions which I don't know how to test
(default_select_section, default_section_type_flags).



Since the section attribute is mutually exclusive with noinit,
defining an attribute_exclusions array with the mutually exclusive
attributes and setting the last member of the corresponding
c_common_attribute_table element to that array (and updating
the arrays for the other mutually exclusive attributes) would
be the expected way to express that constraint:

+  { "noinit", 0, 0, true,  false, false, false,
+ handle_noinit_attribute, NULL },
 
This should also make it possible to remove the explicit handling
from handle_section_attribute and handle_noinit_attribute.  If
that's not completely possible then in the following please be
sure to quote the names of the attributes:

@@ -1912,6 +1915,13 @@ handle_section_attribute (tree *node, tree
ARG_UNUSED (name), tree args,
 goto fail;
   }

+  if (DECL_P (decl) && lookup_attribute ("noinit", DECL_ATTRIBUTES
(decl)) != NULL_TREE)
+{
+  warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+   "section attribute cannot be applied to variables with noinit
attribute");

Martin



Thanks, here is an updated version:
- adds exclusion array
- updates testcase with new error message (because of exclusion)


The exclusion bits look good, thank you!

Martin


- moves testcase to gcc.c-torture/execute
- adds "noinit" effective-target

Christophe


Thanks,

Christophe

gcc/ChangeLog:

2019-07-04  Christophe Lyon  

* doc/extend.texi: Add "noinit" attribute documentation.
* varasm.c
(default_section_type_flags): Add support for "noinit" section.
(default_elf_select_section): Add support for "noinit" attribute.

gcc/c-family/ChangeLog:

2019-07-04  Christophe Lyon  

* c-attribs.c (c_common_attribute_table): Add "noinit" entry.
(handle_section_attribute): Add support for "noinit" attribute.
(handle_noinit_attribute): New function.

gcc/config/ChangeLog:

2019-07-04  Christophe Lyon  

* msp430/msp430.c (msp430_attribute_table): Remove "noinit" entry.

gcc/testsuite/ChangeLog:

2019-07-04  Christophe Lyon  

* gcc.target/arm/noinit-attribute.c: New test.







[PATCH 2/3] change class-key of PODs to struct and others to class (PR 61339)

2019-07-08 Thread Martin Sebor

The attached patch changes the class-key of class definitions that
satisfy the requirements on a POD struct to 'struct', and that of
struct definitions that aren't POD to class, according to the GCC
coding convention.  The patch is also prerequisite for GCC being
able to compile cleanly with -Wmismatched-tags.

I made the changes building GCC with -Wstruct-not-pod and
-Wclass-is-pod enabled, scanning the build log for instances
of each warning, and using a script replacing the class-key
as necessary and adjusting the access of the members declared
immediately after the class-head.

Martin
PR c++/61339 - add mismatch between struct and class [-Wmismatched-tags] to non-bugs

gcc/c-family/ChangeLog:

	PR c++/61339
	* c.opt: 

gcc/cp/ChangeLog:

	PR c++/61339
	* parser.c (cp_parser_type_specifier): 
	(cp_parser_function_definition_after_declarator): 
	(cp_parser_template_declaration_after_parameters): 

gcc/testsuite/ChangeLog:

	PR c++/61339
	* g++.dg/warn/Wclass-is-pod.C: New test.
	* g++.dg/warn/Wstruct-not-pod.C: New test.

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 080066fa608..27b413115e3 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -794,6 +794,14 @@ Wstringop-truncation
 C ObjC C++ LTO ObjC++ Var(warn_stringop_truncation) Warning Init (1) LangEnabledBy(C ObjC C++ LTO ObjC++, Wall)
 Warn about truncation in string manipulation functions like strncat and strncpy.
 
+Wstruct-not-pod
+C++ ObjC++ Var(warn_struct_not_pod) Init (1) LangEnabledBy(C++ ObjC++, Wall)
+Warn about structs that are not POD.
+
+Wclass-is-pod
+C++ ObjC++ Var(warn_class_is_pod) Init (1) LangEnabledBy(C++ ObjC++, Wall)
+Warn about classes that are POD.
+
 Wsuggest-attribute=format
 C ObjC C++ ObjC++ Var(warn_suggest_attribute_format) Warning
 Warn about functions which might be candidates for format attributes.
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 12814102465..e20c26b7ecd 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -262,6 +262,8 @@ static bool cp_parser_omp_declare_reduction_exprs
 static void cp_finalize_oacc_routine
   (cp_parser *, tree, bool);
 
+static void maybe_warn_struct_vs_class (location_t, tree);
+
 /* Manifest constants.  */
 #define CP_LEXER_BUFFER_SIZE ((256 * 1024) / sizeof (cp_token))
 #define CP_SAVED_TOKEN_STACK 5
@@ -17442,6 +17444,8 @@ cp_parser_type_specifier (cp_parser* parser,
 	  type_spec,
 	  token,
 	  /*type_definition_p=*/true);
+
+	  maybe_warn_struct_vs_class (token->location, type_spec);
 	  return type_spec;
 	}
 
@@ -28039,6 +28043,118 @@ cp_parser_function_definition_after_declarator (cp_parser* parser,
   return fn;
 }
 
+/* Return true if the template TYPE appears to meet the requirements
+   of a POD type even if some of its instantiations may not.  */
+
+static bool
+template_pod_p (tree type)
+{
+  if (TYPE_HAS_USER_CONSTRUCTOR (type)
+  || TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type)
+  || (TYPE_HAS_COPY_ASSIGN (type)
+	  && (cxx_dialect != cxx98
+	  || !TYPE_HAS_TRIVIAL_COPY_ASSIGN (type
+return false;
+
+  for (tree fld = TYPE_FIELDS (type); fld; fld = TREE_CHAIN (fld))
+{
+  if (TREE_CODE (fld) == FIELD_DECL
+	  && !TREE_STATIC (fld)
+	  && TREE_TYPE (fld))
+	{
+	  tree fldtype = TREE_TYPE (fld);
+	  if (TREE_CODE (fldtype) == REFERENCE_TYPE)
+	return false;
+	  if (TREE_CODE (fldtype) == RECORD_TYPE
+	  && !template_pod_p (fldtype))
+	return false;
+	}
+  else if (TREE_CODE (fld) == FUNCTION_DECL
+	  && DECL_NONSTATIC_MEMBER_FUNCTION_P (fld)
+	  && DECL_VIRTUAL_P (fld))
+	return false;
+}
+
+  return true;
+}
+
+/* For a DECL of class type, issue a warning when it is a POD type
+   and is declared with the class-key class, or when it is not a POD
+   type and is declared withe the class-key struct.  When DECL refers
+   to a class template, consider instead whether it has a ctor, dtor,
+   or copy assignment operator as a proxy.  */
+
+static void
+maybe_warn_struct_vs_class (location_t loc, tree type)
+{
+  if (TREE_CODE (type) != RECORD_TYPE)
+return;
+
+  const char *key = class_key_or_enum_as_string (type);
+  if (processing_template_decl)
+{
+  if (template_pod_p (type))
+	{
+	  if (!strcmp (key, "class"))
+	warning_at (loc, OPT_Wclass_is_pod,
+			"POD-like template %qT declared with class-key %qs; "
+			"use %qs instead",
+			type, key, "struct");
+	  else
+	inform (loc,
+		"POD-like template %qT declared with class-key %qs "
+		"as expected",
+		type, key);
+	}
+  else if (strcmp (key, "class"))
+	warning_at (loc, OPT_Wstruct_not_pod,
+		"non-POD-like template %qT declared with class-key %qs; "
+		"use %qs instead",
+		type, key, "class");
+  else
+	inform (loc,
+		"non-POD-like template %qT declared with class-key %qs "
+		"as expected",
+		type, key);
+}
+  else
+{
+  if (pod_type_p (type))
+	{
+	  if (!strcmp (key, "class"))
+	warning_at (loc, OPT_Wclass_is_pod,
+			"POD type %qT declared with class-key 

Re: [PATCH 1/2] Come up with function_decl_type and use it in tree_function_decl.

2019-07-08 Thread Jason Merrill
On Mon, Jul 8, 2019 at 8:51 AM Martin Liška  wrote:
>
> On 7/5/19 12:09 AM, Marc Glisse wrote:
> > On Wed, 3 Jul 2019, Richard Biener wrote:
> >
> >> On July 3, 2019 4:53:30 PM GMT+02:00, "Martin Liška"  
> >> wrote:
> >>> On 7/2/19 7:15 PM, Marc Glisse wrote:
>  On Tue, 2 Jul 2019, Martin Liška wrote:
> 
> > After the discussion with Richi and Nathan, I made a place in
> >>> tree_function_decl
> > and I rebased the original Dominik's patch on top of that.
> 
>  So, last time there were some questions about the legality of this
> >>> transformation. Did you change the exact set of functions on which this
> >>> is applied?
> >>>
> >>> Yes. I was not included in the original discussion, but I hope the
> >>> transformation is valid.
> >>> Btw. clang also removes the new/delete pairs and I guess it was the
> >>> original motivation of the patch.
> >>
> >> We also remove malloc/free pairs which the C standard does not explicitly 
> >> allow (but also doesn't explicitly forbid). I don't think standards need 
> >> to enumerate everything allowed and I don't know any explicit wording in 
> >> the C++ standard that forbids this.
> >
> > The C++ standard has explicit wording allowing to remove new-delete pairs 
> > in some circumstances (expr.new, allocator.members), so I would assume that 
> > other circumstances are forbidden (not that I care much, I am just afraid 
> > someone might).
>
> I hope some C++ FE maintainers can help us here?

Seems fine to me.

[expr.new] says "An implementation is allowed to omit a call to a
replaceable global allocation function (17.6.2.1, 17.6.2.2).
When it does so, the storage is instead provided by the implementation
or provided by extending the allocation
of another new-expression."

In the case of this optimization, the implementation is providing no
storage, but that is fine because it is not observable.

The circumstances described after the above quote have to do with when
two allocations can be combined, which doesn't apply to this
optimization.

std::allocate says "it is unspecified when or how often [::operator
new] is called."

Jason


[PATCH 1/3] add -Wstruct-not-pod, -Wclass-is-pod, -Wmismatched-tags (PR 61339)

2019-07-08 Thread Martin Sebor

The attached patch implements three new warnings:

 *  -Wstruct-not-pod triggers for struct definitions that are not
POD structs,
 *  -Wclass-is-pod triggers for class definitions that satisfy
the requirements on POD structs, and
 *  -Wmismatched-tags that triggers for class and struct declarations
with class-key that doesn't match either their definition or
the first declaration (if no definition is provided).

The implementation of -Wclass-is-pod and -Wstruct-not-pod is fairly
straightforward but the -Wmismatched-tags solution is slightly unusual.
It collects struct and class declarations first and diagnoses mismatches
only after the whole tramslation unit has been processed.  This is so
that the definition of a class can guide which declarations to diagnose
no matter which come first.

Martin
gcc/c-family/ChangeLog:

	* c.opt (-Wstruct-not-pod, -Wclass-is-pod): New options.
	(-Wmismatched-tags): Same.

gcc/cp/ChangeLog:

	* parser.c (maybe_warn_struct_vs_class): New function.
	(cp_parser_check_class_key): Add argument.
	(cp_parser_type_specifier): Call maybe_warn_struct_vs_class.
	(cp_parser_elaborated_type_specifier): Call maybe_warn_struct_vs_class
	before setting CLASSTYPE_DECLARED_CLASS.  Avoid setting it for classes
	that are in the process of being defined.
	(cp_parser_class_head): Call maybe_warn_struct_vs_class.
	(class_or_template_pod_p): New static function.
	(maybe_warn_struct_vs_class) Same.
	(class rec_decl_loc_t): New.
	(cp_parser_check_class_key): Record a struct declaration.
	(diag_mismatched_tags): Hanlde -Wmismatched-tags.
	(c_parse_file): Call diag_mismatched_tags.

gcc/ChangeLog:

	* doc/invoke.texi (-Wstruct-not-pod, -Wclass-is-pod): Document new
	options.
	(-Wmismatched-tags): Same.

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 080066fa608..0bc23a6f409 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -736,6 +736,10 @@ Wmisleading-indentation
 C C++ Common Var(warn_misleading_indentation) Warning LangEnabledBy(C C++,Wall)
 Warn when the indentation of the code does not reflect the block structure.
 
+Wmismatched-tags
+C++ Objc++ Var(warn_mismatched_tags) Warning
+Warn when a class is redeclared using a mismatched class-key.
+
 Wmissing-braces
 C ObjC C++ ObjC++ Var(warn_missing_braces) Warning LangEnabledBy(C ObjC,Wall)
 Warn about possibly missing braces around initializers.
@@ -794,6 +798,14 @@ Wstringop-truncation
 C ObjC C++ LTO ObjC++ Var(warn_stringop_truncation) Warning Init (1) LangEnabledBy(C ObjC C++ LTO ObjC++, Wall)
 Warn about truncation in string manipulation functions like strncat and strncpy.
 
+Wstruct-not-pod
+C++ ObjC++ Var(warn_struct_not_pod) Warning
+Warn about structs that are not POD.
+
+Wclass-is-pod
+C++ ObjC++ Var(warn_class_is_pod) Warning
+Warn about classes that are POD.
+
 Wsuggest-attribute=format
 C ObjC C++ ObjC++ Var(warn_suggest_attribute_format) Warning
 Warn about functions which might be candidates for format attributes.
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 12814102465..74accff2c6d 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -262,6 +262,8 @@ static bool cp_parser_omp_declare_reduction_exprs
 static void cp_finalize_oacc_routine
   (cp_parser *, tree, bool);
 
+static void maybe_warn_struct_vs_class (location_t, tree);
+
 /* Manifest constants.  */
 #define CP_LEXER_BUFFER_SIZE ((256 * 1024) / sizeof (cp_token))
 #define CP_SAVED_TOKEN_STACK 5
@@ -2580,7 +2582,7 @@ static enum tag_types cp_parser_token_is_class_key
 static enum tag_types cp_parser_token_is_type_parameter_key
   (cp_token *);
 static void cp_parser_check_class_key
-  (enum tag_types, tree type);
+(cp_parser *, enum tag_types, tree type, bool);
 static void cp_parser_check_access_in_redeclaration
   (tree type, location_t location);
 static bool cp_parser_optional_template_keyword
@@ -17442,6 +17444,8 @@ cp_parser_type_specifier (cp_parser* parser,
 	  type_spec,
 	  token,
 	  /*type_definition_p=*/true);
+
+	  maybe_warn_struct_vs_class (token->location, type_spec);
 	  return type_spec;
 	}
 
@@ -18621,11 +18625,13 @@ cp_parser_elaborated_type_specifier (cp_parser* parser,
 
   if (tag_type != enum_type)
 {
+  /* Diagnose class/struct/union mismatches.  */
+  cp_parser_check_class_key (parser, tag_type, type, false);
+
   /* Indicate whether this class was declared as a `class' or as a
 	 `struct'.  */
-  if (CLASS_TYPE_P (type))
+  if (CLASS_TYPE_P (type) && !currently_open_class (type))
 	CLASSTYPE_DECLARED_CLASS (type) = (tag_type == class_type);
-  cp_parser_check_class_key (tag_type, type);
 }
 
   /* A "<" cannot follow an elaborated type specifier.  If that
@@ -24168,11 +24174,13 @@ cp_parser_class_head (cp_parser* parser,
 		   parser->num_template_parameter_lists);
 }
 
+  /* Diagnose class/struct/union mismatches.  */
+  cp_parser_check_class_key (parser, class_key, type, true);
+
   /* Indicate whether this class was declared as a `class' or as a
  

[PATCH 0/3] add support for POD struct convention (PR 61339)

2019-07-08 Thread Martin Sebor

A couple of GCC's Coding Conventions call to

  1) Use the struct keyword for plain old data (POD) types.
 https://www.gnu.org/software/gcc/codingrationale.html#struct

and

  2) Use the class keyword for non-POD types.
 https://www.gnu.org/software/gcc/codingconventions.html#Class_Use

The rationale for the conventions is to convey information about
some basic properties of a user-defined type such as whether it
is safely and efficiently copyable or assignable via memcpy or
whether it requires a call to a copy ctor/assignment operator
or destructor.

A survey of GCC code shows that the convention is only loosely
followed, making it less helpful than it would be otherwise.
In addition, inconsistencies between declarations and definitions
are wide-spread.  Relying on the convention can lead to using
a non-POD class that's declared using the struct class-key in
contexts that aren't prepared for such a type (although not
a direct result, pr90904, 90923, 90959 are examples of some of
the bugs that this might lead to).

The poor consistency with which the convention is adhered to makes
it easy for authors to miss that it exists.  This can then lead to
inefficiencies in code reviews both for reviewers and for authors.

In addition, Clang users who build GCC are bothered by instances
of Clang's -Wmismatched-tags warning (see pr61339 and the recent
patch: https://gcc.gnu.org/ml/gcc-patches/2019-05/msg01805.html).
Clang includes -Wmismatched-tags in -Wall even on Linux.  Although
-Wmismatched-tags is provided for compatibility with Visual C++
(and there to help avoid a potential ABI problem), the warning
points out the same lack of consistency in the GCC coding
convention: the type being sometimes declared as a struct and
other times as a class.

To make the code base consistent with the convention and enable
relying on the struct and class keywords having the intended
meaning, to reduce the time spent in code reviews on details
that an easily be caught earlier by out tools, and to avoid
the -Wmismatched-tags warnings when using Clang, the attached
patch kit does three things:

1) Implements three new warnings:
   -Wstruct-not-pod triggers for struct definitions that are not
   POD structs,
   -Wclass-is-pod triggers for class definitions that satisfy
   the requirements on POD structs,
   and -Wmismatched-tags that triggers for class and struct
   declarations with class-key that doesn't match either their
   definition or the first declaration (if no definition is
   provided).  All three warnings have to be explicitly enabled
   to have an effect.
2) Makes changes to definitions of GCC classes and structs to
   avoid instances of the three new warnings.
3) Changes declarations of GCC classes and structs to match
   the class-key used in their definition.

If/when the patch kit is approved I will enable the three
warnings for GCC builds.  I expect to take the same approach as
with -Wformat-diag: first prevent any remaining instances of
the warnings from triggering errors, clean up or suppress
the remaining instances, and finally enable them to cause errors
with -Werror.


Re: [C++ Patch] A few additional location improvements to grokdeclarator and check_tag_decl

2019-07-08 Thread Jason Merrill

On 6/23/19 7:58 AM, Paolo Carlini wrote:

+error_at (smallest_type_location (get_type_quals (declspecs),
+ declspecs->locations),


How about adding a smallest_type_location overload that just takes 
declspecs?


Jason



Re: [PATCH] Perform case-insensitive comparison when decoding register names (PR target/70320)

2019-07-08 Thread Segher Boessenkool
On Mon, Jul 08, 2019 at 10:21:29PM +0100, Jozef Lawrynowicz wrote:
> On Mon, 08 Jul 2019 21:14:36 +0100
> Richard Sandiford  wrote:
> 
> > Segher Boessenkool  writes:
> > > It isn't obviously safe either.  Are there any targets that have names
> > > for different registers that differ only in case?  You could say that
> > > such a design deserves what is coming for it, but :-)
> 
> Indeed, I did have a read through all the definitions of REGISTER_NAMES in the
> gcc/config and could not spot any cases where different register nanes 
> differed
> only in their case. I didn't check it programmatically though, so it's
> not impossible I missed something..

You also haven't checked future GCC versions, for future processors ;-)
And, not all out-of-tree ports, either.  Gratuitously breaking those
isn't ideal.

> > >> --- a/gcc/varasm.c
> > >> +++ b/gcc/varasm.c
> > >> @@ -947,7 +947,7 @@ decode_reg_name_and_count (const char *asmspec, int 
> > >> *pnregs)  
> > >
> > > This is used for more than just clobber lists.  Is this change safe, and
> > > a good thing, in the other contexts where it changes things?
> 
> It appears to be used for only two purposes (mostly via the
> "decode_reg_name" wrapper):
> - Decoding the register name in an asm spec and reporting any misuse
> - Decoding parameters passed to command line options
> Generic options using it are -fcall-used/saved-REG and -ffixed-REG
> -fstack-limit-register.
> Backends use it for target specific options such as -mfixed-range= for SPU.
> Apart from that there appears to be a single other use of it in make_decl_rtl
> to report when "register name given for non-register variable", although I
> could not immediately reproduce this myself to understand this specific case 
> it
> is triggered for.

It is used for register asm, yes.  This is e.g.

void f(int x)
{
int y asm("r10");
y = x;
asm ("# %0" :: "r"(y));
}

which complains

  warning: ignoring 'asm' specifier for non-static local variable 'y'

(Making the declaration of y static does nothing, doesn't make it use r10
that is; adding "register" does though).

> Ok, yes a DEFHOOKPOD or similar sounds like a good idea, I'll look into this
> alternative.

What is that, like target macros?  But with some indirection?

Making this target-specific sounds good, thanks Jozef.


Segher


Re: [PATCH] Perform case-insensitive comparison when decoding register names (PR target/70320)

2019-07-08 Thread Jozef Lawrynowicz
On Mon, 08 Jul 2019 21:14:36 +0100
Richard Sandiford  wrote:

> Segher Boessenkool  writes:
> > On Thu, Jul 04, 2019 at 01:32:59PM +0100, Jozef Lawrynowicz wrote:  
> >> The attached patch allows the case of register names used in an asm 
> >> statement
> >> clobber list to be disregarded when checking the validity of the register 
> >> names.
> >> 
> >> Currently, the register name used in asm statement clobber list must 
> >> exactly
> >> match those defined in the targets REGISTER_NAMES macro.  
> >  
> >> All the register names defined in the msp430 REGISTER_NAMES macro use an
> >> upper case 'R', so use of lower case 'r' gets rejected.
> >> 
> >> I guess this could also be fixed by defining all the registers in
> >> ADDITIONAL_REGISTER_NAMES using a lower case r, but I prefer this generic
> >> solution and I cannot think of any negative side effects to what is 
> >> proposed.  
> >
> > It isn't obviously safe either.  Are there any targets that have names
> > for different registers that differ only in case?  You could say that
> > such a design deserves what is coming for it, but :-)

Indeed, I did have a read through all the definitions of REGISTER_NAMES in the
gcc/config and could not spot any cases where different register nanes differed
only in their case. I didn't check it programmatically though, so it's
not impossible I missed something..

> >  
> >> --- a/gcc/varasm.c
> >> +++ b/gcc/varasm.c
> >> @@ -947,7 +947,7 @@ decode_reg_name_and_count (const char *asmspec, int 
> >> *pnregs)  
> >
> > This is used for more than just clobber lists.  Is this change safe, and
> > a good thing, in the other contexts where it changes things?

It appears to be used for only two purposes (mostly via the
"decode_reg_name" wrapper):
- Decoding the register name in an asm spec and reporting any misuse
- Decoding parameters passed to command line options
Generic options using it are -fcall-used/saved-REG and -ffixed-REG
-fstack-limit-register.
Backends use it for target specific options such as -mfixed-range= for SPU.
Apart from that there appears to be a single other use of it in make_decl_rtl
to report when "register name given for non-register variable", although I
could not immediately reproduce this myself to understand this specific case it
is triggered for.

> >  
> >> -  if (!strcmp (asmspec, "memory"))
> >> +  if (!strcasecmp (asmspec, "memory"))
> >>return -4;
> >>  
> >> -  if (!strcmp (asmspec, "cc"))
> >> +  if (!strcasecmp (asmspec, "cc"))
> >>return -3;  
> >
> > You don't need to change these.  
> 
> Agreed.  There's also the problem that if we make this available for
> all targets now, people might start using it without realising that
> it implicitly requires GCC 10+.
> 
> But having the feature opt-in (by a DEFHOOKPOD?) sounds good, and
> definitely more maintainable than having to add aliases in the
> target code.
> 
> Thanks,
> Richard

Ok, yes a DEFHOOKPOD or similar sounds like a good idea, I'll look into this
alternative.

Thanks,
Jozef


Re: [PATCH, Modula-2 (C/C++/D/F/Go/Jit)] (Register spec fn) (v2)

2019-07-08 Thread Matthias Klose
On 14.06.19 15:09, Gaius Mulley wrote:
> 
> Hello,
> 
> here is version two of the patches which introduce Modula-2 into the
> GCC trunk.  The patches include:
> 
>   (*)  a patch to allow all front ends to register a lang spec function.
>(included are patches for all front ends to provide an empty
> callback function).
>   (*)  patch diffs to allow the Modula-2 front end driver to be
>built using GCC Makefile and friends.
> 
> The compressed tarball includes:
> 
>   (*)  gcc/m2  (compiler driver and lang-spec stuff for Modula-2).
>Including the need for registering lang spec functions.
>   (*)  gcc/testsuite/gm2  (a Modula-2 dejagnu test to ensure that
>the gm2 driver is built and can understands --version).
> 
> These patches have been re-written after taking on board the comments
> found in this thread:
> 
>https://gcc.gnu.org/ml/gcc-patches/2013-11/msg02620.html
> 
> it is a revised patch set from:
> 
>https://gcc.gnu.org/ml/gcc-patches/2019-06/msg00220.html
> 
> I've run make bootstrap and run the regression tests on trunk and no
> extra failures occur for all languages touched in the ChangeLog.
> 
> I'm currently tracking gcc trunk and gcc-9 with gm2 (which works well
> with amd64/arm64/i386) - these patches are currently simply for the
> driver to minimise the patch size.  There are also > 1800 tests in a
> dejagnu testsuite for gm2 which can be included at some future time.

I had a look at the GCC 9 version of the patches, with a build including a make
install. Some comments:

 - A parallel build (at least with -j4) isn't working. A sequental
   build works fine.  I think forcing a sequential build will not
   work well, increasing the build time too much.

 - libgm2 multilib builds are not working.  //32/libgm2
   is configured, but not built.

 - The internal tools in the gcclibdir are installed twice, with
   both vanilla names and prefixed/suffixed names.

 - libgm2/configure.a has a libtool version 14:0:0, however all
   shared libraries are installed with soversion 0.

 - no manual page for gm2m.

 - libpth.{a,so} is installed in the system libdir, which
   conflicts with the installation of the libpth packages
   on most distros.

 - There are three letter libraries with pretty generic
   names installed into the system libdir: log, iso, cor,
   min, ulm. At least for log, you have a file conflict
   with another library.  Shouldn't these libraries named
   mpre specific, like libgm2log?

Matthias

The installed tree:

./usr/bin
./usr/bin/x86_64-linux-gnu-gm2-9
./usr/bin/x86_64-linux-gnu-gm2m-9
./usr/lib/gcc/x86_64-linux-gnu
./usr/lib/gcc/x86_64-linux-gnu/9
./usr/lib/gcc/x86_64-linux-gnu/9/cc1gm2
./usr/lib/gcc/x86_64-linux-gnu/9/gm2l
./usr/lib/gcc/x86_64-linux-gnu/9/gm2lcc
./usr/lib/gcc/x86_64-linux-gnu/9/gm2lgen
./usr/lib/gcc/x86_64-linux-gnu/9/gm2lorder
./usr/lib/gcc/x86_64-linux-gnu/9/x86_64-linux-gnu-cc1gm2-9
./usr/lib/gcc/x86_64-linux-gnu/9/x86_64-linux-gnu-gm2l-9
./usr/lib/gcc/x86_64-linux-gnu/9/x86_64-linux-gnu-gm2lcc-9
./usr/lib/gcc/x86_64-linux-gnu/9/x86_64-linux-gnu-gm2lgen-9
./usr/lib/gcc/x86_64-linux-gnu/9/x86_64-linux-gnu-gm2lorder-9
./usr/lib/gcc/x86_64-linux-gnu/9/x86_64-linux-gnu-gm2m-9
./usr/lib/gcc/x86_64-linux-gnu/9/m2
./usr/lib/gcc/x86_64-linux-gnu/9/m2/cor
./usr/lib/gcc/x86_64-linux-gnu/9/m2/cor/Debug.def
./usr/lib/gcc/x86_64-linux-gnu/9/m2/cor/Debug.mod
./usr/lib/gcc/x86_64-linux-gnu/9/m2/cor/Executive.def
./usr/lib/gcc/x86_64-linux-gnu/9/m2/cor/Executive.mod
./usr/lib/gcc/x86_64-linux-gnu/9/m2/cor/KeyBoardLEDs.def
./usr/lib/gcc/x86_64-linux-gnu/9/m2/cor/libcor.a
./usr/lib/gcc/x86_64-linux-gnu/9/m2/cor/SYSTEM.def
./usr/lib/gcc/x86_64-linux-gnu/9/m2/cor/SYSTEM.mod
./usr/lib/gcc/x86_64-linux-gnu/9/m2/cor/TimerHandler.def
./usr/lib/gcc/x86_64-linux-gnu/9/m2/cor/TimerHandler.mod
./usr/lib/gcc/x86_64-linux-gnu/9/m2/iso
./usr/lib/gcc/x86_64-linux-gnu/9/m2/iso/ChanConsts.def
./usr/lib/gcc/x86_64-linux-gnu/9/m2/iso/ChanConsts.mod
./usr/lib/gcc/x86_64-linux-gnu/9/m2/iso/CharClass.def
./usr/lib/gcc/x86_64-linux-gnu/9/m2/iso/CharClass.mod
./usr/lib/gcc/x86_64-linux-gnu/9/m2/iso/ClientSocket.def
./usr/lib/gcc/x86_64-linux-gnu/9/m2/iso/ClientSocket.mod
./usr/lib/gcc/x86_64-linux-gnu/9/m2/iso/ComplexMath.def
./usr/lib/gcc/x86_64-linux-gnu/9/m2/iso/ComplexMath.mod
./usr/lib/gcc/x86_64-linux-gnu/9/m2/iso/ConvStringLong.def
./usr/lib/gcc/x86_64-linux-gnu/9/m2/iso/ConvStringLong.mod
./usr/lib/gcc/x86_64-linux-gnu/9/m2/iso/ConvStringReal.def
./usr/lib/gcc/x86_64-linux-gnu/9/m2/iso/ConvStringReal.mod
./usr/lib/gcc/x86_64-linux-gnu/9/m2/iso/ConvTypes.def
./usr/lib/gcc/x86_64-linux-gnu/9/m2/iso/ConvTypes.mod
./usr/lib/gcc/x86_64-linux-gnu/9/m2/iso/COROUTINES.def
./usr/lib/gcc/x86_64-linux-gnu/9/m2/iso/COROUTINES.mod
./usr/lib/gcc/x86_64-linux-gnu/9/m2/iso/ErrnoCategory.def
./usr/lib/gcc/x86_64-linux-gnu/9/m2/iso/EXCEPTIONS.def
./usr/lib/gcc/x86_64-linux-gnu/9/m2/iso/EXCEPTIONS.mod

[PATCH] rs6000: Ignore GFXOPT (and GPOPT) for choosing machine

2019-07-08 Thread Segher Boessenkool
The function rs6000_machine_from_flags chooses what .machine string to
used based on the rs6000_isa_flags flags.  For that it checks for each
ISA level if something for its ISA_*_MASKS is selected.

This does not work for GFXOPT and GPOPT: these are set as flags in
ISA_2_5_MASKS_SERVER, but they aren't actually new there, they just
are not selected by default for older ISAs (they were optional).

This patch makes OPTION_MASK_PPC_GFXOPT and OPTION_MASK_PPC_GPOPT not
influence the .machine selection.

Tested on all Linux targets; committing.


Segher


2019-07-08  Segher Boessenkool  

* config/rs6000/rs6000.c (rs6000_machine_from_flags): Ignore
OPTION_MASK_PPC_GFXOPT and OPTION_MASK_PPC_GPOPT for selecting the
.machine string.

---
 gcc/config/rs6000/rs6000.c | 22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index f59f3a9..bec3436 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -5547,22 +5547,26 @@ const char *rs6000_machine;
 const char *
 rs6000_machine_from_flags (void)
 {
-  if ((rs6000_isa_flags & (ISA_FUTURE_MASKS_SERVER & ~ISA_3_0_MASKS_SERVER))
-  != 0)
+  HOST_WIDE_INT flags = rs6000_isa_flags;
+
+  /* Disable the flags that should never influence the .machine selection.  */
+  flags &= ~(OPTION_MASK_PPC_GFXOPT | OPTION_MASK_PPC_GPOPT);
+
+  if ((flags & (ISA_FUTURE_MASKS_SERVER & ~ISA_3_0_MASKS_SERVER)) != 0)
 return "future";
-  if ((rs6000_isa_flags & (ISA_3_0_MASKS_SERVER & ~ISA_2_7_MASKS_SERVER)) != 0)
+  if ((flags & (ISA_3_0_MASKS_SERVER & ~ISA_2_7_MASKS_SERVER)) != 0)
 return "power9";
-  if ((rs6000_isa_flags & (ISA_2_7_MASKS_SERVER & ~ISA_2_6_MASKS_SERVER)) != 0)
+  if ((flags & (ISA_2_7_MASKS_SERVER & ~ISA_2_6_MASKS_SERVER)) != 0)
 return "power8";
-  if ((rs6000_isa_flags & (ISA_2_6_MASKS_SERVER & ~ISA_2_5_MASKS_SERVER)) != 0)
+  if ((flags & (ISA_2_6_MASKS_SERVER & ~ISA_2_5_MASKS_SERVER)) != 0)
 return "power7";
-  if ((rs6000_isa_flags & (ISA_2_5_MASKS_SERVER & ~ISA_2_4_MASKS)) != 0)
+  if ((flags & (ISA_2_5_MASKS_SERVER & ~ISA_2_4_MASKS)) != 0)
 return "power6";
-  if ((rs6000_isa_flags & (ISA_2_4_MASKS & ~ISA_2_1_MASKS)) != 0)
+  if ((flags & (ISA_2_4_MASKS & ~ISA_2_1_MASKS)) != 0)
 return "power5";
-  if ((rs6000_isa_flags & ISA_2_1_MASKS) != 0)
+  if ((flags & ISA_2_1_MASKS) != 0)
 return "power4";
-  if ((rs6000_isa_flags & OPTION_MASK_POWERPC64) != 0)
+  if ((flags & OPTION_MASK_POWERPC64) != 0)
 return "ppc64";
   return "ppc";
 }
-- 
1.8.3.1



Re: [RFC][PR89245] Check REG_CALL_DECL note during the tail-merging

2019-07-08 Thread Richard Sandiford
Dragan Mladjenovic  writes:
> From: "Dragan Mladjenovic" 
>
> This patch prevents merging of CALL instructions that that have different
> REG_CALL_DECL notes attached to them.
>
> On most architectures this is not an important distinction. Usually 
> instruction patterns
> for calls to different functions reference different SYMBOL_REF-s, so they 
> won't match.
> On MIPS PIC calls get split into an got_load/*call_internal pair where the 
> latter represents
> indirect register call w/o SYMBOL_REF attached (until machine_reorg pass). 
> The bugzilla issue
> had such two internal_call-s merged despite the fact that they had different 
> register usage
> information assigned by ipa-ra.
>
> The check could be improved by checking if ipa-ra has actually assigned two 
> different
> register sets for two functions involved, but I chose to only do a quick 
> rtx_equal check.

Yeah, I think doing it that would be better (and maybe simpler?).
It should just be a case of:

  get_call_reg_set_usage (i1, _used_regs, call_used_reg_set);
  get_call_reg_set_usage (i2, _used_regs, call_used_reg_set);

and then checking that i1_used_regs and i2_used_regs are equal.

Thanks,
Richard


[PATCH] rs6000: Add testcase for PR88233

2019-07-08 Thread Segher Boessenkool
This testcase tests that with -mcpu=power8 we do not generate any
mtvsr* instructions, and we do the copy with {l,st}xvd2x.

This currently fails with -m32.  That is an RA problem; I'll open a
new PR for that.

Committing.


2019-07-08  Segher Boessenkool  

gcc/testsuite/
PR rtl-optimization/88233
* gcc.target/powerpc/pr88233.c: New testcase.

---
 gcc/testsuite/gcc.target/powerpc/pr88233.c | 13 +
 1 file changed, 13 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/pr88233.c

diff --git a/gcc/testsuite/gcc.target/powerpc/pr88233.c 
b/gcc/testsuite/gcc.target/powerpc/pr88233.c
new file mode 100644
index 000..fa47b57
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr88233.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mcpu=power8" } */
+
+typedef struct { double a[2]; } A;
+A
+foo (const A *a)
+{
+  return *a;
+}
+
+/* { dg-final { scan-assembler-not {\mmtvsr} } } */
+/* { dg-final { scan-assembler-times {\mlxvd2x\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mstxvd2x\M} 1 } } */
-- 
1.8.3.1



Re: [PATCH] Perform case-insensitive comparison when decoding register names (PR target/70320)

2019-07-08 Thread Richard Sandiford
Segher Boessenkool  writes:
> On Thu, Jul 04, 2019 at 01:32:59PM +0100, Jozef Lawrynowicz wrote:
>> The attached patch allows the case of register names used in an asm statement
>> clobber list to be disregarded when checking the validity of the register 
>> names.
>> 
>> Currently, the register name used in asm statement clobber list must exactly
>> match those defined in the targets REGISTER_NAMES macro.
>
>> All the register names defined in the msp430 REGISTER_NAMES macro use an
>> upper case 'R', so use of lower case 'r' gets rejected.
>> 
>> I guess this could also be fixed by defining all the registers in
>> ADDITIONAL_REGISTER_NAMES using a lower case r, but I prefer this generic
>> solution and I cannot think of any negative side effects to what is proposed.
>
> It isn't obviously safe either.  Are there any targets that have names
> for different registers that differ only in case?  You could say that
> such a design deserves what is coming for it, but :-)
>
>> --- a/gcc/varasm.c
>> +++ b/gcc/varasm.c
>> @@ -947,7 +947,7 @@ decode_reg_name_and_count (const char *asmspec, int 
>> *pnregs)
>
> This is used for more than just clobber lists.  Is this change safe, and
> a good thing, in the other contexts where it changes things?
>
>> -  if (!strcmp (asmspec, "memory"))
>> +  if (!strcasecmp (asmspec, "memory"))
>>  return -4;
>>  
>> -  if (!strcmp (asmspec, "cc"))
>> +  if (!strcasecmp (asmspec, "cc"))
>>  return -3;
>
> You don't need to change these.

Agreed.  There's also the problem that if we make this available for
all targets now, people might start using it without realising that
it implicitly requires GCC 10+.

But having the feature opt-in (by a DEFHOOKPOD?) sounds good, and
definitely more maintainable than having to add aliases in the
target code.

Thanks,
Richard


[wwwdocs,C++] Fix projects/cxx-dr-status.html markup (and avoid )

2019-07-08 Thread Gerald Pfeifer
 let alone  are obsolete elements in current HTML, and
validator.w3.org hence has been loudly complaining about 
  projects/cxx-dr-status.html
which Marek kindly added and updated in the last couple of days.

The patch below addresses all 487 validation errors on that page. :-)

Committed (in two parts, since at first I missed the s).

Gerald

Index: cxx-dr-status.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/projects/cxx-dr-status.html,v
retrieving revision 1.21
retrieving revision 1.23
diff -u -r1.21 -r1.23
--- cxx-dr-status.html  8 Jul 2019 16:35:49 -   1.21
+++ cxx-dr-status.html  8 Jul 2019 18:51:50 -   1.23
@@ -228,7 +228,7 @@
 
   http://wg21.link/cwg30;>30
   TC1
-  Valid uses of "::template"
+  Valid uses of "::template"
   ?
   
 
@@ -445,7 +445,7 @@
 
   http://wg21.link/cwg61;>61
   NAD
-  Address of static member function "p-f"
+  Address of static member function "p-f"
   ?
   
 
@@ -606,7 +606,7 @@
 
   http://wg21.link/cwg84;>84
   TC1
-  Overloading and conversion loophole used by auto_ptr
+  Overloading and conversion loophole used by 
auto_ptr
   ?
   
 
@@ -690,7 +690,7 @@
 
   http://wg21.link/cwg96;>96
   C++11
-  Syntactic disambiguation using the template keyword
+  Syntactic disambiguation using the template keyword
   ?
   
 
@@ -781,7 +781,7 @@
 
   http://wg21.link/cwg109;>109
   NAD
-  Allowing ::template in using-declarations
+  Allowing ::template in using-declarations
   ?
   
 
@@ -893,14 +893,14 @@
 
   http://wg21.link/cwg125;>125
   CD1
-  Ambiguity in friend declaration syntax
+  Ambiguity in friend declaration syntax
   ?
   
 
 
   http://wg21.link/cwg126;>126
   TC1
-  Exception specifications and const
+  Exception specifications and const
   ?
   
 
@@ -977,7 +977,7 @@
 
   http://wg21.link/cwg137;>137
   TC1
-  static_cast of cv void*
+  static_cast of cv void*
   ?
   
 
@@ -991,7 +991,7 @@
 
   http://wg21.link/cwg139;>139
   CD1
-  Error in friend lookup example
+  Error in friend lookup example
   ?
   
 
@@ -1026,14 +1026,14 @@
 
   http://wg21.link/cwg144;>144
   open
-  Position of friend specifier
+  Position of friend specifier
   -
   
 
 
   http://wg21.link/cwg145;>145
   TC1
-  Deprecation of prefix ++
+  Deprecation of prefix ++
   ?
   
 
@@ -1082,7 +1082,7 @@
 
   http://wg21.link/cwg152;>152
   TC1
-  explicit copy constructors
+  explicit copy constructors
   ?
   
 
@@ -1138,7 +1138,7 @@
 
   http://wg21.link/cwg160;>160
   CD1
-  Missing std:: qualification
+  Missing std:: qualification
   ?
   
 
@@ -1152,7 +1152,7 @@
 
   http://wg21.link/cwg162;>162
   CD1
-  (C::f)() with nonstatic members
+  (C::f)() with nonstatic members
   ?
   
 
@@ -1278,7 +1278,7 @@
 
   http://wg21.link/cwg180;>180
   CD1
-  typename and elaborated types
+  typename and elaborated types
   ?
   
 
@@ -1299,7 +1299,7 @@
 
   http://wg21.link/cwg183;>183
   TC1
-  typename in explicit specializations
+  typename in explicit specializations
   ?
   
 
@@ -1607,14 +1607,14 @@
 
   http://wg21.link/cwg227;>227
   TC1
-  How many scopes in an if statement?
+  How many scopes in an if statement?
   ?
   
 
 
   http://wg21.link/cwg228;>228
   CD1
-  Use of template keyword with non-member templates
+  Use of template keyword with non-member templates
   ?
   
 
@@ -1841,7 +1841,7 @@
 
   http://wg21.link/cwg260;>260
   open
-  User-defined conversions and built-in operator=
+  User-defined conversions and built-in operator=
   -
   
 
@@ -1932,7 +1932,7 @@
 
   http://wg21.link/cwg273;>273
   CD1
-  POD classes and operator()
+  POD classes and operator()
   ?
   
 
@@ -1988,14 +1988,14 @@
 
   http://wg21.link/cwg281;>281
   CD1
-  inline specifier in friend declarations
+  inline specifier in friend 
declarations
   ?
   
 
 
   http://wg21.link/cwg282;>282
   open
-  Namespace for extended_type_info
+  Namespace for extended_type_info
   -
   
 
@@ -2065,7 +2065,7 @@
 
   http://wg21.link/cwg292;>292
   CD3
-  Deallocation on exception in new before arguments 
evaluated
+  Deallocation on exception in new before arguments 
evaluated
   ?
   
 
@@ -2079,7 +2079,7 @@
 
   http://wg21.link/cwg294;>294
   NAD
-  Can static_cast drop exception specifications?

Re: [PATCH] Add -fprofile-note option.

2019-07-08 Thread Sandra Loosemore

On 7/8/19 11:43 AM, Martin Liška wrote:


@@ -12407,7 +12407,9 @@ To optimize the program based on the collected profile 
information, use
 @item -fprofile-note=@var{path}
 @opindex fprofile-note
 
-If @var{path} is specified, GCC saves gcno filename into @var{path} location.

+If @var{path} is specified, GCC saves @file{.gcno} file into @var{path}
+location.  If you not combine the option with multiple source files,


s/not // ??
It doesn't make sense as written.


+the @file{.gcno} file will be overwritten.
 
 @item -fprofile-update=@var{method}

 @opindex fprofile-update


-Sandra


Re: [COMMITTED] Turn of ipa-ra in builtins test (PR91059)

2019-07-08 Thread Wilco Dijkstra
Hi Alexander,
 
> Would have been nice if the above text also mentioned that PR 78527 has some
> background info for the "obvious".  The paragraph went into commit text, but 
> on
> its own it doesn't explain what's "obvious" here at all :(
 
The "obvious" refers to the patch itself under the commit rules. The issue 
itself is 
non-obvious of course.

> (went looking for the explanation after collecting jaw from the floor; I don't
> agree with triage shown there, but what does it matter...)

Yes, it was a huge waste of time before the underlying cause was found (comment 
#26).
Worse, absolutely nothing happened after all that...

Wilco


Re: [PATCH] PowerPC Prefixed Memory, Patch #4, Add pc-relative reference support

2019-07-08 Thread Segher Boessenkool
On Mon, Jul 08, 2019 at 02:42:13PM -0400, Michael Meissner wrote:
> On Wed, Jul 03, 2019 at 05:09:57PM -0500, Segher Boessenkool wrote:
> > Hi Mike,
> > 
> > On Fri, Jun 28, 2019 at 02:50:33PM -0400, Michael Meissner wrote:
> > > --- gcc/config/rs6000/rs6000-logue.c  (revision 272714)
> > > +++ gcc/config/rs6000/rs6000-logue.c  (working copy)
> > > @@ -1406,23 +1406,13 @@ uses_TOC (void)
> > >  }
> > >  #endif
> > >  
> > > +/* Create a TOC style reference for a symbol.  */
> > >  rtx
> > >  create_TOC_reference (rtx symbol, rtx largetoc_reg)
> > 
> > Does this really belong in this file?  It doesn't do anythin *logue, and
> > it isn't called from anywhere in here.
> 
> Note, when rs6000-logue.c was created, it was put there.

I know.  That doesn't make it right though!  :-)

> I was just trying to
> make the smallest number of changes to the infrastructure.  I can move it back
> to rs6000.c if preferred.

Please do; as a separate patch.  Thanks in advance.  A patch purely moving
it back is pre-approved.

> > > +/* Create either a TOC reference to a locally defined item or a 
> > > pc-relative
> > > +   reference, depending on the ABI.  */
> > > +rtx
> > > +create_data_reference (rtx symbol, rtx largetoc_reg)
> > 
> > Same here.
> > 
> > What is largetoc_reg?  The function comment should say.  It also is only
> > relevant for create_TOC_reference (where such a comment is also missing),
> > so could you factor this better please?
> 
> Again, this was existing code, and I didn't really change the existing code.

No, create_data_reference is a new function.  That's the point.  For
create_TOC_reference it might make some sense (but it should be documented
what this does); for create_data_reference it is a non-sensical parameter:
either all callers use NULL, or you more likely than not have bugs.

> > Probably a create_data_reference with only one argument?  Which calls
> > create_TOC_reference with a NULL second arg.  It looks like your
> > proposed create_data_reference will not do the right thing if called
> > with a non-null second arg if pcrel.  Perhaps that cannot happen, but
> > make that clear then?  Just an assert will do, bigger cleanups are
> > better of course.


Segher


Re: [PATCH] PowerPC Prefixed Memory, Patch #4, Add pc-relative reference support

2019-07-08 Thread Michael Meissner
On Wed, Jul 03, 2019 at 05:09:57PM -0500, Segher Boessenkool wrote:
> Hi Mike,
> 
> On Fri, Jun 28, 2019 at 02:50:33PM -0400, Michael Meissner wrote:
> > --- gcc/config/rs6000/rs6000-logue.c(revision 272714)
> > +++ gcc/config/rs6000/rs6000-logue.c(working copy)
> > @@ -1406,23 +1406,13 @@ uses_TOC (void)
> >  }
> >  #endif
> >  
> > +/* Create a TOC style reference for a symbol.  */
> >  rtx
> >  create_TOC_reference (rtx symbol, rtx largetoc_reg)
> 
> Does this really belong in this file?  It doesn't do anythin *logue, and
> it isn't called from anywhere in here.

Note, when rs6000-logue.c was created, it was put there.  I was just trying to
make the smallest number of changes to the infrastructure.  I can move it back
to rs6000.c if preferred.

> > +/* Create either a TOC reference to a locally defined item or a pc-relative
> > +   reference, depending on the ABI.  */
> > +rtx
> > +create_data_reference (rtx symbol, rtx largetoc_reg)
> 
> Same here.
> 
> What is largetoc_reg?  The function comment should say.  It also is only
> relevant for create_TOC_reference (where such a comment is also missing),
> so could you factor this better please?

Again, this was existing code, and I didn't really change the existing code.

> Probably a create_data_reference with only one argument?  Which calls
> create_TOC_reference with a NULL second arg.  It looks like your
> proposed create_data_reference will not do the right thing if called
> with a non-null second arg if pcrel.  Perhaps that cannot happen, but
> make that clear then?  Just an assert will do, bigger cleanups are
> better of course.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797



Re: [patch] Small improvements to coverage info (3/n)

2019-07-08 Thread Jeff Law
On 7/8/19 3:32 AM, Eric Botcazou wrote:
> Hi,
> 
> a couple of fixes for the RTL middle-end this time, with the same goal of 
> preventing instructions from inheriting random source location information 
> in the debug info generated by the compiler.
> 
> Tested on x86_64-suse-linux, both GCC and GDB, OK for mainline?
> 
> 
> 2019-07-08  Eric Botcazou  
> 
>   * emit-rtl.c (set_insn_locations): New function moved from...
>   * function.c (set_insn_locations): ...here.
>   * ira-emit.c (emit_moves): Propagate location of the first instruction
>   to the inserted move instructions.
>   * reg-stack.c (compensate_edge): Set the location if the sequence is
>   inserted on the edge.
>   * rtl.h (set_insn_locations): Declare.
OK
jeff


Re: [PATCH] Remove usage of ZSTD_CLEVEL_DEFAULT define.

2019-07-08 Thread Jeff Law
On 7/8/19 11:27 AM, Martin Liška wrote:
> Hi.
> 
> As Martin Jambor noticed, a zstd ZSTD_CLEVEL_DEFAULT macro is not
> defined in a bit older releases of zstd. However, one can use 0
> as a default compression level.
> 
> Ready to be installed after regression tests & bootstrap?
> Thanks,
> Martin
> 
> gcc/ChangeLog:
> 
> 2019-07-08  Martin Liska  
> 
> * lto-compress.c (lto_normalized_zstd_level): Do not use
> ZSTD_CLEVEL_DEFAULT as it is not default in old releases
> of libzstd.  One can use 0 as a default compression level.
> ---
>  gcc/lto-compress.c | 11 ---
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> 
OK
jeff


Re: [COMMITTED] Turn of ipa-ra in builtins test (PR91059)

2019-07-08 Thread Alexander Monakov
On Mon, 8 Jul 2019, Wilco Dijkstra wrote:

> The gcc.c-torture/execute/builtins/lib directory contains a reimplementation
> of many C library string functions, which causes non-trivial register 
> allocation
> bugs with LTO and static linked libraries.  To fix this long-standing test 
> issue,
> turn off ipa-ra which avoids the register corruption across calls.  All 
> builtin
> torture tests now pass on aarch64-none-elf.  Committed as obvious.

Would have been nice if the above text also mentioned that PR 78527 has some
background info for the "obvious".  The paragraph went into commit text, but on
its own it doesn't explain what's "obvious" here at all :(

(went looking for the explanation after collecting jaw from the floor; I don't
agree with triage shown there, but what does it matter...)

> ChangeLog:
> 2019-07-08  Wilco Dijkstra  
> 
> testsuite/
>   PR testsuite/91059
>   PR testsuite/78529
>   * gcc.c-torture/execute/builtins/builtins.exp: Add -fno-ipa-ra.

Alexander


Re: [PATCH 1/2] [ARC] Fix and refurbish the interrupts.

2019-07-08 Thread Jeff Law
On 7/8/19 2:35 AM, Claudiu Zissulescu wrote:
> Hi Jeff,
> 
> Originally, I had the scheduler barrier as you suggested. However,
> there were some user cases when an ISR messed up with SP register
> leading to errors. As a solution was to add barriers on either part of
> frame operations. However, I would need to recheck the original
> rationale of that issue, as it may not be the case with newer cores.
For the prologue ISTM that the only issue would be if you had a store
via the frame pointer that moved to a point before allocating the stack.
 I don't think we've ever seen that in practice though.

In the epilogue, the case we've seen several times is we have register
restores via the frame pointer move to a point before deallocating the
stack.

In both cases there'd be live data that would be out of the bounds of
the current stack pointer.  THus if an interrupt occurred the handler
could stomp on that data.

> 
> On the other hand, I found a small error of the current patch when one
> is having the fno-omit-frame-pointer option in the order how some
> auxiliary registers are saved. What will be the best solution having
> in mind that I haven't pushed this patch to the mainline yet:
> - to have the current patch re-spin?
> - to have a separate patch for the new issue.
Your decision.

Jeff


Re: [PATCH] Add -fprofile-note option.

2019-07-08 Thread Martin Liška

On 7/4/19 1:15 AM, Sandra Loosemore wrote:

On 7/2/19 6:37 AM, Martin Liška wrote:


@@ -12403,6 +12403,11 @@ the profile feedback data files. See 
@option{-fprofile-dir}.
 To optimize the program based on the collected profile information, use
 @option{-fprofile-use}.  @xref{Optimize Options}, for more information.

+@item -fprofile-note=@var{path}
+@opindex fprofile-note
+
+If @var{path} is specified, GCC saves gcno filename into @var{path} location.
+
 @item -fprofile-update=@var{method}
 @opindex fprofile-update




Hi.



"gcno filename" is implementor-speak with no context.  In other places the documentation 
uses "@file{.gcno} file".  Please use that here as well, and add a @cindex entry on the 
main definition/discussion of these things and a cross-reference here.


Thank you for the comments. Yes, the suggested wording is much bettern.



I assume this option only makes sense with some other profiling options.  What 
are they?


Yes, I listed the option in the section.



Can there be more than one of these files per gcc invocation?  E.g. if you 
specify a command line like

gcc -c foo.c bar.c

??  It looks like the code part of the patch would cause the file to be 
overwritten.  Maybe this should be like -o and diagnose an error?


Yes, it can be combined, I added a caveat into documentation patch.

Martin



-Sandra



>From d1182429f5565a4e82f035af30ef98151fc36a48 Mon Sep 17 00:00:00 2001
From: Martin Liska 
Date: Mon, 8 Jul 2019 19:37:53 +0200
Subject: [PATCH] Enhance documentation of -fprofile-note option.

gcc/ChangeLog:

2019-07-08  Martin Liska  

	* doc/invoke.texi: Add link from -fprofile-dir option.
	Use better wording for 'gcno filename'.
---
 gcc/doc/invoke.texi | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 73d16b59d91..6c1692eb4b7 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -12369,7 +12369,7 @@ profile data file appears in the same directory as the object file.
 In order to prevent the file name clashing, if the object file name is
 not an absolute path, we mangle the absolute path of the
 @file{@var{sourcename}.gcda} file and use it as the file name of a
-@file{.gcda} file.
+@file{.gcda} file.  See similar option @option{-fprofile-note}.
 
 When an executable is run in a massive parallel environment, it is recommended
 to save profile to different folders.  That can be done with variables
@@ -12407,7 +12407,9 @@ To optimize the program based on the collected profile information, use
 @item -fprofile-note=@var{path}
 @opindex fprofile-note
 
-If @var{path} is specified, GCC saves gcno filename into @var{path} location.
+If @var{path} is specified, GCC saves @file{.gcno} file into @var{path}
+location.  If you not combine the option with multiple source files,
+the @file{.gcno} file will be overwritten.
 
 @item -fprofile-update=@var{method}
 @opindex fprofile-update
-- 
2.22.0



[wwwdocs] Introducing C++ DR status table

2019-07-08 Thread Marek Polacek
For a long time I wished we had a table documenting the status of C++ defect
reports in the C++ FE, like clang has [1].  I finally got around to tackling
this bad boy and created 
and will now commit the attached patch.

The table was created by an awk script which processed
 and then I made
a lot of manual changes.  I also searched through 3000 C++ PRs and linked
various PRs and changed the status of a few DRs.  For this table to be
useful it needs to be much more complete, but this is a good start.

I'm not going to force anyone's hand to go over that list and update random DRs
(though *any* help would be greatly appreciated; even a list of DR #s that you
know are fixed would be useful), but please, when you fix a DR, reflect that
in the table (or let me know and I'll update it for ya).  I want to see more
green!

[1] https://clang.llvm.org/cxx_dr_status.html

Index: cxx-status.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/projects/cxx-status.html,v
retrieving revision 1.86
diff -u -r1.86 cxx-status.html
--- cxx-status.html 29 May 2019 08:14:45 -  1.86
+++ cxx-status.html 8 Jul 2019 16:41:47 -
@@ -21,6 +21,10 @@
 Technical Specifications
   

+  For information about the status of C++ defect reports, please see
+  https://gcc.gnu.org/projects/cxx-dr-status.html;>this page.
+  
+
   For information about the status of the library implementation, please see
   https://gcc.gnu.org/onlinedocs/libstdc++/manual/status.html;>this 
page.
   

--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA


Re: [PATCH] subreg: Add -fsplit-wide-types-early (PR88233)

2019-07-08 Thread Segher Boessenkool
On Mon, Jul 08, 2019 at 12:20:55PM -0500, Segher Boessenkool wrote:
> On Mon, Jul 08, 2019 at 01:27:25PM +0200, Richard Biener wrote:
> > The docs should mention that the new option doesn't have any effect
> > unless -fsplit-wide-types is enabled.
> 
> Yeah I'll make that more explicit.

I added

This option has no effect unless @option{-fsplit-wide-types} is turned on.


Segher


[PATCH] Remove usage of ZSTD_CLEVEL_DEFAULT define.

2019-07-08 Thread Martin Liška

Hi.

As Martin Jambor noticed, a zstd ZSTD_CLEVEL_DEFAULT macro is not
defined in a bit older releases of zstd. However, one can use 0
as a default compression level.

Ready to be installed after regression tests & bootstrap?
Thanks,
Martin

gcc/ChangeLog:

2019-07-08  Martin Liska  

* lto-compress.c (lto_normalized_zstd_level): Do not use
ZSTD_CLEVEL_DEFAULT as it is not default in old releases
of libzstd.  One can use 0 as a default compression level.
---
 gcc/lto-compress.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)


diff --git a/gcc/lto-compress.c b/gcc/lto-compress.c
index b925363ac71..c5c37dbcbfc 100644
--- a/gcc/lto-compress.c
+++ b/gcc/lto-compress.c
@@ -115,13 +115,10 @@ lto_normalized_zstd_level (void)
 {
   int level = flag_lto_compression_level;
 
-  if (level != ZSTD_CLEVEL_DEFAULT)
-{
-  if (level < 1)
-	level = 1;
-  else if (level > ZSTD_maxCLevel ())
-	level = ZSTD_maxCLevel ();
-}
+  if (level < 0)
+level = 0;
+  else if (level > ZSTD_maxCLevel ())
+level = ZSTD_maxCLevel ();
 
   return level;
 }



Re: [PATCH] subreg: Add -fsplit-wide-types-early (PR88233)

2019-07-08 Thread Segher Boessenkool
On Mon, Jul 08, 2019 at 01:27:25PM +0200, Richard Biener wrote:
> On Sun, Jul 7, 2019 at 7:12 PM Segher Boessenkool
>  wrote:
> >
> > Currently the second lower-subreg pass is run right before RA.  This
> > is much too late to be very useful.  At least for targets that do not
> > have RTL patterns for operations on multi-register modes it is a lot
> > better to split patterns earlier, before combine and all related
> > passes.
> >
> > This adds an option -fsplit-wide-types-early that does that, and
> > enables it by default for rs6000.
> 
> Do you by chance have a (target specific) testcase you can add?

I haven't yet figured out how to do a not terribly fragile test for this.
Hrm, maybe the thing in the PR with -mcpu=power8 will do.  I'll work on
that.

> The docs should mention that the new option doesn't have any effect
> unless -fsplit-wide-types is enabled.

Yeah I'll make that more explicit.  Thanks,


Segher


> > 2019-07-07  Segher Boessenkool  
> >
> > PR rtl-optimization/88233
> > * common.opt (fsplit-wide-types-early): New option.
> > * common/config/rs6000/rs6000-common.c
> > (rs6000_option_optimization_table): Add OPT_fsplit_wide_types_early 
> > for
> > OPT_LEVELS_ALL.
> > * doc/invoke.texi (Optimization Options): Add 
> > -fsplit-wide-types-early.
> > * lower-subreg.c (pass_lower_subreg2::gate): Add test for
> > flag_split_wide_types_early.
> > (pass_data_lower_subreg3): New.
> > (pass_lower_subreg3): New.
> > (make_pass_lower_subreg3): New.
> > * passes.def (pass_lower_subreg2): Move after the loop passes.
> > (pass_lower_subreg3): New, inserted where pass_lower_subreg2 was.
> > * tree-pass.h (make_pass_lower_subreg2): Move up, to its new place 
> > in
> > the pass pipeline; its previous place is taken by ...
> > (make_pass_lower_subreg3): ... this.


[COMMITTED] Turn of ipa-ra in builtins test (PR91059)

2019-07-08 Thread Wilco Dijkstra
The gcc.c-torture/execute/builtins/lib directory contains a reimplementation
of many C library string functions, which causes non-trivial register allocation
bugs with LTO and static linked libraries.  To fix this long-standing test 
issue,
turn off ipa-ra which avoids the register corruption across calls.  All builtin
torture tests now pass on aarch64-none-elf.  Committed as obvious.

ChangeLog:
2019-07-08  Wilco Dijkstra  

testsuite/
PR testsuite/91059
PR testsuite/78529
* gcc.c-torture/execute/builtins/builtins.exp: Add -fno-ipa-ra.
--

diff --git a/gcc/testsuite/gcc.c-torture/execute/builtins/builtins.exp 
b/gcc/testsuite/gcc.c-torture/execute/builtins/builtins.exp
index 
fb9d3ece181fee3489e1ddd2f72f40dbbf807bb6..d62f78c7a6c52c3adea5e4c216f12765acadf60e
 100644
--- a/gcc/testsuite/gcc.c-torture/execute/builtins/builtins.exp
+++ b/gcc/testsuite/gcc.c-torture/execute/builtins/builtins.exp
@@ -37,7 +37,7 @@ load_lib c-torture.exp
 torture-init
 set-torture-options $C_TORTURE_OPTIONS {{}} $LTO_TORTURE_OPTIONS
 
-set additional_flags "-fno-tree-dse -fno-tree-loop-distribute-patterns 
-fno-tracer"
+set additional_flags "-fno-tree-dse -fno-tree-loop-distribute-patterns 
-fno-tracer -fno-ipa-ra"
 if [istarget "powerpc-*-darwin*"] {
lappend additional_flags "-Wl,-multiply_defined,suppress"
 }


Re: [PATCH][GCC][AArch64] Make processing less fragile in config.gcc

2019-07-08 Thread James Greenhalgh
On Tue, Jun 25, 2019 at 09:30:30AM +0100, Tamar Christina wrote:
> Hi All,
> 
> This is an update to the patch rebased to after the SVE2 options have been 
> merged.
> 
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> 
> Ok for trunk?

OK.

Thanks,
James

> 
> Thanks,
> Tamar
> 



RE: [patch 1/2][aarch64]: redefine aes patterns

2019-07-08 Thread Sylvia Taylor
Hi James,

I forgot to mention that. Yes, please do commit it on my behalf.

Cheers,
Syl


Re: [patch 1/2][aarch64]: redefine aes patterns

2019-07-08 Thread James Greenhalgh
On Fri, Jul 05, 2019 at 12:24:42PM +0100, Sylvia Taylor wrote:
> Greetings,
> 
> This first patch removes aarch64 usage of the aese/aesmc and aesd/aesimc
> fusions (i.e. aes fusion) implemented in the scheduler due to unpredictable
> behaviour observed in cases such as:
> - when register allocation goes bad (e.g. extra movs)
> - aes operations with xor and zeroed keys among interleaved operations
> 
> A more stable version should be provided by instead doing the aes fusion 
> during the combine pass. Since the aese and aesd patterns have been 
> rewritten as encapsulating a xor operation, the existing combine fusion 
> patterns have also been updated. The purpose is to simplify the need of 
> having additional combine patterns for cases like the ones below:
> 
> For AESE (though it also applies to AESD as both have a xor operation):
> 
> data = data ^ key;
> data = vaeseq_u8(data, zero);
> ---
> eor   v1.16b, v0.16b, v1.16b
> aese  v1.16b, v2.16b
> 
> Should mean and generate the same as:
> 
> data = vaeseq_u8(data, key);
> ---
> aese  v1.16b, v0.16b
> 
> Bootstrapped and tested on aarch64-none-linux-gnu.

OK for trunk.

I couldn't see you in the maintainers file, do you need someone to apply
this on your behalf?

Thanks,
James

> 
> Cheers,
> Syl
> 
> gcc/ChangeLog:
> 
> 2019-07-05  Sylvia Taylor  
> 
>   * config/aarch64/aarch64-simd.md
>   (aarch64_crypto_aesv16qi): Redefine pattern with xor.
>   (aarch64_crypto_aesv16qi): Remove attribute enabled.
>   (*aarch64_crypto_aesv16qi_xor_combine): Remove both.
>   (*aarch64_crypto_aese_fused,
>   *aarch64_crypto_aesd_fused): Update to new definition.
>   * config/aarch64/aarch64.c
>   (aarch_macro_fusion_pair_p): Remove aese/aesmc fusion check.
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-07-05  Sylvia Taylor  
> 
>   * gcc.target/aarch64/crypto-fuse-1.c: Remove.
>   * gcc.target/aarch64/crypto-fuse-2.c: Remove.
>   * gcc.target/aarch64/aes-fuse-1.c: New testcase.
>   * gcc.target/aarch64/aes-fuse-2.c: New testcase.




Re: [PATCH V4] PR88497 - Extend reassoc for vector bit_field_ref

2019-07-08 Thread Segher Boessenkool
Hi Kewen,

On Mon, Jul 08, 2019 at 04:07:00PM +0800, Kewen.Lin wrote:
> gcc/ChangeLog

(You have trailing spaces in the changelog, fwiw).

> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr88497-1.c
> @@ -0,0 +1,60 @@
> +/* { dg-do run } */
> +/* { dg-require-effective-target vect_double } */
> +/* { dg-require-effective-target powerpc_vsx_ok { target { powerpc*-*-* } } 
> } */

For "dg-do run" tests, you need "powerpc_vsx_hw".  "_ok" only tests if
the assembler can handle VSX instructions, not whether the test system
can run them.  (powerpc_vsx_ok is what you need for "dg-do assemble" or
"dg-do link" tests.  It also tests if you can use -mvsx, but that doesn't
do what you might hope it does: you can use -mvsx together with a -mcpu=
that doesn't support VSX, for example).

> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr88497-2.c
> @@ -0,0 +1,37 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target vect_float } */
> +/* { dg-require-effective-target powerpc_altivec_ok { target { powerpc*-*-* 
> } } } */

This one is fine, and the rest of the tests as well.


Segher


Re: Prevent tree-ssa-dce.c from deleting stores at -Og

2019-07-08 Thread Richard Biener
On Mon, Jul 8, 2019 at 4:41 PM Richard Sandiford
 wrote:
>
> Richard Biener  writes:
> > On Sun, Jul 7, 2019 at 9:07 PM Jeff Law  wrote:
> >>
> >> On 7/7/19 3:45 AM, Richard Sandiford wrote:
> >> > DCE tries to delete dead stores to local data and also tries to insert
> >> > debug binds for simple cases:
> >> >
> >> >   /* If this is a store into a variable that is being optimized away,
> >> >  add a debug bind stmt if possible.  */
> >> >   if (MAY_HAVE_DEBUG_BIND_STMTS
> >> >   && gimple_assign_single_p (stmt)
> >> >   && is_gimple_val (gimple_assign_rhs1 (stmt)))
> >> > {
> >> >   tree lhs = gimple_assign_lhs (stmt);
> >> >   if ((VAR_P (lhs) || TREE_CODE (lhs) == PARM_DECL)
> >> > && !DECL_IGNORED_P (lhs)
> >> > && is_gimple_reg_type (TREE_TYPE (lhs))
> >> > && !is_global_var (lhs)
> >> > && !DECL_HAS_VALUE_EXPR_P (lhs))
> >> >   {
> >> > tree rhs = gimple_assign_rhs1 (stmt);
> >> > gdebug *note
> >> >   = gimple_build_debug_bind (lhs, unshare_expr (rhs), stmt);
> >> > gsi_insert_after (i, note, GSI_SAME_STMT);
> >> >   }
> >> > }
> >> >
> >> > But this doesn't help for things like "print *ptr" when ptr points
> >> > to the local variable (tests Og-dce-1.c and Og-dce-2.c).  It also tends
> >> > to make the *live* -- and thus useful -- values optimised out, because
> >> > we can't yet switch back to tracking the memory location as it evolves
> >> > over time (test Og-dce-3.c).
> >> >
> >> > So for -Og I think it'd be better not to delete any stmts with
> >> > vdefs for now.  This also means that we can avoid the potentially
> >> > expensive vop walks (which already have a cut-off, but still).
> >> >
> >> > The patch also fixes the Og failures in gcc.dg/guality/pr54970.c
> >> > (PR 86638).
> >> >
> >> > Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
> >> >
> >> > Richard
> >> >
> >> >
> >> > 2019-07-07  Richard Sandiford  
> >> >
> >> > gcc/
> >> >   PR debug/86638
> >> >   * tree-ssa-dce.c (keep_all_vdefs_p): New function.
> >> >   (mark_stmt_if_obviously_necessary): Mark all stmts with vdefs as
> >> >   necessary if keep_all_vdefs_p is true.
> >> >   (mark_aliased_reaching_defs_necessary): Add a gcc_checking_assert
> >> >   that keep_all_vdefs_p is false.
> >> >   (mark_all_reaching_defs_necessary): Likewise.
> >> >   (propagate_necessity): Skip the vuse scan if keep_all_vdefs_p is 
> >> > true.
> >> >
> >> > gcc/testsuite/
> >> >   * c-c++-common/guality/Og-dce-1.c: New test.
> >> >   * c-c++-common/guality/Og-dce-2.c: Likewise.
> >> >   * c-c++-common/guality/Og-dce-3.c: Likewise.
> >> OK
> >
> > I wonder how code size (and compile-time) is affected by the DSE/DCE patch?
> > Say just look at -Og built cc1?
>
> Overall I see a ~2.5% slowdown and a 4.7% increase in load size.
> That comes almost entirely from the (RTL) DSE side; this patch
> and gimple DSE part don't seem to make much difference.
>
> If I keep the gimple passes as-is and just disable RTL DSE, the slowdown
> is still ~2.5% and there's a 4.4% increase in load size.
>
> These are all measuring cc1plus (built from post-patch sources)
> and using -O2 -g tree-into-ssa.ii for the speed checks.
>
> > Can you restrict the keep-all-vdefs to user variables (and measure the
> > difference this makes)?
>
> In order to avoid wrong debug for pointer dereferences, I think it would
> have to be keep-all-vdefs for writes to either user variables or unknown
> locations.  But as above, I can't measure a significant difference with
> the patch.
>
> > Again I wonder if this makes C++ with -Og impractical runtime-wise.
>
> Got a particular test in mind?

Nothing specific - there are a few C/C++ benchmarks in SPEC and there's
also tramp3d-v4.  I guess SRA is much more important for the abstraction
penalty than DSE - FRE should be able to remove the abstraction, just the
dead stores will remain (but they'd probably nicely execute out-of-order).

Anyway, the biggest runtime penalty from -Og is probably not running
any loop optimization (invariant motion mostly).

Richard.

>
> Richard


Re: [patch, libfortran] Adjust block size for libgfortran for unformatted reads

2019-07-08 Thread Steve Kargl
On Mon, Jul 08, 2019 at 04:02:17PM +0300, Janne Blomqvist wrote:
> 
> Good point. If you happen to have a USB stick handy, can you try the
> simple C benchmark program at
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91030#c38 ?
> 
> (the kernel will coalesce IO's by itself, so the granularity of IO
> syscalls is not necessarily the same as the actual IO to devices.
> Network filesystems like NFS/Lustre/GPFS may have less latitude here
> due to coherency requirements etc.)
> 

Writing to USB is constrained be the speed of the bus.

kernel: ugen6.3:  at usbus6
kernel: umass1 on uhub0
kernel: umass1:  
on usbus6
kernel: da1 at umass-sim1 bus 1 scbus5 target 0 lun 0
kernel: da1:  Removable Direct Access SPC-4 SCSI 
device
kernel: da1: Serial Number 0C9D927F0A77F330A89D1210
kernel: da1: 40.000MB/s transfers
kernel: da1: 29510MB (60437492 512 byte sectors)
kernel: da1: quirks=0x2

Mounting the thumb drive as a MSDOSFS on FreeBSD.

Test using 1 bytes
Block size of file system: 16384
bs =   1024, 8.86 MiB/s
bs =   2048, 7.52 MiB/s
bs =   4096, 7.32 MiB/s
bs =   8192, 7.32 MiB/s
bs =  16384, 7.40 MiB/s
bs =  32768, 7.40 MiB/s
bs =  65536, 5.57 MiB/s
bs = 131072, 7.43 MiB/s
bs = 262144, 5.55 MiB/s
bs = 524288, 7.43 MiB/s
bs =1048576, 5.56 MiB/s
bs =2097152, 7.39 MiB/s
bs =4194304, 7.62 MiB/s
bs =8388608, 5.58 MiB/s
bs =   16777216, 7.42 MiB/s
bs =   33554432, 5.59 MiB/s
bs =   67108864, 4.77 MiB/s

For the same binary, writing to a UFS2 on a SATAII SSD

Test using 1 bytes
Block size of file system: 32768
bs =   1024, 123.09 MiB/s
bs =   2048, 210.30 MiB/s
bs =   4096, 184.75 MiB/s
bs =   8192, 237.28 MiB/s
bs =  16384, 244.42 MiB/s
bs =  32768, 243.20 MiB/s
bs =  65536, 253.77 MiB/s
bs = 131072, 243.32 MiB/s
bs = 262144, 238.81 MiB/s
bs = 524288, 243.87 MiB/s
bs =1048576, 246.55 MiB/s
bs =2097152, 242.39 MiB/s
bs =4194304, 243.68 MiB/s
bs =8388608, 243.88 MiB/s
bs =   16777216, 242.21 MiB/s
bs =   33554432, 253.19 MiB/s
bs =   67108864, 178.29 MiB/s

-- 
Steve


Re: [PING][AArch64] Use scvtf fbits option where appropriate

2019-07-08 Thread Joel Hutton
On 01/07/2019 18:03, James Greenhalgh wrote:

>> gcc/testsuite/ChangeLog:
>>
>> 2019-06-12  Joel Hutton  
>>
>>   * gcc.target/aarch64/fmul_scvtf_1.c: New test.
> This testcase will fail on ILP32 targets where unsigned long will still
> live in a 'w' register.
Updated to use long long and unsigned long long.

Joel

From e10d5fdb9430799cd2050b8a2f567d1b4e43cde1 Mon Sep 17 00:00:00 2001
From: Joel Hutton 
Date: Mon, 8 Jul 2019 11:59:50 +0100
Subject: [PATCH] SCVTF

---
 gcc/config/aarch64/aarch64-protos.h   |   1 +
 gcc/config/aarch64/aarch64.c  |  23 +++
 gcc/config/aarch64/aarch64.md |  39 +
 gcc/config/aarch64/constraints.md |   7 +
 gcc/config/aarch64/predicates.md  |   4 +
 .../gcc.target/aarch64/fmul_scvtf_1.c | 140 ++
 6 files changed, 214 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/fmul_scvtf_1.c

diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index e2f4cc19e68a79368f939cb8a83cf1f6d0412264..568c2d5846c6501c60de85cfd2fa07e0a9e5831a 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -494,6 +494,7 @@ enum aarch64_symbol_type aarch64_classify_tls_symbol (rtx);
 enum reg_class aarch64_regno_regclass (unsigned);
 int aarch64_asm_preferred_eh_data_format (int, int);
 int aarch64_fpconst_pow_of_2 (rtx);
+int aarch64_fpconst_pow2_recip (rtx);
 machine_mode aarch64_hard_regno_caller_save_mode (unsigned, unsigned,
 		   machine_mode);
 int aarch64_uxt_size (int, HOST_WIDE_INT);
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index a18fbd0f0aa8acc000fd57af5d060961ef0a4e13..0dfcef454a1594497a6bc493d92f7b2b7335a244 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -18750,6 +18750,29 @@ aarch64_fpconst_pow_of_2 (rtx x)
   return exact_log2 (real_to_integer (r));
 }
 
+/* If X is a positive CONST_DOUBLE with a value that is the reciprocal of a
+   power of 2 (i.e 1/2^n) return the number of float bits. e.g. for x==(1/2^n)
+   return n. Otherwise return -1.  */
+
+int
+aarch64_fpconst_pow2_recip (rtx x)
+{
+  REAL_VALUE_TYPE r0;
+
+  if (!CONST_DOUBLE_P (x))
+return -1;
+
+  r0 = *CONST_DOUBLE_REAL_VALUE (x);
+  if (exact_real_inverse (DFmode, )
+  && !REAL_VALUE_NEGATIVE (r0))
+{
+	int ret = exact_log2 (real_to_integer ());
+	if (ret >= 1 && ret <= 32)
+	return ret;
+}
+  return -1;
+}
+
 /* If X is a vector of equal CONST_DOUBLE values and that value is
Y, return the aarch64_fpconst_pow_of_2 of Y.  Otherwise return -1.  */
 
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 4d559c4c928e5949d0494bf384a9ea044cf6fc7c..1b03c1fe71630a72fd00221eb1bbde7f0ba2ac1a 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -6021,6 +6021,44 @@
   [(set_attr "type" "f_cvtf2i")]
 )
 
+;; Equal width integer to fp and multiply combine.
+(define_insn "*aarch64_cvtf2_mult"
+  [(set (match_operand:GPF 0 "register_operand" "=w,w")
+	(mult:GPF (FLOATUORS:GPF
+		   (match_operand: 1 "register_operand" "w,?r"))
+		   (match_operand:GPF 2 "aarch64_fp_pow2_recip" "Dt,Dt")))]
+  "TARGET_FLOAT"
+  {
+operands[2] = GEN_INT (aarch64_fpconst_pow2_recip (operands[2]));
+switch (which_alternative)
+{
+  case 0:
+	return "cvtf\t%0, %1, #%2";
+  case 1:
+	return "cvtf\t%0, %1, #%2";
+  default:
+	gcc_unreachable ();
+}
+  }
+  [(set_attr "type" "neon_int_to_fp_,f_cvti2f")
+   (set_attr "arch" "simd,fp")]
+)
+
+;; Unequal width integer to fp and multiply combine.
+(define_insn "*aarch64_cvtf2_mult"
+  [(set (match_operand:GPF 0 "register_operand" "=w")
+	(mult:GPF (FLOATUORS:GPF
+		   (match_operand: 1 "register_operand" "r"))
+		   (match_operand:GPF 2 "aarch64_fp_pow2_recip" "Dt")))]
+  "TARGET_FLOAT"
+  {
+operands[2] = GEN_INT (aarch64_fpconst_pow2_recip (operands[2]));
+return "cvtf\t%0, %1, #%2";
+  }
+  [(set_attr "type" "f_cvti2f")]
+)
+
+;; Equal width integer to fp conversion.
 (define_insn "2"
   [(set (match_operand:GPF 0 "register_operand" "=w,w")
 (FLOATUORS:GPF (match_operand: 1 "register_operand" "w,?r")))]
@@ -6032,6 +6070,7 @@
(set_attr "arch" "simd,fp")]
 )
 
+;; Unequal width integer to fp conversions.
 (define_insn "2"
   [(set (match_operand:GPF 0 "register_operand" "=w")
 (FLOATUORS:GPF (match_operand: 1 "register_operand" "r")))]
diff --git a/gcc/config/aarch64/constraints.md b/gcc/config/aarch64/constraints.md
index 21f9549e660868900256157ea2f7154164ddd607..b0caa13b4358e89281cd5c0a75f459ceee2040f1 100644
--- a/gcc/config/aarch64/constraints.md
+++ b/gcc/config/aarch64/constraints.md
@@ -329,6 +329,13 @@
   (match_test "aarch64_simd_scalar_immediate_valid_for_move (op,
 		 QImode)")))
 
+(define_constraint "Dt"
+  "@internal
+ A const_double which is the reciprocal of an exact power of two, can be
+ used in an scvtf 

Re: [PATCH, Modula-2 (C/C++/D/F/Go/Jit)] (Register spec fn) (v2)

2019-07-08 Thread Gaius Mulley
Gaius Mulley  writes:

> Hi Rainer,
>
> it rather depends upon what you want, if you want the latest complete
> gm2 grafting onto the svn gcc trunk then these two scripts will create a
> patched tree and also rebuild gm2.
>
>
>
> [however please be careful with the scripts - they do assume that
> everything will be built in $HOME/GM2 - read and adapt as necessary].
>
> These will build a complete gm2 (from the master) - whereas the gcc_trunk
> branch is used to track the patches which are posted to gcc-patches
> (which are currently minimal and just build the gm2 driver).

for clarification - eventually the gcc_trunk branch will contain the
complete compiler as more patches and more tarballs are included.  So
during the integration of gm2 into gcc it should equal the gcc svn trunk
and the current gm2 patch on the gcc-patches mailing list.


regards,
Gaius


Re: [PATCH, Modula-2 (C/C++/D/F/Go/Jit)] (Register spec fn) (v2)

2019-07-08 Thread Rainer Orth
Hi Gaius,

>> I meant to give a build with gm2 included a try on Solaris, but ended up
>> pretty much confused:
>>
>> * I've started with the gm2 repo on savannah.  Running the combine-trees
>>   script on master tried to combine gm2 with gcc 4.7.4.  Trying again
>>   with configure --with-gcc=none (no branch, for trunk?) didn't work
>>   either (don't remember the details off-hand).
>>
>> * Next, I discovered and tried the gcc_trunk branch there.  While it
>>   matches the patch set you sent here, it lacks most of the compiler
>>   proper, which only lives on master!?  In addition, the patches in
>>   there lack support for building libgm2.  Those are present on the
>>   master branch (which has both trunk and trunc in
>>   gcc-versionno/gcc/gm2/patches/gcc).  I tried to merge the trees and
>>   apply the patches manually, but failed again later.
>>
>> At this point, I gave up.  Am I missing something fundamental here?
>>
>> Thanks.
>> Rainer
>
> Hi Rainer,
>
> it rather depends upon what you want, if you want the latest complete
> gm2 grafting onto the svn gcc trunk then these two scripts will create a
> patched tree and also rebuild gm2.

that's my goal: I'd like to see if gm2 and libgm2 build on Solaris and
pass at least a reasonable number of tests.

> [however please be careful with the scripts - they do assume that
> everything will be built in $HOME/GM2 - read and adapt as necessary].

Ok, I will give it a try.

> These will build a complete gm2 (from the master) - whereas the gcc_trunk
> branch is used to track the patches which are posted to gcc-patches
> (which are currently minimal and just build the gm2 driver).

That explains the difference.  While the driver are certainly important
for review, they don't help much by themselves when building the code...

> these tarballs are created using combine-trees, hope this helps,

Certainly.  Thanks a lot.

Rainer

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


PING^4: [PATCH] i386: Properly encode xmm16-xmm31/ymm16-ymm31 for vector move

2019-07-08 Thread H.J. Lu
On Tue, Jun 18, 2019 at 8:59 AM H.J. Lu  wrote:
>
> On Fri, May 31, 2019 at 10:38 AM H.J. Lu  wrote:
> >
> > On Tue, May 21, 2019 at 2:43 PM H.J. Lu  wrote:
> > >
> > > On Fri, Feb 22, 2019 at 8:25 AM H.J. Lu  wrote:
> > > >
> > > > Hi Jan, Uros,
> > > >
> > > > This patch fixes the wrong code bug:
> > > >
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89229
> > > >
> > > > Tested on AVX2 and AVX512 with and without --with-arch=native.
> > > >
> > > > OK for trunk?
> > > >
> > > > Thanks.
> > > >
> > > > H.J.
> > > > --
> > > > i386 backend has
> > > >
> > > > INT_MODE (OI, 32);
> > > > INT_MODE (XI, 64);
> > > >
> > > > So, XI_MODE represents 64 INTEGER bytes = 64 * 8 = 512 bit operation,
> > > > in case of const_1, all 512 bits set.
> > > >
> > > > We can load zeros with narrower instruction, (e.g. 256 bit by inherent
> > > > zeroing of highpart in case of 128 bit xor), so TImode in this case.
> > > >
> > > > Some targets prefer V4SF mode, so they will emit float xorps for 
> > > > zeroing.
> > > >
> > > > sse.md has
> > > >
> > > > (define_insn "mov_internal"
> > > >   [(set (match_operand:VMOVE 0 "nonimmediate_operand"
> > > >  "=v,v ,v ,m")
> > > > (match_operand:VMOVE 1 "nonimmediate_or_sse_const_operand"
> > > >  " C,BC,vm,v"))]
> > > > 
> > > >   /* There is no evex-encoded vmov* for sizes smaller than 64-bytes
> > > >  in avx512f, so we need to use workarounds, to access sse 
> > > > registers
> > > >  16-31, which are evex-only. In avx512vl we don't need 
> > > > workarounds.  */
> > > >   if (TARGET_AVX512F &&  < 64 && !TARGET_AVX512VL
> > > >   && (EXT_REX_SSE_REG_P (operands[0])
> > > >   || EXT_REX_SSE_REG_P (operands[1])))
> > > > {
> > > >   if (memory_operand (operands[0], mode))
> > > > {
> > > >   if ( == 32)
> > > > return "vextract64x4\t{$0x0, %g1, %0|%0, 
> > > > %g1, 0x0}";
> > > >   else if ( == 16)
> > > > return "vextract32x4\t{$0x0, %g1, %0|%0, 
> > > > %g1, 0x0}";
> > > >   else
> > > > gcc_unreachable ();
> > > > }
> > > > ...
> > > >
> > > > However, since ix86_hard_regno_mode_ok has
> > > >
> > > >  /* TODO check for QI/HI scalars.  */
> > > >   /* AVX512VL allows sse regs16+ for 128/256 bit modes.  */
> > > >   if (TARGET_AVX512VL
> > > >   && (mode == OImode
> > > >   || mode == TImode
> > > >   || VALID_AVX256_REG_MODE (mode)
> > > >   || VALID_AVX512VL_128_REG_MODE (mode)))
> > > > return true;
> > > >
> > > >   /* xmm16-xmm31 are only available for AVX-512.  */
> > > >   if (EXT_REX_SSE_REGNO_P (regno))
> > > > return false;
> > > >
> > > >   if (TARGET_AVX512F &&  < 64 && !TARGET_AVX512VL
> > > >   && (EXT_REX_SSE_REG_P (operands[0])
> > > >   || EXT_REX_SSE_REG_P (operands[1])))
> > > >
> > > > is a dead code.
> > > >
> > > > Also for
> > > >
> > > > long long *p;
> > > > volatile __m256i yy;
> > > >
> > > > void
> > > > foo (void)
> > > > {
> > > >_mm256_store_epi64 (p, yy);
> > > > }
> > > >
> > > > with AVX512VL, we should generate
> > > >
> > > > vmovdqa %ymm0, (%rax)
> > > >
> > > > not
> > > >
> > > > vmovdqa64   %ymm0, (%rax)
> > > >
> > > > All TYPE_SSEMOV vector moves are consolidated to ix86_output_ssemov:
> > > >
> > > > 1. If xmm16-xmm31/ymm16-ymm31 registers aren't used, SSE/AVX vector
> > > > moves will be generated.
> > > > 2. If xmm16-xmm31/ymm16-ymm31 registers are used:
> > > >a. With AVX512VL, AVX512VL vector moves will be generated.
> > > >b. Without AVX512VL, xmm16-xmm31/ymm16-ymm31 register to register
> > > >   move will be done with zmm register move.
> > > >
> > > > ext_sse_reg_operand is removed since it is no longer needed.
> > > >
> > > > Tested on AVX2 and AVX512 with and without --with-arch=native.
> > > >
> > > > gcc/
> > > >
> > > > PR target/89229
> > > > PR target/89346
> > > > * config/i386/i386-protos.h (ix86_output_ssemov): New prototype.
> > > > * config/i386/i386.c (ix86_get_ssemov): New function.
> > > > (ix86_output_ssemov): Likewise.
> > > > * config/i386/i386.md (*movxi_internal_avx512f): Call
> > > > ix86_output_ssemov for TYPE_SSEMOV.
> > > > (*movoi_internal_avx): Call ix86_output_ssemov for TYPE_SSEMOV.
> > > > Remove ext_sse_reg_operand and TARGET_AVX512VL check.
> > > > (*movti_internal): Likewise.
> > > > (*movdi_internal): Call ix86_output_ssemov for TYPE_SSEMOV.
> > > > Remove ext_sse_reg_operand check.
> > > > (*movsi_internal): Likewise.
> > > > (*movtf_internal): Call ix86_output_ssemov for TYPE_SSEMOV.
> > > > (*movdf_internal): Call ix86_output_ssemov for TYPE_SSEMOV.
> > > > Remove TARGET_AVX512F, TARGET_PREFER_AVX256, TARGET_AVX512VL
> > > > 

Re: [PATCH, Modula-2 (C/C++/D/F/Go/Jit)] (Register spec fn) (v2)

2019-07-08 Thread Gaius Mulley
Rainer Orth  writes:

> Hi Gaius,
>
>> here is version two of the patches which introduce Modula-2 into the
>> GCC trunk.  The patches include:
>>
>>   (*)  a patch to allow all front ends to register a lang spec function.
>>(included are patches for all front ends to provide an empty
>> callback function).
>>   (*)  patch diffs to allow the Modula-2 front end driver to be
>>built using GCC Makefile and friends.
>>
>> The compressed tarball includes:
>>
>>   (*)  gcc/m2  (compiler driver and lang-spec stuff for Modula-2).
>>Including the need for registering lang spec functions.
>>   (*)  gcc/testsuite/gm2  (a Modula-2 dejagnu test to ensure that
>>the gm2 driver is built and can understands --version).
>>
>> These patches have been re-written after taking on board the comments
>> found in this thread:
>>
>>https://gcc.gnu.org/ml/gcc-patches/2013-11/msg02620.html
>>
>> it is a revised patch set from:
>>
>>https://gcc.gnu.org/ml/gcc-patches/2019-06/msg00220.html
>>
>> I've run make bootstrap and run the regression tests on trunk and no
>> extra failures occur for all languages touched in the ChangeLog.
>>
>> I'm currently tracking gcc trunk and gcc-9 with gm2 (which works well
>> with amd64/arm64/i386) - these patches are currently simply for the
>> driver to minimise the patch size.  There are also > 1800 tests in a
>> dejagnu testsuite for gm2 which can be included at some future time.
>
> I meant to give a build with gm2 included a try on Solaris, but ended up
> pretty much confused:
>
> * I've started with the gm2 repo on savannah.  Running the combine-trees
>   script on master tried to combine gm2 with gcc 4.7.4.  Trying again
>   with configure --with-gcc=none (no branch, for trunk?) didn't work
>   either (don't remember the details off-hand).
>
> * Next, I discovered and tried the gcc_trunk branch there.  While it
>   matches the patch set you sent here, it lacks most of the compiler
>   proper, which only lives on master!?  In addition, the patches in
>   there lack support for building libgm2.  Those are present on the
>   master branch (which has both trunk and trunc in
>   gcc-versionno/gcc/gm2/patches/gcc).  I tried to merge the trees and
>   apply the patches manually, but failed again later.
>
> At this point, I gave up.  Am I missing something fundamental here?
>
> Thanks.
> Rainer

Hi Rainer,

it rather depends upon what you want, if you want the latest complete
gm2 grafting onto the svn gcc trunk then these two scripts will create a
patched tree and also rebuild gm2.



trunk-graft-build-all.tar.gz
Description: build scripts for gcc trunk

[however please be careful with the scripts - they do assume that
everything will be built in $HOME/GM2 - read and adapt as necessary].

These will build a complete gm2 (from the master) - whereas the gcc_trunk
branch is used to track the patches which are posted to gcc-patches
(which are currently minimal and just build the gm2 driver).

There are snapshots available for 9.1.0, 8.2.0, 6.4.0 as well as back to
the older 4.7.4 series.

http://floppsie.comp.glam.ac.uk/download/c/gcc-9.1.0+gm2-git-latest.tar.gz
http://floppsie.comp.glam.ac.uk/download/c/gcc-8.2.0+gm2-git-latest.tar.gz
http://floppsie.comp.glam.ac.uk/download/c/gcc-6.4.0+gm2-git-latest.tar.gz

these tarballs are created using combine-trees, hope this helps,


regards,
Gaius


Re: Prevent tree-ssa-dce.c from deleting stores at -Og

2019-07-08 Thread Jeff Law
On 7/8/19 5:34 AM, Richard Biener wrote:
> On Sun, Jul 7, 2019 at 9:07 PM Jeff Law  wrote:
>>
>> On 7/7/19 3:45 AM, Richard Sandiford wrote:
>>> DCE tries to delete dead stores to local data and also tries to insert
>>> debug binds for simple cases:
>>>
>>>   /* If this is a store into a variable that is being optimized away,
>>>  add a debug bind stmt if possible.  */
>>>   if (MAY_HAVE_DEBUG_BIND_STMTS
>>>   && gimple_assign_single_p (stmt)
>>>   && is_gimple_val (gimple_assign_rhs1 (stmt)))
>>> {
>>>   tree lhs = gimple_assign_lhs (stmt);
>>>   if ((VAR_P (lhs) || TREE_CODE (lhs) == PARM_DECL)
>>> && !DECL_IGNORED_P (lhs)
>>> && is_gimple_reg_type (TREE_TYPE (lhs))
>>> && !is_global_var (lhs)
>>> && !DECL_HAS_VALUE_EXPR_P (lhs))
>>>   {
>>> tree rhs = gimple_assign_rhs1 (stmt);
>>> gdebug *note
>>>   = gimple_build_debug_bind (lhs, unshare_expr (rhs), stmt);
>>> gsi_insert_after (i, note, GSI_SAME_STMT);
>>>   }
>>> }
>>>
>>> But this doesn't help for things like "print *ptr" when ptr points
>>> to the local variable (tests Og-dce-1.c and Og-dce-2.c).  It also tends
>>> to make the *live* -- and thus useful -- values optimised out, because
>>> we can't yet switch back to tracking the memory location as it evolves
>>> over time (test Og-dce-3.c).
>>>
>>> So for -Og I think it'd be better not to delete any stmts with
>>> vdefs for now.  This also means that we can avoid the potentially
>>> expensive vop walks (which already have a cut-off, but still).
>>>
>>> The patch also fixes the Og failures in gcc.dg/guality/pr54970.c
>>> (PR 86638).
>>>
>>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>>>
>>> Richard
>>>
>>>
>>> 2019-07-07  Richard Sandiford  
>>>
>>> gcc/
>>>   PR debug/86638
>>>   * tree-ssa-dce.c (keep_all_vdefs_p): New function.
>>>   (mark_stmt_if_obviously_necessary): Mark all stmts with vdefs as
>>>   necessary if keep_all_vdefs_p is true.
>>>   (mark_aliased_reaching_defs_necessary): Add a gcc_checking_assert
>>>   that keep_all_vdefs_p is false.
>>>   (mark_all_reaching_defs_necessary): Likewise.
>>>   (propagate_necessity): Skip the vuse scan if keep_all_vdefs_p is true.
>>>
>>> gcc/testsuite/
>>>   * c-c++-common/guality/Og-dce-1.c: New test.
>>>   * c-c++-common/guality/Og-dce-2.c: Likewise.
>>>   * c-c++-common/guality/Og-dce-3.c: Likewise.
>> OK
> 
> I wonder how code size (and compile-time) is affected by the DSE/DCE patch?
> Say just look at -Og built cc1?  Can you restrict the keep-all-vdefs to
> user variables (and measure the difference this makes)?  Again I wonder if
> this makes C++ with -Og impractical runtime-wise.I've never really seen DSE 
> be terribly important for any codebase.  It's
unfortunate, but that's my experience.

Jeff


Re: Prevent tree-ssa-dce.c from deleting stores at -Og

2019-07-08 Thread Richard Sandiford
Richard Biener  writes:
> On Sun, Jul 7, 2019 at 9:07 PM Jeff Law  wrote:
>>
>> On 7/7/19 3:45 AM, Richard Sandiford wrote:
>> > DCE tries to delete dead stores to local data and also tries to insert
>> > debug binds for simple cases:
>> >
>> >   /* If this is a store into a variable that is being optimized away,
>> >  add a debug bind stmt if possible.  */
>> >   if (MAY_HAVE_DEBUG_BIND_STMTS
>> >   && gimple_assign_single_p (stmt)
>> >   && is_gimple_val (gimple_assign_rhs1 (stmt)))
>> > {
>> >   tree lhs = gimple_assign_lhs (stmt);
>> >   if ((VAR_P (lhs) || TREE_CODE (lhs) == PARM_DECL)
>> > && !DECL_IGNORED_P (lhs)
>> > && is_gimple_reg_type (TREE_TYPE (lhs))
>> > && !is_global_var (lhs)
>> > && !DECL_HAS_VALUE_EXPR_P (lhs))
>> >   {
>> > tree rhs = gimple_assign_rhs1 (stmt);
>> > gdebug *note
>> >   = gimple_build_debug_bind (lhs, unshare_expr (rhs), stmt);
>> > gsi_insert_after (i, note, GSI_SAME_STMT);
>> >   }
>> > }
>> >
>> > But this doesn't help for things like "print *ptr" when ptr points
>> > to the local variable (tests Og-dce-1.c and Og-dce-2.c).  It also tends
>> > to make the *live* -- and thus useful -- values optimised out, because
>> > we can't yet switch back to tracking the memory location as it evolves
>> > over time (test Og-dce-3.c).
>> >
>> > So for -Og I think it'd be better not to delete any stmts with
>> > vdefs for now.  This also means that we can avoid the potentially
>> > expensive vop walks (which already have a cut-off, but still).
>> >
>> > The patch also fixes the Og failures in gcc.dg/guality/pr54970.c
>> > (PR 86638).
>> >
>> > Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>> >
>> > Richard
>> >
>> >
>> > 2019-07-07  Richard Sandiford  
>> >
>> > gcc/
>> >   PR debug/86638
>> >   * tree-ssa-dce.c (keep_all_vdefs_p): New function.
>> >   (mark_stmt_if_obviously_necessary): Mark all stmts with vdefs as
>> >   necessary if keep_all_vdefs_p is true.
>> >   (mark_aliased_reaching_defs_necessary): Add a gcc_checking_assert
>> >   that keep_all_vdefs_p is false.
>> >   (mark_all_reaching_defs_necessary): Likewise.
>> >   (propagate_necessity): Skip the vuse scan if keep_all_vdefs_p is 
>> > true.
>> >
>> > gcc/testsuite/
>> >   * c-c++-common/guality/Og-dce-1.c: New test.
>> >   * c-c++-common/guality/Og-dce-2.c: Likewise.
>> >   * c-c++-common/guality/Og-dce-3.c: Likewise.
>> OK
>
> I wonder how code size (and compile-time) is affected by the DSE/DCE patch?
> Say just look at -Og built cc1?

Overall I see a ~2.5% slowdown and a 4.7% increase in load size.
That comes almost entirely from the (RTL) DSE side; this patch
and gimple DSE part don't seem to make much difference.

If I keep the gimple passes as-is and just disable RTL DSE, the slowdown
is still ~2.5% and there's a 4.4% increase in load size.

These are all measuring cc1plus (built from post-patch sources)
and using -O2 -g tree-into-ssa.ii for the speed checks.

> Can you restrict the keep-all-vdefs to user variables (and measure the
> difference this makes)?

In order to avoid wrong debug for pointer dereferences, I think it would
have to be keep-all-vdefs for writes to either user variables or unknown
locations.  But as above, I can't measure a significant difference with
the patch.

> Again I wonder if this makes C++ with -Og impractical runtime-wise.

Got a particular test in mind?

Richard


Re: [PATCH][ARM][testsuite] Fix address of sg stubs in CMSE tests

2019-07-08 Thread Christophe Lyon
ping?
I think that's almost obvious?
And maybe should be applied to release branches.

Christophe

On Tue, 2 Jul 2019 at 16:41, Christophe Lyon  wrote:
>
> Hi,
>
> While running the GCC testsuite with an armv8-m target, I noticed that
> a few tests where causing the BFD linker to crash. I opened  PR
> ld/24709 for this [1], but fixing it properly is tricky and not worth
> the headache.
>
> I "fixed" the linker so that it emits a useful error message instead
> of crashing, and on the GCC side the "fix" is simply to avoid placing
> the sg stubs section too far from the destination.
>
> This is what this patch does, by replacing
> --section-start,.gnu.sgstubs=0x2040
> with
> --section-start,.gnu.sgstubs=0x0040
>
> OK?
>
> Thanks,
>
> Christophe


Re: [ARM/FDPIC v5 00/21] FDPIC ABI for ARM

2019-07-08 Thread Christophe Lyon
ping^5?

On Mon, 1 Jul 2019 at 14:15, Christophe Lyon  wrote:
>
> ping^4 ?
> https://gcc.gnu.org/ml/gcc-patches/2019-05/msg00817.html
>
> On Mon, 17 Jun 2019 at 13:41, Christophe Lyon
>  wrote:
> >
> > ping^3 ?
> >
> > On Thu, 6 Jun 2019 at 14:36, Christophe Lyon  
> > wrote:
> > >
> > > Hi,
> > >
> > > If this makes review easier, here are the areas covered by the patches:
> > >
> > > - patches 1,3,4,7,8,9,10,12,21: target-specific
> > > - patch 2: configure
> > > - patch 5,6,11,13: generic parts, undef #if defined(__FDPIC__)
> > > - patches 14-20: testsuite
> > >
> > > Christophe
> > >
> > > On Tue, 4 Jun 2019 at 14:57, Christophe Lyon  
> > > wrote:
> > > >
> > > > Ping?
> > > >
> > > >
> > > > On Thu, 23 May 2019 at 14:46, Christophe Lyon  
> > > > wrote:
> > > > >
> > > > > Ping?
> > > > >
> > > > > Any feedback other than what I got on patch 03/21 ?
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Christophe
> > > > >
> > > > >
> > > > > On 15/05/2019 14:39, Christophe Lyon wrote:
> > > > > > Hello,
> > > > > >
> > > > > > This patch series implements the GCC contribution of the FDPIC ABI 
> > > > > > for
> > > > > > ARM targets.
> > > > > >
> > > > > > This ABI enables to run Linux on ARM MMU-less cores and supports
> > > > > > shared libraries to reduce the memory footprint.
> > > > > >
> > > > > > Without MMU, text and data segments relative distances are different
> > > > > > from one process to another, hence the need for a dedicated FDPIC
> > > > > > register holding the start address of the data segment. One of the
> > > > > > side effects is that function pointers require two words to be
> > > > > > represented: the address of the code, and the data segment start
> > > > > > address. These two words are designated as "Function Descriptor",
> > > > > > hence the "FD PIC" name.
> > > > > >
> > > > > > On ARM, the FDPIC register is r9 [1], and the target name is
> > > > > > arm-uclinuxfdpiceabi. Note that arm-uclinux exists, but uses another
> > > > > > ABI and the BFLAT file format; it does not support code sharing.
> > > > > > The -mfdpic option is enabled by default, and -mno-fdpic should be
> > > > > > used to build the Linux kernel.
> > > > > >
> > > > > > This work was developed some time ago by STMicroelectronics, and was
> > > > > > presented during Linaro Connect SFO15 (September 2015). You can 
> > > > > > watch
> > > > > > the discussion and read the slides [2].
> > > > > > This presentation was related to the toolchain published on github 
> > > > > > [3],
> > > > > > which is based on binutils-2.22, gcc-4.7, uclibc-0.9.33.2, gdb-7.5.1
> > > > > > and qemu-2.3.0, and for which pre-built binaries are available [3].
> > > > > >
> > > > > > The ABI itself is described in details in [1].
> > > > > >
> > > > > > Our Linux kernel patches have been updated and committed by Nicolas
> > > > > > Pitre (Linaro) in July 2017. They are required so that the loader is
> > > > > > able to handle this new file type. Indeed, the ELF files are tagged
> > > > > > with ELFOSABI_ARM_FDPIC. This new tag has been allocated by ARM, as
> > > > > > well as the new relocations involved.
> > > > > >
> > > > > > The binutils, QEMU and uclibc-ng patch series have been merged a few
> > > > > > months ago. [4][5][6]
> > > > > >
> > > > > > This series provides support for architectures that support ARM 
> > > > > > and/or
> > > > > > Thumb-2 and has been tested on arm-linux-gnueabi without regression,
> > > > > > as well as arm-uclinuxfdpiceabi, using QEMU. arm-uclinuxfdpiceabi 
> > > > > > has
> > > > > > a few more failures than arm-linux-gnueabi, but is quite functional.
> > > > > >
> > > > > > I have also booted an STM32 board (stm32f469) which uses a cortex-m4
> > > > > > with linux-4.20.17 and ran successfully several tools.
> > > > > >
> > > > > > Are the GCC patches OK for inclusion in master?
> > > > > >
> > > > > > Changes between v4 and v5:
> > > > > > - rebased on top of recent gcc-10 master (April 26th, 2019)
> > > > > > - fixed handling of stack-protector combined patterns in FDPIC mode
> > > > > >
> > > > > > Changes between v3 and v4:
> > > > > >
> > > > > > - improved documentation (patch 1)
> > > > > > - emit an error message (sorry) if the target architecture does not
> > > > > >support arm nor thumb-2 modes (patch 4)
> > > > > > - handle Richard's comments on patch 4 (comments, unspec)
> > > > > > - added .align directive (patch 5)
> > > > > > - fixed use of kernel helpers (__kernel_cmpxchg, __kernel_dmb) 
> > > > > > (patch 6)
> > > > > > - code factorization in patch 7
> > > > > > - typos/internal function name in patch 8
> > > > > > - improved patch 12
> > > > > > - dropped patch 16
> > > > > > - patch 20 introduces arm_arch*_thumb_ok effective targets to help
> > > > > >skip some tests
> > > > > > - I tested patch 2 on xtensa-buildroot-uclinux-uclibc, it adds many
> > > > > >new tests, but a few regressions
> > > > > >(https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00713.html)

Re: Don't run DSE at -Og

2019-07-08 Thread Richard Sandiford
Richard Biener  writes:
> On Sun, Jul 7, 2019 at 9:04 PM Jeff Law  wrote:
>>
>> On 7/7/19 3:43 AM, Richard Sandiford wrote:
>> > This patch stops gimple and rtl DSE from running by default at -Og.
>> > The idea is both to improve compile time and to stop us from deleting
>> > stores that we can't track in debug info.
>> >
>> > We could rein this back in future for stores to local variables
>> > with is_gimple_reg_type, but at the moment we don't have any
>> > infrastructure for switching between binds to specific values
>> > and binds to evolving memory locations.  Even then, location
>> > tracking only works for direct references to the variables, and doesn't
>> > for example help with printing dereferenced pointers (see the next patch
>> > in the series for an example).
>> >
>> > I'm also not sure that DSE is important enough for -Og to justify the
>> > compile time cost -- especially in the case of RTL DSE, which is pretty
>> > expensive.
>> >
>> > Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>> >
>> > Richard
>> >
>> >
>> > 2019-07-07  Richard Sandiford  
>> >
>> > gcc/
>> >   * common.opt (Og): Change the initial value of flag_dse to 0.
>> >   * opts.c (default_options_table): Move OPT_ftree_dse from
>> >   OPT_LEVELS_1_PLUS to OPT_LEVELS_1_PLUS_NOT_DEBUG.  Also add
>> >   OPT_fdse to OPT_LEVELS_1_PLUS_NOT_DEBUG.  Put the OPT_ftree_pta
>> >   entry before the OPT_ftree_sra entry.
>> >   * doc/invoke.texi (Og): Add -fdse and -ftree-dse to the list
>> >   of flags disabled by Og.
>> >
>> > gcc/testsuite/
>> >   * c-c++-common/guality/Og-global-dse-1.c: New test.
>> OK.  I doubt DSE is at all important if someone has asked for Og.
>
> Hmm.  -Og also asks for runtime performance.  Can we instead restrict
> DSE to artificial decls?  I also fear that this leaves a _lot_ of abstraction
> for C++ code given we also do not run SRA with -Og even though that
> has means to preserve debug info.

Following up your question in the other patch: I see a 2.5% slowdown for
cc1plus bulit at -Og when compiling tree-into-ssa.ii at -O2 -g.  I think
it mostly comes from removing RTL DSE.  I get a very similar slowdown with
gimple DSE reenabled.

There again, we're not very C++-heavy, so cc1plus might not be a good
test for that.

Richard


Re: [PATCH 0/3] S/390: Shift count improvements.

2019-07-08 Thread Andreas Krebbel
On 06.07.19 22:53, Robin Dapp wrote:
> Hi,
> 
> these patches introduce a new predicate that recognizes
> shift-count operands instead of the subst patterns we
> used before.  This allows introducing (no-op) subregs in
> the patterns which was not possible via subst before
> (see https://gcc.gnu.org/ml/gcc-patches/2019-05/msg00853.html).
> The second patch adds some tests.
> 
> The third patch defines the shift_truncation_mask and adds
> a test for it.
> 
> Bootstrapped and regtested.
> 
> Regards
>  Robin
> 
> ---
> 
> Robin Dapp (3):
>   S/390: Rework shift count handling.
>   S/390: Shift count tests.
>   S/390: Define shift_truncation_mask.

Ok. Thanks!

Andreas

> 
>  gcc/config/s390/constraints.md |  12 ++
>  gcc/config/s390/predicates.md  |  29 +
>  gcc/config/s390/s390-protos.h  |   1 +
>  gcc/config/s390/s390.c |  59 +-
>  gcc/config/s390/s390.md|  43 +++
>  gcc/config/s390/subst.md   |  72 
>  gcc/config/s390/vector.md  |  14 ++-
>  .../gcc.target/s390/combine-rotate-modulo.c|  36 ++
>  .../s390/combine-shift-rotate-add-mod.c|  29 +
>  .../gcc.target/s390/vector/combine-shift-vec.c | 107 ++
>  .../gcc.target/s390/rotate-truncation-mask.c   |  11 ++
>  11 files changed, 314 insertions(+), 99 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/s390/combine-rotate-modulo.c
>  create mode 100644 
> gcc/testsuite/gcc.target/s390/combine-shift-rotate-add-mod.c
>  create mode 100644 gcc/testsuite/gcc.target/s390/vector/combine-shift-vec.c
>  create mode 100644 gcc/testsuite/gcc.target/s390/rotate-truncation-mask.c
> 



Re: [PATCH,RFC,V3 0/5] Support for CTF in GCC

2019-07-08 Thread Nix
On 5 Jul 2019, Jakub Jelinek outgrape:

> On Fri, Jul 05, 2019 at 07:28:12PM +0100, Nix wrote:
>> > What makes it superior to DWARF stripped down to the above feature set?
>> 
>> Increased compactness. DWARF fundamentally trades off compactness in
>> favour of its regular structure, which makes it easier to parse (but not
>> easier to interpret) but very hard to make it much smaller than it is
>> now. Where DWARF uses word-sized and larger entities for everything, CTF
>> packs everything much more tightly -- and this is quite visible in the
>
> That is just not true, most of the data in DWARF are uleb128/sleb128
> encoded, or often even present just in the abbreviation table
> (DW_FORM_flag_present, DW_FORM_implicit_const), word-sized is typically only
> stuff that needs relocations (at assembly time and more importantly at link
> time).

Hm. I may have misread the spec.

The fact remains that DWARF is (in typical usage) both large and slow to
use: it is not entirely untrue to say that you can spot a DWARF consumer
because it takes ten seconds to start up. This may be something that can
be avoided with sufficiently clever implementations, but I've never seen
any such implementation and we don't appear to be approaching one
terribly fast :( meanwhile, in CTF we already have a working system that
can reduce multigigabyte DWARF input down to 6MiB of compressed CTF
loading in fractions of a second, though it is true that not all of that
input was global-scope type info, so a large portion of that
multigigabyte input would simply have been dropped and should not be
considered relevant. I'm not sure how to determine how much of the input
is type DIEs at global scope... (The 6MiB figure is slightly misleading,
too, since only 1439845 bytes of that is type data: the rest is mostly
compressed string table.)

Possibly sufficiently clever deduplication can do a similar scrunching
job for DWARF, but I note that what DWARF deduplication GCC did in
earlier releases has subsequently been removed because it never really
worked very well. (Having written code that deduplicates DWARF, I can
see why: it's a complex job when you just have C to think about. Doing
it for C++ as well must have made people's brains dribble out of their
ears).

Type signatures in DWARF 4 were supposed to provide this sort of thing,
too, but yet again the promise does not seem to have been borne out:
DWARF debuginfo remains immense and there is no discussion of leaving
unstripped binaries on production systems for the sake of continuous
tracing tools or introspection, because the debuginfo in those binaries
would still be many times the size of the binaries they relate to, and
obviously leaving it unstripped in that case is ridiculous. Meanwhile,
FreeBSD has a leg-up in continuous debugging because they generate (an
older form of) CTF for everything and deduplicate it, and it's small
enough that they can leave it linked into the binaries rather than
stripping it out, and tracers can and do use it. I'm trying to give us
all that advantage, while not leaving us tied to a format with as many
limitations as FreeBSD's CTF.


As a side note, I tried switching to ULEB128 for the representations of
unsigned integers in CTF a while back, but never even pushed it anywhere
because while it shrank the output a little, the compressed sizes
worsened noticeably, by about 10%, and we don't want to hurt the
compressed sizes any more than we do the uncompressed ones. I found this
quite annoying. So I'm not convinced that ULEB actually buys you
much of anything once compressors get into the mix.

Something similar happened when I tried to do clever things with string
tables last month, sharing common string suffixes, slicing strtabs up on
underscores and changes of case and replacing strings where beneficial
with offset tables pointing into the sliced-up pieces: the uncompressed
size shrank by about 50% and the compressed size grew by 20%... I found
this *very* annoying. :)

> For DWARF you also have various techniques at reducing size and optimizing
> redundancies away, look e.g. at the dwz utility.

... interesting! I'll be looking through this and seeing if any of it is
applicable to CTF as well, that's for sure.


Re: [committed] OpenMP scan for combined for simd

2019-07-08 Thread Rainer Orth
Hi Jakub,

> The following patch handles the last yet unsupported scan case,
> composite #pragma omp {,parallel }for simd ... reduction(inscan, ...) ...
> where we want to both parallelize and vectorize; in the first worksharing
> loop use normal scan support we have for
> #pragma omp simd ... reduction(inscan, ...) ...
> and just store those into the per-thread array, then do the normal
> worksharing loop up/down-sweep and then in the second worksharing loop
> use a simple #pragma omp simd that just combines the two partial sums
> and does the user's scan phase.

some of the new testcases come up as UNRESOLVED on
sparc-sun-solaris2.11, both 32 and 64-bit:

UNRESOLVED: libgomp.c++/scan-13.C scan-tree-dump-times vect "vectorized [2-6] 
loops" 2
UNRESOLVED: libgomp.c++/scan-16.C scan-tree-dump-times vect "vectorized [2-6] 
loops" 2

The log shows

libgomp.c++/scan-13.C: dump file does not exist

libgomp.c++/scan-16.C: dump file does not exist

There are similar gcc-testreports results for
mips64el-unknown-linux-gnu, ia64-suse-linux-gnu, m68k-unknown-linux-gnu,
powerpc-ibm-aix7.2.0.0, hppa-unknown-linux-gnu.

While I don't see such failures on i386-pc-solaris2.11, gcc-testresults
shows a couple of cases where all libgomp.c++/scan-1[0-6].C are
unresolved on x86_64-pc-linux-gnu.

Something fishy seems to be going on here.

Rainer

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


Re: [patch, libfortran] Adjust block size for libgfortran for unformatted reads

2019-07-08 Thread Janne Blomqvist
On Sun, Jul 7, 2019 at 11:13 PM Thomas Koenig  wrote:
>
> Hello world,
>
> the attached patch sets the I/O block size for unformatted files to
> 2**17 and makes this, and the block size for formatted files,
> adjustable via environment variables.

Should the default be affected by the page size
(sysconf(_SC_PAGESIZE)) and/or the IO blocksize (we already fstat() a
file when we open it, so we could get st_blksize for free)?

Though one could of course argue those aren't particularly useful;
except for power, everything uses a 4k page size (?), and the IO
blocksize varies from too small (4k, ext4) to too large (1 MB, NFS (or
whatever rsize/wsize is set to), or 4 MB for Lustre).

> 2019-07-07  Thomas König  
>
> PR libfortran/91030
> * gfortran.texi (GFORTRAN_BUFFER_SIZE_FORMATTED): Document
> (GFORTRAN_BUFFER_SIZE_FORMATTED): Likewise.

One of these should be _UNFORMATTED?

-- 
Janne Blomqvist


**ping** Re: [PATCH] Automatics in equivalence statements

2019-07-08 Thread Mark Eggleston

**ping**

On 01/07/2019 10:35, Mark Eggleston wrote:


On 25/06/2019 14:17, Mark Eggleston wrote:


On 25/06/2019 00:17, Jeff Law wrote:

On 6/24/19 2:19 AM, Bernhard Reutner-Fischer wrote:

On Fri, 21 Jun 2019 07:10:11 -0700
Steve Kargl  wrote:


On Fri, Jun 21, 2019 at 02:31:51PM +0100, Mark Eggleston wrote:
Currently variables with the AUTOMATIC attribute can not appear 
in an
EQUIVALENCE statement. However its counterpart, STATIC, can be 
used in

an EQUIVALENCE statement.

Where there is a clear conflict in the attributes of variables in an
EQUIVALENCE statement an error message will be issued as is 
currently

the case.

If there is no conflict e.g. a variable with a AUTOMATIC 
attribute and a

variable(s) without attributes all variables in the EQUIVALENCE will
become AUTOMATIC.

Note: most of this patch was written by Jeff Law 

Please review.

ChangeLogs:

gcc/fortran

      Jeff Law  
      Mark Eggleston 

      * gfortran.h: Add check_conflict declaration.

This is wrong.  By convention a routine that is not static
has the gfc_ prefix.

Updated the code to use gfc_check_conflict instead.



Furthermore doesn't this export indicate that you're committing a
layering violation somehow?

Possibly.  I'm the original author, but my experience in our fortran
front-end is minimal.  I fully expected this patch to need some 
tweaking.


We certainly don't want to recreate all the checking that's done in
check_conflict.  We just need to defer it to a later point --
find_equivalence seemed like a good point since we've got the full
equivalence list handy and can accumulate the attributes across the
entire list, then check for conflicts.

If there's a concrete place where you think we should be doing this, 
I'm

all ears.


Any suggestions will be appreciate.
      * symbol.c (check_conflict): Remove automatic in equivalence 
conflict

      check.
      * symbol.c (save_symbol): Add check for in equivalence to 
stop the

      the save attribute being added.
      * trans-common.c (build_equiv_decl): Add is_auto parameter and
      add !is_auto to condition where TREE_STATIC (decl) is set.
      * trans-common.c (build_equiv_decl): Add local variable 
is_auto,
      set it true if an atomatic attribute is encountered in the 
variable

atomatic? I read atomic but you mean automatic.

Yes.

      list.  Call build_equiv_decl with is_auto as an additional 
parameter.

      flag_dec_format_defaults is enabled.
      * trans-common.c (accumulate_equivalence_attributes) : New 
subroutine.
      * trans-common.c (find_equivalence) : New local variable 
dummy_symbol,
      accumulated equivalence attributes from each symbol then 
check for

      conflicts.
I'm just curious why you don't gfc_copy_attr for the most part of 
accumulate_equivalence_attributes?

thanks,

Simply didn't know about it.  It could probably significantly simplify
the accumulation of attributes step.
Using gfc_copy_attr causes a great many "Duplicate DIMENSION 
attribute specified at (1)" errors. This is because there is a great 
deal of checking done instead of simply keeping track of the 
attributes used which is all that is required for determining whether 
there is a conflict in the equivalence statement.


Also, the final section of accumulate_equivalence_attributes 
involving SAVE, INTENT and ACCESS look suspect to me. I'll check and 
update the patch if necessary.


No need to check intent as there is already a conflict with DUMMY and 
INTENT can only be present for dummy variables.


Please find attached an updated patch. Change logs:

gcc/fortran

    Jeff Law  
    Mark Eggleston  

    * gfortran.h: Add gfc_check_conflict declaration.
    * symbol.c (check_conflict): Rename cfg_check_conflict and remove
    static.
    * symbol.c (cfg_check_conflict): Remove automatic in equivalence
    conflict check.
    * symbol.c (save_symbol): Add check for in equivalence to stop the
    the save attribute being added.
    * trans-common.c (build_equiv_decl): Add is_auto parameter and
    add !is_auto to condition where TREE_STATIC (decl) is set.
    * trans-common.c (build_equiv_decl): Add local variable is_auto,
    set it true if an atomatic attribute is encountered in the variable
    list.  Call build_equiv_decl with is_auto as an additional parameter.
    flag_dec_format_defaults is enabled.
    * trans-common.c (accumulate_equivalence_attributes) : New 
subroutine.
    * trans-common.c (find_equivalence) : New local variable 
dummy_symbol,

    accumulated equivalence attributes from each symbol then check for
    conflicts.

gcc/testsuite

    Mark Eggleston 

    * gfortran.dg/auto_in_equiv_1.f90: New test.
    * gfortran.dg/auto_in_equiv_2.f90: New test.
    * gfortran.dg/auto_in_equiv_3.f90: New test.

If the updated patch is acceptable, please can someone with the 
privileges commit the patch.


Mark




Jeff




--
https://www.codethink.co.uk/privacy.html

>From 321c7c84f9578e99ac0a1fa5f3ed1fd78b328d1f Mon Sep 17 00:00:00 2001

Re: [PATCH, committed] Add myself to MAINTAINERS

2019-07-08 Thread Kito Cheng
Hi Jakub:

Thanks, fixed and checked with maintainers-verify.sh now.

2019-07-08  Kito Cheng  

* MAINTAINERS (Write After Approval): Remove myself, already listed in
RISC-V port maitainer.


Index: MAINTAINERS
===
--- MAINTAINERS (revision 273234)
+++ MAINTAINERS (revision 273235)
@@ -345,7 +345,6 @@
 Chandra Chavva 
 Dehao Chen 
 Fabien Chêne   
-Kito Cheng 
 Harshit Chopra 
 Tamar Christina

 Eric Christopher   

On Mon, Jul 8, 2019 at 8:48 PM Jakub Jelinek  wrote:
>
> On Mon, Jul 08, 2019 at 05:16:26PM +0800, Kito Cheng wrote:
> > Hi Rainer:
> >
> > Thanks your reminder, I've fix it now.
>
> Still not correct, there is FAIL maintainers-verify.sh.
> You are already since r246280 listed as riscv CPU port maintainer (one of 4)
> and maintainers should not be listed in the write after approval section,
> they are already listed in the maintainers (or reviewers) section(s).
> Please remove yourself from Write After Approval.
>
> Jakub


Re: [patch, libfortran] Adjust block size for libgfortran for unformatted reads

2019-07-08 Thread Janne Blomqvist
On Mon, Jul 8, 2019 at 11:18 AM Manfred Schwarb  wrote:
>
> Am 07.07.19 um 22:13 schrieb Thomas Koenig:
> > Hello world,
> >
> > the attached patch sets the I/O block size for unformatted files to
> > 2**17 and makes this, and the block size for formatted files,
> > adjustable via environment variables.
> >
> > The main reason is that -fconvert=big-endian was quite slow on
> > some HPC systems. A bigger buffer should eliminate that.  Also,
> > People who use unformatted files are likely to write large amounts
> > of data, so this seems like a good fit.  Finally, some benchmarking
> > showed that 131072 seemed like a good value to use. Thanks to Jerry
> > for support.
> >
> > I didn't change the value for formatted files because, frankly, we are
> > using a lot of CPU for converting numbers there, so any gain
> > negligible (unless somebody comes up with a benchmark which says
> > otherwise).
>
> formatted write: Did you try writing to an USB stick or similar? I guess
> for flash based devices anything below 64k will slow down operation.
> Computers like Raspberry Pi and the like often have flash based storage,
> and it is not extremely unrealistic to run fortran programs on them.

Good point. If you happen to have a USB stick handy, can you try the
simple C benchmark program at
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91030#c38 ?

(the kernel will coalesce IO's by itself, so the granularity of IO
syscalls is not necessarily the same as the actual IO to devices.
Network filesystems like NFS/Lustre/GPFS may have less latitude here
due to coherency requirements etc.)


--
Janne Blomqvist


Re: [PATCH 1/2] Come up with function_decl_type and use it in tree_function_decl.

2019-07-08 Thread Martin Liška
On 7/5/19 12:09 AM, Marc Glisse wrote:
> On Wed, 3 Jul 2019, Richard Biener wrote:
> 
>> On July 3, 2019 4:53:30 PM GMT+02:00, "Martin Liška"  wrote:
>>> On 7/2/19 7:15 PM, Marc Glisse wrote:
 On Tue, 2 Jul 2019, Martin Liška wrote:

> After the discussion with Richi and Nathan, I made a place in
>>> tree_function_decl
> and I rebased the original Dominik's patch on top of that.

 So, last time there were some questions about the legality of this
>>> transformation. Did you change the exact set of functions on which this
>>> is applied?
>>>
>>> Yes. I was not included in the original discussion, but I hope the
>>> transformation is valid.
>>> Btw. clang also removes the new/delete pairs and I guess it was the
>>> original motivation of the patch.
>>
>> We also remove malloc/free pairs which the C standard does not explicitly 
>> allow (but also doesn't explicitly forbid). I don't think standards need to 
>> enumerate everything allowed and I don't know any explicit wording in the 
>> C++ standard that forbids this.
> 
> The C++ standard has explicit wording allowing to remove new-delete pairs in 
> some circumstances (expr.new, allocator.members), so I would assume that 
> other circumstances are forbidden (not that I care much, I am just afraid 
> someone might).

I hope some C++ FE maintainers can help us here?

> 
> The patch apparently has DECL_IS_OPERATOR_DELETE only on the replaceable 
> global deallocation functions, not all delete operators, contrary to 
> DECL_IS_OPERATOR_NEW, so the name is misleading. On the other hand, those 
> seem to be the ones for which the optimization is legal (well, not quite, the 
> rules are in terms of operator new, and I am not sure how well operator 
> delete has to match, but close enough).

Are you talking about this location where we set OPERATOR_NEW:
https://github.com/gcc-mirror/gcc/blob/master/gcc/cp/decl.c#L13643
?

That's the only place where we set OPERATOR_NEW flag and not OPERATOR_DELETE.

Thanks,
Martin

> 
>> It's only that users can override the allocation functions (but so can they 
>> in C) and it was suggested we need to preserve side effects unknown to the 
>> compiler.
> 



Re: [PATCH, committed] Add myself to MAINTAINERS

2019-07-08 Thread Jakub Jelinek
On Mon, Jul 08, 2019 at 05:16:26PM +0800, Kito Cheng wrote:
> Hi Rainer:
> 
> Thanks your reminder, I've fix it now.

Still not correct, there is FAIL maintainers-verify.sh.
You are already since r246280 listed as riscv CPU port maintainer (one of 4)
and maintainers should not be listed in the write after approval section,
they are already listed in the maintainers (or reviewers) section(s).
Please remove yourself from Write After Approval.

Jakub


[C++] DEFERRED_PARSE

2019-07-08 Thread Nathan Sidwell

Jason, Marek,
can DEFERRED_PARSE trees survive past the in-class-context late parsing 
stage?  My assumption was not, but in reducing a module testcase I 
encountered a situation when one survived to end of compilation (with no 
errors).  It was an exception specifier on a declared-but-not-defined 
template member function.


Is my assumption incorrect?  (I can of course further reduce the 
testcase, if needed.)


nathan
--
Nathan Sidwell


Re: [bikeshed] include path search for main file

2019-07-08 Thread Nathan Sidwell

On 7/8/19 7:58 AM, Richard Biener wrote:

On Mon, Jul 8, 2019 at 1:46 PM Nathan Sidwell  wrote:


One of the facilities C++ modules provides is turning header files into
header units.  Thus you can say:
import ;
(there are some constraints on being able to do that)

This involves compiling the header file, similar to PCH.  However,
needing to provide the full path to the header file on the command line
is awkward, and creates adoption friction, because it's one of the first
thing someone might do.

So, I've added a mechanism to search:
 g++ --lang c++ -std=c++17 -fmodule-header -search-system iostream

I've decoupled these new options from what one does with the file (hence
the -fmodule-header), or what language (hence --lang c++).

Question: Are -search-system and -search-user sensible names? Should
they be more clearly tied to the main file?


Hmm.  How would this integrate with makefiles and dependency generation?
IMHO that's something to try get right first before the above.


If you have the full path, you can still use it (which is what I imagine 
the second iteration of a build might have).  Oh, and I do have a hacked 
up Make that can communicate with in-flight compiles about this stuff. 
It's a little bitrotten right now, but demoed at the February C++ 
meeting.  It was a PoC that build systems could cope.  In that situation 
GCC asks using the resolved pathname of the header file.  (The hacked 
Make is not *required* for C++ modules, it just makes things easier.)


As I said, this is addressing users' perception of ease.  I've already 
been asked, 'I have to wrap the system headers in another header?', 
followed by 'why doesn't the compiler already know where iostream is?'. 
Also, 'why doesn't the compiler just go build it for me?'


nathan

--
Nathan Sidwell


Re: [PATCH] Deprecate -frepo option.

2019-07-08 Thread Martin Liška
On 6/21/19 4:28 PM, Richard Biener wrote:
> On Fri, Jun 21, 2019 at 4:13 PM Jakub Jelinek  wrote:
>>
>> On Fri, Jun 21, 2019 at 04:04:00PM +0200, Martin Liška wrote:
>>> On 6/21/19 1:58 PM, Jakub Jelinek wrote:
 On Fri, Jun 21, 2019 at 01:52:09PM +0200, Martin Liška wrote:
> On 6/21/19 1:47 PM, Jonathan Wakely wrote:
>> On Fri, 21 Jun 2019 at 11:40, Martin Liška wrote:
>>> Yes, I would be fine to deprecate that for GCC 10.1
>>
>> Would it be appropriate to issue a warning in GCC 10.x if the option is 
>> used?
>
> Sure. With the patch attached one will see:
>
> $ gcc -frepo /tmp/main.cc -c
> gcc: warning: switch ‘-frepo’ is no longer supported
>
> I'm sending patch that also removes -frepo tests from test-suite.
> I've been testing the patch.

 IMHO for just deprecation of an option you don't want to remove it from the
 testsuite, just match the warning it will generate in those tests, and
 I'm not convinced you want to remove it from the documentation (rather than
 just saying in the documentation that the option is deprecated and might be
 removed in a later GCC version).
>>>
>>> Agree with you. I'm sending updated version of the patch.
>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> I'm also not convinced about the Deprecated flag, seems like that is a flag
>> that we use for options that have been already removed.
>> So, instead there should be some proper warning in the C++ FE for it,
>> or just Warn.
> 
> In principle -frepo is a nice idea - does it live up to its promises?  That 
> is,
> does it actually work, for example when throwing it on the libstdc++
> testsuite or a larger C++ project? 

I've just tested tramp3d, and it does not survive linking:

g++ tramp3d-v4.o
collect: recompiling tramp3d-v4.cpp
collect: relinking
collect: recompiling tramp3d-v4.cpp
collect: relinking
collect: recompiling tramp3d-v4.cpp
collect: relinking
collect: recompiling tramp3d-v4.cpp
collect: relinking
collect: recompiling tramp3d-v4.cpp
collect: relinking
collect: recompiling tramp3d-v4.cpp
collect: relinking
collect: recompiling tramp3d-v4.cpp
collect: relinking
collect: recompiling tramp3d-v4.cpp
collect: relinking
collect: recompiling tramp3d-v4.cpp
collect: relinking
collect: recompiling tramp3d-v4.cpp
collect: relinking
collect: recompiling tramp3d-v4.cpp
collect: relinking
collect: recompiling tramp3d-v4.cpp
collect: relinking
collect: recompiling tramp3d-v4.cpp
collect: relinking
collect: recompiling tramp3d-v4.cpp
collect: relinking
collect: recompiling tramp3d-v4.cpp
collect: relinking
collect: recompiling tramp3d-v4.cpp
collect: relinking
collect: recompiling tramp3d-v4.cpp
collect: relinking
/usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: 
/tmp/ccEeuyj7.ltrans0.ltrans.o: in function 
`RefCountedBlockPtr, 
ViewEngine<3, IndexFunction >::PositionsFunctor> > >, false, 
RefBlockController, 
ViewEngine<3, IndexFunction >::PositionsFunctor> > > > 
>::RefCountedBlockPtr(RefCountedBlockPtr, ViewEngine<3, IndexFunction >::PositionsFunctor> > >, false, 
RefBlockController, 
ViewEngine<3, IndexFunction >::PositionsFunctor> > > > > const&)':
:(.text+0x4181b): undefined reference to 
`RefCountedPtr, ViewEngine<3, IndexFunction >::PositionsFunctor> > > > 
>::RefCountedPtr(RefCountedPtr, ViewEngine<3, IndexFunction >::PositionsFunctor> > > > > 
const&)'
/usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: 
/tmp/ccEeuyj7.ltrans0.ltrans.o: in function `std::_Vector_base, 
Interval<3> >, std::allocator, Interval<3> > > 
>::_Vector_impl::~_Vector_impl()':
:(.text+0xc1890): undefined reference to 
`std::allocator, Interval<3> > >::~allocator()'
/usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: 
/tmp/ccEeuyj7.ltrans0.ltrans.o: in function `std::_Vector_base, 
Interval<3> >, std::allocator, Interval<3> > > >::_Vector_base()':
:(.text+0xc18aa): undefined reference to 
`std::_Vector_base, Interval<3> >, std::allocator, 
Interval<3> > > >::_Vector_impl::_Vector_impl()'
/usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: 
/tmp/ccEeuyj7.ltrans0.ltrans.o: in function `std::vector, 
std::allocator > >::_S_use_relocate()':
:(.text+0xc496f): undefined reference to `std::vector, 
std::allocator > >::_S_nothrow_relocate(std::integral_constant)'
/usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: 
/tmp/ccEeuyj7.ltrans0.ltrans.o: in function `std::_Vector_base, 
Interval<3> >, std::allocator, Interval<3> > > 
>::~_Vector_base()':
:(.text+0xc4cc1): undefined reference to 
`std::_Vector_base, Interval<3> >, std::allocator, 
Interval<3> > > >::_M_deallocate(Node, Interval<3> >*, unsigned long)'
/usr/lib64/gcc/x86_64-suse-linux/9/../../../../x86_64-suse-linux/bin/ld: 
/tmp/ccEeuyj7.ltrans0.ltrans.o: in function `DataBlockPtr::DataBlockPtr()':
:(.text+0xc6f96): undefined reference to 
`RefCountedBlockPtr 

[PATCH] Re-instantiate access-path based analysis during VN

2019-07-08 Thread Richard Biener


This re-instantiates the patch I had to revert earlier, doing it in
a safer way.  We record the original ref so we can do an additional
disambiguation during vn_reference_lookup_3.

Bootstrap and regtest running on x86_64-unknown-linux-gnu.

Richard.

2019-07-08  Richard Biener  

* tree-ssa-sccvn.c (struct vn_walk_cb_data): Add orig_ref member.
(vn_reference_lookup_3): If the main ref has no access path recorded
but orig_ref has use it to do access-path based disambiguation.
(vn_reference_lookup_pieces): Adjust.
(vn_reference_lookup): Pass down original ref if we valueized.

* gcc.dg/tree-ssa/alias-access-path-1.c: Scan fre1 dump.
* gcc.dg/tree-ssa/alias-access-path-2.c: Likewise.
* gcc.dg/tree-ssa/alias-access-path-8.c: Likewise.

Index: gcc/tree-ssa-sccvn.c
===
--- gcc/tree-ssa-sccvn.c(revision 273232)
+++ gcc/tree-ssa-sccvn.c(working copy)
@@ -1670,15 +1670,18 @@ struct pd_data
 
 struct vn_walk_cb_data
 {
-  vn_walk_cb_data (vn_reference_t vr_, tree *last_vuse_ptr_,
+  vn_walk_cb_data (vn_reference_t vr_, tree orig_ref_, tree *last_vuse_ptr_,
   vn_lookup_kind vn_walk_kind_, bool tbaa_p_)
-: vr (vr_), last_vuse_ptr (last_vuse_ptr_), vn_walk_kind (vn_walk_kind_),
-  tbaa_p (tbaa_p_), known_ranges (NULL)
-   {}
+: vr (vr_), last_vuse_ptr (last_vuse_ptr_),
+  vn_walk_kind (vn_walk_kind_), tbaa_p (tbaa_p_), known_ranges (NULL)
+   {
+ ao_ref_init (_ref, orig_ref_);
+   }
   ~vn_walk_cb_data ();
   void *push_partial_def (const pd_data& pd, tree, HOST_WIDE_INT);
 
   vn_reference_t vr;
+  ao_ref orig_ref;
   tree *last_vuse_ptr;
   vn_lookup_kind vn_walk_kind;
   bool tbaa_p;
@@ -2246,6 +2249,28 @@ vn_reference_lookup_3 (ao_ref *ref, tree
  lhs_ref_ok = true;
}
 
+  /* Besides valueizing the LHS we can also use access-path based
+ disambiguation on the original non-valueized ref.  */
+  if (!ref->ref
+ && lhs_ref_ok
+ && data->orig_ref.ref)
+   {
+ /* We want to use the non-valueized LHS for this, but avoid redundant
+work.  */
+ ao_ref *lref = _ref;
+ ao_ref lref_alt;
+ if (valueized_anything)
+   {
+ ao_ref_init (_alt, lhs);
+ lref = _alt;
+   }
+ if (!refs_may_alias_p_1 (>orig_ref, lref, data->tbaa_p))
+   {
+ *disambiguate_only = true;
+ return NULL;
+   }
+   }
+
   /* If we reach a clobbering statement try to skip it and see if
  we find a VN result with exactly the same value as the
 possible clobber.  In this case we can ignore the clobber
@@ -2983,7 +3008,7 @@ vn_reference_lookup_pieces (tree vuse, a
 {
   ao_ref r;
   unsigned limit = PARAM_VALUE (PARAM_SCCVN_MAX_ALIAS_QUERIES_PER_ACCESS);
-  vn_walk_cb_data data (, NULL, kind, true);
+  vn_walk_cb_data data (, NULL_TREE, NULL, kind, true);
   if (ao_ref_init_from_vn_reference (, set, type, vr1.operands))
*vnresult =
  (vn_reference_t)walk_non_aliased_vuses (, vr1.vuse, true,
@@ -3040,7 +3065,8 @@ vn_reference_lookup (tree op, tree vuse,
  || !ao_ref_init_from_vn_reference (, vr1.set, vr1.type,
 vr1.operands))
ao_ref_init (, op);
-  vn_walk_cb_data data (, last_vuse_ptr, kind, tbaa_p);
+  vn_walk_cb_data data (, r.ref ? NULL_TREE : op,
+   last_vuse_ptr, kind, tbaa_p);
   wvnresult =
(vn_reference_t)walk_non_aliased_vuses (, vr1.vuse, tbaa_p,
vn_reference_lookup_2,
Index: gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-1.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-1.c (revision 273232)
+++ gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-1.c (working copy)
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-fre3" } */
+/* { dg-options "-O2 -fdump-tree-fre1" } */
 struct foo
 {
   int val;
@@ -18,4 +18,4 @@ test ()
   return barptr->val2;
 }
 
-/* { dg-final { scan-tree-dump-times "return 123" 1 "fre3"} } */
+/* { dg-final { scan-tree-dump-times "return 123" 1 "fre1"} } */
Index: gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-2.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-2.c (revision 273232)
+++ gcc/testsuite/gcc.dg/tree-ssa/alias-access-path-2.c (working copy)
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-fre3" } */
+/* { dg-options "-O2 -fdump-tree-fre1" } */
 struct a {
   int val;
 };
@@ -19,4 +19,4 @@ test (int i, int j, int k, int l)
   dptr->c.b[k].a2[l].val=2;
   return cptr->b[i].a[j].val;
 }
-/* { dg-final { scan-tree-dump-times "return 123" 1 "fre3"} } */
+/* 

Re: [bikeshed] include path search for main file

2019-07-08 Thread Richard Biener
On Mon, Jul 8, 2019 at 1:46 PM Nathan Sidwell  wrote:
>
> One of the facilities C++ modules provides is turning header files into
> header units.  Thus you can say:
>import ;
> (there are some constraints on being able to do that)
>
> This involves compiling the header file, similar to PCH.  However,
> needing to provide the full path to the header file on the command line
> is awkward, and creates adoption friction, because it's one of the first
> thing someone might do.
>
> So, I've added a mechanism to search:
> g++ --lang c++ -std=c++17 -fmodule-header -search-system iostream
>
> I've decoupled these new options from what one does with the file (hence
> the -fmodule-header), or what language (hence --lang c++).
>
> Question: Are -search-system and -search-user sensible names? Should
> they be more clearly tied to the main file?

Hmm.  How would this integrate with makefiles and dependency generation?
IMHO that's something to try get right first before the above.

Richard.

> More weedful: I contemplated permitting '' and '"foo"' to
> indicate this, but (a) our specs pattern matcher doesn't really work
> that way, and <, >, " are all shell meta-characters.
>
> nathan
>
> --
> Nathan Sidwell


[bikeshed] include path search for main file

2019-07-08 Thread Nathan Sidwell
One of the facilities C++ modules provides is turning header files into 
header units.  Thus you can say:

  import ;
(there are some constraints on being able to do that)

This involves compiling the header file, similar to PCH.  However, 
needing to provide the full path to the header file on the command line 
is awkward, and creates adoption friction, because it's one of the first 
thing someone might do.


So, I've added a mechanism to search:
   g++ --lang c++ -std=c++17 -fmodule-header -search-system iostream

I've decoupled these new options from what one does with the file (hence 
the -fmodule-header), or what language (hence --lang c++).


Question: Are -search-system and -search-user sensible names? Should 
they be more clearly tied to the main file?


More weedful: I contemplated permitting '' and '"foo"' to 
indicate this, but (a) our specs pattern matcher doesn't really work 
that way, and <, >, " are all shell meta-characters.


nathan

--
Nathan Sidwell


[PATCH] Fix PR91108

2019-07-08 Thread Richard Biener


This fixes bogus non-overlap considerations based on TBAA.  Since
we also have to care for punning through unions the following
uses only align-based checks which are always valid.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

Variant for the branch below, also boostrapped/tested and applied to
GCC 9 sofar, backport to GCC 8 pending.

Richard.

2019-07-08  Richard Biener  

PR tree-optimization/91108
* tree-ssa-sccvn.c: Include builtins.h.
(vn_reference_lookup_3): Use only alignment constraints to
verify same-valued store disambiguation.

* gcc.dg/tree-ssa/ssa-fre-61.c: Adjust back.
* gcc.dg/tree-ssa/ssa-fre-78.c: New testcase.

Index: gcc/tree-ssa-sccvn.c
===
--- gcc/tree-ssa-sccvn.c(revision 273194)
+++ gcc/tree-ssa-sccvn.c(working copy)
@@ -70,6 +70,7 @@ along with GCC; see the file COPYING3.
 #include "tree-ssa-loop.h"
 #include "tree-scalar-evolution.h"
 #include "tree-ssa-loop-niter.h"
+#include "builtins.h"
 #include "tree-ssa-sccvn.h"
 
 /* This algorithm is based on the SCC algorithm presented by Keith
@@ -2248,24 +2249,10 @@ vn_reference_lookup_3 (ao_ref *ref, tree
   /* If we reach a clobbering statement try to skip it and see if
  we find a VN result with exactly the same value as the
 possible clobber.  In this case we can ignore the clobber
-and return the found value.
-Note that we don't need to worry about partial overlapping
-accesses as we then can use TBAA to disambiguate against the
-clobbering statement when looking up a load (thus the
-VN_WALKREWRITE guard).  */
-  if (data->vn_walk_kind == VN_WALKREWRITE
- && is_gimple_reg_type (TREE_TYPE (lhs))
+and return the found value.  */
+  if (is_gimple_reg_type (TREE_TYPE (lhs))
  && types_compatible_p (TREE_TYPE (lhs), vr->type)
- /* The overlap restriction breaks down when either access
-alias-set is zero.  Still for accesses of the size of
-an addressable unit there can be no overlaps.  Overlaps
-between different union members are not an issue since
-activation of a union member via a store makes the
-values of untouched bytes unspecified.  */
- && (known_eq (ref->size, BITS_PER_UNIT)
- || (flag_strict_aliasing
- && get_alias_set (lhs) != 0
- && ao_ref_alias_set (ref) != 0)))
+ && ref->ref)
{
  tree *saved_last_vuse_ptr = data->last_vuse_ptr;
  /* Do not update last_vuse_ptr in vn_reference_lookup_2.  */
@@ -2284,7 +2271,14 @@ vn_reference_lookup_3 (ao_ref *ref, tree
  if (TREE_CODE (rhs) == SSA_NAME)
rhs = SSA_VAL (rhs);
  if (vnresult->result
- && operand_equal_p (vnresult->result, rhs, 0))
+ && operand_equal_p (vnresult->result, rhs, 0)
+ /* We have to honor our promise about union type punning
+and also support arbitrary overlaps with
+-fno-strict-aliasing.  So simply resort to alignment to
+rule out overlaps.  Do this check last because it is
+quite expensive compared to the hash-lookup above.  */
+ && multiple_p (get_object_alignment (ref->ref), ref->size)
+ && multiple_p (get_object_alignment (lhs), ref->size))
return res;
}
}
Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-61.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-61.c  (revision 273194)
+++ gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-61.c  (working copy)
@@ -1,5 +1,5 @@
 /* { dg-do link } */
-/* { dg-options "-O -fstrict-aliasing -fdump-tree-fre1-details" } */
+/* { dg-options "-O -fdump-tree-fre1-details" } */
 
 void link_error (void);
 
Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-78.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-78.c  (nonexistent)
+++ gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-78.c  (working copy)
@@ -0,0 +1,27 @@
+/* { dg-do run } */
+/* { dg-options "-O3 -fstrict-aliasing" } */
+
+union U {
+  struct A { int : 2; int x : 8; } a;
+  struct B { int : 6; int x : 8; } b;
+};
+
+int __attribute__((noipa))
+foo (union U *p, union U *q)
+{
+  p->a.x = 1;
+  q->b.x = 1;
+  return p->a.x;
+}
+
+int
+main()
+{
+  union U x;
+  if (foo (, ) != x.a.x)
+__builtin_abort ();
+  return 0;
+}
+
+/* We support arbitrary punning through unions when it happens through
+   the union type and thus p == q is valid here.  */


2019-07-08  Richard Biener  

PR tree-optimization/91108
* tree-ssa-sccvn.c: Include builtins.h.
(vn_reference_lookup_3): Use only alignment constraints to
verify 

Re: Prevent tree-ssa-dce.c from deleting stores at -Og

2019-07-08 Thread Richard Biener
On Sun, Jul 7, 2019 at 9:07 PM Jeff Law  wrote:
>
> On 7/7/19 3:45 AM, Richard Sandiford wrote:
> > DCE tries to delete dead stores to local data and also tries to insert
> > debug binds for simple cases:
> >
> >   /* If this is a store into a variable that is being optimized away,
> >  add a debug bind stmt if possible.  */
> >   if (MAY_HAVE_DEBUG_BIND_STMTS
> >   && gimple_assign_single_p (stmt)
> >   && is_gimple_val (gimple_assign_rhs1 (stmt)))
> > {
> >   tree lhs = gimple_assign_lhs (stmt);
> >   if ((VAR_P (lhs) || TREE_CODE (lhs) == PARM_DECL)
> > && !DECL_IGNORED_P (lhs)
> > && is_gimple_reg_type (TREE_TYPE (lhs))
> > && !is_global_var (lhs)
> > && !DECL_HAS_VALUE_EXPR_P (lhs))
> >   {
> > tree rhs = gimple_assign_rhs1 (stmt);
> > gdebug *note
> >   = gimple_build_debug_bind (lhs, unshare_expr (rhs), stmt);
> > gsi_insert_after (i, note, GSI_SAME_STMT);
> >   }
> > }
> >
> > But this doesn't help for things like "print *ptr" when ptr points
> > to the local variable (tests Og-dce-1.c and Og-dce-2.c).  It also tends
> > to make the *live* -- and thus useful -- values optimised out, because
> > we can't yet switch back to tracking the memory location as it evolves
> > over time (test Og-dce-3.c).
> >
> > So for -Og I think it'd be better not to delete any stmts with
> > vdefs for now.  This also means that we can avoid the potentially
> > expensive vop walks (which already have a cut-off, but still).
> >
> > The patch also fixes the Og failures in gcc.dg/guality/pr54970.c
> > (PR 86638).
> >
> > Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
> >
> > Richard
> >
> >
> > 2019-07-07  Richard Sandiford  
> >
> > gcc/
> >   PR debug/86638
> >   * tree-ssa-dce.c (keep_all_vdefs_p): New function.
> >   (mark_stmt_if_obviously_necessary): Mark all stmts with vdefs as
> >   necessary if keep_all_vdefs_p is true.
> >   (mark_aliased_reaching_defs_necessary): Add a gcc_checking_assert
> >   that keep_all_vdefs_p is false.
> >   (mark_all_reaching_defs_necessary): Likewise.
> >   (propagate_necessity): Skip the vuse scan if keep_all_vdefs_p is true.
> >
> > gcc/testsuite/
> >   * c-c++-common/guality/Og-dce-1.c: New test.
> >   * c-c++-common/guality/Og-dce-2.c: Likewise.
> >   * c-c++-common/guality/Og-dce-3.c: Likewise.
> OK

I wonder how code size (and compile-time) is affected by the DSE/DCE patch?
Say just look at -Og built cc1?  Can you restrict the keep-all-vdefs to
user variables (and measure the difference this makes)?  Again I wonder if
this makes C++ with -Og impractical runtime-wise.

Richard.

> jeff


Re: Don't run DSE at -Og

2019-07-08 Thread Richard Biener
On Sun, Jul 7, 2019 at 9:04 PM Jeff Law  wrote:
>
> On 7/7/19 3:43 AM, Richard Sandiford wrote:
> > This patch stops gimple and rtl DSE from running by default at -Og.
> > The idea is both to improve compile time and to stop us from deleting
> > stores that we can't track in debug info.
> >
> > We could rein this back in future for stores to local variables
> > with is_gimple_reg_type, but at the moment we don't have any
> > infrastructure for switching between binds to specific values
> > and binds to evolving memory locations.  Even then, location
> > tracking only works for direct references to the variables, and doesn't
> > for example help with printing dereferenced pointers (see the next patch
> > in the series for an example).
> >
> > I'm also not sure that DSE is important enough for -Og to justify the
> > compile time cost -- especially in the case of RTL DSE, which is pretty
> > expensive.
> >
> > Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
> >
> > Richard
> >
> >
> > 2019-07-07  Richard Sandiford  
> >
> > gcc/
> >   * common.opt (Og): Change the initial value of flag_dse to 0.
> >   * opts.c (default_options_table): Move OPT_ftree_dse from
> >   OPT_LEVELS_1_PLUS to OPT_LEVELS_1_PLUS_NOT_DEBUG.  Also add
> >   OPT_fdse to OPT_LEVELS_1_PLUS_NOT_DEBUG.  Put the OPT_ftree_pta
> >   entry before the OPT_ftree_sra entry.
> >   * doc/invoke.texi (Og): Add -fdse and -ftree-dse to the list
> >   of flags disabled by Og.
> >
> > gcc/testsuite/
> >   * c-c++-common/guality/Og-global-dse-1.c: New test.
> OK.  I doubt DSE is at all important if someone has asked for Og.

Hmm.  -Og also asks for runtime performance.  Can we instead restrict
DSE to artificial decls?  I also fear that this leaves a _lot_ of abstraction
for C++ code given we also do not run SRA with -Og even though that
has means to preserve debug info.

Richard.

> BTW, if you're going to be poking at improving Og or debuginfo in
> general, you might want to loop Alex into your plans.  He did a detailed
> evaluation of how the various options impact debugging, -Og issues, etc.
>  He's no longer with Red Hat, but can be reached at Adacore.
>
> jeff


Commited: remove uint32_t declaration from testsuite/gcc.dg/vect/slp-reduc-sad.c

2019-07-08 Thread Joern Wolfgang Rennecke
The declaration was not used, and it clashed with a system declaration 
from newlib.
On a 32 bit typical system, uint32_t can be defined using unsigned, 
unsigned int or unsigned long.


gcc.dg/vect/slp-reduc-sad.c includes gcc.dg/vect/tree-vect.h,
which includes newlib/libc/include/signal.h,
which includes newlib/libc/include/sys/signal.h,
which includes newlib/libc/include/sys/types.h
which includes newlib/libc/include/sys/_stdint.h, which defines:
typedef __uint32_t uint32_t ;

Committed as obvious.
2019-07-08  Joern Rennecke  

Avoid clash with system header declaration.
* testsuite/gcc.dg/vect/slp-reduc-sad.c (uint32_t):
Remove unused declaration.

Index: testsuite/gcc.dg/vect/slp-reduc-sad.c
===
--- testsuite/gcc.dg/vect/slp-reduc-sad.c   (revision 272931)
+++ testsuite/gcc.dg/vect/slp-reduc-sad.c   (working copy)
@@ -2,7 +2,6 @@
 
 #include "tree-vect.h"
 
-typedef unsigned int uint32_t;
 typedef unsigned short uint16_t;
 typedef unsigned char uint8_t;
 


Re: [PATCH] subreg: Add -fsplit-wide-types-early (PR88233)

2019-07-08 Thread Richard Biener
On Sun, Jul 7, 2019 at 7:12 PM Segher Boessenkool
 wrote:
>
> Currently the second lower-subreg pass is run right before RA.  This
> is much too late to be very useful.  At least for targets that do not
> have RTL patterns for operations on multi-register modes it is a lot
> better to split patterns earlier, before combine and all related
> passes.
>
> This adds an option -fsplit-wide-types-early that does that, and
> enables it by default for rs6000.

Do you by chance have a (target specific) testcase you can add?

The docs should mention that the new option doesn't have any effect
unless -fsplit-wide-types is enabled.

Otherwise OK.
Richard.

>
> 2019-07-07  Segher Boessenkool  
>
> PR rtl-optimization/88233
> * common.opt (fsplit-wide-types-early): New option.
> * common/config/rs6000/rs6000-common.c
> (rs6000_option_optimization_table): Add OPT_fsplit_wide_types_early 
> for
> OPT_LEVELS_ALL.
> * doc/invoke.texi (Optimization Options): Add 
> -fsplit-wide-types-early.
> * lower-subreg.c (pass_lower_subreg2::gate): Add test for
> flag_split_wide_types_early.
> (pass_data_lower_subreg3): New.
> (pass_lower_subreg3): New.
> (make_pass_lower_subreg3): New.
> * passes.def (pass_lower_subreg2): Move after the loop passes.
> (pass_lower_subreg3): New, inserted where pass_lower_subreg2 was.
> * tree-pass.h (make_pass_lower_subreg2): Move up, to its new place in
> the pass pipeline; its previous place is taken by ...
> (make_pass_lower_subreg3): ... this.
>
> ---
>  gcc/common.opt   |  4 +++
>  gcc/common/config/rs6000/rs6000-common.c |  2 ++
>  gcc/doc/invoke.texi  |  8 +-
>  gcc/lower-subreg.c   | 46 
> +++-
>  gcc/passes.def   |  3 ++-
>  gcc/tree-pass.h  |  3 ++-
>  6 files changed, 62 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 41514df..b998b25 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -2430,6 +2430,10 @@ fsplit-wide-types
>  Common Report Var(flag_split_wide_types) Optimization
>  Split wide types into independent registers.
>
> +fsplit-wide-types-early
> +Common Report Var(flag_split_wide_types_early) Optimization
> +Split wide types into independent registers earlier.
> +
>  fssa-backprop
>  Common Report Var(flag_ssa_backprop) Init(1) Optimization
>  Enable backward propagation of use properties at the SSA level.
> diff --git a/gcc/common/config/rs6000/rs6000-common.c 
> b/gcc/common/config/rs6000/rs6000-common.c
> index 9857b54..4b0c205 100644
> --- a/gcc/common/config/rs6000/rs6000-common.c
> +++ b/gcc/common/config/rs6000/rs6000-common.c
> @@ -31,6 +31,8 @@
>  /* Implement TARGET_OPTION_OPTIMIZATION_TABLE.  */
>  static const struct default_options rs6000_option_optimization_table[] =
>{
> +/* Split multi-word types early.  */
> +{ OPT_LEVELS_ALL, OPT_fsplit_wide_types_early, NULL, 1 },
>  /* Enable -fsched-pressure for first pass instruction scheduling.  */
>  { OPT_LEVELS_1_PLUS, OPT_fsched_pressure, NULL, 1 },
>  { OPT_LEVELS_NONE, 0, NULL, 0 }
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 73d16b5..6349d4c 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -467,7 +467,7 @@ Objective-C and Objective-C++ Dialects}.
>  -fsignaling-nans @gol
>  -fsingle-precision-constant  -fsplit-ivs-in-unroller  -fsplit-loops@gol
>  -fsplit-paths @gol
> --fsplit-wide-types  -fssa-backprop  -fssa-phiopt @gol
> +-fsplit-wide-types  -fsplit-wide-types-early  -fssa-backprop  -fssa-phiopt 
> @gol
>  -fstdarg-opt  -fstore-merging  -fstrict-aliasing @gol
>  -fthread-jumps  -ftracer  -ftree-bit-ccp @gol
>  -ftree-builtin-call-dce  -ftree-ccp  -ftree-ch @gol
> @@ -8731,6 +8731,12 @@ but may make debugging more difficult.
>  Enabled at levels @option{-O}, @option{-O2}, @option{-O3},
>  @option{-Os}.
>
> +@item -fsplit-wide-types-early
> +@opindex fsplit-wide-types-early
> +Fully split wide types early, instead of very late.
> +
> +This is the default on some targets.
> +
>  @item -fcse-follow-jumps
>  @opindex fcse-follow-jumps
>  In common subexpression elimination (CSE), scan through jump instructions
> diff --git a/gcc/lower-subreg.c b/gcc/lower-subreg.c
> index 4f68a73..e1418e5 100644
> --- a/gcc/lower-subreg.c
> +++ b/gcc/lower-subreg.c
> @@ -1801,7 +1801,8 @@ public:
>{}
>
>/* opt_pass methods: */
> -  virtual bool gate (function *) { return flag_split_wide_types != 0; }
> +  virtual bool gate (function *) { return flag_split_wide_types
> + && flag_split_wide_types_early; }
>virtual unsigned int execute (function *)
>  {
>decompose_multiword_subregs (true);
> @@ -1817,3 +1818,46 @@ make_pass_lower_subreg2 (gcc::context *ctxt)
>  {
>return new pass_lower_subreg2 

Re: [PATCH] Add generic support for "noinit" attribute

2019-07-08 Thread Christophe Lyon
On Sat, 6 Jul 2019 at 19:57, Martin Sebor  wrote:
>
> On 7/4/19 9:27 AM, Christophe Lyon wrote:
> > Hi,
> >
> > Similar to what already exists for TI msp430 or in TI compilers for
> > arm, this patch adds support for the "noinit" attribute.
> >
> > It is convenient for embedded targets where the user wants to keep the
> > value of some data when the program is restarted: such variables are
> > not zero-initialized. It is mostly a helper/shortcut to placing
> > variables in a dedicated section.
> >
> > It's probably desirable to add the following chunk to the GNU linker:
> > diff --git a/ld/emulparams/armelf.sh b/ld/emulparams/armelf.sh
> > index 272a8bc..9555cec 100644
> > --- a/ld/emulparams/armelf.sh
> > +++ b/ld/emulparams/armelf.sh
> > @@ -10,7 +10,19 @@ OTHER_TEXT_SECTIONS='*(.glue_7t) *(.glue_7)
> > *(.vfp11_veneer) *(.v4_bx)'
> >   OTHER_BSS_SYMBOLS="${CREATE_SHLIB+PROVIDE (}__bss_start__ =
> > .${CREATE_SHLIB+)};"
> >   OTHER_BSS_END_SYMBOLS="${CREATE_SHLIB+PROVIDE (}_bss_end__ =
> > .${CREATE_SHLIB+)}; ${CREATE_SHLIB+PROVIDE (}__bss_end__ =
> > .${CREATE_SHLIB+)};"
> >   OTHER_END_SYMBOLS="${CREATE_SHLIB+PROVIDE (}__end__ = .${CREATE_SHLIB+)};"
> >   -OTHER_SECTIONS='.note.gnu.arm.ident 0 : { KEEP (*(.note.gnu.arm.ident)) 
> > }'
> >   +OTHER_SECTIONS='
> >   +.note.gnu.arm.ident 0 : { KEEP (*(.note.gnu.arm.ident)) }
> >   +  /* This section contains data that is not initialised during load
> >   + *or* application reset.  */
> >   +   .noinit (NOLOAD) :
> >   +   {
> >   + . = ALIGN(2);
> >   + PROVIDE (__noinit_start = .);
> >   + *(.noinit)
> >   + . = ALIGN(2);
> >   + PROVIDE (__noinit_end = .);
> >   +   }
> >   +'
> >
> > so that the noinit section has the "NOLOAD" flag.
> >
> > I'll submit that part separately to the binutils project if OK.
> >
> > However, I'm not sure for which other targets (beyond arm), I should
> > update the linker scripts.
> >
> > I left the new testcase in gcc.target/arm, guarded by
> > dg-do run { target { *-*-eabi } }
> > but since this attribute is now generic, I suspect I should move the
> > test to some other place. But then, which target selector should I
> > use?
> >
> > Finally, I tested on arm-eabi, but not on msp430 for which I do not
> > have the environment, so advice from msp430 maintainers is
> > appreciated. Since msp430 does not use the same default helpers as
> > arm, I left the "noinit" handling code in place, to avoid changing
> > other generic functions which I don't know how to test
> > (default_select_section, default_section_type_flags).
> >
>
> Since the section attribute is mutually exclusive with noinit,
> defining an attribute_exclusions array with the mutually exclusive
> attributes and setting the last member of the corresponding
> c_common_attribute_table element to that array (and updating
> the arrays for the other mutually exclusive attributes) would
> be the expected way to express that constraint:
>
> +  { "noinit", 0, 0, true,  false, false, false,
> + handle_noinit_attribute, NULL },
> 
> This should also make it possible to remove the explicit handling
> from handle_section_attribute and handle_noinit_attribute.  If
> that's not completely possible then in the following please be
> sure to quote the names of the attributes:
>
> @@ -1912,6 +1915,13 @@ handle_section_attribute (tree *node, tree
> ARG_UNUSED (name), tree args,
> goto fail;
>   }
>
> +  if (DECL_P (decl) && lookup_attribute ("noinit", DECL_ATTRIBUTES
> (decl)) != NULL_TREE)
> +{
> +  warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
> +   "section attribute cannot be applied to variables with noinit
> attribute");
>
> Martin
>

Thanks, here is an updated version:
- adds exclusion array
- updates testcase with new error message (because of exclusion)
- moves testcase to gcc.c-torture/execute
- adds "noinit" effective-target

Christophe

> > Thanks,
> >
> > Christophe
> >
> > gcc/ChangeLog:
> >
> > 2019-07-04  Christophe Lyon  
> >
> > * doc/extend.texi: Add "noinit" attribute documentation.
> > * varasm.c
> > (default_section_type_flags): Add support for "noinit" section.
> > (default_elf_select_section): Add support for "noinit" attribute.
> >
> > gcc/c-family/ChangeLog:
> >
> > 2019-07-04  Christophe Lyon  
> >
> > * c-attribs.c (c_common_attribute_table): Add "noinit" entry.
> > (handle_section_attribute): Add support for "noinit" attribute.
> > (handle_noinit_attribute): New function.
> >
> > gcc/config/ChangeLog:
> >
> > 2019-07-04  Christophe Lyon  
> >
> > * msp430/msp430.c (msp430_attribute_table): Remove "noinit" entry.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 2019-07-04  Christophe Lyon  
> >
> > * gcc.target/arm/noinit-attribute.c: New test.
> >
>
commit 3a9dd81162eae36a87a067018a5e5bf1e7b978df
Author: Christophe Lyon 
Date:   Thu Jul 4 14:46:00 2019 +

Add generic support for 

[PATCH] RISC-V: Fix splitter for 32-bit AND on 64-bit target.

2019-07-08 Thread Jim Wilson
Fixes github.com/riscv/riscv-gcc issue #161.  We were accidentally using
BITS_PER_WORD to compute shift counts when we should have been using the
bitsize of the operand modes.  This was wrong when we had an SImode shift
and a 64-bit target.

Tested with 32-bit elf and 64-bit linux cross compiler builds and checks.
There were no regressions.  The new test fails without the patch and works
with the patch.

Committed.

Jim

Andrew Waterman  
gcc/
* config/riscv/riscv.md (lshrsi3_zero_extend_3+1): Use operands[1]
bitsize instead of BITS_PER_WORD.
gcc/testsuite/
* gcc.target/riscv/shift-shift-2.c: Add one more test.
---
 gcc/config/riscv/riscv.md  |  5 +++--
 gcc/testsuite/gcc.target/riscv/shift-shift-2.c | 16 ++--
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 0f4626656d6..78260fcf6fd 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -1776,10 +1776,11 @@
   (set (match_dup 0)
(lshiftrt:GPR (match_dup 0) (match_dup 2)))]
 {
-  operands[2] = GEN_INT (BITS_PER_WORD
+  /* Op2 is a VOIDmode constant, so get the mode size from op1.  */
+  operands[2] = GEN_INT (GET_MODE_BITSIZE (GET_MODE (operands[1]))
 - exact_log2 (INTVAL (operands[2]) + 1));
 })
-  
+
 ;; Handle AND with 0xF...F0...0 where there are 32 to 63 zeros.  This can be
 ;; split into two shifts.  Otherwise it requires 3 instructions: li, sll, and.
 (define_split
diff --git a/gcc/testsuite/gcc.target/riscv/shift-shift-2.c 
b/gcc/testsuite/gcc.target/riscv/shift-shift-2.c
index 3f07e7776e7..10a5bb728be 100644
--- a/gcc/testsuite/gcc.target/riscv/shift-shift-2.c
+++ b/gcc/testsuite/gcc.target/riscv/shift-shift-2.c
@@ -25,5 +25,17 @@ sub4 (unsigned long i)
 {
   return (i << 52) >> 52;
 }
-/* { dg-final { scan-assembler-times "slli" 4 } } */
-/* { dg-final { scan-assembler-times "srli" 4 } } */
+
+unsigned int
+sub5 (unsigned int i)
+{
+  unsigned int j;
+  j = i >> 24;
+  j = j * (1 << 24);
+  j = i - j;
+  return j;
+}
+/* { dg-final { scan-assembler-times "slli" 5 } } */
+/* { dg-final { scan-assembler-times "srli" 5 } } */
+/* { dg-final { scan-assembler-times "slliw" 1 } } */
+/* { dg-final { scan-assembler-times "srliw" 1 } } */
-- 
2.17.1



Re: Make nonoverlapping_component_refs work with duplicated main variants

2019-07-08 Thread Jan Hubicka
> > +/* FIELD1 and FIELD2 are two component refs whose bases are either
> > +   both at the same address or completely disjoint.
> > +   Return 1 if FIELD1 and FIELD2 are non-overlapping
> > +   Return 0 if FIELD1 and FIELD2 are having same addresses or are
> > + completely disjoint.
> 
> completely disjoint?  I guess
> 
>   Return 0 if accesses to FIELD1 and FIELD2 are possibly overlapping.
> 
> is better matching actual behavior.  Likewise mentioning 'accesses'
> in the first because of the bitfield treatment (the fields may
> be non-overlapping but actual accesses might be).

I was trying to describe difference between 0 and -1.
We return 0 when we fully structurally matched the path and we know it
is same. -1 means that we arrived to somehting we can not handle (union,
mismatched offsets) and it would make sense to try disambiguating
further.

Currently it means that in addition to
nonoverlapping_component_refs_since_match_p we also do
nonoverlapping_component_refs_p which has some chance to recover from
the mismatched REF pair, match the types later on path and still
disambiguate.  It seem to happen very rarely though.
> 
> > +  /* Different fields of the same record type cannot overlap.
> > +??? Bitfields can overlap at RTL level so punt on them.  */
> > +  if (DECL_BIT_FIELD (field1) && DECL_BIT_FIELD (field2))
> > +   return -1;
> 
> This is similar as the DECL_BIT_FIELD_REPRESENTATIVE check so why
> return -1 instead of 0?

Well, my plan is to put this test before ref_and_offset which still have
chace to suceed if fields are far away. But i am happy to return 0 here
and mess with that later.
> > +  else 
> > +{
> > +  if (operand_equal_p (DECL_FIELD_BIT_OFFSET (field1),
> > +  DECL_FIELD_BIT_OFFSET (field2), 0))
> > +   return 0;
> 
> I think this is overly pessimistic - the offset of a field
> is DECL_FIELD_OFFSET + DECL_FIELD_BIT_OFFSET (the latter is
> only up to DECL_OFFSET_ALIGN, the rest of the constant
> offset spills into DECL_FIELD_OFFSET).  Which also means ...
> 
> > +
> > +  /* Different fields of the same record type cannot overlap.
> > +??? Bitfields can overlap at RTL level so punt on them.  */
> > +  if (DECL_BIT_FIELD (field1) && DECL_BIT_FIELD (field2))
> > +   return -1;
> > +
> > +  poly_uint64 offset1;
> > +  poly_uint64 offset2;
> > +  poly_uint64 size1;
> > +  poly_uint64 size2;
> > +  if (!poly_int_tree_p (DECL_FIELD_BIT_OFFSET (field1), )
> > + || !poly_int_tree_p (DECL_FIELD_BIT_OFFSET (field2), )
> > + || !poly_int_tree_p (DECL_SIZE (field1), )
> > + || !poly_int_tree_p (DECL_SIZE (field2), )
> > + || ranges_maybe_overlap_p (offset1, size1, offset2, size2))
> 
> this is technically wrong in case we had DECL_FIELD_OFFSETs 4 and 8
> and DECL_FIELD_BIT_OFFSETs 32 and 0.
> 
> So you have to compute the combined offsets first.

OK, I guess I can take look at the get_base_ref_and_offset there. Thanks
for pointing this out.
> 
> > +   return -1;
> 
> I think it may make sense to return -1 if any of the !poly_int_tree_p
> tests fire, but otherwise?  I'm not actually sure what -1 vs. 0
> means here - is 0 a must exactly overlap and -1 is a may overlap
> somehow?

Well, we have two fields that overlap partly from two different types
in >nonoverlapping_component_refs_since_match_p  so it can not
continue walking (since the main invariant is broken)
we may still suceed with the nonoverlaping_component_refs 

Thanks, I will update the patch.
Honza


[patch] Small improvements to coverage info (3/n)

2019-07-08 Thread Eric Botcazou
Hi,

a couple of fixes for the RTL middle-end this time, with the same goal of 
preventing instructions from inheriting random source location information 
in the debug info generated by the compiler.

Tested on x86_64-suse-linux, both GCC and GDB, OK for mainline?


2019-07-08  Eric Botcazou  

* emit-rtl.c (set_insn_locations): New function moved from...
* function.c (set_insn_locations): ...here.
* ira-emit.c (emit_moves): Propagate location of the first instruction
to the inserted move instructions.
* reg-stack.c (compensate_edge): Set the location if the sequence is
inserted on the edge.
* rtl.h (set_insn_locations): Declare.

-- 
Eric BotcazouIndex: emit-rtl.c
===
--- emit-rtl.c	(revision 273133)
+++ emit-rtl.c	(working copy)
@@ -6582,6 +6582,18 @@ curr_insn_location (void)
   return curr_location;
 }
 
+/* Set the location of the insn chain starting at INSN to LOC.  */
+void
+set_insn_locations (rtx_insn *insn, location_t loc)
+{
+  while (insn)
+{
+  if (INSN_P (insn))
+	INSN_LOCATION (insn) = loc;
+  insn = NEXT_INSN (insn);
+}
+}
+
 /* Return lexical scope block insn belongs to.  */
 tree
 insn_scope (const rtx_insn *insn)
Index: function.c
===
--- function.c	(revision 273133)
+++ function.c	(working copy)
@@ -5244,19 +5244,6 @@ use_return_register (void)
   diddle_return_value (do_use_return_reg, NULL);
 }
 
-/* Set the location of the insn chain starting at INSN to LOC.  */
-
-static void
-set_insn_locations (rtx_insn *insn, int loc)
-{
-  while (insn != NULL)
-{
-  if (INSN_P (insn))
-	INSN_LOCATION (insn) = loc;
-  insn = NEXT_INSN (insn);
-}
-}
-
 /* Generate RTL for the end of the current function.  */
 
 void
Index: ira-emit.c
===
--- ira-emit.c	(revision 273133)
+++ ira-emit.c	(working copy)
@@ -1011,6 +1011,10 @@ emit_moves (void)
 	tmp = NEXT_INSN (tmp);
 	  if (NOTE_INSN_BASIC_BLOCK_P (tmp))
 	tmp = NEXT_INSN (tmp);
+	  /* Propagate the location of the current first instruction to the
+	 moves so that they don't inherit a random location.  */
+	  if (tmp != NULL_RTX && INSN_P (tmp))
+	set_insn_locations (insns, INSN_LOCATION (tmp));
 	  if (tmp == BB_HEAD (bb))
 	emit_insn_before (insns, tmp);
 	  else if (tmp != NULL_RTX)
Index: reg-stack.c
===
--- reg-stack.c	(revision 273133)
+++ reg-stack.c	(working copy)
@@ -2929,6 +2929,7 @@ compensate_edge (edge e)
   seq = get_insns ();
   end_sequence ();
 
+  set_insn_locations (seq, e->goto_locus);
   insert_insn_on_edge (seq, e);
   return true;
 }
Index: rtl.h
===
--- rtl.h	(revision 273133)
+++ rtl.h	(working copy)
@@ -4338,6 +4338,7 @@ extern void insn_locations_init (void);
 extern void insn_locations_finalize (void);
 extern void set_curr_insn_location (location_t);
 extern location_t curr_insn_location (void);
+extern void set_insn_locations (rtx_insn *, location_t);
 
 /* rtl-error.c */
 extern void _fatal_insn_not_found (const_rtx, const char *, int, const char *)


Re: [PATCH, committed] Add myself to MAINTAINERS

2019-07-08 Thread Kito Cheng
Hi Rainer:

Thanks your reminder, I've fix it now.

On Mon, Jul 8, 2019 at 4:14 PM Rainer Orth  
wrote:
>
> Kito Cheng  writes:
>
> > ChangeLog:
> >
> > 2019-07-08  Kito Cheng  
> >
> > * MAINTAINERS (Write After Approval): Add myself.
> >
> > --- MAINTAINERS (revision 273194)
> > +++ MAINTAINERS (revision 273195)
> > @@ -651,6 +651,7 @@
> > Jon Ziegler
> > Roman Zippel   
> > Josef Zlomek   
> > +Kito Cheng 
>
> Please keep the list sorted by surname.
>
> Thanks.
> Rainer
>
> --
> -
> Rainer Orth, Center for Biotechnology, Bielefeld University


Re: [PATCH][armeb] PR 91060 gcc.c-torture/execute/scal-to-vec1.c fails since r272843

2019-07-08 Thread Christophe Lyon
On Mon, 8 Jul 2019 at 11:06, Kyrill Tkachov  wrote:
>
> Hi Christophe
>
> On 7/8/19 10:01 AM, Christophe Lyon wrote:
> > Hi,
> >
> > This patch fixes PR 91060 where the lane ordering was no longer the
> > right one (GCC's vs architecture's).
> >
> > OK?
> >
> > Thanks to both Richards for most of the debugging!
>
> Thank you to all for tracking this down.
>
> >
> > Christophe
>
>
> pr91060.patch.txt
>
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index 820502a..4c7b5a8 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -12471,7 +12471,7 @@ neon_expand_vector_init (rtx target, rtx vals)
> if (n_var == 1)
>   {
> rtx copy = copy_rtx (vals);
> -  rtx index = GEN_INT (one_var);
> +  rtx index = GEN_INT (1 << one_var);
>
> /* Load constant part of vector, substitute neighboring value for
>  varying element.  */
> @@ -12483,31 +12483,31 @@ neon_expand_vector_init (rtx target, rtx vals)
> switch (mode)
> {
> case E_V8QImode:
> - emit_insn (gen_neon_vset_lanev8qi (target, x, target, index));
> + emit_insn (gen_vec_setv8qi_internal (target, x, index, target));
>   break;
> case E_V16QImode:
> - emit_insn (gen_neon_vset_lanev16qi (target, x, target, index));
> + emit_insn (gen_vec_setv16qi_internal (target, x, index, target));
>   break;
> case E_V4HImode:
> - emit_insn (gen_neon_vset_lanev4hi (target, x, target, index));
> + emit_insn (gen_vec_setv4hi_internal (target, x, index, target));
>   break;
> case E_V8HImode:
> - emit_insn (gen_neon_vset_lanev8hi (target, x, target, index));
> + emit_insn (gen_vec_setv8hi_internal (target, x, index, target));
>   break;
> case E_V2SImode:
> - emit_insn (gen_neon_vset_lanev2si (target, x, target, index));
> + emit_insn (gen_vec_setv2si_internal (target, x, index, target));
>   break;
> case E_V4SImode:
> - emit_insn (gen_neon_vset_lanev4si (target, x, target, index));
> + emit_insn (gen_vec_setv4si_internal (target, x, index, target));
>   break;
> case E_V2SFmode:
> - emit_insn (gen_neon_vset_lanev2sf (target, x, target, index));
> + emit_insn (gen_vec_setv2sf_internal (target, x, index, target));
>   break;
> case E_V4SFmode:
> - emit_insn (gen_neon_vset_lanev4sf (target, x, target, index));
> + emit_insn (gen_vec_setv4sf_internal (target, x, index, target));
>   break;
> case E_V2DImode:
> - emit_insn (gen_neon_vset_lanev2di (target, x, target, index));
> + emit_insn (gen_vec_setv2di_internal (target, x, index, target));
>   break;
> default:
>   gcc_unreachable ();
>
>
> Can we take the opportunity here to remove that switch statement and use the 
> parametrised names machinery:
> https://gcc.gnu.org/onlinedocs/gccint/Parameterized-Names.html#Parameterized-Names
>
> so that we can instead have one call to gen_vec_setv8hi_internal (mode, 
> target, x, merge_mask, target) or something.

Yes, that's what Richard posted in bugzilla, it's much nicer indeed.

> Thanks,
> Kyrill
>


Re: Make nonoverlapping_component_refs work with duplicated main variants

2019-07-08 Thread Richard Biener
On Mon, 8 Jul 2019, Jan Hubicka wrote:

> Hi,
> this patch avoids == compare of main varinats in nonoverlapping_component_refs
> making them work on unmerged type (such as when one is C++ ODR and other is 
> C).
> This is not hard to do
>- nonoverlapping_component_refs_since_match is
>  -fno-strict-aliasing safe and only cares about type sizes/field offsets.
>- nonoverlapping_component_refs_p does same test as aliasing_component_refs
>  (use TBAA to derive the fact that types either fully overlap or not at
>   all) and thus can use types_same_for_tbaa_p.
>  For structures this leads to TYPE_CANONICAL compare so I now use decl
>  uids of canonical types in the loop.
> I have also refactored the code to share the logic about bitfields and uids
> which was copied to multple places.
> 
> Bootstrapped/regtested x86_64-linux, OK?
> 
>   * tree-ssa-alias.c (nonoverlapping_component_refs_p_1): Break out
>   from ...; work also on duplicated types.
>   (nonoverlapping_component_refs_since_match): ... here
>   (ncr_type_uid): Break out from ...
>   (ncr_compar): ... here; look for TYPE_UID of canonical type if
>   available.
>   (nonoverlapping_component_refs_p): Use same_type_for_tbaa to match
>   the types and nonoverlapping_component_refs_p_1 to disambiguate.
>   * g++.dg/lto/alias-3_0.C: New file.
>   * g++.dg/lto/alias-3_1.c: New file.
> 
> Index: tree-ssa-alias.c
> ===
> --- tree-ssa-alias.c  (revision 273193)
> +++ tree-ssa-alias.c  (working copy)
> @@ -1128,6 +1128,63 @@ aliasing_component_refs_p (tree ref1,
>return false;
>  }
>  
> +/* FIELD1 and FIELD2 are two component refs whose bases are either
> +   both at the same address or completely disjoint.
> +   Return 1 if FIELD1 and FIELD2 are non-overlapping
> +   Return 0 if FIELD1 and FIELD2 are having same addresses or are
> + completely disjoint.

completely disjoint?  I guess

  Return 0 if accesses to FIELD1 and FIELD2 are possibly overlapping.

is better matching actual behavior.  Likewise mentioning 'accesses'
in the first because of the bitfield treatment (the fields may
be non-overlapping but actual accesses might be).

> +   Return -1 if we can not decide.  */
> +
> +static int
> +nonoverlapping_component_refs_p_1 (const_tree field1, const_tree field2)
> +{
> +  /* ??? We cannot simply use the type of operand #0 of the refs here
> + as the Fortran compiler smuggles type punning into COMPONENT_REFs
> + for common blocks instead of using unions like everyone else.  */
> +  tree type1 = DECL_CONTEXT (field1);
> +  tree type2 = DECL_CONTEXT (field2);
> +
> +  /* A simple fast path.  */

Merge this and the previous comment please, it's all for the fast path.

> +  if (type1 == type2 && TREE_CODE (type1) == RECORD_TYPE)
> +{
> +  if (field1 == field2)
> + return 0;
> +  /* A field and its representative need to be considered the
> +  same.  */
> +  if (DECL_BIT_FIELD_REPRESENTATIVE (field1) == field2
> +   || DECL_BIT_FIELD_REPRESENTATIVE (field2) == field1)
> + return 0;
> +  /* Different fields of the same record type cannot overlap.
> +  ??? Bitfields can overlap at RTL level so punt on them.  */
> +  if (DECL_BIT_FIELD (field1) && DECL_BIT_FIELD (field2))
> + return -1;

This is similar as the DECL_BIT_FIELD_REPRESENTATIVE check so why
return -1 instead of 0?

> +  return 1;
> +}

Unconditional return above so avoid the else {} below but put
a comment here like

  /* Resort to slower overlap checking by looking at the actual
 (possibly non-constant) offsets and sizes.  */

> +  else 
> +{
> +  if (operand_equal_p (DECL_FIELD_BIT_OFFSET (field1),
> +DECL_FIELD_BIT_OFFSET (field2), 0))
> + return 0;

I think this is overly pessimistic - the offset of a field
is DECL_FIELD_OFFSET + DECL_FIELD_BIT_OFFSET (the latter is
only up to DECL_OFFSET_ALIGN, the rest of the constant
offset spills into DECL_FIELD_OFFSET).  Which also means ...

> +
> +  /* Different fields of the same record type cannot overlap.
> +  ??? Bitfields can overlap at RTL level so punt on them.  */
> +  if (DECL_BIT_FIELD (field1) && DECL_BIT_FIELD (field2))
> + return -1;
> +
> +  poly_uint64 offset1;
> +  poly_uint64 offset2;
> +  poly_uint64 size1;
> +  poly_uint64 size2;
> +  if (!poly_int_tree_p (DECL_FIELD_BIT_OFFSET (field1), )
> +   || !poly_int_tree_p (DECL_FIELD_BIT_OFFSET (field2), )
> +   || !poly_int_tree_p (DECL_SIZE (field1), )
> +   || !poly_int_tree_p (DECL_SIZE (field2), )
> +   || ranges_maybe_overlap_p (offset1, size1, offset2, size2))

this is technically wrong in case we had DECL_FIELD_OFFSETs 4 and 8
and DECL_FIELD_BIT_OFFSETs 32 and 0.

So you have to compute the combined offsets first.

> + return -1;

I think it may make sense to return -1 if any of the 

Re: [PATCH][armeb] PR 91060 gcc.c-torture/execute/scal-to-vec1.c fails since r272843

2019-07-08 Thread Christophe Lyon
On Mon, 8 Jul 2019 at 11:04, Richard Sandiford
 wrote:
>
> Christophe Lyon  writes:
> > Hi,
> >
> > This patch fixes PR 91060 where the lane ordering was no longer the
> > right one (GCC's vs architecture's).
>
> Sorry, we clashed :-)
>
> I'd prefer to go with the version I attached to bugzilla just now.

Yes just saw that, thanks!

>
> Thanks,
> Richard


Re: [PATCH][armeb] PR 91060 gcc.c-torture/execute/scal-to-vec1.c fails since r272843

2019-07-08 Thread Kyrill Tkachov

Hi Christophe

On 7/8/19 10:01 AM, Christophe Lyon wrote:

Hi,

This patch fixes PR 91060 where the lane ordering was no longer the
right one (GCC's vs architecture's).

OK?

Thanks to both Richards for most of the debugging!


Thank you to all for tracking this down.



Christophe



pr91060.patch.txt

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 820502a..4c7b5a8 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -12471,7 +12471,7 @@ neon_expand_vector_init (rtx target, rtx vals)
   if (n_var == 1)
 {
   rtx copy = copy_rtx (vals);
-  rtx index = GEN_INT (one_var);
+  rtx index = GEN_INT (1 << one_var);
 
   /* Load constant part of vector, substitute neighboring value for

 varying element.  */
@@ -12483,31 +12483,31 @@ neon_expand_vector_init (rtx target, rtx vals)
   switch (mode)
{
case E_V8QImode:
- emit_insn (gen_neon_vset_lanev8qi (target, x, target, index));
+ emit_insn (gen_vec_setv8qi_internal (target, x, index, target));
  break;
case E_V16QImode:
- emit_insn (gen_neon_vset_lanev16qi (target, x, target, index));
+ emit_insn (gen_vec_setv16qi_internal (target, x, index, target));
  break;
case E_V4HImode:
- emit_insn (gen_neon_vset_lanev4hi (target, x, target, index));
+ emit_insn (gen_vec_setv4hi_internal (target, x, index, target));
  break;
case E_V8HImode:
- emit_insn (gen_neon_vset_lanev8hi (target, x, target, index));
+ emit_insn (gen_vec_setv8hi_internal (target, x, index, target));
  break;
case E_V2SImode:
- emit_insn (gen_neon_vset_lanev2si (target, x, target, index));
+ emit_insn (gen_vec_setv2si_internal (target, x, index, target));
  break;
case E_V4SImode:
- emit_insn (gen_neon_vset_lanev4si (target, x, target, index));
+ emit_insn (gen_vec_setv4si_internal (target, x, index, target));
  break;
case E_V2SFmode:
- emit_insn (gen_neon_vset_lanev2sf (target, x, target, index));
+ emit_insn (gen_vec_setv2sf_internal (target, x, index, target));
  break;
case E_V4SFmode:
- emit_insn (gen_neon_vset_lanev4sf (target, x, target, index));
+ emit_insn (gen_vec_setv4sf_internal (target, x, index, target));
  break;
case E_V2DImode:
- emit_insn (gen_neon_vset_lanev2di (target, x, target, index));
+ emit_insn (gen_vec_setv2di_internal (target, x, index, target));
  break;
default:
  gcc_unreachable ();


Can we take the opportunity here to remove that switch statement and use the 
parametrised names machinery:
https://gcc.gnu.org/onlinedocs/gccint/Parameterized-Names.html#Parameterized-Names

so that we can instead have one call to gen_vec_setv8hi_internal (mode, target, 
x, merge_mask, target) or something.
Thanks,
Kyrill



Re: [PATCH][armeb] PR 91060 gcc.c-torture/execute/scal-to-vec1.c fails since r272843

2019-07-08 Thread Richard Sandiford
Christophe Lyon  writes:
> Hi,
>
> This patch fixes PR 91060 where the lane ordering was no longer the
> right one (GCC's vs architecture's).

Sorry, we clashed :-)

I'd prefer to go with the version I attached to bugzilla just now.

Thanks,
Richard


[PATCH][armeb] PR 91060 gcc.c-torture/execute/scal-to-vec1.c fails since r272843

2019-07-08 Thread Christophe Lyon
Hi,

This patch fixes PR 91060 where the lane ordering was no longer the
right one (GCC's vs architecture's).

OK?

Thanks to both Richards for most of the debugging!

Christophe
gcc/ChangeLog:

2019-07-08  Christophe Lyon  

PR target/91060
* config/arm/arm.c (neon_expand_vector_init): Use
vec_set*_internal instead of neon_vset_lane*.

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 820502a..4c7b5a8 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -12471,7 +12471,7 @@ neon_expand_vector_init (rtx target, rtx vals)
   if (n_var == 1)
 {
   rtx copy = copy_rtx (vals);
-  rtx index = GEN_INT (one_var);
+  rtx index = GEN_INT (1 << one_var);
 
   /* Load constant part of vector, substitute neighboring value for
 varying element.  */
@@ -12483,31 +12483,31 @@ neon_expand_vector_init (rtx target, rtx vals)
   switch (mode)
{
case E_V8QImode:
- emit_insn (gen_neon_vset_lanev8qi (target, x, target, index));
+ emit_insn (gen_vec_setv8qi_internal (target, x, index, target));
  break;
case E_V16QImode:
- emit_insn (gen_neon_vset_lanev16qi (target, x, target, index));
+ emit_insn (gen_vec_setv16qi_internal (target, x, index, target));
  break;
case E_V4HImode:
- emit_insn (gen_neon_vset_lanev4hi (target, x, target, index));
+ emit_insn (gen_vec_setv4hi_internal (target, x, index, target));
  break;
case E_V8HImode:
- emit_insn (gen_neon_vset_lanev8hi (target, x, target, index));
+ emit_insn (gen_vec_setv8hi_internal (target, x, index, target));
  break;
case E_V2SImode:
- emit_insn (gen_neon_vset_lanev2si (target, x, target, index));
+ emit_insn (gen_vec_setv2si_internal (target, x, index, target));
  break;
case E_V4SImode:
- emit_insn (gen_neon_vset_lanev4si (target, x, target, index));
+ emit_insn (gen_vec_setv4si_internal (target, x, index, target));
  break;
case E_V2SFmode:
- emit_insn (gen_neon_vset_lanev2sf (target, x, target, index));
+ emit_insn (gen_vec_setv2sf_internal (target, x, index, target));
  break;
case E_V4SFmode:
- emit_insn (gen_neon_vset_lanev4sf (target, x, target, index));
+ emit_insn (gen_vec_setv4sf_internal (target, x, index, target));
  break;
case E_V2DImode:
- emit_insn (gen_neon_vset_lanev2di (target, x, target, index));
+ emit_insn (gen_vec_setv2di_internal (target, x, index, target));
  break;
default:
  gcc_unreachable ();


Re: [PATCH 1/2] [ARC] Fix and refurbish the interrupts.

2019-07-08 Thread Claudiu Zissulescu
Hi Jeff,

Originally, I had the scheduler barrier as you suggested. However,
there were some user cases when an ISR messed up with SP register
leading to errors. As a solution was to add barriers on either part of
frame operations. However, I would need to recheck the original
rationale of that issue, as it may not be the case with newer cores.

On the other hand, I found a small error of the current patch when one
is having the fno-omit-frame-pointer option in the order how some
auxiliary registers are saved. What will be the best solution having
in mind that I haven't pushed this patch to the mainline yet:
- to have the current patch re-spin?
- to have a separate patch for the new issue.

Thanks,
Claudiu

On Wed, Jul 3, 2019 at 2:15 AM Jeff Law  wrote:
>
> On 6/28/19 7:39 AM, Claudiu Zissulescu wrote:
> > When entering an interrupt, not only the call save registers needs to
> > be place on stack but also the call clobbers one. More over, the
> > ARC700 return from interrupt instruction needs to be rtie, the same
> > like ARCv2 CPUs. While the ARC6xx family uses j.f [ilinkX]
> > instruction. Additionally, we need to save the state of the ZOL
> > machinery, namely the lp_count, lp_end and lp_start registers. For
> > architectures which are using extension registers (i.e., HS48) we need
> > to save/restore them as well.
> >
> > Ok to apply?
> > Claudiu
> >
> > gcc/
> > -xx-xx  Claudiu Zissulescu  
> >
> >   * config/arc/arc-protos.h (arc_output_function_epilogue): Delete
> >   declaration.
> >   (arc_return_address_register): Likewise.
> >   (arc_compute_function_type): Likewise.
> >   (arc_compute_frame_size): Likewise.
> >   (secondary_reload_info): Likewise.
> >   (arc_get_unalign): Likewise.
> >   (arc_can_use_return_insn): Declare.
> >   * config/arc/arc.c (AUX_LP_START): Define
> >   (AUX_LP_END): Likewise.
> >   (arc_frame_info): Update gmask member to 64-bit datum.
> >   (GMASK_LEN): Update.
> >   (arc_compute_function_type): Make it static, move it forward.
> >   (arc_must_save_register): Update, consider the extra regs.
> >   (arc_compute_millicode_save_restore_regs): Update to use the 64
> >   bit gmask.
> >   (arc_compute_frame_size): Likewise.
> >   (arc_enter_leave_p): Likewise.
> >   (arc_save_callee_saves): Likewise.
> >   (arc_restore_callee_saves): Likewise.
> >   (arc_save_callee_enter): Likewise.
> >   (arc_restore_callee_leave): Likewise.
> >   (arc_save_callee_milli): Likewise.
> >   (arc_restore_callee_milli): Likewise.
> >   (arc_expand_prologue): Add new interrupt handling.
> >   (arc_return_address_register): Make it static, move it forward.
> >   (arc_expand_epilogue): Add new interrupt handling.
> >   (arc_get_unalign): Delete.
> >   (arc_epilogue_uses): Make sure we do not remove the extra
> >   saved/restored registers when interrupt.
> >   (arc_can_use_return_insn): New function.
> >   * config/arc/arc.md (VUNSPEC_ARC_ARC600_RTIE): Define.
> >   (R33_REG): Likewise.
> >   (R34_REG): Likewise.
> >   (R35_REG): Likewise.
> >   (R36_REG): Likewise.
> >   (R37_REG): Likewise.
> >   (R38_REG): Likewise.
> >   (R39_REG): Likewise.
> >   (R45_REG): Likewise.
> >   (R46_REG): Likewise.
> >   (R47_REG): Likewise.
> >   (R48_REG): Likewise.
> >   (R49_REG): Likewise.
> >   (R50_REG): Likewise.
> >   (R51_REG): Likewise.
> >   (R52_REG): Likewise.
> >   (R53_REG): Likewise.
> >   (R54_REG): Likewise.
> >   (R55_REG): Likewise.
> >   (R56_REG): Likewise.
> >   (R58_REG): Likewise.
> >   (type): Add rtie attribute.
> >   (in_call_delay_slot): Use RETURN_ADDR_REGNUM.
> >   (movsi_insn): Accept moves to lp_count.
> >   (rtie): Update pattern.
> >   (simple_return): Simplify it, don't use this pattern as a return
> >   from an interrupt.
> >   (arc600_rtie): New pattern.
> >   (p_return_i): Clean up.
> >   (return): Likewise.
> >   * config/arc/builtins.def (rtie): Only available for non ARC6xx
> >   family CPUs.
> >   * config/arc/predicates.md (move_src_operand): Consider lp_count
> >   as a register.
> >
> > gcc/testsuite
> > -xx-xx  Claudiu Zissulescu  
> >
> >   * gcc.target/arc/arc.exp (check_effective_target_accregs): New
> >   predicate.
> >   * gcc.target/arc/builtin_special.c: Update test/
> >   * gcc.target/arc/interrupt-1.c: Likewise.
> >   * gcc.target/arc/interrupt-10.c: New test.
> >   * gcc.target/arc/interrupt-11.c: Likewise.
> > ---
> > diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
> > index 4fec8fd8929..910c8d2ad30 100644
> > --- a/gcc/config/arc/arc.c
> > +++ b/gcc/config/arc/arc.c
> > @@ -206,6 +206,13 @@ static int rgf_banked_register_count;
> > this to be no less than the 1/p  */
> >  #define MAX_INSNS_SKIPPED 3
> >
> > +/* ZOL control registers.  */
> > +#define 

[Ada] New algorithm for Elaboration order v4.0

2019-07-08 Thread Pierre-Marie de Rodat
This patch introduces several changes to the new elaboration order
mechanism:

   * The concept of "strong" and "weak" edges is introduced. Strong
 edges are the byproduct of language-defined relations between
 units, such as with clauses. Weak edges are the byproduct of
 specilative invocations at elaboration time, which may or may not
 take place depending on control flow.

   * The elaboration order algorithm has been heavily modified to make
 use of the strong and weak edges, and operate on units compiled
 with different elaboration models.

   * The elaboration order algorithm employs the following logic:

- Maintain two sets of vertices, one for all elaborable
  vertices, and one for all waiting vertices.

- Pick the best elaborable vertex, and elaborate its component.

- If no such elaborable vertex is available, pick the best
  weakly elaborable vertex whose unit has been compiled with the
  dynamic model, and elaborate its component.

- If no such weakly elaborable vertex is available, then either
  all vertices were already elaborated, or the graph contains a
  cycle.

 The elaboration of a component employs the same logic, with an
 added step where all successors of some predecessor currently being
 elaborated are notified that they have one fewer predecessor to
 wait on. This may cause certain successors to become elaborable, in
 which case they are moved from the set of waiting vertices to the
 set of elaborable vertices.

   * Three new GNATbind debug switches are introduced, -d_a, -d_b, and
 -d_e, to eliminate the effects of pragmas Elaborate_All,
 Elaborate_Body, and Elaborate respectively.

   * The section on terminology is updated to include new entries.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-08  Hristian Kirtchev  

gcc/ada/

* bindo.adb: Update the section on terminology to include new
concepts.  Update the section on switches to include new
entries.
* bindo.ads: Add type Precedence_Kind.
* bindo-builders.adb: Add with and use clauses for Debug and
Bindo.Validators.  Add use clauses for
Bindo.Validators.Invocation_Graph_Validators and
Bindo.Validators.Library_Graph_Validators.
(Build_Invocation_Graph): Validate the graph immediately after
it was built.
(Build_Library_Graph): Update the parameter profile. The
creation of the graph is now elaboration model-agnostic.
Validate the graph immediately after it was built.
(Create_With_Edge): Create regular with edges for Elaborate and
Elaborate_All edges when the appropriate debug switches are in
effect.
* bindo-builders.ads (Build_Library_Graph): Update the parameter
profile.
* bindo-diagnostics.adb (Diagnose_Cycle): Track the presence of
an Elaborate_All edge throughout the inspection of the cycle's
edges.
(Output_Dynamic_Model_Suggestions): Output the suggestion only
when the cycle contains at least one weak edge where the
successor was statically elaborated.
(Output_Elaborate_Body_Transition, Output_Forced_Transition,
Output_With_Transition): Update the assertions.
* bindo-elaborators.adb: Remove use clauses for
Bindo.Validators.Invocation_Graph_Validators and
Bindo.Validators.Library_Graph_Validators.  Remove strings
Add_To_All_Candidates_Msg and Add_To_Comp_Candidates_Msg.
Remove type String_Ptr.
(Add_Vertex, Add_Vertex_If_Elaborable, Create_All_Candidates_Set
Create_Component_Candidates_Set): Remove.
(Create_Component_Vertex_Sets, Create_Vertex_Sets): New routine.
(Elaborate_Component): Update the parameter profile and the
comment on usage.  Reimplement the elaboration of a component.
The algorithm will now attempt to elaborate as many vertices
possible. If this is not possible, and a weakly elaborable
vertex is available use unit was compiled using the dynamic
model, the algorithm will elaborate it.
(Elaborate_Library_Graph): Reimplement the elaboration of the
graph. The algorithm will now attempt to elaborate as many
vertices along with their components as possible. If this is not
possible, and a weakly elaborable vertex is available use unit
was compiled using the dynamic model, the algorithm will
elaborate it along with its component.
(Elaborate_Units): Merge with the functionality of
Elaborate_Units_Common.
(Elaborate_Units_Common, Elaborate_Units_Dynamic,
Elaborate_Units_Static): Remove.
(Elaborate_Vertex): Update the parameter profile and the comment
on usage.  Reimplemented.
(Find_Best_Candidate): Remove.
(Find_Best_Elaborable_Vertex, Find_Best_Vertex,
  

[Ada] Wrong evaluation of membership test

2019-07-08 Thread Pierre-Marie de Rodat
The code generated by the compiler erroneously evaluates to True
membership tests when their left operand is a a class-wide interface
object and the right operand is a tagged type that implements such
interface type.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-08  Javier Miranda  

gcc/ada/

* exp_ch4.adb (Tagged_Membership): Fix regression silently
introduced in r260738 that erroneouslusy causes the evaluation
to True of the membership test when the left operand of the
membership test is a class-wide interface object and the right
operand is a type that implements such interface type.

gcc/testsuite/

* gnat.dg/interface10.adb: New testcase.--- gcc/ada/exp_ch4.adb
+++ gcc/ada/exp_ch4.adb
@@ -14156,7 +14156,8 @@ package body Exp_Ch4 is
  --Obj1 in DT'Class; --  Compile time error
  --Obj1 in Iface'Class;  --  Compile time error
 
- if not Is_Class_Wide_Type (Left_Type)
+ if not Is_Interface (Left_Type)
+   and then not Is_Class_Wide_Type (Left_Type)
and then (Is_Ancestor (Etype (Right_Type), Left_Type,
   Use_Full_View => True)
   or else (Is_Interface (Etype (Right_Type))

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/interface10.adb
@@ -0,0 +1,22 @@
+--  { dg-do run }
+--  { dg-options "-gnata" }
+
+with Ada.Text_IO;
+
+procedure Interface10 is
+
+   type Iface is interface;
+
+   type My_First_Type is new Iface with null record;
+   type My_Second_Type is new Iface with null record;
+
+   procedure Do_Test (Object : in Iface'Class) is
+   begin
+  pragma Assert
+((Object in My_First_Type) = (Object in My_First_Type'Class));
+   end;
+
+   V : My_Second_Type;
+begin
+   Do_Test (V);
+end Interface10;



[Ada] More data rates supported on Linux

2019-07-08 Thread Pierre-Marie de Rodat
This patch adds additional data rates to the GNAT.Serial_Communications
package (Linux version).

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-08  Bob Duff  

gcc/ada/

* libgnat/g-sercom.ads, libgnat/g-sercom__linux.adb (Data_Rate):
Support additional data rates.--- gcc/ada/libgnat/g-sercom.ads
+++ gcc/ada/libgnat/g-sercom.ads
@@ -100,8 +100,13 @@ package GNAT.Serial_Communications is
--  cases, an explicit port name can be passed directly to Open.
 
type Data_Rate is
- (B75, B110, B150, B300, B600, B1200, B2400, B4800, B9600,
-  B19200, B38400, B57600, B115200);
+ (B75, B110, B150, B300, B600, B1200,
+  B2400, B4800, B9600,
+  B19200, B38400, B57600, B115200,
+  B230400, B460800, B50, B576000, B921600,
+  B100, B1152000, B150,
+  B200, B250, B300,
+  B350, B400);
--  Speed of the communication
 
type Data_Bits is (CS8, CS7);
@@ -173,18 +178,31 @@ private
end record;
 
Data_Rate_Value : constant array (Data_Rate) of Interfaces.C.unsigned :=
-   (B75 =>  75,
-B110=> 110,
-B150=> 150,
-B300=> 300,
-B600=> 600,
-B1200   =>   1_200,
-B2400   =>   2_400,
-B4800   =>   4_800,
-B9600   =>   9_600,
-B19200  =>  19_200,
-B38400  =>  38_400,
-B57600  =>  57_600,
-B115200 => 115_200);
+   (B75  =>75,
+B110 =>   110,
+B150 =>   150,
+B300 =>   300,
+B600 =>   600,
+B1200=> 1_200,
+B2400=> 2_400,
+B4800=> 4_800,
+B9600=> 9_600,
+B19200   =>19_200,
+B38400   =>38_400,
+B57600   =>57_600,
+B115200  =>   115_200,
+B230400  =>   230_400,
+B460800  =>   460_800,
+B50  =>   500_000,
+B576000  =>   576_000,
+B921600  =>   921_600,
+B100 => 1_000_000,
+B1152000 => 1_152_000,
+B150 => 1_500_000,
+B200 => 2_000_000,
+B250 => 2_500_000,
+B300 => 3_000_000,
+B350 => 3_500_000,
+B400 => 4_000_000);
 
 end GNAT.Serial_Communications;

--- gcc/ada/libgnat/g-sercom__linux.adb
+++ gcc/ada/libgnat/g-sercom__linux.adb
@@ -58,19 +58,32 @@ package body GNAT.Serial_Communications is
pragma Import (C, fcntl, "fcntl");
 
C_Data_Rate : constant array (Data_Rate) of unsigned :=
-   (B75 => OSC.B75,
-B110=> OSC.B110,
-B150=> OSC.B150,
-B300=> OSC.B300,
-B600=> OSC.B600,
-B1200   => OSC.B1200,
-B2400   => OSC.B2400,
-B4800   => OSC.B4800,
-B9600   => OSC.B9600,
-B19200  => OSC.B19200,
-B38400  => OSC.B38400,
-B57600  => OSC.B57600,
-B115200 => OSC.B115200);
+   (B75  => OSC.B75,
+B110 => OSC.B110,
+B150 => OSC.B150,
+B300 => OSC.B300,
+B600 => OSC.B600,
+B1200=> OSC.B1200,
+B2400=> OSC.B2400,
+B4800=> OSC.B4800,
+B9600=> OSC.B9600,
+B19200   => OSC.B19200,
+B38400   => OSC.B38400,
+B57600   => OSC.B57600,
+B115200  => OSC.B115200,
+B230400  => OSC.B230400,
+B460800  => OSC.B460800,
+B50  => OSC.B50,
+B576000  => OSC.B576000,
+B921600  => OSC.B921600,
+B100 => OSC.B100,
+B1152000 => OSC.B1152000,
+B150 => OSC.B150,
+B200 => OSC.B200,
+B250 => OSC.B250,
+B300 => OSC.B300,
+B350 => OSC.B350,
+B400 => OSC.B400);
 
C_Bits  : constant array (Data_Bits) of unsigned :=
   

[Ada] Remove dead code from Enclosing_Package_Or_Subprogram routine

2019-07-08 Thread Pierre-Marie de Rodat
Calls to Scope always return unique entities, i.e. package/subprogram
and not their bodies, so there is no need to expect them.

Cleanup only; semantics unaffected. (This routine was only used in CCG
and GNATprove backends anyway.)

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-08  Piotr Trojanek  

gcc/ada/

* sem_util.adb (Enclosing_Package_Or_Subprogram): Do not expect
package and subprogram bodies.--- gcc/ada/sem_util.adb
+++ gcc/ada/sem_util.adb
@@ -6924,12 +6924,7 @@ package body Sem_Util is
   S := Scope (E);
   while Present (S) loop
  if Is_Package_Or_Generic_Package (S)
-   or else Ekind (S) = E_Package_Body
- then
-return S;
-
- elsif Is_Subprogram_Or_Generic_Subprogram (S)
-   or else Ekind (S) = E_Subprogram_Body
+   or else Is_Subprogram_Or_Generic_Subprogram (S)
  then
 return S;
 



[Ada] Diagnostics for Elaboration order v4.0

2019-07-08 Thread Pierre-Marie de Rodat
This patch adds a missing case to the output of cycle diagnostics here a
transition from an Elaborate_Body pair may reach a destination which is
in the context of an active Elaborate_All.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-08  Hristian Kirtchev  

gcc/ada/

* bindo-diagnostics.adb (Diagnose_Cycle): Capture the presence
of an Elaborate_All edge before iterating over the edges of the
cycle.
(Output_Elaborate_Body_Transition): Update the parameter profile
and the comment on usage. Add a missing case where the edge is
within the context of an Elaborate_All.
(Output_Transition): Update the call to
Output_Elaborate_Body_Transition.
* bindo-graphs.ads, bindo-graphs.adb
(Contains_Elaborate_All_Edge): New routine.--- gcc/ada/bindo-diagnostics.adb
+++ gcc/ada/bindo-diagnostics.adb
@@ -115,13 +115,15 @@ package body Bindo.Diagnostics is
  (G: Library_Graph;
   Source   : Library_Graph_Vertex_Id;
   Actual_Destination   : Library_Graph_Vertex_Id;
-  Expected_Destination : Library_Graph_Vertex_Id);
+  Expected_Destination : Library_Graph_Vertex_Id;
+  Elaborate_All_Active : Boolean);
pragma Inline (Output_Elaborate_Body_Transition);
--  Output a transition through an edge of library graph G with successor
-   --  Source and predecessor Actual_Destination. Vertex Source is either a
-   --  spec subject to pragma Elaborate_Body or denotes the body of such a
-   --  spec. Expected_Destination denotes the predecessor as specified by the
-   --  next edge in a cycle.
+   --  Source and predecessor Actual_Destination. Vertex Source is either
+   --  a spec subject to pragma Elaborate_Body or denotes the body of such
+   --  a spec. Expected_Destination denotes the predecessor as specified by
+   --  the next edge in a cycle. Elaborate_All_Active should be set when the
+   --  transition occurs within a cycle that involves an Elaborate_All edge.
 
procedure Output_Elaborate_Suggestions
  (G: Library_Graph;
@@ -160,7 +162,8 @@ package body Bindo.Diagnostics is
--  Output a transition through a Forced edge of library graph G with
--  successor Source and predecessor Actual_Destination. Parameter
--  Expected_Destination denotes the predecessor as specified by the
-   --  next edge in a cycle.
+   --  next edge in a cycle. Elaborate_All_Active should be set when the
+   --  transition occurs within a cycle that involves an Elaborate_All edge.
 
procedure Output_Full_Encoding_Suggestions
  (G  : Library_Graph;
@@ -328,18 +331,21 @@ package body Bindo.Diagnostics is
   Lib_Graph : Library_Graph;
   Cycle : Library_Graph_Cycle_Id)
is
-  Current_Edge : Library_Graph_Edge_Id;
-  Elaborate_All_Active : Boolean;
-  First_Edge   : Library_Graph_Edge_Id;
-  Iter : Edges_Of_Cycle_Iterator;
-  Next_Edge: Library_Graph_Edge_Id;
-
-   begin
   pragma Assert (Present (Inv_Graph));
   pragma Assert (Present (Lib_Graph));
   pragma Assert (Present (Cycle));
 
-  Elaborate_All_Active := False;
+  Elaborate_All_Active : constant Boolean :=
+   Contains_Elaborate_All_Edge
+ (G => Lib_Graph,
+  Cycle => Cycle);
+
+  Current_Edge : Library_Graph_Edge_Id;
+  First_Edge   : Library_Graph_Edge_Id;
+  Iter : Edges_Of_Cycle_Iterator;
+  Next_Edge: Library_Graph_Edge_Id;
+
+   begin
   First_Edge := No_Library_Graph_Edge;
 
   --  Inspect the edges of the cycle in pairs, emitting diagnostics based
@@ -355,11 +361,6 @@ package body Bindo.Diagnostics is
 Next (Iter, Current_Edge);
 
 First_Edge := Current_Edge;
-Elaborate_All_Active :=
-  Is_Elaborate_All_Edge
-(G=> Lib_Graph,
- Edge => First_Edge);
-
 Output_Reason_And_Circularity_Header
   (G  => Lib_Graph,
First_Edge => First_Edge);
@@ -374,12 +375,6 @@ package body Bindo.Diagnostics is
  --  taking into account the predecessors and successors involved, as
  --  well as the nature of the edge.
 
- Elaborate_All_Active :=
-   Elaborate_All_Active
- or else Is_Elaborate_All_Edge
-   (G=> Lib_Graph,
-Edge => Current_Edge);
-
  Output_Transition
(Inv_Graph=> Inv_Graph,
 Lib_Graph=> Lib_Graph,
@@ -590,7 +585,7 @@ package body Bindo.Diagnostics is
   pragma Assert (Present (Expected_Destination));
 
   --  The actual and expected destination vertices match, and denote the
-  --  spec of a unit.
+  --  initial declaration of a unit.
   --
   --Elaborate_All   

[Ada] Small overhaul in Repinfo unit

2019-07-08 Thread Pierre-Marie de Rodat
This creates a List_Type_Info procedure to deal with type entities other
than arrays and records at top level and a List_Common_Type_Info
procedure to handle the common part between them.  No functional
changes.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-08  Eric Botcazou  

gcc/ada/

* repinfo.adb (List_Common_Type_Info): New procedure extracted
from...
(List_Type_Info): ...here.  Call it for the common information,
start with a blank line and output the linker section at the
end, if any.
(List_Mechanisms): Rename to...
(List_Subprogram_Info): ...this.
(List_Array_Info): Call List_Common_Type_Info.
(List_Entities): Adjust to above change and renaming.
(List_Record_Info): Call List_Common_Type_Info.--- gcc/ada/repinfo.adb
+++ gcc/ada/repinfo.adb
@@ -172,6 +172,9 @@ package body Repinfo is
procedure List_Array_Info (Ent : Entity_Id; Bytes_Big_Endian : Boolean);
--  List representation info for array type Ent
 
+   procedure List_Common_Type_Info (Ent : Entity_Id);
+   --  List common type info (name, size, alignment) for type Ent
+
procedure List_Linker_Section (Ent : Entity_Id);
--  List linker section for Ent (caller has checked that Ent is an entity
--  for which the Linker_Section_Pragma field is defined).
@@ -179,10 +182,6 @@ package body Repinfo is
procedure List_Location (Ent : Entity_Id);
--  List location information for Ent
 
-   procedure List_Mechanisms (Ent : Entity_Id);
-   --  List mechanism information for parameters of Ent, which is subprogram,
-   --  subprogram type, or an entry or entry family.
-
procedure List_Object_Info (Ent : Entity_Id);
--  List representation info for object Ent
 
@@ -195,6 +194,9 @@ package body Repinfo is
--  List scalar storage order information for record or array type Ent.
--  Also includes bit order information for record types, if necessary.
 
+   procedure List_Subprogram_Info (Ent : Entity_Id);
+   --  List subprogram info for subprogram Ent
+
procedure List_Type_Info (Ent : Entity_Id);
--  List type info for type Ent
 
@@ -346,7 +348,7 @@ package body Repinfo is
  Write_Line ("{");
   end if;
 
-  List_Type_Info (Ent);
+  List_Common_Type_Info (Ent);
 
   if List_Representation_Info_To_JSON then
  Write_Line (",");
@@ -370,6 +372,81 @@ package body Repinfo is
   end if;
end List_Array_Info;
 
+   ---
+   -- List_Common_Type_Info --
+   ---
+
+   procedure List_Common_Type_Info (Ent : Entity_Id) is
+   begin
+  if List_Representation_Info_To_JSON then
+ Write_Str ("  ""name"": """);
+ List_Name (Ent);
+ Write_Line (""",");
+ List_Location (Ent);
+  end if;
+
+  --  Do not list size info for unconstrained arrays, not meaningful
+
+  if Is_Array_Type (Ent) and then not Is_Constrained (Ent) then
+ null;
+
+  else
+ --  If Esize and RM_Size are the same, list as Size. This is a common
+ --  case, which we may as well list in simple form.
+
+ if Esize (Ent) = RM_Size (Ent) then
+if List_Representation_Info_To_JSON then
+   Write_Str ("  ""Size"": ");
+   Write_Val (Esize (Ent));
+   Write_Line (",");
+else
+   Write_Str ("for ");
+   List_Name (Ent);
+   Write_Str ("'Size use ");
+   Write_Val (Esize (Ent));
+   Write_Line (";");
+end if;
+
+ --  Otherwise list size values separately
+
+ else
+if List_Representation_Info_To_JSON then
+   Write_Str ("  ""Object_Size"": ");
+   Write_Val (Esize (Ent));
+   Write_Line (",");
+
+   Write_Str ("  ""Value_Size"": ");
+   Write_Val (RM_Size (Ent));
+   Write_Line (",");
+
+else
+   Write_Str ("for ");
+   List_Name (Ent);
+   Write_Str ("'Object_Size use ");
+   Write_Val (Esize (Ent));
+   Write_Line (";");
+
+   Write_Str ("for ");
+   List_Name (Ent);
+   Write_Str ("'Value_Size use ");
+   Write_Val (RM_Size (Ent));
+   Write_Line (";");
+end if;
+ end if;
+  end if;
+
+  if List_Representation_Info_To_JSON then
+ Write_Str ("  ""Alignment"": ");
+ Write_Val (Alignment (Ent));
+  else
+ Write_Str ("for ");
+ List_Name (Ent);
+ Write_Str ("'Alignment use ");
+ Write_Val (Alignment (Ent));
+ Write_Line (";");
+  end if;
+   end List_Common_Type_Info;
+
---
-- List_Entities --
---
@@ -428,7 +505,7 @@ package body Repinfo is
and then not In_Subprogram
  then
 Need_Blank_Line := 

[Ada] Remove dependency on Win32 GDI (Graphical Interface)

2019-07-08 Thread Pierre-Marie de Rodat
CommandLineToArgvW drags a dependency on SHELL32.DLL and thus GDI32.DLL.
By loading GDI32.DLL some default GDI objects are allocated. On some
Windows versions this cause the use of a lock on the graphical interface
during process termination. This can impact parallelism significantly as
termination of processes is serialized.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-08  Nicolas Roche  

gcc/ada/

* rtinit.c (__gnat_runtime_initialize): Remove dependency on
CommandLineToArgvW.--- gcc/ada/rtinit.c
+++ gcc/ada/rtinit.c
@@ -62,7 +62,7 @@ extern "C" {
 /* __gnat_runtime_initialize (NT-mingw32 Version) */
 /**/
 
-extern void __gnat_install_handler(void);
+extern void __gnat_install_handler (void);
 
 int __gnat_wide_text_translation_required = 0;
 /* wide text translation, 0=none, 1=activated */
@@ -89,6 +89,189 @@ extern HANDLE ProcListEvt;
 int __gnat_do_argv_expansion = 1;
 #pragma weak __gnat_do_argv_expansion
 
+/* Assuming we are pointing to the beginning of a quoted part of an
+argument, skip until the end of the quoted part.  */
+static void skip_quoted_string (const WCHAR **current_in,
+WCHAR **current_out)
+{
+  /* Number of backslashes buffered.  */
+  int qbs_count = 0;
+
+  /* Pointer to current input character.  */
+  const WCHAR *ci = *current_in;
+
+  /* Pointer to next output character.  */
+  WCHAR *co = *current_out;
+
+  /* Skip initial quote.  */
+  ci++;
+
+  while (*ci)
+{
+  if (*ci == '\\')
+	{
+	  /* Buffer incoming backslashes.  */
+	  qbs_count++;
+	}
+  else if (*ci == '"')
+	{
+	  /* Append qbs_count / 2 backslahes.  */
+	  for (int i=0; i

[Ada] Do not erase precise type on fixed-point real literal

2019-07-08 Thread Pierre-Marie de Rodat
Real literals of fixed-point type are expected to keep their precise
fixed-point type in GNATprove. This is now correctly enforced.

There is no impact on compilation.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-08  Yannick Moy  

gcc/ada/

* expander.adb (Expand): Do not reset Analyzed flag always.
* sem_eval.adb (Fold_Ureal): Mark node as analyzed.--- gcc/ada/expander.adb
+++ gcc/ada/expander.adb
@@ -112,7 +112,12 @@ package body Expander is
 Expand_SPARK (N);
  end if;
 
- Set_Analyzed (N, Full_Analysis);
+ --  Do not reset the Analyzed flag if it has been set on purpose
+ --  during preanalysis.
+
+ if Full_Analysis then
+Set_Analyzed (N);
+ end if;
 
  --  Regular expansion is normally followed by special handling for
  --  transient scopes for unconstrained results, etc. but this is not

--- gcc/ada/sem_eval.adb
+++ gcc/ada/sem_eval.adb
@@ -4611,10 +4611,14 @@ package body Sem_Eval is
   --  will cause semantic errors if it is marked as static), and after
   --  the Resolve step (since Resolve in some cases sets this flag).
 
+  --  We mark the node as analyzed so that its type is not erased by
+  --  calling Analyze_Real_Literal.
+
   Analyze (N);
   Set_Is_Static_Expression (N, Static);
   Set_Etype (N, Typ);
   Resolve (N);
+  Set_Analyzed (N);
   Set_Is_Static_Expression (N, Static);
end Fold_Ureal;
 



[Ada] Fix crash on extension of private type with -gnatRj

2019-07-08 Thread Pierre-Marie de Rodat
This fixes a crash (or an assertion failure) during the processing done
for -gnatRj on the declaration of an extension of a private type.
Generally speaking, extension declarations are delicate in this context
because the front-end does not duplicate the structure of the parent
type, so the processing required to output the structural layout needs
to go up to the declaration of the parent type, which may or may not be
available or usable.

The change also makes the processing more robust by falling back to the
flat layout if the declaration of the parent type cannot be processed.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-08  Eric Botcazou  

gcc/ada/

* repinfo.adb (List_Record_Info): Declare Incomplete_Layout and
Not_In_Extended_Main local exceptions.
(List_Structural_Record_Layout): For an extension, raise the
former if the parent subtype has not been built and the latter
if it is not declared in the main source unit.  Fall back to the
flat layout if either exception has been raised.--- gcc/ada/repinfo.adb
+++ gcc/ada/repinfo.adb
@@ -1125,6 +1125,12 @@ package body Repinfo is
  Indent: Natural := 0);
   --  Internal recursive procedure to display the structural layout
 
+  Incomplete_Layout : exception;
+  --  Exception raised if the layout is incomplete in -gnatc mode
+
+  Not_In_Extended_Main : exception;
+  --  Exception raised when an ancestor is not declared in the main unit
+
   Max_Name_Length : Natural := 0;
   Max_Spos_Length : Natural := 0;
 
@@ -1564,14 +1570,29 @@ package body Repinfo is
 
Disc: Entity_Id;
Listed_Disc : Entity_Id;
+   Parent_Type : Entity_Id;
 
 begin
--  If this is an extension, first list the layout of the parent
--  and then proceed to the extension part, if any.
 
if Is_Extension then
-  List_Structural_Record_Layout
-(Base_Type (Parent_Subtype (Ent)), Outer_Ent);
+  Parent_Type := Parent_Subtype (Ent);
+  if No (Parent_Type) then
+ raise Incomplete_Layout;
+  end if;
+
+  if Is_Private_Type (Parent_Type) then
+ Parent_Type := Full_View (Parent_Type);
+ pragma Assert (Present (Parent_Type));
+  end if;
+
+  Parent_Type := Base_Type (Parent_Type);
+  if not In_Extended_Main_Source_Unit (Parent_Type) then
+ raise Not_In_Extended_Main;
+  end if;
+
+  List_Structural_Record_Layout (Parent_Type, Outer_Ent);
   First := False;
 
   if Present (Record_Extension_Part (Definition)) then
@@ -1733,8 +1754,23 @@ package body Repinfo is
  Write_Line (",");
  Write_Str ("  ""record"": [");
 
+ --  ??? We can output structural layout only for base types fully
+ --  declared in the extended main source unit for the time being,
+ --  because otherwise declarations might not be processed at all.
+
  if Is_Base_Type (Ent) then
-List_Structural_Record_Layout (Ent, Ent);
+begin
+   List_Structural_Record_Layout (Ent, Ent);
+
+exception
+   when Incomplete_Layout
+  | Not_In_Extended_Main
+   =>
+  List_Record_Layout (Ent);
+
+   when others =>
+  raise Program_Error;
+end;
  else
 List_Record_Layout (Ent);
  end if;



[Ada] Crash in interface derivation with null primitive

2019-07-08 Thread Pierre-Marie de Rodat
The frontend crashes processing the derivation of a tagged type whose
ultimate ancestor is an interface type I1 that has a null primitive,
implements another interface I2 derived from I2, and does not override
the null primitive.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-08  Javier Miranda  

gcc/ada/

* exp_disp.adb (Register_Primitive): When registering a
primitive in the secondary dispatch table, handle primitive
inherited through several levels of type derivation (required to
properly handle inherited 'null' primitive).

gcc/testsuite/

* gnat.dg/interface9.adb, gnat.dg/interface9_root-child.ads,
gnat.dg/interface9_root.ads: New testcase.--- gcc/ada/exp_disp.adb
+++ gcc/ada/exp_disp.adb
@@ -7637,7 +7637,7 @@ package body Exp_Disp is
  Unchecked_Convert_To (RTE (RE_Prim_Ptr),
Make_Attribute_Reference (Loc,
  Prefix =>
-   New_Occurrence_Of (Alias (Prim), Loc),
+   New_Occurrence_Of (Ultimate_Alias (Prim), Loc),
  Attribute_Name => Name_Unrestricted_Access;
 
 end if;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/interface9.adb
@@ -0,0 +1,10 @@
+--  { dg-do compile }
+
+with Interface9_Root.Child;
+procedure Interface9 is
+   package R   is new Interface9_Root (Real => Float);
+   package RC  is new R.Child;
+
+begin
+   null;
+end Interface9;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/interface9_root-child.ads
@@ -0,0 +1,7 @@
+generic
+package Interface9_Root.Child is
+type Base_Type is abstract new Base_Interface with null record;
+
+type Derived_Type is abstract new Base_Type and Derived_Interface
+  with null record; --  Test
+end Interface9_Root.Child;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/interface9_root.ads
@@ -0,0 +1,10 @@
+generic
+   type Real is digits <>;
+package Interface9_Root is
+   type Base_Interface is limited interface;
+
+   procedure Primitive1 (B : in out Base_Interface) is abstract;
+   procedure Primitive2 (B : in out Base_Interface) is null;
+
+   type Derived_Interface is limited interface and Base_Interface;
+end Interface9_Root;



[Ada] Assertion failure on validity check for Address

2019-07-08 Thread Pierre-Marie de Rodat
This patch corrects the verification of 'Address clauses to avoid
processing a clause where the prefix of the attribute is a generic
formal object.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-08  Hristian Kirtchev  

gcc/ada/

* sem_ch13.adb (Analyze_Attribute_Definition_Clause): Do not
register an address clause when its prefix denotes a generic
formal object.

gcc/testsuite/

* gnat.dg/addr13.adb, gnat.dg/addr13.ads: New testcase.--- gcc/ada/sem_ch13.adb
+++ gcc/ada/sem_ch13.adb
@@ -5145,6 +5145,7 @@ package body Sem_Ch13 is
  --  aspect case properly.
 
  if Is_Object (O_Ent)
+   and then not Is_Generic_Formal (O_Ent)
and then not Is_Generic_Type (Etype (U_Ent))
and then Address_Clause_Overlay_Warnings
  then

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/addr13.adb
@@ -0,0 +1,9 @@
+--  { dg-do compile }
+
+package body Addr13 is
+   procedure Overlay is
+  Over : Integer with Address => Gen_Obj'Address;
+   begin
+  Over := 123;
+   end Overlay;
+end Addr13;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/addr13.ads
@@ -0,0 +1,5 @@
+generic
+   Gen_Obj : in out Integer;
+package Addr13 is
+   procedure Overlay;
+end Addr13;



[Ada] Set dummy Etype for the fake __HEAP entity in GNATprove

2019-07-08 Thread Pierre-Marie de Rodat
GNATprove represents reads and writes via pointers as operations on a
fake __HEAP entity. This entity already had various properties set to
dummy values (e.g. Scope set to Standard_Standard), so that it can be
processed like other entities without crashing and not special-cased
everywhere. Now it also has a dummy Etype, so it can be processed with
Is_Single_Concurrent_Object.

The modified code is only executed by GNATprove; frontend is not
affected.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-08  Piotr Trojanek  

gcc/ada/

* lib-xref-spark_specific.adb (Create_Heap): Set dummy Etype for
the fake __HEAP entity.--- gcc/ada/lib-xref-spark_specific.adb
+++ gcc/ada/lib-xref-spark_specific.adb
@@ -298,6 +298,7 @@ package body SPARK_Specific is
 
  Set_Ekind   (Heap, E_Variable);
  Set_Is_Internal (Heap, True);
+ Set_Etype   (Heap, Standard_Void_Type);
  Set_Scope   (Heap, Standard_Standard);
  Set_Has_Fully_Qualified_Name (Heap);
   end Create_Heap;



[Ada] Crash on named actual in postcondition for generic subprogram

2019-07-08 Thread Pierre-Marie de Rodat
This patch fixes a crash on compiling the postcondtion for a generic
subprogram, when the postcondition is a call with both positional and
named parameter associations.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-08  Ed Schonberg  

gcc/ada/

* sem_ch13.adb (Analyze_Aspect_Specifications): For a
pre/postcondition of a generic subprogram declaration, do not
use Relocate_Node on the aspect expression to construct the
corresponding attribute specification, to prevent tree anomalies
when the expression is a call with named actual parameters.

gcc/testsuite/

* gnat.dg/predicate9.adb: New testcase.--- gcc/ada/sem_ch13.adb
+++ gcc/ada/sem_ch13.adb
@@ -3495,12 +3495,28 @@ package body Sem_Ch13 is
   --  because subsequent visibility analysis of the aspect
   --  depends on this sharing. This should be cleaned up???
 
-  Make_Aitem_Pragma
-(Pragma_Argument_Associations => New_List (
-   Make_Pragma_Argument_Association (Eloc,
- Chars  => Name_Check,
- Expression => Relocate_Node (Expr))),
-   Pragma_Name=> Pname);
+  --  If the context is generic or involves ASIS, we want
+  --  to preserve the original tree, and simply share it
+  --  between aspect and generated attribute. This parallels
+  --  what is done in sem_prag.adb (see Get_Argument).
+
+  declare
+ New_Expr : Node_Id;
+
+  begin
+ if ASIS_Mode or else Inside_A_Generic then
+New_Expr := Expr;
+ else
+New_Expr := Relocate_Node (Expr);
+ end if;
+
+ Make_Aitem_Pragma
+   (Pragma_Argument_Associations => New_List (
+  Make_Pragma_Argument_Association (Eloc,
+Chars  => Name_Check,
+Expression => New_Expr)),
+  Pragma_Name=> Pname);
+  end;
 
   --  Add message unless exception messages are suppressed
 

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/predicate9.adb
@@ -0,0 +1,21 @@
+--  { dg-do compile }
+procedure Predicate9 is
+  function Num (x : Integer) return Integer is (X + 1);
+  function name (X : String) return Integer is (X'Size);
+  function Post (One : Integer; Two : Integer) return Boolean;
+
+  generic
+ type T is private;
+  procedure Pro (Z : Integer) with Post =>
+Post (Num (5), Two => Name ("yeah"));
+
+  function Post (One : Integer; Two : Integer) return Boolean
+  is (True);
+
+  procedure Pro (Z : Integer) is
+  begin
+ null;
+  end Pro;
+begin
+   null;
+end Predicate9;



[Ada] Crash on timed entry call with a delay given by a type conversion

2019-07-08 Thread Pierre-Marie de Rodat
This patch fixes a compiler crash in the compiler on a timed entry call
whose delay expression is a type conversion, when FLoat_Overflow checks
are enabled.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-08  Ed Schonberg  

gcc/ada/

* exp_ch9.adb (Expand_N_Timed_Entry_Call): Do not insert twice
the assignment statement that computes the delay value, to
prevent improper tree sharing when the value is a type
conversion and Float_Overflow checks are enabled.

gcc/testsuite/

* gnat.dg/entry1.adb, gnat.dg/entry1.ads: New testcase.--- gcc/ada/exp_ch9.adb
+++ gcc/ada/exp_ch9.adb
@@ -3887,6 +3887,7 @@ package body Exp_Ch9 is
 
  if Unprotected then
 Set_Protected_Formal (Formal, Defining_Identifier (New_Param));
+Set_Ekind (Defining_Identifier (New_Param), Ekind (Formal));
  end if;
 
  Append (New_Param, New_Plist);
@@ -10711,7 +10712,7 @@ package body Exp_Ch9 is
   Make_Defining_Identifier (Eloc,
 New_External_Name (Chars (Ename), 'A', Num_Accept));
 
---  Link the acceptor to the original receiving entry
+--  Link the acceptor to the original receiving entry.
 
 Set_Ekind   (PB_Ent, E_Procedure);
 Set_Receiving_Entry (PB_Ent, Eent);
@@ -12658,14 +12659,6 @@ package body Exp_Ch9 is
   Object_Definition   => New_Occurrence_Of (Standard_Integer, Loc),
   Expression  => D_Disc));
 
-  --  Do the assignment at this stage only because the evaluation of the
-  --  expression must not occur earlier (see ACVC C97302A).
-
-  Append_To (Stmts,
-Make_Assignment_Statement (Loc,
-  Name   => New_Occurrence_Of (D, Loc),
-  Expression => D_Conv));
-
   --  Parameter block processing
 
   --  Manually create the parameter block for dispatching calls. In the
@@ -12673,6 +12666,13 @@ package body Exp_Ch9 is
   --  to Build_Simple_Entry_Call.
 
   if Is_Disp_Select then
+ --  Compute the delay at this stage because the evaluation of
+ --  its expression must not occur earlier (see ACVC C97302A).
+
+ Append_To (Stmts,
+   Make_Assignment_Statement (Loc,
+ Name   => New_Occurrence_Of (D, Loc),
+ Expression => D_Conv));
 
  --  Tagged kind processing, generate:
  --K : Ada.Tags.Tagged_Kind :=
@@ -12855,8 +12855,8 @@ package body Exp_Ch9 is
 Next (Stmt);
  end loop;
 
- --  Do the assignment at this stage only because the evaluation
- --  of the expression must not occur earlier (see ACVC C97302A).
+ --  Compute the delay at this stage because the evaluation of
+ --  its expression must not occur earlier (see ACVC C97302A).
 
  Insert_Before (Stmt,
Make_Assignment_Statement (Loc,
@@ -14882,7 +14882,8 @@ package body Exp_Ch9 is
 
  --  Ditto for a package declaration or a full type declaration, etc.
 
- elsif Nkind (N) = N_Package_Declaration
+ elsif
+   (Nkind (N) = N_Package_Declaration and then N /= Specification (N))
or else Nkind (N) in N_Declaration
or else Nkind (N) in N_Renaming_Declaration
  then

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/entry1.adb
@@ -0,0 +1,75 @@
+--  { dg-do compile }
+--  { dg-options "-gnateF" }
+
+PACKAGE BODY Entry1 IS
+
+   PROTECTED TYPE key_buffer IS
+
+  PROCEDURE clear;
+
+  ENTRY incr;
+  ENTRY put (val : IN Natural);
+  ENTRY get (val : OUT Natural);
+
+   PRIVATE
+
+  -- Stores Key states (key state controller)
+  -- purpose: exclusive access
+  max_len : Natural := 10;
+
+  cnt : Natural := 0;
+
+   END key_buffer;
+
+   PROTECTED BODY key_buffer IS
+
+  PROCEDURE clear IS
+  BEGIN
+ cnt := 0;
+  END clear;
+
+  ENTRY incr WHEN cnt < max_len IS
+  BEGIN
+ cnt := cnt + 1;
+  END;
+
+  ENTRY put (val : IN Natural) WHEN cnt < max_len IS
+  BEGIN
+ cnt := val;
+  END put;
+
+  ENTRY get (val : OUT Natural) WHEN cnt > 0 IS
+  BEGIN
+ val := cnt;
+  END get;
+
+   END key_buffer;
+
+   my_buffer : key_buffer;
+
+   FUNCTION pt2 (t : IN Float) RETURN Natural IS
+  c : Natural;
+  t2 : duration := duration (t);
+   BEGIN
+  SELECT
+ my_buffer.get (c);
+ RETURN c;
+  OR
+ DELAY t2;
+ RETURN 0;
+  END SELECT;
+   END pt2;
+
+   FUNCTION pt (t : IN Float) RETURN Natural IS
+  c : Natural;
+   BEGIN
+  SELECT
+ my_buffer.get (c);
+ RETURN c;
+  OR
+ DELAY Duration (t);
+ RETURN 0;
+  END SELECT;
+   END pt;
+
+END Entry1;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/entry1.ads
@@ -0,0 +1,4 @@
+PACKAGE Entry1 IS
+   FUNCTION pt (t : IN Float) RETURN Natural;
+   FUNCTION pt2 (t : IN 

[Ada] Semantics of Delete for fixed strings

2019-07-08 Thread Pierre-Marie de Rodat
This patch corrects a bug in the implementation of Delete in an unusual
boundary case: the RM describes the semantics of Delete as equivalent to
that of Replace_String with a null argument. As a result, deleting a
null string that starts past the end of its argument is a noop and must
not raise Index_Error.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-08  Ed Schonberg  

gcc/ada/

* libgnat/a-strfix.adb (Delete): The RM describes the semantics
of Delete as equivalent to that of Replace_String with a null
argument. As a result, deleting a null string that starts past
the end of its argument is a noop and must not raise
Index_Error.

gcc/testsuite/

* gnat.dg/fixed_delete.adb: New testcase.--- gcc/ada/libgnat/a-strfix.adb
+++ gcc/ada/libgnat/a-strfix.adb
@@ -192,7 +192,15 @@ package body Ada.Strings.Fixed is
   elsif From not in Source'Range
 or else Through > Source'Last
   then
- raise Index_Error;
+ --  In most cases this raises an exception, but the case of deleting
+ --  a null string at the end of the current one is a special-case, and
+ --  reflects the equivalence with Replace_String (RM A.4.3 (86/3)).
+
+ if From = Source'Last + 1 and then From = Through then
+return Source;
+ else
+raise Index_Error;
+ end if;
 
   else
  declare

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/fixed_delete.adb
@@ -0,0 +1,17 @@
+--  { dg-do run }
+
+with Ada.Text_IO; use Ada.Text_IO;
+with Ada.Strings.Fixed; use Ada.Strings.Fixed;
+
+procedure Fixed_Delete is
+   Str  : String := "a";
+   Str1 : String := Replace_Slice (Str, 2, 2, "");
+   Str2 : String := Delete (Str, 2, 2);
+begin
+   if Str1 /= "a" then
+  raise Program_Error;
+   end if;
+   if Str2 /= "a" then
+  raise Program_Error;
+   end if;
+end Fixed_Delete;



[Ada] Arrange not to set DECL_ARTIFICIAL on elab procs

2019-07-08 Thread Pierre-Marie de Rodat
Unlike, say, clones created internally by the compiler, elab procs
materialize specific user code and flagging them artificial now takes
elab code away from gcov's analysis, a regression compared to previous
releases.

On the testcase below:

package Gcov_Q is
   function F (X : Integer) return Integer is (X + 1);
end;

with Gcov_Q;
package Gcov_P is
   Y : Integer := Gcov_Q.F (X => 1);
end;

with Gcov_P;
procedure Gcov_Test is
begin
   if Gcov_P.Y /= 2 then
  raise Program_Error;
   end if;
end;

After compiling with:

  gnatmake -f -g gcov_test.adb \
-cargs -ftest-coverage -fprofile-arcs \
-largs -fprofile-generate

and executing with

  ./gcov_test

We expect

   gcov gcov_p

to produce a gcov_p.ads.gcov report.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-08  Olivier Hainque  

gcc/ada/

* gcc-interface/trans.c (Compilation_Unit_to_gnu): Don't request
DECL_ARTIFICIAL_P on elab proc declarations.--- gcc/ada/gcc-interface/trans.c
+++ gcc/ada/gcc-interface/trans.c
@@ -6276,13 +6276,17 @@ Compilation_Unit_to_gnu (Node_Id gnat_node)
   Node_Id gnat_pragma;
   /* Make the decl for the elaboration procedure.  Emit debug info for it, so
  that users can break into their elaboration code in debuggers.  Kludge:
- don't consider it as a definition so that we have a line map for its body,
- but no subprogram description in debug info. */
+ don't consider it as a definition so that we have a line map for its
+ body, but no subprogram description in debug info.  In addition, don't
+ qualify it as artificial, even though it is not a user subprogram per se,
+ in particular for specs.  Unlike, say, clones created internally by the
+ compiler, this subprogram materializes specific user code and flagging it
+ artificial would take elab code away from gcov's analysis.  */
   tree gnu_elab_proc_decl
 = create_subprog_decl
   (create_concat_name (gnat_unit_entity, body_p ? "elabb" : "elabs"),
NULL_TREE, void_ftype, NULL_TREE,
-   is_default, true, false, true, true, false, NULL, gnat_unit);
+   is_default, true, false, false, true, false, NULL, gnat_unit);
   struct elab_info *info;
 
   vec_safe_push (gnu_elab_proc_stack, gnu_elab_proc_decl);



[Ada] Crash on Image and Value attributes

2019-07-08 Thread Pierre-Marie de Rodat
This patch fixes an issue whereby the creation of an enumeration within
package where Default_Scalar_Storage_Order is in effect may lead to a
crash when the attributes Image or Value are applied to objects of said
type or the type directly.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-08  Justin Squirek  

gcc/ada/

* exp_imgv.adb (Build_Enumeration_Image_Tables): Default SSO for
the building of image tables.
(Expand_Image_Attribute): Minor cleanup.

gcc/testsuite/

* gnat.dg/sso16.adb: New testcase.--- gcc/ada/exp_imgv.adb
+++ gcc/ada/exp_imgv.adb
@@ -69,18 +69,23 @@ package body Exp_Imgv is

 
procedure Build_Enumeration_Image_Tables (E : Entity_Id; N : Node_Id) is
-  Loc  : constant Source_Ptr := Sloc (E);
-  Str  : String_Id;
+  Loc : constant Source_Ptr := Sloc (E);
+
+  Eind : Entity_Id;
+  Estr : Entity_Id;
   Ind  : List_Id;
+  Ityp : Node_Id;
+  Len  : Nat;
   Lit  : Entity_Id;
   Nlit : Nat;
-  Len  : Nat;
-  Estr : Entity_Id;
-  Eind : Entity_Id;
-  Ityp : Node_Id;
+  Str  : String_Id;
+
+  Saved_SSO : constant Character := Opt.Default_SSO;
+  --  Used to save the current scalar storage order during the generation
+  --  of the literal lookup table.
 
begin
-  --  Nothing to do for other than a root enumeration type
+  --  Nothing to do for types other than a root enumeration type
 
   if E /= Root_Type (E) then
  return;
@@ -138,6 +143,15 @@ package body Exp_Imgv is
   Set_Lit_Strings (E, Estr);
   Set_Lit_Indexes (E, Eind);
 
+  --  Temporarily set the current scalar storage order to the default
+  --  during the generation of the literals table, since both the Image and
+  --  Value attributes rely on runtime routines for interpreting table
+  --  values.
+
+  Opt.Default_SSO := ' ';
+
+  --  Generate literal table
+
   Insert_Actions (N,
 New_List (
   Make_Object_Declaration (Loc,
@@ -168,6 +182,10 @@ package body Exp_Imgv is
   Make_Aggregate (Loc,
 Expressions => Ind))),
 Suppress => All_Checks);
+
+  --  Reset the scalar storage order to the saved value
+
+  Opt.Default_SSO := Saved_SSO;
end Build_Enumeration_Image_Tables;
 

@@ -433,13 +451,13 @@ package body Exp_Imgv is
 
   --  Local variables
 
+  Enum_Case : Boolean;
   Imid  : RE_Id;
+  Proc_Ent  : Entity_Id;
   Ptyp  : Entity_Id;
   Rtyp  : Entity_Id;
   Tent  : Entity_Id := Empty;
   Ttyp  : Entity_Id;
-  Proc_Ent  : Entity_Id;
-  Enum_Case : Boolean;
 
   Arg_List : List_Id;
   --  List of arguments for run-time procedure call
@@ -450,6 +468,8 @@ package body Exp_Imgv is
   Snn : constant Entity_Id := Make_Temporary (Loc, 'S');
   Pnn : constant Entity_Id := Make_Temporary (Loc, 'P');
 
+   --  Start of processing for Expand_Image_Attribute
+
begin
   if Is_Object_Image (Pref) then
  Rewrite_Object_Image (N, Pref, Name_Image, Standard_String);

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/sso16.adb
@@ -0,0 +1,55 @@
+--  { dg-do run }
+
+with Ada.Text_IO; use Ada.Text_IO;
+
+procedure SSO16 is
+
+   pragma Default_Scalar_Storage_Order (High_Order_First);
+
+   type Enum_T is
+ (Event_0,
+  Event_1,
+  Event_2,
+  Event_3,
+  Event_4,
+  Event_5,
+  Event_11,
+  Event_12,
+  Event_13,
+  Event_14,
+  Event_15,
+  Event_21,
+  Event_22,
+  Event_23,
+  Event_24,
+  Event_25,
+  Event_31,
+  Event_32,
+  Event_33,
+  Event_34,
+  Event_35,
+  Event_41,
+  Event_42,
+  Event_43,
+  Event_44,
+  Event_45);
+
+   Var : Enum_T := Event_0;
+
+begin
+   if Var'Image /= "EVENT_0" then
+  raise Program_Error;
+   end if;
+
+   if Enum_T'Value ("Event_4")'Image /= "EVENT_4" then
+  raise Program_Error;
+   end if;
+
+   if Enum_T'Val (20)'Image /= "EVENT_35" then
+  raise Program_Error;
+   end if;
+
+   if Enum_T'Pos (Enum_T'Value ("Event_45"))'Image /= " 25" then
+  raise Program_Error;
+   end if;
+end;



[Ada] Spurious visibility error on dynamic_predicate aspect in generic

2019-07-08 Thread Pierre-Marie de Rodat
This patch fixes a spurious error when verifying that the visibility of
the expression of an aspect has not changed between the freeze point of
the entity to which it applies, and the end of the enclosing declarative
part. If the entity is a composite type its components must be made
directly visible for the analysis of the expression. In a generic
context this must be done explicitly at the end of the declarative part.

Tested on x86_64-pc-linux-gnu, committed on trunk

2019-07-08  Ed Schonberg  

gcc/ada/

* sem_ch13.adb (Check_Aspect_At_End_Of_Declarations): For an
unanalized aspect in a generic context that has not been
analyzed yet, if the aspect applies to a type, place the type on
the scope stack to make its components visible, before checking
conformance with the version of the expression analyzed at the
freeze point.

gcc/testsuite/

* gnat.dg/predicate8.adb, gnat.dg/predicate8_pkg.adb,
gnat.dg/predicate8_pkg.ads: New testcase.--- gcc/ada/sem_ch13.adb
+++ gcc/ada/sem_ch13.adb
@@ -3491,7 +3491,9 @@ package body Sem_Ch13 is
 
   --  Build the precondition/postcondition pragma
 
-  --  Add note about why we do NOT need Copy_Tree here???
+  --  We use Relocate_Node here rather than New_Copy_Tree
+  --  because subsequent visibility analysis of the aspect
+  --  depends on this sharing. This should be cleaned up???
 
   Make_Aitem_Pragma
 (Pragma_Argument_Associations => New_List (
@@ -9358,10 +9360,20 @@ package body Sem_Ch13 is
 
   else
  --  In a generic context freeze nodes are not always generated, so
- --  analyze the expression now.
+ --  analyze the expression now. If the aspect is for a type, this
+ --  makes its potential components accessible.
 
  if not Analyzed (Freeze_Expr) and then Inside_A_Generic then
-Preanalyze (Freeze_Expr);
+if A_Id = Aspect_Dynamic_Predicate
+  or else A_Id = Aspect_Predicate
+  or else A_Id = Aspect_Priority
+then
+   Push_Type (Ent);
+   Preanalyze (Freeze_Expr);
+   Pop_Type (Ent);
+else
+   Preanalyze (Freeze_Expr);
+end if;
  end if;
 
  --  Indicate that the expression comes from an aspect specification,

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/predicate8.adb
@@ -0,0 +1,15 @@
+--  { dg-do compile }
+
+pragma Spark_Mode (On);
+
+with Predicate8_Pkg;
+
+procedure Predicate8 is
+   package Ring_Buffer is new Predicate8_Pkg (Element_Type => Integer);
+   use Ring_Buffer;
+
+   X : Ring_Buffer_Type (4);
+
+begin
+   Put (X, 1);
+end Predicate8;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/predicate8_pkg.adb
@@ -0,0 +1,64 @@
+pragma Spark_Mode (On);
+
+package body Predicate8_Pkg is
+   function Empty
+ (Buffer : in Ring_Buffer_Type) return Boolean
+   is (Size (Buffer) = 0);
+
+   function Full
+ (Buffer : in Ring_Buffer_Type) return Boolean
+   is (Size (Buffer) = Buffer.Max_Size);
+
+   function Size
+ (Buffer : in Ring_Buffer_Type) return Natural
+   is (Buffer.Count);
+
+   function Free
+ (Buffer : in Ring_Buffer_Type) return Natural
+   is (Buffer.Max_Size - Size (Buffer));
+
+   function First
+ (Buffer : in Ring_Buffer_Type) return Element_Type
+   is (Buffer.Items (Buffer.Head));
+
+   function Last
+ (Buffer : in Ring_Buffer_Type) return Element_Type
+   is (Buffer.Items (Buffer.Tail));
+
+   procedure Get
+ (Buffer   : in out Ring_Buffer_Type;
+  Element  :out Element_Type)
+   is
+   begin
+  Element := Buffer.Items (Buffer.Head);
+  Buffer  :=
+Buffer'Update
+  (Head  =>
+(if Buffer.Head = Buffer.Max_Size then 1 else Buffer.Head + 1),
+   Count => Buffer.Count - 1);
+   end Get;
+
+   procedure Put
+ (Buffer   : in out Ring_Buffer_Type;
+  Element  : in Element_Type)
+   is
+begin
+  if Buffer.Tail = Buffer.Max_Size then
+ Buffer.Tail := 1;
+  else
+ Buffer.Tail := Buffer.Tail + 1;
+  end if;
+
+  Buffer.Items (Buffer.Tail) := Element;
+  Buffer.Count := Buffer.Count + 1;
+   end Put;
+
+   procedure Clear
+ (Buffer : in out Ring_Buffer_Type)
+   is
+   begin
+  Buffer.Head  := 1;
+  Buffer.Tail  := Buffer.Max_Size;
+  Buffer.Count := 0;
+   end Clear;
+end Predicate8_Pkg;

--- /dev/null
new file mode 100644
+++ gcc/testsuite/gnat.dg/predicate8_pkg.ads
@@ -0,0 +1,81 @@
+pragma Spark_Mode (On);
+
+generic
+  type Element_Type is private;
+
+package Predicate8_Pkg is
+   pragma Annotate (GNATprove, Terminating, Predicate8_Pkg);
+
+   subtype Small_Natural  is Natural range 0 .. Natural'Last / 2;
+   subtype Small_Positive is Natural range 1 .. Natural'Last / 2;
+
+   type Element_Array_Type is array 

  1   2   >