Re: [PATCH] LoongArch: Support loading floating-point zero into MEM[base + index].

[Xi Ruoyao via Gcc-patches]

LGTM.

Nit: it should be "storing" floating-point zero into MEM, not "loading".

On Sat, 2023-09-02 at 12:47 +0800, Guo Jie wrote:
> gcc/ChangeLog:
> 
> * config/loongarch/loongarch.md: Support 'G' -> 'k' in
> movsf_hardfloat and movdf_hardfloat.
> 
> gcc/testsuite/ChangeLog:
> 
> * gcc.target/loongarch/const-double-zero-stx.c: New test.
> 
> ---
>  gcc/config/loongarch/loongarch.md  | 12 ++--
>  .../loongarch/const-double-zero-stx.c  | 18 ++
>  2 files changed, 24 insertions(+), 6 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/loongarch/const-double-zero-stx.c
> 
> diff --git a/gcc/config/loongarch/loongarch.md 
> b/gcc/config/loongarch/loongarch.md
> index b37e070660f..6f47c23a79c 100644
> --- a/gcc/config/loongarch/loongarch.md
> +++ b/gcc/config/loongarch/loongarch.md
> @@ -1915,13 +1915,13 @@ (define_expand "movsf"
>  })
>  
>  (define_insn "*movsf_hardfloat"
> -  [(set (match_operand:SF 0 "nonimmediate_operand" 
> "=f,f,f,m,f,k,m,*f,*r,*r,*r,*m")
> -   (match_operand:SF 1 "move_operand" "f,G,m,f,k,f,G,*r,*f,*G*r,*m,*r"))]
> +  [(set (match_operand:SF 0 "nonimmediate_operand" 
> "=f,f,f,m,f,k,m,k,*f,*r,*r,*r,*m")
> +   (match_operand:SF 1 "move_operand" 
> "f,G,m,f,k,f,G,G,*r,*f,*G*r,*m,*r"))]
>    "TARGET_HARD_FLOAT
>     && (register_operand (operands[0], SFmode)
>     || reg_or_0_operand (operands[1], SFmode))"
>    { return loongarch_output_move (operands[0], operands[1]); }
> -  [(set_attr "move_type" 
> "fmove,mgtf,fpload,fpstore,fpload,fpstore,store,mgtf,mftg,move,load,store")
> +  [(set_attr "move_type" 
> "fmove,mgtf,fpload,fpstore,fpload,fpstore,store,store,mgtf,mftg,move,load,store")
>     (set_attr "mode" "SF")])
>  
>  (define_insn "*movsf_softfloat"
> @@ -1946,13 +1946,13 @@ (define_expand "movdf"
>  })
>  
>  (define_insn "*movdf_hardfloat"
> -  [(set (match_operand:DF 0 "nonimmediate_operand" 
> "=f,f,f,m,f,k,m,*f,*r,*r,*r,*m")
> -   (match_operand:DF 1 "move_operand" "f,G,m,f,k,f,G,*r,*f,*r*G,*m,*r"))]
> +  [(set (match_operand:DF 0 "nonimmediate_operand" 
> "=f,f,f,m,f,k,m,k,*f,*r,*r,*r,*m")
> +   (match_operand:DF 1 "move_operand" 
> "f,G,m,f,k,f,G,G,*r,*f,*r*G,*m,*r"))]
>    "TARGET_DOUBLE_FLOAT
>     && (register_operand (operands[0], DFmode)
>     || reg_or_0_operand (operands[1], DFmode))"
>    { return loongarch_output_move (operands[0], operands[1]); }
> -  [(set_attr "move_type" 
> "fmove,mgtf,fpload,fpstore,fpload,fpstore,store,mgtf,mftg,move,load,store")
> +  [(set_attr "move_type" 
> "fmove,mgtf,fpload,fpstore,fpload,fpstore,store,store,mgtf,mftg,move,load,store")
>     (set_attr "mode" "DF")])
>  
>  (define_insn "*movdf_softfloat"
> diff --git a/gcc/testsuite/gcc.target/loongarch/const-double-zero-stx.c 
> b/gcc/testsuite/gcc.target/loongarch/const-double-zero-stx.c
> new file mode 100644
> index 000..8fb04be8ff5
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/loongarch/const-double-zero-stx.c
> @@ -0,0 +1,18 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +/* { dg-final { scan-assembler-times {stx\..\t\$r0} 2 } } */
> +
> +extern float arr_f[];
> +extern double arr_d[];
> +
> +void
> +test_f (int base, int index)
> +{
> +  arr_f[base + index] = 0.0;
> +}
> +
> +void
> +test_d (int base, int index)
> +{
> +  arr_d[base + index] = 0.0;
> +}

-- 
Xi Ruoyao 
School of Aerospace Science and Technology, Xidian University

[PATCH] LoongArch: Support loading floating-point zero into MEM[base + index].

[Guo Jie]

gcc/ChangeLog:

* config/loongarch/loongarch.md: Support 'G' -> 'k' in
movsf_hardfloat and movdf_hardfloat.

gcc/testsuite/ChangeLog:

* gcc.target/loongarch/const-double-zero-stx.c: New test.

---
 gcc/config/loongarch/loongarch.md  | 12 ++--
 .../loongarch/const-double-zero-stx.c  | 18 ++
 2 files changed, 24 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/loongarch/const-double-zero-stx.c

diff --git a/gcc/config/loongarch/loongarch.md 
b/gcc/config/loongarch/loongarch.md
index b37e070660f..6f47c23a79c 100644
--- a/gcc/config/loongarch/loongarch.md
+++ b/gcc/config/loongarch/loongarch.md
@@ -1915,13 +1915,13 @@ (define_expand "movsf"
 })
 
 (define_insn "*movsf_hardfloat"
-  [(set (match_operand:SF 0 "nonimmediate_operand" 
"=f,f,f,m,f,k,m,*f,*r,*r,*r,*m")
-   (match_operand:SF 1 "move_operand" "f,G,m,f,k,f,G,*r,*f,*G*r,*m,*r"))]
+  [(set (match_operand:SF 0 "nonimmediate_operand" 
"=f,f,f,m,f,k,m,k,*f,*r,*r,*r,*m")
+   (match_operand:SF 1 "move_operand" "f,G,m,f,k,f,G,G,*r,*f,*G*r,*m,*r"))]
   "TARGET_HARD_FLOAT
&& (register_operand (operands[0], SFmode)
|| reg_or_0_operand (operands[1], SFmode))"
   { return loongarch_output_move (operands[0], operands[1]); }
-  [(set_attr "move_type" 
"fmove,mgtf,fpload,fpstore,fpload,fpstore,store,mgtf,mftg,move,load,store")
+  [(set_attr "move_type" 
"fmove,mgtf,fpload,fpstore,fpload,fpstore,store,store,mgtf,mftg,move,load,store")
(set_attr "mode" "SF")])
 
 (define_insn "*movsf_softfloat"
@@ -1946,13 +1946,13 @@ (define_expand "movdf"
 })
 
 (define_insn "*movdf_hardfloat"
-  [(set (match_operand:DF 0 "nonimmediate_operand" 
"=f,f,f,m,f,k,m,*f,*r,*r,*r,*m")
-   (match_operand:DF 1 "move_operand" "f,G,m,f,k,f,G,*r,*f,*r*G,*m,*r"))]
+  [(set (match_operand:DF 0 "nonimmediate_operand" 
"=f,f,f,m,f,k,m,k,*f,*r,*r,*r,*m")
+   (match_operand:DF 1 "move_operand" "f,G,m,f,k,f,G,G,*r,*f,*r*G,*m,*r"))]
   "TARGET_DOUBLE_FLOAT
&& (register_operand (operands[0], DFmode)
|| reg_or_0_operand (operands[1], DFmode))"
   { return loongarch_output_move (operands[0], operands[1]); }
-  [(set_attr "move_type" 
"fmove,mgtf,fpload,fpstore,fpload,fpstore,store,mgtf,mftg,move,load,store")
+  [(set_attr "move_type" 
"fmove,mgtf,fpload,fpstore,fpload,fpstore,store,store,mgtf,mftg,move,load,store")
(set_attr "mode" "DF")])
 
 (define_insn "*movdf_softfloat"
diff --git a/gcc/testsuite/gcc.target/loongarch/const-double-zero-stx.c 
b/gcc/testsuite/gcc.target/loongarch/const-double-zero-stx.c
new file mode 100644
index 000..8fb04be8ff5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/loongarch/const-double-zero-stx.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+/* { dg-final { scan-assembler-times {stx\..\t\$r0} 2 } } */
+
+extern float arr_f[];
+extern double arr_d[];
+
+void
+test_f (int base, int index)
+{
+  arr_f[base + index] = 0.0;
+}
+
+void
+test_d (int base, int index)
+{
+  arr_d[base + index] = 0.0;
+}
-- 
2.20.1

[Bug tree-optimization/111268] [14 Regression] internal compiler error: in to_constant, at poly-int.h:504

[pinskia at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111268

Andrew Pinski  changed:

   What|Removed |Added

   Keywords||ice-on-valid-code,
   ||needs-bisection
  Component|target  |tree-optimization
   Target Milestone|--- |14.0
Summary|internal compiler error: in |[14 Regression] internal
   |to_constant, at |compiler error: in
   |poly-int.h:504  |to_constant, at
   ||poly-int.h:504

--- Comment #4 from Andrew Pinski  ---
Looks like this one is already fixed.

[Bug middle-end/110989] RISC-V vector ICE due to invalid tree code in GIMPLE vect pass

[pinskia at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110989

Andrew Pinski  changed:

   What|Removed |Added

   Keywords||ice-on-valid-code
   Target Milestone|--- |14.0

[Bug tree-optimization/110817] [14 Regression] wrong code with vector compares and vector lowering

[pinskia at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110817

Andrew Pinski  changed:

   What|Removed |Added

URL||https://gcc.gnu.org/piperma
   ||il/gcc-patches/2023-Septemb
   ||er/629152.html
   Keywords|needs-bisection |patch

--- Comment #20 from Andrew Pinski  ---
Patch posted:
https://gcc.gnu.org/pipermail/gcc-patches/2023-September/629152.html

[PATCH] ssa_name_has_boolean_range vs signed-boolean:31 types

[Andrew Pinski via Gcc-patches]

This turns out to be a latent bug in ssa_name_has_boolean_range
where it would return true for all boolean types but all of the
uses of ssa_name_has_boolean_range was expecting 0/1 as the range
rather than [-1,0].
So when I fixed vector lower to do all comparisons in boolean_type
rather than still in the signed-boolean:31 type (to fix a different issue),
the pattern in match for `-(type)!A -> (type)A - 1.` would assume A (which
was signed-boolean:31) had a range of [0,1] which broke down and sometimes
gave us -1/-2 as values rather than what we were expecting of -1/0.

This was the simpliest patch I found while testing.

We have another way of matching [0,1] range which we could use instead
of ssa_name_has_boolean_range except that uses only the global ranges
rather than the local range (during VRP).
I tried to clean this up slightly by using gimple_match_zero_one_valuedp
inside ssa_name_has_boolean_range but that failed because due to using
only the global ranges. I then tried to change get_nonzero_bits to use
the local ranges at the optimization time but that failed also because
we would remove branches to __builtin_unreachable during evrp and lose
information as we don't set the global ranges during evrp.

OK? Bootstrapped and tested on x86_64-linux-gnu.

PR 110817

gcc/ChangeLog:

* tree-ssanames.cc (ssa_name_has_boolean_range): Remove the
check for boolean type as they don't have "[0,1]" range.

gcc/testsuite/ChangeLog:

* gcc.c-torture/execute/pr110817-1.c: New test.
* gcc.c-torture/execute/pr110817-2.c: New test.
* gcc.c-torture/execute/pr110817-3.c: New test.
---
 gcc/testsuite/gcc.c-torture/execute/pr110817-1.c | 13 +
 gcc/testsuite/gcc.c-torture/execute/pr110817-2.c | 16 
 gcc/testsuite/gcc.c-torture/execute/pr110817-3.c | 14 ++
 gcc/tree-ssanames.cc |  4 
 4 files changed, 43 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr110817-1.c
 create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr110817-2.c
 create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr110817-3.c

diff --git a/gcc/testsuite/gcc.c-torture/execute/pr110817-1.c 
b/gcc/testsuite/gcc.c-torture/execute/pr110817-1.c
new file mode 100644
index 000..1d33fa9a207
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr110817-1.c
@@ -0,0 +1,13 @@
+typedef unsigned long __attribute__((__vector_size__ (8))) V;
+
+
+V c;
+
+int
+main (void)
+{
+  V v = ~((V) { } <=0);
+  if (v[0])
+__builtin_abort ();
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr110817-2.c 
b/gcc/testsuite/gcc.c-torture/execute/pr110817-2.c
new file mode 100644
index 000..1f759178425
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr110817-2.c
@@ -0,0 +1,16 @@
+
+typedef unsigned char u8;
+typedef unsigned __attribute__((__vector_size__ (8))) V;
+
+V v;
+unsigned char c;
+
+int
+main (void)
+{
+  V x = (v > 0) > (v != c);
+ // V x = foo ();
+  if (x[0] || x[1])
+__builtin_abort ();
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr110817-3.c 
b/gcc/testsuite/gcc.c-torture/execute/pr110817-3.c
new file mode 100644
index 000..36f09c88dd9
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr110817-3.c
@@ -0,0 +1,14 @@
+typedef unsigned __attribute__((__vector_size__ (1*sizeof(unsigned V;
+
+V v;
+unsigned char c;
+
+int
+main (void)
+{
+  V x = (v > 0) > (v != c);
+  volatile signed int t = x[0];
+  if (t)
+__builtin_abort ();
+  return 0;
+}
diff --git a/gcc/tree-ssanames.cc b/gcc/tree-ssanames.cc
index 23387b90fe3..6c362995c1a 100644
--- a/gcc/tree-ssanames.cc
+++ b/gcc/tree-ssanames.cc
@@ -521,10 +521,6 @@ ssa_name_has_boolean_range (tree op)
 {
   gcc_assert (TREE_CODE (op) == SSA_NAME);
 
-  /* Boolean types always have a range [0..1].  */
-  if (TREE_CODE (TREE_TYPE (op)) == BOOLEAN_TYPE)
-return true;
-
   /* An integral type with a single bit of precision.  */
   if (INTEGRAL_TYPE_P (TREE_TYPE (op))
   && TYPE_UNSIGNED (TREE_TYPE (op))
-- 
2.31.1

Re: [PATCH v6] LoongArch:Implement 128-bit floating point functions in gcc.

[chenglulu]

Hi,RuoYao:

 I have merged the V6 patch into trunk(r14-3635). If the generic 
optimization of copysignf128 cannot be solved,


 we will mention the optimization code under the architecture again.

Thanks!


在 2023/9/1 上午11:22, chenxiaolong 写道:

Brief version history of patch set:

v1 -> v2:
According to the GNU code specification, adjust the format of the
function implementation with "q" as the suffix function.

v2 - >v3:

1.On the LoongArch architecture, refer to the functionality of 64-bit
functions and modify the underlying implementation of __builtin_{nanq, nansq}
functions in libgcc.

2.Modify the function's instruction template to use some instructions such
as "bstrins.d" to implement the 128-bit __builtin_{fabsq, copysignq} function
instead of calling libgcc library support, so as to better play the machine's
performance.

v3 -> v4:

1.The above v1,v2, and v3 all implement 128-bit floating-point functions
with "q" as the suffix, but it is an older implementation. The v4 version
completely abandoned the old implementation by associating the 128-bit
floating-point function with the "q" suffix with the "f128" function that
already existed in GCC.

2.Modify the code so that both "__float128" and "_Float128" function types
can be supported in compiler gcc.

3.Associating a function with the suffix "q" to the "f128" function allows
two different forms of the function to produce the same effect, For example,
__builtin_{huge_{valq/valf128},{infq/inff128},{nanq/nanf128},{nansq/nansf128},
{fabsq/fabsf128}}.

4.For the _builtin_copysignq  function, do not call the new "f128"
implementation, but use the "bstrins" and other instructions in the machine
description file to implement the function function, the result is that the
number of assembly instructions can be reduced and the function optimization
to achieve the optimal effect.

v4 -> v5:

Removed the v4 implementation of the __builtin_fabsf128() function added
to LoongArch.md.

v5 -> v6:

1.Modify the test cases in the math-float-128.c file.

2.Removed the v5 implementation of the __builtin_copysignf128() function
added to LoongArch.md.

During implementation, float128_type_node is bound with the type "__float128"
so that the compiler can correctly identify the type   of the function. The
"q" suffix is associated with the "f128" function, which makes GCC more
flexible to support different user input cases, implementing functions such
as __builtin_{huge_valq, infq, fabsq, copysignq, nanq, nansq}.

gcc/ChangeLog:

* config/loongarch/loongarch-builtins.cc (loongarch_init_builtins):
Associate the __float128 type to float128_type_node so that it can
be recognized by the compiler.
* config/loongarch/loongarch-c.cc (loongarch_cpu_cpp_builtins):
Add the flag "FLOAT128_TYPE" to gcc and associate a function
 with the suffix "q" to "f128".
* doc/extend.texi:Added support for 128-bit floating-point functions on
the LoongArch architecture.

gcc/testsuite/ChangeLog:

* gcc.target/loongarch/math-float-128.c: New test.
---
  gcc/config/loongarch/loongarch-builtins.cc|  5 ++
  gcc/config/loongarch/loongarch-c.cc   | 11 +++
  gcc/doc/extend.texi   | 20 -
  .../gcc.target/loongarch/math-float-128.c | 81 +++
  4 files changed, 114 insertions(+), 3 deletions(-)
  create mode 100644 gcc/testsuite/gcc.target/loongarch/math-float-128.c

diff --git a/gcc/config/loongarch/loongarch-builtins.cc 
b/gcc/config/loongarch/loongarch-builtins.cc
index b929f224dfa..58b612bf445 100644
--- a/gcc/config/loongarch/loongarch-builtins.cc
+++ b/gcc/config/loongarch/loongarch-builtins.cc
@@ -256,6 +256,11 @@ loongarch_init_builtins (void)
unsigned int i;
tree type;
  
+  /* Register the type float128_type_node as a built-in type and

+ give it an alias "__float128".  */
+  (*lang_hooks.types.register_builtin_type) (float128_type_node,
+   "__float128");
+
/* Iterate through all of the bdesc arrays, initializing all of the
   builtin functions.  */
for (i = 0; i < ARRAY_SIZE (loongarch_builtins); i++)
diff --git a/gcc/config/loongarch/loongarch-c.cc 
b/gcc/config/loongarch/loongarch-c.cc
index 67911b78f28..6ffbf748316 100644
--- a/gcc/config/loongarch/loongarch-c.cc
+++ b/gcc/config/loongarch/loongarch-c.cc
@@ -99,6 +99,17 @@ loongarch_cpu_cpp_builtins (cpp_reader *pfile)
else
  builtin_define ("__loongarch_frlen=0");
  
+  /* Add support for FLOAT128_TYPE on the LoongArch architecture.  */

+  builtin_define ("__FLOAT128_TYPE__");
+
+  /* Map the old _Float128 'q' builtins into the new 'f128' builtins.  */
+  builtin_define ("__builtin_fabsq=__builtin_fabsf128");
+  builtin_define ("__builtin_copysignq=__builtin_copysignf128");
+  builtin_define ("__builtin_nanq=__builtin_nanf128");
+  builtin_define ("__builtin_nansq=__builtin_nansf128");
+  builtin_define 

[Bug tree-optimization/110817] [14 Regression] wrong code with vector compares and vector lowering

[pinskia at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110817

--- Comment #19 from Andrew Pinski  ---
(In reply to Andrew Pinski from comment #14)
> So I suspecting it is this pattern:
> /* -(type)!A -> (type)A - 1.  */
> (simplify
>  (negate (convert?:s (logical_inverted_value:s @0)))
>  (if (INTEGRAL_TYPE_P (type)
>   && TREE_CODE (type) != BOOLEAN_TYPE
>   && TYPE_PRECISION (type) > 1
>   && TREE_CODE (@0) == SSA_NAME
>   && ssa_name_has_boolean_range (@0))
>   (plus (convert:type @0) { build_all_ones_cst (type); })))
> 
> We start out with:
> -(unsigned int)(bool:31 == 0)
> 
> Yes bool:31 == 0 will have 0/1 but bool:31 does not.
> 
> Even more:
> bool
> ssa_name_has_boolean_range (tree op)
> {
>   gcc_assert (TREE_CODE (op) == SSA_NAME);
> 
>   /* Boolean types always have a range [0..1].  */
>   if (TREE_CODE (TREE_TYPE (op)) == BOOLEAN_TYPE)
> return true;


In the end I decided to just delete the above check.

[Bug c/111269] location for non-constant expressions inside static assert could be better

[alx at kernel dot org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111269

--- Comment #3 from Alejandro Colomar  ---
On 2023-09-01 18:57, pinskia at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111269
>
> Andrew Pinski  changed:
>
>What|Removed |Added
> 
>See Also||https://gcc.gnu.org/bugzill
>||a/show_bug.cgi?id=55678
>

Did you accidentally point to a different bug?
I don't think that one is related at all.

_Static_assert escapes tick marks just to make me mad

[Bug c/111269] location for non-constant expressions inside static assert could be better

[alx at kernel dot org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111269

--- Comment #2 from Alejandro Colomar  ---
Hi Andrew,

On 2023-09-01 18:55, pinskia at gcc dot gnu.org wrote:
> It is pointing to the whole expression and just the outermost operator, || .

That's what I suspected.

>
> Now the C++ front-end gives a better location and information on why it is not
> a constant expression:
> : In function 'int main()':
> :11:26: error: non-constant condition for static assertion
>11 | _Static_assert(0 || 7 > x, "");
>   |~~^~~~
> :11:33: error: the value of 'x' is not usable in a constant expression
>11 | _Static_assert(0 || 7 > x, "");
>   | ^
> :9:13: note: 'int x' is not const
> 9 | int x = 42;
>   | ^
>
>
> The C front-end could do better ...

Yep, g++ seems good here.  If it's something easy that I could do, I'd
love some pointer to the code I should look at to do the same that the
C++ front end does.

Cheers,
Alex

Re: [PATCH] c++: improve verify_constant diagnostic [PR91483]

[Marek Polacek via Gcc-patches]

On Fri, Sep 01, 2023 at 08:00:01PM -0400, Marek Polacek via Gcc-patches wrote:
>if (TREE_OVERFLOW_P (t))
> diff --git a/gcc/testsuite/g++.dg/diagnostic/constexpr3.C 
> b/gcc/testsuite/g++.dg/diagnostic/constexpr3.C
> new file mode 100644
> index 000..b6e43a93664
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/diagnostic/constexpr3.C
> @@ -0,0 +1,32 @@
> +// { dg-do compile { target c++14 } }

I've added the missing PR c++/91483 here and in the ChangeLog in my local repo.

Marek

[PATCH] c++: improve verify_constant diagnostic [PR91483]

[Marek Polacek via Gcc-patches]

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --

When verify_constant complains, it's pretty terse.  Consider

  void test ()
  {
constexpr int i = 42;
constexpr const int *p = 
  }

where it says "'& i' is not a constant expression".  OK, but why?

With this patch, we say:

b.C:5:28: error: '& i' is not a constant expression
5 |   constexpr const int *p = 
  |^~
b.C:5:28: note: pointer to 'i' is not a constant expression
b.C:4:17: note: address of non-static constexpr variable 'i' may differ on each 
invocation of the enclosing function; add 'static' to give it a constant address
4 |   constexpr int i = 42;
  | ^
  | static

which brings g++ on par with clang++.

gcc/cp/ChangeLog:

* constexpr.cc (verify_constant_explain_r): New.
(verify_constant): Call it.

gcc/testsuite/ChangeLog:

* g++.dg/diagnostic/constexpr3.C: New test.
---
 gcc/cp/constexpr.cc  | 56 +++-
 gcc/testsuite/g++.dg/diagnostic/constexpr3.C | 32 +++
 2 files changed, 87 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/constexpr3.C

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 8bd5c4a47f8..6d5aed82377 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -3381,6 +3381,54 @@ ok:
 }
 }
 
+/* *TP was not deemed constant by reduced_constant_expression_p.  Explain
+   why and suggest what could be done about it.  */
+
+static tree
+verify_constant_explain_r (tree *tp, int *, void *)
+{
+  bool ref_p = false;
+
+  switch (TREE_CODE (*tp))
+{
+CASE_CONVERT:
+  if (TREE_CODE (TREE_OPERAND (*tp, 0)) != ADDR_EXPR)
+   break;
+  ref_p = TYPE_REF_P (TREE_TYPE (*tp));
+  *tp = TREE_OPERAND (*tp, 0);
+  gcc_fallthrough ();
+case ADDR_EXPR:
+  {
+   tree op = TREE_OPERAND (*tp, 0);
+   if (VAR_P (op)
+   && DECL_DECLARED_CONSTEXPR_P (op)
+   && !TREE_STATIC (op)
+   /* ??? We should also say something about temporaries.  */
+   && !DECL_ARTIFICIAL (op))
+ {
+   if (ref_p)
+ inform (location_of (*tp), "reference to %qD is not a constant "
+ "expression", op);
+   else
+ inform (location_of (*tp), "pointer to %qD is not a constant "
+ "expression", op);
+   const location_t op_loc = DECL_SOURCE_LOCATION (op);
+   rich_location richloc (line_table, op_loc);
+   richloc.add_fixit_insert_before (op_loc, "static ");
+   inform (,
+   "address of non-static constexpr variable %qD may differ on 
"
+   "each invocation of the enclosing function; add % "
+   "to give it a constant address", op);
+ }
+   break;
+  }
+default:
+  break;
+}
+
+  return NULL_TREE;
+}
+
 /* Some expressions may have constant operands but are not constant
themselves, such as 1/0.  Call this function to check for that
condition.
@@ -3398,7 +3446,13 @@ verify_constant (tree t, bool allow_non_constant, bool 
*non_constant_p,
   && t != void_node)
 {
   if (!allow_non_constant)
-   error ("%q+E is not a constant expression", t);
+   {
+ auto_diagnostic_group d;
+ error_at (cp_expr_loc_or_input_loc (t),
+   "%q+E is not a constant expression", t);
+ cp_walk_tree_without_duplicates (, verify_constant_explain_r,
+  nullptr);
+   }
   *non_constant_p = true;
 }
   if (TREE_OVERFLOW_P (t))
diff --git a/gcc/testsuite/g++.dg/diagnostic/constexpr3.C 
b/gcc/testsuite/g++.dg/diagnostic/constexpr3.C
new file mode 100644
index 000..b6e43a93664
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/constexpr3.C
@@ -0,0 +1,32 @@
+// { dg-do compile { target c++14 } }
+
+struct X {
+  int const& var;
+};
+
+struct A {
+  A *ap = this;
+};
+
+constexpr A
+foo ()
+{
+  return {};
+}
+
+void
+test ()
+{
+  constexpr int i = 42; // { dg-message "may differ on each invocation" }
+
+  constexpr X x{i}; // { dg-error "not a constant expression" }
+  // { dg-message "reference to .i. is not a constant expression" "" { target 
*-*-* } .-1 }
+  constexpr const int *p =  // { dg-error "not a constant expression" }
+  // { dg-message "pointer to .i. is not a constant expression" "" { target 
*-*-* } .-1 }
+
+  constexpr A a = foo (); // { dg-error "not a constant expression" }
+  // { dg-message "pointer to .a. is not a constant expression|may differ" "" 
{ target *-*-* } .-1 }
+
+  constexpr const int *q = __builtin_launder (); // { dg-error "not a 
constant expression" }
+  // { dg-message "pointer to .i. is not a constant expression" "" { target 
*-*-* } .-1 }
+}

base-commit: 6f06152541d62ae7c8579b7d7bf552be19e15b05
-- 
2.41.0

Re: [PATCH] diagnostics: Delete config pointer before overwriting it.

[David Malcolm via Gcc-patches]

On Fri, 2023-09-01 at 21:16 +0200, Mikael Morin via Gcc-patches wrote:
> Hello,
> 
> this is a fix for a small memory leak in the fortran frontend.
> Tested on x86_64-pc-linux-gnu, nothing stands out besides the
> apparently well-known guality instability.
> OK for master ? 

LGTM, thanks!

Dave

Re: [PATCH] analyzer: call off a superseding when diagnostics are unrelated [PR110830]

[David Malcolm via Gcc-patches]

On Fri, 2023-09-01 at 21:59 +0200, priour...@gmail.com wrote:
> From: benjamin priour 
> 
> Hi,
> 
> Patch succesfully regstrapped off trunk
> 7f2ed06ddc825e8a4e0edfd1d66b5156e6dc1d34
> on x86_64-linux-gnu.
> 
> Is it OK for trunk ?
> 
> Thanks,
> Benjamin.
> 

[...snip...]

>  
> +/* Walk up the two paths to each of their common conditional
> +   branching.  At each branching, make sure both diagnostics'
> +   paths branched similarly.  If there is at least one where
> +   both paths go down a different outcome, then the paths
> +   are incompatible and this function returns FALSE.
> +   Otherwise return TRUE.
> +
> +   Incompatible paths:
> +
> +   
> +   /  \
> +  /    \
> +    true  false
> + |  |
> +    ...    ...
> + |  |
> +    ...   stmt x
> + |
> +   stmt x
> +
> +   Both LHS_PATH and RHS_PATH final enodes should be
> +   over the same gimple statement.  */
> +
> +static bool
> +compatible_epath_p (const exploded_path *lhs_path,
> +   const exploded_path *rhs_path)
> +{
> +  gcc_assert (lhs_path);
> +  gcc_assert (rhs_path);
> +  int i;
> +  const exploded_edge *outer_eedge;
> +  FOR_EACH_VEC_ELT_REVERSE (lhs_path->m_edges, i, outer_eedge)
> +    {
> +  const superedge *outer_sedge = outer_eedge->m_sedge;
> +  if (!outer_sedge || !outer_eedge->m_src)
> +   continue;
> +  const program_point _src_point = outer_eedge->m_src->get_point 
> ();
> +  switch (outer_src_point.get_kind ())
> +   {
> + case PK_AFTER_SUPERNODE:
> +   if (const cfg_superedge *cfg_outer_sedge
> +   = outer_sedge->dyn_cast_cfg_superedge ())
> + {
> +   int j;
> +   const exploded_edge *inner_eedge;
> +   FOR_EACH_VEC_ELT_REVERSE (rhs_path->m_edges, j, inner_eedge)
> + {
> +   const superedge *inner_sedge = inner_eedge->m_sedge;
> +   if (!inner_sedge || !inner_eedge->m_src)
> + continue;
> +   const program_point _src_point
> + = inner_eedge->m_src->get_point ();
> +   switch (inner_src_point.get_kind ())
> + {
> +   case PK_AFTER_SUPERNODE:
> + if (inner_src_point.get_stmt ()
> + != outer_src_point.get_stmt ())
> +   continue;
> + if (const cfg_superedge *cfg_inner_sedge
> + = inner_sedge->dyn_cast_cfg_superedge ())
> +   {
> + if (cfg_inner_sedge->true_value_p ()
> + != cfg_outer_sedge->true_value_p ())
> +   return false;
> +   }
> + break;
> +   default:
> + break;
> + }
> + }
> + }
> +   break;
> +
> + default:
> +   break;
> +   }
> +    }
> +    return true;
> +}

[...snip...]

Thanks for the patch.  I think the high-level idea is good, but I'm not
sure the implementation is correct:

- it is O(n^2), where n is the length of exploded_path.
- it walks backwards through the LHS path, and for each eedge from a
PK_AFTER_SUPERNODE it walks backwards from the end of the RHS epath; it
only looks at the "true" flag on CFG edges.  I think this works for
simple cases, but the way it restarts the rhs_path iteration from the
end of the rhs_path each time "feels" incorrect.

An eedge from a PK_AFTER_SUPERNODE is presumably just an eedge that has
a non-NULL m_sedge i.e. an exploded edge relating to an edge in the
supergraph.  Rather than looking at flags, can we simply compare
superedge pointers?  For example, if we care that we followed the
"true" path of a conditional in both lhs and rhs epaths, we can look to
see if both have an eedge where the superedge is the cfg_superedge
wrapping the CFG "true" edge i.e. I think we can simply compare the
superedge pointers.

Or is there some detail here that I'm misunderstanding?

I *think* it's possible to implement it in O(n) with something like
this:  (warning: untested code follows!)

  /* For compatibility, there should effectively be the same
 vector of superedges followed in both epaths.
 Walk backwards through each epath, looking at the superedges.  */
  // FIXME: really?  Benjamin, have I understood this correctly?

  gcc_assert (lhs_path->length () > 0);
  gcc_assert (rhs_path->length () > 0);

  int lhs_idx = lhs_path->length () - 1;
  int rhs_idx = rhs_path->length () - 1;

  while (lhs_idx >= 0 && rhs_idx >= 0)
{
  /* Find next LHS superedge, if any.  */
  while (lhs_idx >= 0)
{
  const exploded_edge *lhs_eedge = lhs_path->m_edges[lhs_idx];
  if (lhs_eedge->m_sedge)
break;
  else
 

[Bug libstdc++/111050] [11/12/13/14 Regression] ABI break in _Hash_node_value_base since GCC 11

[rs2740 at gmail dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111050

--- Comment #2 from TC  ---
The impacted members we observed are `_Hash_node_value_base::_M_valptr` and
`_Hash_node_value_base::_M_v`. I think the layout of `_Hash_node` didn't
change.

And I'm not seeing why fixing this will require breaking ABI again. For
example, if the affected functions are marked always_inline (or renamed, or
have their mangling otherwise changed), I would expect the resulting code to be
linkable with either the GCC10 version or the current version of the code,
unless I'm missing something?

[pushed] wwwdocs: gcc-12: Improve language around vectorizer and -O2

[Gerald Pfeifer]

Pushed.

Gerald
---
 htdocs/gcc-12/changes.html | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/htdocs/gcc-12/changes.html b/htdocs/gcc-12/changes.html
index 3816d06f..b10f2aa4 100644
--- a/htdocs/gcc-12/changes.html
+++ b/htdocs/gcc-12/changes.html
@@ -127,10 +127,12 @@ You may also want to check out our
 General Improvements
 
 
-  Vectorization is enabled at -O2 which is now equivalent to 
the
-  original -O2 -ftree-vectorize -fvect-cost-model=very-cheap.
-  Note that default vectorizer cost model has been changed which used to 
behave
-  as -fvect-cost-model=cheap were specified.
+  Vectorization is enabled at -O2 which is now
+  equivalent to what would have been
+  -O2 -ftree-vectorize -fvect-cost-model=very-cheap
+  in the past. Note that the default vectorizer cost model has
+  been changed; it used to behave as if
+  -fvect-cost-model=cheap had been specified.
   
   
 GCC now supports the
-- 
2.41.0

[Bug c++/111272] [13/14 Regression] Truncated error messages with -std=c++23 and -std=c++26

[pkeir at outlook dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111272

--- Comment #2 from Paul Keir  ---
Thanks. The `-Winvalid-constexpr` mentioned there is a helpful workaround.

gcc-12-20230901 is now available

[GCC Administrator via Gcc]

Snapshot gcc-12-20230901 is now available on
  https://gcc.gnu.org/pub/gcc/snapshots/12-20230901/
and on various mirrors, see http://gcc.gnu.org/mirrors.html for details.

This snapshot has been generated from the GCC 12 git branch
with the following options: git://gcc.gnu.org/git/gcc.git branch 
releases/gcc-12 revision 2f8ccacd41a45bccf3e19625c0fbd2627472b45e

You'll find:

 gcc-12-20230901.tar.xz   Complete GCC

  SHA256=b0899483d99906d1f462c76aee77182ff5aa92cf0d7318750c98ec1f9bb7a8ae
  SHA1=de8a843db183852e9f0121438deedcf6de8c4062

Diffs from 12-20230825 are available in the diffs/ subdirectory.

When a particular snapshot is ready for public consumption the LATEST-12
link is updated and a message is sent to the gcc list.  Please do not use
a snapshot before it has been announced that way.

Re: RFC: Introduce -fhardened to enable security-related flags

[Qing Zhao via Gcc-patches]

> On Aug 29, 2023, at 3:42 PM, Marek Polacek via Gcc-patches 
>  wrote:
> 
> Improving the security of software has been a major trend in the recent
> years.  Fortunately, GCC offers a wide variety of flags that enable extra
> hardening.  These flags aren't enabled by default, though.  And since
> there are a lot of hardening flags, with more to come, it's been difficult
> to keep on top of them; more so for the users of GCC who ought not to be
> expected to keep track of all the new options.
> 
> To alleviate some of the problems I mentioned, we thought it would
> be useful to provide a new umbrella option that enables a reasonable set
> of hardening flags.  What's "reasonable" in this context is not easy to
> pin down.  Surely, there must be no ABI impact, the option cannot cause
> severe performance issues, and, I suspect, it should not cause build
> errors by enabling stricter compile-time errors (such as, -Wimplicit-int,
> -Wint-conversion).  Including a controversial option in -fhardened
> would likely cause that users would not use -fhardened at all.  It's
> roughly akin to -Wall or -O2 -- those also enable a reasonable set of
> options, and evolve over time, and are not kept in sync with other
> compilers.
> 
> Currently, -fhardened enables:
> 
>  -D_FORTIFY_SOURCE=3 (or =2 for older glibcs)
>  -D_GLIBCXX_ASSERTIONS
>  -ftrivial-auto-var-init=zero
>  -fPIE  -pie  -Wl,-z,relro,-z,now
>  -fstack-protector-strong
>  -fstack-clash-protection
>  -fcf-protection=full (x86 GNU/Linux only)
> 
> -fsanitize=undefined is specifically not enabled.  -fstrict-flex-arrays is
> also liable to break a lot of code so I didn't include it.
> 
> Appended is a proof-of-concept patch.  It doesn't implement --help=hardened
> yet.  A fairly crucial point is that -fhardened will not override options
> that were specified on the command line (before or after -fhardened).  For
> example,
> 
> -D_FORTIFY_SOURCE=1 -fhardened
> 
> means that _FORTIFY_SOURCE=1 will be used.  Similarly,
> 
>  -fhardened -fstack-protector
> 
> will not enable -fstack-protector-strong.
> 
> Thoughts?

In general, I think that it is a very good idea to provide umbrella options
 for software security purpose.  Thanks a lot for this work!

1. I do agree with Martin, multiple-level control for this purpose might be 
needed,
similar as multiple levels for warnings, and multiple levels for optimizations.

Similar as optimization options, can we organize all the security options 
together 
In our manual, then the user will have a good central place to get more and 
complete
Information of the security features our compiler provides? 

2. What’s the major criteria to decide which security feature should go into 
this list?
Later, when we have new security features, how to decide whether to add them to
This list or not?
I am wondering why -fzero-call-used-regs is not included in the list and also 
Why chose -ftrivial-auto-var-init=zero instead of 
-ftrivial-auto-var-init=pattern? 

3. Looks like currently, -fhardened excludes all compilation-time Warning 
options for security purpose,
(For example, -Warray-bounds, --Wstringop-overflow…)
And also excludes all sanitizer options for security purpose 
(-fsanitizer=undifined)

So, shall we also provide an umbrella option for compilation-time warning 
options for security purpose
And a umbrella option for sanitizer options (is the -fsanitizer=undefined this 
one)?

Just some thoughts. -:).

Qing

[Bug tree-optimization/110817] [14 Regression] wrong code with vector compares and vector lowering

[pinskia at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110817

--- Comment #18 from Andrew Pinski  ---
(In reply to Andrew Pinski from comment #17)
> (In reply to Andrew Pinski from comment #16)
> > Or just change ssa_name_has_boolean_range to use gimple_zero_one_valued_p
> > instead:
> > 
> > extern bool gimple_zero_one_valued_p (tree t, tree (*valueize)(tree));
> > bool
> > ssa_name_has_boolean_range (tree op)
> > {
> >   gcc_assert (TREE_CODE (op) == SSA_NAME);
> >   return gimple_zero_one_valued_p (op, NULL);
> > }
> > 
> > Later on we can remove the ssa_name_has_boolean_range wrapper around
> > gimple_zero_one_valued_p  too ...
> 
> But get_nonzero_bits needs to change to use not just the cached version of
> the range (nonzero bits) but instead `get_range_query (cfun)->range_of_expr`.
> 
> Anyways I have a fix still and this should fix other issues too ...

Except that does not work as we remove now during evrp too many
__builtin_unreachable's.

So back to drawing board for a second.

Re: [PATCH 9/12] libgcc _BitInt support [PR102989]

[Joseph Myers]

On Wed, 9 Aug 2023, Jakub Jelinek via Gcc-patches wrote:

> I know that soft-fp is owned by glibc and I think the op-common.h change
> should be propagated there, but the bitint stuff is really GCC specific
> and IMHO doesn't belong into the glibc copy.

The op-common.h change is OK for glibc.

Some additional tests I think should be added to the testsuite for 
floating-point functionality in this patch, that I didn't spot in the 
testsuite patches - if any of these aren't included initially, there 
should at least be bugs filed in Bugzilla for the omissions:

1. Test overflowing conversions to integers (including from inf or NaN) 
raise FE_INVALID.  (Note: it's not specified in the standard whether 
inexact conversions to integers raise FE_INEXACT or not, so testing that 
seems less important.)

2. Test conversions from integers to floating point raise FE_INEXACT when 
inexact, together with FE_OVERFLOW when overflowing (while exact 
conversions don't raise exceptions).

3. Test conversions from integers to floating point respect the rounding 
mode.

4. Test converting floating-point values in the range (-1.0, 0.0] to both 
unsigned and signed _BitInt; I didn't see such tests for binary floating 
types, only for decimal types, and the decimal tests didn't include tests 
of negative zero itself as the value converted to _BitInt.

5. Test conversions of noncanonical BID zero to integers (these tests 
would be specific to BID).  See below for a bug in this area.

For points 2 and 3 above, it's probably appropriate to test only for 
binary floating point, to avoid any issues with the separate DFP rounding 
mode and with DFP arithmetic operations not necessarily working correctly 
with exceptions - but then a bug should be filed in Bugzilla noting the 
omission of such tests for DFP.

For points 1, 2 and 3 above, if the conversions for types such as 
_BitInt(32) might end up using the same conversions as for types such as 
int, then tests for such types should probably be omitted (again with a 
bug filed) given the range of known bugs about exceptions from such 
operations with types such as int.

> +__bid_fixtdbitint (UBILtype *r, SItype rprec, _Decimal128 a)
> +{
> +  FP_DECL_EX;
> +  USItype arprec = rprec < 0 ? -rprec : rprec;
> +  USItype rn = (arprec + BIL_TYPE_SIZE - 1) / BIL_TYPE_SIZE;
> +  union { _Decimal128 d; UDItype u[2]; } u;
> +  UDItype mantissahi, mantissalo, t;
> +  SItype sgn;
> +  SItype exponent;
> +  USItype exp_bits, mant_bits;
> +  UBILtype *pow10v, *resv;
> +  USItype pow10_limbs, res_limbs, min_limbs, mant_limbs, low_zeros;
> +
> +  FP_INIT_EXCEPTIONS;
> +  u.d = a;
> +  mantissahi = u.u[__FLOAT_WORD_ORDER__ != __ORDER_BIG_ENDIAN__];
> +  mantissalo = u.u[__FLOAT_WORD_ORDER__ == __ORDER_BIG_ENDIAN__];
> +  t = mantissahi >> 47;
> +  sgn = (DItype) mantissahi < 0;
> +  if ((t & (3 << 14)) != (3 << 14))
> +{
> +  mantissahi &= UDItype) 1) << 49) - 1);
> +  exponent = (t >> 2) & 0x3fff;

Overflow (thus producing a noncanonical zero) is possible in this case for 
TDmode.  An appropriate test of a noncanonical zero that goes through this 
case should thus be added to the testsuite.

> +}
> +  else if ((t & (3 << 12)) != (3 << 12))
> +{
> +  mantissahi &= UDItype) 1) << 47) - 1);
> +  mantissahi |= ((UDItype) 1) << 49;
> +  exponent = t & 0x3fff;
> +  if (mantissahi > (UDItype) 0x1ed09bead87c0
> +   || (mantissahi == (UDItype) 0x1ed09bead87c0
> +   && mantissalo > 0x378d8e63))
> + {
> +   mantissahi = 0;
> +   mantissalo = 0;
> + }

And in this case, overflow is guaranteed; the check for the overflow 
threshold should thus move to the previous case.

This patch is OK with these fixes.

Note for powerpc architecture maintainers: adding _BitInt support on that 
architecture will mean, as well as adding support for the conversions 
to/from DPD (if S/390 doesn't get there first), also adding support for 
conversions to/from IBM long double.

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

[Bug tree-optimization/107876] [13 Regression] ICE in verify_dominators, at dominance.cc:1184 (error: dominator of 4 should be 14, not 16) since r13-3749-g7314b98b1bcd382c

[egallager at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107876

Eric Gallager  changed:

   What|Removed |Added

 Resolution|FIXED   |---
   Keywords||testsuite-fail
 CC||egallager at gcc dot gnu.org
 Status|RESOLVED|REOPENED

--- Comment #8 from Eric Gallager  ---
(In reply to Francois-Xavier Coudert from comment #7)
> The test case (g++.dg/tree-ssa/pr107876.C) fails on aarch64-darwin.
> 
> FAIL: g++.dg/tree-ssa/pr107876.C  -std=gnu++14  scan-tree-dump unswitch
> "unswitching loop 1 on .switch. with condition: i_[0-9]+\\(D\\) == 2"
> FAIL: g++.dg/tree-ssa/pr107876.C  -std=gnu++17  scan-tree-dump unswitch
> "unswitching loop 1 on .switch. with condition: i_[0-9]+\\(D\\) == 2"
> FAIL: g++.dg/tree-ssa/pr107876.C  -std=gnu++20  scan-tree-dump unswitch
> "unswitching loop 1 on .switch. with condition: i_[0-9]+\\(D\\) == 2"
> 
> The output says:
> 
> ;; Function test17 (_Z6test17i, funcdef_no=0, decl_uid=4194, cgraph_uid=1,
> symbol_order=0)
> 
> Estimating # of iterations of loop 1
> g++.dg/tree-ssa/pr107876.C:21:7: optimized: unswitching loop 1 on 'if' with
> condition: i_7(D) == 0
> g++.dg/tree-ssa/pr107876.C:21:7: note: optimized sizes estimated to 0 (true)
> and 0 (false) from original size 16

reopening, then

[Bug tree-optimization/110817] [14 Regression] wrong code with vector compares and vector lowering

[pinskia at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110817

--- Comment #17 from Andrew Pinski  ---
(In reply to Andrew Pinski from comment #16)
> Or just change ssa_name_has_boolean_range to use gimple_zero_one_valued_p
> instead:
> 
> extern bool gimple_zero_one_valued_p (tree t, tree (*valueize)(tree));
> bool
> ssa_name_has_boolean_range (tree op)
> {
>   gcc_assert (TREE_CODE (op) == SSA_NAME);
>   return gimple_zero_one_valued_p (op, NULL);
> }
> 
> Later on we can remove the ssa_name_has_boolean_range wrapper around
> gimple_zero_one_valued_p  too ...

But get_nonzero_bits needs to change to use not just the cached version of the
range (nonzero bits) but instead `get_range_query (cfun)->range_of_expr`.

Anyways I have a fix still and this should fix other issues too ...

[Bug testsuite/111264] [14 regression] gcc.dg/plugin/analyzer_cpython_plugin.c breaks after r14-3580-g597b9ec69bca8a

[efric at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111264

--- Comment #9 from Eric Feng  ---
(In reply to Hans-Peter Nilsson from comment #4)
> 
> If we move past the difference in semantics of the idioms in the patch, I
> still don't see why there actually was error for the original syntax.  There
> must be something in the difference between the hash_map and hash_set
> declarations.  Eagerly awaiting comments on the patch.  (Probably not the
> best way to learn recent C++ standards, but believe it or not, I'm exposed
> to a worse way, on another track... :)

Thanks again for the patch, Hans-Peter. For those interested, I try to dive
into why hash_map caused an error here whilst hash_set was fine in the original
syntax in the body of this e-mail:
https://gcc.gnu.org/pipermail/gcc/2023-September/242404.html. In short: I think
the issue was due to the compiler having trouble disambiguating between which
hash_map constructor to use in the original syntax in C++11. Please feel free
to chime in and correct my hypothesis. Cheers!

Re: [PATCH 14/12] libgcc _BitInt helper documentation [PR102989]

[Joseph Myers]

On Tue, 22 Aug 2023, Jakub Jelinek via Gcc-patches wrote:

> +significant limb if @var{N} is not divisible by

@var{N} should be @var{n}, throughout.

> +@deftypefn {Runtime Function} void __bid_fixsdbitint (@code{UBILtype} 
> *@var{r}, int32_t @var{rprec}, _Decimal32 @var{a})
> +@deftypefnx {Runtime Function} void __bid_fixddbitint (@code{UBILtype} 
> *@var{r}, int32_t @var{rprec}, _Decimal64 @var{a})
> +@deftypefnx {Runtime Function} void __bid_fixtdbitint (@code{UBILtype} 
> *@var{r}, int32_t @var{rprec}, _Decimal128 @var{a})
> +These functions convert @var{a} to bit-precise integer @var{r}, rounding 
> toward zero.
> +If @var{rprec} is positive, it converts to unsigned bit-precise integer and
> +negative values all become zero, if @var{rprec} is negative, it converts
> +to signed bit-precise integer.
> +@end deftypefn
> +
> +@deftypefn {Runtime Function} _Decimal32 __bid_floatbitintsd 
> (@code{UBILtype} *@var{i}, int32_t @var{iprec})
> +@deftypefnx {Runtime Function} _Decimal64 __bid_floatbitintdd 
> (@code{UBILtype} *@var{i}, int32_t @var{iprec})
> +@deftypefnx {Runtime Function} _Decimal128 __bid_floatbitinttd 
> (@code{UBILtype} *@var{i}, int32_t @var{iprec})
> +These functions convert bit-precise integer @var{i} to decimal floating 
> point.  If
> +@var{iprec} is positive, it is conversion from unsigned bit-precise integer,
> +otherwise from signed bit-precise integer.
> +@end deftypefn

The documentation for __bid_* should say explicitly that these functions 
are for BID format (assuming it's intended that functions for DPD format 
should use __dpd_* when support is added for an architecture using DPD).

> +/* Common final part of __fix?fbitint conversion functions.
> +   The A floating point value should have been converted using
> +   soft-fp macros into RV, U##DI##type DI##_BITS precise normal
> +   integral type and SHIFT, how many bits should that value be
> +   shifted to the left.  R is pointer to limbs array passed to the
> +   function, RN number of limbs in it, ARPREC absolute value of
> +   RPREC argument passed to it, RSIZE number of significant bits in RV.
> +   RSIGNED is non-zero if the result is signed bit-precise integer,
> +   otherwise zero.  If OVF is true, instead of storing RV shifted left
> +   by SHIFT bits and zero or sign extended store minimum or maximum
> +   of the signed or unsigned bit-precise integer type depending on if
> +   RV contains the minimum or maximum signed or unsigned value.  */

As I understand it, OVF is also for the case of a zero result from input 
close to zero, for signed types (when that's not the maximum or minimum) 
in addition to unsigned types.

> +/* Common initial part of __floatbitint?f conversion functions.
> +   I and IPREC are arguments passed to those functions, convert that
> +   into a pair of DI##type IV integer and SHIFT, such that converting
> +   IV to floating point and multiplicating that by pow (2, SHIFT)
> +   gives the expected result.  IV size needs to be chosen such that
> +   it is large than number of bits in floating-point mantissa and

"large than" -> "larger than".

This patch is OK with those fixes.

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

[Bug libstdc++/110572] ld.lld: error: duplicate symbol: std::type_info::operator==(std::type_info const&) const

[peter0x44 at disroot dot org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110572

peter0x44 at disroot dot org changed:

   What|Removed |Added

 CC||peter0x44 at disroot dot org

--- Comment #5 from peter0x44 at disroot dot org ---

#include 
int main() { return typeid(0) == typeid(0); }

The following reproduces for me, although strangely only with -std=c++23 and
-static-libstdc++.

x86_64-w64-mingw32-g++ test.cpp -static-libstdc++ -std=c++20
// no error

x86_64-w64-mingw32-g++ test.cpp -static-libstdc++ -std=c++23
/usr/lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld:
/usr/lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/lib/../lib/libstdc++.a(tinfo.o):
in function `std::type_info::operator==(std::type_info const&) const':
/build/mingw-w64-gcc/src/gcc/libstdc++-v3/libsupc++/tinfo.cc:42: multiple
definition of `std::type_info::operator==(std::type_info const&) const';
/tmp/ccyAJVlk.o:test.cpp:(.text$_ZNKSt9type_infoeqERKS_[_ZNKSt9type_infoeqERKS_]+0x0):
first defined here

Related issue:
https://github.com/skeeto/w64devkit/issues/86

Re: [PATCH 8/12] libgcc: Generated tables for _BitInt <-> _Decimal* conversions [PR102989]

[Joseph Myers]

On Wed, 9 Aug 2023, Jakub Jelinek via Gcc-patches wrote:

> Hi!
> 
> The following patch adds a header with generated helper tables to support
> computation of powers of 10 from 10^0 to 10^6111 inclusive into a
> sufficiently large array of _BitInt limbs.  This is split from the rest
> of the libgcc _BitInt support because it is quite large and together it
> would run into gcc-patches mail length limits.

This patch is OK.

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

[Bug c++/111273] New: Spurious array-bounds error when copying data using _GLIBCXX_DEBUG iterators

[jgrossma at qti dot qualcomm.com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111273

Bug ID: 111273
   Summary: Spurious array-bounds error when copying data using
_GLIBCXX_DEBUG iterators
   Product: gcc
   Version: 13.2.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: jgrossma at qti dot qualcomm.com
  Target Milestone: ---

When compiling with -Warray-bounds and -D_GLIBCXX_DEBUG on g++ 13.2.0, copying
from a vector to an array throws the Warray-bounds error during compilation.

Source:

#include 
#include 
#include 

static const int N = 1;

int func(std::vector const ) {
std::array a;
std::copy_n(v.begin(), N, a.begin());
return a[0];
}


Program should not have any problems at compile time. Copies 1 location to a 1
element array. Changing N=2 doesn't show the error. 

Compile options:

-Werror -O3 -Warray-bounds -D_GLIBCXX_DEBUG -std=c++20


Disabling -D_GLIBCXX_DEBUG doesn't show the error.


Output:

---

In file included from
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/vector:62,
 from :1:
In static member function 'static constexpr _Up* std::__copy_move<_IsMove,
true, std::random_access_iterator_tag>::__copy_m(_Tp*, _Tp*, _Up*) [with _Tp =
const int; _Up = int; bool _IsMove = false]',
inlined from 'constexpr _OI std::__copy_move_a2(_II, _II, _OI) [with bool
_IsMove = false; _II = const int*; _OI = int*]' at
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/stl_algobase.h:506:30,
inlined from 'constexpr _OI std::__copy_move_a1(_II, _II, _OI) [with bool
_IsMove = false; _II = const int*; _OI = int*]' at
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/stl_algobase.h:533:42,
inlined from 'constexpr _OI std::__copy_move_a(_II, _II, _OI) [with bool
_IsMove = false; _II = __gnu_cxx::__normal_iterator > >; _OI = int*]' at
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/stl_algobase.h:540:31,
inlined from '_OI std::__copy_move_a(const
__gnu_debug::_Safe_iterator<_Ite, _Seq, _Cat>&, const
__gnu_debug::_Safe_iterator<_Ite, _Seq, _Cat>&, _OI) [with bool _IsMove =
false; _Ite = __gnu_cxx::__normal_iterator > >; _Seq = __debug::vector; _Cat =
random_access_iterator_tag; _OI = int*]' at
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/debug/safe_iterator.tcc:257:36,
inlined from 'constexpr _OI std::copy(_II, _II, _OI) [with _II =
__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator > >, __debug::vector,
random_access_iterator_tag>; _OI = int*]' at
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/stl_algobase.h:633:7,
inlined from 'constexpr _OutputIterator
std::__copy_n(_RandomAccessIterator, _Size, _OutputIterator,
random_access_iterator_tag) [with _RandomAccessIterator =
__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator > >, __debug::vector,
random_access_iterator_tag>; _Size = int; _OutputIterator = int*]' at
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/stl_algo.h:731:23,
inlined from 'constexpr _OIter std::copy_n(_IIter, _Size, _OIter) [with
_IIter = __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator > >, __debug::vector,
random_access_iterator_tag>; _Size = int; _OIter = int*]' at
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/stl_algo.h:763:27,
inlined from 'int func(const std::__debug::vector&)' at :9:16:
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/stl_algobase.h:437:30:
error: 'void* __builtin_memcpy(void*, const void*, long unsigned int)' forming
offset 4 is out of the bounds [0, 4] of object 'a' with type 'std::array' [-Werror=array-bounds=]
  437 | __builtin_memmove(__result, __first, sizeof(_Tp) * _Num);
  | ~^~~
: In function 'int func(const std::__debug::vector&)':
:8:24: note: 'a' declared here
8 | std::array a;
  |^
cc1plus: all warnings being treated as errors
Compiler returned: 1

---

Interestingly, the error location should not even be hit for a 1 element copy
as it's inside a check for length>1.


Godbolt link shows as well: https://godbolt.org/z/v8GK4nzde


GCC 12 does not seem to show this issue.

Seems related to this one: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107852

Re: [PATCH] analyzer: implement reference count checking for CPython plugin [PR107646]

[Eric Feng via Gcc-patches]

Thank you for the patch!

On Fri, Sep 1, 2023 at 10:51 AM David Malcolm  wrote:
>
> On Fri, 2023-09-01 at 04:49 +0200, Hans-Peter Nilsson wrote:
> > (Looks like this was committed as r14-3580-g597b9ec69bca8a)
> >
> > > Cc: g...@gcc.gnu.org, gcc-patches@gcc.gnu.org, Eric Feng
> > > 
> > > From: Eric Feng via Gcc 
> >
> > > gcc/testsuite/ChangeLog:
> > >   PR analyzer/107646
> > > * gcc.dg/plugin/analyzer_cpython_plugin.c: Implements
> > > reference count
> > >   * checking for PyObjects.
> > > * gcc.dg/plugin/cpython-plugin-test-2.c: Moved to...
> > > * gcc.dg/plugin/cpython-plugin-test-PyList_Append.c:
> > > ...here (and
> > >   * added more tests).
> > > * gcc.dg/plugin/cpython-plugin-test-1.c: Moved to...
> > > * gcc.dg/plugin/cpython-plugin-test-no-plugin.c: ...here
> > > (and added
> > >   * more tests).
> > > * gcc.dg/plugin/plugin.exp: New tests.
> > > * gcc.dg/plugin/cpython-plugin-test-PyList_New.c: New test.
> > > * gcc.dg/plugin/cpython-plugin-test-PyLong_FromLong.c: New
> > > test.
> > > * gcc.dg/plugin/cpython-plugin-test-refcnt-checking.c: New
> > > test.
> >
> > It seems this was more or less a rewrite, but that said,
> > it's generally preferable to always *add* tests, never *modify* them.
> >
> > >  .../gcc.dg/plugin/analyzer_cpython_plugin.c   | 376
> > > +-
> >
> > ^^^ Ouch!  Was it not within reason to keep that test as it
> > was, and just add another test?
Thanks for the feedback. To clarify, 'analyzer_cpython_plugin.c' is
not a test itself but rather a plugin that currently lives within the
testsuite. The core of the test cases were also not modified, rather I
renamed certain filenames containing them for clarity (unless this is
what you meant in terms of modification, in which case noted) and
added to them. However, I understand the preference and will keep that
in mind.
> >
> > Anyway, the test after rewrite fails, and for some targets
> > like cris-elf and apparently m68k-linux, yields an error.
> > I see a PR was already opened.
> >
> > Also, mostly for future reference, several files in the
> > patch miss a final newline, as seen by a "\ No newline at
> > end of file"-marker.
Noted.
> >
> > I think I found the problem; a mismatch between default C++
> > language standard between host-gcc and target-gcc.
> >
> > (It's actually *not* as simple as "auto var = typeofvar()"
> > not being recognized in C++11 --or else there'd be an error
> > for the hash_set declaration too, which I just changed for
> > consistency-- but it's close enough for me.)
> >
> > With this, retesting plugin.exp for cris-elf works.
Sounds good, thanks again! I was also curious about why hash_map had
an issue here with that syntax whilst hash_set did not, so I tried to
investigate a bit further. I believe the issue was due to the compiler
having trouble disambiguating between the hash_map constructors in
C++11.

>From the error message we received:

test/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c:480:58:
error: no matching function for call to 'hash_map::hash_map(hash_map)'
   auto region_to_refcnt = hash_map ();

I think the compiler is mistakenly interpreting the call here as we
would like to create a new hash_map object using its copy constructor,
but we "forgot" to provide the object to be copied, rather than our
intention of using the default constructor.

Looking at hash_map.h and hash_set.h seems to support this hypothesis,
as hash_map has two constructors, one of which resembles a copy
constructor with additional arguments:
https://github.com/gcc-mirror/gcc/blob/master/gcc/hash-map.h#L147.
Perhaps the default arguments here further complicated the ambiguity
as to which constructor to use in the presence of the empty
parenthesis.

On the other hand, hash_set has only the default constructor with
default parameters, and thus there is no ambiguity:
https://github.com/gcc-mirror/gcc/blob/master/gcc/hash-set.h#L40.

I assume this ambiguity was cleared up by later versions, and thus we
observed no problems in C++17. However, I am certainly still a
relative newbie of C++, so please anyone feel free to correct my
reasoning and chime in!
> >
> > Ok to commit?
>
> Sorry about the failing tests.
>
> Thanks for the patch; please go ahead and commit.
>
> Dave
>
> >
> > -- >8 --
> > From: Hans-Peter Nilsson 
> > Date: Fri, 1 Sep 2023 04:36:03 +0200
> > Subject: [PATCH] testsuite: Fix analyzer_cpython_plugin.c
> > declarations, PR testsuite/111264
> >
> > Also, add missing newline at end of file.
> >
> > PR testsuite/111264
> > * gcc.dg/plugin/analyzer_cpython_plugin.c: Make declarations
> > C++11-compatible.
> > ---
> >  gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
> > b/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
> > index 

Re: [PATCH] analyzer: implement reference count checking for CPython plugin [PR107646]

[Eric Feng via Gcc]

Thank you for the patch!

On Fri, Sep 1, 2023 at 10:51 AM David Malcolm  wrote:
>
> On Fri, 2023-09-01 at 04:49 +0200, Hans-Peter Nilsson wrote:
> > (Looks like this was committed as r14-3580-g597b9ec69bca8a)
> >
> > > Cc: gcc@gcc.gnu.org, gcc-patc...@gcc.gnu.org, Eric Feng
> > > 
> > > From: Eric Feng via Gcc 
> >
> > > gcc/testsuite/ChangeLog:
> > >   PR analyzer/107646
> > > * gcc.dg/plugin/analyzer_cpython_plugin.c: Implements
> > > reference count
> > >   * checking for PyObjects.
> > > * gcc.dg/plugin/cpython-plugin-test-2.c: Moved to...
> > > * gcc.dg/plugin/cpython-plugin-test-PyList_Append.c:
> > > ...here (and
> > >   * added more tests).
> > > * gcc.dg/plugin/cpython-plugin-test-1.c: Moved to...
> > > * gcc.dg/plugin/cpython-plugin-test-no-plugin.c: ...here
> > > (and added
> > >   * more tests).
> > > * gcc.dg/plugin/plugin.exp: New tests.
> > > * gcc.dg/plugin/cpython-plugin-test-PyList_New.c: New test.
> > > * gcc.dg/plugin/cpython-plugin-test-PyLong_FromLong.c: New
> > > test.
> > > * gcc.dg/plugin/cpython-plugin-test-refcnt-checking.c: New
> > > test.
> >
> > It seems this was more or less a rewrite, but that said,
> > it's generally preferable to always *add* tests, never *modify* them.
> >
> > >  .../gcc.dg/plugin/analyzer_cpython_plugin.c   | 376
> > > +-
> >
> > ^^^ Ouch!  Was it not within reason to keep that test as it
> > was, and just add another test?
Thanks for the feedback. To clarify, 'analyzer_cpython_plugin.c' is
not a test itself but rather a plugin that currently lives within the
testsuite. The core of the test cases were also not modified, rather I
renamed certain filenames containing them for clarity (unless this is
what you meant in terms of modification, in which case noted) and
added to them. However, I understand the preference and will keep that
in mind.
> >
> > Anyway, the test after rewrite fails, and for some targets
> > like cris-elf and apparently m68k-linux, yields an error.
> > I see a PR was already opened.
> >
> > Also, mostly for future reference, several files in the
> > patch miss a final newline, as seen by a "\ No newline at
> > end of file"-marker.
Noted.
> >
> > I think I found the problem; a mismatch between default C++
> > language standard between host-gcc and target-gcc.
> >
> > (It's actually *not* as simple as "auto var = typeofvar()"
> > not being recognized in C++11 --or else there'd be an error
> > for the hash_set declaration too, which I just changed for
> > consistency-- but it's close enough for me.)
> >
> > With this, retesting plugin.exp for cris-elf works.
Sounds good, thanks again! I was also curious about why hash_map had
an issue here with that syntax whilst hash_set did not, so I tried to
investigate a bit further. I believe the issue was due to the compiler
having trouble disambiguating between the hash_map constructors in
C++11.

>From the error message we received:

test/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c:480:58:
error: no matching function for call to 'hash_map::hash_map(hash_map)'
   auto region_to_refcnt = hash_map ();

I think the compiler is mistakenly interpreting the call here as we
would like to create a new hash_map object using its copy constructor,
but we "forgot" to provide the object to be copied, rather than our
intention of using the default constructor.

Looking at hash_map.h and hash_set.h seems to support this hypothesis,
as hash_map has two constructors, one of which resembles a copy
constructor with additional arguments:
https://github.com/gcc-mirror/gcc/blob/master/gcc/hash-map.h#L147.
Perhaps the default arguments here further complicated the ambiguity
as to which constructor to use in the presence of the empty
parenthesis.

On the other hand, hash_set has only the default constructor with
default parameters, and thus there is no ambiguity:
https://github.com/gcc-mirror/gcc/blob/master/gcc/hash-set.h#L40.

I assume this ambiguity was cleared up by later versions, and thus we
observed no problems in C++17. However, I am certainly still a
relative newbie of C++, so please anyone feel free to correct my
reasoning and chime in!
> >
> > Ok to commit?
>
> Sorry about the failing tests.
>
> Thanks for the patch; please go ahead and commit.
>
> Dave
>
> >
> > -- >8 --
> > From: Hans-Peter Nilsson 
> > Date: Fri, 1 Sep 2023 04:36:03 +0200
> > Subject: [PATCH] testsuite: Fix analyzer_cpython_plugin.c
> > declarations, PR testsuite/111264
> >
> > Also, add missing newline at end of file.
> >
> > PR testsuite/111264
> > * gcc.dg/plugin/analyzer_cpython_plugin.c: Make declarations
> > C++11-compatible.
> > ---
> >  gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
> > b/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
> > index 

[Bug tree-optimization/110817] [14 Regression] wrong code with vector compares and vector lowering

[pinskia at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110817

--- Comment #16 from Andrew Pinski  ---
Or just change ssa_name_has_boolean_range to use gimple_zero_one_valued_p
instead:

extern bool gimple_zero_one_valued_p (tree t, tree (*valueize)(tree));
bool
ssa_name_has_boolean_range (tree op)
{
  gcc_assert (TREE_CODE (op) == SSA_NAME);
  return gimple_zero_one_valued_p (op, NULL);
}

Later on we can remove the ssa_name_has_boolean_range wrapper around
gimple_zero_one_valued_p  too ...

[Bug target/111270] gcc/config/i386/i386-options.cc:3039:8: warning: duplicated ‘if ’ condition [-Wduplicated-cond]

[dcb314 at hotmail dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111270

David Binderman  changed:

   What|Removed |Added

 CC||hubicka at gcc dot gnu.org

--- Comment #4 from David Binderman  ---
(In reply to Jonathan Wakely from comment #3)
> The correct git blame output is:
> 
> 105c2795b0d6 gcc/config/i386/i386-options.c  (Jan Hubicka 
> 2019-07-23 09:35:18 + 3037)   if (ix86_tune_features

Thanks for that. This bug looks like one for Jan, then.

My apologies for the lack of git knowledge. Despite multiple
attempts, I've never really reached a good understanding of it's
more subtle aspects.

[Bug tree-optimization/110817] [14 Regression] wrong code with vector compares and vector lowering

[pinskia at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110817

--- Comment #15 from Andrew Pinski  ---
(In reply to Andrew Pinski from comment #14)
> Even more:
> bool
> ssa_name_has_boolean_range (tree op)
> {
>   gcc_assert (TREE_CODE (op) == SSA_NAME);
> 
>   /* Boolean types always have a range [0..1].  */
>   if (TREE_CODE (TREE_TYPE (op)) == BOOLEAN_TYPE)
> return true;

The other fix is for the above to check if the boolean type is signed or not
...

Re: [PATCH] Fortran: runtime bounds-checking in presence of array constructors [PR31059]

[Harald Anlauf via Gcc-patches]

Hi Mikael,

On 9/1/23 10:41, Mikael Morin via Gcc-patches wrote:

Le 31/08/2023 à 22:42, Harald Anlauf via Fortran a écrit :

Dear all,

gfortran's array bounds-checking code does a mostly reasonable
job for array sections in expressions and assignments, but
forgot the case that (rank-1) expressions can involve array
constructors, which have a shape ;-)

The attached patch walks over the loops generated by the
scalarizer, checks for the presence of a constructor, and
takes the first shape found as reference.  (If several
constructors are present, discrepancies in their shape
seems to be already detected at compile time).

For more details on what will be caught now see testcase.

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


This is OK.


I've pushed this is the first step.


May I suggest to handle functions the same way?


I'll have a look at them, but will need to gather a few
suitable testcases first.

Thanks for the review!

Harald



Thanks.


Thanks,
Harald

[Bug tree-optimization/110817] [14 Regression] wrong code with vector compares and vector lowering

[pinskia at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110817

Andrew Pinski  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
   Assignee|unassigned at gcc dot gnu.org  |pinskia at gcc dot 
gnu.org

--- Comment #14 from Andrew Pinski  ---
So I suspecting it is this pattern:
/* -(type)!A -> (type)A - 1.  */
(simplify
 (negate (convert?:s (logical_inverted_value:s @0)))
 (if (INTEGRAL_TYPE_P (type)
  && TREE_CODE (type) != BOOLEAN_TYPE
  && TYPE_PRECISION (type) > 1
  && TREE_CODE (@0) == SSA_NAME
  && ssa_name_has_boolean_range (@0))
  (plus (convert:type @0) { build_all_ones_cst (type); })))

We start out with:
-(unsigned int)(bool:31 == 0)

Yes bool:31 == 0 will have 0/1 but bool:31 does not.

Even more:
bool
ssa_name_has_boolean_range (tree op)
{
  gcc_assert (TREE_CODE (op) == SSA_NAME);

  /* Boolean types always have a range [0..1].  */
  if (TREE_CODE (TREE_TYPE (op)) == BOOLEAN_TYPE)
return true;

Changing that pattern to:
/* -(type)!A -> (type)A - 1.  */
(simplify
 (negate (convert?:s (logical_inverted_value:s zero_one_valued_p@0)))
 (if (INTEGRAL_TYPE_P (type)
  && TREE_CODE (type) != BOOLEAN_TYPE
  && TYPE_PRECISION (type) > 1)
  (plus (convert:type @0) { build_all_ones_cst (type); })))

Fixes the testcase in comment #10 and should fix the original issue too.

[Bug c++/111272] [13/14 Regression] Truncated error messages with -std=c++23 and -std=c++26

[mpolacek at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111272

Marek Polacek  changed:

   What|Removed |Added

 Ever confirmed|0   |1
   Assignee|unassigned at gcc dot gnu.org  |mpolacek at gcc dot 
gnu.org
Summary|Truncated error messages|[13/14 Regression]
   |with -std=c++23 and |Truncated error messages
   |-std=c++26  |with -std=c++23 and
   ||-std=c++26
   Last reconfirmed||2023-09-01
   Target Milestone|--- |13.3
 CC||mpolacek at gcc dot gnu.org
 Status|UNCONFIRMED |ASSIGNED
   Keywords||diagnostic

--- Comment #1 from Marek Polacek  ---
Started with r13-4112-gc85f8dbb173f45:

commit c85f8dbb173f45053f6d8849d27adc98d9668769
Author: Marek Polacek 
Date:   Wed Nov 2 13:11:02 2022 -0400

c++: P2448 - Relaxing some constexpr restrictions [PR106649]

[Bug target/111270] gcc/config/i386/i386-options.cc:3039:8: warning: duplicated ‘if ’ condition [-Wduplicated-cond]

[redi at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111270

--- Comment #3 from Jonathan Wakely  ---
The correct git blame output is:

105c2795b0d6 gcc/config/i386/i386-options.c  (Jan Hubicka  2019-07-23
09:35:18 + 3037)   if (ix86_tune_features [X86_TUNE_AVOID_256FMA_CHAINS])
eef81eefcdc2 gcc/config/i386/i386-options.cc (Jan Hubicka  2022-12-22
10:55:46 +0100 3038) SET_OPTION_IF_UNSET (opts, opts_set,
param_avoid_fma_max_bits, 512);
eef81eefcdc2 gcc/config/i386/i386-options.cc (Jan Hubicka  2022-12-22
10:55:46 +0100 3039)   else if (ix86_tune_features
[X86_TUNE_AVOID_256FMA_CHAINS])
028d40925205 gcc/config/i386/i386-options.c  (Martin Liska 2019-11-12
11:08:40 +0100 3040) SET_OPTION_IF_UNSET (opts, opts_set,
param_avoid_fma_max_bits, 256);
105c2795b0d6 gcc/config/i386/i386-options.c  (Jan Hubicka  2019-07-23
09:35:18 + 3041)   else if (ix86_tune_features
[X86_TUNE_AVOID_128FMA_CHAINS])
028d40925205 gcc/config/i386/i386-options.c  (Martin Liska 2019-11-12
11:08:40 +0100 3042) SET_OPTION_IF_UNSET (opts, opts_set,
param_avoid_fma_max_bits, 128);

[Bug target/111270] gcc/config/i386/i386-options.cc:3039:8: warning: duplicated ‘if ’ condition [-Wduplicated-cond]

[redi at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111270

--- Comment #2 from Jonathan Wakely  ---
This is nothing to do with me, g:d4ba3b369 doesn't touch that file at all, and
git blame doesn't show that commit for me. Do you have a shallow clone maybe,
and d4ba3b369 is simply the first commit in your history?

[Bug fortran/31059] bounds-check does not detect nonconforming assignment arrays

[cvs-commit at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=31059

--- Comment #12 from CVS Commits  ---
The master branch has been updated by Harald Anlauf :

https://gcc.gnu.org/g:6f06152541d62ae7c8579b7d7bf552be19e15b05

commit r14-3633-g6f06152541d62ae7c8579b7d7bf552be19e15b05
Author: Harald Anlauf 
Date:   Thu Aug 31 22:19:58 2023 +0200

Fortran: runtime bounds-checking in presence of array constructors
[PR31059]

gcc/fortran/ChangeLog:

PR fortran/31059
* trans-array.cc (gfc_conv_ss_startstride): For array bounds
checking,
consider also array constructors in expressions, and use their
shape.

gcc/testsuite/ChangeLog:

PR fortran/31059
* gfortran.dg/bounds_check_fail_5.f90: New test.

[Bug libstdc++/111050] [11/12/13/14 Regression] ABI break in _Hash_node_value_base since GCC 11

[fdumont at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111050

François Dumont  changed:

   What|Removed |Added

 CC||fdumont at gcc dot gnu.org

--- Comment #1 from François Dumont  ---
A 3 years old abi regression seems difficult to fix now. To do so we would need
to break abi again.

It seems to be a limited issue as you need a non-optimized build. The only
impacted member is the _M_next() which is a simple static_cast, I'm very
surprised that it's not always inlined even if non-optimized.

Apart perhaps documenting it I cannot think of anything to do.

[Bug tree-optimization/110817] [14 Regression] wrong code with vector compares and vector lowering

[pinskia at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110817

--- Comment #13 from Andrew Pinski  ---
(In reply to Zdenek Sojka from comment #12)
> Interesting, all the gcc's starting from gcc-6 up to gcc-13 at -O0 to -O3
> (including 32bit and 64bit targets) seem to "PASS" on this testcase.
> 
> ... wait, no, (unsigned)0 <= 0 is TRUE ...

Oh you are correct I read it incorrectly.
Anyways it is definitely a bug in veclower; looking into it.
  _1 = { -1 };
  _9 = VIEW_CONVERT_EXPR<>(_1);
  _10 = _9 != 0;
  _11 = _9 == 0;
  _12 = (unsigned long) _11;
  _13 = (unsigned long) _9;
  _14 = _13 + 18446744073709551615;
  _2 = {_14};

That produces:
_14 = _13 - 1; or rather _9 - 1 .
I suspect there is a missing check somewhere in match.pd really ...

[Bug analyzer/105948] RFE: analyzer could check c++ placement-new sizes

[vultkayn at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105948

--- Comment #3 from Benjamin Priour  ---
I believe the above patch resolves this PR.
I'm letting it sip in trunk for a few days before marking it as solved.

[Bug c++/111272] New: Truncated error messages with -std=c++23 and -std=c++26

[pkeir at outlook dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111272

Bug ID: 111272
   Summary: Truncated error messages with -std=c++23 and
-std=c++26
   Product: gcc
   Version: 14.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: pkeir at outlook dot com
  Target Milestone: ---

The C++14 code below requires the `constexpr` specifier on the `ft` member
function. Compiling this with `-std=c++14`, `-std=c++17` or `-std=c++20` will
provide a helpful error message which identifies the need to add the
`constexpr` specifier to `ft`.

However, if `-std=c++23`, or `-std=c++26` are used, no message indicating the
need for `constexpr` on `ft` is displayed; and the message itself is truncated:
ending only in "...not usable as a 'constexpr' function because:".

struct Jam
{
  // constexpr  // n.b.
  int ft() { return 42; }

  constexpr
  Jam() { ft(); }
};

constexpr bool test()
{
  Jam j;
  return true;
}

static_assert(test(), "");

The full truncated error message is:

$ /opt/gcc-latest/bin/g++ -std=c++26 -c truncated-constexpr-error-gcc.cpp 
truncated-constexpr-error-gcc.cpp:16:19: error: non-constant condition for
static assertion
   16 | static_assert(test(), "");
  |   ^~
truncated-constexpr-error-gcc.cpp:16:19:   in ‘constexpr’ expansion of ‘test()’
truncated-constexpr-error-gcc.cpp:12:7: error: ‘constexpr Jam::Jam()’ called in
a constant expression
   12 |   Jam j;
  |   ^
truncated-constexpr-error-gcc.cpp:7:3: note: ‘constexpr Jam::Jam()’ is not
usable as a ‘constexpr’ function because:
7 |   Jam() { ft(); }
  |   ^~~

[Bug analyzer/105948] RFE: analyzer could check c++ placement-new sizes

[cvs-commit at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105948

--- Comment #2 from CVS Commits  ---
The trunk branch has been updated by Benjamin Priour :

https://gcc.gnu.org/g:e7b267444045c507654a2a3f758efee5d5b550df

commit r14-3632-ge7b267444045c507654a2a3f758efee5d5b550df
Author: benjamin priour 
Date:   Fri Sep 1 00:01:29 2023 +0200

analyzer: Add support of placement new and improved operator new
[PR105948,PR94355]

Fixed spurious possibly-NULL warning always tagging along throwing
operator new despite it never returning NULL.
Now operator new is correctly recognized as possibly returning NULL
if and only if it is non-throwing or exceptions have been disabled.
Different standard signatures of operator new are now properly
recognized.

Added support of placement new, so that it is now properly recognized,
and a 'heap_allocated' region is no longer created for it.
Placement new size is also checked and a 'Wanalyzer-allocation-size'
is emitted when relevant, as well as always a 'Wanalyzer-out-of-bounds'.

'operator new' non-throwing variants are detected y checking the types
of the parameters.
Indeed, in a call to new (std::nothrow) () the chosen overload
has signature 'operator new (void*, std::nothrow_t&)', where the second
parameter is a reference. In a placement new, the second parameter will
always be a void pointer.

Prior to this patch, some buffers first allocated with 'new', then deleted
an thereafter used would result in a 'Wanalyzer-user-after-free'
warning. However the wording was "use after 'free'" instead of the
expected "use after 'delete'".
This patch fixes this by introducing a new kind of poisoned value,
namely POISON_KIND_DELETED.

Due to how the analyzer sees calls to non-throwing variants of
operator new, dereferencing a pointer freshly allocated in this fashion
caused both a 'Wanalyzer-use-of-uninitialized-value' and a
'Wanalyzer-null-dereference' to be emitted, while only the latter was
relevant. As a result, 'null-dereference' now supersedes
'use-of-uninitialized'.

Signed-off-by: benjamin priour 

gcc/analyzer/ChangeLog:

PR analyzer/105948
PR analyzer/94355
* analyzer.h (is_placement_new_p): New declaration.
* call-details.cc
(call_details::deref_ptr_arg): New function.
Dereference the argument at given index if possible.
* call-details.h: Declaration of the above function.
* kf-lang-cp.cc (is_placement_new_p): Returns true if the gcall
is recognized as a placement new.
(kf_operator_delete::impl_call_post): Unbinding a region and its
descendents now poisons with POISON_KIND_DELETED.
(register_known_functions_lang_cp): Known function "operator
delete" is now registered only once independently of its number of
arguments.
* region-model.cc (region_model::eval_condition): Now
recursively calls itself if any of the operand is wrapped in a
cast.
* sm-malloc.cc (malloc_state_machine::on_stmt):
Add placement new recognition.
* svalue.cc (poison_kind_to_str): Wording for the new PK.
* svalue.h (enum poison_kind): Add value POISON_KIND_DELETED.

gcc/testsuite/ChangeLog:

PR analyzer/105948
PR analyzer/94355
* g++.dg/analyzer/out-of-bounds-placement-new.C: Added a directive.
* g++.dg/analyzer/placement-new.C: Added tests.
* g++.dg/analyzer/new-2.C: New test.
* g++.dg/analyzer/noexcept-new.C: New test.
* g++.dg/analyzer/placement-new-size.C: New test.

[Bug analyzer/94355] analyzer support for C++ new expression

[cvs-commit at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94355

--- Comment #16 from CVS Commits  ---
The trunk branch has been updated by Benjamin Priour :

https://gcc.gnu.org/g:e7b267444045c507654a2a3f758efee5d5b550df

commit r14-3632-ge7b267444045c507654a2a3f758efee5d5b550df
Author: benjamin priour 
Date:   Fri Sep 1 00:01:29 2023 +0200

analyzer: Add support of placement new and improved operator new
[PR105948,PR94355]

Fixed spurious possibly-NULL warning always tagging along throwing
operator new despite it never returning NULL.
Now operator new is correctly recognized as possibly returning NULL
if and only if it is non-throwing or exceptions have been disabled.
Different standard signatures of operator new are now properly
recognized.

Added support of placement new, so that it is now properly recognized,
and a 'heap_allocated' region is no longer created for it.
Placement new size is also checked and a 'Wanalyzer-allocation-size'
is emitted when relevant, as well as always a 'Wanalyzer-out-of-bounds'.

'operator new' non-throwing variants are detected y checking the types
of the parameters.
Indeed, in a call to new (std::nothrow) () the chosen overload
has signature 'operator new (void*, std::nothrow_t&)', where the second
parameter is a reference. In a placement new, the second parameter will
always be a void pointer.

Prior to this patch, some buffers first allocated with 'new', then deleted
an thereafter used would result in a 'Wanalyzer-user-after-free'
warning. However the wording was "use after 'free'" instead of the
expected "use after 'delete'".
This patch fixes this by introducing a new kind of poisoned value,
namely POISON_KIND_DELETED.

Due to how the analyzer sees calls to non-throwing variants of
operator new, dereferencing a pointer freshly allocated in this fashion
caused both a 'Wanalyzer-use-of-uninitialized-value' and a
'Wanalyzer-null-dereference' to be emitted, while only the latter was
relevant. As a result, 'null-dereference' now supersedes
'use-of-uninitialized'.

Signed-off-by: benjamin priour 

gcc/analyzer/ChangeLog:

PR analyzer/105948
PR analyzer/94355
* analyzer.h (is_placement_new_p): New declaration.
* call-details.cc
(call_details::deref_ptr_arg): New function.
Dereference the argument at given index if possible.
* call-details.h: Declaration of the above function.
* kf-lang-cp.cc (is_placement_new_p): Returns true if the gcall
is recognized as a placement new.
(kf_operator_delete::impl_call_post): Unbinding a region and its
descendents now poisons with POISON_KIND_DELETED.
(register_known_functions_lang_cp): Known function "operator
delete" is now registered only once independently of its number of
arguments.
* region-model.cc (region_model::eval_condition): Now
recursively calls itself if any of the operand is wrapped in a
cast.
* sm-malloc.cc (malloc_state_machine::on_stmt):
Add placement new recognition.
* svalue.cc (poison_kind_to_str): Wording for the new PK.
* svalue.h (enum poison_kind): Add value POISON_KIND_DELETED.

gcc/testsuite/ChangeLog:

PR analyzer/105948
PR analyzer/94355
* g++.dg/analyzer/out-of-bounds-placement-new.C: Added a directive.
* g++.dg/analyzer/placement-new.C: Added tests.
* g++.dg/analyzer/new-2.C: New test.
* g++.dg/analyzer/noexcept-new.C: New test.
* g++.dg/analyzer/placement-new-size.C: New test.

[PATCH] analyzer: call off a superseding when diagnostics are unrelated [PR110830]

[Benjamin Priour via Gcc-patches]

From: benjamin priour 

Hi,

Patch succesfully regstrapped off trunk 7f2ed06ddc825e8a4e0edfd1d66b5156e6dc1d34
on x86_64-linux-gnu.

Is it OK for trunk ?

Thanks,
Benjamin.

Patch below.
---

Before this patch, a saved_diagnostic would supersede another at
the same statement if and only its vfunc supercedes_p returned true
for the other diagnostic's kind.
That both warning were unrelated, that is resolving one would not fix
the other was not considered in making the above choice.

This patch makes it so that two saved_diagnostics taking a different
outcome of at least one common conditional branching cannot supersede
each other.

Signed-off-by: benjamin priour 

gcc/analyzer/ChangeLog:

PR analyzer/110830
* diagnostic-manager.cc
(compatible_epaths_p): New function.
(saved_diagnostic::supercedes_p): Now calls the above
to determine if the diagnostics do overlap and the superseding
may proceed.

gcc/testsuite/ChangeLog:

PR analyzer/110830
* c-c++-common/analyzer/pr110830.c: New test.
---
 gcc/analyzer/diagnostic-manager.cc|  89 +-
 .../c-c++-common/analyzer/pr110830.c  | 111 ++
 2 files changed, 199 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/c-c++-common/analyzer/pr110830.c

diff --git a/gcc/analyzer/diagnostic-manager.cc 
b/gcc/analyzer/diagnostic-manager.cc
index 10fea486b8c..7cf181e7972 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -887,6 +887,87 @@ saved_diagnostic::add_duplicate (saved_diagnostic *other)
   m_duplicates.safe_push (other);
 }
 
+/* Walk up the two paths to each of their common conditional
+   branching.  At each branching, make sure both diagnostics'
+   paths branched similarly.  If there is at least one where
+   both paths go down a different outcome, then the paths
+   are incompatible and this function returns FALSE.
+   Otherwise return TRUE.
+
+   Incompatible paths:
+
+   
+   /  \
+  /\
+true  false
+ |  |
+......
+ |  |
+...   stmt x
+ |
+   stmt x
+
+   Both LHS_PATH and RHS_PATH final enodes should be
+   over the same gimple statement.  */
+
+static bool
+compatible_epath_p (const exploded_path *lhs_path,
+   const exploded_path *rhs_path)
+{
+  gcc_assert (lhs_path);
+  gcc_assert (rhs_path);
+  int i;
+  const exploded_edge *outer_eedge;
+  FOR_EACH_VEC_ELT_REVERSE (lhs_path->m_edges, i, outer_eedge)
+{
+  const superedge *outer_sedge = outer_eedge->m_sedge;
+  if (!outer_sedge || !outer_eedge->m_src)
+   continue;
+  const program_point _src_point = outer_eedge->m_src->get_point ();
+  switch (outer_src_point.get_kind ())
+   {
+ case PK_AFTER_SUPERNODE:
+   if (const cfg_superedge *cfg_outer_sedge
+   = outer_sedge->dyn_cast_cfg_superedge ())
+ {
+   int j;
+   const exploded_edge *inner_eedge;
+   FOR_EACH_VEC_ELT_REVERSE (rhs_path->m_edges, j, inner_eedge)
+ {
+   const superedge *inner_sedge = inner_eedge->m_sedge;
+   if (!inner_sedge || !inner_eedge->m_src)
+ continue;
+   const program_point _src_point
+ = inner_eedge->m_src->get_point ();
+   switch (inner_src_point.get_kind ())
+ {
+   case PK_AFTER_SUPERNODE:
+ if (inner_src_point.get_stmt ()
+ != outer_src_point.get_stmt ())
+   continue;
+ if (const cfg_superedge *cfg_inner_sedge
+ = inner_sedge->dyn_cast_cfg_superedge ())
+   {
+ if (cfg_inner_sedge->true_value_p ()
+ != cfg_outer_sedge->true_value_p ())
+   return false;
+   }
+ break;
+   default:
+ break;
+ }
+ }
+ }
+   break;
+
+ default:
+   break;
+   }
+}
+return true;
+}
+
+
 /* Return true if this diagnostic supercedes OTHER, and that OTHER should
therefore not be emitted.  */
 
@@ -896,7 +977,13 @@ saved_diagnostic::supercedes_p (const saved_diagnostic 
) const
   /* They should be at the same stmt.  */
   if (m_stmt != other.m_stmt)
 return false;
-  return m_d->supercedes_p (*other.m_d);
+  /* return early if OTHER won't be superseded anyway.  */
+  if (!m_d->supercedes_p (*other.m_d))
+return false;
+
+  /* If the two saved_diagnostics' path are not compatible
+ then they cannot supersede one another.  */
+  return compatible_epath_p (m_best_epath.get (), other.m_best_epath.get ());
 }
 
 /* 

[PATCH v2] RISC-V: zicond: Fix opt2 pattern

[Vineet Gupta]

This was tripping up gcc.c-torture/execute/pr60003.c at -O1 since in
failing case, pattern's asm czero.nez gets both rs2 and rs1 as non zero.

We start with the following src code snippet:

  if (a == 0)
return 0;
  else
return x;
}

which is equivalent to:  "x = (a != 0) ? x : a" where x is NOT 0.


and matches define_insn "*czero.nez..opt2"

| (insn 41 20 38 3 (set (reg/v:DI 136 [ x ])
|(if_then_else:DI (ne (reg/v:DI 134 [ a ])
|(const_int 0 [0]))
|(reg/v:DI 136 [ x ])
|(reg/v:DI 134 [ a ]))) {*czero.nez.didi.opt2}

The corresponding asm pattern generates
czero.nez x, x, a   ; %0, %2, %1

which implies
"x = (a != 0) ? 0 : a"

clearly not what the pattern wants to do.

Essentially "(a != 0) ? x : a" cannot be expressed with CZERO.nez if X
is not guaranteed to be 0.

However this can be fixed with a small tweak

"x = (a != 0) ? x : a"

   is same as

"x = (a == 0) ? a : x" since middle operand is 0 when a == 0.

which can be expressed with CZERO.eqz

before fix  after fix
-   -
lia5,1  lia5,1
lda4,8(sp)  lda4,8(sp)   # a4 is runtime non zero
czero.nez a0,a4,a5 # a0=0 NOK   czero.eqz a0,a4,a5   # a0=a4!=0 OK

The issue only happens at -O1 as at higher optimization levels, the
whole conditional move gets optimized away.

This fixes 4 testsuite failues in a zicond build:

FAIL: gcc.c-torture/execute/pr60003.c   -O1  execution test
FAIL: gcc.dg/setjmp-3.c execution test
FAIL: gcc.dg/torture/stackalign/setjmp-3.c   -O1  execution test
FAIL: gcc.dg/torture/stackalign/setjmp-3.c   -O1 -fpic execution test

gcc/ChangeLog:
* config/riscv/zicond.md: Fix op2 pattern.

Fixes: 1d5bc3285e8a ("[committed][RISC-V] Fix 20010221-1.c with zicond")
Signed-off-by: Vineet Gupta 
---
changes since v1
   - instead of discarding opt2 pattern, fix the asm
---
 gcc/config/riscv/zicond.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/config/riscv/zicond.md b/gcc/config/riscv/zicond.md
index 4619220ef8ac..1721e1011ea8 100644
--- a/gcc/config/riscv/zicond.md
+++ b/gcc/config/riscv/zicond.md
@@ -60,7 +60,7 @@
   (match_operand:GPR 2 "register_operand" "r")
   (match_operand:GPR 3 "register_operand" "1")))]
   "TARGET_ZICOND && rtx_equal_p (operands[1], operands[3])"
-  "czero.nez\t%0,%2,%1"
+  "czero.eqz\t%0,%2,%1"
 )
 
 ;; Combine creates this form in some cases (particularly the coremark
-- 
2.34.1

Re: [PATCH] RISC-V: zicond: remove bogus opt2 pattern

[Vineet Gupta]

On 9/1/23 10:40, Palmer Dabbelt wrote:


Just working through this in email, as there's a lot of 
double-negatives and I managed to screw up my Linux PR this morning so 
I may not be thinking that well...


The docs say "(if_then_else test true-value false-value)".  So in this 
case it's


   test:  (ne (match_operand:X 1 "register_operand" "r") (const_int 0))
   true:  (match_operand:GPR 2 "register_operand" "r")
   false: (match_operand:GPR 3 "register_operand" "1") == 
(match_operand:X 1 "register_operand" "r")


and we're encoding it as

   czero.nez %0,%2,%1

so that's

   rd:  output
   rs1: on-true
   rs2: condition (the value inside the ne in RTL)

That looks correct to me: the instruction's condition source register 
is inside a "(ne ... 0)", but we're doing the cmov.nez so it looks OK.


Yes it is fine, until you end up having both operand 2 and operand 3 
have non-zero values at runtime and somehow match this pattern Then the 
semantics of czero* are not honored.


It might be easier for everyone to understand if you add a specific 
testcase for just the broken codegen.  I'm not having luck 
constructing a small reproducer (though I don't have a clean tree 
lying around, so I might have screwed something up here).


IIUC something like

   long func(long x, long a) {
   if (a != 0)
 return x;
   return 0;
   }

should do it, but I'm getting

   func:
   czero.eqz   a0,a0,a1
   ret


Unfortunately tests any simpler don't trigger it - they code seqs just 
get optimized away - otherwise Jeff would have found this 3 weeks ago ;-)

Just use gcc/testsuite/gcc.c-torture/execute/pr60003.c

Thx,
-Vineet

[PATCH] diagnostics: Delete config pointer before overwriting it.

[Mikael Morin via Gcc-patches]

Hello,

this is a fix for a small memory leak in the fortran frontend.
Tested on x86_64-pc-linux-gnu, nothing stands out besides the
apparently well-known guality instability.
OK for master ?

-- >8 --

Delete m_client_data_hooks before it is reassigned in
tree_diagnostics_defaults.  This fixes a small memory leak in the fortran
frontend, which restores the diagnostics configurations to their default
values with a call to tree_diagnostics_defaults at the end of the main parse
hook.

gcc/ChangeLog:

* tree-diagnostic.cc (tree_diagnostics_defaults): Delete allocated
pointer before overwriting it.
---
 gcc/tree-diagnostic.cc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gcc/tree-diagnostic.cc b/gcc/tree-diagnostic.cc
index 731e3559cd8..d2f6637b6d9 100644
--- a/gcc/tree-diagnostic.cc
+++ b/gcc/tree-diagnostic.cc
@@ -377,5 +377,6 @@ tree_diagnostics_defaults (diagnostic_context *context)
   context->print_path = default_tree_diagnostic_path_printer;
   context->make_json_for_path = default_tree_make_json_for_path;
   context->set_locations_cb = set_inlining_locations;
+  delete context->m_client_data_hooks;
   context->m_client_data_hooks = make_compiler_data_hooks ();
 }
-- 
2.40.1

[Bug testsuite/111264] [14 regression] gcc.dg/plugin/analyzer_cpython_plugin.c breaks after r14-3580-g597b9ec69bca8a

[hp at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111264

Hans-Peter Nilsson  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|NEW |RESOLVED

--- Comment #8 from Hans-Peter Nilsson  ---
.

[Bug testsuite/111264] [14 regression] gcc.dg/plugin/analyzer_cpython_plugin.c breaks after r14-3580-g597b9ec69bca8a

[hp at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111264

--- Comment #7 from Hans-Peter Nilsson  ---
I don't think it's worthwhile to keep this open, as I'm pretty sure I fixed it
for all targets, as the cause wasn't target-related.  Otherwise, reopen; if
adding a sarcastic comment, then preferably also with a humorous twist.

[PATCH v2] c: don't emit -Wmissing-variable-declarations for register variables [PR110947]

[Hamza Mahfooz]

Resolves:
PR c/110947 - Should -Wmissing-variable-declarations not trigger on
register variables?

gcc/c/ChangeLog:

PR c/110947
* c-decl.cc (start_decl): don't emit
-Wmissing-variable-declarations for DECL_REGISTER VAR_DECLs.

gcc/testsuite/ChangeLog:

PR c/110947
* gcc.dg/pr110947.c: New test.

Signed-off-by: Hamza Mahfooz 
---
Please push this for me if you think it looks good. Since, I don't have
write access to the repository.

v2: put "target" before the relevant architectures in pr110947.c.
---
 gcc/c/c-decl.cc | 3 ++-
 gcc/testsuite/gcc.dg/pr110947.c | 4 
 2 files changed, 6 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr110947.c

diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 1f9eb44dbaa..819af6aa050 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -5376,7 +5376,8 @@ start_decl (struct c_declarator *declarator, struct 
c_declspecs *declspecs,
 warning (OPT_Wmain, "%q+D is usually a function", decl);
 
   if (warn_missing_variable_declarations && VAR_P (decl)
-  && !DECL_EXTERNAL (decl) && TREE_PUBLIC (decl) && old_decl == NULL_TREE)
+  && !DECL_EXTERNAL (decl) && !DECL_REGISTER (decl) && TREE_PUBLIC (decl)
+  && old_decl == NULL_TREE)
 warning_at (DECL_SOURCE_LOCATION (decl), 
OPT_Wmissing_variable_declarations,
"no previous declaration for %qD", decl);
 
diff --git a/gcc/testsuite/gcc.dg/pr110947.c b/gcc/testsuite/gcc.dg/pr110947.c
new file mode 100644
index 000..3c0b8a82ab3
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr110947.c
@@ -0,0 +1,4 @@
+/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-Wmissing-variable-declarations" } */
+
+register unsigned long current_stack_pointer asm("rsp");
-- 
2.41.0

[Bug testsuite/111264] [14 regression] gcc.dg/plugin/analyzer_cpython_plugin.c breaks after r14-3580-g597b9ec69bca8a

[cvs-commit at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111264

--- Comment #6 from CVS Commits  ---
The master branch has been updated by Hans-Peter Nilsson :

https://gcc.gnu.org/g:d3dd69706af4c086cb3385ff1f321887b91f49fb

commit r14-3631-gd3dd69706af4c086cb3385ff1f321887b91f49fb
Author: Hans-Peter Nilsson 
Date:   Fri Sep 1 04:36:03 2023 +0200

testsuite: Fix analyzer_cpython_plugin.c declarations, PR testsuite/111264

Also, add missing newline at end of file.

PR testsuite/111264
* gcc.dg/plugin/analyzer_cpython_plugin.c: Make declarations
C++11-compatible.

[Bug testsuite/111264] [14 regression] gcc.dg/plugin/analyzer_cpython_plugin.c breaks after r14-3580-g597b9ec69bca8a

[hp at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111264

Hans-Peter Nilsson  changed:

   What|Removed |Added

  Component|analyzer|testsuite
   Assignee|efric at gcc dot gnu.org   |hp at gcc dot gnu.org

--- Comment #5 from Hans-Peter Nilsson  ---
I'm taking it as I'm about to commit the approved patch.
Also, changing component to testsuite.

Re: [PATCH] RISC-V: zicond: remove bogus opt2 pattern

[Vineet Gupta]

On 9/1/23 06:13, Jeff Law wrote:
I could very well be mistaken, but define_insn is a pattern match and 
opt2 has *ne* so the expression has to be in != form and thus needs 
to work with that condition. No ?

My point was  that

x = (a != 0) ? x : 0

is equivalent to

x = (a == 0) ? 0 : x

You can invert the condition and swap the arms and get the same 
semantics.  Thus if one can be supported, so can the other as they're 
functionally equivalent. 


Ah I see what you mean. Indeed the pattern is fine, it just doesn't map 
to the right asm.

So we certainly need a fix but it could just very well be this:

(define_insn "*czero.nez..opt2"
  [(set (match_operand:GPR 0 "register_operand" "=r")
    (if_then_else:GPR (ne (match_operand:X 1 "register_operand" "r")
  (const_int 0))
  (match_operand:GPR 2 "register_operand" "r")
  (match_operand:GPR 3 "register_operand" "1")))]
  "TARGET_ZICOND && rtx_equal_p (operands[1], operands[3])"
-  "czero.nez\t%0,%2,%1"
}  "czero.eqz\t%0,%2,%1"
)

It may be the at we've goof'd something in handling the inverted case, 
but conceptually we ought to be able to handle both.


Indeed there's a small goof as shown above.



I don't doubt you've got a failure, but it's also the case that I'm 
not seeing the same failure when I turn on zicond and run the 
execute.exp tests.  So clearly there's a difference somewhere in what 
we're doing.


It doesn't show up in execute.exp but as following (perhaps I should add 
that to commit log too).


FAIL: gcc.c-torture/execute/pr60003.c   -O1  execution test
FAIL: gcc.dg/setjmp-3.c execution test
FAIL: gcc.dg/torture/stackalign/setjmp-3.c   -O1  execution test
FAIL: gcc.dg/torture/stackalign/setjmp-3.c   -O1 -fpic execution test




So perhaps we should start with comparing assembly output for the test 
in question.  Can you pass yours along, I'll diff them this afternoon 
and see what we find.


Attached is slightly modified pr60003.c (to differentiate 'X' and 'a') 
and the failing asm and with fix (both the deleted pattern and modified 
pattern produce correct, if slightly different code).


Thx,
-Vineet/* PR tree-optimization/60003 */
/* { dg-require-effective-target indirect_jumps } */

extern void abort (void);

unsigned long long jmp_buf[5];

__attribute__((noinline, noclone)) void
baz (void)
{
  __builtin_longjmp (_buf, 1);
}

void
bar (void)
{
  baz ();
}

__attribute__((noinline, noclone)) int
foo (int x)
{
  int a = 0;

  if (__builtin_setjmp (_buf) == 0)
{
  while (1)
	{
	  a = 1;
	  bar ();  /* OK if baz () instead */
	}
}
  else
{
  if (a == 0)
	return 0;
  else
	return x;
}
}

int
main ()
{
  if (foo (2) == 0)	// orig test has foo (1)
return 1;

  return 0;
}
.file   "pr60003.c"
.option nopic
# GNU C17 (GCC) version 14.0.0 20230830 (experimental) (riscv-unknown-elf)
#   compiled by GNU C version 11.4.0, GMP version 6.1.0, MPFR version 
3.1.4, MPC version 1.0.3, isl version isl-0.18-GMP

# GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
# options passed: -march=rv64gc_zba_zbb_zbs_zicond -mabi=lp64d -O1
.text
.align  1
.globl  baz
.type   baz, @function
baz:
addisp,sp,-16   #,,
sd  ra,8(sp)#,
sd  s0,0(sp)#,
addis0,sp,16#,,
# pr60003.c:11:   __builtin_longjmp (_buf, 1);
lui a5,%hi(.LANCHOR0)   # tmp135,
addia5,a5,%lo(.LANCHOR0)# tmp134, tmp135,
ld  a4,8(a5)# tmp136,
ld  a3,0(a5)# tmp137,
ld  sp,16(a5)   #,
mv  s0,a3   #, tmp137
jr  a4  # tmp136
.size   baz, .-baz
.align  1
.globl  bar
.type   bar, @function
bar:
addisp,sp,-16   #,,
sd  ra,8(sp)#,
# pr60003.c:17:   baz ();
callbaz #
.size   bar, .-bar
.align  1
.globl  foo
.type   foo, @function
foo:
addisp,sp,-224  #,,
sd  ra,216(sp)  #,
sd  s0,208(sp)  #,
sd  s1,200(sp)  #,
sd  s2,192(sp)  #,
sd  s3,184(sp)  #,
sd  s4,176(sp)  #,
sd  s5,168(sp)  #,
sd  s6,160(sp)  #,
sd  s7,152(sp)  #,
sd  s8,144(sp)  #,
sd  s9,136(sp)  #,
sd  s10,128(sp) #,
sd  s11,120(sp) #,
fsd fs0,104(sp) #,
fsd fs1,96(sp)  #,
fsd fs2,88(sp)  #,
fsd fs3,80(sp)  #,
fsd fs4,72(sp)  #,
fsd fs5,64(sp)  #,
fsd fs6,56(sp)  #,
fsd fs7,48(sp)  #,
fsd fs8,40(sp)  #,
fsd fs9,32(sp)  #,
fsd fs10,24(sp) 

[Bug tree-optimization/111262] [14 Regression] error: count of bb not initialized with -O3

[dcb314 at hotmail dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111262

--- Comment #4 from David Binderman  ---
$ git bisect bad 0c78240fd7d519fc
0c78240fd7d519fc27ca822f66a92f85edf43f70 is the first bad commit
commit 0c78240fd7d519fc27ca822f66a92f85edf43f70
Author: Jan Hubicka 
Date:   Thu Aug 24 15:10:46 2023 +0200

Check that passes do not forget to define profile

[PATCH] c: don't emit -Wmissing-variable-declarations for register variables [PR110947]

[Hamza Mahfooz]

Resolves:
PR c/110947 - Should -Wmissing-variable-declarations not trigger on
register variables?

gcc/c/ChangeLog:

PR c/110947
* c-decl.cc (start_decl): don't emit
-Wmissing-variable-declarations for DECL_REGISTER VAR_DECLs.

gcc/testsuite/ChangeLog:

PR c/110947
* gcc.dg/pr110947.c: New test.

Signed-off-by: Hamza Mahfooz 
---
Please push this for me if you think it looks good. Since, I don't have
write access to the repository.
---
 gcc/c/c-decl.cc | 3 ++-
 gcc/testsuite/gcc.dg/pr110947.c | 4 
 2 files changed, 6 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr110947.c

diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 1f9eb44dbaa..819af6aa050 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -5376,7 +5376,8 @@ start_decl (struct c_declarator *declarator, struct 
c_declspecs *declspecs,
 warning (OPT_Wmain, "%q+D is usually a function", decl);
 
   if (warn_missing_variable_declarations && VAR_P (decl)
-  && !DECL_EXTERNAL (decl) && TREE_PUBLIC (decl) && old_decl == NULL_TREE)
+  && !DECL_EXTERNAL (decl) && !DECL_REGISTER (decl) && TREE_PUBLIC (decl)
+  && old_decl == NULL_TREE)
 warning_at (DECL_SOURCE_LOCATION (decl), 
OPT_Wmissing_variable_declarations,
"no previous declaration for %qD", decl);
 
diff --git a/gcc/testsuite/gcc.dg/pr110947.c b/gcc/testsuite/gcc.dg/pr110947.c
new file mode 100644
index 000..19e38ed4d18
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr110947.c
@@ -0,0 +1,4 @@
+/* { dg-do compile { i?86-*-* target x86_64-*-* } } */
+/* { dg-options "-Wmissing-variable-declarations" } */
+
+register unsigned long current_stack_pointer asm("rsp");
-- 
2.41.0

[Bug tree-optimization/111262] [14 Regression] error: count of bb not initialized with -O3

[dcb314 at hotmail dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111262

--- Comment #3 from David Binderman  ---
I am trying a bisection. Current range is g:d96659e34cdcf436
to g:603bdf906af6d42c, which is 11 commits.

Re: [PATCH] RISC-V: Add Types to Un-Typed Risc-v Instructions:

[Jeff Law via Gcc-patches]

On 8/31/23 11:32, Edwin Lu wrote:

Related Discussion:
https://inbox.sourceware.org/gcc-patches/12fb5088-3f28-0a69-de1e-f387371a5...@gmail.com/

This patch updates the riscv instructions to ensure that no insn is left
without a type attribute. Added new types: "trap" (self explanatory) and "cbo"
(for cache related instructions)

Tested for regressions using rv32/64 multilib for linux/newlib. Also tested
rv32/64 gcv for linux.

gcc/Changelog:

* config/riscv/riscv.md: Update/Add types

OK.

jeff

Re: [PATCH] RISC-V: zicond: remove bogus opt2 pattern

[Palmer Dabbelt]

On Thu, 31 Aug 2023 10:57:52 PDT (-0700), Vineet Gupta wrote:



On 8/31/23 06:51, Jeff Law wrote:



On 8/30/23 15:57, Vineet Gupta wrote:

This was tripping up gcc.c-torture/execute/pr60003.c at -O1 since the
pattern semantics can't be expressed by zicond instructions.

This involves test code snippet:

   if (a == 0)
return 0;
   else
return x;
 }

which is equivalent to:  "x = (a != 0) ? x : a"

Isn't it

x = (a == 0) ? 0 : x

Which seems like it ought to fit zicond just fine.


Logically they are equivalent, but 



If we take yours;

x = (a != 0) ? x : a

And simplify with the known value of a on the false arm we get:

x = (a != 0 ) ? x : 0;

Which is equivalent to

x = (a == 0) ? 0 : x;

So ISTM this does fit zicond just fine.


I could very well be mistaken, but define_insn is a pattern match and
opt2 has *ne* so the expression has to be in != form and thus needs to
work with that condition. No ?


and matches define_insn "*czero.nez..opt2"

| (insn 41 20 38 3 (set (reg/v:DI 136 [ x ])
|    (if_then_else:DI (ne (reg/v:DI 134 [ a ])
|    (const_int 0 [0]))
|    (reg/v:DI 136 [ x ])
|    (reg/v:DI 134 [ a ]))) {*czero.nez.didi.opt2}

The corresponding asm pattern generates
 czero.nez x, x, a   ; %0, %2, %1
implying
 "x = (a != 0) ? 0 : a"

I get this from the RTL pattern:

x = (a != 0) ? x : a
x = (a != 0) ? x : 0


This is the issue, for ne, czero.nez can only express
x = (a != 0) ? 0 : x



I think you got the arms reversed.


Just working through this in email, as there's a lot of 
double-negatives and I managed to screw up my Linux PR this morning so I 
may not be thinking that well...


The docs say "(if_then_else test true-value false-value)".  So in this 
case it's


   test:  (ne (match_operand:X 1 "register_operand" "r") (const_int 0))
   true:  (match_operand:GPR 2 "register_operand" "r")
   false: (match_operand:GPR 3 "register_operand" "1") == (match_operand:X 1 
"register_operand" "r")

and we're encoding it as

   czero.nez %0,%2,%1

so that's

   rd:  output
   rs1: on-true
   rs2: condition (the value inside the ne in RTL)

That looks correct to me: the instruction's condition source register is 
inside a "(ne ... 0)", but we're doing the cmov.nez so it looks OK.


The rest of the zero juggling looks sane as well -- I'm not sure if the 
X vs GPR mismatch will confuse something else, but it should be caught 
by the rtx_equal_p() and thus should at least be safe.



What I meant was czero.nez as specified in RTL pattern would generate x
= (a != 0) ? 0 : a, whereas pattern's desired semantics is (a != 0) ? x : 0
And that is a problem because after all equivalents/simplifications, a
ternary operation's middle operand has to be zero to map to czero*, but
it doesn't for the opt2 RTL semantics.

I've sat on this for 2 days, trying to convince myself I was wrong, but
as it stands, it was generating wrong code in the test which is fixed
after the patch.


It might be easier for everyone to understand if you add a specific 
testcase for just the broken codegen.  I'm not having luck constructing 
a small reproducer (though I don't have a clean tree lying around, so I 
might have screwed something up here).


IIUC something like

   long func(long x, long a) {
   if (a != 0)
 return x;
   return 0;
   }

should do it, but I'm getting

   func:
   czero.eqz   a0,a0,a1
   ret

which looks right to me -- though it's not triggering this pattern, so 
not sure that means much.




Thx,
-Vineet

Re: [PATCH] c++: Move consteval folding to cp_fold_r

[Marek Polacek via Gcc-patches]

On Fri, Sep 01, 2023 at 01:23:48PM -0400, Marek Polacek via Gcc-patches wrote:
> --- a/gcc/cp/cp-gimplify.cc
> +++ b/gcc/cp/cp-gimplify.cc
[...]
>  case ADDR_EXPR:
>if (TREE_CODE (TREE_OPERAND (stmt, 0)) == FUNCTION_DECL
> -   && DECL_IMMEDIATE_FUNCTION_P (TREE_OPERAND (stmt, 0)))
> +   && DECL_IMMEDIATE_FUNCTION_P (TREE_OPERAND (stmt, 0))
> +   && !in_immediate_context ())

This hunk isn't actually necessary.  I'm happy to drop it.  Or add the
in_immediate_context check into case PTRMEM_CST too.

Marek

[PATCH 2/2] VR-VALUES: Rewrite test_for_singularity using range_op_handler

[Andrew Pinski via Gcc-patches]

So it turns out there was a simplier way of starting to
improve VRP to start to fix PR 110131, PR 108360, and PR 108397.
That was rewrite test_for_singularity to use range_op_handler
and Value_Range.

This patch implements that and

OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

gcc/ChangeLog:

* vr-values.cc (test_for_singularity): Add edge argument
and rewrite using range_op_handler.
(simplify_compare_using_range_pairs): Use Value_Range
instead of value_range and update test_for_singularity call.

gcc/testsuite/ChangeLog:

* gcc.dg/tree-ssa/vrp124.c: New test.
* gcc.dg/tree-ssa/vrp125.c: New test.
---
 gcc/testsuite/gcc.dg/tree-ssa/vrp124.c | 44 
 gcc/testsuite/gcc.dg/tree-ssa/vrp125.c | 44 
 gcc/vr-values.cc   | 99 --
 3 files changed, 117 insertions(+), 70 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/vrp124.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/vrp125.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp124.c 
b/gcc/testsuite/gcc.dg/tree-ssa/vrp124.c
new file mode 100644
index 000..6ccbda35d1b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp124.c
@@ -0,0 +1,44 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+/* Should be optimized to a == -100 */
+int g(int a)
+{
+  if (a == -100 || a >= 0)
+;
+  else
+return 0;
+  return a < 0;
+}
+
+/* Should optimize to a == 0 */
+int f(int a)
+{
+  if (a == 0 || a > 100)
+;
+  else
+return 0;
+  return a < 50;
+}
+
+/* Should be optimized to a == 0. */
+int f2(int a)
+{
+  if (a == 0 || a > 100)
+;
+  else
+return 0;
+  return a < 100;
+}
+
+/* Should optimize to a == 100 */
+int f1(int a)
+{
+  if (a < 0 || a == 100)
+;
+  else
+return 0;
+  return a > 50;
+}
+
+/* { dg-final { scan-tree-dump-not "goto " "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp125.c 
b/gcc/testsuite/gcc.dg/tree-ssa/vrp125.c
new file mode 100644
index 000..f6c2f8e35f1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp125.c
@@ -0,0 +1,44 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+/* Should be optimized to a == -100 */
+int g(int a)
+{
+  if (a == -100 || a == -50 || a >= 0)
+;
+  else
+return 0;
+  return a < -50;
+}
+
+/* Should optimize to a == 0 */
+int f(int a)
+{
+  if (a == 0 || a == 50 || a > 100)
+;
+  else
+return 0;
+  return a < 50;
+}
+
+/* Should be optimized to a == 0. */
+int f2(int a)
+{
+  if (a == 0 || a == 50 || a > 100)
+;
+  else
+return 0;
+  return a < 25;
+}
+
+/* Should optimize to a == 100 */
+int f1(int a)
+{
+  if (a < 0 || a == 50 || a == 100)
+;
+  else
+return 0;
+  return a > 50;
+}
+
+/* { dg-final { scan-tree-dump-not "goto " "optimized" } } */
diff --git a/gcc/vr-values.cc b/gcc/vr-values.cc
index 52ab4fe6109..2474e57ee90 100644
--- a/gcc/vr-values.cc
+++ b/gcc/vr-values.cc
@@ -904,69 +904,33 @@ simplify_using_ranges::simplify_bit_ops_using_ranges
 }
 
 /* We are comparing trees OP1 and OP2 using COND_CODE.  OP1 has
-   a known value range VR.
+   a known value range OP1_RANGE.
 
If there is one and only one value which will satisfy the
-   conditional, then return that value.  Else return NULL.
-
-   If signed overflow must be undefined for the value to satisfy
-   the conditional, then set *STRICT_OVERFLOW_P to true.  */
+   conditional on the EDGE, then return that value.
+   Else return NULL.  */
 
 static tree
 test_for_singularity (enum tree_code cond_code, tree op1,
- tree op2, const value_range *vr)
+ tree op2, const int_range_max _range, bool edge)
 {
-  tree min = NULL;
-  tree max = NULL;
-
-  /* Extract minimum/maximum values which satisfy the conditional as it was
- written.  */
-  if (cond_code == LE_EXPR || cond_code == LT_EXPR)
+  /* This is already a singularity.  */
+  if (cond_code == NE_EXPR || cond_code == EQ_EXPR)
+return NULL;
+  auto range_op = range_op_handler (cond_code);
+  wide_int w = wi::to_wide (op2);
+  int_range<1> op2_range (TREE_TYPE (op2), w, w);
+  int_range_max vr;
+  if (range_op.op1_range (vr, TREE_TYPE (op1),
+ edge ? range_true () : range_false (),
+ op2_range))
 {
-  min = TYPE_MIN_VALUE (TREE_TYPE (op1));
-
-  max = op2;
-  if (cond_code == LT_EXPR)
-   {
- tree one = build_int_cst (TREE_TYPE (op1), 1);
- max = fold_build2 (MINUS_EXPR, TREE_TYPE (op1), max, one);
- /* Signal to compare_values_warnv this expr doesn't overflow.  */
- if (EXPR_P (max))
-   suppress_warning (max, OPT_Woverflow);
-   }
-}
-  else if (cond_code == GE_EXPR || cond_code == GT_EXPR)
-{
-  max = TYPE_MAX_VALUE (TREE_TYPE (op1));
-
-  min = op2;
-  if (cond_code == GT_EXPR)
-   {
- tree one = build_int_cst 

[PATCH 1/2] VR-VALUES: Rename op0/op1 to op1/op2 for test_for_singularity

[Andrew Pinski via Gcc-patches]

As requested and make easier to understand with the new ranger
code, rename the arguments op0/op1 to op1/op2.

OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions

gcc/ChangeLog:

* vr-values.cc (test_for_singularity): Rename
arguments op0/op1 to op1/op2.
---
 gcc/vr-values.cc | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/gcc/vr-values.cc b/gcc/vr-values.cc
index a4fddd62841..52ab4fe6109 100644
--- a/gcc/vr-values.cc
+++ b/gcc/vr-values.cc
@@ -903,7 +903,7 @@ simplify_using_ranges::simplify_bit_ops_using_ranges
   return true;
 }
 
-/* We are comparing trees OP0 and OP1 using COND_CODE.  OP0 has
+/* We are comparing trees OP1 and OP2 using COND_CODE.  OP1 has
a known value range VR.
 
If there is one and only one value which will satisfy the
@@ -913,8 +913,8 @@ simplify_using_ranges::simplify_bit_ops_using_ranges
the conditional, then set *STRICT_OVERFLOW_P to true.  */
 
 static tree
-test_for_singularity (enum tree_code cond_code, tree op0,
- tree op1, const value_range *vr)
+test_for_singularity (enum tree_code cond_code, tree op1,
+ tree op2, const value_range *vr)
 {
   tree min = NULL;
   tree max = NULL;
@@ -923,13 +923,13 @@ test_for_singularity (enum tree_code cond_code, tree op0,
  written.  */
   if (cond_code == LE_EXPR || cond_code == LT_EXPR)
 {
-  min = TYPE_MIN_VALUE (TREE_TYPE (op0));
+  min = TYPE_MIN_VALUE (TREE_TYPE (op1));
 
-  max = op1;
+  max = op2;
   if (cond_code == LT_EXPR)
{
- tree one = build_int_cst (TREE_TYPE (op0), 1);
- max = fold_build2 (MINUS_EXPR, TREE_TYPE (op0), max, one);
+ tree one = build_int_cst (TREE_TYPE (op1), 1);
+ max = fold_build2 (MINUS_EXPR, TREE_TYPE (op1), max, one);
  /* Signal to compare_values_warnv this expr doesn't overflow.  */
  if (EXPR_P (max))
suppress_warning (max, OPT_Woverflow);
@@ -937,13 +937,13 @@ test_for_singularity (enum tree_code cond_code, tree op0,
 }
   else if (cond_code == GE_EXPR || cond_code == GT_EXPR)
 {
-  max = TYPE_MAX_VALUE (TREE_TYPE (op0));
+  max = TYPE_MAX_VALUE (TREE_TYPE (op1));
 
-  min = op1;
+  min = op2;
   if (cond_code == GT_EXPR)
{
- tree one = build_int_cst (TREE_TYPE (op0), 1);
- min = fold_build2 (PLUS_EXPR, TREE_TYPE (op0), min, one);
+ tree one = build_int_cst (TREE_TYPE (op1), 1);
+ min = fold_build2 (PLUS_EXPR, TREE_TYPE (op1), min, one);
  /* Signal to compare_values_warnv this expr doesn't overflow.  */
  if (EXPR_P (min))
suppress_warning (min, OPT_Woverflow);
@@ -951,10 +951,10 @@ test_for_singularity (enum tree_code cond_code, tree op0,
 }
 
   /* Now refine the minimum and maximum values using any
- value range information we have for op0.  */
+ value range information we have for op1.  */
   if (min && max)
 {
-  tree type = TREE_TYPE (op0);
+  tree type = TREE_TYPE (op1);
   tree tmin = wide_int_to_tree (type, vr->lower_bound ());
   tree tmax = wide_int_to_tree (type, vr->upper_bound ());
   if (compare_values (tmin, min) == 1)
-- 
2.31.1

[Bug middle-end/111243] The -Og option inlines functions, making for a poor debugging experience.

[xry111 at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111243

--- Comment #11 from Xi Ruoyao  ---
(In reply to Alex Mohr from comment #10)
> (In reply to Xi Ruoyao from comment #9)
> > I believe the only real issue is imprecise documentation: "It is a better
> > choice than -O0" has some caveats and it's not always true.
> 
> Is there a way to explicitly enable the compiler passes that collect debug
> info that are disabled at -O0?

I don't think so, many -fxxx optimize options have no effect at -O0.

But if you are using -Og you can add -fno-inline if you really don't want
inlining.

[PATCH] c++: Move consteval folding to cp_fold_r

[Marek Polacek via Gcc-patches]

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --

In the review of P2564:

it turned out that in order to correctly handle an example in the paper,
we should stop doing immediate evaluation in build_over_call and
bot_replace, and instead do it in cp_fold_r.  This patch does that.

Another benefit is that this is a pretty significant simplification, at
least in my opinion.  Also, this fixes the c++/110997 ICE (but the test
doesn't compile yet).

The main drawback seems to be that cp_fold_r doesn't process as much
code as we did before: uninstantiated templates and things like
"false ? foo () : 1".

You'll see that I've reintroduced ADDR_EXPR_DENOTES_CALL_P here.  This
is to detect

  *()) ()
  (s.*::foo) ()

which were deemed ill-formed.

gcc/cp/ChangeLog:

* call.cc (in_immediate_context): No longer static.
(build_over_call): Set ADDR_EXPR_DENOTES_CALL_P.  Don't handle
immediate_invocation_p here.
* constexpr.cc (cxx_eval_call_expression): Use mce_true for
immediate_invocation_p.
* cp-gimplify.cc (cp_fold_r): Expand immediate invocations.
* cp-tree.h (ADDR_EXPR_DENOTES_CALL_P): Define.
(immediate_invocation_p): Declare.
* tree.cc (bot_replace): Don't handle immediate invocations here.

gcc/testsuite/ChangeLog:

* g++.dg/cpp23/consteval-if2.C: Add xfail.
* g++.dg/cpp2a/consteval-memfn1.C: Adjust.
* g++.dg/cpp2a/consteval11.C: Remove dg-message.
* g++.dg/cpp2a/consteval3.C: Remove dg-message and dg-error.
* g++.dg/cpp2a/consteval9.C: Remove dg-message.
* g++.dg/cpp2a/consteval32.C: New test.
* g++.dg/cpp2a/consteval33.C: New test.

libstdc++-v3/ChangeLog:

* testsuite/20_util/allocator/105975.cc: Add dg-error.
---
 gcc/cp/call.cc| 42 +++
 gcc/cp/constexpr.cc   |  5 +++
 gcc/cp/cp-gimplify.cc | 14 ++-
 gcc/cp/cp-tree.h  |  6 +++
 gcc/cp/tree.cc| 23 +-
 gcc/testsuite/g++.dg/cpp23/consteval-if2.C|  2 +-
 gcc/testsuite/g++.dg/cpp2a/consteval-memfn1.C |  7 
 gcc/testsuite/g++.dg/cpp2a/consteval11.C  | 37 
 gcc/testsuite/g++.dg/cpp2a/consteval3.C   |  3 +-
 gcc/testsuite/g++.dg/cpp2a/consteval32.C  |  4 ++
 gcc/testsuite/g++.dg/cpp2a/consteval33.C  | 34 +++
 gcc/testsuite/g++.dg/cpp2a/consteval9.C   |  2 +-
 .../testsuite/20_util/allocator/105975.cc |  2 +-
 13 files changed, 100 insertions(+), 81 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/consteval32.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/consteval33.C

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 40d9fdc0516..abdbc8fff8c 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -9763,7 +9763,7 @@ in_immediate_context ()
 /* Return true if a call to FN with number of arguments NARGS
is an immediate invocation.  */
 
-static bool
+bool
 immediate_invocation_p (tree fn)
 {
   return (TREE_CODE (fn) == FUNCTION_DECL
@@ -10471,6 +10471,10 @@ build_over_call (struct z_candidate *cand, int flags, 
tsubst_flags_t complain)
   fn = build_addr_func (fn, complain);
   if (fn == error_mark_node)
return error_mark_node;
+
+  /* We're actually invoking the function.  (Immediate functions get an
+& when invoking it even though the user didn't use &.)  */
+  ADDR_EXPR_DENOTES_CALL_P (fn) = true;
 }
 
   tree call = build_cxx_call (fn, nargs, argarray, complain|decltype_flag);
@@ -10488,41 +10492,7 @@ build_over_call (struct z_candidate *cand, int flags, 
tsubst_flags_t complain)
   if (TREE_CODE (c) == CALL_EXPR)
suppress_warning (c /* Suppress all warnings.  */);
 }
-  if (TREE_CODE (fn) == ADDR_EXPR)
-{
-  tree fndecl = STRIP_TEMPLATE (TREE_OPERAND (fn, 0));
-  if (immediate_invocation_p (fndecl))
-   {
- tree obj_arg = NULL_TREE;
- /* Undo convert_from_reference called by build_cxx_call.  */
- if (REFERENCE_REF_P (call))
-   call = TREE_OPERAND (call, 0);
- if (DECL_CONSTRUCTOR_P (fndecl))
-   obj_arg = cand->first_arg ? cand->first_arg : (*args)[0];
- if (obj_arg && is_dummy_object (obj_arg))
-   {
- call = build_cplus_new (DECL_CONTEXT (fndecl), call, complain);
- obj_arg = NULL_TREE;
-   }
- /* Look through *(const T *)  */
- else if (obj_arg && INDIRECT_REF_P (obj_arg))
-   {
- tree addr = TREE_OPERAND (obj_arg, 0);
- STRIP_NOPS (addr);
- if (TREE_CODE (addr) == ADDR_EXPR)
-   {
- tree typeo = TREE_TYPE (obj_arg);
- tree typei = TREE_TYPE (TREE_OPERAND (addr, 0));
- if (same_type_ignoring_top_level_qualifiers_p 

[Bug middle-end/111243] The -Og option inlines functions, making for a poor debugging experience.

[amohr at amohr dot org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111243

--- Comment #10 from Alex Mohr  ---
(In reply to Xi Ruoyao from comment #9)
> I believe the only real issue is imprecise documentation: "It is a better
> choice than -O0" has some caveats and it's not always true.

Is there a way to explicitly enable the compiler passes that collect debug info
that are disabled at -O0?

[committed] libstdc++: Fix debug-mode tests for constexpr algorithms

[Jonathan Wakely via Gcc-patches]

Tested x86_64-linux. Pushed to trunk.

-- >8 --

These tests started failing at some point:
FAIL: 25_algorithms/copy/debug/constexpr_neg.cc  (test for errors, line 49)
FAIL: 25_algorithms/copy/debug/constexpr_neg.cc (test for excess errors)
FAIL: 25_algorithms/equal/debug/constexpr_neg.cc  (test for errors, line 47)
FAIL: 25_algorithms/equal/debug/constexpr_neg.cc (test for excess errors)

They only run with -D_GLIBCXX_DEBUG or make check-debug so seem to have
gone unnoticed until now.

libstdc++-v3/ChangeLog:

* testsuite/25_algorithms/copy/debug/constexpr_neg.cc: Adjust
expected errors.
* testsuite/25_algorithms/equal/debug/constexpr_neg.cc:
Likewise.
---
 .../25_algorithms/copy/debug/constexpr_neg.cc  |  8 +++-
 .../25_algorithms/equal/debug/constexpr_neg.cc | 10 --
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/libstdc++-v3/testsuite/25_algorithms/copy/debug/constexpr_neg.cc 
b/libstdc++-v3/testsuite/25_algorithms/copy/debug/constexpr_neg.cc
index 6981c470666..bf3c4939bfb 100644
--- a/libstdc++-v3/testsuite/25_algorithms/copy/debug/constexpr_neg.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/copy/debug/constexpr_neg.cc
@@ -34,6 +34,7 @@ test1()
 }
 
 static_assert(test1()); // { dg-error "non-constant condition" }
+// { dg-error "builtin_unreachable" "" { target *-*-* } 0 }
 
 constexpr bool
 test2()
@@ -46,8 +47,5 @@ test2()
   return out6 == ma0.begin() + 18;
 }
 
-static_assert(test2()); // { dg-error "is outside the bounds" }
-
-// { dg-prune-output "in 'constexpr' expansion" }
-// { dg-prune-output "builtin_unreachable" }
-// { dg-prune-output "non-constant condition" }
+static_assert(test2()); // { dg-error "non-constant condition" }
+// { dg-error "is outside the bounds" "" { target *-*-* } 0 }
diff --git a/libstdc++-v3/testsuite/25_algorithms/equal/debug/constexpr_neg.cc 
b/libstdc++-v3/testsuite/25_algorithms/equal/debug/constexpr_neg.cc
index bb613bef03b..f5e46e58e49 100644
--- a/libstdc++-v3/testsuite/25_algorithms/equal/debug/constexpr_neg.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/equal/debug/constexpr_neg.cc
@@ -32,7 +32,8 @@ test01()
   return outa;
 }
 
-static_assert(test01()); // { dg-error }
+static_assert(test01()); // { dg-error "non-constant condition" }
+// { dg-error "builtin_unreachable" "" { target *-*-* } 0 }
 
 constexpr bool
 test02()
@@ -44,8 +45,5 @@ test02()
   return outa;
 }
 
-static_assert(test02()); // { dg-error "outside the bounds" }
-
-// { dg-prune-output "non-constant condition" }
-// { dg-prune-output "in 'constexpr'" }
-// { dg-prune-output "builtin_unreachable" }
+static_assert(test02()); // { dg-error "non-constant condition" }
+// { dg-error "is outside the bounds" "" { target *-*-* } 0 }
-- 
2.41.0

[Bug middle-end/111243] The -Og option inlines functions, making for a poor debugging experience.

[xry111 at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111243

--- Comment #9 from Xi Ruoyao  ---
(In reply to Alex Mohr from comment #8)
> (In reply to Jonathan Wakely from comment #5)
> > A 4x slowdown isn't really acceptable IMHO. At that point, why not just use
> > -O0 instead?
> 
> I've been using -O0 for years.  I was trying to move to -Og because of this
> from the manual; particularly the second sentence:
> 
> "-Og should be the optimization level of choice for the standard
> edit-compile-debug cycle, offering a reasonable level of optimization while
> maintaining fast compilation and a good debugging experience. It is a better
> choice than -O0 for producing debuggable code because some compiler passes
> that collect debug information are disabled at -O0."
> 
> I definitely do not want to lose debug info.  The speed improvement is a
> nice bonus, but top-notch debugging is my main goal for my debug builds.

I believe the only real issue is imprecise documentation: "It is a better
choice than -O0" has some caveats and it's not always true.

[committed] libstdc++: Add -Wno-self-move to two filesystem tests

[Jonathan Wakely via Gcc-patches]

Tested x86_64-linux. Pushed to trunk.

-- >8 --

libstdc++-v3/ChangeLog:

* testsuite/27_io/filesystem/iterators/91067.cc: Add
-Wno-self-move to options.
* testsuite/27_io/filesystem/path/assign/copy.cc: Likewise.
---
 libstdc++-v3/testsuite/27_io/filesystem/iterators/91067.cc  | 1 +
 libstdc++-v3/testsuite/27_io/filesystem/path/assign/copy.cc | 1 +
 2 files changed, 2 insertions(+)

diff --git a/libstdc++-v3/testsuite/27_io/filesystem/iterators/91067.cc 
b/libstdc++-v3/testsuite/27_io/filesystem/iterators/91067.cc
index b960ee7c798..2bf1e081c25 100644
--- a/libstdc++-v3/testsuite/27_io/filesystem/iterators/91067.cc
+++ b/libstdc++-v3/testsuite/27_io/filesystem/iterators/91067.cc
@@ -17,6 +17,7 @@
 
 // { dg-do link { target c++17 } }
 // { dg-require-filesystem-ts "" }
+// { dg-options "-Wno-self-move" }
 
 #include 
 
diff --git a/libstdc++-v3/testsuite/27_io/filesystem/path/assign/copy.cc 
b/libstdc++-v3/testsuite/27_io/filesystem/path/assign/copy.cc
index dc147b1cf3b..6ec347531bc 100644
--- a/libstdc++-v3/testsuite/27_io/filesystem/path/assign/copy.cc
+++ b/libstdc++-v3/testsuite/27_io/filesystem/path/assign/copy.cc
@@ -1,4 +1,5 @@
 // { dg-do run { target c++17 } }
+// { dg-options "-Wno-self-move" }
 
 // Copyright (C) 2014-2023 Free Software Foundation, Inc.
 //
-- 
2.41.0

[Bug c/111269] Confusing location of error in source code

[pinskia at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111269

Andrew Pinski  changed:

   What|Removed |Added

 Ever confirmed|0   |1
   Keywords||diagnostic
 Status|UNCONFIRMED |NEW
   Last reconfirmed||2023-09-01

--- Comment #1 from Andrew Pinski  ---
It is pointing to the whole expression and just the outermost operator, || .

Now the C++ front-end gives a better location and information on why it is not
a constant expression:
: In function 'int main()':
:11:26: error: non-constant condition for static assertion
   11 | _Static_assert(0 || 7 > x, "");
  |~~^~~~
:11:33: error: the value of 'x' is not usable in a constant expression
   11 | _Static_assert(0 || 7 > x, "");
  | ^
:9:13: note: 'int x' is not const
9 | int x = 42;
  | ^


The C front-end could do better ...

[Bug c/111269] Confusing location of error in source code

[pinskia at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111269

Andrew Pinski  changed:

   What|Removed |Added

   Severity|normal  |enhancement

Re: [PATCH] libstdc++: fix memory clobbering in std::vector [PR110879]

[Jonathan Wakely via Gcc-patches]

At Marek and Jason's suggestion I've moved the new test to a subdir:

   c++: Move new test to 'opt' sub-directory

   gcc/testsuite/ChangeLog:

   * g++.dg/pr110879.C: Moved to...
   * g++.dg/opt/pr110879.C: ...here.

Re: Unexpected behavior of gcc on pointer dereference & increment

[Paul Koning via Gcc]

> On Sep 1, 2023, at 12:35 PM, Tomas Bortoli via Gcc  wrote:
> 
> Hi,
> 
> I recently discovered that the following C statement:
> 
> pointer++;
> 
> is semantically equivalent to the following:
> 
> *pointer++;
> 
> Is this due to operators' priority? To me, that looks weird.

Yes, https://en.cppreference.com/w/c/language/operator_precedence shows that.  
Liberal use of parentheses is a very good practice. 

paul

Re: Unexpected behavior of gcc on pointer dereference & increment

[David Edelsohn via Gcc]

On Fri, Sep 1, 2023 at 12:37 PM Tomas Bortoli via Gcc 
wrote:

> Hi,
>
> I recently discovered that the following C statement:
>
> pointer++;
>
> is semantically equivalent to the following:
>
> *pointer++;
>
> Is this due to operators' priority? To me, that looks weird.
>

Equivalent in the effect, but not the value.  As you suspect, this is due
to operator precedence.

https://en.cppreference.com/w/c/language/operator_precedence

This is probably more appropriate for gcc-help or a general forum about the
C Language.

https://stackoverflow.com/questions/68829154/c-operator-precedence-postfix-increment-and-dereference

Thanks, David

[Bug middle-end/111243] The -Og option inlines functions, making for a poor debugging experience.

[amohr at amohr dot org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111243

--- Comment #8 from Alex Mohr  ---
(In reply to Jonathan Wakely from comment #5)
> A 4x slowdown isn't really acceptable IMHO. At that point, why not just use
> -O0 instead?

I've been using -O0 for years.  I was trying to move to -Og because of this
from the manual; particularly the second sentence:

"-Og should be the optimization level of choice for the standard
edit-compile-debug cycle, offering a reasonable level of optimization while
maintaining fast compilation and a good debugging experience. It is a better
choice than -O0 for producing debuggable code because some compiler passes that
collect debug information are disabled at -O0."

I definitely do not want to lose debug info.  The speed improvement is a nice
bonus, but top-notch debugging is my main goal for my debug builds.

Unexpected behavior of gcc on pointer dereference & increment

[Tomas Bortoli via Gcc]

Hi,

I recently discovered that the following C statement:

pointer++;

is semantically equivalent to the following:

*pointer++;

Is this due to operators' priority? To me, that looks weird.


Thanks in advance,
Tomas

[Bug middle-end/111243] The -Og option inlines functions, making for a poor debugging experience.

[redi at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111243

Jonathan Wakely  changed:

   What|Removed |Added

 CC||redi at gcc dot gnu.org

--- Comment #7 from Jonathan Wakely  ---
I think maybe this is a debuginfo and/or gdb problem, where 'next' doesn't
really do what you want, and 'finish' leaves the frame which has unpredictable
results if anything has been inlined (gdb shows you're in function 'foo' but
since that was actually inlined into its caller, running 'finish' returns from
foo's caller not from foo).

Ideally we'd keep the inlining, but improve the debug experience. Disabling
inlining seems like a hack to work around the fact that gdb doesn't really do
what we want.

[Bug middle-end/111243] The -Og option inlines functions, making for a poor debugging experience.

[redi at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111243

--- Comment #6 from Jonathan Wakely  ---
(In reply to Richard Biener from comment #3)
> it shouldn't make much difference when single-stepping
> statements since the debugger should enter inlined bodies the same as
> not inlined bodies?

'step' works OK, but 'next' and 'finish' are pretty useless. And if you can
only use 'step' then you end up descending into constructors and destructors of
temporaries and other uninteresting junk that isn't what you're trying to
debug.

For example, with this code:

#include 
#include 
#include 

int main()
{
  using namespace std::chrono;
  const sys_seconds expected = sys_days(2023y/August/9) + 20h + 44min;
  file_time tp;

  minutes offset;
  std::string abbrev;
  std::istringstream is("002023-08-09 21:44 +01 BST!");
  assert( is >> parse("%6F %R %z %Z", tp, abbrev, offset) );
}

Compiled with -Og I get:

$ gdb -q  -ex start -ex n -ex n -ex n -ex n -ex n -ex step -ex step -ex step
-ex step -ex step -ex step -ex step -ex  step -ex step  ./parse
Reading symbols from ./parse...
Temporary breakpoint 1 at 0x402296: file parse.cc, line 6.
Starting program: /tmp/parse 
[Thread debugging using libthread_db enabled]   
Using host libthread_db library "/lib64/libthread_db.so.1".

Temporary breakpoint 1, main () at parse.cc:6
6   {
9 file_time tp;
12std::string abbrev;
88__new_allocator() _GLIBCXX_USE_NOEXCEPT { }
13std::istringstream is("002023-08-09 21:44 +01 BST!");
14VERIFY( is >> parse("%6F %R %z %Z", tp, abbrev, offset) );
std::chrono::parse, std::allocator,
std::__cxx11::basic_string, std::allocator
>, std::chrono::time_point > > >
(__offset=std::chrono::duration = { 73728min }, 
__abbrev="", __tp=std::chrono::file_time = { 0s [2174-01-01 00:00:00] },
__fmt=0x406324 "%6F %R %z %Z")
at /home/jwakely/gcc/14/include/c++/14.0.0/bits/chrono_io.h:2980
2980 
&__offset);
std::chrono::__detail::_Parse > >, char,
std::char_traits, std::allocator >::_Parse
(__offset=0x7fffd8d8, __abbrev=0x7fffd8b0, __tp=std::chrono::file_time
= { 0s [2174-01-01 00:00:00] }, 
__fmt=0x406324 "%6F %R %z %Z", this=0x7fffd710) at
/home/jwakely/gcc/14/include/c++/14.0.0/bits/chrono_io.h:2868
2868  _Parse(const _CharT* __fmt, _Parsable& __tp,
std::chrono::__detail::operator>> (__is=..., __p=...) at
/home/jwakely/gcc/14/include/c++/14.0.0/bits/chrono_io.h:2887
2887  operator>>(__stream_type& __is, _Parse&& __p)
std::chrono::from_stream,
std::chrono::duration >, std::allocator >
(__is=..., 
__fmt=0x406324 "%6F %R %z %Z", __tp=std::chrono::file_time = { 0s
[2174-01-01 00:00:00] }, __abbrev=0x7fffd8b0, __offset=0x7fffd8d8)
at /home/jwakely/gcc/14/include/c++/14.0.0/bits/chrono_io.h:2806
2806from_stream(basic_istream<_CharT, _Traits>& __is, const _CharT*
__fmt,
std::chrono::from_stream,
std::chrono::duration >, std::allocator >
(__is=..., 
__fmt=0x406324 "%6F %R %z %Z", __tp=std::chrono::sys_time = { 0s
[1970-01-01 00:00:00] }, __abbrev=0x7fffd8b0, __offset=0x7fffd8d8)
at /home/jwakely/gcc/14/include/c++/14.0.0/bits/chrono_io.h:2705
2705from_stream(basic_istream<_CharT, _Traits>& __is, const _CharT*
__fmt,
2716  __detail::_Parser_t<_Duration> __p(__need);
std::chrono::__detail::_Parser >
>::_Parser (__need=23, this=0x7fffd680)
at /home/jwakely/gcc/14/include/c++/14.0.0/bits/chrono_io.h:2136
2136  _Parser(__format::_ChronoParts __need) : _M_need(__need) { }
std::chrono::time_point > >::time_point
(this=0x7fffd688)
at /home/jwakely/gcc/14/include/c++/14.0.0/bits/chrono.h:935
935 constexpr time_point() : __d(duration::zero())
0x0040306b in
std::chrono::__detail::_Parser >
>::operator(), std::allocator >
(this=this@entry=0x7fffd680, __is=..., __fmt=0x406324 "%6F %R %z %Z",
__abbrev=0x7fffd8b0, __offset=__offset@entry=0x7fffd8d8)
at /home/jwakely/gcc/14/include/c++/14.0.0/bits/chrono_io.h:3007
3007  ios_base::iostate __err = ios_base::goodbit;

I don't know what the __new_allocator constructor is doing there. The
time_point constructor isn't interesting.

If you replace any of those 'step' commands with 'next' you end up somewhere
you don't want to be:

$ gdb -q  -ex start -ex n -ex n -ex n -ex n -ex n -ex step -ex step -ex step
-ex n -ex step -ex step -ex step -ex  step -ex step  ./parse
Reading symbols from ./parse...
Temporary breakpoint 1 at 0x402296: file parse.cc, line 6.
Starting program: /tmp/parse 
[Thread debugging using libthread_db enabled]   
Using host libthread_db library "/lib64/libthread_db.so.1".

Temporary breakpoint 1, main () at parse.cc:6
6   {
9 file_time tp;
12std::string abbrev;
88__new_allocator() _GLIBCXX_USE_NOEXCEPT { }
13std::istringstream is("002023-08-09 

[Bug tree-optimization/107137] (unsigned)-(int)(bool_var) should be optimized to -(unsigned)bool_var

[pinskia at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107137

--- Comment #4 from Andrew Pinski  ---
Solving this one is easier, just going with:
/* (nop_outer_cast)-(inner_cast)var -> -(outer_cast)(var)
   if var is smaller in precision. */
(simplify
 (convert (negate:s@1 (convert:s @0)))
 (if (INTEGRAL_TYPE_P (type)
  && tree_nop_conversion_p (type, TREE_TYPE (@1))
  && TYPE_PRECISION (type) > TYPE_PRECISION (TREE_TYPE (@0)))
(negate (convert @0

[Bug tree-optimization/107765] missing (int)-(unsigned)int_val to just -int_val if int_val is known not to contain INT_MIN

[pinskia at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107765

--- Comment #3 from Andrew Pinski  ---
So I have decided to solve the original testcase differently.

[Bug middle-end/111243] The -Og option inlines functions, making for a poor debugging experience.

[redi at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111243

--- Comment #5 from Jonathan Wakely  ---
A 4x slowdown isn't really acceptable IMHO. At that point, why not just use -O0
instead?

[Bug libstdc++/110879] [14 Regression] Unnecessary reread from memory in a loop with std::vector

[redi at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110879

Jonathan Wakely  changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
 Resolution|--- |FIXED

--- Comment #2 from Jonathan Wakely  ---
This should be fixed now. Thanks for the report and the patch.

Re: [RFC] libstdc++: Make --enable-libstdcxx-backtrace=auto default to yes

[Jonathan Wakely via Gcc-patches]

On Fri, 1 Sept 2023 at 12:16, Jonathan Wakely  wrote:
>
> On Wed, 23 Aug 2023 at 17:03, Jonathan Wakely via Libstdc++
>  wrote:
> >
> > Any objections to this? It's a C++23 feture, so should be enabled by
> > default.
>
> I've pushed this to trunk, so let's see what breaks!

This modules header broke on aarch64, of course:
FAIL: g++.dg/modules/xtreme-header_b.C -std=c++2b (test for excess errors)

>
>
> >
> > -- >8 --
> >
> > This causes libstdc++_libbacktrace.a to be built by default. This might
> > fail on some targets, in which case we can make the 'auto' choice expand
> > to either 'yes' or 'no' depending on the target.
> >
> > libstdc++-v3/ChangeLog:
> >
> > * acinclude.m4 (GLIBCXX_ENABLE_BACKTRACE): Default to yes.
> > * configure: Regenerate.
> > ---
> >  libstdc++-v3/acinclude.m4 | 2 +-
> >  libstdc++-v3/configure| 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
> > index b25378eaace..50c808c6b2d 100644
> > --- a/libstdc++-v3/acinclude.m4
> > +++ b/libstdc++-v3/acinclude.m4
> > @@ -5481,7 +5481,7 @@ BACKTRACE_CPPFLAGS="$BACKTRACE_CPPFLAGS 
> > -DBACKTRACE_ELF_SIZE=$elfsize"
> >
> >AC_MSG_CHECKING([whether to build libbacktrace support])
> >if test "$enable_libstdcxx_backtrace" = "auto"; then
> > -enable_libstdcxx_backtrace=no
> > +enable_libstdcxx_backtrace=yes
> >fi
> >AC_MSG_RESULT($enable_libstdcxx_backtrace)
> >if test "$enable_libstdcxx_backtrace" = "yes"; then
> >

Re: [PATCH] analyzer: Add support of placement new and improved operator new [PR105948,PR94355]

[David Malcolm via Gcc-patches]

On Fri, 2023-09-01 at 16:48 +0200, Benjamin Priour wrote:
> Patch has been updated as per your suggestions and successfully
> regstrapped
> on x86_64-linux-gnu.
> 
> call_details::maybe_get_arg_region is now
> /* If argument IDX's svalue at the callsite is of pointer type,
>     return the region it points to.
>     Otherwise return NULL.  */
> 
> const region *
>  call_details::deref_ptr_arg (unsigned idx) const
>  {
>    const svalue *ptr_sval = get_arg_svalue (idx);
>    return m_model->deref_rvalue (ptr_sval, get_arg_tree (idx),
> m_ctxt);
>  }
> 
> 
> New test is
> 
> +
> +void test_binop ()
> +{
> +  char *p = (char *) malloc (4);
> +  if (!p)
> +    return;
> +  int32_t *i = ::new (p + 1) int32_t; /* { dg-warning "heap-based
> buffer
> overflow" } */
> +  *i = 42; /* { dg-warning "heap-based buffer overflow" } */
> +  free (p);
> +}
> 
> Is it OK for trunk ?
> I didn't resend the whole patch as it otherwise was OK.

Yes, thanks.

Dave

[Bug middle-end/111243] The -Og option inlines functions, making for a poor debugging experience.

[xry111 at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111243

Xi Ruoyao  changed:

   What|Removed |Added

 CC||xry111 at gcc dot gnu.org

--- Comment #4 from Xi Ruoyao  ---
I remember I've asked "why not disable -finline with -Og" via gcc-help and
Jonathan told me it would make any "real" C++ programs stupidly slow.

Re: [committed] libstdc++: Fix compare_exchange_padding.cc test for std::atomic_ref

[Jonathan Wakely via Gcc-patches]

On Mon, 31 Oct 2022 at 15:34, Eric Botcazou wrote:
>
> > The test was only failing for me with -m32 (and not -m64), so I didn't
> > notice until now. That probably means we should make the test fail more
> > reliably if the padding isn't being cleared.
>
> The tests fail randomly for me on SPARC64/Linux:
>
> FAIL: 29_atomics/atomic/compare_exchange_padding.cc execution test
> FAIL: 29_atomics/atomic_ref/compare_exchange_padding.cc execution test
>
> /home/ebotcazou/src/libstdc++-v3/testsuite/29_atomics/atomic_ref/
> compare_exchange_padding.cc:34: int main(): Assertion 'compare_struct(ts, es)'
> failed.
> FAIL: 29_atomics/atomic_ref/compare_exchange_padding.cc execution test
>
>   std::atomic as{ s };
>   auto ts = as.load();
>   VERIFY( !compare_struct(s, ts) ); // padding cleared on construction
>   as.exchange(s);
>   auto es = as.load();
>   VERIFY( compare_struct(ts, es) ); // padding cleared on exchange
>
> How is it supposed to pass exactly?  AFAICS you have no control on the padding
> bits of ts or es and, indeed, at -O2 the loads are scalarized:
>
>   __buf$c_81 = MEM[(struct S *)&__buf].c;
>   __buf$s_59 = MEM[(struct S *)&__buf].s;
>   __buf ={v} {CLOBBER(eol)};
>   ts.c = __buf$c_81;
>   ts.s = __buf$s_59;
> [...]
>   __buf$c_100 = MEM[(struct S *)&__buf].c;
>   __buf$s_35 = MEM[(struct S *)&__buf].s;
>   __buf ={v} {CLOBBER(eol)};
>   es.c = __buf$c_100;
>   es.s = __buf$s_35;
>   _66 = MEM  [(char * {ref-all})];
>   _101 = MEM  [(char * {ref-all})];
>   if (_66 != _101)
> goto ; [0.04%]
>   else
> goto ; [99.96%]
>
> so the result of the 4-byte comparison is random.

This should be fixed now. I rewrote the test to check the padding byte
directly, instead of inspecting a copy of it which might not preserve
the padding bits.

Re: [PATCH]AArch64 xorsign: Fix scalar xorsign lowering

[Richard Sandiford via Gcc-patches]

Tamar Christina  writes:
>> -Original Message-
>> From: Richard Sandiford 
>> Sent: Friday, September 1, 2023 2:36 PM
>> To: Tamar Christina 
>> Cc: gcc-patches@gcc.gnu.org; nd ; Richard Earnshaw
>> ; Marcus Shawcroft
>> ; Kyrylo Tkachov 
>> Subject: Re: [PATCH]AArch64 xorsign: Fix scalar xorsign lowering
>> 
>> Tamar Christina  writes:
>> > Hi All,
>> >
>> > In GCC-9 our scalar xorsign pattern broke and we didn't notice it
>> > because the testcase was not strong enough.  With this commit
>> >
>> > 8d2d39587d941a40f25ea0144cceb677df115040 is the first bad commit
>> > commit 8d2d39587d941a40f25ea0144cceb677df115040
>> > Author: Segher Boessenkool 
>> > Date:   Mon Oct 22 22:23:39 2018 +0200
>> >
>> > combine: Do not combine moves from hard registers
>> >
>> > combine started introducing useless moves on hard registers,  when one
>> > of the arguments to our scalar xorsign is a hardreg we get an additional 
>> > move
>> inserted.
>> >
>> > This leads to combine forming an AND with the immediate inside and
>> > using the superflous move to do the r->w move, instead of what we
>> > wanted before which was for the `and` to be a vector and and have reload
>> pick the right alternative.
>> 
>> IMO, the xorsign optab ought to go away.  IIRC it was just a stop-gap measure
>> that (like most stop-gap measures) never got cleaned up later.
>> 
>> But that's not important now. :)
>> 
>> > To fix this the patch just forces the use of the vector version
>> > directly and so combine has no chance to mess it up.
>> >
>> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
>> >
>> > Ok for master?
>> >
>> > Thanks,
>> > Tamar
>> >
>> > gcc/ChangeLog:
>> >
>> >* config/aarch64/aarch64-simd.md (xorsign3): Renamed to..
>> >(@xorsign3): ...This.
>> >* config/aarch64/aarch64.md (xorsign3): Renamed to...
>> >(@xorsign3): ..This and emit vectors directly
>> >* config/aarch64/iterators.md (VCONQ): Add SF and DF.
>> >
>> > gcc/testsuite/ChangeLog:
>> >
>> >* gcc.target/aarch64/xorsign.c:
>> >
>> > --- inline copy of patch --
>> > diff --git a/gcc/config/aarch64/aarch64-simd.md
>> > b/gcc/config/aarch64/aarch64-simd.md
>> > index
>> >
>> f67eb70577d0c2d9911d8c867d38a4d0b390337c..e955691f1be8830efacc2
>> 3746511
>> > 9764ce2a4942 100644
>> > --- a/gcc/config/aarch64/aarch64-simd.md
>> > +++ b/gcc/config/aarch64/aarch64-simd.md
>> > @@ -500,7 +500,7 @@ (define_expand "ctz2"
>> >}
>> >  )
>> >
>> > -(define_expand "xorsign3"
>> > +(define_expand "@xorsign3"
>> >[(match_operand:VHSDF 0 "register_operand")
>> > (match_operand:VHSDF 1 "register_operand")
>> > (match_operand:VHSDF 2 "register_operand")] diff --git
>> > a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index
>> >
>> 01cf989641fce8e6c3828f6cfef62e101c4142df..9db82347bf891f9bc40aede
>> cdc84
>> > 62c94bf1a769 100644
>> > --- a/gcc/config/aarch64/aarch64.md
>> > +++ b/gcc/config/aarch64/aarch64.md
>> > @@ -6953,31 +6953,20 @@ (define_insn "copysign3_insn"
>> >  ;; EOR   v0.8B, v0.8B, v3.8B
>> >  ;;
>> >
>> > -(define_expand "xorsign3"
>> > +(define_expand "@xorsign3"
>> >[(match_operand:GPF 0 "register_operand")
>> > (match_operand:GPF 1 "register_operand")
>> > (match_operand:GPF 2 "register_operand")]
>> >"TARGET_SIMD"
>> >  {
>> > -
>> > -  machine_mode imode = mode;
>> > -  rtx mask = gen_reg_rtx (imode);
>> > -  rtx op1x = gen_reg_rtx (imode);
>> > -  rtx op2x = gen_reg_rtx (imode);
>> > -
>> > -  int bits = GET_MODE_BITSIZE (mode) - 1;
>> > -  emit_move_insn (mask, GEN_INT (trunc_int_for_mode
>> (HOST_WIDE_INT_M1U << bits,
>> > -   imode)));
>> > -
>> > -  emit_insn (gen_and3 (op2x, mask,
>> > -  lowpart_subreg (imode, operands[2],
>> > -  mode)));
>> > -  emit_insn (gen_xor3 (op1x,
>> > -  lowpart_subreg (imode, operands[1],
>> > -  mode),
>> > -  op2x));
>> > +  rtx tmp = gen_reg_rtx (mode);  rtx op1 = gen_reg_rtx
>> > + (mode);  rtx op2 = gen_reg_rtx (mode);
>> emit_move_insn
>> > + (op1, lowpart_subreg (mode, operands[1], mode));
>> > + emit_move_insn (op2, lowpart_subreg (mode, operands[2],
>> > + mode));  emit_insn (gen_xorsign3(mode, tmp, op1,
>> op2));
>> 
>> Do we need the extra moves into op1 and op2?  I would have expected the
>> subregs to be acceptable as direct operands of the xorsign3.  Making them
>> direct operands should be better, since there's then less risk of having the
>> same value live in different registers at the same time.
>> 
>
> That was the first thing I tried but it doesn't work because validate_subreg 
> seems
> to have the invariant that you can either change mode between the same size
> or make it paradoxical but not both at the same time.
>
> i.e. it rejects subreg:V2DI (subreg:DI (reg:DF))), and lowpart_subreg folds 
> it to
> 

Re: [PATCH] libstdc++: fix memory clobbering in std::vector [PR110879]

[Jonathan Wakely via Gcc-patches]

On Thu, 17 Aug 2023 at 08:43, Vladimir Palevich  wrote:
>
> On Thu, 17 Aug 2023 at 01:51, Jonathan Wakely  wrote:
> >
> > On 09/08/23 01:34 +0300, Vladimir Palevich wrote:
> > >Because of the recent change in _M_realloc_insert and _M_default_append, 
> > >call
> > >to deallocate was ordered after assignment to class members of std::vector
> > >(in the guard destructor), which is causing said members to be 
> > >call-clobbered.
> > >This is preventing further optimization, the compiler is unable to move 
> > >memory
> > >read out of a hot loop in this case.
> > >This patch reorders the call to before assignments by putting guard in its 
> > >own
> > >block. Plus a new testsuite for this case.
> > >I'm not very happy with the new testsuite, but I don't know how to properly
> > >test this.
> > >
> > >Tested on x86_64-pc-linux-gnu.
> > >
> > >Maybe something could be done so that the compiler would be able to 
> > >optimize
> > >such cases anyway. Reads could be moved just after the clobbering calls in
> > >unlikely branches, for example. This should be a fairly common case with
> > >destructors at the end of a function.
> > >
> > >Note: I don't have write access.
> > >
> > >-- >8 --
> > >
> > >Fix ordering to prevent clobbering of class members by a call to deallocate
> > >in _M_realloc_insert and _M_default_append.
> > >
> > >libstdc++-v3/ChangeLog:
> > >PR libstdc++/110879
> > >* include/bits/vector.tcc: End guard lifetime just before assignment to
> > >class members.
> > >* testsuite/libstdc++-dg/conformance.exp: Load scantree.exp.
> > >* testsuite/23_containers/vector/110879.cc: New test.
> > >
> > >Signed-off-by: Vladimir Palevich  
> > >---
> > > libstdc++-v3/include/bits/vector.tcc  | 220 +-
> > > .../testsuite/23_containers/vector/110879.cc  |  35 +++
> > > .../testsuite/libstdc++-dg/conformance.exp|  13 ++
> > > 3 files changed, 163 insertions(+), 105 deletions(-)
> > > create mode 100644 libstdc++-v3/testsuite/23_containers/vector/110879.cc
> > >
> > >diff --git a/libstdc++-v3/include/bits/vector.tcc 
> > >b/libstdc++-v3/include/bits/vector.tcc
> > >index ada396c9b30..80631d1e2a1 100644
> > >--- a/libstdc++-v3/include/bits/vector.tcc
> > >+++ b/libstdc++-v3/include/bits/vector.tcc
> > >@@ -488,78 +488,83 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> > >   private:
> > >   _Guard(const _Guard&);
> > >   };
> > >-  _Guard __guard(__new_start, __len, _M_impl);
> > >
> > >-  // The order of the three operations is dictated by the C++11
> > >-  // case, where the moves could alter a new element belonging
> > >-  // to the existing vector.  This is an issue only for callers
> > >-  // taking the element by lvalue ref (see last bullet of C++11
> > >-  // [res.on.arguments]).
> > >+  {
> > >+  _Guard __guard(__new_start, __len, _M_impl);
> > >
> > >-  // If this throws, the existing elements are unchanged.
> > >+  // The order of the three operations is dictated by the C++11
> > >+  // case, where the moves could alter a new element belonging
> > >+  // to the existing vector.  This is an issue only for callers
> > >+  // taking the element by lvalue ref (see last bullet of C++11
> > >+  // [res.on.arguments]).
> > >+
> > >+  // If this throws, the existing elements are unchanged.
> > > #if __cplusplus >= 201103L
> > >-  _Alloc_traits::construct(this->_M_impl,
> > >- std::__to_address(__new_start + 
> > >__elems_before),
> > >- std::forward<_Args>(__args)...);
> > >+  _Alloc_traits::construct(this->_M_impl,
> > >+   std::__to_address(__new_start + 
> > >__elems_before),
> > >+   std::forward<_Args>(__args)...);
> > > #else
> > >-  _Alloc_traits::construct(this->_M_impl,
> > >- __new_start + __elems_before,
> > >- __x);
> > >+  _Alloc_traits::construct(this->_M_impl,
> > >+   __new_start + __elems_before,
> > >+   __x);
> > > #endif
> > >
> > > #if __cplusplus >= 201103L
> > >-  if _GLIBCXX17_CONSTEXPR (_S_use_relocate())
> > >-  {
> > >-// Relocation cannot throw.
> > >-__new_finish = _S_relocate(__old_start, __position.base(),
> > >-   __new_start, _M_get_Tp_allocator());
> > >-++__new_finish;
> > >-__new_finish = _S_relocate(__position.base(), __old_finish,
> > >-   __new_finish, _M_get_Tp_allocator());
> > >-  }
> > >-  else
> > >+  if _GLIBCXX17_CONSTEXPR (_S_use_relocate())
> > >+{
> > >+  // Relocation cannot throw.
> > >+  __new_finish = _S_relocate(__old_start, __position.base(),
> > >+ __new_start, _M_get_Tp_allocator());
> > >+  ++__new_finish;
> > >+  

[Bug fortran/111271] gcc/fortran/trans-expr.cc:1134:8: warning: duplicated ‘if’ condition [-Wduplicated-cond]

[dcb314 at hotmail dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111271

David Binderman  changed:

   What|Removed |Added

 CC||pault at gcc dot gnu.org

--- Comment #1 from David Binderman  ---
git blame says

6c95fe9bc0 (Paul Thomas  2023-05-16 06:35:40 +0100  1087)   if
(unlimited_poly)

...

6c95fe9bc0 (Paul Thomas  2023-05-16 06:35:40 +0100  1134)   else if
(unlimited_poly)

Adding Paul for their best advice.

[Bug target/111270] gcc/config/i386/i386-options.cc:3039:8: warning: duplicated ‘if ’ condition [-Wduplicated-cond]

[dcb314 at hotmail dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111270

David Binderman  changed:

   What|Removed |Added

 CC||redi at gcc dot gnu.org

--- Comment #1 from David Binderman  ---
git blame says:

^d4ba3b369 (Jonathan Wakely 2022-11-01 09:48:41 + 3037)   if
(ix86_tune_features [X86_TUNE_AVOID_256FMA_CHAINS])
eef81eefcd (Jan Hubicka 2022-12-22 10:55:46 +0100 3038)
SET_OPTION_IF_UNSET (opts, opts_set, param_avoid_fma_max_bits, 512);

Adding Jonathan for their best advice.

[committed] libstdc++: Use std::string::__resize_and_overwrite in std::filesystem

[Jonathan Wakely via Gcc-patches]

Tested x86_64-linux. Pushed to trunk.

-- >8 --

There are a few places in the std::filesystem code that use a string as
a buffer for OS APIs to write to. We can use the new extension
__resize_and_overwrite to avoid redundant initialization of those
buffers.

libstdc++-v3/ChangeLog:

* src/c++17/fs_ops.cc (fs::absolute) [FILESYSTEM_IS_WINDOWS]:
Use __resize_and_overwrite to fill buffer.
(fs::read_symlink) [HAVE_READLINK]: Likewise.
* src/filesystem/ops-common.h (get_temp_directory_from_env)
[FILESYSTEM_IS_WINDOWS]: Likewise.
---
 libstdc++-v3/src/c++17/fs_ops.cc | 45 
 libstdc++-v3/src/filesystem/ops-common.h |  7 ++--
 2 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc
index c94d260632f..6cdeac17c33 100644
--- a/libstdc++-v3/src/c++17/fs_ops.cc
+++ b/libstdc++-v3/src/c++17/fs_ops.cc
@@ -112,18 +112,17 @@ fs::absolute(const path& p, error_code& ec)
   wstring buf;
   do
 {
-  buf.resize(len);
-  len = GetFullPathNameW(s.data(), len, buf.data(), nullptr);
+  buf.__resize_and_overwrite(len, [, ](wchar_t* p, unsigned n) {
+   len = GetFullPathNameW(s.data(), n, p, nullptr);
+   return len > n ? 0 : len;
+  });
 }
   while (len > buf.size());
 
   if (len == 0)
 ec = __last_system_error();
   else
-{
-  buf.resize(len);
-  ret = std::move(buf);
-}
+ret = std::move(buf);
 #else
   ret = current_path(ec);
   ret /= p;
@@ -1187,31 +1186,33 @@ fs::path fs::read_symlink(const path& p, error_code& ec)
   return result;
 }
 
-  std::string buf(st.st_size ? st.st_size + 1 : 128, '\0');
+  std::string buf;
+  size_t bufsz = st.st_size ? st.st_size + 1 : 128;
   do
 {
-  ssize_t len = ::readlink(p.c_str(), buf.data(), buf.size());
-  if (len == -1)
+  ssize_t len;
+  buf.__resize_and_overwrite(bufsz, [, ](char* ptr, size_t n) {
+   len = ::readlink(p.c_str(), ptr, n);
+   return size_t(len) < n ? len : 0;
+  });
+  if (buf.size())
+   {
+ result.assign(std::move(buf));
+ ec.clear();
+ break;
+   }
+  else if (len == -1)
{
  ec.assign(errno, std::generic_category());
  return result;
}
-  else if (len == (ssize_t)buf.size())
+  else if (bufsz > 4096)
{
- if (buf.size() > 4096)
-   {
- ec.assign(ENAMETOOLONG, std::generic_category());
- return result;
-   }
- buf.resize(buf.size() * 2);
+ ec.assign(ENAMETOOLONG, std::generic_category());
+ return result;
}
   else
-   {
- buf.resize(len);
- result.assign(buf);
- ec.clear();
- break;
-   }
+   bufsz *= 2;
 }
   while (true);
 #else
diff --git a/libstdc++-v3/src/filesystem/ops-common.h 
b/libstdc++-v3/src/filesystem/ops-common.h
index 2e4331bb682..79dcb756453 100644
--- a/libstdc++-v3/src/filesystem/ops-common.h
+++ b/libstdc++-v3/src/filesystem/ops-common.h
@@ -700,8 +700,10 @@ _GLIBCXX_BEGIN_NAMESPACE_FILESYSTEM
 std::wstring buf;
 do
   {
-   buf.resize(len);
-   len = GetTempPathW(buf.size(), buf.data());
+   buf.__resize_and_overwrite(len, [](wchar_t* p, unsigned n) {
+ len = GetTempPathW(n, p);
+ return len > n ? 0 : len;
+   });
   }
 while (len > buf.size());
 
@@ -710,7 +712,6 @@ _GLIBCXX_BEGIN_NAMESPACE_FILESYSTEM
 else
   ec.clear();
 
-buf.resize(len);
 return buf;
   }
 #else
-- 
2.41.0

[committed] libstdc++: Use a loop in atomic_ref::compare_exchange_strong [PR111077]

[Jonathan Wakely via Gcc-patches]

Tested x86_64-linux. Pushed to trunk. Backport to gcc-13 needed too.

-- >8 --

We need to use a loop in std::atomic_ref::compare_exchange_strong in
order to properly implement the C++20 requirement that padding bits do
not participate when checking the value for equality. The variable being
modified by a std::atomic_ref might have an initial value with non-zero
padding bits, so when the __atomic_compare_exchange built-in returns
false we need to check whether that was only because of non-equal
padding bits that are not part of the value representation. If the value
bits differ, it's just a failed compare-exchange. If the value bits are
the same, we need to retry the __atomic_compare_exchange using the value
that was just read by the previous failed call. As noted in the
comments, it's possible for that second try to also fail due to another
thread storing the same value but with differences in padding.

Because it's undefined to access a variable directly while it's held by
a std::atomic_ref, and because std::atomic_ref will only ever store
values with zeroed padding, we know that padding bits will never go from
zero to non-zero during the lifetime of a std::atomic_ref. They can only
go from an initial non-zero state to zero. This means the loop will
terminate, rather than looping indefinitely as padding bits flicker on
and off. In theory users could call __atomic_store etc. directly and
write a value with non-zero padding bits, but we don't need to support
that. Users doing that should ensure they do not write non-zero padding,
to be compatibile with our std::atomic_ref's invariants.

This isn't a problem for std::atomic::compare_exchange_strong because
the initial value (and all later stores to the variable) are performed
by the library, so we ensure that stored values always have padding bits
cleared. That means we can simply clear the padding bits of the
'expected' value and we will be comparing two values with equal padding
bits. This means we don't need the loop for std::atomic, so update the
__atomic_impl::__compare_exchange function to take a bool parameter that
says whether it's being used by std::atomic_ref. If not, we can use a
simpler, non-looping implementation.

libstdc++-v3/ChangeLog:

PR libstdc++/111077
* include/bits/atomic_base.h (__atomic_impl::__compare_exchange):
Add _AtomicRef non-type template parameter and use a loop if it
is true.
(__atomic_impl::compare_exchange_weak): Add _AtomicRef NTTP.
(__atomic_impl::compare_exchange_strong): Likewise.
(atomic_ref::compare_exchange_weak): Use true for NTTP.
(atomic_ref::compare_exchange_strong): Use true for NTTP.
* testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc:
Fix test to not rely on atomic_ref::load() to return an object
with padding preserved.
---
 libstdc++-v3/include/bits/atomic_base.h   | 147 --
 .../atomic_ref/compare_exchange_padding.cc|  75 ++---
 2 files changed, 150 insertions(+), 72 deletions(-)

diff --git a/libstdc++-v3/include/bits/atomic_base.h 
b/libstdc++-v3/include/bits/atomic_base.h
index 4ce04a02dd0..974872ad7a6 100644
--- a/libstdc++-v3/include/bits/atomic_base.h
+++ b/libstdc++-v3/include/bits/atomic_base.h
@@ -985,7 +985,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 template
   using _Val = typename remove_volatile<_Tp>::type;
 
-template
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wc++17-extensions"
+
+template
   _GLIBCXX_ALWAYS_INLINE bool
   __compare_exchange(_Tp& __val, _Val<_Tp>& __e, _Val<_Tp>& __i,
 bool __is_weak,
@@ -994,27 +997,79 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__glibcxx_assert(__is_valid_cmpexch_failure_order(__f));
 
using _Vp = _Val<_Tp>;
+   _Tp* const __pval = std::__addressof(__val);
 
-   if _GLIBCXX17_CONSTEXPR (__atomic_impl::__maybe_has_padding<_Vp>())
+   if constexpr (!__atomic_impl::__maybe_has_padding<_Vp>())
  {
-   // We must not modify __e on success, so cannot clear its padding.
-   // Copy into a buffer and clear that, then copy back on failure.
-   alignas(_Vp) unsigned char __buf[sizeof(_Vp)];
-   _Vp* __exp = ::new((void*)__buf) _Vp(__e);
-   __atomic_impl::__clear_padding(*__exp);
-   if (__atomic_compare_exchange(std::__addressof(__val), __exp,
- __atomic_impl::__clear_padding(__i),
+   return __atomic_compare_exchange(__pval, std::__addressof(__e),
+std::__addressof(__i), __is_weak,
+int(__s), int(__f));
+ }
+   else if constexpr (!_AtomicRef) // std::atomic
+ {
+   // Clear padding of the value we want to set:
+   _Vp* const __pi = __atomic_impl::__clear_padding(__i);
+   // Only allowed to modify __e on failure, so make a 

[Bug libstdc++/110879] [14 Regression] Unnecessary reread from memory in a loop with std::vector

[cvs-commit at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110879

--- Comment #1 from CVS Commits  ---
The master branch has been updated by Jonathan Wakely :

https://gcc.gnu.org/g:419c423d3aeca754e47e1ce1bf707735603a90a3

commit r14-3627-g419c423d3aeca754e47e1ce1bf707735603a90a3
Author: Vladimir Palevich 
Date:   Wed Aug 9 01:34:05 2023 +0300

libstdc++: fix memory clobbering in std::vector [PR110879]

Fix ordering to prevent clobbering of class members by a call to deallocate
in _M_realloc_insert and _M_default_append.

Because of recent changes in _M_realloc_insert and _M_default_append,
calls to deallocate were ordered after assignment to class members of
std::vector (in the guard destructor), which is causing said members to
be call-clobbered.  This is preventing further optimization, the
compiler is unable to move memory read out of a hot loop in this case.

This patch reorders the call to before assignments by putting guard in
its own block. Plus a new testsuite for this case.  I'm not very happy
with the new testsuite, but I don't know how to properly test this.

PR libstdc++/110879

libstdc++-v3/ChangeLog:

* include/bits/vector.tcc (_M_realloc_insert): End guard
lifetime just before assignment to class members.
(_M_default_append): Likewise.

gcc/testsuite/ChangeLog:

* g++.dg/pr110879.C: New test.

Signed-off-by: Vladimir Palevich  

[Bug libstdc++/111077] atomic_ref compare_exchange_strong doesn't properly ignore padding bits

[cvs-commit at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111077

--- Comment #8 from CVS Commits  ---
The master branch has been updated by Jonathan Wakely :

https://gcc.gnu.org/g:dcbec954fcba42d97760c6bd98a4c5618473ec93

commit r14-3625-gdcbec954fcba42d97760c6bd98a4c5618473ec93
Author: Jonathan Wakely 
Date:   Wed Aug 23 12:23:37 2023 +0100

libstdc++: Use a loop in atomic_ref::compare_exchange_strong [PR111077]

We need to use a loop in std::atomic_ref::compare_exchange_strong in
order to properly implement the C++20 requirement that padding bits do
not participate when checking the value for equality. The variable being
modified by a std::atomic_ref might have an initial value with non-zero
padding bits, so when the __atomic_compare_exchange built-in returns
false we need to check whether that was only because of non-equal
padding bits that are not part of the value representation. If the value
bits differ, it's just a failed compare-exchange. If the value bits are
the same, we need to retry the __atomic_compare_exchange using the value
that was just read by the previous failed call. As noted in the
comments, it's possible for that second try to also fail due to another
thread storing the same value but with differences in padding.

Because it's undefined to access a variable directly while it's held by
a std::atomic_ref, and because std::atomic_ref will only ever store
values with zeroed padding, we know that padding bits will never go from
zero to non-zero during the lifetime of a std::atomic_ref. They can only
go from an initial non-zero state to zero. This means the loop will
terminate, rather than looping indefinitely as padding bits flicker on
and off. In theory users could call __atomic_store etc. directly and
write a value with non-zero padding bits, but we don't need to support
that. Users doing that should ensure they do not write non-zero padding,
to be compatibile with our std::atomic_ref's invariants.

This isn't a problem for std::atomic::compare_exchange_strong because
the initial value (and all later stores to the variable) are performed
by the library, so we ensure that stored values always have padding bits
cleared. That means we can simply clear the padding bits of the
'expected' value and we will be comparing two values with equal padding
bits. This means we don't need the loop for std::atomic, so update the
__atomic_impl::__compare_exchange function to take a bool parameter that
says whether it's being used by std::atomic_ref. If not, we can use a
simpler, non-looping implementation.

libstdc++-v3/ChangeLog:

PR libstdc++/111077
* include/bits/atomic_base.h (__atomic_impl::__compare_exchange):
Add _AtomicRef non-type template parameter and use a loop if it
is true.
(__atomic_impl::compare_exchange_weak): Add _AtomicRef NTTP.
(__atomic_impl::compare_exchange_strong): Likewise.
(atomic_ref::compare_exchange_weak): Use true for NTTP.
(atomic_ref::compare_exchange_strong): Use true for NTTP.
* testsuite/29_atomics/atomic_ref/compare_exchange_padding.cc:
Fix test to not rely on atomic_ref::load() to return an object
with padding preserved.

[Bug fortran/111271] New: gcc/fortran/trans-expr.cc:1134:8: warning: duplicated ‘if’ condition [-Wduplicated-cond]

[dcb314 at hotmail dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111271

Bug ID: 111271
   Summary: gcc/fortran/trans-expr.cc:1134:8: warning: duplicated
‘if’ condition [-Wduplicated-cond]
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: fortran
  Assignee: unassigned at gcc dot gnu.org
  Reporter: dcb314 at hotmail dot com
  Target Milestone: ---

Source code is

  if (unlimited_poly)
{
   // ...
}
  else if (unlimited_poly)
{ 
  ctree = gfc_class_len_get (var);
  gfc_add_modify (>pre, ctree,
  fold_convert (TREE_TYPE (ctree),
integer_zero_node));
}  

Suggest code rework.

[Bug target/111270] New: gcc/config/i386/i386-options.cc:3039:8: warning: duplicated ‘if ’ condition [-Wduplicated-cond]

[dcb314 at hotmail dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111270

Bug ID: 111270
   Summary: gcc/config/i386/i386-options.cc:3039:8: warning:
duplicated ‘if ’ condition [-Wduplicated-cond]
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: target
  Assignee: unassigned at gcc dot gnu.org
  Reporter: dcb314 at hotmail dot com
  Target Milestone: ---

Source code is

  if (ix86_tune_features [X86_TUNE_AVOID_256FMA_CHAINS])
SET_OPTION_IF_UNSET (opts, opts_set, param_avoid_fma_max_bits, 512);
  else if (ix86_tune_features [X86_TUNE_AVOID_256FMA_CHAINS])
SET_OPTION_IF_UNSET (opts, opts_set, param_avoid_fma_max_bits, 256);
  else if (ix86_tune_features [X86_TUNE_AVOID_128FMA_CHAINS])
SET_OPTION_IF_UNSET (opts, opts_set, param_avoid_fma_max_bits, 128);

Suggest code rework. 

I got this by adding -Wduplicated-cond to a build.

If someone were brave enough to add this warning flag to -Wall, then
other developers could benefit from a well hidden but useful flag.

Re: [PATCH] analyzer: implement reference count checking for CPython plugin [PR107646]

[David Malcolm via Gcc]

On Fri, 2023-09-01 at 04:49 +0200, Hans-Peter Nilsson wrote:
> (Looks like this was committed as r14-3580-g597b9ec69bca8a)
> 
> > Cc: gcc@gcc.gnu.org, gcc-patc...@gcc.gnu.org, Eric Feng
> > 
> > From: Eric Feng via Gcc 
> 
> > gcc/testsuite/ChangeLog:
> >   PR analyzer/107646
> > * gcc.dg/plugin/analyzer_cpython_plugin.c: Implements
> > reference count
> >   * checking for PyObjects.
> > * gcc.dg/plugin/cpython-plugin-test-2.c: Moved to...
> > * gcc.dg/plugin/cpython-plugin-test-PyList_Append.c:
> > ...here (and
> >   * added more tests).
> > * gcc.dg/plugin/cpython-plugin-test-1.c: Moved to...
> > * gcc.dg/plugin/cpython-plugin-test-no-plugin.c: ...here
> > (and added
> >   * more tests).
> > * gcc.dg/plugin/plugin.exp: New tests.
> > * gcc.dg/plugin/cpython-plugin-test-PyList_New.c: New test.
> > * gcc.dg/plugin/cpython-plugin-test-PyLong_FromLong.c: New
> > test.
> > * gcc.dg/plugin/cpython-plugin-test-refcnt-checking.c: New
> > test.
> 
> It seems this was more or less a rewrite, but that said,
> it's generally preferable to always *add* tests, never *modify* them.
> 
> >  .../gcc.dg/plugin/analyzer_cpython_plugin.c   | 376
> > +-
> 
> ^^^ Ouch!  Was it not within reason to keep that test as it
> was, and just add another test?
> 
> Anyway, the test after rewrite fails, and for some targets
> like cris-elf and apparently m68k-linux, yields an error.
> I see a PR was already opened.
> 
> Also, mostly for future reference, several files in the
> patch miss a final newline, as seen by a "\ No newline at
> end of file"-marker.
> 
> I think I found the problem; a mismatch between default C++
> language standard between host-gcc and target-gcc.
> 
> (It's actually *not* as simple as "auto var = typeofvar()"
> not being recognized in C++11 --or else there'd be an error
> for the hash_set declaration too, which I just changed for
> consistency-- but it's close enough for me.)
> 
> With this, retesting plugin.exp for cris-elf works.
> 
> Ok to commit?

Sorry about the failing tests.

Thanks for the patch; please go ahead and commit.

Dave

> 
> -- >8 --
> From: Hans-Peter Nilsson 
> Date: Fri, 1 Sep 2023 04:36:03 +0200
> Subject: [PATCH] testsuite: Fix analyzer_cpython_plugin.c
> declarations, PR testsuite/111264
> 
> Also, add missing newline at end of file.
> 
> PR testsuite/111264
> * gcc.dg/plugin/analyzer_cpython_plugin.c: Make declarations
> C++11-compatible.
> ---
>  gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
> b/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
> index 7af520436549..bf1982e79c37 100644
> --- a/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
> +++ b/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
> @@ -477,8 +477,8 @@ pyobj_refcnt_checker (const region_model *model,
>    if (!ctxt)
>  return;
>  
> -  auto region_to_refcnt = hash_map ();
> -  auto seen_regions = hash_set ();
> +  hash_map region_to_refcnt;
> +  hash_set seen_regions;
>  
>    count_pyobj_references (model, region_to_refcnt, retval,
> seen_regions);
>    check_refcnts (model, old_model, retval, ctxt, region_to_refcnt);
> @@ -561,7 +561,7 @@ public:
>  if (!ctxt)
>    return;
>  region_model *model = cd.get_model ();
> -    auto region_to_refcnt = hash_map ();
> +    hash_map region_to_refcnt;
>  count_all_references(model, region_to_refcnt);
>  dump_refcnt_info(region_to_refcnt, model, ctxt);
>    }
> @@ -1330,4 +1330,4 @@ plugin_init (struct plugin_name_args
> *plugin_info,
>    sorry_no_analyzer ();
>  #endif
>    return 0;
> -}
> \ No newline at end of file
> +}

Re: [PATCH] analyzer: implement reference count checking for CPython plugin [PR107646]

[David Malcolm via Gcc-patches]

On Fri, 2023-09-01 at 04:49 +0200, Hans-Peter Nilsson wrote:
> (Looks like this was committed as r14-3580-g597b9ec69bca8a)
> 
> > Cc: g...@gcc.gnu.org, gcc-patches@gcc.gnu.org, Eric Feng
> > 
> > From: Eric Feng via Gcc 
> 
> > gcc/testsuite/ChangeLog:
> >   PR analyzer/107646
> > * gcc.dg/plugin/analyzer_cpython_plugin.c: Implements
> > reference count
> >   * checking for PyObjects.
> > * gcc.dg/plugin/cpython-plugin-test-2.c: Moved to...
> > * gcc.dg/plugin/cpython-plugin-test-PyList_Append.c:
> > ...here (and
> >   * added more tests).
> > * gcc.dg/plugin/cpython-plugin-test-1.c: Moved to...
> > * gcc.dg/plugin/cpython-plugin-test-no-plugin.c: ...here
> > (and added
> >   * more tests).
> > * gcc.dg/plugin/plugin.exp: New tests.
> > * gcc.dg/plugin/cpython-plugin-test-PyList_New.c: New test.
> > * gcc.dg/plugin/cpython-plugin-test-PyLong_FromLong.c: New
> > test.
> > * gcc.dg/plugin/cpython-plugin-test-refcnt-checking.c: New
> > test.
> 
> It seems this was more or less a rewrite, but that said,
> it's generally preferable to always *add* tests, never *modify* them.
> 
> >  .../gcc.dg/plugin/analyzer_cpython_plugin.c   | 376
> > +-
> 
> ^^^ Ouch!  Was it not within reason to keep that test as it
> was, and just add another test?
> 
> Anyway, the test after rewrite fails, and for some targets
> like cris-elf and apparently m68k-linux, yields an error.
> I see a PR was already opened.
> 
> Also, mostly for future reference, several files in the
> patch miss a final newline, as seen by a "\ No newline at
> end of file"-marker.
> 
> I think I found the problem; a mismatch between default C++
> language standard between host-gcc and target-gcc.
> 
> (It's actually *not* as simple as "auto var = typeofvar()"
> not being recognized in C++11 --or else there'd be an error
> for the hash_set declaration too, which I just changed for
> consistency-- but it's close enough for me.)
> 
> With this, retesting plugin.exp for cris-elf works.
> 
> Ok to commit?

Sorry about the failing tests.

Thanks for the patch; please go ahead and commit.

Dave

> 
> -- >8 --
> From: Hans-Peter Nilsson 
> Date: Fri, 1 Sep 2023 04:36:03 +0200
> Subject: [PATCH] testsuite: Fix analyzer_cpython_plugin.c
> declarations, PR testsuite/111264
> 
> Also, add missing newline at end of file.
> 
> PR testsuite/111264
> * gcc.dg/plugin/analyzer_cpython_plugin.c: Make declarations
> C++11-compatible.
> ---
>  gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
> b/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
> index 7af520436549..bf1982e79c37 100644
> --- a/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
> +++ b/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
> @@ -477,8 +477,8 @@ pyobj_refcnt_checker (const region_model *model,
>    if (!ctxt)
>  return;
>  
> -  auto region_to_refcnt = hash_map ();
> -  auto seen_regions = hash_set ();
> +  hash_map region_to_refcnt;
> +  hash_set seen_regions;
>  
>    count_pyobj_references (model, region_to_refcnt, retval,
> seen_regions);
>    check_refcnts (model, old_model, retval, ctxt, region_to_refcnt);
> @@ -561,7 +561,7 @@ public:
>  if (!ctxt)
>    return;
>  region_model *model = cd.get_model ();
> -    auto region_to_refcnt = hash_map ();
> +    hash_map region_to_refcnt;
>  count_all_references(model, region_to_refcnt);
>  dump_refcnt_info(region_to_refcnt, model, ctxt);
>    }
> @@ -1330,4 +1330,4 @@ plugin_init (struct plugin_name_args
> *plugin_info,
>    sorry_no_analyzer ();
>  #endif
>    return 0;
> -}
> \ No newline at end of file
> +}

Re: [PATCH] analyzer: Add support of placement new and improved operator new [PR105948,PR94355]

[Benjamin Priour via Gcc-patches]

Patch has been updated as per your suggestions and successfully regstrapped
on x86_64-linux-gnu.

call_details::maybe_get_arg_region is now
/* If argument IDX's svalue at the callsite is of pointer type,
return the region it points to.
Otherwise return NULL.  */

const region *
 call_details::deref_ptr_arg (unsigned idx) const
 {
   const svalue *ptr_sval = get_arg_svalue (idx);
   return m_model->deref_rvalue (ptr_sval, get_arg_tree (idx), m_ctxt);
 }


New test is

+
+void test_binop ()
+{
+  char *p = (char *) malloc (4);
+  if (!p)
+return;
+  int32_t *i = ::new (p + 1) int32_t; /* { dg-warning "heap-based buffer
overflow" } */
+  *i = 42; /* { dg-warning "heap-based buffer overflow" } */
+  free (p);
+}

Is it OK for trunk ?
I didn't resend the whole patch as it otherwise was OK.

Thanks,
Benjamin.

On Fri, Sep 1, 2023 at 12:07 PM Benjamin Priour  wrote:

> Hi David,
>
> On Fri, Sep 1, 2023 at 1:59 AM David Malcolm  wrote:
>
>> On Fri, 2023-09-01 at 00:04 +0200, priour...@gmail.com wrote:
>>
>>
> [..snip..]
>
>
>> ...which will only fire if arg 1 is a region_svalue.  This won't
>> trigger if you have e.g. a binop_svalue for pointer arithmetic.
>>
>> What happens e.g. for this one-off-the-end bug:
>>
>>   void *p = malloc (4);
>>   if (!p)
>> return;
>>   int32_t *i = ::new (p + 1) int32_t;
>>   *i = 42;
>>
>> So maybe call_details::maybe_get_arg_region should instead be:
>>
>> /* Return the region that argument IDX points to.  */
>>
>> const region *
>> call_details::deref_ptr_arg (unsigned idx) const
>> {
>>   const svalue *ptr_sval = get_arg_svalue (idx);
>>   return m_model->deref_rvalue (ptr_sval, get_arg_tree (idx), m_ctxt);
>> }
>>
>> (caveat: I didn't test this)
>>
>> > + const region *base_reg = ptr_reg->get_base_region ();
>> > + const svalue *num_bytes_sval = cd.get_arg_svalue (0);
>> > + const region *sized_new_reg
>> > + = mgr->get_sized_region (base_reg,
>> > +  cd.get_lhs_type (),
>> > +  num_bytes_sval);
>>
>> Why do you use the base_reg here, rather than just ptr_reg?
>>
>> In the example above, the *(p + 1) has base region
>> heap_allocated_region, but the ptr_reg is one byte higher; hence
>> check_region_for_write of 4 bytes ought to detect a problem with
>> writing 4 bytes to *(p + 1), but wouldn't complain about the write to
>> *p.
>>
>> ...assuming that I'm reading this code correctly.
>>
>> > + model->check_region_for_write (sized_new_reg,
>> > +nullptr,
>> > +ctxt);
>> > + const svalue *ptr_sval
>> > +   = mgr->get_ptr_svalue (cd.get_lhs_type (), sized_new_reg);
>> > + cd.maybe_set_lhs (ptr_sval);
>> > +   }
>> > +  }
>>
>> [...snip...]
>>
>> The patch is OK for trunk as is; but please can you look into the
>> above.
>>
>>
> Thanks for the test case David, it exposed a missing heap-based over write
> when on the placement new statement.
> I've updated the code as per your suggestions, and it now works properly.
>
>
>> If the above is a problem, you can either do another version of the
>> patch, or do it as a followup patch (whichever you're more comfortable
>> with, but it might be best to get the patch into trunk as-is, given
>> that the GSoC period is nearly over).
>>
>> Thanks
>> Dave
>>
>>
> I will update the patch and regstrap it, so that it is done at once.
> I've compared the new test case to a "C" version of it, resulting in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111266
>
> I will attempt to fix it while I'm regstrapping everything else,
> I still have 4 patches in queue.
> It will give me a brief break from transitioning the tests :)
>
> Thanks for the review,
> Benjamin.
>

[PATCH] c++, v3: Diagnose [basic.scope.block]/2 violations even in compound-stmt of function-try-block [PR52953]

[Jakub Jelinek via Gcc-patches]

On Fri, Sep 01, 2023 at 03:24:54PM +0200, Jakub Jelinek via Gcc-patches wrote:
> So like this?
> 
> It actually changes behaviour on the
> void foo (int x) try {} catch (int x) {} case, where previously
> this triggered the
>|| (TREE_CODE (old) == PARM_DECL
>&& (current_binding_level->kind == sk_catch
>|| current_binding_level->level_chain->kind == 
> sk_catch)
>&& in_function_try_handler))
> {
>   auto_diagnostic_group d;
>   if (permerror (DECL_SOURCE_LOCATION (decl),
>  "redeclaration of %q#D", decl))
> inform (DECL_SOURCE_LOCATION (old),
> "%q#D previously declared here", old);
> diagnostics (note, just the current_binding_level->kind == sk_catch
> case), while now it triggers already the earlier
>   if (b->kind == sk_function_parms)
> {
>   error_at (DECL_SOURCE_LOCATION (decl),
> "declaration of %q#D shadows a parameter", decl);
>   inform (DECL_SOURCE_LOCATION (old),
>   "%q#D previously declared here", old);
> error.  If you think it is important to differentiate that,
> I guess I could guard the while (b->artificial) loop with say
> +   if (!in_function_try_handler
> +   || current_binding_level->kind != sk_catch)
>   while (b->artificial)
> b = b->level_chain;
> and adjust the 2 testcases.

BTW, that in_function_try_handler case doesn't work correctly
(before/after this patch),
void
foo (int x)
try {
}
catch (int)
{
  try {
  } catch (int x)
  {
  }
  try {
  } catch (int)
  {
int x;
  }
}
(which is valid) is rejected, because
|| (TREE_CODE (old) == PARM_DECL
&& (current_binding_level->kind == sk_catch
|| current_binding_level->level_chain->kind == sk_catch)
&& in_function_try_handler))
is true but nothing verified that for the first case
current_binding_level->level_chain->kind == sk_function_params
(with perhaps artificial scopes in between and in the latter case
with one extra level in between).

Here is an untested variant of the patch which does diagnostics of the
in_function_try_handler cases only if it is proven there are no intervening
non-artificial scopes, but uses the old wording + permerror for that case
like before.

Another possibility would be to use permerror for that case but the
"declaration of %q#D shadows a parameter" wording (then we'd need to adjust
both testcases, each on one line).

2023-09-01  Jakub Jelinek  

PR c++/52953
* name-lookup.h (struct cp_binding_level): Add artificial bit-field.
Formatting fixes.
* name-lookup.cc (check_local_shadow): Skip artificial bindings when
checking if parameter scope is parent scope.  Don't special case
FUNCTION_NEEDS_BODY_BLOCK.  Diagnose the in_function_try_handler
cases in the b->kind == sk_function_parms test, verify no
non-artificial intervening scopes but use permerror for that case with
different wording.  Add missing auto_diagnostic_group.
* decl.cc (begin_function_body): Set
current_binding_level->artificial.
* semantics.cc (begin_function_try_block): Likewise.

* g++.dg/diagnostic/redeclaration-3.C: New test.

--- gcc/cp/name-lookup.h.jj 2023-09-01 12:15:22.574619674 +0200
+++ gcc/cp/name-lookup.h2023-09-01 16:11:47.838401045 +0200
@@ -292,11 +292,11 @@ struct GTY(()) cp_binding_level {
   only valid if KIND == SK_TEMPLATE_PARMS.  */
   BOOL_BITFIELD explicit_spec_p : 1;
 
-  /* true means make a BLOCK for this level regardless of all else.  */
+  /* True means make a BLOCK for this level regardless of all else.  */
   unsigned keep : 1;
 
   /* Nonzero if this level can safely have additional
-  cleanup-needing variables added to it.  */
+ cleanup-needing variables added to it.  */
   unsigned more_cleanups_ok : 1;
   unsigned have_cleanups : 1;
 
@@ -308,9 +308,13 @@ struct GTY(()) cp_binding_level {
   unsigned defining_class_p : 1;
 
   /* True for SK_FUNCTION_PARMS of a requires-expression.  */
-  unsigned requires_expression: 1;
+  unsigned requires_expression : 1;
 
-  /* 22 bits left to fill a 32-bit word.  */
+  /* True for artificial blocks which should be ignored when finding
+ parent scope.  */
+  unsigned artificial : 1;
+
+  /* 21 bits left to fill a 32-bit word.  */
 };
 
 /* The binding level currently in effect.  */
--- gcc/cp/name-lookup.cc.jj2023-09-01 12:15:22.566619785 +0200
+++ gcc/cp/name-lookup.cc   2023-09-01 16:19:12.567335710 +0200
@@ -3146,18 +3146,34 @@ check_local_shadow (tree decl)
 them there.  */
  cp_binding_level *b = current_binding_level->level_chain;
 
- if (FUNCTION_NEEDS_BODY_BLOCK (current_function_decl))
-   /* Skip the ctor/dtor cleanup level.  */
+