Re: [Patch, fortran] PR88929 - ICE on building MPICH 3.2 with GCC 9 with ISO_Fortran_binding

[Paul Richard Thomas]

Hi Steve,

Fixed in revision 268231.

This was a copy/paste/modify with the type built in. Have corrected both.
> >   tree
> > + gfc_conv_descriptor_elem_len (tree desc)
> > + {
> > +   tree tmp;
> > +   tree dtype;
> > +
> > +   dtype = gfc_conv_descriptor_dtype (desc);
> > +   tmp = gfc_advance_chain (TYPE_FIELDS (TREE_TYPE (dtype)),
> > +GFC_DTYPE_ELEM_LEN);
> > +   gcc_assert (tmp!= NULL_TREE
>
> space after tmp

All the other corrections were made.

Thanks for the review.

Paul

Re: [PATCH] Refine -Waddress-of-packed-member once more

[Jakub Jelinek]

On Thu, Jan 24, 2019 at 06:39:22AM +, Bernd Edlinger wrote:
> --- gcc/c-family/c-warn.c (revision 268195)
> +++ gcc/c-family/c-warn.c (working copy)
> @@ -2725,14 +2725,18 @@ static tree
>  check_address_or_pointer_of_packed_member (tree type, tree rhs)
>  {
>bool rvalue = true;
> +  bool indirect = false;
>  
>if (INDIRECT_REF_P (rhs))
> -rhs = TREE_OPERAND (rhs, 0);
> +{
> +  rhs = TREE_OPERAND (rhs, 0);
> +  indirect = true;
> +}
>  
>if (TREE_CODE (rhs) == ADDR_EXPR)
>  {
>rhs = TREE_OPERAND (rhs, 0);
> -  rvalue = false;
> +  rvalue = indirect;
>  }

Given the unfolded *&, I wonder if the above actually shouldn't be
a loop with indirection integral counter instead of a boolean
and simply bump the indirection count on INDIRECT_REF_P and decrease on
ADDR_EXPR, and if indirection count is > 0 after the loop return NULL_TREE?
Say for *&*& or similar, and ***var.field always not warning etc.

Otherwise the patch looks good.

Though you might throw in a few test lines with the intermixed casts and
compound exprs multiple times (why you've added correctly the loop in
there).

Jakub

Re: [PATCH] Refine -Waddress-of-packed-member once more

[Bernd Edlinger]

On 1/23/19 4:22 PM, Jakub Jelinek wrote:
> On Tue, Jan 22, 2019 at 02:10:38PM +, Bernd Edlinger wrote:
>> --- gcc/c-family/c-warn.c(revision 268119)
>> +++ gcc/c-family/c-warn.c(working copy)
>> @@ -2796,6 +2796,10 @@ check_address_or_pointer_of_packed_membe
>>if (context)
>>  break;
>>  }
>> +  if (TREE_CODE (TREE_TYPE (rhs)) == ARRAY_TYPE)
>> +rvalue = false;
>> +  if (rvalue)
>> +return NULL_TREE;
> 
> That looks like ARRAY_REF specific stuff, isn't it?  We have various other
> handled components, does e.g. REALPART_EXPR/IMAGPART_EXPR need that?
> What about VIEW_CONVERT_EXPR?  Or is that something you want to do for
> all of them?  In any case, there should be testcases with _Complex and
> __real__/__imag__, address of that as well as value.
> 
>> @@ -52,4 +54,6 @@ void foo (void)
>>i1 = t0.d; /* { dg-warning "may result in an unaligned 
>> pointer value" } */
>>i1 = t1->d;/* { dg-warning "may result in an unaligned 
>> pointer value" } */
>>i1 = t10[0].d; /* { dg-warning "may result in an unaligned 
>> pointer value" } */
>> +  i1 = (int*) [0].e[0];  /* { dg-warning "may result in an unaligned 
>> pointer value" } */
>> +  i1 = (int*) t10[0].e;  /* { dg-warning "may result in an unaligned 
>> pointer value" } */
> 
> Why not also i1 = [0].e[0];
> and i1 = t10[0].e; tests?
> 
> Unrelated to this patch, I'm worried e.g. about
>   if (INDIRECT_REF_P (rhs))
> rhs = TREE_OPERAND (rhs, 0);
>
> at the start of the check_address_or_pointer_of_packed_member
> function, what if we have:
>   i1 = *
> Do we want to warn when in the end we don't care if the address is unaligned
> or not, this is folded eventually into t0.c.
> 
> Plus as I said earlier to H.J., I think
>   bool nop_p;
> 
>   while (TREE_CODE (rhs) == COMPOUND_EXPR)
> rhs = TREE_OPERAND (rhs, 1);
> 
>   tree orig_rhs = rhs;
>   STRIP_NOPS (rhs);
>   nop_p = orig_rhs != rhs;
> should be really:
>   bool nop_p;
>   tree orig_rhs = rhs;
>   STRIP_NOPS (rhs);
>   nop_p = orig_rhs != rhs;
> 
>   while (TREE_CODE (rhs) == COMPOUND_EXPR)
> rhs = TREE_OPERAND (rhs, 1);
> 
>   orig_rhs = rhs;
>   STRIP_NOPS (rhs);
>   nop_p |= orig_rhs != rhs;
> 
> or similar, because if there are casts around COMPOUND_EXPR, it doesn't than
> look through those COMPOUND_EXPRs, and if e.g. in the COMPOUND_EXPR rhs
> there is then COND_EXPR or similar, it should handle that case.
> 

Okay, I updated the patch to address your comments.

Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.
Index: gcc/c-family/c-warn.c
===
--- gcc/c-family/c-warn.c	(revision 268195)
+++ gcc/c-family/c-warn.c	(working copy)
@@ -2725,14 +2725,18 @@ static tree
 check_address_or_pointer_of_packed_member (tree type, tree rhs)
 {
   bool rvalue = true;
+  bool indirect = false;
 
   if (INDIRECT_REF_P (rhs))
-rhs = TREE_OPERAND (rhs, 0);
+{
+  rhs = TREE_OPERAND (rhs, 0);
+  indirect = true;
+}
 
   if (TREE_CODE (rhs) == ADDR_EXPR)
 {
   rhs = TREE_OPERAND (rhs, 0);
-  rvalue = false;
+  rvalue = indirect;
 }
 
   if (!POINTER_TYPE_P (type))
@@ -2796,6 +2800,10 @@ check_address_or_pointer_of_packed_member (tree ty
 	  if (context)
 	break;
 	}
+  if (TREE_CODE (TREE_TYPE (rhs)) == ARRAY_TYPE)
+	rvalue = false;
+  if (rvalue)
+	return NULL_TREE;
   rhs = TREE_OPERAND (rhs, 0);
 }
 
@@ -2811,15 +2819,19 @@ check_address_or_pointer_of_packed_member (tree ty
 static void
 check_and_warn_address_or_pointer_of_packed_member (tree type, tree rhs)
 {
-  bool nop_p;
+  bool nop_p = false;
+  tree orig_rhs;
 
-  while (TREE_CODE (rhs) == COMPOUND_EXPR)
-rhs = TREE_OPERAND (rhs, 1);
+  do
+{
+  while (TREE_CODE (rhs) == COMPOUND_EXPR)
+	rhs = TREE_OPERAND (rhs, 1);
+  orig_rhs = rhs;
+  STRIP_NOPS (rhs);
+  nop_p |= orig_rhs != rhs;
+}
+  while (orig_rhs != rhs);
 
-  tree orig_rhs = rhs;
-  STRIP_NOPS (rhs);
-  nop_p = orig_rhs != rhs;
-
   if (TREE_CODE (rhs) == COND_EXPR)
 {
   /* Check the THEN path.  */
Index: gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c
===
--- gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c	(revision 268195)
+++ gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c	(working copy)
@@ -6,6 +6,8 @@ struct t {
   int b;
   int *c;
   int d[10];
+  int *e[1];
+  _Complex float f;
 } __attribute__((packed));
 
 struct t t0;
@@ -17,6 +19,8 @@ struct t *bar();
 struct t (*baz())[10];
 struct t (*bazz())[10][10];
 int *i1;
+int **i2;
+float f0, *f1;
 __UINTPTR_TYPE__ u1;
 __UINTPTR_TYPE__ baa();
 
@@ -40,6 +44,13 @@ void foo (void)
   i1 = t10[0].c;   /* { dg-bogus "may result in an unaligned pointer value" } */
   u1 = (__UINTPTR_TYPE__)  /* { dg-bogus "may result in an unaligned 

libgo patch committed: Install SIGURG handler in c-archive mode

[Ian Lance Taylor]

This patch by Cherry Zhang changes the libgo runtime to install a
SIGURG handler in c-archive mode.  The precise stack scan uses SIGURG
to trigger a stack scan, so we need to have Go signal handler
installed for SIGURG.  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 268158)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-e3271f3e09337b951822ba5c596d8cfe3b94508e
+d67c4bf0c42b79d54925ba8c5f23278ee6c3efb6
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/go/runtime/signal_unix.go
===
--- libgo/go/runtime/signal_unix.go (revision 268158)
+++ libgo/go/runtime/signal_unix.go (working copy)
@@ -142,8 +142,8 @@ func sigInstallGoHandler(sig uint32) boo
}
 
// When built using c-archive or c-shared, only install signal
-   // handlers for synchronous signals and SIGPIPE.
-   if (isarchive || islibrary) && t.flags&_SigPanic == 0 && sig != 
_SIGPIPE {
+   // handlers for synchronous signals, SIGPIPE, and SIGURG.
+   if (isarchive || islibrary) && t.flags&_SigPanic == 0 && sig != 
_SIGPIPE && sig != _SIGURG {
return false
}
 

Re: [PATCH] Fix an ICE when compiling Go with LTO (PR go/89019)

[Ian Lance Taylor]

On Wed, Jan 23, 2019 at 5:18 PM Nikhil Benesch  wrote:
>
> 2018-01-23  Nikhil Benesch  
>
> PR go/89019
> * go-gcc.cc (Gcc_backend::placeholder_struct_type): Mark
> placeholder structs as requiring structural equality.
> (Gcc_backend::set_placeholder_pointer_type): Propagate
> the canonical type from the referenced type to the pointer type.
> * lib/go-torture.exp: Test compiling with -flto.

This is OK.

Thanks.

Ian

Re: [C++ PATCH] [PR87770] test partial specializations for type dependence

[Alexandre Oliva]

On Jan 21, 2019, Jason Merrill  wrote:

> "does this have its own template arguments, not just the ones from its
> enclosing class?"

> Perhaps compare the number of levels of template arguments of the
> function to that of its enclosing context?

Is this the logic you had in mind?  Or can we assume DECL_P to always be
false at that point, because if the context has template info it must be
a class?  (I'm not sure about the context of generic lambdas)

Any suggestion of a good name for the inline function (or would you
prefer it to be a macro?) that tests whether a decl satisfies this
predicate?  primary_or_partial_spec_p?

Thanks,


@@ -25622,7 +25622,17 @@ type_dependent_expression_p (tree expression)
 that come from the template-id; the template arguments for the
 enclosing class do not make it type-dependent unless they are used in
 the type of the decl.  */
-  if (PRIMARY_TEMPLATE_P (DECL_TI_TEMPLATE (expression))
+  if ((PRIMARY_TEMPLATE_P (DECL_TI_TEMPLATE (expression))
+  || (DECL_TEMPLATE_SPECIALIZATION (DECL_TI_TEMPLATE (expression))
+  && (DECL_P (DECL_CONTEXT (expression))
+  ? (!DECL_TI_TEMPLATE (DECL_CONTEXT (expression))
+ || (TMPL_ARGS_DEPTH (DECL_TI_ARGS (expression))
+ > TMPL_ARGS_DEPTH (DECL_TI_ARGS
+(DECL_CONTEXT (expression)
+  : (!CLASSTYPE_TI_TEMPLATE (DECL_CONTEXT (expression))
+ || (TMPL_ARGS_DEPTH (DECL_TI_ARGS (expression))
+ > TMPL_ARGS_DEPTH (CLASSTYPE_TI_ARGS
+(DECL_CONTEXT (expression
  && (any_dependent_template_arguments_p
  (INNERMOST_TEMPLATE_ARGS (DECL_TI_ARGS (expression)
return true;

-- 
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

[PATCH 2/2] fix comments typo.

[luoxhu]

From: Xiong Hu Luo 

commited in 268229.
---
gcc/ChangeLog

2019-01-24  Xiong Hu Luo  

* tree-ssa-dom.c (test_for_singularity): fix a comment typo.
* vr-values.c (find_case_label_ranges): fix a comment typo.
---
 gcc/tree-ssa-dom.c | 2 +-
 gcc/vr-values.c| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
index 458f711..12647e7 100644
--- a/gcc/tree-ssa-dom.c
+++ b/gcc/tree-ssa-dom.c
@@ -1929,7 +1929,7 @@ test_for_singularity (gimple *stmt, gcond *dummy_cond,
 
3- Very simple redundant store elimination is performed.
 
-   4- We can simpify a condition to a constant or from a relational
+   4- We can simplify a condition to a constant or from a relational
   condition to an equality condition.  */
 
 edge
diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index f4058ea..a734ef9 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -2597,7 +2597,7 @@ find_case_label_ranges (gswitch *stmt, value_range *vr, 
size_t *min_idx1,
 
   take_default = !find_case_label_range (stmt, min, max, , );
 
-  /* Set second range to emtpy.  */
+  /* Set second range to empty.  */
   *min_idx2 = 1;
   *max_idx2 = 0;
 
-- 
2.7.4

[PATCH 1/2] fix tab alignment issue.

[luoxhu]

From: Xiong Hu Luo 

commited in r268228.
---
ChangeLog

2019-01-24  Xiong Hu Luo  
* ChangeLog: replace space with tab.
* MAINTAINERS: delete 1 tab to keep alignment.
---
 ChangeLog   | 4 ++--
 MAINTAINERS | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 60ff3e0..8a5d078 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -21,9 +21,9 @@
 
* MAINTAINERS (Write After Approval): Add myself.
 
- 2019-01-16  Xiong Hu Luo 
+2019-01-16  Xiong Hu Luo 
 
- * MAINTAINERS (Write After Approval): Add myself.
+   * MAINTAINERS (Write After Approval): Add myself.
 
 2019-01-03  Rainer Orth  
 
diff --git a/MAINTAINERS b/MAINTAINERS
index 860ba32..0c362aa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -484,7 +484,7 @@ Manuel López-Ibáñez 

 Carl Love  
 Martin v. Löwis

 H.J. Lu
-Xiong Hu Luo   

+Xiong Hu Luo   
 Christophe Lyon
 Luis Machado   
 Ziga Mahkovec  
-- 
2.7.4

[PATCH] Fix an ICE when compiling Go with LTO (PR go/89019)

[Nikhil Benesch]

This patch fixes an ICE I reported earlier today as PR go/89019, which
occurs when compiling sufficiently complicated Go code with link-time
optimization (i.e., -flto) enabled.

Both of these simple test programs are sufficient to trigger the ICE:

$ cat crash1.go
package main

type fcanary func()

$ cat crash2.go
package main

func f() {
var canary func()
func() {
canary()
}()
}

The cause appears to be that the propagation of structural equality
requirements is mishandled during the handoff from the Go frontend to
the middle-end. Pointers to types that required structural equality
were not always marked as requiring structural equality themselves.
Remarkably, the only code that notices the mistake is the free_lang_data
IPA pass, which is only active when LTO is enabled.

The enclosed patch fixes the issue and provides test coverage by adding
a subtest to the Go torture test suite that compiles with -flto.
Bootstrapped and ran Go test suite on x86_64-pc-linux-gnu.

2018-01-23  Nikhil Benesch  

PR go/89019
* go-gcc.cc (Gcc_backend::placeholder_struct_type): Mark
placeholder structs as requiring structural equality.
(Gcc_backend::set_placeholder_pointer_type): Propagate
the canonical type from the referenced type to the pointer type.
* lib/go-torture.exp: Test compiling with -flto.

diff --git a/gcc/go/go-gcc.cc b/gcc/go/go-gcc.cc
index 7fbdd074119..4e9e0e3026a 100644
--- a/gcc/go/go-gcc.cc
+++ b/gcc/go/go-gcc.cc
@@ -1049,6 +1049,7 @@ Gcc_backend::set_placeholder_pointer_type(Btype* 
placeholder,
 }
   gcc_assert(TREE_CODE(tt) == POINTER_TYPE);
   TREE_TYPE(pt) = TREE_TYPE(tt);
+  TYPE_CANONICAL(pt) = TYPE_CANONICAL(tt);
   if (TYPE_NAME(pt) != NULL_TREE)
 {
   // Build the data structure gcc wants to see for a typedef.
@@ -1080,6 +1081,12 @@ Gcc_backend::placeholder_struct_type(const std::string& 
name,
 get_identifier_from_string(name),
 ret);
   TYPE_NAME(ret) = decl;
+
+  // The struct type that eventually replaces this placeholder will require
+  // structural equality. The placeholder must too, so that the requirement
+  // for structural equality propagates to references that are constructed
+  // before the replacement occurs.
+  SET_TYPE_STRUCTURAL_EQUALITY(ret);
 }
   return this->make_type(ret);
 }
diff --git a/gcc/testsuite/lib/go-torture.exp b/gcc/testsuite/lib/go-torture.exp
index 213711e41df..a7eca184416 100644
--- a/gcc/testsuite/lib/go-torture.exp
+++ b/gcc/testsuite/lib/go-torture.exp
@@ -34,7 +34,8 @@ if ![info exists TORTURE_OPTIONS] {
{ -O2 -fomit-frame-pointer -finline-functions -funroll-loops } \
{ -O2 -fbounds-check } \
{ -O3 -g } \
-   { -Os }]
+   { -Os } \
+   { -flto }]
 }
 
 

RE: [PATCH] Implement P0966 std::string::reserve should not shrink

[Andrew Luo]

Here's an updated diff.  Still a work in progress, ran the tests for 
strings/capacity in both modes, passing now.  I am running all the tests in 
both modes as well, but will take a while to get results.  In the meanwhile I 
thought I'd send this out...

Also, that code works on all released versions of libc++.  Only in the past 
month or so was that overload added, also for a (slightly flawed) 
implementation of P0966.  Anyways, I removed the #ifs - I get your point that 
there's no need to preserve backwards compatibility with non-comforming code, 
and I think any issues with reserve specifically will be rather rare (and easy 
to fix).  (Although, I should point out that push_back is not overloaded if you 
select -std=c++98, only -std=c++11 or later, but that's besides the point).

Thanks,

-Andrew

-Original Message-
From: Jonathan Wakely  
Sent: Wednesday, January 23, 2019 3:26 AM
To: Andrew Luo 
Cc: libstd...@gcc.gnu.org
Subject: Re: [PATCH] Implement P0966 std::string::reserve should not shrink

On 23/01/19 00:44 +, Andrew Luo wrote:
>Thanks for the input.  I will make the suggested changes.
>
>I didn't add a changelog entry yet (which I was planning to do - just wanted 
>to send this out before to get some feedback on the code first).

Understood, I'm just making sure you're aware of the process.

>I will also take care of the copyright assignment (should I send an email to 
>g...@gcc.gnu.org or are you the maintainer that is taking care of this 
>contribution?)...

That will probably be me. I'll send you the form to start the copyright 
assignment.

>Next patch I will send to both libstdc++ and gcc-patches as well.  Somehow I 
>missed that.

Thanks.

>One possibility to avoid changing the behavior for pre-C++2a code is to put 
>the check in a "#if __cplusplus > 201703L" block and force reserve(size_type) 
>to be inline (if we want to go this route, I think the helper function 
>_M_request_capacity would have to stay, as reserve could shrink in C++17 mode 
>or earlier). 

>Other than that, with my current patch, reserve() still works as previously, 
>however reserve(0) has changed (I don't know how common it was to use 
>reserve(0) instead of simply reserve())...

Well as you noted, your patch made it a no-op (more below).


>As for the split of reserve - wouldn't this kind of code break in previous 
>-std modes:
>
>auto f(&::std::string::reserve); // Ambiguous post C++17

Already undefined, as you noted in the later reply. More on that below.


>As for deprecation, I will add it to the no-arg overload.  Regarding giving 
>the deprecation notice for all dialects, technically speaking, it was only 
>deprecated in C++2a so should someone receive that deprecation message if they 
>are compiling with -std=c++98, for example?  I don't know what the general 
>policies around this are for GCC/libstdc++.

Yes, you're right it's only deprecated for C++2a. My thinking was that if it's 
going away at some point in future then it's useful to get a warning saying so, 
period.

But we don't do that for std::auto_ptr in C++98 even though it's deprecated in 
C++11. So maybe only add the attribute for C++2a, which means it can use C++11 
attribute syntax:

#if__cplusplus > 201703L
[[deprecated("use shrink_to_fit() instead")]] #endif


On 23/01/19 03:41 +, Andrew Luo wrote:
>Actually, reserve() doesn't currently work as reserve(0) previously, I 
>missed a line there (should have shrink_to_fit() in the body...)

Oh, I missed that the proposal says the new overload is a shrink-to-fit request 
rather than no-op.

Calling shrink_to_fit() won't work for C++98 mode, because it isn't declared. 
It could be a no-op for C++98 (it's only a non-binding request after all), or 
we could add a new _M_shrink function that
shrink_to_fit() and reserve() call.  I'm still reluctant to add a new exported 
function (which means four new symbols) for a deprecated feature that already 
exists as shrink_to_fit() but maybe that's the best option.

>Also, how do you run the tests for the COW string?  I ran make check in the 
>libstdc++-v3 folder and everything passes...

make check RUNTESTFLAGS=--target_board=unix/-D_GLIBCXX_USE_CXX11_ABI=0

And you can add other options, e.g. to check in C++98 mode:

RUNTESTFLAGS=--target_board=unix/-D_GLIBCXX_USE_CXX11_ABI=0/-std=gnu++98

See https://gcc.gnu.org/onlinedocs/libstdc++/manual/test.html#test.run
for more info.

On 23/01/19 06:17 +, Andrew Luo wrote:
>Minor correction, someone kindly pointed out to me elsewhere that this code:
>
>auto f(&::std::string::reserve); // Ambiguous post C++17
>
>was never conforming to the C++ standard in the first place... as splitting 
>the members was legal pre-C++2a regardless.

Right.

>That said, that piece of (non-conforming) code did work before (at least in 
>MSVC + Dinkumware, I didn't check G++ + libstdc++ just yet, but unless G++ has 
>some special enforcement of this in the compiler, it would seem to "work" from 
>the library 

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

[Joseph Myers]

On Wed, 23 Jan 2019, Martin Sebor wrote:

> Ping: https://gcc.gnu.org/ml/gcc-patches/2019-01/msg00969.html

This patch is OK.

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

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

[Martin Sebor]

Ping: https://gcc.gnu.org/ml/gcc-patches/2019-01/msg00969.html

This patch fixes a couple of P2 regressions.  It was first submitted
last summer and mostly reviewed by Jeff but then slipped through
the cracks.  Can someone pick it up in Jeff's absence?

On 1/16/19 5:41 PM, Martin Sebor wrote:

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. 

Re: [EXT] Re: Fortran vector math header

[Steve Ellcey]

On Wed, 2019-01-23 at 23:53 +0100, Jakub Jelinek wrote:
> External Email
> 
> ---
> ---
> On Wed, Jan 23, 2019 at 09:56:21PM +, Steve Ellcey wrote:
> > I wonder if we even need to pass a string to the target
> > hook.  Instead
> > of:
> > 
> > !GCC$ builtin (cos) attributes simd (notinbranch) if('x86_64-linux-gnu')
> > 
> > We just have:
> > 
> > !GCC$ builtin (cos) attributes simd (notinbranch) if_allowed
> 
> That is not possible.  The whole point why we have a header is that glibc
> as the library provider provides a header which says to gcc which builtins
> have simd entrypoints, for which target and for which multilib.
> 
> If GCC knew or wanted to hardcode that info, we wouldn't have a header, we'd
> just hardcode it.
> The point is that at some later point, glibc might get an implementation for
> say powerpc64 64-bit, or mips n32, or whatever else, say for a subset of the
> builtins x86_64 has, or superset, ...

Duh.  Of course.  I am not sure how I lost track of that fact.

Steve Ellcey
sell...@marvell.com

Re: Fortran vector math header

[Jakub Jelinek]

On Wed, Jan 23, 2019 at 09:56:21PM +, Steve Ellcey wrote:
> I wonder if we even need to pass a string to the target hook.  Instead
> of:
> 
> !GCC$ builtin (cos) attributes simd (notinbranch) if('x86_64-linux-gnu')
> 
> We just have:
> 
> !GCC$ builtin (cos) attributes simd (notinbranch) if_allowed

That is not possible.  The whole point why we have a header is that glibc
as the library provider provides a header which says to gcc which builtins
have simd entrypoints, for which target and for which multilib.

If GCC knew or wanted to hardcode that info, we wouldn't have a header, we'd
just hardcode it.
The point is that at some later point, glibc might get an implementation for
say powerpc64 64-bit, or mips n32, or whatever else, say for a subset of the
builtins x86_64 has, or superset, ...

Jakub

Re: Fortran vector math header

[Steve Ellcey]

On Tue, 2019-01-22 at 13:18 +0100, Richard Biener wrote:
> 
> Yeah, I would even suggest to use a target hook multilib_ABI_active_p
> (const char *)
> for this ... (where the hook should diagnose unknown multilib specifiers).
> 
> Richard.


I wonder if we even need to pass a string to the target hook.  Instead
of:

!GCC$ builtin (cos) attributes simd (notinbranch) if('x86_64-linux-gnu')

We just have:

!GCC$ builtin (cos) attributes simd (notinbranch) if_allowed

When we see if_allowed we call a target function like multilib_ABI_active_p
and it can use whatever criteria it wants to use to determine if the simd
attribute should be honored.

Or, if we are going this far, how about leaving out the if altogher and
everytime we see a simd attribute in Fortran we call a target function
to determine if it should or should not be honored.  The default function
can just return true, target specific versions could look at the ABI and
return true or false as needed.

It might also be worth passing the function (cos) as an argument to the
target function, then we could have different ABI's enabling different
functions and not have to have them all on or all off based on the ABI.

Steve Ellcey
sell...@marvell.com

Re: [Patch, fortran] PR88929 - ICE on building MPICH 3.2 with GCC 9 with ISO_Fortran_binding

[Steve Kargl]

On Wed, Jan 23, 2019 at 07:43:48PM +, Paul Richard Thomas wrote:
> 
> Bootstrapped and regtested on FC28/x86_64 - OK for trunk?
> 

Yes with minor fixes.


> Index: gcc/fortran/trans-array.c
> ===
> *** gcc/fortran/trans-array.c (revision 268193)
> --- gcc/fortran/trans-array.c (working copy)
> *** gfc_conv_descriptor_rank (tree desc)
> *** 293,298 
> --- 293,314 
>   

Can you put a brief comment here that describes what the
function is doing?

>   tree
> + gfc_conv_descriptor_elem_len (tree desc)
> + {
> +   tree tmp;
> +   tree dtype;
> + 
> +   dtype = gfc_conv_descriptor_dtype (desc);
> +   tmp = gfc_advance_chain (TYPE_FIELDS (TREE_TYPE (dtype)),
> +GFC_DTYPE_ELEM_LEN);
> +   gcc_assert (tmp!= NULL_TREE

space after tmp

> *** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
> *** 4950,4958 
> /* All the temporary descriptors are marked as DECL_ARTIFICIAL. If
>the expression type is different from the descriptor type, then
>the offset must be found (eg. to a component ref or substring)
> !  and the dtype updated.  */
> !   type = gfc_typenode_for_spec (>ts);
> !   if (DECL_ARTIFICIAL (parmse->expr)
> && type != gfc_get_element_type (TREE_TYPE (parmse->expr)))
>   {
> /* Obtain the offset to the data.  */
> --- 4952,4965 
> /* All the temporary descriptors are marked as DECL_ARTIFICIAL. If
>the expression type is different from the descriptor type, then
>the offset must be found (eg. to a component ref or substring)
> !  and the dtype updated.  Assumed type entities are only allowed
> !  to be dummies in fortran. They therefore lack the  decl specific
> !  appendiges and so must be treated differently from other fortran
> !  entities passed to CFI descriptors in the interface decl.  */

fortran is normally spelled as Fortran.
Extra space in "lack the  decl"

> + 
> +   /* Intent in requires a temporary for the data. Assumed types do not
> +  work with the standard temporary generation schemes. */

I would prefer INTENT(IN) here.  Your call.

> +   if (e->expr_type == EXPR_VARIABLE && fsym->attr.intent == INTENT_IN)
> + {

-- 
Steve

Re: [EXT] Re: [Patch] PR target/85711 - Fix ICE on aarch64

[Richard Sandiford]

Steve Ellcey  writes:
> On Wed, 2019-01-23 at 16:54 +, Richard Sandiford wrote:
>> 
>> IMO we shouldn't remove the assert.  See:
>> 
>>   https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01969.html
>> 
>> and the thread leading up to it.
>> 
>> Thanks,
>> Richard
>
> OK, I hadn't seen that thread.  I didn't see any patch submitted

Yeah, I kept meaning to get round to it but never did...

> in response to your comment there so I created a new patch.
> This patch leaves the assert in aarch64.c and changes the check
> for the 'p' constraint in constain_operands, this version
> fixes the pr84682-2.c test failure and causes no regressions
> on aarch64 or x86.

Great!

> Steve Ellcey
> sell...@marvell.com
>
>
> 2019-01-23  Bin Cheng  
>   Steve Ellcey 
>
>   PR target/85711
>   * recog.c (address_operand): Return false on wrong mode for address.
>   (constrain_operands): Check for mode with 'p' constraint.

OK, thanks.

Richard

Re: [patch, fortran] Move some array packing to front end

[Thomas Koenig]

Hi,

for the record, the attached version of the patch regtests cleanly and
also passes the test that Dominique pointed out.

I will defer this until stage 1 reopens.

Regards

Thomas
Index: fortran/expr.c
===
--- fortran/expr.c	(Revision 268104)
+++ fortran/expr.c	(Arbeitskopie)
@@ -5582,6 +5582,9 @@ gfc_is_simply_contiguous (gfc_expr *expr, bool str
   gfc_ref *ref, *part_ref = NULL;
   gfc_symbol *sym;
 
+  if (expr->expr_type == EXPR_ARRAY)
+return true;
+
   if (expr->expr_type == EXPR_FUNCTION)
 {
   if (expr->value.function.esym)
Index: fortran/trans-array.c
===
--- fortran/trans-array.c	(Revision 268104)
+++ fortran/trans-array.c	(Arbeitskopie)
@@ -7755,6 +7755,23 @@ array_parameter_size (tree desc, gfc_expr *expr, t
 			   *size, fold_convert (gfc_array_index_type, elem));
 }
 
+/* Helper function - return true if the argument is a pointer.  */
+ 
+static bool
+is_pointer (gfc_expr *e)
+{
+  gfc_symbol *sym;
+
+  if (e->expr_type != EXPR_VARIABLE ||  e->symtree == NULL)
+return false;
+
+  sym = e->symtree->n.sym;
+  if (sym == NULL)
+return false;
+
+  return sym->attr.pointer || sym->attr.proc_pointer;
+}
+
 /* Convert an array for passing as an actual parameter.  */
 
 void
@@ -8006,6 +8023,19 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr *
 			 "Creating array temporary at %L", >where);
 	}
 
+  /* When optmizing, we can use gfc_conv_subref_array_arg for
+	 making the packing and unpacking operation visible to the
+	 optimizers.  */
+
+  if (g77 && optimize && !optimize_size && expr->expr_type == EXPR_VARIABLE
+	  && !is_pointer (expr))
+	{
+	  gfc_conv_subref_array_arg (se, expr, g77,
+ fsym ? fsym->attr.intent : INTENT_INOUT,
+ false, fsym, proc_name, sym);
+	  return;
+	}
+
   ptr = build_call_expr_loc (input_location,
 			 gfor_fndecl_in_pack, 1, desc);
 
Index: fortran/trans-expr.c
===
--- fortran/trans-expr.c	(Revision 268104)
+++ fortran/trans-expr.c	(Arbeitskopie)
@@ -4535,8 +4535,10 @@ gfc_apply_interface_mapping (gfc_interface_mapping
an actual argument derived type array is copied and then returned
after the function call.  */
 void
-gfc_conv_subref_array_arg (gfc_se * parmse, gfc_expr * expr, int g77,
-			   sym_intent intent, bool formal_ptr)
+gfc_conv_subref_array_arg (gfc_se *se, gfc_expr * expr, int g77,
+			   sym_intent intent, bool formal_ptr,
+			   const gfc_symbol *fsym, const char *proc_name,
+			   gfc_symbol *sym)
 {
   gfc_se lse;
   gfc_se rse;
@@ -4553,7 +4555,37 @@ void
   stmtblock_t body;
   int n;
   int dimen;
+  gfc_se work_se;
+  gfc_se *parmse;
+  bool pass_optional;
 
+  pass_optional = fsym && fsym->attr.optional && sym && sym->attr.optional;
+
+  if (pass_optional)
+{
+  gfc_init_se (_se, NULL);
+  parmse = _se;
+}
+  else
+parmse = se;
+
+  if (gfc_option.rtcheck & GFC_RTCHECK_ARRAY_TEMPS)
+{
+  /* We will create a temporary array, so let us warn.  */
+  char * msg;
+
+  if (fsym && proc_name)
+	msg = xasprintf ("An array temporary was created for argument "
+			 "'%s' of procedure '%s'", fsym->name, proc_name);
+  else
+	msg = xasprintf ("An array temporary was created");
+
+  tmp = build_int_cst (logical_type_node, 1);
+  gfc_trans_runtime_check (false, true, tmp, >pre,
+			   >where, msg);
+  free (msg);
+}
+
   gfc_init_se (, NULL);
   gfc_init_se (, NULL);
 
@@ -4807,6 +4839,27 @@ class_array_fcn:
   else
 parmse->expr = gfc_build_addr_expr (NULL_TREE, parmse->expr);
 
+  /* Wrap the above in "if (present(x))" if needed.  */
+
+  if (pass_optional)
+{
+  tree present;
+  tree type;
+  tree parmse_expr;
+  stmtblock_t block;
+
+  type = TREE_TYPE (parmse->expr);
+  gfc_start_block ();
+  gfc_add_block_to_block (, >pre);
+  gfc_add_block_to_block (, >post);
+  parmse_expr = gfc_finish_block ();
+
+  present = gfc_conv_expr_present (sym);
+  tmp = fold_build3_loc (input_location, COND_EXPR, type, present,
+			 parmse_expr, build_int_cst (type, 0));
+  se->expr = tmp;
+}
+
   return;
 }
 
Index: fortran/trans.h
===
--- fortran/trans.h	(Revision 268104)
+++ fortran/trans.h	(Arbeitskopie)
@@ -529,7 +529,10 @@ int gfc_is_intrinsic_libcall (gfc_expr *);
 int gfc_conv_procedure_call (gfc_se *, gfc_symbol *, gfc_actual_arglist *,
 			 gfc_expr *, vec *);
 
-void gfc_conv_subref_array_arg (gfc_se *, gfc_expr *, int, sym_intent, bool);
+void gfc_conv_subref_array_arg (gfc_se *, gfc_expr *, int, sym_intent, bool,
+const gfc_symbol *fsym = NULL,
+const char *proc_name = NULL,
+gfc_symbol *sym = NULL);
 
 /* Generate code for a scalar assignment.  */
 tree gfc_trans_scalar_assign (gfc_se *, gfc_se 

Re: [EXT] Re: [Patch] PR target/85711 - Fix ICE on aarch64

[Steve Ellcey]

On Wed, 2019-01-23 at 16:54 +, Richard Sandiford wrote:
> 
> IMO we shouldn't remove the assert.  See:
> 
>   https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01969.html
> 
> and the thread leading up to it.
> 
> Thanks,
> Richard

OK, I hadn't seen that thread.  I didn't see any patch submitted
in response to your comment there so I created a new patch.
This patch leaves the assert in aarch64.c and changes the check
for the 'p' constraint in constain_operands, this version
fixes the pr84682-2.c test failure and causes no regressions
on aarch64 or x86.

Steve Ellcey
sell...@marvell.com


2019-01-23  Bin Cheng  
Steve Ellcey 

PR target/85711
* recog.c (address_operand): Return false on wrong mode for address.
(constrain_operands): Check for mode with 'p' constraint.


diff --git a/gcc/recog.c b/gcc/recog.c
index d0c498fced2..a9f584bc0dc 100644
--- a/gcc/recog.c
+++ b/gcc/recog.c
@@ -1070,6 +1070,11 @@ general_operand (rtx op, machine_mode mode)
 int
 address_operand (rtx op, machine_mode mode)
 {
+  /* Wrong mode for an address expr.  */
+  if (GET_MODE (op) != VOIDmode
+  && ! SCALAR_INT_MODE_P (GET_MODE (op)))
+return false;
+
   return memory_address_p (mode, op);
 }
 
@@ -2696,10 +2701,13 @@ constrain_operands (int strict, alternative_mask alternatives)
 		/* p is used for address_operands.  When we are called by
 		   gen_reload, no one will have checked that the address is
 		   strictly valid, i.e., that all pseudos requiring hard regs
-		   have gotten them.  */
-		if (strict <= 0
-		|| (strict_memory_address_p (recog_data.operand_mode[opno],
-		 op)))
+		   have gotten them.  We also want to make sure we have a
+		   valid mode.  */
+		if ((GET_MODE (op) == VOIDmode
+		 || SCALAR_INT_MODE_P (GET_MODE (op)))
+		&& (strict <= 0
+			|| (strict_memory_address_p
+			 (recog_data.operand_mode[opno], op
 		  win = 1;
 		break;
 

Re: [PATCH] libgcc2.c: Correct DI/TI -> SF/DF conversions

[H.J. Lu]

On Wed, Jan 23, 2019 at 12:50 PM Joseph Myers  wrote:
>
> On Wed, 23 Jan 2019, H.J. Lu wrote:
>
> > +  fesetround (FE_DOWNWARD);
> > +  float fs = s128;
> > +  if (fs != -0x1p+127)
> > +abort ();
> > +  double ds = s128;
> > +  if (ds != -0x1p+127)
> > +abort ();
>
> This definitely needs #ifdef FE_DOWNWARD; even just limited to glibc
> configurations, there are soft-float 64-bit configurations with no support
> for rounding modes other than to-nearest.  Likewise for the other tests
> using rounding modes other than FE_TONEAREST.  OK with that change.
>
> It's possible it will turn out a new effective-target is needed for fenv.h
> support, if there are still configurations people are testing with that
> lack fenv.h at all.
>

This is the patch I am checking in.

Thanks.

-- 
H.J.
From ecb5e6357cff988a6e34aed324f13b777af82deb Mon Sep 17 00:00:00 2001
From: "H.J. Lu" 
Date: Tue, 22 Jan 2019 18:55:35 -0800
Subject: [PATCH] libgcc2.c: Correct DI/TI -> SF/DF conversions

FSTYPE FUNC (DWtype u) in libgcc2.c, which converts DI/TI to SF/DF, has

  /* No leading bits means u == minimum.  */
  if (count == 0)
return -(Wtype_MAXp1_F * (Wtype_MAXp1_F / 2));

in the third case (where actually count == 0 only means the high part is
minimum).  It should be:

  /* No leading bits means u == minimum.  */
  if (count == 0)
return Wtype_MAXp1_F * (FSTYPE) (hi | ((UWtype) u != 0));

instead.

gcc/testsuite/

2019-01-23  H.J. Lu  

	PR libgcc/88931
	* gcc.dg/torture/fp-int-convert-timode-1.c: New test.
	* gcc.dg/torture/fp-int-convert-timode-2.c: Likewise.
	* gcc.dg/torture/fp-int-convert-timode-3.c: Likewise.
	* gcc.dg/torture/fp-int-convert-timode-4.c: Likewise.

libgcc/

2019-01-23  Joseph Myers  

	PR libgcc/88931
	* libgcc2.c (FSTYPE FUNC (DWtype u)): Correct no leading bits
	case.
---
 .../gcc.dg/torture/fp-int-convert-timode-1.c  | 25 +
 .../gcc.dg/torture/fp-int-convert-timode-2.c  | 27 +++
 .../gcc.dg/torture/fp-int-convert-timode-3.c  | 27 +++
 .../gcc.dg/torture/fp-int-convert-timode-4.c  | 27 +++
 libgcc/libgcc2.c  |  2 +-
 5 files changed, 107 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-1.c
 create mode 100644 gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-2.c
 create mode 100644 gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-3.c
 create mode 100644 gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-4.c

diff --git a/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-1.c b/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-1.c
new file mode 100644
index 000..d6454fada72
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-1.c
@@ -0,0 +1,25 @@
+/* Test for correct rounding of conversions from __int128 to
+   float.  */
+/* { dg-do run } */
+/* { dg-require-effective-target int128 } */
+/* { dg-options "-frounding-math" } */
+
+#include 
+#include 
+
+int
+main (void)
+{
+  volatile unsigned long long h = 0x8000LL;
+  volatile unsigned long long l = 0xdLL;
+  volatile unsigned __int128 u128 = (((unsigned __int128) h) << 64) | l;
+  volatile __int128 s128 = u128;
+  fesetround (FE_TONEAREST);
+  float fs = s128;
+  if (fs != -0x1p+127)
+abort ();
+  double ds = s128;
+  if (ds != -0x1p+127)
+abort ();
+  exit (0);
+}
diff --git a/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-2.c b/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-2.c
new file mode 100644
index 000..dbfa481b4fb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-2.c
@@ -0,0 +1,27 @@
+/* Test for correct rounding of conversions from __int128 to
+   float.  */
+/* { dg-do run } */
+/* { dg-require-effective-target int128 } */
+/* { dg-options "-frounding-math" } */
+
+#include 
+#include 
+
+int
+main (void)
+{
+#ifdef FE_DOWNWARD
+  volatile unsigned long long h = 0x8000LL;
+  volatile unsigned long long l = 0xdLL;
+  volatile unsigned __int128 u128 = (((unsigned __int128) h) << 64) | l;
+  volatile __int128 s128 = u128;
+  fesetround (FE_DOWNWARD);
+  float fs = s128;
+  if (fs != -0x1p+127)
+abort ();
+  double ds = s128;
+  if (ds != -0x1p+127)
+abort ();
+#endif
+  exit (0);
+}
diff --git a/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-3.c b/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-3.c
new file mode 100644
index 000..63a305ec3c2
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-3.c
@@ -0,0 +1,27 @@
+/* Test for correct rounding of conversions from __int128 to
+   float.  */
+/* { dg-do run } */
+/* { dg-require-effective-target int128 } */
+/* { dg-options "-frounding-math" } */
+
+#include 
+#include 
+
+int
+main (void)
+{
+#ifdef FE_UPWARD
+  volatile unsigned long long h = 0x8000LL;
+  volatile unsigned long long l = 0xdLL;
+  volatile unsigned __int128 u128 = (((unsigned __int128) h) << 64) | l;
+  volatile __int128 s128 

Re: [PATCH, i386] Fix PR 88998, bad codegen with mmx instructions

[H.J. Lu]

On Wed, Jan 23, 2019 at 12:27 PM Uros Bizjak  wrote:
>
> On Wed, Jan 23, 2019 at 8:52 PM H.J. Lu  wrote:
> >
> > On Wed, Jan 23, 2019 at 11:22 AM Uros Bizjak  wrote:
> > >
> > > Attached patch adds SSE alternatives to sse2_cvtpi2pd, sse2_cvtpd2pi
> > > and sse2_cvttpd2pi to avoid MMX registers when e.g. _mm_cvtepi32_pd
> > > intrinsics is used. Without the patch, the testcase compiles to (-O2
> > > -mavx):
> > >
> > > _Z7prepareii:
> > > vmovd   %edi, %xmm1
> > > vpinsrd $1, %esi, %xmm1, %xmm0
> > > movdq2q %xmm0, %mm0
> > > cvtpi2pd%mm0, %xmm0
> > > vhaddpd %xmm0, %xmm0, %xmm0
> > > ret
> > >
> > > while patched gcc generates:
> > >
> > > vmovd   %edi, %xmm1
> > > vpinsrd $1, %esi, %xmm1, %xmm0
> > > vcvtdq2pd   %xmm0, %xmm0
> > > vhaddpd %xmm0, %xmm0, %xmm0
> > > ret
> > >
> > > The later avoids transition of FPU to MMX mode.
> > >
> >
> > Is that possible to support 64-bit vectors, like V2SI, with SSE
> > instead of MMX for x86-64 under a command-line switch?
>
> SSE registers are preferred for 64bit vectors (see number of
> exclamation marks in *mov_internal in mmx.md), so the value will
> be passed in SSE regs unless there is pure MMX instruction, where due
> to missing SSE alternatives, RA will need to allocate MMX register.
>

[hjl@gnu-cfl-1 v64-1]$ cat x.i
typedef float __v2sf __attribute__ ((__vector_size__ (8)));

extern __v2sf z;

void
add (__v2sf x, __v2sf y)
{
  z = x + y;
}
[hjl@gnu-cfl-1 v64-1]$ 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
[hjl@gnu-cfl-1 v64-1]$ cat x.s
.file "x.i"
.text
.p2align 4
.globl add
.type add, @function
add:
.LFB0:
.cfi_startproc
movlps %xmm0, -32(%rsp)
movss -32(%rsp), %xmm0
movlps %xmm1, -40(%rsp)
addss -40(%rsp), %xmm0
movss %xmm0, -56(%rsp)
movss -28(%rsp), %xmm0
addss -36(%rsp), %xmm0
movss %xmm0, -52(%rsp)
movq -56(%rsp), %rax
movq %rax, z(%rip)
ret
.cfi_endproc
.LFE0:
.size add, .-add
.ident "GCC: (GNU) 9.0.0 20190122 (experimental)"
.section .note.GNU-stack,"",@progbits
[hjl@gnu-cfl-1 v64-1]$

I am expecting:

addps %xmm1, %xmm0
movlps %xmm0, z(%rip)
retq

-- 
H.J.

Re: [PATCH] libgcc2.c: Correct DI/TI -> SF/DF conversions

[Joseph Myers]

On Wed, 23 Jan 2019, H.J. Lu wrote:

> +  fesetround (FE_DOWNWARD);
> +  float fs = s128;
> +  if (fs != -0x1p+127)
> +abort ();
> +  double ds = s128;
> +  if (ds != -0x1p+127)
> +abort ();

This definitely needs #ifdef FE_DOWNWARD; even just limited to glibc 
configurations, there are soft-float 64-bit configurations with no support 
for rounding modes other than to-nearest.  Likewise for the other tests 
using rounding modes other than FE_TONEAREST.  OK with that change.

It's possible it will turn out a new effective-target is needed for fenv.h 
support, if there are still configurations people are testing with that 
lack fenv.h at all.

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

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

[Jason Merrill]

On Wed, Jan 23, 2019 at 12:57 PM Marek Polacek  wrote:
>
> On Wed, Jan 23, 2019 at 09:00:36AM -0500, Jason Merrill wrote:
> > I was talking about digest_init, not reshape_init.  digest_init calls
> > convert_for_initialization.
>
> /facepalm
>
> So yes, digest_init calls convert_for_initialization which will end up
> calling perform_implicit_conversion_flags which could call convert_like_real
> where the narrowing warnings are given, but it doesn't, we go to this case:
>
>   else if (processing_template_decl && conv->kind != ck_identity)
> {
>   /* In a template, we are only concerned about determining the
>  type of non-dependent expressions, so we do not have to
>  perform the actual conversion.  But for initializers, we
>  need to be able to perform it at instantiation
>  (or instantiate_non_dependent_expr) time.  */
>   expr = build1 (IMPLICIT_CONV_EXPR, type, expr);
>
> finish_decltype_type throws away the expression because it's not dependent, 
> and
> only uses its type.  So narrowing remains undetected.  Not sure if I should 
> mess
> with perform_implicit_conversion_flags.

Let's try that; this is a situation where the comment is incorrect.
Perhaps just call check_narrowing here if appropriate, rather than go
through the whole conversion machinery.

Jason

Re: [PATCH, i386] Fix PR 88998, bad codegen with mmx instructions

[Uros Bizjak]

On Wed, Jan 23, 2019 at 8:52 PM H.J. Lu  wrote:
>
> On Wed, Jan 23, 2019 at 11:22 AM Uros Bizjak  wrote:
> >
> > Attached patch adds SSE alternatives to sse2_cvtpi2pd, sse2_cvtpd2pi
> > and sse2_cvttpd2pi to avoid MMX registers when e.g. _mm_cvtepi32_pd
> > intrinsics is used. Without the patch, the testcase compiles to (-O2
> > -mavx):
> >
> > _Z7prepareii:
> > vmovd   %edi, %xmm1
> > vpinsrd $1, %esi, %xmm1, %xmm0
> > movdq2q %xmm0, %mm0
> > cvtpi2pd%mm0, %xmm0
> > vhaddpd %xmm0, %xmm0, %xmm0
> > ret
> >
> > while patched gcc generates:
> >
> > vmovd   %edi, %xmm1
> > vpinsrd $1, %esi, %xmm1, %xmm0
> > vcvtdq2pd   %xmm0, %xmm0
> > vhaddpd %xmm0, %xmm0, %xmm0
> > ret
> >
> > The later avoids transition of FPU to MMX mode.
> >
>
> Is that possible to support 64-bit vectors, like V2SI, with SSE
> instead of MMX for x86-64 under a command-line switch?

SSE registers are preferred for 64bit vectors (see number of
exclamation marks in *mov_internal in mmx.md), so the value will
be passed in SSE regs unless there is pure MMX instruction, where due
to missing SSE alternatives, RA will need to allocate MMX register.

Uros.

Re: [PATCH, i386] Fix PR 88998, bad codegen with mmx instructions

[H.J. Lu]

On Wed, Jan 23, 2019 at 11:22 AM Uros Bizjak  wrote:
>
> Attached patch adds SSE alternatives to sse2_cvtpi2pd, sse2_cvtpd2pi
> and sse2_cvttpd2pi to avoid MMX registers when e.g. _mm_cvtepi32_pd
> intrinsics is used. Without the patch, the testcase compiles to (-O2
> -mavx):
>
> _Z7prepareii:
> vmovd   %edi, %xmm1
> vpinsrd $1, %esi, %xmm1, %xmm0
> movdq2q %xmm0, %mm0
> cvtpi2pd%mm0, %xmm0
> vhaddpd %xmm0, %xmm0, %xmm0
> ret
>
> while patched gcc generates:
>
> vmovd   %edi, %xmm1
> vpinsrd $1, %esi, %xmm1, %xmm0
> vcvtdq2pd   %xmm0, %xmm0
> vhaddpd %xmm0, %xmm0, %xmm0
> ret
>
> The later avoids transition of FPU to MMX mode.
>

Is that possible to support 64-bit vectors, like V2SI, with SSE
instead of MMX for x86-64 under a command-line switch?

-- 
H.J.

[testsuite, rfa] Add gthreads dependency to some failing libstdc++ tests

[Sandra Loosemore]

I ran libstdc++ tests on nios2-elf target.  I observed several new tests 
failing with


error: 'mutex' in namespace 'std' does not name a type

The definition of class mutex in include/bits/std_mutex.h is guarded 
with "#ifdef _GLIBCXX_HAS_GTHREADS" so I assume these tests are not 
supposed to work on this target.  This patch adds the equivalent 
dependency to the failing tests.


OK to commit?  (I guess it is possible that this is actually a bug in 
the code instead, and the tests are supposed to pass)


-Sandra
2019-01-23  Sandra Loosemore  

	libstdc++-v3/
	* testsuite/experimental/net/execution_context/use_service.cc,
	testsuite/experimental/net/headers.cc,
	testsuite/experimental/net/internet/address/v4/comparisons.cc,
	testsuite/experimental/net/internet/address/v4/cons.cc,
	testsuite/experimental/net/internet/address/v4/creation.cc,
	testsuite/experimental/net/internet/address/v4/members.cc,
	testsuite/experimental/net/internet/resolver/base.cc,
	testsuite/experimental/net/internet/resolver/ops/lookup.cc,
	testsuite/experimental/net/internet/resolver/ops/reverse.cc,
	testsuite/experimental/net/timer/waitable/cons.cc,
	testsuite/experimental/net/timer/waitable/dest.cc,
	testsuite/experimental/net/timer/waitable/ops.cc: Require gthreads.

diff --git a/libstdc++-v3/testsuite/experimental/net/execution_context/use_service.cc b/libstdc++-v3/testsuite/experimental/net/execution_context/use_service.cc
index 5b30870..67929a4 100644
--- a/libstdc++-v3/testsuite/experimental/net/execution_context/use_service.cc
+++ b/libstdc++-v3/testsuite/experimental/net/execution_context/use_service.cc
@@ -16,6 +16,7 @@
 // .
 
 // { dg-do run { target c++14 } }
+// { dg-require-gthreads "" }
 
 #include 
 #include 
diff --git a/libstdc++-v3/testsuite/experimental/net/headers.cc b/libstdc++-v3/testsuite/experimental/net/headers.cc
index 1705d2d..959ce0d 100644
--- a/libstdc++-v3/testsuite/experimental/net/headers.cc
+++ b/libstdc++-v3/testsuite/experimental/net/headers.cc
@@ -16,6 +16,7 @@
 // .
 
 // { dg-do compile }
+// { dg-require-gthreads "" }
 
 #include 
 
diff --git a/libstdc++-v3/testsuite/experimental/net/internet/address/v4/comparisons.cc b/libstdc++-v3/testsuite/experimental/net/internet/address/v4/comparisons.cc
index 83a8bab..83359ec 100644
--- a/libstdc++-v3/testsuite/experimental/net/internet/address/v4/comparisons.cc
+++ b/libstdc++-v3/testsuite/experimental/net/internet/address/v4/comparisons.cc
@@ -17,6 +17,7 @@
 
 // { dg-do run { target c++14 } }
 // { dg-add-options net_ts }
+// { dg-require-gthreads "" }
 
 #include 
 #include 
diff --git a/libstdc++-v3/testsuite/experimental/net/internet/address/v4/cons.cc b/libstdc++-v3/testsuite/experimental/net/internet/address/v4/cons.cc
index 1577480..65f3100 100644
--- a/libstdc++-v3/testsuite/experimental/net/internet/address/v4/cons.cc
+++ b/libstdc++-v3/testsuite/experimental/net/internet/address/v4/cons.cc
@@ -17,6 +17,7 @@
 
 // { dg-do run { target c++14 } }
 // { dg-add-options net_ts }
+// { dg-require-gthreads "" }
 
 #include 
 #include 
diff --git a/libstdc++-v3/testsuite/experimental/net/internet/address/v4/creation.cc b/libstdc++-v3/testsuite/experimental/net/internet/address/v4/creation.cc
index 5919845..1a933b9 100644
--- a/libstdc++-v3/testsuite/experimental/net/internet/address/v4/creation.cc
+++ b/libstdc++-v3/testsuite/experimental/net/internet/address/v4/creation.cc
@@ -17,6 +17,7 @@
 
 // { dg-do run { target c++14 } }
 // { dg-add-options net_ts }
+// { dg-require-gthreads "" }
 
 #include 
 #include 
diff --git a/libstdc++-v3/testsuite/experimental/net/internet/address/v4/members.cc b/libstdc++-v3/testsuite/experimental/net/internet/address/v4/members.cc
index 37ca8c8..2d71581 100644
--- a/libstdc++-v3/testsuite/experimental/net/internet/address/v4/members.cc
+++ b/libstdc++-v3/testsuite/experimental/net/internet/address/v4/members.cc
@@ -17,6 +17,7 @@
 
 // { dg-do run { target c++14 } }
 // { dg-add-options net_ts }
+// { dg-require-gthreads "" }
 
 #include 
 #include 
diff --git a/libstdc++-v3/testsuite/experimental/net/internet/resolver/base.cc b/libstdc++-v3/testsuite/experimental/net/internet/resolver/base.cc
index 746557a..f9bea5f 100644
--- a/libstdc++-v3/testsuite/experimental/net/internet/resolver/base.cc
+++ b/libstdc++-v3/testsuite/experimental/net/internet/resolver/base.cc
@@ -17,6 +17,7 @@
 
 // { dg-do run { target c++14 } }
 // { dg-add-options net_ts }
+// { dg-require-gthreads "" }
 
 #include 
 #include 
diff --git a/libstdc++-v3/testsuite/experimental/net/internet/resolver/ops/lookup.cc b/libstdc++-v3/testsuite/experimental/net/internet/resolver/ops/lookup.cc
index 39fb7fd..40cb3db 100644
--- a/libstdc++-v3/testsuite/experimental/net/internet/resolver/ops/lookup.cc
+++ b/libstdc++-v3/testsuite/experimental/net/internet/resolver/ops/lookup.cc
@@ -17,6 +17,7 @@
 
 // { dg-do run { target c++14 } }
 // { dg-add-options net_ts }
+// { dg-require-gthreads 

[Patch, fortran] PR88929 - ICE on building MPICH 3.2 with GCC 9 with ISO_Fortran_binding

[Paul Richard Thomas]

The attached patch allows MPICH 3.2 to build correctly and to test successfully.

Two problems were addressed:
(i) The original implementation of ISO_Fortran_binding did not take
account of the possibility that assumed rank/assumed type arrays could
be passed as dummy arguments. This necessitated the e->rank condition
being changed to e->rank !=0 since assumed rank entities have rank ==
-1 in gfc_exprs.
(ii) Intent in requires that a copy be made of the data to be passed
to the C procedure. This is now implemented.

The testcase provides two interfaces to the C-procedure; one with
intent in and the other with intent inout. The C procedure changes the
data and so this is detected in the inout case but not in  the intent
in case. For both, the C procedure checks that the sum over the array
is correct.

>From the point of view of the release, this is completely safe since
the patch is isolated to the ISO_Fortran_binding interface, which is
newly introduced and has no effect on the rest of the testsuite. I
have bumped the testcase number by 1 to allow for a corrected version
of the withdrawn patch for the test of the errors from the CFI API
functions. I will return to this as soon as I can.

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

Paul

2019-01-23  Paul Thomas  

PR fortran/88929
* trans-array.c (gfc_conv_descriptor_elem_len): New function.
* trans-array.h : Add prototype for above.
* trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Take account of
assumed rank arrays being flagged by rank = -1 in expressions.
Intent in arrays need a pointer to a copy of the data to be
assigned to the descriptor passed for conversion. This should
then be freed, together with the CFI descriptor on return from
the C call.

2019-01-23  Paul Thomas  

PR fortran/88929
* gfortran.dg/ISO_Fortran_binding_3.f90 : New test
* gfortran.dg/ISO_Fortran_binding_3.c : Subsidiary source.
Index: gcc/fortran/trans-array.c
===
*** gcc/fortran/trans-array.c	(revision 268193)
--- gcc/fortran/trans-array.c	(working copy)
*** gfc_conv_descriptor_rank (tree desc)
*** 293,298 
--- 293,314 
  
  
  tree
+ gfc_conv_descriptor_elem_len (tree desc)
+ {
+   tree tmp;
+   tree dtype;
+ 
+   dtype = gfc_conv_descriptor_dtype (desc);
+   tmp = gfc_advance_chain (TYPE_FIELDS (TREE_TYPE (dtype)),
+ 			   GFC_DTYPE_ELEM_LEN);
+   gcc_assert (tmp!= NULL_TREE
+ 	  && TREE_TYPE (tmp) == size_type_node);
+   return fold_build3_loc (input_location, COMPONENT_REF, TREE_TYPE (tmp),
+ 			  dtype, tmp, NULL_TREE);
+ }
+ 
+ 
+ tree
  gfc_conv_descriptor_attribute (tree desc)
  {
tree tmp;
Index: gcc/fortran/trans-array.h
===
*** gcc/fortran/trans-array.h	(revision 268193)
--- gcc/fortran/trans-array.h	(working copy)
*** tree gfc_conv_descriptor_offset_get (tre
*** 169,174 
--- 169,175 
  tree gfc_conv_descriptor_span_get (tree);
  tree gfc_conv_descriptor_dtype (tree);
  tree gfc_conv_descriptor_rank (tree);
+ tree gfc_conv_descriptor_elem_len (tree);
  tree gfc_conv_descriptor_attribute (tree);
  tree gfc_get_descriptor_dimension (tree);
  tree gfc_conv_descriptor_stride_get (tree, tree);
Index: gcc/fortran/trans-expr.c
===
*** gcc/fortran/trans-expr.c	(revision 268193)
--- gcc/fortran/trans-expr.c	(working copy)
*** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
*** 4924,4929 
--- 4924,4931 
tree tmp;
tree cfi_desc_ptr;
tree gfc_desc_ptr;
+   tree ptr = NULL_TREE;
+   tree size;
tree type;
int attribute;
symbol_attribute attr = gfc_expr_attr (e);
*** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
*** 4939,4945 
  	attribute = 1;
  }
  
!   if (e->rank)
  {
gfc_conv_expr_descriptor (parmse, e);
  
--- 4941,4947 
  	attribute = 1;
  }
  
!   if (e->rank != 0)
  {
gfc_conv_expr_descriptor (parmse, e);
  
*** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
*** 4950,4958 
/* All the temporary descriptors are marked as DECL_ARTIFICIAL. If
  	 the expression type is different from the descriptor type, then
  	 the offset must be found (eg. to a component ref or substring)
! 	 and the dtype updated.  */
!   type = gfc_typenode_for_spec (>ts);
!   if (DECL_ARTIFICIAL (parmse->expr)
  	  && type != gfc_get_element_type (TREE_TYPE (parmse->expr)))
  	{
  	  /* Obtain the offset to the data.  */
--- 4952,4965 
/* All the temporary descriptors are marked as DECL_ARTIFICIAL. If
  	 the expression type is different from the descriptor type, then
  	 the offset must be found (eg. to a component ref or substring)
! 	 and the dtype updated.  Assumed type entities are only allowed
! 	 to be dummies in fortran. They therefore lack the  decl specific
! 	 appendiges and so 

Re: [PATCH] sched-ebb.c: avoid moving table jumps (PR rtl-optimization/88423)

[Segher Boessenkool]

On Wed, Jan 23, 2019 at 04:52:24PM +0300, Alexander Monakov wrote:
> On Wed, 23 Jan 2019, Andrey Belevantsev wrote:
> > For that, I'm not sure.  Your patch will leave the tablejump unscheduled at
> > all, i.e. any fields like INSN_TICK would be unfilled and thus the later
> > passes like bundling on ia64 will not work.  Maybe we can just stop
> > tablejumps from moving within its block, Alexander?
> 
> On the Bugzilla there's an alternative patch by Segher that adjusts the assert
> to accept this situation (there's still a barrier insn after the tablejump 
> insn,
> it's just after a jump_table_data insn rather than immediately following the
> jump).  That should be a better approach, and David said he was testing it.
> 
> That said, I'm really concerned that on this testcase we should not be moving
> the tablejump *at all*: we are moving it up past a 'use ax' insn (the use is
> for the function's return value). So after the move the use is in an 
> unreachable
> block, which makes no sense.

Yes, that makes no sense indeed.  Is it acceptable (for performance) to
simply bail out on table jumps here?

> So my concern is that just fixing the assert changes the issue from the ICE 
> to a
> (latent) wrong-code issue.
> 
> There should have been an anti-dependence between the use and the jump. I'll 
> try
> to investigate.

Or that.  Thanks!


Segher

[PATCH, i386] Fix PR 88998, bad codegen with mmx instructions

[Uros Bizjak]

Attached patch adds SSE alternatives to sse2_cvtpi2pd, sse2_cvtpd2pi
and sse2_cvttpd2pi to avoid MMX registers when e.g. _mm_cvtepi32_pd
intrinsics is used. Without the patch, the testcase compiles to (-O2
-mavx):

_Z7prepareii:
vmovd   %edi, %xmm1
vpinsrd $1, %esi, %xmm1, %xmm0
movdq2q %xmm0, %mm0
cvtpi2pd%mm0, %xmm0
vhaddpd %xmm0, %xmm0, %xmm0
ret

while patched gcc generates:

vmovd   %edi, %xmm1
vpinsrd $1, %esi, %xmm1, %xmm0
vcvtdq2pd   %xmm0, %xmm0
vhaddpd %xmm0, %xmm0, %xmm0
ret

The later avoids transition of FPU to MMX mode.

2019-01-23  Uroš Bizjak  

PR target/88998
* config/i386/sse.md (sse2_cvtpi2pd): Add SSE alternatives.
Disparage MMX alternative.
(sse2_cvtpd2pi): Ditto.
(sse2_cvttpd2pi): Ditto.

testsuite/ChangeLog:

2019-01-23  Uroš Bizjak  

PR target/88998
* g++.target/i386/pr88998.c: New test.

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

Committed to mainline, will be backported to release branches.

Uros.
Index: config/i386/sse.md
===
--- config/i386/sse.md  (revision 268188)
+++ config/i386/sse.md  (working copy)
@@ -4997,37 +4997,49 @@
 ;
 
 (define_insn "sse2_cvtpi2pd"
-  [(set (match_operand:V2DF 0 "register_operand" "=x,x")
-   (float:V2DF (match_operand:V2SI 1 "nonimmediate_operand" "y,m")))]
+  [(set (match_operand:V2DF 0 "register_operand" "=v,x")
+   (float:V2DF (match_operand:V2SI 1 "nonimmediate_operand" "vBm,?!y")))]
   "TARGET_SSE2"
-  "cvtpi2pd\t{%1, %0|%0, %1}"
+  "@
+   %vcvtdq2pd\t{%1, %0|%0, %1}
+   cvtpi2pd\t{%1, %0|%0, %1}"
   [(set_attr "type" "ssecvt")
-   (set_attr "unit" "mmx,*")
-   (set_attr "prefix_data16" "1,*")
+   (set_attr "unit" "*,mmx")
+   (set_attr "prefix_data16" "*,1")
+   (set_attr "prefix" "maybe_vex,*")
(set_attr "mode" "V2DF")])
 
 (define_insn "sse2_cvtpd2pi"
-  [(set (match_operand:V2SI 0 "register_operand" "=y")
-   (unspec:V2SI [(match_operand:V2DF 1 "nonimmediate_operand" "xm")]
+  [(set (match_operand:V2SI 0 "register_operand" "=v,?!y")
+   (unspec:V2SI [(match_operand:V2DF 1 "nonimmediate_operand" "vBm,xm")]
 UNSPEC_FIX_NOTRUNC))]
   "TARGET_SSE2"
-  "cvtpd2pi\t{%1, %0|%0, %1}"
+  "@
+   * return TARGET_AVX ? \"vcvtpd2dq{x}\t{%1, %0|%0, %1}\" : \"cvtpd2dq\t{%1, 
%0|%0, %1}\";
+   cvtpd2pi\t{%1, %0|%0, %1}"
   [(set_attr "type" "ssecvt")
-   (set_attr "unit" "mmx")
+   (set_attr "unit" "*,mmx")
+   (set_attr "amdfam10_decode" "double")
+   (set_attr "athlon_decode" "vector")
(set_attr "bdver1_decode" "double")
-   (set_attr "btver2_decode" "direct")
-   (set_attr "prefix_data16" "1")
-   (set_attr "mode" "DI")])
+   (set_attr "prefix_data16" "*,1")
+   (set_attr "prefix" "maybe_vex,*")
+   (set_attr "mode" "TI")])
 
 (define_insn "sse2_cvttpd2pi"
-  [(set (match_operand:V2SI 0 "register_operand" "=y")
-   (fix:V2SI (match_operand:V2DF 1 "nonimmediate_operand" "xm")))]
+  [(set (match_operand:V2SI 0 "register_operand" "=v,?!y")
+   (fix:V2SI (match_operand:V2DF 1 "nonimmediate_operand" "vBm,xm")))]
   "TARGET_SSE2"
-  "cvttpd2pi\t{%1, %0|%0, %1}"
+  "@
+   * return TARGET_AVX ? \"vcvttpd2dq{x}\t{%1, %0|%0, %1}\" : 
\"cvttpd2dq\t{%1, %0|%0, %1}\";
+   cvttpd2pi\t{%1, %0|%0, %1}"
   [(set_attr "type" "ssecvt")
-   (set_attr "unit" "mmx")
+   (set_attr "unit" "*,mmx")
+   (set_attr "amdfam10_decode" "double")
+   (set_attr "athlon_decode" "vector")
(set_attr "bdver1_decode" "double")
-   (set_attr "prefix_data16" "1")
+   (set_attr "prefix_data16" "*,1")
+   (set_attr "prefix" "maybe_vex,*")
(set_attr "mode" "TI")])
 
 (define_insn "sse2_cvtsi2sd"
Index: testsuite/g++.target/i386/pr88998.C
===
--- testsuite/g++.target/i386/pr88998.C (nonexistent)
+++ testsuite/g++.target/i386/pr88998.C (working copy)
@@ -0,0 +1,31 @@
+// PR target/88998
+// { dg-do run { target sse2_runtime } }
+// { dg-options "-O2 -msse2 -mfpmath=387" }
+// { dg-require-effective-target c++11 }
+
+#include 
+#include 
+#include 
+
+double
+__attribute__((noinline))
+prepare (int a, int b)
+{
+  __m128i is = _mm_setr_epi32 (a, b, 0, 0);
+  __m128d ds = _mm_cvtepi32_pd (is);
+  return ds[0] + ds[1];
+}
+
+int
+main (int, char **)
+{
+  double d = prepare (1, 2);
+
+  std::unordered_map < int, int >m;
+  m.insert ({0, 0});
+  m.insert ({1, 1});
+  assert (m.load_factor () <= m.max_load_factor ());
+
+  assert (d == 3);
+  return 0;
+}

Re: [PATCH] Remove a barrier when EDGE_CROSSING is remoed (PR lto/88858).

[Segher Boessenkool]

Hi Martin,

On Wed, Jan 23, 2019 at 10:29:40AM +0100, Martin Liška wrote:
> diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c
> index 172bdf585d0..5dd316efb63 100644
> --- a/gcc/cfgrtl.c
> +++ b/gcc/cfgrtl.c
> @@ -4396,6 +4396,25 @@ cfg_layout_redirect_edge_and_branch (edge e, 
> basic_block dest)
>"Removing crossing jump while redirecting edge form %i to 
> %i\n",
>e->src->index, dest->index);
>delete_insn (BB_END (src));
> +
> +  /* Unlink a BARRIER that can be still in BB_FOOTER.  */
> +  rtx_insn *insn = BB_FOOTER (src);
> +  while (insn != NULL_RTX && !BARRIER_P (insn))
> + insn = NEXT_INSN (insn);
> +
> +  if (insn != NULL_RTX)
> + {
> +   if (insn == BB_FOOTER (src))
> + BB_FOOTER (src) = NULL;
> +   else
> + {
> +   if (PREV_INSN (insn))
> + SET_NEXT_INSN (PREV_INSN (insn)) = NEXT_INSN (insn);
> +   if (NEXT_INSN (insn))
> + SET_PREV_INSN (NEXT_INSN (insn)) = PREV_INSN (insn);
> + }
> + }


combine.c has nicer code to do this, in update_cfg_for_uncondjump.  Split
it out into some common routine?  Something in cfgrtl.c I guess.


Segher

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

[Marek Polacek]

On Wed, Jan 23, 2019 at 09:00:36AM -0500, Jason Merrill wrote:
> I was talking about digest_init, not reshape_init.  digest_init calls
> convert_for_initialization.

/facepalm

So yes, digest_init calls convert_for_initialization which will end up
calling perform_implicit_conversion_flags which could call convert_like_real
where the narrowing warnings are given, but it doesn't, we go to this case:

  else if (processing_template_decl && conv->kind != ck_identity)
{
  /* In a template, we are only concerned about determining the
 type of non-dependent expressions, so we do not have to
 perform the actual conversion.  But for initializers, we
 need to be able to perform it at instantiation
 (or instantiate_non_dependent_expr) time.  */
  expr = build1 (IMPLICIT_CONV_EXPR, type, expr);

finish_decltype_type throws away the expression because it's not dependent, and
only uses its type.  So narrowing remains undetected.  Not sure if I should mess
with perform_implicit_conversion_flags.

Re: [PATCH] Refine -Waddress-of-packed-member once more

[Bernd Edlinger]

On 1/23/19 4:22 PM, Jakub Jelinek wrote:
> On Tue, Jan 22, 2019 at 02:10:38PM +, Bernd Edlinger wrote:
>> --- gcc/c-family/c-warn.c(revision 268119)
>> +++ gcc/c-family/c-warn.c(working copy)
>> @@ -2796,6 +2796,10 @@ check_address_or_pointer_of_packed_membe
>>if (context)
>>  break;
>>  }
>> +  if (TREE_CODE (TREE_TYPE (rhs)) == ARRAY_TYPE)
>> +rvalue = false;
>> +  if (rvalue)
>> +return NULL_TREE;
> 
> That looks like ARRAY_REF specific stuff, isn't it?  We have various other
> handled components, does e.g. REALPART_EXPR/IMAGPART_EXPR need that?
> What about VIEW_CONVERT_EXPR?  Or is that something you want to do for
> all of them?  In any case, there should be testcases with _Complex and
> __real__/__imag__, address of that as well as value.
> 

Okay, my guess it it will work with _Complex, but test cases would be
straight forward, and no reason not to have at least a few.

I have no idea what test case would be needed for VIEW_CONVERT_EXPR.

>> @@ -52,4 +54,6 @@ void foo (void)
>>i1 = t0.d; /* { dg-warning "may result in an unaligned 
>> pointer value" } */
>>i1 = t1->d;/* { dg-warning "may result in an unaligned 
>> pointer value" } */
>>i1 = t10[0].d; /* { dg-warning "may result in an unaligned 
>> pointer value" } */
>> +  i1 = (int*) [0].e[0];  /* { dg-warning "may result in an unaligned 
>> pointer value" } */
>> +  i1 = (int*) t10[0].e;  /* { dg-warning "may result in an unaligned 
>> pointer value" } */
> 
> Why not also i1 = [0].e[0];
> and i1 = t10[0].e; tests?
> 

I will add that.

> Unrelated to this patch, I'm worried e.g. about
>   if (INDIRECT_REF_P (rhs))
> rhs = TREE_OPERAND (rhs, 0);
>
> at the start of the check_address_or_pointer_of_packed_member
> function, what if we have:
>   i1 = *
> Do we want to warn when in the end we don't care if the address is unaligned
> or not, this is folded eventually into t0.c.
> 

Porbably not. I tried your example, and find currently this is inconsistent
between C and C++, C folds *& away, before the warning, and C++ does not and
gets a warning.  I feel like I want to fix that too.

As a side note, your * has a VIEW_CONVERT_EXPR in C++ but not in C:

Breakpoint 1, check_address_or_pointer_of_packed_member (type=0x76eef9d8, 
rhs=0x7702d760) at ../../gcc-trunk/gcc/c-family/c-warn.c:2727
2727  bool rvalue = true;
(gdb) call debug(rhs)
*_CONVERT_EXPR(t0).c

... but it does not affect the warning.


> Plus as I said earlier to H.J., I think
>   bool nop_p;
> 
>   while (TREE_CODE (rhs) == COMPOUND_EXPR)
> rhs = TREE_OPERAND (rhs, 1);
> 
>   tree orig_rhs = rhs;
>   STRIP_NOPS (rhs);
>   nop_p = orig_rhs != rhs;
> should be really:
>   bool nop_p;
>   tree orig_rhs = rhs;
>   STRIP_NOPS (rhs);
>   nop_p = orig_rhs != rhs;
> 
>   while (TREE_CODE (rhs) == COMPOUND_EXPR)
> rhs = TREE_OPERAND (rhs, 1);
> 
>   orig_rhs = rhs;
>   STRIP_NOPS (rhs);
>   nop_p |= orig_rhs != rhs;
> 
> or similar, because if there are casts around COMPOUND_EXPR, it doesn't than
> look through those COMPOUND_EXPRs, and if e.g. in the COMPOUND_EXPR rhs
> there is then COND_EXPR or similar, it should handle that case.
> 

Ah, yes.  That was not on my radar.

So this works in C and C++:
i1 = (0, (int*));

But this is again inconsistent between C and C++:
i1 = (int*) (0, );

C gets a warning, and C++ no warning.

I agree and see that consistency betwenn C and C++ as important goal.
So, I will try to fix that as well.


Bernd.

Re: [patch, fortran] Move some array packing to front end

[Thomas Koenig]

Hi Dominique,


FAIL: gfortran.dg/internal_pack_4.f90   -O3 -fomit-frame-pointer -funroll-loops 
-fpeel-loops -ftracer -finline-functions  execution test

with -m32.

gfc /opt/gcc/work/gcc/testsuite/gfortran.dg/internal_pack_4.f90 -O3 
-funroll-loops -ftracer -m32

is enough to trigger the miscomputation.


Thanks, I will look into it.


The changes in the test suite are quite messy and I hope I did not miss any test (you 
should do "diff -N …" for the new tests).


I don't think this is a good idea.  Applying the patch twice will then
double the test case.



Do you have test showing a speed-up?


It' in the PR.



I agree with Richard that this patch should be held until the next stage 1.


I just realized that we are, in principle, in regression-only mode.

I do wish the announcements were also made to the fortran mailing lists.

Anyway, I'll see if I can fix that bug, then attach the combined
patch to the PR for later inclusion.

Regards

Thomas

Re: [Patch] PR target/85711 - Fix ICE on aarch64

[Richard Sandiford]

Steve Ellcey  writes:
> The test gcc.dg/torture/pr84682-2.c has been failing for some time on
> aarch64.  Bin Cheng submitted a patch for this some time ago, the 
> original patch was:
>
> https://gcc.gnu.org/ml/gcc-patches/2018-03/msg00784.html
>
> But Richard Sandiford thought it should be fixed in recog.c instead of
> just in aarch64.c.  Bin submitted a followup:
>
> https://gcc.gnu.org/ml/gcc-patches/2018-03/msg01166.html
>
> But there were no replies to that patch submission.

IMO we shouldn't remove the assert.  See:

  https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01969.html

and the thread leading up to it.

Thanks,
Richard

Re: C++ PATCH for c++/88757 - qualified name treated wrongly as type

[Jason Merrill]

On 1/23/19 11:05 AM, Marek Polacek wrote:

Since C++20 P0634R3 we sometimes treat certain qualified-ids as types, so that
the user doesn't have to type 'typename'.  But this was broken for this case:

   template T::type N::v(T::value);

which, if T::value is a type, is a function template declaration.  But if
T::value is a value, it's a variable template definition.  So we can't
always assume T::value is a type.

This patch fixes it according to the proposed resolution discussed on the core
reflector (2018-10-29) -- perform name lookup to see if N::v is one or more
function templates, and if it isn't, don't consider 'typename' optional in the
parameter list.

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

2019-01-23  Marek Polacek  

PR c++/88757 - qualified name treated wrongly as type.
* parser.c (cp_parser_direct_declarator): Don't treat qualified-ids
in parameter-list as types if name lookup for declarator-id didn't
find one or more function templates.


OK.

Jason

Re: [PATCH] PR c++/87893 - constexpr ctor ICE on ARM.

[Jason Merrill]

On 1/23/19 9:26 AM, Ramana Radhakrishnan wrote:

On Wed, Jan 23, 2019 at 1:54 PM Jason Merrill  wrote:


Since in r265788 I made cxx_eval_outermost_constant_expr more insistent that
the returned value have the right type, it became more important that
initialized_type be correct.  These two PRs were cases of it giving the wrong
answer.  On ARM, a constructor returns a pointer to the object, but
initialized_type should return the class type, not that pointer type.  And we
need to look through COMPOUND_EXPR.

I tested that this fixes one of the affected testcases on ARM, and causes no
regressions on x86_64-pc-linux-gnu.  I haven't been able to get a cross
toolchain to work properly; currently link tests are failing with
undefined __sync_synchronize.  Applying to trunk.



rearnsha pointed this out to me - You probably need this with newlib
and it looks like the patch dropped between the cracks :(

https://sourceware.org/ml/newlib/2015/msg00653.html

I'll try and dust this off in the coming week.


Thanks, but compiling that fails for me with

sync_synchronize.s: Error: unaligned opcodes detected in executable segment

Would it also work to configure for a more specific target than arm-eabi?

Jason

[Patch] PR target/85711 - Fix ICE on aarch64

[Steve Ellcey]

The test gcc.dg/torture/pr84682-2.c has been failing for some time on
aarch64.  Bin Cheng submitted a patch for this some time ago, the 
original patch was:

https://gcc.gnu.org/ml/gcc-patches/2018-03/msg00784.html

But Richard Sandiford thought it should be fixed in recog.c instead of
just in aarch64.c.  Bin submitted a followup:

https://gcc.gnu.org/ml/gcc-patches/2018-03/msg01166.html

But there were no replies to that patch submission.  I have retested
the second patch and verified it fixes the aarch64 failure and causes
no regressions on aarch64 or x86.

OK to checkin?

Steve Ellcey
sell...@marvell.com




2019-01-23  Bin Cheng  
Steve Ellcey 

* recog.c (address_operand): Return false on wrong mode for address.
* config/aarch64/aarch64.c (aarch64_classify_address): Remove assert
since it's checked in general code now.




diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 5df5a8b..aa3054d 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -6565,9 +6565,6 @@ aarch64_classify_address (struct aarch64_address_info *info,
   && (code != POST_INC && code != REG))
 return false;
 
-  gcc_checking_assert (GET_MODE (x) == VOIDmode
-		   || SCALAR_INT_MODE_P (GET_MODE (x)));
-
   switch (code)
 {
 case REG:
diff --git a/gcc/recog.c b/gcc/recog.c
index d0c498f..fb90302 100644
--- a/gcc/recog.c
+++ b/gcc/recog.c
@@ -1070,6 +1070,11 @@ general_operand (rtx op, machine_mode mode)
 int
 address_operand (rtx op, machine_mode mode)
 {
+  /* Wrong mode for an address expr.  */
+  if (GET_MODE (op) != VOIDmode
+  && ! SCALAR_INT_MODE_P (GET_MODE (op)))
+return false;
+
   return memory_address_p (mode, op);
 }
 

Re: [PATCH, rs6000] Port cleanup patch, use rtl.h convenience macros, etc.

[Segher Boessenkool]

Hi!

On Tue, Dec 04, 2018 at 10:12:51AM -0600, Peter Bergner wrote:
> We talked about replacing rs6000'c regno_or_subregno() with the generic
> reg_or_subregno() function from jump.c.  I agree the geberic version is
> better because it has an assert that ensures we have a REG.  There were
> also a couple of places that could use reg_or_subregno() where we weren't.
> I made those changes and tested it and ran into no regressions.
> 
> Nearby the changes above, there were cases where could could have been using
> the rtl convenience macros like REG_P, etc. to simplify the code so I did
> that too...and kept going and just ended up converting all the cases I could
> find that should have been using them, to use them.  So the patch is large,
> but pretty straight forward.

In the future, please do separate patches?  Especially mechanical ones
could be separated (and then be much easier to review -- you also post
the script, and then I don't have to check it really is mechanical,
instead of just boring and you could have made mistakes).

>   (rs6000_init_hard_regno_mode_ok, direct_move_p): Use 
> HARD_REGISTER_NUM_P.

This line is too long.

And that is the worst I see in that whole patch!  Please apply it.

Thanks!


Segher

Re: [PATCH] aarch64: fix use-after-free in -march=native (PR driver/89014)

[Richard Earnshaw (lists)]

On 23/01/2019 17:12, David Malcolm wrote:
> Running:
>   $ valgrind ./xgcc -B. -c test.c -march=native
> on aarch64 shows a use-after-free in host_detect_local_cpu due
> to the std::string result of aarch64_get_extension_string_for_isa_flags
> only living until immediately after a c_str call.
> 
> This leads to corrupt "-march=" values being passed to cc1.
> 
> This patch fixes the use-after-free, though it appears to also need
> Tamar's patch here:
>   https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01302.html
> in order to generate valid values for cc1.  This may have worked by
> accident in the past, if the corrupt "-march=" value happened to be
> 0-terminated in the "right" place; with this patch it now appears
> to reliably break without Tamar's patch.
> 
> Lightly tested: I've manually verified that this cleans up the valgrind
> output for the driver, but I haven't bootstrapped with it.
> 
> OK for trunk?
> 

OK.

R.

> gcc/ChangeLog:
>   PR driver/89014
>   * config/aarch64/driver-aarch64.c (host_detect_local_cpu): Fix
>   use-after-free of the result of
>   aarch64_get_extension_string_for_isa_flags.
> ---
>  gcc/config/aarch64/driver-aarch64.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/gcc/config/aarch64/driver-aarch64.c 
> b/gcc/config/aarch64/driver-aarch64.c
> index 2bf1f9a..100e0c3 100644
> --- a/gcc/config/aarch64/driver-aarch64.c
> +++ b/gcc/config/aarch64/driver-aarch64.c
> @@ -178,7 +178,6 @@ host_detect_local_cpu (int argc, const char **argv)
>unsigned int variants[2] = { ALL_VARIANTS, ALL_VARIANTS };
>unsigned int n_variants = 0;
>bool processed_exts = false;
> -  const char *ext_string = "";
>unsigned long extension_flags = 0;
>unsigned long default_flags = 0;
>  
> @@ -348,11 +347,12 @@ host_detect_local_cpu (int argc, const char **argv)
>if (tune)
>  return res;
>  
> -  ext_string
> -= aarch64_get_extension_string_for_isa_flags (extension_flags,
> -   default_flags).c_str ();
> -
> -  res = concat (res, ext_string, NULL);
> +  {
> +std::string extension
> +  = aarch64_get_extension_string_for_isa_flags (extension_flags,
> + default_flags);
> +res = concat (res, extension.c_str (), NULL);
> +  }
>  
>return res;
>  
> 

Re: [PATCH][GCC][AArch64] Have empty HWCAPs string ignored during native feature detection

[Jakub Jelinek]

On Thu, Jan 10, 2019 at 04:57:47PM +, Kyrill Tkachov wrote:
> --- a/gcc/config/aarch64/driver-aarch64.c
> +++ b/gcc/config/aarch64/driver-aarch64.c
> @@ -253,6 +253,12 @@ host_detect_local_cpu (int argc, const char **argv)
> char *p = NULL;
> char *feat_string
>   = concat (aarch64_extensions[i].feat_string, NULL);
> +
> +   /* If the feature contains no HWCAPS string then ignore it for the
> +  auto detection.  */
> +   if (strlen (feat_string) == 0)
> + continue;
> 
> I think this can avoid a strlen call by checking (*feat_string == '\0') 
> though I believe most compilers will optimise it that way anyway.
> It might be more immediately readable your way.
> I wouldn't let it hold off this patch.

Well, that isn't the only problem of this code.
Another one is that concat (str, NULL) is much better written as xstrdup (str);
Another one is that the if (*feat_string == '\0') check
should be probably if (aarch64_extensions[i].feat_string[0] == '\0')
before the xstrdup, because there is no point to allocate the memory
in that case.
Another one is that it leaks the feat_string (right now always, otherwise
if it isn't empty).

Another one, I wonder if the xstrdup + strtok isn't a too heavy hammer for
something so simple, especially when you have complete control over those
feature strings.  They use exactly one space to separate.
So just do
  const char *p = aarch64_extensions[i].feat_string;
  bool enabled = true;

  /* If the feature contains no HWCAPS string then ignore it for the
 auto detection.  */
  if (*p == '\0')
continue;

  size_t len = strlen (buf);
  do
{
  const char *end = strchr (p, ' ');
  if (end == NULL)
end = strchr (p, '\0');
  if (memmem (buf, len, p, end - p) == NULL)
{
  /* Failed to match this token.  Turn off the
 features we'd otherwise enable.  */
  enabled = false;
  break;
}
  if (*end == '\0')
break;
  p = end + 1;
}
  while (1);

  if (enabled)
extension_flags |= aarch64_extensions[i].flag;
  else
extension_flags &= ~(aarch64_extensions[i].flag);
?

Last thing, for not_found, there is:
return "";, why not return NULL; ?  That is what other detect cpu routines
do, returning "" means a confusion between when a heap allocated string is
returned that needs freeing and when a .rodata string is returned which
doesn't.

Jakub

[PATCH] aarch64: fix use-after-free in -march=native (PR driver/89014)

[David Malcolm]

Running:
  $ valgrind ./xgcc -B. -c test.c -march=native
on aarch64 shows a use-after-free in host_detect_local_cpu due
to the std::string result of aarch64_get_extension_string_for_isa_flags
only living until immediately after a c_str call.

This leads to corrupt "-march=" values being passed to cc1.

This patch fixes the use-after-free, though it appears to also need
Tamar's patch here:
  https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01302.html
in order to generate valid values for cc1.  This may have worked by
accident in the past, if the corrupt "-march=" value happened to be
0-terminated in the "right" place; with this patch it now appears
to reliably break without Tamar's patch.

Lightly tested: I've manually verified that this cleans up the valgrind
output for the driver, but I haven't bootstrapped with it.

OK for trunk?

gcc/ChangeLog:
PR driver/89014
* config/aarch64/driver-aarch64.c (host_detect_local_cpu): Fix
use-after-free of the result of
aarch64_get_extension_string_for_isa_flags.
---
 gcc/config/aarch64/driver-aarch64.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/gcc/config/aarch64/driver-aarch64.c 
b/gcc/config/aarch64/driver-aarch64.c
index 2bf1f9a..100e0c3 100644
--- a/gcc/config/aarch64/driver-aarch64.c
+++ b/gcc/config/aarch64/driver-aarch64.c
@@ -178,7 +178,6 @@ host_detect_local_cpu (int argc, const char **argv)
   unsigned int variants[2] = { ALL_VARIANTS, ALL_VARIANTS };
   unsigned int n_variants = 0;
   bool processed_exts = false;
-  const char *ext_string = "";
   unsigned long extension_flags = 0;
   unsigned long default_flags = 0;
 
@@ -348,11 +347,12 @@ host_detect_local_cpu (int argc, const char **argv)
   if (tune)
 return res;
 
-  ext_string
-= aarch64_get_extension_string_for_isa_flags (extension_flags,
- default_flags).c_str ();
-
-  res = concat (res, ext_string, NULL);
+  {
+std::string extension
+  = aarch64_get_extension_string_for_isa_flags (extension_flags,
+   default_flags);
+res = concat (res, extension.c_str (), NULL);
+  }
 
   return res;
 
-- 
1.8.5.3

Re: [PATCH] sched-ebb.c: avoid moving table jumps (PR rtl-optimization/88423)

[Alexander Monakov]

On Wed, 23 Jan 2019, Alexander Monakov wrote:

> That said, I'm really concerned that on this testcase we should not be moving
> the tablejump *at all*: we are moving it up past a 'use ax' insn (the use is
> for the function's return value). So after the move the use is in an 
> unreachable
> block, which makes no sense.
> 
> So my concern is that just fixing the assert changes the issue from the ICE 
> to a
> (latent) wrong-code issue.
> 
> There should have been an anti-dependence between the use and the jump. I'll 
> try
> to investigate.

It appears that sched-deps tries to take notice of a barrier after a jump, but
similarly to sched-ebb doesn't anticipate that for a tablejump the barrier will
appear after two more insns (a code_label and a jump_table_data).

If so, it needs a fixup just like the posted change for the assert. I'll fire up
a bootstrap/regtest.

Alexander

* sched-deps.c (sched_analyze_insn): Take into account that for
tablejumps the barrier appears after a label and a jump_table_data.

--- a/gcc/sched-deps.c
+++ b/gcc/sched-deps.c
@@ -3005,6 +3005,8 @@ sched_analyze_insn (struct deps_desc *deps, rtx x, 
rtx_insn *insn)
   if (JUMP_P (insn))
 {
   rtx_insn *next = next_nonnote_nondebug_insn (insn);
+  if (LABEL_P (next) && JUMP_TABLE_DATA_P (NEXT_INSN (next)))
+   next = NEXT_INSN (NEXT_INSN (next));
   if (next && BARRIER_P (next))
reg_pending_barrier = MOVE_BARRIER;
   else

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

[Steve Ellcey]

On Wed, 2019-01-23 at 12:50 +, Tamar Christina wrote:
> Hi Steve,
> 
> > 
> > Hi Steve,
> > 
> > No we are using aarch64_be-*-* but this is the only one that popped
> > up as a
> > failure which is why I didn't change the others.
> > But now I'm wondering why... I'll check the log file manually
> > tomorrow to be
> > sure.
> 
> I've checked and the reason this didn't show up is because we don't
> run AArch64 big-endian cross linux tests by default and the baremetal
> builds don't support gomp.
> 
> I'm happy to update them for you if you want though.

I already updated them.  Thanks for checking on the failures.

Steve Ellcey
sell...@cavium.com

C++ PATCH for c++/88757 - qualified name treated wrongly as type

[Marek Polacek]

Since C++20 P0634R3 we sometimes treat certain qualified-ids as types, so that
the user doesn't have to type 'typename'.  But this was broken for this case:

  template T::type N::v(T::value);

which, if T::value is a type, is a function template declaration.  But if
T::value is a value, it's a variable template definition.  So we can't
always assume T::value is a type.

This patch fixes it according to the proposed resolution discussed on the core
reflector (2018-10-29) -- perform name lookup to see if N::v is one or more
function templates, and if it isn't, don't consider 'typename' optional in the
parameter list.

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

2019-01-23  Marek Polacek  

PR c++/88757 - qualified name treated wrongly as type.
* parser.c (cp_parser_direct_declarator): Don't treat qualified-ids
in parameter-list as types if name lookup for declarator-id didn't
find one or more function templates.

* g++.dg/cpp0x/dependent2.C: New test.
* g++.dg/cpp2a/typename10.C: Remove dg-error.
* g++.dg/cpp2a/typename12.C: New test.
* g++.dg/template/static30.C: Remove dg-error.

diff --git gcc/cp/parser.c gcc/cp/parser.c
index 7d7b0292650..dc9d651308a 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -21098,6 +21098,33 @@ cp_parser_direct_declarator (cp_parser* parser,
 
if (pack_expansion_p)
  maybe_warn_variadic_templates ();
+
+   /* We're looking for this case in [temp.res]:
+  A qualified-id is assumed to name a type if [...]
+  - it is a decl-specifier of the decl-specifier-seq of a
+parameter-declaration in a declarator of a function or
+function template declaration, ... */
+   if (cxx_dialect >= cxx2a
+   && (flags & CP_PARSER_FLAGS_TYPENAME_OPTIONAL)
+   && declarator->kind == cdk_id
+   /* ...whose declarator-id is qualified.  */
+   && qualifying_scope != NULL_TREE
+   && !at_class_scope_p ()
+   && cp_lexer_next_token_is (parser->lexer, CPP_OPEN_PAREN))
+ {
+   /* Now we have something like
+  template  int C::x(S::p);
+  which can be a function template declaration or a
+  variable template definition.  If name lookup for
+  the declarator-id C::x finds one or more function
+  templates, assume S::p to name a type.  Otherwise,
+  don't.  */
+   tree decl
+ = cp_parser_lookup_name_simple (parser, unqualified_name,
+ token->location);
+   if (!is_overloaded_fn (decl))
+ flags &= ~CP_PARSER_FLAGS_TYPENAME_OPTIONAL;
+ }
  }
 
handle_declarator:;
diff --git gcc/testsuite/g++.dg/cpp0x/dependent2.C 
gcc/testsuite/g++.dg/cpp0x/dependent2.C
new file mode 100644
index 000..a0740d404a3
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/dependent2.C
@@ -0,0 +1,10 @@
+// PR c++/88757
+// { dg-do compile { target c++11 } }
+
+template  struct C {
+static int x;
+};
+template  struct S {
+static const int size = 1;
+};
+template  int C::x(S::size);
diff --git gcc/testsuite/g++.dg/cpp2a/typename10.C 
gcc/testsuite/g++.dg/cpp2a/typename10.C
index fa2cd000b5d..1413268ba16 100644
--- gcc/testsuite/g++.dg/cpp2a/typename10.C
+++ gcc/testsuite/g++.dg/cpp2a/typename10.C
@@ -11,7 +11,7 @@ namespace N2 {
   template extern T::type v; // #1a
   //template T::type v(typename T::value); // #1b
 }
-template T::type N2::v(T::value); // { dg-error "" }
+template T::type N2::v(T::value);
 
 namespace A {
   inline namespace B { template int f(typename T::foo); }
diff --git gcc/testsuite/g++.dg/cpp2a/typename12.C 
gcc/testsuite/g++.dg/cpp2a/typename12.C
new file mode 100644
index 000..97962e53d65
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp2a/typename12.C
@@ -0,0 +1,20 @@
+// P0634R3
+// { dg-do compile { target c++2a } }
+
+struct W {
+  template
+  static int fn1 (T::X);
+  template
+  static int fn2 (T::X);
+  template
+  static int fn2 (T::X, int);
+};
+
+template
+int W::fn1 (T::X p) { return p; }
+
+template
+int W::fn2 (T::X p) { return p; }
+
+template
+int fn2 (typename T::X p) { return p; }
diff --git gcc/testsuite/g++.dg/template/static30.C 
gcc/testsuite/g++.dg/template/static30.C
index 8b8637a1abe..07dafe23ffa 100644
--- gcc/testsuite/g++.dg/template/static30.C
+++ gcc/testsuite/g++.dg/template/static30.C
@@ -6,5 +6,5 @@ template  struct A
   static const int i2;
 };
 
-template  const int A::i1(A::i); // { dg-error "no declaration 
matches" "" { target c++2a } }
+template  const int A::i1(A::i);
 template  const int A::i2(3, A::i); // { dg-error "expression 
list" }

Re: [PATCH 2/2] PR libstdc++/86756 Move rest of std::filesystem to libstdc++.so

[Christophe Lyon]

On Wed, 23 Jan 2019 at 16:28, Jonathan Wakely  wrote:
>
> On 09/01/19 13:53 +0100, Christophe Lyon wrote:
> >On Wed, 9 Jan 2019 at 11:11, Jonathan Wakely  wrote:
> >>
> >> On 09/01/19 10:09 +, Jonathan Wakely wrote:
> >> >On 08/01/19 11:13 +0100, Christophe Lyon wrote:
> >> >>On Mon, 7 Jan 2019 at 15:14, Christophe Lyon 
> >> >> wrote:
> >> >>>
> >> >>>On Mon, 7 Jan 2019 at 13:39, Jonathan Wakely  wrote:
> >> 
> >>  On 07/01/19 09:48 +, Jonathan Wakely wrote:
> >>  >On 07/01/19 10:24 +0100, Christophe Lyon wrote:
> >>  >>Hi Jonathan
> >>  >>
> >>  >>On Sun, 6 Jan 2019 at 23:37, Jonathan Wakely  
> >>  >>wrote:
> >>  >>>
> >>  >>>Move std::filesystem directory iterators and operations from
> >>  >>>libstdc++fs.a to main libstdc++ library. These components have many
> >>  >>>dependencies on OS support, which is not available on all targets. 
> >>  >>>Some
> >>  >>>additional autoconf checks and conditional compilation is needed to
> >>  >>>ensure the files will build for all targets. Previously this code 
> >>  >>>was
> >>  >>>not compiled without --enable-libstdcxx-filesystem-ts but the C++17
> >>  >>>components should be available for all hosted builds.
> >>  >>>
> >>  >>>The tests for these components no longer need to link to 
> >>  >>>libstdc++fs.a,
> >>  >>>but are not expected to pass on all targets. To avoid numerous 
> >>  >>>failures
> >>  >>>on targets which are not expected to pass the tests (due to 
> >>  >>>missing OS
> >>  >>>functionality) leave the dg-require-filesystem-ts directives in 
> >>  >>>place
> >>  >>>for now. This will ensure the tests only run for builds where the
> >>  >>>filesystem-ts library is built, which presumably means some level 
> >>  >>>of OS
> >>  >>>support is present.
> >>  >>>
> >>  >>>
> >>  >>>Tested x86_64-linux (old/new string ABIs, 32/64 bit), 
> >>  >>>x86_64-w64-mingw32.
> >>  >>>
> >>  >>>Committed to trunk.
> >>  >>>
> >>  >>
> >>  >>After this commit (r267616), I've noticed build failures for my
> >>  >>newlib-based toolchains:
> >>  >>aarch64-elf, arm-eabi:
> >>  >>
> >>  >>In file included from
> >>  >>/tmp/5241593_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libstdc++-v3/src/c++17/fs_ops.cc:57:
> >>  >>/tmp/5241593_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libstdc++-v3/src/c++17/../filesystem/ops-common.h:142:11:
> >>  >>error: '::truncate' has not been declared
> >>  >> 142 |   using ::truncate;
> >>  >> |   ^~~~
> >>  >>/tmp/5241593_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libstdc++-v3/src/c++17/fs_ops.cc:
> >>  >>In function 'void std::filesystem::resize_file(const
> >>  >>std::filesystem::__cxx11::path&, uintmax_t, std::error_code&)':
> >>  >>/tmp/5241593_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libstdc++-v3/src/c++17/fs_ops.cc:1274:19:
> >>  >>error: 'truncate' is not a member of 'posix'
> >>  >>1274 |   else if (posix::truncate(p.c_str(), size))
> >>  >> |   ^~~~
> >>  >>make[5]: *** [fs_ops.lo] Error 1
> >>  >>
> >>  >>I'm not sure if there's an obvious fix? Note that I'm using a rather
> >>  >>old newlib version, if that matters.
> >>  >
> >>  >That's probably the reason, as I didn't see this in my tests with
> >>  >newlib builds.
> >>  >
> >>  >The fix is to add yet another autoconf check and guard the uses of
> >>  >truncate with a _GLIBCXX_USE_TRUNCATE macro. I'll do that now ...
> >> 
> >> 
> >>  Should be fixed with this patch, committed to trunk as r267647.
> >> 
> >> >>>
> >> >>>Yes, it works. Thanks!
> >> >>>
> >> >>
> >> >>Hi Jonathan,
> >> >>
> >> >>So... this was a confirmation that the GCC build succeeded, not that
> >> >>the tests pass :)
> >> >>
> >> >>And there are actually a couple new errors with my newlib-based 
> >> >>toolchains:
> >> >>FAIL: 27_io/filesystem/operations/all.cc (test for excess errors)
> >> >>FAIL: 27_io/filesystem/operations/resize_file.cc (test for excess errors)
> >> >>FAIL: 27_io/filesystem/path/generation/normal2.cc (test for excess 
> >> >>errors)
> >> >>which are also UNRESOLVED, because of link-time undefined reference to 
> >> >>`chdir',
> >> >>chmod, mkdir, pathconf and getcwd.
> >> >
> >> >Ah, I was assuming if  is present, then those basic
> >> >functions will be present. More autoconf checks needed, I guess.
> >> >That isn't hard to do, just tedious.
> >> >
> >> >>On aarch64, I'm seeing an addtional:
> >> >>FAIL: 27_io/filesystem/path/compare/strings.cc execution test
> >> >>because:
> >> >>/libstdc++-v3/testsuite/27_io/filesystem/path/compare/strings.cc:39:
> >> >>void test01(): Assertion 'p.compare(p0) == p.compare(s0)' failed.
> >> >
> >> >Odd, I don't know why that would be target-specific. It's probably
> >> >just latent on other targets. I'll try to reproduce it on 

Re: [PATCH 2/2] PR libstdc++/86756 Move rest of std::filesystem to libstdc++.so

[Jonathan Wakely]

On 09/01/19 13:53 +0100, Christophe Lyon wrote:

On Wed, 9 Jan 2019 at 11:11, Jonathan Wakely  wrote:


On 09/01/19 10:09 +, Jonathan Wakely wrote:
>On 08/01/19 11:13 +0100, Christophe Lyon wrote:
>>On Mon, 7 Jan 2019 at 15:14, Christophe Lyon  
wrote:
>>>
>>>On Mon, 7 Jan 2019 at 13:39, Jonathan Wakely  wrote:

 On 07/01/19 09:48 +, Jonathan Wakely wrote:
 >On 07/01/19 10:24 +0100, Christophe Lyon wrote:
 >>Hi Jonathan
 >>
 >>On Sun, 6 Jan 2019 at 23:37, Jonathan Wakely  wrote:
 >>>
 >>>Move std::filesystem directory iterators and operations from
 >>>libstdc++fs.a to main libstdc++ library. These components have many
 >>>dependencies on OS support, which is not available on all targets. Some
 >>>additional autoconf checks and conditional compilation is needed to
 >>>ensure the files will build for all targets. Previously this code was
 >>>not compiled without --enable-libstdcxx-filesystem-ts but the C++17
 >>>components should be available for all hosted builds.
 >>>
 >>>The tests for these components no longer need to link to libstdc++fs.a,
 >>>but are not expected to pass on all targets. To avoid numerous failures
 >>>on targets which are not expected to pass the tests (due to missing OS
 >>>functionality) leave the dg-require-filesystem-ts directives in place
 >>>for now. This will ensure the tests only run for builds where the
 >>>filesystem-ts library is built, which presumably means some level of OS
 >>>support is present.
 >>>
 >>>
 >>>Tested x86_64-linux (old/new string ABIs, 32/64 bit), 
x86_64-w64-mingw32.
 >>>
 >>>Committed to trunk.
 >>>
 >>
 >>After this commit (r267616), I've noticed build failures for my
 >>newlib-based toolchains:
 >>aarch64-elf, arm-eabi:
 >>
 >>In file included from
 
>>/tmp/5241593_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libstdc++-v3/src/c++17/fs_ops.cc:57:
 
>>/tmp/5241593_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libstdc++-v3/src/c++17/../filesystem/ops-common.h:142:11:
 >>error: '::truncate' has not been declared
 >> 142 |   using ::truncate;
 >> |   ^~~~
 
>>/tmp/5241593_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libstdc++-v3/src/c++17/fs_ops.cc:
 >>In function 'void std::filesystem::resize_file(const
 >>std::filesystem::__cxx11::path&, uintmax_t, std::error_code&)':
 
>>/tmp/5241593_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libstdc++-v3/src/c++17/fs_ops.cc:1274:19:
 >>error: 'truncate' is not a member of 'posix'
 >>1274 |   else if (posix::truncate(p.c_str(), size))
 >> |   ^~~~
 >>make[5]: *** [fs_ops.lo] Error 1
 >>
 >>I'm not sure if there's an obvious fix? Note that I'm using a rather
 >>old newlib version, if that matters.
 >
 >That's probably the reason, as I didn't see this in my tests with
 >newlib builds.
 >
 >The fix is to add yet another autoconf check and guard the uses of
 >truncate with a _GLIBCXX_USE_TRUNCATE macro. I'll do that now ...


 Should be fixed with this patch, committed to trunk as r267647.

>>>
>>>Yes, it works. Thanks!
>>>
>>
>>Hi Jonathan,
>>
>>So... this was a confirmation that the GCC build succeeded, not that
>>the tests pass :)
>>
>>And there are actually a couple new errors with my newlib-based toolchains:
>>FAIL: 27_io/filesystem/operations/all.cc (test for excess errors)
>>FAIL: 27_io/filesystem/operations/resize_file.cc (test for excess errors)
>>FAIL: 27_io/filesystem/path/generation/normal2.cc (test for excess errors)
>>which are also UNRESOLVED, because of link-time undefined reference to 
`chdir',
>>chmod, mkdir, pathconf and getcwd.
>
>Ah, I was assuming if  is present, then those basic
>functions will be present. More autoconf checks needed, I guess.
>That isn't hard to do, just tedious.
>
>>On aarch64, I'm seeing an addtional:
>>FAIL: 27_io/filesystem/path/compare/strings.cc execution test
>>because:
>>/libstdc++-v3/testsuite/27_io/filesystem/path/compare/strings.cc:39:
>>void test01(): Assertion 'p.compare(p0) == p.compare(s0)' failed.
>
>Odd, I don't know why that would be target-specific. It's probably
>just latent on other targets. I'll try to reproduce it on my aarch64
>system, but it will take a while to build current trunk.
>
>If you have time, could you please apply this patch, re-run that test

*This* patch:

--- a/libstdc++-v3/testsuite/27_io/filesystem/path/compare/strings.cc
+++ b/libstdc++-v3/testsuite/27_io/filesystem/path/compare/strings.cc
@@ -36,6 +36,7 @@ test01()
 path p(s);
 VERIFY( p.compare(s) == 0 );
 VERIFY( p.compare(s.c_str()) == 0 );
+__builtin_printf("Comparing %s as path:%d as string:%d\n", s.c_str(), 
p.compare(p0), p.compare(s0));
 VERIFY( p.compare(p0) == p.compare(s0) );
 VERIFY( p.compare(p0) == p.compare(s0.c_str()) );
   }




>(cd $target/libstdc++-v3 && make check

Re: [PATCH] Refine -Waddress-of-packed-member once more

[Jakub Jelinek]

On Tue, Jan 22, 2019 at 02:10:38PM +, Bernd Edlinger wrote:
> --- gcc/c-family/c-warn.c (revision 268119)
> +++ gcc/c-family/c-warn.c (working copy)
> @@ -2796,6 +2796,10 @@ check_address_or_pointer_of_packed_membe
> if (context)
>   break;
>   }
> +  if (TREE_CODE (TREE_TYPE (rhs)) == ARRAY_TYPE)
> + rvalue = false;
> +  if (rvalue)
> + return NULL_TREE;

That looks like ARRAY_REF specific stuff, isn't it?  We have various other
handled components, does e.g. REALPART_EXPR/IMAGPART_EXPR need that?
What about VIEW_CONVERT_EXPR?  Or is that something you want to do for
all of them?  In any case, there should be testcases with _Complex and
__real__/__imag__, address of that as well as value.

> @@ -52,4 +54,6 @@ void foo (void)
>i1 = t0.d; /* { dg-warning "may result in an unaligned 
> pointer value" } */
>i1 = t1->d;/* { dg-warning "may result in an unaligned 
> pointer value" } */
>i1 = t10[0].d; /* { dg-warning "may result in an unaligned 
> pointer value" } */
> +  i1 = (int*) [0].e[0];  /* { dg-warning "may result in an unaligned 
> pointer value" } */
> +  i1 = (int*) t10[0].e;  /* { dg-warning "may result in an unaligned 
> pointer value" } */

Why not also i1 = [0].e[0];
and i1 = t10[0].e; tests?

Unrelated to this patch, I'm worried e.g. about
  if (INDIRECT_REF_P (rhs))
rhs = TREE_OPERAND (rhs, 0);
   
at the start of the check_address_or_pointer_of_packed_member
function, what if we have:
  i1 = *
Do we want to warn when in the end we don't care if the address is unaligned
or not, this is folded eventually into t0.c.

Plus as I said earlier to H.J., I think
  bool nop_p;

  while (TREE_CODE (rhs) == COMPOUND_EXPR)
rhs = TREE_OPERAND (rhs, 1);

  tree orig_rhs = rhs;
  STRIP_NOPS (rhs);
  nop_p = orig_rhs != rhs;
should be really:
  bool nop_p;
  tree orig_rhs = rhs;
  STRIP_NOPS (rhs);
  nop_p = orig_rhs != rhs;

  while (TREE_CODE (rhs) == COMPOUND_EXPR)
rhs = TREE_OPERAND (rhs, 1);

  orig_rhs = rhs;
  STRIP_NOPS (rhs);
  nop_p |= orig_rhs != rhs;

or similar, because if there are casts around COMPOUND_EXPR, it doesn't than
look through those COMPOUND_EXPRs, and if e.g. in the COMPOUND_EXPR rhs
there is then COND_EXPR or similar, it should handle that case.

Jakub

Re: [patch, fortran] Move some array packing to front end

[Dominique d'Humières]

Hi Thomas,

With your patch I see

FAIL: gfortran.dg/internal_pack_4.f90   -O3 -fomit-frame-pointer -funroll-loops 
-fpeel-loops -ftracer -finline-functions  execution test

with -m32.

gfc /opt/gcc/work/gcc/testsuite/gfortran.dg/internal_pack_4.f90 -O3 
-funroll-loops -ftracer -m32

is enough to trigger the miscomputation.

The changes in the test suite are quite messy and I hope I did not miss any 
test (you should do "diff -N …" for the new tests).

Do you have test showing a speed-up?

I agree with Richard that this patch should be held until the next stage 1.

Thanks for this work.

Dominique

Re: [PATCH] PR c++/87893 - constexpr ctor ICE on ARM.

[Ramana Radhakrishnan]

On Wed, Jan 23, 2019 at 1:54 PM Jason Merrill  wrote:
>
> Since in r265788 I made cxx_eval_outermost_constant_expr more insistent that
> the returned value have the right type, it became more important that
> initialized_type be correct.  These two PRs were cases of it giving the wrong
> answer.  On ARM, a constructor returns a pointer to the object, but
> initialized_type should return the class type, not that pointer type.  And we
> need to look through COMPOUND_EXPR.
>
> I tested that this fixes one of the affected testcases on ARM, and causes no
> regressions on x86_64-pc-linux-gnu.  I haven't been able to get a cross
> toolchain to work properly; currently link tests are failing with
> undefined __sync_synchronize.  Applying to trunk.
>

rearnsha pointed this out to me - You probably need this with newlib
and it looks like the patch dropped between the cracks :(

https://sourceware.org/ml/newlib/2015/msg00653.html

I'll try and dust this off in the coming week.

Ramana



> PR c++/88293 - ICE with comma expression.
> * constexpr.c (initialized_type): Don't shortcut non-void type.
> Handle COMPOUND_EXPR.
> (cxx_eval_outermost_constant_expr): Return early for void type.
> ---
>  gcc/cp/constexpr.c| 8 +---
>  gcc/testsuite/g++.dg/cpp0x/constexpr-comma1.C | 9 +
>  gcc/cp/ChangeLog  | 8 
>  3 files changed, 22 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-comma1.C
>
> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> index ed4bbeeb157..42681416760 100644
> --- a/gcc/cp/constexpr.c
> +++ b/gcc/cp/constexpr.c
> @@ -2848,9 +2848,7 @@ initialized_type (tree t)
>if (TYPE_P (t))
>  return t;
>tree type = TREE_TYPE (t);
> -  if (!VOID_TYPE_P (type))
> -/* No need to look deeper.  */;
> -  else if (TREE_CODE (t) == CALL_EXPR)
> +  if (TREE_CODE (t) == CALL_EXPR)
>  {
>/* A constructor call has void type, so we need to look deeper.  */
>tree fn = get_function_named_in_call (t);
> @@ -2858,6 +2856,8 @@ initialized_type (tree t)
>   && DECL_CXX_CONSTRUCTOR_P (fn))
> type = DECL_CONTEXT (fn);
>  }
> +  else if (TREE_CODE (t) == COMPOUND_EXPR)
> +return initialized_type (TREE_OPERAND (t, 1));
>else if (TREE_CODE (t) == AGGR_INIT_EXPR)
>  type = TREE_TYPE (AGGR_INIT_EXPR_SLOT (t));
>return cv_unqualified (type);
> @@ -5061,6 +5061,8 @@ cxx_eval_outermost_constant_expr (tree t, bool 
> allow_non_constant,
>
>tree type = initialized_type (t);
>tree r = t;
> +  if (VOID_TYPE_P (type))
> +return t;
>if (AGGREGATE_TYPE_P (type) || VECTOR_TYPE_P (type))
>  {
>/* In C++14 an NSDMI can participate in aggregate initialization,
> diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-comma1.C 
> b/gcc/testsuite/g++.dg/cpp0x/constexpr-comma1.C
> new file mode 100644
> index 000..9dd1299ddf4
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-comma1.C
> @@ -0,0 +1,9 @@
> +// PR c++/88293
> +// { dg-do compile { target c++11 } }
> +
> +struct A
> +{
> +  constexpr A () { }
> +};
> +
> +const A  = (A (), A ());
> diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
> index 111782aeaba..20a54719578 100644
> --- a/gcc/cp/ChangeLog
> +++ b/gcc/cp/ChangeLog
> @@ -1,3 +1,11 @@
> +2019-01-21  Jason Merrill  
> +
> +   PR c++/87893 - constexpr ctor ICE on ARM.
> +   PR c++/88293 - ICE with comma expression.
> +   * constexpr.c (initialized_type): Don't shortcut non-void type.
> +   Handle COMPOUND_EXPR.
> +   (cxx_eval_outermost_constant_expr): Return early for void type.
> +
>  2019-01-21  Jakub Jelinek  
>
> PR c++/88949
>
> base-commit: feb90a0dd6fc4e12786dce8338f716253d50b545
> --
> 2.20.1
>

[PATCH] Update assertion in sched-ebb.c to cope with table jumps

[David Malcolm]

On Wed, 2019-01-23 at 16:52 +0300, Alexander Monakov wrote:
> On Wed, 23 Jan 2019, Andrey Belevantsev wrote:
> 
> > For that, I'm not sure.  Your patch will leave the tablejump
> > unscheduled at
> > all, i.e. any fields like INSN_TICK would be unfilled and thus the
> > later
> > passes like bundling on ia64 will not work.  Maybe we can just stop
> > tablejumps from moving within its block, Alexander?
> 
> On the Bugzilla there's an alternative patch by Segher that adjusts
> the assert
> to accept this situation (there's still a barrier insn after the
> tablejump insn,
> it's just after a jump_table_data insn rather than immediately
> following the
> jump).  That should be a better approach, and David said he was
> testing it.
> 
> That said, I'm really concerned that on this testcase we should not
> be moving
> the tablejump *at all*: we are moving it up past a 'use ax' insn (the
> use is
> for the function's return value). So after the move the use is in an
> unreachable
> block, which makes no sense.
> 
> So my concern is that just fixing the assert changes the issue from
> the ICE to a
> (latent) wrong-code issue.
> 
> There should have been an anti-dependence between the use and the
> jump. I'll try
> to investigate.
> 
> Alexander

Thanks everyone for their clarifications (this is somewhat outside
my normal comfort zone of diagnostics/frontend stuff...)

Here's Segher's patch to sched-ebb.c, along with a couple of compile-only
testcases I put together (which triggered ICEs on x86_64 and powerpc
without the sched-ebb.c fix).

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu, and this
fixes the ICE in the the powerpc testcase.

That said, I share your concern that this might be papering over a
latent wrong-code issue.

Dave

gcc/ChangeLog:
PR rtl-optimization/88347
PR rtl-optimization/88423
* sched-ebb.c (begin_move_insn): Update assertion to handle table
jumps.

gcc/testsuite/ChangeLog:
PR rtl-optimization/88347
PR rtl-optimization/88423
* gcc.c-torture/compile/pr88347.c: New test.
* gcc.c-torture/compile/pr88423.c: New test.
---
 gcc/sched-ebb.c   | 6 +-
 gcc/testsuite/gcc.c-torture/compile/pr88347.c | 4 
 gcc/testsuite/gcc.c-torture/compile/pr88423.c | 5 +
 3 files changed, 14 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr88347.c
 create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr88423.c

diff --git a/gcc/sched-ebb.c b/gcc/sched-ebb.c
index d459e09..76a72b0 100644
--- a/gcc/sched-ebb.c
+++ b/gcc/sched-ebb.c
@@ -172,7 +172,11 @@ begin_move_insn (rtx_insn *insn, rtx_insn *last)
if (e)
  gcc_checking_assert (NOTE_P (x) || LABEL_P (x));
else
- gcc_checking_assert (BARRIER_P (x));
+ {
+   if (LABEL_P (x) && JUMP_TABLE_DATA_P (NEXT_INSN (x)))
+ x = NEXT_INSN (NEXT_INSN (x));
+   gcc_checking_assert (BARRIER_P (x));
+ }
   }
 
   if (e)
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr88347.c 
b/gcc/testsuite/gcc.c-torture/compile/pr88347.c
new file mode 100644
index 000..4e9b438
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr88347.c
@@ -0,0 +1,4 @@
+/* { dg-do compile { target { powerpc-*-* powerpcle-*-* } } } */
+/* { dg-options "-mcpu=603e -fsched-stalled-insns -fsched2-use-superblocks 
-fschedule-insns2 -fno-dce -fno-guess-branch-probability --param 
max-cse-insns=4" } */
+
+#include "../../gcc.target/powerpc/ppc-switch-2.c"
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr88423.c 
b/gcc/testsuite/gcc.c-torture/compile/pr88423.c
new file mode 100644
index 000..4948817
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr88423.c
@@ -0,0 +1,5 @@
+/* { dg-do compile { target { i?86-*-* x86_64-*-* } } } */
+/* { dg-options "-march=skylake -fPIC -fsched2-use-superblocks 
-fno-if-conversion" } */
+/* { dg-require-effective-target fpic } */
+
+#include "../../gcc.dg/20030309-1.c"
-- 
1.8.5.3

[PATCH] Fix reassoc leaving * 0 in the IL.

[Richard Biener]

$subject - on trunk the PR is hidden because those do not appear
for other reasons.  Still stray * 0 are prone to causing failures
thus the following.

On the branches this hides the SLSR bug.

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

Richard.

2019-01-23  Richard Biener  

PR tree-optimization/89008
* tree-ssa-reassoc.c (eliminate_using_constants): For * 0 do
not leave another stray operand.

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

Index: gcc/tree-ssa-reassoc.c
===
--- gcc/tree-ssa-reassoc.c  (revision 268182)
+++ gcc/tree-ssa-reassoc.c  (working copy)
@@ -1015,7 +1015,7 @@ eliminate_using_constants (enum tree_cod
fprintf (dump_file, "Found * 0, removing all other ops\n");
 
  reassociate_stats.ops_eliminated += ops->length () - 1;
- ops->truncate (1);
+ ops->truncate (0);
  ops->quick_push (oelast);
  return;
}
Index: gcc/testsuite/gcc.dg/torture/pr89008.c
===
--- gcc/testsuite/gcc.dg/torture/pr89008.c  (nonexistent)
+++ gcc/testsuite/gcc.dg/torture/pr89008.c  (working copy)
@@ -0,0 +1,27 @@
+/* { dg-do run } */
+/* { dg-require-effective-target int32plus } */
+
+unsigned long a, c;
+unsigned b;
+int d, e;
+long f()
+{
+  unsigned long g = 0;
+  for (d = 0; d < 5; d += 2)
+for (e = 0; e < 5; e += 3)
+  {
+   c = 4 + b;
+   g = -b - b;
+   b = 5 * (b << 24);
+  }
+  a = g;
+  return 0;
+}
+
+int main()
+{
+  f();
+  if (a)
+__builtin_abort();
+  return 0;
+}

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

[Jason Merrill]

On 1/22/19 4:10 PM, Marek Polacek wrote:

On Mon, Jan 21, 2019 at 03:14:53PM -0500, Jason Merrill wrote:

On 1/18/19 9:12 AM, Marek Polacek wrote:

On Thu, Jan 17, 2019 at 04:17:29PM -0500, Jason Merrill wrote:

On 1/17/19 2:09 PM, Marek Polacek wrote:

This patch ought to fix the rest of 78244, a missing narrowing warning in
decltype.

As I explained in Bugzilla, there can be three scenarios:

1) decltype is in a template and it has no dependent expressions, which
is the problematical case.  finish_compound_literal just returns the
compound literal without checking narrowing if processing_template_decl.


This is the sort of thing that we've been gradually fixing: if the compound
literal isn't dependent at all, we want to do the normal processing.  And
then usually return a result based on the original trees rather than the
result of processing.  For instance, finish_call_expr.  Something like that
ought to work here, too, and be more generally applicable; this shouldn't be
limited to casting to a scalar type, casting to a known class type can also
involve narrowing.


Great, that works just fine.  I also had to check if the type is
type-dependent, otherwise complete_type could fail.


The check in the other patch that changes instantiation_dependent_r should
be more similar to the one in finish_compound_literal.  Or perhaps you could
set a flag here in finish_compound_literal to indicate that it's
instantiation-dependent, and just check that in instantiation_dependent_r.


Done, but I feel bad about adding another flag.  But I guess it's cheaper
this way.  Thanks!

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

2019-01-18  Marek Polacek  

PR c++/88815 - narrowing conversion lost in decltype.
PR c++/78244 - narrowing conversion in template not detected.
* cp-tree.h (CONSTRUCTOR_IS_DEPENDENT): New.
* pt.c (instantiation_dependent_r): Consider a CONSTRUCTOR with
CONSTRUCTOR_IS_DEPENDENT instantiation-dependent.
* semantics.c (finish_compound_literal): When the compound literal
isn't instantiation-dependent and the type isn't type-dependent,
fall back to the normal processing.  Don't only call check_narrowing
for scalar types.  Set CONSTRUCTOR_IS_DEPENDENT.

* g++.dg/cpp0x/Wnarrowing15.C: New test.
* g++.dg/cpp0x/constexpr-decltype3.C: New test.
* g++.dg/cpp1y/Wnarrowing1.C: New test.

diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h
index 5cc8f88d522..778874cccd6 100644
--- gcc/cp/cp-tree.h
+++ gcc/cp/cp-tree.h
@@ -424,6 +424,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
 DECL_FINAL_P (in FUNCTION_DECL)
 QUALIFIED_NAME_IS_TEMPLATE (in SCOPE_REF)
 DECLTYPE_FOR_INIT_CAPTURE (in DECLTYPE_TYPE)
+  CONSTRUCTOR_IS_DEPENDENT (in CONSTRUCTOR)
 TINFO_USED_TEMPLATE_ID (in TEMPLATE_INFO)
 PACK_EXPANSION_SIZEOF_P (in *_PACK_EXPANSION)
 OVL_USING_P (in OVERLOAD)
@@ -4202,6 +4203,11 @@ more_aggr_init_expr_args_p (const 
aggr_init_expr_arg_iterator *iter)
  B b{1,2}, not B b({1,2}) or B b = {1,2}.  */
   #define CONSTRUCTOR_IS_DIRECT_INIT(NODE) (TREE_LANG_FLAG_0 
(CONSTRUCTOR_CHECK (NODE)))
+/* True if this CONSTRUCTOR is instantiation-dependent and needs to be
+   substituted.  */
+#define CONSTRUCTOR_IS_DEPENDENT(NODE) \
+  (TREE_LANG_FLAG_1 (CONSTRUCTOR_CHECK (NODE)))
+
   /* True if this CONSTRUCTOR should not be used as a variable initializer
  because it was loaded from a constexpr variable with mutable fields.  */
   #define CONSTRUCTOR_MUTABLE_POISON(NODE) \
diff --git gcc/cp/pt.c gcc/cp/pt.c
index e4f76478f54..ae77bae6b29 100644
--- gcc/cp/pt.c
+++ gcc/cp/pt.c
@@ -25800,6 +25800,11 @@ instantiation_dependent_r (tree *tp, int 
*walk_subtrees,
return *tp;
 break;
+case CONSTRUCTOR:
+  if (CONSTRUCTOR_IS_DEPENDENT (*tp))
+   return *tp;
+  break;
+
   default:
 break;
   }
diff --git gcc/cp/semantics.c gcc/cp/semantics.c
index e654750d249..4ff09ad3fb7 100644
--- gcc/cp/semantics.c
+++ gcc/cp/semantics.c
@@ -2795,11 +2795,14 @@ finish_compound_literal (tree type, tree 
compound_literal,
  return error_mark_node;
 }
-  if (processing_template_decl)
+  if (instantiation_dependent_expression_p (compound_literal)
+  || dependent_type_p (type))
   {
 TREE_TYPE (compound_literal) = type;
 /* Mark the expression as a compound literal.  */
 TREE_HAS_CONSTRUCTOR (compound_literal) = 1;
+  /* And as instantiation-dependent.  */
+  CONSTRUCTOR_IS_DEPENDENT (compound_literal) = true;
 if (fcl_context == fcl_c99)
CONSTRUCTOR_C99_COMPOUND_LITERAL (compound_literal) = 1;
 return compound_literal;
@@ -2822,8 +2825,7 @@ finish_compound_literal (tree type, tree compound_literal,
 && check_array_initializer (NULL_TREE, type, compound_literal))
   return error_mark_node;
 compound_literal = reshape_init (type, compound_literal, 

Re: [C++ PATCH] Fix switch genericization ICE (PR c++/88984)

[Jason Merrill]

On 1/22/19 5:25 PM, Jakub Jelinek wrote:

Hi!

The following testcase ICEs, because we assert that if during parsing we
haven't found any break; stmts in SWITCH_STMT_BODY, the switch statement
break label will not be used.  It is used in this case though, because
while during parsing we expect break stmts only from switch body to go to
the switch's break label and break stmts from the condition to go to
whatever outer construct supports break (if any, otherwise error),
during genericization we actually genericize it as break in condition
(possible only with statement expressions in there, so GNU extension)
jumping to the switch stmt break label.

Fixed thusly.  This patch makes C++ also behave like C on this testcase.

Bootstrapped/regtested on {x86_64,i686,powerpc64{,le}}-linux, ok for trunk?

2019-01-22  Jakub Jelinek  

PR c++/88984
* cp-gimplify.c (genericize_switch_stmt): Move cond genericization
before the begin_bc_block call.


OK.

Jason

Re: [C++ PATCH] Change similarly also loop genericization (PR c/44715)

[Jason Merrill]

On 1/22/19 5:25 PM, Jakub Jelinek wrote:

Hi!

This patch adjusts genericize_cp_loop similarly to the
PR88984 change in genericize_switch_stmt, so that the following testcase now
behaves the same in both C and C++, plus adds the testcase and
documentation.

Bootstrapped/regtested on {x86_64,i686,powerpc64{,le}}-linux, ok for trunk?

2019-01-22  Jakub Jelinek  

PR c/44715
* cp-gimplify.c (genericize_cp_loop): Call begin_bc_block only
after genericizing cond and incr expressions.
* doc/extend.texi: Document break and continue behavior in
statement expressions.


OK.

Jason

[PATCH] PR c++/87893 - constexpr ctor ICE on ARM.

[Jason Merrill]

Since in r265788 I made cxx_eval_outermost_constant_expr more insistent that
the returned value have the right type, it became more important that
initialized_type be correct.  These two PRs were cases of it giving the wrong
answer.  On ARM, a constructor returns a pointer to the object, but
initialized_type should return the class type, not that pointer type.  And we
need to look through COMPOUND_EXPR.

I tested that this fixes one of the affected testcases on ARM, and causes no
regressions on x86_64-pc-linux-gnu.  I haven't been able to get a cross
toolchain to work properly; currently link tests are failing with
undefined __sync_synchronize.  Applying to trunk.

PR c++/88293 - ICE with comma expression.
* constexpr.c (initialized_type): Don't shortcut non-void type.
Handle COMPOUND_EXPR.
(cxx_eval_outermost_constant_expr): Return early for void type.
---
 gcc/cp/constexpr.c| 8 +---
 gcc/testsuite/g++.dg/cpp0x/constexpr-comma1.C | 9 +
 gcc/cp/ChangeLog  | 8 
 3 files changed, 22 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-comma1.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index ed4bbeeb157..42681416760 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -2848,9 +2848,7 @@ initialized_type (tree t)
   if (TYPE_P (t))
 return t;
   tree type = TREE_TYPE (t);
-  if (!VOID_TYPE_P (type))
-/* No need to look deeper.  */;
-  else if (TREE_CODE (t) == CALL_EXPR)
+  if (TREE_CODE (t) == CALL_EXPR)
 {
   /* A constructor call has void type, so we need to look deeper.  */
   tree fn = get_function_named_in_call (t);
@@ -2858,6 +2856,8 @@ initialized_type (tree t)
  && DECL_CXX_CONSTRUCTOR_P (fn))
type = DECL_CONTEXT (fn);
 }
+  else if (TREE_CODE (t) == COMPOUND_EXPR)
+return initialized_type (TREE_OPERAND (t, 1));
   else if (TREE_CODE (t) == AGGR_INIT_EXPR)
 type = TREE_TYPE (AGGR_INIT_EXPR_SLOT (t));
   return cv_unqualified (type);
@@ -5061,6 +5061,8 @@ cxx_eval_outermost_constant_expr (tree t, bool 
allow_non_constant,
 
   tree type = initialized_type (t);
   tree r = t;
+  if (VOID_TYPE_P (type))
+return t;
   if (AGGREGATE_TYPE_P (type) || VECTOR_TYPE_P (type))
 {
   /* In C++14 an NSDMI can participate in aggregate initialization,
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-comma1.C 
b/gcc/testsuite/g++.dg/cpp0x/constexpr-comma1.C
new file mode 100644
index 000..9dd1299ddf4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-comma1.C
@@ -0,0 +1,9 @@
+// PR c++/88293
+// { dg-do compile { target c++11 } }
+
+struct A
+{ 
+  constexpr A () { }
+};
+
+const A  = (A (), A ());
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 111782aeaba..20a54719578 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,11 @@
+2019-01-21  Jason Merrill  
+
+   PR c++/87893 - constexpr ctor ICE on ARM.
+   PR c++/88293 - ICE with comma expression.
+   * constexpr.c (initialized_type): Don't shortcut non-void type.
+   Handle COMPOUND_EXPR.
+   (cxx_eval_outermost_constant_expr): Return early for void type.
+
 2019-01-21  Jakub Jelinek  
 
PR c++/88949

base-commit: feb90a0dd6fc4e12786dce8338f716253d50b545
-- 
2.20.1

Re: [PATCH] sched-ebb.c: avoid moving table jumps (PR rtl-optimization/88423)

[Alexander Monakov]

On Wed, 23 Jan 2019, Andrey Belevantsev wrote:

> For that, I'm not sure.  Your patch will leave the tablejump unscheduled at
> all, i.e. any fields like INSN_TICK would be unfilled and thus the later
> passes like bundling on ia64 will not work.  Maybe we can just stop
> tablejumps from moving within its block, Alexander?

On the Bugzilla there's an alternative patch by Segher that adjusts the assert
to accept this situation (there's still a barrier insn after the tablejump insn,
it's just after a jump_table_data insn rather than immediately following the
jump).  That should be a better approach, and David said he was testing it.

That said, I'm really concerned that on this testcase we should not be moving
the tablejump *at all*: we are moving it up past a 'use ax' insn (the use is
for the function's return value). So after the move the use is in an unreachable
block, which makes no sense.

So my concern is that just fixing the assert changes the issue from the ICE to a
(latent) wrong-code issue.

There should have been an anti-dependence between the use and the jump. I'll try
to investigate.

Alexander

[committed] Bump BASE-VER for stage4

[Jakub Jelinek]

Hi!

We forgot to bump BASE-VER when we've entered stage4, doing it now:

2019-01-23  Jakub Jelinek  

* BASE-VER: Bump to 9.0.1.

--- gcc/BASE-VER(revision 268183)
+++ gcc/BASE-VER(revision 268184)
@@ -1 +1 @@
-9.0.0
+9.0.1

Jakub

Re: [PATCH] Update URLs in libsanitizer/README.gcc [PR Bug sanitizer/89010]

[Jonny Grant]

On 23/01/2019 13:19, Jakub Jelinek wrote:

On Wed, Jan 23, 2019 at 11:30:29AM +, Jonny Grant wrote:

Not a member of this list, please include my email address in any replies.


2019-01-23  Jonny Grant  

PR 89010


PR sanitizer/89010
is what should be used.


* libsanitizer/README.gcc: Update to current https URLs


And period at the end of the above line.

Committed to trunk with those changes, thanks.



Excellent, many thanks Jakub

Re: [PATCH] Update URLs in libsanitizer/README.gcc [PR Bug sanitizer/89010]

[Jakub Jelinek]

On Wed, Jan 23, 2019 at 11:30:29AM +, Jonny Grant wrote:
> Not a member of this list, please include my email address in any replies.
> 
> 
> 2019-01-23  Jonny Grant  
> 
>   PR 89010

PR sanitizer/89010
is what should be used.

>   * libsanitizer/README.gcc: Update to current https URLs

And period at the end of the above line.

Committed to trunk with those changes, thanks.

> Index: trunk/libsanitizer/README.gcc
> ===
> --- trunk/libsanitizer/README.gcc (revision 268180)
> +++ trunk/libsanitizer/README.gcc (working copy)
> @@ -1,9 +1,9 @@
> -AddressSanitizer (http://code.google.com/p/address-sanitizer) and
> -ThreadSanitizer (http://code.google.com/p/thread-sanitizer/) are
> +AddressSanitizer and ThreadSanitizer (https://github.com/google/sanitizers) 
> are
>  projects initially developed by Google Inc.
> +
>  Both tools consist of a compiler module and a run-time library.
>  The sources of the run-time library for these projects are hosted at
> -http://llvm.org/svn/llvm-project/compiler-rt in the following directories:
> +https://llvm.org/svn/llvm-project/compiler-rt in the following directories:
>include/sanitizer
>lib/sanitizer_common
>lib/interception


Jakub

[PATCH] Handle timeout warnings in dg-extract-results

[Christophe Lyon]

Hi,

dg-extract-results currently moves lines like
WARNING: program timed out
at the end of each .exp section when it generates .sum files.

This is because it sorts its output based on the 2nd field, which is
normally the testname as in:
FAIL: gcc.c-torture/execute/20020129-1.c   -O2 -flto
-fno-use-linker-plugin -flto-partition=none  execution test

As you can notice 'program' comes after
gcc.c-torture/execute/20020129-1.c alphabetically, and generally after
most (all?) GCC testnames.

This is a bit of a pain when trying to handle transient test failures
because you can no longer match such a WARNING line to its FAIL
counterpart.

The attached patch changes this behavior by replacing the line
WARNING: program timed out
with
WARNING: gcc.c-torture/execute/20020129-1.c   -O2 -flto
-fno-use-linker-plugin -flto-partition=none  execution test program
timed out

The effect is that this line will now appear immediately above the
FAIL: gcc.c-torture/execute/20020129-1.c   -O2 -flto
-fno-use-linker-plugin -flto-partition=none  execution test
so that it's easier to match them.


I'm not sure how much people depend on the .sum format, I also
considered emitting
WARNING: program timed out gcc.c-torture/execute/20020129-1.c   -O2
-flto -fno-use-linker-plugin -flto-partition=none  execution test

I also restricted the patch to handling only 'program timed out'
cases, to avoid breaking other things.

I considered fixing this in Dejagnu, but it seemed more complicated,
and would delay adoption in GCC anyway.

What do people think about this?

Thanks,

Christophe
2019-01-23  Christophe Lyon  

contrib/
* dg-extract-results.py: Keep timeout warnings next to their
matching test.
* dg-extract-results.sh: Likewise.

diff --git a/contrib/dg-extract-results.py b/contrib/dg-extract-results.py
index 4b02a5b..ed62f73 100644
--- a/contrib/dg-extract-results.py
+++ b/contrib/dg-extract-results.py
@@ -239,6 +239,7 @@ class Prog:
 harness = None
 segment = None
 final_using = 0
+has_warning = 0
 
 # If this is the first run for this variation, add any text before
 # the first harness to the header.
@@ -292,8 +293,20 @@ class Prog:
 # Ugly hack to get the right order for gfortran.
 if name.startswith ('gfortran.dg/g77/'):
 name = 'h' + name
-key = (name, len (harness.results))
-harness.results.append ((key, line))
+# If we have a time out warning, make sure it appears
+# before the following testcase diagnostic: we insert
+# the testname before 'program' so that sort faces a
+# list of testhanes.
+if line.startswith ('WARNING: program timed out'):
+  has_warning = 1
+else:
+  if has_warning == 1:
+  key = (name, len (harness.results))
+  myline = 'WARNING: %s program timed out.\n' % name
+  harness.results.append ((key, myline))
+  has_warning = 0
+  key = (name, len (harness.results))
+  harness.results.append ((key, line))
 if not first_key and sort_logs:
 first_key = key
 if line.startswith ('ERROR: (DejaGnu)'):
diff --git a/contrib/dg-extract-results.sh b/contrib/dg-extract-results.sh
index 6ee3d26..e9833c1 100755
--- a/contrib/dg-extract-results.sh
+++ b/contrib/dg-extract-results.sh
@@ -298,6 +298,8 @@ BEGIN {
   cnt=0
   print_using=0
   need_close=0
+  has_timeout=0
+  timeout_cnt=0
 }
 /^EXPFILE: / {
   expfiles[expfileno] = \$2
@@ -329,16 +331,36 @@ BEGIN {
   # Ugly hack for gfortran.dg/dg.exp
   if ("$TOOL" == "gfortran" && testname ~ /^gfortran.dg\/g77\//)
 testname="h"testname
+  if (\$1 == "WARNING:" && \$2 == "program" && \$3 == "timed" && (\$4 == "out" 
|| \$4 == "out.")) {
+has_timeout=1
+timeout_cnt=cnt
+  } else {
+  # Prepare timeout replacement message in case it's needed
+timeout_msg=\$0
+sub(\$1, "WARNING:", timeout_msg)
+  }
 }
 /^$/ { if ("$MODE" == "sum") next }
 { if (variant == curvar && curfile) {
 if ("$MODE" == "sum") {
-  printf "%s %08d|", testname, cnt >> curfile
-  cnt = cnt + 1
+  # Do not print anything if the current line is a timeout
+  if (has_timeout == 0) {
+# If the previous line was a timeout,
+# insert the full current message without keyword
+if (timeout_cnt != 0) {
+  printf "%s %08d|%s program timed out.\n", testname, timeout_cnt, 
timeout_msg >> curfile
+  timeout_cnt = 0
+}
+printf "%s %08d|", testname, cnt >> curfile
+cnt = cnt + 1
+filewritten[curfile]=1
+need_close=1
+if (timeout_cnt == 0)
+  print >> curfile
+  }
+
+  has_timeout=0
 }
-filewritten[curfile]=1
-need_close=1

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

[Tamar Christina]

Hi Steve,

> 
> Hi Steve,
> 
> No we are using aarch64_be-*-* but this is the only one that popped up as a
> failure which is why I didn't change the others.
> But now I'm wondering why... I'll check the log file manually tomorrow to be
> sure.

I've checked and the reason this didn't show up is because we don't run AArch64 
big-endian cross linux tests by default and the baremetal builds don't support 
gomp.

I'm happy to update them for you if you want though.

Cheers,
Tamar

> 
> Cheers,
> Tamar
> 
> From: Steve Ellcey 
> Sent: Tuesday, January 22, 2019 10:34 PM
> To: Tamar Christina; Richard Sandiford
> Cc: gcc-patches@gcc.gnu.org; nd; christophe.l...@linaro.org
> Subject: Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI
> 
> On Mon, 2019-01-21 at 18:00 +, Tamar Christina wrote:
> >
> > > That would need to be aarch64*-*-* to include big-endian.  Fixing
> > > that here and in the other tests is OK under the obvious rule.
> >
> > Ah, true, I didn't look at the testcase. I tested the ILP32 case and
> > committed the fix for both.
> >
> > Thanks,
> > Tamar
> >
> > >
> > > Adding && lp64 (as per Steve's patch below) is OK too if it works.
> > >
> > > Thanks,
> > > Richard
> 
> Thanks for fixing this test Tamar.  I am going to change the others to use
> 'aarch64*-*-*' instead of 'aarch64-*-*' since that seems to be the standard
> method (though I see a few other tests that are not using
> aarch64 instead of aarch64*).
> 
> I guess that while you are testing big-endian the target triplet you are 
> using is
> still aarch64-*-* and not aarch64be-*-*?  Otherwise I would have expected
> you to have more failures.
> 
> Steve Ellcey
> sell...@marvell.com

[PATCH] libgcc2.c: Correct DI/TI -> SF/DF conversions

[H.J. Lu]

FSTYPE FUNC (DWtype u) in libgcc2.c, which converts DI/TI to SF/DF, has

  /* No leading bits means u == minimum.  */
  if (count == 0)
return -(Wtype_MAXp1_F * (Wtype_MAXp1_F / 2));

in the third case (where actually count == 0 only means the high part is
minimum).  It should be:

  /* No leading bits means u == minimum.  */
  if (count == 0)
return Wtype_MAXp1_F * (FSTYPE) (hi | ((UWtype) u != 0));

instead.

gcc/testsuite/

2019-01-23  H.J. Lu  

PR libgcc/88931
* gcc.dg/torture/fp-int-convert-timode-1.c: New test.
* gcc.dg/torture/fp-int-convert-timode-2.c: Likewise.
* gcc.dg/torture/fp-int-convert-timode-3.c: Likewise.
* gcc.dg/torture/fp-int-convert-timode-4.c: Likewise.

libgcc/

2019-01-23  Joseph Myers  

PR libgcc/88931
* libgcc2.c (FSTYPE FUNC (DWtype u)): Correct no leading bits
case.
---
 .../gcc.dg/torture/fp-int-convert-timode-1.c  | 25 +++
 .../gcc.dg/torture/fp-int-convert-timode-2.c  | 25 +++
 .../gcc.dg/torture/fp-int-convert-timode-3.c  | 25 +++
 .../gcc.dg/torture/fp-int-convert-timode-4.c  | 25 +++
 libgcc/libgcc2.c  |  2 +-
 5 files changed, 101 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-1.c
 create mode 100644 gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-2.c
 create mode 100644 gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-3.c
 create mode 100644 gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-4.c

diff --git a/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-1.c 
b/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-1.c
new file mode 100644
index 000..d6454fada72
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-1.c
@@ -0,0 +1,25 @@
+/* Test for correct rounding of conversions from __int128 to
+   float.  */
+/* { dg-do run } */
+/* { dg-require-effective-target int128 } */
+/* { dg-options "-frounding-math" } */
+
+#include 
+#include 
+
+int
+main (void)
+{
+  volatile unsigned long long h = 0x8000LL;
+  volatile unsigned long long l = 0xdLL;
+  volatile unsigned __int128 u128 = (((unsigned __int128) h) << 64) | l;
+  volatile __int128 s128 = u128;
+  fesetround (FE_TONEAREST);
+  float fs = s128;
+  if (fs != -0x1p+127)
+abort ();
+  double ds = s128;
+  if (ds != -0x1p+127)
+abort ();
+  exit (0);
+}
diff --git a/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-2.c 
b/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-2.c
new file mode 100644
index 000..8f831f7e98a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-2.c
@@ -0,0 +1,25 @@
+/* Test for correct rounding of conversions from __int128 to
+   float.  */
+/* { dg-do run } */
+/* { dg-require-effective-target int128 } */
+/* { dg-options "-frounding-math" } */
+
+#include 
+#include 
+
+int
+main (void)
+{
+  volatile unsigned long long h = 0x8000LL;
+  volatile unsigned long long l = 0xdLL;
+  volatile unsigned __int128 u128 = (((unsigned __int128) h) << 64) | l;
+  volatile __int128 s128 = u128;
+  fesetround (FE_DOWNWARD);
+  float fs = s128;
+  if (fs != -0x1p+127)
+abort ();
+  double ds = s128;
+  if (ds != -0x1p+127)
+abort ();
+  exit (0);
+}
diff --git a/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-3.c 
b/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-3.c
new file mode 100644
index 000..12cac66f39d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-3.c
@@ -0,0 +1,25 @@
+/* Test for correct rounding of conversions from __int128 to
+   float.  */
+/* { dg-do run } */
+/* { dg-require-effective-target int128 } */
+/* { dg-options "-frounding-math" } */
+
+#include 
+#include 
+
+int
+main (void)
+{
+  volatile unsigned long long h = 0x8000LL;
+  volatile unsigned long long l = 0xdLL;
+  volatile unsigned __int128 u128 = (((unsigned __int128) h) << 64) | l;
+  volatile __int128 s128 = u128;
+  fesetround (FE_UPWARD);
+  float fs = s128;
+  if (fs != -0x1.fep+126)
+abort ();
+  double ds = s128;
+  if (ds != -0x1.fp+126)
+abort ();
+  exit (0);
+}
diff --git a/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-4.c 
b/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-4.c
new file mode 100644
index 000..b1d8b83eb50
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/fp-int-convert-timode-4.c
@@ -0,0 +1,25 @@
+/* Test for correct rounding of conversions from __int128 to
+   float.  */
+/* { dg-do run } */
+/* { dg-require-effective-target int128 } */
+/* { dg-options "-frounding-math" } */
+
+#include 
+#include 
+
+int
+main (void)
+{
+  volatile unsigned long long h = 0x8000LL;
+  volatile unsigned long long l = 0xdLL;
+  volatile unsigned __int128 u128 = (((unsigned __int128) h) << 64) | l;
+  volatile __int128 s128 = u128;
+  fesetround (FE_TOWARDZERO);
+  float fs = s128;
+  if (fs != 

[PATCH] Update URLs in libsanitizer/README.gcc [PR Bug sanitizer/89010]

[Jonny Grant]

Not a member of this list, please include my email address in any replies.


2019-01-23  Jonny Grant  

PR 89010
* libsanitizer/README.gcc: Update to current https URLs
Index: trunk/libsanitizer/README.gcc
===
--- trunk/libsanitizer/README.gcc	(revision 268180)
+++ trunk/libsanitizer/README.gcc	(working copy)
@@ -1,9 +1,9 @@
-AddressSanitizer (http://code.google.com/p/address-sanitizer) and
-ThreadSanitizer (http://code.google.com/p/thread-sanitizer/) are
+AddressSanitizer and ThreadSanitizer (https://github.com/google/sanitizers) are
 projects initially developed by Google Inc.
+
 Both tools consist of a compiler module and a run-time library.
 The sources of the run-time library for these projects are hosted at
-http://llvm.org/svn/llvm-project/compiler-rt in the following directories:
+https://llvm.org/svn/llvm-project/compiler-rt in the following directories:
   include/sanitizer
   lib/sanitizer_common
   lib/interception

Re: [PATCH] sched-ebb.c: avoid moving table jumps (PR rtl-optimization/88423)

[Andrey Belevantsev]

Hi David,

On 18.01.2019 19:58, David Malcolm wrote:
> On Fri, 2019-01-18 at 12:32 -0500, David Malcolm wrote:
> 
> [CCing Abel]
> 
>> PR rtl-optimization/88423 reports an ICE within sched-ebb.c's
>> begin_move_insn, failing the assertion at line 175, where there's
>> no fall-through edge:
>>
>> 171 rtx_insn *x = NEXT_INSN (insn);
>> 172 if (e)
>> 173   gcc_checking_assert (NOTE_P (x) || LABEL_P (x));
>> 174 else
>> 175   gcc_checking_assert (BARRIER_P (x));
>>
>> "insn" is a jump_insn for a table jump, and its NEXT_INSN is the
>> placeholder code_label, followed by the jump_table_data.

This code was added at the time of our work for the speculation support in
ia64.  I would guess it was just never designed to support tablejumps,
rather the jumps to recovery blocks or some such.  But I might be wrong, it
was back in 2006.  If you look at fix_jump_move, which was added about that
time, it doesn't have any traces of tablejumps as well.

>>
>> It's not clear to me if such a jump_insn can be repositioned within
>> the insn stream, or if the scheduler is able to do so.  I believe a
>> tablejump is always at the end of such a head/tail insn sub-stream.
>> Is it a requirement that the placeholder code_label for the jump_insn
>> is always its NEXT_INSN?
>>
>> The loop at the start of schedule_ebb adjusts the head and tail
>> of the insns to be scheduled so that it skips leading and trailing
>> notes
>> and debug insns.
>>
>> This patch adjusts that loop to also skip trailing jump_insn
>> instances
>> that are table jumps, so that we don't attempt to move such table
>> jumps.
>>
>> This fixes the ICE, but I'm not very familiar with this part of the
>> compiler - so I have two questions:
>>
>> (1) What does GCC mean by "ebb" in this context?
>>
>> My understanding is that the normal definition of an "extended basic
>> block" (e.g. Muchnick's book pp175-177) is that it's a maximal
>> grouping
>> of basic blocks where only one BB in each group has multiple in-edges
>> and all other BBs in the group have a single in-edge (and thus e.g.
>> there's a tree-like structure of BBs within each EBB).
>>
>> From what I can tell, schedule_ebbs is iterating over BBs, looking
>> for
>> runs of BBs joined by next_bb that are connected by fallthrough edges
>> and don't have labels (and aren't flagged with BB_DISABLE_SCHEDULE).
>> It uses this run of BBs to generate a run of instructions within the
>> NEXT_INSN/PREV_INSN doubly-linked list, which it passes as "head"
>> and "tail" to schedule_ebb.
>>
>> This sounds like it will be a group of basic blocks with single in-
>> edges
>> internally, but it isn't a *maximal* group of such BBs - but perhaps
>> it's "maximal" in the sense of what the NEXT_INSN/PREV_INSN
>> representation can cope with?

You are right, it's not a tree structure in the sense of classical EBBs but
rather a trace.  There was a code to also perform tail duplication in order
to have better traces for scheduling, but the corresponding option got
deprecated back in 2010.

>>
>> There (presumably) can't be a fallthrough edge after a table jump, so
>> a table jump could only ever be at the end of such a chain, never in
>> the
>> middle.
>>
>> (2) Is it OK to omit "tail" from consideration here, from a dataflow
>> and insn-dependency point-of-view?  Presumably the scheduler is
>> written
>> to ensure that data used by subsequent basic blocks will still be
>> available
>> after the insns within an "EBB" are reordered, so presumably any data
>> uses *within* the jump_insn are still going to be available - but, as
>> I
>> said, I'm not very familiar with this part of the code.  (I think I'm
>> also
>> assuming that the jump_insn can't clobber data, just the PC)

For that, I'm not sure.  Your patch will leave the tablejump unscheduled at
all, i.e. any fields like INSN_TICK would be unfilled and thus the later
passes like bundling on ia64 will not work.  Maybe we can just stop
tablejumps from moving within its block, Alexander?

Andrey

>>
>> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>>
>> OK for trunk?
>>
>> gcc/ChangeLog:
>>  PR rtl-optimization/88423
>>  * sched-ebb.c (schedule_ebb): Don't move the jump_insn for a
>> table
>>  jump.
>>
>> gcc/testsuite/ChangeLog:
>>  PR rtl-optimization/88423
>>  * gcc.c-torture/compile/pr88423.c: New test.
>> ---
>>  gcc/sched-ebb.c   | 4 
>>  gcc/testsuite/gcc.c-torture/compile/pr88423.c | 5 +
>>  2 files changed, 9 insertions(+)
>>  create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr88423.c
>>
>> diff --git a/gcc/sched-ebb.c b/gcc/sched-ebb.c
>> index d459e09..1fe0b76 100644
>> --- a/gcc/sched-ebb.c
>> +++ b/gcc/sched-ebb.c
>> @@ -485,6 +485,10 @@ schedule_ebb (rtx_insn *head, rtx_insn *tail,
>> bool modulo_scheduling)
>>  tail = PREV_INSN (tail);
>>else if (LABEL_P (head))
>>  head = NEXT_INSN (head);
>> +  else if (tablejump_p (tail, NULL, 

[PATCH][wwwdocs][Arm][AArch64] Update changes with new features and flags.

[Tamar Christina]

Hi All,

This patch adds the documentation for Stack clash protection and Armv8.3-a 
support to
changes.html for GCC 9.
I have validated the html using the W3C validator.

Ok for cvs?

Thanks,
Tamar

-- 
Index: htdocs/gcc-9/changes.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-9/changes.html,v
retrieving revision 1.35
diff -u -r1.35 changes.html
--- htdocs/gcc-9/changes.html	15 Jan 2019 13:17:49 -	1.35
+++ htdocs/gcc-9/changes.html	22 Jan 2019 11:16:07 -
@@ -214,6 +214,27 @@
 -mtune=cortex-a76.cortex-a55 or as arguments to the equivalent target
 attributes and pragmas.
   
+  
+The AArch64 port now has support for stack clash protection using the
+-fstack-clash-protection option.  The protection also works for
+SVE systems.  The probing interval/guard size can be set by using
+--param stack-clash-protection-guard-size=12|16.
+The value of this parameter must be in bytes represented as a power of two.
+The only two supported values for this parameter are 12 and 16 being
+4Kb (2^12) and 64Kb (2^16) respectively.
+
+The default value is 16 (64Kb) and can be changed at configure
+time using the flag --with-stack-clash-protection-guard-size=12|16.
+  
+  
+The Armv8.3-A complex number instructions are now supported via intrinsics
+when the option -march=armv8.3-a or equivalent is specified.
+For the half-precision floating-point variants of these instructions use the
+architecture extension flag +fp16, e.g.
+-march=armv8.3-a+fp16.
+
+The intrinsics are defined by the ACLE specification.
+  
 
 
 ARC
@@ -250,6 +271,15 @@
  (which have no known implementations) has been removed.
  Note that Armv5T, Armv5TE and Armv5TEJ architectures remain supported.
   
+  
+The Armv8.3-A complex number instructions are now supported via intrinsics
+when the option -march=armv8.3-a or equivalent is specified.
+For the half-precision floating-point variants of these instructions use the
+architecture extension flag +fp16, e.g.
+-march=armv8.3-a+fp16.
+
+The intrinsics are defined by the ACLE specification.
+  
 
 
 

Re: [PATCH 3/3][GCC][AARCH64] Add support for pointer authentication B key

[Sam Tebbs]

On 14/01/2019 10:43, Kyrill Tkachov wrote:

>
> On 08/01/19 11:38, Sam Tebbs wrote:
>>
>> On 1/7/19 6:28 PM, James Greenhalgh wrote:
>> > On Fri, Dec 21, 2018 at 09:00:10AM -0600, Sam Tebbs wrote:
>> >> On 11/9/18 11:04 AM, Sam Tebbs wrote:
>> >
>> > 
>> >
>> >> Attached is an improved patch with "hint" removed from the test 
>> scans,
>> >> pauth_hint_num_a and pauth_hint_num_b merged into pauth_hint_num 
>> and the
>> >> "gcc_assert (cfun->machine->frame.laid_out)" removal reverted 
>> since was
>> >> an unnecessary change.
>> >>
>> >> OK for trunk?
>> > While the AArch64 parts look OK to me and are buried behind an 
>> option so are
>> > relatively safe even though we're late in development, you'll need 
>> someone
>> > else to approve the libgcc changes. Especially as you change a generic
>> > routine with an undocumented (?) AArch64-specific change.
>> >
>> > Thanks,
>> > James
>>
>> Thanks James, CC'ing Ian Lance Taylor.
>>
>
> Jeff, could you help with reviewing the libgcc changes please?
> I believe the latest version was posted at:
> https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01569.html
>
> Thanks,
> Kyrill
ping 3. The preceding two patches were committed a while ago but require 
the minor libgcc changes in this patch, which are the only parts left to 
be reviewed.
>
>> The documentation relevant to the libgcc change is expected to be
>> published in the near future.

Re: [patch] Fix segfault during inlining of thunk

[Richard Biener]

On Wed, Jan 23, 2019 at 10:48 AM Eric Botcazou  wrote:
>
> Hi,
>
> this is a regression present in Ada on the mainline since we switched to using
> the internal support for thunks: expand_thunk segfaults during inlining when
> trying to access the DECL_RESULT of the special alias built to make the call
> from the thunk local, if the DECL_RESULT of the thunk is DECL_BY_REFERENCE.
>
> It turns out that neither the C++ nor the Ada front-end builds the DECL_RESULT
> of this special alias (the C++ front-end apparently doesn't even build that of
> the thunk itself so expand_thunk has code to patch this up).  Moreover it's a
> bit strange to first try:
>
>restmp = gimple_fold_indirect_ref (resdecl);
>
> i.e. build the dereference using the type of resdecl and then use the type of
> the DECL_RESULT of the alias if this failed.  So the attached fix changes this
> to using the type of resdecl in the latter case too.
>
> Bootstrapped/regtested on x86_64-suse-linux, OK for the mainline?

OK.

I guess the original (cut?) error was to look at DECL_RESULT (alias)
instead of DECL_RESULT (thunk_fndecl) which would have made this
consistent as well.

Richard.

>
> 2019-01-22  Eric Botcazou  
>
> * cgraphunit.c (cgraph_node::expand_thunk): When expanding a GIMPLE
> thunk that returns by reference, use the type of the return object
> of the thunk instead of that of the alias to build the dereference.
>
>
> 2019-01-23  Eric Botcazou  
>
> * gnat.dg/specs/opt4.ads: New test.
>
> --
> Eric Botcazou

[PATCH] rs6000: Add support for the vec_sbox_be, vec_cipher_be etc. builtins.

[luoxhu]

From: Xiong Hu Luo 

The 5 new builtins vec_sbox_be, vec_cipher_be, vec_cipherlast_be, vec_ncipher_be
and vec_ncipherlast_be only support vector unsigned char type parameters.
Add new instruction crypto_vsbox_ and crypto__ to handle
them accordingly, where the new mode CR_vqdi can be expanded to vector unsigned
long long for none _be postfix builtins or vector unsigned char for _be postfix
builtins.

---
gcc/ChangeLog

2019-01-23  Xiong Hu Luo  

* gcc/config/rs6000/altivec.h (vec_sbox_be, vec_cipher_be,
vec_cipherlast_be, vec_ncipher_be, vec_ncipherlast_be): New #defines.
* gcc/config/rs6000/crypto.md (CR_vqdi): New define_mode_iterator.
(crypto_vsbox_, crypto__): New define_insns.
* gcc/config/rs6000/rs6000-builtin.def (VSBOX_BE): New BU_CRYPTO_1.
(VCIPHER_BE, VCIPHERLAST_BE, VNCIPHER_BE, VNCIPHERLAST_BE):
New BU_CRYPTO_2.
* gcc/config/rs6000/rs6000.c (builtin_function_type)
: New switch options.
* gcc/doc/extend.texi (vec_sbox_be, vec_cipher_be, vec_cipherlast_be,
vec_ncipher_be, vec_ncipherlast_be): New builtin functions.

gcc/testsuite/ChangeLog

2019-01-23  Xiong Hu Luo  

* gcc/testsuite/gcc.target/powerpc/crypto-builtin-1.c
(crpyto1_be, crpyto2_be, crpyto3_be, crpyto4_be, crpyto5_be):
New testcases.
---
 gcc/config/rs6000/altivec.h|  5 +++
 gcc/config/rs6000/crypto.md| 17 +-
 gcc/config/rs6000/rs6000-builtin.def   | 19 +---
 gcc/config/rs6000/rs6000.c |  5 +++
 gcc/doc/extend.texi| 13 
 .../gcc.target/powerpc/crypto-builtin-1.c  | 36 +++---
 6 files changed, 78 insertions(+), 17 deletions(-)

diff --git a/gcc/config/rs6000/altivec.h b/gcc/config/rs6000/altivec.h
index bf29d46..d66ae7c 100644
--- a/gcc/config/rs6000/altivec.h
+++ b/gcc/config/rs6000/altivec.h
@@ -418,6 +418,11 @@
 #define vec_vupkhsw __builtin_vec_vupkhsw
 #define vec_vupklsw __builtin_vec_vupklsw
 #define vec_revb __builtin_vec_revb
+#define vec_sbox_be __builtin_crypto_vsbox_be
+#define vec_cipher_be __builtin_crypto_vcipher_be
+#define vec_cipherlast_be __builtin_crypto_vcipherlast_be
+#define vec_ncipher_be __builtin_crypto_vncipher_be
+#define vec_ncipherlast_be __builtin_crypto_vncipherlast_be
 #endif
 
 #ifdef __POWER9_VECTOR__
diff --git a/gcc/config/rs6000/crypto.md b/gcc/config/rs6000/crypto.md
index 2ee3e3a..b9917b0 100644
--- a/gcc/config/rs6000/crypto.md
+++ b/gcc/config/rs6000/crypto.md
@@ -48,6 +48,9 @@
 ;; Iterator for VSHASIGMAD/VSHASIGMAW
 (define_mode_iterator CR_hash [V4SI V2DI])
 
+;; Iterator for VSBOX/VCIPHER/VNCIPHER/VCIPHERLAST/VNCIPHERLAST
+(define_mode_iterator CR_vqdi [V16QI V2DI])
+
 ;; Iterator for the other crypto functions
 (define_int_iterator CR_code   [UNSPEC_VCIPHER
UNSPEC_VNCIPHER
@@ -60,10 +63,10 @@
  (UNSPEC_VNCIPHERLAST "vncipherlast")])
 
 ;; 2 operand crypto instructions
-(define_insn "crypto_"
-  [(set (match_operand:V2DI 0 "register_operand" "=v")
-   (unspec:V2DI [(match_operand:V2DI 1 "register_operand" "v")
- (match_operand:V2DI 2 "register_operand" "v")]
+(define_insn "crypto__"
+  [(set (match_operand:CR_vqdi 0 "register_operand" "=v")
+   (unspec:CR_vqdi [(match_operand:CR_vqdi 1 "register_operand" "v")
+ (match_operand:CR_vqdi 2 "register_operand" "v")]
 CR_code))]
   "TARGET_CRYPTO"
   " %0,%1,%2"
@@ -90,9 +93,9 @@
   [(set_attr "type" "vecperm")])
 
 ;; 1 operand crypto instruction
-(define_insn "crypto_vsbox"
-  [(set (match_operand:V2DI 0 "register_operand" "=v")
-   (unspec:V2DI [(match_operand:V2DI 1 "register_operand" "v")]
+(define_insn "crypto_vsbox_"
+  [(set (match_operand:CR_vqdi 0 "register_operand" "=v")
+   (unspec:CR_vqdi [(match_operand:CR_vqdi 1 "register_operand" "v")]
 UNSPEC_VSBOX))]
   "TARGET_CRYPTO"
   "vsbox %0,%1"
diff --git a/gcc/config/rs6000/rs6000-builtin.def 
b/gcc/config/rs6000/rs6000-builtin.def
index 60b3bd0..0a2bdb7 100644
--- a/gcc/config/rs6000/rs6000-builtin.def
+++ b/gcc/config/rs6000/rs6000-builtin.def
@@ -2418,13 +2418,22 @@ BU_P9_OVERLOAD_2 (CMPRB2,   "byte_in_either_range")
 BU_P9_OVERLOAD_2 (CMPEQB,  "byte_in_set")
 
 /* 1 argument crypto functions.  */
-BU_CRYPTO_1 (VSBOX,"vsbox",  CONST, crypto_vsbox)
+BU_CRYPTO_1 (VSBOX,"vsbox",  CONST, crypto_vsbox_v2di)
+BU_CRYPTO_1 (VSBOX_BE, "vsbox_be",   CONST, crypto_vsbox_v16qi)
 
 /* 2 argument crypto functions.  */
-BU_CRYPTO_2 (VCIPHER,  "vcipher",CONST, crypto_vcipher)
-BU_CRYPTO_2 (VCIPHERLAST,  "vcipherlast",CONST, crypto_vcipherlast)
-BU_CRYPTO_2 (VNCIPHER, "vncipher",   CONST, crypto_vncipher)
-BU_CRYPTO_2 (VNCIPHERLAST, "vncipherlast",   CONST, crypto_vncipherlast)

[patch] Fix segfault during inlining of thunk

[Eric Botcazou]

Hi,

this is a regression present in Ada on the mainline since we switched to using 
the internal support for thunks: expand_thunk segfaults during inlining when 
trying to access the DECL_RESULT of the special alias built to make the call 
from the thunk local, if the DECL_RESULT of the thunk is DECL_BY_REFERENCE.

It turns out that neither the C++ nor the Ada front-end builds the DECL_RESULT 
of this special alias (the C++ front-end apparently doesn't even build that of 
the thunk itself so expand_thunk has code to patch this up).  Moreover it's a 
bit strange to first try:

   restmp = gimple_fold_indirect_ref (resdecl);

i.e. build the dereference using the type of resdecl and then use the type of 
the DECL_RESULT of the alias if this failed.  So the attached fix changes this 
to using the type of resdecl in the latter case too.

Bootstrapped/regtested on x86_64-suse-linux, OK for the mainline?


2019-01-22  Eric Botcazou  

* cgraphunit.c (cgraph_node::expand_thunk): When expanding a GIMPLE
thunk that returns by reference, use the type of the return object
of the thunk instead of that of the alias to build the dereference.


2019-01-23  Eric Botcazou  

* gnat.dg/specs/opt4.ads: New test.

-- 
Eric BotcazouIndex: cgraphunit.c
===
--- cgraphunit.c	(revision 268071)
+++ cgraphunit.c	(working copy)
@@ -1916,10 +1916,9 @@ cgraph_node::expand_thunk (bool output_a
 	  restmp = gimple_fold_indirect_ref (resdecl);
 	  if (!restmp)
 		restmp = build2 (MEM_REF,
- TREE_TYPE (TREE_TYPE (DECL_RESULT (alias))),
+ TREE_TYPE (TREE_TYPE (resdecl)),
  resdecl,
- build_int_cst (TREE_TYPE
-   (DECL_RESULT (alias)), 0));
+ build_int_cst (TREE_TYPE (resdecl), 0));
 	}
 	  else if (!is_gimple_reg_type (restype))
 	{
-- { dg-do compile }
-- { dg-options "-O" }

package Opt4 is

   type Rec (D : Boolean := False) is record
  case D is
 when False => null;
 when True => I : Integer;
  end case;
   end record;

   Null_Rec : constant Rec := (D => False);

   type I1 is limited interface;

   type I2 is limited interface;

   function Func (Data : I2) return Rec is abstract;

   type Ext is limited new I1 and I2 with null record;

   overriding function Func (Data : Ext) return Rec is (Null_Rec);

end Opt4;

Re: [PATCH,GDC] Add netbsd support to GDC

[coypu]

(Oops, added the wrong email out of habit to the first reply :-))

On Tue, Jan 22, 2019 at 08:37:25PM +0100, Iain Buclaw wrote:
> > diff --git a/gcc/d/d-builtins.cc b/gcc/d/d-builtins.cc
> > index b0a315a3ed9..ca105c7635d 100644
> > --- a/gcc/d/d-builtins.cc
> > +++ b/gcc/d/d-builtins.cc
> > @@ -382,6 +382,8 @@ d_add_builtin_version (const char* ident)
> >  global.params.isWindows = true;
> >else if (strcmp (ident, "FreeBSD") == 0)
> >  global.params.isFreeBSD = true;
> > +  else if (strcmp (ident, "NetBSD") == 0)
> > +global.params.isNetBSD = true;
> >else if (strcmp (ident, "OpenBSD") == 0)
> >  global.params.isOpenBSD = true;
> >else if (strcmp (ident, "Solaris") == 0)
> 
> Is this necessary?  I'd prefer to not set this field if it can be
> avoided.  The same goes for the others, I intend to remove them at
> soonest convenience once the problematic parts of the front-end logic
> has been abstracted away.
> 
> Other than that, looks reasonable to me.

It's not necessary. Here's an updated diff without it.
I also forgot to add netbsd-d.c, which is referenced in the previous
diff.

libphobos/libdruntime changes were contributed upstream:
https://github.com/dlang/druntime/pull/2472
(caveat: pending a change to netbsd/execinfo.d)

gcc/config.gcc (*-*-netbsd*): add netbsd-d.o
gcc/config/t-netbsd: add netbsd-d.o
gcc/config/netbsd-d.c: New, define Posix and NetBSD builtins for NetBSD targets

gcc/d/d-system.h: NetBSD is POSIX
libphobos/configure.tgt: Mark netbsd/x86 as supported
---
 gcc/config.gcc  |  2 ++
 gcc/config/netbsd-d.c   | 41 +
 gcc/config/t-netbsd |  4 
 gcc/d/d-system.h|  3 ++-
 libphobos/configure.tgt |  2 ++
 5 files changed, 51 insertions(+), 1 deletion(-)
 create mode 100644 gcc/config/netbsd-d.c

diff --git a/gcc/config.gcc b/gcc/config.gcc
index a189cb19f63..c5d3044b017 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -839,6 +839,7 @@ case ${target} in
   tm_p_file="${tm_p_file} netbsd-protos.h"
   tmake_file="t-netbsd t-slibgcc"
   extra_objs="${extra_objs} netbsd.o"
+  d_target_objs="${d_target_objs} netbsd-d.o"
   gas=yes
   gnu_ld=yes
   use_gcc_stdint=wrap
@@ -847,6 +848,7 @@ case ${target} in
   esac
   nbsd_tm_file="netbsd.h netbsd-stdint.h netbsd-elf.h"
   default_use_cxa_atexit=yes
+  target_has_targetdm=yes
   ;;
 *-*-openbsd*)
   tmake_file="t-openbsd"
diff --git a/gcc/config/netbsd-d.c b/gcc/config/netbsd-d.c
new file mode 100644
index 000..8593f8bd430
--- /dev/null
+++ b/gcc/config/netbsd-d.c
@@ -0,0 +1,41 @@
+/* Functions for generic NetBSD as target machine for GNU D compiler.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "tm.h"
+#include "tree.h"
+#include "varasm.h"
+#include "netbsd-protos.h"
+#include "tm_p.h"
+#include "d/d-target.h"
+#include "d/d-target-def.h"
+
+static void
+netbsd_d_os_builtins (void)
+{
+  d_add_builtin_version ("Posix");
+  d_add_builtin_version ("NetBSD");
+}
+
+#undef TARGET_D_OS_VERSIONS
+#define TARGET_D_OS_VERSIONS netbsd_d_os_builtins
+
+struct gcc_targetdm targetdm = TARGETDM_INITIALIZER;
diff --git a/gcc/config/t-netbsd b/gcc/config/t-netbsd
index 4626e963ebf..716a94f86c6 100644
--- a/gcc/config/t-netbsd
+++ b/gcc/config/t-netbsd
@@ -19,3 +19,7 @@
 netbsd.o: $(srcdir)/config/netbsd.c
$(COMPILE) $<
$(POSTCOMPILE)
+
+netbsd-d.o: $(srcdir)/config/netbsd-d.c
+   $(COMPILE) $<
+   $(POSTCOMPILE)
diff --git a/gcc/d/d-system.h b/gcc/d/d-system.h
index cd59b827812..c32825d4ad1 100644
--- a/gcc/d/d-system.h
+++ b/gcc/d/d-system.h
@@ -24,7 +24,8 @@
 
 /* Used by the dmd front-end to determine if we have POSIX-style IO.  */
 #define POSIX (__linux__ || __GLIBC__ || __gnu_hurd__ || __APPLE__ \
-  || __FreeBSD__ || __OpenBSD__ || __DragonFly__ || __sun)
+  || __FreeBSD__ || __NetBSD__ || __OpenBSD__ || __DragonFly__ \
+  || __sun)
 
 /* Forward assert invariants to gcc_assert.  */
 #undef assert
diff --git a/libphobos/configure.tgt b/libphobos/configure.tgt
index 2b2a9746811..0471bfd816b 100644
--- a/libphobos/configure.tgt
+++ b/libphobos/configure.tgt
@@ -30,6 +30,8 @@ case "${target}" in
;;
   x86_64-*-linux* | i?86-*-linux*)
;;
+  x86_64-*-netbsd* | 

Re: Mitigation for PR target/88469 on arm-based systems bootstrapping with gcc-6/7/8

[Richard Earnshaw (lists)]

On 22/01/2019 15:46, Jakub Jelinek wrote:
> On Tue, Jan 22, 2019 at 02:43:38PM +, Richard Earnshaw (lists) wrote:
>>  PR target/88469
>>  * profile-count.h (profile_count): Add dummy file with 64-bit alignment
>>  on arm-based systems using gcc-6/7/8.
>>
> 
>> diff --git a/gcc/profile-count.h b/gcc/profile-count.h
>> index c83fa3beb8f..ddfda2cddf4 100644
>> --- a/gcc/profile-count.h
>> +++ b/gcc/profile-count.h
>> @@ -645,6 +645,12 @@ private:
>>  
>>uint64_t m_val : n_bits;
>>enum profile_quality m_quality : 3;
>> +#if defined (__arm__) && (__GNUC__ >= 6 && __GNUC__ <= 8)
>> +  /* Work-around for PR88469.  A bug in the gcc-6/7/8 PCS layout code
>> + incorrectly detects the alignment of a structure where the only
>> + 64-bit aligned element is a bit-field.  */
>> +  uint64_t m_force_alignment;
>> +#endif
> 
> Adding another member is very costly.
> Can't you do something like:
> #if defined (__arm__) && (__GNUC__ >= 6 && __GNUC__ <= 8)
> #define UINT64_BITFIELD_ALIGN \
>   __attribute__((__aligned__ (__alignof__ (uint64_t
> #else
> #define UINT64_BITFIELD_ALIGN
> #endif
> and use
>   uint64_t m_val UINT64_BITFIELD_ALIGN : n_bits;
> ?
> 
>   Jakub
> 

Close.  The alignment has to come before the m_val...


PR target/88469
* profile-count.h (profile_count): Force alignment of
m_val when building on Arm-based systems with gcc-6/7/8.

OK for trunk/gcc-8?

R.
diff --git a/gcc/profile-count.h b/gcc/profile-count.h
index 06564ddf4bd..63076476400 100644
--- a/gcc/profile-count.h
+++ b/gcc/profile-count.h
@@ -649,9 +649,17 @@ public:
 private:
   static const uint64_t uninitialized_count = ((uint64_t) 1 << n_bits) - 1;
 
-  uint64_t m_val : n_bits;
+#if defined (__arm__) && (__GNUC__ >= 6 && __GNUC__ <= 8)
+  /* Work-around for PR88469.  A bug in the gcc-7/8 PCS layout code
+ incorrectly detects the alignment of a structure where the only
+ 64-bit aligned object is a bit-field.  We force the alignment
+ of the entire field to mitigate this.  */
+#define UINT64_BITFIELD_ALIGN __attribute__((aligned(8)))
+#else
+#define UINT64_BITFIELD_ALIGN
+#endif
+  uint64_t UINT64_BITFIELD_ALIGN m_val : n_bits;
   enum profile_quality m_quality : 3;
-
   /* Return true if both values can meaningfully appear in single function
  body.  We have either all counters in function local or global, otherwise
  operations between them are not really defined well.  */

[PATCH] Fix broken filename for .gcda files starting with '..' (PR gcov-profile/88994).

[Martin Liška]

Hi.

The patch fixes path creation for situations when a filename.gcda
starts with e.g. '..'.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
I'll install the patch in few days.

Thanks,
Martin

gcc/ChangeLog:

2019-01-23  Martin Liska  

PR gcov-profile/88994
* gcov-io.c (mangle_path): Do not allocate a bigger buffer,
result will be always smaller or equal to the original.
* gcov.c (mangle_name): Fix else branch where we should
also copy to PTR and shift the pointer.
---
 gcc/gcov-io.c |  2 +-
 gcc/gcov.c| 16 +---
 2 files changed, 10 insertions(+), 8 deletions(-)


diff --git a/gcc/gcov-io.c b/gcc/gcov-io.c
index e3b1c5ce7ae..1f8ac375931 100644
--- a/gcc/gcov-io.c
+++ b/gcc/gcov-io.c
@@ -547,7 +547,7 @@ mangle_path (char const *base)
   /* Convert '/' to '#', convert '..' to '^',
  convert ':' to '~' on DOS based file system.  */
   const char *probe;
-  char *buffer = (char *)xmalloc (strlen (base) + 10);
+  char *buffer = (char *)xmalloc (strlen (base) + 1);
   char *ptr = buffer;
 
 #if HAVE_DOS_BASED_FILE_SYSTEM
diff --git a/gcc/gcov.c b/gcc/gcov.c
index b8ce1ee0e09..cb6bc7ef85f 100644
--- a/gcc/gcov.c
+++ b/gcc/gcov.c
@@ -2520,6 +2520,9 @@ make_gcov_file_name (const char *input_name, const char *src_name)
   return result;
 }
 
+/* Mangle BASE name, copy it at the beginning of PTR buffer and
+   return address of the \0 character of the buffer.  */
+
 static char *
 mangle_name (char const *base, char *ptr)
 {
@@ -2527,14 +2530,13 @@ mangle_name (char const *base, char *ptr)
 
   /* Generate the source filename part.  */
   if (!flag_preserve_paths)
-{
-  base = lbasename (base);
-  len = strlen (base);
-  memcpy (ptr, base, len);
-  ptr += len;
-}
+base = lbasename (base);
   else
-ptr = mangle_path (base);
+base = mangle_path (base);
+
+  len = strlen (base);
+  memcpy (ptr, base, len);
+  ptr += len;
 
   return ptr;
 }

[PATCH] Remove a barrier when EDGE_CROSSING is remoed (PR lto/88858).

[Martin Liška]

Hi.

The PR is about a verification error where we have a FALLTHRU edge that
contains barrier instruction. The instruction is created during bbpart pass
in add_labels_and_missing_jumps (emit_barrier_after_bb). At that time, the
basic blocks live in a different partition (hot,cold). Later then the edge
is redirected and the basic blocks are in a same partition. The code in 
cfg_layout_redirect_edge_and_branch correctly sets FALLTHRU flag, but forgets
to remove the created barrier.

The problem is seen during build of Firefox with LTO+PGO in one LTRANS unit.

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

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2019-01-23  Martin Liska  

PR lto/88858
* cfgrtl.c (cfg_layout_redirect_edge_and_branch): When BB_FOOTER
contains a BARRIER and we're removing a crossing jump, remove
the barrier.
---
 gcc/cfgrtl.c | 19 +++
 1 file changed, 19 insertions(+)


diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c
index 172bdf585d0..5dd316efb63 100644
--- a/gcc/cfgrtl.c
+++ b/gcc/cfgrtl.c
@@ -4396,6 +4396,25 @@ cfg_layout_redirect_edge_and_branch (edge e, basic_block dest)
 	  	 "Removing crossing jump while redirecting edge form %i to %i\n",
 		 e->src->index, dest->index);
   delete_insn (BB_END (src));
+
+  /* Unlink a BARRIER that can be still in BB_FOOTER.  */
+  rtx_insn *insn = BB_FOOTER (src);
+  while (insn != NULL_RTX && !BARRIER_P (insn))
+	insn = NEXT_INSN (insn);
+
+  if (insn != NULL_RTX)
+	{
+	  if (insn == BB_FOOTER (src))
+	BB_FOOTER (src) = NULL;
+	  else
+	{
+	  if (PREV_INSN (insn))
+		SET_NEXT_INSN (PREV_INSN (insn)) = NEXT_INSN (insn);
+	  if (NEXT_INSN (insn))
+		SET_PREV_INSN (NEXT_INSN (insn)) = PREV_INSN (insn);
+	}
+	}
+
   e->flags |= EDGE_FALLTHRU;
 }
 

Re: UNSPEC related notes and libbacktrace

[Martin Liška]

On 1/22/19 10:50 AM, Gerald Pfeifer wrote:
> I've been seeing the following in my testsuite runs which I didn't
> get a month or so ago.  Are these due to your changes, Jakub, or 
> some changes around libbacktrace?

I can confirm that and I've just created a PR for it:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89006

Martin

Re: [PATCH] Fix gimple-loop-interchange ICE (PR tree-optimization/88964)

[Richard Biener]

On Tue, 22 Jan 2019, Jakub Jelinek wrote:

> Hi!
> 
> SCEV can analyze not just integral/pointer IVs, but (scalar) float ones as
> well.  Calling build_int_cst on such types results in ICE, build_zero_cst
> works.  Though the loop invariant PHI IVs, if we represent them as using
> +0.0 step, aren't correct if honoring signed zeros, as + 0.0 will make
> a +0.0 out of -0.0.
> 
> Bootstrapped/regtested on {x86_64,i686,powerpc64{,le}}-linux, ok for trunk?

OK.

Richard.

> 2019-01-22  Jakub Jelinek  
> 
>   PR tree-optimization/88964
>   * gimple-loop-interchange.cc (loop_cand::analyze_induction_var): Use
>   build_zero_cst instead of build_int_cst.  Return false for loop
>   invariants which honor signed zeros.
> 
>   * gfortran.dg/pr88964.f90: New test.
> 
> --- gcc/gimple-loop-interchange.cc.jj 2019-01-01 12:37:17.416970701 +0100
> +++ gcc/gimple-loop-interchange.cc2019-01-22 11:34:42.303796570 +0100
> @@ -688,11 +688,16 @@ loop_cand::analyze_induction_var (tree v
>/* Var is loop invariant, though it's unlikely to happen.  */
>if (tree_does_not_contain_chrecs (chrec))
>  {
> +  /* Punt on floating point invariants if honoring signed zeros,
> +  representing that as + 0.0 would change the result if init
> +  is -0.0.  */
> +  if (HONOR_SIGNED_ZEROS (chrec))
> + return false;
>struct induction *iv = XCNEW (struct induction);
>iv->var = var;
>iv->init_val = init;
>iv->init_expr = chrec;
> -  iv->step = build_int_cst (TREE_TYPE (chrec), 0);
> +  iv->step = build_zero_cst (TREE_TYPE (chrec));
>m_inductions.safe_push (iv);
>return true;
>  }
> --- gcc/testsuite/gfortran.dg/pr88964.f90.jj  2019-01-22 11:54:46.046154065 
> +0100
> +++ gcc/testsuite/gfortran.dg/pr88964.f90 2019-01-22 11:54:23.172526590 
> +0100
> @@ -0,0 +1,57 @@
> +! PR tree-optimization/88964
> +! { dg-do compile }
> +! { dg-options "-O3 -fno-tree-forwprop --param 
> sccvn-max-alias-queries-per-access=1" }
> +
> +MODULE pr88964
> +  INTEGER, PARAMETER :: dp=8
> +  REAL(KIND=dp) :: p, q, o
> +CONTAINS
> +  SUBROUTINE foo(a,b,c,f,h)
> +IMPLICIT NONE
> +INTEGER :: a, b, c
> +REAL(KIND=dp) :: f(b*c), h(a*c)
> +CALL bar(h)
> +CALL baz(f)
> +CALL qux(h)
> +  END SUBROUTINE foo
> +  SUBROUTINE bar(h)
> +IMPLICIT NONE
> +REAL(KIND=dp) :: h(1*1)
> +INTEGER :: r, s, t, u
> +DO u = 1,3
> +  DO t = 1,1
> +DO s = 1,3
> +  DO r = 1,1
> +h((t-1)*1+r) = h((t-1)*1+r)-p*o
> +  END DO
> +END DO
> +  END DO
> +END DO
> +  END SUBROUTINE bar
> +  SUBROUTINE baz(f)
> +IMPLICIT NONE
> +REAL(KIND=dp) :: f(3*1)
> +INTEGER :: s, t, u
> +DO u = 1,4
> +  DO t = 1,1
> +DO s = 1,3
> +  f((t-1)*3+s) = f((t-1)*3+s) - q
> +END DO
> +  END DO
> +END DO
> +  END SUBROUTINE baz
> +  SUBROUTINE qux(h)
> +IMPLICIT NONE
> +REAL(KIND=dp) :: h(1*1)
> +INTEGER :: r, s, t, u
> +DO u = 1,5
> +  DO t = 1,1
> +DO s = 1,3
> +  DO r = 1,1
> +h((t-1)*1+r) = h((t-1)*1+r)-p*o
> +  END DO
> +END DO
> +  END DO
> +END DO
> +  END SUBROUTINE qux
> +END MODULE pr88964
> 
>   Jakub
> 
> 

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

[committed][nvptx, libgomp] Fix cuMemAlloc with size zero

[Tom de Vries]

Hi,

Consider test-case:
...
int
main (void)
{
  #pragma acc parallel async
  ;
  #pragma acc parallel async
  ;
  #pragma acc wait

  return 0;
}
...

This fails with:
...
libgomp: cuMemAlloc error: invalid argument
Segmentation fault (core dumped)
...
The cuMemAlloc error is due to the fact that we're try to allocate 0 bytes.

Fix this by preventing calling map_push with size zero argument in nvptx_exec.

This also has the consequence that for the abort-1.c test-case, we end up
calling cuMemFree during map_fini for the struct cuda_map allocated in
map_init, which fails because an abort happened.  Fix this by calling
cuMemFree with CUDA_CALL_NOCHECK in cuda_map_destroy.

Committed to trunk.

Thanks,
- Tom

[nvptx, libgomp] Fix cuMemAlloc with size zero

2019-01-22  Tom de Vries  

PR target/PR88946
* plugin/plugin-nvptx.c (cuda_map_destroy): Use CUDA_CALL_NOCHECK for
cuMemFree.
(nvptx_exec): Don't call map_push if mapnum == 0.
* testsuite/libgomp.oacc-c-c++-common/pr88946.c: New test.

---
 libgomp/plugin/plugin-nvptx.c  | 48 +-
 .../testsuite/libgomp.oacc-c-c++-common/pr88946.c  | 15 +++
 2 files changed, 43 insertions(+), 20 deletions(-)

diff --git a/libgomp/plugin/plugin-nvptx.c b/libgomp/plugin/plugin-nvptx.c
index 4a67191932e..ff90b67cb86 100644
--- a/libgomp/plugin/plugin-nvptx.c
+++ b/libgomp/plugin/plugin-nvptx.c
@@ -260,7 +260,7 @@ cuda_map_destroy (struct cuda_map *map)
atexit handler (PR83795).  */
 ;
   else
-CUDA_CALL_ASSERT (cuMemFree, map->d);
+CUDA_CALL_NOCHECK (cuMemFree, map->d);
 
   free (map);
 }
@@ -1164,7 +1164,7 @@ nvptx_exec (void (*fn), size_t mapnum, void **hostaddrs, 
void **devaddrs,
   struct ptx_stream *dev_str;
   void *kargs[1];
   void *hp;
-  CUdeviceptr dp;
+  CUdeviceptr dp = 0;
   struct nvptx_thread *nvthd = nvptx_thread ();
   int warp_size = nvthd->ptx_dev->warp_size;
   const char *maybe_abort_msg = "(perhaps abort was called)";
@@ -1361,23 +1361,27 @@ nvptx_exec (void (*fn), size_t mapnum, void 
**hostaddrs, void **devaddrs,
   dims[GOMP_DIM_VECTOR]);
 }
 
-  /* This reserves a chunk of a pre-allocated page of memory mapped on both
- the host and the device. HP is a host pointer to the new chunk, and DP is
- the corresponding device pointer.  */
-  pthread_mutex_lock (_event_lock);
-  dp = map_push (dev_str, mapnum * sizeof (void *));
-  pthread_mutex_unlock (_event_lock);
-
-  GOMP_PLUGIN_debug (0, "  %s: prepare mappings\n", __FUNCTION__);
-
-  /* Copy the array of arguments to the mapped page.  */
-  hp = alloca(sizeof(void *) * mapnum);
-  for (i = 0; i < mapnum; i++)
-((void **) hp)[i] = devaddrs[i];
+  if (mapnum > 0)
+{
+  /* This reserves a chunk of a pre-allocated page of memory mapped on both
+the host and the device. HP is a host pointer to the new chunk, and DP 
is
+the corresponding device pointer.  */
+  pthread_mutex_lock (_event_lock);
+  dp = map_push (dev_str, mapnum * sizeof (void *));
+  pthread_mutex_unlock (_event_lock);
+
+  GOMP_PLUGIN_debug (0, "  %s: prepare mappings\n", __FUNCTION__);
+
+  /* Copy the array of arguments to the mapped page.  */
+  hp = alloca(sizeof(void *) * mapnum);
+  for (i = 0; i < mapnum; i++)
+   ((void **) hp)[i] = devaddrs[i];
+
+  /* Copy the (device) pointers to arguments to the device */
+  CUDA_CALL_ASSERT (cuMemcpyHtoD, dp, hp,
+   mapnum * sizeof (void *));
+}
 
-  /* Copy the (device) pointers to arguments to the device */
-  CUDA_CALL_ASSERT (cuMemcpyHtoD, dp, hp,
-   mapnum * sizeof (void *));
   GOMP_PLUGIN_debug (0, "  %s: kernel %s: launch"
 " gangs=%u, workers=%u, vectors=%u\n",
 __FUNCTION__, targ_fn->launch->fn, dims[GOMP_DIM_GANG],
@@ -1422,7 +1426,8 @@ nvptx_exec (void (*fn), size_t mapnum, void **hostaddrs, 
void **devaddrs,
 
   CUDA_CALL_ASSERT (cuEventRecord, *e, dev_str->stream);
 
-  event_add (PTX_EVT_KNL, e, (void *)dev_str, 0);
+  if (mapnum > 0)
+   event_add (PTX_EVT_KNL, e, (void *)dev_str, 0);
 }
 #else
   r = CUDA_CALL_NOCHECK (cuCtxSynchronize, );
@@ -1439,7 +1444,10 @@ nvptx_exec (void (*fn), size_t mapnum, void **hostaddrs, 
void **devaddrs,
 #ifndef DISABLE_ASYNC
   if (async < acc_async_noval)
 #endif
-map_pop (dev_str);
+{
+  if (mapnum > 0)
+   map_pop (dev_str);
+}
 }
 
 void * openacc_get_current_cuda_context (void);
diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/pr88946.c 
b/libgomp/testsuite/libgomp.oacc-c-c++-common/pr88946.c
new file mode 100644
index 000..ad56ded1d2b
--- /dev/null
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/pr88946.c
@@ -0,0 +1,15 @@
+/* { dg-do run } */
+
+int
+main (void)
+{
+  #pragma acc parallel async
+  ;
+
+  #pragma acc parallel async
+  ;
+
+  #pragma acc wait
+
+  return 0;
+}

Re: [patch, fortran] Move some array packing to front end

[Richard Biener]

On Tue, Jan 22, 2019 at 9:59 PM Thomas Koenig  wrote:
>
> Hello world,
>
> the attached patch moves the packing / unpacking of arrays to the front
> end when optimizing, but not for size.
>
> Rationale: internal_pack and internal_unpack are opaque to the compiler.
> This can lead to a lot of information loss for inlining and inter-
> procedural optimization, and in extreme cases can lead to huge
> slowdowns.
>
> I don't want to do this for -Os or for -O0.  -Os because I want to avoid
> size increases, and -O0 for several reasons: The current method works
> well, if there should turn out to be a bug still hiding in this code I
> want to at least have "works with -O0" in the bug report, and finally
> I did not want to rewrite all test cases.
>
> Because run test cases cycle through a lot of optimization options,
> I had to split some of them up - test the pattern matches with -O0, test
> for run time correctness under all the options.
>
> I have regression-tested this.  I would, however, prefer if some people
> could run this patch against their non-testsuite code and report
> any problems that this may introduce. So, if you can spare the time
> and the cycles, that would be great.
>
> The nice thing about this kind of patch is that, if this does not
> work for a certain condition, it is usually straightforward to
> check for the condition and then simply not do the optimization.
>
> So, comments?  Bug reports?  OK for trunk if nobody has come
> up with a bug in the next few days?

Note for this kind of changes it is approprate to wait for stage1 to open.

Of course since you're not release critical you can override this
suggestion as you please.

Richard.

> Regards
>
> Thomas
>
> 2019-01-22  Thomas Koenig  
>
>  PR fortran/88821
>  * expr.c (gfc_is_simply_contiguous): Return true for
>  an EXPR_ARRAY.
>  * trans-array.c (is_pointer): New function.
>  (gfc_conv_array_parameter): Call gfc_conv_subref_array_arg
>  when not optimizing and not optimizing for size if the formal
>  arg is passed by reference.
>  * trans-expr.c (gfc_conv_subref_array_arg): Add arguments
>  fsym, proc_name and sym.  Add run-time warning for temporary
>  array creation.  Wrap argument if passing on an optional
>  argument to an optional argument.
>  * trans.h (gfc_conv_subref_array_arg): Add optional arguments
>  fsym, proc_name and sym to prototype.
>
> 2019-01-22  Thomas Koenig  
>
>  PR fortran/88821
>  * gfortran.dg/alloc_comp_auto_array_3.f90: Add -O0 to dg-options
>  to make sure the test for internal_pack is retained.
>  * gfortran.dg/assumed_type_2.f90: Split compile and run time
>  tests into this and
>  * gfortran.dg/assumed_type_2a.f90: New file.
>  * gfortran.dg/c_loc_test_22.f90: Likewise.
>  * gfortran.dg/contiguous_3.f90: Likewise.
>  * gfortran.dg/internal_pack_11.f90: Likewise.
>  * gfortran.dg/internal_pack_12.f90: Likewise.
>  * gfortran.dg/internal_pack_16.f90: Likewise.
>  * gfortran.dg/internal_pack_17.f90: Likewise.
>  * gfortran.dg/internal_pack_18.f90: Likewise.
>  * gfortran.dg/internal_pack_4.f90: Likewise.
>  * gfortran.dg/internal_pack_5.f90: Add -O0 to dg-options
>  to make sure the test for internal_pack is retained.
>  * gfortran.dg/internal_pack_6.f90: Split compile and run time
>  tests into this and
>  * gfortran.dg/internal_pack_6a.f90: New file.
>  * gfortran.dg/internal_pack_8.f90: Likewise.
>  * gfortran.dg/missing_optional_dummy_6: Split compile and run time
>  tests into this and
>  * gfortran.dg/missing_optional_dummy_6a.f90: New file.
>  * gfortran.dg/no_arg_check_2.f90: Split compile and run time tests
>  into this and
>  * gfortran.dg/no_arg_check_2a.f90: New file.
>  * gfortran.dg/typebound_assignment_5.f90: Split compile and run
> time
>  tests into this and
>  * gfortran.dg/typebound_assignment_5a.f90: New file.
>  * gfortran.dg/typebound_assignment_6.f90: Split compile and run
> time
>  tests into this and
>  * gfortran.dg/typebound_assignment_6a.f90: New file.
>  * gfortran.dg/internal_pack_19.f90: New file.
>  * gfortran.dg/internal_pack_20.f90: New file.

[committed][nvptx, libgomp] Fix assert (!s->map->active) in map_fini

[Tom de Vries]

Hi,

There are currently two situations where this assert triggers:
...
libgomp/plugin/plugin-nvptx.c: map_fini: Assertion `!s->map->active' failed.
...

First, in abort-1.c, a parallel region triggering an abort:
...
int
main (void)
{
  #pragma acc parallel
  abort ();

  return 0;
}
...

The abort is detected in nvptx_exec as the CUDA_ERROR_ILLEGAL_INSTRUCTION
return status of the cuStreamSynchronize call after kernel launch, which is
then handled by calling non-returning function GOMP_PLUGIN_fatal.
Consequently, the map_pop in nvptx_exec that in case of cuStreamSynchronize
success would remove or inactive the element added by the map_push earlier in
nvptx_exec, does not trigger.  With the element no longer active, but still
marked active and a member of s->map,  we run into the assert during
GOMP_OFFLOAD_fini_device, which is triggered from atexit handler
gomp_target_fini (which is triggered by the GOMP_PLUGIN_fatal mentioned above
calling exit).

Second, in pr88941.c, an async parallel region without wait:
...
int
main (void)
{
  #pragma acc parallel async
  ;

  /* no #pragma acc wait */
  return 0;
}
...

Because nvptx_exec is handling an async region, it does not call map_pop for
the element added by map_push, but schedules an kernel execution completion
event to call map_pop.  Again, we run into the assert during
GOMP_OFFLOAD_fini_device, which is triggered from atexit handler
gomp_target_fini, but the exit in this case is triggered by returning from main.
So either the kernel is still running, or the kernel has completed but the
corresponding event that is supposed to call map_pop is stuck in the event
queue, waiting for an event_gc.

Fix this by removing the assert, and skipping the freeing of device memory if
the map is still marked active (though in the async case, this is more a
workaround than an fix).

Committed to trunk.

Thanks,
- Tom

[nvptx, libgomp] Fix assert (!s->map->active) in map_fini

2019-01-22  Tom de Vries  

PR target/88941
PR target/88939
* plugin/plugin-nvptx.c (cuda_map_destroy): Handle map->active case.
(map_fini): Remove "assert (!s->map->active)".
* testsuite/libgomp.oacc-c-c++-common/pr88941.c: New test.

---
 libgomp/plugin/plugin-nvptx.c  | 27 --
 .../testsuite/libgomp.oacc-c-c++-common/pr88941.c  | 15 
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/libgomp/plugin/plugin-nvptx.c b/libgomp/plugin/plugin-nvptx.c
index a220560b189..4a67191932e 100644
--- a/libgomp/plugin/plugin-nvptx.c
+++ b/libgomp/plugin/plugin-nvptx.c
@@ -237,7 +237,31 @@ cuda_map_create (size_t size)
 static void
 cuda_map_destroy (struct cuda_map *map)
 {
-  CUDA_CALL_ASSERT (cuMemFree, map->d);
+  if (map->active)
+/* Possible reasons for the map to be still active:
+   - the associated async kernel might still be running.
+   - the associated async kernel might have finished, but the
+ corresponding event that should trigger the pop_map has not been
+processed by event_gc.
+   - the associated sync kernel might have aborted
+
+   The async cases could happen if the user specified an async region
+   without adding a corresponding wait that is guaranteed to be executed
+   (before returning from main, or in an atexit handler).
+   We do not want to deallocate a device pointer that is still being
+   used, so skip it.
+
+   In the sync case, the device pointer is no longer used, but deallocating
+   it using cuMemFree will not succeed, so skip it.
+
+   TODO: Handle this in a more constructive way, by f.i. waiting for 
streams
+   to finish before de-allocating them (PR88981), or by ensuring the CUDA
+   lib atexit handler is called before rather than after the libgomp plugin
+   atexit handler (PR83795).  */
+;
+  else
+CUDA_CALL_ASSERT (cuMemFree, map->d);
+
   free (map);
 }
 
@@ -268,7 +292,6 @@ static bool
 map_fini (struct ptx_stream *s)
 {
   assert (s->map->next == NULL);
-  assert (!s->map->active);
 
   cuda_map_destroy (s->map);
 
diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/pr88941.c 
b/libgomp/testsuite/libgomp.oacc-c-c++-common/pr88941.c
new file mode 100644
index 000..e31bb527df3
--- /dev/null
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/pr88941.c
@@ -0,0 +1,15 @@
+/* { dg-do run } */
+
+#include 
+
+int
+main (void)
+{
+
+#pragma acc parallel async
+  ;
+
+  /* no #pragma acc wait */
+  return 0;
+}
+

[committed][nvptx, libgomp] Fix map_push

[Tom de Vries]

Hi,

The map field of a struct ptx_stream is a FIFO.  The FIFO is implemented as a
single linked list, with pop-from-the-front semantics.

The function map_pop pops an element, either by:
- deallocating the element, if there is more than one element
- or marking the element inactive, if there's only one element

The responsibility of map_push is to push an element to the back, as well as
selecting the element to push, by:
- allocating an element, or
- reusing the element at the front if inactive and big enough, or
- dropping the element at the front if inactive and not big enough, and
  allocating one that's big enough

The current implemention gets at least the first and most basic scenario wrong:

> map = cuda_map_create (size);

We create an element, and assign it to map.

> for (t = s->map; t->next != NULL; t = t->next)
>   ;

We determine the last element in the fifo.

> t->next = map;

We append the new element.

> s->map = map;

But here, we throw away the rest of the FIFO, and declare the FIFO to be just
the new element.

This problem causes the test-case asyncwait-1.c to fail intermittently on some
systems.  The pr87835.c test-case added here is a a minimized and modified
version of asyncwait-1.c (avoiding the kernel construct) that is more likely to
fail.

Fix this by rewriting map_pop more robustly, by:
- seperating the function in two phases: select element, push element
- when reusing or dropping an element, making sure that the element is cleanly
  popped from the queue
- rewriting the push element part in such a way that it can handle all cases
  without needing if statements, such that each line is exercised for each of
  the three cases.

Committed to trunk.

Thanks,
- Tom

[nvptx, libgomp] Fix map_push

2019-01-22  Tom de Vries  

PR target/87835
* plugin/plugin-nvptx.c (map_push): Fix adding of allocated element.
* testsuite/libgomp.oacc-c-c++-common/pr87835.c: New test.

---
 libgomp/plugin/plugin-nvptx.c  | 47 +---
 .../testsuite/libgomp.oacc-c-c++-common/pr87835.c  | 62 ++
 2 files changed, 91 insertions(+), 18 deletions(-)

diff --git a/libgomp/plugin/plugin-nvptx.c b/libgomp/plugin/plugin-nvptx.c
index dd2bcf3083f..a220560b189 100644
--- a/libgomp/plugin/plugin-nvptx.c
+++ b/libgomp/plugin/plugin-nvptx.c
@@ -296,35 +296,46 @@ map_pop (struct ptx_stream *s)
 static CUdeviceptr
 map_push (struct ptx_stream *s, size_t size)
 {
-  struct cuda_map *map = NULL, *t = NULL;
+  struct cuda_map *map = NULL;
+  struct cuda_map **t;
 
   assert (s);
   assert (s->map);
 
-  /* Each PTX stream requires a separate data region to store the
- launch arguments for cuLaunchKernel.  Allocate a new
- cuda_map and push it to the end of the list.  */
+  /* Select an element to push.  */
   if (s->map->active)
+map = cuda_map_create (size);
+  else
 {
-  map = cuda_map_create (size);
+  /* Pop the inactive front element.  */
+  struct cuda_map *pop = s->map;
+  s->map = pop->next;
+  pop->next = NULL;
 
-  for (t = s->map; t->next != NULL; t = t->next)
-   ;
+  if (pop->size < size)
+   {
+ cuda_map_destroy (pop);
 
-  t->next = map;
-}
-  else if (s->map->size < size)
-{
-  cuda_map_destroy (s->map);
-  map = cuda_map_create (size);
+ map = cuda_map_create (size);
+   }
+  else
+   map = pop;
 }
-  else
-map = s->map;
 
-  s->map = map;
-  s->map->active = true;
+  /* Check that the element is as expected.  */
+  assert (map->next == NULL);
+  assert (!map->active);
+
+  /* Mark the element active.  */
+  map->active = true;
+
+  /* Push the element to the back of the list.  */
+  for (t = >map; (*t) != NULL; t = &(*t)->next)
+;
+  assert (t != NULL && *t == NULL);
+  *t = map;
 
-  return s->map->d;
+  return map->d;
 }
 
 /* Target data function launch information.  */
diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/pr87835.c 
b/libgomp/testsuite/libgomp.oacc-c-c++-common/pr87835.c
new file mode 100644
index 000..310a485e74f
--- /dev/null
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/pr87835.c
@@ -0,0 +1,62 @@
+/* { dg-do run { target openacc_nvidia_accel_selected } } */
+/* { dg-additional-options "-lcuda" } */
+
+#include 
+#include 
+#include "cuda.h"
+
+#include 
+
+#define n 128
+
+int
+main (void)
+{
+  CUresult r;
+  CUstream stream1;
+  int N = n;
+  int a[n];
+  int b[n];
+  int c[n];
+
+  acc_init (acc_device_nvidia);
+
+  r = cuStreamCreate (, CU_STREAM_NON_BLOCKING);
+  if (r != CUDA_SUCCESS)
+{
+  fprintf (stderr, "cuStreamCreate failed: %d\n", r);
+  abort ();
+}
+
+  acc_set_cuda_stream (1, stream1);
+
+  for (int i = 0; i < n; i++)
+{
+  a[i] = 3;
+  c[i] = 0;
+}
+
+#pragma acc data copy (a, b, c) copyin (N)
+  {
+#pragma acc parallel async (1)
+;
+
+#pragma acc parallel async (1) num_gangs (320)
+#pragma loop gang
+for (int ii = 0; ii <