[PATCH] asan_test.C: disable -Wstringop-overflow, PR/93058

2020-01-23 Thread slyfox.inbox.ru via gcc-patches
From: Sergei Trofimovich 

From: Sergei Trofimovich 

asan's test allocates 2 pages via pvalloc(kPageSize + 100)
and makes sure dereference of 'kPageSize + 101' does not
trigger asan checks.

glibc's and gcc's malloc-like attribute checkers trigger
a warning:
asan_test.cc:129:22: error: writing 1 byte into a region
of size 0 [-Werror=stringop-overflow=]

As there is no easy way to convey pvalloc()'s granularity
to gcc let's just disable the warning for this test.

* g++.dg/asan/asan_test.C: disable -Wstringop-overflow.
---
 gcc/testsuite/ChangeLog   | 4 
 gcc/testsuite/g++.dg/asan/asan_test.C | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 9a87cfe595b..847b73e7287 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2020-01-24  Sergei Trofimovich 
+
+   * g++.dg/asan/asan_test.C: disable -Wstringop-overflow.
+
 2020-01-22  Andrew Pinski  
 
* tree-ssa.exp: Set DEFAULT_VECTCFLAGS and DEFAULT_VECTCFLAGS.
diff --git a/gcc/testsuite/g++.dg/asan/asan_test.C 
b/gcc/testsuite/g++.dg/asan/asan_test.C
index f3f7626ef3b..8e18744c7c6 100644
--- a/gcc/testsuite/g++.dg/asan/asan_test.C
+++ b/gcc/testsuite/g++.dg/asan/asan_test.C
@@ -2,7 +2,7 @@
 // { dg-skip-if "" { *-*-* } { "*" } { "-O2" } }
 // { dg-skip-if "" { *-*-* } { "-flto" } { "" } }
 // { dg-additional-sources "asan_globals_test-wrapper.cc" }
-// { dg-options "-std=c++11 -fsanitize=address -fno-builtin -Wall -Werror -g 
-DASAN_UAR=0 -DASAN_HAS_EXCEPTIONS=1 -DASAN_HAS_BLACKLIST=0 
-DSANITIZER_USE_DEJAGNU_GTEST=1 -lasan -lpthread -ldl" }
+// { dg-options "-std=c++11 -fsanitize=address -fno-builtin -Wall -Werror 
-Wno-stringop-overflow -g -DASAN_UAR=0 -DASAN_HAS_EXCEPTIONS=1 
-DASAN_HAS_BLACKLIST=0 -DSANITIZER_USE_DEJAGNU_GTEST=1 -lasan -lpthread -ldl" }
 // { dg-additional-options "-DASAN_NEEDS_SEGV=1" { target { ! arm*-*-* } } }
 // { dg-additional-options "-DASAN_LOW_MEMORY=1 -DASAN_NEEDS_SEGV=0" { target 
arm*-*-* } }
 // { dg-additional-options "-DASAN_AVOID_EXPENSIVE_TESTS=1" { target { ! 
run_expensive_tests } } }
-- 
2.25.0



Re: [PATCH] i386: prefer vpermilpd over vpermpd [PR93395]

2020-01-23 Thread Uros Bizjak
On Thu, Jan 23, 2020 at 10:48 PM Jakub Jelinek  wrote:
>
> Hi!
>
> In Agner Fog's tables, vpermilp[sd] with immediates seem to be
> much faster than vpermpd with immediate, for a good reason,
> the former only permute something within the lanes and don't do anything
> intra-lane, while vpermpd can.  So, functionality-wise, vpermilpd
> is more efficient subset of vpermpd.  We use the same RTL for those
> though (and also for certain broadcast).
>
> Now, the problem was that the vpermpd pattern appeared first in sse.md,
> followed by the broadcast patterns, followed by the vpermilp[sd].
> Which means unless -mavx -mno-avx2, we'd emit vpermpd instead of the
> more efficient alternatives.
>
> The following patch reorders them, so that vpermpd comes last, if we
> can match a broadcast, we do, if we can match a vpermilp[sd] that is not a
> broadcast, we will, otherwise fall back (of course only if -mavx2) to
> vpermpd.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2020-01-23  Jakub Jelinek  
>
> PR target/93395
> * config/i386/sse.md (*avx_vperm_broadcast_v4sf,
> *avx_vperm_broadcast_,
> _vpermil,
> *_vpermilp):
> Move before avx2_perm/avx512f_perm.
>
> * gcc.target/i386/pr93395.c: New test.
> * gcc.target/i386/avx512vl-vpermilpdi-1.c: Remove xfail.

LGTM.

Thanks,
Uros.

> --- gcc/config/i386/sse.md.jj   2020-01-23 19:24:14.851423969 +0100
> +++ gcc/config/i386/sse.md  2020-01-23 19:41:58.729091766 +0100
> @@ -19875,6 +19875,164 @@ (define_insn "_permvar (set_attr "prefix" "")
> (set_attr "mode" "")])
>
> +;; Recognize broadcast as a vec_select as produced by builtin_vec_perm.
> +;; If it so happens that the input is in memory, use vbroadcast.
> +;; Otherwise use vpermilp (and in the case of 256-bit modes, vperm2f128).
> +(define_insn "*avx_vperm_broadcast_v4sf"
> +  [(set (match_operand:V4SF 0 "register_operand" "=v,v,v")
> +   (vec_select:V4SF
> + (match_operand:V4SF 1 "nonimmediate_operand" "m,o,v")
> + (match_parallel 2 "avx_vbroadcast_operand"
> +   [(match_operand 3 "const_int_operand" "C,n,n")])))]
> +  "TARGET_AVX"
> +{
> +  int elt = INTVAL (operands[3]);
> +  switch (which_alternative)
> +{
> +case 0:
> +case 1:
> +  operands[1] = adjust_address_nv (operands[1], SFmode, elt * 4);
> +  return "vbroadcastss\t{%1, %0|%0, %k1}";
> +case 2:
> +  operands[2] = GEN_INT (elt * 0x55);
> +  return "vpermilps\t{%2, %1, %0|%0, %1, %2}";
> +default:
> +  gcc_unreachable ();
> +}
> +}
> +  [(set_attr "type" "ssemov,ssemov,sselog1")
> +   (set_attr "prefix_extra" "1")
> +   (set_attr "length_immediate" "0,0,1")
> +   (set_attr "prefix" "maybe_evex")
> +   (set_attr "mode" "SF,SF,V4SF")])
> +
> +(define_insn_and_split "*avx_vperm_broadcast_"
> +  [(set (match_operand:VF_256 0 "register_operand" "=v,v,v")
> +   (vec_select:VF_256
> + (match_operand:VF_256 1 "nonimmediate_operand" "m,o,?v")
> + (match_parallel 2 "avx_vbroadcast_operand"
> +   [(match_operand 3 "const_int_operand" "C,n,n")])))]
> +  "TARGET_AVX"
> +  "#"
> +  "&& reload_completed && (mode != V4DFmode || !TARGET_AVX2)"
> +  [(set (match_dup 0) (vec_duplicate:VF_256 (match_dup 1)))]
> +{
> +  rtx op0 = operands[0], op1 = operands[1];
> +  int elt = INTVAL (operands[3]);
> +
> +  if (REG_P (op1))
> +{
> +  int mask;
> +
> +  if (TARGET_AVX2 && elt == 0)
> +   {
> + emit_insn (gen_vec_dup (op0, gen_lowpart (mode,
> + op1)));
> + DONE;
> +   }
> +
> +  /* Shuffle element we care about into all elements of the 128-bit lane.
> +The other lane gets shuffled too, but we don't care.  */
> +  if (mode == V4DFmode)
> +   mask = (elt & 1 ? 15 : 0);
> +  else
> +   mask = (elt & 3) * 0x55;
> +  emit_insn (gen_avx_vpermil (op0, op1, GEN_INT (mask)));
> +
> +  /* Shuffle the lane we care about into both lanes of the dest.  */
> +  mask = (elt / ( / 2)) * 0x11;
> +  if (EXT_REX_SSE_REG_P (op0))
> +   {
> + /* There is no EVEX VPERM2F128, but we can use either VBROADCASTSS
> +or VSHUFF128.  */
> + gcc_assert (mode == V8SFmode);
> + if ((mask & 1) == 0)
> +   emit_insn (gen_avx2_vec_dupv8sf (op0,
> +gen_lowpart (V4SFmode, op0)));
> + else
> +   emit_insn (gen_avx512vl_shuf_f32x4_1 (op0, op0, op0,
> + GEN_INT (4), GEN_INT (5),
> + GEN_INT (6), GEN_INT (7),
> + GEN_INT (12), GEN_INT (13),
> + GEN_INT (14), GEN_INT 
> (15)));
> + DONE;
> +   }
> +
> +  emit_insn (gen_avx_vperm2f1283 (op0, op0, op0, GEN_INT (mask)));
> +  

Re: [PR47785] COLLECT_AS_OPTIONS

2020-01-23 Thread Prathamesh Kulkarni
On Mon, 20 Jan 2020 at 15:44, Richard Biener  wrote:
>
> On Wed, Jan 8, 2020 at 11:20 AM Prathamesh Kulkarni
>  wrote:
> >
> > On Tue, 5 Nov 2019 at 17:38, Richard Biener  
> > wrote:
> > >
> > > On Tue, Nov 5, 2019 at 12:17 AM Kugan Vivekanandarajah
> > >  wrote:
> > > >
> > > > Hi,
> > > > Thanks for the review.
> > > >
> > > > On Tue, 5 Nov 2019 at 03:57, H.J. Lu  wrote:
> > > > >
> > > > > On Sun, Nov 3, 2019 at 6:45 PM Kugan Vivekanandarajah
> > > > >  wrote:
> > > > > >
> > > > > > Thanks for the reviews.
> > > > > >
> > > > > >
> > > > > > On Sat, 2 Nov 2019 at 02:49, H.J. Lu  wrote:
> > > > > > >
> > > > > > > On Thu, Oct 31, 2019 at 6:33 PM Kugan Vivekanandarajah
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > On Wed, 30 Oct 2019 at 03:11, H.J. Lu  
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > On Sun, Oct 27, 2019 at 6:33 PM Kugan Vivekanandarajah
> > > > > > > > >  wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Richard,
> > > > > > > > > >
> > > > > > > > > > Thanks for the review.
> > > > > > > > > >
> > > > > > > > > > On Wed, 23 Oct 2019 at 23:07, Richard Biener 
> > > > > > > > > >  wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Mon, Oct 21, 2019 at 10:04 AM Kugan Vivekanandarajah
> > > > > > > > > > >  wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Hi Richard,
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks for the pointers.
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > On Fri, 11 Oct 2019 at 22:33, Richard Biener 
> > > > > > > > > > > >  wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Fri, Oct 11, 2019 at 6:15 AM Kugan Vivekanandarajah
> > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Hi Richard,
> > > > > > > > > > > > > > Thanks for the review.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Wed, 2 Oct 2019 at 20:41, Richard Biener 
> > > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > On Wed, Oct 2, 2019 at 10:39 AM Kugan 
> > > > > > > > > > > > > > > Vivekanandarajah
> > > > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Hi,
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > As mentioned in the PR, attached patch adds 
> > > > > > > > > > > > > > > > COLLECT_AS_OPTIONS for
> > > > > > > > > > > > > > > > passing assembler options specified with -Wa, 
> > > > > > > > > > > > > > > > to the link-time driver.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > The proposed solution only works for uniform 
> > > > > > > > > > > > > > > > -Wa options across all
> > > > > > > > > > > > > > > > TUs. As mentioned by Richard Biener, supporting 
> > > > > > > > > > > > > > > > non-uniform -Wa flags
> > > > > > > > > > > > > > > > would require either adjusting partitioning 
> > > > > > > > > > > > > > > > according to flags or
> > > > > > > > > > > > > > > > emitting multiple object files  from a single 
> > > > > > > > > > > > > > > > LTRANS CU. We could
> > > > > > > > > > > > > > > > consider this as a follow up.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Bootstrapped and regression tests on  
> > > > > > > > > > > > > > > > arm-linux-gcc. Is this OK for trunk?
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > While it works for your simple cases it is 
> > > > > > > > > > > > > > > unlikely to work in practice since
> > > > > > > > > > > > > > > your implementation needs the assembler options 
> > > > > > > > > > > > > > > be present at the link
> > > > > > > > > > > > > > > command line.  I agree that this might be the way 
> > > > > > > > > > > > > > > for people to go when
> > > > > > > > > > > > > > > they face the issue but then it needs to be 
> > > > > > > > > > > > > > > documented somewhere
> > > > > > > > > > > > > > > in the manual.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > That is, with COLLECT_AS_OPTION (why singular?  
> > > > > > > > > > > > > > > I'd expected
> > > > > > > > > > > > > > > COLLECT_AS_OPTIONS) available to cc1 we could 
> > > > > > > > > > > > > > > stream this string
> > > > > > > > > > > > > > > to lto_options and re-materialize it at link time 
> > > > > > > > > > > > > > > (and diagnose mismatches
> > > > > > > > > > > > > > > even if we like).
> > > > > > > > > > > > > > OK. I will try to implement this. So the idea is if 
> > > > > > > > > > > > > > we provide
> > > > > > > > > > > > > > -Wa,options as part of the lto compile, this should 
> > > > > > > > > > > > > > be available
> > > > > > > > > > > > > > during link time. Like in:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > arm-linux-gnueabihf-gcc -march=armv7-a -mthumb -O2 
> > > > > > > > > > > > > > -flto
> > > > > > > > > > > > > > -Wa,-mimplicit-it=always,-mthumb -c test.c
> > > > > > > > > > > > > > arm-linux-gnueabihf-gcc  -flto  test.o
> > > > > > > > > > > > > >

[PATCH] Suppress deprecation warnings in tbb effective target check

2020-01-23 Thread Thomas Rodgers
From cfd3c2e2a49dd3e29b42baa0f22feffd4b346231 Mon Sep 17 00:00:00 2001
From: Thomas Rodgers 
Date: Thu, 23 Jan 2020 21:54:44 -0800
Subject: [PATCH] Suppress deprecation warnings in tbb effective target check

TBB 2020 added deprecation warnings which produced output not expected by
check_effective_target_tbb-backend
---
 libstdc++-v3/testsuite/lib/libstdc++.exp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libstdc++-v3/testsuite/lib/libstdc++.exp b/libstdc++-v3/testsuite/lib/libstdc++.exp
index 94f3fdb2bc8..f451719bdd4 100644
--- a/libstdc++-v3/testsuite/lib/libstdc++.exp
+++ b/libstdc++-v3/testsuite/lib/libstdc++.exp
@@ -1597,7 +1597,8 @@ proc check_effective_target_tbb-backend { } {
 puts $f "}"
 close $f
 
-set lines [v3_target_compile $src $exe executable "additional_flags=-std=c++17 additional_flags=-ltbb"]
+set lines [v3_target_compile $src $exe executable "additional_flags=-std=c++17 additional_flags=-ltbb
+   additional_flags=-DTBB_SUPPRESS_DEPRECATED_MESSAGES=1"]
 file delete $src
 
 if [string match "" $lines] {
-- 
2.21.1



[committed] analyzer: avoid relying on system in testsuite (PR 93367)

2020-01-23 Thread David Malcolm
PR analyzer/93367 reports a testsuite failure in abort.c on
hppa64-hp-hpux11.11 when detecting if the analyzer "knows" that the
condition holds after the assert.

The root cause is that the assertion failure function in that
configuration's  is not marked with
__attribute__ ((__noreturn__)).

This patch reworks the test to avoid  in favor of a custom
implementation of assert, so that the test demonstrates the idea without
relying on properties of .

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to master as r10-6195-ga0b935ac66bc9804b0864151e5f1bfde5ac1ddeb.

gcc/testsuite/ChangeLog:
PR analyzer/93367
* gcc.dg/analyzer/abort.c: Remove include of .
Replace use of assert with a custom assertion implementation.
---
 gcc/testsuite/gcc.dg/analyzer/abort.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/analyzer/abort.c 
b/gcc/testsuite/gcc.dg/analyzer/abort.c
index ea1756e47cb..9497ae30b46 100644
--- a/gcc/testsuite/gcc.dg/analyzer/abort.c
+++ b/gcc/testsuite/gcc.dg/analyzer/abort.c
@@ -1,4 +1,3 @@
-#include 
 #include 
 #include 
 #include "analyzer-decls.h"
@@ -62,11 +61,20 @@ void test_4 (void *ptr)
 
 /**/
 
+/* Verify that we discover conditions from assertions if the assert macro
+   isn't disabled, and that it has its failure-handler labelled with
+   __attribute__ ((__noreturn__)).
+   This attribute isn't present for all implementations of , so
+   we have to test the idea using our own assert macro.  */
+
+extern void my_assert_fail (const char *expr, const char *file, int line)
+  __attribute__ ((__noreturn__));
+
+#define MY_ASSERT(EXPR) \
+  do { if (!(EXPR)) my_assert_fail (#EXPR, __FILE__, __LINE__); } while (0)
+
 void test_5 (int i)
 {
-  assert (i < 10);
-
-  /* We have not defined NDEBUG, so this will call __assert_fail if
- i >= 10, which is labelled with __attribute__ ((__noreturn__)).  */
+  MY_ASSERT (i < 10);
   __analyzer_eval (i < 10); /* { dg-warning "TRUE" } */
 }
-- 
2.21.0



Re: [PATCH] Relax invalidation of TOP N counters in PGO.

2020-01-23 Thread Jan Hubicka
Hi,
it seems that there is additional bug in merging which caused the extra
0 counts.
The attached patch imprves the stats on gcc dir of profiledbootstrap:

stats for indirect_call:
  total: 9876
  invalid: 705
  tracked values:
0 values: 7189 times (72.79%)
1 values: 1890 times (19.14%)
2 values:   50 times (0.51%)
3 values:   20 times (0.20%)
4 values:   22 times (0.22%)

stats for topn:
  total: 1527
  invalid: 150
  tracked values:
0 values: 1193 times (78.13%)
1 values:  150 times (9.82%)
2 values:   16 times (1.05%)
3 values:9 times (0.59%)
4 values:9 times (0.59%)

to:

stats for indirect_call:
  total: 9876
  invalid: 769
  tracked values:
0 values: 6289 times (63.68%)
1 values: 2383 times (24.13%)
2 values:  293 times (2.97%)
3 values:  142 times (1.44%)
4 values:0 times (0.00%)

stats for topn:
  total: 1527
  invalid: 205
  tracked values:
0 values: 1038 times (67.98%)
1 values:  166 times (10.87%)
2 values:   65 times (4.26%)
3 values:   51 times (3.34%)
4 values:2 times (0.13%)

I plan to commit it tomorrow.
It bootstraped x86_64-linux, regtesting is running.
Stll the number of invalidated counters outnumbers those sucesfully
tracked which are not trivial (0 or 1 val).

libgcc/ChangeLog:

2020-01-24  Jan Hubicka  

* libgcov-merge.c (merge_topn_values_set): Fix merging.

diff --git a/libgcc/libgcov-merge.c b/libgcc/libgcov-merge.c
index b658aec46c6..19b8ee72ae9 100644
--- a/libgcc/libgcov-merge.c
+++ b/libgcc/libgcov-merge.c
@@ -112,9 +112,11 @@ merge_topn_values_set (gcov_type *counters)
   for (unsigned i = 0; i < GCOV_TOPN_VALUES; i++)
 {
   if (read_counters[2 * i + 1] == 0)
-   return;
+   continue;
 
   unsigned j;
+  int slot = -1;
+
   for (j = 0; j < GCOV_TOPN_VALUES; j++)
{
  if (counters[2 * j] == read_counters[2 * i])
@@ -123,18 +125,23 @@ merge_topn_values_set (gcov_type *counters)
  break;
}
  else if (counters[2 * j + 1] == 0)
-   {
- counters[2 * j] += read_counters[2 * i];
- counters[2 * j + 1] += read_counters[2 * i + 1];
- break;
-   }
+   slot = j;
}
 
-  /* We haven't found a slot, bail out.  */
   if (j == GCOV_TOPN_VALUES)
{
- counters[1] = -1;
- return;
+ if (slot > 0)
+   {
+ /* If we found empty slot, add the value.  */
+ counters[2 * slot] = read_counters[2 * i];
+ counters[2 * slot + 1] = read_counters[2 * i + 1];
+   }
+ else
+   {
+ /* We haven't found a slot, bail out.  */
+ counters[1] = -1;
+ return;
+   }
}
 }
 }


Re: libgo patch committed: Update to Go1.14beta1

2020-01-23 Thread Ian Lance Taylor
On Thu, Jan 23, 2020 at 11:32 AM Maciej W. Rozycki  wrote:
>
> On Tue, 21 Jan 2020, Ian Lance Taylor wrote:
>
> > I've committed a patch to update libgo to Go 1.14beta1.  As usual with
> > these updates the patch is far too large to include in this e-mail
> > message.  I've included the diffs for gccgo-specific files.
>
>  It seems to have broken the `riscv64-linux-gnu' target:
>
> cpugen.go:2:7: error: redefinition of 'CacheLinePadSize'
> 2 | const CacheLinePadSize = 64
>   |   ^
> .../libgo/go/internal/cpu/cpu_riscv64.go:7:7: note: previous definition of 
> 'CacheLinePadSize' was here
> 7 | const CacheLinePadSize = 32
>   |   ^
> make[4]: *** [Makefile:2830: internal/cpu.lo] Error 1
> make[4]: Leaving directory '.../riscv64-linux-gnu/libgo'

Thanks.  Fixed like so.  Committed to mainline.

Ian
b83c78fe8fd12ce6c14fe2ca234edbbdac22cd79
diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE
index a8ba3afe86e..fc1dbaca8b9 100644
--- a/gcc/go/gofrontend/MERGE
+++ b/gcc/go/gofrontend/MERGE
@@ -1,4 +1,4 @@
-7d3081ce69dda123d77e35e8b9d282e40e9465e2
+10a8dbfc9945c672d59af8edb9790e2019cdeb27
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
diff --git a/libgo/go/internal/cpu/cpu_riscv64.go 
b/libgo/go/internal/cpu/cpu_riscv64.go
index c49cab79fdc..d920fcf7a6d 100644
--- a/libgo/go/internal/cpu/cpu_riscv64.go
+++ b/libgo/go/internal/cpu/cpu_riscv64.go
@@ -3,5 +3,3 @@
 // license that can be found in the LICENSE file.
 
 package cpu
-
-const CacheLinePadSize = 32


Re: drop -aux{dir,base}, revamp -dump{dir,base}

2020-01-23 Thread Alexandre Oliva
On Jan 22, 2020, Richard Biener  wrote:

> I think it's fine to make that change now.

FTR, the combined patch, to be installed in GCC 11, is commit
f798a915a2a00ff7921644d0e08cb88e7db581a2, in
refs/users/aoliva/heads/aux-dump-revamp

I'm not reposting the monster patch right now.

-- 
Alexandre Oliva, freedom fighter   he/him   https://FSFLA.org/blogs/lxo
Free Software Evangelist   Stallman was right, but he's left :(
GNU Toolchain EngineerFSMatrix: It was he who freed the first of us
FSF & FSFLA board memberThe Savior shall return (true);


Re: [patch, testsuite] Add -fdelete-null-pointer-checks to some C++ testcases

2020-01-23 Thread Jeff Law
On Thu, 2020-01-23 at 14:59 -0700, Sandra Loosemore wrote:
> On 1/23/20 1:17 PM, Jeff Law wrote:
> > On Tue, 2020-01-21 at 15:00 -0700, Sandra Loosemore wrote:
> > > In doing some nios2-elf testing, I ran into a bunch of failures in
> > > constexpr-related tests in the C++ testsuite.  This target defaults to
> > > -fno-delete-null-pointer-checks at the request of Altera/Intel, in order
> > > to support some of their BSPs where 0 is a legitimate memory address.
> > > Some other bare-metal targets also default to
> > > -fno-delete-null-pointer-checks.
> > > 
> > > This patch makes the dependence of these tests on
> > > -fdelete-null-pointer-checks explicit.  I've previously fixed some other
> > > tests that failed on nios2-elf in the same way so this is borderline
> > > obvious, but it's a little troubling to me that the correct semantics of
> > > some of these testcases seems to depend on what we document in the
> > > manual as an optimization option.  :-S  Maybe there is some other bug 
> > > here?
> > > 
> > > Anyway, if nobody has any objections or better ideas, I will go ahead
> > > and commit this in a few days.
> > It'd be nice to know why that flag matters for constexpr.  But I've got
> > no problem with the change itself.
> 
> I haven't looked at all of the failing tests in detail, but the ones I 
> did peek at all seemed to be related to constant-folding pointer 
> comparisons against null.
> 
> The thing that nags at me is that e.g. in g++.dg/cpp0x/constexpr-odr1.C 
> we have:
> 
> template struct A {
>static const bool x;
>static_assert(, ""); // odr-uses A<...>::x
> };
> 
> int g;
> 
> template
> const bool A::x = (g = 42, false);
> 
> void f(A<0>) {}// A<0> must be complete, so is instantiated
> int main()
> {
>if (g != 42)
>  __builtin_abort ();
> }
> 
> Whether or not this is valid code depends on whether " != 0" can be 
> constant-folded.  Without -fdelete-null-pointer-checks, g++ says:
> 
> /path/to/g++.dg/cpp0x/constexpr-odr1.C:
> In instantiation of 'struct A<0>':
> /path/to/g++.dg/cpp0x/constexpr-odr1.C:14:12:
> required from here
> /path/to/g++.dg/cpp0x/constexpr-odr1.C:6:17:
> error: non-constant condition for static assertion
> /path/to/g++.dg/cpp0x/constexpr-odr1.C:12:12:
> error: '((& A<0>::x) != 0)' is not a constant expression
> 
> So is -fdelete-null-pointer-checks really an optimization option, or a 
> language semantics option?
It's an optimization option, pure and simple.

But determining if an expression is a constant sometimes requires
folding (as in this case).  Normally folding will assume that the
address of an object can't be zero.  -fno-delete-pointer-checks turns
off that assumption (as it should).



> 
> BTW, here's Altera's original description of the over-eager optimization 
> problems they were trying to work around:
Yea.  Using -fno-delete-pointer-checks is the right thing for them to
do in their case.  


> https://gcc.gnu.org/ml/gcc/2015-02/msg00163.html
> 



Re: [PATCH] doc: clarify the situation with pointer arithmetic

2020-01-23 Thread Uecker, Martin
Am Donnerstag, den 23.01.2020, 14:18 +0100 schrieb Richard Biener:
> On Wed, Jan 22, 2020 at 12:40 PM Martin Sebor  wrote:
> > 
> > On 1/22/20 8:32 AM, Richard Biener wrote:
> > > On Tue, 21 Jan 2020, Alexander Monakov wrote:
> > > 
> > > > On Tue, 21 Jan 2020, Richard Biener wrote:
> > > > 
> > > > > Fourth.  That PNVI (I assume it's the whole pointer-provenance stuff)
> > > > > wants to get the "best" of both which can never be done since a 
> > > > > compiler
> > > > > needs to have a way to be conservative - in this area it's conflicting
> > > > > conservative treatment which is impossible.
> > > > 
> > > > This paragraph is unclear, I don't immediately see what the conflicting 
> > > > goals
> > > > are. The rest is clear enough given the previous discussions I saw.
> > > > 
> > > > Did you mean the restriction that you cannot do arithmetic involving two
> > > > integers based on pointers, get a value corresponding to one of them,
> > > > cast it back and get a pointer suitable for accessing either of two
> > > > originally pointed-to objects? I don't see that as a conflict because
> > > > it places a restriction on users, not the compiler.
> > > 
> > > As far as I remember the discussions PNVI requires to track
> > > provenance for correctness, you may not miss or attach wrong provenance
> > > to a pointer and there's only "single" provenance, not "many"
> > > (aka, may point to A and B).  I don't see how you can ever implement that.

I have not idea how you came to that conclusion. PNVI is perfectly
compatible with a naive compiler who does not track provenance at
all as well as an abstract machine that actually carries run-time
provenance around with each pointer and checks every operation. 
It was designed specifically to allow both cases and everything
in between (especially compilers who track provenance during
compile time but the programs then do not track provenance at
run-time).

You may be confused by the abstract formulation that indeed
assigns a single provenance to each pointer. A compiler would
track its *knowledge about provenance*, which would be a set
of possible targets.

> > The PVNI variant preferred by the object model group is referred
> > to as "PNVI-ae-udi" which stands for "PNVI exposed-address user-
> > disambiguation."  (The PNVI part stands for "Provenance Not Via
> > Integers.)  This base PVNI model basically prohibits provenance
> > tracking via integers, making it possible for programs to derive
> > pointers to unrelated objects via casts between pointers and
> > integers (and modifying the integer in between the casts).  This
> > is considered a new restriction on implementations because
> > the standard doesn't permit it (as you said upthread, all it
> > specifies is that a pointer is equal to one obtained by casting
> > the original to a intptr_t and back).

This is not entirely clear what the standard means. 7.20.1.4.

In my opinion, converting the same integer back should yield
a valid pointer where "same" is defined in the usual sense
(i.e. via mathematical identity and not via provenance).

> > The -ae-udi variant limits this restriction on implementations
> > to escaped pointers and provides a means for users/programs to
> > disambiguate between pointers to adjacent objects (i.e., a past
> > the end pointer and one to the beginning of the object stored
> > there).  The latest proposal is in N2362, with an overview in
> > N2378).  At the last WG14 meeting there was broad discomfort
> > with adopting the proposal for C2X because of the absence of
> > implementation experience and concerns raised by implementers.
> > The guidance to the study group was to target a separate technical
> > specification for the proposal and allow time for implementation
> > experience.  If the feedback from implementers is positive
> > (whatever that might mean) WG14 said it would consider adopting
> > the model for a revision of C after C2X.
> >
> > Overall, the impact of the proposals as well as their goal is to
> > constrain implementations to the (presumed) benefit of programs
> > in terms of expressiveness.  There are numerous examples of code
> > that's currently treated as undefined by one or more compilers
> > (as a consequence of optimizations) that the model makes valid.
> > I'm not aware of any optimization opportunities the proposal
> > might open up in GCC.
> 
> Well, PNVI limits optimization opportunities of GCC which currently
> _does_ track provenance through integers and thus only allows
> a very limited set(*) of "unrelated" pointers to appear here (documented
> is that none are, implementation details differ from version to version).
> 
> There are no optimization "opportunities" by making pointer <-> integer
> conversions lose information.

You are right: It is meant to constrain optimizations.

The reasoning behind this that currently all compilers behave
inconsistently and this is not terrible useful to anybody.

At the same time, there does not appear to
be any 

[committed] [PR translation/90162] Fix diagnostic to better follow guidelines

2020-01-23 Thread Jeff Law

Not a regression, but only affects a diagnostic on the H8 port.

As pointed out in the BZ, we should not be emitting an exclamation
point in the diagnostic and the capitalization was incorrect.  Fixed by
the attached patch which I've committed to the trunk.
Jeff
commit dfa075d00d396f554b4aca183c3ca685fbb73bbd
Author: Jeff Law 
Date:   Thu Jan 23 16:03:27 2020 -0700

Fix diagnostic text on H8.

* config/h8300/h8300.c (h8300_option_override): Fix diagnostic text.

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index c00deeed142..023e9c398ea 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,7 @@
+2020-01-23  Jeff Law  
+
+   * config/h8300/h8300.c (h8300_option_override): Fix diagnostic text.
+
 2020-01-23  Mikael Tillenius  
 
* config/h8300/h8300.h (FUNCTION_PROFILER): Fix emission of
diff --git a/gcc/config/h8300/h8300.c b/gcc/config/h8300/h8300.c
index 6c84b0abea7..ffbfa9eaaa9 100644
--- a/gcc/config/h8300/h8300.c
+++ b/gcc/config/h8300/h8300.c
@@ -378,8 +378,8 @@ h8300_option_override (void)
 
  if ((!TARGET_H8300S  &&  TARGET_NEXR) && (!TARGET_H8300SX && TARGET_NEXR))
{
-  warning (OPT_mno_exr, "%<-mno-exr%> valid only with %<-ms%> or "
-  "%<-msx%> - Option ignored!");
+  warning (OPT_mno_exr, "%<-mno-exr%> is valid only with %<-ms%> or "
+  "%<-msx%> - option ignored");
}
 
 #ifdef H8300_LINUX 


[committed] [PR target/92269] Fix label for profiling on H8

2020-01-23 Thread Jeff Law


As mentioned in the PR, there's a bug in the H8 profiling support which
results in the wrong label name being emitted.  Specifically the label
is missing a ".".

While we're in stage4, this affects just the H8 and only when profiling
is enabled, so I went ahead and committed the fix to the trunk.





[COMMITTED] libstdc++: Simplify makefile rule for largefile-config.h (PR91947)

2020-01-23 Thread Jonathan Wakely
The previous rule could leave an incomplete file if the build was
interrupted, which would then not be remade if make was run again.

This makes the rule more robust by writing to a temporary file and only
moving it into place as the final step. It also simplifies the rule so
that only the essential macro definitions are written to the file, not
the explanatory comments and commented out #undef lines.

Also, the macro for enabling LFS on Mac OS X 10.5 is now set
unconditionally, which is a bug fix from upstream autoconf.

PR libstdc++/91947
* include/Makefile.am (${host_builddir}/largefile-config.h): Simplify
rule.
* include/Makefile.in: Regenerate.

Tested x86_64-linux and powerpc64le-linux. Committed to trunk.

I'll also backport this to the gcc-9 branch.


commit 04681fca936b5bca1dd374dfb0fbca7ccb028994
Author: Jonathan Wakely 
Date:   Thu Jan 23 14:02:32 2020 +

libstdc++: Simplify makefile rule for largefile-config.h (PR91947)

The previous rule could leave an incomplete file if the build was
interrupted, which would then not be remade if make was run again.

This makes the rule more robust by writing to a temporary file and only
moving it into place as the final step. It also simplifies the rule so
that only the essential macro definitions are written to the file, not
the explanatory comments and commented out #undef lines.

Also, the macro for enabling LFS on Mac OS X 10.5 is now set
unconditionally, which is a bug fix from upstream autoconf.

PR libstdc++/91947
* include/Makefile.am (${host_builddir}/largefile-config.h): 
Simplify
rule.
* include/Makefile.in: Regenerate.

diff --git a/libstdc++-v3/include/Makefile.am b/libstdc++-v3/include/Makefile.am
index ad4404793be..89835759069 100644
--- a/libstdc++-v3/include/Makefile.am
+++ b/libstdc++-v3/include/Makefile.am
@@ -1245,22 +1245,12 @@ stamp-float128:
 endif
 
 # This header is not installed, it's only used to build libstdc++ itself.
-${host_builddir}/largefile-config.h: ${CONFIG_HEADER} \
-${glibcxx_srcdir}/include/bits/c++config
-   @echo '#if defined _GLIBCXX_CXX_CONFIG_H || defined _FEATURES_H' > $@
-   @echo '# error "This file must be included before any others"' >> $@
-   @echo '#endif' >> $@
-   @echo >> $@
-   @echo '/* Enable large inode numbers on Mac OS X 10.5.  */' >> $@
-   @echo '#ifndef _DARWIN_USE_64_BIT_INODE' >> $@
-   @echo '# define _DARWIN_USE_64_BIT_INODE 1' >> $@
-   @echo '#endif' >> $@
-   @echo >> $@
-   @echo '/* Number of bits in a file offset, on hosts where this is 
settable. */' >> $@
-   @grep '_FILE_OFFSET_BITS' ${CONFIG_HEADER} >> $@
-   @echo >> $@
-   @echo '/* Define for large files, on AIX-style hosts. */' >> $@
-   @grep '_LARGE_FILES' ${CONFIG_HEADER} >> $@
+${host_builddir}/largefile-config.h: ${CONFIG_HEADER}
+   @rm -f $@.tmp
+   @-grep 'define _DARWIN_USE_64_BIT_INODE' ${CONFIG_HEADER} >> $@.tmp
+   @-grep 'define _FILE_OFFSET_BITS' ${CONFIG_HEADER} >> $@.tmp
+   @-grep 'define _LARGE_FILES' ${CONFIG_HEADER} >> $@.tmp
+   @mv $@.tmp $@
 
 # NB: The non-empty default ldbl_compat works around an AIX sed
 # oddity, see libstdc++/31957 for details.


[PATCH] libstdc++: Fix conformance issues in (PR92895)

2020-01-23 Thread Jonathan Wakely
Fix synchronization issues in . Replace shared_ptr with
_Stop_state_ref and a reference count embedded in the shared state.
Replace std::mutex with spinlock using one bit of a std::atomic<> that
also tracks whether a stop request has been made and how many
stop_source objects share ownership of the state.

The synchronization with callbacks being destroyed is based on the
implementation by Lewis Baker and Nico Josuttis. It allows the
callback being destroyed to detect whether it's currently running, and
if so whether on the current thread or a different one.

Tom, please take a look and give a review. As discussed, the
binary_semaphore is temporary, until we have the real thing.


commit 011476e4282eca13d19ca18f21bc010f40134ac5
Author: Jonathan Wakely 
Date:   Thu Jan 23 16:46:17 2020 +

libstdc++: Fix conformance issues in  (PR92895)

Fix synchronization issues in . Replace shared_ptr with
_Stop_state_ref and a reference count embedded in the shared state.
Replace std::mutex with spinlock using one bit of a std::atomic<> that
also tracks whether a stop request has been made and how many
stop_source objects share ownership of the state.

PR libstdc++/92895
* include/std/stop_token (stop_token::stop_possible()): Call new
_M_stop_possible() function.
(stop_token::stop_requested()): Do not use stop_possible().
(stop_token::binary_semaphore): New class, as temporary stand-in for
std::binary_semaphore.
(stop_token::_Stop_cb::_M_callback): Add noexcept to type.
(stop_token::_Stop_cb::_M_destroyed, stop_token::_Stop_cb::_M_done):
New data members for symchronization with stop_callback destruction.
(stop_token::_Stop_cb::_Stop_cb): Make non-template.
(stop_token::_Stop_cb::_M_linked, stop_token::_Stop_cb::_S_execute):
Remove.
(stop_token::_Stop_cb::_M_run): New member function.
(stop_token::_Stop_state::_M_stopped, stop_token::_Stop_state::_M_mtx):
Remove.
(stop_token::_Stop_state::_M_owners): New data member to track
reference count for ownership.
(stop_token::_Stop_state::_M_value): New data member combining a
spinlock, the stop requested flag, and the reference count for
associated stop_source objects.
(stop_token::_Stop_state::_M_requester): New data member for
synchronization with stop_callback destruction.
(stop_token::_Stop_state::_M_stop_possible()): New member function.
(stop_token::_Stop_state::_M_stop_requested()): Inspect relevant bit
of _M_value.
(stop_token::_Stop_state::_M_add_owner)
(stop_token::_Stop_state::_M_release_ownership)
(stop_token::_Stop_state::_M_add_ssrc)
(stop_token::_Stop_state::_M_sub_ssrc): New member functions for
updating reference counts.
(stop_token::_Stop_state::_M_lock, stop_token::_Stop_state::_M_unlock)
(stop_token::_Stop_state::_M_lock, stop_token::_Stop_state::_M_unlock)
(stop_token::_Stop_state::_M_try_lock)
(stop_token::_Stop_state::_M_try_lock_and_stop)
(stop_token::_Stop_state::_M_do_try_lock): New member functions for
managing spinlock.
(stop_token::_Stop_state::_M_request_stop): Use atomic operations to
read and update state. Release lock while running callbacks. Use new
data members to synchronize with callback destruction.
(stop_token::_Stop_state::_M_remove_callback): Likewise.
(stop_token::_Stop_state::_M_register_callback): Use atomic operations
to read and update state.
(stop_token::_Stop_state_ref): Handle type to manage _Stop_state,
replacing shared_ptr.
(stop_source::stop_source(const stop_source&)): Update reference count.
(stop_source::operator=(const stop_source&)): Likewise.
(stop_source::~stop_source()): Likewise.
(stop_source::stop_source(stop_source&&)): Define as defaulted.
(stop_source::operator=(stop_source&&)): Establish postcondition on
parameter.
(stop_callback): Enforce preconditions on template parameter. Replace
base class with data member of new _Cb_impl type.
(stop_callback::stop_callback(const stop_token&, Cb&&))
(stop_callback::stop_callback(stop_token&&, Cb&&)): Fix TOCTTOU race.
(stop_callback::_Cb_impl): New type wrapping _Callback member and
defining the _S_execute member function.
* testsuite/30_threads/stop_token/stop_callback/deadlock-mt.cc: New
test.
* testsuite/30_threads/stop_token/stop_callback/deadlock.cc: New test.
* testsuite/30_threads/stop_token/stop_callback/destroy.cc: New 

[PATCH] analyzer: fix setjmp-detection and support sigsetjmp

2020-01-23 Thread David Malcolm
On Wed, 2020-01-22 at 20:40 +0100, Jakub Jelinek wrote:
> On Wed, Jan 22, 2020 at 02:35:13PM -0500, David Malcolm wrote:
> > PR analyzer/93316 reports various testsuite failures where I
> > accidentally relied on properties of x86_64-pc-linux-gnu.
> > 
> > The following patch fixes them on sparc-sun-solaris2.11 (gcc211 in
> > the
> > GCC compile farm), and, I hope, the other configurations showing
> > failures.
> > 
> > There may still be other failures for pattern-test-2.c, which I'm
> > tracking separately as PR analyzer/93291.
> > 
> > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu;
> > tested on stage 1 on sparc-sun-solaris2.11.
> > 
> > gcc/analyzer/ChangeLog:
> > PR analyzer/93316
> > * analyzer.cc (is_setjmp_call_p): Check for "setjmp" as well as
> > "_setjmp".
> 
> Please see calls.c (special_function_p), you should treat certainly
> also sigsetjmp as a setjmp call, and similarly to special_function_p,
> skip over _ or __ prefixes before the setjmp or sigsetjmp name.
> Similarly for longjmp/siglongjmp.
> 
>   Jakub

Thanks.

This patch removes the hack in is_setjmp_call_p of looking for
"setjmp" and "_setjmp", replacing it with some logic adapted from
special_function_p in calls.c, ignoring up to 2 leading underscores from
the fndecl's name when checking for a function by name.

It also requires that such functions are "extern" and at file scope
for them to be matched.

The patch also generalizes the setjmp/longjmp handling in the analyzer
to also work with sigsetjmp/siglongjmp.  Doing so requires generalizing
some hardcoded functions in diagnostics (which were hardcoded to avoid
user-facing messages referring to "_setjmp", which is an implementation
detail) - the patch adds a new function, get_user_facing_name for this,
for use on calls that matched is_named_call_p and
is_specical_named_call_p.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.

OK for master?

gcc/analyzer/ChangeLog:
* analyzer.cc  (is_named_call_p): Check that fndecl is "extern"
and at file scope.  Potentially disregard prefix _ or __ in
fndecl's name.  Bail if the identifier is NULL.
(is_setjmp_call_p): Expect a gcall rather than plain gimple.
Remove special-case check for leading prefix, and also check for
sigsetjmp.
(is_longjmp_call_p): Also check for siglongjmp.
(get_user_facing_name): New function.
* analyzer.h (is_setjmp_call_p): Expect a gcall rather than plain
gimple.
(get_user_facing_name): New decl.
* checker-path.cc (setjmp_event::get_desc): Use
get_user_facing_name to avoid hardcoding the function name.
(rewind_event::rewind_event): Add rewind_info param, using it to
initialize new m_rewind_info field, and strengthen the assertion.
(rewind_from_longjmp_event::get_desc): Use get_user_facing_name to
avoid hardcoding the function name.
(rewind_to_setjmp_event::get_desc): Likewise.
* checker-path.h (setjmp_event::setjmp_event): Add setjmp_call
param and use it to initialize...
(setjmp_event::m_setjmp_call): New field.
(rewind_event::rewind_event): Add rewind_info param.
(rewind_event::m_rewind_info): New protected field.
(rewind_from_longjmp_event::rewind_from_longjmp_event): Add
rewind_info param.
(class rewind_to_setjmp_event): Move rewind_info field to parent
class.
* diagnostic-manager.cc (diagnostic_manager::add_events_for_eedge):
Update setjmp-handling for is_setjmp_call_p requiring a gcall;
pass the call to the new setjmp_event.
* engine.cc (exploded_node::on_stmt): Update for is_setjmp_call_p
requiring a gcall.
(stale_jmp_buf::emit): Use get_user_facing_name to avoid
hardcoding the function names.
(exploded_node::on_longjmp): Pass the longjmp_call when
constructing rewind_info.
(rewind_info_t::add_events_to_path): Pass the rewind_info_t to the
rewind_from_longjmp_event's ctor.
* exploded-graph.h (rewind_info_t::rewind_info_t): Add
longjmp_call param.
(rewind_info_t::get_longjmp_call): New.
(rewind_info_t::m_longjmp_call): New.
* region-model.cc (region_model::on_setjmp): Update comment to
indicate this is also for sigsetjmp.
* region-model.h (struct setjmp_record): Likewise.
(class setjmp_svalue): Likewise.

gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/sigsetjmp-5.c: New test.
* gcc.dg/analyzer/sigsetjmp-6.c: New test.
---
 gcc/analyzer/analyzer.cc| 83 +
 gcc/analyzer/analyzer.h |  4 +-
 gcc/analyzer/checker-path.cc| 14 ++--
 gcc/analyzer/checker-path.h | 30 +---
 gcc/analyzer/diagnostic-manager.cc  |  6 +-
 gcc/analyzer/engine.cc  | 21 +++---
 

[PATCH] analyzer: fixes to tree_cmp and other comparators

2020-01-23 Thread David Malcolm
On Thu, 2020-01-23 at 18:46 +0300, Alexander Monakov wrote:
> On Thu, 23 Jan 2020, David Malcolm wrote:
> 
> > Removing the assertions fixes it for me (a stage1 build, at least,
> > and
> > it then passes the testsuite).
> > 
> > I've made this blunder in four places in the analyzer:
> > 
> >   call-string.cc:162:  call_string::cmp
> >   program-point.cc:461:  function_point::cmp_within_supernode
> 
> These two don't raise concerns on brief inspection.
> 
> >   engine.cc:1820:  worklist::key_t::cmp
> 
> This one tries to use signed difference of hashes as return value,
> this is
> not going to work in general while passing your assert almost always
> (except
> when difference of hashes is exactly -2^31). The problem is,
> difference of
> hashes leads to non-transitive comparison and qsort_chk can catch it
> on a
> suitable testcase.
> 
> >   region-model.cc:1878:  tree_cmp
> 
> This is the one under discussion here.
> 
> > IIRC, I added these checks as I was finding it difficult to debug
> > things when qsort_chk failed - the first three of the comparators
> > in
> > the list above build on each other:  worklist::key_t::cmp uses both
> > function_point::cmp_within_supernode and call_string::cmp (and
> > various
> > other things), so if the worklist qsort_chk fails, it sometimes
> > required a fair bit of digging into which of the nested comparators
> > had
> > failed (also, all the void * don't help; I'd love to have a
> > template
> > that does a typesafe qsort without needing casts in the
> > comparator).
> 
> FWIW qsort_chk_error is a separate function to make that easier: if
> you place a
> breakpoint on qsort_chk_error you just need to step a few times to
> get into a
> comparator that is under suspicion, and don't need to type out casts
> in gdb.
> 
> Alexander

Thanks - that makes much more sense to me now.

How does the following patch look:

region_model.cc's tree_cmp attempted to verify that the ordering
is symmetric by asserting that
  tree_cmp (x, y) == -tree_cmp (y, x)

This condition is too strong: it's only required for a comparator that
  sign (tree_cmp (x, y)) == -sign (tree_cmp (y, x))
and the incorrect form of the assertion doesn't hold e.g. on s390x where
for certain inputs x, y, tree_cmp (x, y) == 1 and tree_cmp (y, x) == -2,
breaking the build in "make selftest" in stage1.

In any case, these checks are redundant, since qsort_chk performs them.

Additionally, there is a potential lack of transitivity in
worklist::key_t::cmp where hashval_t values are compared by subtraction,
which could fail to be transitive if overflows occur.

This patch eliminates the redundant checks and reimplements the hashval_t
comparisons in terms of < and >, fixing these issues.

Fixes build on s390x-ibm-linux-gnu for stage 1, at least, with no
testsuite regressions.  Full bootstrap and regression test run
in progress.

OK for master if it passes?

gcc/analyzer/ChangeLog:
* call-string.cc (call_string::cmp_1): Delete, moving body to...
(call_string::cmp): ...here.
* call-string.h (call_string::cmp_1): Delete decl.
* engine.cc (worklist::key_t::cmp_1): Delete, moving body to...
(worklist::key_t::cmp): ...here.  Implement hash comparisons
via comparison rather than subtraction to avoid overflow issues.
* exploded-graph.h (worklist::key_t::cmp_1): Delete decl.
* region-model.cc (tree_cmp): Eliminate buggy checking for
symmetry.
---
 gcc/analyzer/call-string.cc   | 23 +--
 gcc/analyzer/call-string.h|  3 ---
 gcc/analyzer/engine.cc| 35 +++
 gcc/analyzer/exploded-graph.h |  1 -
 gcc/analyzer/region-model.cc  | 16 +---
 5 files changed, 9 insertions(+), 69 deletions(-)

diff --git a/gcc/analyzer/call-string.cc b/gcc/analyzer/call-string.cc
index 3d398c39a88..288953ed37c 100644
--- a/gcc/analyzer/call-string.cc
+++ b/gcc/analyzer/call-string.cc
@@ -149,6 +149,7 @@ call_string::calc_recursion_depth () const
 }
 
 /* Comparator for call strings.
+   This implements a version of lexicographical order.
Return negative if A is before B.
Return positive if B is after A.
Return 0 if they are equal.  */
@@ -156,28 +157,6 @@ call_string::calc_recursion_depth () const
 int
 call_string::cmp (const call_string ,
  const call_string )
-{
-  int result = cmp_1 (a, b);
-
-  /* Check that the ordering is symmetric  */
-#if CHECKING_P
-  int reversed = cmp_1 (b, a);
-  gcc_assert (reversed == -result);
-#endif
-
-  /* We should only have 0 for equal pairs.  */
-  gcc_assert (result != 0
- || a == b);
-
-  return result;
-}
-
-/* Implementation of call_string::cmp.
-   This implements a version of lexicographical order.  */
-
-int
-call_string::cmp_1 (const call_string ,
-   const call_string )
 {
   unsigned len_a = a.length ();
   unsigned len_b = b.length ();
diff --git a/gcc/analyzer/call-string.h b/gcc/analyzer/call-string.h

[PATCH] diagnostics: Support conversion of tabs to spaces [PR86904]

2020-01-23 Thread Lewis Hyatt
Hello-

In the discussion of r279137, which makes diagnostic-show-locus.c aware that
characters may take up more than one display column, David suggested that I
also look at handling tabs there (expanding into the correct number of
spaces): https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00625.html. This
addresses also PR 86904 which complains that the column number counts tabs
as one byte. The attached patch adds support for expanding tabs into spaces
when outputting source lines in diagnostics. It does not fully resolve PR
86904, as it does not change the column number yet, but it does fix the
alignment of the output, which is particularly noticeable with mixed
tabs-and-spaces styles. The full resolution will be done by one more patch,
which should close out both PR 86904 and PR 49973 (the latter is similar but
for multibyte characters as opposed to tabs). I believe the current
consensus was to add a new option -fdiagnostics-column-unit which could take
a few options to control whether or not to expand tabs, and whether to count
bytes or display columns (or perhaps codepoints or something else) in
computing this number. This option would then also affect machine-readable
fixit outputs too. I will work on that next, it should be pretty
straightforward as the current patch handles all the details of actually
expanding the tabs.

Tabs can mostly be handled the same way as multibyte characters, so no major
conceptual changes to diagnostic-show-locus.c were necessary. The only
wrinkle is that a tab's effective width is context-dependent, so some state
must be remembered. To facilitate remembering the state (what column you are
currently at), the functionality like cpp_byte_column_to_display_column()
was moved into a class instead, so there are some changes prompted by that,
which result in a bit simpler code throughout also.

In order to track the necessary state, diagnostic-show-locus has to become
aware of how many display columns it has output so far, which means it no
longer outputs strictly byte-by-byte. This has the advantage of fixing also
an issue with colorization corrupting multibyte characters, which I
mentioned here: https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00915.html.
Consequently, review of that patch is no longer needed. I have added the
testcase into this one.

In this patch, the existing -ftabstop option (which controls the behavior of
C indentation-based warnings) is reused to also control the size of the
tabstop on output. Because the tab expansion is common to all front ends,
-ftabstop was moved from c.opt into common.opt, and its value is now stored
independent of any cpp_reader instance.

A fair number of tests (generally those making use of
dg-begin-multiline-output) needed trivial changes to the whitespace in their
expected output, since they assumed tabs would become single spaces. I
considered whether to add an option to set the tabstop for diagnostics
separately from the tabstop for indentation warnings, so that these tests
could be fixed rather by adding an option like -fdiagnostics-tabstop=1. No
reason that can't be done, but it seemed worthwhile to change the test
output anyway, as it looks more natural when it is properly aligned. I
couldn't think of any other use case that would require -ftabstop to be
separated from diagnostics, so I left it like this.

Bootstrapped and reg-tested on x86-64-linux, results the same before and after:
FAIL 106 106
PASS 461357 461357
UNRESOLVED 1 1
UNSUPPORTED 11151 11151
UNTESTED 202 202
XFAIL 1721 1721
XPASS 35 35

Thanks for taking a look at it!

-Lewis

gcc/ChangeLog:

2020-01-23  Lewis Hyatt  

PR other/86904
* common.opt: Handle -ftabstop here instead of in c-family
options.
* opts.c (common_handle_option): Likewise.
* diagnostic-show-locus.c (struct line_bounds): Clarified that the
units are now always display columns.  Renamed members accordingly.
Added constructor.
(layout::print_source_line): Added support for tab expansion.
(layout::print_annotation_line): Adapt to struct line_bounds changes.
(layout::print_line): Likewise.
(test_layout_x_offset_display_tab): New selftest.
(test_one_liner_colorized_utf8): Likewise.
(test_tab_expansion): Likewise.
(test_diagnostic_show_locus_one_liner_utf8): Call the new tests.
(diagnostic_show_locus_c_tests): Likewise.
* input.h (location_compute_display_column): Added tabstop argument.
* input.c (location_compute_display_column): Likewise.
(test_cpp_utf8): Added selftest for tab expansion.

gcc/c-family/ChangeLog:

2020-01-23  Lewis Hyatt  

PR other/86904
* c-indentation.c (should_warn_for_misleading_indentation): Get
global tabstop from the new source.
* c-opts.c (c_common_handle_option): Remove handling of -ftabstop, which
is now a common option instead of a c-family option.
* c.opt: Likewise.


Re: [patch, testsuite] Add -fdelete-null-pointer-checks to some C++ testcases

2020-01-23 Thread Sandra Loosemore

On 1/23/20 1:17 PM, Jeff Law wrote:

On Tue, 2020-01-21 at 15:00 -0700, Sandra Loosemore wrote:

In doing some nios2-elf testing, I ran into a bunch of failures in
constexpr-related tests in the C++ testsuite.  This target defaults to
-fno-delete-null-pointer-checks at the request of Altera/Intel, in order
to support some of their BSPs where 0 is a legitimate memory address.
Some other bare-metal targets also default to
-fno-delete-null-pointer-checks.

This patch makes the dependence of these tests on
-fdelete-null-pointer-checks explicit.  I've previously fixed some other
tests that failed on nios2-elf in the same way so this is borderline
obvious, but it's a little troubling to me that the correct semantics of
some of these testcases seems to depend on what we document in the
manual as an optimization option.  :-S  Maybe there is some other bug here?

Anyway, if nobody has any objections or better ideas, I will go ahead
and commit this in a few days.

It'd be nice to know why that flag matters for constexpr.  But I've got
no problem with the change itself.


I haven't looked at all of the failing tests in detail, but the ones I 
did peek at all seemed to be related to constant-folding pointer 
comparisons against null.


The thing that nags at me is that e.g. in g++.dg/cpp0x/constexpr-odr1.C 
we have:


template struct A {
  static const bool x;
  static_assert(, ""); // odr-uses A<...>::x
};

int g;

template
const bool A::x = (g = 42, false);

void f(A<0>) {}// A<0> must be complete, so is instantiated
int main()
{
  if (g != 42)
__builtin_abort ();
}

Whether or not this is valid code depends on whether " != 0" can be 
constant-folded.  Without -fdelete-null-pointer-checks, g++ says:


/path/to/g++.dg/cpp0x/constexpr-odr1.C:
In instantiation of 'struct A<0>':
/path/to/g++.dg/cpp0x/constexpr-odr1.C:14:12:
required from here
/path/to/g++.dg/cpp0x/constexpr-odr1.C:6:17:
error: non-constant condition for static assertion
/path/to/g++.dg/cpp0x/constexpr-odr1.C:12:12:
error: '((& A<0>::x) != 0)' is not a constant expression

So is -fdelete-null-pointer-checks really an optimization option, or a 
language semantics option?


BTW, here's Altera's original description of the over-eager optimization 
problems they were trying to work around:


https://gcc.gnu.org/ml/gcc/2015-02/msg00163.html

-Sandra


[committed] testsuite: Require lp64 target rather x86_64-*-* in pr93027.c. [PR93027]

2020-01-23 Thread Jakub Jelinek
Hi!

I've noticed this test failed on x86_64-linux with -m32 or -mx32 testing,
the triplet doesn't really say which actual multilib it is, and the test
really works with lp64.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
committed to trunk as obvious.

2020-01-23  Jakub Jelinek  

PR inline-asm/93027
* gcc.target/i386/pr93027.c: Require lp64 target rather x86_64-*-*.

--- gcc/testsuite/gcc.target/i386/pr93027.c.jj  2020-01-12 11:54:37.986389677 
+0100
+++ gcc/testsuite/gcc.target/i386/pr93027.c 2020-01-23 19:55:42.264886784 
+0100
@@ -1,5 +1,5 @@
 /* PR inline-asm/93027 */
-/* { dg-do compile  { target x86_64-*-* } } */
+/* { dg-do compile  { target lp64 } } */
 /* { dg-options "-O0" } */
 
 int main (void) {


Jakub



[PATCH] i386: prefer vpermilpd over vpermpd [PR93395]

2020-01-23 Thread Jakub Jelinek
Hi!

In Agner Fog's tables, vpermilp[sd] with immediates seem to be
much faster than vpermpd with immediate, for a good reason,
the former only permute something within the lanes and don't do anything
intra-lane, while vpermpd can.  So, functionality-wise, vpermilpd
is more efficient subset of vpermpd.  We use the same RTL for those
though (and also for certain broadcast).

Now, the problem was that the vpermpd pattern appeared first in sse.md,
followed by the broadcast patterns, followed by the vpermilp[sd].
Which means unless -mavx -mno-avx2, we'd emit vpermpd instead of the
more efficient alternatives.

The following patch reorders them, so that vpermpd comes last, if we
can match a broadcast, we do, if we can match a vpermilp[sd] that is not a
broadcast, we will, otherwise fall back (of course only if -mavx2) to
vpermpd.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2020-01-23  Jakub Jelinek  

PR target/93395
* config/i386/sse.md (*avx_vperm_broadcast_v4sf,
*avx_vperm_broadcast_,
_vpermil,
*_vpermilp):
Move before avx2_perm/avx512f_perm.

* gcc.target/i386/pr93395.c: New test.
* gcc.target/i386/avx512vl-vpermilpdi-1.c: Remove xfail.

--- gcc/config/i386/sse.md.jj   2020-01-23 19:24:14.851423969 +0100
+++ gcc/config/i386/sse.md  2020-01-23 19:41:58.729091766 +0100
@@ -19875,6 +19875,164 @@ (define_insn "_permvar")
(set_attr "mode" "")])
 
+;; Recognize broadcast as a vec_select as produced by builtin_vec_perm.
+;; If it so happens that the input is in memory, use vbroadcast.
+;; Otherwise use vpermilp (and in the case of 256-bit modes, vperm2f128).
+(define_insn "*avx_vperm_broadcast_v4sf"
+  [(set (match_operand:V4SF 0 "register_operand" "=v,v,v")
+   (vec_select:V4SF
+ (match_operand:V4SF 1 "nonimmediate_operand" "m,o,v")
+ (match_parallel 2 "avx_vbroadcast_operand"
+   [(match_operand 3 "const_int_operand" "C,n,n")])))]
+  "TARGET_AVX"
+{
+  int elt = INTVAL (operands[3]);
+  switch (which_alternative)
+{
+case 0:
+case 1:
+  operands[1] = adjust_address_nv (operands[1], SFmode, elt * 4);
+  return "vbroadcastss\t{%1, %0|%0, %k1}";
+case 2:
+  operands[2] = GEN_INT (elt * 0x55);
+  return "vpermilps\t{%2, %1, %0|%0, %1, %2}";
+default:
+  gcc_unreachable ();
+}
+}
+  [(set_attr "type" "ssemov,ssemov,sselog1")
+   (set_attr "prefix_extra" "1")
+   (set_attr "length_immediate" "0,0,1")
+   (set_attr "prefix" "maybe_evex")
+   (set_attr "mode" "SF,SF,V4SF")])
+
+(define_insn_and_split "*avx_vperm_broadcast_"
+  [(set (match_operand:VF_256 0 "register_operand" "=v,v,v")
+   (vec_select:VF_256
+ (match_operand:VF_256 1 "nonimmediate_operand" "m,o,?v")
+ (match_parallel 2 "avx_vbroadcast_operand"
+   [(match_operand 3 "const_int_operand" "C,n,n")])))]
+  "TARGET_AVX"
+  "#"
+  "&& reload_completed && (mode != V4DFmode || !TARGET_AVX2)"
+  [(set (match_dup 0) (vec_duplicate:VF_256 (match_dup 1)))]
+{
+  rtx op0 = operands[0], op1 = operands[1];
+  int elt = INTVAL (operands[3]);
+
+  if (REG_P (op1))
+{
+  int mask;
+
+  if (TARGET_AVX2 && elt == 0)
+   {
+ emit_insn (gen_vec_dup (op0, gen_lowpart (mode,
+ op1)));
+ DONE;
+   }
+
+  /* Shuffle element we care about into all elements of the 128-bit lane.
+The other lane gets shuffled too, but we don't care.  */
+  if (mode == V4DFmode)
+   mask = (elt & 1 ? 15 : 0);
+  else
+   mask = (elt & 3) * 0x55;
+  emit_insn (gen_avx_vpermil (op0, op1, GEN_INT (mask)));
+
+  /* Shuffle the lane we care about into both lanes of the dest.  */
+  mask = (elt / ( / 2)) * 0x11;
+  if (EXT_REX_SSE_REG_P (op0))
+   {
+ /* There is no EVEX VPERM2F128, but we can use either VBROADCASTSS
+or VSHUFF128.  */
+ gcc_assert (mode == V8SFmode);
+ if ((mask & 1) == 0)
+   emit_insn (gen_avx2_vec_dupv8sf (op0,
+gen_lowpart (V4SFmode, op0)));
+ else
+   emit_insn (gen_avx512vl_shuf_f32x4_1 (op0, op0, op0,
+ GEN_INT (4), GEN_INT (5),
+ GEN_INT (6), GEN_INT (7),
+ GEN_INT (12), GEN_INT (13),
+ GEN_INT (14), GEN_INT (15)));
+ DONE;
+   }
+
+  emit_insn (gen_avx_vperm2f1283 (op0, op0, op0, GEN_INT (mask)));
+  DONE;
+}
+
+  operands[1] = adjust_address (op1, mode,
+   elt * GET_MODE_SIZE (mode));
+})
+
+(define_expand "_vpermil"
+  [(set (match_operand:VF2 0 "register_operand")
+   (vec_select:VF2
+ (match_operand:VF2 1 "nonimmediate_operand")
+ (match_operand:SI 2 "const_0_to_255_operand")))]
+  

[wwwdocs] Fix markup of (to ).

2020-01-23 Thread Gerald Pfeifer
A small fix on top of the last edits to gitwrite.html; spelling
"<" and ">" the HTML way is painful, and visually hard to grok.

Pushed.

Gerald
---
 htdocs/gitwrite.html | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/htdocs/gitwrite.html b/htdocs/gitwrite.html
index 047c139f..ad073b53 100644
--- a/htdocs/gitwrite.html
+++ b/htdocs/gitwrite.html
@@ -497,7 +497,7 @@ to that vendor's branches.
 If you have set up push access, then the branch can similarly be pushed to
   using:
 
-git push vendors/vendor vendor/topic
+git push vendors/vendor vendor/topic
 
 
 The script can be re-run with, or without --enable-push
@@ -516,7 +516,7 @@ to that vendor's branches.
   run and starting from the most recent commit on master:
 
 
-contrib/git-add-vendor-branch.sh  vendor/topic master
+contrib/git-add-vendor-branch.sh  vendor/topic master
 
 
 This will create the branch both locally and on the server, but will not
-- 
2.25.0


[wwwdocs] Abstract away verbal references to SVN in the GOMP project description.

2020-01-23 Thread Gerald Pfeifer
Technically not necessary, but while I was at it this simplified
things (and there were no similarly many references anywhere else).

Pushed.

Gerald
---
 htdocs/projects/gomp/index.html | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/htdocs/projects/gomp/index.html b/htdocs/projects/gomp/index.html
index 39c8d3ad..70e6d95a 100644
--- a/htdocs/projects/gomp/index.html
+++ b/htdocs/projects/gomp/index.html
@@ -69,7 +69,7 @@ 
href="https://www.openmp.org/wp-content/uploads/openmp-4.5.pdf;>OpenMP v4.5
 specification has been released.
 
 October 13, 2015
-The gomp-4_1-branch has been merged into SVN
+The gomp-4_1-branch has been merged into
 mainline, so GCC 6 and later will feature OpenMP v4.5 support for
 C and C++.
 
@@ -80,10 +80,10 @@ GCC 4.9.1 release.
 
 June 18, 2014
 The last major part of Fortran OpenMP v4.0 support has been
-committed into SVN mainline.
+committed into mainline.
 
 October 11, 2013
-The gomp-4_0-branch has been merged into SVN
+The gomp-4_0-branch has been merged into
 mainline, so GCC 4.9 and later will feature OpenMP v4.0 support for
 C and C++.
 
@@ -91,7 +91,7 @@ C and C++.
 The final OpenMP v4.0 specification has been released.
 
 August 2, 2011
-The gomp-3_1-branch has been merged into SVN
+The gomp-3_1-branch has been merged into
 mainline, so GCC 4.7 and later will feature OpenMP v3.1 support.
 
 July 9, 2011
@@ -100,10 +100,10 @@ mainline, so GCC 4.7 and later will feature OpenMP v3.1 
support.
 February 6, 2011
 A draft of the OpenMP v3.1 specification has been released for
 public review.  The gomp-3_1-branch branch has been
-created in SVN and work began on implementing v3.1 support.
+created and work began on implementing v3.1 support.
 
 June 6, 2008
-The gomp-3_0-branch has been merged into SVN
+The gomp-3_0-branch has been merged into
 mainline, so GCC 4.4 and later will feature OpenMP v3.0 support.
 
 May 12, 2008
@@ -112,7 +112,7 @@ mainline, so GCC 4.4 and later will feature OpenMP v3.0 
support.
 October 22, 2007
 Draft of the OpenMP v3.0 specification has been released for
 public review, the gomp-3_0-branch branch has been
-created in SVN and work began on implementing v3.0 support.
+created and work began on implementing v3.0 support.
 
 March 9, 2006
 The branch has been merged into mainline, so starting with
-- 
2.25.0


Re: [PATCH] i386: Fix up -fdollars-in-identifiers with identifiers starting with $ in -masm=att (PR target/91298)

2020-01-23 Thread Jakub Jelinek
On Thu, Jan 23, 2020 at 09:19:52PM +0100, Rainer Orth wrote:
> Unlikely: there's barely any development on the Solaris assemblers these
> days, though I prefer to xfail tests if possible to get notified if they
> suddenly start to work for some reason.

Ok.

Jakub



Re: [PATCH] i386: Fix up -fdollars-in-identifiers with identifiers starting with $ in -masm=att (PR target/91298)

2020-01-23 Thread Rainer Orth
Hi Jakub,

> On Thu, Jan 23, 2020 at 09:12:27PM +0100, Rainer Orth wrote:
>> +FAIL: gcc.target/i386/pr91298-2.c (test for excess errors)
>> 
>> It only allows letters, digits, '_' and '.' in identifiers:
>> https://docs.oracle.com/cd/E37838_01/html/E61064/eqbsx.html#XALRMeoqjw
>
> So, no way to emit other letters in identifiers through some other syntax?

none that I know of.  There's certainly nothing documented.

>> Right now, we don't have an effective-target keyword matching
>> -fdollars-in-identifiers.  There are only 3 other tests using that
>> option:
>> 
>>   gcc.dg/cpp/lexident.c  preprocess only
>>   gcc.dg/ucnid-5-utf8.c  skipped on avr*-*, powerpc-ibm-aix* (no 
>> $)
>>  skipped if ! ucn
>>   gcc.dg/ucnid-5.c   skipped on avr*-*, powerpc-ibm-aix* (no $)
>> 
>> So we might just xfail the new ones on *86*-*-solaris2* && !gas, I guess.
>
> Or even dg-skip-if or { target { ! *86*-*-solaris2* || gas } }, I guess it
> really depends on whether there is any hope it might work there.

Unlikely: there's barely any development on the Solaris assemblers these
days, though I prefer to xfail tests if possible to get notified if they
suddenly start to work for some reason.

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University


Re: [patch, testsuite] Add -fdelete-null-pointer-checks to some C++ testcases

2020-01-23 Thread Jeff Law
On Tue, 2020-01-21 at 15:00 -0700, Sandra Loosemore wrote:
> In doing some nios2-elf testing, I ran into a bunch of failures in 
> constexpr-related tests in the C++ testsuite.  This target defaults to 
> -fno-delete-null-pointer-checks at the request of Altera/Intel, in order 
> to support some of their BSPs where 0 is a legitimate memory address. 
> Some other bare-metal targets also default to 
> -fno-delete-null-pointer-checks.
> 
> This patch makes the dependence of these tests on 
> -fdelete-null-pointer-checks explicit.  I've previously fixed some other 
> tests that failed on nios2-elf in the same way so this is borderline 
> obvious, but it's a little troubling to me that the correct semantics of 
> some of these testcases seems to depend on what we document in the 
> manual as an optimization option.  :-S  Maybe there is some other bug here?
> 
> Anyway, if nobody has any objections or better ideas, I will go ahead 
> and commit this in a few days.
It'd be nice to know why that flag matters for constexpr.  But I've got
no problem with the change itself.

jeff



Re: [PATCH] i386: Fix up -fdollars-in-identifiers with identifiers starting with $ in -masm=att (PR target/91298)

2020-01-23 Thread Jakub Jelinek
On Thu, Jan 23, 2020 at 09:12:27PM +0100, Rainer Orth wrote:
> +FAIL: gcc.target/i386/pr91298-2.c (test for excess errors)
> 
> It only allows letters, digits, '_' and '.' in identifiers:
> https://docs.oracle.com/cd/E37838_01/html/E61064/eqbsx.html#XALRMeoqjw

So, no way to emit other letters in identifiers through some other syntax?

> Right now, we don't have an effective-target keyword matching
> -fdollars-in-identifiers.  There are only 3 other tests using that
> option:
> 
>   gcc.dg/cpp/lexident.c   preprocess only
>   gcc.dg/ucnid-5-utf8.c   skipped on avr*-*, powerpc-ibm-aix* (no 
> $)
>   skipped if ! ucn
>   gcc.dg/ucnid-5.cskipped on avr*-*, powerpc-ibm-aix* (no $)
> 
> So we might just xfail the new ones on *86*-*-solaris2* && !gas, I guess.

Or even dg-skip-if or { target { ! *86*-*-solaris2* || gas } }, I guess it
really depends on whether there is any hope it might work there.

Jakub



Re: [PATCH] doc: target.def (flags_regnum): Mention effect on delay slot filling.

2020-01-23 Thread Jeff Law
On Thu, 2020-01-23 at 20:38 +0100, Hans-Peter Nilsson wrote:
> gcc:
> * target.def (flags_regnum): Also mention effect on delay slot filling.
> * doc/tm.texi: Regenerate.
> 
> Noticed the "hard way" dealing with performance fallout for the
> CRIS decc0ration.
> 
> Previously, the documentation blurb only mentioned an effect on
> compare elimination.  The technical contents is obvious but I'm
> more worried about grammar.  I don't see hyphenation elsewhere
> when referring to delay slot filling, but then again the term
> used for dbr-scheduling isn't consistent in that part of the
> documentation.  Note also that it's not just branches, says the
> code in reorg.c.
> 
> Dvi and info results inspected for sanity.
> 
> Ok to commit?
OK
jeff




Re: [PATCH 2/4] [ARC] Propagate uncached type attribute.

2020-01-23 Thread Jeff Law
On Wed, 2020-01-22 at 10:14 +0200, Claudiu Zissulescu wrote:
> Like `packed` type attribute, the ARC's `uncached` type attribute
> needs to be propagated to each member of the struct where it is used.
> Fix this behavior and add a test.
> 
> gcc/
> -xx-xx  Claudiu Zissulescu  
> 
>   * config/arc/arc.c (arc_is_uncached_mem_p): Check struct
>   attributes if needed.
> 
> testsuite/
> -xx-xx  Claudiu Zissulescu  
> 
>   * gcc.target/arc/uncached-3.c: New test.
> 
> [ARC] Fix pending errors with uncached type attribute.
> 
> uncached type attribute applied to a structure needs to be propagated
> to its members, triggering the .di flag for any access of the struct
> members. Also, any complex CFG manipulation may drop memory pointer
> type attributes, leading to the impossibility to discriminate the
> direct accesses from normal ones. To solve this issue, we will treat
> the direct memory accessed specially via unspecs.
> 
> gcc/
> -xx-xx  Claudiu Zissulescu  
>   Petro Karashchenko  
> 
>   * config/arc/arc.c (prepare_move_operands): Generate special
>   unspec instruction for direct access.
>   (arc_isuncached_mem_p): Propagate uncached attribute to each
>   structure member.
>   * config/arc/arc.md (VUNSPEC_ARC_LDDI): Define.
>   (VUNSPEC_ARC_STDI): Likewise.
>   (ALLI): New mode iterator.
>   (mALLI): New mode attribute.
>   (lddi): New instruction pattern.
>   (stdi): Likewise.
>   (stdidi_split): Split instruction for architectures which are not
>   supporting ll64 option.
>   (lddidi_split): Likewise.
> 
> testsuite/
> -xx-xx  Claudiu Zissulescu  
>   Petro Karashchenko  
> 
>   * gcc.target/arc/uncached-4.c: New file.
>   * gcc.target/arc/uncached-5.c: Likewise.
>   * gcc.target/arc/uncached-6.c: Likewise.
>   * gcc.target/arc/uncached-7.c: Likewise.
>   * gcc.target/arc/uncached-8.c: Likewise.
>   * gcc.target/arc/arc.exp (ll64): New predicate.
OK
jeff
> 



Re: [PATCH] i386: Fix up -fdollars-in-identifiers with identifiers starting with $ in -masm=att (PR target/91298)

2020-01-23 Thread Rainer Orth
Hi Jakub,

> In AT syntax leading $ is special, so if we have identifiers that start
> with dollar, we usually fail to assemble it (or assemble incorrectly).
> As mentioned in the PR, what works is wrapping the identifiers inside of
> parens, like:
>   movl$($a), %eax
>   leaq($a)(,%rdi,4), %rax
>   movl($a)(%rip), %eax
>   movl($a)+16(%rip), %eax
>   .globl  $a
>   .type   $a, @object
>   .size   $a, 72
> $a:
>   .string "$a"
>   .quad   ($a)
> (this is x86_64 -fno-pic -O2).  In some places ($a) is not accepted,
> like as .globl operand, in .type, .size, so the patch overrides
> ASM_OUTPUT_SYMBOL_REF rather than e.g. ASM_OUTPUT_LABELREF.
> I didn't want to duplicate what assemble_name is doing (following
> transparent aliases), so split assemble_name into two parts; just
> mere looking at the first character of a name before calling assemble_name
> wouldn't be good enough, a transparent alias could lead from a name
> not starting with $ to one starting with it and vice versa.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

the new testcases FAIL on Solaris/x86 with the native assembler:

+FAIL: gcc.target/i386/pr91298-1.c (test for excess errors)

Excess errors:
Assembler: pr91298-1.c
"/var/tmp//ccE6r3xb.s", line 5 : Syntax error
Near line: ".globl  $quux"
"/var/tmp//ccE6r3xb.s", line 6 : Syntax error
Near line: ".type   $quux, @function"
"/var/tmp//ccE6r3xb.s", line 7 : Syntax error
Near line: "$quux:"
"/var/tmp//ccE6r3xb.s", line 15 : Syntax error
Near line: ".size   $quux, .-$quux"
"/var/tmp//ccE6r3xb.s", line 24 : Syntax error
Near line: "movl$($a), %eax"
"/var/tmp//ccE6r3xb.s", line 38 : Syntax error
Near line: "leal($a)(,%eax,4), %eax"
"/var/tmp//ccE6r3xb.s", line 51 : Syntax error
Near line: "movl($a), %eax"
"/var/tmp//ccE6r3xb.s", line 63 : Syntax error
Near line: "movl($a)+16, %eax"
"/var/tmp//ccE6r3xb.s", line 97 : Syntax error
Near line: "movl$($quux), %eax"
"/var/tmp//ccE6r3xb.s", line 101 : Syntax error
Near line: ".globl  $a"
"/var/tmp//ccE6r3xb.s", line 104 : Syntax error
Near line: ".type   $a, @object"
"/var/tmp//ccE6r3xb.s", line 105 : Syntax error
Near line: ".size   $a, 72"
"/var/tmp//ccE6r3xb.s", line 106 : Syntax error
Near line: "$a:"
"/var/tmp//ccE6r3xb.s", line 228 : Syntax error
Near line: ".long   ($a)"

+FAIL: gcc.target/i386/pr91298-2.c (test for excess errors)

It only allows letters, digits, '_' and '.' in identifiers:
https://docs.oracle.com/cd/E37838_01/html/E61064/eqbsx.html#XALRMeoqjw

Right now, we don't have an effective-target keyword matching
-fdollars-in-identifiers.  There are only 3 other tests using that
option:

  gcc.dg/cpp/lexident.c preprocess only
  gcc.dg/ucnid-5-utf8.c skipped on avr*-*, powerpc-ibm-aix* (no $)
skipped if ! ucn
  gcc.dg/ucnid-5.c  skipped on avr*-*, powerpc-ibm-aix* (no $)

So we might just xfail the new ones on *86*-*-solaris2* && !gas, I guess.

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University


Re: [PATCH] libsanitizer: Add missign file and regen Makefile.in

2020-01-23 Thread Jeff Law
On Wed, 2020-01-22 at 22:23 +0100, Andreas Tobler wrote:
> Hi all,
> 
> I'm digginig out old patches and I want to complete the libasan support 
> for FreeBSD x86_64. The below one was not that obvious when you have 
> been away for the past years.
> 
> In the last import the sanitizer_platform_limits_freebsd.cpp got
> forgotten. Fix this.
> 
> Ok for trunk once it's open again?
> 
> Thanks,
> Andreas
> 
> libsanitizer/sanitizer_common:
> 
>  * Makefile.am: Add sanitizer_platform_limits_freebsd.cpp.
>  * makefile.in: Regenerate
I think all the patches in this space are fine for the trunk.  As
someone else mentioned, the sanitizer patches should probably go
through the upstream project as GCC is downstream.

Jeff
> 



Re: libgo patch committed: Update to Go1.14beta1

2020-01-23 Thread Rainer Orth
Hi Ian,

> On Wed, Jan 22, 2020 at 12:18 PM Rainer Orth
>  wrote:
>>
>> > I've committed a patch to update libgo to Go 1.14beta1.  As usual with
>> > these updates the patch is far too large to include in this e-mail
>> > message.  I've included the diffs for gccgo-specific files.
>> > Bootstrapped and ran Go tests on x86_64-pc-linux-gnu.  Committed to
>> > mainline.
>>
>> the patch broke Solaris bootstrap:
>>
>> /vol/gcc/src/hg/master/local/libgo/go/runtime/os_only_solaris.go:11:1:
>> error: redefinition of 'getncpu'
>>11 | func getncpu() int32 {
>>   | ^
>> /vol/gcc/src/hg/master/local/libgo/go/runtime/os3_solaris.go:20:1: note:
>> previous definition of 'getncpu' was here
>>20 | func getncpu() int32 {
>>   | ^
>>
>> There are 3 definitions in the Solaris/Illumos space:
>>
>> * os_only_solaris.go is guarded by !illumos
>>
>> * os3_solaris.go has no explicit guard
>>
>> * illumos hat its own one in os_illumos.go
>>
>> so the os3_solaris.go one can go.
>>
>> /vol/gcc/src/hg/master/local/libgo/go/runtime/stubs2.go:40:3: error:
>> osinit is not defined
>>40 | //go:linkname osinit runtime.osinit
>>   |   ^
>>
>> Upstream has a definition in os3_solaris.go.
>>
>> The following patch allows compilation to succeed.
>
> Thanks, I already committed a patch before I got to your e-mail.
> Sorry for the duplicate work.

no worries: it didn't take long to devise the fix.

Thanks.
Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University


Re: [PATCH] cprop: Don't replace fixed hard regs with pseudos [PR93124]

2020-01-23 Thread Jeff Law
On Thu, 2020-01-23 at 11:43 +, Richard Sandiford wrote:
> Jeff Law  writes:
> > On Wed, 2020-01-22 at 12:02 +, Richard Sandiford wrote:
> > > One consequence of r276318 was that cselib now preserves sp-based
> > > values across function calls.  This in turn convinced cprop to
> > > replace the clobber in:
> > > 
> > >(set (reg PSUEDO) (reg sp))
> > >...
> > >(call ...)
> > >...
> > >(clobber (mem:BLK (reg sp)))
> > > 
> > > with:
> > > 
> > >(clobber (mem:BLK (reg PSEUDO)))
> > > 
> > > But I doubt this could ever be an optimisation, regardless of what the
> > > changed instruction is.  Extending the lifetimes of pseudos can lead to
> > > extra spills, whereas sp is available everywhere.
> > > 
> > > More generally, I don't think we should replace any fixed hard register
> > > with a pseudo.  Replacing them with a constant is still potentially
> > > useful though, since we'll only make the change if the insn pattern
> > > allows it.
> > > 
> > > This part 1 of the fix for PR93124.  Part 2 contains the testcase.
> > > 
> > > Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
> > > 
> > > Richard
> > > 
> > > 
> > > 2020-01-22  Richard Sandiford  
> > > 
> > > gcc/
> > >   PR rtl-optimization/93124
> > >   * cprop.c (cprop_replace_with_reg_p): New function.
> > >   (cprop_insn, do_local_cprop): Use it.
> > In theory there may be cases where replacing a fixed hard register with
> > a pseudo  in turn might allow a allocation of the pseudo to a different
> > hard register which *could* have a different cost.
> > 
> > But in a CLOBBER insn, none of that should matter.  Would it make sense
> > to only do this on CLOBBERS?  I'm not rejecting.  Mostly I'm worried
> > about unintended consequences and wondering if we narrow the cases
> > where we're changing behavior that unintended consequences are less
> > likely to pop up.
> 
> Yeah, I guess there is a danger of unintended consequences.  E.g. on
> aarch64 the sp register isn't usable everywhere that a normal GPR is.
> Ideally that kind of thing would be enforced by the predicates,
> but it generally isn't yet, and it would be nice if the .md format
> could make this easier to get right (e.g. generating predicates
> automatically from constraints for simple cases).
> 
> I didn't make it clear at all, but clobbers don't really pose a separate
> problem in the context of this patch.  I assume we made this kind of
> sp->pseudo change between call boundaries before r276318 too, and the
> auto-inc-dec patch is enough to fix the PR on its own.
Right.  You may not have been explicit, but I certainly got the
impression that the auto-inc-dec patch was sufficient to fix this
instance, but both provide a higher degree of coverage.

> 
> It's just that when I saw where the (clobber (mem (reg PSEUDO))) was
> coming from, it looked like a misfeature for general non-clobber insns
> too.  In the testcase it was making a pseudo live across a call when it
> wasn't before, meaning that either the pseudo would need to be spilled
> around the call or that we'd need to save an extra call-saved
> register.  The fact that we did this for an sp equivalence is a
> regression from GCC 9.
ACK.  Interestingly enough we had another problem in this space pop up
today.

Finding a way to drop the naked clobbers/uses would be a better way
forward.  I'm a bit surprised we need them as much as we apparently do.
We're conflating issues a bit here though.


> 
> But maybe the patch is tackling this in the wrong place.  In general,
> I think we should be careful about propagating registers across calls
> that they were previously dead at, and this should probably be tackled
> in those terms instead.
Yea, there's certainly a cost when we make something live across a call
that wasn't previously.  I don't think we generally account for that.
> 
> So maybe the safest thing is to go with just the auto-inc-dec patch for
> GCC 10.
I'm certainly willing to go with your judgment on this.  Again I've got
a slight concern about the possibility of unintended consequences. 
THen again we may have positive unintended consequences if we were to
go forward with this patch now.


Jeff



Re: [PATCH] Do not print params in --help except --help=param.

2020-01-23 Thread Jeff Law
On Thu, 2020-01-23 at 15:26 +0100, Martin Liška wrote:
> Hi.
> 
> The patch finishes stripping of params in --help dump
> for e.g. --help=joined or --help=separate.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Ready to be installed?
> Thanks,
> Martin
> 
> gcc/ChangeLog:
> 
> 2020-01-23  Martin Liska  
> 
>   * opts.c (print_help): Exclude params from
>   all except --help=param.
OK
jeff
> 



Re: [PATCH] Filter out language specific options from --help=common.

2020-01-23 Thread Jeff Law
On Thu, 2020-01-23 at 15:28 +0100, Martin Liška wrote:
> Hi.
> 
> The following is attempt to fix the PR. Idea is such that
> all non-driver Common options that are also used in context
> of a languages should not be printed in --help=common. An
> exception would be an option that will be included in all languages.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Ready to be installed?
> Thanks,
> Martin
> 
> gcc/ChangeLog:
> 
> 2020-01-23  Martin Liska  
> 
>   PR driver/91220
>   * opts.c (print_filtered_help): Exclude language-specific
>   options from --help=common unless enabled in all FEs.
It's not marked as a regression, but one could argue this is a doc fix.

OK
jeff



Re: drop -aux{dir,base}, revamp -dump{dir,base}

2020-01-23 Thread Alexandre Oliva
On Jan 22, 2020, Richard Biener  wrote:

>> I suppose I might go ahead and install the libiberty follow-up patch
>> approved by Joseph, and squash the lto-wrapper portion into the larger
>> patch.  Please let me know in case you think the libiberty change to
>> preserve empty arguments should also be deferred to 11.

> I think it's fine to make that change now.

Here's the split-out patch I'm installing now.  The other fragment is
being combined with the patch pending for GCC 11 stage 1.


[libiberty] output empty args as a pair of quotes

From: Alexandre Oliva 

writeargv writes out empty arguments in a way that expandargv skips
them instead of preserving them.  Fixed by writing out a pair of
quotes for them.


for  libiberty/ChangeLog

* argv.c (writeargv): Output empty args as "".
---
 libiberty/argv.c |8 
 1 file changed, 8 insertions(+)

diff --git a/libiberty/argv.c b/libiberty/argv.c
index 8c9794db..6a72208 100644
--- a/libiberty/argv.c
+++ b/libiberty/argv.c
@@ -327,6 +327,14 @@ writeargv (char * const *argv, FILE *f)
   arg++;
 }
 
+  /* Write out a pair of quotes for an empty argument.  */
+  if (arg == *argv)
+   if (EOF == fputs ("\"\"", f))
+ {
+   status = 1;
+   goto done;
+ }
+
   if (EOF == fputc ('\n', f))
 {
   status = 1;


-- 
Alexandre Oliva, freedom fighter   he/him   https://FSFLA.org/blogs/lxo
Free Software Evangelist   Stallman was right, but he's left :(
GNU Toolchain EngineerFSMatrix: It was he who freed the first of us
FSF & FSFLA board memberThe Savior shall return (true);


Re: tolerate padding in mbstate_t

2020-01-23 Thread Alexandre Oliva
On Jan 22, 2020, Jonathan Wakely  wrote:

>>> I don't think so, that wouldn't work. I think pos02 could just be
>>> removed from the test.

>> Will do.

> Thanks, OK to commit.

Thanks, here's what I tested and am about to install.


tolerate padding in mbstate_t

From: Alexandre Oliva 

Padding in mbstate_t objects may get the memcmp to fail.
Attempt to avoid the failure with zero initialization.


for  libstdc++-v3/ChangeLog

* testsuite/27_io/fpos/mbstate_t/1.cc: Zero-init mbstate_t.
---
 libstdc++-v3/testsuite/27_io/fpos/mbstate_t/1.cc |   28 +-
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/libstdc++-v3/testsuite/27_io/fpos/mbstate_t/1.cc 
b/libstdc++-v3/testsuite/27_io/fpos/mbstate_t/1.cc
index f92d68f..28fec8e 100644
--- a/libstdc++-v3/testsuite/27_io/fpos/mbstate_t/1.cc
+++ b/libstdc++-v3/testsuite/27_io/fpos/mbstate_t/1.cc
@@ -28,24 +28,38 @@
 void test01()
 {
   typedef std::mbstate_t state_type;
-  state_type state01 = state_type();
-  state_type state02 = state_type();
+  // Use zero-initialization of the underlying memory so that padding
+  // bytes, if any, stand a better chance of comparing the same.
+  // Zero-initialized memory is guaranteed to be a valid initial
+  // state.  This doesn't quite guarantee that any padding bits won't
+  // be overwritten when copying from other instances that haven't
+  // been fully initialized: this data type is compatible with C, so
+  // it is likely plain old data, but it could have a default ctor
+  // that initializes only the relevant fields, whereas copy-ctor and
+  // operator= could be implemented as a full-object memcpy, including
+  // padding bits, rather than fieldwise copying.  However, since
+  // we're comparing two values copied from the same state_type
+  // instance, if padding bits are copied, we'll get the same for both
+  // of them, and if they aren't, we'll keep the values we initialized
+  // them with, so this should be good.
+  state_type state[2];
+  std::memset(state, 0, sizeof (state));
+
 
   std::streampos pos01(0);
-  std::streampos pos02(0);
 
   // 27.4.3.1 fpos members
   // void state(state_type s);
   // state_type state();
 
   // XXX Need to have better sanity checking for the mbstate_t type,
-  // or whatever the insantiating type for class fpos happens to be
+  // or whatever the instantiating type for class fpos happens to be
   // for streampos, as things like equality operators and assignment
   // operators, increment and deincrement operators need to be in
   // place.
-  pos01.state(state02);
-  state01 = pos01.state();
-  VERIFY( std::memcmp(, , sizeof(state_type)) == 0 );
+  pos01.state(state[1]);
+  state[0] = pos01.state();
+  VERIFY( std::memcmp([0], [1], sizeof(state_type)) == 0 );
 }
 
 int main() 


-- 
Alexandre Oliva, freedom fighter   he/him   https://FSFLA.org/blogs/lxo
Free Software Evangelist   Stallman was right, but he's left :(
GNU Toolchain EngineerFSMatrix: It was he who freed the first of us
FSF & FSFLA board memberThe Savior shall return (true);


Re: Define HAVE_ for math long double functions declared in vxworks headers

2020-01-23 Thread Alexandre Oliva
On Jan 23, 2020, Jonathan Wakely  wrote:

> On 23/01/20 00:20 -0300, Alexandre Oliva wrote:
>> On Jan 22, 2020, Jonathan Wakely  wrote:
>> 
>>> Isn't allowing arithmetic on function pointers a GNU extension?
>> 
>> Does that matter?  This test is only supposed to be compiled by GCC.

> Maybe if somebody was crazy enough to build GCC with -pedantic-errors?

In TFLAGS while configuring libstdc++...  Yeah, that would fail.

>>> I think just adding the #undef to what you had originally is the best
>>> version.
>> 
>> 'k, thanks, will adjust, test, post and install.

> Thanks.

Here's the revised version I'm about to install...  Regstrapped on
x86_64-linux-gnu along with other patches, also retested on an affected
target.


reject macros in math decl check

From: Alexandre Oliva 

The C++ headers #undef the functions we are testing for, just in case
they're implemented as macros, so do that in the cross math decl tests
as well.


for  libstdc++-v3/ChangeLog

* crossconfig.m4 (GLIBCXX_CHECK_MATH_DECL): Reject macros.
* configure: Rebuild.
---
 libstdc++-v3/configure  |   22 ++
 libstdc++-v3/crossconfig.m4 |1 +
 2 files changed, 23 insertions(+)

diff --git a/libstdc++-v3/crossconfig.m4 b/libstdc++-v3/crossconfig.m4
index 2a0cb04..fe18288 100644
--- a/libstdc++-v3/crossconfig.m4
+++ b/libstdc++-v3/crossconfig.m4
@@ -333,6 +333,7 @@ AC_DEFUN([GLIBCXX_CHECK_MATH_DECL], [
 #ifdef HAVE_IEEEFP_H
 # include 
 #endif
+#undef $1
 ], [
   void (*f)(void) = (void (*)(void))$1;
 ], [glibcxx_cv_func_$1_use=yes


-- 
Alexandre Oliva, freedom fighter   he/him   https://FSFLA.org/blogs/lxo
Free Software Evangelist   Stallman was right, but he's left :(
GNU Toolchain EngineerFSMatrix: It was he who freed the first of us
FSF & FSFLA board memberThe Savior shall return (true);


[PATCH] doc: target.def (flags_regnum): Mention effect on delay slot filling.

2020-01-23 Thread Hans-Peter Nilsson
gcc:
* target.def (flags_regnum): Also mention effect on delay slot filling.
* doc/tm.texi: Regenerate.

Noticed the "hard way" dealing with performance fallout for the
CRIS decc0ration.

Previously, the documentation blurb only mentioned an effect on
compare elimination.  The technical contents is obvious but I'm
more worried about grammar.  I don't see hyphenation elsewhere
when referring to delay slot filling, but then again the term
used for dbr-scheduling isn't consistent in that part of the
documentation.  Note also that it's not just branches, says the
code in reorg.c.

Dvi and info results inspected for sanity.

Ok to commit?

---
 gcc/doc/tm.texi | 4 +++-
 gcc/target.def  | 8 
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 4aec468..19985ad 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -6532,7 +6532,9 @@ returns @code{VOIDmode}.
 @end deftypefn
 
 @deftypevr {Target Hook} {unsigned int} TARGET_FLAGS_REGNUM
-If the target has a dedicated flags register, and it needs to use the 
post-reload comparison elimination pass, then this value should be set 
appropriately.
+If the target has a dedicated flags register, and it needs to use the
+post-reload comparison elimination pass, or the delay slot filler pass,
+then this value should be set appropriately.
 @end deftypevr
 
 @node Costs
diff --git a/gcc/target.def b/gcc/target.def
index 81cea0d..b5e82ff 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -3716,10 +3716,10 @@ of spill registers and print a fatal error message.",
target is constrainted to use post-reload comparison elimination.  */
 DEFHOOKPOD
 (flags_regnum,
- "If the target has a dedicated flags register, and it needs to use the\
- post-reload comparison elimination pass, then this value should be set\
- appropriately.",
- unsigned int, INVALID_REGNUM)
+ "If the target has a dedicated flags register, and it needs to use the\n\
+post-reload comparison elimination pass, or the delay slot filler pass,\n\
+then this value should be set appropriately.",
+unsigned int, INVALID_REGNUM)
 
 /* Compute a (partial) cost for rtx X.  Return true if the complete
cost has been computed, and false if subexpressions should be
-- 
2.11.0

brgds, H-P


Re: libgo patch committed: Update to Go1.14beta1

2020-01-23 Thread Maciej W. Rozycki
On Tue, 21 Jan 2020, Ian Lance Taylor wrote:

> I've committed a patch to update libgo to Go 1.14beta1.  As usual with
> these updates the patch is far too large to include in this e-mail
> message.  I've included the diffs for gccgo-specific files.

 It seems to have broken the `riscv64-linux-gnu' target:

cpugen.go:2:7: error: redefinition of 'CacheLinePadSize'
2 | const CacheLinePadSize = 64
  |   ^
.../libgo/go/internal/cpu/cpu_riscv64.go:7:7: note: previous definition of 
'CacheLinePadSize' was here
7 | const CacheLinePadSize = 32
  |   ^
make[4]: *** [Makefile:2830: internal/cpu.lo] Error 1
make[4]: Leaving directory '.../riscv64-linux-gnu/libgo'

  Maciej


Re: [PATCH] postreload: Fix up postreload combine [PR93402]

2020-01-23 Thread Jeff Law
On Thu, 2020-01-23 at 20:05 +0100, Richard Biener wrote:
> On January 23, 2020 7:22:02 PM GMT+01:00, Jakub Jelinek  
> wrote:
> > Hi!
> > 
> > The following testcase is miscompiled, because the postreload pass
> > changes:
> > -(insn 14 13 23 2 (parallel [
> > -(set (reg:DI 1 dx [94])
> > -(plus:DI (reg:DI 1 dx [95])
> > -(reg:DI 5 di [92])))
> > -(clobber (reg:CC 17 flags))
> > -]) "pr93402.c":8:30 186 {*adddi_1}
> > - (expr_list:REG_EQUAL (plus:DI (reg:DI 5 di [92])
> > -(const_int  [0x19debd01c7]))
> > -(nil)))
> > -(insn 23 14 25 2 (set (reg:SI 0 ax)
> > +(insn 23 13 25 2 (set (reg:SI 0 ax)
> > (const_int 0 [0])) "pr93402.c":10:1 67 {*movsi_internal}
> >  (nil))
> > (insn 25 23 26 2 (use (reg:SI 0 ax)) "pr93402.c":10:1 -1
> >  (nil))
> > -(insn 26 25 35 2 (use (reg:DI 1 dx)) "pr93402.c":10:1 -1
> > +(insn 26 25 35 2 (use (plus:DI (reg:DI 1 dx [95])
> > +(reg:DI 5 di [92]))) "pr93402.c":10:1 -1
> >  (nil))
> > A USE insn is not a normal insn and verify_changes called from
> > apply_change_group is happy about any changes into it.
> > The following patch avoids this optimization if we were to change
> > the USE operand (this routine only changes a reg into (plus reg reg2)).
> > 
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and
> > later for release branches?
> 
> Ok. Looks like the same thing Richard fixed for autoinc. 
Yup.  I suppose we still need the naked clobber/use insns and that
there's more of these problems lurking in the weeks.

jeff
> 



Re: [C++ Patch] PR 92804 [10 Regression] ICE trying to use concept as a nested-name-specifier

2020-01-23 Thread Jason Merrill

On 1/23/20 4:31 AM, Paolo Carlini wrote:

Hi,

On 22/01/20 22:32, Jason Merrill wrote:

On 1/22/20 3:31 PM, Paolo Carlini wrote:

Hi,

On 22/01/20 17:27, Jason Merrill wrote:

On 1/22/20 10:22 AM, Paolo Carlini wrote:

Hi,

in this simple issue we either wrongly talked about variable 
template-id in c++17 mode or ICEd in c++2a. I think we simply want 
to handle concept-ids first, both as represented in c++17 mode and 
as represented in c++2a mode. Tested x86_64-linux.
What happens if you try to use a function template/function concept 
name?


AFAICS no ICEs, no regressions but indeed we can do better, tell 
concepts from function templates. The below does that and passes 
testing but I'm not sure if the wording is optimal, whether we always 
want to talk about concept-id.


I think it's fine either way.


+// { dg-do compile { target c++17 } }
+// { dg-options "-w -fconcepts" }


Does it work to use -fconcepts-ts instead of -w -fconcepts?


Sure, it does. Attached what I tested.


OK.



[PATCH] simplify-rtx: Punt for modes with precision above MAX_BITSIZE_MODE_ANY_INT [PR93376]

2020-01-23 Thread Jakub Jelinek
Hi!

The following patch makes sure we punt in the 3 spots if precision is above
MAX_BITSIZE_MODE_ANY_INT.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2020-01-23  Jakub Jelinek  

PR target/93376
* simplify-rtx.c (simplify_const_unary_operation,
simplify_const_binary_operation): Punt for mode precision above
MAX_BITSIZE_MODE_ANY_INT.

--- gcc/simplify-rtx.c.jj   2020-01-12 11:54:36.933405563 +0100
+++ gcc/simplify-rtx.c  2020-01-23 11:52:17.292459465 +0100
@@ -1824,6 +1824,9 @@ simplify_const_unary_operation (enum rtx
   if (CONST_SCALAR_INT_P (op) && is_a  (mode, _mode))
 {
   unsigned int width = GET_MODE_PRECISION (result_mode);
+  if (width > MAX_BITSIZE_MODE_ANY_INT)
+   return 0;
+
   wide_int result;
   scalar_int_mode imode = (op_mode == VOIDmode
   ? result_mode
@@ -1968,6 +1971,9 @@ simplify_const_unary_operation (enum rtx
   && is_int_mode (mode, _mode))
 {
   unsigned int width = GET_MODE_PRECISION (result_mode);
+  if (width > MAX_BITSIZE_MODE_ANY_INT)
+   return 0;
+
   /* Although the overflow semantics of RTL's FIX and UNSIGNED_FIX
 operators are intentionally left unspecified (to ease implementation
 by target backends), for consistency, this routine implements the
@@ -4422,7 +4428,8 @@ simplify_const_binary_operation (enum rt
   scalar_int_mode int_mode;
   if (is_a  (mode, _mode)
   && CONST_SCALAR_INT_P (op0)
-  && CONST_SCALAR_INT_P (op1))
+  && CONST_SCALAR_INT_P (op1)
+  && GET_MODE_PRECISION (int_mode) <= MAX_BITSIZE_MODE_ANY_INT)
 {
   wide_int result;
   wi::overflow_type overflow;

Jakub



Re: [PATCH] postreload: Fix up postreload combine [PR93402]

2020-01-23 Thread Richard Biener
On January 23, 2020 7:22:02 PM GMT+01:00, Jakub Jelinek  
wrote:
>Hi!
>
>The following testcase is miscompiled, because the postreload pass
>changes:
>-(insn 14 13 23 2 (parallel [
>-(set (reg:DI 1 dx [94])
>-(plus:DI (reg:DI 1 dx [95])
>-(reg:DI 5 di [92])))
>-(clobber (reg:CC 17 flags))
>-]) "pr93402.c":8:30 186 {*adddi_1}
>- (expr_list:REG_EQUAL (plus:DI (reg:DI 5 di [92])
>-(const_int  [0x19debd01c7]))
>-(nil)))
>-(insn 23 14 25 2 (set (reg:SI 0 ax)
>+(insn 23 13 25 2 (set (reg:SI 0 ax)
> (const_int 0 [0])) "pr93402.c":10:1 67 {*movsi_internal}
>  (nil))
> (insn 25 23 26 2 (use (reg:SI 0 ax)) "pr93402.c":10:1 -1
>  (nil))
>-(insn 26 25 35 2 (use (reg:DI 1 dx)) "pr93402.c":10:1 -1
>+(insn 26 25 35 2 (use (plus:DI (reg:DI 1 dx [95])
>+(reg:DI 5 di [92]))) "pr93402.c":10:1 -1
>  (nil))
>A USE insn is not a normal insn and verify_changes called from
>apply_change_group is happy about any changes into it.
>The following patch avoids this optimization if we were to change
>the USE operand (this routine only changes a reg into (plus reg reg2)).
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and
>later for release branches?

Ok. Looks like the same thing Richard fixed for autoinc. 

Richard. 

>2020-01-23  Jakub Jelinek  
>
>   PR rtl-optimization/93402
>   * postreload.c (reload_combine_recognize_pattern): Don't try to adjust
>   USE insns.
>
>   * gcc.c-torture/execute/pr93402.c: New test.
>
>--- gcc/postreload.c.jj2020-01-12 11:54:36.0 +0100
>+++ gcc/postreload.c   2020-01-23 17:23:25.359929516 +0100
>@@ -1078,6 +1078,10 @@ reload_combine_recognize_pattern (rtx_in
>   struct reg_use *use = reg_state[regno].reg_use + i;
>   if (GET_MODE (*use->usep) != mode)
>   return false;
>+  /* Don't try to adjust (use (REGX)).  */
>+  if (GET_CODE (PATTERN (use->insn)) == USE
>+&&  (PATTERN (use->insn), 0) == use->usep)
>+  return false;
> }
> 
>   /* Look for (set (REGX) (CONST_INT))
>--- gcc/testsuite/gcc.c-torture/execute/pr93402.c.jj   2020-01-23
>17:25:46.496803852 +0100
>+++ gcc/testsuite/gcc.c-torture/execute/pr93402.c  2020-01-23
>17:25:05.221425501 +0100
>@@ -0,0 +1,21 @@
>+/* PR rtl-optimization/93402 */
>+
>+struct S { unsigned int a; unsigned long long b; };
>+
>+__attribute__((noipa)) struct S
>+foo (unsigned long long x)
>+{
>+  struct S ret;
>+  ret.a = 0;
>+  ret.b = x * 111ULL + ULL;
>+  return ret;
>+}
>+
>+int
>+main ()
>+{
>+  struct S a = foo (1);
>+  if (a.a != 0 || a.b != 1222ULL)
>+__builtin_abort ();
>+  return 0;
>+}
>
>   Jakub



[PATCH] postreload: Fix up postreload combine [PR93402]

2020-01-23 Thread Jakub Jelinek
Hi!

The following testcase is miscompiled, because the postreload pass changes:
-(insn 14 13 23 2 (parallel [
-(set (reg:DI 1 dx [94])
-(plus:DI (reg:DI 1 dx [95])
-(reg:DI 5 di [92])))
-(clobber (reg:CC 17 flags))
-]) "pr93402.c":8:30 186 {*adddi_1}
- (expr_list:REG_EQUAL (plus:DI (reg:DI 5 di [92])
-(const_int  [0x19debd01c7]))
-(nil)))
-(insn 23 14 25 2 (set (reg:SI 0 ax)
+(insn 23 13 25 2 (set (reg:SI 0 ax)
 (const_int 0 [0])) "pr93402.c":10:1 67 {*movsi_internal}
  (nil))
 (insn 25 23 26 2 (use (reg:SI 0 ax)) "pr93402.c":10:1 -1
  (nil))
-(insn 26 25 35 2 (use (reg:DI 1 dx)) "pr93402.c":10:1 -1
+(insn 26 25 35 2 (use (plus:DI (reg:DI 1 dx [95])
+(reg:DI 5 di [92]))) "pr93402.c":10:1 -1
  (nil))
A USE insn is not a normal insn and verify_changes called from
apply_change_group is happy about any changes into it.
The following patch avoids this optimization if we were to change
the USE operand (this routine only changes a reg into (plus reg reg2)).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and
later for release branches?

2020-01-23  Jakub Jelinek  

PR rtl-optimization/93402
* postreload.c (reload_combine_recognize_pattern): Don't try to adjust
USE insns.

* gcc.c-torture/execute/pr93402.c: New test.

--- gcc/postreload.c.jj 2020-01-12 11:54:36.0 +0100
+++ gcc/postreload.c2020-01-23 17:23:25.359929516 +0100
@@ -1078,6 +1078,10 @@ reload_combine_recognize_pattern (rtx_in
   struct reg_use *use = reg_state[regno].reg_use + i;
   if (GET_MODE (*use->usep) != mode)
return false;
+  /* Don't try to adjust (use (REGX)).  */
+  if (GET_CODE (PATTERN (use->insn)) == USE
+ &&  (PATTERN (use->insn), 0) == use->usep)
+   return false;
 }
 
   /* Look for (set (REGX) (CONST_INT))
--- gcc/testsuite/gcc.c-torture/execute/pr93402.c.jj2020-01-23 
17:25:46.496803852 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr93402.c   2020-01-23 
17:25:05.221425501 +0100
@@ -0,0 +1,21 @@
+/* PR rtl-optimization/93402 */
+
+struct S { unsigned int a; unsigned long long b; };
+
+__attribute__((noipa)) struct S
+foo (unsigned long long x)
+{
+  struct S ret;
+  ret.a = 0;
+  ret.b = x * 111ULL + ULL;
+  return ret;
+}
+
+int
+main ()
+{
+  struct S a = foo (1);
+  if (a.a != 0 || a.b != 1222ULL)
+__builtin_abort ();
+  return 0;
+}

Jakub



[COMMITTED] c++: Avoid ICE when constexpr __builtin_strchr fails.

2020-01-23 Thread Jason Merrill
If we can't change the argument to &"...", use the original arg instead of
the partially munged one.

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

PR c++/93331 - ICE with __builtin_strchr.
* constexpr.c (cxx_eval_builtin_function_call): Use the original
argument if we didn't manage to extract a STRING_CST.
---
 gcc/cp/constexpr.c | 3 +++
 .../{gcc.c-torture/compile => c-c++-common}/pr34029-1.c| 0
 2 files changed, 3 insertions(+)
 rename gcc/testsuite/{gcc.c-torture/compile => c-c++-common}/pr34029-1.c (100%)

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 5864b67d4de..f6b8f331bc9 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1293,6 +1293,7 @@ cxx_eval_builtin_function_call (const constexpr_ctx *ctx, 
tree t, tree fun,
   for (i = 0; i < nargs; ++i)
 {
   tree arg = CALL_EXPR_ARG (t, i);
+  tree oarg = arg;
 
   /* To handle string built-ins we need to pass ADDR_EXPR since
 expand_builtin doesn't know how to look in the values table.  */
@@ -1327,6 +1328,8 @@ cxx_eval_builtin_function_call (const constexpr_ctx *ctx, 
tree t, tree fun,
arg = braced_lists_to_strings (TREE_TYPE (arg), arg);
  if (TREE_CODE (arg) == STRING_CST)
arg = build_address (arg);
+ else
+   arg = oarg;
}
 
   args[i] = arg;
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr34029-1.c 
b/gcc/testsuite/c-c++-common/pr34029-1.c
similarity index 100%
rename from gcc/testsuite/gcc.c-torture/compile/pr34029-1.c
rename to gcc/testsuite/c-c++-common/pr34029-1.c

base-commit: 6d00f052ef209bacdd93f503b0c5fb428cc6c434
-- 
2.18.1



[COMMITTED] c++: Fix ICE with defaulted dtor and template (PR93345)

2020-01-23 Thread Jason Merrill
In a template we don't instantiate a deferred noexcept-spec, and we don't
need it because we aren't going to do anything with the value of
throwing_cleanup in a template anyway.

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

PR c++/93345 - ICE with defaulted dtor and template.
PR c++/33799
* decl.c (cxx_maybe_build_cleanup): Don't try to set
throwing_cleanup in a template.
---
 gcc/cp/decl.c  |  3 ++-
 gcc/testsuite/g++.dg/cpp0x/initlist-cleanup1.C | 17 +
 2 files changed, 19 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist-cleanup1.C

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 47ff7eea88f..98ed79f3579 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -17393,7 +17393,8 @@ cxx_maybe_build_cleanup (tree decl, tsubst_flags_t 
complain)
   && !mark_used (decl, complain) && !(complain & tf_error))
 return error_mark_node;
 
-  if (cleanup && cfun && !expr_noexcept_p (cleanup, tf_none))
+  if (cleanup && cfun && !processing_template_decl
+  && !expr_noexcept_p (cleanup, tf_none))
 cp_function_chain->throwing_cleanup = true;
 
   return cleanup;
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist-cleanup1.C 
b/gcc/testsuite/g++.dg/cpp0x/initlist-cleanup1.C
new file mode 100644
index 000..1e3ac122480
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist-cleanup1.C
@@ -0,0 +1,17 @@
+// PR c++/93345
+// { dg-do compile { target c++11 } }
+
+struct ln {
+  ~ln ();
+};
+
+struct ry {
+  ln kj;
+};
+
+template
+void
+dz ()
+{
+  ry{};
+}

base-commit: 6d00f052ef209bacdd93f503b0c5fb428cc6c434
-- 
2.18.1



Re: [PATCH] Add --with-diagnostics-urls configuration option and GCC_URLS env var

2020-01-23 Thread Bernd Edlinger
Hi,


I just wanted to ask, if there was any progress on this?


Thanks
Bernd.

On 12/21/19 10:50 AM, Bernd Edlinger wrote:
> Hi David,
> 
> thanks for fixing this issue.
> 
> Just one comment:
> 
>> +[DIAGNOSTICS_URLS_DEFAULT=DIAGNOSTICS_URL_AUTO])
>> +AC_DEFINE_UNQUOTED(DIAGNOSTICS_URLS_DEFAULT, $DIAGNOSTICS_URLS_DEFAULT,
>> +   [The default for -fdiagnostics-urls option])
> 
> I think for a feature as disruptive as this, when a
> terminal implements that it ought to advertise this feature via
> an environment variable.  So I would prefer the default
> to be auto-if-env.
> 
> Ideally there should be some neutral variable similar to TERM,
> maybe TERM_URL=yes if that convention does not exist yet,
> maybe we should invent it?
> 
> 
> Thanks
> Bernd.
> 


[committed] analyzer: avoid ICE with missing arguments (PR 93375)

2020-01-23 Thread David Malcolm
PR analyzer/93375 reports an ICE under certain circumstances
involving a call where the number of arguments at the callsite
is less than the parameter count of the callee,

Specifically, the ICE occurs when pruning a checker_path for a
diagnostic, when attempting to maintain which expression is of
interest through such a call.

The root cause is an assumption that there were enough arguments at
the callsite, within callgraph_superedge's methods for mapping
expressions between callee and caller.

This patch adds checks for this to the relevant methods, fixing the ICE.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu;
pushed to master as r10-6182-g648796dab42b6e839f10fee5835f24cd2016a9f4.

gcc/analyzer/ChangeLog:
PR analyzer/93375
* supergraph.cc (callgraph_superedge::get_arg_for_parm): Fail
gracefully is the number of parameters at the callee exceeds the
number of arguments at the call stmt.
(callgraph_superedge::get_parm_for_arg): Likewise.

gcc/testsuite/ChangeLog:
PR analyzer/93375
* gcc.dg/analyzer/pr93375.c: New test.
---
 gcc/analyzer/supergraph.cc  | 14 ++
 gcc/testsuite/gcc.dg/analyzer/pr93375.c | 15 +++
 2 files changed, 25 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr93375.c

diff --git a/gcc/analyzer/supergraph.cc b/gcc/analyzer/supergraph.cc
index 4660239082a..a5bf52d8aca 100644
--- a/gcc/analyzer/supergraph.cc
+++ b/gcc/analyzer/supergraph.cc
@@ -879,16 +879,19 @@ callgraph_superedge::get_arg_for_parm (tree parm_to_find,
   gcc_assert  (TREE_CODE (parm_to_find) == PARM_DECL);
 
   tree callee = get_callee_decl ();
+  const gcall *call_stmt = get_call_stmt ();
 
-  int i = 0;
+  unsigned i = 0;
   for (tree iter_parm = DECL_ARGUMENTS (callee); iter_parm;
iter_parm = DECL_CHAIN (iter_parm), ++i)
 {
+  if (i >= gimple_call_num_args (call_stmt))
+   return NULL_TREE;
   if (iter_parm == parm_to_find)
{
  if (out)
*out = callsite_expr::from_zero_based_param (i);
- return gimple_call_arg (get_call_stmt (), i);
+ return gimple_call_arg (call_stmt, i);
}
 }
 
@@ -906,12 +909,15 @@ callgraph_superedge::get_parm_for_arg (tree arg_to_find,
   callsite_expr *out) const
 {
   tree callee = get_callee_decl ();
+  const gcall *call_stmt = get_call_stmt ();
 
-  int i = 0;
+  unsigned i = 0;
   for (tree iter_parm = DECL_ARGUMENTS (callee); iter_parm;
iter_parm = DECL_CHAIN (iter_parm), ++i)
 {
-  tree param = gimple_call_arg (get_call_stmt (), i);
+  if (i >= gimple_call_num_args (call_stmt))
+   return NULL_TREE;
+  tree param = gimple_call_arg (call_stmt, i);
   if (arg_to_find == param)
{
  if (out)
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr93375.c 
b/gcc/testsuite/gcc.dg/analyzer/pr93375.c
new file mode 100644
index 000..93a3e87f2cb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/pr93375.c
@@ -0,0 +1,15 @@
+/* { dg-additional-options "-Wno-implicit-int" } */
+
+void
+en (jm)
+{
+}
+
+void
+p2 ()
+{
+  char *rl = 0;
+
+  en ();
+  __builtin_memcpy (rl, 0, sizeof (0)); /* { dg-warning "dereference of NULL" 
} */
+}
-- 
2.21.0



Re: [PATCH] wwwdocs: document scripts to access personal and vendor spaces

2020-01-23 Thread Richard Earnshaw (lists)

On 21/01/2020 18:58, Richard Earnshaw (lists) wrote:
This patch documents some of the scripts that I've published for 
managing the personal and vendor spaces on the server.  It also covers 
some of the other features that those scripts enable, so that it's all 
in one place.  This is a complete rewrite of the material I had written 
previously since before we did not have these scripts to make use of.


I've not filled in the documentation for gcc-descr and gcc-undescr, 
Jakub has agreed to provide that at a later date.


R.


I've pushed this.  I think it's better to have this in than nothing at 
all.  We can iterate on it as required.


R.


Re: [RFC c-common PATCH] PR c++/40752 - useless -Wconversion with short +=.

2020-01-23 Thread Jason Merrill

On 1/22/20 5:58 PM, Jason Merrill wrote:

On 1/22/20 1:20 PM, Jason Merrill wrote:

On 1/22/20 5:14 AM, Martin Liška wrote:

Hi.

Just for the record, after the change 526.blender_r fails due to:

blender/source/blender/blenlib/intern/math_color.c: In function 
'rgb_float_to_uchar':
blender/source/blender/blenlib/BLI_utildefines.h:251:22: error: 
conversion from 'float' to 'unsigned char' may change value 
[-Werror=float-conversion]

   251 | #define FTOCHAR(val) ((CHECK_TYPE_INLINE(val, float)), \
   |  ^
blender/source/blender/blenlib/BLI_utildefines.h:257:13: note: in 
expansion of macro 'FTOCHAR'
   257 |   (v1)[0] = 
FTOCHAR((v2[0]));   \

   | ^~~
blender/source/blender/blenlib/intern/math_color.c:421:2: note: in 
expansion of macro 'F3TOCHAR3'

   421 |  F3TOCHAR3(col_f, r_col);
   |  ^

where the project sets -Werror=float-conversion dues to:
$ cat blender/source/blender/blenlib/BLI_strict_flags.h

#ifdef __GNUC__
#  if (__GNUC__ * 100 + __GNUC_MINOR__) >= 406  /* gcc4.6+ only */
#    pragma GCC diagnostic error "-Wsign-compare"
#    pragma GCC diagnostic error "-Wconversion"
#  endif


Thanks, investigating.


I wasn't able to reproduce this particular error, but I saw a different 
one due to -funsigned-char, fixed thus:


One more tweak, tested powerpc64-linux-gnu.
commit 608199de99de8e4f64e92bdf8b96564d2adeae5e
Author: Jason Merrill 
Date:   Thu Jan 23 10:37:18 2020 -0500

c-family: One more 40752 tweak for unsigned char.

My last patch didn't fix all the failures on unsignd char targets.  We were
missing one warning because by suppressing -Wsign-conversion for the second
operand of + we missed an overflow that we want to warn about, and we
properly don't warn about unsigned / or %.

PR testsuite/93391 - PR 40752 test fails with unsigned plain char.
* c-warn.c (conversion_warning): Change -Wsign-conversion handling.
* lib/target-supports.exp (check_effective_target_unsigned_char):
New.

diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index 6c73317b4a6..9315cac683e 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -1315,10 +1315,14 @@ conversion_warning (location_t loc, tree type, tree expr, tree result)
 	for (int i = 0; i < arith_ops; ++i)
 	  {
 		tree op = TREE_OPERAND (expr, i);
-		tree opr = convert (type, op);
 		/* Avoid -Wsign-conversion for (unsigned)(x + (-1)).  */
-		bool minus = TREE_CODE (expr) == PLUS_EXPR && i == 1;
-		if (unsafe_conversion_p (type, op, opr, !minus))
+		if (TREE_CODE (expr) == PLUS_EXPR && i == 1
+		&& INTEGRAL_TYPE_P (type) && TYPE_UNSIGNED (type)
+		&& TREE_CODE (op) == INTEGER_CST
+		&& tree_int_cst_sgn (op) < 0)
+		  op = fold_build1 (NEGATE_EXPR, TREE_TYPE (op), op);
+		tree opr = convert (type, op);
+		if (unsafe_conversion_p (type, op, opr, true))
 		  goto op_unsafe;
 	  }
 	/* The operands seem safe, we might still want to warn if
diff --git a/gcc/testsuite/c-c++-common/Wconversion-pr40752a.c b/gcc/testsuite/c-c++-common/Wconversion-pr40752a.c
index cd70e34c390..8e3ffae06f6 100644
--- a/gcc/testsuite/c-c++-common/Wconversion-pr40752a.c
+++ b/gcc/testsuite/c-c++-common/Wconversion-pr40752a.c
@@ -14,9 +14,10 @@ void foo(char c, char c2)
   c *= 2;			/* { dg-warning "conversion" } */
   c *= c2;			/* { dg-warning "conversion" } */
   c /= 2;
-  c /= c2;			/* { dg-warning "conversion" } */
+  /* If char is unsigned we avoid promoting to int.  */
+  c /= c2;  /* { dg-warning "conversion" "" { target { ! unsigned_char } } } */
   c %= 2;
-  c %= c2;			/* { dg-warning "conversion" } */
+  c %= c2;  /* { dg-warning "conversion" "" { target { ! unsigned_char } } } */
   c = -c2;			/* { dg-warning "conversion" } */
   c = ~c2;			/* { dg-warning "conversion" } */
   c = c2++;
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index f65a8100da9..3ca3dd3a9e4 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -3115,6 +3115,14 @@ proc check_effective_target_dfprt { } {
 }]
 }
 
+# Return 1 iff target has unsigned plain 'char' by default.
+
+proc check_effective_target_unsigned_char {} {
+return [check_no_compiler_messages unsigned_char assembly {
+	char ar[(char)-1];
+}]
+}
+
 proc check_effective_target_powerpc_popcntb_ok { } {
 return [check_cached_effective_target powerpc_popcntb_ok {
 


Re: [PATCH] Fix comparison of trees via tree_cmp

2020-01-23 Thread Alexander Monakov
On Thu, 23 Jan 2020, David Malcolm wrote:

> Removing the assertions fixes it for me (a stage1 build, at least, and
> it then passes the testsuite).
> 
> I've made this blunder in four places in the analyzer:
> 
>   call-string.cc:162:  call_string::cmp
>   program-point.cc:461:  function_point::cmp_within_supernode

These two don't raise concerns on brief inspection.

>   engine.cc:1820:  worklist::key_t::cmp

This one tries to use signed difference of hashes as return value, this is
not going to work in general while passing your assert almost always (except
when difference of hashes is exactly -2^31). The problem is, difference of
hashes leads to non-transitive comparison and qsort_chk can catch it on a
suitable testcase.

>   region-model.cc:1878:  tree_cmp

This is the one under discussion here.

> IIRC, I added these checks as I was finding it difficult to debug
> things when qsort_chk failed - the first three of the comparators in
> the list above build on each other:  worklist::key_t::cmp uses both
> function_point::cmp_within_supernode and call_string::cmp (and various
> other things), so if the worklist qsort_chk fails, it sometimes
> required a fair bit of digging into which of the nested comparators had
> failed (also, all the void * don't help; I'd love to have a template
> that does a typesafe qsort without needing casts in the comparator).

FWIW qsort_chk_error is a separate function to make that easier: if you place a
breakpoint on qsort_chk_error you just need to step a few times to get into a
comparator that is under suspicion, and don't need to type out casts in gdb.

Alexander


Re: [PATCH][AArch64] ACLE 8-bit integer matrix multiply-accumulate intrinsics

2020-01-23 Thread Richard Sandiford
Dennis Zhang  writes:
> Hi all,
> On 16/12/2019 13:53, Dennis Zhang wrote:
>> Hi all,
>> 
>> This patch is part of a series adding support for Armv8.6-A features.
>> It depends on the Armv8.6-A effective target checking patch, 
>> https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00857.html.
>> 
>> This patch adds intrinsics for matrix multiply-accumulate operations 
>> including vmmlaq_s32, vmmlaq_u32, and vusmmlaq_s32.
>> 
>> ACLE documents are at https://developer.arm.com/docs/101028/latest
>> ISA documents are at https://developer.arm.com/docs/ddi0596/latest
>> 
>> Regtested & bootstrapped for aarch64-none-linux-gnu.
>> 
>> Is it OK for trunk please?
>> 
>
> This patch is rebased to the trunk top.
> There is no dependence on any other patches now.
> Regtested again.
>
> Is it OK for trunk please?
>
> Cheers
> Dennis
>
> gcc/ChangeLog:
>
> 2020-01-23  Dennis Zhang  
>
>   * config/aarch64/aarch64-builtins.c (TYPES_TERNOP_SSUS): New macro.
>   * config/aarch64/aarch64-simd-builtins.def (simd_smmla): New.
>   (simd_ummla, simd_usmmla): New.
>   * config/aarch64/aarch64-simd.md (aarch64_simd_mmlav16qi): New.
>   * config/aarch64/arm_neon.h (vmmlaq_s32, vmmlaq_u32): New.
>   (vusmmlaq_s32): New.
>   * config/aarch64/iterators.md (unspec): Add UNSPEC_SMATMUL,
>   UNSPEC_UMATMUL, and UNSPEC_USMATMUL.
>   (sur): Likewise.
>   (MATMUL): New iterator.
>
> gcc/testsuite/ChangeLog:
>
> 2020-01-23  Dennis Zhang  
>
>   * gcc.target/aarch64/advsimd-intrinsics/vmmla.c: New test.
>
> diff --git a/gcc/config/aarch64/aarch64-builtins.c 
> b/gcc/config/aarch64/aarch64-builtins.c
> index f0e0461b7f0..033a6d4e92f 100644
> --- a/gcc/config/aarch64/aarch64-builtins.c
> +++ b/gcc/config/aarch64/aarch64-builtins.c
> @@ -176,6 +176,10 @@ 
> aarch64_types_ternopu_imm_qualifiers[SIMD_MAX_BUILTIN_ARGS]
>= { qualifier_unsigned, qualifier_unsigned,
>qualifier_unsigned, qualifier_immediate };
>  #define TYPES_TERNOPUI (aarch64_types_ternopu_imm_qualifiers)
> +static enum aarch64_type_qualifiers
> +aarch64_types_ternop_ssus_qualifiers[SIMD_MAX_BUILTIN_ARGS]
> +  = { qualifier_none, qualifier_none, qualifier_unsigned, qualifier_none };
> +#define TYPES_TERNOP_SSUS (aarch64_types_ternop_ssus_qualifiers)
>  
>  
>  static enum aarch64_type_qualifiers
> diff --git a/gcc/config/aarch64/aarch64-simd-builtins.def 
> b/gcc/config/aarch64/aarch64-simd-builtins.def
> index 57fc5933b43..06025b110cc 100644
> --- a/gcc/config/aarch64/aarch64-simd-builtins.def
> +++ b/gcc/config/aarch64/aarch64-simd-builtins.def
> @@ -682,3 +682,8 @@
>BUILTIN_VSFDF (UNOP, frint32x, 0)
>BUILTIN_VSFDF (UNOP, frint64z, 0)
>BUILTIN_VSFDF (UNOP, frint64x, 0)
> +
> +  /* Implemented by aarch64_simd_mmlav16qi.  */
> +  VAR1 (TERNOP, simd_smmla, 0, v16qi)
> +  VAR1 (TERNOPU, simd_ummla, 0, v16qi)
> +  VAR1 (TERNOP_SSUS, simd_usmmla, 0, v16qi)
> \ No newline at end of file
> diff --git a/gcc/config/aarch64/aarch64-simd.md 
> b/gcc/config/aarch64/aarch64-simd.md
> index 2989096b170..409ec28d293 100644
> --- a/gcc/config/aarch64/aarch64-simd.md
> +++ b/gcc/config/aarch64/aarch64-simd.md
> @@ -7025,3 +7025,15 @@
>"xtn\t%0., %1."
>[(set_attr "type" "neon_shift_imm_narrow_q")]
>  )
> +
> +;; 8-bit integer matrix multiply-accumulate
> +(define_insn "aarch64_simd_mmlav16qi"
> +  [(set (match_operand:V4SI 0 "register_operand" "=w")
> + (plus:V4SI (match_operand:V4SI 1 "register_operand" "0")
> +(unspec:V4SI [(match_operand:V16QI 2 "register_operand" "w")
> +  (match_operand:V16QI 3 "register_operand" "w")]
> + MATMUL)))]
> +  "TARGET_I8MM"
> +  "mmla\\t%0.4s, %2.16b, %3.16b"
> +  [(set_attr "type" "neon_mla_s_q")]
> +)
> \ No newline at end of file

(Would be good to add the newline)

The canonical rtl order for commutative operations like plus is
to put the most complicated expression first (roughly speaking --
the rules are a bit more precise than that).  So this should be:

  [(set (match_operand:V4SI 0 "register_operand" "=w")
(plus:V4SI (unspec:V4SI [(match_operand:V16QI 2 "register_operand" "w")
 (match_operand:V16QI 3 "register_operand" "w")]
MATMUL)
   (match_operand:V4SI 1 "register_operand" "0")))]

> diff --git a/gcc/config/aarch64/arm_neon.h b/gcc/config/aarch64/arm_neon.h
> index eaba156e26c..918000d98dc 100644
> --- a/gcc/config/aarch64/arm_neon.h
> +++ b/gcc/config/aarch64/arm_neon.h
> @@ -34609,6 +34609,36 @@ vrnd64xq_f64 (float64x2_t __a)
>  
>  #pragma GCC pop_options
>  
> +/* AdvSIMD 8-bit Integer Matrix Multiply (I8MM) intrinsics.  */
> +
> +#pragma GCC push_options
> +#pragma GCC target ("arch=armv8.2-a+i8mm")
> +
> +/* Matrix Multiply-Accumulate.  */
> +
> +__extension__ extern __inline int32x4_t
> +__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> +vmmlaq_s32 (int32x4_t __r, int8x16_t __a, int8x16_t __b)
> +{
> +  return 

Re: [PATCH] Proposal for IPA init() constant propagation

2020-01-23 Thread Erick Ochoa
Forgot to attach the test cases.

On 2020-01-23 10:20 a.m., Erick Ochoa wrote:
> Hi Jan,
> 
> Here is a patch I just rebased against
> 
> commit 2589beb1d1a065f75a5515c9e2698de12a421913 (origin/master, origin/HEAD, 
> master)
> Author: GCC Administrator 
> Date:   Sun Jan 19 00:16:30 2020 +
> 
> I also include some test cases.
> To run test cases, just set your path for the gcc
> executable compiled with the patch and run make all.
> The test verify the semantics of C code haven't changed.
> 
> If ./test $number prints out several "SUCCESS", we haven't
> changed the semantics.
> If there is at least 1 FAILURE, the semantics have changed.
> I just ran and there is all "SUCCESS".
> 
> To verify that the variables have indeed been propagated,
> you can do: grep -n -R 'Eliminated' and it should eliminate
> variables prefixed with "SUCCESS_{0,1,2,3}".
> Eliminating variables prefixed with "FAILURE_" is indicative
> of unsound changes. After running the tests, I confirm the
> patch eliminated the variables targeted.
> 
> 
> Please let me know if you have any questions.
> 
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index b1423d1dbfd..a9bdc162e63 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -1400,6 +1400,7 @@ OBJS = \
>   internal-fn.o \
>   ipa-cp.o \
>   ipa-sra.o \
> + ipa-initcall-cp.o \
>   ipa-devirt.o \
>   ipa-fnsummary.o \
>   ipa-polymorphic-call.o \
> diff --git a/gcc/cgraph.c b/gcc/cgraph.c
> index 95b523d6be5..82cdf660f07 100644
> --- a/gcc/cgraph.c
> +++ b/gcc/cgraph.c
> @@ -3626,6 +3626,7 @@ cgraph_node::get_untransformed_body (void)
>name = lto_get_decl_name_mapping (file_data, name);
>struct lto_in_decl_state *decl_state
>= lto_get_function_in_decl_state (file_data, decl);
> +  //gcc_assert (decl_state != NULL);
>  
>cgraph_node *origin = this;
>while (origin->clone_of)
> diff --git a/gcc/cgraph.h b/gcc/cgraph.h
> index 0ace13df1f9..e9c96fc5d32 100644
> --- a/gcc/cgraph.h
> +++ b/gcc/cgraph.h
> @@ -937,9 +937,11 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node : 
> public symtab_node
>split_part (false), indirect_call_target (false), local (false),
>versionable (false), can_change_signature (false),
>redefined_extern_inline (false), tm_may_enter_irr (false),
> -  ipcp_clone (false), m_uid (uid), m_summary_id (-1)
> +  ipcp_clone (false), m_uid (uid), m_summary_id (-1), skip_ipa_cp(false)
>{}
>  
> +  bool skip_ipa_cp;
> +
>/* Remove the node from cgraph and all inline clones inlined into it.
>   Skip however removal of FORBIDDEN_NODE and return true if it needs to be
>   removed.  This allows to call the function from outer loop walking clone
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 5692cd04374..6ee9852d6b4 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -3376,4 +3376,10 @@ fipa-ra
>  Common Report Var(flag_ipa_ra) Optimization
>  Use caller save register across calls if possible.
>  
> +finitcall-cp
> +Common Report Var(flag_initcall_cp) TBD.
> +
> +finitcall-cp-dryrun
> +Common Report Var(flag_initcall_cp_dryrun) TBD.
> +
>  ; This comment is to ensure we retain the blank line above.
> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
> index 17da1d8e8a7..13311c24e43 100644
> --- a/gcc/ipa-cp.c
> +++ b/gcc/ipa-cp.c
> @@ -1188,7 +1188,7 @@ initialize_node_lattices (struct cgraph_node *node)
>  
>gcc_checking_assert (node->has_gimple_body_p ());
>  
> -  if (!ipa_get_param_count (info))
> +  if (!ipa_get_param_count (info) || node->skip_ipa_cp)
>  disable = true;
>else if (node->local)
>  {
> diff --git a/gcc/ipa-initcall-cp.c b/gcc/ipa-initcall-cp.c
> new file mode 100644
> index 000..264bf38549d
> --- /dev/null
> +++ b/gcc/ipa-initcall-cp.c
> @@ -0,0 +1,1467 @@
> +/* Initcall constant propagation pass.  */
> +
> +#include "config.h"
> +#include "system.h"
> +#include "coretypes.h"
> +#include "backend.h"
> +#include "tree.h"
> +#include "tree-eh.h"
> +#include "gimple.h"
> +#include "gimple-expr.h"
> +#include "gimple-iterator.h"
> +#include "predict.h"
> +#include "alloc-pool.h"
> +#include "tree-pass.h"
> +#include "cgraph.h"
> +#include "diagnostic.h"
> +#include "fold-const.h"
> +#include "gimple-fold.h"
> +#include "symbol-summary.h"
> +#include "tree-vrp.h"
> +#include "ipa-prop.h"
> +#include "tree-pretty-print.h"
> +#include "tree-inline.h"
> +#include "ipa-fnsummary.h"
> +#include "ipa-utils.h"
> +#include "tree-ssa-ccp.h"
> +#include "stringpool.h"
> +#include "attribs.h"
> +#include "gimple-pretty-print.h"
> +#include "ssa.h"
> +
> +bool
> +reach_nodes_via_bb1 (cgraph_node *n2);
> +static bool
> +bb1_dominates_bb2 (basic_block, basic_block, cgraph_node *);
> +static bool
> +write_before_call (struct ipa_ref *, struct ipa_ref *);
> +static bool
> +call_before_read (struct ipa_ref *, struct ipa_ref *);
> +static hash_map *clone_num_suffixes1;
> +static hash_map *func_to_clone;
> +static vec
> +get_nondominated_callees 

Re: [PATCH] Proposal for IPA init() constant propagation

2020-01-23 Thread Erick Ochoa
Hi Jan,

Here is a patch I just rebased against

commit 2589beb1d1a065f75a5515c9e2698de12a421913 (origin/master, origin/HEAD, 
master)
Author: GCC Administrator 
Date:   Sun Jan 19 00:16:30 2020 +

I also include some test cases.
To run test cases, just set your path for the gcc
executable compiled with the patch and run make all.
The test verify the semantics of C code haven't changed.

If ./test $number prints out several "SUCCESS", we haven't
changed the semantics.
If there is at least 1 FAILURE, the semantics have changed.
I just ran and there is all "SUCCESS".

To verify that the variables have indeed been propagated,
you can do: grep -n -R 'Eliminated' and it should eliminate
variables prefixed with "SUCCESS_{0,1,2,3}".
Eliminating variables prefixed with "FAILURE_" is indicative
of unsound changes. After running the tests, I confirm the
patch eliminated the variables targeted.


Please let me know if you have any questions.

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index b1423d1dbfd..a9bdc162e63 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1400,6 +1400,7 @@ OBJS = \
internal-fn.o \
ipa-cp.o \
ipa-sra.o \
+   ipa-initcall-cp.o \
ipa-devirt.o \
ipa-fnsummary.o \
ipa-polymorphic-call.o \
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 95b523d6be5..82cdf660f07 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -3626,6 +3626,7 @@ cgraph_node::get_untransformed_body (void)
   name = lto_get_decl_name_mapping (file_data, name);
   struct lto_in_decl_state *decl_state
 = lto_get_function_in_decl_state (file_data, decl);
+  //gcc_assert (decl_state != NULL);
 
   cgraph_node *origin = this;
   while (origin->clone_of)
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 0ace13df1f9..e9c96fc5d32 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -937,9 +937,11 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node : public 
symtab_node
   split_part (false), indirect_call_target (false), local (false),
   versionable (false), can_change_signature (false),
   redefined_extern_inline (false), tm_may_enter_irr (false),
-  ipcp_clone (false), m_uid (uid), m_summary_id (-1)
+  ipcp_clone (false), m_uid (uid), m_summary_id (-1), skip_ipa_cp(false)
   {}
 
+  bool skip_ipa_cp;
+
   /* Remove the node from cgraph and all inline clones inlined into it.
  Skip however removal of FORBIDDEN_NODE and return true if it needs to be
  removed.  This allows to call the function from outer loop walking clone
diff --git a/gcc/common.opt b/gcc/common.opt
index 5692cd04374..6ee9852d6b4 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -3376,4 +3376,10 @@ fipa-ra
 Common Report Var(flag_ipa_ra) Optimization
 Use caller save register across calls if possible.
 
+finitcall-cp
+Common Report Var(flag_initcall_cp) TBD.
+
+finitcall-cp-dryrun
+Common Report Var(flag_initcall_cp_dryrun) TBD.
+
 ; This comment is to ensure we retain the blank line above.
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 17da1d8e8a7..13311c24e43 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1188,7 +1188,7 @@ initialize_node_lattices (struct cgraph_node *node)
 
   gcc_checking_assert (node->has_gimple_body_p ());
 
-  if (!ipa_get_param_count (info))
+  if (!ipa_get_param_count (info) || node->skip_ipa_cp)
 disable = true;
   else if (node->local)
 {
diff --git a/gcc/ipa-initcall-cp.c b/gcc/ipa-initcall-cp.c
new file mode 100644
index 000..264bf38549d
--- /dev/null
+++ b/gcc/ipa-initcall-cp.c
@@ -0,0 +1,1467 @@
+/* Initcall constant propagation pass.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "backend.h"
+#include "tree.h"
+#include "tree-eh.h"
+#include "gimple.h"
+#include "gimple-expr.h"
+#include "gimple-iterator.h"
+#include "predict.h"
+#include "alloc-pool.h"
+#include "tree-pass.h"
+#include "cgraph.h"
+#include "diagnostic.h"
+#include "fold-const.h"
+#include "gimple-fold.h"
+#include "symbol-summary.h"
+#include "tree-vrp.h"
+#include "ipa-prop.h"
+#include "tree-pretty-print.h"
+#include "tree-inline.h"
+#include "ipa-fnsummary.h"
+#include "ipa-utils.h"
+#include "tree-ssa-ccp.h"
+#include "stringpool.h"
+#include "attribs.h"
+#include "gimple-pretty-print.h"
+#include "ssa.h"
+
+bool
+reach_nodes_via_bb1 (cgraph_node *n2);
+static bool
+bb1_dominates_bb2 (basic_block, basic_block, cgraph_node *);
+static bool
+write_before_call (struct ipa_ref *, struct ipa_ref *);
+static bool
+call_before_read (struct ipa_ref *, struct ipa_ref *);
+static hash_map *clone_num_suffixes1;
+static hash_map *func_to_clone;
+static vec
+get_nondominated_callees (cgraph_node *caller, cgraph_node *callee,
+ bool *exitEarly = NULL);
+
+static bool
+comdat_can_be_unshared_p_1 (symtab_node *node)
+{
+  if (!node->externally_visible)
+return true;
+  if (node->address_can_be_compared_p ())
+{
+  struct ipa_ref *ref;
+
+  for (unsigned int i = 0; node->iterate_referring (i, ref); 

Re: [PATCH] Fix patchable-function-entry on arc

2020-01-23 Thread Jeff Law
On Wed, 2020-01-22 at 17:25 -0800, apin...@marvell.com wrote:
> From: Andrew Pinski 
> 
> The problem here is arc looks at current_output_insn unconditional
> but sometimes current_output_insn is NULL.  With patchable-function-entry,
> it will be. This is similar to how the nios2, handles "%.".
> 
> Committed as obvious after a simple test with -fpatchable-function-entry=1.
I almost did this myself, but just didn't feel comfortable ;-)

jeff
> 



Re: [PATCH 2/2][MIPS][RFC] Emit .note.GNU-stack for hard-float linux targets.

2020-01-23 Thread Jeff Law
On Thu, 2020-01-23 at 10:58 +, Dragan Mladjenovic wrote:
> On 07.12.2019. 19:33, Jeff Law wrote:
> > On Thu, 2019-11-07 at 17:05 +, Dragan Mladjenovic wrote:
> > > On 01.11.2019. 11:32, Dragan Mladjenovic wrote:
> > > > On 10.08.2019. 00:15, Joseph Myers wrote:
> > > > > On Fri, 9 Aug 2019, Jeff Law wrote:
> > > > > 
> > > > > > > 2019-08-05  Dragan Mladjenovic  
> > > > > > > 
> > > > > > >  * config.in: Regenerated.
> > > > > > >  * config/mips/linux.h (NEED_INDICATE_EXEC_STACK): Define
> > > > > > > to 1
> > > > > > >  for TARGET_LIBC_GNUSTACK.
> > > > > > >  * configure: Regenerated.
> > > > > > >  * configure.ac: Define TARGET_LIBC_GNUSTACK if glibc
> > > > > > > version is
> > > > > > >  found 2.31 or greater.
> > > > > > My only concern here is the configure bits.  So for example,
> > > > > > will it do
> > > > > > the right thing if you're cross-compiling to a MIPS linux
> > > > > > target?  If
> > > > > > so, how?  If not, do we need to make it a first class configure
> > > > > > option
> > > > > > so that it can be specified when building cross MIPS linux
> > > > > > toolchains?
> > > > > 
> > > > > The key point of using GCC_GLIBC_VERSION_GTE_IFELSE is that (a)
> > > > > it checks
> > > > > the target glibc headers if available when GCC is built and (b)
> > > > > if not
> > > > > available, you can still use --with-glibc-version when
> > > > > configuring
> > > > > GCC, to
> > > > > get the right configuration in a bootstrap compiler built before
> > > > > glibc is
> > > > > built (the latter is necessary on some architectures to get the
> > > > > right
> > > > > stack-protector configuration for bootstrapping glibc, but may be
> > > > > useful
> > > > > in other cases as well).
> > > > > 
> > > > > My main concern about this patch is the one I gave in
> > > > > 
> > > > > about what
> > > > > the configuration mechanism should be, on a whole-toolchain
> > > > > level, to say
> > > > > whether you are OK with a requirement for a 4.8 or later kernel.
> > > > > 
> > > > 
> > > > Sorry for the late reply.
> > > > 
> > > > I was waiting to backport [1] to most of the glibc release branches
> > > > in
> > > > use, but I got sidetracked along the way.
> > > > 
> > > > After this patch lands the preferred way to configure gcc would be
> > > > using
> > > > --with-glibc-version=2.31 and to use said glibc.
> > > > If the user/distribution can live with minimal kernel requirement
> > > > of 4.8
> > > > the glibc used should be configured with --enable-kernel=4.8.
> > > > I also plan to backport the [1] to limit the opportunity for
> > > > building
> > > > the possibly broken glibc with the gcc w/ enabled .note.GNU-stack.
> > > > 
> > > > This is all tedious and user has to be aware of all of it to make
> > > > it
> > > > work, but hopefully over time the distributions will default to
> > > > --with-glibc-version=2.31 and --enable-kernel=4.8. I guess
> > > > providing the
> > > > detailed NEWS entry for this change would help a bit.
> > > > 
> > > > Is there any objections to getting this on the trunk before the end
> > > > of
> > > > stage1?
> > > > 
> > > > [1] https://sourceware.org/ml/libc-alpha/2019-08/msg00639.html
> > > > 
> > > 
> > > Small update and gentle ping. The glibc change was backported all
> > > the
> > > way back to 2.24.
> > I think this is fine.  And yes, we'd like to get it mentioned in the
> > release notes since I suspect folks will want the GNU-stack ELF notes.
> > 
> > jeff
> > 
> > 
> 
> I left this to fall through the cracks once again. In light of [1], is 
> there any leeway for me to push this now?
> 
> [1] https://gcc.gnu.org/ml/gcc/2020-01/msg00199.html
We generally allow more leeway for the ports simply because getting
something wrong doesn't have as wide an impact.  Additionally, you're
using a configure-time flag to turn on the new behavior which further
limits the impact if something were to go wrong.

So if you want to commit it now, go ahead

jeff
> 
> 



Re: [PATCH] Fix comparison of trees via tree_cmp

2020-01-23 Thread David Malcolm
On Wed, 2020-01-22 at 19:02 +0100, Jakub Jelinek wrote:
> On Wed, Jan 22, 2020 at 08:08:32PM +0300, Alexander Monakov wrote:
> > 
> > On Wed, 22 Jan 2020, Stefan Schulze Frielinghaus wrote:
> > 
> > > Hi David,
> > > 
> > > In function `tree_cmp` an invariant [1] is assumed which does not
> > > necessarily
> > > exist. In case both input trees are finally compared via
> > > `strcmp`, then
> > > 
> > >   tree_cmp (t1, t2) == -tree_cmp (t2, t1)
> > > 
> > > does not hold in general, since function `strcmp (x, y)`
> > > guarantees only that a
> > > negative integer is returned in case x > > case x>y.
> > > Currently this breaks s390x where, for example, for certain
> > > inputs x,y
> > > `tree_cmp (x, y) == 1` and `tree_cmp (y, x) == -2` hold.  The
> > > attached patch
> > > normalizes the output from `strcmp` to -1, 0, or 1 while using an
> > > auxiliary
> > > function `sign` (stolen from the Hacker's Delight book ;-)).
> > > 
> > > Bootstrapped and tested on s390x. Any thoughts?
> > 
> > It's more appropriate to fix the assert rather than the comparator,
> > like
> > 
> >   gcc_assert (sign (reversed) == -sign (result));
> > 
> > But qsort_chk already checks that, and more, so why is the assert
> > there?
> > Shouldn't it be simply removed?
> 
> Yeah.  Note there is also return DECL_UID (t1) - DECL_UID (t2); that
> also
> doesn't guarantee -1/0/1 return values, so the patch as posted isn't
> sufficient.

Sorry about the breakage; to be specific, I've reproduced this on
s390x-ibm-linux-gnu, where it fails the selftests in stage 1, breaking
the build (unless configured with --disable-analyzer).

Removing the assertions fixes it for me (a stage1 build, at least, and
it then passes the testsuite).

I've made this blunder in four places in the analyzer:

  call-string.cc:162:  call_string::cmp
  engine.cc:1820:  worklist::key_t::cmp
  program-point.cc:461:  function_point::cmp_within_supernode
  region-model.cc:1878:  tree_cmp

IIRC, I added these checks as I was finding it difficult to debug
things when qsort_chk failed - the first three of the comparators in
the list above build on each other:  worklist::key_t::cmp uses both
function_point::cmp_within_supernode and call_string::cmp (and various
other things), so if the worklist qsort_chk fails, it sometimes
required a fair bit of digging into which of the nested comparators had
failed (also, all the void * don't help; I'd love to have a template
that does a typesafe qsort without needing casts in the comparator).

Some approaches for fixing this:
(a) I can simply remove these checks on all the comparators
(b) I could fix the checks so that they merely look at the signedness
of the results
(c) Remove the checks from tree_cmp, but retain them in the other
places, fixing them to just look at signedness, for ease of debugging.

I think I prefer (c) as it makes debugging failures easier, though I
appreciate it's rather redundant.

Thoughts?

Dave



Re: [PATCH] wide-int: i386: Fix ICEs on TImode signed overflow add/sub patterns [PR93376]

2020-01-23 Thread Uros Bizjak
On Thu, Jan 23, 2020 at 2:17 PM Jakub Jelinek  wrote:
>
> On Thu, Jan 23, 2020 at 10:38:31AM +0100, Uros Bizjak wrote:
> > On Thu, Jan 23, 2020 at 10:33 AM Jakub Jelinek  wrote:
> > >
> > > On Thu, Jan 23, 2020 at 09:14:42AM +, Richard Sandiford wrote:
> > > > > The other patch is something suggested by Richard S., avoid using 
> > > > > OImode
> > > > > for this and instead use a partial int mode that is smaller.  This is 
> > > > > still
> > > > > playing with fire because even the partial int mode is larger than
> > > > > MAX_BITSIZE_MODE_ANY_INT, but at least its precision fits into those 3
> > > > > WIDE_INT_MAX_ELTS wide-int.h uses.
> > > >
> > > > I think we should increase MAX_BITSIZE_MODE_ANY_INT to the precision
> > > > of POImode at the same time, and make that precision less than 192
> > > > (191 maybe?).  That way everything is "correct" without increasing
> > > > the size of wide_int.
> > >
> > > The 192 was chosen just to be nice and round, I guess it could be just 130
> > > or say 160 (128 + 32).
> > >
> > > Uros, would you be ok with bumping MAX_BITSIZE_MODE_ANY_INT to 160
> > > and changing the POImode patch to use 160 bits instead of 192
> > > if it passes testing?
> >
> > We can try 160 and hope that nothing breaks due to "weird" number.
>
> Following passed bootstrap/regtest on x86_64-linux and i686-linux.
>
> Ok for trunk then?
>
> 2020-01-23  Jakub Jelinek  
>
> PR target/93376
> * config/i386/i386-modes.def (POImode): New mode.
> (MAX_BITSIZE_MODE_ANY_INT): Change from 128 to 160.
> * config/i386/i386.md (DPWI): New mode attribute.
> (addv4, subv4): Use  instead of .
> (QWI): Rename to...
> (QPWI): ... this.  Use POI instead of OI for TImode.
> (*addv4_doubleword, *addv4_doubleword_1,
> *subv4_doubleword, *subv4_doubleword_1): Use 
> instead of .
>
> * gcc.dg/pr93376.c: New test.

LGTM.

Thanks,
Uros.

> --- gcc/config/i386/i386-modes.def.jj   2020-01-22 16:10:29.812837184 +0100
> +++ gcc/config/i386/i386-modes.def  2020-01-23 11:43:37.931345071 +0100
> @@ -107,10 +107,19 @@ INT_MODE (XI, 64);
>  PARTIAL_INT_MODE (HI, 16, P2QI);
>  PARTIAL_INT_MODE (SI, 32, P2HI);
>
> +/* Mode used for signed overflow checking of TImode.  As
> +   MAX_BITSIZE_MODE_ANY_INT is only 160, wide-int.h reserves only that
> +   rounded up to multiple of HOST_BITS_PER_WIDE_INT bits in wide_int etc.,
> +   so OImode is too large.  For the overflow checking we actually need
> +   just 1 or 2 bits beyond TImode precision.  Use 160 bits to have
> +   a multiple of 32.  */
> +PARTIAL_INT_MODE (OI, 160, POI);
> +
>  /* Keep the OI and XI modes from confusing the compiler into thinking
> that these modes could actually be used for computation.  They are
> -   only holders for vectors during data movement.  */
> -#define MAX_BITSIZE_MODE_ANY_INT (128)
> +   only holders for vectors during data movement.  Include POImode precision
> +   though.  */
> +#define MAX_BITSIZE_MODE_ANY_INT (160)
>
>  /* The symbol Pmode stands for one of the above machine modes (usually 
> SImode).
> The tm.h file specifies which one.  It is not a distinct mode.  */
> --- gcc/config/i386/i386.md.jj  2020-01-22 16:10:29.815837139 +0100
> +++ gcc/config/i386/i386.md 2020-01-23 11:40:54.923822116 +0100
> @@ -6054,15 +6054,18 @@ (define_insn "*addqi_ext_2"
>[(set_attr "type" "alu")
> (set_attr "mode" "QI")])
>
> +;; Like DWI, but use POImode instead of OImode.
> +(define_mode_attr DPWI [(QI "HI") (HI "SI") (SI "DI") (DI "TI") (TI "POI")])
> +
>  ;; Add with jump on overflow.
>  (define_expand "addv4"
>[(parallel [(set (reg:CCO FLAGS_REG)
>(eq:CCO
> -(plus:
> -  (sign_extend:
> +(plus:
> +  (sign_extend:
>  (match_operand:SWIDWI 1 "nonimmediate_operand"))
>(match_dup 4))
> -(sign_extend:
> +(sign_extend:
>(plus:SWIDWI (match_dup 1)
>  (match_operand:SWIDWI 2
>"")
> @@ -6078,7 +6081,7 @@ (define_expand "addv4"
>if (CONST_SCALAR_INT_P (operands[2]))
>  operands[4] = operands[2];
>else
> -operands[4] = gen_rtx_SIGN_EXTEND (mode, operands[2]);
> +operands[4] = gen_rtx_SIGN_EXTEND (mode, operands[2]);
>  })
>
>  (define_insn "*addv4"
> @@ -6123,17 +6126,17 @@ (define_insn "addv4_1"
>   (const_string "")))])
>
>  ;; Quad word integer modes as mode attribute.
> -(define_mode_attr QWI [(SI "TI") (DI "OI")])
> +(define_mode_attr QPWI [(SI "TI") (DI "POI")])
>
>  (define_insn_and_split "*addv4_doubleword"
>[(set (reg:CCO FLAGS_REG)
> (eq:CCO
> - (plus:
> -   (sign_extend:
> + (plus:
> +   (sign_extend:
>   (match_operand: 1 "nonimmediate_operand" "%0,0"))
> - 

Re: [PATCH][AArch64] ACLE 8-bit integer matrix multiply-accumulate intrinsics

2020-01-23 Thread Dennis Zhang
Hi all,

On 16/12/2019 13:53, Dennis Zhang wrote:
> Hi all,
> 
> This patch is part of a series adding support for Armv8.6-A features.
> It depends on the Armv8.6-A effective target checking patch, 
> https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00857.html.
> 
> This patch adds intrinsics for matrix multiply-accumulate operations 
> including vmmlaq_s32, vmmlaq_u32, and vusmmlaq_s32.
> 
> ACLE documents are at https://developer.arm.com/docs/101028/latest
> ISA documents are at https://developer.arm.com/docs/ddi0596/latest
> 
> Regtested & bootstrapped for aarch64-none-linux-gnu.
> 
> Is it OK for trunk please?
> 

This patch is rebased to the trunk top.
There is no dependence on any other patches now.
Regtested again.

Is it OK for trunk please?

Cheers
Dennis

gcc/ChangeLog:

2020-01-23  Dennis Zhang  

* config/aarch64/aarch64-builtins.c (TYPES_TERNOP_SSUS): New macro.
* config/aarch64/aarch64-simd-builtins.def (simd_smmla): New.
(simd_ummla, simd_usmmla): New.
* config/aarch64/aarch64-simd.md (aarch64_simd_mmlav16qi): New.
* config/aarch64/arm_neon.h (vmmlaq_s32, vmmlaq_u32): New.
(vusmmlaq_s32): New.
* config/aarch64/iterators.md (unspec): Add UNSPEC_SMATMUL,
UNSPEC_UMATMUL, and UNSPEC_USMATMUL.
(sur): Likewise.
(MATMUL): New iterator.

gcc/testsuite/ChangeLog:

2020-01-23  Dennis Zhang  

* gcc.target/aarch64/advsimd-intrinsics/vmmla.c: New test.
diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
index f0e0461b7f0..033a6d4e92f 100644
--- a/gcc/config/aarch64/aarch64-builtins.c
+++ b/gcc/config/aarch64/aarch64-builtins.c
@@ -176,6 +176,10 @@ aarch64_types_ternopu_imm_qualifiers[SIMD_MAX_BUILTIN_ARGS]
   = { qualifier_unsigned, qualifier_unsigned,
   qualifier_unsigned, qualifier_immediate };
 #define TYPES_TERNOPUI (aarch64_types_ternopu_imm_qualifiers)
+static enum aarch64_type_qualifiers
+aarch64_types_ternop_ssus_qualifiers[SIMD_MAX_BUILTIN_ARGS]
+  = { qualifier_none, qualifier_none, qualifier_unsigned, qualifier_none };
+#define TYPES_TERNOP_SSUS (aarch64_types_ternop_ssus_qualifiers)
 
 
 static enum aarch64_type_qualifiers
diff --git a/gcc/config/aarch64/aarch64-simd-builtins.def b/gcc/config/aarch64/aarch64-simd-builtins.def
index 57fc5933b43..06025b110cc 100644
--- a/gcc/config/aarch64/aarch64-simd-builtins.def
+++ b/gcc/config/aarch64/aarch64-simd-builtins.def
@@ -682,3 +682,8 @@
   BUILTIN_VSFDF (UNOP, frint32x, 0)
   BUILTIN_VSFDF (UNOP, frint64z, 0)
   BUILTIN_VSFDF (UNOP, frint64x, 0)
+
+  /* Implemented by aarch64_simd_mmlav16qi.  */
+  VAR1 (TERNOP, simd_smmla, 0, v16qi)
+  VAR1 (TERNOPU, simd_ummla, 0, v16qi)
+  VAR1 (TERNOP_SSUS, simd_usmmla, 0, v16qi)
\ No newline at end of file
diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md
index 2989096b170..409ec28d293 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -7025,3 +7025,15 @@
   "xtn\t%0., %1."
   [(set_attr "type" "neon_shift_imm_narrow_q")]
 )
+
+;; 8-bit integer matrix multiply-accumulate
+(define_insn "aarch64_simd_mmlav16qi"
+  [(set (match_operand:V4SI 0 "register_operand" "=w")
+	(plus:V4SI (match_operand:V4SI 1 "register_operand" "0")
+		   (unspec:V4SI [(match_operand:V16QI 2 "register_operand" "w")
+ (match_operand:V16QI 3 "register_operand" "w")]
+		MATMUL)))]
+  "TARGET_I8MM"
+  "mmla\\t%0.4s, %2.16b, %3.16b"
+  [(set_attr "type" "neon_mla_s_q")]
+)
\ No newline at end of file
diff --git a/gcc/config/aarch64/arm_neon.h b/gcc/config/aarch64/arm_neon.h
index eaba156e26c..918000d98dc 100644
--- a/gcc/config/aarch64/arm_neon.h
+++ b/gcc/config/aarch64/arm_neon.h
@@ -34609,6 +34609,36 @@ vrnd64xq_f64 (float64x2_t __a)
 
 #pragma GCC pop_options
 
+/* AdvSIMD 8-bit Integer Matrix Multiply (I8MM) intrinsics.  */
+
+#pragma GCC push_options
+#pragma GCC target ("arch=armv8.2-a+i8mm")
+
+/* Matrix Multiply-Accumulate.  */
+
+__extension__ extern __inline int32x4_t
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
+vmmlaq_s32 (int32x4_t __r, int8x16_t __a, int8x16_t __b)
+{
+  return __builtin_aarch64_simd_smmlav16qi (__r, __a, __b);
+}
+
+__extension__ extern __inline uint32x4_t
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
+vmmlaq_u32 (uint32x4_t __r, uint8x16_t __a, uint8x16_t __b)
+{
+  return __builtin_aarch64_simd_ummlav16qi_ (__r, __a, __b);
+}
+
+__extension__ extern __inline int32x4_t
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
+vusmmlaq_s32 (int32x4_t __r, uint8x16_t __a, int8x16_t __b)
+{
+  return __builtin_aarch64_simd_usmmlav16qi_ssus (__r, __a, __b);
+}
+
+#pragma GCC pop_options
+
 #include "arm_bf16.h"
 
 #undef __aarch64_vget_lane_any
diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
index b9843b83c5f..57aca36f646 100644
--- a/gcc/config/aarch64/iterators.md
+++ b/gcc/config/aarch64/iterators.md
@@ 

Re: [PATCH] Add OpenACC 2.6 `acc_get_property' support

2020-01-23 Thread Thomas Schwinge
Hi Frederik!

On 2020-01-20T15:01:01+0100, "Harwath, Frederik"  
wrote:
> I have attached a patch containing the changes that you suggested.

> On 16.01.20 17:00, Thomas Schwinge wrote:
>> On 2019-12-20T17:46:57+0100, "Harwath, Frederik"  
>> wrote:
> Ok to push the commit to master?

Thanks, OK.  Reviewed-by: Thomas Schwinge 


As a low-priority follow-up, please look into:


source-gcc/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-aux.c: 
In function 'expect_device_properties':

source-gcc/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-aux.c:74:24:
 warning: format '%d' expects argument of type 'int', but argument 3 has type 
'const char *' [-Wformat=]
   74 |   fprintf (stderr, "Expected value of unknown string property 
to be NULL, "
  |
^~~~
   75 | "but was %d.\n", s);
  |  ~
  |  |
  |  const char *

source-gcc/libgomp/testsuite/libgomp.oacc-c-c++-common/acc_get_property-aux.c:75:19:
 note: format string is defined here
   75 | "but was %d.\n", s);
  |  ~^
  |   |
  |   int
  |  %s

..., and (random example):

>int unknown_property = 16058;
> -  int v = acc_get_property (dev_num, dev_type, 
> (acc_device_property_t)unknown_property);
> +  size_t v = acc_get_property (dev_num, dev_type, 
> (acc_device_property_t)unknown_property);
>if (v != 0)
>  {
>fprintf (stderr, "Expected value of unknown numeric property to equal 
> 0, "
> -"but was %d.\n", v);
> +"but was %zd.\n", v);
>abort ();
>  }

..., shouldn't that be '%zu' given that 'size_t' is 'unsigned'?

libgomp.oacc-c-c++-common/acc_get_property-aux.c:  fprintf (stderr, 
"Expected acc_property_memory to equal %zd, "
libgomp.oacc-c-c++-common/acc_get_property-aux.c:"but was 
%zd.\n", expected_memory, total_mem);
libgomp.oacc-c-c++-common/acc_get_property-aux.c:", but free 
memory was %zd and total memory was %zd.\n",
libgomp.oacc-c-c++-common/acc_get_property-aux.c:"but was 
%zd.\n", v);
libgomp.oacc-c-c++-common/acc_get_property.c:  printf ("Total 
memory: %zd\n", v);
libgomp.oacc-c-c++-common/acc_get_property.c:  printf ("Free 
memory: %zd\n", v);


Grüße
 Thomas


signature.asc
Description: PGP signature


[PATCH] Filter out language specific options from --help=common.

2020-01-23 Thread Martin Liška

Hi.

The following is attempt to fix the PR. Idea is such that
all non-driver Common options that are also used in context
of a languages should not be printed in --help=common. An
exception would be an option that will be included in all languages.

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

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2020-01-23  Martin Liska  

PR driver/91220
* opts.c (print_filtered_help): Exclude language-specific
options from --help=common unless enabled in all FEs.
---
 gcc/opts.c | 8 
 1 file changed, 8 insertions(+)


diff --git a/gcc/opts.c b/gcc/opts.c
index 3b2cc854af1..7affeb41a96 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -1285,6 +1285,14 @@ print_filtered_help (unsigned int include_flags,
 			   | CL_COMMON | CL_TARGET)) == 0)
 	continue;
 
+  /* If an option contains a language specification,
+	 exclude it from common unless all languages are present.  */
+  if ((include_flags & CL_COMMON)
+	  && !(option->flags & CL_DRIVER)
+	  && (option->flags & CL_LANG_ALL)
+	  && (option->flags & CL_LANG_ALL) != CL_LANG_ALL)
+	continue;
+
   found = true;
   /* Skip switches that have already been printed.  */
   if (opts->x_help_printed[i])



[PATCH] Do not print params in --help except --help=param.

2020-01-23 Thread Martin Liška

Hi.

The patch finishes stripping of params in --help dump
for e.g. --help=joined or --help=separate.

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

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2020-01-23  Martin Liska  

* opts.c (print_help): Exclude params from
all except --help=param.
---
 gcc/opts.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


diff --git a/gcc/opts.c b/gcc/opts.c
index 33a662b54f5..3b2cc854af1 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2140,7 +2140,9 @@ print_help (struct gcc_options *opts, unsigned int lang_mask,
   /* We started using PerFunction/Optimization for parameters and
  a warning.  We should exclude these from optimization options.  */
   if (include_flags & CL_OPTIMIZATION)
-exclude_flags |= CL_WARNING | CL_PARAMS;
+exclude_flags |= CL_WARNING;
+  if (!(include_flags & CL_PARAMS))
+exclude_flags |= CL_PARAMS;
 
   if (include_flags)
 print_specific_help (include_flags, exclude_flags, 0, opts,



[PATCH] tree-optimization/93397 delay converted reduction chain adjustment

2020-01-23 Thread Richard Biener
The following delays adjusting the SLP graph for converted reduction
chains to a point where the SLP build no longer can fail since we
otherwise fail to undo marking the conversion as a group.

Bootstrap & regtest running on x86_64-unknown-linux-gnu.

Richard.

2020-01-23  Richard Biener  

PR tree-optimization/93397
* tree-vect-slp.c (vect_analyze_slp_instance): Delay
converted reduction chain SLP graph adjustment.

* gcc.dg/torture/pr93397.c: New testcase.
---
 gcc/testsuite/gcc.dg/torture/pr93397.c | 14 
 gcc/tree-vect-slp.c| 58 ++
 2 files changed, 44 insertions(+), 28 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr93397.c

diff --git a/gcc/testsuite/gcc.dg/torture/pr93397.c 
b/gcc/testsuite/gcc.dg/torture/pr93397.c
new file mode 100644
index 000..c19b7983e58
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr93397.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-ftree-vectorize" } */
+
+char
+bn (char *vu)
+{
+  int b6;
+  char wv = 0;
+
+  for (b6 = 0; b6 <= 64; b6 += 4)
+wv += vu[b6] + vu[b6 + 1];
+
+  return wv;
+}
diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c
index cebaa811dd2..b13beeb3689 100644
--- a/gcc/tree-vect-slp.c
+++ b/gcc/tree-vect-slp.c
@@ -2210,34 +2210,6 @@ vect_analyze_slp_instance (vec_info *vinfo,
  _size, bst_map);
   if (node != NULL)
 {
-  /* If this is a reduction chain with a conversion in front
- amend the SLP tree with a node for that.  */
-  if (!dr
- && REDUC_GROUP_FIRST_ELEMENT (stmt_info)
- && STMT_VINFO_DEF_TYPE (stmt_info) != vect_reduction_def)
-   {
- /* Get at the conversion stmt - we know it's the single use
-of the last stmt of the reduction chain.  */
- gimple *tem = vect_orig_stmt (scalar_stmts[group_size - 1])->stmt;
- use_operand_p use_p;
- gimple *use_stmt;
- bool r = single_imm_use (gimple_assign_lhs (tem), _p, _stmt);
- gcc_assert (r);
- next_info = vinfo->lookup_stmt (use_stmt);
- next_info = vect_stmt_to_vectorize (next_info);
- scalar_stmts = vNULL;
- scalar_stmts.create (group_size);
- for (unsigned i = 0; i < group_size; ++i)
-   scalar_stmts.quick_push (next_info);
- slp_tree conv = vect_create_new_slp_node (scalar_stmts);
- SLP_TREE_CHILDREN (conv).quick_push (node);
- node = conv;
- /* We also have to fake this conversion stmt as SLP reduction group
-so we don't have to mess with too much code elsewhere.  */
- REDUC_GROUP_FIRST_ELEMENT (next_info) = next_info;
- REDUC_GROUP_NEXT_ELEMENT (next_info) = NULL;
-   }
-
   /* Calculate the unrolling factor based on the smallest type.  */
   poly_uint64 unrolling_factor
= calculate_unrolling_factor (max_nunits, group_size);
@@ -2355,6 +2327,36 @@ vect_analyze_slp_instance (vec_info *vinfo,
}
}
 
+ /* If this is a reduction chain with a conversion in front
+amend the SLP tree with a node for that.  */
+ if (!dr
+ && REDUC_GROUP_FIRST_ELEMENT (stmt_info)
+ && STMT_VINFO_DEF_TYPE (stmt_info) != vect_reduction_def)
+   {
+ /* Get at the conversion stmt - we know it's the single use
+of the last stmt of the reduction chain.  */
+ gimple *tem = vect_orig_stmt (scalar_stmts[group_size - 1])->stmt;
+ use_operand_p use_p;
+ gimple *use_stmt;
+ bool r = single_imm_use (gimple_assign_lhs (tem),
+  _p, _stmt);
+ gcc_assert (r);
+ next_info = vinfo->lookup_stmt (use_stmt);
+ next_info = vect_stmt_to_vectorize (next_info);
+ scalar_stmts = vNULL;
+ scalar_stmts.create (group_size);
+ for (unsigned i = 0; i < group_size; ++i)
+   scalar_stmts.quick_push (next_info);
+ slp_tree conv = vect_create_new_slp_node (scalar_stmts);
+ SLP_TREE_CHILDREN (conv).quick_push (node);
+ SLP_INSTANCE_TREE (new_instance) = conv;
+ /* We also have to fake this conversion stmt as SLP reduction
+group so we don't have to mess with too much code
+elsewhere.  */
+ REDUC_GROUP_FIRST_ELEMENT (next_info) = next_info;
+ REDUC_GROUP_NEXT_ELEMENT (next_info) = NULL;
+   }
+
  vinfo->slp_instances.safe_push (new_instance);
 
  if (dump_enabled_p ())
-- 
2.16.4


[COMMITTED] aarch64: Fix -mtrack-speculation for irreversible conditions [PR93341]

2020-01-23 Thread Richard Sandiford
We can't yet represent the inverse of all conditions in rtl
(see g:865257c447cc50f5819e), triggering an ICE in the pass
that handles -mtrack-speculation.  Since we don't expect these
insns to be optimised in any way, the easiest fix seemed to be
to add an insn that reverses the condition internally.

Tested on aarch64-linux-gnu.  Also tested by making sure there
were no changes in asm output for fold-const.ii when compiled
with -O2 -mtrack-speculation.  Committed.

Richard


2020-01-23  Richard Sandiford  

gcc/
PR target/93341
* config/aarch64/aarch64.md (UNSPEC_SPECULATION_TRACKER_REV): New
unspec.
(speculation_tracker_rev): New pattern.
* config/aarch64/aarch64-speculation.cc (aarch64_do_track_speculation):
Use speculation_tracker_rev to track the inverse condition.

gcc/testsuite/
PR target/93341
* gcc.target/aarch64/pr93341.c: New test.
---
 gcc/config/aarch64/aarch64-speculation.cc  | 17 +
 gcc/config/aarch64/aarch64.md  | 15 +++
 gcc/testsuite/gcc.target/aarch64/pr93341.c | 14 ++
 3 files changed, 34 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/pr93341.c

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 55dde54b16a..4f5898185f5 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -262,6 +262,7 @@ (define_c_enum "unspec" [
 UNSPEC_REV_SUBREG
 UNSPEC_REINTERPRET
 UNSPEC_SPECULATION_TRACKER
+UNSPEC_SPECULATION_TRACKER_REV
 UNSPEC_COPYSIGN
 UNSPEC_TTEST   ; Represent transaction test.
 UNSPEC_UPDATE_FFR
@@ -7219,6 +7220,20 @@ (define_insn "speculation_tracker"
   [(set_attr "type" "csel")]
 )
 
+;; Like speculation_tracker, but track the inverse condition.
+(define_insn "speculation_tracker_rev"
+  [(set (reg:DI SPECULATION_TRACKER_REGNUM)
+   (unspec:DI [(reg:DI SPECULATION_TRACKER_REGNUM) (match_operand 0)]
+UNSPEC_SPECULATION_TRACKER_REV))]
+  ""
+  {
+operands[1] = gen_rtx_REG (DImode, SPECULATION_TRACKER_REGNUM);
+output_asm_insn ("csel\\t%1, %1, xzr, %M0", operands);
+return "";
+  }
+  [(set_attr "type" "csel")]
+)
+
 ;; BTI  instructions
 (define_insn "bti_noarg"
   [(unspec_volatile [(const_int 0)] UNSPECV_BTI_NOARG)]
diff --git a/gcc/config/aarch64/aarch64-speculation.cc 
b/gcc/config/aarch64/aarch64-speculation.cc
index 98dcc11fb75..f490b64ae61 100644
--- a/gcc/config/aarch64/aarch64-speculation.cc
+++ b/gcc/config/aarch64/aarch64-speculation.cc
@@ -215,20 +215,13 @@ aarch64_do_track_speculation ()
  && REG_P (XEXP (cond, 0))
  && REGNO (XEXP (cond, 0)) == CC_REGNUM
  && XEXP (cond, 1) == const0_rtx);
- enum rtx_code inv_cond_code
-   = reversed_comparison_code (cond, insn);
- /* We should be able to reverse all conditions.  */
- gcc_assert (inv_cond_code != UNKNOWN);
- rtx inv_cond = gen_rtx_fmt_ee (inv_cond_code, GET_MODE (cond),
-copy_rtx (XEXP (cond, 0)),
-copy_rtx (XEXP (cond, 1)));
+ rtx branch_tracker = gen_speculation_tracker (copy_rtx (cond));
+ rtx fallthru_tracker = gen_speculation_tracker_rev (cond);
  if (inverted)
-   std::swap (cond, inv_cond);
+   std::swap (branch_tracker, fallthru_tracker);
 
- insert_insn_on_edge (gen_speculation_tracker (cond),
-  BRANCH_EDGE (bb));
- insert_insn_on_edge (gen_speculation_tracker (inv_cond),
-  FALLTHRU_EDGE (bb));
+ insert_insn_on_edge (branch_tracker, BRANCH_EDGE (bb));
+ insert_insn_on_edge (fallthru_tracker, FALLTHRU_EDGE (bb));
  needs_tracking = true;
}
  else if (GET_CODE (PATTERN (insn)) == RETURN)
diff --git a/gcc/testsuite/gcc.target/aarch64/pr93341.c 
b/gcc/testsuite/gcc.target/aarch64/pr93341.c
new file mode 100644
index 000..1efebeee21a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr93341.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-mtrack-speculation" } */
+
+int synths_ ( float * rc)
+{
+  float r1, r2;
+  int i;
+  for (i = 0; i < 128; ++i)
+{
+  r2 = rc[i];
+  r1 = ((r2) <= (.99f) ? (r2) : (.99f));
+  rc[i] = ((r1) >= (-.99f) ? (r1) : (-.99f));
+}
+}


Re: [PATCH] Make target_clones resolver fn static.

2020-01-23 Thread Alexander Monakov


On Thu, 23 Jan 2020, Martin Liška wrote:
> > So this doesn't help review including two different target changes.  Making
> > ifunc dispatchers of public functions non-public looks like an unrelated
> > thing
> > to the bug (sorry if I mis-suggested).  So I feel comfortable approving the
> > earlier patch which just dropped the extra mangling for non-public
> > dispatchers
> > in the x86 backend.
> 
> Works for me.

If you will be revising the patch, can you please improve the new comment?

I mean this addition:

  /* Make the resolver function static.
  ... */

but it's not what the following code does.

Thanks.
Alexander

Re: [PATCH] Proposal for IPA init() constant propagation

2020-01-23 Thread Jan Hubicka
> Hi.
> 
> Thank you for the patch. I'm CCing the author of current IPA CP
> and IPA maintainer.
OK, happy to be in CC, but I wonder where I find the last version of the
patch?
https://gcc.gnu.org/bugzilla/attachment.cgi?id=47455
Honza
> 
> Martin


Re: Fix false -Wodr warnings

2020-01-23 Thread Thomas Schwinge
Hi!

On 2019-04-15T13:55:39+0200, Richard Biener  wrote:
> On Sun, Apr 14, 2019 at 10:59 PM Jan Hubicka  wrote:
>> this patch fixes false warning that is output when different -std
>> settings are used. In this case C++ FE produces same declaration in
>> different representations which differ by 0 sized fileds only.
>> The patch makes them to be ignored (and I checked we ignore them for
>> canonical type merging too)
>>
>> Bootstrapped/regtested x86_64-linux, comitted.

In 13ef7dc2448c0f9d8981577bb5448ee9b6842b1c "Backport
d2a0371d2641e85c5e6ca396029be32204d976df", Martin backported this to
releases/gcc-8, but without including the fix-up for...

> The testcase is bogus
>
> WARNING: lto.exp does not support dg-do
> WARNING: lto.exp does not support dg-options in primary source file

... that one.  As obvious, pushed Dominique's trunk r270390/commit
ef9387d8fe79219a106c3f9d6c6a87b0a9065d54 to releases/gcc-8 in
8e55f241ab8c754a2ab8ec2fe39afd9173589401 "pr89358_0.C: Replace dg-* with
dg-lto-*.", see attached.


Grüße
 Thomas


>> PR lto/89358
>> * g++.dg/lto/pr89358_0.C: New testcase.
>> * g++.dg/lto/pr89358_1.C: New testcase.
>> * ipa-devirt.c (skip_in_fields_list_p): New.
>> (odr_types_equivalent_p): Use it.
>> Index: testsuite/g++.dg/lto/pr89358_0.C
>> ===
>> --- testsuite/g++.dg/lto/pr89358_0.C(nonexistent)
>> +++ testsuite/g++.dg/lto/pr89358_0.C(working copy)
>> @@ -0,0 +1,11 @@
>> +/* { dg-do link } */
>> +/* { dg-options "-std=c++17"  } */
>> +#include 
>> +
>> +extern void test();
>> +
>> +int main()
>> +{
>> +std::map m;
>> +test();
>> +}
>> Index: testsuite/g++.dg/lto/pr89358_1.C
>> ===
>> --- testsuite/g++.dg/lto/pr89358_1.C(nonexistent)
>> +++ testsuite/g++.dg/lto/pr89358_1.C(working copy)
>> @@ -0,0 +1,7 @@
>> +/* { dg-options "-std=c++14"  } */
>> +#include 
>> +
>> +void test()
>> +{
>> +std::map m;
>> +}
>> Index: ipa-devirt.c
>> ===
>> --- ipa-devirt.c(revision 270324)
>> +++ ipa-devirt.c(working copy)
>> @@ -1282,6 +1282,24 @@ warn_types_mismatch (tree t1, tree t2, l
>>  inform (loc_t2, "the incompatible type is defined here");
>>  }
>>
>> +/* Return true if T should be ignored in TYPE_FIELDS for ODR comparsion.  */
>> +
>> +static bool
>> +skip_in_fields_list_p (tree t)
>> +{
>> +  if (TREE_CODE (t) != FIELD_DECL)
>> +return true;
>> +  /* C++ FE introduces zero sized fields depending on -std setting, see
>> + PR89358.  */
>> +  if (DECL_SIZE (t)
>> +  && integer_zerop (DECL_SIZE (t))
>> +  && DECL_ARTIFICIAL (t)
>> +  && DECL_IGNORED_P (t)
>> +  && !DECL_NAME (t))
>> +return true;
>> +  return false;
>> +}
>> +
>>  /* Compare T1 and T2, report ODR violations if WARN is true and set
>> WARNED to true if anything is reported.  Return true if types match.
>> If true is returned, the types are also compatible in the sense of
>> @@ -1548,9 +1566,9 @@ odr_types_equivalent_p (tree t1, tree t2
>>  f1 = TREE_CHAIN (f1), f2 = TREE_CHAIN (f2))
>>   {
>> /* Skip non-fields.  */
>> -   while (f1 && TREE_CODE (f1) != FIELD_DECL)
>> +   while (f1 && skip_in_fields_list_p (f1))
>>   f1 = TREE_CHAIN (f1);
>> -   while (f2 && TREE_CODE (f2) != FIELD_DECL)
>> +   while (f2 && skip_in_fields_list_p (f2))
>>   f2 = TREE_CHAIN (f2);
>> if (!f1 || !f2)
>>   break;


From 8e55f241ab8c754a2ab8ec2fe39afd9173589401 Mon Sep 17 00:00:00 2001
From: Dominique d'Humieres 
Date: Tue, 16 Apr 2019 15:24:58 +0200
Subject: [PATCH] pr89358_0.C: Replace dg-* with dg-lto-*.

2019-04-16  Dominique d'Humieres  

	* g++.dg/lto/pr89358_0.C: Replace dg-* with dg-lto-*.

(cherry picked from trunk r270390/commit ef9387d8fe79219a106c3f9d6c6a87b0a9065d54)
---
 gcc/testsuite/ChangeLog  | 7 +++
 gcc/testsuite/g++.dg/lto/pr89358_0.C | 4 ++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 6c2c5559b9b5..1d0ed10a84ac 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2020-01-23  Thomas Schwinge  
+
+	Backport:
+	2019-04-16  Dominique d'Humieres  
+
+	* g++.dg/lto/pr89358_0.C: Replace dg-* with dg-lto-*.
+
 2020-01-22  Joseph Myers  
 
 	Backport from mainline:
diff --git a/gcc/testsuite/g++.dg/lto/pr89358_0.C b/gcc/testsuite/g++.dg/lto/pr89358_0.C
index aebd2127c0cf..328c60c2fb18 100644
--- a/gcc/testsuite/g++.dg/lto/pr89358_0.C
+++ b/gcc/testsuite/g++.dg/lto/pr89358_0.C
@@ -1,5 +1,5 @@
-/* { dg-do link } */
-/* { dg-options "-std=c++17"  } */
+/* { dg-lto-do link } */
+/* { dg-lto-options "-std=c++17"  } */
 #include 
 
 extern void test();
-- 

Re: [PATCH] Proposal for IPA init() constant propagation

2020-01-23 Thread Martin Liška

Hi.

Thank you for the patch. I'm CCing the author of current IPA CP
and IPA maintainer.

Martin


Re: [PATCH] Make target_clones resolver fn static.

2020-01-23 Thread Martin Liška

On 1/23/20 2:09 PM, Richard Biener wrote:

On Tue, Jan 21, 2020 at 1:48 PM Martin Liška  wrote:


On 1/20/20 3:52 PM, Richard Biener wrote:

On Mon, Jan 20, 2020 at 3:46 PM H.J. Lu  wrote:


On Mon, Jan 20, 2020 at 6:41 AM Alexander Monakov  wrote:




On Mon, 20 Jan 2020, H.J. Lu wrote:


Bare IFUNC's don't seem to have this restriction. Why do we want to
constrain target clones this way?



foo's resolver acts as foo.  It should have the same visibility as foo.


What do you mean by that? From the implementation standpoint, there's
two symbols of different type with the same value. There's no problem
allowing one of them have local binding and the other have global binding.

Is there something special about target clones that doesn't come into
play with ifuncs?



I stand corrected.   Resolver should be static and it shouldn't be weak.


Reading the patch again and looking up more context it seems that the resolver
is already static we just mangle it extra when the original function
is public (?)
If so the patch looks quite obvious to me if we use some character not valid
in indetifiers in C but valid in assembly for the resolver decl.


Hello.

I'm sending updated version of the patch where I'm adding a run-time test
for 2 static symbols (with the same name) and it works fine. Moreover
I'm newly setting TREE_PUBLIC (ifunc_alias_decl) = 0 which seems to
work properly.

I tested both x86_64-linux-gnu and ppc64le-linux-gnu.


So this doesn't help review including two different target changes.  Making
ifunc dispatchers of public functions non-public looks like an unrelated thing
to the bug (sorry if I mis-suggested).  So I feel comfortable approving the
earlier patch which just dropped the extra mangling for non-public dispatchers
in the x86 backend.


Works for me.


 The other thing looks like sth for next stage1?


Yes.

Martin



Thanks,
Richard.


Martin



Richard.



--
H.J.






Re: [PATCH] doc: clarify the situation with pointer arithmetic

2020-01-23 Thread Richard Biener
On Wed, Jan 22, 2020 at 12:40 PM Martin Sebor  wrote:
>
> On 1/22/20 8:32 AM, Richard Biener wrote:
> > On Tue, 21 Jan 2020, Alexander Monakov wrote:
> >
> >> On Tue, 21 Jan 2020, Richard Biener wrote:
> >>
> >>> Fourth.  That PNVI (I assume it's the whole pointer-provenance stuff)
> >>> wants to get the "best" of both which can never be done since a compiler
> >>> needs to have a way to be conservative - in this area it's conflicting
> >>> conservative treatment which is impossible.
> >>
> >> This paragraph is unclear, I don't immediately see what the conflicting 
> >> goals
> >> are. The rest is clear enough given the previous discussions I saw.
> >>
> >> Did you mean the restriction that you cannot do arithmetic involving two
> >> integers based on pointers, get a value corresponding to one of them,
> >> cast it back and get a pointer suitable for accessing either of two
> >> originally pointed-to objects? I don't see that as a conflict because
> >> it places a restriction on users, not the compiler.
> >
> > As far as I remember the discussions PNVI requires to track
> > provenance for correctness, you may not miss or attach wrong provenance
> > to a pointer and there's only "single" provenance, not "many"
> > (aka, may point to A and B).  I don't see how you can ever implement that.
>
> The PVNI variant preferred by the object model group is referred
> to as "PNVI-ae-udi" which stands for "PNVI exposed-address user-
> disambiguation."  (The PNVI part stands for "Provenance Not Via
> Integers.)  This base PVNI model basically prohibits provenance
> tracking via integers, making it possible for programs to derive
> pointers to unrelated objects via casts between pointers and
> integers (and modifying the integer in between the casts).  This
> is considered a new restriction on implementations because
> the standard doesn't permit it (as you said upthread, all it
> specifies is that a pointer is equal to one obtained by casting
> the original to a intptr_t and back).
>
> The -ae-udi variant limits this restriction on implementations
> to escaped pointers and provides a means for users/programs to
> disambiguate between pointers to adjacent objects (i.e., a past
> the end pointer and one to the beginning of the object stored
> there).  The latest proposal is in N2362, with an overview in
> N2378).  At the last WG14 meeting there was broad discomfort
> with adopting the proposal for C2X because of the absence of
> implementation experience and concerns raised by implementers.
> The guidance to the study group was to target a separate technical
> specification for the proposal and allow time for implementation
> experience.  If the feedback from implementers is positive
> (whatever that might mean) WG14 said it would consider adopting
> the model for a revision of C after C2X.
>
> Overall, the impact of the proposals as well as their goal is to
> constrain implementations to the (presumed) benefit of programs
> in terms of expressiveness.  There are numerous examples of code
> that's currently treated as undefined by one or more compilers
> (as a consequence of optimizations) that the model makes valid.
> I'm not aware of any optimization opportunities the proposal
> might open up in GCC.

Well, PNVI limits optimization opportunities of GCC which currently
_does_ track provenance through integers and thus only allows
a very limited set(*) of "unrelated" pointers to appear here (documented
is that none are, implementation details differ from version to version).

There are no optimization "opportunities" by making pointer <-> integer
conversions lose information.

(*) for recent versions we allow pointers to globals to be "invented" but
place strict restrictions on automatic variables based on the idea that
you cannot have reliable absolute addressing of those (but you could
place objects at 0x12340 if you like via linker scripts)

Then there are of course bugs in GCC (just found PR93381) when
tracking provenance through integers (GCC also disregards the
possibility of someone actually moving pointers in very weird ways
which you could say is a bug).

You also can't easily disregard aligning bitwise ands of leaving the
current object if you consider overaligning so you'd again need
precise tracking of pointer offsets in points-to analysis which we don't have.

Richard.

>
> Martin


Re: [PATCH] wide-int: i386: Fix ICEs on TImode signed overflow add/sub patterns [PR93376]

2020-01-23 Thread Jakub Jelinek
On Thu, Jan 23, 2020 at 10:38:31AM +0100, Uros Bizjak wrote:
> On Thu, Jan 23, 2020 at 10:33 AM Jakub Jelinek  wrote:
> >
> > On Thu, Jan 23, 2020 at 09:14:42AM +, Richard Sandiford wrote:
> > > > The other patch is something suggested by Richard S., avoid using OImode
> > > > for this and instead use a partial int mode that is smaller.  This is 
> > > > still
> > > > playing with fire because even the partial int mode is larger than
> > > > MAX_BITSIZE_MODE_ANY_INT, but at least its precision fits into those 3
> > > > WIDE_INT_MAX_ELTS wide-int.h uses.
> > >
> > > I think we should increase MAX_BITSIZE_MODE_ANY_INT to the precision
> > > of POImode at the same time, and make that precision less than 192
> > > (191 maybe?).  That way everything is "correct" without increasing
> > > the size of wide_int.
> >
> > The 192 was chosen just to be nice and round, I guess it could be just 130
> > or say 160 (128 + 32).
> >
> > Uros, would you be ok with bumping MAX_BITSIZE_MODE_ANY_INT to 160
> > and changing the POImode patch to use 160 bits instead of 192
> > if it passes testing?
> 
> We can try 160 and hope that nothing breaks due to "weird" number.

Following passed bootstrap/regtest on x86_64-linux and i686-linux.

Ok for trunk then?

2020-01-23  Jakub Jelinek  

PR target/93376
* config/i386/i386-modes.def (POImode): New mode.
(MAX_BITSIZE_MODE_ANY_INT): Change from 128 to 160.
* config/i386/i386.md (DPWI): New mode attribute.
(addv4, subv4): Use  instead of .
(QWI): Rename to...
(QPWI): ... this.  Use POI instead of OI for TImode.
(*addv4_doubleword, *addv4_doubleword_1,
*subv4_doubleword, *subv4_doubleword_1): Use 
instead of .

* gcc.dg/pr93376.c: New test.

--- gcc/config/i386/i386-modes.def.jj   2020-01-22 16:10:29.812837184 +0100
+++ gcc/config/i386/i386-modes.def  2020-01-23 11:43:37.931345071 +0100
@@ -107,10 +107,19 @@ INT_MODE (XI, 64);
 PARTIAL_INT_MODE (HI, 16, P2QI);
 PARTIAL_INT_MODE (SI, 32, P2HI);
 
+/* Mode used for signed overflow checking of TImode.  As
+   MAX_BITSIZE_MODE_ANY_INT is only 160, wide-int.h reserves only that
+   rounded up to multiple of HOST_BITS_PER_WIDE_INT bits in wide_int etc.,
+   so OImode is too large.  For the overflow checking we actually need
+   just 1 or 2 bits beyond TImode precision.  Use 160 bits to have
+   a multiple of 32.  */
+PARTIAL_INT_MODE (OI, 160, POI);
+
 /* Keep the OI and XI modes from confusing the compiler into thinking
that these modes could actually be used for computation.  They are
-   only holders for vectors during data movement.  */
-#define MAX_BITSIZE_MODE_ANY_INT (128)
+   only holders for vectors during data movement.  Include POImode precision
+   though.  */
+#define MAX_BITSIZE_MODE_ANY_INT (160)
 
 /* The symbol Pmode stands for one of the above machine modes (usually SImode).
The tm.h file specifies which one.  It is not a distinct mode.  */
--- gcc/config/i386/i386.md.jj  2020-01-22 16:10:29.815837139 +0100
+++ gcc/config/i386/i386.md 2020-01-23 11:40:54.923822116 +0100
@@ -6054,15 +6054,18 @@ (define_insn "*addqi_ext_2"
   [(set_attr "type" "alu")
(set_attr "mode" "QI")])
 
+;; Like DWI, but use POImode instead of OImode.
+(define_mode_attr DPWI [(QI "HI") (HI "SI") (SI "DI") (DI "TI") (TI "POI")])
+
 ;; Add with jump on overflow.
 (define_expand "addv4"
   [(parallel [(set (reg:CCO FLAGS_REG)
   (eq:CCO
-(plus:
-  (sign_extend:
+(plus:
+  (sign_extend:
 (match_operand:SWIDWI 1 "nonimmediate_operand"))
   (match_dup 4))
-(sign_extend:
+(sign_extend:
   (plus:SWIDWI (match_dup 1)
 (match_operand:SWIDWI 2
   "")
@@ -6078,7 +6081,7 @@ (define_expand "addv4"
   if (CONST_SCALAR_INT_P (operands[2]))
 operands[4] = operands[2];
   else
-operands[4] = gen_rtx_SIGN_EXTEND (mode, operands[2]);
+operands[4] = gen_rtx_SIGN_EXTEND (mode, operands[2]);
 })
 
 (define_insn "*addv4"
@@ -6123,17 +6126,17 @@ (define_insn "addv4_1"
  (const_string "")))])
 
 ;; Quad word integer modes as mode attribute.
-(define_mode_attr QWI [(SI "TI") (DI "OI")])
+(define_mode_attr QPWI [(SI "TI") (DI "POI")])
 
 (define_insn_and_split "*addv4_doubleword"
   [(set (reg:CCO FLAGS_REG)
(eq:CCO
- (plus:
-   (sign_extend:
+ (plus:
+   (sign_extend:
  (match_operand: 1 "nonimmediate_operand" "%0,0"))
-   (sign_extend:
+   (sign_extend:
  (match_operand: 2 "x86_64_hilo_general_operand" "r,o")))
- (sign_extend:
+ (sign_extend:
(plus: (match_dup 1) (match_dup 2)
(set (match_operand: 0 "nonimmediate_operand" "=ro,r")
(plus: (match_dup 1) 

Re: [PATCH] Make target_clones resolver fn static.

2020-01-23 Thread Richard Biener
On Tue, Jan 21, 2020 at 1:48 PM Martin Liška  wrote:
>
> On 1/20/20 3:52 PM, Richard Biener wrote:
> > On Mon, Jan 20, 2020 at 3:46 PM H.J. Lu  wrote:
> >>
> >> On Mon, Jan 20, 2020 at 6:41 AM Alexander Monakov  
> >> wrote:
> >>>
> >>>
> >>>
> >>> On Mon, 20 Jan 2020, H.J. Lu wrote:
> >>>
> > Bare IFUNC's don't seem to have this restriction. Why do we want to
> > constrain target clones this way?
> >
> 
>  foo's resolver acts as foo.  It should have the same visibility as foo.
> >>>
> >>> What do you mean by that? From the implementation standpoint, there's
> >>> two symbols of different type with the same value. There's no problem
> >>> allowing one of them have local binding and the other have global binding.
> >>>
> >>> Is there something special about target clones that doesn't come into
> >>> play with ifuncs?
> >>>
> >>
> >> I stand corrected.   Resolver should be static and it shouldn't be weak.
> >
> > Reading the patch again and looking up more context it seems that the 
> > resolver
> > is already static we just mangle it extra when the original function
> > is public (?)
> > If so the patch looks quite obvious to me if we use some character not valid
> > in indetifiers in C but valid in assembly for the resolver decl.
>
> Hello.
>
> I'm sending updated version of the patch where I'm adding a run-time test
> for 2 static symbols (with the same name) and it works fine. Moreover
> I'm newly setting TREE_PUBLIC (ifunc_alias_decl) = 0 which seems to
> work properly.
>
> I tested both x86_64-linux-gnu and ppc64le-linux-gnu.

So this doesn't help review including two different target changes.  Making
ifunc dispatchers of public functions non-public looks like an unrelated thing
to the bug (sorry if I mis-suggested).  So I feel comfortable approving the
earlier patch which just dropped the extra mangling for non-public dispatchers
in the x86 backend.  The other thing looks like sth for next stage1?

Thanks,
Richard.

> Martin
>
> >
> > Richard.
> >
> >>
> >> --
> >> H.J.
>


[committed, libgomp,amdgcn] Fix plugin-gcn.c bug

2020-01-23 Thread Andrew Stubbs

I've committed this patch to fix a bug in the OpenMP argument parsing.

It was using a value instead of a mask to read from the arguments. Since 
the value in question happens to be zero, the comparison would always 
return true. I've not observed any bad behaviour from this, but in 
theory it could read attempt to read any unhandled argument as the 
thread limit.


Andrew
Fix libgomp plugin-gcn bug

2020-01-23  Andrew Stubbs  

	libgomp/
	* plugin/plugin-gcn.c (parse_target_attributes): Use correct mask for
	the device id.

diff --git a/libgomp/plugin/plugin-gcn.c b/libgomp/plugin/plugin-gcn.c
index de470a3dd33..7854c142f05 100644
--- a/libgomp/plugin/plugin-gcn.c
+++ b/libgomp/plugin/plugin-gcn.c
@@ -1228,7 +1228,8 @@ parse_target_attributes (void **input,
 	  grid_attrs_found = true;
 	  break;
 	}
-  else if ((id & GOMP_TARGET_ARG_DEVICE_ALL) == GOMP_TARGET_ARG_DEVICE_ALL)
+  else if ((id & GOMP_TARGET_ARG_DEVICE_MASK)
+	   == GOMP_TARGET_ARG_DEVICE_ALL)
 	{
 	  gcn_dims_found = true;
 	  switch (id & GOMP_TARGET_ARG_ID_MASK)


Re: [PATCH] Relax invalidation of TOP N counters in PGO.

2020-01-23 Thread Martin Liška

On 1/23/20 1:41 PM, Jan Hubicka wrote:

So my bet is that before the patch we had a bogus code. We detected invalid
stated with hiving first counter value == -1. Which could be also reached
with decrement of all values (0 - 1 == -1).

Maybe we would be interested in usage of a huge negative number to reflect
invalid state?


As discusse dover lunch, use of -1 should be safe because it matters
only in the on-disk counters which are prunned first and all -1 which
are result of actual time profiling should disappear.  However looking
at the code there seems bug:

diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c
index cdb611de2a2..e5f93943c87 100644
--- a/libgcc/libgcov-driver.c
+++ b/libgcc/libgcov-driver.c
@@ -219,9 +219,6 @@ static struct gcov_fn_buffer *fn_buffer;
  static void
  prune_topn_counter (gcov_type *counters, gcov_type all)
  {
-  if (counters[1] == -1)
-return;
-
for (unsigned i = 0; i < GCOV_TOPN_VALUES; i++)
  {
if (counters[2 * i + 1] < all)

Does this make sense and help something? :)


No, this one can be really removed.

I'm going to test it and commit.

Martin


Honza





Re: [PATCH] Relax invalidation of TOP N counters in PGO.

2020-01-23 Thread Jan Hubicka
> So my bet is that before the patch we had a bogus code. We detected invalid
> stated with hiving first counter value == -1. Which could be also reached
> with decrement of all values (0 - 1 == -1).
> 
> Maybe we would be interested in usage of a huge negative number to reflect
> invalid state?

As discusse dover lunch, use of -1 should be safe because it matters
only in the on-disk counters which are prunned first and all -1 which
are result of actual time profiling should disappear.  However looking
at the code there seems bug:

diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c
index cdb611de2a2..e5f93943c87 100644
--- a/libgcc/libgcov-driver.c
+++ b/libgcc/libgcov-driver.c
@@ -219,9 +219,6 @@ static struct gcov_fn_buffer *fn_buffer;
 static void
 prune_topn_counter (gcov_type *counters, gcov_type all)
 {
-  if (counters[1] == -1)
-return;
-
   for (unsigned i = 0; i < GCOV_TOPN_VALUES; i++)
 {
   if (counters[2 * i + 1] < all)

Does this make sense and help something? :)
Honza


Re: Define HAVE_ for math long double functions declared in vxworks headers

2020-01-23 Thread Jonathan Wakely

On 23/01/20 00:20 -0300, Alexandre Oliva wrote:

On Jan 22, 2020, Jonathan Wakely  wrote:


Isn't allowing arithmetic on function pointers a GNU extension?


Does that matter?  This test is only supposed to be compiled by GCC.


Maybe if somebody was crazy enough to build GCC with -pedantic-errors?


I think just adding the #undef to what you had originally is the best
version.


'k, thanks, will adjust, test, post and install.


Thanks.



Re: [PATCH] cprop: Don't replace fixed hard regs with pseudos [PR93124]

2020-01-23 Thread Richard Sandiford
Jeff Law  writes:
> On Wed, 2020-01-22 at 12:02 +, Richard Sandiford wrote:
>> One consequence of r276318 was that cselib now preserves sp-based
>> values across function calls.  This in turn convinced cprop to
>> replace the clobber in:
>> 
>>(set (reg PSUEDO) (reg sp))
>>...
>>(call ...)
>>...
>>(clobber (mem:BLK (reg sp)))
>> 
>> with:
>> 
>>(clobber (mem:BLK (reg PSEUDO)))
>> 
>> But I doubt this could ever be an optimisation, regardless of what the
>> changed instruction is.  Extending the lifetimes of pseudos can lead to
>> extra spills, whereas sp is available everywhere.
>> 
>> More generally, I don't think we should replace any fixed hard register
>> with a pseudo.  Replacing them with a constant is still potentially
>> useful though, since we'll only make the change if the insn pattern
>> allows it.
>> 
>> This part 1 of the fix for PR93124.  Part 2 contains the testcase.
>> 
>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>> 
>> Richard
>> 
>> 
>> 2020-01-22  Richard Sandiford  
>> 
>> gcc/
>>  PR rtl-optimization/93124
>>  * cprop.c (cprop_replace_with_reg_p): New function.
>>  (cprop_insn, do_local_cprop): Use it.
> In theory there may be cases where replacing a fixed hard register with
> a pseudo  in turn might allow a allocation of the pseudo to a different
> hard register which *could* have a different cost.
>
> But in a CLOBBER insn, none of that should matter.  Would it make sense
> to only do this on CLOBBERS?  I'm not rejecting.  Mostly I'm worried
> about unintended consequences and wondering if we narrow the cases
> where we're changing behavior that unintended consequences are less
> likely to pop up.

Yeah, I guess there is a danger of unintended consequences.  E.g. on
aarch64 the sp register isn't usable everywhere that a normal GPR is.
Ideally that kind of thing would be enforced by the predicates,
but it generally isn't yet, and it would be nice if the .md format
could make this easier to get right (e.g. generating predicates
automatically from constraints for simple cases).

I didn't make it clear at all, but clobbers don't really pose a separate
problem in the context of this patch.  I assume we made this kind of
sp->pseudo change between call boundaries before r276318 too, and the
auto-inc-dec patch is enough to fix the PR on its own.

It's just that when I saw where the (clobber (mem (reg PSEUDO))) was
coming from, it looked like a misfeature for general non-clobber insns
too.  In the testcase it was making a pseudo live across a call when it
wasn't before, meaning that either the pseudo would need to be spilled
around the call or that we'd need to save an extra call-saved
register.  The fact that we did this for an sp equivalence is a
regression from GCC 9.

But maybe the patch is tackling this in the wrong place.  In general,
I think we should be careful about propagating registers across calls
that they were previously dead at, and this should probably be tackled
in those terms instead.

So maybe the safest thing is to go with just the auto-inc-dec patch for
GCC 10.

Thanks,
Richard



Re: [PATCH] Prune invalid filename due to makefile syntax.

2020-01-23 Thread Nathan Sidwell

On 1/23/20 4:54 AM, Martin Liška wrote:

On 1/22/20 9:12 AM, Richard Biener wrote:

Ah, maybe it's worth to export this functionality to libiberty?


That would make sense to me. Can you please Nathan factor out
the function to libiberty?


you're not the boss of me :)

But seriously, I'm not really going to have time to invest in this in 
the near future.


nathan

--
Nathan Sidwell


Re: [PATCH] tree-optimization/93354 FRE redundant store removal validity fix

2020-01-23 Thread Richard Biener
On Wed, 22 Jan 2020, Richard Biener wrote:

> 
> This fixes the validity check for redundant store removal by looking
> at the actual stmt that represents the last (relevant) change to
> the TBAA state rather than the imperfectly tracked alias set in the
> expression hash table which then becomes unused (see a followup change
> to get rid of that).
> 
> On the way this allows re-enabling of VN_WALKREWRITE, fixing PR91123.
> 
> This also exposes an out-of-bound array access in 
> real.c:clear_significand_below fixed as well.
> 
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

So this didn't work out.  The following addresses this in a different
way, postponing a fix for PR91123 which needs another adjustment.

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


This fixes tracking of the alias-set of partial defs for use by
redundant store removal.

2020-01-23  Richard Biener  

PR tree-optimization/93381
* tree-ssa-sccvn.c (vn_walk_cb_data::push_partial_def): Take
alias-set of the def as argument and record the first one.
(vn_walk_cb_data::first_set): New member.
(vn_reference_lookup_3): Pass the alias-set of the current def
to push_partial_def.  Fix alias-set used in the aggregate copy
case.
(vn_reference_lookup): Consistently set *last_vuse_ptr.
* real.c (clear_significand_below): Fix out-of-bound access.

* gcc.dg/torture/pr93354.c: New testcase.

diff --git a/gcc/real.c b/gcc/real.c
index a57e5df113f..46d8126efe4 100644
--- a/gcc/real.c
+++ b/gcc/real.c
@@ -420,7 +420,10 @@ clear_significand_below (REAL_VALUE_TYPE *r, unsigned int 
n)
   for (i = 0; i < w; ++i)
 r->sig[i] = 0;
 
-  r->sig[w] &= ~(((unsigned long)1 << (n % HOST_BITS_PER_LONG)) - 1);
+  /* We are actually passing N == SIGNIFICAND_BITS which would result
+ in an out-of-bound access below.  */
+  if (n % HOST_BITS_PER_LONG != 0)
+r->sig[w] &= ~(((unsigned long)1 << (n % HOST_BITS_PER_LONG)) - 1);
 }
 
 /* Divide the significands of A and B, placing the result in R.  Return
diff --git a/gcc/testsuite/gcc.dg/torture/pr93354.c 
b/gcc/testsuite/gcc.dg/torture/pr93354.c
new file mode 100644
index 000..8ccf1e6d2c0
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr93354.c
@@ -0,0 +1,22 @@
+/* { dg-do run } */
+
+typedef __INT32_TYPE__ int32_t;
+typedef __INT64_TYPE__ int64_t;
+struct X { int32_t i; int32_t j; };
+void foo (int64_t *z)
+{
+  ((struct X *)z)->i = 0x05060708;
+  ((struct X *)z)->j = 0x01020304;
+  *z = 0x0102030405060708;
+}
+
+int main()
+{
+  int64_t l = 0;
+  int64_t *p;
+  asm ("" : "=r" (p) : "0" ());
+  foo (p);
+  if (l != 0x0102030405060708)
+__builtin_abort ();
+  return 0;
+}
diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c
index 0b8ee586139..6e0b2202157 100644
--- a/gcc/tree-ssa-sccvn.c
+++ b/gcc/tree-ssa-sccvn.c
@@ -1693,7 +1693,8 @@ struct vn_walk_cb_data
  ao_ref_init (_ref, orig_ref_);
}
   ~vn_walk_cb_data ();
-  void *push_partial_def (const pd_data& pd, tree, HOST_WIDE_INT);
+  void *push_partial_def (const pd_data& pd, tree,
+ alias_set_type, HOST_WIDE_INT);
 
   vn_reference_t vr;
   ao_ref orig_ref;
@@ -1706,6 +1707,7 @@ struct vn_walk_cb_data
   /* The first defs range to avoid splay tree setup in most cases.  */
   pd_range first_range;
   tree first_vuse;
+  alias_set_type first_set;
   splay_tree known_ranges;
   obstack ranges_obstack;
 };
@@ -1746,13 +1748,13 @@ pd_tree_dealloc (void *, void *)
 }
 
 /* Push PD to the vector of partial definitions returning a
-   value when we are ready to combine things with VUSE and MAXSIZEI,
+   value when we are ready to combine things with VUSE, SET and MAXSIZEI,
NULL when we want to continue looking for partial defs or -1
on failure.  */
 
 void *
 vn_walk_cb_data::push_partial_def (const pd_data , tree vuse,
-  HOST_WIDE_INT maxsizei)
+  alias_set_type set, HOST_WIDE_INT maxsizei)
 {
   const HOST_WIDE_INT bufsize = 64;
   /* We're using a fixed buffer for encoding so fail early if the object
@@ -1773,6 +1775,7 @@ vn_walk_cb_data::push_partial_def (const pd_data , 
tree vuse,
   first_range.offset = pd.offset;
   first_range.size = pd.size;
   first_vuse = vuse;
+  first_set = set;
   last_vuse_ptr = NULL;
   /* Continue looking for partial defs.  */
   return NULL;
@@ -1903,10 +1906,10 @@ vn_walk_cb_data::push_partial_def (const pd_data , 
tree vuse,
   if (dump_file && (dump_flags & TDF_DETAILS))
fprintf (dump_file,
 "Successfully combined %u partial definitions\n", ndefs);
-  /* ???  If we track partial defs alias-set we could use that if it
- is the same for all.  Use zero for now.  */
+  /* We are using the alias-set of the first store we encounter which
+should be appropriate here.  */
   return vn_reference_lookup_or_insert_for_pieces
-   

Re: [PATCH] Relax invalidation of TOP N counters in PGO.

2020-01-23 Thread Martin Liška

On 1/22/20 2:35 PM, Jan Hubicka wrote:

On 1/22/20 12:09 PM, Martin Liška wrote:

stats for indirect_call:
    total: 9210
    invalid: 600
    tracked values:
  0 values: 6280 times (68.19%)
  1 values: 1856 times (20.15%)
  2 values:  264 times (2.87%)
  3 values:  157 times (1.70%)
  4 values:   53 times (0.58%)

stats for topn:
    total: 1513
    invalid: 178
    tracked values:
  0 values: 1034 times (68.34%)
  1 values:  176 times (11.63%)
  2 values:   68 times (4.49%)
  3 values:   39 times (2.58%)
  4 values:   18 times (1.19%)


After the fix patch I get to:

== Stats for gcc ==
stats for indirect_call:
   total: 9210
   invalid: 518
   tracked values:
 0 values: 7746 times (84.10%)
 1 values:  827 times (8.98%)
 2 values:   53 times (0.58%)
 3 values:   60 times (0.65%)
 4 values:6 times (0.07%)

Which seems to me a reasonable improvement.
Martin


I am not too sure - the odd thing is that number of 0 values hash grown
up from 6280 to 7746.  (68->84%) Do you have any idea why?


So my bet is that before the patch we had a bogus code. We detected invalid
stated with hiving first counter value == -1. Which could be also reached
with decrement of all values (0 - 1 == -1).

Maybe we would be interested in usage of a huge negative number to reflect
invalid state?

Martin



Honza



diff --git a/gcc/gcov-io.h b/gcc/gcov-io.h
index d21a43c4c31..775264709b6 100644
--- a/gcc/gcov-io.h
+++ b/gcc/gcov-io.h
@@ -272,6 +272,9 @@ GCOV_COUNTERS
 /* Total number of single value counters.  */
 #define GCOV_TOPN_VALUES_COUNTERS (2 * GCOV_TOPN_VALUES + 1)
 
+/* Constant used for an invalid TOPN counter.  */
+#define GCOV_TOPN_INVALID ((gcov_type)0x8000LL)
+
 /* Convert a counter index to a tag.  */
 #define GCOV_TAG_FOR_COUNTER(COUNT)\
 	(GCOV_TAG_COUNTER_BASE + ((gcov_unsigned_t)(COUNT) << 17))
diff --git a/gcc/profile.c b/gcc/profile.c
index 1a669b86467..0156a218b1e 100644
--- a/gcc/profile.c
+++ b/gcc/profile.c
@@ -769,8 +769,8 @@ compute_branch_probabilities (unsigned cfg_checksum, unsigned lineno_checksum)
 static void
 sort_hist_values (histogram_value hist)
 {
-  /* counters[2] equal to -1 means that all counters are invalidated.  */
-  if (hist->hvalue.counters[2] == -1)
+  /* counters[2] equal to GCOV_TOPN_INVALID means that all counters are invalidated.  */
+  if (hist->hvalue.counters[2] == GCOV_TOPN_INVALID)
 return;
 
   gcc_assert (hist->type == HIST_TYPE_TOPN_VALUES
@@ -869,7 +869,7 @@ compute_value_histograms (histogram_values values, unsigned cfg_checksum,
 	  || hist->type == HIST_TYPE_INDIR_CALL)
 	{
 	  /* Each count value is multiplied by GCOV_TOPN_VALUES.  */
-	  if (hist->hvalue.counters[2] != -1)
+	  if (hist->hvalue.counters[2] != GCOV_TOPN_INVALID)
 	for (unsigned i = 0; i < GCOV_TOPN_VALUES; i++)
 	  hist->hvalue.counters[2 * i + 2]
 		= RDIV (hist->hvalue.counters[2 * i + 2], GCOV_TOPN_VALUES);
diff --git a/gcc/value-prof.c b/gcc/value-prof.c
index b7c7d7eaea5..491225b9cff 100644
--- a/gcc/value-prof.c
+++ b/gcc/value-prof.c
@@ -723,7 +723,7 @@ get_nth_most_common_value (gimple *stmt, const char *counter_type,
 			   histogram_value hist, gcov_type *value,
 			   gcov_type *count, gcov_type *all, unsigned n)
 {
-  if (hist->hvalue.counters[2] == -1)
+  if (hist->hvalue.counters[2] == GCOV_TOPN_INVALID)
 return false;
 
   gcc_assert (n < GCOV_TOPN_VALUES);
diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c
index cdb611de2a2..2ced903a06c 100644
--- a/libgcc/libgcov-driver.c
+++ b/libgcc/libgcov-driver.c
@@ -219,7 +219,7 @@ static struct gcov_fn_buffer *fn_buffer;
 static void
 prune_topn_counter (gcov_type *counters, gcov_type all)
 {
-  if (counters[1] == -1)
+  if (counters[1] == GCOV_TOPN_INVALID)
 return;
 
   for (unsigned i = 0; i < GCOV_TOPN_VALUES; i++)
diff --git a/libgcc/libgcov-merge.c b/libgcc/libgcov-merge.c
index b658aec46c6..6127e401278 100644
--- a/libgcc/libgcov-merge.c
+++ b/libgcc/libgcov-merge.c
@@ -103,9 +103,9 @@ merge_topn_values_set (gcov_type *counters)
   read_counters[2 * i + 1] = gcov_get_counter_ignore_scaling (-1);
 }
 
-  if (read_counters[1] == -1)
+  if (read_counters[1] == GCOV_TOPN_INVALID)
 {
-  counters[1] = -1;
+  counters[1] = GCOV_TOPN_INVALID;
   return;
 }
 
@@ -133,7 +133,7 @@ merge_topn_values_set (gcov_type *counters)
   /* We haven't found a slot, bail out.  */
   if (j == GCOV_TOPN_VALUES)
 	{
-	  counters[1] = -1;
+	  counters[1] = GCOV_TOPN_INVALID;
 	  return;
 	}
 }


Re: [PATCH][gcc] libgccjit: handle long literals in playback::context::new_string_literal

2020-01-23 Thread Andrea Corallo
> Andrea Corallo writes:
>
>> Andrea Corallo writes:
>>
>>> Andrea Corallo writes:
>>>
 Hi all,
 yesterday I've found an interesting bug in libgccjit.
 Seems we have an hard limitation of 200 characters for literal strings.
 Attempting to create longer strings lead to ICE during pass_expand
 while performing a sanity check in get_constant_size.

 Tracking down the issue seems the code we have was inspired from
 c-family/c-common.c:c_common_nodes_and_builtins were array_domain_type
 is actually defined with a size of 200.
 The comment that follows that point sounded premonitory :) :)

 /* Make a type for arrays of characters.
With luck nothing will ever really depend on the length of this
array type.  */

 At least in the current implementation the type is set by
 fix_string_type were the actual string length is taken in account.

 I attach a patch updating the logic accordingly and a new testcase
  for that.

 make check-jit is passing clean.

 Best Regards
   Andrea

 gcc/jit/ChangeLog
 2019-??-??  Andrea Corallo  

* jit-playback.h
(gcc::jit::recording::context m_recording_ctxt): Remove
m_char_array_type_node field.
* jit-playback.c
(playback::context::context) Remove m_char_array_type_node from member
initializer list.
(playback::context::new_string_literal) Fix logic to handle string
length > 200.

 gcc/testsuite/ChangeLog
 2019-??-??  Andrea Corallo  

* jit.dg/all-non-failing-tests.h: Add test-long-string-literal.c.
* jit.dg/test-long-string-literal.c: New testcase.
 diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
 index 9eeb2a7..a26b8d3 100644
 --- a/gcc/jit/jit-playback.c
 +++ b/gcc/jit/jit-playback.c
 @@ -88,7 +88,6 @@ playback::context::context (recording::context *ctxt)
: log_user (ctxt->get_logger ()),
  m_recording_ctxt (ctxt),
  m_tempdir (NULL),
 -m_char_array_type_node (NULL),
  m_const_char_ptr (NULL)
  {
JIT_LOG_SCOPE (get_logger ());
 @@ -670,9 +669,12 @@ playback::rvalue *
  playback::context::
  new_string_literal (const char *value)
  {
 -  tree t_str = build_string (strlen (value), value);
 -  gcc_assert (m_char_array_type_node);
 -  TREE_TYPE (t_str) = m_char_array_type_node;
 +  /* Compare with c-family/c-common.c: fix_string_type.  */
 +  size_t len = strlen (value);
 +  tree i_type = build_index_type (size_int (len));
 +  tree a_type = build_array_type (char_type_node, i_type);
 +  tree t_str = build_string (len, value);
 +  TREE_TYPE (t_str) = a_type;

/* Convert to (const char*), loosely based on
   c/c-typeck.c: array_to_pointer_conversion,
 @@ -2703,10 +2705,6 @@ playback::context::
  replay ()
  {
JIT_LOG_SCOPE (get_logger ());
 -  /* Adapted from c-common.c:c_common_nodes_and_builtins.  */
 -  tree array_domain_type = build_index_type (size_int (200));
 -  m_char_array_type_node
 -= build_array_type (char_type_node, array_domain_type);

m_const_char_ptr
  = build_pointer_type (build_qualified_type (char_type_node,
 diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
 index d4b148e..801f610 100644
 --- a/gcc/jit/jit-playback.h
 +++ b/gcc/jit/jit-playback.h
 @@ -322,7 +322,6 @@ private:

auto_vec m_functions;
auto_vec m_globals;
 -  tree m_char_array_type_node;
tree m_const_char_ptr;

/* Source location handling.  */
 diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h 
 b/gcc/testsuite/jit.dg/all-non-failing-tests.h
 index 0272e6f8..1b3d561 100644
 --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
 +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
 @@ -220,6 +220,13 @@
  #undef create_code
  #undef verify_code

 +/* test-long-string-literal.c */
 +#define create_code create_code_long_string_literal
 +#define verify_code verify_code_long_string_literal
 +#include "test-long-string-literal.c"
 +#undef create_code
 +#undef verify_code
 +
  /* test-sum-of-squares.c */
  #define create_code create_code_sum_of_squares
  #define verify_code verify_code_sum_of_squares
 diff --git a/gcc/testsuite/jit.dg/test-long-string-literal.c 
 b/gcc/testsuite/jit.dg/test-long-string-literal.c
 new file mode 100644
 index 000..882567c
 --- /dev/null
 +++ b/gcc/testsuite/jit.dg/test-long-string-literal.c
 @@ -0,0 +1,48 @@
 +#include 
 +#include 
 +#include 
 +
 +#include "libgccjit.h"
 +
 +#include "harness.h"
 +
 +const char very_long_string[] =
 +  
 "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"
 +  

Re: [PATCH] Fix libstdc++ testsuite to handle VxWorks gthreads implementation

2020-01-23 Thread Corentin Gay




On 20/12/2019 11:33, Jonathan Wakely wrote:


Was it tested on AIX?

I think dg-require-gthreads will prevent the tests running for the
single-threaded multilib on AIX, so it will work OK. But there's a
chance it will need fixing. Let's wait and see (I'm currently unable
to build GCC on AIX).

OK for trunk, thanks.



Thank you for the input Jonathan, it hasn't been tested on AIX.
We are in the process of doing so.

Cheers,
--
Corentin Gay


Re: wwwdocs: Document the gcc git repository layout

2020-01-23 Thread Richard Earnshaw (lists)

On 22/01/2020 22:15, Joseph Myers wrote:

On Wed, 22 Jan 2020, Richard Earnshaw (lists) wrote:


Joseph, have I got all of these right?


refs/meta in that list should be refs/meta/config, there isn't anything
else under refs/meta/ at present.



I've pushed this version...


diff --git a/htdocs/git.html b/htdocs/git.html
index 6100146e..66d68ebc 100644
--- a/htdocs/git.html
+++ b/htdocs/git.html
@@ -197,6 +197,61 @@ To create a worktree for a new project branch based on master, do
 git worktree add -b project ../project master
 
 
+Repository Layout
+
+By default, a git clone operation will only fetch the
+main development, release branches and their associated tags from the
+server.  This will be sufficient for most users, but a number of
+additional branches can also be fetched if necessary.
+
+The following areas exist on the server:
+
+  refs/heads/master - The active development version of the
+compiler.
+  refs/heads/releases/* - Release branches.
+  refs/heads/devel/* - topic-specific development branches
+- see Active Development Branches.
+Branches and tags in this space may be moved to refs/dead once active
+development is completed or abandoned.
+  refs/tags/* - tags for all of the above branches.
+  refs/vendors/*/{heads,tags}/* - vendor-specific branches and
+tags.  These are owned and maintained by the respective
+vendor, but made available publicly.  Non-fast-forward pushes
+are permitted on these branches.
+  refs/users/*/{heads,tags}/* - personal developer branches and
+tags.  These are owned and maintained by the individual developer.
+Non-fast-forward pushes are permitted on these branches.
+  refs/dead/heads/* - completed or abandoned development
+branches.
+  refs/meta/config - local configuration data for the
+commit hooks on the server.
+  refs/deleted/*/{heads,tags}/* - old branches and tags
+from the SVN repository that were deleted before the conversion to
+git.
+  refs/git-svn-old/* - various branches and tags from the
+git-svn pre-conversion git mirror.
+  refs/git-old/* - various branches and tags from the
+git-svn pre-conversion git mirror that were local to that git
+repository.
+
+
+You can download any of the additional branches by adding a suitable
+fetch specification to your local copy of the git repostiory.  For
+example, if your remote is called 'origin' (the default with git
+clone) you can add the 'dead' development branches by running:
+
+
+git config --add remote.origin.fetch "+refs/dead/heads/*:refs/remotes/origin/dead/*"
+git config --add remote.origin.fetch "+refs/dead/tags/*:refs/tags/dead/*"
+git fetch origin
+
+
+which will place the dead branches in remotes/origin/dead
+  and the tags in tags/dead.
+
+You can use git ls-remote to get a complete list of
+refs that the server holds.
+
 Active Development Branches
 
 


Re: [PATCH 2/2][MIPS][RFC] Emit .note.GNU-stack for hard-float linux targets.

2020-01-23 Thread Dragan Mladjenovic
On 07.12.2019. 19:33, Jeff Law wrote:
> On Thu, 2019-11-07 at 17:05 +, Dragan Mladjenovic wrote:
>> On 01.11.2019. 11:32, Dragan Mladjenovic wrote:
>>> On 10.08.2019. 00:15, Joseph Myers wrote:
 On Fri, 9 Aug 2019, Jeff Law wrote:

>> 2019-08-05  Dragan Mladjenovic  
>>
>>  * config.in: Regenerated.
>>  * config/mips/linux.h (NEED_INDICATE_EXEC_STACK): Define
>> to 1
>>  for TARGET_LIBC_GNUSTACK.
>>  * configure: Regenerated.
>>  * configure.ac: Define TARGET_LIBC_GNUSTACK if glibc
>> version is
>>  found 2.31 or greater.
> My only concern here is the configure bits.  So for example,
> will it do
> the right thing if you're cross-compiling to a MIPS linux
> target?  If
> so, how?  If not, do we need to make it a first class configure
> option
> so that it can be specified when building cross MIPS linux
> toolchains?

 The key point of using GCC_GLIBC_VERSION_GTE_IFELSE is that (a)
 it checks
 the target glibc headers if available when GCC is built and (b)
 if not
 available, you can still use --with-glibc-version when
 configuring
 GCC, to
 get the right configuration in a bootstrap compiler built before
 glibc is
 built (the latter is necessary on some architectures to get the
 right
 stack-protector configuration for bootstrapping glibc, but may be
 useful
 in other cases as well).

 My main concern about this patch is the one I gave in
 
 about what
 the configuration mechanism should be, on a whole-toolchain
 level, to say
 whether you are OK with a requirement for a 4.8 or later kernel.

>>>
>>> Sorry for the late reply.
>>>
>>> I was waiting to backport [1] to most of the glibc release branches
>>> in
>>> use, but I got sidetracked along the way.
>>>
>>> After this patch lands the preferred way to configure gcc would be
>>> using
>>> --with-glibc-version=2.31 and to use said glibc.
>>> If the user/distribution can live with minimal kernel requirement
>>> of 4.8
>>> the glibc used should be configured with --enable-kernel=4.8.
>>> I also plan to backport the [1] to limit the opportunity for
>>> building
>>> the possibly broken glibc with the gcc w/ enabled .note.GNU-stack.
>>>
>>> This is all tedious and user has to be aware of all of it to make
>>> it
>>> work, but hopefully over time the distributions will default to
>>> --with-glibc-version=2.31 and --enable-kernel=4.8. I guess
>>> providing the
>>> detailed NEWS entry for this change would help a bit.
>>>
>>> Is there any objections to getting this on the trunk before the end
>>> of
>>> stage1?
>>>
>>> [1] https://sourceware.org/ml/libc-alpha/2019-08/msg00639.html
>>>
>>
>> Small update and gentle ping. The glibc change was backported all
>> the
>> way back to 2.24.
> I think this is fine.  And yes, we'd like to get it mentioned in the
> release notes since I suspect folks will want the GNU-stack ELF notes.
>
> jeff
>
>

I left this to fall through the cracks once again. In light of [1], is 
there any leeway for me to push this now?

[1] https://gcc.gnu.org/ml/gcc/2020-01/msg00199.html




[committed] update gcc-10/changes.html

2020-01-23 Thread Martin Sebor

I pushed the attached diff to the wwwdocs repository.

In links to the manuals that I added I pointed to the upcoming
release, 10.1.0, even though the new version directory hasn't
been created yet.  I also updated existing links pointing to
the development version of GCC to point to 10.1.0 instead.
That, of course, makes testing the links error-prone.  Rather
than having to remember to do all this by hand and making
mistakes in the process I wonder if it could be automated.  For
instance, could we instead set up a script to update the links
automatically either when the release directory is created, or
via a post-commit hook if that's possible?  Or, if that can't
be done, could the script that checks the validity of the html
be modified to also check the links and complain if they don't
point to the next release?

Marti
diff --git a/htdocs/gcc-10/changes.html b/htdocs/gcc-10/changes.html
index 66440102..6e752157 100644
--- a/htdocs/gcc-10/changes.html
+++ b/htdocs/gcc-10/changes.html
@@ -54,21 +54,34 @@ a work-in-progress.
 
 General Improvements
 
-The following built-in functions have been introduced.
 
-  __builtin_roundeven for the corresponding function from
-ISO/IEC TS 18661.
+  New built-in functions:
+
+  The
+	https://gcc.gnu.org/onlinedocs/gcc-10.1.0/cpp/_005f_005fhas_005fbuiltin.html#g_t_005f_005fhas_005fbuiltin;>__has_builtin
+	built-in preprocessor operator can be used to query support
+	for built-in functions provided by GCC and other compilers
+	that support it.
+  
+  __builtin_roundeven for the corresponding function from
+	ISO/IEC TS 18661.
+  
+
   
   
-A new option, https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#index-fno-allocation-dce;>-fallocation-dce
-has been added. The option removes unneeded pairs of new
-and delete operators.
+New command-line options:
+
+  https://gcc.gnu.org/onlinedocs/gcc-10.1.0/gcc/Optimize-Options.html#index-fno-allocation-dce;>-fallocation-dce
+	removes unneeded pairs of new and delete
+	operators.
+  
+
   
   
 Profile driven optimization improvements:
 
   
-Using https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#index-fprofile-values;>-fprofile-values,
+Using https://gcc.gnu.org/onlinedocs/gcc-10.1.0/gcc/Optimize-Options.html#index-fprofile-values;>-fprofile-values,
 an instrumented binary can track multiple
 values (up to 4) for e.g. indirect calls and provide more precise profile information.
   
@@ -78,7 +91,7 @@ a work-in-progress.
 Link-time optimization improvements:
 
   
-A new binary https://gcc.gnu.org/onlinedocs/gcc/lto-dump.html;>lto-dump
+A new binary https://gcc.gnu.org/onlinedocs/gcc-10.1.0/gcc/lto-dump.html;>lto-dump
 has been added.  The program can dump various
 information about a LTO bytecode object file.
   
@@ -102,7 +115,63 @@ a work-in-progress.
 
 
 
-
+C family
+
+  New attributes:
+
+ 
+   The access function and type attribute has been added
+   to describe how a function accesses objects passed to it by pointer
+   or reference, and to associate such arguments with integer arguments
+   denoting the objects' sizes.  The attribute is used to enable
+   the detection of invalid accesses by user-defined functions, such
+   as those diagnosed by -Wstringop-overflow.
+ 
+
+  
+  New warnings:
+
+  https://gcc.gnu.org/onlinedocs/gcc-10.1.0/gcc/Warning-Options.html#index-Wstring-compare;>-Wstring-compare, enabled
+	by -Wextra, warns about equality and inequality
+	expressions between zero and the result of a call to either
+	strcmp and strncmp that evaluate to
+	a constant as a result of the length of one argument being greater
+	than the size of the array pointed to by the other.
+  
+  https://gcc.gnu.org/onlinedocs/gcc-10.1.0/gcc/Warning-Options.html#index-Wzero-length-bounds;>-Wzero-length-bounds,
+	enabled by -Warray-bounds, warns about accesses to
+	elements of zero-length arrays that might overlap other members
+	of the same object.
+  
+
+  
+  Enhancements to existing warnings:
+
+  https://gcc.gnu.org/onlinedocs/gcc-10.1.0/gcc/Warning-Options.html#index-Warray-bounds;>-Warray-bounds
+	detects more out-of-bounds accesses to member arrays as well as
+	accesses to elements of zero-length arrays.
+  
+  https://gcc.gnu.org/onlinedocs/gcc-10.1.0/gcc/Warning-Options.html#index-Wformat-overflow;>-Wformat-overflow
+	makes full use of string length information computed by
+	the strlen optimization pass.
+  
+  https://gcc.gnu.org/onlinedocs/gcc-10.1.0/gcc/Warning-Options.html#index-Wrestrict;>-Wrestrict
+	detects overlapping accesses to dynamically allocated objects.
+  
+  https://gcc.gnu.org/onlinedocs/gcc-10.1.0/gcc/Warning-Options.html#index-Wreturn-local-addr;>-Wreturn-local-addr
+	diagnoses more instances of return statements returning
+	addresses of 

Re: [PATCH] analyzer: testsuite: introduce analyzer-torture.exp

2020-01-23 Thread Richard Sandiford
David Malcolm  writes:
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
> OK for master?  I'm working on various followup bugfixes that could
> use this for test coverage.
>
> gcc/testsuite/ChangeLog:
>   * gcc.dg/analyzer/data-model-3.c: Remove hardcoded "-O2" and move
>   to torture/conftest-1.c.
>   * gcc.dg/analyzer/torture/analyzer-torture.exp: New.
>   * gcc.dg/analyzer/torture/conftest-1.c: Move here from
>   analyzer/data-model-3.c.
>   * gcc.dg/analyzer/torture/poc.c: New test.

OK, thanks.

Richard

> ---
>  gcc/testsuite/gcc.dg/analyzer/data-model-3.c  | 15 ---
>  .../analyzer/torture/analyzer-torture.exp | 44 +++
>  .../gcc.dg/analyzer/torture/conftest-1.c  | 10 +
>  gcc/testsuite/gcc.dg/analyzer/torture/poc.c   | 24 ++
>  4 files changed, 78 insertions(+), 15 deletions(-)
>  delete mode 100644 gcc/testsuite/gcc.dg/analyzer/data-model-3.c
>  create mode 100644 gcc/testsuite/gcc.dg/analyzer/torture/analyzer-torture.exp
>  create mode 100644 gcc/testsuite/gcc.dg/analyzer/torture/conftest-1.c
>  create mode 100644 gcc/testsuite/gcc.dg/analyzer/torture/poc.c
>
> diff --git a/gcc/testsuite/gcc.dg/analyzer/data-model-3.c 
> b/gcc/testsuite/gcc.dg/analyzer/data-model-3.c
> deleted file mode 100644
> index 3d572eb8d73..000
> --- a/gcc/testsuite/gcc.dg/analyzer/data-model-3.c
> +++ /dev/null
> @@ -1,15 +0,0 @@
> -/* { dg-additional-options "-O2" } */
> -/* TODO:is there a way to automatically run the tests on various
> -   optimizations levels, and with/without debuginfo, rather than
> -   hardcoding options?  Adapt from torture .exp, presumably.  */
> -
> -#include 
> -int
> -main ()
> -{
> -  FILE *f = fopen ("conftest.out", "w");
> -  return ferror (f) || fclose (f) != 0;
> -
> -  ;
> -  return 0;
> -}
> diff --git a/gcc/testsuite/gcc.dg/analyzer/torture/analyzer-torture.exp 
> b/gcc/testsuite/gcc.dg/analyzer/torture/analyzer-torture.exp
> new file mode 100644
> index 000..a4d98bb2297
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/analyzer/torture/analyzer-torture.exp
> @@ -0,0 +1,44 @@
> +#   Copyright (C) 2020 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +# 
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +# 
> +# You should have received a copy of the GNU General Public License
> +# along with GCC; see the file COPYING3.  If not see
> +# .
> +
> +# This harness is for tests that should be run at all optimisation levels.
> +
> +load_lib gcc-dg.exp
> +
> +# If the analyzer has not been enabled, bail.
> +if { ![check_effective_target_analyzer] } {
> +return
> +}
> +
> +dg-init
> +
> +global DEFAULT_CFLAGS
> +if [info exists DEFAULT_CFLAGS] then {
> +  set save_default_cflags $DEFAULT_CFLAGS
> +}
> +
> +# If a testcase doesn't have special options, use these.
> +set DEFAULT_CFLAGS "-fanalyzer -fdiagnostics-path-format=separate-events 
> -Wanalyzer-too-complex -fanalyzer-call-summaries"
> +
> +gcc-dg-runtest [lsort [glob $srcdir/$subdir/*.c]] "" $DEFAULT_CFLAGS
> +
> +dg-finish
> +
> +if [info exists save_default_cflags] {
> +  set DEFAULT_CFLAGS $save_default_cflags
> +} else {
> +  unset DEFAULT_CFLAGS
> +}
> diff --git a/gcc/testsuite/gcc.dg/analyzer/torture/conftest-1.c 
> b/gcc/testsuite/gcc.dg/analyzer/torture/conftest-1.c
> new file mode 100644
> index 000..0cf85f0ebe1
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/analyzer/torture/conftest-1.c
> @@ -0,0 +1,10 @@
> +#include 
> +int
> +main ()
> +{
> +  FILE *f = fopen ("conftest.out", "w");
> +  return ferror (f) || fclose (f) != 0;
> +
> +  ;
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.dg/analyzer/torture/poc.c 
> b/gcc/testsuite/gcc.dg/analyzer/torture/poc.c
> new file mode 100644
> index 000..1ad45b2f63e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/analyzer/torture/poc.c
> @@ -0,0 +1,24 @@
> +/* { dg-do link } */
> +
> +#include 
> +
> +void test_1 (void *ptr)
> +{
> +  free (ptr);
> +  free (ptr); /* { dg-warning "double-free" } */
> +}
> +
> +struct s
> +{
> +  void *ptr;
> +};
> +
> +void test_2 (struct s *x)
> +{
> +  free (x->ptr);
> +  free (x->ptr); /* { dg-warning "double-free" } */
> +}
> +
> +/* TODO: be more precise about what is freed.  */
> +
> +int main () {}


Re: [PATCH] Prune invalid filename due to makefile syntax.

2020-01-23 Thread Martin Liška

On 1/22/20 9:12 AM, Richard Biener wrote:

Ah, maybe it's worth to export this functionality to libiberty?


That would make sense to me. Can you please Nathan factor out
the function to libiberty?

Thanks,
Martin


[COMMITTED] aarch64: Skip whilele_1.C test for ILP32

2020-01-23 Thread Richard Sandiford
The definitions of the integer types for ILP32 newlib make the
resolution of some of the bool-related tests ambiguous.

Tested on aarch64-linux-gnu and aarch64_be-elf, committed.

Richard


2020-01-23  Richard Sandiford  

gcc/testsuite/
* g++.target/aarch64/sve/acle/general-c++/whilele_1.C: Skip for ILP32.
---
 .../g++.target/aarch64/sve/acle/general-c++/whilele_1.C | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/g++.target/aarch64/sve/acle/general-c++/whilele_1.C 
b/gcc/testsuite/g++.target/aarch64/sve/acle/general-c++/whilele_1.C
index 9571e668b81..31b054255c8 100644
--- a/gcc/testsuite/g++.target/aarch64/sve/acle/general-c++/whilele_1.C
+++ b/gcc/testsuite/g++.target/aarch64/sve/acle/general-c++/whilele_1.C
@@ -1,4 +1,4 @@
-// { dg-do compile }
+// { dg-do compile { target { ! ilp32 } } }
 
 #include 
 


Re: [PATCH] wide-int: i386: Fix ICEs on TImode signed overflow add/sub patterns [PR93376]

2020-01-23 Thread Uros Bizjak
On Thu, Jan 23, 2020 at 10:33 AM Jakub Jelinek  wrote:
>
> On Thu, Jan 23, 2020 at 09:14:42AM +, Richard Sandiford wrote:
> > > The other patch is something suggested by Richard S., avoid using OImode
> > > for this and instead use a partial int mode that is smaller.  This is 
> > > still
> > > playing with fire because even the partial int mode is larger than
> > > MAX_BITSIZE_MODE_ANY_INT, but at least its precision fits into those 3
> > > WIDE_INT_MAX_ELTS wide-int.h uses.
> >
> > I think we should increase MAX_BITSIZE_MODE_ANY_INT to the precision
> > of POImode at the same time, and make that precision less than 192
> > (191 maybe?).  That way everything is "correct" without increasing
> > the size of wide_int.
>
> The 192 was chosen just to be nice and round, I guess it could be just 130
> or say 160 (128 + 32).
>
> Uros, would you be ok with bumping MAX_BITSIZE_MODE_ANY_INT to 160
> and changing the POImode patch to use 160 bits instead of 192
> if it passes testing?

We can try 160 and hope that nothing breaks due to "weird" number.

Uros.

> I guess it would still affect the sizes of the wide-int.cc arrays:
>   unsigned HOST_HALF_WIDE_INT
> u[4 * MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_HALF_WIDE_INT];
>   unsigned HOST_HALF_WIDE_INT
> v[4 * MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_HALF_WIDE_INT];
>   /* The '2' in 'R' is because we are internally doing a full
>  multiply.  */
>   unsigned HOST_HALF_WIDE_INT
> r[2 * 4 * MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_HALF_WIDE_INT];
> so if we didn't want that, we would need to use 132; but those
> are weird anyway, because they round down rather than up.
>
> Jakub
>


Re: [PATCH, GCC/ARM] Fix MVE scalar shift tests

2020-01-23 Thread Christophe Lyon
On Mon, 20 Jan 2020 at 19:01, Mihail Ionescu
 wrote:
>
> Hi,
>
> This patch fixes the scalar shifts tests added in:
> https://gcc.gnu.org/ml/gcc-patches/2019-11/msg01195.html
> https://gcc.gnu.org/ml/gcc-patches/2019-11/msg01196.html
> By adding mthumb and ensuring that the target supports
> thumb2 instructions.
>
>
> *** gcc/testsuite/ChangeLog ***
>
> 2020-01-20  Mihail-Calin Ionescu  
>
> * gcc/testsuite/gcc.target/arm/armv8_1m-shift-imm-1.c: Add mthumb and 
> target check.
> * gcc/testsuite/gcc.target/arm/armv8_1m-shift-reg-1.c: Likewise.
>
>
> Is this ok for trunk?
>

Why not add a new entry in check_effective_target_arm_arch_FUNC_ok?
(there are already plenty, including v8m_main for instance)

Christophe

>
> Regards,
> Mihail
>
>
> ### Attachment also inlined for ease of reply
> ###
>
>
> diff --git a/gcc/testsuite/gcc.target/arm/armv8_1m-shift-imm-1.c 
> b/gcc/testsuite/gcc.target/arm/armv8_1m-shift-imm-1.c
> index 
> 5ffa3769e6ba42466242d3038857734e87b2f1fc..9822f59643c662c9302ad43c09057c59f3cbe07a
>  100644
> --- a/gcc/testsuite/gcc.target/arm/armv8_1m-shift-imm-1.c
> +++ b/gcc/testsuite/gcc.target/arm/armv8_1m-shift-imm-1.c
> @@ -1,5 +1,6 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -march=armv8.1-m.main+mve -mfloat-abi=softfp" } */
> +/* { dg-options "-O2 -mthumb -march=armv8.1-m.main+mve -mfloat-abi=softfp" } 
> */
> +/* { dg-require-effective-target arm_thumb2_ok } */
>
>  long long longval1;
>  long long unsigned longval2;
> diff --git a/gcc/testsuite/gcc.target/arm/armv8_1m-shift-reg-1.c 
> b/gcc/testsuite/gcc.target/arm/armv8_1m-shift-reg-1.c
> index 
> a97e9d687ef66e9642dd1d735125c8ee941fb151..a9aa7ed3ad9204c03d2c15dc6920ca3159403fa0
>  100644
> --- a/gcc/testsuite/gcc.target/arm/armv8_1m-shift-reg-1.c
> +++ b/gcc/testsuite/gcc.target/arm/armv8_1m-shift-reg-1.c
> @@ -1,5 +1,6 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -march=armv8.1-m.main+mve -mfloat-abi=softfp" } */
> +/* { dg-options "-O2 -mthumb -march=armv8.1-m.main+mve -mfloat-abi=softfp" } 
> */
> +/* { dg-require-effective-target arm_thumb2_ok  } */
>
>  long long longval2;
>  int intval2;
>


Re: [PATCH] wide-int: i386: Fix ICEs on TImode signed overflow add/sub patterns [PR93376]

2020-01-23 Thread Jakub Jelinek
On Thu, Jan 23, 2020 at 09:14:42AM +, Richard Sandiford wrote:
> > The other patch is something suggested by Richard S., avoid using OImode
> > for this and instead use a partial int mode that is smaller.  This is still
> > playing with fire because even the partial int mode is larger than
> > MAX_BITSIZE_MODE_ANY_INT, but at least its precision fits into those 3
> > WIDE_INT_MAX_ELTS wide-int.h uses.
> 
> I think we should increase MAX_BITSIZE_MODE_ANY_INT to the precision
> of POImode at the same time, and make that precision less than 192
> (191 maybe?).  That way everything is "correct" without increasing
> the size of wide_int.

The 192 was chosen just to be nice and round, I guess it could be just 130
or say 160 (128 + 32).

Uros, would you be ok with bumping MAX_BITSIZE_MODE_ANY_INT to 160
and changing the POImode patch to use 160 bits instead of 192
if it passes testing?

I guess it would still affect the sizes of the wide-int.cc arrays:
  unsigned HOST_HALF_WIDE_INT
u[4 * MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_HALF_WIDE_INT];
  unsigned HOST_HALF_WIDE_INT
v[4 * MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_HALF_WIDE_INT];
  /* The '2' in 'R' is because we are internally doing a full
 multiply.  */
  unsigned HOST_HALF_WIDE_INT
r[2 * 4 * MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_HALF_WIDE_INT];
so if we didn't want that, we would need to use 132; but those
are weird anyway, because they round down rather than up.

Jakub



Re: [C++ Patch] PR 92804 [10 Regression] ICE trying to use concept as a nested-name-specifier

2020-01-23 Thread Paolo Carlini

Hi,

On 22/01/20 22:32, Jason Merrill wrote:

On 1/22/20 3:31 PM, Paolo Carlini wrote:

Hi,

On 22/01/20 17:27, Jason Merrill wrote:

On 1/22/20 10:22 AM, Paolo Carlini wrote:

Hi,

in this simple issue we either wrongly talked about variable 
template-id in c++17 mode or ICEd in c++2a. I think we simply want 
to handle concept-ids first, both as represented in c++17 mode and 
as represented in c++2a mode. Tested x86_64-linux.
What happens if you try to use a function template/function concept 
name?


AFAICS no ICEs, no regressions but indeed we can do better, tell 
concepts from function templates. The below does that and passes 
testing but I'm not sure if the wording is optimal, whether we always 
want to talk about concept-id.


I think it's fine either way.


+// { dg-do compile { target c++17 } }
+// { dg-options "-w -fconcepts" }


Does it work to use -fconcepts-ts instead of -w -fconcepts?


Sure, it does. Attached what I tested.

Thanks, Paolo.

//

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index caafbefda8e..85a4ea5be82 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6467,16 +6467,27 @@ cp_parser_nested_name_specifier_opt (cp_parser *parser,
  tree fns = get_fns (tid);
  if (OVL_SINGLE_P (fns))
tmpl = OVL_FIRST (fns);
- error_at (token->location, "function template-id %qD "
-   "in nested-name-specifier", tid);
+ if (function_concept_p (fns))
+   error_at (token->location, "concept-id %qD "
+ "in nested-name-specifier", tid);
+ else
+   error_at (token->location, "function template-id "
+ "%qD in nested-name-specifier", tid);
}
  else
{
- /* Variable template.  */
  tmpl = TREE_OPERAND (tid, 0);
- gcc_assert (variable_template_p (tmpl));
- error_at (token->location, "variable template-id %qD "
-   "in nested-name-specifier", tid);
+ if (variable_concept_p (tmpl)
+ || standard_concept_p (tmpl))
+   error_at (token->location, "concept-id %qD "
+ "in nested-name-specifier", tid);
+ else
+   {
+ /* Variable template.  */
+ gcc_assert (variable_template_p (tmpl));
+ error_at (token->location, "variable template-id "
+   "%qD in nested-name-specifier", tid);
+   }
}
  if (tmpl)
inform (DECL_SOURCE_LOCATION (tmpl),
diff --git a/gcc/testsuite/g++.dg/concepts/pr92804-1.C 
b/gcc/testsuite/g++.dg/concepts/pr92804-1.C
new file mode 100644
index 000..cc21426bb9e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr92804-1.C
@@ -0,0 +1,19 @@
+// { dg-do compile { target c++17 } }
+// { dg-options "-fconcepts" }
+
+template
+concept foo = true;  // { dg-message "declared here" }
+
+template
+void bar(T t)
+{
+  if constexpr (foo::value)  // { dg-error "17:concept-id .foo. in 
nested-name-specifier" }
+  // { dg-error "expected|value" "" { target c++17 } .-1 }
+  {
+  }
+}
+
+int main()
+{
+  bar(1);
+}
diff --git a/gcc/testsuite/g++.dg/concepts/pr92804-2.C 
b/gcc/testsuite/g++.dg/concepts/pr92804-2.C
new file mode 100644
index 000..32a15543310
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr92804-2.C
@@ -0,0 +1,19 @@
+// { dg-do compile { target c++17 } }
+// { dg-options "-fconcepts-ts" }
+
+template
+concept bool foo() { return true; };  // { dg-message "declared here" }
+
+template
+void bar(T t)
+{
+  if constexpr (foo::value)  // { dg-error "17:concept-id .foo. in 
nested-name-specifier" }
+  // { dg-error "expected|value" "" { target *-*-* } .-1 }
+  {
+  }
+}
+
+int main()
+{
+  bar(1);
+}


Re: [PATCH] wide-int: i386: Fix ICEs on TImode signed overflow add/sub patterns [PR93376]

2020-01-23 Thread Richard Sandiford
Jakub Jelinek  writes:
> Hi!
>
> The following testcase ICEs, because during try_combine of i3:
> (insn 18 17 19 2 (parallel [
> (set (reg:CCO 17 flags)
> (eq:CCO (plus:OI (sign_extend:OI (reg:TI 96))
> (const_int 1 [0x1]))
> (sign_extend:OI (plus:TI (reg:TI 96)
> (const_int 1 [0x1])
> (set (reg:TI 98)
> (plus:TI (reg:TI 96)
> (const_int 1 [0x1])))
> ]) "pr93376.c":8:10 223 {*addvti4_doubleword_1}
>  (expr_list:REG_UNUSED (reg:TI 98)
> (expr_list:REG_DEAD (reg:TI 96)
> (nil
> and i2:
> (insn 17 37 18 2 (set (reg:TI 96)
> (const_wide_int 0x7fff)) "pr93376.c":8:10 
> 65 {*movti_internal}
>  (nil))
> the eq in there gets simplified into:
> (eq:CCO (const_wide_int 0x08000)
> (const_wide_int 0x8000))
> and simplify-rtx.c tries to simplify it by simplifying MINUS
> of the two operands.
> Now, i386 defines MAX_BITSIZE_MODE_ANY_INT to 128, because OImode
> and XImode are used mainly as a placeholder for the vector modes;
> these new signed overflow patterns are an exception to that,
> but what they really need is just TImode precision + 1 (maybe 2 worst case)
> bits at any time.
>
> wide-int.h defines WIDE_INT_MAX_ELTS in a way that it contains one more
> HWI above number of HWIs to cover WIDE_INT_MAX_ELTS, so on i386 that is
> 3 HWIs, meaning that TImode precision + 1/2 bits is still representable in
> there.  Unfortunately, the way wi::sub_large is implemented, it needs
> not just those 3 HWIs, but one HWI above the maximum of the lengths of
> both operands, which means it buffer overflows, overwrites the following
> precision in wide_int_storage and ICEs later on.  The need for 4 HWIs is
> only temporary, because canonize immediately after it canonicalizes it
> back to 3 HWIs only.
>
> Attached are two patches, the first one is admittedly a hack, but could be
> considered an optimization too, simply before overwriting the last HWI
> ({add,sub}_large seem to be the only one that have such len++) perform
> a cheap check if canonize won't optimize it away immediately and in such
> case don't store it.  Yes, we are playing with fire because full OImode
> is 256 bits and we can't store that many, but we in the end only need 129
> bits.
>
> The other patch is something suggested by Richard S., avoid using OImode
> for this and instead use a partial int mode that is smaller.  This is still
> playing with fire because even the partial int mode is larger than
> MAX_BITSIZE_MODE_ANY_INT, but at least its precision fits into those 3
> WIDE_INT_MAX_ELTS wide-int.h uses.

I think we should increase MAX_BITSIZE_MODE_ANY_INT to the precision
of POImode at the same time, and make that precision less than 192
(191 maybe?).  That way everything is "correct" without increasing
the size of wide_int.

> The disadvantage of that approach is that it is more costly at compile
> time, various RA data structures contains number_of_modes^2 sized
> arrays, and RA initialization will want to compute at runtime various
> properties of each of the modes.

True, but if that's a real source of slow compilation in practice then
we should do something about it independently of this, since many mode
combinations make no sense.

If I've counted correctly, x86 already has 109 modes, so we're talking
about less than a 1% increase in O(modes) operations and less than a 1.9%
increase in O(modes^2) operations.

Thanks,
Richard

> I've tried to think about other ways how to represent signed overflow check
> in RTL, but didn't find any that would actually properly describe it and not
> be way too complicated for the patterns.
>
> So, my preference is the first patch, but Richard S. doesn't like that much.
>
> Both patches have been bootstrapped/regtested on x86_64-linux and
> i686-linux, ok for trunk (which one)?
>
>   Jakub


Re: [PATCH] i386: Use bzhi for x & ((1 << y) - 1) or x & ((1U << y) - 1) [PR93346]

2020-01-23 Thread Uros Bizjak
On Thu, Jan 23, 2020 at 8:56 AM Jakub Jelinek  wrote:
>
> Hi!
>
> The bzhi patterns are quite complicated because they need to accurately
> describe the behavior of the instruction for all input values.
> The following patterns are simple and make bzhi recognizable even for
> cases where not all input values are valid, because the user used
> a shift, in which case the low 8 bit of the last operand need to be in
> between 0 and precision-1.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2020-01-23  Jakub Jelinek  
>
> PR target/93346
> * config/i386/i386.md (*bmi2_bzhi_3_2, *bmi2_bzhi_3_3):
> New define_insn patterns.
>
> * gcc.target/i386/pr93346.c: New test.

We could also define zero_extended version for x86_64, but I don't
think it is worth complication.

OK.

Thanks,
Uros.

> --- gcc/config/i386/i386.md.jj  2020-01-22 09:49:27.379413302 +0100
> +++ gcc/config/i386/i386.md 2020-01-22 13:34:35.226270365 +0100
> @@ -14304,6 +14304,35 @@ (define_insn "*bmi2_bzhi_3_1_ccz"
> (set_attr "prefix" "vex")
> (set_attr "mode" "")])
>
> +(define_insn "*bmi2_bzhi_3_2"
> +  [(set (match_operand:SWI48 0 "register_operand" "=r")
> +   (and:SWI48
> + (plus:SWI48
> +   (ashift:SWI48 (const_int 1)
> + (match_operand:QI 2 "register_operand" "r"))
> +   (const_int -1))
> + (match_operand:SWI48 1 "nonimmediate_operand" "rm")))
> +   (clobber (reg:CC FLAGS_REG))]
> +  "TARGET_BMI2"
> +  "bzhi\t{%2, %1, %0|%0, %1, %2}"
> +  [(set_attr "type" "bitmanip")
> +   (set_attr "prefix" "vex")
> +   (set_attr "mode" "")])
> +
> +(define_insn "*bmi2_bzhi_3_3"
> +  [(set (match_operand:SWI48 0 "register_operand" "=r")
> +   (and:SWI48
> + (not:SWI48
> +   (ashift:SWI48 (const_int -1)
> + (match_operand:QI 2 "register_operand" "r")))
> + (match_operand:SWI48 1 "nonimmediate_operand" "rm")))
> +   (clobber (reg:CC FLAGS_REG))]
> +  "TARGET_BMI2"
> +  "bzhi\t{%2, %1, %0|%0, %1, %2}"
> +  [(set_attr "type" "bitmanip")
> +   (set_attr "prefix" "vex")
> +   (set_attr "mode" "")])
> +
>  (define_insn "bmi2_pdep_3"
>[(set (match_operand:SWI48 0 "register_operand" "=r")
>  (unspec:SWI48 [(match_operand:SWI48 1 "register_operand" "r")
> --- gcc/testsuite/gcc.target/i386/pr93346.c.jj  2020-01-22 13:42:27.907101420 
> +0100
> +++ gcc/testsuite/gcc.target/i386/pr93346.c 2020-01-22 13:42:09.330383163 
> +0100
> @@ -0,0 +1,76 @@
> +/* PR target/93346 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -mbmi2" } */
> +/* { dg-final { scan-assembler-times "\tbzhi\t" 12 } } */
> +
> +unsigned int
> +f1 (unsigned int x, unsigned int y)
> +{
> +  return x & ((1 << y) - 1);
> +}
> +
> +unsigned int
> +f2 (unsigned int x, unsigned int y)
> +{
> +  return x & ((1U << y) - 1);
> +}
> +
> +int
> +f3 (int x, unsigned int y)
> +{
> +  return x & ((1 << y) - 1);
> +}
> +
> +unsigned long
> +f4 (unsigned long x, unsigned int y)
> +{
> +  return x & ((1L << y) - 1);
> +}
> +
> +unsigned long
> +f5 (unsigned long x, unsigned int y)
> +{
> +  return x & ((1UL << y) - 1);
> +}
> +
> +long
> +f6 (long x, unsigned int y)
> +{
> +  return x & ((1L << y) - 1);
> +}
> +
> +unsigned int
> +f7 (unsigned int x, int y)
> +{
> +  return x & ((1 << y) - 1);
> +}
> +
> +unsigned int
> +f8 (unsigned int x, int y)
> +{
> +  return x & ((1U << y) - 1);
> +}
> +
> +int
> +f9 (int x, int y)
> +{
> +  return x & ((1 << y) - 1);
> +}
> +
> +unsigned long
> +f10 (unsigned long x, int y)
> +{
> +  return x & ((1L << y) - 1);
> +}
> +
> +unsigned long
> +f11 (unsigned long x, int y)
> +{
> +  return x & ((1UL << y) - 1);
> +}
> +
> +long
> +f12 (long x, int y)
> +{
> +  return x & ((1L << y) - 1);
> +}
>
> Jakub
>


[wwwdocs] Abstract a statement to refer to our infrastructure, no specifics.

2020-01-23 Thread Gerald Pfeifer
Another reference to svn gone - and more abstract in case of another
change in 2049.

Pushed.

---
 htdocs/contribute.html | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/htdocs/contribute.html b/htdocs/contribute.html
index 042ff069..b671354e 100644
--- a/htdocs/contribute.html
+++ b/htdocs/contribute.html
@@ -186,7 +186,7 @@ be plaintext rather than part of the patch since the top of 
the
 ChangeLog changes rapidly and a patch to the ChangeLog would probably
 no longer apply by the time your patch is reviewed.
 If your change fixes a PR, put text in the ChangeLog entry mentioning
-the PR.  The svn commit machinery understands how to
+the PR.  Our infrastructure understands how to
 extract this information and automatically append the commit log to
 the PR.  In order to be recognized, the text must fit a particular
 form.  It must start with "PR", and then must include the category
-- 
2.24.1


[PATCH] wide-int: i386: Fix ICEs on TImode signed overflow add/sub patterns [PR93376]

2020-01-23 Thread Jakub Jelinek
Hi!

The following testcase ICEs, because during try_combine of i3:
(insn 18 17 19 2 (parallel [
(set (reg:CCO 17 flags)
(eq:CCO (plus:OI (sign_extend:OI (reg:TI 96))
(const_int 1 [0x1]))
(sign_extend:OI (plus:TI (reg:TI 96)
(const_int 1 [0x1])
(set (reg:TI 98)
(plus:TI (reg:TI 96)
(const_int 1 [0x1])))
]) "pr93376.c":8:10 223 {*addvti4_doubleword_1}
 (expr_list:REG_UNUSED (reg:TI 98)
(expr_list:REG_DEAD (reg:TI 96)
(nil
and i2:
(insn 17 37 18 2 (set (reg:TI 96)
(const_wide_int 0x7fff)) "pr93376.c":8:10 
65 {*movti_internal}
 (nil))
the eq in there gets simplified into:
(eq:CCO (const_wide_int 0x08000)
(const_wide_int 0x8000))
and simplify-rtx.c tries to simplify it by simplifying MINUS
of the two operands.
Now, i386 defines MAX_BITSIZE_MODE_ANY_INT to 128, because OImode
and XImode are used mainly as a placeholder for the vector modes;
these new signed overflow patterns are an exception to that,
but what they really need is just TImode precision + 1 (maybe 2 worst case)
bits at any time.

wide-int.h defines WIDE_INT_MAX_ELTS in a way that it contains one more
HWI above number of HWIs to cover WIDE_INT_MAX_ELTS, so on i386 that is
3 HWIs, meaning that TImode precision + 1/2 bits is still representable in
there.  Unfortunately, the way wi::sub_large is implemented, it needs
not just those 3 HWIs, but one HWI above the maximum of the lengths of
both operands, which means it buffer overflows, overwrites the following
precision in wide_int_storage and ICEs later on.  The need for 4 HWIs is
only temporary, because canonize immediately after it canonicalizes it
back to 3 HWIs only.

Attached are two patches, the first one is admittedly a hack, but could be
considered an optimization too, simply before overwriting the last HWI
({add,sub}_large seem to be the only one that have such len++) perform
a cheap check if canonize won't optimize it away immediately and in such
case don't store it.  Yes, we are playing with fire because full OImode
is 256 bits and we can't store that many, but we in the end only need 129
bits.

The other patch is something suggested by Richard S., avoid using OImode
for this and instead use a partial int mode that is smaller.  This is still
playing with fire because even the partial int mode is larger than
MAX_BITSIZE_MODE_ANY_INT, but at least its precision fits into those 3
WIDE_INT_MAX_ELTS wide-int.h uses.  The disadvantage of that approach is
that it is more costly at compile time, various RA data structures contains
number_of_modes^2 sized arrays, and RA initialization will want to compute
at runtime various properties of each of the modes.

I've tried to think about other ways how to represent signed overflow check
in RTL, but didn't find any that would actually properly describe it and not
be way too complicated for the patterns.

So, my preference is the first patch, but Richard S. doesn't like that much.

Both patches have been bootstrapped/regtested on x86_64-linux and
i686-linux, ok for trunk (which one)?

Jakub
2020-01-23  Jakub Jelinek  

PR target/93376
* wide-int.cc (wi::add_large, wi::sub_large): Don't extend len by
writing SIGN_MASK of the highest element that canonize would remove
again.

* gcc.dg/pr93376.c: New test.

--- gcc/wide-int.cc.jj  2020-01-12 11:54:38.535381394 +0100
+++ gcc/wide-int.cc 2020-01-22 12:05:06.739486117 +0100
@@ -1155,8 +1155,14 @@ wi::add_large (HOST_WIDE_INT *val, const
 
   if (len * HOST_BITS_PER_WIDE_INT < prec)
 {
-  val[len] = mask0 + mask1 + carry;
-  len++;
+  HOST_WIDE_INT val_top = mask0 + mask1 + carry;
+  /* Don't extend len unnecessarily when canonize would shrink it
+again immediately.  */
+  if (SIGN_MASK (val[len - 1]) != val_top)
+   {
+ val[len] = val_top;
+ len++;
+   }
   if (overflow)
*overflow
  = (sgn == UNSIGNED && carry) ? wi::OVF_OVERFLOW : wi::OVF_NONE;
@@ -1566,8 +1572,14 @@ wi::sub_large (HOST_WIDE_INT *val, const
 
   if (len * HOST_BITS_PER_WIDE_INT < prec)
 {
-  val[len] = mask0 - mask1 - borrow;
-  len++;
+  HOST_WIDE_INT val_top = mask0 - mask1 - borrow;
+  /* Don't extend len unnecessarily when canonize would shrink it
+again immediately.  */
+  if (SIGN_MASK (val[len - 1]) != val_top)
+   {
+ val[len] = val_top;
+ len++;
+   }
   if (overflow)
*overflow = (sgn == UNSIGNED && borrow) ? OVF_UNDERFLOW : OVF_NONE;
 }
--- gcc/testsuite/gcc.dg/pr93376.c.jj   2020-01-22 12:16:40.231937796 +0100
+++ gcc/testsuite/gcc.dg/pr93376.c  2020-01-22 12:16:23.953190057 +0100
@@ -0,0 +1,20 @@
+/* PR target/93376 */
+/* { dg-do compile { target