Re: [PATCH] IBM Z: Fix testcase vcond-shift.c

[Stefan Schulze Frielinghaus via Gcc-patches]

On Tue, Mar 02, 2021 at 08:08:14AM +0100, Andreas Krebbel wrote:
> On 3/1/21 5:00 PM, Stefan Schulze Frielinghaus wrote:
> > As of commit 3a6e3ad38a17a03ee0139b49a0946e7b9ded1eb1 expressions
> > x CMP y ? -1 : 0 are fold into x CMP y.  Due to this we do not see
> > shifts anymore after expand in our testcases but comparisons.  Thus
> > replace instructions vesraX by corresponding vchX.  Keep testcases
> > vchX_{lt,gt} where only a relational comparison is done and no shift in
> > order to keep test coverage for vectorization.
> 
> The vcond-shift optimization verified by the testcase is currently 
> implemented in s390_expand_vcond
> but due to the common code change we go the vec_cmp route now. So we probably 
> should do the same
> also in s390_expand_vec_compare now. Perhaps like this ... it appears to fix 
> the testcase for me:
> 
> diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
> index 9d2cee950d0b..9d9f5a0f6f4e 100644
> --- a/gcc/config/s390/s390.c
> +++ b/gcc/config/s390/s390.c
> @@ -6562,6 +6562,7 @@ s390_expand_vec_compare (rtx target, enum rtx_code cond,
> 
>if (GET_MODE_CLASS (GET_MODE (cmp_op1)) == MODE_VECTOR_FLOAT)
>  {
> +  cmp_op2 = force_operand (cmp_op2, 0);
>switch (cond)
> {
>   /* NE a != b -> !(a == b) */
> @@ -6600,6 +6601,19 @@ s390_expand_vec_compare (rtx target, enum rtx_code 
> cond,
>  }
>else
>  {
> +  /* Turn x < 0 into x >> (bits - )  */
> +  if (cond == LT && cmp_op2 == CONST0_RTX (mode))
> +   {
> + int shift = GET_MODE_BITSIZE (GET_MODE_INNER (mode)) - 1;
> + rtx res = expand_simple_binop (mode, ASHIFTRT, cmp_op1,
> +GEN_INT (shift), target,
> +0, OPTAB_DIRECT);
> + if (res != target)
> +   emit_move_insn (target, res);
> + return;
> +   }
> +  cmp_op2 = force_operand (cmp_op2, 0);
> +
>switch (cond)
> {
>   /* NE: a != b -> !(a == b) */
> diff --git a/gcc/config/s390/vector.md b/gcc/config/s390/vector.md
> index bc52211c55e5..c80d582a300d 100644
> --- a/gcc/config/s390/vector.md
> +++ b/gcc/config/s390/vector.md
> @@ -1589,7 +1589,7 @@
>[(set (match_operand:  0 "register_operand" "")
> (match_operator: 1 "vcond_comparison_operator"
>   [(match_operand:V_HW 2 "register_operand" "")
> -  (match_operand:V_HW 3 "register_operand" "")]))]
> +  (match_operand:V_HW 3 "nonmemory_operand" "")]))]
>"TARGET_VX"
>  {
>s390_expand_vec_compare (operands[0], GET_CODE(operands[1]), operands[2], 
> operands[3]);

Sounds great to me.  Also eliminates the extra vzero :)

Cheers,
Stefan

> 
> Andreas
> 
> 
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > * gcc.target/s390/vector/vcond-shift.c: Replace vesraX
> > instructions by corresponding vchX instructions.
> > ---
> >  .../gcc.target/s390/vector/vcond-shift.c  | 31 ++-
> >  1 file changed, 17 insertions(+), 14 deletions(-)
> > 
> > diff --git a/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c 
> > b/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c
> > index a6b4e97aa50..9e472aef960 100644
> > --- a/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c
> > +++ b/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c
> > @@ -3,10 +3,13 @@
> >  /* { dg-do compile { target { s390*-*-* } } } */
> >  /* { dg-options "-O3 -march=z13 -mzarch" } */
> >  
> > -/* { dg-final { scan-assembler-times "vesraf\t%v.?,%v.?,31" 6 } } */
> > -/* { dg-final { scan-assembler-times "vesrah\t%v.?,%v.?,15" 6 } } */
> > -/* { dg-final { scan-assembler-times "vesrab\t%v.?,%v.?,7" 6 } } */
> > -/* { dg-final { scan-assembler-not "vzero\t*" } } */
> > +/* { dg-final { scan-assembler-times "vzero\t" 9 } } */
> > +/* { dg-final { scan-assembler-times "vchf\t" 6 } } */
> > +/* { dg-final { scan-assembler-times "vesraf\t%v.?,%v.?,1" 2 } } */
> > +/* { dg-final { scan-assembler-times "vchh\t" 6 } } */
> > +/* { dg-final { scan-assembler-times "vesrah\t%v.?,%v.?,1" 2 } } */
> > +/* { dg-final { scan-assembler-times "vchb\t" 6 } } */
> > +/* { dg-final { scan-assembler-times "vesrab\t%v.?,%v.?,1" 2 } } */
> >  /* { dg-final { scan-assembler-times "vesrlf\t%v.?,%v.?,31" 4 } } */
> >  /* { dg-final { scan-assembler-times "vesrlh\t%v.?,%v.?,15" 4 } } */
> >  /* { dg-final { scan-assembler-times "vesrlb\t%v.?,%v.?,7" 4 } } */
> > @@ -15,19 +18,19 @@
> >  #define ITER(X) (2 * (16 / sizeof (X[1])))
> >  
> >  void
> > -vesraf_div (int *x)
> > +vchf_vesraf_div (int *x)
> >  {
> >int i;
> >int *xx = __builtin_assume_aligned (x, 8);
> >  
> >/* Should expand to (xx + (xx < 0 ? 1 : 0)) >> 1
> > - which in turn should get simplified to (xx + (xx >> 31)) >> 1.  */
> > + which in turn should get simplified to (xx - (xx < 0)) >> 1.  */
> >for (i = 0; i < ITER (xx); i++)
> >  xx[i] = xx[i] / 2;
> >  }
> >  
> >  void
> > -vesrah_div (short *x)
> > 

Re: PR analyzer/94362 Partial Fix

[brian.sobulefsky via Gcc-patches]

Hi,

It may not be worth altering at this point, but it seems like it would leave 
less
bugs open if all the constraints go in as "closed" ranges and all evals are
translated to closed intervals. So, if (idx > 0) and if (idx >= 1) are the same
constraint. I know this won't be an option for eventual float support, but
that is a different can of worms. For integers, you can fix it at those entry 
points
and then all other subroutines can ignore the issue of open or closed ranges.

I fully understand the eye glaze and did not want to have to write it that
way. I am thinking if there is a cleaner way to do it. Anyway, that is why I
put a comment in each case to derive the result. This issue of "sides of the
condition" and "inverted operator" as you call it in some places is a recurring
theme. It is especially irritating when we lose commutativity, as we do with 
MINUS.

Adding logic in my subroutine for MULT or DIV is not hard, handling overflow
is a bit more involved. At the very least, we would need to know what the max 
or min
of a particular variable is, which might be in the type tree. We would also 
need to
define how we want to handle the issue.

The problem is (and I have been thinking about this a lot in terms of constraint
merging), there are currently no "or constraints," which would be helpful in 
merging too.
So for overflow, when you have something like

if (idx > 0)
 {
  idx += num;
  __analyzer_eval(idx > num);
 }

you have gone from a single constraint (idx > 0), to an "or condition"
(idx > num || idx < MIN_INT + num). The only solution now, other than ignoring
overflow as a bug that is tolerated, is to just pass it off as unhandled (and
therefore UNKNOWN). Perhaps you may want to add overflow as one of your analyzer
warnings if it cannot be ruled out?

I did try to run a test with a simple || in the condition before just to see 
what
would happen, and as you probably know it is not handled at all. I did not watch
in gdb, but it is obvious from constraint-manager.cc that there is nothing to 
handle
it. I think I actually did an __analyzer_eval() of the same || condition 
verbatim
that was in the if() conditional and still got an UNKNOWN.

It is a pretty intrusive change to add logic for that, which is why I have not
done any work on it yet. Without the concept of "or" I don't see how we could
handle overflow, but maybe you don't really want to handle it anyway, but
rather just emit a warning if it might be considered poor practice to rely
on something that is technically machine dependent anyway.

Re: [PATCH] IBM Z: Fix testcase vcond-shift.c

[Andreas Krebbel via Gcc-patches]

On 3/1/21 5:00 PM, Stefan Schulze Frielinghaus wrote:
> As of commit 3a6e3ad38a17a03ee0139b49a0946e7b9ded1eb1 expressions
> x CMP y ? -1 : 0 are fold into x CMP y.  Due to this we do not see
> shifts anymore after expand in our testcases but comparisons.  Thus
> replace instructions vesraX by corresponding vchX.  Keep testcases
> vchX_{lt,gt} where only a relational comparison is done and no shift in
> order to keep test coverage for vectorization.

The vcond-shift optimization verified by the testcase is currently implemented 
in s390_expand_vcond
but due to the common code change we go the vec_cmp route now. So we probably 
should do the same
also in s390_expand_vec_compare now. Perhaps like this ... it appears to fix 
the testcase for me:

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 9d2cee950d0b..9d9f5a0f6f4e 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -6562,6 +6562,7 @@ s390_expand_vec_compare (rtx target, enum rtx_code cond,

   if (GET_MODE_CLASS (GET_MODE (cmp_op1)) == MODE_VECTOR_FLOAT)
 {
+  cmp_op2 = force_operand (cmp_op2, 0);
   switch (cond)
{
  /* NE a != b -> !(a == b) */
@@ -6600,6 +6601,19 @@ s390_expand_vec_compare (rtx target, enum rtx_code cond,
 }
   else
 {
+  /* Turn x < 0 into x >> (bits - )  */
+  if (cond == LT && cmp_op2 == CONST0_RTX (mode))
+   {
+ int shift = GET_MODE_BITSIZE (GET_MODE_INNER (mode)) - 1;
+ rtx res = expand_simple_binop (mode, ASHIFTRT, cmp_op1,
+GEN_INT (shift), target,
+0, OPTAB_DIRECT);
+ if (res != target)
+   emit_move_insn (target, res);
+ return;
+   }
+  cmp_op2 = force_operand (cmp_op2, 0);
+
   switch (cond)
{
  /* NE: a != b -> !(a == b) */
diff --git a/gcc/config/s390/vector.md b/gcc/config/s390/vector.md
index bc52211c55e5..c80d582a300d 100644
--- a/gcc/config/s390/vector.md
+++ b/gcc/config/s390/vector.md
@@ -1589,7 +1589,7 @@
   [(set (match_operand:  0 "register_operand" "")
(match_operator: 1 "vcond_comparison_operator"
  [(match_operand:V_HW 2 "register_operand" "")
-  (match_operand:V_HW 3 "register_operand" "")]))]
+  (match_operand:V_HW 3 "nonmemory_operand" "")]))]
   "TARGET_VX"
 {
   s390_expand_vec_compare (operands[0], GET_CODE(operands[1]), operands[2], 
operands[3]);

Andreas


> 
> gcc/testsuite/ChangeLog:
> 
>   * gcc.target/s390/vector/vcond-shift.c: Replace vesraX
>   instructions by corresponding vchX instructions.
> ---
>  .../gcc.target/s390/vector/vcond-shift.c  | 31 ++-
>  1 file changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c 
> b/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c
> index a6b4e97aa50..9e472aef960 100644
> --- a/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c
> +++ b/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c
> @@ -3,10 +3,13 @@
>  /* { dg-do compile { target { s390*-*-* } } } */
>  /* { dg-options "-O3 -march=z13 -mzarch" } */
>  
> -/* { dg-final { scan-assembler-times "vesraf\t%v.?,%v.?,31" 6 } } */
> -/* { dg-final { scan-assembler-times "vesrah\t%v.?,%v.?,15" 6 } } */
> -/* { dg-final { scan-assembler-times "vesrab\t%v.?,%v.?,7" 6 } } */
> -/* { dg-final { scan-assembler-not "vzero\t*" } } */
> +/* { dg-final { scan-assembler-times "vzero\t" 9 } } */
> +/* { dg-final { scan-assembler-times "vchf\t" 6 } } */
> +/* { dg-final { scan-assembler-times "vesraf\t%v.?,%v.?,1" 2 } } */
> +/* { dg-final { scan-assembler-times "vchh\t" 6 } } */
> +/* { dg-final { scan-assembler-times "vesrah\t%v.?,%v.?,1" 2 } } */
> +/* { dg-final { scan-assembler-times "vchb\t" 6 } } */
> +/* { dg-final { scan-assembler-times "vesrab\t%v.?,%v.?,1" 2 } } */
>  /* { dg-final { scan-assembler-times "vesrlf\t%v.?,%v.?,31" 4 } } */
>  /* { dg-final { scan-assembler-times "vesrlh\t%v.?,%v.?,15" 4 } } */
>  /* { dg-final { scan-assembler-times "vesrlb\t%v.?,%v.?,7" 4 } } */
> @@ -15,19 +18,19 @@
>  #define ITER(X) (2 * (16 / sizeof (X[1])))
>  
>  void
> -vesraf_div (int *x)
> +vchf_vesraf_div (int *x)
>  {
>int i;
>int *xx = __builtin_assume_aligned (x, 8);
>  
>/* Should expand to (xx + (xx < 0 ? 1 : 0)) >> 1
> - which in turn should get simplified to (xx + (xx >> 31)) >> 1.  */
> + which in turn should get simplified to (xx - (xx < 0)) >> 1.  */
>for (i = 0; i < ITER (xx); i++)
>  xx[i] = xx[i] / 2;
>  }
>  
>  void
> -vesrah_div (short *x)
> +vchh_vesrah_div (short *x)
>  {
>int i;
>short *xx = __builtin_assume_aligned (x, 8);
> @@ -38,7 +41,7 @@ vesrah_div (short *x)
>  
>  
>  void
> -vesrab_div (signed char *x)
> +vchb_vesrab_div (signed char *x)
>  {
>int i;
>signed char *xx = __builtin_assume_aligned (x, 8);
> @@ -50,7 +53,7 @@ vesrab_div (signed char *x)
>  
>  
>  int
> -vesraf_lt (int *x)
> 

Re: [PATCH] avoid -Wnonull for dynamic_cast (PR 99251)

[Jason Merrill via Gcc-patches]

On 3/1/21 7:44 PM, Martin Sebor wrote:

On 3/1/21 1:33 PM, Jason Merrill wrote:

On 3/1/21 12:10 PM, Martin Sebor wrote:

On 2/24/21 8:13 PM, Jason Merrill wrote:

On 2/24/21 5:25 PM, Martin Sebor wrote:

In r11-6900 (PR 98646 - static_cast confuses -Wnonnull) we decided
that issuing -Wnonnull for dereferencing the result of dynamic_cast
was helpful despite the false positives it causes when the pointer
is guaranteed not to be null because of a prior test.

The test case in PR 99251 along with the feedback I got from Martin
Liska have convinced me it was not the right decision.

The attached patch arranges for dynamic_cast to also suppress 
-Wnonnull

analogously to static_cast.  Since there already is a helper function
that builds the if-not-null test (ifnonnull) and sets TREE_NO_WARNING,
I factored out the corresponding code from build_base_path that sets
the additional TREE_NO_WARNING bit for static_cast into the function
and called it from both places.  I also renamed the function to make
its purpose clearer and for consistency with other build_xxx APIs.


Let's call it build_if_nonnull, as it builds the COND_EXPR as well 
as the test.


Done.




+  /* The dynamic_cast might fail but so a warning might be justified
+ but not when the operand is guarded.  See pr99251.  */
+  if (B *q = p->bptr ())
+    dynamic_cast(q)->g ();    // { dg-bogus 
"\\\[-Wnonnull" }


This guard is no more necessary than it is for static_cast; both 
cases deal with null arguments.  Let's not add these checks to the 
testcases.


Done.



This guard doesn't check for the mentioned case of dynamic_cast 
failing because the B* does not in fact point to a C.


I think we can just change the dg-warning to dg-bogus.  Sure, 
dynamic_cast might fail, but AFAICT -Wnonnull isn't supposed to warn 
about arguments that *might* be null, only arguments that are 
*known* to be null.


Done.

I had added the 'if' to the test to make it clear why the warning
isn't expected.  I replaced it with a comment to that effect.



FWIW, I do think a warning for dynamic_cast to a pointer would be
helpful in the absence of an if statement in these cases:

   void f (B *p)
   {
 dynamic_cast(p)->g ();
   }

Not because p may be null but because the result of the cast may be
for a nonnull p.  If it's f's precondition that D is derived from B
(in addition to p being nonnull) the cast would be better written as
one to a reference to D.


Agreed, or if it isn't a precondition,

if (D* dp = dynamic_cast(p))
   dp->g();


Such a warning would need to be issued by the middle end and although
it could be controlled by a new option (say -Wdynamic-cast, along with
the cases discussed in PR 12277)


Sounds good.


I don't think issuing -Wnonnull in this case would be inappropriate.


I disagree; issuing -Wnonnull for this case would be wrong.  Again, 
-Wnonnull is supposed to warn when an argument *is* null, not when it 
*might* be null.


I think warning about this case is a good idea, just not as part of 
-Wnonnull.



Anyway, that's something to consider for GCC 12.  For now, please see
the revised patch.



* rtti.c (ifnonnull): Rename...
(build_nonnull_test): ...to this.  Set no-warning bit on COND_EXPR.


The ChangeLog needs updating.

+  /* Unlike static_cast, dynamic cast may fail for a nonnull operand 
but


Yes, but...


+ since the front end can't see if the cast is guarded against being
+ invoked with a null


No; my point was that whether the cast is guarded against being 
invoked with a null is no more relevant for dynamic_cast than it is 
for static_cast, as both casts give a null result for a null argument.


For this test, dynamic_cast is not significantly different from 
static_cast.  For both casts, the bug was the compiler warning about a 
nullptr that it introduced itself.


It seems like splitting hairs.  The comment (much as the original
if guard) is just meant to explain why -Wnonnull isn't expected in
case a new warning is added that detects the bad assumption above.
I want to make it clear that if/when that happens a failure here
doesn't mislead the author into thinking we don't want any warning
there at all.

I have reworded the comments yet again.




verify there's no warning.  See also pr99251.  */


Yes.

-  dynamic_cast(p->bptr ())->g ();   // { dg-warning 
"\\\[-Wnonnull" }
+  dynamic_cast(p)->g ();    // { dg-bogus 
"\\\[-Wnonnull" }


Please put back the ->bptr()s.


Done.


OK, thanks.

Jason

Re: PR analyzer/94362 Partial Fix

[David Malcolm via Gcc-patches]

On Sat, 2021-02-27 at 10:04 +, brian.sobulefsky wrote:
> Hi,
> 
> Please find a patch to fix part of the bug PR analyzer/94362.

Thanks.  Various comments inline below.

>  This bug is a
> false positive for a null dereference found when compiling openssl.
> The cause
> is the constraint_manager not knowing that i >= 0 within the for
> block:
> 
> for ( ; i-- > 0; )
> 
> The bug can be further reduced to the constraint manager not knowing
> that i >= 0
> within the if block:
> 
> if (i-- > 0)
> 
> which is not replicated for other operators, such as prefix
> decrement. The
> cause is that the constraint is applied to the initial_svalue of i,
> while it
> is a binop_svalue of i that enters the block (with op PLUS and arg1 -
> 1). The
> constraint_manager does not have any constraints for this svalue and
> has no
> handler. A handler has been added that essentially recurs on the
> remaining arg
> if the other arg and other side of the condition are both constants
> and the op
> is PLUS_EXPR or MINUS_EXPR.
> 
> This in essence fixed the problem, except an off by one error had
> been hiding
> in range::eval_condition. This error is hard to notice, because, for
> example,
> the code
> 
> if(idx > 0)
>   __analyzer_eval(idx >= 1);
> 
> will compile as (check -fdump-ipa-analyzer to see)
> 
> void test (int idx)
> {
>   _Bool _1;
>   int _2;
> 
>    :
>   if (idx_4(D) > 0)
>     goto ; [INV]
>   else
>     goto ; [INV]
> 
>    :
>   _1 = idx_4(D) > 0;
>   _2 = (int) _1;
>   __analyzer_eval (_2);
> 
>    :
>   return;
> 
> }
> 
> and will print "TRUE" to the screen, but as you can see, it is for
> the wrong
> reason, because we are not really testing the condition we wanted to
> test.
> 
> You can force the failure (i.e. "UNKNOWN") for yourself with the
> following:
> 
> void test(int idx)
> {
>   int check = 1;
>   if(idx > 0)
>     __analyzer_eval(idx >= check);
> }
> 
> which the compiler will not "fix" on us. 

Thank.  This looks like a good way to create DejaGnu tests that verify
the constraint_manager code, rather than accidentally testing the
optimizer.


> An examination of range::eval_condition
> should convince you that there is an off by one error. 

Yes, looking at the switch statement, the fact that LT_EXPR and LE_EXPR
share the same case suggests the boundaries aren't properly handled
(and the same for GT_EXPR and GE_EXPR)


> Incidentally, I might
> recommend doing away with "open intervals of integers" entirely.

What would the alternative be?

Note that in the range class a bound can have a NULL m_constant, in
which case that bound is a kind of null bound (the comments should
probably spell this out).

> When running the initial bug (the for loop), you will find that the
> analyzer
> prints "UNKNOWN" twice for the postfix operator, and "TRUE" "UNKNOWN"
> for other
> operators. This patch reduces postfix to the same state as the other
> operators.
> The second "UNKNOWN" seems to be due to a second "iterated" pass
> through the
> loop with a widening_svalue. A full fix of the bug will need a
> handler for the
> widening svalue, and much more critically, a correct merge of the
> constraints
> at the loop entrance. 

Sounds correct to me.

> That, unfortunately, looks to be a hard problem.

I think it's worth cleaning up this patch and getting this into trunk,
and leave the second part as a followup.

> This patch fixes a few xfails as noted in the commit message. These
> were tests
> that were evidently devised to test whether the analyzer would
> understand
> arithmetic being done on constrained values. Addition and subtraction
> is now
> working as expected, a handler for multiplication and division can be
> added.
> 
> As was noted in those test files, consideration at some point should
> be given to
> overflow.

Indeed.  I think the patch needs to take that into account when
updating bounds in eval_condition.

Various comments inline below throughout.


> commit d4052e8c273ca267f6dcf782084d60acfc50a609
> Author: Brian Sobulefsky 
> Date:   Sat Feb 27 00:36:40 2021 -0800
> 
> Changes to support eventual solution to bug PR analyzer/94362. This bug
> originated with a false positive null dereference during compilation of
> openssl. The bug is in effect caused by an error in constraint handling,
> specifically that within the for block:
> 
> for ( ; i-- > 0; )
>   {
>   }
> 
> the constraint_manager should know i >= 0 but does not. A reduced form of
> this bug was found where the constraint manager did not know within the if
> block:
> 
> if (i-- > 0)
>   {
>   }
> 
> that i >= 0. This latter error was only present for the postfix
> operators, and not for other forms, like --i > 0. It was due to the
> constraint being set for the initial_svalue associated with i, but a
> binop_svalue being what entered the if block for which no constraint
> rules existed.
> 
> By adding handling logic for a 

Re: [PATCH] c++: ICE with deduction guide in checking type-dep [PR99009, PR97034]

[Jason Merrill via Gcc-patches]

On 3/1/21 7:59 PM, Marek Polacek wrote:

On Mon, Mar 01, 2021 at 03:08:58PM -0500, Jason Merrill wrote:

On 3/1/21 2:54 PM, Marek Polacek wrote:

On Thu, Feb 25, 2021 at 10:45:29PM -0500, Jason Merrill via Gcc-patches wrote:

On 2/25/21 5:41 PM, Marek Polacek wrote:

On Thu, Feb 25, 2021 at 10:59:49AM -0500, Jason Merrill wrote:

On 2/12/21 6:12 PM, Marek Polacek wrote:

We represent deduction guides with FUNCTION_DECLs, but they are built
without DECL_CONTEXT


Hmm, that seems wrong: "A deduction-guide shall be declared in the
same scope as the corresponding class template and, for a member class
template, with the same access."  But it probably isn't necessary to change
this:


leading to an ICE in type_dependent_expression_p
on the assert that the type of a function template with no dependent
(innermost!) template arguments must be non-dependent.  Consider the
attached class-deduction79.C: we create a deduction guide:

  template G(T)-> E::G

we deduce T and create a partial instantiation:

  G(T) -> E::G [with T = int]

And then do_class_deduction wants to create a CALL_EXPR from the above
using build_new_function_call -> build_over_call which calls mark_used
-> maybe_instantiate_noexcept -> type_dependent_expression_p.

There, the innermost template arguments are non-dependent (), but
the fntype is dependent -- the return type is a TYPENAME_TYPE, and
since we have no DECL_CONTEXT, this check holds:

  /* Otherwise, if the function decl isn't from a dependent scope, it can't 
be
 type-dependent.  Checking this is important for functions with auto 
return
 type, which looks like a dependent type.  */
  if (TREE_CODE (expression) == FUNCTION_DECL
  && !(DECL_CLASS_SCOPE_P (expression)
   && dependent_type_p (DECL_CONTEXT (expression)))

whereupon we ICE.

Experiments with setting DECL_CONTEXT didn't pan out.


In c8 of the PR it looks like you were using the class itself as
DECL_CONTEXT; the quote above says that the right context is the enclosing
scope of the class.


Sadly, using CP_TYPE_CONTEXT (type) would result in a crash in
retrieve_specialization:

 /* There should be as many levels of arguments as there are
levels of parameters.  */
 gcc_assert (TMPL_ARGS_DEPTH (args)
 == (TREE_CODE (tmpl) == TEMPLATE_DECL
 ? TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (tmpl))
 : template_class_depth (DECL_CONTEXT (tmpl;


Yeah, probably simpler not to bother.


So perhaps we
just want to skip the assert for deduction guides, because they are
a little special.  Better ideas solicited.


In c3 you mention that one of the variants broke with r269093; this is
because my change to check CLASSTYPE_TEMPLATE_INSTANTIATION is false for the
template pattern itself (E).


And the original test started with my r11-1713 because using TREE_TYPE
directly instead of decltype (which is a non-deduced context) means we
can deduced from the argument.

But I think probably the right answer is to defer this deduction until the
enclosing scope is non-dependent, i.e. (untested)


Thanks.  That mostly works, except the new class-deduction-aggr[89].C
tests.  Consider 8:

namespace N {
template  struct S {
 template  S(T, U);
};
} // namespace N
template  struct E {
 template  struct G { T t; };
 void fn() { G{N::S{'a', 1}}; }
};

void
g ()
{
 E<1> e;
 e.fn ();
}

With your patch, when in do_class_deduction when processing_template_decl,
we just return.  When we call do_class_deduction again when p_t_d is 0,
maybe_aggr_guide returns early here:

 if (!CP_AGGREGATE_TYPE_P (type))
   return NULL_TREE

because G is not complete (and rightly so, we didn't instantiate it).  So
we aren't able to deduce the template parameters.  I'm not sure if I should
pursue this direction further.  :(


I think so; we just need to test CP_AGGREGATE_TYPE_P on the original
template pattern instead of the instantiation E<1>::G.


I'm sorry, I've got stuck again.

Yes, using the original template pattern helps us get past the
CP_AGGREGATE_TYPE_P check.

However, using TREE_TYPE (DECL_TI_TEMPLATE (tmpl)) as the type of the deduction 
guide
means the guide will be "template G(T)-> E< >::G" which
results in

class-deduction-aggr8.C:11:15: error: invalid use of dependent type 'typename 
E< >::G >'

which makes sense I guess: when we defer building up the guide until
we've instantiated E<1>, finding the dependent type E<> is not expected.


Yeah, I was only thinking to use the pattern for the aggregate check.


Ack.  Though I think I also have to use the pattern here:

   init = reshape_init (type, init, complain);

otherwise reshape_init returns a TARGET_EXPR and we immediately
crash in collect_ctor_idx_types because that only expects a CONSTRUCTOR.
And what we need to get is the type T -- of the constructor's index.
  

But creating the guide with "struct E<1>::G" as its type seems
wrong; I'm not even sure if a 

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

[Jason Merrill via Gcc-patches]

On 3/1/21 6:09 PM, Patrick Palka wrote:

On Mon, 1 Mar 2021, Jason Merrill wrote:


On 2/28/21 12:40 PM, Patrick Palka wrote:

On Fri, 12 Feb 2021, Jason Merrill wrote:


On 2/10/21 9:41 AM, Patrick Palka wrote:

On Tue, 9 Feb 2021, Jason Merrill wrote:


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

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;
-}
-
  

Re: [PATCH 6/6] c++: Consolidate REQUIRES_EXPR evaluation/diagnostic routines

[Jason Merrill via Gcc-patches]

On 2/28/21 12:59 PM, Patrick Palka wrote:

This folds the diagnose_requires_expr routines into the corresponding
tsubst_requires_expr ones.  This is achieved by making the latter
routines take a sat_info instead of a subst_info, and assigning the
appropriate meanings to the flags sat_info::noisy and
sat_info::diagnose_unsatisfaction_p during tsubst_requires_expr:
info.noisy() controls whether to diagnose invalid types and expressions
inside the requires-expression, and info.diagnose_unsatisfaction_p()
controls whether to diagnose why the requires-expression evaluates to
false.

gcc/cp/ChangeLog:

* constraint.cc (struct sat_info): Document the different
meanings of noisy() and diagnose_unsatisfaction_p() during
satisfaction and requires-expression evaluation.
(tsubst_valid_expression_requirement): Take a sat_info instead
of a subst_info.  Perform the substitution quietly first.  Fold
in error-replaying code from diagnose_valid_expression.
(tsubst_simple_requirement): Take a sat_info instead of a
subst_info.
(tsubst_type_requirement_1): New.  Fold in error-replaying code
from diagnose_valid_type.
(tsubst_type_requirement): Use the above.  Take a sat_info
instead of a subst_info.
(tsubst_compound_requirement): Likewise.  Fold in
error-replaying code from diagnose_compound_requirement.
(tsubst_nested_requirement): Take a sat_info instead of a
subst_info.  Fold in error-replaying code from
diagnose_nested_requirement.
(tsubst_requirement): Take a sat_info instead of a subst_info.
(tsubst_requires_expr): Split into two versions, one that takes
a sat_info argument and another that takes a complain and
in_decl argument.  Remove outdated documentation.  Document the
effects of the sat_info argument.
(diagnose_trait_expr): Make static.  Take a template argument
vector instead of a parameter mapping.
(diagnose_valid_expression): Remove.
(diagnose_valid_type): Remove.
(diagnose_simple_requirement): Remove.
(diagnose_compound_requirement): Remove.
(diagnose_type_requirement): Remove.
(diagnose_nested_requirement): Remove.
(diagnose_requirement): Remove.
(diagnose_requires_expr): Remove.
(diagnose_atomic_constraint): Take a sat_info instead of a
subst_info.  Adjust call to diagnose_trait_expr.  Call
tsubst_requires_expr instead of diagnose_requires_expr.
(diagnose_constraints): Call tsubst_requires_expr instead of
diagnose_requires_expr.

gcc/testsuite/ChangeLog:

* g++.dg/concepts/diagnostic1.C: Adjust expected diagnostics
now that we diagnose only the first failed requirement of a
requires-expression.
---
  gcc/cp/constraint.cc| 416 +---
  gcc/testsuite/g++.dg/concepts/diagnostic1.C |   2 +-
  2 files changed, 179 insertions(+), 239 deletions(-)

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index cf319b34da0..31f32c25dfe 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -100,17 +100,30 @@ struct subst_info
  
  /* Provides additional context for satisfaction.
  
-   The flag noisy() controls whether to diagnose ill-formed satisfaction,

-   such as the satisfaction value of an atom being non-bool or non-constant.
-
-   The flag diagnose_unsatisfaction_p() controls whether to explain why
-   a constraint is not satisfied.
-
-   The entrypoints to satisfaction for which we set noisy+unsat are
-   diagnose_constraints and diagnose_nested_requirement.  The entrypoint for
-   which we set noisy-unsat is the replay inside constraint_satisfaction_value.
-   From constraints_satisfied_p, we enter satisfaction quietly (both flags
-   cleared).  */
+   During satisfaction:
+- The flag noisy() controls whether to diagnose ill-formed satisfaction,
+  such as the satisfaction value of an atom being non-bool or non-constant.
+- The flag diagnose_unsatisfaction_p() controls whether to explain why
+  a constraint is not satisfied.
+- We enter satisfaction with noisy+unsat from diagnose_constraints.
+- We enter satisfaction with noisy-unsat from the replay inside
+  constraint_satisfaction_value.
+- We enter satisfaction quietly (both flags cleared) from
+  constraints_satisfied_p.
+
+   During evaluation of a requires-expression:
+- The flag noisy() controls whether to diagnose ill-formed types and
+  expressions inside its requirements.
+- The flag diagnose_unsatisfaction_p() controls whether to explain why
+  the requires-expression evaluates to false.
+- We enter tsubst_requires_expr with noisy+unsat from diagnose_constraints
+  and from diagnose_atomic_constraint.
+- We enter tsubst_requires_expr with noisy-unsat from
+  cp_parser_requires_expression when processing a 

PING 5 [PATCH] correct fix to avoid false positives for vectorized stores (PR 96963, 94655)

[Martin Sebor via Gcc-patches]

Ping 5:
https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564059.html

On 2/22/21 5:20 PM, Martin Sebor wrote:

Ping 4:
https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564059.html

On 2/14/21 5:43 PM, Martin Sebor wrote:

Ping 3:
https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564059.html

This is a fix for two P2 bugs (false positives).

On 2/6/21 10:13 AM, Martin Sebor wrote:

Ping 2:
   https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564059.html

On 1/29/21 10:20 AM, Martin Sebor wrote:
Ping: 
https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564059.html


On 1/21/21 4:38 PM, Martin Sebor wrote:

The hack I put in compute_objsize() last January for pr93200 isn't
quite correct.  It happened to suppress the false positive there
but, due to what looks like a thinko on my part, not in some other
test cases involving vectorized stores.

The attached change adjusts the hack to have compute_objsize() give
up on all MEM_REFs with a vector type.  This effectively disables
-Wstringop-{overflow,overread} for vector accesses (either written
by the user or synthesized by GCC from ordinary accesses).  It
doesn't affect -Warray-bounds because this warning doesn't use
compute_objsize() yet.  When it does (it should considerably
simplify the code) some additional changes will be needed to
preserve -Warray-bounds for out of bounds vector accesses.
The test this patch adds should serve as a reminder to make
it if we forget.

Tested on x86_64-linux.  Since PR 94655 was reported against GCC
10 I'd like to apply this fix to both the trunk and the 10 branch.

Martin

Re: [PATCH] c++: ICE with deduction guide in checking type-dep [PR99009, PR97034]

[Marek Polacek via Gcc-patches]

On Mon, Mar 01, 2021 at 03:08:58PM -0500, Jason Merrill wrote:
> On 3/1/21 2:54 PM, Marek Polacek wrote:
> > On Thu, Feb 25, 2021 at 10:45:29PM -0500, Jason Merrill via Gcc-patches 
> > wrote:
> > > On 2/25/21 5:41 PM, Marek Polacek wrote:
> > > > On Thu, Feb 25, 2021 at 10:59:49AM -0500, Jason Merrill wrote:
> > > > > On 2/12/21 6:12 PM, Marek Polacek wrote:
> > > > > > We represent deduction guides with FUNCTION_DECLs, but they are 
> > > > > > built
> > > > > > without DECL_CONTEXT
> > > > > 
> > > > > Hmm, that seems wrong: "A deduction-guide shall be declared in the
> > > > > same scope as the corresponding class template and, for a member class
> > > > > template, with the same access."  But it probably isn't necessary to 
> > > > > change
> > > > > this:
> > > > > 
> > > > > > leading to an ICE in type_dependent_expression_p
> > > > > > on the assert that the type of a function template with no dependent
> > > > > > (innermost!) template arguments must be non-dependent.  Consider the
> > > > > > attached class-deduction79.C: we create a deduction guide:
> > > > > > 
> > > > > >  template G(T)-> E::G
> > > > > > 
> > > > > > we deduce T and create a partial instantiation:
> > > > > > 
> > > > > >  G(T) -> E::G [with T = int]
> > > > > > 
> > > > > > And then do_class_deduction wants to create a CALL_EXPR from the 
> > > > > > above
> > > > > > using build_new_function_call -> build_over_call which calls 
> > > > > > mark_used
> > > > > > -> maybe_instantiate_noexcept -> type_dependent_expression_p.
> > > > > > 
> > > > > > There, the innermost template arguments are non-dependent (), 
> > > > > > but
> > > > > > the fntype is dependent -- the return type is a TYPENAME_TYPE, and
> > > > > > since we have no DECL_CONTEXT, this check holds:
> > > > > > 
> > > > > >  /* Otherwise, if the function decl isn't from a dependent 
> > > > > > scope, it can't be
> > > > > > type-dependent.  Checking this is important for functions 
> > > > > > with auto return
> > > > > > type, which looks like a dependent type.  */
> > > > > >  if (TREE_CODE (expression) == FUNCTION_DECL
> > > > > >  && !(DECL_CLASS_SCOPE_P (expression)
> > > > > >   && dependent_type_p (DECL_CONTEXT (expression)))
> > > > > > 
> > > > > > whereupon we ICE.
> > > > > > 
> > > > > > Experiments with setting DECL_CONTEXT didn't pan out.
> > > > > 
> > > > > In c8 of the PR it looks like you were using the class itself as
> > > > > DECL_CONTEXT; the quote above says that the right context is the 
> > > > > enclosing
> > > > > scope of the class.
> > > > 
> > > > Sadly, using CP_TYPE_CONTEXT (type) would result in a crash in
> > > > retrieve_specialization:
> > > > 
> > > > /* There should be as many levels of arguments as there are
> > > >levels of parameters.  */
> > > > gcc_assert (TMPL_ARGS_DEPTH (args)
> > > > == (TREE_CODE (tmpl) == TEMPLATE_DECL
> > > > ? TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (tmpl))
> > > > : template_class_depth (DECL_CONTEXT (tmpl;
> > > 
> > > Yeah, probably simpler not to bother.
> > > 
> > > > > > So perhaps we
> > > > > > just want to skip the assert for deduction guides, because they are
> > > > > > a little special.  Better ideas solicited.
> > > > > 
> > > > > In c3 you mention that one of the variants broke with r269093; this is
> > > > > because my change to check CLASSTYPE_TEMPLATE_INSTANTIATION is false 
> > > > > for the
> > > > > template pattern itself (E).
> > > > 
> > > > And the original test started with my r11-1713 because using TREE_TYPE
> > > > directly instead of decltype (which is a non-deduced context) means we
> > > > can deduced from the argument.
> > > > > But I think probably the right answer is to defer this deduction 
> > > > > until the
> > > > > enclosing scope is non-dependent, i.e. (untested)
> > > > 
> > > > Thanks.  That mostly works, except the new class-deduction-aggr[89].C
> > > > tests.  Consider 8:
> > > > 
> > > > namespace N {
> > > > template  struct S {
> > > > template  S(T, U);
> > > > };
> > > > } // namespace N
> > > > template  struct E {
> > > > template  struct G { T t; };
> > > > void fn() { G{N::S{'a', 1}}; }
> > > > };
> > > > 
> > > > void
> > > > g ()
> > > > {
> > > > E<1> e;
> > > > e.fn ();
> > > > }
> > > > 
> > > > With your patch, when in do_class_deduction when 
> > > > processing_template_decl,
> > > > we just return.  When we call do_class_deduction again when p_t_d is 0,
> > > > maybe_aggr_guide returns early here:
> > > > 
> > > > if (!CP_AGGREGATE_TYPE_P (type))
> > > >   return NULL_TREE
> > > > 
> > > > because G is not complete (and rightly so, we didn't instantiate it).  
> > > > So
> > > > we aren't able to deduce the template parameters.  I'm not sure if I 
> > > > should
> > > > pursue this direction further.  :(
> > > 
> > > I think so; we just need to test CP_AGGREGATE_TYPE_P on the 

Re: [PATCH] avoid -Wnonull for dynamic_cast (PR 99251)

[Martin Sebor via Gcc-patches]

On 3/1/21 1:33 PM, Jason Merrill wrote:

On 3/1/21 12:10 PM, Martin Sebor wrote:

On 2/24/21 8:13 PM, Jason Merrill wrote:

On 2/24/21 5:25 PM, Martin Sebor wrote:

In r11-6900 (PR 98646 - static_cast confuses -Wnonnull) we decided
that issuing -Wnonnull for dereferencing the result of dynamic_cast
was helpful despite the false positives it causes when the pointer
is guaranteed not to be null because of a prior test.

The test case in PR 99251 along with the feedback I got from Martin
Liska have convinced me it was not the right decision.

The attached patch arranges for dynamic_cast to also suppress -Wnonnull
analogously to static_cast.  Since there already is a helper function
that builds the if-not-null test (ifnonnull) and sets TREE_NO_WARNING,
I factored out the corresponding code from build_base_path that sets
the additional TREE_NO_WARNING bit for static_cast into the function
and called it from both places.  I also renamed the function to make
its purpose clearer and for consistency with other build_xxx APIs.


Let's call it build_if_nonnull, as it builds the COND_EXPR as well as 
the test.


Done.




+  /* The dynamic_cast might fail but so a warning might be justified
+ but not when the operand is guarded.  See pr99251.  */
+  if (B *q = p->bptr ())
+    dynamic_cast(q)->g ();    // { dg-bogus 
"\\\[-Wnonnull" }


This guard is no more necessary than it is for static_cast; both 
cases deal with null arguments.  Let's not add these checks to the 
testcases.


Done.



This guard doesn't check for the mentioned case of dynamic_cast 
failing because the B* does not in fact point to a C.


I think we can just change the dg-warning to dg-bogus.  Sure, 
dynamic_cast might fail, but AFAICT -Wnonnull isn't supposed to warn 
about arguments that *might* be null, only arguments that are *known* 
to be null.


Done.

I had added the 'if' to the test to make it clear why the warning
isn't expected.  I replaced it with a comment to that effect.



FWIW, I do think a warning for dynamic_cast to a pointer would be
helpful in the absence of an if statement in these cases:

   void f (B *p)
   {
 dynamic_cast(p)->g ();
   }

Not because p may be null but because the result of the cast may be
for a nonnull p.  If it's f's precondition that D is derived from B
(in addition to p being nonnull) the cast would be better written as
one to a reference to D.


Agreed, or if it isn't a precondition,

if (D* dp = dynamic_cast(p))
   dp->g();


Such a warning would need to be issued by the middle end and although
it could be controlled by a new option (say -Wdynamic-cast, along with
the cases discussed in PR 12277)


Sounds good.


I don't think issuing -Wnonnull in this case would be inappropriate.


I disagree; issuing -Wnonnull for this case would be wrong.  Again, 
-Wnonnull is supposed to warn when an argument *is* null, not when it 
*might* be null.


I think warning about this case is a good idea, just not as part of 
-Wnonnull.



Anyway, that's something to consider for GCC 12.  For now, please see
the revised patch.



* rtti.c (ifnonnull): Rename...
(build_nonnull_test): ...to this.  Set no-warning bit on COND_EXPR.


The ChangeLog needs updating.


+  /* Unlike static_cast, dynamic cast may fail for a nonnull operand but


Yes, but...


+ since the front end can't see if the cast is guarded against being
+ invoked with a null


No; my point was that whether the cast is guarded against being invoked 
with a null is no more relevant for dynamic_cast than it is for 
static_cast, as both casts give a null result for a null argument.


For this test, dynamic_cast is not significantly different from 
static_cast.  For both casts, the bug was the compiler warning about a 
nullptr that it introduced itself.


It seems like splitting hairs.  The comment (much as the original
if guard) is just meant to explain why -Wnonnull isn't expected in
case a new warning is added that detects the bad assumption above.
I want to make it clear that if/when that happens a failure here
doesn't mislead the author into thinking we don't want any warning
there at all.

I have reworded the comments yet again.




verify there's no warning.  See also pr99251.  */


Yes.

-  dynamic_cast(p->bptr ())->g ();   // { dg-warning 
"\\\[-Wnonnull" }
+  dynamic_cast(p)->g ();    // { dg-bogus 
"\\\[-Wnonnull" }


Please put back the ->bptr()s.


Done.

Martin



Jason



PR c++/99251 - inconsistent -Wnonnull warning behaviour with dynamic_cast

gcc/cp/ChangeLog:

	PR c++/99251
	* class.c (build_base_path): Call build_if_nonnull.
	* cp-tree.h (build_if_nonnull): Declare.
	* rtti.c (ifnonnull): Rename...
	(build_if_nonnull): ...to this.  Set no-warning bit on COND_EXPR.
	(build_dynamic_cast_1): Adjust to name change.

gcc/testsuite/ChangeLog:

	PR c++/99251
	* g++.dg/warn/Wnonnull9.C: Expect no warnings.
	* g++.dg/warn/Wnonnull12.C: New test.

diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 

Re: [PATCH 2/3 V2] Do not include stdio.h in libgcc's Decimal/Float128 conversions.

[Segher Boessenkool]

On Mon, Mar 01, 2021 at 12:18:52PM -0500, Michael Meissner wrote:
> The _sprintfkf.c file was including stdio.h to get the definition of sprintf.

(declaration of)

> This patch modifies this so that stdio.h is not included in order to support
> freestanding cross compilers that might not provide stdio.h.

So the code cannot work at all there?  Will not link?

> +   We use __builtin_sprintf so that we don't have to include stdio.h to 
> define
> +   sprintf.  Stdio.h might not be present for freestanding cross compilers 
> that
> +   do not need to include a library.  */

"declare sprintf", and the function is called stdio.g (all lowercase).
It is often written , which makes it easier to handle in
sentences.

> @@ -54,5 +57,5 @@ int __sprintfkf (char *restrict string,
>if (__sprintfieee128)
>  return __sprintfieee128 (string, format, number);
>  
> -  return sprintf (string, format, (long double) number);
> +  return __builtin_sprintf (string, format, (long double) number);

sprintf as well as __builtin_sprintf do not exist exactly when 
does not (or are not guaranteed to exist, anyway).


Segher

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

[Martin Sebor via Gcc-patches]

On 2/25/21 4:40 PM, Jeff Law wrote:



On 2/8/21 3:44 PM, Martin Sebor wrote:


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.

OK.  I guess it's a minor question of semantics between pr97595 and
pr98266  being the same.  But I think we can put that behind us now.

I think the big question is whether or not DECL_ARTIFICIAL is the right
way to detect these cases, or if there simply isn't a way as one message
from Jason seems to imply.


DECL_ARTIFICIAL happens to work for the vtbl case but it doesn't work
for other accesses to members of virtual bases  that don't involve
artificial members but where the offset is constant.  I didn't think
those came up but as Jason showed, they do: in accesses in destructors
(and also in some in constructors).  So the DECL_ARTIFICIAL test isn't
the right way to detect all those cases.

I have posted an updated patch that handles those cases as well as
direct vtbl accesses by detecting the COMPONENT_REF (MEM_REF (), m))
access to member m of object d with virtual bases, that doesn't depend
on DECL_ARTIFICIAL.

Martin

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

[Martin Sebor via Gcc-patches]

On 2/24/21 5:35 PM, Jason Merrill wrote:

On 2/23/21 6:07 PM, Martin Sebor wrote:

On 2/23/21 2:52 PM, Jason Merrill wrote:

On 2/23/21 11:02 AM, Martin Sebor wrote:

[CC Jason for any further comments/clarification]

On 2/9/21 10:49 AM, Martin Sebor wrote:

On 2/8/21 4:11 PM, Jeff Law wrote:



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 *) + 24B]._vptr.D =   [(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.


The part of the IL with the MEM_REF is this:

void g ()
{
   void * D.2789;
   struct E D.2652;

    [local count: 1073741824]:
   E::E (, "");
   f ();

    [local count: 1073741824]:
   MEM[(struct D *) + 24B]._vptr.D =   
[(void *)&_ZTC1E24_1D + 24B];

   ...

The access here is to the _vptr.D pointer member of D.2652 which is
just past the end of the parent object (as reflected by its SIZE):
it sets sets up the virtual table pointer.

The access in pr97595 is to the member subobject, which, as Jason
explained (and I accordingly documented under DECL_SIZE in tree.h),
is also laid out separately from the parent object.

These cases aren't exactly the same (which is also why the test
I added for -Warray-bounds in pr97595 didn't expose this bug) but
they are closely related.  The one here can be distinguished by
DECL_ARTIFICAL.  The other by the DECL_SIZE != TYPE_SIZE member
inequality.

Might this impact other warnings?  I'd say so if they don't take
these things into account.  I just learned about this in pr97595
which was a -Wstringop-overflow false positive but I also saw
a similar instance of -Warray-bounds with my patch to improve
caching and enhance array bounds checking.  I dealt with that
instance of the warning in that patch but proactively added
a test case to the fix for pr97595.  But the test case is focused
on the subobject access and not on one to the virtual table so
(as I said above) it didn't expose this bug.

Might this also impact optimizations?  I can imagine someone
unaware of this "gotcha" making the same "naive" assumption
I did, but I'd also expect such an invalid assumption to be
found either in code review or quickly cause problems in
testing.


Jeff, does this answer your question?


I don't see how the issue here depends on the artificiality of the 
vptr; I'd expect to see the same problem with a data member.  The 
problem is that a D base subobject is smaller than a complete D 
object, and in this case the base subobject is allocated such that if 
it were a full D object, it would overlap the end of E.  And we're 
checking the 

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

[Patrick Palka via Gcc-patches]

On Mon, 1 Mar 2021, Jason Merrill wrote:

> On 2/28/21 12:40 PM, Patrick Palka wrote:
> > On Fri, 12 Feb 2021, Jason Merrill wrote:
> > 
> > > On 2/10/21 9:41 AM, Patrick Palka wrote:
> > > > On Tue, 9 Feb 2021, Jason Merrill wrote:
> > > > 
> > > > > On 2/8/21 2:03 PM, Patrick Palka wrote:
> > > > > > 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 @@ 

Re: [PATCH 5/6] c++: Clean up normalization / satisfaction routines

[Jason Merrill via Gcc-patches]

On 2/28/21 12:58 PM, Patrick Palka wrote:

This patch mostly performs some straightforward refactoring:

   - Renamed satisfy_constraint to satisfy_normalized_constraints
   - Renamed the three-parameter version of satisfy_constraint_expression
 to satisfy_nondeclaration_constraints
   - Removed normalize_(non)?template_requirements
   - Removed satisfy_associated_constraints (and made its callers
 check for dependent template args sooner, before normalization)
   - Removed the tsubst_flags_t parameter of evaluate_concept_check
   - Combined the two versions of constraint_satisfaction_value
   - Combined the two versions of constraint_satisfied_p

Additionally, this patch removes the handling of bare
constraint-expressions from satisfy_nondeclaration_constraints, and
hence constraints_satisfied_p and constraint_satisfaction_value now only
take things that carry their own template information needed for
normalization.  In practice, this only means it's no longer possible to
evaluate bare REQUIRES_EXPRs via the satisfaction routines, and so this
patch adjusts the affected callers to instead use tsubst_requires_expr.


It's probably better to have a different entry point than 
tsubst_requires_expr for callers that have nothing to do with templates. 
 Whether that's a one-line wrapper for the call to tsubst_requires_expr 
("evaluate_requires_expr"?) or calling tsubst_requires_expr from 
satisfy_nondeclaration_constraints, I don't have an opinion either way.



For convenience, the function diagnose_constraints continues to accept
REQUIRES_EXPRs, but it now handles them by calling diagnose_require_expr
directly.


This might argue for the latter choice above.


(That we used to evaluate REQUIRES_EXPR via satisfaction might even be a
correctness issue: since we cache satisfaction in special ways that don't
apply to regular evaluation, going through satisfaction could in theory
cause us to reuse a cached value for a REQUIRES_EXPR when we shouldn't
have.)



gcc/cp/ChangeLog:

* constexpr.c (cxx_eval_call_expression): Adjust call to
evaluate_concept_check.
(cxx_eval_constant_expression) : Use
tsubst_requires_expr instead of satisfy_constraint_expression.
: Adjust call to evaluate_concept_check.
* constraint.cc (struct sat_info): Adjust comment about which
satisfaction entrypoints use noisy-unsat.
(normalize_template_requirements): Remove (and adjust callers
appropriately).
(normalize_nontemplate_requirements): Likewise.
(tsubst_nested_requirement): Use constraint_satisfaction_value
instead of satisfy_constraint_expression, which'll do the
noisy replaying of ill-formed quiet satisfaction for us.
(decl_satisfied_cache): Adjust comment.
(satisfy_constraint): Rename to ...
(satisfy_normalized_constraints): ... this.
(satisfy_associated_constraints): Remove (and make its
callers check for dependent arguments).
(satisfy_constraint_expression): Rename to ...
(satisfy_nondeclaration_constraints): ... this.  Assert that
'args' is empty when 't' is a concept-id.  Removing handling
bare constraint-expressions.  Adjust comment accordingly.
(satisfy_declaration_constraints): Assert in the two-parameter
version that 't' is not a TEMPLATE_DECL.  Adjust following
removal of normalize_(non)?template_requirements and
satisfy_asociated_constraints.
(constraint_satisfaction_value): Combine the two- and
three-parameter versions in the natural way.
(constraints_satisfied_p): Combine the one- and two-parameter
versions in the natural way.  Improve documentation.
(evaluate_concept_check): Remove 'complain' parameter.  Use
constraint_satisfaction_value instead of
satisfy_constraint_expression.
(diagnose_nested_requirement): Adjust following renaming of
satisfy_constraint_expression.
(diagnose_constraints): Handle REQUIRES_EXPR by going through
diagnose_requires_expr directly instead of treating it as a
constraint-expression.  Improve documentation.
* cp-gimplify.c (cp_genericize_r) : Adjust call
to evaluate_concept_check.
: Use tsubst_requires_expr instead of
constraints_satisfied_p.
: Adjust call to evaluate_concept_check.
* cp-tree.h (evaluate_concept_check): Remove tsubst_flag_t
parameter.
(satisfy_constraint_expression): Remove declaration.
(constraints_satisfied_p): Remove one-parameter declaration.
Add a default argument to the two-parameter declaration.
* cvt.c (convert_to_void): Adjust call to
evaluate_concept_check.
---
  gcc/cp/constexpr.c   |   6 +-
  gcc/cp/constraint.cc | 210 ---
  gcc/cp/cp-gimplify.c |   7 +-
  gcc/cp/cp-tree.h |   6 +-
  gcc/cp/cvt.c |   2 +-
  5 files 

Re: [PATCH 4/5] barrier: use int instead of unsigned char for the phase state

[Thiago Macieira via Gcc-patches]

On Monday, 1 March 2021 14:31:09 PST Ville Voutilainen wrote:
> On Tue, 2 Mar 2021 at 00:21, Thiago Macieira  
wrote:
> > But the code I posted, if people are careful to use write like I did,
> > would
> > allow us to have the experimental "we're not sure this is right"
> > implementation of atomic waits, latches, barriers and semaphores right
> > now.
> 
> The code assumes that as soon as __cplusplus bumps and a header is
> present, things
> are stable.

Not exactly. Re-posting the code for reference:

#if __cplusplus >= 202002L && __has_include()
#  include 
#endif
#if __cpp_lib_latch < 201907L
#  error "Please upgrade your Standard Library"
#endif

The first section simply assumes that  can be #included. The 
__cplusplus check is necessary because MSVC STL's headers are present but 
can't be #include'd otherwise (they cause #error).

It's the second check that is authoritative.

> I don't think that's a safe assumption to make.
> Furthermore, the second  #if
> tries to check a feature-testing macro without including either the
> corresponding header
> or . That doesn't work. You need to either include 
> and check a macro,
> or check that  exists, then include  and then check the macro.

 would work, but why can't you check the macro anyway? It may trigger 
a warning with GCC's -Wundef, but it's just a warning. The preprocessor is 
defined to replace any undefined identifier with 0. If you want to avoid the 
warning, you could write:

#if defined(__cpp_lib_latch) && __cpp_lib_latch < 201907L

Is there anything I'm not seeing?

> But other than that, sure, as QoI, vendors could already provide the
> standard macros with
> numbers that are lower than the standard ever specified. Going
> forward, if existing facilities
> are changed, this stops working because now you'd have to track the
> WPs to see which
> values are "vendor-bogus". I find it better to just change the macros
> whose facilities changed
> during a standard cycle, and in those cases bump the IS macro, that'll
> then work going forward.
> In the meanwhile, when vendors can, they could use the technique you
> describe, but that's
> barely worth proposing as an SG10 guideline because they would've
> needed to do it already, but didn't. :P

I am not opposed to that. Your solution is better.

But this solution is less work on the standard and still works, even if it 
creates a little more work on the users of said features. It's not 
unsurmountable, since we often need to check which C++ standard edition it 
came with anyway. So it doesn't matter what value it assumes: we'll be 
consulting a table anyway.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel DPG Cloud Engineering

Re: [PATCH 1/3 V2] Fix __sprintfkf prototype in libgcc.

[Segher Boessenkool]

On Mon, Mar 01, 2021 at 12:17:34PM -0500, Michael Meissner wrote:
> The prototype of __sprintfkf in _sprintfkf.h did not match the function in
> _sprintfkf.c.  This patch fixes the prototype.  I also included the
> _sprintfkf.h file in _sprintfkf.c to make sure the prototype is correct and to
> eliminate a warning about declaring the function without a previous
> declaration.

> libgcc/
> 2021-03-01  Michael Meissner  
> 
>   * config/rs6000/_sprintfkf.h (__sprintfkf): Fix prototype to match
>   the function.
>   * config/rs6000/_sprintfkf.c: Include _sprintfkf.h.

This is okay for trunk.  Thank you!


Segher

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

[Jason Merrill via Gcc-patches]

On 2/28/21 12:55 PM, Patrick Palka wrote:

On Fri, 12 Feb 2021, Jason Merrill wrote:


On 2/11/21 5:14 PM, Patrick Palka wrote:

On Thu, 11 Feb 2021, Jason Merrill wrote:


On 2/8/21 2:03 PM, 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.

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 

Re: [PATCH 4/5] barrier: use int instead of unsigned char for the phase state

[Ville Voutilainen via Gcc-patches]

On Tue, 2 Mar 2021 at 00:21, Thiago Macieira  wrote:

> But the code I posted, if people are careful to use write like I did, would
> allow us to have the experimental "we're not sure this is right"
> implementation of atomic waits, latches, barriers and semaphores right now.

The code assumes that as soon as __cplusplus bumps and a header is
present, things
are stable. I don't think that's a safe assumption to make.
Furthermore, the second  #if
tries to check a feature-testing macro without including either the
corresponding header
or . That doesn't work. You need to either include 
and check a macro,
or check that  exists, then include  and then check the macro.

But other than that, sure, as QoI, vendors could already provide the
standard macros with
numbers that are lower than the standard ever specified. Going
forward, if existing facilities
are changed, this stops working because now you'd have to track the
WPs to see which
values are "vendor-bogus". I find it better to just change the macros
whose facilities changed
during a standard cycle, and in those cases bump the IS macro, that'll
then work going forward.
In the meanwhile, when vendors can, they could use the technique you
describe, but that's
barely worth proposing as an SG10 guideline because they would've
needed to do it already, but didn't. :P

[PATCH] [og10] openmp: Scale precision of collapsed iteration variable

[Kwok Cheung Yeung]

Hello

When two or more nested loops are collapsed using the OpenMP collapse clause, a 
single iteration variable is used to represent the combined iteration space. In 
the usual case (i.e. statically scheduled, no ordered clause), the type of this 
variable is picked by taking the unsigned version of the largest of the iterator 
types in the loop nest:


 else if (i == 0
  || TYPE_PRECISION (iter_type)
 < TYPE_PRECISION (TREE_TYPE (loop->v)))
   iter_type
 = build_nonstandard_integer_type
 (TYPE_PRECISION (TREE_TYPE (loop->v)), 1);

If needed, the original indices of the collapsed loops are recalculated from the 
combined index. However, this can be problematic if the combined iteration space 
of the collapsed loops is larger than can be represented by the type of the 
largest individual loop iterator type. e.g.


for (int i = 0; i < 8; i++)
  for (int j = 0; j < 8; j++)

In this case, the combined iteration space is [0..6,400,000,000), which is 
larger than the [0..4,294,967,296) range of a 32-bit unsigned int.


This patch attempts to avoid this problem by setting the precision of the 
combined iteration variable to the sum of the precision of the collapsed 
iterators, rounded up to the nearest power of 2. This is capped at the size of a 
long long (i.e. 64 bits) to avoid an excessive performance hit. If any of the 
loops use a larger type (e.g. __int128), then that is used instead.


I believe OpenACC suffers from a similar problem, but it uses a different 
code-path and should be dealt with separately. The patch caused regressions in 
some OpenACC tests related to tiling (pr84955-1.c, pr84955.c, tile-1.c, 
pr84955.f90) due to the type of diff_type changing between when it was used to 
define the '.tile' variables in expand_oacc_collapse_init and when the '.tile' 
variables are used in expand_oacc_for. I fixed this by adding a cast to the 
current diff_type when '.tile' is multiplied.


Okay for OG10?

Thanks

Kwok
From df1332b7a1575920c8de17359b2dfcad5404a112 Mon Sep 17 00:00:00 2001
From: Kwok Cheung Yeung 
Date: Mon, 1 Mar 2021 14:15:30 -0800
Subject: [PATCH] openmp: Scale type precision of collapsed iterator variable

This sets the type precision of the collapsed iterator variable to the
sum of the precision of the collapsed loop variables, up to a maximum of
sizeof(long long) (i.e. 64-bits).

2021-03-01  Kwok Cheung Yeung  

gcc/
* omp-expand.c (expand_oacc_for): Convert .tile variable to
diff_type before multiplying.
* omp-general.c (omp_extract_for_data): Use accumulated precision
of all collapsed for-loops as precision of iteration variable, up
to the precision of a long long.

libgomp/
* testsuite/libgomp.c-c++-common/collapse-4.c: New.
* testsuite/libgomp.fortran/collapse5.f90: New.
---
 gcc/ChangeLog.omp  |  8 ++
 gcc/omp-expand.c   |  5 +++-
 gcc/omp-general.c  | 29 +-
 libgomp/ChangeLog.omp  |  5 
 .../testsuite/libgomp.c-c++-common/collapse-4.c| 19 ++
 libgomp/testsuite/libgomp.fortran/collapse5.f90| 14 +++
 6 files changed, 73 insertions(+), 7 deletions(-)
 create mode 100644 libgomp/testsuite/libgomp.c-c++-common/collapse-4.c
 create mode 100644 libgomp/testsuite/libgomp.fortran/collapse5.f90

diff --git a/gcc/ChangeLog.omp b/gcc/ChangeLog.omp
index a59c25b..374665d 100644
--- a/gcc/ChangeLog.omp
+++ b/gcc/ChangeLog.omp
@@ -1,3 +1,11 @@
+2021-03-01  Kwok Cheung Yeung  
+
+   * omp-expand.c (expand_oacc_for): Convert .tile variable to
+   diff_type before multiplying.
+   * omp-general.c (omp_extract_for_data): Use accumulated precision
+   of all collapsed for-loops as precision of iteration variable, up
+   to the precision of a long long.
+
 2021-02-24  Julian Brown  
 
Backport from mainline
diff --git a/gcc/omp-expand.c b/gcc/omp-expand.c
index e4a2f3a..f8347c0 100644
--- a/gcc/omp-expand.c
+++ b/gcc/omp-expand.c
@@ -7328,7 +7328,10 @@ expand_oacc_for (struct omp_region *region, struct 
omp_for_data *fd)
   tile_size = create_tmp_var (diff_type, ".tile_size");
   expr = build_int_cst (diff_type, 1);
   for (int ix = 0; ix < fd->collapse; ix++)
-   expr = fold_build2 (MULT_EXPR, diff_type, counts[ix].tile, expr);
+   {
+ tree tile = fold_convert (diff_type, counts[ix].tile);
+ expr = fold_build2 (MULT_EXPR, diff_type, tile, expr);
+   }
   expr = force_gimple_operand_gsi (, expr, true,
   NULL_TREE, true, GSI_SAME_STMT);
   ass = gimple_build_assign (tile_size, expr);
diff --git a/gcc/omp-general.c b/gcc/omp-general.c
index 8e5b961..97f94e1 100644
--- a/gcc/omp-general.c
+++ b/gcc/omp-general.c
@@ -356,6 +356,7 @@ omp_extract_for_data 

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

[Jason Merrill via Gcc-patches]

On 2/28/21 12:40 PM, Patrick Palka wrote:

On Fri, 12 Feb 2021, Jason Merrill wrote:


On 2/10/21 9:41 AM, Patrick Palka wrote:

On Tue, 9 Feb 2021, Jason Merrill wrote:


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

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 

Re: [PATCH 4/5] barrier: use int instead of unsigned char for the phase state

[Thiago Macieira via Gcc-patches]

On Monday, 1 March 2021 14:04:34 PST Ville Voutilainen wrote:
> Well, this would be different. What I'm suggesting is not quite that;
> for any *new* facility, we'd make sure
> that its draft macro and the final IS macro are different, but the
> minimum value is the first draft version,
> not anything below it. I don't care too much, that approach and yours
> would work the same way. Things that already
> had an IS value for a macro and haven't changed since don't need to be
> changed. And we don't
> need to bump all values of existing facilities either, just for those
> that got changes, so some existing macros
> would be considered lost causes. Like the ones we're talking about,
> because the cats are already out of the
> bag.

But the code I posted, if people are careful to use write like I did, would 
allow us to have the experimental "we're not sure this is right" 
implementation of atomic waits, latches, barriers and semaphores right now.

It would simply require that we decrement the macros by 1 in the libstdc++ 
headers.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel DPG Cloud Engineering

*PING* [PATCH] PR libfortran/99218 - [8/9/10/11 Regression] matmul on temporary array accesses invalid memory

[Harald Anlauf via Gcc-patches]

Early ping.

Harald


> Gesendet: Dienstag, 23. Februar 2021 um 22:46 Uhr
> Von: "Harald Anlauf" 
> An: "fortran" , "gcc-patches" 
> Betreff: [PATCH] PR libfortran/99218 - [8/9/10/11 Regression] matmul on 
> temporary array accesses invalid memory
>
> Dear all,
>
> under certain circumstances a call to MATMUL for rank-2 times rank-1
> would invoke a highly tuned rank-2 times rank-2 algorithm which could
> lead to invalid reads and writes.  The solution is to check the rank
> of the second argument to matmul and fall back to a regular algorithm
> for rank-1.  The invalid accesses did show up with valgrind.
>
> I have not been able to create a testcase that gives wrong results.
>
> Regtested on x86_64-pc-linux-gnu, and verified with valgrind.
>
> OK for master?
>
> As this affects all open branches down to 8, ok for backports?
>
> Thanks,
> Harald
>
>
> PR libfortran/99218 - matmul on temporary array accesses invalid memory
>
> Do not invoke tuned rank-2 times rank-2 matmul if rank(b) == 1.
>
> libgfortran/ChangeLog:
>
>   PR libfortran/99218
>   * m4/matmul_internal.m4: Invoke tuned matmul only for rank(b)>1.
>   * generated/matmul_c10.c: Regenerated.
> * generated/matmul_c16.c: Likewise.
> * generated/matmul_c4.c: Likewise.
> * generated/matmul_c8.c: Likewise.
> * generated/matmul_i1.c: Likewise.
> * generated/matmul_i16.c: Likewise.
> * generated/matmul_i2.c: Likewise.
> * generated/matmul_i4.c: Likewise.
> * generated/matmul_i8.c: Likewise.
> * generated/matmul_r10.c: Likewise.
> * generated/matmul_r16.c: Likewise.
> * generated/matmul_r4.c: Likewise.
> * generated/matmul_r8.c: Likewise.
> * generated/matmulavx128_c10.c: Likewise.
> * generated/matmulavx128_c16.c: Likewise.
> * generated/matmulavx128_c4.c: Likewise.
> * generated/matmulavx128_c8.c: Likewise.
> * generated/matmulavx128_i1.c: Likewise.
> * generated/matmulavx128_i16.c: Likewise.
> * generated/matmulavx128_i2.c: Likewise.
> * generated/matmulavx128_i4.c: Likewise.
> * generated/matmulavx128_i8.c: Likewise.
> * generated/matmulavx128_r10.c: Likewise.
> * generated/matmulavx128_r16.c: Likewise.
> * generated/matmulavx128_r4.c: Likewise.
> * generated/matmulavx128_r8.c: Likewise.
>
>

[Patch, part1] PR fortran/49278 - ICE when combining DATA with default initialization

[Harald Anlauf via Gcc-patches]

Dear all,

the present PR has two issues.  The first one, addressed by this patch,
was about accepting invalid code where a variable appeared both in a
declaration with PARAMETER as well as in a DATA statement, which could
lead to an ICE.  We now reject this.

(There is a separate issue about combining default initialization with
DATA leading to a wrong constructor and thus wrong code.  I'd prefer to
have this addressed separately.)

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

Thanks,
Harald


PR fortran/49278 - ICE when combining DATA with default initialization

A variable with the PARAMETER attribute may not appear in a DATA statement.

gcc/fortran/ChangeLog:

PR fortran/49278
* data.c (gfc_assign_data_value): Reject variable with PARAMETER
attribute in DATA statement.

gcc/testsuite/ChangeLog:

PR fortran/49278
* gfortran.dg/parameter_data.f90: New test.

diff --git a/gcc/fortran/data.c b/gcc/fortran/data.c
index 13e3506dd1e..25e97930169 100644
--- a/gcc/fortran/data.c
+++ b/gcc/fortran/data.c
@@ -244,6 +244,13 @@ gfc_assign_data_value (gfc_expr *lvalue, gfc_expr *rvalue, mpz_t index,
 		"array-element nor a scalar-structure-component";

   symbol = lvalue->symtree->n.sym;
+  if (symbol->attr.flavor == FL_PARAMETER)
+{
+  gfc_error ("PARAMETER %qs shall not appear in a DATA statement at %L",
+		 symbol->name, >where);
+  return false;
+}
+
   init = symbol->value;
   last_ts = >ts;
   last_con = NULL;
diff --git a/gcc/testsuite/gfortran.dg/parameter_data.f90 b/gcc/testsuite/gfortran.dg/parameter_data.f90
new file mode 100644
index 000..b95f9c90696
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/parameter_data.f90
@@ -0,0 +1,13 @@
+! { dg-do compile }
+! PR fortran/49278 - ICE when combining DATA with default initialization
+
+program p
+  implicit none
+  type t
+ real :: a
+  end type t
+  integer, parameter :: b = 42
+  type(t), parameter :: z = t(4.0)
+  data b   / 666 / ! { dg-error "shall not appear in a DATA statement" }
+  data z%a / 3.0 / ! { dg-error "shall not appear in a DATA statement" }
+end

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

[Anthony Sharp via Gcc-patches]

Hi all,

Here is the patch as promised. Regression tested on the c++ side and
everything seems okay. Compiles fine.

Sounds good, though strip_using_decl (parent_field) may be overloaded if
> the using-decl brings in multiple functions with that name.


Could you give my new regression test a quick glance over then just to make
sure I've not forgotten about something? It definitely seems to work but
I'm no expert in all the different ways using statements can be
constructed. If you were to use 'using comma' (in the testcase), it
generates another error because it's ambiguous, so that's okay. And if you
use a comma-separated list like I have in the test (i.e. using A2::comma,
A1::comma) it seems to find the correct bits just fine. Unless I'm getting
really lucky and it's actually just a coincidence.

It seems that strip_using_decl doesn't return an overloaded list. Or, if it
does, the first entry in that list just so happens to always be the right
answer, so it can be treated like it's a regular decl for this purpose. For
example, even if we change up the order of baseclasses, using statements
and class definitions, it still seems to work, e.g. the following *seems*
to work just fine:

class A2
{
  protected:
  int separate(int a);
  int comma(int a);
  int alone;
};

class A1
{
  protected:
  int separate();
  int comma();
};

class A3
{
  protected:
  int comma(int a, int b);
};

class B:private A3, private A1, private A2
{
  // Using decls in a comma-separated list.
  using A2::comma, A3::comma, A1::comma;  // { dg-message "declared" }
  // Separate using statements.
  using A2::separate;
  using A1::separate; // { dg-message "declared" }
  // No ambiguity, just for the sake of it.
  using A2::alone; // { dg-message "declared" }
};

class C:public B
{
  void f()
  {
comma(); // { dg-error "private" }
separate(); // { dg-error "private" }
alone = 5; // { dg-error "private" }
  }
};

Anthony


On Thu, 25 Feb 2021 at 03:37, Jason Merrill  wrote:

> On 2/24/21 4:17 PM, Anthony Sharp wrote:
> >> "special"
> >
> >
> > It wouldn't be my code if it didn't have sp3ling mstakes innit!
> > Actually to be fair I already changed that spelling mistake a few days
> > ago in my local code ;)
> >
> > I was actually thinking about this last night as I was falling asleep
> > (as you do) and I realised that the whole of my using decl lookup is
> > redundant. I can simply do this (formatting probably messes up here):
> >
> > /* 1.  If the "using" keyword is used to inherit DECL within the parent,
> >   this may cause DECL to be private, so we should return the using
> >   statement as the source of the problem.
> >
> >   Scan the fields of PARENT_BINFO and see if there are any using
> decls.  If
> >   there are, see if they inherit DECL.  If they do, that's where
> DECL must
> >   have been declared private.  */
> >
> >for (tree parent_field = TYPE_FIELDS (BINFO_TYPE (parent_binfo));
> > parent_field;
> > parent_field = DECL_CHAIN (parent_field))
> >  {
> >/* Not necessary, but also check TREE_PRIVATE for the sake of
> >eliminating obviously non-relevant using decls.  */
> >if (TREE_CODE (parent_field) == USING_DECL
> >   && TREE_PRIVATE (parent_field))
> > {
> > /* If the using statement inherits DECL, it is the source of the
> >   access failure, so return it.  */
> >   if (cp_tree_equal (strip_using_decl (parent_field), decl))
> > return parent_field;
> > }
> >  }
> >
> > I was wrong to say that the using decl does not store "where it came
> > from/what it inherits" - that's exactly what strip_using_decl
> > achieves. I think the problem was that when I did my initial testing
> > in trying out ways to get the original decl, I didn't strip it, so the
> > comparison failed, which led me to make the whole redundant lookup,
> > blah blah blah.
> >
> > I've run a quick test and it seems to work, even with the overloads.
> >
> > Will test it some more and if all's good I will probably send a new
> > patch some time this weekend.
>
> Sounds good, though strip_using_decl (parent_field) may be overloaded if
> the using-decl brings in multiple functions with that name.
>
> Jason
>
>
From b57afd2da59c2ec14c13b76c39aba9126170078d Mon Sep 17 00:00:00 2001
From: Anthony Sharp 
Date: Mon, 1 Mar 2021 21:58:35 +
Subject: [PATCH] c++: Private parent access check for using decls [PR19377]

This bug was already mostly fixed by the patch for PR17314. This
patch continues that by ensuring that where a using decl is used,
causing an access failure to a child class because the using decl is
private, the compiler correctly points to the using decl as the
source of the problem.

gcc/cp/ChangeLog:

2021-03-01  Anthony Sharp  

	* semantics.c (get_class_access_diagnostic_decl): New
	function that examines special cases when a parent
	class causes a private access failure.
	(enforce_access): Slightly modified to call function
	above.

gcc/testsuite/ChangeLog:

Re: [PATCH 4/5] barrier: use int instead of unsigned char for the phase state

[Ville Voutilainen via Gcc-patches]

On Mon, 1 Mar 2021 at 23:54, Thiago Macieira  wrote:
> But your suggestion does work. We don't need to apply them to all macros, only
> those that are new in a given version, like __cpp_lib_atomic_wait or
> __cpp_lib_latch in this case. Alternatively, implementations can set the macro
> to a given value below the standard-required one (incl. 1) to indicate that
> they haven't achieved stability.
>
> That would make my check:
>
> #if __cplusplus >= 202002L && __has_include()
> #  include 
> #endif
> #if __cpp_lib_latch < 201907L
> #  error "Please upgrade your Standard Library"
> #endif

Well, this would be different. What I'm suggesting is not quite that;
for any *new* facility, we'd make sure
that its draft macro and the final IS macro are different, but the
minimum value is the first draft version,
not anything below it. I don't care too much, that approach and yours
would work the same way. Things that already
had an IS value for a macro and haven't changed since don't need to be
changed. And we don't
need to bump all values of existing facilities either, just for those
that got changes, so some existing macros
would be considered lost causes. Like the ones we're talking about,
because the cats are already out of the
bag.

I'll write a paper. That won't help you now, but it gives us tools in
the future.

Re: [PATCH 4/5] barrier: use int instead of unsigned char for the phase state

[Thiago Macieira via Gcc-patches]

On Monday, 1 March 2021 12:35:05 PST Ville Voutilainen wrote:
> I can't make the above code work, in any reasonable manner, because
> it's doing feature detection incorrectly. :)
> What I can imagine doing, however, is this:
> 
> 1) a published IS always bumps feature-macro values (this won't help
> now, it's a future fix, we don't currently do this in WG21)

This is mostly already the case. I haven't seen any need to bump the values.

> 2) an implementation uses the pre-IS draft values before it deems itself
> stable 

Before the IS is stable? From what I've seen so far, the macros are always 
around the month of the latest draft of a given feature. So if an 
implementation implemented an earlier draft, the macro version would increase 
in a new draft coming before the IS.

I think we just need to be careful of updates done after the latest draft is 
adopted, usually by NBs. Those have to bump the macro too. I think you're in 
good speaking terms with the Finland NB :-)

> 3) users who want stability should require the feature-testing macro
> to have at least the IS value

True. That's not the stability I was talking about, but it could be done.

> 4) adventurous users can either use the macro without checking its
> value, or use at least the value that gives them
> whatever shiny toy they're interested in

Fair enough.

Please note I am not talking about the stability of the feature as described 
in the standard or a proposal or draft. I am talking about the stability of 
the implementation. C++20 is out and the atomic-wait feature is stable, as 
defined by the standard. What isn't stable is GCC's implementation of it.

But your suggestion does work. We don't need to apply them to all macros, only 
those that are new in a given version, like __cpp_lib_atomic_wait or 
__cpp_lib_latch in this case. Alternatively, implementations can set the macro 
to a given value below the standard-required one (incl. 1) to indicate that 
they haven't achieved stability.

That would make my check:

#if __cplusplus >= 202002L && __has_include()
#  include 
#endif
#if __cpp_lib_latch < 201907L
#  error "Please upgrade your Standard Library"
#endif

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel DPG Cloud Engineering

Re: [PATCH 4/5] barrier: use int instead of unsigned char for the phase state

[Ville Voutilainen via Gcc-patches]

On Mon, 1 Mar 2021 at 21:44, Thiago Macieira  wrote:
> But you can see someone doing:
>
> #if __cplusplus >= 202002L && __has_include()
> #  include 
> #else
> #  error "Please upgrade your compiler & standard library"
> #endif
>
> and using  in their inline code. And as you say, if they then mix DSOs
> compiled with different versions of GCC, one of them might be the old,
> experimental and binary-incompatible code. Remember that before April 2024,
> the most recent Ubuntu LTS will be 22.04, which will be released before GCC
> 12, which means it will contain GCC 11.
>
> So, wholly aside from how we fix the inefficiencies, we must decide how to
> deal with the ABI break. We can:
>
> a) not have an ABI break in the first place, by having the above code not
>compile with GCC until we promise ABI compatibility
> b) cause a non-silent ABI break
> c) accept the silent ABI break
>
> I advocate for (a) and vehemently disagree with (c). Meanwhile, (b) can be
> something like (untested!):

I can't make the above code work, in any reasonable manner, because
it's doing feature detection incorrectly. :)
What I can imagine doing, however, is this:

1) a published IS always bumps feature-macro values (this won't help
now, it's a future fix, we don't currently do this in WG21)
2) an implementation uses the pre-IS draft values before it deems itself stable
3) users who want stability should require the feature-testing macro
to have at least the IS value
4) adventurous users can either use the macro without checking its
value, or use at least the value that gives them
whatever shiny toy they're interested in

With that, we can keep allowing adventurous users to opt in early, and
you have a portable way to detect that
features are stable, for an implementation-defined definition of "stable".

Re: [PATCH] avoid -Wnonull for dynamic_cast (PR 99251)

[Jason Merrill via Gcc-patches]

On 3/1/21 12:10 PM, Martin Sebor wrote:

On 2/24/21 8:13 PM, Jason Merrill wrote:

On 2/24/21 5:25 PM, Martin Sebor wrote:

In r11-6900 (PR 98646 - static_cast confuses -Wnonnull) we decided
that issuing -Wnonnull for dereferencing the result of dynamic_cast
was helpful despite the false positives it causes when the pointer
is guaranteed not to be null because of a prior test.

The test case in PR 99251 along with the feedback I got from Martin
Liska have convinced me it was not the right decision.

The attached patch arranges for dynamic_cast to also suppress -Wnonnull
analogously to static_cast.  Since there already is a helper function
that builds the if-not-null test (ifnonnull) and sets TREE_NO_WARNING,
I factored out the corresponding code from build_base_path that sets
the additional TREE_NO_WARNING bit for static_cast into the function
and called it from both places.  I also renamed the function to make
its purpose clearer and for consistency with other build_xxx APIs.


Let's call it build_if_nonnull, as it builds the COND_EXPR as well as 
the test.


Done.




+  /* The dynamic_cast might fail but so a warning might be justified
+ but not when the operand is guarded.  See pr99251.  */
+  if (B *q = p->bptr ())
+    dynamic_cast(q)->g ();    // { dg-bogus 
"\\\[-Wnonnull" }


This guard is no more necessary than it is for static_cast; both cases 
deal with null arguments.  Let's not add these checks to the testcases.


Done.



This guard doesn't check for the mentioned case of dynamic_cast 
failing because the B* does not in fact point to a C.


I think we can just change the dg-warning to dg-bogus.  Sure, 
dynamic_cast might fail, but AFAICT -Wnonnull isn't supposed to warn 
about arguments that *might* be null, only arguments that are *known* 
to be null.


Done.

I had added the 'if' to the test to make it clear why the warning
isn't expected.  I replaced it with a comment to that effect.



FWIW, I do think a warning for dynamic_cast to a pointer would be
helpful in the absence of an if statement in these cases:

   void f (B *p)
   {
     dynamic_cast(p)->g ();
   }

Not because p may be null but because the result of the cast may be
for a nonnull p.  If it's f's precondition that D is derived from B
(in addition to p being nonnull) the cast would be better written as
one to a reference to D.


Agreed, or if it isn't a precondition,

if (D* dp = dynamic_cast(p))
  dp->g();


Such a warning would need to be issued by the middle end and although
it could be controlled by a new option (say -Wdynamic-cast, along with
the cases discussed in PR 12277)


Sounds good.


I don't think issuing -Wnonnull in this case would be inappropriate.


I disagree; issuing -Wnonnull for this case would be wrong.  Again, 
-Wnonnull is supposed to warn when an argument *is* null, not when it 
*might* be null.


I think warning about this case is a good idea, just not as part of 
-Wnonnull.



Anyway, that's something to consider for GCC 12.  For now, please see
the revised patch.



* rtti.c (ifnonnull): Rename...
(build_nonnull_test): ...to this.  Set no-warning bit on COND_EXPR.


The ChangeLog needs updating.


+  /* Unlike static_cast, dynamic cast may fail for a nonnull operand but


Yes, but...


+ since the front end can't see if the cast is guarded against being
+ invoked with a null


No; my point was that whether the cast is guarded against being invoked 
with a null is no more relevant for dynamic_cast than it is for 
static_cast, as both casts give a null result for a null argument.


For this test, dynamic_cast is not significantly different from 
static_cast.  For both casts, the bug was the compiler warning about a 
nullptr that it introduced itself.



verify there's no warning.  See also pr99251.  */


Yes.


-  dynamic_cast(p->bptr ())->g ();   // { dg-warning "\\\[-Wnonnull" }
+  dynamic_cast(p)->g ();// { dg-bogus "\\\[-Wnonnull" }


Please put back the ->bptr()s.

Jason

Re: [PATCH] c++: ICE with deduction guide in checking type-dep [PR99009, PR97034]

[Jason Merrill via Gcc-patches]

On 3/1/21 2:54 PM, Marek Polacek wrote:

On Thu, Feb 25, 2021 at 10:45:29PM -0500, Jason Merrill via Gcc-patches wrote:

On 2/25/21 5:41 PM, Marek Polacek wrote:

On Thu, Feb 25, 2021 at 10:59:49AM -0500, Jason Merrill wrote:

On 2/12/21 6:12 PM, Marek Polacek wrote:

We represent deduction guides with FUNCTION_DECLs, but they are built
without DECL_CONTEXT


Hmm, that seems wrong: "A deduction-guide shall be declared in the
same scope as the corresponding class template and, for a member class
template, with the same access."  But it probably isn't necessary to change
this:


leading to an ICE in type_dependent_expression_p
on the assert that the type of a function template with no dependent
(innermost!) template arguments must be non-dependent.  Consider the
attached class-deduction79.C: we create a deduction guide:

 template G(T)-> E::G

we deduce T and create a partial instantiation:

 G(T) -> E::G [with T = int]

And then do_class_deduction wants to create a CALL_EXPR from the above
using build_new_function_call -> build_over_call which calls mark_used
-> maybe_instantiate_noexcept -> type_dependent_expression_p.

There, the innermost template arguments are non-dependent (), but
the fntype is dependent -- the return type is a TYPENAME_TYPE, and
since we have no DECL_CONTEXT, this check holds:

 /* Otherwise, if the function decl isn't from a dependent scope, it can't 
be
type-dependent.  Checking this is important for functions with auto 
return
type, which looks like a dependent type.  */
 if (TREE_CODE (expression) == FUNCTION_DECL
 && !(DECL_CLASS_SCOPE_P (expression)
  && dependent_type_p (DECL_CONTEXT (expression)))

whereupon we ICE.

Experiments with setting DECL_CONTEXT didn't pan out.


In c8 of the PR it looks like you were using the class itself as
DECL_CONTEXT; the quote above says that the right context is the enclosing
scope of the class.


Sadly, using CP_TYPE_CONTEXT (type) would result in a crash in
retrieve_specialization:

/* There should be as many levels of arguments as there are
   levels of parameters.  */
gcc_assert (TMPL_ARGS_DEPTH (args)
== (TREE_CODE (tmpl) == TEMPLATE_DECL
? TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (tmpl))
: template_class_depth (DECL_CONTEXT (tmpl;


Yeah, probably simpler not to bother.


So perhaps we
just want to skip the assert for deduction guides, because they are
a little special.  Better ideas solicited.


In c3 you mention that one of the variants broke with r269093; this is
because my change to check CLASSTYPE_TEMPLATE_INSTANTIATION is false for the
template pattern itself (E).


And the original test started with my r11-1713 because using TREE_TYPE
directly instead of decltype (which is a non-deduced context) means we
can deduced from the argument.

But I think probably the right answer is to defer this deduction until the
enclosing scope is non-dependent, i.e. (untested)


Thanks.  That mostly works, except the new class-deduction-aggr[89].C
tests.  Consider 8:

namespace N {
template  struct S {
template  S(T, U);
};
} // namespace N
template  struct E {
template  struct G { T t; };
void fn() { G{N::S{'a', 1}}; }
};

void
g ()
{
E<1> e;
e.fn ();
}

With your patch, when in do_class_deduction when processing_template_decl,
we just return.  When we call do_class_deduction again when p_t_d is 0,
maybe_aggr_guide returns early here:

if (!CP_AGGREGATE_TYPE_P (type))
  return NULL_TREE

because G is not complete (and rightly so, we didn't instantiate it).  So
we aren't able to deduce the template parameters.  I'm not sure if I should
pursue this direction further.  :(


I think so; we just need to test CP_AGGREGATE_TYPE_P on the original
template pattern instead of the instantiation E<1>::G.


I'm sorry, I've got stuck again.

Yes, using the original template pattern helps us get past the
CP_AGGREGATE_TYPE_P check.

However, using TREE_TYPE (DECL_TI_TEMPLATE (tmpl)) as the type of the deduction 
guide
means the guide will be "template G(T)-> E< >::G" which
results in

   class-deduction-aggr8.C:11:15: error: invalid use of dependent type 'typename E< 
>::G >'

which makes sense I guess: when we defer building up the guide until
we've instantiated E<1>, finding the dependent type E<> is not expected.


Yeah, I was only thinking to use the pattern for the aggregate check.


But creating the guide with "struct E<1>::G" as its type seems
wrong; I'm not even sure if a guide like

   template G(T)-> E<1>::G

makes sense.


It looks fine to me.


In any case the deduction fails (when we call
build_new_function_call in do_class_deduction), because we've got
a mismatch: the template parameter T has level 1, but the template
function parameter has level 2.


Sure, because E<1>::G has been partially instantiated, so the T has 
been reduced from level 2 to 1.


You'll need to do a similar partial instantiation 

Re: [PATCH] C++: target attribute - local decl

[Jason Merrill via Gcc-patches]

On 3/1/21 11:59 AM, Martin Liška wrote:

On 3/1/21 5:36 PM, Jason Merrill wrote:

On 3/1/21 7:43 AM, Martin Liška wrote:

On 2/22/21 11:53 PM, Jason Merrill wrote:
The problem seems to be with the handling of local decls.  If 
DECL_LOCAL_DECL_P, you need to look at DECL_LOCAL_DECL_ALIAS to find 
the namespace-scope decl.  But then if there is no preceding 
namespace-scope declaration, the new decl created by 
push_local_extern_decl_alias doesn't have a cgraph node, either.  I 
guess maybe_function_versions 
also needs to look through DECL_LOCAL_DECL_ALIAS.


Ah, I see. Are you sure about the name 'maybe_function_versions'? I 
can't find it.


Ah, it's maybe_version_functions, sorry.


Thanks, I see the function now.
So about your guess:

I guess maybe_function_versions also needs to look through 
DECL_LOCAL_DECL_ALIAS.


Do you mean maybe_version_functions's argument 'record' should depend on 
DECL_LOCAL_DECL_ALIAS of newdecl/oldddecl
(if present)? Or that DECL_FUNCTION_VERSIONED should be set for 
DECL_LOCAL_DECL_ALIASes of the newdecl/olddecl

function declarations?


The latter.

Jason

Re: [PATCH] c++: ICE with deduction guide in checking type-dep [PR99009, PR97034]

[Marek Polacek via Gcc-patches]

On Thu, Feb 25, 2021 at 10:45:29PM -0500, Jason Merrill via Gcc-patches wrote:
> On 2/25/21 5:41 PM, Marek Polacek wrote:
> > On Thu, Feb 25, 2021 at 10:59:49AM -0500, Jason Merrill wrote:
> > > On 2/12/21 6:12 PM, Marek Polacek wrote:
> > > > We represent deduction guides with FUNCTION_DECLs, but they are built
> > > > without DECL_CONTEXT
> > > 
> > > Hmm, that seems wrong: "A deduction-guide shall be declared in the
> > > same scope as the corresponding class template and, for a member class
> > > template, with the same access."  But it probably isn't necessary to 
> > > change
> > > this:
> > > 
> > > > leading to an ICE in type_dependent_expression_p
> > > > on the assert that the type of a function template with no dependent
> > > > (innermost!) template arguments must be non-dependent.  Consider the
> > > > attached class-deduction79.C: we create a deduction guide:
> > > > 
> > > > template G(T)-> E::G
> > > > 
> > > > we deduce T and create a partial instantiation:
> > > > 
> > > > G(T) -> E::G [with T = int]
> > > > 
> > > > And then do_class_deduction wants to create a CALL_EXPR from the above
> > > > using build_new_function_call -> build_over_call which calls mark_used
> > > > -> maybe_instantiate_noexcept -> type_dependent_expression_p.
> > > > 
> > > > There, the innermost template arguments are non-dependent (), but
> > > > the fntype is dependent -- the return type is a TYPENAME_TYPE, and
> > > > since we have no DECL_CONTEXT, this check holds:
> > > > 
> > > > /* Otherwise, if the function decl isn't from a dependent scope, it 
> > > > can't be
> > > >type-dependent.  Checking this is important for functions with 
> > > > auto return
> > > >type, which looks like a dependent type.  */
> > > > if (TREE_CODE (expression) == FUNCTION_DECL
> > > > && !(DECL_CLASS_SCOPE_P (expression)
> > > >  && dependent_type_p (DECL_CONTEXT (expression)))
> > > > 
> > > > whereupon we ICE.
> > > > 
> > > > Experiments with setting DECL_CONTEXT didn't pan out.
> > > 
> > > In c8 of the PR it looks like you were using the class itself as
> > > DECL_CONTEXT; the quote above says that the right context is the enclosing
> > > scope of the class.
> > 
> > Sadly, using CP_TYPE_CONTEXT (type) would result in a crash in
> > retrieve_specialization:
> > 
> >/* There should be as many levels of arguments as there are
> >   levels of parameters.  */
> >gcc_assert (TMPL_ARGS_DEPTH (args)
> >== (TREE_CODE (tmpl) == TEMPLATE_DECL
> >? TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (tmpl))
> >: template_class_depth (DECL_CONTEXT (tmpl;
> 
> Yeah, probably simpler not to bother.
> 
> > > > So perhaps we
> > > > just want to skip the assert for deduction guides, because they are
> > > > a little special.  Better ideas solicited.
> > > 
> > > In c3 you mention that one of the variants broke with r269093; this is
> > > because my change to check CLASSTYPE_TEMPLATE_INSTANTIATION is false for 
> > > the
> > > template pattern itself (E).
> > 
> > And the original test started with my r11-1713 because using TREE_TYPE
> > directly instead of decltype (which is a non-deduced context) means we
> > can deduced from the argument.
> > > But I think probably the right answer is to defer this deduction until the
> > > enclosing scope is non-dependent, i.e. (untested)
> > 
> > Thanks.  That mostly works, except the new class-deduction-aggr[89].C
> > tests.  Consider 8:
> > 
> > namespace N {
> > template  struct S {
> >template  S(T, U);
> > };
> > } // namespace N
> > template  struct E {
> >template  struct G { T t; };
> >void fn() { G{N::S{'a', 1}}; }
> > };
> > 
> > void
> > g ()
> > {
> >E<1> e;
> >e.fn ();
> > }
> > 
> > With your patch, when in do_class_deduction when processing_template_decl,
> > we just return.  When we call do_class_deduction again when p_t_d is 0,
> > maybe_aggr_guide returns early here:
> > 
> >if (!CP_AGGREGATE_TYPE_P (type))
> >  return NULL_TREE
> > 
> > because G is not complete (and rightly so, we didn't instantiate it).  So
> > we aren't able to deduce the template parameters.  I'm not sure if I should
> > pursue this direction further.  :(
> 
> I think so; we just need to test CP_AGGREGATE_TYPE_P on the original
> template pattern instead of the instantiation E<1>::G.

I'm sorry, I've got stuck again.

Yes, using the original template pattern helps us get past the
CP_AGGREGATE_TYPE_P check.

However, using TREE_TYPE (DECL_TI_TEMPLATE (tmpl)) as the type of the deduction 
guide
means the guide will be "template G(T)-> E< >::G" which
results in

  class-deduction-aggr8.C:11:15: error: invalid use of dependent type 'typename 
E< >::G >'

which makes sense I guess: when we defer building up the guide until
we've instantiated E<1>, finding the dependent type E<> is not expected.

But creating the guide with "struct E<1>::G" as its type seems
wrong; I'm not even sure 

Re: [PATCH 4/5] barrier: use int instead of unsigned char for the phase state

[Thiago Macieira via Gcc-patches]

On Monday, 1 March 2021 10:12:35 PST Ville Voutilainen wrote:
> I do have a question about the intent/concern here, regardless of what
> your patch technically
> does. The ABI break _is_ your concern, and the "heisenbugs" you were
> worried about would
> in fact be caused by the ABI break? So if you embed these things in
> your QAtomicThing, the ABI
> break may mess it up(*)? Is that a correct understanding of the concern
> here?

Let me clarify. I am stating that the current implementation is inefficient 
because Linux currently only implements 32-bit futexes. With that in mind, I 
am arguing we need to change the implementation in a way that will break ABI 
in a new release.

The concern is that such a change, despite being in experimental code for 
which ABI stability has never been promised, is still troublesome. The problem 
there is that even people who have read the documentation and waited until 
2024 to write code using the feature may still be affected. This isn't about 
Qt because I have no plans to use wait and notify in inline API.

But you can see someone doing:

#if __cplusplus >= 202002L && __has_include()
#  include 
#else
#  error "Please upgrade your compiler & standard library"
#endif

and using  in their inline code. And as you say, if they then mix DSOs 
compiled with different versions of GCC, one of them might be the old, 
experimental and binary-incompatible code. Remember that before April 2024, 
the most recent Ubuntu LTS will be 22.04, which will be released before GCC 
12, which means it will contain GCC 11.

So, wholly aside from how we fix the inefficiencies, we must decide how to 
deal with the ABI break. We can:

a) not have an ABI break in the first place, by having the above code not 
   compile with GCC until we promise ABI compatibility
b) cause a non-silent ABI break
c) accept the silent ABI break

I advocate for (a) and vehemently disagree with (c). Meanwhile, (b) can be 
something like (untested!):

 _GLIBCXX_BEGIN_NAMESPACE_VERSION
   namespace __detail
   {
+struct __waiters;
+inline namespace __experimental
+{
+  // from atomic_wait.cc
+  __waiters &__get_waiter(const void *);
+}
 using __platform_wait_t = int;
 
 constexpr auto __atomic_spin_count_1 = 16;
@@ -187,11 +193,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   static __waiters&
   _S_for(const void* __t)
   {
-   const unsigned char __mask = 0xf;
-   static __waiters __w[__mask + 1];
-
-   auto __key = _Hash_impl::hash(__t) & __mask;
-   return __w[__key];
+   return __get_waiter(__t);
   }
 };

That way, any DSO compiled using GCC 11 will fail to load when using GCC 12's 
libstdc++. And probably put "std::__detail::__experimental" in the 
GLIBCXX_EXPERIMENTAL ELF version so it's even clearer that it's experimental, 
thus helping Linux distributions and other binary artefact distributors 
realise they've made a mistake.

I still don't like (b) because it pushes the problem to the wrong people: the 
packagers. But it's far better than (c).

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel DPG Cloud Engineering

Re: [PATCH 4/5] barrier: use int instead of unsigned char for the phase state

[Thomas Rodgers]

On 2021-03-01 10:06, Thiago Macieira wrote:

On Monday, 1 March 2021 09:38:58 PST Thomas Rodgers wrote: And 
_M_phase, despite being non-atomic, is never accessed without the

atomic_ref, aside from the constructor. Both arrive() and wait() start
off by
creating the atomic_ref.
If it's non-atomic, then how is wait() supposed to wait on it,
atomically?



Hey, it's your code :-)

It is using atomics to operate on the value. It's just that the type on 
the

structure isn't an atomic by itself.



Why, I don't know. Olivier's original code did use atomics
:
   std::atomic expected_adjustment;
   std::atomic<__phase_t> phase;


It is atomic, in that it is always an atomic_ref. This change was made 
at

Jonathan's request.


And I am not disagreeing with that. I am, however saying, that I know
this particular implementation (well the upstream one it is based on)
has been extensively tested by the original author (ogiroux) including
time on Summit. If we are going to start changing his design decisions
(beyond the largely cosmetic, not algorithmic, ones that I have made as
per Jonathan's request), they should be motivated by more than a 'well
we feel int's would be better here because Futex' justification.



That's a reasonable request.

I'd like to see the benchmark results of using a directly-futexable 
type vs
using unsigned char. But even the timing results need to be weighed 
against
the increased memory use for std::barrier, which means it's not a 
directly-
objective conclusion. And given we *may* get 8-bit futexes, it might be 
worth
keeping them this way and just tell people with large contentions to 
upgrade.


We may also want to introduce a central barrier type for memory 
constrained environments. I specifically

removed that from this patch because it -

1) wasn't clear how we'd go about making that decision
2) this support in GCC11 is experimental


That of course rests on the contended atomic_wait being out-of-line.

Re: [PATCH 4/5] barrier: use int instead of unsigned char for the phase state

[Ville Voutilainen via Gcc-patches]

On Mon, 1 Mar 2021 at 20:09, Thiago Macieira via Libstdc++
 wrote:
>
> On Sunday, 28 February 2021 07:05:47 PST Hans-Peter Nilsson wrote:
> > On Fri, 26 Feb 2021, Thiago Macieira via Gcc-patches wrote:
> > > ints can be used in futexes. chars can't.
> >
> > Shouldn't that be an atomic type instead of a bare int then?
>
> There are a couple of atomic_refs:
>
>   using __atomic_phase_ref_t = std::__atomic_ref<__barrier_phase_t>;
>   using __atomic_phase_const_ref_t = std::__atomic_ref __barrier_phase_t>;
>
> And _M_phase, despite being non-atomic, is never accessed without the
> atomic_ref, aside from the constructor. Both arrive() and wait() start off by
> creating the atomic_ref.
>
> But I confess I don't understand this code sufficiently to say it is correct.
> I'm simply saying that waiting on unsigned chars will not use a futex, at
> least until https://lkml.org/lkml/2019/12/4/1373 is merged into the kernel.

I do have a question about the intent/concern here, regardless of what
your patch technically
does. The ABI break _is_ your concern, and the "heisenbugs" you were
worried about would
in fact be caused by the ABI break? So if you embed these things in
your QAtomicThing, the ABI
break may mess it up(*)? Is that a correct understanding of the concern here?

(*) And inlining might not help because users might compile a
different std::atomic_thing in their
program than what your DSO has been compiled with, and you may end up
using mixtures.

Re: [PATCH 4/5] barrier: use int instead of unsigned char for the phase state

[Thiago Macieira via Gcc-patches]

On Monday, 1 March 2021 09:38:58 PST Thomas Rodgers wrote:
> > And _M_phase, despite being non-atomic, is never accessed without the
> > atomic_ref, aside from the constructor. Both arrive() and wait() start
> > off by
> > creating the atomic_ref.
> 
> If it's non-atomic, then how is wait() supposed to wait on it,
> atomically?

Hey, it's your code :-)

It is using atomics to operate on the value. It's just that the type on the 
structure isn't an atomic by itself.

Why, I don't know. Olivier's original code did use atomics 
:
std::atomic expected_adjustment;
std::atomic<__phase_t> phase;


> And I am not disagreeing with that. I am, however saying, that I know
> this particular implementation (well the upstream one it is based on)
> has been extensively tested by the original author (ogiroux) including
> time on Summit. If we are going to start changing his design decisions
> (beyond the largely cosmetic, not algorithmic, ones that I have made as
> per Jonathan's request), they should be motivated by more than a 'well
> we feel int's would be better here because Futex' justification.

That's a reasonable request.

I'd like to see the benchmark results of using a directly-futexable type vs 
using unsigned char. But even the timing results need to be weighed against 
the increased memory use for std::barrier, which means it's not a directly-
objective conclusion. And given we *may* get 8-bit futexes, it might be worth 
keeping them this way and just tell people with large contentions to upgrade.

That of course rests on the contended atomic_wait being out-of-line.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel DPG Cloud Engineering

[PATCH] clarify nonaliasing property of attribute malloc (PR 99295)

[Martin Sebor via Gcc-patches]

The documentation change made for pr83023 - branch probabilities
pessimize malloc, introduced an ambiguity into the description of
attribute malloc pointed out in pr99295.

The change suggests that GCC assumes that most calls only to malloc
and calloc but not to realloc return non-null.  Subsequent changes
exacerbated this problem by inserting additional text in between.

The attached change rewords the description to avoid this unintended
reading.

Martin
PR middle-end/99295 - documentation on __attribute__((malloc)) is wrong

gcc/ChangeLog:

	PR middle-end/99295
	* doc/extend.texi (attribute malloc): Reword and clarify nonaliasing
	property.

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 02578cd953f..0debf84e726 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -3240,7 +3240,9 @@ calls.
 Attribute @code{malloc} indicates that a function is @code{malloc}-like,
 i.e., that the pointer @var{P} returned by the function cannot alias any
 other pointer valid when the function returns, and moreover no
-pointers to valid objects occur in any storage addressed by @var{P}.
+pointers to valid objects occur in any storage addressed by @var{P}. In
+addition, the GCC predicts that a function with the attribute returns
+non-null in most cases.
 
 Independently, the form of the attribute with one or two arguments
 associates @code{deallocator} as a suitable deallocation function for
@@ -3248,13 +3250,14 @@ pointers returned from the @code{malloc}-like function.  @var{ptr-index}
 denotes the positional argument to which when the pointer is passed in
 calls to @code{deallocator} has the effect of deallocating it.
 
-Using the attribute with no arguments is designed to improve optimization.
-The compiler predicts that a function with the attribute returns non-null
-in most cases.  Functions like @code{malloc} and @code{calloc} have this
-property because they return a pointer to uninitialized or zeroed-out
-storage.  However, functions like @code{realloc} do not have this property,
-as they may return pointers to storage containing pointers to existing
-objects.
+Using the attribute with no arguments is designed to improve optimization
+by relying on the aliasing property it implies.  Functions like @code{malloc}
+and @code{calloc} have this property because they return a pointer to
+uninitialized or zeroed-out, newly obtained storage.  However, functions
+like @code{realloc} do not have this property, as they may return pointers
+to storage containing pointers to existing objects.  Additionally, since
+all such functions are assumed to return null only infrequently, callers
+can be optimized based on that assumption.
 
 Associating a function with a @var{deallocator} helps detect calls to
 mismatched allocation and deallocation functions and diagnose them under

Re: [PATCH 4/5] barrier: use int instead of unsigned char for the phase state

[Thomas Rodgers]

Botched replying to the list, sending again

On 2021-03-01 09:38, Thomas Rodgers wrote:


On 2021-03-01 09:24, Thiago Macieira via Libstdc++ wrote:

On Sunday, 28 February 2021 07:05:47 PST Hans-Peter Nilsson wrote: On 
Fri, 26 Feb 2021, Thiago Macieira via Gcc-patches wrote: ints can be 
used in futexes. chars can't.

Shouldn't that be an atomic type instead of a bare int then?



There are a couple of atomic_refs:

using __atomic_phase_ref_t = std::__atomic_ref<__barrier_phase_t>;
using __atomic_phase_const_ref_t = std::__atomic_ref;

And _M_phase, despite being non-atomic, is never accessed without the
atomic_ref, aside from the constructor. Both arrive() and wait() start 
off by

creating the atomic_ref.


If it's non-atomic, then how is wait() supposed to wait on it, 
atomically?


But I confess I don't understand this code sufficiently to say it is 
correct.
I'm simply saying that waiting on unsigned chars will not use a futex, 
at
least until https://lkml.org/lkml/2019/12/4/1373 is merged into the 
kernel.


And I am not disagreeing with that. I am, however saying, that I know 
this particular implementation (well the upstream one it is based on) 
has been extensively tested by the original author (ogiroux) including 
time on Summit. If we are going to start changing his design decisions 
(beyond the largely cosmetic, not algorithmic, ones that I have made as 
per Jonathan's request), they should be motivated by more than a 'well 
we feel int's would be better here because Futex' justification.

Re: [PATCH 4/5] barrier: use int instead of unsigned char for the phase state

[Thomas Rodgers]

On 2021-03-01 09:24, Thiago Macieira via Libstdc++ wrote:

On Sunday, 28 February 2021 07:05:47 PST Hans-Peter Nilsson wrote: On 
Fri, 26 Feb 2021, Thiago Macieira via Gcc-patches wrote: ints can be 
used in futexes. chars can't.

Shouldn't that be an atomic type instead of a bare int then?



There are a couple of atomic_refs:

 using __atomic_phase_ref_t = std::__atomic_ref<__barrier_phase_t>;
 using __atomic_phase_const_ref_t = std::__atomic_ref;

And _M_phase, despite being non-atomic, is never accessed without the
atomic_ref, aside from the constructor. Both arrive() and wait() start 
off by

creating the atomic_ref.


If it's non-atomic, then how is wait() supposed to wait on it, 
atomically?


But I confess I don't understand this code sufficiently to say it is 
correct.
I'm simply saying that waiting on unsigned chars will not use a futex, 
at
least until https://lkml.org/lkml/2019/12/4/1373 is merged into the 
kernel.


And I am not disagreeing with that. I am, however saying, that I know 
this particular implementation (well the upstream one it is based on) 
has been extensively tested by the original author (ogiroux) including 
time on Summit. If we are going to start changing his design decisions 
(beyond the largely cosmetic, not algorithmic, ones that I have made as 
per Jonathan's request), they should be motivated by more than a 'well 
we feel int's would be better here because Futex' justification.

Re: [PATCH 4/5] barrier: use int instead of unsigned char for the phase state

[Thiago Macieira via Gcc-patches]

On Sunday, 28 February 2021 07:05:47 PST Hans-Peter Nilsson wrote:
> On Fri, 26 Feb 2021, Thiago Macieira via Gcc-patches wrote:
> > ints can be used in futexes. chars can't.
> 
> Shouldn't that be an atomic type instead of a bare int then?

There are a couple of atomic_refs:

  using __atomic_phase_ref_t = std::__atomic_ref<__barrier_phase_t>;
  using __atomic_phase_const_ref_t = std::__atomic_ref;

And _M_phase, despite being non-atomic, is never accessed without the 
atomic_ref, aside from the constructor. Both arrive() and wait() start off by 
creating the atomic_ref.

But I confess I don't understand this code sufficiently to say it is correct. 
I'm simply saying that waiting on unsigned chars will not use a futex, at 
least until https://lkml.org/lkml/2019/12/4/1373 is merged into the kernel.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel DPG Cloud Engineering

[PATCH 3/3 V2] Do not build Decimal/Float128 conversions if decimal is disabled.

[Michael Meissner via Gcc-patches]

[PATCH 3/3 V2] Do not build Decimal/Float128 conversions if decimal is disabled.

This patch suppresses building the Decimal <-> Float128 conversions if the user
used --disable-decimal-float when configuring GCC.

I have done bootstraps on a little endian power9 system with each of the long
double variants (128-bit IBM, 128-bit IEEE, 64-bit) enabled, and there were no
regressions in each of the builds with the previous version.

In addition, I have built 2 cross compilers from my x86_64 system to little
endian PowerPC Linux.  One build enabled decimal support and one disabled the
decimal support.  On the build that disabled decimal support, I verified that
the _Float128 <-> Decimal conversions were not built.

Can I check this patch into the master branch for GCC 11?

libgcc/
2021-03-01  Michael Meissner  

* config.host (powerpc*-*-linux*): Add t-float128-dec if Decimal
arithmetic is supported.
* config/rs6000/t-float128: Add conditions to suppress building the
Decimal <-> Float128 conversions if --disable-decimal-float.
* config/rs6000/t-float128-dec: New file.
* configure.ac (powerpc*-*-linux*): Record whether decimal arithmetic
is supported.
* configure: Regenerate.
---
 libgcc/config.host  |  6 ++
 libgcc/config/rs6000/t-float128 |  4 
 libgcc/config/rs6000/t-float128-dec |  4 
 libgcc/configure| 21 -
 libgcc/configure.ac |  7 +++
 5 files changed, 41 insertions(+), 1 deletion(-)
 create mode 100644 libgcc/config/rs6000/t-float128-dec

diff --git a/libgcc/config.host b/libgcc/config.host
index f808b61be70..4ab1952899f 100644
--- a/libgcc/config.host
+++ b/libgcc/config.host
@@ -1217,6 +1217,12 @@ powerpc*-*-linux*)
esac
 
if test $libgcc_cv_powerpc_float128 = yes; then
+   # Enable building the decimal/Float128 conversions if decimal
+   # arithmetic is supported in the compiler.
+   if test $libgcc_cv_powerpc_float128_dec = yes; then
+   tmake_file="${tmake_file} rs6000/t-float128-dec"
+   fi
+
tmake_file="${tmake_file} rs6000/t-float128"
fi
 
diff --git a/libgcc/config/rs6000/t-float128 b/libgcc/config/rs6000/t-float128
index 6fb1a3d871b..1d9a0a5e7d7 100644
--- a/libgcc/config/rs6000/t-float128
+++ b/libgcc/config/rs6000/t-float128
@@ -23,6 +23,9 @@ fp128_softfp_shared_obj   = $(addsuffix 
-sw_s$(objext),$(fp128_softfp_funcs))
 fp128_softfp_obj   = $(fp128_softfp_static_obj) $(fp128_softfp_shared_obj)
 
 # Decimal <-> _Float128 conversions
+# FP128_DECIMAL_CONVERT is set in t-float128-dec if decimal arithmetic is
+# supported.
+ifeq ($(FP128_DECIMAL_CONVERT),yes)
 fp128_dec_funcs= _kf_to_sd _kf_to_dd _kf_to_td \
  _sd_to_kf _dd_to_kf _td_to_kf
 
@@ -33,6 +36,7 @@ fp128_decstr_funcs= _strtokf _sprintfkf
 # Decimal <-> __ibm128 conversions
 ibm128_dec_funcs   = _tf_to_sd _tf_to_dd _tf_to_td \
  _sd_to_tf _dd_to_tf _td_to_tf
+endif
 
 # New functions for software emulation
 fp128_ppc_funcs= floattikf floatuntikf fixkfti fixunskfti \
diff --git a/libgcc/config/rs6000/t-float128-dec 
b/libgcc/config/rs6000/t-float128-dec
new file mode 100644
index 000..2873006bb36
--- /dev/null
+++ b/libgcc/config/rs6000/t-float128-dec
@@ -0,0 +1,4 @@
+# Enable building the Float128/Decimal conversion routines.  If decimal support
+# is not enabled, this makefile fragment is not included.
+
+FP128_DECIMAL_CONVERT  = yes
diff --git a/libgcc/configure b/libgcc/configure
index 78fc22a5784..23de3a3adfc 100755
--- a/libgcc/configure
+++ b/libgcc/configure
@@ -4913,7 +4913,7 @@ case "$host" in
 case "$enable_cet" in
   auto)
# Check if target supports multi-byte NOPs
-   # and if assembler supports CET insn.
+   # and if compiler and assembler support CET insn.
cet_save_CFLAGS="$CFLAGS"
CFLAGS="$CFLAGS -fcf-protection"
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
@@ -5228,6 +5228,25 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $libgcc_cv_powerpc_float128" 
>&5
 $as_echo "$libgcc_cv_powerpc_float128" >&6; }
 
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for PowerPC __float128 
libraries with decimal support" >&5
+$as_echo_n "checking for PowerPC __float128 libraries with decimal support... 
" >&6; }
+if ${libgcc_cv_powerpc_float128_dec+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+_Float128 convert (_Decimal128 a) { return a; }
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+  libgcc_cv_powerpc_float128_dec=yes
+else
+  libgcc_cv_powerpc_float128_dec=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: 
$libgcc_cv_powerpc_float128_dec" >&5
+$as_echo 

[PATCH 2/3 V2] Do not include stdio.h in libgcc's Decimal/Float128 conversions.

[Michael Meissner via Gcc-patches]

[PATCH 2/3 V2] Do not include stdio.h in libgcc's Decimal/Float128 conversions.

The _sprintfkf.c file was including stdio.h to get the definition of sprintf.
This patch modifies this so that stdio.h is not included in order to support
freestanding cross compilers that might not provide stdio.h.

I have done bootstraps on a little endian power9 system with each of the long
double variants (128-bit IBM, 128-bit IEEE, 64-bit) enabled, and there were no
regressions in each of the builds with the previous version.

In addition, I have built 2 cross compilers from my x86_64 system to little
endian PowerPC Linux.  One build enabled decimal support and one disabled the
decimal support.  On the build that disabled decimal support, I verified that
the _Float128 <-> Decimal conversions were not built.

Can I check this patch into the master branch for GCC 11?

libgcc/
2021-03-01  Michael Meissner  

* config/rs6000/_sprintfkf.c: Do not include stdio.h.
(__sprintfkf): Call __builtin_sprintf not sprintf.
---
 libgcc/config/rs6000/_sprintfkf.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/libgcc/config/rs6000/_sprintfkf.c 
b/libgcc/config/rs6000/_sprintfkf.c
index 2d624f14e25..9bbc26145ab 100644
--- a/libgcc/config/rs6000/_sprintfkf.c
+++ b/libgcc/config/rs6000/_sprintfkf.c
@@ -27,7 +27,6 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If 
not, see
 #include 
 #include 
 #include 
-#include 
 #include <_sprintfkf.h>
 
 /* This function must be built with IBM 128-bit as long double, so that we can
@@ -42,7 +41,11 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If 
not, see
If we are linked against an earlier library, we will have fake it by
converting the value to long double, and using sprintf to do the conversion.
This isn't ideal, as IEEE 128-bit has more exponent range than IBM
-   128-bit.  */
+   128-bit.
+
+   We use __builtin_sprintf so that we don't have to include stdio.h to define
+   sprintf.  Stdio.h might not be present for freestanding cross compilers that
+   do not need to include a library.  */
 
 extern int __sprintfieee128 (char *restrict, const char *restrict, ...)
   __attribute__ ((__weak__));
@@ -54,5 +57,5 @@ int __sprintfkf (char *restrict string,
   if (__sprintfieee128)
 return __sprintfieee128 (string, format, number);
 
-  return sprintf (string, format, (long double) number);
+  return __builtin_sprintf (string, format, (long double) number);
 }
-- 
2.22.0


-- 
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

[PATCH 1/3 V2] Fix __sprintfkf prototype in libgcc.

[Michael Meissner via Gcc-patches]

[PATCH 1/3 V2] Fix __sprintfkf prototype in libgcc.

The prototype of __sprintfkf in _sprintfkf.h did not match the function in
_sprintfkf.c.  This patch fixes the prototype.  I also included the
_sprintfkf.h file in _sprintfkf.c to make sure the prototype is correct and to
eliminate a warning about declaring the function without a previous
declaration.

I have done bootstraps on a little endian power9 system with each of the long
double variants (128-bit IBM, 128-bit IEEE, 64-bit) enabled, and there were no
regressions in each of the builds with the previous version.

In addition, I have built 2 cross compilers from my x86_64 system to little
endian PowerPC Linux.  One build enabled decimal support and one disabled the
decimal support.  On the build that disabled decimal support, I verified that
the _Float128 <-> Decimal conversions were not built.

Can I check this patch into the master branch for GCC 11?

libgcc/
2021-03-01  Michael Meissner  

* config/rs6000/_sprintfkf.h (__sprintfkf): Fix prototype to match
the function.
* config/rs6000/_sprintfkf.c: Include _sprintfkf.h.
---
 libgcc/config/rs6000/_sprintfkf.c | 1 +
 libgcc/config/rs6000/_sprintfkf.h | 3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libgcc/config/rs6000/_sprintfkf.c 
b/libgcc/config/rs6000/_sprintfkf.c
index a7fdfb483c9..2d624f14e25 100644
--- a/libgcc/config/rs6000/_sprintfkf.c
+++ b/libgcc/config/rs6000/_sprintfkf.c
@@ -28,6 +28,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If 
not, see
 #include 
 #include 
 #include 
+#include <_sprintfkf.h>
 
 /* This function must be built with IBM 128-bit as long double, so that we can
access the strfroml function if do not have an IEEE 128-bit version, and if
diff --git a/libgcc/config/rs6000/_sprintfkf.h 
b/libgcc/config/rs6000/_sprintfkf.h
index 637d104c882..de9d7137f69 100644
--- a/libgcc/config/rs6000/_sprintfkf.h
+++ b/libgcc/config/rs6000/_sprintfkf.h
@@ -24,5 +24,4 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If 
not, see
 /* Declaration of the conversion function to IEEE 128-bit floating point from
string using snprintf.  */
 
-extern int __sprintfkf (char *restrict, const char *restrict, ...);
-
+extern int __sprintfkf (char *restrict, const char *restrict, _Float128);
-- 
2.22.0


-- 
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

[PATCH 0/3 V2] Honor --disable-decimal-float in PowerPC libgcc _Float128

[Michael Meissner via Gcc-patches]

I have broken the patches I submitted on Friday February 26th into 3 patches.
These patches allow us to build the libgcc library on PowerPC for VSX systems
and optionally enable/disable the Decimal support.  If Decimal support is
disabled, then the Float128 <-> Decimal conversions are not built.

I have done bootstraps on a little endian power9 system with each of the long
double variants (128-bit IBM, 128-bit IEEE, 64-bit) enabled, and there were no
regressions in each of the builds with the previous version.

In addition, I have built 2 cross compilers from my x86_64 system to little
endian PowerPC Linux.  One build enabled decimal support and one disabled the
decimal support.  On the build that disabled decimal support, I verified that
the _Float128 <-> Decimal conversions were not built.

Can I check these patches into the master branch for GCC 11?

The 3 patches are:

   #1: Fix __sprintfkf prototype to match between .h and .c files;
   #2: Eliminate including stdio.h in _sprintfkf.c; (and)
   #3: Disable building _Float128 <-> Decimal conversions if
   --disable-decimal-float.

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

Re: [PATCH] avoid -Wnonull for dynamic_cast (PR 99251)

[Martin Sebor via Gcc-patches]

On 2/24/21 8:13 PM, Jason Merrill wrote:

On 2/24/21 5:25 PM, Martin Sebor wrote:

In r11-6900 (PR 98646 - static_cast confuses -Wnonnull) we decided
that issuing -Wnonnull for dereferencing the result of dynamic_cast
was helpful despite the false positives it causes when the pointer
is guaranteed not to be null because of a prior test.

The test case in PR 99251 along with the feedback I got from Martin
Liska have convinced me it was not the right decision.

The attached patch arranges for dynamic_cast to also suppress -Wnonnull
analogously to static_cast.  Since there already is a helper function
that builds the if-not-null test (ifnonnull) and sets TREE_NO_WARNING,
I factored out the corresponding code from build_base_path that sets
the additional TREE_NO_WARNING bit for static_cast into the function
and called it from both places.  I also renamed the function to make
its purpose clearer and for consistency with other build_xxx APIs.


Let's call it build_if_nonnull, as it builds the COND_EXPR as well as 
the test.


Done.




+  /* The dynamic_cast might fail but so a warning might be justified
+ but not when the operand is guarded.  See pr99251.  */
+  if (B *q = p->bptr ())
+    dynamic_cast(q)->g ();    // { dg-bogus 
"\\\[-Wnonnull" }


This guard is no more necessary than it is for static_cast; both cases 
deal with null arguments.  Let's not add these checks to the testcases.


Done.



This guard doesn't check for the mentioned case of dynamic_cast failing 
because the B* does not in fact point to a C.


I think we can just change the dg-warning to dg-bogus.  Sure, 
dynamic_cast might fail, but AFAICT -Wnonnull isn't supposed to warn 
about arguments that *might* be null, only arguments that are *known* to 
be null.


Done.

I had added the 'if' to the test to make it clear why the warning
isn't expected.  I replaced it with a comment to that effect.

FWIW, I do think a warning for dynamic_cast to a pointer would be
helpful in the absence of an if statement in these cases:

  void f (B *p)
  {
dynamic_cast(p)->g ();
  }

Not because p may be null but because the result of the cast may be
for a nonnull p.  If it's f's precondition that D is derived from B
(in addition to p being nonnull) the cast would be better written as
one to a reference to D.

Such a warning would need to be issued by the middle end and although
it could be controlled by a new option (say -Wdynamic-cast, along with
the cases discussed in PR 12277) I don't think issuing -Wnonnull in
this case would be inappropriate.

Anyway, that's something to consider for GCC 12.  For now, please see
the revised patch.

Martin



Jason



PR c++/99251 - inconsistent -Wnonnull warning behaviour with dynamic_cast

gcc/cp/ChangeLog:

	PR c++/99251
	* class.c (build_base_path): Call build_nonnull_test.
	* cp-tree.h (build_nonnull_test): Declare.
	* rtti.c (ifnonnull): Rename...
	(build_nonnull_test): ...to this.  Set no-warning bit on COND_EXPR.
	(build_dynamic_cast_1): Adjust to name change.

gcc/testsuite/ChangeLog:

	PR c++/99251
	* g++.dg/warn/Wnonnull9.C: Expect no warnings.
	* g++.dg/warn/Wnonnull12.C: New test.

diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 40f5fef7baa..b948b601d3f 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -402,16 +402,9 @@ build_base_path (enum tree_code code,
   if (TREE_SIDE_EFFECTS (expr) && (null_test || virtual_access))
 expr = save_expr (expr);
 
-  /* Now that we've saved expr, build the real null test.  */
+  /* Store EXPR and build the real null test just before returning.  */
   if (null_test)
-{
-  tree zero = cp_convert (TREE_TYPE (expr), nullptr_node, complain);
-  null_test = build2_loc (input_location, NE_EXPR, boolean_type_node,
-			  expr, zero);
-  /* This is a compiler generated comparison, don't emit
-	 e.g. -Wnonnull-compare warning for it.  */
-  TREE_NO_WARNING (null_test) = 1;
-}
+null_test = expr;
 
   /* If this is a simple base reference, express it as a COMPONENT_REF.  */
   if (code == PLUS_EXPR && !virtual_access
@@ -516,14 +509,8 @@ build_base_path (enum tree_code code,
 
  out:
   if (null_test)
-{
-  expr = fold_build3_loc (input_location, COND_EXPR, target_type, null_test,
-			  expr, build_zero_cst (target_type));
-  /* Avoid warning for the whole conditional expression (in addition
-	 to NULL_TEST itself -- see above) in case the result is used in
-	 a nonnull context that the front end -Wnonnull checks.  */
-  TREE_NO_WARNING (expr) = 1;
-}
+/* Wrap EXPR in a null test.  */
+expr = build_if_nonnull (null_test, expr, complain);
 
   return expr;
 }
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 38b31e3908f..a4f6bd67cef 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7271,6 +7271,7 @@ extern void emit_support_tinfos			(void);
 extern bool emit_tinfo_decl			(tree);
 extern unsigned get_pseudo_tinfo_index		(tree);
 extern tree get_pseudo_tinfo_type		(unsigned);

Re: [PATCH] C++: target attribute - local decl

[Martin Liška]

On 3/1/21 5:36 PM, Jason Merrill wrote:

On 3/1/21 7:43 AM, Martin Liška wrote:

On 2/22/21 11:53 PM, Jason Merrill wrote:

The problem seems to be with the handling of local decls.  If 
DECL_LOCAL_DECL_P, you need to look at DECL_LOCAL_DECL_ALIAS to find the 
namespace-scope decl.  But then if there is no preceding namespace-scope 
declaration, the new decl created by push_local_extern_decl_alias doesn't have 
a cgraph node, either.  I guess maybe_function_versions also needs to look 
through DECL_LOCAL_DECL_ALIAS.


Ah, I see. Are you sure about the name 'maybe_function_versions'? I can't find 
it.


Ah, it's maybe_version_functions, sorry.


Thanks, I see the function now.
So about your guess:


I guess maybe_function_versions also needs to look through 
DECL_LOCAL_DECL_ALIAS.


Do you mean maybe_version_functions's argument 'record' should depend on 
DECL_LOCAL_DECL_ALIAS of newdecl/oldddecl
(if present)? Or that DECL_FUNCTION_VERSIONED should be set for 
DECL_LOCAL_DECL_ALIASes of the newdecl/olddecl
function declarations?

Thanks,
Martin



Jason

Re: [PATCH] C++: target attribute - local decl

[Jason Merrill via Gcc-patches]

On 3/1/21 7:43 AM, Martin Liška wrote:

On 2/22/21 11:53 PM, Jason Merrill wrote:
The problem seems to be with the handling of local decls.  If 
DECL_LOCAL_DECL_P, you need to look at DECL_LOCAL_DECL_ALIAS to find 
the namespace-scope decl.  But then if there is no preceding 
namespace-scope declaration, the new decl created by 
push_local_extern_decl_alias doesn't have a cgraph node, either.  I 
guess maybe_function_versions 
also needs to look through DECL_LOCAL_DECL_ALIAS.


Ah, I see. Are you sure about the name 'maybe_function_versions'? I 
can't find it.


Ah, it's maybe_version_functions, sorry.

Jason

Re: [PATCH 1/5] std::latch: reduce internal implementation from ptrdiff_t to int

[Thomas Rodgers]

On 2021-02-28 13:31, Hans-Peter Nilsson wrote:


On Fri, 26 Feb 2021, Thiago Macieira via Gcc-patches wrote:

On Friday, 26 February 2021 11:31:00 PST Andreas Schwab wrote: On Feb 
26 2021, Thiago Macieira wrote: On Friday, 26 February 2021 10:14:42 
PST Andreas Schwab wrote: On Feb 26 2021, Thiago Macieira via 
Gcc-patches wrote: -alignas(__alignof__(ptrdiff_t)) ptrdiff_t 
_M_a;

+alignas(__alignof__(int)) int _M_a;
Futexes must be aligned to 4 bytes.

Agreed, but doesn't this accomplish that?

No.  It uses whatever alignment the type already has, and is an
elaborate no-op.
I thought so too when I read the original line. But I expected it was 
written
like that for a reason, especially since the same pattern appears in 
other

places.


I can change to "alignas(4)" (which is a GCC extension, I believe). Is 
that

the correct solution?
IMNSHO make use of the corresponding atomic type.  Then there'd
be no need for separate what's-the-right-align-curse games.


There is no predicate wait on atomic.


brgds, H-P

Re: [PATCH 4/5] barrier: use int instead of unsigned char for the phase state

[Thomas Rodgers]

On 2021-02-28 07:05, Hans-Peter Nilsson wrote:


On Fri, 26 Feb 2021, Thiago Macieira via Gcc-patches wrote:


ints can be used in futexes. chars can't.


Shouldn't that be an atomic type instead of a bare int then?



It's an atomic_ref.


---
libstdc++-v3/include/std/barrier | 21 -
1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/libstdc++-v3/include/std/barrier 
b/libstdc++-v3/include/std/barrier

index e09212dfcb9..ae058bd3dc3 100644
--- a/libstdc++-v3/include/std/barrier
+++ b/libstdc++-v3/include/std/barrier
@@ -70,7 +70,7 @@ It looks different from literature pseudocode for 
two main reasons:


*/

-  enum class __barrier_phase_t : unsigned char { };
+  enum class __barrier_phase_t : int { };


brgds, H-P

Re: add -mpowerpc-gpopt to options for sqrt insn on PowerPC

[Segher Boessenkool]

On Mon, Mar 01, 2021 at 09:46:29AM +0100, Richard Biener wrote:
> On Sat, Feb 27, 2021 at 2:43 AM Segher Boessenkool
>  wrote:
> >
> > On Fri, Feb 26, 2021 at 12:31:16PM -0500, David Edelsohn wrote:
> > > On Fri, Feb 26, 2021 at 11:09 AM Alexandre Oliva  
> > > wrote:
> > > >
> > > > This patch avoids an ICE in gimplefe-28.c, in our ppc64-vxworks7r2
> > > > tests.  Tested on x86_64-linux-gnu, and on the affected platform.  Ok to
> > > > install?
> > >
> > > I'm sort of surprised that sqrt instruction would be available for the
> > > target but not enabled by default.  Is this really the method that
> > > customers would use?  It seems that it might be more appropriate to
> > > xfail or skip the test for PPC64 VxWorks.
> >
> > You should essentially never use -mpowerpc-gpopt, but instead use a
> > -mcpu= that has it.  You also of course whould not do that in run tests
> > if the cpu does not support those insns.
> >
> > But, Alex, you say this avoids an ICE?  You shouldn't avoid the ICE in
> > that testcase then -- instead, fix it!  (Or report it).
> 
> The testcase uses a .SQRT direct-internal function and guards itself with
> 
> /* { dg-do compile { target sqrt_insn } } */
> /* { dg-options "-fgimple -O2" } */
> /* { dg-add-options sqrt_insn } */
> 
> if the power dg setup of sqrt_insn doesn't support this it should be adjusted.

Yeah.  It should test _ARCH_PPCSQ.  I'll make a patch.

> Using -mcpu=X for sqrt_insn is likely undesirable but for this testcase would
> work (it would make testing with, say, -mcpu=power4 not work).

Works differently on different dejagnu versions, which is why we have
the -mdejagnu=cpu= kludge.

> So I'd
> say as alternative to Alex patch you could adjust
> 
> # Return 1 if the target supports hardware square root instructions.
> 
> proc check_effective_target_sqrt_insn { } {
> return [check_cached_effective_target sqrt_insn {
>   expr { [istarget i?86-*-*] || [istarget x86_64-*-*]
>  || [istarget powerpc*-*-*]
>  || [istarget aarch64*-*-*]
>  || ([istarget arm*-*-*] && [check_effective_target_arm_vfp_ok])
>  || ([istarget s390*-*-*]
>  && [check_effective_target_s390_vx])
>  || [istarget amdgcn-*-*] }}]
> }
> 
> to only say yes in case the selected CPU supports sqrt.

Right, replace

  || [istarget powerpc*-*-*]

with

  || [check_effective_target_powerpc_sqrt]

(and define that :-) )


Segher

[PATCH] IBM Z: Fix testcase vcond-shift.c

[Stefan Schulze Frielinghaus via Gcc-patches]

As of commit 3a6e3ad38a17a03ee0139b49a0946e7b9ded1eb1 expressions
x CMP y ? -1 : 0 are fold into x CMP y.  Due to this we do not see
shifts anymore after expand in our testcases but comparisons.  Thus
replace instructions vesraX by corresponding vchX.  Keep testcases
vchX_{lt,gt} where only a relational comparison is done and no shift in
order to keep test coverage for vectorization.

gcc/testsuite/ChangeLog:

* gcc.target/s390/vector/vcond-shift.c: Replace vesraX
instructions by corresponding vchX instructions.
---
 .../gcc.target/s390/vector/vcond-shift.c  | 31 ++-
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c 
b/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c
index a6b4e97aa50..9e472aef960 100644
--- a/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c
+++ b/gcc/testsuite/gcc.target/s390/vector/vcond-shift.c
@@ -3,10 +3,13 @@
 /* { dg-do compile { target { s390*-*-* } } } */
 /* { dg-options "-O3 -march=z13 -mzarch" } */
 
-/* { dg-final { scan-assembler-times "vesraf\t%v.?,%v.?,31" 6 } } */
-/* { dg-final { scan-assembler-times "vesrah\t%v.?,%v.?,15" 6 } } */
-/* { dg-final { scan-assembler-times "vesrab\t%v.?,%v.?,7" 6 } } */
-/* { dg-final { scan-assembler-not "vzero\t*" } } */
+/* { dg-final { scan-assembler-times "vzero\t" 9 } } */
+/* { dg-final { scan-assembler-times "vchf\t" 6 } } */
+/* { dg-final { scan-assembler-times "vesraf\t%v.?,%v.?,1" 2 } } */
+/* { dg-final { scan-assembler-times "vchh\t" 6 } } */
+/* { dg-final { scan-assembler-times "vesrah\t%v.?,%v.?,1" 2 } } */
+/* { dg-final { scan-assembler-times "vchb\t" 6 } } */
+/* { dg-final { scan-assembler-times "vesrab\t%v.?,%v.?,1" 2 } } */
 /* { dg-final { scan-assembler-times "vesrlf\t%v.?,%v.?,31" 4 } } */
 /* { dg-final { scan-assembler-times "vesrlh\t%v.?,%v.?,15" 4 } } */
 /* { dg-final { scan-assembler-times "vesrlb\t%v.?,%v.?,7" 4 } } */
@@ -15,19 +18,19 @@
 #define ITER(X) (2 * (16 / sizeof (X[1])))
 
 void
-vesraf_div (int *x)
+vchf_vesraf_div (int *x)
 {
   int i;
   int *xx = __builtin_assume_aligned (x, 8);
 
   /* Should expand to (xx + (xx < 0 ? 1 : 0)) >> 1
- which in turn should get simplified to (xx + (xx >> 31)) >> 1.  */
+ which in turn should get simplified to (xx - (xx < 0)) >> 1.  */
   for (i = 0; i < ITER (xx); i++)
 xx[i] = xx[i] / 2;
 }
 
 void
-vesrah_div (short *x)
+vchh_vesrah_div (short *x)
 {
   int i;
   short *xx = __builtin_assume_aligned (x, 8);
@@ -38,7 +41,7 @@ vesrah_div (short *x)
 
 
 void
-vesrab_div (signed char *x)
+vchb_vesrab_div (signed char *x)
 {
   int i;
   signed char *xx = __builtin_assume_aligned (x, 8);
@@ -50,7 +53,7 @@ vesrab_div (signed char *x)
 
 
 int
-vesraf_lt (int *x)
+vchf_lt (int *x)
 {
   int i;
   int *xx = __builtin_assume_aligned (x, 8);
@@ -60,7 +63,7 @@ vesraf_lt (int *x)
 }
 
 int
-vesrah_lt (short *x)
+vchh_lt (short *x)
 {
   int i;
   short *xx = __builtin_assume_aligned (x, 8);
@@ -70,7 +73,7 @@ vesrah_lt (short *x)
 }
 
 int
-vesrab_lt (signed char *x)
+vchb_lt (signed char *x)
 {
   int i;
   signed char *xx = __builtin_assume_aligned (x, 8);
@@ -82,7 +85,7 @@ vesrab_lt (signed char *x)
 
 
 int
-vesraf_ge (int *x)
+vchf_ge (int *x)
 {
   int i;
   int *xx = __builtin_assume_aligned (x, 8);
@@ -92,7 +95,7 @@ vesraf_ge (int *x)
 }
 
 int
-vesrah_ge (short *x)
+vchh_ge (short *x)
 {
   int i;
   short *xx = __builtin_assume_aligned (x, 8);
@@ -102,7 +105,7 @@ vesrah_ge (short *x)
 }
 
 int
-vesrab_ge (signed char *x)
+vchb_ge (signed char *x)
 {
   int i;
   signed char *xx = __builtin_assume_aligned (x, 8);
-- 
2.23.0

Re: [arm/testsuite]: Skip pr97969.c if -mthumb is not compatible [PR target/97969]

[Christophe Lyon via Gcc-patches]

Ping?

On Wed, 3 Feb 2021 at 10:01, Christophe Lyon  wrote:
>
> Ping?
> I guess that's obvious enough?
>
> On Wed, 27 Jan 2021 at 10:03, Christophe Lyon
>  wrote:
> >
> > Depending on how the toolchain is configured or how the testsuite is
> > executed, -mthumb may not be compatible. Like for other tests, skip
> > pr97969.c in this case.
> >
> > For instance arm-linux-gnueabihf and -march=armv5t in RUNTESTFLAGS.
> >
> > 2021-01-27  Christophe Lyon  
> >
> > gcc/testsuite/
> > PR target/97969
> > * gcc.target/arm/pr97969.c: Skip if thumb mode is not available.
> >
> > diff --git a/gcc/testsuite/gcc.target/arm/pr97969.c
> > b/gcc/testsuite/gcc.target/arm/pr97969.c
> > index 714a1d1..0b5d07f 100644
> > --- a/gcc/testsuite/gcc.target/arm/pr97969.c
> > +++ b/gcc/testsuite/gcc.target/arm/pr97969.c
> > @@ -1,4 +1,5 @@
> >  /* { dg-do compile } */
> > +/* { dg-skip-if "" { ! { arm_thumb1_ok || arm_thumb2_ok } } } */
> >  /* { dg-options "-std=c99 -fno-omit-frame-pointer -mthumb -w -Os" } */
> >
> >  typedef a[23];

[PATCH][pushed] s390: add exceptions for param modified by target pragma

[Martin Liška]

There are 4 more exceptions for params that are modified
in target pragma.

Martin

gcc/ChangeLog:

PR target/99313
* optc-save-gen.awk: Add 4 more exceptions.

gcc/testsuite/ChangeLog:

PR target/99313
* gcc.target/s390/target-attribute/pr99313.c: New test.
---
 gcc/optc-save-gen.awk| 6 ++
 gcc/testsuite/gcc.target/s390/target-attribute/pr99313.c | 5 +
 2 files changed, 11 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/s390/target-attribute/pr99313.c

diff --git a/gcc/optc-save-gen.awk b/gcc/optc-save-gen.awk
index b9c7187a3b9..14b8d03888e 100644
--- a/gcc/optc-save-gen.awk
+++ b/gcc/optc-save-gen.awk
@@ -1445,6 +1445,12 @@ checked_options["TARGET_CASE_VECTOR_PC_RELATIVE"]++
 checked_options["arc_size_opt_level"]++
 # arm exceptions
 checked_options["arm_fp16_format"]++
+# s390 exceptions
+checked_options["param_max_completely_peel_times"]++
+checked_options["param_max_completely_peeled_insns"]++
+checked_options["param_max_unroll_times"]++
+checked_options["param_max_unrolled_insns"]++
+
 
 for (i = 0; i < n_opts; i++) {

name = var_name(flags[i]);
diff --git a/gcc/testsuite/gcc.target/s390/target-attribute/pr99313.c 
b/gcc/testsuite/gcc.target/s390/target-attribute/pr99313.c
new file mode 100644
index 000..bd85983964c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/target-attribute/pr99313.c
@@ -0,0 +1,5 @@
+/* PR target/99313 */
+
+#pragma GCC push_options
+#pragma GCC target ("arch=z13")
+#pragma GCC pop_options
--
2.30.1

c++: Completeness of typedef structs [PR 99294]

[Nathan Sidwell]

When we	read in	a class	definition, we use fixup_type_variants to 
propagate the now-completed fields of the class's TYPE to other 
variants.  Unfortunately that doesn't propagate all of them, and in this 
case we had a typedef to an (incomplete) instantiation.  That typedef 
ended up with a VOIDmode, which blew up gimple expansion as the type 
itself isn't VOID.  Without modules, that information is propagated in 
finalize_type_size when laying out the class.  But that doesn't happen 
with stream-in -- we already know the layout.  There is already some 
overlap between the two functions, now there's a bit more.  In 
fixup_type_variants, I pay attention to the TYPE_NAME	to decide whether 
to override a user's TYPE_ALIGN -- variants of the main-variant typedef 
just copy the main-variant.  Other variants recalculate.  Overaligning 
is still permitted.


I also added a TYPE_ALIGN_RAW accessor,	and fixed a bug	in the alignment 
streaming I noticed.	I did not refactor TYPE_ALIGN beyond using the new 
accessor.  (It could be written as ((1 << align_raw) >> 1), rather than 
use the conditional.)


PR c++/99294
gcc/
* gcc/tree.h (TYPE_ALIGN_RAW): New accessor.
(TYPE_ALIGN): Use it.
gcc/cp/
* class.c (fixup_type_variants): Propagate mode, precision,
alignment & emptiness.
* module.cc (trees_out::type_node): Use TYPE_ALIGN_RAW.
(trees_in::tree_node): Rematerialize alignment here.
gcc/testsuite/
* g++.dg/modules/pr99294.h: New.
* g++.dg/modules/pr99294_a.C: New.
* g++.dg/modules/pr99294_b.C: New.



--
Nathan Sidwell
diff --git c/gcc/cp/class.c w/gcc/cp/class.c
index 40f5fef7baa..ea007e88e6e 100644
--- c/gcc/cp/class.c
+++ w/gcc/cp/class.c
@@ -2005,35 +2005,45 @@ determine_primary_bases (tree t)
 /* Update the variant types of T.  */
 
 void
-fixup_type_variants (tree t)
+fixup_type_variants (tree type)
 {
-  tree variants;
-
-  if (!t)
+  if (!type)
 return;
 
-  for (variants = TYPE_NEXT_VARIANT (t);
-   variants;
-   variants = TYPE_NEXT_VARIANT (variants))
+  for (tree variant = TYPE_NEXT_VARIANT (type);
+   variant;
+   variant = TYPE_NEXT_VARIANT (variant))
 {
   /* These fields are in the _TYPE part of the node, not in
 	 the TYPE_LANG_SPECIFIC component, so they are not shared.  */
-  TYPE_HAS_USER_CONSTRUCTOR (variants) = TYPE_HAS_USER_CONSTRUCTOR (t);
-  TYPE_NEEDS_CONSTRUCTING (variants) = TYPE_NEEDS_CONSTRUCTING (t);
-  TYPE_HAS_NONTRIVIAL_DESTRUCTOR (variants)
-	= TYPE_HAS_NONTRIVIAL_DESTRUCTOR (t);
+  TYPE_HAS_USER_CONSTRUCTOR (variant) = TYPE_HAS_USER_CONSTRUCTOR (type);
+  TYPE_NEEDS_CONSTRUCTING (variant) = TYPE_NEEDS_CONSTRUCTING (type);
+  TYPE_HAS_NONTRIVIAL_DESTRUCTOR (variant)
+	= TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type);
 
-  TYPE_POLYMORPHIC_P (variants) = TYPE_POLYMORPHIC_P (t);
-  CLASSTYPE_FINAL (variants) = CLASSTYPE_FINAL (t);
+  TYPE_POLYMORPHIC_P (variant) = TYPE_POLYMORPHIC_P (type);
+  CLASSTYPE_FINAL (variant) = CLASSTYPE_FINAL (type);
 
-  TYPE_BINFO (variants) = TYPE_BINFO (t);
+  TYPE_BINFO (variant) = TYPE_BINFO (type);
 
   /* Copy whatever these are holding today.  */
-  TYPE_VFIELD (variants) = TYPE_VFIELD (t);
-  TYPE_FIELDS (variants) = TYPE_FIELDS (t);
+  TYPE_VFIELD (variant) = TYPE_VFIELD (type);
+  TYPE_FIELDS (variant) = TYPE_FIELDS (type);
+
+  TYPE_SIZE (variant) = TYPE_SIZE (type);
+  TYPE_SIZE_UNIT (variant) = TYPE_SIZE_UNIT (type);
+
+  if (!TYPE_USER_ALIGN (variant)
+	  || TYPE_NAME (variant) == TYPE_NAME (type)
+	  || TYPE_ALIGN_RAW (variant) < TYPE_ALIGN_RAW (type))
+	{
+	  TYPE_ALIGN_RAW (variant) =  TYPE_ALIGN_RAW (type);
+	  TYPE_USER_ALIGN (variant) = TYPE_USER_ALIGN (type);
+	}
 
-  TYPE_SIZE (variants) = TYPE_SIZE (t);
-  TYPE_SIZE_UNIT (variants) = TYPE_SIZE_UNIT (t);
+  TYPE_PRECISION (variant) = TYPE_PRECISION (type);
+  TYPE_MODE_RAW (variant) = TYPE_MODE_RAW (type);
+  TYPE_EMPTY_P (variant) = TYPE_EMPTY_P (type);
 }
 }
 
diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc
index 0cb5bd9b644..369a02604fe 100644
--- c/gcc/cp/module.cc
+++ w/gcc/cp/module.cc
@@ -8714,7 +8714,7 @@ trees_out::type_node (tree type)
   else
 	{
 	  if (TYPE_USER_ALIGN (type))
-	flags = exact_log2 (TYPE_ALIGN (type));
+	flags = TYPE_ALIGN_RAW (type);
 	}
 
   if (streaming_p ())
@@ -9510,7 +9510,7 @@ trees_in::tree_node (bool is_use)
 	  }
 	else
 	  {
-	res = build_aligned_type (res, 1u << flags);
+	res = build_aligned_type (res, (1u << flags) >> 1);
 	TYPE_USER_ALIGN (res) = true;
 	  }
 
diff --git c/gcc/testsuite/g++.dg/modules/pr99294.h w/gcc/testsuite/g++.dg/modules/pr99294.h
new file mode 100644
index 000..757113c0283
--- /dev/null
+++ w/gcc/testsuite/g++.dg/modules/pr99294.h
@@ -0,0 +1,14 @@
+
+template 
+class basic_string;
+
+typedef basic_string string;
+
+template 
+class basic_string
+{
+ public:
+ 

Re: [PATCH] PING implement pre-c++20 contracts

[Jeff Chapman via Gcc-patches]

On 1/18/21, Jason Merrill  wrote:
> On 1/4/21 9:58 AM, Jeff Chapman wrote:
>> Ping. re:
>> https://gcc.gnu.org/pipermail/gcc-patches/2020-December/561135.html
>> 
>>
>> https://github.com/lock3/gcc/tree/contracts-jac-alt
>> 
>>
> Why is some of the code in c-family?  From the modules merge there is
> now a cp_handle_option function that you could add the option handling
> to, and I don't see a reason for cxx-contracts.c to be in c-family/
> rather than cp/.

I've been pushing changes that address the points raised and wanted to
ping to see if there's more feedback and give a status summary. The
notable change is making sure the implementation plays nicely with
modules and a mangling change that did away with a good chunk of code.

The contracts code outside of cp/ has been moved into it, and the
contract attributes have been altered to work with language independent
handling code. More of the contracts code can probably still be moved to
cxx-contracts which I'll loop back to after other refactoring. The
naming, spelling, and comments (or lack thereof) have been addressed.

>> +set_decl_contracts (tree decl, tree contract_attrs)
> I think you want to use 'chainon' here.

>> +build_arg_list (tree fn)
> You can use is_this_parameter here.

Thanks; I knew of chainon but didn't think of it and is_this_parameter
is new to me. Always fun to delete code :)

>> +  /* If we have contracts, check that they're valid in this context.  */
>> +  // FIXME: These aren't entirely correct.
>
> How not?  Can local extern function decls have contract attributes?
>
>> + /* FIXME when we instatiate a template class with guarded
>> +  * members, particularly guarded template members, the 
>> resulting
>> +  * pre/post functions end up being inaccessible because their
>> +  * template info's context is the original uninstantiated 
>> class.
>
> This sounds like a significant functionality gap.  I'm guessing you want
> to reconsider the unshare_template approach.
>
>> +  /* FIXME do we need magic to perfectly forward this so we don't 
>> clobber
>> +RVO/NRVO etc?  */
>
> Yes.  CALL_FROM_THUNK_P will probably get you some of the magic you want.

These points are still being investigated and addressed; including them
for reference.

> More soon.

Please let me know what other issues need work.


Thank you!
Jeff Chapman

Patch ping^2

[Jakub Jelinek via Gcc-patches]

On Tue, Feb 23, 2021 at 09:49:08AM +0100, Jakub Jelinek via Gcc-patches wrote:
> I'd like to ping the
> https://gcc.gnu.org/pipermail/gcc-patches/2021-February/565350.html
> patch, P2 PR99085 ice-on-valid-code fix in fixup_partitions.

Ping

Thanks

Jakub

Re: [PATCH] C++: target attribute - local decl

[Martin Liška]

On 2/22/21 11:53 PM, Jason Merrill wrote:

The problem seems to be with the handling of local decls.  If 
DECL_LOCAL_DECL_P, you need to look at DECL_LOCAL_DECL_ALIAS to find the 
namespace-scope decl.  But then if there is no preceding namespace-scope 
declaration, the new decl created by push_local_extern_decl_alias doesn't have 
a cgraph node, either.  I guess maybe_function_versions also needs to look 
through DECL_LOCAL_DECL_ALIAS.


Ah, I see. Are you sure about the name 'maybe_function_versions'? I can't find 
it.

Thanks,
Martin

Re: [PATCH V3 0/5] Support for the CTF and BTF debug formats

[Jose E. Marchesi via Gcc-patches]

> - More testing: we are building Gentoo with -gctf activated by
>   default and fixing problems as we find them.  More than 160 packages
>   built so far.]

This count is now 860 packages built ;)

Re: [PATCH] Change semantics of -frecord-gcc-switches and add -frecord-gcc-switches-format.

[Martin Liška]

PING

On 2/18/21 10:18 AM, Martin Liška wrote:

On 2/16/21 10:17 PM, Qing Zhao wrote:

Hello,

What’s the status of this patch now? Is there any major technical issue with 
the patch?

Our company has been waiting for this patch for almost one year, we need it for 
our important application.


Hello.

You are right, we've been waiting for quite some time.



Could this one be approved and committed to gcc11?


@richi: ?

Thanks,
Martin



Thanks.

Qing


On Feb 5, 2021, at 3:34 AM, Martin Liška  wrote:

Hello.

Based on discussion with Richi, I'm re-sending the patch. Note that the patch
has been waiting for a review for almost one year and I would like to see
it in GCC 11.1.

Thank you,
Martin
<0001-Change-semantics-of-frecord-gcc-switches-and-add-fre.patch>

[Patch] libgfortran: Fix negation for largest integer [PR81986]

[Tobias Burnus]

Found by Vittorio Zecca when using the sanitizer on libgomp
(bootstrap-ubsan) on pr66311.f90, which uses '(-huge(0_16))-1'.

The error then occurs in libgfortran/runtime/string.c's
  gfc_itoa (GFC_INTEGER_LARGEST n, char *buffer, size_t len)
where (as shown 'n') is signed and 't' is:
  GFC_UINTEGER_LARGEST t;

As proposed by Vittorio (comment 2) and acknowledged by Jakub
(comment 3), the conversion to UINT has to happen before the
negation.


For completeness, the result is the following.
(Test is similar in pr66311.f90, albeit slightly different.)

  write(buffer,*) huge(0_16)
  if (buffer /= "  170141183460469231731687303715884105727") stop 1

  write(buffer,*) huge(0_16)-1
  if (buffer /= "  170141183460469231731687303715884105726") stop 2

  write(buffer,*) -(huge(0_16)-1)
  if (buffer /= " -170141183460469231731687303715884105726") stop 3

  ! Undefined behaviour as (-huge(0_16)) overflows:
  write(buffer,*) -huge(0_16)-1
  if (buffer /= " -170141183460469231731687303715884105728") stop 4


I intent to commit the patch later as obvious unless there are further comments.

Tobias

-
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München 
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank 
Thürauf
libgfortran: Fix negation for largest integer [PR81986]

libgfortran/ChangeLog:

2021-03-01  Vittorio Zecca  
	Tobias Burnus  

	PR libfortran/81986
	* runtime/string.c (gfc_itoa):

diff --git a/libgfortran/runtime/string.c b/libgfortran/runtime/string.c
index 37c4da0a98a..536a9cd3f2b 100644
--- a/libgfortran/runtime/string.c
+++ b/libgfortran/runtime/string.c
@@ -196,7 +196,7 @@ gfc_itoa (GFC_INTEGER_LARGEST n, char *buffer, size_t len)
   if (n < 0)
 {
   negative = 1;
-  t = -n; /*must use unsigned to protect from overflow*/
+  t = -(GFC_UINTEGER_LARGEST) n;  /* Must use unsigned to protect from overflow. */
 }
 
   p = buffer + GFC_ITOA_BUF_SIZE - 1;

Re: [PATCH][comitted] Testsuite: Disable PR99149 test on big-endian

[Richard Sandiford via Gcc-patches]

Tamar Christina  writes:
> Hi All,
>
> This patch disables the test for PR99149 on Big-endian
> where for standard AArch64 the patterns are disabled.
>
> Regtested on aarch64-none-linux-gnu and no issues.
>
> Committed under the obvious rule.
>
> Thanks,
> Tamar
>
> gcc/testsuite/ChangeLog:
>
>   PR tree-optimization/99149
>   * g++.dg/vect/pr99149.cc: Disabled on BE.
>
> --- inline copy of patch -- 
> diff --git a/gcc/testsuite/g++.dg/vect/pr99149.cc 
> b/gcc/testsuite/g++.dg/vect/pr99149.cc
> index 
> 9002e3e5268a6c431d0de076d6768c12d79f39f0..00ebe9d9cdf600ada8e66b4b854f0e18ad0b6a7d
>  100755
> --- a/gcc/testsuite/g++.dg/vect/pr99149.cc
> +++ b/gcc/testsuite/g++.dg/vect/pr99149.cc
> @@ -1,4 +1,4 @@
> -/* { dg-do compile { target { aarch64*-*-* } } } */
> +/* { dg-do compile { target { aarch64-*-* } } } */

This isn't enough for multilibs.  I think it should be:

/* { dg-do compile { target aarch64_little_endian } } */

instead.

Thanks,
Richard

[PATCH V3 3/5] CTF/BTF testsuites

[Jose E. Marchesi via Gcc-patches]

This commit adds a new testsuite for the CTF debug format.

2021-02-18  Indu Bhagat  
David Faust  

gcc/testsuite/

* gcc.dg/debug/btf/btf-1.c: New test.
* gcc.dg/debug/btf/btf-2.c: Likewise.
* gcc.dg/debug/btf/btf-anonymous-struct-1.c: Likewise.
* gcc.dg/debug/btf/btf-anonymous-union-1.c: Likewise.
* gcc.dg/debug/btf/btf-array-1.c: Likewise.
* gcc.dg/debug/btf/btf-bitfields-1.c: Likewise.
* gcc.dg/debug/btf/btf-bitfields-2.c: Likewise.
* gcc.dg/debug/btf/btf-bitfields-3.c: Likewise.
* gcc.dg/debug/btf/btf-cvr-quals-1.c: Likewise.
* gcc.dg/debug/btf/btf-enum-1.c: Likewise.
* gcc.dg/debug/btf/btf-forward-1.c: Likewise.
* gcc.dg/debug/btf/btf-function-1.c: Likewise.
* gcc.dg/debug/btf/btf-function-2.c: Likewise.
* gcc.dg/debug/btf/btf-int-1.c: Likewise.
* gcc.dg/debug/btf/btf-pointers-1.c: Likewise.
* gcc.dg/debug/btf/btf-struct-1.c: Likewise.
* gcc.dg/debug/btf/btf-typedef-1.c: Likewise.
* gcc.dg/debug/btf/btf-union-1.c: Likewise.
* gcc.dg/debug/btf/btf-variables-1.c: Likewise.
* gcc.dg/debug/btf/btf.exp: Likewise.
* gcc.dg/debug/ctf/ctf-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-2.c: Likewise.
* gcc.dg/debug/ctf/ctf-anonymous-struct-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-anonymous-union-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-array-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-array-2.c: Likewise.
* gcc.dg/debug/ctf/ctf-array-3.c: Likewise.
* gcc.dg/debug/ctf/ctf-array-4.c: Likewise.
* gcc.dg/debug/ctf/ctf-attr-mode-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-attr-used-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-bitfields-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-bitfields-2.c: Likewise.
* gcc.dg/debug/ctf/ctf-bitfields-3.c: Likewise.
* gcc.dg/debug/ctf/ctf-bitfields-4.c: Likewise.
* gcc.dg/debug/ctf/ctf-complex-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-cvr-quals-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-cvr-quals-2.c: Likewise.
* gcc.dg/debug/ctf/ctf-cvr-quals-3.c: Likewise.
* gcc.dg/debug/ctf/ctf-cvr-quals-4.c: Likewise.
* gcc.dg/debug/ctf/ctf-enum-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-enum-2.c: Likewise.
* gcc.dg/debug/ctf/ctf-file-scope-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-float-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-forward-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-forward-2.c: Likewise.
* gcc.dg/debug/ctf/ctf-func-index-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-function-pointers-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-function-pointers-2.c: Likewise.
* gcc.dg/debug/ctf/ctf-function-pointers-3.c: Likewise.
* gcc.dg/debug/ctf/ctf-functions-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-int-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-objt-index-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-pointers-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-pointers-2.c: Likewise.
* gcc.dg/debug/ctf/ctf-preamble-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-skip-types-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-skip-types-2.c: Likewise.
* gcc.dg/debug/ctf/ctf-skip-types-3.c: Likewise.
* gcc.dg/debug/ctf/ctf-skip-types-4.c: Likewise.
* gcc.dg/debug/ctf/ctf-skip-types-5.c: Likewise.
* gcc.dg/debug/ctf/ctf-skip-types-6.c: Likewise.
* gcc.dg/debug/ctf/ctf-str-table-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-struct-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-struct-2.c: Likewise.
* gcc.dg/debug/ctf/ctf-struct-array-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-struct-pointer-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-struct-pointer-2.c: Likewise.
* gcc.dg/debug/ctf/ctf-typedef-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-typedef-2.c: Likewise.
* gcc.dg/debug/ctf/ctf-typedef-3.c: Likewise.
* gcc.dg/debug/ctf/ctf-typedef-struct-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-typedef-struct-2.c: Likewise.
* gcc.dg/debug/ctf/ctf-typedef-struct-3.c: Likewise.
* gcc.dg/debug/ctf/ctf-union-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-variables-1.c: Likewise.
* gcc.dg/debug/ctf/ctf-variables-2.c: Likewise.
* gcc.dg/debug/ctf/ctf.exp: Likewise.
* gcc.dg/debug/dwarf2-ctf-1.c: Likewise.
---
 gcc/testsuite/gcc.dg/debug/btf/btf-1.c|  6 ++
 gcc/testsuite/gcc.dg/debug/btf/btf-2.c| 10 +++
 .../gcc.dg/debug/btf/btf-anonymous-struct-1.c | 23 ++
 .../gcc.dg/debug/btf/btf-anonymous-union-1.c  | 23 ++
 gcc/testsuite/gcc.dg/debug/btf/btf-array-1.c  | 31 +++
 .../gcc.dg/debug/btf/btf-bitfields-1.c| 34 
 .../gcc.dg/debug/btf/btf-bitfields-2.c| 26 ++
 .../gcc.dg/debug/btf/btf-bitfields-3.c| 43 ++
 .../gcc.dg/debug/btf/btf-cvr-quals-1.c| 52 
 

[PATCH V3 5/5] Enable BTF generation in the BPF backend

[Jose E. Marchesi via Gcc-patches]

This patch changes the BPF GCC backend in order to use the DWARF debug
hooks and therefore enables the user to generate BTF debugging
information with -gbtf.  Generating BTF is crucial when compiling BPF
programs, since the CO-RE (compile-once, run-everwhere) mechanism
used by the kernel BPF loader relies on it.

Note that since in eBPF it is not possible to unwind frames due to the
restrictive nature of the target architecture, we are disabling the
generation of CFA in this target.

2021-01-22  David Faust 

* config/bpf/bpf.c (bpf_expand_prologue): Do not mark insns as
frame related.
(bpf_expand_epilogue): Likewise.
* config/bpf/bpf.h (DWARF2_FRAME_INFO): Define to 0.
Do not define DBX_DEBUGGING_INFO.
---
 gcc/config/bpf/bpf.c |  4 
 gcc/config/bpf/bpf.h | 12 ++--
 2 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/gcc/config/bpf/bpf.c b/gcc/config/bpf/bpf.c
index 126d4a2798d..e635f9edb40 100644
--- a/gcc/config/bpf/bpf.c
+++ b/gcc/config/bpf/bpf.c
@@ -349,7 +349,6 @@ bpf_expand_prologue (void)
  hard_frame_pointer_rtx,
  fp_offset - 8));
  insn = emit_move_insn (mem, gen_rtx_REG (DImode, regno));
- RTX_FRAME_RELATED_P (insn) = 1;
  fp_offset -= 8;
}
}
@@ -364,7 +363,6 @@ bpf_expand_prologue (void)
 {
   insn = emit_move_insn (stack_pointer_rtx,
 hard_frame_pointer_rtx);
-  RTX_FRAME_RELATED_P (insn) = 1;
 
   if (size > 0)
{
@@ -372,7 +370,6 @@ bpf_expand_prologue (void)
 gen_rtx_PLUS (Pmode,
   stack_pointer_rtx,
   GEN_INT (-size;
- RTX_FRAME_RELATED_P (insn) = 1;
}
 }
 }
@@ -412,7 +409,6 @@ bpf_expand_epilogue (void)
  hard_frame_pointer_rtx,
  fp_offset - 8));
  insn = emit_move_insn (gen_rtx_REG (DImode, regno), mem);
- RTX_FRAME_RELATED_P (insn) = 1;
  fp_offset -= 8;
}
}
diff --git a/gcc/config/bpf/bpf.h b/gcc/config/bpf/bpf.h
index 9e2f5260900..ef0123b56a6 100644
--- a/gcc/config/bpf/bpf.h
+++ b/gcc/config/bpf/bpf.h
@@ -235,17 +235,9 @@ enum reg_class
 
 / Debugging Info /
 
-/* We cannot support DWARF2 because of the limitations of eBPF.  */
+/* In eBPF it is not possible to unwind frames. Disable CFA.  */
 
-/* elfos.h insists in using DWARF.  Undo that here.  */
-#ifdef DWARF2_DEBUGGING_INFO
-# undef DWARF2_DEBUGGING_INFO
-#endif
-#ifdef PREFERRED_DEBUGGING_TYPE
-# undef PREFERRED_DEBUGGING_TYPE
-#endif
-
-#define DBX_DEBUGGING_INFO
+#define DWARF2_FRAME_INFO 0
 
 / Stack Layout and Calling Conventions.  */
 
-- 
2.25.0.2.g232378479e

[PATCH V3 4/5] CTF/BTF documentation

[Jose E. Marchesi via Gcc-patches]

This commit documents the new command line options introduced by the
CTF and BTF debug formats.

2021-02-18  Indu Bhagat  

* doc/invoke.texi: Document the CTF and BTF debug info options.
---
 gcc/doc/invoke.texi | 20 
 1 file changed, 20 insertions(+)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 4bd4f390ded..9bed12b4fd6 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -461,6 +461,7 @@ Objective-C and Objective-C++ Dialects}.
 @item Debugging Options
 @xref{Debugging Options,,Options for Debugging Your Program}.
 @gccoptlist{-g  -g@var{level}  -gdwarf  -gdwarf-@var{version} @gol
+-gbtf -gctf  -gctf@var{level} @gol
 -ggdb  -grecord-gcc-switches  -gno-record-gcc-switches @gol
 -gstabs  -gstabs+  -gstrict-dwarf  -gno-strict-dwarf @gol
 -gas-loc-support  -gno-as-loc-support @gol
@@ -9657,6 +9658,25 @@ other DWARF-related options such as
 @option{-fno-dwarf2-cfi-asm}) retain a reference to DWARF Version 2
 in their names, but apply to all currently-supported versions of DWARF.
 
+@item -gbtf
+@opindex gbtf
+Request BTF debug information.
+
+@item -gctf
+@itemx -gctf@var{level}
+@opindex gctf
+Request CTF debug information and use level to specify how much CTF debug
+information should be produced.  If -gctf is specified without a value for
+level, the default level of CTF debug information is 2.
+
+Level 0 produces no CTF debug information at all.  Thus, -gctf0 negates -gctf.
+
+Level 1 produces CTF information for tracebacks only.  This includes callsite
+information, but does not include type information.
+
+Level 2 produces type information for entities (functions, data objects etc.)
+at file-scope or global-scope only.
+
 @item -gstabs
 @opindex gstabs
 Produce debugging information in stabs format (if that is supported),
-- 
2.25.0.2.g232378479e

[PATCH V3 0/5] Support for the CTF and BTF debug formats

[Jose E. Marchesi via Gcc-patches]

[Changes from V2:
- Rebased to latest master.
- Several bug fixes in the CTF support.
- Several bug fixes in the BTF support.
- New tests for BTF.
- Fix a couple of tests in the CTF testsuite.
- More testing: we are building Gentoo with -gctf activated by
  default and fixing problems as we find them.  More than 160 packages
  built so far.]

Hi people!

Last year we submitted a first patch series introducing support for
the CTF debugging format in GCC [1].  We got a lot of feedback that
prompted us to change the approach used to generate the debug info,
and this patch series is the result of that.

This series also add support for the BTF debug format, which is needed
by the BPF backend (more on this below.)

This implementation works, but there are several points that need
discussion and agreement with the upstream community, as they impact
the way debugging options work.  We are also proposing a way to add
additional debugging formats in the future.  See below for more
details.

Finally, a patch makes the BPF GCC backend to use the DWARF debug
hooks in order to make -gbtf available to it.

[1] https://gcc.gnu.org/legacy-ml/gcc-patches/2019-05/msg01297.html

About CTF
=

CTF is a debugging format designed in order to express C types in a
very compact way.  The key is compactness and simplicity.  For more
information see:

- CTF specification
  http://www.esperi.org.uk/~oranix/ctf/ctf-spec.pdf

- Compact C-Type support in the GNU toolchain (talk + slides)
  https://linuxplumbersconf.org/event/4/contributions/396/

- On type de-duplication in CTF (talk + slides)
  https://linuxplumbersconf.org/event/7/contributions/725/

About BTF
=

BTF is a debugging format, similar to CTF, that is used in the Linux
kernel as the debugging format for BPF programs.  From the kernel
documentation:

"BTF (BPF Type Format) is the metadata format which encodes the debug
 info related to BPF program/map. The name BTF was used initially to
 describe data types. The BTF was later extended to include function
 info for defined subroutines, and line info for source/line
 information."

Supporting BTF in GCC is important because compiled BPF programs
(which GCC supports as a target) require the type information in order
to be loaded and run in diverse kernel versions.  This mechanism is
known as CO-RE (compile-once, run-everywhere) and is described in the
"Update of the BPF support in the GNU Toolchain" talk mentioned below.

The BTF is documented in the Linux kernel documentation tree:
- linux/Documentation/bpf/btf.rst

CTF in the GNU Toolchain


During the last year we have been working in adding support for CTF to
several components of the GNU toolchain:

- binutils support is already upstream.  It supports linking objects
  with CTF information with full type de-duplication.

- GDB support is to be sent upstream very shortly.  It makes the
  debugger capable to use the CTF information whenever available.
  This is useful in cases where DWARF has been stripped out but CTF is
  kept.

- GCC support is being discussed and submitted in this series.

Overview of the Implementation
==

  dwarf2out.c

The enabled debug formats are hooked in dwarf2out_early_finish.

  dwarf2ctf.c

Code that tranform the internal GCC DWARF DIEs into CTF container
structures.  This file is included in dwarf2out.c.

  ctfc.c
  ctfc.h

These two files implement the "CTF container", which is shared
among CTF and BTF, due to the many similarities between both
formats.

  ctfout.c

Code that emits assembler with the .ctf section data, from the CTF
container.

  btfout.c

Code that emits assembler with the .BTF section data, from the CTF
container.

>From debug hooks to debug formats
=

Our first attempt in adding CTF to GCC used the obvious approach of
adding a new set of debug hooks as defined in gcc/debug.h.

During our first interaction with the upstream community we were told
to _not_ use debug hooks, because these are to be obsoleted at some
point.  We were suggested to instead hook our handlers (which
processed type TREE nodes producing CTF types from them) somewhere
else.  So we did.

However at the time we were also facing the need to support BTF, which
is another type-related debug format needed by the BPF GCC backend.
Hooking here and there doesn't sound like such a good idea when it
comes to support several debug formats.

Therefore we thought about how to make GCC support diverse debugging
formats in a better way.  This led to a proposal we tried to discuss
at the GNU Tools Track in LPC2020:

- Update of the BPF support in the GNU Toolchain
  https://linuxplumbersconf.org/event/7/contributions/724/

Basically, the current situation in terms of diversity of debugging
formats in GCC can be summarized in the following like:

 tree --+  +--> dwarf2out
 rtl  --+  

[PATCH V3 1/5] Add new function lang_GNU_GIMPLE

[Jose E. Marchesi via Gcc-patches]

2021-02-18  Indu Bhagat  

* langhooks.c (lang_GNU_GIMPLE): New Function.
* langhooks.h: New Prototype.
---
 gcc/langhooks.c | 9 +
 gcc/langhooks.h | 1 +
 2 files changed, 10 insertions(+)

diff --git a/gcc/langhooks.c b/gcc/langhooks.c
index 2354386f7b4..0082ee9f350 100644
--- a/gcc/langhooks.c
+++ b/gcc/langhooks.c
@@ -929,3 +929,12 @@ lang_GNU_OBJC (void)
 {
   return strncmp (lang_hooks.name, "GNU Objective-C", 15) == 0;
 }
+
+/* Returns true if the current lang_hooks represents the GNU GIMPLE
+   frontend.  */
+
+bool
+lang_GNU_GIMPLE (void)
+{
+  return strncmp (lang_hooks.name, "GNU GIMPLE", 10) == 0;
+}
diff --git a/gcc/langhooks.h b/gcc/langhooks.h
index 842e605c439..f9c82eff0cb 100644
--- a/gcc/langhooks.h
+++ b/gcc/langhooks.h
@@ -638,5 +638,6 @@ extern bool lang_GNU_C (void);
 extern bool lang_GNU_CXX (void);
 extern bool lang_GNU_Fortran (void);
 extern bool lang_GNU_OBJC (void);
+extern bool lang_GNU_GIMPLE (void);
 
 #endif /* GCC_LANG_HOOKS_H */
-- 
2.25.0.2.g232378479e

Re: [PATCH 1/5] std::latch: reduce internal implementation from ptrdiff_t to int

[Richard Biener via Gcc-patches]

On Sun, Feb 28, 2021 at 10:53 PM Hans-Peter Nilsson  wrote:
>
>
>
> On Fri, 26 Feb 2021, Thiago Macieira via Gcc-patches wrote:
>
> > On Friday, 26 February 2021 11:31:00 PST Andreas Schwab wrote:
> > > On Feb 26 2021, Thiago Macieira wrote:
> > > > On Friday, 26 February 2021 10:14:42 PST Andreas Schwab wrote:
> > > >> On Feb 26 2021, Thiago Macieira via Gcc-patches wrote:
> > > >> > -alignas(__alignof__(ptrdiff_t)) ptrdiff_t _M_a;
> > > >> > +alignas(__alignof__(int)) int _M_a;
> > > >>
> > > >> Futexes must be aligned to 4 bytes.
> > > >
> > > > Agreed, but doesn't this accomplish that?
> > >
> > > No.  It uses whatever alignment the type already has, and is an
> > > elaborate no-op.
> >
> > I thought so too when I read the original line. But I expected it was 
> > written
> > like that for a reason, especially since the same pattern appears in other
> > places.
> >
> > I can change to "alignas(4)" (which is a GCC extension, I believe). Is that
> > the correct solution?
>
> IMNSHO make use of the corresponding atomic type.  Then there'd
> be no need for separate what's-the-right-align-curse games.

Or align as the corresponding atomic type (in case using an actual
std::atomic is undesirable).  OTOH the proposed code isn't
any more bogus than the previous ...

Richard.

> brgds, H-P

Re: add -mpowerpc-gpopt to options for sqrt insn on PowerPC

[Richard Biener via Gcc-patches]

On Sat, Feb 27, 2021 at 2:43 AM Segher Boessenkool
 wrote:
>
> On Fri, Feb 26, 2021 at 12:31:16PM -0500, David Edelsohn wrote:
> > On Fri, Feb 26, 2021 at 11:09 AM Alexandre Oliva  wrote:
> > >
> > > This patch avoids an ICE in gimplefe-28.c, in our ppc64-vxworks7r2
> > > tests.  Tested on x86_64-linux-gnu, and on the affected platform.  Ok to
> > > install?
> >
> > I'm sort of surprised that sqrt instruction would be available for the
> > target but not enabled by default.  Is this really the method that
> > customers would use?  It seems that it might be more appropriate to
> > xfail or skip the test for PPC64 VxWorks.
>
> You should essentially never use -mpowerpc-gpopt, but instead use a
> -mcpu= that has it.  You also of course whould not do that in run tests
> if the cpu does not support those insns.
>
> But, Alex, you say this avoids an ICE?  You shouldn't avoid the ICE in
> that testcase then -- instead, fix it!  (Or report it).

The testcase uses a .SQRT direct-internal function and guards itself with

/* { dg-do compile { target sqrt_insn } } */
/* { dg-options "-fgimple -O2" } */
/* { dg-add-options sqrt_insn } */

if the power dg setup of sqrt_insn doesn't support this it should be adjusted.
Using -mcpu=X for sqrt_insn is likely undesirable but for this testcase would
work (it would make testing with, say, -mcpu=power4 not work).   So I'd
say as alternative to Alex patch you could adjust

# Return 1 if the target supports hardware square root instructions.

proc check_effective_target_sqrt_insn { } {
return [check_cached_effective_target sqrt_insn {
  expr { [istarget i?86-*-*] || [istarget x86_64-*-*]
 || [istarget powerpc*-*-*]
 || [istarget aarch64*-*-*]
 || ([istarget arm*-*-*] && [check_effective_target_arm_vfp_ok])
 || ([istarget s390*-*-*]
 && [check_effective_target_s390_vx])
 || [istarget amdgcn-*-*] }}]
}

to only say yes in case the selected CPU supports sqrt.

Richard.

>
> Segher

[PATCH] middle-end/97855 - fix diagnostic with default pretty printer

[Richard Biener]

The default diagnostic tree printer relies on dump_generic_node which
for some reason manages to clobber the diagnostic pretty-printer state
so we see garbled diagnostics like

/home/rguenther/src/trunk/gcc/calls.c: In function 'expand_call':
D.6750.coeffs[0]'/home/rguenther/src/trunk/gcc/dojump.c:118:28: warning:  may 
be used uninitialized in this function [-Wmaybe-uninitialized]

when the diagnostic is emitted by the LTO fronted.  The following
approach using a temporary pretty-printer for the dump_generic_node
call fixes this for some unknown reason and we issue

/home/rguenther/src/trunk/gcc/calls.c: In function 'expand_call':
/home/rguenther/src/trunk/gcc/dojump.c:118:28: warning: 'MEM[(struct poly_int 
*)].D.6750.coeffs[0]' may be used uninitialized in this function 
[-Wmaybe-uninitialized]

[LTO] Bootstrapped and tested on x86_64-unknown-linux-gnu, OK for trunk?

Thanks,
Richard.

2021-02-26  Richard Biener  

PR middle-end/97855
* tree-diagnostic.c (default_tree_printer): Use a temporary
pretty-printer when formatting a tree via dump_generic_node.
---
 gcc/tree-diagnostic.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gcc/tree-diagnostic.c b/gcc/tree-diagnostic.c
index 95b8ef30070..f124e627aad 100644
--- a/gcc/tree-diagnostic.c
+++ b/gcc/tree-diagnostic.c
@@ -300,7 +300,11 @@ default_tree_printer (pretty_printer *pp, text_info *text, 
const char *spec,
   pp_string (pp, n);
 }
   else
-dump_generic_node (pp, t, 0, TDF_SLIM, 0);
+{
+  pretty_printer buffer;
+  dump_generic_node (, t, 0, TDF_SLIM, 0);
+  pp_string (pp, pp_formatted_text ());
+}
 
   return true;
 }
-- 
2.26.2

Re: [PATCH] dwarf2unwind : Force the CFA after remember/restore pairs [44107/48097].

[Richard Biener via Gcc-patches]

On Fri, Feb 26, 2021 at 11:04 PM Iain Sandoe  wrote:
>
> Hi
>
> This address one of the more long-standing and serious regressions
> for Darwin.  GCC emits unwind code by default on the assumption that
> the unwinder will be (or have the same capability) as the one in the
> current libgcc_s.  For Darwin platforms, this is not the case - some
> of them are based on the libgcc_s from GCC-4.2.1 and some are using
> the unwinder provided by libunwind (part of the LLVM project).  The
> latter implementation has gradually adopted a section that deals with
> GNU unwind.
>
> The most serious problem for some of the platform versions is in
> handling DW_CFA_remember/restore_state pairs.  The DWARF description
> talks about these in terms of saving/restoring register rows; this is
> what GCC originally did (and is what the unwinders do for the Darwin
> versions based on libgcc_s).
>
> However, in r118068, this was changed so that not only the registers
> but also the current frame address expression were saved.  The unwind
> code-gen assumes that the unwinder will do this; some of Darwin's unwinders
> do not, leading to lockups etc.  To date, the only solution has been to 
> replace
> the system libgcc_s with a newer one which is not a usable solution for many
> end-users (since that means overwritting the one provided with the system
> installation).
>
> The fix here provides a target hook that allows the target to specify
> that the CFA expression should be reinstated after a DW_CFA_restore.  This
> fixes the open PR (and also the closed WONTFIX of 44107).
>
> (As a matter of record, it also fixes reported Java issues if
>  backported to GCC-5).
>
> tested on *-darwin* and x86_64-linux-gnu,
> OK for master / backports to open branches?

OK for master and branches after a while.

Thanks,
Richard.

> thanks
> Iain
>
> gcc/ChangeLog:
>
> PR target/44107
> PR target/48097
> * config/darwin-protos.h (darwin_should_restore_cfa_state): New.
> * config/darwin.c (darwin_should_restore_cfa_state): New.
> * config/darwin.h (TARGET_ASM_SHOULD_RESTORE_CFA_STATE): New.
> * doc/tm.texi: Regenerated.
> * doc/tm.texi.in: Document TARGET_ASM_SHOULD_RESTORE_CFA_STATE.
> * dwarf2cfi.c (connect_traces): If the target requests, restore
> the CFA expression after a DW_CFA_restore.
> * target.def (TARGET_ASM_SHOULD_RESTORE_CFA_STATE): New hook.
> ---
>  gcc/config/darwin-protos.h |  1 +
>  gcc/config/darwin.c| 10 ++
>  gcc/config/darwin.h|  5 +
>  gcc/doc/tm.texi|  4 
>  gcc/doc/tm.texi.in |  2 ++
>  gcc/dwarf2cfi.c|  6 ++
>  gcc/target.def | 14 ++
>  7 files changed, 42 insertions(+)
>
> diff --git a/gcc/config/darwin-protos.h b/gcc/config/darwin-protos.h
> index 2120eb62c56..f5ef82456aa 100644
> --- a/gcc/config/darwin-protos.h
> +++ b/gcc/config/darwin-protos.h
> @@ -70,6 +70,7 @@ extern void darwin_non_lazy_pcrel (FILE *, rtx);
>  extern void darwin_emit_unwind_label (FILE *, tree, int, int);
>  extern void darwin_emit_except_table_label (FILE *);
>  extern rtx darwin_make_eh_symbol_indirect (rtx, bool);
> +extern bool darwin_should_restore_cfa_state (void);
>
>  extern void darwin_pragma_ignore (struct cpp_reader *);
>  extern void darwin_pragma_options (struct cpp_reader *);
> diff --git a/gcc/config/darwin.c b/gcc/config/darwin.c
> index 119f3195b63..e2e60bbf1b2 100644
> --- a/gcc/config/darwin.c
> +++ b/gcc/config/darwin.c
> @@ -2236,6 +2236,16 @@ darwin_make_eh_symbol_indirect (rtx orig, bool 
> ARG_UNUSED (pubvis))
> /*stub_p=*/false));
>  }
>
> +/* The unwinders in earlier Darwin versions are based on an old version
> +   of libgcc_s and need current frame address stateto be reset after a
> +   DW_CFA_restore_state recovers the register values.  */
> +
> +bool
> +darwin_should_restore_cfa_state (void)
> +{
> +  return generating_for_darwin_version <= 10;
> +}
> +
>  /* Return, and mark as used, the name of the stub for the mcount function.
> Currently, this is only called by X86 code in the expansion of the
> FUNCTION_PROFILER macro, when stubs are enabled.  */
> diff --git a/gcc/config/darwin.h b/gcc/config/darwin.h
> index 5a9fb43f3c6..d2b2c141c8e 100644
> --- a/gcc/config/darwin.h
> +++ b/gcc/config/darwin.h
> @@ -614,6 +614,11 @@ extern GTY(()) int darwin_ms_struct;
>  /* Make an EH (personality or LDSA) symbol indirect as needed.  */
>  #define TARGET_ASM_MAKE_EH_SYMBOL_INDIRECT darwin_make_eh_symbol_indirect
>
> +/* Some of Darwin's unwinders need current frame address state to be reset
> +   after a DW_CFA_restore_state recovers the register values.  */
> +#undef TARGET_ASM_SHOULD_RESTORE_CFA_STATE
> +#define TARGET_ASM_SHOULD_RESTORE_CFA_STATE darwin_should_restore_cfa_state
> +
>  /* Our profiling scheme doesn't LP labels and counter words.  */
>
>  #define NO_PROFILE_COUNTERS1
> 

Re: [PATCH] RISC-V: Implment __builtin_thread_pointer

[Matthias Klose]

On 7/8/20 9:59 PM, Jim Wilson wrote:
> On Tue, Jul 7, 2020 at 2:52 AM Kito Cheng  wrote:
>> gcc/ChangeLog:
>> * gcc/config/riscv/riscv.md (): New.
>> (TP_REGNUM): Ditto.
>> * doc/extend.texi (Target Builtins): Add RISC-V built-in section.
>> Document __builtin_thread_pointer.
>> gcc/testsuite/ChangeLog:
>> * gcc.target/riscv/read-thread-pointer.c: New.
> 
> It looks OK to me in general.
> 
> You added builtin_thread_pointer but not builtin_set_thread_pointer.
> Maybe we should implement both as long as we are implementing one?  If
> clang only implements one, maybe it should implement the other also?
> This doesn't have to be part of this patch.  This could be a separate
> issue.
> 
> The builtin_thread_pointer docs looks out-of-date.  It is documented
> for alpha and SH, but it is implemented in gcc/builtins.c not in the
> backends.  A scan of md files show that quite a few targets support it
> but don't document it.  I think it should be documented in the generic
> builtins section not in the target dependent builtins sections with
> some language that says not all targets support it.  This doesn't have
> to be part of this patch.  This could be a separate issue.
> 
> We have two existing undocumented builtins.  __builtin_riscv_fsflags
> and __builtin_riscv_frflags for setting or reading the FP flags.  I
> don't know if anyone uses them though.  newlib and glbic both use
> extended asms for these operations.  This doesn't have to be part of
> this patch.  This could be a separate issue.
> 
> There is a document https://github.com/riscv/riscv-c-api-doc for
> coordinating gcc and llvm work that has an empty list of builtin
> functions.  I'm not sure if this document is still useful.  If this is
> a RISC-V specific builtin then it should be listed here, but I don't
> think it should be considered a RISC-V specific builtin.  There is an
> unresolved pull request for the frflags and fsflags builtins.  I guess
> I forgot about that.
> 
> Jim

LLVM 12 now uses __builtin_thread_pointer, and fails to build with GCC 10 (but
builds with GCC 11).  Could you consider backporting this one to GCC 10?

Thanks, Matthias