Re: [PATCH] Add LVAL argument to c_fully_fold* and propagate it through (PR c/66618, PR c/69960)

2017-12-25 Thread Tom de Vries 

On 11/09/2017 11:54 AM, Jakub Jelinek wrote:

On Wed, Nov 08, 2017 at 06:57:55PM +0100, Jakub Jelinek wrote:

On Wed, Nov 08, 2017 at 06:51:34PM +0100, Marek Polacek wrote:

Ok, so like this if it passes bootstrap/regtest?

Changes from the last patch:
1) false instead of lval for COMPOUND_EXPR and *COND_EXPR op1/op2


So...


Oops, I've hand-edited it in the patch and then regenerated the patch
after doing there more changes.

Here is updated patch with that in again:


FYI, bootstrapped/regtested on x86_64-linux and i686-linux successfully.



This caused: PR83590 - [nvptx] openacc reduction C regressions ( 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83590 ).


Thanks,
- Tom

Re: [PATCH] PR Fortran/83548 -- a LOGICAL fix

2017-12-25 Thread Steve Kargl 
On Mon, Dec 25, 2017 at 11:12:49PM +0100, Bernhard Reutner-Fischer wrote:
> On 25 December 2017 20:45:40 CET, Steve Kargl 
>  wrote:
> >The attach patch fixes a problem when a LOGICAL subprogram
> >appears as the first element in an array constructor, which
> >is interpreted as a LOGICAL type-spec, which fails because
> >the argument is of type LOGICAL instead of INTEGER.
> >
> >Regression tested on i686-*-freebsd and x86_64-*-freebsd.
> >
> >OK to commit?
> 
> +   p = [real(kind=4) :: x,  y]
> +   q = [real(kind=8) :: x,  y]
> +   if (any(p .ne. r2)) call abort
> +   if (any(q .ne. r3)) call aborts
> +end program foo
> 
> aborts? abort? 
> 

Whoops.  Thanks for noticing.  The test is compile-only,
so I missed the typo during testing.

-- 
Steve

Re: [PATCH] PR Fortran/83548 -- a LOGICAL fix

2017-12-25 Thread Bernhard Reutner-Fischer 
On 25 December 2017 20:45:40 CET, Steve Kargl 
 wrote:
>The attach patch fixes a problem when a LOGICAL subprogram
>appears as the first element in an array constructor, which
>is interpreted as a LOGICAL type-spec, which fails because
>the argument is of type LOGICAL instead of INTEGER.
>
>Regression tested on i686-*-freebsd and x86_64-*-freebsd.
>
>OK to commit?

+   p = [real(kind=4) :: x,  y]
+   q = [real(kind=8) :: x,  y]
+   if (any(p .ne. r2)) call abort
+   if (any(q .ne. r3)) call aborts
+end program foo

aborts? abort? 

Thanks,

[v3 PATCH] Make optional conditionally trivially_{copy,move}_{constructible,assignable}

2017-12-25 Thread Ville Voutilainen 
In the midst of the holiday season, the king and ruler of all elves, otherwise
known as The Elf, was told by little elves that users are complaining how
stlstl and libc++ make optional's copy and move operations conditionally
trivial, but libstdc++ doesn't. This made The Elf fairly angry, and he spoke
"this will not stand".

Tested on Linux-PPC64. The change is an ABI break due to changing
optional to a trivially copyable type. It's perhaps
better to get that ABI break in now rather than later.

So here you go (ho ho ho):

2017-12-25  Ville Voutilainen  

Make optional conditionally
trivially_{copy,move}_{constructible,assignable}
* include/std/optional (_Optional_payload): Fix the comment in
the class head and turn into a primary and one specialization.
(_Optional_payload::_M_engaged): Strike the NSDMI.
(_Optional_payload<_Tp, false>::operator=(const _Optional_payload&)):
New.
(_Optional_payload<_Tp, false>::operator=(_Optional_payload&&)):
Likewise.
(_Optional_payload<_Tp, false>::_M_get): Likewise.
(_Optional_payload<_Tp, false>::_M_reset): Likewise.
(_Optional_base_impl): Likewise.
(_Optional_base): Turn into a primary and three specializations.
(optional(nullopt)): Change the base init.
* testsuite/20_util/optional/assignment/8.cc: New.
* testsuite/20_util/optional/cons/trivial.cc: Likewise.
* testsuite/20_util/optional/cons/value_neg.cc: Adjust.
diff --git a/libstdc++-v3/include/std/optional 
b/libstdc++-v3/include/std/optional
index e017eed..9d1e625 100644
--- a/libstdc++-v3/include/std/optional
+++ b/libstdc++-v3/include/std/optional
@@ -100,15 +100,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   // Payload for constexpr optionals.
   template ::value
- && is_trivially_move_constructible<_Tp>::value,
-   bool /*_ShouldProvideDestructor*/ =
+   bool /*_HasTrivialDestructor*/ =
  is_trivially_destructible<_Tp>::value>
 struct _Optional_payload
 {
   constexpr _Optional_payload()
-   : _M_empty() {}
+   : _M_empty(), _M_engaged(false) {}
 
   template
   constexpr _Optional_payload(in_place_t, _Args&&... __args)
@@ -131,7 +128,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   {}
 
   constexpr _Optional_payload(__ctor_tag)
-   : _M_empty()
+   : _M_empty(), _M_engaged(false)
   {}
 
   constexpr _Optional_payload(__ctor_tag, _Tp&& __other)
@@ -161,12 +158,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   _Empty_byte _M_empty;
   _Stored_type _M_payload;
   };
-  bool _M_engaged = false;
+  bool _M_engaged;
 };
 
-  // Payload for non-constexpr optionals with non-trivial destructor.
+  // Payload for optionals with non-trivial destructor.
   template 
-struct _Optional_payload<_Tp, false, false>
+struct _Optional_payload<_Tp, false>
 {
   constexpr _Optional_payload()
: _M_empty() {}
@@ -203,6 +200,39 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
  this->_M_construct(std::move(__other._M_payload));
   }
 
+  _Optional_payload&
+  operator=(const _Optional_payload& __other)
+  {
+if (this->_M_engaged && __other._M_engaged)
+  this->_M_get() = __other._M_get();
+else
+ {
+   if (__other._M_engaged)
+ this->_M_construct(__other._M_get());
+   else
+ this->_M_reset();
+ }
+
+return *this;
+  }
+
+  _Optional_payload&
+  operator=(_Optional_payload&& __other)
+  noexcept(__and_,
+ is_nothrow_move_assignable<_Tp>>())
+  {
+   if (this->_M_engaged && __other._M_engaged)
+ this->_M_get() = std::move(__other._M_get());
+   else
+ {
+   if (__other._M_engaged)
+ this->_M_construct(std::move(__other._M_get()));
+   else
+ this->_M_reset();
+ }
+   return *this;
+  }
+
   using _Stored_type = remove_const_t<_Tp>;
   struct _Empty_byte { };
   union {
@@ -226,95 +256,86 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 _Stored_type(std::forward<_Args>(__args)...);
   this->_M_engaged = true;
 }
-};
-
-  // Payload for non-constexpr optionals with trivial destructor.
-  template 
-struct _Optional_payload<_Tp, false, true>
-{
-  constexpr _Optional_payload()
-   : _M_empty() {}
-
-  template 
-  constexpr _Optional_payload(in_place_t, _Args&&... __args)
-   : _M_payload(std::forward<_Args>(__args)...),
- _M_engaged(true) {}
-
-  template
-  constexpr _Optional_payload(std::initializer_list<_Up> __il,
- _Args&&... __args)
-   : _M_payload(__il, std::forward<_Args>(__args)...),
- _M_engaged(true) {}
-  constexpr
-  _Optional_payload(bool __engaged, const _Optional_payload& __other)
-   : _Optional_payload(__other)
-

Re: [patch, lingfortran] Bug 83560 - list-directed formatting of INTEGER is missing plus on output

2017-12-25 Thread Jerry DeLisle 
On 12/25/2017 05:10 AM, Dominique d'Humières wrote:
> Dear Jerry,
> 
> The lines
> 
> +a=12.3456
> 
> and
> 
> +open(unit=10,sign='plus')
> 
> in gfortran.dg/integer_plus.f90 could probably be removed.
>

Yes, left over from some other testing I was doing

> From comment 2 in the PR (and the attached test), it seems that the reporter 
> is expecting sign=‘plus’ to apply also to namelists, which is not the case 
> with this patch.
> 
> This seems supported by (my understanding of)
> 
>> 10.11.4.2 Namelist output editing
>>
>> 1 Values in namelist output records are edited as for list-directed output 
>> (10.10.4).
> 
> Merry Christmas!
> 
> Dominique
> 
> 

What I did last night made perfect sense at the time. Now, your point well
taken. The previous write_integer suppressed leading spaces nicely for writing
repeat counts, write_decimal does not do this directly. I am going to have to be
careful we don't put plus signs on repeat counts.

Merry Christmas to you and all!

Jerry

[PATCH] PR Fortran/83548 -- a LOGICAL fix

2017-12-25 Thread Steve Kargl 
The attach patch fixes a problem when a LOGICAL subprogram
appears as the first element in an array constructor, which
is interpreted as a LOGICAL type-spec, which fails because
the argument is of type LOGICAL instead of INTEGER.

Regression tested on i686-*-freebsd and x86_64-*-freebsd.

OK to commit?

2017-12-25  Steven G. Kargl  

PR Fortran/83548
* match.c (gfc_match_type_spec): Check for LOGICAL conflict in
type-spec versus LOGICAL intrinsic subprogram.

2017-12-25  Steven G. Kargl  

PR Fortran/83548
* gfortran.dg/array_constructor_type_22.f03: New test.

-- 
Steve
Index: gcc/fortran/match.c
===
--- gcc/fortran/match.c	(revision 255997)
+++ gcc/fortran/match.c	(working copy)
@@ -2102,27 +2102,31 @@ gfc_match_type_spec (gfc_typespec *ts)
   return m;
 }
 
-  if (gfc_match ("logical") == MATCH_YES)
-{
-  ts->type = BT_LOGICAL;
-  ts->kind = gfc_default_logical_kind;
-  goto kind_selector;
-}
-
   /* REAL is a real pain because it can be a type, intrinsic subprogram,
  or list item in a type-list of an OpenMP reduction clause.  Need to
  differentiate REAL([KIND]=scalar-int-initialization-expr) from
- REAL(A,[KIND]) and REAL(KIND,A).  */
+ REAL(A,[KIND]) and REAL(KIND,A).  Logically, when this code was
+ written the use of LOGICAL as a type-spec or intrinsic subprogram 
+ was overlooked.  */
 
   m = gfc_match (" %n", name);
-  if (m == MATCH_YES && strcmp (name, "real") == 0)
+  if (m == MATCH_YES
+  && (strcmp (name, "real") == 0 || strcmp (name, "logical") == 0))
 {
   char c;
   gfc_expr *e;
   locus where;
 
-  ts->type = BT_REAL;
-  ts->kind = gfc_default_real_kind;
+  if (*name == 'r')
+	{
+	  ts->type = BT_REAL;
+	  ts->kind = gfc_default_real_kind;
+	}
+  else
+	{
+	  ts->type = BT_LOGICAL;
+	  ts->kind = gfc_default_logical_kind;
+	}
 
   gfc_gobble_whitespace ();
 
@@ -2154,7 +2158,7 @@ gfc_match_type_spec (gfc_typespec *ts)
 	  c = gfc_next_char ();
 	  if (c == '=')
 	{
-	  if (strcmp(name, "a") == 0)
+	  if (strcmp(name, "a") == 0 || strcmp(name, "l") == 0)
 		return MATCH_NO;
 	  else if (strcmp(name, "kind") == 0)
 		goto found;
@@ -2194,7 +2198,7 @@ found:
 
 	  gfc_next_char (); /* Burn the ')'. */
 	  ts->kind = (int) mpz_get_si (e->value.integer);
-	  if (gfc_validate_kind (BT_REAL, ts->kind , true) == -1)
+	  if (gfc_validate_kind (ts->type, ts->kind , true) == -1)
 	{
 	  gfc_error ("Invalid type-spec at %C");
 	  return MATCH_ERROR;
Index: gcc/testsuite/gfortran.dg/array_constructor_type_22.f03
===
--- gcc/testsuite/gfortran.dg/array_constructor_type_22.f03	(nonexistent)
+++ gcc/testsuite/gfortran.dg/array_constructor_type_22.f03	(working copy)
@@ -0,0 +1,29 @@
+! { dg-do compile }
+! PR Fortran/83548
+program foo
+
+   implicit none
+
+   logical, parameter :: t = .true., f = .false.
+   logical, parameter :: a1(2) = [t, f]
+   logical(kind=1), parameter :: a2(2) = [logical(kind=1) :: t,  f]
+   logical(kind=4), parameter :: a3(2) = [logical(kind=4) :: t,  f]
+   logical(kind=1), parameter :: a4(2) = [logical(t, 1), logical(f, 1)]
+   logical(kind=4), parameter :: a5(2) = [logical(t, 4), logical(f, 4)]
+   logical(kind=1) b(2)
+   logical(kind=4) c(2)
+
+   real, parameter :: x = 1, y = 2
+   real, parameter :: r1(2) = [x, y]
+   real(kind=4), parameter :: r2(2) = [real(kind=4) :: x,  y]
+   real(kind=8), parameter :: r3(2) = [real(kind=8) :: x,  y]
+   real(kind=4), parameter :: r4(2) = [real(x, 4), real(y, 4)]
+   real(kind=8), parameter :: r5(2) = [real(x, 8), real(y, 8)]
+   real(kind=4) p(2)
+   real(kind=8) q(2)
+
+   p = [real(kind=4) :: x,  y]
+   q = [real(kind=8) :: x,  y]
+   if (any(p .ne. r2)) call abort
+   if (any(q .ne. r3)) call aborts
+end program foo

[PATCH] sel-sched: fix zero-usefulness case in sel_rank_for_schedule (PR 83513)

2017-12-25 Thread Alexander Monakov 
Hello,

we need the following follow-up fix for priority comparison in
sel_rank_for_schedule as demonstrated by PR 83513.  Checked on
x86_64 by running a bootstrap and also checking for no regressions in
make -k check-gcc 
RUNTESTFLAGS="--target_board=unix/-fselective-scheduling/-fschedule-insns"

OK to apply?

PR rtl-optimization/83513
* sel-sched.c (sel_rank_for_schedule): Order by non-zero usefulness
before priority comparison.

diff --git a/gcc/sel-sched.c b/gcc/sel-sched.c
index c1be0136551..be3813717ba 100644
--- a/gcc/sel-sched.c
+++ b/gcc/sel-sched.c
@@ -3396,17 +3396,22 @@ sel_rank_for_schedule (const void *x, const void *y)
   else if (control_flow_insn_p (tmp2_insn) && !control_flow_insn_p (tmp_insn))
 return 1;

+  /* Prefer an expr with non-zero usefulness.  */
+  int u1 = EXPR_USEFULNESS (tmp), u2 = EXPR_USEFULNESS (tmp2);
+
+  if (u1 == 0)
+{
+  if (u2 == 0)
+u1 = u2 = 1;
+  else
+return 1;
+}
+  else if (u2 == 0)
+return -1;
+
   /* Prefer an expr with greater priority.  */
-  if (EXPR_USEFULNESS (tmp) != 0 || EXPR_USEFULNESS (tmp2) != 0)
-{
-  int p2 = EXPR_PRIORITY (tmp2) + EXPR_PRIORITY_ADJ (tmp2),
-  p1 = EXPR_PRIORITY (tmp) + EXPR_PRIORITY_ADJ (tmp);
-
-  val = p2 * EXPR_USEFULNESS (tmp2) - p1 * EXPR_USEFULNESS (tmp);
-}
-  else
-val = EXPR_PRIORITY (tmp2) - EXPR_PRIORITY (tmp)
- + EXPR_PRIORITY_ADJ (tmp2) - EXPR_PRIORITY_ADJ (tmp);
+  val = (u2 * (EXPR_PRIORITY (tmp2) + EXPR_PRIORITY_ADJ (tmp2))
+ - u1 * (EXPR_PRIORITY (tmp) + EXPR_PRIORITY_ADJ (tmp)));
   if (val)
 return val;

Re: [patch, lingfortran] Bug 83560 - list-directed formatting of INTEGER is missing plus on output

2017-12-25 Thread Dominique d'Humières 
Dear Jerry,

The lines

+a=12.3456

and

+open(unit=10,sign='plus')

in gfortran.dg/integer_plus.f90 could probably be removed.

>From comment 2 in the PR (and the attached test), it seems that the reporter 
>is expecting sign=‘plus’ to apply also to namelists, which is not the case 
>with this patch.

This seems supported by (my understanding of)

> 10.11.4.2 Namelist output editing
>
> 1 Values in namelist output records are edited as for list-directed output 
> (10.10.4).

Merry Christmas!

Dominique

Re: [patch, lingfortran] Bug 83560 - list-directed formatting of INTEGER is missing plus on output

2017-12-25 Thread Thomas Koenig 

Hi Jerry,


OK for trunk?


OK. Thanks for the patch.

And: Merry Christmas to everybody!

Thomas

RE: [PATCH] Fix various x86 avx512{bitalg,vpopcntdq,vbmi2} issues (PR target/83488)

2017-12-25 Thread Koval, Julia 
Thank you very much for fixing those issues.

Note, __builtin_ia32_vpshufbitqmb{128,256,512}_mask are implemented
> incorrectly, can somebody from Intel handle that?  The inlines in the
> intrinsic header look correct, but the builtins aren't and what's even worse
> is that the define_insns are wrong too.  According to the documentation
> and inline fn, the intrinsics have an __mmask{16,32,64} input mask and
> also __mmask{16,32,64} output mask.  The builtins use
> UHI_FTYPE_V2DI_V2DI_UHI
> USI_FTYPE_V4DI_V4DI_USI
> UQI_FTYPE_V8DI_V8DI_UQI
> types (first two are correct, the last one is wrong, should have been
> UDI_FTYPE_V8DI_V8DI_UDI), and the define_insn has:
> (set (match_operand:QI 0 ("register_operand") ("=Yk"))
> (and:QI (unspec:QI [
> (match_operand:V2DI 1 ("register_operand") ("v"))
> (match_operand:V2DI 2 ("nonimmediate_operand") ("vm"))
> ] 214)
> (match_operand:QI 3 ("register_operand") ("Yk"
> (incorrect, should use :HI result and :HI mask input),
> (set (match_operand:QI 0 ("register_operand") ("=Yk"))
> (and:QI (unspec:QI [
> (match_operand:V4DI 1 ("register_operand") ("v"))
> (match_operand:V4DI 2 ("nonimmediate_operand") ("vm"))
> ] 214)
> (match_operand:QI 3 ("register_operand") ("Yk"
> (incorrect, should use :SI result and :SI mask input),
> (set (match_operand:QI 0 ("register_operand") ("=Yk"))
> (and:QI (unspec:QI [
> (match_operand:V8DI 1 ("register_operand") ("v"))
> (match_operand:V8DI 2 ("nonimmediate_operand") ("vm"))
> ] 214)
> (match_operand:QI 3 ("register_operand") ("Yk"
> (incorrect, should use :DI result and :DI mask input).  Similarly the
> non-masked patterns, where just the result is incorrect, not the operand 3
> which doesn't exist).  I'll file a PR to track this.

I'll fix that.

Thanks,
Julia

> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> ow...@gcc.gnu.org] On Behalf Of Jakub Jelinek
> Sent: Friday, December 22, 2017 7:40 PM
> To: Kirill Yukhin ; Uros Bizjak 
> Cc: Koval, Julia ; GCC Patches  patc...@gcc.gnu.org>
> Subject: [PATCH] Fix various x86 avx512{bitalg,vpopcntdq,vbmi2} issues (PR
> target/83488)
> 
> On Fri, Dec 22, 2017 at 03:38:03PM +0300, Kirill Yukhin wrote:
> > Hello, Julia,
> > On 12 Nov 12:51, Koval, Julia wrote:
> > > Hi, this patch enables AVX512BITALG and AVX512VPOPCNTDQ instructions
> from
> https://software.intel.com/sites/default/files/managed/c5/15/architecture-
> instruction-set-extensions-programming-reference.pdf. Ok for trunk?
> > OK for trunk. I've checked it in.
> 
> Unfortunately, there are various issues in this patch as well as earlier
> vbmi2 support.
> 
> 1) as for various AVX512BITALG and AVX512VPOPCNTDQ builtins we need not
> just
> that ISA, but also AVX512VL or AVX512BW or both, these two ISAs need to be
> moved over from ix86_isa_flags2 to ix86_isa_flags.
> 2) while the PDF doesn't say that explicitly, for builtins that map to
> hw insns that don't have AVX512BW listed as CPUID, if they use (or set)
> 32-bit or 64-bit %k? mask register, we need AVX512BW for the builtin,
> because otherwise we get ICEs when LRA is trying to load (or store) the
> 32-bit or 64-bit %k? mask register.  Most of the intrin*.h headers got the
> requirements right (but see below), but not i386-builtins.def, so using
> intrin headers was fine, but using builtins directly resulted in numerous
> ICEs.
> 3) some builtins where the define_insns were requiring AVX512VL didn't have
> that requirement on the builtins, so again, numerous ICEs when using the
> builtins directly.
> 4) for some builtins the intrin headers were uselessly requiring avx512bw
> even when it wasn't needed at all (either when they don't have any mask
> argument or when they have an 8-bit or 16-bit only mask).
> 5) the def_builtin/ix86_expand_builtin stuff didn't handle
> OPTION_MASK_ISA_something | OPTION_MASK_ISA_AVX512BW or
> OPTION_MASK_ISA_something | OPTION_MASK_ISA_AVX512VL |
> OPTION_MASK_ISA_AVX512BW
> right (while the VL is handled there as "require the other ISAs and VL",
> for BW we don't do that).  There were some hacks for GFNI and VPCLMULQDQ,
> but incomplete and I think it is far better to treat BW and F like VL
> instead of those 2.  Plus we can improve stuff in def_builtin by only doing
> this special handling if the whole mask isn't a single bit mask, then there
> is no reason for just not requiring the isa.
> 6) in i386-common.c I've noticed a major problem, for the new avx512
> extensions that live in flags2 rather than flags (after this patch it is
> just avx5124fmaps and avx512vnniw), doing say -mavx5124fmaps -mno-avx512f
> would properly