[patch][middle-end/PR102359]Not add initialization for DECL_VALUE_EXPR variables with -ftrivial-auto-var-init

2021-10-04 Thread Qing Zhao via Gcc-patches
Hi, 

This is the patch to fix this issue based on our discussion.

I have tested it on aarch64 with bootstrap and regtests. X86 bootstrap was 
done, regtests is ongoing.

Okay for trunk?

Thanks.

Qing

==
>From d349ef0145512efe7f9af2c6bbd01f636475bce3 Mon Sep 17 00:00:00 2001
From: qing zhao 
Date: Mon, 4 Oct 2021 15:26:03 -0700
Subject: [PATCH] middle-end/102359 Not add initialization for variables that
 have been  initialized by FEs.

C++ FE creates proxy variables, which have associated DECL_VALUE_EXPR
and have been initialized by FE. For such auto variable, we should not
add initialization when -ftrivial-auto-var-init presents.

gcc/ChangeLog:

2021-10-04  qing zhao  

* gimplify.c (is_decl_init_by_fe): New function.
(gimplify_decl_expr): Not add initialization for an auto variable
when it has been initialized by frontend.

gcc/testsuite/ChangeLog:

2021-10-04  qing zhao  

* g++.dg/pr102359_1.C: New test.
* g++.dg/pr102359_2.C: New test.
---
 gcc/gimplify.c| 21 -
 gcc/testsuite/g++.dg/pr102359_1.C | 13 +
 gcc/testsuite/g++.dg/pr102359_2.C | 13 +
 3 files changed, 46 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/pr102359_1.C
 create mode 100644 gcc/testsuite/g++.dg/pr102359_2.C

diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index b27776a..d6865ad 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -1819,6 +1819,19 @@ gimple_add_padding_init_for_auto_var (tree decl, bool 
is_vla,
   gimplify_seq_add_stmt (seq_p, call);
 }
 
+/* Return true if the DECL is initialized by FE.
+   If the VAR_DECL has DECL_VALUE_EXPR that was created by FE (usually C++FE),
+   it's a proxy varaible, and FE already initializd the DECL_VALUE_EXPR of it.
+*/
+static bool
+is_decl_init_by_fe (tree decl, bool is_created_by_fe)
+{
+  if (DECL_HAS_VALUE_EXPR_P (decl)
+  && is_created_by_fe)
+return true;
+  return false;
+}
+
 /* Return true if the DECL need to be automaticly initialized by the
compiler.  */
 static bool
@@ -1871,8 +1884,13 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
   if (VAR_P (decl) && !DECL_EXTERNAL (decl))
 {
   tree init = DECL_INITIAL (decl);
+  bool is_value_expr_created_by_fe = false;
   bool is_vla = false;
 
+  /* Check whether a decl has FE created VALUE_EXPR here BEFORE 
+gimplify_vla_decl creates VALUE_EXPR for vla decl.  */
+  is_value_expr_created_by_fe = DECL_HAS_VALUE_EXPR_P (decl);
+
   poly_uint64 size;
   if (!poly_int_tree_p (DECL_SIZE_UNIT (decl), )
  || (!TREE_STATIC (decl)
@@ -1934,7 +1952,8 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
   /* When there is no explicit initializer, if the user requested,
 We should insert an artifical initializer for this automatic
 variable.  */
-  else if (is_var_need_auto_init (decl))
+  else if (is_var_need_auto_init (decl)
+  && !is_decl_init_by_fe (decl, is_value_expr_created_by_fe))
{
  gimple_add_init_for_auto_var (decl,
flag_auto_var_init,
diff --git a/gcc/testsuite/g++.dg/pr102359_1.C 
b/gcc/testsuite/g++.dg/pr102359_1.C
new file mode 100644
index 000..da643cd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr102359_1.C
@@ -0,0 +1,13 @@
+/* PR middle-end/102359 ICE gimplification failed since
+   r12-3433-ga25e0b5e6ac8a77a.  */
+/* { dg-do compile } */
+/* { dg-options "-ftrivial-auto-var-init=zero" } */
+/* { dg-require-effective-target c++17 } */
+
+struct A {
+  double a = 111;
+  auto foo() {
+return [*this] { return a; };
+  }
+};
+int X = A{}.foo()();
diff --git a/gcc/testsuite/g++.dg/pr102359_2.C 
b/gcc/testsuite/g++.dg/pr102359_2.C
new file mode 100644
index 000..d026d72
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr102359_2.C
@@ -0,0 +1,13 @@
+/* PR middle-end/102359 ICE gimplification failed since
+   r12-3433-ga25e0b5e6ac8a77a.  */
+/* { dg-do run} */
+/* { dg-options "-ftrivial-auto-var-init=zero" } */
+/* { dg-require-effective-target c++17 } */
+
+int main()
+{
+ int i = 42;
+ auto l = [=]() mutable { return i; };
+ if (l() != i)
+   __builtin_abort ();
+}
-- 
1.9.1




Re: [PATCH] c++: templated static local var has value-dep addr [PR98930]

2021-10-04 Thread Jason Merrill via Gcc-patches

On 10/4/21 17:09, Patrick Palka wrote:

Here uses_template_parms is returning false for the dependent type
A<::i>, which causes tsubst_aggr_type to think it's non-dependent
and not bothering substituting into it, leading to breakage.

This patch fixes this by making has_value_dependent_address also return
true for templated static local variables.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?


OK.


PR c++/98930

gcc/cp/ChangeLog:

* pt.c (has_value_dependent_address): Return true for a static
local variable from a function template.

gcc/testsuite/ChangeLog:

* g++.dg/cpp1z/nontype4.C: New test.
* g++.dg/cpp1z/nontype4a.C: New test.
---
  gcc/cp/pt.c|  7 +++
  gcc/testsuite/g++.dg/cpp1z/nontype4.C  | 14 ++
  gcc/testsuite/g++.dg/cpp1z/nontype4a.C | 14 ++
  3 files changed, 35 insertions(+)
  create mode 100644 gcc/testsuite/g++.dg/cpp1z/nontype4.C
  create mode 100644 gcc/testsuite/g++.dg/cpp1z/nontype4a.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 5e819c9598c..06579e1be07 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -6763,8 +6763,15 @@ has_value_dependent_address (tree op)
if (DECL_P (op))
  {
tree ctx = CP_DECL_CONTEXT (op);
+
if (TYPE_P (ctx) && dependent_type_p (ctx))
return true;
+
+  if (VAR_P (op)
+ && TREE_STATIC (op)
+ && TREE_CODE (ctx) == FUNCTION_DECL
+ && type_dependent_expression_p (ctx))
+   return true;
  }
  
return false;

diff --git a/gcc/testsuite/g++.dg/cpp1z/nontype4.C 
b/gcc/testsuite/g++.dg/cpp1z/nontype4.C
new file mode 100644
index 000..3f345b339c6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/nontype4.C
@@ -0,0 +1,14 @@
+// PR c++/98930
+// { dg-do compile { target c++17 } }
+
+template
+struct A { A() { } };
+
+template
+void impl() {
+  static int i;
+  static A<> h;
+}
+
+template void impl();
+template void impl();
diff --git a/gcc/testsuite/g++.dg/cpp1z/nontype4a.C 
b/gcc/testsuite/g++.dg/cpp1z/nontype4a.C
new file mode 100644
index 000..5b742926b6d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/nontype4a.C
@@ -0,0 +1,14 @@
+// PR c++/98930
+// { dg-do compile { target c++17 } }
+
+template
+struct A { };
+
+template
+auto impl() {
+  static int i;
+  return A<>();
+}
+
+using type = decltype(impl());
+using type = decltype(impl()); // { dg-error "conflicting declaration" }





Re: [PATCH] restore ancient -Waddress for weak symbols [PR33925]

2021-10-04 Thread Eric Gallager via Gcc-patches
On Mon, Oct 4, 2021 at 2:43 PM Martin Sebor via Gcc-patches
 wrote:
>
> While resolving the recent -Waddress enhancement request (PR
> PR102103) I came across a 2007 problem report about GCC 4 having
> stopped warning for using the address of inline functions in
> equality comparisons with null.  With inline functions being
> commonplace in C++ this seems like an important use case for
> the warning.
>
> The change that resulted in suppressing the warning in these
> cases was introduced inadvertently in a fix for PR 22252.
>
> To restore the warning, the attached patch enhances
> the decl_with_nonnull_addr_p() function to return true also for
> weak symbols for which a definition has been provided.
>
> Tested on x86_64-linux and by comparing the GCC output for new
> test cases to Clang's which diagnoses all but one instance of
> these cases with either -Wtautological-pointer-compare or
> -Wpointer-bool-conversion, depending on context.

Would it make sense to use the same names as clang's flags here, too,
instead of dumping them all under -Waddress? I think the additional
granularity could be helpful for people who only want some warnings,
but not others.

> The one case where Clang doesn't issue a warning but GCC
> with the patch does is for a symbol explicitly declared with
> attribute weak for which a definition has been provided.
> I believe the address of such symbols is necessarily nonnull and
> so issuing the warning is helpful
> (both GCC and Clang fold such comparisons to a constant).
>
> Martin


Re: [PATCH RFA] vec: Fix --enable-gather-detailed-mem-stats

2021-10-04 Thread Iain Sandoe



> On 4 Oct 2021, at 22:27, Jason Merrill via Gcc-patches 
>  wrote:
> 
> On 10/4/21 14:37, Iain Sandoe wrote:
>> Hi Jason,
>>> On 4 Oct 2021, at 19:27, Jason Merrill via Gcc-patches 
>>>  wrote:
>>> 
>>> When r12-4038 introduced the global auto_vec save_opt_decoded_options, it 
>>> broke compilers configured with --enable-gather-detailed-mem-stats, due to 
>>> the memory descriptors getting discarded before the auto_vec was destroyed. 
>>>  Attached below are two approaches to making this work, either by using the 
>>> init_priority attribute, or turning vec_mem_desc into a singleton function. 
>>>  I prefer the first one, primarily because it doesn't require auto_vec 
>>> variables to force immediate allocation.  It relies on a G++ extension, but 
>>> I figure that's OK for code that is only exercised with a debugging 
>>> configure flag.
>> I suspect the problem is not necessarily that it’s a G++ extension, (init 
>> priority has some support elsewhere) - but that some targets (with 
>> non-binutils ld) cannot support it [between multiple TUs at least] even with 
>> a native G++ (Darwin at least cannot).  OTOH, there are worse broken things 
>> from this than a gathering of stats…
> 
> Hmm, that was previously handled for other linkers with the collect2 wrapper. 
>  I haven't followed what has happened with collect2 in recent years, does 
> Darwin not use it?

It does use collect2, but init_priority is, nevertheless disabled for the 
target; I will investigate some more.
thanks for the head’s up,
Iain



Re: [PATCH] restore ancient -Waddress for weak symbols [PR33925]

2021-10-04 Thread Jason Merrill via Gcc-patches

On 10/4/21 14:42, Martin Sebor wrote:

While resolving the recent -Waddress enhancement request (PR
PR102103) I came across a 2007 problem report about GCC 4 having
stopped warning for using the address of inline functions in
equality comparisons with null.  With inline functions being
commonplace in C++ this seems like an important use case for
the warning.

The change that resulted in suppressing the warning in these
cases was introduced inadvertently in a fix for PR 22252.

To restore the warning, the attached patch enhances
the decl_with_nonnull_addr_p() function to return true also for
weak symbols for which a definition has been provided.


I think you probably want to merge this function with 
fold-const.c:maybe_nonzero_address, which already handles more cases.


Jason



Re: [PATCH RFA] vec: Fix --enable-gather-detailed-mem-stats

2021-10-04 Thread Jason Merrill via Gcc-patches

On 10/4/21 14:37, Iain Sandoe wrote:

Hi Jason,


On 4 Oct 2021, at 19:27, Jason Merrill via Gcc-patches 
 wrote:

When r12-4038 introduced the global auto_vec save_opt_decoded_options, it broke 
compilers configured with --enable-gather-detailed-mem-stats, due to the memory 
descriptors getting discarded before the auto_vec was destroyed.  Attached 
below are two approaches to making this work, either by using the init_priority 
attribute, or turning vec_mem_desc into a singleton function.  I prefer the 
first one, primarily because it doesn't require auto_vec variables to force 
immediate allocation.  It relies on a G++ extension, but I figure that's OK for 
code that is only exercised with a debugging configure flag.


I suspect the problem is not necessarily that it’s a G++ extension, (init 
priority has some support elsewhere) - but that some targets (with non-binutils 
ld) cannot support it [between multiple TUs at least] even with a native G++ 
(Darwin at least cannot).  OTOH, there are worse broken things from this than a 
gathering of stats…


Hmm, that was previously handled for other linkers with the collect2 
wrapper.  I haven't followed what has happened with collect2 in recent 
years, does Darwin not use it?


Jason



[PATCH] c++: templated static local var has value-dep addr [PR98930]

2021-10-04 Thread Patrick Palka via Gcc-patches
Here uses_template_parms is returning false for the dependent type
A<::i>, which causes tsubst_aggr_type to think it's non-dependent
and not bothering substituting into it, leading to breakage.

This patch fixes this by making has_value_dependent_address also return
true for templated static local variables.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

PR c++/98930

gcc/cp/ChangeLog:

* pt.c (has_value_dependent_address): Return true for a static
local variable from a function template.

gcc/testsuite/ChangeLog:

* g++.dg/cpp1z/nontype4.C: New test.
* g++.dg/cpp1z/nontype4a.C: New test.
---
 gcc/cp/pt.c|  7 +++
 gcc/testsuite/g++.dg/cpp1z/nontype4.C  | 14 ++
 gcc/testsuite/g++.dg/cpp1z/nontype4a.C | 14 ++
 3 files changed, 35 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/nontype4.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/nontype4a.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 5e819c9598c..06579e1be07 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -6763,8 +6763,15 @@ has_value_dependent_address (tree op)
   if (DECL_P (op))
 {
   tree ctx = CP_DECL_CONTEXT (op);
+
   if (TYPE_P (ctx) && dependent_type_p (ctx))
return true;
+
+  if (VAR_P (op)
+ && TREE_STATIC (op)
+ && TREE_CODE (ctx) == FUNCTION_DECL
+ && type_dependent_expression_p (ctx))
+   return true;
 }
 
   return false;
diff --git a/gcc/testsuite/g++.dg/cpp1z/nontype4.C 
b/gcc/testsuite/g++.dg/cpp1z/nontype4.C
new file mode 100644
index 000..3f345b339c6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/nontype4.C
@@ -0,0 +1,14 @@
+// PR c++/98930
+// { dg-do compile { target c++17 } }
+
+template
+struct A { A() { } };
+
+template
+void impl() {
+  static int i;
+  static A<> h;
+}
+
+template void impl();
+template void impl();
diff --git a/gcc/testsuite/g++.dg/cpp1z/nontype4a.C 
b/gcc/testsuite/g++.dg/cpp1z/nontype4a.C
new file mode 100644
index 000..5b742926b6d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/nontype4a.C
@@ -0,0 +1,14 @@
+// PR c++/98930
+// { dg-do compile { target c++17 } }
+
+template
+struct A { };
+
+template
+auto impl() {
+  static int i;
+  return A<>();
+}
+
+using type = decltype(impl());
+using type = decltype(impl()); // { dg-error "conflicting declaration" }
-- 
2.33.0.610.gcefe983a32



Re: [PATCH][i386] target: support spaces in target attribute.

2021-10-04 Thread Andrew Pinski via Gcc-patches
On Mon, Oct 4, 2021 at 8:11 AM Martin Liška  wrote:
>
> Hello.
>
> The patch is about supporting target attribute values like "no-avx, sse2   ".
> I'm planning doing the same change for aarch64 and other archs as well once
> this one is accepted.

It might be useful to skip tabs for the same reason as spaces really.

Thanks,
Andrew

>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
> Thanks,
> Martin
>
> PR target/102374
>
> gcc/ChangeLog:
>
> * config/i386/i386-options.c (ix86_valid_target_attribute_inner_p):
> * system.h (strip_spaces):
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/i386/pr102374.c: New test.
> ---
>   gcc/config/i386/i386-options.c   |  2 ++
>   gcc/system.h | 21 +
>   gcc/testsuite/gcc.target/i386/pr102374.c |  3 +++
>   3 files changed, 26 insertions(+)
>   create mode 100644 gcc/testsuite/gcc.target/i386/pr102374.c
>
> diff --git a/gcc/config/i386/i386-options.c b/gcc/config/i386/i386-options.c
> index e7a3bd4aaea..c88bc77b04d 100644
> --- a/gcc/config/i386/i386-options.c
> +++ b/gcc/config/i386/i386-options.c
> @@ -1146,6 +1146,8 @@ ix86_valid_target_attribute_inner_p (tree fndecl, tree 
> args, char *p_strings[],
>   next_optstr = NULL;
> }
>
> +  p = strip_spaces (p, );
> +
> /* Recognize no-xxx.  */
> if (len > 3 && p[0] == 'n' && p[1] == 'o' && p[2] == '-')
> {
> diff --git a/gcc/system.h b/gcc/system.h
> index adde3e264b6..35b15203a96 100644
> --- a/gcc/system.h
> +++ b/gcc/system.h
> @@ -1305,4 +1305,25 @@ startswith (const char *str, const char *prefix)
> return strncmp (str, prefix, strlen (prefix)) == 0;
>   }
>
> +/* Strip spaces from STRING with LEN length.
> +   A stripped string is returned and LEN is updated accordingly.  */
> +
> +static inline char *
> +strip_spaces (char *string, size_t *len)
> +{
> +  while (string[0] == ' ')
> +{
> +  --(*len);
> +  ++string;
> +}
> +
> +  while (string[*len - 1] == ' ')
> +{
> +  string[*len - 1] = '\0';
> +  --(*len);
> +}
> +
> +  return string;
> +}
> +
>   #endif /* ! GCC_SYSTEM_H */
> diff --git a/gcc/testsuite/gcc.target/i386/pr102374.c 
> b/gcc/testsuite/gcc.target/i386/pr102374.c
> new file mode 100644
> index 000..db84aeed9a5
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr102374.c
> @@ -0,0 +1,3 @@
> +/* PR target/102374 */
> +
> +void calculate_sse(void) __attribute__ ((__target__ ("no-avx, sse2   ")));
> --
> 2.33.0
>


Re: [PATCH] Remove static marker for range in alloca pass.

2021-10-04 Thread Andrew MacLeod via Gcc-patches

On 10/4/21 4:15 AM, Richard Biener via Gcc-patches wrote:

On Mon, Oct 4, 2021 at 8:55 AM Aldy Hernandez via Gcc-patches
 wrote:

The m_ranges[] field in int_range are trees, so they live in GC
space.  Since invalid_range is static, it must be marked with GTY
magic.  However, calculating invalid_range is not particularly slow,
or on a critical path, so we can just put it in local scope and
recalculate every time.

Tested on x86-64 Linux.

Since this is more of a GC thing than a range thing, I'd like a nod from
a global reviewer.

OK?

OK, but can we somehow make int_range<>::intersect work
with non-tree as well?  That is, can we somehow make this
operation w/o constructing temporary trees?

Thanks,
Richard.


Yeah, I'll shortly provide an intersect which takes 2 wide_ints, along 
with some other performance improvements.


 It can be more efficient since the general case has to build into a 
temporary range, and if its single pair, we can go directly into the 
original range, and possible even avoid ever creating a new tree.


Andrew




[PATCH, rs6000 GCC-11 committed] Fix fusion testcase counts

2021-10-04 Thread Pat Haugen via Gcc-patches
Somehow there was an issue when a larger patch was backported, and this chunk 
did not make it. Tested and committed as obvious.

-Pat


2021-10-04  Pat Haugen  

gcc/testsuite/ChangeLog:

* gcc.target/powerpc/fusion-p10-ldcmpi.c: Update counts.


diff --git a/gcc/testsuite/gcc.target/powerpc/fusion-p10-ldcmpi.c 
b/gcc/testsuite/gcc.target/powerpc/fusion-p10-ldcmpi.c
index ea1d5d01950..526a026d874 100644
--- a/gcc/testsuite/gcc.target/powerpc/fusion-p10-ldcmpi.c
+++ b/gcc/testsuite/gcc.target/powerpc/fusion-p10-ldcmpi.c
@@ -53,16 +53,16 @@ TEST(int16_t)
 TEST(uint8_t)
 TEST(int8_t)
 
-/* { dg-final { scan-assembler-times "lbz_cmpldi_cr0_QI_clobber_CCUNS_zero"   
2 { target lp64 } } } */
+/* { dg-final { scan-assembler-times "lbz_cmpldi_cr0_QI_clobber_CCUNS_zero"   
4 { target lp64 } } } */
 /* { dg-final { scan-assembler-times "ld_cmpdi_cr0_DI_DI_CC_none" 
4 { target lp64 } } } */
 /* { dg-final { scan-assembler-times "ld_cmpdi_cr0_DI_clobber_CC_none"
4 { target lp64 } } } */
 /* { dg-final { scan-assembler-times "ld_cmpldi_cr0_DI_DI_CCUNS_none" 
1 { target lp64 } } } */
 /* { dg-final { scan-assembler-times "ld_cmpldi_cr0_DI_clobber_CCUNS_none"
1 { target lp64 } } } */
-/* { dg-final { scan-assembler-times "lha_cmpdi_cr0_HI_clobber_CC_sign"   
8 { target lp64 } } } */
-/* { dg-final { scan-assembler-times "lhz_cmpldi_cr0_HI_clobber_CCUNS_zero"   
2 { target lp64 } } } */
-/* { dg-final { scan-assembler-times "lwa_cmpdi_cr0_SI_EXTSI_CC_sign" 
3 { target lp64 } } } */
+/* { dg-final { scan-assembler-times "lha_cmpdi_cr0_HI_clobber_CC_sign"  
16 { target lp64 } } } */
+/* { dg-final { scan-assembler-times "lhz_cmpldi_cr0_HI_clobber_CCUNS_zero"   
4 { target lp64 } } } */
+/* { dg-final { scan-assembler-times "lwa_cmpdi_cr0_SI_EXTSI_CC_sign" 
0 { target lp64 } } } */
 /* { dg-final { scan-assembler-times "lwa_cmpdi_cr0_SI_clobber_CC_none"   
4 { target lp64 } } } */
-/* { dg-final { scan-assembler-times "lwz_cmpldi_cr0_SI_EXTSI_CCUNS_zero" 
2 { target lp64 } } } */
+/* { dg-final { scan-assembler-times "lwz_cmpldi_cr0_SI_EXTSI_CCUNS_zero" 
0 { target lp64 } } } */
 /* { dg-final { scan-assembler-times "lwz_cmpldi_cr0_SI_clobber_CCUNS_none"   
2 { target lp64 } } } */
 
 /* { dg-final { scan-assembler-times "lbz_cmpldi_cr0_QI_clobber_CCUNS_zero"   
2 { target ilp32 } } } */


Re: [committed] libstdc++: Specialize std::pointer_traits<__normal_iterator>

2021-10-04 Thread Jonathan Wakely via Gcc-patches
On Mon, 4 Oct 2021 at 21:28, François Dumont via Libstdc++
 wrote:
>
> On 04/10/21 10:05 pm, François Dumont wrote:
> > On 02/10/21 10:24 pm, Jonathan Wakely wrote:
> >> On Sat, 2 Oct 2021 at 18:27, François Dumont wrote:
> >>> I would like to propose this alternative approach.
> >>>
> >>> In this patch I make __normal_iterator and random iterator
> >>> _Safe_iterator compatible for pointer_traits primary template.
> >>>
> >>> Regarding pointer_traits I wonder if it shouldn't check for the
> >>> to_pointer method availability and use per default: return {
> >>> std::addressof(__e) }; otherwise. This way we wouldn't have to
> >>> provide a
> >>> pointer_to method on __normal_iterator.
> >> But I would rather not have these members present in vector::iterator
> >> and string::iterator, in case users accidentally start to rely on them
> >> being present.
> >
> > Making pointer_traits friends would help but I do not like it neither.
> >
> >
> >>
> >> Another option would be to overload std::__to_address so it knows how
> >> to get the address from __normal_iterator and _Safe_iterator.
> >>
> >> .
> >
> > I start thinking that rather than proposing not-useful and even
> > incorrect code in the case of the _Safe_iterator<> it might be a
> > better approach.
> >
> > Even the rebind for __normal_iterator is a little strange because when
> > doing rebind on std::vector::iterator for long it produces
> > __normal_iterator>, quite inconsistent even if
> > useless.
> >
> > But there's something that I'm missing, what is the relation between
> > __addressof and std::pointer_traits ? Is it that __builtin_addressof
> > is using it ?
> >
> Ignore this last question, I realized that we are talking about
> __to_address, not __addressof.

Yes, see the definition of std::__to_address in 
and also PR 96416.


Re: [committed] libstdc++: Specialize std::pointer_traits<__normal_iterator>

2021-10-04 Thread François Dumont via Gcc-patches

On 04/10/21 10:05 pm, François Dumont wrote:

On 02/10/21 10:24 pm, Jonathan Wakely wrote:

On Sat, 2 Oct 2021 at 18:27, François Dumont wrote:

I would like to propose this alternative approach.

In this patch I make __normal_iterator and random iterator
_Safe_iterator compatible for pointer_traits primary template.

Regarding pointer_traits I wonder if it shouldn't check for the
to_pointer method availability and use per default: return {
std::addressof(__e) }; otherwise. This way we wouldn't have to 
provide a

pointer_to method on __normal_iterator.

But I would rather not have these members present in vector::iterator
and string::iterator, in case users accidentally start to rely on them
being present.


Making pointer_traits friends would help but I do not like it neither.




Another option would be to overload std::__to_address so it knows how
to get the address from __normal_iterator and _Safe_iterator.

.


I start thinking that rather than proposing not-useful and even 
incorrect code in the case of the _Safe_iterator<> it might be a 
better approach.


Even the rebind for __normal_iterator is a little strange because when 
doing rebind on std::vector::iterator for long it produces 
__normal_iterator>, quite inconsistent even if 
useless.


But there's something that I'm missing, what is the relation between 
__addressof and std::pointer_traits ? Is it that __builtin_addressof 
is using it ?


Ignore this last question, I realized that we are talking about 
__to_address, not __addressof.




Re: [committed] libstdc++: Specialize std::pointer_traits<__normal_iterator>

2021-10-04 Thread François Dumont via Gcc-patches

On 02/10/21 10:24 pm, Jonathan Wakely wrote:

On Sat, 2 Oct 2021 at 18:27, François Dumont wrote:

I would like to propose this alternative approach.

In this patch I make __normal_iterator and random iterator
_Safe_iterator compatible for pointer_traits primary template.

Regarding pointer_traits I wonder if it shouldn't check for the
to_pointer method availability and use per default: return {
std::addressof(__e) }; otherwise. This way we wouldn't have to provide a
pointer_to method on __normal_iterator.

But I would rather not have these members present in vector::iterator
and string::iterator, in case users accidentally start to rely on them
being present.


Making pointer_traits friends would help but I do not like it neither.




Another option would be to overload std::__to_address so it knows how
to get the address from __normal_iterator and _Safe_iterator.

.


I start thinking that rather than proposing not-useful and even 
incorrect code in the case of the _Safe_iterator<> it might be a better 
approach.


Even the rebind for __normal_iterator is a little strange because when 
doing rebind on std::vector::iterator for long it produces 
__normal_iterator>, quite inconsistent even if 
useless.


But there's something that I'm missing, what is the relation between 
__addressof and std::pointer_traits ? Is it that __builtin_addressof is 
using it ?




[r12-4143 Regression] FAIL: gfortran.dg/predict-2.f90 -O scan-tree-dump-times profile_estimate "Fortran loop preheader heuristics of edge" 2 on Linux/x86_64

2021-10-04 Thread sunil.k.pandey via Gcc-patches
On Linux/x86_64,

55a3be2f5255d69e740d61b2c5aaba1ccbc643b8 is the first bad commit
commit 55a3be2f5255d69e740d61b2c5aaba1ccbc643b8
Author: Richard Biener 
Date:   Mon Oct 4 10:57:45 2021 +0200

tree-optimization/102570 - teach VN about internal functions

caused

FAIL: gfortran.dg/predict-2.f90   -O   scan-tree-dump-times profile_estimate 
"Fortran loop preheader heuristics of edge" 2

with GCC configured with

../../gcc/configure 
--prefix=/local/skpandey/gccwork/toolwork/gcc-bisect-master/master/r12-4143/usr 
--enable-clocale=gnu --with-system-zlib --with-demangler-in-ld 
--with-fpmath=sse --enable-languages=c,c++,fortran --enable-cet --without-isl 
--enable-libmpx x86_64-linux --disable-bootstrap

To reproduce:

$ cd {build_dir}/gcc && make check 
RUNTESTFLAGS="dg.exp=gfortran.dg/predict-2.f90 --target_board='unix{-m32}'"
$ cd {build_dir}/gcc && make check 
RUNTESTFLAGS="dg.exp=gfortran.dg/predict-2.f90 --target_board='unix{-m32\ 
-march=cascadelake}'"
$ cd {build_dir}/gcc && make check 
RUNTESTFLAGS="dg.exp=gfortran.dg/predict-2.f90 --target_board='unix{-m64}'"
$ cd {build_dir}/gcc && make check 
RUNTESTFLAGS="dg.exp=gfortran.dg/predict-2.f90 --target_board='unix{-m64\ 
-march=cascadelake}'"

(Please do not reply to this email, for question about this report, contact me 
at skpgkp2 at gmail dot com)


[PATCH] restore ancient -Waddress for weak symbols [PR33925]

2021-10-04 Thread Martin Sebor via Gcc-patches

While resolving the recent -Waddress enhancement request (PR
PR102103) I came across a 2007 problem report about GCC 4 having
stopped warning for using the address of inline functions in
equality comparisons with null.  With inline functions being
commonplace in C++ this seems like an important use case for
the warning.

The change that resulted in suppressing the warning in these
cases was introduced inadvertently in a fix for PR 22252.

To restore the warning, the attached patch enhances
the decl_with_nonnull_addr_p() function to return true also for
weak symbols for which a definition has been provided.

Tested on x86_64-linux and by comparing the GCC output for new
test cases to Clang's which diagnoses all but one instance of
these cases with either -Wtautological-pointer-compare or
-Wpointer-bool-conversion, depending on context.  The one case
where Clang doesn't issue a warning but GCC with the patch does
is for a symbol explicitly declared with attribute weak for which
a definition has been provided.  I believe the address of such
symbols is necessarily nonnull and so issuing the warning is
helpful (both GCC and Clang fold such comparisons to a constant).

Martin
PR c/33925 - missing -Waddress with the address of an inline function

gcc/c-family/ChangeLog:

	PR c/33925
	* c-common.c (decl_with_nonnull_addr_p): Also return true for weak
	symbols for which a definition has been provided.

gcc/testsuite/ChangeLog:

	PR c/33925
	* c-c++-common/Waddress-5.c: New test.
	* g++.dg/warn/Waddress-7.C: New test.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 9d19e352725..ba02d3981c0 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -3395,7 +3395,8 @@ c_wrap_maybe_const (tree expr, bool non_const)
 
 /* Return whether EXPR is a declaration whose address can never be NULL.
The address of the first struct member could be NULL only if it were
-   accessed through a NULL pointer, and such an access would be invalid.  */
+   accessed through a NULL pointer, and such an access would be invalid.
+   The address of a weak symbol may be null unless it has a definition.  */
 
 bool
 decl_with_nonnull_addr_p (const_tree expr)
@@ -3404,7 +3405,9 @@ decl_with_nonnull_addr_p (const_tree expr)
 	  && (TREE_CODE (expr) == FIELD_DECL
 	  || TREE_CODE (expr) == PARM_DECL
 	  || TREE_CODE (expr) == LABEL_DECL
-	  || !DECL_WEAK (expr)));
+	  || !DECL_WEAK (expr)
+	  || (DECL_INITIAL (expr)
+		  && DECL_INITIAL (expr) != error_mark_node)));
 }
 
 /* Prepare expr to be an argument of a TRUTH_NOT_EXPR,
diff --git a/gcc/testsuite/c-c++-common/Waddress-5.c b/gcc/testsuite/c-c++-common/Waddress-5.c
new file mode 100644
index 000..fa6658fa133
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Waddress-5.c
@@ -0,0 +1,133 @@
+/* PR c/33925 - missing -Waddress with the address of an inline function
+   { dg-do compile }
+   { dg-options "-Wall" }
+   { dg-require-weak "" } */
+
+extern inline int eifn (void);
+extern inline int eifn_def (void) { return 0; }
+
+static inline int sifn (void);
+static inline int sifn_def (void) { return 0; }
+
+inline int ifn (void);
+inline int ifn_def (void) { return 0; }
+
+extern __attribute__ ((weak)) int ewfn (void);
+extern __attribute__ ((weak)) int ewfn_def (void) { return 0;  }
+
+__attribute__ ((weak)) int wfn (void);
+__attribute__ ((weak)) int wfn_def (void) { return 0; }
+
+static __attribute__((weakref ("ewfn"))) int swrfn;
+
+void test_function_eqz (int *p)
+{
+  *p++ = eifn == 0; // { dg-warning "-Waddress" }
+  *p++ = eifn_def == 0; // { dg-warning "-Waddress" }
+  *p++ = sifn == 0; // { dg-warning "-Waddress" }
+  *p++ = sifn_def == 0; // { dg-warning "-Waddress" }
+  *p++ = ifn == 0;  // { dg-warning "-Waddress" }
+  *p++ = ifn_def == 0;  // { dg-warning "-Waddress" }
+  *p++ = ewfn == 0;
+  *p++ = ewfn_def == 0; // { dg-warning "-Waddress" }
+  *p++ = wfn == 0;
+  *p++ = wfn_def == 0;  // { dg-warning "-Waddress" }
+  *p++ = swrfn == 0;
+}
+
+
+int test_function_if (int i)
+{
+  if (eifn) // { dg-warning "-Waddress" }
+i++;
+  if (eifn_def) // { dg-warning "-Waddress" }
+i++;
+  if (sifn) // { dg-warning "-Waddress" }
+i++;
+  if (sifn_def) // { dg-warning "-Waddress" }
+i++;
+  if (ifn)  // { dg-warning "-Waddress" }
+i++;
+  if (ifn_def)  // { dg-warning "-Waddress" }
+i++;
+  if (ewfn)
+i++;
+  if (ewfn_def) // { dg-warning "-Waddress" }
+i++;
+  if (wfn)
+i++;
+  if(wfn_def)   // { dg-warning "-Waddress" }
+i++;
+  if (swrfn)
+i++;
+  return i;
+}
+
+
+extern int ei;
+extern int ei_def = 1;
+
+static int si;
+static int si_def = 1;
+

Re: [PATCH RFA] vec: Fix --enable-gather-detailed-mem-stats

2021-10-04 Thread Iain Sandoe
Hi Jason,

> On 4 Oct 2021, at 19:27, Jason Merrill via Gcc-patches 
>  wrote:
> 
> When r12-4038 introduced the global auto_vec save_opt_decoded_options, it 
> broke compilers configured with --enable-gather-detailed-mem-stats, due to 
> the memory descriptors getting discarded before the auto_vec was destroyed.  
> Attached below are two approaches to making this work, either by using the 
> init_priority attribute, or turning vec_mem_desc into a singleton function.  
> I prefer the first one, primarily because it doesn't require auto_vec 
> variables to force immediate allocation.  It relies on a G++ extension, but I 
> figure that's OK for code that is only exercised with a debugging configure 
> flag.

I suspect the problem is not necessarily that it’s a G++ extension, (init 
priority has some support elsewhere) - but that some targets (with non-binutils 
ld) cannot support it [between multiple TUs at least] even with a native G++ 
(Darwin at least cannot).  OTOH, there are worse broken things from this than a 
gathering of stats…
Iain

> Thoughts?  Either one OK for 
> trunk?<0001-vec-Fix-enable-gather-detailed-mem-stats.patch><0002-vec-make-vec_mem_desc-a-singleton-function.patch>



[PATCH RFA] vec: Fix --enable-gather-detailed-mem-stats

2021-10-04 Thread Jason Merrill via Gcc-patches
When r12-4038 introduced the global auto_vec save_opt_decoded_options, 
it broke compilers configured with --enable-gather-detailed-mem-stats, 
due to the memory descriptors getting discarded before the auto_vec was 
destroyed.  Attached below are two approaches to making this work, 
either by using the init_priority attribute, or turning vec_mem_desc 
into a singleton function.  I prefer the first one, primarily because it 
doesn't require auto_vec variables to force immediate allocation.  It 
relies on a G++ extension, but I figure that's OK for code that is only 
exercised with a debugging configure flag.


Thoughts?  Either one OK for trunk?From b12018cb7842a0bab1f5bcf259f73ec17186f4ad Mon Sep 17 00:00:00 2001
From: Jason Merrill 
Date: Sat, 2 Oct 2021 05:45:02 -0400
Subject: [PATCH 1/2] vec: Fix --enable-gather-detailed-mem-stats
To: gcc-patches@gcc.gnu.org

When r12-4038 introduced the global auto_vec save_opt_decoded_options, it
broke compilers configured with --enable-gather-detailed-mem-stats due to
the memory descriptors getting discarded before the auto_vec was destroyed.
We can fix the ordering of construction/destruction using the init_priority
attribute.

gcc/ChangeLog:

	* vec.c (vec_mem_desc): Add init_priority attribute.
---
 gcc/vec.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/gcc/vec.c b/gcc/vec.c
index 6d767cc12c1..715460397c6 100644
--- a/gcc/vec.c
+++ b/gcc/vec.c
@@ -111,6 +111,11 @@ public:
 };
 
 /* Vector memory description.  */
+#if GATHER_STATISTICS
+/* With --enable-gather-detailed-mem-stats, this needs to be constructed before
+   any auto_vec variables.  The number 237 is entirely arbitrary.  */
+[[gnu::init_priority (237)]]
+#endif
 static mem_alloc_description  vec_mem_desc;
 
 /* Account the overhead.  */
-- 
2.27.0

From 7641bceec2cb53e76c57bd70c38f390610bb82b6 Mon Sep 17 00:00:00 2001
From: Jason Merrill 
Date: Fri, 1 Oct 2021 16:09:57 -0400
Subject: [PATCH 2/2] vec: make vec_mem_desc a singleton function
To: gcc-patches@gcc.gnu.org

When r12-4038 introduced the global auto_vec save_opt_decoded_options, it
broke compilers configured with --enable-gather-detailed-mem-stats due to
the memory descriptors getting discarded before the auto_vec was destroyed.
We can fix the ordering of construction/destruction by turning vec_mem_desc
into a singleton function.

For this to work, the construction of save_opt_decoded_options needs to
allocate immediately, so that allocation calls vec_mem_desc.

gcc/ChangeLog:

	* vec.c (vec_mem_desc): Change to function.
	(vec_prefix::register_overhead): Adjust.
	(vec_prefix::release_overhead): Adjust.
	(dump_vec_loc_statistics): Adjust.
	* toplev.c (save_opt_decoded_options): Allocate.
---
 gcc/toplev.c |  7 ++-
 gcc/vec.c| 28 +++-
 2 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/gcc/toplev.c b/gcc/toplev.c
index ec9f998a49b..727a9972dd0 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -122,7 +122,12 @@ struct cl_decoded_option *save_decoded_options;
 unsigned int save_decoded_options_count;
 
 /* Vector of saved Optimization decoded command line options.  */
-auto_vec save_opt_decoded_options;
+auto_vec save_opt_decoded_options
+#if GATHER_STATISTICS
+/* Force allocation so vec_mem_desc is called.  */
+(1)
+#endif
+;
 
 /* Used to enable -fvar-tracking, -fweb and -frename-registers according
to optimize in process_options ().  */
diff --git a/gcc/vec.c b/gcc/vec.c
index 715460397c6..4858753223c 100644
--- a/gcc/vec.c
+++ b/gcc/vec.c
@@ -110,13 +110,15 @@ public:
   size_t m_element_size;
 };
 
-/* Vector memory description.  */
-#if GATHER_STATISTICS
-/* With --enable-gather-detailed-mem-stats, this needs to be constructed before
-   any auto_vec variables.  The number 237 is entirely arbitrary.  */
-[[gnu::init_priority (237)]]
-#endif
-static mem_alloc_description  vec_mem_desc;
+/* Vector memory description.  This is a singleton function rather than a
+   variable so that the object's [cd]tor are properly ordered relative to any
+   auto_vec variables.  */
+static mem_alloc_description  &
+vec_mem_desc ()
+{
+  static mem_alloc_description m;
+  return m;
+}
 
 /* Account the overhead.  */
 
@@ -124,10 +126,10 @@ void
 vec_prefix::register_overhead (void *ptr, size_t elements,
 			   size_t element_size MEM_STAT_DECL)
 {
-  vec_mem_desc.register_descriptor (ptr, VEC_ORIGIN, false
+  vec_mem_desc ().register_descriptor (ptr, VEC_ORIGIN, false
 FINAL_PASS_MEM_STAT);
   vec_usage *usage
-= vec_mem_desc.register_instance_overhead (elements * element_size, ptr);
+= vec_mem_desc ().register_instance_overhead (elements * element_size, ptr);
   usage->m_element_size = element_size;
   usage->m_items += elements;
   if (usage->m_items_peak < usage->m_items)
@@ -140,10 +142,10 @@ void
 vec_prefix::release_overhead (void *ptr, size_t size, size_t elements,
 			  bool in_dtor MEM_STAT_DECL)
 {
-  if (!vec_mem_desc.contains_descriptor_for_instance (ptr))
-

Re: [PATCH v3 0/6] rs6000: Support more SSE4 intrinsics

2021-10-04 Thread Paul A. Clarke via Gcc-patches
Ping.

On Thu, Sep 16, 2021 at 09:59:39AM -0500, Paul A. Clarke via Gcc-patches wrote:
> Ping.
> 
> On Mon, Aug 23, 2021 at 02:03:04PM -0500, Paul A. Clarke via Gcc-patches 
> wrote:
> > v3: Add "nmmintrin.h". _mm_cmpgt_epi64 is part of SSE4.2
> > and users will expect to be able to include "nmmintrin.h",
> > even though "nmmintrin.h" just includes "smmintrin.h"
> > where all of the SSE4.2 implementations actually appear.
> > 
> > Only patch 5/6 changed from v2.
> > 
> > Tested ppc64le (POWER9) and ppc64/32 (POWER7).
> > 
> > OK for trunk?
> > 
> > Paul A. Clarke (6):
> >   rs6000: Support SSE4.1 "round" intrinsics
> >   rs6000: Support SSE4.1 "min" and "max" intrinsics
> >   rs6000: Simplify some SSE4.1 "test" intrinsics
> >   rs6000: Support SSE4.1 "cvt" intrinsics
> >   rs6000: Support more SSE4 "cmp", "mul", "pack" intrinsics
> >   rs6000: Guard some x86 intrinsics implementations
> > 
> >  gcc/config/rs6000/emmintrin.h |  12 +-
> >  gcc/config/rs6000/nmmintrin.h |  40 ++
> >  gcc/config/rs6000/pmmintrin.h |   4 +
> >  gcc/config/rs6000/smmintrin.h | 427 --
> >  gcc/config/rs6000/tmmintrin.h |  12 +
> >  gcc/testsuite/gcc.target/powerpc/pr78102.c|  23 +
> >  .../gcc.target/powerpc/sse4_1-packusdw.c  |  73 +++
> >  .../gcc.target/powerpc/sse4_1-pcmpeqq.c   |  46 ++
> >  .../gcc.target/powerpc/sse4_1-pmaxsb.c|  46 ++
> >  .../gcc.target/powerpc/sse4_1-pmaxsd.c|  46 ++
> >  .../gcc.target/powerpc/sse4_1-pmaxud.c|  47 ++
> >  .../gcc.target/powerpc/sse4_1-pmaxuw.c|  47 ++
> >  .../gcc.target/powerpc/sse4_1-pminsb.c|  46 ++
> >  .../gcc.target/powerpc/sse4_1-pminsd.c|  46 ++
> >  .../gcc.target/powerpc/sse4_1-pminud.c|  47 ++
> >  .../gcc.target/powerpc/sse4_1-pminuw.c|  47 ++
> >  .../gcc.target/powerpc/sse4_1-pmovsxbd.c  |  42 ++
> >  .../gcc.target/powerpc/sse4_1-pmovsxbq.c  |  42 ++
> >  .../gcc.target/powerpc/sse4_1-pmovsxbw.c  |  42 ++
> >  .../gcc.target/powerpc/sse4_1-pmovsxdq.c  |  42 ++
> >  .../gcc.target/powerpc/sse4_1-pmovsxwd.c  |  42 ++
> >  .../gcc.target/powerpc/sse4_1-pmovsxwq.c  |  42 ++
> >  .../gcc.target/powerpc/sse4_1-pmovzxbd.c  |  43 ++
> >  .../gcc.target/powerpc/sse4_1-pmovzxbq.c  |  43 ++
> >  .../gcc.target/powerpc/sse4_1-pmovzxbw.c  |  43 ++
> >  .../gcc.target/powerpc/sse4_1-pmovzxdq.c  |  43 ++
> >  .../gcc.target/powerpc/sse4_1-pmovzxwd.c  |  43 ++
> >  .../gcc.target/powerpc/sse4_1-pmovzxwq.c  |  43 ++
> >  .../gcc.target/powerpc/sse4_1-pmuldq.c|  51 +++
> >  .../gcc.target/powerpc/sse4_1-pmulld.c|  46 ++
> >  .../gcc.target/powerpc/sse4_1-round3.h|  81 
> >  .../gcc.target/powerpc/sse4_1-roundpd.c   | 143 ++
> >  .../gcc.target/powerpc/sse4_1-roundps.c   |  98 
> >  .../gcc.target/powerpc/sse4_1-roundsd.c   | 256 +++
> >  .../gcc.target/powerpc/sse4_1-roundss.c   | 208 +
> >  .../gcc.target/powerpc/sse4_2-check.h |  18 +
> >  .../gcc.target/powerpc/sse4_2-pcmpgtq.c   |  46 ++
> >  37 files changed, 2407 insertions(+), 59 deletions(-)
> >  create mode 100644 gcc/config/rs6000/nmmintrin.h
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/pr78102.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-packusdw.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-pcmpeqq.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-pmaxsb.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-pmaxsd.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-pmaxud.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-pmaxuw.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-pminsb.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-pminsd.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-pminud.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-pminuw.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-pmovsxbd.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-pmovsxbq.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-pmovsxbw.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-pmovsxdq.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-pmovsxwd.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-pmovsxwq.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-pmovzxbd.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-pmovzxbq.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-pmovzxbw.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-pmovzxdq.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-pmovzxwd.c
> >  create mode 100644 gcc/testsuite/gcc.target/powerpc/sse4_1-pmovzxwq.c
> >  create mode 100644 

Re: [PATCH] middle-end/102587 - avoid auto-init for VLA vectors

2021-10-04 Thread Qing Zhao via Gcc-patches


> On Oct 4, 2021, at 12:19 PM, Richard Biener  wrote:
> 
> On October 4, 2021 7:00:10 PM GMT+02:00, Qing Zhao  
> wrote:
>> I have several questions on this fix:
>> 
>> 1. This fix avoided expanding “.DEFERRED_INIT” when !tree_fits_uhwi_p 
>> (TYPE_SIZE_UNIT (var_type)).
>>   As a result, this call to .DEFERRED_INIT will NOT be expanded at all.
> 
> Yes. 

Then, should we exclude such auto init during gimplification phase?

> 
>>   Then not expanding .DEFERRED_INIT in RTL expanding phase will trigger more 
>> issues in later RTL phases, this looks not correct to me. (Actually, with is 
>> the patch, this testing case still failed in a later RTL stage). 
>> 
>>   So, If we really want to avoid auto-init for VLA vectors, we should not 
>> add call to .DEFERRED_INIT in gimplification phase at all. 


>> 
>> 
>> 2. For the added .DEFERRED_INIT:
>> 
>> __SVFloat64_t f64;
>> 
>> f64 = .DEFERRED_INIT (POLY_INT_CST [16, 16], 2, 0);
>> 
>> What does “POLY_INT_CST[16,16]” mean? Is this a constant size? If YES, 
>> what’s the value of it? If Not, can we use “memset” to expand it?
> 
> When the target is a register memset doesn't work. I'm not sure the memset 
> expansion path will work as-is either for aggregates with vla parts -

Stupid question here:  what does POLY_INT_CST[16,16] mean?   It’s not a 
constant? 

Qing

> but I'll leave that to Richard S. to sort out. 


> 
> Richard. 
> 
>> Thanks.
>> 
>> Qing
>> 
>> 
>> 
>>> On Oct 4, 2021, at 3:57 AM, Richard Biener via Gcc-patches 
>>>  wrote:
>>> 
>>> This avoids ICEing for VLA vector auto-init by not initializing.
>>> 
>>> Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
>>> 
>>> 2021-10-04  Richard Biener  
>>> 
>>> PR middle-end/102587
>>> * internal-fn.c (expand_DEFERRED_INIT): Guard register
>>> initialization path an avoid initializing VLA registers
>>> with it.
>>> 
>>> * gcc.target/aarch64/sve/pr102587-1.c: New testcase.
>>> * gcc.target/aarch64/sve/pr102587-2.c: Likewise.
>>> ---
>>> gcc/internal-fn.c | 3 ++-
>>> gcc/testsuite/gcc.target/aarch64/sve/pr102587-1.c | 4 
>>> gcc/testsuite/gcc.target/aarch64/sve/pr102587-2.c | 4 
>>> 3 files changed, 10 insertions(+), 1 deletion(-)
>>> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr102587-1.c
>>> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr102587-2.c
>>> 
>>> diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
>>> index 8312d08aab2..ef5dc90db56 100644
>>> --- a/gcc/internal-fn.c
>>> +++ b/gcc/internal-fn.c
>>> @@ -3035,7 +3035,8 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
>>>  /* Expand this memset call.  */
>>>  expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
>>>}
>>> -  else
>>> +  /* ???  Deal with poly-int sized registers.  */
>>> +  else if (tree_fits_uhwi_p (TYPE_SIZE_UNIT (var_type)))
>>>{
>>>  /* If this variable is in a register, use expand_assignment might
>>>  generate better code.  */
>>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr102587-1.c 
>>> b/gcc/testsuite/gcc.target/aarch64/sve/pr102587-1.c
>>> new file mode 100644
>>> index 000..2b9a68b0b59
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr102587-1.c
>>> @@ -0,0 +1,4 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-ftrivial-auto-var-init=zero" } */
>>> +
>>> +void foo() { __SVFloat64_t f64; }
>>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr102587-2.c 
>>> b/gcc/testsuite/gcc.target/aarch64/sve/pr102587-2.c
>>> new file mode 100644
>>> index 000..4cdb9056002
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr102587-2.c
>>> @@ -0,0 +1,4 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-ftrivial-auto-var-init=pattern" } */
>>> +
>>> +void foo() { __SVFloat64_t f64; }
>>> -- 
>>> 2.31.1
>> 
> 



Re: [PATCH] middle-end/102587 - avoid auto-init for VLA vectors

2021-10-04 Thread Richard Biener via Gcc-patches
On October 4, 2021 7:00:10 PM GMT+02:00, Qing Zhao  wrote:
>I have several questions on this fix:
>
>1. This fix avoided expanding “.DEFERRED_INIT” when !tree_fits_uhwi_p 
>(TYPE_SIZE_UNIT (var_type)).
>As a result, this call to .DEFERRED_INIT will NOT be expanded at all.

Yes. 

>Then not expanding .DEFERRED_INIT in RTL expanding phase will trigger more 
> issues in later RTL phases, this looks not correct to me. (Actually, with is 
> the patch, this testing case still failed in a later RTL stage). 
>
>So, If we really want to avoid auto-init for VLA vectors, we should not 
> add call to .DEFERRED_INIT in gimplification phase at all. 
>
>
>2. For the added .DEFERRED_INIT:
>
>  __SVFloat64_t f64;
>
>  f64 = .DEFERRED_INIT (POLY_INT_CST [16, 16], 2, 0);
>
>What does “POLY_INT_CST[16,16]” mean? Is this a constant size? If YES, what’s 
>the value of it? If Not, can we use “memset” to expand it?

When the target is a register memset doesn't work. I'm not sure the memset 
expansion path will work as-is either for aggregates with vla parts - but I'll 
leave that to Richard S. to sort out. 

Richard. 

>Thanks.
>
>Qing
>
>
>
>> On Oct 4, 2021, at 3:57 AM, Richard Biener via Gcc-patches 
>>  wrote:
>> 
>> This avoids ICEing for VLA vector auto-init by not initializing.
>> 
>> Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
>> 
>> 2021-10-04  Richard Biener  
>> 
>>  PR middle-end/102587
>>  * internal-fn.c (expand_DEFERRED_INIT): Guard register
>>  initialization path an avoid initializing VLA registers
>>  with it.
>> 
>>  * gcc.target/aarch64/sve/pr102587-1.c: New testcase.
>>  * gcc.target/aarch64/sve/pr102587-2.c: Likewise.
>> ---
>> gcc/internal-fn.c | 3 ++-
>> gcc/testsuite/gcc.target/aarch64/sve/pr102587-1.c | 4 
>> gcc/testsuite/gcc.target/aarch64/sve/pr102587-2.c | 4 
>> 3 files changed, 10 insertions(+), 1 deletion(-)
>> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr102587-1.c
>> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr102587-2.c
>> 
>> diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
>> index 8312d08aab2..ef5dc90db56 100644
>> --- a/gcc/internal-fn.c
>> +++ b/gcc/internal-fn.c
>> @@ -3035,7 +3035,8 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
>>   /* Expand this memset call.  */
>>   expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
>> }
>> -  else
>> +  /* ???  Deal with poly-int sized registers.  */
>> +  else if (tree_fits_uhwi_p (TYPE_SIZE_UNIT (var_type)))
>> {
>>   /* If this variable is in a register, use expand_assignment might
>>   generate better code.  */
>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr102587-1.c 
>> b/gcc/testsuite/gcc.target/aarch64/sve/pr102587-1.c
>> new file mode 100644
>> index 000..2b9a68b0b59
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr102587-1.c
>> @@ -0,0 +1,4 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-ftrivial-auto-var-init=zero" } */
>> +
>> +void foo() { __SVFloat64_t f64; }
>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr102587-2.c 
>> b/gcc/testsuite/gcc.target/aarch64/sve/pr102587-2.c
>> new file mode 100644
>> index 000..4cdb9056002
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr102587-2.c
>> @@ -0,0 +1,4 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-ftrivial-auto-var-init=pattern" } */
>> +
>> +void foo() { __SVFloat64_t f64; }
>> -- 
>> 2.31.1
>



Re: [PATCH] middle-end/102587 - avoid auto-init for VLA vectors

2021-10-04 Thread Qing Zhao via Gcc-patches
I have several questions on this fix:

1. This fix avoided expanding “.DEFERRED_INIT” when !tree_fits_uhwi_p 
(TYPE_SIZE_UNIT (var_type)).
As a result, this call to .DEFERRED_INIT will NOT be expanded at all.
Then not expanding .DEFERRED_INIT in RTL expanding phase will trigger more 
issues in later RTL phases, this looks not correct to me. (Actually, with is 
the patch, this testing case still failed in a later RTL stage). 

So, If we really want to avoid auto-init for VLA vectors, we should not add 
call to .DEFERRED_INIT in gimplification phase at all. 


2. For the added .DEFERRED_INIT:

  __SVFloat64_t f64;

  f64 = .DEFERRED_INIT (POLY_INT_CST [16, 16], 2, 0);

What does “POLY_INT_CST[16,16]” mean? Is this a constant size? If YES, what’s 
the value of it? If Not, can we use “memset” to expand it?

Thanks.

Qing



> On Oct 4, 2021, at 3:57 AM, Richard Biener via Gcc-patches 
>  wrote:
> 
> This avoids ICEing for VLA vector auto-init by not initializing.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
> 
> 2021-10-04  Richard Biener  
> 
>   PR middle-end/102587
>   * internal-fn.c (expand_DEFERRED_INIT): Guard register
>   initialization path an avoid initializing VLA registers
>   with it.
> 
>   * gcc.target/aarch64/sve/pr102587-1.c: New testcase.
>   * gcc.target/aarch64/sve/pr102587-2.c: Likewise.
> ---
> gcc/internal-fn.c | 3 ++-
> gcc/testsuite/gcc.target/aarch64/sve/pr102587-1.c | 4 
> gcc/testsuite/gcc.target/aarch64/sve/pr102587-2.c | 4 
> 3 files changed, 10 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr102587-1.c
> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr102587-2.c
> 
> diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
> index 8312d08aab2..ef5dc90db56 100644
> --- a/gcc/internal-fn.c
> +++ b/gcc/internal-fn.c
> @@ -3035,7 +3035,8 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
>   /* Expand this memset call.  */
>   expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
> }
> -  else
> +  /* ???  Deal with poly-int sized registers.  */
> +  else if (tree_fits_uhwi_p (TYPE_SIZE_UNIT (var_type)))
> {
>   /* If this variable is in a register, use expand_assignment might
>generate better code.  */
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr102587-1.c 
> b/gcc/testsuite/gcc.target/aarch64/sve/pr102587-1.c
> new file mode 100644
> index 000..2b9a68b0b59
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr102587-1.c
> @@ -0,0 +1,4 @@
> +/* { dg-do compile } */
> +/* { dg-options "-ftrivial-auto-var-init=zero" } */
> +
> +void foo() { __SVFloat64_t f64; }
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr102587-2.c 
> b/gcc/testsuite/gcc.target/aarch64/sve/pr102587-2.c
> new file mode 100644
> index 000..4cdb9056002
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr102587-2.c
> @@ -0,0 +1,4 @@
> +/* { dg-do compile } */
> +/* { dg-options "-ftrivial-auto-var-init=pattern" } */
> +
> +void foo() { __SVFloat64_t f64; }
> -- 
> 2.31.1



Re: [PATCH] libiberty: prevent buffer overflow when decoding user input

2021-10-04 Thread Luís Ferreira
On Thu, 2021-09-23 at 09:50 -0600, Jeff Law wrote:
> 
> 
> On 9/23/2021 4:16 AM, ibuclaw--- via Gcc-patches wrote:
> > > On 22/09/2021 03:10 Luís Ferreira  wrote:
> > > 
> > >   
> > > Currently a stack/heap overflow may happen if a crafted mangle is
> > > maliciously used to cause denial of service, such as intentional
> > > crashes
> > > by accessing a reserved memory space.
> > > 
> > Hi,
> > 
> > Thanks for this.  Is there a test that could trigger this code
> > path?
> I don't think Luis has commit privs, so I went ahead and committed
> this 
> patch.
> 
> Yea, a testcase would be great.
> 
> Jeff
> 

Does the test suite runned against address sanitization? if yes, I can
submit a patch to make this fail, otherwise it is hard to trigger a
consistent crash for this issue.

-- 
Sincerely,
Luís Ferreira @ lsferreira.net



signature.asc
Description: This is a digitally signed message part


Re: [committed] d: gdc driver ignores -static-libstdc++ when automatically linking libstdc++ library

2021-10-04 Thread Iain Sandoe
Hi Iain

> On 4 Oct 2021, at 17:00, Iain Buclaw via Gcc-patches 
>  wrote:
> 
> Hi,
> 
> This patch adds handling of `-static-libstc++' to the gdc driver, so
> that libstdc++ is appropriately linked if it is either needed or seen on
> the command-line.
> 
> This is required for bootstrapping the self hosted D front-end, so will
> also be backported to all supported releases.
> 
> Bootstrapped and regression tested, and committed to mainline.
> 
> Regards
> Iain.
> 
> ---
>   PR d/102574
> 
> gcc/d/ChangeLog:
> 
>   * d-spec.cc (lang_specific_driver): Link libstdc++ statically if
>   -static-libstdc++ was given on command-line.
> ---
> gcc/d/d-spec.cc | 43 ++-
> 1 file changed, 34 insertions(+), 9 deletions(-)
> 
> diff --git a/gcc/d/d-spec.cc b/gcc/d/d-spec.cc
> index 16ff1539e9f..5adc662c6f2 100644
> --- a/gcc/d/d-spec.cc
> +++ b/gcc/d/d-spec.cc
> @@ -83,6 +83,9 @@ lang_specific_driver (cl_decoded_option 
> **in_decoded_options,
>   /* "-lstdc++" if it appears on the command line.  */
>   const cl_decoded_option *saw_libcxx = 0;
> 
> +  /* True if we saw `-static-libstdc++'.  */
> +  bool saw_static_libcxx = false;
> +
>   /* Whether we need the C++ STD library.  */
>   bool need_stdcxx = false;
> 
> @@ -248,6 +251,11 @@ lang_specific_driver (cl_decoded_option 
> **in_decoded_options,
> shared_libgcc = false;
> break;
> 
> + case OPT_static_libstdc__:
> +   saw_static_libcxx = true;
> +   args[i] |= SKIPOPT;
> +   break;
> +
>   case OPT_static_libphobos:
> if (phobos_library != PHOBOS_NOLINK)
>   phobos_library = PHOBOS_STATIC;
> @@ -452,16 +460,33 @@ lang_specific_driver (cl_decoded_option 
> **in_decoded_options,
> #endif
> }
> 
> -  if (saw_libcxx)
> -new_decoded_options[j++] = *saw_libcxx;
> -  else if (need_stdcxx)
> +  if (saw_libcxx || need_stdcxx)
> {
> -  generate_option (OPT_l,
> -(saw_profile_flag
> - ? LIBSTDCXX_PROFILE
> - : LIBSTDCXX),
> -1, CL_DRIVER, _decoded_options[j++]);
> -  added_libraries++;
> +#ifdef HAVE_LD_STATIC_DYNAMIC
> +  if (saw_static_libcxx && !static_link)
> + {
> +   generate_option (OPT_Wl_, LD_STATIC_OPTION, 1, CL_DRIVER,
> +_decoded_options[j++]);
> + }

For targets that don’t support HAVE_LD_STATIC_DYNAMIC it would be useful
to push the option back out, so that they can use that to substitute a static 
version
of the library using %:replace-outfile(-lx libx+.a%s) [ see darwin.h 
for 
examples. ] .. I suppose we could figure out a follow-on patch and test that on
Darwin?

so
#else
 … code to push the -libstdc++ out

(yes, we do have this problem also with the g++ driver… I posted a patch eons
 ago .. but suspect it was never applied)

Iain

> +#endif
> +  if (saw_libcxx)
> + new_decoded_options[j++] = *saw_libcxx;
> +  else if (need_stdcxx)
> + {
> +   generate_option (OPT_l,
> +(saw_profile_flag
> + ? LIBSTDCXX_PROFILE
> + : LIBSTDCXX),
> +1, CL_DRIVER, _decoded_options[j++]);
> +   added_libraries++;
> + }
> +#ifdef HAVE_LD_STATIC_DYNAMIC
> +  if (saw_static_libcxx && !static_link)
> + {
> +   generate_option (OPT_Wl_, LD_DYNAMIC_OPTION, 1, CL_DRIVER,
> +_decoded_options[j++]);
> + }
> +#endif
> }
> 
>   if (shared_libgcc && !static_link)
> -- 
> 2.30.2
> 



[PATCH] c++: odr-use argument to a function NTTP [PR53164]

2021-10-04 Thread Patrick Palka via Gcc-patches
When passing a function template as the argument to a function NTTP
inside a template, we resolve it to the right specialization ahead of
time via resolve_address_of_overloaded_function, though the call to
mark_used within defers odr-using it until instantiation time (as usual).
But at instantiation time we end up never calling mark_used on the
specialization.

This patch fixes this by adding a call to mark_used in
convert_nontype_argument_function.

PR c++/53164

gcc/cp/ChangeLog:

* pt.c (convert_nontype_argument_function): Call mark_used.

gcc/testsuite/ChangeLog:

* g++.dg/template/non-dependent16.C: New test.
---
 gcc/cp/pt.c |  3 +++
 gcc/testsuite/g++.dg/template/non-dependent16.C | 16 
 2 files changed, 19 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/template/non-dependent16.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index f950f4a21b7..5e819c9598c 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -6668,6 +6668,9 @@ convert_nontype_argument_function (tree type, tree expr,
   return NULL_TREE;
 }
 
+  if (!mark_used (fn_no_ptr, complain) && !(complain & tf_error))
+return NULL_TREE;
+
   linkage = decl_linkage (fn_no_ptr);
   if (cxx_dialect >= cxx11 ? linkage == lk_none : linkage != lk_external)
 {
diff --git a/gcc/testsuite/g++.dg/template/non-dependent16.C 
b/gcc/testsuite/g++.dg/template/non-dependent16.C
new file mode 100644
index 000..b7dca8f6752
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/non-dependent16.C
@@ -0,0 +1,16 @@
+// PR c++/53164
+
+template
+void f(T) {
+  T::fail; // { dg-error "not a member" }
+}
+
+template
+struct A { };
+
+template
+void g() {
+  A a;
+}
+
+template void g<0>();
-- 
2.33.0.610.gcefe983a32



Re: [PATCH 1v2/3][vect] Add main vectorized loop unrolling

2021-10-04 Thread Richard Sandiford via Gcc-patches
Richard Biener via Gcc-patches  writes:
> On Thu, 30 Sep 2021, Andre Vieira (lists) wrote:
>
>> Hi,
>> 
>> 
>> >> That just forces trying the vector modes we've tried before. Though I 
>> >> might
>> >> need to revisit this now I think about it. I'm afraid it might be possible
>> >> for
>> >> this to generate an epilogue with a vf that is not lower than that of the
>> >> main
>> >> loop, but I'd need to think about this again.
>> >>
>> >> Either way I don't think this changes the vector modes used for the
>> >> epilogue.
>> >> But maybe I'm just missing your point here.
>> > Yes, I was refering to the above which suggests that when we vectorize
>> > the main loop with V4SF but unroll then we try vectorizing the
>> > epilogue with V4SF as well (but not unrolled).  I think that's
>> > premature (not sure if you try V8SF if the main loop was V4SF but
>> > unrolled 4 times).
>> 
>> My main motivation for this was because I had a SVE loop that vectorized with
>> both VNx8HI, then V8HI which beat VNx8HI on cost, then it decided to unroll
>> V8HI by two and skipped using VNx8HI as a predicated epilogue which would've
>> been the best choice.
>
> I see, yes - for fully predicated epilogues it makes sense to consider
> the same vector mode as for the main loop anyways (independent on
> whether we're unrolling or not).  One could argue that with an
> unrolled V4SImode main loop a predicated V8SImode epilogue would also
> be a good match (but then somehow costing favored the unrolled V4SI
> over the V8SI for the main loop...).
>
>> So that is why I decided to just 'reset' the vector_mode selection. In a
>> scenario where you only have the traditional vector modes it might make less
>> sense.
>> 
>> Just realized I still didn't add any check to make sure the epilogue has a
>> lower VF than the previous loop, though I'm still not sure that could happen.
>> I'll go look at where to add that if you agree with this.
>
> As said above, it only needs a lower VF in case the epilogue is not
> fully masked - otherwise the same VF would be OK.
>
>> >> I can move it there, it would indeed remove the need for the change to
>> >> vect_update_vf_for_slp, the change to
>> >> vect_determine_partial_vectors_and_peeling would still be required I 
>> >> think.
>> >> It
>> >> is meant to disable using partial vectors in an unrolled loop.
>> > Why would we disable the use of partial vectors in an unrolled loop?
>> The motivation behind that is that the overhead caused by generating
>> predicates for each iteration will likely be too much for it to be profitable
>> to unroll. On top of that, when dealing with low iteration count loops, if
>> executing one predicated iteration would be enough we now still need to
>> execute all other unrolled predicated iterations, whereas if we keep them
>> unrolled we skip the unrolled loops.
>
> OK, I guess we're not factoring in costs when deciding on predication
> but go for it if it's gernally enabled and possible.

Yeah.  That's mostly be design in SVE's case, but I can see that it might
need tweaking for other targets.

I don't think that's the really the “problem” (if it is a problem)
for the unroll decision though.  The “correct” way to unroll SVE loops,
which we're hoping to add at some point, is to predicate only the final
vector in each unrolled iteration.  So if the unroll factor is 4, say,
the unrolled iterations would be:

  - unrolled 4x
- unpredicated vector 0
- unpredicated vector 1
- unpredicated vector 2
- predicated vector 3
 
The epilogue loop would then be predicated and execute at most
3 times.  Alternatively, there could be two non-looping epilogues:

  - unrolled 2x
- unpredicated vector 0
- predicated vector 1
  
  - not unrolled
- predicated vector 0

This ensures that every executed vector operation does at least
some useful work.

Like Andre says, if we predicate as things stand, we'd have:

  - unrolled 4x
- predicated vector 0
- predicated vector 1
- predicated vector 2
- predicated vector 3

where the code could end up executing 4 vector operations per
scalar operation even if the final vector iteration only needs
to process 2 elements.  I don't think that's a costing issue,
it's just building in redundancy that doesn't need to be there.

Thanks,
Richard


Re: [RFC] More jump threading restrictions in the presence of loops.

2021-10-04 Thread Michael Matz via Gcc-patches
Hello,

On Mon, 4 Oct 2021, Jeff Law wrote:

> And just in case it got lost.  Here's the analysis on 960218-1 on visium:
> 
> We've got this going into ethread:
> 
> int f (int x)
> {
>   int D.1420;
>   int a;
> 
> ;;   basic block 2, loop depth 0, maybe hot
> ;;    prev block 0, next block 3, flags: (NEW, REACHABLE, VISITED)
> ;;    pred:   ENTRY (FALLTHRU,EXECUTABLE)
>   a_4 = ~x_3(D);
>   goto ; [INV]
> ;;    succ:   4 (FALLTHRU,EXECUTABLE)
> 
> ;;   basic block 3, loop depth 1, maybe hot
> ;;    prev block 2, next block 4, flags: (NEW, REACHABLE, VISITED)
> ;;    pred:   4 (TRUE_VALUE,EXECUTABLE)
>   gl = a_1;
> ;;    succ:   4 (FALLTHRU,DFS_BACK,EXECUTABLE)
> 
> ;;   basic block 4, loop depth 1, maybe hot
> ;;    prev block 3, next block 5, flags: (NEW, REACHABLE, VISITED)
> ;;    pred:   2 (FALLTHRU,EXECUTABLE)
> ;;    3 (FALLTHRU,DFS_BACK,EXECUTABLE)
>   # a_1 = PHI 
>   if (a_1 != 0)
>     goto ; [INV]
>   else
>     goto ; [INV]
> ;;    succ:   3 (TRUE_VALUE,EXECUTABLE)
> ;;    5 (FALSE_VALUE,EXECUTABLE)
> 
> ;;   basic block 5, loop depth 0, maybe hot
> ;;    prev block 4, next block 1, flags: (NEW, REACHABLE, VISITED)
> ;;    pred:   4 (FALSE_VALUE,EXECUTABLE)
>   return;
> ;;    succ:   EXIT
> 
> }

First notice that this doesn't have an empty latch block to start with 
(in fact, there is no separate latch block at all), so the loop optimizers 
haven't been initialized for simple latches at this point.  Still, I would 
say that even then we want to be careful with the latch edge, as putting 
code onto it will most probably create a problem downstream once we _do_ 
want to intialize the loop optimizers for simple latches.  So, that we are 
careful here is okay, we are just too careful in this specific case.

> There's a pretty obvious jump thread in there.  3->4->5.
> 
> We used to allow this, but it's currently being rejected and I'm not 
> sure it should.
> 
> In simplest terms we're threading from inside the loop, through the 
> latch to a point outside the loop.  At first glance, that seems safe.

Is at least the unrestricted jump threader (the one after loop optimizers) 
picking this up?

Independend of that, I agree, that this specific instance of threading 
through the latch block, even though followed by to-be-copied 
instructions, is okay.  We need to determine precisely when that is okay, 
though.  In this case there are several reasons I could see why this is 
so:
(a) while the thread is through the latch block, it's _not_ through the
latch edge (which is 4->3).  That would create the problem, so here
we're fine.
(b) even if it were through the latch edge, and would be followed by 
to-be-copied instructions (and hence fill the latch edge) the ultimate 
end of the path is outside the loop, so all the edges and blocks that 
would now carry instructions would also be outside the loop, and hence 
be no problem

Now, capture this precisely ;-)

I think something like so: we have a candidate path

  S -> L -> B1 (-> B2 ... Bn)

(Start, Latch, Blocks 1 to n following the latch).  (I think in our 
notation that means that the jump in L is redirected to Bn, and all code 
from B1..Bn be copied, right?  Do we even support multiple blocks after 
the to-be-redirected jump?)

Now if L is a latch block, but L->B1 is no latch/back edge : no 
restrictions apply.

Otherwise, L->B1 is a latch/back edge (that means B1 is a loop header): if 
all of B2..Bn-1 don't contain jumps back into the loop, and Bn is outside 
the loop, then the thread is okay as well.  (B2..Bn-1 can be inside the 
loop, as long as they don't contain jumps back into the loop, after 
copying by the threader, they don't create problems: their copies will be 
placed outside the loop and won't generate side entries back into the 
loop; the copied latch edge will not be a latch edge anymore, but a loop 
exit edge).

It's quite possible that the full generality above is not necessary.  
It's probably enough to just not deal with the (b) case above (and 
continue to reject it), and simply only accept the candidate path if none 
of the involved edges is a latch/back edge (despite one block being the 
latch block).  Especially so because I'm not convinced that I handled 
the idea of case (b) correctly above ;-)


Ciao,
Michael.


Re: [PING^2][PATCH] libgcc, emutls: Allow building weak definitions of the emutls functions.

2021-10-04 Thread Richard Sandiford via Gcc-patches
Iain Sandoe  writes:
> Hi,
>
> So let’s ignore the questions for now - OK for the non-Darwin parts of the
> patch ?

Looks OK to me.

Thanks,
Richard

>
>> On 24 Sep 2021, at 17:57, Iain Sandoe  wrote:
>> 
>
>> as noted below the non-Darwin parts of this are trivial (and a no-OP).
>> I’d like to apply this to start work towards solving Darwin’s libgcc issues,
>
>>> On 20 Sep 2021, at 09:25, Iain Sandoe  wrote:
>>> 
>
>>> The non-Darwin part of this patch is trivial but raises a couple of 
>>> questions
>>> 
>>> A/
>>> We define builtins to support emulated TLS.
>>> These are defined with void * pointers
>>> The implementation (in libgcc) uses the correct type (struct 
>>> __emutls_object *)
>>> in both a forward declaration of the functions and in thier eventual 
>>> implementation.
>>> 
>>> This leads to a (long-standing, nothing new) complaint at build-time about
>>> the mismatch in the builtin/implementation decls.
>>> 
>>> AFAICT, there’s no way to fix that unless we introduce struct 
>>> __emutls_object *
>>> as a built-in type?
>>> 
>>> B/ 
>>> It seems that a consequence of the mismatch in decls means that if I apply
>>> attributes to the decl (in the implementation file), they are ignored and I 
>>> have
>>> to apply them to the definition in order for this to work.
>>> 
>>> This (B) is what the patch below does.
>>> 
>>> tested on powerpc,i686,x86_64-darwin, x86_64-linux
>>> OK for master?
>>> thanks,
>>> Iain
>>> 
>>> If the current situation is that A or B indicates “there’s a bug”, please 
>>> could that
>>> be considered as distinct from the current patch (which doesn’t alter this 
>>> in any
>>> way) so that we can make progress on fixing Darwin libgcc issues.
>>> 
>>> = commit log
>>> 
>>> In order to better support use of the emulated TLS between objects with
>>> DSO dependencies and static-linked libgcc, allow a target to make weak
>>> definitions.
>>> 
>>> Signed-off-by: Iain Sandoe 
>>> 
>>> libgcc/ChangeLog:
>>> 
>>> * config.host: Add weak-defined emutls crt.
>>> * config/t-darwin: Build weak-defined emutls objects.
>>> * emutls.c (__emutls_get_address): Add optional attributes.
>>> (__emutls_register_common): Likewise.
>>> (EMUTLS_ATTR): New.
>>> ---
>>> libgcc/config.host |  2 +-
>>> libgcc/config/t-darwin | 13 +
>>> libgcc/emutls.c| 17 +++--
>>> 3 files changed, 29 insertions(+), 3 deletions(-)
>>> 
>>> diff --git a/libgcc/config.host b/libgcc/config.host
>>> index 6c34b13d611..a447ac7ae30 100644
>>> --- a/libgcc/config.host
>>> +++ b/libgcc/config.host
>>> @@ -215,7 +215,7 @@ case ${host} in
>>> *-*-darwin*)
>>>  asm_hidden_op=.private_extern
>>>  tmake_file="$tmake_file t-darwin ${cpu_type}/t-darwin t-libgcc-pic 
>>> t-slibgcc-darwin"
>>> -  extra_parts="crt3.o libd10-uwfef.a crttms.o crttme.o"
>>> +  extra_parts="crt3.o libd10-uwfef.a crttms.o crttme.o libemutls_w.a"
>>>  ;;
>>> *-*-dragonfly*)
>>>  tmake_file="$tmake_file t-crtstuff-pic t-libgcc-pic t-eh-dw2-dip"
>>> diff --git a/libgcc/config/t-darwin b/libgcc/config/t-darwin
>>> index 14ae6b35a4e..d6f688d66d5 100644
>>> --- a/libgcc/config/t-darwin
>>> +++ b/libgcc/config/t-darwin
>>> @@ -15,6 +15,19 @@ crttme.o: $(srcdir)/config/darwin-crt-tm.c
>>> LIB2ADDEH = $(srcdir)/unwind-dw2.c $(srcdir)/config/unwind-dw2-fde-darwin.c 
>>> \
>>>  $(srcdir)/unwind-sjlj.c $(srcdir)/unwind-c.c
>>> 
>>> +# Make emutls weak so that we can deal with -static-libgcc, override the
>>> +# hidden visibility when this is present in libgcc_eh.
>>> +emutls.o: HOST_LIBGCC2_CFLAGS += \
>>> +  -DEMUTLS_ATTR='__attribute__((__weak__,__visibility__("default")))'
>>> +emutls_s.o: HOST_LIBGCC2_CFLAGS += \
>>> +  -DEMUTLS_ATTR='__attribute__((__weak__,__visibility__("default")))'
>>> +
>>> +# Make the emutls crt as a convenience lib so that it can be linked
>>> +# optionally, use the shared version so that we can link with DSO.
>>> +libemutls_w.a: emutls_s.o
>>> +   $(AR_CREATE_FOR_TARGET) $@ $<
>>> +   $(RANLIB_FOR_TARGET) $@
>>> +
>>> # Patch to __Unwind_Find_Enclosing_Function for Darwin10.
>>> d10-uwfef.o: $(srcdir)/config/darwin10-unwind-find-enc-func.c
>>> $(crt_compile) -mmacosx-version-min=10.6 -c $<
>>> diff --git a/libgcc/emutls.c b/libgcc/emutls.c
>>> index ed2658170f5..d553a74728f 100644
>>> --- a/libgcc/emutls.c
>>> +++ b/libgcc/emutls.c
>>> @@ -50,7 +50,16 @@ struct __emutls_array
>>>  void **data[];
>>> };
>>> 
>>> +/* EMUTLS_ATTR is provided to allow targets to build the emulated tls
>>> +   routines as weak definitions, for example.
>>> +   If there is no definition, fall back to the default.  */
>>> +#ifndef EMUTLS_ATTR
>>> +#  define EMUTLS_ATTR
>>> +#endif
>>> +
>>> +EMUTLS_ATTR
>>> void *__emutls_get_address (struct __emutls_object *);
>>> +EMUTLS_ATTR
>>> void __emutls_register_common (struct __emutls_object *, word, word, void 
>>> *);
>>> 
>>> #ifdef __GTHREADS
>>> @@ -123,7 +132,11 @@ emutls_alloc (struct __emutls_object *obj)
>>>  return ret;
>>> }
>>> 
>>> -void *
>>> 

[committed] d: gdc driver ignores -static-libstdc++ when automatically linking libstdc++ library

2021-10-04 Thread Iain Buclaw via Gcc-patches
Hi,

This patch adds handling of `-static-libstc++' to the gdc driver, so
that libstdc++ is appropriately linked if it is either needed or seen on
the command-line.

This is required for bootstrapping the self hosted D front-end, so will
also be backported to all supported releases.

Bootstrapped and regression tested, and committed to mainline.

Regards
Iain.

---
PR d/102574

gcc/d/ChangeLog:

* d-spec.cc (lang_specific_driver): Link libstdc++ statically if
-static-libstdc++ was given on command-line.
---
 gcc/d/d-spec.cc | 43 ++-
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/gcc/d/d-spec.cc b/gcc/d/d-spec.cc
index 16ff1539e9f..5adc662c6f2 100644
--- a/gcc/d/d-spec.cc
+++ b/gcc/d/d-spec.cc
@@ -83,6 +83,9 @@ lang_specific_driver (cl_decoded_option **in_decoded_options,
   /* "-lstdc++" if it appears on the command line.  */
   const cl_decoded_option *saw_libcxx = 0;
 
+  /* True if we saw `-static-libstdc++'.  */
+  bool saw_static_libcxx = false;
+
   /* Whether we need the C++ STD library.  */
   bool need_stdcxx = false;
 
@@ -248,6 +251,11 @@ lang_specific_driver (cl_decoded_option 
**in_decoded_options,
  shared_libgcc = false;
  break;
 
+   case OPT_static_libstdc__:
+ saw_static_libcxx = true;
+ args[i] |= SKIPOPT;
+ break;
+
case OPT_static_libphobos:
  if (phobos_library != PHOBOS_NOLINK)
phobos_library = PHOBOS_STATIC;
@@ -452,16 +460,33 @@ lang_specific_driver (cl_decoded_option 
**in_decoded_options,
 #endif
 }
 
-  if (saw_libcxx)
-new_decoded_options[j++] = *saw_libcxx;
-  else if (need_stdcxx)
+  if (saw_libcxx || need_stdcxx)
 {
-  generate_option (OPT_l,
-  (saw_profile_flag
-   ? LIBSTDCXX_PROFILE
-   : LIBSTDCXX),
-  1, CL_DRIVER, _decoded_options[j++]);
-  added_libraries++;
+#ifdef HAVE_LD_STATIC_DYNAMIC
+  if (saw_static_libcxx && !static_link)
+   {
+ generate_option (OPT_Wl_, LD_STATIC_OPTION, 1, CL_DRIVER,
+  _decoded_options[j++]);
+   }
+#endif
+  if (saw_libcxx)
+   new_decoded_options[j++] = *saw_libcxx;
+  else if (need_stdcxx)
+   {
+ generate_option (OPT_l,
+  (saw_profile_flag
+   ? LIBSTDCXX_PROFILE
+   : LIBSTDCXX),
+  1, CL_DRIVER, _decoded_options[j++]);
+ added_libraries++;
+   }
+#ifdef HAVE_LD_STATIC_DYNAMIC
+  if (saw_static_libcxx && !static_link)
+   {
+ generate_option (OPT_Wl_, LD_DYNAMIC_OPTION, 1, CL_DRIVER,
+  _decoded_options[j++]);
+   }
+#endif
 }
 
   if (shared_libgcc && !static_link)
-- 
2.30.2



[committed] d: Save target node before parsing optimize options in case it changes.

2021-10-04 Thread Iain Buclaw via Gcc-patches
Hi,

This patch saves target node information before calling
parse_optimize_options in the D "optimize" attribute handler, the same
as is done for C/C++ optimize.

Fixes an ICE seen on PowerPC targets with the attr_optimize*.d tests in
the gdc.dg testsuite.

Bootstrapped and regression tested, and committed to mainline.

Regards
Iain.

---
gcc/d/ChangeLog:

* d-attribs.cc (d_handle_optimize_attribute): Save target node before
calling parse_optimize_options in case it changes.
---
 gcc/d/d-attribs.cc | 8 
 1 file changed, 8 insertions(+)

diff --git a/gcc/d/d-attribs.cc b/gcc/d/d-attribs.cc
index 070866020a2..d81b7d122f7 100644
--- a/gcc/d/d-attribs.cc
+++ b/gcc/d/d-attribs.cc
@@ -896,6 +896,8 @@ d_handle_optimize_attribute (tree *node, tree name, tree 
args, int,
 
   /* Save current options.  */
   cl_optimization_save (_opts, _options, _options_set);
+  tree prev_target_node = build_target_option_node (_options,
+   _options_set);
 
   /* If we previously had some optimization options, use them as the
 default.  */
@@ -914,10 +916,16 @@ d_handle_optimize_attribute (tree *node, tree name, tree 
args, int,
   parse_optimize_options (args);
   DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node)
= build_optimization_node (_options, _options_set);
+  tree target_node = build_target_option_node (_options,
+  _options_set);
+  if (prev_target_node != target_node)
+   DECL_FUNCTION_SPECIFIC_TARGET (*node) = target_node;
 
   /* Restore current options.  */
   cl_optimization_restore (_options, _options_set,
   _opts);
+  cl_target_option_restore (_options, _options_set,
+   TREE_TARGET_OPTION (prev_target_node));
   if (saved_global_options != NULL)
{
  cl_optimization_compare (saved_global_options, _options);
-- 
2.30.2



Re: [PATCH 8/8] OpenMP 5.0: [WIP, RFC] Clause ordering for OpenMP 5.0 (topological sorting by base pointer)

2021-10-04 Thread Julian Brown
On Wed, 11 Aug 2021 09:59:35 -0700
Julian Brown  wrote:

> This patch reimplements the omp_target_reorder_clauses function in
> anticipation of supporting "deeper" struct mappings (that is, with
> several structure dereference operators, or similar).

FWIW, this patch has been superseded by the following:

  https://gcc.gnu.org/pipermail/gcc-patches/2021-October/580724.html

from the series posted starting from here:

  https://gcc.gnu.org/pipermail/gcc-patches/2021-October/580721.html

HTH,

Julian




Re: [RFC] More jump threading restrictions in the presence of loops.

2021-10-04 Thread Jeff Law via Gcc-patches




On 10/4/2021 7:36 AM, Aldy Hernandez wrote:
Note that none of the other tests have been adjusted so it'll likely 
result in multiple threading regressions.
Yea ;-)  So maybe we first focus on how those affect visium since that's 
what started us down this path.


The original test of regressions for visium were:
visium-sim: gcc.c-torture/execute/960218-1.c   -Os  (test for excess errors)
visium-sim: gcc.c-torture/execute/961125-1.c   -O3 -fomit-frame-pointer 
-funroll-loops -fpeel-loops -ftracer -finline-functions  (test for 
excess errors)
visium-sim: gcc.c-torture/execute/961125-1.c   -O3 -g  (test for excess 
errors)
visium-sim: gcc.c-torture/execute/pending-4.c   -O1  (test for excess 
errors)

visium-sim: gcc.c-torture/execute/pr58209.c   -O2  (test for excess errors)
visium-sim: gcc.c-torture/execute/pr58209.c   -O2 -flto 
-fno-use-linker-plugin -flto-partition=none  (test for excess errors)
visium-sim: gcc.c-torture/execute/pr58209.c   -O2 -flto 
-fuse-linker-plugin -fno-fat-lto-objects  (test for excess errors)
visium-sim: gcc.c-torture/execute/pr58209.c   -O3 -g  (test for excess 
errors)

visium-sim: gcc.c-torture/execute/pr68911.c   -O1  (test for excess errors)

I'm pretty confident these are all cases where we previously threaded 
one or more jumps and as a result we were able to remove all calls to 
abort ().   Your change doesn't affect any of those :(


So probably the first thing to do is get a patch that fixes one or more 
of those failures *or* make a determination that one or more of those 
tests are things we do not want to optimize.   The one we had previously 
looked at (960218-1)  had a thread path from inside the loop to a point 
outside the loop.  My sense is we probably want to allow that.


And just in case it got lost.  Here's the analysis on 960218-1 on visium:

We've got this going into ethread:

int f (int x)
{
  int D.1420;
  int a;

;;   basic block 2, loop depth 0, maybe hot
;;    prev block 0, next block 3, flags: (NEW, REACHABLE, VISITED)
;;    pred:   ENTRY (FALLTHRU,EXECUTABLE)
  a_4 = ~x_3(D);
  goto ; [INV]
;;    succ:   4 (FALLTHRU,EXECUTABLE)

;;   basic block 3, loop depth 1, maybe hot
;;    prev block 2, next block 4, flags: (NEW, REACHABLE, VISITED)
;;    pred:   4 (TRUE_VALUE,EXECUTABLE)
  gl = a_1;
;;    succ:   4 (FALLTHRU,DFS_BACK,EXECUTABLE)

;;   basic block 4, loop depth 1, maybe hot
;;    prev block 3, next block 5, flags: (NEW, REACHABLE, VISITED)
;;    pred:   2 (FALLTHRU,EXECUTABLE)
;;    3 (FALLTHRU,DFS_BACK,EXECUTABLE)
  # a_1 = PHI 
  if (a_1 != 0)
    goto ; [INV]
  else
    goto ; [INV]
;;    succ:   3 (TRUE_VALUE,EXECUTABLE)
;;    5 (FALSE_VALUE,EXECUTABLE)

;;   basic block 5, loop depth 0, maybe hot
;;    prev block 4, next block 1, flags: (NEW, REACHABLE, VISITED)
;;    pred:   4 (FALSE_VALUE,EXECUTABLE)
  return;
;;    succ:   EXIT

}


There's a pretty obvious jump thread in there.  3->4->5.  We used to 
allow this, but it's currently being rejected and I'm not sure it should.


In simplest terms we're threading from inside the loop, through the 
latch to a point outside the loop.  At first glance, that seems safe.


960218-1.c, compiled with -Os

int gl;

g (x)
{
  gl = x;
  return 0;
}

f (x)
{
  int a = ~x;
  while (a)
    a = g (a);
}

main ()
{
  f (3);
  if (gl != -4)
    abort ();
  exit (0);
}


Ideally in the final assembly there would be no calls to abort since it 
can be determined at compile-time that the call can never happen.


jeff



[PATCH][i386] target: support spaces in target attribute.

2021-10-04 Thread Martin Liška

Hello.

The patch is about supporting target attribute values like "no-avx, sse2   ".
I'm planning doing the same change for aarch64 and other archs as well once
this one is accepted.

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

Ready to be installed?
Thanks,
Martin

PR target/102374

gcc/ChangeLog:

* config/i386/i386-options.c (ix86_valid_target_attribute_inner_p):
* system.h (strip_spaces):

gcc/testsuite/ChangeLog:

* gcc.target/i386/pr102374.c: New test.
---
 gcc/config/i386/i386-options.c   |  2 ++
 gcc/system.h | 21 +
 gcc/testsuite/gcc.target/i386/pr102374.c |  3 +++
 3 files changed, 26 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr102374.c

diff --git a/gcc/config/i386/i386-options.c b/gcc/config/i386/i386-options.c
index e7a3bd4aaea..c88bc77b04d 100644
--- a/gcc/config/i386/i386-options.c
+++ b/gcc/config/i386/i386-options.c
@@ -1146,6 +1146,8 @@ ix86_valid_target_attribute_inner_p (tree fndecl, tree 
args, char *p_strings[],
  next_optstr = NULL;
}
 
+  p = strip_spaces (p, );

+
   /* Recognize no-xxx.  */
   if (len > 3 && p[0] == 'n' && p[1] == 'o' && p[2] == '-')
{
diff --git a/gcc/system.h b/gcc/system.h
index adde3e264b6..35b15203a96 100644
--- a/gcc/system.h
+++ b/gcc/system.h
@@ -1305,4 +1305,25 @@ startswith (const char *str, const char *prefix)
   return strncmp (str, prefix, strlen (prefix)) == 0;
 }
 
+/* Strip spaces from STRING with LEN length.

+   A stripped string is returned and LEN is updated accordingly.  */
+
+static inline char *
+strip_spaces (char *string, size_t *len)
+{
+  while (string[0] == ' ')
+{
+  --(*len);
+  ++string;
+}
+
+  while (string[*len - 1] == ' ')
+{
+  string[*len - 1] = '\0';
+  --(*len);
+}
+
+  return string;
+}
+
 #endif /* ! GCC_SYSTEM_H */
diff --git a/gcc/testsuite/gcc.target/i386/pr102374.c 
b/gcc/testsuite/gcc.target/i386/pr102374.c
new file mode 100644
index 000..db84aeed9a5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr102374.c
@@ -0,0 +1,3 @@
+/* PR target/102374 */
+
+void calculate_sse(void) __attribute__ ((__target__ ("no-avx, sse2   ")));
--
2.33.0



Re: [PATCH] libiberty: d-demangle: use switch instead of if-else

2021-10-04 Thread Luís Ferreira
On Sun, 2021-10-03 at 15:55 -0600, Jeff Law wrote:
> 
> 
> On 9/29/2021 7:08 PM, Luís Ferreira wrote:
> > This patch allows the compiler to efficiently generate jump tables
> > instead of
> > using if-else-if.
> > 
> > Signed-off-by: Luís Ferreira 
> I'm not sure this is terribly useful.  Compilers have the ability to 
> analyze the underlying code and make sensible decisions for how to 
> implement either form.   So the right metric here is does this make the
> code cleaner/easier to understand.  With just 3 clauses it's hard (for 
> me) to make the case that it is considerably cleaner.
> 
> Jeff
> 
> 

Well, my major point on the patch was performance, eventhough on this
number of clauses it doesn't matter for GCC. I can totally agree with
you.

Although when we talk about a higher number of cases the optimizer
reach a threashold to optimize with jump tables and if-else-if starts
to loose there. Furthermore, if-else-if are hard to optimize, since the
compiler need to check if the condition order is important and with a
high number of cases the compiler may just give up on that check. I'm
not particularly aware of how GCC theoretically optimize it, so take
that with a grain of salt, but, in practice, it sure have a difference.
https://godbolt.org/z/rT9drW117

Even though performance may not be terribly bad (some compilers may not
be really that smart, specially in higher number of cases, as shown
above) I can still consider this change for future additions, even
though debatable, since mangling characters may exapand this logic and
increase the number of cases making if-else-if code a bit longer to
read.

Overall, although, this is by far a minor thing. My intention with this
was to reflect some changes I found relevant while reading this file.

Side note:

This was a previously sent patch. As requested by you, if this is
somehow being considered to be merged, here is the changelog:

ChangeLog:

libiberty/
* d-demangle.c (dlang_type): Change if-else-if to switch case.

-- 
Sincerely,
Luís Ferreira @ lsferreira.net



signature.asc
Description: This is a digitally signed message part


[PATCH] tree-optimization/102570 - teach VN about internal functions

2021-10-04 Thread Richard Biener via Gcc-patches
We're now using internal functions for a lot of stuff but there's
still missing VN support out of laziness.  The following instantiates
support and adds testcases for FRE and PRE (hoisting).

Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

2021-10-04  Richard Biener  

PR tree-optimization/102570
* tree-ssa-sccvn.h (vn_reference_op_struct): Document
we are using clique for the internal function code.
* tree-ssa-sccvn.c (vn_reference_op_eq): Compare the
internal function code.
(print_vn_reference_ops): Print the internal function code.
(vn_reference_op_compute_hash): Hash it.
(copy_reference_ops_from_call): Record it.
(visit_stmt): Remove the restriction around internal function
calls.
(fully_constant_vn_reference_p): Use fold_const_call and handle
internal functions.
(vn_reference_eq): Compare call return types.
* tree-ssa-pre.c (create_expression_by_pieces): Handle
generating calls to internal functions.
(compute_avail): Remove the restriction around internal function
calls.

* gcc.dg/tree-ssa/ssa-fre-96.c: New testcase.
* gcc.dg/tree-ssa/ssa-pre-33.c: Likewise.
---
 gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-96.c | 14 
 gcc/testsuite/gcc.dg/tree-ssa/ssa-pre-33.c | 15 
 gcc/tree-ssa-pre.c | 27 ---
 gcc/tree-ssa-sccvn.c   | 91 +-
 gcc/tree-ssa-sccvn.h   |  3 +-
 5 files changed, 103 insertions(+), 47 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-96.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ssa-pre-33.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-96.c 
b/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-96.c
new file mode 100644
index 000..fd1d5713b5f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-96.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-fre1" } */
+
+_Bool f1(unsigned x, unsigned y, unsigned *res)
+{
+_Bool t = __builtin_add_overflow(x, y, res);
+unsigned res1;
+_Bool t1 = __builtin_add_overflow(x, y, );
+*res -= res1;
+return t==t1;
+}
+
+/* { dg-final { scan-tree-dump-times "ADD_OVERFLOW" 1 "fre1" } } */
+/* { dg-final { scan-tree-dump "return 1;" "fre1" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-pre-33.c 
b/gcc/testsuite/gcc.dg/tree-ssa/ssa-pre-33.c
new file mode 100644
index 000..3b3bd629bc2
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-pre-33.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-pre" } */
+
+_Bool f1(unsigned x, unsigned y, unsigned *res, int flag, _Bool *t)
+{
+  if (flag)
+*t = __builtin_add_overflow(x, y, res);
+  unsigned res1;
+  _Bool t1 = __builtin_add_overflow(x, y, );
+  *res -= res1;
+  return *t==t1;
+}
+
+/* We should hoist the .ADD_OVERFLOW to before the check.  */
+/* { dg-final { scan-tree-dump-times "ADD_OVERFLOW" 1 "pre" } } */
diff --git a/gcc/tree-ssa-pre.c b/gcc/tree-ssa-pre.c
index 08755847f66..1cc1aae694f 100644
--- a/gcc/tree-ssa-pre.c
+++ b/gcc/tree-ssa-pre.c
@@ -2855,9 +2855,13 @@ create_expression_by_pieces (basic_block block, pre_expr 
expr,
  unsigned int operand = 1;
  vn_reference_op_t currop = >operands[0];
  tree sc = NULL_TREE;
- tree fn  = find_or_generate_expression (block, currop->op0, stmts);
- if (!fn)
-   return NULL_TREE;
+ tree fn = NULL_TREE;
+ if (currop->op0)
+   {
+ fn = find_or_generate_expression (block, currop->op0, stmts);
+ if (!fn)
+   return NULL_TREE;
+   }
  if (currop->op1)
{
  sc = find_or_generate_expression (block, currop->op1, stmts);
@@ -2873,12 +2877,19 @@ create_expression_by_pieces (basic_block block, 
pre_expr expr,
return NULL_TREE;
  args.quick_push (arg);
}
- gcall *call = gimple_build_call_vec (fn, args);
+ gcall *call;
+ if (currop->op0)
+   {
+ call = gimple_build_call_vec (fn, args);
+ gimple_call_set_fntype (call, currop->type);
+   }
+ else
+   call = gimple_build_call_internal_vec ((internal_fn)currop->clique,
+  args);
  gimple_set_location (call, expr->loc);
- gimple_call_set_fntype (call, currop->type);
  if (sc)
gimple_call_set_chain (call, sc);
- tree forcedname = make_ssa_name (TREE_TYPE (currop->type));
+ tree forcedname = make_ssa_name (ref->type);
  gimple_call_set_lhs (call, forcedname);
  /* There's no CCP pass after PRE which would re-compute alignment
 information so make sure we re-materialize this here.  */
@@ -4004,10 +4015,6 @@ compute_avail (function *fun)
vn_reference_s ref1;

Re: [committed] libstdc++: Allow visiting inherited variants [PR 90943]

2021-10-04 Thread Jonathan Wakely via Gcc-patches
On Sun, 3 Oct 2021 at 10:07, Jonathan Wakely wrote:
>
>
>
> On Sat, 2 Oct 2021, 13:50 Daniel Krügler via Libstdc++, 
>  wrote:
>>
>> Am Fr., 1. Okt. 2021 um 21:57 Uhr schrieb Jonathan Wakely via
>> Libstdc++ :
>> >
>> > Implement the changes from P2162R2 (as a DR for C++17).
>> >
>> > Signed-off-by: Jonathan Wakely 
>> >
>> > libstdc++-v3/ChangeLog:
>> >
>> > PR libstdc++/90943
>> > * include/std/variant (__cpp_lib_variant): Update value.
>> > (__detail::__variant::__as): New helpers implementing the
>> > as-variant exposition-only function templates.
>> > (visit, visit): Use __as to upcast the variant parameters.
>> > * include/std/version (__cpp_lib_variant): Update value.
>> > * testsuite/20_util/variant/visit_inherited.cc: New test.
>> >
>> > Tested powerpc64le-linux. Committed to trunk.
>> >
>>
>> I'm wondering why the first __as overload is not noexcept as well (or
>> asking it the other way around: Why different exception-specifications
>> are used for the different overloads):
>>
>> +  // The __as function templates implement the exposition-only "as-variant"
>> +
>> +  template
>> +constexpr std::variant<_Types...>&
>> +__as(std::variant<_Types...>& __v)
>> +{ return __v; }
>
>
> Probably just an oversight, I'll check again and fix it. Thanks!

Fixed by the attached patch, thanks again.

Tested powerpc64le-linux, pushed to trunk.
commit 728e639d82099035fdfe69b716a54717ae7050e0
Author: Jonathan Wakely 
Date:   Mon Oct 4 10:21:58 2021

libstdc++: Add missing noexcept to std::variant helper

libstdc++-v3/ChangeLog:

* include/std/variant (__detail::__variant::__as): Add missing
noexcept to first overload.

diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index d50c6b7de1d..6377b6731ea 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -206,7 +206,7 @@ namespace __variant
 
   template
 constexpr std::variant<_Types...>&
-__as(std::variant<_Types...>& __v)
+__as(std::variant<_Types...>& __v) noexcept
 { return __v; }
 
   template


[committed] libstdc++: Implement P1518R2 for container deduction guides

2021-10-04 Thread Jonathan Wakely via Gcc-patches
This implements the C++23 P1518R2 proposal "Stop overconstraining
allocators in container deduction guides" as a fix for C++17 and C++20
too.

The changes allow class template argument deduction to ignore the type
of a constructor argument that initializes an allocator_type parameter
if the type should be deducible only from the other arguments. So for
the constructor vector(const vector&, const allocator_type&) only the
first argument is used for deduction, allowing the second argument to be
anything that is implicitly convertible to argument_type. Previously
deduction would fail or an ill-formed type would be deduced if the
second argument wasn't of type allocator_type.

The unordered containers do not need changes, because their
allocator-extended constructors use the allocator_type alias, which
comes from the dependent base class so is already a non-deduced context.

libstdc++-v3/ChangeLog:

* include/bits/forward_list.h (forward_list): Use non-deduced
context for allocator parameter of allocator-extended copy and
move constructors.
* include/bits/stl_bvector.h (vector): Likewise.
* include/bits/stl_deque.h (deque): Likewise.
* include/bits/stl_list.h (list): Likewise.
* include/bits/stl_map.h (map): Likewise.
* include/bits/stl_multimap.h (multimap): Likewise.
* include/bits/stl_multiset.h (multiset): Likewise.
* include/bits/stl_set.h (set): Likewise.
* include/bits/stl_vector.h (vector): Likewise.
* include/bits/stl_queue.h (queue, priority_queue): Do not
constrain Allocator template parameter of deduction guides that
have a Container parameter.
* include/bits/stl_stack.h (stack): Likewise.
* include/debug/deque (__gnu_debug::deque): Use non-deduced
context for allocator parameter of allocator-extended copy and
move constructors.
* include/debug/list (__gnu_debug::list): Likewise.
* include/debug/map.h (__gnu_debug::map): Likewise.
* include/debug/multimap.h (__gnu_debug::multimap): Likewise.
* include/debug/multiset.h (__gnu_debug::multiset): Likewise.
* include/debug/set.h (__gnu_debug::set): Likewise.
* include/debug/vector (__gnu_debug::vector): Likewise.
* testsuite/23_containers/deque/cons/deduction.cc: Test class
template argument deduction with non-deduced allocator
arguments.
* testsuite/23_containers/forward_list/cons/deduction.cc:
Likewise.
* testsuite/23_containers/list/cons/deduction.cc: Likewise.
* testsuite/23_containers/map/cons/deduction.cc: Likewise.
* testsuite/23_containers/multimap/cons/deduction.cc: Likewise.
* testsuite/23_containers/multiset/cons/deduction.cc: Likewise.
* testsuite/23_containers/priority_queue/deduction.cc: Likewise.
* testsuite/23_containers/queue/deduction.cc: Likewise.
* testsuite/23_containers/set/cons/deduction.cc: Likewise.
* testsuite/23_containers/stack/deduction.cc: Likewise.
* testsuite/23_containers/unordered_map/cons/deduction.cc:
Likewise.
* testsuite/23_containers/unordered_multimap/cons/deduction.cc:
Likewise.
* testsuite/23_containers/unordered_multiset/cons/deduction.cc:
Likewise.
* testsuite/23_containers/unordered_set/cons/deduction.cc:
Likewise.
* testsuite/23_containers/vector/cons/deduction.cc: Likewise.

Tested powerpc64le-linux. Committed to trunk.

commit 22d34a2a50651d01669b6fbcdb9677c18d2197c5
Author: Jonathan Wakely 
Date:   Mon Oct 4 14:04:20 2021

libstdc++: Implement P1518R2 for container deduction guides

This implements the C++23 P1518R2 proposal "Stop overconstraining
allocators in container deduction guides" as a fix for C++17 and C++20
too.

The changes allow class template argument deduction to ignore the type
of a constructor argument that initializes an allocator_type parameter
if the type should be deducible only from the other arguments. So for
the constructor vector(const vector&, const allocator_type&) only the
first argument is used for deduction, allowing the second argument to be
anything that is implicitly convertible to argument_type. Previously
deduction would fail or an ill-formed type would be deduced if the
second argument wasn't of type allocator_type.

The unordered containers do not need changes, because their
allocator-extended constructors use the allocator_type alias, which
comes from the dependent base class so is already a non-deduced context.

libstdc++-v3/ChangeLog:

* include/bits/forward_list.h (forward_list): Use non-deduced
context for allocator parameter of allocator-extended copy and
move constructors.
* include/bits/stl_bvector.h (vector): Likewise.
* include/bits/stl_deque.h (deque): 

[committed] libstdc++: Disable std::string{, _view} construction from nullptr (P2166R1)

2021-10-04 Thread Jonathan Wakely via Gcc-patches
Implement this C++23 feature. Because construction from a null pointer
is undefined, we can implement it for C++11 and up, turning undefined
behaviour into a compilation error.

libstdc++-v3/ChangeLog:

* include/bits/basic_string.h (basic_string(nullptr_t)): Define
as deleted.
(operator=(nullptr_t)): Likewise.
* include/bits/cow_string.h (basic_string(nullptr_t)): Likewise.
(operator=(nullptr_t)): Likewise.
* include/std/string_view (basic_string_view(nullptr_t)):
Likewise.
* testsuite/21_strings/basic_string/cons/char/nullptr.cc: New test.
* testsuite/21_strings/basic_string_view/cons/char/nonnull.cc:
Change dg-warning to dg-error.
* testsuite/21_strings/basic_string_view/cons/wchar_t/nonnull.cc:
Likewise.

Tested powerpc64le-linux. Committed to trunk.

commit cf876562c592193732f869e9f96034a42d0fad89
Author: Jonathan Wakely 
Date:   Fri Oct 1 21:27:24 2021

libstdc++: Disable std::string{,_view} construction from nullptr (P2166R1)

Implement this C++23 feature. Because construction from a null pointer
is undefined, we can implement it for C++11 and up, turning undefined
behaviour into a compilation error.

libstdc++-v3/ChangeLog:

* include/bits/basic_string.h (basic_string(nullptr_t)): Define
as deleted.
(operator=(nullptr_t)): Likewise.
* include/bits/cow_string.h (basic_string(nullptr_t)): Likewise.
(operator=(nullptr_t)): Likewise.
* include/std/string_view (basic_string_view(nullptr_t)):
Likewise.
* testsuite/21_strings/basic_string/cons/char/nullptr.cc: New test.
* testsuite/21_strings/basic_string_view/cons/char/nonnull.cc:
Change dg-warning to dg-error.
* testsuite/21_strings/basic_string_view/cons/wchar_t/nonnull.cc:
Likewise.

diff --git a/libstdc++-v3/include/bits/basic_string.h 
b/libstdc++-v3/include/bits/basic_string.h
index 24c454d863a..68c388408f0 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -623,6 +623,8 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
  _M_construct(__str.begin(), __str.end());
   }
 
+  basic_string(nullptr_t) = delete;
+  basic_string& operator=(nullptr_t) = delete;
 #endif // C++11
 
   /**
diff --git a/libstdc++-v3/include/bits/cow_string.h 
b/libstdc++-v3/include/bits/cow_string.h
index ba4a8cc2e98..f0540128364 100644
--- a/libstdc++-v3/include/bits/cow_string.h
+++ b/libstdc++-v3/include/bits/cow_string.h
@@ -667,6 +667,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
else
  _M_dataplus._M_p = _S_construct(__str.begin(), __str.end(), __a);
   }
+
+  basic_string(nullptr_t) = delete;
+  basic_string& operator=(nullptr_t) = delete;
 #endif // C++11
 
   /**
diff --git a/libstdc++-v3/include/std/string_view 
b/libstdc++-v3/include/std/string_view
index d8cbee9bee0..996b03f9346 100644
--- a/libstdc++-v3/include/std/string_view
+++ b/libstdc++-v3/include/std/string_view
@@ -168,6 +168,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif // C++23
 #endif // C++20
 
+  basic_string_view(nullptr_t) = delete;
+
   constexpr basic_string_view&
   operator=(const basic_string_view&) noexcept = default;
 
diff --git 
a/libstdc++-v3/testsuite/21_strings/basic_string/cons/char/nullptr.cc 
b/libstdc++-v3/testsuite/21_strings/basic_string/cons/char/nullptr.cc
new file mode 100644
index 000..fdb24aeeb89
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/cons/char/nullptr.cc
@@ -0,0 +1,4 @@
+// { dg-do compile { target c++11 } }
+#include 
+
+std::string s = nullptr; // { dg-error "deleted" "P2166R1" }
diff --git 
a/libstdc++-v3/testsuite/21_strings/basic_string_view/cons/char/nonnull.cc 
b/libstdc++-v3/testsuite/21_strings/basic_string_view/cons/char/nonnull.cc
index b33af088a3d..f42f95e2fdd 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string_view/cons/char/nonnull.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string_view/cons/char/nonnull.cc
@@ -25,5 +25,5 @@ test01()
 {
   std::string_view s((const char*)nullptr); // { dg-warning "\\\[-Wnonnull" }
   std::string_view t((char*)nullptr);  // { dg-warning "\\\[-Wnonnull" }
-  std::string_view u(nullptr); // { dg-warning "\\\[-Wnonnull" }
+  std::string_view u(nullptr); // { dg-error "deleted" }
 }
diff --git 
a/libstdc++-v3/testsuite/21_strings/basic_string_view/cons/wchar_t/nonnull.cc 
b/libstdc++-v3/testsuite/21_strings/basic_string_view/cons/wchar_t/nonnull.cc
index 4c1013177eb..e480f7c4923 100644
--- 
a/libstdc++-v3/testsuite/21_strings/basic_string_view/cons/wchar_t/nonnull.cc
+++ 
b/libstdc++-v3/testsuite/21_strings/basic_string_view/cons/wchar_t/nonnull.cc
@@ -25,5 +25,5 @@ test01()
 {
   std::wstring_view s((const wchar_t*)nullptr);// { dg-warning 
"\\\[-Wnonnull" }
   std::wstring_view 

Re: [RFC][Patch][middle-end/PR102359]Not add initialization for READONLY variables with -ftrivial-auto-var-init

2021-10-04 Thread Qing Zhao via Gcc-patches


> On Oct 4, 2021, at 1:44 AM, Richard Biener  wrote:
> 
> On Fri, 1 Oct 2021, Qing Zhao wrote:
> 
>> 
>> 
>>> On Oct 1, 2021, at 10:33 AM, Jason Merrill  wrote:
>>> 
>>> On 10/1/21 10:54, Qing Zhao wrote:
> On Sep 30, 2021, at 2:31 PM, Jason Merrill  wrote:
> 
> On 9/30/21 11:42, Qing Zhao wrote:
>>> On Sep 30, 2021, at 1:54 AM, Richard Biener  wrote:
>>> 
>>> On Thu, 30 Sep 2021, Jason Merrill wrote:
>>> 
 On 9/29/21 17:30, Qing Zhao wrote:
> Hi,
> 
> PR102359 (ICE gimplification failed since  r12-3433-ga25e0b5e6ac8a77a)
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102359
> 
> Is due to -ftrivial-auto-var-init adding initialization for READONLY
> variable “this” in the following routine: (t.cpp.005t.original)
> 
> ===
> 
> ;; Function A::foo():: (null)
> ;; enabled by -tree-original
> 
> {
> const struct A * const this [value-expr: &__closure->__this];
>   const struct A * const this [value-expr: &__closure->__this];
> return  = (double) ((const struct A *) this)->a;
> }
> ===
> 
> However, in the above routine, “this” is NOT marked as READONLY, but 
> its
> value-expr "&__closure->__this” is marked as READONLY.
> 
> There are two major issues:
> 
> 1. In the routine “is_var_need_auto_init”, we should exclude “decl” 
> that is
> marked as READONLY;
> 2. In the C++ FE, “this” should be marked as READONLY.
> 
> The idea solution will be:
> 
> 1. Fix “is_var_need_auto_init” to exclude TREE_READONLY (decl);
> 2. Fix C++ FE to mark “this” as TREE_READONLY (decl)==true;
> 
> Not sure whether it’s hard for C++ FE to fix the 2nd issue or not?
> 
> In the case it’s not a quick fix in C++FE, I proposed the following 
> fix in
> middle end:
> 
> Let me know your comments or suggestions on this.
> 
> Thanks a lot for the help.
 
 I'd think is_var_need_auto_init should be false for any variable with
 DECL_HAS_VALUE_EXPR_P, as they aren't really variables, just ways of 
 naming
 objects that are initialized elsewhere.
>>> 
>>> IIRC handing variables with DECL_HAS_VALUE_EXPR_P is necessary to
>>> auto-init VLAs, otherwise I tend to agree - would we handle those
>>> when we see a DECL_EXPR then?
>> The current implementation is:
>> gimplify_decl_expr:
>> For each DECL_EXPR “decl”
>>   If (VAR_P (decl) && !DECL_EXTERNAL (decl))
>> {
>>  if (is_vla (decl))
>>  gimplify_vla_decl (decl, …);  /* existing handling: create 
>> a VALUE_EXPR for this vla decl*/
>>  …
>>  if (has_explicit_init (decl))
>>{
>> …; /* existing handling.  */
>>}
>>  else if (is_var_need_auto_init (decl))  /*. New code. */
>>{
>>  gimple_add_init_for_auto_var (….);   /*  new code.  */
>>  ...
>>}
>> }
>> Since the “DECL_VALUE_EXPR (decl)” is NOT a DECL_EXPR, it will not be 
>> scanned and added initialization.
>> if we do not add initialization for a decl that has DECL_VALUE_EXPR, 
>> then the “DECL_VALUE_EXPR (decl)” will not be added an initialization 
>> either.  We will miss adding initializations for these decls.
>> So, I think that the current implementation is correct.
>> And if C++ FE will not mark “this” as READONLY, only mark 
>> DECL_VALUE_EXPR(this) as READONLY, the proposed fix is correct too.
>> Let me know your opinion on this.
> 
> The problem with this test is not whether the 'this' proxy is marked 
> READONLY, the problem is that you're trying to initialize lambda capture 
> proxies at all; the lambda capture objects were already initialized when 
> forming the closure object.  So this test currently aborts with 
> -ftrivial-auto-var-init=zero because you "initialize" the i capture field 
> to 0 after it was previously initialized to 42:
> 
> int main()
> {
> int i = 42;
> auto l = [=]() mutable { return i; };
> if (l() != i)
>   __builtin_abort ();
> }
> 
> I believe the same issue applies to the proxy variables in coroutines 
> that work much like lambdas.
>>> 
 So, how should the middle end determine that a variable is “proxy 
 variable”?
>>> 
>>> In the front end, is_capture_proxy will identify a lambda capture proxy 
>>> variable.  But that won't be true for the similar proxies used by 
>>> coroutines.
>> 
>> Does this mean that in middle end, especially in gimplification phase, there 
>> is Not a simple way to determine whether a variable is a proxy variable?
>>> 
 Have all “proxy variables” been initialized by C++ FE already?

[PATCH] Improve integer bit test on atomic builtin return

2021-10-04 Thread H.J. Lu via Gcc-patches
commit adedd5c173388ae505470df152b9cb3947339566
Author: Jakub Jelinek 
Date:   Tue May 3 13:37:25 2016 +0200

re PR target/49244 (__sync or __atomic builtins will not emit 'lock 
bts/btr/btc')

optimized bit test on atomic builtin return with lock bts/btr/btc.  But
it works only for unsigned integers since atomic builtins operate on the
'uintptr_t' type.  It fails on bool:

  _1 = atomic builtin;
  _4 = (_Bool) _1;

and signed integers:

  _1 = atomic builtin;
  _2 = (int) _1;
  _5 = _2 & (1 << N);

Improve bit test on atomic builtin return by converting:

  _1 = atomic builtin;
  _4 = (_Bool) _1;

to

  _1 = atomic builtin;
  _5 = _1 & (1 << 0);
  _4 = (_Bool) _5;

and converting:

  _1 = atomic builtin;
  _2 = (int) _1;
  _5 = _2 & (1 << N);

to
  _1 = atomic builtin;
  _6 = _1 & (1 << N);
  _5 = (int) _6;

gcc/

PR middle-end/102566
* tree-ssa-ccp.c (optimize_atomic_bit_test_and): Handle cast
between atomic builtin and bit test.

gcc/testsuite/

PR middle-end/102566
* g++.target/i386/pr102566-1.C: New test.
* gcc.target/i386/pr102566-1a.c: Likewise.
* gcc.target/i386/pr102566-1b.c: Likewise.
* gcc.target/i386/pr102566-2.c: Likewise.
---
 gcc/testsuite/g++.target/i386/pr102566-1.C  |  12 ++
 gcc/testsuite/gcc.target/i386/pr102566-1a.c | 188 
 gcc/testsuite/gcc.target/i386/pr102566-1b.c | 107 +++
 gcc/testsuite/gcc.target/i386/pr102566-2.c  |  14 ++
 gcc/tree-ssa-ccp.c  | 136 +-
 5 files changed, 452 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.target/i386/pr102566-1.C
 create mode 100644 gcc/testsuite/gcc.target/i386/pr102566-1a.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr102566-1b.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr102566-2.c

diff --git a/gcc/testsuite/g++.target/i386/pr102566-1.C 
b/gcc/testsuite/g++.target/i386/pr102566-1.C
new file mode 100644
index 000..6e33298d8bf
--- /dev/null
+++ b/gcc/testsuite/g++.target/i386/pr102566-1.C
@@ -0,0 +1,12 @@
+/* { dg-do compile { target c++11 } } */
+/* { dg-options "-O2" } */
+
+#include 
+
+bool tbit(std::atomic )
+{
+  return i.fetch_or(1, std::memory_order_relaxed) & 1;
+}
+
+/* { dg-final { scan-assembler-times "lock;?\[ \t\]*btsl" 1 } } */
+/* { dg-final { scan-assembler-not "cmpxchg" } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr102566-1a.c 
b/gcc/testsuite/gcc.target/i386/pr102566-1a.c
new file mode 100644
index 000..a915de354e5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr102566-1a.c
@@ -0,0 +1,188 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+void bar (void);
+
+__attribute__((noinline, noclone)) int
+f1 (int *a, int bit)
+{
+  int mask = 1 << bit;
+  return (__sync_fetch_and_or (a, mask) & mask) != 0;
+}
+
+__attribute__((noinline, noclone)) int
+f2 (int *a, int bit)
+{
+  int mask = 1 << bit;
+  int t1 = __atomic_fetch_or (a, mask, __ATOMIC_RELAXED);
+  int t2 = t1 & mask;
+  return t2 != 0;
+}
+
+__attribute__((noinline, noclone)) long int
+f3 (long int *a, int bit)
+{
+  long int mask = 1l << bit;
+  return (__atomic_fetch_or (a, mask, __ATOMIC_SEQ_CST) & mask) == 0;
+}
+
+__attribute__((noinline, noclone)) int
+f4 (int *a)
+{
+  int mask = 1 << 7;
+  return (__sync_fetch_and_or (a, mask) & mask) != 0;
+}
+
+__attribute__((noinline, noclone)) int
+f5 (int *a)
+{
+  int mask = 1 << 13;
+  return (__atomic_fetch_or (a, mask, __ATOMIC_RELAXED) & mask) != 0;
+}
+
+__attribute__((noinline, noclone)) int
+f6 (int *a)
+{
+  int mask = 1 << 0;
+  return (__atomic_fetch_or (a, mask, __ATOMIC_SEQ_CST) & mask) != 0;
+}
+
+__attribute__((noinline, noclone)) void
+f7 (int *a, int bit)
+{
+  int mask = 1 << bit;
+  if ((__sync_fetch_and_xor (a, mask) & mask) != 0)
+bar ();
+}
+
+__attribute__((noinline, noclone)) void
+f8 (int *a, int bit)
+{
+  int mask = 1 << bit;
+  if ((__atomic_fetch_xor (a, mask, __ATOMIC_RELAXED) & mask) == 0)
+bar ();
+}
+
+__attribute__((noinline, noclone)) int
+f9 (int *a, int bit)
+{
+  int mask = 1 << bit;
+  return (__atomic_fetch_xor (a, mask, __ATOMIC_SEQ_CST) & mask) != 0;
+}
+
+__attribute__((noinline, noclone)) int
+f10 (int *a)
+{
+  int mask = 1 << 7;
+  return (__sync_fetch_and_xor (a, mask) & mask) != 0;
+}
+
+__attribute__((noinline, noclone)) int
+f11 (int *a)
+{
+  int mask = 1 << 13;
+  return (__atomic_fetch_xor (a, mask, __ATOMIC_RELAXED) & mask) != 0;
+}
+
+__attribute__((noinline, noclone)) int
+f12 (int *a)
+{
+  int mask = 1 << 0;
+  return (__atomic_fetch_xor (a, mask, __ATOMIC_SEQ_CST) & mask) != 0;
+}
+
+__attribute__((noinline, noclone)) int
+f13 (int *a, int bit)
+{
+  int mask = 1 << bit;
+  return (__sync_fetch_and_and (a, ~mask) & mask) != 0;
+}
+
+__attribute__((noinline, noclone)) int
+f14 (int *a, int bit)
+{
+  int mask = 1 << bit;
+  return (__atomic_fetch_and (a, ~mask, __ATOMIC_RELAXED) & mask) != 0;
+}
+
+__attribute__((noinline, noclone)) int
+f15 (int *a, int bit)
+{

Re: [RFC] More jump threading restrictions in the presence of loops.

2021-10-04 Thread Aldy Hernandez via Gcc-patches
Note that none of the other tests have been adjusted so it'll likely result
in multiple threading regressions.

Thanks for looking at this.

Aldy

On Mon, Oct 4, 2021, 15:31 Jeff Law  wrote:

>
>
> On 10/4/2021 3:43 AM, Aldy Hernandez wrote:
> > It is frustrating that virtually all the regressions with the hybrid
> > threader for VRP, have not been with the engine itself, but with the
> > independent restrictions we have agreed upon.
> >
> > The following patch is a collection of discussions with Richi, Jeff,
> > and Michael Matz regarding jump threading limitations in the presence
> > of loops, that I hope can lead to further refinements.
> >
> > As I have mentioned before, most of our threading tests are too
> > fragile, so in this patch I have distilled various restrictions into
> > gimple FE tests that I hope can help in maintaining the threader going
> > forward.  The goal is to have one test with no valid threads
> > whatsover, and one with exclusively one valid thread per function.
> > This should make it trivial to maintain this going forward.
> >
> > I would like to request the relevant experts to not only examine the
> > patch, but review the tests in this patch, to make sure we agree upon
> > these restrictions.  I have distilled the smallest possible test for
> > each restriction and have annotated said tests to make reviewing easy.
> >
> > Note that the test in ssa-thread-valid.c is a thread that Jeff has
> > suggested should be an exception to the path crossing loops
> > restriction, but I have not implemented it yet, because even if we
> > could loosen the restriction, it would violate Richi's restriction of
> > a path that rotates a loop.  Comments are highly welcome.
> >
> > By the way, these restrictions trigger *a lot*.  We seem to be
> > rotating loops left and right.  So I wouldn't be surprised if this
> > requires (as usual) a lot of test tweaking.
> >
> > Untested patch follows.
> >
> > p.s. Note that I'm just facilitating the discussion.  I'm highly
> > dependent on the loop experts here ;-).
> >
> > gcc/ChangeLog:
> >
> >   * tree-ssa-threadupdate.c
> >   (jt_path_registry::cancel_invalid_paths): Tweak.
> >
> > gcc/testsuite/ChangeLog:
> >
> >   * gcc.dg/tree-ssa/ssa-thread-invalid.c: New test.
> >   * gcc.dg/tree-ssa/ssa-thread-valid.c: New test.
> Let me throw that into the tester and see what pops.
>
> Jeff
>
>


Re: [RFC] More jump threading restrictions in the presence of loops.

2021-10-04 Thread Jeff Law via Gcc-patches




On 10/4/2021 3:43 AM, Aldy Hernandez wrote:

It is frustrating that virtually all the regressions with the hybrid
threader for VRP, have not been with the engine itself, but with the
independent restrictions we have agreed upon.

The following patch is a collection of discussions with Richi, Jeff,
and Michael Matz regarding jump threading limitations in the presence
of loops, that I hope can lead to further refinements.

As I have mentioned before, most of our threading tests are too
fragile, so in this patch I have distilled various restrictions into
gimple FE tests that I hope can help in maintaining the threader going
forward.  The goal is to have one test with no valid threads
whatsover, and one with exclusively one valid thread per function.
This should make it trivial to maintain this going forward.

I would like to request the relevant experts to not only examine the
patch, but review the tests in this patch, to make sure we agree upon
these restrictions.  I have distilled the smallest possible test for
each restriction and have annotated said tests to make reviewing easy.

Note that the test in ssa-thread-valid.c is a thread that Jeff has
suggested should be an exception to the path crossing loops
restriction, but I have not implemented it yet, because even if we
could loosen the restriction, it would violate Richi's restriction of
a path that rotates a loop.  Comments are highly welcome.

By the way, these restrictions trigger *a lot*.  We seem to be
rotating loops left and right.  So I wouldn't be surprised if this
requires (as usual) a lot of test tweaking.

Untested patch follows.

p.s. Note that I'm just facilitating the discussion.  I'm highly
dependent on the loop experts here ;-).

gcc/ChangeLog:

* tree-ssa-threadupdate.c
(jt_path_registry::cancel_invalid_paths): Tweak.

gcc/testsuite/ChangeLog:

* gcc.dg/tree-ssa/ssa-thread-invalid.c: New test.
* gcc.dg/tree-ssa/ssa-thread-valid.c: New test.

Let me throw that into the tester and see what pops.

Jeff



Re: [PATCH] Improve jump threading dump output.

2021-10-04 Thread Jeff Law via Gcc-patches




On 10/4/2021 6:05 AM, Aldy Hernandez wrote:

On Thu, Sep 30, 2021 at 8:26 PM Jeff Law  wrote:


So I'm really wondering if these were caused by that patch you'd sent me
privately for the visium issue.  Right now we're regressing in a few
places, but it's not bad.

visium & bfin are the only embedded targets failing.

visium fails:
Tests that now fail, but worked before (9 tests):

visium-sim: gcc.c-torture/execute/960218-1.c   -Os  (test for excess errors)
visium-sim: gcc.c-torture/execute/961125-1.c   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  (test for
excess errors)
visium-sim: gcc.c-torture/execute/961125-1.c   -O3 -g  (test for excess
errors)
visium-sim: gcc.c-torture/execute/pending-4.c   -O1  (test for excess
errors)
visium-sim: gcc.c-torture/execute/pr58209.c   -O2  (test for excess errors)
visium-sim: gcc.c-torture/execute/pr58209.c   -O2 -flto
-fno-use-linker-plugin -flto-partition=none  (test for excess errors)
visium-sim: gcc.c-torture/execute/pr58209.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects  (test for excess errors)
visium-sim: gcc.c-torture/execute/pr58209.c   -O3 -g  (test for excess
errors)
visium-sim: gcc.c-torture/execute/pr68911.c   -O1  (test for excess errors)

We've already discussed 960218-1 a bit.  I wouldn't be surprised if
they're all the same problem in the end.  These started with:

Is this still an issue?  I'm having some trouble reproducing it.

I tried building a combined tree with adapted instructions from here
(dunno if they still apply):

https://gcc.gnu.org/wiki/Building_Cross_Toolchains_with_gcc

Building visium-sim fails with:

 BFD does not support target visium-sim-none.

Building visium-elf or even bfin-elf has libctf compile errors.

Instead, I tried just building cc1 for both visium and bfin but I
don't get any *compilation* errors from the above tests.  I'm assuming
compilation errors since they say "test for excess errors".  Are they
compilation errors?

If this is still an issue, is there an easy way to reproduce?
For the visium stuff, look for a call to abort () in the resulting 
assembly code of 960218-1.  Due to the newlib/libgloss bug on the visum 
port, any call to abort () results in a link error which triggers the 
excess errors failure.


Jeff


[PATCH] avoid hardreg autoinit

2021-10-04 Thread Richard Biener via Gcc-patches
This avoids initializating "uninitialized" hardregs like SP.

Bootstrap / regtest pending on x86_64-unknown-linux-gnu.

2021-10-04  Richard Biener  

* gimplify.c (is_var_need_auto_init): DECL_HARD_REGISTER
variables are not to be initialized.

* gcc.dg/auto-init-hardreg-1.c: New testcase.
---
 gcc/gimplify.c | 2 ++
 gcc/testsuite/gcc.dg/auto-init-hardreg-1.c | 9 +
 2 files changed, 11 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/auto-init-hardreg-1.c

diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index f4bc649632e..b27776af7c8 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -1825,6 +1825,8 @@ static bool
 is_var_need_auto_init (tree decl)
 {
   if (auto_var_p (decl)
+  && (TREE_CODE (decl) != VAR_DECL
+ || !DECL_HARD_REGISTER (decl))
   && (flag_auto_var_init > AUTO_INIT_UNINITIALIZED)
   && (!lookup_attribute ("uninitialized", DECL_ATTRIBUTES (decl)))
   && !is_empty_type (TREE_TYPE (decl)))
diff --git a/gcc/testsuite/gcc.dg/auto-init-hardreg-1.c 
b/gcc/testsuite/gcc.dg/auto-init-hardreg-1.c
new file mode 100644
index 000..7c9de99bf05
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/auto-init-hardreg-1.c
@@ -0,0 +1,9 @@
+/* { dg-do run { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-ftrivial-auto-var-init=zero" } */
+
+int
+main ()
+{
+  register long *sp __asm__("sp");
+  return 0;
+}
-- 
2.31.1


Re: [PATCH] debug: Handle x_flag_var_tracking_* in finish_options [PR102585]

2021-10-04 Thread Martin Liška

On 10/4/21 13:45, Jakub Jelinek wrote:

On Mon, Oct 04, 2021 at 01:41:36PM +0200, Richard Biener via Gcc-patches wrote:

The patch sets -1 for x_flag_var_tracking and x_flag_var_tracking_assignments 
similarly
to toplev.c.

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


Shouldn't all those flag_* == AUTODETECT_VALUE be converted to
!global_options_set.x_flag_* or at least
!global_options_set.x_flag_* && flag_* == AUTODETECT_VALUE ?


Reading the code one more time, yes, I think so. It's pretty ugly code right 
now,
let me investigate what can we do with that..

Martin



Re: [PATCH] Improve jump threading dump output.

2021-10-04 Thread Aldy Hernandez via Gcc-patches
On Thu, Sep 30, 2021 at 8:26 PM Jeff Law  wrote:

> So I'm really wondering if these were caused by that patch you'd sent me
> privately for the visium issue.  Right now we're regressing in a few
> places, but it's not bad.
>
> visium & bfin are the only embedded targets failing.
>
> visium fails:
> Tests that now fail, but worked before (9 tests):
>
> visium-sim: gcc.c-torture/execute/960218-1.c   -Os  (test for excess errors)
> visium-sim: gcc.c-torture/execute/961125-1.c   -O3 -fomit-frame-pointer
> -funroll-loops -fpeel-loops -ftracer -finline-functions  (test for
> excess errors)
> visium-sim: gcc.c-torture/execute/961125-1.c   -O3 -g  (test for excess
> errors)
> visium-sim: gcc.c-torture/execute/pending-4.c   -O1  (test for excess
> errors)
> visium-sim: gcc.c-torture/execute/pr58209.c   -O2  (test for excess errors)
> visium-sim: gcc.c-torture/execute/pr58209.c   -O2 -flto
> -fno-use-linker-plugin -flto-partition=none  (test for excess errors)
> visium-sim: gcc.c-torture/execute/pr58209.c   -O2 -flto
> -fuse-linker-plugin -fno-fat-lto-objects  (test for excess errors)
> visium-sim: gcc.c-torture/execute/pr58209.c   -O3 -g  (test for excess
> errors)
> visium-sim: gcc.c-torture/execute/pr68911.c   -O1  (test for excess errors)
>
> We've already discussed 960218-1 a bit.  I wouldn't be surprised if
> they're all the same problem in the end.  These started with:

Is this still an issue?  I'm having some trouble reproducing it.

I tried building a combined tree with adapted instructions from here
(dunno if they still apply):

https://gcc.gnu.org/wiki/Building_Cross_Toolchains_with_gcc

Building visium-sim fails with:

BFD does not support target visium-sim-none.

Building visium-elf or even bfin-elf has libctf compile errors.

Instead, I tried just building cc1 for both visium and bfin but I
don't get any *compilation* errors from the above tests.  I'm assuming
compilation errors since they say "test for excess errors".  Are they
compilation errors?

If this is still an issue, is there an easy way to reproduce?

Thanks.
Aldy



Re: [PATCH] debug: Handle x_flag_var_tracking_* in finish_options [PR102585]

2021-10-04 Thread Jakub Jelinek via Gcc-patches
On Mon, Oct 04, 2021 at 01:41:36PM +0200, Richard Biener via Gcc-patches wrote:
> > The patch sets -1 for x_flag_var_tracking and 
> > x_flag_var_tracking_assignments similarly
> > to toplev.c.
> >
> > Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Shouldn't all those flag_* == AUTODETECT_VALUE be converted to
!global_options_set.x_flag_* or at least
!global_options_set.x_flag_* && flag_* == AUTODETECT_VALUE ?

Jakub



[PATCH] middle-end/102285 - refine .DEFERRED_INIT expansion

2021-10-04 Thread Richard Biener via Gcc-patches
This refines the way we figure whether we are facing a register
that cannot be initialized by emitting a memset away from inspecting
expanded RTL of the LHS to using the predicates expand_assignment
is using to detect decls or MEM_REFs with non-memory DECL_RTL.

Bootstrapped and tested on x86_64-unknown-linux-gnu, I'm double-checking
the testsuite with -ftrivial-auto-var-init=zero right now.

Richard.

2021-10-04  Richard Biener  

* expr.h (non_mem_decl_p): Declare.
(mem_ref_refers_to_non_mem_p): Likewise.
* expr.c (non_mem_decl_p): Export.
(mem_ref_refers_to_non_mem_p): Likewise.
* internal-fn.c (expand_DEFERRED_INIT): Do not expand the LHS
but check the base with mem_ref_refers_to_non_mem_p
and non_mem_decl_p.

* c-c++-common/pr102285.c: New testcase.
---
 gcc/expr.c|  4 ++--
 gcc/expr.h|  3 +++
 gcc/internal-fn.c |  7 +--
 gcc/testsuite/c-c++-common/pr102285.c | 10 ++
 4 files changed, 20 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/pr102285.c

diff --git a/gcc/expr.c b/gcc/expr.c
index e0bcbccd905..eb33643bd77 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -5305,7 +5305,7 @@ get_bit_range (poly_uint64_pod *bitstart, poly_uint64_pod 
*bitend, tree exp,
has non-BLKmode.  DECL_RTL must not be a MEM; if
DECL_RTL was not set yet, return false.  */
 
-static inline bool
+bool
 non_mem_decl_p (tree base)
 {
   if (!DECL_P (base)
@@ -5322,7 +5322,7 @@ non_mem_decl_p (tree base)
 /* Returns true if REF refers to an object that does not
reside in memory and has non-BLKmode.  */
 
-static inline bool
+bool
 mem_ref_refers_to_non_mem_p (tree ref)
 {
   tree base;
diff --git a/gcc/expr.h b/gcc/expr.h
index 94a85b40dca..890ec19ac7e 100644
--- a/gcc/expr.h
+++ b/gcc/expr.h
@@ -346,4 +346,7 @@ extern void expand_operands (tree, tree, rtx, rtx*, rtx*,
 /* Return an rtx for the size in bytes of the value of an expr.  */
 extern rtx expr_size (tree);
 
+extern bool mem_ref_refers_to_non_mem_p (tree);
+extern bool non_mem_decl_p (tree);
+
 #endif /* GCC_EXPR_H */
diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
index ef5dc90db56..110145218b9 100644
--- a/gcc/internal-fn.c
+++ b/gcc/internal-fn.c
@@ -3015,8 +3015,11 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
 reg_lhs = true;
   else
 {
-  rtx tem = expand_expr (lhs, NULL_RTX, VOIDmode, EXPAND_WRITE);
-  reg_lhs = !MEM_P (tem);
+  tree lhs_base = lhs;
+  while (handled_component_p (lhs_base))
+   lhs_base = TREE_OPERAND (lhs_base, 0);
+  reg_lhs = (mem_ref_refers_to_non_mem_p (lhs_base)
+|| non_mem_decl_p (lhs_base));
 }
 
   if (!reg_lhs)
diff --git a/gcc/testsuite/c-c++-common/pr102285.c 
b/gcc/testsuite/c-c++-common/pr102285.c
new file mode 100644
index 000..644054ba585
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr102285.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-O -ftrivial-auto-var-init=zero -Wuninitialized" } */
+
+int
+qy (void)
+{
+  int tw = 4;
+  int fb[tw];
+  return fb[2]; /* { dg-warning "uninitialized" } */
+}
-- 
2.31.1


Re: [PATCH] debug: Handle x_flag_var_tracking_* in finish_options [PR102585]

2021-10-04 Thread Richard Biener via Gcc-patches
On Mon, Oct 4, 2021 at 12:31 PM Martin Liška  wrote:
>
> Hello.
>
> The patch sets -1 for x_flag_var_tracking and x_flag_var_tracking_assignments 
> similarly
> to toplev.c.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
> Thanks,
> Martin
>
> PR debug/102585
>
> gcc/ChangeLog:
>
> * opts.c (finish_options): Similarly to toplev.c
>   (process_options), set value -1.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/pr102585.c: New test.
> ---
>   gcc/opts.c  | 3 +++
>   gcc/testsuite/gcc.dg/pr102585.c | 6 ++
>   2 files changed, 9 insertions(+)
>   create mode 100644 gcc/testsuite/gcc.dg/pr102585.c
>
> diff --git a/gcc/opts.c b/gcc/opts.c
> index 6503980cd33..f2f058b3ece 100644
> --- a/gcc/opts.c
> +++ b/gcc/opts.c
> @@ -1343,6 +1343,9 @@ finish_options (struct gcc_options *opts, struct 
> gcc_options *opts_set,
>   opts->x_flag_complex_method = 1;
> else if (opts_set->x_flag_cx_fortran_rules)
>   opts->x_flag_complex_method = opts->x_flag_default_complex_method;
> +
> +  if (opts->x_flag_var_tracking_assignments && !opts->x_flag_var_tracking)
> +opts->x_flag_var_tracking = opts->x_flag_var_tracking_assignments = -1;

I can see how this affects later tests in process_options like

  if (flag_var_tracking_uninit == AUTODETECT_VALUE)
flag_var_tracking_uninit = flag_var_tracking;

would now become -1 instead of zero?

So iff then probably this whole block would need to move but the option
handling code is a sophisticated mess between FE, target and middle-end ...

diff --git a/gcc/toplev.c b/gcc/toplev.c
index ec9f998a49b..fec72b1365a 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -2147,8 +2147,6 @@ standard_type_bitsize (int bitsize)
 static void
 do_compile ()
 {
-  process_options ();
-
   /* Don't do any more if an error has already occurred.  */
   if (!seen_error ())
 {
@@ -2368,6 +2366,8 @@ toplev::main (int argc, char **argv)
   /* Exit early if we can (e.g. -help).  */
   if (!exit_after_options)
 {
+  process_options ();
+
   if (m_use_TV_TOTAL)
start_timevars ();
   do_compile ();

would at least make the flow a bit more obvious ;)  IIRC process_options
was/is supposed to be the place to emit diagnostics about options but
clearly it's doing even more.  I guess outlining the call to
lang_hooks.post_options
would be another piece to make the flow clearer, likewise setting of
optimization_default_node ...

Richard.

>   }
>
>   #define LEFT_COLUMN   27
> diff --git a/gcc/testsuite/gcc.dg/pr102585.c b/gcc/testsuite/gcc.dg/pr102585.c
> new file mode 100644
> index 000..efd066b4a4e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr102585.c
> @@ -0,0 +1,6 @@
> +/* PR debug/102585 */
> +/* { dg-do compile } */
> +/* { dg-options "-fvar-tracking-assignments -fno-var-tracking" } */
> +
> +#pragma GCC optimize 0
> +void d_demangle_callback_Og() { int c = 0; }
> --
> 2.33.0
>


Re: [PATCH] gcov: make profile merging smarter

2021-10-04 Thread Martin Liška

On 10/4/21 13:16, Richard Biener wrote:

I meant in merge_one_data do not check ->stamp or ->checksum but instead rely
on the counter merging code to detect mismatches (there's read_mismatch and
read_error).  There's multiple things we can do when we run into those:

  - when we did not actually merged any counter yet we could issue the
warning as before and drop the old data on the floor
  - when we_did_  merge some counters already we could hard-error
(I suppose we can't roll-back merging that took place already)
  - we could do the merging two-stage, first see whether the data matches
and only if it did perform the merging


I've got your point, you are basically suggesting a fine grained merging
(function based). Huh, I don't like it much as it's typically a mistake
in the build setup that 2 objects (with a different checksum) want to emit
profile to the same .gcda file.

My patch handles the obvious situation where an object file is built exactly
the same way (so no e.g. -O0 and -O2).



Note that all of the changes (including yours) have user-visible effects and
the behavior is somewhat unobvious.  Not merging when the object was
re-built is indeed the most obvious behavior so I'm not sure it's a good
idea.  A new env variable to say whether to simply keep the_old_  data
when merging in new data isn't possible would be another "fix" I guess?


Even for a situation when checksum matches, but the timestamp is different?
Sure, we can provide env. variables that can tweak the behavior.

Cheers,
Martin



Re: [PATCH 3/N] Come up with casm global state.

2021-10-04 Thread Martin Liška

On 9/16/21 15:12, Martin Liška wrote:

This patch comes up with asm_out_state and a new global variable casm.

Tested on all cross compilers.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin


As explained in the '3/N' part, we will have to live with asm_out_file macro
for some time. Is this renaming patch fine?

Thanks,
Martin


Re: [PATCH] gcov: make profile merging smarter

2021-10-04 Thread Richard Biener via Gcc-patches
On Fri, Oct 1, 2021 at 12:53 PM Martin Liška  wrote:
>
> On 10/1/21 12:17, Richard Biener wrote:
> > On Fri, Oct 1, 2021 at 11:55 AM Martin Liška  wrote:
> >>
> >> Support merging of profiles that are built from a different .o files
> >> but belong to the same source file. Moreover, a checksum is verified
> >> during profile merging and so we can safely combine such profile.
> >>
> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >> I'm going to install the patch if there are no comments about it.
> >
> > Is the ->stamp field now unused?
>
> Yes, it's still used for pairing of .gcda and .gcno files when --coverage is 
> used.
>
> >
> > I wonder whether crc32 is good enough or whether we need to enable
>
> Dunno. We can alternatively use a stronger hashing function, maybe inchash?
>
> > -fprofile-correction by default for example?
>
> Probably not.
>
> > But -fprofile-correction
> > is about -fprofile-use, not profile merging, right?  What does the latter
> > do upon mismatch?
>
> It prints the 'libgcov profiling error'.
>
> >
> > Alternatively would it be possible to keep multiple sets of data in the 
> > file,
> > one for each 'stamp'?
>
> Yes, we can theoretically do it, but I'm not planning working on that now.
>
> > How does the profile-use step figure a mismatch
> > here, or does it not care whether it mixes input with 'different stamp'?
>
> Based on function id, it does verification for cfg_checksum and 
> lineno_checksum.
>
> >
> > The current behavior is also somewhat odd:
> >
> >> ./a.out
> > libgcov profiling error:/tmp/t.gcda:overwriting an existing profile
> > data with a different timestamp
> >
> > but it should probably say 'warning' since it indeed simply overwrites data
> > instead of merging it.
>
> Yes, I can prepare an incremental patch for it.
>
> >
> > I wonder if we can simply drop the stamp check alltogether and make
> > the checks that match up the data against the number of counters behave
> > as to warning and overwriting the data instead of failing fatally?  The
> > actual merging of data would need to be delayed, but at least until
> > the first actual merge was done we could simply proceed?
>
> Do you mean doing only merging of functions that have correct checksum, and 
> bailing
> out for the functions that don't?

I meant in merge_one_data do not check ->stamp or ->checksum but instead rely
on the counter merging code to detect mismatches (there's read_mismatch and
read_error).  There's multiple things we can do when we run into those:

 - when we did not actually merged any counter yet we could issue the
   warning as before and drop the old data on the floor
 - when we _did_ merge some counters already we could hard-error
   (I suppose we can't roll-back merging that took place already)
 - we could do the merging two-stage, first see whether the data matches
   and only if it did perform the merging

Note that all of the changes (including yours) have user-visible effects and
the behavior is somewhat unobvious.  Not merging when the object was
re-built is indeed the most obvious behavior so I'm not sure it's a good
idea.  A new env variable to say whether to simply keep the _old_ data
when merging in new data isn't possible would be another "fix" I guess?

Richard.

>
> Thanks for the ideas.
>
> Martin
>
> >
> > Richard.
> >
> >> Thanks,
> >> Martin
> >>
> >>  PR gcov-profile/90364
> >>
> >> gcc/ChangeLog:
> >>
> >>  * coverage.c (build_info): Emit checksum to the global variable.
> >>  (build_info_type): Add new field for checksum.
> >>  (coverage_obj_finish): Pass object_checksum.
> >>  (coverage_init): Use 0 as checksum for .gcno files.
> >>  * gcov-dump.c (dump_gcov_file): Dump also new checksum field.
> >>  * gcov.c (read_graph_file): Read also checksum.
> >>
> >> libgcc/ChangeLog:
> >>
> >>  * libgcov-driver.c (merge_one_data): Skip timestamp and verify
> >>  checksums.
> >>  (write_one_data): Write also checksum.
> >>  * libgcov-util.c (read_gcda_file): Read also checksum field.
> >>  * libgcov.h (struct gcov_info): Add new field.
> >> ---
> >>gcc/coverage.c  | 50 -
> >>gcc/gcov-dump.c |  9 
> >>gcc/gcov.c  |  5 +
> >>libgcc/libgcov-driver.c |  8 +--
> >>libgcc/libgcov-util.c   |  3 +++
> >>libgcc/libgcov.h|  1 +
> >>6 files changed, 54 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/gcc/coverage.c b/gcc/coverage.c
> >> index 10d7f8366cb..4467f1eaa5c 100644
> >> --- a/gcc/coverage.c
> >> +++ b/gcc/coverage.c
> >> @@ -129,16 +129,7 @@ static const char *const ctr_names[GCOV_COUNTERS] = {
> >>#undef DEF_GCOV_COUNTER
> >>
> >>/* Forward declarations.  */
> >> -static void read_counts_file (void);
> >>static tree build_var (tree, tree, int);
> >> -static void build_fn_info_type (tree, unsigned, tree);
> >> -static 

Re: [PATCH 3/N] Come up with casm global state.

2021-10-04 Thread Martin Liška

On 9/22/21 11:59, Richard Biener wrote:

On Thu, Sep 16, 2021 at 3:12 PM Martin Liška  wrote:


This patch comes up with asm_out_state and a new global variable casm.

Tested on all cross compilers.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?


 * output.h (struct asm_out_file): New struct that replaces a
^^^
asm_out_state?


Yes, sure!



You replace a lot of asm_out_file - do we still need the macro then?
(I'd have simplified the patch leaving that replacement out at first)


Well, I actually replaced only a very small fraction of the usage of 
asm_out_file.

$ git grep asm_out_file | grep -v ChangeLog | wc -l

1601


So I think we should live with the macro for some time ...



You leave quite some global state out of the struct that is obviously
related, in the diff I see object_block_htab for example.


Yes, I'm aware of it. Can we do that incrementally?


Basically
everything initialized in init_varasm_once is a candidate (which
then shows const_desc_htab and shared_constant_pool as well
as pending_assemble_externals_set).


Well, these would probably need a different header file (or another #include 
... must
be added before output.h :// ).


For the goal of outputting
early DWARF to another file the state CTOR could have a mode
to not initialize those parts or we could have asm-out-state-with-sections
as base of asm-out-state.


Yes, right now asm_out_state ctor is minimal:

  asm_out_state (): out_file (NULL), in_section (NULL),
sec ({}), anchor_labelno (0), in_cold_section_p (false)
  {
section_htab = hash_table::create_ggc (31);
  }



In the end there will be a target part of the state so I think
construction needs to be defered to the target which can
derive from asm-out-state and initialize the part it needs.
That's currently what targetm.asm_out.init_sections () does
and we'd transform that to a targetm.asm_out.create () or so.
That might already be necessary for the DWARF stuff.


So what do you want to with content of init_varasm_once function?



That said, dealing with the target stuff piecemail is OK, but maybe
try to make sure that init_varasm_once is actually identical
to what the CTOR does?


So you want asm_out_state::asm_out_state doing what we current initialize
in init_varasm_once, right?

Thanks,
Cheers,
Martin




Richard.


Thanks,
Martin




[PATCH] debug: Handle x_flag_var_tracking_* in finish_options [PR102585]

2021-10-04 Thread Martin Liška

Hello.

The patch sets -1 for x_flag_var_tracking and x_flag_var_tracking_assignments 
similarly
to toplev.c.

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

Ready to be installed?
Thanks,
Martin

PR debug/102585

gcc/ChangeLog:

* opts.c (finish_options): Similarly to toplev.c
  (process_options), set value -1.

gcc/testsuite/ChangeLog:

* gcc.dg/pr102585.c: New test.
---
 gcc/opts.c  | 3 +++
 gcc/testsuite/gcc.dg/pr102585.c | 6 ++
 2 files changed, 9 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/pr102585.c

diff --git a/gcc/opts.c b/gcc/opts.c
index 6503980cd33..f2f058b3ece 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -1343,6 +1343,9 @@ finish_options (struct gcc_options *opts, struct 
gcc_options *opts_set,
 opts->x_flag_complex_method = 1;
   else if (opts_set->x_flag_cx_fortran_rules)
 opts->x_flag_complex_method = opts->x_flag_default_complex_method;
+
+  if (opts->x_flag_var_tracking_assignments && !opts->x_flag_var_tracking)
+opts->x_flag_var_tracking = opts->x_flag_var_tracking_assignments = -1;
 }
 
 #define LEFT_COLUMN	27

diff --git a/gcc/testsuite/gcc.dg/pr102585.c b/gcc/testsuite/gcc.dg/pr102585.c
new file mode 100644
index 000..efd066b4a4e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr102585.c
@@ -0,0 +1,6 @@
+/* PR debug/102585 */
+/* { dg-do compile } */
+/* { dg-options "-fvar-tracking-assignments -fno-var-tracking" } */
+
+#pragma GCC optimize 0
+void d_demangle_callback_Og() { int c = 0; }
--
2.33.0



[RFC] More jump threading restrictions in the presence of loops.

2021-10-04 Thread Aldy Hernandez via Gcc-patches
It is frustrating that virtually all the regressions with the hybrid
threader for VRP, have not been with the engine itself, but with the
independent restrictions we have agreed upon.

The following patch is a collection of discussions with Richi, Jeff,
and Michael Matz regarding jump threading limitations in the presence
of loops, that I hope can lead to further refinements.

As I have mentioned before, most of our threading tests are too
fragile, so in this patch I have distilled various restrictions into
gimple FE tests that I hope can help in maintaining the threader going
forward.  The goal is to have one test with no valid threads
whatsover, and one with exclusively one valid thread per function.
This should make it trivial to maintain this going forward.

I would like to request the relevant experts to not only examine the
patch, but review the tests in this patch, to make sure we agree upon
these restrictions.  I have distilled the smallest possible test for
each restriction and have annotated said tests to make reviewing easy.

Note that the test in ssa-thread-valid.c is a thread that Jeff has
suggested should be an exception to the path crossing loops
restriction, but I have not implemented it yet, because even if we
could loosen the restriction, it would violate Richi's restriction of
a path that rotates a loop.  Comments are highly welcome.

By the way, these restrictions trigger *a lot*.  We seem to be
rotating loops left and right.  So I wouldn't be surprised if this
requires (as usual) a lot of test tweaking.

Untested patch follows.

p.s. Note that I'm just facilitating the discussion.  I'm highly
dependent on the loop experts here ;-).

gcc/ChangeLog:

* tree-ssa-threadupdate.c
(jt_path_registry::cancel_invalid_paths): Tweak.

gcc/testsuite/ChangeLog:

* gcc.dg/tree-ssa/ssa-thread-invalid.c: New test.
* gcc.dg/tree-ssa/ssa-thread-valid.c: New test.
---
 .../gcc.dg/tree-ssa/ssa-thread-invalid.c  | 102 ++
 .../gcc.dg/tree-ssa/ssa-thread-valid.c|  57 ++
 gcc/tree-ssa-threadupdate.c   |  29 -
 3 files changed, 186 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-invalid.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-valid.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-invalid.c 
b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-invalid.c
new file mode 100644
index 000..bd56a62a4b4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-invalid.c
@@ -0,0 +1,102 @@
+// { dg-do compile }
+// { dg-options "-O2 -fgimple -fdump-statistics" }
+//
+// This is a collection of seemingly threadble paths that should not be 
allowed.
+
+void foobar (int);
+
+// Possible thread from 2->4->3, but it would rotate the loop.
+void __GIMPLE (ssa)
+f1 ()
+{
+  int i;
+
+  // Pre-header.
+  __BB(2):
+  goto __BB4;
+
+  // Latch.
+  __BB(3):
+  foobar (i_1);
+  i_5 = i_1 + 1;
+  goto __BB4;
+
+  __BB(4,loop_header(1)):
+  i_1 = __PHI (__BB2: 0, __BB3: i_5);
+  if (i_1 != 101)
+goto __BB3;
+  else
+goto __BB5;
+
+  __BB(5):
+  return;
+
+}
+
+// Possible thread from 2->3->5 but threading through the empty latch
+// would create a non-empty latch.
+void __GIMPLE (ssa)
+f2 ()
+{
+  int i;
+
+  // Pre-header.
+  __BB(2):
+  goto __BB3;
+
+  __BB(3,loop_header(1)):
+  i_8 = __PHI (__BB5: i_5, __BB2: 0);
+  foobar (i_8);
+  i_5 = i_8 + 1;
+  if (i_5 != 256)
+goto __BB5;
+  else
+goto __BB4;
+
+  // Latch.
+  __BB(5):
+  goto __BB3;
+
+  __BB(4):
+  return;
+
+}
+
+// Possible thread from 3->5->6->3 but this would thread through the
+// header but not exit the loop.
+int __GIMPLE (ssa)
+f3 (int a)
+{
+  int i;
+
+  __BB(2):
+  goto __BB6;
+
+  __BB(3):
+  if (i_1 != 0)
+goto __BB4;
+  else
+goto __BB5;
+
+  __BB(4):
+  foobar (5);
+  goto __BB5;
+
+  // Latch.
+  __BB(5):
+  i_7 = i_1 + 1;
+  goto __BB6;
+
+  __BB(6,loop_header(1)):
+  i_1 = __PHI (__BB2: 1, __BB5: i_7);
+  if (i_1 <= 99)
+goto __BB3;
+  else
+goto __BB7;
+
+  __BB(7):
+  return;
+
+}
+
+// { dg-final { scan-tree-dump-not "Jumps threaded" "statistics" } }
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-valid.c 
b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-valid.c
new file mode 100644
index 000..e10e877d01a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-valid.c
@@ -0,0 +1,57 @@
+// { dg-do compile }
+// { dg-options "-O2 -fgimple -fdump-statistics" }
+//
+// This is a collection of threadable paths.  To simplify maintenance,
+// there should only be one threadable path per function.
+
+int global;
+
+// ** FIXME: This currently fails**
+//
+// There is a path crossing loops through 3->4->5.
+//
+// Jeff has stipulated that...
+//
+// This might be an exception since this goes from inside the loop to
+// outside the loop without entering another loop.  That is, we have
+// found an early exit from the loop that doesn't require us to go
+// 

[PATCH] var-tracking: Fix a wrong-debug issue caused by my r10-7665 var-tracking change [PR102441]

2021-10-04 Thread Jakub Jelinek via Gcc-patches
Hi!

Since my r10-7665-g33c45e51b4914008064d9b77f2c1fc0eea1ad060 change, we get
wrong-debug on e.g. the following testcase at -O2 -g on x86_64-linux for the
x parameter:
void bar (int *r);
int
foo (int x)
{
  int r = 0;
  bar ();
  return r;
}
At the start of function, we have
subq$24, %rsp
leaq12(%rsp), %rdi
instructions.  The x parameter is passed in %rdi, but isn't used in the
function and so the leaq instruction overwrites %rdi without remembering
%rdi anywhere.  Before the r10-7665 change (which was trying to fix a large
(3% for 32-bit, 1% for 64-bit x86-64) debug info/loc growth introduced with
r10-7515), the leaq insn above resulted in a MO_VAL_SET micro-operation that
said that the value of sp + 12, a cselib_sp_derived_value_p, is stored into
the %rdi register.  The r10-7665 change added a change to add_stores that
added no micro-operation for the leaq store, with the rationale that the sp
based values can be and will be always computable some other more compact
and primarily more stable way (cfa based expression like DW_OP_fbreg, that
is the same in the whole function).  That is true.  But by throwing the
micro-operation on the floor, we miss another important part of the
MO_VAL_SET, in particular that the destination of the store, %rdi in this
case, now has a different value from what it had before, so the vt_*
dataflow code thinks that even after the leaq instruction %rdi still holds
the x argument value (and changes it to DW_OP_entry_value (%rdi) only in the
middle of the call to bar).  Previously and with the patches below,
the location for x changes already at the end of leaq instruction to
DW_OP_entry_value (%rdi).

My first attempt to fix this was instead of dropping the MO_VAL_SET add
a MO_CLOBBER operation:
--- gcc/var-tracking.c.jj   2021-05-04 21:02:24.196799586 +0200
+++ gcc/var-tracking.c  2021-09-24 19:23:16.420154828 +0200
@@ -6133,7 +6133,9 @@ add_stores (rtx loc, const_rtx expr, voi
 {
   if (preserve)
preserve_value (v);
-  return;
+  mo.type = MO_CLOBBER;
+  mo.u.loc = loc;
+  goto log_and_return;
 }
 
   nloc = replace_expr_with_values (oloc);
so don't track that the value lives in the loc destination, but track
that the previous value doesn't live there anymore.  That failed bootstrap
miserably, the vt_* code isn't prepared to see MO_CLOBBER of a MEM that
isn't tracked (e.g. has MEM_EXPR on it that the var-tracking code wants
to track, i.e. track_p in add_stores).  On the other side, thinking about
it more, in the most common case where a cselib_sp_derived_value_p value
is stored into the sp register (and which is the reason why PR94495
testcase got larger), dropping the micro-operation on the floor is the
right thing, because we have that cselib_sp_derived_value_p tracking, any
reads from the sp hard register will be treated as
cselib_sp_derived_value_p.
Then I've tried 3 different patches, patch{1,2,3} attached below and all of
these passed bootstrap/regtest on x86_64-linux and i686-linux. 
Additionally, I've gathered statistics from cc1plus by always reverting the
var-tracking.c change after finished bootstrap/regtest and rebuilding the
stage3 var-tracking.o and cc1plus, such that it would be comparable.
dwlocstat and .debug_{info,loclists} section sizes detailed below.
patch3 uses MO_VAL_SET (i.e. essentially reversion of the r10-7665
change) when destination is not a REG_P and !track_p, otherwise if
destination is sp drops the micro-operation on the floor (i.e. no change),
otherwise adds a MO_CLOBBER.
patch1 is similar, except it checks for destination not equal to sp and
!track_p, i.e. for !track_p REG_P destinations other than sp it will use
MO_VAL_SET rather than MO_CLOBBER.
Finally, patch2, the shortest patch, uses MO_VAL_SET whenever destination
is not sp and otherwise drops the micro-operation on the floor.
All the 3 patches don't affect the PR94495 testcase, all the changes
there were caused by stores of sp based values into %rsp.

While the patch2 (and patch1 which results in exactly the same sizes)
causes the largest debug loclists/info growth from the 3, it is still quite
minor (0.651% on 64-bit and 0.114% on 32-bit) compared
to the 1% and 3% PR94495 was trying to solve, and I actually think it is the
best thing to do.  Because, if we have say
  int q[10];
  int *p = [0];
or similar and we load the [0] sp based value into some hard register,
by noting in the debug info that p lives in some hard reg for some part
of the function and a user is trying to change the p var in the debugger,
if we say it lives in some register or memory, there is some chance that
the changing of the value could work successfully (of course, nothing
is guaranteed, we don't have tracking of where each var lives at which
moment for changing purposes (i.e. what register, memory or else you need
to change in order to change behavior of the code)), while if we just say
that p's location is DW_OP_fbreg 16 DW_OP_stack_value, that 

RE: [PATCH][GCC] arm: Add Cortex-R52+ multilib

2021-10-04 Thread Przemyslaw Wirkus via Gcc-patches
> > On Thu, Sep 30, 2021, 3:37 PM Przemyslaw Wirkus 
> >  wrote:
> > Subject: Re: [PATCH][GCC] arm: Add Cortex-R52+ multilib
> >
> > I think the RTEMS multilibs are based on the products that RTEMS supports,
> > so this is really the RTEMS maintainers' call.
> > 
> > Joel?

> > > Ping :)

> I'm ok deferring it since Sebastian doesn't think there is a user right now. 
> But I'm actually rather ambivalent. If it makes it easier to maintain versus 
> the other embedded arm targets then I'm all for it. Maintaining these 
> configurations are a pain.

OK, let's discard this patch as there is no consensus it's useful.

Cheers!

/Przemyslaw

> --joel

> On 22/09/2021 09:46, Przemyslaw Wirkus via Gcc-patches wrote:
> > Patch is adding multilib entries for `cortex-r52plus` CPU.
> >
> > See:
> > https://www.arm.com/products/silicon-ip-cpu/cortex-r/cortex-r52-plus
> >
> > OK for master?
> >
> > gcc/ChangeLog:
> >
> > 2021-09-16  Przemyslaw Wirkus  
> >
> >     * config/arm/t-rtems: Add "-mthumb -mcpu=cortex-r52plus
> >     -mfloat-abi=hard" multilib.
> >


[PATCH] middle-end/102587 - avoid auto-init for VLA vectors

2021-10-04 Thread Richard Biener via Gcc-patches
This avoids ICEing for VLA vector auto-init by not initializing.

Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

2021-10-04  Richard Biener  

PR middle-end/102587
* internal-fn.c (expand_DEFERRED_INIT): Guard register
initialization path an avoid initializing VLA registers
with it.

* gcc.target/aarch64/sve/pr102587-1.c: New testcase.
* gcc.target/aarch64/sve/pr102587-2.c: Likewise.
---
 gcc/internal-fn.c | 3 ++-
 gcc/testsuite/gcc.target/aarch64/sve/pr102587-1.c | 4 
 gcc/testsuite/gcc.target/aarch64/sve/pr102587-2.c | 4 
 3 files changed, 10 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr102587-1.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr102587-2.c

diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
index 8312d08aab2..ef5dc90db56 100644
--- a/gcc/internal-fn.c
+++ b/gcc/internal-fn.c
@@ -3035,7 +3035,8 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
   /* Expand this memset call.  */
   expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
 }
-  else
+  /* ???  Deal with poly-int sized registers.  */
+  else if (tree_fits_uhwi_p (TYPE_SIZE_UNIT (var_type)))
 {
   /* If this variable is in a register, use expand_assignment might
 generate better code.  */
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr102587-1.c 
b/gcc/testsuite/gcc.target/aarch64/sve/pr102587-1.c
new file mode 100644
index 000..2b9a68b0b59
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/sve/pr102587-1.c
@@ -0,0 +1,4 @@
+/* { dg-do compile } */
+/* { dg-options "-ftrivial-auto-var-init=zero" } */
+
+void foo() { __SVFloat64_t f64; }
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr102587-2.c 
b/gcc/testsuite/gcc.target/aarch64/sve/pr102587-2.c
new file mode 100644
index 000..4cdb9056002
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/sve/pr102587-2.c
@@ -0,0 +1,4 @@
+/* { dg-do compile } */
+/* { dg-options "-ftrivial-auto-var-init=pattern" } */
+
+void foo() { __SVFloat64_t f64; }
-- 
2.31.1


[Ada] Incremental patch for restriction No_Dynamic_Accessibility_Checks

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
This patch corrects various issues discovered during testing of the
No_Dynamic_Accessibility_Checks restriction and documents the feature in
the GNAT RM.

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

gcc/ada/

* doc/gnat_rm/standard_and_implementation_defined_restrictions.rst:
Add new entry for No_Dynamic_Accessibility_Checks documenting
behavior.
* gnat_rm.texi: Regenerate.
* exp_ch4.adb (Expand_N_In): Perform special expansion for
membership tests when No_Dynamic_Accessibility_Checks is active.
* sem_attr.adb (Resolve_Attribute): Skip static accessibility
check on actuals for anonymous access type formal parameters,
and add constants for readability.
* sem_util.adb (Function_Call_Or_Allocator_Level): Use the
innermost master for determining the level for function calls
within the alternative accessibility model.
(Type_Access_Level): Properly get the level for anonymous access
function result types.

patch.diff.gz
Description: application/gzip


[Ada] Document the current behaviour of -gnateA switch

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
The runtime checks for overlaping actual parameters, which gets enabled
by the -gnateA switch, were introduced to combine proof and test for the
antialiasing rules of SPARK.

The original implementation of these checks contained few mistakes which
resulted in spurious compilation errors. These are now fixed, but the
current implementation no longer matches the aliasing rules from the
SPARK RM 6.4.2; instead, it is based on the Ada RM 6.2(12/3).

This patch documents the current behaviour in the GNAT User's Guide.

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

gcc/ada/

* doc/gnat_ugn/building_executable_programs_with_gnat.rst
(gnateA): This switch no longer looks at the formal parameter
type being composite (as originally mandated by SPARK), but in
the parameter passing mechanism being not specified (as
currently mandated by Ada).
* gnat_ugn.texi: Regenerate.diff --git a/gcc/ada/doc/gnat_ugn/building_executable_programs_with_gnat.rst b/gcc/ada/doc/gnat_ugn/building_executable_programs_with_gnat.rst
--- a/gcc/ada/doc/gnat_ugn/building_executable_programs_with_gnat.rst
+++ b/gcc/ada/doc/gnat_ugn/building_executable_programs_with_gnat.rst
@@ -1497,9 +1497,10 @@ Alphabetical List of All Switches
 
 :switch:`-gnateA`
   Check that the actual parameters of a subprogram call are not aliases of one
-  another. To qualify as aliasing, the actuals must denote objects of a composite
-  type, their memory locations must be identical or overlapping, and at least one
-  of the corresponding formal parameters must be of mode OUT or IN OUT.
+  another. To qualify as aliasing, their memory locations must be identical or
+  overlapping, at least one of the corresponding formal parameters must be of
+  mode OUT or IN OUT, and at least one of the corresponding formal parameters
+  must have its parameter passing mechanism not specified.
 
 
   .. code-block:: ada


diff --git a/gcc/ada/gnat_ugn.texi b/gcc/ada/gnat_ugn.texi
--- a/gcc/ada/gnat_ugn.texi
+++ b/gcc/ada/gnat_ugn.texi
@@ -8933,9 +8933,10 @@ also suppresses generation of cross-reference information
 @item @code{-gnateA}
 
 Check that the actual parameters of a subprogram call are not aliases of one
-another. To qualify as aliasing, the actuals must denote objects of a composite
-type, their memory locations must be identical or overlapping, and at least one
-of the corresponding formal parameters must be of mode OUT or IN OUT.
+another. To qualify as aliasing, their memory locations must be identical or
+overlapping, at least one of the corresponding formal parameters must be of
+mode OUT or IN OUT, and at least one of the corresponding formal parameters
+must have its parameter passing mechanism not specified.
 
 @example
 type Rec_Typ is record




[Ada] Handle properly user_defined literals given by operators.

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
This patch expands the implementation of aspects Integer_Literal,
Real_Literal and String_Literal, so that the value of the aspect, which
must be a function name, can be specified by an Operator_Symbol.

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

gcc/ada/

* sem_ch6.adb (Analyze_Operator_Symbol): Recognize strings as
operator names when they are the value of one of the Ada2022
aspects for User_Defined_Literals.
* sem_ch13.adb (Analyze_One_Aspect): Handle an aspect value
given by an Operator_Name.
(Validate_Literal_Aspect): Call Analyze_Operator_Symbol when
needed.diff --git a/gcc/ada/sem_ch13.adb b/gcc/ada/sem_ch13.adb
--- a/gcc/ada/sem_ch13.adb
+++ b/gcc/ada/sem_ch13.adb
@@ -2991,7 +2991,15 @@ package body Sem_Ch13 is
 --  Copy expression for later processing by the procedures
 --  Check_Aspect_At_[Freeze_Point | End_Of_Declarations]
 
-Set_Entity (Id, New_Copy_Tree (Expr));
+--  The expression may be a subprogram name, and can
+--  be an operator name that appears as a string, but
+--  requires its own analysis procedure (see sem_ch6).
+
+if Nkind (Expr) = N_Operator_Symbol then
+   Set_Entity (Id, Expr);
+else
+   Set_Entity (Id, New_Copy_Tree (Expr));
+end if;
 
 --  Set Delay_Required as appropriate to aspect
 
@@ -16668,7 +16676,15 @@ package body Sem_Ch13 is
   end if;
 
   if not Overloaded and then not Present (Entity (Func_Name)) then
- Analyze (Func_Name);
+ --  The aspect is specified by a subprogram name, which
+ --  may be an operator name given originally by a string.
+
+ if Is_Operator_Name (Chars (Func_Name)) then
+Analyze_Operator_Symbol (Func_Name);
+ else
+Analyze (Func_Name);
+ end if;
+
  Overloaded := Is_Overloaded (Func_Name);
   end if;
 


diff --git a/gcc/ada/sem_ch6.adb b/gcc/ada/sem_ch6.adb
--- a/gcc/ada/sem_ch6.adb
+++ b/gcc/ada/sem_ch6.adb
@@ -2126,8 +2126,15 @@ package body Sem_Ch6 is
   and then Attribute_Name (Par) /= Name_Value)
 or else (Nkind (Maybe_Aspect_Spec) = N_Aspect_Specification
   and then Get_Aspect_Id (Maybe_Aspect_Spec)
---  include other aspects here ???
-in Aspect_Stable_Properties | Aspect_Aggregate)
+
+--  Include aspects that can be specified by a
+--  subprogram name, which can be an operator.
+
+in  Aspect_Stable_Properties
+  | Aspect_Integer_Literal
+  | Aspect_Real_Literal
+  | Aspect_String_Literal
+  | Aspect_Aggregate)
   then
  Find_Direct_Name (N);
 




[Ada] Fix indentation in generated AST construction functions

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
In the generated nmake.adb file the "return" keyword in function
declarations was printed at the start of a line. Now it is correctly
indented, which makes the generated code easier to read.

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

gcc/ada/

* gen_il-gen.adb (Put_Make_Spec): Don't emit the LF character in
the middle of a string, because the Put routine won't indent it
properly.diff --git a/gcc/ada/gen_il-gen.adb b/gcc/ada/gen_il-gen.adb
--- a/gcc/ada/gen_il-gen.adb
+++ b/gcc/ada/gen_il-gen.adb
@@ -2423,7 +2423,8 @@ package body Gen_IL.Gen is
 end if;
  end loop;
 
- Put (S, ")" & LF & "return " & Node_Or_Entity (Root) & "_Id");
+ Put (S, ")" & LF);
+ Put (S, "return " & Node_Or_Entity (Root) & "_Id");
  Decrease_Indent (S, 2);
  Decrease_Indent (S, 1);
   end Put_Make_Spec;




[Ada] Implement CUDA_Device

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
This commit removes entities marked with the CUDA_Device pragma from the
packages specs and bodies they exist in.

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

gcc/ada/

* gnat_cuda.adb (Remove_CUDA_Device_Entities): New function.
(Expand_CUDA_Package): Call Remove_CUDA_Device_Entities.
* gnat_cuda.ads (Expand_CUDA_Package): Expand documentation.
* sem_prag.adb (Analyze_Pragma): Remove warning about
CUDA_Device not being implemented.diff --git a/gcc/ada/gnat_cuda.adb b/gcc/ada/gnat_cuda.adb
--- a/gcc/ada/gnat_cuda.adb
+++ b/gcc/ada/gnat_cuda.adb
@@ -25,22 +25,25 @@
 
 --  This package defines CUDA-specific datastructures and functions.
 
-with Atree;   use Atree;
-with Debug;   use Debug;
-with Elists;  use Elists;
-with Namet;   use Namet;
-with Nlists;  use Nlists;
-with Nmake;   use Nmake;
-with Rtsfind; use Rtsfind;
-with Sinfo;   use Sinfo;
-with Sinfo.Nodes; use Sinfo.Nodes;
-with Stringt; use Stringt;
-with Tbuild;  use Tbuild;
-with Uintp;   use Uintp;
-with Sem; use Sem;
-with Sem_Aux; use Sem_Aux;
-with Sem_Util;use Sem_Util;
-with Snames;  use Snames;
+with Atree;  use Atree;
+with Debug;  use Debug;
+with Einfo.Entities; use Einfo.Entities;
+with Einfo.Utils;use Einfo.Utils;
+with Elists; use Elists;
+with Errout; use Errout;
+with Namet;  use Namet;
+with Nlists; use Nlists;
+with Nmake;  use Nmake;
+with Rtsfind;use Rtsfind;
+with Sem;use Sem;
+with Sem_Aux;use Sem_Aux;
+with Sem_Util;   use Sem_Util;
+with Sinfo.Nodes;use Sinfo.Nodes;
+with Sinfo;  use Sinfo;
+with Snames; use Snames;
+with Stringt;use Stringt;
+with Tbuild; use Tbuild;
+with Uintp;  use Uintp;
 
 with GNAT.HTable;
 
@@ -120,6 +123,10 @@ package body GNAT_CUDA is
--  are declared within package body Pack_Body. Returns No_Elist if Pack_Id
--  does not contain such procedures.
 
+   procedure Remove_CUDA_Device_Entities (Pack_Id : Entity_Id);
+   --  Removes all entities marked with the CUDA_Device pragma from package
+   --  Pack_Id. Must only be called when compiling for the host.
+
procedure Set_CUDA_Device_Entities
  (Pack_Id : Entity_Id;
   E   : Elist_Id);
@@ -226,6 +233,13 @@ package body GNAT_CUDA is
 
   Empty_CUDA_Global_Subprograms (N);
 
+  --  Remove CUDA_Device entities (except if they are also CUDA_Host), as
+  --  they can only be referenced from the device and might reference
+  --  device-only symbols.
+
+  Remove_CUDA_Device_Entities
+(Package_Specification (Corresponding_Spec (N)));
+
   --  If procedures marked with CUDA_Global have been defined within N,
   --  we need to register them with the CUDA runtime at program startup.
   --  This requires multiple declarations and function calls which need
@@ -718,6 +732,54 @@ package body GNAT_CUDA is
   Analyze (New_Stmt);
end Build_And_Insert_CUDA_Initialization;
 
+   -
+   -- Remove_CUDA_Device_Entities --
+   -
+
+   procedure Remove_CUDA_Device_Entities (Pack_Id : Entity_Id) is
+  Device_Entities : constant Elist_Id :=
+Get_CUDA_Device_Entities (Pack_Id);
+  Device_Elmt : Elmt_Id;
+  Device_Entity   : Entity_Id;
+  Bod : Node_Id;
+   begin
+  pragma Assert (Debug_Flag_Underscore_C);
+
+  if Device_Entities = No_Elist then
+ return;
+  end if;
+
+  Device_Elmt := First_Elmt (Device_Entities);
+  while Present (Device_Elmt) loop
+ Device_Entity := Node (Device_Elmt);
+ Next_Elmt (Device_Elmt);
+
+ case Ekind (Device_Entity) is
+when E_Function | E_Procedure =>
+   Bod := Subprogram_Body (Device_Entity);
+
+   if Nkind (Parent (Bod)) = N_Subunit
+ and then Present (Corresponding_Stub (Parent (Bod)))
+   then
+  Error_Msg_N
+("Cuda_Device not suported on separate subprograms",
+ Corresponding_Stub (Parent (Bod)));
+   else
+  Remove (Bod);
+  Remove (Subprogram_Spec (Device_Entity));
+   end if;
+
+when E_Variable | E_Constant =>
+   Remove (Declaration_Node (Device_Entity));
+
+when others =>
+   pragma Assert (False);
+ end case;
+
+ Remove_Entity_And_Homonym (Device_Entity);
+  end loop;
+   end Remove_CUDA_Device_Entities;
+
--
-- Set_CUDA_Device_Entities --
--


diff --git a/gcc/ada/gnat_cuda.ads b/gcc/ada/gnat_cuda.ads
--- a/gcc/ada/gnat_cuda.ads
+++ b/gcc/ada/gnat_cuda.ads
@@ -86,7 +86,10 @@ package GNAT_CUDA is
--  entity of its parent package body.
 
procedure 

[Ada] Completion of support for AI12-0409 (attribute Preelaborable_Initialization)

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
This set of changes implements proper checking of types completing
private types in a generic unit where the full type has components of
formal types of the generic and the private type has a
Preelaborable_Initialization aspect given by a conjunction of one or
more references to Preelab_Initialization attributes whose prefixes name
formal types. Also done as part of this work is the replacement of
Preelaborable_Initialization pragmas in the Ada bounded containers units
with the corresponding aspect, defining the aspect's expression with
references to P_I attributes applied to formal types of the generic
units as now specified in AI12-0409 and the Ada 2022 RM (and also
replacing the pragma with the aspect on Cursor types).

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

gcc/ada/

* sem_ch7.adb (Analyze_Package_Specification): For types marked
as Must_Have_Preelab_Init, we now check for the presence of a
Preelaborable_Initialization aspect on the type, and pass the
aspect's expression (if any) on the call to
Has_Preelaborable_Initialization (or pass Empty if the type has
no such aspect or the aspect has no associated expression).
* sem_util.ads (Has_Preelaborable_Initialization): Change
Boolean formal parameter Formal_Types_Have_Preelab_Init to
instead be a formal of type Node_Id (named Preelab_Init_Expr),
to allow passing an expression that may be a conjunction of
Preelaborable_Initialization aspects. Revise spec comment
accordingly (and remove ??? comment).
* sem_util.adb (Type_Named_In_Preelab_Init_Expression): New
nested function with a result indicating whether a given type is
named as the prefix of a Preelaborable_Initialization attribute
in the expression of a corresponding P_I aspect.
(Has_Preelaborable_Initialization): For generic formal derived
and private types, test whether the type is named in the
expression Preelab_Init_Expr (by calling
Type_Named_In_Preelab_Init_Expression), and if so, treat the
formal type as having preelaborable initialization (returning
True).
* libgnat/a-cobove.ads (Vector): Replace pragma
Preelaborable_Initialization with the aspect, specifying its
value as Element_Type'Preelaborable_Initialization.
(Cursor): Replace pragma P_I with the aspect (defaulting to
True).
* libgnat/a-cbdlli.ads (List): Replace pragma
Preelaborable_Initialization with the aspect, specifying its
value as Element_Type'Preelaborable_Initialization.
(Cursor): Replace pragma P_I with the aspect (defaulting to
True).
* libgnat/a-cbhama.ads (Map): Replace pragma
Preelaborable_Initialization with the aspect, specifying its
value as (Element_Type'Preelaborable_Initialization and
Key_Type'Preelaborable_Initialization).
(Cursor): Replace pragma P_I with the aspect (defaulting to
True).
* libgnat/a-cborma.ads (Map): Replace pragma
Preelaborable_Initialization with the aspect, specifying its
value as (Element_Type'Preelaborable_Initialization and
Key_Type'Preelaborable_Initialization).
(Cursor): Replace pragma P_I with the aspect (defaulting to
True).
* libgnat/a-cbhase.ads (Set): Replace pragma
Preelaborable_Initialization with the aspect, specifying its
value as Element_Type'Preelaborable_Initialization.
(Cursor): Replace pragma P_I with the aspect (defaulting to
True).
* libgnat/a-cborse.ads (Set): Replace pragma
Preelaborable_Initialization with the aspect, specifying its
value as Element_Type'Preelaborable_Initialization.
(Cursor): Replace pragma P_I with the aspect (defaulting to
True).
* libgnat/a-cbmutr.ads (Tree): Replace pragma
Preelaborable_Initialization with the aspect, specifying its
value as Element_Type'Preelaborable_Initialization.
(Cursor): Replace pragma P_I with the aspect (defaulting to
True).
* libgnat/a-coboho.ads (Holder): Replace pragma
Preelaborable_Initialization with the aspect, specifying its
value as Element_Type'Preelaborable_Initialization.
(Cursor): Replace pragma P_I with the aspect (defaulting to
True).diff --git a/gcc/ada/libgnat/a-cbdlli.ads b/gcc/ada/libgnat/a-cbdlli.ads
--- a/gcc/ada/libgnat/a-cbdlli.ads
+++ b/gcc/ada/libgnat/a-cbdlli.ads
@@ -57,11 +57,11 @@ is
   Default_Iterator  => Iterate,
   Iterator_Element  => Element_Type,
   Aggregate => (Empty=> Empty,
-Add_Unnamed  => Append);
-   pragma Preelaborable_Initialization (List);
+Add_Unnamed  => Append),
+  Preelaborable_Initialization
+=> Element_Type'Preelaborable_Initialization;
 
-   type 

[Ada] Mark Ada.Text_IO in SPARK

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
Now that access-to-constant types are supported in SPARK, type
File_Access is accepted and the public interface of Ada.Text_IO can be
marked in SPARK.

Fix a missing save/restore for the global variable storing whether
SPARK_Mode should be ignored inside instances, around the analysis of a
new unit (e.g.  from a with_clause).

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

gcc/ada/

* libgnat/a-textio.adb: Mark body out of SPARK.
* libgnat/a-textio.ads: Mark spec in SPARK and private part out
of SPARK.
* sem.adb (Semantics.Do_Analyze): Similar to ghost code
attributes, save and restore value of
Ignore_SPARK_Mode_Pragmas_In_Instance.diff --git a/gcc/ada/libgnat/a-textio.adb b/gcc/ada/libgnat/a-textio.adb
--- a/gcc/ada/libgnat/a-textio.adb
+++ b/gcc/ada/libgnat/a-textio.adb
@@ -44,6 +44,7 @@ pragma Elaborate_All (System.File_IO);
 --  Needed because of calls to Chain_File in package body elaboration
 
 package body Ada.Text_IO with
+  SPARK_Mode => Off,
   Refined_State => (File_System => (Standard_In,
 Standard_Out,
 Standard_Err,


diff --git a/gcc/ada/libgnat/a-textio.ads b/gcc/ada/libgnat/a-textio.ads
--- a/gcc/ada/libgnat/a-textio.ads
+++ b/gcc/ada/libgnat/a-textio.ads
@@ -56,8 +56,9 @@ with System.File_Control_Block;
 with System.WCh_Con;
 
 package Ada.Text_IO with
-  Abstract_State=> (File_System),
-  Initializes   => (File_System),
+  SPARK_Mode,
+  Abstract_State=> File_System,
+  Initializes   => File_System,
   Initial_Condition => Line_Length = 0 and Page_Length = 0
 is
pragma Elaborate_Body;
@@ -547,6 +548,7 @@ is
Layout_Error : exception renames IO_Exceptions.Layout_Error;
 
 private
+   pragma SPARK_Mode (Off);
 
--  The following procedures have a File_Type formal of mode IN OUT because
--  they may close the original file. The Close operation may raise an


diff --git a/gcc/ada/sem.adb b/gcc/ada/sem.adb
--- a/gcc/ada/sem.adb
+++ b/gcc/ada/sem.adb
@@ -1402,7 +1402,9 @@ package body Sem is
   procedure Do_Analyze is
  Saved_GM  : constant Ghost_Mode_Type := Ghost_Mode;
  Saved_IGR : constant Node_Id := Ignored_Ghost_Region;
- --  Save the Ghost-related attributes to restore on exit
+ Saved_ISMP : constant Boolean:=
+Ignore_SPARK_Mode_Pragmas_In_Instance;
+ --  Save Ghost and SPARK mode-related data to restore on exit
 
  --  Generally style checks are preserved across compilations, with
  --  one exception: s-oscons.ads, which allows arbitrary long lines
@@ -1421,6 +1423,7 @@ package body Sem is
  --  Set up a clean environment before analyzing
 
  Install_Ghost_Region (None, Empty);
+ Ignore_SPARK_Mode_Pragmas_In_Instance := False;
 
  Outer_Generic_Scope := Empty;
  Scope_Suppress  := Suppress_Options;
@@ -1443,9 +1446,11 @@ package body Sem is
 
  Pop_Scope;
  Restore_Scope_Stack  (List);
- Restore_Ghost_Region (Saved_GM, Saved_IGR);
  Style_Max_Line_Length := Saved_ML;
  Style_Check_Max_Line_Length := Saved_CML;
+
+ Restore_Ghost_Region (Saved_GM, Saved_IGR);
+ Ignore_SPARK_Mode_Pragmas_In_Instance := Saved_ISMP;
   end Do_Analyze;
 
   --  Local variables




[Ada] Minor comment fix in System.Regpat

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
If Data_First is set to the special value of -1 (the default), then we
cannot look at Data (Data_First).

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

gcc/ada/

* libgnat/s-regpat.ads: Change Data_First to Data'First. Change
"still" to "always".  Similar changes for Data_Last.diff --git a/gcc/ada/libgnat/s-regpat.ads b/gcc/ada/libgnat/s-regpat.ads
--- a/gcc/ada/libgnat/s-regpat.ads
+++ b/gcc/ada/libgnat/s-regpat.ads
@@ -482,18 +482,17 @@ package System.Regpat is
--Data_First is the lower bound for the match, i.e. Data (Data_First)
--will be the first character to be examined. If Data_First is set to
--the special value of -1 (the default), then the first character to
-   --be examined is Data (Data_First). However, the regular expression
-   --character ^ (start of string) still refers to the first character
+   --be examined is Data (Data'First). However, the regular expression
+   --character ^ (start of string) always refers to the first character
--of the full string (Data (Data'First)), which is why there is a
--separate mechanism for specifying Data_First.
 
--Data_Last is the upper bound for the match, i.e. Data (Data_Last)
--will be the last character to be examined. If Data_Last is set to
--the special value of Positive'Last (the default), then the last
-   --character to be examined is Data (Data_Last). However, the regular
-   --expression character $ (end of string) still refers to the last
-   --character of the full string (Data (Data'Last)), which is why there
-   --is a separate mechanism for specifying Data_Last.
+   --character to be examined is Data (Data'Last). However, the regular
+   --expression character $ (end of string) always refers to the last
+   --character of the full string (Data (Data'Last)).
 
--Note: the use of Data_First and Data_Last is not equivalent to
--simply passing a slice as Expression because of the handling of




[Ada] Refactor duplicate code for pretty-printing GNAT AST

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
Remove duplicated CASE branches detected by infer; semantics is
unaffected.

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

gcc/ada/

* sprint.adb (Sprint_Node_Actual): Refactor code for generic
package and subprogram declarations.diff --git a/gcc/ada/sprint.adb b/gcc/ada/sprint.adb
--- a/gcc/ada/sprint.adb
+++ b/gcc/ada/sprint.adb
@@ -2075,7 +2075,7 @@ package body Sprint is
 Sprint_Node (Name (Node));
 Write_Char (';');
 
- when N_Generic_Package_Declaration =>
+ when N_Generic_Declaration =>
 Extra_Blank_Line;
 Write_Indent_Str_Sloc ("generic ");
 Sprint_Indented_List (Generic_Formal_Declarations (Node));
@@ -2097,14 +2097,6 @@ package body Sprint is
 Sprint_Node (Name (Node));
 Write_Char (';');
 
- when N_Generic_Subprogram_Declaration =>
-Extra_Blank_Line;
-Write_Indent_Str_Sloc ("generic ");
-Sprint_Indented_List (Generic_Formal_Declarations (Node));
-Write_Indent;
-Sprint_Node (Specification (Node));
-Write_Char (';');
-
  when N_Goto_Statement =>
 Write_Indent_Str_Sloc ("goto ");
 Sprint_Node (Name (Node));




[Ada] Remove repeated calls to Prefix in resolution of array accesses

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
Code cleanup related to recent fixes for resolution of array slices;
behaviour is unaffected.

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

gcc/ada/

* sem_res.adb (Resolve_Indexed_Component, Resolve_Slice): Rename
the local constant Name to Pref; remove repeated calls to
Prefix.diff --git a/gcc/ada/sem_res.adb b/gcc/ada/sem_res.adb
--- a/gcc/ada/sem_res.adb
+++ b/gcc/ada/sem_res.adb
@@ -9253,7 +9253,7 @@ package body Sem_Res is
---
 
procedure Resolve_Indexed_Component (N : Node_Id; Typ : Entity_Id) is
-  Name   : constant Node_Id := Prefix (N);
+  Pref   : constant Node_Id := Prefix (N);
   Expr   : Node_Id;
   Array_Type : Entity_Id := Empty; -- to prevent junk warning
   Index  : Node_Id;
@@ -9264,7 +9264,7 @@ package body Sem_Res is
  return;
   end if;
 
-  if Is_Overloaded (Name) then
+  if Is_Overloaded (Pref) then
 
  --  Use the context type to select the prefix that yields the correct
  --  component type.
@@ -9273,11 +9273,10 @@ package body Sem_Res is
 I : Interp_Index;
 It: Interp;
 I1: Interp_Index := 0;
-P : constant Node_Id := Prefix (N);
 Found : Boolean := False;
 
  begin
-Get_First_Interp (P, I, It);
+Get_First_Interp (Pref, I, It);
 while Present (It.Typ) loop
if (Is_Array_Type (It.Typ)
  and then Covers (Typ, Component_Type (It.Typ)))
@@ -9289,7 +9288,7 @@ package body Sem_Res is
  Component_Type (Designated_Type (It.Typ
then
   if Found then
- It := Disambiguate (P, I1, I, Any_Type);
+ It := Disambiguate (Pref, I1, I, Any_Type);
 
  if It = No_Interp then
 Error_Msg_N ("ambiguous prefix for indexing",  N);
@@ -9314,11 +9313,11 @@ package body Sem_Res is
  end;
 
   else
- Array_Type := Etype (Name);
+ Array_Type := Etype (Pref);
   end if;
 
-  Resolve (Name, Array_Type);
-  Array_Type := Get_Actual_Subtype_If_Available (Name);
+  Resolve (Pref, Array_Type);
+  Array_Type := Get_Actual_Subtype_If_Available (Pref);
 
   --  If the prefix's type is an access type, get to the real array type.
   --  Note: we do not apply an access check because an explicit dereference
@@ -9361,19 +9360,18 @@ package body Sem_Res is
  end loop;
   end if;
 
-  Resolve_Implicit_Dereference (Prefix (N));
+  Resolve_Implicit_Dereference (Pref);
   Analyze_Dimension (N);
 
   --  Do not generate the warning on suspicious index if we are analyzing
   --  package Ada.Tags; otherwise we will report the warning with the
   --  Prims_Ptr field of the dispatch table.
 
-  if Scope (Etype (Prefix (N))) = Standard_Standard
+  if Scope (Etype (Pref)) = Standard_Standard
 or else not
-  Is_RTU (Cunit_Entity (Get_Source_Unit (Etype (Prefix (N,
-  Ada_Tags)
+  Is_RTU (Cunit_Entity (Get_Source_Unit (Etype (Pref))), Ada_Tags)
   then
- Warn_On_Suspicious_Index (Name, First (Expressions (N)));
+ Warn_On_Suspicious_Index (Pref, First (Expressions (N)));
  Eval_Indexed_Component (N);
   end if;
 
@@ -9385,16 +9383,16 @@ package body Sem_Res is
   if Nkind (N) = N_Indexed_Component
 and then Is_Atomic_Ref_With_Address (N)
 and then not (Has_Atomic_Components (Array_Type)
-   or else (Is_Entity_Name (Prefix (N))
+   or else (Is_Entity_Name (Pref)
  and then Has_Atomic_Components
-(Entity (Prefix (N)
+(Entity (Pref
 and then not Is_Atomic (Component_Type (Array_Type))
 and then Ada_Version < Ada_2022
   then
  Error_Msg_N
-   ("??access to non-atomic component of atomic array", Prefix (N));
+   ("??access to non-atomic component of atomic array", Pref);
  Error_Msg_N
-   ("??\may cause unexpected accesses to atomic object", Prefix (N));
+   ("??\may cause unexpected accesses to atomic object", Pref);
   end if;
end Resolve_Indexed_Component;
 
@@ -11202,13 +11200,13 @@ package body Sem_Res is
 
procedure Resolve_Slice (N : Node_Id; Typ : Entity_Id) is
   Drange : constant Node_Id := Discrete_Range (N);
-  Name   : constant Node_Id := Prefix (N);
+  Pref   : constant Node_Id := Prefix (N);
   Array_Type : Entity_Id:= Empty;
   Dexpr  : Node_Id  := Empty;
   Index_Type : Entity_Id;
 
begin
-  if Is_Overloaded (Name) then
+  if Is_Overloaded (Pref) then
 
  --  Use the context type 

[Ada] Emit specific SCOs for decisions of quantified expressions

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
Coverage analysis on quantified expressions now requires always
processing their predicate expression as a decision.

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

gcc/ada/

* scos.ads: Extend the documentation.
* par_sco.adb (Process_Decisions): Emit specific SCOs for
quantified expressions.diff --git a/gcc/ada/par_sco.adb b/gcc/ada/par_sco.adb
--- a/gcc/ada/par_sco.adb
+++ b/gcc/ada/par_sco.adb
@@ -827,6 +827,14 @@ package body Par_SCO is
   return Skip;
end;
 
+when N_Quantified_Expression =>
+   declare
+  Cond : constant Node_Id := Condition (N);
+   begin
+  Process_Decisions (Cond, 'W', Pragma_Sloc);
+  return Skip;
+   end;
+
 --  All other cases, continue scan
 
 when others =>


diff --git a/gcc/ada/scos.ads b/gcc/ada/scos.ads
--- a/gcc/ada/scos.ads
+++ b/gcc/ada/scos.ads
@@ -257,7 +257,7 @@ package SCOs is
--  I   decision in IF statement or if expression
--  P   decision in pragma Assert / Check / Pre/Post_Condition
--  A[name] decision in aspect Pre/Post (aspect name optional)
-   --  W   decision in WHILE iteration scheme
+   --  W   decision in WHILE iteration scheme or quantified expression
--  X   decision in some other expression context
 
--For E, G, I, P, W, sloc is the source location of the EXIT, ENTRY, IF,




[Ada] Fix comment about expansion of slices

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
Cleanup related to handling of array accesses in CodePeer mode.

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

gcc/ada/

* checks.adb (Selected_Range_Checks): Fix style.
* exp_ch4.adb (Expand_N_Slice): Fix style and comment.
* sem_res.adb (Resolve_Indexed_Component): Fix style.diff --git a/gcc/ada/checks.adb b/gcc/ada/checks.adb
--- a/gcc/ada/checks.adb
+++ b/gcc/ada/checks.adb
@@ -10400,7 +10400,7 @@ package body Checks is
   Exptyp  : Entity_Id;
   Cond: Node_Id := Empty;
   Do_Access   : Boolean := False;
-  Wnode   : Node_Id  := Warn_Node;
+  Wnode   : Node_Id := Warn_Node;
   Ret_Result  : Check_Result := (Empty, Empty);
   Num_Checks  : Natural := 0;
 


diff --git a/gcc/ada/exp_ch4.adb b/gcc/ada/exp_ch4.adb
--- a/gcc/ada/exp_ch4.adb
+++ b/gcc/ada/exp_ch4.adb
@@ -11546,7 +11546,7 @@ package body Exp_Ch4 is
 then
Par := Parent (Par);
 
-   --  Any other case is not what we are looking for
+--  Any other case is not what we are looking for
 
 else
return False;
@@ -11582,7 +11582,7 @@ package body Exp_Ch4 is
 
   --  Local variables
 
-  Pref : constant Node_Id := Prefix (N);
+  Pref : constant Node_Id := Prefix (N);
 
--  Start of processing for Expand_N_Slice
 
@@ -11608,7 +11608,7 @@ package body Exp_Ch4 is
   --   situation correctly in the assignment statement expansion).
 
   --2. Prefix of indexed component (the slide is optimized away in this
-  --   case, see the start of Expand_N_Slice.)
+  --   case, see the start of Expand_N_Indexed_Component.)
 
   --3. Object renaming declaration, since we want the name of the
   --   slice, not the value.


diff --git a/gcc/ada/sem_res.adb b/gcc/ada/sem_res.adb
--- a/gcc/ada/sem_res.adb
+++ b/gcc/ada/sem_res.adb
@@ -9253,7 +9253,7 @@ package body Sem_Res is
---
 
procedure Resolve_Indexed_Component (N : Node_Id; Typ : Entity_Id) is
-  Name   : constant Node_Id := Prefix  (N);
+  Name   : constant Node_Id := Prefix (N);
   Expr   : Node_Id;
   Array_Type : Entity_Id := Empty; -- to prevent junk warning
   Index  : Node_Id;




[Ada] Add Ada RM description of Ada.Strings.Bounded as comments in the spec

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
To facilitate using the standard library, Ada RM description from
section A.4.4 is added to the code as comments.

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

gcc/ada/

* libgnat/a-strbou.ads: Add comments.diff --git a/gcc/ada/libgnat/a-strbou.ads b/gcc/ada/libgnat/a-strbou.ads
--- a/gcc/ada/libgnat/a-strbou.ads
+++ b/gcc/ada/libgnat/a-strbou.ads
@@ -33,6 +33,16 @@
 --  --
 --
 
+--  The language-defined package Strings.Bounded provides a generic package
+--  each of whose instances yields a private type Bounded_String and a set
+--  of operations. An object of a particular Bounded_String type represents
+--  a String whose low bound is 1 and whose length can vary conceptually
+--  between 0 and a maximum size established at the generic instantiation. The
+--  subprograms for fixed-length string handling are either overloaded directly
+--  for Bounded_String, or are modified as needed to reflect the variability in
+--  length. Additionally, since the Bounded_String type is private, appropriate
+--  constructor and selector operations are provided.
+
 with Ada.Strings.Maps; use type Ada.Strings.Maps.Character_Mapping_Function;
 with Ada.Strings.Superbounded;
 with Ada.Strings.Search;
@@ -65,11 +75,16 @@ package Ada.Strings.Bounded with SPARK_Mode is
   pragma Preelaborable_Initialization (Bounded_String);
 
   Null_Bounded_String : constant Bounded_String;
+  --  Null_Bounded_String represents the null string. If an object of type
+  --  Bounded_String is not otherwise initialized, it will be initialized
+  --  to the same value as Null_Bounded_String.
 
   subtype Length_Range is Natural range 0 .. Max_Length;
 
   function Length (Source : Bounded_String) return Length_Range with
 Global => null;
+  --  The Length function returns the length of the string represented by
+  --  Source.
 
   
   -- Conversion, Concatenation, and Selection Functions --
@@ -94,9 +109,24 @@ package Ada.Strings.Bounded with SPARK_Mode is
=>
  To_String (To_Bounded_String'Result) =
Source (Source'First .. Source'First - 1 + Max_Length));
+  --  If Source'Length <= Max_Length, then this function returns a
+  --  Bounded_String that represents Source. Otherwise, the effect
+  --  depends on the value of Drop:
+  --
+  --  * If Drop=Left, then the result is a Bounded_String that represents
+  --the string comprising the rightmost Max_Length characters of
+  --Source.
+  --
+  --  * If Drop=Right, then the result is a Bounded_String that represents
+  --the string comprising the leftmost Max_Length characters of Source.
+  --
+  --  * If Drop=Error, then Strings.Length_Error is propagated.
 
   function To_String (Source : Bounded_String) return String with
 Global => null;
+  --  To_String returns the String value with lower bound 1
+  --  represented by Source. If B is a Bounded_String, then
+  --  B = To_Bounded_String(To_String(B)).
 
   procedure Set_Bounded_String
 (Target : out Bounded_String;
@@ -119,6 +149,14 @@ package Ada.Strings.Bounded with SPARK_Mode is
  To_String (Target) =
Source (Source'First .. Source'First - 1 + Max_Length));
   pragma Ada_05 (Set_Bounded_String);
+  --  Equivalent to Target := To_Bounded_String (Source, Drop);
+
+  --  Each of the Append functions returns a Bounded_String obtained by
+  --  concatenating the string or character given or represented by one
+  --  of the parameters, with the string or character given or represented
+  --  by the other parameter, and applying To_Bounded_String to the
+  --  concatenation result string, with Drop as provided to the Append
+  --  function.
 
   function Append
 (Left  : Bounded_String;
@@ -324,6 +362,10 @@ package Ada.Strings.Bounded with SPARK_Mode is
Slice (Right, 1, Max_Length - 1)
and then Element (Append'Result, 1) = Left);
 
+  --  Each of the procedures Append(Source, New_Item, Drop) has the same
+  --  effect as the corresponding assignment
+  --  Source := Append(Source, New_Item, Drop).
+
   procedure Append
 (Source   : in out Bounded_String;
  New_Item : Bounded_String;
@@ -455,6 +497,9 @@ package Ada.Strings.Bounded with SPARK_Mode is
Slice (Source'Old, 2, Max_Length)
and then Element (Source, Max_Length) = New_Item);
 
+  --  Each of the "&" functions has the same effect as the corresponding
+  --  Append function, with Error as the Drop parameter.
+
   function "&"
 (Left  : Bounded_String;
  Right : Bounded_String) return 

[Ada] Fix for a static Leading_Part attribute raising constraint error

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
Attribute Leading_Part, when its second parameter is zero or negative,
is now raising a constraint error when evaluated at compilation time
just like it did when evaluated at run time.

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

gcc/ada/

* sem_attr.adb (Eval_Attribute): Evaluation of attribute
Leading_Part with illegal second parameter is now similar to
evaluation of Remainder with its second parameter being zero.diff --git a/gcc/ada/sem_attr.adb b/gcc/ada/sem_attr.adb
--- a/gcc/ada/sem_attr.adb
+++ b/gcc/ada/sem_attr.adb
@@ -9135,12 +9135,26 @@ package body Sem_Attr is
   -- Leading_Part --
   --
 
-  when Attribute_Leading_Part =>
+  when Attribute_Leading_Part => Leading_Part : declare
+ Radix_Digits : constant Uint := Expr_Value (E2);
+
+  begin
+ if UI_Le (Radix_Digits, Uint_0) then
+Apply_Compile_Time_Constraint_Error
+  (N, "Radix_Digits in Leading_Part is zero or negative",
+   CE_Explicit_Raise,
+   Warn => not Static);
+
+Check_Expressions;
+return;
+ end if;
+
  Fold_Ureal
(N,
 Eval_Fat.Leading_Part
-  (P_Base_Type, Expr_Value_R (E1), Expr_Value (E2)),
+  (P_Base_Type, Expr_Value_R (E1), Radix_Digits),
 Static);
+  end Leading_Part;
 
   
   -- Length --




[Ada] Fix resolution of Declare_Expressions involving transient scopes

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
This patch modifies the resolution of Declare_Expressions to avoid the
use of a fake scope to perform name capture in the expression, because
such a scope (needed to analyze the declarations of the construct)
conflicts with the transient scopes that may be generated by the
presence of calls in the expression that may require finalization.

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

gcc/ada/

* sem_res.adb (Resolve_Declare_Expression): Use tree traversals
to perform name capture of local entities in the expression of
the construct.
* exp_util.adb (Possible_Side_Effects_In_SPARK): Do not apply to
the prefix of an attribute reference Reduce when that prefix is
an aggregate, because it will be expanded into a loop, and has
no identifiable type.diff --git a/gcc/ada/exp_util.adb b/gcc/ada/exp_util.adb
--- a/gcc/ada/exp_util.adb
+++ b/gcc/ada/exp_util.adb
@@ -11737,10 +11737,15 @@ package body Exp_Util is
   --  case and it is better not to make an additional one for the attribute
   --  itself, because the return type of many of them is universal integer,
   --  which is a very large type for a temporary.
+  --  The prefix of an attribute reference Reduce may be syntactically an
+  --  aggregate, but will be expanded into a loop, so no need to remove
+  --  side-effects.
 
   if Nkind (Exp) = N_Attribute_Reference
 and then Side_Effect_Free_Attribute (Attribute_Name (Exp))
 and then Side_Effect_Free (Expressions (Exp), Name_Req, Variable_Ref)
+and then (Attribute_Name (Exp) /= Name_Reduce
+   or else Nkind (Prefix (Exp)) /= N_Aggregate)
 and then not Is_Name_Reference (Prefix (Exp))
   then
  Remove_Side_Effects (Prefix (Exp), Name_Req, Variable_Ref);


diff --git a/gcc/ada/sem_res.adb b/gcc/ada/sem_res.adb
--- a/gcc/ada/sem_res.adb
+++ b/gcc/ada/sem_res.adb
@@ -7487,66 +7487,76 @@ package body Sem_Res is
  (N   : Node_Id;
   Typ : Entity_Id)
is
-  Decl : Node_Id;
-  Need_Transient_Scope : Boolean := False;
-   begin
-  --  Install the scope created for local declarations, if
-  --  any. The syntax allows a Declare_Expression with no
-  --  declarations, in analogy with block statements.
-  --  Note that that scope has no explicit declaration, but
-  --  appears as the scope of all entities declared therein.
+  Expr : constant Node_Id := Expression (N);
 
-  Decl := First (Actions (N));
-  while Present (Decl) loop
- exit when Nkind (Decl)
- in N_Object_Declaration | N_Object_Renaming_Declaration;
- Next (Decl);
-  end loop;
+  Decl  : Node_Id;
+  Local : Entity_Id := Empty;
 
-  if Present (Decl) then
+  function Replace_Local (N  : Node_Id) return Traverse_Result;
+  --  Use a tree traversal to replace each ocurrence of the name of
+  --  a local object declared in the construct, with the corresponding
+  --  entity. This replaces the usual way to perform name capture by
+  --  visibility, because it is not possible to place on the scope
+  --  stack the fake scope created for the analysis of the local
+  --  declarations; such a scope conflicts with the transient scopes
+  --  that may be generated if the expression includes function calls
+  --  requiring finalization.
 
- --  Need to establish a transient scope in case Expression (N)
- --  requires actions to be wrapped.
+  ---
+  -- Replace_Local --
+  ---
 
- declare
-Node : Node_Id;
- begin
-Node := First (Actions (N));
-while Present (Node) loop
-   if Nkind (Node) = N_Object_Declaration
- and then Requires_Transient_Scope
-(Etype (Defining_Identifier (Node)))
-   then
-  Need_Transient_Scope := True;
-  exit;
-   end if;
+  function Replace_Local (N  : Node_Id) return Traverse_Result is
+  begin
+ --  The identifier may be the prefix of a selected component,
+ --  but not a selector name, because the local entities do not
+ --  have a scope that can be named: a selected component whose
+ --  selector is a homonym of a local entity must denote some
+ --  global entity.
+
+ if Nkind (N) = N_Identifier
+   and then Chars (N) = Chars (Local)
+   and then No (Entity (N))
+   and then
+ (Nkind (Parent (N)) /= N_Selected_Component
+   or else N = Prefix (Parent (N)))
+ then
+Set_Entity (N, Local);
+Set_Etype (N, Etype (Local));
+ end if;
 
-   Next (Node);
-end loop;
- end;
+ return OK;
+  end Replace_Local;
 
- if Need_Transient_Scope then
-   

[Ada] Declaration_Node for Itypes returns Empty or declaration

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
This patch changes Declaration_Node for Itypes so that it either returns
Empty or returns a proper declaration node. If the tree structure is
that of a normal type or subtype declaration, so the parent of the Itype
is that declaration, then we return the declaration. Otherwise, we
return Empty rather than returning some more-or-less arbitrary node.

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

gcc/ada/

* einfo.ads (Declaration_Node): Document that Declaration_Node
for Itypes.
* einfo-utils.adb (Declaration_Node): Make it return Empty for
Itypes, or a proper type or subtype declaration.
* gen_il-gen.adb: Minor comment improvement.diff --git a/gcc/ada/einfo-utils.adb b/gcc/ada/einfo-utils.adb
--- a/gcc/ada/einfo-utils.adb
+++ b/gcc/ada/einfo-utils.adb
@@ -655,16 +655,21 @@ package body Einfo.Utils is
  P := Parent (Id);
   end if;
 
+  while Nkind (P) in N_Selected_Component | N_Expanded_Name
+or else (Nkind (P) = N_Defining_Program_Unit_Name
+   and then Is_Child_Unit (Id))
   loop
- if Nkind (P) in N_Selected_Component | N_Expanded_Name
-   or else (Nkind (P) = N_Defining_Program_Unit_Name
- and then Is_Child_Unit (Id))
- then
-P := Parent (P);
- else
-return P;
- end if;
+ P := Parent (P);
   end loop;
+
+  if Is_Itype (Id)
+and then Nkind (P) not in
+  N_Full_Type_Declaration | N_Subtype_Declaration
+  then
+ P := Empty;
+  end if;
+
+  return P;
end Declaration_Node;
 
-


diff --git a/gcc/ada/einfo.ads b/gcc/ada/einfo.ads
--- a/gcc/ada/einfo.ads
+++ b/gcc/ada/einfo.ads
@@ -829,7 +829,9 @@ package Einfo is
 --   a private type, then we obtain the declaration node denoted by the
 --   full type, i.e. the full type declaration node. Also note that for
 --   subprograms, this returns the {function,procedure}_specification, not
---   the subprogram_declaration.
+--   the subprogram_declaration. If the parent of an Itype is a type or
+--   subtype declaration, we return the declaration node as for any other
+--   type. For other Itypes, we return Empty.
 
 --Default_Aspect_Component_Value [base type only]
 --   Defined in array types. Holds the static value specified in a


diff --git a/gcc/ada/gen_il-gen.adb b/gcc/ada/gen_il-gen.adb
--- a/gcc/ada/gen_il-gen.adb
+++ b/gcc/ada/gen_il-gen.adb
@@ -710,6 +710,8 @@ package body Gen_IL.Gen is
   Type_Table (T).Last  := T;
   Add_Concrete_Descendant_To_Ancestors
 (Type_Table (T).Parent, T);
+  --  Parent cannot be No_Type here, because T is a concrete
+  --  type, and therefore not a root type.
 
when Abstract_Type =>
   declare




[Ada] Fix handling of slices with subtype names

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
When resolving a slice with subtype name we created a shallow copy of
the type range and applied range checks to this copy. This was wrong,
because when applying checks we were modifying the type declaration
itself (and the type could be anything, for example the predefined
Integer type).

Instead of a shallow copy, we now create a subtype'range attribute,
analyze it and apply checks, but insert those checks into the slice
range expression node.

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

gcc/ada/

* sem_res.adb (Resolve_Slice): Fix application of range checks
to slice range given as a subtype name.diff --git a/gcc/ada/sem_res.adb b/gcc/ada/sem_res.adb
--- a/gcc/ada/sem_res.adb
+++ b/gcc/ada/sem_res.adb
@@ -11347,18 +11347,19 @@ package body Sem_Res is
   then
  null;
 
-  --  The discrete_range is specified by a subtype indication. Create a
-  --  shallow copy and inherit the type, parent and source location from
-  --  the discrete_range. This ensures that the range check is inserted
-  --  relative to the slice and that the runtime exception points to the
-  --  proper construct.
+  --  The discrete_range is specified by a subtype name. Create an
+  --  equivalent range attribute, apply checks to this attribute, but
+  --  insert them into the range expression of the slice itself.
 
   elsif Is_Entity_Name (Drange) then
- Dexpr := New_Copy (Scalar_Range (Entity (Drange)));
+ Dexpr :=
+   Make_Attribute_Reference
+ (Sloc (Drange),
+  Prefix =>
+New_Occurrence_Of (Entity (Drange), Sloc (Drange)),
+  Attribute_Name => Name_Range);
 
- Set_Etype  (Dexpr, Etype  (Drange));
- Set_Parent (Dexpr, Parent (Drange));
- Set_Sloc   (Dexpr, Sloc   (Drange));
+ Analyze_And_Resolve (Dexpr, Etype  (Drange));
 
   elsif Nkind (Drange) = N_Subtype_Indication then
  Dexpr := Range_Expression (Constraint (Drange));
@@ -11379,7 +11380,7 @@ package body Sem_Res is
   end if;
 
   if Present (Dexpr) then
- Apply_Range_Check (Dexpr, Index_Type);
+ Apply_Range_Check (Dexpr, Index_Type, Insert_Node => Drange);
   end if;
 
   Set_Slice_Subtype (N);




[Ada] Fix missing check on slice with a subtype indication

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
A slice range can be specified either by a subtype name, regular range,
or subtype indication with constraints. The compiler missed the last
case.

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

gcc/ada/

* sem_res.adb (Resolve_Slice): Handle range given as a subtype
indication.diff --git a/gcc/ada/sem_res.adb b/gcc/ada/sem_res.adb
--- a/gcc/ada/sem_res.adb
+++ b/gcc/ada/sem_res.adb
@@ -11360,7 +11360,11 @@ package body Sem_Res is
  Set_Parent (Dexpr, Parent (Drange));
  Set_Sloc   (Dexpr, Sloc   (Drange));
 
-  --  The discrete_range is a regular range. Resolve the bounds and remove
+  elsif Nkind (Drange) = N_Subtype_Indication then
+ Dexpr := Range_Expression (Constraint (Drange));
+
+  --  The discrete_range is a regular range (or a range attribute, which
+  --  will be resolved into a regular range). Resolve the bounds and remove
   --  their side effects.
 
   else




[Ada] Fix handling of 'Image acting as a prefix of a slice in CodePeer

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
In CodePeer mode attribute Image is not expanded and has its Etype set
as the unconstrained String type. When this attribute appears as a
prefix of an indexed component, we get a check; when it appears as a
prefix of a slice, we don't get a check.

For indexed components, the check effectively comes from
Apply_Scalar_Range_Check, where prefixes of unconstrained array types
are handled specifically. For slices, the seemingly similar routine
Apply_Range_Check doesn't specifically handle such prefixes. Instead, we
need to give this routine a constrained subtype, just like we do when
slice is prefixed with a call to a function that returns an
unconstrained array.

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

gcc/ada/

* sem_res.adb (Resolve_Slice): Add custom handling of attribute
Image and similar in CodePeer mode. This complements the
existing custom handling of these attributes in
Expand_N_Attribute_Reference.diff --git a/gcc/ada/sem_res.adb b/gcc/ada/sem_res.adb
--- a/gcc/ada/sem_res.adb
+++ b/gcc/ada/sem_res.adb
@@ -11275,10 +11275,20 @@ package body Sem_Res is
 end;
  end if;
 
+  --  In CodePeer mode the attribute Image is not expanded, so when it
+  --  acts as a prefix of a slice, we handle it like a call to function
+  --  returning an unconstrained string. Same for the Wide variants of
+  --  attribute Image.
+
   elsif Is_Entity_Name (Name)
 or else Nkind (Name) = N_Explicit_Dereference
 or else (Nkind (Name) = N_Function_Call
   and then not Is_Constrained (Etype (Name)))
+or else (CodePeer_Mode
+  and then Nkind (Name) = N_Attribute_Reference
+  and then Attribute_Name (Name) in Name_Image
+  | Name_Wide_Image
+  | Name_Wide_Wide_Image)
   then
  Array_Type := Get_Actual_Subtype (Name);
 




[Ada] Spurious non-variable error on implicitly dereferenced in-mode formal

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
This patch corrects an issue in the compiler whereby taking 'Access of a
protected subprogram based on an implicitly dereferenced protected
object causes a spurious non-variable error when such an object is an
in-mode access type formal.

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

gcc/ada/

* sem_util.adb (Is_Variable): Add check for implicitly
dereferenced access types
(Is_Dependent_Component_Of_Mutable_Object): Set Prefix_Type when
not specified.diff --git a/gcc/ada/sem_util.adb b/gcc/ada/sem_util.adb
--- a/gcc/ada/sem_util.adb
+++ b/gcc/ada/sem_util.adb
@@ -16868,6 +16868,15 @@ package body Sem_Util is
 end if;
 
 if Is_Entity_Name (P) then
+   --  The Etype may not be set on P (which is wrong) in certain
+   --  corner cases involving the deprecated front-end inlining of
+   --  subprograms (via -gnatN), so use the Etype set on the
+   --  the entity for these instances since we know it is present.
+
+   if No (Prefix_Type) then
+  Prefix_Type := Etype (Entity (P));
+   end if;
+
if Ekind (Entity (P)) = E_Generic_In_Out_Parameter then
   Prefix_Type := Base_Type (Prefix_Type);
end if;
@@ -21145,6 +21154,9 @@ package body Sem_Util is
-- Is_Variable --
-
 
+   --  Should Is_Variable be refactored to better handle dereferences and
+   --  technical debt ???
+
function Is_Variable
  (N : Node_Id;
   Use_Original_Node : Boolean := True) return Boolean
@@ -21313,6 +21325,10 @@ package body Sem_Util is
 and then Nkind (Parent (E)) /= N_Exception_Handler)
   or else (K = E_Component
 and then not In_Protected_Function (E))
+  or else (Present (Etype (E))
+and then Is_Access_Object_Type (Etype (E))
+and then Is_Access_Variable (Etype (E))
+and then Is_Dereferenced (N))
   or else K = E_Out_Parameter
   or else K = E_In_Out_Parameter
   or else K = E_Generic_In_Out_Parameter




[Ada] Adjust documentation of -fdump-ada-spec in GNAT UG

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
This explicitly documents the behavior for /include/-ending paths and also
updates other parts of the description.

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

gcc/ada/

* doc/gnat_ugn/the_gnat_compilation_model.rst (Binding generation):
Document specific behavior for /include/-ending paths and update.
* gnat_ugn.texi: Regenerate.diff --git a/gcc/ada/doc/gnat_ugn/the_gnat_compilation_model.rst b/gcc/ada/doc/gnat_ugn/the_gnat_compilation_model.rst
--- a/gcc/ada/doc/gnat_ugn/the_gnat_compilation_model.rst
+++ b/gcc/ada/doc/gnat_ugn/the_gnat_compilation_model.rst
@@ -4526,8 +4526,8 @@ Some of the known limitations include:
   constants. Function macros (macros with arguments) are partially translated
   as comments, to be completed manually if needed.
 * some extensions (e.g. vector types) are not supported
-* pointers to pointers or complex structures are mapped to System.Address
-* identifiers with identical name (except casing) will generate compilation
+* pointers to pointers are mapped to System.Address
+* identifiers with identical name (except casing) may generate compilation
   errors (e.g. ``shm_get`` vs ``SHM_GET``).
 
 The code is generated using Ada 2012 syntax, which makes it easier to interface
@@ -4546,14 +4546,17 @@ header files needed by these files transitively. For example:
 
 .. code-block:: sh
 
-  $ g++ -c -fdump-ada-spec -C /usr/include/time.h
+  $ gcc -c -fdump-ada-spec -C /usr/include/time.h
   $ gcc -c *.ads
 
 will generate, under GNU/Linux, the following files: :file:`time_h.ads`,
 :file:`bits_time_h.ads`, :file:`stddef_h.ads`, :file:`bits_types_h.ads` which
 correspond to the files :file:`/usr/include/time.h`,
-:file:`/usr/include/bits/time.h`, etc..., and will then compile these Ada specs
-in Ada 2005 mode.
+:file:`/usr/include/bits/time.h`, etc..., and then compile these Ada specs.
+That is to say, the name of the Ada specs is in keeping with the relative path
+under :file:`/usr/include/` of the header files. This behavior is specific to
+paths ending with :file:`/include/`; in all the other cases, the name of the
+Ada specs is derived from the simple name of the header files instead.
 
 The :switch:`-C` switch tells ``gcc`` to extract comments from headers,
 and will attempt to generate corresponding Ada comments.
@@ -4564,39 +4567,8 @@ can use instead the :switch:`-fdump-ada-spec-slim` switch.
 You can optionally specify a parent unit, of which all generated units will
 be children, using :switch:`-fada-spec-parent={unit}`.
 
-Note that we recommend when possible to use the *g++* driver to
-generate bindings, even for most C headers, since this will in general
-generate better Ada specs. For generating bindings for C++ headers, it is
-mandatory to use the *g++* command, or *gcc -x c++* which
-is equivalent in this case. If *g++* cannot work on your C headers
-because of incompatibilities between C and C++, then you can fallback to
-``gcc`` instead.
-
-For an example of better bindings generated from the C++ front-end,
-the name of the parameters (when available) are actually ignored by the C
-front-end. Consider the following C header:
-
-.. code-block:: c
-
- extern void foo (int variable);
-
-with the C front-end, ``variable`` is ignored, and the above is handled as:
-
-.. code-block:: c
-
- extern void foo (int);
-
-generating a generic:
-
-.. code-block:: ada
-
- procedure foo (param1 : int);
-
-with the C++ front-end, the name is available, and we generate:
-
-.. code-block:: ada
-
- procedure foo (variable : int);
+The simple ``gcc```-based command works only for C headers. For C++ headers
+you need to use either the ``g++`` command or the combination ``gcc -x c++```.
 
 In some cases, the generated bindings will be more complete or more meaningful
 when defining some macros, which you can do via the :switch:`-D` switch. This
@@ -4604,7 +4576,7 @@ is for example the case with :file:`Xlib.h` under GNU/Linux:
 
 .. code-block:: sh
 
-  $ g++ -c -fdump-ada-spec -DXLIB_ILLEGAL_ACCESS -C /usr/include/X11/Xlib.h
+  $ gcc -c -fdump-ada-spec -DXLIB_ILLEGAL_ACCESS -C /usr/include/X11/Xlib.h
 
 The above will generate more complete bindings than a straight call without
 the :switch:`-DXLIB_ILLEGAL_ACCESS` switch.
@@ -4626,7 +4598,7 @@ and then generate Ada bindings from this file:
 
 .. code-block:: sh
 
-  $ g++ -c -fdump-ada-spec readline1.h
+  $ gcc -c -fdump-ada-spec readline1.h
 
 
 .. _Generating_bindings_for_C++_headers:


diff --git a/gcc/ada/gnat_ugn.texi b/gcc/ada/gnat_ugn.texi
--- a/gcc/ada/gnat_ugn.texi
+++ b/gcc/ada/gnat_ugn.texi
@@ -21,7 +21,7 @@
 
 @copying
 @quotation
-GNAT User's Guide for Native Platforms , Aug 03, 2021
+GNAT User's Guide for Native Platforms , Sep 28, 2021
 
 AdaCore
 
@@ -6421,10 +6421,10 @@ as comments, to be completed manually if needed.
 some extensions (e.g. vector types) are not supported
 
 @item 
-pointers to pointers or complex structures are mapped to 

[Ada] PR ada/102073

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
Add missing return statements in socket.c as flagged by cppcheck.

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

gcc/ada/

PR ada/102073
* socket.c (__gnat_gethostbyaddr, __gnat_inet_pton): Add missing
return statements.diff --git a/gcc/ada/socket.c b/gcc/ada/socket.c
--- a/gcc/ada/socket.c
+++ b/gcc/ada/socket.c
@@ -314,6 +314,7 @@ __gnat_gethostbyaddr (const char *addr, int len, int type,
   ret->h_addrtype  = AF_INET;
   ret->h_length= 4;
   ret->h_addr_list = _h_addr_list;
+  return 0;
 }
 
 int
@@ -587,6 +588,9 @@ __gnat_inet_pton (int af, const char *src, void *dst) {
 *(in_addr_t *)dst = addr;
   }
   return rc;
+
+#else
+  return -1;
 #endif
 }
 #endif




[Ada] Crash on allocator in alternative accessibility modes

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
This patch corrects an issue in the compiler whereby the level for
allocated objects of anonymous access types was calculated incorrectly.

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

gcc/ada/

* sem_util.adb (Function_Or_Allocator_Level): Properly handle
direct function calls in the default alternative accessibility
checking mode.diff --git a/gcc/ada/sem_util.adb b/gcc/ada/sem_util.adb
--- a/gcc/ada/sem_util.adb
+++ b/gcc/ada/sem_util.adb
@@ -391,8 +391,7 @@ package body Sem_Util is
and then (Is_Static_Coextension (N)
   or else Is_Dynamic_Coextension (N))
  then
-return Make_Level_Literal
- (Scope_Depth (Standard_Standard));
+return Make_Level_Literal (Scope_Depth (Standard_Standard));
  end if;
 
  --  Named access types have a designated level
@@ -416,9 +415,14 @@ package body Sem_Util is
if Debug_Flag_Underscore_B then
   return Make_Level_Literal (Typ_Access_Level (Etype (N)));
 
-   --  Otherwise the level is that of the subprogram
+   --  For function calls the level is that of the subprogram,
+   --  otherwise (for allocators etc.) we get the level of the
+   --  corresponding anonymous access type which is calculated
+   --  through the normal path of execution.
 
-   else
+   elsif Nkind (N) = N_Function_Call
+ and then Nkind (Name (N)) /= N_Explicit_Dereference
+   then
   return Make_Level_Literal
(Subprogram_Access_Level (Entity (Name (N;
end if;
@@ -29287,7 +29291,7 @@ package body Sem_Util is
(Designated_Type (Btyp), Allow_Alt_Model);
end if;
 
-   --  When an anonymous access type's Assoc_Ent is specifiedi,
+   --  When an anonymous access type's Assoc_Ent is specified,
--  calculate the result based on the general accessibility
--  level routine.
 




[Ada] Spurious accessibility error on renamed expression

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
This patch corrects an issue in the compiler whereby a renaming of a
subprogram (as opposed to an object) may cause accessibility levels to
be incorrectly calculated on such renamings -- leading to spurious
accessibility errors at run time.

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

gcc/ada/

* sem_util.adb (Accessibility_Level): Add a case to handle
renamed subprograms in addition to renamed objects.diff --git a/gcc/ada/sem_util.adb b/gcc/ada/sem_util.adb
--- a/gcc/ada/sem_util.adb
+++ b/gcc/ada/sem_util.adb
@@ -713,15 +713,25 @@ package body Sem_Util is
 
return Make_Level_Literal (Typ_Access_Level (E) + 1);
 
---  Move up the renamed entity if it came from source since
---  expansion may have created a dummy renaming under certain
---  circumstances.
+--  Move up the renamed entity or object if it came from source
+--  since expansion may have created a dummy renaming under
+--  certain circumstances.
+
+--  Note: We check if the original node of the renaming comes
+--  from source because the node may have been rewritten.
 
 elsif Present (Renamed_Object (E))
-  and then Comes_From_Source (Renamed_Object (E))
+  and then Comes_From_Source (Original_Node (Renamed_Object (E)))
 then
return Accessibility_Level (Renamed_Object (E));
 
+--  Move up renamed entities
+
+elsif Present (Renamed_Entity (E))
+  and then Comes_From_Source (Original_Node (Renamed_Entity (E)))
+then
+   return Accessibility_Level (Renamed_Entity (E));
+
 --  Named access types get their level from their associated type
 
 elsif Is_Named_Access_Type (Etype (E)) then




[Ada] VxWorks inconsistent use of return type Fixup

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
Fix STATUS and int return types for Vxworks6 in legacy.

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

gcc/ada/

* libgnarl/s-osinte__vxworks.ads (tlsKeyCreate): Return int.
* libgnarl/s-tpopsp__vxworks-rtp.adb (ERROR): Declare from
System.VxWorks.Ext.ERROR.
(Initialize): Declare IERR. Assert it.
* libgnarl/s-tpopsp__vxworks.adb (ERROR): Declare from
System.VxWorks.Ext.ERROR.
(Is_Valid_Task): Declare IERR. Test return.
* libgnarl/s-vxwext__kernel.adb (semDelete): Return STATUS.diff --git a/gcc/ada/libgnarl/s-osinte__vxworks.ads b/gcc/ada/libgnarl/s-osinte__vxworks.ads
--- a/gcc/ada/libgnarl/s-osinte__vxworks.ads
+++ b/gcc/ada/libgnarl/s-osinte__vxworks.ads
@@ -340,7 +340,7 @@ package System.OS_Interface is
--  Can only be called from the VxWorks 6 run-time libary that supports
--  tlsLib, and not by the VxWorks 6.6 SMP library
 
-   function tlsKeyCreate return STATUS;
+   function tlsKeyCreate return int;
pragma Import (C, tlsKeyCreate, "tlsKeyCreate");
 
function tlsValueGet (key : int) return System.Address;


diff --git a/gcc/ada/libgnarl/s-tpopsp__vxworks-rtp.adb b/gcc/ada/libgnarl/s-tpopsp__vxworks-rtp.adb
--- a/gcc/ada/libgnarl/s-tpopsp__vxworks-rtp.adb
+++ b/gcc/ada/libgnarl/s-tpopsp__vxworks-rtp.adb
@@ -35,6 +35,8 @@
 separate (System.Task_Primitives.Operations)
 package body Specific is
 
+   ERROR : constant STATUS := System.VxWorks.Ext.ERROR;
+
ATCB_Key : int := 0;
--  Key used to find the Ada Task_Id associated with a thread
 
@@ -43,9 +45,10 @@ package body Specific is

 
procedure Initialize is
+  IERR : constant := -1;
begin
   ATCB_Key := tlsKeyCreate;
-  pragma Assert (ATCB_Key /= ERROR);
+  pragma Assert (ATCB_Key /= IERR);
end Initialize;
 
---


diff --git a/gcc/ada/libgnarl/s-tpopsp__vxworks.adb b/gcc/ada/libgnarl/s-tpopsp__vxworks.adb
--- a/gcc/ada/libgnarl/s-tpopsp__vxworks.adb
+++ b/gcc/ada/libgnarl/s-tpopsp__vxworks.adb
@@ -35,6 +35,8 @@
 separate (System.Task_Primitives.Operations)
 package body Specific is
 
+   ERROR  : constant STATUS := System.VxWorks.Ext.ERROR;
+
ATCB_Key : aliased System.Address := System.Null_Address;
--  Key used to find the Ada Task_Id associated with a thread
 
@@ -70,8 +72,9 @@ package body Specific is
---
 
function Is_Valid_Task return Boolean is
+  IERR : constant := -1;
begin
-  return taskVarGet (taskIdSelf, ATCB_Key'Access) /= ERROR;
+  return taskVarGet (taskIdSelf, ATCB_Key'Access) /= IERR;
end Is_Valid_Task;
 
-


diff --git a/gcc/ada/libgnarl/s-vxwext__kernel.adb b/gcc/ada/libgnarl/s-vxwext__kernel.adb
--- a/gcc/ada/libgnarl/s-vxwext__kernel.adb
+++ b/gcc/ada/libgnarl/s-vxwext__kernel.adb
@@ -59,7 +59,7 @@ package body System.VxWorks.Ext is
---
 
function semDelete (Sem : SEM_ID) return STATUS is
-  function Os_Sem_Delete (Sem : SEM_ID) return int;
+  function Os_Sem_Delete (Sem : SEM_ID) return STATUS;
   pragma Import (C, Os_Sem_Delete, "semDelete");
begin
   return Os_Sem_Delete (Sem);




[Ada] Emit debugging information for TSD object

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
This makes the compiler emit debugging information for the Type-Specific
Data object generated for tagged types, so as to make sure that debugging
information is emitted for its type in ell circumstances.

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

gcc/ada/

* exp_disp.adb (Make_DT): Copy the Needs_Debug_Info flag from the
type onto the TSD object.diff --git a/gcc/ada/exp_disp.adb b/gcc/ada/exp_disp.adb
--- a/gcc/ada/exp_disp.adb
+++ b/gcc/ada/exp_disp.adb
@@ -5703,6 +5703,11 @@ package body Exp_Disp is
 
   Set_Is_True_Constant (TSD, Building_Static_DT (Typ));
 
+  --  The debugging information for type Ada.Tags.Type_Specific_Data is
+  --  needed by the debugger in order to display values of tagged types.
+
+  Set_Needs_Debug_Info (TSD, Needs_Debug_Info (Typ));
+
   --  Initialize or declare the dispatch table object
 
   if not Has_DT (Typ) then




[Ada] Fix compiler internal error

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
In some cases with -gnat2022 enabled, an enabled postcondition
containing a quantified expression containing an Old attribute reference
could result in an internal error during compilation. Fix this.

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

gcc/ada/

* sem_util.adb (Is_Repeatedly_Evaluated): Handle the case of an
Old attribute reference that occurs within what was originally a
quantified expression but which expansion has transformed into
an Expression_With_Actions.diff --git a/gcc/ada/sem_util.adb b/gcc/ada/sem_util.adb
--- a/gcc/ada/sem_util.adb
+++ b/gcc/ada/sem_util.adb
@@ -31510,7 +31510,12 @@ package body Sem_Util is
   --quantified_expression.
 
   if Nkind (Par) = N_Quantified_Expression
- and then Trailer = Condition (Par)
+and then Trailer = Condition (Par)
+  then
+ return True;
+  elsif Nkind (Par) = N_Expression_With_Actions
+and then
+  Nkind (Original_Node (Par)) = N_Quantified_Expression
   then
  return True;
   end if;




[Ada] Improve checking for invalid index values when accessing array elements

2021-10-04 Thread Pierre-Marie de Rodat via Gcc-patches
Two improvements to the previous change on this topic:

1) Add a guard to prevent a call to Number_Of_Dimensions that would pass
   in a non-array type. This is needed in error cases (see ACATS test
   B95094C).

2) Do not generate the new validity checks in the case where the
   index type in question has a specified Default_Initial_Value aspect
   (which rules out the possibility that an object is invalid because it
   is uninitialized).

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

gcc/ada/

* exp_ch4.adb (Expand_N_Indexed_Component): The two improvements
described above.diff --git a/gcc/ada/exp_ch4.adb b/gcc/ada/exp_ch4.adb
--- a/gcc/ada/exp_ch4.adb
+++ b/gcc/ada/exp_ch4.adb
@@ -7255,11 +7255,15 @@ package body Exp_Ch4 is
   --  Generate index and validity checks
 
   declare
- Dims_Checked : Dimension_Set (Dimensions => Number_Dimensions (T));
+ Dims_Checked : Dimension_Set (Dimensions =>
+ (if Is_Array_Type (T)
+  then Number_Dimensions (T)
+  else 1));
  --  Dims_Checked is used to avoid generating two checks (one in
  --  Generate_Index_Checks, one in Apply_Subscript_Validity_Checks)
  --  for the same index value in cases where the index check eliminates
- --  the need for the validity check.
+ --  the need for the validity check. The Is_Array_Type test avoids
+ --  cascading errors.
 
   begin
  Generate_Index_Checks (N, Checks_Generated => Dims_Checked);
@@ -7284,6 +7288,27 @@ package body Exp_Ch4 is
--  If Validity_Check_Subscripts is True then we need to
--  ensure validity, so we adjust Dims_Checked accordingly.
Dims_Checked.Elements := (others => False);
+
+elsif Is_Array_Type (T) then
+   --  We are only adding extra validity checks here to
+   --  deal with uninitialized variables (but this includes
+   --  assigning one uninitialized variable to another). Other
+   --  ways of producing invalid objects imply erroneousness, so
+   --  the compiler can do whatever it wants for those cases.
+   --  If an index type has the Default_Value aspect specified,
+   --  then we don't have to worry about the possibility of an
+   --  uninitialized variable, so no need for these extra
+   --  validity checks.
+
+   declare
+  Idx : Node_Id := First_Index (T);
+   begin
+  for No_Check_Needed of Dims_Checked.Elements loop
+ No_Check_Needed := No_Check_Needed
+   or else Has_Aspect (Etype (Idx), Aspect_Default_Value);
+ Next_Index (Idx);
+  end loop;
+   end;
 end if;
 
 Apply_Subscript_Validity_Checks




Re: [PATCH] libiberty: d-demangle: remove parenthesis where it is not needed

2021-10-04 Thread Andreas Schwab
On Sep 29 2021, Luís Ferreira wrote:

> diff --git a/libiberty/d-demangle.c b/libiberty/d-demangle.c
> index 3adf7b562d1..a05e72d8efe 100644
> --- a/libiberty/d-demangle.c
> +++ b/libiberty/d-demangle.c
> @@ -253,15 +253,15 @@ dlang_hexdigit (const char *mangled, char *ret)
>  
>c = mangled[0];
>if (!ISDIGIT (c))
> -(*ret) = (c - (ISUPPER (c) ? 'A' : 'a') + 10);
> +*ret = (c - (ISUPPER (c) ? 'A' : 'a') + 10);

The outer pair of parens around rhs is also redundant.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."


Re: [PATCH] Remove static marker for range in alloca pass.

2021-10-04 Thread Richard Biener via Gcc-patches
On Mon, Oct 4, 2021 at 8:55 AM Aldy Hernandez via Gcc-patches
 wrote:
>
> The m_ranges[] field in int_range are trees, so they live in GC
> space.  Since invalid_range is static, it must be marked with GTY
> magic.  However, calculating invalid_range is not particularly slow,
> or on a critical path, so we can just put it in local scope and
> recalculate every time.
>
> Tested on x86-64 Linux.
>
> Since this is more of a GC thing than a range thing, I'd like a nod from
> a global reviewer.
>
> OK?

OK, but can we somehow make int_range<>::intersect work
with non-tree as well?  That is, can we somehow make this
operation w/o constructing temporary trees?

Thanks,
Richard.

> gcc/ChangeLog:
>
> PR tree-optimization/102560
> * gimple-ssa-warn-alloca.c (alloca_call_type): Remove static
> marker for invalid_range.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/Walloca2.C: New test.
> ---
>  gcc/gimple-ssa-warn-alloca.c| 7 +++
>  gcc/testsuite/g++.dg/Walloca2.C | 6 ++
>  2 files changed, 9 insertions(+), 4 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/Walloca2.C
>
> diff --git a/gcc/gimple-ssa-warn-alloca.c b/gcc/gimple-ssa-warn-alloca.c
> index 4fc7125d378..d59cea8d4ec 100644
> --- a/gcc/gimple-ssa-warn-alloca.c
> +++ b/gcc/gimple-ssa-warn-alloca.c
> @@ -221,10 +221,9 @@ alloca_call_type (gimple *stmt, bool is_vla)
>&& !r.varying_p ())
>  {
>// The invalid bits are anything outside of [0, MAX_SIZE].
> -  static int_range<2> invalid_range (build_int_cst (size_type_node, 0),
> -build_int_cst (size_type_node,
> -   max_size),
> -VR_ANTI_RANGE);
> +  int_range<2> invalid_range (build_int_cst (size_type_node, 0),
> + build_int_cst (size_type_node, max_size),
> + VR_ANTI_RANGE);
>
>r.intersect (invalid_range);
>if (r.undefined_p ())
> diff --git a/gcc/testsuite/g++.dg/Walloca2.C b/gcc/testsuite/g++.dg/Walloca2.C
> new file mode 100644
> index 000..b6992d08bf3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/Walloca2.C
> @@ -0,0 +1,6 @@
> +// { dg-do compile }
> +// { dg-options "-Walloca-larger-than=4207115063 
> -Wvla-larger-than=1233877270 -O2 --param ggc-min-heapsize=0 --param 
> ggc-min-expand=0 -w" }
> +// { dg-require-effective-target alloca }
> +
> +int a;
> +char *b = static_cast(__builtin_alloca (a));
> --
> 2.31.1
>


Re: [PATCH] libiberty: d-demangle: remove parenthesis where it is not needed

2021-10-04 Thread ibuclaw--- via Gcc-patches
> On 29/09/2021 18:26 Luís Ferreira  wrote:
> 
>  
> Those parenthesis doesn't increase readability at all and this patch makes the
> source code a bit more consistent with the rest of the dereferencing
> assignments.
> 

OK, but can you write up a changelog entry for it?

Thanks,
Iain.


Re: [PATCH] libiberty: d-demangle: rename function symbols to be more consistent

2021-10-04 Thread Iain Buclaw via Gcc-patches
> On 30/09/2021 02:48 Luís Ferreira  wrote:
> 
>  
> There is some function names with `dlang_parse_` prefix and some with only
> `dlang_` prefix that does parsing. The same happens with `dlang_decode_`.
> 
> To make things a bit more consistent and easier to understand, this patch adds
> the missing prefix, according to the functions signatures.
> 

Not too keen on grand the renaming without a changelog entry to go with.

Iain.


Re: [PATCH] libiberty: d-demangle: use switch instead of if-else

2021-10-04 Thread ibuclaw--- via Gcc-patches
> On 03/10/2021 23:55 Jeff Law via Gcc-patches  wrote:
> 
>  
> On 9/29/2021 7:08 PM, Luís Ferreira wrote:
> > This patch allows the compiler to efficiently generate jump tables instead 
> > of
> > using if-else-if.
> >
> > Signed-off-by: Luís Ferreira 
> I'm not sure this is terribly useful.  Compilers have the ability to 
> analyze the underlying code and make sensible decisions for how to 
> implement either form.   So the right metric here is does this make the 
> code cleaner/easier to understand.  With just 3 clauses it's hard (for 
> me) to make the case that it is considerably cleaner.
> 

I'd be inclined to agree here.

FAOD, I put together a quick example of what difference this patch makes.  
Other than freely reordering the conditions, the answer is nothing.

https://godbolt.org/z/nKjjv64zM

Iain.


Remove dead code in config/rs6000/vxworks.h

2021-10-04 Thread Eric Botcazou via Gcc-patches
These lines were added last year:

/* Initialize library function table.  */
#undef TARGET_INIT_LIBFUNCS
#define TARGET_INIT_LIBFUNCS rs6000_vxworks_init_libfuncs

but TARGET_INIT_LIBFUNCS is #undef-ed in config/rs6000/rs6000.c and 
rs6000_vxworks_init_libfuncs is nowhere defined in any case.

Applied on mainline and 11 branch as obvious.


2021-10-04  Eric Botcazou  

* config/rs6000/vxworks.h (TARGET_INIT_LIBFUNCS): Delete.

-- 
Eric Botcazoudiff --git a/gcc/config/rs6000/vxworks.h b/gcc/config/rs6000/vxworks.h
index 5facbbb392f..d8ecc0296c8 100644
--- a/gcc/config/rs6000/vxworks.h
+++ b/gcc/config/rs6000/vxworks.h
@@ -147,10 +147,6 @@ along with GCC; see the file COPYING3.  If not see
 #undef FUNCTION_PROFILER
 #define FUNCTION_PROFILER(FILE,LABELNO) VXWORKS_FUNCTION_PROFILER(FILE,LABELNO)
 
-/* Initialize library function table.  */
-#undef TARGET_INIT_LIBFUNCS
-#define TARGET_INIT_LIBFUNCS rs6000_vxworks_init_libfuncs
-
 /* Nor sdata, for kernel mode.  We use this in
SUBSUBTARGET_INITIALIZE_OPTIONS, after rs6000_rtp has been initialized.  */
 #undef SDATA_DEFAULT_SIZE


[PATCH] Remove static marker for range in alloca pass.

2021-10-04 Thread Aldy Hernandez via Gcc-patches
The m_ranges[] field in int_range are trees, so they live in GC
space.  Since invalid_range is static, it must be marked with GTY
magic.  However, calculating invalid_range is not particularly slow,
or on a critical path, so we can just put it in local scope and
recalculate every time.

Tested on x86-64 Linux.

Since this is more of a GC thing than a range thing, I'd like a nod from
a global reviewer.

OK?

gcc/ChangeLog:

PR tree-optimization/102560
* gimple-ssa-warn-alloca.c (alloca_call_type): Remove static
marker for invalid_range.

gcc/testsuite/ChangeLog:

* g++.dg/Walloca2.C: New test.
---
 gcc/gimple-ssa-warn-alloca.c| 7 +++
 gcc/testsuite/g++.dg/Walloca2.C | 6 ++
 2 files changed, 9 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/Walloca2.C

diff --git a/gcc/gimple-ssa-warn-alloca.c b/gcc/gimple-ssa-warn-alloca.c
index 4fc7125d378..d59cea8d4ec 100644
--- a/gcc/gimple-ssa-warn-alloca.c
+++ b/gcc/gimple-ssa-warn-alloca.c
@@ -221,10 +221,9 @@ alloca_call_type (gimple *stmt, bool is_vla)
   && !r.varying_p ())
 {
   // The invalid bits are anything outside of [0, MAX_SIZE].
-  static int_range<2> invalid_range (build_int_cst (size_type_node, 0),
-build_int_cst (size_type_node,
-   max_size),
-VR_ANTI_RANGE);
+  int_range<2> invalid_range (build_int_cst (size_type_node, 0),
+ build_int_cst (size_type_node, max_size),
+ VR_ANTI_RANGE);
 
   r.intersect (invalid_range);
   if (r.undefined_p ())
diff --git a/gcc/testsuite/g++.dg/Walloca2.C b/gcc/testsuite/g++.dg/Walloca2.C
new file mode 100644
index 000..b6992d08bf3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/Walloca2.C
@@ -0,0 +1,6 @@
+// { dg-do compile }
+// { dg-options "-Walloca-larger-than=4207115063 -Wvla-larger-than=1233877270 
-O2 --param ggc-min-heapsize=0 --param ggc-min-expand=0 -w" }
+// { dg-require-effective-target alloca }
+
+int a;
+char *b = static_cast(__builtin_alloca (a));
-- 
2.31.1



Re: [RFC][Patch][middle-end/PR102359]Not add initialization for READONLY variables with -ftrivial-auto-var-init

2021-10-04 Thread Richard Biener via Gcc-patches
On Fri, 1 Oct 2021, Qing Zhao wrote:

> 
> 
> > On Oct 1, 2021, at 10:33 AM, Jason Merrill  wrote:
> > 
> > On 10/1/21 10:54, Qing Zhao wrote:
> >>> On Sep 30, 2021, at 2:31 PM, Jason Merrill  wrote:
> >>> 
> >>> On 9/30/21 11:42, Qing Zhao wrote:
> > On Sep 30, 2021, at 1:54 AM, Richard Biener  wrote:
> > 
> > On Thu, 30 Sep 2021, Jason Merrill wrote:
> > 
> >> On 9/29/21 17:30, Qing Zhao wrote:
> >>> Hi,
> >>> 
> >>> PR102359 (ICE gimplification failed since  r12-3433-ga25e0b5e6ac8a77a)
> >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102359
> >>> 
> >>> Is due to -ftrivial-auto-var-init adding initialization for READONLY
> >>> variable “this” in the following routine: (t.cpp.005t.original)
> >>> 
> >>> ===
> >>> 
> >>> ;; Function A::foo():: (null)
> >>> ;; enabled by -tree-original
> >>> 
> >>> {
> >>>  const struct A * const this [value-expr: &__closure->__this];
> >>>const struct A * const this [value-expr: &__closure->__this];
> >>>  return  = (double) ((const struct A *) this)->a;
> >>> }
> >>> ===
> >>> 
> >>> However, in the above routine, “this” is NOT marked as READONLY, but 
> >>> its
> >>> value-expr "&__closure->__this” is marked as READONLY.
> >>> 
> >>> There are two major issues:
> >>> 
> >>> 1. In the routine “is_var_need_auto_init”, we should exclude “decl” 
> >>> that is
> >>> marked as READONLY;
> >>> 2. In the C++ FE, “this” should be marked as READONLY.
> >>> 
> >>> The idea solution will be:
> >>> 
> >>> 1. Fix “is_var_need_auto_init” to exclude TREE_READONLY (decl);
> >>> 2. Fix C++ FE to mark “this” as TREE_READONLY (decl)==true;
> >>> 
> >>> Not sure whether it’s hard for C++ FE to fix the 2nd issue or not?
> >>> 
> >>> In the case it’s not a quick fix in C++FE, I proposed the following 
> >>> fix in
> >>> middle end:
> >>> 
> >>> Let me know your comments or suggestions on this.
> >>> 
> >>> Thanks a lot for the help.
> >> 
> >> I'd think is_var_need_auto_init should be false for any variable with
> >> DECL_HAS_VALUE_EXPR_P, as they aren't really variables, just ways of 
> >> naming
> >> objects that are initialized elsewhere.
> > 
> > IIRC handing variables with DECL_HAS_VALUE_EXPR_P is necessary to
> > auto-init VLAs, otherwise I tend to agree - would we handle those
> > when we see a DECL_EXPR then?
>  The current implementation is:
>  gimplify_decl_expr:
>  For each DECL_EXPR “decl”
> If (VAR_P (decl) && !DECL_EXTERNAL (decl))
>   {
>   if (is_vla (decl))
>    gimplify_vla_decl (decl, …);  /* existing handling: create 
>  a VALUE_EXPR for this vla decl*/
>   …
>   if (has_explicit_init (decl))
> {
>  …; /* existing handling.  */
> }
>   else if (is_var_need_auto_init (decl))  /*. New code. */
> {
>   gimple_add_init_for_auto_var (….);   /*  new code.  */
>   ...
> }
>   }
>  Since the “DECL_VALUE_EXPR (decl)” is NOT a DECL_EXPR, it will not be 
>  scanned and added initialization.
>  if we do not add initialization for a decl that has DECL_VALUE_EXPR, 
>  then the “DECL_VALUE_EXPR (decl)” will not be added an initialization 
>  either.  We will miss adding initializations for these decls.
>  So, I think that the current implementation is correct.
>  And if C++ FE will not mark “this” as READONLY, only mark 
>  DECL_VALUE_EXPR(this) as READONLY, the proposed fix is correct too.
>  Let me know your opinion on this.
> >>> 
> >>> The problem with this test is not whether the 'this' proxy is marked 
> >>> READONLY, the problem is that you're trying to initialize lambda capture 
> >>> proxies at all; the lambda capture objects were already initialized when 
> >>> forming the closure object.  So this test currently aborts with 
> >>> -ftrivial-auto-var-init=zero because you "initialize" the i capture field 
> >>> to 0 after it was previously initialized to 42:
> >>> 
> >>> int main()
> >>> {
> >>>  int i = 42;
> >>>  auto l = [=]() mutable { return i; };
> >>>  if (l() != i)
> >>>__builtin_abort ();
> >>> }
> >>> 
> >>> I believe the same issue applies to the proxy variables in coroutines 
> >>> that work much like lambdas.
> > 
> >> So, how should the middle end determine that a variable is “proxy 
> >> variable”?
> > 
> > In the front end, is_capture_proxy will identify a lambda capture proxy 
> > variable.  But that won't be true for the similar proxies used by 
> > coroutines.
> 
> Does this mean that in middle end, especially in gimplification phase, there 
> is Not a simple way to determine whether a variable is a proxy variable?
> > 
> >> Have all “proxy variables” been initialized by C++ FE already?
> > 
> > Yes.
> > 
> >>> You can't just assume that a