Re: [Vectorizer] Add SLP support for masked loads

[Richard Biener]

On Wed, Jan 16, 2019 at 2:37 PM Alejandro Martinez Vicente
 wrote:
>
> Hi,
>
> Current vectorizer doesn't support masked loads for SLP. We should add that, 
> to
> allow things like:
>
> void
> f (int *restrict x, int *restrict y, int *restrict z, int n)
> {
>   for (int i = 0; i < n; i += 2)
> {
>   x[i] = y[i] ? z[i] : 1;
>   x[i + 1] = y[i + 1] ? z[i + 1] : 2;
> }
> }
>
> to be vectorized using contiguous loads rather than LD2 and ST2.
>
> This patch was motivated by SVE, but it is completely generic and should apply
> to any architecture with masked loads.
>
> After the patch is applied, the above code generates this output
> (-march=armv8.2-a+sve -O2 -ftree-vectorize):
>
>  :
>0:   717fcmp w3, #0x0
>4:   540002cdb.le5c 
>8:   51000464sub w4, w3, #0x1
>c:   d283mov x3, #0x0// #0
>   10:   9005adrpx5, 0 
>   14:   25d8e3e0ptrue   p0.d
>   18:   53017c84lsr w4, w4, #1
>   1c:   91a5add x5, x5, #0x0
>   20:   11000484add w4, w4, #0x1
>   24:   85c0e0a1ld1rd   {z1.d}, p0/z, [x5]
>   28:   2598e3e3ptrue   p3.s
>   2c:   d37ff884lsl x4, x4, #1
>   30:   25a41fe2whilelo p2.s, xzr, x4
>   34:   d503201fnop
>   38:   a5434820ld1w{z0.s}, p2/z, [x1, x3, lsl #2]
>   3c:   25808c11cmpne   p1.s, p3/z, z0.s, #0
>   40:   25808810cmpne   p0.s, p2/z, z0.s, #0
>   44:   a5434040ld1w{z0.s}, p0/z, [x2, x3, lsl #2]
>   48:   05a1c400sel z0.s, p1, z0.s, z1.s
>   4c:   e5434800st1w{z0.s}, p2, [x0, x3, lsl #2]
>   50:   04b0e3e3incwx3
>   54:   25a41c62whilelo p2.s, x3, x4
>   58:   5401b.ne38   // b.any
>   5c:   d65f03c0ret
>
>
> I tested this patch in an aarch64 machine bootstrapping the compiler and
> running the checks.

Thanks for implementing this - note this is stage1 material and I will have
a look when time allows unless Richard beats me to it.

It might be interesting to note that "non-SLP" code paths are likely to
go away in GCC 10 to streamline the vectorizer and make further changes
easier (so you'll see group_size == 1 SLP instances).

There are quite a few other cases missing SLP handling.

Richard.

> Alejandro
>
> gcc/Changelog:
>
> 2019-01-16  Alejandro Martinez  
>
> * config/aarch64/aarch64-sve.md (copysign3): New define_expand.
> (xorsign3): Likewise.
> internal-fn.c: Marked mask_load_direct and mask_store_direct as
> vectorizable.
> tree-data-ref.c (data_ref_compare_tree): Fixed comment typo.
> tree-vect-data-refs.c (can_group_stmts_p): Allow masked loads to be
> combined even if masks different.
> (slp_vect_only_p): New function to detect masked loads that are only
> vectorizable using SLP.
> (vect_analyze_data_ref_accesses): Mark SLP only vectorizable groups.
> tree-vect-loop.c (vect_dissolve_slp_only_groups): New function to
> dissolve SLP-only vectorizable groups when SLP has been discarded.
> (vect_analyze_loop_2): Call vect_dissolve_slp_only_groups when needed.
> tree-vect-slp.c (vect_get_and_check_slp_defs): Check masked loads
> masks.
> (vect_build_slp_tree_1): Fixed comment typo.
> (vect_build_slp_tree_2): Include masks from masked loads in SLP tree.
> tree-vect-stmts.c (vect_get_vec_defs_for_operand): New function to get
> vec_defs for operand with optional SLP and vectype.
> (vectorizable_load): Allow vectorizaion of masked loads for SLP only.
> tree-vectorizer.h (_stmt_vec_info): Added flag for SLP-only
> vectorizable.
> tree-vectorizer.c (vec_info::new_stmt_vec_info): Likewise.
>
> gcc/testsuite/Changelog:
>
> 2019-01-16  Alejandro Martinez  
>
> * gcc.target/aarch64/sve/mask_load_slp_1.c: New test for SLP
> vectorized masked loads.

Re: [PATCH] avoid issuing -Warray-bounds during folding (PR 88800)

[Richard Biener]

On Thu, Jan 17, 2019 at 2:51 AM Martin Sebor  wrote:
>
> On 1/16/19 6:14 PM, Jeff Law wrote:
> > On 1/15/19 8:21 AM, Martin Sebor wrote:
> >> On 1/15/19 4:07 AM, Richard Biener wrote:
> >>> On Tue, Jan 15, 2019 at 1:08 AM Martin Sebor  wrote:
> 
>  The gimple_fold_builtin_memory_op() function folds calls to memcpy
>  and similar to MEM_REF when the size of the copy is a small power
>  of 2, but it does so without considering whether the copy might
>  write (or read) past the end of one of the objects.  To detect
>  these kinds of errors (and help distinguish them from -Westrict)
>  the folder calls into the wrestrict pass and lets it diagnose them.
>  Unfortunately, that can lead to false positives for even some fairly
>  straightforward code that is ultimately found to be unreachable.
>  PR 88800 is a report of one such problem.
> 
>  To avoid these false positives the attached patch adjusts
>  the function to avoid issuing -Warray-bounds for out-of-bounds
>  calls to memcpy et al.  Instead, the patch disables the folding
>  of such invalid calls (and only those).  Those that are not
>  eliminated during DCE or other subsequent passes are eventually
>  diagnosed by the wrestrict pass.
> 
>  Since this change required removing the dependency of the detection
>  on the warning options (originally done as a micro-optimization to
>  avoid spending compile-time cycles on something that wasn't needed)
>  the patch also adds tests to verify that code generation is not
>  affected as a result of warnings being enabled or disabled.  With
>  the patch as is, the invalid memcpy calls end up emitted (currently
>  they are folded into equally invalid MEM_REFs).  At some point,
>  I'd like us to consider whether they should be replaced with traps
>  (possibly under the control of  as has been proposed a number of
>  times in the past.  If/when that's done, these tests will need to
>  be adjusted to look for traps instead.
> 
>  Tested on x86_64-linux.
> >>>
> >>> I've said in the past that I feel delaying of folding is wrong.
> >>>
> >>> To understand, the PR is about emitting a warning for out-of-bound
> >>> accesses in a dead code region?
> >>
> >> Yes.  I am keeping in my mind your preference of not delaying
> >> the folding of valid code.
> >>
> >>>
> >>> If we think delaying/disablign the folding is the way to go the
> >>> patch looks OK.
> >>
> >> I do, at least for now.  I'm taking this as your approval to commit
> >> the patch (please let me know if you didn't mean it that way).
> > Note we are in stage4, so we're supposed to be addressing regression
> > bugfixes and documentation issues.
> >
> > So  I think Richi needs to be explicit about whether or not he wants
> > this in gcc-9 or if it should defer to gcc-10.
> >
> > I have no technical objections to the patch and would easily ack it in
> > stage1 or stage3.
>
> The warning is a regression introduced in GCC 8.  I was just about
> to commit the fix so please let me know if I should hold off until
> stage 1.

As it fixes a regression it is fine now - I also like the cleaning up
of the helper function to work independently of the -W* state.

Richard.

> Martin

[patch, fortran] Fix PR 88871

[Thomas Koenig]

Hello world,

the attached patch fixes PR 88871, a regression introduced by
my recent patch for removing unnecessary substrings.

Regression-tested; this now also works with valgrind on Linux,
where the failure did not show up otherwise.

No test case because, well - it did show up on a few systems, so we will
notice if it regresses. OK for trunk?

Regards

Thomas

2019-01-17  Thomas Koenig  

PR fortran/88871
* resolve.c (resolve_ref):  Fix logic for removal of
reference.
Index: resolve.c
===
--- resolve.c	(Revision 267953)
+++ resolve.c	(Arbeitskopie)
@@ -5046,6 +5046,7 @@ resolve_ref (gfc_expr *expr)
   int current_part_dimension, n_components, seen_part_dimension;
   gfc_ref *ref, **prev;
   bool equal_length;
+  bool breakout;
 
   for (ref = expr->ref; ref; ref = ref->next)
 if (ref->type == REF_ARRAY && ref->u.ar.as == NULL)
@@ -5054,12 +5055,12 @@ resolve_ref (gfc_expr *expr)
 	break;
   }
 
-  
-  for (ref = expr->ref, prev = >ref; ref; prev = >next, ref = ref->next)
-switch (ref->type)
+  breakout = false;
+  for (prev = >ref; !breakout && *prev != NULL; prev = &(*prev)->next)
+switch ((*prev)->type)
   {
   case REF_ARRAY:
-	if (!resolve_array_ref (>u.ar))
+	if (!resolve_array_ref (&(*prev)->u.ar))
 	  return false;
 	break;
 
@@ -5069,17 +5070,20 @@ resolve_ref (gfc_expr *expr)
 
   case REF_SUBSTRING:
 	equal_length = false;
-	if (!resolve_substring (ref, _length))
+	if (!resolve_substring (*prev, _length))
 	  return false;
 
 	if (expr->expr_type != EXPR_SUBSTRING && equal_length)
 	  {
 	/* Remove the reference and move the charlen, if any.  */
+	ref = *prev;
 	*prev = ref->next;
 	ref->next = NULL;
 	expr->ts.u.cl = ref->u.ss.length;
 	ref->u.ss.length = NULL;
 	gfc_free_ref_list (ref);
+	if (*prev == NULL)
+	  breakout = true;
 	  }
 	break;
   }

V2 [PATCH] c-family: Update unaligned adress of packed member check

[H.J. Lu]

On Wed, Jan 16, 2019 at 01:28:26PM +0100, Jakub Jelinek wrote:
> On Wed, Jan 16, 2019 at 04:11:44AM -0800, H.J. Lu wrote:
> > > Why?  What is so special about C and (implicit?) casts where the rhs isn't
> > > ADDR_EXPR?  Aren't all casts (explicit or implicit) from one pointer type
> > > to another pointer and satisfy the rules something that should be warned
> > 
> > -Wincompatible-pointer-types is C only.   In C++,  incompatible pointer 
> > types
> > aren't allowed at all.
> 
> How so?  You can certainly:
> struct B { int i; };
> struct C { struct B b; } __attribute__ ((packed));
> 
> extern struct C *p;
> long* g8 (void) { return (long *)p; }
> 
> and similarly for C.  So, why is explicit cast something that shouldn't
> be warned about in this case and implicit cast should get a warning,
> especially when it already does get one (and one even enabled by default,
> -Wincompatible-pointer-types)?
> Either such casts are problematic and then it should treat explicit and
> implicit casts the same, or they aren't, and then
> -Wincompatible-pointer-types is all you need.
> 

I am testing this patch.


H.J.
---
Check unaligned pointer conversion and strip NOPS.

gcc/c-family/

PR c/51628
PR c/88664
* c-common.h (warn_for_address_or_pointer_of_packed_member):
Remove the boolean argument.
* c-warn.c (check_address_of_packed_member): Renamed to ...
(check_address_or_pointer_of_packed_member): This.  Also
warn pointer conversion.
(check_and_warn_address_of_packed_member): Renamed to ...
(check_and_warn_address_or_pointer_of_packed_member): This.
Also warn pointer conversion.
(warn_for_address_or_pointer_of_packed_member): Remove the
boolean argument.  Don't check pointer conversion here.

gcc/c

PR c/51628
PR c/88664
* c-typeck.c (convert_for_assignment): Upate the
warn_for_address_or_pointer_of_packed_member call.

gcc/cp

PR c/51628
PR c/88664
* call.c (convert_for_arg_passing): Upate the
warn_for_address_or_pointer_of_packed_member call.
* typeck.c (convert_for_assignment): Likewise.

gcc/testsuite/

PR c/51628
PR c/88664
* c-c++-common/pr51628-33.c: New test.
* c-c++-common/pr51628-35.c: New test.
* c-c++-common/pr88664-1.c: Likewise.
* c-c++-common/pr88664-2.c: Likewise.
* gcc.dg/pr51628-34.c: Likewise.
---
 gcc/c-family/c-common.h |   2 +-
 gcc/c-family/c-warn.c   | 154 +---
 gcc/c/c-typeck.c|   6 +-
 gcc/cp/call.c   |   2 +-
 gcc/cp/typeck.c |   2 +-
 gcc/testsuite/c-c++-common/pr51628-33.c |  19 +++
 gcc/testsuite/c-c++-common/pr51628-35.c |  15 +++
 gcc/testsuite/c-c++-common/pr88664-1.c  |  20 +++
 gcc/testsuite/c-c++-common/pr88664-2.c  |  22 
 gcc/testsuite/gcc.dg/pr51628-34.c   |  25 
 10 files changed, 190 insertions(+), 77 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/pr51628-33.c
 create mode 100644 gcc/testsuite/c-c++-common/pr51628-35.c
 create mode 100644 gcc/testsuite/c-c++-common/pr88664-1.c
 create mode 100644 gcc/testsuite/c-c++-common/pr88664-2.c
 create mode 100644 gcc/testsuite/gcc.dg/pr51628-34.c

diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index db16ae94b64..460954fefd8 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1284,7 +1284,7 @@ extern void c_do_switch_warnings (splay_tree, location_t, 
tree, tree, bool,
  bool);
 extern void warn_for_omitted_condop (location_t, tree);
 extern bool warn_for_restrict (unsigned, tree *, unsigned);
-extern void warn_for_address_or_pointer_of_packed_member (bool, tree, tree);
+extern void warn_for_address_or_pointer_of_packed_member (tree, tree);
 
 /* Places where an lvalue, or modifiable lvalue, may be required.
Used to select diagnostic messages in lvalue_error and
diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index 79b2d8ad449..5df17ba2e1b 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -2713,12 +2713,14 @@ check_alignment_of_packed_member (tree type, tree field)
   return NULL_TREE;
 }
 
-/* Return struct or union type if the right hand value, RHS, takes the
-   unaligned address of packed member of struct or union when assigning
-   to TYPE.  Otherwise, return NULL_TREE.  */
+/* Return struct or union type if the right hand value, RHS:
+   1. Is a pointer value which isn't aligned to a pointer type TYPE.
+   2. Is an address which takes the unaligned address of packed member
+  of struct or union when assigning to TYPE.
+   Otherwise, return NULL_TREE.  */
 
 static tree
-check_address_of_packed_member (tree type, tree rhs)
+check_address_or_pointer_of_packed_member (tree type, tree rhs)
 {
   if (INDIRECT_REF_P (rhs))
 rhs = TREE_OPERAND (rhs, 0);
@@ -2726,6 

Re: [C++ PATCH] [PR86648] use auto identifier for class placeholder templates

[Jason Merrill]

On 1/16/19 11:10 PM, Alexandre Oliva wrote:

On Jan  8, 2019, Jason Merrill  wrote:


On 12/28/18 2:45 PM, Alexandre Oliva wrote:

+  if (template_placeholder_p (t)
+ && DECL_P (CLASS_PLACEHOLDER_TEMPLATE (t))
+ && TYPE_IDENTIFIER (TREE_TYPE (CLASS_PLACEHOLDER_TEMPLATE (t



Are these extra checks needed?


Nope, that was just my overly defensive programming ;-)


I would expect them to be true whenever template_placeholder_p is.


Testing agrees with your expectation.  Here's a test without the extra
checks, regstrapped on x86_64- and i686-linux-gnu.  Ok to install?


[PR86648] use auto identifier for class placeholder templates

dwarf2out recognizes unspecified auto types by the identifier.  C++
template class placeholders are unspecified auto types that take the
identifier of the class rather than those used by preexisting auto
types, so dwarf2out ICEs when it finds one of those.  Alas, they may
be visible to dwarf2out, since the types of e.g. static data members
of templates are only deduced at member instantiation, i.e., if the
data member is actually referenced, but the data member is added as a
field, still with unspecified auto placeholder type, when the
enclosing class is instantiated.

I've changed placeholder creator to use an auto identifier instead,
which allowed dropping the placeholder test in C++'s is_auto (alas, it
can't be used in dwarf2out, think LTO).  To avoid losing information
in error messages and dumps and whatnot, I've added code to recognize
placeholders for template classes say A and print them out as
A<...auto...>.

for  gcc/cp/ChangeLog

PR c++/86648
 * pt.c (make_template_placeholder): Use auto_identifier.
 (is_auto): Drop CLASS_PLACEHOLDER_TEMPLATE test.
 * error.c (dump_type): Handle template placeholders.
 * cxx-pretty-print.c (pp_cx_unqualified_id): Likewise.


OK.

Jason

Re: [C++ PATCH] [PR87768] do not suppress location wrappers when tsubsting

[Jason Merrill]

On 1/16/19 11:26 PM, Alexandre Oliva wrote:

On Jan  2, 2019, Jason Merrill  wrote:


On 12/30/18 11:31 PM, Alexandre Oliva wrote:

Concepts-checking and other kinds of early tsubsting may often take
place while location wrappers are suppressed, e.g. because we've
triggered template instantiation within template parameter lists.

With that, exprs that are usually wrapped by VIEW_CONVERT_EXPRs
location wrappers may end up wrapped by NON_LVALUE_EXPRs that are not
marked as location wrappers.  If such NON_LVALUE_EXPRs tsubsted exprs
undergo another round of tsubsting, say for constraint checking, or
even for another round of specialization, they will be rejected by
tsubst_copy_and_build.



This patch introduces a sentinel to reset suppress_location_wrappers
to zero, and uses it for template class and decl instantiation,
including constraint checking.



Instead of a new class, let's add this to saved_scope and
push_to/pop_from_top_level.


Like this?  Regstrapped on x86_64- and i686-linux-gnu.


[PR87768] reset location wrapper suppression when reentering top level

Concepts-checking and other kinds of early tsubsting may often take
place while location wrappers are suppressed, e.g. because we've
triggered template instantiation within template parameter lists.

With that, exprs that are usually wrapped by VIEW_CONVERT_EXPRs
location wrappers may end up wrapped by NON_LVALUE_EXPRs that are not
marked as location wrappers.  If such NON_LVALUE_EXPRs tsubsted exprs
undergo another round of tsubsting, say for constraint checking, or
even for another round of specialization, they will be rejected by
tsubst_copy_and_build.

This patch arranges for suppress_location_wrappers to be saved and
reset when pushing to the top level, and restored when popping from
it.


for  gcc/cp/ChangeLog

PR c++/87768
* cp-tree.h (saved_scope): Add suppress_location_wrappers.
* name-lookup.c (do_push_to_top_level): Save and reset it.
(do_pop_from_top_level): Restore it.


OK.

Jason

Re: [C++ PATCH] [PR87768] do not suppress location wrappers when tsubsting

[Alexandre Oliva]

On Jan  2, 2019, Jason Merrill  wrote:

> On 12/30/18 11:31 PM, Alexandre Oliva wrote:
>> Concepts-checking and other kinds of early tsubsting may often take
>> place while location wrappers are suppressed, e.g. because we've
>> triggered template instantiation within template parameter lists.
>> 
>> With that, exprs that are usually wrapped by VIEW_CONVERT_EXPRs
>> location wrappers may end up wrapped by NON_LVALUE_EXPRs that are not
>> marked as location wrappers.  If such NON_LVALUE_EXPRs tsubsted exprs
>> undergo another round of tsubsting, say for constraint checking, or
>> even for another round of specialization, they will be rejected by
>> tsubst_copy_and_build.

>> This patch introduces a sentinel to reset suppress_location_wrappers
>> to zero, and uses it for template class and decl instantiation,
>> including constraint checking.

> Instead of a new class, let's add this to saved_scope and
> push_to/pop_from_top_level.

Like this?  Regstrapped on x86_64- and i686-linux-gnu.


[PR87768] reset location wrapper suppression when reentering top level

Concepts-checking and other kinds of early tsubsting may often take
place while location wrappers are suppressed, e.g. because we've
triggered template instantiation within template parameter lists.

With that, exprs that are usually wrapped by VIEW_CONVERT_EXPRs
location wrappers may end up wrapped by NON_LVALUE_EXPRs that are not
marked as location wrappers.  If such NON_LVALUE_EXPRs tsubsted exprs
undergo another round of tsubsting, say for constraint checking, or
even for another round of specialization, they will be rejected by
tsubst_copy_and_build.

This patch arranges for suppress_location_wrappers to be saved and
reset when pushing to the top level, and restored when popping from
it.


for  gcc/cp/ChangeLog

PR c++/87768
* cp-tree.h (saved_scope): Add suppress_location_wrappers.
* name-lookup.c (do_push_to_top_level): Save and reset it.
(do_pop_from_top_level): Restore it.

for  gcc/testsuite/ChangeLog

PR c++/87768
* g++.dg/concepts/pr87768.C: New.
---
 gcc/cp/cp-tree.h|1 +
 gcc/cp/name-lookup.c|3 +++
 gcc/testsuite/g++.dg/concepts/pr87768.C |   14 ++
 3 files changed, 18 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/concepts/pr87768.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 6a2004330d23..c3a53b8292cf 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -1625,6 +1625,7 @@ struct GTY(()) saved_scope {
 
   int x_processing_template_decl;
   int x_processing_specialization;
+  int suppress_location_wrappers;
   BOOL_BITFIELD x_processing_explicit_instantiation : 1;
   BOOL_BITFIELD need_pop_function_context : 1;
 
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index b65fd5f463a1..d7b9029b0a3a 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -7133,6 +7133,7 @@ do_push_to_top_level (void)
   s->function_decl = current_function_decl;
   s->unevaluated_operand = cp_unevaluated_operand;
   s->inhibit_evaluation_warnings = c_inhibit_evaluation_warnings;
+  s->suppress_location_wrappers = suppress_location_wrappers;
   s->x_stmt_tree.stmts_are_full_exprs_p = true;
 
   scope_chain = s;
@@ -7143,6 +7144,7 @@ do_push_to_top_level (void)
   push_class_stack ();
   cp_unevaluated_operand = 0;
   c_inhibit_evaluation_warnings = 0;
+  suppress_location_wrappers = 0;
 }
 
 static void
@@ -7175,6 +7177,7 @@ do_pop_from_top_level (void)
   current_function_decl = s->function_decl;
   cp_unevaluated_operand = s->unevaluated_operand;
   c_inhibit_evaluation_warnings = s->inhibit_evaluation_warnings;
+  suppress_location_wrappers = s->suppress_location_wrappers;
 
   /* Make this saved_scope structure available for reuse by
  push_to_top_level.  */
diff --git a/gcc/testsuite/g++.dg/concepts/pr87768.C 
b/gcc/testsuite/g++.dg/concepts/pr87768.C
new file mode 100644
index ..de436e5d9edd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr87768.C
@@ -0,0 +1,14 @@
+// { dg-do compile { target c++17 } }
+// { dg-options "-fconcepts" }
+
+struct a {};
+template  using b = a;
+
+template  struct c;
+template 
+  requires requires(d e) { e[0]; }
+struct c {
+  static constexpr bool f = [] { return false; }();
+};
+
+struct g : b::f> {};


-- 
Alexandre Oliva, freedom fighter   https://FSFLA.org/blogs/lxo
Be the change, be Free! FSF Latin America board member
GNU Toolchain EngineerFree Software Evangelist
Hay que enGNUrecerse, pero sin perder la terGNUra jamás-GNUChe

Re: [C++ Patch] [PR c++/88146] do not crash synthesizing inherited ctor(...)

[Alexandre Oliva]

On Jan  4, 2019, Jason Merrill  wrote:

> if (tree fn = cp_get_callee_fndecl_nofold (expr))
>   if (DECL_CONSTRUCTOR_P (fn) || DECL_DESTRUCTOR_P (fn))

> ?  The other patch is OK with that change.

Thanks, I've regstrapped this on x86_64- and i686-linux-gnu, and I'm
checking it in momentarily.


[PR88146] avoid diagnostics diffs if cdtor_returns_this

Diagnostics for testsuite/g++.dg/cpp0x/inh-ctor32.C varied across
platforms.  Specifically, on ARM, the diagnostics within the subtest
derived_ctor::inherited_derived_ctor::constexpr_noninherited_ctor did
not match those displayed on other platforms, and the test failed.

The difference seemed to have to do with locations assigned to ctors,
but it was more subtle: on ARM, the instantiation of bor's template
ctor was nested within the instantiation of bar's template ctor
inherited from bor.  The reason turned out to be related with the
internal return type of ctors: arm_cxx_cdtor_returns_this is enabled
for because of AAPCS, while cxx.cdtor_returns_this is disabled on most
other platforms.  While convert_to_void returns early with a VOID
expr, the non-VOID return type of the base ctor CALL_EXPR causes
convert_to_void to inspect the called decl for nodiscard attributes:
maybe_warn_nodiscard -> cp_get_fndecl_from_callee ->
maybe_constant_init -> cxx_eval_outermost_constant_expr ->
instantiate_constexpr_fns -> nested instantiation.

The internal return type assigned to a cdtor should not affect
instantiation (constexpr or template) decisions, IMHO.  We know it
affects diagnostics, but I have a hunch this might bring deeper issues
with it, so I've arranged for the CALL_EXPR handler in convert_to_void
to disregard cdtors, regardless of the ABI.


for  gcc/cp/ChangeLog

PR c++/88146
* cvt.c (convert_to_void): Handle all cdtor calls as if
returning void.
---
 gcc/cp/cvt.c |   10 ++
 1 file changed, 10 insertions(+)

diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
index 914281193980..82a44f353c76 100644
--- a/gcc/cp/cvt.c
+++ b/gcc/cp/cvt.c
@@ -1175,6 +1175,16 @@ convert_to_void (tree expr, impl_conv_void implicit, 
tsubst_flags_t complain)
   break;
 
 case CALL_EXPR:   /* We have a special meaning for volatile void fn().  */
+  /* cdtors may return this or void, depending on
+targetm.cxx.cdtor_returns_this, but this shouldn't affect our
+decisions here: neither nodiscard warnings (nodiscard cdtors
+are nonsensical), nor should any constexpr or template
+instantiations be affected by an ABI property that is, or at
+least ought to be transparent to the language.  */
+  if (tree fn = cp_get_callee_fndecl_nofold (expr))
+   if (DECL_CONSTRUCTOR_P (fn) || DECL_DESTRUCTOR_P (fn))
+ return expr;
+
   maybe_warn_nodiscard (expr, implicit);
   break;
 


-- 
Alexandre Oliva, freedom fighter   https://FSFLA.org/blogs/lxo
Be the change, be Free! FSF Latin America board member
GNU Toolchain EngineerFree Software Evangelist
Hay que enGNUrecerse, pero sin perder la terGNUra jamás-GNUChe

Re: [C++ PATCH] [PR86648] use auto identifier for class placeholder templates

[Alexandre Oliva]

On Jan  8, 2019, Jason Merrill  wrote:

> On 12/28/18 2:45 PM, Alexandre Oliva wrote:
>> +  if (template_placeholder_p (t)
>> +  && DECL_P (CLASS_PLACEHOLDER_TEMPLATE (t))
>> +  && TYPE_IDENTIFIER (TREE_TYPE (CLASS_PLACEHOLDER_TEMPLATE (t

> Are these extra checks needed?

Nope, that was just my overly defensive programming ;-)

> I would expect them to be true whenever template_placeholder_p is.

Testing agrees with your expectation.  Here's a test without the extra
checks, regstrapped on x86_64- and i686-linux-gnu.  Ok to install?


[PR86648] use auto identifier for class placeholder templates

dwarf2out recognizes unspecified auto types by the identifier.  C++
template class placeholders are unspecified auto types that take the
identifier of the class rather than those used by preexisting auto
types, so dwarf2out ICEs when it finds one of those.  Alas, they may
be visible to dwarf2out, since the types of e.g. static data members
of templates are only deduced at member instantiation, i.e., if the
data member is actually referenced, but the data member is added as a
field, still with unspecified auto placeholder type, when the
enclosing class is instantiated.

I've changed placeholder creator to use an auto identifier instead,
which allowed dropping the placeholder test in C++'s is_auto (alas, it
can't be used in dwarf2out, think LTO).  To avoid losing information
in error messages and dumps and whatnot, I've added code to recognize
placeholders for template classes say A and print them out as
A<...auto...>.

for  gcc/cp/ChangeLog

PR c++/86648
* pt.c (make_template_placeholder): Use auto_identifier.
(is_auto): Drop CLASS_PLACEHOLDER_TEMPLATE test.
* error.c (dump_type): Handle template placeholders.
* cxx-pretty-print.c (pp_cx_unqualified_id): Likewise.

for  gcc/testsuite/ChangeLog

PR c++/86648
* gcc.dg/cpp1z/pr86648.C: New.
---
 gcc/cp/cxx-pretty-print.c|8 +++-
 gcc/cp/error.c   |6 ++
 gcc/cp/pt.c  |5 ++---
 gcc/testsuite/g++.dg/cpp1z/pr86648.C |5 +
 4 files changed, 20 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/pr86648.C

diff --git a/gcc/cp/cxx-pretty-print.c b/gcc/cp/cxx-pretty-print.c
index 47eebd1729a3..a114d6654e91 100644
--- a/gcc/cp/cxx-pretty-print.c
+++ b/gcc/cp/cxx-pretty-print.c
@@ -187,7 +187,13 @@ pp_cxx_unqualified_id (cxx_pretty_printer *pp, tree t)
 
 case TEMPLATE_TYPE_PARM:
 case TEMPLATE_TEMPLATE_PARM:
-  if (TYPE_IDENTIFIER (t))
+  if (template_placeholder_p (t))
+   {
+ t = TREE_TYPE (CLASS_PLACEHOLDER_TEMPLATE (t));
+ pp_cxx_unqualified_id (pp, TYPE_IDENTIFIER (t));
+ pp_string (pp, "<...auto...>");
+   }
+  else if (TYPE_IDENTIFIER (t))
pp_cxx_unqualified_id (pp, TYPE_IDENTIFIER (t));
   else
pp_cxx_canonical_template_parameter (pp, t);
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index 367721328834..f585d5fe58fe 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -546,6 +546,12 @@ dump_type (cxx_pretty_printer *pp, tree t, int flags)
   pp_cxx_cv_qualifier_seq (pp, t);
   if (tree c = PLACEHOLDER_TYPE_CONSTRAINTS (t))
pp_cxx_constrained_type_spec (pp, c);
+  else if (template_placeholder_p (t))
+   {
+ t = TREE_TYPE (CLASS_PLACEHOLDER_TEMPLATE (t));
+ pp_cxx_tree_identifier (pp, TYPE_IDENTIFIER (t));
+ pp_string (pp, "<...auto...>");
+   }
   else if (TYPE_IDENTIFIER (t))
pp_cxx_tree_identifier (pp, TYPE_IDENTIFIER (t));
   else
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index c6fc1cfeffb2..8e3311b63ef0 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -26456,7 +26456,7 @@ make_auto (void)
 tree
 make_template_placeholder (tree tmpl)
 {
-  tree t = make_auto_1 (DECL_NAME (tmpl), true);
+  tree t = make_auto_1 (auto_identifier, true);
   CLASS_PLACEHOLDER_TEMPLATE (t) = tmpl;
   return t;
 }
@@ -27389,8 +27389,7 @@ is_auto (const_tree type)
 {
   if (TREE_CODE (type) == TEMPLATE_TYPE_PARM
   && (TYPE_IDENTIFIER (type) == auto_identifier
- || TYPE_IDENTIFIER (type) == decltype_auto_identifier
- || CLASS_PLACEHOLDER_TEMPLATE (type)))
+ || TYPE_IDENTIFIER (type) == decltype_auto_identifier))
 return true;
   else
 return false;
diff --git a/gcc/testsuite/g++.dg/cpp1z/pr86648.C 
b/gcc/testsuite/g++.dg/cpp1z/pr86648.C
new file mode 100644
index ..20ee4c8c0d44
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/pr86648.C
@@ -0,0 +1,5 @@
+// { dg-do compile { target c++17 } }
+
+template  class A;
+template  struct B { static A a{T::a}; };
+void foo () { B a; }


-- 
Alexandre Oliva, freedom fighter   https://FSFLA.org/blogs/lxo
Be the change, be Free! FSF Latin America board member
GNU Toolchain EngineerFree Software Evangelist
Hay que enGNUrecerse, pero sin perder la terGNUra jamás-GNUChe

libgo patch committed: Drop G before changing status

[Ian Lance Taylor]

This libgo patch by Cherry Zhang changes the runtime package to call
dropg before changing the  g status to _Grunnable/_Gwaiting.
Currently, we dropg (which clears gp.m) after we CAS the g status to
_Grunnable or _Gwaiting.  Immediately after CASing the g status,
another thread may CAS it to _Gscan status and scan its stack.  With
precise stack scan, it accesses gp.m in order to switch to g and back
(in doscanstackswitch).  This races with dropg.  If doscanstackswitch
reads gp.m, then dropg runs, when we restore the m at the end of the
scan it will set to a stale value.  Worse, if dropg runs after
doscanstackswitch sets the new m, gp will be running with a nil m.

To fix this, we do dropg before CAS g status to _Grunnable or
_Gwaiting.  We can do this safely if we are CASing from _Grunning, as
we own the g when it is in _Grunning.  There is one case where we CAS
from _Gsyscall to _Grunnable.  It is not safe to dropg when it is in
_Gsyscall, as precise stack scan needs to read gp.m in order to signal
the m.  So we need to introduce a transient state, _Gexitingsyscall,
between _Gsyscall and _Grunnable, where the GC should not scan its
stack.

Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 267989)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-ee94431c133a90ca5c3c5ebbebcb019c60258dac
+d6576c83016d856217758c06d945bfc363ffb817
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/go/runtime/proc.go
===
--- libgo/go/runtime/proc.go(revision 267941)
+++ libgo/go/runtime/proc.go(working copy)
@@ -956,6 +956,10 @@ loop:
break loop
}
 
+   case _Gexitingsyscall:
+   // This is a transient state during which we should not 
scan its stack.
+   // Try again.
+
case _Gscanwaiting:
// newstack is doing a scan for us right now. Wait.
 
@@ -2635,8 +2639,8 @@ func park_m(gp *g) {
traceGoPark(_g_.m.waittraceev, _g_.m.waittraceskip)
}
 
-   casgstatus(gp, _Grunning, _Gwaiting)
dropg()
+   casgstatus(gp, _Grunning, _Gwaiting)
 
if _g_.m.waitunlockf != nil {
fn := *(*func(*g, unsafe.Pointer) 
bool)(unsafe.Pointer(&_g_.m.waitunlockf))
@@ -2660,8 +2664,8 @@ func goschedImpl(gp *g) {
dumpgstatus(gp)
throw("bad g status")
}
-   casgstatus(gp, _Grunning, _Grunnable)
dropg()
+   casgstatus(gp, _Grunning, _Grunnable)
lock()
globrunqput(gp)
unlock()
@@ -3054,8 +3058,9 @@ func exitsyscallfast_pidle() bool {
 func exitsyscall0(gp *g) {
_g_ := getg()
 
-   casgstatus(gp, _Gsyscall, _Grunnable)
+   casgstatus(gp, _Gsyscall, _Gexitingsyscall)
dropg()
+   casgstatus(gp, _Gexitingsyscall, _Grunnable)
lock()
_p_ := pidleget()
if _p_ == nil {
Index: libgo/go/runtime/runtime2.go
===
--- libgo/go/runtime/runtime2.go(revision 267941)
+++ libgo/go/runtime/runtime2.go(working copy)
@@ -70,6 +70,12 @@ const (
// stack is owned by the goroutine that put it in _Gcopystack.
_Gcopystack // 8
 
+   // _Gexitingsyscall means this goroutine is exiting from a
+   // system call. This is like _Gsyscall, but the GC should not
+   // scan its stack. Currently this is only used in exitsyscall0
+   // as a transient state when it drops the G.
+   _Gexitingsyscall // 9
+
// _Gscan combined with one of the above states other than
// _Grunning indicates that GC is scanning the stack. The
// goroutine is not executing user code and the stack is owned
Index: libgo/go/runtime/traceback_gccgo.go
===
--- libgo/go/runtime/traceback_gccgo.go (revision 267941)
+++ libgo/go/runtime/traceback_gccgo.go (working copy)
@@ -122,13 +122,14 @@ func isExportedRuntime(name string) bool
 }
 
 var gStatusStrings = [...]string{
-   _Gidle:  "idle",
-   _Grunnable:  "runnable",
-   _Grunning:   "running",
-   _Gsyscall:   "syscall",
-   _Gwaiting:   "waiting",
-   _Gdead:  "dead",
-   _Gcopystack: "copystack",
+   _Gidle:   "idle",
+   _Grunnable:   "runnable",
+   _Grunning:"running",
+   _Gsyscall:"syscall",
+   _Gwaiting:"waiting",
+   _Gdead:   "dead",
+   _Gcopystack:  "copystack",
+   _Gexitingsyscall: "exiting syscall",
 }
 
 func goroutineheader(gp *g) {

Re: [PATCH,Fortran][RFC] PR 87939, 87326 - STAT= and ERRMSG= specifiers in several image control statements; NEW_INDEX= specifier in FORM TEAM statement

[Steve Kargl]

Nathan,

Thanks for taking an interesting in improving gfortran.  A
scan of the bug database certainly suggests we can use the
help particularly with coarray bugs.  Before we can go much
further, do you have a copyright assignment on file the FSF.
If not, please see https://gcc.gnu.org/contribute.html

-- 
steve

On Wed, Jan 16, 2019 at 06:16:12PM -0600, Nathan Weeks wrote:
> Hi all,
> 
> To facilitate more complete Fortran 2018 failed images support, I'm
> particularly interested in interested in seeing PR 87939 eventually
> resolved (i.e., allow STAT= and ERRMSG= specifiers in FORM TEAM,
> CHANGE TEAM, SYNC TEAM, END TEAM, and CRITICAL statements). To get the
> ball rolling (I realize that the boat has been missed for this kind of
> change in GCC 9 trunk), I've attempted the following patch (which,
> since it was convenient to do while modifying FORM TEAM-related code,
> also adds the NEW_INDEX= specifier to the FORM TEAM statement as
> desired in PR 87326).
> 
> This is the first gfortran patch I've attempted, and I certainly could
> have made some noob mistakes, so verbose feedback would be
> appreciated.
> 
> A few comments:
> 
> * In resolve.c, the newly-added functions that type check STAT= and
> ERRMSG= arguments for FORM TEAM, CHANGE TEAM, and SYNC TEAM also add
> (previously-absent) type checking for their TEAM_TYPE arguments. If
> it's more appropriate, I could separate this change into its own PR.
> 
> * The existing -fcoarray=lib implementation of CRITICAL acquires a
> LOCK on a lock variable on image 1 (in the current team). However, a
> CRITICAL statement stat-value of STAT_FAILED_IMAGE (i.e., the image
> that enter the CRITICAL construct failed) is analogous to the LOCK
> stat-value of STAT_UNLOCKED_FAILED_IMAGE (i.e., the image that
> acquired the lock failed---see section 11.6.11 (7 & 10) in Fortran
> 2018 draft N2146), whereas a LOCK STAT_FAILED_IMAGE means the image on
> which the lock variable resides has failed (no analog in the CRITICAL
> statement, which is oblivious to this underlying implementation). So
> in addition to adding the stat value STAT_UNLOCKED_FAILED_IMAGE to
> libgfortran.h & libcaf.h, I had CRITICAL swap a LOCK
> STAT_UNLOCKED_FAILED_IMAGE for STAT_FAILED_IMAGE, and (perhaps
> unimaginatively) a LOCK STAT_FAILED_IMAGE for
> STAT_UNLOCKED_FAILED_IMAGE (which, while it has no defined meaning for
> a CRITICAL statement, fits the definition of a "processor-dependent
> value other than STAT_FAILED_IMAGE").
> 
> * A couple negative tests for syntax errors (coarray_critical_2.f90 &
> team_end_2.f90) fail due to spurious "Error: Expecting END PROGRAM
> statement at (1)" errors that are also emitted by gfortran 8.2.0 as
> well.
> 
> Thanks,
> 
> --
> Nathan
> 
> frontend:
> 
> 2019-01-16  Nathan Weeks  
> 
> PR fortran/87939
> PR fortran/87326
> * gfortran.h: Add an additional gfc_expr member to struct gfc_code.
> * libcaf.h: Add support for STAT_UNLOCKED_FAILED_IMAGE.
> * match.c (gfc_match_critical): Add STAT= and ERRMSG=.
> (gfc_match_change_team): Likewise.
> (gfc_match_end_team): Likewise.
> (gfc_match_sync_team): Likewise.
> (gfc_match_form_team): Add STAT=, ERRMSG=, and NEW_INDEX=.
> * resolve.c (resolve_form_team): New. Type check team-variable
> argument in
> addition to new STAT= and ERRMSG= arguments.
> (resolve_change_sync_team): New. Adds type checking for team-value
> argument.
> (resolve_end_team): New.
> (resolve_critical): Add STAT= and ERRMSG=.
> * trans-decl.c (gfc_build_builtin_function_decls): Additional stat,
> errmsg, and errmsg_len arguments to _gfortran_caf_form_team(),
> _gfortran_caf_change_team(), _gfortran_caf_end_team(), and
> _gfortran_caf_sync_team(), and additional new_index argument to
> _gfortran_caf_form_team().
> * trans-stmt.c (gfc_trans_form_team): Support STAT=, ERRMSG=, and
> NEW_INDEX=.
> (gfc_trans_change_team): Support STAT= and ERRMSG=.
> (gfc_trans_end_team): Likewise.
> (gfc_trans_sync_team): Likewise.
> (gfc_trans_critical): Likewise. Also support assigning 
> STAT_FAILED_IMAGE
> to a stat-variable.
> 
> libgfortran:
> 
> 2019-01-16  Nathan Weeks  
> 
> PR fortran/87939
> * libgfortran.h: Add support for STAT_UNLOCKED_FAILED_IMAGE
> 
> testsuite:
> 
> 2019-01-16  Nathan Weeks  
> 
> PR fortran/87939
> PR fortran/87326
> * gfortran.dg/coarray_critical_2.f90: New test
> * gfortran.dg/coarray_critical_3.f90: New test
> * gfortran.dg/coarray_critical_4.f90: New test
> * gfortran.dg/team_change_2.f90: New test
> * gfortran.dg/team_change_3.f90: New test
> * gfortran.dg/team_end_2.f90: New test
> * gfortran.dg/team_end_3.f90: New test
> * gfortran.dg/team_form_2.f90: New test
> * gfortran.dg/team_form_3.f90: New test
>  

Re: [wwwdoc][Patch] Mention Loongson 3a1000 3a2000 3a3000 2k1000 support in gcc9

[Paul Hua]

Hi Gerald,

Updated version, please review.

Thanks.

On Mon, Jan 14, 2019 at 7:46 AM Gerald Pfeifer  wrote:
>
> Hi Paul,
>
> On Mon, 31 Dec 2018, Paul Hua wrote:
> > The attached patch mention Loongson 3a1000 3a2000 3a3000 2k1000 support
> > in gcc9.
>
> thanks for putting this together.  Only a couple of editorial changes:
>
> Index: changes.html
> ===
> +  
> +The Loongson loongson-mmi and loongson-ext
> +extension has now been splited from loongson3a:
>
> "...extensions have been split..."
>
> +
> +   loongson-mmi which contains
> +   the Loongson MultiMedia extension Instructions operations.
> +   loongson-ext which contains
> +   the Loongson EXTension instructions.
>
> Here I'd omit omit the two instances of "which"
>
> +The Loongson EXTension R2 instructions is now supported.
>
> "is" -> "are"
>
> +Use -mxxx or -mno-xxx will enable or disable those extersions.
>
> Since it's only two options, how about listing both of them?
>
> "extersion" -> "extension"
>
> +for example: Using 
> -mloongson-mmi/-mno-loongson-mmi
> +will enable/disable Loongson MultiMedia Instructions extensions.
>
> "...the Loongson MultiMedia Instructions extension."
>
> +   which default enable loongson-mmi, 
> loongson-ext.
>
> "which enables ... by default"  (also in the following two items)
>
>
> Please look into those comments and then simply post the updated patch
> as you're committing it.
>
> Gerald
Index: changes.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-9/changes.html,v
retrieving revision 1.35
diff -u -r1.35 changes.html
--- changes.html	15 Jan 2019 13:17:49 -	1.35
+++ changes.html	17 Jan 2019 02:03:20 -
@@ -261,6 +261,45 @@
   
 
 
+MIPS
+
+  
+The Loongson loongson-mmi and loongson-ext
+extensions have been split from loongson3a:
+
+   loongson-mmi contains
+   the Loongson MultiMedia extension Instructions operations.
+   loongson-ext contains 
+   the Loongson EXTension instructions.
+
+  
+The Loongson EXTension R2 instructions are now supported.
+
+   loongson-ext2 which contains the Loongson EXTension R2 instructions.
+
+Use -m[no-]loongson-mmi -m[no-]loongson-ext -m[no-]loongson-ext2
+will enable or disable those extensions,
+for example: Using -mloongson-mmi/-mno-loongson-mmi 
+will enable/disable the Loongson MultiMedia Instructions extensions.
+  
+Support has been added for the following processors
+(GCC identifiers in parentheses):
+
+	Loongson 3A1000 (gs464)
+	which enables loongson-mmi, loongson-ext by default.
+	Loongson 3A2000/3A3000 (gs464e)
+	which enables loongson-mmi, loongson-ext, loongson-ext2 by default.
+	Loongson 2K1000 (gs264e)
+	which enables loongson-ext, loongson-ext2, msa by default.
+
+The GCC identifiers can be used
+as arguments to the -mcpu or -mtune options,
+for example: -mcpu=gs464 or
+-mtune=gs464e or as arguments to the equivalent target
+attributes and pragmas.
+  
+
+
 
 
 

Re: [PATCH] avoid issuing -Warray-bounds during folding (PR 88800)

[Martin Sebor]

On 1/16/19 6:14 PM, Jeff Law wrote:

On 1/15/19 8:21 AM, Martin Sebor wrote:

On 1/15/19 4:07 AM, Richard Biener wrote:

On Tue, Jan 15, 2019 at 1:08 AM Martin Sebor  wrote:


The gimple_fold_builtin_memory_op() function folds calls to memcpy
and similar to MEM_REF when the size of the copy is a small power
of 2, but it does so without considering whether the copy might
write (or read) past the end of one of the objects.  To detect
these kinds of errors (and help distinguish them from -Westrict)
the folder calls into the wrestrict pass and lets it diagnose them.
Unfortunately, that can lead to false positives for even some fairly
straightforward code that is ultimately found to be unreachable.
PR 88800 is a report of one such problem.

To avoid these false positives the attached patch adjusts
the function to avoid issuing -Warray-bounds for out-of-bounds
calls to memcpy et al.  Instead, the patch disables the folding
of such invalid calls (and only those).  Those that are not
eliminated during DCE or other subsequent passes are eventually
diagnosed by the wrestrict pass.

Since this change required removing the dependency of the detection
on the warning options (originally done as a micro-optimization to
avoid spending compile-time cycles on something that wasn't needed)
the patch also adds tests to verify that code generation is not
affected as a result of warnings being enabled or disabled.  With
the patch as is, the invalid memcpy calls end up emitted (currently
they are folded into equally invalid MEM_REFs).  At some point,
I'd like us to consider whether they should be replaced with traps
(possibly under the control of  as has been proposed a number of
times in the past.  If/when that's done, these tests will need to
be adjusted to look for traps instead.

Tested on x86_64-linux.


I've said in the past that I feel delaying of folding is wrong.

To understand, the PR is about emitting a warning for out-of-bound
accesses in a dead code region?


Yes.  I am keeping in my mind your preference of not delaying
the folding of valid code.



If we think delaying/disablign the folding is the way to go the
patch looks OK.


I do, at least for now.  I'm taking this as your approval to commit
the patch (please let me know if you didn't mean it that way).

Note we are in stage4, so we're supposed to be addressing regression
bugfixes and documentation issues.

So  I think Richi needs to be explicit about whether or not he wants
this in gcc-9 or if it should defer to gcc-10.

I have no technical objections to the patch and would easily ack it in
stage1 or stage3.


The warning is a regression introduced in GCC 8.  I was just about
to commit the fix so please let me know if I should hold off until
stage 1.

Martin

Re: PING #2 [PATCH] handle expressions in __builtin_has_attribute (PR 88383)

[Jeff Law]

On 1/15/19 9:31 AM, Martin Sebor wrote:
> Ping: https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00337.html
> 
> Jeff, do you have any more questions/concerns or is this patch
> good to commit?
I think both Jakub and I were concerned about handling expressions in
this code.  I don't recall a resolution on that fundamental question.

jeff

Re: [PATCH] avoid issuing -Warray-bounds during folding (PR 88800)

[Jeff Law]

On 1/15/19 8:21 AM, Martin Sebor wrote:
> On 1/15/19 4:07 AM, Richard Biener wrote:
>> On Tue, Jan 15, 2019 at 1:08 AM Martin Sebor  wrote:
>>>
>>> The gimple_fold_builtin_memory_op() function folds calls to memcpy
>>> and similar to MEM_REF when the size of the copy is a small power
>>> of 2, but it does so without considering whether the copy might
>>> write (or read) past the end of one of the objects.  To detect
>>> these kinds of errors (and help distinguish them from -Westrict)
>>> the folder calls into the wrestrict pass and lets it diagnose them.
>>> Unfortunately, that can lead to false positives for even some fairly
>>> straightforward code that is ultimately found to be unreachable.
>>> PR 88800 is a report of one such problem.
>>>
>>> To avoid these false positives the attached patch adjusts
>>> the function to avoid issuing -Warray-bounds for out-of-bounds
>>> calls to memcpy et al.  Instead, the patch disables the folding
>>> of such invalid calls (and only those).  Those that are not
>>> eliminated during DCE or other subsequent passes are eventually
>>> diagnosed by the wrestrict pass.
>>>
>>> Since this change required removing the dependency of the detection
>>> on the warning options (originally done as a micro-optimization to
>>> avoid spending compile-time cycles on something that wasn't needed)
>>> the patch also adds tests to verify that code generation is not
>>> affected as a result of warnings being enabled or disabled.  With
>>> the patch as is, the invalid memcpy calls end up emitted (currently
>>> they are folded into equally invalid MEM_REFs).  At some point,
>>> I'd like us to consider whether they should be replaced with traps
>>> (possibly under the control of  as has been proposed a number of
>>> times in the past.  If/when that's done, these tests will need to
>>> be adjusted to look for traps instead.
>>>
>>> Tested on x86_64-linux.
>>
>> I've said in the past that I feel delaying of folding is wrong.
>>
>> To understand, the PR is about emitting a warning for out-of-bound
>> accesses in a dead code region?
> 
> Yes.  I am keeping in my mind your preference of not delaying
> the folding of valid code.
> 
>>
>> If we think delaying/disablign the folding is the way to go the
>> patch looks OK.
> 
> I do, at least for now.  I'm taking this as your approval to commit
> the patch (please let me know if you didn't mean it that way).
Note we are in stage4, so we're supposed to be addressing regression
bugfixes and documentation issues.

So  I think Richi needs to be explicit about whether or not he wants
this in gcc-9 or if it should defer to gcc-10.

I have no technical objections to the patch and would easily ack it in
stage1 or stage3.

Jeff

Re: [PATCH] detect references to statics in inline function signatures (PR 88718)

[Joseph Myers]

On Wed, 16 Jan 2019, Martin Sebor wrote:

> +  /* Iterate over tentative records (all those at the head of the list
> + with a null FUNCTION) and either associate them with DECL when ADD
> + is set or remove them from it otherwise.  */
> +  for (c_inline_static *csi = c_inline_statics, *last = csi;
> +   csi; csi = csi->next)

(Here we start with last == c_inline_statics.)

> +  if (add)
> + {
> +   if (csi->static_decl)
> + csi->function = decl;
> + }

I don't see any way in which csi->static_decl can be NULL (both callers of 
record_inline_static appear only to use non-NULL values for that argument, 
and it being non-NULL seems implicit in the semantics of the function 
given by the comment above it).  That is, I don't see the use of the inner 
"if" here.

> +   if (last == c_inline_statics)
> + c_inline_statics = last = csi->next;
> +   else
> + last->next = csi->next;

Here, if either last or c_inline_statics changes, they remain the same, so 
I don't see how they can ever be different.

So I don't see the need for having the variable last at all, or the 
conditional "if (last == c_inline_statics)".  It's always the case that, 
if removing entries from the list, it's some number of leading entries, so 
I'd expect just "c_inline_statics = csi->next;" to be sufficient for 
removing an entry.

> +  /* FUNCTION is unset for a declaration whose signature references
> +  a static variable and for which a definition wasn't provided.  */
> +  if (!csi->function)
> + continue;

But those should have been removed after the declaration in all cases via 
a call to update_tentative_inline_static.  So why is this conditional 
needed here?

> +  /* Only extern functions are of interest.  STATIC_DECL is null
> +  for inline functions that have been defined that contain
> +  no references to statics (but whose subsequent declarations
> +  might).  */
> +  if (!DECL_EXTERNAL (csi->function)
> +   || !csi->static_decl)
> + continue;

As above, I don't see the need for the !csi->static_decl check, as 
csi->static_decl should never be NULL.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH] tighten up -Wbuiltin-declaration-mismatch (PR 86125, 88886, 86308)

[Martin Sebor]

On 7/5/18 2:22 PM, Jeff Law wrote:

On 06/28/2018 09:14 AM, Martin Sebor wrote:

On 06/27/2018 11:20 PM, Jeff Law wrote:

On 06/26/2018 05:32 PM, Martin Sebor wrote:

Attached is an updated patch to tighten up the warning and also
prevent ICEs in the middle-end like in PR 86308 or PR 86202.

I took Richard's suggestion to add the POINTER_TYPE_P() check
to detect pointer/integer conflicts.  That also avoids the ICEs
above.

I also dealt with the fileptr_type_node problem so that file
I/O built-ins can be declared to take any object pointer type
as an argument, and that argument has to be the same for all
them.

I'm not too happy about the interaction with -Wextra but short
of enabling the stricter checks even without it or introducing
multiple levels for -Wbuiltin-declaration-mismatch I don't see
a good alternative.

Martin

gcc-86125.diff


PR c/86125 - missing -Wbuiltin-declaration-mismatch on a mismatched
return type
PR middle-end/86308 - ICE in verify_gimple calling index() with an
invalid declaration
PR middle-end/86202 - ICE in get_range_info calling an invalid
memcpy() declaration

gcc/c/ChangeLog:

 PR c/86125
 PR middle-end/86202
 PR middle-end/86308
 * c-decl.c (match_builtin_function_types): Add arguments.
 (diagnose_mismatched_decls): Diagnose mismatched declarations
 of built-ins more strictly.
 * doc/invoke.texi (-Wbuiltin-declaration-mismatch): Update.

gcc/testsuite/ChangeLog:

 PR c/86125
 PR middle-end/86202
 PR middle-end/86308
 * gcc.dg/Wbuiltin-declaration-mismatch.c: New test.
 * gcc.dg/Wbuiltin-declaration-mismatch-2.c: New test.
 * gcc.dg/Wbuiltin-declaration-mismatch-3.c: New test.
 * gcc.dg/Wbuiltin-declaration-mismatch-4.c: New test.
 * gcc.dg/builtins-69.c: New test.


[ ... ]



diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index af16cfd..6c9e667 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -1628,43 +1628,82 @@ c_bind (location_t loc, tree decl, bool
is_global)
    bind (DECL_NAME (decl), decl, scope, false, nested, loc);
  }

+
  /* Subroutine of compare_decls.  Allow harmless mismatches in return
     and argument types provided that the type modes match.  This
function
-   return a unified type given a suitable match, and 0 otherwise.  */
+   returns a unified type given a suitable match, and 0 otherwise.  */

  static tree
-match_builtin_function_types (tree newtype, tree oldtype)
+match_builtin_function_types (tree newtype, tree oldtype,
+  tree *strict, unsigned *argno)

As Joseph notes, you need to update the function comment here.

[ ... ]


+  /* Store the first FILE* argument type seen (whatever it is),

+ and expect any subsequent declarations of file I/O built-ins
+ to refer to it rather than to fileptr_type_node which is just
+ void*.  */
+  static tree last_fileptr_type;

Is this actually safe?  Isn't the type in GC memory?  And if so, what
prevents it from being GC'd?  At the least I think you need to register
this as a GC root.  Why are we handling fileptr_types specially here to
begin with?


IIUC, garbage collection runs after front end processing (between
separate passes) so the node should not be freed while the front
end is holding on to it.  There are other examples in the FE of
similar static usage (e.g., in c-format.c).You've stuffed a potentially GC'd 
object into a static and that's going

to trigger a "is this correct/safe" discussion every time it's noticed :-)


I missed this response this summer and as I got busy with other
things forgot to follow up, but PR 6 that was just filed today
reminded me that this still is a problem.  Since the PR is a GCC 9
regression, as is 86308, I'd like to revive this patch.  It seems
that the only thing that prevented its approval was the use of
a static local variable and so...


Yes it's true that GC only happens at well known points and if an object
lives entirely in the front-end you can probably get away without the
GTY marker.  But then you have to actually prove there's nothing in the
middle/back ends that potentially call into this code.

I generally dislike that approach because it's bad from a long term
maintenance standpoint.  It's an implementation constraint that someone
has to remember forever to avoid hard to find bugs from being introduced.

Another way to help alleviate these concerns would be to assign the
object NULL once we're done parsing.

Or you can add a GTY marker.  There's a bit of overhead to this since
the GC system has to walk through all the registered roots.

Or you can conditionalize the code on some other variable which
indicates whether or not the parser is still running. "the_parser" might
be usable for this purpose.


...I added the GTY marker to the variable.


The code detects mismatches between arguments to different file
I/O functions, as in:

   struct SomeFile;

   // okay, FILE is struct SomeFile
   int fputc (int, struct SomeFile*);

   struct OtherFile;
  

Re: [PATCH 7/9] [libbacktrace] Handle DW_FORM_GNU_ref_alt

[Ian Lance Taylor]

On Wed, Jan 16, 2019 at 4:17 PM Tom de Vries  wrote:
>
> this handles DW_FORM_GNU_ref_alt which references the .debug_info
> section in the .gnu_debugaltlink file.
>
> OK for trunk?
>
> Thanks,
> - Tom
>
> On 11-12-18 11:14, Tom de Vries wrote:
> > 2018-12-10  Tom de Vries  
> >
> >   * dwarf.c (enum attr_val_encoding): Add ATTR_VAL_REF_ALT_INFO.
> >   (struct unit): Add low and high fields.
> >   (struct unit_vector): New type.
> >   (struct dwarf_data): Add units and units_counts fields.
> >   (read_attribute): Handle DW_FORM_GNU_ref_alt using
> >   ATTR_VAL_REF_ALT_INFO.
> >   (find_unit): New function.
> >   (find_address_ranges): Add and handle unit_tag parameter.
> >   (build_address_map): Add and handle units_vec parameter.
> >   (read_referenced_name_1): Handle DW_FORM_GNU_ref_alt.
> >   (build_dwarf_data): Pass units_vec to build_address_map.  Store 
> > resulting
> >   units vector.


> > @@ -281,6 +283,10 @@ struct unit
> >/* The offset of UNIT_DATA from the start of the information for
> >   this compilation unit.  */
> >size_t unit_data_offset;
> > +  /* Start of the compilation unit.  */
> > +  size_t low;
> > +  /* End of the compilation unit.  */
> > +  size_t high;

The comments should say what low and high are measured in, which I
guess is file offset.  Or is it offset from the start of UNIT_DATA?
Either way, If that is right, then the fields should be named
low_offset and high_offset.  Otherwise it seems easy to confuse with
function_addrs, where low and high refer to PC values.

Also if they are offsets from UNIT_DATA then size_t is OK, but if the
are file offsets they should be off_t.


> > @@ -2144,6 +2198,22 @@ read_referenced_name_1 (struct dwarf_data *ddata, 
> > struct unit *u,
> >|| val->encoding == ATTR_VAL_REF_UNIT)
> >  return read_referenced_name (ddata, u, val->u.uint, error_callback, 
> > data);
> >
> > +  if (val->encoding == ATTR_VAL_REF_ALT_INFO)
> > +{
> > +  struct unit *alt_unit
> > + = find_unit (ddata->altlink->units, ddata->altlink->units_count,
> > +  val->u.uint);
> > +  if (alt_unit == NULL)
> > + {
> > +   error_callback (data,
> > +   "Could not find unit for DW_FORM_GNU_ref_alt", 0);

s/Could/could/

or maybe just skip this error_callback call as discussed earlier.


> > +   return NULL;
> > + }
> > +  uint64_t unit_offset = val->u.uint - alt_unit->low;

Earlier a unit_offset was the offset of the unit within unit_data, but
here this is an offset within a single unit.  This should just be
called offset, which is the name used by read_referenced_name.

This is OK with those changes.

Thanks.

Ian

Re: [PATCH 7/9] [libbacktrace] Handle DW_FORM_GNU_ref_alt

[Tom de Vries]

Hi,

this handles DW_FORM_GNU_ref_alt which references the .debug_info
section in the .gnu_debugaltlink file.

OK for trunk?

Thanks,
- Tom

On 11-12-18 11:14, Tom de Vries wrote:
> 2018-12-10  Tom de Vries  
> 
>   * dwarf.c (enum attr_val_encoding): Add ATTR_VAL_REF_ALT_INFO.
>   (struct unit): Add low and high fields.
>   (struct unit_vector): New type.
>   (struct dwarf_data): Add units and units_counts fields.
>   (read_attribute): Handle DW_FORM_GNU_ref_alt using
>   ATTR_VAL_REF_ALT_INFO.
>   (find_unit): New function.
>   (find_address_ranges): Add and handle unit_tag parameter.
>   (build_address_map): Add and handle units_vec parameter.
>   (read_referenced_name_1): Handle DW_FORM_GNU_ref_alt.
>   (build_dwarf_data): Pass units_vec to build_address_map.  Store 
> resulting
>   units vector.
> ---
>  libbacktrace/dwarf.c | 101 
> ++-
>  1 file changed, 91 insertions(+), 10 deletions(-)
> 
> diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c
> index 99e5f4c3f51..9a0b93120c8 100644
> --- a/libbacktrace/dwarf.c
> +++ b/libbacktrace/dwarf.c
> @@ -143,6 +143,8 @@ enum attr_val_encoding
>ATTR_VAL_REF_UNIT,
>/* An offset to other data within the .dwarf_info section.  */
>ATTR_VAL_REF_INFO,
> +  /* An offset to other data within the alt .dwarf_info section.  */
> +  ATTR_VAL_REF_ALT_INFO,
>/* An offset to data in some other section.  */
>ATTR_VAL_REF_SECTION,
>/* A type signature.  */
> @@ -281,6 +283,10 @@ struct unit
>/* The offset of UNIT_DATA from the start of the information for
>   this compilation unit.  */
>size_t unit_data_offset;
> +  /* Start of the compilation unit.  */
> +  size_t low;
> +  /* End of the compilation unit.  */
> +  size_t high;
>/* DWARF version.  */
>int version;
>/* Whether unit is DWARF64.  */
> @@ -339,6 +345,14 @@ struct unit_addrs_vector
>size_t count;
>  };
>  
> +/* A growable vector of compilation unit pointer.  */
> +
> +struct unit_vector
> +{
> +  struct backtrace_vector vec;
> +  size_t count;
> +};
> +
>  /* The information we need to map a PC to a file and line.  */
>  
>  struct dwarf_data
> @@ -353,6 +367,10 @@ struct dwarf_data
>struct unit_addrs *addrs;
>/* Number of address ranges in list.  */
>size_t addrs_count;
> +  /* A sorted list of units.  */
> +  struct unit **units;
> +  /* Number of units in the list.  */
> +  size_t units_count;
>/* The unparsed .debug_info section.  */
>const unsigned char *dwarf_info;
>size_t dwarf_info_size;
> @@ -840,7 +858,7 @@ read_attribute (enum dwarf_form form, struct dwarf_buf 
> *buf,
> val->encoding = ATTR_VAL_NONE;
> return 1;
>   }
> -  val->encoding = ATTR_VAL_REF_SECTION;
> +  val->encoding = ATTR_VAL_REF_ALT_INFO;
>return 1;
>  case DW_FORM_GNU_strp_alt:
>{
> @@ -866,6 +884,34 @@ read_attribute (enum dwarf_form form, struct dwarf_buf 
> *buf,
>  }
>  }
>  
> +/* Compare a unit offset against a unit for bsearch.  */
> +
> +static int
> +units_search (const void *vkey, const void *ventry)
> +{
> +  const uintptr_t *key = (const uintptr_t *) vkey;
> +  const struct unit *entry = *((const struct unit *const *) ventry);
> +  uintptr_t offset;
> +
> +  offset = *key;
> +  if (offset < entry->low)
> +return -1;
> +  else if (offset >= entry->high)
> +return 1;
> +  else
> +return 0;
> +}
> +
> +/* Find a unit in PU containing OFFSET.  */
> +
> +static struct unit *
> +find_unit (struct unit **pu, size_t units_count, size_t offset)
> +{
> +  struct unit **u;
> +  u = bsearch (, pu, units_count, sizeof (struct unit *), 
> units_search);
> +  return u == NULL ? NULL : *u;
> +}
> +
>  /* Compare function_addrs for qsort.  When ranges are nested, make the
> smallest one sort last.  */
>  
> @@ -1299,7 +1345,7 @@ find_address_ranges (struct backtrace_state *state, 
> uintptr_t base_address,
>int is_bigendian, backtrace_error_callback error_callback,
>void *data, struct unit *u,
>struct unit_addrs_vector *addrs,
> -  struct dwarf_data *altlink)
> +  struct dwarf_data *altlink, enum dwarf_tag *unit_tag)
>  {
>while (unit_buf->left > 0)
>  {
> @@ -1322,6 +1368,9 @@ find_address_ranges (struct backtrace_state *state, 
> uintptr_t base_address,
>if (abbrev == NULL)
>   return 0;
>  
> +  if (unit_tag != NULL)
> + *unit_tag = abbrev->tag;
> +
>lowpc = 0;
>have_lowpc = 0;
>highpc = 0;
> @@ -1434,7 +1483,7 @@ find_address_ranges (struct backtrace_state *state, 
> uintptr_t base_address,
>   dwarf_str, dwarf_str_size,
>   dwarf_ranges, dwarf_ranges_size,
>   is_bigendian, error_callback, data,
> - u, addrs, altlink))
> +  

[PATCH,Fortran][RFC] PR 87939, 87326 - STAT= and ERRMSG= specifiers in several image control statements; NEW_INDEX= specifier in FORM TEAM statement

[Nathan Weeks]

Hi all,

To facilitate more complete Fortran 2018 failed images support, I'm
particularly interested in interested in seeing PR 87939 eventually
resolved (i.e., allow STAT= and ERRMSG= specifiers in FORM TEAM,
CHANGE TEAM, SYNC TEAM, END TEAM, and CRITICAL statements). To get the
ball rolling (I realize that the boat has been missed for this kind of
change in GCC 9 trunk), I've attempted the following patch (which,
since it was convenient to do while modifying FORM TEAM-related code,
also adds the NEW_INDEX= specifier to the FORM TEAM statement as
desired in PR 87326).

This is the first gfortran patch I've attempted, and I certainly could
have made some noob mistakes, so verbose feedback would be
appreciated.

A few comments:

* In resolve.c, the newly-added functions that type check STAT= and
ERRMSG= arguments for FORM TEAM, CHANGE TEAM, and SYNC TEAM also add
(previously-absent) type checking for their TEAM_TYPE arguments. If
it's more appropriate, I could separate this change into its own PR.

* The existing -fcoarray=lib implementation of CRITICAL acquires a
LOCK on a lock variable on image 1 (in the current team). However, a
CRITICAL statement stat-value of STAT_FAILED_IMAGE (i.e., the image
that enter the CRITICAL construct failed) is analogous to the LOCK
stat-value of STAT_UNLOCKED_FAILED_IMAGE (i.e., the image that
acquired the lock failed---see section 11.6.11 (7 & 10) in Fortran
2018 draft N2146), whereas a LOCK STAT_FAILED_IMAGE means the image on
which the lock variable resides has failed (no analog in the CRITICAL
statement, which is oblivious to this underlying implementation). So
in addition to adding the stat value STAT_UNLOCKED_FAILED_IMAGE to
libgfortran.h & libcaf.h, I had CRITICAL swap a LOCK
STAT_UNLOCKED_FAILED_IMAGE for STAT_FAILED_IMAGE, and (perhaps
unimaginatively) a LOCK STAT_FAILED_IMAGE for
STAT_UNLOCKED_FAILED_IMAGE (which, while it has no defined meaning for
a CRITICAL statement, fits the definition of a "processor-dependent
value other than STAT_FAILED_IMAGE").

* A couple negative tests for syntax errors (coarray_critical_2.f90 &
team_end_2.f90) fail due to spurious "Error: Expecting END PROGRAM
statement at (1)" errors that are also emitted by gfortran 8.2.0 as
well.

Thanks,

--
Nathan

frontend:

2019-01-16  Nathan Weeks  

PR fortran/87939
PR fortran/87326
* gfortran.h: Add an additional gfc_expr member to struct gfc_code.
* libcaf.h: Add support for STAT_UNLOCKED_FAILED_IMAGE.
* match.c (gfc_match_critical): Add STAT= and ERRMSG=.
(gfc_match_change_team): Likewise.
(gfc_match_end_team): Likewise.
(gfc_match_sync_team): Likewise.
(gfc_match_form_team): Add STAT=, ERRMSG=, and NEW_INDEX=.
* resolve.c (resolve_form_team): New. Type check team-variable
argument in
addition to new STAT= and ERRMSG= arguments.
(resolve_change_sync_team): New. Adds type checking for team-value
argument.
(resolve_end_team): New.
(resolve_critical): Add STAT= and ERRMSG=.
* trans-decl.c (gfc_build_builtin_function_decls): Additional stat,
errmsg, and errmsg_len arguments to _gfortran_caf_form_team(),
_gfortran_caf_change_team(), _gfortran_caf_end_team(), and
_gfortran_caf_sync_team(), and additional new_index argument to
_gfortran_caf_form_team().
* trans-stmt.c (gfc_trans_form_team): Support STAT=, ERRMSG=, and
NEW_INDEX=.
(gfc_trans_change_team): Support STAT= and ERRMSG=.
(gfc_trans_end_team): Likewise.
(gfc_trans_sync_team): Likewise.
(gfc_trans_critical): Likewise. Also support assigning STAT_FAILED_IMAGE
to a stat-variable.

libgfortran:

2019-01-16  Nathan Weeks  

PR fortran/87939
* libgfortran.h: Add support for STAT_UNLOCKED_FAILED_IMAGE

testsuite:

2019-01-16  Nathan Weeks  

PR fortran/87939
PR fortran/87326
* gfortran.dg/coarray_critical_2.f90: New test
* gfortran.dg/coarray_critical_3.f90: New test
* gfortran.dg/coarray_critical_4.f90: New test
* gfortran.dg/team_change_2.f90: New test
* gfortran.dg/team_change_3.f90: New test
* gfortran.dg/team_end_2.f90: New test
* gfortran.dg/team_end_3.f90: New test
* gfortran.dg/team_form_2.f90: New test
* gfortran.dg/team_form_3.f90: New test
* gfortran.dg/team_sync_1.f90: New test
* gfortran.dg/team_sync_2.f90: New test


stat-errmsg-new_index.diff
Description: Binary data

Re: [PATCH 1/9] [libbacktrace] Read .gnu_debugaltlink

[Ian Lance Taylor]

On Wed, Jan 16, 2019 at 2:48 PM Tom de Vries  wrote:
>
> For now, I've dropped the error callback for .gnu_debugaltlink.

This version is OK.

Thanks.

Ian

[PATCH] Fix failing filesystem tests on mingw targets

[Jonathan Wakely]

* config/abi/pre/gnu.ver (GLIBCXX_3.4.26): Add exports for fstream
constructors and open members taking wide strings. Fix patterns for
filesystem::path members to match wstring_view parameters. Add
exports for shared_ptr members used by directory iterators.
* src/c++17/fs_ops.cc (remove(const path&, error_code&)): Clear the
error code parameter if the file doesn't exist.
* src/filesystem/ops.cc (remove(const path&, error_code&)):
Likewise.
* testsuite/27_io/filesystem/operations/canonical.cc: Fix expected
values for mingw targets, where "/" is not an absolute path. Do not
test symlinks on mingw targets.
* testsuite/experimental/filesystem/operations/canonical.cc: Likewise.
* testsuite/27_io/filesystem/operations/copy.cc: Do not test symlinks
on mingw targets.
* testsuite/experimental/filesystem/operations/copy.cc: Likewise.
* testsuite/27_io/filesystem/operations/create_directories.cc: Check
that each component of the path is created.
* testsuite/experimental/filesystem/operations/create_directories.cc:
Likewise.
* testsuite/27_io/filesystem/operations/exists.cc: Do not test
permissions on mingw targets.
* testsuite/experimental/filesystem/operations/exists.cc: Likewise.
* testsuite/27_io/filesystem/operations/is_empty.cc: Likewise.
* testsuite/experimental/filesystem/operations/is_empty.cc: Likewise.
* testsuite/27_io/filesystem/operations/permissions.cc: XFAIL for
mingw targets.
* testsuite/experimental/filesystem/operations/permissions.cc:
Likewise.
* testsuite/27_io/filesystem/operations/remove.cc: Do not test
symlinks or permissions on mingw targets.
* testsuite/experimental/filesystem/operations/remove.cc: Likewise.
* testsuite/27_io/filesystem/operations/remove_all.cc: Do not test
symlinks on mingw targets.
* testsuite/experimental/filesystem/operations/remove_all.cc:
Likewise.
* testsuite/27_io/filesystem/operations/status.cc: Do not test
permissions on mingw targets.
* testsuite/27_io/filesystem/operations/weakly_canonical.cc: Do not
test symlinks on mingw targets.
* testsuite/experimental/filesystem/operations/space.cc: Fix test
for mingw targets.

Tested x86_64-linux and x86_64-w64-mingw32, committed to trunk.


commit 232bff17dd50f728d8140d25002dcb157e8838bc
Author: Jonathan Wakely 
Date:   Wed Jan 16 11:32:26 2019 +

Fix failing filesystem tests on mingw targets

* config/abi/pre/gnu.ver (GLIBCXX_3.4.26): Add exports for fstream
constructors and open members taking wide strings. Fix patterns for
filesystem::path members to match wstring_view parameters. Add
exports for shared_ptr members used by directory iterators.
* src/c++17/fs_ops.cc (remove(const path&, error_code&)): Clear the
error code parameter if the file doesn't exist.
* src/filesystem/ops.cc (remove(const path&, error_code&)):
Likewise.
* testsuite/27_io/filesystem/operations/canonical.cc: Fix expected
values for mingw targets, where "/" is not an absolute path. Do not
test symlinks on mingw targets.
* testsuite/experimental/filesystem/operations/canonical.cc: 
Likewise.
* testsuite/27_io/filesystem/operations/copy.cc: Do not test 
symlinks
on mingw targets.
* testsuite/experimental/filesystem/operations/copy.cc: Likewise.
* testsuite/27_io/filesystem/operations/create_directories.cc: Check
that each component of the path is created.
* 
testsuite/experimental/filesystem/operations/create_directories.cc:
Likewise.
* testsuite/27_io/filesystem/operations/exists.cc: Do not test
permissions on mingw targets.
* testsuite/experimental/filesystem/operations/exists.cc: Likewise.
* testsuite/27_io/filesystem/operations/is_empty.cc: Likewise.
* testsuite/experimental/filesystem/operations/is_empty.cc: 
Likewise.
* testsuite/27_io/filesystem/operations/permissions.cc: XFAIL for
mingw targets.
* testsuite/experimental/filesystem/operations/permissions.cc:
Likewise.
* testsuite/27_io/filesystem/operations/remove.cc: Do not test
symlinks or permissions on mingw targets.
* testsuite/experimental/filesystem/operations/remove.cc: Likewise.
* testsuite/27_io/filesystem/operations/remove_all.cc: Do not test
symlinks on mingw targets.
* testsuite/experimental/filesystem/operations/remove_all.cc:
Likewise.
* testsuite/27_io/filesystem/operations/status.cc: Do not test
permissions 

Re: [PATCH] c-family: Update unaligned adress of packed member check

[Jakub Jelinek]

On Mon, Jan 14, 2019 at 03:23:07PM -0800, H.J. Lu wrote:
> There are no regressions with this patch:
> 
> https://gcc.gnu.org/ml/gcc-patches/2019-01/msg00792.html

As the patch seems to be a step forward and fixes an important regression,
the patch is ok for trunk, but I'd like to keep discussions on the convert_p
stuff afterwards (see other mail).

Jakub

[PATCH] C++: Fix ICE when adding overloaded operator via using_decl (PR c++/88699)

[David Malcolm]

PR c++/88699 reports an ICE within this assertion in add_method:

  gcc_assert (!current_fns || !DECL_DESTRUCTOR_P (method));

when adding an overloaded operator to a class via a using_decl, due to
DECL_DESTRUCTOR_P requiring a FUNCTION_DECL, but "method" being a
USING_DECL.

This patch weakens the assertion to avoid testing DECL_DESTRUCTOR_P
for the case where "via_using" is true, fixing the ICE.

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

OK for trunk?

gcc/cp/ChangeLog:
PR c++/88699
* class.c (add_method): Don't use DECL_DESTRUCTOR_P on
USING_DECLs.

gcc/testsuite/ChangeLog:
PR c++/88699
* g++.dg/template/pr88699.C: New test.
---
 gcc/cp/class.c  |  2 +-
 gcc/testsuite/g++.dg/template/pr88699.C | 13 +
 2 files changed, 14 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/template/pr88699.C

diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index e7897f2..e8773c2 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -1134,7 +1134,7 @@ add_method (tree type, tree method, bool via_using)
 }
 
   /* A class should never have more than one destructor.  */
-  gcc_assert (!current_fns || !DECL_DESTRUCTOR_P (method));
+  gcc_assert (!current_fns || via_using || !DECL_DESTRUCTOR_P (method));
 
   current_fns = ovl_insert (method, current_fns, via_using);
 
diff --git a/gcc/testsuite/g++.dg/template/pr88699.C 
b/gcc/testsuite/g++.dg/template/pr88699.C
new file mode 100644
index 000..ecd26ce
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/pr88699.C
@@ -0,0 +1,13 @@
+// { dg-do compile }
+
+template 
+struct A {
+  void operator= (int);
+  template  class B;
+};
+template 
+template 
+struct A::B : A {
+  using A::operator=;
+  void operator= (B);
+};
-- 
1.8.5.3

Re: [PATCH] Don't add unnecessary var self-conflicts (PR tree-optimization/86214)

[Jeff Law]

On 1/16/19 3:49 PM, Jakub Jelinek wrote:
> Hi!
> 
> While looking at this PR (I've just started) I've noticed that
> add_stack_var_conflict is quite often called with x == y.
> We don't really need to record that a variable conflicts with itself,
> the only reader of the conflicts bitmaps, stack_var_conflict_p,
> starts with
>   if (x == y)
> return false;
> conflicts bitmap are set either by this function, or by the
>   EXECUTE_IF_SET_IN_BITMAP (work, 0, i, bi)
> {
>   struct stack_var *a = _vars[i];
>   if (!a->conflicts)
> a->conflicts = BITMAP_ALLOC (_var_bitmap_obstack);
>   bitmap_ior_into (a->conflicts, work);
> }
> code (where work isn't derived from any conflicts bitmap though, so
> doesn't care if we've added those self-conflicts or not).  The above
> bitmap_ior_into stuff actually always sets self-conflicts (if you think
> bitmap_clear_bit is worth it, I can add it afterwards though).
> 
> But I think the following patch is helpful, don't create the conflicts
> bitmaps at all if all we'd record is just self-conflict which we'll ignore.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2019-01-16  Jakub Jelinek  
> 
>   PR tree-optimization/86214
>   * cfgexpand.c (add_stack_var_conflict): Don't add any conflicts
>   if x == y.
Yea.  Seems reasonable.

jeff

Re: [PATCH] Don't DCE const/pure calls that can throw if cfg can't be altered (PR rtl-optimization/88870)

[Jakub Jelinek]

On Wed, Jan 16, 2019 at 03:48:07PM -0700, Jeff Law wrote:
> > 2019-01-16  Jakub Jelinek  
> > 
> > PR rtl-optimization/88870
> > * dce.c (deletable_insn_p): Never delete const/pure calls that can
> > throw if we can't alter the cfg or delete dead exceptions.
> > (mark_insn): Don't call find_call_stack_args for such calls.
> > 
> > * gcc.dg/pr88870.c: New test.
> OK.  Though I wonder if we want to continue to support
> -fnon-call-exceptions.  With GCJ gone is there any value left in that
> capability?  There's little doubt in my mind other parts of GCC are not
> -fnon-call-exception safe.

AFAIK Ada and Go use -fnon-call-exceptions by default and heavily rely on
it.

Jakub

Re: [PATCH] Don't DCE const/pure calls that can throw if cfg can't be altered (PR rtl-optimization/88870)

[Jeff Law]

On 1/16/19 3:43 PM, Jakub Jelinek wrote:
> Hi!
> 
> For normal instructions, deletable_insn_p has:
>   /* Don't delete insns that may throw if we cannot do so.  */
>   if (!(cfun->can_delete_dead_exceptions && can_alter_cfg)
>   && !insn_nothrow_p (insn))
> return false;
> 
> The following patch adds that for the const/pure non-looping calls
> that are handled earlier in the function as well (I haven't moved this test
> earlier so we don't check insn_nothrow_p on jump insns etc.).
> 
> The other change is just to handle those calls the same as non-const/pure,
> if we never consider them to be deletable, there is no point in
> find_call_stack_args for them, like we don't do that for normal calls.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2019-01-16  Jakub Jelinek  
> 
>   PR rtl-optimization/88870
>   * dce.c (deletable_insn_p): Never delete const/pure calls that can
>   throw if we can't alter the cfg or delete dead exceptions.
>   (mark_insn): Don't call find_call_stack_args for such calls.
> 
>   * gcc.dg/pr88870.c: New test.
OK.  Though I wonder if we want to continue to support
-fnon-call-exceptions.  With GCJ gone is there any value left in that
capability?  There's little doubt in my mind other parts of GCC are not
-fnon-call-exception safe.

jeff

[PATCH] Don't add unnecessary var self-conflicts (PR tree-optimization/86214)

[Jakub Jelinek]

Hi!

While looking at this PR (I've just started) I've noticed that
add_stack_var_conflict is quite often called with x == y.
We don't really need to record that a variable conflicts with itself,
the only reader of the conflicts bitmaps, stack_var_conflict_p,
starts with
  if (x == y)
return false;
conflicts bitmap are set either by this function, or by the
  EXECUTE_IF_SET_IN_BITMAP (work, 0, i, bi)
{
  struct stack_var *a = _vars[i];
  if (!a->conflicts)
a->conflicts = BITMAP_ALLOC (_var_bitmap_obstack);
  bitmap_ior_into (a->conflicts, work);
}
code (where work isn't derived from any conflicts bitmap though, so
doesn't care if we've added those self-conflicts or not).  The above
bitmap_ior_into stuff actually always sets self-conflicts (if you think
bitmap_clear_bit is worth it, I can add it afterwards though).

But I think the following patch is helpful, don't create the conflicts
bitmaps at all if all we'd record is just self-conflict which we'll ignore.

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

2019-01-16  Jakub Jelinek  

PR tree-optimization/86214
* cfgexpand.c (add_stack_var_conflict): Don't add any conflicts
if x == y.

--- gcc/cfgexpand.c.jj  2019-01-16 09:35:09.131247513 +0100
+++ gcc/cfgexpand.c 2019-01-16 20:14:11.445467399 +0100
@@ -470,6 +470,8 @@ add_stack_var_conflict (size_t x, size_t
 {
   struct stack_var *a = _vars[x];
   struct stack_var *b = _vars[y];
+  if (x == y)
+return;
   if (!a->conflicts)
 a->conflicts = BITMAP_ALLOC (_var_bitmap_obstack);
   if (!b->conflicts)

Jakub

Re: [PATCH 1/9] [libbacktrace] Read .gnu_debugaltlink

[Tom de Vries]

On 16-01-19 18:14, Ian Lance Taylor wrote:
> On Wed, Jan 16, 2019 at 8:26 AM Tom de Vries  wrote:
>>
>> On 16-01-19 01:56, Ian Lance Taylor wrote:
>>> On Tue, Dec 11, 2018 at 2:14 AM Tom de Vries  wrote:

 Read the elf file pointed at by the .gnu_debugaltlink section, and verify 
 that
 the build id matches.

 2018-11-11  Tom de Vries  

 * elf.c (elf_add): Add and handle with_buildid_data and
 with_buildid_size parameters.  Handle .gnu_debugaltlink section.
 (phdr_callback, backtrace_initialize): Add arguments to elf_add 
 calls.
 ---
>>>
>>>
>>>
>>> @@ -2899,6 +2918,27 @@ elf_add (struct backtrace_state *state, const
>>> char *filename, int descriptor,
 }
 }

 +  if (!debugaltlink_view_valid
 + && strcmp (name, ".gnu_debugaltlink") == 0)
 +   {
 + const char *debugaltlink_data;
 + size_t debugaltlink_name_len;
 +
 + if (!backtrace_get_view (state, descriptor, shdr->sh_offset,
 +  shdr->sh_size, error_callback, data,
 +  _view))
 +   goto fail;
 +
 + debugaltlink_view_valid = 1;
 + debugaltlink_data = (const char *) debugaltlink_view.data;
 + debugaltlink_name = debugaltlink_data;
 + debugaltlink_name_len = strnlen (debugaltlink_data, 
 shdr->sh_size);
 + debugaltlink_buildid_data = (debugaltlink_data
 +  + debugaltlink_name_len
 +  + 1);
 + debugaltlink_buildid_size = shdr->sh_size - 
 debugaltlink_name_len - 1;
 +   }
 +
>>>
>>> This doesn't look quite right.  debugaltlink_buildid_size is unsigned.
>>> If there is some misunderstanding of the format it's possible for
>>> strnlen to return shdr->sh_size.  If it does,
>>> debugaltlink_buildid_size will be set to a very large value.
>>>
>>
>> I see, thanks for finding that.
>>
>> Fixed like this:
>> ...
>> debugaltlink_name_len = strnlen (debugaltlink_data, shdr->sh_size);
>> if (debugaltlink_name_len < shdr->sh_size)
>>   {
>> /* Include terminating zero.  */
>> debugaltlink_name_len =+ 1;
>>
>> debugaltlink_buildid_data
>>   = debugaltlink_data + debugaltlink_name_len;
>> debugaltlink_buildid_size
>>   = shdr->sh_size - debugaltlink_name_len;
>>   }
>> ...
>>
 +  if (debugaltlink_name != NULL)
 +{
 +  int d;
 +
 +  d = elf_open_debugfile_by_debuglink (state, filename, 
 debugaltlink_name,
 +  0, error_callback, data);
 +  if (d >= 0)
 +   {
 + int ret;
 +
 + ret = elf_add (state, filename, d, base_address, error_callback, 
 data,
 +fileline_fn, found_sym, found_dwarf, 0, 1,
 +debugaltlink_buildid_data, 
 debugaltlink_buildid_size);
 + backtrace_release_view (state, _view, 
 error_callback,
 + data);
 + debugaltlink_view_valid = 0;
 + if (ret < 0)
 +   {
 + backtrace_close (d, error_callback, data);
 + return ret;
 +   }
 +   }
 +  else
 +   {
 + error_callback (data,
 + "Could not open .gnu_debugaltlink", 0);
 + /* Don't goto fail, but try continue without the info in the
 +.gnu_debugaltlink.  */
 +   }
 +}
>>>
>>> The strings passed to error_callback always start with a lowercase
>>> letter (unless they start with something like ELF) because the
>>> callback will most likely print them with some prefix.
>>>
>>
>> Fixed.
>>
>>> More seriously, we don't call error_callback in any cases that
>>> correspond to this.  We just carry on.
>>
>> Indeed, I noticed that, and filed PR88244 - "[libbacktrace] Failure to
>> open .gnu_debuglink is silent".
>>
>> [ The scenario there is: an executable has a .gnu_debuglink, but the
>> file the .gnu_debuglink is pointing to is absent, because f.i. it has
>> been removed, or moved to a different location. If a user runs this
>> executable and a backtrace is triggered, the information relating to the
>> functions in the executable will be missing in the backtrace, but
>> there's no error explaining to the user why that information is missing.
>>  Note: there is a default error "no debug info in ELF executable" in
>> elf_nodebug, but AFAIU this is not triggered if debug info for one of
>> the shared libraries is present. ]
>>
>> BTW, though in the code above an error_callback is called, we don't
>> error out, but do carry on afterwards (as the comment explicitly states).
>>
>>> Is there any reason to call
>>> 

[PATCH] Don't DCE const/pure calls that can throw if cfg can't be altered (PR rtl-optimization/88870)

[Jakub Jelinek]

Hi!

For normal instructions, deletable_insn_p has:
  /* Don't delete insns that may throw if we cannot do so.  */
  if (!(cfun->can_delete_dead_exceptions && can_alter_cfg)
  && !insn_nothrow_p (insn))
return false;

The following patch adds that for the const/pure non-looping calls
that are handled earlier in the function as well (I haven't moved this test
earlier so we don't check insn_nothrow_p on jump insns etc.).

The other change is just to handle those calls the same as non-const/pure,
if we never consider them to be deletable, there is no point in
find_call_stack_args for them, like we don't do that for normal calls.

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

2019-01-16  Jakub Jelinek  

PR rtl-optimization/88870
* dce.c (deletable_insn_p): Never delete const/pure calls that can
throw if we can't alter the cfg or delete dead exceptions.
(mark_insn): Don't call find_call_stack_args for such calls.

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

--- gcc/dce.c.jj2019-01-01 12:37:21.308906844 +0100
+++ gcc/dce.c   2019-01-16 11:39:13.432604633 +0100
@@ -108,7 +108,10 @@ deletable_insn_p (rtx_insn *insn, bool f
   /* We can delete dead const or pure calls as long as they do not
  infinite loop.  */
   && (RTL_CONST_OR_PURE_CALL_P (insn)
- && !RTL_LOOPING_CONST_OR_PURE_CALL_P (insn)))
+ && !RTL_LOOPING_CONST_OR_PURE_CALL_P (insn))
+  /* Don't delete calls that may throw if we cannot do so.  */
+  && ((cfun->can_delete_dead_exceptions && can_alter_cfg)
+ || insn_nothrow_p (insn)))
 return find_call_stack_args (as_a  (insn), false,
 fast, arg_stores);
 
@@ -201,7 +204,9 @@ mark_insn (rtx_insn *insn, bool fast)
  && !df_in_progress
  && !SIBLING_CALL_P (insn)
  && (RTL_CONST_OR_PURE_CALL_P (insn)
- && !RTL_LOOPING_CONST_OR_PURE_CALL_P (insn)))
+ && !RTL_LOOPING_CONST_OR_PURE_CALL_P (insn))
+ && ((cfun->can_delete_dead_exceptions && can_alter_cfg)
+ || insn_nothrow_p (insn)))
find_call_stack_args (as_a  (insn), true, fast, NULL);
 }
 }
--- gcc/testsuite/gcc.dg/pr88870.c.jj   2019-01-16 11:25:01.434552788 +0100
+++ gcc/testsuite/gcc.dg/pr88870.c  2019-01-16 11:24:48.172769383 +0100
@@ -0,0 +1,23 @@
+/* PR rtl-optimization/88870 */
+/* { dg-do compile } */
+/* { dg-options "-O1 -fexceptions -fnon-call-exceptions -ftrapv 
-fno-tree-dominator-opts" } */
+
+int a, b;
+
+void
+foo (int *x)
+{
+  int c = 0;
+  {
+int d;
+x = 
+for (;;)
+  {
+x = 
+b = 0;
+d = c + 1;
+b = c = 1;
+++a;
+  }
+  }
+}


Jakub

Re: [PATCH 2/9] [libbacktrace] Add altlink field to struct dwarf_data

[Ian Lance Taylor]

On Wed, Jan 16, 2019 at 2:18 PM Tom de Vries  wrote:
>
> On 16-01-19 18:17, Ian Lance Taylor wrote:
> > On Wed, Jan 16, 2019 at 8:33 AM Tom de Vries  wrote:
> >>
> >>> Why is it void**?
> >>
> >> It's really struct dwarf_data *, but struct dwarf_data is a type
> >> declared in dwarf.c, so it's not known in other files.
> >
> > It woud be OK to add "struct dwarf_data;" to internal.h and refer to
> > the struct, without dereferencing it, in elf.c.
>
> Updated accordingly.

This is OK.

Thanks.

I see that the struct declaration is in this patch.

Ian

libgo patch committed: Mark syscall functions noescape

[Ian Lance Taylor]

This patch by Cherry Zhang marks the syscall functions that call into
C code as noescape, so that calling syscall functions that take
pointers do not require allocation.  Bootstrapped and ran Go testsuite
on x86_64-pc-linux-gnu.  Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 267956)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-9a79c333e896ea49f6a708d459148074d29a2af6
+ee94431c133a90ca5c3c5ebbebcb019c60258dac
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/go/syscall/exec_linux.go
===
--- libgo/go/syscall/exec_linux.go  (revision 267941)
+++ libgo/go/syscall/exec_linux.go  (working copy)
@@ -62,6 +62,7 @@ func runtime_AfterFork()
 func runtime_AfterForkInChild()
 
 // Implemented in clone_linux.c
+//go:noescape
 func rawClone(flags _C_ulong, child_stack *byte, ptid *Pid_t, ctid *Pid_t, 
regs unsafe.Pointer) _C_long
 
 // Fork, dup fd onto 0..len(fd), and exec(argv0, argvv, envv) in child.
Index: libgo/go/syscall/mksyscall.awk
===
--- libgo/go/syscall/mksyscall.awk  (revision 267941)
+++ libgo/go/syscall/mksyscall.awk  (working copy)
@@ -98,6 +98,7 @@ BEGIN {
 printf("// Automatically generated wrapper for %s/%s\n", gofnname, cfnname)
 if (!(cfnname in cfns)) {
 cfns[cfnname] = 1
+printf("//go:noescape\n")
 printf("//extern %s\n", cfnname)
 printf("func c_%s(%s) %s\n", cfnname, cfnparams, cfnresult)
 }

Re: [PR c++/86610] lambda captures in templates

[Jason Merrill]

On 1/16/19 4:48 PM, Nathan Sidwell wrote:
This PR reports a bug where we select a non-const operator function and 
then apply it to a const object.  That's happening because the 
expression 'c[0]' is not dependent, so we figure end up resolving it. 
But the lambda capture logic doesn't capture 'c' at that point and we 
have a non-const qualified 'c'.  At instantiation time we do the capture 
and the by-value lambda results in const-qualified captures.


Jason, the orginal test in process_outer_var_ref looked a little funky 
-- why not just processing_template_decl? That would satisfy what the 
comment says it checking.  Anyway changing the test to check DECL's 
type-dependency makes the right things happen, and a bootstrap passes. 
Could you review please.


Hmm, I don't remember exactly my rationale for deferring captures within 
a template, but if this doesn't obviously break anything it seems 
reasonable.  Go ahead.


Jason

Re: [PATCH 2/9] [libbacktrace] Add altlink field to struct dwarf_data

[Ian Lance Taylor]

On Wed, Jan 16, 2019 at 2:20 PM Tom de Vries  wrote:
>
> On 16-01-19 17:34, Tom de Vries wrote:
> > On 16-01-19 17:33, Tom de Vries wrote:
> >> On 16-01-19 02:02, Ian Lance Taylor wrote:
> >>> On Tue, Dec 11, 2018 at 2:14 AM Tom de Vries  wrote:
> 
>  Add an altlink field to struct dwarf_data, and initialize it with the 
>  pointer
>  to the struct dwarf_data for the .gnu_debugaltlink.
> 
>  2018-11-11  Tom de Vries  
> 
>  * dwarf.c (struct dwarf_data): Add altlink field.
>  (backtrace_dwarf_add): Add and handle fileline_entry and
>  fileline_altlink parameters.
>  * elf.c (elf_add): Add and handle fileline_entry parameter.  Add 
>  args to
>  backtrace_dwarf_add call.
>  (phdr_callback, backtrace_initialize): Add arguments to elf_add 
>  calls.
>  * internal.h (backtrace_dwarf_add): Add fileline_entry and
>  fileline_altlink parameters.
>  * pecoff.c (coff_add): Add args to backtrace_dwarf_add call.
>  * xcoff.c (xcoff_add): Same.
> >>>
> >>>
>  @@ -2968,7 +2970,7 @@ build_dwarf_data (struct backtrace_state *state,
>    size_t dwarf_str_size,
>    int is_bigendian,
>    backtrace_error_callback error_callback,
>  - void *data)
>  + void *data, struct dwarf_data *altlink)
>   {
> >>>
> >>> error_callback and data should remain the last two parameters, as they
> >>> are for many of the functions in this file.
> >>>
> >>>
> >>
> >> Done.
> >>
>    @@ -3031,7 +3034,8 @@ backtrace_dwarf_add (struct backtrace_state 
>  *state,
>   size_t dwarf_str_size,
>   int is_bigendian,
>   backtrace_error_callback error_callback,
>  -void *data, fileline *fileline_fn)
>  +void *data, fileline *fileline_fn, void 
>  **fileline_entry,
>  +void *fileline_altlink)
> >>>
> >>> The new fileline_altlink parameter should come before error_callback,
> >>> as it is not error_callback/data and is not a result parameter.
> >>>
> >>
> >> Done.
> >>
> >>> What is fileline_entry for?
> >>
> >> There are two bits to this patch:
> >> - add fileline_entry parameter to elf_add.  This allows the callers of
> >>   elf_add access to the struct dwarf_data pointer corresponding to the
> >>   added elf.
> >> - add an altlink field to struct dwarf_data, and initialize it with the
> >>   pointer to the struct dwarf_data for the .gnu_debugaltlink.
> >>
> >> I've split the patch up this way now, hoping it will make things clearer
> >> and/or easier to review.
> >>
> >>> Why is it void**?
> >>
> >> It's really struct dwarf_data *, but struct dwarf_data is a type
> >> declared in dwarf.c, so it's not known in other files.
> >>
> >> Thanks,
> >> - Tom
> >>
> >> Here's the first part.
> >>
> >
> > And here's the second part.
>
> Updated to use 'struct dwarf_data' instead of 'void'.

I think you need to explicitly add a standalone line

struct dwarf_data;

 in internal.h before the declaration of backtrace_dwarf_add to avoid
compiler warnings in some cases.  (Like the declaration of
backtrace_state in backtrace.h.)

This is OK with that change.

Thanks.

Ian

Re: [PATCH 2/9] [libbacktrace] Add altlink field to struct dwarf_data

[Tom de Vries]

On 16-01-19 17:34, Tom de Vries wrote:
> On 16-01-19 17:33, Tom de Vries wrote:
>> On 16-01-19 02:02, Ian Lance Taylor wrote:
>>> On Tue, Dec 11, 2018 at 2:14 AM Tom de Vries  wrote:

 Add an altlink field to struct dwarf_data, and initialize it with the 
 pointer
 to the struct dwarf_data for the .gnu_debugaltlink.

 2018-11-11  Tom de Vries  

 * dwarf.c (struct dwarf_data): Add altlink field.
 (backtrace_dwarf_add): Add and handle fileline_entry and
 fileline_altlink parameters.
 * elf.c (elf_add): Add and handle fileline_entry parameter.  Add 
 args to
 backtrace_dwarf_add call.
 (phdr_callback, backtrace_initialize): Add arguments to elf_add 
 calls.
 * internal.h (backtrace_dwarf_add): Add fileline_entry and
 fileline_altlink parameters.
 * pecoff.c (coff_add): Add args to backtrace_dwarf_add call.
 * xcoff.c (xcoff_add): Same.
>>>
>>>
 @@ -2968,7 +2970,7 @@ build_dwarf_data (struct backtrace_state *state,
   size_t dwarf_str_size,
   int is_bigendian,
   backtrace_error_callback error_callback,
 - void *data)
 + void *data, struct dwarf_data *altlink)
  {
>>>
>>> error_callback and data should remain the last two parameters, as they
>>> are for many of the functions in this file.
>>>
>>>
>>
>> Done.
>>
   @@ -3031,7 +3034,8 @@ backtrace_dwarf_add (struct backtrace_state *state,
  size_t dwarf_str_size,
  int is_bigendian,
  backtrace_error_callback error_callback,
 -void *data, fileline *fileline_fn)
 +void *data, fileline *fileline_fn, void 
 **fileline_entry,
 +void *fileline_altlink)
>>>
>>> The new fileline_altlink parameter should come before error_callback,
>>> as it is not error_callback/data and is not a result parameter.
>>>
>>
>> Done.
>>
>>> What is fileline_entry for? 
>>
>> There are two bits to this patch:
>> - add fileline_entry parameter to elf_add.  This allows the callers of
>>   elf_add access to the struct dwarf_data pointer corresponding to the
>>   added elf.
>> - add an altlink field to struct dwarf_data, and initialize it with the
>>   pointer to the struct dwarf_data for the .gnu_debugaltlink.
>>
>> I've split the patch up this way now, hoping it will make things clearer
>> and/or easier to review.
>>
>>> Why is it void**?
>>
>> It's really struct dwarf_data *, but struct dwarf_data is a type
>> declared in dwarf.c, so it's not known in other files.
>>
>> Thanks,
>> - Tom
>>
>> Here's the first part.
>>
> 
> And here's the second part.

Updated to use 'struct dwarf_data' instead of 'void'.

Thanks,
- Tom
> 
[libbacktrace] Add altlink field to struct dwarf_data

Add an altlink field to struct dwarf_data, and initialize it with the pointer
to the struct dwarf_data for the .gnu_debugaltlink.

2018-11-11  Tom de Vries  

	* dwarf.c (struct dwarf_data): Add altlink field.
	(backtrace_dwarf_add): Add and handle fileline_altlink parameter.
	* elf.c	(elf_add): Add argument to backtrace_dwarf_add call.
	(phdr_callback, backtrace_initialize): Add argument to elf_add calls.
	* internal.h (backtrace_dwarf_add): Add fileline_altlink parameter.
	* pecoff.c (coff_add): Add argument to backtrace_dwarf_add call.
	* xcoff.c (xcoff_add): Same.

---
 libbacktrace/dwarf.c| 7 ++-
 libbacktrace/elf.c  | 4 +++-
 libbacktrace/internal.h | 1 +
 libbacktrace/pecoff.c   | 1 +
 libbacktrace/xcoff.c| 1 +
 5 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c
index 190c6fc131f..83cdb52ef26 100644
--- a/libbacktrace/dwarf.c
+++ b/libbacktrace/dwarf.c
@@ -343,6 +343,8 @@ struct dwarf_data
 {
   /* The data for the next file we know about.  */
   struct dwarf_data *next;
+  /* The data for .gnu_debugaltlink.  */
+  struct dwarf_data *altlink;
   /* The base address for this file.  */
   uintptr_t base_address;
   /* A sorted list of address ranges.  */
@@ -3000,6 +3002,7 @@ build_dwarf_data (struct backtrace_state *state,
 		  const unsigned char *dwarf_str,
 		  size_t dwarf_str_size,
 		  int is_bigendian,
+		  struct dwarf_data *altlink,
 		  backtrace_error_callback error_callback,
 		  void *data)
 {
@@ -3028,6 +3031,7 @@ build_dwarf_data (struct backtrace_state *state,
 return NULL;
 
   fdata->next = NULL;
+  fdata->altlink = altlink;
   fdata->base_address = base_address;
   fdata->addrs = addrs;
   fdata->addrs_count = addrs_count;
@@ -3063,6 +3067,7 @@ backtrace_dwarf_add (struct backtrace_state *state,
 		 const unsigned char *dwarf_str,
 		 size_t dwarf_str_size,
 		 int is_bigendian,
+		 struct dwarf_data *fileline_altlink,
 		 backtrace_error_callback error_callback,
 		 void *data, 

Re: [PATCH 2/9] [libbacktrace] Add altlink field to struct dwarf_data

[Tom de Vries]

On 16-01-19 18:17, Ian Lance Taylor wrote:
> On Wed, Jan 16, 2019 at 8:33 AM Tom de Vries  wrote:
>>
>>> Why is it void**?
>>
>> It's really struct dwarf_data *, but struct dwarf_data is a type
>> declared in dwarf.c, so it's not known in other files.
> 
> It woud be OK to add "struct dwarf_data;" to internal.h and refer to
> the struct, without dereferencing it, in elf.c.

Updated accordingly.

Thanks,
- Tom
[libbacktrace] Return struct dwarf_data pointer from elf_add

Allow the caller of elf_add access to the struct dwarf_data pointer
corresponding to the added elf.

2018-11-11  Tom de Vries  

	* internal.h (backtrace_dwarf_add): Add fileline_entry parameter.
	* dwarf.c (backtrace_dwarf_add): Add and handle fileline_entry parameter.
	* elf.c	(elf_add): Add and handle fileline_entry parameter.  Add
	argument to backtrace_dwarf_add call.
	(phdr_callback, backtrace_initialize): Add argument to elf_add calls.
	* pecoff.c (coff_add): Add argument to backtrace_dwarf_add call.
	* xcoff.c (xcoff_add): Same.

---
 libbacktrace/dwarf.c|  6 +-
 libbacktrace/elf.c  | 23 ++-
 libbacktrace/internal.h |  5 -
 libbacktrace/pecoff.c   |  3 ++-
 libbacktrace/xcoff.c|  3 ++-
 5 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c
index dca5d909d9f..190c6fc131f 100644
--- a/libbacktrace/dwarf.c
+++ b/libbacktrace/dwarf.c
@@ -3064,7 +3064,8 @@ backtrace_dwarf_add (struct backtrace_state *state,
 		 size_t dwarf_str_size,
 		 int is_bigendian,
 		 backtrace_error_callback error_callback,
-		 void *data, fileline *fileline_fn)
+		 void *data, fileline *fileline_fn,
+		 struct dwarf_data **fileline_entry)
 {
   struct dwarf_data *fdata;
 
@@ -3076,6 +3077,9 @@ backtrace_dwarf_add (struct backtrace_state *state,
   if (fdata == NULL)
 return 0;
 
+  if (fileline_entry != NULL)
+*fileline_entry = fdata;
+
   if (!state->threaded)
 {
   struct dwarf_data **pp;
diff --git a/libbacktrace/elf.c b/libbacktrace/elf.c
index e363e470525..2841c06cdb2 100644
--- a/libbacktrace/elf.c
+++ b/libbacktrace/elf.c
@@ -2638,8 +2638,8 @@ static int
 elf_add (struct backtrace_state *state, const char *filename, int descriptor,
 	 uintptr_t base_address, backtrace_error_callback error_callback,
 	 void *data, fileline *fileline_fn, int *found_sym, int *found_dwarf,
-	 int exe, int debuginfo, const char *with_buildid_data,
-	 uint32_t with_buildid_size)
+	 struct dwarf_data **fileline_entry, int exe, int debuginfo,
+	 const char *with_buildid_data, uint32_t with_buildid_size)
 {
   struct backtrace_view ehdr_view;
   b_elf_ehdr ehdr;
@@ -3042,7 +3042,8 @@ elf_add (struct backtrace_state *state, const char *filename, int descriptor,
 	backtrace_release_view (state, _view, error_callback,
 data);
 	  ret = elf_add (state, NULL, d, base_address, error_callback, data,
-			 fileline_fn, found_sym, found_dwarf, 0, 1, NULL, 0);
+			 fileline_fn, found_sym, found_dwarf, NULL, 0, 1, NULL,
+			 0);
 	  if (ret < 0)
 	backtrace_close (d, error_callback, data);
 	  else
@@ -3080,7 +3081,8 @@ elf_add (struct backtrace_state *state, const char *filename, int descriptor,
 	backtrace_release_view (state, _view, error_callback,
 data);
 	  ret = elf_add (state, NULL, d, base_address, error_callback, data,
-			 fileline_fn, found_sym, found_dwarf, 0, 1, NULL, 0);
+			 fileline_fn, found_sym, found_dwarf, NULL, 0, 1, NULL,
+			 0);
 	  if (ret < 0)
 	backtrace_close (d, error_callback, data);
 	  else
@@ -3106,8 +3108,9 @@ elf_add (struct backtrace_state *state, const char *filename, int descriptor,
 	  int ret;
 
 	  ret = elf_add (state, filename, d, base_address, error_callback, data,
-			 fileline_fn, found_sym, found_dwarf, 0, 1,
-			 debugaltlink_buildid_data, debugaltlink_buildid_size);
+			 fileline_fn, found_sym, found_dwarf, NULL,
+			 0, 1, debugaltlink_buildid_data,
+			 debugaltlink_buildid_size);
 	  backtrace_release_view (state, _view, error_callback,
   data);
 	  debugaltlink_view_valid = 0;
@@ -3269,7 +3272,8 @@ elf_add (struct backtrace_state *state, const char *filename, int descriptor,
 			sections[DEBUG_STR].data,
 			sections[DEBUG_STR].size,
 			ehdr.e_ident[EI_DATA] == ELFDATA2MSB,
-			error_callback, data, fileline_fn))
+			error_callback, data, fileline_fn,
+			fileline_entry))
 goto fail;
 
   *found_dwarf = 1;
@@ -3359,7 +3363,7 @@ phdr_callback (struct dl_phdr_info *info, size_t size ATTRIBUTE_UNUSED,
 
   if (elf_add (pd->state, filename, descriptor, info->dlpi_addr,
 	   pd->error_callback, pd->data, _fileline_fn, pd->found_sym,
-	   _dwarf, 0, 0, NULL, 0))
+	   _dwarf, NULL, 0, 0, NULL, 0))
 {
   if (found_dwarf)
 	{
@@ -3387,7 +3391,8 @@ backtrace_initialize (struct backtrace_state *state, const char *filename,
   struct phdr_data pd;
 
   ret = elf_add (state, filename, descriptor, 0, error_callback, data,
-		 

Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI

[Steve Ellcey]

On Wed, 2019-01-16 at 08:50 +, Richard Sandiford wrote:
> 
> I suggest for now we add:
> 
> /* { dg-excess-errors "partial simd clone support" { target { aarch64*-*-* } 
> } }  */

OK, that works.

> 
> ...this doesn't handle explicit simdlen correctly.  The vecsize_int and
> vecsize_float need to be set from the user's simdlen, with num only
> making a difference for the default simdlen.  And there should only
> be 1 size for an explicit simdlen.
> 
> Maybe this would be more obvious if we have something like:

OK, that works better, I think returning 2 instead of 1 is what was
causing the ICE.  I used your code to set the return value and the
vecsize and everything works now.

Here is a new version of the patch, it bootstraps on aarch64 and x86
and the GCC testsuite ran with no regressions on both platforms as
well.

Steve Ellcey
sell...@marvell.com


2018-01-16  Steve Ellcey  

* config/aarch64/aarch64.c (cgraph.h): New include.
(intl.h): New include.
(supported_simd_type): New function.
(currently_supported_simd_type): Ditto.
(aarch64_simd_clone_compute_vecsize_and_simdlen): Ditto.
(aarch64_simd_clone_adjust): Ditto.
(aarch64_simd_clone_usable): Ditto.
(TARGET_SIMD_CLONE_COMPUTE_VECSIZE_AND_SIMDLEN): New macro.
(TARGET_SIMD_CLONE_ADJUST): Ditto.
(TARGET_SIMD_CLONE_USABLE): Ditto.
* config/i386/i386.c (ix86_simd_clone_adjust): Add definition check.
* omp-simd-clone.c (expand_simd_clones): Add targetm.simd_clone.adjust
call.

2018-01-16  Steve Ellcey  

* c-c++-common/gomp/pr60823-1.c: Add aarch64 specific
warning checks and assembler scans.
* c-c++-common/gomp/pr60823-3.c: Ditto.
* c-c++-common/gomp/pr63328.c: Ditto.
* g++.dg/gomp/declare-simd-1.C: Ditto.
* g++.dg/gomp/declare-simd-3.C: Ditto.
* g++.dg/gomp/declare-simd-4.C: Ditto.
* g++.dg/gomp/declare-simd-7.C: Ditto.
* g++.dg/gomp/pr88182.C: Ditto.
* g++.dg/vect/simd-clone-7.cc: Ditto.
* gcc.dg/gomp/declare-simd-1.c: Ditto.
* gcc.dg/gomp/declare-simd-3.c: Ditto.
* gcc.dg/gomp/pr59669-2.c: Ditto.
* gcc.dg/gomp/pr87895-1.c: Ditto.
* gcc.dg/gomp/pr87895-2.c: Ditto.
* gcc.dg/gomp/simd-clones-2.c: Ditto.
* gfortran.dg/gomp/declare-simd-2.f90: Ditto.
* gfortran.dg/gomp/pr79154-1.f90: Ditto.
* gfortran.dg/gomp/pr83977.f90: Ditto.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index fd60bdd..7331482 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -40,6 +40,7 @@
 #include "regs.h"
 #include "emit-rtl.h"
 #include "recog.h"
+#include "cgraph.h"
 #include "diagnostic.h"
 #include "insn-attr.h"
 #include "alias.h"
@@ -71,6 +72,7 @@
 #include "selftest.h"
 #include "selftest-rtl.h"
 #include "rtx-vector-builder.h"
+#include "intl.h"
 
 /* This file should be included last.  */
 #include "target-def.h"
@@ -18420,6 +18422,149 @@ aarch64_estimated_poly_value (poly_int64 val)
   return val.coeffs[0] + val.coeffs[1] * over_128 / 128;
 }
 
+
+/* Return true for types that could be supported as SIMD return or
+   argument types.  */
+
+static bool supported_simd_type (tree t)
+{
+  if (SCALAR_FLOAT_TYPE_P (t) || INTEGRAL_TYPE_P (t) || POINTER_TYPE_P (t))
+{
+  HOST_WIDE_INT s = tree_to_shwi (TYPE_SIZE_UNIT (t));
+  return s == 1 || s == 2 || s == 4 || s == 8;
+}
+  return false;
+}
+
+/* Return true for types that currently are supported as SIMD return
+   or argument types.  */
+
+static bool currently_supported_simd_type (tree t, tree b)
+{
+  if (COMPLEX_FLOAT_TYPE_P (t))
+return false;
+
+  if (TYPE_SIZE (t) != TYPE_SIZE (b))
+return false;
+
+  return supported_simd_type (t);
+}
+
+/* Implement TARGET_SIMD_CLONE_COMPUTE_VECSIZE_AND_SIMDLEN.  */
+
+static int
+aarch64_simd_clone_compute_vecsize_and_simdlen (struct cgraph_node *node,
+	struct cgraph_simd_clone *clonei,
+	tree base_type, int num)
+{
+  tree t, ret_type, arg_type;
+  unsigned int elt_bits, vec_bits, count;
+
+  if (!TARGET_SIMD)
+return 0;
+
+  if (clonei->simdlen
+  && (clonei->simdlen < 2
+	  || clonei->simdlen > 1024
+	  || (clonei->simdlen & (clonei->simdlen - 1)) != 0))
+{
+  warning_at (DECL_SOURCE_LOCATION (node->decl), 0,
+		  "unsupported simdlen %d", clonei->simdlen);
+  return 0;
+}
+
+  ret_type = TREE_TYPE (TREE_TYPE (node->decl));
+  if (TREE_CODE (ret_type) != VOID_TYPE
+  && !currently_supported_simd_type (ret_type, base_type))
+{
+  if (TYPE_SIZE (ret_type) != TYPE_SIZE (base_type))
+	warning_at (DECL_SOURCE_LOCATION (node->decl), 0,
+		"GCC does not currently support mixed size types "
+		"for % functions");
+  else if (supported_simd_type (ret_type))
+	warning_at (DECL_SOURCE_LOCATION (node->decl), 0,
+		"GCC does not currently support return type %qT "
+		"for % functions", 

[PR c++/86610] lambda captures in templates

[Nathan Sidwell]

This PR reports a bug where we select a non-const operator function and 
then apply it to a const object.  That's happening because the 
expression 'c[0]' is not dependent, so we figure end up resolving it. 
But the lambda capture logic doesn't capture 'c' at that point and we 
have a non-const qualified 'c'.  At instantiation time we do the capture 
and the by-value lambda results in const-qualified captures.


Jason, the orginal test in process_outer_var_ref looked a little funky 
-- why not just processing_template_decl? That would satisfy what the 
comment says it checking.  Anyway changing the test to check DECL's 
type-dependency makes the right things happen, and a bootstrap passes. 
Could you review please.


nathan
--
Nathan Sidwell
2019-01-16  Nathan Sidwell  

	PR c++/86610
	* semantics.c (process_outer_var_ref): Only skip dependent types
	in templates.

	PR c++/86610
	* g++.dg/cpp0x/pr86610.C: New.

Index: cp/semantics.c
===
--- cp/semantics.c	(revision 267983)
+++ cp/semantics.c	(working copy)
@@ -3438,10 +3438,9 @@ process_outer_var_ref (tree decl, tsubst
 }
 
   /* In a lambda within a template, wait until instantiation
- time to implicitly capture.  */
+ time to implicitly capture a dependent type.  */
   if (context == containing_function
-  && DECL_TEMPLATE_INFO (containing_function)
-  && uses_template_parms (DECL_TI_ARGS (containing_function)))
+  && dependent_type_p (TREE_TYPE (decl)))
 return decl;
 
   if (lambda_expr && VAR_P (decl)
Index: testsuite/g++.dg/cpp0x/pr86610.C
===
--- testsuite/g++.dg/cpp0x/pr86610.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/pr86610.C	(working copy)
@@ -0,0 +1,31 @@
+// { dg-do run { target c++11 } }
+// PR c++86610 lambda capture inside template
+
+struct C
+{
+  int operator[](int)
+  { return 1; }
+
+  int operator[](int) const
+  { return 0; } // Want this one
+};
+
+int q()
+{
+  C c;
+  return [=] { return c[0]; }();
+}
+
+template 
+int f()
+{
+  C c;
+  T d;
+  return [=] { return c[0]; }() 
++ [=] { return c[0] + d[0]; }();
+}
+
+int main()
+{
+  return q () + f();
+}

Re: [PATCH] detect references to statics in inline function signatures (PR 88718)

[Martin Sebor]

On 1/11/19 2:27 PM, Joseph Myers wrote:

On Fri, 11 Jan 2019, Martin Sebor wrote:


gcc/testsuite/ChangeLog:

PR c/88718
* gcc.dg/inline-40.c: New test.
* gcc.dg/inline-41.c: New test.


We already have tests inline-40.c and inline-41.c; these need to be
renumbered accordingly.


Done.




+  if (add)
+   {
+ if (csi->function == func
+ || csi->function)
+   continue;


Since func is non-NULL, this is equivalent to "if (csi->function)".


Yes.  Done.



Don't you want to break, not continue, in the case where csi->function (as
all the tentative entries come first)?  As-is, this looks like it would
have quadratic cost if a file has many inline functions each referencing
some file-scope static.  (The "Avoid adding another tentative record for
this DECL if one already exists." also looks quadratic, but having very
many functions in a file seems more likely than very many references to
statics in a single declaration that is not a definition.)


Yes, good catch!




@@ -5165,6 +5259,11 @@ finish_decl (tree decl, location_t init_loc, tree
set_user_assembler_name (decl, asmspec);
}
  
+  /* Remove any tentative record of a non-static inline function

+referencing a static decl made a DECL that is not a definition.  */
+  if (TREE_CODE (decl) == FUNCTION_DECL)
+   update_tentative_inline_static (decl);
+   


You also need to do the update when the declaration wasn't a function
declaration at all.  The present patch produces spurious warnings for:

static int i;
int a[sizeof i];
inline void f (void) {}

because the reference to i in the declaration of a gets treated as if it
were in the definition of f.


Done.



Also, you need to distinguish the case of updating for a function
definition, and updating for a declaration that's not a definition, for a
function that was previously defined.  E.g.

inline void f (int a[sizeof (int)]) {}
static int i;
inline void f (int a[sizeof i]);

must not diagnose a reference to i in f (you have such tests with the
declaration before the definition, but none that I can see with the
declaration after the definition).  I think this means the caller of
update_tentative_inline_static should specify whether to add or remove,
rather than update_tentative_inline_static trying to deduce it from
properties of a DECL (that don't say whether the particular declaration in
question is a definition or not, just whether a definition has been seen).


Also true.  Done.

Attached is a revision with these fixes.

Thanks for the careful review!

Martin
PR c/88718 - Strange inconsistency between old style and new style definitions of inline functions.

gcc/c/ChangeLog:

	PR c/88718
	* c-decl.c (update_tentative_inline_static): New function.
	(check_inline_statics): Handle tentative records for inline
	declarations without definitions.
	Print static declaration location.
	(finish_decl): Add tentative records of references to statics.
	(finish_function): Same.
	* c-typeck.c (build_external_ref): Handle all references to statics.

gcc/testsuite/ChangeLog:

	PR c/88718
	* gcc.dg/inline-42.c: New test.
	* gcc.dg/inline-43.c: New test.

diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 57049f80073..ddc09abb669 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -826,9 +826,11 @@ c_finish_incomplete_decl (tree decl)
 }
 }
 
-/* Record that inline function FUNC contains a reference (location
-   LOC) to static DECL (file-scope or function-local according to
-   TYPE).  */
+/* Record that inline function FUNC either does contain or may contain
+   a reference (location LOC) to static DECL (file-scope or function-local
+   according to TYPE).  For a null FUNC, a tentative record is created that
+   reflects a reference in the function signature and that is either updated
+   or removed when the function declaration is complete.  */
 
 void
 record_inline_static (location_t loc, tree func, tree decl,
@@ -843,6 +845,51 @@ record_inline_static (location_t loc, tree func, tree decl,
   c_inline_statics = csi;
 }
 
+/* Update tentative records of references to static declarations in
+   an inline declaration function DECL, or remove them if DECL isn't
+   a function declared inline or when ADD is false.  A tentative
+   record is one whose FUNCTION is null.  */
+
+static void
+update_tentative_inline_static (tree decl, bool add)
+{
+  gcc_assert (decl);
+
+  if (!c_inline_statics)
+return;
+
+  /* True to associate DECL with the tentative records, false to remove
+ them.  */
+  add = (add
+	 && TREE_CODE (decl) == FUNCTION_DECL
+	 && DECL_DECLARED_INLINE_P (decl)
+	 && DECL_EXTERNAL (decl)
+	 && DECL_INITIAL (decl));
+
+  /* Iterate over tentative records (all those at the head of the list
+ with a null FUNCTION) and either associate them with DECL when ADD
+ is set or remove them from it otherwise.  */
+  for (c_inline_static *csi = c_inline_statics, *last = csi;
+   csi; csi = 

[PATCH, PR d/87824] Committed fix for failing EH execution test on i386.

[Iain Buclaw]

Hi,

This patches fixes one of the problems noted in PR d/87824, where
exception chaining does not work as per expected semantics if
-forder-blocks-and-partition is enabled on i386.

Having it default off if not explicitly given seems the most
reasonable thing to do, as there's no alternative way to determine if
two in-flight exceptions should be chained at run-time if this
optimization is turned on.

Bootstrapped and tested on x86_64-linux-gnu /
RUNTESTFLAGS="--target_board=unix/-m32"

Committed to trunk as r267985.

-- 
Iain
---
gcc/d/ChangeLog:

2019-01-16  Iain Buclaw  

PR d/87824
* d-lang.cc (d_post_options): Disable implicit
-forder-blocks-and-partition.
---
diff --git a/gcc/d/d-lang.cc b/gcc/d/d-lang.cc
index 109bf4bf139..b53e56e65a2 100644
--- a/gcc/d/d-lang.cc
+++ b/gcc/d/d-lang.cc
@@ -725,6 +725,12 @@ d_post_options (const char ** fn)
 	global.params.useSwitchError = false;
 }
 
+  /* Turn off partitioning unless it was explicitly requested, as it doesn't
+ work with D exception chaining, where EH handler uses LSDA to determine
+ whether two thrown exception are in the same context.  */
+  if (!global_options_set.x_flag_reorder_blocks_and_partition)
+global_options.x_flag_reorder_blocks_and_partition = 0;
+
   /* Error about use of deprecated features.  */
   if (global.params.useDeprecated == DIAGNOSTICinform
   && global.params.warnings == DIAGNOSTICerror)

Re: Fortran vector math header

[Joseph Myers]

On Wed, 16 Jan 2019, Jakub Jelinek wrote:

> Perhaps easier would be to add optional if clause to the !GCC$ builtin
> with constant expression argument which if present and evaluates to .false.
> would tell us to ignore the attribute.  Or, add !GCC$ if/else/end if which
> would act like preprocessing conditionals or something similar.
> Not really sure one can query in Fortran what the multilib is some way (say
> look at size of a pointer etc.).

If something like that is done, I'd suggest doing it in a form which 
allows each multilib's information about glibc functions to go in a 
separate generated header (so having !GCC$ include or similar to include a 
per-multilib file, under appropriate conditionals).  Otherwise you need to 
bring back logic in glibc to make a compiler building glibc for one 
multilib use appropriate -D and -U options to get its C headers to define 
things appropriately for another multilib, so that the all-multilib 
Fortran header can be generated in a single glibc build.  (Like the old 
logic for generating bits/syscall.h that was removed in commits 
2dba5ce7b8115d6a2789bf279892263621088e74 and 
ee17d4e99af9e49378217209d3708053ef148032.)

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH][AArch64] Initial -mcpu=ares tuning

[Andrew Pinski]

On Wed, Jan 16, 2019 at 10:28 AM James Greenhalgh
 wrote:
>
> On Tue, Jan 15, 2019 at 09:29:46AM -0600, Kyrill Tkachov wrote:
> > Hi all,
> >
> > This patch adds a tuning struct for the Arm Ares CPU and uses it for 
> > -m{cpu,tune}=ares.
> > The tunings are an initial attempt and may be improved upon in the future, 
> > but they serve
> > as a decent starting point for GCC 9.
> >
> > With this I see a 1.3% improvement on SPEC2006 int and 0.3% on SPEC2006 fp 
> > with -mcpu=ares.
> > On SPEC2017 I see a 0.6% improvement in intrate and changes in the noise 
> > for fprate.
> >
> > Bootstrapped and tested on aarch64-none-linux-gnu.
> >
> > Ok for trunk?
>
> This only changes non-default tuning.
>
> OK.
>
> Are we nearly done with these types of changes in AArch64 for GCC 9? I'd
> like to see us start acting like it is stage 4 soon!

I am in the process of getting OcteonTX2 patches in shape to submit.
I hope to have them in a good shape by end of next week.

Thanks,
Andrew Pinski

>
> James
>
> > 2019-01-15  Kyrylo Tkachov  
> >
> >  * config/aarch64/aarch64.c (ares_tunings): Define.
> >  * config/aarch64/aarch64-cores.def (ares): Use the above.

Re: [PATCH 8/9] [libbacktrace] Add btest_dwz test-case

[Ian Lance Taylor]

On Wed, Jan 16, 2019 at 8:39 AM Tom de Vries  wrote:
>
> On 16-01-19 02:19, Ian Lance Taylor wrote:
> > On Tue, Dec 11, 2018 at 2:16 AM Tom de Vries  wrote:
> >>
> >> 2018-11-11  Tom de Vries  
> >>
> >> * configure.ac (DWZ): Set with AC_CHECK_PROG.
> >> (HAVE_DWZ): Set with AM_CONDITIONAL.
> >> * configure: Regenerate.
> >> * Makefile.am (TESTS): Add btest_dwz.
> >> * Makefile.in: Regenerate.
> >
> >> diff --git a/libbacktrace/Makefile.am b/libbacktrace/Makefile.am
> >> index 1a3680bc98c..497cc2f5c97 100644
> >> --- a/libbacktrace/Makefile.am
> >> +++ b/libbacktrace/Makefile.am
> >> @@ -157,6 +157,18 @@ btest_alloc_LDADD = libbacktrace_alloc.la
> >>
> >>  check_PROGRAMS += btest_alloc
> >>
> >> +if HAVE_DWZ
> >> +
> >> +%_dwz: %
> >> +   rm -f $@_common.debug
> >> +   cp $< $@
> >> +   cp $< $@_2
> >> +   $(DWZ) -m $@_common.debug $@ $@_2
> >
> > This doesn't look right.  A Makefile recipe must always create the
> > target as the very last command.  Otherwise, if the recipe is
> > interrupted for any reason, such as, in this case, a failure to run
> > dwz, then when you run make again it will think that the recipe has
> > already been run.
>
> Fixed.

> +%_dwz: %
> + rm -f $@_common.debug

Add $@ to the initial rm -f, so that you remove the target.

> + cp $@_1 $@

Make this a mv.  You don't need $@_1 after this.  Also, rm $@_2 here
or in mostlyclean or something.

This is OK with those changes.

Thanks.

Ian

Re: [PATCH][AArch64] Initial -mcpu=ares tuning

[James Greenhalgh]

On Tue, Jan 15, 2019 at 09:29:46AM -0600, Kyrill Tkachov wrote:
> Hi all,
> 
> This patch adds a tuning struct for the Arm Ares CPU and uses it for 
> -m{cpu,tune}=ares.
> The tunings are an initial attempt and may be improved upon in the future, 
> but they serve
> as a decent starting point for GCC 9.
> 
> With this I see a 1.3% improvement on SPEC2006 int and 0.3% on SPEC2006 fp 
> with -mcpu=ares.
> On SPEC2017 I see a 0.6% improvement in intrate and changes in the noise for 
> fprate.
> 
> Bootstrapped and tested on aarch64-none-linux-gnu.
> 
> Ok for trunk?

This only changes non-default tuning.

OK.

Are we nearly done with these types of changes in AArch64 for GCC 9? I'd
like to see us start acting like it is stage 4 soon!

James

> 2019-01-15  Kyrylo Tkachov  
> 
>  * config/aarch64/aarch64.c (ares_tunings): Define.
>  * config/aarch64/aarch64-cores.def (ares): Use the above.

Re: [PATCH 6/9] [libbacktrace] Factor out read_referenced_name_1

[Ian Lance Taylor]

On Wed, Jan 16, 2019 at 8:37 AM Tom de Vries  wrote:
>
> On 16-01-19 02:15, Ian Lance Taylor wrote:
> > On Tue, Dec 11, 2018 at 2:15 AM Tom de Vries  wrote:
> >>
> >> Factor out the common handling of DW_AT_abstract_origin and
> >> DW_AT_specification from read_function_entry and read_referenced_name.
> >>
> >> 2018-12-10  Tom de Vries  
> >>
> >> * dwarf.c (read_referenced_name_1): New function.  Factor out of 
> >> ...
> >> (read_referenced_name): ... here, and ...
> >> (read_function_entry): ... here.
> >
> >> +static const char *read_referenced_name (struct dwarf_data *, struct unit 
> >> *,
> >> +uint64_t, 
> >> backtrace_error_callback,
> >> +void *);
> >> +
> >
> > We don't need this declaration.  Only add a static declaration if
> > there is a forward reference.
> >
> >
>
> We need this static declaration because there's a forward reference to
> read_referenced_name in read_referenced_name_1.

Whoops, sorry.

This patch is OK.

Thanks.

Ian

Re: [PATCH 3/9] [libbacktrace] Handle alt FORMS without .gnu_debugaltlink

[Ian Lance Taylor]

On Wed, Jan 16, 2019 at 8:35 AM Tom de Vries  wrote:
>
> On 16-01-19 02:06, Ian Lance Taylor wrote:
> > On Tue, Dec 11, 2018 at 2:14 AM Tom de Vries  wrote:
> >>
> >> Handle DW_FORM_GNU_strp_alt and DW_FORM_GNU_ref_alt references robustly in
> >> presence of missing .gnu_debugaltlink file.
> >>
> >> 2018-11-11  Tom de Vries  
> >>
> >> * dwarf.c (enum attr_val_encoding): Add ATTR_VAL_NONE.
> >> (read_attribute): Add altlink parameter.  Handle missing altlink 
> >> for
> >> DW_FORM_GNU_strp_alt and DW_FORM_GNU_ref_alt.
> >> (find_address_ranges, build_address_map, build_dwarf_data): Add and
> >> handle altlink parameter.
> >> (read_referenced_name, read_function_entry): Add argument to
> >> read_attribute call.
> >
> >
> >
> >>  read_attribute (enum dwarf_form form, struct dwarf_buf *buf,
> >> int is_dwarf64, int version, int addrsize,
> >> const unsigned char *dwarf_str, size_t dwarf_str_size,
> >> -   struct attr_val *val)
> >> +   struct attr_val *val, struct dwarf_data *altlink)
> >>  {
> >
> > altlink is not a result parameter, so it should be before val.
> >
> >
>
> Done.
>
> >> @@ -1277,7 +1289,8 @@ find_address_ranges (struct backtrace_state *state, 
> >> uintptr_t base_address,
> >>  size_t dwarf_ranges_size,
> >>  int is_bigendian, backtrace_error_callback 
> >> error_callback,
> >>  void *data, struct unit *u,
> >> -struct unit_addrs_vector *addrs)
> >> +struct unit_addrs_vector *addrs,
> >> +struct dwarf_data *altlink)
> >
> > Same.  altlink should be before the error_callback parameter.
> >
> >
>
> Done.
>
> >> @@ -1431,7 +1444,8 @@ build_address_map (struct backtrace_state *state, 
> >> uintptr_t base_address,
> >>const unsigned char *dwarf_ranges, size_t 
> >> dwarf_ranges_size,
> >>const unsigned char *dwarf_str, size_t dwarf_str_size,
> >>int is_bigendian, backtrace_error_callback 
> >> error_callback,
> >> -  void *data, struct unit_addrs_vector *addrs)
> >> +  void *data, struct unit_addrs_vector *addrs,
> >> +  struct dwarf_data *altlink)
> >
> > Same.
> >
>
> Done.

This patch is OK.

Thanks.

Ian

Re: [PATCH][GCC][AArch64] Rename stack-clash CFA register to avoid clash.

[James Greenhalgh]

On Wed, Jan 16, 2019 at 11:03:41AM -0600, Tamar Christina wrote:
> Hi All,
> 
> We had multiple patches in flight that required used of scratch registers in
> frame layout code.  As it happens two of these features picked the same 
> register
> and landed at around the same time.  As such there is a clash when both are 
> used
> at the same time.   This patch changes the temporary r15 to r11 for stack 
> clash
> and documents the "reserved" registers in the frame layout comment.
> 
> Cross compiled and regtested on aarch64-none-elf with SVE on by default and no
> issues.
> Bootstrapped on aarch64-none-linux-gnu and no issues.
> 
> Ok for trunk?

My comments are all on your new comment detailing the register allocations,
which I fully support.

This patch is OK with those changes.

> gcc/ChangeLog:
> 
> 2019-01-16  Tamar Christina  
> 
>   PR target/88851
>   * config/aarch64/aarch64.md (STACK_CLASH_SVE_CFA_REGNUM): New.
>   * config/aarch64/aarch64.c (aarch64_allocate_and_probe_stack_space): Use
>   it and document registers.
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-01-16  Tamar Christina  
> 
>   PR target/88851
>   * gcc.target/aarch64/stack-check-cfa-3.c: Update test.
> 
> -- 

> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 
> fd60bddb1e1cbcb3dd46c319ccd182c7b9d1cd41..6a5f4956247b89932f955abbe96776a1b1ffb9cb
>  100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -5317,11 +5317,11 @@ aarch64_allocate_and_probe_stack_space (rtx temp1, 
> rtx temp2,
>   {
> /* This is done to provide unwinding information for the stack
>adjustments we're about to do, however to prevent the optimizers
> -  from removing the R15 move and leaving the CFA note (which would be
> +  from removing the R11 move and leaving the CFA note (which would be
>very wrong) we tie the old and new stack pointer together.
>The tie will expand to nothing but the optimizers will not touch
>the instruction.  */
> -   rtx stack_ptr_copy = gen_rtx_REG (Pmode, R15_REGNUM);
> +   rtx stack_ptr_copy = gen_rtx_REG (Pmode, STACK_CLASH_SVE_CFA_REGNUM);
> emit_move_insn (stack_ptr_copy, stack_pointer_rtx);
> emit_insn (gen_stack_tie (stack_ptr_copy, stack_pointer_rtx));
>  
> @@ -5548,7 +5548,18 @@ aarch64_add_cfa_expression (rtx_insn *insn, unsigned 
> int reg,
> to the stack we track as implicit probes are the FP/LR stores.
>  
> For outgoing arguments we probe if the size is larger than 1KB, such that
> -   the ABI specified buffer is maintained for the next callee.  */
> +   the ABI specified buffer is maintained for the next callee.
> +
> +   Aside from LR, FP, IP1 and IP0 there are a few other registers that if 
> used
> +   would clash with other features:

How about...

 The following registers are reserved during frame layout and should not be
 used for any other purpose.

  - LR 

> +
> +   - r14 and r15: Used by mitigation code.

"Used for speculation tracking." seems more correct. 'Mitigation Code' is
too broad I think.

> +   - r16 and r17: Used by indirect tailcalls
> +   - r12 and r13: Used as temporaries for stack adjustment
> + (EP0_REGNUM/EP1_REGNUM)
> +   - r11: Used by stack clash protection when SVE is enabled.

Put them in numerical (or other logical) order?

> +
> +   These registers should be avoided in frame layout related code.  */

s/should/must/

>  
>  /* Generate the prologue instructions for entry into a function.
> Establish the stack frame by decreasing the stack pointer with a

Re: [PATCH][GCC][AArch64] Fix big-endian neon-intrinsics ICEs

[James Greenhalgh]

On Mon, Jan 14, 2019 at 08:01:47AM -0600, Tamar Christina wrote:
> Hi All,
> 
> 
> This patch fixes some ICEs when the fcmla_lane intrinsics are used on
> big endian by correcting the lane indices and removing the hardcoded byte
> offset from subreg calls and instead use subreg_lowpart_offset.

Woops.

> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> Cross compiled and regtested on aarch64_be-none-elf and no issues.
> 
> Ok for trunk?

OK.

Thanks,
James

> gcc/ChangeLog:
> 
> 2019-01-14  Tamar Christina  
> 
>   * config/aarch64/aarch64-builtins.c (aarch64_simd_expand_args): Use 
> correct
>   max nunits for endian swap.
>   (aarch64_expand_fcmla_builtin): Correct subreg code.
>   * config/aarch64/aarch64-simd.md (aarch64_fcmla_lane,
>   aarch64_fcmla_laneqv4hf, aarch64_fcmlaq_lane): Correct 
> lane
>   endianness.
> 
> -- 

Re: Fortran vector math header

[Jakub Jelinek]

On Wed, Jan 16, 2019 at 05:42:06PM +, Joseph Myers wrote:
> On Wed, 16 Jan 2019, Joseph Myers wrote:
> 
> > On Wed, 16 Jan 2019, Jakub Jelinek wrote:
> > 
> > > In normal C headers, we can #if __WORDSIZE == 32 or __SIZEOF_LONG__ == 4 
> > > or
> > > defined(__ILP64__) and similar, but in these headers we can't, as no
> > > preprocessing is happening.
> > 
> > (With such preprocessing, the mechanism glibc uses for gnu/stubs.h and 
> > gnu/lib-names.h could be used.)
> 
> And I guess this leads to the question:
> 
> Since the Fortran front end has preprocessing support, could it be made to 
> run the preprocessor on such preincluded headers?  If it could, the driver 
> code could be changed to avoid looking in multilib locations for this 
> header, and that glibc machinery could be used.  (It would be necessary to 
> resolve the FIXME in fortran/cpp.c so that architecture-specific macros 
> get predefined; __x86_64__, __LP64__ and __ILP32__ all need to be properly 
> defined or not defined for this mechanism to work for x86.)

Well, it has, but extremely inefficient one.  Basically, it preprocesses
everything into a temporary file and then parses that temporary file instead
of the original input.
Doing such preprocessing for this tiny header file that is now going to be
included in every fortran compilation would slow it down, require the driver
to tell where to create the temporary file etc.

Perhaps easier would be to add optional if clause to the !GCC$ builtin
with constant expression argument which if present and evaluates to .false.
would tell us to ignore the attribute.  Or, add !GCC$ if/else/end if which
would act like preprocessing conditionals or something similar.
Not really sure one can query in Fortran what the multilib is some way (say
look at size of a pointer etc.).

Jakub

[committed] [PATCH][GCC] Fix PR88046 testcase.

[Tamar Christina]

Hi All,

The test tries to link with -shared and compile with -fPIC
without checking to see if the target actually supports this.

This patch adds effective-target requirements for fpic and shared.

Regtested on aarch64-none-elf and x86_64-unknown-linux-gnu and no
issues.

Committed under the GCC obvious rules.

Thanks,
Tamar

gcc/testsuite/ChangeLog:

2019-01-16  Tamar Christina  

PR debug/88046
* g++.dg/lto/pr88046_0.C: Check for shared and fPIC.

-- 
diff --git a/gcc/testsuite/g++.dg/lto/pr88046_0.C b/gcc/testsuite/g++.dg/lto/pr88046_0.C
index a254dd03586b8f79529e318908216993bcd62ff8..734ce86e9b8fb562918bc13bf3db7269362e3bf8 100644
--- a/gcc/testsuite/g++.dg/lto/pr88046_0.C
+++ b/gcc/testsuite/g++.dg/lto/pr88046_0.C
@@ -1,5 +1,7 @@
 // { dg-lto-do link }
 // { dg-lto-options { { -O2 -fPIC -flto } } }
+// { dg-require-effective-target shared }
+// { dg-require-effective-target fpic }
 // { dg-extra-ld-options "-shared -g" }
 
 class a {};

Re: Fortran vector math header

[Joseph Myers]

On Wed, 16 Jan 2019, Joseph Myers wrote:

> On Wed, 16 Jan 2019, Jakub Jelinek wrote:
> 
> > In normal C headers, we can #if __WORDSIZE == 32 or __SIZEOF_LONG__ == 4 or
> > defined(__ILP64__) and similar, but in these headers we can't, as no
> > preprocessing is happening.
> 
> (With such preprocessing, the mechanism glibc uses for gnu/stubs.h and 
> gnu/lib-names.h could be used.)

And I guess this leads to the question:

Since the Fortran front end has preprocessing support, could it be made to 
run the preprocessor on such preincluded headers?  If it could, the driver 
code could be changed to avoid looking in multilib locations for this 
header, and that glibc machinery could be used.  (It would be necessary to 
resolve the FIXME in fortran/cpp.c so that architecture-specific macros 
get predefined; __x86_64__, __LP64__ and __ILP32__ all need to be properly 
defined or not defined for this mechanism to work for x86.)

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH 2/9] [libbacktrace] Add altlink field to struct dwarf_data

[Ian Lance Taylor]

On Wed, Jan 16, 2019 at 8:33 AM Tom de Vries  wrote:
>
> > Why is it void**?
>
> It's really struct dwarf_data *, but struct dwarf_data is a type
> declared in dwarf.c, so it's not known in other files.

It woud be OK to add "struct dwarf_data;" to internal.h and refer to
the struct, without dereferencing it, in elf.c.

Ian

Re: [PATCH, rs6000, testsuite] Fix PR87306

[Segher Boessenkool]

Hi!

On Wed, Jan 16, 2019 at 05:29:30PM +0800, Kewen.Lin wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87306, it's older POWER 
> hardware specific test case issue. On POWER7 and earlier, we implicitly 
> set flag -mno-allow-movmisalign which disables vectorization when to 
> vectorize the code requires that misaligned loads and stores. For POWER8
> and upper, the opposite value is implicitly set(-mallow-movmisalign),
> then the vectorization performs as expected.
> 
> Similar to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65484, the proposed 
> fix is to make dg-final checks whether vect_hw_misalign support on powerpc,
> only further verify the dump pattern when it's on to consitent to the 
> behavior.

> 2019-01-16  Kewen Lin  
> 
>   PR target/87306
>   * gcc.dg/vect/bb-slp-pow-1.c : Modify to reflect that the loop is not

No space before the colon.

>   vectorized on POWER unless hardware misaligned loads are available.
> 
> diff --git a/gcc/testsuite/gcc.dg/vect/bb-slp-pow-1.c 
> b/gcc/testsuite/gcc.dg/vect/bb-slp-pow-1.c
> index 5a05bd4..6742e12 100644
> --- a/gcc/testsuite/gcc.dg/vect/bb-slp-pow-1.c
> +++ b/gcc/testsuite/gcc.dg/vect/bb-slp-pow-1.c
> @@ -25,4 +25,8 @@ main (void)
>return 0;
>  }
> 
> -/* { dg-final { scan-tree-dump-times "basic block vectorized" 1 "slp2" } } */
> +/* On older powerpc hardware (POWER7 and earlier), the default flag
> +   -mno-allow-movmisalign prevents vectorization.  On POWER8 and later,
> +   when vect_hw_misalign is true, vectorization occurs. */

Two spaces after a full stop.

> +/* { dg-final { scan-tree-dump-times "basic block vectorized" 1 "slp2" { 
> target {{ ! powerpc*-*-* } || { powerpc*-*-* && vect_hw_misalign }} } } } */

Okay for trunk with those things fixed.  Thanks!


Segher

Re: [PATCH 1/9] [libbacktrace] Read .gnu_debugaltlink

[Ian Lance Taylor via gcc-patches]

On Wed, Jan 16, 2019 at 8:26 AM Tom de Vries  wrote:
>
> On 16-01-19 01:56, Ian Lance Taylor wrote:
> > On Tue, Dec 11, 2018 at 2:14 AM Tom de Vries  wrote:
> >>
> >> Read the elf file pointed at by the .gnu_debugaltlink section, and verify 
> >> that
> >> the build id matches.
> >>
> >> 2018-11-11  Tom de Vries  
> >>
> >> * elf.c (elf_add): Add and handle with_buildid_data and
> >> with_buildid_size parameters.  Handle .gnu_debugaltlink section.
> >> (phdr_callback, backtrace_initialize): Add arguments to elf_add 
> >> calls.
> >> ---
> >
> >
> >
> > @@ -2899,6 +2918,27 @@ elf_add (struct backtrace_state *state, const
> > char *filename, int descriptor,
> >> }
> >> }
> >>
> >> +  if (!debugaltlink_view_valid
> >> + && strcmp (name, ".gnu_debugaltlink") == 0)
> >> +   {
> >> + const char *debugaltlink_data;
> >> + size_t debugaltlink_name_len;
> >> +
> >> + if (!backtrace_get_view (state, descriptor, shdr->sh_offset,
> >> +  shdr->sh_size, error_callback, data,
> >> +  _view))
> >> +   goto fail;
> >> +
> >> + debugaltlink_view_valid = 1;
> >> + debugaltlink_data = (const char *) debugaltlink_view.data;
> >> + debugaltlink_name = debugaltlink_data;
> >> + debugaltlink_name_len = strnlen (debugaltlink_data, 
> >> shdr->sh_size);
> >> + debugaltlink_buildid_data = (debugaltlink_data
> >> +  + debugaltlink_name_len
> >> +  + 1);
> >> + debugaltlink_buildid_size = shdr->sh_size - 
> >> debugaltlink_name_len - 1;
> >> +   }
> >> +
> >
> > This doesn't look quite right.  debugaltlink_buildid_size is unsigned.
> > If there is some misunderstanding of the format it's possible for
> > strnlen to return shdr->sh_size.  If it does,
> > debugaltlink_buildid_size will be set to a very large value.
> >
>
> I see, thanks for finding that.
>
> Fixed like this:
> ...
> debugaltlink_name_len = strnlen (debugaltlink_data, shdr->sh_size);
> if (debugaltlink_name_len < shdr->sh_size)
>   {
> /* Include terminating zero.  */
> debugaltlink_name_len =+ 1;
>
> debugaltlink_buildid_data
>   = debugaltlink_data + debugaltlink_name_len;
> debugaltlink_buildid_size
>   = shdr->sh_size - debugaltlink_name_len;
>   }
> ...
>
> >> +  if (debugaltlink_name != NULL)
> >> +{
> >> +  int d;
> >> +
> >> +  d = elf_open_debugfile_by_debuglink (state, filename, 
> >> debugaltlink_name,
> >> +  0, error_callback, data);
> >> +  if (d >= 0)
> >> +   {
> >> + int ret;
> >> +
> >> + ret = elf_add (state, filename, d, base_address, error_callback, 
> >> data,
> >> +fileline_fn, found_sym, found_dwarf, 0, 1,
> >> +debugaltlink_buildid_data, 
> >> debugaltlink_buildid_size);
> >> + backtrace_release_view (state, _view, 
> >> error_callback,
> >> + data);
> >> + debugaltlink_view_valid = 0;
> >> + if (ret < 0)
> >> +   {
> >> + backtrace_close (d, error_callback, data);
> >> + return ret;
> >> +   }
> >> +   }
> >> +  else
> >> +   {
> >> + error_callback (data,
> >> + "Could not open .gnu_debugaltlink", 0);
> >> + /* Don't goto fail, but try continue without the info in the
> >> +.gnu_debugaltlink.  */
> >> +   }
> >> +}
> >
> > The strings passed to error_callback always start with a lowercase
> > letter (unless they start with something like ELF) because the
> > callback will most likely print them with some prefix.
> >
>
> Fixed.
>
> > More seriously, we don't call error_callback in any cases that
> > correspond to this.  We just carry on.
>
> Indeed, I noticed that, and filed PR88244 - "[libbacktrace] Failure to
> open .gnu_debuglink is silent".
>
> [ The scenario there is: an executable has a .gnu_debuglink, but the
> file the .gnu_debuglink is pointing to is absent, because f.i. it has
> been removed, or moved to a different location. If a user runs this
> executable and a backtrace is triggered, the information relating to the
> functions in the executable will be missing in the backtrace, but
> there's no error explaining to the user why that information is missing.
>  Note: there is a default error "no debug info in ELF executable" in
> elf_nodebug, but AFAIU this is not triggered if debug info for one of
> the shared libraries is present. ]
>
> BTW, though in the code above an error_callback is called, we don't
> error out, but do carry on afterwards (as the comment explicitly states).
>
> > Is there any reason to call
> > error_callback here?
>
> A similar scenario: an executable has a .gnu_altdebuglink, but the 

Re: [PATCH] rs6000: Add missing prototypes for vec_ld/vec_st

[Segher Boessenkool]

Hi Kewen,

On Wed, Jan 16, 2019 at 10:08:46PM +0800, Kewen.Lin wrote:
> On target rs6000, for all types, use of vec_ld/vec_st historically permits 
> the dereferenced pointer to be of a scalar type or the corresponding vector 
> type. But for vector unsigned/signed long long and double, we have an 
> omission in our table for vec_ld/vec_st. 
> 
> Ok for trunk, and eventual backport to GCC 7 and 8 aftersome burn-in time?

This looks fine, thanks!  Okay for trunk.  Also okay for 8 and 7 (do the
backports in that order please) after a week or so.


Segher


> 2019-01-16  Kewen Lin  
> 
>   * doc/extend.texi: Add four new prototypes for vec_ld and seven new
>   prototypes for vec_st.
>   * config/rs6000/rs6000-c.c (altivec_overloaded_builtins): Add entries
>   for scalar address type variants of altivec_vec_ld/altivec_vec_st,
>   mainly on signed/unsigned long long and double.
> 
> gcc/testsuite/ChangeLog
> 
> 2019-01-16  Kewen Lin  
> 
>   * gcc.target/powerpc/altivec_vld_vst_addr.c: New test.

[PATCH][GCC][AArch64] Rename stack-clash CFA register to avoid clash.

[Tamar Christina]

Hi All,

We had multiple patches in flight that required used of scratch registers in
frame layout code.  As it happens two of these features picked the same register
and landed at around the same time.  As such there is a clash when both are used
at the same time.   This patch changes the temporary r15 to r11 for stack clash
and documents the "reserved" registers in the frame layout comment.

Cross compiled and regtested on aarch64-none-elf with SVE on by default and no
issues.
Bootstrapped on aarch64-none-linux-gnu and no issues.

Ok for trunk?

Thanks,
Tamar

gcc/ChangeLog:

2019-01-16  Tamar Christina  

PR target/88851
* config/aarch64/aarch64.md (STACK_CLASH_SVE_CFA_REGNUM): New.
* config/aarch64/aarch64.c (aarch64_allocate_and_probe_stack_space): Use
it and document registers.

gcc/testsuite/ChangeLog:

2019-01-16  Tamar Christina  

PR target/88851
* gcc.target/aarch64/stack-check-cfa-3.c: Update test.

-- 
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index fd60bddb1e1cbcb3dd46c319ccd182c7b9d1cd41..6a5f4956247b89932f955abbe96776a1b1ffb9cb 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -5317,11 +5317,11 @@ aarch64_allocate_and_probe_stack_space (rtx temp1, rtx temp2,
 	{
 	  /* This is done to provide unwinding information for the stack
 	 adjustments we're about to do, however to prevent the optimizers
-	 from removing the R15 move and leaving the CFA note (which would be
+	 from removing the R11 move and leaving the CFA note (which would be
 	 very wrong) we tie the old and new stack pointer together.
 	 The tie will expand to nothing but the optimizers will not touch
 	 the instruction.  */
-	  rtx stack_ptr_copy = gen_rtx_REG (Pmode, R15_REGNUM);
+	  rtx stack_ptr_copy = gen_rtx_REG (Pmode, STACK_CLASH_SVE_CFA_REGNUM);
 	  emit_move_insn (stack_ptr_copy, stack_pointer_rtx);
 	  emit_insn (gen_stack_tie (stack_ptr_copy, stack_pointer_rtx));
 
@@ -5548,7 +5548,18 @@ aarch64_add_cfa_expression (rtx_insn *insn, unsigned int reg,
to the stack we track as implicit probes are the FP/LR stores.
 
For outgoing arguments we probe if the size is larger than 1KB, such that
-   the ABI specified buffer is maintained for the next callee.  */
+   the ABI specified buffer is maintained for the next callee.
+
+   Aside from LR, FP, IP1 and IP0 there are a few other registers that if used
+   would clash with other features:
+
+   - r14 and r15: Used by mitigation code.
+   - r16 and r17: Used by indirect tailcalls
+   - r12 and r13: Used as temporaries for stack adjustment
+ (EP0_REGNUM/EP1_REGNUM)
+   - r11: Used by stack clash protection when SVE is enabled.
+
+   These registers should be avoided in frame layout related code.  */
 
 /* Generate the prologue instructions for entry into a function.
Establish the stack frame by decreasing the stack pointer with a
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 513aec1872a7a5d29233d0bcb0bd1331d306eaaf..eb95a3c0935505e5c13c6e83a92b4e4db832ef94 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -35,15 +35,10 @@
 (R11_REGNUM		11)
 (R12_REGNUM		12)
 (R13_REGNUM		13)
-;; Scratch registers for prologue/epilogue use.
-(EP0_REGNUM		12)
-(EP1_REGNUM		13)
 (R14_REGNUM		14)
 (R15_REGNUM		15)
 (R16_REGNUM		16)
-(IP0_REGNUM		16)
 (R17_REGNUM		17)
-(IP1_REGNUM		17)
 (R18_REGNUM		18)
 (R19_REGNUM		19)
 (R20_REGNUM		20)
@@ -57,7 +52,6 @@
 (R28_REGNUM		28)
 (R29_REGNUM		29)
 (R30_REGNUM		30)
-(LR_REGNUM		30)
 (SP_REGNUM		31)
 (V0_REGNUM		32)
 (V1_REGNUM		33)
@@ -113,10 +107,20 @@
 (P13_REGNUM		81)
 (P14_REGNUM		82)
 (P15_REGNUM		83)
+;; Scratch register used by stack clash protection to calculate
+;; SVE CFA offsets during probing.
+(STACK_CLASH_SVE_CFA_REGNUM 11)
+;; Scratch registers for prologue/epilogue use.
+(EP0_REGNUM 12)
+(EP1_REGNUM 13)
 ;; A couple of call-clobbered registers that we need to reserve when
 ;; tracking speculation this is not ABI, so is subject to change.
-(SPECULATION_TRACKER_REGNUM 15)
 (SPECULATION_SCRATCH_REGNUM 14)
+(SPECULATION_TRACKER_REGNUM 15)
+;; Scratch registers used in frame layout.
+(IP0_REGNUM 16)
+(IP1_REGNUM 17)
+(LR_REGNUM  30)
   ]
 )
 
diff --git a/gcc/testsuite/gcc.target/aarch64/stack-check-cfa-3.c b/gcc/testsuite/gcc.target/aarch64/stack-check-cfa-3.c
index 41579f26ba9156f3e500f090d132ba9cf28364d3..c4b7bb601c442a981ca309d0c3e8f29341b9b466 100644
--- a/gcc/testsuite/gcc.target/aarch64/stack-check-cfa-3.c
+++ b/gcc/testsuite/gcc.target/aarch64/stack-check-cfa-3.c
@@ -8,6 +8,6 @@
need to make sure we can unwind correctly before the frame is set up.  So
check that we're emitting r15 with a copy of sp an setting the CFA 

Re: [PATCH 8/9] [libbacktrace] Add btest_dwz test-case

[Tom de Vries]

On 16-01-19 02:19, Ian Lance Taylor wrote:
> On Tue, Dec 11, 2018 at 2:16 AM Tom de Vries  wrote:
>>
>> 2018-11-11  Tom de Vries  
>>
>> * configure.ac (DWZ): Set with AC_CHECK_PROG.
>> (HAVE_DWZ): Set with AM_CONDITIONAL.
>> * configure: Regenerate.
>> * Makefile.am (TESTS): Add btest_dwz.
>> * Makefile.in: Regenerate.
> 
>> diff --git a/libbacktrace/Makefile.am b/libbacktrace/Makefile.am
>> index 1a3680bc98c..497cc2f5c97 100644
>> --- a/libbacktrace/Makefile.am
>> +++ b/libbacktrace/Makefile.am
>> @@ -157,6 +157,18 @@ btest_alloc_LDADD = libbacktrace_alloc.la
>>
>>  check_PROGRAMS += btest_alloc
>>
>> +if HAVE_DWZ
>> +
>> +%_dwz: %
>> +   rm -f $@_common.debug
>> +   cp $< $@
>> +   cp $< $@_2
>> +   $(DWZ) -m $@_common.debug $@ $@_2
> 
> This doesn't look right.  A Makefile recipe must always create the
> target as the very last command.  Otherwise, if the recipe is
> interrupted for any reason, such as, in this case, a failure to run
> dwz, then when you run make again it will think that the recipe has
> already been run.

Fixed.

Thanks,
- Tom


[libbacktrace] Add btest_dwz test-case

Add test-case to verify that libbacktrace can read debug info that was
compressed with dwz.

2018-11-11  Tom de Vries  

	* configure.ac (DWZ): Set with AC_CHECK_PROG.
	(HAVE_DWZ): Set with AM_CONDITIONAL.
	* configure: Regenerate.
	* Makefile.am (TESTS): Add btest_dwz.
	* Makefile.in: Regenerate.

---
 libbacktrace/Makefile.am  | 13 +++
 libbacktrace/Makefile.in  | 33 ---
 libbacktrace/configure| 57 +--
 libbacktrace/configure.ac |  3 +++
 4 files changed, 96 insertions(+), 10 deletions(-)

diff --git a/libbacktrace/Makefile.am b/libbacktrace/Makefile.am
index 527353aa3f0..0331e9272e8 100644
--- a/libbacktrace/Makefile.am
+++ b/libbacktrace/Makefile.am
@@ -178,6 +178,19 @@ btest_alloc_LDADD = libbacktrace_alloc.la
 
 check_PROGRAMS += btest_alloc
 
+if HAVE_DWZ
+
+%_dwz: %
+	rm -f $@_common.debug
+	cp $< $@_1
+	cp $< $@_2
+	$(DWZ) -m $@_common.debug $@_1 $@_2
+	cp $@_1 $@
+
+TESTS += btest_dwz
+
+endif HAVE_DWZ
+
 stest_SOURCES = stest.c
 stest_LDADD = libbacktrace.la
 
diff --git a/libbacktrace/Makefile.in b/libbacktrace/Makefile.in
index c595a8b4a3e..a7fa47fdb52 100644
--- a/libbacktrace/Makefile.in
+++ b/libbacktrace/Makefile.in
@@ -15,7 +15,7 @@
 @SET_MAKE@
 
 # Makefile.am -- Backtrace Makefile.
-# Copyright (C) 2012-2018 Free Software Foundation, Inc.
+# Copyright (C) 2012-2019 Free Software Foundation, Inc.
 
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
@@ -126,11 +126,12 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3)
 @NATIVE_TRUE@	allocfail btest btest_alloc stest stest_alloc \
 @NATIVE_TRUE@	ztest ztest_alloc edtest edtest_alloc
 @NATIVE_TRUE@am__append_2 = allocfail.sh
-@HAVE_ZLIB_TRUE@@NATIVE_TRUE@am__append_3 = -lz
+@HAVE_DWZ_TRUE@@NATIVE_TRUE@am__append_3 = btest_dwz
 @HAVE_ZLIB_TRUE@@NATIVE_TRUE@am__append_4 = -lz
-@HAVE_PTHREAD_TRUE@@NATIVE_TRUE@am__append_5 = ttest ttest_alloc
-@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@am__append_6 = dtest
-@HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@am__append_7 = ctestg ctesta \
+@HAVE_ZLIB_TRUE@@NATIVE_TRUE@am__append_5 = -lz
+@HAVE_PTHREAD_TRUE@@NATIVE_TRUE@am__append_6 = ttest ttest_alloc
+@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@am__append_7 = dtest
+@HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@am__append_8 = ctestg ctesta \
 @HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@	ctestg_alloc \
 @HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@	ctesta_alloc
 subdir = .
@@ -632,6 +633,7 @@ CYGPATH_W = @CYGPATH_W@
 DEFS = @DEFS@
 DSYMUTIL = @DSYMUTIL@
 DUMPBIN = @DUMPBIN@
+DWZ = @DWZ@
 ECHO_C = @ECHO_C@
 ECHO_N = @ECHO_N@
 ECHO_T = @ECHO_T@
@@ -786,7 +788,8 @@ libbacktrace_la_LIBADD = \
 	$(ALLOC_FILE)
 
 libbacktrace_la_DEPENDENCIES = $(libbacktrace_la_LIBADD)
-TESTS = $(check_PROGRAMS) $(am__append_2) $(am__append_6)
+TESTS = $(check_PROGRAMS) $(am__append_2) $(am__append_3) \
+	$(am__append_7)
 @NATIVE_TRUE@check_LTLIBRARIES = libbacktrace_alloc.la \
 @NATIVE_TRUE@	libbacktrace_noformat.la \
 @NATIVE_TRUE@	libbacktrace_instrumented_alloc.la
@@ -831,9 +834,9 @@ TESTS = $(check_PROGRAMS) $(am__append_2) $(am__append_6)
 @NATIVE_TRUE@stest_alloc_LDADD = libbacktrace_alloc.la
 @NATIVE_TRUE@ztest_SOURCES = ztest.c testlib.c
 @NATIVE_TRUE@ztest_CFLAGS = -DSRCDIR=\"$(srcdir)\"
-@NATIVE_TRUE@ztest_LDADD = libbacktrace.la $(am__append_3) \
+@NATIVE_TRUE@ztest_LDADD = libbacktrace.la $(am__append_4) \
 @NATIVE_TRUE@	$(CLOCK_GETTIME_LINK)
-@NATIVE_TRUE@ztest_alloc_LDADD = libbacktrace_alloc.la $(am__append_4) \
+@NATIVE_TRUE@ztest_alloc_LDADD = libbacktrace_alloc.la $(am__append_5) \
 @NATIVE_TRUE@	$(CLOCK_GETTIME_LINK)
 @NATIVE_TRUE@ztest_alloc_SOURCES = $(ztest_SOURCES)
 @NATIVE_TRUE@ztest_alloc_CFLAGS = $(ztest_CFLAGS)
@@ -1575,6 +1578,13 @@ 

Re: [PATCH 6/9] [libbacktrace] Factor out read_referenced_name_1

[Tom de Vries]

On 16-01-19 02:15, Ian Lance Taylor wrote:
> On Tue, Dec 11, 2018 at 2:15 AM Tom de Vries  wrote:
>>
>> Factor out the common handling of DW_AT_abstract_origin and
>> DW_AT_specification from read_function_entry and read_referenced_name.
>>
>> 2018-12-10  Tom de Vries  
>>
>> * dwarf.c (read_referenced_name_1): New function.  Factor out of ...
>> (read_referenced_name): ... here, and ...
>> (read_function_entry): ... here.
> 
>> +static const char *read_referenced_name (struct dwarf_data *, struct unit *,
>> +uint64_t, backtrace_error_callback,
>> +void *);
>> +
> 
> We don't need this declaration.  Only add a static declaration if
> there is a forward reference.
> 
> 

We need this static declaration because there's a forward reference to
read_referenced_name in read_referenced_name_1.

>> +/* Read the name of a function from a DIE referenced by ATTR with VAL.  */
>> +
>> +static const char *
>> +read_referenced_name_1 (struct dwarf_data *ddata, struct unit *u,
>> +   struct attr *attr, struct attr_val *val,
>> +   backtrace_error_callback error_callback, void *data)
>> +{
> 
> Let's not use a name ending in "_1".  That is a cop-out.  Pick a
> meaningful name.  Perhaps read_referenced_name_from_attr.

Used read_referenced_name_from_attr.

Thanks,
- Tom
[libbacktrace] Factor out read_referenced_name_from_attr

Factor out the common handling of DW_AT_abstract_origin and
DW_AT_specification from read_function_entry and read_referenced_name.

2018-12-10  Tom de Vries  

	* dwarf.c (read_referenced_name_from_attr): New function.  Factor out
	of ...
 	(read_referenced_name): ... here, and ...
	(read_function_entry): ... here.

---
 libbacktrace/dwarf.c | 89 +++-
 1 file changed, 54 insertions(+), 35 deletions(-)

diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c
index 03365e05778..2a58d938de6 100644
--- a/libbacktrace/dwarf.c
+++ b/libbacktrace/dwarf.c
@@ -2111,6 +2111,43 @@ read_line_info (struct backtrace_state *state, struct dwarf_data *ddata,
   return 0;
 }
 
+static const char *read_referenced_name (struct dwarf_data *, struct unit *,
+	 uint64_t, backtrace_error_callback,
+	 void *);
+
+/* Read the name of a function from a DIE referenced by ATTR with VAL.  */
+
+static const char *
+read_referenced_name_from_attr (struct dwarf_data *ddata, struct unit *u,
+struct attr *attr, struct attr_val *val,
+backtrace_error_callback error_callback,
+void *data)
+{
+  switch (attr->name)
+{
+case DW_AT_abstract_origin:
+case DW_AT_specification:
+  break;
+default:
+  return NULL;
+}
+
+  if (attr->form == DW_FORM_ref_addr
+  || attr->form == DW_FORM_ref_sig8)
+{
+  /* This refers to an abstract origin defined in
+	 some other compilation unit.  We can handle
+	 this case if we must, but it's harder.  */
+  return NULL;
+}
+
+  if (val->encoding == ATTR_VAL_UINT
+  || val->encoding == ATTR_VAL_REF_UNIT)
+return read_referenced_name (ddata, u, val->u.uint, error_callback, data);
+
+  return NULL;
+}
+
 /* Read the name of a function from a DIE referenced by a
DW_AT_abstract_origin or DW_AT_specification tag.  OFFSET is within
the same compilation unit.  */
@@ -2194,24 +2231,14 @@ read_referenced_name (struct dwarf_data *ddata, struct unit *u,
 	case DW_AT_specification:
 	  /* Second name preference: override DW_AT_name, don't override
 	 DW_AT_linkage_name.  */
-	  if (abbrev->attrs[i].form == DW_FORM_ref_addr
-	  || abbrev->attrs[i].form == DW_FORM_ref_sig8)
-	{
-	  /* This refers to a specification defined in some other
-		 compilation unit.  We can handle this case if we
-		 must, but it's harder.  */
-	  break;
-	}
-	  if (val.encoding == ATTR_VAL_UINT
-	  || val.encoding == ATTR_VAL_REF_UNIT)
-	{
-	  const char *name;
+	  {
+	const char *name;
 
-	  name = read_referenced_name (ddata, u, val.u.uint,
-	   error_callback, data);
-	  if (name != NULL)
-		ret = name;
-	}
+	name = read_referenced_name_from_attr (ddata, u, >attrs[i],
+		   , error_callback, data);
+	if (name != NULL)
+	  ret = name;
+	  }
 	  break;
 
 	default:
@@ -2436,24 +2463,16 @@ read_function_entry (struct backtrace_state *state, struct dwarf_data *ddata,
 		 DW_AT_linkage_name.  */
 		  if (have_linkage_name)
 		break;
-		  if (abbrev->attrs[i].form == DW_FORM_ref_addr
-		  || abbrev->attrs[i].form == DW_FORM_ref_sig8)
-		{
-		  /* This refers to an abstract origin defined in
-			 some other compilation unit.  We can handle
-			 this case if we must, but it's harder.  */
-		  break;
-		}
-		  if (val.encoding == ATTR_VAL_UINT
-		  || val.encoding == ATTR_VAL_REF_UNIT)
-		{
-		  const char *name;
-
-		  name = read_referenced_name (ddata, u, 

Re: [PATCH 3/9] [libbacktrace] Handle alt FORMS without .gnu_debugaltlink

[Tom de Vries]

On 16-01-19 02:06, Ian Lance Taylor wrote:
> On Tue, Dec 11, 2018 at 2:14 AM Tom de Vries  wrote:
>>
>> Handle DW_FORM_GNU_strp_alt and DW_FORM_GNU_ref_alt references robustly in
>> presence of missing .gnu_debugaltlink file.
>>
>> 2018-11-11  Tom de Vries  
>>
>> * dwarf.c (enum attr_val_encoding): Add ATTR_VAL_NONE.
>> (read_attribute): Add altlink parameter.  Handle missing altlink for
>> DW_FORM_GNU_strp_alt and DW_FORM_GNU_ref_alt.
>> (find_address_ranges, build_address_map, build_dwarf_data): Add and
>> handle altlink parameter.
>> (read_referenced_name, read_function_entry): Add argument to
>> read_attribute call.
> 
> 
> 
>>  read_attribute (enum dwarf_form form, struct dwarf_buf *buf,
>> int is_dwarf64, int version, int addrsize,
>> const unsigned char *dwarf_str, size_t dwarf_str_size,
>> -   struct attr_val *val)
>> +   struct attr_val *val, struct dwarf_data *altlink)
>>  {
> 
> altlink is not a result parameter, so it should be before val.
> 
> 

Done.

>> @@ -1277,7 +1289,8 @@ find_address_ranges (struct backtrace_state *state, 
>> uintptr_t base_address,
>>  size_t dwarf_ranges_size,
>>  int is_bigendian, backtrace_error_callback 
>> error_callback,
>>  void *data, struct unit *u,
>> -struct unit_addrs_vector *addrs)
>> +struct unit_addrs_vector *addrs,
>> +struct dwarf_data *altlink)
> 
> Same.  altlink should be before the error_callback parameter.
> 
> 

Done.

>> @@ -1431,7 +1444,8 @@ build_address_map (struct backtrace_state *state, 
>> uintptr_t base_address,
>>const unsigned char *dwarf_ranges, size_t 
>> dwarf_ranges_size,
>>const unsigned char *dwarf_str, size_t dwarf_str_size,
>>int is_bigendian, backtrace_error_callback error_callback,
>> -  void *data, struct unit_addrs_vector *addrs)
>> +  void *data, struct unit_addrs_vector *addrs,
>> +  struct dwarf_data *altlink)
> 
> Same.
> 

Done.

Thanks,
- Tom
[libbacktrace] Handle alt FORMS without .gnu_debugaltlink

Handle DW_FORM_GNU_strp_alt and DW_FORM_GNU_ref_alt references robustly in
presence of missing .gnu_debugaltlink file.

2018-11-11  Tom de Vries  

	* dwarf.c (enum attr_val_encoding): Add ATTR_VAL_NONE.
	(read_attribute): Add altlink parameter.  Handle missing altlink for
	DW_FORM_GNU_strp_alt and DW_FORM_GNU_ref_alt.
	(find_address_ranges, build_address_map, build_dwarf_data): Add and
	handle altlink parameter.
	(read_referenced_name, read_function_entry): Add argument to
	read_attribute call.

---
 libbacktrace/dwarf.c | 44 +---
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c
index 5469f968468..0e2d57122a2 100644
--- a/libbacktrace/dwarf.c
+++ b/libbacktrace/dwarf.c
@@ -129,6 +129,8 @@ struct abbrevs
 
 enum attr_val_encoding
 {
+  /* No attribute value.  */
+  ATTR_VAL_NONE,
   /* An address.  */
   ATTR_VAL_ADDRESS,
   /* A unsigned integer.  */
@@ -700,7 +702,7 @@ static int
 read_attribute (enum dwarf_form form, struct dwarf_buf *buf,
 		int is_dwarf64, int version, int addrsize,
 		const unsigned char *dwarf_str, size_t dwarf_str_size,
-		struct attr_val *val)
+		struct dwarf_data *altlink, struct attr_val *val)
 {
   /* Avoid warnings about val.u.FIELD may be used uninitialized if
  this function is inlined.  The warnings aren't valid but can
@@ -806,7 +808,7 @@ read_attribute (enum dwarf_form form, struct dwarf_buf *buf,
 	form = read_uleb128 (buf);
 	return read_attribute ((enum dwarf_form) form, buf, is_dwarf64,
 			   version, addrsize, dwarf_str, dwarf_str_size,
-			   val);
+			   altlink, val);
   }
 case DW_FORM_sec_offset:
   val->encoding = ATTR_VAL_REF_SECTION;
@@ -832,12 +834,22 @@ read_attribute (enum dwarf_form form, struct dwarf_buf *buf,
   val->u.uint = read_uleb128 (buf);
   return 1;
 case DW_FORM_GNU_ref_alt:
-  val->encoding = ATTR_VAL_REF_SECTION;
   val->u.uint = read_offset (buf, is_dwarf64);
+  if (altlink == NULL)
+	{
+	  val->encoding = ATTR_VAL_NONE;
+	  return 1;
+	}
+  val->encoding = ATTR_VAL_REF_SECTION;
   return 1;
 case DW_FORM_GNU_strp_alt:
-  val->encoding = ATTR_VAL_REF_SECTION;
   val->u.uint = read_offset (buf, is_dwarf64);
+  if (altlink == NULL)
+	{
+	  val->encoding = ATTR_VAL_NONE;
+	  return 1;
+	}
+  val->encoding = ATTR_VAL_REF_SECTION;
   return 1;
 default:
   dwarf_buf_error (buf, "unrecognized DWARF form");
@@ -1275,9 +1287,9 @@ find_address_ranges (struct backtrace_state *state, uintptr_t base_address,
 		 const unsigned char *dwarf_str, size_t dwarf_str_size,
 		 const unsigned char *dwarf_ranges,
 		 size_t dwarf_ranges_size,

Re: [PATCH 2/9] [libbacktrace] Add altlink field to struct dwarf_data

[Tom de Vries]

On 16-01-19 17:33, Tom de Vries wrote:
> On 16-01-19 02:02, Ian Lance Taylor wrote:
>> On Tue, Dec 11, 2018 at 2:14 AM Tom de Vries  wrote:
>>>
>>> Add an altlink field to struct dwarf_data, and initialize it with the 
>>> pointer
>>> to the struct dwarf_data for the .gnu_debugaltlink.
>>>
>>> 2018-11-11  Tom de Vries  
>>>
>>> * dwarf.c (struct dwarf_data): Add altlink field.
>>> (backtrace_dwarf_add): Add and handle fileline_entry and
>>> fileline_altlink parameters.
>>> * elf.c (elf_add): Add and handle fileline_entry parameter.  Add 
>>> args to
>>> backtrace_dwarf_add call.
>>> (phdr_callback, backtrace_initialize): Add arguments to elf_add 
>>> calls.
>>> * internal.h (backtrace_dwarf_add): Add fileline_entry and
>>> fileline_altlink parameters.
>>> * pecoff.c (coff_add): Add args to backtrace_dwarf_add call.
>>> * xcoff.c (xcoff_add): Same.
>>
>>
>>> @@ -2968,7 +2970,7 @@ build_dwarf_data (struct backtrace_state *state,
>>>   size_t dwarf_str_size,
>>>   int is_bigendian,
>>>   backtrace_error_callback error_callback,
>>> - void *data)
>>> + void *data, struct dwarf_data *altlink)
>>>  {
>>
>> error_callback and data should remain the last two parameters, as they
>> are for many of the functions in this file.
>>
>>
> 
> Done.
> 
>>>   @@ -3031,7 +3034,8 @@ backtrace_dwarf_add (struct backtrace_state *state,
>>>  size_t dwarf_str_size,
>>>  int is_bigendian,
>>>  backtrace_error_callback error_callback,
>>> -void *data, fileline *fileline_fn)
>>> +void *data, fileline *fileline_fn, void 
>>> **fileline_entry,
>>> +void *fileline_altlink)
>>
>> The new fileline_altlink parameter should come before error_callback,
>> as it is not error_callback/data and is not a result parameter.
>>
> 
> Done.
> 
>> What is fileline_entry for? 
> 
> There are two bits to this patch:
> - add fileline_entry parameter to elf_add.  This allows the callers of
>   elf_add access to the struct dwarf_data pointer corresponding to the
>   added elf.
> - add an altlink field to struct dwarf_data, and initialize it with the
>   pointer to the struct dwarf_data for the .gnu_debugaltlink.
> 
> I've split the patch up this way now, hoping it will make things clearer
> and/or easier to review.
> 
>> Why is it void**?
> 
> It's really struct dwarf_data *, but struct dwarf_data is a type
> declared in dwarf.c, so it's not known in other files.
> 
> Thanks,
> - Tom
> 
> Here's the first part.
> 

And here's the second part.

[libbacktrace] Add altlink field to struct dwarf_data

Add an altlink field to struct dwarf_data, and initialize it with the pointer
to the struct dwarf_data for the .gnu_debugaltlink.

2018-11-11  Tom de Vries  

	* dwarf.c (struct dwarf_data): Add altlink field.
	(backtrace_dwarf_add): Add and handle fileline_altlink parameter.
	* elf.c	(elf_add): Add argument to backtrace_dwarf_add call.
	(phdr_callback, backtrace_initialize): Add argument to elf_add calls.
	* internal.h (backtrace_dwarf_add): Add fileline_altlink parameter.
	* pecoff.c (coff_add): Add argument to backtrace_dwarf_add call.
	* xcoff.c (xcoff_add): Same.

---
 libbacktrace/dwarf.c| 7 ++-
 libbacktrace/elf.c  | 4 +++-
 libbacktrace/internal.h | 1 +
 libbacktrace/pecoff.c   | 1 +
 libbacktrace/xcoff.c| 1 +
 5 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c
index c7913f43617..5469f968468 100644
--- a/libbacktrace/dwarf.c
+++ b/libbacktrace/dwarf.c
@@ -343,6 +343,8 @@ struct dwarf_data
 {
   /* The data for the next file we know about.  */
   struct dwarf_data *next;
+  /* The data for .gnu_debugaltlink.  */
+  struct dwarf_data *altlink;
   /* The base address for this file.  */
   uintptr_t base_address;
   /* A sorted list of address ranges.  */
@@ -2981,6 +2983,7 @@ build_dwarf_data (struct backtrace_state *state,
 		  const unsigned char *dwarf_str,
 		  size_t dwarf_str_size,
 		  int is_bigendian,
+		  struct dwarf_data *altlink,
 		  backtrace_error_callback error_callback,
 		  void *data)
 {
@@ -3009,6 +3012,7 @@ build_dwarf_data (struct backtrace_state *state,
 return NULL;
 
   fdata->next = NULL;
+  fdata->altlink = altlink;
   fdata->base_address = base_address;
   fdata->addrs = addrs;
   fdata->addrs_count = addrs_count;
@@ -3044,6 +3048,7 @@ backtrace_dwarf_add (struct backtrace_state *state,
 		 const unsigned char *dwarf_str,
 		 size_t dwarf_str_size,
 		 int is_bigendian,
+		 void *fileline_altlink,
 		 backtrace_error_callback error_callback,
 		 void *data, fileline *fileline_fn,
 		 void **fileline_entry)
@@ -3054,7 +3059,7 @@ backtrace_dwarf_add (struct backtrace_state *state,
 			dwarf_line, dwarf_line_size, dwarf_abbrev,
 			

Re: [PATCH 2/9] [libbacktrace] Add altlink field to struct dwarf_data

[Tom de Vries]

On 16-01-19 02:02, Ian Lance Taylor wrote:
> On Tue, Dec 11, 2018 at 2:14 AM Tom de Vries  wrote:
>>
>> Add an altlink field to struct dwarf_data, and initialize it with the pointer
>> to the struct dwarf_data for the .gnu_debugaltlink.
>>
>> 2018-11-11  Tom de Vries  
>>
>> * dwarf.c (struct dwarf_data): Add altlink field.
>> (backtrace_dwarf_add): Add and handle fileline_entry and
>> fileline_altlink parameters.
>> * elf.c (elf_add): Add and handle fileline_entry parameter.  Add 
>> args to
>> backtrace_dwarf_add call.
>> (phdr_callback, backtrace_initialize): Add arguments to elf_add 
>> calls.
>> * internal.h (backtrace_dwarf_add): Add fileline_entry and
>> fileline_altlink parameters.
>> * pecoff.c (coff_add): Add args to backtrace_dwarf_add call.
>> * xcoff.c (xcoff_add): Same.
> 
> 
>> @@ -2968,7 +2970,7 @@ build_dwarf_data (struct backtrace_state *state,
>>   size_t dwarf_str_size,
>>   int is_bigendian,
>>   backtrace_error_callback error_callback,
>> - void *data)
>> + void *data, struct dwarf_data *altlink)
>>  {
> 
> error_callback and data should remain the last two parameters, as they
> are for many of the functions in this file.
> 
> 

Done.

>>   @@ -3031,7 +3034,8 @@ backtrace_dwarf_add (struct backtrace_state *state,
>>  size_t dwarf_str_size,
>>  int is_bigendian,
>>  backtrace_error_callback error_callback,
>> -void *data, fileline *fileline_fn)
>> +void *data, fileline *fileline_fn, void 
>> **fileline_entry,
>> +void *fileline_altlink)
> 
> The new fileline_altlink parameter should come before error_callback,
> as it is not error_callback/data and is not a result parameter.
> 

Done.

> What is fileline_entry for? 

There are two bits to this patch:
- add fileline_entry parameter to elf_add.  This allows the callers of
  elf_add access to the struct dwarf_data pointer corresponding to the
  added elf.
- add an altlink field to struct dwarf_data, and initialize it with the
  pointer to the struct dwarf_data for the .gnu_debugaltlink.

I've split the patch up this way now, hoping it will make things clearer
and/or easier to review.

> Why is it void**?

It's really struct dwarf_data *, but struct dwarf_data is a type
declared in dwarf.c, so it's not known in other files.

Thanks,
- Tom

Here's the first part.
[libbacktrace] Return struct dwarf_data pointer from elf_add

Allow the caller of elf_add access to the struct dwarf_data pointer
corresponding to the added elf.

2018-11-11  Tom de Vries  

	* internal.h (backtrace_dwarf_add): Add fileline_entry parameter.
	* dwarf.c (backtrace_dwarf_add): Add and handle fileline_entry parameter.
	* elf.c	(elf_add): Add and handle fileline_entry parameter.  Add
	argument to backtrace_dwarf_add call.
	(phdr_callback, backtrace_initialize): Add argument to elf_add calls.
	* pecoff.c (coff_add): Add argument to backtrace_dwarf_add call.
	* xcoff.c (xcoff_add): Same.

---
 libbacktrace/dwarf.c|  6 +-
 libbacktrace/elf.c  | 23 ++-
 libbacktrace/internal.h |  3 ++-
 libbacktrace/pecoff.c   |  3 ++-
 libbacktrace/xcoff.c|  3 ++-
 5 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c
index af864d68b00..c7913f43617 100644
--- a/libbacktrace/dwarf.c
+++ b/libbacktrace/dwarf.c
@@ -3045,7 +3045,8 @@ backtrace_dwarf_add (struct backtrace_state *state,
 		 size_t dwarf_str_size,
 		 int is_bigendian,
 		 backtrace_error_callback error_callback,
-		 void *data, fileline *fileline_fn)
+		 void *data, fileline *fileline_fn,
+		 void **fileline_entry)
 {
   struct dwarf_data *fdata;
 
@@ -3057,6 +3058,9 @@ backtrace_dwarf_add (struct backtrace_state *state,
   if (fdata == NULL)
 return 0;
 
+  if (fileline_entry != NULL)
+*fileline_entry = fdata;
+
   if (!state->threaded)
 {
   struct dwarf_data **pp;
diff --git a/libbacktrace/elf.c b/libbacktrace/elf.c
index e363e470525..6f99461db84 100644
--- a/libbacktrace/elf.c
+++ b/libbacktrace/elf.c
@@ -2638,8 +2638,8 @@ static int
 elf_add (struct backtrace_state *state, const char *filename, int descriptor,
 	 uintptr_t base_address, backtrace_error_callback error_callback,
 	 void *data, fileline *fileline_fn, int *found_sym, int *found_dwarf,
-	 int exe, int debuginfo, const char *with_buildid_data,
-	 uint32_t with_buildid_size)
+	 void **fileline_entry, int exe, int debuginfo,
+	 const char *with_buildid_data, uint32_t with_buildid_size)
 {
   struct backtrace_view ehdr_view;
   b_elf_ehdr ehdr;
@@ -3042,7 +3042,8 @@ elf_add (struct backtrace_state *state, const char *filename, int descriptor,
 	backtrace_release_view (state, _view, error_callback,
 data);
 	  ret = elf_add (state, NULL, d, 

Re: [PATCH][rs6000] avoid using unaligned vsx or lxvd2x/stxvd2x for memcpy/memmove inline expansion

[Segher Boessenkool]

On Mon, Jan 14, 2019 at 12:49:33PM -0600, Aaron Sawdey wrote:
> The patch for this was committed to trunk as 267562 (see below). Is this also 
> ok for backport to 8?

Yes please.  Thanks!


Segher


> On 12/20/18 5:44 PM, Segher Boessenkool wrote:
> > On Thu, Dec 20, 2018 at 05:34:54PM -0600, Aaron Sawdey wrote:
> >> On 12/20/18 3:51 AM, Segher Boessenkool wrote:
> >>> On Wed, Dec 19, 2018 at 01:53:05PM -0600, Aaron Sawdey wrote:
>  Because of POWER9 dd2.1 issues with certain unaligned vsx instructions
>  to cache inhibited memory, here is a patch that keeps memmove (and 
>  memcpy)
>  inline expansion from doing unaligned vector or using vector load/store
>  other than lvx/stvx. More description of the issue is here:
> 
>  https://patchwork.ozlabs.org/patch/814059/
> 
>  OK for trunk if bootstrap/regtest ok?
> >>>
> >>> Okay, but see below.
> >>>
> >> [snip]
> >>>
> >>> This is extraordinarily clumsy :-)  Maybe something like:
> >>>
> >>> static rtx
> >>> gen_lvx_v4si_move (rtx dest, rtx src)
> >>> {
> >>>   gcc_assert (!(MEM_P (dest) && MEM_P (src));
> >>>   gcc_assert (GET_MODE (dest) == V4SImode && GET_MODE (src) == V4SImode);
> >>>   if (MEM_P (dest))
> >>> return gen_altivec_stvx_v4si_internal (dest, src);
> >>>   else if (MEM_P (src))
> >>> return gen_altivec_lvx_v4si_internal (dest, src);
> >>>   else
> >>> gcc_unreachable ();
> >>> }
> >>>
> >>> (Or do you allow VOIDmode for src as well?)  Anyway, at least get rid of
> >>> the useless extra variable.
> >>
> >> I think this should be better:
> > 
> > The gcc_unreachable at the end catches the non-mem to non-mem case.
> > 
> >> static rtx
> >> gen_lvx_v4si_move (rtx dest, rtx src)
> >> {
> >>   gcc_assert ((MEM_P (dest) && !MEM_P (src)) || (MEM_P (src) && 
> >> !MEM_P(dest)));
> > 
> > But if you prefer this, how about
> > 
> > {
> >   gcc_assert (MEM_P (dest) ^ MEM_P (src));
> >   gcc_assert (GET_MODE (dest) == V4SImode && GET_MODE (src) == V4SImode);
> > 
> >   if (MEM_P (dest))
> > return gen_altivec_stvx_v4si_internal (dest, src);
> >   else
> > return gen_altivec_lvx_v4si_internal (dest, src);
> > }
> > 
> > :-)
> > 
> > 
> > Segher
> > 
> 
> 2019-01-03  Aaron Sawdey  
> 
> * config/rs6000/rs6000-string.c (expand_block_move): Don't use
> unaligned vsx and avoid lxvd2x/stxvd2x.
> (gen_lvx_v4si_move): New function.
> 
> 
> Index: gcc/config/rs6000/rs6000-string.c
> ===
> --- gcc/config/rs6000/rs6000-string.c (revision 267299)
> +++ gcc/config/rs6000/rs6000-string.c (working copy)
> @@ -2669,6 +2669,25 @@
>return true;
>  }
> 
> +/* Generate loads and stores for a move of v4si mode using lvx/stvx.
> +   This uses altivec_{l,st}vx__internal which use unspecs to
> +   keep combine from changing what instruction gets used.
> +
> +   DEST is the destination for the data.
> +   SRC is the source of the data for the move.  */
> +
> +static rtx
> +gen_lvx_v4si_move (rtx dest, rtx src)
> +{
> +  gcc_assert (MEM_P (dest) ^ MEM_P (src));
> +  gcc_assert (GET_MODE (dest) == V4SImode && GET_MODE (src) == V4SImode);
> +
> +  if (MEM_P (dest))
> +return gen_altivec_stvx_v4si_internal (dest, src);
> +  else
> +return gen_altivec_lvx_v4si_internal (dest, src);
> +}
> +
>  /* Expand a block move operation, and return 1 if successful.  Return 0
> if we should let the compiler generate normal code.
> 
> @@ -2721,11 +2740,11 @@
> 
>/* Altivec first, since it will be faster than a string move
>when it applies, and usually not significantly larger.  */
> -  if (TARGET_ALTIVEC && bytes >= 16 && (TARGET_EFFICIENT_UNALIGNED_VSX 
> || align >= 128))
> +  if (TARGET_ALTIVEC && bytes >= 16 && align >= 128)
>   {
> move_bytes = 16;
> mode = V4SImode;
> -   gen_func.mov = gen_movv4si;
> +   gen_func.mov = gen_lvx_v4si_move;
>   }
>else if (bytes >= 8 && TARGET_POWERPC64
>  && (align >= 64 || !STRICT_ALIGNMENT))
> 
> 
> 
> -- 
> Aaron Sawdey, Ph.D.  acsaw...@linux.vnet.ibm.com
> 050-2/C113  (507) 253-7520 home: 507/263-0782
> IBM Linux Technology Center - PPC Toolchain

Re: [PATCH 1/9] [libbacktrace] Read .gnu_debugaltlink

[Tom de Vries]

On 16-01-19 01:56, Ian Lance Taylor wrote:
> On Tue, Dec 11, 2018 at 2:14 AM Tom de Vries  wrote:
>>
>> Read the elf file pointed at by the .gnu_debugaltlink section, and verify 
>> that
>> the build id matches.
>>
>> 2018-11-11  Tom de Vries  
>>
>> * elf.c (elf_add): Add and handle with_buildid_data and
>> with_buildid_size parameters.  Handle .gnu_debugaltlink section.
>> (phdr_callback, backtrace_initialize): Add arguments to elf_add 
>> calls.
>> ---
> 
> 
> 
> @@ -2899,6 +2918,27 @@ elf_add (struct backtrace_state *state, const
> char *filename, int descriptor,
>> }
>> }
>>
>> +  if (!debugaltlink_view_valid
>> + && strcmp (name, ".gnu_debugaltlink") == 0)
>> +   {
>> + const char *debugaltlink_data;
>> + size_t debugaltlink_name_len;
>> +
>> + if (!backtrace_get_view (state, descriptor, shdr->sh_offset,
>> +  shdr->sh_size, error_callback, data,
>> +  _view))
>> +   goto fail;
>> +
>> + debugaltlink_view_valid = 1;
>> + debugaltlink_data = (const char *) debugaltlink_view.data;
>> + debugaltlink_name = debugaltlink_data;
>> + debugaltlink_name_len = strnlen (debugaltlink_data, shdr->sh_size);
>> + debugaltlink_buildid_data = (debugaltlink_data
>> +  + debugaltlink_name_len
>> +  + 1);
>> + debugaltlink_buildid_size = shdr->sh_size - debugaltlink_name_len 
>> - 1;
>> +   }
>> +
> 
> This doesn't look quite right.  debugaltlink_buildid_size is unsigned.
> If there is some misunderstanding of the format it's possible for
> strnlen to return shdr->sh_size.  If it does,
> debugaltlink_buildid_size will be set to a very large value.
> 

I see, thanks for finding that.

Fixed like this:
...
debugaltlink_name_len = strnlen (debugaltlink_data, shdr->sh_size);
if (debugaltlink_name_len < shdr->sh_size)
  {
/* Include terminating zero.  */
debugaltlink_name_len =+ 1;

debugaltlink_buildid_data
  = debugaltlink_data + debugaltlink_name_len;
debugaltlink_buildid_size
  = shdr->sh_size - debugaltlink_name_len;
  }
...

>> +  if (debugaltlink_name != NULL)
>> +{
>> +  int d;
>> +
>> +  d = elf_open_debugfile_by_debuglink (state, filename, 
>> debugaltlink_name,
>> +  0, error_callback, data);
>> +  if (d >= 0)
>> +   {
>> + int ret;
>> +
>> + ret = elf_add (state, filename, d, base_address, error_callback, 
>> data,
>> +fileline_fn, found_sym, found_dwarf, 0, 1,
>> +debugaltlink_buildid_data, 
>> debugaltlink_buildid_size);
>> + backtrace_release_view (state, _view, error_callback,
>> + data);
>> + debugaltlink_view_valid = 0;
>> + if (ret < 0)
>> +   {
>> + backtrace_close (d, error_callback, data);
>> + return ret;
>> +   }
>> +   }
>> +  else
>> +   {
>> + error_callback (data,
>> + "Could not open .gnu_debugaltlink", 0);
>> + /* Don't goto fail, but try continue without the info in the
>> +.gnu_debugaltlink.  */
>> +   }
>> +}
> 
> The strings passed to error_callback always start with a lowercase
> letter (unless they start with something like ELF) because the
> callback will most likely print them with some prefix.
> 

Fixed.

> More seriously, we don't call error_callback in any cases that
> correspond to this.  We just carry on.

Indeed, I noticed that, and filed PR88244 - "[libbacktrace] Failure to
open .gnu_debuglink is silent".

[ The scenario there is: an executable has a .gnu_debuglink, but the
file the .gnu_debuglink is pointing to is absent, because f.i. it has
been removed, or moved to a different location. If a user runs this
executable and a backtrace is triggered, the information relating to the
functions in the executable will be missing in the backtrace, but
there's no error explaining to the user why that information is missing.
 Note: there is a default error "no debug info in ELF executable" in
elf_nodebug, but AFAIU this is not triggered if debug info for one of
the shared libraries is present. ]

BTW, though in the code above an error_callback is called, we don't
error out, but do carry on afterwards (as the comment explicitly states).

> Is there any reason to call
> error_callback here?

A similar scenario: an executable has a .gnu_altdebuglink, but the file
the .gnu_debugaltlink is pointing to is absent, because f.i. it has been
removed, or moved to a different location. If a user runs this
executable and a backtrace is triggered, the information stored in the
.gnu_debugaltlink file will be missing in the backtrace, but there's no
error explaining to the user 

Re: [C++ Patch] Use locations[ds_storage_class] in error messages about ill-formed uses of mutable

[Jason Merrill]

On 1/15/19 5:27 PM, Paolo Carlini wrote:

Hi Martin,

On 15/01/19 21:42, Martin Sebor wrote:

On 1/15/19 9:58 AM, Paolo Carlini wrote:

Hi,

something a little different from my last patches but nevertheless 
pretty straightforward (noticed while I was wondering whether we 
should immediately move the location_t grokdeclarator local even 
further up). Tested x86_64-linux, as usual.


Since you're already making changes to the tests, would be too much
more work to also add quoting around static and const in the error
messages below where mutable is already quoted:

-  error ("static %qs cannot be declared %", name);
+  error_at (sloc, "static %qs cannot be declared %", 
name);

   storage_class = sc_none;
 }
   else if (type_quals & TYPE_QUAL_CONST)
 {
-  error ("const %qs cannot be declared %", name);
+  error_at (sloc, "const %qs cannot be declared %", name);

(I can see it being a hassle if there were many other tests where
the messages expect to find static and const with no quotes.)


No problem, that's not the case. I'm finishing testing the below, then.


OK.

Jason

[PATCH, alpha]: Correctly handle split _Complex float variable arguments

[Uros Bizjak]

Hello!

Attached patch corrects handing of split _Complex float variable
arguments. Alpha is not able to pass 32bit floats in float registers
as variable arguments (see the comment in alpha_pass_by_reference), so
we pass them by reference. However, complex float arguments are split
to their real and imaginary part and passed separately. When passed by
reference, every part is passed separately by a separate reference.
Attached patch fixes this oversight in alpha_gimplify_va_arg.

The patch fixes:

FAIL: gcc.dg/compat/scalar-by-value-4 c_compat_x_tst.o-c_compat_y_tst.o execute
FAIL: gcc.dg/compat/scalar-return-4 c_compat_x_tst.o-c_compat_y_tst.o execute

2019-01-16  Uroš Bizjak  

* config/alpha/alpha.c (alpha_gimplify_va_arg):
Handle split indirect COMPLEX_TYPE arguments.

Bootstrapped and regression tested on alphaev68-linux-gnu.

Committed to mainline SVN.

Uros.
diff --git a/gcc/config/alpha/alpha.c b/gcc/config/alpha/alpha.c
index ce45c54eeb7a..f0e8124797f3 100644
--- a/gcc/config/alpha/alpha.c
+++ b/gcc/config/alpha/alpha.c
@@ -6378,8 +6378,40 @@ alpha_gimplify_va_arg (tree valist, tree type, 
gimple_seq *pre_p,
   offset = get_initialized_tmp_var (t, pre_p, NULL);
 
   indirect = pass_by_reference (NULL, TYPE_MODE (type), type, false);
+
   if (indirect)
-type = build_pointer_type_for_mode (type, ptr_mode, true);
+{
+  if (TREE_CODE (type) == COMPLEX_TYPE
+ && targetm.calls.split_complex_arg (type))
+   {
+ tree real_part, imag_part, real_temp;
+
+ tree ptr_type = build_pointer_type_for_mode (TREE_TYPE (type),
+  ptr_mode, true);
+
+ real_part = alpha_gimplify_va_arg_1 (ptr_type, base,
+  offset, pre_p);
+ real_part = build_va_arg_indirect_ref (real_part);
+
+ /* Copy the value into a new temporary, lest the formal temporary
+be reused out from under us.  */
+ real_temp = get_initialized_tmp_var (real_part, pre_p, NULL);
+
+ imag_part = alpha_gimplify_va_arg_1 (ptr_type, base,
+  offset, pre_p);
+ imag_part = build_va_arg_indirect_ref (imag_part);
+
+ r = build2 (COMPLEX_EXPR, type, real_temp, imag_part);
+
+ /* Stuff the offset temporary back into its field.  */
+ gimplify_assign (unshare_expr (offset_field),
+  fold_convert (TREE_TYPE (offset_field), offset),
+  pre_p);
+ return r;
+   }
+  else
+   type = build_pointer_type_for_mode (type, ptr_mode, true);
+}
 
   /* Find the value.  Note that this will be a stable indirection, or
  a composite of stable indirections in the case of complex.  */

Re: C++ PATCH for c++/78244 - narrowing conversion in template not detected

[Jason Merrill]

On 1/16/19 7:32 AM, Marek Polacek wrote:

While looking into 88815, a 9 regression, I realized we'll have to fix 78244
first.  This patch fixes one half of 78244.  The problem was that we never
detected narrowing conversion in a template, because the IMPLICIT_CONV_EXPR
template code doesn't have the information whether it was initialized by a
braced-init-list.  This patch adds such a bit, which works much like
IMPLICIT_CONV_EXPR_DIRECT_INIT.

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


OK.

Jason

Re: [PATCH] Fix ICE due to "combine" creating unreachable EH blocks (PR target/88861)

[Segher Boessenkool]

Hi!

Thanks for working on this.

On Wed, Jan 16, 2019 at 10:20:26AM -0500, David Malcolm wrote:
> PR target/88861 reports an ICE in "ce2" due to an unreachable
> basic block.
> 
> The block becomes unreachable in "combine" when delete_noop_moves
> deletes an insn with a REG_EH_REGION, deleting the EH edge, the
> only edge leading to the basic block.
> 
> Normally, rest_of_handle_combine would call cleanup_cfg, deleting
> unreachable blocks, if combine_instructions returns true, and
> combine_instructions does return true for some cases of edge-removal,
> but it doesn't for this case, leading to the ICE.
> 
> This patch updates delete_noop_moves so that it returns true if
> it deletes any edges, and passes that through to combine_instructions,
> so that it too will return true if any edges were deleted, ensuring that
> cleanup_cfg will be called by rest_of_handle_combine for this case,
> deleting the now-unreachable block, and fixing the ICE.

> -/* Delete any insns that copy a register to itself.  */
> +/* Delete any insns that copy a register to itself.
> +   Return true if any edges are eliminated.  */

"if the CFG was changed" or similar text?

> -static void
> +static bool
>  delete_noop_moves (void)
>  {
>rtx_insn *insn, *next;
>basic_block bb;
>  
> +  bool edges_deleted = false;
> +
>FOR_EACH_BB_FN (bb, cfun)
>  {
>for (insn = BB_HEAD (bb); insn != NEXT_INSN (BB_END (bb)); insn = next)
> @@ -1001,10 +1004,11 @@ delete_noop_moves (void)
> if (dump_file)
>   fprintf (dump_file, "deleting noop move %d\n", INSN_UID (insn));
>  
> -   delete_insn_and_edges (insn);
> +   edges_deleted |= delete_insn_and_edges (insn);
>   }
>   }
>  }

Empty line here, too, please.

> +  return edges_deleted;
>  }

>  /* Main entry point for combiner.  F is the first insn of the function.
> NREGS is the first unused pseudo-reg number.
>  
> -   Return nonzero if the combiner has turned an indirect jump
> -   instruction into a direct jump.  */
> +   Return nonzero if any edges have been removed (e.g. if the combiner has
> +   turned an indirect jump instruction into a direct jump).  */

"if the CFG was changed", again.

>  static int
>  combine_instructions (rtx_insn *f, unsigned int nregs)


Okay with that.  Thanks!


Segher

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

[Martin Jambor]

Hi,

On Mon, Dec 10 2018, Richard Biener wrote:
> On Fri, Dec 7, 2018 at 3:59 PM Martin Jambor  wrote:
>>
>> Hi,
>>
>> ICE in PR 88214 happens because a type-mismatch in K C code makes
>> IPA-CP analysis call ao_ref_init_from_ptr_and_size on an integer
>> SSA_NAME, this function in turn constructs a temporary MEM_REF based on
>> that integer SSA_NAME and then later on call_may_clobber_ref_p_1 treats
>> the MEM_REF base as a pointer, gets its SSA_NAME_PTR_INFO and tries to
>> work with bitmaps there.  But because the SSA_NAME is an integer, there
>> is no SSA_NAME_PTR_INFO, there is range info instead and this leads to a
>> crash.
>>

...

>> The bug itself can be fixed with the patch below.  I have verified it
>> avoids the ICE on powerpc64-linux and did a full bootstrap and test on
>> an x86_64-linux.  The patch is simple enough that I believe that is good
>> enough.
>
> OK.
>
> Richard.

I have bootstrapped the patch on gcc-8 an gcc-7 branches too and will
commit it there in a few moments too.

Thanks,

Martin


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

PR target/86891 __builtin__overflow issues on AArch64 (redux)

[Richard Earnshaw (lists)]

Further investigation showed that my previous patch for this issue was
still incomplete.

The problem stemmed from what I suspect was a mis-understanding of the
way overflow is calculated on aarch64 when values are subtracted (and
hence in comparisons).  In this case, unlike addition, the carry flag is
/cleared/ if there is overflow (technically, underflow) and set when
that does not happen.  This patch clears up this issue by using CCmode
for all subtractive operations (this can fully describe the normal
overflow conditions without anything particularly fancy); clears up the
way we express normal unsigned overflow using CC_Cmode (the result of a
sum is less than one of the operands) and adds a new mode, CC_ADCmode to
handle expressing overflow of an add-with-carry operation, where the
standard idiom is no-longer sufficient to describe the overflow condition.

PR target/86891
* config/aarch64/aarch64-modes.def: Add comment about how the carry
bit is set by add and compare.
(CC_ADC): New CC_MODE.
* config/aarch64/aarch64.c (aarch64_select_cc_mode): Use variables
to cache the code and mode of X.  Adjust the shape of a CC_Cmode
comparison.  Add detection for CC_ADCmode.
(aarch64_get_condition_code_1): Update code support for CC_Cmode.  Add
CC_ADCmode.
* config/aarch64/aarch64.md (uaddv4): Use LTU with CCmode.
(uaddvti4): Comparison result is in CC_ADCmode and the condition is GEU.
(add3_compareC_cconly_imm): Delete.  Merge into...
(add3_compareC_cconly): ... this.  Restructure the comparison
to eliminate the need for zero-extending the operands.
(add3_compareC_imm): Delete.  Merge into ...
(add3_compareC): ... this.  Restructure the comparison to
eliminate the need for zero-extending the operands.
(add3_carryin): Use LTU for the overflow detection.
(add3_carryinC): Use CC_ADCmode for the result of the carry out.
Reexpress comparison for overflow.
(add3_carryinC_zero): Update for change to add3_carryinC.
(add3_carryinC): Likewise.
(add3_carryinV): Use LTU for carry between partials.
* config/aarch64/predicates.md (aarch64_carry_operation): Update
handling of CC_Cmode and add CC_ADCmode.
(aarch64_borrow_operation): Likewise.

Bootstrapped on aarch64-linux.  Applied to trunk.

diff --git a/gcc/config/aarch64/aarch64-modes.def b/gcc/config/aarch64/aarch64-modes.def
index 5fe5ef0..14c1a43 100644
--- a/gcc/config/aarch64/aarch64-modes.def
+++ b/gcc/config/aarch64/aarch64-modes.def
@@ -18,12 +18,25 @@
along with GCC; see the file COPYING3.  If not see
.  */
 
+/* Important note about Carry generation in AArch64.
+
+   Unlike some architectures, the C flag generated by a subtract
+   operation, or a simple compare operation is set to 1 if the result
+   does not overflow in an unsigned sense.  That is, if there is no
+   borrow needed from a higher word.  That means that overflow from
+   addition will set C, but overflow from a subtraction will clear C.
+   We use CC_Cmode to represent detection of overflow from addition as
+   CCmode is used for 'normal' compare (subtraction) operations.  For
+   ADC, the representation becomes more complex still, since we cannot
+   use the normal idiom of comparing the result to one of the input
+   operands; instead we use CC_ADCmode to represent this case.  */
 CC_MODE (CCFP);
 CC_MODE (CCFPE);
 CC_MODE (CC_SWP);
 CC_MODE (CC_NZ);/* Only N and Z bits of condition flags are valid.  */
 CC_MODE (CC_Z); /* Only Z bit of condition flags is valid.  */
-CC_MODE (CC_C); /* Only C bit of condition flags is valid.  */
+CC_MODE (CC_C); /* C represents unsigned overflow of a simple addition.  */
+CC_MODE (CC_ADC);   /* Unsigned overflow from an ADC (add with carry).  */
 CC_MODE (CC_V); /* Only V bit of condition flags is valid.  */
 
 /* Half-precision floating point for __fp16.  */
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index fd60bdd..da48c09 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -7089,9 +7089,12 @@ aarch64_emit_call_insn (rtx pat)
 machine_mode
 aarch64_select_cc_mode (RTX_CODE code, rtx x, rtx y)
 {
+  machine_mode mode_x = GET_MODE (x);
+  rtx_code code_x = GET_CODE (x);
+
   /* All floating point compares return CCFP if it is an equality
  comparison, and CCFPE otherwise.  */
-  if (GET_MODE_CLASS (GET_MODE (x)) == MODE_FLOAT)
+  if (GET_MODE_CLASS (mode_x) == MODE_FLOAT)
 {
   switch (code)
 	{
@@ -7122,55 +7125,65 @@ aarch64_select_cc_mode (RTX_CODE code, rtx x, rtx y)
  using the TST instruction with the appropriate bitmask.  */
   if (y == const0_rtx && REG_P (x)
   && (code == EQ || code == NE)
-  && (GET_MODE (x) == HImode || GET_MODE (x) == QImode))
+  && (mode_x == HImode || mode_x == QImode))
 return 

Re: [PATCH] x86: Revert patches to fix PR target/88794

[Jakub Jelinek]

On Wed, Jan 16, 2019 at 10:48:28PM +0800, Wei Xiao wrote:
> The original runtime testcases are incorrect and I have fixed them as 
> attached.
> Is it ok to do the revert and fix the testcases for trunk?

LGTM.

> 2019-01-16  Wei Xiao  
> 
> * gcc.target/i386/avx512f-vfixupimmpd-2.c: Fix the test cases for
> VFIXUPIMM* intrinsics.
> * gcc.target/i386/avx512f-vfixupimmps-2.c: Ditto.
> * gcc.target/i386/avx512f-vfixupimmsd-2.c: Ditto.
> * gcc.target/i386/avx512f-vfixupimmss-2.c: Ditto.

Jakub

Re: [PATCH] x86: Revert patches to fix PR target/88794

[Wei Xiao]

The original runtime testcases are incorrect and I have fixed them as attached.
Is it ok to do the revert and fix the testcases for trunk?

Wei

2019-01-16  Wei Xiao  

* gcc.target/i386/avx512f-vfixupimmpd-2.c: Fix the test cases for
VFIXUPIMM* intrinsics.
* gcc.target/i386/avx512f-vfixupimmps-2.c: Ditto.
* gcc.target/i386/avx512f-vfixupimmsd-2.c: Ditto.
* gcc.target/i386/avx512f-vfixupimmss-2.c: Ditto.


fixupimm_testcases.diff
Description: Binary data

[PATCH] Fix ICE due to "combine" creating unreachable EH blocks (PR target/88861)

[David Malcolm]

PR target/88861 reports an ICE in "ce2" due to an unreachable
basic block.

The block becomes unreachable in "combine" when delete_noop_moves
deletes an insn with a REG_EH_REGION, deleting the EH edge, the
only edge leading to the basic block.

Normally, rest_of_handle_combine would call cleanup_cfg, deleting
unreachable blocks, if combine_instructions returns true, and
combine_instructions does return true for some cases of edge-removal,
but it doesn't for this case, leading to the ICE.

This patch updates delete_noop_moves so that it returns true if
it deletes any edges, and passes that through to combine_instructions,
so that it too will return true if any edges were deleted, ensuring that
cleanup_cfg will be called by rest_of_handle_combine for this case,
deleting the now-unreachable block, and fixing the ICE.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu
and on powerpc64le-unknown-linux-gnu.

OK for trunk?

gcc/ChangeLog:
PR target/88861
* combine.c (delete_noop_moves): Convert to "bool" return,
returning true if any edges are eliminated.
(combine_instructions): Also return true if delete_noop_moves
returns true.

gcc/testsuite/ChangeLog:
PR target/88861
* g++.dg/torture/pr88861.C: New test.
---
 gcc/combine.c  | 16 ++--
 gcc/testsuite/g++.dg/torture/pr88861.C | 11 +++
 2 files changed, 21 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/torture/pr88861.C

diff --git a/gcc/combine.c b/gcc/combine.c
index a8c9d15..010795e 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -983,14 +983,17 @@ combine_validate_cost (rtx_insn *i0, rtx_insn *i1, 
rtx_insn *i2, rtx_insn *i3,
 }
 
 
-/* Delete any insns that copy a register to itself.  */
+/* Delete any insns that copy a register to itself.
+   Return true if any edges are eliminated.  */
 
-static void
+static bool
 delete_noop_moves (void)
 {
   rtx_insn *insn, *next;
   basic_block bb;
 
+  bool edges_deleted = false;
+
   FOR_EACH_BB_FN (bb, cfun)
 {
   for (insn = BB_HEAD (bb); insn != NEXT_INSN (BB_END (bb)); insn = next)
@@ -1001,10 +1004,11 @@ delete_noop_moves (void)
  if (dump_file)
fprintf (dump_file, "deleting noop move %d\n", INSN_UID (insn));
 
- delete_insn_and_edges (insn);
+ edges_deleted |= delete_insn_and_edges (insn);
}
}
 }
+  return edges_deleted;
 }
 
 
@@ -1143,8 +1147,8 @@ insn_a_feeds_b (rtx_insn *a, rtx_insn *b)
 /* Main entry point for combiner.  F is the first insn of the function.
NREGS is the first unused pseudo-reg number.
 
-   Return nonzero if the combiner has turned an indirect jump
-   instruction into a direct jump.  */
+   Return nonzero if any edges have been removed (e.g. if the combiner has
+   turned an indirect jump instruction into a direct jump).  */
 static int
 combine_instructions (rtx_insn *f, unsigned int nregs)
 {
@@ -1529,7 +1533,7 @@ retry:
   default_rtl_profile ();
   clear_bb_flags ();
   new_direct_jump_p |= purge_all_dead_edges ();
-  delete_noop_moves ();
+  new_direct_jump_p |= delete_noop_moves ();
 
   /* Clean up.  */
   obstack_free (_link_obstack, NULL);
diff --git a/gcc/testsuite/g++.dg/torture/pr88861.C 
b/gcc/testsuite/g++.dg/torture/pr88861.C
new file mode 100644
index 000..d2b6a4b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/torture/pr88861.C
@@ -0,0 +1,11 @@
+// { dg-options "-fnon-call-exceptions" }
+
+struct Ax {
+  int n, a[];
+};
+
+int i = 12345678;
+int main() {
+  static Ax s{456, i};
+  ((s.a[0]) ? (void)0 : (void)0);
+}
-- 
1.8.5.3

[committed] Fix up pr51628-10.c testcase (PR target/88682)

[Jakub Jelinek]

Hi!

This testcase violates aliasing and so is "miscompiled" e.g. on powerpc64
or aarch64 at -O2.  Fixed thusly, tested on powerpc64-linux, committed to
trunk as obvious.

2019-01-16  Jakub Jelinek  

PR c/51628
PR target/88682
* c-c++-common/pr51628-10.c (unaligned_int128_t): Add
may_alias attribute.

--- gcc/testsuite/c-c++-common/pr51628-10.c.jj  2018-12-21 00:40:49.048937834 
+0100
+++ gcc/testsuite/c-c++-common/pr51628-10.c 2019-01-16 15:15:51.515279138 
+0100
@@ -11,7 +11,7 @@ struct pair_t
 typedef struct unaligned_int128_t_
 {
   __int128_t value;
-} __attribute__((packed)) unaligned_int128_t;
+} __attribute__((packed, may_alias)) unaligned_int128_t;
 
 struct pair_t p = {0, 1};
 unaligned_int128_t *addr = (unaligned_int128_t *) 

Jakub

[PATCH] rs6000: Add missing prototypes for vec_ld/vec_st

[Kewen.Lin]

Hi,

On target rs6000, for all types, use of vec_ld/vec_st historically permits the 
dereferenced pointer to be of a scalar type or the corresponding vector type. 
But for vector unsigned/signed long long and double, we have an omission in our 
table for vec_ld/vec_st. 

Ok for trunk, and eventual backport to GCC 7 and 8 aftersome burn-in time?

---
Changelog:

gcc/ChangeLog

2019-01-16  Kewen Lin  

* doc/extend.texi: Add four new prototypes for vec_ld and seven new
prototypes for vec_st.
* config/rs6000/rs6000-c.c (altivec_overloaded_builtins): Add entries
for scalar address type variants of altivec_vec_ld/altivec_vec_st,
mainly on signed/unsigned long long and double.

gcc/testsuite/ChangeLog

2019-01-16  Kewen Lin  

* gcc.target/powerpc/altivec_vld_vst_addr.c: New test.


Index: gcc/config/rs6000/rs6000-c.c
===
--- gcc/config/rs6000/rs6000-c.c(revision 267912)
+++ gcc/config/rs6000/rs6000-c.c(working copy)
@@ -1531,12 +1531,19 @@ const struct altivec_builtin_types altivec_overloa

   { ALTIVEC_BUILTIN_VEC_LD, ALTIVEC_BUILTIN_LVX_V2DF,
 RS6000_BTI_V2DF, RS6000_BTI_INTSI, ~RS6000_BTI_V2DF, 0 },
+  { ALTIVEC_BUILTIN_VEC_LD, ALTIVEC_BUILTIN_LVX_V2DF,
+RS6000_BTI_V2DF, RS6000_BTI_INTSI, ~RS6000_BTI_double, 0 },
   { ALTIVEC_BUILTIN_VEC_LD, ALTIVEC_BUILTIN_LVX_V2DI,
 RS6000_BTI_V2DI, RS6000_BTI_INTSI, ~RS6000_BTI_V2DI, 0 },
   { ALTIVEC_BUILTIN_VEC_LD, ALTIVEC_BUILTIN_LVX_V2DI,
+RS6000_BTI_V2DI, RS6000_BTI_INTSI, ~RS6000_BTI_long_long, 0 },
+  { ALTIVEC_BUILTIN_VEC_LD, ALTIVEC_BUILTIN_LVX_V2DI,
 RS6000_BTI_unsigned_V2DI, RS6000_BTI_INTSI,
 ~RS6000_BTI_unsigned_V2DI, 0 },
   { ALTIVEC_BUILTIN_VEC_LD, ALTIVEC_BUILTIN_LVX_V2DI,
+RS6000_BTI_unsigned_V2DI, RS6000_BTI_INTSI,
+~RS6000_BTI_unsigned_long_long, 0 },
+  { ALTIVEC_BUILTIN_VEC_LD, ALTIVEC_BUILTIN_LVX_V2DI,
 RS6000_BTI_bool_V2DI, RS6000_BTI_INTSI, ~RS6000_BTI_bool_V2DI, 0 },
   { ALTIVEC_BUILTIN_VEC_LD, ALTIVEC_BUILTIN_LVX_V4SF,
 RS6000_BTI_V4SF, RS6000_BTI_INTSI, ~RS6000_BTI_V4SF, 0 },
@@ -3737,14 +3744,27 @@ const struct altivec_builtin_types altivec_overloa

   { ALTIVEC_BUILTIN_VEC_ST, ALTIVEC_BUILTIN_STVX_V2DF,
 RS6000_BTI_void, RS6000_BTI_V2DF, RS6000_BTI_INTSI, ~RS6000_BTI_V2DF },
+  { ALTIVEC_BUILTIN_VEC_ST, ALTIVEC_BUILTIN_STVX_V2DF,
+RS6000_BTI_void, RS6000_BTI_V2DF, RS6000_BTI_INTSI, ~RS6000_BTI_double },
   { ALTIVEC_BUILTIN_VEC_ST, ALTIVEC_BUILTIN_STVX_V2DI,
 RS6000_BTI_void, RS6000_BTI_V2DI, RS6000_BTI_INTSI, ~RS6000_BTI_V2DI },
   { ALTIVEC_BUILTIN_VEC_ST, ALTIVEC_BUILTIN_STVX_V2DI,
+RS6000_BTI_void, RS6000_BTI_V2DI, RS6000_BTI_INTSI, ~RS6000_BTI_long_long 
},
+  { ALTIVEC_BUILTIN_VEC_ST, ALTIVEC_BUILTIN_STVX_V2DI,
 RS6000_BTI_void, RS6000_BTI_unsigned_V2DI, RS6000_BTI_INTSI,
 ~RS6000_BTI_unsigned_V2DI },
   { ALTIVEC_BUILTIN_VEC_ST, ALTIVEC_BUILTIN_STVX_V2DI,
+RS6000_BTI_void, RS6000_BTI_unsigned_V2DI, RS6000_BTI_INTSI,
+~RS6000_BTI_unsigned_long_long },
+  { ALTIVEC_BUILTIN_VEC_ST, ALTIVEC_BUILTIN_STVX_V2DI,
 RS6000_BTI_void, RS6000_BTI_bool_V2DI, RS6000_BTI_INTSI,
 ~RS6000_BTI_bool_V2DI },
+  { ALTIVEC_BUILTIN_VEC_ST, ALTIVEC_BUILTIN_STVX_V2DI,
+RS6000_BTI_void, RS6000_BTI_bool_V2DI, RS6000_BTI_INTSI,
+~RS6000_BTI_long_long },
+  { ALTIVEC_BUILTIN_VEC_ST, ALTIVEC_BUILTIN_STVX_V2DI,
+RS6000_BTI_void, RS6000_BTI_bool_V2DI, RS6000_BTI_INTSI,
+~RS6000_BTI_unsigned_long_long },
   { ALTIVEC_BUILTIN_VEC_ST, ALTIVEC_BUILTIN_STVX_V4SF,
 RS6000_BTI_void, RS6000_BTI_V4SF, RS6000_BTI_INTSI, ~RS6000_BTI_V4SF },
   { ALTIVEC_BUILTIN_VEC_ST, ALTIVEC_BUILTIN_STVX_V4SF,
Index: gcc/doc/extend.texi
===
--- gcc/doc/extend.texi (revision 267912)
+++ gcc/doc/extend.texi (working copy)
@@ -18451,6 +18451,10 @@ vector double vec_div (vector double, vector doubl
 vector long vec_div (vector long, vector long);
 vector unsigned long vec_div (vector unsigned long, vector unsigned long);
 vector double vec_floor (vector double);
+vector signed long long vec_ld (int, const vector signed long long *);
+vector signed long long vec_ld (int, const signed long long *);
+vector unsigned long long vec_ld (int, const vector unsigned long long *);
+vector unsigned long long vec_ld (int, const unsigned long long *);
 vector __int128 vec_ld (int, const vector __int128 *);
 vector unsigned __int128 vec_ld (int, const vector unsigned __int128 *);
 vector __int128 vec_ld (int, const __int128 *);
@@ -18529,6 +18533,13 @@ vector signed long vec_splats (signed long);
 vector unsigned long vec_splats (unsigned long);
 vector float vec_sqrt (vector float);
 vector double vec_sqrt (vector double);
+void vec_st (vector signed long long, int, vector signed long long *);
+void vec_st (vector signed long long, int, signed long long *);
+void vec_st (vector unsigned long long, int, vector unsigned 

Re: Fortran vector math header

[Joseph Myers]

On Wed, 16 Jan 2019, Jakub Jelinek wrote:

> In normal C headers, we can #if __WORDSIZE == 32 or __SIZEOF_LONG__ == 4 or
> defined(__ILP64__) and similar, but in these headers we can't, as no
> preprocessing is happening.

(With such preprocessing, the mechanism glibc uses for gnu/stubs.h and 
gnu/lib-names.h could be used.)

> So, I think we need more multi-arch like setup for these locations, say
> /usr/include/finclude/{,/}/
> or similar which would be less ambiguous.

My understanding from the previous discussions is that with 
--enable-multiarch, these files will (or should) use multiarch names - but 
we don't have any support for using multiarch names for only some files in 
a toolchain.  (Which leads to suggestions like this one for having a 
target triplet in there which isn't actually a canonical multiarch name.)

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: Fortran vector math header

[Joseph Myers]

On Wed, 16 Jan 2019, Martin Liška wrote:

> Now we should add the header file into glibc, I kicked a discussion here:
> https://sourceware.org/ml/libc-help/2018-11/msg00015.html

libc-help is for user questions.  It's not suitable for glibc development 
discussions.

-- 
Joseph S. Myers
jos...@codesourcery.com

[Vectorizer] Add SLP support for masked loads

[Alejandro Martinez Vicente]

Hi,

Current vectorizer doesn't support masked loads for SLP. We should add that, to
allow things like:

void
f (int *restrict x, int *restrict y, int *restrict z, int n)
{
  for (int i = 0; i < n; i += 2)
{
  x[i] = y[i] ? z[i] : 1;
  x[i + 1] = y[i + 1] ? z[i + 1] : 2;
}
}

to be vectorized using contiguous loads rather than LD2 and ST2.

This patch was motivated by SVE, but it is completely generic and should apply
to any architecture with masked loads.

After the patch is applied, the above code generates this output
(-march=armv8.2-a+sve -O2 -ftree-vectorize):

 :
   0:   717fcmp w3, #0x0
   4:   540002cdb.le5c 
   8:   51000464sub w4, w3, #0x1
   c:   d283mov x3, #0x0// #0
  10:   9005adrpx5, 0 
  14:   25d8e3e0ptrue   p0.d
  18:   53017c84lsr w4, w4, #1
  1c:   91a5add x5, x5, #0x0
  20:   11000484add w4, w4, #0x1
  24:   85c0e0a1ld1rd   {z1.d}, p0/z, [x5]
  28:   2598e3e3ptrue   p3.s
  2c:   d37ff884lsl x4, x4, #1
  30:   25a41fe2whilelo p2.s, xzr, x4
  34:   d503201fnop
  38:   a5434820ld1w{z0.s}, p2/z, [x1, x3, lsl #2]
  3c:   25808c11cmpne   p1.s, p3/z, z0.s, #0
  40:   25808810cmpne   p0.s, p2/z, z0.s, #0
  44:   a5434040ld1w{z0.s}, p0/z, [x2, x3, lsl #2]
  48:   05a1c400sel z0.s, p1, z0.s, z1.s
  4c:   e5434800st1w{z0.s}, p2, [x0, x3, lsl #2]
  50:   04b0e3e3incwx3
  54:   25a41c62whilelo p2.s, x3, x4
  58:   5401b.ne38   // b.any
  5c:   d65f03c0ret


I tested this patch in an aarch64 machine bootstrapping the compiler and
running the checks.

Alejandro

gcc/Changelog:

2019-01-16  Alejandro Martinez  

* config/aarch64/aarch64-sve.md (copysign3): New define_expand.
(xorsign3): Likewise.
internal-fn.c: Marked mask_load_direct and mask_store_direct as
vectorizable.
tree-data-ref.c (data_ref_compare_tree): Fixed comment typo.
tree-vect-data-refs.c (can_group_stmts_p): Allow masked loads to be
combined even if masks different.
(slp_vect_only_p): New function to detect masked loads that are only
vectorizable using SLP.
(vect_analyze_data_ref_accesses): Mark SLP only vectorizable groups.
tree-vect-loop.c (vect_dissolve_slp_only_groups): New function to
dissolve SLP-only vectorizable groups when SLP has been discarded.
(vect_analyze_loop_2): Call vect_dissolve_slp_only_groups when needed.
tree-vect-slp.c (vect_get_and_check_slp_defs): Check masked loads
masks.
(vect_build_slp_tree_1): Fixed comment typo.
(vect_build_slp_tree_2): Include masks from masked loads in SLP tree.
tree-vect-stmts.c (vect_get_vec_defs_for_operand): New function to get
vec_defs for operand with optional SLP and vectype.
(vectorizable_load): Allow vectorizaion of masked loads for SLP only.
tree-vectorizer.h (_stmt_vec_info): Added flag for SLP-only
vectorizable.
tree-vectorizer.c (vec_info::new_stmt_vec_info): Likewise.

gcc/testsuite/Changelog:
 
2019-01-16  Alejandro Martinez  

* gcc.target/aarch64/sve/mask_load_slp_1.c: New test for SLP
vectorized masked loads.


mask_load_slp_1.patch
Description: mask_load_slp_1.patch

Re: [PATCH, rs6000, testsuite] Fix PR87306

[Bill Schmidt]

I can't approve the patch, but I agree this is the right fix.

Bill


On 1/16/19 3:29 AM, Kewen.Lin wrote:
> Hi,
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87306, it's older POWER 
> hardware specific test case issue. On POWER7 and earlier, we implicitly 
> set flag -mno-allow-movmisalign which disables vectorization when to 
> vectorize the code requires that misaligned loads and stores. For POWER8
> and upper, the opposite value is implicitly set(-mallow-movmisalign),
> then the vectorization performs as expected.
>
> Similar to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65484, the proposed 
> fix is to make dg-final checks whether vect_hw_misalign support on powerpc,
> only further verify the dump pattern when it's on to consitent to the 
> behavior.
>
> Tested on trunk for POWER8 LE and POWER7 BE systems.
> Is this ok for trunk?
>
> ---
>
>  gcc/testsuite/ChangeLog
>
> 2019-01-16  Kewen Lin  
>
>   PR target/87306
>   * gcc.dg/vect/bb-slp-pow-1.c : Modify to reflect that the loop is not
>   vectorized on POWER unless hardware misaligned loads are available.
>
> diff --git a/gcc/testsuite/gcc.dg/vect/bb-slp-pow-1.c 
> b/gcc/testsuite/gcc.dg/vect/bb-slp-pow-1.c
> index 5a05bd4..6742e12 100644
> --- a/gcc/testsuite/gcc.dg/vect/bb-slp-pow-1.c
> +++ b/gcc/testsuite/gcc.dg/vect/bb-slp-pow-1.c
> @@ -25,4 +25,8 @@ main (void)
>return 0;
>  }
>
> -/* { dg-final { scan-tree-dump-times "basic block vectorized" 1 "slp2" } } */
> +/* On older powerpc hardware (POWER7 and earlier), the default flag
> +   -mno-allow-movmisalign prevents vectorization.  On POWER8 and later,
> +   when vect_hw_misalign is true, vectorization occurs. */
> +
> +/* { dg-final { scan-tree-dump-times "basic block vectorized" 1 "slp2" { 
> target {{ ! powerpc*-*-* } || { powerpc*-*-* && vect_hw_misalign }} } } } */

Re: Fortran vector math header

[Jakub Jelinek]

On Wed, Jan 16, 2019 at 10:42:13AM +0100, Martin Liška wrote:
> On 1/15/19 6:45 PM, Joseph Myers wrote:
> > On Mon, 14 Jan 2019, Martin Liška wrote:
> > 
> >> Thanks for review, fixed that in updated version of the patch.
> >>
> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >>
> >> Ready to be installed?
> > 
> > This patch is OK.
> > 
> 
> Great, thank you very much for advises you provided.
> 
> Now we should add the header file into glibc, I kicked a discussion here:
> https://sourceware.org/ml/libc-help/2018-11/msg00015.html
> 
> Joseph: will you please help with a patch for glibc?
> 
> I'm also planning to document the header file in gfortran so that other
> compiler can leverage that.

I'd like to raise a concern I have.
On IRC you've mentioned you'd like to see that header in
/usr/include/finclude/{,32/}math-vector-fortran.h
(with 32/ header being empty) which looks problematic to me.
The 32/ in there is the compiler's multilib suffix, which is though
dependent on how exactly one configures the compiler.
On x86_64-linux-gnu, one can either build (native) x86_64-pc-linux-gnu compiler
that defaults to -m64, supports -m32 and uses . and 32/ multilibs for those,
or one can build also native i686-pc-linux-gnu compiler (ideally with 32-bit
binutils too) that only supports -m32 and uses . multilib for the 32-bit.
If glibc installs the above, then if one uses the 32-bit i686-linux
gfortran, it will misbehave, as it will think glibc.i686 provides the
math-vector.h entrypoints when it doesn't.

Right now no other target supports them, but if say powerpc64-linux got
them, there one can build powerpc-linux gcc that supports -m32 and -m64
with . and 64/ multilib subdirs, or powerpc64-linux gcc that supports
-m64 and -m32 with . and 32/ multilib subdirs.  So, what would
/usr/include/finclude/math-vector-fortran.h stand for in that case (again,
assuming the entry points are implemented in one of the multilibs only).

In normal C headers, we can #if __WORDSIZE == 32 or __SIZEOF_LONG__ == 4 or
defined(__ILP64__) and similar, but in these headers we can't, as no
preprocessing is happening.

So, I think we need more multi-arch like setup for these locations, say
/usr/include/finclude/{,/}/
or similar which would be less ambiguous.

Jakub

C++ PATCH for c++/78244 - narrowing conversion in template not detected

[Marek Polacek]

While looking into 88815, a 9 regression, I realized we'll have to fix 78244
first.  This patch fixes one half of 78244.  The problem was that we never
detected narrowing conversion in a template, because the IMPLICIT_CONV_EXPR
template code doesn't have the information whether it was initialized by a
braced-init-list.  This patch adds such a bit, which works much like
IMPLICIT_CONV_EXPR_DIRECT_INIT.

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

2019-01-16  Marek Polacek  

PR c++/78244 - narrowing conversion in template not detected.
* call.c (perform_implicit_conversion_flags): Set
IMPLICIT_CONV_EXPR_BRACED_INIT.
* cp-tree.h (IMPLICIT_CONV_EXPR_BRACED_INIT): New.
* pt.c (tsubst_copy_and_build): Use it.

* g++.dg/cpp0x/Wnarrowing13.C: New test.
* g++.dg/cpp0x/Wnarrowing14.C: New test.

diff --git gcc/cp/call.c gcc/cp/call.c
index 8bc8566e8d6..4f04b610004 100644
--- gcc/cp/call.c
+++ gcc/cp/call.c
@@ -11018,6 +11018,8 @@ perform_implicit_conversion_flags (tree type, tree expr,
   expr = build1 (IMPLICIT_CONV_EXPR, type, expr);
   if (!(flags & LOOKUP_ONLYCONVERTING))
IMPLICIT_CONV_EXPR_DIRECT_INIT (expr) = true;
+  if (flags & LOOKUP_NO_NARROWING)
+   IMPLICIT_CONV_EXPR_BRACED_INIT (expr) = true;
 }
   else
 expr = convert_like (conv, expr, complain);
diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h
index 6a2004330d2..1c85b37ba7f 100644
--- gcc/cp/cp-tree.h
+++ gcc/cp/cp-tree.h
@@ -442,6 +442,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
   OVL_HIDDEN_P (in OVERLOAD)
   SWITCH_STMT_NO_BREAK_P (in SWITCH_STMT)
   LAMBDA_EXPR_CAPTURE_OPTIMIZED (in LAMBDA_EXPR)
+  IMPLICIT_CONV_EXPR_BRACED_INIT (in IMPLICIT_CONV_EXPR)
3: (TREE_REFERENCE_EXPR) (in NON_LVALUE_EXPR) (commented-out).
   ICS_BAD_FLAG (in _CONV)
   FN_TRY_BLOCK_P (in TRY_BLOCK)
@@ -4229,6 +4230,11 @@ more_aggr_init_expr_args_p (const 
aggr_init_expr_arg_iterator *iter)
 #define IMPLICIT_CONV_EXPR_NONTYPE_ARG(NODE) \
   (TREE_LANG_FLAG_1 (IMPLICIT_CONV_EXPR_CHECK (NODE)))
 
+/* True if NODE represents a conversion for braced-init-list in a
+   template.  Set by perform_implicit_conversion_flags.  */
+#define IMPLICIT_CONV_EXPR_BRACED_INIT(NODE) \
+  (TREE_LANG_FLAG_2 (IMPLICIT_CONV_EXPR_CHECK (NODE)))
+
 /* Nonzero means that an object of this type cannot be initialized using
an initializer list.  */
 #define CLASSTYPE_NON_AGGREGATE(NODE) \
diff --git gcc/cp/pt.c gcc/cp/pt.c
index c6fc1cfeffb..83bceb26474 100644
--- gcc/cp/pt.c
+++ gcc/cp/pt.c
@@ -18210,6 +18210,8 @@ tsubst_copy_and_build (tree t,
int flags = LOOKUP_IMPLICIT;
if (IMPLICIT_CONV_EXPR_DIRECT_INIT (t))
  flags = LOOKUP_NORMAL;
+   if (IMPLICIT_CONV_EXPR_BRACED_INIT (t))
+ flags |= LOOKUP_NO_NARROWING;
RETURN (perform_implicit_conversion_flags (type, expr, complain,
  flags));
   }
diff --git gcc/testsuite/g++.dg/cpp0x/Wnarrowing13.C 
gcc/testsuite/g++.dg/cpp0x/Wnarrowing13.C
new file mode 100644
index 000..3750f293df3
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/Wnarrowing13.C
@@ -0,0 +1,8 @@
+// PR c++/78244
+// { dg-do compile { target c++11 } }
+
+template
+struct S {
+  static const int i{1.1}; // { dg-error "narrowing conversion" }
+  static const int i2 = {1.1}; // { dg-error "narrowing conversion" }
+};
diff --git gcc/testsuite/g++.dg/cpp0x/Wnarrowing14.C 
gcc/testsuite/g++.dg/cpp0x/Wnarrowing14.C
new file mode 100644
index 000..1aa22196f33
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/Wnarrowing14.C
@@ -0,0 +1,17 @@
+// PR c++/78244
+// { dg-do compile { target c++11 } }
+
+using Number = unsigned int;
+
+template 
+struct S {
+  S() {
+const Number x = {-1}; // { dg-error "narrowing conversion" }
+(void)x;
+  }
+};
+
+int main()
+{
+  S<1> s;
+}

Re: [PATCH] c-family: Update unaligned adress of packed member check

[Jakub Jelinek]

On Wed, Jan 16, 2019 at 04:11:44AM -0800, H.J. Lu wrote:
> > Why?  What is so special about C and (implicit?) casts where the rhs isn't
> > ADDR_EXPR?  Aren't all casts (explicit or implicit) from one pointer type
> > to another pointer and satisfy the rules something that should be warned
> 
> -Wincompatible-pointer-types is C only.   In C++,  incompatible pointer types
> aren't allowed at all.

How so?  You can certainly:
struct B { int i; };
struct C { struct B b; } __attribute__ ((packed));

extern struct C *p;
long* g8 (void) { return (long *)p; }

and similarly for C.  So, why is explicit cast something that shouldn't
be warned about in this case and implicit cast should get a warning,
especially when it already does get one (and one even enabled by default,
-Wincompatible-pointer-types)?
Either such casts are problematic and then it should treat explicit and
implicit casts the same, or they aren't, and then
-Wincompatible-pointer-types is all you need.

Jakub

Re: [PATCH] c-family: Update unaligned adress of packed member check

[H.J. Lu]

On Wed, Jan 16, 2019 at 3:30 AM Jakub Jelinek  wrote:
>
> On Mon, Jan 14, 2019 at 10:00:11AM -0800, H.J. Lu wrote:
> > On Mon, Jan 14, 2019 at 6:22 AM Jakub Jelinek  wrote:
> > >
> > > On Sun, Jan 13, 2019 at 06:54:05AM -0800, H.J. Lu wrote:
> > > > > What always matters is whether we take address of a packed structure
> > > > > field/non-static data member or whether we just read that field.
> > > > > The former should be warned about, the latter not.
> > > > >
> > > >
> > > > How about this patch?  It checks if address is taken with NOP.
> > >
> > > I'd like to first understand the convert_p argument to
> > > warn_for_address_or_pointer_of_packed_member.
> > >
> > > To me it seems you want to emit two different warnings, perhaps one
> > > surpressed if the other one is emitted, but you actually from the start
> > > decide which of the two you are going to check for.  That is just weird.
> >
> > convert_p  is only for C.
>
> Why?  What is so special about C and (implicit?) casts where the rhs isn't
> ADDR_EXPR?  Aren't all casts (explicit or implicit) from one pointer type
> to another pointer and satisfy the rules something that should be warned

-Wincompatible-pointer-types is C only.   In C++,  incompatible pointer types
aren't allowed at all.

> about if the conditions are met?
>

convert_p is set to TRUE to handle this case for incompatible pointer types:

[hjl@gnu-cfl-1 pr51628-3]$ cat x.i
struct B { int i; };
struct C { struct B b; } __attribute__ ((packed));

extern struct C *p;

long* g8 (void) { return p; }
[hjl@gnu-cfl-1 pr51628-3]$ make x.s
/export/build/gnu/tools-build/gcc-debug/build-x86_64-linux/gcc/xgcc
-B/export/build/gnu/tools-build/gcc-debug/build-x86_64-linux/gcc/ -O2
-S x.i
x.i: In function \u2018g8\u2019:
x.i:6:26: warning: returning \u2018struct C *\u2019 from a function
with incompatible return type \u2018long int *\u2019
[-Wincompatible-pointer-types]
6 | long* g8 (void) { return p; }
  |  ^
x.i:6:1: warning: converting a packed \u2018struct C *\u2019 pointer
(alignment 1) to \u2018long int *\u2019 (alignment 8) may result in an
unaligned pointer value [-Waddress-of-packed-member]
6 | long* g8 (void) { return p; }
  | ^~~~
x.i:2:8: note: defined here
2 | struct C { struct B b; } __attribute__ ((packed));
  |^
[hjl@gnu-cfl-1 pr51628-3]$

I am using the same function to warn both addresses and pointers.  I need a way
to tell that we are converting pointers, not assigning address to pointer.  This
info is readily available in the front-end.

-- 
H.J.

Re: [PATCH] Reset proper type on vector types (PR middle-end/88587).

[Richard Biener]

On Wed, Jan 16, 2019 at 10:20 AM Martin Liška  wrote:
>
> Hi.
>
> The patch is about resetting TYPE_MODE of vector types. This is problematic
> when an inlining among different ISAs happen. Then we end up with a different
> mode than when it's expected from debug info.
>
> When creating a new function decl in target_clones, we must valid_attribute_p 
> early
> so that the declaration has a proper cl_target_.. node and so that inliner can
> fix modes.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?

I don't like the new failure mode too much.  It looks like
create_version_clone_with_body
can fail so why not simply return NULL when
targetm.target_option.valid_attribute_p
returns false and handle that case in multi-versioning?

That is,

+  return !seen_error ();

that looks very wrong to me.

Richard.

> Thanks,
> Martin
>
> gcc/ChangeLog:
>
> 2019-01-16  Martin Liska  
> Richard Biener  
>
> PR middle-end/88587
> * cgraph.h (create_version_clone_with_body): Add new argument
> with attributes.
> * cgraphclones.c (cgraph_node::create_version_clone): Add
> DECL_ATTRIBUTES to a newly created decl.  And call
> valid_attribute_p so that proper cl_target_optimization_node
> is set for the newly created declaration.
> * multiple_target.c (create_target_clone): Set DECL_ATTRIBUTES
> for declaration.
> (expand_target_clones): Do not call valid_attribute_p, it must
> be already done.
> * tree-inline.c (copy_decl_for_dup_finish): Reset mode for
> vector types.
>
> gcc/testsuite/ChangeLog:
>
> 2019-01-16  Martin Liska  
>
> PR middle-end/88587
> * g++.target/i386/pr88587.C: New test.
> * gcc.target/i386/mvc13.c: New test.
> ---
>  gcc/cgraph.h|  7 +-
>  gcc/cgraphclones.c  | 18 +-
>  gcc/multiple_target.c   | 32 -
>  gcc/testsuite/g++.target/i386/pr88587.C | 15 
>  gcc/testsuite/gcc.target/i386/mvc13.c   |  9 +++
>  gcc/tree-inline.c   |  4 
>  6 files changed, 61 insertions(+), 24 deletions(-)
>  create mode 100644 gcc/testsuite/g++.target/i386/pr88587.C
>  create mode 100644 gcc/testsuite/gcc.target/i386/mvc13.c
>
>

Re: [PATCH] Reset proper type on vector types (PR middle-end/88587).

[Richard Biener]

On Wed, Jan 16, 2019 at 12:59 PM Jakub Jelinek  wrote:
>
> On Wed, Jan 16, 2019 at 12:50:23PM +0100, Richard Biener wrote:
> > I guess so.  There's not much we can do about this other than making
> > DECL_MODE dynamic the same way as TYPE_MODE.  I still believe
> > this is the wrong direction and instead RTL expansion should properly
> > adjust DECL_RTL and generally on GIMPLE we shouldn't need to
> > commit to modes early at all...
>
> Are there cases where DECL_MODE (decl) should not be equal to
> TYPE_MODE (TREE_TYPE (decl))?  Is that for structs with flexible array
> members where the initializer is filling up the flexible array member, or
> something different?

ISTR seeing the most differences in FIELD_DECL modes (bitfields?
packed fields getting BLKmode but types not being packed/etc.?)

> E.g. in expand_debug* it shouldn't be hard to use TYPE_MODE if
> VECTOR_TYPE_P (TREE_TYPE (decl)), but there are many other uses of
> DECL_MODE.
>
> Jakub

Re: [PATCH] Reset proper type on vector types (PR middle-end/88587).

[Jakub Jelinek]

On Wed, Jan 16, 2019 at 12:50:23PM +0100, Richard Biener wrote:
> I guess so.  There's not much we can do about this other than making
> DECL_MODE dynamic the same way as TYPE_MODE.  I still believe
> this is the wrong direction and instead RTL expansion should properly
> adjust DECL_RTL and generally on GIMPLE we shouldn't need to
> commit to modes early at all...

Are there cases where DECL_MODE (decl) should not be equal to
TYPE_MODE (TREE_TYPE (decl))?  Is that for structs with flexible array
members where the initializer is filling up the flexible array member, or
something different?

E.g. in expand_debug* it shouldn't be hard to use TYPE_MODE if
VECTOR_TYPE_P (TREE_TYPE (decl)), but there are many other uses of
DECL_MODE.

Jakub

Re: [PATCH] Reset proper type on vector types (PR middle-end/88587).

[Richard Biener]

On Wed, Jan 16, 2019 at 10:26 AM Martin Liška  wrote:
>
> And there's patch with Richi's validation check that he provided.
> It fails on following 2 tests in test-suite:
>
> $ ./xgcc -B. 
> /home/marxin/Programming/gcc/gcc/testsuite/gcc.target/i386/pr68674.c
> DECL_MODE BLK vs TYPE_MODE V8SI [V8SI]: a
> ...
>
> $ ./xgcc -B. 
> /home/marxin/Programming/gcc/gcc/testsuite/gcc.target/i386/pr80583.c -c
> DECL_MODE BLK vs TYPE_MODE V8SI [V8SI]: a
> ...
>
> In both cases we access a global variable from a function with a different 
> target
> attributes. I guess Honza has seen that in inliner.
>
> What to do with these, can it be potentially dangerous?

I guess so.  There's not much we can do about this other than making
DECL_MODE dynamic the same way as TYPE_MODE.  I still believe
this is the wrong direction and instead RTL expansion should properly
adjust DECL_RTL and generally on GIMPLE we shouldn't need to
commit to modes early at all...

[but then we eventually want to expose more ABI details to GIMPLE]

Richard.

>
> Thanks,
> Martin
>

Re: [PATCH] c-family: Update unaligned adress of packed member check

[Jakub Jelinek]

On Mon, Jan 14, 2019 at 10:00:11AM -0800, H.J. Lu wrote:
> On Mon, Jan 14, 2019 at 6:22 AM Jakub Jelinek  wrote:
> >
> > On Sun, Jan 13, 2019 at 06:54:05AM -0800, H.J. Lu wrote:
> > > > What always matters is whether we take address of a packed structure
> > > > field/non-static data member or whether we just read that field.
> > > > The former should be warned about, the latter not.
> > > >
> > >
> > > How about this patch?  It checks if address is taken with NOP.
> >
> > I'd like to first understand the convert_p argument to
> > warn_for_address_or_pointer_of_packed_member.
> >
> > To me it seems you want to emit two different warnings, perhaps one
> > surpressed if the other one is emitted, but you actually from the start
> > decide which of the two you are going to check for.  That is just weird.
> 
> convert_p  is only for C.

Why?  What is so special about C and (implicit?) casts where the rhs isn't
ADDR_EXPR?  Aren't all casts (explicit or implicit) from one pointer type
to another pointer and satisfy the rules something that should be warned
about if the conditions are met?

Jakub

Re: [PATCH] x86: Revert patches to fix PR target/88794

[Wei Xiao]

> > Yes, but please test the compiler after the revert. Please also create
> > a runtime testcase out of the testcase in the PR.
Yes, we have tested it but current runtime testcase can't cover the corner
case to expose the incorrectness of SDM. We will add some after the revert.

> For r267160, I'd expect you want to revert just the config/i386/ part and
> keep the testcases, they should work even with the changes reverted, right?
>
The testcase part also need to be reverted since we have changed them
according to the incorrect intrinsic list in SDM.

Jakub Jelinek  于2019年1月15日周二 下午11:20写道：
>
> On Tue, Jan 15, 2019 at 04:14:06PM +0100, Uros Bizjak wrote:
> > On Tue, Jan 15, 2019 at 3:40 PM Wei Xiao  wrote:
> > >
> > > Hi,
> > >
> > > It turns out that the Intel 64 and IA-32 Architectures Software Developer
> > > Manuals (SDM) description about the fixupimm intrinsic is incorrect. So 
> > > we need
> > > to revert 3 patches related to it: r265827, r266026 and r267160.
> > > Sorry for the inconvenience.
> > >
> > > Is it ok?
> >
> > Yes, but please test the compiler after the revert. Please also create
> > a runtime testcase out of the testcase in the PR.
>
> For r267160, I'd expect you want to revert just the config/i386/ part and
> keep the testcases, they should work even with the changes reverted, right?
>
> Jakub

Re: [PATCH] Check requested alignment in SET_{DECL,TYPE}_ALIGN is pow2_or_zerop before aligning on targets with partial int modes

[Jozef Lawrynowicz]

On Fri, 11 Jan 2019 13:00:22 -0700
Jeff Law  wrote:

> On 12/30/18 4:51 AM, Jozef Lawrynowicz wrote:
> > 
> > The attached patch adds an assertion that SET_{DECL,TYPE}_ALIGN is called 
> > with
> > a value which is a power of 2, or 0, for targets which support a partial int
> > mode. This should catch issues in the future with non-power of 2
> > alignments being set, which could propagate into serious problems later in 
> > the
> > compilation process.
> > 
> Let's defer to gcc-10.
> 
> I'm not sure it's worth trying to conditionalize this on the partial
> integer modes.  Why not just do it all the time?  I'd be surprised if
> it's that expensive.

Yes in hindsight it was overkill to conditionalize the checks.

I'll redo and submit this again with yours and Richard's suggestions for
GCC-10.

Thanks,
Jozef

Re: [PATCH][GCC][Arm] Fix arm big-endian intrinsics regressions.

[Kyrill Tkachov]

Hi Tamar,

On 15/01/19 17:05, Tamar Christina wrote:

Hi All,

We are a bit inconsistent when it comes to lane index endianness on Arm,
we don't seem to always stick to the expected GCC vector extensions index
endianness, for these tests since they are modelled as UNSPEC anyway just
keep the indexes in Arm NEON order.

There are other intrinsics that require an update, but for now these will
fix the new ones.

Bootstrapped Regtested on arm-none-Linux-gnueabihf and no issues.
Cross compiled on armeb-none-eabi and regtested and no issues.
Verified example by hand with execution tests and no issues.

Ok for trunk?



Ok.
We should aim to make the lane-flipping consistent in the future.

Thanks,
Kyrill


Thanks,
Tamar

gcc/ChangeLog:

2019-01-15  Tamar Christina  

* config/arm/arm-protos.h (neon_vcmla_lane_prepare_operands): Remove 
patternmode.
* config/arm/arm.c (neon_vcmla_lane_prepare_operands): Likewise.
* config/arm/neon.md (neon_vcmla_lane, 
neon_vcmla_laneq,
neon_vcmlaq_lane): Remove endianness conversion.

--

Re: PATCH: Add -Waddress-of-packed-member to GCC 9 porting guide

[Martin Liška]

On 1/15/19 5:25 PM, H.J. Lu wrote:
> On Tue, Jan 15, 2019 at 7:05 AM Martin Liška  wrote:
>>
>> On 1/15/19 3:19 PM, H.J. Lu wrote:
>>> On Tue, Jan 15, 2019 at 6:07 AM Martin Liška  wrote:

 On 1/14/19 3:14 PM, H.J. Lu wrote:
> On Mon, Jan 14, 2019 at 5:53 AM Richard Biener
>  wrote:
>>
>> On Mon, Jan 14, 2019 at 2:46 PM H.J. Lu  wrote:
>>>
>>> This patch adds -Waddress-of-packed-member to GCC 9 porting guide.
>>>
>>> OK to install?
>>
>> The docs fail to mention what to do when the unaligned pointer is _not_
>> safe to use.  That is, how do I fix
>>
>> struct { char c; int i[4]; } s __attribute__((packed));
>> int foo()
>> {
>>   int *p = s.i;
>>   return bar (p);
>> }
>> int bar (int *q)
>> {
>>   return *q;
>> }
>>
>> for the cases where eliding the pointer isn't easily possible?
>
> You can't have both packed struct and aligned array at the same time.
> The only thing I can say is "don't do it".
>
>> Please also mention the new warning in changes.html
>> (it seems to be enabled by default even?).
>
> I will add a paragraph.
>
> H.J.
>> IIRC the frontends themselves build "bogus" pointer types
>> to aligned data from a simple [1] because the FIELD_DECLs
>> types are naturally aligned.
>>
>> Richard.
>>
>>> Thanks.
>>>
>>> H.J.
>>> ---
>>> Index: gcc-9/porting_to.html
>>> ===
>>> RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-9/porting_to.html,v
>>> retrieving revision 1.1
>>> diff -u -r1.1 porting_to.html
>>> --- gcc-9/porting_to.html   11 Jan 2019 18:21:45 -  1.1
>>> +++ gcc-9/porting_to.html   14 Jan 2019 13:46:07 -
>>> @@ -56,13 +56,36 @@
>>>}
>>>
>>>
>>> +C/C++ language issues
>>> +
>>> +>> id="Waddress-of-packed-member">-Waddress-of-packed-member
>>> +is enabled by default
>>> +
>>> +
>>> +  When address of packed member of struct or union is taken, it may 
>>> result
>>> +  in an unaligned pointer value.  A new warning
>>> +  -Waddress-of-packed-member was added to check alignment 
>>> at
>>> +  pointer assignment.  It warns both unaligned address and unaligned
>>> +  pointer.
>>> +
>>> +
>>> +
>>> +  If the pointer value is safe to use, you can suppress
>>> +  -Waddress-of-packed-member warnings by using pragmas:
>>> +
>>> +  
>>> +#pragma GCC diagnostic push
>>> +#pragma GCC diagnostic ignored "-Waddress-of-packed-member"
>>> +/* (code for which the warning is to be disabled)  */
>>> +#pragma GCC diagnostic pop
>>> +  
>>> +
>>>  
>>>
>>>  
>>>
>>>  >>>
 It presents the new warning, run-time memory layout dump and also 
 sanitizer error.

 Thoughts?
>>>
>>> This doesn't help port to GCC 9.
>>>
>>>
>>
>> But it can still go into changes as example of a code
>> for which the warning is triggered.
>>
>> Thoughts?
> 
> How about this?
> 

I would then at least mention what's the natural alignment of type that's 
reported
in the warning.

Martin

Fortran vector math header

[Martin Liška]

On 1/15/19 6:45 PM, Joseph Myers wrote:
> On Mon, 14 Jan 2019, Martin Liška wrote:
> 
>> Thanks for review, fixed that in updated version of the patch.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
> 
> This patch is OK.
> 

Great, thank you very much for advises you provided.

Now we should add the header file into glibc, I kicked a discussion here:
https://sourceware.org/ml/libc-help/2018-11/msg00015.html

Joseph: will you please help with a patch for glibc?

I'm also planning to document the header file in gfortran so that other
compiler can leverage that.

Thanks,
Martin

[PATCH, rs6000, testsuite] Fix PR87306

[Kewen.Lin]

Hi,

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87306, it's older POWER 
hardware specific test case issue. On POWER7 and earlier, we implicitly 
set flag -mno-allow-movmisalign which disables vectorization when to 
vectorize the code requires that misaligned loads and stores. For POWER8
and upper, the opposite value is implicitly set(-mallow-movmisalign),
then the vectorization performs as expected.

Similar to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65484, the proposed 
fix is to make dg-final checks whether vect_hw_misalign support on powerpc,
only further verify the dump pattern when it's on to consitent to the 
behavior.

Tested on trunk for POWER8 LE and POWER7 BE systems.
Is this ok for trunk?

---

 gcc/testsuite/ChangeLog

2019-01-16  Kewen Lin  

PR target/87306
* gcc.dg/vect/bb-slp-pow-1.c : Modify to reflect that the loop is not
vectorized on POWER unless hardware misaligned loads are available.

diff --git a/gcc/testsuite/gcc.dg/vect/bb-slp-pow-1.c 
b/gcc/testsuite/gcc.dg/vect/bb-slp-pow-1.c
index 5a05bd4..6742e12 100644
--- a/gcc/testsuite/gcc.dg/vect/bb-slp-pow-1.c
+++ b/gcc/testsuite/gcc.dg/vect/bb-slp-pow-1.c
@@ -25,4 +25,8 @@ main (void)
   return 0;
 }

-/* { dg-final { scan-tree-dump-times "basic block vectorized" 1 "slp2" } } */
+/* On older powerpc hardware (POWER7 and earlier), the default flag
+   -mno-allow-movmisalign prevents vectorization.  On POWER8 and later,
+   when vect_hw_misalign is true, vectorization occurs. */
+
+/* { dg-final { scan-tree-dump-times "basic block vectorized" 1 "slp2" { 
target {{ ! powerpc*-*-* } || { powerpc*-*-* && vect_hw_misalign }} } } } */

Re: [PATCH] Reset proper type on vector types (PR middle-end/88587).

[Martin Liška]

And there's patch with Richi's validation check that he provided.
It fails on following 2 tests in test-suite:

$ ./xgcc -B. 
/home/marxin/Programming/gcc/gcc/testsuite/gcc.target/i386/pr68674.c
DECL_MODE BLK vs TYPE_MODE V8SI [V8SI]: a
...

$ ./xgcc -B. 
/home/marxin/Programming/gcc/gcc/testsuite/gcc.target/i386/pr80583.c -c
DECL_MODE BLK vs TYPE_MODE V8SI [V8SI]: a
...

In both cases we access a global variable from a function with a different 
target
attributes. I guess Honza has seen that in inliner.

What to do with these, can it be potentially dangerous?

Thanks,
Martin

>From bf22210f96a2ead9d50e351102f095aadf9d879f Mon Sep 17 00:00:00 2001
From: marxin 
Date: Wed, 16 Jan 2019 09:05:21 +0100
Subject: [PATCH] Sanitize about VECTOR_TYPEs.

---
 gcc/tree-cfg.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 6041f4208b0..fc1dd36fbb7 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -5108,6 +5108,17 @@ verify_node_sharing_1 (tree *tp, int *walk_subtrees, void *data)
 {
   hash_set *visited = (hash_set *) data;
 
+  if (DECL_P (*tp)
+  && VECTOR_TYPE_P (TREE_TYPE (*tp))
+  && DECL_MODE (*tp) != TYPE_MODE (TREE_TYPE (*tp)))
+{
+  fprintf (stderr, "DECL_MODE %s vs TYPE_MODE %s [%s]: ",
+  mode_name[DECL_MODE (*tp)],
+  mode_name[TYPE_MODE (TREE_TYPE (*tp))],
+  mode_name[TYPE_MODE_RAW (TREE_TYPE (*tp))]);
+  print_generic_expr (stderr, *tp);
+  fprintf (stderr, "\n");
+}
   if (tree_node_can_be_shared (*tp))
 {
   *walk_subtrees = false;
-- 
2.20.1

[PATCH] Reset proper type on vector types (PR middle-end/88587).

[Martin Liška]

Hi.

The patch is about resetting TYPE_MODE of vector types. This is problematic
when an inlining among different ISAs happen. Then we end up with a different
mode than when it's expected from debug info.

When creating a new function decl in target_clones, we must valid_attribute_p 
early
so that the declaration has a proper cl_target_.. node and so that inliner can
fix modes.

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

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2019-01-16  Martin Liska  
Richard Biener  

PR middle-end/88587
* cgraph.h (create_version_clone_with_body): Add new argument
with attributes.
* cgraphclones.c (cgraph_node::create_version_clone): Add
DECL_ATTRIBUTES to a newly created decl.  And call
valid_attribute_p so that proper cl_target_optimization_node
is set for the newly created declaration.
* multiple_target.c (create_target_clone): Set DECL_ATTRIBUTES
for declaration.
(expand_target_clones): Do not call valid_attribute_p, it must
be already done.
* tree-inline.c (copy_decl_for_dup_finish): Reset mode for
vector types.

gcc/testsuite/ChangeLog:

2019-01-16  Martin Liska  

PR middle-end/88587
* g++.target/i386/pr88587.C: New test.
* gcc.target/i386/mvc13.c: New test.
---
 gcc/cgraph.h|  7 +-
 gcc/cgraphclones.c  | 18 +-
 gcc/multiple_target.c   | 32 -
 gcc/testsuite/g++.target/i386/pr88587.C | 15 
 gcc/testsuite/gcc.target/i386/mvc13.c   |  9 +++
 gcc/tree-inline.c   |  4 
 6 files changed, 61 insertions(+), 24 deletions(-)
 create mode 100644 gcc/testsuite/g++.target/i386/pr88587.C
 create mode 100644 gcc/testsuite/gcc.target/i386/mvc13.c


diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index c016da3875c..bb231833328 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -1019,12 +1019,17 @@ public:
  If non-NULL BLOCK_TO_COPY determine what basic blocks to copy.
  If non_NULL NEW_ENTRY determine new entry BB of the clone.
 
+ If TARGET_ATTRIBUTES is non-null, when creating a new declaration,
+ add the attributes to DECL_ATTRIBUTES.  And call valid_attribute_p
+ that will promote value of the attribute DECL_FUNCTION_SPECIFIC_TARGET
+ of the declaration.
+
  Return the new version's cgraph node.  */
   cgraph_node *create_version_clone_with_body
 (vec redirect_callers,
  vec *tree_map, bitmap args_to_skip,
  bool skip_return, bitmap bbs_to_copy, basic_block new_entry_block,
- const char *clone_name);
+ const char *clone_name, tree target_attributes = NULL_TREE);
 
   /* Insert a new cgraph_function_version_info node into cgraph_fnver_htab
  corresponding to cgraph_node.  */
diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c
index 4688de91cfe..f1c0351584d 100644
--- a/gcc/cgraphclones.c
+++ b/gcc/cgraphclones.c
@@ -1012,6 +1012,11 @@ cgraph_node::create_version_clone (tree new_decl,
If non-NULL BLOCK_TO_COPY determine what basic blocks to copy.
If non_NULL NEW_ENTRY determine new entry BB of the clone.
 
+   If TARGET_ATTRIBUTES is non-null, when creating a new declaration,
+   add the attributes to DECL_ATTRIBUTES.  And call valid_attribute_p
+   that will promote value of the attribute DECL_FUNCTION_SPECIFIC_TARGET
+   of the declaration.
+
Return the new version's cgraph node.  */
 
 cgraph_node *
@@ -1019,7 +1024,7 @@ cgraph_node::create_version_clone_with_body
   (vec redirect_callers,
vec *tree_map, bitmap args_to_skip,
bool skip_return, bitmap bbs_to_copy, basic_block new_entry_block,
-   const char *suffix)
+   const char *suffix, tree target_attributes)
 {
   tree old_decl = decl;
   cgraph_node *new_version_node = NULL;
@@ -1044,6 +1049,17 @@ cgraph_node::create_version_clone_with_body
 
   DECL_VIRTUAL_P (new_decl) = 0;
 
+  if (target_attributes)
+{
+  DECL_ATTRIBUTES (new_decl) = target_attributes;
+
+  location_t saved_loc = input_location;
+  tree v = TREE_VALUE (target_attributes);
+  input_location = DECL_SOURCE_LOCATION (new_decl);
+  targetm.target_option.valid_attribute_p (new_decl, NULL, v, 0);
+  input_location = saved_loc;
+}
+
   /* When the old decl was a con-/destructor make sure the clone isn't.  */
   DECL_STATIC_CONSTRUCTOR (new_decl) = 0;
   DECL_STATIC_DESTRUCTOR (new_decl) = 0;
diff --git a/gcc/multiple_target.c b/gcc/multiple_target.c
index 589d059424a..fcec0f4676d 100644
--- a/gcc/multiple_target.c
+++ b/gcc/multiple_target.c
@@ -294,7 +294,8 @@ create_new_asm_name (char *old_asm_name, char *new_asm_name)
 /*  Creates target clone of NODE.  */
 
 static cgraph_node *
-create_target_clone (cgraph_node *node, bool definition, char *name)
+create_target_clone (cgraph_node *node, bool definition, char *name,
+		 tree attributes)
 {
   

Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI

[Richard Sandiford]

Steve Ellcey  writes:
> Here are the failures I am getting with this patch:
>
> c-c++-common/gomp/pr63328.c
> gcc.dg/gomp/pr87895-2.c
>
> These tests include another test (which passes) and the included tests
> have a dg-warning check.  For some reason the dg-warning in the include
> is ignored

Yeah, that's expected.  Only dg- markup in the test file itself
is relevant.

> and when I tried adding one in the main file (that includes
> the other test), that didn't work either.

I suggest for now we add:

/* { dg-excess-errors "partial simd clone support" { target { aarch64*-*-* } } 
}  */

> gcc.dg/gomp/simd-clones-1.c
> g++.dg/gomp/declare-simd-1.C
>
> These two tests are generating an ICE and I am not sure why.
>
> I cut declare-simd-1.C down to:
>
> #pragma omp declare simd simdlen (2) aligned (b : sizeof (long long) * 2)
> __extension__ long long
> f10 (long long *b)
> {
>   return *b;
> }
>
> And it results in:
>
> % install/usr/bin/g++ -fopenmp-simd -c b1.C
> during RTL pass: expand
> b1.C: In function ‘long long int f10(long long int*)’:
> b1.C:5:15: internal compiler error: in expand_assignment, at expr.c:5101
> 5 |   return *b;
>   |   ^
> 0xaeee73 expand_assignment(tree_node*, tree_node*, bool)
>   /home/sellcey/gcc-vect/src/gcc/gcc/expr.c:5101
> 0x9aeecf expand_gimple_stmt_1
>   /home/sellcey/gcc-vect/src/gcc/gcc/cfgexpand.c:3746
> 0x9aeecf expand_gimple_stmt
>   /home/sellcey/gcc-vect/src/gcc/gcc/cfgexpand.c:3844
> 0x9b682f expand_gimple_basic_block
>   /home/sellcey/gcc-vect/src/gcc/gcc/cfgexpand.c:5880
> 0x9b90a7 execute
>   /home/sellcey/gcc-vect/src/gcc/gcc/cfgexpand.c:6503
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See  for instructions.

I haven't checked whether this is the reason, but:

> +  clonei->vecsize_mangle = 'n';
> +  clonei->mask_mode = VOIDmode;
> +  clonei->vecsize_int = (num == 0) ? 64 :128;
> +  clonei->vecsize_float = (num == 0) ? 64 :128;
> +  if (clonei->simdlen == 0)
> +{
> +  if (SCALAR_INT_MODE_P (TYPE_MODE (base_type)))
> + clonei->simdlen = clonei->vecsize_int;
> +  else
> + clonei->simdlen = clonei->vecsize_float;
> +  clonei->simdlen /= GET_MODE_BITSIZE (SCALAR_TYPE_MODE (base_type));
> +  return 2;
> +}
> +
> +  /* Restrict ourselves to vectors that fit in a single register  */
> +
> +  gcc_assert (tree_fits_shwi_p (TYPE_SIZE (base_type)));
> +  vsize = clonei->simdlen * tree_to_shwi (TYPE_SIZE (base_type));
> +  if (vsize != 64 && vsize != 128)
> +{
> +  warning_at (DECL_SOURCE_LOCATION (node->decl), 0,
> +   "GCC does not currently support simdlen %d for type %qT",
> +   clonei->simdlen, base_type);
> +  return 0;
> +}
> +  return 2;
> +}

...this doesn't handle explicit simdlen correctly.  The vecsize_int and
vecsize_float need to be set from the user's simdlen, with num only
making a difference for the default simdlen.  And there should only
be 1 size for an explicit simdlen.

Maybe this would be more obvious if we have something like:

  unsigned int elt_bits = GET_MODE_BITSIZE (SCALAR_TYPE_MODE (base_type));
  unsigned int vec_bits, count;
  if (clone->simdlen == 0)
{
  count = 2;
  vec_bits = (num == 0 ? 64 : 128);
  clone->simdlen = vec_bits / elt_bits;
}
  else
{
  count = 1;
  vec_bits = clone->simdlen * elt_bits;
  if (vec_bits != 64 && vec_bits != 128)
{
  ...warning...
  return 0;
}
}
  clone->vecsize_int = vec_bits;
  clone->vecsize_float = vec_bits;
  return count;

Thanks,
Richard

[PATCH] Add myself to MAINTAINERS

[Kewen.Lin]

Hi,

this adds myself to MAINTAINERS in the Write After Approval section.

Committed to trunk as r267965.

---

Index: ChangeLog
===
--- ChangeLog   (revision 267964)
+++ ChangeLog   (working copy)
@@ -1,3 +1,7 @@
+2019-01-16  Kewen Lin  
+
+   * MAINTAINERS (Write After Approval): Add myself.
+
  2019-01-16  Xiong Hu Luo 

  * MAINTAINERS (Write After Approval): Add myself.
Index: MAINTAINERS
===
--- MAINTAINERS (revision 267964)
+++ MAINTAINERS (working copy)
@@ -471,6 +471,7 @@ Ilya Leoshkevich

 Kriang Lerdsuwanakij   
 Renlin Li  
 Xinliang David Li  
+Kewen Lin  
 Chen Liqin 
 Jiangning Liu  
 Sa Liu