Re: [patch, fortran] Fix PR 89496, error with alternate return

[Thomas Koenig]

Hi Dominique,



AFAICT there is no patch attached.


I guess that would have helped :-)

Here it is.


2019-02-25  Thomas Koenig  

PR fortran/89496
* trans-types.c (get_formal_from_actual_arglist): If
the actual arglist has no expression, the corresponding
formal arglist is an alternate return.

2019-02-25  Thomas Koenig  

PR fortran/89496
* gfortran.dg/altreturn_9_0.f90: New file.
* gfortran.dg/altreturn_9_1.f90: New file.
! { dg-do compile }
! { dg-options "-std=gnu" }
! See altreturn_9_0.f90
subroutine sub(i, *, j)
  if (i == 10 .and. j == 20) return 1
  return
end subroutine sub
! { dg-do  run }
! { dg-options -std=gnu }
! { dg-additional-sources altreturn_9_1.f90 }
! PR 89496 - wrong type for alternate return was generated

program main
  call sub(10, *10, 20)
  stop 1
10 continue
end program main
Index: trans-types.c
===
--- trans-types.c	(Revision 269161)
+++ trans-types.c	(Arbeitskopie)
@@ -2988,9 +2988,9 @@ get_formal_from_actual_arglist (gfc_symbol *sym, g
   f = &sym->formal;
   for (a = actual_args; a != NULL; a = a->next)
 {
+  (*f) = gfc_get_formal_arglist ();
   if (a->expr)
 	{
-	  (*f) = gfc_get_formal_arglist ();
 	  snprintf (name, GFC_MAX_SYMBOL_LEN, "_formal_%d", var_num ++);
 	  gfc_get_symbol (name, NULL, &s);
 	  if (a->expr->ts.type == BT_PROCEDURE)
@@ -3012,6 +3012,9 @@ get_formal_from_actual_arglist (gfc_symbol *sym, g
 	  s->attr.intent = INTENT_UNKNOWN;
 	  (*f)->sym = s;
 	}
+  else  /* If a->expr is NULL, this is an alternate rerturn.  */
+	(*f)->sym = NULL;
+
   f = &((*f)->next);
 }
 }

[doc, committed] fix PR c/80409

[Sandra Loosemore]

I've checked in this patch to document the GNU extension to va_arg for 
pointer types, which is really a POSIX extension per discussion in the 
issue.


-Sandra
2019-02-25  Sandra Loosemore  

	PR c/80409

	gcc/
	* doc/extend.texi (Variadic Pointer Args): New section.
Index: gcc/doc/extend.texi
===
--- gcc/doc/extend.texi	(revision 269202)
+++ gcc/doc/extend.texi	(working copy)
@@ -47,6 +47,7 @@ extensions, accepted by GCC in C90 mode
 * Escaped Newlines::Slightly looser rules for escaped newlines.
 * Subscripting::Any array can be subscripted, even if not an lvalue.
 * Pointer Arith::   Arithmetic on @code{void}-pointers and function pointers.
+* Variadic Pointer Args::  Pointer arguments to variadic functions.
 * Pointers to Arrays::  Pointers to arrays with qualifiers work as expected.
 * Initializers::Non-constant initializers.
 * Compound Literals::   Compound literals give structures, unions
@@ -1944,6 +1945,22 @@ and on function types, and returns 1.
 The option @option{-Wpointer-arith} requests a warning if these extensions
 are used.
 
+@node Variadic Pointer Args
+@section Pointer Arguments in Variadic Functions
+@cindex pointer arguments in variadic functions
+@cindex variadic functions, pointer arguments
+
+Standard C requires that pointer types used with @code{va_arg} in
+functions with variable argument lists either must be compatible with
+that of the actual argument, or that one type must be a pointer to
+@code{void} and the other a pointer to a character type.  GNU C
+implements the POSIX XSI extension that additionally permits the use
+of @code{va_arg} with a pointer type to receive arguments of any other
+pointer type.
+
+In particular, in GNU C @samp{va_arg (ap, void *)} can safely be used
+to consume an argument of any pointer type.
+
 @node Pointers to Arrays
 @section Pointers to Arrays with Qualifiers Work as Expected
 @cindex pointers to arrays

Re: [patch, fortran] Fix PR 87689, wrong decls / ABI violation on POWER

[Steve Kargl]

On Mon, Feb 25, 2019 at 05:54:44PM -0800, Bob Deen via fortran wrote:
> On 2/19/19 2:44 PM, Thomas Koenig wrote:
> > Bob,
> > 
> >> Some of us still use varargs interfaces (in my case, Fortran calling C
> >> stdarg subroutines).
> > 
> > The problem for us is that that sometimes using varargs made standard-
> > conforming Fortran code like, in file a.f
> > 
> > subroutine foo(a)
> > print *,a
> > end
> > 
> > and in file main.f
> > 
> > programme main
> > call foo(1.0)
> > end
> > 
> > depend ABI details: The call to foo used to be called using
> > the varargs convention, and the subroutine foo was compiled
> > as a non-varargs function.
> > 
> > This "worked" until PR 87689 showed that this breaks
> > standard-conforming Fortran code on a primary gcc platform.
> > 
> > I don't know if that makes a difference for the platform you work
> > on.  For the System V AMD64 ABI, I suspect it actually might not
> > matter (at least from glancing at the corresponding Wikipedia
> > article), but I am _not_ an expert in this field, so please take this
> > with a chunk of rock salt of appropriate size.
> > 
> > So, we cannot really keep this as a feature (note that varargs
> > are also not C interoperable).
> 
> Okay.  But I hope you don't let the perfect be the enemy of the good. 
> That particular platform is not a concern for us, so if you break it 
> there I'm not happy, but it's not the end of the world either.  But 
> please don't break it other places just because it doesn't work on that 
> one platform.  I know it's not good design practice to have that kind of 
> platform-dependent behavior, but sometimes practicalities force less 
> than ideal choices.
> 

Instead of fixing a bug in the compiler and conforming
to the Fortran standard, we should just let this one go
as an "enhancement".  How many more of these "enhancements"
should be allowed?  If an "enhancement" works on one
architecture but not on another, we'll need to start adding
pre-processing directives into otherwise machine-independent
code.  It becomes a maintainence nightmare.  We have these
maintainence issues in libgfortran, where we do have to
take the architecture into consider.  Keeping things working 
for everyone can be (ahem) fun.

If you need a vararg-like interface, it seems you should
consider encapulating the specific routines in a module
and build a generic interface with optional arguments.

-- 
Steve

Re: [patch, fortran] Fix PR 87689, wrong decls / ABI violation on POWER

[Bob Deen via gcc-patches]

On 2/19/19 2:44 PM, Thomas Koenig wrote:

Bob,


Some of us still use varargs interfaces (in my case, Fortran calling C
stdarg subroutines).


The problem for us is that that sometimes using varargs made standard-
conforming Fortran code like, in file a.f

subroutine foo(a)
print *,a
end

and in file main.f

programme main
call foo(1.0)
end

depend ABI details: The call to foo used to be called using
the varargs convention, and the subroutine foo was compiled
as a non-varargs function.

This "worked" until PR 87689 showed that this breaks
standard-conforming Fortran code on a primary gcc platform.

I don't know if that makes a difference for the platform you work
on.  For the System V AMD64 ABI, I suspect it actually might not
matter (at least from glancing at the corresponding Wikipedia
article), but I am _not_ an expert in this field, so please take this
with a chunk of rock salt of appropriate size.

So, we cannot really keep this as a feature (note that varargs
are also not C interoperable).


Okay.  But I hope you don't let the perfect be the enemy of the good. 
That particular platform is not a concern for us, so if you break it 
there I'm not happy, but it's not the end of the world either.  But 
please don't break it other places just because it doesn't work on that 
one platform.  I know it's not good design practice to have that kind of 
platform-dependent behavior, but sometimes practicalities force less 
than ideal choices.


Thanks...

-Bob

[PATCH] PR c/43673 - Incorrect warning in dfp printf.

[luoxhu]

From: Xiong Hu Luo 

dfp printf/scanf of Ha/HA, Da/DA and DDa/DDA is not set properly, cause
incorrect warning happens:
"use of 'D' length modifier with 'a' type character".

Regression-tested on powerpc64le-linux, OK for trunk and gcc-8?

gcc/c-family/ChangeLog:

2019-02-25  Xiong Hu Luo  

PR c/43673
* c-format.c (print_char_table, scanf_char_table): Replace BADLEN with
TEX_D32, TEX_D64 or TEX_D128.

gcc/testsuit/ChangeLog:

2019-02-25  Xiong Hu Luo  

PR c/43673
* gcc.dg/format-dfp-printf-1.c: New test.
* gcc.dg/format-dfp-scanf-1.c: Likewise.
---
 gcc/c-family/c-format.c|  4 ++--
 gcc/testsuite/gcc.dg/format/dfp-printf-1.c | 28 ++--
 gcc/testsuite/gcc.dg/format/dfp-scanf-1.c  | 22 --
 3 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
index 9b48ee3..af33ef9 100644
--- a/gcc/c-family/c-format.c
+++ b/gcc/c-family/c-format.c
@@ -674,7 +674,7 @@ static const format_char_info print_char_table[] =
   { "n",   1, STD_C89, { T89_I,   T99_SC,  T89_S,   T89_L,   T9L_LL,  BADLEN,  
T99_SST, T99_PD,  T99_IM,  BADLEN,  BADLEN,  BADLEN }, "",  "W",  NULL 
},
   /* C99 conversion specifiers.  */
   { "F",   0, STD_C99, { T99_D,   BADLEN,  BADLEN,  T99_D,   BADLEN,  T99_LD,  
BADLEN,  BADLEN,  BADLEN,  TEX_D32, TEX_D64, TEX_D128 }, "-wp0 +#'I", "",   
NULL },
-  { "aA",  0, STD_C99, { T99_D,   BADLEN,  BADLEN,  T99_D,   BADLEN,  T99_LD,  
BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN }, "-wp0 +#",   "",   NULL 
},
+  { "aA",  0, STD_C99, { T99_D,   BADLEN,  BADLEN,  T99_D,   BADLEN,  T99_LD,  
BADLEN,  BADLEN,  BADLEN,  TEX_D32, TEX_D64,  TEX_D128 }, "-wp0 +#",   "",   
NULL },
   /* X/Open conversion specifiers.  */
   { "C",   0, STD_EXT, { TEX_WI,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  
BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN }, "-w","",   NULL 
},
   { "S",   1, STD_EXT, { TEX_W,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  
BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN }, "-wp",   "R",  NULL 
},
@@ -847,7 +847,7 @@ static const format_char_info scan_char_table[] =
   { "n", 1, STD_C89, { T89_I,   T99_SC,  T89_S,   T89_L,   T9L_LL,  
BADLEN,  T99_SST, T99_PD,  T99_IM,  BADLEN,  BADLEN,  BADLEN }, "", "W",   
NULL },
   /* C99 conversion specifiers.  */
   { "F",   1, STD_C99, { T99_F,   BADLEN,  BADLEN,  T99_D,   BADLEN,  T99_LD,  
BADLEN,  BADLEN,  BADLEN,  TEX_D32, TEX_D64, TEX_D128 }, "*w'",  "W",   NULL },
-  { "aA",   1, STD_C99, { T99_F,   BADLEN,  BADLEN,  T99_D,   BADLEN,  T99_LD, 
 BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN }, "*w'",  "W",   NULL },
+  { "aA",   1, STD_C99, { T99_F,   BADLEN,  BADLEN,  T99_D,   BADLEN,  T99_LD, 
 BADLEN,  BADLEN,  BADLEN,  TEX_D32,  TEX_D64,  TEX_D128 }, "*w'",  "W",   NULL 
},
   /* X/Open conversion specifiers.  */
   { "C", 1, STD_EXT, { TEX_W,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  
BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN }, "*mw",   "W",   
NULL },
   { "S", 1, STD_EXT, { TEX_W,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  
BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN }, "*amw",  "W",   
NULL },
diff --git a/gcc/testsuite/gcc.dg/format/dfp-printf-1.c 
b/gcc/testsuite/gcc.dg/format/dfp-printf-1.c
index e92f161..a290895 100644
--- a/gcc/testsuite/gcc.dg/format/dfp-printf-1.c
+++ b/gcc/testsuite/gcc.dg/format/dfp-printf-1.c
@@ -17,6 +17,8 @@ foo (_Decimal32 x, _Decimal64 y, _Decimal128 z, int i, 
unsigned int j,
 
   /* Check lack of warnings for valid usage.  */
 
+  printf ("%Ha\n", x);
+  printf ("%HA\n", x);
   printf ("%Hf\n", x);
   printf ("%HF\n", x);
   printf ("%He\n", x);
@@ -24,6 +26,8 @@ foo (_Decimal32 x, _Decimal64 y, _Decimal128 z, int i, 
unsigned int j,
   printf ("%Hg\n", x);
   printf ("%HG\n", x);
 
+  printf ("%Da\n", y);
+  printf ("%DA\n", y);
   printf ("%Df\n", y);
   printf ("%DF\n", y);
   printf ("%De\n", y);
@@ -31,6 +35,8 @@ foo (_Decimal32 x, _Decimal64 y, _Decimal128 z, int i, 
unsigned int j,
   printf ("%Dg\n", y);
   printf ("%DG\n", y);
 
+  printf ("%DDa\n", z);
+  printf ("%DDA\n", z);
   printf ("%DDf\n", z);
   printf ("%DDF\n", z);
   printf ("%DDe\n", z);
@@ -43,12 +49,16 @@ foo (_Decimal32 x, _Decimal64 y, _Decimal128 z, int i, 
unsigned int j,
 
   /* Check warnings for type mismatches.  */
 
+  printf ("%Ha\n", y); /* { dg-warning "expects argument" "bad use of %H" } */
+  printf ("%HA\n", y); /* { dg-warning "expects argument" "bad use of %H" } */
   printf ("%Hf\n", y); /* { dg-warning "expects argument" "bad use of %H" } */
   printf ("%HF\n", y); /* { dg-warning "expects argument" "bad use of %H" } */
   printf ("%He\n", y); /* { dg-warning "expects argument" "bad use of %H" } */
   printf ("%HE\n", y); /* { dg-warning "expects argument" "bad use of %H" } */
   printf ("%Hg\n", y); /* { dg-warning "expects argument" "bad use of %H" } */
   pri

libgo patch committed: Update to 1.12rc1

[Ian Lance Taylor]

I've committed this patch to update libgo to the 1.12rc1 release of
the master library.  Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu.  Committed to mainline.

Ian


patch.txt.bz2
Description: application/bzip

Re: [PATCH] Fix up strnlen handling in tree-ssa-strlen.c (PR tree-optimization/89500)

[Martin Sebor]

On 2/25/19 4:12 PM, Jakub Jelinek wrote:

Hi!

The following testcase ICEs, because rhs (previously known string length)
is SSA_NAME (return value from the earlier strlen), but bound is 0 and so
MIN_EXPR yields also size_zero_node.  The noncst_bound initialization
checks if new_rhs is INTEGER_CST and if it is, assumes rhs must be too
and uses tree_int_cst_lt.  While we could just add
|| TREE_CODE (rhs) != INTEGER_CST
before the || tree_int_cst_lt (new_rhs, rhs), I fail to see what
noncst_bound (which would be misnamed anyway) is good for.
For further optimizations in the strlen pass we care about string lengths
only, and we can prove strnlen returns that length only if all of rhs,
new_rhs and bound are actually INTEGER_CSTs for which we can prove that
new_rhs is equal to rhs.  The
   if (si != NULL
   && TREE_CODE (si->nonzero_chars) != SSA_NAME
   && TREE_CODE (si->nonzero_chars) != INTEGER_CST
   && !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (lhs))
 {
   si = unshare_strinfo (si);
   si->nonzero_chars = lhs;
   gcc_assert (si->full_string_p);
 }
hunk is though about trying to improve the case where we could compute
the length through some extra code (e.g. through strchr transformations,
stpcpy etc.) and for the next time we don't want to repeat that computation,
but just use the lhs of the strlen call, i.e. SSA_NAME.  As written above,
we explicitly don't do anything if it is INTEGER_CST, we already know very
well the length.  So the only case where we can prove something is not
useful here.
The only remaining code in the function before returning is:
   if (strlen_to_stridx)
 strlen_to_stridx->put (lhs, stridx_strlenloc (idx, loc));
but I believe doing that for strnlen wasn't really intentional, in the
case we don't know the length before we don't store it either:
   if (strlen_to_stridx && !bound)
 strlen_to_stridx->put (lhs, stridx_strlenloc (idx, loc));
it is for the warnings and I believe it is desirable to do that for strlen
only.


Storing the strnlen result is intentional when it equals strlen.
I'm surprised there is no test case exercising it but the before
and after difference can be seen in this test case:

  void f (void)
  {
char a[30] = "123456789";

int n = __builtin_strlen (a);

__builtin_strncpy (d, a, n);   // -Wstringop-overflow here
  }


  void g (void)
  {
char a[30] = "123456789";

int n = __builtin_strnlen (a, 20);

__builtin_strncpy (d, a, n);   // -Wstringop-overflow here
 }

By not storing the length we lose the second warning.  I'd prefer
not to lose the warning in the second case so your first suggestion
sound like a good fix to me.  I can handle that and add tests for
this if you like.

Martin



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

2019-02-25  Jakub Jelinek  

PR tree-optimization/89500
* tree-ssa-strlen.c (handle_builtin_strlen): Remove noncst_bound
variable, return early after optimizing stmt if bound is non-NULL.

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

--- gcc/tree-ssa-strlen.c.jj2019-01-18 00:33:19.466003372 +0100
+++ gcc/tree-ssa-strlen.c   2019-02-25 22:13:27.165521677 +0100
@@ -1294,18 +1294,8 @@ handle_builtin_strlen (gimple_stmt_itera
  if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
rhs = fold_convert_loc (loc, TREE_TYPE (lhs), rhs);
  
-	  /* Set for strnlen() calls with a non-constant bound.  */

- bool noncst_bound = false;
  if (bound)
-   {
- tree new_rhs
-   = fold_build2_loc (loc, MIN_EXPR, TREE_TYPE (rhs), rhs, bound);
-
- noncst_bound = (TREE_CODE (new_rhs) != INTEGER_CST
- || tree_int_cst_lt (new_rhs, rhs));
-
- rhs = new_rhs;
-   }
+   rhs = fold_build2_loc (loc, MIN_EXPR, TREE_TYPE (rhs), rhs, bound);
  
  	  if (!update_call_from_tree (gsi, rhs))

gimplify_and_update_call_from_tree (gsi, rhs);
@@ -1317,9 +1307,12 @@ handle_builtin_strlen (gimple_stmt_itera
  print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
}
  
-	  /* Avoid storing the length for calls to strnlen() with

-a non-constant bound.  */
- if (noncst_bound)
+ /* Don't record anything below for strnlen, it is to simplify
+length computation if si->nonzero_chars is complex, but
+the only case where we could prove strnlen return value is
+the string length is if si->nonzero_chars is constant and
+bound is too and si->nonzero_chars <= bound.  */
+ if (bound)
return;
  
  	  if (si != NULL

--- gcc/testsuite/gcc.dg/pr89500.c.jj   2019-02-25 22:14:46.468222667 +0100
+++ gcc/testsuite/gcc.dg/pr89500.c  2019-02-25 22:14:26.948542413 +0100
@@ -0,0 +1,17 @@
+/* PR tree-optimization/89500 */
+/* { dg-do 

Re: [PATCH] Fix UB in c-format.c (maybe_read_dollar_number) (PR c/89495)

[Joseph Myers]

On Mon, 25 Feb 2019, Jakub Jelinek wrote:

> Hi!
> 
> The testcases Martin has added recently that contain precision or width
> that doesn't fit into int cause UB in the following routine, as 10 * argnum
> or that + (*fcp - '0') can result in signed integer overflow.
> 
> The following patch just does the computation in UHWI, which we know is
> wider than int (I think we don't support 64-bit int hosts yet).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

This patch is OK.

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

[PATCH] Fix up strnlen handling in tree-ssa-strlen.c (PR tree-optimization/89500)

[Jakub Jelinek]

Hi!

The following testcase ICEs, because rhs (previously known string length)
is SSA_NAME (return value from the earlier strlen), but bound is 0 and so
MIN_EXPR yields also size_zero_node.  The noncst_bound initialization
checks if new_rhs is INTEGER_CST and if it is, assumes rhs must be too
and uses tree_int_cst_lt.  While we could just add
|| TREE_CODE (rhs) != INTEGER_CST
before the || tree_int_cst_lt (new_rhs, rhs), I fail to see what
noncst_bound (which would be misnamed anyway) is good for.
For further optimizations in the strlen pass we care about string lengths
only, and we can prove strnlen returns that length only if all of rhs,
new_rhs and bound are actually INTEGER_CSTs for which we can prove that
new_rhs is equal to rhs.  The
  if (si != NULL
  && TREE_CODE (si->nonzero_chars) != SSA_NAME
  && TREE_CODE (si->nonzero_chars) != INTEGER_CST
  && !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (lhs))
{
  si = unshare_strinfo (si);
  si->nonzero_chars = lhs;
  gcc_assert (si->full_string_p);
}
hunk is though about trying to improve the case where we could compute
the length through some extra code (e.g. through strchr transformations,
stpcpy etc.) and for the next time we don't want to repeat that computation,
but just use the lhs of the strlen call, i.e. SSA_NAME.  As written above,
we explicitly don't do anything if it is INTEGER_CST, we already know very
well the length.  So the only case where we can prove something is not
useful here.
The only remaining code in the function before returning is:
  if (strlen_to_stridx)
strlen_to_stridx->put (lhs, stridx_strlenloc (idx, loc));
but I believe doing that for strnlen wasn't really intentional, in the
case we don't know the length before we don't store it either:
  if (strlen_to_stridx && !bound)
strlen_to_stridx->put (lhs, stridx_strlenloc (idx, loc));
it is for the warnings and I believe it is desirable to do that for strlen
only.

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

2019-02-25  Jakub Jelinek  

PR tree-optimization/89500
* tree-ssa-strlen.c (handle_builtin_strlen): Remove noncst_bound
variable, return early after optimizing stmt if bound is non-NULL.

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

--- gcc/tree-ssa-strlen.c.jj2019-01-18 00:33:19.466003372 +0100
+++ gcc/tree-ssa-strlen.c   2019-02-25 22:13:27.165521677 +0100
@@ -1294,18 +1294,8 @@ handle_builtin_strlen (gimple_stmt_itera
  if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
rhs = fold_convert_loc (loc, TREE_TYPE (lhs), rhs);
 
- /* Set for strnlen() calls with a non-constant bound.  */
- bool noncst_bound = false;
  if (bound)
-   {
- tree new_rhs
-   = fold_build2_loc (loc, MIN_EXPR, TREE_TYPE (rhs), rhs, bound);
-
- noncst_bound = (TREE_CODE (new_rhs) != INTEGER_CST
- || tree_int_cst_lt (new_rhs, rhs));
-
- rhs = new_rhs;
-   }
+   rhs = fold_build2_loc (loc, MIN_EXPR, TREE_TYPE (rhs), rhs, bound);
 
  if (!update_call_from_tree (gsi, rhs))
gimplify_and_update_call_from_tree (gsi, rhs);
@@ -1317,9 +1307,12 @@ handle_builtin_strlen (gimple_stmt_itera
  print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
}
 
- /* Avoid storing the length for calls to strnlen() with
-a non-constant bound.  */
- if (noncst_bound)
+ /* Don't record anything below for strnlen, it is to simplify
+length computation if si->nonzero_chars is complex, but
+the only case where we could prove strnlen return value is
+the string length is if si->nonzero_chars is constant and
+bound is too and si->nonzero_chars <= bound.  */
+ if (bound)
return;
 
  if (si != NULL
--- gcc/testsuite/gcc.dg/pr89500.c.jj   2019-02-25 22:14:46.468222667 +0100
+++ gcc/testsuite/gcc.dg/pr89500.c  2019-02-25 22:14:26.948542413 +0100
@@ -0,0 +1,17 @@
+/* PR tree-optimization/89500 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+typedef __SIZE_TYPE__ size_t;
+extern size_t strlen (const char *);
+extern size_t strnlen (const char *, size_t);
+extern void bar (char *);
+
+void
+foo (int *a)
+{
+  char c[64];
+  bar (c);
+  a[0] = strlen (c);
+  a[1] = strnlen (c, 0);
+}

Jakub

[PATCH] Use RANGE_EXPR in Fortran array initializers some more (PR fortran/43210)

[Jakub Jelinek]

Hi!

When initializing whole array with a const, we can save quite some compile
time memory (and time in some cases) by using RANGE_EXPRs, instead of
duplicating the same initializer thousands of times in the CONSTRUCTOR.
In some cases the gimplifier even can optimize those better.

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

2019-02-25  Jakub Jelinek  

PR fortran/43210
* trans-array.c (gfc_conv_array_initializer): Use RANGE_EXPR instead
of duplicating the initializer possibly many times.

--- gcc/fortran/trans-array.c.jj2019-02-10 12:05:42.753718023 +0100
+++ gcc/fortran/trans-array.c   2019-02-25 18:11:44.948166509 +0100
@@ -5986,7 +5986,6 @@ gfc_conv_array_initializer (tree type, g
 {
   gfc_constructor *c;
   tree tmp;
-  offset_int wtmp;
   gfc_se se;
   tree index, range;
   vec *v = NULL;
@@ -6009,13 +6008,10 @@ gfc_conv_array_initializer (tree type, g
   else
gfc_conv_structure (&se, expr, 1);
 
-  wtmp = wi::to_offset (TYPE_MAX_VALUE (TYPE_DOMAIN (type))) + 1;
-  /* This will probably eat buckets of memory for large arrays.  */
-  while (wtmp != 0)
-{
- CONSTRUCTOR_APPEND_ELT (v, NULL_TREE, se.expr);
- wtmp -= 1;
-}
+  CONSTRUCTOR_APPEND_ELT (v, build2 (RANGE_EXPR, gfc_array_index_type,
+TYPE_MIN_VALUE (TYPE_DOMAIN (type)),
+TYPE_MAX_VALUE (TYPE_DOMAIN (type))),
+ se.expr);
   break;
 
 case EXPR_ARRAY:

Jakub

[committed] Add testcases for PR c/77754

[Jakub Jelinek]

Hi!

All the following testcases were fixed by r266271 aka PR87229 fix
already.  I've regtested them on x86_64-linux and i686-linux and committed
to trunk as obvious.

2019-02-25  Jakub Jelinek  

PR c/77754
* gcc.c-torture/compile/pr77754-1.c: New test.
* gcc.c-torture/compile/pr77754-2.c: New test.
* gcc.c-torture/compile/pr77754-3.c: New test.
* gcc.c-torture/compile/pr77754-4.c: New test.
* gcc.c-torture/compile/pr77754-5.c: New test.
* gcc.c-torture/compile/pr77754-6.c: New test.

--- gcc/testsuite/gcc.c-torture/compile/pr77754-1.c.jj  2019-02-25 
17:40:26.652905532 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr77754-1.c 2019-02-25 
17:40:26.652905532 +0100
@@ -0,0 +1,11 @@
+/* PR c/77754 */
+
+int fn3();
+
+void fn4(int[][fn3 ()]);
+
+void fn1() {
+  void fn2(int[][fn3 ()]);
+  int a[10][fn3 ()];
+  fn4 (a);
+}
--- gcc/testsuite/gcc.c-torture/compile/pr77754-2.c.jj  2019-02-25 
17:40:26.652905532 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr77754-2.c 2019-02-25 
17:40:26.652905532 +0100
@@ -0,0 +1,10 @@
+/* PR c/77754 */
+
+int fn3();
+
+void (**fn5) (int[][fn3 ()]);
+
+void fn1 () {
+  int a[10][fn3 ()];
+  (**fn5) (a);
+}
--- gcc/testsuite/gcc.c-torture/compile/pr77754-3.c.jj  2019-02-25 
17:40:26.651905548 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr77754-3.c 2019-02-25 
17:40:26.651905548 +0100
@@ -0,0 +1,11 @@
+/* PR c/77754 */
+
+int fn3();
+
+typedef void (*fn6) (int[][fn3 ()]);
+fn6 **fn7;
+
+void fn1 () {
+  int a[10][fn3 ()];
+  (**fn7) (a);
+}
--- gcc/testsuite/gcc.c-torture/compile/pr77754-4.c.jj  2019-02-25 
17:40:26.652905532 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr77754-4.c 2019-02-25 
17:40:26.652905532 +0100
@@ -0,0 +1,17 @@
+/* PR c/77754 */
+
+int fn3();
+
+typedef void (*fn6) (int[][fn3 ()]);
+struct S {
+  fn6 **fn7;
+  fn6 *fn8;
+  fn6 fn9;
+} s;
+
+void fn1 () {
+  int a[10][fn3 ()];
+  (**s.fn7) (a);
+  (*s.fn8) (a);
+  s.fn9 (a);
+}
--- gcc/testsuite/gcc.c-torture/compile/pr77754-5.c.jj  2019-02-25 
17:40:26.652905532 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr77754-5.c 2019-02-25 
17:40:26.652905532 +0100
@@ -0,0 +1,14 @@
+/* PR c/77754 */
+
+int fn3();
+
+void fn4(int[][fn3 ()]);
+void fn4(int x[][fn3 ()])
+{
+}
+
+void fn1() {
+  void fn2(int[][fn3 ()]);
+  int a[10][fn3 ()];
+  fn4 (a);
+}
--- gcc/testsuite/gcc.c-torture/compile/pr77754-6.c.jj  2019-02-25 
17:40:26.651905548 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr77754-6.c 2019-02-25 
17:40:26.651905548 +0100
@@ -0,0 +1,10 @@
+/* PR c/77754 */
+
+int fn3();
+
+void fn4(int (*)[fn3 ()][fn3 () + 1][fn3 () + 2], struct S { int a[fn3 ()]; } 
*);
+
+void fn1() {
+  int a[10][fn3 ()][fn3 () + 1][fn3 () + 2];
+  fn4 (a, 0);
+}

Jakub

Re: [PATCH] Fix up remove_partial_avx_dependency (PR target/89474)

[H.J. Lu]

On Mon, Feb 25, 2019 at 2:53 PM Jakub Jelinek  wrote:
>
> Hi!
>
> The following patch fixes two issues in the new rpad pass.
> One is that the insertion at the start of a basic block didn't work properly
> if the basic block didn't contain any non-NOTE/non-DEBUG_INSN instructions.
> next_nonnote_nondebug_insn hapilly turns through into another basic block
> and the insertion can insert an instruction in between basic blocks or into
> a different basic block from where we wanted to emit it.
> I believe we want to emit it after CODE_LABEL, after NOTE_INSN_BASIC_BLOCK
> and if possible, after debug insns in there, the patch emits it before the
> first normal insn in the bb if any, or after the BB_END (thus extending
> BB_END).
>
> Another issue is that it is quite weird/dangerous to add the v4sf_const0
> pseudo uses in lots of places in the IL, register those changes with df,
> then do df_analyze with different flags and finally emit the setter.
> I understand the goal was not to do df_analyze etc. in the usual case where
> there are no instructions that need this treatment.  This patch does the
> df_analyze at the spot we find the first insn, but before we actually change
> that instruction, so the changes are after the df_analyze.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2019-02-25  Jakub Jelinek  
>
> PR target/89474
> * config/i386/i386.c (remove_partial_avx_dependency): Call
> df_analyze etc. before creation of the v4sf_const0 pseudo, rather than
> after changing possibly many instructions to use that pseudo.  Fix up
> insertion of v4sf_const0 setter at the start of bb.
>
> * gcc.target/i386/pr89474.c: New test.
>
> --- gcc/config/i386/i386.c.jj   2019-02-22 23:02:47.805117610 +0100
> +++ gcc/config/i386/i386.c  2019-02-25 14:20:05.793608879 +0100
> @@ -2835,7 +2835,14 @@ remove_partial_avx_dependency (void)
> continue;
>
>   if (!v4sf_const0)
> -   v4sf_const0 = gen_reg_rtx (V4SFmode);
> +   {
> + calculate_dominance_info (CDI_DOMINATORS);
> + df_set_flags (DF_DEFER_INSN_RESCAN);
> + df_chain_add_problem (DF_DU_CHAIN | DF_UD_CHAIN);
> + df_md_add_problem ();
> + df_analyze ();
> + v4sf_const0 = gen_reg_rtx (V4SFmode);
> +   }
>
>   /* Convert PARTIAL_XMM_UPDATE_TRUE insns, DF -> SF, SF -> DF,
>  SI -> SF, SI -> DF, DI -> SF, DI -> DF, to vec_dup and
> @@ -2883,12 +2890,6 @@ remove_partial_avx_dependency (void)
>
>if (v4sf_const0)
>  {
> -  calculate_dominance_info (CDI_DOMINATORS);
> -  df_set_flags (DF_DEFER_INSN_RESCAN);
> -  df_chain_add_problem (DF_DU_CHAIN | DF_UD_CHAIN);
> -  df_md_add_problem ();
> -  df_analyze ();
> -
>/* (Re-)discover loops so that bb->loop_father can be used in the
>  analysis below.  */
>loop_optimizer_init (AVOID_CFG_MODIFICATIONS);
> @@ -2904,11 +2905,23 @@ remove_partial_avx_dependency (void)
> bb = get_immediate_dominator (CDI_DOMINATORS,
>   bb->loop_father->header);
>
> -  insn = BB_HEAD (bb);
> -  if (!NONDEBUG_INSN_P (insn))
> -   insn = next_nonnote_nondebug_insn (insn);
>set = gen_rtx_SET (v4sf_const0, CONST0_RTX (V4SFmode));
> -  set_insn = emit_insn_before (set, insn);
> +
> +  insn = BB_HEAD (bb);
> +  while (insn && !NONDEBUG_INSN_P (insn))
> +   {
> + if (insn == BB_END (bb))
> +   {
> + insn = NULL;
> + break;
> +   }
> + insn = NEXT_INSN (insn);
> +   }
> +  if (insn == BB_HEAD (bb))
> +set_insn = emit_insn_before (set, insn);
> +  else
> +   set_insn = emit_insn_after (set,
> +   insn ? PREV_INSN (insn) : BB_END (bb));
>df_insn_rescan (set_insn);
>df_process_deferred_rescans ();
>loop_optimizer_finalize ();
> --- gcc/testsuite/gcc.target/i386/pr89474.c.jj  2019-02-25 14:21:51.651867104 
> +0100
> +++ gcc/testsuite/gcc.target/i386/pr89474.c 2019-02-25 14:21:34.373151405 
> +0100
> @@ -0,0 +1,14 @@
> +/* PR target/89474 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -mavx" } */
> +
> +int a;
> +void foo (double);
> +int baz (void);
> +
> +void
> +bar (void)
> +{
> +  while (baz ())
> +foo (a);
> +}
>
> Jakub

Thanks.


-- 
H.J.

[PATCH] Fix UB in c-format.c (maybe_read_dollar_number) (PR c/89495)

[Jakub Jelinek]

Hi!

The testcases Martin has added recently that contain precision or width
that doesn't fit into int cause UB in the following routine, as 10 * argnum
or that + (*fcp - '0') can result in signed integer overflow.

The following patch just does the computation in UHWI, which we know is
wider than int (I think we don't support 64-bit int hosts yet).

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

Or do you prefer computations in unsigned int?

2019-02-25  Jakub Jelinek  

PR c/89495
* c-format.c (maybe_read_dollar_number): Compute nargnum in
HOST_WIDE_INT type to avoid overflows and change overflow_flag
checking.

--- gcc/c-family/c-format.c.jj  2019-01-16 09:35:04.565323073 +0100
+++ gcc/c-family/c-format.c 2019-02-25 16:26:07.872810237 +0100
@@ -1268,9 +1268,9 @@ maybe_read_dollar_number (const char **f
   overflow_flag = 0;
   while (ISDIGIT (*fcp))
 {
-  int nargnum;
-  nargnum = 10 * argnum + (*fcp - '0');
-  if (nargnum < 0 || nargnum / 10 != argnum)
+  HOST_WIDE_INT nargnum
+   = HOST_WIDE_INT_UC (10) * argnum + (*fcp - '0');
+  if ((int) nargnum != nargnum)
overflow_flag = 1;
   argnum = nargnum;
   fcp++;

Jakub

[PATCH] Fix up remove_partial_avx_dependency (PR target/89474)

[Jakub Jelinek]

Hi!

The following patch fixes two issues in the new rpad pass.
One is that the insertion at the start of a basic block didn't work properly
if the basic block didn't contain any non-NOTE/non-DEBUG_INSN instructions.
next_nonnote_nondebug_insn hapilly turns through into another basic block
and the insertion can insert an instruction in between basic blocks or into
a different basic block from where we wanted to emit it.
I believe we want to emit it after CODE_LABEL, after NOTE_INSN_BASIC_BLOCK
and if possible, after debug insns in there, the patch emits it before the
first normal insn in the bb if any, or after the BB_END (thus extending
BB_END).

Another issue is that it is quite weird/dangerous to add the v4sf_const0
pseudo uses in lots of places in the IL, register those changes with df,
then do df_analyze with different flags and finally emit the setter.
I understand the goal was not to do df_analyze etc. in the usual case where
there are no instructions that need this treatment.  This patch does the
df_analyze at the spot we find the first insn, but before we actually change
that instruction, so the changes are after the df_analyze.

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

2019-02-25  Jakub Jelinek  

PR target/89474
* config/i386/i386.c (remove_partial_avx_dependency): Call
df_analyze etc. before creation of the v4sf_const0 pseudo, rather than
after changing possibly many instructions to use that pseudo.  Fix up
insertion of v4sf_const0 setter at the start of bb.

* gcc.target/i386/pr89474.c: New test.

--- gcc/config/i386/i386.c.jj   2019-02-22 23:02:47.805117610 +0100
+++ gcc/config/i386/i386.c  2019-02-25 14:20:05.793608879 +0100
@@ -2835,7 +2835,14 @@ remove_partial_avx_dependency (void)
continue;
 
  if (!v4sf_const0)
-   v4sf_const0 = gen_reg_rtx (V4SFmode);
+   {
+ calculate_dominance_info (CDI_DOMINATORS);
+ df_set_flags (DF_DEFER_INSN_RESCAN);
+ df_chain_add_problem (DF_DU_CHAIN | DF_UD_CHAIN);
+ df_md_add_problem ();
+ df_analyze ();
+ v4sf_const0 = gen_reg_rtx (V4SFmode);
+   }
 
  /* Convert PARTIAL_XMM_UPDATE_TRUE insns, DF -> SF, SF -> DF,
 SI -> SF, SI -> DF, DI -> SF, DI -> DF, to vec_dup and
@@ -2883,12 +2890,6 @@ remove_partial_avx_dependency (void)
 
   if (v4sf_const0)
 {
-  calculate_dominance_info (CDI_DOMINATORS);
-  df_set_flags (DF_DEFER_INSN_RESCAN);
-  df_chain_add_problem (DF_DU_CHAIN | DF_UD_CHAIN);
-  df_md_add_problem ();
-  df_analyze ();
-
   /* (Re-)discover loops so that bb->loop_father can be used in the
 analysis below.  */
   loop_optimizer_init (AVOID_CFG_MODIFICATIONS);
@@ -2904,11 +2905,23 @@ remove_partial_avx_dependency (void)
bb = get_immediate_dominator (CDI_DOMINATORS,
  bb->loop_father->header);
 
-  insn = BB_HEAD (bb);
-  if (!NONDEBUG_INSN_P (insn))
-   insn = next_nonnote_nondebug_insn (insn);
   set = gen_rtx_SET (v4sf_const0, CONST0_RTX (V4SFmode));
-  set_insn = emit_insn_before (set, insn);
+
+  insn = BB_HEAD (bb);
+  while (insn && !NONDEBUG_INSN_P (insn))
+   {
+ if (insn == BB_END (bb))
+   {
+ insn = NULL;
+ break;
+   }
+ insn = NEXT_INSN (insn);
+   }
+  if (insn == BB_HEAD (bb))
+set_insn = emit_insn_before (set, insn);
+  else
+   set_insn = emit_insn_after (set,
+   insn ? PREV_INSN (insn) : BB_END (bb));
   df_insn_rescan (set_insn);
   df_process_deferred_rescans ();
   loop_optimizer_finalize ();
--- gcc/testsuite/gcc.target/i386/pr89474.c.jj  2019-02-25 14:21:51.651867104 
+0100
+++ gcc/testsuite/gcc.target/i386/pr89474.c 2019-02-25 14:21:34.373151405 
+0100
@@ -0,0 +1,14 @@
+/* PR target/89474 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -mavx" } */
+
+int a;
+void foo (double);
+int baz (void);
+
+void
+bar (void)
+{
+  while (baz ())
+foo (a);
+}

Jakub

[C++ PATCH] Fix up constexpr changing of active union member (PR c++/89481)

[Jakub Jelinek]

Hi!

cxx_eval_store_expression has code to propagate no_zero_init from outer
ctors to inner ctors, if the outer ctor is known to be zero initialized, the
inner one should be as well.

As the following patch shows, when changing active union member there needs
to be an exception, even if the whole containing aggregate was zero
initialized before, if we change the union member and just initialize some
portion of the new one, we can't assume the rest is initialized.

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

2019-02-25  Jakub Jelinek  

PR c++/89481
* constexpr.c (cxx_eval_store_expression): When changing active union
member, set no_zero_init.

* g++.dg/cpp1y/constexpr-89481.C: New test.

--- gcc/cp/constexpr.c.jj   2019-02-21 22:20:24.838100126 +0100
+++ gcc/cp/constexpr.c  2019-02-25 10:18:32.526462730 +0100
@@ -3860,6 +3860,7 @@ cxx_eval_store_expression (const constex
}
  /* Changing active member.  */
  vec_safe_truncate (CONSTRUCTOR_ELTS (*valp), 0);
+ no_zero_init = true;
}
 
  for (idx = 0;
--- gcc/testsuite/g++.dg/cpp1y/constexpr-89481.C.jj 2019-02-25 
10:27:39.188382381 +0100
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-89481.C2019-02-25 
10:27:12.957818089 +0100
@@ -0,0 +1,24 @@
+// PR c++/89481
+// { dg-do compile { target c++14 } }
+
+constexpr int
+foo ()
+{
+  union U { long long a; int b[2]; } u { 5LL };
+  u.b[1] = 4;  // { dg-error "change of the active member of a union 
from" "" { target c++17_down } }
+  return u.b[0];
+}
+
+constexpr int
+bar ()
+{
+  union U { long long a; int b[2]; } u { 5LL };
+  u.b[1] = 4;  // { dg-error "change of the active member of a union 
from" "" { target c++17_down } }
+  return u.b[1];
+}
+
+static_assert (foo () == 0, "");   // { dg-error "non-constant condition 
for static assertion" }
+   // { dg-message "in 'constexpr' 
expansion of" "" { target *-*-* } .-1 }
+   // { dg-error "accessing uninitialized 
array element" "" { target c++2a } .-2 }
+static_assert (bar () == 4, "");   // { dg-error "non-constant condition 
for static assertion" "" { target c++17_down } }
+   // { dg-message "in 'constexpr' 
expansion of" "" { target c++17_down } .-1 }

Jakub

[patch, fortran] Fix PR 89496, error with alternate return

[Thomas König]

Hello world,

the attached patch fixes a regression introduced by my recent patch for
PR 87689, where the alternate return arguments were not handled
correctly.

Some experimentation resulted in a test case which actually segfaulted
on a normal compiler, instead of just being visible on an instrumened
one.  The test case also checks that the alternate return runs
correctly (and uses the "dg-do  run" hack :-)

I also checked this with -flto and by inspecting the output of
-fdump-fortran-original.

Thanks to Martin Liska for the bug report.

Regression-tested.  OK for trunk?

Regards

Thomas

2019-02-25  Thomas Koenig  

PR fortran/89496
* trans-types.c (get_formal_from_actual_arglist): If
the actual arglist has no expression, the corresponding
formal arglist is an alternate return.

2019-02-25  Thomas Koenig  

PR fortran/89496
* gfortran.dg/altreturn_9_0.f90: New file.
* gfortran.dg/altreturn_9_1.f90: New file.

Re: [patch, fortran] Fix PR 89174, segfault on allocate with MOLD

[Steve Kargl]

On Mon, Feb 25, 2019 at 01:58:27PM -0800, Steve Kargl wrote:
> On Mon, Feb 25, 2019 at 10:03:29PM +0100, Thomas König wrote:
> > Hi Steve,
> > 
> > >> I think this was introduced quite some time ago, not sure if it
> > >> was ever documented anywhere.  I guess we should do so.
> > >>
> > > Probably want to document this in the testcase.
> > 
> > I just checked and found 77 occurences in the test suite (most of
> > them mine, to be sure).  So, maybe an entry in the wiki would be
> > more appropriate.
> > 
> > I am just trying to think of who introduced this (or if this
> > is even gfortran specific), but coming up blank.  Does anybody
> > remember?
> > 
> 
> It looks like a fortuitous accident.  
> 
> % svn annotate gcc/testsuite/lib/gfortran-dg.exp
>  84792   tobi   # look if this is dg-do-run test, in which case
>  84792   tobi   # we cycle through the option list, otherwise we don't
>  84792   tobi   if [expr [search_for $test "dg-do run"]] {
> 135381  janis   set option_list $torture_with_loops
>  84792   tobi   } else {
>  84792   tobi   set option_list [list { -O } ]
>  84792   tobi   }
> 
> The string "dg-do  run" does not match, so the torture loops are run.
> 

Whoop. s/are run/are not run/

IIRC, r84792 is the earliest svn revision for gfortran.
Need to go back to the old cvs repository to find out 
more.
-- 
Steve
20170425 https://www.youtube.com/watch?v=VWUpyCsUKR4
20161221 https://www.youtube.com/watch?v=IbCHE-hONow

Re: [patch, fortran] Fix PR 89174, segfault on allocate with MOLD

[Steve Kargl]

On Mon, Feb 25, 2019 at 10:03:29PM +0100, Thomas König wrote:
> Hi Steve,
> 
> >> I think this was introduced quite some time ago, not sure if it
> >> was ever documented anywhere.  I guess we should do so.
> >>
> > Probably want to document this in the testcase.
> 
> I just checked and found 77 occurences in the test suite (most of
> them mine, to be sure).  So, maybe an entry in the wiki would be
> more appropriate.
> 
> I am just trying to think of who introduced this (or if this
> is even gfortran specific), but coming up blank.  Does anybody
> remember?
> 

It looks like a fortuitous accident.  

% svn annotate gcc/testsuite/lib/gfortran-dg.exp
 84792   tobi   # look if this is dg-do-run test, in which case
 84792   tobi   # we cycle through the option list, otherwise we don't
 84792   tobi   if [expr [search_for $test "dg-do run"]] {
135381  janis   set option_list $torture_with_loops
 84792   tobi   } else {
 84792   tobi   set option_list [list { -O } ]
 84792   tobi   }

The string "dg-do  run" does not match, so the torture loops are run.

-- 
Steve

[PR fortran/89492, patch] - [9 Regression] Endless compilation of an invalid TRANSFER after r269177

[Harald Anlauf]

Rev. 269177 uncovered a latent issue in TRANSFER when the MOLD argument
had storage size 0, resulting in an infinite loop.  The attached patch
rejects illegal cases and handles the case where storage size of both
SOURCE and MOLD are 0.  It also verifies proper simplification for
some cases, some of which were not always properly handled before.

Thanks to Dominique for the heads-up which I initially missed.

Regtested on x86_64-pc-linux-gnu; the testcase is based on the report
and slightly extended.

OK for trunk?

Thanks, and sorry for any inconvenience caused,
Harald

2019-02-25  Harald Anlauf  

PR fortran/89492
* check.c (gfc_calculate_transfer_sizes): Handle cases where
storage size of elements of MOLD is 0.

2019-02-25  Harald Anlauf  

PR fortran/89492
* gfortran.dg/pr89492.f90: New test.

Index: gcc/fortran/check.c
===
--- gcc/fortran/check.c (revision 269177)
+++ gcc/fortran/check.c (working copy)
@@ -5487,6 +5487,26 @@
   if (!gfc_element_size (mold, &result_elt_size))
 return false;
 
+  if (result_elt_size == 0 && *source_size > 0)
+{
+  gfc_error ("% argument of % intrinsic at %L "
+ "shall not have storage size 0 when % "
+"argument has size greater than 0", &mold->where);
+  return false;
+}
+
+  /* If MOLD is a scalar and SIZE is absent, the result is a scalar.
+   * If MOLD is an array and SIZE is absent, the result is an array and of
+   * rank one. Its size is as small as possible such that its physical
+   * representation is not shorter than that of SOURCE.
+   */
+  if (result_elt_size == 0 && *source_size == 0 && !size)
+{
+  *result_size = 0;
+  *result_length_p = 0;
+  return true;
+}
+
   if ((result_elt_size > 0 && (mold->expr_type == EXPR_ARRAY || mold->rank))
   || size)
 {
Index: gcc/testsuite/gfortran.dg/pr89492.f90
===
--- gcc/testsuite/gfortran.dg/pr89492.f90   (nonexistent)
+++ gcc/testsuite/gfortran.dg/pr89492.f90   (working copy)
@@ -0,0 +1,27 @@
+! { dg-do compile }
+!
+! PR fortran/89492 - Endless compilation of an invalid TRANSFER after r269177
+! Test error recovery for invalid uses of TRANSFER
+! Test proper simplification for MOLD with size 0
+!
+! Derived from original testcase by Dominique d'Humieres
+
+program bug4a
+  implicit none
+  type bug4
+! Intentionally left empty
+  end type bug4
+  integer, parameter :: k = size(transfer('',['']))  ! k = 0
+  integer, parameter :: i = len (transfer('',['']))  ! i = 0
+  integer, parameter :: l = len (transfer('', '' ))  ! l = 0
+  integer, parameter :: m(k) = k
+  integer, parameter :: j(i) = i
+  integer, parameter :: n(l) = l
+  print *, k,i,l,m,j,n
+  print *,  transfer(1,[''])! { dg-error "shall not have 
storage size 0" }
+  print *,  transfer(1, '' )! { dg-error "shall not have 
storage size 0" }
+  print *, size(transfer(1,['']))   ! { dg-error "shall not have 
storage size 0" }
+  print *, len (transfer(1, '' ))   ! { dg-error "shall not have 
storage size 0" }
+  print *, size(transfer([1],[bug4()])) ! { dg-error "shall not have 
storage size 0" }
+  print *, transfer(transfer([1],[bug4()]),[1]) ! { dg-error "shall not have 
storage size 0" }
+end program bug4a

Re: [patch, fortran] Fix PR 89174, segfault on allocate with MOLD

[Thomas König]

Hi Steve,


I think this was introduced quite some time ago, not sure if it
was ever documented anywhere.  I guess we should do so.


Probably want to document this in the testcase.


I just checked and found 77 occurences in the test suite (most of
them mine, to be sure).  So, maybe an entry in the wiki would be
more appropriate.

I am just trying to think of who introduced this (or if this
is even gfortran specific), but coming up blank.  Does anybody
remember?

Regards

Thomas

Re: [backtrace] Avoid segfault

[Tom de Vries]

On 25-02-19 15:12, Gerald Pfeifer wrote:
> Specifically I am now seeing
> 
>   gmake[4]: *** No rule to make target 'b3test_dwz_buildid', 
>   needed by 'b3test_dwz_buildid.log'.
> 
> in my build/test logs.  (Note, this is GNU make 4.2.1, so might reproduce 
> on your SUSE systems as well?)

Hi Gerald,

I managed to reproduce this by adding:
...
mkdir bin
(
cd bin
ln -s $(which false) dwz
)
export PATH=$(pwd -P)/bin:$PATH
...
to my libbacktrace test script.

The problem is that:
...
TESTS += b3test_dwz_buildid
...
is guarded by HAVE_ELF and HAVE_OBJCOPY_DEBUGLINK, but not by HAVE_DWZ.

Thanks,
- Tom

Re: [PATCH v2][rs6000] PR89338, PR89339: Fix compat vector intrinsics for BE and 32-bit

[Segher Boessenkool]

Hi Paul,

On Tue, Feb 19, 2019 at 03:03:58PM -0600, Paul Clarke wrote:
> Test FAILS: sse2-cvtpd2dq-1, sse2-cvtpd2ps, sse2-cvttpd2dq on powerpc64
> (big-endian).
> 
> _mm_cvtpd_epi32, _mm_cvtpd_ps, _mm_cvttpd_epi32: Type conversion from
> vector doubleword type to vector word type leaves the results in even
> lanes in big endian mode.
> 
> Test FAILS: sse-cvtss2si-1, sse-cvtss2si-2, sse-movmskb-1 on powerpc
> (32-bit big-endian).
> 
> Incorrect type for interpreting the result from mfvsrd instruction leads
> to incorrect results.  Also, mfvsrd instruction only works as expected in
> 64-bit mode or for 32-bit quantities in 32-bit mode.  A more general,
> if slower, solution is needed for 32-bit mode.

Sorry for not reviewing this before.  Thanks for the ping :-)

> 2019-02-19  Paul A. Clarke  
> 
> [gcc]
> 
>   * config/rs6000/emmintrin.h (_mm_cvtpd_epi32): Fix big endian.
>   (_mm_cvtpd_ps): Likewise.
>   (_mm_cvttpd_epi32): Likewise.
> 
>   PR89338

This should be

PR target/89338

>   * config/rs6000/xmmintrin.h (_mm_cvtss_f32):  Fix type mismatch.
>   (_mm_cvt_ss2si): Fix type mismatch and 32-bit.
> 
>   PR89339
>   * config/rs6000/xmmintrin.h (_mm_movemask_pi8): Fix 32-bit.

Okay for trunk with those corrected.  Thanks!


Segher

Re: [EXT] Re: [Patch, Aarch64] Implement TARGET_GET_MULTILIB_ABI_NAME for aarch64 (for use in Fortran vector header file)

[Steve Ellcey]

On Wed, 2019-02-20 at 10:04 +, Richard Sandiford wrote:
> 
> (E.g. __attribute__((vector_size)) never creates an ABI-level SVE vector,
> even with -msve-vector-bits=N, but it can create an ABI-level Advanced
> SIMD vector.)
> 
> I think we should leave the SVE stuff out for now though.  ISTM that:
> 
> !GCC$ builtin (sin) attributes simd (notinbranch) if('aarch64')
> !GCC$ builtin (sin) attributes simd (notinbranch) if('aarch64_sve')
> 
> is trying to say that there's both an Advanced SIMD implementation of sin
> and an SVE implementation of sin.  But AFAICT the patch would just treat
> the two the same way when SVE is enabled, which I think in practice means
> that both would declare an Advanced SIMD function.

It looks like you are right.  I did not know that GCC would essentially
use the non-SVE vector functions to handle SVE vectors. So right now,
GCC doesn't seem to need to differentiate between SVE and non-SVE.

> Once we support SVE vector functions, how would the header file declare
> the Advanced SIMD function when SVE is enabled on the command line?

I guess this is what I was thinking the aarch64_sve if would be for,
but there is a difference between supporting SVE (using non-SVE vector
functions) and supporting SVE with functions that can take and return
an SVE vector.  I think that we should use 'if('aarch64_sve')' to mean
SVE is enabled and we want GCC to call SVE vector functions.  So right
now this would always return false, until such time as GCC is changed
to do calls with SVE vectors as arguments.

Given that it would always return false I guess we could just drop it
out for now.

Steve Ellcey
sell...@marvell.com


FYI: If anyone is interested here is a test program I was compiling to
see what GCC does with SVE, I compiled with -march=armv8.5-a,
-march=armv8.5-a+sve and -msve-vector-bits=[256,2048,scalable] to see
what sort of code got generated.  I was a little surprised that using
-march=armv8.5-a (without the +sve) and -msve-vector-bits= did not give
a warning.  Why set sve-vector-bits if SVE is not enabled.


__attribute__ ((__simd__ ("notinbranch"))) extern float
xsinf (float __x) __attribute__ ((__nothrow__ , __leaf__, __const__));
__attribute__ ((__simd__ ("notinbranch"))) extern float
xcosf (float __x) __attribute__ ((__nothrow__ , __leaf__, __const__));

void foo (float * __restrict__ a,float * __restrict__ b,float *
__restrict__ c)
{
int i;
for (i = 0; i < 1; i++)
a[i] = xsinf(b[i]) * xcosf(c[i]);
}

Re: [patch, fortran] Fix PR 89174, segfault on allocate with MOLD

[Steve Kargl]

On Mon, Feb 25, 2019 at 07:15:32PM +0100, Thomas Koenig wrote:
> Hi Dominique,
> 
> > I see a double space in
> > 
> > ! { dg-do  run }
> > 
> > Is this intended?
> 
> Yes, it is.  This is for tests which should not be run with all
> the options cycling, but only once.
> 
> I think this was introduced quite some time ago, not sure if it
> was ever documented anywhere.  I guess we should do so.
> 

Probably want to document this in the testcase.

! { dg-do  run }
!^^
! Above two spaces are intentional.  It prevents cycling through options.

-- 
Steve

Re: [PATCH v2][rs6000] PR89338, PR89339: Fix compat vector intrinsics for BE and 32-bit

[Paul Clarke]

ping.

On 02/19/2019 03:03 PM, Paul Clarke wrote:
> Test FAILS: sse2-cvtpd2dq-1, sse2-cvtpd2ps, sse2-cvttpd2dq on powerpc64
> (big-endian).
> 
> _mm_cvtpd_epi32, _mm_cvtpd_ps, _mm_cvttpd_epi32: Type conversion from
> vector doubleword type to vector word type leaves the results in even
> lanes in big endian mode.
> 
> Test FAILS: sse-cvtss2si-1, sse-cvtss2si-2, sse-movmskb-1 on powerpc
> (32-bit big-endian).
> 
> Incorrect type for interpreting the result from mfvsrd instruction leads
> to incorrect results.  Also, mfvsrd instruction only works as expected in
> 64-bit mode or for 32-bit quantities in 32-bit mode.  A more general,
> if slower, solution is needed for 32-bit mode.
> 
> 2019-02-19  Paul A. Clarke  
> 
> [gcc]
> 
>   * config/rs6000/emmintrin.h (_mm_cvtpd_epi32): Fix big endian.
>   (_mm_cvtpd_ps): Likewise.
>   (_mm_cvttpd_epi32): Likewise.
> 
>   PR89338
>   * config/rs6000/xmmintrin.h (_mm_cvtss_f32):  Fix type mismatch.
>   (_mm_cvt_ss2si): Fix type mismatch and 32-bit.
> 
>   PR89339
>   * config/rs6000/xmmintrin.h (_mm_movemask_pi8): Fix 32-bit.
> 
> ---
> v2: more elegant solution for the 32-bit mode fix in _mm_movemask_pi8,
> as suggested by Segher.
> 
> Index: gcc/config/rs6000/emmintrin.h
> ===
> diff --git a/trunk/gcc/config/rs6000/emmintrin.h 
> b/trunk/gcc/config/rs6000/emmintrin.h
> --- a/trunk/gcc/config/rs6000/emmintrin.h (revision 268997)
> +++ b/trunk/gcc/config/rs6000/emmintrin.h (working copy)
> @@ -887,7 +887,11 @@ _mm_cvtpd_epi32 (__m128d __A)
>: );
>  
>  #ifdef _ARCH_PWR8
> +#ifdef __LITTLE_ENDIAN__
>temp = vec_mergeo (temp, temp);
> +#else
> +  temp = vec_mergee (temp, temp);
> +#endif
>result = (__v4si) vec_vpkudum ((__vector long long) temp,
>(__vector long long) vzero);
>  #else
> @@ -922,7 +926,11 @@ _mm_cvtpd_ps (__m128d __A)
>: );
>  
>  #ifdef _ARCH_PWR8
> +#ifdef __LITTLE_ENDIAN__
>temp = vec_mergeo (temp, temp);
> +#else
> +  temp = vec_mergee (temp, temp);
> +#endif
>result = (__v4sf) vec_vpkudum ((__vector long long) temp,
>(__vector long long) vzero);
>  #else
> @@ -951,7 +959,11 @@ _mm_cvttpd_epi32 (__m128d __A)
>: );
>  
>  #ifdef _ARCH_PWR8
> +#ifdef __LITTLE_ENDIAN__
>temp = vec_mergeo (temp, temp);
> +#else
> +  temp = vec_mergee (temp, temp);
> +#endif
>result = (__v4si) vec_vpkudum ((__vector long long) temp,
>(__vector long long) vzero);
>  #else
> Index: gcc/config/rs6000/xmmintrin.h
> ===
> diff --git a/trunk/gcc/config/rs6000/xmmintrin.h 
> b/trunk/gcc/config/rs6000/xmmintrin.h
> --- a/trunk/gcc/config/rs6000/xmmintrin.h (revision 268997)
> +++ b/trunk/gcc/config/rs6000/xmmintrin.h (working copy)
> @@ -905,7 +905,7 @@ _mm_cvtss_f32 (__m128 __A)
>  extern __inline int __attribute__((__gnu_inline__, __always_inline__, 
> __artificial__))
>  _mm_cvtss_si32 (__m128 __A)
>  {
> -  __m64 res = 0;
> +  int res;
>  #ifdef _ARCH_PWR8
>double dtmp;
>__asm__(
> @@ -938,8 +938,8 @@ _mm_cvt_ss2si (__m128 __A)
>  extern __inline long long __attribute__((__gnu_inline__, __always_inline__, 
> __artificial__))
>  _mm_cvtss_si64 (__m128 __A)
>  {
> -  __m64 res = 0;
> -#ifdef _ARCH_PWR8
> +  long long res;
> +#if defined (_ARCH_PWR8) && defined (__powerpc64__)
>double dtmp;
>__asm__(
>  #ifdef __LITTLE_ENDIAN__
> @@ -1577,6 +1577,7 @@ _m_pminub (__m64 __A, __m64 __B)
>  extern __inline int __attribute__((__gnu_inline__, __always_inline__, 
> __artificial__))
>  _mm_movemask_pi8 (__m64 __A)
>  {
> +#ifdef __powerpc64__
>unsigned long long p =
>  #ifdef __LITTLE_ENDIAN__
>   0x0008101820283038UL; // permute control for sign 
> bits
> @@ -1584,6 +1585,12 @@ _mm_movemask_pi8 (__m64 __A)
>   0x3830282018100800UL; // permute control for sign 
> bits
>  #endif
>return __builtin_bpermd (p, __A);
> +#else
> +  unsigned int mask = 0x20283038UL;
> +  unsigned int r1 = __builtin_bpermd (mask, __A) & 0xf;
> +  unsigned int r2 = __builtin_bpermd (mask, __A >> 32) & 0xf;
> +  return (r2 << 4) | r1;
> +#endif
>  }
>  
>  extern __inline int __attribute__((__gnu_inline__, __always_inline__, 
> __artificial__))
> 

Re: [patch, fortran] Fix PR 89174, segfault on allocate with MOLD

[Thomas Koenig]

Hi Dominique,


I see a double space in

! { dg-do  run }

Is this intended?


Yes, it is.  This is for tests which should not be run with all
the options cycling, but only once.

I think this was introduced quite some time ago, not sure if it
was ever documented anywhere.  I guess we should do so.

Regards

Thomas

Re: [PATCH][GCC][AArch64] Fix command line options canonicalization version #2. (PR target/88530)

[Tamar Christina]

The 02/21/2019 22:34, James Greenhalgh wrote:
> On Wed, Feb 20, 2019 at 08:00:38AM -0600, Tamar Christina wrote:
> > Hi All,
> > 
> > Commandline options on AArch64 don't get canonicalized into the smallest
> > possible set before output to the assembler. This means that overlapping 
> > feature
> > sets are emitted with superfluous parts.
> > 
> > Normally this isn't an issue, but in the case of crypto we have 
> > retro-actively
> > split it into aes and sha2. We need to emit only +crypto to the assembler
> > so old assemblers continue to work.
> > 
> > Because of how -mcpu=native and -march=native work they end up enabling all
> > feature bits. Instead we need to get the smallest possible set, which would 
> > also
> > fix the problem with older the assemblers and the retro-active split.
> > 
> > The function that handles this is called quite often.  It is called for any
> > push/pop options or attribute that changes arch, cpu etc.  In order to not 
> > make
> > this search for the smallest set too expensive we sort the options based on 
> > the
> > number of features (bits) they enable.  This allows us to process the list
> > linearly instead of quadratically (Once we have enabled a feature, we know 
> > that
> > anything else that enables it can be ignored.  By sorting we'll get the 
> > biggest
> > groups first and thus the smallest combination of commandline flags).
> > 
> > The Option handling structures have been extended to have a boolean to 
> > indicate
> > whether the option is synthetic, with that I mean if the option flag itself
> > enables a new feature.
> > 
> > e.g. +crypto isn't an actual feature, it just enables other features, but 
> > others
> > like +rdma enable multiple dependent features but is itself also a feature.
> > 
> > There are two ways to solve this.
> > 
> > 1) Either have the options that are feature bits also turn themselves on, 
> > e.g.
> >change rdma to turn on FP, SIMD and RDMA as dependency bits.
> > 
> > 2) Make a distinction between these two different type of features and have 
> > the
> >framework handle it correctly.
> > 
> > Even though it's more code I went for the second approach, as it's the one
> > that'll be less fragile (people can't forget it) and gives the least 
> > surprises.
> > 
> > Effectively this patch changes the following:
> > 
> > The values before the => are the old compiler and after the => the new code.
> > 
> > -march=armv8.2-a+crypto+sha2 => -march=armv8.2-a+crypto
> > -march=armv8.2-a+sha2+aes => -march=armv8.2-a+crypto
> > 
> > The remaining behaviors stay the same.
> > 
> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> > 
> > Ok for trunk?
> 
> OK, but I don't understand why the CRC special case is needed. My copy of
> the Arm Architecture Reference Manual suggests that all versions of the
> architceture from ARmv8.1-A are required to implement the CRC32 extension.
> Is there some old assembler that doesn't honour that? Whatever is driving
> that requirement could usefully be added to the comments.

From my understanding it's because of a bug in an older assembler.  I am not
sure really what our support Window for this kind of stuff is to be honest.
But probably somewhere, someone will still be using the latest GCC with a
very old binutils.

I have updated the patch with a comment to this effect.  I have also made a
small change in splitting the loop that emits + and +no into two
different ones.  No change aside from the copied "for..".  The reason for this
is that the assemblers expect all + before +no, as a final full toolchain
regtest pointed out.

I have committed this as trivial as no change was done to the working of the
options loop. And also added a comment to this requirement in the code.

Thanks,
Tamar

> 
> I find it very hard to believe that this code is what we need for correct
> behaviour on AArch64; this level of complexity implies we're doing something
> very wrong with either the definition or the implementation of these features
> bits which makes it hard for us to maintain, and hard for users and dependent
> tools (e.g. assemblers) to know what to expect from the compiler.
> 
> I've seen a number of bugs in this code recently. While I appreciate your
> patch for fixing one of them, I find the cases and expectations so hard
> to reason about that I can't say I am sure we are now bug free.
> 
> OK for trunk as an improvement over today, and to help us get towards a
> release; but I'm very unhappy with this corner of the compiler!
> 
> Thanks,
> James
> 
> > gcc/ChangeLog:
> > 
> > 2019-02-20  Tamar Christina  
> > 
> > PR target/88530
> > * common/config/aarch64/aarch64-common.c
> > (struct aarch64_option_extension): Add is_synthetic.
> > (all_extensions): Use it.
> > (TARGET_OPTION_INIT_STRUCT): Define hook.
> > (struct gcc_targetm_common): Moved to end.
> > (all_extensions_by_on): New.
> > (opt_ext_cmp, typedef opt_ext): New.
> > (aarch64_option_init_s

[PATCH, OpenACC, libgomp, v6, stage1] Async-rework update

[Chung-Lin Tang]

Hi Thomas,
I have incorporated all your patches you've included in the last mail (with
some modifications, though pretty minor I think).

The default_async, GOMP_PLUGIN_IF_VERSION, and testsuite changes have all been 
removed.
We can work on them later as we clarify more things.

Thanks,
Chung-Lin
Index: libgomp/oacc-async.c
===
--- libgomp/oacc-async.c(revision 269183)
+++ libgomp/oacc-async.c(working copy)
@@ -27,49 +27,162 @@
.  */
 
 #include 
+#include 
 #include "openacc.h"
 #include "libgomp.h"
 #include "oacc-int.h"
 
-int
-acc_async_test (int async)
+static struct goacc_thread *
+get_goacc_thread (void)
 {
-  if (!async_valid_p (async))
-gomp_fatal ("invalid async argument: %d", async);
-
   struct goacc_thread *thr = goacc_thread ();
 
   if (!thr || !thr->dev)
 gomp_fatal ("no device active");
 
-  return thr->dev->openacc.async_test_func (async);
+  return thr;
 }
 
-int
-acc_async_test_all (void)
+static struct gomp_device_descr *
+get_goacc_thread_device (void)
 {
   struct goacc_thread *thr = goacc_thread ();
 
   if (!thr || !thr->dev)
 gomp_fatal ("no device active");
 
-  return thr->dev->openacc.async_test_all_func ();
+  return thr->dev;
 }
 
-void
-acc_wait (int async)
+static int
+validate_async_val (int async)
 {
   if (!async_valid_p (async))
-gomp_fatal ("invalid async argument: %d", async);
+gomp_fatal ("invalid async-argument: %d", async);
 
+  if (async == acc_async_sync)
+return -1;
+
+  if (async == acc_async_noval)
+return 0;
+
+  if (async >= 0)
+/* TODO: we reserve 0 for acc_async_noval before we can clarify the
+   semantics of "default_async".  */
+return 1 + async;
+  else
+__builtin_unreachable ();
+}
+
+/* Return the asyncqueue to be used for OpenACC async-argument ASYNC.  This
+   might return NULL if no asyncqueue is to be used.  Otherwise, if CREATE,
+   create the asyncqueue if it doesn't exist yet.  */
+
+attribute_hidden struct goacc_asyncqueue *
+lookup_goacc_asyncqueue (struct goacc_thread *thr, bool create, int async)
+{
+  async = validate_async_val (async);
+  if (async < 0)
+return NULL;
+
+  struct goacc_asyncqueue *ret_aq = NULL;
+  struct gomp_device_descr *dev = thr->dev;
+
+  gomp_mutex_lock (&dev->openacc.async.lock);
+
+  if (!create
+  && (async >= dev->openacc.async.nasyncqueue
+ || !dev->openacc.async.asyncqueue[async]))
+goto end;
+
+  if (async >= dev->openacc.async.nasyncqueue)
+{
+  int diff = async + 1 - dev->openacc.async.nasyncqueue;
+  dev->openacc.async.asyncqueue
+   = gomp_realloc (dev->openacc.async.asyncqueue,
+   sizeof (goacc_aq) * (async + 1));
+  memset (dev->openacc.async.asyncqueue + dev->openacc.async.nasyncqueue,
+ 0, sizeof (goacc_aq) * diff);
+  dev->openacc.async.nasyncqueue = async + 1;
+}
+
+  if (!dev->openacc.async.asyncqueue[async])
+{
+  dev->openacc.async.asyncqueue[async] = dev->openacc.async.construct_func 
();
+
+  if (!dev->openacc.async.asyncqueue[async])
+   {
+ gomp_mutex_unlock (&dev->openacc.async.lock);
+ gomp_fatal ("async %d creation failed", async);
+   }
+  
+  /* Link new async queue into active list.  */
+  goacc_aq_list n = gomp_malloc (sizeof (struct goacc_asyncqueue_list));
+  n->aq = dev->openacc.async.asyncqueue[async];
+  n->next = dev->openacc.async.active;
+  dev->openacc.async.active = n;
+}
+
+  ret_aq = dev->openacc.async.asyncqueue[async];
+
+ end:
+  gomp_mutex_unlock (&dev->openacc.async.lock);
+  return ret_aq;
+}
+
+/* Return the asyncqueue to be used for OpenACC async-argument ASYNC.  This
+   might return NULL if no asyncqueue is to be used.  Otherwise, create the
+   asyncqueue if it doesn't exist yet.  */
+
+attribute_hidden struct goacc_asyncqueue *
+get_goacc_asyncqueue (int async)
+{
+  struct goacc_thread *thr = get_goacc_thread ();
+  return lookup_goacc_asyncqueue (thr, true, async);
+}
+
+int
+acc_async_test (int async)
+{
   struct goacc_thread *thr = goacc_thread ();
 
   if (!thr || !thr->dev)
 gomp_fatal ("no device active");
 
-  thr->dev->openacc.async_wait_func (async);
+  goacc_aq aq = lookup_goacc_asyncqueue (thr, false, async);
+  if (!aq)
+return 1;
+  else
+return thr->dev->openacc.async.test_func (aq);
 }
 
+int
+acc_async_test_all (void)
+{
+  struct goacc_thread *thr = get_goacc_thread ();
+
+  int ret = 1;
+  gomp_mutex_lock (&thr->dev->openacc.async.lock);
+  for (goacc_aq_list l = thr->dev->openacc.async.active; l; l = l->next)
+if (!thr->dev->openacc.async.test_func (l->aq))
+  {
+   ret = 0;
+   break;
+  }
+  gomp_mutex_unlock (&thr->dev->openacc.async.lock);
+  return ret;
+}
+
+void
+acc_wait (int async)
+{
+  struct goacc_thread *thr = get_goacc_thread ();
+
+  goacc_aq aq = lookup_goacc_asyncqueue (thr, false, async);
+  

Re: Move -Wmaybe-uninitialized to -Wextra

[Michael Matz]

Hi,

On Wed, 20 Feb 2019, Jeff Law wrote:

> No, I'm saying the distinction between maybe and always uninitialized is
> a false distinction.  Code duplication can easily take something that
> triggers a "maybe" warning and turn it into a "always" warning.  The
> distinction between them is just bogus.
> 
> To take your example
> 
> if (a)
>   b = 42
> ...
> if (a)
>   f(b);
> 
> Can be rewritten as
> 
> if (a) {
>   b = 42
>   [ copy of ...]
>   f (b);
> } else {
>   ...
>   f (b);   // xxx
> }

No, it cannot.  It would introduce a call to f(b) in a path (at xxx) where 
it wasn't before.  Any transformation that transforms maybe-uninit into 
must-uninit is either wrong or introduces more-obviously-dead-than-before 
code, like in this variant of yours that is correct:

if (a) {
  b = 42
  [ copy of ...]
  f (b);
} else {
  ...
  if (a) // clearly false, as dominated by if (!a)
f (b);   // hence more obviously dead than in original code
}

> x4 = PHI (x0(D), x1, x2, x3)
> use x4;
> 
> That's a maybe uninitialized warning by way of the value flow of x0
> through the PHI into x4.

So clearly the use of x4 when reached via edge using x0(D) must be dead 
(or we have a real uninit use) but we can't differ because there's only 
one use of x4 statically ... nevertheless it's unobviously-dead.

> Which we then simplify into
> 
> BB:
> use x0(D)
> goto join
> 
> BB:
> x4 = PHI (x1, x2, x3)
> use x4
> goto join

And this makes the uninit use of x0(D) more obviously dead because now the 
problematic and non-problematic uses are separated.  So, the 
transformation introduced more-obviously-dead code, which it just as well 
could have removed/not generated, getting rid of the must-uninit 
warning.

I guess what I'm saying is that whenever a transformation is about to 
introduce a must-uninit from a maybe-uninit that instead all paths leading 
to that must-uninit should be removed instead.

> That changes the warning from a maybe-uninitialized into an always
> uninitialized.  Note that we still may or may not reach the use at
> runtime.   THe distinction between maybe and always is just bogus.

I think as stated that's a bit extreme :)  On user code before 
optimization the distinction is useful.  The problem stems from 
us generating warnings somewhen in the middle of the optimization 
pipeline.  As long as we continue doing that we will always have these 
different opinions about what is or isn't rightfully a maybe-uninit or not 
:-/


Ciao,
Michael.

Re: [PATCH doc] correct/improve -Wmissing-attributes and -Wattribute-alias

[Martin Sebor]

On 2/15/19 6:57 PM, Sandra Loosemore wrote:

On 2/6/19 9:16 AM, Martin Sebor wrote:

The manual documents the -Wno-missing-attributes form of the option
as if it was enabled by default, even though it's enabled by -Wall
(I can't get this -Wno- convention straight in my head).  I also
got private comments on the documentation of the option suggesting
to add cross-references, and to list the attributes
-Wattribute-alias considers (the same ones as -Wmissing-attributes).

The attached patch makes these changes.


I found the discussion of both options incomprehensible even with this 
patch.  :-(  The defaults are incorrect, there are typos, awkward 
wording and confusing paragraph organization, etc.  So I consulted the 
sources and came up with the attached alternative patch.  Can you review 
this for correctness and generally making sense?


Thanks.  I think your update is fine.  There's some overlap between
the -Wattribute-alias and -Wmissing-attributes warnings, as well
as some gaps in one or both, that could stand to be reviewed and
adjusted.  I'm also don't think that diagnosing likely bugs only
at the optional level 2 and diagnosing potential optimization
oportunities at the default level 1 was the right decision.  But
that will have to wait until GCC 10.

Martin

Index: gcc/common.opt
...
@@ -552,11 +552,11 @@ Warn about inappropriate attribute usage

 Wattribute-alias
 Common Alias(Wattribute_alias=, 1, 0) Warning
-Warn about type safety and similar errors and mismatches in attribute 
alias and related.
+Warn about type safety and similar errors and mismatches in 
declarations with alias attributes.


The "and related" part refers to other attributes that are similar to
but distinct from alias.  I think the only such attribute is weakref.
It's probably not essential to make this one sentence 100% accurate
but I thought I'd mention it just for clarity.

Re: [PATCH][libbacktrace] Add btest_lto

[Tom de Vries]

On 25-02-19 11:48, Thomas Schwinge wrote:
> Hi Tom!
> 
> On Fri, 8 Feb 2019 10:42:24 +0100, Tom de Vries  wrote:
>> Add libbacktrace test-case using -flto.
> 
> I'm seeing this one fail is some configurations, but only in the
> 'build-gcc/libbacktrace/btest_lto.log' variant:
> 

Hi Thomas,

Meaning, compiling libbacktrace using the host compiler, so it would be
useful to known which compiler that is.

[ I've tried a gcc-4.8 and gcc-6 and gcc-7 as host compiler, and a
couple of CFLAGS settings (-O2, -O3) to reproduce this, but didn't manage. ]

> test5: unexpected syminfo name got global.2537 expected global
> PASS: backtrace_full noinline
> PASS: backtrace_full inline
> PASS: backtrace_simple noinline
> PASS: backtrace_simple inline
> FAIL: backtrace_syminfo variable
> FAIL btest_lto (exit status: 1)
> 
> I haven't looked yet which details about these GCC build configurations
> might be different/important; maybe you've got an idea already?
> 

Well, the backtrace_syminfo function looks at the minimal symbol info
(so, not the dwarf info) and it seems lto has done an optimization that
has changed the name of the variable in the minimal symbol info.

There's probably a standard way to annotate the 'global' variable to
prevent the optimization from happening, which would fix the failure
(but, we need to know which optimization renamed it).

OTOH, we could just limit this test to target libbacktrace only, given
the fact that host compilers may not even support flto.

Thanks,
- Tom

[committed fortran] New test for PR89282 - Garbage arithmetics results in fortran with -O3 and overloaded operators

[Dominique d'Humières]

New test committed as r269190.

Dominique

[C++ Patch] PR 89488 ("[9 Regression] ICE in merge_exception_specifiers, at cp/typeck2.c:2395")

[Paolo Carlini]

Hi,

this error recovery regression has to do with the recent changes 
committed by Jason for c++/88368. What happens is that 
maybe_instantiate_noexcept fails the hard way, thus, toward the end of 
the function, doesn't update TREE_TYPE (fn) and just returns false. 
process_subob_fn doesn't notice and proceeds to call 
merge_exception_specifiers anyway where of course the gcc_assert 
(!DEFERRED_NOEXCEPT_SPEC_P (add)) triggers, because 
maybe_instantiate_noexcept has not done its normal job. To improve 
error-recovery I think we can simply leave *spec_p alone in such cases, 
because we would merge the *spec_p with a TYPE_RAISES_EXCEPTIONS 
(TREE_TYPE (fn)) where TREE_TYPE (fn) has not been normally computed. I 
tried a few other things which prima facie looked sensible but nothing 
else worked - eg, returning false from maybe_instantiate_noexcept and 
also updating TREE_TYPE (fn) to a noexcept_false_spec variant causes 
regressions exactly for the testcases of c++/88368.


Tested x86_64-linux.

Thanks, Paolo.



/cp
2019-02-25  Paolo Carlini  

PR c++/89488
* method.c (process_subob_fn): When maybe_instantiate_noexcept returns
false don't call merge_exception_specifiers.

/testsuite
2019-02-25  Paolo Carlini  

PR c++/89488
* g++.dg/cpp0x/nsdmi15.C: New.
Index: cp/method.c
===
--- cp/method.c (revision 269187)
+++ cp/method.c (working copy)
@@ -1254,9 +1254,8 @@ process_subob_fn (tree fn, tree *spec_p, bool *tri
   return;
 }
 
-  if (spec_p)
+  if (spec_p && maybe_instantiate_noexcept (fn))
 {
-  maybe_instantiate_noexcept (fn);
   tree raises = TYPE_RAISES_EXCEPTIONS (TREE_TYPE (fn));
   *spec_p = merge_exception_specifiers (*spec_p, raises);
 }
Index: testsuite/g++.dg/cpp0x/nsdmi15.C
===
--- testsuite/g++.dg/cpp0x/nsdmi15.C(nonexistent)
+++ testsuite/g++.dg/cpp0x/nsdmi15.C(working copy)
@@ -0,0 +1,8 @@
+// PR c++/89488
+// { dg-do compile { target c++11 } }
+
+struct zl {
+  struct {
+int x2 = zl ();  // { dg-error "default member|cannot convert" }
+  } fx;
+};

[committed] df-scan: fix use of mw_order in df_mw_compare (PR 86096)

[Alexander Monakov]

Hi,

df_mw_compare tries to order df_mw_hardreg structures lexicographically, but
the last comparison step wrongly tests one field (mw_reg) while subtracting
another (mw_order).  This makes the comparison non-transitive.

Fix this by simply returning difference of mw_order.

Patch pre-approved by Richi in the Bugzilla, bootstrapped on x86_64, applied.

Alexander

PR rtl-optimization/86096
* df-scan.c (df_mw_compare): Do not check mw_reg fields when
comparing mw_order values.

--- a/gcc/df-scan.c
+++ b/gcc/df-scan.c
@@ -2208,10 +2208,7 @@ df_mw_compare (const df_mw_hardreg *mw1, const 
df_mw_hardreg *mw2)
   if (mw1->end_regno != mw2->end_regno)
 return mw1->end_regno - mw2->end_regno;

-  if (mw1->mw_reg != mw2->mw_reg)
-return mw1->mw_order - mw2->mw_order;
-
-  return 0;
+  return mw1->mw_order - mw2->mw_order;
 }

 /* Like df_mw_compare, but compare two df_mw_hardreg** pointers R1 and R2.  */

[committed] Add the primary testcase from PR c++/89285

[Jakub Jelinek]

Hi!

This testcase ICEs on the 8.x branch and ICEd as well with my earlier
approach to get the testcase rejected on the trunk again when foo is
constexpr, but actually with the latest approach that was committed
(where constexpr evaluation is done on pre-cp_fold_function trees and
cp_fold can optimize away those reinterpret_casts) it doesn't ICE.

So, I've tested this on the trunk with check-c++-all, verified that 8.x ICEs
on it and committed to trunk as obvious.

2019-02-25  Jakub Jelinek  

PR c++/89285
* g++.dg/cpp1y/constexpr-89285-2.C: New test.

--- gcc/testsuite/g++.dg/cpp1y/constexpr-89285-2.C.jj   2019-02-25 
15:47:55.189265284 +0100
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-89285-2.C  2019-02-25 
15:47:34.937592247 +0100
@@ -0,0 +1,20 @@
+// PR c++/89285
+// { dg-do compile { target c++14 } }
+
+struct A {
+  int a {};
+};
+struct B {
+  int b {};
+  constexpr B (A *x) {
+int *c = &x->a;
+while (*c)
+  c = reinterpret_cast((reinterpret_cast(c) + *c));
+*c = reinterpret_cast(this) - reinterpret_cast(c);
+  }
+};
+struct C : A {
+  B bar {this};
+};
+
+C foo {};

Jakub

Re: [backtrace] Avoid segfault

[Gerald Pfeifer]

Hi Tom,

I'm afraid this triggers on my (FreeBSD-based) testers:

  2019-01-29  Tom de Vries  

* install-debuginfo-for-buildid.sh.in: New script.
* Makefile.am (check_PROGRAMS): Add b2test and b3test.
(TESTS): Add b2test_buildid and b3test_dwz_buildid.
* Makefile.in: Regenerate.
* configure.ac (HAVE_ELF): Set with AM_CONDITIONAL.
(READELF): Set with AC_CHECK_PROG.
(install-debuginfo-for-buildid.sh): Generate with AC_CONFIG_FILES.
* configure: Regenerate.
* elf.c (SYSTEM_BUILD_ID_DIR): Factor out of ...
(elf_open_debugfile_by_buildid): ... here.

Specifically I am now seeing

  gmake[4]: *** No rule to make target 'b3test_dwz_buildid', 
  needed by 'b3test_dwz_buildid.log'.

in my build/test logs.  (Note, this is GNU make 4.2.1, so might reproduce 
on your SUSE systems as well?)

Gerald

Re: [patch, fortran] Fix PR 89174, segfault on allocate with MOLD

[Dominique d'Humières]

Hi Thomas,

I see a double space in

! { dg-do  run }

Is this intended? If yes, it should probably be documented, otherwise it should 
be fixed.

TIA

Dominique

[PATCH committed Fortran] PR89274 - Inconsistent list directed output of INTEGER(16)

[Dominique d'Humières]

Committed as revision r269187.

Dominique

Re: [PATCH][libbacktrace] Add btest_lto

[Thomas Schwinge]

Hi Tom!

On Fri, 8 Feb 2019 10:42:24 +0100, Tom de Vries  wrote:
> Add libbacktrace test-case using -flto.

I'm seeing this one fail is some configurations, but only in the
'build-gcc/libbacktrace/btest_lto.log' variant:

test5: unexpected syminfo name got global.2537 expected global
PASS: backtrace_full noinline
PASS: backtrace_full inline
PASS: backtrace_simple noinline
PASS: backtrace_simple inline
FAIL: backtrace_syminfo variable
FAIL btest_lto (exit status: 1)

I haven't looked yet which details about these GCC build configurations
might be different/important; maybe you've got an idea already?


Grüße
 Thomas


> [libbacktrace] Add btest_lto
> 
> 2019-02-08  Tom de Vries  
> 
>   * Makefile.am (BUILDTESTS): Add btest_lto.
>   * Makefile.in: Regenerate.
>   * btest.c (test1, f2, f3, test3, f22, f23): Declare with
>   __attribute__((noclone)).
> 
> ---
>  libbacktrace/Makefile.am |  6 ++
>  libbacktrace/Makefile.in | 47 ---
>  libbacktrace/btest.c | 12 ++--
>  3 files changed, 52 insertions(+), 13 deletions(-)
> 
> diff --git a/libbacktrace/Makefile.am b/libbacktrace/Makefile.am
> index 71a2ed478cc..46d7de48fd1 100644
> --- a/libbacktrace/Makefile.am
> +++ b/libbacktrace/Makefile.am
> @@ -222,6 +222,12 @@ btest_LDADD = libbacktrace.la
>  
>  BUILDTESTS += btest
>  
> +btest_lto_SOURCES = btest.c testlib.c
> +btest_lto_CFLAGS = $(AM_CFLAGS) -g -O -flto
> +btest_lto_LDADD = libbacktrace.la
> +
> +BUILDTESTS += btest_lto
> +
>  btest_alloc_SOURCES = $(btest_SOURCES)
>  btest_alloc_CFLAGS = $(btest_CFLAGS)
>  btest_alloc_LDADD = libbacktrace_alloc.la
> diff --git a/libbacktrace/Makefile.in b/libbacktrace/Makefile.in
> index a2b595e9bb0..c65c40d95d8 100644
> --- a/libbacktrace/Makefile.in
> +++ b/libbacktrace/Makefile.in
> @@ -126,8 +126,8 @@ TESTS = $(am__append_4) $(am__append_6) $(am__append_7) \
>  @HAVE_ELF_TRUE@@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@am__append_1 = 
> libbacktrace_elf_for_test.la
>  @NATIVE_TRUE@am__append_2 = test_elf test_xcoff_32 test_xcoff_64 \
>  @NATIVE_TRUE@test_pecoff test_unknown unittest unittest_alloc \
> -@NATIVE_TRUE@btest btest_alloc stest stest_alloc ztest \
> -@NATIVE_TRUE@ztest_alloc edtest edtest_alloc
> +@NATIVE_TRUE@btest btest_lto btest_alloc stest stest_alloc \
> +@NATIVE_TRUE@ztest ztest_alloc edtest edtest_alloc
>  @NATIVE_TRUE@am__append_3 = allocfail
>  @NATIVE_TRUE@am__append_4 = allocfail.sh
>  @HAVE_ELF_TRUE@@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@am__append_5 = 
> b2test \
> @@ -205,10 +205,10 @@ libbacktrace_noformat_la_OBJECTS =  \
>  @NATIVE_TRUE@test_xcoff_64$(EXEEXT) test_pecoff$(EXEEXT) \
>  @NATIVE_TRUE@test_unknown$(EXEEXT) unittest$(EXEEXT) \
>  @NATIVE_TRUE@unittest_alloc$(EXEEXT) btest$(EXEEXT) \
> -@NATIVE_TRUE@btest_alloc$(EXEEXT) stest$(EXEEXT) \
> -@NATIVE_TRUE@stest_alloc$(EXEEXT) ztest$(EXEEXT) \
> -@NATIVE_TRUE@ztest_alloc$(EXEEXT) edtest$(EXEEXT) \
> -@NATIVE_TRUE@edtest_alloc$(EXEEXT)
> +@NATIVE_TRUE@btest_lto$(EXEEXT) btest_alloc$(EXEEXT) \
> +@NATIVE_TRUE@stest$(EXEEXT) stest_alloc$(EXEEXT) \
> +@NATIVE_TRUE@ztest$(EXEEXT) ztest_alloc$(EXEEXT) \
> +@NATIVE_TRUE@edtest$(EXEEXT) edtest_alloc$(EXEEXT)
>  @HAVE_PTHREAD_TRUE@@NATIVE_TRUE@am__EXEEXT_4 = ttest$(EXEEXT) \
>  @HAVE_PTHREAD_TRUE@@NATIVE_TRUE@ ttest_alloc$(EXEEXT)
>  @HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@am__EXEEXT_5 =  \
> @@ -253,6 +253,13 @@ btest_alloc_OBJECTS = $(am_btest_alloc_OBJECTS)
>  btest_alloc_LINK = $(LIBTOOL) $(AM_V_lt) --tag=CC $(AM_LIBTOOLFLAGS) \
>   $(LIBTOOLFLAGS) --mode=link $(CCLD) $(btest_alloc_CFLAGS) \
>   $(CFLAGS) $(AM_LDFLAGS) $(LDFLAGS) -o $@
> +@NATIVE_TRUE@am_btest_lto_OBJECTS = btest_lto-btest.$(OBJEXT) \
> +@NATIVE_TRUE@btest_lto-testlib.$(OBJEXT)
> +btest_lto_OBJECTS = $(am_btest_lto_OBJECTS)
> +@NATIVE_TRUE@btest_lto_DEPENDENCIES = libbacktrace.la
> +btest_lto_LINK = $(LIBTOOL) $(AM_V_lt) --tag=CC $(AM_LIBTOOLFLAGS) \
> + $(LIBTOOLFLAGS) --mode=link $(CCLD) $(btest_lto_CFLAGS) \
> + $(CFLAGS) $(AM_LDFLAGS) $(LDFLAGS) -o $@
>  @HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@am_ctesta_OBJECTS = 
> ctesta-btest.$(OBJEXT) \
>  @HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@ctesta-testlib.$(OBJEXT)
>  ctesta_OBJECTS = $(am_ctesta_OBJECTS)
> @@ -410,7 +417,7 @@ SOURCES = $(libbacktrace_la_SOURCES) 
> $(EXTRA_libbacktrace_la_SOURCES) \
>   $(libbacktrace_instrumented_alloc_la_SOURCES) \
>   $(libbacktrace_noformat_la_SOURCES) $(allocfail_SOURCES) \
>   $(b2test_SOURCES) $(b3test_SOURCES) $(btest_SOURCES) \
> - $(btest_alloc_SOURCES) $(ctesta_SOURCES) \
> + $(btest_alloc_SOURCES) $(btest_lto_SOURCES) $(ctesta_SOURCES) \
>   $(ctesta_alloc_SOURCES) $(ctestg_SOURCES) \
>   $(ctestg_alloc_SOURCES) $(edtest_SOURCES) \
>   $(edtest_alloc_SOURCES) $(stest_SOURCES) \
> @@ -

Re: [PATCH] Fix arm *subsi3_carryin_{compare_,}const patterns (PR target/89434)

[Kyrill Tkachov]

On 2/24/19 2:26 PM, Jakub Jelinek wrote:

Hi!

As the testcase shows, *subsi3_carryin_{const,compare_const} patterns
don't express in RTL what they are actually doing, which may (on the
testcase) does cause miscompilation if we manage to propagate constants
into it or for other reason simplify-rtx.c etc. tries to simplify them,
or if combiner matches them for how they look like.

The *subsi3_carryin{,_compare} patterns look correct, the reason why the
_const patterns want to be different is that the canonical form of
(minus (reg) (const_int N)) is actually (plus (reg) (const_int -N));
the patterns were actually implementing (plus (reg) (const_int ~N))
which is off-by-one.  I had to fix up also two splitters that were
generating these.  Finally, (plus (reg) (const_int 0)) is not canonical,
it should be just (reg), but it is pretty widely used case,
so I've added two simpler patterns for those (the const0 ones).

Bootstrapped/regtested on armv7hl-linux-gnueabi, ok for trunk?



Ouch, how did we get away with this so far? :(

Ok.

Thanks,

Kyrill



2019-02-24  Jakub Jelinek  

    PR target/89434
    * config/arm/arm.md (*subsi3_carryin_const): Use
    arm_neg_immediate_operand predicate instead of
    arm_not_immediate_operand, "L" constraint instead of "K" and
    print it using %n2 instead of %B2.
    (*subsi3_carryin_const0): New define_insn.
    (*subsi3_carryin_compare_const): Use const_int_I_operand predicate
    instead of arm_not_operand and "I" constraint instead of "K" and
    print it using %n3 instead of %B2.  Instead of using match_dup 
2 add
    another match_operand and in the condition check that it is 
negation

    of operands[2].
    (*subsi3_carryin_compare_const0): New define_ins.
    (*subdi_di_zesidi): Adjust to use *subsi3_carryin_const0 
instead of

    *subsi3_carryin_const.
    (*arm_cmpdi_insn): Fix splitting into 
*subsi3_carryin_compare_const,

    split into *subsi3_carryin_compare_const0 if the highpart is zero.

    * gcc.c-torture/execute/pr89434.c: New test.

--- gcc/config/arm/arm.md.jj    2019-02-22 15:22:16.034999035 +0100
+++ gcc/config/arm/arm.md   2019-02-23 12:10:47.079659675 +0100
@@ -1145,10 +1145,20 @@ (define_insn "*subsi3_carryin"
 (define_insn "*subsi3_carryin_const"
   [(set (match_operand:SI 0 "s_register_operand" "=r")
 (minus:SI (plus:SI (match_operand:SI 1 "s_register_operand" "r")
-   (match_operand:SI 2 
"arm_not_immediate_operand" "K"))
+   (match_operand:SI 2 
"arm_neg_immediate_operand" "L"))

   (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0]
   "TARGET_32BIT"
-  "sbc\\t%0, %1, #%B2"
+  "sbc\\t%0, %1, #%n2"
+  [(set_attr "conds" "use")
+   (set_attr "type" "adc_imm")]
+)
+
+(define_insn "*subsi3_carryin_const0"
+  [(set (match_operand:SI 0 "s_register_operand" "=r")
+    (minus:SI (match_operand:SI 1 "s_register_operand" "r")
+  (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0]
+  "TARGET_32BIT"
+  "sbc\\t%0, %1, #0"
   [(set_attr "conds" "use")
    (set_attr "type" "adc_imm")]
 )
@@ -1170,13 +1180,26 @@ (define_insn "*subsi3_carryin_compare"
 (define_insn "*subsi3_carryin_compare_const"
   [(set (reg:CC CC_REGNUM)
 (compare:CC (match_operand:SI 1 "reg_or_int_operand" "r")
-    (match_operand:SI 2 "arm_not_operand" "K")))
+    (match_operand:SI 2 "const_int_I_operand" "I")))
    (set (match_operand:SI 0 "s_register_operand" "=r")
 (minus:SI (plus:SI (match_dup 1)
-   (match_dup 2))
+   (match_operand:SI 3 
"arm_neg_immediate_operand" "L"))

+  (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0]
+  "TARGET_32BIT && UINTVAL (operands[2]) == -UINTVAL (operands[3])"
+  "sbcs\\t%0, %1, #%n3"
+  [(set_attr "conds" "set")
+   (set_attr "type" "adcs_imm")]
+)
+
+(define_insn "*subsi3_carryin_compare_const0"
+  [(set (reg:CC CC_REGNUM)
+    (compare:CC (match_operand:SI 1 "reg_or_int_operand" "r")
+   (const_int 0)))
+   (set (match_operand:SI 0 "s_register_operand" "=r")
+    (minus:SI (match_dup 1)
   (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0]
   "TARGET_32BIT"
-  "sbcs\\t%0, %1, #%B2"
+  "sbcs\\t%0, %1, #0"
   [(set_attr "conds" "set")
    (set_attr "type" "adcs_imm")]
 )
@@ -1301,14 +1324,13 @@ (define_insn_and_split "*subdi_di_zesidi
   [(parallel [(set (reg:CC CC_REGNUM)
    (compare:CC (match_dup 1) (match_dup 2)))
   (set (match_dup 0) (minus:SI (match_dup 1) (match_dup 
2)))])

-   (set (match_dup 3) (minus:SI (plus:SI (match_dup 4) (match_dup 5))
+   (set (match_dup 3) (minus:SI (match_dup 4)
 (ltu:SI (reg:CC_C CC_REGNUM) 
(const_int 0]

   {
 operands[3] = gen_highpart (SImode, operands[0]);
 operands[0] = gen_lowpart (SImode, operands[0]);
 operands[4] = gen_highp

[PATCH][wwwdocs][AArch64/arm] Mention Neoverse N1 and Neoverse E1 support for GCC 9

[Kyrill Tkachov]

Hi all,

Here's a wwwdocs patch mentioning the recently-added support for the Arm 
Neoverse N1 and Neoverse E1 processors.

Checked the output on Firefox.

Ok to commit (from an aarch64 perspective)?

Thanks,
Kyrill

Index: htdocs/gcc-9/changes.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-9/changes.html,v
retrieving revision 1.43
diff -U 3 -r1.43 changes.html
--- htdocs/gcc-9/changes.html	21 Feb 2019 10:32:55 -	1.43
+++ htdocs/gcc-9/changes.html	22 Feb 2019 09:04:53 -
@@ -267,6 +267,7 @@
 
 	Arm Cortex-A76 (cortex-a76).
 	Arm Cortex-A55/Cortex-A76 DynamIQ big.LITTLE (cortex-a76.cortex-a55).
+	Arm Neoverse N1 (neoverse-n1).
 
 The GCC identifiers can be used
 as arguments to the -mcpu or -mtune options,
@@ -288,6 +289,10 @@
 AArch64 specific
 
   
+Support has been added for the Arm Neoverse E1 processor (-mcpu=neoverse-e1)
+  
+
+  
 The AArch64 port now has support for stack clash protection using the
 https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html#index-fstack-protector";>-fstack-clash-protection option.  The probing interval/guard
 size can be set by using --param stack-clash-protection-guard-size=12|16.

Re: [PATCH] Improve arm and aarch64 casesi (PR target/70341)

[Kyrill Tkachov]

Hi Jakub,

On 2/25/19 10:19 AM, Jakub Jelinek wrote:

On Mon, Feb 25, 2019 at 10:05:46AM +, Kyrill Tkachov wrote:

Hi Jakub,

On 2/23/19 12:20 AM, Jakub Jelinek wrote:

Hi!

The testcase in the PR doesn't hoist any memory loads from the large
switch
before the switch on aarch64 and arm (unlike e.g. x86), because the
arm/aarch64 casesi patterns don't properly annotate the memory load from
the
jump table.  It is created by gen_* and in RTL directly one can't specify
the needed flags (MEM_READONLY_P and MEM_NOTRAP_P).

Fixed thusly, bootstrapped/regtested on armv7hl-linux-gnueabi and
aarch64-linux, ok for trunk?


Since you're changing Arm and Thumb-2-specific paths can you please make
sure to bootstrap configurations --with-mode=arm and --with-mode=thumb.

The only bootstraps I'm doing are distro builds with
 --with-tune=generic-armv7-a --with-arch=armv7-a \
 --with-float=hard --with-fpu=vfpv3-d16 --with-abi=aapcs-linux
I don't have setup nor experience with configuring anything else, don't
really know what is and what isn't ABI compatible etc.
Isn't --with-mode=arm the default with the above set of options?  Can
--with-mode=thumb be used ABI compatibly with that, or is that incompatible?



They are ABI-compatible. Running the testsuite with -mthumb in 
RUNTESTFLAGS would also be enough in this case if you don't have the 
cycles for a bootstrap.


Thanks,

Kyrill




Jakub

Re: [PATCH] Improve arm and aarch64 casesi (PR target/70341)

[Jakub Jelinek]

On Mon, Feb 25, 2019 at 10:05:46AM +, Kyrill Tkachov wrote:
> Hi Jakub,
> 
> On 2/23/19 12:20 AM, Jakub Jelinek wrote:
> > Hi!
> > 
> > The testcase in the PR doesn't hoist any memory loads from the large
> > switch
> > before the switch on aarch64 and arm (unlike e.g. x86), because the
> > arm/aarch64 casesi patterns don't properly annotate the memory load from
> > the
> > jump table.  It is created by gen_* and in RTL directly one can't specify
> > the needed flags (MEM_READONLY_P and MEM_NOTRAP_P).
> > 
> > Fixed thusly, bootstrapped/regtested on armv7hl-linux-gnueabi and
> > aarch64-linux, ok for trunk?
> > 
> Since you're changing Arm and Thumb-2-specific paths can you please make
> sure to bootstrap configurations --with-mode=arm and --with-mode=thumb.

The only bootstraps I'm doing are distro builds with
--with-tune=generic-armv7-a --with-arch=armv7-a \
--with-float=hard --with-fpu=vfpv3-d16 --with-abi=aapcs-linux
I don't have setup nor experience with configuring anything else, don't
really know what is and what isn't ABI compatible etc.
Isn't --with-mode=arm the default with the above set of options?  Can
--with-mode=thumb be used ABI compatibly with that, or is that incompatible?

Jakub

Re: [PATCH] Improve arm and aarch64 casesi (PR target/70341)

[Kyrill Tkachov]

Hi Jakub,

On 2/23/19 12:20 AM, Jakub Jelinek wrote:

Hi!

The testcase in the PR doesn't hoist any memory loads from the large 
switch

before the switch on aarch64 and arm (unlike e.g. x86), because the
arm/aarch64 casesi patterns don't properly annotate the memory load 
from the

jump table.  It is created by gen_* and in RTL directly one can't specify
the needed flags (MEM_READONLY_P and MEM_NOTRAP_P).

Fixed thusly, bootstrapped/regtested on armv7hl-linux-gnueabi and
aarch64-linux, ok for trunk?

Since you're changing Arm and Thumb-2-specific paths can you please make 
sure to bootstrap configurations --with-mode=arm and --with-mode=thumb.


Otherwise the arm parts (and aarch64 FWIW) look ok to me. Most of the 
patch content is splitting the define_insn into a define_expand so that 
we can annotate the MEM with the right attributes.


Thanks,

Kyrill



2019-02-23  Jakub Jelinek  

    PR target/70341
    * config/arm/arm.md (arm_casesi_internal): New define_expand.  
Rename

    old define_insn to ...
    (*arm_casesi_internal): ... this.  Add mode to LABEL_REFs.
    * config/arm/thumb2.md (thumb2_casesi_internal): New 
define_expand.

    Rename old define_insn to ...
    (*thumb2_casesi_internal): ... this.  Add mode to LABEL_REFs.
    (thumb2_casesi_internal_pic): New define_expand. Rename old
    define_insn to ...
    (*thumb2_casesi_internal_pic): ... this.  Add mode to LABEL_REFs.
    * config/aarch64/aarch64.md (casesi): Create the casesi_dispatch
    MEM manually here, set MEM_READONLY_P and MEM_NOTRAP_P on it.

--- gcc/config/arm/arm.md.jj    2019-02-18 20:48:32.643732307 +0100
+++ gcc/config/arm/arm.md   2019-02-21 14:40:50.603452028 +0100
@@ -8914,16 +8914,35 @@ (define_expand "casesi"

 ;; The USE in this pattern is needed to tell flow analysis that this is
 ;; a CASESI insn.  It has no other purpose.
-(define_insn "arm_casesi_internal"
+(define_expand "arm_casesi_internal"
+  [(parallel [(set (pc)
+  (if_then_else
+   (leu (match_operand:SI 0 "s_register_operand")
+    (match_operand:SI 1 "arm_rhs_operand"))
+   (match_dup 4)
+   (label_ref:SI (match_operand 3 ""
+ (clobber (reg:CC CC_REGNUM))
+ (use (label_ref:SI (match_operand 2 "")))])]
+  "TARGET_ARM"
+{
+  operands[4] = gen_rtx_MULT (SImode, operands[0], GEN_INT (4));
+  operands[4] = gen_rtx_PLUS (SImode, operands[4],
+ gen_rtx_LABEL_REF (SImode, operands[2]));
+  operands[4] = gen_rtx_MEM (SImode, operands[4]);
+  MEM_READONLY_P (operands[4]) = 1;
+  MEM_NOTRAP_P (operands[4]) = 1;
+})
+
+(define_insn "*arm_casesi_internal"
   [(parallel [(set (pc)
    (if_then_else
 (leu (match_operand:SI 0 "s_register_operand" "r")
  (match_operand:SI 1 "arm_rhs_operand" "rI"))
 (mem:SI (plus:SI (mult:SI (match_dup 0) (const_int 4))
-    (label_ref (match_operand 2 "" ""
-   (label_ref (match_operand 3 "" ""
+    (label_ref:SI (match_operand 2 "" ""
+   (label_ref:SI (match_operand 3 "" ""
   (clobber (reg:CC CC_REGNUM))
- (use (label_ref (match_dup 2)))])]
+ (use (label_ref:SI (match_dup 2)))])]
   "TARGET_ARM"
   "*
 if (flag_pic)
--- gcc/config/arm/thumb2.md.jj 2019-01-01 12:37:28.280792453 +0100
+++ gcc/config/arm/thumb2.md    2019-02-21 15:00:26.811137210 +0100
@@ -1079,17 +1079,37 @@ (define_insn "thumb2_zero_extendqisi2_v6
    (set_attr "neg_pool_range" "*,250")]
 )

-(define_insn "thumb2_casesi_internal"
+(define_expand "thumb2_casesi_internal"
+  [(parallel [(set (pc)
+  (if_then_else
+   (leu (match_operand:SI 0 "s_register_operand")
+    (match_operand:SI 1 "arm_rhs_operand"))
+   (match_dup 4)
+   (label_ref:SI (match_operand 3 ""
+ (clobber (reg:CC CC_REGNUM))
+ (clobber (match_scratch:SI 5))
+ (use (label_ref:SI (match_operand 2 "")))])]
+  "TARGET_THUMB2 && !flag_pic"
+{
+  operands[4] = gen_rtx_MULT (SImode, operands[0], GEN_INT (4));
+  operands[4] = gen_rtx_PLUS (SImode, operands[4],
+ gen_rtx_LABEL_REF (SImode, operands[2]));
+  operands[4] = gen_rtx_MEM (SImode, operands[4]);
+  MEM_READONLY_P (operands[4]) = 1;
+  MEM_NOTRAP_P (operands[4]) = 1;
+})
+
+(define_insn "*thumb2_casesi_internal"
   [(parallel [(set (pc)
    (if_then_else
 (leu (match_operand:SI 0 "s_register_operand" "r")
  (match_operand:SI 1 "arm_rhs_operand" "rI"))
 (mem:SI (plus:SI (mult:SI (match_dup 0) (const_int 4))
-    (label_ref (match_operand 2 "" ""
-   (label_ref (match_operand 3 "" ""
+    (label_ref:SI (match_opera

Re: [PATCH] Fix var-tracking ICE on ARM due to backend bug (PR target/89438)

[Kyrill Tkachov]

On 2/25/19 9:35 AM, Jakub Jelinek wrote:

On Mon, Feb 25, 2019 at 09:25:19AM +, Kyrill Tkachov wrote:

--- gcc/config/arm/vfp.md.jj    2019-02-18 20:48:32.642732323 +0100
+++ gcc/config/arm/vfp.md   2019-02-22 00:37:36.730795663 +0100
@@ -871,14 +871,15 @@ (define_insn_and_split "*negdf2_vfp"
    if (REGNO (operands[0]) == REGNO (operands[1]))
  {
    operands[0] = gen_highpart (SImode, operands[0]);
-  operands[1] = gen_rtx_XOR (SImode, operands[0], GEN_INT
(0x8000));
+  operands[1] = gen_rtx_XOR (SImode, operands[0],
+    GEN_INT (HOST_WIDE_INT_C (-0x8000)));


const_int generation in RTL always subtly confuses me :(

So why is 0x8000 now -0x8000? Shouldn't we be using
trunc_int_for_mode?

We surely can, it will be tiny bit slower though.
HOST_WIDE_INT_C (-0x8000) is basically inlining of what
trunc_int_for_mode (0x8000, SImode) will do.

Or we could use gen_int_mode (0x8000, SImode); instead of
the whole GEN_INT (HOST_WIDE_INT_C (-0x8000)).


Let's use gen_int_mode. It looks the most clear and self-documenting to me.

Ok with that change.

Thanks,

Kyrill




Whatever you prefer.

Jakub

Re: [PATCH] Fix var-tracking ICE on ARM due to backend bug (PR target/89438)

[Jakub Jelinek]

On Mon, Feb 25, 2019 at 09:25:19AM +, Kyrill Tkachov wrote:
> > --- gcc/config/arm/vfp.md.jj    2019-02-18 20:48:32.642732323 +0100
> > +++ gcc/config/arm/vfp.md   2019-02-22 00:37:36.730795663 +0100
> > @@ -871,14 +871,15 @@ (define_insn_and_split "*negdf2_vfp"
> >    if (REGNO (operands[0]) == REGNO (operands[1]))
> >  {
> >    operands[0] = gen_highpart (SImode, operands[0]);
> > -  operands[1] = gen_rtx_XOR (SImode, operands[0], GEN_INT
> > (0x8000));
> > +  operands[1] = gen_rtx_XOR (SImode, operands[0],
> > +    GEN_INT (HOST_WIDE_INT_C (-0x8000)));
> 
> 
> const_int generation in RTL always subtly confuses me :(
> 
> So why is 0x8000 now -0x8000? Shouldn't we be using
> trunc_int_for_mode?

We surely can, it will be tiny bit slower though.
HOST_WIDE_INT_C (-0x8000) is basically inlining of what
trunc_int_for_mode (0x8000, SImode) will do.

Or we could use gen_int_mode (0x8000, SImode); instead of
the whole GEN_INT (HOST_WIDE_INT_C (-0x8000)).

Whatever you prefer.

Jakub

Re: [PATCH] Fix var-tracking ICE on ARM due to backend bug (PR target/89438)

[Kyrill Tkachov]

Hi Jakub,

On 2/22/19 10:19 PM, Jakub Jelinek wrote:

Hi!

The following testcase ICEs on arm, because the backend creates
non-canonical SImode constants (0x8000).  CONST_INTs always need to be
sign-extended from their corresponding mode to HOST_WIDE_INT.

Fixed thusly, bootstrapped/regtested on armv7hl-linux-gnueabi, ok for 
trunk?


2019-02-22  Jakub Jelinek  

    PR target/89438
    * config/arm.vfp.md (*negdf2_vfp): Use HOST_WIDE_INT_C 
(-0x8000)

    instead of 0x8000.
    * config/arm/neon.md (neon_copysignf): Likewise.

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

--- gcc/config/arm/vfp.md.jj    2019-02-18 20:48:32.642732323 +0100
+++ gcc/config/arm/vfp.md   2019-02-22 00:37:36.730795663 +0100
@@ -871,14 +871,15 @@ (define_insn_and_split "*negdf2_vfp"
   if (REGNO (operands[0]) == REGNO (operands[1]))
 {
   operands[0] = gen_highpart (SImode, operands[0]);
-  operands[1] = gen_rtx_XOR (SImode, operands[0], GEN_INT 
(0x8000));

+  operands[1] = gen_rtx_XOR (SImode, operands[0],
+    GEN_INT (HOST_WIDE_INT_C (-0x8000)));



const_int generation in RTL always subtly confuses me :(

So why is 0x8000 now -0x8000? Shouldn't we be using 
trunc_int_for_mode?


Thanks,

Kyrill



 }
   else
 {
   rtx in_hi, in_lo, out_hi, out_lo;

   in_hi = gen_rtx_XOR (SImode, gen_highpart (SImode, operands[1]),
-  GEN_INT (0x8000));
+  GEN_INT (HOST_WIDE_INT_C (-0x8000)));
   in_lo = gen_lowpart (SImode, operands[1]);
   out_hi = gen_highpart (SImode, operands[0]);
   out_lo = gen_lowpart (SImode, operands[0]);
--- gcc/config/arm/neon.md.jj   2019-02-07 17:33:38.840669157 +0100
+++ gcc/config/arm/neon.md  2019-02-22 00:40:15.243249783 +0100
@@ -3610,7 +3610,7 @@ (define_expand "neon_copysignf"
   "{
  rtx v_bitmask_cast;
  rtx v_bitmask = gen_reg_rtx (mode);
- rtx c = GEN_INT (0x8000);
+ rtx c = GEN_INT (HOST_WIDE_INT_C (-0x8000));

  emit_move_insn (v_bitmask,
  gen_const_vec_duplicate 
(mode, c));
--- gcc/testsuite/gcc.dg/pr89438.c.jj   2019-02-22 00:45:26.662292609 
+0100
+++ gcc/testsuite/gcc.dg/pr89438.c  2019-02-22 00:45:02.130682728 
+0100

@@ -0,0 +1,22 @@
+/* PR target/89438 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -g -w" } */
+
+struct S { double b, c; struct T { double d, e; } f[16]; } g;
+int h, i, j;
+double k;
+
+double
+foo (void)
+{
+  int m;
+  if (j)
+    return k;
+  long a, p = a - 80;
+  double b, n;
+  n = b * h + g.f[p].e;
+  m = n;
+  double o = 1 ? m : 1.0;
+  k = i ? -o : o;
+  return k;
+}

    Jakub

[patch] Fix wrong code for boolean negation in condition at -O

[Eric Botcazou]

Hi,

this is a regression present on the mainline and 8 branch, introduced by the 
new code in edge_info::derive_equivalences dealing with BIT_AND_EXPR for SSA 
names with boolean range:

  /* If either operand has a boolean range, then we
 know its value must be one, otherwise we just know it
 is nonzero.  The former is clearly useful, I haven't
 seen cases where the latter is helpful yet.  */
  if (TREE_CODE (rhs1) == SSA_NAME)
{
  if (ssa_name_has_boolean_range (rhs1))
{
  value = build_one_cst (TREE_TYPE (rhs1));
  derive_equivalences (rhs1, value, recursion_limit - 1);
}
}
  if (TREE_CODE (rhs2) == SSA_NAME)
{
  if (ssa_name_has_boolean_range (rhs2))
{
  value = build_one_cst (TREE_TYPE (rhs2));
  derive_equivalences (rhs2, value, recursion_limit - 1);
}
}

and visible on the attached Ada testcase at -O1 or above.

The sequence of events is as follows: for boolean types with precision > 1 
(the normal boolean types in Ada), the gimplifier turns a TRUTH_NOT_EXPR into 
a BIT_XOR_EXPR with 1 in order to preserve the 0-or-1-value invariant:

/* The parsers are careful to generate TRUTH_NOT_EXPR
   only with operands that are always zero or one.
   We do not fold here but handle the only interesting case
   manually, as fold may re-introduce the TRUTH_NOT_EXPR.  */
*expr_p = gimple_boolify (*expr_p);
if (TYPE_PRECISION (TREE_TYPE (*expr_p)) == 1)
  *expr_p = build1_loc (input_location, BIT_NOT_EXPR,
TREE_TYPE (*expr_p),
TREE_OPERAND (*expr_p, 0));
else
  *expr_p = build2_loc (input_location, BIT_XOR_EXPR,
TREE_TYPE (*expr_p),
TREE_OPERAND (*expr_p, 0),
build_int_cst (TREE_TYPE (*expr_p), 1));

Now this TRUTH_NOT_EXPR is part of a conjunction which has been turned into a 
BIT_AND_EXPR by the folder, so this gives BIT_AND_EXPR >.

After some optimization passes, the second operand of the BIT_AND_EXPR is also 
folded into 1 and, consequently, the following match.pd pattern kicks in:

/* Fold (X & Y) ^ Y and (X ^ Y) & Y as ~X & Y.  */
(for opo (bit_and bit_xor)
 opi (bit_xor bit_and)
 (simplify
  (opo:c (opi:c @0 @1) @1) 
  (bit_and (bit_not @0) @1)))

and yields BIT_AND_EXPR .  This is still correct, in the 
sense that the 0-or-1-value invariant is preserved.

Then the new code in edge_info::derive_equivalences above deduces from this 
that the BIT_NOT_EXPR has value 1 on one of the edges.  But the same function 
also handles the BIT_NOT_EXPR itself and further deduces that its operand has 
value ~1 or 254 (the precision of boolean types is 8) on this edge, which 
breaks the 0-or-1-value invariant and leads to wrong code downstream.

Given the new code for BIT_AND_EXPR in edge_info::derive_equivalences for 
boolean types, I think that the same special treatment must be added for 
boolean types in the BIT_NOT_EXPR case to preserve the 0-or-1-value invariant.

Bootstrapped/regtested on x86_64-suse-linux, OK for mainline and 8 branch?


2019-02-25  Eric Botcazou  

* tree-ssa-dom.c (edge_info::derive_equivalences) : Fix
and move around comment.
: Likewise.
: Add specific handling for boolean types.


2019-02-25  Eric Botcazou  

* gnat.dg/opt77.adb: New test.
* gnat.dg/opt77_pkg.ad[sb]: Likewise.

-- 
Eric BotcazouIndex: tree-ssa-dom.c
===
--- tree-ssa-dom.c	(revision 268994)
+++ tree-ssa-dom.c	(working copy)
@@ -170,11 +170,10 @@ edge_info::derive_equivalences (tree nam
   gimple *def_stmt = SSA_NAME_DEF_STMT (name);
   if (is_gimple_assign (def_stmt))
 {
-  /* We know the result of DEF_STMT was zero.  See if that allows
-	 us to deduce anything about the SSA_NAMEs used on the RHS.  */
   enum tree_code code = gimple_assign_rhs_code (def_stmt);
   switch (code)
 	{
+	/* If the result of an OR is zero, then its operands are, too.  */
 	case BIT_IOR_EXPR:
 	  if (integer_zerop (value))
 	{
@@ -188,8 +187,7 @@ edge_info::derive_equivalences (tree nam
 	}
 	  break;
 
-  /* We know the result of DEF_STMT was one.  See if that allows
-	 us to deduce anything about the SSA_NAMEs used on the RHS.  */
+	/* If the result of an AND is nonzero, then its operands are, too.  */
 	case BIT_AND_EXPR:
 	  if (!integer_zerop (value))
 	{
@@ -296,7 +294,6 @@ edge_info::derive_equivalences (tree nam
 	break;
 	  }
 
-
 	case EQ_EXPR:
 	case NE_EXPR:
 	  {
@@ -336,7