Re: [C++ debug PATCH] [PR88534] accept VAR_DECL in class literal template parms

2019-03-27 Thread Martin Sebor

On 3/27/19 3:11 PM, Martin Sebor wrote:

On 3/27/19 4:44 AM, Jonathan Wakely wrote:

On 21/03/19 15:03 -0400, Jason Merrill wrote:

On 3/20/19 6:06 PM, Marek Polacek wrote:

On Wed, Mar 20, 2019 at 10:58:32PM +0100, Jakub Jelinek wrote:

On Wed, Mar 20, 2019 at 05:55:04PM -0400, Marek Polacek wrote:

On Wed, Mar 20, 2019 at 04:56:33PM -0300, Alexandre Oliva wrote:

On Mar 20, 2019, Marek Polacek  wrote:


This test fails with
pr88534.C:58:1: sorry, unimplemented: string literal in function 
template signature


Interesting...  gcc-8 rejected it with an error message rejecting 
the
template parameter, but my latest trunk build (dated Mar 13, 
r269641)

compiles it all right.  Was there a subsequent fix, maybe?  I didn't
realize it was supposed to be rejected.


Ah, that problem only started with r269814, namely this hunk:


Maybe this is done too early and should be postponed to genericization
(perhaps except for TREE_STATIC vars)?


Or skip when DECL is template_parm_object_p.


Or handle it in mangle.c.  I don't see any reason we shouldn't accept

struct A
{
 char c[4];
};

template  struct B { };
B b;

Probably we should use the same mangling whether the initializer for 
c was spelled as a string literal or list of integers.


The thing we still don't want to allow is mangling the *address* of a 
string literal.


Will that help PR 47488 as well?


What I have (attached) accepts all three test cases from PR 47488
(comment #0, #1, #2, and #5) but with different mangling.

The difference in the mangling of the function in the test case
in comment #0 is

   Clang: _Z1gIiEDTcl1fcvT__ELA1_KcEEERKS0_
   GCC 9: _Z1gIiEDTcl1fcvT__ELKc0EEERKS0_

I'm not very familiar with the C++ ABI mangling but from what
I can tell, Clang treats the type as a literal array of 1 const
char element (LA1_Kc) without actually encoding its value, while
with my patch GCC encodes it as a constant literal initializer
consisting of 1 null char (LKc0).  In other words, Clang would
mangle these two the same for the same T:

   template < typename T >
   decltype (f (T(), "123")) g (const T&);

   template < typename T >
   decltype (f (T(), "abc")) g (const T&);

while GCC would mangle them differently.

Which is correct?  Clang's seems correct in this case but would
it also be correct to mangle Jason's B the same as
B?


After some poking around I found the issue below that seems
to answer the question at least for Jason's example:

  https://github.com/itanium-cxx-abi/cxx-abi/issues/64

But this is an open issue that leaves some other questions
unanswered, mainly that of the exact encoding of the literal
(the length of the directly encoded subsequence and the hash
algorithm to compute the hash value of the string).

In any event, using the same encoding as for initializer
lists (i.e., (elt-type, elt-value)...) doesn't follow this
approach.

Should I prototype the solution outlined in the issue for
GCC 9?

I suppose there's still the question of compatibility between
initializer lists and string literals, namely whether this

  B

is supposed to mangle the same as

  B

The ABI issue only talks about string literals and not braced
initializer lists.  I see in Revision History [150204] Add
mangling for braced initializer lists, so presumably there
is a difference, but I can't find what it is.

Martin


Re: [Patch, Fortran, F08] PR 85537: Invalid memory reference at runtime when calling subroutine through procedure pointer

2019-03-27 Thread Janus Weil
Am Mi., 27. März 2019 um 23:32 Uhr schrieb Thomas Koenig
:
>
> Am 27.03.19 um 22:54 schrieb Thomas Koenig:
> > I do not think this is correct.
>
> ... and I missed the point that this was specifically about
> initializations.
>
> So, OK from my side.  Thanks!

Great. Thanks for the reviews, everyone! Committed to trunk as r269980.

Cheers,
Janus


Re: [Patch, Fortran, F08] PR 85537: Invalid memory reference at runtime when calling subroutine through procedure pointer

2019-03-27 Thread Thomas Koenig

Am 27.03.19 um 22:54 schrieb Thomas Koenig:

I do not think this is correct.


... and I missed the point that this was specifically about
initializations.

So, OK from my side.  Thanks!

Regards

Thomas


Re: [Patch, Fortran, F08] PR 85537: Invalid memory reference at runtime when calling subroutine through procedure pointer

2019-03-27 Thread Steve Kargl
On Wed, Mar 27, 2019 at 11:22:49PM +0100, Janus Weil wrote:
> 
> > > the attached patch implements some missing constraints from Fortran
> > > 2008 concerning procedure pointer initialization (cf. the standard
> > > quote in comment #18), thus fixing two accepts-invalid and
> > > ICE-on-invalid problems.
> >
> > I do not think this is correct.
> >
> > F2008:
> >
> > # 12.2.2.4 Procedure pointers
> >
> > #  A procedure pointer is a procedure that has the EXTERNAL and POINTER
> > # attributes; it may be pointer associated with an external procedure, #
> > # an internal procedure, [...]
> >
> > So a procedure pointer can be associated with an internal procedure.
> >
> > Comment#18 from the PR does not quote anything that says that
> > a procedure pointer cannot be associated with an internal procedure.
> 
> Absolutely, a procedure pointer can in principle be associated with an
> internal procedure, and my patch does not change that. What the patch
> rejects is the ('static' in the C sense) pointer initialization of a
> procedure pointer with an internal procedure, which is forbidden per
> F2008:C1220.
> 
> procedure(..), pointer :: pp => internal_proc
> 
> A normal pointer assignment is still allowed per F2008:C729:
> 
> pp => internal_proc
> 
> Hope you agree (and sorry for not being more verbose in my explanation
> in the first place).
> 

Just to add to the confusion/clarification.  Janus's patch
is concerned with an initial-proc-target, which is has restriction
that do not apply to a proc-target.

-- 
Steve


Re: [Patch, Fortran, F08] PR 85537: Invalid memory reference at runtime when calling subroutine through procedure pointer

2019-03-27 Thread Steve Kargl
On Wed, Mar 27, 2019 at 11:12:38PM +0100, Janus Weil wrote:
> Hi Steve,
> 
> > > the attached patch implements some missing constraints from Fortran
> > > 2008 concerning procedure pointer initialization (cf. the standard
> > > quote in comment #18), thus fixing two accepts-invalid and
> > > ICE-on-invalid problems.
> > >
> > > It regtests cleanly on x86_64-linux-gnu. Ok for trunk?
> >
> > Looks OK to me.  Just minor comment.  If there are numbered
> > constraints in either F2008 or F2018 for each of the if()
> > conditionals, can you add a comment.
> 
> I fully agree. Luckily the reference to F08:C1220 is already present
> in gfc_check_assign_symbol (but unfortunately not visible in the
> diff), and it applies in the same way to the one IF clause that is
> already there as well as the two new ones I'm adding, which is why I'm
> not putting in an additional reference.
> 

Ah, ok.  Thanks for the reply.

-- 
Steve


[PATCH] Follow-up regcprop fixes

2019-03-27 Thread Jakub Jelinek
On Wed, Mar 27, 2019 at 04:32:15PM +0100, Jakub Jelinek wrote:
> On Wed, Mar 27, 2019 at 09:24:46AM -0600, Jeff Law wrote:
> > > 2) another thing I've noticed today, we queue up the debug insn changes,
> > > but if we iterate the second time for any bb, we overwrite and thus lose 
> > > any
> > > of the debug insn queued changes from the first iteration, so those 
> > > changes
> > > aren't applied (wrong-debug?)
> > This is actually what worries me, both with the current bits and with
> > any bits that change to a worklist of blocks to reprocess.  As is I
> > think we preserve the debug stuff across the iterations which should
> > keep debug info correct.  Managing that in a world where we queue up the
> > iteration step is going to be tougher.
> 
> The patch I've posted would do df_analyze, all bbs once, then df_analyze,
> process queued debug changes, and if anything needs to be repeated (just
> once), redo the bbs from worklist and repeat the above.
> That seems to me the easiest way to get rid of the per-bb df_analyze calls
> as well as fixing the debug stuff.  Because otherwise, we'd need to somehow
> merge the queued debug insns from the first and second pass and figure out
> how to deal with that.

Here is everything in patch on top of current trunk which contains your
regcprop.c changes.

In addition to the previously mentioned changes, as you've requested it
clears the visited sbitmap between the two passes, fixes clearing of the
all_vd[bb->index].e[regno].debug_insn_changes pointers after
cprop_hardreg_debug processes them and fixes up the n_debug_insn_changes
updates, so that the invariant that it always is the sum of length of the
debug_insn_changes chains for all hard regs in the bb is true (before that
n_debug_insn_changes could be higher, and in the final debug insn processing
case wasn't actually decremented at all, so the == 0 check was useless
there).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk if it
succeeds on some other targets too?

2019-03-27  Jakub Jelinek  

* regcprop.c (copyprop_hardreg_forward_1): Remove redundant INSN_P
test.
(cprop_hardreg_bb, cprop_hardreg_debug): New functions.
(pass_cprop_hardreg::execute): Use those.  Don't repeat bb processing
immediately after first one with df_analyze in between, but rather
process all bbs, queueing ones that need second pass in a worklist,
df_analyze, process queued debug insn changes and if second pass is
needed, process bbs from worklist, df_analyze, process queued debug
insns again.

--- gcc/regcprop.c.jj   2019-03-27 18:13:45.296783680 +0100
+++ gcc/regcprop.c  2019-03-27 21:45:15.382232151 +0100
@@ -801,7 +801,6 @@ copyprop_hardreg_forward_1 (basic_block
   /* Detect obviously dead sets (via REG_UNUSED notes) and remove them.  */
   if (set
  && !RTX_FRAME_RELATED_P (insn)
- && INSN_P (insn)
  && !may_trap_p (set)
  && find_reg_note (insn, REG_UNUSED, SET_DEST (set))
  && !side_effects_p (SET_SRC (set))
@@ -1282,6 +1281,76 @@ public:
 
 }; // class pass_cprop_hardreg
 
+static bool
+cprop_hardreg_bb (basic_block bb, struct value_data *all_vd, sbitmap visited)
+{
+  bitmap_set_bit (visited, bb->index);
+
+  /* If a block has a single predecessor, that we've already
+ processed, begin with the value data that was live at
+ the end of the predecessor block.  */
+  /* ??? Ought to use more intelligent queuing of blocks.  */
+  if (single_pred_p (bb)
+  && bitmap_bit_p (visited, single_pred (bb)->index)
+  && ! (single_pred_edge (bb)->flags & (EDGE_ABNORMAL_CALL | EDGE_EH)))
+{
+  all_vd[bb->index] = all_vd[single_pred (bb)->index];
+  if (all_vd[bb->index].n_debug_insn_changes)
+   {
+ unsigned int regno;
+
+ for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
+   {
+ if (all_vd[bb->index].e[regno].debug_insn_changes)
+   {
+ struct queued_debug_insn_change *cur;
+ for (cur = all_vd[bb->index].e[regno].debug_insn_changes;
+  cur; cur = cur->next)
+   --all_vd[bb->index].n_debug_insn_changes;
+ all_vd[bb->index].e[regno].debug_insn_changes = NULL;
+ if (all_vd[bb->index].n_debug_insn_changes == 0)
+   break;
+   }
+   }
+   }
+}
+  else
+init_value_data (all_vd + bb->index);
+
+  return copyprop_hardreg_forward_1 (bb, all_vd + bb->index);
+}
+
+static void
+cprop_hardreg_debug (function *fun, struct value_data *all_vd)
+{
+  basic_block bb;
+
+  FOR_EACH_BB_FN (bb, fun)
+if (all_vd[bb->index].n_debug_insn_changes)
+  {
+   unsigned int regno;
+   bitmap live;
+
+   live = df_get_live_out (bb);
+   for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
+ if (all_vd[bb->index].e[regno].debug_insn_changes)
+   {
+  

Re: [Patch, Fortran, F08] PR 85537: Invalid memory reference at runtime when calling subroutine through procedure pointer

2019-03-27 Thread Janus Weil
Hi Thomas,

> > the attached patch implements some missing constraints from Fortran
> > 2008 concerning procedure pointer initialization (cf. the standard
> > quote in comment #18), thus fixing two accepts-invalid and
> > ICE-on-invalid problems.
>
> I do not think this is correct.
>
> F2008:
>
> # 12.2.2.4 Procedure pointers
>
> #  A procedure pointer is a procedure that has the EXTERNAL and POINTER
> # attributes; it may be pointer associated with an external procedure, #
> # an internal procedure, [...]
>
> So a procedure pointer can be associated with an internal procedure.
>
> Comment#18 from the PR does not quote anything that says that
> a procedure pointer cannot be associated with an internal procedure.

Absolutely, a procedure pointer can in principle be associated with an
internal procedure, and my patch does not change that. What the patch
rejects is the ('static' in the C sense) pointer initialization of a
procedure pointer with an internal procedure, which is forbidden per
F2008:C1220.

procedure(..), pointer :: pp => internal_proc

A normal pointer assignment is still allowed per F2008:C729:

pp => internal_proc

Hope you agree (and sorry for not being more verbose in my explanation
in the first place).

Cheers,
Janus


[PATCH] Small cleanup in rtl.h

2019-03-27 Thread Jakub Jelinek
Hi!

This cleans up something I found ugly already several times.
NONDEBUG_INSN_P(X) used to be defined as
((GET_CODE (X) == INSN || GET_CODE (X) == DEBUG_INSN
  || GET_CODE (X) == JUMP_INSN || GET_CODE (X) == CALL_INSN)
 && GET_CODE (X) != DEBUG_INSN)
rather than the simpler
(GET_CODE (X) == INSN || GET_CODE (X) == JUMP_INSN || GET_CODE (X) == CALL_INSN)

INSN_P is defined the same as before, just with DEBUG_INSN test at the end.

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

2019-03-27  Jakub Jelinek  

* rtl.h (NONDEBUG_INSN_P): Define as NONJUMP_INSN_P or JUMP_P
or CALL_P instead of INSN_P && !DEBUG_INSN_P.
(INSN_P): Define using NONDEBUG_INSN_P or DEBUG_INSN_P.

--- gcc/rtl.h.jj2019-01-25 23:46:08.058166588 +0100
+++ gcc/rtl.h   2019-03-27 17:35:39.348562954 +0100
@@ -840,7 +840,7 @@ struct GTY(()) rtvec_def {
 #define DEBUG_INSN_P(X) (GET_CODE (X) == DEBUG_INSN)
 
 /* Predicate yielding nonzero iff X is an insn that is not a debug insn.  */
-#define NONDEBUG_INSN_P(X) (INSN_P (X) && !DEBUG_INSN_P (X))
+#define NONDEBUG_INSN_P(X) (NONJUMP_INSN_P (X) || JUMP_P (X) || CALL_P (X))
 
 /* Nonzero if DEBUG_MARKER_INSN_P may possibly hold.  */
 #define MAY_HAVE_DEBUG_MARKER_INSNS debug_nonbind_markers_p
@@ -851,8 +851,7 @@ struct GTY(()) rtvec_def {
   (MAY_HAVE_DEBUG_MARKER_INSNS || MAY_HAVE_DEBUG_BIND_INSNS)
 
 /* Predicate yielding nonzero iff X is a real insn.  */
-#define INSN_P(X) \
-  (NONJUMP_INSN_P (X) || DEBUG_INSN_P (X) || JUMP_P (X) || CALL_P (X))
+#define INSN_P(X) (NONDEBUG_INSN_P (X) || DEBUG_INSN_P (X))
 
 /* Predicate yielding nonzero iff X is a note insn.  */
 #define NOTE_P(X) (GET_CODE (X) == NOTE)

Jakub


Re: [Patch, Fortran, F08] PR 85537: Invalid memory reference at runtime when calling subroutine through procedure pointer

2019-03-27 Thread Janus Weil
Hi Steve,

> > the attached patch implements some missing constraints from Fortran
> > 2008 concerning procedure pointer initialization (cf. the standard
> > quote in comment #18), thus fixing two accepts-invalid and
> > ICE-on-invalid problems.
> >
> > It regtests cleanly on x86_64-linux-gnu. Ok for trunk?
>
> Looks OK to me.  Just minor comment.  If there are numbered
> constraints in either F2008 or F2018 for each of the if()
> conditionals, can you add a comment.

I fully agree. Luckily the reference to F08:C1220 is already present
in gfc_check_assign_symbol (but unfortunately not visible in the
diff), and it applies in the same way to the one IF clause that is
already there as well as the two new ones I'm adding, which is why I'm
not putting in an additional reference.

Cheers,
Janus


Re: [Patch, Fortran, F08] PR 85537: Invalid memory reference at runtime when calling subroutine through procedure pointer

2019-03-27 Thread Thomas Koenig

Hi Janus,


the attached patch implements some missing constraints from Fortran
2008 concerning procedure pointer initialization (cf. the standard
quote in comment #18), thus fixing two accepts-invalid and
ICE-on-invalid problems.


I do not think this is correct.

F2008:

# 12.2.2.4 Procedure pointers

#  A procedure pointer is a procedure that has the EXTERNAL and POINTER
# attributes; it may be pointer associated with an external procedure, # 
# an internal procedure, [...]


So a procedure pointer can be associated with an internal procedure.

Comment#18 from the PR does not quote anything that says that
a procedure pointer cannot be associated with an internal procedure.

Also, for clarification, see Note 12.8, which states

# An internal procedure cannot be invoked using a procedure pointer from
# either Fortran or C after the host instance completes execution,
# because the pointer is then undefined. While the host instance is
# active, however, the internal procedure may be invoked from outside of
# the host procedure scoping unit if that internal procedure was passed
# as an actual argument or is the target of a procedure pointer.

So, we have to support this.

Regards

Thomas


Re: [Patch, Fortran, F08] PR 85537: Invalid memory reference at runtime when calling subroutine through procedure pointer

2019-03-27 Thread Steve Kargl
On Wed, Mar 27, 2019 at 10:35:33PM +0100, Janus Weil wrote:
> 
> the attached patch implements some missing constraints from Fortran
> 2008 concerning procedure pointer initialization (cf. the standard
> quote in comment #18), thus fixing two accepts-invalid and
> ICE-on-invalid problems.
> 
> It regtests cleanly on x86_64-linux-gnu. Ok for trunk?

Looks OK to me.  Just minor comment.  If there are numbered
constraints in either F2008 or F2018 for each of the if()
conditionals, can you add a comment.  Perhaps, something
like.

 /* F2008:C1089. */
> +  if (attr.proc == PROC_INTERNAL)
> + {
> +   gfc_error ("Internal procedure %qs is invalid in "
> +  "procedure pointer initialization at %L",
> +  rvalue->symtree->name, >where);
> +   return false;
> + }

 /* F2008:C1142. */
> +  if (attr.dummy)
> + {
> +   gfc_error ("Dummy procedure %qs is invalid in "
> +  "procedure pointer initialization at %L",
> +  rvalue->symtree->name, >where);
> +   return false;
> + }
>  }

-- 
Steve
20170425 https://www.youtube.com/watch?v=VWUpyCsUKR4
20161221 https://www.youtube.com/watch?v=IbCHE-hONow


C++ PATCH for c++/89852 - ICE with C++11 functional cast with { }

2019-03-27 Thread Marek Polacek
Here we have a non-dependent constructor in a template:

  { VIEW_CONVERT_EXPR(j) }

In digest_init we call massage_init_elt, which calls digest_init_r on the
element.  We convert the element, but we're in a template, so
perform_implicit_conversion added an IMPLICIT_CONV_EXPR around it.  And then
massage_init_elt calls maybe_constant_init on the element and the usual sadness
ensues.

Fixed as below, so that we don't introduce additional template codes in the
middle of converting the element.

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

2019-03-27  Marek Polacek  

PR c++/89852 - ICE with C++11 functional cast with { }.
* semantics.c (finish_compound_literal): Clear processing_template_decl.

* g++.dg/cpp0x/initlist115.C: New test.

diff --git gcc/cp/semantics.c gcc/cp/semantics.c
index a08a2a57f5f..4bbd506d96f 100644
--- gcc/cp/semantics.c
+++ gcc/cp/semantics.c
@@ -2872,6 +2872,11 @@ finish_compound_literal (tree type, tree 
compound_literal,
   if (type == error_mark_node)
return error_mark_node;
 }
+
+  /* Here the constructor is non-dependent, so perform any conversions in
+ non-dependent context as well.  */
+  processing_template_decl_sentinel s;
+
   compound_literal = digest_init_flags (type, compound_literal,
LOOKUP_NORMAL | LOOKUP_NO_NARROWING,
complain);
diff --git gcc/testsuite/g++.dg/cpp0x/initlist115.C 
gcc/testsuite/g++.dg/cpp0x/initlist115.C
new file mode 100644
index 000..ee4b6d4a870
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/initlist115.C
@@ -0,0 +1,18 @@
+// PR c++/89852
+// { dg-do compile { target c++11 } }
+
+struct A {
+  int b;
+};
+
+struct B {
+  A g;
+};
+
+const auto j = A{};
+
+template 
+void k()
+{
+  B{j};
+}


[Patch, Fortran, F08] PR 85537: Invalid memory reference at runtime when calling subroutine through procedure pointer

2019-03-27 Thread Janus Weil
Hi all,

the attached patch implements some missing constraints from Fortran
2008 concerning procedure pointer initialization (cf. the standard
quote in comment #18), thus fixing two accepts-invalid and
ICE-on-invalid problems.

It regtests cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus
diff --git a/gcc/fortran/ChangeLog b/gcc/fortran/ChangeLog
index e1fdb93f3d0..372c517487f 100644
--- a/gcc/fortran/ChangeLog
+++ b/gcc/fortran/ChangeLog
@@ -1,3 +1,9 @@
+2019-03-27  Janus Weil  
+
+	PR fortran/85537
+	* expr.c (gfc_check_assign_symbol): Reject internal and dummy procedures
+	in procedure pointer initialization.
+
 2019-03-27  Paul Thomas  
 
 	PR fortran/88247
diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c
index f54affae18d..478a5557723 100644
--- a/gcc/fortran/expr.c
+++ b/gcc/fortran/expr.c
@@ -4407,6 +4407,20 @@ gfc_check_assign_symbol (gfc_symbol *sym, gfc_component *comp, gfc_expr *rvalue)
 		 "may not be a procedure pointer", >where);
 	  return false;
 	}
+  if (attr.proc == PROC_INTERNAL)
+	{
+	  gfc_error ("Internal procedure %qs is invalid in "
+		 "procedure pointer initialization at %L",
+		 rvalue->symtree->name, >where);
+	  return false;
+	}
+  if (attr.dummy)
+	{
+	  gfc_error ("Dummy procedure %qs is invalid in "
+		 "procedure pointer initialization at %L",
+		 rvalue->symtree->name, >where);
+	  return false;
+	}
 }
 
   return true;
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 0679cb72e52..f6df0b1281c 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2019-03-27  Janus Weil  
+
+	PR fortran/85537
+	* gfortran.dg/dummy_procedure_11.f90: Fix test case.
+	* gfortran.dg/pointer_init_11.f90: New test case.
+
 2019-03-27  Richard Biener  
 
 	* gcc.dg/torture/20190327-1.c: New testcase.
diff --git a/gcc/testsuite/gfortran.dg/dummy_procedure_11.f90 b/gcc/testsuite/gfortran.dg/dummy_procedure_11.f90
index f51c5455c05..3e4b2b1d6f0 100644
--- a/gcc/testsuite/gfortran.dg/dummy_procedure_11.f90
+++ b/gcc/testsuite/gfortran.dg/dummy_procedure_11.f90
@@ -5,16 +5,18 @@
 ! Contributed by Vladimir Fuka 
 
 type :: t
-  procedure(g), pointer, nopass :: ppc => g
+  procedure(g), pointer, nopass :: ppc
 end type
 
-procedure(g), pointer :: pp => g
+procedure(g), pointer :: pp
 type(t)::x
 
 print *, f(g)
 print *, f(g())  ! { dg-error "Expected a procedure for argument" }
+pp => g
 print *, f(pp)
 print *, f(pp()) ! { dg-error "Expected a procedure for argument" }
+x%ppc => g
 print *, f(x%ppc)
 print *, f(x%ppc())  ! { dg-error "Expected a procedure for argument" }
 
diff --git a/gcc/testsuite/gfortran.dg/pointer_init_11.f90 b/gcc/testsuite/gfortran.dg/pointer_init_11.f90
new file mode 100644
index 000..3113e157687
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pointer_init_11.f90
@@ -0,0 +1,44 @@
+! { dg-do compile }
+!
+! PR 85537: [F08] Invalid memory reference at runtime when calling subroutine through procedure pointer
+!
+! Contributed by Tiziano Müller 
+
+module m1
+implicit none
+contains
+subroutine foo()
+  integer :: a
+
+  abstract interface
+subroutine ibar()
+end subroutine
+  end interface
+
+  procedure(ibar), pointer :: bar_ptr => bar_impl  ! { dg-error "invalid in procedure pointer initialization" }
+
+contains
+  subroutine bar_impl()
+write (*,*) "foo"
+a = a + 1
+  end subroutine
+
+end subroutine
+end module
+
+
+module m2
+implicit none
+contains
+subroutine foo(dbar)
+  interface
+subroutine dbar()
+end subroutine
+  end interface
+
+  procedure(dbar), pointer :: bar_ptr => dbar  ! { dg-error "invalid in procedure pointer initialization" }
+
+  call bar_ptr()
+
+end subroutine
+end module


Re: [C++ debug PATCH] [PR88534] accept VAR_DECL in class literal template parms

2019-03-27 Thread Martin Sebor

On 3/27/19 4:44 AM, Jonathan Wakely wrote:

On 21/03/19 15:03 -0400, Jason Merrill wrote:

On 3/20/19 6:06 PM, Marek Polacek wrote:

On Wed, Mar 20, 2019 at 10:58:32PM +0100, Jakub Jelinek wrote:

On Wed, Mar 20, 2019 at 05:55:04PM -0400, Marek Polacek wrote:

On Wed, Mar 20, 2019 at 04:56:33PM -0300, Alexandre Oliva wrote:

On Mar 20, 2019, Marek Polacek  wrote:


This test fails with
pr88534.C:58:1: sorry, unimplemented: string literal in function 
template signature


Interesting...  gcc-8 rejected it with an error message rejecting the
template parameter, but my latest trunk build (dated Mar 13, r269641)
compiles it all right.  Was there a subsequent fix, maybe?  I didn't
realize it was supposed to be rejected.


Ah, that problem only started with r269814, namely this hunk:


Maybe this is done too early and should be postponed to genericization
(perhaps except for TREE_STATIC vars)?


Or skip when DECL is template_parm_object_p.


Or handle it in mangle.c.  I don't see any reason we shouldn't accept

struct A
{
 char c[4];
};

template  struct B { };
B b;

Probably we should use the same mangling whether the initializer for c 
was spelled as a string literal or list of integers.


The thing we still don't want to allow is mangling the *address* of a 
string literal.


Will that help PR 47488 as well?


What I have (attached) accepts all three test cases from PR 47488
(comment #0, #1, #2, and #5) but with different mangling.

The difference in the mangling of the function in the test case
in comment #0 is

  Clang: _Z1gIiEDTcl1fcvT__ELA1_KcEEERKS0_
  GCC 9: _Z1gIiEDTcl1fcvT__ELKc0EEERKS0_

I'm not very familiar with the C++ ABI mangling but from what
I can tell, Clang treats the type as a literal array of 1 const
char element (LA1_Kc) without actually encoding its value, while
with my patch GCC encodes it as a constant literal initializer
consisting of 1 null char (LKc0).  In other words, Clang would
mangle these two the same for the same T:

  template < typename T >
  decltype (f (T(), "123")) g (const T&);

  template < typename T >
  decltype (f (T(), "abc")) g (const T&);

while GCC would mangle them differently.

Which is correct?  Clang's seems correct in this case but would
it also be correct to mangle Jason's B the same as
B?

Martin
PR c++/89833 - sorry, unimplemented: string literal in function template signature

gcc/cp/ChangeLog:

	PR c++/89833
	* mangle.c (write_expression): Convert braced initializer lists
	to STRING_CSTs.
	(write_template_arg_literal): Mangle strings the same as braced
	initializer lists.

Index: gcc/cp/mangle.c
===
--- gcc/cp/mangle.c	(revision 269965)
+++ gcc/cp/mangle.c	(working copy)
@@ -3136,6 +3136,10 @@ write_expression (tree expr)
 }
   else if (code == CONSTRUCTOR)
 {
+  /* Convert braced initializer lists to STRING_CSTs so that
+	 A<"Foo"> and A<{'F', 'o', 'o', 0}> mangle the same.  */
+  expr = braced_lists_to_strings (TREE_TYPE (expr), expr);
+
   vec *elts = CONSTRUCTOR_ELTS (expr);
   unsigned i; tree val;
 
@@ -3354,8 +3358,13 @@ static void
 write_template_arg_literal (const tree value)
 {
   write_char ('L');
-  write_type (TREE_TYPE (value));
 
+  tree valtype = TREE_TYPE (value);
+  if (TREE_CODE (value) == STRING_CST)
+valtype = TREE_TYPE (valtype);
+
+  write_type (valtype);
+
   /* Write a null member pointer value as (type)0, regardless of its
  real representation.  */
   if (null_member_pointer_value_p (value))
@@ -3397,7 +3406,15 @@ write_template_arg_literal (const tree value)
 	break;
 
   case STRING_CST:
-	sorry ("string literal in function template signature");
+	/* Mangle strings the same as braced initializer lists.  */
+	for (const char *p = TREE_STRING_POINTER (value); ; ++p)
+	  {
+	write_unsigned_number (*(const unsigned char*)p);
+	if (!*p)
+	  break;
+	write_string ("EL");
+	write_type (valtype);
+	  }
 	break;
 
   default:


Re: [PATCH] Integration of parallel standard algorithms for c++17

2019-03-27 Thread Thomas Rodgers


Jonathan Wakely writes:

> On 27/03/19 15:51 +0100, Thomas Schwinge wrote:
>>Hi!
>>
>>If that's of any help to document the version dependencies:
>
> Thanks for t his.
>
>>On Fri, 22 Mar 2019 00:04:30 +, Jonathan Wakely  
>>wrote:
>>> I keep forgetting to add that docs for this stuff will be coming some
>>> time next week, describing the TBB dependency that's needed to use
>>> these parallel algos.
>>
>>On an Ubuntu 12.10 x86_64 GNU/Linux system (yes, a bit old by now), I saw
>>all these test UNSUPPORTED.
>>
>>I then installed 'libtbb-dev' (and the implied 'libtbb2'; both version
>>4.0+r233-1), and then saw all? tests FAIL because of:
>>
>>[...]/libstdc++-v3/include/pstl/parallel_backend_tbb.h:25: fatal error: 
>> tbb/task_arena.h: No such file or directory
>>
>>I then manually installed the Ubuntu trusty (14.04LTS) version
>>4.2~20130725-1.1ubuntu1 packages, and then saw all? tests FAIL because
>>of:
>>
>>[...]/libstdc++-v3/include/pstl/parallel_backend_tbb.h:29: error: #error 
>> Intel(R) Threading Building Blocks 2018 is required; older versions are not 
>> supported.
>
> Yes, that's a known dependency.
>
> Tom, I think check_effective_target_tbb-backend should probably check
> the TBB_VERSION_MAJOR macro  in  and fail if an
> older version is detected.
>

Ok, I'll look at adding that check.

> That will only help when running our tests though, not when users try
> to use the algos, so we probably need to fail more gracefully if the
> user has an odler TBB installed. If there's a header which is present
> in TBB-2018 but not older versions then  could check
> for that instead of (or as well as) .
>

Yes the TBB-2018 is a hard dependency in the Intel backend, but I can
likely make this more graceful. There is also a serial 'fallback' backend
currently in the works in the upstream, which would at least give
something to fallback to here.

>
>>So I suppose I need the most recent Ubuntu disco version 2018~U6-4
>>packages.  Because of:
>>
>>dpkg-deb: error: archive 'libtbb-dev_2018~U6-4_amd64.deb' contains not 
>> understood data member control.tar.xz, giving up
>>
>>..., I manually had to convert to '*.gz' files the'.xz' files inside of
>>these '*.deb' archives, which yet still failed to install:
>>
>>dpkg: dependency problems prevent configuration of libtbb2:amd64:
>> libtbb2:amd64 depends on libstdc++6 (>= 7); however:
>>  Version of libstdc++6:amd64 on system is 4.7.2-2ubuntu1.
>>
>>..., which isn't really a problem as I'll obviously be testing against a
>>new-enough GCC/libstdc++ ;-) -- so, installed these packages with
>>'--force-depends-version'.
>>
>>With that, the tests then all PASS for the default multilib, but all?
>>FAIL for '-m32' testing:
>>
>>[...]/ld: cannot find -ltbb
>>
>>There is no 32-bit 'libtbb' available.  I suppose the problem is that
>>'check_effective_target_tbb-backend' just does a 'preprocess' test
>>checking for existence of '', but doesn't actually try to
>>link, whereas all? the test cases specify '-ltbb' via 'dg-options'.
>
> Right. Jakub noticed the same issue.
>
>>That said, instead of specifying:
>>
>>// { dg-options "-std=gnu++17 -ltbb" }
>>// { dg-do run { target c++17 } }
>>// { dg-require-effective-target tbb-backend }
>>
>>... in all? these test case files, isn't there some DejaGnu directive
>>that checks whether support is available (else UNSUPPORTED), while also
>>adding the necessary compiler options (here: '-ltbb')?
>
> I don't think we can do that in one go, can we?
>
> We can add something so { dg-add-options tbb } adds the required
> options, but I don't think it will make it UNSUPPORTED when necessary.
> Maybe I'm missing something.
>
>>Also, shouldn't 'check_effective_target_tbb-backend' (and also some of
>>the other checks in 'libstdc++-v3/testsuite/lib/libstdc++.exp'?) be using
>>'check_v3_target_prop_cached' to avoid checking the same things again and
>>again?
>
> Yes. I think it was probably written before that caching helper was
> added.



Re: [C++ debug PATCH] [PR88534] accept VAR_DECL in class literal template parms

2019-03-27 Thread Jason Merrill

On 3/27/19 12:24 PM, Martin Sebor wrote:

On 3/21/19 1:03 PM, Jason Merrill wrote:

On 3/20/19 6:06 PM, Marek Polacek wrote:

On Wed, Mar 20, 2019 at 10:58:32PM +0100, Jakub Jelinek wrote:

On Wed, Mar 20, 2019 at 05:55:04PM -0400, Marek Polacek wrote:

On Wed, Mar 20, 2019 at 04:56:33PM -0300, Alexandre Oliva wrote:

On Mar 20, 2019, Marek Polacek  wrote:


This test fails with
pr88534.C:58:1: sorry, unimplemented: string literal in function 
template signature


Interesting...  gcc-8 rejected it with an error message rejecting the
template parameter, but my latest trunk build (dated Mar 13, r269641)
compiles it all right.  Was there a subsequent fix, maybe?  I didn't
realize it was supposed to be rejected.


Ah, that problem only started with r269814, namely this hunk:


Maybe this is done too early and should be postponed to genericization
(perhaps except for TREE_STATIC vars)?


Or skip when DECL is template_parm_object_p.


Or handle it in mangle.c.  I don't see any reason we shouldn't accept

struct A
{
   char c[4];
};

template  struct B { };
B b;

Probably we should use the same mangling whether the initializer for c 
was spelled as a string literal or list of integers.


So just loop over the STRING_CST and mangle each character the same
way as it were an element of the braced initializer list?


That's probably best, yes.

Jason


[PATCH] PR88395 Fix Nullptr when compiling with -fconcepts

2019-03-27 Thread 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, committed] Fix PR89834

2019-03-27 Thread Bill Schmidt
Hi,

PR89834 identifies a problem with test case gcc.dg/vect/pr81740-2.c, which 
relies
implicitly on vectorizing unaligned accesses.  This causes it to fail when the
target processor does not support this.  The attached patch requires 
vect_hw_misalign
so that the test is skipped on such targets.

Tested on powerpc64le-unknown-linux-gnu (P9) and powerpc64-unknown-linux-gnu 
(P7),
correct behavior verified.  Committed as requested in the bugzilla (r269978).

Thanks,
Bill


2019-03-27  Bill Schmidt  

* gcc.dg/vect/pr81740-2.c: Require vect_hw_misalign.


Index: gcc/testsuite/gcc.dg/vect/pr81740-2.c
===
--- gcc/testsuite/gcc.dg/vect/pr81740-2.c   (revision 269967)
+++ gcc/testsuite/gcc.dg/vect/pr81740-2.c   (working copy)
@@ -1,5 +1,6 @@
 /* { dg-do run } */
 /* { dg-require-effective-target vect_int } */
+/* { dg-require-effective-target vect_hw_misalign } */
 
 #include "tree-vect.h"
 



[C++ PATCH] PR c++/89831 - error with qualified-id in const member function.

2019-03-27 Thread Jason Merrill
Since the fix for 15272 we were remembering the wrong function to use at
instantiation time, because the type of the SCOPE_REF didn't reflect the
cv-quals of 'this'.  Conveniently, we can fix this by simplifying the code.

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

* semantics.c (finish_non_static_data_member): Use object cv-quals
in scoped case, too.
---
 gcc/cp/semantics.c | 19 +--
 gcc/testsuite/g++.dg/template/scope6.C | 17 +
 gcc/cp/ChangeLog   |  4 
 3 files changed, 30 insertions(+), 10 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/scope6.C

diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index fea269657f9..a08a2a57f5f 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -1861,7 +1861,7 @@ finish_non_static_data_member (tree decl, tree object, 
tree qualifying_scope)
 
   if (current_class_ptr)
 TREE_USED (current_class_ptr) = 1;
-  if (processing_template_decl && !qualifying_scope)
+  if (processing_template_decl)
 {
   tree type = TREE_TYPE (decl);
 
@@ -1882,17 +1882,16 @@ finish_non_static_data_member (tree decl, tree object, 
tree qualifying_scope)
  type = cp_build_qualified_type (type, quals);
}
 
-  ret = (convert_from_reference
- (build_min (COMPONENT_REF, type, object, decl, NULL_TREE)));
+  if (qualifying_scope)
+   /* Wrap this in a SCOPE_REF for now.  */
+   ret = build_qualified_name (type, qualifying_scope, decl,
+   /*template_p=*/false);
+  else
+   ret = (convert_from_reference
+  (build_min (COMPONENT_REF, type, object, decl, NULL_TREE)));
 }
   /* If PROCESSING_TEMPLATE_DECL is nonzero here, then
- QUALIFYING_SCOPE is also non-null.  Wrap this in a SCOPE_REF
- for now.  */
-  else if (processing_template_decl)
-ret = build_qualified_name (TREE_TYPE (decl),
-   qualifying_scope,
-   decl,
-   /*template_p=*/false);
+ QUALIFYING_SCOPE is also non-null.  */
   else
 {
   tree access_type = TREE_TYPE (object);
diff --git a/gcc/testsuite/g++.dg/template/scope6.C 
b/gcc/testsuite/g++.dg/template/scope6.C
new file mode 100644
index 000..6d28fb2ec9e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/scope6.C
@@ -0,0 +1,17 @@
+// PR c++/89831
+
+struct Q { 
+int operator[](int i) { return 0; }
+int operator[](int i) const { return 0; }
+};
+
+struct Base {
+Q x;
+};
+struct X : public Base {
+template 
+void f(T) const {
+int q = Base::x[0];
+}   
+};
+int main() { X().f(3); }
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index bdd5211777b..9803ef947f8 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,5 +1,9 @@
 2019-03-27  Jason Merrill  
 
+   PR c++/89831 - error with qualified-id in const member function.
+   * semantics.c (finish_non_static_data_member): Use object cv-quals
+   in scoped case, too.
+
PR c++/89421 - ICE with lambda in template parameter list.
* parser.c (cp_parser_lambda_expression): Also reject a lambda in a
template parameter list before C++20.

base-commit: 8d96c4f034b9c5d24a5132b10cf2a074fb35dc64
-- 
2.20.1



[Ping] [Patch, fortran] PR83515, PR85797 - ICE in gfc_element_size

2019-03-27 Thread Harald Anlauf
Ping!?

On 03/20/19 23:20, Harald Anlauf wrote:
> The PRs originated in gfc_element_size lacking a treatment of
> procedure pointers, which has been added.  The testcase is currently
> a pure compile test.  When a reduced run-time test for PR83515
> becomes available, it will be added to the testsuite.
>
> Regtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
>
> Thanks,
> Harald
>
> 2019-03-20  Harald Anlauf  
>
>   PR fortran/83515
>   PR fortran/85797
>   * trans-types.c (gfc_typenode_for_spec): Handle conversion for
>   procedure pointers.
>   * target-memory.c (gfc_element_size): Handle size determination
>   for procedure pointers.
>
> 2019-03-20  Harald Anlauf  
>
>   PR fortran/83515
>   PR fortran/85797
>   * gfortran.dg/pr85797.f90: New test.
>



Re: [Patch] Bug85667-(x86_64) ms_abi rules aren't followed when returning short structs with float values(32-bit)

2019-03-27 Thread Uros Bizjak
On Wed, Mar 27, 2019 at 3:08 PM Eric Botcazou  wrote:
>
> > I modified the patch.
> > Please let me know your thoughts.
>
> Thanks.  This version is certainly better, but I cannot approve it myself.
>
> Uros, given that the bug was fixed in 64-bit mode for GCC 9, I think that it
> would make sense to have it fixed in 32-bit mode too.

I would really like for MS_ABI maintainer to approve the patch, but
this particular patch looks straightforward, so I'll approve it
myself.

So, OK for mainline and eventual backporting to release branches.

Thanks,
Uros.


C++ PATCH for c++/89836 - bool constant expression and explicit conversions

2019-03-27 Thread Marek Polacek
I noticed that this test doesn't compile because build_converted_constant_expr
failed to consider explicit conversion functions.  That's wrong: while Core
Issue 1981 specifies that explicit conversion functions are not considered for
contextual conversions, they are considered in contextual conversions to bool,
as defined in Core 2039.

Fixed by adding a new wrapper that uses LOOKUP_NORMAL instead of
LOOKUP_IMPLICIT.

Besides [dcl.fct.spec], these: [except.spec], [dcl.dcl]/6 (static_assert),
[stmt.if] (constexpr if) all talk about "a contextually converted constant
expression of type bool", so it would seem to make sense to use
build_converted_constant_bool_expr for them also.  E.g. use that instead of
perform_implicit_conversion_flags in build_noexcept_spec.  But that doesn't
work for this test:

int e();
int fn() noexcept(e);

because build_converted_constant_expr would issue a conversion error.  We're
converting "e" to "bool".  We have a ck_lvalue conversion from "int f()" to
"int (*f)()" and then a ck_std conversion from "int (*f)()" to bool.  Those
should work fine but we issue an error for the ck_std conversion.

Also build_converted_constant_expr doesn't have the processing_template_decl
handling that perform_implicit_conversion_flags does, which (I belive) lead me
to changing check_narrowing to use maybe_constant_value instead of
fold_non_dependent_expr.

Anyway, this patch should be safe.

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

2019-03-27  Marek Polacek  

PR c++/89836 - bool constant expression and explicit conversions.
* call.c (build_converted_constant_expr_internal): New function,
renamed from...
(build_converted_constant_expr): ...this.  New.
(build_converted_constant_bool_expr): New.
* cp-tree.h (build_converted_constant_bool_expr): Declare.
* decl.c (build_explicit_specifier): Call
build_converted_constant_bool_expr.

* g++.dg/cpp2a/explicit15.C: New test.

diff --git gcc/cp/call.c gcc/cp/call.c
index 9efca735b16..bc5179416a5 100644
--- gcc/cp/call.c
+++ gcc/cp/call.c
@@ -4175,18 +4175,11 @@ build_user_type_conversion (tree totype, tree expr, int 
flags,
   return ret;
 }
 
-/* Subroutine of convert_nontype_argument.
-
-   EXPR is an expression used in a context that requires a converted
-   constant-expression, such as a template non-type parameter.  Do any
-   necessary conversions (that are permitted for converted
-   constant-expressions) to convert it to the desired type.
-
-   If conversion is successful, returns the converted expression;
-   otherwise, returns error_mark_node.  */
+/* Worker for build_converted_constant_expr.  */
 
-tree
-build_converted_constant_expr (tree type, tree expr, tsubst_flags_t complain)
+static tree
+build_converted_constant_expr_internal (tree type, tree expr,
+   int flags, tsubst_flags_t complain)
 {
   conversion *conv;
   void *p;
@@ -4200,8 +4193,7 @@ build_converted_constant_expr (tree type, tree expr, 
tsubst_flags_t complain)
   p = conversion_obstack_alloc (0);
 
   conv = implicit_conversion (type, TREE_TYPE (expr), expr,
- /*c_cast_p=*/false,
- LOOKUP_IMPLICIT, complain);
+ /*c_cast_p=*/false, flags, complain);
 
   /* A converted constant expression of type T is an expression, implicitly
  converted to type T, where the converted expression is a constant
@@ -4304,6 +4296,38 @@ build_converted_constant_expr (tree type, tree expr, 
tsubst_flags_t complain)
   return expr;
 }
 
+/* Subroutine of convert_nontype_argument.
+
+   EXPR is an expression used in a context that requires a converted
+   constant-expression, such as a template non-type parameter.  Do any
+   necessary conversions (that are permitted for converted
+   constant-expressions) to convert it to the desired type.
+
+   This function doesn't consider explicit conversion functions.  If
+   you mean to use "a contextually converted constant expression of type
+   bool", use build_converted_constant_bool_expr.
+
+   If conversion is successful, returns the converted expression;
+   otherwise, returns error_mark_node.  */
+
+tree
+build_converted_constant_expr (tree type, tree expr, tsubst_flags_t complain)
+{
+  return build_converted_constant_expr_internal (type, expr, LOOKUP_IMPLICIT,
+complain);
+}
+
+/* Used to create "a contextually converted constant expression of type
+   bool".  This differs from build_converted_constant_expr in that it
+   also considers explicit conversion functions.  */
+
+tree
+build_converted_constant_bool_expr (tree expr, tsubst_flags_t complain)
+{
+  return build_converted_constant_expr_internal (boolean_type_node, expr,
+LOOKUP_NORMAL, complain);
+}
+
 /* Do any initial processing on the arguments to a function call.  */
 
 static vec *
diff 

Re: [Patch, fortran] PR89841 - improper descriptor information passed to C

2019-03-27 Thread Steve Kargl
On Wed, Mar 27, 2019 at 06:50:41PM +, Paul Richard Thomas wrote:
> This corrects a screw-up on my part. The attribute field of the CFI
> descriptor must be set by the formal argument in the interface and not
> the actual argument.
> 
> Most of the work was in correcting
> 
> Bootstrapped and regtested on FC29/x86_64 - OK for trunk?
> 

OK.

-- 
Steve


[Patch, fortran] PR89841 - improper descriptor information passed to C

2019-03-27 Thread Paul Richard Thomas
This corrects a screw-up on my part. The attribute field of the CFI
descriptor must be set by the formal argument in the interface and not
the actual argument.

Most of the work was in correcting

Bootstrapped and regtested on FC29/x86_64 - OK for trunk?

Cheers

Paul

2019-03-27  Paul Thomas  

PR fortran/89841
* trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Use the formal
argument attributes rather than those of the actual argument.

2019-03-27  Paul Thomas  

PR fortran/89841
* gfortran.dg/ISO_Fortran_binding_1.f90: Change the interfaces
for c_deallocate, c_allocate and c_assumed_size so that the
attributes of the array arguments are correct and are typed.
* gfortran.dg/ISO_Fortran_binding_7.f90: New test.
* gfortran.dg/ISO_Fortran_binding_7.c: Additional source.
Index: gcc/fortran/trans-expr.c
===
*** gcc/fortran/trans-expr.c	(revision 269962)
--- gcc/fortran/trans-expr.c	(working copy)
*** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
*** 4998,5006 
attribute = 2;
if (!e->rank || gfc_get_full_arrayspec_from_expr (e))
  {
!   if (attr.pointer)
  	attribute = 0;
!   else if (attr.allocatable)
  	attribute = 1;
  }
  
--- 4998,5006 
attribute = 2;
if (!e->rank || gfc_get_full_arrayspec_from_expr (e))
  {
!   if (fsym->attr.pointer)
  	attribute = 0;
!   else if (fsym->attr.allocatable)
  	attribute = 1;
  }
  
Index: gcc/testsuite/gfortran.dg/ISO_Fortran_binding_1.f90
===
*** gcc/testsuite/gfortran.dg/ISO_Fortran_binding_1.f90	(revision 269961)
--- gcc/testsuite/gfortran.dg/ISO_Fortran_binding_1.f90	(working copy)
***
*** 25,37 
  FUNCTION c_deallocate(a) BIND(C, NAME="deallocate_c") RESULT(err)
USE, INTRINSIC :: ISO_C_BINDING
INTEGER(C_INT) :: err
!   type(*), DIMENSION(..) :: a
  END FUNCTION c_deallocate
  
  FUNCTION c_allocate(a, lower, upper) BIND(C, NAME="allocate_c") RESULT(err)
USE, INTRINSIC :: ISO_C_BINDING
INTEGER(C_INT) :: err
!   type(*), DIMENSION(..) :: a
integer(C_INTPTR_T), DIMENSION(15) :: lower, upper
  END FUNCTION c_allocate
  
--- 25,37 
  FUNCTION c_deallocate(a) BIND(C, NAME="deallocate_c") RESULT(err)
USE, INTRINSIC :: ISO_C_BINDING
INTEGER(C_INT) :: err
!   INTEGER(C_INT), DIMENSION(..), allocatable :: a
  END FUNCTION c_deallocate
  
  FUNCTION c_allocate(a, lower, upper) BIND(C, NAME="allocate_c") RESULT(err)
USE, INTRINSIC :: ISO_C_BINDING
INTEGER(C_INT) :: err
!   INTEGER(C_INT), DIMENSION(..), allocatable :: a
integer(C_INTPTR_T), DIMENSION(15) :: lower, upper
  END FUNCTION c_allocate
  
***
*** 67,73 
USE, INTRINSIC :: ISO_C_BINDING
INTEGER(C_INT) :: err
INTEGER(C_INT), dimension(2) :: lbounds
!   type(*), DIMENSION(..) :: a
  END FUNCTION c_setpointer
  
  FUNCTION c_assumed_size(a) BIND(C, NAME="assumed_size_c") RESULT(err)
--- 67,73 
USE, INTRINSIC :: ISO_C_BINDING
INTEGER(C_INT) :: err
INTEGER(C_INT), dimension(2) :: lbounds
!   INTEGER(C_INT), DIMENSION(..), pointer :: a
  END FUNCTION c_setpointer
  
  FUNCTION c_assumed_size(a) BIND(C, NAME="assumed_size_c") RESULT(err)
Index: gcc/testsuite/gfortran.dg/ISO_Fortran_binding_7.c
===
*** gcc/testsuite/gfortran.dg/ISO_Fortran_binding_7.c	(nonexistent)
--- gcc/testsuite/gfortran.dg/ISO_Fortran_binding_7.c	(working copy)
***
*** 0 
--- 1,102 
+ /* Test the fix for PR89841.  */
+ 
+ /* Contributed by Reinhold Bader   */
+ 
+ #include "../../../libgfortran/ISO_Fortran_binding.h"
+ #include 
+ #include 
+ #include 
+ 
+ typedef struct
+   {
+ int i;
+ float r[2];
+   } cstruct;
+ 
+ 
+ int Psuba(CFI_cdesc_t *this, CFI_cdesc_t *that, int Dcase) {
+ int status = 0;
+ cstruct *cu;
+ float *ct;
+ CFI_dim_t *dim;
+ if (this->elem_len != sizeof(float))
+   {
+ 	printf("FAIL: Dcase %i - this->elem_len %i\n",Dcase, (int) this->elem_len);
+ 	status++;
+   }
+ if (this->type != CFI_type_float)
+   {
+ 	printf("FAIL: Dcase %i - this->type\n", Dcase);
+ 	status++;
+   }
+ if (this->rank != 2)
+   {
+ 	printf("FAIL: Dcase %i - this->rank %i\n",Dcase,this->rank);
+ 	status++;
+   }
+ if (this->attribute != CFI_attribute_other)
+   {
+ 	printf("FAIL: Dcase %i - this->attribute\n", Dcase);
+ 	status++;
+   }
+ 
+ dim = this->dim;
+ if (dim[0].lower_bound != 0 || dim[0].extent != 3) 
+   {
+ 	printf("FAIL: Dcase %i - dim[0] %i %i %i\n",Dcase, (int) dim[0].lower_bound,
+ 	  (int)dim[0].extent,(int)dim[0].sm);
+ 	status++;
+   }
+ if (dim[1].lower_bound != 0 || dim[1].extent != 7)
+   {
+ 	printf("FAIL: 

Re: C++ PATCH for c++/87145 - bogus error converting class type in template argument list

2019-03-27 Thread Marek Polacek
Ping.

On Wed, Mar 20, 2019 at 04:12:13PM -0400, Marek Polacek wrote:
> The fix for 77656 caused us to call convert_nontype_argument even for
> value-dependent arguments, to perform the conversion in order to avoid
> a bogus warning.
> 
> In this case, the argument is Pod{N}.  The call to 
> build_converted_constant_expr
> in convert_nontype_argument produces Pod::operator Enum(&{N}).  It doesn't 
> crash
> because we're in a template and build_address no longer crashes on 
> CONSTRUCTORs
> in a template.
> 
> Then when instantiating the function foo we substitute its argument: &{N}.  So
> we're in tsubst_copy_and_build/ADDR_EXPR.  The call to
> tsubst_non_call_postfix_expression turns {N} into TARGET_EXPR  {.val=2}>.
> Then build_x_unary_op is supposed to put the ADDR_EXPR back.  It calls
> cp_build_addr_expr_strict.  But it's *strict*, so the prvalue of class type
> TARGET_EXPR  isn't allowed -> error.
> 
> It's _strict since ,
> that seem like a desirable change, and we had a warning for taking the address
> of a TARGET_EXPR in build_x_unary_op even before that.
> 
> So rather than messing with _strict, let's avoid this scenario altogether.
> I checked whether we have a case in the testsuite that results in convert_like
> getting a value-dependent CONSTRUCTOR, but found none.
> 
> With this patch, we avoid it, and only call convert_nontype_argument after
> substitution, at which point maybe_constant_value will be able to evaluate
> the conversion to a constant.
> 
> This problem doesn't occur when passing Pod{N} as an argument to a function,
> or using it as an array dimension; seems we avoid converting the argument if
> it's value-dependent.
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
> 
> 2019-03-20  Marek Polacek  
> 
>   PR c++/87145 - bogus error converting class type in template arg list.
>   * pt.c (convert_template_argument): Don't call convert_nontype_argument
>   if it could involve calling a conversion function with a value-dependent
>   constructor as its argument.
> 
>   * g++.dg/cpp0x/constexpr-conv3.C: New test.
>   * g++.dg/cpp0x/constexpr-conv4.C: New test.
> 
> diff --git gcc/cp/pt.c gcc/cp/pt.c
> index 0acc16d1b92..6878583d99b 100644
> --- gcc/cp/pt.c
> +++ gcc/cp/pt.c
> @@ -8056,7 +8056,16 @@ convert_template_argument (tree parm,
>   t = canonicalize_type_argument (t, complain);
>  
>if (!type_dependent_expression_p (orig_arg)
> -   && !uses_template_parms (t))
> +   && !uses_template_parms (t)
> +   /* This might trigger calling a conversion function with
> +  a value-dependent argument, which could invoke taking
> +  the address of a temporary representing the result of
> +  the conversion.  */
> +   && !(COMPOUND_LITERAL_P (orig_arg)
> +&& MAYBE_CLASS_TYPE_P (TREE_TYPE (orig_arg))
> +&& TYPE_HAS_CONVERSION (TREE_TYPE (orig_arg))
> +&& INTEGRAL_OR_ENUMERATION_TYPE_P (t)
> +&& value_dependent_expression_p (orig_arg)))
>   /* We used to call digest_init here.  However, digest_init
>  will report errors, which we don't want when complain
>  is zero.  More importantly, digest_init will try too
> @@ -8092,7 +8101,7 @@ convert_template_argument (tree parm,
> && TREE_CODE (TREE_TYPE (innertype)) == FUNCTION_TYPE
> && TREE_OPERAND_LENGTH (inner) > 0
>&& reject_gcc_builtin (TREE_OPERAND (inner, 0)))
> -  return error_mark_node;
> + return error_mark_node;
>  }
>  
>if (TREE_CODE (val) == SCOPE_REF)
> diff --git gcc/testsuite/g++.dg/cpp0x/constexpr-conv3.C 
> gcc/testsuite/g++.dg/cpp0x/constexpr-conv3.C
> new file mode 100644
> index 000..3f47c58cd2a
> --- /dev/null
> +++ gcc/testsuite/g++.dg/cpp0x/constexpr-conv3.C
> @@ -0,0 +1,25 @@
> +// PR c++/87145
> +// { dg-do compile { target c++11 } }
> +
> +template struct integral_constant {
> +  static constexpr T value = t;
> +};
> +
> +enum class Enum : unsigned {};
> +
> +struct Pod {
> +  unsigned val;
> +
> +  constexpr operator Enum() const {
> +return static_cast(val);
> +  }
> +};
> +
> +template
> +constexpr void foo() {
> +  using Foo = integral_constant;
> +}
> +
> +int main() {
> +  foo<2>();
> +}
> diff --git gcc/testsuite/g++.dg/cpp0x/constexpr-conv4.C 
> gcc/testsuite/g++.dg/cpp0x/constexpr-conv4.C
> new file mode 100644
> index 000..f4e3f00a585
> --- /dev/null
> +++ gcc/testsuite/g++.dg/cpp0x/constexpr-conv4.C
> @@ -0,0 +1,25 @@
> +// PR c++/87145
> +// { dg-do compile { target c++11 } }
> +
> +struct S {
> +  int val;
> +
> +  constexpr operator int() const {
> +return static_cast(val);
> +  }
> +};
> +
> +template
> +struct F { };
> +
> +template
> +constexpr void foo() {
> +  F f;
> +  F f2;
> +}
> +
> +int
> +main()
> +{
> +  foo<2>();
> +}

Marek


Re: PING [RFA patch, Fortran] PR60144 - Misleading error message when missing "then" after "if" and "else if"

2019-03-27 Thread Thomas Koenig

Hi Dominique,



Patch posted at https://gcc.gnu.org/ml/fortran/2019-03/msg00098.html


I think the patch is OK.  I think the patch is probably appropriate for
stage 1 once that reopens.

Regards

Thomas


[C++ PATCH] PR c++/89241 - ICE with lambda in template parameter list.

2019-03-27 Thread Jason Merrill
We were getting confused by a lambda in template definition context that
isn't actually in the scope of any templated entity.  Fixed by telling
type_dependent_expression_p that such a lambda is type-dependent even if we
can't tell that from its closure type.  I've also restored the error for
defining a non-lambda class in a default template argument, and for a lambda
befor C++20.

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

* parser.c (cp_parser_lambda_expression): Also reject a lambda in a
template parameter list before C++20.
* pt.c (type_dependent_expression_p): True for LAMBDA_EXPR.
* semantics.c (begin_class_definition): Restore error about defining
non-lambda class in template parm list.
---
 gcc/cp/parser.c  |  2 +-
 gcc/cp/pt.c  |  6 ++
 gcc/cp/semantics.c   |  6 ++
 gcc/testsuite/g++.dg/cpp2a/lambda-uneval10.C | 12 
 gcc/testsuite/g++.dg/ext/complit16.C |  5 +
 gcc/cp/ChangeLog |  9 +
 6 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-uneval10.C
 create mode 100644 gcc/testsuite/g++.dg/ext/complit16.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 4119d2a3265..348e4acf2d1 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -10397,7 +10397,7 @@ cp_parser_lambda_expression (cp_parser* parser)
}
   ok = false;
 }
-  else if (parser->in_template_argument_list_p)
+  else if (parser->in_template_argument_list_p || processing_template_parmlist)
 {
   if (!token->error_reported)
{
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index dc982f3ed55..229b34a197e 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -25692,6 +25692,12 @@ type_dependent_expression_p (tree expression)
   || TREE_CODE (expression) == WILDCARD_DECL)
 return true;
 
+  /* A lambda-expression in template context is dependent.  dependent_type_p is
+ true for a lambda in the scope of a class or function template, but that
+ doesn't cover all template contexts, like a default template argument.  */
+  if (TREE_CODE (expression) == LAMBDA_EXPR)
+return true;
+
   /* A fold expression is type-dependent. */
   if (TREE_CODE (expression) == UNARY_LEFT_FOLD_EXPR
   || TREE_CODE (expression) == UNARY_RIGHT_FOLD_EXPR
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index d7e97e0510d..fea269657f9 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -3043,6 +3043,12 @@ begin_class_definition (tree t)
   if (error_operand_p (t) || error_operand_p (TYPE_MAIN_DECL (t)))
 return error_mark_node;
 
+  if (processing_template_parmlist && !LAMBDA_TYPE_P (t))
+{
+  error ("definition of %q#T inside template parameter list", t);
+  return error_mark_node;
+}
+
   /* According to the C++ ABI, decimal classes defined in ISO/IEC TR 24733
  are passed the same as decimal scalar types.  */
   if (TREE_CODE (t) == RECORD_TYPE
diff --git a/gcc/testsuite/g++.dg/cpp2a/lambda-uneval10.C 
b/gcc/testsuite/g++.dg/cpp2a/lambda-uneval10.C
new file mode 100644
index 000..86065ebca64
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/lambda-uneval10.C
@@ -0,0 +1,12 @@
+// PR c++/89421
+
+template  // { dg-message "lambda" "" { target 
c++17_down } }
+struct B
+{
+  static const int i = I;
+};
+
+#if __cplusplus > 201703L
+B<> b;
+static_assert (b.i == 1);
+#endif
diff --git a/gcc/testsuite/g++.dg/ext/complit16.C 
b/gcc/testsuite/g++.dg/ext/complit16.C
new file mode 100644
index 000..5625cedd94e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/complit16.C
@@ -0,0 +1,5 @@
+// { dg-options "" }
+
+template  struct B{}; // { dg-error "" }
+
+B<0> b;
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index bce9b108723..48071948f4e 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,12 @@
+2019-03-27  Jason Merrill  
+
+   PR c++/89241 - ICE with lambda in template parameter list.
+   * parser.c (cp_parser_lambda_expression): Also reject a lambda in a
+   template parameter list before C++20.
+   * pt.c (type_dependent_expression_p): True for LAMBDA_EXPR.
+   * semantics.c (begin_class_definition): Restore error about defining
+   non-lambda class in template parm list.
+
 2019-03-26  Jason Merrill  
 
PR c++/86932 - missed SFINAE with empty pack.

base-commit: b099e9c8685035e9c0809fc10cebc66bd540156d
-- 
2.20.1



Re: [PATCH] Move stack protector epilogue before loading return hard reg(s) from pseudo(s) (PR rtl-optimization/87485)

2019-03-27 Thread Jeff Law
On 2/2/19 3:22 AM, Jakub Jelinek wrote:
> On Fri, Feb 01, 2019 at 11:52:04PM +0100, Eric Botcazou wrote:
>>> So, can we e.g. keep emitting the epilogue where it is now for
>>> naked_return_label != NULL_RTX and move it otherwise?
>>> For __builtin_return the setter and use of the hard register won't be
>>> adjacent in any case.
>>
>> See my comment in the audit trail of the PR; I'd suspend it and go to bed. 
>> ;-)
> 
> While the set of -fno- and -f options in some PRs are unlikely to be used by
> people in the wild, often those PRs uncover latent bugs that could cause
> serious wrong-code or ICEs, of course not always.  So IMHO we shouldn't
> ignore those PRs, especially if they are regressions.
> 
> In the meantime, I've bootstrapped/regtested successfully following version
> of the patch that should fix the builtin return case.
> In addition to normal {x86_64,i686}-linux bootstrap/regtest I've done a
> distro build where we 1) build the compiler itself with
> -fstack-protector-strong 2) run testsuite with
> --tool-test=\{,-fstack-protector-strong\}, so far on
> {x86_64,i686,powerpc64le}-linux, other targets still pending.
> 
> The only "regression" was gcc.target/i386/call-1.c with
> -fstack-protector-strong, but that is because the test is invalid:
> void set_eax(int val)
> {
>   __asm__ __volatile__ ("mov %0, %%eax" : : "m" (val));
> }
> - missing "eax" clobber or "=a" (dummy) output and when set_eax is inlined
> that can break optimizations badly.  Of course, in addition to fixing that,
> I'd expect if the tests wants to test what it originally wanted to test, it
> needs to disable inlining or perhaps all IPA opts, not sure if just for
> set_eax or also for foo/bar.  Perhaps we can keep the testcase as is with
> the "eax" clobber and add another one with __attribute__((noipa)) on
> set_eax/foo/bar.
> 
> 2019-02-01  Jakub Jelinek  
> 
>   PR rtl-optimization/87485
>   * function.c (expand_function_end): Move stack_protect_epilogue
>   before loading of return value into hard register(s).
> 
>   * gcc.dg/pr87485.c: New test.
OK.  Though you do need to twiddle the call-1.c test in some manner.
I've got no strong opinions on setting noipa on everything in the test
vs twiddling the clobbers list.  Your call on best approach for
adjusting the call-1 test.

jeff


[committed gcn] Fix scc clobber in movdi_symbol

2019-03-27 Thread Andrew Stubbs
This patch fixes a bug in which symbol load instructions created by LRA 
clobbered the SCC register, and therefore caused rare bad code bugs.


The patch now saves and restores SCC, if called during LRA. The save and 
restore instructions are later optimized away if SCC isn't actually live.


Tested for amdgcn-unknown-amdhsa with no regressions.

Andrew Stubbs
Mentor Graphics / CodeSourcery
Fix scc clobber in movdi_symbol.

2019-03-27  Andrew Stubbs  

	gcc/
	* config/gcn/gcn.md (CC_SAVE_REG): New constant.
	(movdi): Call gen_movdi_symbol_save_scc.
	(gen_movdi_symbol_save_scc): New insn and split.

diff --git a/gcc/config/gcn/gcn.md b/gcc/config/gcn/gcn.md
index 4573a4ce32f..2b805a73c56 100644
--- a/gcc/config/gcn/gcn.md
+++ b/gcc/config/gcn/gcn.md
@@ -24,6 +24,7 @@
 ; Named registers
 (define_constants
   [(FIRST_SGPR_REG		 0)
+   (CC_SAVE_REG			 22)
(LAST_SGPR_REG		 101)
(FLAT_SCRATCH_REG		 102)
(FLAT_SCRATCH_LO_REG		 102)
@@ -403,7 +404,10 @@
 	&& (GET_CODE (operands[1]) == SYMBOL_REF
 	|| GET_CODE (operands[1]) == LABEL_REF))
   {
-	emit_insn (gen_movdi_symbol (operands[0], operands[1]));
+	if (lra_in_progress)
+	  emit_insn (gen_movdi_symbol_save_scc (operands[0], operands[1]));
+	else
+	  emit_insn (gen_movdi_symbol (operands[0], operands[1]));
 	DONE;
   }
   })
@@ -826,6 +830,19 @@
  [(set_attr "type" "mult")
   (set_attr "length" "32")])
 
+(define_insn_and_split "movdi_symbol_save_scc"
+ [(set (match_operand:DI 0 "nonimmediate_operand" "=Sg")
+   (match_operand:DI 1 "general_operand" "Y"))
+  (clobber (reg:BI CC_SAVE_REG))]
+ "GET_CODE (operands[1]) == SYMBOL_REF || GET_CODE (operands[1]) == LABEL_REF
+  && (lra_in_progress || reload_completed)"
+ "#"
+ "reload_completed"
+ [(set (reg:BI CC_SAVE_REG) (reg:BI SCC_REG))
+  (parallel [(set (match_dup 0) (match_dup 1))
+	 (clobber (reg:BI SCC_REG))])
+  (set (reg:BI SCC_REG) (reg:BI CC_SAVE_REG))])
+
 (define_insn "gcn_indirect_call"
   [(call (mem (match_operand:DI 0 "register_operand" "Sg"))
 	 (match_operand 1 "" ""))


Re: [PATCH, asmcons] Fix PR rtl-optimization/89313: ICE in process_alt_operands, at lra-constraints.c:2962

2019-03-27 Thread Peter Bergner
On 3/25/19 3:36 PM, Jeff Law wrote:
> On 2/20/19 8:19 PM, Peter Bergner wrote:
>> gcc/
>>  PR rtl-optimization/89313
>>  * function.c (matching_constraint_num): New static function.
>>  (match_asm_constraints_1): Use it.  Fixup white space and comment.
>>  Don't replace inputs with non-matching constraints which conflict
>>  with early clobber outputs.
>>
>> gcc/testsuite/
>>  PR rtl-optimization/89313
>>  * gcc.dg/pr89313.c: New test.
> OK.

I did another round of bootstrap and regtesting, since I was on vacation
and trunk has changed.  The testing was still clean, so it's committed now.
Thanks!

Peter



Re: [committed][PR rtl-optimization/87761] Limited iteration in regcprop to pick up secondary opportunities

2019-03-27 Thread Jakub Jelinek
On Wed, Mar 27, 2019 at 10:20:17AM -0600, Jeff Law wrote:
> --- a/gcc/regcprop.c
> +++ b/gcc/regcprop.c
> @@ -800,9 +800,9 @@ copyprop_hardreg_forward_1 (basic_block bb, struct 
> value_data *vd)
>  
>/* Detect obviously dead sets (via REG_UNUSED notes) and remove them.  
> */
>if (set
> -   && !may_trap_p (set)
> && !RTX_FRAME_RELATED_P (insn)
> && INSN_P (insn)
> +   && !may_trap_p (set)

One more nitpick, I think the && INSN (insn) test is redundant there.
A few lines above this we have:
  if (!NONDEBUG_INSN_P (insn))
{
  ...
  break; or continue;
}

  // code that doesn't change insn, at most delete_insn + break/continue

and as
#define NONDEBUG_INSN_P(X) (INSN_P (X) && !DEBUG_INSN_P (X))
INSN_P (insn) must be always true.

Jakub


Re: [C++ debug PATCH] [PR88534] accept VAR_DECL in class literal template parms

2019-03-27 Thread Martin Sebor

On 3/21/19 1:03 PM, Jason Merrill wrote:

On 3/20/19 6:06 PM, Marek Polacek wrote:

On Wed, Mar 20, 2019 at 10:58:32PM +0100, Jakub Jelinek wrote:

On Wed, Mar 20, 2019 at 05:55:04PM -0400, Marek Polacek wrote:

On Wed, Mar 20, 2019 at 04:56:33PM -0300, Alexandre Oliva wrote:

On Mar 20, 2019, Marek Polacek  wrote:


This test fails with
pr88534.C:58:1: sorry, unimplemented: string literal in function 
template signature


Interesting...  gcc-8 rejected it with an error message rejecting the
template parameter, but my latest trunk build (dated Mar 13, r269641)
compiles it all right.  Was there a subsequent fix, maybe?  I didn't
realize it was supposed to be rejected.


Ah, that problem only started with r269814, namely this hunk:


Maybe this is done too early and should be postponed to genericization
(perhaps except for TREE_STATIC vars)?


Or skip when DECL is template_parm_object_p.


Or handle it in mangle.c.  I don't see any reason we shouldn't accept

struct A
{
   char c[4];
};

template  struct B { };
B b;

Probably we should use the same mangling whether the initializer for c 
was spelled as a string literal or list of integers.


So just loop over the STRING_CST and mangle each character the same
way as it were an element of the braced initializer list?

Martin



The thing we still don't want to allow is mangling the *address* of a 
string literal.


Jason




Re: [committed][PR rtl-optimization/87761] Limited iteration in regcprop to pick up secondary opportunities

2019-03-27 Thread Jeff Law
On 3/27/19 9:24 AM, Jeff Law wrote:
> On 3/27/19 8:36 AM, Jakub Jelinek wrote:
>> On Sun, Mar 24, 2019 at 09:20:07AM -0600, Jeff Law wrote:
>>> However, I'm increasingly of the opinion that MIPS targets need to drop
>>> off the priority platform list.  Given the trajectory I see for MIPS
>>> based processors in industry, it's really hard to justify spending this
>>> much time on them, particularly for low priority code quality issues.
>>
>> Besides what has been discussed on IRC for the PR89826 fix, that we really
>> need a df_analyze before processing the first block, because otherwise we
>> can't rely on the REG_UNUSED notes in the IL, I see some other issues, but I
>> admit I don't know much about df nor regcprop.
> RIght.  I plan to commit that today along with the test reordering you
> pointed out.
And the actual patch committed after the usual bootstrapping &
regression test on x86_64 plus testing on various *-elf platforms,
mips64-linux-gnu, mips64el-linux-gnu and others :-)


Jeff
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 07a1333cee0..f8a353d16c0 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,13 @@
+2019-03-27  Jeff Law  
+
+
+   PR rtl-optimization/87761
+   PR rtl-optimization/89826
+   * regcprop.c (copyprop_hardreg_forward_1): Move may_trap_p test
+   slightly later.
+   (pass_cprop_hardreg::execute): Call df_analyze after adding the
+   note problem to get REG_DEAD/REG_UNUSED notes updated.
+
 2019-03-27  Richard Biener  
 
PR tree-optimization/89463
diff --git a/gcc/regcprop.c b/gcc/regcprop.c
index 8ca523ffe23..3efe21f377c 100644
--- a/gcc/regcprop.c
+++ b/gcc/regcprop.c
@@ -800,9 +800,9 @@ copyprop_hardreg_forward_1 (basic_block bb, struct 
value_data *vd)
 
   /* Detect obviously dead sets (via REG_UNUSED notes) and remove them.  */
   if (set
- && !may_trap_p (set)
  && !RTX_FRAME_RELATED_P (insn)
  && INSN_P (insn)
+ && !may_trap_p (set)
  && find_reg_note (insn, REG_UNUSED, SET_DEST (set))
  && !side_effects_p (SET_SRC (set))
  && !side_effects_p (SET_DEST (set)))
@@ -1293,7 +1293,10 @@ pass_cprop_hardreg::execute (function *fun)
   auto_sbitmap visited (last_basic_block_for_fn (fun));
   bitmap_clear (visited);
 
+  /* We need accurate notes.  Earlier passes such as if-conversion may
+ leave notes in an inconsistent state.  */
   df_note_add_problem ();
+  df_analyze ();
 
   /* It is tempting to set DF_LR_RUN_DCE, but DCE may choose to delete
  an insn and this pass would not have visibility into the removal.
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 0679cb72e52..b2c649cfb3e 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2019-03-26  Jeff Law  
+
+   PR rtl-optimization/87761
+   PR rtl-optimization/89826
+   * gcc.c-torture/execute/pr89826.c: New test.
+
 2019-03-27  Richard Biener  
 
* gcc.dg/torture/20190327-1.c: New testcase.
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr89826.c 
b/gcc/testsuite/gcc.c-torture/execute/pr89826.c
new file mode 100644
index 000..091644849d3
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr89826.c
@@ -0,0 +1,21 @@
+typedef unsigned int u32;
+typedef unsigned long long u64;
+u64 a;
+u32 b;
+
+u64
+foo (u32 d)
+{
+  a -= d ? 0 : ~a;
+  return a + b;
+}
+
+int
+main (void)
+{
+  u64 x = foo (2);
+  if (x != 0)
+__builtin_abort();
+  return 0;
+}
+


Re: [committed][PR rtl-optimization/87761] Limited iteration in regcprop to pick up secondary opportunities

2019-03-27 Thread Jakub Jelinek
On Wed, Mar 27, 2019 at 09:24:46AM -0600, Jeff Law wrote:
> > 2) another thing I've noticed today, we queue up the debug insn changes,
> > but if we iterate the second time for any bb, we overwrite and thus lose any
> > of the debug insn queued changes from the first iteration, so those changes
> > aren't applied (wrong-debug?)
> This is actually what worries me, both with the current bits and with
> any bits that change to a worklist of blocks to reprocess.  As is I
> think we preserve the debug stuff across the iterations which should
> keep debug info correct.  Managing that in a world where we queue up the
> iteration step is going to be tougher.

The patch I've posted would do df_analyze, all bbs once, then df_analyze,
process queued debug changes, and if anything needs to be repeated (just
once), redo the bbs from worklist and repeat the above.
That seems to me the easiest way to get rid of the per-bb df_analyze calls
as well as fixing the debug stuff.  Because otherwise, we'd need to somehow
merge the queued debug insns from the first and second pass and figure out
how to deal with that.

Jakub


Re: [committed][PR rtl-optimization/87761] Limited iteration in regcprop to pick up secondary opportunities

2019-03-27 Thread Jeff Law
On 3/27/19 8:36 AM, Jakub Jelinek wrote:
> On Sun, Mar 24, 2019 at 09:20:07AM -0600, Jeff Law wrote:
>> However, I'm increasingly of the opinion that MIPS targets need to drop
>> off the priority platform list.  Given the trajectory I see for MIPS
>> based processors in industry, it's really hard to justify spending this
>> much time on them, particularly for low priority code quality issues.
> 
> Besides what has been discussed on IRC for the PR89826 fix, that we really
> need a df_analyze before processing the first block, because otherwise we
> can't rely on the REG_UNUSED notes in the IL, I see some other issues, but I
> admit I don't know much about df nor regcprop.
RIght.  I plan to commit that today along with the test reordering you
pointed out.

> 
> 1) the df_analyze () after every (successful) processing of a basic block
> is IMHO way too expensive, I would be very surprised if df_analyze () isn't
> quadratic in number of basic blocks and so one could construct testcases
> with millions of basic blocks and at least one regcprop change in each bb
> and get at cubic complexity (correct me if I'm wrong, and I'm aware of the
> 95% bbs you said won't have any changes at all)
I'm going to look this further today.

> 
> 2) another thing I've noticed today, we queue up the debug insn changes,
> but if we iterate the second time for any bb, we overwrite and thus lose any
> of the debug insn queued changes from the first iteration, so those changes
> aren't applied (wrong-debug?)
This is actually what worries me, both with the current bits and with
any bits that change to a worklist of blocks to reprocess.  As is I
think we preserve the debug stuff across the iterations which should
keep debug info correct.  Managing that in a world where we queue up the
iteration step is going to be tougher.

Queuing the iteration step feels like gcc-10 to me, but we'll see if it
falls out easier than expected.

> 
> 3) as mentioned on IRC, the order of tests in copyprop_hardreg_forward_1
> conditional doesn't start from the cheapest to most expensive one
That's going to be fixed in today's commit since it's been tested.

Jeff


Re: [PATCH] Integration of parallel standard algorithms for c++17

2019-03-27 Thread Jonathan Wakely

On 27/03/19 15:51 +0100, Thomas Schwinge wrote:

Hi!

If that's of any help to document the version dependencies:


Thanks for t his.


On Fri, 22 Mar 2019 00:04:30 +, Jonathan Wakely  wrote:

I keep forgetting to add that docs for this stuff will be coming some
time next week, describing the TBB dependency that's needed to use
these parallel algos.


On an Ubuntu 12.10 x86_64 GNU/Linux system (yes, a bit old by now), I saw
all these test UNSUPPORTED.

I then installed 'libtbb-dev' (and the implied 'libtbb2'; both version
4.0+r233-1), and then saw all? tests FAIL because of:

   [...]/libstdc++-v3/include/pstl/parallel_backend_tbb.h:25: fatal error: 
tbb/task_arena.h: No such file or directory

I then manually installed the Ubuntu trusty (14.04LTS) version
4.2~20130725-1.1ubuntu1 packages, and then saw all? tests FAIL because
of:

   [...]/libstdc++-v3/include/pstl/parallel_backend_tbb.h:29: error: #error 
Intel(R) Threading Building Blocks 2018 is required; older versions are not 
supported.


Yes, that's a known dependency.

Tom, I think check_effective_target_tbb-backend should probably check
the TBB_VERSION_MAJOR macro  in  and fail if an
older version is detected.

That will only help when running our tests though, not when users try
to use the algos, so we probably need to fail more gracefully if the
user has an odler TBB installed. If there's a header which is present
in TBB-2018 but not older versions then  could check
for that instead of (or as well as) .



So I suppose I need the most recent Ubuntu disco version 2018~U6-4
packages.  Because of:

   dpkg-deb: error: archive 'libtbb-dev_2018~U6-4_amd64.deb' contains not 
understood data member control.tar.xz, giving up

..., I manually had to convert to '*.gz' files the'.xz' files inside of
these '*.deb' archives, which yet still failed to install:

   dpkg: dependency problems prevent configuration of libtbb2:amd64:
libtbb2:amd64 depends on libstdc++6 (>= 7); however:
 Version of libstdc++6:amd64 on system is 4.7.2-2ubuntu1.

..., which isn't really a problem as I'll obviously be testing against a
new-enough GCC/libstdc++ ;-) -- so, installed these packages with
'--force-depends-version'.

With that, the tests then all PASS for the default multilib, but all?
FAIL for '-m32' testing:

   [...]/ld: cannot find -ltbb

There is no 32-bit 'libtbb' available.  I suppose the problem is that
'check_effective_target_tbb-backend' just does a 'preprocess' test
checking for existence of '', but doesn't actually try to
link, whereas all? the test cases specify '-ltbb' via 'dg-options'.


Right. Jakub noticed the same issue.


That said, instead of specifying:

   // { dg-options "-std=gnu++17 -ltbb" }
   // { dg-do run { target c++17 } }
   // { dg-require-effective-target tbb-backend }

... in all? these test case files, isn't there some DejaGnu directive
that checks whether support is available (else UNSUPPORTED), while also
adding the necessary compiler options (here: '-ltbb')?


I don't think we can do that in one go, can we?

We can add something so { dg-add-options tbb } adds the required
options, but I don't think it will make it UNSUPPORTED when necessary.
Maybe I'm missing something.


Also, shouldn't 'check_effective_target_tbb-backend' (and also some of
the other checks in 'libstdc++-v3/testsuite/lib/libstdc++.exp'?) be using
'check_v3_target_prop_cached' to avoid checking the same things again and
again?


Yes. I think it was probably written before that caching helper was
added.



Re: [PATCH] Integration of parallel standard algorithms for c++17

2019-03-27 Thread Thomas Schwinge
Hi!

If that's of any help to document the version dependencies:

On Fri, 22 Mar 2019 00:04:30 +, Jonathan Wakely  wrote:
> I keep forgetting to add that docs for this stuff will be coming some
> time next week, describing the TBB dependency that's needed to use
> these parallel algos.

On an Ubuntu 12.10 x86_64 GNU/Linux system (yes, a bit old by now), I saw
all these test UNSUPPORTED.

I then installed 'libtbb-dev' (and the implied 'libtbb2'; both version
4.0+r233-1), and then saw all? tests FAIL because of:

[...]/libstdc++-v3/include/pstl/parallel_backend_tbb.h:25: fatal error: 
tbb/task_arena.h: No such file or directory

I then manually installed the Ubuntu trusty (14.04LTS) version
4.2~20130725-1.1ubuntu1 packages, and then saw all? tests FAIL because
of:

[...]/libstdc++-v3/include/pstl/parallel_backend_tbb.h:29: error: #error 
Intel(R) Threading Building Blocks 2018 is required; older versions are not 
supported.

So I suppose I need the most recent Ubuntu disco version 2018~U6-4
packages.  Because of:

dpkg-deb: error: archive 'libtbb-dev_2018~U6-4_amd64.deb' contains not 
understood data member control.tar.xz, giving up

..., I manually had to convert to '*.gz' files the'.xz' files inside of
these '*.deb' archives, which yet still failed to install:

dpkg: dependency problems prevent configuration of libtbb2:amd64:
 libtbb2:amd64 depends on libstdc++6 (>= 7); however:
  Version of libstdc++6:amd64 on system is 4.7.2-2ubuntu1.

..., which isn't really a problem as I'll obviously be testing against a
new-enough GCC/libstdc++ ;-) -- so, installed these packages with
'--force-depends-version'.

With that, the tests then all PASS for the default multilib, but all?
FAIL for '-m32' testing:

[...]/ld: cannot find -ltbb

There is no 32-bit 'libtbb' available.  I suppose the problem is that
'check_effective_target_tbb-backend' just does a 'preprocess' test
checking for existence of '', but doesn't actually try to
link, whereas all? the test cases specify '-ltbb' via 'dg-options'.

That said, instead of specifying:

// { dg-options "-std=gnu++17 -ltbb" }
// { dg-do run { target c++17 } }
// { dg-require-effective-target tbb-backend }

... in all? these test case files, isn't there some DejaGnu directive
that checks whether support is available (else UNSUPPORTED), while also
adding the necessary compiler options (here: '-ltbb')?


Also, shouldn't 'check_effective_target_tbb-backend' (and also some of
the other checks in 'libstdc++-v3/testsuite/lib/libstdc++.exp'?) be using
'check_v3_target_prop_cached' to avoid checking the same things again and
again?


Grüße
 Thomas


Re: [committed][PR rtl-optimization/87761] Limited iteration in regcprop to pick up secondary opportunities

2019-03-27 Thread Jakub Jelinek
On Sun, Mar 24, 2019 at 09:20:07AM -0600, Jeff Law wrote:
> However, I'm increasingly of the opinion that MIPS targets need to drop
> off the priority platform list.  Given the trajectory I see for MIPS
> based processors in industry, it's really hard to justify spending this
> much time on them, particularly for low priority code quality issues.

Besides what has been discussed on IRC for the PR89826 fix, that we really
need a df_analyze before processing the first block, because otherwise we
can't rely on the REG_UNUSED notes in the IL, I see some other issues, but I
admit I don't know much about df nor regcprop.

1) the df_analyze () after every (successful) processing of a basic block
is IMHO way too expensive, I would be very surprised if df_analyze () isn't
quadratic in number of basic blocks and so one could construct testcases
with millions of basic blocks and at least one regcprop change in each bb
and get at cubic complexity (correct me if I'm wrong, and I'm aware of the
95% bbs you said won't have any changes at all)

2) another thing I've noticed today, we queue up the debug insn changes,
but if we iterate the second time for any bb, we overwrite and thus lose any
of the debug insn queued changes from the first iteration, so those changes
aren't applied (wrong-debug?)

3) as mentioned on IRC, the order of tests in copyprop_hardreg_forward_1
conditional doesn't start from the cheapest to most expensive one

So, do we want something like the following untested patch (only tested
that the PR89826 testcase is fixed on ARM, but df_analyze right after
df_set_flags is all that is needed to cure that.  I've outlined
two hunks of code into separate functions, so that I can call them from two
different spots.  And, I don't see why in the debug processing loop we
should test the visited bitmap, isn't it guaranteed that it is set on all
the bbs after going through the first FOR_EACH_BB_FN loop?
On the other side, I thought it might be beneficial not to do the debug
FOR_EACH_BB_FN loop(s) if there aren't any debug insn changes queued (not
sure how often would that happen, if not often enough, that part can be
dropped).

--- gcc/regcprop.c.jj   2019-03-25 11:02:55.826998948 +0100
+++ gcc/regcprop.c  2019-03-27 15:15:23.505651565 +0100
@@ -800,9 +800,9 @@ copyprop_hardreg_forward_1 (basic_block
 
   /* Detect obviously dead sets (via REG_UNUSED notes) and remove them.  */
   if (set
- && !may_trap_p (set)
- && !RTX_FRAME_RELATED_P (insn)
  && INSN_P (insn)
+ && !RTX_FRAME_RELATED_P (insn)
+ && !may_trap_p (set)
  && find_reg_note (insn, REG_UNUSED, SET_DEST (set))
  && !side_effects_p (SET_SRC (set))
  && !side_effects_p (SET_DEST (set)))
@@ -1282,6 +1282,66 @@ public:
 
 }; // class pass_cprop_hardreg
 
+static bool
+cprop_hardreg_bb (basic_block bb, struct value_data *all_vd, sbitmap visited)
+{
+  bitmap_set_bit (visited, bb->index);
+
+  /* If a block has a single predecessor, that we've already
+ processed, begin with the value data that was live at
+ the end of the predecessor block.  */
+  /* ??? Ought to use more intelligent queuing of blocks.  */
+  if (single_pred_p (bb)
+  && bitmap_bit_p (visited, single_pred (bb)->index)
+  && ! (single_pred_edge (bb)->flags & (EDGE_ABNORMAL_CALL | EDGE_EH)))
+{
+  all_vd[bb->index] = all_vd[single_pred (bb)->index];
+  if (all_vd[bb->index].n_debug_insn_changes)
+   {
+ unsigned int regno;
+
+ for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
+   {
+ if (all_vd[bb->index].e[regno].debug_insn_changes)
+   {
+ all_vd[bb->index].e[regno].debug_insn_changes = NULL;
+ if (--all_vd[bb->index].n_debug_insn_changes == 0)
+   break;
+   }
+   }
+   }
+}
+  else
+init_value_data (all_vd + bb->index);
+
+  return copyprop_hardreg_forward_1 (bb, all_vd + bb->index);
+}
+
+static void
+cprop_hardreg_debug (function *fun, struct value_data *all_vd)
+{
+  basic_block bb;
+
+  FOR_EACH_BB_FN (bb, fun)
+if (all_vd[bb->index].n_debug_insn_changes)
+  {
+   unsigned int regno;
+   bitmap live;
+
+   live = df_get_live_out (bb);
+   for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
+ if (all_vd[bb->index].e[regno].debug_insn_changes)
+   {
+ if (REGNO_REG_SET_P (live, regno))
+   apply_debug_insn_changes (all_vd + bb->index, regno);
+ if (all_vd[bb->index].n_debug_insn_changes == 0)
+   break;
+   }
+  }
+
+  queued_debug_insn_change_pool.release ();
+}
+
 unsigned int
 pass_cprop_hardreg::execute (function *fun)
 {
@@ -1293,6 +1353,9 @@ pass_cprop_hardreg::execute (function *f
   auto_sbitmap visited (last_basic_block_for_fn (fun));
   bitmap_clear (visited);
 
+  auto_vec redo;
+  bool any_debug_changes = false;
+
   df_note_add_problem 

[C++ PATCH] PR c++/86932 - missed SFINAE with empty pack.

2019-03-27 Thread Jason Merrill
The issue here was that when processing the explicit template args in
fn_type_unification we added an empty argument pack for the parameter pack,
so we never tried to do any deduction for it, and therefore never looked at
its type.  We need that empty pack behavior for partial ordering, but we
don't want it here, so let's make it conditional on tf_partial.

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

* pt.c (coerce_template_parms): Don't add an empty pack if
tf_partial.
(fn_type_unification): Pass tf_partial to coerce_template_parms.
---
 gcc/cp/pt.c   |  3 ++-
 gcc/testsuite/g++.dg/cpp0x/sfinae65.C | 15 +++
 gcc/cp/ChangeLog  |  5 +
 3 files changed, 22 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/sfinae65.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 05d5371d8a6..dc982f3ed55 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -8436,6 +8436,7 @@ coerce_template_parms (tree parms,
arg = NULL_TREE;
 
   if (template_parameter_pack_p (TREE_VALUE (parm))
+ && (arg || !(complain & tf_partial))
  && !(arg && ARGUMENT_PACK_P (arg)))
 {
  /* Some arguments will be placed in the
@@ -20077,7 +20078,7 @@ fn_type_unification (tree fn,
 substitution context.  */
   explicit_targs
= (coerce_template_parms (tparms, explicit_targs, NULL_TREE,
- complain,
+ complain|tf_partial,
  /*require_all_args=*/false,
  /*use_default_args=*/false));
   if (explicit_targs == error_mark_node)
diff --git a/gcc/testsuite/g++.dg/cpp0x/sfinae65.C 
b/gcc/testsuite/g++.dg/cpp0x/sfinae65.C
new file mode 100644
index 000..66790df77c6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/sfinae65.C
@@ -0,0 +1,15 @@
+// PR c++/86932
+// { dg-do compile { target c++11 } }
+
+template struct enable_if { using type = T; };
+template struct enable_if { };
+
+template struct is_foo { static constexpr bool value = false; };
+
+// { dg-error "enable_if" "" { target *-*-* } .+1 }
+template::value, int>::type...> void f() 
{}
+
+int main()
+{
+  f();// { dg-error "no match" }
+}
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 550b7541d9f..bce9b108723 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,5 +1,10 @@
 2019-03-26  Jason Merrill  
 
+   PR c++/86932 - missed SFINAE with empty pack.
+   * pt.c (coerce_template_parms): Don't add an empty pack if
+   tf_partial.
+   (fn_type_unification): Pass tf_partial to coerce_template_parms.
+
PR c++/86429 - constexpr variable in lambda.
PR c++/82643
PR c++/87327

base-commit: 496d55fdc6a7305f342a814f51482df29b1ce4f9
-- 
2.20.1



PING [RFA patch, Fortran] PR60144 - Misleading error message when missing "then" after "if" and "else if"

2019-03-27 Thread Dominique d'Humières
PING!

Patch posted at https://gcc.gnu.org/ml/fortran/2019-03/msg00098.html

TIA

Dominique


Re: [Patch] Bug85667-(x86_64) ms_abi rules aren't followed when returning short structs with float values(32-bit)

2019-03-27 Thread Eric Botcazou
> I modified the patch.
> Please let me know your thoughts.

Thanks.  This version is certainly better, but I cannot approve it myself.

Uros, given that the bug was fixed in 64-bit mode for GCC 9, I think that it 
would make sense to have it fixed in 32-bit mode too.

-- 
Eric Botcazou


[RS6000] PR89271, gcc.target/powerpc/vsx-simode2.c

2019-03-27 Thread Alan Modra
This patch makes a number of corrections to rs6000_register_move_cost,
adds a new register union class, GEN_OR_VSX_REGS, and adjusts insn
alternative to suit.

The patch initially just corrected register move cost when direct
moves are available, but that resulted in regressions.  Inspection of
those regressions showed ALL_REGS being used as the register allocno
class, which isn't ideal.  gcc/doc/tm.texi says: "You should define a
class for the union of two classes whenever some instruction allows
both classes".  Thus, define GEN_OR_VSX_REGS for the register
allocator.  (IRA wants to use the union of two register classes when
the costs of the classes are below memory cost, which happens more
often with the low direct move cost.)

As per https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89271#c11 we ought
to be returning the minimal cost for union classes.  That can be done
by rs6000_register_move_cost testing for vsx first, where the number
of regs for a given mode might be smaller than the same mode in gprs,
and adding another test to the LINK_OR_CTR_REGS case to exclude
SPEC_OR_GEN_REGS and NON_FLOAT_REGS.

I removed the VECTOR_MEM_VSX_P test since that leads to silly results
for scalar mode moves between altivec and float when TARGET_VSX.  eg.
rs6000_register_move_cost:, ret=2, mode=DF, from=FLOAT_REGS, to=FLOAT_REGS
rs6000_register_move_cost:, ret=16, mode=DF, from=FLOAT_REGS, to=ALTIVEC_REGS
rs6000_register_move_cost:, ret=2, mode=DF, from=FLOAT_REGS, to=VSX_REGS

The patch also fixes wrong results for moves within and between any of
the non-gpr, non-vsx special reg classes.  The comment about "moving
between two similar registers is just one instruction" is false.  We
can't move lr to ctr directly, for example.  I believe the intent of
the "reg_classes_intersect_p (to, from)" was to cover moves within
float or altivec, so I moved that test inside the code handling vsx,
and made sure the intersection wasn't anything besides vsx by masking
off everything else.  Masking isn't strictly necessary at the moment,
but would be if we create a GEN_OR_ALTIVEC_REGS class some time in the
future.

TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS is needed for rs6000 in order
to fix the 20% cactus_adm spec regression when using GEN_OR_VSX_REGS
as an allocno class.  It is similar to the aarch64 version but without
any selection by regno mode if the best class is a union class.

Without the .md file tweaks, these tests fail:
+FAIL: gcc.target/powerpc/p9-dimode1.c scan-assembler-not \\mmtvsrd\\M
+FAIL: gcc.target/powerpc/p9-splat-4.c scan-assembler mtvsrdd
+FAIL: gcc.target/powerpc/pr81348.c scan-assembler \\mlxsihzx\\M
+FAIL: gcc.target/powerpc/pr81348.c scan-assembler \\mvextsh2d\\M
+FAIL: gcc.target/powerpc/vec-set-char.c scan-assembler-not mtvsrwz
+FAIL: gcc.target/powerpc/vec-set-char.c scan-assembler vspltisb
+FAIL: gcc.target/powerpc/vec-set-int.c scan-assembler-not mtvsrwz
+FAIL: gcc.target/powerpc/vec-set-int.c scan-assembler vspltisw
+FAIL: gcc.target/powerpc/vec-set-short.c scan-assembler-not mtvsrwz
+FAIL: gcc.target/powerpc/vec-set-short.c scan-assembler vspltish
+FAIL: gcc.target/powerpc/vsx-simode3.c scan-assembler lxsiwzx

Also, using a register move cost of 2 for for power9 direct moves
gives these fails, even with the .md file tweaks:
+FAIL: gcc.target/powerpc/vec-set-char.c scan-assembler-not mtvsrwz
+FAIL: gcc.target/powerpc/vec-set-char.c scan-assembler vspltisb
+FAIL: gcc.target/powerpc/vec-set-char.c scan-assembler xxspltib
+FAIL: gcc.target/powerpc/vec-set-int.c scan-assembler-not mtvsrwz
+FAIL: gcc.target/powerpc/vec-set-int.c scan-assembler vspltisw
+FAIL: gcc.target/powerpc/vec-set-short.c scan-assembler-not mtvsrwz
+FAIL: gcc.target/powerpc/vec-set-short.c scan-assembler vspltish
+FAIL: gcc.target/powerpc/vec-set-short.c scan-assembler xxspltib
+FAIL: gcc.target/powerpc/vsx-himode3.c scan-assembler lxsihzx
+FAIL: gcc.target/powerpc/vsx-qimode3.c scan-assembler lxsibzx
These can be all be fixed by removing "?"s disparaging vector
alternatives in movsi_internal1 and mov_internal.

As is, net testsuite change for powerpc64le-linux all langs bootstrap
at r269867 is:
-FAIL: gcc.target/powerpc/fold-vec-extract-double.p8.c scan-assembler-times 
\\mmtvsrd\\M 1
-FAIL: gcc.target/powerpc/fold-vec-extract-double.p8.c scan-assembler-times 
\\mrldic\\M|\\mrlwinm\\M 1
-FAIL: gcc.target/powerpc/fold-vec-extract-double.p8.c scan-assembler-times 
\\mvslo\\M 1
-FAIL: gcc.target/powerpc/fold-vec-extract-double.p8.c scan-assembler-times 
\\mxori\\M 1
-FAIL: gcc.target/powerpc/fold-vec-extract-double.p8.c scan-assembler-times 
\\mxxlor\\M 2
-FAIL: gcc.target/powerpc/fold-vec-extract-longlong.p9.c scan-assembler-times 
\\mmfvsrd\\M 6
-FAIL: gcc.target/powerpc/fold-vec-extract-longlong.p9.c scan-assembler-times 
\\mmtvsrdd\\M 3
-FAIL: gcc.target/powerpc/fold-vec-extract-longlong.p9.c scan-assembler-times 
\\mrldic\\M 3
-FAIL: gcc.target/powerpc/fold-vec-extract-longlong.p9.c scan-assembler-times 
\\mvslo\\M 3
-FAIL: 

New testcase

2019-03-27 Thread Richard Biener


The following adds a testcase I managed to break when trying to make
SSA names safe_from_p.

Richard.

2019-03-27  Richard Biener  

* gcc.dg/torture/20190327-1.c: New testcase.

Index: gcc/testsuite/gcc.dg/torture/20190327-1.c
===
--- gcc/testsuite/gcc.dg/torture/20190327-1.c   (nonexistent)
+++ gcc/testsuite/gcc.dg/torture/20190327-1.c   (working copy)
@@ -0,0 +1,18 @@
+/* { dg-do run } */
+
+typedef long v2di __attribute__((vector_size(16)));
+v2di v;
+void __attribute__((noinline))
+foo()
+{
+  v = (v2di){v[1], v[0]};
+}
+
+int main()
+{
+  v[0] = 1;
+  foo ();
+  if (v[0] != 0 || v[1] != 1)
+__builtin_abort ();
+  return 0;
+}


Re: [PATCH] Fix PR89698, bogus folding to BIT_FIELD_REF

2019-03-27 Thread Richard Sandiford
Richard Biener  writes:
> On Wed, 27 Mar 2019, Richard Sandiford wrote:
>
>> Richard Biener  writes:
>> > Our beloved condition combining code to BIT_FIELD_REFs miscompiles
>> > the testcase because it relies on operand_equal_p to detect equal
>> > bases.  For some reason that very same operand_equal_p is
>> > treating any dereference of a pointer based on the same pointer
>> > or decl the same - idependent on the actual type used for the
>> > access (in this case, two different sized structs).  Weirdly
>> > it already checks alignment...
>> >
>> > The following patch makes operand_equal_p behave and more
>> > in line with what it does for MEM_REF handling.
>> >
>> > Bootstrap & regtest running on x86_64-unknown-linux-gnu.
>> >
>> > Richard.
>> >
>> > 2019-03-14  Richard Biener  
>> >
>> >PR middle-end/89698
>> >* fold-const.c (operand_equal_p): For INDIRECT_REF check
>> >that the access types are similar.
>> >
>> >* g++.dg/torture/pr89698.C: New testcase.
>> >
>> > Index: gcc/fold-const.c
>> > ===
>> > --- gcc/fold-const.c   (revision 269641)
>> > +++ gcc/fold-const.c   (working copy)
>> > @@ -3220,10 +3220,16 @@ operand_equal_p (const_tree arg0, const_
>> >switch (TREE_CODE (arg0))
>> >{
>> >case INDIRECT_REF:
>> > -if (!(flags & OEP_ADDRESS_OF)
>> > -&& (TYPE_ALIGN (TREE_TYPE (arg0))
>> > -!= TYPE_ALIGN (TREE_TYPE (arg1
>> > -  return 0;
>> > +if (!(flags & OEP_ADDRESS_OF))
>> > +  {
>> > +if (TYPE_ALIGN (TREE_TYPE (arg0))
>> > +!= TYPE_ALIGN (TREE_TYPE (arg1)))
>> > +  return 0;
>> > +/* Verify that the access types are compatible.  */
>> > +if (TYPE_MAIN_VARIANT (TREE_TYPE (arg0))
>> > +!= TYPE_MAIN_VARIANT (TREE_TYPE (arg1)))
>> > +  return 0;
>> 
>> Question from the peanut gallery, sorry, but: why's TYPE_MAIN_VARIANT
>> rather than types_compatible_p the right check here?  E.g. if the
>> dereferenced types are both pointers, but pointers to different types,
>> wouldn't they normally be equivalent in gimple?
>
> But this is GENERIC where the above is the usual check for
> "same type".  In GIMPLE you never see an INDIRECT_REF.

Fair point :-)


Re: Further bugs in extended C interop

2019-03-27 Thread Paul Richard Thomas
Hi Reinhold,

Many thanks - the bug number is out by one on the last. Jakub Jelnik
got one in before it :-)

I'll take a look this afternoon.

Cheers

Paul

On Wed, 27 Mar 2019 at 10:25, Bader, Reinhold  wrote:
>
> Dear Paul,
>
> Here are further bug reports, mostly on the various functions for manipulating
> descriptors:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89841
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89842
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89843
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89844
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89845
>
> Regards
>
> Reinhold



-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein


Re: [PATCH] Fix PR89698, bogus folding to BIT_FIELD_REF

2019-03-27 Thread Richard Biener
On Wed, 27 Mar 2019, Richard Sandiford wrote:

> Richard Biener  writes:
> > Our beloved condition combining code to BIT_FIELD_REFs miscompiles
> > the testcase because it relies on operand_equal_p to detect equal
> > bases.  For some reason that very same operand_equal_p is
> > treating any dereference of a pointer based on the same pointer
> > or decl the same - idependent on the actual type used for the
> > access (in this case, two different sized structs).  Weirdly
> > it already checks alignment...
> >
> > The following patch makes operand_equal_p behave and more
> > in line with what it does for MEM_REF handling.
> >
> > Bootstrap & regtest running on x86_64-unknown-linux-gnu.
> >
> > Richard.
> >
> > 2019-03-14  Richard Biener  
> >
> > PR middle-end/89698
> > * fold-const.c (operand_equal_p): For INDIRECT_REF check
> > that the access types are similar.
> >
> > * g++.dg/torture/pr89698.C: New testcase.
> >
> > Index: gcc/fold-const.c
> > ===
> > --- gcc/fold-const.c(revision 269641)
> > +++ gcc/fold-const.c(working copy)
> > @@ -3220,10 +3220,16 @@ operand_equal_p (const_tree arg0, const_
> >switch (TREE_CODE (arg0))
> > {
> > case INDIRECT_REF:
> > - if (!(flags & OEP_ADDRESS_OF)
> > - && (TYPE_ALIGN (TREE_TYPE (arg0))
> > - != TYPE_ALIGN (TREE_TYPE (arg1
> > -   return 0;
> > + if (!(flags & OEP_ADDRESS_OF))
> > +   {
> > + if (TYPE_ALIGN (TREE_TYPE (arg0))
> > + != TYPE_ALIGN (TREE_TYPE (arg1)))
> > +   return 0;
> > + /* Verify that the access types are compatible.  */
> > + if (TYPE_MAIN_VARIANT (TREE_TYPE (arg0))
> > + != TYPE_MAIN_VARIANT (TREE_TYPE (arg1)))
> > +   return 0;
> 
> Question from the peanut gallery, sorry, but: why's TYPE_MAIN_VARIANT
> rather than types_compatible_p the right check here?  E.g. if the
> dereferenced types are both pointers, but pointers to different types,
> wouldn't they normally be equivalent in gimple?

But this is GENERIC where the above is the usual check for
"same type".  In GIMPLE you never see an INDIRECT_REF.

Richard.

> Thanks,
> Richard
> 
> 
> > +   }
> >   flags &= ~OEP_ADDRESS_OF;
> >   return OP_SAME (0);
> >  
> > Index: gcc/testsuite/g++.dg/torture/pr89698.C
> > ===
> > --- gcc/testsuite/g++.dg/torture/pr89698.C  (nonexistent)
> > +++ gcc/testsuite/g++.dg/torture/pr89698.C  (working copy)
> > @@ -0,0 +1,28 @@
> > +/* { dg-do run } */
> > +
> > +extern "C" void abort (void);
> > +
> > +class A {
> > +virtual void f(){};
> > +public:
> > +int x;
> > +A(int in): x(in) {};
> > +};
> > +
> > +class B: public A {
> > +public:
> > +int y;
> > +B(int in):A(in-1), y(in) {};
> > +};
> > +
> > +int test(void)
> > +{
> > +  int res;
> > +  B b(2);
> > +  A* bp = 
> > +  void* vp = dynamic_cast(bp);
> > +  if (((A*)vp)->x == 1 && ((B*)vp)->y == 2)
> > +return 1;
> > +  return 0;
> > +}
> > +int main() { if (test() != 1) abort (); return 0; }
> 

-- 
Richard Biener 
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)

Re: [PATCH] Fix PR89698, bogus folding to BIT_FIELD_REF

2019-03-27 Thread Richard Sandiford
Richard Biener  writes:
> Our beloved condition combining code to BIT_FIELD_REFs miscompiles
> the testcase because it relies on operand_equal_p to detect equal
> bases.  For some reason that very same operand_equal_p is
> treating any dereference of a pointer based on the same pointer
> or decl the same - idependent on the actual type used for the
> access (in this case, two different sized structs).  Weirdly
> it already checks alignment...
>
> The following patch makes operand_equal_p behave and more
> in line with what it does for MEM_REF handling.
>
> Bootstrap & regtest running on x86_64-unknown-linux-gnu.
>
> Richard.
>
> 2019-03-14  Richard Biener  
>
>   PR middle-end/89698
>   * fold-const.c (operand_equal_p): For INDIRECT_REF check
>   that the access types are similar.
>
>   * g++.dg/torture/pr89698.C: New testcase.
>
> Index: gcc/fold-const.c
> ===
> --- gcc/fold-const.c  (revision 269641)
> +++ gcc/fold-const.c  (working copy)
> @@ -3220,10 +3220,16 @@ operand_equal_p (const_tree arg0, const_
>switch (TREE_CODE (arg0))
>   {
>   case INDIRECT_REF:
> -   if (!(flags & OEP_ADDRESS_OF)
> -   && (TYPE_ALIGN (TREE_TYPE (arg0))
> -   != TYPE_ALIGN (TREE_TYPE (arg1
> - return 0;
> +   if (!(flags & OEP_ADDRESS_OF))
> + {
> +   if (TYPE_ALIGN (TREE_TYPE (arg0))
> +   != TYPE_ALIGN (TREE_TYPE (arg1)))
> + return 0;
> +   /* Verify that the access types are compatible.  */
> +   if (TYPE_MAIN_VARIANT (TREE_TYPE (arg0))
> +   != TYPE_MAIN_VARIANT (TREE_TYPE (arg1)))
> + return 0;

Question from the peanut gallery, sorry, but: why's TYPE_MAIN_VARIANT
rather than types_compatible_p the right check here?  E.g. if the
dereferenced types are both pointers, but pointers to different types,
wouldn't they normally be equivalent in gimple?

Thanks,
Richard


> + }
> flags &= ~OEP_ADDRESS_OF;
> return OP_SAME (0);
>  
> Index: gcc/testsuite/g++.dg/torture/pr89698.C
> ===
> --- gcc/testsuite/g++.dg/torture/pr89698.C(nonexistent)
> +++ gcc/testsuite/g++.dg/torture/pr89698.C(working copy)
> @@ -0,0 +1,28 @@
> +/* { dg-do run } */
> +
> +extern "C" void abort (void);
> +
> +class A {
> +virtual void f(){};
> +public:
> +int x;
> +A(int in): x(in) {};
> +};
> +
> +class B: public A {
> +public:
> +int y;
> +B(int in):A(in-1), y(in) {};
> +};
> +
> +int test(void)
> +{
> +  int res;
> +  B b(2);
> +  A* bp = 
> +  void* vp = dynamic_cast(bp);
> +  if (((A*)vp)->x == 1 && ((B*)vp)->y == 2)
> +return 1;
> +  return 0;
> +}
> +int main() { if (test() != 1) abort (); return 0; }


Re: [C++ debug PATCH] [PR88534] accept VAR_DECL in class literal template parms

2019-03-27 Thread Jonathan Wakely

On 21/03/19 15:03 -0400, Jason Merrill wrote:

On 3/20/19 6:06 PM, Marek Polacek wrote:

On Wed, Mar 20, 2019 at 10:58:32PM +0100, Jakub Jelinek wrote:

On Wed, Mar 20, 2019 at 05:55:04PM -0400, Marek Polacek wrote:

On Wed, Mar 20, 2019 at 04:56:33PM -0300, Alexandre Oliva wrote:

On Mar 20, 2019, Marek Polacek  wrote:


This test fails with
pr88534.C:58:1: sorry, unimplemented: string literal in function template 
signature


Interesting...  gcc-8 rejected it with an error message rejecting the
template parameter, but my latest trunk build (dated Mar 13, r269641)
compiles it all right.  Was there a subsequent fix, maybe?  I didn't
realize it was supposed to be rejected.


Ah, that problem only started with r269814, namely this hunk:


Maybe this is done too early and should be postponed to genericization
(perhaps except for TREE_STATIC vars)?


Or skip when DECL is template_parm_object_p.


Or handle it in mangle.c.  I don't see any reason we shouldn't accept

struct A
{
 char c[4];
};

template  struct B { };
B b;

Probably we should use the same mangling whether the initializer for c 
was spelled as a string literal or list of integers.


The thing we still don't want to allow is mangling the *address* of a 
string literal.


Will that help PR 47488 as well?





Further bugs in extended C interop

2019-03-27 Thread Bader, Reinhold
Dear Paul,

Here are further bug reports, mostly on the various functions for manipulating 
descriptors:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89841
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89842
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89843
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89844
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89845

Regards

Reinhold


smime.p7s
Description: S/MIME cryptographic signature


[PATCH] Revert r254150 (PR bootstrap/89829).

2019-03-27 Thread Martin Liška
Hi.

The revision should be reverted as prev-* is the location
where stagetrain stores all *.gcda files.

I've tested that with PGO on x86_64-linux-gnu.

Ready for trunk?
Thanks,
Martin

ChangeLog:

2019-03-27  Martin Liska  

PR bootstrap/89829
* Makefile.in: Revert r254150.
* Makefile.tpl: Likewise.
---
 Makefile.in  | 4 ++--
 Makefile.tpl | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)


diff --git a/Makefile.in b/Makefile.in
index 28539a45372..231cc07cc0f 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -55960,8 +55960,8 @@ stageprofile-end::
 stagefeedback-start::
 	@r=`${PWD_COMMAND}`; export r; \
 	s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
-	for i in stageprofile-*; do \
-	  j=`echo $$i | sed s/^stageprofile-//`; \
+	for i in prev-*; do \
+	  j=`echo $$i | sed s/^prev-//`; \
 	  cd $$r/$$i && \
 	  { find . -type d | sort | sed 's,.*,$(SHELL) '"$$s"'/mkinstalldirs "../'$$j'/&",' | $(SHELL); } && \
 	  { find . -name '*.*da' | sed 's,.*,$(LN) -f "&" "../'$$j'/&",' | $(SHELL); }; \
diff --git a/Makefile.tpl b/Makefile.tpl
index 126296fb49a..54057a0182f 100644
--- a/Makefile.tpl
+++ b/Makefile.tpl
@@ -1759,8 +1759,8 @@ stageprofile-end::
 stagefeedback-start::
 	@r=`${PWD_COMMAND}`; export r; \
 	s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \
-	for i in stageprofile-*; do \
-	  j=`echo $$i | sed s/^stageprofile-//`; \
+	for i in prev-*; do \
+	  j=`echo $$i | sed s/^prev-//`; \
 	  cd $$r/$$i && \
 	  { find . -type d | sort | sed 's,.*,$(SHELL) '"$$s"'/mkinstalldirs "../'$$j'/&",' | $(SHELL); } && \
 	  { find . -name '*.*da' | sed 's,.*,$(LN) -f "&" "../'$$j'/&",' | $(SHELL); }; \



[PATCH 2/3] Extend format of -fdbg-cnt: add aux_base filter.

2019-03-27 Thread marxin

gcc/ChangeLog:

2019-03-27  Martin Liska  

* dbgcnt.c (dbg_cnt_set_limit_by_name): Add new argument
aux_base and filter based on aux_base_name.
(dbg_cnt_process_single_pair): Parse aux_base.
* doc/invoke.texi: Document new extended format.

gcc/testsuite/ChangeLog:

2019-03-27  Martin Liska  

* gcc.dg/dbg-cnt-1.c: New test.
---
 gcc/dbgcnt.c | 11 ---
 gcc/doc/invoke.texi  |  8 ++--
 gcc/testsuite/gcc.dg/dbg-cnt-1.c |  6 ++
 3 files changed, 20 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/dbg-cnt-1.c

diff --git a/gcc/dbgcnt.c b/gcc/dbgcnt.c
index e2f65f449e5..5a7c9a8bf6e 100644
--- a/gcc/dbgcnt.c
+++ b/gcc/dbgcnt.c
@@ -24,6 +24,7 @@ See dbgcnt.def for usage information.  */
 #include "coretypes.h"
 #include "diagnostic-core.h"
 #include "dumpfile.h"
+#include "options.h"
 
 #include "dbgcnt.h"
 
@@ -87,8 +88,11 @@ dbg_cnt_set_limit_by_index (enum debug_counter index, int low, int high)
 }
 
 static bool
-dbg_cnt_set_limit_by_name (const char *name, int low, int high)
+dbg_cnt_set_limit_by_name (const char *name, int low, int high, char *aux_base)
 {
+  if (aux_base != NULL && strcmp (aux_base_name, aux_base) != 0)
+return true;
+
   if (high < low)
 {
   error ("%<-fdbg-cnt=%s:%d:%d%> has smaller upper limit than the lower",
@@ -123,7 +127,7 @@ dbg_cnt_set_limit_by_name (const char *name, int low, int high)
 }
 
 
-/* Process a single "name:value" pair.
+/* Process a single "name:value1[:value2][:aux_base]" tuple.
Returns NULL if there's no valid pair is found.
Otherwise returns a pointer to the end of the pair. */
 
@@ -134,6 +138,7 @@ dbg_cnt_process_single_pair (const char *arg)
   char *name = strtok (str, ":");
   char *value1 = strtok (NULL, ":");
   char *value2 = strtok (NULL, ":");
+  char *aux_base = strtok (NULL, ":");
 
   int high, low;
 
@@ -151,7 +156,7 @@ dbg_cnt_process_single_pair (const char *arg)
   high = strtol (value2, NULL, 10);
 }
 
-  return dbg_cnt_set_limit_by_name (name, low, high);
+  return dbg_cnt_set_limit_by_name (name, low, high, aux_base);
 }
 
 void
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 4735b0ab673..d2934edd36d 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -15386,10 +15386,14 @@ Print the name and the counter upper bound for all debug counters.
 @item -fdbg-cnt=@var{counter-value-list}
 @opindex fdbg-cnt
 Set the internal debug counter lower and upper bound.  @var{counter-value-list}
-is a comma-separated list of @var{name}:@var{lower_bound}:@var{upper_bound}
+is a comma-separated list of
+@var{name}:@var{lower_bound}:@var{upper_bound}:@var{aux_base_name}
 tuples which sets the lower and the upper bound of each debug
 counter @var{name}.  The @var{lower_bound} is optional and is zero
-initialized if not set.
+initialized if not set.  When @var{aux_base_name} is set, the debug counter
+is only applied to source files that match by @option{-auxbase}.
+The @var{aux_base_name} is also optional.
+
 All debug counters have the initial upper bound of @code{UINT_MAX};
 thus @code{dbg_cnt} returns true always unless the upper bound
 is set by this option.
diff --git a/gcc/testsuite/gcc.dg/dbg-cnt-1.c b/gcc/testsuite/gcc.dg/dbg-cnt-1.c
new file mode 100644
index 000..2afd2428eb9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/dbg-cnt-1.c
@@ -0,0 +1,6 @@
+/* { dg-do compile } */
+/* { dg-options "-fdbg-cnt=vect_loop:1:2,vect_slp:2,merged_ipa_icf:7:8:dbg-cnt-1" } */
+/* { dg-additional-options "-mavx2" { target { i?86-*-* x86_64-*-* } } } */
+/* { dg-prune-output "dbg_cnt 'vect_loop' set to 1-2" } */
+/* { dg-prune-output "dbg_cnt 'vect_slp' set to 0-2" } */
+/* { dg-prune-output "dbg_cnt 'merged_ipa_icf' set to 7-8" } */


[PATCH 1/3] Fix multiple values for -fdbg-cnt.

2019-03-27 Thread marxin

gcc/ChangeLog:

2019-03-27  Martin Liska  

* dbgcnt.c (dbg_cnt_process_single_pair): Fix GNU coding style.
(dbg_cnt_process_opt): Parse first tokens aas
dbg_cnt_process_single_pair is also using strtok.
---
 gcc/dbgcnt.c | 25 +++--
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/gcc/dbgcnt.c b/gcc/dbgcnt.c
index ebaa310821c..e2f65f449e5 100644
--- a/gcc/dbgcnt.c
+++ b/gcc/dbgcnt.c
@@ -151,30 +151,35 @@ dbg_cnt_process_single_pair (const char *arg)
   high = strtol (value2, NULL, 10);
 }
 
-   return dbg_cnt_set_limit_by_name (name, low, high);
+  return dbg_cnt_set_limit_by_name (name, low, high);
 }
 
 void
 dbg_cnt_process_opt (const char *arg)
 {
   char *str = xstrdup (arg);
-  const char *next = strtok (str, ",");
   unsigned int start = 0;
 
-   do {
- if (!dbg_cnt_process_single_pair (arg))
+  auto_vec tokens;
+  for (const char *next = strtok (str, ","); next != NULL;
+   next = strtok (NULL, ","))
+tokens.safe_push (next);
+
+  unsigned i;
+  for (i = 0; i < tokens.length (); i++)
+{
+ if (!dbg_cnt_process_single_pair (tokens[i]))
break;
- start += strlen (arg) + 1;
- next = strtok (NULL, ",");
-   } while (next != NULL);
+ start += strlen (tokens[i]) + 1;
+}
 
-   if (next != NULL)
+   if (i != tokens.length ())
  {
char *buffer = XALLOCAVEC (char, start + 2);
sprintf (buffer, "%*c", start + 1, '^');
error ("cannot find a valid counter:value pair:");
-   error ("%<-fdbg-cnt=%s%>", next);
-   error ("  %s", buffer);
+   error ("%<-fdbg-cnt=%s%>", arg);
+   error ("   %s", buffer);
  }
 }
 


[PATCH 3/3] Dump -fdbg-cnt limit reach also to stderr stream.

2019-03-27 Thread marxin

gcc/ChangeLog:

2019-03-27  Martin Liska  

* dbgcnt.c (print_limit_reach): New function.
(dbg_cnt): Use it.
---
 gcc/dbgcnt.c | 26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/gcc/dbgcnt.c b/gcc/dbgcnt.c
index 5a7c9a8bf6e..cfa03d611ee 100644
--- a/gcc/dbgcnt.c
+++ b/gcc/dbgcnt.c
@@ -59,21 +59,27 @@ dbg_cnt_is_enabled (enum debug_counter index)
   return v > limit_low[index] && v <= limit_high[index];
 }
 
+static void
+print_limit_reach (const char *counter, int limit, bool upper_p)
+{
+  char buffer[128];
+  sprintf (buffer, "***dbgcnt: %s limit %d reached for %s.***\n",
+	   upper_p ? "upper" : "lower", limit, counter);
+  fputs (buffer, stderr);
+  if (dump_file)
+fputs (buffer, dump_file);
+}
+
 bool
 dbg_cnt (enum debug_counter index)
 {
   count[index]++;
 
-  if (dump_file)
-{
-  /* Do not print the info for default lower limit.  */
-  if (count[index] == limit_low[index] && limit_low[index] > 0)
-	fprintf (dump_file, "***dbgcnt: lower limit %d reached for %s.***\n",
-		 limit_low[index], map[index].name);
-  else if (count[index] == limit_high[index])
-	fprintf (dump_file, "***dbgcnt: upper limit %d reached for %s.***\n",
-		 limit_high[index], map[index].name);
-}
+  /* Do not print the info for default lower limit.  */
+  if (count[index] == limit_low[index] && limit_low[index] > 0)
+print_limit_reach (map[index].name, limit_low[index], false);
+  else if (count[index] == limit_high[index])
+print_limit_reach (map[index].name, limit_high[index], true);
 
   return dbg_cnt_is_enabled (index);
 }


[PATCH 0/3] Fix and improve -fdbg-cnt option.

2019-03-27 Thread marxin
Hi.

While I was working on PR84201 it was very handy for me to use debug counters
for a particular source file. That's because I can't run a verification
on a spec result and I was unable to persuade runspec command to use a modified 
binary
for run.

Thus's I'm suggesting extension of the option to filter aux_base_name.

I've been testing the series right now.
Thoughts?
Thanks,
Martin

marxin (3):
  Fix multiple values for -fdbg-cnt.
  Extend format of -fdbg-cnt: add aux_base filter.
  Dump -fdbg-cnt limit reach also to stderr stream.

 gcc/dbgcnt.c | 60 
 gcc/doc/invoke.texi  |  8 +++--
 gcc/testsuite/gcc.dg/dbg-cnt-1.c |  6 
 3 files changed, 50 insertions(+), 24 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/dbg-cnt-1.c

-- 
2.21.0



Re: [PATCH] Fix PR89463 (and dups?), wrong-debug with loop removal

2019-03-27 Thread Richard Biener
On Mon, 25 Mar 2019, Richard Biener wrote:

> 
> This PR and possibly quite some dups that have been accumulating lately
> run into an artifact of DCEs edge removal code when generating debug
> stmts.  Here DCE removes edges from the CFG at the same time it removes
> dead controlling stmts but before PHI nodes are removed.  This causes
> us to remove PHI arguments associated with removed edges and make
> those PHI appearing as degenerate, causing wrong debug stmts to be
> generated.  Consider for example i_1 = PHI  where the edge
> leading to i_2 is removed - at the time we remove the PHI it looks
> like i_1 = PHI <0> and a i = 0 debug-stmt is generated.
> 
> The fix is to delay edge removal and move PHI removal back to the
> place it had originally been, doing that in reverse dominator order
> as intended for debug stmt creation (it jumped from where it'll be
> after the patch to all-before to all-after).
> 
> The testcase shows up as UNRESOLVED where it formerly was
> wrong-debug because we change from i = 0 to i = NULL.  I've seen
> no way to mark "optimized-out" as expected and at -O1 we
> retain the loop and the correct value of i.
> 
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> 
> OK (well, I think it is, just double-checking).

I have applied this now, it doesn't immediately solve any of the
other issues.

Richard.