[C++ PATCH] PR c++/92496 - ICE with <=> and no #include .

2019-12-12 Thread Jason Merrill
Normal error-recovery.

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

* typeck.c (cp_build_binary_op): Handle error from spaceship_type.
---
 gcc/cp/typeck.c   |  6 +-
 gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg3.C | 12 
 2 files changed, 17 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg3.C

diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index d0f739895c9..d3814585e3f 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -5418,7 +5418,11 @@ cp_build_binary_op (const op_location_t ,
result_type = NULL_TREE;
 
   if (result_type)
-   build_type = spaceship_type (result_type, complain);
+   {
+ build_type = spaceship_type (result_type, complain);
+ if (build_type == error_mark_node)
+   return error_mark_node;
+   }
 
   if (result_type && arithmetic_types_p)
{
diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg3.C 
b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg3.C
new file mode 100644
index 000..45ce4ee047a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg3.C
@@ -0,0 +1,12 @@
+// PR c++/92496
+// { dg-do compile { target c++2a } }
+
+template
+struct A {};
+
+struct B {
+constexpr auto operator<=>(const B&) const = default; // { dg-error "" }
+int value;
+};
+
+A t;

base-commit: 3dbaca45883c354c9d4e9542d5c95f054997f844
-- 
2.18.1



[PATCH PR92926]Fix wrong code caused by ctor node translation unit wide sharing

2019-12-12 Thread bin.cheng
Hi,

As reported in PR92926, constant ctor is shared translation unit wide because 
of constexpr_call_table,
however, during gimplify, the shared ctor could be modified.  This patch fixes 
the issue by unsharing
it before modification in gimplify.  A test is reduced from cppcoro library and 
added.

Bootstrap and test ongoing.  Not sure if this is the correct fix though, any 
comments?

Thanks,
bin

2019-12-13  Bin Cheng  

PR tree-optimization/92926
* gimplify.c (gimplify_init_constructor): Unshare ctor node before
clearing.

gcc/testsuite
2019-12-13  Bin Cheng  

PR tree-optimization/92926
* g++.dg/pr92926.C: New test.

0001-pr92926.patch
Description: Binary data


Re: [PATCH 11/49] Add diagnostic_metadata and CWE support

2019-12-12 Thread David Malcolm
On Wed, 2019-12-04 at 10:36 -0700, Martin Sebor wrote:
> On 11/15/19 6:22 PM, David Malcolm wrote:
> > This patch adds support for associating a diagnostic with an
> > optional
> > diagnostic_metadata object, so that plugins can add extra data to
> > their
> > diagnostics (e.g. mapping a diagnostic to a taxonomy or coding
> > standard
> > such as from CERT or MISRA).
> > 
> > Currently this only supports associating a CWE identifier with a
> > diagnostic (which is what I'm using for the analyzer warnings later
> > in
> > the patch kit), but adding a diagnostic_metadata class allows for
> > future
> > growth in this area without an explosion of further "warning_at"
> > overloads
> > for all of the different kinds of custom data that a plugin might
> > want to
> > add.
> 
> We discussed this in the past so I'll be repeating some of my
> comments from that thread.  I like the feature at a high level,
> but I'm not sure I see how to make it work in a robust way.
> 
> There is no one-to-one mapping between GCC warnings and any of
> these classifications.  A single GCC warning might map to several
> CERT or MISRA guidelines, and a single guideline might map to two
> or more different GCC warnings.  This is an M to N mapping times
> the number of coding standards/classifications.  I haven't looked
> at the patch kit in enough detail to understand how it tries to
> handle it.  Can you explain?

The patch kit gives the ability to (optionally) specify a single CWE
identifier when emitting a diagnostic.

The patch kit uses this in various places for its new warnings for the
cases where there is a well-defined CWE weakness identifier (and
doesn't otherwise).

It doesn't attempt to classify any existing warnings.

It doesn't attempt to support other classifications in this patch, but
it does introduce a diagnostic_metadata object that could gain this
functionality at some later time, if we come up with a scheme we're
happy with (a slight violation of YAGNI, but it's such a pain touching
all the diagnostic fns that I prefer to do it once).

> My other concern here is that these guidelines evolve.  New ones
> are being added, existing ones moved or broken up, etc.  Does
> the infrastructure cope with this evolution (e.g., by reading
> the mapping from a file)?

Let's cross that bridge when we come to it.  If it becomes a problem,
then we can come up with a fix (e.g. a command-line option to specify
the CWE version).

Dave

> Martin
> 
> > gcc/ChangeLog:
> > * common.opt (fdiagnostics-show-metadata): New option.
> > * diagnostic-core.h (class diagnostic_metadata): New forward
> > decl.
> > (warning_at): Add overload taking a const diagnostic_metadata
> > &.
> > (emit_diagnostic_valist): Add overload taking a
> > const diagnostic_metadata *.
> > * diagnostic-format-json.cc: Include "diagnostic-metadata.h".
> > (json_from_metadata): New function.
> > (json_end_diagnostic): Call it to add "metadata" child for
> > diagnostics with metadata.
> > (diagnostic_output_format_init): Clear context->show_metadata.
> > * diagnostic-metadata.h: New file.
> > * diagnostic.c: Include "diagnostic-metadata.h".
> > (diagnostic_impl): Add const diagnostic_metadata * param.
> > (diagnostic_n_impl): Likewise.
> > (diagnostic_initialize): Initialize context->show_metadata.
> > (diagnostic_set_info_translated): Initialize diagnostic-
> > >metadata.
> > (get_cwe_url): New function.
> > (print_any_metadata): New function.
> > (diagnostic_report_diagnostic): Call print_any_metadata if the
> > diagnostic has non-NULL metadata.
> > (emit_diagnostic): Pass NULL as the metadata in the call to
> > diagnostic_impl.
> > (emit_diagnostic_valist): Likewise.
> > (emit_diagnostic_valist): New overload taking a
> > const diagnostic_metadata *.
> > (inform): Pass NULL as the metadata in the call to
> > diagnostic_impl.
> > (inform_n): Likewise for diagnostic_n_impl.
> > (warning): Likewise.
> > (warning_at): Likewise.  Add overload that takes a
> > const diagnostic_metadata &.
> > (warning_n): Pass NULL as the metadata in the call to
> > diagnostic_n_impl.
> > (pedwarn): Likewise for diagnostic_impl.
> > (permerror): Likewise.
> > (error): Likewise.
> > (error_n): Likewise.
> > (error_at): Likewise.
> > (sorry): Likewise.
> > (sorry_at): Likewise.
> > (fatal_error): Likewise.
> > (internal_error): Likewise.
> > (internal_error_no_backtrace): Likewise.
> > * diagnostic.h (diagnostic_info::metadata): New field.
> > (diagnostic_context::show_metadata): New field.
> > * doc/invoke.texi (-fno-diagnostics-show-metadata): New option.
> > * opts.c (common_handle_option): Handle
> > OPT_fdiagnostics_show_metadata.
> > * toplev.c (general_init): Initialize global_dc->show_metadata.
> > ---
> >   gcc/common.opt|   4 ++
> >   gcc/diagnostic-core.h |  10 

Re: [PATCH] Fix x86_64 va_arg (ap, __int128) handling (PR target/92904)

2019-12-12 Thread Jan Hubicka
> Hi!
> 
> As the following testcase shows, for va_arg (ap, __int128) or va_arg (ap, X)
> where X is __attribute__((aligned (16))) 16 byte struct containing just
> integral fields we sometimes emit incorrect read.  While on the stack (i.e.
> the overflow area) both of these are 16-byte aligned, when __int128 or
> 16-byte aligned e.g. { long a, b; } struct are passed in registers,
> there is no alignment of it being only passed in even registers or something
> similar (correct, the ABI says that), but in the end this means that when we
> spill those GPRs into the gpr save area, the __int128 or aligned struct
> might be only 8-byte aligned there rather than 16-byte aligned.
> In the testcase, e.g. for f1 that isn't a problem, we just load the two
> registers separately as 64-bit loads, but in f4 or f6 as it is a copy from
> memory to memory, we actually optimize it using an aligned SSE load into an
> SSE register and store from there back to memory (again, aligned SSE load,
> that one is correct).
> The current va_arg expanded code just computes the address from which the
> __int128 etc. should be loaded, in two different branches and the load is
> then done in the joiner bb.
> So, what we could do is either move the loads from the joiner bb into the
> two predecessors of that bb, have the one for the case of read from gpr save
> area use 8-byte only aligned load and the one reading from overflow area
> with 16-byte alignment, but I'd say that would result in unnecessarily
> longer sequence, or we can as the patch just lower the alignment of the
> MEM_REF in the joiner bb, which will for the cases we used vmovdqa etc.
> just use vmovdqu instead, but will not grow code size and on contemporary
> CPUs if the address is actually aligned, it shouldn't be really slower.
> In most cases the __int128 is used anyway directly and in those cases likely
> will be loaded into two GRPs.

Agreed, movdqu is usually about as cheap as movdqa ;)
> 
> The patch does this only for the needed_intregs case and !need_temp,
> when need_temp, this kind of problem doesn't exist, the value is copied
> either using memcpy or just 64-bit loads + stores into the aligned
> temporary, and for needed_sseregs the slots in the fp save area are 16-byte
> aligned already, and 32-byte aligned structures just would have 32-byte size
> and thus would be passed in memory for ... args, no matter if they contain
> __mm256 or __mm512 or something else.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux and after a while
> release branches?
> 
> 2019-12-12  Jakub Jelinek  
> 
>   PR target/92904
>   * config/i386/i386.c (ix86_gimplify_va_arg): If need_intregs and
>   not need_temp, decrease alignment of the read because the GPR save
>   area only guarantees 8-byte alignment.
> 
>   * gcc.c-torture/execute/pr92904.c: New test.
OK,
thanks!
honza
> 
> --- gcc/config/i386/i386.c.jj 2019-12-10 10:01:08.384171578 +0100
> +++ gcc/config/i386/i386.c2019-12-12 13:57:38.584461843 +0100
> @@ -4277,6 +4277,7 @@ ix86_gimplify_va_arg (tree valist, tree
>tree ptrtype;
>machine_mode nat_mode;
>unsigned int arg_boundary;
> +  unsigned int type_align;
>  
>/* Only 64bit target needs something special.  */
>if (is_va_list_char_pointer (TREE_TYPE (valist)))
> @@ -4334,6 +4335,7 @@ ix86_gimplify_va_arg (tree valist, tree
>/* Pull the value out of the saved registers.  */
>  
>addr = create_tmp_var (ptr_type_node, "addr");
> +  type_align = TYPE_ALIGN (type);
>  
>if (container)
>  {
> @@ -4504,6 +4506,9 @@ ix86_gimplify_va_arg (tree valist, tree
> t = build2 (PLUS_EXPR, TREE_TYPE (gpr), gpr,
> build_int_cst (TREE_TYPE (gpr), needed_intregs * 8));
> gimplify_assign (gpr, t, pre_p);
> +   /* The GPR save area guarantees only 8-byte alignment.  */
> +   if (!need_temp)
> + type_align = MIN (type_align, 64);
>   }
>  
>if (needed_sseregs)
> @@ -4548,6 +4556,7 @@ ix86_gimplify_va_arg (tree valist, tree
>if (container)
>  gimple_seq_add_stmt (pre_p, gimple_build_label (lab_over));
>  
> +  type = build_aligned_type (type, type_align);
>ptrtype = build_pointer_type_for_mode (type, ptr_mode, true);
>addr = fold_convert (ptrtype, addr);
>  
> --- gcc/testsuite/gcc.c-torture/execute/pr92904.c.jj  2019-12-12 
> 15:04:59.203302591 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/pr92904.c 2019-12-12 
> 15:08:05.190449143 +0100
> @@ -0,0 +1,395 @@
> +/* PR target/92904 */
> +
> +#include 
> +
> +struct S { long long a, b; };
> +struct __attribute__((aligned (16))) T { long long a, b; };
> +struct U { double a, b, c, d; };
> +struct __attribute__((aligned (32))) V { double a, b, c, d; };
> +struct W { double a; long long b; };
> +struct __attribute__((aligned (16))) X { double a; long long b; };
> +#if __SIZEOF_INT128__ == 2 * __SIZEOF_LONG_LONG__
> +__int128 b;
> +#endif
> +struct S c;
> +struct T d;
> +struct U e;
> +struct V f;
> 

Re: [PATCH] Fix simplify-rtx.c handling of avx512 vector comparisons (PR target/92908)

2019-12-12 Thread Jakub Jelinek
On Thu, Dec 12, 2019 at 04:59:40PM +0100, Richard Biener wrote:
> >If it starts being ambiguous somewhere, could we use some target macro
> >or
> >target hook to decide? 
> 
> Ambiguous IL is bad :/  IL semantics dependent on a target hook, too. Just 
> look at SHIFT_COUNT_TRUNCATED... 

This is something I've also successfully bootstrapped/regtested on
x86_64-linux and i686-linux:

2019-12-11  Jakub Jelinek  

PR target/92908
* simplify-rtx.c (simplify_relational_operation): Punt for vector
cmp_mode and scalar mode, if simplify_relational_operation returned
const_true_rtx.
(simplify_const_relational_operation): Change VOID_mode in function
comment to VOIDmode.

* gcc.target/i386/avx512bw-pr92908.c: New test.

--- gcc/simplify-rtx.c.jj   2019-11-19 22:27:02.58742 +0100
+++ gcc/simplify-rtx.c  2019-12-11 13:31:57.197809704 +0100
@@ -5037,6 +5037,15 @@ simplify_relational_operation (enum rtx_
  return NULL_RTX;
 #endif
}
+  /* For vector comparison with scalar int result, it is unknown
+if the target means here a comparison into an integral bitmask,
+or comparison where all comparisons true mean const_true_rtx
+whole result, or where any comparisons true mean const_true_rtx
+whole result.  For const0_rtx all the cases are the same.  */
+  if (VECTOR_MODE_P (cmp_mode)
+ && SCALAR_INT_MODE_P (mode)
+ && tem == const_true_rtx)
+   return NULL_RTX;
 
   return tem;
 }
@@ -5383,7 +5392,7 @@ comparison_result (enum rtx_code code, i
 }
 
 /* Check if the given comparison (done in the given MODE) is actually
-   a tautology or a contradiction.  If the mode is VOID_mode, the
+   a tautology or a contradiction.  If the mode is VOIDmode, the
comparison is done in "infinite precision".  If no simplification
is possible, this function returns zero.  Otherwise, it returns
either const_true_rtx or const0_rtx.  */
--- gcc/testsuite/gcc.target/i386/avx512bw-pr92908.c.jj 2019-12-11 
14:24:12.083418762 +0100
+++ gcc/testsuite/gcc.target/i386/avx512bw-pr92908.c2019-12-11 
14:23:56.071665326 +0100
@@ -0,0 +1,21 @@
+/* PR target/92908 */
+/* { dg-do run } */
+/* { dg-options "-Og -fno-tree-fre -mavx512bw" } */
+/* { dg-require-effective-target avx512bw } */
+
+#define AVX512BW
+#include "avx512f-helper.h"
+
+typedef unsigned short V __attribute__ ((__vector_size__ (64)));
+
+V v;
+
+void
+TEST (void)
+{
+  int i;
+  v = (V) v == v;
+  for (i = 0; i < 32; i++)
+if (v[i] != 0x)
+  abort ();
+}


Jakub



[PATCH] Fix x86_64 va_arg (ap, __int128) handling (PR target/92904)

2019-12-12 Thread Jakub Jelinek
Hi!

As the following testcase shows, for va_arg (ap, __int128) or va_arg (ap, X)
where X is __attribute__((aligned (16))) 16 byte struct containing just
integral fields we sometimes emit incorrect read.  While on the stack (i.e.
the overflow area) both of these are 16-byte aligned, when __int128 or
16-byte aligned e.g. { long a, b; } struct are passed in registers,
there is no alignment of it being only passed in even registers or something
similar (correct, the ABI says that), but in the end this means that when we
spill those GPRs into the gpr save area, the __int128 or aligned struct
might be only 8-byte aligned there rather than 16-byte aligned.
In the testcase, e.g. for f1 that isn't a problem, we just load the two
registers separately as 64-bit loads, but in f4 or f6 as it is a copy from
memory to memory, we actually optimize it using an aligned SSE load into an
SSE register and store from there back to memory (again, aligned SSE load,
that one is correct).
The current va_arg expanded code just computes the address from which the
__int128 etc. should be loaded, in two different branches and the load is
then done in the joiner bb.
So, what we could do is either move the loads from the joiner bb into the
two predecessors of that bb, have the one for the case of read from gpr save
area use 8-byte only aligned load and the one reading from overflow area
with 16-byte alignment, but I'd say that would result in unnecessarily
longer sequence, or we can as the patch just lower the alignment of the
MEM_REF in the joiner bb, which will for the cases we used vmovdqa etc.
just use vmovdqu instead, but will not grow code size and on contemporary
CPUs if the address is actually aligned, it shouldn't be really slower.
In most cases the __int128 is used anyway directly and in those cases likely
will be loaded into two GRPs.

The patch does this only for the needed_intregs case and !need_temp,
when need_temp, this kind of problem doesn't exist, the value is copied
either using memcpy or just 64-bit loads + stores into the aligned
temporary, and for needed_sseregs the slots in the fp save area are 16-byte
aligned already, and 32-byte aligned structures just would have 32-byte size
and thus would be passed in memory for ... args, no matter if they contain
__mm256 or __mm512 or something else.

Bootstrapped/regtested on x86_64-linux and i686-linux and after a while
release branches?

2019-12-12  Jakub Jelinek  

PR target/92904
* config/i386/i386.c (ix86_gimplify_va_arg): If need_intregs and
not need_temp, decrease alignment of the read because the GPR save
area only guarantees 8-byte alignment.

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

--- gcc/config/i386/i386.c.jj   2019-12-10 10:01:08.384171578 +0100
+++ gcc/config/i386/i386.c  2019-12-12 13:57:38.584461843 +0100
@@ -4277,6 +4277,7 @@ ix86_gimplify_va_arg (tree valist, tree
   tree ptrtype;
   machine_mode nat_mode;
   unsigned int arg_boundary;
+  unsigned int type_align;
 
   /* Only 64bit target needs something special.  */
   if (is_va_list_char_pointer (TREE_TYPE (valist)))
@@ -4334,6 +4335,7 @@ ix86_gimplify_va_arg (tree valist, tree
   /* Pull the value out of the saved registers.  */
 
   addr = create_tmp_var (ptr_type_node, "addr");
+  type_align = TYPE_ALIGN (type);
 
   if (container)
 {
@@ -4504,6 +4506,9 @@ ix86_gimplify_va_arg (tree valist, tree
  t = build2 (PLUS_EXPR, TREE_TYPE (gpr), gpr,
  build_int_cst (TREE_TYPE (gpr), needed_intregs * 8));
  gimplify_assign (gpr, t, pre_p);
+ /* The GPR save area guarantees only 8-byte alignment.  */
+ if (!need_temp)
+   type_align = MIN (type_align, 64);
}
 
   if (needed_sseregs)
@@ -4548,6 +4556,7 @@ ix86_gimplify_va_arg (tree valist, tree
   if (container)
 gimple_seq_add_stmt (pre_p, gimple_build_label (lab_over));
 
+  type = build_aligned_type (type, type_align);
   ptrtype = build_pointer_type_for_mode (type, ptr_mode, true);
   addr = fold_convert (ptrtype, addr);
 
--- gcc/testsuite/gcc.c-torture/execute/pr92904.c.jj2019-12-12 
15:04:59.203302591 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr92904.c   2019-12-12 
15:08:05.190449143 +0100
@@ -0,0 +1,395 @@
+/* PR target/92904 */
+
+#include 
+
+struct S { long long a, b; };
+struct __attribute__((aligned (16))) T { long long a, b; };
+struct U { double a, b, c, d; };
+struct __attribute__((aligned (32))) V { double a, b, c, d; };
+struct W { double a; long long b; };
+struct __attribute__((aligned (16))) X { double a; long long b; };
+#if __SIZEOF_INT128__ == 2 * __SIZEOF_LONG_LONG__
+__int128 b;
+#endif
+struct S c;
+struct T d;
+struct U e;
+struct V f;
+struct W g;
+struct X h;
+
+#if __SIZEOF_INT128__ == 2 * __SIZEOF_LONG_LONG__
+__attribute__((noipa)) __int128
+f1 (int x, ...)
+{
+  __int128 r;
+  va_list ap;
+  va_start (ap, x);
+  while (x--)
+va_arg (ap, int);
+  r = va_arg (ap, __int128);
+  va_end 

[PATCH] Ensure colorization doesn't corrupt multibyte sequences in diagnostics

2019-12-12 Thread Lewis Hyatt
Hello-

In the original discussion of implementing UTF-8 identifiers
( https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67224#c26 ), I pointed out
that colorization would corrupt the appearance of certain diagnostics. For
example, this code, with -std=c99:

--
int ٩x;
--

Produces:

t2.cpp:1:5: error: extended character ٩ is not valid at the start of an 
identifier
1 | int ٩x;
  | ^

The diagnostic location contains only the first byte of the character, so
when colorization is enabled, the ANSI escapes are inserted in the middle
of the UTF-8 sequence and produce corrupted output on the terminal.

I feel like there are two separate issues here:

#1. diagnostic_show_locus() should be sure it will not corrupt output in
this way, regardless of what ranges it is given to work with.

#2. libcpp should probably generate a range that includes the whole UTF-8
character. Actually in other ways the range seems not ideal, for example
if an invalid character appears in the middle of the identifier, the
diagnostic still points to the first byte of the identifier.

The attached patch fixes #1. It's essentially a one-line change, plus a
new selftest. Would you please have a look at it sometime? bootstrap
and testsuite were done on linux x86-64.

Other questions that I have:

- I am not quite clear when a selftest is preferred vs a dejagnu test. In
  this case I stuck with the selftest because color diagnostics don't seem
  to work well with dg-error etc, and it didn't seem worth creating a new
  plugin-based test like g++.dg/plugin just for this. (I also considered
  using the existing g++.dg plugin, but it seems this test should run for
  gcc as well.)

- I wasn't sure if I should create a PR for an issue such as this, if
  there is already a patch readily available. And if I did create a PR,
  not sure if it's preferred to post the patch to gcc-patches, or as an
  attachment to the PR.

- Does it seem worth me looking into #2? I think the patch to address #1 is
  appropriate in any case, because it handles generically all potential
  cases where this may arise, but still perhaps the ranges coming out of
  libcpp could be improved?

Thanks...

-Lewis
gcc/ChangeLog:

2019-12-12  Lewis Hyatt  

* diagnostic-show-locus.c (layout::print_source_line): Do not emit
color codes in the middle of a UTF-8 sequence.
(test_one_liner_colorized_utf8): New test.
(test_diagnostic_show_locus_one_liner_utf8): Call the new test.
diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index c87603caf41..7385b8a1692 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -1482,6 +1482,8 @@ layout::print_source_line (linenum_type row, const char *line, int line_bytes,
   int last_non_ws = 0;
   for (int col_byte = 1 + x_offset_bytes; col_byte <= line_bytes; col_byte++)
 {
+  char c = *line;
+
   /* Assuming colorization is enabled for the caret and underline
 	 characters, we may also colorize the associated characters
 	 within the source line.
@@ -1493,8 +1495,13 @@ layout::print_source_line (linenum_type row, const char *line, int line_bytes,
 
 	 For frontends that only generate carets, we don't colorize the
 	 characters above them, since this would look strange (e.g.
-	 colorizing just the first character in a token).  */
-  if (m_colorize_source_p)
+	 colorizing just the first character in a token).
+
+	 We need to avoid inserting color codes ahead of UTF-8 continuation
+	 bytes to avoid corrupting the output, in case the location range
+	 points only to the first byte of a multibyte sequence.  */
+
+  if (m_colorize_source_p && (((unsigned int) c) & 0xC0) != 0x80)
 	{
 	  bool in_range_p;
 	  point_state state;
@@ -1507,7 +1514,6 @@ layout::print_source_line (linenum_type row, const char *line, int line_bytes,
 	  else
 	m_colorizer.set_normal_text ();
 	}
-  char c = *line;
   if (c == '\0' || c == '\t' || c == '\r')
 	c = ' ';
   if (c != ' ')
@@ -3836,6 +3842,27 @@ test_one_liner_labels_utf8 ()
   }
 }
 
+/* Make sure that colorization codes don't interrupt a multibyte
+   sequence, which would corrupt it.  */
+static void
+test_one_liner_colorized_utf8 ()
+{
+  test_diagnostic_context dc;
+  dc.colorize_source_p = true;
+  diagnostic_color_init (, DIAGNOSTICS_COLOR_YES);
+  const location_t pi = linemap_position_for_column (line_table, 12);
+  rich_location richloc (line_table, pi);
+  diagnostic_show_locus (, , DK_ERROR);
+
+  /* In order to avoid having the test depend on exactly how the colorization
+ was effected, just confirm there are two pi characters in the output.  */
+  const char *result = pp_formatted_text (dc.printer);
+  const char *null_term = result + strlen (result);
+  const char *first_pi = strstr (result, "\xcf\x80");
+  ASSERT_TRUE (first_pi && first_pi <= null_term - 2);
+  ASSERT_STR_CONTAINS (first_pi + 2, "\xcf\x80");
+}
+
 /* Run the various one-liner tests.  */
 
 static 

Re: [Patch, Fortran] PR92898 - [9/10 Regression] ICE in gfc_check_is_contiguous, at fortran/check.c:7157

2019-12-12 Thread Harald Anlauf
Hi Tobias,

> Gesendet: Donnerstag, 12. Dezember 2019 um 09:01 Uhr
> Von: "Tobias Burnus" 
> An: "Harald Anlauf" , "Thomas Koenig" 
> Cc: gfortran , gcc-patches 
> Betreff: Re: Aw: Re: [Patch, Fortran] PR92898 - [9/10 Regression] ICE in 
> gfc_check_is_contiguous, at fortran/check.c:7157
>
> Hi Harald,
> 
> let's add a LGTM or OK to this – the patch is rather obvious and Steve 
> explained how the now-removed check ended up in gfortran.

thanks for the clarification, and thanks for the quick review.

> Thanks for the patch!

Committed as svn rev.279314 (trunk) and 279315 (9-branch).

Harald

> Tobias
> 
> On 12/11/19 11:24 PM, Harald Anlauf wrote:
> > Hi Thomas,
> >
> >> Gesendet: Dienstag, 10. Dezember 2019 um 23:34 Uhr
> >> Von: "Thomas Koenig" 
> >> An: "Harald Anlauf" , gfortran , 
> >> gcc-patches 
> >> Betreff: Re: [Patch, Fortran] PR92898 - [9/10 Regression] ICE in 
> >> gfc_check_is_contiguous, at fortran/check.c:7157
> >>
> >> Hello Harald,
> >>
> >>> Index: gcc/fortran/check.c
> >>> ===
> >>> --- gcc/fortran/check.c   (Revision 279183)
> >>> +++ gcc/fortran/check.c   (Arbeitskopie)
> >>> @@ -7154,7 +7154,9 @@ bool
> >>>gfc_check_is_contiguous (gfc_expr *array)
> >>>{
> >>>  if (array->expr_type == EXPR_NULL
> >>> -  && array->symtree->n.sym->attr.pointer == 1)
> >>> +  && (!array->symtree ||
> >>> +   (array->symtree->n.sym &&
> >>> +array->symtree->n.sym->attr.pointer == 1)))
> >> I have to admit I do not understand the original code here, nor
> >> do I quite understand your fix.
> >>
> >> Is there any circumstance where array->expr_type == EXPR_NULL, but
> >> is_contiguous is valid?  What would go wrong if the other tests
> >> were removed?
> > Actually I do not know what the additional check was supposed to do.
> > Removing it does not seem to do any harm.  See below.
> >
> >>> Index: gcc/testsuite/gfortran.dg/pr91641.f90
> >>> ===
> >>> --- gcc/testsuite/gfortran.dg/pr91641.f90 (Revision 279183)
> >>> +++ gcc/testsuite/gfortran.dg/pr91641.f90 (Arbeitskopie)
> >>> @@ -1,7 +1,9 @@
> >>>! { dg-do compile }
> >>>! PR fortran/91641
> >>> -! Code conyributed by Gerhard Steinmetz
> >>> +! PR fortran/92898
> >>> +! Code contributed by Gerhard Steinmetz
> >>>program p
> >>>   real, pointer :: z(:)
> >>>   print *, is_contiguous (null(z))! { dg-error "shall be an 
> >>> associated" }
> >>> +   print *, is_contiguous (null()) ! { dg-error "shall be an 
> >>> associated" }
> >>>end
> >> Sometimes, it is necessary to change test cases, when error messages
> >> change.  If this is not the case, it is better to add new tests to
> >> new test cases - this makes regression hunting much easier.
> >>
> >> Regards
> >>
> >>Thomas
> > Agreed.  Please find the modified patches below.  OK for trunk / 9 ?
> >
> > Thanks,
> > Harald
> >
> > Index: gcc/fortran/check.c
> > ===
> > --- gcc/fortran/check.c (Revision 279254)
> > +++ gcc/fortran/check.c (Arbeitskopie)
> > @@ -7153,8 +7153,7 @@ gfc_check_ttynam_sub (gfc_expr *unit, gfc_expr *na
> >   bool
> >   gfc_check_is_contiguous (gfc_expr *array)
> >   {
> > -  if (array->expr_type == EXPR_NULL
> > -  && array->symtree->n.sym->attr.pointer == 1)
> > +  if (array->expr_type == EXPR_NULL)
> >   {
> > gfc_error ("Actual argument at %L of %qs intrinsic shall be an "
> >   "associated pointer", >where, 
> > gfc_current_intrinsic);
> >
> >
> >
> > Index: gcc/testsuite/gfortran.dg/pr92898.f90
> > ===
> > --- gcc/testsuite/gfortran.dg/pr92898.f90   (nicht existent)
> > +++ gcc/testsuite/gfortran.dg/pr92898.f90   (Arbeitskopie)
> > @@ -0,0 +1,6 @@
> > +! { dg-do compile }
> > +! PR fortran/92898
> > +! Code contributed by Gerhard Steinmetz
> > +program p
> > +  print *, is_contiguous (null()) ! { dg-error "shall be an 
> > associated" }
> > +end
> >
> >
> > 2019-12-11  Harald Anlauf  
> >
> > PR fortran/92898
> > * check.c (gfc_check_is_contiguous): Simplify check to handle
> > arbitrary NULL() argument.
> >
> > 2019-12-11  Harald Anlauf  
> >
> > PR fortran/92898
> > * gfortran.dg/pr92898.f90: New test.
> >
>


Re: C++ PATCH for c++/88337 - Implement P1327R1: Allow dynamic_cast in constexpr

2019-12-12 Thread Jason Merrill

On 12/11/19 5:50 PM, Marek Polacek wrote:

On Fri, Nov 22, 2019 at 04:11:53PM -0500, Jason Merrill wrote:

On 11/8/19 4:24 PM, Marek Polacek wrote:



2) [class.cdtor] says that when a dynamic_cast is used in a constructor or
destructor and the operand of the dynamic_cast refers to the object under
construction or destruction, this object is considered to be a most derived
object.


This means that during the 'tor the vtable pointer refers to the type_info
for that class and the offset-to-top is 0.  Can you use that?


I can't seem to: For e.g.

struct C : A, C2 { A *c = dynamic_cast(static_cast(this)); };

the object under construction is C, the call to __dynamic_cast will be
__dynamic_cast (SAVE_EXPR <&((struct C *) this)->D.2119>, &_ZTI2C2, &_ZTI1A, -2)
here, OBJ is f.D.2156.D.2119 and ctx->global->ctor_object is f.D.2156.  So OBJ
refers to the object under construction.

But I don't see C anywhere; CLASSTYPE_TYPEINFO_VAR of OBJTYPE is _ZTI2C2.

Am I looking into the wrong place?


Evaluating build_vfield_ref (obj, objtype) will give you the vtable 
pointer, in this case &_ZTV1C + 40.  And then you can get C from 
DECL_CONTEXT (_ZTV1C).


Or get_tinfo_decl_dynamic (obj) will give you the type_info.

Jason



[PATCH, OpenACC] Fix potential race condition in OpenACC "exit data" operations

2019-12-12 Thread Julian Brown
Hi,

This is a fix for PR92881, broken out of the larger "reference counting
overhaul" patch last posted here:

https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02235.html

The current implementation (potentially with synchronous unmapping of
a variable/memory block immediately after an asynchronous copyout which
might not complete beforehand) is "obviously incorrect" by observation,
but does not appear to cause any issues with the current testsuite
(i.e., there is no test improvement with this patch).

Tested with offloading to AMD GCN. OK for trunk?

Thanks,

Julian

ChangeLog

2019-12-12  Julian Brown  
Thomas Schwinge  

PR libgomp/92881

libgomp/
* libgomp.h (gomp_remove_var_async): Add prototype.
* oacc-mem.c (delete_copyout): Call gomp_remove_var_async instead of
gomp_remove_var.
* target.c (gomp_unref_tgt): Change return type to bool, indicating
whether target_mem_desc was unmapped.
(gomp_unref_tgt_void): New.
(gomp_remove_var): Reimplement in terms of...
(gomp_remove_var_internal): ...this new helper function.
(gomp_remove_var_async): New, implemented using above helper function.
(gomp_unmap_vars_internal): Use gomp_unref_tgt_void instead of
gomp_unref_tgt.
commit 7a5134adb474c65269bb16b1e31bf92bb9d6a06a
Author: Julian Brown 
Date:   Thu Dec 12 10:39:23 2019 -0800

Fix potential race condition in OpenACC "exit data" operations

PR libgomp/92881

libgomp/
* libgomp.h (gomp_remove_var_async): Add prototype.
* oacc-mem.c (delete_copyout): Call gomp_remove_var_async instead of
gomp_remove_var.
* target.c (gomp_unref_tgt): Change return type to bool, indicating
whether target_mem_desc was unmapped.
(gomp_unref_tgt_void): New.
(gomp_remove_var): Reimplement in terms of...
(gomp_remove_var_internal): ...this new helper function.
(gomp_remove_var_async): New, implemented using above helper function.
(gomp_unmap_vars_internal): Use gomp_unref_tgt_void instead of
gomp_unref_tgt.

diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index 8cbfe0b1f30..6390187dff5 100644
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -1166,6 +1166,8 @@ extern bool gomp_fini_device (struct gomp_device_descr *);
 extern void gomp_free_memmap (struct splay_tree_s *);
 extern void gomp_unload_device (struct gomp_device_descr *);
 extern bool gomp_remove_var (struct gomp_device_descr *, splay_tree_key);
+extern void gomp_remove_var_async (struct gomp_device_descr *, splay_tree_key,
+   struct goacc_asyncqueue *);
 extern bool gomp_offload_target_available_p (int);
 
 /* work.c */
diff --git a/libgomp/oacc-mem.c b/libgomp/oacc-mem.c
index a809d0495a6..196b7e2a520 100644
--- a/libgomp/oacc-mem.c
+++ b/libgomp/oacc-mem.c
@@ -660,7 +660,6 @@ static void
 delete_copyout (unsigned f, void *h, size_t s, int async, const char *libfnname)
 {
   splay_tree_key n;
-  void *d;
   struct goacc_thread *thr = goacc_thread ();
   struct gomp_device_descr *acc_dev = thr->dev;
 
@@ -689,9 +688,6 @@ delete_copyout (unsigned f, void *h, size_t s, int async, const char *libfnname)
   gomp_fatal ("[%p,%d] is not mapped", (void *)h, (int)s);
 }
 
-  d = (void *) (n->tgt->tgt_start + n->tgt_offset
-		+ (uintptr_t) h - n->host_start);
-
   if ((uintptr_t) h < n->host_start || (uintptr_t) h + s > n->host_end)
 {
   size_t host_size = n->host_end - n->host_start;
@@ -723,12 +719,15 @@ delete_copyout (unsigned f, void *h, size_t s, int async, const char *libfnname)
 
   if (n->refcount == 0)
 {
+  goacc_aq aq = get_goacc_asyncqueue (async);
+
   if (f & FLAG_COPYOUT)
 	{
-	  goacc_aq aq = get_goacc_asyncqueue (async);
+	  void *d = (void *) (n->tgt->tgt_start + n->tgt_offset
+			  + (uintptr_t) h - n->host_start);
 	  gomp_copy_dev2host (acc_dev, aq, h, d, s);
 	}
-  gomp_remove_var (acc_dev, n);
+  gomp_remove_var_async (acc_dev, n, aq);
 }
 
   gomp_mutex_unlock (_dev->lock);
diff --git a/libgomp/target.c b/libgomp/target.c
index d2fd90be539..d2990e06348 100644
--- a/libgomp/target.c
+++ b/libgomp/target.c
@@ -1128,32 +1128,65 @@ gomp_unmap_tgt (struct target_mem_desc *tgt)
   free (tgt);
 }
 
-attribute_hidden bool
-gomp_remove_var (struct gomp_device_descr *devicep, splay_tree_key k)
+static bool
+gomp_unref_tgt (void *ptr)
 {
   bool is_tgt_unmapped = false;
-  splay_tree_remove (>mem_map, k);
-  if (k->link_key)
-splay_tree_insert (>mem_map, (splay_tree_node) k->link_key);
-  if (k->tgt->refcount > 1)
-k->tgt->refcount--;
+
+  struct target_mem_desc *tgt = (struct target_mem_desc *) ptr;
+
+  assert (tgt->refcount != REFCOUNT_INFINITY);
+
+  if (tgt->refcount > 1)
+tgt->refcount--;
   else
 {
+  gomp_unmap_tgt (tgt);
   is_tgt_unmapped = true;
-  gomp_unmap_tgt (k->tgt);
 }
+
   return is_tgt_unmapped;
 }
 
 static void
-gomp_unref_tgt (void 

Guard inclusion of vxAtomicLib.h from gthr-vxworks.h

2019-12-12 Thread Olivier Hainque
Hello,

This is the first of a batch of changes for VxWorks, fixing
issues observed when activating libstdc++ and the associated
testing. Many of the issues are just plain bugs not (libstd)c++
specific; they are just more easily exposed in a c++ testing
context.

CC'ed Joel and Jerome on this starter one as they have been
involved in this activity very closely. I'll then cc individual
contributors on subsequent patches, in addition to listing them
in the ChangeLog entry headers.

Most changes were initially developed for gcc-8 based
toolchains.

They were first proposed, reorganized and adjusted in-house for
our gcc-9 based tree, then tested for powerpc-vxworks6 and aarch64-vxworks7
where we now have clean results on a few Ada oriented internal
testsuites and much improved results on gcc/g++/libstdc++ testsuites,
for a mix of RTP and kernel mode runs.

I then verified on mainline that I could build a powerpc-vxworks6
toolchain and that a couple of internal testuites pass with this
compiler as well.

--

This change guards the #inclusion of vxAtomicLib.h from
gthr-vxworks.h so it only takes place in the LIBGC2 context
where we need it.

This fixes build failures of libstdc++ when activated,
caused by uses of gthr-vxworks.h in c++98 compilation contexts
while vxAtomicLib.h for C++ requires C++ >= 11.

Olivier

2019-12-12  Olivier Hainque  

libgcc/
* config/gthr-vxworks.h: Guard #include vxAtomicLib.h
by IN_LIBGCC2.



0002-Guard-inclusion-of-vxAtomicLib.h-from-gthr-vxworks.h.patch
Description: Binary data


Re: [PATCH 43/49] analyzer: new file: exploded-graph.h

2019-12-12 Thread David Malcolm
On Thu, 2019-12-12 at 10:28 -0500, David Malcolm wrote:
> On Wed, 2019-12-11 at 13:04 -0700, Jeff Law wrote:
> > On Fri, 2019-11-15 at 20:23 -0500, David Malcolm wrote:

[...]

> > > +
> > > +
> > > //
> > > //
> > NIT.  Is there some reason you don't just use whitespace for these
> > kind
> > of vertical separators.  THe more I see them, the more they bug me,
> > probably because that's not been the style we've used for GCC.
> 
> I've been using them to highlight new "chapters" in a source file;
> typically there's another comment following them.  I'm happy to drop
> the one where there isn't a trailing comment, but, I'm hoping there's
> an acceptable way to have an "section-header"-style comment.
> 
> We have "^L" in a bunch of places, but my editor (emacs) doesn't do a
> great job of highlighting them.
> 
> There's some precedence for: e.g. 
> 
> gengtype.c:5094:/*** Manage input files.  **/
> 
> lto-section-in.c has e.g.:
> 
> /
> */
> /* Record renamings of static
> declarations   */
> /
> */
> 
> tree-vect-loop-manip.c has:
> 
> /
> *
>   Simple Loop Peeling Utilities
> 
>   Utilities to support loop peeling for vectorization purposes.
>  
> */
> 
> but I suspect given how sporadic these are that this stuff snuck past
> review.
> 
> Alternatively I can just drop these and retain the trailing comments,
> keeping their formatting.

I went ahead and did that, and the result is growing on me; sorry for
the noise.


Dave




Re: Add a compatible_vector_types_p target hook

2019-12-12 Thread Richard Sandiford
Richard Biener  writes:
> On December 12, 2019 5:44:25 PM GMT+01:00, Richard Sandiford 
>  wrote:
>>Richard Biener  writes:
>>> On December 12, 2019 4:10:33 PM GMT+01:00, Richard Sandiford
>> wrote:
One problem with adding an N-bit vector extension to an existing
architecture is to decide how N-bit vectors should be passed to
functions and returned from functions.  Allowing all N-bit vector
types to be passed in registers breaks backwards compatibility,
since N-bit vectors could be used (and emulated) before the vector
extension was added.  But always passing N-bit vectors on the
stack would be inefficient for things like vector libm functions.

For SVE we took the compromise position of predefining new SVE vector
types that are distinct from all existing vector types, including
GNU-style vectors.  The new types are passed and returned in an
efficient way while existing vector types are passed and returned
in the traditional way.  In the right circumstances, the two types
are inter-convertible.

The SVE types are created using:

  vectype = build_distinct_type_copy (vectype);
  SET_TYPE_STRUCTURAL_EQUALITY (vectype);
  TYPE_ARTIFICIAL (vectype) = 1;

The C frontend maintains this distinction, using VIEW_CONVERT_EXPR
to convert from one type to the other.  However, the distinction can
be lost during gimple, which treats two vector types with the same
mode, number of elements, and element type as equivalent.  And for
most targets that's the right thing to do.
>>>
>>> And why's that a problem? The difference appears only in the function
>>call ABI which is determined by the function signature rather than
>>types or modes of the actual arguments? 
>>
>>We use the type of the actual arguments when deciding how arguments
>>should be passed to functions:
>>
>>/* I counts args in order (to be) pushed; ARGPOS counts in order
>>written.  */
>>  for (argpos = 0; argpos < num_actuals; i--, argpos++)
>>{
>>  tree type = TREE_TYPE (args[i].tree_value);
>>  [...]
>>   /* See if this argument should be passed by invisible reference.  */
>>  function_arg_info arg (type, argpos < n_named_args);
>>
>>And it has to be that way for calls to unprototyped functions,
>>or for varargs.
>
> So even for varargs the passing is different? Also we have CALL_EXPR_FNTYPE 
> which you could populate specially even for unprototyped or varargs functions.
>
> I realize we now look at the type of values but you have to realize that 
> differences that are not relevant for values are discarded.  Artificially 
> preserving such non-real differences everywhere(!) while it only matters at 
> call boundaries doesn't look correct. 

But isn't this similar to the way that we preserve the difference
between:

  struct s1 { int i; };
  struct s2 { int i; };

?  They're the same value as far as the target machine is concerned,
but we preserve the difference for other reasons.

>>The AArch64 port emits an error if calls pass values of SVE type to an
>>unprototyped function.  To do that we need to know whether the value
>>really is an SVE type rathr than a plain vector.
>>
>>For varags the ABI is the same for 256 bits+.  But we'll have the
>>same problem there once we support -msve-vector-bits=128, since the
>>layout of SVE and Advanced SIMD vectors differ for big-endian.
>
> But then why don't you have different modes?

Yeah, true, modes will probably help for the Advanced SIMD/SVE
difference.  But from a vector value POV, a vector of 4 ints is a vector
of 4 ints, so even distinguishing based on the mode is artificial.

SVE is AFAIK the first target to have different modes for potentially
the "same" vector type, and I had to add new infrastructure to allow
targets to define multiple modes of the same size.  So the fact that
gimple distinguishes otherwise identical vectors based on mode is a
relatively recent thing.  AFAIK it just fell out in the wash rather
than being deliberately planned.  It happens to be convenient in this
context, but it hasn't been important until now.

The hook doesn't seem any worse than distinguishing based on the mode.
Another way to avoid this would have been to define separate SVE modes
for the predefined vectors.  The big downside of that is that we'd end
up doubling the number of SVE patterns.

Extra on-the-side metadata is going to be easy to drop accidentally,
and this is something we need for correctness rather than optimisation.

Thanks,
Richard


[Committed, testsuite] Fix PR92870

2019-12-12 Thread Sudakshina Das
Hi

With my recent commit, I added a test that is not passing on all 
targets. My change was valid for targets that have a vector/scalar 
shift/rotate optabs (optab that supports vector shifted by scalar).

Since it does not seem to be easy to find out which targets would 
support it, I am limiting the test to the target that I know pass.

Committed as obvious r279310.

gcc/testsuite/ChangeLog

2019-12-12  Sudakshina Das  

PR testsuite/92870
* gcc.dg/vect/vect-shift-5.c: Add target to scan-tree-dump.
diff --git a/gcc/testsuite/gcc.dg/vect/vect-shift-5.c b/gcc/testsuite/gcc.dg/vect/vect-shift-5.c
index c1fd4f2..68e517e 100644
--- a/gcc/testsuite/gcc.dg/vect/vect-shift-5.c
+++ b/gcc/testsuite/gcc.dg/vect/vect-shift-5.c
@@ -16,4 +16,7 @@ int foo (uint32_t arr[4][4])
 return (((uint16_t)sum) + ((uint32_t)sum >> 16)) >> 1;
 }
 
-/* { dg-final { scan-tree-dump {vectorizable_shift ===[\n\r][^\n]*prologue_cost = 0} "vect" } } */
+/* For a target that has a vector/scalar shift/rotate optab, check
+   that we are not adding the cost of creating a vector from the scalar
+   in the prologue.  */
+/* { dg-final { scan-tree-dump {vectorizable_shift ===[\n\r][^\n]*prologue_cost = 0} "vect" { target { aarch64*-*-* x86_64-*-* } } } } */


Re: AW: [PATCH] m68k architecture: support ccmode + lra

2019-12-12 Thread Stefan Franke

Am 2019-12-12 10:32, schrieb Richard Sandiford:

Stefan Franke  writes:

Am 2019-12-08 01:54, schrieb Oleg Endo:

On Tue, 2019-11-26 at 07:38 +0100, ste...@franke.ms wrote:

> On 11/21/19 10:30 AM, ste...@franke.ms wrote:
> > Hi there,
> >
> > here is mc68k's patch to switch the m68k architecture over to ccmode
> > and
> > lra. See https://github.com/mc68kghost/gcc 68k-ccmode branch.
>
> Bernd Schmidt posted a conversion of the m68k port to ccmode a couple
> weeks before yours.  We've already ACK'd it for installing onto the
> trunk.
>
> Jeff

To be honest:
- 8 days is hardly "a couple weeks before"
- ccmode is not the same as ccmode+lra

The paperwork for contributing to fsf is on the way and the repo at
https://github.com/mc68kghost/gcc got an update. Tests are not yet 
at

100%
(master branch fails too many tests) but it's closer to master 
branch

now.
The code is to 50% identical, a fair amount has swapped cmp/bcc, few
are a
tad worse and some yield surprisingly better code.



You can still submit patches for further improvements, like adding
support for LRA.  Now that the main CCmode conversion is on trunk and
has been confirmed and tested, it should be much easier for you to
pinpoint problems in your changes.

Cheers,
Oleg


The problems are in the gcc implementation

- the lra implementation is buggy
- the compare elimination can't handle parallel's containing a compare
- df-core considers parallel's containing a compare also as a USE
- some optimizations/mechanisms do only work if HAVE_CC0 is defined
- way more ...

And the current implementation is IMHO unusable for lra since it did 
not

introduce a CC register to track clobbering. So it's a dead end.


I'm not sure about this last bit.  LRA simply isn't set up to cope
with move instructions that could clobber the CC register, so on
targets like m68k, I think we'd want to hide any CC register until
after LRA anyway.  Having a CC register could be useful for post-RA
optimisation, but not having one shouldn't affect the choice of RA.



The lra bugs are not related to a CC register.

And it's no fun to realize that my ccmode+lra patch did not work because 
there are several other passes which yield buggy code.
So fixing my patch was mainly fixing other passes or the exiting m68k 
implementation. /duh




/cheers
Stefan


Re: [testsuite][arm] Remove xfail for vect-epilogues test

2019-12-12 Thread Rainer Orth
Hi Andre,

> gcc/testsuite/ChangeLog:
>
> 2019-12-12  Andre Vieira  
>
> * gcc.dg/vect/vect-epilogues.c: XFAIL for arm big endian.
> * lib/target-supports.exp (check_effective_target_arm_big_endian):
> New target selector.

as always, this needs documenting in sourcebuild.texi.

Rainer

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


Re: [PATCH][Arm] Enable CLI for Armv8.6-a: armv8.6-a, i8mm and bf16

2019-12-12 Thread Dennis Zhang
Hi all,

On 22/11/2019 14:33, Dennis Zhang wrote:
> Hi all,
> 
> This patch is part of a series adding support for Armv8.6-A features.
> It enables options including -march=armv8.6-a, +i8mm and +bf16.
> The +i8mm and +bf16 features are optional for Armv8.2-a and onward.
> Documents are at https://developer.arm.com/docs/ddi0596/latest
> 
> Regtested for arm-none-linux-gnueabi-armv8-a.
> 

This is an update to rebase the patch to the top.
Some issues are fixed according to the recent CLI patch for AArch64.
ChangeLog is updated as following:

gcc/ChangeLog:

2019-12-12  Dennis Zhang  

* config/arm/arm-c.c (arm_cpu_builtins): Define
__ARM_FEATURE_MATMUL_INT8, __ARM_FEATURE_BF16_VECTOR_ARITHMETIC,
__ARM_FEATURE_BF16_SCALAR_ARITHMETIC, and
__ARM_BF16_FORMAT_ALTERNATIVE when enabled.
* config/arm/arm-cpus.in (armv8_6, i8mm, bf16): New features.
* config/arm/arm-tables.opt: Regenerated.
* config/arm/arm.c (arm_option_reconfigure_globals): Initialize
arm_arch_i8mm and arm_arch_bf16 when enabled.
* config/arm/arm.h (TARGET_I8MM): New macro.
(TARGET_BF16_FP, TARGET_BF16_SIMD): Likewise.
* config/arm/t-aprofile: Add matching rules for -march=armv8.6-a.
* config/arm/t-arm-elf (all_v8_archs): Add armv8.6-a.
* config/arm/t-multilib: Add matching rules for -march=armv8.6-a.
(v8_6_a_simd_variants): New.
(v8_*_a_simd_variants): Add i8mm and bf16.
* doc/invoke.texi (armv8.6-a, i8mm, bf16): Document new options.

gcc/testsuite/ChangeLog:

2019-12-12  Dennis Zhang  

* gcc.target/arm/multilib.exp: Add combination tests for armv8.6-a.

Is it OK for trunk?

Many thanks!
Dennis
diff --git a/gcc/config/arm/arm-c.c b/gcc/config/arm/arm-c.c
index 546b35a5cbd..9cd1c5bdcba 100644
--- a/gcc/config/arm/arm-c.c
+++ b/gcc/config/arm/arm-c.c
@@ -226,6 +226,14 @@ arm_cpu_builtins (struct cpp_reader* pfile)
 
   builtin_define_with_int_value ("__ARM_FEATURE_COPROC", coproc_level);
 }
+
+  def_or_undef_macro (pfile, "__ARM_FEATURE_MATMUL_INT8", TARGET_I8MM);
+  def_or_undef_macro (pfile, "__ARM_FEATURE_BF16_SCALAR_ARITHMETIC",
+		  TARGET_BF16_FP);
+  def_or_undef_macro (pfile, "__ARM_FEATURE_BF16_VECTOR_ARITHMETIC",
+		  TARGET_BF16_SIMD);
+  def_or_undef_macro (pfile, "__ARM_BF16_FORMAT_ALTERNATIVE",
+		  TARGET_BF16_FP || TARGET_BF16_SIMD);
 }
 
 void
diff --git a/gcc/config/arm/arm-cpus.in b/gcc/config/arm/arm-cpus.in
index 7090775aa7e..a2f6ce00af4 100644
--- a/gcc/config/arm/arm-cpus.in
+++ b/gcc/config/arm/arm-cpus.in
@@ -123,6 +123,9 @@ define feature armv8_4
 # Architecture rel 8.5.
 define feature armv8_5
 
+# Architecture rel 8.6.
+define feature armv8_6
+
 # M-Profile security extensions.
 define feature cmse
 
@@ -191,6 +194,12 @@ define feature sb
 # v8-A architectures, added by default from v8.5-A
 define feature predres
 
+# 8-bit Integer Matrix Multiply extension. Optional from v8.2-A.
+define feature i8mm
+
+# Brain half-precision floating-point extension. Optional from v8.2-A.
+define feature bf16
+
 # Feature groups.  Conventionally all (or mostly) upper case.
 # ALL_FPU lists all the feature bits associated with the floating-point
 # unit; these will all be removed if the floating-point unit is disabled
@@ -213,7 +222,7 @@ define fgroup ALL_CRYPTO	crypto
 # strip off 32 D-registers, but does not remove support for
 # double-precision FP.
 define fgroup ALL_SIMD_INTERNAL	fp_d32 neon ALL_CRYPTO
-define fgroup ALL_SIMD_EXTERNAL dotprod fp16fml
+define fgroup ALL_SIMD_EXTERNAL dotprod fp16fml i8mm
 define fgroup ALL_SIMD	ALL_SIMD_INTERNAL ALL_SIMD_EXTERNAL
 
 # List of all FPU bits to strip out if -mfpu is used to override the
@@ -221,7 +230,7 @@ define fgroup ALL_SIMD	ALL_SIMD_INTERNAL ALL_SIMD_EXTERNAL
 define fgroup ALL_FPU_INTERNAL	vfpv2 vfpv3 vfpv4 fpv5 fp16conv fp_dbl ALL_SIMD_INTERNAL
 # Similarly, but including fp16 and other extensions that aren't part of
 # -mfpu support.
-define fgroup ALL_FPU_EXTERNAL fp16
+define fgroup ALL_FPU_EXTERNAL fp16 bf16
 
 # Everything related to the FPU extensions (FP or SIMD).
 define fgroup ALL_FP	ALL_FPU_EXTERNAL ALL_FPU_INTERNAL ALL_SIMD
@@ -256,6 +265,7 @@ define fgroup ARMv8_2aARMv8_1a armv8_2
 define fgroup ARMv8_3aARMv8_2a armv8_3
 define fgroup ARMv8_4aARMv8_3a armv8_4
 define fgroup ARMv8_5aARMv8_4a armv8_5 sb predres
+define fgroup ARMv8_6aARMv8_5a armv8_6
 define fgroup ARMv8m_base ARMv6m armv8 cmse tdiv
 define fgroup ARMv8m_main ARMv7m armv8 cmse
 define fgroup ARMv8r  ARMv8a
@@ -563,6 +573,8 @@ begin arch armv8.2-a
  option dotprod add FP_ARMv8 DOTPROD
  option sb add sb
  option predres add predres
+ option i8mm add i8mm FP_ARMv8 NEON
+ option bf16 add bf16 FP_ARMv8 NEON
 end arch armv8.2-a
 
 begin arch armv8.3-a
@@ -580,6 +592,8 @@ begin arch armv8.3-a
  option dotprod add FP_ARMv8 DOTPROD
  option sb add sb
  option predres add predres
+ option i8mm add i8mm FP_ARMv8 NEON
+ option bf16 add 

Re: Add a compatible_vector_types_p target hook

2019-12-12 Thread Richard Biener
On December 12, 2019 5:44:25 PM GMT+01:00, Richard Sandiford 
 wrote:
>Richard Biener  writes:
>> On December 12, 2019 4:10:33 PM GMT+01:00, Richard Sandiford
> wrote:
>>>One problem with adding an N-bit vector extension to an existing
>>>architecture is to decide how N-bit vectors should be passed to
>>>functions and returned from functions.  Allowing all N-bit vector
>>>types to be passed in registers breaks backwards compatibility,
>>>since N-bit vectors could be used (and emulated) before the vector
>>>extension was added.  But always passing N-bit vectors on the
>>>stack would be inefficient for things like vector libm functions.
>>>
>>>For SVE we took the compromise position of predefining new SVE vector
>>>types that are distinct from all existing vector types, including
>>>GNU-style vectors.  The new types are passed and returned in an
>>>efficient way while existing vector types are passed and returned
>>>in the traditional way.  In the right circumstances, the two types
>>>are inter-convertible.
>>>
>>>The SVE types are created using:
>>>
>>>  vectype = build_distinct_type_copy (vectype);
>>>  SET_TYPE_STRUCTURAL_EQUALITY (vectype);
>>>  TYPE_ARTIFICIAL (vectype) = 1;
>>>
>>>The C frontend maintains this distinction, using VIEW_CONVERT_EXPR
>>>to convert from one type to the other.  However, the distinction can
>>>be lost during gimple, which treats two vector types with the same
>>>mode, number of elements, and element type as equivalent.  And for
>>>most targets that's the right thing to do.
>>
>> And why's that a problem? The difference appears only in the function
>call ABI which is determined by the function signature rather than
>types or modes of the actual arguments? 
>
>We use the type of the actual arguments when deciding how arguments
>should be passed to functions:
>
>/* I counts args in order (to be) pushed; ARGPOS counts in order
>written.  */
>  for (argpos = 0; argpos < num_actuals; i--, argpos++)
>{
>  tree type = TREE_TYPE (args[i].tree_value);
>  [...]
>   /* See if this argument should be passed by invisible reference.  */
>  function_arg_info arg (type, argpos < n_named_args);
>
>And it has to be that way for calls to unprototyped functions,
>or for varargs.

So even for varargs the passing is different? Also we have CALL_EXPR_FNTYPE 
which you could populate specially even for unprototyped or varargs functions.

I realize we now look at the type of values but you have to realize that 
differences that are not relevant for values are discarded.  Artificially 
preserving such non-real differences everywhere(!) while it only matters at 
call boundaries doesn't look correct. 

>The AArch64 port emits an error if calls pass values of SVE type to an
>unprototyped function.  To do that we need to know whether the value
>really is an SVE type rathr than a plain vector.
>
>For varags the ABI is the same for 256 bits+.  But we'll have the
>same problem there once we support -msve-vector-bits=128, since the
>layout of SVE and Advanced SIMD vectors differ for big-endian.

But then why don't you have different modes?

Richard. 

>Thanks,
>Richard
>
>>
>> Richard. 
>>
>>>This patch therefore adds a hook that lets the target choose
>>>whether such vector types are indeed equivalent.
>>>
>>>Note that the new tests fail for -mabi=ilp32 in the same way as other
>>>ACLE-based tests.  I'm still planning to fix that as a follow-on.
>>>
>>>Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>>>
>>>Richard
>>>
>>>
>>>2019-12-12  Richard Sandiford  
>>>
>>>gcc/
>>> * target.def (compatible_vector_types_p): New target hook.
>>> * hooks.h (hook_bool_const_tree_const_tree_true): Declare.
>>> * hooks.c (hook_bool_const_tree_const_tree_true): New function.
>>> * doc/tm.texi.in (TARGET_COMPATIBLE_VECTOR_TYPES_P): New hook.
>>> * doc/tm.texi: Regenerate.
>>> * gimple-expr.c: Include target.h.
>>> (useless_type_conversion_p): Use targetm.compatible_vector_types_p.
>>> * config/aarch64/aarch64.c (aarch64_compatible_vector_types_p): New
>>> function.
>>> (TARGET_COMPATIBLE_VECTOR_TYPES_P): Define.
>>> * config/aarch64/aarch64-sve-builtins.cc
>>>(gimple_folder::convert_pred):
>>> Use the original predicate if it already has a suitable type.
>>>
>>>gcc/testsuite/
>>> * gcc.target/aarch64/sve/pcs/gnu_vectors_1.c: New test.
>>> * gcc.target/aarch64/sve/pcs/gnu_vectors_2.c: Likewise.
>>>
>>>Index: gcc/target.def
>>>===
>>>--- gcc/target.def   2019-11-30 18:48:18.531984101 +
>>>+++ gcc/target.def   2019-12-12 15:07:43.960415368 +
>>>@@ -3411,6 +3411,29 @@ must have move patterns for this mode.",
>>>  hook_bool_mode_false)
>>> 
>>> DEFHOOK
>>>+(compatible_vector_types_p,
>>>+ "Return true if there is no target-specific reason for treating\n\
>>>+vector types @var{type1} and @var{type2} as distinct types.  The
>>>caller\n\
>>>+has already checked 

[avr,committed] Add support for some avrxmega3 devices.

2019-12-12 Thread Georg-Johann Lay

Applied this patchlet to add support for:

ATtiny1604, ATtiny1606, ATtiny1607, ATtiny402, ATtiny404, ATtiny406, 
ATtiny804, ATtiny806, ATtiny807, ATtiny202, ATtiny204.


Johann

Add support for some more AVR devices from avrxmega3 family.

* config/avr/avr-mcus.def (attiny1604, attiny1606, attiny1607)
(attiny402, attiny404, attiny406)
(attiny804, attiny806, attiny807)
(attiny202, attiny204): Add AVR_MCU lines to support them.
* doc/avr-mmcu.texi: Regenerate.

Index: config/avr/avr-mcus.def
===
--- config/avr/avr-mcus.def	(revision 279308)
+++ config/avr/avr-mcus.def	(revision 279309)
@@ -307,6 +307,17 @@ AVR_MCU ("atxmega32c4",  ARCH_AVRXME
 AVR_MCU ("atxmega32e5",  ARCH_AVRXMEGA2, AVR_ISA_NONE, "__AVR_ATxmega32E5__",  0x2000, 0x0, 0x9000, 0)
 /* Xmega, Flash + RAM < 64K, flash visible in RAM address space */
 AVR_MCU ("avrxmega3",ARCH_AVRXMEGA3, AVR_ISA_NONE,  NULL,  0x3f00, 0x0, 0x8000, 0)
+AVR_MCU ("attiny202",ARCH_AVRXMEGA3, AVR_ISA_RCALL, "__AVR_ATtiny202__",   0x3f80, 0x0, 0x800,  0x8000)
+AVR_MCU ("attiny204",ARCH_AVRXMEGA3, AVR_ISA_RCALL, "__AVR_ATtiny204__",   0x3f80, 0x0, 0x800,  0x8000)
+AVR_MCU ("attiny402",ARCH_AVRXMEGA3, AVR_ISA_RCALL, "__AVR_ATtiny402__",   0x3f00, 0x0, 0x1000, 0x8000)
+AVR_MCU ("attiny404",ARCH_AVRXMEGA3, AVR_ISA_RCALL, "__AVR_ATtiny404__",   0x3f00, 0x0, 0x1000, 0x8000)
+AVR_MCU ("attiny406",ARCH_AVRXMEGA3, AVR_ISA_RCALL, "__AVR_ATtiny406__",   0x3f00, 0x0, 0x1000, 0x8000)
+AVR_MCU ("attiny804",ARCH_AVRXMEGA3, AVR_ISA_RCALL, "__AVR_ATtiny804__",   0x3e00, 0x0, 0x2000, 0x8000)
+AVR_MCU ("attiny806",ARCH_AVRXMEGA3, AVR_ISA_RCALL, "__AVR_ATtiny806__",   0x3e00, 0x0, 0x2000, 0x8000)
+AVR_MCU ("attiny807",ARCH_AVRXMEGA3, AVR_ISA_RCALL, "__AVR_ATtiny807__",   0x3e00, 0x0, 0x2000, 0x8000)
+AVR_MCU ("attiny1604",   ARCH_AVRXMEGA3, AVR_ISA_NONE,  "__AVR_ATtiny1604__",  0x3c00, 0x0, 0x4000, 0x8000)
+AVR_MCU ("attiny1606",   ARCH_AVRXMEGA3, AVR_ISA_NONE,  "__AVR_ATtiny1606__",  0x3c00, 0x0, 0x4000, 0x8000)
+AVR_MCU ("attiny1607",   ARCH_AVRXMEGA3, AVR_ISA_NONE,  "__AVR_ATtiny1607__",  0x3c00, 0x0, 0x4000, 0x8000)
 AVR_MCU ("attiny212",ARCH_AVRXMEGA3, AVR_ISA_RCALL, "__AVR_ATtiny212__",   0x3f80, 0x0, 0x800,  0x8000)
 AVR_MCU ("attiny214",ARCH_AVRXMEGA3, AVR_ISA_RCALL, "__AVR_ATtiny214__",   0x3f80, 0x0, 0x800,  0x8000)
 AVR_MCU ("attiny412",ARCH_AVRXMEGA3, AVR_ISA_RCALL, "__AVR_ATtiny412__",   0x3f00, 0x0, 0x1000, 0x8000)
Index: doc/avr-mmcu.texi
===
--- doc/avr-mmcu.texi	(revision 279308)
+++ doc/avr-mmcu.texi	(revision 279309)
@@ -54,7 +54,7 @@
 
 @item avrxmega3
 ``XMEGA'' devices with up to 64@tie{}KiB of combined program memory and RAM, and with program memory visible in the RAM address space.
-@*@var{mcu}@tie{}= @code{attiny212}, @code{attiny214}, @code{attiny412}, @code{attiny414}, @code{attiny416}, @code{attiny417}, @code{attiny814}, @code{attiny816}, @code{attiny817}, @code{attiny1614}, @code{attiny1616}, @code{attiny1617}, @code{attiny3214}, @code{attiny3216}, @code{attiny3217}, @code{atmega808}, @code{atmega809}, @code{atmega1608}, @code{atmega1609}, @code{atmega3208}, @code{atmega3209}, @code{atmega4808}, @code{atmega4809}.
+@*@var{mcu}@tie{}= @code{attiny202}, @code{attiny204}, @code{attiny212}, @code{attiny214}, @code{attiny402}, @code{attiny404}, @code{attiny406}, @code{attiny412}, @code{attiny414}, @code{attiny416}, @code{attiny417}, @code{attiny804}, @code{attiny806}, @code{attiny807}, @code{attiny814}, @code{attiny816}, @code{attiny817}, @code{attiny1604}, @code{attiny1606}, @code{attiny1607}, @code{attiny1614}, @code{attiny1616}, @code{attiny1617}, @code{attiny3214}, @code{attiny3216}, @code{attiny3217}, @code{atmega808}, @code{atmega809}, @code{atmega1608}, @code{atmega1609}, @code{atmega3208}, @code{atmega3209}, @code{atmega4808}, @code{atmega4809}.
 
 @item avrxmega4
 ``XMEGA'' devices with more than 64@tie{}KiB and up to 128@tie{}KiB of program memory.


Re: [testsuite][arm] Remove xfail for vect-epilogues test

2019-12-12 Thread Andre Vieira (lists)

Yeah didn't test that, thanks.


This OK?

gcc/testsuite/ChangeLog:

2019-12-12  Andre Vieira  

* gcc.dg/vect/vect-epilogues.c: XFAIL for arm big endian.
* lib/target-supports.exp (check_effective_target_arm_big_endian):
New target selector.

On 12/12/2019 16:42, Christophe Lyon wrote:

On Wed, 11 Dec 2019 at 12:27, Andre Vieira (lists)
 wrote:


Hi,

We can now vectorize an epilogue for this loop for arm too, so removing
xfail.

Is this OK for trunk? Wasn't entirely sure whether I could commit this
under obvious.



This fails on armeb :-(


gcc/testsuite/ChangeLog:
2019-12-11  Andre Vieira  

  * gcc.dg/vect/vect-epilogues.c: Remove xfail for arm.
diff --git a/gcc/testsuite/gcc.dg/vect/vect-epilogues.c b/gcc/testsuite/gcc.dg/vect/vect-epilogues.c
index de95310a65eed78e1f75c4cd7581f9f7a86afd16..90ebe69a8200225b7b000c378cdd1e99add09eea 100644
--- a/gcc/testsuite/gcc.dg/vect/vect-epilogues.c
+++ b/gcc/testsuite/gcc.dg/vect/vect-epilogues.c
@@ -16,4 +16,4 @@ void pixel_avg( unsigned char *dst, int i_dst_stride,
  }
  }
 
-/* { dg-final { scan-tree-dump "LOOP EPILOGUE VECTORIZED" "vect" } } */
+/* { dg-final { scan-tree-dump "LOOP EPILOGUE VECTORIZED" "vect" { xfail { arm_big_endian } } } }  */
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 5b4cc02f9219ed8cfa329f97732abffca677883c..6ded35500b807eb831617386bae4267bfcd11f3c 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -3467,6 +3467,19 @@ proc check_effective_target_arm_nothumb { } {
 }]
 }
 
+# Return 1 if this is a big-endian ARM target
+proc check_effective_target_arm_big_endian { } {
+if { ![istarget arm*-*-*] } {
+	return 0
+}
+
+return ![check_no_compiler_messages arm_little_endian assembly {
+	#if !defined(__arm__) || !defined(__ARMEL__)
+	#error !__arm__ || !__ARMEL__
+	#endif
+}]
+}
+
 # Return 1 if this is a little-endian ARM target
 proc check_effective_target_arm_little_endian { } {
 if { ![istarget arm*-*-*] } {


Re: [PATCH][AArch64] Enable CLI for Armv8.6-a: armv8.6-a, i8mm and bf16

2019-12-12 Thread Dennis Zhang
Hi Richard,

On 06/12/2019 10:22, Richard Sandiford wrote:
> Dennis Zhang  writes:
>> 2019-12-04  Dennis Zhang  
>>
>>  * config/aarch64/aarch64-arches.def (armv8.6-a): New.
>>  * config/aarch64/aarch64-c.c (aarch64_update_cpp_builtins): Define
>>  __ARM_FEATURE_MATMUL_INT8, __ARM_FEATURE_BF16_VECTOR_ARITHMETIC and
>>  __ARM_FEATURE_BF16_SCALAR_ARITHMETIC when enabled.
>>  * config/aarch64/aarch64-option-extensions.def (i8mm, bf16): New.
>>  (fp): Disabling fp also disables i8mm and bf16.
>>  (simd): Disabling simd also disables i8mm.
>>  * config/aarch64/aarch64.h (AARCH64_FL_V8_6): New macro.
>>  (AARCH64_FL_I8MM, AARCH64_FL_BF16, AARCH64_FL_FOR_ARCH8_6): Likewise.
>>  (AARCH64_ISA_V8_6, AARCH64_ISA_I8MM, AARCH64_ISA_BF16): Likewise.
>>  (TARGET_I8MM, TARGET_BF16_FP, TARGET_BF16_SIMD): Likewise.
>>  * doc/invoke.texi (armv8.6-a, i8mm, bf16): Document new options. Add
>>  a new table to list permissible values for ARCH.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2019-12-04  Dennis Zhang  
>>
>>  * gcc.target/aarch64/pragma_cpp_predefs_2.c: Add tests for i8mm
>>  and bf16 features.
> 
> Thanks for the update, looks great.  A couple of comments below.
> 
>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>> index d165f31a865..1192e8f4b06 100644
>> --- a/gcc/doc/invoke.texi
>> +++ b/gcc/doc/invoke.texi
>> @@ -16050,25 +16050,22 @@ Specify the name of the target architecture and, 
>> optionally, one or
>>   more feature modifiers.  This option has the form
>>   @option{-march=@var{arch}@r{@{}+@r{[}no@r{]}@var{feature}@r{@}*}}.
>>   
>> -The permissible values for @var{arch} are @samp{armv8-a},
>> -@samp{armv8.1-a}, @samp{armv8.2-a}, @samp{armv8.3-a}, @samp{armv8.4-a},
>> -@samp{armv8.5-a} or @var{native}.
>> -
>> -The value @samp{armv8.5-a} implies @samp{armv8.4-a} and enables compiler
>> -support for the ARMv8.5-A architecture extensions.
>> -
>> -The value @samp{armv8.4-a} implies @samp{armv8.3-a} and enables compiler
>> -support for the ARMv8.4-A architecture extensions.
>> -
>> -The value @samp{armv8.3-a} implies @samp{armv8.2-a} and enables compiler
>> -support for the ARMv8.3-A architecture extensions.
>> -
>> -The value @samp{armv8.2-a} implies @samp{armv8.1-a} and enables compiler
>> -support for the ARMv8.2-A architecture extensions.
>> -
>> -The value @samp{armv8.1-a} implies @samp{armv8-a} and enables compiler
>> -support for the ARMv8.1-A architecture extension.  In particular, it
>> -enables the @samp{+crc}, @samp{+lse}, and @samp{+rdma} features.
>> +The table below summarizes the permissible values for @var{arch}
>> +and the features that they enable by default:
>> +
>> +@multitable @columnfractions 0.20 0.20 0.60
>> +@headitem @var{arch} value @tab Architecture @tab Includes by default
> 
> We should have an armv8-a entry here, something like:
> 
> @item @samp{armv8-a} @tab Armv8-A @tab @samp{+fp}, @samp{+simd}
> 

The armv8-a entry is added.

>> +@item @samp{armv8.1-a} @tab Armv8.1-A
>> +@tab @samp{armv8-a}, @samp{+crc}, @samp{+lse}, @samp{+rdma}
>> +@item @samp{armv8.2-a} @tab Armv8.2-A @tab @samp{armv8.1-a}
>> +@item @samp{armv8.3-a} @tab Armv8.3-A @tab @samp{armv8.2-a}
>> +@item @samp{armv8.4-a} @tab Armv8.4-A
>> +@tab @samp{armv8.3-a}, @samp{+fp16fml}, @samp{+dotprod}
>> +@item @samp{armv8.5-a} @tab Armv8.5-A
>> +@tab @samp{armv8.4-a}, @samp{+sb}, @samp{+ssbs}, @samp{+predres}
>> +@item @samp{armv8.6-a} @tab Armv8.6-A
>> +@tab @samp{armv8.5-a}, @samp{+bf16}, @samp{+i8mm}
>> +@end multitable
> 
> I should have tried a proof of concept of this before suggesting it, sorry.
> Trying the patch locally I get:
> 
> gcc.pod around line 18643: You can't have =items (as at line 18649) unless 
> the first thing after the =over is an =item
> POD document had syntax errors at /usr/bin/pod2man line 71.
> Makefile:3363: recipe for target 'doc/gcc.1' failed
> make: [doc/gcc.1] Error 1 (ignored)
> 
> (Odd that this is an ignored error, since we end up with an empty man page.)
> 
> I've posted a texi2pod.pl patch for that:
> 
>  https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00407.html
> 
> However, even with that patch, the script needs the full table row to be
> on a single line, so I think we need to do that and live with the long lines.
> 

The items are kept in a single line for each.

>> [...]
>> diff --git a/gcc/testsuite/gcc.target/aarch64/pragma_cpp_predefs_2.c 
>> b/gcc/testsuite/gcc.target/aarch64/pragma_cpp_predefs_2.c
>> index 608b89d19ce..5ae39bc6cf0 100644
>> --- a/gcc/testsuite/gcc.target/aarch64/pragma_cpp_predefs_2.c
>> +++ b/gcc/testsuite/gcc.target/aarch64/pragma_cpp_predefs_2.c
>> @@ -13,6 +13,92 @@
>>   #error "__ARM_FEATURE_TME is defined but should not be!"
>>   #endif
>>   
>> +/* Test Armv8.6-a features.  */
>> +
>> +#pragma GCC push_options
>> +#pragma GCC target ("arch=armv8-a")
> 
> These two pragmas should be at the beginning of the file, so that we
> start with base armv8-a for all the tests.

The pragmas are 

Re: [PATCH 3/X] [libsanitizer] Add option to bootstrap using HWASAN

2019-12-12 Thread Kyrill Tkachov

Hi Matthew,

Martin is the authority on this but I have a small comment inline...

On 12/12/19 3:19 PM, Matthew Malcomson wrote:

This is an analogous option to --bootstrap-asan to configure.  It allows
bootstrapping GCC using HWASAN.

For the same reasons as for ASAN we have to avoid using the HWASAN
sanitizer when compiling libiberty and the lto-plugin.

Also add a function to query whether -fsanitize=hwaddress has been
passed.

ChangeLog:

2019-08-29  Matthew Malcomson 

    * configure: Regenerate.
    * configure.ac: Add --bootstrap-hwasan option.

config/ChangeLog:

2019-12-12  Matthew Malcomson 

    * bootstrap-hwasan.mk: New file.

libiberty/ChangeLog:

2019-12-12  Matthew Malcomson 

    * configure: Regenerate.
    * configure.ac: Avoid using sanitizer.

lto-plugin/ChangeLog:

2019-12-12  Matthew Malcomson 

    * Makefile.am: Avoid using sanitizer.
    * Makefile.in: Regenerate.



### Attachment also inlined for ease of reply    
###



diff --git a/config/bootstrap-hwasan.mk b/config/bootstrap-hwasan.mk
new file mode 100644
index 
..4f60bed3fd6e98b47a3a38aea6eba2a7c320da25

--- /dev/null
+++ b/config/bootstrap-hwasan.mk
@@ -0,0 +1,8 @@
+# This option enables -fsanitize=hwaddress for stage2 and stage3.
+
+STAGE2_CFLAGS += -fsanitize=hwaddress
+STAGE3_CFLAGS += -fsanitize=hwaddress
+POSTSTAGE1_LDFLAGS += -fsanitize=hwaddress -static-libhwasan \
+ -B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/ \
+ -B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/hwasan/ \
+ -B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/hwasan/.libs
diff --git a/configure b/configure
index 
aec9186b2b0123d3088b69eb1ee541567654953e..6f71b111bd18ec053180beecf83dd4549e83c2b9 
100755

--- a/configure
+++ b/configure
@@ -7270,7 +7270,7 @@ fi
 # or bootstrap-ubsan, bootstrap it.
 if echo " ${target_configdirs} " | grep " libsanitizer " > /dev/null 
2>&1; then

   case "$BUILD_CONFIG" in
-    *bootstrap-asan* | *bootstrap-ubsan* )
+    *bootstrap-hwasan* | *bootstrap-asan* | *bootstrap-ubsan* )
bootstrap_target_libs=${bootstrap_target_libs}target-libsanitizer,
   bootstrap_fixincludes=yes
   ;;
diff --git a/configure.ac b/configure.ac
index 
b8ce2ad20b9d03e42731252a9ec2a8417c13e566..16bfdf164555dad94c789f17b6a63ba1a2e3e9f4 
100644

--- a/configure.ac
+++ b/configure.ac
@@ -2775,7 +2775,7 @@ fi
 # or bootstrap-ubsan, bootstrap it.
 if echo " ${target_configdirs} " | grep " libsanitizer " > /dev/null 
2>&1; then

   case "$BUILD_CONFIG" in
-    *bootstrap-asan* | *bootstrap-ubsan* )
+    *bootstrap-hwasan* | *bootstrap-asan* | *bootstrap-ubsan* )
bootstrap_target_libs=${bootstrap_target_libs}target-libsanitizer,
   bootstrap_fixincludes=yes
   ;;
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 
6c9579bfaff955eb43875b404fb7db1a667bf522..da9a8809c3440827ac22ef6936e080820197f4e7 
100644

--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -2645,6 +2645,13 @@ Some examples of build configurations designed 
for developers of GCC are:
 Compiles GCC itself using Address Sanitization in order to catch 
invalid memory

 accesses within the GCC code.

+@item @samp{bootstrap-hwasan}
+Compiles GCC itself using HWAddress Sanitization in order to catch 
invalid
+memory accesses within the GCC code.  This option is only available 
on AArch64

+targets with a very recent linux kernel (5.4 or later).




Using terms like "very recent" in documentation is discouraged. It won't 
be very recent in a couple of years time and I doubt any of us will 
remember to come update this snippet :)


I suggest something like "this option requires a Linux kernel support 
that supports the right ABI () (5.4 
or later)".


Thanks,

Kyrill




+
+@end table
+
 @section Building a cross compiler

 When building a cross compiler, it is not generally possible to do a
diff --git a/libiberty/configure b/libiberty/configure
index 
7a34dabec32b0b383bd33f07811757335f4dd39c..cb2dd4ff5295598343cc18b3a79a86a778f2261d 
100755

--- a/libiberty/configure
+++ b/libiberty/configure
@@ -5261,6 +5261,7 @@ fi
 NOASANFLAG=
 case " ${CFLAGS} " in
   *\ -fsanitize=address\ *) NOASANFLAG=-fno-sanitize=address ;;
+  *\ -fsanitize=hwaddress\ *) NOASANFLAG=-fno-sanitize=hwaddress ;;
 esac


diff --git a/libiberty/configure.ac b/libiberty/configure.ac
index 
f1ce76010c9acde79c5dc46686a78b2e2f19244e..043237628b79cbf37d07359b59c5ffe17a7a22ef 
100644

--- a/libiberty/configure.ac
+++ b/libiberty/configure.ac
@@ -240,6 +240,7 @@ AC_SUBST(PICFLAG)
 NOASANFLAG=
 case " ${CFLAGS} " in
   *\ -fsanitize=address\ *) NOASANFLAG=-fno-sanitize=address ;;
+  *\ -fsanitize=hwaddress\ *) NOASANFLAG=-fno-sanitize=hwaddress ;;
 esac
 AC_SUBST(NOASANFLAG)

diff --git a/lto-plugin/Makefile.am b/lto-plugin/Makefile.am
index 
28dc21014b2e86988fa88adabd63ce6092e18e02..34aa397d785e3cc9b6975de460d065900364c3ff 
100644

--- a/lto-plugin/Makefile.am
+++ b/lto-plugin/Makefile.am
@@ -11,8 +11,8 @@ AM_CPPFLAGS = 

Re: Add a compatible_vector_types_p target hook

2019-12-12 Thread Richard Sandiford
Richard Biener  writes:
> On December 12, 2019 4:10:33 PM GMT+01:00, Richard Sandiford 
>  wrote:
>>One problem with adding an N-bit vector extension to an existing
>>architecture is to decide how N-bit vectors should be passed to
>>functions and returned from functions.  Allowing all N-bit vector
>>types to be passed in registers breaks backwards compatibility,
>>since N-bit vectors could be used (and emulated) before the vector
>>extension was added.  But always passing N-bit vectors on the
>>stack would be inefficient for things like vector libm functions.
>>
>>For SVE we took the compromise position of predefining new SVE vector
>>types that are distinct from all existing vector types, including
>>GNU-style vectors.  The new types are passed and returned in an
>>efficient way while existing vector types are passed and returned
>>in the traditional way.  In the right circumstances, the two types
>>are inter-convertible.
>>
>>The SVE types are created using:
>>
>>  vectype = build_distinct_type_copy (vectype);
>>  SET_TYPE_STRUCTURAL_EQUALITY (vectype);
>>  TYPE_ARTIFICIAL (vectype) = 1;
>>
>>The C frontend maintains this distinction, using VIEW_CONVERT_EXPR
>>to convert from one type to the other.  However, the distinction can
>>be lost during gimple, which treats two vector types with the same
>>mode, number of elements, and element type as equivalent.  And for
>>most targets that's the right thing to do.
>
> And why's that a problem? The difference appears only in the function call 
> ABI which is determined by the function signature rather than types or modes 
> of the actual arguments? 

We use the type of the actual arguments when deciding how arguments
should be passed to functions:

  /* I counts args in order (to be) pushed; ARGPOS counts in order written.  */
  for (argpos = 0; argpos < num_actuals; i--, argpos++)
{
  tree type = TREE_TYPE (args[i].tree_value);
  [...]
  /* See if this argument should be passed by invisible reference.  */
  function_arg_info arg (type, argpos < n_named_args);

And it has to be that way for calls to unprototyped functions,
or for varargs.

The AArch64 port emits an error if calls pass values of SVE type to an
unprototyped function.  To do that we need to know whether the value
really is an SVE type rathr than a plain vector.

For varags the ABI is the same for 256 bits+.  But we'll have the
same problem there once we support -msve-vector-bits=128, since the
layout of SVE and Advanced SIMD vectors differ for big-endian.

Thanks,
Richard

>
> Richard. 
>
>>This patch therefore adds a hook that lets the target choose
>>whether such vector types are indeed equivalent.
>>
>>Note that the new tests fail for -mabi=ilp32 in the same way as other
>>ACLE-based tests.  I'm still planning to fix that as a follow-on.
>>
>>Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>>
>>Richard
>>
>>
>>2019-12-12  Richard Sandiford  
>>
>>gcc/
>>  * target.def (compatible_vector_types_p): New target hook.
>>  * hooks.h (hook_bool_const_tree_const_tree_true): Declare.
>>  * hooks.c (hook_bool_const_tree_const_tree_true): New function.
>>  * doc/tm.texi.in (TARGET_COMPATIBLE_VECTOR_TYPES_P): New hook.
>>  * doc/tm.texi: Regenerate.
>>  * gimple-expr.c: Include target.h.
>>  (useless_type_conversion_p): Use targetm.compatible_vector_types_p.
>>  * config/aarch64/aarch64.c (aarch64_compatible_vector_types_p): New
>>  function.
>>  (TARGET_COMPATIBLE_VECTOR_TYPES_P): Define.
>>  * config/aarch64/aarch64-sve-builtins.cc
>>(gimple_folder::convert_pred):
>>  Use the original predicate if it already has a suitable type.
>>
>>gcc/testsuite/
>>  * gcc.target/aarch64/sve/pcs/gnu_vectors_1.c: New test.
>>  * gcc.target/aarch64/sve/pcs/gnu_vectors_2.c: Likewise.
>>
>>Index: gcc/target.def
>>===
>>--- gcc/target.def2019-11-30 18:48:18.531984101 +
>>+++ gcc/target.def2019-12-12 15:07:43.960415368 +
>>@@ -3411,6 +3411,29 @@ must have move patterns for this mode.",
>>  hook_bool_mode_false)
>> 
>> DEFHOOK
>>+(compatible_vector_types_p,
>>+ "Return true if there is no target-specific reason for treating\n\
>>+vector types @var{type1} and @var{type2} as distinct types.  The
>>caller\n\
>>+has already checked for target-independent reasons, meaning that
>>the\n\
>>+types are known to have the same mode, to have the same number of
>>elements,\n\
>>+and to have what the caller considers to be compatible element
>>types.\n\
>>+\n\
>>+The main reason for defining this hook is to reject pairs of types\n\
>>+that are handled differently by the target's calling convention.\n\
>>+For example, when a new @var{N}-bit vector architecture is added\n\
>>+to a target, the target may want to handle normal @var{N}-bit\n\
>>+@code{VECTOR_TYPE} arguments and return values in the same way as\n\
>>+before, to maintain backwards compatibility.  

Re: [testsuite][arm] Remove xfail for vect-epilogues test

2019-12-12 Thread Christophe Lyon
On Wed, 11 Dec 2019 at 12:27, Andre Vieira (lists)
 wrote:
>
> Hi,
>
> We can now vectorize an epilogue for this loop for arm too, so removing
> xfail.
>
> Is this OK for trunk? Wasn't entirely sure whether I could commit this
> under obvious.
>

This fails on armeb :-(

> gcc/testsuite/ChangeLog:
> 2019-12-11  Andre Vieira  
>
>  * gcc.dg/vect/vect-epilogues.c: Remove xfail for arm.


Re: [PATCH] Fix simplify-rtx.c handling of avx512 vector comparisons (PR target/92908)

2019-12-12 Thread Jakub Jelinek
On Thu, Dec 12, 2019 at 04:59:40PM +0100, Richard Biener wrote:
> >If it starts being ambiguous somewhere, could we use some target macro
> >or
> >target hook to decide? 
> 
> Ambiguous IL is bad :/  IL semantics dependent on a target hook, too. Just 
> look at SHIFT_COUNT_TRUNCATED... 

The comparisons are like that since forever.
What is the result of a scalar comparison depends on the STORE_FLAG_VALUE macro,
true can be -1 or 1.
What is the result of vector comparison with vector result is unspecified,
unless VECTOR_STORE_FLAG_VALUE is defined (only s390 defines it).
What is the result of vector comparison with CC*mode result is unspecified,
could very well be either that all elements compare that way, or at least
one of them; who knows?  Bet the only case where
simplify_const_relational_operation handles vector comparisons doesn't care
about this distinction though.
And similarly, what exactly means vector comparison with int mode result is
unspecified, right now only i386 backend uses it for the bitmasks.

Anyway, I guess I can live with punting, I'm afraid I don't have spare 2
months to rewrite all of AVX512* masking just to fix this wrong-code, and it
wouldn't be appropriate in stage4 anyway.

Jakub



Re: [PATCH][ARM][GCC][0/x]: Support for MVE ACLE intrinsics.

2019-12-12 Thread Kyrill Tkachov

Hi Srinath,

On 11/14/19 7:12 PM, Srinath Parvathaneni wrote:

Hello,

This patches series is to support Arm MVE ACLE intrinsics.

Please refer to Arm reference manual [1] and MVE intrinsics [2] for 
more details.

Please refer to Chapter 13 MVE ACLE [3] for MVE intrinsics concepts.

This patch series depends on upstream patches "Armv8.1-M Mainline 
Security Extension" [4],
"CLI and multilib support for Armv8.1-M Mainline MVE extensions" [5] 
and "support for Armv8.1-M

Mainline scalar shifts" [6].

[1] 
https://static.docs.arm.com/ddi0553/bh/DDI0553B_h_armv8m_arm.pdf?_ga=2.102521798.659307368.1572453718-1501600630.1548848914
[2] 
https://developer.arm.com/architectures/instruction-sets/simd-isas/helium/mve-intrinsics
[3] 
https://static.docs.arm.com/101028/0009/Q3-ACLE_2019Q3_release-0009.pdf?_ga=2.239684871.588348166.1573726994-1501600630.1548848914

[4] https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01654.html
[5] https://gcc.gnu.org/ml/gcc-patches/2019-11/msg00641.html
[6] https://gcc.gnu.org/ml/gcc-patches/2019-11/msg01194.html

Srinath Parvathaneni(38):
[PATCH][ARM][GCC][1/x]: MVE ACLE intrinsics framework patch.
[PATCH][ARM][GCC][2/x]: MVE ACLE intrinsics framework patch.
[PATCH][ARM][GCC][3/x]: MVE ACLE intrinsics framework patch.
[PATCH][ARM][GCC][4/x]: MVE ACLE vector interleaving store intrinsics.
[PATCH][ARM][GCC][1/1x]: Patch to support MVE ACLE intrinsics with 
unary operand.

[PATCH][ARM][GCC][2/1x]: MVE intrinsics with unary operand.
[PATCH][ARM][GCC][3/1x]: MVE intrinsics with unary operand.
[PATCH][ARM][GCC][4/1x]: MVE intrinsics with unary operand.
[PATCH][ARM][GCC][1/2x]: MVE intrinsics with binary operands.
[PATCH][ARM][GCC][2/2x]: MVE intrinsics with binary operands.
[PATCH][ARM][GCC][3/2x]: MVE intrinsics with binary operands.
[PATCH][ARM][GCC][4/2x]: MVE intrinsics with binary operands.
[PATCH][ARM][GCC][5/2x]: MVE intrinsics with binary operands.
[PATCH][ARM][GCC][1/3x]: MVE intrinsics with ternary operands.
[PATCH][ARM][GCC][2/3x]: MVE intrinsics with ternary operands.
[PATCH][ARM][GCC][3/3x]: MVE intrinsics with ternary operands.
[PATCH][ARM][GCC][1/4x]: MVE intrinsics with quaternary operands.
[PATCH][ARM][GCC][2/4x]: MVE intrinsics with quaternary operands.
[PATCH][ARM][GCC][3/4x]: MVE intrinsics with quaternary operands.
[PATCH][ARM][GCC][4/4x]: MVE intrinsics with quaternary operands.
[PATCH][ARM][GCC][1/5x]: MVE store intrinsics.
[PATCH][ARM][GCC][2/5x]: MVE load intrinsics.
[PATCH][ARM][GCC][3/5x]: MVE store intrinsics with predicated suffix.
[PATCH][ARM][GCC][4/5x]: MVE load intrinsics with zero(_z) suffix.
[PATCH][ARM][GCC][5/5x]: MVE ACLE load intrinsics which load a byte, 
halfword, or word from memory.
[PATCH][ARM][GCC][6/5x]: Remaining MVE load intrinsics which loads 
half word and word or double word from memory.
[PATCH][ARM][GCC][7/5x]: MVE store intrinsics which stores byte,half 
word or word to memory.
[PATCH][ARM][GCC][8/5x]: Remaining MVE store intrinsics which stores 
an half word, word and double word to memory.
[PATCH][ARM][GCC][6x]:MVE ACLE vaddq intrinsics using arithmetic plus 
operator.

[PATCH][ARM][GCC][7x]: MVE vreinterpretq and vuninitializedq intrinsics.
[PATCH][ARM][GCC][1/8x]: MVE ACLE vidup, vddup, viwdup and vdwdup 
intrinsics with writeback.
[PATCH][ARM][GCC][2/8x]: MVE ACLE gather load and scatter store 
intrinsics with writeback.
[PATCH][ARM][GCC][9x]: MVE ACLE predicated intrinsics with (dont-care) 
variant.
[PATCH][ARM][GCC][10x]: MVE ACLE intrinsics "add with carry across 
beats" and "beat-wise substract".
[PATCH][ARM][GCC][11x]: MVE ACLE vector interleaving store and 
deinterleaving load intrinsics and also aliases to vstr and vldr 
intrinsics.

[PATCH][ARM][GCC][12x]: MVE ACLE intrinsics to set and get vector lane.
[PATCH][ARM][GCC][13x]: MVE ACLE scalar shift intrinsics.
[PATCH][ARM][GCC][14x]: MVE ACLE whole vector left shift with carry 
intrinsics.



Thank you for working on these.

I will reply to individual patches with more targeted comments.

As this is a fairly large amount of code, here's my high-level view:

The MVE intrinsics spec has more complexities than the Neon intrinsics one:

* It needs support for both the user-namespace versions, and the __arm_* 
ones.


* There are also overloaded forms that in C are implemented using _Generic.

The above two facts make for a rather bulky and messy arm_mve.h 
implementation.


In the case of the _Generic usage we hit the performance problems 
reported in PR c/91937.


Ideally, I'd like to see the frontend parts of these intrinsics 
implemented in a similar way to the SVE ACLE 
(https://gcc.gnu.org/ml/gcc-patches/2019-10/msg00413.html)


i.e. have the compiler inject the right functions into the language and 
do overload resolution through the appropriate hooks, thus keeping the 
(unavoidable) complexity in the backend rather than arm_mve.h


That being said, this is a major feature that I would very much like to 
see in GCC 10 and the current implementation, outside of the new .md 

Re: Add a compatible_vector_types_p target hook

2019-12-12 Thread Richard Biener
On December 12, 2019 4:10:33 PM GMT+01:00, Richard Sandiford 
 wrote:
>One problem with adding an N-bit vector extension to an existing
>architecture is to decide how N-bit vectors should be passed to
>functions and returned from functions.  Allowing all N-bit vector
>types to be passed in registers breaks backwards compatibility,
>since N-bit vectors could be used (and emulated) before the vector
>extension was added.  But always passing N-bit vectors on the
>stack would be inefficient for things like vector libm functions.
>
>For SVE we took the compromise position of predefining new SVE vector
>types that are distinct from all existing vector types, including
>GNU-style vectors.  The new types are passed and returned in an
>efficient way while existing vector types are passed and returned
>in the traditional way.  In the right circumstances, the two types
>are inter-convertible.
>
>The SVE types are created using:
>
>  vectype = build_distinct_type_copy (vectype);
>  SET_TYPE_STRUCTURAL_EQUALITY (vectype);
>  TYPE_ARTIFICIAL (vectype) = 1;
>
>The C frontend maintains this distinction, using VIEW_CONVERT_EXPR
>to convert from one type to the other.  However, the distinction can
>be lost during gimple, which treats two vector types with the same
>mode, number of elements, and element type as equivalent.  And for
>most targets that's the right thing to do.

And why's that a problem? The difference appears only in the function call ABI 
which is determined by the function signature rather than types or modes of the 
actual arguments? 

Richard. 

>This patch therefore adds a hook that lets the target choose
>whether such vector types are indeed equivalent.
>
>Note that the new tests fail for -mabi=ilp32 in the same way as other
>ACLE-based tests.  I'm still planning to fix that as a follow-on.
>
>Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>
>Richard
>
>
>2019-12-12  Richard Sandiford  
>
>gcc/
>   * target.def (compatible_vector_types_p): New target hook.
>   * hooks.h (hook_bool_const_tree_const_tree_true): Declare.
>   * hooks.c (hook_bool_const_tree_const_tree_true): New function.
>   * doc/tm.texi.in (TARGET_COMPATIBLE_VECTOR_TYPES_P): New hook.
>   * doc/tm.texi: Regenerate.
>   * gimple-expr.c: Include target.h.
>   (useless_type_conversion_p): Use targetm.compatible_vector_types_p.
>   * config/aarch64/aarch64.c (aarch64_compatible_vector_types_p): New
>   function.
>   (TARGET_COMPATIBLE_VECTOR_TYPES_P): Define.
>   * config/aarch64/aarch64-sve-builtins.cc
>(gimple_folder::convert_pred):
>   Use the original predicate if it already has a suitable type.
>
>gcc/testsuite/
>   * gcc.target/aarch64/sve/pcs/gnu_vectors_1.c: New test.
>   * gcc.target/aarch64/sve/pcs/gnu_vectors_2.c: Likewise.
>
>Index: gcc/target.def
>===
>--- gcc/target.def 2019-11-30 18:48:18.531984101 +
>+++ gcc/target.def 2019-12-12 15:07:43.960415368 +
>@@ -3411,6 +3411,29 @@ must have move patterns for this mode.",
>  hook_bool_mode_false)
> 
> DEFHOOK
>+(compatible_vector_types_p,
>+ "Return true if there is no target-specific reason for treating\n\
>+vector types @var{type1} and @var{type2} as distinct types.  The
>caller\n\
>+has already checked for target-independent reasons, meaning that
>the\n\
>+types are known to have the same mode, to have the same number of
>elements,\n\
>+and to have what the caller considers to be compatible element
>types.\n\
>+\n\
>+The main reason for defining this hook is to reject pairs of types\n\
>+that are handled differently by the target's calling convention.\n\
>+For example, when a new @var{N}-bit vector architecture is added\n\
>+to a target, the target may want to handle normal @var{N}-bit\n\
>+@code{VECTOR_TYPE} arguments and return values in the same way as\n\
>+before, to maintain backwards compatibility.  However, it may also\n\
>+provide new, architecture-specific @code{VECTOR_TYPE}s that are
>passed\n\
>+and returned in a more efficient way.  It is then important to
>maintain\n\
>+a distinction between the ``normal'' @code{VECTOR_TYPE}s and the
>new\n\
>+architecture-specific ones.\n\
>+\n\
>+The default implementation returns true, which is correct for most
>targets.",
>+ bool, (const_tree type1, const_tree type2),
>+ hook_bool_const_tree_const_tree_true)
>+
>+DEFHOOK
> (vector_alignment,
> "This hook can be used to define the alignment for a vector of type\n\
>@var{type}, in order to comply with a platform ABI.  The default is
>to\n\
>Index: gcc/hooks.h
>===
>--- gcc/hooks.h2019-11-04 21:13:57.727755548 +
>+++ gcc/hooks.h2019-12-12 15:07:43.960415368 +
>@@ -45,6 +45,7 @@ extern bool hook_bool_uint_uint_mode_fal
> extern bool hook_bool_uint_mode_true (unsigned int, machine_mode);
> extern bool hook_bool_tree_false (tree);
> 

Re: [PATCH] Fix simplify-rtx.c handling of avx512 vector comparisons (PR target/92908)

2019-12-12 Thread Richard Biener
On December 12, 2019 10:53:26 AM GMT+01:00, Jakub Jelinek  
wrote:
>On Thu, Dec 12, 2019 at 09:22:47AM +, Richard Sandiford wrote:
>> > So there's no whole vector equality RTX but we have to pun to
>integer
>> > modes for that?  The eq:SImode would suggest that.  Guess we should
>have
>> > used a BImode vector representation...
>
>Probably something like
>V2BImode/V4BImode/V8BImode/V16BImode/V32BImode/V64BImode, yes, but I'm
>afraid changing it would be extremely hard, all the builtins that take
>the
>masks have int/long long arguments and we already generate terrible
>code
>for the mask registers with various PRs, using a different mode would
>make
>it even worse (but sure, more clear).
>> >
>> > Can you check whether we have any target with whole vector compare
>patterns that would break here? 
>
>All I can see is both x86 and rs6000 using eq:CC with vector operands
>(the former in cbranchv*, the latter in various patterns that set both
>vector mode comparison result and CCmode comparison result, and
>aarch64 having cbranch even for vector modes, but only in define_expand
>and
>expanding that to ptest which uses UNSPECs.
>
>> I wonder how easy it would be to make the mask registers use
>> MODE_VECTOR_BOOL instead of scalar integers... :-)
>
>I'm afraid hard.
>
>> For now I think the safest thing would be to punt, rather than assume
>> one thing or the other.  simplify_const_relational_operation doesn't
>> handle many vector cases anyway, and most interesting optimisations
>> like this should happen on gimple, where we know whether the
>condition
>> result is a vector or a scalar.
>
>If it starts being ambiguous somewhere, could we use some target macro
>or
>target hook to decide? 

Ambiguous IL is bad :/  IL semantics dependent on a target hook, too. Just look 
at SHIFT_COUNT_TRUNCATED... 

Richard. 

 We already use various target macros in this
>code,
>e.g. FLOAT_STORE_FLAG_VALUE, VECTOR_STORE_FLAG_VALUE, and if they
>aren't
>defined, we punt.
>If we used some macro for this case too, we could punt by default if we
>see
>this case and only if the backend said how to handle vector cmp_mode
>with
>scalar int result we could fold it?
>
>   Jakub



[PATCH 3/X] [libsanitizer] Add option to bootstrap using HWASAN

2019-12-12 Thread Matthew Malcomson
Updated to include documentation ChangeLog and apply cleanly on the
bootstrap-asan documentation I mentioned in a different email.



This is an analogous option to --bootstrap-asan to configure.  It allows
bootstrapping GCC using HWASAN.

For the same reasons as for ASAN we have to avoid using the HWASAN
sanitizer when compiling libiberty and the lto-plugin.

Also add a function to query whether -fsanitize=hwaddress has been
passed.

ChangeLog:

2019-08-29  Matthew Malcomson  

* configure: Regenerate.
* configure.ac: Add --bootstrap-hwasan option.

config/ChangeLog:

2019-12-12  Matthew Malcomson  

* bootstrap-hwasan.mk: New file.

gcc/ChangeLog:

2019-12-12  Matthew Malcomson  

* doc/install.texi: Document new option.

libiberty/ChangeLog:

2019-12-12  Matthew Malcomson  

* configure: Regenerate.
* configure.ac: Avoid using sanitizer.

lto-plugin/ChangeLog:

2019-12-12  Matthew Malcomson  

* Makefile.am: Avoid using sanitizer.
* Makefile.in: Regenerate.



### Attachment also inlined for ease of reply###


diff --git a/config/bootstrap-hwasan.mk b/config/bootstrap-hwasan.mk
new file mode 100644
index 
..4f60bed3fd6e98b47a3a38aea6eba2a7c320da25
--- /dev/null
+++ b/config/bootstrap-hwasan.mk
@@ -0,0 +1,8 @@
+# This option enables -fsanitize=hwaddress for stage2 and stage3.
+
+STAGE2_CFLAGS += -fsanitize=hwaddress
+STAGE3_CFLAGS += -fsanitize=hwaddress
+POSTSTAGE1_LDFLAGS += -fsanitize=hwaddress -static-libhwasan \
+ -B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/ \
+ -B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/hwasan/ \
+ -B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/hwasan/.libs
diff --git a/configure b/configure
index 
f6027397cedd5c28cd821a0bc7629d6537393ecf..0b502f2784927d72eab248741633305296bf09e7
 100755
--- a/configure
+++ b/configure
@@ -7270,7 +7270,7 @@ fi
 # or bootstrap-ubsan, bootstrap it.
 if echo " ${target_configdirs} " | grep " libsanitizer " > /dev/null 2>&1; then
   case "$BUILD_CONFIG" in
-*bootstrap-asan* | *bootstrap-ubsan* )
+*bootstrap-hwasan* | *bootstrap-asan* | *bootstrap-ubsan* )
   bootstrap_target_libs=${bootstrap_target_libs}target-libsanitizer,
   bootstrap_fixincludes=yes
   ;;
diff --git a/configure.ac b/configure.ac
index 
50e2fa135b10486170f68ffa162dcbeee2adff90..f793e14e0316e26d05ba72696e8c7c10641c7c96
 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2775,7 +2775,7 @@ fi
 # or bootstrap-ubsan, bootstrap it.
 if echo " ${target_configdirs} " | grep " libsanitizer " > /dev/null 2>&1; then
   case "$BUILD_CONFIG" in
-*bootstrap-asan* | *bootstrap-ubsan* )
+*bootstrap-hwasan* | *bootstrap-asan* | *bootstrap-ubsan* )
   bootstrap_target_libs=${bootstrap_target_libs}target-libsanitizer,
   bootstrap_fixincludes=yes
   ;;
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 
c5937c94b7cebd3d0f1d7d551a7b593c128e7673..37cb8ed15671205567ee20286e21fb904b3e3c9f
 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -2676,6 +2676,12 @@ Some examples of build configurations designed for 
developers of GCC are:
 @item @samp{bootstrap-asan}
 Compiles GCC itself using Address Sanitization in order to catch invalid memory
 accesses within the GCC code.
+
+@item @samp{bootstrap-hwasan}
+Compiles GCC itself using HWAddress Sanitization in order to catch invalid
+memory accesses within the GCC code.  This option is only available on AArch64
+targets with a very recent linux kernel (5.4 or later).
+
 @end table
 
 @section Building a cross compiler
diff --git a/libiberty/configure b/libiberty/configure
index 
7a34dabec32b0b383bd33f07811757335f4dd39c..cb2dd4ff5295598343cc18b3a79a86a778f2261d
 100755
--- a/libiberty/configure
+++ b/libiberty/configure
@@ -5261,6 +5261,7 @@ fi
 NOASANFLAG=
 case " ${CFLAGS} " in
   *\ -fsanitize=address\ *) NOASANFLAG=-fno-sanitize=address ;;
+  *\ -fsanitize=hwaddress\ *) NOASANFLAG=-fno-sanitize=hwaddress ;;
 esac
 
 
diff --git a/libiberty/configure.ac b/libiberty/configure.ac
index 
f1ce76010c9acde79c5dc46686a78b2e2f19244e..043237628b79cbf37d07359b59c5ffe17a7a22ef
 100644
--- a/libiberty/configure.ac
+++ b/libiberty/configure.ac
@@ -240,6 +240,7 @@ AC_SUBST(PICFLAG)
 NOASANFLAG=
 case " ${CFLAGS} " in
   *\ -fsanitize=address\ *) NOASANFLAG=-fno-sanitize=address ;;
+  *\ -fsanitize=hwaddress\ *) NOASANFLAG=-fno-sanitize=hwaddress ;;
 esac
 AC_SUBST(NOASANFLAG)
 
diff --git a/lto-plugin/Makefile.am b/lto-plugin/Makefile.am
index 
28dc21014b2e86988fa88adabd63ce6092e18e02..34aa397d785e3cc9b6975de460d065900364c3ff
 100644
--- a/lto-plugin/Makefile.am
+++ b/lto-plugin/Makefile.am
@@ -11,8 +11,8 @@ AM_CPPFLAGS = -I$(top_srcdir)/../include $(DEFS)
 AM_CFLAGS = @ac_lto_plugin_warn_cflags@
 AM_LDFLAGS = @ac_lto_plugin_ldflags@
 AM_LIBTOOLFLAGS = --tag=disable-static
-override CFLAGS := $(filter-out 

Document --with-build-config=bootstrap-asan option.

2019-12-12 Thread Matthew Malcomson
Document how to configure using asan (bootstrap-asan option).

Since I'm adding a bootstrap-hwasan option and documenting that, 
bootstrap-asan should also be documented.

(As Martin pointed out in the hwasan reviews).



(FYI I now notice that my hwasan patch 3/X needs this to apply cleanly).



gcc/ChangeLog:

2019-12-12  Matthew Malcomson  

* doc/install.texi: Document bootstrap-asan configuration option.





###


commit 6e0bbe33120ad3f92e4266ecfe4ecb8ce8958865
Author: Matthew Malcomson 
Date:   Wed Nov 6 12:48:08 2019 +

 Document bootstrap-asan compilation option

diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 80b4781..c5937c9 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -2670,6 +2670,14 @@ the build tree.

  @end table

+Some examples of build configurations designed for developers of GCC are:
+
+@table @asis
+@item @samp{bootstrap-asan}
+Compiles GCC itself using Address Sanitization in order to catch 
invalid memory
+accesses within the GCC code.
+@end table
+
  @section Building a cross compiler

  When building a cross compiler, it is not generally possible to do a



Re: [PATCH 43/49] analyzer: new file: exploded-graph.h

2019-12-12 Thread David Malcolm
On Wed, 2019-12-11 at 13:04 -0700, Jeff Law wrote:
> On Fri, 2019-11-15 at 20:23 -0500, David Malcolm wrote:
> > This patch adds exploded_graph and related classes, for managing
> > exploring paths through the user's code as a directed graph
> > of  pairs.
> > 
> > gcc/ChangeLog:
> > * analyzer/exploded-graph.h: New file.
> > ---
> >  gcc/analyzer/exploded-graph.h | 754
> > ++
> >  1 file changed, 754 insertions(+)
> >  create mode 100644 gcc/analyzer/exploded-graph.h
> > 
> > diff --git a/gcc/analyzer/exploded-graph.h b/gcc/analyzer/exploded-
> > graph.h
> > new file mode 100644
> > index 000..f97d2b6
> > --- /dev/null
> > +++ b/gcc/analyzer/exploded-graph.h
> > @@ -0,0 +1,754 @@
> > +/* Classes for managing a directed graph of  pairs.
> > +   Copyright (C) 2019 Free Software Foundation, Inc.
> > +   Contributed by David Malcolm .
> > +
> > +This file is part of GCC.
> > +
> > +GCC is free software; you can redistribute it and/or modify it
> > +under the terms of the GNU General Public License as published by
> > +the Free Software Foundation; either version 3, or (at your
> > option)
> > +any later version.
> > +
> > +GCC is distributed in the hope that it will be useful, but
> > +WITHOUT ANY WARRANTY; without even the implied warranty of
> > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +General Public License for more details.
> > +
> > +You should have received a copy of the GNU General Public License
> > +along with GCC; see the file COPYING3.  If not see
> > +;;.  */
> > +
> > +#ifndef GCC_ANALYZER_EXPLODED_GRAPH_H
> > +#define GCC_ANALYZER_EXPLODED_GRAPH_H
> > +
> > +#include "fibonacci_heap.h"
> > +#include "analyzer/analyzer-logging.h"
> > +#include "analyzer/constraint-manager.h"
> > +#include "analyzer/diagnostic-manager.h"
> > +#include "analyzer/program-point.h"
> > +#include "analyzer/program-state.h"
> > +#include "analyzer/shortest-paths.h"
> > +
> > +//
> > //
> NIT.  Is there some reason you don't just use whitespace for these
> kind
> of vertical separators.  THe more I see them, the more they bug me,
> probably because that's not been the style we've used for GCC.

I've been using them to highlight new "chapters" in a source file;
typically there's another comment following them.  I'm happy to drop
the one where there isn't a trailing comment, but, I'm hoping there's
an acceptable way to have an "section-header"-style comment.

We have "^L" in a bunch of places, but my editor (emacs) doesn't do a
great job of highlighting them.

There's some precedence for: e.g. 

gengtype.c:5094:/*** Manage input files.  **/

lto-section-in.c has e.g.:

/*/
/* Record renamings of static declarations   */
/*/

tree-vect-loop-manip.c has:

/*
  Simple Loop Peeling Utilities

  Utilities to support loop peeling for vectorization purposes.
 */

but I suspect given how sporadic these are that this stuff snuck past
review.

Alternatively I can just drop these and retain the trailing comments,
keeping their formatting.

> > ///
> > +
> > +/* Concrete implementation of region_model_context, wiring it up
> > to
> > the
> > +   rest of the analysis engine.  */
> > +
> > +class impl_region_model_context : public region_model_context,
> > public log_user
> Multiple inheritance?  Is it really needed?  Can we handle via
> composition instead?

It's handy here, but I can probably eliminate it.

> 
> > +/* A  pair, used internally by
> > +   exploded_node as its immutable data, and as a key for
> > identifying
> > +   exploded_nodes we've already seen in the graph.  */
> > +
> > +struct point_and_state
> Shouldn't this be a class?

Will fix (originally was just a pair of public fields, but then I added
the cached hash value, so there's an argument for "class-ifying" this).

> +
> > +/* Per-program_point data for an exploded_graph.  */
> > +
> > +struct per_program_point_data
> Here too?  THere may be others.  I'd suggest reviewing all your
> "structs" and determine if we're better off calling them
> "class".  I'm
> not going to insist on it though since I think the last discussion in
> this space was to relax the conventions :-)

Both fields are public, but they're not-POD.

I'll look into them.

> > +
> > +class exploded_graph : public digraph, public log_user
> Multiple inheritance again?

Again, it's handy here, but I think I can probably eliminate it.



Dave



[PATCH 7/X] [libsanitizer] Add tests

2019-12-12 Thread Matthew Malcomson
Adding hwasan tests.

Only interesting thing here is that we have to make sure the tagging mechanism
is deterministic to avoid flaky tests.

gcc/testsuite/ChangeLog:

2019-12-12  Matthew Malcomson  

* c-c++-common/hwasan/aligned-alloc.c: New test.
* c-c++-common/hwasan/alloca-array-accessible.c: New test.
* c-c++-common/hwasan/alloca-gets-different-tag.c: New test.
* c-c++-common/hwasan/alloca-outside-caught.c: New test.
* c-c++-common/hwasan/arguments.c: New test.
* c-c++-common/hwasan/arguments-1.c: New test.
* c-c++-common/hwasan/arguments-2.c: New test.
* c-c++-common/hwasan/arguments-3.c: New test.
* c-c++-common/hwasan/asan-pr63316.c: New test.
* c-c++-common/hwasan/asan-pr70541.c: New test.
* c-c++-common/hwasan/asan-pr78106.c: New test.
* c-c++-common/hwasan/asan-pr79944.c: New test.
* c-c++-common/hwasan/asan-rlimit-mmap-test-1.c: New test.
* c-c++-common/hwasan/bitfield-1.c: New test.
* c-c++-common/hwasan/bitfield-2.c: New test.
* c-c++-common/hwasan/builtin-special-handling.c: New test.
* c-c++-common/hwasan/check-interface.c: New test.
* c-c++-common/hwasan/halt_on_error-1.c: New test.
* c-c++-common/hwasan/heap-overflow.c: New test.
* c-c++-common/hwasan/hwasan-poison-optimisation.c: New test.
* c-c++-common/hwasan/hwasan-thread-access-parent.c: New test.
* c-c++-common/hwasan/hwasan-thread-basic-failure.c: New test.
* c-c++-common/hwasan/hwasan-thread-clears-stack.c: New test.
* c-c++-common/hwasan/hwasan-thread-success.c: New test.
* c-c++-common/hwasan/kernel-defaults.c: New test.
* c-c++-common/hwasan/large-aligned-0.c: New test.
* c-c++-common/hwasan/large-aligned-1.c: New test.
* c-c++-common/hwasan/large-aligned-untagging-0.c: New test.
* c-c++-common/hwasan/large-aligned-untagging-1.c: New test.
* c-c++-common/hwasan/macro-definition.c: New test.
* c-c++-common/hwasan/no-sanitize-attribute.c: New test.
* c-c++-common/hwasan/param-instrument-reads-and-writes.c: New test.
* c-c++-common/hwasan/param-instrument-reads.c: New test.
* c-c++-common/hwasan/param-instrument-writes.c: New test.
* c-c++-common/hwasan/param-memintrin.c: New test.
* c-c++-common/hwasan/random-frame-tag.c: New test.
* c-c++-common/hwasan/sanity-check-pure-c.c: New test.
* c-c++-common/hwasan/setjmp-longjmp-0.c: New test.
* c-c++-common/hwasan/setjmp-longjmp-1.c: New test.
* c-c++-common/hwasan/stack-tagging-basic-0.c: New test.
* c-c++-common/hwasan/stack-tagging-basic-1.c: New test.
* c-c++-common/hwasan/stack-tagging-disable.c: New test.
* c-c++-common/hwasan/unprotected-allocas-0.c: New test.
* c-c++-common/hwasan/unprotected-allocas-1.c: New test.
* c-c++-common/hwasan/use-after-free.c: New test.
* c-c++-common/hwasan/vararray-outside-caught.c: New test.
* c-c++-common/hwasan/vararray-stack-restore-correct.c: New test.
* c-c++-common/hwasan/very-large-objects.c: New test.
* g++.dg/hwasan/hwasan.exp: New file.
* g++.dg/hwasan/rvo-handled.C: New test.
* g++.dg/hwasan/try-catch-0.cpp: New test.
* g++.dg/hwasan/try-catch-1.cpp: New test.
* gcc.dg/hwasan/hwasan.exp: New file.
* gcc.dg/hwasan/nested-functions-0.c: New test.
* gcc.dg/hwasan/nested-functions-1.c: New test.
* gcc.dg/hwasan/nested-functions-2.c: New test.
* lib/hwasan-dg.exp: New file.



### Attachment also inlined for ease of reply###


diff --git a/gcc/testsuite/c-c++-common/hwasan/aligned-alloc.c 
b/gcc/testsuite/c-c++-common/hwasan/aligned-alloc.c
new file mode 100644
index 
..e5837a9f0766fc4d91fabc1a3c2e0017f845dc46
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/hwasan/aligned-alloc.c
@@ -0,0 +1,15 @@
+/* { dg-do run } */
+/* { dg-shouldfail "hwasan" } */
+/* This program fails at runtime in the libhwasan library.
+   The allocator can't handle the requested invalid alignment.  */
+
+int
+main ()
+{
+  void *p = __builtin_aligned_alloc (17, 100);
+  if (((unsigned long long)p & 0x10) == 0)
+return 0;
+  return 1;
+}
+
+/* { dg-output "HWAddressSanitizer: invalid alignment requested in 
aligned_alloc: 17" } */
diff --git a/gcc/testsuite/c-c++-common/hwasan/alloca-array-accessible.c 
b/gcc/testsuite/c-c++-common/hwasan/alloca-array-accessible.c
new file mode 100644
index 
..c6b4d264b8c477b52b859a0411aabb0fc8dde352
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/hwasan/alloca-array-accessible.c
@@ -0,0 +1,32 @@
+/* { dg-do run } */
+
+#define alloca __builtin_alloca
+
+int __attribute__ ((noinline))
+using_alloca (int num)
+{
+  int retval = 0;
+  int *big_array = (int*)alloca (num * 

[PATCH 6/X] [libsanitizer] Add hwasan pass and associated gimple changes

2019-12-12 Thread Matthew Malcomson
There are four main features to this change:

1) Check pointer tags match address tags.

In the new `hwasan` pass we put HWASAN_CHECK internal functions around
all memory accesses, to check that tags in the pointer being used match
the tag stored in shadow memory for the memory region being used.

These internal functions are expanded into actual checks in the sanopt
pass that happens just before expansion into RTL.

We use the same mechanism that currently inserts ASAN_CHECK internal
functions to insert the new HWASAN_CHECK functions.

2) Instrument known builtin function calls.

Handle all builtin functions that we know use memory accesses.
This commit uses the machinery added for ASAN to identify builtin
functions that access memory.

The main differences between the approaches for HWASAN and ASAN are:
 - libhwasan intercepts much less builtin functions.
 - Alloca needs to be transformed differently (instead of adding
   redzones it needs to tag shadow memory and return a tagged pointer).
 - stack_restore needs to untag the shadow stack between the current
   position and where it's going.
 - `noreturn` functions can not be handled by simply unpoisoning the
   entire shadow stack -- there is no "always valid" tag.
   (exceptions and things such as longjmp need to be handled in a
   different way).

For hardware implemented checking (such as AArch64's memory tagging
extension) alloca and stack_restore will need to be handled by hooks in
the backend rather than transformation at the gimple level.  This will
allow architecture specific handling of such stack modifications.

3) Introduce HWASAN block-scope poisoning

Here we use exactly the same mechanism as ASAN_MARK to poison/unpoison
variables on entry/exit of a block.

In order to simply use the exact same machinery we're using the same
internal functions until the SANOPT pass.  This means that all handling
of ASAN_MARK is the same.
This has the negative that the naming may be a little confusing, but a
positive that handling of the internal function doesn't have to be
duplicated for a function that behaves exactly the same but has a
different name.

gcc/ChangeLog:

2019-12-12  Matthew Malcomson  

* asan.c (handle_builtin_stack_restore): Account for HWASAN.
(handle_builtin_alloca): Account for HWASAN.
(get_mem_refs_of_builtin_call): Special case strlen for HWASAN.
(report_error_func): Assert not HWASAN.
(build_check_stmt): Make HWASAN_CHECK instead of ASAN_CHECK.
(instrument_derefs): HWASAN does not tag globals.
(maybe_instrument_call): Don't instrument `noreturn` functions.
(initialize_sanitizer_builtins): Add new type.
(asan_expand_mark_ifn): Account for HWASAN.
(asan_expand_check_ifn): Assert never called by HWASAN.
(asan_expand_poison_ifn): Account for HWASAN.
(hwasan_instrument): New.
(hwasan_base): New.
(hwasan_emit_untag_frame): Free block-scope-var hash map.
(hwasan_check_func): New.
(hwasan_expand_check_ifn): New.
(hwasan_expand_mark_ifn): New.
(gate_hwasan): New.
(class pass_hwasan): New.
(make_pass_hwasan): New.
(class pass_hwasan_O0): New.
(make_pass_hwasan_O0): New.
* asan.h (hwasan_base): New decl.
(hwasan_expand_check_ifn): New decl.
(hwasan_expand_mark_ifn): New decl.
(gate_hwasan): New decl.
(enum hwasan_mark_flags): New.
(asan_intercepted_p): Always false for hwasan.
(asan_sanitize_use_after_scope): Account for HWASAN.
* builtin-types.def (BT_FN_PTR_CONST_PTR_UINT8): New.
* gimple-pretty-print.c (dump_gimple_call_args): Account for
HWASAN.
* gimplify.c (asan_poison_variable): Account for HWASAN.
(gimplify_function_tree): Remove requirement of
SANITIZE_ADDRESS, requiring asan or hwasan is accounted for in
`asan_sanitize_use_after_scope`.
* internal-fn.c (expand_HWASAN_CHECK): New.
(expand_HWASAN_CHOOSE_TAG): New.
(expand_HWASAN_MARK): New.
(expand_HWASAN_ALLOCA_UNPOISON): New.
* internal-fn.def (HWASAN_CHOOSE_TAG): New.
(HWASAN_CHECK): New.
(HWASAN_MARK): New.
(HWASAN_ALLOCA_UNPOISON): New.
* passes.def: Add hwasan and hwasan_O0 passes.
* sanitizer.def (BUILT_IN_HWASAN_LOAD1): New.
(BUILT_IN_HWASAN_LOAD2): New.
(BUILT_IN_HWASAN_LOAD4): New.
(BUILT_IN_HWASAN_LOAD8): New.
(BUILT_IN_HWASAN_LOAD16): New.
(BUILT_IN_HWASAN_LOADN): New.
(BUILT_IN_HWASAN_STORE1): New.
(BUILT_IN_HWASAN_STORE2): New.
(BUILT_IN_HWASAN_STORE4): New.
(BUILT_IN_HWASAN_STORE8): New.
(BUILT_IN_HWASAN_STORE16): New.
(BUILT_IN_HWASAN_STOREN): New.
(BUILT_IN_HWASAN_LOAD1_NOABORT): New.
(BUILT_IN_HWASAN_LOAD2_NOABORT): New.
(BUILT_IN_HWASAN_LOAD4_NOABORT): New.
(BUILT_IN_HWASAN_LOAD8_NOABORT): 

[PATCH 5/X] [libsanitizer][mid-end] Introduce stack variable handling for HWASAN

2019-12-12 Thread Matthew Malcomson
Handling stack variables has three features.

1) Ensure HWASAN required alignment for stack variables

When tagging shadow memory, we need to ensure that each tag granule is
only used by one variable at a time.

This is done by ensuring that each tagged variable is aligned to the tag
granule representation size and also ensure that the end of each
variable as an alignment boundary between the end and the start of any
other data stored on the stack.

This patch ensures that by adding alignment requirements in
`align_local_variable` and forcing all stack variable allocation to be
deferred so that `expand_stack_vars` can ensure the stack pointer is
aligned before allocating any variable for the current frame.

2) Put tags into each stack variable pointer

Make sure that every pointer to a stack variable includes a tag of some
sort on it.

The way tagging works is:
  1) For every new stack frame, a random tag is generated.
  2) A base register is formed from the stack pointer value and this
 random tag.
  3) References to stack variables are now formed with RTL describing an
 offset from this base in both tag and value.

The random tag generation is handled by a backend hook.  This hook
decides whether to introduce a random tag or use the stack background
based on the parameter hwasan-random-frame-tag.  Using the stack
background is necessary for testing and bootstrap.  It is necessary
during bootstrap to avoid breaking the `configure` test program for
determining stack direction.

Using the stack background means that every stack frame has the initial
tag of zero and variables are tagged with incrementing tags from 1,
which also makes debugging a bit easier.

The tag offsets are also handled by a backend hook.

This patch also adds some macros defining how the HWASAN shadow memory
is stored and how a tag is stored in a pointer.

3) For each stack variable, tag and untag the shadow stack on function
   prologue and epilogue.

On entry to each function we tag the relevant shadow stack region for
each stack variable the tag to match the tag added to each pointer for
that variable.

This is the first patch where we use the HWASAN shadow space, so we need
to add in the libhwasan initialisation code that creates this shadow
memory region into the binary we produce.  This instrumentation is done
in `compile_file`.

When exiting a function we need to ensure the shadow stack for this
function has no remaining tag.  Without clearing the shadow stack area
for this stack frame, later function calls could get false positives
when those later function calls check untagged areas (such as parameters
passed on the stack) against a shadow stack area with left-over tag.

Hence we ensure that the entire stack frame is cleared on function exit.

gcc/ChangeLog:

2019-12-12  Matthew Malcomson  

* asan.c (hwasan_record_base): New function.
(hwasan_emit_untag_frame): New.
(hwasan_increment_tag): New function.
(hwasan_with_tag): New function.
(hwasan_tag_init): New function.
(initialize_sanitizer_builtins): Define new builtins.
(ATTR_NOTHROW_LIST): New macro.
(hwasan_current_tag): New.
(hwasan_emit_prologue): New.
(hwasan_create_untagged_base): New.
(hwasan_finish_file): New.
(hwasan_sanitize_stack_p): New.
(hwasan_sanitize_p): New.
* asan.h (hwasan_record_base): New declaration.
(hwasan_emit_untag_frame): New.
(hwasan_increment_tag): New declaration.
(hwasan_with_tag): New declaration.
(hwasan_sanitize_stack_p): New declaration.
(hwasan_tag_init): New declaration.
(hwasan_sanitize_p): New declaration.
(HWASAN_TAG_SIZE): New macro.
(HWASAN_TAG_GRANULE_SIZE):New macro.
(HWASAN_SHIFT):New macro.
(HWASAN_SHIFT_RTX):New macro.
(HWASAN_STACK_BACKGROUND):New macro.
(hwasan_finish_file): New.
(hwasan_current_tag): New.
(hwasan_create_untagged_base): New.
(hwasan_emit_prologue): New.
* cfgexpand.c (struct stack_vars_data): Add information to
record hwasan variable stack offsets.
(expand_stack_vars): Ensure variables are offset from a tagged
base. Record offsets for hwasan. Ensure alignment.
(expand_used_vars): Call function to emit prologue, and get
untagging instructions for function exit.
(align_local_variable): Ensure alignment.
(defer_stack_allocation): Ensure all variables are deferred so
they can be handled by `expand_stack_vars`.
(expand_one_stack_var_at): Account for tags in
variables when using HWASAN.
(expand_one_stack_var_1): Pass new argument to
expand_one_stack_var_at.
(init_vars_expansion): Initialise hwasan internal variables when
starting variable expansion.
* doc/tm.texi (TARGET_MEMTAG_GENTAG): Document.
* doc/tm.texi.in (TARGET_MEMTAG_GENTAG): Document.
 

[PATCH 1/X] [libsanitizer] Tie the hwasan library into our build system

2019-12-12 Thread Matthew Malcomson
This patch tries to tie libhwasan into the GCC build system in the same way
that the other sanitizer runtime libraries are handled.

libsanitizer/ChangeLog:

2019-12-12  Matthew Malcomson  

* Makefile.am:  Build libhwasan.
* Makefile.in:  Build libhwasan.
* asan/Makefile.in:  Build libhwasan.
* configure:  Build libhwasan.
* configure.ac:  Build libhwasan.
* hwasan/Makefile.am: New file.
* hwasan/Makefile.in: New file.
* hwasan/libtool-version: New file.
* interception/Makefile.in: Build libhwasan.
* libbacktrace/Makefile.in: Build libhwasan.
* libsanitizer.spec.in: Build libhwasan.
* lsan/Makefile.in: Build libhwasan.
* merge.sh: Build libhwasan.
* sanitizer_common/Makefile.in: Build libhwasan.
* tsan/Makefile.in: Build libhwasan.
* ubsan/Makefile.in: Build libhwasan.



### Attachment also inlined for ease of reply###


diff --git a/libsanitizer/Makefile.am b/libsanitizer/Makefile.am
index 
65ed1e712378ef453f820f86c4d3221f9dee5f2c..2a7e8e1debe838719db0f0fad218b2543cc3111b
 100644
--- a/libsanitizer/Makefile.am
+++ b/libsanitizer/Makefile.am
@@ -14,11 +14,12 @@ endif
 if LIBBACKTRACE_SUPPORTED
 SUBDIRS += libbacktrace
 endif
-SUBDIRS += lsan asan ubsan
+SUBDIRS += lsan asan ubsan hwasan
 nodist_saninclude_HEADERS += \
   include/sanitizer/lsan_interface.h \
   include/sanitizer/asan_interface.h \
-  include/sanitizer/tsan_interface.h
+  include/sanitizer/tsan_interface.h \
+  include/sanitizer/hwasan_interface.h
 if TSAN_SUPPORTED
 SUBDIRS += tsan
 endif
diff --git a/libsanitizer/Makefile.in b/libsanitizer/Makefile.in
index 
0d789b3a59d21ea2e5a23057ca3afe15425feec4..36aa952af7e04bc0e4fb94cdcd584d539193d781
 100644
--- a/libsanitizer/Makefile.in
+++ b/libsanitizer/Makefile.in
@@ -92,7 +92,8 @@ target_triplet = @target@
 @SANITIZER_SUPPORTED_TRUE@am__append_1 = 
include/sanitizer/common_interface_defs.h \
 @SANITIZER_SUPPORTED_TRUE@ include/sanitizer/lsan_interface.h \
 @SANITIZER_SUPPORTED_TRUE@ include/sanitizer/asan_interface.h \
-@SANITIZER_SUPPORTED_TRUE@ include/sanitizer/tsan_interface.h
+@SANITIZER_SUPPORTED_TRUE@ include/sanitizer/tsan_interface.h \
+@SANITIZER_SUPPORTED_TRUE@ include/sanitizer/hwasan_interface.h
 @SANITIZER_SUPPORTED_TRUE@@USING_MAC_INTERPOSE_FALSE@am__append_2 = 
interception
 @LIBBACKTRACE_SUPPORTED_TRUE@@SANITIZER_SUPPORTED_TRUE@am__append_3 = 
libbacktrace
 @SANITIZER_SUPPORTED_TRUE@@TSAN_SUPPORTED_TRUE@am__append_4 = tsan
@@ -206,7 +207,7 @@ ETAGS = etags
 CTAGS = ctags
 CSCOPE = cscope
 DIST_SUBDIRS = sanitizer_common interception libbacktrace lsan asan \
-   ubsan tsan
+   ubsan hwasan tsan
 ACLOCAL = @ACLOCAL@
 ALLOC_FILE = @ALLOC_FILE@
 AMTAR = @AMTAR@
@@ -328,6 +329,7 @@ install_sh = @install_sh@
 libdir = @libdir@
 libexecdir = @libexecdir@
 link_libasan = @link_libasan@
+link_libhwasan = @link_libhwasan@
 link_liblsan = @link_liblsan@
 link_libtsan = @link_libtsan@
 link_libubsan = @link_libubsan@
@@ -361,7 +363,7 @@ sanincludedir = 
$(libdir)/gcc/$(target_alias)/$(gcc_version)/include/sanitizer
 nodist_saninclude_HEADERS = $(am__append_1)
 @SANITIZER_SUPPORTED_TRUE@SUBDIRS = sanitizer_common $(am__append_2) \
 @SANITIZER_SUPPORTED_TRUE@ $(am__append_3) lsan asan ubsan \
-@SANITIZER_SUPPORTED_TRUE@ $(am__append_4)
+@SANITIZER_SUPPORTED_TRUE@ hwasan $(am__append_4)
 gcc_version := $(shell @get_gcc_base_ver@ $(top_srcdir)/../gcc/BASE-VER)
 
 # Work around what appears to be a GNU make bug handling MAKEFLAGS
diff --git a/libsanitizer/asan/Makefile.in b/libsanitizer/asan/Makefile.in
index 
00b6082da5372efd679ddc230f588bbc58161ef6..76689c3b224b1fb04895ae48829eac4b6784cd84
 100644
--- a/libsanitizer/asan/Makefile.in
+++ b/libsanitizer/asan/Makefile.in
@@ -382,6 +382,7 @@ install_sh = @install_sh@
 libdir = @libdir@
 libexecdir = @libexecdir@
 link_libasan = @link_libasan@
+link_libhwasan = @link_libhwasan@
 link_liblsan = @link_liblsan@
 link_libtsan = @link_libtsan@
 link_libubsan = @link_libubsan@
diff --git a/libsanitizer/configure b/libsanitizer/configure
index 
79b5c1eadb59018bca13a33f19f3494c170365ee..ff72af73e6f77aaf93bf39e6799f896851a377dd
 100755
--- a/libsanitizer/configure
+++ b/libsanitizer/configure
@@ -657,6 +657,7 @@ USING_MAC_INTERPOSE_TRUE
 link_liblsan
 link_libubsan
 link_libtsan
+link_libhwasan
 link_libasan
 LSAN_SUPPORTED_FALSE
 LSAN_SUPPORTED_TRUE
@@ -12334,7 +12335,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 12337 "configure"
+#line 12338 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -12440,7 +12441,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 12443 "configure"
+#line 12444 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -15916,6 +15917,10 @@ fi
 

[Patch 0/X] HWASAN v3

2019-12-12 Thread Matthew Malcomson
Hello,

I've gone through the suggestions Martin made and implemented  the ones I think
I can implement for GCC10.

The two functionality changes in this version are:
Added the --param's hwasan-instrument-reads, hwasan-instrument-writes,
hwasan-instrument-allocas, hwasan-memintrin, options.  I.e. Those that asan has
and that make sense for hwasan.

Avoided HWASAN_STACK_BACKGROUND in hwasan_increment_tag when using a
deterministic tagging approach.


There are a lot of extra comments and tests.


Bootstrapped and regtested on x86_64 and AArch64.
Bootstrapped with `--with-build-config=bootstrap-hwasan` on AArch64 and hwasan
features tested there.
Built the linux kernel using this feature and ran the test_kasan.ko testing to
check the this works for the kernel.
(NOTE: I actually did all the above testing before a search and replace of
`memory_tagging_p` for `hwasan_sanitize_p` and fixing a typo in the
`hwasan-instrument-allocas` parameter name, I will run all the tests again
before committing but figure I'll send this out now since I fully expect the
tests to still pass).


I noticed one extra testsuite failure from those mentioned in the previous
version emails: g++.dg/cpp2a/ucn2.C.
I believe this is HWASAN correctly catching a problem in the compiler.
I've logged the issue here https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92919 .


I haven't gotten ASAN_MARK to print as HWASAN_MARK when using memory tagging,
since I'm not sure the way I found to implement this would be acceptable.  The
inlined patch below works but it requires a special declaration instead of just
an ~#include~.


diff --git a/gcc/internal-fn.h b/gcc/internal-fn.h
index a1bc081..d81eb12 100644
--- a/gcc/internal-fn.h
+++ b/gcc/internal-fn.h
@@ -101,10 +101,16 @@ extern void init_internal_fns ();
 
 extern const char *const internal_fn_name_array[];
 
+
+extern bool hwasan_sanitize_p (void);
 static inline const char *
 internal_fn_name (enum internal_fn fn)
 {
-  return internal_fn_name_array[(int) fn];
+  const char *ret = internal_fn_name_array[(int) fn];
+  if (! strcmp (ret, "ASAN_MARK")
+  && hwasan_sanitize_p ())
+return "HWASAN_MARK";
+  return ret;
 }
 
 extern internal_fn lookup_internal_fn (const char *);


Entire patch series attached to cover letter.

all-patches.tar.gz
Description: all-patches.tar.gz


[PATCH 3/X] [libsanitizer] Add option to bootstrap using HWASAN

2019-12-12 Thread Matthew Malcomson
This is an analogous option to --bootstrap-asan to configure.  It allows
bootstrapping GCC using HWASAN.

For the same reasons as for ASAN we have to avoid using the HWASAN
sanitizer when compiling libiberty and the lto-plugin.

Also add a function to query whether -fsanitize=hwaddress has been
passed.

ChangeLog:

2019-08-29  Matthew Malcomson  

* configure: Regenerate.
* configure.ac: Add --bootstrap-hwasan option.

config/ChangeLog:

2019-12-12  Matthew Malcomson  

* bootstrap-hwasan.mk: New file.

libiberty/ChangeLog:

2019-12-12  Matthew Malcomson  

* configure: Regenerate.
* configure.ac: Avoid using sanitizer.

lto-plugin/ChangeLog:

2019-12-12  Matthew Malcomson  

* Makefile.am: Avoid using sanitizer.
* Makefile.in: Regenerate.



### Attachment also inlined for ease of reply###


diff --git a/config/bootstrap-hwasan.mk b/config/bootstrap-hwasan.mk
new file mode 100644
index 
..4f60bed3fd6e98b47a3a38aea6eba2a7c320da25
--- /dev/null
+++ b/config/bootstrap-hwasan.mk
@@ -0,0 +1,8 @@
+# This option enables -fsanitize=hwaddress for stage2 and stage3.
+
+STAGE2_CFLAGS += -fsanitize=hwaddress
+STAGE3_CFLAGS += -fsanitize=hwaddress
+POSTSTAGE1_LDFLAGS += -fsanitize=hwaddress -static-libhwasan \
+ -B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/ \
+ -B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/hwasan/ \
+ -B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/hwasan/.libs
diff --git a/configure b/configure
index 
aec9186b2b0123d3088b69eb1ee541567654953e..6f71b111bd18ec053180beecf83dd4549e83c2b9
 100755
--- a/configure
+++ b/configure
@@ -7270,7 +7270,7 @@ fi
 # or bootstrap-ubsan, bootstrap it.
 if echo " ${target_configdirs} " | grep " libsanitizer " > /dev/null 2>&1; then
   case "$BUILD_CONFIG" in
-*bootstrap-asan* | *bootstrap-ubsan* )
+*bootstrap-hwasan* | *bootstrap-asan* | *bootstrap-ubsan* )
   bootstrap_target_libs=${bootstrap_target_libs}target-libsanitizer,
   bootstrap_fixincludes=yes
   ;;
diff --git a/configure.ac b/configure.ac
index 
b8ce2ad20b9d03e42731252a9ec2a8417c13e566..16bfdf164555dad94c789f17b6a63ba1a2e3e9f4
 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2775,7 +2775,7 @@ fi
 # or bootstrap-ubsan, bootstrap it.
 if echo " ${target_configdirs} " | grep " libsanitizer " > /dev/null 2>&1; then
   case "$BUILD_CONFIG" in
-*bootstrap-asan* | *bootstrap-ubsan* )
+*bootstrap-hwasan* | *bootstrap-asan* | *bootstrap-ubsan* )
   bootstrap_target_libs=${bootstrap_target_libs}target-libsanitizer,
   bootstrap_fixincludes=yes
   ;;
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 
6c9579bfaff955eb43875b404fb7db1a667bf522..da9a8809c3440827ac22ef6936e080820197f4e7
 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -2645,6 +2645,13 @@ Some examples of build configurations designed for 
developers of GCC are:
 Compiles GCC itself using Address Sanitization in order to catch invalid memory
 accesses within the GCC code.
 
+@item @samp{bootstrap-hwasan}
+Compiles GCC itself using HWAddress Sanitization in order to catch invalid
+memory accesses within the GCC code.  This option is only available on AArch64
+targets with a very recent linux kernel (5.4 or later).
+
+@end table
+
 @section Building a cross compiler
 
 When building a cross compiler, it is not generally possible to do a
diff --git a/libiberty/configure b/libiberty/configure
index 
7a34dabec32b0b383bd33f07811757335f4dd39c..cb2dd4ff5295598343cc18b3a79a86a778f2261d
 100755
--- a/libiberty/configure
+++ b/libiberty/configure
@@ -5261,6 +5261,7 @@ fi
 NOASANFLAG=
 case " ${CFLAGS} " in
   *\ -fsanitize=address\ *) NOASANFLAG=-fno-sanitize=address ;;
+  *\ -fsanitize=hwaddress\ *) NOASANFLAG=-fno-sanitize=hwaddress ;;
 esac
 
 
diff --git a/libiberty/configure.ac b/libiberty/configure.ac
index 
f1ce76010c9acde79c5dc46686a78b2e2f19244e..043237628b79cbf37d07359b59c5ffe17a7a22ef
 100644
--- a/libiberty/configure.ac
+++ b/libiberty/configure.ac
@@ -240,6 +240,7 @@ AC_SUBST(PICFLAG)
 NOASANFLAG=
 case " ${CFLAGS} " in
   *\ -fsanitize=address\ *) NOASANFLAG=-fno-sanitize=address ;;
+  *\ -fsanitize=hwaddress\ *) NOASANFLAG=-fno-sanitize=hwaddress ;;
 esac
 AC_SUBST(NOASANFLAG)
 
diff --git a/lto-plugin/Makefile.am b/lto-plugin/Makefile.am
index 
28dc21014b2e86988fa88adabd63ce6092e18e02..34aa397d785e3cc9b6975de460d065900364c3ff
 100644
--- a/lto-plugin/Makefile.am
+++ b/lto-plugin/Makefile.am
@@ -11,8 +11,8 @@ AM_CPPFLAGS = -I$(top_srcdir)/../include $(DEFS)
 AM_CFLAGS = @ac_lto_plugin_warn_cflags@
 AM_LDFLAGS = @ac_lto_plugin_ldflags@
 AM_LIBTOOLFLAGS = --tag=disable-static
-override CFLAGS := $(filter-out -fsanitize=address,$(CFLAGS))
-override LDFLAGS := $(filter-out -fsanitize=address,$(LDFLAGS))
+override CFLAGS := $(filter-out -fsanitize=address 
-fsanitize=hwaddress,$(CFLAGS))
+override LDFLAGS := $(filter-out 

[PATCH 4/X] [libsanitizer][options] Add hwasan flags and argument parsing

2019-12-12 Thread Matthew Malcomson
These flags can't be used at the same time as any of the other
sanitizers.
We add an equivalent flag to -static-libasan in -static-libhwasan to
ensure static linking.

The -fsanitize=kernel-hwaddress option is for compiling targeting the
kernel.  This flag has defaults that allow compiling KASAN with tags as
it is currently implemented.
These defaults are that we do not sanitize variables on the stack and
always recover from a detected bug.
Stack tagging in the kernel is a future aim, stack instrumentation has
not yet been enabled for the kernel for clang either
(https://lists.infradead.org/pipermail/linux-arm-kernel/2019-October/687121.html).

We introduce a backend hook `targetm.memtag.can_tag_addresses` that
indicates to the mid-end whether a target has a feature like AArch64 TBI
where the top byte of an address is ignored.
Without this feature hwasan sanitization is not done.

gcc/ChangeLog:

2019-12-12  Matthew Malcomson  

* common.opt (flag_sanitize_recover): Default for kernel
hwaddress.
(static-libhwasan): New cli option.
* config/aarch64/aarch64.c (aarch64_can_tag_addresses): New.
(TARGET_MEMTAG_CAN_TAG_ADDRESSES): New.
* config/gnu-user.h (LIBHWASAN_EARLY_SPEC): hwasan equivalent of
asan command line flags.
* cppbuiltin.c (define_builtin_macros_for_compilation_flags):
Add hwasan equivalent of __SANITIZE_ADDRESS__.
* doc/tm.texi: Document new hook.
* doc/tm.texi.in: Document new hook.
* flag-types.h (enum sanitize_code): New sanitizer values.
* gcc.c (STATIC_LIBHWASAN_LIBS): New macro.
(LIBHWASAN_SPEC): New macro.
(LIBHWASAN_EARLY_SPEC): New macro.
(SANITIZER_EARLY_SPEC): Update to include hwasan.
(SANITIZER_SPEC): Update to include hwasan.
(sanitize_spec_function): Use hwasan options.
* opts.c (finish_options): Describe conflicts between address
sanitizers.
(sanitizer_opts): Introduce new sanitizer flags.
(common_handle_option): Add defaults for kernel sanitizer.
* params.opt (hwasan-stack): New
(hwasan-random-frame-tag): New
(hwasan-instrument-allocas): New
(hwasan-instrument-reads): New
(hwasan-instrument-writes): New
(hwasan-memintrin): New
* target.def (HOOK_PREFIX): Add new hook.
* targhooks.c (default_memtag_can_tag_addresses): New.
* toplev.c (process_options): Ensure hwasan only on TBI
architectures.

gcc/c-family/ChangeLog:

2019-12-12  Matthew Malcomson  

* c-attribs.c (handle_no_sanitize_hwaddress_attribute): New
attribute.



### Attachment also inlined for ease of reply###


diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 
dc56e2ec62ffc7f494cc59ddf02452ac0cb406de..81f8dfc6dd3f54067d7cc0def3c0babc03bcd9c4
 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -54,6 +54,8 @@ static tree handle_cold_attribute (tree *, tree, tree, int, 
bool *);
 static tree handle_no_sanitize_attribute (tree *, tree, tree, int, bool *);
 static tree handle_no_sanitize_address_attribute (tree *, tree, tree,
  int, bool *);
+static tree handle_no_sanitize_hwaddress_attribute (tree *, tree, tree,
+   int, bool *);
 static tree handle_no_sanitize_thread_attribute (tree *, tree, tree,
 int, bool *);
 static tree handle_no_address_safety_analysis_attribute (tree *, tree, tree,
@@ -412,6 +414,8 @@ const struct attribute_spec c_common_attribute_table[] =
  handle_no_sanitize_attribute, NULL },
   { "no_sanitize_address",0, 0, true, false, false, false,
  handle_no_sanitize_address_attribute, NULL },
+  { "no_sanitize_hwaddress",0, 0, true, false, false, false,
+ handle_no_sanitize_hwaddress_attribute, NULL },
   { "no_sanitize_thread", 0, 0, true, false, false, false,
  handle_no_sanitize_thread_attribute, NULL },
   { "no_sanitize_undefined",  0, 0, true, false, false, false,
@@ -946,6 +950,22 @@ handle_no_sanitize_address_attribute (tree *node, tree 
name, tree, int,
   return NULL_TREE;
 }
 
+/* Handle a "no_sanitize_hwaddress" attribute; arguments as in
+   struct attribute_spec.handler.  */
+
+static tree
+handle_no_sanitize_hwaddress_attribute (tree *node, tree name, tree, int,
+ bool *no_add_attrs)
+{
+  *no_add_attrs = true;
+  if (TREE_CODE (*node) != FUNCTION_DECL)
+warning (OPT_Wattributes, "%qE attribute ignored", name);
+  else
+add_no_sanitize_value (*node, SANITIZE_HWADDRESS);
+
+  return NULL_TREE;
+}
+
 /* Handle a "no_sanitize_thread" attribute; arguments as in
struct attribute_spec.handler.  */
 
diff --git a/gcc/common.opt b/gcc/common.opt

[PATCH 2/X] [libsanitizer] Only build libhwasan when targeting AArch64

2019-12-12 Thread Matthew Malcomson
Though the library has limited support for x86, we don't have any
support for generating code targeting x86 so there is no point building
for that target.

libsanitizer/ChangeLog:

2019-12-12  Matthew Malcomson  

* Makefile.am: Condition building hwasan directory.
* Makefile.in: Regenerate.
* configure: Regenerate.
* configure.ac: Set HWASAN_SUPPORTED based on target
architecture.
* configure.tgt: Likewise.



### Attachment also inlined for ease of reply###


diff --git a/libsanitizer/Makefile.am b/libsanitizer/Makefile.am
index 
2a7e8e1debe838719db0f0fad218b2543cc3111b..065a65e78d49f7689a01ecb64db1f07ca83aa987
 100644
--- a/libsanitizer/Makefile.am
+++ b/libsanitizer/Makefile.am
@@ -14,7 +14,7 @@ endif
 if LIBBACKTRACE_SUPPORTED
 SUBDIRS += libbacktrace
 endif
-SUBDIRS += lsan asan ubsan hwasan
+SUBDIRS += lsan asan ubsan
 nodist_saninclude_HEADERS += \
   include/sanitizer/lsan_interface.h \
   include/sanitizer/asan_interface.h \
@@ -23,6 +23,9 @@ nodist_saninclude_HEADERS += \
 if TSAN_SUPPORTED
 SUBDIRS += tsan
 endif
+if HWASAN_SUPPORTED
+SUBDIRS += hwasan
+endif
 endif
 
 ## May be used by toolexeclibdir.
diff --git a/libsanitizer/Makefile.in b/libsanitizer/Makefile.in
index 
36aa952af7e04bc0e4fb94cdcd584d539193d781..75a99491cb1d4422fd5e2d93cae93eb883ae0963
 100644
--- a/libsanitizer/Makefile.in
+++ b/libsanitizer/Makefile.in
@@ -97,6 +97,7 @@ target_triplet = @target@
 @SANITIZER_SUPPORTED_TRUE@@USING_MAC_INTERPOSE_FALSE@am__append_2 = 
interception
 @LIBBACKTRACE_SUPPORTED_TRUE@@SANITIZER_SUPPORTED_TRUE@am__append_3 = 
libbacktrace
 @SANITIZER_SUPPORTED_TRUE@@TSAN_SUPPORTED_TRUE@am__append_4 = tsan
+@HWASAN_SUPPORTED_TRUE@@SANITIZER_SUPPORTED_TRUE@am__append_5 = hwasan
 subdir = .
 ACLOCAL_M4 = $(top_srcdir)/aclocal.m4
 am__aclocal_m4_deps = $(top_srcdir)/../config/acx.m4 \
@@ -207,7 +208,7 @@ ETAGS = etags
 CTAGS = ctags
 CSCOPE = cscope
 DIST_SUBDIRS = sanitizer_common interception libbacktrace lsan asan \
-   ubsan hwasan tsan
+   ubsan tsan hwasan
 ACLOCAL = @ACLOCAL@
 ALLOC_FILE = @ALLOC_FILE@
 AMTAR = @AMTAR@
@@ -363,7 +364,7 @@ sanincludedir = 
$(libdir)/gcc/$(target_alias)/$(gcc_version)/include/sanitizer
 nodist_saninclude_HEADERS = $(am__append_1)
 @SANITIZER_SUPPORTED_TRUE@SUBDIRS = sanitizer_common $(am__append_2) \
 @SANITIZER_SUPPORTED_TRUE@ $(am__append_3) lsan asan ubsan \
-@SANITIZER_SUPPORTED_TRUE@ hwasan $(am__append_4)
+@SANITIZER_SUPPORTED_TRUE@ $(am__append_4) $(am__append_5)
 gcc_version := $(shell @get_gcc_base_ver@ $(top_srcdir)/../gcc/BASE-VER)
 
 # Work around what appears to be a GNU make bug handling MAKEFLAGS
diff --git a/libsanitizer/configure b/libsanitizer/configure
index 
ff72af73e6f77aaf93bf39e6799f896851a377dd..4e95194fe3567b1227c4036c2f5bf6540f735975
 100755
--- a/libsanitizer/configure
+++ b/libsanitizer/configure
@@ -659,6 +659,8 @@ link_libubsan
 link_libtsan
 link_libhwasan
 link_libasan
+HWASAN_SUPPORTED_FALSE
+HWASAN_SUPPORTED_TRUE
 LSAN_SUPPORTED_FALSE
 LSAN_SUPPORTED_TRUE
 TSAN_SUPPORTED_FALSE
@@ -12335,7 +12337,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 12338 "configure"
+#line 12340 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -12441,7 +12443,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 12444 "configure"
+#line 12446 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -15792,6 +15794,7 @@ fi
 # Get target configury.
 unset TSAN_SUPPORTED
 unset LSAN_SUPPORTED
+unset HWASAN_SUPPORTED
 . ${srcdir}/configure.tgt
  if test "x$TSAN_SUPPORTED" = "xyes"; then
   TSAN_SUPPORTED_TRUE=
@@ -15809,6 +15812,14 @@ else
   LSAN_SUPPORTED_FALSE=
 fi
 
+ if test "x$HWASAN_SUPPORTED" = "xyes"; then
+  HWASAN_SUPPORTED_TRUE=
+  HWASAN_SUPPORTED_FALSE='#'
+else
+  HWASAN_SUPPORTED_TRUE='#'
+  HWASAN_SUPPORTED_FALSE=
+fi
+
 
 # Check for functions needed.
 for ac_func in clock_getres clock_gettime clock_settime lstat readlink
@@ -16791,7 +16802,7 @@ ac_config_files="$ac_config_files Makefile 
libsanitizer.spec libbacktrace/backtr
 ac_config_headers="$ac_config_headers config.h"
 
 
-ac_config_files="$ac_config_files interception/Makefile 
sanitizer_common/Makefile libbacktrace/Makefile lsan/Makefile asan/Makefile 
hwasan/Makefile ubsan/Makefile"
+ac_config_files="$ac_config_files interception/Makefile 
sanitizer_common/Makefile libbacktrace/Makefile lsan/Makefile asan/Makefile 
ubsan/Makefile"
 
 
 if test "x$TSAN_SUPPORTED" = "xyes"; then
@@ -16799,6 +16810,11 @@ if test "x$TSAN_SUPPORTED" = "xyes"; then
 
 fi
 
+if test "x$HWASAN_SUPPORTED" = "xyes"; then
+  ac_config_files="$ac_config_files hwasan/Makefile"
+
+fi
+
 
 
 
@@ -17059,6 +17075,10 @@ if test -z "${LSAN_SUPPORTED_TRUE}" && test -z 
"${LSAN_SUPPORTED_FALSE}"; then
   as_fn_error $? "conditional 

[C++ PATCH] Make same_type_p return false for gnu_vector_type_p differences (PR 92789)

2019-12-12 Thread Richard Sandiford
As Jason pointed out in the review of the C++ gnu_vector_type_p patch:

https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00173.html

the real fix for the XFAILs in acle/general-c++/gnu_vectors_*.C is to
make same_type_p return false for two types if one is gnu_vector_type_p
and the other isn't.  This patch does that and fixes the fallout in the
way Jason suggested above.

I'd said at the time that some of the tests require:

  /* If the constructor already has the array type, it's been through
 digest_init, so we shouldn't try to do anything more.  */
  bool digested = same_type_p (atype, TREE_TYPE (init));

(build_vec_init) to be true for gnu vectors built from non-gnu vectors.
I can no longer reproduce that, maybe because my original same_type_p
change was bogus or because something in the later sizeless type
support made it moot (or something else :-)).

This means that sve-sizeless-2.C (the length-specific test) now reports
some errors for comparisons and pointer differences that sve-sizeless-1.C
(the length-agnostic test) already had.  I think requiring a cast is OK
for those cases, since there's no reason to mix GNU and non-GNU versions
of the same vector in the same object.  Even -flax-vector-conversions
would require a cast for different vector types here, for both C and C++.

same_type_p had:

  else if (strict == COMPARE_STRUCTURAL)
return structural_comptypes (t1, t2, COMPARE_STRICT);
  else
return structural_comptypes (t1, t2, strict);

Since structural_comptypes ignored the COMPARE_STRUCTURAL bit of "strict"
before the patch, the "else if" was harmless but AFAICT unnecessary.

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

Richard


2019-12-12  Richard Sandiford  

gcc/c-family/
PR c++/92789
* c-common.c (vector_targets_convertible_p): Handle types that
differ in gnu_vector_type_p but are otherwise identical.

gcc/cp/
PR c++/92789
* typeck.c (structural_comptypes): For strict checking, make sure
that two vector types agree on gnu_vector_type_p.
(comptypes): Pass the original strictness argument to
structural_comptypes.
(similar_type_p): Use a structural comparision for vectors.

gcc/testsuite/
PR c++/92789
* g++.dg/ext/sve-sizeless-2.C (statements): Expect pointer
difference and comparisons between GNU and non-GNU types
to be rejected.  Expect __is_same to be false for such pairs.
* g++.target/aarch64/sve/acle/general-c++/gnu_vectors_1.C: Remove
XFAILs.
* g++.target/aarch64/sve/acle/general-c++/gnu_vectors_2.C: Likewise.

Index: gcc/c-family/c-common.c
===
--- gcc/c-family/c-common.c 2019-12-06 10:31:11.286433308 +
+++ gcc/c-family/c-common.c 2019-12-12 15:13:55.789913884 +
@@ -923,11 +923,18 @@ bool_promoted_to_int_p (tree t)
 bool
 vector_targets_convertible_p (const_tree t1, const_tree t2)
 {
-  if (VECTOR_TYPE_P (t1) && VECTOR_TYPE_P (t2)
-  && (TYPE_VECTOR_OPAQUE (t1) || TYPE_VECTOR_OPAQUE (t2))
+  if (!VECTOR_TYPE_P (t1) || !VECTOR_TYPE_P (t2))
+return false;
+
+  if ((TYPE_VECTOR_OPAQUE (t1) || TYPE_VECTOR_OPAQUE (t2))
   && tree_int_cst_equal (TYPE_SIZE (t1), TYPE_SIZE (t2)))
 return true;
 
+  if (gnu_vector_type_p (t1) != gnu_vector_type_p (t2)
+  && known_eq (TYPE_VECTOR_SUBPARTS (t1), TYPE_VECTOR_SUBPARTS (t2))
+  && lang_hooks.types_compatible_p (TREE_TYPE (t1), TREE_TYPE (t2)))
+return true;
+
   return false;
 }
 
Index: gcc/cp/typeck.c
===
--- gcc/cp/typeck.c 2019-12-10 09:17:17.668010318 +
+++ gcc/cp/typeck.c 2019-12-12 15:13:55.789913884 +
@@ -1429,6 +1429,9 @@ structural_comptypes (tree t1, tree t2,
   if (maybe_ne (TYPE_VECTOR_SUBPARTS (t1), TYPE_VECTOR_SUBPARTS (t2))
  || !same_type_p (TREE_TYPE (t1), TREE_TYPE (t2)))
return false;
+  if (strict == COMPARE_STRICT
+ && gnu_vector_type_p (t1) != gnu_vector_type_p (t2))
+   return false;
   break;
 
 case TYPE_PACK_EXPANSION:
@@ -1526,8 +1529,6 @@ comptypes (tree t1, tree t2, int strict)
   else
return structural_comptypes (t1, t2, strict);
 }
-  else if (strict == COMPARE_STRUCTURAL)
-return structural_comptypes (t1, t2, COMPARE_STRICT);
   else
 return structural_comptypes (t1, t2, strict);
 }
@@ -1556,6 +1557,9 @@ similar_type_p (tree type1, tree type2)
   if (type1 == error_mark_node || type2 == error_mark_node)
 return false;
 
+  if (type1 == type2)
+return true;
+
   /* Informally, two types are similar if, ignoring top-level cv-qualification:
  * they are the same type; or
  * they are both pointers, and the pointed-to types are similar; or
@@ -1564,8 +1568,18 @@ similar_type_p (tree type1, tree type2)
  * they are both arrays of the same size or both arrays of unknown bound,
 

Fix tree-nrv.c ICE for direct internal functions

2019-12-12 Thread Richard Sandiford
pass_return_slot::execute has:

  /* Ignore internal functions without direct optabs,
 those are expanded specially and aggregate_value_p
 on their result might result in undesirable warnings
 with some backends.  */
  && (!gimple_call_internal_p (stmt)
  || direct_internal_fn_p (gimple_call_internal_fn (stmt)))
  && aggregate_value_p (TREE_TYPE (gimple_call_lhs (stmt)),
gimple_call_fndecl (stmt)))

But what the comment says applies to directly-mapped internal functions
too, since they're only used if the target supports them without a
libcall.

This was triggering an ICE on the attached testcase.  The svld3 call
is folded to an IFN_LOAD_LANES, which returns an array of vectors with
VNx48QImode.  Since no such return type can exist in C, the target hook
was complaining about an unexpected use of SVE modes.  (And we want to
keep asserting for that, so that we don't accidentally define an ABI for
an unexpected corner case.)

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

Richard


2019-12-12  Richard Sandiford  

gcc/
* tree-nrv.c (pass_return_slot::execute): Handle all internal
functions the same way, rather than singling out those that
aren't mapped directly to optabs.

gcc/testsuite/
* gcc.target/aarch64/sve/acle/general/nrv_1.c: New test.

Index: gcc/tree-nrv.c
===
--- gcc/tree-nrv.c  2019-03-08 18:15:26.824777889 +
+++ gcc/tree-nrv.c  2019-12-12 15:11:48.662767466 +
@@ -378,12 +378,10 @@ pass_return_slot::execute (function *fun
  if (stmt
  && gimple_call_lhs (stmt)
  && !gimple_call_return_slot_opt_p (stmt)
- /* Ignore internal functions without direct optabs,
-those are expanded specially and aggregate_value_p
-on their result might result in undesirable warnings
-with some backends.  */
- && (!gimple_call_internal_p (stmt)
- || direct_internal_fn_p (gimple_call_internal_fn (stmt)))
+ /* Ignore internal functions, those are expanded specially
+and aggregate_value_p on their result might result in
+undesirable warnings with some backends.  */
+ && !gimple_call_internal_p (stmt)
  && aggregate_value_p (TREE_TYPE (gimple_call_lhs (stmt)),
gimple_call_fndecl (stmt)))
{
Index: gcc/testsuite/gcc.target/aarch64/sve/acle/general/nrv_1.c
===
--- /dev/null   2019-09-17 11:41:18.176664108 +0100
+++ gcc/testsuite/gcc.target/aarch64/sve/acle/general/nrv_1.c   2019-12-12 
15:11:48.662767466 +
@@ -0,0 +1,17 @@
+/* { dg-options "-O -msve-vector-bits=256" } */
+
+#include 
+
+typedef uint8_t v32qi __attribute__((vector_size (32)));
+
+struct triple { v32qi v0, v1, v2; };
+
+struct triple f (uint8_t *ptr)
+{
+  svuint8x3_t data = svld3 (svptrue_b8 (), ptr);
+  struct triple res;
+  res.v0 = svget3 (data, 0);
+  res.v1 = svget3 (data, 1);
+  res.v2 = svget3 (data, 2);
+  return res;
+}


Add a compatible_vector_types_p target hook

2019-12-12 Thread Richard Sandiford
One problem with adding an N-bit vector extension to an existing
architecture is to decide how N-bit vectors should be passed to
functions and returned from functions.  Allowing all N-bit vector
types to be passed in registers breaks backwards compatibility,
since N-bit vectors could be used (and emulated) before the vector
extension was added.  But always passing N-bit vectors on the
stack would be inefficient for things like vector libm functions.

For SVE we took the compromise position of predefining new SVE vector
types that are distinct from all existing vector types, including
GNU-style vectors.  The new types are passed and returned in an
efficient way while existing vector types are passed and returned
in the traditional way.  In the right circumstances, the two types
are inter-convertible.

The SVE types are created using:

  vectype = build_distinct_type_copy (vectype);
  SET_TYPE_STRUCTURAL_EQUALITY (vectype);
  TYPE_ARTIFICIAL (vectype) = 1;

The C frontend maintains this distinction, using VIEW_CONVERT_EXPR
to convert from one type to the other.  However, the distinction can
be lost during gimple, which treats two vector types with the same
mode, number of elements, and element type as equivalent.  And for
most targets that's the right thing to do.

This patch therefore adds a hook that lets the target choose
whether such vector types are indeed equivalent.

Note that the new tests fail for -mabi=ilp32 in the same way as other
ACLE-based tests.  I'm still planning to fix that as a follow-on.

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

Richard


2019-12-12  Richard Sandiford  

gcc/
* target.def (compatible_vector_types_p): New target hook.
* hooks.h (hook_bool_const_tree_const_tree_true): Declare.
* hooks.c (hook_bool_const_tree_const_tree_true): New function.
* doc/tm.texi.in (TARGET_COMPATIBLE_VECTOR_TYPES_P): New hook.
* doc/tm.texi: Regenerate.
* gimple-expr.c: Include target.h.
(useless_type_conversion_p): Use targetm.compatible_vector_types_p.
* config/aarch64/aarch64.c (aarch64_compatible_vector_types_p): New
function.
(TARGET_COMPATIBLE_VECTOR_TYPES_P): Define.
* config/aarch64/aarch64-sve-builtins.cc (gimple_folder::convert_pred):
Use the original predicate if it already has a suitable type.

gcc/testsuite/
* gcc.target/aarch64/sve/pcs/gnu_vectors_1.c: New test.
* gcc.target/aarch64/sve/pcs/gnu_vectors_2.c: Likewise.

Index: gcc/target.def
===
--- gcc/target.def  2019-11-30 18:48:18.531984101 +
+++ gcc/target.def  2019-12-12 15:07:43.960415368 +
@@ -3411,6 +3411,29 @@ must have move patterns for this mode.",
  hook_bool_mode_false)
 
 DEFHOOK
+(compatible_vector_types_p,
+ "Return true if there is no target-specific reason for treating\n\
+vector types @var{type1} and @var{type2} as distinct types.  The caller\n\
+has already checked for target-independent reasons, meaning that the\n\
+types are known to have the same mode, to have the same number of elements,\n\
+and to have what the caller considers to be compatible element types.\n\
+\n\
+The main reason for defining this hook is to reject pairs of types\n\
+that are handled differently by the target's calling convention.\n\
+For example, when a new @var{N}-bit vector architecture is added\n\
+to a target, the target may want to handle normal @var{N}-bit\n\
+@code{VECTOR_TYPE} arguments and return values in the same way as\n\
+before, to maintain backwards compatibility.  However, it may also\n\
+provide new, architecture-specific @code{VECTOR_TYPE}s that are passed\n\
+and returned in a more efficient way.  It is then important to maintain\n\
+a distinction between the ``normal'' @code{VECTOR_TYPE}s and the new\n\
+architecture-specific ones.\n\
+\n\
+The default implementation returns true, which is correct for most targets.",
+ bool, (const_tree type1, const_tree type2),
+ hook_bool_const_tree_const_tree_true)
+
+DEFHOOK
 (vector_alignment,
  "This hook can be used to define the alignment for a vector of type\n\
 @var{type}, in order to comply with a platform ABI.  The default is to\n\
Index: gcc/hooks.h
===
--- gcc/hooks.h 2019-11-04 21:13:57.727755548 +
+++ gcc/hooks.h 2019-12-12 15:07:43.960415368 +
@@ -45,6 +45,7 @@ extern bool hook_bool_uint_uint_mode_fal
 extern bool hook_bool_uint_mode_true (unsigned int, machine_mode);
 extern bool hook_bool_tree_false (tree);
 extern bool hook_bool_const_tree_false (const_tree);
+extern bool hook_bool_const_tree_const_tree_true (const_tree, const_tree);
 extern bool hook_bool_tree_true (tree);
 extern bool hook_bool_const_tree_true (const_tree);
 extern bool hook_bool_gsiptr_false (gimple_stmt_iterator *);
Index: gcc/hooks.c

[PATCH] libstdc++: Simplify std::common_comparison_category

2019-12-12 Thread Jonathan Wakely
* libsupc++/compare (common_comparison_category): Define without using
concepts and optimise for compilation time.
(__detail::__cmp_cat_ids): Remove.
(__detail::__common_cmp_cat): Replace class template and
specializations with constexpr function.

Tested x86_64-linux, committed to trunk.


commit 70a1dc5fb4465642c00306d003164bdd9a5b8e08
Author: Jonathan Wakely 
Date:   Thu Dec 12 13:21:07 2019 +

libstdc++: Simplify std::common_comparison_category

* libsupc++/compare (common_comparison_category): Define without 
using
concepts and optimise for compilation time.
(__detail::__cmp_cat_ids): Remove.
(__detail::__common_cmp_cat): Replace class template and
specializations with constexpr function.

diff --git a/libstdc++-v3/libsupc++/compare b/libstdc++-v3/libsupc++/compare
index 412ec6861d3..f77b7d71e04 100644
--- a/libstdc++-v3/libsupc++/compare
+++ b/libstdc++-v3/libsupc++/compare
@@ -385,53 +385,43 @@ namespace std
   is_gteq(partial_ordering __cmp) noexcept
   { return __cmp >= 0; }
 
-#if __cpp_lib_concepts
   namespace __detail
   {
 template
   inline constexpr unsigned __cmp_cat_id = 1;
 template<>
-  inline constexpr unsigned __cmp_cat_id = 2;
+  inline constexpr unsigned __cmp_cat_id = 2;
 template<>
   inline constexpr unsigned __cmp_cat_id = 4;
 template<>
-  inline constexpr unsigned __cmp_cat_id = 8;
+  inline constexpr unsigned __cmp_cat_id = 8;
 
 template
-  constexpr unsigned __cmp_cat_ids()
-  { return (__cmp_cat_id<_Ts> | ...); }
-
-template
-  struct __common_cmp_cat;
-
-// If any Ti is not a comparison category type, U is void.
-template
-  requires ((_Bits & 1) == 1)
-  struct __common_cmp_cat<_Bits> { using type = void; };
-
-// Otherwise, if at least one Ti is std::partial_ordering,
-// U is std::partial_ordering.
-template
-  requires ((_Bits & 0b1001) == 0b1000)
-  struct __common_cmp_cat<_Bits> { using type = partial_ordering; };
-
-// Otherwise, if at least one Ti is std::weak_ordering,
-// U is std::weak_ordering.
-template
-  requires ((_Bits & 0b1101) == 0b0100)
-  struct __common_cmp_cat<_Bits> { using type = weak_ordering; };
-
-// Otherwise, U is std::strong_ordering.
-template<>
-  struct __common_cmp_cat<0b0010> { using type = strong_ordering; };
+  constexpr auto __common_cmp_cat()
+  {
+   constexpr unsigned __cats = (__cmp_cat_id<_Ts> | ...);
+   // If any Ti is not a comparison category type, U is void.
+   if constexpr (__cats & 1)
+ return;
+   // Otherwise, if at least one Ti is std::partial_ordering,
+   // U is std::partial_ordering.
+   else if constexpr (bool(__cats & __cmp_cat_id))
+ return partial_ordering::equivalent;
+   // Otherwise, if at least one Ti is std::weak_ordering,
+   // U is std::weak_ordering.
+   else if constexpr (bool(__cats & __cmp_cat_id))
+ return weak_ordering::equivalent;
+   // Otherwise, U is std::strong_ordering.
+   else
+ return strong_ordering::equivalent;
+  }
   } // namespace __detail
 
   // [cmp.common], common comparison category type
   template
 struct common_comparison_category
 {
-  using type
-   = __detail::__common_cmp_cat<__detail::__cmp_cat_ids<_Ts...>()>::type;
+  using type = decltype(__detail::__common_cmp_cat<_Ts...>());
 };
 
   // Partial specializations for one and zero argument cases.
@@ -460,6 +450,7 @@ namespace std
 using common_comparison_category_t
   = typename common_comparison_category<_Ts...>::type;
 
+#if __cpp_lib_concepts
   namespace __detail
   {
 template


Fix ipa-cp bit propagation streaming

2019-12-12 Thread Jan Hubicka
Hi,
this rather nasty bug makes value and mask to be exchanged during
streaming.  This makes us to sometimes set bogus pointer alignments
and causes misoptimization of Firefox when built with GCC 9.

Comitted as obvious.  I will backport it to release branches soon - it
is quite dangerous bug.

Honza

* ipa-prop.c (read_ipcp_transformation_info): Fix undefined ordering
of execution of function call parameters.
Index: ipa-prop.c
===
--- ipa-prop.c  (revision 278815)
+++ ipa-prop.c  (working copy)
@@ -4715,9 +4715,10 @@ read_ipcp_transformation_info (lto_input
  bool known = bp_unpack_value (, 1);
  if (known)
{
+ const widest_int value = streamer_read_widest_int (ib);
+ const widest_int mask = streamer_read_widest_int (ib);
  ipa_bits *bits
-   = ipa_get_ipa_bits_for_value (streamer_read_widest_int (ib),
- streamer_read_widest_int (ib));
+   = ipa_get_ipa_bits_for_value (value, mask);
  (*ts->bits)[i] = bits;
}
}


[modulo-sched][PATCH] Fix PR92591

2019-12-12 Thread Roman Zhuykov

Hello.

As pointed out in the PR 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92591#c1, the test can be 
fixed by DFA-checking more adjacent row sequences in the partial 
schedule.
I've found that on powerpc64 gcc.c-torture/execute/pr61682.c test 
catches same issue with -Os -fmodulo-sched-allow-regmoves with some 
non-zero sms-dfa-history parameter values, so I added that test using 
#include as second test into the patch.


Minor separate patch about modulo-sched parameters is also attached.
If no objection, I'll commit this two patches into trunk tomorrow 
together with my PR90001 fix.


Trunk and 8/9 branches succesfully regstrapped on x64, and 
cross-compiler check-gcc tested on ppc, ppc64, arm, aarch64, ia64 and 
s390. Certainly a lot of testing were also done with changing default 
sms-dfa-history value to some other than zero.


Roman

gcc/ChangeLog:

2019-12-10  Roman Zhuykov  

* modulo-sched.c (ps_add_node_check_conflicts): Improve checking
for history > 0 case.

gcc/testsuite/ChangeLog:

2019-12-10  Roman Zhuykov  

* gcc.dg/pr92951-1.c: New test.
* gcc.dg/pr92951-2.c: New test.


diff --git a/gcc/modulo-sched.c b/gcc/modulo-sched.c
--- a/gcc/modulo-sched.c
+++ b/gcc/modulo-sched.c
@@ -3200,7 +3200,7 @@ ps_add_node_check_conflicts (partial_schedule_ptr 
ps, int n,

 int c, sbitmap must_precede,
 sbitmap must_follow)
 {
-  int has_conflicts = 0;
+  int i, first, amount, has_conflicts = 0;
   ps_insn_ptr ps_i;

   /* First add the node to the PS, if this succeeds check for
@@ -3208,23 +3208,32 @@ ps_add_node_check_conflicts 
(partial_schedule_ptr ps, int n,

   if (! (ps_i = add_node_to_ps (ps, n, c, must_precede, must_follow)))
 return NULL; /* Failed to insert the node at the given cycle.  */

-  has_conflicts = ps_has_conflicts (ps, c, c)
- || (ps->history > 0
- && ps_has_conflicts (ps,
-  c - ps->history,
-  c + ps->history));
-
-  /* Try different issue slots to find one that the given node can be
- scheduled in without conflicts.  */
-  while (has_conflicts)
+  while (1)
 {
+  has_conflicts = ps_has_conflicts (ps, c, c);
+  if (ps->history > 0 && !has_conflicts)
+   {
+ /* Check all 2h+1 intervals, starting from c-2h..c up to c..2h,
+but not more than ii intervals.  */
+ first = c - ps->history;
+ amount = 2 * ps->history + 1;
+ if (amount > ps->ii)
+   amount = ps->ii;
+ for (i = first; i < first + amount; i++)
+   {
+ has_conflicts = ps_has_conflicts (ps,
+   i - ps->history,
+   i + ps->history);
+ if (has_conflicts)
+   break;
+   }
+   }
+  if (!has_conflicts)
+   break;
+  /* Try different issue slots to find one that the given node can 
be

+scheduled in without conflicts.  */
   if (! ps_insn_advance_column (ps, ps_i, must_follow))
break;
-  has_conflicts = ps_has_conflicts (ps, c, c)
- || (ps->history > 0
- && ps_has_conflicts (ps,
-  c - ps->history,
-  c + ps->history));
 }

   if (has_conflicts)
diff --git a/gcc/testsuite/gcc.dg/pr92951-1.c 
b/gcc/testsuite/gcc.dg/pr92951-1.c

--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr92951-1.c
@@ -0,0 +1,11 @@
+/* PR rtl-optimization/92591 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fmodulo-sched -fweb -fno-dce -fno-ivopts 
-fno-sched-pressure -fno-tree-loop-distribute-patterns --param 
sms-dfa-history=1" } */
+/* { dg-additional-options "-mcpu=e500mc" { target { powerpc-*-* } } } 
*/

+
+void
+wf (char *mr, int tc)
+{
+  while (tc-- > 0)
+*mr++ = 0;
+}
diff --git a/gcc/testsuite/gcc.dg/pr92951-2.c 
b/gcc/testsuite/gcc.dg/pr92951-2.c

--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr92951-2.c
@@ -0,0 +1,5 @@
+/* PR rtl-optimization/92591 */
+/* { dg-do compile } */
+/* { dg-options "-Os -fmodulo-sched -fmodulo-sched-allow-regmoves 
--param sms-dfa-history=8" } */

+
+#include "../gcc.c-torture/execute/pr61682.c"
gcc/ChangeLog:

2019-11-26  Roman Zhuykov  

* modulo-sched.c (sms_schedule): Use param_sms_max_ii_factor
value instead of macro.  Adjust comment.
(sms_schedule_by_order): Use parameter value without macro.
* params.opt: Add ranges for modulo scheduler parameters,
set param_sms_max_ii_factor = 2 by default.

diff --git a/gcc/modulo-sched.c b/gcc/modulo-sched.c
index 98af549..2dc9af7 100644
--- a/gcc/modulo-sched.c
+++ b/gcc/modulo-sched.c
@@ -1331,9 +1331,6 @@ setup_sched_infos (void)
version may be entered.  Just a guess.  */
 #define PROB_SMS_ENOUGH_ITERATIONS 80
 
-/* Used to calculate the 

[PATCH] OpenACC device-pointer lookup with globally-mapped variables (PR92888)

2019-12-12 Thread Julian Brown
Hi,

This patch provides a fix for PR92888, wherein global variables mapped
using an OpenACC 'declare' directive would not be visible to
device-pointer lookups.

Tested with offloading to nvptx. OK?

Thanks,

Julian

ChangeLog

2019-12-12  Julian Brown  

PR libgomp/92888

libgomp/
* oacc-parallel.c (GOACC_parallel_keyed): Add tgt_start in target
function address calculation.
* target.c (gomp_load_image_to_device): Record address range for
target_mem_desc for mapped functions and global variables, and adjust
tgt_offsets to be within that range.
(gomp_get_target_fn_addr): Add tgt_start in target function address
calculation.
* testsuite/libgomp.oacc-c-c++-common/acc_map_data-device_already-3.c:
Remove XFAIL.
commit 16e774d2ce86af90ff282b9126cf615e66e7efae
Author: Julian Brown 
Date:   Mon Dec 9 11:04:58 2019 -0800

Find address range for offloaded functions and global variables (PR92888)

PR libgomp/92888

libgomp/
* oacc-parallel.c (GOACC_parallel_keyed): Add tgt_start in target
function address calculation.
* target.c (gomp_load_image_to_device): Record address range for
target_mem_desc for mapped functions and global variables, and adjust
offsets to be within that range.
(gomp_get_target_fn_addr): Add tgt_start in target function address
calculation.
* testsuite/libgomp.oacc-c-c++-common/acc_map_data-device_already-3.c:
Remove XFAIL.

diff --git a/libgomp/oacc-parallel.c b/libgomp/oacc-parallel.c
index f5ef5050bbd..5a5697cf6e6 100644
--- a/libgomp/oacc-parallel.c
+++ b/libgomp/oacc-parallel.c
@@ -377,7 +377,7 @@ GOACC_parallel_keyed (int flags_m, void (*fn) (void *),
   if (tgt_fn_key == NULL)
 	gomp_fatal ("target function wasn't mapped");
 
-  tgt_fn = (void (*)) tgt_fn_key->tgt_offset;
+  tgt_fn = (void (*)) (tgt_fn_key->tgt->tgt_start + tgt_fn_key->tgt_offset);
 }
   else
 tgt_fn = (void (*)) fn;
diff --git a/libgomp/target.c b/libgomp/target.c
index bb392dd1c8f..b023e3daf1a 100644
--- a/libgomp/target.c
+++ b/libgomp/target.c
@@ -1759,6 +1759,8 @@ gomp_load_image_to_device (struct gomp_device_descr *devicep, unsigned version,
   tgt->device_descr = devicep;
   splay_tree_node array = tgt->array;
 
+  uintptr_t max_addr = 0, min_addr = ~(uintptr_t) 0;
+
   for (i = 0; i < num_funcs; i++)
 {
   splay_tree_key k = >key;
@@ -1766,6 +1768,10 @@ gomp_load_image_to_device (struct gomp_device_descr *devicep, unsigned version,
   k->host_end = k->host_start + 1;
   k->tgt = tgt;
   k->tgt_offset = target_table[i].start;
+  if (target_table[i].start < min_addr)
+	min_addr = target_table[i].start;
+  if (target_table[i].end > max_addr)
+	max_addr = target_table[i].end;
   k->refcount = REFCOUNT_INFINITY;
   k->virtual_refcount = 0;
   k->aux = NULL;
@@ -1799,6 +1805,10 @@ gomp_load_image_to_device (struct gomp_device_descr *devicep, unsigned version,
 	= k->host_start + (size_mask & (uintptr_t) host_var_table[i * 2 + 1]);
   k->tgt = tgt;
   k->tgt_offset = target_var->start;
+  if (target_var->start < min_addr)
+	min_addr = target_var->start;
+  if (target_var->end > max_addr)
+	max_addr = target_var->end;
   k->refcount = target_size & link_bit ? REFCOUNT_LINK : REFCOUNT_INFINITY;
   k->virtual_refcount = 0;
   k->aux = NULL;
@@ -1808,6 +1818,17 @@ gomp_load_image_to_device (struct gomp_device_descr *devicep, unsigned version,
   array++;
 }
 
+  /* Make the tgt_mem_desc cover all of the functions and variables so that
+ oacc-mem.c:lookup_dev can find mapped global variables properly.  */
+  tgt->tgt_start = min_addr;
+  tgt->tgt_end = max_addr;
+
+  for (array = tgt->array, i = 0; i < num_vars + num_funcs; i++, array++)
+{
+  splay_tree_key k = >key;
+  k->tgt_offset -= min_addr;
+}
+
   free (target_table);
 }
 
@@ -2170,7 +2191,7 @@ gomp_get_target_fn_addr (struct gomp_device_descr *devicep,
   if (tgt_fn == NULL)
 	return NULL;
 
-  return (void *) tgt_fn->tgt_offset;
+  return (void *) (tgt_fn->tgt->tgt_start + tgt_fn->tgt_offset);
 }
 }
 
diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_map_data-device_already-3.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_map_data-device_already-3.c
index 7cd2936219a..0807bc9d694 100644
--- a/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_map_data-device_already-3.c
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_map_data-device_already-3.c
@@ -24,5 +24,5 @@ main ()
 
 
 /* { dg-output "CheCKpOInT(\n|\r\n|\r).*" } */
-/* { dg-output "device address \\\[\[0-9a-fA-FxX\]+, \\\+8\\\] is already mapped" { xfail *-*-* } } TODO */
-/* { dg-shouldfail "TODO" { INV-AL-ID } } */
+/* { dg-output "device address \\\[\[0-9a-fA-FxX\]+, \\\+8\\\] is already mapped" } */
+/* { dg-shouldfail "" } */
diff --git 

[Ada] Handling up-level references in protected entries and freeze nodes

2019-12-12 Thread Pierre-Marie de Rodat
For GNAT-LLVM, up-level references within the procedure created for a
protected entry were not being handled right inside the block created to
hold the declarations and statements of the entry. Calls to procedures
declared within the block that make up-level references (such as to the
init proc for a task declared within the entry, as occurred in one test)
weren't passing an AREC parameter. Part of the fix is to analyze the
block and reset Scope fields on declarations within the block right
after the block is created (in Build_Protected_Entry), so that they
refer to the block rather than the original entry declaration. However,
there were still cases where subprograms occurring within an
N_Freeze_Entity weren't getting their scope set. This is addressed by
traversing the Actions list of the freeze node, recursively calling
Reset_Scopes on each of the actions, plus also adding specific handling
for subprogram bodies, ensuring that their entity's Scope is reset
(deals with cases where there is not a separate subprogram declaration).

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

2019-12-12  Gary Dismukes  

gcc/ada/

* exp_ch9.adb (Build_Protected_Entry): Analyze the block created
to hold the declarations and statements of the protected entry
body right after it's created, and then call Reset_Scopes_To on
that block to reset the Scope of nested entities to the block
scope.
(Reset_Scope): Add handling for N_Freeze_Entity nodes, calling
Reset_Scopes recursively on the Actions of such nodes. Also, for
subprogram bodies that are encountered that might not have a
separate declaration (such as type init procedures), reset the
Scope of the subprogram's entity.--- gcc/ada/exp_ch9.adb
+++ gcc/ada/exp_ch9.adb
@@ -48,6 +48,7 @@ with Rident;   use Rident;
 with Rtsfind;  use Rtsfind;
 with Sem;  use Sem;
 with Sem_Aux;  use Sem_Aux;
+with Sem_Ch5;  use Sem_Ch5;
 with Sem_Ch6;  use Sem_Ch6;
 with Sem_Ch8;  use Sem_Ch8;
 with Sem_Ch9;  use Sem_Ch9;
@@ -3722,6 +3723,14 @@ package body Exp_Ch9 is
 Declarations   => Decls,
 Handled_Statement_Sequence => Handled_Statement_Sequence (N)));
 
+  --  Analyze now and reset scopes for declarations so that Scope fields
+  --  currently denoting the entry will now denote the block scope.
+
+  Analyze_Statements (Bod_Stmts);
+
+  Reset_Scopes_To
+(First (Bod_Stmts), Entity (Identifier (First (Bod_Stmts;
+
   case Corresponding_Runtime_Package (Pid) is
  when System_Tasking_Protected_Objects_Entries =>
 Append_To (Bod_Stmts,
@@ -14977,7 +14986,27 @@ package body Exp_Ch9 is
Next (Decl);
 end loop;
 
+ elsif Nkind (N) = N_Freeze_Entity then
+
+--  Scan the actions associated with a freeze node, which may
+--  actually be declarations with entities that need to have
+--  their scopes reset.
+
+Decl := First (Actions (N));
+while Present (Decl) loop
+   Reset_Scopes (Decl);
+   Next (Decl);
+end loop;
+
  elsif N /= Bod and then Nkind (N) in N_Proper_Body then
+
+--  A subprogram without a separate declaration may be encountered,
+--  and we need to reset the subprogram's entity's scope.
+
+if Nkind (N) = N_Subprogram_Body then
+   Set_Scope (Defining_Entity (Specification (N)), E);
+end if;
+
 return Skip;
  end if;
 



[Ada] Fix wrong value of 'Size for slices of bit-packed arrays (2)

2019-12-12 Thread Pierre-Marie de Rodat
This completes the previous fix, which is not sufficient when the Size
attribute is applied to a formal object of mode In Out and the generic
is instantiated on a slice of a bit-packed array whose size is not a
multiple of the storage unit.

The hurdle here is that introducing a temporary may change the value
of the Size attribute, because a temporary is a stand-alone object and
the size of stand-alone objects is always a multiple of the storage
unit with GNAT.

Instead of trying to look through them in Expand_Size_Attribute, this
changes the expansion done by Expand_N_Slice to avoid creating these
problematic temporaries for slices of bit-packed arrays that are the
prefix of the Size attribute.

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

2019-12-12  Eric Botcazou  

gcc/ada/

* exp_attr.adb (Expand_Size_Attribute): Look directly at the
prefix to detect the bit-packed slices.  Apply the checks last
in case the attribute needs to be processed by the back-end.
* exp_ch4.adb (Expand_N_Slice): Do not create a temporary for
a prefix of the Size attribute.--- gcc/ada/exp_attr.adb
+++ gcc/ada/exp_attr.adb
@@ -7455,8 +7455,6 @@ package body Exp_Attr is
   --  All other cases are handled by the back end
 
   else
- Apply_Universal_Integer_Attribute_Checks (N);
-
  --  If Size is applied to a formal parameter that is of a packed
  --  array subtype, then apply Size to the actual subtype.
 
@@ -7489,9 +7487,7 @@ package body Exp_Attr is
  --  System.Unsigned_Types.Packed_Byte for code generation purposes so
  --  the size is always rounded up in the back end.
 
- elsif Nkind (Original_Node (Pref)) = N_Slice
-   and then Is_Bit_Packed_Array (Ptyp)
- then
+ elsif Nkind (Pref) = N_Slice and then Is_Bit_Packed_Array (Ptyp) then
 Rewrite (N,
   Make_Op_Multiply (Loc,
 Make_Attribute_Reference (Loc,
@@ -7503,6 +7499,9 @@ package body Exp_Attr is
 Analyze_And_Resolve (N, Typ);
  end if;
 
+ --  Apply the required checks last, after rewriting has taken place
+
+ Apply_Universal_Integer_Attribute_Checks (N);
  return;
   end if;
 

--- gcc/ada/exp_ch4.adb
+++ gcc/ada/exp_ch4.adb
@@ -11013,7 +11013,8 @@ package body Exp_Ch4 is
 
   --5. Prefix of an address attribute (this is an error which is caught
   --   elsewhere, and the expansion would interfere with generating the
-  --   error message).
+  --   error message) or of a size attribute (because 'Size may change
+  --   when applied to the temporary instead of the slice directly).
 
   if not Is_Packed (Typ) then
 
@@ -11039,7 +11040,8 @@ package body Exp_Ch4 is
  return;
 
   elsif Nkind (Parent (N)) = N_Attribute_Reference
-and then Attribute_Name (Parent (N)) = Name_Address
+and then (Attribute_Name (Parent (N)) = Name_Address
+   or else Attribute_Name (Parent (N)) = Name_Size)
   then
  return;
 



[Ada] Fix Global contract for the predefined Yield procedure

2019-12-12 Thread Pierre-Marie de Rodat
In GNATprove flow analysis is not aware of implicitly evaluated
routines, e.g. subtype predicates and user-defined equality for record
types.  Therefore SPARK 2014 mandates both routines to not reference any
globals.  Likewise, we don't want them to have any effects related to
tasking, i.e.  we don't want them to be potentially blocking or call
protected routines (which might violate checks for the ceiling-priority
protocol).

The cleanest way to enforce this is to require all potentially blocking
operation to reference Ada.Task_Identification.Tasking_State, just like
the delay statements do.

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

2019-12-12  Piotr Trojanek  

gcc/ada/

* libgnarl/a-dispat.ads (Yield): Update Global contract.--- gcc/ada/libgnarl/a-dispat.ads
+++ gcc/ada/libgnarl/a-dispat.ads
@@ -13,11 +13,13 @@
 --  --
 --
 
+with Ada.Task_Identification;
+
 package Ada.Dispatching is
pragma Preelaborate (Dispatching);
 
procedure Yield with
- Global => null;
+ Global => (In_Out => Ada.Task_Identification.Tasking_State);
 
Dispatching_Policy_Error : exception;
 end Ada.Dispatching;



[Ada] Fix related to handling up-level references in protected entries

2019-12-12 Thread Pierre-Marie de Rodat
This change fixes a regression that occurred on a Ravenscar test, where
gigi blew up because an itype inside the block created for a protected
entry procedure had its scope set incorrectly (designating the enclosing
package) so appeared to gigi not to be declared within the procedure.
The scope of the block is now set to designate the entry procedure's
entity.

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

2019-12-12  Gary Dismukes  

gcc/ada/

* exp_ch9.adb (Build_Protected_Entry): Set the Scope of the new
block to be the entity of the procedure created for the entry.--- gcc/ada/exp_ch9.adb
+++ gcc/ada/exp_ch9.adb
@@ -3724,10 +3724,13 @@ package body Exp_Ch9 is
 Handled_Statement_Sequence => Handled_Statement_Sequence (N)));
 
   --  Analyze now and reset scopes for declarations so that Scope fields
-  --  currently denoting the entry will now denote the block scope.
+  --  currently denoting the entry will now denote the block scope, and
+  --  the block's scope will be set to the new procedure entity.
 
   Analyze_Statements (Bod_Stmts);
 
+  Set_Scope (Entity (Identifier (First (Bod_Stmts))), Bod_Id);
+
   Reset_Scopes_To
 (First (Bod_Stmts), Entity (Identifier (First (Bod_Stmts;
 



[Ada] Missing length check on private type with unknown discriminants

2019-12-12 Thread Pierre-Marie de Rodat
Compiler fails to emit a length check on the right-hand side of an
assignment when the type involved is a private type with unknown
discriminants whose full view is an unconstrained array type.

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

2019-12-12  Ed Schonberg  

gcc/ada/

* exp_ch5.adb (Expand_N_Assognment_Statement): Extend the
processing involving private types with unknown discriminants to
handle the case where the full view of the type is an
unconstrained array type.--- gcc/ada/exp_ch5.adb
+++ gcc/ada/exp_ch5.adb
@@ -2409,14 +2409,23 @@ package body Exp_Ch5 is
   --  checking. Convert Lhs as well, otherwise the actual subtype might
   --  not be constructible. If the discriminants have defaults the type
   --  is unconstrained and there is nothing to check.
+  --  Ditto if a private type with unknown discriminants has a full view
+  --  that is an unconstrained array, in which case a length check is
+  --  needed.
 
-  elsif Has_Unknown_Discriminants (Base_Type (Etype (Lhs)))
-and then Has_Discriminants (Typ)
-and then not Has_Defaulted_Discriminants (Typ)
-  then
- Rewrite (Rhs, OK_Convert_To (Base_Type (Typ), Rhs));
- Rewrite (Lhs, OK_Convert_To (Base_Type (Typ), Lhs));
- Apply_Discriminant_Check (Rhs, Typ, Lhs);
+  elsif Has_Unknown_Discriminants (Base_Type (Etype (Lhs))) then
+ if Has_Discriminants (Typ)
+   and then not Has_Defaulted_Discriminants (Typ)
+ then
+Rewrite (Rhs, OK_Convert_To (Base_Type (Typ), Rhs));
+Rewrite (Lhs, OK_Convert_To (Base_Type (Typ), Lhs));
+Apply_Discriminant_Check (Rhs, Typ, Lhs);
+
+ elsif Is_Array_Type (Typ) and then Is_Constrained (Typ)  then
+Rewrite (Rhs, OK_Convert_To (Base_Type (Typ), Rhs));
+Rewrite (Lhs, OK_Convert_To (Base_Type (Typ), Lhs));
+Apply_Length_Check (Rhs, Typ);
+ end if;
 
   --  In the access type case, we need the same discriminant check, and
   --  also range checks if we have an access to constrained array.



[Ada] Fix repeated words and typos in doc and comments

2019-12-12 Thread Pierre-Marie de Rodat
Found with 'grep " \([[:alpha:]]\+\) \1[[:space:]]" *.ad?', just like in
2015; fixed manually with refilling comments where possible.

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

2019-12-12  Piotr Trojanek  

gcc/ada/

* libgnat/g-altive.ads: Fix typo in comment.
* bindo-graphs.adb: Fix repeated words in comment.
* exp_ch4.adb: Likewise.
* exp_ch5.adb: Likewise.
* exp_ch7.adb: Likewise.
* exp_pakd.adb: Likewise.
* exp_unst.adb: Likewise.
* exp_util.adb: Likewise.
* freeze.adb: Likewise.
* inline.adb: Likewise.
* layout.adb: Likewise.
* sem_ch12.adb: Likewise.
* sem_ch13.adb: Likewise.
* sem_ch4.adb: Likewise.
* sem_ch9.adb: Likewise.
* sem_elab.adb: Likewise.
* doc/gnat_ugn/gnat_and_program_execution.rst: Fix repeated
words in user documentation.
* gnat_ugn.texi: Regenerate.--- gcc/ada/bindo-graphs.adb
+++ gcc/ada/bindo-graphs.adb
@@ -1676,7 +1676,7 @@ package body Bindo.Graphs is
  --  successor and predecessor are kept consistent in both cases, and
  --  Add_Edge_With_Return will prevent the creation of the second edge.
 
- --  Assume that that no Body_Before_Spec is necessary
+ --  Assume that no Body_Before_Spec is necessary
 
  Edge := No_Library_Graph_Edge;
 

--- gcc/ada/doc/gnat_ugn/gnat_and_program_execution.rst
+++ gcc/ada/doc/gnat_ugn/gnat_and_program_execution.rst
@@ -1214,8 +1214,7 @@ for more information.
 Profiling
 =
 
-This section describes how to use the the ``gprof`` profiler tool on Ada
-programs.
+This section describes how to use the ``gprof`` profiler tool on Ada programs.
 
 .. index:: !  gprof
 .. index:: Profiling

--- gcc/ada/exp_ch4.adb
+++ gcc/ada/exp_ch4.adb
@@ -11840,7 +11840,7 @@ package body Exp_Ch4 is
--  The case where the target type is an anonymous access type of
--  a discriminant is excluded, because the level of such a type
--  depends on the context and currently the level returned for such
-   --  types is zero, resulting in warnings about about check failures
+   --  types is zero, resulting in warnings about check failures
--  in certain legal cases involving class-wide interfaces as the
--  designated type (some cases, such as return statements, are
--  checked at run time, but not clear if these are handled right
@@ -12320,8 +12320,8 @@ package body Exp_Ch4 is
 
--  Remove the unchecked expression node from the tree. Its job was simply
--  to make sure that its constituent expression was handled with checks
-   --  off, and now that that is done, we can remove it from the tree, and
-   --  indeed must, since Gigi does not expect to see these nodes.
+   --  off, and now that is done, we can remove it from the tree, and indeed
+   --  must, since Gigi does not expect to see these nodes.
 
procedure Expand_N_Unchecked_Expression (N : Node_Id) is
   Exp : constant Node_Id := Expression (N);

--- gcc/ada/exp_ch5.adb
+++ gcc/ada/exp_ch5.adb
@@ -1065,8 +1065,8 @@ package body Exp_Ch5 is
end if;
 
--  Reset the Analyzed flag, because the bounds of the index
-   --  type itself may be universal, and must must be reanalyzed
-   --  to acquire the proper type for the back end.
+   --  type itself may be universal, and must be reanalyzed to
+   --  acquire the proper type for the back end.
 
Set_Analyzed (Cleft_Lo, False);
Set_Analyzed (Cright_Lo, False);

--- gcc/ada/exp_ch7.adb
+++ gcc/ada/exp_ch7.adb
@@ -376,7 +376,7 @@ package body Exp_Ch7 is
 
procedure Check_Unnesting_In_Decls_Or_Stmts (Decls_Or_Stmts : List_Id);
--  Similarly, the declarations or statements in library-level packages may
-   --  have created blocks blocks with nested subprograms. Such a block must be
+   --  have created blocks with nested subprograms. Such a block must be
--  transformed into a procedure followed by a call to it, so that unnesting
--  can handle uplevel references within these nested subprograms (typically
--  subprograms that handle finalization actions). This also applies to

--- gcc/ada/exp_pakd.adb
+++ gcc/ada/exp_pakd.adb
@@ -1564,7 +1564,7 @@ package body Exp_Pakd is
  Silly_Boolean_Array_Xor_Test (N, R, Rtyp);
   end if;
 
-  --  Now that that silliness is taken care of, get packed array type
+  --  Now that silliness is taken care of, get packed array type
 
   Convert_To_PAT_Type (L);
   Convert_To_PAT_Type (R);

--- gcc/ada/exp_unst.adb
+++ gcc/ada/exp_unst.adb
@@ -526,8 +526,8 @@ package body Exp_Unst is
procedure Note_Uplevel_Bound (N : Node_Id; Ref : Node_Id) is
begin
   --  Entity name case. Make sure that the entity is declared
-  --  in a 

[Ada] Compiler crash on prefix call in generic body

2019-12-12 Thread Pierre-Marie de Rodat
This patch fixes the following bug: If prefix notation is used to call a
subprogram, and the call is within a generic package body that is within
a package body P, and the called subprogram is not declared in the spec
of P, the compiler crashes when compiling an instance of the generic
package.

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

2019-12-12  Bob Duff  

gcc/ada/

* sem_ch4.adb (Transform_Object_Operation): Deal properly with
prefix notation in instances.--- gcc/ada/sem_ch4.adb
+++ gcc/ada/sem_ch4.adb
@@ -8574,7 +8574,7 @@ package body Sem_Ch4 is
   procedure Transform_Object_Operation
 (Call_Node   : out Node_Id;
  Node_To_Replace : out Node_Id);
-  --  Transform Obj.Operation (X, Y,,) into Operation (Obj, X, Y ..)
+  --  Transform Obj.Operation (X, Y, ...) into Operation (Obj, X, Y ...).
   --  Call_Node is the resulting subprogram call, Node_To_Replace is
   --  either N or the parent of N, and Subprog is a reference to the
   --  subprogram we are trying to match.
@@ -9299,7 +9299,7 @@ package body Sem_Ch4 is
  --  Prefix notation can also be used on operations that are not
  --  primitives of the type, but are declared in the same immediate
  --  declarative part, which can only mean the corresponding package
- --  body (See RM 4.1.3 (9.2/3)). If we are in that body we extend the
+ --  body (see RM 4.1.3 (9.2/3)). If we are in that body we extend the
  --  list of primitives with body operations with the same name that
  --  may be candidates, so that Try_Primitive_Operations can examine
  --  them if no real primitive is found.
@@ -9425,56 +9425,55 @@ package body Sem_Ch4 is
 
  function Extended_Primitive_Ops (T : Entity_Id) return Elist_Id is
 Type_Scope : constant Entity_Id := Scope (T);
-
-Body_Decls : List_Id;
-Op_Found   : Boolean;
-Op : Entity_Id;
-Op_List: Elist_Id;
-
+Op_List: Elist_Id := Primitive_Operations (T);
  begin
-Op_List := Primitive_Operations (T);
-
-if Ekind (Type_Scope) = E_Package
-  and then In_Package_Body (Type_Scope)
-  and then In_Open_Scopes (Type_Scope)
+if Ekind_In (Type_Scope, E_Package, E_Generic_Package)
+  and then ((In_Package_Body (Type_Scope)
+  and then In_Open_Scopes (Type_Scope)) or else In_Instance_Body)
 then
-   --  Retrieve list of declarations of package body.
-
-   Body_Decls :=
- Declarations
-   (Unit_Declaration_Node
- (Corresponding_Body
-   (Unit_Declaration_Node (Type_Scope;
-
-   Op   := Current_Entity (Subprog);
-   Op_Found := False;
-   while Present (Op) loop
-  if Comes_From_Source (Op)
-and then Is_Overloadable (Op)
-
---  Exclude overriding primitive operations of a type
---  extension declared in the package body, to prevent
---  duplicates in extended list.
-
-and then not Is_Primitive (Op)
-and then Is_List_Member (Unit_Declaration_Node (Op))
-and then List_Containing (Unit_Declaration_Node (Op)) =
-   Body_Decls
-  then
- if not Op_Found then
+   --  Retrieve list of declarations of package body if possible
 
---  Copy list of primitives so it is not affected for
---  other uses.
+   declare
+  The_Body : constant Node_Id :=
+Corresponding_Body (Unit_Declaration_Node (Type_Scope));
+   begin
+  if Present (The_Body) then
+ declare
+Body_Decls : constant List_Id :=
+  Declarations (Unit_Declaration_Node (The_Body));
+Op_Found : Boolean := False;
+Op : Entity_Id := Current_Entity (Subprog);
+ begin
+while Present (Op) loop
+   if Comes_From_Source (Op)
+ and then Is_Overloadable (Op)
+
+ --  Exclude overriding primitive operations of a
+ --  type extension declared in the package body,
+ --  to prevent duplicates in extended list.
+
+ and then not Is_Primitive (Op)
+ and then Is_List_Member
+   (Unit_Declaration_Node (Op))
+ and then List_Containing
+   

[Ada] Missing error on incorrect use of Result attribute

2019-12-12 Thread Pierre-Marie de Rodat
This patch fixes an issue whereby the compiler failed to issue an error
on the use of the attribute Result when the prefix is a non-subprogram.

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

2019-12-12  Justin Squirek  

gcc/ada/

* sem_attr.adb (Analyze_Attribute): Add error message for
invalid usage of Attribute_Result.--- gcc/ada/sem_attr.adb
+++ gcc/ada/sem_attr.adb
@@ -5414,6 +5414,7 @@ package body Sem_Attr is
 Spec_Id := Entity (P);
 
  elsif not Legal then
+Error_Attr ("prefix of % attribute must be a function", P);
 return;
  end if;
 



[Ada] Crash on Descriptor_Size attribute

2019-12-12 Thread Pierre-Marie de Rodat
This patch fixes the following bug: If the Descriptor_Size attribute is
used in the private part of a generic package, then the compiler crashes
when compiling an instance of that generic.

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

2019-12-12  Bob Duff  

gcc/ada/

* sem_attr.adb (Eval_Attribute): Never mark T'Descriptor_Size as
static, even if T is a static subtype, because otherwise we will
request the value of the attribute, which will crash because we
have not evaluated it.--- gcc/ada/sem_attr.adb
+++ gcc/ada/sem_attr.adb
@@ -7852,6 +7852,8 @@ package body Sem_Attr is
 
   --  is legal, since here this expression appears in a statically
   --  unevaluated position, so it does not actually raise an exception.
+  --
+  --  T'Descriptor_Size is never static, even if T is static.
 
   if Is_Scalar_Type (P_Entity)
 and then (not Is_Generic_Type (P_Entity))
@@ -7865,6 +7867,7 @@ package body Sem_Attr is
   (No (E2)
 or else (Is_Static_Expression (E2)
   and then Is_Scalar_Type (Etype (E1
+and then Id /= Attribute_Descriptor_Size
   then
  Static := True;
  Set_Is_Static_Expression (N, True);



[Ada] Tighten up semantic checking for protected subprogram declarations

2019-12-12 Thread Pierre-Marie de Rodat
The B940010 ACATS test includes some legality violations that GNAT was
failing to reject (at compile time).  With this change these violations
are detected and appropriate error messages are produced. Most of the
required error messages that are not generated initially are because
splitting is required - that is a separate issue. Even after appropriate
splitting, the compiler was failing to detect the violations associated
with the L and N procedures for types Protected_3 and Protected_5.

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

2019-12-12  Steve Baird  

gcc/ada/

* sem_ch6.adb
(New_Overloaded_Entity.Check_Conforming_Paramters): Add new
Conformance_Type parameter. With the value of
Subtype_Conformant, the behavior of Check_Conforming_Parameters
is unchanged.  The call in Matching_Entry_Or_Subprogram to
instead passes in Type_Conformant. This corresponds to the use
of "type conformant" in Ada RM 9.4(11.4/3).
(New_Overloaded_Entity.Has_Matching_Entry_Or_Subprogram): Add
new Normalized_First_Parameter_Type function to help in ignoring
the distinction between protected and access-to-protected first
parameters when checking prefixed-view profile matching. Replace
computations of the type of the first parameter with calls to
this function as appropriate.--- gcc/ada/sem_ch6.adb
+++ gcc/ada/sem_ch6.adb
@@ -10487,9 +10487,10 @@ package body Sem_Ch6 is
   is
  function Check_Conforming_Parameters
(E1_Param : Node_Id;
-E2_Param : Node_Id) return Boolean;
+E2_Param : Node_Id;
+Ctype: Conformance_Type) return Boolean;
  --  Starting from the given parameters, check that all the parameters
- --  of two entries or subprograms are subtype conformant. Used to skip
+ --  of two entries or subprograms are conformant. Used to skip
  --  the check on the controlling argument.
 
  function Matching_Entry_Or_Subprogram
@@ -10516,26 +10517,38 @@ package body Sem_Ch6 is
  --  whose name matches the original name of Subp and has a profile
  --  conformant with the profile of Subp; return Empty if not found.
 
+ function Normalized_First_Parameter_Type
+   (E : Entity_Id) return Entity_Id;
+ --  Return the type of the first parameter unless that type
+ --  is an anonymous access type, in which case return the
+ --  designated type. Used to treat anonymous-access-to-synchronized
+ --  the same as synchronized for purposes of checking for
+ --  prefixed view profile conflicts.
+
  -
  -- Check_Conforming_Parameters --
  -
 
  function Check_Conforming_Parameters
(E1_Param : Node_Id;
-E2_Param : Node_Id) return Boolean
+E2_Param : Node_Id;
+Ctype: Conformance_Type) return Boolean
  is
 Param_E1 : Node_Id := E1_Param;
 Param_E2 : Node_Id := E2_Param;
 
  begin
 while Present (Param_E1) and then Present (Param_E2) loop
-   if Ekind (Defining_Identifier (Param_E1)) /=
-Ekind (Defining_Identifier (Param_E2))
- or else not
+   if (Ctype >= Mode_Conformant) and then
+ Ekind (Defining_Identifier (Param_E1)) /=
+ Ekind (Defining_Identifier (Param_E2))
+   then
+  return False;
+   elsif not
Conforming_Types
  (Find_Parameter_Type (Param_E1),
   Find_Parameter_Type (Param_E2),
-  Subtype_Conformant)
+  Ctype)
then
   return False;
end if;
@@ -10568,7 +10581,8 @@ package body Sem_Ch6 is
  and then
Check_Conforming_Parameters
  (First (Parameter_Specifications (Parent (E))),
-  Next (First (Parameter_Specifications (Parent (Subp)
+  Next (First (Parameter_Specifications (Parent (Subp,
+  Type_Conformant)
then
   return E;
end if;
@@ -10608,7 +10622,8 @@ package body Sem_Ch6 is
  and then
Check_Conforming_Parameters
  (First (Parameter_Specifications (Parent (Ent))),
-  Next (First (Parameter_Specifications (Parent (E)
+  Next (First (Parameter_Specifications (Parent (E,
+  Subtype_Conformant)
then
   return E;
end if;
@@ -10662,6 +10677,21 @@ package body Sem_Ch6 is
 return Empty;
  end Matching_Original_Protected_Subprogram;
 
+   

[Ada] Broken privacy on Controlled type extensions

2019-12-12 Thread Pierre-Marie de Rodat
This patch fixes an issue whereby the compiler incorrectly resolves
non-visible controlled primitives such as the case where predefined
controlled operations get called on a type extension whose parent is
a private extension completed with a controlled extension.

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

2019-12-12  Justin Squirek  

gcc/ada/

* sem_ch4.adb (Analyze_One_Call): Add condition to check for
incorrectly resolved hidden controlled primitives.--- gcc/ada/sem_ch4.adb
+++ gcc/ada/sem_ch4.adb
@@ -3249,6 +3249,7 @@ package body Sem_Ch4 is
   --  is already known to be compatible, and because this may be an
   --  indexing of a call with default parameters.
 
+  First_Form  : Entity_Id;
   Formal  : Entity_Id;
   Actual  : Node_Id;
   Is_Indexed  : Boolean := False;
@@ -3581,8 +3582,9 @@ package body Sem_Ch4 is
  --  Normalize_Actuals has chained the named associations in the
  --  correct order of the formals.
 
- Actual := First_Actual (N);
- Formal := First_Formal (Nam);
+ Actual := First_Actual (N);
+ Formal := First_Formal (Nam);
+ First_Form := Formal;
 
  --  If we are analyzing a call rewritten from object notation, skip
  --  first actual, which may be rewritten later as an explicit
@@ -3742,6 +3744,54 @@ package body Sem_Ch4 is
 end if;
  end loop;
 
+ --  Due to our current model of controlled type expansion we may
+ --  have resolved a user call to a non-visible controlled primitive
+ --  since these inherited subprograms may be generated in the current
+ --  scope. This is a side-effect of the need for the expander to be
+ --  able to resolve internally generated calls.
+
+ --  Specifically, the issue appears when predefined controlled
+ --  operations get called on a type extension whose parent is a
+ --  private extension completed with a controlled extension - see
+ --  below:
+
+ --  package X is
+ -- type Par_Typ is tagged private;
+ --  private
+ -- type Par_Typ is new Controlled with null record;
+ --  end;
+ --  ...
+ --  procedure Main is
+ -- type Ext_Typ is new Par_Typ with null record;
+ -- Obj : Ext_Typ;
+ --  begin
+ -- Finalize (Obj); --  Will improperly resolve
+ --  end;
+
+ --  To avoid breaking privacy, Is_Hidden gets set elsewhere on such
+ --  primitives, but we still need to verify that Nam is indeed a
+ --  controlled subprogram. So, we do that here and issue the
+ --  appropriate error.
+
+ if Is_Hidden (Nam)
+   and then not In_Instance
+   and then not Comes_From_Source (Nam)
+   and then Comes_From_Source (N)
+
+   --  Verify Nam is a controlled primitive
+
+   and then Nam_In (Chars (Nam), Name_Adjust,
+ Name_Finalize,
+ Name_Initialize)
+   and then Ekind (Nam) = E_Procedure
+   and then Is_Controlled (Etype (First_Form))
+   and then No (Next_Formal (First_Form))
+ then
+Error_Msg_Node_2 := Etype (First_Form);
+Error_Msg_NE ("call to non-visible controlled primitive & on type"
+& " &", N, Nam);
+ end if;
+
  --  On exit, all actuals match
 
  Indicate_Name_And_Type;



[Ada] Use correct subtype for call to Last in formal vectors

2019-12-12 Thread Pierre-Marie de Rodat
Correct the subtype used for a call to Last in Find_Index so that it no
longer crashes on empty vectors with checks enabled.

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

2019-12-12  Claire Dross  

gcc/ada/

* libgnat/a-cofove.adb, libgnat/a-cfinve.adb (Find_Index): Use
Extended_Index for call to Last.--- gcc/ada/libgnat/a-cfinve.adb
+++ gcc/ada/libgnat/a-cfinve.adb
@@ -458,7 +458,7 @@ is
   Index : Index_Type := Index_Type'First) return Extended_Index
is
   K: Count_Type;
-  Last : constant Index_Type := Last_Index (Container);
+  Last : constant Extended_Index := Last_Index (Container);
 
begin
   K := Capacity_Range (Int (Index) - Int (No_Index));

--- gcc/ada/libgnat/a-cofove.adb
+++ gcc/ada/libgnat/a-cofove.adb
@@ -379,7 +379,7 @@ is
   Index : Index_Type := Index_Type'First) return Extended_Index
is
   K: Count_Type;
-  Last : constant Index_Type := Last_Index (Container);
+  Last : constant Extended_Index := Last_Index (Container);
 
begin
   K := Capacity_Range (Int (Index) - Int (No_Index));



[Ada] Improved handling of circular compilation dependencies

2019-12-12 Thread Pierre-Marie de Rodat
In the case described on this ticket, a predefined unit (the body of
Ada.Finalization) is recompiled with a configuration pragma in effect
(an Interrupt_State pragma) which has has the effect of introducing a
circular compilation dependency.  Previously this resulted in an
internal error. With this change, a better error message is produced.

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

2019-12-12  Steve Baird  

gcc/ada/

* sem_ch10.adb (Install_With_Clause): Check for the case of a
circular dependency involving a predefined (or GNAT-defined)
unit and handle that case by generating an appropropriate error
message.--- gcc/ada/sem_ch10.adb
+++ gcc/ada/sem_ch10.adb
@@ -5336,6 +5336,20 @@ package body Sem_Ch10 is
 Error_Msg_N
   ("instantiation depends on itself", Name (With_Clause));
 
+ elsif not Analyzed (Uname)
+   and then Is_Internal_Unit (Current_Sem_Unit)
+   and then not Is_Visible_Lib_Unit (Uname)
+   and then No (Scope (Uname))
+ then
+if Is_Predefined_Unit (Current_Sem_Unit) then
+   Error_Msg_N
+ ("predefined unit depends on itself", Name (With_Clause));
+else
+   Error_Msg_N
+ ("GNAT-defined unit depends on itself", Name (With_Clause));
+end if;
+return;
+
  elsif not Is_Visible_Lib_Unit (Uname) then
 
 --  Abandon processing in case of previous errors



[Ada] Implement AI12-0036 (a new legality check for instantiations)

2019-12-12 Thread Pierre-Marie de Rodat
AI12-0036 is a binding interpretation which adds the following legality
rule:

   The actual type for a formal derived type shall be tagged if and only
   if the formal derived type is a private extension.

Implement this new compile-time check. The check is implemented without
checking the value of Ada_Version because the AI is a binding
intepretation, and because the possible consequences of failing to
detect a violation may be severe.

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

2019-12-12  Steve Baird  

gcc/ada/

* sem_ch12.adb
(Instantiate_Type.Validate_Derived_Type_Instance): Implement the
legality check of AI12-0036--- gcc/ada/sem_ch12.adb
+++ gcc/ada/sem_ch12.adb
@@ -13166,6 +13166,35 @@ package body Sem_Ch12 is
Abandon_Instantiation (Actual);
 end if;
  end if;
+
+ --  Don't check Ada_Version here (for now) because AI12-0036 is
+ --  a binding interpretation; this decision may be reversed if
+ --  the situation turns out to be similar to that of the preceding
+ --  Is_Limited_Type test (see preceding comment).
+
+ declare
+Formal_Is_Private_Extension : constant Boolean :=
+  Nkind (Parent (A_Gen_T)) = N_Private_Extension_Declaration;
+
+Actual_Is_Tagged : constant Boolean := Is_Tagged_Type (Act_T);
+ begin
+if Actual_Is_Tagged /= Formal_Is_Private_Extension then
+   if In_Instance then
+  null;
+   else
+  if Actual_Is_Tagged then
+ Error_Msg_NE
+   ("actual for & cannot be a tagged type",
+Actual, Gen_T);
+  else
+ Error_Msg_NE
+   ("actual for & must be a tagged type",
+Actual, Gen_T);
+  end if;
+  Abandon_Instantiation (Actual);
+   end if;
+end if;
+ end;
   end Validate_Derived_Type_Instance;
 
   



[Ada] Constraint is ignored on constrained access record component

2019-12-12 Thread Pierre-Marie de Rodat
This patch fixes an omission in the generation of constraint checks,
when assigning to a record component whose type is a constrained access
to a discriminated record.

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

2019-12-12  Ed Schonberg  

gcc/ada/

* sem_ch3.adb (Constrain_Access): Remove obsolete comments and
warning concerning component types of an access type whose
designated type is a constrained record type. (Such constraints
were previously ignored). Set scope of itype for component to
the scope of the enclosing record.
* sem_ch4.adb: Remove call to Set_Ekind.
* sem_util.adb (Build_Actual_Subtype_Of_Component): Handle
components whose type is an access to a constrained
discriminant, where the constraints may be given by the
discriminants of the enclosing type. New subprogram
Build_Access_Record_Constraint.

gcc/testsuite/

* gnat.dg/warn24.adb: Remove expected warning.--- gcc/ada/sem_ch3.adb
+++ gcc/ada/sem_ch3.adb
@@ -12971,29 +12971,39 @@ package body Sem_Ch3 is
   or else Is_Incomplete_Or_Private_Type (Desig_Type))
 and then not Is_Constrained (Desig_Type)
   then
- --  ??? The following code is a temporary bypass to ignore a
- --  discriminant constraint on access type if it is constraining
- --  the current record. Avoid creating the implicit subtype of the
- --  record we are currently compiling since right now, we cannot
- --  handle these. For now, just return the access type itself.
+ --  If this is a constrained access definition for a record
+ --  component, we leave the type as an unconstrained access,
+ --  and mark the component so that its actual type is build
+ --  at a point of use (e.g an assignment statement). THis is
+ --  handled in sem_util, Build_Actual_Subtype_Of_Component.
 
  if Desig_Type = Current_Scope
and then No (Def_Id)
  then
-Error_Msg_Warn := SPARK_Mode /= On;
-Error_Msg_N ("< Scope (Desig_Type));
 Set_Ekind (Desig_Subtype, E_Record_Subtype);
 Def_Id := Entity (Subtype_Mark (S));
 
+--  We indicate that the component has a pet-object
+--  constraint for uniform treatment at a point of use,
+--  even though the constraint may be independent of
+--  discriminants of enclosing type.
+
+if Nkind (Related_Nod) = N_Component_Declaration then
+   Set_Has_Per_Object_Constraint
+ (Defining_Identifier (Related_Nod));
+end if;
+
 --  This call added to ensure that the constraint is analyzed
 --  (needed for a B test). Note that we still return early from
---  this procedure to avoid recursive processing. ???
+--  this procedure to avoid recursive processing.
 
 Constrain_Discriminated_Type
   (Desig_Subtype, S, Related_Nod, For_Access => True);
 return;
+
  end if;
 
  --  Enforce rule that the constraint is illegal if there is an

--- gcc/ada/sem_ch4.adb
+++ gcc/ada/sem_ch4.adb
@@ -4812,16 +4812,15 @@ package body Sem_Ch4 is
  Set_Etype (N, Etype (Comp));
 
   else
- --  Component type depends on discriminants. Enter the
- --  main attributes of the subtype.
+ --  If discriminants were present in the component
+ --  declaration, they have been replaced by the
+ --  actual values in the prefix object.
 
  declare
 Subt : constant Entity_Id :=
  Defining_Identifier (Act_Decl);
-
  begin
 Set_Etype (Subt, Base_Type (Etype (Comp)));
-Set_Ekind (Subt, Ekind (Etype (Comp)));
 Set_Etype (N, Subt);
  end;
   end if;

--- gcc/ada/sem_util.adb
+++ gcc/ada/sem_util.adb
@@ -1187,18 +1187,28 @@ package body Sem_Util is
is
   Loc   : constant Source_Ptr := Sloc (N);
   P : constant Node_Id:= Prefix (N);
+
   D : Elmt_Id;
   Id: Node_Id;
   Index_Typ : Entity_Id;
+  Sel   : Entity_Id  := Empty;
 
   Desig_Typ : Entity_Id;
   --  This is either a copy of T, or if T is an access type, then it is
   --  the directly designated type of this access type.
 
+  function Build_Access_Record_Constraint (C : List_Id) return List_Id;
+  --  If the record component is a constrained access to the current
+  --  record, the subtype has not been constructed during analysis of
+  --  the enclosing record type (see Analyze_Access). In that case build
+  --  a constrainted access subtype after replacing 

[Ada] Crash on use of Loop_Entry, Result, and Old as actuals

2019-12-12 Thread Pierre-Marie de Rodat
This patch fixes an issue whereby the compiler crashes generating
accessibility checks for the attribute references 'Loop_Entry, 'Old, and
'Result when they are used as actuals.

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

2019-12-12  Justin Squirek  

gcc/ada/

* exp_ch6.adb (Expand_Call_Helper): Added null case for
'Loop_Entry, 'Old, and 'Result when calculating whether to
create extra accessibility parameters.
* sem_util.adb (Dynamic_Accessibility_Level): Added null case
for 'Loop_Entry, 'Old, and 'Result when  calculating
accessibility level based on access-valued attributes.  Also
added special handling for uses of 'Loop_Entry when used in its
indexed component form.--- gcc/ada/exp_ch6.adb
+++ gcc/ada/exp_ch6.adb
@@ -3389,6 +3389,15 @@ package body Exp_Ch6 is
case Nkind (Prev_Orig) is
   when N_Attribute_Reference =>
  case Get_Attribute_Id (Attribute_Name (Prev_Orig)) is
+--  Ignore 'Result, 'Loop_Entry, and 'Old as they can
+--  be used to identify access objects and do not have
+--  an effect on accessibility level.
+
+when Attribute_Loop_Entry
+   | Attribute_Old
+   | Attribute_Result
+=>
+   null;
 
 --  For X'Access, pass on the level of the prefix X
 

--- gcc/ada/sem_util.adb
+++ gcc/ada/sem_util.adb
@@ -6488,7 +6488,7 @@ package body Sem_Util is
 
   --  Local variables
 
-  Expr : constant Node_Id := Original_Node (N);
+  Expr : Node_Id := Original_Node (N);
   --  Expr references the original node because at this stage N may be the
   --  reference to a variable internally created by the frontend to remove
   --  side effects of an expression.
@@ -6516,6 +6516,21 @@ package body Sem_Util is
   --  Unimplemented: Ptr.all'Access, where Ptr has Extra_Accessibility ???
 
   case Nkind (Expr) is
+ --  It may be possible that we have an access object denoted by an
+ --  attribute reference for 'Loop_Entry which may, in turn, have an
+ --  indexed component representing a loop identifier.
+
+ --  In this case we must climb up the indexed component and set expr
+ --  to the attribute reference so the rest of the machinery can
+ --  operate as expected.
+
+ when N_Indexed_Component =>
+if Nkind (Prefix (Expr)) = N_Attribute_Reference
+  and then Get_Attribute_Id (Attribute_Name (Prefix (Expr)))
+ = Attribute_Loop_Entry
+then
+   Expr := Prefix (Expr);
+end if;
 
  --  For access discriminant, the level of the enclosing object
 
@@ -6530,6 +6545,13 @@ package body Sem_Util is
  when N_Attribute_Reference =>
 case Get_Attribute_Id (Attribute_Name (Expr)) is
 
+   --  Ignore 'Loop_Entry, 'Result, and 'Old as they can be used to
+   --  identify access objects and do not have an effect on
+   --  accessibility level.
+
+   when Attribute_Loop_Entry | Attribute_Old | Attribute_Result =>
+  null;
+
--  For X'Access, the level of the prefix X
 
when Attribute_Access =>



[Ada] Mark Ada subprograms and variables referenced from gigi

2019-12-12 Thread Pierre-Marie de Rodat
In addition to the C bindings automatically generated during the build
process (einfo.h, sinfo.h, snames.h), gigi also makes use of a manually
maintained C interface to the front-end (atree.h, elists.h, fe.h, namet.h,
nlists.h, repinfo.h, scos.h, stringt.h, types.h, uintp.h, urealp.h).

This change makes sure that the Ada side is fully aware of this manually
maintained C interface by adding WARNING lines to the commentary in the
appropriate places, either globally if there is a specific C header file
or individually for the declarations referenced in the fe.h file.

No functional changes.

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

2019-12-12  Eric Botcazou  

gcc/ada/

* atree.ads, comperr.ads, debug.ads, einfo.ads, elists.ads,
err_vars.ads, errout.ads, exp_ch11.ads, exp_code.ads,
exp_dbug.ads, exp_tss.ads, exp_util.ads, lib.ads, namet.ads,
nlists.ads, opt.ads, repinfo.ads, restrict.ads, scos.ads,
sem_aggr.ads, sem_aux.ads, sem_eval.ads, sem_util.ads,
sinfo.ads, sinput.ads, stringt.ads, targparm.ads, types.ads,
urealp.ads warnsw.ads: Add WARNING line(s) in commentary.
* atree.h, elists.h, namet.h, nlists.h, repinfo.h, scos.h,
stringt.h, types.h, uintp.h, urealp.h: Tidy up.
* fe.h: Likewise.  Document WARNING mark.

patch.diff.gz
Description: application/gzip


[Ada] Improve end of command line arguments detection

2019-12-12 Thread Pierre-Marie de Rodat
Add routine Get_Argument with End_Of_Arguments Boolean out parameter
into GNAT.Command_Line API to distinguish between the empty argument and
the end of arguments.

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

2019-12-12  Dmitriy Anisimkov  

gcc/ada/

* libgnat/g-comlin.ads (Get_Argument): New routine similar to
original Get_Argument but with one more out parameter
End_Of_Arguments.
(Get_Arguments): Comment improved.
* libgnat/g-comlin.adb (Get_Argument): Implementation taken from
original Get_Argument and improved.
(Get_Argument): Calls new routine Get_Argument with additional
parameter.--- gcc/ada/libgnat/g-comlin.adb
+++ gcc/ada/libgnat/g-comlin.adb
@@ -385,10 +385,25 @@ package body GNAT.Command_Line is
--
 
function Get_Argument
- (Do_Expansion : Boolean:= False;
+ (Do_Expansion : Boolean := False;
   Parser   : Opt_Parser := Command_Line_Parser) return String
is
+  End_Of_Args : Boolean;
begin
+  return Get_Argument (Do_Expansion, Parser, End_Of_Args);
+   end Get_Argument;
+
+   --
+   -- Get_Argument --
+   --
+
+   function Get_Argument
+ (Do_Expansion : Boolean:= False;
+  Parser   : Opt_Parser := Command_Line_Parser;
+  End_Of_Arguments : out Boolean) return String is
+   begin
+  End_Of_Arguments := False;
+
   if Parser.In_Expansion then
  declare
 S : constant String := Expansion (Parser.Expansion_It);
@@ -415,6 +430,7 @@ package body GNAT.Command_Line is
 end loop;
 
  else
+End_Of_Arguments := True;
 return String'(1 .. 0 => ' ');
  end if;
 
@@ -436,9 +452,11 @@ package body GNAT.Command_Line is
   end loop;
 
   if Parser.Current_Argument > Parser.Arg_Count then
+ End_Of_Arguments := True;
  return String'(1 .. 0 => ' ');
+
   elsif Parser.Section (Parser.Current_Argument) = 0 then
- return Get_Argument (Do_Expansion);
+ return Get_Argument (Do_Expansion, Parser, End_Of_Arguments);
   end if;
 
   Parser.Current_Argument := Parser.Current_Argument + 1;
@@ -451,13 +469,10 @@ package body GNAT.Command_Line is
   Argument (Parser, Parser.Current_Argument - 1);
  begin
 for Index in Arg'Range loop
-   if Arg (Index) = '*'
- or else Arg (Index) = '?'
- or else Arg (Index) = '['
-   then
+   if Arg (Index) in '*' | '?' | '[' then
   Parser.In_Expansion := True;
   Start_Expansion (Parser.Expansion_It, Arg);
-  return Get_Argument (Do_Expansion, Parser);
+  return Get_Argument (Do_Expansion, Parser, End_Of_Arguments);
end if;
 end loop;
  end;

--- gcc/ada/libgnat/g-comlin.ads
+++ gcc/ada/libgnat/g-comlin.ads
@@ -462,8 +462,9 @@ package GNAT.Command_Line is
function Get_Argument
  (Do_Expansion : Boolean := False;
   Parser   : Opt_Parser := Command_Line_Parser) return String;
-   --  Returns the next element on the command line that is not a switch.  This
-   --  function should not be called before Getopt has returned ASCII.NUL.
+   --  Returns the next element on the command line that is not a switch. This
+   --  function should be called either after Getopt has returned ASCII.NUL or
+   --  after Getopt procedure call.
--
--  If Do_Expansion is True, then the parameter on the command line will
--  be considered as a filename with wildcards, and will be expanded. The
@@ -472,6 +473,16 @@ package GNAT.Command_Line is
--  When there are no more arguments on the command line, this function
--  returns an empty string.
 
+   function Get_Argument
+ (Do_Expansion : Boolean:= False;
+  Parser   : Opt_Parser := Command_Line_Parser;
+  End_Of_Arguments : out Boolean) return String;
+   --  The same as above but able to distinguish empty element in argument list
+   --  from end of arguments.
+   --  End_Of_Arguments is True if the end of the command line has been reached
+   --  (i.e. all available arguments have been returned by previous calls to
+   --  Get_Argument).
+
function Parameter
  (Parser : Opt_Parser := Command_Line_Parser) return String;
--  Returns parameter associated with the last switch returned by Getopt.



[GCC][testsuite][ARM][AArch64] Add ARM v8.6 effective target checks to target-supports.exp

2019-12-12 Thread Stam Markianos-Wright
Hi all,

This small patch adds support for the ARM v8.6 extensions +bf16 and 
+i8mm to the testsuite. This will be tested through other upcoming 
patches, which is why we are not providing any explicit tests here.

Ok for trunk?

Also I don't have commit rights, so if someone could commit on my 
behalf, that would be great :)

The functionality here depends on CLI patches:
https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02415.html
https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02195.html

but this patch applies cleanly without them, too.

Cheers,
Stam


gcc/testsuite/ChangeLog:

2019-12-11  Stam Markianos-Wright  

* lib/target-supports.exp
(check_effective_target_arm_v8_2a_i8mm_ok_nocache): New.
(check_effective_target_arm_v8_2a_i8mm_ok): New.
(add_options_for_arm_v8_2a_i8mm): New.
(check_effective_target_arm_v8_2a_bf16_neon_ok_nocache): New.
(check_effective_target_arm_v8_2a_bf16_neon_ok): New.
(add_options_for_arm_v8_2a_bf16_neon): New.
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 5b4cc02f921..36fb63e9929 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -4781,6 +4781,49 @@ proc add_options_for_arm_v8_2a_dotprod_neon { flags } {
 return "$flags $et_arm_v8_2a_dotprod_neon_flags"
 }
 
+# Return 1 if the target supports ARMv8.2+i8mm Adv.SIMD Dot Product
+# instructions, 0 otherwise.  The test is valid for ARM and for AArch64.
+# Record the command line options needed.
+
+proc check_effective_target_arm_v8_2a_i8mm_ok_nocache { } {
+global et_arm_v8_2a_i8mm_flags
+set et_arm_v8_2a_i8mm_flags ""
+
+if { ![istarget arm*-*-*] && ![istarget aarch64*-*-*] } {
+return 0;
+}
+
+# Iterate through sets of options to find the compiler flags that
+# need to be added to the -march option.
+foreach flags {"" "-mfloat-abi=hard -mfpu=neon-fp-armv8" "-mfloat-abi=softfp -mfpu=neon-fp-armv8" } {
+if { [check_no_compiler_messages_nocache \
+  arm_v8_2a_i8mm_ok object {
+#include 
+#if !defined (__ARM_FEATURE_MATMUL_INT8)
+#error "__ARM_FEATURE_MATMUL_INT8 not defined"
+#endif
+} "$flags -march=armv8.2-a+i8mm"] } {
+set et_arm_v8_2a_i8mm_flags "$flags -march=armv8.2-a+i8mm"
+return 1
+}
+}
+
+return 0;
+}
+
+proc check_effective_target_arm_v8_2a_i8mm_ok { } {
+return [check_cached_effective_target arm_v8_2a_i8mm_ok \
+check_effective_target_arm_v8_2a_i8mm_ok_nocache]
+}
+
+proc add_options_for_arm_v8_2a_i8mm { flags } {
+if { ! [check_effective_target_arm_v8_2a_i8mm_ok] } {
+return "$flags"
+}
+global et_arm_v8_2a_i8mm_flags
+return "$flags $et_arm_v8_2a_i8mm_flags"
+}
+
 # Return 1 if the target supports FP16 VFMAL and VFMSL
 # instructions, 0 otherwise.
 # Record the command line options needed.
@@ -4826,6 +4869,45 @@ proc add_options_for_arm_fp16fml_neon { flags } {
 return "$flags $et_arm_fp16fml_neon_flags"
 }
 
+# Return 1 if the target supports BFloat16 SIMD instructions, 0 otherwise.
+# The test is valid for ARM and for AArch64.
+
+proc check_effective_target_arm_v8_2a_bf16_neon_ok_nocache { } {
+global et_arm_v8_2a_bf16_neon_flags
+set et_arm_v8_2a_bf16_neon_flags ""
+
+if { ![istarget arm*-*-*] && ![istarget aarch64*-*-*] } {
+return 0;
+}
+
+foreach flags {"" "-mfloat-abi=hard -mfpu=neon-fp-armv8" "-mfloat-abi=softfp -mfpu=neon-fp-armv8" } {
+if { [check_no_compiler_messages_nocache arm_v8_2a_bf16_neon_ok object {
+#include 
+#if !defined (__ARM_FEATURE_BF16_VECTOR_ARITHMETIC)
+#error "__ARM_FEATURE_BF16_VECTOR_ARITHMETIC not defined"
+#endif
+} "$flags -march=armv8.2-a+bf16"] } {
+set et_arm_v8_2a_bf16_neon_flags "$flags -march=armv8.2-a+bf16"
+return 1
+}
+}
+
+return 0;
+}
+
+proc check_effective_target_arm_v8_2a_bf16_neon_ok { } {
+return [check_cached_effective_target arm_v8_2a_bf16_neon_ok \
+check_effective_target_arm_v8_2a_bf16_neon_ok_nocache]
+}
+
+proc add_options_for_arm_v8_2a_bf16_neon { flags } {
+if { ! [check_effective_target_arm_v8_2a_bf16_neon_ok] } {
+return "$flags"
+}
+global et_arm_v8_2a_bf16_neon_flags
+return "$flags $et_arm_v8_2a_bf16_neon_flags"
+}
+
 # Return 1 if the target supports executing ARMv8 NEON instructions, 0
 # otherwise.
 


Re: [PATCH] Fix simplify-rtx.c handling of avx512 vector comparisons (PR target/92908)

2019-12-12 Thread Jakub Jelinek
On Thu, Dec 12, 2019 at 09:22:47AM +, Richard Sandiford wrote:
> > So there's no whole vector equality RTX but we have to pun to integer
> > modes for that?  The eq:SImode would suggest that.  Guess we should have
> > used a BImode vector representation...

Probably something like
V2BImode/V4BImode/V8BImode/V16BImode/V32BImode/V64BImode, yes, but I'm
afraid changing it would be extremely hard, all the builtins that take the
masks have int/long long arguments and we already generate terrible code
for the mask registers with various PRs, using a different mode would make
it even worse (but sure, more clear).
> >
> > Can you check whether we have any target with whole vector compare patterns 
> > that would break here? 

All I can see is both x86 and rs6000 using eq:CC with vector operands
(the former in cbranchv*, the latter in various patterns that set both
vector mode comparison result and CCmode comparison result, and
aarch64 having cbranch even for vector modes, but only in define_expand and
expanding that to ptest which uses UNSPECs.

> I wonder how easy it would be to make the mask registers use
> MODE_VECTOR_BOOL instead of scalar integers... :-)

I'm afraid hard.

> For now I think the safest thing would be to punt, rather than assume
> one thing or the other.  simplify_const_relational_operation doesn't
> handle many vector cases anyway, and most interesting optimisations
> like this should happen on gimple, where we know whether the condition
> result is a vector or a scalar.

If it starts being ambiguous somewhere, could we use some target macro or
target hook to decide?  We already use various target macros in this code,
e.g. FLOAT_STORE_FLAG_VALUE, VECTOR_STORE_FLAG_VALUE, and if they aren't
defined, we punt.
If we used some macro for this case too, we could punt by default if we see
this case and only if the backend said how to handle vector cmp_mode with
scalar int result we could fold it?

Jakub



Re: [PATCH] PR 92846: [ARC] generate signaling FDCMPF for hard float comparisons

2019-12-12 Thread Claudiu Zissulescu Ianculescu
Pushed. Thank you for your contribution,
Claudiu

On Wed, Dec 11, 2019 at 12:47 AM Vineet Gupta
 wrote:
>
> On 12/10/19 1:12 AM, Claudiu Zissulescu wrote:
> > Hi,
> >
> > Thank you for your contribution, I'll push it asap. As far as I understand, 
> > you need this patch both in gcc9 branch and mainline.
> >
> > Cheers,
> > Claudiu
>
> Indeed both mainline and gcc9
>
> Thx
> -Vineet
>
> >
> >> -Original Message-
> >> From: Vineet Gupta [mailto:vgu...@synopsys.com]
> >> Sent: Monday, December 09, 2019 8:02 PM
> >> To: gcc-patches@gcc.gnu.org
> >> Cc: Claudiu Zissulescu ;
> >> andrew.burg...@embecosm.com; linux-snps-...@lists.infradead.org;
> >> Vineet Gupta 
> >> Subject: [PATCH] PR 92846: [ARC] generate signaling FDCMPF for hard float
> >> comparisons
> >>
> >> ARC gcc generates FDCMP instructions which raises Invalid operation for
> >> signaling NaN only. This causes glibc iseqsig() primitives to fail (in
> >> the current ongoing glibc port to ARC)
> >>
> >> So split up the hard float compares into two categories and for unordered
> >> compares generate the FDCMPF instruction (vs. FDCMP) which raises
> >> exception
> >> for either NaNs.
> >>
> >> With this fix testsuite/gcc.dg/torture/pr52451.c passes for ARC.
> >>
> >> Also passes 6 additional tests in glibc testsuite (test*iseqsig) and no
> >> regressions
> >>
> >> gcc/
> >> -xx-xx  Vineet Gupta  
> >>
> >>  * config/arc/arc-modes.def (CC_FPUE): New Mode CC_FPUE which
> >>  helps codegen generate exceptions even for quiet NaN.
> >>  * config/arc/arc.c (arc_init_reg_tables): Handle New CC_FPUE mode.
> >>  (get_arc_condition_code): Likewise.
> >>  (arc_select_cc_mode): LT, LE, GT, GE to use the New CC_FPUE
> >> mode.
> >>  * config/arc/arc.h (REVERSE_CONDITION): Handle New CC_FPUE
> >> mode.
> >>  * config/arc/predicates.md (proper_comparison_operator):
> >> Likewise.
> >>  * config/arc/fpu.md (cmpsf_fpu_trap): New Pattern for CC_FPUE.
> >>  (cmpdf_fpu_trap): Likewise.
> >>
> >> Signed-off-by: Vineet Gupta 
> >> ---
> >>  gcc/config/arc/arc-modes.def |  1 +
> >>  gcc/config/arc/arc.c |  8 ++--
> >>  gcc/config/arc/arc.h |  2 +-
> >>  gcc/config/arc/fpu.md| 24 
> >>  gcc/config/arc/predicates.md |  1 +
> >>  5 files changed, 33 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/gcc/config/arc/arc-modes.def b/gcc/config/arc/arc-modes.def
> >> index 36a2f4abfb25..d16b6a289a15 100644
> >> --- a/gcc/config/arc/arc-modes.def
> >> +++ b/gcc/config/arc/arc-modes.def
> >> @@ -38,4 +38,5 @@ VECTOR_MODES (INT, 16);   /* V16QI V8HI V4SI V2DI
> >> */
> >>
> >>  /* FPU condition flags.  */
> >>  CC_MODE (CC_FPU);
> >> +CC_MODE (CC_FPUE);
> >>  CC_MODE (CC_FPU_UNEQ);
> >> diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
> >> index 28305f459dcd..cbb95d6e9043 100644
> >> --- a/gcc/config/arc/arc.c
> >> +++ b/gcc/config/arc/arc.c
> >> @@ -1564,6 +1564,7 @@ get_arc_condition_code (rtx comparison)
> >>  default : gcc_unreachable ();
> >>  }
> >>  case E_CC_FPUmode:
> >> +case E_CC_FPUEmode:
> >>switch (GET_CODE (comparison))
> >>  {
> >>  case EQ: return ARC_CC_EQ;
> >> @@ -1686,11 +1687,13 @@ arc_select_cc_mode (enum rtx_code op, rtx x,
> >> rtx y)
> >>case UNLE:
> >>case UNGT:
> >>case UNGE:
> >> +return CC_FPUmode;
> >> +
> >>case LT:
> >>case LE:
> >>case GT:
> >>case GE:
> >> -return CC_FPUmode;
> >> +return CC_FPUEmode;
> >>
> >>case LTGT:
> >>case UNEQ:
> >> @@ -1844,7 +1847,7 @@ arc_init_reg_tables (void)
> >>if (i == (int) CCmode || i == (int) CC_ZNmode || i == (int) CC_Zmode
> >>|| i == (int) CC_Cmode
> >>|| i == CC_FP_GTmode || i == CC_FP_GEmode || i ==
> >> CC_FP_ORDmode
> >> -  || i == CC_FPUmode || i == CC_FPU_UNEQmode)
> >> +  || i == CC_FPUmode || i == CC_FPUEmode || i ==
> >> CC_FPU_UNEQmode)
> >>  arc_mode_class[i] = 1 << (int) C_MODE;
> >>else
> >>  arc_mode_class[i] = 0;
> >> @@ -8401,6 +8404,7 @@ arc_reorg (void)
> >>
> >>/* Avoid FPU instructions.  */
> >>if ((GET_MODE (XEXP (XEXP (pc_target, 0), 0)) == CC_FPUmode)
> >> +  || (GET_MODE (XEXP (XEXP (pc_target, 0), 0)) == CC_FPUEmode)
> >>|| (GET_MODE (XEXP (XEXP (pc_target, 0), 0)) ==
> >> CC_FPU_UNEQmode))
> >>  continue;
> >>
> >> diff --git a/gcc/config/arc/arc.h b/gcc/config/arc/arc.h
> >> index 4d7ac3281b41..c08ca3d0d432 100644
> >> --- a/gcc/config/arc/arc.h
> >> +++ b/gcc/config/arc/arc.h
> >> @@ -1531,7 +1531,7 @@ enum arc_function_type {
> >>(((MODE) == CC_FP_GTmode || (MODE) == CC_FP_GEmode \
> >>  || (MODE) == CC_FP_UNEQmode || (MODE) == CC_FP_ORDmode   \
> >>  || (MODE) == CC_FPXmode || (MODE) == CC_FPU_UNEQmode \
> >> -|| (MODE) == CC_FPUmode) \
> >> +|| (MODE) == 

Re: [PATCH] [ARC] Use hardware support for double-precision compare instructions.

2019-12-12 Thread Claudiu Zissulescu Ianculescu
Thank you for your review. Patch pushed to mainline and gcc9 branch.

//Claudiu

On Wed, Dec 11, 2019 at 8:59 PM Jeff Law  wrote:
>
> On Mon, 2019-12-09 at 11:52 +0200, Claudiu Zissulescu wrote:
> > Although the FDCMP (the double precision floating point compare
> > instruction) is added to the compiler, it is not properly used via
> > cstoredi pattern. Fix it.
> >
> > OK to apply?
> > Claudidu
> >
> > -xx-xx  Claudiu Zissulescu  
> >
> >   * config/arc/arc.md (iterator SDF): Check TARGET_FP_DP_BASE.
> >   (cstoredi4): Use TARGET_HARD_FLOAT.
> OK
> jeff
>


Re: AW: [PATCH] m68k architecture: support ccmode + lra

2019-12-12 Thread Richard Sandiford
Stefan Franke  writes:
> Am 2019-12-08 01:54, schrieb Oleg Endo:
>> On Tue, 2019-11-26 at 07:38 +0100, ste...@franke.ms wrote:
>>> > On 11/21/19 10:30 AM, ste...@franke.ms wrote:
>>> > > Hi there,
>>> > >
>>> > > here is mc68k's patch to switch the m68k architecture over to ccmode
>>> > > and
>>> > > lra. See https://github.com/mc68kghost/gcc 68k-ccmode branch.
>>> >
>>> > Bernd Schmidt posted a conversion of the m68k port to ccmode a couple
>>> > weeks before yours.  We've already ACK'd it for installing onto the
>>> > trunk.
>>> >
>>> > Jeff
>>> 
>>> To be honest:
>>> - 8 days is hardly "a couple weeks before"
>>> - ccmode is not the same as ccmode+lra
>>> 
>>> The paperwork for contributing to fsf is on the way and the repo at
>>> https://github.com/mc68kghost/gcc got an update. Tests are not yet at 
>>> 100%
>>> (master branch fails too many tests) but it's closer to master branch 
>>> now.
>>> The code is to 50% identical, a fair amount has swapped cmp/bcc, few 
>>> are a
>>> tad worse and some yield surprisingly better code.
>>> 
>> 
>> You can still submit patches for further improvements, like adding
>> support for LRA.  Now that the main CCmode conversion is on trunk and
>> has been confirmed and tested, it should be much easier for you to
>> pinpoint problems in your changes.
>> 
>> Cheers,
>> Oleg
>
> The problems are in the gcc implementation
>
> - the lra implementation is buggy
> - the compare elimination can't handle parallel's containing a compare
> - df-core considers parallel's containing a compare also as a USE
> - some optimizations/mechanisms do only work if HAVE_CC0 is defined
> - way more ...
>
> And the current implementation is IMHO unusable for lra since it did not 
> introduce a CC register to track clobbering. So it's a dead end.

I'm not sure about this last bit.  LRA simply isn't set up to cope
with move instructions that could clobber the CC register, so on
targets like m68k, I think we'd want to hide any CC register until
after LRA anyway.  Having a CC register could be useful for post-RA
optimisation, but not having one shouldn't affect the choice of RA.

(To be clear: this is in reply to the above two lines only.
Thanks for doing this work, and especially for considering the
transition to LRA.  I doubt it will be long before we deprecate
all targets that require old reload.)

Richard

>
> I can live with the fact that my patch was refuted since I simply use my 
> *working* fork, where I fixed the issues mentioned above.
>
> /cheers
> Stefan


Re: [PATCH] rs6000: Fix 2 for PR92661, Do not define builtins that overload disabled builtins

2019-12-12 Thread Segher Boessenkool
On Mon, Dec 09, 2019 at 02:16:51PM -0600, Peter Bergner wrote:
> I'm not sure that's necessary.  DFP enablement isn't triggered by
> assembler support.  Just the gcc/configure fragment (ignoring manually
> using --enable-decimal-float):
> 
>   case $target in
> powerpc*-*-linux* | i?86*-*-linux* | x86_64*-*-linux* | s390*-*-linux* | \
> i?86*-*-elfiamcu | i?86*-*-gnu* | x86_64*-*-gnu* | \
> i?86*-*-mingw* | x86_64*-*-mingw* | \
> i?86*-*-cygwin* | x86_64*-*-cygwin*)
>   enable_decimal_float=yes

Hmm.  Do we still only support it on Linux?  Is that a bit too
conservative?  The only thing required from the OS is treating the
FPSCR as a 64-bit register (instead of 32-bit), which is required by
the architecture since, what, ISA 2.05?  12 years ago?

OTOH, if there is no demand for this elsewhere, the status quo will
work fine of course, and it's simple as can be.

This configure thing does not gate whether the backend code supports
DFP though?  Or is that handled somewhere?  I.e. can we ever end up
with TARGET_DFP set while enable_decimal_float is not set?


Segher


Re: [PATCH] Fix simplify-rtx.c handling of avx512 vector comparisons (PR target/92908)

2019-12-12 Thread Richard Sandiford
Richard Biener  writes:
> On December 12, 2019 12:56:01 AM GMT+01:00, Jakub Jelinek  
> wrote:
>>Hi!
>>
>>The AVX512{F,VL} vector comparisons that set %kN registers are
>>represented
>>in RTL as comparisons with vector mode operands and scalar integral
>>result,
>>where at runtime the scalar integer is filled with a bitmask.
>>Unfortunately, simplify_relational_operation would fold e.g.
>>(eq:SI (reg:V32HI x) (reg:V32HI x))
>>into (const_int 1) rather than (const_int -1) that is expected (all
>>elements
>>equal).  simplify_const_relational_operation is documented to always
>>return
>>just const0_rtx or const_true_rtx and simplify_relational_operation is
>>expected to fix this up, for vector comparisons with vector result it
>>duplicates the 0 or -1 into all elements, etc., so this patch adds
>>handling
>>for this case there too.
>>
>>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> So there's no whole vector equality RTX but we have to pun to integer modes 
> for that? The eq:SImode would suggest that. Guess we should have used a 
> BImode vector representation... 
>
> Can you check whether we have any target with whole vector compare patterns 
> that would break here? 

We also have vector cbranch4 for vectors, which does a genuine
scalar comparison of vectors.  It happens that the representation of
them on x86 (using CCmodes only) means that there's currently no
ambiguity with the elementwise vector-to-scalar comparisons above.
But I think further encoding this assumption is going to cause problems
eventually.

E.g. the scalar comparison { 0, 0 } == { 0, 1 } should be false when
used with cbranch4, but should be 2 when used with an elementwise
interpretation.  If the cbranch4 interpretation is turned into
a cstore for (cbranch4 ? 1 : 0), we could easily end up wanting
(eq:SI ...) of those vectors to return false rather than 2.  Even for
{ 0, 0 } == { 0, 0 }, a cstore (eq:SI ...) would need to be 1 rather
than 3.

I wonder how easy it would be to make the mask registers use
MODE_VECTOR_BOOL instead of scalar integers... :-)

For now I think the safest thing would be to punt, rather than assume
one thing or the other.  simplify_const_relational_operation doesn't
handle many vector cases anyway, and most interesting optimisations
like this should happen on gimple, where we know whether the condition
result is a vector or a scalar.

Thanks,
Richard

>
> Richard. 
>
>>2019-12-11  Jakub Jelinek  
>>
>>  PR target/92908
>>  * simplify-rtx.c (simplify_relational_operation): For vector cmp_mode
>>  and scalar mode, if simplify_relational_operation returned
>>  const_true_rtx, return a scalar bitmask of all ones.
>>  (simplify_const_relational_operation): Change VOID_mode in function
>>  comment to VOIDmode.
>>
>>  * gcc.target/i386/avx512bw-pr92908.c: New test.
>>
>>--- gcc/simplify-rtx.c.jj 2019-11-19 22:27:02.58742 +0100
>>+++ gcc/simplify-rtx.c2019-12-11 13:31:57.197809704 +0100
>>@@ -5037,6 +5037,23 @@ simplify_relational_operation (enum rtx_
>>return NULL_RTX;
>> #endif
>>  }
>>+  if (VECTOR_MODE_P (cmp_mode)
>>+   && SCALAR_INT_MODE_P (mode)
>>+   && tem == const_true_rtx)
>>+ {
>>+   /* Vector comparisons that expect a scalar integral
>>+  bitmask.  For const0_rtx the result is already correct,
>>+  for const_true_rtx we need all bits set.  */
>>+   int n_elts;
>>+   scalar_int_mode smode = as_a  (mode);
>>+   gcc_assert (GET_MODE_NUNITS (cmp_mode).is_constant (_elts)
>>+   && GET_MODE_PRECISION (smode) <= n_elts);
>>+   if (GET_MODE_PRECISION (smode) == n_elts)
>>+ return constm1_rtx;
>>+   if (n_elts < HOST_BITS_PER_WIDE_INT)
>>+ return GEN_INT ((HOST_WIDE_INT_1U << n_elts) - 1);
>>+   return NULL_RTX;
>>+ }
>> 
>>   return tem;
>> }
>>@@ -5383,7 +5400,7 @@ comparison_result (enum rtx_code code, i
>> }
>> 
>> /* Check if the given comparison (done in the given MODE) is actually
>>-   a tautology or a contradiction.  If the mode is VOID_mode, the
>>+   a tautology or a contradiction.  If the mode is VOIDmode, the
>>comparison is done in "infinite precision".  If no simplification
>>is possible, this function returns zero.  Otherwise, it returns
>>either const_true_rtx or const0_rtx.  */
>>--- gcc/testsuite/gcc.target/i386/avx512bw-pr92908.c.jj   2019-12-11
>>14:24:12.083418762 +0100
>>+++ gcc/testsuite/gcc.target/i386/avx512bw-pr92908.c  2019-12-11
>>14:23:56.071665326 +0100
>>@@ -0,0 +1,21 @@
>>+/* PR target/92908 */
>>+/* { dg-do run } */
>>+/* { dg-options "-Og -fno-tree-fre -mavx512bw" } */
>>+/* { dg-require-effective-target avx512bw } */
>>+
>>+#define AVX512BW
>>+#include "avx512f-helper.h"
>>+
>>+typedef unsigned short V __attribute__ ((__vector_size__ (64)));
>>+
>>+V v;
>>+
>>+void
>>+TEST (void)
>>+{
>>+  int i;
>>+  v = (V) v == v;
>>+  for (i = 0; i < 32; i++)
>>+if (v[i] != 0x)
>>+  

Re: AW: [PATCH] m68k architecture: support ccmode + lra

2019-12-12 Thread Richard Biener
On December 12, 2019 7:43:58 AM GMT+01:00, Stefan Franke  
wrote:
>Am 2019-12-08 01:54, schrieb Oleg Endo:
>> On Tue, 2019-11-26 at 07:38 +0100, ste...@franke.ms wrote:
>>> > On 11/21/19 10:30 AM, ste...@franke.ms wrote:
>>> > > Hi there,
>>> > >
>>> > > here is mc68k's patch to switch the m68k architecture over to
>ccmode
>>> > > and
>>> > > lra. See https://github.com/mc68kghost/gcc 68k-ccmode branch.
>>> >
>>> > Bernd Schmidt posted a conversion of the m68k port to ccmode a
>couple
>>> > weeks before yours.  We've already ACK'd it for installing onto
>the
>>> > trunk.
>>> >
>>> > Jeff
>>> 
>>> To be honest:
>>> - 8 days is hardly "a couple weeks before"
>>> - ccmode is not the same as ccmode+lra
>>> 
>>> The paperwork for contributing to fsf is on the way and the repo at
>>> https://github.com/mc68kghost/gcc got an update. Tests are not yet
>at 
>>> 100%
>>> (master branch fails too many tests) but it's closer to master
>branch 
>>> now.
>>> The code is to 50% identical, a fair amount has swapped cmp/bcc, few
>
>>> are a
>>> tad worse and some yield surprisingly better code.
>>> 
>> 
>> You can still submit patches for further improvements, like adding
>> support for LRA.  Now that the main CCmode conversion is on trunk and
>> has been confirmed and tested, it should be much easier for you to
>> pinpoint problems in your changes.
>> 
>> Cheers,
>> Oleg
>
>The problems are in the gcc implementation
>
>- the lra implementation is buggy
>- the compare elimination can't handle parallel's containing a compare
>- df-core considers parallel's containing a compare also as a USE
>- some optimizations/mechanisms do only work if HAVE_CC0 is defined
>- way more ...
>
>And the current implementation is IMHO unusable for lra since it did
>not 
>introduce a CC register to track clobbering. So it's a dead end.
>
>I can live with the fact that my patch was refuted since I simply use
>my 
>*working* fork, where I fixed the issues mentioned above.

You can of course also submit those changes. I think the work that got in was 
the minimal work required to get the target off CC0. 

Richard. 

>/cheers
>Stefan



Re: [PATCH] Fix simplify-rtx.c handling of avx512 vector comparisons (PR target/92908)

2019-12-12 Thread Richard Biener
On December 12, 2019 12:56:01 AM GMT+01:00, Jakub Jelinek  
wrote:
>Hi!
>
>The AVX512{F,VL} vector comparisons that set %kN registers are
>represented
>in RTL as comparisons with vector mode operands and scalar integral
>result,
>where at runtime the scalar integer is filled with a bitmask.
>Unfortunately, simplify_relational_operation would fold e.g.
>(eq:SI (reg:V32HI x) (reg:V32HI x))
>into (const_int 1) rather than (const_int -1) that is expected (all
>elements
>equal).  simplify_const_relational_operation is documented to always
>return
>just const0_rtx or const_true_rtx and simplify_relational_operation is
>expected to fix this up, for vector comparisons with vector result it
>duplicates the 0 or -1 into all elements, etc., so this patch adds
>handling
>for this case there too.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

So there's no whole vector equality RTX but we have to pun to integer modes for 
that? The eq:SImode would suggest that. Guess we should have used a BImode 
vector representation... 

Can you check whether we have any target with whole vector compare patterns 
that would break here? 

Richard. 

>2019-12-11  Jakub Jelinek  
>
>   PR target/92908
>   * simplify-rtx.c (simplify_relational_operation): For vector cmp_mode
>   and scalar mode, if simplify_relational_operation returned
>   const_true_rtx, return a scalar bitmask of all ones.
>   (simplify_const_relational_operation): Change VOID_mode in function
>   comment to VOIDmode.
>
>   * gcc.target/i386/avx512bw-pr92908.c: New test.
>
>--- gcc/simplify-rtx.c.jj  2019-11-19 22:27:02.58742 +0100
>+++ gcc/simplify-rtx.c 2019-12-11 13:31:57.197809704 +0100
>@@ -5037,6 +5037,23 @@ simplify_relational_operation (enum rtx_
> return NULL_RTX;
> #endif
>   }
>+  if (VECTOR_MODE_P (cmp_mode)
>+&& SCALAR_INT_MODE_P (mode)
>+&& tem == const_true_rtx)
>+  {
>+/* Vector comparisons that expect a scalar integral
>+   bitmask.  For const0_rtx the result is already correct,
>+   for const_true_rtx we need all bits set.  */
>+int n_elts;
>+scalar_int_mode smode = as_a  (mode);
>+gcc_assert (GET_MODE_NUNITS (cmp_mode).is_constant (_elts)
>+&& GET_MODE_PRECISION (smode) <= n_elts);
>+if (GET_MODE_PRECISION (smode) == n_elts)
>+  return constm1_rtx;
>+if (n_elts < HOST_BITS_PER_WIDE_INT)
>+  return GEN_INT ((HOST_WIDE_INT_1U << n_elts) - 1);
>+return NULL_RTX;
>+  }
> 
>   return tem;
> }
>@@ -5383,7 +5400,7 @@ comparison_result (enum rtx_code code, i
> }
> 
> /* Check if the given comparison (done in the given MODE) is actually
>-   a tautology or a contradiction.  If the mode is VOID_mode, the
>+   a tautology or a contradiction.  If the mode is VOIDmode, the
>comparison is done in "infinite precision".  If no simplification
>is possible, this function returns zero.  Otherwise, it returns
>either const_true_rtx or const0_rtx.  */
>--- gcc/testsuite/gcc.target/i386/avx512bw-pr92908.c.jj2019-12-11
>14:24:12.083418762 +0100
>+++ gcc/testsuite/gcc.target/i386/avx512bw-pr92908.c   2019-12-11
>14:23:56.071665326 +0100
>@@ -0,0 +1,21 @@
>+/* PR target/92908 */
>+/* { dg-do run } */
>+/* { dg-options "-Og -fno-tree-fre -mavx512bw" } */
>+/* { dg-require-effective-target avx512bw } */
>+
>+#define AVX512BW
>+#include "avx512f-helper.h"
>+
>+typedef unsigned short V __attribute__ ((__vector_size__ (64)));
>+
>+V v;
>+
>+void
>+TEST (void)
>+{
>+  int i;
>+  v = (V) v == v;
>+  for (i = 0; i < 32; i++)
>+if (v[i] != 0x)
>+  abort ();
>+}
>
>   Jakub



Re: Aw: Re: [Patch, Fortran] PR92898 - [9/10 Regression] ICE in gfc_check_is_contiguous, at fortran/check.c:7157

2019-12-12 Thread Tobias Burnus

Hi Harald,

let's add a LGTM or OK to this – the patch is rather obvious and Steve 
explained how the now-removed check ended up in gfortran.


Thanks for the patch!

Tobias

On 12/11/19 11:24 PM, Harald Anlauf wrote:

Hi Thomas,


Gesendet: Dienstag, 10. Dezember 2019 um 23:34 Uhr
Von: "Thomas Koenig" 
An: "Harald Anlauf" , gfortran , gcc-patches 

Betreff: Re: [Patch, Fortran] PR92898 - [9/10 Regression] ICE in 
gfc_check_is_contiguous, at fortran/check.c:7157

Hello Harald,


Index: gcc/fortran/check.c
===
--- gcc/fortran/check.c (Revision 279183)
+++ gcc/fortran/check.c (Arbeitskopie)
@@ -7154,7 +7154,9 @@ bool
   gfc_check_is_contiguous (gfc_expr *array)
   {
 if (array->expr_type == EXPR_NULL
-  && array->symtree->n.sym->attr.pointer == 1)
+  && (!array->symtree ||
+ (array->symtree->n.sym &&
+  array->symtree->n.sym->attr.pointer == 1)))

I have to admit I do not understand the original code here, nor
do I quite understand your fix.

Is there any circumstance where array->expr_type == EXPR_NULL, but
is_contiguous is valid?  What would go wrong if the other tests
were removed?

Actually I do not know what the additional check was supposed to do.
Removing it does not seem to do any harm.  See below.


Index: gcc/testsuite/gfortran.dg/pr91641.f90
===
--- gcc/testsuite/gfortran.dg/pr91641.f90   (Revision 279183)
+++ gcc/testsuite/gfortran.dg/pr91641.f90   (Arbeitskopie)
@@ -1,7 +1,9 @@
   ! { dg-do compile }
   ! PR fortran/91641
-! Code conyributed by Gerhard Steinmetz
+! PR fortran/92898
+! Code contributed by Gerhard Steinmetz
   program p
  real, pointer :: z(:)
  print *, is_contiguous (null(z))! { dg-error "shall be an associated" 
}
+   print *, is_contiguous (null()) ! { dg-error "shall be an associated" }
   end

Sometimes, it is necessary to change test cases, when error messages
change.  If this is not the case, it is better to add new tests to
new test cases - this makes regression hunting much easier.

Regards

Thomas

Agreed.  Please find the modified patches below.  OK for trunk / 9 ?

Thanks,
Harald

Index: gcc/fortran/check.c
===
--- gcc/fortran/check.c (Revision 279254)
+++ gcc/fortran/check.c (Arbeitskopie)
@@ -7153,8 +7153,7 @@ gfc_check_ttynam_sub (gfc_expr *unit, gfc_expr *na
  bool
  gfc_check_is_contiguous (gfc_expr *array)
  {
-  if (array->expr_type == EXPR_NULL
-  && array->symtree->n.sym->attr.pointer == 1)
+  if (array->expr_type == EXPR_NULL)
  {
gfc_error ("Actual argument at %L of %qs intrinsic shall be an "
  "associated pointer", >where, gfc_current_intrinsic);



Index: gcc/testsuite/gfortran.dg/pr92898.f90
===
--- gcc/testsuite/gfortran.dg/pr92898.f90   (nicht existent)
+++ gcc/testsuite/gfortran.dg/pr92898.f90   (Arbeitskopie)
@@ -0,0 +1,6 @@
+! { dg-do compile }
+! PR fortran/92898
+! Code contributed by Gerhard Steinmetz
+program p
+  print *, is_contiguous (null()) ! { dg-error "shall be an associated" }
+end


2019-12-11  Harald Anlauf  

PR fortran/92898
* check.c (gfc_check_is_contiguous): Simplify check to handle
arbitrary NULL() argument.

2019-12-11  Harald Anlauf  

PR fortran/92898
* gfortran.dg/pr92898.f90: New test.