Re: [PATCH] powerpc: Fix ICE with fp conditional move (PR target/93073)

[Nicholas Krause]

On 1/20/20 6:51 PM, Segher Boessenkool wrote:

On Tue, Jan 21, 2020 at 12:23:02AM +0100, Jakub Jelinek wrote:

On Mon, Jan 20, 2020 at 05:10:55PM -0600, Segher Boessenkool wrote:

On Mon, Jan 20, 2020 at 11:52:55PM +0100, Jakub Jelinek wrote:

PR target/93073
* config/rs6000/rs6000.c (rs6000_emit_cmove): Punt for compare_mode
other than SFmode or DFmode.

"If using fsel, punt for..." etc.

Ack.


+  /* Don't allow compare_mode other than SFmode or DFmode, for others there
+ is no fsel instruction.  */
+  if (compare_mode != SFmode && compare_mode != DFmode)
+return 0;

And move this a bit later, to right after
   /* At this point we know we can use fsel.  */
please?

I can move it if you want (not sure what is the gain, because the mode
check is cheaper than some of the other checks in there), but don't
understand why to after that comment rather than say immediately before that
comment.  Because with TFmode or KFmode we know we can't use fsel, and
the comment seems to mark the point at which we have decided we can use the
instruction and just prepare arguments for it.

We can (and should) use other instructions than just fsel here as well
(say, xscmpgedp followed by xxsel).  This can also work for QP float,
which also can be TFmode, just to complicate matters more.

This function is a bit big, and the logic is all over the place, but
let's try to make it better, little by little :-)


Segher

Segher,
If your goal is make it smaller I just looked at it but wasn't sure due 
to this line:

enum rtx_code code = GET_CODE (op);
in rs6000_emit_cmove and how to move it out but without duplicating it.
However the switch case for GE lowering seems to have two main cases 
that can

be removed into helper functions. The cases for ORDERED,EQ  should
be removed to another function and for UNGE,GT another function.
Not sure if that was your goal but it would make the function smaller and
perhaps less of a mess as at least a third of the function is probably
that switch statement :).

Nick

Re: Add a new combine pass

[Nicholas Krause]

On 11/17/19 6:35 PM, Richard Sandiford wrote:

(It's 23:35 local time, so it's still just about stage 1. :-))

While working on SVE, I've noticed several cases in which we fail
to combine instructions because the combined form would need to be
placed earlier in the instruction stream than the last of the
instructions being combined.  This includes one very important
case in the handling of the first fault register (FFR).

Combine currently requires the combined instruction to live at the same
location as i3.  I thought about trying to relax that restriction, but it
would be difficult to do with the current pass structure while keeping
everything linear-ish time.

So this patch instead goes for an option that has been talked about
several times over the years: writing a new combine pass that just
does instruction combination, and not all the other optimisations
that have been bolted onto combine over time.  E.g. it deliberately
doesn't do things like nonzero-bits tracking, since that really ought
to be a separate, more global, optimisation.

This is still far from being a realistic replacement for the even
the combine parts of the current combine pass.  E.g.:

- it only handles combinations that can be built up from individual
   two-instruction combinations.

- it doesn't allow new hard register clobbers to be added.

- it doesn't have the special treatment of CC operations.

- etc.

But we have to start somewhere.

On a more positive note, the pass handles things that the current
combine pass doesn't:

- the main motivating feature mentioned above: it works out where
   the combined instruction could validly live and moves it there
   if necessary.  If there are a range of valid places, it tries
   to pick the best one based on register pressure (although only
   with a simple heuristic for now).

- once it has combined two instructions, it can try combining the
   result with both later and earlier code, i.e. it can combine
   in both directions.

- it tries using REG_EQUAL notes for the final instruction.

- it can parallelise two independent instructions that both read from
   the same register or both read from memory.

This last feature is useful for generating more load-pair combinations
on AArch64.  In some cases it can also produce more store-pair combinations,
but only for consecutive stores.  However, since the pass currently does
this in a very greedy, peephole way, it only allows load/store-pair
combinations if the first memory access has a higher alignment than
the second, i.e. if we can be sure that the combined access is naturally
aligned.  This should help it to make better decisions than the post-RA
peephole pass in some cases while not being too aggressive.

The pass is supposed to be linear time without debug insns.
It only tries a constant number C of combinations per instruction
and its bookkeeping updates are constant-time.  Once it has combined two
instructions, it'll try up to C combinations on the result, but this can
be counted against the instruction that was deleted by the combination
and so effectively just doubles the constant.  (Note that C depends
on MAX_RECOG_OPERANDS and the new NUM_RANGE_USERS constant.)

Unfortunately, debug updates via propagate_for_debug are more expensive.
This could probably be fixed if the pass did more to track debug insns
itself, but using propagate_for_debug matches combine's behaviour.

The patch adds two instances of the new pass: one before combine and
one after it.  By default both are disabled, but this can be changed
using the new 3-bit run-combine param, where:

- bit 0 selects the new pre-combine pass
- bit 1 selects the main combine pass
- bit 2 selects the new post-combine pass

The idea is that run-combine=3 can be used to see which combinations
are missed by the new pass, while run-combine=6 (which I hope to be
the production setting for AArch64 at -O2+) just uses the new pass
to mop up cases that normal combine misses.  Maybe in some distant
future, the pass will be good enough for run-combine=[14] to be a
realistic option.

I ended up having to add yet another validate_simplify_* routine,
this time to do the equivalent of:

newx = simplify_replace_rtx (*loc, old_rtx, new_rtx);
validate_change (insn, loc, newx, 1);

but in a more memory-efficient way.  validate_replace_rtx isn't suitable
because it deliberately only tries simplifications in limited cases:

   /* Do changes needed to keep rtx consistent.  Don't do any other
  simplifications, as it is not our job.  */

And validate_simplify_insn isn't useful for this case because it works
on patterns that have already had changes made to them and expects
those patterns to be valid rtxes.  simplify-replace operations instead
need to simplify as they go, when the original modes are still to hand.

As far as compile-time goes, I tried compiling optabs.ii at -O2
with an --enable-checking=release compiler:

run-combine=2 (normal combine):  100.0% (baseline)
run-combine=4 (

Re: Add a new combine pass

[Nicholas Krause]

On 11/23/19 5:34 PM, Segher Boessenkool wrote:

Hi!

On Mon, Nov 18, 2019 at 05:55:13PM +, Richard Sandiford wrote:

Richard Sandiford  writes:

(It's 23:35 local time, so it's still just about stage 1. :-))

Or actually, just under 1 day after end of stage 1.  Oops.
Could have sworn stage 1 ended on the 17th :-(  Only realised
I'd got it wrong when catching up on Saturday's email traffic.

And inevitably, I introduced a couple of stupid mistakes while
trying to clean the patch up for submission by that (non-)deadline.
Here's a version that fixes an inverted overlapping memref check
and that correctly prunes the use list for combined instructions.
(This last one is just a compile-time saving -- the old code was
correct, just suboptimal.)

I've build the Linux kernel with the previous version, as well as this
one.  R0 is unmodified GCC, R1 is the first patch, R2 is this one:

(I've forced --param=run-combine=6 for R1 and R2):
(Percentages are relative to R0):

 R0R1R2R1R2
alpha   6107088   6101088   6101088   99.902%   99.902%
  arc   4008224   4006568   4006568   99.959%   99.959%
  arm   9206728   9200936   9201000   99.937%   99.938%
arm64  13056174  13018174  13018194   99.709%   99.709%
armhf 0 0 0 0 0
  c6x   2337237   2337077   2337077   99.993%   99.993%
 csky   3356602 0 0 0 0
h8300   1166996   1166776   1166776   99.981%   99.981%
 i386  11352159 0 0 0 0
 ia64  18230640  18167000  18167000   99.651%   99.651%
 m68k   3714271 0 0 0 0
   microblaze   4982749   4979945   4979945   99.944%   99.944%
 mips   8499309   8495205   8495205   99.952%   99.952%
   mips64   7042036   7039816   7039816   99.968%   99.968%
nds32   4486663 0 0 0 0
nios2   3680001   3679417   3679417   99.984%   99.984%
 openrisc   4226076   4225868   4225868   99.995%   99.995%
   parisc   7681895   7680063   7680063   99.976%   99.976%
 parisc64   8677077   8676581   8676581   99.994%   99.994%
  powerpc  10687611  10682199  10682199   99.949%   99.949%
powerpc64  17671082  17658570  17658570   99.929%   99.929%
  powerpc64le  17671082  17658570  17658570   99.929%   99.929%
  riscv32   1554938   1554758   1554758   99.988%   99.988%
  riscv64   6634342   6632788   6632788   99.977%   99.977%
 s390  13049643  13014939  13014939   99.734%   99.734%
   sh   3254743 0 0 0 0
  shnommu   1632364   1632124   1632124   99.985%   99.985%
sparc   4404993   4399593   4399593   99.877%   99.877%
  sparc64   6796711   6797491   6797491  100.011%  100.011%
   x86_64  19713174  19712817  19712817   99.998%   99.998%
   xtensa 0 0 0 0 0

0 means it didn't build.

armhf is probably my own problem, not sure yet.

xtensa starts with
/tmp/ccmJoY7l.s: Assembler messages:
/tmp/ccmJoY7l.s:407: Error: cannot represent `BFD_RELOC_8' relocation in object 
file
and it doesn't get better.

My powerpc64 config actually built the powerpc64le config, since the
kernel since a while looks what the host system is, for its defconfig.
Oh well, fixed now.

There are fivew new failures, with either of the combine2 patches.  And
all five are actually different (different symptoms, at least):

- csky fails on libgcc build:

/home/segher/src/gcc/libgcc/fp-bit.c: In function '__fixdfsi':
/home/segher/src/gcc/libgcc/fp-bit.c:1405:1: error: unable to generate reloads 
for:
  1405 | }
   | ^
(insn 199 86 87 8 (parallel [
 (set (reg:SI 101)
 (plus:SI (reg:SI 98)
 (const_int -32 [0xffe0])))
 (set (reg:CC 33 c)
 (lt:CC (plus:SI (reg:SI 98)
 (const_int -32 [0xffe0]))
 (const_int 0 [0])))
 ]) "/home/segher/src/gcc/libgcc/fp-bit.c":1403:23 207 {*cskyv2_declt}
  (nil))
during RTL pass: reload

Target problem?

- i386 goes into an infinite loop compiling, or at least an hour or so...
Erm I forgot too record what it was compiling.  I did attach a GDB...  It
is something from lra_create_live_ranges.

- m68k:

/home/segher/src/kernel/fs/exec.c: In function 'copy_strings':
/home/segher/src/kernel/fs/exec.c:590:1: internal compiler error: in 
final_scan_insn_1, at final.c:3048
   590 | }
   | ^
0x10408307 final_scan_insn_1
 /home/segher/src/gcc/gcc/final.c:3048
0x10408383 final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*)
 /home/segher/src/gcc/gcc/final.c:3152
0x10408797 final_1
 /home/segher/src/gcc/gcc/final.c:2020
0x104091f7 rest_of_handle_final
 /home/segher/src/gcc/gcc/final.c:4658
0x104091f7 execute

Re: Add a new combine pass

[Nicholas Krause]

On 11/23/19 6:09 PM, Segher Boessenkool wrote:

On Sat, Nov 23, 2019 at 06:01:28PM -0500, Nicholas Krause wrote:

Please just CC to this conversation as I keep getting removed.

Everyone who was on Cc: for this thread still is.  This is how email
works.  If you want to see everything on the list, subscribe to the
mailing list?


Segher


I was part of the original CCs on my comments but seems that there were 
two or seemed
to be two splitting versions of the thread. I would like to just keep 
all comments merged

in one thread is all.

Sorry for the confusion Segher,
Nick

[PATCH] Add missing noexpect causes in tuple for move functions

[Nicholas Krause]

This adds the remainging noexcept causes required for this cause
to meet the spec as dicussed last year and documented here:
http://cplusplus.github.io/LWG/lwg-active.html#2899.

Signed-off-by: Nicholas Krause 
---
 libstdc++-v3/include/std/tuple | 4 
 1 file changed, 4 insertions(+)

diff --git a/libstdc++-v3/include/std/tuple b/libstdc++-v3/include/std/tuple
index 56b97c25eed..d17512a1b7e 100644
--- a/libstdc++-v3/include/std/tuple
+++ b/libstdc++-v3/include/std/tuple
@@ -214,6 +214,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
enable_if::type>
 explicit
 constexpr _Tuple_impl(_UHead&& __head, _UTail&&... __tail)
+noexcept(__and_,
+ is_nothrow_move_constructible<_Inherited>>::value)
: _Inherited(std::forward<_UTail>(__tail)...),
  _Base(std::forward<_UHead>(__head)) { }
 
@@ -237,6 +239,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   template
 constexpr _Tuple_impl(_Tuple_impl<_Idx, _UHead, _UTails...>&& __in)
+noexcept(__and_,
+ is_nothrow_move_constructible<_Inherited>>::value)
: _Inherited(std::move
 (_Tuple_impl<_Idx, _UHead, _UTails...>::_M_tail(__in))),
  _Base(std::forward<_UHead>
-- 
2.17.1

[PATCH] Use proper print formatter in main function in fixincl.c

[Nicholas Krause]

This fixes the bug id, 71176 to use the proper known
code print formatter type, %lu for size_t rather than
%d which is considered best pratice for print statements.

Signed-off-by: Nicholas Krause 
---
 fixincludes/fixincl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fixincludes/fixincl.c b/fixincludes/fixincl.c
index 6dba2f6e830..8e16a2f7792 100644
--- a/fixincludes/fixincl.c
+++ b/fixincludes/fixincl.c
@@ -159,7 +159,7 @@ main (int argc, char** argv)
 tSCC zFmt[] =
   "\
 Processed %5d files containing %d bytes\n\
-Applying  %5d fixes to %d files\n\
+Applying  %5d fixes to %lu files\n\
 Altering  %5d of them\n";
 
 fprintf (stderr, zFmt, process_ct, ttl_data_size, apply_ct,
-- 
2.17.1

[PATCH] Use proper print formatter in main function in fixincl.c

[Nicholas Krause]

This fixes the bug id, 71176 to use the proper known
code print formatter type, %lu for size_t rather than
%d which is considered best pratice for print statements.

Signed-off-by: Nicholas Krause 
---
 fixincludes/fixincl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fixincludes/fixincl.c b/fixincludes/fixincl.c
index 6dba2f6e830..4e3010df0a6 100644
--- a/fixincludes/fixincl.c
+++ b/fixincludes/fixincl.c
@@ -159,10 +159,10 @@ main (int argc, char** argv)
 tSCC zFmt[] =
   "\
 Processed %5d files containing %d bytes\n\
-Applying  %5d fixes to %d files\n\
+Applying  %5lu fixes to %d files\n\
 Altering  %5d of them\n";
 
-fprintf (stderr, zFmt, process_ct, ttl_data_size, apply_ct,
+fprintf (stderr, zFmt, process_ct, (unsigned long int) ttl_data_size, 
apply_ct,
  fixed_ct, altered_ct);
   }
 #endif /* DO_STATS */
-- 
2.17.1

[PATCH] Use proper print formatter in main function in fixincl.c

[Nicholas Krause]

This fixes the bug id, 71176 to use the proper known
code print formatter type, %lu for size_t rather than
%d which is considered best pratice for print statements.

Signed-off-by: Nicholas Krause 
---
 fixincludes/fixincl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fixincludes/fixincl.c b/fixincludes/fixincl.c
index 6dba2f6e830..5b8b77a77f0 100644
--- a/fixincludes/fixincl.c
+++ b/fixincludes/fixincl.c
@@ -158,11 +158,11 @@ main (int argc, char** argv)
   if (VLEVEL( VERB_PROGRESS )) {
 tSCC zFmt[] =
   "\
-Processed %5d files containing %d bytes\n\
+Processed %5d files containing %lu bytes\n\
 Applying  %5d fixes to %d files\n\
 Altering  %5d of them\n";
 
-fprintf (stderr, zFmt, process_ct, ttl_data_size, apply_ct,
+fprintf (stderr, zFmt, process_ct, (unsigned int long) ttl_data_size, 
apply_ct,
  fixed_ct, altered_ct);
   }
 #endif /* DO_STATS */
-- 
2.17.1

Re: Monotonically increasing counter (was Re: [Contrib PATCH] Add scripts to convert GCC repo from SVN to Git)

[Nicholas Krause]

On 9/21/19 2:18 PM, Segher Boessenkool wrote:

On Thu, Sep 19, 2019 at 03:29:27PM -0400, Jason Merrill wrote:

I suppose we also need to decide what form we want to use for the
equivalent of gcc.gnu.org/rN.  I figure it needs to be some prefix
followed by a "commit-ish" (hash, tag, etc.).  Perhaps "g:" as the
prefix, so

gcc.gnu.org/g:HEAD
gcc.gnu.org/g:gcc-9-branch
gcc.gnu.org/g:3cf0d8938a953e

Hrm, but we should probably not allow everything here, some things can
be expensive to compute (HEAD~123456 for example), and we don't want to
expose the reflog on the server (if there even is one), etc.  OTOH
allowing pretty much everything here is a quite neat idea.

What do we use for gitweb thing?  That might have a solution for this
already.  Currently we seem to use plain gitweb, maybe cgit or similar
would be nicer?  It looks more modern, anyway :-P


Segher


If I recall correctly using git branches based off tags is the preferred 
way. And to


Seger's point after a server there is none after pulling down in git. 
Everything is


off line unless he means something else. The biggest thing as I pointed 
out at


Cauldron in terms of issues are:

a) How much history do you need in terms of how far back for git bisect 
or rebasing


and

b. Branching after a major release or for other non trunk branches. How 
to allow


other branches or how to set them up using tags e.t.c in git for this.

Mostly the problem with git is getting in right for these two based on 
the project


requirments,

Nick

[PATCH] Fix bug 86293

[Nicholas Krause]

This fixes the bug on the gcc bugzilla with id, 86293. Basically
a variable is undefined in certain build configuration scentarios
and must be enabled with the attribute marco and the flag, unused 
for it to avoid this build error. Build and regtested on x86_64_gnu,
ok for trunk?

Signed-off-by: Nicholas Krause 
---
 libitm/method-serial.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libitm/method-serial.cc b/libitm/method-serial.cc
index e4804946a34..ab23d0b5660 100644
--- a/libitm/method-serial.cc
+++ b/libitm/method-serial.cc
@@ -306,7 +306,7 @@ GTM::gtm_thread::serialirr_mode ()
   // We're already serial, so we don't need to ensure privatization safety
   // for other transactions here.
   gtm_word priv_time = 0;
-  bool ok = disp->trycommit (priv_time);
+  bool ok __attribute__((unused)) = disp->trycommit (priv_time);
   // Given that we're already serial, the trycommit better work.
   assert (ok);
 }
-- 
2.17.1

[PATCH] Proposed patch to fix bug id, 89796 on bugzilla

[Nicholas Krause]

Not sure if this is a correct fix to this bug found here:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89796 but
comments are welcome. If a backtrace is required please
let me know.

Signed-off-by: Nicholas Krause 
---
 gcc/cp/constraint.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 9884eb0db50..a78d0a9a49b 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1882,7 +1882,7 @@ tsubst_requires_expr (tree t, tree args,
   tree parms = TREE_OPERAND (t, 0);
   if (parms)
 {
-  parms = tsubst_constraint_variables (parms, args, complain, in_decl);
+  parms = tsubst_constraint_variables (PARM_CONSTR_PARMS (parms), args, 
complain, in_decl);
   if (parms == error_mark_node)
 return error_mark_node;
 }
-- 
2.17.1

[PATCH] PR88395 Fix Nullptr when compiling with -fconcepts

[Nicholas Krause]

This fixes bug id,88395 by checking if the second tree reqs
is correctly not a null pointer before calling the function,
tsubst_requirement_body. Otherwise we will get a nullptr
if compiling with -fconcepts for concepts enabled.

Signed-off-by: Nicholas Krause 
---
 gcc/cp/constraint.cc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 9884eb0db50..a4bf1632021 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1888,9 +1888,11 @@ tsubst_requires_expr (tree t, tree args,
 }
 
   tree reqs = TREE_OPERAND (t, 1);
+  if (reqs) {
   reqs = tsubst_requirement_body (reqs, args, complain, in_decl);
   if (reqs == error_mark_node)
 return error_mark_node;
+  }
 
   return finish_requires_expr (parms, reqs);
 }
-- 
2.17.1

[PATCH] Fix function, tsubst_requires_expr for callers to tsubst_constraint_variables

[Nicholas Krause]

This fixes both callers in tsubst_requires_expr to
tsubst_constraint_variables to wrap their respective
trees in PARM_CONSTR_PARMS. This is to get the correct
parmeter constraints from the tree before calling
tsubst_constraint_variables like other callers
in constraint.cc and to avoid subtle bugs in the
future.

Signed-off-by: Nicholas Krause 
---
 gcc/cp/constraint.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 9884eb0db50..cb06d0ec6e2 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1882,13 +1882,13 @@ tsubst_requires_expr (tree t, tree args,
   tree parms = TREE_OPERAND (t, 0);
   if (parms)
 {
-  parms = tsubst_constraint_variables (parms, args, complain, in_decl);
+  parms = tsubst_constraint_variables (PARM_CONSTR_PARMS (parms), args, 
complain, in_decl);
   if (parms == error_mark_node)
 return error_mark_node;
 }
 
   tree reqs = TREE_OPERAND (t, 1);
-  reqs = tsubst_requirement_body (reqs, args, complain, in_decl);
+  reqs = tsubst_requirement_body (PARM_CONSTR_PARMS (reqs), args, complain, 
in_decl);
   if (reqs == error_mark_node)
 return error_mark_node;
 
-- 
2.17.1

[PATCH] Remove unnecessary FIXME header comments in gimplify.c

[Nicholas Krause]

After dicussing this on the list as archived here,
https://gcc.gnu.org/ml/gcc/2019-04/msg00026.html
these comments are not needed it seems. Therefore
remove them to update the comments with these issues
as no longer required fixes due to several things
being required to build properly from those header
files being included.

Signed-off-by: Nicholas Krause 
---
 gcc/gimplify.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 6ac1b718ebd..e51c9b7d493 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -31,7 +31,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "tm_p.h"
 #include "gimple.h"
 #include "gimple-predict.h"
-#include "tree-pass.h" /* FIXME: only for PROP_gimple_any */
+#include "tree-pass.h"
 #include "ssa.h"
 #include "cgraph.h"
 #include "tree-pretty-print.h"
@@ -59,7 +59,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "gomp-constants.h"
 #include "splay-tree.h"
 #include "gimple-walk.h"
-#include "langhooks-def.h" /* FIXME: for lhd_set_decl_assembler_name */
+#include "langhooks-def.h"
 #include "builtins.h"
 #include "stringpool.h"
 #include "attribs.h"
-- 
2.17.1

[PATCH] PR88395 Fix Nullptr when compiling with -fconcepts

[Nicholas Krause]

This fixes both callers in tsubst_requires_expr to
tsubst_constraint_variables to wrap their respective
trees in PARM_CONSTR_PARMS. This is to get the correct
parmeter constraints from the tree before calling
tsubst_constraint_variables like other callers
in constraint.cc and to fix the bug id, 88395 on
the gcc bugzilla.

Signed-off-by: Nicholas Krause 
---
 gcc/cp/constraint.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 9884eb0db50..cb06d0ec6e2 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1882,13 +1882,13 @@ tsubst_requires_expr (tree t, tree args,
   tree parms = TREE_OPERAND (t, 0);
   if (parms)
 {
-  parms = tsubst_constraint_variables (parms, args, complain, in_decl);
+  parms = tsubst_constraint_variables (PARM_CONSTR_PARMS (parms), args, 
complain, in_decl);
   if (parms == error_mark_node)
 return error_mark_node;
 }
 
   tree reqs = TREE_OPERAND (t, 1);
-  reqs = tsubst_requirement_body (reqs, args, complain, in_decl);
+  reqs = tsubst_requirement_body (PARM_CONSTR_PARMS (reqs), args, complain, 
in_decl);
   if (reqs == error_mark_node)
 return error_mark_node;
 
-- 
2.17.1

[PATCH] PR88395 Fix Nullptr when compiling with -fconcepts

[Nicholas Krause]

This fixes the caller in tsubst_requires_expr to
tsubst_constraint_variables to wrap their respective
trees in PARM_CONSTR_PARMS. This is to get the correct
parmeter constraints from the tree before calling
tsubst_constraint_variables like other callers
in constraint.cc and to fix the bug id, 88395 on
the gcc bugzilla. OK for merge?

Signed-off-by: Nicholas Krause 
---
 gcc/cp/constraint.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 9884eb0db50..a78d0a9a49b 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1882,7 +1882,7 @@ tsubst_requires_expr (tree t, tree args,
   tree parms = TREE_OPERAND (t, 0);
   if (parms)
 {
-  parms = tsubst_constraint_variables (parms, args, complain, in_decl);
+  parms = tsubst_constraint_variables (PARM_CONSTR_PARMS (parms), args, 
complain, in_decl);
   if (parms == error_mark_node)
 return error_mark_node;
 }
-- 
2.17.1

[PATCH]C++/PR80188: Add correct marco call to error_at call in maybe_complain_about_tail_call

[Nicholas Krause]

This fixes maybe_complain_about_tail_call to in correctly use the marco, _ 
around reason in the
call to error_at. This fixes the issue with only the first part of the function 
being
correctly translated to English when translation is done. Passes standard 
testing on 
x86_64_linux_gnu without any new regressions. Ok for trunk?

Signed-off-by: Nicholas Krause 
---
 gcc/calls.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/calls.c b/gcc/calls.c
index 72cf9e016c8..38b4dc7617b 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -1515,7 +1515,7 @@ maybe_complain_about_tail_call (tree call_expr, const 
char *reason)
   if (!CALL_EXPR_MUST_TAIL_CALL (call_expr))
 return;
 
-  error_at (EXPR_LOCATION (call_expr), "cannot tail-call: %s", reason);
+  error_at (EXPR_LOCATION (call_expr), "cannot tail-call: %s", _(reason));
 }
 
 /* Fill in ARGS_SIZE and ARGS array based on the parameters found in
-- 
2.11.0

Re: [PATCH] rs6000: Save/restore r31 if frame_pointer_needed is true

[Nicholas Krause via Gcc-patches]

On 3/30/20 12:18 AM, luoxhu via Gcc-patches wrote:



On 2020/3/28 00:04, Segher Boessenkool wrote:

Hi!

On Fri, Mar 27, 2020 at 09:34:00AM +0800, luoxhu wrote:

On 2020/3/27 07:59, Segher Boessenkool wrote:

On Wed, Mar 25, 2020 at 11:15:22PM -0500, luo...@linux.ibm.com wrote:
frame_pointer_needed is set to true in reload pass 
setup_can_eliminate,
but regs_ever_live[31] is false, so pro_and_epilogue doesn't 
save/restore
r31 even it is used actually, causing CPU2006 465.tonto segment 
fault of

loading from invalid addresses.


If df_regs_ever_live_p(31) is false there is no hard frame pointer
anywhere in the program.  How can it be used then?


There is a piece of code emit move instruction to r31 even 
df_regs_ever_live_p(31) is false

in pro_and_epilogue.


Can you point out where (in rs6000-logue.c ot similar)?  We should fix
*that*.


As frame_point_needed is true and frame_pointer_needed is widely
used in this function, so I propose to save r31 in save_reg_p 
instead of check
(frame_pointer_needed && df_regs_ever_live_p(31), I haven't verify 
whether this works yet).

Is this reasonable?  Thanks.


frame_pointer_needed is often true when the backend can figure out we do
not actually need it.


rs6000-logue.c
void
rs6000_emit_prologue (void)
{
...
bbd21807fdf6 (geoffk 2000-03-16 03:16:41 + 26840)   /* 
Set frame pointer, if needed.  */
bbd21807fdf6 (geoffk 2000-03-16 03:16:41 + 26841)   if 
(frame_pointer_needed)

bbd21807fdf6 (geoffk 2000-03-16 03:16:41 + 26842) {
0d6c02bf24e4 (jakub  2005-06-30 14:26:32 + 26843)   
insn = emit_move_insn (gen_rtx_REG (Pmode, HARD_FRAME_POINTER_REGNUM),
bbd21807fdf6 (geoffk 2000-03-16 03:16:41 + 
26844)    sp_reg_rtx);
bbd21807fdf6 (geoffk 2000-03-16 03:16:41 + 26845)   
RTX_FRAME_RELATED_P (insn) = 1;

6b02f2a5c61e (meissner   1995-11-30 20:02:16 + 26846) }
d1bd513ed578 (kenner 1992-02-09 19:26:21 + 26847)
...
}


Ah, so this you mean.  I see.  It looks like if you change this to

   if (frame_pointer_needed && df_regs_ever_live_p 
(HARD_FRAME_POINTER_REGNUM))

 {
   insn = emit_move_insn (gen_rtx_REG (Pmode, 
HARD_FRAME_POINTER_REGNUM),

 sp_reg_rtx);
   RTX_FRAME_RELATED_P (insn) = 1;
 }

(so just that "if" clause changes), it'll all be fine.  Could you test
that please?


Tried with below patch, it still fails at same place, I guess this is 
not enough.
The instruction in 100dc034 change r31 from 0x105fdd70 to 
0x7fffbbf0 and didn't

restore it before return.
100dc020 <__atomvec_module_MOD_atom_index_from_pos>:
   100dc020:   51 10 40 3c lis r2,4177
   100dc024:   00 7f 42 38 addi    r2,r2,32512
   100dc028:   28 00 e3 e8 ld  r7,40(r3)
   100dc02c:   e1 ff 21 f8 stdu    r1,-32(r1)
   100dc030:   00 00 27 2c cmpdi   r7,0
   100dc034:   78 0b 3f 7c mr  r31,r1
   100dc038:   08 00 82 40 bne 100dc040 
<__atomvec_module_MOD_atom_index_from_pos+0x20>

   100dc03c:   01 00 e0 38 li  r7,1
   100dc040:   38 00 43 e9 ld  r10,56(r3)
   100dc044:   30 00 23 e9 ld  r9,48(r3)
   100dc048:   00 00 03 e9 ld  r8,0(r3)


Diff:

diff --git a/gcc/config/rs6000/rs6000-logue.c 
b/gcc/config/rs6000/rs6000-logue.c

index 4cbf228eb79..28fda1866d8 100644
--- a/gcc/config/rs6000/rs6000-logue.c
+++ b/gcc/config/rs6000/rs6000-logue.c
@@ -3658,7 +3658,7 @@ rs6000_emit_prologue (void)
    }

  /* Set frame pointer, if needed.  */
-  if (frame_pointer_needed)
+  if (frame_pointer_needed && df_regs_ever_live_p 
(HARD_FRAME_POINTER_REGNUM))

    {
  insn = emit_move_insn (gen_rtx_REG (Pmode, 
HARD_FRAME_POINTER_REGNUM),

    sp_reg_rtx);
@@ -4534,7 +4534,8 @@ rs6000_emit_epilogue (enum epilogue_type 
epilogue_type)

    }
  /* If we have a frame pointer, we can restore the old stack pointer
 from it.  */
-  else if (frame_pointer_needed)
+  else if (frame_pointer_needed
+  && df_regs_ever_live_p (HARD_FRAME_POINTER_REGNUM))
    {
  frame_reg_rtx = sp_reg_rtx;
  if (DEFAULT_ABI == ABI_V4)
@@ -4873,7 +4874,8 @@ rs6000_emit_epilogue (enum epilogue_type 
epilogue_type)

    a REG_CFA_DEF_CFA note, but that's OK;  A duplicate is
    discarded by dwarf2cfi.c/dwarf2out.c, and in any case would
    be harmless if emitted.  */
-  if (frame_pointer_needed)
+  if (frame_pointer_needed
+ && df_regs_ever_live_p (HARD_FRAME_POINTER_REGNUM))
   {
 insn = get_last_insn ();
 add_reg_note (insn, REG_CFA_DEF_CFA,



Program received signal SIGSEGV: Segmentation fault - invalid memory 
reference.


Backtrace for this error:
#0  0x200a2243 in ???
#1  0x200a0abb in ???
#2  0x200604d7 in ???
#3  0x1027418c in __mol_module_MOD_make_image_of_shell
   at 
/home/luoxhu/workspace/cpu2006/benchspec/CPU2006/465.tonto/build/build_peak_none./mol.fppized.f90:9376

#

Re: [PATCH] Describe coding conventions surrounding "auto"

[Nicholas Krause via Gcc-patches]

On 5/18/20 6:51 PM, Martin Sebor via Gcc-patches wrote:

On 5/18/20 12:02 PM, Richard Sandiford wrote:

Martin Sebor  writes:

On 5/16/20 4:43 AM, Richard Sandiford wrote:

Sorry for the empty subject line earlier...

Jason Merrill  writes:

On Fri, May 15, 2020 at 9:47 PM Martin Sebor  wrote:


On 5/15/20 8:08 AM, Richard Sandiford wrote:

Those are all good examples.  Mind putting that into a patch
for the coding conventions?

How's this?  I added "new" expressions as another example of the
first category.

I'm sure I've missed other good uses, but we can always add to the
list later if necessary.

Thanks,
Richard


0001-Describe-coding-conventions-surrounding-auto.patch

   From 10b27e367de0fa9d5bf91544385401cdcbdb8c00 Mon Sep 17 
00:00:00 2001

From: Richard Sandiford
Date: Fri, 15 May 2020 14:58:46 +0100
Subject: [PATCH] Describe coding conventions surrounding "auto"

---
    htdocs/codingconventions.html | 53 
+++

    htdocs/codingrationale.html   | 17 +++
    2 files changed, 70 insertions(+)

diff --git a/htdocs/codingconventions.html

b/htdocs/codingconventions.html

index f4732ef6..ae49fb91 100644
--- a/htdocs/codingconventions.html
+++ b/htdocs/codingconventions.html
@@ -51,6 +51,7 @@ the conventions separately from any other 
changes to

the code.

    Language Use
    
    Variable Definitions
+    Use of auto
    Struct Definitions
    Class Definitions
    Constructors and

Destructors
@@ -884,6 +885,58 @@ Variables may be simultaneously defined and 
tested

in control expressions.
    Rationale and 
Discussion

    

+Use of auto
+
+auto should be used in the following circumstances:
+
+  when the expression gives the C++ type explicitly.  For

example

+
+    
+if (auto *table = dyn_cast  (insn)) // OK

+  ...
+if (rtx_jump_table_data *table = dyn_cast 


(insn))  // Avoid

+  ...
+auto *map = new hash_map ;

  // OK
+hash_map  *map = new hash_map ;

// Avoid

+
+    This rule does not apply to abbreviated type names 
embedded in
+    an identifier, such as the result of 
tree_to_shwi.

+  
+  
+    when the expression simply provides an alternative view 
of an

object

+    and is bound to a read-only temporary.  For example:
+
+    
+auto wioff = wi::to_wide (off); // OK
+wide_int wioff = wi::to_wide (off); // Avoid if wioff is 
read-only

+auto minus1 = std::shwi (-1, prec); // OK
+wide_int minus1 = std::shwi (-1, prec); // Avoid if minus1 is

read-only

+
+    In principle this rule applies to other views of an 
object too,

+    such as a reversed view of a list, or a sequential view of a
+    hash_set.  It does not apply to general

temporaries.

+  
+  
+    the type of an iterator.  For example:
+
+    
+auto it = std::find (names.begin (), names.end (),

needle);    // OK

+vector ::iterator it = std::find (names.begin (),
+    names.end (), 
needle); //

Avoid

+  
+  
+    the type of a lambda expression.  For example:
+
+    
+auto f = [] (int x) { return x + 1; }; //

OK

+  
+
+
+auto should not be used in other 
contexts.


This seems like a severe (and unnecessary) restriction...


+
+
+Rationale and Discussion
+

    Struct Definitions

diff --git a/htdocs/codingrationale.html 
b/htdocs/codingrationale.html

index 0b44f1da..a919023c 100644
--- a/htdocs/codingrationale.html
+++ b/htdocs/codingrationale.html
@@ -50,6 +50,23 @@ if (info *q = get_any_available_info ()) {
    }
    

+Use of auto
+
+The reason for preferring auto in expressions like:
+auto wioff = wi::to_wide (off);
+is that using the natural type of the expression is more 
efficient than

+converting it to types like wide_int.
+
+The reason for excluding other uses of auto is that
+in most other cases the type carries useful information.  For 
example:

+for (const std::pair 

&elt : indirect_pool)

+  ...
+makes it obvious that elt is a pair and gives the 
types of

+elt.first and elt.second.  In contrast:
+for (const auto &elt : indirect_pool)
+  ...
+gives no immediate indication what elt is or what can
+be done with it.


...there are countless constructs in C++ 98 as well in C where there
is no such indication yet we don't (and very well can't) try to avoid
using them.  Examples include macros, members of structures defined
far away from the point of their use, results of ordinary function
calls, results of overloaded functions or templates, default function
arguments, default template parameters, etc.

By way of a random example from genrecog.c:

   int_set::iterator end
  = std::set_union (trans1->labels.begin (), 
trans1->labels.end (),

    combined->begin (), combined->end (),
    next->begin ());

There is no immediate indication precisely what 

Re: [PATCH] vectorizer: add _bb_vec_info::const_iterator

[Nicholas Krause via Gcc-patches]

On 6/12/20 11:49 AM, Richard Sandiford wrote:

Martin Liška  writes:

On 6/12/20 3:22 PM, Richard Sandiford wrote:

Martin Liška  writes:

diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
index 636ad59c001..eac372e6abc 100644
--- a/gcc/tree-vect-patterns.c
+++ b/gcc/tree-vect-patterns.c
@@ -5120,20 +5120,12 @@ vect_determine_precisions (vec_info *vinfo)
 else
   {
 bb_vec_info bb_vinfo = as_a  (vinfo);
-  gimple_stmt_iterator si = bb_vinfo->region_end;
-  gimple *stmt;
-  do
+  for (gimple *stmt: *bb_vinfo)
{
- if (!gsi_stmt (si))
-   si = gsi_last_bb (bb_vinfo->bb);
- else
-   gsi_prev (&si);
- stmt = gsi_stmt (si);
  stmt_vec_info stmt_info = vinfo->lookup_stmt (stmt);
  if (stmt_info && STMT_VINFO_VECTORIZABLE (stmt_info))
vect_determine_stmt_precisions (vinfo, stmt_info);
}
-  while (stmt != gsi_stmt (bb_vinfo->region_begin));
   }
   }


This loop wants a reverse iterator: it starts at the end and walks
backwards.  That's important because vect_determine_stmt_precisions
acts based on information recorded about later uses.


I thought that it may be a problematic change. Note that we don't a have
test coverage for the situation in testsuite (on x86_64). So I'm also
introducing reverse_iterator for this.


There's definitely coverage on aarch64.  Thought there would be on x86
too for the PAVG stuff, but obviously not.


diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
index cdd6f6c5e5d..766598862d4 100644
--- a/gcc/tree-vect-stmts.c
+++ b/gcc/tree-vect-stmts.c
@@ -1342,7 +1342,7 @@ vect_init_vector_1 (vec_info *vinfo, stmt_vec_info 
stmt_vinfo, gimple *new_stmt,
 else
  {
 bb_vec_info bb_vinfo = dyn_cast  (vinfo);
- gimple_stmt_iterator gsi_region_begin = bb_vinfo->region_begin;
+ gimple_stmt_iterator gsi_region_begin = bb_vinfo->begin ().gsi;
  gsi_insert_before (&gsi_region_begin, new_stmt, GSI_SAME_STMT);
  }
   }


Feels like this kind-of breaks the abstraction a bit.

Would it make sense instead to add the operators to gimple_stmt_iterator
itself and just make const_iterator a typedef of that?


Well, I'm planning to use the _bb_vec_info::const_iterator to jump in between
BBs, so it can't be a simple typedef.


But what happens to this code when that happens?  Is inserting at
begin().gsi meaningful?  I.e. is the stmt at *begin() guaranteed
to dominate all the SLP code even with multple BBs?

Personally I'd prefer either:

(a) const_iterator starts out as a typedef of gimple_stmt_iterator,
 and with your later changes becomes a derived class of it; or

(b) we just provide a region_begin() function that returns the gsi
 directly.


+  reverse_iterator rbegin () const
+{
+  reverse_iterator it = reverse_iterator (m_region_end);
+  if (*it == NULL)
+   return reverse_iterator (gsi_last_bb (m_region_end.bb));
+  else
+   return it;
+}


I think the else case should return “++it” instead, since AIUI
m_region_end is an exclusive rather than inclusive endpoint.

Also, one minor formatting nit, sorry: the other functions instead
indent the “{” block by the same amount as the function prototype,
which seems more consistent with the usual out-of-class formatting.

Thanks,
Richard


Hi Martin,

I was curious do we ever plan to use r-values here or in code that
can iterator? There was a new version in C++11 for r values that
is basically the same, but the deference operator * is overloaded
to do std::move(value) rather than what you have currently. Otherwise
the other comments about having a reverse iterator are all I have.

Doesn't seem useful here but just wanted to point it out with the
move to C++11,

Nick


--
Fundamentally an organism has conscious mental states if and only if 
there is something that it is like to be that organism--something it is 
like for the organism. - Thomas Nagel

[PATCH RFC] Propose Updating Basic Block Checking Limits in variable_tracking_main_1

[Nicholas Krause via Gcc-patches]

From: Nicholas Krause 

Changelog:gcc/
*var-tracking.c(variable_tracking_main): Update
numbers for both number of basic blocks per
function and number of edges per function to 
basic blocks to more sane numbers, in order to 
avoid extra edge cases.

Signed-off-by: Nicholas Krause 
---
 gcc/var-tracking.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/var-tracking.c b/gcc/var-tracking.c
index fc861a0..9cf1b5d 100644
--- a/gcc/var-tracking.c
+++ b/gcc/var-tracking.c
@@ -10470,8 +10470,8 @@ variable_tracking_main_1 (void)
   if (!flag_var_tracking)
 return 0;
 
-  if (n_basic_blocks_for_fn (cfun) > 500
-  && n_edges_for_fn (cfun) / n_basic_blocks_for_fn (cfun) >= 20)
+  if (n_basic_blocks_for_fn (cfun) > 1
+  && n_edges_for_fn (cfun) / n_basic_blocks_for_fn (cfun) >= 500)
 {
   vt_debug_insns_local (true);
   return 0;
-- 
1.8.3.1

Re: [PATCH RFC] Propose Updating Basic Block Checking Limits in variable_tracking_main_1

[Nicholas Krause via Gcc-patches]

On 6/18/20 2:32 PM, Nicholas Krause wrote:

From: Nicholas Krause 

Changelog:gcc/
*var-tracking.c(variable_tracking_main): Update
numbers for both number of basic blocks per
function and number of edges per function to
basic blocks to more sane numbers, in order to
avoid extra edge cases.

Signed-off-by: Nicholas Krause 
---
  gcc/var-tracking.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/var-tracking.c b/gcc/var-tracking.c
index fc861a0..9cf1b5d 100644
--- a/gcc/var-tracking.c
+++ b/gcc/var-tracking.c
@@ -10470,8 +10470,8 @@ variable_tracking_main_1 (void)
if (!flag_var_tracking)
  return 0;
  
-  if (n_basic_blocks_for_fn (cfun) > 500

-  && n_edges_for_fn (cfun) / n_basic_blocks_for_fn (cfun) >= 20)
+  if (n_basic_blocks_for_fn (cfun) > 1
+  && n_edges_for_fn (cfun) / n_basic_blocks_for_fn (cfun) >= 500)
  {
vt_debug_insns_local (true);
return 0;



Commit message seems to not have been added. Sorry about that:

This proposes based on issues reported in PR93092 to update
the check for basic blocks and the ratio of it to edges in
variable_tracking_main_1 in order to avoid edge cases as
found in the reported PR with huge numbers of edges or
basic blocks per function. Therefore update the numbers
to a more sane limit, while preserving the current ratio
of 25:1 for basic blocks per function to edges per function.

Changelog:gcc/
*var-tracking.c(variable_tracking_main): Update
numbers for both number of basic blocks per
function and number of edges per function to
basic blocks to more sane numbers, in order to
avoid extra edge cases.

Signed-off-by: Nicholas Krause 
---
 gcc/var-tracking.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/var-tracking.c b/gcc/var-tracking.c
index fc861a0..9cf1b5d 100644
--- a/gcc/var-tracking.c
+++ b/gcc/var-tracking.c
@@ -10470,8 +10470,8 @@ variable_tracking_main_1 (void)
   if (!flag_var_tracking)
 return 0;

-  if (n_basic_blocks_for_fn (cfun) > 500
-  && n_edges_for_fn (cfun) / n_basic_blocks_for_fn (cfun) >= 20)
+  if (n_basic_blocks_for_fn (cfun) > 1
+  && n_edges_for_fn (cfun) / n_basic_blocks_for_fn (cfun) >= 500)
 {
   vt_debug_insns_local (true);
   return 0;
--
1.8.3.1


--
Fundamentally an organism has conscious mental states if and only if 
there is something that it is like to be that organism--something it is 
like for the organism. - Thomas Nagel

[PATCH] Handle TYPE_PACK_EXPANSION in cxx_incomplete_type_diagnostic

[Nicholas Krause via Gcc-patches]

This fixs the PR95672 by adding the missing TYPE_PACK_EXPANSION case in
cxx_incomplete_type_diagnostic in order to avoid ICES on diagnosing
incomplete template pack expansion cases.
Signed-off-by: Nicholas Krause 

Changelog:
*cp/type2ck.c: Add missing case TYPE_PACK_EXPANSION for diagnosticing
incomplete parameter pack expansion in cxx_incomplete_type_diagnostic
in order to avoid ICEing here.
---
 gcc/cp/typeck2.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 5fd3b82..dc5bd5e 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -552,6 +552,11 @@ cxx_incomplete_type_diagnostic (location_t loc, const_tree 
value,
   TYPE_NAME (type));
   break;
 
+case TYPE_PACK_EXPANSION:
+ emit_diagnostic(diag_kind, loc, 0,
+"invalid expansion of template expansion parameter %qT",
+ PACK_EXPANSION_PATTERN(type));
+  break;
 case TYPENAME_TYPE:
 case DECLTYPE_TYPE:
   emit_diagnostic (diag_kind, loc, 0,
-- 
1.8.3.1

[PATCHv3] Handle TYPE_PACK_EXPANSION in cxx_incomplete_type_diagnostic

[Nicholas Krause via Gcc-patches]

From: Nicholas Krause 

This fixs the PR95672 by adding the missing TYPE_PACK_EXPANSION case in
cxx_incomplete_type_diagnostic in order to avoid ICES on diagnosing
incomplete template pack expansion cases. In v2, add the missing required
test case for all new patches. v3 Fixes both the test case to compile in
C++11 mode and the message to print out only the type.

Signed-off-by: Nicholas Krause 
Changelog:
*gcc/cp/typeck2.c: Add missing case TYPE_PACK_EXPANSION for diagnosing
incomplete parameter pack expansion in cxx_incomplete_type_diagnostic
in order to avoid ICEing here.
*gcc/testsuite/g++.db/template/PR95672.C:Add new testcase for PR95672.
---
 gcc/cp/typeck2.c| 6 ++
 gcc/testsuite/g++.dg/template/PR95672.C | 3 +++
 2 files changed, 9 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/template/PR95672.C

diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 5fd3b82fa89..28b32fe0b5a 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -552,6 +552,12 @@ cxx_incomplete_type_diagnostic (location_t loc, const_tree 
value,
   TYPE_NAME (type));
   break;
 
+case TYPE_PACK_EXPANSION:
+ emit_diagnostic (diag_kind, loc, 0,
+"invalid use of pack expansion %qT",
+ type);
+  break;
+
 case TYPENAME_TYPE:
 case DECLTYPE_TYPE:
   emit_diagnostic (diag_kind, loc, 0,
diff --git a/gcc/testsuite/g++.dg/template/PR95672.C 
b/gcc/testsuite/g++.dg/template/PR95672.C
new file mode 100644
index 000..8795c13f365
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/PR95672.C
@@ -0,0 +1,3 @@
+//c++ PR96572 
+// {dg-options "-std=c++11"}
+struct g_class : decltype  (auto) ... {  } ; { dg-error "invalid use of pack 
expansion" }
-- 
2.20.1

[PATCHv4] Handle TYPE_PACK_EXPANSION in cxx_incomplete_type_diagnostic

[Nicholas Krause via Gcc-patches]

From: Nicholas Krause 

This fixs the PR95672 by adding the missing TYPE_PACK_EXPANSION case in
cxx_incomplete_type_diagnostic in order to avoid ICES on diagnosing
incomplete template pack expansion cases. In v2, add the missing required
test case for all new patches. v3 Fixes both the test case to compile in
C++11 mode and the message to print out only the type. v4 fixes the testcase
to only target C++11.

gcc/cp/ChangeLog:

* typeck2.c (cxx_incomplete_type_diagnostic):

gcc/testsuite/ChangeLog:

* g++.dg/template/PR95672.C: New test.

Signed-off-by: Nicholas Krause 
---
 gcc/cp/typeck2.c| 6 ++
 gcc/testsuite/g++.dg/template/PR95672.C | 3 +++
 2 files changed, 9 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/template/PR95672.C

diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 5fd3b82fa89..28b32fe0b5a 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -552,6 +552,12 @@ cxx_incomplete_type_diagnostic (location_t loc, const_tree 
value,
   TYPE_NAME (type));
   break;
 
+case TYPE_PACK_EXPANSION:
+ emit_diagnostic (diag_kind, loc, 0,
+"invalid use of pack expansion %qT",
+ type);
+  break;
+
 case TYPENAME_TYPE:
 case DECLTYPE_TYPE:
   emit_diagnostic (diag_kind, loc, 0,
diff --git a/gcc/testsuite/g++.dg/template/PR95672.C 
b/gcc/testsuite/g++.dg/template/PR95672.C
new file mode 100644
index 000..fcc3da0a132
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/PR95672.C
@@ -0,0 +1,3 @@
+//c++ PR96572 
+// { dg-do compile { target c++11 } }
+struct g_class : decltype  (auto) ... {  } ; //{ dg-error "invalid use of pack 
expansion" }
-- 
2.20.1

[PATCHv5] Handle TYPE_PACK_EXPANSION in cxx_incomplete_type_diagnostic

[Nicholas Krause via Gcc-patches]

From: Nicholas Krause 

This fixs the PR95672 by adding the missing TYPE_PACK_EXPANSION case in
cxx_incomplete_type_diagnostic in order to avoid ICES on diagnosing
incomplete template pack expansion cases. In v2, add the missing required
test case for all new patches. v3 Fixes both the test case to compile in
C++11 mode and the message to print out only the type. v4 fixes the testcase
to only target C++11. v5 fixes test case to run properly.

gcc/cp/ChangeLog:

* typeck2.c (cxx_incomplete_type_diagnostic): Add missing 
TYPE_EXPANSION_PACK
  check for diagnosticing incomplete types in 
cxx_incomplete_type_diagnostic.

gcc/testsuite/ChangeLog:

* g++.dg/template/PR95672.C: New test.

Signed-off-by: Nicholas Krause 
---
 gcc/cp/typeck2.c| 6 ++
 gcc/testsuite/g++.dg/template/PR95672.C | 3 +++
 2 files changed, 9 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/template/PR95672.C

diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 5fd3b82fa89..28b32fe0b5a 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -552,6 +552,12 @@ cxx_incomplete_type_diagnostic (location_t loc, const_tree 
value,
   TYPE_NAME (type));
   break;
 
+case TYPE_PACK_EXPANSION:
+ emit_diagnostic (diag_kind, loc, 0,
+"invalid use of pack expansion %qT",
+ type);
+  break;
+
 case TYPENAME_TYPE:
 case DECLTYPE_TYPE:
   emit_diagnostic (diag_kind, loc, 0,
diff --git a/gcc/testsuite/g++.dg/template/PR95672.C 
b/gcc/testsuite/g++.dg/template/PR95672.C
new file mode 100644
index 000..fcc3da0a132
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/PR95672.C
@@ -0,0 +1,3 @@
+//c++ PR96572 
+// { dg-do compile { target c++11 } }
+struct g_class : decltype  (auto) ... {  } ; // { dg-error "invalid use of 
pack expansion" }
-- 
2.20.1

Re: [PATCHv5] Handle TYPE_PACK_EXPANSION in cxx_incomplete_type_diagnostic

[Nicholas Krause via Gcc-patches]

On 6/22/20 5:51 PM, Jason Merrill wrote:

On 6/22/20 4:01 PM, Nicholas Krause wrote:

From: Nicholas Krause 

This fixs the PR95672 by adding the missing TYPE_PACK_EXPANSION case in
cxx_incomplete_type_diagnostic in order to avoid ICES on diagnosing
incomplete template pack expansion cases. In v2, add the missing required
test case for all new patches. v3 Fixes both the test case to compile in
C++11 mode and the message to print out only the type. v4 fixes the 
testcase

to only target C++11. v5 fixes test case to run properly.

gcc/cp/ChangeLog:

* typeck2.c (cxx_incomplete_type_diagnostic): Add missing 
TYPE_EXPANSION_PACK
  check for diagnosticing incomplete types in 
cxx_incomplete_type_diagnostic.


gcc/testsuite/ChangeLog:

* g++.dg/template/PR95672.C: New test.

Signed-off-by: Nicholas Krause 
---
  gcc/cp/typeck2.c    | 6 ++
  gcc/testsuite/g++.dg/template/PR95672.C | 3 +++
  2 files changed, 9 insertions(+)
  create mode 100644 gcc/testsuite/g++.dg/template/PR95672.C

diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 5fd3b82fa89..28b32fe0b5a 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -552,6 +552,12 @@ cxx_incomplete_type_diagnostic (location_t loc, 
const_tree value,

 TYPE_NAME (type));
    break;
+    case TYPE_PACK_EXPANSION:
+ emit_diagnostic (diag_kind, loc, 0,
+ "invalid use of pack expansion %qT",
+  type);
+  break;
+
  case TYPENAME_TYPE:
  case DECLTYPE_TYPE:
    emit_diagnostic (diag_kind, loc, 0,
diff --git a/gcc/testsuite/g++.dg/template/PR95672.C 
b/gcc/testsuite/g++.dg/template/PR95672.C

new file mode 100644
index 000..fcc3da0a132
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/PR95672.C
@@ -0,0 +1,3 @@
+//c++ PR96572
+// { dg-do compile { target c++11 } }
+struct g_class : decltype  (auto) ... {  } ; // { dg-error "invalid 
use of pack expansion" }


FAIL: g++.dg/template/PR95672.C  -std=c++11  (test for errors, line 3)
FAIL: g++.dg/template/PR95672.C  -std=c++11 (test for excess errors)



It seems to pass for me with make check RUNTESTFLAGS=dg.exp=PR96752.ext

on one of the farm machines. Not sure why it fails for you unless that

extra space before the semicolon is the issue.

Nick
--
Fundamentally an organism has conscious mental states if and only if 
there is something that it is like to be that organism--something it is 
like for the organism. - Thomas Nagel

Re: [PATCHv5] Handle TYPE_PACK_EXPANSION in cxx_incomplete_type_diagnostic

[Nicholas Krause via Gcc-patches]

On 6/22/20 7:47 PM, Marek Polacek wrote:

On Mon, Jun 22, 2020 at 06:55:01PM -0400, Nicholas Krause via Gcc-patches wrote:



On 6/22/20 5:51 PM, Jason Merrill wrote:

On 6/22/20 4:01 PM, Nicholas Krause wrote:

From: Nicholas Krause 

This fixs the PR95672 by adding the missing TYPE_PACK_EXPANSION case in
cxx_incomplete_type_diagnostic in order to avoid ICES on diagnosing
incomplete template pack expansion cases. In v2, add the missing required
test case for all new patches. v3 Fixes both the test case to compile in
C++11 mode and the message to print out only the type. v4 fixes the
testcase
to only target C++11. v5 fixes test case to run properly.

gcc/cp/ChangeLog:

    * typeck2.c (cxx_incomplete_type_diagnostic): Add missing
TYPE_EXPANSION_PACK
      check for diagnosticing incomplete types in
cxx_incomplete_type_diagnostic.

gcc/testsuite/ChangeLog:

    * g++.dg/template/PR95672.C: New test.

Signed-off-by: Nicholas Krause 
---
  gcc/cp/typeck2.c                        | 6 ++
  gcc/testsuite/g++.dg/template/PR95672.C | 3 +++
  2 files changed, 9 insertions(+)
  create mode 100644 gcc/testsuite/g++.dg/template/PR95672.C

diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 5fd3b82fa89..28b32fe0b5a 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -552,6 +552,12 @@ cxx_incomplete_type_diagnostic (location_t loc,
const_tree value,
                 TYPE_NAME (type));
        break;
+    case TYPE_PACK_EXPANSION:
+     emit_diagnostic (diag_kind, loc, 0,
+             "invalid use of pack expansion %qT",
+              type);
+      break;
+
      case TYPENAME_TYPE:
      case DECLTYPE_TYPE:
        emit_diagnostic (diag_kind, loc, 0,
diff --git a/gcc/testsuite/g++.dg/template/PR95672.C
b/gcc/testsuite/g++.dg/template/PR95672.C
new file mode 100644
index 000..fcc3da0a132
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/PR95672.C
@@ -0,0 +1,3 @@
+//c++ PR96572
+// { dg-do compile { target c++11 } }
+struct g_class : decltype  (auto) ... {  } ; // { dg-error "invalid
use of pack expansion" }


FAIL: g++.dg/template/PR95672.C  -std=c++11  (test for errors, line 3)
FAIL: g++.dg/template/PR95672.C  -std=c++11 (test for excess errors)



It seems to pass for me with make check RUNTESTFLAGS=dg.exp=PR96752.ext


That doesn't test anything at all, because it's the wrong filename and the
wrong suffix.  Use

GXX_TESTSUITE_STDS=98,11,14,17,2a make check-c++ RUNTESTFLAGS=dg.exp=PR95672.C

Marek

That makes sense. What is the difference between target and dg-options 
as both seem to be the same for language support. Yet it seems that this

only wants to run under:
/* { dg-options "-std=c++11" } */

The testsuite wiki page mentions no difference between the two for 
targets as related to lanuage so just curious.


Thanks,
Nick

--
Fundamentally an organism has conscious mental states if and only if 
there is something that it is like to be that organism--something it is 
like for the organism. - Thomas Nagel

[PATCHv6] Handle TYPE_PACK_EXPANSION in cxx_incomplete_type_diagnostic

[Nicholas Krause via Gcc-patches]

From: Nicholas Krause 

This fixs the PR95672 by adding the missing TYPE_PACK_EXPANSION case in
cxx_incomplete_type_diagnostic in order to avoid ICES on diagnosing
incomplete template pack expansion cases. In v2, add the missing required
test case for all new patches. v3 Fixes both the test case to compile in
C++11 mode and the message to print out only the type. v4 fixes the testcase
to only target C++11. v5 and v6 fix the test case properly.

gcc/cp/ChangeLog:

* typeck2.c (cxx_incomplete_type_diagnostic): Add missing 
TYPE_EXPANSION_PACK
  check for diagnosticing incomplete types in 
cxx_incomplete_type_diagnostic.

gcc/testsuite/ChangeLog:

* g++.dg/template/PR95672.C: New test.

Signed-off-by: Nicholas Krause 
---
 gcc/cp/typeck2.c| 6 ++
 gcc/testsuite/g++.dg/template/PR95672.C | 3 +++
 2 files changed, 9 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/template/PR95672.C

diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 5fd3b82fa89..28b32fe0b5a 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -552,6 +552,12 @@ cxx_incomplete_type_diagnostic (location_t loc, const_tree 
value,
   TYPE_NAME (type));
   break;
 
+case TYPE_PACK_EXPANSION:
+ emit_diagnostic (diag_kind, loc, 0,
+"invalid use of pack expansion %qT",
+ type);
+  break;
+
 case TYPENAME_TYPE:
 case DECLTYPE_TYPE:
   emit_diagnostic (diag_kind, loc, 0,
diff --git a/gcc/testsuite/g++.dg/template/PR95672.C 
b/gcc/testsuite/g++.dg/template/PR95672.C
new file mode 100644
index 000..fcc3da0a132
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/PR95672.C
@@ -0,0 +1,3 @@
+// PR c++/96572 
+// { dg-do compile}
+// { dg-options "-std=c++11" }
+struct g_class : decltype  (auto) ... {  }; // { dg-error "invalid use of pack 
expansion" }
-- 
2.20.1

Re: [PATCHv6] Handle TYPE_PACK_EXPANSION in cxx_incomplete_type_diagnostic

[Nicholas Krause via Gcc-patches]

On 6/22/20 10:01 PM, Marek Polacek wrote:

On Mon, Jun 22, 2020 at 09:42:51PM -0400, Nicholas Krause via Gcc-patches wrote:

From: Nicholas Krause 

This fixs the PR95672 by adding the missing TYPE_PACK_EXPANSION case in
cxx_incomplete_type_diagnostic in order to avoid ICES on diagnosing
incomplete template pack expansion cases. In v2, add the missing required
test case for all new patches. v3 Fixes both the test case to compile in
C++11 mode and the message to print out only the type. v4 fixes the testcase
to only target C++11. v5 and v6 fix the test case properly.

gcc/cp/ChangeLog:

* typeck2.c (cxx_incomplete_type_diagnostic): Add missing 
TYPE_EXPANSION_PACK
  check for diagnosticing incomplete types in 
cxx_incomplete_type_diagnostic.


It's already been pointed out to you that it's "diagnosing".


gcc/testsuite/ChangeLog:

* g++.dg/template/PR95672.C: New test.

Signed-off-by: Nicholas Krause 
---
  gcc/cp/typeck2.c| 6 ++
  gcc/testsuite/g++.dg/template/PR95672.C | 3 +++
  2 files changed, 9 insertions(+)
  create mode 100644 gcc/testsuite/g++.dg/template/PR95672.C

diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 5fd3b82fa89..28b32fe0b5a 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -552,6 +552,12 @@ cxx_incomplete_type_diagnostic (location_t loc, const_tree 
value,
   TYPE_NAME (type));
break;
  
+case TYPE_PACK_EXPANSION:

+ emit_diagnostic (diag_kind, loc, 0,


Bad indenting.


Sorry seems Jason didn't catch that.



+"invalid use of pack expansion %qT",
+ type);
+  break;
+
  case TYPENAME_TYPE:
  case DECLTYPE_TYPE:
emit_diagnostic (diag_kind, loc, 0,
diff --git a/gcc/testsuite/g++.dg/template/PR95672.C 
b/gcc/testsuite/g++.dg/template/PR95672.C
new file mode 100644
index 000..fcc3da0a132
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/PR95672.C
@@ -0,0 +1,3 @@
+// PR c++/96572
+// { dg-do compile}
+// { dg-options "-std=c++11" }
+struct g_class : decltype  (auto) ... {  }; // { dg-error "invalid use of pack 
expansion" }


No, this is completely broken.  It passes only because the patch file is
malformed and the last line will be lost when applying the patch, so the
test is an empty file.

Marek



Yes but now that I look at it seems the issue is that the error message 
was changed. My concern is that the test will have to be updated

from the current every time the message changes like the current:
error: expected primary-expression before ‘auto’
1 | struct g_class : decltype  (auto) ... {  };

Is this something I should care about as seems emit_diagnostic and 
friends sometimes do this.


Thanks,
Nick

--
Fundamentally an organism has conscious mental states if and only if 
there is something that it is like to be that organism--something it is 
like for the organism. - Thomas Nagel

[PATCHv7] Handle TYPE_PACK_EXPANSION in cxx_incomplete_type_diagnostic

[Nicholas Krause via Gcc-patches]

From: Nicholas Krause 

This fixs the PR95672 by adding the missing TYPE_PACK_EXPANSION case in
cxx_incomplete_type_diagnostic in order to avoid ICES on diagnosing
incomplete template pack expansion cases. In v2, add the missing required
test case for all new patches. v3 Fixes both the test case to compile in
C++11 mode and the message to print out only the type. v4 fixes the testcase
to only target C++11. v5 and v6 fix the test case properly. v7 fixes running

gcc/cp/ChangeLog:

* typeck2.c (cxx_incomplete_type_diagnostic):Add missing 
  TYPE_EXPANSION_PACK check for diagnosicing incomplete 
  types in cxx_incomplete_type_diagnostic.

gcc/testsuite/ChangeLog:

* g++.dg/template/PR95672.C: New test.

Signed-off-by: Nicholas Krause 
---
 gcc/cp/typeck2.c| 5 +
 gcc/testsuite/g++.dg/template/PR95672.C | 2 ++
 2 files changed, 7 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/template/PR95672.C

diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 5fd3b82fa89..dac135a2e11 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -552,6 +552,11 @@ cxx_incomplete_type_diagnostic (location_t loc, const_tree 
value,
   TYPE_NAME (type));
   break;
 
+case TYPE_PACK_EXPANSION:
+  emit_diagnostic (diag_kind, loc, 0,
+  "invalid use of pack expansion %qT", type);
+  break;
+
 case TYPENAME_TYPE:
 case DECLTYPE_TYPE:
   emit_diagnostic (diag_kind, loc, 0,
diff --git a/gcc/testsuite/g++.dg/template/PR95672.C 
b/gcc/testsuite/g++.dg/template/PR95672.C
new file mode 100644
index 000..104e125287f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/PR95672.C
@@ -0,0 +1,2 @@
+// { dg-do compile { target c++14 } } 
+struct g_class : decltype  (auto) ... {  }; // { dg-error "invalid use of pack 
expansion" } 
-- 
2.20.1

[PATCHv8] c++:Handle TYPE_PACK_EXPANSION in cxx_incomplete_type_diagnostic[PR96752]

[Nicholas Krause via Gcc-patches]

From: Nicholas Krause 

This fixes the PR95672 by adding the missing TYPE_PACK_EXPANSION case in
cxx_incomplete_type_diagnostic in order to avoid ICES on diagnosing
incomplete template pack expansion cases. In v2, add the missing required
test case for all new patches. v3 Fixes both the test case to compile in
C++11 mode and the message to print out only the type. v4 fixes the testcase
to only target C++11. v5 and v6 fix the test case properly. v7 fixes running
the testcase. v8 fixes grammar errors. Tested on  powerpc64le-unknown-linux-gnu.

gcc/cp/ChangeLog:

* typeck2.c (cxx_incomplete_type_diagnostic):
  Add missing TYPE_EXPANSION_PACK check for 
  diagnosing incomplete types in 
  cxx_incomplete_type_diagnostic to fix
  c++/PR95672.

gcc/testsuite/ChangeLog:

* g++.dg/template/PR95672.C: New test.

Signed-off-by: Nicholas Krause 
---
 gcc/cp/typeck2.c| 5 +
 gcc/testsuite/g++.dg/template/PR95672.C | 2 ++
 2 files changed, 7 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/template/PR95672.C

diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index 5fd3b82fa89..dac135a2e11 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -552,6 +552,11 @@ cxx_incomplete_type_diagnostic (location_t loc, const_tree 
value,
   TYPE_NAME (type));
   break;
 
+case TYPE_PACK_EXPANSION:
+  emit_diagnostic (diag_kind, loc, 0,
+  "invalid use of pack expansion %qT", type);
+  break;
+
 case TYPENAME_TYPE:
 case DECLTYPE_TYPE:
   emit_diagnostic (diag_kind, loc, 0,
diff --git a/gcc/testsuite/g++.dg/template/PR95672.C 
b/gcc/testsuite/g++.dg/template/PR95672.C
new file mode 100644
index 000..104e125287f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/PR95672.C
@@ -0,0 +1,2 @@
+// { dg-do compile { target c++14 } } 
+struct g_class : decltype  (auto) ... {  }; // { dg-error "invalid use of pack 
expansion" } 
-- 
2.20.1