Re: [PATCH] Docs: Document that taint analyzer checker stops other checkers

2022-03-23 Thread Avinash Sonawane via Gcc-patches
On Wed, 23 Mar 2022 18:33:38 -0400
David Malcolm  wrote:
 
> This is almost ready to push to trunk, but there are a couple of extra
> tasks needed to be done:

Done.

Please find the attached v3 patch and let me know if something needs to
be changed/added. 

Thanks!

Regards,
Avinash Sonawane (rootKea)
https://www.rootkea.me
>From 8248c65cab7d5de72152832d05cfe9b3f8299b40 Mon Sep 17 00:00:00 2001
From: Avinash Sonawane 
Date: Tue, 22 Mar 2022 07:32:44 +0530
Subject: [PATCH] Docs: Document that taint analyzer checker disables some
 warnings

gcc/ChangeLog:
	* doc/invoke.texi: Document that enabling taint analyzer
	checker disables some warnings from `-fanalyzer`.

Signed-off-by: Avinash Sonawane 
---
 gcc/doc/invoke.texi | 33 +++--
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 4da4a1170f5..1d16f0f167f 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -421,14 +421,13 @@ Objective-C and Objective-C++ Dialects}.
 -fanalyzer-checker=@var{name} @gol
 -fno-analyzer-feasibility @gol
 -fanalyzer-fine-grained @gol
--fanalyzer-state-merge @gol
--fanalyzer-state-purge @gol
+-fno-analyzer-state-merge @gol
+-fno-analyzer-state-purge @gol
 -fanalyzer-transitivity @gol
 -fanalyzer-verbose-edges @gol
 -fanalyzer-verbose-state-changes @gol
 -fanalyzer-verbosity=@var{level} @gol
 -fdump-analyzer @gol
--fdump-analyzer-stderr @gol
 -fdump-analyzer-callgraph @gol
 -fdump-analyzer-exploded-graph @gol
 -fdump-analyzer-exploded-nodes @gol
@@ -438,6 +437,7 @@ Objective-C and Objective-C++ Dialects}.
 -fdump-analyzer-feasibility @gol
 -fdump-analyzer-json @gol
 -fdump-analyzer-state-purge @gol
+-fdump-analyzer-stderr @gol
 -fdump-analyzer-supergraph @gol
 -Wno-analyzer-double-fclose @gol
 -Wno-analyzer-double-free @gol
@@ -9659,22 +9659,24 @@ Enabling this option effectively enables the following warnings:
 -Wanalyzer-free-of-non-heap @gol
 -Wanalyzer-malloc-leak @gol
 -Wanalyzer-mismatching-deallocation @gol
--Wanalyzer-possible-null-argument @gol
--Wanalyzer-possible-null-dereference @gol
 -Wanalyzer-null-argument @gol
 -Wanalyzer-null-dereference @gol
+-Wanalyzer-possible-null-argument @gol
+-Wanalyzer-possible-null-dereference @gol
 -Wanalyzer-shift-count-negative @gol
 -Wanalyzer-shift-count-overflow @gol
 -Wanalyzer-stale-setjmp-buffer @gol
+@ignore
 -Wanalyzer-tainted-allocation-size @gol
 -Wanalyzer-tainted-array-index @gol
 -Wanalyzer-tainted-divisor @gol
 -Wanalyzer-tainted-offset @gol
 -Wanalyzer-tainted-size @gol
+@end ignore
 -Wanalyzer-unsafe-call-within-signal-handler @gol
 -Wanalyzer-use-after-free @gol
--Wanalyzer-use-of-uninitialized-value @gol
 -Wanalyzer-use-of-pointer-in-stale-stack-frame @gol
+-Wanalyzer-use-of-uninitialized-value @gol
 -Wanalyzer-write-to-const @gol
 -Wanalyzer-write-to-string-literal @gol
 }
@@ -10015,6 +10017,25 @@ such as the @code{taint} checker that implements
 @option{-Wanalyzer-tainted-array-index}, and this option is required
 to enable them.
 
+@emph{Note:} currently, @option{-fanalyzer-checker=taint} disables following
+warnings from @option{-fanalyzer}:
+
+@gccoptlist{ @gol
+-Wanalyzer-double-fclose @gol
+-Wanalyzer-double-free @gol
+-Wanalyzer-exposure-through-output-file @gol
+-Wanalyzer-file-leak @gol
+-Wanalyzer-free-of-non-heap @gol
+-Wanalyzer-malloc-leak @gol
+-Wanalyzer-mismatching-deallocation @gol
+-Wanalyzer-null-argument @gol
+-Wanalyzer-null-dereference @gol
+-Wanalyzer-possible-null-argument @gol
+-Wanalyzer-possible-null-dereference @gol
+-Wanalyzer-unsafe-call-within-signal-handler @gol
+-Wanalyzer-use-after-free @gol
+}
+
 @item -fno-analyzer-feasibility
 @opindex fanalyzer-feasibility
 @opindex fno-analyzer-feasibility
-- 
2.32.0



hardened conditionals: drop copied identifiers

2022-03-23 Thread Alexandre Oliva via Gcc-patches


The copies of identifiers, indended to associate hardening SSA
temporaries to the original variables they refer to, end up causing
-fcompare-debug to fail, because DECL_UIDs are not identical, and the
nouid flag used in compare-debug dumps doesn't affect the uids in
naked identifiers, so the divergence becomes apparent.

This patch drops the naked identifiers.  Though somewhat desirable,
they're not necessary.

Regstrapped on x86_64-linux-gnu.  Ok to install?


for  gcc/ChangeLog

PR debug/104564
* gimple-harden-conditionals.cc (detach_value): Keep temps
anonymous.

for  gcc/testsuite/ChangeLog

PR debug/104564
* c-c++-common/torture/harden-comp.c: Adjust.
* c-c++-common/torture/harden-cond.c: Adjust.
---
 gcc/gimple-harden-conditionals.cc|   11 ---
 gcc/testsuite/c-c++-common/torture/harden-comp.c |2 +-
 gcc/testsuite/c-c++-common/torture/harden-cond.c |2 +-
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/gcc/gimple-harden-conditionals.cc 
b/gcc/gimple-harden-conditionals.cc
index be01f3ea8c44a..c7e5e077a74f6 100644
--- a/gcc/gimple-harden-conditionals.cc
+++ b/gcc/gimple-harden-conditionals.cc
@@ -126,14 +126,11 @@ detach_value (location_t loc, gimple_stmt_iterator *gsip, 
tree val)
   return val;
 }
 
-  /* Create a SSA "copy" of VAL.  This could be an anonymous
- temporary, but it's nice to have it named after the corresponding
- variable.  Alas, when VAL is a DECL_BY_REFERENCE RESULT_DECL,
- setting (a copy of) it would be flagged by checking, so we don't
- use copy_ssa_name: we create an anonymous SSA name, and then give
- it the same identifier (rather than decl) as VAL.  */
+  /* Create a SSA "copy" of VAL.  It would be nice to have it named
+ after the corresponding variable, but sharing the same decl is
+ problematic when VAL is a DECL_BY_REFERENCE RESULT_DECL, and
+ copying just the identifier hits -fcompare-debug failures.  */
   tree ret = make_ssa_name (TREE_TYPE (val));
-  SET_SSA_NAME_VAR_OR_IDENTIFIER (ret, SSA_NAME_IDENTIFIER (val));
 
   /* Some modes won't fit in general regs, so we fall back to memory
  for them.  ??? It would be ideal to try to identify an alternate,
diff --git a/gcc/testsuite/c-c++-common/torture/harden-comp.c 
b/gcc/testsuite/c-c++-common/torture/harden-comp.c
index 1ee0b3663443d..502f52e25be24 100644
--- a/gcc/testsuite/c-c++-common/torture/harden-comp.c
+++ b/gcc/testsuite/c-c++-common/torture/harden-comp.c
@@ -11,4 +11,4 @@ f (int i, int j)
 /* { dg-final { scan-tree-dump-times "Adding reversed compare" 1 "hardcmp" } } 
*/
 /* { dg-final { scan-tree-dump-times "__builtin_trap" 1 "hardcmp" } } */
 /* { dg-final { scan-tree-dump-times "_\[0-9\]* = i_\[0-9\]*\[(\]D\[)\] < 
j_\[0-9\]*\[(\]D\[)\];" 1 "hardcmp" } } */
-/* { dg-final { scan-tree-dump-times "_\[0-9\]* = i_\[0-9\]* >= j_\[0-9\]*;" 1 
"hardcmp" } } */
+/* { dg-final { scan-tree-dump-times "_\[0-9\]* = _\[0-9\]* >= _\[0-9\]*;" 1 
"hardcmp" } } */
diff --git a/gcc/testsuite/c-c++-common/torture/harden-cond.c 
b/gcc/testsuite/c-c++-common/torture/harden-cond.c
index 86de8e155ed38..213b048b25af5 100644
--- a/gcc/testsuite/c-c++-common/torture/harden-cond.c
+++ b/gcc/testsuite/c-c++-common/torture/harden-cond.c
@@ -15,4 +15,4 @@ f (int i, int j)
 /* { dg-final { scan-tree-dump-times "Adding reversed compare" 2 "hardcbr" } } 
*/
 /* { dg-final { scan-tree-dump-times "__builtin_trap" 2 "hardcbr" } } */
 /* { dg-final { scan-tree-dump-times "if \[(\]i_\[0-9\]*\[(\]D\[)\] < 
j_\[0-9\]*\[(\]D\[)\]\[)\]" 1 "hardcbr" } } */
-/* { dg-final { scan-tree-dump-times "if \[(\]i_\[0-9\]* >= j_\[0-9\]*\[)\]" 2 
"hardcbr" } } */
+/* { dg-final { scan-tree-dump-times "if \[(\]_\[0-9\]* >= _\[0-9\]*\[)\]" 2 
"hardcbr" } } */


-- 
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
   Free Software Activist   GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about 


[PATCH] tree-optimization/104970: Limit size computation for access attribute

2022-03-23 Thread Siddhesh Poyarekar
Limit object size computation only to the simple case where access
attribute has been explicitly specified.  The object passed to
__builtin_dynamic_object_size could either be a pointer or a VLA whose
size has been described only using access attribute.

Further, return a valid size only if the object is a void * pointer or
points to (or is a VLA of) a type that has a constant size.

gcc/ChangeLog:

PR tree-optimization/104970
* tree-object-size.cc (parm_object_size): Restrict size
computation scenarios to explicit access attributes.

gcc/testsuite/ChangeLog:

PR tree-optimization/104970
* gcc.dg/builtin-dynamic-object-size-0.c (test_parmsz_simple2,
test_parmsz_simple3, test_parmsz_extern, test_parmsz_internal,
test_parmsz_internal2, test_parmsz_internal3): New tests.
(main): Use them.

Signed-off-by: Siddhesh Poyarekar 
---

Tested:
- x86_64 bootstrap and test
- x86_64 ubsan bootstrap
- i686 test

 .../gcc.dg/builtin-dynamic-object-size-0.c| 71 +++
 gcc/tree-object-size.cc   | 11 ++-
 2 files changed, 79 insertions(+), 3 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c 
b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
index e5dc23a908d..b5b0b3a677c 100644
--- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
+++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
@@ -380,6 +380,22 @@ test_parmsz_simple (void *obj, size_t sz)
   return __builtin_dynamic_object_size (obj, 0);
 }
 
+size_t
+__attribute__ ((access (__read_write__, 2, 1)))
+__attribute__ ((noinline))
+test_parmsz_simple2 (size_t sz, char obj[])
+{
+  return __builtin_dynamic_object_size (obj, 0);
+}
+
+/* Implicitly constructed access attributes not supported yet.  */
+size_t
+__attribute__ ((noinline))
+test_parmsz_simple3 (size_t sz, char obj[sz])
+{
+  return __builtin_dynamic_object_size (obj, 0);
+}
+
 size_t
 __attribute__ ((noinline))
 __attribute__ ((access (__read_write__, 1, 2)))
@@ -412,6 +428,38 @@ test_parmsz_unknown (void *obj, void *unknown, size_t sz, 
int cond)
   return __builtin_dynamic_object_size (cond ? obj : unknown, 0);
 }
 
+struct S;
+size_t
+__attribute__ ((access (__read_write__, 1, 2)))
+__attribute__ ((noinline))
+test_parmsz_extern (struct S *obj, size_t sz)
+{
+  return __builtin_dynamic_object_size (obj, 0);
+}
+
+/* Implicitly constructed access attributes not supported yet.  */
+size_t
+__attribute__ ((noinline))
+test_parmsz_internal (size_t sz, double obj[][sz])
+{
+  return __builtin_dynamic_object_size (obj, 0);
+}
+
+size_t
+__attribute__ ((access (__read_write__, 2, 1)))
+__attribute__ ((noinline))
+test_parmsz_internal2 (size_t sz, double obj[][sz])
+{
+  return __builtin_dynamic_object_size (obj, 0);
+}
+
+size_t
+__attribute__ ((noinline))
+test_parmsz_internal3 (size_t sz1, size_t sz2, double obj[sz1][sz2])
+{
+  return __builtin_dynamic_object_size (obj, 0);
+}
+
 /* Loops.  */
 
 size_t
@@ -532,9 +580,22 @@ main (int argc, char **argv)
   if (test_parmsz_simple (argv[0], __builtin_strlen (argv[0]) + 1)
   != __builtin_strlen (argv[0]) + 1)
 FAIL ();
+  if (test_parmsz_simple2 (__builtin_strlen (argv[0]) + 1, argv[0])
+  != __builtin_strlen (argv[0]) + 1)
+FAIL ();
+  /* Only explicitly added access attributes are supported for now.  */
+  if (test_parmsz_simple3 (__builtin_strlen (argv[0]) + 1, argv[0]) != -1)
+FAIL ();
   int arr[42];
   if (test_parmsz_scaled (arr, 42) != sizeof (arr))
 FAIL ();
+  if (test_parmsz_scaled (arr, 40) != 40 * sizeof (int))
+FAIL ();
+  /* __bdos cannot see the actual size of ARR, so it will return what it was
+ passed.  Fortunately though the overflow warnings see this caller side and
+ warns of the problematic size.  */
+  if (test_parmsz_scaled (arr, 44) != 44 * sizeof (int)) /* { dg-warning 
"-Wstringop-overflow=" } */
+FAIL ();
   if (test_parmsz_unknown (argv[0], argv[0], __builtin_strlen (argv[0]) + 1, 0)
   != -1)
   if (test_parmsz (argv[0], __builtin_strlen (argv[0]) + 1, -1) != 0)
@@ -550,6 +611,16 @@ main (int argc, char **argv)
 FAIL ();
   if (test_parmsz_scaled_off (arr, 42, 2) != 40 * sizeof (int))
 FAIL ();
+  struct S *s;
+  if (test_parmsz_extern (s, 42) != -1)
+FAIL ();
+  double obj[4][4];
+  if (test_parmsz_internal (4, obj) != -1)
+FAIL ();
+  if (test_parmsz_internal2 (4, obj) != -1)
+FAIL ();
+  if (test_parmsz_internal3 (4, 4, obj) != -1)
+FAIL ();
   if (test_loop (arr, 42, 0, 32, 1) != 10 * sizeof (int))
 FAIL ();
   if (test_loop (arr, 42, 32, -1, -1) != 0)
diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
index b0b50774936..fc062b94d76 100644
--- a/gcc/tree-object-size.cc
+++ b/gcc/tree-object-size.cc
@@ -1477,14 +1477,19 @@ parm_object_size (struct object_size_info *osi, tree 
var)
   tree typesize = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (parm)));
   tree sz = NULL_TREE;
 
-  if (access && access->sizarg != 

[PATCH] rs6000: Support UN[GL][ET] in rs6000_maybe_emit_maxc_minc [PR105002]

2022-03-23 Thread Kewen.Lin via Gcc-patches
Hi,

Commit r12-7687 exposed one miss optimization chance in function
rs6000_maybe_emit_maxc_minc, for now it only considers comparison
codes GE/GT/LE/LT, but it can support more variants with codes
UNLT/UNLE/UNGT/UNGE by reversing them into the equivalent ones
with GE/GT/LE/LT.

Bootstrapped and regtested on powerpc64-linux-gnu P8 and
powerpc64le-linux-gnu P9 and P10.

Is it ok for trunk?

BR,
Kewen
-
PR target/105002

gcc/ChangeLog:

* config/rs6000/rs6000.cc (rs6000_maybe_emit_maxc_minc): Support more
comparison codes UNLT/UNLE/UNGT/UNGE.

---
 gcc/config/rs6000/rs6000.cc | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index 283e8306ff7..b6a2509788f 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -15872,6 +15872,13 @@ rs6000_maybe_emit_maxc_minc (rtx dest, rtx op, rtx 
true_cond, rtx false_cond)
   if (result_mode != compare_mode)
 return false;

+  /* Canonicalize UN[GL][ET] to [LG][TE].  */
+  if (code == UNGE || code == UNGT || code == UNLE || code == UNLT)
+{
+  code = reverse_condition_maybe_unordered (code);
+  std::swap (true_cond, false_cond);
+}
+
   if (code == GE || code == GT)
 max_p = true;
   else if (code == LE || code == LT)
--
2.27.0


Re: [PATCH] rs6000: Skip overload instances with NULL fntype [PR104967]

2022-03-23 Thread Kewen.Lin via Gcc-patches
on 2022/3/23 8:29 PM, David Edelsohn wrote:
> On Wed, Mar 23, 2022 at 5:33 AM Kewen.Lin  wrote:
>>
>> Hi,
>>
>> As shown in PR104967, for some overload built-in function instance,
>> if it requires a date type which isn't defined on the target, its
>> fntype would be initialized as NULL.  This patch is to consider
>> this possibility in function find_instance to avoid ICE.
>>
>> Bootstrapped and regtested on powerpc64-linux-gnu P8 and
>> powerpc64le-linux-gnu P9 and P10.
>>
>> Is it ok for trunk?
> 
> Okay.
> 
> Thanks, David
> 

Thanks David!  Committed as r12-7792-g497bde3ab92b2c.


>> As shown in PR104967, for some overload built-in function instance,
>> if it requires a date type which isn't defined on the target, its
> nit: s/date/data/
> 

Thanks for catching Paul!  Updated in pushed commit log.

BR,
Kewen


hardcmp: split before dispatch edge

2022-03-23 Thread Alexandre Oliva via Gcc-patches


If we harden a compare at the end of a block with an edge to the
abnormal dispatch block, it won't have a single successor.  Arrange to
split the block at its final stmt so as to have a single succ.

Regstrapped on x86_64-linux-gnu.  Ok to install?


for  gcc/ChangeLog

PR middle-end/104975
* gimple-harden-conditionals.cc
(pass_harden_compares::execute): Force split in case of
multiple edges.

for  gcc/testsuite/ChangeLog

PR middle-end/104975
* gcc.dg/pr104975.c: New.
---
 gcc/gimple-harden-conditionals.cc |   12 +---
 gcc/testsuite/gcc.dg/pr104975.c   |   20 
 2 files changed, 29 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr104975.c

diff --git a/gcc/gimple-harden-conditionals.cc 
b/gcc/gimple-harden-conditionals.cc
index 6a5fc3fb9e1a2..be01f3ea8c44a 100644
--- a/gcc/gimple-harden-conditionals.cc
+++ b/gcc/gimple-harden-conditionals.cc
@@ -509,10 +509,16 @@ pass_harden_compares::execute (function *fun)
gsi_insert_before (_split, asgnck, GSI_SAME_STMT);
 
/* We wish to insert a cond_expr after the compare, so arrange
-  for it to be at the end of a block if it isn't.  */
-   if (!gsi_end_p (gsi_split))
+  for it to be at the end of a block if it isn't, and for it
+  to have a single successor in case there's more than
+  one, as in PR104975.  */
+   if (!gsi_end_p (gsi_split)
+   || !single_succ_p (gsi_bb (gsi_split)))
  {
-   gsi_prev (_split);
+   if (!gsi_end_p (gsi_split))
+ gsi_prev (_split);
+   else
+ gsi_split = gsi_last_bb (gsi_bb (gsi_split));
basic_block obb = gsi_bb (gsi_split);
basic_block nbb = split_block (obb, gsi_stmt (gsi_split))->dest;
gsi_next (_split);
diff --git a/gcc/testsuite/gcc.dg/pr104975.c b/gcc/testsuite/gcc.dg/pr104975.c
new file mode 100644
index 0..04532fc444340
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr104975.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -fharden-compares -fno-inline -fno-ipa-pure-const" } */
+
+__attribute__ ((pure, returns_twice)) int
+bar (int);
+
+int
+quux (void)
+{
+  return 0;
+}
+
+int
+foo (short int x)
+{
+  x = !x;
+  bar (quux ());
+
+  return x;
+}


-- 
Alexandre Oliva, happy hackerhttps://FSFLA.org/blogs/lxo/
   Free Software Activist   GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about 


[PATCH] c++: call complete_type after performing auto deduction [PR80351]

2022-03-23 Thread Pokechu22 via Gcc-patches
When cp_finish_decl calls cp_apply_type_quals_to_decl on a const auto or
constexpr auto variable, the type might not be complete the first time
(this happened when auto deduces to an initializer_list).
cp_apply_type_quals_to_decl removes the const qualifier if the type is
not complete, which is appropriate for grokdeclarator, on the assumption
that the type will be complete when called by cp_finish_decl.

Tested on x86_64 Ubuntu under WSL1 by bootstrapping and comparing results
from 24d51e749570dcb85bd43d3b528f58ad6141de26 with results from this
change.  As far as I can tell, this fixes Wunused-var-38.C and
Wunused-var-39.C without regressions.
(Wunused-var-37.C passed before this change.)

gcc/cp/ChangeLog:

PR c++/80351
* decl.cc (cp_finish_decl): Call complete_type after performing
auto deduction.

gcc/testsuite/ChangeLog:

PR c++/80351
* g++.dg/warn/Wunused-var-37.C: New test.
* g++.dg/warn/Wunused-var-38.C: New test.
* g++.dg/warn/Wunused-var-39.C: New test.
---
 gcc/cp/decl.cc |  2 +-
 gcc/testsuite/g++.dg/warn/Wunused-var-37.C | 64 ++
 gcc/testsuite/g++.dg/warn/Wunused-var-38.C | 16 ++
 gcc/testsuite/g++.dg/warn/Wunused-var-39.C | 16 ++
 4 files changed, 97 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/Wunused-var-37.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wunused-var-38.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Wunused-var-39.C

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 34d9dad9fb0..e382b8ad218 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -8098,7 +8098,7 @@ cp_finish_decl (tree decl, tree init, bool
init_const_expr_p,
   TREE_TYPE (decl) = error_mark_node;
   return;
 }
-  cp_apply_type_quals_to_decl (cp_type_quals (type), decl);
+  cp_apply_type_quals_to_decl (cp_type_quals (complete_type (type)), decl);
 }

   if (ensure_literal_type_for_constexpr_object (decl) == error_mark_node)
diff --git a/gcc/testsuite/g++.dg/warn/Wunused-var-37.C
b/gcc/testsuite/g++.dg/warn/Wunused-var-37.C
new file mode 100644
index 000..54e76ac4e11
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wunused-var-37.C
@@ -0,0 +1,64 @@
+// Tangentially to PR c++/80351
+// { dg-do compile { target c++17 } }
+// { dg-options "-Wunused-variable" }
+#include 
+
+// Warnings:
+static int   int_s1  = 0; // { dg-warning "defined but not used" }
+static int   int_s2  = 0; // { dg-warning "defined but not used" }
+inline static intint_is1 = 0; // { dg-warning "defined but not used" }
+inline static intint_is2 = 0; // { dg-warning "defined but not used" }
+// No warnings:
+constexpr static int int_cs1 = 0; // { dg-bogus "defined but not used" }
+constexpr static int int_cs2 = 0; // { dg-bogus "defined but not used" }
+int  int_1   = 0; // { dg-bogus "defined but not used" }
+int  int_2   = 0; // { dg-bogus "defined but not used" }
+inline int   int_i1  = 0; // { dg-bogus "defined but not used" }
+inline int   int_i2  = 0; // { dg-bogus "defined but not used" }
+constexpr intint_c1  = 0; // { dg-bogus "defined but not used" }
+constexpr intint_c2  = 0; // { dg-bogus "defined but not used" }
+
+// Warnings:
+static auto   int_as1  = 0; // { dg-warning "defined but not used" }
+static auto   int_as2  = 0; // { dg-warning "defined but not used" }
+inline static autoint_ais1 = 0; // { dg-warning "defined but not used" }
+inline static autoint_ais2 = 0; // { dg-warning "defined but not used" }
+// No warnings:
+constexpr static auto int_acs1 = 0; // { dg-bogus "defined but not used" }
+constexpr static auto int_acs2 = 0; // { dg-bogus "defined but not used" }
+auto  int_a1   = 0; // { dg-bogus "defined but not used" }
+auto  int_a2   = 0; // { dg-bogus "defined but not used" }
+inline auto   int_ai1  = 0; // { dg-bogus "defined but not used" }
+inline auto   int_ai2  = 0; // { dg-bogus "defined but not used" }
+constexpr autoint_ac1  = 0; // { dg-bogus "defined but not used" }
+constexpr autoint_ac2  = 0; // { dg-bogus "defined but not used" }
+
+// Warnings:
+static std::initializer_list   il_s1  = {0, 1}; // {
dg-warning "defined but not used" }
+static std::initializer_list   il_s2  = {0, 1}; // {
dg-warning "defined but not used" }
+inline static std::initializer_listil_is1 = {0, 1}; // {
dg-warning "defined but not used" }
+inline static std::initializer_listil_is2 = {0, 1}; // {
dg-warning "defined but not used" }
+// No warnings:
+constexpr static std::initializer_list il_cs1 = {0, 1}; // {
dg-bogus "defined but not used" }
+constexpr static std::initializer_list il_cs2 = {0, 1}; // {
dg-bogus "defined but not used" }
+std::initializer_list  il_1   = {0, 1}; // {
dg-bogus "defined but not used" }
+std::initializer_list  il_2   = {0, 1}; // 

Re: [PATCH] c++: FIX_TRUNC_EXPR in tsubst [PR102990]

2022-03-23 Thread Marek Polacek via Gcc-patches
On Wed, Mar 23, 2022 at 04:35:32PM -0400, Jason Merrill wrote:
> On 3/22/22 19:55, Marek Polacek wrote:
> > This is a crash where a FIX_TRUNC_EXPR gets into tsubst_copy_and_build
> > where it hits gcc_unreachable ().
> > 
> > The history of tsubst_copy_and_build/FIX_TRUNC_EXPR is such that it
> > was introduced in r181478, but it did the wrong thing, whereupon it
> > was turned into gcc_unreachable () in r258821 (see this thread:
> > ).
> > 
> > In a template, we should never create a FIX_TRUNC_EXPR (that's what
> > conv_unsafe_in_template_p is for).  But in this test we are NOT in
> > a template when we call digest_nsdmi_init which ends up calling
> > convert_like, converting 1.0e+0 to int, so convert_to_integer_1
> > gives us a FIX_TRUNC_EXPR.
> > 
> > But then when we get to parsing f's parameters, we are in a template
> > when processing decltype(Helpers{}), and since r268321, when the
> > compound literal isn't instantiation-dependent and the type isn't
> > type-dependent, finish_compound_literal falls back to the normal
> > processing, so it calls digest_init, which does fold_non_dependent_init
> > and since the FIX_TRUNC_EXPR isn't dependent, we instantiate and
> > therefore crash in tsubst_copy_and_build.
> 
> Hmm, we shouldn't be doing fold_non_dependent_init on the result of
> get_nsdmi.  Why does that happen?

OK, so we have decltype(Helpers{}), finish_compound_literal gets
Helpers{}, it's not type-dependent, so:

- call digest_init (type=Helpers, init={})
  init is BRACE_ENCLOSED_INITIALIZER_P, type is !TYPE_NON_AGGREGATE_CLASS
  so go down to...
- process_init_constructor_record (type=Helpers, init={})
   - we walk the fields of Helpers, there's 'inputs' of type knob_t
 type_build_ctor_call (knob_t) is true, we want to default-init this
 field
   - create a {} as the init for default-initialization
   - call massage_init_elt (type=knob_t, init={}) to adjust the init
 - we do so by calling digest_init_r (type=knob_t, init={})
   - here we again call process_init_constructor_record, walk knob_t's
 fields, see the field 'value', it has a DECL_INITIAL, so call
 get_nsdmi, that returns the FIX_TRUNC_EXPR we've created before
   - so digesting {} for knob_t produced
 init = {.value=(int) NON_LVALUE_EXPR <1.0e+0>}
 - then we call fold_non_dependent_init on this init, and die

Do we have a mechanism to avoid folding get_nsdmi's trees that I've missed?
 
> > Either I can tweak p_c_e to say that a FIX_TRUNC_EXPR in a template
> > is not potentially constant, or I can just remove the whole F_T_E
> > case, since:
> > a) we could not have created an IMPLICIT_CONV_EXPR here, and
> > b) similar code, FLOAT_EXPR, is not handled here, either.
> > 
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/11?
> > 
> > PR c++/102990
> > 
> > gcc/cp/ChangeLog:
> > 
> > * pt.cc (tsubst_copy_and_build) : Remove.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > * g++.dg/cpp0x/nsdmi-template22.C: New test.
> > * g++.dg/cpp0x/nsdmi-template23.C: New test.
> > ---
> >   gcc/cp/pt.cc  |  4 
> >   gcc/testsuite/g++.dg/cpp0x/nsdmi-template22.C | 13 +
> >   gcc/testsuite/g++.dg/cpp0x/nsdmi-template23.C | 13 +
> >   3 files changed, 26 insertions(+), 4 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp0x/nsdmi-template22.C
> >   create mode 100644 gcc/testsuite/g++.dg/cpp0x/nsdmi-template23.C
> > 
> > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > index 715eea27577..a3becc19290 100644
> > --- a/gcc/cp/pt.cc
> > +++ b/gcc/cp/pt.cc
> > @@ -20184,10 +20184,6 @@ tsubst_copy_and_build (tree t,
> > templated_operator_saved_lookups (t),
> > complain|decltype_flag));
> > -case FIX_TRUNC_EXPR:
> > -  /* convert_like should have created an IMPLICIT_CONV_EXPR.  */
> > -  gcc_unreachable ();
> > -
> >   case ADDR_EXPR:
> > op1 = TREE_OPERAND (t, 0);
> > if (TREE_CODE (op1) == LABEL_DECL)
> > diff --git a/gcc/testsuite/g++.dg/cpp0x/nsdmi-template22.C 
> > b/gcc/testsuite/g++.dg/cpp0x/nsdmi-template22.C
> > new file mode 100644
> > index 000..4ed2501035c
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp0x/nsdmi-template22.C
> > @@ -0,0 +1,13 @@
> > +// PR c++/102990
> > +// { dg-do compile { target c++11 } }
> > +
> > +struct knob_t {
> > +  /* Let's create a FIX_TRUNC_EXPR.  */
> > +  int value = 1.0;
> > +};
> > +
> > +struct Helpers {
> > +  knob_t inputs;
> > +};
> > +
> > +template void f(decltype(Helpers{}));
> > diff --git a/gcc/testsuite/g++.dg/cpp0x/nsdmi-template23.C 
> > b/gcc/testsuite/g++.dg/cpp0x/nsdmi-template23.C
> > new file mode 100644
> > index 000..240cab4347a
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp0x/nsdmi-template23.C
> > @@ -0,0 +1,13 @@
> > +// PR c++/102990
> > +// { dg-do compile { target c++11 } 

Re: [PATCH] Docs: Document that taint analyzer checker stops other checkers

2022-03-23 Thread David Malcolm via Gcc-patches
On Wed, 2022-03-23 at 11:47 +0530, Avinash Sonawane wrote:
> Apologies! Forgot to attach the patch v2.
> 
> Done now.

Thanks for the patch! (forgetting to attach it is something of a rite
of passage)

The content of the v2 patch looks good to me; I tried regenerating the
manpage and I think it's a nice improvement in readability.  I double-
checked the options and I think you got it all correct.

I wondered if there's a dedicated syntax for adding a cautionary note
to a texinfo document, but there doesn't seem to be, so that part of
the patch is good as-is.

This is almost ready to push to trunk, but there are a couple of extra
tasks needed to be done:

(a) the patch needs a ChangeLog entry in its commit message to pass our
git hooks, giving a brief description of what the change is.  See
https://gcc.gnu.org/codingconventions.html#ChangeLogs
and you can use
  ./contrib/gcc-changelog/git_check_commit.py HEAD
to test that the ChangeLog message in the git commit will be pushable.

(b) the patch is now sufficiently large that it might be above the
threshold where the FSF cares about copyright, so please add a sign-off
line to the patch to certify that you wrote it (and various other legal
niceties).  See https://gcc.gnu.org/dco.html


Some more notes on GCC patches can be seen at:
https://gcc.gnu.org/contribute.html


Thanks again for the patch; let me know if anything's unclear.
Dave




[committed] analyzer: fix accessing wrong stack frame on interprocedural return [PR104979]

2022-03-23 Thread David Malcolm via Gcc-patches
PR analyzer/104979 reports a leak false positive when handling an
interprocedural return to a caller:

  LHS = CALL(ARGS);

where the LHS is a certain non-trivial compound expression.

The root cause is that parts of the LHS were being erroneously
evaluated with respect to the stack frame of the called function,
rather than tha of the caller.  When LHS contained a local variable
within the caller as part of certain nested expressions, this local
variable was looked for within the called frame, rather than that of the
caller.  This lookup in the wrong stack frame led to the local variable
being treated as uninitialized, and thus the write to LHS was considered
as writing to a garbage location, leading to the return value being
lost, and thus being considered as a leak.

The region_model code uses the analyzer's path_var class to try to
extend the tree type with stack depth information.  Based on the above,
I think that the path_var class is fundamentally broken, but it's used
in a few other places in the analyzer, so I don't want to rip it out
until the next stage 1.

In the meantime, this patch reworks how region_model::pop_frame works so
that the destination region for an interprocedural return value is
computed after the frame is popped, so that the region_model has the
stack frame for the *caller* at that point.  Doing so fixes the issue.

I attempted a more ambitious fix which moved the storing of the return
svalue into the destination region from region_model::pop_region into
region_model::update_for_return_gcall, with pop_frame returning the
return svalue.  Unfortunately, this regressed g++.dg/analyzer/pr93212.C,
which returns a pointer into a stale frame.
unbind_region_and_descendents and poison_any_pointers_to_descendents are
only set up to poison regions with bindings into the stale frame, not
individual svalues, and updating that became more invasive than I'm
comfortable with in stage 4.

The patch also adds assertions to verify that we have the correct
function when looking up locals/SSA names in a stack frame.  There
doesn't seem to be a general-purpose way to get at the function of an
SSA name, so the assertions go from SSA name to def-stmt to basic_block,
and from there use the analyzer's supergraph to get the function from
the basic_block.  If there's a simpler way to do this, please let me know.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r12-7790-g4cebae0924248b.

gcc/analyzer/ChangeLog:
PR analyzer/104979
* engine.cc (impl_run_checkers): Create the engine after the
supergraph, and pass the supergraph to the engine.
* region-model.cc (region_model::get_lvalue_1): Pass ctxt to
frame_region::get_region_for_local.
(region_model::update_for_return_gcall): Pass the lvalue for the
result to pop_frame as a tree, rather than as a region.
(region_model::pop_frame): Update for above change, determining
the destination region after the frame is popped and thus with
respect to the caller frame rather than the called frame.
Likewise, set the value of the region to the return value after
the frame is popped.
(engine::engine): Add supergraph pointer.
(selftest::test_stack_frames): Set the DECL_CONTECT of PARM_DECLs.
(selftest::test_get_representative_path_var): Likewise.
(selftest::test_state_merging): Likewise.
* region-model.h (region_model::pop_frame): Convert first param
from a const region * to a tree.
(engine::engine): Add param "sg".
(engine::m_sg): New field.
* region.cc: Include "analyzer/sm.h" and
"analyzer/program-state.h".
(frame_region::get_region_for_local): Add "ctxt" param.
Add assertions that VAR_DECLs are locals, and that expr is for the
correct function.
* region.h (frame_region::get_region_for_local): Add "ctxt" param.

gcc/testsuite/ChangeLog:
PR analyzer/104979
* gcc.dg/analyzer/boxed-malloc-1-29.c: Deleted test, moving the
now fixed test_29 to...
* gcc.dg/analyzer/boxed-malloc-1.c: ...here.
* gcc.dg/analyzer/stale-frame-1.c: Add test coverage.

Signed-off-by: David Malcolm 
---
 gcc/analyzer/engine.cc|  4 +-
 gcc/analyzer/region-model.cc  | 50 +++
 gcc/analyzer/region-model.h   |  7 +--
 gcc/analyzer/region.cc| 50 ---
 gcc/analyzer/region.h |  6 ++-
 .../gcc.dg/analyzer/boxed-malloc-1-29.c   | 36 -
 .../gcc.dg/analyzer/boxed-malloc-1.c  |  9 
 gcc/testsuite/gcc.dg/analyzer/stale-frame-1.c | 29 +++
 8 files changed, 120 insertions(+), 71 deletions(-)
 delete mode 100644 gcc/testsuite/gcc.dg/analyzer/boxed-malloc-1-29.c

diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index caa8796b494..0f40e064a22 100644
--- 

Re: [PATCH] c++: extern thread_local declarations in constexpr [PR104994]

2022-03-23 Thread Jason Merrill via Gcc-patches

On 3/22/22 03:35, Jakub Jelinek wrote:

Hi!

C++14 to C++20 apparently should allow extern thread_local declarations in
constexpr functions, however useless they are there (because accessing
such vars is not valid in a constant expression, perhaps sizeof/decltype).
P2242 changed that for C++23 to passing through declaration but
https://cplusplus.github.io/CWG/issues/2552.html
has been filed for it yesterday.

The following patch implements the proposed wording of CWG 2552 in addition
to fixing the C++14 - C++20 handling bug.
If you'd like instead to keep the current pedantic C++23 wording for now,
that would mean taking out the first hunk (cxx_eval_constant_expression) and
g++.dg/cpp23/constexpr-nonlit2.C hunk.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk
(or ok without those 2 hunks for now)?


OK as a whole.


2022-03-22  Jakub Jelinek  

PR c++/104994
* constexpr.cc (cxx_eval_constant_expression): Don't diagnose passing
through extern thread_local declarations.  Change wording from
declaration to definition.
(potential_constant_expression_1): Don't diagnose extern thread_local
declarations.  Change wording from declared to defined.
* decl.cc (start_decl): Likewise.

* g++.dg/diagnostic/constexpr1.C: Change expected diagnostic wording
from declared to defined.
* g++.dg/cpp23/constexpr-nonlit1.C: Likewise.
(garply): Change dg-error into dg-bogus.
* g++.dg/cpp23/constexpr-nonlit2.C: Change expected diagnostic wording
from declaration to definition.
* g++.dg/cpp23/constexpr-nonlit6.C: Change expected diagnostic wording
from declared to defined.
* g++.dg/cpp23/constexpr-nonlit7.C: New test.
* g++.dg/cpp2a/constexpr-try5.C: Change expected diagnostic wording
from declared to defined.
* g++.dg/cpp2a/consteval3.C: Likewise.

--- gcc/cp/constexpr.cc.jj  2022-03-18 19:02:18.964688873 +0100
+++ gcc/cp/constexpr.cc 2022-03-21 15:52:09.000602363 +0100
@@ -6723,17 +6723,18 @@ cxx_eval_constant_expression (const cons
  }
  
  	if (VAR_P (r)

-   && (TREE_STATIC (r) || CP_DECL_THREAD_LOCAL_P (r))
+   && (TREE_STATIC (r)
+   || (CP_DECL_THREAD_LOCAL_P (r) && !DECL_REALLY_EXTERN (r)))
/* Allow __FUNCTION__ etc.  */
&& !DECL_ARTIFICIAL (r))
  {
if (!ctx->quiet)
  {
if (CP_DECL_THREAD_LOCAL_P (r))
- error_at (loc, "control passes through declaration of %qD "
+ error_at (loc, "control passes through definition of %qD "
 "with thread storage duration", r);
else
- error_at (loc, "control passes through declaration of %qD "
+ error_at (loc, "control passes through definition of %qD "
 "with static storage duration", r);
  }
*non_constant_p = true;
@@ -9188,17 +9189,17 @@ potential_constant_expression_1 (tree t,
tmp = DECL_EXPR_DECL (t);
if (VAR_P (tmp) && !DECL_ARTIFICIAL (tmp))
{
- if (CP_DECL_THREAD_LOCAL_P (tmp))
+ if (CP_DECL_THREAD_LOCAL_P (tmp) && !DECL_REALLY_EXTERN (tmp))
{
  if (flags & tf_error)
-   error_at (DECL_SOURCE_LOCATION (tmp), "%qD declared "
+   error_at (DECL_SOURCE_LOCATION (tmp), "%qD defined "
  "% in % context", tmp);
  return false;
}
  else if (TREE_STATIC (tmp))
{
  if (flags & tf_error)
-   error_at (DECL_SOURCE_LOCATION (tmp), "%qD declared "
+   error_at (DECL_SOURCE_LOCATION (tmp), "%qD defined "
  "% in % context", tmp);
  return false;
}
--- gcc/cp/decl.cc.jj   2022-03-14 10:34:34.246922669 +0100
+++ gcc/cp/decl.cc  2022-03-21 15:51:17.053317191 +0100
@@ -5774,15 +5774,15 @@ start_decl (const cp_declarator *declara
&& cxx_dialect < cxx23)
  {
bool ok = false;
-  if (CP_DECL_THREAD_LOCAL_P (decl))
+  if (CP_DECL_THREAD_LOCAL_P (decl) && !DECL_REALLY_EXTERN (decl))
error_at (DECL_SOURCE_LOCATION (decl),
- "%qD declared % in %qs function only "
+ "%qD defined % in %qs function only "
  "available with %<-std=c++2b%> or %<-std=gnu++2b%>", decl,
  DECL_IMMEDIATE_FUNCTION_P (current_function_decl)
  ? "consteval" : "constexpr");
else if (TREE_STATIC (decl))
error_at (DECL_SOURCE_LOCATION (decl),
- "%qD declared % in %qs function only available "
+ "%qD defined % in %qs function only available "
  "with %<-std=c++2b%> or %<-std=gnu++2b%>", decl,
  DECL_IMMEDIATE_FUNCTION_P (current_function_decl)
  ? 

Re: [PATCH] c++: FIX_TRUNC_EXPR in tsubst [PR102990]

2022-03-23 Thread Jason Merrill via Gcc-patches

On 3/22/22 19:55, Marek Polacek wrote:

This is a crash where a FIX_TRUNC_EXPR gets into tsubst_copy_and_build
where it hits gcc_unreachable ().

The history of tsubst_copy_and_build/FIX_TRUNC_EXPR is such that it
was introduced in r181478, but it did the wrong thing, whereupon it
was turned into gcc_unreachable () in r258821 (see this thread:
).

In a template, we should never create a FIX_TRUNC_EXPR (that's what
conv_unsafe_in_template_p is for).  But in this test we are NOT in
a template when we call digest_nsdmi_init which ends up calling
convert_like, converting 1.0e+0 to int, so convert_to_integer_1
gives us a FIX_TRUNC_EXPR.

But then when we get to parsing f's parameters, we are in a template
when processing decltype(Helpers{}), and since r268321, when the
compound literal isn't instantiation-dependent and the type isn't
type-dependent, finish_compound_literal falls back to the normal
processing, so it calls digest_init, which does fold_non_dependent_init
and since the FIX_TRUNC_EXPR isn't dependent, we instantiate and
therefore crash in tsubst_copy_and_build.


Hmm, we shouldn't be doing fold_non_dependent_init on the result of 
get_nsdmi.  Why does that happen?



Either I can tweak p_c_e to say that a FIX_TRUNC_EXPR in a template
is not potentially constant, or I can just remove the whole F_T_E
case, since:
a) we could not have created an IMPLICIT_CONV_EXPR here, and
b) similar code, FLOAT_EXPR, is not handled here, either.

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

PR c++/102990

gcc/cp/ChangeLog:

* pt.cc (tsubst_copy_and_build) : Remove.

gcc/testsuite/ChangeLog:

* g++.dg/cpp0x/nsdmi-template22.C: New test.
* g++.dg/cpp0x/nsdmi-template23.C: New test.
---
  gcc/cp/pt.cc  |  4 
  gcc/testsuite/g++.dg/cpp0x/nsdmi-template22.C | 13 +
  gcc/testsuite/g++.dg/cpp0x/nsdmi-template23.C | 13 +
  3 files changed, 26 insertions(+), 4 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp0x/nsdmi-template22.C
  create mode 100644 gcc/testsuite/g++.dg/cpp0x/nsdmi-template23.C

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 715eea27577..a3becc19290 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -20184,10 +20184,6 @@ tsubst_copy_and_build (tree t,
templated_operator_saved_lookups (t),
complain|decltype_flag));
  
-case FIX_TRUNC_EXPR:

-  /* convert_like should have created an IMPLICIT_CONV_EXPR.  */
-  gcc_unreachable ();
-
  case ADDR_EXPR:
op1 = TREE_OPERAND (t, 0);
if (TREE_CODE (op1) == LABEL_DECL)
diff --git a/gcc/testsuite/g++.dg/cpp0x/nsdmi-template22.C 
b/gcc/testsuite/g++.dg/cpp0x/nsdmi-template22.C
new file mode 100644
index 000..4ed2501035c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/nsdmi-template22.C
@@ -0,0 +1,13 @@
+// PR c++/102990
+// { dg-do compile { target c++11 } }
+
+struct knob_t {
+  /* Let's create a FIX_TRUNC_EXPR.  */
+  int value = 1.0;
+};
+
+struct Helpers {
+  knob_t inputs;
+};
+
+template void f(decltype(Helpers{}));
diff --git a/gcc/testsuite/g++.dg/cpp0x/nsdmi-template23.C 
b/gcc/testsuite/g++.dg/cpp0x/nsdmi-template23.C
new file mode 100644
index 000..240cab4347a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/nsdmi-template23.C
@@ -0,0 +1,13 @@
+// PR c++/102990
+// { dg-do compile { target c++11 } }
+
+struct knob_t {
+  /* Let's create a FLOAT_EXPR.  */
+  double value = 1UL;
+};
+
+struct Helpers {
+  knob_t inputs;
+};
+
+template void f(decltype(Helpers{}));

base-commit: 5d2233f4033dfa37ad88dc2eab138524fe64242e




Re: [PATCH] c++: alias template arguments are evaluated [PR101906]

2022-03-23 Thread Jason Merrill via Gcc-patches

On 3/22/22 14:31, Patrick Palka wrote:

On Tue, 22 Mar 2022, Patrick Palka wrote:


Here we're neglecting to clear cp_unevaluated_operand when substituting
into the arguments of the alias template-id skip<(T(), 0), T> with T=A,
which means cp_unevaluated_operand remains set during mark_used for
A::A() and so we never synthesize it.  Later constant evaluation for
the substituted template argument (A(), 0) (during coerce_template_parms)
fails with "'constexpr A::A()' used before its definition" since it was
never synthesized.


It occurred to me to check the case where 'skip' is a function/variable
template instead of an alias template, and unfortunately seems we run
into the same issue:

   template T skip();  // Function template
   // template T skip; // Variable template

   template
   constexpr unsigned sizeof_() {
 return sizeof(skip<(T(), 0), T>());
 // return sizeof(skip<(T(), 0), T>);
   }

   struct A {
 int m = -1;
   };

   static_assert(sizeof_() == sizeof(A), "");

: In instantiation of ‘constexpr unsigned int sizeof_() [with T = A]’:
:14:25:   required from here
:6:34: error: ‘constexpr A::A()’ used before its definition

We can fix this similarly by clearing cp_unevaluated_operand when
substituting into the arguments of a TEMPLATE_ID_EXPR, but now I'm
worried this cp_unevaluated_operand business might not be the best
approach (despite it being consistent with what tsubst_aggr_type does).

Maybe instantiate_cx_fn_r should be responsible for making sure A::A()
gets synthesized?


Or cxx_eval_call_expression, but just as a workaround: 
manifestly-constant-evaluated expressions are evaluated even in an 
unevaluated operand, so I think adjusting cp_unevaluated_operand is correct.


Perhaps tsubst_template_args should use cp_evaluated, and places that 
use plain tsubst for substituting template args should use it instead?


Jason



Re: [pushed] c++: designated init and aggregate members [PR102337]

2022-03-23 Thread Jason Merrill via Gcc-patches

On 3/22/22 10:19, Patrick Palka wrote:

On Mon, 21 Mar 2022, Jason Merrill via Gcc-patches wrote:


Our C++20 designated initializer handling was broken with members of class
type; we would find the relevant member and then try to find a member of
the member with the same name.  Or we would sometimes ignore the designator
entirely.  The former problem is fixed by the change to reshape_init_class,
the latter by the change to reshape_init_r.

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

PR c++/103337
PR c++/102740
PR c++/103299
PR c++/102538

gcc/cp/ChangeLog:

* decl.cc (reshape_init_class): Avoid looking for designator
after we found it.
(reshape_init_r): Keep looking for designator.

gcc/testsuite/ChangeLog:

* g++.dg/ext/flexary3.C: Remove one error.
* g++.dg/parse/pr43765.C: Likewise.
* g++.dg/cpp2a/desig22.C: New test.
* g++.dg/cpp2a/desig23.C: New test.
* g++.dg/cpp2a/desig24.C: New test.
* g++.dg/cpp2a/desig25.C: New test.
---
  gcc/cp/decl.cc   | 47 +---
  gcc/testsuite/g++.dg/cpp2a/desig22.C | 11 +++
  gcc/testsuite/g++.dg/cpp2a/desig23.C | 20 
  gcc/testsuite/g++.dg/cpp2a/desig24.C | 11 +++
  gcc/testsuite/g++.dg/cpp2a/desig25.C | 13 
  gcc/testsuite/g++.dg/ext/flexary3.C  |  2 +-
  gcc/testsuite/g++.dg/parse/pr43765.C |  6 ++--
  7 files changed, 101 insertions(+), 9 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/desig22.C
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/desig23.C
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/desig24.C
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/desig25.C

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 375385e0013..34d9dad9fb0 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -6598,8 +6598,9 @@ reshape_init_class (tree type, reshape_iter *d, bool 
first_initializer_p,
  {
tree field_init;
constructor_elt *old_cur = d->cur;
+  bool direct_desig = false;
  
-  /* Handle designated initializers, as an extension.  */

+  /* Handle C++20 designated initializers.  */
if (d->cur->index)
{
  if (d->cur->index == error_mark_node)
@@ -6617,7 +6618,10 @@ reshape_init_class (tree type, reshape_iter *d, bool 
first_initializer_p,
}
}
  else if (TREE_CODE (d->cur->index) == IDENTIFIER_NODE)
-   field = get_class_binding (type, d->cur->index);
+   {
+ field = get_class_binding (type, d->cur->index);
+ direct_desig = true;
+   }
  else
{
  if (complain & tf_error)
@@ -6669,6 +6673,7 @@ reshape_init_class (tree type, reshape_iter *d, bool 
first_initializer_p,
  break;
  gcc_assert (aafield);
  field = aafield;
+ direct_desig = false;
}
}
  
@@ -6683,9 +6688,32 @@ reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p,

   assumed to correspond to no elements of the initializer list.  */
goto continue_;
  
-  field_init = reshape_init_r (TREE_TYPE (field), d,

-  /*first_initializer_p=*/NULL_TREE,
-  complain);
+  if (direct_desig)
+   {
+ /* The designated field F is initialized from this one element:
+Temporarily clear the designator so a recursive reshape_init_class
+doesn't try to find it again in F, and adjust d->end so we don't
+try to use the next initializer to initialize another member of F.
+
+Note that we don't want these changes if we found the designator
+inside an anon aggr above; we leave them alone to implement:
+
+"If the element is an anonymous union member and the initializer
+list is a brace-enclosed designated- initializer-list, the element
+is initialized by the designated-initializer-list { D }, where D
+is the designated- initializer-clause naming a member of the
+anonymous union member."  */
+ auto end_ = make_temp_override (d->end, d->cur + 1);
+ auto idx_ = make_temp_override (d->cur->index, NULL_TREE);


IIUC this override of d->cur->index mutates the underlying CONSTRUCTOR, which
is probably fine, but I wonder if it'd be worthwhile to avoid the mutation by
using a temporary reshape_iter for the recursive call, something like:

   constructor_elt elt = { /*index=*/NULL_TREE, d->cur->value };
   reshape_iter iter = { ,  + 1 };
   field_init = reshape_init_r (TREE_TYPE (field), ,
/*first_initializer_p=*/NULL_TREE,
complain);
   d->cur++;


Sure, makes sense.  I also decided to factor it out.
From b64fe36a7e539a26ac0a32aeca2258c23ac3a2df Mon Sep 17 00:00:00 2001
From: Jason Merrill 
Date: Wed, 23 Mar 

Re: [PATCH] rtl-optimization/105028 - fix compile-time hog in form_threads_from_copies

2022-03-23 Thread Vladimir Makarov via Gcc-patches



On 2022-03-23 07:49, Richard Biener wrote:

form_threads_from_copies processes a sorted array of copies, skipping
those with the same thread and conflicting threads and merging the
first non-conflicting ones.  After that it terminates the loop and
gathers the remaining elements of the array, skipping same thread
copies, re-starting the process.  For a large number of copies this
gathering of the rest takes considerable time and it also appears
pointless.  The following simply continues processing the array
which should be equivalent as far as I can see.


It looks the same to me that the result code is equivalent to the 
original one.


As I remember originally it was more sophisticated but even more slower 
algorithm taking into account that merging 2 threads could remove 
several copies (not just one) from the array and choosing the best copy 
with this point of view.  It was transformed into this ineffective 
leftover code.



This takes form_threads_from_copies off the profile radar from
previously taking ~50% of the compile-time.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

OK if testing succeeds?

Yes.  Thank you for working on this, Richard.



Re: [pushed] c++: using from enclosing class template [PR105006]

2022-03-23 Thread Jason Merrill via Gcc-patches

On 3/23/22 10:29, Patrick Palka wrote:

I wonder if it'd be worthwhile to also test dependent_type_p (scope) here
here to avoid iterating over the lookup set when it can't possibly contain
a dependent using-decl.


Good thought:

From b1005f60c17d693e9fbc38a9481b3cd896d26785 Mon Sep 17 00:00:00 2001
From: Jason Merrill 
Date: Wed, 23 Mar 2022 12:22:20 -0400
Subject: [PATCH] c++: tweak PR105006 fix
To: gcc-patches@gcc.gnu.org

Checking dependent_type_p avoids needing to walk the overloads in cases
where it would not be possible to find a dependent using.

	PR c++/105006

gcc/cp/ChangeLog:

	* name-lookup.cc (lookup_using_decl): Check that scope is
	a dependent type before looking for dependent using.
---
 gcc/cp/name-lookup.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index ea947fabb7e..3c7b626350f 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -5667,7 +5667,7 @@ lookup_using_decl (tree scope, name_lookup )
 
   /* If the lookup in the base contains a dependent using, this
 	 using is also dependent.  */
-  if (!dependent_p && lookup.value)
+  if (!dependent_p && lookup.value && dependent_type_p (scope))
 	{
 	  tree val = lookup.value;
 	  if (tree fns = maybe_get_fns (val))
-- 
2.27.0



Re: [PATCH v2] Document that the 'access' and 'nonnull' attributes are independent

2022-03-23 Thread Sebastian Huber

On 23/03/2022 17:31, Martin Sebor via Gcc-patches wrote:


The concern is that the constraints implied by atttributes access and
nonnull are independent of each other.  I would suggest to document
that without talking about dereferencing because that's not implied
by either of them.  E.g., something like this (feel free to tweak it
as you see fit):

   Note that the @code{access} attribute doesn't imply the same
   constraint as attribute @code{nonnull} (@pxref{Attribute nonnull}).
   The latter attribute should be used to annotate arguments that must
   never be null, regardless of the value of the size argument.


I would not give an advice on using the nonnull attribute here. This 
attribute could have pretty dangerous effects in the function definition 
(removal of null pointer checks).


--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/


Re: [PATCH] ipa: Create LOAD references when necessary during inlining (PR 103171)

2022-03-23 Thread Martin Jambor
Hello,

I would like to ping this patch, please.

Thanks,

Martin

On Fri, Jan 28 2022, Martin Jambor wrote:
> Hi,
>
> in r12-2523-g13586172d0b70c ipa-prop tracking of jump functions during
> inlining got the ability to remove ADDR references when inlining
> discovered that they were not necessary or turn them into LOAD
> references when we know that what was a function call argument passed
> by reference will end up as a load (one or more).
>
> Unfortunately, the code only creates the LOAD references when
> replacing removed ADDR references and PR 103171 showed that with some
> ordering of inlining, we need to add the LOAD reference before we know
> we can remove the ADDR one - or the reference will be lost, leading to
> link errors or even ICEs.
>
> Specifically in testcase gcc.dg/lto/pr103171_1.c added in this patch,
> if foo() is inlined to entry(), we need to create the LOAD reference
> so that when later bar() is inlined into foo() and we discover that
> the paameter is unused, we can remove the ADDR reference and still
> keep the varaible around for the load.
>
> Bootstrapped, LTO bootstrapped and tested on x86_64-linux.  OK for
> trunk?
>
> Thanks,
>
> Martin
>
>
>
> gcc/ChangeLog:
>
> 2022-01-28  Martin Jambor  
>
>   PR ipa/103171
>   * ipa-prop.cc (propagate_controlled_uses): Add a LOAD reference
>   always when an ADDR_EXPR constant is known to reach a load because
>   of inlining, not just when removing an ADDR reference.
>
> gcc/testsuite/ChangeLog:
>
> 2022-01-28  Martin Jambor  
>
>   PR ipa/103171
>   * gcc.dg/ipa/remref-6.c: Adjust dump scan string.
>   * gcc.dg/ipa/remref-7.c: New test.
>   * gcc.dg/lto/pr103171_0.c: New test.
>   * gcc.dg/lto/pr103171_1.c: Likewise.
> ---
>  gcc/ipa-prop.cc   | 30 ---
>  gcc/testsuite/gcc.dg/ipa/remref-6.c   |  2 +-
>  gcc/testsuite/gcc.dg/ipa/remref-7.c   | 33 +
>  gcc/testsuite/gcc.dg/lto/pr103171_0.c | 11 +
>  gcc/testsuite/gcc.dg/lto/pr103171_1.c | 35 +++
>  5 files changed, 96 insertions(+), 15 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/ipa/remref-7.c
>  create mode 100644 gcc/testsuite/gcc.dg/lto/pr103171_0.c
>  create mode 100644 gcc/testsuite/gcc.dg/lto/pr103171_1.c
>
> diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc
> index e55fe2776f2..72aa3e2f60d 100644
> --- a/gcc/ipa-prop.cc
> +++ b/gcc/ipa-prop.cc
> @@ -4181,6 +4181,20 @@ propagate_controlled_uses (struct cgraph_edge *cs)
> int d = ipa_get_controlled_uses (old_root_info, i);
> int c = rdesc->refcount;
> rdesc->refcount = combine_controlled_uses_counters (c, d);
> +   if (rdesc->refcount != IPA_UNDESCRIBED_USE
> +   && ipa_get_param_load_dereferenced (old_root_info, i))
> + {
> +   tree cst = ipa_get_jf_constant (jf);
> +   gcc_checking_assert (TREE_CODE (cst) == ADDR_EXPR
> +&& (TREE_CODE (TREE_OPERAND (cst, 0))
> +== VAR_DECL));
> +   symtab_node *n = symtab_node::get (TREE_OPERAND (cst, 0));
> +   new_root->create_reference (n, IPA_REF_LOAD, NULL);
> +   if (dump_file)
> + fprintf (dump_file, "ipa-prop: Address IPA constant will reach "
> +  "a load so adding LOAD reference from %s to %s.\n",
> +  new_root->dump_name (), n->dump_name ());
> + }
> if (rdesc->refcount == 0)
>   {
> tree cst = ipa_get_jf_constant (jf);
> @@ -4193,20 +4207,8 @@ propagate_controlled_uses (struct cgraph_edge *cs)
> symtab_node *n = symtab_node::get (TREE_OPERAND (cst, 0));
> if (n)
>   {
> -   struct cgraph_node *clone;
> -   bool removed = remove_described_reference (n, rdesc);
> -   /* The reference might have been removed by IPA-CP.  */
> -   if (removed
> -   && ipa_get_param_load_dereferenced (old_root_info, i))
> - {
> -   new_root->create_reference (n, IPA_REF_LOAD, NULL);
> -   if (dump_file)
> - fprintf (dump_file, "ipa-prop: ...replaced it with "
> -  "LOAD one from %s to %s.\n",
> -  new_root->dump_name (), n->dump_name ());
> - }
> -
> -   clone = cs->caller;
> +   remove_described_reference (n, rdesc);
> +   cgraph_node *clone = cs->caller;
> while (clone->inlined_to
>&& clone->ipcp_clone
>&& clone != rdesc->cs->caller)
> diff --git a/gcc/testsuite/gcc.dg/ipa/remref-6.c 
> b/gcc/testsuite/gcc.dg/ipa/remref-6.c
> index 7deae3114a4..f31f4c14319 100644
> --- a/gcc/testsuite/gcc.dg/ipa/remref-6.c
> +++ b/gcc/testsuite/gcc.dg/ipa/remref-6.c
> @@ -20,5 +20,5 @@ void entry()
>  }
>  
>  /* { 

Re: [PATCH v2] Document that the 'access' and 'nonnull' attributes are independent

2022-03-23 Thread Martin Sebor via Gcc-patches

On 3/23/22 07:01, David Malcolm wrote:

On Mon, 2022-03-14 at 16:18 -0600, Martin Sebor wrote:

On 3/9/22 14:57, David Malcolm via Gcc wrote:

On Wed, 2022-03-09 at 13:30 -0800, Andrew Pinski wrote:

On Wed, Mar 9, 2022 at 1:25 PM David Malcolm via Gcc
 wrote:


We gained __attribute__ ((access, ...)) in GCC 10:

https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html

which identifies one of the pointer/reference arguments of a
function
as being accessed according to an access-mode: read_only,
read_write,
write_only, or none.

We also have __attribute__ ((nonnull)) to indicate that a
function
argument (or all of them) must be non-NULL.

There doesn't seem to be a relationship between these in the
implementation, but it strikes me that almost anywhere that a
user
might use the "access" attribute, that parameter is probably
going
to
be required to be nonnull - though perhaps there are cases
where
APIs
check for NULL and reject them gracefully?


No, I think they are separate. The access just says these access
attributes are read only, write only, read-write or don't access
what
the pointer points to; it does not say they have to be read or
written
to.
I think it is a bad idea to connect the two ideas because you
could
have some cases where an argument is optional but is only read
from;
or is only written to (there are many in GCC sources even).


Thanks for the clarification...



Thanks,
Andrew Pinski



Might we want to somehow make __attribute__ ((access, ...))
imply
__attribute__ ((nonnull))?  (for non "none" access modes,
perhaps?)

If so, one place to implement this might be in tree.cc's
get_nonnull_args, and have it add to the bitmap any arguments
that
have an appropriate access attribute.

get_nonnull_args is used in various places:
- validating builtins
- in ranger_cache::block_apply_nonnull
- by -Wnonnull (in pass_post_ipa_warn::execute)
- by -Wanalyzer-possible-null-argument and -Wanalyzer-null-
argument;
I'm tracking the failure of these last two to make use of
__attribute__
((access)) in PR analyzer/104860.

So do we:

(a) leave it up to the user, requiring them to specify
__attribute__
((nonnull)) in addition to  __attribute__ ((access, ...))


...so that's (a) then.

I think it might be more user-friendly to be explicit about this in
the
documentation, maybe something like the attached?


I agree it's worth clarifying the manual.

But I don't think there's a way to annotate a function to indicate
that it will definitely access an object (or dereference a pointer).
Attribute access just implies that it might dereference it (unless
the size is zero), and attribute nonnull that the pointer must not
be null, not that it will be dereferenced (or even that it must be
valid, although that's implied by the language and should probably
be enforced in all contexts by some other warning).

The combination of access with nonzero size and nonnull only means
that the pointer must be nonnull and point to an object with at least
size elements.


Here's an updated version of the patch which expresses that also.

OK for trunk?

Dave



Martin



(not yet fully tested, but seems to build)

Dave






(b) leave it up to the individual sites in GCC that currently
make
use
of get_nonnull_args to add logic for handling   __attribute__
((access,
...))

(c) extend get_nonnull_args

?

Thoughts?
Dave









gcc/ChangeLog:
* doc/extend.texi (Common Function Attributes): Document that
'access' does not imply 'nonnull'.

Signed-off-by: David Malcolm 
---
  gcc/doc/extend.texi | 9 +
  1 file changed, 9 insertions(+)

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index a4a25e86928..790014f3da5 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -2652,6 +2652,15 @@ The mode is intended to be used as a means to help 
validate the expected
  object size, for example in functions that call @code{__builtin_object_size}.
  @xref{Object Size Checking}.
  
+Note that the @code{access} attribute merely specifies how an object

+referenced by the pointer argument can be accessed; it does not imply that
+an access @strong{will} happen.  If the pointer will definitely be
+dereferenced, you may want to also add
+@code{__attribute__((nonnull (@var{arg-index})))} to the function for the
+pointer parameter in question (though the presence of the @code{nonnull}
+attribute does not imply that the parameter will be dereferenced, merely
+that it must be non-null).
+
  @item alias ("@var{target}")
  @cindex @code{alias} function attribute
  The @code{alias} attribute causes the declaration to be emitted as an alias


I'm sorry, I probably wasn't clear.  This text sounds contradictory:

  If the pointer will definitely be dereferenced ... does not imply
  that the parameter will be dereferenced...

The concern is that the constraints implied by atttributes access and
nonnull are independent of each other.  I would suggest to document
that without talking about dereferencing because 

Re: [PATCH PING] ipa: Careful processing ANCESTOR jump functions and NULL pointers (PR 103083)

2022-03-23 Thread Martin Jambor
Hello,

I would like to ping this patch, please.

Thanks,

Martin

On Mon, Feb 14 2022, Martin Jambor wrote:
> Hello Honza,
>
> On Mon, Dec 13 2021, Jan Hubicka wrote:
>>> >>> + || (only_for_nonzero && 
>>> >>> !src_lats->bits_lattice.known_nonzero_p ()))
>>> >>> +   {
>>> >>> + if (jfunc->bits)
>>> >>> +   return dest_lattice->meet_with (jfunc->bits->value,
>>> >>> +   jfunc->bits->mask, 
>>> >>> precision);
>>> >>> + else
>>> >>> +   return dest_lattice->set_to_bottom ();
>>> >>> +   }
>>> >> I do not think you need to set to bottom here. For pointers, we
>>> >> primarily track alignment and NULL is aligned - all you need to do is to
>>> >> make sure that we do not believe that some bits are 1.
>>> >
>>> > I am not sure I understand, the testcase you wrote has all bits as zeros
>>> > and still miscompiles?  It is primarily used for alignment but not only
>>> > for that.
>>
>> Maybe I misunderstand the code.  But if you have only_for_nonzero and
>> you do not know htat src lattice is non-zero you will get
>>  - if src is 0, then dest is 0
>>  - if src is non-zero then dest is src+offset
>>
>> So all trialing bits that are known to be 0 in src and offset are known
>> to be 0 in the result.  But I do not see where th eoffset is mixed in?
>>
>
> I went back and tried to figure out again what you meant and I hope it
> was the following patch, which does not drop the lattice to bottom for
> ANCESTORs but instead masks any bits that would be considered known
> ones.  It is indeed clever and I did not see the possibility when
> writing the patch.
>
> The following has passed bootstrap, LTO bootstrap and testing on
> x86-64.  OK for trunk (and then to be backported to 11 and 10)?
>
> Thanks,
>
> Martin
>
>
> IPA_JF_ANCESTOR jump functions are constructed also when the formal
> parameter of the caller is first checked whether it is NULL and left
> as it is if it is NULL, to accommodate C++ casts to an ancestor class.
>
> The jump function type was invented for devirtualization and IPA-CP
> propagation of tree constants is also careful to apply it only to
> existing DECLs(*) but as PR 103083 shows, the part propagating "known
> bits" was not careful about this, which can lead to miscompilations.
>
> This patch introduces a flag to the ancestor jump functions which
> tells whether a NULL-check was elided when creating it and makes the
> bits propagation behave accordingly, masking any bits otherwise would
> be known to be one.  This should safely preserve alignment info, which
> is the primary ifnormation that we keep in bits for pointers.
>
> (*) There still may remain problems when a DECL resides on address
> zero (with -fno-delete-null-pointer-checks ...I hope it cannot happen
> otherwise).  I am looking into that now but I think it will be easier
> for everyone if I do so in a follow-up patch.
>
> gcc/ChangeLog:
>
> 2022-02-11  Martin Jambor  
>
>   PR ipa/103083
>   * ipa-prop.h (ipa_ancestor_jf_data): New flag keep_null;
>   (ipa_get_jf_ancestor_keep_null): New function.
>   * ipa-prop.c (ipa_set_ancestor_jf): Initialize keep_null field of the
>   ancestor function.
>   (compute_complex_assign_jump_func): Pass false to keep_null
>   parameter of ipa_set_ancestor_jf.
>   (compute_complex_ancestor_jump_func): Pass true to keep_null
>   parameter of ipa_set_ancestor_jf.
>   (update_jump_functions_after_inlining): Carry over keep_null from the
>   original ancestor jump-function or merge them.
>   (ipa_write_jump_function): Stream keep_null flag.
>   (ipa_read_jump_function): Likewise.
>   (ipa_print_node_jump_functions_for_edge): Print the new flag.
>   * ipa-cp.c (class ipcp_bits_lattice): Make various getters const.  New
>   member function known_nonzero_p.
>   (ipcp_bits_lattice::known_nonzero_p): New.
>   (ipcp_bits_lattice::meet_with_1): New parameter drop_all_ones,
>   observe it.
>   (ipcp_bits_lattice::meet_with): Likewise.
>   (propagate_bits_across_jump_function): Simplify.  Pass true in
>   drop_all_ones when it is necessary.
>   (propagate_aggs_across_jump_function): Take care of keep_null
>   flag.
>   (ipa_get_jf_ancestor_result): Propagate NULL accross keep_null
>   jump functions.
>
> gcc/testsuite/ChangeLog:
>
> 2021-11-25  Martin Jambor  
>
>   * gcc.dg/ipa/pr103083-1.c: New test.
>   * gcc.dg/ipa/pr103083-2.c: Likewise.
> ---
>  gcc/ipa-cp.cc | 75 ++-
>  gcc/ipa-prop.cc   | 20 +--
>  gcc/ipa-prop.h| 13 +
>  gcc/testsuite/gcc.dg/ipa/pr103083-1.c | 28 ++
>  gcc/testsuite/gcc.dg/ipa/pr103083-2.c | 30 +++
>  5 files changed, 137 insertions(+), 29 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/ipa/pr103083-1.c
>  create mode 100644 gcc/testsuite/gcc.dg/ipa/pr103083-2.c
>
> diff 

Re: [PATCH PING] ipa-cp: Do not create clones for values outside known value range (PR 102513)

2022-03-23 Thread Martin Jambor
Hello,

I would like to ping this patch, please.

Thanks,

Martin


On Mon, Feb 14 2022, Martin Jambor wrote:
> Hi,
>
> PR 102513 shows we emit bogus array access warnings when IPA-CP
> creates clones specialized for values which it deduces from arithmetic
> jump functions describing self-recursive calls.  Those can however be
> avoided if we consult the IPA-VR information that the same pass also
> has.
>
> The patch below does that at the stage when normally values are only
> examined for profitability.  It would be better not to create lattices
> describing such bogus values in the first place, however that presents
> an ordering problem, the pass currently propagates all information,
> and so both constants and VR, in no particular order when processing
> SCCs, and so this approach seemed much simpler.
>
> I plan to rearrange the pass so that it clones in multiple passes over
> the call graph (or rather the lattice dependence graph) and it feels
> natural to only do propagation for these kinds of recursion in the
> second or later passes, which would fix the issue more elegantly.
>
> Bootstrapped and tested on x86_64, OK for trunk (and perhaps also for
> GCC 11)?
>
> Thanks,
>
> Martin
>
>
> gcc/ChangeLog:
>
> 2022-02-14  Martin Jambor  
>
>   PR ipa/102513
>   * ipa-cp.cc (decide_whether_version_node): Skip scalar values
>   which do not fit the known value_range.
>
> gcc/testsuite/ChangeLog:
>
> 2022-02-14  Martin Jambor  
>
>   PR ipa/102513
>   * gcc.dg/ipa/pr102513.c: New test.
> ---
>  gcc/ipa-cp.cc   | 28 ++--
>  gcc/testsuite/gcc.dg/ipa/pr102513.c | 33 +
>  2 files changed, 59 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/ipa/pr102513.c
>
> diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc
> index 453e9c93cc3..ec37632d487 100644
> --- a/gcc/ipa-cp.cc
> +++ b/gcc/ipa-cp.cc
> @@ -6154,8 +6154,32 @@ decide_whether_version_node (struct cgraph_node *node)
>   {
> ipcp_value *val;
> for (val = lat->values; val; val = val->next)
> - ret |= decide_about_value (node, i, -1, val, ,
> -_gen_clones);
> + {
> +   /* If some values generated for self-recursive calls with
> +  arithmetic jump functions fall outside of the known
> +  value_range for the parameter, we can skip them.  VR interface
> +  supports this only for integers now.  */
> +   if (TREE_CODE (val->value) == INTEGER_CST
> +   && !plats->m_value_range.bottom_p ()
> +   && !plats->m_value_range.m_vr.contains_p (val->value))
> + {
> +   /* This can happen also if a constant present in the source
> +  code falls outside of the range of parameter's type, so we
> +  cannot assert.  */
> +   if (dump_file && (dump_flags & TDF_DETAILS))
> + {
> +   fprintf (dump_file, " - skipping%s value ",
> +val->self_recursion_generated_p ()
> +? " self_recursion_generated" : "");
> +   print_ipcp_constant_value (dump_file, val->value);
> +   fprintf (dump_file, " because it is outside known "
> +"value range.\n");
> + }
> +   continue;
> + }
> +   ret |= decide_about_value (node, i, -1, val, ,
> +  _gen_clones);
> + }
>   }
>  
>if (!plats->aggs_bottom)
> diff --git a/gcc/testsuite/gcc.dg/ipa/pr102513.c 
> b/gcc/testsuite/gcc.dg/ipa/pr102513.c
> new file mode 100644
> index 000..9ee5431b730
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/ipa/pr102513.c
> @@ -0,0 +1,33 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -Warray-bounds" } */
> +
> +extern int block2[7][256];
> +
> +static int encode_block(int block2[7][256], unsigned level)
> +{
> +int best_score = 0;
> +
> +for (unsigned x = 0; x < level; x++) {
> +int v = block2[1][x];
> +block2[level][x] = 0;
> +best_score += v * v;
> +}
> +
> +if (level > 0 && best_score > 64) {
> +int score = 0;
> +
> +score += encode_block(block2, level - 1);
> +score += encode_block(block2, level - 1);
> +
> +if (score < best_score) {
> +best_score = score;
> +}
> +}
> +
> +return best_score;
> +}
> +
> +int foo(void)
> +{
> +return encode_block(block2, 5);
> +}
> -- 
> 2.34.1


[PATCH] target/102125 - alternative memcpy folding improvement

2022-03-23 Thread Richard Biener via Gcc-patches
The following extends the heuristical memcpy folding path with the
ability to use misaligned accesses on strict-alignment targets just
like the size-based path does.  That avoids regressing the following
testcase on arm

uint64_t bar64(const uint8_t *rData1)
{
uint64_t buffer;
memcpy(, rData1, sizeof(buffer));
return buffer;
}

when r12-3482-g5f6a6c91d7c592 is reverted.

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

OK to revert r12-3482-g5f6a6c91d7c592?

Thanks,
Richard.

2022-03-23  Richard Biener  

PR target/102125
* gimple-fold.cc (gimple_fold_builtin_memory_op): Allow the
use of movmisalign when either the source or destination
decl is properly aligned.
---
 gcc/gimple-fold.cc | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
index c9179abb27e..5eff7d68ac1 100644
--- a/gcc/gimple-fold.cc
+++ b/gcc/gimple-fold.cc
@@ -1254,7 +1254,11 @@ gimple_fold_builtin_memory_op (gimple_stmt_iterator *gsi,
srcvar = fold_build2 (MEM_REF, desttype, src, off0);
  else
{
- if (STRICT_ALIGNMENT)
+ enum machine_mode mode = TYPE_MODE (desttype);
+ if ((mode == BLKmode && STRICT_ALIGNMENT)
+ || (targetm.slow_unaligned_access (mode, src_align)
+ && (optab_handler (movmisalign_optab, mode)
+ == CODE_FOR_nothing)))
return false;
  srctype = build_aligned_type (TYPE_MAIN_VARIANT (desttype),
src_align);
@@ -1267,7 +1271,11 @@ gimple_fold_builtin_memory_op (gimple_stmt_iterator *gsi,
destvar = fold_build2 (MEM_REF, srctype, dest, off0);
  else
{
- if (STRICT_ALIGNMENT)
+ enum machine_mode mode = TYPE_MODE (srctype);
+ if ((mode == BLKmode && STRICT_ALIGNMENT)
+ || (targetm.slow_unaligned_access (mode, dest_align)
+ && (optab_handler (movmisalign_optab, mode)
+ == CODE_FOR_nothing)))
return false;
  desttype = build_aligned_type (TYPE_MAIN_VARIANT (srctype),
 dest_align);
-- 
2.34.1


Re: [PATCH] gcov-tool: Allow merging of empty profile lists

2022-03-23 Thread Sebastian Huber

Hello Martin,

On 23/03/2022 13:19, Martin Liška wrote:

On 3/23/22 10:34, Sebastian Huber wrote:

Hello.

Thanks for the patch. Note we're in stage4, so the patch can eventually go
in in the next stage1.


ok, good.



The gcov_profile_merge() already had code to deal with profile 
information
which had no counterpart to merge with.  For profile information from 
files
with no associated counterpart, the profile information is simply used 
as is
with the weighting transformation applied.  Make sure that 
gcov_profile_merge()

works with an empty target profile list.  Return the merged profile list.


Can you please provide a simple demo with a pair of profiles
where I'll see what changes?


The background for this feature is that I would like to combine the gcov 
information obtained from several test executables. Each test executable 
will print something like this (log.txt):


*** BEGIN OF GCOV INFO ***

/home/EB/sebastian_h/src/lwip/b-xilinx_zynq_a9_qemu/init.gcda
YWRjZ1IzMEKtLjW3AQMAAADcaps855EX05p4KUUAAKEBOgEAAQAB
AAEAAQABAAEAAQEA
AQABAAEAAQABAAEAAQAA
AAABAQABAAEA
AAEAAQABAAEBAwAAACXn3k16
TDqmuIMwpAAAoQECAgABAwAAADzkvDcfSnvcuIMwpAAAoQH+AQMA
AACnWNZaIM7GWZ9hiOIAAKEBBAEBAwAAAPkGW3YHFUOO6Old
2wAAoQECAQABAwAAAIvy4CE9FxuM6Old2wAAoQECAQAB
AwAAANyvBDZiERlQ6Old2wAAoQECAQABAwAAACKQjCp2pYlIuIMwpAAAoQEC
AQABAwAAAKSSXEjQFDluuIMwpAAAoQH+AA==

...

*** END OF GCOV INFO ***

The attached script reads the log file and creates the *.gcda files 
using gcov-tool. Initially, the target files do not exist.






gcc/
* gcov-tool.cc (gcov_profile_merge): Adjust return type.
(profile_merge): Allow merging of directories which contain no 
profile

files.

libgcc/
* libgcov-util.c (gcov_profile_merge): Return the list of merged
profiles.  Accept empty target and source profile lists.
---
  gcc/gcov-tool.cc  | 27 ++-
  libgcc/libgcov-util.c | 15 +--
  2 files changed, 19 insertions(+), 23 deletions(-)

diff --git a/gcc/gcov-tool.cc b/gcc/gcov-tool.cc
index f4e42ae763c..2e4083a664d 100644
--- a/gcc/gcov-tool.cc
+++ b/gcc/gcov-tool.cc
@@ -40,7 +40,8 @@ see the files COPYING3 and COPYING.RUNTIME 
respectively.  If not, see

  #endif
  #include 
-extern int gcov_profile_merge (struct gcov_info*, struct gcov_info*, 
int, int);

+extern struct gcov_info *gcov_profile_merge (struct gcov_info*,
+ struct gcov_info*, int, int);
  extern int gcov_profile_overlap (struct gcov_info*, struct gcov_info*);
  extern int gcov_profile_normalize (struct gcov_info*, gcov_type);
  extern int gcov_profile_scale (struct gcov_info*, float, int, int);
@@ -141,26 +142,18 @@ profile_merge (const char *d1, const char *d2, 
const char *out, int w1, int w2)

  {
    struct gcov_info *d1_profile;
    struct gcov_info *d2_profile;
-  int ret;
+  struct gcov_info *merged_profile;
    d1_profile = gcov_read_profile_dir (d1, 0);
-  if (!d1_profile)
-    return 1;
-
-  if (d2)
-    {
-  d2_profile = gcov_read_profile_dir (d2, 0);
-  if (!d2_profile)
-    return 1;
+  d2_profile = gcov_read_profile_dir (d2, 0);


Is it fine calling 'gcov_read_profile_dir (d2, 0)' without 'if (d2)'?


Yes, the caller ensures that d1 and d2 are both provided:

  if (argc - optind != 2)
merge_usage ();

  return profile_merge (argv[optind], argv[optind+1], output_dir, w1, w2);

Maybe we should provide a better error message, if the user doesn't 
provide two directories instead of the general usage message.





-  /* The actual merge: we overwrite to d1_profile.  */
-  ret = gcov_profile_merge (d1_profile, d2_profile, w1, w2);
+  /* The actual merge: we overwrite to d1_profile.  */
+  merged_profile = gcov_profile_merge (d1_profile, d2_profile, w1, w2);
-  if (ret)
-    return ret;
-    }
-
-  gcov_output_files (out, d1_profile);
+  if (merged_profile)
+    gcov_output_files (out, merged_profile);
+  else if (verbose)
+    fnotice (stdout, "no profile files were merged\n");
    return 0;
  }
diff --git a/libgcc/libgcov-util.c b/libgcc/libgcov-util.c
index ba7fb924b53..e5496f4ade2 100644
--- a/libgcc/libgcov-util.c
+++ b/libgcc/libgcov-util.c
@@ -677,13 +677,13 @@ find_match_gcov_info (struct gcov_info **array, 
int size,

 Return 0 on success: without mismatch.
 Reutrn 1 on error.  */


Please adjust the function comment.


Oh, yes.

--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899

Re: [PATCH] rs6000: Skip overload instances with NULL fntype [PR104967]

2022-03-23 Thread Paul A. Clarke via Gcc-patches
On Wed, Mar 23, 2022 at 05:33:21PM +0800, Kewen.Lin via Gcc-patches wrote:
> As shown in PR104967, for some overload built-in function instance,
> if it requires a date type which isn't defined on the target, its

nit: s/date/data/

> fntype would be initialized as NULL.  This patch is to consider
> this possibility in function find_instance to avoid ICE.

PC


Re: [pushed] c++: using from enclosing class template [PR105006]

2022-03-23 Thread Patrick Palka via Gcc-patches
On Wed, 23 Mar 2022, Jason Merrill via Gcc-patches wrote:

> Here, DECL_DEPENDENT_P was false for the second using because Row is
> "the current instantiation", so lookup succeeds.  But since Row itself has a
> dependent using-decl for operator(), the set of functions imported by the
> second using is dependent, so we should set the flag.
> 
> Tested x86_64-pc-linux-gnu, applying to trunk.
> 
>   PR c++/105006
> 
> gcc/cp/ChangeLog:
> 
>   * name-lookup.cc (lookup_using_decl): Set DECL_DEPENDENT_P if lookup
>   finds a dependent using.
> 
> gcc/testsuite/ChangeLog:
> 
>   * g++.dg/template/using30.C: New test.
> ---
>  gcc/cp/name-lookup.cc   | 15 +++
>  gcc/testsuite/g++.dg/template/using30.C | 13 +
>  2 files changed, 28 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/template/using30.C
> 
> diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
> index 323f96bcd24..ea947fabb7e 100644
> --- a/gcc/cp/name-lookup.cc
> +++ b/gcc/cp/name-lookup.cc
> @@ -5665,6 +5665,21 @@ lookup_using_decl (tree scope, name_lookup )
>   lookup.value = lookup_member (binfo, lookup.name, /*protect=*/2,
> /*want_type=*/false, tf_none);
>  
> +  /* If the lookup in the base contains a dependent using, this
> +  using is also dependent.  */
> +  if (!dependent_p && lookup.value)

I wonder if it'd be worthwhile to also test dependent_type_p (scope) here
here to avoid iterating over the lookup set when it can't possibly contain
a dependent using-decl.

> + {
> +   tree val = lookup.value;
> +   if (tree fns = maybe_get_fns (val))
> + val = fns;
> +   for (tree f: lkp_range (val))
> + if (TREE_CODE (f) == USING_DECL && DECL_DEPENDENT_P (f))
> +   {
> + dependent_p = true;
> + break;
> +   }
> + }
> +
>if (!depscope && b_kind < bk_proper_base)
>   {
> if (cxx_dialect >= cxx20 && lookup.value
> diff --git a/gcc/testsuite/g++.dg/template/using30.C 
> b/gcc/testsuite/g++.dg/template/using30.C
> new file mode 100644
> index 000..914252dd14c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/using30.C
> @@ -0,0 +1,13 @@
> +// PR c++/105006
> +
> +template
> +class Row {
> +  using eT::operator();
> +  void operator()();
> +  class fixed;
> +};
> +
> +template
> +class Row::fixed : Row {
> +  using Row::operator();
> +};
> 
> base-commit: 4a9e92164a547afcf8cd3fc593c7660238ad2d59
> -- 
> 2.27.0
> 
> 



Re: [PATCH v2] Add TARGET_MOVE_WITH_MODE_P

2022-03-23 Thread Richard Biener via Gcc-patches
On Tue, Mar 22, 2022 at 9:20 AM Richard Biener
 wrote:
>
> On Tue, Mar 22, 2022 at 3:51 AM H.J. Lu  wrote:
> >
> > On Mon, Mar 14, 2022 at 8:44 AM Richard Sandiford
> >  wrote:
> > >
> > > Richard Biener  writes:
> > > > On Wed, Mar 9, 2022 at 7:04 PM Richard Sandiford
> > > >  wrote:
> > > >>
> > > >> Richard Biener via Gcc-patches  writes:
> > > >> > On Wed, Mar 2, 2022 at 10:18 PM H.J. Lu  wrote:
> > > >> >>
> > > >> >> On Wed, Mar 02, 2022 at 09:51:26AM +0100, Richard Biener wrote:
> > > >> >> > On Tue, Mar 1, 2022 at 11:41 PM H.J. Lu via Gcc-patches
> > > >> >> >  wrote:
> > > >> >> > >
> > > >> >> > > Add TARGET_FOLD_MEMCPY_MAX for the maximum number of bytes to 
> > > >> >> > > fold memcpy.
> > > >> >> > > The default is
> > > >> >> > >
> > > >> >> > > MOVE_MAX * MOVE_RATIO (optimize_function_for_size_p (cfun))
> > > >> >> > >
> > > >> >> > > For x86, it is MOVE_MAX to restore the old behavior before
> > > >> >> >
> > > >> >> > I know we've discussed this to death in the PR, I just want to 
> > > >> >> > repeat here
> > > >> >> > that the GIMPLE folding expects to generate a single load and a 
> > > >> >> > single
> > > >> >> > store (that is what it does on the GIMPLE level) which is why 
> > > >> >> > MOVE_MAX
> > > >> >> > was chosen originally (it's documented to what a "single 
> > > >> >> > instruction" does).
> > > >> >> > In practice MOVE_MAX does not seem to cover vector register sizes
> > > >> >> > so Richard pulled MOVE_RATIO which is really intended to cover
> > > >> >> > the case of using multiple instructions for moving memory (but 
> > > >> >> > then I
> > > >> >> > don't remember whether for the ARM case the single load/store 
> > > >> >> > GIMPLE
> > > >> >> > will be expanded to multiple load/store instructions).
> > > >> >> >
> > > >> >> > TARGET_FOLD_MEMCPY_MAX sounds like a stop-gap solution,
> > > >> >> > being very specific for memcpy folding (we also fold memmove btw).
> > > >> >> >
> > > >> >> > There is also MOVE_MAX_PIECES which _might_ be more appropriate
> > > >> >> > than MOVE_MAX here and still honor the idea of single 
> > > >> >> > instructions.
> > > >> >> > Now neither arm nor aarch64 define this and it defaults to 
> > > >> >> > MOVE_MAX,
> > > >> >> > not MOVE_MAX * MOVE_RATIO.
> > > >> >> >
> > > >> >> > So if we need a new hook then that hook should at least get the
> > > >> >> > 'speed' argument of MOVE_RATIO and it should get a better name.
> > > >> >> >
> > > >> >> > I still think that it should be possible to improve the insn 
> > > >> >> > check to
> > > >> >> > avoid use of "disabled" modes, maybe that's also a point to add
> > > >> >> > a new hook like .move_with_mode_p or so?  To quote, we do
> > > >> >>
> > > >> >> Here is the v2 patch to add TARGET_MOVE_WITH_MODE_P.
> > > >> >
> > > >> > Again I'd like to shine light on MOVE_MAX_PIECES which explicitely
> > > >> > mentions "a load or store used TO COPY MEMORY" (emphasis mine)
> > > >> > and whose x86 implementation would already be fine (doing larger 
> > > >> > moves
> > > >> > and also not doing too large moves).  But appearantly the arm folks
> > > >> > decided that that's not fit and instead (mis-?)used MOVE_MAX * 
> > > >> > MOVE_RATIO.
> > > >>
> > > >> It seems like there are old comments and old documentation that justify
> > > >> both interpretations, so there are good arguments on both sides.  But
> > > >> with this kind of thing I think we have to infer the meaning of the
> > > >> macro from the way it's currently used, rather than trusting such old
> > > >> and possibly out-of-date and contradictory information.
> > > >>
> > > >> FWIW, I agree that (if we exclude old reload, which we should!) the
> > > >> only direct uses of MOVE_MAX before the patch were not specific to
> > > >> integer registers and so MOVE_MAX should include vectors if the
> > > >> target wants vector modes to be used for general movement.
> > > >>
> > > >> Even if people disagree that that's the current meaning, I think it's
> > > >> at least a sensible meaning.  It provides information that AFAIK isn't
> > > >> available otherwise, and it avoids overlap with MAX_FIXED_MODE_SIZE.
> > > >>
> > > >> So FWIW, I think it'd be reasonable to change non-x86 targets if they
> > > >> want vector modes to be used for single-insn copies.
> > > >
> > > > Note a slight complication in the GIMPLE folding case is that we
> > > > do not end up using vector modes but we're using "fake"
> > > > integer modes like OImode which x86 has move patterns for.
> > > > If we'd use vector modes we could use existing target hooks to
> > > > eventually decide whether auto-using those is desired or not.
> > >
> > > Hmm, yeah.  Certainly we shouldn't require the target to support
> > > a scalar integer equivalent of every vector mode.
> > >
> >
> > I'd like to resolve this before GCC 12 is released.
>
> I've come to the conclusion that we should revert r12-3482-g5f6a6c91d7c592,
> it abuses MOVE_MAX * MOVE_RATIO to trick GIMPLE into thinking we can
> 

[PATCH] Fortran: Fix directory stat check for '.' [PR103560]

2022-03-23 Thread Tobias Burnus

As reported in the PR103560. The patch is a slightly modified version
of the patch proposed by the bug reporter (with pseudonym?) Carlos Une.

The problem seems to be that 'stat ("./", ...)' fails with MinGW while
'stat (".", ...)' works. Besides, I think it makes sense to defer the
usage of '/' until needed - and I believe the error messages also make
more sense now.

Due to diagnostic changes in GCC 12, this pre-exising old issue was
exposed and shows up as bootstrap fail in GCC 12.

I intent to commit it later today (to GCC 12, only),
unless someone suggests a change.

Tobias

PS: I was briefly considering to use DIR_SEPARATOR (as defined in system.h),
but that one is '/' (while DIR_SEPARATOR_2 is '\\', if defined). Thus, it
does not seem to be worthwhile - especially as '/' needs to be converted to
"/" for strcat - or
  fullname[strlen(path)] = DIR_SEPARATOR; fullname[strlen(path)+1]='\0';
has to be used.
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955
Fortran: Fix directory stat check for '.' [PR103560]

MinGW does not like a call to 'stat' for './' via gfc_do_check_include_dir.
Solution: Only append '/' when concatenating the path with the filename.

gcc/fortran/ChangeLog:

	PR fortran/103560
	* scanner.cc (add_path_to_list): Don't append '/' to the
	save include path.
	(open_included_file): Use '/' in concatenating path + file name.
	* module.cc (gzopen_included_file_1): Likewise.

gcc/testsuite/ChangeLog:

	PR fortran/103560
	* gfortran.dg/include_14.f90: Update dg-warning.
	* gfortran.dg/include_17.f90: Likewise.
	* gfortran.dg/include_18.f90: Likewise.
	* gfortran.dg/include_6.f90: Update dg-*.
---
 gcc/fortran/module.cc| 3 ++-
 gcc/fortran/scanner.cc   | 7 +++
 gcc/testsuite/gfortran.dg/include_14.f90 | 4 ++--
 gcc/testsuite/gfortran.dg/include_17.f90 | 4 ++--
 gcc/testsuite/gfortran.dg/include_18.f90 | 4 ++--
 gcc/testsuite/gfortran.dg/include_6.f90  | 2 +-
 6 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/gcc/fortran/module.cc b/gcc/fortran/module.cc
index 281b1b17fbf..85aa153bd77 100644
--- a/gcc/fortran/module.cc
+++ b/gcc/fortran/module.cc
@@ -1095,8 +1095,9 @@ gzopen_included_file_1 (const char *name, gfc_directorylist *list,
   if (module && !p->use_for_modules)
continue;
 
-  fullname = (char *) alloca(strlen (p->path) + strlen (name) + 1);
+  fullname = (char *) alloca(strlen (p->path) + strlen (name) + 2);
   strcpy (fullname, p->path);
+  strcat (fullname, "/");
   strcat (fullname, name);
 
   f = gzopen (fullname, "r");
diff --git a/gcc/fortran/scanner.cc b/gcc/fortran/scanner.cc
index b52282b687b..2dff2514700 100644
--- a/gcc/fortran/scanner.cc
+++ b/gcc/fortran/scanner.cc
@@ -409,9 +409,7 @@ add_path_to_list (gfc_directorylist **list, const char *path,
 *list = dir;
   dir->use_for_modules = use_for_modules;
   dir->warn = warn;
-  dir->path = XCNEWVEC (char, strlen (p) + 2);
-  strcpy (dir->path, p);
-  strcat (dir->path, "/");	/* make '/' last character */
+  dir->path = xstrdup (p);
 }
 
 /* defer_warn is set to true while parsing the commandline.  */
@@ -476,8 +474,9 @@ open_included_file (const char *name, gfc_directorylist *list,
   if (module && !p->use_for_modules)
 	continue;
 
-  fullname = (char *) alloca(strlen (p->path) + strlen (name) + 1);
+  fullname = (char *) alloca(strlen (p->path) + strlen (name) + 2);
   strcpy (fullname, p->path);
+  strcat (fullname, "/");
   strcat (fullname, name);
 
   f = gfc_open_file (fullname);
diff --git a/gcc/testsuite/gfortran.dg/include_14.f90 b/gcc/testsuite/gfortran.dg/include_14.f90
index 8110e49bf43..39acf69b1b4 100644
--- a/gcc/testsuite/gfortran.dg/include_14.f90
+++ b/gcc/testsuite/gfortran.dg/include_14.f90
@@ -1,6 +1,6 @@
 ! { dg-additional-options "-cpp -idirafter /fdaf/ -I bar -J foo/bar" }
 end
 ! default: warn for -I and -J but ignore other options.
-! { dg-warning "Nonexistent include directory 'bar/'" "" { target *-*-* } 0 }
-! { dg-warning "Nonexistent include directory 'foo/bar/'" "" { target *-*-* } 0 }
+! { dg-warning "Nonexistent include directory 'bar'" "" { target *-*-* } 0 }
+! { dg-warning "Nonexistent include directory 'foo/bar'" "" { target *-*-* } 0 }
 
diff --git a/gcc/testsuite/gfortran.dg/include_17.f90 b/gcc/testsuite/gfortran.dg/include_17.f90
index 06677590be3..f09b22f079a 100644
--- a/gcc/testsuite/gfortran.dg/include_17.f90
+++ b/gcc/testsuite/gfortran.dg/include_17.f90
@@ -1,6 +1,6 @@
 ! { dg-do compile }
 ! { dg-options "-I foo-bar -J foo/bar" }
 end 
-! { dg-warning "Nonexistent include directory 'foo-bar/'" "" { target *-*-* } 0 }
-! { dg-warning "Nonexistent include directory 'foo/bar/'" "" { target *-*-* } 0 }
+! { dg-warning "Nonexistent include directory 

[PATCH v2] Document that the 'access' and 'nonnull' attributes are independent

2022-03-23 Thread David Malcolm via Gcc-patches
On Mon, 2022-03-14 at 16:18 -0600, Martin Sebor wrote:
> On 3/9/22 14:57, David Malcolm via Gcc wrote:
> > On Wed, 2022-03-09 at 13:30 -0800, Andrew Pinski wrote:
> > > On Wed, Mar 9, 2022 at 1:25 PM David Malcolm via Gcc
> > >  wrote:
> > > > 
> > > > We gained __attribute__ ((access, ...)) in GCC 10:
> > > >
> > > > https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html
> > > > which identifies one of the pointer/reference arguments of a
> > > > function
> > > > as being accessed according to an access-mode: read_only,
> > > > read_write,
> > > > write_only, or none.
> > > > 
> > > > We also have __attribute__ ((nonnull)) to indicate that a
> > > > function
> > > > argument (or all of them) must be non-NULL.
> > > > 
> > > > There doesn't seem to be a relationship between these in the
> > > > implementation, but it strikes me that almost anywhere that a
> > > > user
> > > > might use the "access" attribute, that parameter is probably
> > > > going
> > > > to
> > > > be required to be nonnull - though perhaps there are cases
> > > > where
> > > > APIs
> > > > check for NULL and reject them gracefully?
> > > 
> > > No, I think they are separate. The access just says these access
> > > attributes are read only, write only, read-write or don't access
> > > what
> > > the pointer points to; it does not say they have to be read or
> > > written
> > > to.
> > > I think it is a bad idea to connect the two ideas because you
> > > could
> > > have some cases where an argument is optional but is only read
> > > from;
> > > or is only written to (there are many in GCC sources even).
> > 
> > Thanks for the clarification...
> > 
> > > 
> > > Thanks,
> > > Andrew Pinski
> > > 
> > > > 
> > > > Might we want to somehow make __attribute__ ((access, ...))
> > > > imply
> > > > __attribute__ ((nonnull))?  (for non "none" access modes,
> > > > perhaps?)
> > > > 
> > > > If so, one place to implement this might be in tree.cc's
> > > > get_nonnull_args, and have it add to the bitmap any arguments
> > > > that
> > > > have an appropriate access attribute.
> > > > 
> > > > get_nonnull_args is used in various places:
> > > > - validating builtins
> > > > - in ranger_cache::block_apply_nonnull
> > > > - by -Wnonnull (in pass_post_ipa_warn::execute)
> > > > - by -Wanalyzer-possible-null-argument and -Wanalyzer-null-
> > > > argument;
> > > > I'm tracking the failure of these last two to make use of
> > > > __attribute__
> > > > ((access)) in PR analyzer/104860.
> > > > 
> > > > So do we:
> > > > 
> > > > (a) leave it up to the user, requiring them to specify
> > > > __attribute__
> > > > ((nonnull)) in addition to  __attribute__ ((access, ...))
> > 
> > ...so that's (a) then.
> > 
> > I think it might be more user-friendly to be explicit about this in
> > the
> > documentation, maybe something like the attached?
> 
> I agree it's worth clarifying the manual.
> 
> But I don't think there's a way to annotate a function to indicate
> that it will definitely access an object (or dereference a pointer).
> Attribute access just implies that it might dereference it (unless
> the size is zero), and attribute nonnull that the pointer must not
> be null, not that it will be dereferenced (or even that it must be
> valid, although that's implied by the language and should probably
> be enforced in all contexts by some other warning).
> 
> The combination of access with nonzero size and nonnull only means
> that the pointer must be nonnull and point to an object with at least
> size elements.

Here's an updated version of the patch which expresses that also.

OK for trunk?

Dave

> 
> Martin
> 
> > 
> > (not yet fully tested, but seems to build)
> > 
> > Dave
> > 
> > 
> > 
> > 
> > > > 
> > > > (b) leave it up to the individual sites in GCC that currently
> > > > make
> > > > use
> > > > of get_nonnull_args to add logic for handling   __attribute__
> > > > ((access,
> > > > ...))
> > > > 
> > > > (c) extend get_nonnull_args
> > > > 
> > > > ?
> > > > 
> > > > Thoughts?
> > > > Dave
> > > > 
> > > 
> > 
> 

gcc/ChangeLog:
* doc/extend.texi (Common Function Attributes): Document that
'access' does not imply 'nonnull'.

Signed-off-by: David Malcolm 
---
 gcc/doc/extend.texi | 9 +
 1 file changed, 9 insertions(+)

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index a4a25e86928..790014f3da5 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -2652,6 +2652,15 @@ The mode is intended to be used as a means to help 
validate the expected
 object size, for example in functions that call @code{__builtin_object_size}.
 @xref{Object Size Checking}.
 
+Note that the @code{access} attribute merely specifies how an object
+referenced by the pointer argument can be accessed; it does not imply that
+an access @strong{will} happen.  If the pointer will definitely be
+dereferenced, you may want to also add
+@code{__attribute__((nonnull (@var{arg-index})))} to the function for the
+pointer parameter in 

[pushed] c++: using from enclosing class template [PR105006]

2022-03-23 Thread Jason Merrill via Gcc-patches
Here, DECL_DEPENDENT_P was false for the second using because Row is
"the current instantiation", so lookup succeeds.  But since Row itself has a
dependent using-decl for operator(), the set of functions imported by the
second using is dependent, so we should set the flag.

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

PR c++/105006

gcc/cp/ChangeLog:

* name-lookup.cc (lookup_using_decl): Set DECL_DEPENDENT_P if lookup
finds a dependent using.

gcc/testsuite/ChangeLog:

* g++.dg/template/using30.C: New test.
---
 gcc/cp/name-lookup.cc   | 15 +++
 gcc/testsuite/g++.dg/template/using30.C | 13 +
 2 files changed, 28 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/template/using30.C

diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index 323f96bcd24..ea947fabb7e 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -5665,6 +5665,21 @@ lookup_using_decl (tree scope, name_lookup )
lookup.value = lookup_member (binfo, lookup.name, /*protect=*/2,
  /*want_type=*/false, tf_none);
 
+  /* If the lookup in the base contains a dependent using, this
+using is also dependent.  */
+  if (!dependent_p && lookup.value)
+   {
+ tree val = lookup.value;
+ if (tree fns = maybe_get_fns (val))
+   val = fns;
+ for (tree f: lkp_range (val))
+   if (TREE_CODE (f) == USING_DECL && DECL_DEPENDENT_P (f))
+ {
+   dependent_p = true;
+   break;
+ }
+   }
+
   if (!depscope && b_kind < bk_proper_base)
{
  if (cxx_dialect >= cxx20 && lookup.value
diff --git a/gcc/testsuite/g++.dg/template/using30.C 
b/gcc/testsuite/g++.dg/template/using30.C
new file mode 100644
index 000..914252dd14c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/using30.C
@@ -0,0 +1,13 @@
+// PR c++/105006
+
+template
+class Row {
+  using eT::operator();
+  void operator()();
+  class fixed;
+};
+
+template
+class Row::fixed : Row {
+  using Row::operator();
+};

base-commit: 4a9e92164a547afcf8cd3fc593c7660238ad2d59
-- 
2.27.0



[committed] analyzer: use tainted_allocation_size::m_mem_space [PR105017]

2022-03-23 Thread David Malcolm via Gcc-patches
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r12-7783-ge6a3991ea15c0b.

gcc/analyzer/ChangeLog:
PR analyzer/105017
* sm-taint.cc (taint_diagnostic::subclass_equal_p): Check
m_has_bounds as well as m_arg.
(tainted_allocation_size::subclass_equal_p): Chain up to base
class implementation.  Also check m_mem_space.
(tainted_allocation_size::emit): Add note showing stack-based vs
heap-based allocations.

gcc/testsuite/ChangeLog:
PR analyzer/105017
* gcc.dg/analyzer/taint-alloc-1.c: Add expected messages relating
to heap vs stack.

Signed-off-by: David Malcolm 
---
 gcc/analyzer/sm-taint.cc  | 82 +--
 gcc/testsuite/gcc.dg/analyzer/taint-alloc-1.c |  2 +
 2 files changed, 58 insertions(+), 26 deletions(-)

diff --git a/gcc/analyzer/sm-taint.cc b/gcc/analyzer/sm-taint.cc
index e2c78cdd42b..17669ae7685 100644
--- a/gcc/analyzer/sm-taint.cc
+++ b/gcc/analyzer/sm-taint.cc
@@ -137,7 +137,9 @@ public:
 
   bool subclass_equal_p (const pending_diagnostic _other) const OVERRIDE
   {
-return same_tree_p (m_arg, ((const taint_diagnostic &)base_other).m_arg);
+const taint_diagnostic  = (const taint_diagnostic &)base_other;
+return (same_tree_p (m_arg, other.m_arg)
+   && m_has_bounds == other.m_has_bounds);
   }
 
   label_text describe_state_change (const evdesc::state_change )
@@ -523,6 +525,15 @@ public:
 return "tainted_allocation_size";
   }
 
+  bool subclass_equal_p (const pending_diagnostic _other) const OVERRIDE
+  {
+if (!taint_diagnostic::subclass_equal_p (base_other))
+  return false;
+const tainted_allocation_size 
+  = (const tainted_allocation_size &)base_other;
+return m_mem_space == other.m_mem_space;
+  }
+
   int get_controlling_option () const FINAL OVERRIDE
   {
 return OPT_Wanalyzer_tainted_allocation_size;
@@ -533,29 +544,32 @@ public:
 diagnostic_metadata m;
 /* "CWE-789: Memory Allocation with Excessive Size Value".  */
 m.add_cwe (789);
-// TODO: make use of m_mem_space
+
+bool warned;
 if (m_arg)
   switch (m_has_bounds)
{
default:
  gcc_unreachable ();
case BOUNDS_NONE:
- return warning_meta (rich_loc, m, get_controlling_option (),
-  "use of attacker-controlled value %qE as"
-  " allocation size without bounds checking",
-  m_arg);
+ warned = warning_meta (rich_loc, m, get_controlling_option (),
+"use of attacker-controlled value %qE as"
+" allocation size without bounds checking",
+m_arg);
  break;
case BOUNDS_UPPER:
- return warning_meta (rich_loc, m, get_controlling_option (),
-  "use of attacker-controlled value %qE as"
-  " allocation size without lower-bounds checking",
-  m_arg);
+ warned = warning_meta (rich_loc, m, get_controlling_option (),
+"use of attacker-controlled value %qE as"
+" allocation size without"
+" lower-bounds checking",
+m_arg);
  break;
case BOUNDS_LOWER:
- return warning_meta (rich_loc, m, get_controlling_option (),
-  "use of attacker-controlled value %qE as"
-  " allocation size without upper-bounds checking",
-m_arg);
+ warned = warning_meta (rich_loc, m, get_controlling_option (),
+"use of attacker-controlled value %qE as"
+" allocation size without"
+" upper-bounds checking",
+m_arg);
  break;
}
 else
@@ -564,24 +578,40 @@ public:
default:
  gcc_unreachable ();
case BOUNDS_NONE:
- return warning_meta (rich_loc, m, get_controlling_option (),
-  "use of attacker-controlled value as"
-  " allocation size without bounds"
-  " checking");
+ warned = warning_meta (rich_loc, m, get_controlling_option (),
+"use of attacker-controlled value as"
+" allocation size without bounds"
+" checking");
  break;
case BOUNDS_UPPER:
- return warning_meta (rich_loc, m, get_controlling_option (),
-  "use of attacker-controlled value as"
-  " allocation size without lower-bounds"
-  " checking");
+ warned = 

[committed] analyzer: fix ICE adding note to disabled diagnostic [PR104997]

2022-03-23 Thread David Malcolm via Gcc-patches
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r12-7782-g160b095fc9ded4.

gcc/analyzer/ChangeLog:
PR analyzer/104997
* diagnostic-manager.cc (diagnostic_manager::add_diagnostic):
Convert return type from "void" to "bool", reporting success vs
failure to caller, for both overloads.
* diagnostic-manager.h (diagnostic_manager::add_diagnostic):
Likewise.
* engine.cc (impl_region_model_context::warn): Propagate return
value from diagnostic_manager::add_diagnostic.

gcc/testsuite/ChangeLog:
PR analyzer/104997
* gcc.dg/analyzer/write-to-string-literal-4-disabled.c: New test,
adapted from write-to-string-literal-4.c.

Signed-off-by: David Malcolm 
---
 gcc/analyzer/diagnostic-manager.cc| 18 
 gcc/analyzer/diagnostic-manager.h |  4 +--
 gcc/analyzer/engine.cc|  9 ++
 .../write-to-string-literal-4-disabled.c  | 28 +++
 4 files changed, 45 insertions(+), 14 deletions(-)
 create mode 100644 
gcc/testsuite/gcc.dg/analyzer/write-to-string-literal-4-disabled.c

diff --git a/gcc/analyzer/diagnostic-manager.cc 
b/gcc/analyzer/diagnostic-manager.cc
index d5e5b6926cc..bf7c8fc5147 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -874,9 +874,11 @@ diagnostic_manager::diagnostic_manager (logger *logger, 
engine *eng,
 {
 }
 
-/* Queue pending_diagnostic D at ENODE for later emission.  */
+/* Queue pending_diagnostic D at ENODE for later emission.
+   Return true/false signifying if the diagnostic was actually added.
+   Take ownership of D (or delete it).  */
 
-void
+bool
 diagnostic_manager::add_diagnostic (const state_machine *sm,
exploded_node *enode,
const supernode *snode, const gimple *stmt,
@@ -907,7 +909,7 @@ diagnostic_manager::add_diagnostic (const state_machine *sm,
d->get_kind ());
  delete d;
  m_num_disabled_diagnostics++;
- return;
+ return false;
}
 }
 
@@ -920,18 +922,22 @@ diagnostic_manager::add_diagnostic (const state_machine 
*sm,
 log ("adding saved diagnostic %i at SN %i to EN %i: %qs",
 sd->get_index (),
 snode->m_index, enode->m_index, d->get_kind ());
+  return true;
 }
 
-/* Queue pending_diagnostic D at ENODE for later emission.  */
+/* Queue pending_diagnostic D at ENODE for later emission.
+   Return true/false signifying if the diagnostic was actually added.
+   Take ownership of D (or delete it).  */
 
-void
+bool
 diagnostic_manager::add_diagnostic (exploded_node *enode,
const supernode *snode, const gimple *stmt,
stmt_finder *finder,
pending_diagnostic *d)
 {
   gcc_assert (enode);
-  add_diagnostic (NULL, enode, snode, stmt, finder, NULL_TREE, NULL, 0, d);
+  return add_diagnostic (NULL, enode, snode, stmt, finder, NULL_TREE,
+NULL, 0, d);
 }
 
 /* Add PN to the most recent saved_diagnostic.  */
diff --git a/gcc/analyzer/diagnostic-manager.h 
b/gcc/analyzer/diagnostic-manager.h
index 34abf56d11f..fc5dc043c78 100644
--- a/gcc/analyzer/diagnostic-manager.h
+++ b/gcc/analyzer/diagnostic-manager.h
@@ -107,7 +107,7 @@ public:
 
   json::object *to_json () const;
 
-  void add_diagnostic (const state_machine *sm,
+  bool add_diagnostic (const state_machine *sm,
   exploded_node *enode,
   const supernode *snode, const gimple *stmt,
   stmt_finder *finder,
@@ -116,7 +116,7 @@ public:
   state_machine::state_t state,
   pending_diagnostic *d);
 
-  void add_diagnostic (exploded_node *enode,
+  bool add_diagnostic (exploded_node *enode,
   const supernode *snode, const gimple *stmt,
   stmt_finder *finder,
   pending_diagnostic *d);
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index f911ed4ac39..caa8796b494 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -129,12 +129,9 @@ impl_region_model_context::warn (pending_diagnostic *d)
   return false;
 }
   if (m_eg)
-{
-  m_eg->get_diagnostic_manager ().add_diagnostic
-   (m_enode_for_diag, m_enode_for_diag->get_supernode (),
-m_stmt, m_stmt_finder, d);
-  return true;
-}
+return m_eg->get_diagnostic_manager ().add_diagnostic
+  (m_enode_for_diag, m_enode_for_diag->get_supernode (),
+   m_stmt, m_stmt_finder, d);
   else
 {
   delete d;
diff --git a/gcc/testsuite/gcc.dg/analyzer/write-to-string-literal-4-disabled.c 
b/gcc/testsuite/gcc.dg/analyzer/write-to-string-literal-4-disabled.c
new file mode 100644
index 000..fa21af13341
--- /dev/null
+++ 

Re: [PATCH] rs6000: Skip overload instances with NULL fntype [PR104967]

2022-03-23 Thread David Edelsohn via Gcc-patches
On Wed, Mar 23, 2022 at 5:33 AM Kewen.Lin  wrote:
>
> Hi,
>
> As shown in PR104967, for some overload built-in function instance,
> if it requires a date type which isn't defined on the target, its
> fntype would be initialized as NULL.  This patch is to consider
> this possibility in function find_instance to avoid ICE.
>
> Bootstrapped and regtested on powerpc64-linux-gnu P8 and
> powerpc64le-linux-gnu P9 and P10.
>
> Is it ok for trunk?

Okay.

Thanks, David

>
> BR,
> Kewen
>
> PR target/104967
>
> gcc/ChangeLog:
>
> * config/rs6000/rs6000-c.cc (find_instance): Skip instances with null
> function types.
>
> ---
>  gcc/config/rs6000/rs6000-c.cc | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/gcc/config/rs6000/rs6000-c.cc b/gcc/config/rs6000/rs6000-c.cc
> index 15251efc209..b0f9fce4b54 100644
> --- a/gcc/config/rs6000/rs6000-c.cc
> +++ b/gcc/config/rs6000/rs6000-c.cc
> @@ -1678,6 +1678,10 @@ find_instance (bool *unsupported_builtin, ovlddata 
> **instance,
>
>ovlddata *inst = *instance;
>gcc_assert (inst != NULL);
> +  /* It is possible for an instance to require a data type that isn't
> + defined on this target, in which case inst->fntype will be NULL.  */
> +  if (!inst->fntype)
> +return error_mark_node;
>tree fntype = rs6000_builtin_info[inst->bifid].fntype;
>tree parmtype0 = TREE_VALUE (TYPE_ARG_TYPES (fntype));
>tree parmtype1 = TREE_VALUE (TREE_CHAIN (TYPE_ARG_TYPES (fntype)));
> --
> 2.25.1


[committed] libstdc++: Add missing constraints to std::bit_cast [PR105027]

2022-03-23 Thread Jonathan Wakely via Gcc-patches
Tested powerpc64le-linux, pushed to trunk.

Backport to gcc-11 to follow.

-- >8 --

Our std::bit_cast was relying on the compiler to check for errors inside
__builtin_bit_cast, instead of checking them as constraints. That means
std::bit_cast was not SFINAE-friendly.

This fix uses a requires-clause, so for old versions of Clang without
concepts support the function will still be unconstrained. At some point
in future we can remove the #ifdef __cpp_concepts check and rely on all
compilers having full concepts support in C++20 mode.

libstdc++-v3/ChangeLog:

PR libstdc++/105027
* include/std/bit (bit_cast): Add constraints.
* testsuite/26_numerics/bit/bit.cast/105027.cc: New test.
---
 libstdc++-v3/include/std/bit   |  4 
 .../26_numerics/bit/bit.cast/105027.cc | 18 ++
 2 files changed, 22 insertions(+)
 create mode 100644 libstdc++-v3/testsuite/26_numerics/bit/bit.cast/105027.cc

diff --git a/libstdc++-v3/include/std/bit b/libstdc++-v3/include/std/bit
index cfd98e24eb5..a40f1ce99df 100644
--- a/libstdc++-v3/include/std/bit
+++ b/libstdc++-v3/include/std/bit
@@ -73,6 +73,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 [[nodiscard]]
 constexpr _To
 bit_cast(const _From& __from) noexcept
+#ifdef __cpp_concepts
+requires (sizeof(_To) == sizeof(_From))
+  && __is_trivially_copyable(_To) && __is_trivially_copyable(_From)
+#endif
 {
   return __builtin_bit_cast(_To, __from);
 }
diff --git a/libstdc++-v3/testsuite/26_numerics/bit/bit.cast/105027.cc 
b/libstdc++-v3/testsuite/26_numerics/bit/bit.cast/105027.cc
new file mode 100644
index 000..301d94ec575
--- /dev/null
+++ b/libstdc++-v3/testsuite/26_numerics/bit/bit.cast/105027.cc
@@ -0,0 +1,18 @@
+// { dg-options "-std=gnu++20" }
+// { dg-do compile { target c++20 } }
+
+// PR libstdc++/105027 - Missing constraints on std::bit_cast
+
+#include 
+
+template
+concept BitCastable = requires(const U& u) { std::bit_cast(u); };
+
+static_assert(BitCastable); // OK
+
+static_assert(!BitCastable); // #1: different size
+
+struct A { A(A const&); int i; };
+static_assert(!BitCastable); // #2: not trivially copyable
+
+static_assert(!BitCastable); // #3: sizeof(int()) is ill-formed
-- 
2.34.1



Re: [PATCH] gcov-tool: Allow merging of empty profile lists

2022-03-23 Thread Martin Liška

On 3/23/22 10:34, Sebastian Huber wrote:

Hello.

Thanks for the patch. Note we're in stage4, so the patch can eventually go
in in the next stage1.


The gcov_profile_merge() already had code to deal with profile information
which had no counterpart to merge with.  For profile information from files
with no associated counterpart, the profile information is simply used as is
with the weighting transformation applied.  Make sure that gcov_profile_merge()
works with an empty target profile list.  Return the merged profile list.


Can you please provide a simple demo with a pair of profiles
where I'll see what changes?



gcc/
* gcov-tool.cc (gcov_profile_merge): Adjust return type.
(profile_merge): Allow merging of directories which contain no profile
files.

libgcc/
* libgcov-util.c (gcov_profile_merge): Return the list of merged
profiles.  Accept empty target and source profile lists.
---
  gcc/gcov-tool.cc  | 27 ++-
  libgcc/libgcov-util.c | 15 +--
  2 files changed, 19 insertions(+), 23 deletions(-)

diff --git a/gcc/gcov-tool.cc b/gcc/gcov-tool.cc
index f4e42ae763c..2e4083a664d 100644
--- a/gcc/gcov-tool.cc
+++ b/gcc/gcov-tool.cc
@@ -40,7 +40,8 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If 
not, see
  #endif
  #include 
  
-extern int gcov_profile_merge (struct gcov_info*, struct gcov_info*, int, int);

+extern struct gcov_info *gcov_profile_merge (struct gcov_info*,
+struct gcov_info*, int, int);
  extern int gcov_profile_overlap (struct gcov_info*, struct gcov_info*);
  extern int gcov_profile_normalize (struct gcov_info*, gcov_type);
  extern int gcov_profile_scale (struct gcov_info*, float, int, int);
@@ -141,26 +142,18 @@ profile_merge (const char *d1, const char *d2, const char 
*out, int w1, int w2)
  {
struct gcov_info *d1_profile;
struct gcov_info *d2_profile;
-  int ret;
+  struct gcov_info *merged_profile;
  
d1_profile = gcov_read_profile_dir (d1, 0);

-  if (!d1_profile)
-return 1;
-
-  if (d2)
-{
-  d2_profile = gcov_read_profile_dir (d2, 0);
-  if (!d2_profile)
-return 1;
+  d2_profile = gcov_read_profile_dir (d2, 0);


Is it fine calling 'gcov_read_profile_dir (d2, 0)' without 'if (d2)'?

  
-  /* The actual merge: we overwrite to d1_profile.  */

-  ret = gcov_profile_merge (d1_profile, d2_profile, w1, w2);
+  /* The actual merge: we overwrite to d1_profile.  */
+  merged_profile = gcov_profile_merge (d1_profile, d2_profile, w1, w2);
  
-  if (ret)

-return ret;
-}
-
-  gcov_output_files (out, d1_profile);
+  if (merged_profile)
+gcov_output_files (out, merged_profile);
+  else if (verbose)
+fnotice (stdout, "no profile files were merged\n");
  
return 0;

  }
diff --git a/libgcc/libgcov-util.c b/libgcc/libgcov-util.c
index ba7fb924b53..e5496f4ade2 100644
--- a/libgcc/libgcov-util.c
+++ b/libgcc/libgcov-util.c
@@ -677,13 +677,13 @@ find_match_gcov_info (struct gcov_info **array, int size,
 Return 0 on success: without mismatch.
 Reutrn 1 on error.  */


Please adjust the function comment.

Cheers,
Martin

  
-int

+struct gcov_info *
  gcov_profile_merge (struct gcov_info *tgt_profile, struct gcov_info 
*src_profile,
  int w1, int w2)
  {
struct gcov_info *gi_ptr;
struct gcov_info **tgt_infos;
-  struct gcov_info *tgt_tail;
+  struct gcov_info **tgt_tail;
struct gcov_info **in_src_not_tgt;
unsigned tgt_cnt = 0, src_cnt = 0;
unsigned unmatch_info_cnt = 0;
@@ -703,7 +703,10 @@ gcov_profile_merge (struct gcov_info *tgt_profile, struct 
gcov_info *src_profile
for (gi_ptr = tgt_profile, i = 0; gi_ptr; gi_ptr = gi_ptr->next, i++)
  tgt_infos[i] = gi_ptr;
  
-  tgt_tail = tgt_infos[tgt_cnt - 1];

+  if (tgt_cnt)
+ tgt_tail = _infos[tgt_cnt - 1]->next;
+  else
+ tgt_tail = _profile;
  
/* First pass on tgt_profile, we multiply w1 to all counters.  */

if (w1 > 1)
@@ -732,14 +735,14 @@ gcov_profile_merge (struct gcov_info *tgt_profile, struct 
gcov_info *src_profile
gi_ptr = in_src_not_tgt[i];
gcov_merge (gi_ptr, gi_ptr, w2 - 1);
gi_ptr->next = NULL;
-  tgt_tail->next = gi_ptr;
-  tgt_tail = gi_ptr;
+  *tgt_tail = gi_ptr;
+  tgt_tail = _ptr->next;
  }
  
free (in_src_not_tgt);

free (tgt_infos);
  
-  return 0;

+  return tgt_profile;
  }
  
  typedef gcov_type (*counter_op_fn) (gcov_type, void*, void*);




[PATCH] rtl-optimization/105028 - fix compile-time hog in form_threads_from_copies

2022-03-23 Thread Richard Biener via Gcc-patches
form_threads_from_copies processes a sorted array of copies, skipping
those with the same thread and conflicting threads and merging the
first non-conflicting ones.  After that it terminates the loop and
gathers the remaining elements of the array, skipping same thread
copies, re-starting the process.  For a large number of copies this
gathering of the rest takes considerable time and it also appears
pointless.  The following simply continues processing the array
which should be equivalent as far as I can see.

This takes form_threads_from_copies off the profile radar from
previously taking ~50% of the compile-time.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

OK if testing succeeds?

Thanks,
Richard.

2022-03-23  Richard Biener  

PR rtl-optimization/105028
* ira-color.cc (form_threads_from_copies): Remove unnecessary
copying of the sorted_copies tail.
---
 gcc/ira-color.cc | 71 +++-
 1 file changed, 28 insertions(+), 43 deletions(-)

diff --git a/gcc/ira-color.cc b/gcc/ira-color.cc
index e01d1841a08..4a1a325e8e3 100644
--- a/gcc/ira-color.cc
+++ b/gcc/ira-color.cc
@@ -2348,58 +2348,43 @@ form_threads_from_copies (int cp_num)
 {
   ira_allocno_t a, thread1, thread2;
   ira_copy_t cp;
-  int i, n;
 
   qsort (sorted_copies, cp_num, sizeof (ira_copy_t), copy_freq_compare_func);
   /* Form threads processing copies, most frequently executed
  first.  */
-  for (; cp_num != 0;)
+  for (int i = 0; i < cp_num; i++)
 {
-  for (i = 0; i < cp_num; i++)
+  cp = sorted_copies[i];
+  thread1 = ALLOCNO_COLOR_DATA (cp->first)->first_thread_allocno;
+  thread2 = ALLOCNO_COLOR_DATA (cp->second)->first_thread_allocno;
+  if (thread1 == thread2)
+   continue;
+  if (! allocno_thread_conflict_p (thread1, thread2))
{
- cp = sorted_copies[i];
- thread1 = ALLOCNO_COLOR_DATA (cp->first)->first_thread_allocno;
- thread2 = ALLOCNO_COLOR_DATA (cp->second)->first_thread_allocno;
- if (thread1 == thread2)
-   continue;
- if (! allocno_thread_conflict_p (thread1, thread2))
+ if (internal_flag_ira_verbose > 3 && ira_dump_file != NULL)
+   fprintf
+   (ira_dump_file,
+"Forming thread by copy %d:a%dr%d-a%dr%d (freq=%d):\n",
+cp->num, ALLOCNO_NUM (cp->first), ALLOCNO_REGNO (cp->first),
+ALLOCNO_NUM (cp->second), ALLOCNO_REGNO (cp->second),
+cp->freq);
+ merge_threads (thread1, thread2);
+ if (internal_flag_ira_verbose > 3 && ira_dump_file != NULL)
{
- if (internal_flag_ira_verbose > 3 && ira_dump_file != NULL)
-   fprintf
- (ira_dump_file,
-  "Forming thread by copy %d:a%dr%d-a%dr%d 
(freq=%d):\n",
-  cp->num, ALLOCNO_NUM (cp->first), ALLOCNO_REGNO (cp->first),
-  ALLOCNO_NUM (cp->second), ALLOCNO_REGNO (cp->second),
-  cp->freq);
- merge_threads (thread1, thread2);
- if (internal_flag_ira_verbose > 3 && ira_dump_file != NULL)
-   {
- thread1 = ALLOCNO_COLOR_DATA (thread1)->first_thread_allocno;
- fprintf (ira_dump_file, "  Result (freq=%d): 
a%dr%d(%d)",
-  ALLOCNO_COLOR_DATA (thread1)->thread_freq,
-  ALLOCNO_NUM (thread1), ALLOCNO_REGNO (thread1),
-  ALLOCNO_FREQ (thread1));
- for (a = ALLOCNO_COLOR_DATA (thread1)->next_thread_allocno;
-  a != thread1;
-  a = ALLOCNO_COLOR_DATA (a)->next_thread_allocno)
-   fprintf (ira_dump_file, " a%dr%d(%d)",
-ALLOCNO_NUM (a), ALLOCNO_REGNO (a),
-ALLOCNO_FREQ (a));
- fprintf (ira_dump_file, "\n");
-   }
- i++;
- break;
+ thread1 = ALLOCNO_COLOR_DATA (thread1)->first_thread_allocno;
+ fprintf (ira_dump_file, "  Result (freq=%d): a%dr%d(%d)",
+  ALLOCNO_COLOR_DATA (thread1)->thread_freq,
+  ALLOCNO_NUM (thread1), ALLOCNO_REGNO (thread1),
+  ALLOCNO_FREQ (thread1));
+ for (a = ALLOCNO_COLOR_DATA (thread1)->next_thread_allocno;
+  a != thread1;
+  a = ALLOCNO_COLOR_DATA (a)->next_thread_allocno)
+   fprintf (ira_dump_file, " a%dr%d(%d)",
+ALLOCNO_NUM (a), ALLOCNO_REGNO (a),
+ALLOCNO_FREQ (a));
+ fprintf (ira_dump_file, "\n");
}
}
-  /* Collect the rest of copies.  */
-  for (n = 0; i < cp_num; i++)
-   {
- cp = sorted_copies[i];
- if (ALLOCNO_COLOR_DATA (cp->first)->first_thread_allocno
- != 

Re: [PATCH] rs6000: Adjust error messages.

2022-03-23 Thread Segher Boessenkool
On Wed, Mar 23, 2022 at 11:31:12AM +0100, Martin Liska wrote:
> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -4345,8 +4345,8 @@ rs6000_option_override_internal (bool global_init_p)
>   rs6000_veclib_handler = rs6000_builtin_vectorized_libmass;
> else
>   {
> -   error ("unknown vectorization library ABI type %qs for "
> -  "%qs switch", rs6000_veclibabi_name, "-mveclibabi=");
> +   error ("unknown vectorization library type in "
> +  "%<-mveclibabi=%s%>", rs6000_veclibabi_name);

"ABI type".

Okay for trunk with that restored.  Thanks!


Segher


[PATCH] rs6000: Adjust error messages.

2022-03-23 Thread Martin Liska
gcc/ChangeLog:

* config/rs6000/rs6000-c.cc (altivec_resolve_overloaded_builtin):
Use %qs in format.
* config/rs6000/rs6000.cc (rs6000_option_override_internal):
Reword the error message.
---
 gcc/config/rs6000/rs6000-c.cc | 5 +++--
 gcc/config/rs6000/rs6000.cc   | 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/gcc/config/rs6000/rs6000-c.cc b/gcc/config/rs6000/rs6000-c.cc
index ad976753a3c..2f0f4fe337d 100644
--- a/gcc/config/rs6000/rs6000-c.cc
+++ b/gcc/config/rs6000/rs6000-c.cc
@@ -1788,8 +1788,9 @@ altivec_resolve_overloaded_builtin (location_t loc, tree 
fndecl,
{
  if (TYPE_READONLY (TREE_TYPE (type))
  && !TYPE_READONLY (TREE_TYPE (decl_type)))
-   warning (0, "passing argument %d of %qE discards % "
-"qualifier from pointer target type", n + 1, fndecl);
+   warning (0, "passing argument %d of %qE discards %qs "
+"qualifier from pointer target type", n + 1, fndecl,
+"const");
  type = build_qualified_type (TREE_TYPE (type), 0);
  type = build_pointer_type (type);
  arg = fold_convert (type, arg);
diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index a7645820b1e..53c70b82529 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -4345,8 +4345,8 @@ rs6000_option_override_internal (bool global_init_p)
rs6000_veclib_handler = rs6000_builtin_vectorized_libmass;
  else
{
- error ("unknown vectorization library ABI type %qs for "
-"%qs switch", rs6000_veclibabi_name, "-mveclibabi=");
+ error ("unknown vectorization library type in "
+"%<-mveclibabi=%s%>", rs6000_veclibabi_name);
  ret = false;
}
}
-- 
2.35.1



[committed] libstdc++: Fix feature test macros in for freestanding

2022-03-23 Thread Jonathan Wakely via Gcc-patches
Tested powerpc64le-linux, pushed to trunk.

Backport to gcc-11 needed too.

-- >8 --

Some C++17 and C++20 feature test macros are only defined in 
for hosted builds, even though the features are supported for
freestanding.

All C++23 feature test macros are defined in  for freestanding,
but most of the features are only supported for hosted.

libstdc++-v3/ChangeLog:

* include/std/version [!_GLIBCXX_HOSTED]
(__cpp_lib_hardware_interference_size): Define for freestanding.
(__cpp_lib_bit_cast): Likewise.
(__cpp_lib_is_layout_compatible): Likewise.
(__cpp_lib_is_pointer_interconvertible): Likewise.
(__cpp_lib_adaptor_iterator_pair_constructor): Do not define for
freestanding.
(__cpp_lib_invoke_r): Likewise.
(__cpp_lib_ios_noreplace): Likewise.
(__cpp_lib_monadic_optional): Likewise.
(__cpp_lib_move_only_function): Likewise.
(__cpp_lib_spanstream): Likewise.
(__cpp_lib_stacktrace): Likewise.
(__cpp_lib_string_contains): Likewise.
(__cpp_lib_string_resize_and_overwrite): Likewise.
(__cpp_lib_to_underlying): Likewise.
---
 libstdc++-v3/include/std/version | 35 +---
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/libstdc++-v3/include/std/version b/libstdc++-v3/include/std/version
index d730a7ea3c7..535f095108a 100644
--- a/libstdc++-v3/include/std/version
+++ b/libstdc++-v3/include/std/version
@@ -108,6 +108,9 @@
 #ifdef _GLIBCXX_HAVE_BUILTIN_HAS_UNIQ_OBJ_REP
 # define __cpp_lib_has_unique_object_representations 201606L
 #endif
+#ifdef __GCC_DESTRUCTIVE_SIZE
+# define __cpp_lib_hardware_interference_size 201703L
+#endif
 #ifdef _GLIBCXX_HAVE_BUILTIN_IS_AGGREGATE
 # define __cpp_lib_is_aggregate 201703L
 #endif
@@ -142,9 +145,6 @@
 #define __cpp_lib_filesystem 201703L
 #define __cpp_lib_gcd 201606L
 #define __cpp_lib_gcd_lcm 201606L
-#ifdef __GCC_DESTRUCTIVE_SIZE
-# define __cpp_lib_hardware_interference_size 201703L
-#endif
 #define __cpp_lib_hypot 201603L
 #define __cpp_lib_invoke 201411L
 #define __cpp_lib_lcm 201606L
@@ -188,6 +188,9 @@
 #define __cpp_lib_atomic_float 201711L
 #define __cpp_lib_atomic_ref 201806L
 #define __cpp_lib_atomic_value_initialization 201911L
+#if __has_builtin(__builtin_bit_cast)
+# define __cpp_lib_bit_cast 201806L
+#endif
 #define __cpp_lib_bitops 201907L
 #define __cpp_lib_bounded_array_traits 201902L
 // __cpp_lib_char8_t is defined in 
@@ -202,7 +205,15 @@
 #ifdef _GLIBCXX_HAVE_IS_CONSTANT_EVALUATED
 # define __cpp_lib_is_constant_evaluated 201811L
 #endif
+#if __has_builtin(__is_layout_compatible) \
+  && __has_builtin(__builtin_is_corresponding_member)
+# define __cpp_lib_is_layout_compatible 201907L
+#endif
 #define __cpp_lib_is_nothrow_convertible 201806L
+#if __has_builtin(__is_pointer_interconvertible_base_of) \
+ && __has_builtin(__builtin_is_pointer_interconvertible_with_class)
+# define __cpp_lib_is_pointer_interconvertible 201907L
+#endif
 #define __cpp_lib_remove_cvref 201711L
 #if __has_builtin(__builtin_source_location)
 # define __cpp_lib_source_location 201907L
@@ -224,9 +235,6 @@
 # endif
 #endif
 #define __cpp_lib_bind_front 201907L
-#if __has_builtin(__builtin_bit_cast)
-# define __cpp_lib_bit_cast 201806L
-#endif
 // FIXME: #define __cpp_lib_execution 201902L
 #define __cpp_lib_integer_comparison_functions 202002L
 #define __cpp_lib_constexpr_algorithms 201806L
@@ -255,14 +263,6 @@
 #ifdef _GLIBCXX_HAS_GTHREADS
 # define __cpp_lib_jthread 201911L
 #endif
-#if __has_builtin(__is_layout_compatible) \
-  && __has_builtin(__builtin_is_corresponding_member)
-# define __cpp_lib_is_layout_compatible 201907L
-#endif
-#if __has_builtin(__is_pointer_interconvertible_base_of) \
- && __has_builtin(__builtin_is_pointer_interconvertible_with_class)
-# define __cpp_lib_is_pointer_interconvertible 201907L
-#endif
 #if __cpp_lib_atomic_wait
 # define __cpp_lib_latch 201907L
 #endif
@@ -300,12 +300,14 @@
 
 #if __cplusplus > 202002L
 // c++2b
-#define __cpp_lib_adaptor_iterator_pair_constructor 202106L
 #define __cpp_lib_byteswap 202110L
 #define __cpp_lib_constexpr_typeinfo 202106L
+#define __cpp_lib_is_scoped_enum 202011L
+
+#if _GLIBCXX_HOSTED
+#define __cpp_lib_adaptor_iterator_pair_constructor 202106L
 #define __cpp_lib_invoke_r 202106L
 #define __cpp_lib_ios_noreplace 202200L
-#define __cpp_lib_is_scoped_enum 202011L
 #if __cpp_lib_concepts
 # define __cpp_lib_monadic_optional 202110L
 #endif
@@ -321,6 +323,7 @@
 # define __cpp_lib_string_resize_and_overwrite 202110L
 #endif
 #define __cpp_lib_to_underlying 202102L
+#endif
 #endif // C++2b
 #endif // C++20
 #endif // C++17
-- 
2.34.1



[committed] libstdc++: Disable atomic wait for freestanding [PR105021]

2022-03-23 Thread Jonathan Wakely via Gcc-patches
Tested powerpc64le-linux, pushed to trunk.

Backport to gcc-11 needed too.

-- >8 --

We use either condition variables or futexes to implement atomic waits,
so we can't do it in freestanding. This is non-conforming, so should be
revisited later, probably by making freestanding atomic waiting
operations spin without ever blocking.

Reviewed-by: Thomas Rodgers 

libstdc++-v3/ChangeLog:

PR libstdc++/105021
* include/bits/atomic_base.h [!_GLIBCXX_HOSTED]: Do not include
 for freestanding.
---
 libstdc++-v3/include/bits/atomic_base.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libstdc++-v3/include/bits/atomic_base.h 
b/libstdc++-v3/include/bits/atomic_base.h
index d86766cf39b..2cad8486c69 100644
--- a/libstdc++-v3/include/bits/atomic_base.h
+++ b/libstdc++-v3/include/bits/atomic_base.h
@@ -37,7 +37,7 @@
 #include 
 #include 
 
-#if __cplusplus > 201703L
+#if __cplusplus > 201703L && _GLIBCXX_HOSTED
 #include 
 #endif
 
-- 
2.34.1



[PATCH] gcov-tool: Allow merging of empty profile lists

2022-03-23 Thread Sebastian Huber
The gcov_profile_merge() already had code to deal with profile information
which had no counterpart to merge with.  For profile information from files
with no associated counterpart, the profile information is simply used as is
with the weighting transformation applied.  Make sure that gcov_profile_merge()
works with an empty target profile list.  Return the merged profile list.

gcc/
* gcov-tool.cc (gcov_profile_merge): Adjust return type.
(profile_merge): Allow merging of directories which contain no profile
files.

libgcc/
* libgcov-util.c (gcov_profile_merge): Return the list of merged
profiles.  Accept empty target and source profile lists.
---
 gcc/gcov-tool.cc  | 27 ++-
 libgcc/libgcov-util.c | 15 +--
 2 files changed, 19 insertions(+), 23 deletions(-)

diff --git a/gcc/gcov-tool.cc b/gcc/gcov-tool.cc
index f4e42ae763c..2e4083a664d 100644
--- a/gcc/gcov-tool.cc
+++ b/gcc/gcov-tool.cc
@@ -40,7 +40,8 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If 
not, see
 #endif
 #include 
 
-extern int gcov_profile_merge (struct gcov_info*, struct gcov_info*, int, int);
+extern struct gcov_info *gcov_profile_merge (struct gcov_info*,
+struct gcov_info*, int, int);
 extern int gcov_profile_overlap (struct gcov_info*, struct gcov_info*);
 extern int gcov_profile_normalize (struct gcov_info*, gcov_type);
 extern int gcov_profile_scale (struct gcov_info*, float, int, int);
@@ -141,26 +142,18 @@ profile_merge (const char *d1, const char *d2, const char 
*out, int w1, int w2)
 {
   struct gcov_info *d1_profile;
   struct gcov_info *d2_profile;
-  int ret;
+  struct gcov_info *merged_profile;
 
   d1_profile = gcov_read_profile_dir (d1, 0);
-  if (!d1_profile)
-return 1;
-
-  if (d2)
-{
-  d2_profile = gcov_read_profile_dir (d2, 0);
-  if (!d2_profile)
-return 1;
+  d2_profile = gcov_read_profile_dir (d2, 0);
 
-  /* The actual merge: we overwrite to d1_profile.  */
-  ret = gcov_profile_merge (d1_profile, d2_profile, w1, w2);
+  /* The actual merge: we overwrite to d1_profile.  */
+  merged_profile = gcov_profile_merge (d1_profile, d2_profile, w1, w2);
 
-  if (ret)
-return ret;
-}
-
-  gcov_output_files (out, d1_profile);
+  if (merged_profile)
+gcov_output_files (out, merged_profile);
+  else if (verbose)
+fnotice (stdout, "no profile files were merged\n");
 
   return 0;
 }
diff --git a/libgcc/libgcov-util.c b/libgcc/libgcov-util.c
index ba7fb924b53..e5496f4ade2 100644
--- a/libgcc/libgcov-util.c
+++ b/libgcc/libgcov-util.c
@@ -677,13 +677,13 @@ find_match_gcov_info (struct gcov_info **array, int size,
Return 0 on success: without mismatch.
Reutrn 1 on error.  */
 
-int
+struct gcov_info *
 gcov_profile_merge (struct gcov_info *tgt_profile, struct gcov_info 
*src_profile,
 int w1, int w2)
 {
   struct gcov_info *gi_ptr;
   struct gcov_info **tgt_infos;
-  struct gcov_info *tgt_tail;
+  struct gcov_info **tgt_tail;
   struct gcov_info **in_src_not_tgt;
   unsigned tgt_cnt = 0, src_cnt = 0;
   unsigned unmatch_info_cnt = 0;
@@ -703,7 +703,10 @@ gcov_profile_merge (struct gcov_info *tgt_profile, struct 
gcov_info *src_profile
   for (gi_ptr = tgt_profile, i = 0; gi_ptr; gi_ptr = gi_ptr->next, i++)
 tgt_infos[i] = gi_ptr;
 
-  tgt_tail = tgt_infos[tgt_cnt - 1];
+  if (tgt_cnt)
+ tgt_tail = _infos[tgt_cnt - 1]->next;
+  else
+ tgt_tail = _profile;
 
   /* First pass on tgt_profile, we multiply w1 to all counters.  */
   if (w1 > 1)
@@ -732,14 +735,14 @@ gcov_profile_merge (struct gcov_info *tgt_profile, struct 
gcov_info *src_profile
   gi_ptr = in_src_not_tgt[i];
   gcov_merge (gi_ptr, gi_ptr, w2 - 1);
   gi_ptr->next = NULL;
-  tgt_tail->next = gi_ptr;
-  tgt_tail = gi_ptr;
+  *tgt_tail = gi_ptr;
+  tgt_tail = _ptr->next;
 }
 
   free (in_src_not_tgt);
   free (tgt_infos);
 
-  return 0;
+  return tgt_profile;
 }
 
 typedef gcov_type (*counter_op_fn) (gcov_type, void*, void*);
-- 
2.34.1



[PATCH] rs6000: Skip overload instances with NULL fntype [PR104967]

2022-03-23 Thread Kewen.Lin via Gcc-patches
Hi,

As shown in PR104967, for some overload built-in function instance,
if it requires a date type which isn't defined on the target, its
fntype would be initialized as NULL.  This patch is to consider
this possibility in function find_instance to avoid ICE.

Bootstrapped and regtested on powerpc64-linux-gnu P8 and
powerpc64le-linux-gnu P9 and P10.

Is it ok for trunk?

BR,
Kewen

PR target/104967

gcc/ChangeLog:

* config/rs6000/rs6000-c.cc (find_instance): Skip instances with null
function types.

---
 gcc/config/rs6000/rs6000-c.cc | 4 
 1 file changed, 4 insertions(+)

diff --git a/gcc/config/rs6000/rs6000-c.cc b/gcc/config/rs6000/rs6000-c.cc
index 15251efc209..b0f9fce4b54 100644
--- a/gcc/config/rs6000/rs6000-c.cc
+++ b/gcc/config/rs6000/rs6000-c.cc
@@ -1678,6 +1678,10 @@ find_instance (bool *unsupported_builtin, ovlddata 
**instance,

   ovlddata *inst = *instance;
   gcc_assert (inst != NULL);
+  /* It is possible for an instance to require a data type that isn't
+ defined on this target, in which case inst->fntype will be NULL.  */
+  if (!inst->fntype)
+return error_mark_node;
   tree fntype = rs6000_builtin_info[inst->bifid].fntype;
   tree parmtype0 = TREE_VALUE (TYPE_ARG_TYPES (fntype));
   tree parmtype1 = TREE_VALUE (TREE_CHAIN (TYPE_ARG_TYPES (fntype)));
--
2.25.1


Re: [PATCH] testsuite: Fix up sse2-v1ti-shift-3.c test [PR102986]

2022-03-23 Thread Richard Biener via Gcc-patches
On Wed, 23 Mar 2022, Jakub Jelinek wrote:

> Hi!
> 
> This test is dg-do run and invokes UB when these rotate functions
> are called with 0 as second argument.  There are some other tests
> that do this but they are dg-do compile only and not even call those
> functions at all, so it IMHO doesn't matter that they are only well
> defined for [1,127] and not [0,127].
> 
> The following patch fixes it, we pattern recognize both forms as rotates
> and we emit identical assembly.
> 
> Tested on x86_64-linux -m32/-m64, ok for trunk?

OK.

> 2022-03-23  Jakub Jelinek  
> 
>   PR target/102986
>   * gcc.target/i386/sse2-v1ti-shift-3.c (rotr_v1ti, rotl_v1ti, rotr_ti,
>   rotl_ti): Use -i&127 instead of 128-i to avoid UB on i == 0.
> 
> --- gcc/testsuite/gcc.target/i386/sse2-v1ti-shift-3.c.jj  2021-12-30 
> 15:12:43.709143657 +0100
> +++ gcc/testsuite/gcc.target/i386/sse2-v1ti-shift-3.c 2022-03-23 
> 09:36:41.622181842 +0100
> @@ -14,14 +14,14 @@ typedef __int128 ti;
>  uv1ti ashl_v1ti(uv1ti x, unsigned int i) { return x << i; }
>  uv1ti lshr_v1ti(uv1ti x, unsigned int i) { return x >> i; }
>  sv1ti ashr_v1ti(sv1ti x, unsigned int i) { return x >> i; }
> -uv1ti rotr_v1ti(uv1ti x, unsigned int i) { return (x >> i) | (x << (128-i)); 
> }
> -uv1ti rotl_v1ti(uv1ti x, unsigned int i) { return (x << i) | (x >> (128-i)); 
> }
> +uv1ti rotr_v1ti(uv1ti x, unsigned int i) { return (x >> i) | (x << 
> (-i&127)); }
> +uv1ti rotl_v1ti(uv1ti x, unsigned int i) { return (x << i) | (x >> 
> (-i&127)); }
>  
>  uti ashl_ti(uti x, unsigned int i) { return x << i; }
>  uti lshr_ti(uti x, unsigned int i) { return x >> i; }
>  sti ashr_ti(sti x, unsigned int i) { return x >> i; }
> -uti rotr_ti(uti x, unsigned int i) { return (x >> i) | (x << (128-i)); }
> -uti rotl_ti(uti x, unsigned int i) { return (x << i) | (x >> (128-i)); }
> +uti rotr_ti(uti x, unsigned int i) { return (x >> i) | (x << (-i&127)); }
> +uti rotl_ti(uti x, unsigned int i) { return (x << i) | (x >> (-i&127)); }
>  
>  void test(ti x)
>  {
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)


[PATCH] testsuite: Fix up sse2-v1ti-shift-3.c test [PR102986]

2022-03-23 Thread Jakub Jelinek via Gcc-patches
Hi!

This test is dg-do run and invokes UB when these rotate functions
are called with 0 as second argument.  There are some other tests
that do this but they are dg-do compile only and not even call those
functions at all, so it IMHO doesn't matter that they are only well
defined for [1,127] and not [0,127].

The following patch fixes it, we pattern recognize both forms as rotates
and we emit identical assembly.

Tested on x86_64-linux -m32/-m64, ok for trunk?

2022-03-23  Jakub Jelinek  

PR target/102986
* gcc.target/i386/sse2-v1ti-shift-3.c (rotr_v1ti, rotl_v1ti, rotr_ti,
rotl_ti): Use -i&127 instead of 128-i to avoid UB on i == 0.

--- gcc/testsuite/gcc.target/i386/sse2-v1ti-shift-3.c.jj2021-12-30 
15:12:43.709143657 +0100
+++ gcc/testsuite/gcc.target/i386/sse2-v1ti-shift-3.c   2022-03-23 
09:36:41.622181842 +0100
@@ -14,14 +14,14 @@ typedef __int128 ti;
 uv1ti ashl_v1ti(uv1ti x, unsigned int i) { return x << i; }
 uv1ti lshr_v1ti(uv1ti x, unsigned int i) { return x >> i; }
 sv1ti ashr_v1ti(sv1ti x, unsigned int i) { return x >> i; }
-uv1ti rotr_v1ti(uv1ti x, unsigned int i) { return (x >> i) | (x << (128-i)); }
-uv1ti rotl_v1ti(uv1ti x, unsigned int i) { return (x << i) | (x >> (128-i)); }
+uv1ti rotr_v1ti(uv1ti x, unsigned int i) { return (x >> i) | (x << (-i&127)); }
+uv1ti rotl_v1ti(uv1ti x, unsigned int i) { return (x << i) | (x >> (-i&127)); }
 
 uti ashl_ti(uti x, unsigned int i) { return x << i; }
 uti lshr_ti(uti x, unsigned int i) { return x >> i; }
 sti ashr_ti(sti x, unsigned int i) { return x >> i; }
-uti rotr_ti(uti x, unsigned int i) { return (x >> i) | (x << (128-i)); }
-uti rotl_ti(uti x, unsigned int i) { return (x << i) | (x >> (128-i)); }
+uti rotr_ti(uti x, unsigned int i) { return (x >> i) | (x << (-i&127)); }
+uti rotl_ti(uti x, unsigned int i) { return (x << i) | (x >> (-i&127)); }
 
 void test(ti x)
 {

Jakub



Re: [Patch] LTO: Fixes for renaming issues with offload/OpenMP [PR104285]

2022-03-23 Thread Richard Biener via Gcc-patches
On Tue, 22 Mar 2022, Tobias Burnus wrote:

> Hi Jakub, hi Richi,
> 
> On 22.03.22 15:27, Jakub Jelinek wrote:
> > On Tue, Mar 22, 2022 at 01:56:27PM +0100, Tobias Burnus wrote:
> >> name = lto_get_decl_name_mapping (file_data, name);
> >> [...]
> >> +  name = lto_get_decl_name_mapping (file_data, name);
> >>
> >> This looks weird.  IMHO we should make sure that the hash table contains
> >> always the final names, so that we don't need to call it twice or even more
> >> times.
> 
> I concur – and I have changed it (see attached patch). And it still passes the
> testsuite.
> 
> (It came about because I started reverse – and in
> 'cgraph_node::get_untransformed_body',
> it happened that the second map was already present + I added the second
> lookup.
> Extending the tests, I found that I also had to change
> privatize_symbol_name_1. Thus,
> I added the additional mapping there.  However, it seems to be enough to only
> change
> privatize_symbol_name_1 to track the double renaming in one step.
> That's what the updated patch does (+ unchanged changes for linkptr + the
> hash-key part).
> 
> > The lto.cc changes LGTM.  The rest I'm unfortunately not too familiar with,
> > Richi or Honza, could you please have a look?
> 
> (^ to be done by Honza or Richi)
> 
> > This is the same thing again as above, right?
> 
> Yes. (Now also removed.)
> 
> Thanks for the first review and for nagging about the rename tracking.

OK.

Thanks,
Richard.


Re: [PATCH] Fix ICE caused by NULL_RTX returned by lowpart_subreg.

2022-03-23 Thread Uros Bizjak via Gcc-patches
On Wed, Mar 23, 2022 at 7:04 AM liuhongt  wrote:
>
> In validate_subreg, both (subreg:V2HF (reg:SI) 0)
> and (subreg:V8HF (reg:V2HF) 0) are valid, but not
> for (subreg:V8HF (reg:SI) 0) which causes ICE.
>
> Ideally it should be handled in validate_subreg to support
> subreg for all modes available in TARGET_CAN_CHANGE_MODE_CLASS, but
> that would be too risky in stage4, so the patch is a walkround in the
> backend to force_reg operands before lowpart_subreg for expanders or
> pre_reload splitters.
>
> Bootstrapped and regtest on x86_64-pc-linux-gnu{-m32,}.
> Also with native on SPR.
> Ok for trunk?
>
> gcc/ChangeLog:
>
> PR target/104976
> * config/i386/sse.md (ssePSmodelower): New.
> (*avx_cmp3_ltint_not): Force_reg operand before
> lowpart_subreg to avoid NULL_RTX.
> (_fmaddc__mask1,
> _fcmaddc__mask1,
> fma__fmaddc_bcst, fma__fcmaddc_bcst,
> ___mask,
> avx512fp16_fcmaddcsh_v8hf_mask1,
> avx512fp16_fcmaddcsh_v8hf_mask3,
> avx512fp16_fmaddcsh_v8hf_mask3,
> avx512fp16_fmaddcsh_v8hf_mask3,
> floatv4hf2,
> floatv2div2hf2,
> fix_truncv4hf2,
> fix_truncv2hfv2di2, extendv4hf2,
> extendv2hfv2df2,
> truncv4hf2,truncv2dfv2hf2,
> *avx512bw_permvar_truncv16siv16hi_1,
> *avx512bw_permvar_truncv16siv16hi_1_hf,
> *avx512f_permvar_truncv8siv8hi_1,
> *avx512f_permvar_truncv8siv8hi_1_hf,
> *avx512f_vpermvar_truncv8div8si_1,
> *avx512f_permvar_truncv32hiv32qi_1,
> *avx512f_permvar_truncv16hiv16qi_1,
> *avx512f_permvar_truncv4div4si_1,
> *avx512f_pshufb_truncv8hiv8qi_1,
> *avx512f_pshufb_truncv4siv4hi_1,
> *avx512f_pshufd_truncv2div2si_1,
> sdot_prod, avx2_pblend_1,
> ashrv2di3,ashrv2di3,usdot_prod): Ditto.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/i386/pr104976.c: New test.
> * gcc.target/i386/avx512fp16-vfcmaddcph-1a.c: Scan either
> vblendps or masked vmovaps.
> * gcc.target/i386/avx512fp16-vfmaddcph-1a.c: Ditto
> * gcc.target/i386/avx512fp16vl-vfcmaddcph-1a.c: Ditto.
> * gcc.target/i386/avx512fp16vl-vfmaddcph-1a.c: Ditto.

LGTM.

Thanks,
Uros.

> ---
>  gcc/config/i386/sse.md| 236 +-
>  .../i386/avx512fp16-vfcmaddcph-1a.c   |   2 +-
>  .../gcc.target/i386/avx512fp16-vfmaddcph-1a.c |   2 +-
>  .../i386/avx512fp16vl-vfcmaddcph-1a.c |   4 +-
>  .../i386/avx512fp16vl-vfmaddcph-1a.c  |   4 +-
>  gcc/testsuite/gcc.target/i386/pr104976.c  |  13 +
>  6 files changed, 196 insertions(+), 65 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr104976.c
>
> diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
> index 6f7af2f21d6..a9e18d38323 100644
> --- a/gcc/config/i386/sse.md
> +++ b/gcc/config/i386/sse.md
> @@ -1059,6 +1059,18 @@ (define_mode_attr ssePSmode
> (V4DF "V8SF") (V2DF "V4SF")
> (V32HF "V16SF") (V16HF "V8SF") (V8HF "V4SF")])
>
> +(define_mode_attr ssePSmodelower
> +  [(V16SI "v16sf") (V8DF "v16sf")
> +   (V16SF "v16sf") (V8DI "v16sf")
> +   (V64QI "v16sf") (V32QI "v8sf") (V16QI "v4sf")
> +   (V32HI "v16sf") (V16HI "v8sf") (V8HI "v4sf")
> +   (V8SI "v8sf") (V4SI "v4sf")
> +   (V4DI "v8sf") (V2DI "v4sf")
> +   (V4TI "v16sf") (V2TI "v8sf") (V1TI "v4sf")
> +   (V8SF "v8sf") (V4SF "v4sf")
> +   (V4DF "v8sf") (V2DF "v4sf")
> +   (V32HF "v16sf") (V16HF "v8sf") (V8HF "v4sf")])
> +
>  (define_mode_attr ssePSmode2
>[(V8DI "V8SF") (V4DI "V4SF")])
>
> @@ -3617,6 +3629,9 @@ (define_insn_and_split "*avx_cmp3_ltint_not"
>operands[1] = force_reg (mode,
>   gen_lowpart (mode, operands[1]));
>operands[2] = gen_lowpart (mode, operands[2]);
> +
> +  if (!MEM_P (operands[3]))
> +operands[3] = force_reg (mode, operands[3]);
>operands[3] = lowpart_subreg (mode, operands[3], mode);
>  })
>
> @@ -6319,7 +6334,7 @@ (define_expand 
> "_fmaddc__mask1"
> (match_operand: 4 "register_operand")]
>"TARGET_AVX512FP16 && "
>  {
> -  rtx op0, op1;
> +  rtx op0, op1, dest;
>if ()
>  emit_insn (gen__fmaddc__mask (
>operands[0], operands[1], operands[2], operands[3],
> @@ -6328,9 +6343,16 @@ (define_expand 
> "_fmaddc__mask1"
>  emit_insn (gen__fmaddc__mask (operands[0],
>operands[1], operands[2], operands[3], operands[4]));
>
> -  op0 = lowpart_subreg (mode, operands[0], mode);
> +  op0 = lowpart_subreg (mode,
> +   force_reg (mode, operands[0]),
> +   mode);
> +  dest = gen_reg_rtx (mode);
> +  if (!MEM_P (operands[1]))
> +operands[1] = force_reg (mode, operands[1]);
>op1 = lowpart_subreg (mode, operands[1], mode);
> -  emit_insn (gen__mask (op0, op0, op1, operands[4]));
> +  emit_insn (gen__mask (dest, op0, op1, operands[4]));
> +  emit_move_insn (operands[0],
> + lowpart_subreg (mode, dest, mode));
>DONE;
>  })
>
> 

Re: [PATCH] Docs: Document that taint analyzer checker stops other checkers

2022-03-23 Thread Avinash Sonawane via Gcc-patches
Apologies! Forgot to attach the patch v2.

Done now.
>From 1d986ffd8b20007a6a2140e50935ade1bebc2228 Mon Sep 17 00:00:00 2001
From: Avinash Sonawane 
Date: Tue, 22 Mar 2022 07:32:44 +0530
Subject: [PATCH] Docs: Document that taint analyzer checker disables some
 warnings

---
 gcc/doc/invoke.texi | 33 +++--
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 4da4a1170f5..1d16f0f167f 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -421,14 +421,13 @@ Objective-C and Objective-C++ Dialects}.
 -fanalyzer-checker=@var{name} @gol
 -fno-analyzer-feasibility @gol
 -fanalyzer-fine-grained @gol
--fanalyzer-state-merge @gol
--fanalyzer-state-purge @gol
+-fno-analyzer-state-merge @gol
+-fno-analyzer-state-purge @gol
 -fanalyzer-transitivity @gol
 -fanalyzer-verbose-edges @gol
 -fanalyzer-verbose-state-changes @gol
 -fanalyzer-verbosity=@var{level} @gol
 -fdump-analyzer @gol
--fdump-analyzer-stderr @gol
 -fdump-analyzer-callgraph @gol
 -fdump-analyzer-exploded-graph @gol
 -fdump-analyzer-exploded-nodes @gol
@@ -438,6 +437,7 @@ Objective-C and Objective-C++ Dialects}.
 -fdump-analyzer-feasibility @gol
 -fdump-analyzer-json @gol
 -fdump-analyzer-state-purge @gol
+-fdump-analyzer-stderr @gol
 -fdump-analyzer-supergraph @gol
 -Wno-analyzer-double-fclose @gol
 -Wno-analyzer-double-free @gol
@@ -9659,22 +9659,24 @@ Enabling this option effectively enables the following warnings:
 -Wanalyzer-free-of-non-heap @gol
 -Wanalyzer-malloc-leak @gol
 -Wanalyzer-mismatching-deallocation @gol
--Wanalyzer-possible-null-argument @gol
--Wanalyzer-possible-null-dereference @gol
 -Wanalyzer-null-argument @gol
 -Wanalyzer-null-dereference @gol
+-Wanalyzer-possible-null-argument @gol
+-Wanalyzer-possible-null-dereference @gol
 -Wanalyzer-shift-count-negative @gol
 -Wanalyzer-shift-count-overflow @gol
 -Wanalyzer-stale-setjmp-buffer @gol
+@ignore
 -Wanalyzer-tainted-allocation-size @gol
 -Wanalyzer-tainted-array-index @gol
 -Wanalyzer-tainted-divisor @gol
 -Wanalyzer-tainted-offset @gol
 -Wanalyzer-tainted-size @gol
+@end ignore
 -Wanalyzer-unsafe-call-within-signal-handler @gol
 -Wanalyzer-use-after-free @gol
--Wanalyzer-use-of-uninitialized-value @gol
 -Wanalyzer-use-of-pointer-in-stale-stack-frame @gol
+-Wanalyzer-use-of-uninitialized-value @gol
 -Wanalyzer-write-to-const @gol
 -Wanalyzer-write-to-string-literal @gol
 }
@@ -10015,6 +10017,25 @@ such as the @code{taint} checker that implements
 @option{-Wanalyzer-tainted-array-index}, and this option is required
 to enable them.
 
+@emph{Note:} currently, @option{-fanalyzer-checker=taint} disables following
+warnings from @option{-fanalyzer}:
+
+@gccoptlist{ @gol
+-Wanalyzer-double-fclose @gol
+-Wanalyzer-double-free @gol
+-Wanalyzer-exposure-through-output-file @gol
+-Wanalyzer-file-leak @gol
+-Wanalyzer-free-of-non-heap @gol
+-Wanalyzer-malloc-leak @gol
+-Wanalyzer-mismatching-deallocation @gol
+-Wanalyzer-null-argument @gol
+-Wanalyzer-null-dereference @gol
+-Wanalyzer-possible-null-argument @gol
+-Wanalyzer-possible-null-dereference @gol
+-Wanalyzer-unsafe-call-within-signal-handler @gol
+-Wanalyzer-use-after-free @gol
+}
+
 @item -fno-analyzer-feasibility
 @opindex fanalyzer-feasibility
 @opindex fno-analyzer-feasibility
-- 
2.32.0



Re: [PATCH] Docs: Document that taint analyzer checker stops other checkers

2022-03-23 Thread Avinash Sonawane via Gcc-patches
On Tue, 22 Mar 2022 19:15:02 -0400
David Malcolm  wrote:

> So it would be even better if the text listed the warnings that are
> affected by this: basically all of the ones that are implemented in
> checkers i.e. analyzer/sm-*.cc (apart from sm-taint, of course).

Done! I could find 13 warnings in analyzer/sm-*.cc. Hope I didn't miss
any. :)

I also rearranged some flags to maintain the sort order.

Please find the attached v2 of the patch. Should I post another
fresh mail with subject line "[PATCH v2] Docs:..."? Or should've the
subject of this current thread been edited to "[PATCH v2]" from
"[PATCH]"?

Sorry for bothering. This is my first time submitting the patch on a
mailing list. :)

Thanks!

Regards,
Avinash Sonawane (rootKea)
https://www.rootkea.me


Re: [PATCH] Fix ICE caused by NULL_RTX returned by lowpart_subreg.

2022-03-23 Thread Hongtao Liu via Gcc-patches
On Wed, Mar 23, 2022 at 2:05 PM liuhongt via Gcc-patches
 wrote:
>
> In validate_subreg, both (subreg:V2HF (reg:SI) 0)
> and (subreg:V8HF (reg:V2HF) 0) are valid, but not
> for (subreg:V8HF (reg:SI) 0) which causes ICE.
>
> Ideally it should be handled in validate_subreg to support
> subreg for all modes available in TARGET_CAN_CHANGE_MODE_CLASS, but
> that would be too risky in stage4, so the patch is a walkround in the
> backend to force_reg operands before lowpart_subreg for expanders or
> pre_reload splitters.
>
> Bootstrapped and regtest on x86_64-pc-linux-gnu{-m32,}.
> Also with native on SPR.
> Ok for trunk?
>
> gcc/ChangeLog:
>
> PR target/104976
> * config/i386/sse.md (ssePSmodelower): New.
> (*avx_cmp3_ltint_not): Force_reg operand before
> lowpart_subreg to avoid NULL_RTX.
> (_fmaddc__mask1,
> _fcmaddc__mask1,
> fma__fmaddc_bcst, fma__fcmaddc_bcst,
> ___mask,
> avx512fp16_fcmaddcsh_v8hf_mask1,
> avx512fp16_fcmaddcsh_v8hf_mask3,
> avx512fp16_fmaddcsh_v8hf_mask3,
> avx512fp16_fmaddcsh_v8hf_mask3,
> floatv4hf2,
> floatv2div2hf2,
> fix_truncv4hf2,
> fix_truncv2hfv2di2, extendv4hf2,
> extendv2hfv2df2,
> truncv4hf2,truncv2dfv2hf2,
> *avx512bw_permvar_truncv16siv16hi_1,
> *avx512bw_permvar_truncv16siv16hi_1_hf,
> *avx512f_permvar_truncv8siv8hi_1,
> *avx512f_permvar_truncv8siv8hi_1_hf,
> *avx512f_vpermvar_truncv8div8si_1,
> *avx512f_permvar_truncv32hiv32qi_1,
> *avx512f_permvar_truncv16hiv16qi_1,
> *avx512f_permvar_truncv4div4si_1,
> *avx512f_pshufb_truncv8hiv8qi_1,
> *avx512f_pshufb_truncv4siv4hi_1,
> *avx512f_pshufd_truncv2div2si_1,
> sdot_prod, avx2_pblend_1,
> ashrv2di3,ashrv2di3,usdot_prod): Ditto.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/i386/pr104976.c: New test.
> * gcc.target/i386/avx512fp16-vfcmaddcph-1a.c: Scan either
> vblendps or masked vmovaps.
> * gcc.target/i386/avx512fp16-vfmaddcph-1a.c: Ditto
> * gcc.target/i386/avx512fp16vl-vfcmaddcph-1a.c: Ditto.
> * gcc.target/i386/avx512fp16vl-vfmaddcph-1a.c: Ditto.
> ---
>  gcc/config/i386/sse.md| 236 +-
>  .../i386/avx512fp16-vfcmaddcph-1a.c   |   2 +-
>  .../gcc.target/i386/avx512fp16-vfmaddcph-1a.c |   2 +-
>  .../i386/avx512fp16vl-vfcmaddcph-1a.c |   4 +-
>  .../i386/avx512fp16vl-vfmaddcph-1a.c  |   4 +-
>  gcc/testsuite/gcc.target/i386/pr104976.c  |  13 +
>  6 files changed, 196 insertions(+), 65 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr104976.c
>
> diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
> index 6f7af2f21d6..a9e18d38323 100644
> --- a/gcc/config/i386/sse.md
> +++ b/gcc/config/i386/sse.md
> @@ -1059,6 +1059,18 @@ (define_mode_attr ssePSmode
> (V4DF "V8SF") (V2DF "V4SF")
> (V32HF "V16SF") (V16HF "V8SF") (V8HF "V4SF")])
>
> +(define_mode_attr ssePSmodelower
> +  [(V16SI "v16sf") (V8DF "v16sf")
> +   (V16SF "v16sf") (V8DI "v16sf")
> +   (V64QI "v16sf") (V32QI "v8sf") (V16QI "v4sf")
> +   (V32HI "v16sf") (V16HI "v8sf") (V8HI "v4sf")
> +   (V8SI "v8sf") (V4SI "v4sf")
> +   (V4DI "v8sf") (V2DI "v4sf")
> +   (V4TI "v16sf") (V2TI "v8sf") (V1TI "v4sf")
> +   (V8SF "v8sf") (V4SF "v4sf")
> +   (V4DF "v8sf") (V2DF "v4sf")
> +   (V32HF "v16sf") (V16HF "v8sf") (V8HF "v4sf")])
> +
>  (define_mode_attr ssePSmode2
>[(V8DI "V8SF") (V4DI "V4SF")])
>
> @@ -3617,6 +3629,9 @@ (define_insn_and_split "*avx_cmp3_ltint_not"
>operands[1] = force_reg (mode,
>   gen_lowpart (mode, operands[1]));
>operands[2] = gen_lowpart (mode, operands[2]);
> +
> +  if (!MEM_P (operands[3]))
> +operands[3] = force_reg (mode, operands[3]);
>operands[3] = lowpart_subreg (mode, operands[3], mode);
>  })
>
> @@ -6319,7 +6334,7 @@ (define_expand 
> "_fmaddc__mask1"
> (match_operand: 4 "register_operand")]
>"TARGET_AVX512FP16 && "
>  {
> -  rtx op0, op1;
> +  rtx op0, op1, dest;
>if ()
>  emit_insn (gen__fmaddc__mask (
>operands[0], operands[1], operands[2], operands[3],
> @@ -6328,9 +6343,16 @@ (define_expand 
> "_fmaddc__mask1"
>  emit_insn (gen__fmaddc__mask (operands[0],
>operands[1], operands[2], operands[3], operands[4]));
>
> -  op0 = lowpart_subreg (mode, operands[0], mode);
> +  op0 = lowpart_subreg (mode,
> +   force_reg (mode, operands[0]),
> +   mode);
> +  dest = gen_reg_rtx (mode);
> +  if (!MEM_P (operands[1]))
> +operands[1] = force_reg (mode, operands[1]);
>op1 = lowpart_subreg (mode, operands[1], mode);
> -  emit_insn (gen__mask (op0, op0, op1, operands[4]));
> +  emit_insn (gen__mask (dest, op0, op1, operands[4]));
> +  emit_move_insn (operands[0],
> + lowpart_subreg (mode, dest, mode));
>DONE;
>  })
>
> @@ 

[PATCH] Fix ICE caused by NULL_RTX returned by lowpart_subreg.

2022-03-23 Thread liuhongt via Gcc-patches
In validate_subreg, both (subreg:V2HF (reg:SI) 0)
and (subreg:V8HF (reg:V2HF) 0) are valid, but not
for (subreg:V8HF (reg:SI) 0) which causes ICE.

Ideally it should be handled in validate_subreg to support
subreg for all modes available in TARGET_CAN_CHANGE_MODE_CLASS, but
that would be too risky in stage4, so the patch is a walkround in the
backend to force_reg operands before lowpart_subreg for expanders or
pre_reload splitters.

Bootstrapped and regtest on x86_64-pc-linux-gnu{-m32,}.
Also with native on SPR.
Ok for trunk?

gcc/ChangeLog:

PR target/104976
* config/i386/sse.md (ssePSmodelower): New.
(*avx_cmp3_ltint_not): Force_reg operand before
lowpart_subreg to avoid NULL_RTX.
(_fmaddc__mask1,
_fcmaddc__mask1,
fma__fmaddc_bcst, fma__fcmaddc_bcst,
___mask,
avx512fp16_fcmaddcsh_v8hf_mask1,
avx512fp16_fcmaddcsh_v8hf_mask3,
avx512fp16_fmaddcsh_v8hf_mask3,
avx512fp16_fmaddcsh_v8hf_mask3,
floatv4hf2,
floatv2div2hf2,
fix_truncv4hf2,
fix_truncv2hfv2di2, extendv4hf2,
extendv2hfv2df2,
truncv4hf2,truncv2dfv2hf2,
*avx512bw_permvar_truncv16siv16hi_1,
*avx512bw_permvar_truncv16siv16hi_1_hf,
*avx512f_permvar_truncv8siv8hi_1,
*avx512f_permvar_truncv8siv8hi_1_hf,
*avx512f_vpermvar_truncv8div8si_1,
*avx512f_permvar_truncv32hiv32qi_1,
*avx512f_permvar_truncv16hiv16qi_1,
*avx512f_permvar_truncv4div4si_1,
*avx512f_pshufb_truncv8hiv8qi_1,
*avx512f_pshufb_truncv4siv4hi_1,
*avx512f_pshufd_truncv2div2si_1,
sdot_prod, avx2_pblend_1,
ashrv2di3,ashrv2di3,usdot_prod): Ditto.

gcc/testsuite/ChangeLog:

* gcc.target/i386/pr104976.c: New test.
* gcc.target/i386/avx512fp16-vfcmaddcph-1a.c: Scan either
vblendps or masked vmovaps.
* gcc.target/i386/avx512fp16-vfmaddcph-1a.c: Ditto
* gcc.target/i386/avx512fp16vl-vfcmaddcph-1a.c: Ditto.
* gcc.target/i386/avx512fp16vl-vfmaddcph-1a.c: Ditto.
---
 gcc/config/i386/sse.md| 236 +-
 .../i386/avx512fp16-vfcmaddcph-1a.c   |   2 +-
 .../gcc.target/i386/avx512fp16-vfmaddcph-1a.c |   2 +-
 .../i386/avx512fp16vl-vfcmaddcph-1a.c |   4 +-
 .../i386/avx512fp16vl-vfmaddcph-1a.c  |   4 +-
 gcc/testsuite/gcc.target/i386/pr104976.c  |  13 +
 6 files changed, 196 insertions(+), 65 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr104976.c

diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
index 6f7af2f21d6..a9e18d38323 100644
--- a/gcc/config/i386/sse.md
+++ b/gcc/config/i386/sse.md
@@ -1059,6 +1059,18 @@ (define_mode_attr ssePSmode
(V4DF "V8SF") (V2DF "V4SF")
(V32HF "V16SF") (V16HF "V8SF") (V8HF "V4SF")])
 
+(define_mode_attr ssePSmodelower
+  [(V16SI "v16sf") (V8DF "v16sf")
+   (V16SF "v16sf") (V8DI "v16sf")
+   (V64QI "v16sf") (V32QI "v8sf") (V16QI "v4sf")
+   (V32HI "v16sf") (V16HI "v8sf") (V8HI "v4sf")
+   (V8SI "v8sf") (V4SI "v4sf")
+   (V4DI "v8sf") (V2DI "v4sf")
+   (V4TI "v16sf") (V2TI "v8sf") (V1TI "v4sf")
+   (V8SF "v8sf") (V4SF "v4sf")
+   (V4DF "v8sf") (V2DF "v4sf")
+   (V32HF "v16sf") (V16HF "v8sf") (V8HF "v4sf")])
+
 (define_mode_attr ssePSmode2
   [(V8DI "V8SF") (V4DI "V4SF")])
 
@@ -3617,6 +3629,9 @@ (define_insn_and_split "*avx_cmp3_ltint_not"
   operands[1] = force_reg (mode,
  gen_lowpart (mode, operands[1]));
   operands[2] = gen_lowpart (mode, operands[2]);
+
+  if (!MEM_P (operands[3]))
+operands[3] = force_reg (mode, operands[3]);
   operands[3] = lowpart_subreg (mode, operands[3], mode);
 })
 
@@ -6319,7 +6334,7 @@ (define_expand 
"_fmaddc__mask1"
(match_operand: 4 "register_operand")]
   "TARGET_AVX512FP16 && "
 {
-  rtx op0, op1;
+  rtx op0, op1, dest;
   if ()
 emit_insn (gen__fmaddc__mask (
   operands[0], operands[1], operands[2], operands[3],
@@ -6328,9 +6343,16 @@ (define_expand 
"_fmaddc__mask1"
 emit_insn (gen__fmaddc__mask (operands[0],
   operands[1], operands[2], operands[3], operands[4]));
 
-  op0 = lowpart_subreg (mode, operands[0], mode);
+  op0 = lowpart_subreg (mode,
+   force_reg (mode, operands[0]),
+   mode);
+  dest = gen_reg_rtx (mode);
+  if (!MEM_P (operands[1]))
+operands[1] = force_reg (mode, operands[1]);
   op1 = lowpart_subreg (mode, operands[1], mode);
-  emit_insn (gen__mask (op0, op0, op1, operands[4]));
+  emit_insn (gen__mask (dest, op0, op1, operands[4]));
+  emit_move_insn (operands[0],
+ lowpart_subreg (mode, dest, mode));
   DONE;
 })
 
@@ -6356,7 +6378,7 @@ (define_expand 
"_fcmaddc__mask1"
(match_operand: 4 "register_operand")]
   "TARGET_AVX512FP16 && "
 {
-  rtx op0, op1;
+  rtx op0, op1, dest;
   if ()
 emit_insn (gen__fcmaddc__mask (
   operands[0], operands[1], operands[2], operands[3],
@@ -6367,9 +6389,16 @@ (define_expand