Re: PR98974: Fix vectorizable_condition after STMT_VINFO_VEC_STMTS

[Richard Sandiford via Gcc-patches]

"Andre Vieira (lists)"  writes:
> On 05/02/2021 12:47, Richard Sandiford wrote:
>> "Andre Vieira (lists)"  writes:
>>> Hi,
>>>
>>> As mentioned in the PR, this patch fixes up the nvectors parameter passed 
>>> to vect_get_loop_mask in vectorizable_condition.
>>> Before the STMT_VINFO_VEC_STMTS rework we used to handle each ncopy 
>>> separately, now we gather them all at the same time and don't need to 
>>> multiply vec_num with ncopies.
>>>
>>> The reduced testcase I used to illustrate the issue in the PR gives a 
>>> warning, if someone knows how to get rid of that (it's Fortran) I'd include 
>>> it as a testcase for this.
>> Looks like Richi's since posted one.
> Included it.
>>> diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
>>> index 
>>> 0bc1cb1c5b4f6c1f0447241b4d31434bf7cca1a4..d07602f6d38f9c51936ac09942599fc5a14f46ab
>>>  100644
>>> --- a/gcc/tree-vect-stmts.c
>>> +++ b/gcc/tree-vect-stmts.c
>>> @@ -10237,8 +10237,7 @@ vectorizable_condition (vec_info *vinfo,
>>> {
>>>   unsigned vec_num = vec_oprnds0.length ();
>>>   tree loop_mask
>>> -   = vect_get_loop_mask (gsi, masks, vec_num * ncopies,
>>> - vectype, i);
>>> +   = vect_get_loop_mask (gsi, masks, vec_num, vectype, i);
>>>   tree tmp2 = make_ssa_name (vec_cmp_type);
>>>   gassign *g
>>> = gimple_build_assign (tmp2, BIT_AND_EXPR, vec_compare,
>> Does removing the shadowed vec_num work?  I think that would be less
>> confusing, and means that the calculation stays in sync with the
> Yeah that works too.
>
> Here's a reworked patch.
>
>
> gcc/ChangeLog:
> 2021-02-05  Andre Vieira  
>
>      PR middle-end/98974
>      * tree-vect-stmts.c (vectorizable_condition): Fix nvectors 
> parameter
>      for vect_get_loop_mask call.
>
> gcc/testsuite/ChangeLog:
> 2021-02-05  Andre Vieira  
>
>      * gfortran.dg/pr98974.F90: New test.
>
> diff --git a/gcc/testsuite/gfortran.dg/pr98974.F90 
> b/gcc/testsuite/gfortran.dg/pr98974.F90
> new file mode 100644
> index 
> ..b3db6a6654a0b36bc567405c70429a5dbe168d1e
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/pr98974.F90
> @@ -0,0 +1,20 @@
> +! PR middle-end/98974
> +! { dg-do compile { target { aarch64*-*-* } } }
> +! { dg-options "-Ofast -mcpu=neoverse-v1" }

Only -mcpu=neoverse-v1 is specific to aarch64*, so I think this should be:

! { dg-do compile }
! { dg-options "-Ofast" }
! { dg-additional-options "-mcpu=neoverse-v1" { target aarch64*-*-* } }

OK with that change, thanks.

Richard

> +
> +module module_foobar
> +  integer,parameter :: fp_kind = selected_real_kind(15)
> +contains
> + subroutine foobar( foo, ix ,jx ,kx,iy,ky)
> +   real, dimension( ix, kx, jx )  :: foo
> +   real(fp_kind), dimension( iy, ky, 3 ) :: bar, baz
> +   do k=1,ky
> +  do i=1,iy
> +if ( baz(i,k,1) > 0. ) then
> +  bar(i,k,1) = 0
> +endif
> +foo(i,nk,j) = baz0 *  bar(i,k,1)
> +  enddo
> +   enddo
> + end
> +end
> diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
> index 
> 0bc1cb1c5b4f6c1f0447241b4d31434bf7cca1a4..064e5d138ce9a151287662a0caefd9925b0a2920
>  100644
> --- a/gcc/tree-vect-stmts.c
> +++ b/gcc/tree-vect-stmts.c
> @@ -10235,7 +10235,6 @@ vectorizable_condition (vec_info *vinfo,
>  
> if (masks)
>   {
> -   unsigned vec_num = vec_oprnds0.length ();
> tree loop_mask
>   = vect_get_loop_mask (gsi, masks, vec_num * ncopies,
> vectype, i);

RE: [PATCH] tree-optimization/97236 - fix bad use of VMAT_CONTIGUOUS

[Kyrylo Tkachov via Gcc-patches]

> -Original Message-
> From: Richard Biener 
> Sent: 05 February 2021 13:51
> To: Kyrylo Tkachov 
> Cc: gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH] tree-optimization/97236 - fix bad use of
> VMAT_CONTIGUOUS
> 
> On Fri, 5 Feb 2021, Kyrylo Tkachov wrote:
> 
> > Hi Richard,
> >
> > > -Original Message-
> > > From: Gcc-patches  On Behalf Of
> > > Richard Biener
> > > Sent: 01 October 2020 14:15
> > > To: gcc-patches@gcc.gnu.org
> > > Subject: [PATCH] tree-optimization/97236 - fix bad use of
> > > VMAT_CONTIGUOUS
> > >
> > > This avoids using VMAT_CONTIGUOUS with single-element interleaving
> > > when using V1mode vectors.  Instead keep VMAT_ELEMENTWISE but
> > > continue to avoid load-lanes and gathers.
> > >
> > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> >
> > I've checked that this fix also fixes the recently-reported PR 98949 on
> aarch64 on the GCC 9 branch.
> > I've bootstrapped and tested it on the branch on aarch64-none-linux.
> > Is it okay to backport to the branch?
> 
> Yes.

Thanks, is it ok for the GCC 8 branch too?
Tested that the testcase is fixed on that branch and bootstrapped and regtested 
on aarch64-none-linux-gnu.
Kyrill

> 
> Thanks,
> Richard.
> 
> > Thanks,
> > Kyrill
> >
> > >
> > > Richard.
> > >
> > > 2020-10-01  Richard Biener  
> > >
> > >   PR tree-optimization/97236
> > >   * tree-vect-stmts.c (get_group_load_store_type): Keep
> > >   VMAT_ELEMENTWISE for single-element vectors.
> > >
> > >   * gcc.dg/vect/pr97236.c: New testcase.
> > > ---
> > >  gcc/testsuite/gcc.dg/vect/pr97236.c | 43
> > > +
> > >  gcc/tree-vect-stmts.c   | 20 ++
> > >  2 files changed, 52 insertions(+), 11 deletions(-)
> > >  create mode 100644 gcc/testsuite/gcc.dg/vect/pr97236.c
> > >
> > > diff --git a/gcc/testsuite/gcc.dg/vect/pr97236.c
> > > b/gcc/testsuite/gcc.dg/vect/pr97236.c
> > > new file mode 100644
> > > index 000..03e0cc38984
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.dg/vect/pr97236.c
> > > @@ -0,0 +1,43 @@
> > > +typedef unsigned char __uint8_t;
> > > +typedef __uint8_t uint8_t;
> > > +typedef struct plane_t {
> > > +  uint8_t *p_pixels;
> > > +  int i_lines;
> > > +  int i_pitch;
> > > +} plane_t;
> > > +
> > > +typedef struct {
> > > +  plane_t p[5];
> > > +} picture_t;
> > > +
> > > +#define N 4
> > > +
> > > +void __attribute__((noipa))
> > > +picture_Clone(picture_t *picture, picture_t *res)
> > > +{
> > > +  for (int i = 0; i < N; i++) {
> > > +res->p[i].p_pixels = picture->p[i].p_pixels;
> > > +res->p[i].i_lines = picture->p[i].i_lines;
> > > +res->p[i].i_pitch = picture->p[i].i_pitch;
> > > +  }
> > > +}
> > > +
> > > +int
> > > +main()
> > > +{
> > > +  picture_t aaa, bbb;
> > > +  uint8_t pixels[10] = {1, 1, 1, 1, 1, 1, 1, 1};
> > > +
> > > +  for (unsigned i = 0; i < N; i++)
> > > +aaa.p[i].p_pixels = pixels;
> > > +
> > > +  picture_Clone (&aaa, &bbb);
> > > +
> > > +  uint8_t c;
> > > +  for (unsigned i = 0; i < N; i++)
> > > +c += bbb.p[i].p_pixels[0];
> > > +
> > > +  if (c != N)
> > > +__builtin_abort ();
> > > +  return 0;
> > > +}
> > > diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
> > > index 191957c3543..3575f25241f 100644
> > > --- a/gcc/tree-vect-stmts.c
> > > +++ b/gcc/tree-vect-stmts.c
> > > @@ -2235,25 +2235,23 @@ get_group_load_store_type (vec_info
> *vinfo,
> > > stmt_vec_info stmt_info,
> > > /* First cope with the degenerate case of a single-element
> > >vector.  */
> > > if (known_eq (TYPE_VECTOR_SUBPARTS (vectype), 1U))
> > > - *memory_access_type = VMAT_CONTIGUOUS;
> > > + ;
> > >
> > > /* Otherwise try using LOAD/STORE_LANES.  */
> > > -   if (*memory_access_type == VMAT_ELEMENTWISE
> > > -   && (vls_type == VLS_LOAD
> > > -   ? vect_load_lanes_supported (vectype, group_size,
> > > masked_p)
> > > -   : vect_store_lanes_supported (vectype, group_size,
> > > - masked_p)))
> > > +   else if (vls_type == VLS_LOAD
> > > +? vect_load_lanes_supported (vectype, group_size,
> > > masked_p)
> > > +: vect_store_lanes_supported (vectype, group_size,
> > > +  masked_p))
> > >   {
> > > *memory_access_type = VMAT_LOAD_STORE_LANES;
> > > overrun_p = would_overrun_p;
> > >   }
> > >
> > > /* If that fails, try using permuting loads.  */
> > > -   if (*memory_access_type == VMAT_ELEMENTWISE
> > > -   && (vls_type == VLS_LOAD
> > > -   ? vect_grouped_load_supported (vectype,
> > > single_element_p,
> > > -  group_size)
> > > -   : vect_grouped_store_supported (vectype, group_size)))
> > > +   else if (vls_type == VLS_LOAD
> > > +? vect_grouped_load_supported (vectype,
> > > single_element_p,
> > > +   group_size)
> > > +: vect_groupe

RE: [PATCH] tree-optimization/97236 - fix bad use of VMAT_CONTIGUOUS

[Richard Biener]

On Mon, 8 Feb 2021, Kyrylo Tkachov wrote:

> 
> 
> > -Original Message-
> > From: Richard Biener 
> > Sent: 05 February 2021 13:51
> > To: Kyrylo Tkachov 
> > Cc: gcc-patches@gcc.gnu.org
> > Subject: RE: [PATCH] tree-optimization/97236 - fix bad use of
> > VMAT_CONTIGUOUS
> > 
> > On Fri, 5 Feb 2021, Kyrylo Tkachov wrote:
> > 
> > > Hi Richard,
> > >
> > > > -Original Message-
> > > > From: Gcc-patches  On Behalf Of
> > > > Richard Biener
> > > > Sent: 01 October 2020 14:15
> > > > To: gcc-patches@gcc.gnu.org
> > > > Subject: [PATCH] tree-optimization/97236 - fix bad use of
> > > > VMAT_CONTIGUOUS
> > > >
> > > > This avoids using VMAT_CONTIGUOUS with single-element interleaving
> > > > when using V1mode vectors.  Instead keep VMAT_ELEMENTWISE but
> > > > continue to avoid load-lanes and gathers.
> > > >
> > > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> > >
> > > I've checked that this fix also fixes the recently-reported PR 98949 on
> > aarch64 on the GCC 9 branch.
> > > I've bootstrapped and tested it on the branch on aarch64-none-linux.
> > > Is it okay to backport to the branch?
> > 
> > Yes.
> 
> Thanks, is it ok for the GCC 8 branch too?

Yes.

> Tested that the testcase is fixed on that branch and bootstrapped and 
> regtested on aarch64-none-linux-gnu.
> Kyrill
> 
> > 
> > Thanks,
> > Richard.
> > 
> > > Thanks,
> > > Kyrill
> > >
> > > >
> > > > Richard.
> > > >
> > > > 2020-10-01  Richard Biener  
> > > >
> > > > PR tree-optimization/97236
> > > > * tree-vect-stmts.c (get_group_load_store_type): Keep
> > > > VMAT_ELEMENTWISE for single-element vectors.
> > > >
> > > > * gcc.dg/vect/pr97236.c: New testcase.
> > > > ---
> > > >  gcc/testsuite/gcc.dg/vect/pr97236.c | 43
> > > > +
> > > >  gcc/tree-vect-stmts.c   | 20 ++
> > > >  2 files changed, 52 insertions(+), 11 deletions(-)
> > > >  create mode 100644 gcc/testsuite/gcc.dg/vect/pr97236.c
> > > >
> > > > diff --git a/gcc/testsuite/gcc.dg/vect/pr97236.c
> > > > b/gcc/testsuite/gcc.dg/vect/pr97236.c
> > > > new file mode 100644
> > > > index 000..03e0cc38984
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/gcc.dg/vect/pr97236.c
> > > > @@ -0,0 +1,43 @@
> > > > +typedef unsigned char __uint8_t;
> > > > +typedef __uint8_t uint8_t;
> > > > +typedef struct plane_t {
> > > > +  uint8_t *p_pixels;
> > > > +  int i_lines;
> > > > +  int i_pitch;
> > > > +} plane_t;
> > > > +
> > > > +typedef struct {
> > > > +  plane_t p[5];
> > > > +} picture_t;
> > > > +
> > > > +#define N 4
> > > > +
> > > > +void __attribute__((noipa))
> > > > +picture_Clone(picture_t *picture, picture_t *res)
> > > > +{
> > > > +  for (int i = 0; i < N; i++) {
> > > > +res->p[i].p_pixels = picture->p[i].p_pixels;
> > > > +res->p[i].i_lines = picture->p[i].i_lines;
> > > > +res->p[i].i_pitch = picture->p[i].i_pitch;
> > > > +  }
> > > > +}
> > > > +
> > > > +int
> > > > +main()
> > > > +{
> > > > +  picture_t aaa, bbb;
> > > > +  uint8_t pixels[10] = {1, 1, 1, 1, 1, 1, 1, 1};
> > > > +
> > > > +  for (unsigned i = 0; i < N; i++)
> > > > +aaa.p[i].p_pixels = pixels;
> > > > +
> > > > +  picture_Clone (&aaa, &bbb);
> > > > +
> > > > +  uint8_t c;
> > > > +  for (unsigned i = 0; i < N; i++)
> > > > +c += bbb.p[i].p_pixels[0];
> > > > +
> > > > +  if (c != N)
> > > > +__builtin_abort ();
> > > > +  return 0;
> > > > +}
> > > > diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
> > > > index 191957c3543..3575f25241f 100644
> > > > --- a/gcc/tree-vect-stmts.c
> > > > +++ b/gcc/tree-vect-stmts.c
> > > > @@ -2235,25 +2235,23 @@ get_group_load_store_type (vec_info
> > *vinfo,
> > > > stmt_vec_info stmt_info,
> > > >   /* First cope with the degenerate case of a single-element
> > > >  vector.  */
> > > >   if (known_eq (TYPE_VECTOR_SUBPARTS (vectype), 1U))
> > > > -   *memory_access_type = VMAT_CONTIGUOUS;
> > > > +   ;
> > > >
> > > >   /* Otherwise try using LOAD/STORE_LANES.  */
> > > > - if (*memory_access_type == VMAT_ELEMENTWISE
> > > > - && (vls_type == VLS_LOAD
> > > > - ? vect_load_lanes_supported (vectype, group_size,
> > > > masked_p)
> > > > - : vect_store_lanes_supported (vectype, group_size,
> > > > -   masked_p)))
> > > > + else if (vls_type == VLS_LOAD
> > > > +  ? vect_load_lanes_supported (vectype, group_size,
> > > > masked_p)
> > > > +  : vect_store_lanes_supported (vectype, group_size,
> > > > +masked_p))
> > > > {
> > > >   *memory_access_type = VMAT_LOAD_STORE_LANES;
> > > >   overrun_p = would_overrun_p;
> > > > }
> > > >
> > > >   /* If that fails, try using permuting loads.  */
> > > > - if (*memory_access_type =

Re: calibrate intervals to avoid zero in futures poll test

[Christophe Lyon via Gcc-patches]

On Thu, 14 Jan 2021 at 19:57, Alexandre Oliva  wrote:
>
> On Jan 14, 2021, Jonathan Wakely  wrote:
>
> >> +  /* Got for some 10 cycles, but we're already past that and still
>
> > I can't parse "Got for some 10 cycles". If that's just a typo
>
> Yeah, I meant "Go for ... but if ..." and managed to double-mangle it.
> Thanks for spotting it.  Here's the patch I'm installing, with the typos
> fixed.  Thanks!
>
>
> calibrate intervals to avoid zero in futures poll test
>
> From: Alexandre Oliva 
>
> We get occasional failures of 30_threads/future/members/poll.cc
> on some platforms whose high resolution clock doesn't have such a high
> resolution; wait_for_0 ends up as 0, and then some asserts fail as
> intervals measured as longer than zero are tested for less than
> several times zero.
>
> This patch adds some calibration in the iteration count to set a
> measurable base time interval with some additional margin.
>

Seeing such random errors when testing arm target under qemu
on shared servers. I noticed several such errors on gcc-testresults, too.

So I guess this is a ping for this patch, to clear this noise in the results?

Thanks,

Christophe


>
> for  libstdc++-v3/ChangeLog
>
> * testsuite/30_threads/future/members/poll.cc: Calibrate
> iteration count.
> ---
>  .../testsuite/30_threads/future/members/poll.cc|   33 
> +++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/libstdc++-v3/testsuite/30_threads/future/members/poll.cc 
> b/libstdc++-v3/testsuite/30_threads/future/members/poll.cc
> index 91f685b172d73..133dae15ac471 100644
> --- a/libstdc++-v3/testsuite/30_threads/future/members/poll.cc
> +++ b/libstdc++-v3/testsuite/30_threads/future/members/poll.cc
> @@ -25,7 +25,7 @@
>  #include 
>  #include 
>
> -const int iterations = 200;
> +int iterations = 200;
>
>  using namespace std;
>
> @@ -45,10 +45,41 @@ int main()
>promise p;
>future f = p.get_future();
>
> + start_over:
>auto start = chrono::high_resolution_clock::now();
>for(int i = 0; i < iterations; i++)
>  f.wait_for(chrono::seconds(0));
>auto stop = chrono::high_resolution_clock::now();
> +
> +  /* We've run too few iterations for the clock resolution.
> + Attempt to calibrate it.  */
> +  if (start == stop)
> +{
> +  /* Loop until the clock advances, so that start is right after a
> +time increment.  */
> +  do
> +   start = chrono::high_resolution_clock::now();
> +  while (start == stop);
> +  int i = 0;
> +  /* Now until the clock advances again, so that stop is right
> +after another time increment.  */
> +  do
> +   {
> + f.wait_for(chrono::seconds(0));
> + stop = chrono::high_resolution_clock::now();
> + i++;
> +   }
> +  while (start == stop);
> +  /* Go for some 10 cycles, but if we're already past that and
> +still get into the calibration loop, double the iteration
> +count and try again.  */
> +  if (iterations < i * 10)
> +   iterations = i * 10;
> +  else
> +   iterations *= 2;
> +  goto start_over;
> +}
> +
>double wait_for_0 = print("wait_for(0s)", stop - start);
>
>start = chrono::high_resolution_clock::now();
>
>
> --
> Alexandre Oliva, happy hacker  https://FSFLA.org/blogs/lxo/
>Free Software Activist GNU Toolchain Engineer
> Vim, Vi, Voltei pro Emacs -- GNUlius Caesar

[PATCH] lto/96591 - walk VECTOR_CST elements in walk_tree

[Richard Biener]

This implements walking of VECTOR_CST elements in walk_tree, mimicing
the walk of COMPLEX_CST elements.  Without this free-lang-data fails
to see some types in case they are only refered to via tree constants
used only as VECTOR_CST elements.

LTO bootstrapped, testing on x86_64-unknown-linux-gnu in progress,
will push after a non-LTO bootstrap since I used --disable-werror.

2021-02-08  Richard Biener  

PR lto/96591
* tree.c (walk_tree_1): Walk VECTOR_CST elements.

* g++.dg/lto/pr96591_0.C: New testcase.
---
 gcc/testsuite/g++.dg/lto/pr96591_0.C | 45 
 gcc/tree.c   | 13 +++-
 2 files changed, 57 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/lto/pr96591_0.C

diff --git a/gcc/testsuite/g++.dg/lto/pr96591_0.C 
b/gcc/testsuite/g++.dg/lto/pr96591_0.C
new file mode 100644
index 000..ae2dc98efc4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr96591_0.C
@@ -0,0 +1,45 @@
+// { dg-lto-do assemble }
+// { dg-lto-options { { -O -flto } } }
+
+template 
+struct builtin_simd
+{
+  using type [[gnu::vector_size(sizeof(scalar_t) * length)]] = scalar_t;
+};
+
+struct simd_traits
+{
+  using scalar_type = int;
+
+  template 
+  using rebind = typename builtin_simd::type;
+};
+
+template 
+constexpr simd_t fill(typename simd_traits::scalar_type const scalar)
+{
+  return simd_t{scalar};
+}
+
+class Test
+{
+using score_type = typename builtin_simd::type;
+score_type data[1]{fill(8)};
+};
+
+struct TestFactoryBase
+{
+  virtual Test *CreateTest() = 0;
+};
+
+template 
+struct TestFactoryImpl : public TestFactoryBase
+{
+  Test *CreateTest() override { return new TestClass; }
+};
+
+void MakeAndRegisterTestInfo(TestFactoryBase *factory);
+
+int main() {
+  MakeAndRegisterTestInfo(new TestFactoryImpl);
+}
diff --git a/gcc/tree.c b/gcc/tree.c
index 430b76168b2..c09434d7293 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -12131,7 +12131,6 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
 case INTEGER_CST:
 case REAL_CST:
 case FIXED_CST:
-case VECTOR_CST:
 case STRING_CST:
 case BLOCK:
 case PLACEHOLDER_EXPR:
@@ -12162,6 +12161,18 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
WALK_SUBTREE_TAIL (TREE_VEC_ELT (*tp, 0));
   }
 
+case VECTOR_CST:
+  {
+   unsigned len = vector_cst_encoded_nelts (*tp);
+   if (len == 0)
+ break;
+   /* Walk all elements but the first.  */
+   while (--len)
+ WALK_SUBTREE (VECTOR_CST_ENCODED_ELT (*tp, len));
+   /* Now walk the first one as a tail call.  */
+   WALK_SUBTREE_TAIL (VECTOR_CST_ENCODED_ELT (*tp, 0));
+  }
+
 case COMPLEX_CST:
   WALK_SUBTREE (TREE_REALPART (*tp));
   WALK_SUBTREE_TAIL (TREE_IMAGPART (*tp));
-- 
2.26.2

Re: [PATCH] opts: fix handling of -fpatchable-function-entries option

[Richard Biener via Gcc-patches]

On Fri, Feb 5, 2021 at 2:49 PM Martin Liška  wrote:
>
> Hello.
>
> As seen the flag -fpatchable-function-entry is properly marked as 
> Optimization.
> However, it's the argument is parsed early and stored into the following 
> tuple:
>
> ; How many NOP insns to place at each function entry by default
> Variable
> HOST_WIDE_INT function_entry_patch_area_size
>
> ; And how far the real asm entry point is into this area
> Variable
> HOST_WIDE_INT function_entry_patch_area_start
>
> That does not work with set_current_function where per-function arguments are 
> restored.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?

OK.

Thanks,
Richard.

> Thanks,
> Martin
>
> gcc/ChangeLog:
>
> PR lto/98971
> * cfgexpand.c (pass_expand::execute): Parse per-function option
> flag_patchable_function_entry and use it.
> * common.opt: Remove function_entry_patch_area_size and
> function_entry_patch_area_start global variables.
> * opts.c (parse_and_check_patch_area): New function.
> (common_handle_option): Use it.
> * opts.h (parse_and_check_patch_area): New function.
> * toplev.c (process_options): Parse and use
> function_entry_patch_area_size.
> ---
>   gcc/cfgexpand.c |  6 +++--
>   gcc/common.opt  | 10 +---
>   gcc/opts.c  | 65 +++--
>   gcc/opts.h  |  4 +++
>   gcc/toplev.c|  6 -
>   5 files changed, 55 insertions(+), 36 deletions(-)
>
> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
> index 8d20ca6cefb..aef9e916fcd 100644
> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -73,6 +73,7 @@ along with GCC; see the file COPYING3.  If not see
>   #include "tree-ssa-address.h"
>   #include "output.h"
>   #include "builtins.h"
> +#include "opts.h"
>
>   /* Some systems use __main in a way incompatible with its use in gcc, in 
> these
>  cases use the macros NAME__MAIN to give a quoted symbol and SYMBOL__MAIN 
> to
> @@ -6845,8 +6846,9 @@ pass_expand::execute (function *fun)
> if (crtl->tail_call_emit)
>   fixup_tail_calls ();
>
> -  unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size;
> -  unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start;
> +  HOST_WIDE_INT patch_area_size, patch_area_entry;
> +  parse_and_check_patch_area (flag_patchable_function_entry, false,
> + &patch_area_size, &patch_area_entry);
>
> tree patchable_function_entry_attr
>   = lookup_attribute ("patchable_function_entry",
> diff --git a/gcc/common.opt b/gcc/common.opt
> index a8a2b67a99d..c75dd36843e 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -144,14 +144,6 @@ bool flag_stack_usage_info = false
>   Variable
>   int flag_debug_asm
>
> -; How many NOP insns to place at each function entry by default
> -Variable
> -HOST_WIDE_INT function_entry_patch_area_size
> -
> -; And how far the real asm entry point is into this area
> -Variable
> -HOST_WIDE_INT function_entry_patch_area_start
> -
>   ; Balance between GNAT encodings and standard DWARF to emit.
>   Variable
>   enum dwarf_gnat_encodings gnat_encodings = DWARF_GNAT_ENCODINGS_DEFAULT
> @@ -2309,7 +2301,7 @@ Common Var(flag_profile_reorder_functions) Optimization
>   Enable function reordering that improves code placement.
>
>   fpatchable-function-entry=
> -Common Joined Optimization
> +Common Var(flag_patchable_function_entry) Joined Optimization
>   Insert NOP instructions at each function entry.
>
>   frandom-seed
> diff --git a/gcc/opts.c b/gcc/opts.c
> index 1f1cf8388f7..fc5f61e13cc 100644
> --- a/gcc/opts.c
> +++ b/gcc/opts.c
> @@ -2130,6 +2130,44 @@ check_alignment_argument (location_t loc, const char 
> *flag, const char *name,
>   }
>   }
>
> +/* Parse argument of -fpatchable-function-entry option ARG and store
> +   corresponding values to PATCH_AREA_SIZE and PATCH_AREA_START.
> +   If REPORT_ERROR is set to true, generate error for a problematic
> +   option arguments.  */
> +
> +void
> +parse_and_check_patch_area (const char *arg, bool report_error,
> +   HOST_WIDE_INT *patch_area_size,
> +   HOST_WIDE_INT *patch_area_start)
> +{
> +  *patch_area_size = 0;
> +  *patch_area_start = 0;
> +
> +  if (arg == NULL)
> +return;
> +
> +  char *patch_area_arg = xstrdup (arg);
> +  char *comma = strchr (patch_area_arg, ',');
> +  if (comma)
> +{
> +  *comma = '\0';
> +  *patch_area_size = integral_argument (patch_area_arg);
> +  *patch_area_start = integral_argument (comma + 1);
> +}
> +  else
> +*patch_area_size = integral_argument (patch_area_arg);
> +
> +  if (*patch_area_size < 0
> +  || *patch_area_size > USHRT_MAX
> +  || *patch_area_start < 0
> +  || *patch_area_start > USHRT_MAX
> +  || *patch_area_size < *patch_area_start)
> +if (report_error)
> +  error ("invalid arguments for %<-fpatchable

Re: [PATCH] document BLOCK_ABSTRACT_ORIGIN et al.

[Richard Biener via Gcc-patches]

On Sat, Feb 6, 2021 at 8:11 PM Martin Sebor  wrote:
>
> On 2/4/21 1:48 AM, Richard Biener wrote:
> > On Wed, Feb 3, 2021 at 6:12 PM Martin Sebor  wrote:
> >>
> >> On 2/3/21 5:01 AM, Richard Biener wrote:
> >>> On Mon, Feb 1, 2021 at 5:20 PM Martin Sebor  wrote:
> 
>  I have pushed the tree.h comments in g:6a2053773b8.  I will wait
>  for an approval of the changes to the manual.
> >>>
> >>> Sorry for not looking earlier.
> >>
> >> Sorry, I thought you were fine with the text after your first review.
> >> I'll adjust the tree.h comments when we're done, though I'd like to
> >> think the example in the manual will do a lot more to help make it
> >> clear than the comments in tree.h can.
> >>
> >>>
> >>> +/* The scope enclosing the scope NODE, or FUNCTION_DECL for the 
> >>> "outermost"
> >>> +   function scope.  Inlined functions are chained by this so that given
> >>> +   expression E and its TREE_BLOCK(E) B, BLOCK_SUPERCONTEXT(B) is the 
> >>> scope
> >>> +   in which E has been made or into which E has been inlined.   */
> >>>
> >>> I can't really understand what you are trying to say with the second
> >>> sentence.  There's
> >>> nothing really special about BLOCK_SUPERCONTEXT and inlines so I believe 
> >>> this
> >>> sentence only adds confusion.
> >>
> >> The sentence explains how SUPERCONTEXT chains inlined blocks.  In
> >> the manual diff I show an example:
> >>
> >>   void f0 (char *p, int n) { memset (p, 1, n); }
> >>
> >> void f1 (char *p, int n) { f0 (p + 1, n + 1); }
> >>
> >>   void f2 (char *p, int n) { f1 (p + 1, n + 1); }
> >>
> >> int a[6];
> >> void f3 (char *p, int n) { f2 (a, 3); }
> >>
> >> The blocks for all calls inlined into f3 are chained like so:
> >>
> >> CALL_EXPR: memset  E
> >>
> >> BLOCK #13 <--+ TREE_BLOCK (E)
> >> +-- SUPERCONTEXT: BLOCK #12  |
> >> | ABSTRACT_ORIGIN: BLOCK #0 --+
> >> |||
> >> +-> BLOCK #12 (f1)<--|-+  |
> >> +-- SUPERCONTEXT: BLOCK #10  | |  |
> >> | SUBBLOCKS: BLOCK #13 --|-|  |
> >> | ABSTRACT_ORIGIN: f0 ---+ |  |
> >> |  |  |
> >> +-> BLOCK #10 (f2) <-+ |  |
> >> +--- SUPERCONTEXT: BLOCK #8  | |  |
> >> |SUBBLOCKS: BLOCK #12 ---|-|  |
> >> |ABSTRACT_ORIGIN: f1 --+  |
> >> |||
> >> +-> BLOCK #8 (f3)||
> >> + SUPERCONTEXT: BLOCK #0 ||
> >> | SUBBLOCKS: BLOCK #10 --||
> >> | ABSTRACT_ORIGIN: f2 ---+|
> >> | |
> >> +-> BLOCK #0 (f3) <---+
> >>   SUPERCONTEXT: f3
> >>   SUBBLOCKS: BLOCK #8
> >>
> >> Does the following sound better? (Dropping the "in which E has been
> >> made.")
> >>
> >> Inlined functions are chained by this so that given expression E
> >> and its TREE_BLOCK(E) B, BLOCK_SUPERCONTEXT(B) is the scope into
> >> which E has been inlined.
> >
> > Oh, I see what you mean.  But this is misleading for the case where f0
> > has any blocks:
> >
> >   f0 (..) { { int tem; { memset (..); } } if () { }... }
> >
> > because BLOCK_SUPERCONTEXT is simply the parent scope ( { int tem; ... } ) 
> > and
> > not yet the artifical scope we generate to wrap f0.
>
> I haven't seen those scopes in my (admittedly simplistic) tests and
> based on the internals manual have been assuming most lexical scopes
> are removed during gimplification:
>
>Additionally, all the control structures used in GENERIC are lowered
>into conditional jumps, lexical scopes are removed and exception
>regions are converted into an on the side exception region tree.
>
> Is the above wrong or inaccurate?  Do all nested scope not get removed?

The scopes are not present in the GIMPLE IL but the lexical scope tree
is still there in a function decls DECL_INITIAL.  What happens is that
scopes are gone semantically since all automatic vars get promoted to function
scope by GIMPLE but we preserve (some) out-of-scope going by inserting
CLOBBER instructions into the IL (there's nothing similar at the point
the vars originally went live - liveness analysis has to be done for this
and the compiler can freely move defs outside of the original scope).

> > To figure the
> > scope a block was
> > inlined to you'd have to do sth like
> >
> >b = TREE_BLOCK (E);
> >gcc_assert (BLOCK_ABSTRACT_ORIGIN (b)); // it was inlined
> >while (!inlined_function_outer_scope_p (b))
> >  b = BLOCK_SUPERCONTEXT (b);
> >now BLOCK_SUPERCONTEXT (b) is the block the function containing E was
> >inlined to.
> >
> > So again, I think tree.h is not the place to document this.  There
> > BLOCK_SUPERCONTEXT
> > should simply say it's pointing  to the parent BLOCK.
>
> I can simplify the text but want to make it clear it doesn't
> necessarily point to a BLOCK but can also point to a FUNCTION_DECL.
> 

[PATCH 0/8 RFC] unbreak --with-included-gettext, and other configury stuff

[Nick Alcock via Gcc-patches]

Most of this series serves one goal: fixing problems Stephen Casner reported
with a binutils built --with-included-gettext or built on a platform that
doesn't have a gettext in a system libintl or in libc. This has long been broken
in binutils.

Firstly, two commits from last year that allow intl/ to build on systems with
Bison 3 are missing. That's easy to fix, but even with that fixed we have other
problems.  libctf would like to compile programs that use libctf.la in its
testsuite, without having to account for the fact that some of them might
include libintl symbols (i.e. it would like libctf.la to be complete, and
ideally libbfd.la too); but making these libraries complete is stymied by the
problem that you cannot link the built-in libintl symbols into shared libraries
on many platforms because it's not PIC. So we picize it, much as libiberty
already is. This then lets us fix gdbserver, which builds a preloaded shared
object and thus won't build without libintl if gettext isn't in libc.

That fixes the shared library case -- but then there's the problem Stephen
reported, which is that LIBINTL in intl/config.intl contains things like this:

LIBINTL='${top_builddir}/../libintl.a -liconv"

(sometimes without the -liconv part).

Linking with this doesn't work with static libraries on some platforms
(e.g. MacOS X) which build libintl.a *into* libctf.a or whatever other library,
and then fail to find symbols in it at build time because a .a file isn't an .o
file.  The solution here is to rewrite that into

LIBINTL='-L${top_builddir}/.. -lintl -liconv"

which always works, static or shared, library or not.

The way LIBINTL is transformed is a bit gross: rather than trickle changes
through gettext.m4 (which is frankly painful to contemplate: gettext.m4 has so
many other users and is so complicated...) we just modify the sedding that's
already done in the in-tree intl/configure.ac to sed the thing a bit
differently.

Doing this also lets us rip out some system-dependent hardwired gunge around
whether libintl comes from libc or not and replace it with using the results of
the checks intl/configure has already done.

Nothing seems to break: I've built and tested both binutils-gdb and gcc plus the
two libintl patches with --enable-shared and --disable-shared and with and
without --with-included-gettext on systems with gettext in libc, with gettext in
a system libintl and with no gettext anywhere.  Stephen has verified that MacOS 
X
(a system which had explicit system-dependent libintl gunge in
{bfd,opcodes,libctf}/configure.ac, all of which has gone away) gets further into
the build now (before failing in gdb for not-really-related reasons). Cygwin
(the other system with system-dependent gunge) has been able to have most of it
ripped out, and still builds as above.

Is this sort of thing acceptable? Probably for trunk only, unless people think
that the build failures on MacOS X merit backporting of this to the 2.36 release
branch.

(also in this series, some stuff not needing review: Tcl version checks for the
libctf testsuite and some small configury improvements to libctf.)

(All mails sent to binut...@sourceware.org: relevant subset Cc:ed to gdb and gcc
as well.)

Cc: gdb-patc...@sourceware.org
Cc: gcc-patc...@gnu.org

Jakub Jelinek (2): (sync from gcc)
  intl: Allow building both with old bison and bison >= 3 [PR92008]
  intl: Unbreak intl build with bison 3 when no regeneration is needed
[PR92008]

Nick Alcock (6):
  intl: always picify
  intl: turn LIBINTL into -L / -l form
  bfd, opcodes, libctf: support --with-included-gettext
  gdbserver: explicitly include gettext, etc in inprocess agent
  libctf: require a Tcl capable of try/catch to run tests
  libctf: add missing header in BFD ELF check

 bfd/configure |  11 ++--
 bfd/configure.ac  |  11 ++--
 gdbserver/Makefile.in |   2 +-
 intl/Makefile.in  |  16 --
 intl/aclocal.m4   |   1 +
 intl/configure| 113 +-
 intl/configure.ac |  30 ++-
 intl/plural-config.h  |   1 +
 intl/plural-exp.h |   8 ++-
 intl/plural.c |  62 +++
 intl/plural.y |  27 +-
 libctf/Makefile.am|   6 ++-
 libctf/Makefile.in|  91 +++---
 libctf/configure  |  91 +++---
 libctf/configure.ac   |  27 +++---
 opcodes/configure |   8 ++-
 opcodes/configure.ac  |   8 ++-
 17 files changed, 413 insertions(+), 100 deletions(-)
 create mode 100644 intl/plural-config.h

-- 
2.30.0.252.gc27e85e57d

[PATCH 3/8] intl: always picify

[Nick Alcock via Gcc-patches]

libintl is included in several shared libraries (at least
libinproctrace.so and libctf.so): unconditionally picify with code
borrowed from libiberty configure.  (It's not performance-critical, so
don't bother making separate PIC and non-PIC libraries like libiberty
does.)

Cc: gcc-patc...@gnu.org

intl/ChangeLog
2021-02-02  Nick Alcock  

* aclocal.m4: include picflag.m4.
* configure.ac (PICFLAG): Add and substitute.
* Makefile.in (PICFLAG): New.
(COMPILE): Use it.
* configure: Regenerate.
---
 intl/Makefile.in  |  3 +-
 intl/aclocal.m4   |  1 +
 intl/configure| 86 +++
 intl/configure.ac |  5 +++
 4 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/intl/Makefile.in b/intl/Makefile.in
index 356c8ab9b65..96df727baea 100644
--- a/intl/Makefile.in
+++ b/intl/Makefile.in
@@ -49,8 +49,9 @@ CFLAGS = @CFLAGS@
 LDFLAGS = @LDFLAGS@
 LIBS = @LIBS@
 DEFS = -DHAVE_CONFIG_H
+PICFLAG = @PICFLAG@
 
-COMPILE = $(CC) -c $(CPPFLAGS) $(CFLAGS) $(DEFS) $(DEFS-$@) $(INCLUDES)
+COMPILE = $(CC) -c $(CPPFLAGS) $(CFLAGS) $(PICFLAG) $(DEFS) $(DEFS-$@) 
$(INCLUDES)
 
 HEADERS = \
   gmo.h \
diff --git a/intl/aclocal.m4 b/intl/aclocal.m4
index 473ec622323..0a42b42ea8c 100644
--- a/intl/aclocal.m4
+++ b/intl/aclocal.m4
@@ -26,6 +26,7 @@ m4_include([../config/lib-link.m4])
 m4_include([../config/lib-prefix.m4])
 m4_include([../config/nls.m4])
 m4_include([../config/override.m4])
+m4_include([../config/picflag.m4])
 m4_include([../config/po.m4])
 m4_include([../config/progtest.m4])
 m4_include([../config/stdint_h.m4])
diff --git a/intl/configure b/intl/configure
index d69767b7d21..6498a392570 100755
--- a/intl/configure
+++ b/intl/configure
@@ -624,6 +624,7 @@ ac_subst_vars='LTLIBOBJS
 LIBOBJS
 BISON3_NO
 BISON3_YES
+PICFLAG
 INCINTL
 LIBINTL_DEP
 MAINT
@@ -6793,6 +6794,91 @@ case $USE_INCLUDED_LIBINTL in
 ;;
 esac
 
+# intl is sometimes linked into shared libraries even without --enable-shared
+# (e.g. gdbsupport's inprocess agent): so always PICify, just in case.
+
+
+
+
+case "${host}" in
+# PIC is the default on some targets or must not be used.
+*-*-darwin*)
+   # For darwin, common symbols are not allowed in MH_DYLIB files
+   case "${CFLAGS}" in
+ # If we are using a compiler supporting mdynamic-no-pic
+ # and the option has been tested as safe to add, then cancel
+ # it here, since the code generated is incompatible with shared
+ # libs.
+ *-mdynamic-no-pic*) PICFLAG='-fno-common -mno-dynamic-no-pic' ;;
+ *) PICFLAG=-fno-common ;;
+   esac
+   ;;
+alpha*-dec-osf5*)
+   # PIC is the default.
+   ;;
+hppa*64*-*-hpux*)
+   # PIC is the default for 64-bit PA HP-UX.
+   ;;
+i[34567]86-*-cygwin* | x86_64-*-cygwin*)
+   ;;
+i[34567]86-*-mingw* | x86_64-*-mingw*)
+   ;;
+i[34567]86-*-interix[3-9]*)
+   # Interix 3.x gcc -fpic/-fPIC options generate broken code.
+   # Instead, we relocate shared libraries at runtime.
+   ;;
+i[34567]86-*-nto-qnx*)
+   # QNX uses GNU C++, but need to define -shared option too, otherwise
+   # it will coredump.
+   PICFLAG='-fPIC -shared'
+   ;;
+i[34567]86-pc-msdosdjgpp*)
+   # DJGPP does not support shared libraries at all.
+   ;;
+ia64*-*-hpux*)
+   # On IA64 HP-UX, PIC is the default but the pic flag
+   # sets the default TLS model and affects inlining.
+   PICFLAG=-fPIC
+   ;;
+mips-sgi-irix6*)
+   # PIC is the default.
+   ;;
+rs6000-ibm-aix* | powerpc-ibm-aix*)
+   # All AIX code is PIC.
+   ;;
+
+# Some targets support both -fPIC and -fpic, but prefer the latter.
+# FIXME: Why?
+i[34567]86-*-* | x86_64-*-*)
+   PICFLAG=-fpic
+   ;;
+# FIXME: Override -fPIC default in libgcc only?
+sh-*-linux* | sh[2346lbe]*-*-linux*)
+   PICFLAG=-fpic
+   ;;
+# FIXME: Simplify to sh*-*-netbsd*?
+sh-*-netbsdelf* | shl*-*-netbsdelf* | sh5-*-netbsd* | sh5l*-*-netbsd* | \
+  sh64-*-netbsd* | sh64l*-*-netbsd*)
+   PICFLAG=-fpic
+   ;;
+# Default to -fPIC unless specified otherwise.
+*)
+   PICFLAG=-fPIC
+   ;;
+esac
+
+# If the user explicitly uses -fpic/-fPIC, keep that.
+case "${CFLAGS}" in
+*-fpic*)
+   PICFLAG=-fpic
+   ;;
+*-fPIC*)
+   PICFLAG=-fPIC
+   ;;
+esac
+
+
+
 BISON3_YES='#'
 BISON3_NO=
 if test "$INTLBISON" != :; then
diff --git a/intl/configure.ac b/intl/configure.ac
index 6363e55e68a..5ec7b0944e2 100644
--- a/intl/configure.ac
+++ b/intl/configure.ac
@@ -47,6 +47,11 @@ case $USE_INCLUDED_LIBINTL in
 ;;
 esac
 
+# intl is sometimes linked into shared libraries even without --enable-shared
+# (e.g. gdbsupport's inprocess agent): so always PICify, just in case.
+GCC_PICFLAG
+AC_SUBST(PICFLAG)
+
 BISON3_YES='#'
 BISON3_NO=
 if test "$INTLBISON" != :; then
-- 
2.30.0.252.gc27e85e57d

[PATCH 4/8] intl: turn LIBINTL into -L / -l form

[Nick Alcock via Gcc-patches]

This variable currently refers directly, not to a .la file, but to an .a
file.  This produces wrong results when building into a library on some
platforms: so convert it to the general form "-L${top_builddir}../intl
-lintl ..." ... so that both libtool and non-libtool builds will always
do the right thing for both static and shared links.

Cc: gcc-patc...@gnu.org

intl/ChangeLog
2021-02-04  Nick Alcock  

* configure.ac (LIBINTL): Transform into -L/-lintl form.
* configure: Regenerate.
---
 intl/configure| 3 +--
 intl/configure.ac | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/intl/configure b/intl/configure
index 6498a392570..7ddb624249d 100755
--- a/intl/configure
+++ b/intl/configure
@@ -6787,8 +6787,7 @@ LIBINTL_DEP=
 INCINTL=
 case $USE_INCLUDED_LIBINTL in
   yes)
-LIBINTL=`echo $LIBINTL | sed 's,${top_builddir},&/..,' `
-LTLIBINTL=`echo $LTLIBINTL | sed 's,${top_builddir},&/..,' `
+LIBINTL=`echo $LIBINTL | sed 's,${top_builddir},-L&/..,; 
s,\.\./intl/libintl\.a,../intl -lintl,' `
 LIBINTL_DEP='${top_builddir}/../intl/libintl.a'
 INCINTL='-I${top_builddir}/../intl'
 ;;
diff --git a/intl/configure.ac b/intl/configure.ac
index 5ec7b0944e2..77e2fd2d8c5 100644
--- a/intl/configure.ac
+++ b/intl/configure.ac
@@ -40,8 +40,7 @@ LIBINTL_DEP=
 INCINTL=
 case $USE_INCLUDED_LIBINTL in
   yes)
-LIBINTL=`echo $LIBINTL | sed 's,${top_builddir},&/..,' `
-LTLIBINTL=`echo $LTLIBINTL | sed 's,${top_builddir},&/..,' `
+LIBINTL=`echo $LIBINTL | sed 's,${top_builddir},-L&/..,; 
s,\.\./intl/libintl\.a,../intl -lintl,' `
 LIBINTL_DEP='${top_builddir}/../intl/libintl.a'
 INCINTL='-I${top_builddir}/../intl'
 ;;
-- 
2.30.0.252.gc27e85e57d

Re: [PATCH] mklog: automatically fill in generated entries

[Martin Liška]

On 2/7/21 9:20 PM, Mike Frysinger via Gcc-patches wrote:

contrib/ChangeLog:

* mklog.py (generated_files): New set.
(generate_changelog): Add entries based on generated_files.
---
  contrib/mklog.py | 5 +
  1 file changed, 5 insertions(+)

diff --git a/contrib/mklog.py b/contrib/mklog.py
index a70536a6849a..6509886741d7 100755
--- a/contrib/mklog.py
+++ b/contrib/mklog.py
@@ -55,6 +55,9 @@ bugzilla_url = 
'https://gcc.gnu.org/bugzilla/rest.cgi/bug?id=%s&;' \
  
  function_extensions = {'.c', '.cpp', '.C', '.cc', '.h', '.inc', '.def', '.md'}
  
+# NB: Makefile.in isn't listed as it's not always generated.

+generated_files = {'aclocal.m4', 'config.h.in', 'configure'}
+
  help_message = """\
  Generate ChangeLog template for PATCH.
  PATCH must be generated using diff(1)'s -up or -cp options
@@ -192,6 +195,8 @@ def generate_changelog(data, no_functions=False, 
fill_pr_titles=False):
  if new_path.startswith(changelog):
  new_path = new_path[len(changelog):].lstrip('/')
  out += '\t* %s: ...here.\n' % (new_path)
+elif os.path.basename(file.path) in generated_files:
+out += '\t* %s: Regenerate.\n' % (relative_path)
  else:
  if not no_functions:
  for hunk in file:



Thanks for the patch, it's fine.

Please install it.
Martin

Re: [PATCH, rs6000, expand, hooks]: Fix PR98872, handle uninitialized opaque mode variables

[Richard Sandiford via Gcc-patches]

Peter Bergner  writes:
> Adding Richard since he's reviewed the generic opaque mode code in
> the past and this patch contains some more eneric support.
>
> GCC handles pseudos that are used uninitialized, by emitting a
> (set (reg: ) CONST0_RTX(regmode)) before their uninitialized
> pseudo usage.  Currently, CONST0_RTX(mode) is NULL for opaque modes,
> which leads to an ICE.  The solution is to enhance init_emit_once() to
> add initialization of CONST0_RTX for opaque modes using a target hook,
> since CONST0_RTX probably are different for each opaque mode and each
> target.  The default hook throws an error to force the target to think
> hard about what their CONST0_RTX values should be for each mode.

Hmm, but it looks like the hook uses const0_rtx, i.e. a const_int,
for something that isn't an integer mode.  Also, the unspec for XOmode
isn't a constant in the normal sense (CONSTANT_P).

I think we should either add a new rtx code for constant opaque modes
or make init-regs just emit the clobber for opaque modes (and not emit
the move).

Thanks,
Richard

>
> This passed bootstrap and regtesting on x86_64-linux and powerpc64le-linux
> with no regressions.  Ok for mainline?
>
> Peter
>
>
> gcc/
>   PR target/98872
>   * config/rs6000/mma.md (*movoo): Accept zero constraint.
>   (mma_xxsetaccz): Use CONST0_RTX.
>   * config/rs6000/predicates.md: Recognize OOmode CONST0_RTX.
>   * config/rs6000/rs6000.c (TARGET_OPAQUE_CONST0_RTX): Define.
>   (rs6000_split_multireg_move): Handle splitting an OOmode register
>   set to CONST0_RTX.
>   (rs6000_opaque_const0_rtx): New function.
>   * emit-rtl.c (init_emit_once): Initialize CONST0_RTX for opaque modes.
>   * hooks.c (hook_rtx_mode_unreachable): New function.
>   * hooks.h (hook_rtx_mode_unreachable): Declare
>   * target.def (opaque_const0_rtx): New target hook.
>   * doc/tm.texi.in: Document it.
>   * doc/tm.texi: Regenerate.
>
> gcc/testsuite/
>   * gcc.target/powerpc/pr98872.c: New test.
>
> diff --git a/gcc/config/rs6000/mma.md b/gcc/config/rs6000/mma.md
> index 87569f1c31d..fab849a4f12 100644
> --- a/gcc/config/rs6000/mma.md
> +++ b/gcc/config/rs6000/mma.md
> @@ -272,7 +272,7 @@
>  
>  (define_insn_and_split "*movoo"
>[(set (match_operand:OO 0 "nonimmediate_operand" "=wa,m,wa")
> - (match_operand:OO 1 "input_operand" "m,wa,wa"))]
> + (match_operand:OO 1 "input_operand" "m,wa,waO"))]
>"TARGET_MMA
> && (gpc_reg_operand (operands[0], OOmode)
> || gpc_reg_operand (operands[1], OOmode))"
> @@ -473,9 +473,7 @@
>   (const_int 0))]
>"TARGET_MMA"
>  {
> -  rtx xo0 = gen_rtx_UNSPEC (XOmode, gen_rtvec (1, const0_rtx),
> - UNSPEC_MMA_XXSETACCZ);
> -  emit_insn (gen_rtx_SET (operands[0], xo0));
> +  emit_insn (gen_rtx_SET (operands[0], CONST0_RTX (XOmode)));
>DONE;
>  })
>  
> diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
> index 5d1952e59d3..30805ab0619 100644
> --- a/gcc/config/rs6000/predicates.md
> +++ b/gcc/config/rs6000/predicates.md
> @@ -1081,6 +1081,10 @@
>&& easy_vector_constant (op, mode))
>  return 1;
>  
> +  /* For OOmode, zero is an easy constant.  */
> +  if (mode == OOmode && op == CONST0_RTX (mode))
> +return 1;
> +
>/* For floating-point or multi-word mode, the only remaining valid type
>   is a register.  */
>if (SCALAR_FLOAT_MODE_P (mode)
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index f5565a1a253..c726aa09d26 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -1752,6 +1752,10 @@ static const struct attribute_spec 
> rs6000_attribute_table[] =
>  
>  #undef TARGET_INVALID_CONVERSION
>  #define TARGET_INVALID_CONVERSION rs6000_invalid_conversion
> +
> +#undef TARGET_OPAQUE_CONST0_RTX
> +#define TARGET_OPAQUE_CONST0_RTX rs6000_opaque_const0_rtx
> +
>  
>  
>  /* Processor table.  */
> @@ -16624,6 +16628,19 @@ rs6000_split_multireg_move (rtx dst, rtx src)
> return;
>   }
>  
> +  /* Split the clearing of an OOmode register pair into clearing
> +  of its two constituent registers.  */
> +  if (REG_P (dst) && mode == OOmode && src == CONST0_RTX (mode))
> + {
> +   int regno = REGNO (dst);
> +   gcc_assert (VSX_REGNO_P (regno));
> +   emit_insn (gen_rtx_SET (gen_rtx_REG (reg_mode, regno),
> +   CONST0_RTX (reg_mode)));
> +   emit_insn (gen_rtx_SET (gen_rtx_REG (reg_mode, regno + 1),
> +   CONST0_RTX (reg_mode)));
> +   return;
> + }
> +
>/* Register -> register moves can use common code.  */
>  }
>  
> @@ -27477,6 +27494,25 @@ rs6000_output_addr_vec_elt (FILE *file, int value)
>fprintf (file, "\n");
>  }
>  
> +/* Implement TARGET_OPAQUE_CONST0_RTX.  */
> +
> +rtx
> +rs6000_opaque_const0_rtx (machine_mode mode)
> +{
> +  gcc_assert (OPAQUE_MODE_P (mode));
> +
> +  switch (mode)
> +{
> +

[PATCH] x86: Always save and restore shadow stack pointer

[H.J. Lu via Gcc-patches]

When the SHSTK feature is not available or not enabled, RDSSP is a NOP,
always save and restore shadow stack pointer to support compiling source
codes, containing __builtin_setjmp and __builtin_longjmp, with different
-fcf-protection options.

PR target/98997
* config/i386/i386.md (save_stack_nonlocal): Always save shadow
stack pointer.
(restore_stack_nonlocal): Always restore shadow stack pointer.
(@rdssp): Make it unconditional.
(@incssp): Likewise.
---
 gcc/config/i386/i386.md | 150 ++--
 1 file changed, 68 insertions(+), 82 deletions(-)

diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index b60784a2908..2510bd8a73d 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -19398,21 +19398,16 @@ (define_expand "save_stack_nonlocal"
 (match_operand 1 "register_operand"))]
   ""
 {
-  rtx stack_slot;
-
-  if (flag_cf_protection & CF_RETURN)
-{
-  /* Copy shadow stack pointer to the first slot
-and stack pointer to the second slot.  */
-  rtx ssp_slot = adjust_address (operands[0], word_mode, 0);
-  stack_slot = adjust_address (operands[0], Pmode, UNITS_PER_WORD);
-
-  rtx reg_ssp = force_reg (word_mode, const0_rtx);
-  emit_insn (gen_rdssp (word_mode, reg_ssp, reg_ssp));
-  emit_move_insn (ssp_slot, reg_ssp);
-}
-  else
-stack_slot = adjust_address (operands[0], Pmode, 0);
+  /* Copy shadow stack pointer to the first slot and stack pointer to
+ the second slot.  */
+  rtx ssp_slot = adjust_address (operands[0], word_mode, 0);
+  rtx stack_slot = adjust_address (operands[0], Pmode, UNITS_PER_WORD);
+
+  /* If the SHSTK feature is not available or not enabled, the RDSSP
+ instruction is a NOP and REG_SSP is 0.  */
+  rtx reg_ssp = force_reg (word_mode, const0_rtx);
+  emit_insn (gen_rdssp (word_mode, reg_ssp, reg_ssp));
+  emit_move_insn (ssp_slot, reg_ssp);
   emit_move_insn (stack_slot, operands[1]);
   DONE;
 })
@@ -19422,72 +19417,63 @@ (define_expand "restore_stack_nonlocal"
(match_operand 1 "memory_operand" ""))]
   ""
 {
-  rtx stack_slot;
+  /* Restore shadow stack pointer from the first slot and stack pointer
+ from the second slot.  */
+  rtx ssp_slot = adjust_address (operands[1], word_mode, 0);
+  rtx stack_slot = adjust_address (operands[1], Pmode, UNITS_PER_WORD);
 
-  if (flag_cf_protection & CF_RETURN)
-{
-  /* Restore shadow stack pointer from the first slot
-and stack pointer from the second slot.  */
-  rtx ssp_slot = adjust_address (operands[1], word_mode, 0);
-  stack_slot = adjust_address (operands[1], Pmode, UNITS_PER_WORD);
-
-  /* Get the current shadow stack pointer.  The code below will check if
-SHSTK feature is enabled.  If it is not enabled the RDSSP instruction
-is a NOP.  */
-  rtx reg_ssp = force_reg (word_mode, const0_rtx);
-  emit_insn (gen_rdssp (word_mode, reg_ssp, reg_ssp));
-
-  /* Compare through subtraction the saved and the current ssp
-to decide if ssp has to be adjusted.  */
-  reg_ssp = expand_simple_binop (word_mode, MINUS,
-reg_ssp, ssp_slot,
-reg_ssp, 1, OPTAB_DIRECT);
-
-  /* Compare and jump over adjustment code.  */
-  rtx noadj_label = gen_label_rtx ();
-  emit_cmp_and_jump_insns (reg_ssp, const0_rtx, EQ, NULL_RTX,
-  word_mode, 1, noadj_label);
-
-  /* Compute the number of frames to adjust.  */
-  rtx reg_adj = gen_lowpart (ptr_mode, reg_ssp);
-  rtx reg_adj_neg = expand_simple_unop (ptr_mode, NEG, reg_adj,
-   NULL_RTX, 1);
-
-  reg_adj = expand_simple_binop (ptr_mode, LSHIFTRT, reg_adj_neg,
-GEN_INT (exact_log2 (UNITS_PER_WORD)),
-reg_adj, 1, OPTAB_DIRECT);
-
-  /* Check if number of frames <= 255 so no loop is needed.  */
-  rtx inc_label = gen_label_rtx ();
-  emit_cmp_and_jump_insns (reg_adj, GEN_INT (255), LEU, NULL_RTX,
-  ptr_mode, 1, inc_label);
-
-  /* Adjust the ssp in a loop.  */
-  rtx loop_label = gen_label_rtx ();
-  emit_label (loop_label);
-  LABEL_NUSES (loop_label) = 1;
-
-  rtx reg_255 = force_reg (word_mode, GEN_INT (255));
-  emit_insn (gen_incssp (word_mode, reg_255));
-
-  reg_adj = expand_simple_binop (ptr_mode, MINUS,
-reg_adj, GEN_INT (255),
-reg_adj, 1, OPTAB_DIRECT);
-
-  /* Compare and jump to the loop label.  */
-  emit_cmp_and_jump_insns (reg_adj, GEN_INT (255), GTU, NULL_RTX,
-  ptr_mode, 1, loop_label);
-
-  emit_label (inc_label);
-  LABEL_NUSES (inc_label) = 1;
-
-  emit_insn (gen_incssp (word_mode, reg_ssp));
-
-  emit_label (noadj_label);
-  L

[RFC] ldist: Recognize rawmemchr loop patterns

[Stefan Schulze Frielinghaus via Gcc-patches]

This patch adds support for recognizing loops which mimic the behaviour
of function rawmemchr, and replaces those with an internal function call
in case a target provides them.  In contrast to the original rawmemchr
function, this patch also supports different instances where the memory
pointed to and the pattern are interpreted as 8, 16, and 32 bit sized,
respectively.

This patch is not final and I'm looking for some feedback:

Previously, only loops which mimic the behaviours of functions memset,
memcpy, and memmove have been detected and replaced by corresponding
function calls.  One characteristic of those loops/partitions is that
they don't have a reduction.  In contrast, loops which mimic the
behaviour of rawmemchr compute a result and therefore have a reduction.
My current attempt is to ensure that the reduction statement is not used
in any other partition and only in that case ignore the reduction and
replace the loop by a function call.  We then only need to replace the
reduction variable of the loop which contained the loop result by the
variable of the lhs of the internal function call.  This should ensure
that the transformation is correct independently of how partitions are
fused/distributed in the end.  Any thoughts about this?

Furthermore, I simply added two new members (pattern, fn) to structure
builtin_info which I consider rather hacky.  For the long run I thought
about to split up structure builtin_info into a union where each member
is a structure for a particular builtin of a partition, i.e., something
like this:

union builtin_info
{
  struct binfo_memset *memset;
  struct binfo_memcpymove *memcpymove;
  struct binfo_rawmemchr *rawmemchr;
};

Such that a structure for one builtin does not get "polluted" by a
different one.  Any thoughts about this?

Cheers,
Stefan
---
 gcc/internal-fn.c|  42 ++
 gcc/internal-fn.def  |   3 +
 gcc/target-insns.def |   3 +
 gcc/tree-loop-distribution.c | 257 ++-
 4 files changed, 272 insertions(+), 33 deletions(-)

diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
index dd7173126fb..9cd62544a1a 100644
--- a/gcc/internal-fn.c
+++ b/gcc/internal-fn.c
@@ -2917,6 +2917,48 @@ expand_VEC_CONVERT (internal_fn, gcall *)
   gcc_unreachable ();
 }
 
+static void
+expand_RAWMEMCHR8 (internal_fn, gcall *stmt)
+{
+  if (targetm.have_rawmemchr8 ())
+{
+  rtx result = expand_expr (gimple_call_lhs (stmt), NULL_RTX, VOIDmode, 
EXPAND_WRITE);
+  rtx start = expand_normal (gimple_call_arg (stmt, 0));
+  rtx pattern = expand_normal (gimple_call_arg (stmt, 1));
+  emit_insn (targetm.gen_rawmemchr8 (result, start, pattern));
+}
+  else
+gcc_unreachable();
+}
+
+static void
+expand_RAWMEMCHR16 (internal_fn, gcall *stmt)
+{
+  if (targetm.have_rawmemchr16 ())
+{
+  rtx result = expand_expr (gimple_call_lhs (stmt), NULL_RTX, VOIDmode, 
EXPAND_WRITE);
+  rtx start = expand_normal (gimple_call_arg (stmt, 0));
+  rtx pattern = expand_normal (gimple_call_arg (stmt, 1));
+  emit_insn (targetm.gen_rawmemchr16 (result, start, pattern));
+}
+  else
+gcc_unreachable();
+}
+
+static void
+expand_RAWMEMCHR32 (internal_fn, gcall *stmt)
+{
+  if (targetm.have_rawmemchr32 ())
+{
+  rtx result = expand_expr (gimple_call_lhs (stmt), NULL_RTX, VOIDmode, 
EXPAND_WRITE);
+  rtx start = expand_normal (gimple_call_arg (stmt, 0));
+  rtx pattern = expand_normal (gimple_call_arg (stmt, 1));
+  emit_insn (targetm.gen_rawmemchr32 (result, start, pattern));
+}
+  else
+gcc_unreachable();
+}
+
 /* Expand the IFN_UNIQUE function according to its first argument.  */
 
 static void
diff --git a/gcc/internal-fn.def b/gcc/internal-fn.def
index daeace7a34e..34247859704 100644
--- a/gcc/internal-fn.def
+++ b/gcc/internal-fn.def
@@ -348,6 +348,9 @@ DEF_INTERNAL_FN (MUL_OVERFLOW, ECF_CONST | ECF_LEAF | 
ECF_NOTHROW, NULL)
 DEF_INTERNAL_FN (TSAN_FUNC_EXIT, ECF_NOVOPS | ECF_LEAF | ECF_NOTHROW, NULL)
 DEF_INTERNAL_FN (VA_ARG, ECF_NOTHROW | ECF_LEAF, NULL)
 DEF_INTERNAL_FN (VEC_CONVERT, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
+DEF_INTERNAL_FN (RAWMEMCHR8, ECF_PURE | ECF_LEAF | ECF_NOTHROW, NULL)
+DEF_INTERNAL_FN (RAWMEMCHR16, ECF_PURE | ECF_LEAF | ECF_NOTHROW, NULL)
+DEF_INTERNAL_FN (RAWMEMCHR32, ECF_PURE | ECF_LEAF | ECF_NOTHROW, NULL)
 
 /* An unduplicable, uncombinable function.  Generally used to preserve
a CFG property in the face of jump threading, tail merging or
diff --git a/gcc/target-insns.def b/gcc/target-insns.def
index 672c35698d7..9248554cbf3 100644
--- a/gcc/target-insns.def
+++ b/gcc/target-insns.def
@@ -106,3 +106,6 @@ DEF_TARGET_INSN (trap, (void))
 DEF_TARGET_INSN (unique, (void))
 DEF_TARGET_INSN (untyped_call, (rtx x0, rtx x1, rtx x2))
 DEF_TARGET_INSN (untyped_return, (rtx x0, rtx x1))
+DEF_TARGET_INSN (rawmemchr8, (rtx x0, rtx x1, rtx x2))
+DEF_TARGET_INSN (rawmemchr16, (rtx x0, rtx x1, rtx x2))
+DEF_TARGET_INSN (rawmemchr32, (rtx x0, rtx x1

Re: [PATCH, rs6000, expand, hooks]: Fix PR98872, handle uninitialized opaque mode variables

[Segher Boessenkool]

Hi!

On Mon, Feb 08, 2021 at 12:38:01PM +, Richard Sandiford wrote:
> Peter Bergner  writes:
> > Adding Richard since he's reviewed the generic opaque mode code in
> > the past and this patch contains some more eneric support.
> >
> > GCC handles pseudos that are used uninitialized, by emitting a
> > (set (reg: ) CONST0_RTX(regmode)) before their uninitialized
> > pseudo usage.  Currently, CONST0_RTX(mode) is NULL for opaque modes,
> > which leads to an ICE.  The solution is to enhance init_emit_once() to
> > add initialization of CONST0_RTX for opaque modes using a target hook,
> > since CONST0_RTX probably are different for each opaque mode and each
> > target.  The default hook throws an error to force the target to think
> > hard about what their CONST0_RTX values should be for each mode.
> 
> Hmm, but it looks like the hook uses const0_rtx, i.e. a const_int,
> for something that isn't an integer mode.  Also, the unspec for XOmode
> isn't a constant in the normal sense (CONSTANT_P).
> 
> I think we should either add a new rtx code for constant opaque modes
> or make init-regs just emit the clobber for opaque modes (and not emit
> the move).

Thanks for looking Richard.  That last option sounds good to me as well.

Some comments on the patch:

> > --- a/gcc/config/rs6000/mma.md
> > +++ b/gcc/config/rs6000/mma.md
> > @@ -473,9 +473,7 @@

Please look at the commentary in $GCCSRC/.gitattributes on how to get
context shown in .md diffs (it needs a local configuration step).

> > @@ -16624,6 +16628,19 @@ rs6000_split_multireg_move (rtx dst, rtx src)
> > +  /* Split the clearing of an OOmode register pair into clearing
> > +of its two constituent registers.  */
> > +  if (REG_P (dst) && mode == OOmode && src == CONST0_RTX (mode))
> > +   {
> > + int regno = REGNO (dst);
> > + gcc_assert (VSX_REGNO_P (regno));
> > + emit_insn (gen_rtx_SET (gen_rtx_REG (reg_mode, regno),
> > + CONST0_RTX (reg_mode)));
> > + emit_insn (gen_rtx_SET (gen_rtx_REG (reg_mode, regno + 1),
> > + CONST0_RTX (reg_mode)));
> > + return;
> > +   }

That is fine.

> > +/* Implement TARGET_OPAQUE_CONST0_RTX.  */
> > +
> > +rtx
> > +rs6000_opaque_const0_rtx (machine_mode mode)
> > +{
> > +  gcc_assert (OPAQUE_MODE_P (mode));
> > +
> > +  switch (mode)
> > +{
> > +case E_OOmode:
> > +  return const0_rtx;
> > +case E_XOmode:
> > +  return gen_rtx_UNSPEC (XOmode, gen_rtvec (1, const0_rtx),
> > +UNSPEC_MMA_XXSETACCZ);
> > +default:
> > +  gcc_unreachable ();
> > +}
> > +}

Why are XO and OO handled in different ways?  This needs a comment, or
better, not be handled in different ways ;-)

> > --- a/gcc/emit-rtl.c
> > +++ b/gcc/emit-rtl.c
> > @@ -6408,6 +6408,11 @@ init_emit_once (void)
> >  if (GET_MODE_CLASS ((machine_mode) i) == MODE_CC)
> >const_tiny_rtx[0][i] = const0_rtx;
> >  
> > +  FOR_EACH_MODE_IN_CLASS (mode, MODE_OPAQUE)
> > +{
> > +  const_tiny_rtx[0][(int) mode] = targetm.opaque_const0_rtx (mode);
> > +}

This does not sound like a good idea, it is asking for trouble imo.

> > +rtx
> > +hook_rtx_mode_unreachable (machine_mode mode ATTRIBUTE_UNUSED)
> > +{
> > +  gcc_unreachable ();
> > +}

If you want to use this in any hook, that hook needs documentation that
it can not be used on some targets (an ICE is not acceptable from a UI
point of view, in almost all cases).

> > +DEFHOOK
> > +(opaque_const0_rtx,
> > + "Return an RTX representing the value @code{0} for opaque mode 
> > @var{mode}.\n\
> > +The default version of this hook always throws an error.",

So that documentation needs a severe warning in it.

> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/powerpc/pr98872.c
> > @@ -0,0 +1,20 @@
> > +/* PR target/98872 */
> > +/* { dg-do compile } */
> > +/* { dg-require-effective-target power10_ok } */
> > +/* { dg-options "-O2 -mdejagnu-cpu=power10" } */
> > +
> > +/* Verify we do not ICE on the tests below.  */

Do the existing tests already check the expected code for this?


I would expect the code that initialises uninitialised values to handle
this, instead (possibly call the same hook, but :-) )


Segher

Re: [PATCH 3/4] openacc: Fix lowering for derived-type mappings through array elements

[Tobias Burnus]

On 06.02.21 11:49, Julian Brown wrote:


if (n->expr)
  for (gfc_ref *ref = n->expr->ref; ref; ref = ref->next)
-   if (ref->type == REF_COMPONENT || ref->type == REF_ARRAY)
+   if (ref->type == REF_COMPONENT)
  lastref = ref;
+   else if (ref->type == REF_ARRAY)


This one fails to build:

../../repos/gcc/gcc/fortran/trans-openmp.c: In function ‘tree_node* 
gfc_trans_omp_clauses(stmtblock_t*, gfc_omp_clauses*, locus, bool, bool)’:
../../repos/gcc/gcc/fortran/trans-openmp.c:2681:18: error: suggest explicit 
braces to avoid ambiguous ‘else’ [-Werror=dangling-else]
 2681 |   if (n->expr)
  |  ^


Previous patch in this 3/4 thread:

+ && !lastref->u.c.component->attr.dimension) + { + /* Derived type
access with last component being a scalar. */


I was wondering whether that causes any issues with local access to coarrays,
but that seems to work (compile with -fcoarray=single or -fcoarray=lib 
-lcaf_single);
I have not check the dump whether it indeed works.

(The interesting case is -fcoarray=lib; all those accesses go
to the local variable – remote access (like A[image_idx]) is
correctly rejected with 'List item shall not be coindexed'
or with a parse error.)

implicit none
type t2
  integer :: x, y
end type t2
type t
  type(t2), allocatable :: B[:], BB(:)[:]
end type t
type(t) :: var
type(t2), allocatable :: A[:], AA(:)[:]
type(t2) :: C[*], CC(10)[*]

!$acc update self(var%B, var%BB)
!$acc update self(var%BB(1))
!$acc update self(var%B%x)
!$acc update self(var%BB%x)
!$acc update self(var%BB(1)%x)
!$acc update self(A, AA)
!$acc update self(AA(1))
!$acc update self(A%x, AA%y)
!$acc update self(AA(1)%y)
!$acc update self(C, CC)
!$acc update self(CC(1))
!$acc update self(C%x, CC%y)
!$acc update self(CC(1)%y)
end



else - sorry ("unhandled derived-type component"); + sorry ("unhandled
expression type");


Nit: That's not an expression type but a reference type. Remaining
are: REF_SUBSTRING and REF_INQUIRY. I wonder whether that should be 
'gcc_unreachable' or 'internal_error'
but 'sorry' does not make sense. I think that message occurs twice.
Otherwise, it looks good to me. Tobias PS: I have a follow-up patch
related to %re/%im or %kind, to be submitted later today.


-
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München 
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank 
Thürauf

[PATCH, OG10, committed] Support A->B expressions in map clause

[Chung-Lin Tang]

This patch tries to allow map(A->ptr) to be properly handled the same way as
map(B.ptr) expressions. map(struct:*A) clauses are now produced during
gimplify.

Julian, I'm CCing you since IIRC you seemed to be the author of this area of
code. Would appreciate if you gave a look if you have time, though I've already
went ahead and pushed to OG10 after testing results looked okay.

Thanks,
Chung-Lin

gcc/ChangeLog:

* gimplify.c ("tree-hash-traits.h"): Add include.
(gimplify_scan_omp_clauses): Change struct_map_to_clause to type
hash_map *. Adjust struct map handling to handle
cases of *A and A->B expressions.
(gimplify_adjust_omp_clauses): Move GOMP_MAP_STRUCT removal code for
exit data directives code to earlier position.

gcc/testsuite/ChangeLog:

* g++.dg/gomp/target-3.C: Adjust testcase gimple scanning.
* g++.dg/gomp/target-this-2.C: Likewise.
* g++.dg/gomp/target-this-3.C: Likewise.
* g++.dg/gomp/target-this-4.C: Likewise.

libgomp/ChangeLog:

* testsuite/libgomp.c++/target-23.C: New testcase.
From bf8605f14ec33ea31233a3567f3184fee667b695 Mon Sep 17 00:00:00 2001
From: Chung-Lin Tang 
Date: Mon, 8 Feb 2021 07:53:55 -0800
Subject: [PATCH] Enable gimplify GOMP_MAP_STRUCT handling of (COMPONENT_REF
 (INDIRECT_REF ...)) map clauses.

This patch tries to allow map(A->ptr) to be properly handled the same way as
map(B.ptr) expressions. map(struct:*A) clauses are now produced during
gimplify.

This patch, as of time of commit, is only pushed to devel/omp/gcc-10, not yet
submitted as mainline patch to upstream.

2021-02-08  Chung-Lin Tang  

gcc/ChangeLog:

* gimplify.c ("tree-hash-traits.h"): Add include.
(gimplify_scan_omp_clauses): Change struct_map_to_clause to type
hash_map *. Adjust struct map handling to handle
cases of *A and A->B expressions.
(gimplify_adjust_omp_clauses): Move GOMP_MAP_STRUCT removal code for
exit data directives code to earlier position.

gcc/testsuite/ChangeLog:

* g++.dg/gomp/target-3.C: Adjust testcase gimple scanning.
* g++.dg/gomp/target-this-2.C: Likewise.
* g++.dg/gomp/target-this-3.C: Likewise.
* g++.dg/gomp/target-this-4.C: Likewise.

libgomp/ChangeLog:

* testsuite/libgomp.c++/target-23.C: New testcase.
---
 gcc/gimplify.c| 51 +++
 gcc/testsuite/g++.dg/gomp/target-3.C  |  2 +-
 gcc/testsuite/g++.dg/gomp/target-this-2.C |  2 +-
 gcc/testsuite/g++.dg/gomp/target-this-3.C |  2 +-
 gcc/testsuite/g++.dg/gomp/target-this-4.C |  4 +--
 libgomp/testsuite/libgomp.c++/target-23.C | 34 +
 6 files changed, 78 insertions(+), 17 deletions(-)
 create mode 100644 libgomp/testsuite/libgomp.c++/target-23.C

diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index b90ba5b..ba19017 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -53,6 +53,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "langhooks.h"
 #include "tree-cfg.h"
 #include "tree-ssa.h"
+#include "tree-hash-traits.h"
 #include "omp-general.h"
 #include "omp-low.h"
 #include "gimple-low.h"
@@ -8514,7 +8515,7 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq 
*pre_p,
 {
   struct gimplify_omp_ctx *ctx, *outer_ctx;
   tree c;
-  hash_map *struct_map_to_clause = NULL;
+  hash_map *struct_map_to_clause = NULL;
   hash_set *struct_deref_set = NULL;
   tree *prev_list_p = NULL, *orig_list_p = list_p;
   int handled_depend_iterators = -1;
@@ -9082,12 +9083,15 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq 
*pre_p,
  && TREE_CODE (decl) == INDIRECT_REF
  && TREE_CODE (TREE_OPERAND (decl, 0)) == COMPONENT_REF
  && (TREE_CODE (TREE_TYPE (TREE_OPERAND (decl, 0)))
- == REFERENCE_TYPE))
+ == REFERENCE_TYPE)
+ && (OMP_CLAUSE_MAP_KIND (c)
+ != GOMP_MAP_POINTER_TO_ZERO_LENGTH_ARRAY_SECTION))
{
  pd = &TREE_OPERAND (decl, 0);
  decl = TREE_OPERAND (decl, 0);
}
  bool indir_p = false;
+ bool component_ref_p = false;
  tree orig_decl = decl;
  tree decl_ref = NULL_TREE;
  if ((region_type & (ORT_ACC | ORT_TARGET | ORT_TARGET_DATA)) != 0
@@ -9098,6 +9102,7 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq 
*pre_p,
  while (TREE_CODE (decl) == COMPONENT_REF)
{
  decl = TREE_OPERAND (decl, 0);
+ component_ref_p = true;
  if (((TREE_CODE (decl) == MEM_REF
&& integer_zerop (TREE_OPERAND (decl, 1)))
   || INDIRECT_REF_P (decl))
@@ -9117,8 +9122,11 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq 
*pre_p,
}
}
}
-

[PING] Add conversions between _Float128 and Decimal.

[Michael Meissner via Gcc-patches]

Ping patch.  This really needs to go in to allow switching the long double type
to IEEE 128-bit.

https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564486.html
| Subject: [PATCH] Add conversions between _Float128 and Decimal.
| Message-ID: <20210129024208.ga25...@ibm-toto.the-meissners.org>

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

[pushed] c++: Fix typo in CLASSTYPE_TI_TEMPLATE comment.

[Marek Polacek via Gcc-patches]

gcc/cp/ChangeLog:

* cp-tree.h (CLASSTYPE_TI_TEMPLATE): Fix typo.
---
 gcc/cp/cp-tree.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 970ed5e77bb..41472a895b5 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -3775,8 +3775,8 @@ struct GTY(()) lang_decl {
 
   template  struct S {};
   template  struct S {};
-  
-   the CLASSTPYE_TI_TEMPLATE for S will be S, not the S.
+
+   the CLASSTYPE_TI_TEMPLATE for S will be S, not the S.
 
For a member class template, CLASSTYPE_TI_TEMPLATE always refers to the
partial instantiation rather than the primary template.  CLASSTYPE_TI_ARGS

base-commit: 40c92180df970143249f3cd5056f8fb48a4d9333
-- 
2.29.2

PR 96391? Can we fix it for gcc11?

[Qing Zhao via Gcc-patches]

Hi, 

The bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96391 


Bug 96391 - [10/11 Regression] internal compiler error: in 
linemap_compare_locations, at libcpp/line-map.c:1359 

has been opened on 7/30/2020, and multiple users reported the same issue. 

For our important application, all the C++ modules failed with this bug when we 
use gcc10 or gcc11. Then we have 
To use icc to compile C++, and gcc to compile C, it’s very inconvenient. 

I have raised the priority of this bug to P2 on 10/09/2020,  hope it can be 
fixed in gcc11. 

I see that Michael Cronenworth has attached a preprocessed file for the 
reproducing purpose in comment 4. 

So, can we have the fix of the bug in gcc11?

Thanks a lot.

Qing

RE: [AArch64] Fix vector multiplication costs

[Kyrylo Tkachov via Gcc-patches]

> -Original Message-
> From: Andre Vieira (lists) 
> Sent: 03 February 2021 17:59
> To: gcc-patches@gcc.gnu.org
> Cc: Kyrylo Tkachov 
> Subject: [AArch64] Fix vector multiplication costs
> 
> This patch introduces a vect.mul RTX cost and decouples the vector
> multiplication costing from the scalar one.
> 
> After Wilco's "AArch64: Add cost table for Cortex-A76" patch we saw a
> regression in vector codegen. Reproduceable with the small test added in
> this patch.
> Upon further investigation we noticed 'aarch64_rtx_mult_cost' was using
> scalar costs to calculate the cost of vector multiplication, which was
> now lower and preventing 'choose_mult_variant' from making the right
> choice to expand such vector multiplications with constants as shift and
> sub's. I also added a special case for SSRA to use the default vector
> cost rather than mult, SSRA seems to be cost using
> 'aarch64_rtx_mult_cost', which to be fair is quite curious. I believe we
> should have a better look at 'aarch64_rtx_costs' altogether and
> completely decouple vector and scalar costs. Though that is something
> that requires more rewriting than I believe should be done in Stage 4.
> 
> I gave all targets a vect.mult cost of 4x the vect.alu cost, with the
> exception of targets with cost 0 for vect.alu, those I gave the cost 4.
> 
> Bootstrapped on aarch64.
> 
> Is this OK for trunk?

Ok.
Thanks,
Kyrill

> 
> gcc/ChangeLog:
> 
>      * config/aarch64/aarch64-cost-tables.h: Add entries for vect.mul.
>      * config/aarch64/aarch64.c (aarch64_rtx_mult_cost): Use
> vect.mul for
>      vector multiplies and vect.alu for SSRA.
>      * config/arm/aarch-common-protos.h (struct vector_cost_table):
> Define
>      vect.mul cost field.
>      * config/arm/aarch-cost-tables.h: Add entries for vect.mul.
>      * config/arm/arm.c: Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
>      * gcc.target/aarch64/asimd-mul-to-shl-sub.c: New test.

[Patch] Fortran: %re/%im fixes for OpenMP/OpenACC + gfc_is_simplify_contiguous

[Tobias Burnus]

Found when looking at Julian's 3/4 OpenACC patch, which has not
yet been committed (and needs still to be revised a bit.)

The fix (a) avoids an ICE when Julian's patch has been applied.
The patch (b) just makes one error message less confusing.

The testcase shows that only %re/%im run reach the new code as
%kind/%len are != EXPR_VARIABLE. (The error message is slightly
misleading if the 'list item'/'var' is not a variable.)


(a) We need to handle REF_INQUIRY gfc_is_simplify_contiguous.

That function is used for 'is_contiguous(a(:)%re)', but it works
without this patch and simplifies already to .false.
And it is used in openmp.c, which can ICE without this patch.

(b) Just makes the error message nicer - as only EXPR_VARIABLE
reaches that code, only %re and %im are mentioned in the
error message.

(Actually, it is not completely clear whether %re/%im are invalid
or not; I think they should be – but one can argue otherwise.
For OpenMP I just saw that it is tacked internally in Issue 2661,
for OpenACC it is tracked as Issue 346 – but neither has been
discussed, yet.)

OK for mainline?

Tobias

PS: '%re'/'%im' permit accessing the real/imaginary part of a
complex variable as lvalue (in the C++ sense) and also permit
"var(:)%re = 1.0", which real(z) or imag(z) does not permit.

var%kind == kind(var), but derived types also permit
parameterized derived types (PDT), which can use '%foo' for respective
'len' and 'kind' components.

-
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München 
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank 
Thürauf
Fortran: %re/%im fixes for OpenMP/OpenACC + gfc_is_simplify_contiguous

gcc/fortran/ChangeLog:

	* expr.c (gfc_is_simplify_contiguous): Handle REF_INQUIRY, i.e.
	%im and %re which are EXPR_VARIABLE.
	* openmp.c (resolve_omp_clauses): Diagnose %re/%im explicitly.

gcc/testsuite/ChangeLog:

	* gfortran.dg/goacc-gomp/ref_inquiry.f90: New test.

 gcc/fortran/expr.c |  2 ++
 gcc/fortran/openmp.c   |  8 +
 .../gfortran.dg/goacc-gomp/ref_inquiry.f90 | 42 ++
 3 files changed, 52 insertions(+)

diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c
index 4f456fc629a..92a6700568d 100644
--- a/gcc/fortran/expr.c
+++ b/gcc/fortran/expr.c
@@ -5854,6 +5854,8 @@ gfc_is_simply_contiguous (gfc_expr *expr, bool strict, bool permit_element)
 	part_ref  = ref;
   else if (ref->type == REF_SUBSTRING)
 	return false;
+  else if (ref->type == REF_INQUIRY)
+	return false;
   else if (ref->u.ar.type != AR_ELEMENT)
 	ar = &ref->u.ar;
 }
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 797f6c86b62..f0307c801a5 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -5219,6 +5219,14 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses,
 && array_ref->next->type == REF_SUBSTRING)))
 		  gfc_error ("Unexpected substring reference in %s clause "
  "at %L", name, &n->where);
+		else if (array_ref && array_ref->type == REF_INQUIRY)
+		  {
+			gcc_assert (array_ref->u.i == INQUIRY_RE
+|| array_ref->u.i == INQUIRY_IM);
+			gfc_error ("Unexpected complex-parts designator "
+   "reference in %s clause at %L",
+   name, &n->where);
+		  }
 		else if (!resolved
 			|| n->expr->expr_type != EXPR_VARIABLE
 			|| array_ref->next
diff --git a/gcc/testsuite/gfortran.dg/goacc-gomp/ref_inquiry.f90 b/gcc/testsuite/gfortran.dg/goacc-gomp/ref_inquiry.f90
new file mode 100644
index 000..f92a6632af9
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/goacc-gomp/ref_inquiry.f90
@@ -0,0 +1,42 @@
+implicit none
+type t
+  integer :: i
+  character :: c
+  complex :: z
+end type t
+
+integer :: i
+character(kind=4, len=5) :: c
+complex :: z, zz(5)
+type(t) :: x
+
+print *, is_contiguous(zz(:)%re)
+
+! EXPR_VARIABLE / Cf. also OpenMP spec issue 2661
+!$omp target enter data map(to: z%re)! { dg-error "Unexpected complex-parts designator" }
+!$omp target enter data map(to: z%im)! { dg-error "Unexpected complex-parts designator" }
+!$omp target enter data map(to: x%z%re)  ! { dg-error "Unexpected complex-parts designator" }
+!$omp target enter data map(to: x%z%im)  ! { dg-error "Unexpected complex-parts designator" }
+
+! Fails differently as it is not a variable (EXPR_VARIABLE)
+!$omp target enter data map(to: i%kind, c%len) ! { dg-error "not a proper array section" }
+!$omp target enter data map(to: x%i%kind, x%c%len) ! { dg-error "not a proper array section" }
+
+! Likewise for OpenACC
+!$acc enter data copyin(i%kind, c%len) ! { dg-error "not a proper array section" }
+!$acc enter data copyin(x%i%kind)  ! { dg-error "not a proper array section" }
+!$acc enter data copyin(x%c%len)   ! { dg-error "not a proper array section" }
+!$acc update self(i%kind, c%len)   ! { dg-error "not a proper array section" }
+!$acc update self(x%i%kind)   

[PATCH 1/4] c++: Avoid building garbage trees from tsubst_requires_expr

[Patrick Palka via Gcc-patches]

Since we no longer partially instantiate REQUIRES_EXPRs, we don't need
to rebuild its requirements during tsubst_requires_expr.

gcc/cp/ChangeLog:

* constraint.cc (tsubst_simple_requirement): Just return
boolean_true_node on success.
(tsubst_type_requirement): Likewise.
(tsubst_compound_requirement): Likewise.
(tsubst_nested_requirement): Likewise.
(tsubst_requirement_body): Remove.
(check_constaint_variables): Rename to ...
(check_constraint_variables): ... this.
(tsubst_constraint_variables): Adjust.
(tsubst_requires_expr): Fold tsubst_requirement_body into here.
---
 gcc/cp/constraint.cc | 46 ++--
 1 file changed, 14 insertions(+), 32 deletions(-)

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 31e0fb5079a..3e599fe8c47 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1962,7 +1962,7 @@ tsubst_simple_requirement (tree t, tree args, subst_info 
info)
   tree expr = tsubst_valid_expression_requirement (t0, args, info);
   if (expr == error_mark_node)
 return error_mark_node;
-  return finish_simple_requirement (EXPR_LOCATION (t), expr);
+  return boolean_true_node;
 }
 
 /* Substitute through the type requirement.  */
@@ -1974,7 +1974,7 @@ tsubst_type_requirement (tree t, tree args, subst_info 
info)
   tree type = tsubst (t0, args, info.complain, info.in_decl);
   if (type == error_mark_node)
 return error_mark_node;
-  return finish_type_requirement (EXPR_LOCATION (t), type);
+  return boolean_true_node;
 }
 
 /* True if TYPE can be deduced from EXPR.  */
@@ -2080,8 +2080,7 @@ tsubst_compound_requirement (tree t, tree args, 
subst_info info)
return error_mark_node;
 }
 
-  return finish_compound_requirement (EXPR_LOCATION (t),
- expr, type, noexcept_p);
+  return boolean_true_node;
 }
 
 static tree
@@ -2100,7 +2099,7 @@ tsubst_nested_requirement (tree t, tree args, subst_info 
info)
 }
   if (result != boolean_true_node)
 return error_mark_node;
-  return result;
+  return boolean_true_node;
 }
 
 /* Substitute ARGS into the requirement T.  */
@@ -2125,24 +2124,6 @@ tsubst_requirement (tree t, tree args, subst_info info)
   gcc_unreachable ();
 }
 
-/* Substitute ARGS into the list of requirements T. Note that
-   substitution failures here result in ill-formed programs. */
-
-static tree
-tsubst_requirement_body (tree t, tree args, subst_info info)
-{
-  tree result = NULL_TREE;
-  while (t)
-{
-  tree req = tsubst_requirement (TREE_VALUE (t), args, info);
-  if (req == error_mark_node)
-   return error_mark_node;
-  result = tree_cons (NULL_TREE, req, result);
-  t = TREE_CHAIN (t);
-}
-  return nreverse (result);
-}
-
 static tree
 declare_constraint_vars (tree parms, tree vars)
 {
@@ -2168,7 +2149,7 @@ declare_constraint_vars (tree parms, tree vars)
if an error occurred.  */
 
 static tree
-check_constaint_variables (tree t, tree args, subst_info info)
+check_constraint_variables (tree t, tree args, subst_info info)
 {
   tree types = NULL_TREE;
   tree p = t;
@@ -2193,7 +2174,7 @@ static tree
 tsubst_constraint_variables (tree t, tree args, subst_info info)
 {
   /* Perform a trial substitution to check for type errors.  */
-  tree parms = check_constaint_variables (t, args, info);
+  tree parms = check_constraint_variables (t, args, info);
   if (parms == error_mark_node)
 return error_mark_node;
 
@@ -2253,19 +2234,20 @@ tsubst_requires_expr (tree t, tree args,
   return t;
 }
 
-  tree parms = REQUIRES_EXPR_PARMS (t);
-  if (parms)
+  if (tree parms = REQUIRES_EXPR_PARMS (t))
 {
   parms = tsubst_constraint_variables (parms, args, info);
   if (parms == error_mark_node)
return boolean_false_node;
 }
 
-  tree reqs = REQUIRES_EXPR_REQS (t);
-  reqs = tsubst_requirement_body (reqs, args, info);
-  if (reqs == error_mark_node)
-return boolean_false_node;
-
+  for (tree reqs = REQUIRES_EXPR_REQS (t); reqs; reqs = TREE_CHAIN (reqs))
+{
+  tree req = TREE_VALUE (reqs);
+  tree result = tsubst_requirement (req, args, info);
+  if (result == error_mark_node)
+   return boolean_false_node;
+}
   return boolean_true_node;
 }
 
-- 
2.30.0.452.gfb7fa4a1fd

[PATCH 2/4] c++: Preparatory type canonicalization fixes

[Patrick Palka via Gcc-patches]

The subsequent patches revealed some latent type canonicalization
issues during normalization and satisfaction:

1. In tsubst_parameter_mapping, we're canonicalizing the arguments of a
   TYPE_ARGUMENT_PACK only if 'arg' wasn't a TYPE_ARGUMENT_PACK to begin
   with.

2. We currently set DECL_CONTEXT and CONSTRAINT_VAR_P on each of the
   parameters introduced in a requires-expression _after_ we're done
   processing the requirements.  But meanwhile we may have already
   computed the canonical form of a type that depends on one of these
   PARM_DECLs, which depends on the result of cp_tree_equal, which
   depends on CONSTRAINT_VAR_P and DECL_CONTEXT.  So we must set these
   fields earlier, before processing the requirements.

3. In do_auto_deduction, we use the result of finish_decltype_type later
   as a template argument, so we should canonicalize the result too.
   (While we're here, we should pass 'complain' to finish_decltype_type,
   which fixes the testcase auto1.C below.)

gcc/cp/ChangeLog:

* constraint.cc (tsubst_parameter_mapping): Canonicalize the
arguments of a TYPE_ARGUMENT_PACK even if we've started with a
TYPE_ARGUMENT_PACK.
(finish_requires_expr): Don't set DECL_CONTEXT and
CONSTRAINT_VAR_P on each of the introduced parameters here.
* parser.c (cp_parser_requirement_parameter_list): Instead set
these fields earlier, here.

gcc/testsuite/ChangeLog:

* g++.dg/cpp1z/auto1.C: New test.
---
 gcc/cp/constraint.cc   | 25 -
 gcc/cp/parser.c| 12 
 gcc/cp/pt.c|  8 ++--
 gcc/testsuite/g++.dg/cpp1z/auto1.C | 13 +
 4 files changed, 39 insertions(+), 19 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/auto1.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 3e599fe8c47..39c97986082 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -2319,15 +2319,15 @@ tsubst_parameter_mapping (tree map, tree args, 
subst_info info)
  new_arg = tsubst_template_arg (arg, args, complain, in_decl);
  if (TYPE_P (new_arg))
new_arg = canonicalize_type_argument (new_arg, complain);
- if (TREE_CODE (new_arg) == TYPE_ARGUMENT_PACK)
+   }
+  if (TREE_CODE (new_arg) == TYPE_ARGUMENT_PACK)
+   {
+ tree pack_args = ARGUMENT_PACK_ARGS (new_arg);
+ for (int i = 0; i < TREE_VEC_LENGTH (pack_args); i++)
{
- tree pack_args = ARGUMENT_PACK_ARGS (new_arg);
- for (int i = 0; i < TREE_VEC_LENGTH (pack_args); i++)
-   {
- tree& pack_arg = TREE_VEC_ELT (pack_args, i);
- if (TYPE_P (pack_arg))
-   pack_arg = canonicalize_type_argument (pack_arg, complain);
-   }
+ tree& pack_arg = TREE_VEC_ELT (pack_args, i);
+ if (TYPE_P (pack_arg))
+   pack_arg = canonicalize_type_argument (pack_arg, complain);
}
}
   if (new_arg == error_mark_node)
@@ -3253,15 +3253,6 @@ evaluate_concept_check (tree check, tsubst_flags_t 
complain)
 tree
 finish_requires_expr (location_t loc, tree parms, tree reqs)
 {
-  /* Modify the declared parameters by removing their context
- so they don't refer to the enclosing scope and explicitly
- indicating that they are constraint variables. */
-  for (tree parm = parms; parm; parm = DECL_CHAIN (parm))
-{
-  DECL_CONTEXT (parm) = NULL_TREE;
-  CONSTRAINT_VAR_P (parm) = true;
-}
-
   /* Build the node. */
   tree r = build_min (REQUIRES_EXPR, boolean_type_node, parms, reqs, 
NULL_TREE);
   TREE_SIDE_EFFECTS (r) = false;
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 5da8670f0e2..fef10b9661d 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -28779,6 +28779,18 @@ cp_parser_requirement_parameter_list (cp_parser 
*parser)
   if (!parens.require_close (parser))
 return error_mark_node;
 
+  /* Modify the declared parameters by removing their context
+ so they don't refer to the enclosing scope and explicitly
+ indicating that they are constraint variables. */
+  for (tree parm = parms; parm; parm = TREE_CHAIN (parm))
+{
+  if (parm == void_list_node || parm == explicit_void_list_node)
+   break;
+  tree decl = TREE_VALUE (parm);
+  DECL_CONTEXT (decl) = NULL_TREE;
+  CONSTRAINT_VAR_P (decl) = true;
+}
+
   return parms;
 }
 
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 3605b67e424..bceb942e79a 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -29478,9 +29478,13 @@ do_auto_deduction (tree type, tree init, tree 
auto_node,
 || ((TREE_CODE (init) == COMPONENT_REF
  || TREE_CODE (init) == SCOPE_REF)
 && !REF_PARENTHESIZED_P (init)));
+  tree deduced = finish_decltype_type (init, id, complain);
+  deduced = canonicalize_type_argument (deduced, complain);
+  if (deduced

[PATCH 3/4] c++: Delay normalizing nested requirements until satisfaction

[Patrick Palka via Gcc-patches]

This sets up the functionality for controlling the initial set of
template parameters to pass to normalization when dealing with a
constraint-expression that is not associated with some constrained
declaration, for instance when normalizing a nested requirement of a
requires expression, or the constraints on a placeholder type.

The main new ingredient here is the data member norm_info::initial_parms
which can be set by callers of the normalization routines to communicate
the in-scope template parameters for the supplied constraint-expression,
rather than always falling back to using current_template_parms.

This patch then uses this functionality in our handling of nested
requirements so that we can delay normalizing them until needed for
satisfaction.  We currently immediately normalize nested requirements at
parse time, where we have the necessary template context, and cache the
normal form in their TREE_TYPE node.  With this patch, we now delay
normalization until needed (as with other constraint expressions), and
instead store the current value of current_template_parms in their
TREE_TYPE node (which we use to restore the template context at
normalization time).

In the subsequent patch, this functionality will also be used to
normalize placeholder type constraints during auto deduction.

gcc/cp/ChangeLog:

* constraint.cc (build_parameter_mapping): Rely on the caller to
determine the in-scope template parameters.
(norm_info::norm_info): Delegate the one-parameter constructor
to the two-parameter constructor.  In the two-parameter
constructor, fold in the definition of make_context, set
initial_parms appropriately, and don't set the now-removed
orig_decl member.
(norm_info::make_context): Remove, now that its only use is
inlined into the caller.
(norm_info::update_context): Adjust call to
build_parameter_mapping to pass in the relevant set of in-scope
template parameters.
(norm_info::ctx_parms): Define this member function.
(norm_info::context): Initialize to NULL_TREE.
(norm_info::orig_decl): Remove this data member.
(norm_info::initial_parms): Define this data member.
(normalize_atom): Adjust call to build_parameter_mapping to pass
in the relevant set of in-scope template parameters.  Use
info.initial_parms instead of info.orig_decl.
(normalize_constraint_expression): Define an overload that takes
a norm_info object.  Cache the result of normalization.  Define
the other overload in terms of this one, and handle a NESTED_REQ
argument by setting info.initial_parms appropriately.
(tsubst_nested_requirement): Go through
satisfy_constraint_expression so that we normalize on demand.
(finish_nested_requirement): Set the TREE_TYPE of the NESTED_REQ
to current_template_parms.
(diagnose_nested_requirements): Go through
satisfy_constraint_expression, as with tsubst_nested_requirement.
---
 gcc/cp/constraint.cc | 140 +++
 gcc/cp/cp-tree.h |   4 +-
 2 files changed, 78 insertions(+), 66 deletions(-)

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 39c97986082..56134f8b2bf 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -133,7 +133,7 @@ struct sat_info : subst_info
   bool diagnose_unsatisfaction;
 };
 
-static tree satisfy_constraint (tree, tree, sat_info);
+static tree satisfy_constraint_expression (tree, tree, sat_info);
 
 /* True if T is known to be some type other than bool. Note that this
is false for dependent types and errors.  */
@@ -594,26 +594,12 @@ map_arguments (tree parms, tree args)
   return parms;
 }
 
-/* Build the parameter mapping for EXPR using ARGS.  */
+/* Build the parameter mapping for EXPR using ARGS, where CTX_PARMS
+   are the template parameters in scope for EXPR.  */
 
 static tree
-build_parameter_mapping (tree expr, tree args, tree decl)
+build_parameter_mapping (tree expr, tree args, tree ctx_parms)
 {
-  tree ctx_parms = NULL_TREE;
-  if (decl)
-{
-  gcc_assert (TREE_CODE (decl) == TEMPLATE_DECL);
-  ctx_parms = DECL_TEMPLATE_PARMS (decl);
-}
-  else if (current_template_parms)
-{
-  /* TODO: This should probably be the only case, but because the
-point of declaration of concepts is currently set after the
-initializer, the template parameter lists are not available
-when normalizing concept definitions, hence the case above.  */
-  ctx_parms = current_template_parms;
-}
-
   tree parms = find_template_parameters (expr, ctx_parms);
   tree map = map_arguments (parms, args);
   return map;
@@ -645,53 +631,63 @@ parameter_mapping_equivalent_p (tree t1, tree t2)
 
 struct norm_info : subst_info
 {
-  explicit norm_info (tsubst_flags_t complain)
-: subst_info (tf_warning_or_error | complain, NULL_TREE),
-  context(

[PATCH 4/4] c++: dependent constraint on placeholder 'auto' [PR96443]

[Patrick Palka via Gcc-patches]

This fixes the way we check satisfaction of constraints on placeholder
types in various contexts, and in particular when the constraint is
dependent.

Firstly, when evaluating the return type requirement of a compound
requirement, we currently substitute the outer template arguments into
the constraint before checking satisfaction. But we should instead be
passing in the complete set of template arguments to satisfaction and
not do a prior separate substitution.  Our current approach leads to us
incorrectly rejecting the testcase concepts-return-req2.C below.

Secondly, when checking the constraints on a placeholder variable or
return type, we don't substitute the template arguments of the enclosing
context at all.  This leads to bogus errors during satisfaction when the
constraint is dependent as in the testcase concepts-placeholder3.C
below.

In order to fix these two issues, we need to be able to properly
normalize the constraints on a placeholder 'auto', which in turn
requires us to know the template parameters that were in-scope where an
'auto' was introduced.  This information currently doesn't seem to be
easily available when we need it, so this patch adds an auxiliary hash
table that keeps track of the value of current_template_parms when each
constrained 'auto' was formed.

This patch also removes some seemingly wrong handling of placeholder
type arguments from tsubst_parameter_mapping.  The code doesn't trigger
with the example used in the comments, because type_uses_auto doesn't
look inside non-deduced contexts such as the operand of decltype.  And
the call to do_auto_deduction seems confused because if 'arg' is a type,
then so is 'parm', and therefore 'init' too is a type, but
do_auto_deduction expects it to be an expression.  Before this patch,
this code was dead (as far as our testsuite can tell), but now it breaks
other parts of this patch, so let's remove it.

gcc/cp/ChangeLog:

PR c++/96443
* constraint.cc (type_deducible_p): Don't substitute into the
constraints, and instead just pass 'args' to do_auto_deduction
as the outer template arguments.
(tsubst_parameter_mapping): Remove confused code for handling
placeholder type arguments.
(normalize_placeholder_type_constraint): Define.
(satisfy_constraint_expression): Use it to handle placeholder
'auto' types.
* cp-tree.h (get_constrained_auto_context): Declare.
* pt.c (constrained_auto_context_map): Define.
(get_placeholder_type_constraint_context): Define.
(set_placeholder_type_constraints): Define.
(copy_placeholder_type_constraints): Define.
(tsubst) : Use
copy_placeholder_type_constraints.
(make_constrained_placeholder_type): Use
set_placeholder_type_constraints.
(do_auto_deduction): Clarify comments about the outer_targs
parameter.  Rework satisfaction of a placeholder type constraint
to pass in the complete set of template arguments directly to
constraints_satisfied_p.
(splice_late_return_type): Use copy_placeholder_type_constraints.

gcc/testsuite/ChangeLog:

PR c++/96443
* g++.dg/cpp2a/concepts-placeholder3.C: New test.
* g++.dg/cpp2a/concepts-return-req2.C: New test.
* g++.dg/concepts-ts1.C: Add dg-bogus directive to the call to
f15 that we expect to accept.
---
 gcc/cp/constraint.cc  | 106 --
 gcc/cp/cp-tree.h  |   1 +
 gcc/cp/pt.c   | 101 +++--
 .../g++.dg/cpp2a/concepts-placeholder3.C  |  19 
 .../g++.dg/cpp2a/concepts-return-req2.C   |  13 +++
 gcc/testsuite/g++.dg/cpp2a/concepts-ts1.C |   2 +-
 6 files changed, 146 insertions(+), 96 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-placeholder3.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-return-req2.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 56134f8b2bf..53588047d44 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -2007,39 +2007,19 @@ type_deducible_p (tree expr, tree type, tree 
placeholder, tree args,
  references are preserved in the result.  */
   expr = force_paren_expr_uneval (expr);
 
-  /* Replace the constraints with the instantiated constraints. This
- substitutes args into any template parameters in the trailing
- result type.  */
-  tree saved_constr = PLACEHOLDER_TYPE_CONSTRAINTS (placeholder);
-  tree subst_constr
-= tsubst_constraint (saved_constr,
-args,
-info.complain | tf_partial,
-info.in_decl);
-
-  if (subst_constr == error_mark_node)
-return false;
-
-  PLACEHOLDER_TYPE_CONSTRAINTS (placeholder) = subst_constr;
-
-  /* Temporarily unlink the canonical type.  */
-  tree saved_type = TYPE_CANONICAL (placeholder);
-  TYPE_CANONICAL (placeholder) = NULL_TR

Re: [PATCH] adjust "partly out of bounds" warning (PR 98503)

[Jeff Law via Gcc-patches]

On 2/3/21 3:45 PM, Martin Sebor wrote:
> On 2/3/21 2:57 PM, Jeff Law wrote:
>>
>>
>> On 1/28/21 4:03 PM, Martin Sebor wrote:
>>> The GCC 11 -Warray-bounds enhancement to diagnose accesses whose
>>> leading offset is in bounds but whose trailing offset is not has
>>> been causing some confusion.  When the warning is issued for
>>> an access to an in-bounds member via a pointer to a struct that's
>>> larger than the pointed-to object, phrasing this strictly invalid
>>> access in terms of array subscripts can be misleading, especially
>>> when the source code doesn't involve any arrays or indexing.
>>>
>>> Since the problem boils down to an aliasing violation much more
>>> so than an actual out-of-bounds access, the attached patch adjusts
>>> the code to issue a -Wstrict-aliasing warning in these cases instead
>>> of -Warray-bounds.  In addition, since the aliasing assumptions in
>>> GCC can be disabled by -fno-strict-aliasing, the patch also makes
>>> these instances of the warning conditional on -fstrict-aliasing
>>> being in effect.
>>>
>>> Martin
>>>
>>> gcc-98503.diff
>>>
>>> PR middle-end/98503 -Warray-bounds when -Wstrict-aliasing would be
>>> more appropriate
>>>
>>> gcc/ChangeLog:
>>>
>>> PR middle-end/98503
>>> * gimple-array-bounds.cc (array_bounds_checker::check_mem_ref):
>>> Issue -Wstrict-aliasing for a subset of violations.
>>> (array_bounds_checker::check_array_bounds):  Set new member.
>>> * gimple-array-bounds.h (array_bounds_checker::cref_of_mref): New
>>> data member.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> PR middle-end/98503
>>> * g++.dg/warn/Warray-bounds-10.C: Adjust text of expected warnings.
>>> * g++.dg/warn/Warray-bounds-11.C: Same.
>>> * g++.dg/warn/Warray-bounds-12.C: Same.
>>> * g++.dg/warn/Warray-bounds-13.C: Same.
>>> * gcc.dg/Warray-bounds-63.c: Avoid -Wstrict-aliasing.  Adjust text
>>> of expected warnings.
>>> * gcc.dg/Warray-bounds-66.c: Adjust text of expected warnings.
>>> * gcc.dg/Wstrict-aliasing-2.c: New test.
>>> * gcc.dg/Wstrict-aliasing-3.c: New test.
>> What I don't like here is the strict-aliasing warnings inside the file
>> and analysis phase for array bounds checking.
>>
>> ISTM that catching this at cast time would be better.  So perhaps in
>> build_c_cast and the C++ equivalent?
>>
>> It would mean we're warning at the cast site rather than the
>> dereference, but that may ultimately be better for the warning anyway.
>> I'm not sure.
>
> I had actually experimented with a this (in the middle end, and only
> for accesses) but even that caused so many warnings that I abandoned
> it, at least for now.  -Warray-bounds has the benefit of flow analysis
> (and dead code elimination).  In the front end it would have neither
> and be both excessively noisy and ineffective at the same time.  For
> example:
I think we agree that this really is an aliasing issue and I would go
further and claim that it has nothing to do with array bounds checking. 
Therefore warning for it within gimple-array-bounds just seems wrong.

I realize that you like having DCE run and the ability to look at
offsets and such, but it really feels like the wrong place to do this. 
Aliasing issues are also more of a front-end thing since the language
defines what is and what is not valid aliasing -- one might even argue
that any aliasing warning needs to be identified by the front-ends
exclusively (though possibly pruned away later).

Jeff

Re: [PATCH 4/4] c++: dependent constraint on placeholder 'auto' [PR96443]

[Patrick Palka via Gcc-patches]

On Mon, 8 Feb 2021, Patrick Palka wrote:

> This fixes the way we check satisfaction of constraints on placeholder
> types in various contexts, and in particular when the constraint is
> dependent.
> 
> Firstly, when evaluating the return type requirement of a compound
> requirement, we currently substitute the outer template arguments into
> the constraint before checking satisfaction. But we should instead be
> passing in the complete set of template arguments to satisfaction and
> not do a prior separate substitution.  Our current approach leads to us
> incorrectly rejecting the testcase concepts-return-req2.C below.
> 
> Secondly, when checking the constraints on a placeholder variable or
> return type, we don't substitute the template arguments of the enclosing
> context at all.  This leads to bogus errors during satisfaction when the
> constraint is dependent as in the testcase concepts-placeholder3.C
> below.
> 
> In order to fix these two issues, we need to be able to properly
> normalize the constraints on a placeholder 'auto', which in turn
> requires us to know the template parameters that were in-scope where an
> 'auto' was introduced.  This information currently doesn't seem to be
> easily available when we need it, so this patch adds an auxiliary hash
> table that keeps track of the value of current_template_parms when each
> constrained 'auto' was formed.
> 
> This patch also removes some seemingly wrong handling of placeholder
> type arguments from tsubst_parameter_mapping.  The code doesn't trigger
> with the example used in the comments, because type_uses_auto doesn't
> look inside non-deduced contexts such as the operand of decltype.  And
> the call to do_auto_deduction seems confused because if 'arg' is a type,
> then so is 'parm', and therefore 'init' too is a type, but
> do_auto_deduction expects it to be an expression.  Before this patch,
> this code was dead (as far as our testsuite can tell), but now it breaks
> other parts of this patch, so let's remove it.

This series was bootstrapped and regtested on x86_64-pc-linux-gnu and
tested on range-v3 and cmcstl2, with and without --enable-checking=release.

Re: [PATCH] avoid -Warray-bounds checks for vtable assignments (PR 98266)

[Jeff Law via Gcc-patches]

On 1/19/21 5:56 PM, Martin Sebor via Gcc-patches wrote:
> Similar to the problem reported for -Wstringop-overflow in pr98266
> and already fixed, -Warray-bounds is also susceptible to false
> positives in assignments and copies involving virtual inheritance.
> Because the two warnings don't share code yet (hopefully in GCC 12)
> the attached patch adds its own workaround for this problem to
> gimple-array-bounds.cc, this one slightly more crude because of
> the limited insight the array bounds checking has into the checked
> expressions.
>
> Tested on x86_64-linux.
>
> Martin
>
> gcc-98266.diff
>
> PR middle-end/98266 - bogus array subscript is partly outside array bounds on 
> virtual inheritance
>
> gcc/ChangeLog:
>
>   PR middle-end/98266
>   * gimple-array-bounds.cc (array_bounds_checker::check_array_bounds):
>   Avoid checking references involving artificial members.
>
> gcc/testsuite/ChangeLog:
>
>   PR middle-end/98266
>   * g++.dg/warn/Warray-bounds-15.C: New test.
It seems to me that we've got the full statement at some point  and thus
the full expression so at some point couldn't we detect when
TYPE_SIZE_UNIT!= DECL_SIZE_UNIT?  Or should we be using TYPE_SIZE_UNIT
rather than DECL_SIZE_UNIT in gimple-array-bounds.cc

Am I missing something?


jeff

Re: [PATCH][Bug libstdc++/70303] Value-initialized debug iterators

[François Dumont via Gcc-patches]

On 01/02/21 8:09 pm, Jonathan Wakely wrote:

On 01/02/21 19:30 +0100, François Dumont via Libstdc++ wrote:

On 01/02/21 6:43 pm, Jonathan Wakely wrote:

On 31/01/21 16:59 +0100, François Dumont via Libstdc++ wrote:
After the debug issue has been fixed in PR 98466 the problem was 
not in the debug iterator implementation itself but in the deque 
iterator operator- implementation.


    libstdc++: Make deque iterator operator- usable with value-init 
iterators


    N3644 implies that operator- can be used on value-init 
iterators. We now return
    0 if both iterators are value initialized. If only one is value 
initialized we
    keep the UB by returning the result of a normal computation 
which is an unexpected

    value.

    libstdc++/ChangeLog:

    PR libstdc++/70303
    * include/bits/stl_deque.h 
(std::deque<>::operator-(iterator, iterator)):

    Return 0 if both iterators are value-initialized.
    * testsuite/23_containers/deque/70303.cc: New test.
    * testsuite/23_containers/vector/70303.cc: New test.

Tested under Linux x86_64, ok to commit ?


OK.

I don't like adding the branch there though. Even with the
__builtin_expect it causes worse code to be generated than the
original.

This would be branchless, but a bit harder to understand:

    return difference_type(__x._S_buffer_size())
  * (__x._M_node - __y._M_node - int(__x._M_node == __y._M_node))
  + (__x._M_cur - __x._M_first) + (__y._M_last - __y._M_cur);

Please commit the fix and we can think about it later.


I do not think this work because for value-init iterators we have 
__x._M_node == __y._M_node == nullptr so the diff would give 
-_S_buffer_size().


But I got the idear, I'll prepare the patch.


Yeah, I just came back to the computer to say that it's wrong. But
maybe this:

    return difference_type(_S_buffer_size())
  * (__x._M_node - __y._M_node - int(__x._M_node && __y._M_node))
  + (__x._M_cur - __x._M_first) + (__y._M_last - __y._M_cur);

For value-init'd iterators we'd get _S_buffer_size() * 0 + 0 - 0

We could even just use int(__x._M_node != 0) because if one is null
and the other isn't, it's UB anyway.


This is what I've tested with success. As it is a recent kind of 
regression you might want to consider it now.


    libstdc++: Remove execution branch in deque iterator operator-

    libstdc++-v3/ChangeLog:

    * include/bits/stl_deque.h
    (std::operator-(deque::iterator, deque::iterator)): Replace 
if/then with

    a null pointer test.

Ok to commit ?

François

diff --git a/libstdc++-v3/include/bits/stl_deque.h b/libstdc++-v3/include/bits/stl_deque.h
index 04b70b77621..8bba7a3740f 100644
--- a/libstdc++-v3/include/bits/stl_deque.h
+++ b/libstdc++-v3/include/bits/stl_deque.h
@@ -352,12 +352,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
   friend difference_type
   operator-(const _Self& __x, const _Self& __y) _GLIBCXX_NOEXCEPT
   {
-	if (__builtin_expect(__x._M_node || __y._M_node, true))
-	  return difference_type(_S_buffer_size())
-	* (__x._M_node - __y._M_node - 1) + (__x._M_cur - __x._M_first)
-	+ (__y._M_last - __y._M_cur);
-
-	return 0;
+	return difference_type(_S_buffer_size())
+	  * (__x._M_node - __y._M_node - int(__x._M_node != 0))
+	  + (__x._M_cur - __x._M_first)
+	  + (__y._M_last - __y._M_cur);
   }
 
   // _GLIBCXX_RESOLVE_LIB_DEFECTS
@@ -369,12 +367,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	operator-(const _Self& __x,
 		  const _Deque_iterator<_Tp, _RefR, _PtrR>& __y) _GLIBCXX_NOEXCEPT
 	{
-	  if (__builtin_expect(__x._M_node || __y._M_node, true))
-	return difference_type(_S_buffer_size())
-	  * (__x._M_node - __y._M_node - 1) + (__x._M_cur - __x._M_first)
-	  + (__y._M_last - __y._M_cur);
-
-	  return 0;
+	  return difference_type(_S_buffer_size())
+	* (__x._M_node - __y._M_node - int(__x._M_node != 0))
+	+ (__x._M_cur - __x._M_first)
+	+ (__y._M_last - __y._M_cur);
 	}
 
   friend _Self

[PATCH] don't enable DWARF5 by default on Windows (PR98860)

[Mikael Pettersson via Gcc-patches]

PR98860 is a gcc-11 regression where bootstrap fails on Windows since
the switch to enable DWARF5 by default. The symptoms are that
executables generated by the stage1 compiler fail to run with "Exec
format error", which confuses subsequent configure steps and causes
hard errors. This happens even with the very latest binutils master.

Fixed by updating SUBTARGET_OVERRIDE_OPTIONS to set dwarf_version to 4
unless the user explicitly requested another version. I see some other
targets did the same.

Tested on Cygwin64 and mingw-w64.

Ok for master?

2021-02-08  Mikael Pettersson  

PR bootstrap/98860
* config/i386/cygming.h (SUBTARGET_OVERRIDE_OPTIONS): Override
dwarf_version to 4.

--- a/gcc/config/i386/cygming.h
+++ b/gcc/config/i386/cygming.h
@@ -209,6 +209,9 @@ along with GCC; see the file COPYING3.  If not see
 #define SUBTARGET_OVERRIDE_OPTIONS \
 do {   \
   flag_pic = TARGET_64BIT ? 1 : 0;  \
+  /* DWARF5 currently does not work on Windows. */ \
+  if (!global_options_set.x_dwarf_version) \
+dwarf_version = 4; \
 } while (0)

 /* Define this macro if references to a symbol must be treated
From 9d1b9e26cc77d325fd5574cb422771588d6aa71f Mon Sep 17 00:00:00 2001
From: Mikael Pettersson 
Date: Sat, 6 Feb 2021 22:59:43 +0100
Subject: [PATCH] cygming: DWARF5 does not work on Windows

---
 gcc/config/i386/cygming.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gcc/config/i386/cygming.h b/gcc/config/i386/cygming.h
index cfbca34f996..071f83cfd2d 100644
--- a/gcc/config/i386/cygming.h
+++ b/gcc/config/i386/cygming.h
@@ -209,6 +209,9 @@ along with GCC; see the file COPYING3.  If not see
 #define SUBTARGET_OVERRIDE_OPTIONS	\
 do {	\
   flag_pic = TARGET_64BIT ? 1 : 0;  \
+  /* DWARF5 currently does not work on Windows. */			\
+  if (!global_options_set.x_dwarf_version)\
+dwarf_version = 4;			\
 } while (0)
 
 /* Define this macro if references to a symbol must be treated
-- 
2.26.2

Re: [PATCH] avoid -Warray-bounds checks for vtable assignments (PR 98266)

[Martin Sebor via Gcc-patches]

On 2/8/21 12:59 PM, Jeff Law wrote:



On 1/19/21 5:56 PM, Martin Sebor via Gcc-patches wrote:

Similar to the problem reported for -Wstringop-overflow in pr98266
and already fixed, -Warray-bounds is also susceptible to false
positives in assignments and copies involving virtual inheritance.
Because the two warnings don't share code yet (hopefully in GCC 12)
the attached patch adds its own workaround for this problem to
gimple-array-bounds.cc, this one slightly more crude because of
the limited insight the array bounds checking has into the checked
expressions.

Tested on x86_64-linux.

Martin

gcc-98266.diff

PR middle-end/98266 - bogus array subscript is partly outside array bounds on 
virtual inheritance

gcc/ChangeLog:

PR middle-end/98266
* gimple-array-bounds.cc (array_bounds_checker::check_array_bounds):
Avoid checking references involving artificial members.

gcc/testsuite/ChangeLog:

PR middle-end/98266
* g++.dg/warn/Warray-bounds-15.C: New test.

It seems to me that we've got the full statement at some point  and thus
the full expression so at some point couldn't we detect when
TYPE_SIZE_UNIT!= DECL_SIZE_UNIT?  Or should we be using TYPE_SIZE_UNIT
rather than DECL_SIZE_UNIT in gimple-array-bounds.cc

Am I missing something?


The expression we're looking at when the false positive is issued
is the MEM_REF in the LHS of:

MEM[(struct D *)&D.2652 + 24B]._vptr.D = &MEM  [(void 
*)&_ZTC1E24_1D + 24B];


TREE_TYPE(LHS) is D, DECL_SIZE_UNIT (D.2652) is 24, and
TYPE_SIZE_UNIT(D) is also 24, so there's no discrepancy between
DECL_SIZE and TYPE_SIZE.

Martin

Re: [PING] Add conversions between _Float128 and Decimal.

[Segher Boessenkool]

On Mon, Feb 08, 2021 at 11:32:19AM -0500, Michael Meissner wrote:
> Ping patch.  This really needs to go in to allow switching the long double 
> type
> to IEEE 128-bit.

Please send a version that incorporates fixes to Will's nits?  Especially
fix the copyright dates.


Segher

Re: [PATCH] avoid -Warray-bounds checks for vtable assignments (PR 98266)

[Jeff Law via Gcc-patches]

On 2/8/21 2:56 PM, Martin Sebor wrote:
> On 2/8/21 12:59 PM, Jeff Law wrote:
>>
>>
>> On 1/19/21 5:56 PM, Martin Sebor via Gcc-patches wrote:
>>> Similar to the problem reported for -Wstringop-overflow in pr98266
>>> and already fixed, -Warray-bounds is also susceptible to false
>>> positives in assignments and copies involving virtual inheritance.
>>> Because the two warnings don't share code yet (hopefully in GCC 12)
>>> the attached patch adds its own workaround for this problem to
>>> gimple-array-bounds.cc, this one slightly more crude because of
>>> the limited insight the array bounds checking has into the checked
>>> expressions.
>>>
>>> Tested on x86_64-linux.
>>>
>>> Martin
>>>
>>> gcc-98266.diff
>>>
>>> PR middle-end/98266 - bogus array subscript is partly outside array
>>> bounds on virtual inheritance
>>>
>>> gcc/ChangeLog:
>>>
>>> PR middle-end/98266
>>> * gimple-array-bounds.cc
>>> (array_bounds_checker::check_array_bounds):
>>> Avoid checking references involving artificial members.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> PR middle-end/98266
>>> * g++.dg/warn/Warray-bounds-15.C: New test.
>> It seems to me that we've got the full statement at some point  and thus
>> the full expression so at some point couldn't we detect when
>> TYPE_SIZE_UNIT!= DECL_SIZE_UNIT?  Or should we be using TYPE_SIZE_UNIT
>> rather than DECL_SIZE_UNIT in gimple-array-bounds.cc
>>
>> Am I missing something?
>
> The expression we're looking at when the false positive is issued
> is the MEM_REF in the LHS of:
>
> MEM[(struct D *)&D.2652 + 24B]._vptr.D = &MEM  [(void
> *)&_ZTC1E24_1D + 24B];
>
> TREE_TYPE(LHS) is D, DECL_SIZE_UNIT (D.2652) is 24, and
> TYPE_SIZE_UNIT(D) is also 24, so there's no discrepancy between
> DECL_SIZE and TYPE_SIZE.
So that seems like it's a different issue then, unrelated to 97595.  Right?

Jeff

Re: [PATCH] libstdc++: Don't use reserved identifiers in simd headers

[Jonathan Wakely via Gcc-patches]

On 01/02/21 13:21 +0100, Rainer Orth wrote:

Two simd tests FAIL on Solaris, both SPARC and x86:

FAIL: experimental/simd/standard_abi_usable.cc -msse2 -O2 -Wno-psabi (test for 
excess errors)
FAIL: experimental/simd/standard_abi_usable_2.cc -msse2 -O2 -Wno-psabi (test 
for excess errors)

This happens because the simd headers use identifiers documented in the
libstdc++ manual as reserved by system headers.

Fixed as follows, tested on i386-pc-solaris2.11, sparc-sun-solaris2.11,
and x86_64-pc-linux-gnu.

Ok for master?


OK, thanks.



As an aside, the use of vim: markers initially confused the hell out of
me.  As an Emacs user, I rarely use vi for much more than a pager, but
when I wanted to check the lines mentioned in the g++ errors, I had no
idea what was going on or how to disable the folding enabled there:

// vim: foldmethod=marker sw=2 noet ts=8 sts=2 tw=80

I can't help but feel that this is just a personal preference and
doesn't belong into the upstream code.

For avoidance of doubt, I'd consider equivalent Emacs local variables
equally inappropriate.


We do have Emacs mode lines in all the libstdc++ headers FWIW. I use a
modified copy of https://www.vim.org/scripts/script.php?script_id=3381
to parse those modelines and set Vim config like sw=2 noet ts=2 etc.
when it sees them. So I don't need vim: lines, because the Emacs ones
work for me.

I agree with removing the vim: lines if Matthias is happy to. The
foldmethod is definitely awkward if that's not your preference.

Re: [PATCH] mklog: automatically fill in generated entries

[Joseph Myers]

On Sun, 7 Feb 2021, Mike Frysinger via Gcc-patches wrote:

> +# NB: Makefile.in isn't listed as it's not always generated.
> +generated_files = {'aclocal.m4', 'config.h.in', 'configure'}

libiberty/aclocal.m4 isn't a generated file either.

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

Re: [PATCH] avoid -Warray-bounds checks for vtable assignments (PR 98266)

[Martin Sebor via Gcc-patches]

On 2/8/21 3:26 PM, Jeff Law wrote:



On 2/8/21 2:56 PM, Martin Sebor wrote:

On 2/8/21 12:59 PM, Jeff Law wrote:



On 1/19/21 5:56 PM, Martin Sebor via Gcc-patches wrote:

Similar to the problem reported for -Wstringop-overflow in pr98266
and already fixed, -Warray-bounds is also susceptible to false
positives in assignments and copies involving virtual inheritance.
Because the two warnings don't share code yet (hopefully in GCC 12)
the attached patch adds its own workaround for this problem to
gimple-array-bounds.cc, this one slightly more crude because of
the limited insight the array bounds checking has into the checked
expressions.

Tested on x86_64-linux.

Martin

gcc-98266.diff

PR middle-end/98266 - bogus array subscript is partly outside array
bounds on virtual inheritance

gcc/ChangeLog:

 PR middle-end/98266
 * gimple-array-bounds.cc
(array_bounds_checker::check_array_bounds):
 Avoid checking references involving artificial members.

gcc/testsuite/ChangeLog:

 PR middle-end/98266
 * g++.dg/warn/Warray-bounds-15.C: New test.

It seems to me that we've got the full statement at some point  and thus
the full expression so at some point couldn't we detect when
TYPE_SIZE_UNIT!= DECL_SIZE_UNIT?  Or should we be using TYPE_SIZE_UNIT
rather than DECL_SIZE_UNIT in gimple-array-bounds.cc

Am I missing something?


The expression we're looking at when the false positive is issued
is the MEM_REF in the LHS of:

MEM[(struct D *)&D.2652 + 24B]._vptr.D = &MEM  [(void
*)&_ZTC1E24_1D + 24B];

TREE_TYPE(LHS) is D, DECL_SIZE_UNIT (D.2652) is 24, and
TYPE_SIZE_UNIT(D) is also 24, so there's no discrepancy between
DECL_SIZE and TYPE_SIZE.

So that seems like it's a different issue then, unrelated to 97595.  Right?


I think the underlying problem is the same.  We're getting a size
that doesn't correspond to what's actually being accessed, and it
happens because of the virtual inheritance.  In pr97595 Jason
suggested to use the decl/type size inequality to identify this
case but I think we could have just as well used DECL_ARTIFICIAL
instead.  At least the test cases from pr97595 both pass with
this change.

Martin



Jeff

Re: [PATCH] adjust "partly out of bounds" warning (PR 98503)

[Martin Sebor via Gcc-patches]

On 2/8/21 12:09 PM, Jeff Law wrote:



On 2/3/21 3:45 PM, Martin Sebor wrote:

On 2/3/21 2:57 PM, Jeff Law wrote:



On 1/28/21 4:03 PM, Martin Sebor wrote:

The GCC 11 -Warray-bounds enhancement to diagnose accesses whose
leading offset is in bounds but whose trailing offset is not has
been causing some confusion.  When the warning is issued for
an access to an in-bounds member via a pointer to a struct that's
larger than the pointed-to object, phrasing this strictly invalid
access in terms of array subscripts can be misleading, especially
when the source code doesn't involve any arrays or indexing.

Since the problem boils down to an aliasing violation much more
so than an actual out-of-bounds access, the attached patch adjusts
the code to issue a -Wstrict-aliasing warning in these cases instead
of -Warray-bounds.  In addition, since the aliasing assumptions in
GCC can be disabled by -fno-strict-aliasing, the patch also makes
these instances of the warning conditional on -fstrict-aliasing
being in effect.

Martin

gcc-98503.diff

PR middle-end/98503 -Warray-bounds when -Wstrict-aliasing would be
more appropriate

gcc/ChangeLog:

 PR middle-end/98503
 * gimple-array-bounds.cc (array_bounds_checker::check_mem_ref):
 Issue -Wstrict-aliasing for a subset of violations.
 (array_bounds_checker::check_array_bounds):  Set new member.
 * gimple-array-bounds.h (array_bounds_checker::cref_of_mref): New
 data member.

gcc/testsuite/ChangeLog:

 PR middle-end/98503
 * g++.dg/warn/Warray-bounds-10.C: Adjust text of expected warnings.
 * g++.dg/warn/Warray-bounds-11.C: Same.
 * g++.dg/warn/Warray-bounds-12.C: Same.
 * g++.dg/warn/Warray-bounds-13.C: Same.
 * gcc.dg/Warray-bounds-63.c: Avoid -Wstrict-aliasing.  Adjust text
 of expected warnings.
 * gcc.dg/Warray-bounds-66.c: Adjust text of expected warnings.
 * gcc.dg/Wstrict-aliasing-2.c: New test.
 * gcc.dg/Wstrict-aliasing-3.c: New test.

What I don't like here is the strict-aliasing warnings inside the file
and analysis phase for array bounds checking.

ISTM that catching this at cast time would be better.  So perhaps in
build_c_cast and the C++ equivalent?

It would mean we're warning at the cast site rather than the
dereference, but that may ultimately be better for the warning anyway.
I'm not sure.


I had actually experimented with a this (in the middle end, and only
for accesses) but even that caused so many warnings that I abandoned
it, at least for now.  -Warray-bounds has the benefit of flow analysis
(and dead code elimination).  In the front end it would have neither
and be both excessively noisy and ineffective at the same time.  For
example:

I think we agree that this really is an aliasing issue and I would go
further and claim that it has nothing to do with array bounds checking.
Therefore warning for it within gimple-array-bounds just seems wrong.

I realize that you like having DCE run and the ability to look at
offsets and such, but it really feels like the wrong place to do this.
Aliasing issues are also more of a front-end thing since the language
defines what is and what is not valid aliasing -- one might even argue
that any aliasing warning needs to be identified by the front-ends
exclusively (though possibly pruned away later).


The aliasing restrictions are on accesses, which are [defined in
C as] execution-time actions on objects.  Analyzing execution-time
actions unavoidably depends on flow analysis which the front ends
have only very limited support for (simple expressions only).
I gave one example showing how the current -Wstrict-aliasing in
the front end is ineffective against all but the most simplistic
bugs, but there are many more.  For instance:

  int i;
  void *p = &i;// valid
  float *q = p;// valid
  *q = 0;  // aliasing violation

This bug is easily detectable in the middle end but impossible
to do in the front end (same as all other invalid accesses).

Whether this is done in gimple-array-bounds or some other pass seems
to me like a minor implementation detail.  It naturally came out of
an enhancement I implemented there (which would detect the above
with float replaced by any larger type as an out-of-bounds access)
but I have no problem with moving this subset to some other pass
(or duplicating it there).  In fact, as I said, I'd like to enhance
-Wstrict-aliasing to detect more bugs at some point, so that might
be a good time to move this instance of the warning there as well.
But the enhancement I'm thinking of is in the middle end, not in
the front end.

In any event, the warning is valid, just the phrasing is misleading
since there in the case of the struct member there isn't necessarily
any subscripting involved or even an access to members beyond the end
of the accessed object.  Issuing it under -Warray-bounds and with
-fno-strict-aliasing compounds the problem.  I put together this
patch in response to the feedback I got from you and from the

Re: [PATCH] avoid -Warray-bounds checks for vtable assignments (PR 98266)

[Jeff Law via Gcc-patches]

On 2/8/21 3:44 PM, Martin Sebor wrote:
> On 2/8/21 3:26 PM, Jeff Law wrote:
>>
>>
>> On 2/8/21 2:56 PM, Martin Sebor wrote:
>>> On 2/8/21 12:59 PM, Jeff Law wrote:


 On 1/19/21 5:56 PM, Martin Sebor via Gcc-patches wrote:
> Similar to the problem reported for -Wstringop-overflow in pr98266
> and already fixed, -Warray-bounds is also susceptible to false
> positives in assignments and copies involving virtual inheritance.
> Because the two warnings don't share code yet (hopefully in GCC 12)
> the attached patch adds its own workaround for this problem to
> gimple-array-bounds.cc, this one slightly more crude because of
> the limited insight the array bounds checking has into the checked
> expressions.
>
> Tested on x86_64-linux.
>
> Martin
>
> gcc-98266.diff
>
> PR middle-end/98266 - bogus array subscript is partly outside array
> bounds on virtual inheritance
>
> gcc/ChangeLog:
>
>  PR middle-end/98266
>  * gimple-array-bounds.cc
> (array_bounds_checker::check_array_bounds):
>  Avoid checking references involving artificial members.
>
> gcc/testsuite/ChangeLog:
>
>  PR middle-end/98266
>  * g++.dg/warn/Warray-bounds-15.C: New test.
 It seems to me that we've got the full statement at some point  and
 thus
 the full expression so at some point couldn't we detect when
 TYPE_SIZE_UNIT!= DECL_SIZE_UNIT?  Or should we be using TYPE_SIZE_UNIT
 rather than DECL_SIZE_UNIT in gimple-array-bounds.cc

 Am I missing something?
>>>
>>> The expression we're looking at when the false positive is issued
>>> is the MEM_REF in the LHS of:
>>>
>>> MEM[(struct D *)&D.2652 + 24B]._vptr.D = &MEM  [(void
>>> *)&_ZTC1E24_1D + 24B];
>>>
>>> TREE_TYPE(LHS) is D, DECL_SIZE_UNIT (D.2652) is 24, and
>>> TYPE_SIZE_UNIT(D) is also 24, so there's no discrepancy between
>>> DECL_SIZE and TYPE_SIZE.
>> So that seems like it's a different issue then, unrelated to 97595. 
>> Right?
>
> I think the underlying problem is the same.  We're getting a size
> that doesn't correspond to what's actually being accessed, and it
> happens because of the virtual inheritance.  In pr97595 Jason
> suggested to use the decl/type size inequality to identify this
> case but I think we could have just as well used DECL_ARTIFICIAL
> instead.  At least the test cases from pr97595 both pass with
> this change.
But in the 98266 case the type and decl sizes are the same.  So to be
true that would mean that the underlying type we're using to access
memory differs from its actual type.  Is that the case in the IL?  And
does this have wider implications for diagnostics or optimizations that
rely on accurate type sizing?

I'm just trying to make sure I understand, not accepting or rejecting
the patch yet.

jeff

Re: [PATCH] run -Wnonnull later (PR 87489)

[Jeff Law via Gcc-patches]

On 1/31/21 5:31 PM, Martin Sebor via Gcc-patches wrote:
> The initial -Wnonnull implementation in the middle end took place
> too late in the pipeline (just before expansion), and as a result
> was prone to false positives (bug 78817).  In an attempt to avoid
> the worst of those, the warning was moved to the ccp2 pass in
> r243874.  However, as the test case in PR 87489 shows, this is
> in turn too early and causes other false positives as a result.
>
> A few experiments with running the warning later suggest that
> just before the mergephi2 pass is a good point to avoid this class
> of false positives without causing any regressions or introducing
> any new warnings either in Glibc or in Binutils/GDB.
>
> Since PR 87489 is a GCC 8-11 regression I propose to make this
> change on the trunk as well as on the release branches.
>
> Martin
>
> gcc-87489.diff
>
> PR middle-end/87489 - Spurious -Wnonnull warning on bitwise arithmetic and 
> inlining
>
> gcc/ChangeLog:
>
>   PR middle-end/87489
>   * passes.def (pass_post_ipa_warn): Move later.
>
> gcc/testsuite/ChangeLog:
>
>   PR middle-end/87489
>   * gcc.dg/Wnonnull-6.c: New test.
>   * gcc.dg/Wnonnull-7.c: New test.
So moving passes is generally not the right solution to most problems --
it usually turns into a game of wack-a-mole.    So rather than looking
at pass reordering, let's look at the IL and see if there's reasonable
optimizations we could do that in turn would avoid the false positive. 
I mentioned this in c#11 in the BZ.

What I missed last year was that we could improve the backwards threader.


If we look at the forwprop1 dump we have this leading up to the
problematical strlen call:

;;   basic block 2, loop depth 0, maybe hot
;;    prev block 0, next block 3, flags: (NEW, VISITED)
;;    pred:   ENTRY (FALLTHRU,EXECUTABLE)
  j = 0;
  if (i_13(D) != 0)
    goto ; [INV]
  else
    goto ; [INV]
;;    succ:   3 (TRUE_VALUE,EXECUTABLE)
;;    4 (FALSE_VALUE,EXECUTABLE)

;;   basic block 3, loop depth 0, maybe hot
;;    prev block 2, next block 4, flags: (NEW, VISITED)
;;    pred:   2 (TRUE_VALUE,EXECUTABLE)
  j.0_1 = j;
  _2 = j.0_1 | 1;
  j = _2;
;;    succ:   4 (FALLTHRU,EXECUTABLE)

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

;;   basic block 5, loop depth 0, maybe hot
;;    prev block 4, next block 6, flags: (NEW, VISITED)
;;    pred:   4 (TRUE_VALUE,EXECUTABLE)
  f (&j, 0);
;;    succ:   6 (FALLTHRU,EXECUTABLE)

;;   basic block 6, loop depth 0, maybe hot
;;    prev block 5, next block 7, flags: (NEW, VISITED)
;;    pred:   4 (FALSE_VALUE,EXECUTABLE)
;;    5 (FALLTHRU,EXECUTABLE)
  j.2_4 = j;
  _5 = j.2_4 & 1;
  if (_5 != 0)
    goto ; [INV]
  else
    goto ; [INV]
;;    succ:   7 (TRUE_VALUE,EXECUTABLE)
;;    8 (FALSE_VALUE,EXECUTABLE)

Of particular interest is the fact that we always know where BB4 will
go.  If we take 2->3->4 then 4 will always transfer to 5.  If we take
2->4 then 4 will always transfer to 6.

The problem is the backwards threader is a bit dumb in that it only
allows a very limited number of RHS expressions.  So when it sees j.1_3
= j in BB4 it gives up.

If we fix that and use walk_aliased_vdefs we can pretty easily find the
j = 0 statement in BB2 and thread 2->4->6.  That in turn allows
subsequent optimizers to do a better job and the problematical call is gone.

While that is enough to fix the testcase and it helps clean up the code
in the original BZ a bit, we still get the warning.  We may need to
handle variants of this kind of code:
;;   basic block 4, loop depth 0, maybe hot
;;    prev block 3, next block 5, flags: (NEW, VISITED)
;;    pred:   2 (TRUE_VALUE,EXECUTABLE)
  _1 = xl_xinfo.xinfo;
  _2 = _1 | 16;
  xl_xinfo.xinfo = _2;
  xl_twophase.xid = twophase_xid_16(D);
  _3 = xl_xinfo.xinfo;
  if (_3 != 0)
    goto ; [INV]
  else
    goto ; [INV]
;;    succ:   5 (TRUE_VALUE,EXECUTABLE)
;;    6 (FALSE_VALUE,EXECUTABLE)

We can see pretty easily that _3 is never zero.  The backwards threader
can't make that deduction, but I  think I see how to add it pretty
easily.  The combination of those two extensions *should* allow the
backwards threader to clean this up much earlier in the pipeline and
avoid the warning.

Anyways, still poking around, but my general sense is that changing pass
ordering can be avoided here.

jeff

Re: [PATCH] adjust "partly out of bounds" warning (PR 98503)

[Jeff Law via Gcc-patches]

On 2/8/21 4:07 PM, Martin Sebor wrote:
> On 2/8/21 12:09 PM, Jeff Law wrote:
>>
>>
>> On 2/3/21 3:45 PM, Martin Sebor wrote:
>>> On 2/3/21 2:57 PM, Jeff Law wrote:


 On 1/28/21 4:03 PM, Martin Sebor wrote:
> The GCC 11 -Warray-bounds enhancement to diagnose accesses whose
> leading offset is in bounds but whose trailing offset is not has
> been causing some confusion.  When the warning is issued for
> an access to an in-bounds member via a pointer to a struct that's
> larger than the pointed-to object, phrasing this strictly invalid
> access in terms of array subscripts can be misleading, especially
> when the source code doesn't involve any arrays or indexing.
>
> Since the problem boils down to an aliasing violation much more
> so than an actual out-of-bounds access, the attached patch adjusts
> the code to issue a -Wstrict-aliasing warning in these cases instead
> of -Warray-bounds.  In addition, since the aliasing assumptions in
> GCC can be disabled by -fno-strict-aliasing, the patch also makes
> these instances of the warning conditional on -fstrict-aliasing
> being in effect.
>
> Martin
>
> gcc-98503.diff
>
> PR middle-end/98503 -Warray-bounds when -Wstrict-aliasing would be
> more appropriate
>
> gcc/ChangeLog:
>
>  PR middle-end/98503
>  * gimple-array-bounds.cc (array_bounds_checker::check_mem_ref):
>  Issue -Wstrict-aliasing for a subset of violations.
>  (array_bounds_checker::check_array_bounds):  Set new member.
>  * gimple-array-bounds.h (array_bounds_checker::cref_of_mref):
> New
>  data member.
>
> gcc/testsuite/ChangeLog:
>
>  PR middle-end/98503
>  * g++.dg/warn/Warray-bounds-10.C: Adjust text of expected
> warnings.
>  * g++.dg/warn/Warray-bounds-11.C: Same.
>  * g++.dg/warn/Warray-bounds-12.C: Same.
>  * g++.dg/warn/Warray-bounds-13.C: Same.
>  * gcc.dg/Warray-bounds-63.c: Avoid -Wstrict-aliasing.  Adjust
> text
>  of expected warnings.
>  * gcc.dg/Warray-bounds-66.c: Adjust text of expected warnings.
>  * gcc.dg/Wstrict-aliasing-2.c: New test.
>  * gcc.dg/Wstrict-aliasing-3.c: New test.
 What I don't like here is the strict-aliasing warnings inside the file
 and analysis phase for array bounds checking.

 ISTM that catching this at cast time would be better.  So perhaps in
 build_c_cast and the C++ equivalent?

 It would mean we're warning at the cast site rather than the
 dereference, but that may ultimately be better for the warning anyway.
 I'm not sure.
>>>
>>> I had actually experimented with a this (in the middle end, and only
>>> for accesses) but even that caused so many warnings that I abandoned
>>> it, at least for now.  -Warray-bounds has the benefit of flow analysis
>>> (and dead code elimination).  In the front end it would have neither
>>> and be both excessively noisy and ineffective at the same time.  For
>>> example:
>> I think we agree that this really is an aliasing issue and I would go
>> further and claim that it has nothing to do with array bounds checking.
>> Therefore warning for it within gimple-array-bounds just seems wrong.
>>
>> I realize that you like having DCE run and the ability to look at
>> offsets and such, but it really feels like the wrong place to do this.
>> Aliasing issues are also more of a front-end thing since the language
>> defines what is and what is not valid aliasing -- one might even argue
>> that any aliasing warning needs to be identified by the front-ends
>> exclusively (though possibly pruned away later).
>
> The aliasing restrictions are on accesses, which are [defined in
> C as] execution-time actions on objects.  Analyzing execution-time
> actions unavoidably depends on flow analysis which the front ends
> have only very limited support for (simple expressions only).
> I gave one example showing how the current -Wstrict-aliasing in
> the front end is ineffective against all but the most simplistic
> bugs, but there are many more.  For instance:
I'm aware of all this.  Even so I think trying to deal with strict
aliasing in the middle-end is fundamentally wrong.  The middle end is
not C and it can not assume C semantics and putting the warning in the
middle of the array bounds pass is, IMHO, just wrong.

That doesn't change the need to address the problems with the warning,
namely that it makes references to array bounds violations for accesses
which aren't arrays.

The details of *how* to address those problems are still quite fuzzy. 
The most obvious ideas to me are either rely on something like
__builtin_warning or to mark the problem expressions in the front-end,
but defer issuing the warning until we're done with the optimization
pipeline.

Jeff

Re: [PATCH] mklog: automatically fill in generated entries

[Mike Frysinger via Gcc-patches]

On 08 Feb 2021 22:44, Joseph Myers wrote:
> On Sun, 7 Feb 2021, Mike Frysinger via Gcc-patches wrote:
> > +# NB: Makefile.in isn't listed as it's not always generated.
> > +generated_files = {'aclocal.m4', 'config.h.in', 'configure'}
> 
> libiberty/aclocal.m4 isn't a generated file either.

that is ... weird.  maybe it's a vintage thing.  any reason it needs
to stay this way ?  looks like it'd be trivial to switch it over to
acinclude.m4.
-mike

[pushed] c++: constexpr, union, and no_unique_address [PR98994]

[Jason Merrill via Gcc-patches]

My second patch for 97566 omits nested CONSTRUCTORs for empty fields, but we
do want them for empty union members.

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

gcc/cp/ChangeLog:

PR c++/98994
PR c++/97566
* constexpr.c (cxx_eval_store_expression): Only skip empty fields in
RECORD_TYPE.

gcc/testsuite/ChangeLog:

PR c++/98994
* g++.dg/cpp2a/no_unique_address12.C: New test.
---
 gcc/cp/constexpr.c   |  2 +-
 gcc/testsuite/g++.dg/cpp2a/no_unique_address12.C | 12 
 2 files changed, 13 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/no_unique_address12.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 1dbc2db9643..53567ad6f8b 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -5292,7 +5292,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree 
t,
   type = refs->pop();
   tree index = refs->pop();
 
-  if (is_empty_field (index))
+  if (code == RECORD_TYPE && is_empty_field (index))
/* Don't build a sub-CONSTRUCTOR for an empty base or field, as they
   have no data and might have an offset lower than previously declared
   fields, which confuses the middle-end.  The code below will notice
diff --git a/gcc/testsuite/g++.dg/cpp2a/no_unique_address12.C 
b/gcc/testsuite/g++.dg/cpp2a/no_unique_address12.C
new file mode 100644
index 000..761d2086516
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/no_unique_address12.C
@@ -0,0 +1,12 @@
+// PR c++/98994
+// { dg-do compile { target c++20 } }
+
+struct empty {};
+
+union U {
+  constexpr U(): a() { }
+
+  [[no_unique_address]] empty a;
+};
+
+constexpr U u;

base-commit: 2da7ce23cfd81b67f77dc102d6f97dd19363b5f4
-- 
2.27.0

[pushed] c++: generic lambda, fn* conv, empty class [PR98326]

[Jason Merrill via Gcc-patches]

Here, in the thunk returned from the captureless lambda conversion to
pointer-to-function, we try to pass through invisible reference parameters
by reference, without doing a copy.  The empty class copy optimization was
messing that up.

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

gcc/cp/ChangeLog:

PR c++/98326
PR c++/20408
* cp-gimplify.c (simple_empty_class_p): Don't touch an invisiref
parm.

gcc/testsuite/ChangeLog:

PR c++/98326
* g++.dg/cpp1y/lambda-generic-empty1.C: New test.
---
 gcc/cp/cp-gimplify.c   | 12 
 gcc/testsuite/g++.dg/cpp1y/lambda-generic-empty1.C |  9 +
 2 files changed, 21 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/lambda-generic-empty1.C

diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index 4d0e92c3321..1c5e15b957f 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -324,6 +324,18 @@ simple_empty_class_p (tree type, tree op, tree_code code)
   && TYPE_HAS_TRIVIAL_DESTRUCTOR (type))
 /* The TARGET_EXPR is itself a simple copy, look through it.  */
 return simple_empty_class_p (type, TARGET_EXPR_INITIAL (op), code);
+
+  if (TREE_CODE (op) == PARM_DECL
+  && TREE_ADDRESSABLE (TREE_TYPE (op)))
+{
+  tree fn = DECL_CONTEXT (op);
+  if (DECL_THUNK_P (fn)
+ || lambda_static_thunk_p (fn))
+   /* In a thunk, we pass through invisible reference parms, so this isn't
+  actually a copy.  */
+   return false;
+}
+
   return
 (TREE_CODE (op) == EMPTY_CLASS_EXPR
  || code == MODIFY_EXPR
diff --git a/gcc/testsuite/g++.dg/cpp1y/lambda-generic-empty1.C 
b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-empty1.C
new file mode 100644
index 000..ffb0cf149ce
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-empty1.C
@@ -0,0 +1,9 @@
+// PR c++/98326
+// { dg-do compile { target c++14 } }
+
+struct A {
+A() = default;
+A(const A&) {}
+};
+
+void (*fptr)(A) = [](auto){};

base-commit: 2da7ce23cfd81b67f77dc102d6f97dd19363b5f4
prerequisite-patch-id: 67efffa865b1469c2b63fe7a6ae9fec692c63079
-- 
2.27.0

[pushed] c++: consteval and explicit instantiation [PR96905]

[Jason Merrill via Gcc-patches]

Normally, an explicit instantiation means we want to write out the
instantiation.  But not for a consteval function.

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

gcc/cp/ChangeLog:

PR c++/96905
* pt.c (mark_decl_instantiated): Exit early if consteval.

gcc/testsuite/ChangeLog:

PR c++/96905
* g++.dg/cpp2a/consteval-expinst1.C: New test.
---
 gcc/cp/pt.c   |  5 +
 .../g++.dg/cpp2a/consteval-expinst1.C | 20 +++
 2 files changed, 25 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/consteval-expinst1.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 3605b67e424..f73deb3aee3 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -24154,6 +24154,11 @@ mark_decl_instantiated (tree result, int extern_p)
   if (TREE_ASM_WRITTEN (result))
 return;
 
+  /* consteval functions are never emitted.  */
+  if (TREE_CODE (result) == FUNCTION_DECL
+  && DECL_IMMEDIATE_FUNCTION_P (result))
+return;
+
   /* For anonymous namespace we don't need to do anything.  */
   if (decl_anon_ns_mem_p (result))
 {
diff --git a/gcc/testsuite/g++.dg/cpp2a/consteval-expinst1.C 
b/gcc/testsuite/g++.dg/cpp2a/consteval-expinst1.C
new file mode 100644
index 000..01452ddbbab
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/consteval-expinst1.C
@@ -0,0 +1,20 @@
+// PR c++/96905
+// { dg-do compile { target c++20 } }
+
+template
+struct duration
+{
+  static consteval int
+  gcd(int m, int n) noexcept
+  {
+while (m != 0 && n != 0)
+{
+  int rem = m % n;
+  m = n;
+  n = rem;
+}
+return m + n;
+  }
+};
+
+template class duration;

base-commit: 2da7ce23cfd81b67f77dc102d6f97dd19363b5f4
prerequisite-patch-id: 67efffa865b1469c2b63fe7a6ae9fec692c63079
prerequisite-patch-id: 827552b0b335a925cabfb34023b39d0b0753fb46
-- 
2.27.0

Re: [PATCH] c++: Private parent access check for using decls [PR19377]

[Jason Merrill via Gcc-patches]

On 2/5/21 12:28 PM, Anthony Sharp wrote:

Hi Marek,


+  if ((TREE_CODE (parent_field) == USING_DECL)


This first term doesn't need to be wrapped in ().


I normally wouldn't use the (), but I think that's how Jason likes it
so that's why I did it. I guess it makes it more readable.


Ah, no, I don't see any need for the extra () there.  When I asked for 
added parens previously:



+   if (parent_binfo != NULL_TREE
+   && context_for_name_lookup (decl)
+   != BINFO_TYPE (parent_binfo))


Here you want parens around the second != expression and its != token
aligned with "context"


The parens are to help the editor line up the last line properly.  If 
the subexpression fits on one line, the parens aren't needed.



We usually use dg-do compile.


True, but wouldn't that technically be slower? I would agree if it
were more than just error-handling code.


No; "compile" means translate from C++ to assembly, "assemble" means 
that, plus call the assembler to produce an object file.  Though since 
compilation errors out, assembling never happens.



+  if ((TREE_CODE (parent_field) == USING_DECL)
+&& TREE_PRIVATE (parent_field)
+&& (DECL_NAME (decl) == DECL_NAME (parent_field)))
+   return parent_field;


Following our discussion about an earlier revision, this will often 
produce the right using-declaration, but might not if two functions of 
the same name are imported from different bases.  If I split the 
using-declaration in using9.C into two, i.e.


  using A2::i; // { dg-message "declared" }
  using A::i;


then I get


wa.ii: In member function ‘void C::f()’:
wa.ii:28:7: error: ‘int A::i()’ is private within this context
   28 | i();
  |   ^
wa.ii:20:13: note: declared private here
   20 |   using A2::i;


pointing out the wrong using-declaration because it happens to be first. 
 You could bring back your lookup from the earlier patch and look 
through the result to see if the function we're complaining about is 
part of what the particular using-decl brings in.


I tried both: 
(SAME_BINFO_TYPE_P (context_for_name_lookup (decl), parent_binfo))

and
(SAME_BINFO_TYPE_P (TYPE_BINFO (context_for_name_lookup (decl)), parent_binfo))


I think you want

SAME_BINFO_TYPE_P (context_for_name_lookup (decl),
   BINFO_TYPE (parent_binfo))

i.e. just change same_type_p to SAME_BINFO_TYPE_P.


  tree parent_binfo = get_parent_with_private_access (decl,
- basetype_path);
+  basetype_path);


This line was indented properly before, and is changed to be indented 
one space too far.



+ diag_location = get_class_access_diagnostic_decl (parent_binfo,
+  diag_decl);


This line needs one more space.


  complain_about_access (decl, diag_decl, diag_location, true,
-parent_access);
+   access_failure_reason);


This line, too.


+};
\ No newline at end of file


Let's add a newline.

Jason

Re: [PATCH 1/4] c++: Avoid building garbage trees from tsubst_requires_expr

[Jason Merrill via Gcc-patches]

On 2/8/21 2:03 PM, Patrick Palka wrote:

Since we no longer partially instantiate REQUIRES_EXPRs, we don't need
to rebuild its requirements during tsubst_requires_expr.


OK.


gcc/cp/ChangeLog:

* constraint.cc (tsubst_simple_requirement): Just return
boolean_true_node on success.
(tsubst_type_requirement): Likewise.
(tsubst_compound_requirement): Likewise.
(tsubst_nested_requirement): Likewise.
(tsubst_requirement_body): Remove.
(check_constaint_variables): Rename to ...
(check_constraint_variables): ... this.
(tsubst_constraint_variables): Adjust.
(tsubst_requires_expr): Fold tsubst_requirement_body into here.
---
  gcc/cp/constraint.cc | 46 ++--
  1 file changed, 14 insertions(+), 32 deletions(-)

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 31e0fb5079a..3e599fe8c47 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1962,7 +1962,7 @@ tsubst_simple_requirement (tree t, tree args, subst_info 
info)
tree expr = tsubst_valid_expression_requirement (t0, args, info);
if (expr == error_mark_node)
  return error_mark_node;
-  return finish_simple_requirement (EXPR_LOCATION (t), expr);
+  return boolean_true_node;
  }
  
  /* Substitute through the type requirement.  */

@@ -1974,7 +1974,7 @@ tsubst_type_requirement (tree t, tree args, subst_info 
info)
tree type = tsubst (t0, args, info.complain, info.in_decl);
if (type == error_mark_node)
  return error_mark_node;
-  return finish_type_requirement (EXPR_LOCATION (t), type);
+  return boolean_true_node;
  }
  
  /* True if TYPE can be deduced from EXPR.  */

@@ -2080,8 +2080,7 @@ tsubst_compound_requirement (tree t, tree args, 
subst_info info)
return error_mark_node;
  }
  
-  return finish_compound_requirement (EXPR_LOCATION (t),

- expr, type, noexcept_p);
+  return boolean_true_node;
  }
  
  static tree

@@ -2100,7 +2099,7 @@ tsubst_nested_requirement (tree t, tree args, subst_info 
info)
  }
if (result != boolean_true_node)
  return error_mark_node;
-  return result;
+  return boolean_true_node;
  }
  
  /* Substitute ARGS into the requirement T.  */

@@ -2125,24 +2124,6 @@ tsubst_requirement (tree t, tree args, subst_info info)
gcc_unreachable ();
  }
  
-/* Substitute ARGS into the list of requirements T. Note that

-   substitution failures here result in ill-formed programs. */
-
-static tree
-tsubst_requirement_body (tree t, tree args, subst_info info)
-{
-  tree result = NULL_TREE;
-  while (t)
-{
-  tree req = tsubst_requirement (TREE_VALUE (t), args, info);
-  if (req == error_mark_node)
-   return error_mark_node;
-  result = tree_cons (NULL_TREE, req, result);
-  t = TREE_CHAIN (t);
-}
-  return nreverse (result);
-}
-
  static tree
  declare_constraint_vars (tree parms, tree vars)
  {
@@ -2168,7 +2149,7 @@ declare_constraint_vars (tree parms, tree vars)
 if an error occurred.  */
  
  static tree

-check_constaint_variables (tree t, tree args, subst_info info)
+check_constraint_variables (tree t, tree args, subst_info info)
  {
tree types = NULL_TREE;
tree p = t;
@@ -2193,7 +2174,7 @@ static tree
  tsubst_constraint_variables (tree t, tree args, subst_info info)
  {
/* Perform a trial substitution to check for type errors.  */
-  tree parms = check_constaint_variables (t, args, info);
+  tree parms = check_constraint_variables (t, args, info);
if (parms == error_mark_node)
  return error_mark_node;
  
@@ -2253,19 +2234,20 @@ tsubst_requires_expr (tree t, tree args,

return t;
  }
  
-  tree parms = REQUIRES_EXPR_PARMS (t);

-  if (parms)
+  if (tree parms = REQUIRES_EXPR_PARMS (t))
  {
parms = tsubst_constraint_variables (parms, args, info);
if (parms == error_mark_node)
return boolean_false_node;
  }
  
-  tree reqs = REQUIRES_EXPR_REQS (t);

-  reqs = tsubst_requirement_body (reqs, args, info);
-  if (reqs == error_mark_node)
-return boolean_false_node;
-
+  for (tree reqs = REQUIRES_EXPR_REQS (t); reqs; reqs = TREE_CHAIN (reqs))
+{
+  tree req = TREE_VALUE (reqs);
+  tree result = tsubst_requirement (req, args, info);
+  if (result == error_mark_node)
+   return boolean_false_node;
+}
return boolean_true_node;
  }
  

Re: [PATCH 2/4] c++: Preparatory type canonicalization fixes

[Jason Merrill via Gcc-patches]

On 2/8/21 2:03 PM, Patrick Palka wrote:

The subsequent patches revealed some latent type canonicalization
issues during normalization and satisfaction:

1. In tsubst_parameter_mapping, we're canonicalizing the arguments of a
TYPE_ARGUMENT_PACK only if 'arg' wasn't a TYPE_ARGUMENT_PACK to begin
with.

2. We currently set DECL_CONTEXT and CONSTRAINT_VAR_P on each of the
parameters introduced in a requires-expression _after_ we're done
processing the requirements.  But meanwhile we may have already
computed the canonical form of a type that depends on one of these
PARM_DECLs, which depends on the result of cp_tree_equal, which
depends on CONSTRAINT_VAR_P and DECL_CONTEXT.  So we must set these
fields earlier, before processing the requirements.

3. In do_auto_deduction, we use the result of finish_decltype_type later
as a template argument, so we should canonicalize the result too.
(While we're here, we should pass 'complain' to finish_decltype_type,
which fixes the testcase auto1.C below.)

gcc/cp/ChangeLog:

* constraint.cc (tsubst_parameter_mapping): Canonicalize the
arguments of a TYPE_ARGUMENT_PACK even if we've started with a
TYPE_ARGUMENT_PACK.
(finish_requires_expr): Don't set DECL_CONTEXT and
CONSTRAINT_VAR_P on each of the introduced parameters here.
* parser.c (cp_parser_requirement_parameter_list): Instead set
these fields earlier, here.

gcc/testsuite/ChangeLog:

* g++.dg/cpp1z/auto1.C: New test.
---
  gcc/cp/constraint.cc   | 25 -
  gcc/cp/parser.c| 12 
  gcc/cp/pt.c|  8 ++--
  gcc/testsuite/g++.dg/cpp1z/auto1.C | 13 +
  4 files changed, 39 insertions(+), 19 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp1z/auto1.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 3e599fe8c47..39c97986082 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -2319,15 +2319,15 @@ tsubst_parameter_mapping (tree map, tree args, 
subst_info info)
  new_arg = tsubst_template_arg (arg, args, complain, in_decl);
  if (TYPE_P (new_arg))
new_arg = canonicalize_type_argument (new_arg, complain);
- if (TREE_CODE (new_arg) == TYPE_ARGUMENT_PACK)
+   }
+  if (TREE_CODE (new_arg) == TYPE_ARGUMENT_PACK)
+   {
+ tree pack_args = ARGUMENT_PACK_ARGS (new_arg);
+ for (int i = 0; i < TREE_VEC_LENGTH (pack_args); i++)
{
- tree pack_args = ARGUMENT_PACK_ARGS (new_arg);
- for (int i = 0; i < TREE_VEC_LENGTH (pack_args); i++)
-   {
- tree& pack_arg = TREE_VEC_ELT (pack_args, i);
- if (TYPE_P (pack_arg))
-   pack_arg = canonicalize_type_argument (pack_arg, complain);
-   }
+ tree& pack_arg = TREE_VEC_ELT (pack_args, i);
+ if (TYPE_P (pack_arg))
+   pack_arg = canonicalize_type_argument (pack_arg, complain);
}
}
if (new_arg == error_mark_node)
@@ -3253,15 +3253,6 @@ evaluate_concept_check (tree check, tsubst_flags_t 
complain)
  tree
  finish_requires_expr (location_t loc, tree parms, tree reqs)
  {
-  /* Modify the declared parameters by removing their context
- so they don't refer to the enclosing scope and explicitly
- indicating that they are constraint variables. */
-  for (tree parm = parms; parm; parm = DECL_CHAIN (parm))
-{
-  DECL_CONTEXT (parm) = NULL_TREE;
-  CONSTRAINT_VAR_P (parm) = true;
-}
-
/* Build the node. */
tree r = build_min (REQUIRES_EXPR, boolean_type_node, parms, reqs, 
NULL_TREE);
TREE_SIDE_EFFECTS (r) = false;
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 5da8670f0e2..fef10b9661d 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -28779,6 +28779,18 @@ cp_parser_requirement_parameter_list (cp_parser 
*parser)
if (!parens.require_close (parser))
  return error_mark_node;
  
+  /* Modify the declared parameters by removing their context

+ so they don't refer to the enclosing scope and explicitly
+ indicating that they are constraint variables. */
+  for (tree parm = parms; parm; parm = TREE_CHAIN (parm))
+{
+  if (parm == void_list_node || parm == explicit_void_list_node)
+   break;
+  tree decl = TREE_VALUE (parm);
+  DECL_CONTEXT (decl) = NULL_TREE;
+  CONSTRAINT_VAR_P (decl) = true;
+}
+
return parms;
  }
  
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c

index 3605b67e424..bceb942e79a 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -29478,9 +29478,13 @@ do_auto_deduction (tree type, tree init, tree 
auto_node,
 || ((TREE_CODE (init) == COMPONENT_REF
  || TREE_CODE (init) == SCOPE_REF)
 && !REF_PARENTHESIZED_P (init)));
+  tree deduced = finish_decltype_type (init, id, complain);
+  de

Re: [PATCH] don't enable DWARF5 by default on Windows (PR98860)

[Richard Biener via Gcc-patches]

On February 8, 2021 10:44:26 PM GMT+01:00, Mikael Pettersson via Gcc-patches 
 wrote:
>PR98860 is a gcc-11 regression where bootstrap fails on Windows since
>the switch to enable DWARF5 by default. The symptoms are that
>executables generated by the stage1 compiler fail to run with "Exec
>format error", which confuses subsequent configure steps and causes
>hard errors. This happens even with the very latest binutils master.
>
>Fixed by updating SUBTARGET_OVERRIDE_OPTIONS to set dwarf_version to 4
>unless the user explicitly requested another version. I see some other
>targets did the same.
>
>Tested on Cygwin64 and mingw-w64.
>
>Ok for master?

It might be better to expose the default via configure (I've heard desires to 
control that elsewhere). 

Richard. 

>2021-02-08  Mikael Pettersson  
>
>PR bootstrap/98860
>* config/i386/cygming.h (SUBTARGET_OVERRIDE_OPTIONS): Override
>dwarf_version to 4.
>
>--- a/gcc/config/i386/cygming.h
>+++ b/gcc/config/i386/cygming.h
>@@ -209,6 +209,9 @@ along with GCC; see the file COPYING3.  If not see
>#define SUBTARGET_OVERRIDE_OPTIONS
>\
>do {  
>\
>flag_pic = TARGET_64BIT ? 1 : 0;  \
>+  /* DWARF5 currently does not work on Windows. */
>\
>+  if (!global_options_set.x_dwarf_version)
>\
>+dwarf_version = 4;
>\
> } while (0)
>
> /* Define this macro if references to a symbol must be treated

[PATCH, V2] Add conversions between _Float128 and Decimal.

[Michael Meissner via Gcc-patches]

[PATCH V2] Add conversions between _Float128 and Decimal.

This patch implements conversions between _Float128 and the 3 Decimal floating
types.  It does this by extendending the dfp-bit conversions to add a new
binary floating point type (KF), and doing the conversions in the same manner
as the other binary/decimal conversions.

For conversions from _Float128 to Decimal, this patch uses a function
(__sprintfkf) instead of the sprintf function to convert long double values to
strings.  The __sprintfkf function determines if GLIBC 2.32 or newer is used
and calls the IEEE 128-bit version of sprintf (__sprintfieee128).  If the GLIBC
is earlier than 2.32, the code will convert _Float128 to __ibm128 and then use
the normal sprintf to convert this value.

For conversions from Decimal to _Float128, this patch uses a function
(__strtokf) instead of strtold to convert the strings from the Decimal
conversion to long double.  The __strtokf function determines if GLIBC 2.32 or
newer is used, and if it is, calls the IEEE 128-bit version (__strtoieee128).
If the GLIBC is earlier than 2.32, the code will call strtold and convert the
__ibm128 value to _Float128.

These functions will primarily be used if/when the default PowerPC long double
type is changed to IEEE 128-bit, but they could also be used if the user
explicitly converts _Float128 to/from a Decimal type.

I have tested this patch by doing builds, bootstraps, and make check with 3
builds on a power9 little endian server:

*   Build one used the default long double being IBM 128-bit;
*   Build two set the long double default to IEEE 128-bit; (and)
*   Build three set the long double default to 64-bit.

The compilers built fine providing I recompiled gmp, mpc, and mpfr with the
appropriate long double options.  There were a few differences in the test
suite runs that will be addressed in later patches, but over all it works
well.  This patch is required to be able to build a toolchain where the default
long double is IEEE 128-bit.  Can I check this patch into the master branch for
GCC 11?

One test case relating to Decimal fails if I build a compiler where the default
is IEEE 128-bit:

*   c-c++-common/dfp/convert-bfp-11.c

I have patches for this test, and they have been submitted separately.

I have also built compilers with this patch on a big endian power8 system that
has both 32-bit and 64-bit support.  There were no regressions in running these
tests on the system.

Compared to the previous version of the patch, this version uses 2021 copyright
dates without mentioning other years.  I also rewrote some of the commentary
that Will mentioned to be clearer.

libgcc/
2021-02-09  Michael Meissner  

* config/rs6000/_dd_to_kf.c: New file.
* config/rs6000/_kf_to_dd.c: New file.
* config/rs6000/_kf_to_sd.c: New file.
* config/rs6000/_kf_to_td.c: New file.
* config/rs6000/_sd_to_kf.c: New file.
* config/rs6000/_sprintfkf.c: New file.
* config/rs6000/_sprintfkf.h: New file.
* config/rs6000/_strtokf.h: New file.
* config/rs6000/_strtokf.c: New file.
* config/rs6000/_td_to_kf.c: New file.
* config/rs6000/quad-float128.h: Add new declarations.
* config/rs6000/t-float128 (fp128_dec_funcs): New macro.
(fp128_decstr_funcs): New macro.
(ibm128_dec_funcs): New macro.
(fp128_ppc_funcs): Add the new conversions.
(fp128_dec_objs): Force Decimal <-> __float128 conversions to be
compiled with -mabi=ieeelongdouble.
(fp128_decstr_objs): Force __float128 <-> string conversions to be
compiled with -mabi=ibmlongdouble.
(ibm128_dec_objs): Force Decimal <-> __float128 conversions to be
compiled with -mabi=ieeelongdouble.
(FP128_CFLAGS_DECIMAL): New macro.
(IBM128_CFLAGS_DECIMAL): New macro.
* dfp-bit.c (DFP_TO_BFP): Add PowerPC _Float128 support.
(BFP_TO_DFP): Add PowerPC _Float128 support.
* dfp-bit.h (BFP_KIND): Add new binary floating point kind for
IEEE 128-bit floating point.
(DFP_TO_BFP): Add PowerPC _Float128 support.
(BFP_TO_DFP): Add PowerPC _Float128 support.
(BFP_SPRINTF): New macro.
---
 libgcc/config/rs6000/_dd_to_kf.c | 37 ++
 libgcc/config/rs6000/_kf_to_dd.c | 37 ++
 libgcc/config/rs6000/_kf_to_sd.c | 37 ++
 libgcc/config/rs6000/_kf_to_td.c | 37 ++
 libgcc/config/rs6000/_sd_to_kf.c | 37 ++
 libgcc/config/rs6000/_sprintfkf.c| 57 
 libgcc/config/rs6000/_sprintfkf.h| 28 ++
 libgcc/config/rs6000/_strtokf.c  | 56 +++
 libgcc/config/rs6000/_strtokf.h  | 27 +
 libgcc/config/rs6000/_td_to_kf.c | 37 ++
 libgcc/config/rs6000/quad-float128.h |  8 
 libgcc/config/rs6000/t-float128  | 37 +-
 libgcc/dfp-

Re: [PATCH] adjust "partly out of bounds" warning (PR 98503)

[Richard Biener via Gcc-patches]

On Tue, Feb 9, 2021 at 1:04 AM Martin Sebor via Gcc-patches
 wrote:
>
> On 2/8/21 12:09 PM, Jeff Law wrote:
> >
> >
> > On 2/3/21 3:45 PM, Martin Sebor wrote:
> >> On 2/3/21 2:57 PM, Jeff Law wrote:
> >>>
> >>>
> >>> On 1/28/21 4:03 PM, Martin Sebor wrote:
>  The GCC 11 -Warray-bounds enhancement to diagnose accesses whose
>  leading offset is in bounds but whose trailing offset is not has
>  been causing some confusion.  When the warning is issued for
>  an access to an in-bounds member via a pointer to a struct that's
>  larger than the pointed-to object, phrasing this strictly invalid
>  access in terms of array subscripts can be misleading, especially
>  when the source code doesn't involve any arrays or indexing.
> 
>  Since the problem boils down to an aliasing violation much more
>  so than an actual out-of-bounds access, the attached patch adjusts
>  the code to issue a -Wstrict-aliasing warning in these cases instead
>  of -Warray-bounds.  In addition, since the aliasing assumptions in
>  GCC can be disabled by -fno-strict-aliasing, the patch also makes
>  these instances of the warning conditional on -fstrict-aliasing
>  being in effect.
> 
>  Martin
> 
>  gcc-98503.diff
> 
>  PR middle-end/98503 -Warray-bounds when -Wstrict-aliasing would be
>  more appropriate
> 
>  gcc/ChangeLog:
> 
>   PR middle-end/98503
>   * gimple-array-bounds.cc (array_bounds_checker::check_mem_ref):
>   Issue -Wstrict-aliasing for a subset of violations.
>   (array_bounds_checker::check_array_bounds):  Set new member.
>   * gimple-array-bounds.h (array_bounds_checker::cref_of_mref): New
>   data member.
> 
>  gcc/testsuite/ChangeLog:
> 
>   PR middle-end/98503
>   * g++.dg/warn/Warray-bounds-10.C: Adjust text of expected warnings.
>   * g++.dg/warn/Warray-bounds-11.C: Same.
>   * g++.dg/warn/Warray-bounds-12.C: Same.
>   * g++.dg/warn/Warray-bounds-13.C: Same.
>   * gcc.dg/Warray-bounds-63.c: Avoid -Wstrict-aliasing.  Adjust text
>   of expected warnings.
>   * gcc.dg/Warray-bounds-66.c: Adjust text of expected warnings.
>   * gcc.dg/Wstrict-aliasing-2.c: New test.
>   * gcc.dg/Wstrict-aliasing-3.c: New test.
> >>> What I don't like here is the strict-aliasing warnings inside the file
> >>> and analysis phase for array bounds checking.
> >>>
> >>> ISTM that catching this at cast time would be better.  So perhaps in
> >>> build_c_cast and the C++ equivalent?
> >>>
> >>> It would mean we're warning at the cast site rather than the
> >>> dereference, but that may ultimately be better for the warning anyway.
> >>> I'm not sure.
> >>
> >> I had actually experimented with a this (in the middle end, and only
> >> for accesses) but even that caused so many warnings that I abandoned
> >> it, at least for now.  -Warray-bounds has the benefit of flow analysis
> >> (and dead code elimination).  In the front end it would have neither
> >> and be both excessively noisy and ineffective at the same time.  For
> >> example:
> > I think we agree that this really is an aliasing issue and I would go
> > further and claim that it has nothing to do with array bounds checking.
> > Therefore warning for it within gimple-array-bounds just seems wrong.
> >
> > I realize that you like having DCE run and the ability to look at
> > offsets and such, but it really feels like the wrong place to do this.
> > Aliasing issues are also more of a front-end thing since the language
> > defines what is and what is not valid aliasing -- one might even argue
> > that any aliasing warning needs to be identified by the front-ends
> > exclusively (though possibly pruned away later).
>
> The aliasing restrictions are on accesses, which are [defined in
> C as] execution-time actions on objects.  Analyzing execution-time
> actions unavoidably depends on flow analysis which the front ends
> have only very limited support for (simple expressions only).
> I gave one example showing how the current -Wstrict-aliasing in
> the front end is ineffective against all but the most simplistic
> bugs, but there are many more.  For instance:
>
>int i;
>void *p = &i;// valid
>float *q = p;// valid
>*q = 0;  // aliasing violation
>
> This bug is easily detectable in the middle end but impossible
> to do in the front end (same as all other invalid accesses).

But the code is valid in GIMPLE which allows to re-use the 'int i' storage
with storing using a different type.

> Whether this is done in gimple-array-bounds or some other pass seems
> to me like a minor implementation detail.  It naturally came out of
> an enhancement I implemented there (which would detect the above
> with float replaced by any larger type as an out-of-bounds access)
> but I have no problem with moving this subset to some other pass
> (or d