Re: [PATCH] Add define_insn_and_split for combine to detect x < 123U ? -1 : 0 (PR target/88425)

[Jakub Jelinek]

On Tue, Dec 11, 2018 at 08:44:00AM +0100, Uros Bizjak wrote:
> > --- gcc/config/i386/i386.md.jj  2018-11-22 10:40:31.179683319 +0100
> > +++ gcc/config/i386/i386.md 2018-12-10 11:24:49.785830186 +0100
> > @@ -17195,6 +17195,24 @@ (define_insn "*x86_movcc_0_m1_neg"
> > (set_attr "mode" "")
> > (set_attr "length_immediate" "0")])
> >
> > +(define_insn_and_split "*x86_movcc_0_m1_neg_leu"
> > +  [(set (match_operand:SWI48 0 "register_operand" "=r")
> > +   (neg:SWI48
> > + (leu:SWI48
> > +   (match_operand:SWI 1 "nonimmediate_operand" "m")
> > +   (match_operand:SWI 2 "" ""
> 
> You can use const_int_operand predicate with "n" constraint here.

The point was to disallow 64-bit constants, so if I use const_int_operand
above, I'd need to replace CONST_INT_P (operands[2]) test with
trunc_int_for_mode (INTVAL (operands[2]), SImode) == INTVAL (operands[2])
or similar, perhaps do it for the 64-bit mode only, so
  (mode != DImode
   || (trunc_int_for_mode (INTVAL (operands[2]), SImode)
   == INTVAL (operands[2])))

> > +   (clobber (reg:CC FLAGS_REG))]
> > +  "CONST_INT_P (operands[2])
> > +   && INTVAL (operands[2]) != -1
> > +   && INTVAL (operands[2]) != 2147483647"
> 
> Can UINTVAL be used here?

Just for the latter, or for both?  For the first one it would
require UINTVAL (operands[2]) != HOST_WIDE_INT_M1U
or so.

Jakub

Re: [PATCH] Make LTO tests that require recent binutils UNSUPPORTED with older ones (PR lto/86004)

[Rainer Orth]

Hi Jakub,

> As mentioned in the PR, older binutils (like 2.25) complain on these tests
> when using -r that:
> plugin needed to handle lto object
> The following patch introduces an effective target and guards those tests on
> a linker with this issue fixed.
>
> Regtested on x86_64-linux and i686-linux, plus regtested on x86_64-linux
> with 2.25 binutils, where the patch converts all those FAILs into
> UNSUPPORTEDs.
>
> Ok for trunk?

please document the new effective-target keyword in sourcebuild.texi.

Thanks.
Rainer

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

Re: [PATCH] Add define_insn_and_split for combine to detect x < 123U ? -1 : 0 (PR target/88425)

[Uros Bizjak]

On Tue, Dec 11, 2018 at 8:27 AM Jakub Jelinek  wrote:
>
> Hi!
>
> For the following testcase (x < 123U ? -1U : 0) we emit worse code than for
> (x < y ? -1U : 0).  That is because the generic code canonicalizes x < 123U
> comparisons into x <= 122U.  For this particular case, we want LTU though,
> as that sets carry and we can then just sbb.
>
> The following patch adds an insn with splitter for this.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2018-12-11  Jakub Jelinek  
>
> PR target/88425
> * config/i386/i386.md (*x86_movcc_0_m1_neg_leu):
> New define_insn_and_split.
>
> * gcc.target/i386/pr88425.c: New test.
>
> --- gcc/config/i386/i386.md.jj  2018-11-22 10:40:31.179683319 +0100
> +++ gcc/config/i386/i386.md 2018-12-10 11:24:49.785830186 +0100
> @@ -17195,6 +17195,24 @@ (define_insn "*x86_movcc_0_m1_neg"
> (set_attr "mode" "")
> (set_attr "length_immediate" "0")])
>
> +(define_insn_and_split "*x86_movcc_0_m1_neg_leu"
> +  [(set (match_operand:SWI48 0 "register_operand" "=r")
> +   (neg:SWI48
> + (leu:SWI48
> +   (match_operand:SWI 1 "nonimmediate_operand" "m")
> +   (match_operand:SWI 2 "" ""

You can use const_int_operand predicate with "n" constraint here.

> +   (clobber (reg:CC FLAGS_REG))]
> +  "CONST_INT_P (operands[2])
> +   && INTVAL (operands[2]) != -1
> +   && INTVAL (operands[2]) != 2147483647"

Can UINTVAL be used here?

Uros.

> +  "#"
> +  ""
> +  [(set (reg:CC FLAGS_REG) (compare:CC (match_dup 1) (match_dup 2)))
> +   (parallel [(set (match_dup 0)
> +  (neg:SWI48 (ltu:SWI48 (reg:CC FLAGS_REG) (const_int 0
> + (clobber (reg:CC FLAGS_REG))])]
> +  "operands[2] = GEN_INT (INTVAL (operands[2]) + 1);")
> +
>  (define_insn "*movcc_noc"
>[(set (match_operand:SWI248 0 "register_operand" "=r,r")
> (if_then_else:SWI248 (match_operator 1 "ix86_comparison_operator"
> --- gcc/testsuite/gcc.target/i386/pr88425.c.jj  2018-12-10 11:31:35.507196453 
> +0100
> +++ gcc/testsuite/gcc.target/i386/pr88425.c 2018-12-10 11:31:17.231495274 
> +0100
> @@ -0,0 +1,53 @@
> +/* PR target/88425 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -masm=att" } */
> +/* { dg-final { scan-assembler-times "sbb\[lq]\[ \t]" 8 } } */
> +/* { dg-final { scan-assembler-not "setbe\[ \t]" } } */
> +
> +unsigned long
> +f1 (unsigned long x)
> +{
> +  return x < 123UL ? -1UL : 0;
> +}
> +
> +unsigned long
> +f2 (unsigned int x)
> +{
> +  return x < 12345U ? -1UL : 0;
> +}
> +
> +unsigned long
> +f3 (unsigned short *x)
> +{
> +  return x[0] < 1234U ? -1UL : 0;
> +}
> +
> +unsigned long
> +f4 (unsigned char *x)
> +{
> +  return x[0] < 123U ? -1UL : 0;
> +}
> +
> +unsigned int
> +f5 (unsigned long x)
> +{
> +  return x < 123UL ? -1U : 0;
> +}
> +
> +unsigned int
> +f6 (unsigned int x)
> +{
> +  return x < 12345U ? -1U : 0;
> +}
> +
> +unsigned int
> +f7 (unsigned short *x)
> +{
> +  return x[0] < 1234U ? -1U : 0;
> +}
> +
> +unsigned int
> +f8 (unsigned char *x)
> +{
> +  return x[0] < 123U ? -1U : 0;
> +}
>
> Jakub

[PATCH] Make LTO tests that require recent binutils UNSUPPORTED with older ones (PR lto/86004)

[Jakub Jelinek]

Hi!

As mentioned in the PR, older binutils (like 2.25) complain on these tests
when using -r that:
plugin needed to handle lto object
The following patch introduces an effective target and guards those tests on
a linker with this issue fixed.

Regtested on x86_64-linux and i686-linux, plus regtested on x86_64-linux
with 2.25 binutils, where the patch converts all those FAILs into
UNSUPPORTEDs.

Ok for trunk?

2018-12-11  Jakub Jelinek  

PR lto/86004
* lib/target-supports.exp (check_effective_target_lto_incremental):
New.
* g++.dg/lto/pr69137_0.C: Require lto_incremental effective target.
* g++.dg/lto/pr65316_0.C: Likewise.
* g++.dg/lto/pr85176_0.C: Likewise.
* g++.dg/lto/pr79000_0.C: Likewise.
* g++.dg/lto/pr66180_0.C: Likewise.
* g++.dg/lto/pr65193_0.C: Likewise.
* g++.dg/lto/pr69077_0.C: Likewise.
* g++.dg/lto/pr68057_0.C: Likewise.
* g++.dg/lto/pr66705_0.C: Likewise.
* g++.dg/lto/pr65302_0.C: Likewise.
* g++.dg/lto/20091002-1_0.C: Likewise.
* g++.dg/lto/pr81940_0.C: Likewise.
* g++.dg/lto/pr64043_0.C: Likewise.
* g++.dg/lto/pr65549_0.C: Likewise.
* g++.dg/lto/pr69133_0.C: Likewise.
* gfortran.dg/lto/pr79108_0.f90: Likewise.

--- gcc/testsuite/lib/target-supports.exp.jj2018-12-04 20:40:01.491379395 
+0100
+++ gcc/testsuite/lib/target-supports.exp   2018-12-10 15:39:37.714763670 
+0100
@@ -8014,6 +8014,18 @@ proc check_effective_target_lto { } {
 } "-flto"]
 }
 
+# Return 1 if the compiler and linker support incremental link-time
+# optimization.
+
+proc check_effective_target_lto_incremental { } {
+if ![check_effective_target_lto] {
+   return 0
+}
+return [check_no_compiler_messages lto_incremental executable {
+   int main () { return 0; }
+} "-flto -r -nostdlib"]
+}
+
 # Return 1 if -mx32 -maddress-mode=short can compile, 0 otherwise.
 
 proc check_effective_target_maybe_x32 { } {
--- gcc/testsuite/g++.dg/lto/pr69137_0.C.jj 2016-01-15 20:37:28.533589631 
+0100
+++ gcc/testsuite/g++.dg/lto/pr69137_0.C2018-12-10 15:42:38.355806095 
+0100
@@ -1,4 +1,5 @@
 // { dg-lto-do link }
+// { dg-require-effective-target lto_incremental }
 // { dg-lto-options { { -std=c++11 -g -flto } } }
 // { dg-extra-ld-options "-r -nostdlib" }
 
--- gcc/testsuite/g++.dg/lto/pr65316_0.C.jj 2017-11-06 17:24:08.773286226 
+0100
+++ gcc/testsuite/g++.dg/lto/pr65316_0.C2018-12-10 15:41:01.092398551 
+0100
@@ -1,4 +1,5 @@
 // { dg-lto-do link }
+// { dg-require-effective-target lto_incremental }
 // { dg-lto-options { { -flto -std=c++11 -g2 -fno-lto-odr-type-merging -O2 
-Wno-return-type } } }
 // { dg-extra-ld-options "-r -nostdlib -O2 -fno-lto-odr-type-merging" }
 
--- gcc/testsuite/g++.dg/lto/pr85176_0.C.jj 2018-04-04 16:13:42.972551086 
+0200
+++ gcc/testsuite/g++.dg/lto/pr85176_0.C2018-12-10 15:43:20.838110547 
+0100
@@ -1,4 +1,5 @@
 // { dg-lto-do link }
+// { dg-require-effective-target lto_incremental }
 // { dg-lto-options { { -flto -g1 } } }
 // { dg-extra-ld-options "-r -nostdlib" }
 namespace a {
--- gcc/testsuite/g++.dg/lto/pr79000_0.C.jj 2017-01-09 11:35:04.310821717 
+0100
+++ gcc/testsuite/g++.dg/lto/pr79000_0.C2018-12-10 15:42:54.657539190 
+0100
@@ -1,4 +1,5 @@
 // { dg-lto-do link }
+// { dg-require-effective-target lto_incremental }
 // { dg-lto-options { "-flto -g" } }
 // { dg-extra-ld-options "-r -nostdlib" }
 
--- gcc/testsuite/g++.dg/lto/pr66180_0.C.jj 2015-05-29 15:04:33.064803028 
+0200
+++ gcc/testsuite/g++.dg/lto/pr66180_0.C2018-12-10 15:41:39.730765940 
+0100
@@ -1,4 +1,5 @@
 // { dg-lto-do link }
+// { dg-require-effective-target lto_incremental }
 // { dg-lto-options { { -flto -std=c++14 -r -nostdlib } } }
 #include 
 namespace {
--- gcc/testsuite/g++.dg/lto/pr65193_0.C.jj 2017-11-06 17:24:08.774286214 
+0100
+++ gcc/testsuite/g++.dg/lto/pr65193_0.C2018-12-10 15:40:23.564012993 
+0100
@@ -1,5 +1,6 @@
 /* { dg-lto-do link } */
 /* { dg-require-effective-target fpic } */
+/* { dg-require-effective-target lto_incremental } */
 /* { dg-lto-options {{-fPIC -r -nostdlib -flto -O2 -g -Wno-return-type}} } */
 
 void frexp (int, int *);
--- gcc/testsuite/g++.dg/lto/pr69077_0.C.jj 2017-11-06 17:24:08.770286263 
+0100
+++ gcc/testsuite/g++.dg/lto/pr69077_0.C2018-12-10 15:42:19.886108489 
+0100
@@ -1,4 +1,5 @@
 // { dg-lto-do link }
+// { dg-require-effective-target lto_incremental }
 // { dg-lto-options { { -O3 -g -flto } } }
 // { dg-extra-ld-options "-r -nostdlib" }
 
--- gcc/testsuite/g++.dg/lto/pr68057_0.C.jj 2015-11-09 13:39:21.418394784 
+0100
+++ gcc/testsuite/g++.dg/lto/pr68057_0.C2018-12-10 15:42:08.058302145 
+0100
@@ -1,4 +1,5 @@
 // { dg-lto-do link }
+// { dg-require-effective-target lto_incremental }
 /* { dg-extra-ld-options { -O2 -Wno-odr -r -nostdlib } } */
 struct SPxPricer;
 struct SoPlex {
--- 

[PATCH] Add define_insn_and_split for combine to detect x < 123U ? -1 : 0 (PR target/88425)

[Jakub Jelinek]

Hi!

For the following testcase (x < 123U ? -1U : 0) we emit worse code than for
(x < y ? -1U : 0).  That is because the generic code canonicalizes x < 123U
comparisons into x <= 122U.  For this particular case, we want LTU though,
as that sets carry and we can then just sbb.

The following patch adds an insn with splitter for this.

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

2018-12-11  Jakub Jelinek  

PR target/88425
* config/i386/i386.md (*x86_movcc_0_m1_neg_leu):
New define_insn_and_split.

* gcc.target/i386/pr88425.c: New test.

--- gcc/config/i386/i386.md.jj  2018-11-22 10:40:31.179683319 +0100
+++ gcc/config/i386/i386.md 2018-12-10 11:24:49.785830186 +0100
@@ -17195,6 +17195,24 @@ (define_insn "*x86_movcc_0_m1_neg"
(set_attr "mode" "")
(set_attr "length_immediate" "0")])
 
+(define_insn_and_split "*x86_movcc_0_m1_neg_leu"
+  [(set (match_operand:SWI48 0 "register_operand" "=r")
+   (neg:SWI48
+ (leu:SWI48
+   (match_operand:SWI 1 "nonimmediate_operand" "m")
+   (match_operand:SWI 2 "" ""
+   (clobber (reg:CC FLAGS_REG))]
+  "CONST_INT_P (operands[2])
+   && INTVAL (operands[2]) != -1
+   && INTVAL (operands[2]) != 2147483647"
+  "#"
+  ""
+  [(set (reg:CC FLAGS_REG) (compare:CC (match_dup 1) (match_dup 2)))
+   (parallel [(set (match_dup 0)
+  (neg:SWI48 (ltu:SWI48 (reg:CC FLAGS_REG) (const_int 0
+ (clobber (reg:CC FLAGS_REG))])]
+  "operands[2] = GEN_INT (INTVAL (operands[2]) + 1);")
+
 (define_insn "*movcc_noc"
   [(set (match_operand:SWI248 0 "register_operand" "=r,r")
(if_then_else:SWI248 (match_operator 1 "ix86_comparison_operator"
--- gcc/testsuite/gcc.target/i386/pr88425.c.jj  2018-12-10 11:31:35.507196453 
+0100
+++ gcc/testsuite/gcc.target/i386/pr88425.c 2018-12-10 11:31:17.231495274 
+0100
@@ -0,0 +1,53 @@
+/* PR target/88425 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -masm=att" } */
+/* { dg-final { scan-assembler-times "sbb\[lq]\[ \t]" 8 } } */
+/* { dg-final { scan-assembler-not "setbe\[ \t]" } } */
+
+unsigned long
+f1 (unsigned long x)
+{
+  return x < 123UL ? -1UL : 0;
+}
+
+unsigned long
+f2 (unsigned int x)
+{
+  return x < 12345U ? -1UL : 0;
+}
+
+unsigned long
+f3 (unsigned short *x)
+{
+  return x[0] < 1234U ? -1UL : 0;
+}
+
+unsigned long
+f4 (unsigned char *x)
+{
+  return x[0] < 123U ? -1UL : 0;
+}
+
+unsigned int
+f5 (unsigned long x)
+{
+  return x < 123UL ? -1U : 0;
+}
+
+unsigned int
+f6 (unsigned int x)
+{
+  return x < 12345U ? -1U : 0;
+}
+
+unsigned int
+f7 (unsigned short *x)
+{
+  return x[0] < 1234U ? -1U : 0;
+}
+
+unsigned int
+f8 (unsigned char *x)
+{
+  return x[0] < 123U ? -1U : 0;
+}

Jakub

[C PATCH] Fix ubsan -fsanitize=float-cast-overflow ICE (PR sanitizer/88426)

[Jakub Jelinek]

Hi!

The following testcase ICEs since the c_save_expr removal.  Unlike other
spots where we use save_expr and potentially pass that to function ubsan
calls, in this case we weren't calling c_fully_fold and
c_fully_fold_internal unfortunately doesn't recurse into CALL_EXPRs, so
the gimplifier then sees C_MAYBE_CONST_EXPRs and ICEs on them.  E.g.
for shift sanitization etc. we call c_fully_fold like this.

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

2018-12-11  Jakub Jelinek  

PR sanitizer/88426
* c-convert.c (convert): Call c_fully_fold before calling
ubsan_instrument_float_cast.

* c-c++-common/ubsan/float-cast-overflow-11.c: New test.

--- gcc/c/c-convert.c.jj2018-01-03 10:20:20.119537950 +0100
+++ gcc/c/c-convert.c   2018-12-10 09:26:57.846455754 +0100
@@ -115,6 +115,7 @@ convert (tree type, tree expr)
  && COMPLETE_TYPE_P (type))
{
  expr = save_expr (expr);
+ expr = c_fully_fold (expr, false, NULL);
  tree check = ubsan_instrument_float_cast (loc, type, expr);
  expr = fold_build1 (FIX_TRUNC_EXPR, type, expr);
  if (check == NULL_TREE)
--- gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-11.c.jj
2018-12-10 09:30:05.548386877 +0100
+++ gcc/testsuite/c-c++-common/ubsan/float-cast-overflow-11.c   2018-12-10 
09:29:49.027656990 +0100
@@ -0,0 +1,10 @@
+/* PR sanitizer/88426 */
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=float-cast-overflow" } */
+
+int
+foo (void)
+{
+  const float v = 0.0f;
+  return (int) (v < 0.0f ? v : 0.0f);
+}

Jakub

Re: [PATCH] accept all C integer types in function parameters referenced by alloc_align (PR 88363)

[Jakub Jelinek]

On Mon, Dec 10, 2018 at 04:30:11PM -0700, Martin Sebor wrote:
> Some of my testing exposed a minor problem in GCC 9's validation
> of the type of function parameters referred to by attribute
> positional arguments.  Whereas GCC 8 accepts all C integer types,
> including enumerated types, such as:
> 
>   enum AllocAlign { Align16 = 16, Align32 = 32 };
> 
>   __attribute__ ((alloc_align (1))) void*
>   alloc (size_t, enum AllocAlign)
> 
> GCC 9 only accepts signed and unsigned integer types.  This change
> (introduced by myself) was unintentional, and a fix for it is in
> the attached trivial patch.  I plan to commit it without approval
> in the next day or so unless any concerns or suggestions come up.

There is nothing obvious about this, so please don't commit it without
approval.  GCC 8 and older used to accept
even float or void * or struct arguments and just ignored them.
I think we need to discuss what types we want to allow without warnings and
what we should warn.
As I wrote in the PR, I believe e.g. using alloc_align/alloc_size with
bool/_Bool is just a clear bug, you can store 0 or 1 in there, but e.g.
alignment 0 doesn't make sense.
Enums are on the border line, I'll defer to C/C++ maintainers whether we
want to include that or not.

Jakub

Re: [PATCH] Set DEMANGLE_RECURSION_LIMIT to 1536

[Jakub Jelinek]

On Mon, Dec 10, 2018 at 05:33:19PM -0700, Jeff Law wrote:
> >> where di.num_comps is just strlen (mangled) * 2.  Without any analysis
> >> whatsoever, bumping the "recursion" limit will just mean we can process 1.5
> >> times long names.  Either we need more precise analysis on what we are
> >> looking for (how big arrays we'll need) or it needs to be an independent
> >> limit and certainly should allow say 10KB symbols too if they are
> >> reasonable.
> > 
> > If the problem is alloca, we could avoid using alloca if the size
> > passes a threshold.  Perhaps even use a better data structure than a
> > preallocated array based on a guess about the number of components...
> Actually I would strongly suggest avoiding alloca completely.  This
> isn't particularly performance sensitive code and alloca can be abused
> in all kinds of interesting ways.

We can't use malloc, therefore on some targets alloca (or VLAs) are the only
option, and for small sizes even if mmap is available using it is too
costly.

Though, I like Jason's suggestion of just adding a maxinum of the number
of components and number of substitutions and failing if we need more.

Jakub

[committed] [PR tree-optimization/80520] Throttle path splitting slightly.

[Jeff Law]

This is a pre-req for fixing 80520.  Essentially the goal here is to
keep the key code in this form:

  
  [ ... ]
  if (_20 != 0)
goto ; [50.00%]
  else
goto ; [50.00%]

   [local count: 531502203]:
  _18 = _25 ^ 2567483615;

   [local count: 1063004407]:
  # prephitmp_49 = PHI <_25(3), _18(4)>
  _2 = (void *) ivtmp.8_30;
  MEM[base: _2, offset: 0B] = prephitmp_49;
  ivtmp.8_29 = ivtmp.8_30 + 8;
  if (ivtmp.8_29 != _6)
goto ; [98.99%]
  else
goto ; [1.01%]


Split-paths wants to duplicate bb5 into bb4.  It's just not all that
profitable to do so.  We can get ever-so-slightly better code on a
target like microblaze and perhaps others with delay slots and no
conditional move/execution capabilities.  But that seems more like
something we should be tackling at the RTL level.

To finish fixing 80520 we will need to improve the RTL if-conversion
where we presumably can cost things and make a good choice between the
branchy code we have vs straightline code with a conditional move or
conditional execution.  I'm not tackling that yet.

Note that split-path-5 has the same basic structure.  A half-diamond
with a single statement in the middle block that should be trivially
if-convertable if profitable.  So I adjusted that testcase.

Bootstrapped and regression tested on x86_64.  Installing on the trunk
momentarily.

commit d90b13427e4940adabc4320c68ca88513dee2eef
Author: Jeff Law 
Date:   Mon Dec 10 21:46:41 2018 -0700

PR tree-optimization/80520
* gimple-ssa-split-paths.c (is_feasible_trace): Recognize half
diamonds that are likely if convertable.

* gcc.dg/tree-ssa/split-path-5.c: Update expected output.
* gcc.dg/tree-ssa/split-path-11.c: New test.

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 14c52ad64be..eddcdc3f843 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2018-12-10  Jeff Law  
+
+   PR tree-optimization/80520
+   * gimple-ssa-split-paths.c (is_feasible_trace): Recognize half
+   diamonds that are likely if convertable.
+
 2018-12-10  Martin Sebor  
 
PR tree-optimization/86196
diff --git a/gcc/gimple-ssa-split-paths.c b/gcc/gimple-ssa-split-paths.c
index a8515119ce5..91596526045 100644
--- a/gcc/gimple-ssa-split-paths.c
+++ b/gcc/gimple-ssa-split-paths.c
@@ -203,6 +203,98 @@ is_feasible_trace (basic_block bb)
}
 }
 
+  /* Canonicalize the form.  */
+  if (num_stmts_in_pred1 == 0 && num_stmts_in_pred2 == 1)
+{
+  std::swap (pred1, pred2);
+  std::swap (num_stmts_in_pred1, num_stmts_in_pred2);
+}
+
+  /* Another variant.  This one is half-diamond.  */
+  if (num_stmts_in_pred1 == 1 && num_stmts_in_pred2 == 0
+  && dominated_by_p (CDI_DOMINATORS, pred1, pred2))
+{
+  gimple *stmt1 = last_and_only_stmt (pred1);
+
+  /* The only statement in PRED1 must be an assignment that is
+not a good candidate for if-conversion.   This may need some
+generalization.  */
+  if (stmt1 && gimple_code (stmt1) == GIMPLE_ASSIGN)
+   {
+ enum tree_code code1 = gimple_assign_rhs_code (stmt1);
+
+ if (!poor_ifcvt_candidate_code (code1))
+   {
+ tree lhs1 = gimple_assign_lhs (stmt1);
+ tree rhs1 = gimple_assign_rhs1 (stmt1);
+
+ gimple_stmt_iterator gsi;
+ for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next ())
+   {
+ gimple *phi = gsi_stmt (gsi);
+ if ((gimple_phi_arg_def (phi, 0) == lhs1
+  && gimple_phi_arg_def (phi, 1) == rhs1)
+ || (gimple_phi_arg_def (phi, 1) == lhs1
+ && gimple_phi_arg_def (phi, 0) == rhs1))
+   {
+ if (dump_file && (dump_flags & TDF_DETAILS))
+   fprintf (dump_file,
+"Block %d appears to be a join point for "
+"if-convertable half-diamond.\n",
+bb->index);
+ return false;
+   }
+   }
+   }
+   }
+}
+
+  /* Canonicalize the form.  */
+  if (num_stmts_in_pred1 == 0 && num_stmts_in_pred2 == 1)
+{
+  std::swap (pred1, pred2);
+  std::swap (num_stmts_in_pred1, num_stmts_in_pred2);
+}
+
+  /* Another variant.  This one is half-diamond.  */
+  if (num_stmts_in_pred1 == 1 && num_stmts_in_pred2 == 0
+  && dominated_by_p (CDI_DOMINATORS, pred1, pred2))
+{
+  gimple *stmt1 = last_and_only_stmt (pred1);
+
+  /* The only statement in PRED1 must be an assignment that is
+not a good candidate for if-conversion.   This may need some
+generalization.  */
+  if (stmt1 && gimple_code (stmt1) == GIMPLE_ASSIGN)
+   {
+ enum tree_code code1 = gimple_assign_rhs_code (stmt1);
+
+ if (!poor_ifcvt_candidate_code (code1))
+   {
+ tree lhs1 = 

[wwwdocs] gcc-4.7/changes.html -- simplify reference to OpenMP

[Gerald Pfeifer]

At first I was going to change http to https for this link, but
given this is GCC 4.7, simply using a textual reference feels safer.

Applied.

Gerald

Index: gcc-4.7/changes.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.7/changes.html,v
retrieving revision 1.153
diff -u -r1.153 changes.html
--- gcc-4.7/changes.html30 Sep 2018 14:38:51 -  1.153
+++ gcc-4.7/changes.html11 Dec 2018 04:00:39 -
@@ -285,8 +285,7 @@
 New Languages and Language specific improvements
 
   
-Version 3.1 of the http://www.openmp.org/specifications/;>OpenMP specification
+Version 3.1 of the OpenMP specification
 is now supported for the C, C++, and Fortran compilers.
   
 

[libstdc++,doc] Update reference to epubcheck in libstdc++ docs

[Gerald Pfeifer]

Committed.

Gerald

2018-12-10  Gerald Pfeifer  

* doc/xml/manual/documentation_hacking.xml: Update reference
to epubcheck.

Index: libstdc++-v3/doc/xml/manual/documentation_hacking.xml
===
--- libstdc++-v3/doc/xml/manual/documentation_hacking.xml   (revision 
266718)
+++ libstdc++-v3/doc/xml/manual/documentation_hacking.xml   (working copy)
@@ -777,7 +777,7 @@
   
 
   
-   For epub output, the http://www.w3.org/1999/xlink; 
xlink:href="https://sourceforge.net/projects/docbook/files/epub3/;>stylesheets
 for EPUB3 are required. These stylesheets are still in development. To 
validate the created file, http://www.w3.org/1999/xlink; 
xlink:href="https://github.com/IDPF/epubcheck;>epubcheck is necessary.
+   For epub output, the http://www.w3.org/1999/xlink; 
xlink:href="https://sourceforge.net/projects/docbook/files/epub3/;>stylesheets
 for EPUB3 are required. These stylesheets are still in development. To 
validate the created file, http://www.w3.org/1999/xlink; 
xlink:href="https://github.com/w3c/epubcheck;>epubcheck is necessary.
   
 
 

[Committed] PR fortran/87922 -- Additional checks for ASYNCHRONOUS

[Steve Kargl]

I've committed the attached patch to trunk and branch-8.
ChangeLogs on trunk record wrong PR number.  I fix those
someday.

2018-12-10  Steven G. Kargl  

PR fortran/87922
* io.c (gfc_match_open): Additional checks on ASYNCHRONOUS.

2018-12-10  Steven G. Kargl  

PR fortran/87922
* gfortran.dg/io_constraints_8.f90: Update error message.
* gfortran.dg/pr87922.f90: New test.

-- 
Steve
Index: gcc/fortran/io.c
===
--- gcc/fortran/io.c	(revision 266959)
+++ gcc/fortran/io.c	(working copy)
@@ -2205,6 +2205,21 @@ gfc_match_open (void)
   if (!is_char_type ("ASYNCHRONOUS", open->asynchronous))
 	goto cleanup;
 
+  if (open->asynchronous->ts.kind != 1)
+	{
+	  gfc_error ("ASYNCHRONOUS= specifier at %L must be of default "
+		 "CHARACTER kind", >asynchronous->where);
+	  return MATCH_ERROR;
+	}
+
+  if (open->asynchronous->expr_type == EXPR_ARRAY
+	  || open->asynchronous->expr_type == EXPR_STRUCTURE)
+	{
+	  gfc_error ("ASYNCHRONOUS= specifier at %L must be scalar",
+		 >asynchronous->where);
+	  return MATCH_ERROR;
+	}
+
   if (open->asynchronous->expr_type == EXPR_CONSTANT)
 	{
 	  static const char * asynchronous[] = { "YES", "NO", NULL };
@@ -3798,6 +3813,21 @@ if (condition) \
 
   if (!is_char_type ("ASYNCHRONOUS", dt->asynchronous))
 	return MATCH_ERROR;
+
+  if (dt->asynchronous->ts.kind != 1)
+	{
+	  gfc_error ("ASYNCHRONOUS= specifier at %L must be of default "
+		 "CHARACTER kind", >asynchronous->where);
+	  return MATCH_ERROR;
+	}
+
+  if (dt->asynchronous->expr_type == EXPR_ARRAY
+	  || dt->asynchronous->expr_type == EXPR_STRUCTURE)
+	{
+	  gfc_error ("ASYNCHRONOUS= specifier at %L must be scalar",
+		 >asynchronous->where);
+	  return MATCH_ERROR;
+	}
 
   if (!compare_to_allowed_values
 		("ASYNCHRONOUS", asynchronous, NULL, NULL,
Index: gcc/testsuite/gfortran.dg/io_constraints_8.f90
===
--- gcc/testsuite/gfortran.dg/io_constraints_8.f90	(revision 266958)
+++ gcc/testsuite/gfortran.dg/io_constraints_8.f90	(working copy)
@@ -14,7 +14,7 @@ integer :: i
 
 OPEN(99, access=4_'direct') ! { dg-error "must be a character string of default kind" }
 OPEN(99, action=4_'read')   ! { dg-error "must be a character string of default kind" }
-OPEN(99, asynchronous=4_'no')   ! { dg-error "must be a character string of default kind" })
+OPEN(99, asynchronous=4_'no')   ! { dg-error "must be of default CHARACTER kind" }
 OPEN(99, blank=4_'null')! { dg-error "must be a character string of default kind" }
 OPEN(99, decimal=4_'comma') ! { dg-error "must be a character string of default kind" }
 OPEN(99, delim=4_'quote')   ! { dg-error "must be a character string of default kind" }
Index: gcc/testsuite/gfortran.dg/pr87922.f90
===
--- gcc/testsuite/gfortran.dg/pr87922.f90	(nonexistent)
+++ gcc/testsuite/gfortran.dg/pr87922.f90	(working copy)
@@ -0,0 +1,10 @@
+! { dg-do compile }
+! PR fortran/87922
+subroutine p
+   read(1, asynchronous=['no'])   ! { dg-error "must be scalar" }
+   read(1, asynchronous=[character::])! { dg-error "must be scalar" }
+end
+subroutine q
+   write(1, asynchronous=['no'])  ! { dg-error "must be scalar" }
+   write(1, asynchronous=[character::])   ! { dg-error "must be scalar" }
+end

C++ PATCH for c++/86608, reading constexpr volatile variable

[Marek Polacek]

A template-argument for a non-type template-parameter shall be a converted
constant expression.  But an lvalue-to-rvalue conversion applied to a volatile
glvalue is not allowed to be part of the evaluation of a constant expression.
So this test should be rejected.

The non_const_var_error tweaks are needed to give the correct diagnostic.

I don't plan to backport this to older branches, unless you think otherwise.

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

2018-12-10  Marek Polacek  

PR c++/86608 - reading constexpr volatile variable.
* constexpr.c (non_const_var_error): Add a boolean parameter.  Use it.
(cxx_eval_constant_expression): Adjust call to non_const_var_error.
(potential_constant_expression_1): Also give error when reading a
constexpr volatile variable.

* g++.dg/cpp0x/constexpr-volatile2.C: New test.
* g++.dg/cpp0x/pr65327.C: Add dg-error.

diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
index 1c844a8c2ef..e78cc4a81d6 100644
--- gcc/cp/constexpr.c
+++ gcc/cp/constexpr.c
@@ -3431,10 +3431,11 @@ cxx_eval_indirect_ref (const constexpr_ctx *ctx, tree t,
 
 /* Complain about R, a VAR_DECL, not being usable in a constant expression.
Shared between potential_constant_expression and
-   cxx_eval_constant_expression.  */
+   cxx_eval_constant_expression.  If SKIP_OWN_INIT_P is true, don't report
+   that R was used in its own initializer, as the problem is elsewhere.  */
 
 static void
-non_const_var_error (tree r)
+non_const_var_error (tree r, bool skip_own_init_p)
 {
   tree type = TREE_TYPE (r);
   error ("the value of %qD is not usable in a constant "
@@ -3442,7 +3443,7 @@ non_const_var_error (tree r)
   /* Avoid error cascade.  */
   if (DECL_INITIAL (r) == error_mark_node)
 return;
-  if (DECL_DECLARED_CONSTEXPR_P (r))
+  if (!skip_own_init_p && DECL_DECLARED_CONSTEXPR_P (r))
 inform (DECL_SOURCE_LOCATION (r),
"%qD used in its own initializer", r);
   else if (INTEGRAL_OR_ENUMERATION_TYPE_P (type))
@@ -4251,7 +4252,7 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, 
tree t,
   if (DECL_P (r))
{
  if (!ctx->quiet)
-   non_const_var_error (r);
+   non_const_var_error (r, /*skip_own_init_p=*/false);
  *non_constant_p = true;
}
   break;
@@ -5692,7 +5693,7 @@ potential_constant_expression_1 (tree t, bool want_rval, 
bool strict, bool now,
   if (want_rval
  && !var_in_maybe_constexpr_fn (t)
  && !type_dependent_expression_p (t)
- && !decl_maybe_constant_var_p (t)
+ && (!decl_maybe_constant_var_p (t) || TREE_THIS_VOLATILE (t))
  && (strict
  || !CP_TYPE_CONST_NON_VOLATILE_P (TREE_TYPE (t))
  || (DECL_INITIAL (t)
@@ -5701,7 +5702,7 @@ potential_constant_expression_1 (tree t, bool want_rval, 
bool strict, bool now,
  && !is_really_empty_class (TREE_TYPE (t)))
 {
   if (flags & tf_error)
-non_const_var_error (t);
+   non_const_var_error (t, TREE_THIS_VOLATILE (t));
   return false;
 }
   return true;
diff --git gcc/testsuite/g++.dg/cpp0x/constexpr-volatile2.C 
gcc/testsuite/g++.dg/cpp0x/constexpr-volatile2.C
new file mode 100644
index 000..2054fb83768
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-volatile2.C
@@ -0,0 +1,13 @@
+// PR c++/86608
+// { dg-do compile { target c++11 } }
+
+template struct X {};
+
+int
+main ()
+{
+  static constexpr volatile int a = 3;
+  constexpr volatile int b = 2;
+  return (sizeof(X) // { dg-error "not usable in a constant 
expression" }
+ + sizeof(X)); // { dg-error "not usable in a constant 
expression" }
+}
diff --git gcc/testsuite/g++.dg/cpp0x/pr65327.C 
gcc/testsuite/g++.dg/cpp0x/pr65327.C
index c6cefaba692..0ba189a24b8 100644
--- gcc/testsuite/g++.dg/cpp0x/pr65327.C
+++ gcc/testsuite/g++.dg/cpp0x/pr65327.C
@@ -15,4 +15,4 @@ constexpr volatile int
 bar ()
 {
   return i;
-}
+} // { dg-error "not usable in a constant expression" }

Re: [PATCH AutoFDO/4]Fix profile count computation/propagation.

[Bin.Cheng]

On Thu, Nov 8, 2018 at 6:33 AM Jeff Law  wrote:
>
> On 10/31/18 12:34 AM, bin.cheng wrote:
> > Hi,
> > This patch fixes AutoFDO breakage on trunk.  The main reason for breakage 
> > is AutoFDO
> > relies on standalone edge count computing and propagating profile 
> > count/probability info
> > on CFG, but in new infra, edge count is actually computed from probability, 
> > which leads
> > to chicken-egg problem and corrupted profile count.  This patch fixes the 
> > issue by using
> > explicit edge count.
> >
> > There is another issue not touched yet that, in quite common case, profiled 
> > samples are
> > not enough and profile info computed for lots of blocks is ZERO.  In the 
> > future, we may
> > add some heuristics checking quality of sampled counts and reverting to 
> > guessed profile
> > count if necessary.  I think change made in this patch is also needed for 
> > that.
> >
> > Package mysql server is used in test of this patch set.  It can't be 
> > compiled with autofdo
> > on trunk, even with compilation issues worked-around, there isn't 
> > performance improvement.
> > I local experiments, with this patch set it's improved by 12.3%, 4.3% 
> > irrespectively for
> > read-only/write-heavy benchmarks.  Unfortunately,  this patch set was 
> > written against
> > GCC 8 branch a while ago, improvement gets worse on trunk and I haven't 
> > investigated
> > the reason yet.  I guess there are still other issues which need to be 
> > fixed in the future.
> >
> > Bootstrap and test on x86_64 in patch set.  Is it OK?
> >
> > Thanks,
> > bin
> > 2018-10-31  Bin Cheng  
> >
> >   * auto-profile.c (AFDO_EINFO): New macro.
> >   (struct edge_info): New structure.
> >   (is_edge_annotated, set_edge_annotated): Delete.
> >   (afdo_propagate_edge, afdo_propagate_circuit, afdo_propagate): Remove
> >   parameter.  Adjust edge count computation and annotation using struct
> >   edge_info.
> >   (afdo_calculate_branch_prob): Ditto.
> >   (afdo_annotate_cfg): Simplify code setting basic block profile count.
> >
> >
> > 0004-Fix-AutoFDO-breakage-after-profile-count-rewriting.patch
> >
> > From 6506c12d1b633b6d1bfae839b3633a4f99b3a481 Mon Sep 17 00:00:00 2001
> > From: chengbin 
> > Date: Mon, 20 Aug 2018 15:25:02 +0800
> > Subject: [PATCH 4/4] Fix AutoFDO breakage after profile count rewriting.
> >
> > ---
> >  gcc/auto-profile.c | 190 
> > ++---
> >  1 file changed, 95 insertions(+), 95 deletions(-)
> >
> > diff --git a/gcc/auto-profile.c b/gcc/auto-profile.c
> > index cde4f41c1d9..ff3ea23d830 100644
> > --- a/gcc/auto-profile.c
> > +++ b/gcc/auto-profile.c
> > @@ -101,6 +101,17 @@ along with GCC; see the file COPYING3.  If not see
> >  namespace autofdo
> >  {
> >
> > +/* Intermediate edge info used when propagating AutoFDO profile 
> > information.
> > +   We can't edge->count() directly since it's computed from edge's 
> > probability
> > +   while probability is yet not decided during propagation.  */
> > +#define AFDO_EINFO(e) ((struct edge_info *) e->aux)
> > +struct edge_info
> > +{
> > +  edge_info () : count (profile_count::zero ().afdo ()), annotated_p 
> > (false) {}
> > +  profile_count count;
> > +  bool annotated_p;
> > +};
> edge_info isn't POD, so make it a class rather than a struct.
>
> OK with that change assuming it does not have a hard dependency on prior
> patches in this series.
Thanks very much for review.  Now that all prerequisite patches are
approved and committed, I update this one by making edge_info a class
as suggested.
Bootstrap and test as before, is it OK?

Thanks,
bin

2018-12-11  Bin Cheng  

* auto-profile.c (AFDO_EINFO): New macro.
(class edge_info): New class.
(is_edge_annotated, set_edge_annotated): Delete.
(afdo_propagate_edge, afdo_propagate_circuit, afdo_propagate): Remove
parameter.  Adjust edge count computation and annotation using class
edge_info.
(afdo_calculate_branch_prob, afdo_annotate_cfg): Likewise.


0003-Fix-AutoFDO-breakage-after-profile-count-rewriting.patch
Description: Binary data

[committed] avoid -Wrestrict on structs copies with unknown offset and constant size (PR 86196)

[Martin Sebor]

A fix for pr85753 I committed back in may introduced a subtle
logic error that cause false positives for copies involving
unknown offsets into non-array objects.  The attached patch
corrects this mistake.  I have committed it as obvious in
r266967 after verifying it on x86_64-linux.  The committed patch
is attached.

Martin
PR tree-optimization/86196 - Bogus -Wrestrict on memcpy between array elements at unequal indices

gcc/ChangeLog:

	PR tree-optimization/86196
	* gimple-ssa-warn-restrict.c (builtin_memref::builtin_memref): Use
	base size only of arrays.

gcc/testsuite/ChangeLog:

	PR tree-optimization/86196
	* gcc.dg/Wrestrict-18.c: New test.

Index: gcc/gimple-ssa-warn-restrict.c
===
--- gcc/gimple-ssa-warn-restrict.c	(revision 266963)
+++ gcc/gimple-ssa-warn-restrict.c	(working copy)
@@ -272,15 +272,16 @@ builtin_memref::builtin_memref (tree expr, tree si
 
   offset_int maxoff = maxobjsize;
   tree basetype = TREE_TYPE (base);
-  if (TREE_CODE (basetype) == ARRAY_TYPE
-  && ref
-  && array_at_struct_end_p (ref))
-;   /* Use the maximum possible offset for last member arrays.  */
-  else if (tree basesize = TYPE_SIZE_UNIT (basetype))
-if (TREE_CODE (basesize) == INTEGER_CST)
-  /* Size could be non-constant for a variable-length type such
-	 as a struct with a VLA member (a GCC extension).  */
-  maxoff = wi::to_offset (basesize);
+  if (TREE_CODE (basetype) == ARRAY_TYPE)
+{
+  if (ref && array_at_struct_end_p (ref))
+	;   /* Use the maximum possible offset for last member arrays.  */
+  else if (tree basesize = TYPE_SIZE_UNIT (basetype))
+	if (TREE_CODE (basesize) == INTEGER_CST)
+	  /* Size could be non-constant for a variable-length type such
+	 as a struct with a VLA member (a GCC extension).  */
+	  maxoff = wi::to_offset (basesize);
+}
 
   if (offrange[0] >= 0)
 {
Index: gcc/testsuite/gcc.dg/Wrestrict-18.c
===
--- gcc/testsuite/gcc.dg/Wrestrict-18.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/Wrestrict-18.c	(working copy)
@@ -0,0 +1,37 @@
+/* PR tree-optimization/86196 - Bogus -Wrestrict on memcpy between array
+   elements at unequal indices
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+typedef __SIZE_TYPE__ size_t;
+
+extern void* memcpy (void*, const void*, size_t);
+
+struct S
+{
+  int n;
+  void * p;
+};
+
+/* Test case submitted in the PR.  */
+
+void pr86196_c0 (struct S * a, size_t n)
+{
+  for (size_t i = 0, j = 0; i != n; ++i)
+{
+  if (a[i].n == 0)
+	{
+	  if (i != j)
+	memcpy ([j], [i], sizeof (struct S));   /* { dg-bogus "\\\[-Wrestrict" } */
+	  ++j;
+	}
+}
+}
+
+/* Reduced test case.  */
+
+void pr86196_c1 (struct S *a, int i, int j)
+{
+  if (i != j)
+memcpy ([j], [i], sizeof (struct S));   /* { dg-bogus "\\\[-Wrestrict" } */
+}

Re: [PATCH] Set DEMANGLE_RECURSION_LIMIT to 1536

[Jeff Law]

On 12/10/18 8:34 AM, Jason Merrill wrote:
> On Mon, Dec 10, 2018 at 10:10 AM Jakub Jelinek  wrote:
>> On Mon, Dec 10, 2018 at 02:52:39PM +, Michael Matz wrote:
>>> On Fri, 7 Dec 2018, H.J. Lu wrote:
>>>
>> On Thu, Dec 6, 2018 at 3:12 AM Nick Clifton  wrote:
>>>
>>>   Is the patch OK with you ?
>>
>
> This caused:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88409
>

 Here is the fix.   OK for trunk?
>>>
>>> I think this points toward the limit being _much_ too low.  With template
>>> meta programming you easily get these mangled names, it's not even a
>>> particularly long one.  But I'm wondering a bit, without tracing the
>>> demangler, just looking at the symbol name and demangled result I don't
>>> readily see where the depth of recursion really is more than 1024, are
>>> there perhaps some recursion_level-- statements skipped?
>>
>> That is because the recursion_level limit isn't hit in this case at all (far
>> from it).
>>
>> What breaks it is this:
>>
>>   /* PR 87675 - Check for a mangled string that is so long
>>  that we do not have enough stack space to demangle it.  */
>>   if (((options & DMGL_NO_RECURSE_LIMIT) == 0)
>>   /* This check is a bit arbitrary, since what we really want to do is to
>>  compare the sizes of the di.comps and di.subs arrays against the
>>  amount of stack space remaining.  But there is no portable way to do
>>  this, so instead we use the recursion limit as a guide to the 
>> maximum
>>  size of the arrays.  */
>>   && (unsigned long) di.num_comps > DEMANGLE_RECURSION_LIMIT)
>> {
>>   /* FIXME: We need a way to indicate that a stack limit has been 
>> reached.  */
>>   return 0;
>> }
> 
>> where di.num_comps is just strlen (mangled) * 2.  Without any analysis
>> whatsoever, bumping the "recursion" limit will just mean we can process 1.5
>> times long names.  Either we need more precise analysis on what we are
>> looking for (how big arrays we'll need) or it needs to be an independent
>> limit and certainly should allow say 10KB symbols too if they are
>> reasonable.
> 
> If the problem is alloca, we could avoid using alloca if the size
> passes a threshold.  Perhaps even use a better data structure than a
> preallocated array based on a guess about the number of components...
Actually I would strongly suggest avoiding alloca completely.  This
isn't particularly performance sensitive code and alloca can be abused
in all kinds of interesting ways.

jeff

Re: [PATCH] Set DEMANGLE_RECURSION_LIMIT to 1536

[Jason Merrill]

On Mon, Dec 10, 2018 at 1:55 PM Jakub Jelinek  wrote:
>
> On Mon, Dec 10, 2018 at 10:20:24AM -0800, Ian Lance Taylor wrote:
> > > I think it is a bad idea to use the same macro and value for both the
> > > recursion limit and essentially stack limit.  For the latter, it should
> > > actually compute expected stack size
> > > (i.e. di.num_comps * sizeof (*di.comps) + di.num_subs * sizeof (*di.subs))
> > > and rather than just giving up should say that memory up to 64KB this
> > > way will be handled via alloca, more through say mmap (I'd avoid malloc
> > > because some users are using these APIs in cases where malloc isn't 
> > > usable).
> > > And have only much larger limit, like say 1MB for these arrays on which to
> > > give up.
> >
> > That makes sense.
> >
> > We've wanted to avoid malloc in this code because some programs call
> > the demangler from a signal handler.  But using mmap should be fine in
> > practice.
>
> mmap is not available everywhere, but we could just have a smaller limit
> on how big mangled names we handle on targets where mmap isn't available or
> if it fails.
>
> And, the other option is just try to parse the string without doing all the
> processing first and compute (perhaps upper bound) on how many components
> and substitutions we need.  Many components have longish names, or numbers,
> etc. so the 2 * strlen is really very upper bound and strlen for number of
> substitutions too.

Or, in that situation, we could put an upper bound on the number of
components and substitutions, and fail if we end up needing more than
that.

Jason

[PATCH] accept all C integer types in function parameters referenced by alloc_align (PR 88363)

[Martin Sebor]

Some of my testing exposed a minor problem in GCC 9's validation
of the type of function parameters referred to by attribute
positional arguments.  Whereas GCC 8 accepts all C integer types,
including enumerated types, such as:

  enum AllocAlign { Align16 = 16, Align32 = 32 };

  __attribute__ ((alloc_align (1))) void*
  alloc (size_t, enum AllocAlign)

GCC 9 only accepts signed and unsigned integer types.  This change
(introduced by myself) was unintentional, and a fix for it is in
the attached trivial patch.  I plan to commit it without approval
in the next day or so unless any concerns or suggestions come up.

Tested on x86_64-linux.

Martin
PR c/88363 - alloc_align attribute doesn't accept enumerated arguments

gcc/c-family/ChangeLog:

	PR c/88363
	* c-attribs.c (positional_argument): Also accept enumerated types.

gcc/testsuite/ChangeLog:

	PR c/88363
	* c-c++-common/attributes-4.c: New test.

gcc/ChangeLog:

	PR c/88363
	* doc/extend.texi (attribute alloc_align, alloc_size): Update.

Index: gcc/c-family/c-attribs.c
===
--- gcc/c-family/c-attribs.c	(revision 266963)
+++ gcc/c-family/c-attribs.c	(working copy)
@@ -630,11 +630,11 @@ positional_argument (const_tree fntype, const_tree
 	  return NULL_TREE;
 	}
 
-  /* Where the expected code is STRING_CST accept any pointer
-	 to a narrow character type, qualified or otherwise.  */
   bool type_match;
   if (code == STRING_CST && POINTER_TYPE_P (argtype))
 	{
+	  /* Where the expected code is STRING_CST accept any pointer
+	 to a narrow character type, qualified or otherwise.  */
 	  tree type = TREE_TYPE (argtype);
 	  type = TYPE_MAIN_VARIANT (type);
 	  type_match = (type == char_type_node
@@ -641,6 +641,10 @@ positional_argument (const_tree fntype, const_tree
 			|| type == signed_char_type_node
 			|| type == unsigned_char_type_node);
 	}
+  else if (code == INTEGER_TYPE)
+	/* For integers, accept bool, enums, and other types that
+	   match INTEGRAL_TYPE_P.  */
+	type_match = INTEGRAL_TYPE_P (argtype);
   else
 	type_match = TREE_CODE (argtype) == code;
 
Index: gcc/doc/extend.texi
===
--- gcc/doc/extend.texi	(revision 266963)
+++ gcc/doc/extend.texi	(working copy)
@@ -2471,7 +2471,8 @@ The @code{aligned} attribute can also be used for
 @item alloc_align (@var{position})
 @cindex @code{alloc_align} function attribute
 The @code{alloc_align} attribute may be applied to a function that
-returns a pointer and takes at least one argument of an integer type.
+returns a pointer and takes at least one argument of an integer or
+enumerated type.
 It indicates that the returned pointer is aligned on a boundary given
 by the function argument at @var{position}.  Meaningful alignments are
 powers of 2 greater than one.  GCC uses this information to improve
@@ -2495,7 +2496,8 @@ given by parameter 1.
 @itemx alloc_size (@var{position-1}, @var{position-2})
 @cindex @code{alloc_size} function attribute
 The @code{alloc_size} attribute may be applied to a function that
-returns a pointer and takes at least one argument of an integer type.
+returns a pointer and takes at least one argument of an integer or
+enumerated type.
 It indicates that the returned pointer points to memory whose size is
 given by the function argument at @var{position-1}, or by the product
 of the arguments at @var{position-1} and @var{position-2}.  Meaningful
Index: gcc/testsuite/c-c++-common/attributes-4.c
===
--- gcc/testsuite/c-c++-common/attributes-4.c	(nonexistent)
+++ gcc/testsuite/c-c++-common/attributes-4.c	(working copy)
@@ -0,0 +1,46 @@
+/* PR c/88363 - alloc_align attribute doesn't accept enumerated arguments
+   Verify that attribute positional arguments can refer to all C integer
+   types in both C and C++.
+   { dg-do compile }
+   { dg-options "-Wall" }
+   { dg-options "-Wall -Wno-c++-compat" { target c } } */
+
+#define ATTR(...) __attribute__ ((__VA_ARGS__))
+
+#if __cplusplus == 199711L
+typedef __CHAR16_TYPE__ char16_t;
+typedef __CHAR32_TYPE__ char32_t;
+#elif !__cplusplus
+typedef _Bool   bool;
+typedef __CHAR16_TYPE__ char16_t;
+typedef __CHAR32_TYPE__ char32_t;
+typedef __WCHAR_TYPE__  wchar_t;
+#endif
+
+enum A { A0 };
+
+/* Using bool here is of marginal utility but it's accepted nonetheless.   */
+ATTR (alloc_align (1)) void* falloc_align_bool (bool);
+ATTR (alloc_align (1)) void* falloc_align_char (char);
+ATTR (alloc_align (1)) void* falloc_align_char16 (char16_t);
+ATTR (alloc_align (1)) void* falloc_align_char32 (char32_t);
+ATTR (alloc_align (1)) void* falloc_align_wchar (wchar_t);
+/* Using an enum might make sense in an API that limits the alignments
+   it accepts to just the set of the defined enumerators.   */
+ATTR (alloc_align (1)) void* falloc_align_enum (enum A);
+ATTR (alloc_align (1)) void* falloc_align_int128 (__int128_t);

Re: [committed] hppa: Add target support infrastructure for D front end

[John David Anglin]

On 2018-12-09 5:19 p.m., Iain Buclaw wrote:
> Thanks!  Shouldn't this be made available to all hppa* configurations
> though?  Or just conservatively adding it for now.
I was just being conservative.  It took a couple of days to add the
linux details for libphobos and I'm
more familiar with it.

Dave

-- 
John David Anglin  dave.ang...@bell.net

Re: PR88346, Inconsistent list of CPUs supported by the rs6000 backend after r266502

[Segher Boessenkool]

Hi Alan,

Let's ask David?  (Cc:ed).  Strange that no one noticed powerpc64 before;
titan and rs64 aren't so strange though ;-)


Segher


On Fri, Dec 07, 2018 at 09:00:39PM +1030, Alan Modra wrote:
> This patch removes the %e error for AIX, since it seems there has been
> no attempt to keep ASM_CPU_SPEC cpu support up to date for AIX, and
> adds missing entries to ASM_CPU_SPEC in rs6000.h.  Removing the %e
> isn't ideal, but leaving it in and hitting a compiler error for -mcpu
> cases where the AIX assembler works fine with default options, is
> worse.
> 
> The rs64a->rs64 name change happened a long time ago as a fix for
> PR20813 (git commit c92b4c3f5b).
> 
> Bootstrapped and regression tested powerpc64le-linux.  OK?
> 
>   PR 88346
>   * config/rs6000/rs6000.h (ASM_CPU_SPEC): Correct %e message.  Handle
>   -mcpu=rs64, not -mcpu=rs64a.  Handle -mcpu=powerpc64 and -mcpu=titan.
>   * config/rs6000/driver-rs6000.c (asm_names): Similarly.
>   * config/rs6000/aix71.h (ASM_CPU_SPEC): Delete %e message.  Handle
>   -mcpu=rs64, not -mcpu=rs64a.
>   * config/rs6000/aix72.h (ASM_CPU_SPEC): Likewise.
> 
> diff --git a/gcc/config/rs6000/aix71.h b/gcc/config/rs6000/aix71.h
> index 2398ed64baa..d2fbba4f509 100644
> --- a/gcc/config/rs6000/aix71.h
> +++ b/gcc/config/rs6000/aix71.h
> @@ -77,7 +77,7 @@ do {
> \
>mcpu=power4: -mpwr4; \
>mcpu=power3: -m620; \
>mcpu=powerpc: -mppc; \
> -  mcpu=rs64a: -mppc; \
> +  mcpu=rs64: -mppc; \
>mcpu=603: -m603; \
>mcpu=603e: -m603; \
>mcpu=604: -m604; \
> @@ -88,8 +88,7 @@ do {
> \
>!mcpu*: %{mvsx: -mpwr6; \
>   maltivec: -m970; \
>   maix64|mpowerpc64: -mppc64; \
> - : %(asm_default)}; \
> -  :%eMissing -mcpu option in ASM_SPEC_CPU?\n} \
> + : %(asm_default)}} \
>  -many"
>  
>  #undef   ASM_DEFAULT_SPEC
> diff --git a/gcc/config/rs6000/aix72.h b/gcc/config/rs6000/aix72.h
> index cfb0258acce..211010748c6 100644
> --- a/gcc/config/rs6000/aix72.h
> +++ b/gcc/config/rs6000/aix72.h
> @@ -77,7 +77,7 @@ do {
> \
>mcpu=power4: -mpwr4; \
>mcpu=power3: -m620; \
>mcpu=powerpc: -mppc; \
> -  mcpu=rs64a: -mppc; \
> +  mcpu=rs64: -mppc; \
>mcpu=603: -m603; \
>mcpu=603e: -m603; \
>mcpu=604: -m604; \
> @@ -88,8 +88,7 @@ do {
> \
>!mcpu*: %{mvsx: -mpwr6; \
>   maltivec: -m970; \
>   maix64|mpowerpc64: -mppc64; \
> - : %(asm_default)}; \
> -  :%eMissing -mcpu option in ASM_SPEC_CPU?\n} \
> + : %(asm_default)}} \
>  -many"
>  
>  #undef   ASM_DEFAULT_SPEC
> diff --git a/gcc/config/rs6000/driver-rs6000.c 
> b/gcc/config/rs6000/driver-rs6000.c
> index 0a48d46d658..51c6b79e741 100644
> --- a/gcc/config/rs6000/driver-rs6000.c
> +++ b/gcc/config/rs6000/driver-rs6000.c
> @@ -449,7 +449,7 @@ static const struct asm_name asm_names[] = {
>{ "power8","-mpwr8" },
>{ "power9","-mpwr9" },
>{ "powerpc",   "-mppc" },
> -  { "rs64a", "-mppc" },
> +  { "rs64",  "-mppc" },
>{ "603",   "-m603" },
>{ "603e",  "-m603" },
>{ "604",   "-m604" },
> @@ -477,8 +477,9 @@ static const struct asm_name asm_names[] = {
>{ "power9","-mpower9" },
>{ "a2","-ma2" },
>{ "powerpc",   "-mppc" },
> +  { "powerpc64", "-mppc64" },
>{ "powerpc64le", "%{mpower9-vector:-mpower9;:-mpower8}" },
> -  { "rs64a", "-mppc64" },
> +  { "rs64",  "-mppc64" },
>{ "401",   "-mppc" },
>{ "403",   "-m403" },
>{ "405",   "-m405" },
> @@ -519,6 +520,7 @@ static const struct asm_name asm_names[] = {
>{ "e500mc64",  "-me500mc64" },
>{ "e5500", "-me5500" },
>{ "e6500", "-me6500" },
> +  { "titan", "-mtitan" },
>{ NULL,"\
>  %{mpower9-vector: -mpower9; \
>mpower8-vector|mcrypto|mdirect-move|mhtm: -mpower8; \
> diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
> index 2a62679bdb8..e7e998d1492 100644
> --- a/gcc/config/rs6000/rs6000.h
> +++ b/gcc/config/rs6000/rs6000.h
> @@ -87,9 +87,10 @@
>mcpu=power4: -mpower4; \
>mcpu=power3: -mppc64; \
>mcpu=powerpc: -mppc; \
> +  mcpu=powerpc64: -mppc64; \
>mcpu=a2: -ma2; \
>mcpu=cell: -mcell; \
> -  mcpu=rs64a: -mppc64; \
> +  mcpu=rs64: -mppc64; \
>mcpu=401: -mppc; \
>mcpu=403: -m403; \
>mcpu=405: -m405; \
> @@ -130,11 +131,12 @@
>mcpu=e500mc64: -me500mc64; \
>mcpu=e5500: -me5500; \
>mcpu=e6500: -me6500; \
> +  mcpu=titan: -mtitan; \
>!mcpu*: %{mpower9-vector: -mpower9; \
>   mpower8-vector|mcrypto|mdirect-move|mhtm: -mpower8; \
>   mvsx: -mpower7; \
>   mpowerpc64: -mppc64;: %(asm_default)}; \
> -  :%eMissing -mcpu option in ASM_SPEC_CPU?\n} \
> +  :%eMissing -mcpu option in 

libbacktrace integration for _GLIBCXX_DEBUG mode

[François Dumont]

Hi

    Here is the integration of libbacktrace to provide the backtrace on 
_GLIBCXX_DEBUG assertions.


    I decided to integrate it without impacting the build scripts. 
Users just need to install libbacktrace and once done _GLIBCXX_DEBUG 
will look for it and start using it if supported. The drawback is that 
as soon as libbacktrace is installed users will have to add -lbacktrace 
in order to use _GLIBCXX_DEBUG mode. But I expect that if you install 
libbacktrace it is for a reason.


    Note that when libbacktrace is not supported I include stdint.h to 
get uintptr_t, I hope it is the correct way to get it in a portable way.


    I also explcitely define BACKTRACE_SUPPORTED to 0 to make sure 
libstdc++ has no libbacktrace dependency after usual build.


    As it starts to make a lot of information displayed on Debug 
assertion I have created print_function to filter output of functions. 
It removes things like __cxx1998::, std::allocator and greatly 
simplified _Safe_iterator rendering.


    Here is an example of output when building 
23_containers/vector/debug/construct3_neg.cc:


/home/fdt/dev/gcc/install/include/c++/9.0.0/debug/safe_iterator.h:321:
In function:
    __gnu_debug::_Safe_iterator<_Iterator, _Sequence, _Category>&
    __gnu_debug::_Safe_iterator<_Iterator, _Sequence,
    _Category>::operator++() [with _Iterator = std::_List_iterator;
    _Sequence = std::__debug::list; _Category =
    std::forward_iterator_tag]

Backtrace:
    0x40275f 
__gnu_debug::_Safe_iterator>::operator++()

/home/fdt/dev/gcc/install/include/c++/9.0.0/debug/safe_iterator.h:321
    0x402181 
__gnu_debug::_Safe_iterator>::operator++()

/home/fdt/dev/gcc/install/include/c++/9.0.0/debug/safe_iterator.h:570
    0x404082 
std::iterator_traits<__gnu_debug::_Safe_iterator> 
>::difference_type 
std::__distance<__gnu_debug::_Safe_iterator> 
>(__gnu_debug::_Safe_iterator>, 
__gnu_debug::_Safe_iterator>, 
std::input_iterator_tag)

/home/fdt/dev/gcc/install/include/c++/9.0.0/bits/stl_iterator_base_funcs.h:89
    0x403795 
std::iterator_traits<__gnu_debug::_Safe_iterator> 
>::difference_type 
std::distance<__gnu_debug::_Safe_iterator> 
>(__gnu_debug::_Safe_iterator>, 
__gnu_debug::_Safe_iterator>)

/home/fdt/dev/gcc/install/include/c++/9.0.0/bits/stl_iterator_base_funcs.h:141
    0x4030b9 void 
std::vector::_M_range_initialize<__gnu_debug::_Safe_iterator> 
>(__gnu_debug::_Safe_iterator>, 
__gnu_debug::_Safe_iterator>, 
std::forward_iterator_tag)

/home/fdt/dev/gcc/install/include/c++/9.0.0/bits/stl_vector.h:1541
    0x402a2d 
std::vector::vector<__gnu_debug::_Safe_iterator>, 
void>(__gnu_debug::_Safe_iterator>, 
__gnu_debug::_Safe_iterator>)

/home/fdt/dev/gcc/install/include/c++/9.0.0/bits/stl_vector.h:618
    0x4022ec 
std::__debug::vector::vector<__gnu_debug::_Safe_iterator>, 
void>(__gnu_debug::_Safe_iterator>, 
__gnu_debug::_Safe_iterator>)

    /home/fdt/dev/gcc/install/include/c++/9.0.0/debug/vector:195
    0x401e2c void 
__gnu_test::check_construct3 >()

    ./util/debug/checks.h:234
    0x401460 test01()
    /home/fdt/dev/poc/construct3_neg.cc:26
    0x40146c main
    /home/fdt/dev/poc/construct3_neg.cc:31

Error: attempt to increment a past-the-end iterator.

Objects involved in the operation:
    iterator "this" @ 0x0x7fff068adce0 {
  type = std::_List_iterator (mutable iterator);
  state = past-the-end;
  references sequence with type 'std::__debug::list' @ 
0x0x7fff068ae080

    }

    * include/debug/formatter.h: Check for backtrace-supported.h access
    and include it.
    [BACKTRACE_SUPPORTED] Include 
    (_Error_formatter::_Bt_full_t): New function pointer type.
    (_Error_formatter::_M_backtrace_state): New.
    (_Error_formatter::_M_backtrace_full_func): New.
    * src/c++11/debug.cc: Include .
    (PrintContext::_M_demangle_name): New.
    (_Print_func_t): New.
    (print_word(PrintContext&, const char*)): New.
    (print_raw(PrintContext&, const char*)): New.
    (print_function(PrintContext&, const char*, _Print_func_t)): New.
    (print_type): Use latter.
    (print_string(PrintContext&, const char*)): New.
    (print_backtrace(void*, uintptr_t, const char*, int, const char*)):
    New.
    (_Error_formatter::_M_error()): Adapt.

Tested under Linux x86_64.

Ok to commit ? One day ?

François


diff --git a/libstdc++-v3/include/debug/formatter.h b/libstdc++-v3/include/debug/formatter.h
index 1f03f251488..8fb5e57a4d4 100644
--- a/libstdc++-v3/include/debug/formatter.h
+++ b/libstdc++-v3/include/debug/formatter.h
@@ -31,6 +31,25 @@
 
 #include 
 
+#if !defined(BACKTRACE_SUPPORTED) \
+  && defined(__has_include) && __has_include()
+# include 
+#endif
+
+#if BACKTRACE_SUPPORTED
+# include 
+#else
+# include  // For uintptr_t.
+
+// Extracted from libbacktrace.
+typedef void (*backtrace_error_callback) (void*, const char*, int);
+
+typedef int (*backtrace_full_callback) (void*, uintptr_t, const char*, int,
+	const char*);
+
+struct backtrace_state;
+#endif
+
 #if __cpp_rtti
 # include 
 

[PATCH 3/4] c/c++, asm: Use nicer error for const and restrict

[Segher Boessenkool]

Not all qualifiers are asm qualifiers.  We can talk about that in a
nicer way than just giving a generic parser error.

This also adds two testcases for C++, that previously were for C only.


2018-12-10  Segher Boessenkool  

c/
* c-parser.c (c_parser_asm_statement) : Give
a more specific error message (instead of just falling through).

cp/
* parser.c (cp_parser_asm_definition) : Give
a more specific error message (instead of just falling through).

testsuite/
* g++.dg/asm-qual-1.C: New testcase.
* g++.dg/asm-qual-2.C: New testcase.
* gcc.dg/asm-qual-1.c: Update.

---
 gcc/c/c-parser.c  |  6 +
 gcc/cp/parser.c   |  6 +
 gcc/testsuite/g++.dg/asm-qual-1.C | 13 +++
 gcc/testsuite/g++.dg/asm-qual-2.C | 46 +++
 gcc/testsuite/gcc.dg/asm-qual-1.c |  6 ++---
 5 files changed, 73 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/asm-qual-1.C
 create mode 100644 gcc/testsuite/g++.dg/asm-qual-2.C

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 652e53c..0def497 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -6411,6 +6411,12 @@ c_parser_asm_statement (c_parser *parser)
  c_parser_consume_token (parser);
  continue;
 
+   case RID_CONST:
+   case RID_RESTRICT:
+ error_at (loc, "%qE is not an asm qualifier", token->value);
+ c_parser_consume_token (parser);
+ continue;
+
default:
  break;
}
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 06a6bb0..4072afe 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -19693,6 +19693,12 @@ cp_parser_asm_definition (cp_parser* parser)
cp_lexer_consume_token (parser->lexer);
continue;
 
+ case RID_CONST:
+ case RID_RESTRICT:
+   error_at (loc, "%qT is not an asm qualifier", token->u.value);
+   cp_lexer_consume_token (parser->lexer);
+   continue;
+
  default:
break;
  }
diff --git a/gcc/testsuite/g++.dg/asm-qual-1.C 
b/gcc/testsuite/g++.dg/asm-qual-1.C
new file mode 100644
index 000..3fba592
--- /dev/null
+++ b/gcc/testsuite/g++.dg/asm-qual-1.C
@@ -0,0 +1,13 @@
+// Test that qualifiers other than volatile are disallowed on asm.
+// { dg-do compile }
+// { dg-options "-std=gnu++98" }
+
+void
+f ()
+{
+  asm volatile ("");
+
+  asm const (""); // { dg-error {'const' is not an asm qualifier} }
+
+  asm __restrict (""); // { dg-error {'__restrict' is not an asm qualifier} }
+}
diff --git a/gcc/testsuite/g++.dg/asm-qual-2.C 
b/gcc/testsuite/g++.dg/asm-qual-2.C
new file mode 100644
index 000..52968bd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/asm-qual-2.C
@@ -0,0 +1,46 @@
+// Test that qualifiers on asm are allowed in any order.
+// { dg-do compile }
+// { dg-options "-std=c++98" }
+
+void
+f ()
+{
+  asm volatile goto (""  lab);
+  asm volatile inline ("" :::);
+  asm inline volatile ("" :::);
+  asm inline goto (""  lab);
+  asm goto volatile (""  lab);
+  asm goto inline (""  lab);
+
+  asm volatile inline goto (""  lab);
+  asm volatile goto inline (""  lab);
+  asm inline volatile goto (""  lab);
+  asm inline goto volatile (""  lab);
+  asm goto volatile inline (""  lab);
+  asm goto inline volatile (""  lab);
+
+  /* Duplicates are not allowed.  */
+  asm goto volatile volatile (""  lab);  /* { dg-error "" } */
+  asm volatile goto volatile (""  lab);  /* { dg-error "" } */
+  asm volatile volatile goto (""  lab);  /* { dg-error "" } */
+  asm goto goto volatile (""  lab);  /* { dg-error "" } */
+  asm goto volatile goto (""  lab);  /* { dg-error "" } */
+  asm volatile goto goto (""  lab);  /* { dg-error "" } */
+
+  asm inline volatile volatile ("" :::);  /* { dg-error "" } */
+  asm volatile inline volatile ("" :::);  /* { dg-error "" } */
+  asm volatile volatile inline ("" :::);  /* { dg-error "" } */
+  asm inline inline volatile ("" :::);  /* { dg-error "" } */
+  asm inline volatile inline ("" :::);  /* { dg-error "" } */
+  asm volatile inline inline ("" :::);  /* { dg-error "" } */
+
+  asm goto inline inline (""  lab);  /* { dg-error "" } */
+  asm inline goto inline (""  lab);  /* { dg-error "" } */
+  asm inline inline goto (""  lab);  /* { dg-error "" } */
+  asm goto goto inline (""  lab);  /* { dg-error "" } */
+  asm goto inline goto (""  lab);  /* { dg-error "" } */
+  asm inline goto goto (""  lab);  /* { dg-error "" } */
+
+lab:
+  ;
+}
diff --git a/gcc/testsuite/gcc.dg/asm-qual-1.c 
b/gcc/testsuite/gcc.dg/asm-qual-1.c
index cb37283..eff6b45 100644
--- a/gcc/testsuite/gcc.dg/asm-qual-1.c
+++ b/gcc/testsuite/gcc.dg/asm-qual-1.c
@@ -8,9 +8,7 @@ f (void)
 {
   asm volatile ("");
 
-  asm const (""); /* { dg-error {expected '\(' before 'const'} } */
-  /* { dg-error {expected identifier} {} {target *-*-*} .-1 } */
+  asm const 

[PATCH 4/4] c++, asm: Do not handle any asm-qualifiers in top-level asm

[Segher Boessenkool]

Previously, "volatile" was allowed.  Changing this simplifies the code,
makes things more regular, and makes the C and C++ frontends handle
this the same way.


2018-12-10  Segher Boessenkool  

cp/
* parser.c (cp_parser_asm_definition): Do not allow any asm qualifiers
on top-level asm.

testsuite/
* g++.dg/asm-qual-3.C: New testcase.
* gcc.dg/asm-qual-3.c: New testcase.

---
 gcc/cp/parser.c   |  7 ++-
 gcc/testsuite/g++.dg/asm-qual-3.C | 12 
 gcc/testsuite/gcc.dg/asm-qual-3.c |  9 +
 3 files changed, 23 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/asm-qual-3.C
 create mode 100644 gcc/testsuite/gcc.dg/asm-qual-3.c

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 4072afe..62c2a4f 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -19649,7 +19649,8 @@ cp_parser_asm_definition (cp_parser* parser)
   location_t volatile_loc = UNKNOWN_LOCATION;
   location_t inline_loc = UNKNOWN_LOCATION;
   location_t goto_loc = UNKNOWN_LOCATION;
-  if (cp_parser_allow_gnu_extensions_p (parser))
+
+  if (cp_parser_allow_gnu_extensions_p (parser) && parser->in_function_body)
 for (;;)
   {
cp_token *token = cp_lexer_peek_token (parser->lexer);
@@ -19668,8 +19669,6 @@ cp_parser_asm_definition (cp_parser* parser)
continue;
 
  case RID_INLINE:
-   if (!parser->in_function_body)
- break;
if (inline_loc)
  {
error_at (loc, "duplicate asm qualifier %qT", token->u.value);
@@ -19681,8 +19680,6 @@ cp_parser_asm_definition (cp_parser* parser)
continue;
 
  case RID_GOTO:
-   if (!parser->in_function_body)
- break;
if (goto_loc)
  {
error_at (loc, "duplicate asm qualifier %qT", token->u.value);
diff --git a/gcc/testsuite/g++.dg/asm-qual-3.C 
b/gcc/testsuite/g++.dg/asm-qual-3.C
new file mode 100644
index 000..95c9b57
--- /dev/null
+++ b/gcc/testsuite/g++.dg/asm-qual-3.C
@@ -0,0 +1,12 @@
+// Test that asm-qualifiers are not allowed on toplevel asm.
+// { dg-do compile }
+// { dg-options "-std=gnu++98" }
+
+asm const ("");// { dg-error {expected '\(' before 'const'} }
+asm volatile (""); // { dg-error {expected '\(' before 'volatile'} }
+asm restrict (""); // { dg-error {expected '\(' before 'restrict'} }
+asm inline ("");   // { dg-error {expected '\(' before 'inline'} }
+asm goto (""); // { dg-error {expected '\(' before 'goto'} }
+
+// There are many other things wrong with this code, so:
+// { dg-excess-errors "" }
diff --git a/gcc/testsuite/gcc.dg/asm-qual-3.c 
b/gcc/testsuite/gcc.dg/asm-qual-3.c
new file mode 100644
index 000..f85d8bf
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asm-qual-3.c
@@ -0,0 +1,9 @@
+/* Test that asm-qualifiers are not allowed on toplevel asm.  */
+/* { dg-do compile } */
+/* { dg-options "-std=gnu99" } */
+
+asm const ("");/* { dg-error {expected '\(' before 'const'} } */
+asm volatile (""); /* { dg-error {expected '\(' before 'volatile'} } */
+asm restrict (""); /* { dg-error {expected '\(' before 'restrict'} } */
+asm inline ("");   /* { dg-error {expected '\(' before 'inline'} } */
+asm goto (""); /* { dg-error {expected '\(' before 'goto'} } */
-- 
1.8.3.1

[PATCH 2/4] c/c++, asm: Use nicer error for duplicate asm qualifiers

[Segher Boessenkool]

Also as suggested by Jason.


Segher


2018-12-10  Segher Boessenkool  

c/
* c-parser.c (c_parser_asm_statement): Keep track of the location each
asm qualifier is first seen; use that to give nicer "duplicate asm
qualifier" messages.  Delete 'quals" variable, instead pass the
"is_volatile_ flag to build_asm_stmt directly.
* c-tree.h (build_asm_stmt): Make the first arg bool instead of tree.
* c-typeck.c (build_asm_stmt): Ditto; adjust.

cp/
* parser.c (cp_parser_asm_definition): Rewrite the loop to work without
"done" boolean variable.
* parser.c (cp_parser_asm_definition): Keep track of the location each
asm qualifier is first seen; use that to give nicer "duplicate asm
qualifier" messages.

---
 gcc/c/c-parser.c | 58 
 gcc/c/c-tree.h   |  2 +-
 gcc/c/c-typeck.c |  4 ++--
 gcc/cp/parser.c  | 45 +++
 4 files changed, 73 insertions(+), 36 deletions(-)

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 121a91c..652e53c 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -6360,41 +6360,54 @@ c_parser_for_statement (c_parser *parser, bool ivdep, 
unsigned short unroll,
 static tree
 c_parser_asm_statement (c_parser *parser)
 {
-  tree quals, str, outputs, inputs, clobbers, labels, ret;
-  bool simple, is_volatile, is_inline, is_goto;
+  tree str, outputs, inputs, clobbers, labels, ret;
+  bool simple;
   location_t asm_loc = c_parser_peek_token (parser)->location;
   int section, nsections;
 
   gcc_assert (c_parser_next_token_is_keyword (parser, RID_ASM));
   c_parser_consume_token (parser);
 
-  quals = NULL_TREE;
-  is_volatile = false;
-  is_inline = false;
-  is_goto = false;
+  /* Handle the asm-qualifier-list.  */
+  location_t volatile_loc = UNKNOWN_LOCATION;
+  location_t inline_loc = UNKNOWN_LOCATION;
+  location_t goto_loc = UNKNOWN_LOCATION;
   for (;;)
 {
-  switch (c_parser_peek_token (parser)->keyword)
+  c_token *token = c_parser_peek_token (parser);
+  location_t loc = token->location;
+  switch (token->keyword)
{
case RID_VOLATILE:
- if (is_volatile)
-   break;
- is_volatile = true;
- quals = c_parser_peek_token (parser)->value;
+ if (volatile_loc)
+   {
+ error_at (loc, "duplicate asm qualifier %qE", token->value);
+ inform (volatile_loc, "first seen here");
+   }
+ else
+   volatile_loc = loc;
  c_parser_consume_token (parser);
  continue;
 
case RID_INLINE:
- if (is_inline)
-   break;
- is_inline = true;
+ if (inline_loc)
+   {
+ error_at (loc, "duplicate asm qualifier %qE", token->value);
+ inform (inline_loc, "first seen here");
+   }
+ else
+   inline_loc = loc;
  c_parser_consume_token (parser);
  continue;
 
case RID_GOTO:
- if (is_goto)
-   break;
- is_goto = true;
+ if (goto_loc)
+   {
+ error_at (loc, "duplicate asm qualifier %qE", token->value);
+ inform (goto_loc, "first seen here");
+   }
+ else
+   goto_loc = loc;
  c_parser_consume_token (parser);
  continue;
 
@@ -6405,6 +6417,10 @@ c_parser_asm_statement (c_parser *parser)
   break;
 }
 
+  bool is_volatile = (volatile_loc != UNKNOWN_LOCATION);
+  bool is_inline = (inline_loc != UNKNOWN_LOCATION);
+  bool is_goto = (goto_loc != UNKNOWN_LOCATION);
+
   /* ??? Follow the C++ parser rather than using the
  lex_untranslated_string kludge.  */
   parser->lex_untranslated_string = true;
@@ -6479,9 +6495,9 @@ c_parser_asm_statement (c_parser *parser)
   if (!c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>"))
 c_parser_skip_to_end_of_block_or_statement (parser);
 
-  ret = build_asm_stmt (quals, build_asm_expr (asm_loc, str, outputs, inputs,
-  clobbers, labels, simple,
-  is_inline));
+  ret = build_asm_stmt (is_volatile,
+   build_asm_expr (asm_loc, str, outputs, inputs,
+   clobbers, labels, simple, is_inline));
 
  error:
   parser->lex_untranslated_string = false;
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index f08a8fc..dc9e3cd 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -679,7 +679,7 @@ extern tree c_start_case (location_t, location_t, tree, 
bool);
 extern void c_finish_case (tree, tree);
 extern tree build_asm_expr (location_t, tree, tree, tree, tree, tree, bool,
bool);
-extern tree build_asm_stmt (tree, tree);
+extern tree build_asm_stmt (bool, tree);
 extern int c_types_compatible_p (tree, tree);
 extern tree c_begin_compound_stmt (bool);
 extern tree 

[PATCH 0/4] c/c++, asm: Various updates

[Segher Boessenkool]

This ties up some loose ends, and adds some more testcases.

Tested on powerpc64-linux {-m32,-m64}.  Is this okay for trunk?


Segher


Segher Boessenkool (4):
  c/c++, asm: Write the asm-qualifier loop without "done" boolean
  c/c++, asm: Use nicer error for duplicate asm qualifiers
  c/c++, asm: Use nicer warning for const and restrict
  c++, asm: Do not handle any asm-qualifiers in top-level asm

 gcc/c/c-parser.c  | 106 ++---
 gcc/c/c-tree.h|   2 +-
 gcc/c/c-typeck.c  |   4 +-
 gcc/cp/parser.c   | 107 ++
 gcc/testsuite/g++.dg/asm-qual-1.C |  13 +
 gcc/testsuite/g++.dg/asm-qual-2.C |  46 
 gcc/testsuite/g++.dg/asm-qual-3.C |  12 +
 gcc/testsuite/gcc.dg/asm-qual-1.c |   6 +--
 gcc/testsuite/gcc.dg/asm-qual-3.c |   9 
 9 files changed, 209 insertions(+), 96 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/asm-qual-1.C
 create mode 100644 gcc/testsuite/g++.dg/asm-qual-2.C
 create mode 100644 gcc/testsuite/g++.dg/asm-qual-3.C
 create mode 100644 gcc/testsuite/gcc.dg/asm-qual-3.c

-- 
1.8.3.1

[PATCH 1/4] c/c++, asm: Write the asm-qualifier loop without "done" boolean

[Segher Boessenkool]

As suggested by Jason.


Segher


2018-12-10  Segher Boessenkool  

c/
* c-parser.c (c_parser_asm_statement): Rewrite the loop to work without
"done" boolean variable.

cp/
* parser.c (cp_parser_asm_definition): Rewrite the loop to work without
"done" boolean variable.

---
 gcc/c/c-parser.c | 66 +---
 gcc/cp/parser.c  | 71 +---
 2 files changed, 63 insertions(+), 74 deletions(-)

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index b875c4f..121a91c 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -6372,40 +6372,37 @@ c_parser_asm_statement (c_parser *parser)
   is_volatile = false;
   is_inline = false;
   is_goto = false;
-  for (bool done = false; !done; )
-switch (c_parser_peek_token (parser)->keyword)
-  {
-  case RID_VOLATILE:
-   if (!is_volatile)
- {
-   is_volatile = true;
-   quals = c_parser_peek_token (parser)->value;
-   c_parser_consume_token (parser);
- }
-   else
- done = true;
-   break;
-  case RID_INLINE:
-   if (!is_inline)
- {
-   is_inline = true;
-   c_parser_consume_token (parser);
- }
-   else
- done = true;
-   break;
-  case RID_GOTO:
-   if (!is_goto)
- {
-   is_goto = true;
-   c_parser_consume_token (parser);
- }
-   else
- done = true;
-   break;
-  default:
-   done = true;
-  }
+  for (;;)
+{
+  switch (c_parser_peek_token (parser)->keyword)
+   {
+   case RID_VOLATILE:
+ if (is_volatile)
+   break;
+ is_volatile = true;
+ quals = c_parser_peek_token (parser)->value;
+ c_parser_consume_token (parser);
+ continue;
+
+   case RID_INLINE:
+ if (is_inline)
+   break;
+ is_inline = true;
+ c_parser_consume_token (parser);
+ continue;
+
+   case RID_GOTO:
+ if (is_goto)
+   break;
+ is_goto = true;
+ c_parser_consume_token (parser);
+ continue;
+
+   default:
+ break;
+   }
+  break;
+}
 
   /* ??? Follow the C++ parser rather than using the
  lex_untranslated_string kludge.  */
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index adfe09e..1cc34ba 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -19648,47 +19648,38 @@ cp_parser_asm_definition (cp_parser* parser)
   cp_function_chain->invalid_constexpr = true;
 }
 
-  /* See if the next token is `volatile'.  */
+  /* Handle the asm-qualifier-list.  */
   if (cp_parser_allow_gnu_extensions_p (parser))
-for (bool done = false; !done ; )
-  switch (cp_lexer_peek_token (parser->lexer)->keyword)
-   {
-   case RID_VOLATILE:
- if (!volatile_p)
-   {
- /* Remember that we saw the `volatile' keyword.  */
- volatile_p = true;
- /* Consume the token.  */
- cp_lexer_consume_token (parser->lexer);
-   }
- else
-   done = true;
- break;
-   case RID_INLINE:
- if (!inline_p && parser->in_function_body)
-   {
- /* Remember that we saw the `inline' keyword.  */
- inline_p = true;
- /* Consume the token.  */
- cp_lexer_consume_token (parser->lexer);
-   }
- else
-   done = true;
- break;
-   case RID_GOTO:
- if (!goto_p && parser->in_function_body)
-   {
- /* Remember that we saw the `goto' keyword.  */
- goto_p = true;
- /* Consume the token.  */
- cp_lexer_consume_token (parser->lexer);
-   }
- else
-   done = true;
- break;
-   default:
- done = true;
-   }
+for (;;)
+  {
+   switch (cp_lexer_peek_token (parser->lexer)->keyword)
+ {
+ case RID_VOLATILE:
+   if (volatile_p)
+ break;
+   volatile_p = true;
+   cp_lexer_consume_token (parser->lexer);
+   continue;
+
+ case RID_INLINE:
+   if (inline_p || !parser->in_function_body)
+ break;
+   inline_p = true;
+   cp_lexer_consume_token (parser->lexer);
+   continue;
+
+ case RID_GOTO:
+   if (goto_p || !parser->in_function_body)
+ break;
+   goto_p = true;
+   cp_lexer_consume_token (parser->lexer);
+   continue;
+
+ default:
+   break;
+ }
+   break;
+  }
 
   /* Look for the opening `('.  */
   if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN))
-- 
1.8.3.1

Re: [PATCH 1/4] introduce struct strlen_data_t into gimple-fold

[Martin Sebor]

Jeff, is there something you are expecting me to change in
response to this or have you just not gotten around to reviewing
the rest?

Martin

On 11/28/18 9:26 PM, Jeff Law wrote:

On 11/25/18 5:05 PM, Martin Sebor wrote:



If so, then I think we need
to look for a better name than MAXSIZE and MAXLEN.


I find these names quite fitting and I'm not sure what might work
better.  I renamed MAXSIZE to MAXBOUND but nothing comes to mind
as a replacement for MAXLEN.  Please suggest something you think
is better.




+  /* When non-null, NONSTR refers to the declaration known to store
+ an unterminated constant character array, as in:
+ const char s[] = { 'a', 'b', 'c' };
+ It is used to diagnose uses of such arrays in functions such as
+ strlen() that expect a nul-terminated string as an argument.  */
+  tree nonstr;

So rather than NONSTR, DECL may make more sense -- if for no other
reason than you don't have to think in terms of "not a string".


Done, but I think DECL is a poor choice for the reasons below.

The field is only set when the thing the object refers to is
a character array that is not a string.  It identifies the first
array the expression refers to that's not a terminated string
(there could be multiple).  I can't think of anything else one
might want to think of it as than "a declaration of an array
that is not a string."

As a name, DECL is generic and used all over the place for any
sort of a declaration so it's not a good choice also for that
reason.  It's only marginally more descriptive that the pervasive
NODE or T, but just as useless to grep for (which I have been
relying on when working with this patch).

I have been using the name NONSTR in all contexts where
I introduced the unterminated array handling, so renaming
the member to DECL makes this scheme inconsistent

NONSTR requires you to think in the negative and it sounds more like a
boolean property to me, but it's actually carrying more information than
just a boolean.

I'm certainly not wed to DECL.  If you've got a better name, please
suggest one.





+  /* ELTSIZE is set to 1 for single byte character strings, and 2 or 4
+ for wide characer strings.  */
+  unsigned eltsize;

Bernd's suggestion that we separate the input vs output paramters may be
a reasonable one -- I think this is the only in-parameter you're passing
with the structure, right?  And everything else is a pure output?  If so
we may be better off continuing to pass the element size as a separate
parameter.


I changed it in the updated patch.  I had chosen to make it
a member to reduce the number of arguments to these functions and
in anticipation of having them update it before returning if they
discover that the actual element size doesn't match the expected
size, as in:

   printf ("%ls", "narrow string");

Similarly to what I proposed here:
   https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01321.html

I don't see what has been gained by making it an argument again.

It's the separation of inputs from outputs he was trying to achieve that
I said *may* be a reasonable one.

It's not always a good thing, nor is it always a bad thing.  If there
are subsequent patches are going to have callees changing it, then it
absolutely makes sense to include it.  But adding it to the structure
for the mere sake of reducing arguments may not.

And just to be clear, I'm not inherently against pulling stuff into
structures and classes.   Quite the opposite.




+  /* FLEXARRAY is true if the range of the string lengths has been
+ obtained from the upper bound of an array at the end of a struct.
+ Such an array may hold a string that's longer than its upper bound
+ due to it being used as a poor-man's flexible array member.  */
+  bool flexarray;
+
+  /* Set ELTSIZE and value-initialize all other members.  */
+  strlen_data_t (unsigned eltbytes)
+    : minlen (), maxlen (), maxsize (), nonstr (), eltsize (eltbytes),
+  flexarray () { }

I think if you pull ELTSIZE out and pass it as a distinct parameter,
then you don't need a ctor and you can have a POD.  You can then
initialize with memset rather than having to individually initialize
each field -- meaning it's easier to add more output fields in the
future.


Without ELTSIZE neither a ctor nor memset is necessary for
initialization.  This works too and is the preferred style
in C++ 98:

   c_strlen_data data = { };

I thought that didn't work somewhere.  I certainly would have preferred
that over memset when I was twiddling yours and Bernd's earlier patches.







I don't think any of the suggestions above change the behavior of the
patch.  Let's hold off committing though (I assume you've got a GIT
topic branch where you can make these changes and update the subsequent
patches independent of everything else...)


I do but I don't know how to make these changes without having
to resolve all the conflicts with the intervening changes on
trunk at each step so I have just a single patch now 

Re: [PATCH] PR86957

[Indu Bhagat]

On 12/06/2018 05:54 PM, Indu Bhagat wrote:
2. I do however see other tests (a total of 23) which are have 
regressed from

   PASS --> UNRESOLVED. A diff is attached.

   Each one of them is due to "Error/Warning threshold exceeded: 1 0 
(max. 1 3)"

False alarm.

Looks like there is some flakiness I ran into.

New testsuite runs (make check-gcc) on a fresh builds of the GCC (with 
and without missing-profile) enabled look OK.
One additional failure as expected (Wmissing-profile.c). No additional 
unresolved tests.


Indu

Re: [PATCH] Delete powerpcspe

[Segher Boessenkool]

On Mon, Dec 10, 2018 at 06:25:31PM +, Andrew Jenner wrote:
> Sorry for the slow response on this, I was on vacation last week.
> 
> On 03/12/2018 21:48, Jakub Jelinek wrote:
> >I'd give the maintainers the last week to act if they don't want this
> >to happen and if nothing happens, commit it.  PR81084 lists all the reasons
> >why it should be removed when it is totally unmaintained.
> >Just make sure to put stuff that belongs there to gcc/ChangeLog and without
> >gcc/ prefixes.
> 
> Yes, please go ahead and commit

Committed to trunk as r266961.

> - it's not fair on other maintainers to 
> have to work around my lack of action on this port. I will continue to 
> work on it out-of-tree and hope to restore it once it is in proper shape.

The more important thing is maintenance...  Regular and/or frequent tests
(posted to gcc-testresults@), bug tracker maintenance, etc.  You need to
be visible.


Segher

[Committed] PR fortran/88269 -- Check for missing UNIT/NEWUNIT

[Steve Kargl]

The attach patch has been coommitted to 7-branch, 8-branch,
and trunk after successful regression testing.

2018-12-10  Steven G. Kargl  

PR fortran/88269
* io.c (io_constraint): Update macro. If locus line buffer is NULL,
use gfc_current_locus in error messages.
(check_io_constraints): Catch missing IO UNIT in write and read
statements.  io_constraint macro is incompatible here.

2018-12-10  Steven G. Kargl  

PR fortran/88269
* gfortran.dg/pr88269.f90: New test.

-- 
Steve
Index: gcc/fortran/io.c
===
--- gcc/fortran/io.c	(revision 266958)
+++ gcc/fortran/io.c	(working copy)
@@ -3678,10 +3678,13 @@ static match
 check_io_constraints (io_kind k, gfc_dt *dt, gfc_code *io_code,
 		  locus *spec_end)
 {
-#define io_constraint(condition,msg,arg)\
+#define io_constraint(condition, msg, arg)\
 if (condition) \
   {\
-gfc_error(msg,arg);\
+if ((arg)->lb != NULL)\
+  gfc_error ((msg), (arg));\
+else\
+  gfc_error ((msg), _current_locus);\
 m = MATCH_ERROR;\
   }
 
@@ -3741,11 +3744,14 @@ if (condition) \
   if (expr && expr->ts.type != BT_CHARACTER)
 {
 
-  io_constraint (gfc_pure (NULL) && (k == M_READ || k == M_WRITE),
-		 "IO UNIT in %s statement at %C must be "
+  if (gfc_pure (NULL) && (k == M_READ || k == M_WRITE))
+	{
+	  gfc_error ("IO UNIT in %s statement at %C must be "
 		 "an internal file in a PURE procedure",
 		 io_kind_name (k));
-
+	  return MATCH_ERROR;
+	}
+	  
   if (k == M_READ || k == M_WRITE)
 	gfc_unset_implicit_pure (NULL);
 }
Index: gcc/testsuite/gfortran.dg/pr88269.f90
===
--- gcc/testsuite/gfortran.dg/pr88269.f90	(nonexistent)
+++ gcc/testsuite/gfortran.dg/pr88269.f90	(working copy)
@@ -0,0 +1,6 @@
+! { dg-do compile }
+! PR fortran/88269
+program p
+   write (end=1e1) ! { dg-error "tag not allowed" }
+end
+

C++ PATCH for c++/88216, ICE with class type in non-type template parameter

[Marek Polacek]

Here we were crashing in cxx_eval_constant_expression:
 4600   /* We can only get here in checking mode via
 4601  build_non_dependent_expr,  because any expression that
 4602  calls or takes the address of the function will have
 4603  pulled a FUNCTION_DECL out of the COMPONENT_REF.  */
 4604   gcc_checking_assert (ctx->quiet || errorcount);
 4605   *non_constant_p = true;
 4606   return t;
 4607 }
where we got via 
 6661   if (!TREE_CONSTANT (expr))
 6662 {
 6663   if ((complain & tf_error)
 6664   && require_rvalue_constant_expression (expr))
 6665 cxx_constant_value (expr);
    return error_mark_node;
 6667 }
because the expr wasn't constant.  The fix would be to add error() I think.

But the testcase is valid, so we need (also) something else.  This problem
occurs when using a class type non-type template parameter as a template
argument, because the TREE_CONSTANT check in get_template_parm_object will
fail--we're in a template and build_converted_constant_expr didn't produce
a constant, because the expression contains template codes.  So I thought
it would make sense to just return and create the template parameter object
at instantiation.

But then I was running into sorry() in the demangler so I had to make tweaks
there too.  I don't know much about that code, so I hope I'm not doing anything
overly stoopid.

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

2018-12-10  Marek Polacek  

PR c++/88216 - ICE with class type in non-type template parameter.
* mangle.c (write_expression): Handle TARGET_EXPR and
VIEW_CONVERT_EXPR.
* pt.c (get_template_parm_object): Just return for value dependent
expressions.

* g++.dg/cpp2a/nontype-class9.C: New test.

diff --git gcc/cp/mangle.c gcc/cp/mangle.c
index 64415894bc5..56247883010 100644
--- gcc/cp/mangle.c
+++ gcc/cp/mangle.c
@@ -2836,13 +2836,21 @@ write_expression (tree expr)
 {
   enum tree_code code = TREE_CODE (expr);
 
+  if (TREE_CODE (expr) == TARGET_EXPR)
+{
+  expr = TARGET_EXPR_INITIAL (expr);
+  code = TREE_CODE (expr);
+}
+
   /* Skip NOP_EXPR and CONVERT_EXPR.  They can occur when (say) a pointer
  argument is converted (via qualification conversions) to another type.  */
   while (CONVERT_EXPR_CODE_P (code)
 || location_wrapper_p (expr)
 /* Parentheses aren't mangled.  */
 || code == PAREN_EXPR
-|| code == NON_LVALUE_EXPR)
+|| code == NON_LVALUE_EXPR
+|| (code == VIEW_CONVERT_EXPR
+&& TREE_CODE (TREE_OPERAND (expr, 0)) == TEMPLATE_PARM_INDEX))
 {
   expr = TREE_OPERAND (expr, 0);
   code = TREE_CODE (expr);
diff --git gcc/cp/pt.c gcc/cp/pt.c
index 8560e588593..878a354b193 100644
--- gcc/cp/pt.c
+++ gcc/cp/pt.c
@@ -6655,6 +6655,9 @@ invalid_tparm_referent_p (tree type, tree expr, 
tsubst_flags_t complain)
 static tree
 get_template_parm_object (tree expr, tsubst_flags_t complain)
 {
+  if (processing_template_decl && value_dependent_expression_p (expr))
+return expr;
+
   if (TREE_CODE (expr) == TARGET_EXPR)
 expr = TARGET_EXPR_INITIAL (expr);
 
diff --git gcc/testsuite/g++.dg/cpp2a/nontype-class9.C 
gcc/testsuite/g++.dg/cpp2a/nontype-class9.C
new file mode 100644
index 000..737f712be47
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp2a/nontype-class9.C
@@ -0,0 +1,29 @@
+// PR c++/88216
+// { dg-do compile { target c++2a } }
+
+template  struct same;
+template  struct same {};
+
+struct T { };
+
+template 
+struct U { };
+
+template 
+void f (U)
+{
+  same s;
+  same s2;
+}
+
+template
+U u;
+
+T t;
+U u2;
+
+void
+g ()
+{
+  f(u2);
+}

Re: [PATCH, Fortran] pad char to int conversions with spaces instead of zeros (legacy)

[Fritz Reese]

> On Mon, Dec 10, 2018 at 02:09:50PM +, Mark Eggleston wrote:
> >
> > On 06/12/2018 10:20, Mark Eggleston wrote:
> > > > Yes. Mark, you'll need to also patch iresolve.c (gfc_resolve_transfer)
> > > > to affect non-constant resolution.
> > > Thanks for the hint.
> >
> > I've looked at gfc_resolve_transfer regarding handling of padding when a
> > character variable is passed to transfer instead of a literal. This routine
> > is not called so can't be where a variable would be handled.
> >
> > Don't yet know where to make the change.

It actually is called through a function pointer
(gfc_intrinsic_sym->resolve) in intrinsic.c (resolve_intrinsic), as
with all intrinsics. You can find the backtrace by running f951
(`gfortran -print-prog-name=f951`) through gdb and setting a
breakpoint on gfc_resolve_transfer. That being said...

On Mon, Dec 10, 2018 at 12:12 PM Jakub Jelinek  wrote:
> I think you want to change gfc_conv_intrinsic_transfer

... in this case Jakub is right. I hadn't looked at the body of the
resolve function yet, but peeking at it tells me the transfer
expression doesn't contain support for padding information, so you'd
have to deal with it yourself in translation.

> I guess you want to add after this, guarded with flag_dec only,
> code to compare (at runtime) if extent < dest_word_len and if so,
> use fill_with_spaces to pad it with spaces at the end (from
> that first memcpy's argument + extent dest_word_len - extent bytes),
> with a comment why you are doing it.  Guess the array case will need
> something similar.

Alternatively you could call BUILT_IN_MEMSET(, '\x20',
dest_word_len) just prior to the MEMCPY whenever flag_dec is set.

Fritz

Re: [PATCH] [RFC] PR target/52813 and target/11807

[Dimitar Dimitrov]

On понеделник, 10 декември 2018 г. 11:21:53 EET Richard Sandiford wrote:
> Dimitar Dimitrov  writes:
> > I have tested this fix on x86_64 host, and found no regression in the C
> > and C++ testsuites.  I'm marking this patch as RFC simply because I don't
> > have experience with other architectures, and I don't have a setup to
> > test all architectures supported by GCC.
> > 
> > gcc/ChangeLog:
> > 
> > 2018-12-07  Dimitar Dimitrov  
> > 
> > * cfgexpand.c (asm_clobber_reg_is_valid): Also produce
> > error when stack pointer is clobbered.
> > (expand_asm_stmt): Refactor clobber check in separate function.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 2018-12-07  Dimitar Dimitrov  
> > 
> > * gcc.target/i386/pr52813.c: New test.
> > 
> > Signed-off-by: Dimitar Dimitrov 
> 
> LGTM.  Do you have a copyright assignment on file?  'Fraid this is
> probably big enough to need one.
Yes, I have copyright assignment.

Regards,
Dimitar

Re: [PATCH] Set DEMANGLE_RECURSION_LIMIT to 1536

[Jakub Jelinek]

On Mon, Dec 10, 2018 at 10:20:24AM -0800, Ian Lance Taylor wrote:
> > I think it is a bad idea to use the same macro and value for both the
> > recursion limit and essentially stack limit.  For the latter, it should
> > actually compute expected stack size
> > (i.e. di.num_comps * sizeof (*di.comps) + di.num_subs * sizeof (*di.subs))
> > and rather than just giving up should say that memory up to 64KB this
> > way will be handled via alloca, more through say mmap (I'd avoid malloc
> > because some users are using these APIs in cases where malloc isn't usable).
> > And have only much larger limit, like say 1MB for these arrays on which to
> > give up.
> 
> That makes sense.
> 
> We've wanted to avoid malloc in this code because some programs call
> the demangler from a signal handler.  But using mmap should be fine in
> practice.

mmap is not available everywhere, but we could just have a smaller limit
on how big mangled names we handle on targets where mmap isn't available or
if it fails.

And, the other option is just try to parse the string without doing all the
processing first and compute (perhaps upper bound) on how many components
and substitutions we need.  Many components have longish names, or numbers,
etc. so the 2 * strlen is really very upper bound and strlen for number of
substitutions too.

Jakub

Re: [PATCH] Delete powerpcspe

[Andrew Jenner]

Sorry for the slow response on this, I was on vacation last week.

On 03/12/2018 21:48, Jakub Jelinek wrote:

I'd give the maintainers the last week to act if they don't want this
to happen and if nothing happens, commit it.  PR81084 lists all the reasons
why it should be removed when it is totally unmaintained.
Just make sure to put stuff that belongs there to gcc/ChangeLog and without
gcc/ prefixes.


Yes, please go ahead and commit - it's not fair on other maintainers to 
have to work around my lack of action on this port. I will continue to 
work on it out-of-tree and hope to restore it once it is in proper shape.


Thanks,

Andrew

Re: [PATCH] Set DEMANGLE_RECURSION_LIMIT to 1536

[Ian Lance Taylor via gcc-patches]

On Mon, Dec 10, 2018 at 7:35 AM Jakub Jelinek  wrote:
>
> On Mon, Dec 10, 2018 at 03:26:18PM +, Nick Clifton wrote:
> > >> My current suggestion
> > >> is to raise the limit to 2048, which allows the libiberty patch to
> > >> pass.  But do you have a feel for how much is a realistic limit ?
> > >
> > > For recursion limit I think that is fine.
> > > For just stack size limit, I think it is extremely small.
> > > I see that in the function it allocates on 64-bit 24 bytes times
> > > num_comps using alloca, so 48 bytes per character in the mangled name,
> > > and a pointer for each character in the mangled name.
> > > That is 112KB per 2048 bytes long mangled name.
> > >
> > > Dunno how much stack can we expect to be usable.
> >
> > Currently the patched libiberty uses the DEMANGLE_RECURSION_LIMIT value
> > in two ways.  The first is as a limit on the number of levels of recursion
> > of specific functions inside the demangler.  The second is as a check on
> > the number of component structures that will be allocated on the stack.
> > (See cp-demangle.c:d_demangle_callback).  One of the CVEs that I was 
> > checking
> > was triggering stack exhaustion this way, which is why I added the check.
> >
> > There is at least one other function where a similar stack allocation
> > happens (cplus_demangle_print_callback) but I was not sure if this could
> > be triggered with the other limits in place, and I did not have a reproducer
> > that touched it, so I left it alone.
>
> I think it is a bad idea to use the same macro and value for both the
> recursion limit and essentially stack limit.  For the latter, it should
> actually compute expected stack size
> (i.e. di.num_comps * sizeof (*di.comps) + di.num_subs * sizeof (*di.subs))
> and rather than just giving up should say that memory up to 64KB this
> way will be handled via alloca, more through say mmap (I'd avoid malloc
> because some users are using these APIs in cases where malloc isn't usable).
> And have only much larger limit, like say 1MB for these arrays on which to
> give up.

That makes sense.

We've wanted to avoid malloc in this code because some programs call
the demangler from a signal handler.  But using mmap should be fine in
practice.

Ian

Re: [PATCH, Fortran] pad char to int conversions with spaces instead of zeros (legacy)

[Jakub Jelinek]

On Mon, Dec 10, 2018 at 02:09:50PM +, Mark Eggleston wrote:
> 
> On 06/12/2018 10:20, Mark Eggleston wrote:
> > > Yes. Mark, you'll need to also patch iresolve.c (gfc_resolve_transfer)
> > > to affect non-constant resolution.
> > Thanks for the hint.
> 
> I've looked at gfc_resolve_transfer regarding handling of padding when a
> character variable is passed to transfer instead of a literal. This routine
> is not called so can't be where a variable would be handled.
> 
> Don't yet know where to make the change.

I think you want to change gfc_conv_intrinsic_transfer
For the scalar case, which is the one for which I've posted testcase, there
is:
  extent = fold_build2_loc (input_location, MIN_EXPR, gfc_array_index_type,
dest_word_len, source_bytes);
  extent = fold_build2_loc (input_location, MAX_EXPR, gfc_array_index_type,
extent, gfc_index_zero_node);
and later:
  tmp = build_call_expr_loc (input_location,
 builtin_decl_explicit (BUILT_IN_MEMCPY), 3,
 fold_convert (pvoid_type_node, tmp),
 fold_convert (pvoid_type_node, ptr),
 fold_convert (size_type_node, extent));
  gfc_add_expr_to_block (>pre, tmp);
I guess you want to add after this, guarded with flag_dec only,
code to compare (at runtime) if extent < dest_word_len and if so,
use fill_with_spaces to pad it with spaces at the end (from
that first memcpy's argument + extent dest_word_len - extent bytes),
with a comment why you are doing it.  Guess the array case will need
something similar.  But, for these runtime checks, guess you should start by
looking what the other compilers are exactly doing for the different kinds
of transfers, if they pad for non-constants at all, if they only pad if
transfer is from a character object to whatever, etc.

Looking at ifort on godbolt
function foo (e)
  integer(kind=8) :: foo
  character(len=4) :: e
  foo = transfer(e, 1_8)
end
I see it calls for_cpystr (..., 8, ..., 4, ...)
so I guess it uses standard library function to copy string into the
destination in that case, so essentially what gfc_trans_string_copy
does inline or our fstrcpy (internal library function).

Jakub

Re: [gofrontend-dev] Re: libgo patch committed: Add precise stack scan support

[Cherry Zhang via gcc-patches]

On Mon, Dec 10, 2018 at 1:41 AM Matthias Klose  wrote:

> On 06.12.18 00:09, Ian Lance Taylor wrote:
> > This libgo patch by Cherry Zhang adds support for precise stack
> > scanning to the Go runtime.  This uses per-function stack maps stored
> > in the exception tables in the language-specific data area.  The
> > compiler needs to generate these stack maps; currently this is only
> > done by a version of LLVM, not by GCC.  Each safepoint in a function
> > is associated with a (real or dummy) landing pad, and its "type info"
> > in the exception table is a pointer to the stack map. When a stack is
> > scanned, the stack map is found by the stack unwinding code.
> >
> > For precise stack scan we need to unwind the stack. There are three
> cases:
> >
> > - If a goroutine is scanning its own stack, it can unwind the stack
> > and scan the frames.
> >
> > - If a goroutine is scanning another, stopped, goroutine, it cannot
> > directly unwind the target stack. We handle this by switching
> > (runtime.gogo) to the target g, letting it unwind and scan the stack,
> > and switch back.
> >
> > - If we are scanning a goroutine that is blocked in a syscall, we send
> > a signal to the target goroutine's thread, and let the signal handler
> > unwind and scan the stack. Extra care is needed as this races with
> > enter/exit syscall.
> >
> > Currently this is only implemented on GNU/Linux.
> >
> > Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
> > to mainline.
>
> this broke the libgo build on ARM32:
>
> ../../../src/libgo/runtime/go-unwind.c: In function
> 'scanstackwithmap_callback':
> ../../../src/libgo/runtime/go-unwind.c:754:18: error: '_URC_NORMAL_STOP'
> undeclared (first use in this function)
>   754 |   return _URC_NORMAL_STOP;
>   |  ^~~~
> ../../../src/libgo/runtime/go-unwind.c:754:18: note: each undeclared
> identifier
> is reported only once for each function i
> t appears in
> ../../../src/libgo/runtime/go-unwind.c: In function
> 'probestackmaps_callback':
> ../../../src/libgo/runtime/go-unwind.c:802:10: error: '_URC_NORMAL_STOP'
> undeclared (first use in this function)
>   802 |   return _URC_NORMAL_STOP;
>   |  ^~~~
> ../../../src/libgo/runtime/go-unwind.c:803:1: warning: control reaches end
> of
> non-void function [-Wreturn-type]
>   803 | }
>   | ^
> make[6]: *** [Makefile:1474: runtime/go-unwind.lo] Error 1
> make[6]: Leaving directory
> '/<>/build/arm-linux-gnueabihf/libgo'
>
>
Hell Matthias,

Thank you for the report. And sorry about the breakage. Does
https://go-review.googlesource.com/c/gofrontend/+/153417 (or the patch
below) fix ARM32 build? I don't have an ARM32 machine at hand to test.

Thanks,
Cherry

diff --git a/libgo/runtime/go-unwind.c b/libgo/runtime/go-unwind.c
index a1a95585..c44755f9 100644
--- a/libgo/runtime/go-unwind.c
+++ b/libgo/runtime/go-unwind.c
@@ -392,6 +392,12 @@ parse_lsda_header (struct _Unwind_Context *context,
const unsigned char *p,
 #define CONTINUE_UNWINDING return _URC_CONTINUE_UNWIND
 #endif

+#ifdef __ARM_EABI_UNWINDER__
+#define STOP_UNWINDING _URC_FAILURE
+#else
+#define STOP_UNWINDING _URC_NORMAL_STOP
+#endif
+
 #ifdef __USING_SJLJ_EXCEPTIONS__
 #define PERSONALITY_FUNCTION__gccgo_personality_sj0
 #define __builtin_eh_return_data_regno(x) x
@@ -751,7 +757,7 @@ scanstackwithmap_callback (struct _Unwind_Context
*context, void *arg)
   // TODO: print gp, pc, sp
   runtime_throw ("no stack map");
 }
-  return _URC_NORMAL_STOP;
+  return STOP_UNWINDING;
 }
   case FOUND:
 break;
@@ -799,7 +805,7 @@ probestackmaps_callback (struct _Unwind_Context
*context,

   // Found a stack map. No need to keep unwinding.
   runtime_usestackmaps = true;
-  return _URC_NORMAL_STOP;
+  return STOP_UNWINDING;
 }

 // Try to find a stack map, store the result in global variable
runtime_usestackmaps.

[PATCH, i386]: Fix PR88418, ICE in extract_insn

[Uros Bizjak]

Hello!

For vector modes we have to use vector_operand predicate, which
rejects unaligned operands for non-avx targets.

2018-12-10  Uros Bizjak  

PR target/88418
* config/i386/i386.c (ix86_expand_sse_cmp): For vector modes,
check operand 1 with vector_operand predicate.
(ix86_expand_sse_movcc): For vector modes, check op_true with
vector_operand, not nonimmediate_operand.

testsuite/ChangeLog:

2018-12-10  Uros Bizjak  

PR target/88418
* gcc.target/i386/pr88418.c: New test.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Committed to mainline, patch will be backported to release branches.

Uros.
Index: config/i386/i386.c
===
--- config/i386/i386.c  (revision 266926)
+++ config/i386/i386.c  (working copy)
@@ -23483,7 +23483,7 @@ ix86_expand_sse_fp_minmax (rtx dest, enum rtx_code
   return true;
 }
 
-/* Expand an sse vector comparison.  Return the register with the result.  */
+/* Expand an SSE comparison.  Return the register with the result.  */
 
 static rtx
 ix86_expand_sse_cmp (rtx dest, enum rtx_code code, rtx cmp_op0, rtx cmp_op1,
@@ -23508,9 +23508,12 @@ ix86_expand_sse_cmp (rtx dest, enum rtx_code code,
   else
 cmp_mode = cmp_ops_mode;
 
+  cmp_op0 = force_reg (cmp_ops_mode, cmp_op0);
 
-  cmp_op0 = force_reg (cmp_ops_mode, cmp_op0);
-  if (!nonimmediate_operand (cmp_op1, cmp_ops_mode))
+  int (*op1_predicate)(rtx, machine_mode)
+= VECTOR_MODE_P (cmp_ops_mode) ? vector_operand : nonimmediate_operand;
+
+  if (!op1_predicate (cmp_op1, cmp_ops_mode))
 cmp_op1 = force_reg (cmp_ops_mode, cmp_op1);
 
   if (optimize
@@ -23627,7 +23630,7 @@ ix86_expand_sse_movcc (rtx dest, rtx cmp, rtx op_t
   rtx (*gen) (rtx, rtx, rtx, rtx) = NULL;
   rtx d = dest;
 
-  if (!nonimmediate_operand (op_true, mode))
+  if (!vector_operand (op_true, mode))
op_true = force_reg (mode, op_true);
 
   op_false = force_reg (mode, op_false);
Index: testsuite/gcc.target/i386/pr88418.c
===
--- testsuite/gcc.target/i386/pr88418.c (nonexistent)
+++ testsuite/gcc.target/i386/pr88418.c (working copy)
@@ -0,0 +1,15 @@
+/* PR target/88418 */
+/* { dg-do compile } */
+/* { dg-options "-O1 -fpack-struct -msse4.1" } */
+
+typedef long long v2di __attribute__ ((__vector_size__ (16)));
+
+union df {
+  v2di se[2];
+};
+
+void
+qg (union df *jz, union df *pl)
+{
+  jz->se[0] = jz->se[0] == pl->se[0];
+}

Re: [PATCH] Set DEMANGLE_RECURSION_LIMIT to 1536

[Jakub Jelinek]

On Mon, Dec 10, 2018 at 03:26:18PM +, Nick Clifton wrote:
> >> My current suggestion
> >> is to raise the limit to 2048, which allows the libiberty patch to 
> >> pass.  But do you have a feel for how much is a realistic limit ?
> > 
> > For recursion limit I think that is fine.
> > For just stack size limit, I think it is extremely small.
> > I see that in the function it allocates on 64-bit 24 bytes times
> > num_comps using alloca, so 48 bytes per character in the mangled name,
> > and a pointer for each character in the mangled name.
> > That is 112KB per 2048 bytes long mangled name.
> > 
> > Dunno how much stack can we expect to be usable.
> 
> Currently the patched libiberty uses the DEMANGLE_RECURSION_LIMIT value
> in two ways.  The first is as a limit on the number of levels of recursion
> of specific functions inside the demangler.  The second is as a check on
> the number of component structures that will be allocated on the stack.
> (See cp-demangle.c:d_demangle_callback).  One of the CVEs that I was checking
> was triggering stack exhaustion this way, which is why I added the check.
> 
> There is at least one other function where a similar stack allocation
> happens (cplus_demangle_print_callback) but I was not sure if this could
> be triggered with the other limits in place, and I did not have a reproducer
> that touched it, so I left it alone.

I think it is a bad idea to use the same macro and value for both the
recursion limit and essentially stack limit.  For the latter, it should
actually compute expected stack size
(i.e. di.num_comps * sizeof (*di.comps) + di.num_subs * sizeof (*di.subs))
and rather than just giving up should say that memory up to 64KB this
way will be handled via alloca, more through say mmap (I'd avoid malloc
because some users are using these APIs in cases where malloc isn't usable).
And have only much larger limit, like say 1MB for these arrays on which to
give up.

Jakub

Re: [PATCH] Set DEMANGLE_RECURSION_LIMIT to 1536

[Jason Merrill]

On Mon, Dec 10, 2018 at 10:10 AM Jakub Jelinek  wrote:
> On Mon, Dec 10, 2018 at 02:52:39PM +, Michael Matz wrote:
> > On Fri, 7 Dec 2018, H.J. Lu wrote:
> >
> > > > > On Thu, Dec 6, 2018 at 3:12 AM Nick Clifton  wrote:
> > > > > >
> > > > > >   Is the patch OK with you ?
> > > > >
> > > >
> > > > This caused:
> > > >
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88409
> > > >
> > >
> > > Here is the fix.   OK for trunk?
> >
> > I think this points toward the limit being _much_ too low.  With template
> > meta programming you easily get these mangled names, it's not even a
> > particularly long one.  But I'm wondering a bit, without tracing the
> > demangler, just looking at the symbol name and demangled result I don't
> > readily see where the depth of recursion really is more than 1024, are
> > there perhaps some recursion_level-- statements skipped?
>
> That is because the recursion_level limit isn't hit in this case at all (far
> from it).
>
> What breaks it is this:
>
>   /* PR 87675 - Check for a mangled string that is so long
>  that we do not have enough stack space to demangle it.  */
>   if (((options & DMGL_NO_RECURSE_LIMIT) == 0)
>   /* This check is a bit arbitrary, since what we really want to do is to
>  compare the sizes of the di.comps and di.subs arrays against the
>  amount of stack space remaining.  But there is no portable way to do
>  this, so instead we use the recursion limit as a guide to the maximum
>  size of the arrays.  */
>   && (unsigned long) di.num_comps > DEMANGLE_RECURSION_LIMIT)
> {
>   /* FIXME: We need a way to indicate that a stack limit has been 
> reached.  */
>   return 0;
> }

> where di.num_comps is just strlen (mangled) * 2.  Without any analysis
> whatsoever, bumping the "recursion" limit will just mean we can process 1.5
> times long names.  Either we need more precise analysis on what we are
> looking for (how big arrays we'll need) or it needs to be an independent
> limit and certainly should allow say 10KB symbols too if they are
> reasonable.

If the problem is alloca, we could avoid using alloca if the size
passes a threshold.  Perhaps even use a better data structure than a
preallocated array based on a guess about the number of components...

Jason

Re: [PATCH] Set DEMANGLE_RECURSION_LIMIT to 1536

[Nick Clifton]

Hi David,

> Apologies in advance if this has been covered, as I've only been half-
> watching this thread, but is it always the case that the recursion
> depth can be bounded by some scalar multiple of the number of
> characters in the name?

Probably, but the point of this patch is to add a fixed limit that
prevents too much recursion from being performed.  The CVEs that I
have been trying to fix have been using mangled names with 20K-30K
characters in them, so creating a recursion limit based on the 
length of the input would not prevent the stack exhaustion. :-(

My hope is that we can choose a value that will allow any realistic
mangled name to be decoded, but which will prevent these fuzzers from
generating arbitrary length strings which exhaust the machines resources.

Cheers
  Nick

Re: [PATCH] Set DEMANGLE_RECURSION_LIMIT to 1536

[Nick Clifton]

Hi Jakub,

>> My current suggestion
>> is to raise the limit to 2048, which allows the libiberty patch to 
>> pass.  But do you have a feel for how much is a realistic limit ?
> 
> For recursion limit I think that is fine.
> For just stack size limit, I think it is extremely small.
> I see that in the function it allocates on 64-bit 24 bytes times
> num_comps using alloca, so 48 bytes per character in the mangled name,
> and a pointer for each character in the mangled name.
> That is 112KB per 2048 bytes long mangled name.
> 
> Dunno how much stack can we expect to be usable.

Currently the patched libiberty uses the DEMANGLE_RECURSION_LIMIT value
in two ways.  The first is as a limit on the number of levels of recursion
of specific functions inside the demangler.  The second is as a check on
the number of component structures that will be allocated on the stack.
(See cp-demangle.c:d_demangle_callback).  One of the CVEs that I was checking
was triggering stack exhaustion this way, which is why I added the check.

There is at least one other function where a similar stack allocation
happens (cplus_demangle_print_callback) but I was not sure if this could
be triggered with the other limits in place, and I did not have a reproducer
that touched it, so I left it alone.

Cheers
  Nick

[PATCH] Make test for Filesystem TS actually use the Filesystem TS

[Jonathan Wakely]

This test was copied from 27_io/filesystem/path/query/is_absolute.cc but
should have been modified to test the path type from the TS instead of
std::filesystem::path.

* testsuite/experimental/filesystem/path/query/is_absolute.cc: Fix
test to use TS, not C++17.

Tested x86_64-linux, committed to trunk.

commit 55f8e90ef66e884683a64e0f0819b5e20caab91c
Author: Jonathan Wakely 
Date:   Mon Dec 10 14:22:09 2018 +

Make test for Filesystem TS actually use the Filesystem TS

This test was copied from 27_io/filesystem/path/query/is_absolute.cc but
should have been modified to test the path type from the TS instead of
std::filesystem::path.

* testsuite/experimental/filesystem/path/query/is_absolute.cc: Fix
test to use TS, not C++17.

diff --git 
a/libstdc++-v3/testsuite/experimental/filesystem/path/query/is_absolute.cc 
b/libstdc++-v3/testsuite/experimental/filesystem/path/query/is_absolute.cc
index 974dec4381d..0eb3e7dfc66 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/path/query/is_absolute.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/path/query/is_absolute.cc
@@ -1,5 +1,5 @@
-// { dg-options "-std=gnu++17 -lstdc++fs" }
-// { dg-do run { target c++17 } }
+// { dg-options "-DUSE_FILESYSTEM_TS -lstdc++fs" }
+// { dg-do run { target c++11 } }
 // { dg-require-filesystem-ts "" }
 
 // Copyright (C) 2018 Free Software Foundation, Inc.
@@ -21,15 +21,15 @@
 
 // 8.4.9 path decomposition [path.decompose]
 
-#include 
+#include 
 #include 
 
-using std::filesystem::path;
+using std::experimental::filesystem::path;
 
 void
 test01()
 {
-  #ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
+#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
   const bool is_posix = false;
 #else
   const bool is_posix = true;

Re: [PATCH 01/10] Fix IRA ICE.

[Andrew Stubbs]

On 08/12/2018 16:23, Jeff Law wrote:

On 12/8/18 5:14 AM, Richard Sandiford wrote:

Andrew Stubbs  writes:

On 21/11/2018 00:47, Jeff Law wrote:

This seems like a really gross hack and sets an expectation that
generating registers in the target after IRA has started is OK.  It is
not OK.  THe fact that this works is, IMHO, likely an accident.


What's the proper test for this? Neither lra_in_progress nor
reload_in_progress is set here, and can_create_pseudos returns true.

The patterns have the ability to not generate registers, but they don't
know not to.

Richard Sandiford has stated that it should be OK, but perhaps the other
architectures also work by accident?


Yeah, my understanding was that targets could create new registers here,
and I thought targets did in some situations.  But that's also why I'm
suspicious of the patch.  If GCN is doing something valid and IRA isn't
coping, then the patch seems to be fixing the problem in the wrong place.

IRA/LRA can create new pseudos internally.  What I'm much less sure
about is whether or not the target can create them.  WHen IRA/LRA
creates one internally it has the chance to update the various internal
structures it needs.  That can't happen with a pseudo created by the
target this late.  Vlad would know for sure.


In any case, the new patch series that I will post soon does not appear 
to need this patch any more. (I'll post that as soon as I figure out an 
unrelated problem with jump threading.)


Andrew

Re: [PATCH] Set DEMANGLE_RECURSION_LIMIT to 1536

[David Malcolm]

On Mon, 2018-12-10 at 14:52 +, Michael Matz wrote:
> Hi,
> 
> On Fri, 7 Dec 2018, H.J. Lu wrote:
> 
> > > > On Thu, Dec 6, 2018 at 3:12 AM Nick Clifton 
> > > > wrote:
> > > > > 
> > > > >   Is the patch OK with you ?
> > > 
> > > This caused:
> > > 
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88409
> > > 
> > 
> > Here is the fix.   OK for trunk?
> 
> I think this points toward the limit being _much_ too low.  With
> template 
> meta programming you easily get these mangled names, it's not even a 
> particularly long one.  But I'm wondering a bit, without tracing the 
> demangler, just looking at the symbol name and demangled result I
> don't 
> readily see where the depth of recursion really is more than 1024,
> are 
> there perhaps some recursion_level-- statements skipped?

Apologies in advance if this has been covered, as I've only been half-
watching this thread, but is it always the case that the recursion
depth can be bounded by some scalar multiple of the number of
characters in the name?

The name that's causing trouble is 654 characters long, and the
proposed limit of 1306 is slightly below double that.  There may well
be a bug in the implementation as Michael points out, but is the
recursion depth always guaranteed to be less than 2 * num_chars seen,
or somesuch limit.  If so, could the limit be dynamic, rather than
hardcoded?  That would trap cases where we're consuming stack frames
without consuming input characters, and eliminate having a hardcoded
limit that's bound to eventually become wrong as people write more and
more complicated C++ programs.

Hope this is constructive
Dave

Re: [PATCH] Set DEMANGLE_RECURSION_LIMIT to 1536

[Jakub Jelinek]

On Mon, Dec 10, 2018 at 03:12:53PM +, Nick Clifton wrote:
> Hi Michael,
> 
> > I think this points toward the limit being _much_ too low.
> 
> Fair enough - several other people have said this as well.  So
> I have proposed an alternative patch instead.  My current suggestion
> is to raise the limit to 2048, which allows the libiberty patch to 
> pass.  But do you have a feel for how much is a realistic limit ?

For recursion limit I think that is fine.
For just stack size limit, I think it is extremely small.
I see that in the function it allocates on 64-bit 24 bytes times
num_comps using alloca, so 48 bytes per character in the mangled name,
and a pointer for each character in the mangled name.
That is 112KB per 2048 bytes long mangled name.

Dunno how much stack can we expect to be usable.

Jakub

Re: [PATCH] Set DEMANGLE_RECURSION_LIMIT to 1536

[Nick Clifton]

Hi Michael,

> I think this points toward the limit being _much_ too low.

Fair enough - several other people have said this as well.  So
I have proposed an alternative patch instead.  My current suggestion
is to raise the limit to 2048, which allows the libiberty patch to 
pass.  But do you have a feel for how much is a realistic limit ?


> demangler, just looking at the symbol name and demangled result I don't 
> readily see where the depth of recursion really is more than 1024, are 
> there perhaps some recursion_level-- statements skipped?

I do not think so.  Unless there are some long jumps in the demangling code ?
I did a quick scan and did not find any, but I could have missed something.
Plus of course I cannot guarantee that my patch is bug free, although looking
at it again I do not see where I missed a level decrement.  

I think that the demangling code just is really recursive...

Cheers
  Nick

Re: [PATCH] Set DEMANGLE_RECURSION_LIMIT to 1536

[Jakub Jelinek]

On Mon, Dec 10, 2018 at 02:52:39PM +, Michael Matz wrote:
> On Fri, 7 Dec 2018, H.J. Lu wrote:
> 
> > > > On Thu, Dec 6, 2018 at 3:12 AM Nick Clifton  wrote:
> > > > >
> > > > >   Is the patch OK with you ?
> > > >
> > >
> > > This caused:
> > >
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88409
> > >
> > 
> > Here is the fix.   OK for trunk?
> 
> I think this points toward the limit being _much_ too low.  With template 
> meta programming you easily get these mangled names, it's not even a 
> particularly long one.  But I'm wondering a bit, without tracing the 
> demangler, just looking at the symbol name and demangled result I don't 
> readily see where the depth of recursion really is more than 1024, are 
> there perhaps some recursion_level-- statements skipped?

That is because the recursion_level limit isn't hit in this case at all (far
from it).

What breaks it is this:

  /* PR 87675 - Check for a mangled string that is so long
 that we do not have enough stack space to demangle it.  */
  if (((options & DMGL_NO_RECURSE_LIMIT) == 0)
  /* This check is a bit arbitrary, since what we really want to do is to
 compare the sizes of the di.comps and di.subs arrays against the
 amount of stack space remaining.  But there is no portable way to do
 this, so instead we use the recursion limit as a guide to the maximum
 size of the arrays.  */
  && (unsigned long) di.num_comps > DEMANGLE_RECURSION_LIMIT)
{
  /* FIXME: We need a way to indicate that a stack limit has been reached.  
*/
  return 0;
}

where di.num_comps is just strlen (mangled) * 2.  Without any analysis
whatsoever, bumping the "recursion" limit will just mean we can process 1.5
times long names.  Either we need more precise analysis on what we are
looking for (how big arrays we'll need) or it needs to be an independent
limit and certainly should allow say 10KB symbols too if they are
reasonable.

Jakub

Re: [PATCH] Set DEMANGLE_RECURSION_LIMIT to 1536

[Michael Matz]

Hi,

On Fri, 7 Dec 2018, H.J. Lu wrote:

> > > On Thu, Dec 6, 2018 at 3:12 AM Nick Clifton  wrote:
> > > >
> > > >   Is the patch OK with you ?
> > >
> >
> > This caused:
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88409
> >
> 
> Here is the fix.   OK for trunk?

I think this points toward the limit being _much_ too low.  With template 
meta programming you easily get these mangled names, it's not even a 
particularly long one.  But I'm wondering a bit, without tracing the 
demangler, just looking at the symbol name and demangled result I don't 
readily see where the depth of recursion really is more than 1024, are 
there perhaps some recursion_level-- statements skipped?


Ciao,
Michael.

Re: [PATCH] Improve overhead of tree-affine.c a bit

[Richard Biener]

On Mon, 10 Dec 2018, Kyrill Tkachov wrote:

> Hi Richard,
> 
> On 10/12/18 13:44, Richard Biener wrote:
> > 
> > When working on PR63184 I noticed that tree-affine.c is still one of
> > the gimple_assign_rhs_to_tree callers and does so quite aggressively
> > for no good reason.  The following restricts it to the cases we
> > handle in tree_to_aff_combination, axing one unreachable case.
> > It also makes sure to cache expanded combinations with SSA names
> > as keys rather than a GENERIC conversion tree which isn't shared
> > anyways.
> > 
> > Bootstrapped and tested on x86_64-unknown-linux-gnu, object files
> > in gcc/ are the same patched/unpatched with the exception of
> > tree-affine.o.
> > 
> > Applied to trunk.
> > 
> > Richard.
> > 
> > 2018-12-10  Richard Biener  
> > 
> > * tree-affine.c (tree_to_aff_combination): Remove unreachable
> > MEM_REF case.
> > (aff_combination_expand): Cache on SSA names, not possibly
> > on conversion trees.  Avoid expanding cases we do not handle.
> > 
> > diff --git a/gcc/tree-affine.c b/gcc/tree-affine.c
> > index 96b479dfc4c..41c52ab468d 100644
> > --- a/gcc/tree-affine.c
> > +++ b/gcc/tree-affine.c
> > @@ -351,6 +351,7 @@ tree_to_aff_combination (tree expr, tree type, aff_tree
> > *comb)
> >return;
> > 
> >  case MEM_REF:
> > +  gcc_unreachable ();
> >if (TREE_CODE (TREE_OPERAND (expr, 0)) == ADDR_EXPR)
> >  tree_to_aff_combination (TREE_OPERAND (TREE_OPERAND (expr, 0), 0),
> >   type, comb);
> 
> The code after the gcc_unreachable is now dead and can be removed?

Yes, sorry - that is what I committed, the above is what I tested.

Richard.

> Thanks,
> Kyrill
> 
> > @@ -721,21 +722,39 @@ aff_combination_expand (aff_tree *comb
> > ATTRIBUTE_UNUSED,
> >if (TREE_CODE_CLASS (code) == tcc_reference)
> >  continue;
> > 
> > -  if (!*cache)
> > -   *cache = new hash_map;
> > -  name_expansion **slot = &(*cache)->get_or_insert (e);
> > -  exp = *slot;
> > -
> > +  name_expansion **slot = NULL;
> > +  if (*cache)
> > +   slot = (*cache)->get (name);
> > +  exp = slot ? *slot : NULL;
> >if (!exp)
> >  {
> > + /* Only bother to handle cases tree_to_aff_combination will.  */
> > + switch (code)
> > +   {
> > +   case POINTER_PLUS_EXPR:
> > +   case PLUS_EXPR:
> > +   case MINUS_EXPR:
> > +   case MULT_EXPR:
> > +   case NEGATE_EXPR:
> > +   case BIT_NOT_EXPR:
> > +   CASE_CONVERT:
> > + rhs = gimple_assign_rhs_to_tree (def);
> > + break;
> > +   case ADDR_EXPR:
> > +   case INTEGER_CST:
> > +   case POLY_INT_CST:
> > + rhs = gimple_assign_rhs1 (def);
> > + break;
> > +   default:
> > + continue;
> > +   }
> > + tree_to_aff_combination (rhs, TREE_TYPE (name), );
> >exp = XNEW (struct name_expansion);
> >exp->in_progress = 1;
> > - *slot = exp;
> > - rhs = gimple_assign_rhs_to_tree (def);
> > - if (e != name)
> > -   rhs = fold_convert (type, rhs);
> > -
> > - tree_to_aff_combination_expand (rhs, comb->type, , cache);
> > + if (!*cache)
> > +   *cache = new hash_map;
> > + (*cache)->put (name, exp);
> > + aff_combination_expand (, cache);
> >exp->expansion = current;
> >exp->in_progress = 0;
> >  }
> > @@ -746,6 +765,8 @@ aff_combination_expand (aff_tree *comb ATTRIBUTE_UNUSED,
> >gcc_assert (!exp->in_progress);
> >current = exp->expansion;
> >  }
> > +  if (!useless_type_conversion_p (comb->type, current.type))
> > +   aff_combination_convert (, comb->type);
> > 
> >/* Accumulate the new terms to TO_ADD, so that we do not modify
> >   COMB while traversing it; include the term -coef * E, to remove
> 
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)

Re: [PATCH, Fortran] pad char to int conversions with spaces instead of zeros (legacy)

[Mark Eggleston]

On 06/12/2018 10:20, Mark Eggleston wrote:

Yes. Mark, you'll need to also patch iresolve.c (gfc_resolve_transfer)
to affect non-constant resolution.

Thanks for the hint.


I've looked at gfc_resolve_transfer regarding handling of padding when a 
character variable is passed to transfer instead of a literal. This 
routine is not called so can't be where a variable would be handled.


Don't yet know where to make the change.

regards,

Mark

--
https://www.codethink.co.uk/privacy.html

Re: [PATCH] Improve overhead of tree-affine.c a bit

[Kyrill Tkachov]

Hi Richard,

On 10/12/18 13:44, Richard Biener wrote:


When working on PR63184 I noticed that tree-affine.c is still one of
the gimple_assign_rhs_to_tree callers and does so quite aggressively
for no good reason.  The following restricts it to the cases we
handle in tree_to_aff_combination, axing one unreachable case.
It also makes sure to cache expanded combinations with SSA names
as keys rather than a GENERIC conversion tree which isn't shared
anyways.

Bootstrapped and tested on x86_64-unknown-linux-gnu, object files
in gcc/ are the same patched/unpatched with the exception of
tree-affine.o.

Applied to trunk.

Richard.

2018-12-10  Richard Biener  

* tree-affine.c (tree_to_aff_combination): Remove unreachable
MEM_REF case.
(aff_combination_expand): Cache on SSA names, not possibly
on conversion trees.  Avoid expanding cases we do not handle.

diff --git a/gcc/tree-affine.c b/gcc/tree-affine.c
index 96b479dfc4c..41c52ab468d 100644
--- a/gcc/tree-affine.c
+++ b/gcc/tree-affine.c
@@ -351,6 +351,7 @@ tree_to_aff_combination (tree expr, tree type, aff_tree 
*comb)
   return;

 case MEM_REF:
+  gcc_unreachable ();
   if (TREE_CODE (TREE_OPERAND (expr, 0)) == ADDR_EXPR)
 tree_to_aff_combination (TREE_OPERAND (TREE_OPERAND (expr, 0), 0),
  type, comb);


The code after the gcc_unreachable is now dead and can be removed?

Thanks,
Kyrill


@@ -721,21 +722,39 @@ aff_combination_expand (aff_tree *comb ATTRIBUTE_UNUSED,
   if (TREE_CODE_CLASS (code) == tcc_reference)
 continue;

-  if (!*cache)
-   *cache = new hash_map;
-  name_expansion **slot = &(*cache)->get_or_insert (e);
-  exp = *slot;
-
+  name_expansion **slot = NULL;
+  if (*cache)
+   slot = (*cache)->get (name);
+  exp = slot ? *slot : NULL;
   if (!exp)
 {
+ /* Only bother to handle cases tree_to_aff_combination will.  */
+ switch (code)
+   {
+   case POINTER_PLUS_EXPR:
+   case PLUS_EXPR:
+   case MINUS_EXPR:
+   case MULT_EXPR:
+   case NEGATE_EXPR:
+   case BIT_NOT_EXPR:
+   CASE_CONVERT:
+ rhs = gimple_assign_rhs_to_tree (def);
+ break;
+   case ADDR_EXPR:
+   case INTEGER_CST:
+   case POLY_INT_CST:
+ rhs = gimple_assign_rhs1 (def);
+ break;
+   default:
+ continue;
+   }
+ tree_to_aff_combination (rhs, TREE_TYPE (name), );
   exp = XNEW (struct name_expansion);
   exp->in_progress = 1;
- *slot = exp;
- rhs = gimple_assign_rhs_to_tree (def);
- if (e != name)
-   rhs = fold_convert (type, rhs);
-
- tree_to_aff_combination_expand (rhs, comb->type, , cache);
+ if (!*cache)
+   *cache = new hash_map;
+ (*cache)->put (name, exp);
+ aff_combination_expand (, cache);
   exp->expansion = current;
   exp->in_progress = 0;
 }
@@ -746,6 +765,8 @@ aff_combination_expand (aff_tree *comb ATTRIBUTE_UNUSED,
   gcc_assert (!exp->in_progress);
   current = exp->expansion;
 }
+  if (!useless_type_conversion_p (comb->type, current.type))
+   aff_combination_convert (, comb->type);

   /* Accumulate the new terms to TO_ADD, so that we do not modify
  COMB while traversing it; include the term -coef * E, to remove

[PATCH] Improve overhead of tree-affine.c a bit

[Richard Biener]

When working on PR63184 I noticed that tree-affine.c is still one of
the gimple_assign_rhs_to_tree callers and does so quite aggressively
for no good reason.  The following restricts it to the cases we
handle in tree_to_aff_combination, axing one unreachable case.
It also makes sure to cache expanded combinations with SSA names
as keys rather than a GENERIC conversion tree which isn't shared
anyways.

Bootstrapped and tested on x86_64-unknown-linux-gnu, object files
in gcc/ are the same patched/unpatched with the exception of
tree-affine.o.

Applied to trunk.

Richard.

2018-12-10  Richard Biener  

* tree-affine.c (tree_to_aff_combination): Remove unreachable
MEM_REF case.
(aff_combination_expand): Cache on SSA names, not possibly
on conversion trees.  Avoid expanding cases we do not handle.

diff --git a/gcc/tree-affine.c b/gcc/tree-affine.c
index 96b479dfc4c..41c52ab468d 100644
--- a/gcc/tree-affine.c
+++ b/gcc/tree-affine.c
@@ -351,6 +351,7 @@ tree_to_aff_combination (tree expr, tree type, aff_tree 
*comb)
   return;
 
 case MEM_REF:
+  gcc_unreachable ();
   if (TREE_CODE (TREE_OPERAND (expr, 0)) == ADDR_EXPR)
tree_to_aff_combination (TREE_OPERAND (TREE_OPERAND (expr, 0), 0),
 type, comb);
@@ -721,21 +722,39 @@ aff_combination_expand (aff_tree *comb ATTRIBUTE_UNUSED,
   if (TREE_CODE_CLASS (code) == tcc_reference)
continue;
 
-  if (!*cache)
-   *cache = new hash_map;
-  name_expansion **slot = &(*cache)->get_or_insert (e);
-  exp = *slot;
-
+  name_expansion **slot = NULL;
+  if (*cache)
+   slot = (*cache)->get (name);
+  exp = slot ? *slot : NULL;
   if (!exp)
{
+ /* Only bother to handle cases tree_to_aff_combination will.  */
+ switch (code)
+   {
+   case POINTER_PLUS_EXPR:
+   case PLUS_EXPR:
+   case MINUS_EXPR:
+   case MULT_EXPR:
+   case NEGATE_EXPR:
+   case BIT_NOT_EXPR:
+   CASE_CONVERT:
+ rhs = gimple_assign_rhs_to_tree (def);
+ break;
+   case ADDR_EXPR:
+   case INTEGER_CST:
+   case POLY_INT_CST:
+ rhs = gimple_assign_rhs1 (def);
+ break;
+   default:
+ continue;
+   }
+ tree_to_aff_combination (rhs, TREE_TYPE (name), );
  exp = XNEW (struct name_expansion);
  exp->in_progress = 1;
- *slot = exp;
- rhs = gimple_assign_rhs_to_tree (def);
- if (e != name)
-   rhs = fold_convert (type, rhs);
-
- tree_to_aff_combination_expand (rhs, comb->type, , cache);
+ if (!*cache)
+   *cache = new hash_map;
+ (*cache)->put (name, exp);
+ aff_combination_expand (, cache);
  exp->expansion = current;
  exp->in_progress = 0;
}
@@ -746,6 +765,8 @@ aff_combination_expand (aff_tree *comb ATTRIBUTE_UNUSED,
  gcc_assert (!exp->in_progress);
  current = exp->expansion;
}
+  if (!useless_type_conversion_p (comb->type, current.type))
+   aff_combination_convert (, comb->type);
 
   /* Accumulate the new terms to TO_ADD, so that we do not modify
 COMB while traversing it; include the term -coef * E, to remove

[committed] Fix gcc.target/i386/pr87955.c testcase

[Jakub Jelinek]

Hi!

The testcase doesn't emit expected diagnostics on i?86, because the default
is -mfpmath=387 there and so the function is perfectly inlinable.

The following patch makes sure we compile the testcase with -msse2 -mfpmath=sse
everywhere, so it succeeds (or FAILs with the ipa-inline.c change reverted)
on all x86 configurations the same.

Tested on x86_64-linux, committed to trunk as obvious.

2018-12-10  Jakub Jelinek  

PR ipa/87955
* gcc.target/i386/pr87955.c: Add -msse2 -mfpmath=sse to dg-options.

--- gcc/testsuite/gcc.target/i386/pr87955.c.jj  2018-11-14 00:55:33.844621126 
+0100
+++ gcc/testsuite/gcc.target/i386/pr87955.c 2018-12-10 14:23:07.713905223 
+0100
@@ -1,4 +1,4 @@
-/* { dg-options "-O2 -fopt-info-inline-missed" } */
+/* { dg-options "-O2 -msse2 -mfpmath=sse -fopt-info-inline-missed" } */
 
 float a;
 

Jakub

Re: [PATCH] hppa: Add libphobos support

[John David Anglin]

On 2018-12-09 6:10 p.m., Iain Buclaw wrote:
> On Sun, 9 Dec 2018 at 21:16, John David Anglin  wrote:
>> The attached change implements a first cut at libphobos support on
>> hppa/glibc/linux.  Test
>> results are here:
>> .
>>
>> Okay?
>>
> >From what I can see, everything is properly scoped and looks valid (no
> accidental C-style syntax anywhere).  Looking at the test results,
> seems like nothing runs, which may not be entirely unexpected from a
> new port, but I would have expected a little better.
I see about 10% fails for gdc (27700 passes, 2641 fails).  For
libphobos, I see 242 passes and 46 fails.

I haven't had a chance to debug any of the fails.  The stack grows up on
hppa.  This is one big difference
from all other ports.  I also was a bit unsure about the setjmp
implementation.

> I've just tried
> building a cross-compiler and running some small programs under qemu,
> but it seems that nothing works under emulation, not even C programs.
I added Helge Deller to Cc.  He has worked on qemu for hppa.
> Would you mind if I send this to the upstream druntime first?
> Repository is here https://github.com/dlang/druntime
No problem.  Go ahead.

Dave

-- 
John David Anglin  dave.ang...@bell.net

Re: [PATCH, PPC/Darwin] Fix long double symbol exports.

[Jonathan Wakely]

On 06/12/18 14:36 -0800, Mike Stump wrote:

On Dec 6, 2018, at 11:52 AM, Iain Sandoe  wrote:


During 8.x, the rs6000 target-specific mangling was reorganised which uncovered
a long-standing bug in Darwin’s mangling for ‘IBM’ long double.  Now the symbols
are correctly mangled, and we end up with a bunch of test link fails.

This patch adds the necessary subset of the Linux long double exports to 
Darwin’s
export table.

I have tested this on a few bootstrap/regtest cycles on powerpc-darwin9, and on 
the
power7 linux system.

For the record, I’ve noted the library versions from the Linux side, although 
Darwin
does not version symbols in this way.

OK for trunk and 8.x?


Don't know if the libstdc++ want to review this or they want me to...  I'm fine 
with it.


Me too. OK for trunk and 8.x - thanks.

[PATCH] Fix PR88427

[Richard Biener]

The following restores behavior to before my r266557 fix for symbolic
ranges, avoiding the ICE.

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

Richard.

2018-12-10  Richard Biener  

PR tree-optimization/88427
* vr-values.c (vr_values::extract_range_from_phi_node):
Handle symbolic ranges conservatively when trying to drop
to Inf +- 1.

* gcc.dg/pr88427.c: New testcase.

Index: gcc/vr-values.c
===
--- gcc/vr-values.c (revision 266950)
+++ gcc/vr-values.c (working copy)
@@ -2891,8 +2891,9 @@ vr_values::extract_range_from_phi_node (
   if (cmp_min < 0)
new_min = lhs_vr->min ();
   else if (cmp_min > 0
-  && tree_int_cst_lt (vrp_val_min (vr_result->type ()),
-  vr_result->min ()))
+  && (TREE_CODE (vr_result->min ()) != INTEGER_CST
+  || tree_int_cst_lt (vrp_val_min (vr_result->type ()),
+  vr_result->min (
new_min = int_const_binop (PLUS_EXPR,
   vrp_val_min (vr_result->type ()),
   build_int_cst (vr_result->type (), 1));
@@ -2901,8 +2902,9 @@ vr_values::extract_range_from_phi_node (
   if (cmp_max > 0)
new_max = lhs_vr->max ();
   else if (cmp_max < 0
-  && tree_int_cst_lt (vr_result->max (),
-  vrp_val_max (vr_result->type (
+  && (TREE_CODE (vr_result->max ()) != INTEGER_CST
+  || tree_int_cst_lt (vr_result->max (),
+  vrp_val_max (vr_result->type ()
new_max = int_const_binop (MINUS_EXPR,
   vrp_val_max (vr_result->type ()),
   build_int_cst (vr_result->type (), 1));
Index: gcc/testsuite/gcc.dg/pr88427.c
===
--- gcc/testsuite/gcc.dg/pr88427.c  (nonexistent)
+++ gcc/testsuite/gcc.dg/pr88427.c  (working copy)
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-tree-dce -fno-tree-fre -Wno-div-by-zero" } */
+
+void
+uj (int eq, int s4)
+{
+  short int tm = 0;
+
+  for (;;)
+if (eq == s4)
+  {
+   tm += !!s4;
+   if (tm == s4)
+ {
+   eq += tm;
+   for (;;)
+ eq /= 0;
+ }
+  }
+}

[PATCH] Fix PR88415

[Richard Biener]

The following fixes gimple_assign_set_rhs_with_ops to transfer EH
info so that the stale stmts are not left in the EH tables.

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

Richard.

2018-12-10  Richard Biener  

PR middle-end/88415
* gimple.c (gimple_assign_set_rhs_with_ops): Transfer EH
info to a newly allocated stmt.

* gcc.dg/gomp/pr88415.c: New testcase.

Index: gcc/gimple.c
===
--- gcc/gimple.c(revision 266945)
+++ gcc/gimple.c(working copy)
@@ -1729,16 +1729,15 @@ gimple_assign_set_rhs_with_ops (gimple_s
 {
   unsigned new_rhs_ops = get_gimple_rhs_num_ops (code);
   gimple *stmt = gsi_stmt (*gsi);
+  gimple *old_stmt = stmt;
 
   /* If the new CODE needs more operands, allocate a new statement.  */
   if (gimple_num_ops (stmt) < new_rhs_ops + 1)
 {
-  tree lhs = gimple_assign_lhs (stmt);
-  gimple *new_stmt = gimple_alloc (gimple_code (stmt), new_rhs_ops + 1);
-  memcpy (new_stmt, stmt, gimple_size (gimple_code (stmt)));
-  gimple_init_singleton (new_stmt);
-  gsi_replace (gsi, new_stmt, false);
-  stmt = new_stmt;
+  tree lhs = gimple_assign_lhs (old_stmt);
+  stmt = gimple_alloc (gimple_code (old_stmt), new_rhs_ops + 1);
+  memcpy (stmt, old_stmt, gimple_size (gimple_code (old_stmt)));
+  gimple_init_singleton (stmt);
 
   /* The LHS needs to be reset as this also changes the SSA name
 on the LHS.  */
@@ -1752,6 +1751,8 @@ gimple_assign_set_rhs_with_ops (gimple_s
 gimple_assign_set_rhs2 (stmt, op2);
   if (new_rhs_ops > 2)
 gimple_assign_set_rhs3 (stmt, op3);
+  if (stmt != old_stmt)
+gsi_replace (gsi, stmt, true);
 }
 
 
Index: gcc/testsuite/gcc.dg/gomp/pr88415.c
===
--- gcc/testsuite/gcc.dg/gomp/pr88415.c (nonexistent)
+++ gcc/testsuite/gcc.dg/gomp/pr88415.c (working copy)
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-fexceptions -fnon-call-exceptions -fopenmp -fsignaling-nans 
-funsafe-math-optimizations -fno-associative-math" } */
+
+void
+lx (_Complex int *yn)
+{
+  int mj;
+
+#pragma omp for
+  for (mj = 0; mj < 1; ++mj)
+yn[mj] += 1;
+}

Re: [PATCH] Fix *ivdep* tests on SPARC etc. (PR testsuite/88369, take 2)

[Rainer Orth]

Hi Jakub,

> As mentioned in the PR, another testcase was missed, this patch modifies
> that one too.  Ok for trunk?
>
> 2018-12-10  Jakub Jelinek  
>
>   PR testsuite/88369
>   * gcc.dg/vect/vect-ivdep-1.c: Prune versioning for alignment messages.
>   * gcc.dg/vect/vect-ivdep-2.c: Likewise.
>   * gcc.dg/vect/nodump-vect-opt-info-1.c: Likewise.
>   * g++.dg/vect/pr33426-ivdep.cc: Likewise.
>   * g++.dg/vect/pr33426-ivdep-2.cc: Likewise. 
>   * g++.dg/vect/pr33426-ivdep-3.cc: Likewise.
>   * g++.dg/vect/pr33426-ivdep-4.cc: Likewise.

> --- gcc/testsuite/gcc.dg/vect/nodump-vect-opt-info-1.c.jj 2018-09-29 
> 18:03:11.562727541 +0200
> +++ gcc/testsuite/gcc.dg/vect/nodump-vect-opt-info-1.c2018-12-10 
> 12:57:56.714484728 +0100
> @@ -6,6 +6,7 @@ vadd (int *dst, int *op1, int *op2, int
>  {
>  /* { dg-optimized "loop vectorized" "" { target *-*-* } .+2 } */
>  /* { dg-optimized "loop versioned for vectorization because of possible 
> aliasing" "" { target *-*-* } .+1 } */
> +/* { dg-prune-output " version\[^\n\r]* alignment" } */
>for (int i = 0; i < count; ++i)
>  dst[i] = op1[i] + op2[i];
>  }

this is wrong: either move the dg-prune-output before the first
dg-optimized (preferably) or adjust the line number distances to account
for the additional line.

Ok with that fixed.

Thanks.
Rainer

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

[PATCH, libphobos] Committed fix modify immutable error on Solaris

[Iain Buclaw]

Hi,

This is another Solaris backport from druntime 2.079, fixing a build
error in the core.thread module.

Bootstrapped and tested on i386-pc-solaris2.11.

Committed to trunk as r266950.

-- 
Iain
---
diff --git a/libphobos/libdruntime/core/thread.d b/libphobos/libdruntime/core/thread.d
index ff15d066a49..98a81425f47 100644
--- a/libphobos/libdruntime/core/thread.d
+++ b/libphobos/libdruntime/core/thread.d
@@ -1547,7 +1547,7 @@ private:
 
 version (Solaris)
 {
-__gshared immutable bool m_isRTClass;
+__gshared bool m_isRTClass;
 }
 
 private:

[PATCH] Fix *ivdep* tests on SPARC etc. (PR testsuite/88369, take 2)

[Jakub Jelinek]

On Thu, Dec 06, 2018 at 08:07:30AM +0100, Jakub Jelinek wrote:
> On (at least some of these) tests and on some targets, GCC prints messages
> like:
> ./cc1 -quiet -O3 -fopt-info-vec-optimized -mvis vect-ivdep-1.c
> vect-ivdep-1.c:11:3: optimized: loop vectorized using 8 byte vectors
> vect-ivdep-1.c:11:3: optimized:  loop versioned for vectorization to enhance 
> alignment
> and the versioning for alignment message according to this PR makes those
> tests FAIL.  We just want to make sure in these testcases we don't version
> for alias, so this patch just prunes the versioning for alignment
> diagnostics.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, does it work on SPARC
> and ok for trunk in that case?

As mentioned in the PR, another testcase was missed, this patch modifies
that one too.  Ok for trunk?

2018-12-10  Jakub Jelinek  

PR testsuite/88369
* gcc.dg/vect/vect-ivdep-1.c: Prune versioning for alignment messages.
* gcc.dg/vect/vect-ivdep-2.c: Likewise.
* gcc.dg/vect/nodump-vect-opt-info-1.c: Likewise.
* g++.dg/vect/pr33426-ivdep.cc: Likewise.
* g++.dg/vect/pr33426-ivdep-2.cc: Likewise. 
* g++.dg/vect/pr33426-ivdep-3.cc: Likewise.
* g++.dg/vect/pr33426-ivdep-4.cc: Likewise.

--- gcc/testsuite/gcc.dg/vect/vect-ivdep-1.c.jj 2015-05-29 15:04:27.894882932 
+0200
+++ gcc/testsuite/gcc.dg/vect/vect-ivdep-1.c2018-12-05 17:37:54.032485003 
+0100
@@ -15,3 +15,4 @@ void foo(int n, int *a, int *b, int *c,
 
 /* { dg-message "loop vectorized" "" { target *-*-* } 0 } */
 /* { dg-bogus " version\[^\n\r]* alias" "" { target *-*-* } 0 } */
+/* { dg-prune-output " version\[^\n\r]* alignment" } */
--- gcc/testsuite/gcc.dg/vect/vect-ivdep-2.c.jj 2015-05-29 15:04:27.908882716 
+0200
+++ gcc/testsuite/gcc.dg/vect/vect-ivdep-2.c2018-12-05 17:38:00.528377814 
+0100
@@ -31,3 +31,4 @@ void bar(int n, int *a, int *b, int *c)
 
 /* { dg-message "loop vectorized" "" { target *-*-* } 0 } */
 /* { dg-bogus " version\[^\n\r]* alias" "" { target *-*-* } 0 } */
+/* { dg-prune-output " version\[^\n\r]* alignment" } */
--- gcc/testsuite/gcc.dg/vect/nodump-vect-opt-info-1.c.jj   2018-09-29 
18:03:11.562727541 +0200
+++ gcc/testsuite/gcc.dg/vect/nodump-vect-opt-info-1.c  2018-12-10 
12:57:56.714484728 +0100
@@ -6,6 +6,7 @@ vadd (int *dst, int *op1, int *op2, int
 {
 /* { dg-optimized "loop vectorized" "" { target *-*-* } .+2 } */
 /* { dg-optimized "loop versioned for vectorization because of possible 
aliasing" "" { target *-*-* } .+1 } */
+/* { dg-prune-output " version\[^\n\r]* alignment" } */
   for (int i = 0; i < count; ++i)
 dst[i] = op1[i] + op2[i];
 }
--- gcc/testsuite/g++.dg/vect/pr33426-ivdep.cc.jj   2015-05-29 
15:04:31.748823367 +0200
+++ gcc/testsuite/g++.dg/vect/pr33426-ivdep.cc  2018-12-05 17:35:08.818212056 
+0100
@@ -15,3 +15,4 @@ void foo(int n, int *a, int *b, int *c,
 
 /* { dg-message "loop vectorized" "" { target *-*-* } 0 } */
 /* { dg-bogus " version\[^\n\r]* alias" "" { target *-*-* } 0 } */
+/* { dg-prune-output " version\[^\n\r]* alignment" } */
--- gcc/testsuite/g++.dg/vect/pr33426-ivdep-2.cc.jj 2015-05-29 
15:04:31.739823506 +0200
+++ gcc/testsuite/g++.dg/vect/pr33426-ivdep-2.cc2018-12-05 
17:35:28.772881921 +0100
@@ -30,6 +30,7 @@ void bar(int n, int *a, int *b, int *c)
 
 /* { dg-message "loop vectorized" "" { target *-*-* } 0 } */
 /* { dg-bogus " version\[^\n\r]* alias" "" { target *-*-* } 0 } */
+/* { dg-prune-output " version\[^\n\r]* alignment" } */
 
 /* { dg-final { scan-tree-dump-times "ANNOTATE_EXPR " 2 "original" } } */
 /* { dg-final { scan-tree-dump-times "ANNOTATE " 2 "gimple" } } */
--- gcc/testsuite/g++.dg/vect/pr33426-ivdep-3.cc.jj 2015-05-29 
15:04:31.747823383 +0200
+++ gcc/testsuite/g++.dg/vect/pr33426-ivdep-3.cc2018-12-05 
17:37:10.182208572 +0100
@@ -17,6 +17,7 @@ void foo(int *a) {
 
 /* { dg-message "loop vectorized" "" { target *-*-* } 0 } */
 /* { dg-bogus " version\[^\n\r]* alias" "" { target *-*-* } 0 } */
+/* { dg-prune-output " version\[^\n\r]* alignment" } */
 
 /* { dg-final { scan-tree-dump-times "ANNOTATE_EXPR " 1 "original" } } */
 /* { dg-final { scan-tree-dump-times "ANNOTATE " 1 "gimple" } } */
--- gcc/testsuite/g++.dg/vect/pr33426-ivdep-4.cc.jj 2015-05-29 
15:04:31.740823491 +0200
+++ gcc/testsuite/g++.dg/vect/pr33426-ivdep-4.cc2018-12-05 
17:37:43.715655239 +0100
@@ -22,6 +22,7 @@ void foo(std::vector *ar, int *b) {
 
 /* { dg-message "loop vectorized" "" { target *-*-* } 0 } */
 /* FIXME: dg-bogus " version\[^\n\r]* alias" "" { target *-*-* } 0  */
+/* { dg-prune-output " version\[^\n\r]* alignment" } */
 
 /* { dg-final { scan-tree-dump-times "ANNOTATE_EXPR " 1 "original" } } */
 /* { dg-final { scan-tree-dump-times "ANNOTATE " 1 "gimple" } } */


Jakub

Re: [PATCH] [RFC] PR target/52813 and target/11807

[Richard Sandiford]

Dimitar Dimitrov  writes:
> I have tested this fix on x86_64 host, and found no regression in the C
> and C++ testsuites.  I'm marking this patch as RFC simply because I don't
> have experience with other architectures, and I don't have a setup to
> test all architectures supported by GCC.
>
> gcc/ChangeLog:
>
> 2018-12-07  Dimitar Dimitrov  
>
>   * cfgexpand.c (asm_clobber_reg_is_valid): Also produce
>   error when stack pointer is clobbered.
>   (expand_asm_stmt): Refactor clobber check in separate function.
>
> gcc/testsuite/ChangeLog:
>
> 2018-12-07  Dimitar Dimitrov  
>
>   * gcc.target/i386/pr52813.c: New test.
>
> Signed-off-by: Dimitar Dimitrov 

LGTM.  Do you have a copyright assignment on file?  'Fraid this is
probably big enough to need one.

Thanks,
Richard

[testsuite] Small tweak for Visium

[Eric Botcazou]

Tested on visium-elf and x86_64-suse-linux, applied on mainline and 8 branch.


2018-12-10  Eric Botcazou  

* c-c++-common/patchable_function_entry-decl.c: Pass -mcpu=gr6 for
Visium and remove other specific handling.
* c-c++-common/patchable_function_entry-default.c: Likewise.
* c-c++-common/patchable_function_entry-definition.c: Likewise.

-- 
Eric BotcazouIndex: c-c++-common/patchable_function_entry-decl.c
===
--- c-c++-common/patchable_function_entry-decl.c	(revision 266906)
+++ c-c++-common/patchable_function_entry-decl.c	(working copy)
@@ -1,8 +1,8 @@
 /* { dg-do compile { target { ! nvptx*-*-* } } } */
 /* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
-/* { dg-final { scan-assembler-times "nop|NOP" 2 { target { ! { alpha*-*-* visium-*-* } } } } } */
+/* { dg-additional-options "-mcpu=gr6" { target visium-*-* } }
+/* { dg-final { scan-assembler-times "nop|NOP" 2 { target { ! { alpha*-*-* } } } } } */
 /* { dg-final { scan-assembler-times "bis" 2 { target alpha*-*-* } } } */
-/* { dg-final { scan-assembler-times "nop" 3 { target visium-*-* } } } */
 
 extern int a;
 
Index: c-c++-common/patchable_function_entry-default.c
===
--- c-c++-common/patchable_function_entry-default.c	(revision 266906)
+++ c-c++-common/patchable_function_entry-default.c	(working copy)
@@ -1,8 +1,8 @@
 /* { dg-do compile { target { ! nvptx*-*-* } } } */
 /* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
-/* { dg-final { scan-assembler-times "nop|NOP" 3 { target { ! { alpha*-*-* visium-*-* } } } } } */
+/* { dg-additional-options "-mcpu=gr6" { target visium-*-* } }
+/* { dg-final { scan-assembler-times "nop|NOP" 3 { target { ! { alpha*-*-* } } } } } */
 /* { dg-final { scan-assembler-times "bis" 3 { target alpha*-*-* } } } */
-/* { dg-final { scan-assembler-times "nop" 4 { target visium-*-* } } } */
 
 extern int a;
 
Index: c-c++-common/patchable_function_entry-definition.c
===
--- c-c++-common/patchable_function_entry-definition.c	(revision 266906)
+++ c-c++-common/patchable_function_entry-definition.c	(working copy)
@@ -1,8 +1,8 @@
 /* { dg-do compile { target { ! nvptx*-*-* } } } */
 /* { dg-options "-O2 -fpatchable-function-entry=3,1" } */
-/* { dg-final { scan-assembler-times "nop|NOP" 1 { target { ! { alpha*-*-* visium-*-* } } } } } */
+/* { dg-additional-options "-mcpu=gr6" { target visium-*-* } }
+/* { dg-final { scan-assembler-times "nop|NOP" 1 { target { ! { alpha*-*-* } } } } } */
 /* { dg-final { scan-assembler-times "bis" 1 { target alpha*-*-* } } } */
-/* { dg-final { scan-assembler-times "nop" 2 { target visium-*-* } } } */
 
 extern int a;
 

Add missing def of TARGET_VXWORKS_HAVE_CTORS_DTORS for VxWorksAE

[Olivier Hainque]

Hello,

The attached patch fixes an oversight from 

  https://gcc.gnu.org/ml/gcc-patches/2018-11/msg01432.html

where a new VxWorks ports control macro was introduced,
and the 

* config/vxworksae.h: Also define TARGET_VXWORKS_HAVE_CTORS_DTORS.

part was missing from what was checked in.

Committing to mainline, which fixes build failures for
--target=powerpc-wrs-vxworksae.


Olivier



ctors-653.diff
Description: Binary data

Re: [PR 88214] Check that an argument is pointer before attempting agg jf construction from it

[Richard Biener]

On Fri, Dec 7, 2018 at 3:59 PM Martin Jambor  wrote:
>
> Hi,
>
> ICE in PR 88214 happens because a type-mismatch in K C code makes
> IPA-CP analysis call ao_ref_init_from_ptr_and_size on an integer
> SSA_NAME, this function in turn constructs a temporary MEM_REF based on
> that integer SSA_NAME and then later on call_may_clobber_ref_p_1 treats
> the MEM_REF base as a pointer, gets its SSA_NAME_PTR_INFO and tries to
> work with bitmaps there.  But because the SSA_NAME is an integer, there
> is no SSA_NAME_PTR_INFO, there is range info instead and this leads to a
> crash.
>
> On a related note, would people object to adding the following assert,
> which would have made this bug much more straightforward to find?

That's fine with me.

> index 85a5de7..66cf2f2 100644
> --- a/gcc/tree-ssa-alias.c
> +++ b/gcc/tree-ssa-alias.c
> @@ -710,6 +710,7 @@ ao_ref_init_from_ptr_and_size (ao_ref *ref, tree ptr, 
> tree size)
>  }
>else
>  {
> +  gcc_assert (POINTER_TYPE_P (TREE_TYPE (ptr)));
>ref->base = build2 (MEM_REF, char_type_node,
>   ptr, null_pointer_node);
>ref->offset = 0;
>
>
> The bug itself can be fixed with the patch below.  I have verified it
> avoids the ICE on powerpc64-linux and did a full bootstrap and test on
> an x86_64-linux.  The patch is simple enough that I believe that is good
> enough.

OK.

Richard.

>
> 2018-12-06  Martin Jambor  
>
> PR ipa/88214
> * ipa-prop.c (determine_locally_known_aggregate_parts): Make sure
> we check pointers against pointers.
>
> testsuite/
> * gcc.dg/ipa/pr88214.c: New test.
> ---
>  gcc/ipa-prop.c |  3 ++-
>  gcc/testsuite/gcc.dg/ipa/pr88214.c | 10 ++
>  2 files changed, 12 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.dg/ipa/pr88214.c
>
> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> index 74052350ac1..4dbe26829e3 100644
> --- a/gcc/ipa-prop.c
> +++ b/gcc/ipa-prop.c
> @@ -1569,7 +1569,8 @@ determine_locally_known_aggregate_parts (gcall *call, 
> tree arg,
>if (TREE_CODE (arg) == SSA_NAME)
> {
>   tree type_size;
> -  if (!tree_fits_uhwi_p (TYPE_SIZE (TREE_TYPE (arg_type
> +  if (!tree_fits_uhwi_p (TYPE_SIZE (TREE_TYPE (arg_type)))
> + || !POINTER_TYPE_P (TREE_TYPE (arg)))
>  return;
>   check_ref = true;
>   arg_base = arg;
> diff --git a/gcc/testsuite/gcc.dg/ipa/pr88214.c 
> b/gcc/testsuite/gcc.dg/ipa/pr88214.c
> new file mode 100644
> index 000..4daa9829e75
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/ipa/pr88214.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +void i();
> +  short a;
> +  void b(e) char * e;
> +  {
> +i();
> +b(a);
> +  }
> --
> 2.19.1
>
>
>

[C++ Patch] Add location_t parameter to grokvardecl

[Paolo Carlini]

Hi,

the other day I noticed that we weren't getting right the first location 
of pr53037-4.C, for a variable, whereas the next one, for a function, 
was Ok. Indeed, we were passing a location only to grokfndecl. In other 
terms, I found a good empirical reason to move the declaration of the 
local loc = declarator ? declarator->id_loc : input_location further up 
;) Tested x86_64-linux.


Thanks, Paolo.

//

/cp
2018-12-10  Paolo Carlini  

* decl2.c (grokvardecl): Add location_t parameter and use it
in build_lang_decl_loc and build_decl calls.
(grokdeclarator): Move up loc declaration and use it in the
grokvardecl call too.

/testsuite
2018-12-10  Paolo Carlini  

* g++.dg/pr53037-4.C: Test the first two locations too.
Index: cp/decl.c
===
--- cp/decl.c   (revision 266943)
+++ cp/decl.c   (working copy)
@@ -68,7 +68,7 @@ static int decl_jump_unsafe (tree);
 static void require_complete_types_for_parms (tree);
 static tree grok_reference_init (tree, tree, tree, int);
 static tree grokvardecl (tree, tree, tree, const cp_decl_specifier_seq *,
-int, int, int, bool, int, tree);
+int, int, int, bool, int, tree, location_t);
 static void check_static_variable_definition (tree, tree);
 static void record_unknown_type (tree, const char *);
 static tree builtin_function_1 (tree, tree, bool);
@@ -9282,7 +9282,8 @@ grokvardecl (tree type,
 int inlinep,
 bool conceptp,
 int template_count,
-tree scope)
+tree scope,
+location_t location)
 {
   tree decl;
   tree explicit_scope;
@@ -9318,9 +9319,9 @@ grokvardecl (tree type,
  /* Similarly for explicit specializations.  */
  || (orig_declarator
  && TREE_CODE (orig_declarator) == TEMPLATE_ID_EXPR)))
-decl = build_lang_decl (VAR_DECL, name, type);
+decl = build_lang_decl_loc (location, VAR_DECL, name, type);
   else
-decl = build_decl (input_location, VAR_DECL, name, type);
+decl = build_decl (location, VAR_DECL, name, type);
 
   if (explicit_scope && TREE_CODE (explicit_scope) == NAMESPACE_DECL)
 set_decl_namespace (decl, explicit_scope, 0);
@@ -12200,6 +12201,7 @@ grokdeclarator (const cp_declarator *declarator,
 
   {
 tree decl = NULL_TREE;
+location_t loc = declarator ? declarator->id_loc : input_location;
 
 if (decl_context == PARM)
   {
@@ -12216,13 +12218,13 @@ grokdeclarator (const cp_declarator *declarator,
if (!staticp && !friendp && TREE_CODE (type) != METHOD_TYPE)
  if (tree auto_node = type_uses_auto (type))
{
- location_t loc = declspecs->locations[ds_type_spec];
+ location_t tloc = declspecs->locations[ds_type_spec];
  if (CLASS_PLACEHOLDER_TEMPLATE (auto_node))
-   error_at (loc, "invalid use of template-name %qE without an "
+   error_at (tloc, "invalid use of template-name %qE without an "
  "argument list",
  CLASS_PLACEHOLDER_TEMPLATE (auto_node));
  else
-   error_at (loc, "non-static data member declared with "
+   error_at (tloc, "non-static data member declared with "
  "placeholder %qT", auto_node);
  type = error_mark_node;
}
@@ -12487,7 +12489,6 @@ grokdeclarator (const cp_declarator *declarator,
 
if (decl == NULL_TREE)
  {
-   location_t loc = declarator ? declarator->id_loc : input_location;
if (staticp)
  {
/* C++ allows static class members.  All other work
@@ -12704,7 +12705,8 @@ grokdeclarator (const cp_declarator *declarator,
inlinep,
concept_p,
template_count,
-   ctype ? ctype : in_namespace);
+   ctype ? ctype : in_namespace,
+   loc);
if (decl == NULL_TREE)
  return error_mark_node;
 
Index: testsuite/g++.dg/pr53037-4.C
===
--- testsuite/g++.dg/pr53037-4.C(revision 266943)
+++ testsuite/g++.dg/pr53037-4.C(working copy)
@@ -2,11 +2,11 @@
 /* { dg-do compile } */
 /* { dg-options "-O0" } */
 
-int foo1 __attribute__((warn_if_not_aligned(8))); /* { dg-error 
"'warn_if_not_aligned' may not be specified for 'foo1'" } */
+int foo1 __attribute__((warn_if_not_aligned(8))); /* { dg-error 
"5:'warn_if_not_aligned' may not be specified for 'foo1'" } */
 
 __attribute__((warn_if_not_aligned(8)))
 void
-foo2 (void) /* { dg-error "'warn_if_not_aligned' may not be specified for 
'void foo2\\(\\)'" } */
+foo2 (void) /* { dg-error "1:'warn_if_not_aligned' may not be specified for 
'void foo2\\(\\)'" } */
 {
 }
 

Re: [PATCH 6/6, OpenACC, libgomp] Async re-work, nvptx changes

[Chung-Lin Tang]

On 2018/12/7 04:57 AM, Thomas Schwinge wrote>> --- 
a/libgomp/plugin/plugin-nvptx.c

+++ b/libgomp/plugin/plugin-nvptx.c



+struct goacc_asyncqueue *
+GOMP_OFFLOAD_openacc_async_construct (void)
+{
+  struct goacc_asyncqueue *aq
+= GOMP_PLUGIN_malloc (sizeof (struct goacc_asyncqueue));
+  aq->cuda_stream = NULL;
+  CUDA_CALL_ASSERT (cuStreamCreate, >cuda_stream, CU_STREAM_DEFAULT);


Curiously (this was the same in the code before): does this have to be
"CU_STREAM_DEFAULT" instead of "CU_STREAM_NON_BLOCKING", because we want
to block anything from running in parallel with "acc_async_sync" GPU
kernels, that use the "NULL" stream?  (Not asking you to change this now,
but I wonder if this is overly strict?)


IIUC, this non-blocking only pertains to the "Legacy Default Stream" in 
CUDA, which we're pretty much ignoring; we should be using the newer 
per-thread default stream model. We could review this issue later.



+  if (aq->cuda_stream == NULL)
+GOMP_PLUGIN_fatal ("CUDA stream create NULL\n");


Can this actually happen, given the "CUDA_CALL_ASSERT" usage above?


Hmm, yeah I think this is superfluous too...


+  CUDA_CALL_ASSERT (cuStreamSynchronize, aq->cuda_stream);


Why is the synchronization needed here?


I don't remember, could likely be something added during debug.
I'll remove this and test if things are okay.

Thanks,
Chung-Lin

Don't try to use libgcc-unwind.map with --disable-shared (PR bootstrap/65725)

[Rainer Orth]

When configuring with --disable-shared, Solaris bootstrap with the
system ld fails since libgcc-unwind.map is missing.  This is no wonder,
given that the file is only generated with --enable-shared.  The
following patch, from the PR, fixes this.

Bootstrapped on amd64-pc-solaris2.11, installed on mainline.

There were many unrelated issues, usually occuring on Linux as well as
Solaris, for a --disable-shared configuration, which I'll either fix or
report separately.

Rainer

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


2018-12-09  Fredrik Nyström  

PR bootstrap/65725
* config/sol2.h: Only use libgcc-unwind.map if
ENABLE_SHARED_LIBGCC.

# HG changeset patch
# Parent  aefe43ddbbcf7dca119fa6cb2121526461337953
Don't try to use libgcc-unwind.map with --disable-shared (PR bootstrap/65725)

diff --git a/gcc/config/sol2.h b/gcc/config/sol2.h
--- a/gcc/config/sol2.h
+++ b/gcc/config/sol2.h
@@ -397,7 +397,7 @@ along with GCC; see the file COPYING3.  
 #define SYSROOT_SPEC "-z sysroot=%R"
 #endif
 
-#ifndef USE_GLD
+#if !defined(USE_GLD) && defined(ENABLE_SHARED_LIBGCC)
 /* With Sun ld, use mapfile to enforce direct binding to libgcc_s unwinder.  */
 #define LINK_LIBGCC_MAPFILE_SPEC \
   "%{shared|shared-libgcc:-M %slibgcc-unwind.map}"

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

[Chung-Lin Tang]

On 2018/12/6 2:16 AM, Maciej W. Rozycki wrote:

AFAIK, things like strlen are already available in iso_c_binding, in forms
like "C_strlen".
Can you check again if that 'openacc_c_string' module is really necessary?

  Any pointers please?

  I can't see `c_strlen' or any equivalent interface defined either in the
Fortran 2003 language standard or in GCC documentation, and neither `grep'
over the GCC tree shows anything relevant.  The `iso_c_binding' module
defines only a bunch of procedures according to said documentation.  The
`strlen' function provided here has been taken from one of our Fortran
test cases, which strongly indicates there's no such API already available
or whoever wrote the test case would have chosen to use it I suppose.


Okay I see. I think I mixed up the common convention with the actual interface
standard.


+   CUcontext new_ctx;
+
+   CUDA_CALL_ERET (propval, cuCtxCreate, _ctx, CU_CTX_SCHED_AUTO,
+   dev);
+   CUDA_CALL_ERET (propval, cuMemGetInfo, _mem, _mem);
+   CUDA_CALL_ASSERT (cuCtxDestroy, new_ctx);
+ }

(I'm CCing Tom here, as he is maintainer for these parts)

As we discussed earlier on our internal list, I think properly using
GOMP_OFFLOAD_init_device
is the right way, instead of using the lower level CUDA context
create/destroy.

I did not mean for you to first init the device and then immediately destroy
it by
GOMP_OFFLOAD_fini_device, just to obtain the property, but for you to just
take the opportunity to initialize
it for use, and leave it there until program exit. That should save resources
overall.
(BTW, CUDA contexts should be quite expensive to create/destroy, using a
cuCtxCreate/Destroy pair is probably
almost as slow)

  I have argued that this looks like a corner-case use case to me, as
querying for the remaining (rather than total) memory available to a
device that hasn't been (yet) used looks like of hardly any use to me,
because obviously at such a stage no memory has been used.  The OpenACC
standard does require us to handle such a request somehow, with returning
0 being another option, however I thought we may well have a quick peek
without pulling in all the state.

  I guess I have no strong opinion either way and I can adapt accordingly.

  NB that would have to be `gomp_init_device' rather than
`GOMP_OFFLOAD_init_device' AFAICS.


You'll have to use GOMP_OFFLOAD_init_device, as you are inside the plugin, 
gomp_init_device()
should not be available.

However, looking into this further, the checking conventions of 
GOMP_OFFLOAD_init_device
will have to be slightly tweaked to accommodate possible further initing from 
libgomp proper,
so this may requirement a longer string of changes...I think it's not worth it, 
or can
be adjusted later. I now think your current approach with the CUDA contexts is 
okay.

I think the patch is okay, although still needs approval from Thomas and Tom to 
commit.

Thanks,
Chung-Lin