Re: [PATCH, rs6000] Fix PR72827 (ada bootstrap failure)

[Eric Botcazou]

> DSE should really detect this is happening and not do the wrong thing.
> Maybe add an assert somewhere?  Much easier to debug, that way.

That sounds fragile, functions are allowed to fiddle with the frame pointer in 
the prologue or epilogue (but of course not in the body).  I think that DSE is 
not the only RTL pass which makes this assumption of invariant frame pointer 
in the body, it seems rather fundamental in the RTL middle-end.

-- 
Eric Botcazou

[ARM] Fix RTL checking failure in Thumb mode

[Eric Botcazou]

Hi,

compiling any non-trivial program in Thumb mode yields with RTL checking:

eric@arcturus:~/build/gcc/arm-eabi> gcc/xgcc -Bgcc -S t.c -mthumb
t.c:4:5: internal compiler error: RTL check: expected code 'const_int', have 
'reg' in thumb1_size_rtx_costs, at config/arm/arm.c:9130
 int foo (int a, int b, int c, int d, double i)
 ^~~
0xb0da47 rtl_check_failed_code1(rtx_def const*, rtx_code, char const*, int, 
char const*)
/home/eric/svn/gcc/gcc/rtl.c:811
0xf13472 thumb1_size_rtx_costs
/home/eric/svn/gcc/gcc/config/arm/arm.c:9130
0xf1cd10 arm_size_rtx_costs
/home/eric/svn/gcc/gcc/config/arm/arm.c:9239
0xf1cd10 arm_rtx_costs
/home/eric/svn/gcc/gcc/config/arm/arm.c:11290

so even libgcc cannot be built.

(gdb) frame 2
#2  0x00f13473 in thumb1_size_rtx_costs (x=x@entry=0x76a622d0, 
code=code@entry=SET, outer=outer@entry=INSN)
at /home/eric/svn/gcc/gcc/config/arm/arm.c:9130
9130  || (UINTVAL (SET_SRC (x)) >= 256
(gdb) p debug_rtx(x)
(set (reg:SI 110)
(reg:SI 111))

Proposed fix attached, built on arm-eabi, OK for the mainline?


2016-08-31  Eric Botcazou  

* config/arm/arm.c (thumb1_size_rtx_costs) : Add missing guard.

-- 
Eric BotcazouIndex: config/arm/arm.c
===
--- config/arm/arm.c	(revision 239842)
+++ config/arm/arm.c	(working copy)
@@ -9127,7 +9127,8 @@ thumb1_size_rtx_costs (rtx x, enum rtx_code code,
   if (satisfies_constraint_J (SET_SRC (x))
 	  || satisfies_constraint_K (SET_SRC (x))
 	 /* Too big an immediate for a 2-byte mov, using MOVT.  */
-	  || (UINTVAL (SET_SRC (x)) >= 256
+	  || (CONST_INT_P (SET_SRC (x))
+	  && UINTVAL (SET_SRC (x)) >= 256
 	  && TARGET_HAVE_MOVT
 	  && satisfies_constraint_j (SET_SRC (x)))
 	 /* thumb1_movdi_insn.  */

Re: [patch, libgfortran] PR77393 [7 Regression] Revision r237735 changed the behavior of F0.0

[Janne Blomqvist]

On Wed, Aug 31, 2016 at 6:04 AM, Jerry DeLisle  wrote:
> Hi all,
>
> The attached patch fixes the problem by adding a new helper function to
> determine the buffer size needed for F0 editing depending on the kind. In this
> new function there are some constants presented which document the limits 
> needed
> for each kind type.
>
> As can be seen, the required buffers are fixed on stack at 256 bytes which 
> will
> handle almost all cases unless a user is doing something with unusually wide
> formats.  The buffer is malloc'ed if a larger size is needed.
>
> I have not changed the buffering mechanism, only the method of determining the
> needed size.
>
> Regression tested on x86-linux. New test case provided.
>
> OK for trunk?

Ok, thanks for the patch!


-- 
Janne Blomqvist

Re: PR35503 - warn for restrict pointer

[Tom de Vries]

On 30/08/16 16:54, Tom de Vries wrote:

On 26/08/16 13:39, Prathamesh Kulkarni wrote:

Hi,
The following patch adds option -Wrestrict that warns when an argument
is passed to a restrict qualified parameter and it aliases with
another argument.

eg:
int foo (const char *__restrict buf, const char *__restrict fmt, ...);

void f(void)
{
  char buf[100] = "hello";
  foo (buf, "%s-%s", buf, "world");
}


Does -Wrestrict generate a warning for this example?

...
void h(int n, int * restrict p, int * restrict q, int * restrict r)
{
  int i;
  for (i = 0; i < n; i++)
p[i] = q[i] + r[i];
}

h (100, a, b, b)
...

[ Note that this is valid C, and does not violate the restrict
definition. ]

If -Wrestrict indeed generates a warning, then we should be explicit in
the documentation that while the warning triggers on this type of
example, the code is correct.


So, what I had in mind is something like this:
...
Warn when an argument passed to a restrict-qualified parameter pointing 
to a non-const type aliases with another argument.


The warning will trigger for the following illegal use of restrict:

  void foo (int *restrict a, int *b) { *a = 1; *b = 2; }
  void foo2 (void) { int o; foo (&o, &o); }

The use of restrict is illegal because in the lifetime of the scope of 
the restrict pointer 'a' both:

- the object 'o' that the restrict pointer 'a' points to is modified,
  and
- the object 'o' is accessed through pointer 'b', which is not based on
  restrict pointer 'a'.

However, the warning will also trigger for the following legal use of 
restrict:


  int foo (int *restrict a, int *b) { return *a + *b; }
  int foo2 (void) { int o = 1; return foo (&o, &o); }

The use of restrict is legal, because the object 'o' that the restrict 
pointer 'a' points to is not modified  in the lifetime of the scope of 
the restrict pointer 'a'. The warning can be silenced by changing the 
type of 'a' to 'const int *restrict'.

...

Thanks,
- Tom

Re: [ARM] Fix RTL checking failure in Thumb mode

[Kyrill Tkachov]

On 31/08/16 08:31, Eric Botcazou wrote:

Hi,

compiling any non-trivial program in Thumb mode yields with RTL checking:

eric@arcturus:~/build/gcc/arm-eabi> gcc/xgcc -Bgcc -S t.c -mthumb
t.c:4:5: internal compiler error: RTL check: expected code 'const_int', have
'reg' in thumb1_size_rtx_costs, at config/arm/arm.c:9130
  int foo (int a, int b, int c, int d, double i)
  ^~~
0xb0da47 rtl_check_failed_code1(rtx_def const*, rtx_code, char const*, int,
char const*)
 /home/eric/svn/gcc/gcc/rtl.c:811
0xf13472 thumb1_size_rtx_costs
 /home/eric/svn/gcc/gcc/config/arm/arm.c:9130
0xf1cd10 arm_size_rtx_costs
 /home/eric/svn/gcc/gcc/config/arm/arm.c:9239
0xf1cd10 arm_rtx_costs
 /home/eric/svn/gcc/gcc/config/arm/arm.c:11290

so even libgcc cannot be built.

(gdb) frame 2
#2  0x00f13473 in thumb1_size_rtx_costs (x=x@entry=0x76a622d0,
 code=code@entry=SET, outer=outer@entry=INSN)
 at /home/eric/svn/gcc/gcc/config/arm/arm.c:9130
9130  || (UINTVAL (SET_SRC (x)) >= 256
(gdb) p debug_rtx(x)
(set (reg:SI 110)
 (reg:SI 111))

Proposed fix attached, built on arm-eabi, OK for the mainline?


Ok.
Thanks,
Kyrill



2016-08-31  Eric Botcazou  

* config/arm/arm.c (thumb1_size_rtx_costs) : Add missing guard.

libgo/runtime: Fix signal stack size for ia64

[Andreas Schwab]

On ia64, MINSIGSTKSZ is 128K.

Andreas.

* libgo/runtime/runtime.c (runtime_mpreinit): Increase stack size to
128K.
---
 libgo/runtime/runtime.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libgo/runtime/runtime.c b/libgo/runtime/runtime.c
index c7d33bc..e8eb957 100644
--- a/libgo/runtime/runtime.c
+++ b/libgo/runtime/runtime.c
@@ -272,7 +272,7 @@ runtime_tickspersecond(void)
 void
 runtime_mpreinit(M *mp)
 {
-   mp->gsignal = runtime_malg(32*1024, (byte**)&mp->gsignalstack, 
&mp->gsignalstacksize);  // OS X wants >=8K, Linux >=2K
+   mp->gsignal = runtime_malg(128*1024, (byte**)&mp->gsignalstack, 
&mp->gsignalstacksize); // OS X wants >=8K, Linux >=2K, ia64 >=128K
mp->gsignal->m = mp;
 }
 
-- 
2.9.3

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] selftest.c: avoid explicit "selftest::" qualifiers

[Bernd Schmidt]

On 08/31/2016 02:31 AM, David Malcolm wrote:


The following follow-up patch removes them, moving the
"namespace selftest {" to the top of the file so it covers
everything.


LGTM.


Bernd

Re: [PATCH, rs6000] Fix PR72827 (ada bootstrap failure)

[Segher Boessenkool]

On Wed, Aug 31, 2016 at 09:08:37AM +0200, Eric Botcazou wrote:
> > DSE should really detect this is happening and not do the wrong thing.
> > Maybe add an assert somewhere?  Much easier to debug, that way.
> 
> That sounds fragile, functions are allowed to fiddle with the frame pointer 
> in 
> the prologue or epilogue (but of course not in the body).  I think that DSE 
> is 
> not the only RTL pass which makes this assumption of invariant frame pointer 
> in the body, it seems rather fundamental in the RTL middle-end.

Yes exactly, but we do not detect violations of that anywhere it seems.


Segher

Re: Implement -Wimplicit-fallthrough (version 7)

[Marek Polacek]

On Tue, Aug 30, 2016 at 01:08:22PM -0400, Jason Merrill wrote:
> On Tue, Aug 30, 2016 at 10:25 AM, Marek Polacek  wrote:
> >> > @@ -10585,15 +10610,26 @@ cp_parser_statement (cp_parser* parser, tree 
> >> > in_statement_expr,
> >> > }
> >> >/* Look for an expression-statement instead.  */
> >> >statement = cp_parser_expression_statement (parser, 
> >> > in_statement_expr);
> >> > +
> >> > +  /* Handle [[fallthrough]];.  */
> >> > +  if (std_attrs != NULL_TREE
> >> > + && is_attribute_p ("fallthrough", get_attribute_name 
> >> > (std_attrs))
> >> > + && statement == NULL_TREE)
> >> > +   {
> >> > + tree fn = build_call_expr_internal_loc (statement_location,
> >> > + IFN_FALLTHROUGH,
> >> > + void_type_node, 0);
> >>
> >> Let's use the 'statement' variable rather than a new variable fn so
> >> that we set the call's location below.  Let's also warn here about
> >> [[fallthrough]] on non-null expression statements.
> >
> > Done.  But I couldn't figure out a testcase where the warning would 
> > trigger, so
> > not sure how important that is.
> 
> What happens for
> 
> [[fallthrough]] 42;
> 
> ?  What ought to happen is a warning that the attribute only applies
> to null statements.

This:
q.C: In function ‘void f(int)’:
q.C:8:8: error: expected identifier before ‘[’ token
   [[fallthrough]] 42;
^
q.C: In lambda function:
q.C:8:23: error: expected ‘{’ before numeric constant
   [[fallthrough]] 42;
   ^~

And the same with e.g. [[unused]].  G++ 6 behaves exactly the same,
so it isn't something my patch changed.  It seems we never even reach
the code in question, at least std_attrs is null.

It seems g++ is trying to parse a lambda expression.

Marek

Re: [PATCH] C: fixits for modernizing structure member designators

[Marek Polacek]

On Tue, Aug 30, 2016 at 11:22:21AM -0400, David Malcolm wrote:
> On Tue, 2016-08-30 at 16:50 +0200, Marek Polacek wrote:
> > On Tue, Aug 30, 2016 at 04:40:57PM +0200, Bernd Schmidt wrote:
> > > On 08/30/2016 04:38 PM, David Malcolm wrote:
> > > 
> > > > In conjunction with the not-yet-in-trunk -fdiagnostics-generate
> > > > -patch,
> > > > this can generate patches like this:
> > > > 
> > > > --- modernize-named-inits.c
> > > > +++ modernize-named-inits.c
> > > > @@ -16,7 +16,7 @@
> > > >  /* Old-style named initializers.  */
> > > > 
> > > >  struct foo old_style_f = {
> > > > - foo: 1,
> > > > - bar: 2,
> > > > + .foo= 1,
> > > > + .bar= 2,
> > > >  };
> > > 
> > > What happens if there are newlines in between any of the tokens?
> > 
> > It's easy to check for yourself: when the identifier and colon are
> > not
> > on the same line, we print something like
> > 
> > w.c: In function ‘main’:
> > w.c:7:1: warning: obsolete use of designated initializer with ‘:’ [
> > -Wpedantic]
> >  :
> >  ^
> >   =
> > 
> > which I don't think is desirable -- giving up on the fix-it hint in
> > such case
> > could be appropriate.
> 
> I think that's a bug in diagnostic-show-locus.c; currently it only
> prints lines and fixits for the lines references in the ranges within
> the rich_location.  I'll try to fix that.
> 
> > I admit I dislike the lack of a space before = in ".bar= 2", but
> > using
> >   
> >   richloc.add_fixit_replace (colon_loc, " =");
> > 
> > wouldn't work for "foo : 1" I think.
> 
> I actually tried that first, but I didn't like the way it was printed
> e.g.:
> 
> w.c: In function ‘main’:> w.c:7:1: warning: obsolete use of designated
> initializer with ‘:’ [-Wpedantic]>
>   foo: 1,
>  ^
>   .   =
> 
> I'm looking at rewriting how fixits get printed, to print the affected
> range of the affected lines (using the edit-context.c work posted last
> week), so this would appear as:
> 
>   foo: 1,
>  ^
>   .foo =
 
This would be perfect.

> Also, there may be a case for adding some smarts to gcc_rich_location for 
> adding fixits in a formatting-aware way, by looking at the neighboring 
> whitespace (this might help for the issue with adding "break;" etc in the 
> fallthru patch kit).

Thanks.  I hope it won't be too hard to implement :/

Marek

Re: [MPX] Fix for PR77267

[Ilya Enkovich]

2016-08-30 21:53 GMT+03:00 Alexander Ivchenko :
> Would something like that count?
>
> I did not do the warning thing, cause the problem only appears when
> you provide the -Wl,-as-needed option to the linker.
> (In all other cases warning would be redundant). Are we able to check
> that on runtime?
>
>
> diff --git a/gcc/config.in b/gcc/config.in
> index fc3321c..a736de3 100644
> --- a/gcc/config.in
> +++ b/gcc/config.in
> @@ -1538,6 +1538,12 @@
>  #endif
>
>
> +/* Define if your linker supports --push-state/--pop-state */
> +#ifndef USED_FOR_TARGET
> +#undef HAVE_LD_PUSHPOPSTATE_SUPPORT
> +#endif
> +
> +
>  /* Define if your linker links a mix of read-only and read-write sections 
> into
> a read-write section. */
>  #ifndef USED_FOR_TARGET
> diff --git a/gcc/config/i386/linux-common.h b/gcc/config/i386/linux-common.h
> index 4b9910f..6aa195d 100644
> --- a/gcc/config/i386/linux-common.h
> +++ b/gcc/config/i386/linux-common.h
> @@ -79,13 +79,23 @@ along with GCC; see the file COPYING3.  If not see
>  #endif
>  #endif
>
> +#ifdef HAVE_LD_PUSHPOPSTATE_SUPPORT
> +#define MPX_LD_AS_NEEDED_GUARD_PUSH "--push-state --no-as-needed"
> +#define MPX_LD_AS_NEEDED_GUARD_POP "--pop-state"
> +#else
> +#define MPX_LD_AS_NEEDED_GUARD_PUSH ""
> +#define MPX_LD_AS_NEEDED_GUARD_POP ""
> +#endif
> +
>  #ifndef LIBMPX_SPEC
>  #if defined(HAVE_LD_STATIC_DYNAMIC)
>  #define LIBMPX_SPEC "\
>  %{mmpx:%{fcheck-pointer-bounds:\
>  %{static:--whole-archive -lmpx --no-whole-archive" LIBMPX_LIBS "}\
>  %{!static:%{static-libmpx:" LD_STATIC_OPTION " --whole-archive}\
> --lmpx %{static-libmpx:--no-whole-archive " LD_DYNAMIC_OPTION \
> +" MPX_LD_AS_NEEDED_GUARD_PUSH  " -lmpx " MPX_LD_AS_NEEDED_GUARD_POP "\
> +%{static-libmpx:--no-whole-archive "\
> +LD_DYNAMIC_OPTION \

Looks like you add guards for both static-libmpx and dynamic linking cases.
You shouldn't need it for static-libmpx case.

>  LIBMPX_LIBS ""
>  #else
>  #define LIBMPX_SPEC "\
> @@ -99,7 +109,8 @@ along with GCC; see the file COPYING3.  If not see
>  %{mmpx:%{fcheck-pointer-bounds:%{!fno-chkp-use-wrappers:\
>  %{static:-lmpxwrappers}\
>  %{!static:%{static-libmpxwrappers:" LD_STATIC_OPTION " --whole-archive}\
> --lmpxwrappers %{static-libmpxwrappers:--no-whole-archive "\
> +" MPX_LD_AS_NEEDED_GUARD_PUSH " -lmpxwrappers "
> MPX_LD_AS_NEEDED_GUARD_POP "\
> +%{static-libmpxwrappers:--no-whole-archive "\
>  LD_DYNAMIC_OPTION "}"

I believe wrappers should work fine with --as-needed and don't need
this guard. Otherwise looks OK.

Ilya

>  #else
>  #define LIBMPXWRAPPERS_SPEC "\
> diff --git a/gcc/configure b/gcc/configure
> index 871ed0c..094 100755
> --- a/gcc/configure
> +++ b/gcc/configure
> @@ -29609,6 +29609,30 @@ fi
>  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ld_bndplt_support" >&5
>  $as_echo "$ld_bndplt_support" >&6; }
>
> +# Check linker supports '--push-state'/'--pop-state'
> +ld_pushpopstate_support=no
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking linker
> --push-state/--pop-state options" >&5
> +$as_echo_n "checking linker --push-state/--pop-state options... " >&6; }
> +if test x"$ld_is_gold" = xno; then
> +  if test $in_tree_ld = yes ; then
> +if test "$gcc_cv_gld_major_version" -eq 2 -a
> "$gcc_cv_gld_minor_version" -ge 25 -o "$gcc_cv_gld_major_version" -gt
> 2; then
> +  ld_pushpopstate_support=yes
> +fi
> +  elif test x$gcc_cv_ld != x; then
> +# Check if linker supports --push-state/--pop-state options
> +if $gcc_cv_ld --help 2>/dev/null | grep -- '--push-state' > /dev/null; 
> then
> +  ld_pushpopstate_support=yes
> +fi
> +  fi
> +fi
> +if test x"$ld_pushpopstate_support" = xyes; then
> +
> +$as_echo "#define HAVE_LD_PUSHPOPSTATE_SUPPORT 1" >>confdefs.h
> +
> +fi
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ld_pushpopstate_support" 
> >&5
> +$as_echo "$ld_pushpopstate_support" >&6; }
> +
>  # Configure the subdirectories
>  # AC_CONFIG_SUBDIRS($subdirs)
>
> diff --git a/gcc/configure.ac b/gcc/configure.ac
> index 241e82d..93af766 100644
> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -6237,6 +6237,27 @@ if test x"$ld_bndplt_support" = xyes; then
>  fi
>  AC_MSG_RESULT($ld_bndplt_support)
>
> +# Check linker supports '--push-state'/'--pop-state'
> +ld_pushpopstate_support=no
> +AC_MSG_CHECKING(linker --push-state/--pop-state options)
> +if test x"$ld_is_gold" = xno; then
> +  if test $in_tree_ld = yes ; then
> +if test "$gcc_cv_gld_major_version" -eq 2 -a
> "$gcc_cv_gld_minor_version" -ge 25 -o "$gcc_cv_gld_major_version" -gt
> 2; then
> +  ld_pushpopstate_support=yes
> +fi
> +  elif test x$gcc_cv_ld != x; then
> +# Check if linker supports --push-state/--pop-state options
> +if $gcc_cv_ld --help 2>/dev/null | grep -- '--push-state' > /dev/null; 
> then
> +  ld_pushpopstate_support=yes
> +fi
> +  fi
> +fi
> +if test x"$ld_pushpopstate_support" = xyes; then
> +  AC_DEFINE(HAVE_LD_PUSHPOPSTATE_SUPPORT, 1,
> + 

[patch] Fix PR fortran/72743

[Chung-Lin Tang]

Hi Richard, Martin,
this issue is actually sort of like PR 70856, basically the same ICE
after IPA-ICF, due to DECL_PT_UIDs not consistent after reaching for the
ultimate_alias_target().

The reason this wasn't covered by the PR70856 fix is that, in this case,
the DECL_PT_UID was not set in original->decl, evading the condition, and
hence the the merged alias doesn't have DECL_PT_UID set, and causes the
ICE in the second round of IPA-PTA (under -fopenacc).

My fix is to simply remove the DECL_PT_UID_SET_P (original->decl) guard,
and allow the DECL_PT_UID to be set using the DECL_UID in this case.

Does this fix make sense?
Testing does show no regressions, and the PR testcase ICE is fixed.

Thanks,
Chung-Lin

PR fortran/72743
* ipa-icf.c (sem_variable::merge): Remove guard condition for
setting DECL_PT_UID (alias->decl).

testsuite/
* gfortran.dg/goacc/pr72743.f90: New test.


Index: ipa-icf.c
===
--- ipa-icf.c   (revision 239624)
+++ ipa-icf.c   (working copy)
@@ -2258,8 +2258,7 @@ sem_variable::merge (sem_item *alias_item)
 
   varpool_node::create_alias (alias_var->decl, decl);
   alias->resolve_alias (original);
-  if (DECL_PT_UID_SET_P (original->decl))
-   SET_DECL_PT_UID (alias->decl, DECL_PT_UID (original->decl));
+  SET_DECL_PT_UID (alias->decl, DECL_PT_UID (original->decl));
 
   if (dump_file)
fprintf (dump_file, "Unified; Variable alias has been created.\n\n");
Index: testsuite/gfortran.dg/goacc/pr72743.f90
===
--- testsuite/gfortran.dg/goacc/pr72743.f90 (revision 0)
+++ testsuite/gfortran.dg/goacc/pr72743.f90 (revision 0)
@@ -0,0 +1,15 @@
+! { dg-do compile }
+! { dg-additional-options "-O2" }
+
+program p
+   integer, parameter :: n = 8
+   integer :: i, z(n)
+   z = [(i, i=1,n)]
+   print *, z
+end
+subroutine s
+   integer, parameter :: n = 8
+   integer :: i, z(n)
+   z = [(i, i=1,n)]
+   print *, z
+end

Re: [ARM][PR target/77281] Fix an invalid check for vectors of, the same floating-point constants.

[Richard Earnshaw (lists)]

On 30/08/16 14:49, Matthew Wahab wrote:
> Ping.
> 
> On 19/08/16 15:47, Richard Earnshaw (lists) wrote:
>> On 19/08/16 15:06, Matthew Wahab wrote:
>>> On 19/08/16 14:30, Richard Earnshaw (lists) wrote:
 On 19/08/16 12:48, Matthew Wahab wrote:
> 2016-08-19  Matthew Wahab  
>
>   PR target/77281
>   * config/arm/arm.c (neon_valid_immediate): Delete declaration.
>   Use const_vec_duplicate to check for duplicated elements.
>
> Ok for trunk?

 OK.

 Thanks.

 R.
>>>
>>> Is this ok to backport to gcc-6?
>>> Matthew
>>>
>>
>> I believe we're in a release process, so backporting needs RM approval.
>>
>> R.
>>
> 
> Is this ok to backport to gcc-6?
> I've tested for arm-none-linux-gnueabihf with native bootstrap and
> check-gcc.
> 
> Matthew


Yes, this is now OK

R.

Re: [PATCH] Fix detection of setrlimit in libstdc++ testsuite

[Maxim Kuvyrkov]

> On Apr 5, 2016, at 2:20 PM, Jonathan Wakely  wrote:
> 
>> This patch fixes an obscure cross-testing problem that crashed (OOMed) our 
>> boards at Linaro.  Several tests in libstdc++ (e.g., [1]) limit themselves 
>> to some reasonable amount of RAM and then try to allocate 32 gigs.  
>> Unfortunately, the configure test that checks presence of setrlimit is 
>> rather strange: if target is native, then try compile file with call to 
>> setrlimit -- if compilation succeeds, then use setrlimit, otherwise, ignore 
>> setrlimit.  The strange part is that the compilation check is done only for 
>> native targets, as if cross-toolchains can't generate working executables.  
>> [This is rather odd, and I might be missing some underlaying caveat.]
> 
> I went spelunking, and the IS_NATIVE check has been there since
> r70167, which replaced:
> 
> if test  x"$GLIBCXX_IS_CROSS_COMPILING" = xfalse; then
>   # Do checks for memory limit functions.
>   GLIBCXX_CHECK_SETRLIMIT
> 
> That arrived in r68067, but that seems to eb just a refactoring, and I
> got lost tracking it further.
> 
> So there has been a similar check since at least 2003.
> 
>> Therefore, when testing a cross toolchain, the test [1] still tries to 
>> allocate 32GB of RAM with no setrlimit restrictions.  On most targets that 
>> people use for cross-testing this is not an issue because either
>> - the target is 32-bit, so there is no 32GB user-space to speak of, or
>> - the target board has small amount of RAM and no swap, so allocation 
>> immediately fails, or
>> - the target board has plenty of RAM, so allocating 32GB is not an issue.
>> 
>> However, if one is testing on a 64-bit board with 16GB or RAM and 16GB of 
>> swap, then one gets into an obscure near-OOM swapping condition.  This is 
>> exactly the case with cross-testing aarch64-linux-gnu toolchains on APM 
>> Mustang.
>> 
>> The attached patch removes "native" restriction from configure test for 
>> setrlimit.  This enables setrlimit restrictions on the testsuite, and the 
>> test [1] expectedly fails to allocate 32GB due to setrlimit restriction.
>> 
>> I have tested it on x86_64-linux-gnu and i686-linux-gnu native toolchains, 
>> and aarch64-linux-gnu and arm-linux-gnueabi[hf] cross-toolchains with no 
>> regressions [*].
>> 
>> OK to commit?
> 
> This issue has been present for well over a decade so it doesn't seem
> critical to fix in stage4, but as it only affects the testsuite I am
> OK with the change if the RMs have no objections.

Hi Jonathan,

This issue dropped off my patch queue.  The original patch still applies 
without conflicts, and I'm retesting it on fresh mainline -- both cross and 
native.

OK to commit if no regressions?

Thanks,

--
Maxim Kuvyrkov
www.linaro.org

[PATCH, PING] DWARF: space-optimize loc. descr. for integer literals on 32-bit targets

[Pierre-Marie de Rodat]

Hello,

Ping for the patch submitted at 
. Also, here’s 
the update that Trevor suggested.


Thanks!

--
Pierre-Marie de Rodat
>From da6753ad98127778e8923db63ce51b52a853ed17 Mon Sep 17 00:00:00 2001
From: Pierre-Marie de Rodat 
Date: Fri, 24 Jul 2015 15:48:33 +0200
Subject: [PATCH] DWARF: space-optimize loc. descr. for integer literals on
 32-bit targets

This enhances location description generation so that the generated
opcodes for integer literals are as space-efficient when HOST_WIDE_INT
is 64-bits large than when it's 32-bits large. In particular, this
reduces the size of the opcodes generated to produce big unsigned
literals using small literal integers instead.

gcc/

	* dwarf2out.c (int_loc_descriptor): Generate opcodes for another
	equivalent 32-bit constant (modulo 2**32) when that yields
	smaller instructions.
	(size_of_int_loc_descriptor): Update accordingly.
---
 gcc/dwarf2out.c  | 29 -
 gcc/testsuite/gnat.dg/debug8.adb | 29 +
 2 files changed, 53 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gnat.dg/debug8.adb

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 0fdab9a..2591f89 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -11954,20 +11954,35 @@ int_loc_descriptor (HOST_WIDE_INT i)
 	/* DW_OP_const1u X DW_OP_litY DW_OP_shl takes just 4 bytes,
 	   while DW_OP_const4u is 5 bytes.  */
 	return int_shift_loc_descriptor (i, HOST_BITS_PER_WIDE_INT - clz - 8);
+
+  else if (DWARF2_ADDR_SIZE == 4 && i > 0x7fff
+	   && size_of_int_loc_descriptor ((HOST_WIDE_INT) (int32_t) i)
+		  <= 4)
+	{
+	  /* As i >= 2**31, the double cast above will yield a negative number.
+	 Since wrapping is defined in DWARF expressions we can output big
+	 positive integers as small negative ones, regardless of the size
+	 of host wide ints.
+
+	 Here, since the evaluator will handle 32-bit values and since i >=
+	 2**31, we know it's going to be interpreted as a negative literal:
+	 store it this way if we can do better than 5 bytes this way.  */
+	  return int_loc_descriptor ((HOST_WIDE_INT) (int32_t) i);
+	}
   else if (HOST_BITS_PER_WIDE_INT == 32 || i <= 0x)
 	op = DW_OP_const4u;
+
+  /* Past this point, i >= 0x1 and thus DW_OP_constu will take at
+	 least 6 bytes: see if we can do better before falling back to it.  */
   else if (clz + ctz >= HOST_BITS_PER_WIDE_INT - 8
 	   && clz + 8 + 255 >= HOST_BITS_PER_WIDE_INT)
-	/* DW_OP_const1u X DW_OP_const1u Y DW_OP_shl takes just 5 bytes,
-	   while DW_OP_constu of constant >= 0x1 takes at least
-	   6 bytes.  */
+	/* DW_OP_const1u X DW_OP_const1u Y DW_OP_shl takes just 5 bytes.  */
 	return int_shift_loc_descriptor (i, HOST_BITS_PER_WIDE_INT - clz - 8);
   else if (clz + ctz >= HOST_BITS_PER_WIDE_INT - 16
 	   && clz + 16 + (size_of_uleb128 (i) > 5 ? 255 : 31)
 		  >= HOST_BITS_PER_WIDE_INT)
 	/* DW_OP_const2u X DW_OP_litY DW_OP_shl takes just 5 bytes,
-	   DW_OP_const2u X DW_OP_const1u Y DW_OP_shl takes 6 bytes,
-	   while DW_OP_constu takes in this case at least 6 bytes.  */
+	   DW_OP_const2u X DW_OP_const1u Y DW_OP_shl takes 6 bytes.  */
 	return int_shift_loc_descriptor (i, HOST_BITS_PER_WIDE_INT - clz - 16);
   else if (clz + ctz >= HOST_BITS_PER_WIDE_INT - 32
 	   && clz + 32 + 31 >= HOST_BITS_PER_WIDE_INT
@@ -12192,6 +12207,10 @@ size_of_int_loc_descriptor (HOST_WIDE_INT i)
 	   && clz + 8 + 31 >= HOST_BITS_PER_WIDE_INT)
 	return size_of_int_shift_loc_descriptor (i, HOST_BITS_PER_WIDE_INT
 		- clz - 8);
+  else if (DWARF2_ADDR_SIZE == 4 && i > 0x7fff
+	   && size_of_int_loc_descriptor ((HOST_WIDE_INT) (int32_t) i)
+		  <= 4)
+	return size_of_int_loc_descriptor ((HOST_WIDE_INT) (int32_t) i);
   else if (HOST_BITS_PER_WIDE_INT == 32 || i <= 0x)
 	return 5;
   s = size_of_uleb128 ((unsigned HOST_WIDE_INT) i);
diff --git a/gcc/testsuite/gnat.dg/debug8.adb b/gcc/testsuite/gnat.dg/debug8.adb
new file mode 100644
index 000..fabcc22
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/debug8.adb
@@ -0,0 +1,29 @@
+-- { dg-do compile }
+-- { dg-options "-cargs -g -fgnat-encodings=minimal -dA" }
+-- { dg-final { scan-assembler-not "DW_OP_const4u" } }
+-- { dg-final { scan-assembler-not "DW_OP_const8u" } }
+
+--  The DW_AT_byte_size attribute DWARF expression for the
+--  DW_TAG_structure_type DIE that describes Rec_Type contains the -4u literal.
+--  Check that it is not created using an inefficient encoding (DW_OP_const1s
+--  is expected).
+
+procedure Debug8 is
+
+   type Rec_Type (I : Integer) is record
+  B : Boolean;
+  case I is
+ when 0 =>
+null;
+ when 1 .. 10 =>
+C : Character;
+ when others =>
+N : Natural;
+  end case;
+   end record;
+
+   R : access Rec_Type := null;
+
+begin
+   null;
+end Debug8;
-- 
1.8.2.1

Re: [patch, libstdc++] std::shuffle: Generate two swap positions at a time if possible

[Jonathan Wakely]

On 03/05/16 16:42 +0200, Eelis van der Weegen wrote:

Ah, thanks, I forgot to re-attach when I sent to include the libstdc++ list.

On 2016-05-03 14:38, Jonathan Wakely wrote:

ENOPATCH

On 1 May 2016 at 15:21, Eelis  wrote:

Sorry, forgot to include the libstdc++ list.

On 2016-05-01 16:18, Eelis wrote:


Hi,

The attached patch optimizes std::shuffle for the very common case
where the generator range is large enough that a single invocation
can produce two swap positions.

This reduces the runtime of the following testcase by 37% on my machine:

 int main()
 {
 std::mt19937 gen;

 std::vector v;
 v.reserve(1);
 for (int i = 0; i != 1; ++i)
 {
 v.push_back(i);
 std::shuffle(v.begin(), v.end(), gen);
 }

 std::cout << v.front() << '\n';
 }

Thoughts?

Thanks,

Eelis









Index: libstdc++-v3/include/bits/stl_algo.h
===
--- libstdc++-v3/include/bits/stl_algo.h(revision 235680)
+++ libstdc++-v3/include/bits/stl_algo.h(working copy)
@@ -3708,6 +3708,22 @@
#endif

#ifdef _GLIBCXX_USE_C99_STDINT_TR1
+
+  template


We should avoid introducing new names based on "uniform random number
generator" and use _UniformRandomBitGenerator as per
https://wg21.link/p0346r1


+inline _IntType
+__generate_random_index_below(_IntType __bound, 
_UniformRandomNumberGenerator& __g)
+{
+  const _IntType __urngrange = __g.max() - __g.min() + 1;


Similarly, let's use __urbgrange here.


+  const _IntType __scaling = __urngrange / __bound;


I think I'd like either a comment on the function documenting the
assumption about __bound and __g, or an explicit check:

   __glibcxx_assert( __scaling >= __bound );


+  const _IntType __past = __bound * __scaling;
+
+  for (;;)
+  {
+   const _IntType __r = _IntType(__g()) - __g.min();
+   if (__r < __past) return __r / __scaling;


This is basically the same algorithm as uniform_int_distribution so
doesn't introduce any bias, right?

Is this significantly faster than just using
uniform_int_distribution<_IntType>{0, __bound - 1}(__g) so we don't
need to duplicate the logic? (And people maintaining the code won't
reconvince themselves it's correct every time they look at it :-)



+  }
+}
+
  /**
   *  @brief Shuffle the elements of a sequence using a uniform random
   * number generator.
@@ -3740,6 +3756,40 @@
  typedef typename std::make_unsigned<_DistanceType>::type __ud_type;
  typedef typename std::uniform_int_distribution<__ud_type> __distr_type;
  typedef typename __distr_type::param_type __p_type;
+
+  typedef typename 
std::remove_reference<_UniformRandomNumberGenerator>::type _Gen;
+  typedef typename std::common_type::type __uc_type;
+
+  const __uc_type __urngrange = _Gen::max() - _Gen::min() + 1;
+  const __uc_type __urange = __uc_type(__last - __first);
+
+  if (__urngrange / __urange >= __urange)
+// I.e. (__urngrange >= __urange * __urange) but without wrap issues.
+  {
+   for (_RandomAccessIterator __i = __first + 1; __i != __last; )
+   {
+ const __uc_type __swap_range = __uc_type(__i - __first) + 1;
+
+ if (__i + 1 == __last)


Could we hoist this test out of the loop somehow?

If we change the loop condition to be __i+1 < __last we don't need to
test it on every iteration, and then after the loop we can just do
the final swap if (__urange % 2).


+ {
+   const __uc_type __pos = __generate_random_index_below(__swap_range, 
__g);
+   std::iter_swap(__i, __first + __pos);
+   return;
+ }
+
+ // Use a single generator invocation to produce swap positions for
+ // both of the next two elements:
+
+ const __uc_type __comp_range = __swap_range * (__swap_range + 1);
+ const __uc_type __pospos = 
__generate_random_index_below(__comp_range, __g);
+
+ std::iter_swap(__i++, __first + (__pospos % __swap_range));
+ std::iter_swap(__i++, __first + (__pospos / __swap_range));


I think I've convinced myself this is correct :-)

Values of __pospos will be uniformly distributed in [0, __comp_range)
and so the / and % results will be too.


+   }
+
+   return;
+  }
+
  __distr_type __d;

  for (_RandomAccessIterator __i = __first + 1; __i != __last; ++__i)

Re: [PATCH] Fix detection of setrlimit in libstdc++ testsuite

[Jonathan Wakely]

On 31/08/16 14:22 +0300, Maxim Kuvyrkov wrote:

On Apr 5, 2016, at 2:20 PM, Jonathan Wakely  wrote:


This patch fixes an obscure cross-testing problem that crashed (OOMed) our 
boards at Linaro.  Several tests in libstdc++ (e.g., [1]) limit themselves to 
some reasonable amount of RAM and then try to allocate 32 gigs.  Unfortunately, 
the configure test that checks presence of setrlimit is rather strange: if 
target is native, then try compile file with call to setrlimit -- if 
compilation succeeds, then use setrlimit, otherwise, ignore setrlimit.  The 
strange part is that the compilation check is done only for native targets, as 
if cross-toolchains can't generate working executables.  [This is rather odd, 
and I might be missing some underlaying caveat.]


I went spelunking, and the IS_NATIVE check has been there since
r70167, which replaced:

if test  x"$GLIBCXX_IS_CROSS_COMPILING" = xfalse; then
  # Do checks for memory limit functions.
  GLIBCXX_CHECK_SETRLIMIT

That arrived in r68067, but that seems to eb just a refactoring, and I
got lost tracking it further.

So there has been a similar check since at least 2003.


Therefore, when testing a cross toolchain, the test [1] still tries to allocate 
32GB of RAM with no setrlimit restrictions.  On most targets that people use 
for cross-testing this is not an issue because either
- the target is 32-bit, so there is no 32GB user-space to speak of, or
- the target board has small amount of RAM and no swap, so allocation 
immediately fails, or
- the target board has plenty of RAM, so allocating 32GB is not an issue.

However, if one is testing on a 64-bit board with 16GB or RAM and 16GB of swap, 
then one gets into an obscure near-OOM swapping condition.  This is exactly the 
case with cross-testing aarch64-linux-gnu toolchains on APM Mustang.

The attached patch removes "native" restriction from configure test for 
setrlimit.  This enables setrlimit restrictions on the testsuite, and the test [1] 
expectedly fails to allocate 32GB due to setrlimit restriction.

I have tested it on x86_64-linux-gnu and i686-linux-gnu native toolchains, and 
aarch64-linux-gnu and arm-linux-gnueabi[hf] cross-toolchains with no 
regressions [*].

OK to commit?


This issue has been present for well over a decade so it doesn't seem
critical to fix in stage4, but as it only affects the testsuite I am
OK with the change if the RMs have no objections.


Hi Jonathan,

This issue dropped off my patch queue.  The original patch still applies 
without conflicts, and I'm retesting it on fresh mainline -- both cross and 
native.

OK to commit if no regressions?


Yes, OK. Thanks.

Re: [MPX] Fix for PR77267

[Alexander Ivchenko]

2016-08-31 12:18 GMT+03:00 Ilya Enkovich :
> 2016-08-30 21:53 GMT+03:00 Alexander Ivchenko :
>> Would something like that count?
>>
>> I did not do the warning thing, cause the problem only appears when
>> you provide the -Wl,-as-needed option to the linker.
>> (In all other cases warning would be redundant). Are we able to check
>> that on runtime?
>>
>>
>> diff --git a/gcc/config.in b/gcc/config.in
>> index fc3321c..a736de3 100644
>> --- a/gcc/config.in
>> +++ b/gcc/config.in
>> @@ -1538,6 +1538,12 @@
>>  #endif
>>
>>
>> +/* Define if your linker supports --push-state/--pop-state */
>> +#ifndef USED_FOR_TARGET
>> +#undef HAVE_LD_PUSHPOPSTATE_SUPPORT
>> +#endif
>> +
>> +
>>  /* Define if your linker links a mix of read-only and read-write sections 
>> into
>> a read-write section. */
>>  #ifndef USED_FOR_TARGET
>> diff --git a/gcc/config/i386/linux-common.h b/gcc/config/i386/linux-common.h
>> index 4b9910f..6aa195d 100644
>> --- a/gcc/config/i386/linux-common.h
>> +++ b/gcc/config/i386/linux-common.h
>> @@ -79,13 +79,23 @@ along with GCC; see the file COPYING3.  If not see
>>  #endif
>>  #endif
>>
>> +#ifdef HAVE_LD_PUSHPOPSTATE_SUPPORT
>> +#define MPX_LD_AS_NEEDED_GUARD_PUSH "--push-state --no-as-needed"
>> +#define MPX_LD_AS_NEEDED_GUARD_POP "--pop-state"
>> +#else
>> +#define MPX_LD_AS_NEEDED_GUARD_PUSH ""
>> +#define MPX_LD_AS_NEEDED_GUARD_POP ""
>> +#endif
>> +
>>  #ifndef LIBMPX_SPEC
>>  #if defined(HAVE_LD_STATIC_DYNAMIC)
>>  #define LIBMPX_SPEC "\
>>  %{mmpx:%{fcheck-pointer-bounds:\
>>  %{static:--whole-archive -lmpx --no-whole-archive" LIBMPX_LIBS "}\
>>  %{!static:%{static-libmpx:" LD_STATIC_OPTION " --whole-archive}\
>> --lmpx %{static-libmpx:--no-whole-archive " LD_DYNAMIC_OPTION \
>> +" MPX_LD_AS_NEEDED_GUARD_PUSH  " -lmpx " MPX_LD_AS_NEEDED_GUARD_POP "\
>> +%{static-libmpx:--no-whole-archive "\
>> +LD_DYNAMIC_OPTION \
>
> Looks like you add guards for both static-libmpx and dynamic linking cases.
> You shouldn't need it for static-libmpx case.

Are you sure about that? May be I missed something, buy when I do
> gcc out-of-bounds.c -mmpx -fcheck-pointer-bounds -static -###

I got: ... -whole-archive -lmpx --no-whole-archive -lpthread
-lmpxwrappers --start-group -lgcc -lgcc_eh -lc --end-group ...
So no guards for static case. Could you clarify?



>>  LIBMPX_LIBS ""
>>  #else
>>  #define LIBMPX_SPEC "\
>> @@ -99,7 +109,8 @@ along with GCC; see the file COPYING3.  If not see
>>  %{mmpx:%{fcheck-pointer-bounds:%{!fno-chkp-use-wrappers:\
>>  %{static:-lmpxwrappers}\
>>  %{!static:%{static-libmpxwrappers:" LD_STATIC_OPTION " --whole-archive}\
>> --lmpxwrappers %{static-libmpxwrappers:--no-whole-archive "\
>> +" MPX_LD_AS_NEEDED_GUARD_PUSH " -lmpxwrappers "
>> MPX_LD_AS_NEEDED_GUARD_POP "\
>> +%{static-libmpxwrappers:--no-whole-archive "\
>>  LD_DYNAMIC_OPTION "}"
>
> I believe wrappers should work fine with --as-needed and don't need
> this guard. Otherwise looks OK.

wrappers also are not linked to the binary with -as-needed. So if I
remove guards from libmpxwrappers:
>> gcc out-of-bounds.c -mmpx -fcheck-pointer-bounds -Wl,-as-needed -###
... -as-needed --push-state --no-as-needed -lmpx --pop-state
-lmpxwrappers -z bndplt -lgcc --as-needed -lgcc_s --no-as-needed -lc
-lgcc --as-needed -lgcc_ ...

> ldd a.out
linux-vdso.so.1 (0x7ffd987cf000)
libmpx.so.2 => not found
libc.so.6 => /lib64/libc.so.6 (0x7f0f27bf3000)
/lib64/ld-linux-x86-64.so.2 (0x55bfba052000)



> Ilya
>
>>  #else
>>  #define LIBMPXWRAPPERS_SPEC "\
>> diff --git a/gcc/configure b/gcc/configure
>> index 871ed0c..094 100755
>> --- a/gcc/configure
>> +++ b/gcc/configure
>> @@ -29609,6 +29609,30 @@ fi
>>  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ld_bndplt_support" >&5
>>  $as_echo "$ld_bndplt_support" >&6; }
>>
>> +# Check linker supports '--push-state'/'--pop-state'
>> +ld_pushpopstate_support=no
>> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking linker
>> --push-state/--pop-state options" >&5
>> +$as_echo_n "checking linker --push-state/--pop-state options... " >&6; }
>> +if test x"$ld_is_gold" = xno; then
>> +  if test $in_tree_ld = yes ; then
>> +if test "$gcc_cv_gld_major_version" -eq 2 -a
>> "$gcc_cv_gld_minor_version" -ge 25 -o "$gcc_cv_gld_major_version" -gt
>> 2; then
>> +  ld_pushpopstate_support=yes
>> +fi
>> +  elif test x$gcc_cv_ld != x; then
>> +# Check if linker supports --push-state/--pop-state options
>> +if $gcc_cv_ld --help 2>/dev/null | grep -- '--push-state' > /dev/null; 
>> then
>> +  ld_pushpopstate_support=yes
>> +fi
>> +  fi
>> +fi
>> +if test x"$ld_pushpopstate_support" = xyes; then
>> +
>> +$as_echo "#define HAVE_LD_PUSHPOPSTATE_SUPPORT 1" >>confdefs.h
>> +
>> +fi
>> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ld_pushpopstate_support" 
>> >&5
>> +$as_echo "$ld_pushpopstate_support" >&6; }
>> +
>>  # Configure the subdirectories
>>  # AC_CONFIG_SUBDIRS($su

Re: [PATCH] C: fixits for modernizing structure member designators

[David Malcolm]

On Wed, 2016-08-31 at 11:09 +0200, Marek Polacek wrote:
> On Tue, Aug 30, 2016 at 11:22:21AM -0400, David Malcolm wrote:
> > On Tue, 2016-08-30 at 16:50 +0200, Marek Polacek wrote:
> > > On Tue, Aug 30, 2016 at 04:40:57PM +0200, Bernd Schmidt wrote:
> > > > On 08/30/2016 04:38 PM, David Malcolm wrote:
> > > > 
> > > > > In conjunction with the not-yet-in-trunk -fdiagnostics
> > > > > -generate
> > > > > -patch,
> > > > > this can generate patches like this:
> > > > > 
> > > > > --- modernize-named-inits.c
> > > > > +++ modernize-named-inits.c
> > > > > @@ -16,7 +16,7 @@
> > > > >  /* Old-style named initializers.  */
> > > > > 
> > > > >  struct foo old_style_f = {
> > > > > - foo: 1,
> > > > > - bar: 2,
> > > > > + .foo= 1,
> > > > > + .bar= 2,
> > > > >  };
> > > > 
> > > > What happens if there are newlines in between any of the
> > > > tokens?
> > > 
> > > It's easy to check for yourself: when the identifier and colon
> > > are
> > > not
> > > on the same line, we print something like
> > > 
> > > w.c: In function ‘main’:
> > > w.c:7:1: warning: obsolete use of designated initializer with ‘:’
> > > [
> > > -Wpedantic]
> > >  :
> > >  ^
> > >   =
> > > 
> > > which I don't think is desirable -- giving up on the fix-it hint
> > > in
> > > such case
> > > could be appropriate.
> > 
> > I think that's a bug in diagnostic-show-locus.c; currently it only
> > prints lines and fixits for the lines references in the ranges
> > within
> > the rich_location.  I'll try to fix that.
> > 
> > > I admit I dislike the lack of a space before = in ".bar= 2", but
> > > using
> > >   
> > >   richloc.add_fixit_replace (colon_loc, " =");
> > > 
> > > wouldn't work for "foo : 1" I think.
> > 
> > I actually tried that first, but I didn't like the way it was
> > printed
> > e.g.:
> > 
> > w.c: In function ‘main’:> w.c:7:1: warning: obsolete use of
> > designated
> > initializer with ‘:’ [-Wpedantic]>
> >   foo: 1,
> >  ^
> >   .   =
> > 
> > I'm looking at rewriting how fixits get printed, to print the
> > affected
> > range of the affected lines (using the edit-context.c work posted
> > last
> > week), so this would appear as:
> > 
> >   foo: 1,
> >  ^
> >   .foo =
>  
> This would be perfect.

BTW, this is currently blocked, I need review of this patch:
  "[PATCH 2/4] Improvements to typed_splay_tree"
   * https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01777.html

(I've rewritten edit_context to work on a per-line basis, so it should
now be efficient enough for this use-case)

> > Also, there may be a case for adding some smarts to
> > gcc_rich_location for adding fixits in a formatting-aware way, by
> > looking at the neighboring whitespace (this might help for the
> > issue with adding "break;" etc in the fallthru patch kit).
> 
> Thanks.  I hope it won't be too hard to implement :/

I'll try...

Re: libgo/runtime: Fix signal stack size for ia64

[Ian Lance Taylor]

On Wed, Aug 31, 2016 at 1:24 AM, Andreas Schwab  wrote:
> On ia64, MINSIGSTKSZ is 128K.
>
> Andreas.
>
> * libgo/runtime/runtime.c (runtime_mpreinit): Increase stack size to
> 128K.
> ---
>  libgo/runtime/runtime.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libgo/runtime/runtime.c b/libgo/runtime/runtime.c
> index c7d33bc..e8eb957 100644
> --- a/libgo/runtime/runtime.c
> +++ b/libgo/runtime/runtime.c
> @@ -272,7 +272,7 @@ runtime_tickspersecond(void)
>  void
>  runtime_mpreinit(M *mp)
>  {
> -   mp->gsignal = runtime_malg(32*1024, (byte**)&mp->gsignalstack, 
> &mp->gsignalstacksize);  // OS X wants >=8K, Linux >=2K
> +   mp->gsignal = runtime_malg(128*1024, (byte**)&mp->gsignalstack, 
> &mp->gsignalstacksize); // OS X wants >=8K, Linux >=2K, ia64 >=128K
> mp->gsignal->m = mp;
>  }

Thanks.  There doesn't seem to be a need to increase the stack size on
all architectures, so I applied this patch.  Bootstrapped and ran Go
tests on x86_64-pc-linux-gnu, which I admit means little.  Committed
to mainline.  Let me know if it doesn't fix the problem on ia64 (does
Go really work on ia64?).

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 239872)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-394486a1cec9bbb81216311ed153179d9fe1c2c5
+c8cf90f2daf62428ca6aa0b5674572cd99f25fe3
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/runtime/runtime.c
===
--- libgo/runtime/runtime.c (revision 239872)
+++ libgo/runtime/runtime.c (working copy)
@@ -272,7 +272,14 @@ runtime_tickspersecond(void)
 void
 runtime_mpreinit(M *mp)
 {
-   mp->gsignal = runtime_malg(32*1024, (byte**)&mp->gsignalstack, 
&mp->gsignalstacksize);  // OS X wants >=8K, Linux >=2K
+   int32 stacksize = 32 * 1024;// OS X wants >=8K, Linux >=2K
+
+#ifdef SIGSTKSZ
+   if(stacksize < SIGSTKSZ)
+   stacksize = SIGSTKSZ;
+#endif
+
+   mp->gsignal = runtime_malg(stacksize, (byte**)&mp->gsignalstack, 
&mp->gsignalstacksize);
mp->gsignal->m = mp;
 }
 

Fwd: libgo/runtime: Fix signal stack size for ia64

[Ian Lance Taylor]

Forgot to send to gofrontend-dev.

Ian


-- Forwarded message --
From: Ian Lance Taylor 
Date: Wed, Aug 31, 2016 at 6:59 AM
Subject: Re: libgo/runtime: Fix signal stack size for ia64
To: Andreas Schwab 
Cc: gcc-patches 


On Wed, Aug 31, 2016 at 1:24 AM, Andreas Schwab  wrote:
> On ia64, MINSIGSTKSZ is 128K.
>
> Andreas.
>
> * libgo/runtime/runtime.c (runtime_mpreinit): Increase stack size to
> 128K.
> ---
>  libgo/runtime/runtime.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libgo/runtime/runtime.c b/libgo/runtime/runtime.c
> index c7d33bc..e8eb957 100644
> --- a/libgo/runtime/runtime.c
> +++ b/libgo/runtime/runtime.c
> @@ -272,7 +272,7 @@ runtime_tickspersecond(void)
>  void
>  runtime_mpreinit(M *mp)
>  {
> -   mp->gsignal = runtime_malg(32*1024, (byte**)&mp->gsignalstack, 
> &mp->gsignalstacksize);  // OS X wants >=8K, Linux >=2K
> +   mp->gsignal = runtime_malg(128*1024, (byte**)&mp->gsignalstack, 
> &mp->gsignalstacksize); // OS X wants >=8K, Linux >=2K, ia64 >=128K
> mp->gsignal->m = mp;
>  }

Thanks.  There doesn't seem to be a need to increase the stack size on
all architectures, so I applied this patch.  Bootstrapped and ran Go
tests on x86_64-pc-linux-gnu, which I admit means little.  Committed
to mainline.  Let me know if it doesn't fix the problem on ia64 (does
Go really work on ia64?).

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 239872)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-394486a1cec9bbb81216311ed153179d9fe1c2c5
+c8cf90f2daf62428ca6aa0b5674572cd99f25fe3
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/runtime/runtime.c
===
--- libgo/runtime/runtime.c (revision 239872)
+++ libgo/runtime/runtime.c (working copy)
@@ -272,7 +272,14 @@ runtime_tickspersecond(void)
 void
 runtime_mpreinit(M *mp)
 {
-   mp->gsignal = runtime_malg(32*1024, (byte**)&mp->gsignalstack, 
&mp->gsignalstacksize);  // OS X wants >=8K, Linux >=2K
+   int32 stacksize = 32 * 1024;// OS X wants >=8K, Linux >=2K
+
+#ifdef SIGSTKSZ
+   if(stacksize < SIGSTKSZ)
+   stacksize = SIGSTKSZ;
+#endif
+
+   mp->gsignal = runtime_malg(stacksize, (byte**)&mp->gsignalstack, 
&mp->gsignalstacksize);
mp->gsignal->m = mp;
 }
 

Re: [PATCH, rs6000] Fix PR72827 (ada bootstrap failure)

[Bill Schmidt]

On 8/31/16 1:19 AM, Segher Boessenkool wrote:
> Hi Bill,
>
> On Tue, Aug 30, 2016 at 08:23:46PM -0500, Bill Schmidt wrote:
>> The ada bootstrap failure reported in 
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72827
>> occurs because of a latent bug in the powerpc back end.  The immediate cause 
>> is dead store
>> elimination removing two stores relative to the frame pointer that are not 
>> dead; however,
>> DSE is tricked into doing this because we have temporarily adjusted the 
>> frame pointer prior
>> to performing the loads.  DSE relies on the frame pointer remaining constant 
>> to be able to
>> infer stack stores that are dead.
> DSE should really detect this is happening and not do the wrong thing.
> Maybe add an assert somewhere?  Much easier to debug, that way.

I'm not sure I'm the right person to do that, as I don't really have any
familiarity with the
DSE code.  I can't even prove to myself that this code is alloca-safe;
it doesn't look like it.
>
>> Is this ok for trunk, and eventually for backport to the 5 and 6 branches?
> Will it backport without changes?

I haven't looked to be sure, but I think only minor changes would be
required.  The bug has
been there since 2010.
>
>> Index: gcc/config/rs6000/rs6000.c
>> ===
>> --- gcc/config/rs6000/rs6000.c   (revision 239871)
>> +++ gcc/config/rs6000/rs6000.c   (working copy)
>> @@ -24506,15 +24506,31 @@ rs6000_split_multireg_move (rtx dst, rtx src)
>>&& REG_P (basereg)
>>&& REG_P (offsetreg)
>>&& REGNO (basereg) != REGNO (offsetreg));
>> -  if (REGNO (basereg) == 0)
>> +  /* We mustn't modify the stack pointer or frame pointer
>> + as this will confuse dead store elimination.  */
>> +  if ((REGNO (basereg) == STACK_POINTER_REGNUM
>> +   || REGNO (basereg) == HARD_FRAME_POINTER_REGNUM)
>> +  && REGNO (offsetreg) != 0)
> This should only check for HARD_FRAME_POINTER_REGNUM if there *is* a
> frame pointer?
So, check if hard_frame_pointer_rtx is non-nil?  I can add that.
>
>>  {
>> -  rtx tmp = offsetreg;
>> -  offsetreg = basereg;
>> -  basereg = tmp;
> std::swap (basereg, offsetreg);
I didn't actually change that code (which predates the C++ switch), but
sure,
I can make the change.
>
>> +  emit_insn (gen_add3_insn (offsetreg, basereg,
>> +offsetreg));
>> +  restore_basereg = gen_sub3_insn (offsetreg, offsetreg,
>> +   basereg);
>> +  dst = replace_equiv_address (dst, offsetreg);
>>  }
>> -  emit_insn (gen_add3_insn (basereg, basereg, offsetreg));
>> -  restore_basereg = gen_sub3_insn (basereg, basereg, offsetreg);
>> -  dst = replace_equiv_address (dst, basereg);
>> +  else
>> +{
>> +  if (REGNO (basereg) == 0)
>> +{
>> +  rtx tmp = offsetreg;
>> +  offsetreg = basereg;
>> +  basereg = tmp;
>> +}
>> +  emit_insn (gen_add3_insn (basereg, basereg, offsetreg));
>> +  restore_basereg = gen_sub3_insn (basereg, basereg,
>> +   offsetreg);
>> +  dst = replace_equiv_address (dst, basereg);
>> +}
>>  }
>>  }
>>else if (GET_CODE (XEXP (dst, 0)) != LO_SUM)
> If (say) base=r1 offset=r0 this will now adjust r1?  That cannot be good.
Mm, yeah, that wasn't well-thought.  Was thinking 0, not r0.  Will have
to avoid
that.

Bill
>
>
> Segher
>

Re: [PATCH 2/4] Improvements to typed_splay_tree

[Bernd Schmidt]

On 08/24/2016 03:28 AM, David Malcolm wrote:

+/* Helper type for typed_splay_tree::foreach.  */
+
+template 
+struct closure


Is this in the global namespace? If so, something more specific than 
"closure" might be more appropriate. Or move the struct into the 
typed_splay_tree class definition.


Looks ok otherwise.


Bernd

Re: libgo/runtime: Fix signal stack size for ia64

[Andreas Schwab]

On Aug 31 2016, Ian Lance Taylor  wrote:

> Go really work on ia64?).

http://gcc.gnu.org/ml/gcc-testresults/2016-08/msg03154.html

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] Fixes accidental renaming of gdb.py file (i.e. libstdc++.so.6.0.22-gdb.py)

[Gerald Pfeifer]

Michael,

I noticed you did not copy the libstc++ list, so let me do this
for you.  (If any further ping is necessary, please include that
list.)

Jonathan, if you approve, Michael does not have commit access.

Gerald

On Thu, 16 Jul 2015, Michael Darling wrote:
> Ping.  I don't have write access.
> 
> The attached patch still works with current trunk source, only failing
> on the ChangeLog due to submissions since then.
> 
> On Fri, Jul 3, 2015 at 9:50 PM, Michael Darling  wrote:
>> The addition of libstdc++fs broke an inexact and fragile method in the
>> libstdc++-v3/python makefile, so it mis-names a python script after
>> libstdc++fs rather than libstdc++.
>>
>> With DESTDIR /usr/lib, toolexeclibdir ../lib, and the .so version of
>> 6.0.21, this makefile used to install the python script to
>> /usr/lib/libstdc++.so.6.0.21-gdb.py.
>>
>> Once libstdc++fs was added, this makefile installs the python script
>> to /usr/lib/libstdc++fs.a-gdb.py.
>>
>> This makefile examines files named libstdc++* in
>> DESTDIR/toolexeclibdir, excluding: symlinks; *.la files; and previous
>> *-gdb.py files.  Its comments report it is done this way because
>> "libtool hides the real names from us".
>>
>> This patch changes the makefile so it examines files named libstdc++.*
>> (notice the addition of the dot.)  Although this is still not an
>> optimum method, it at least puts the makefile on the right track
>> again.  Adding the dot is more future-proof than excluding files
>> starting with libstdc++fs, because of the possibility of future
>> additions of similarly named libraries.
>>
>> The patch below is also an attachment to this email.
>>
>>
>>
>> Index: libstdc++-v3/ChangeLog
>> ===
>> --- libstdc++-v3/ChangeLog(revision 225409)
>> +++ libstdc++-v3/ChangeLog(working copy)
>> @@ -1,3 +1,9 @@
>> +2015-07-03  Michael Darling  
>> +
>> +* python/Makefile.am: python script name based off libstdc++.* rather
>> +than libstdc++*, to avoid being mis-named after libstdc++fs.
>> +* python/Makefile.in: Regenerate.
>> +
>>  2015-07-03  Jonathan Wakely  
>>
>>  * doc/xml/manual/status_cxx2017.xml: Update status table.
>> Index: libstdc++-v3/python/Makefile.am
>> ===
>> --- libstdc++-v3/python/Makefile.am(revision 225409)
>> +++ libstdc++-v3/python/Makefile.am(working copy)
>> @@ -45,11 +45,11 @@
>>  @$(mkdir_p) $(DESTDIR)$(toolexeclibdir)
>>  ## We want to install gdb.py as SOMETHING-gdb.py.  SOMETHING is the
>>  ## full name of the final library.  We want to ignore symlinks, the
>> -## .la file, and any previous -gdb.py file.  This is inherently
>> -## fragile, but there does not seem to be a better option, because
>> -## libtool hides the real names from us.
>> +## .la file, any previous -gdb.py file, and libstdc++fs*.  This is
>> +## inherently fragile, but there does not seem to be a better option,
>> +## because libtool hides the real names from us.
>>  @here=`pwd`; cd $(DESTDIR)$(toolexeclibdir); \
>> -  for file in libstdc++*; do \
>> +  for file in libstdc++.*; do \
>>  case $$file in \
>>*-gdb.py) ;; \
>>*.la) ;; \
>> Index: libstdc++-v3/python/Makefile.in
>> ===
>> --- libstdc++-v3/python/Makefile.in(revision 225409)
>> +++ libstdc++-v3/python/Makefile.in(working copy)
>> @@ -547,7 +547,7 @@
>>  install-data-local: gdb.py
>>  @$(mkdir_p) $(DESTDIR)$(toolexeclibdir)
>>  @here=`pwd`; cd $(DESTDIR)$(toolexeclibdir); \
>> -  for file in libstdc++*; do \
>> +  for file in libstdc++.*; do \
>>  case $$file in \
>>*-gdb.py) ;; \
>>*.la) ;; \

gcc.libstdc++-v3.python.dot.fix.patch
Description: Binary data

Re: [PATCH] Fixes accidental renaming of gdb.py file (i.e. libstdc++.so.6.0.22-gdb.py)

[Jonathan Wakely]

On 31/08/16 16:52 +0200, Gerald Pfeifer wrote:

Michael,

I noticed you did not copy the libstc++ list, so let me do this
for you.  (If any further ping is necessary, please include that
list.)


Thanks, Gerald.


Jonathan, if you approve, Michael does not have commit access.


I found the problem myself and fixed it exactly the same way with
https://gcc.gnu.org/r227030 (with a mangled changelog sadly).




Gerald

On Thu, 16 Jul 2015, Michael Darling wrote:

Ping.  I don't have write access.

The attached patch still works with current trunk source, only failing
on the ChangeLog due to submissions since then.


N.B. The usual solution to that problem is to send the ChangeLog entry
separately (either as a separate attachment, or inline in the email
body) not as part of the patch.

[PATCH] Fix some warnings/errors that appear when enabling -Wnarrowing when building gcc

[Eric Gallager]

In https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01526.html I tried
enabling -Wnarrowing when building GCC and produced a log of the
resulting (uniq-ed) warnings/errors. The attached patch here fixes
some of them by using the 'U' suffix to make certain constants
unsigned so they don't become negative when applying the '~' operator
to them. After applying, there were still some narrowing issues
remaining that would have required modifying gcc/optc-gen.awk to fix
properly, but that looked too complicated so I'm avoiding it for now.
Still, at least by patching the files I did patch, I allowed bootstrap
to continue a little farther...

Thanks,
Eric Gallager
 gcc/config/i386/i386.c   | 60 ++--
 gcc/config/i386/x86-tune.def |  6 ++---
 gcc/opts.c   |  4 +--
 3 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 4531647..181fc39 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -2162,45 +2162,45 @@ const struct processor_costs *ix86_tune_cost = 
&pentium_cost;
 const struct processor_costs *ix86_cost = &pentium_cost;
 
 /* Processor feature/optimization bitmasks.  */
-#define m_386 (1<

Re: [PATCH, rs6000] Fix PR72827 (ada bootstrap failure)

[Eric Botcazou]

> I'm not sure I'm the right person to do that, as I don't really have any
> familiarity with the DSE code.  I can't even prove to myself that this code
> is alloca-safe; it doesn't look like it.

No FUD, please ;-)  The code is alloca-safe, alloca is about moving the stack 
pointer, not the frame pointer.  The compiler wouldn't be able to compile a 
simple Ada program with optimization enabled if that wasn't the case.

-- 
Eric Botcazou

[ARM] Fix broken sibcall with longcall, APCS frame and VFP

[Eric Botcazou]

Hi,

this is a regression present on the mainline and 6 branch and introduced by:

2014-04-25  Jiong Wang  

* config/arm/predicates.md (call_insn_operand): Add long_call check.
* config/arm/arm.md (sibcall, sibcall_value): Force the address to
reg for long_call.
* config/arm/arm.c (arm_function_ok_for_sibcall): Remove long_call
restriction.

For a longcall, any sibcall becomes an indirect sibcall and therefore requires 
a register to hold the target address.  Now if all the argument registers are  
taken, this register will be IP but, for APCS frames and VFP, IP can be used 
in the sibcall epilogue to restore the VFP registers, so the target address is 
overwritten and the call goes astray.  Testcase attached, compile it with e.g. 
-mapcs-frame -mfloat-abi=soft -O -foptimize-sibling-calls -ffunction-sections
and you'll see for arm-eabi:

sub ip, fp, #36
vldmip!, {d8}
sub sp, fp, #28
ldmfd   sp, {r4, r5, r6, r7, fp, sp, lr}
bx  ip

The attached patch reinstates the restriction for APCS frames and VFP.  This 
might be deemed a big hammer, but isn't a regression from earlier releases of 
the compiler and APCS frames are deprecated in any case (we still support them 
for VxWorks 6 but VxWorks 7 switched to AAPCS).

Tested on ARM/VxWorks 6 and ARM/EABI, OK for mainline and 6 branch?


2016-08-31  Eric Botcazou  

* config/arm/arm.c (arm_function_ok_for_sibcall): Add back restriction
for long calls with APCS frame and VFP.


2016-08-31  Eric Botcazou  

* gcc.target/arm/vfp-longcall-apcs.c: New test.

-- 
Eric BotcazouIndex: config/arm/arm.c
===
--- config/arm/arm.c	(revision 239888)
+++ config/arm/arm.c	(working copy)
@@ -6773,6 +6773,14 @@ arm_function_ok_for_sibcall (tree decl, tree exp)
   if (TARGET_VXWORKS_RTP && flag_pic && decl && !targetm.binds_local_p (decl))
 return false;
 
+  /* ??? Cannot tail-call to long calls with APCS frame and VFP, because IP
+ may be used both as target of the call and base register for restoring
+ the VFP registers  */
+  if (TARGET_APCS_FRAME && TARGET_ARM
+  && TARGET_HARD_FLOAT && TARGET_VFP
+  && decl && arm_is_long_call_p (decl))
+return false;
+
   /* If we are interworking and the function is not declared static
  then we can't tail-call it unless we know that it exists in this
  compilation unit (since it might be a Thumb routine).  */
/* { dg-do run } */
/* { dg-require-effective-target arm_vfp_ok } */
/* { dg-options "-mapcs-frame -mfpu=vfp -mfloat-abi=softfp -O -foptimize-sibling-calls -ffunction-sections" } */

extern void abort (void);

static __attribute__((noclone, noinline, long_call))
int foo (int a, int b, int c, int d, double i)
{
  return a;
}

static __attribute__((noclone, noinline))
double baz (double i)
{
  return i;
}

static __attribute__((noclone, noinline))
int bar (int a, int b, int c, int d, double i, double j)
{
  double l = baz (i) * j;
  return foo (a, b, c, d, l);
}

int
main (void)
{
  if (bar (0, 0, 0, 0, 0.0, 0.0) != 0)
abort ();

  return 0;
}

Re: [ARM] Fix broken sibcall with longcall, APCS frame and VFP

[Richard Earnshaw (lists)]

On 31/08/16 16:35, Eric Botcazou wrote:
> Hi,
> 
> this is a regression present on the mainline and 6 branch and introduced by:
> 
> 2014-04-25  Jiong Wang  
> 
>   * config/arm/predicates.md (call_insn_operand): Add long_call check.
>   * config/arm/arm.md (sibcall, sibcall_value): Force the address to
>   reg for long_call.
>   * config/arm/arm.c (arm_function_ok_for_sibcall): Remove long_call
>   restriction.
> 
> For a longcall, any sibcall becomes an indirect sibcall and therefore 
> requires 
> a register to hold the target address.  Now if all the argument registers are 
>  
> taken, this register will be IP but, for APCS frames and VFP, IP can be used 
> in the sibcall epilogue to restore the VFP registers, so the target address 
> is 
> overwritten and the call goes astray.  Testcase attached, compile it with 
> e.g. 
> -mapcs-frame -mfloat-abi=soft -O -foptimize-sibling-calls -ffunction-sections
> and you'll see for arm-eabi:
> 
>   sub ip, fp, #36
>   vldmip!, {d8}
>   sub sp, fp, #28
>   ldmfd   sp, {r4, r5, r6, r7, fp, sp, lr}
>   bx  ip
> 
> The attached patch reinstates the restriction for APCS frames and VFP.  This 
> might be deemed a big hammer, but isn't a regression from earlier releases of 
> the compiler and APCS frames are deprecated in any case (we still support 
> them 
> for VxWorks 6 but VxWorks 7 switched to AAPCS).
> 
> Tested on ARM/VxWorks 6 and ARM/EABI, OK for mainline and 6 branch?
> 
> 

Since you're going to need a back-port there should be a PR filed for this.


> 2016-08-31  Eric Botcazou  
> 
> * config/arm/arm.c (arm_function_ok_for_sibcall): Add back restriction
> for long calls with APCS frame and VFP.
> 
> 
> 2016-08-31  Eric Botcazou  
> 
>   * gcc.target/arm/vfp-longcall-apcs.c: New test.
> 

Have you checked that this works with multi-lib dejagnu runs and on
hard-float systems?  I doubt this will work in armhf-linux, for example.

R.

> 
> p.diff
> 
> 
> Index: config/arm/arm.c
> ===
> --- config/arm/arm.c  (revision 239888)
> +++ config/arm/arm.c  (working copy)
> @@ -6773,6 +6773,14 @@ arm_function_ok_for_sibcall (tree decl, tree exp)
>if (TARGET_VXWORKS_RTP && flag_pic && decl && !targetm.binds_local_p 
> (decl))
>  return false;
>  
> +  /* ??? Cannot tail-call to long calls with APCS frame and VFP, because IP
> + may be used both as target of the call and base register for restoring
> + the VFP registers  */
> +  if (TARGET_APCS_FRAME && TARGET_ARM
> +  && TARGET_HARD_FLOAT && TARGET_VFP
> +  && decl && arm_is_long_call_p (decl))
> +return false;
> +
>/* If we are interworking and the function is not declared static
>   then we can't tail-call it unless we know that it exists in this
>   compilation unit (since it might be a Thumb routine).  */
> 
> 
> vfp-longcall-apcs.c
> 
> 
> /* { dg-do run } */
> /* { dg-require-effective-target arm_vfp_ok } */
> /* { dg-options "-mapcs-frame -mfpu=vfp -mfloat-abi=softfp -O 
> -foptimize-sibling-calls -ffunction-sections" } */
> 
> extern void abort (void);
> 
> static __attribute__((noclone, noinline, long_call))
> int foo (int a, int b, int c, int d, double i)
> {
>   return a;
> }
> 
> static __attribute__((noclone, noinline))
> double baz (double i)
> {
>   return i;
> }
> 
> static __attribute__((noclone, noinline))
> int bar (int a, int b, int c, int d, double i, double j)
> {
>   double l = baz (i) * j;
>   return foo (a, b, c, d, l);
> }
> 
> int
> main (void)
> {
>   if (bar (0, 0, 0, 0, 0.0, 0.0) != 0)
> abort ();
> 
>   return 0;
> }
> 

[x86] Disable STV pass if -mstackrealign is enabled.

[Eric Botcazou]

Hi,

the new STV pass generates SSE instructions in 32-bit mode very late in the 
pipeline and doesn't bother about realigning the stack, so it wreaks havoc on 
OSes where you need to realign the stack, e.g. Windows, but I guess Solaris is 
equally affected.  Therefore the attached patch disables it if -mstackrealign 
is enabled (the option is automatically enabled on Windows and Solaris when 
SSE support is enabled), as already done for -mpreferred-stack-boundary={2,3} 
and -mincoming-stack-boundary={2,3}.

Tested on x86/Windows, OK for mainline and 6 branch?


2016-08-31  Eric Botcazou  

* config/i386/i386.c (ix86_option_override_internal): Also disable the
STV pass if -mstackrealign is enabled.

-- 
Eric BotcazouIndex: config/i386/i386.c
===
--- config/i386/i386.c	(revision 239842)
+++ config/i386/i386.c	(working copy)
@@ -5957,11 +5957,12 @@ ix86_option_override_internal (bool main_args_p,
   if (!(opts_set->x_target_flags & MASK_STV))
 opts->x_target_flags |= MASK_STV;
   /* Disable STV if -mpreferred-stack-boundary={2,3} or
- -mincoming-stack-boundary={2,3} - the needed
+ -mincoming-stack-boundary={2,3} or -mstackrealign - the needed
  stack realignment will be extra cost the pass doesn't take into
  account and the pass can't realign the stack.  */
   if (ix86_preferred_stack_boundary < 128
-  || ix86_incoming_stack_boundary < 128)
+  || ix86_incoming_stack_boundary < 128
+  || opts->x_ix86_force_align_arg_pointer)
 opts->x_target_flags &= ~MASK_STV;
   if (!ix86_tune_features[X86_TUNE_AVX256_UNALIGNED_LOAD_OPTIMAL]
   && !(opts_set->x_target_flags & MASK_AVX256_SPLIT_UNALIGNED_LOAD))

Re: PR35503 - warn for restrict pointer (-Wrestrict)

[Prathamesh Kulkarni]

On 31 August 2016 at 03:45, Mike Stump  wrote:
> On Aug 30, 2016, at 4:57 AM, Prathamesh Kulkarni 
>  wrote:
>>
>> On 30 August 2016 at 17:11, Eric Gallager  wrote:
>>> On 8/29/16, Jason Merrill  wrote:
 On Mon, Aug 29, 2016 at 10:28 AM, Marek Polacek  wrote:
> On Mon, Aug 29, 2016 at 09:20:53AM -0400, Eric Gallager wrote:
>> I tried this patch on my fork of gdb-binutils and got a few warnings
>> from it. Would it be possible to have the caret point to the argument
>> mentioned, instead of the function name? And also print the option
>> name? E.g., instead of the current:
>>
>> or32-opc.c: In function ‘or32_print_register’:
>> or32-opc.c:956:3: warning: passing argument 1 to restrict qualified
>> parameter aliases with argument 3
>>   sprintf (disassembled, "%sr%d", disassembled, regnum);
>>   ^~~
>>
>> could it look like:
>>
>> or32-opc.c: In function ‘or32_print_register’:
>> or32-opc.c:956:3: warning: passing argument 1 to restrict qualified
>> parameter aliases with argument 3 [-Wrestrict]
>>   sprintf (disassembled, "%sr%d", disassembled, regnum);
>>^~~~
>>
>> instead?
>
> I didn't try to implement it, but I think this should be fairly easy to
> achieve in the C FE, because c_parser_postfix_expression_after_primary
> has arg_loc, which is a vector of parameter locations.

 The C++ FE doesn't have this currently, but it could be added without
 too much trouble: in cp_parser_parenthesized_expression_list, extract
 the locations from the cp_expr return value of
 cp_parser_assignment_expression, and then pass the locations back up
 to cp_parser_postfix_expression.

 Jason

>>>
>>>
>>> On the topic of how to get this warning working with various
>>> frontends, is there any reason why the Objective C frontend doesn't
>>> handle -Wrestrict? Currently when trying to use it, it just says:
>>>
>>> cc1obj: warning: command line option '-Wrestrict' is valid for C/C++
>>> but not for ObjC
>> Hi Eric,
>> I am not sure if restrict is valid for ObjC/Obj-C++ and hence didn't
>> add the option for these front-ends.
>> If it is valid, I will enable the option for ObjC and Obj-C++.
>
> This is wrong, C/C++ options should always be ObjC/ObjC++ options.
Thanks, I will add the warning for ObjC and ObjC++.

Thanks,
Prathamesh
>

Re: PR35503 - warn for restrict pointer

[Prathamesh Kulkarni]

On 30 August 2016 at 18:49, David Malcolm  wrote:
> On Tue, 2016-08-30 at 17:08 +0530, Prathamesh Kulkarni wrote:
>> On 30 August 2016 at 05:34, David Malcolm 
>> wrote:
>> > On Mon, 2016-08-29 at 20:01 -0400, David Malcolm wrote:
>> > > On Mon, 2016-08-29 at 19:55 -0400, David Malcolm wrote:
>> > > [...]
>> > > > Assuming you have the location_t values available, you can
>> > > > create a
>> > > > rich_location for the primary range, and then add secondary
>> > > > ranges
>> > > > like
>> > > > this:
>> > > >
>> > > >   rich_location richloc (loc_of_arg1);
>> > >
>> > > Oops, the above should be:
>> > >
>> > > rich_location richloc (line_table, loc_of_arg1);
>> > >
>> > > or:
>> > >
>> > > gcc_rich_location (loc_of_arg1);
>> > and this should be:
>> >
>> >  gcc_rich_location richloc (loc_of_arg1);
>> > > which does the same thing (#include "gcc-rich-location.h").
>> >
>> > Clearly I need to sleep :)
>> Hi David,
>> Thanks for the suggestions. I can now see multiple source ranges for
>> pr35503-2.c (included in patch).
>> Output shows: http://pastebin.com/FNAVDU8A
>> (Posted pastebin link to avoid mangling by the mailer)
>
> The underlines look great, thanks for working on this.
Thanks -;)
>
>> However the test for underline fails:
>> FAIL: c-c++-common/pr35503-2.c  -Wc++-compat   expected multiline
>> pattern lines 12-13 not found: "\s*f \(&alpha, &beta, &alpha,
>> &alpha\);.*\n  \^~ ~~  ~~ .*\n"
>> I have attached gcc.log for the test-case. Presumably I have written
>> the test-case incorrectly.
>> Could you please have a look at it ?
>
> (I hope this doesn't get too badly mangled by Evolution...)
>
> I think you have an extra trailing space on the line containing the
> expected underlines within the multiline directive:
>
> +/* { dg-begin-multiline-output "" }
> +   f (&alpha, &beta, &alpha, &alpha);
> +  ^~ ~~  ~~
> ^ EXTRA SPACE HERE
> +   { dg-end-multiline-output "" } */
> +}
>
> as the actual output is:
>
>f (&alpha, &beta, &alpha, &alpha);
>   ^~ ~~  ~~
>   ^ LINE ENDS HERE, with no trailing
> space present
>
> This space shows up in the error here:
>
> FAIL: c-c++-common/pr35503-2.c  -Wc++-compat   expected multiline
> pattern lines 12-13 not found: "\s*f \(&alpha, &beta, &alpha,
> &alpha\);.*\n  \^~ ~~  ~~ .*\n"
>  ^ EXTRA SPACE
>
> BTW, the .* at the end of the pattern means it's ok to have additional
> material in the actual output that isn't matched (e.g. for comments
> containing dg- directives [1] ) but it doesn't work the other way
> around: all of the text within the dg-begin/end-multiline directives
> has to be in the actual output.
>
>
> [1] so you can have e.g.:
>
>   f (&alpha, &beta, &alpha, &alpha); /* { dg-warning "passing argument 1 to 
> restrict-qualified parameter aliases with arguments 3, 4" } */
>
> and:
>
> /* { dg-begin-multiline-output "" }
>f (&alpha, &beta, &alpha, &alpha);
>   ^~ ~~  ~~
>{ dg-end-multiline-output "" } */
>
> where the actual output will look like:
>
> pr35503-2.c:8:6: warning: passing argument 1 to restrict-qualified parameter 
> aliases with arguments 3, 4 [-Wrestrict]
>f (&alpha, &beta, &alpha, &alpha); /* { dg-warning "passing argument 1 to 
> restrict-qualified parameter aliases with arguments 3, 4" } */
>  ^~ ~~  ~~
>
> and you can omit the copy of the dg-warning directive in the expected
> multiline output (which would otherwise totally confuse DejaGnu).
> Doing so avoids having to specify the line number.
Thanks for the detailed explanation! The test-case is now fixed.

Regards,
Prathamesh
>
>> Thanks,
>> Prathamesh
>> >
>> > > >   richloc.add_range (loc_of_arg3, false);  /* false here =
>> > > > don't
>> > > > draw
>> > > > a
>> > > > caret, just the underline */
>> > > >   richloc.add_range (loc_of_arg4, false);
>> > > >   warning_at_rich_loc (&richloc, OPT_Wrestrict, etc...
>> > > >
>> > > > See line-map.h for more information on rich_location.
>> > >
>> > > [...]

Re: Fix bogus warning with -Wlogical-not-parentheses (PR c/77292)

[Martin Sebor]

On 08/26/2016 07:44 AM, Marek Polacek wrote:

On Thu, Aug 25, 2016 at 11:35:53AM -0600, Martin Sebor wrote:

+foo (int a, int b)
+{
+  int r = 0;
+  r += !a == (a < b);
+  r += !a == (a > b);
+  r += !a == (a >= b);
+  r += !a == (a <= b);
+  r += !a == (a != b);
+  r += !a == (a == b);
+  r += !a == (a || b);
+  r += !a == (a && b);
+  r += !a == (!b);
+
+  r += !a == (a ^ b); /* { dg-warning "logical not is only applied to the left hand 
side of comparison" } */
+  r += !a == (a | b); /* { dg-warning "logical not is only applied to the left hand 
side of comparison" } */
+  r += !a == (a & b); /* { dg-warning "logical not is only applied to the left hand 
side of comparison" } */


A question more than a comment: warning on the last three expressions
above makes sense to me when the operands are integers but I'm less
sure that the same logic applies when the operands are Boolean.  Does
it make sense to issue the warning for those given that (a | b) and
(a & b) are the same as (a || b) and (a && b) for which the warning
is suppressed?

In other words, is warning on the latter of the two below but not on
the former a good idea or should they be treated the same way?


I gave this a shot but it seems to be quite complicated, and I'm not
sure if it's worth the effort.  If you want, open a BZ and I'll look
into this later.


I opened bug 77423 for this.

Martin

Re: [PATCH, rs6000] Fix PR72827 (ada bootstrap failure)

[Bill Schmidt]

> On Aug 31, 2016, at 10:31 AM, Eric Botcazou  wrote:
> 
>> I'm not sure I'm the right person to do that, as I don't really have any
>> familiarity with the DSE code.  I can't even prove to myself that this code
>> is alloca-safe; it doesn't look like it.
> 
> No FUD, please ;-)  The code is alloca-safe, alloca is about moving the stack 
> pointer, not the frame pointer.  The compiler wouldn't be able to compile a 
> simple Ada program with optimization enabled if that wasn't the case.


Heh, not my intent. :)  The alloca code was hiding off in cselib anyway...as
I said, I don't have any familiarity here...  

> 
> -- 
> Eric Botcazou
> 

Re: [PATCH] Fixups for Martin's gimple-ssa-sprintf.c patch

[David Malcolm]

On Wed, 2016-08-31 at 10:23 -0600, Martin Sebor wrote:
> On 08/25/2016 10:30 AM, Martin Sebor wrote:
> > On 08/25/2016 10:23 AM, David Malcolm wrote:
> > > Martin: here are the fixups for your patch I needed to apply to
> > > make
> > > it work with mine.  I couldn't actually get any of your existing
> > > test
> > > cases to emit locations within the string literals, due to them
> > > all
> > > being embedded in macro expansions (possibly relating to PR
> > > c/77328),
> > > so I added a simple testcase using -fdiagnostics-show-caret,
> > > which
> > > does successfully show a range within the string.
> > > 
> > > Posting in the hope that it's helpful; I haven't attempted a
> > > bootstrap
> > > with it.
> 
> I've tried the patch but the changes don't compile because
> substring_loc is not declared.  I see the class defined in
> c-family/c-common.h which I can't include here in the middle
> end.  Am I missing some another patch?

The fixup patch is on top of
  https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01811.html
which moves class substring_loc to gcc/substring-locations.h.

Re: [PATCH] Fixups for Martin's gimple-ssa-sprintf.c patch

[Martin Sebor]

On 08/25/2016 10:30 AM, Martin Sebor wrote:

On 08/25/2016 10:23 AM, David Malcolm wrote:

Martin: here are the fixups for your patch I needed to apply to make
it work with mine.  I couldn't actually get any of your existing test
cases to emit locations within the string literals, due to them all
being embedded in macro expansions (possibly relating to PR c/77328),
so I added a simple testcase using -fdiagnostics-show-caret, which
does successfully show a range within the string.

Posting in the hope that it's helpful; I haven't attempted a bootstrap
with it.


I've tried the patch but the changes don't compile because
substring_loc is not declared.  I see the class defined in
c-family/c-common.h which I can't include here in the middle
end.  Am I missing some another patch?

Thanks
Martin

Re: [v3 PATCH, RFC] Implement LWG 2729 for tuple

[Ville Voutilainen]

On 31 August 2016 at 18:40, Ville Voutilainen
 wrote:
> Now tested with the full testsuite on Linux-PPC64, test in the patch
> amended slightly.
> New patch attached.

I added some more torture to the new test and re-indented it.
diff --git a/libstdc++-v3/include/std/tuple b/libstdc++-v3/include/std/tuple
index c06a040..9f43732 100644
--- a/libstdc++-v3/include/std/tuple
+++ b/libstdc++-v3/include/std/tuple
@@ -220,8 +220,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   constexpr _Tuple_impl(const _Tuple_impl&) = default;
 
   constexpr
-  _Tuple_impl(_Tuple_impl&& __in)
-  noexcept(__and_,
+  _Tuple_impl(typename conditional<
+ __and_,
+is_move_constructible<_Inherited>>::value,
+ _Tuple_impl&&, __nonesuch&&>::type __in)
+   noexcept(__and_,
  is_nothrow_move_constructible<_Inherited>>::value)
   : _Inherited(std::move(_M_tail(__in))),
_Base(std::forward<_Head>(_M_head(__in))) { }
@@ -232,7 +235,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
  _Base(_Tuple_impl<_Idx, _UElements...>::_M_head(__in)) { }
 
   template
-constexpr _Tuple_impl(_Tuple_impl<_Idx, _UHead, _UTails...>&& __in)
+constexpr _Tuple_impl(typename conditional<
+ __and_,
+ is_move_constructible<_Inherited>>::value,
+ _Tuple_impl<_Idx, _UHead, _UTails...>&&,
+ __nonesuch&&>::type __in)
: _Inherited(std::move
 (_Tuple_impl<_Idx, _UHead, _UTails...>::_M_tail(__in))),
  _Base(std::forward<_UHead>
@@ -338,6 +345,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   }
 };
 
+  template
+struct __is_tuple_impl_trait_impl : false_type
+  { };
+
+  template
+struct __is_tuple_impl_trait_impl<_Tuple_impl<_Idx, _Tp...>> : true_type
+  { };
+
+  template
+struct __is_tuple_impl_trait : public __is_tuple_impl_trait_impl<_Tp>
+  { };
+
   // Basis case of inheritance recursion.
   template
 struct _Tuple_impl<_Idx, _Head>
@@ -356,11 +375,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   constexpr _Tuple_impl()
   : _Base() { }
 
+  template::value,
+ bool>::type=true>
   explicit
   constexpr _Tuple_impl(const _Head& __head)
   : _Base(__head) { }
 
-  template
+  template,
+__not_<__is_tuple_impl_trait<
+  typename
+remove_reference<_UHead>::type>>
+>::value,
+ bool>::type = true>
 explicit
 constexpr _Tuple_impl(_UHead&& __head)
: _Base(std::forward<_UHead>(__head)) { }
@@ -368,15 +396,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   constexpr _Tuple_impl(const _Tuple_impl&) = default;
 
   constexpr
-  _Tuple_impl(_Tuple_impl&& __in)
+  _Tuple_impl(typename conditional<
+ is_move_constructible<_Head>::value,
+ _Tuple_impl&&, __nonesuch&&>::type __in)
   noexcept(is_nothrow_move_constructible<_Head>::value)
   : _Base(std::forward<_Head>(_M_head(__in))) { }
 
-  template
+  template::value,
+ bool>::type = true>
 constexpr _Tuple_impl(const _Tuple_impl<_Idx, _UHead>& __in)
: _Base(_Tuple_impl<_Idx, _UHead>::_M_head(__in)) { }
 
-  template
+  template::value,
+ bool>::type = true>
 constexpr _Tuple_impl(_Tuple_impl<_Idx, _UHead>&& __in)
: _Base(std::forward<_UHead>(_Tuple_impl<_Idx, _UHead>::_M_head(__in)))
{ }
@@ -832,14 +866,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{ }
 
   tuple&
-  operator=(const tuple& __in)
+  operator=(typename
+   conditional<__and_...>::value,
+   const tuple&, const __nonesuch&>::type __in)
   {
static_cast<_Inherited&>(*this) = __in;
return *this;
   }
 
   tuple&
-  operator=(tuple&& __in)
+  operator=(typename
+   conditional<__and_...>::value,
+   tuple&&, __nonesuch&&>::type __in)
   noexcept(is_nothrow_move_assignable<_Inherited>::value)
   {
static_cast<_Inherited&>(*this) = std::move(__in);
@@ -848,7 +886,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   template::type>
+== sizeof...(_Elements)
+  &&
+  __and_...>::value>::type>
 tuple&
 operator=(const tuple<_UElements...>& __in)
 {
@@ -858,7 +899,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   template::type>
+== sizeof...(_Elements)
+  &&
+  __and_...>::value>::type>
 tuple&
 operator=(tuple<_UElements...>&& __in)
 {
@@ -1189,14 +1233,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   

Re: [PATCH] Fixups for Martin's gimple-ssa-sprintf.c patch

[Martin Sebor]

On 08/31/2016 10:26 AM, David Malcolm wrote:

On Wed, 2016-08-31 at 10:23 -0600, Martin Sebor wrote:

On 08/25/2016 10:30 AM, Martin Sebor wrote:

On 08/25/2016 10:23 AM, David Malcolm wrote:

Martin: here are the fixups for your patch I needed to apply to
make
it work with mine.  I couldn't actually get any of your existing
test
cases to emit locations within the string literals, due to them
all
being embedded in macro expansions (possibly relating to PR
c/77328),
so I added a simple testcase using -fdiagnostics-show-caret,
which
does successfully show a range within the string.

Posting in the hope that it's helpful; I haven't attempted a
bootstrap
with it.


I've tried the patch but the changes don't compile because
substring_loc is not declared.  I see the class defined in
c-family/c-common.h which I can't include here in the middle
end.  Am I missing some another patch?


The fixup patch is on top of
   https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01811.html
which moves class substring_loc to gcc/substring-locations.h.


Great!  With that patch my pass builds fine, all my tests pass,
and based on a quick comparison the output of the diagnostics
looks unchanged.

Thanks
Martin

Re: [PATCH] Define feature-test macro for std::enable_shared_from_this

[Jonathan Wakely]

On 04/08/16 13:33 +0100, Jonathan Wakely wrote:

On 03/08/16 20:11 +0100, Jonathan Wakely wrote:

Another feature we already support, so just define the macro.

* include/bits/shared_ptr_base.h (__cpp_lib_enable_shared_from_this):
Define feature-test macro.
* testsuite/20_util/enable_shared_from_this/members/reinit.cc: Test
for the macro.

Tested x86_64-linux, committed to trunk.


I realised we don't actually implement the whole feature, because we
don't have the new weak_from_this() members (careless of me to forget
the contents of my own proposal!)

This adds them for C++17, or gnu++1*, and only defines the
feature-test macro when those members are present.

Tested powerpc64-linux, committed to trunk.





commit 7c1f28db94c3cb1a28dba4efd0c648bc6c6bb329
Author: Jonathan Wakely 
Date:   Thu Aug 4 13:04:14 2016 +0100

   Define std::enable_shared_from_this::weak_from_this
   
   	* testsuite/20_util/enable_shared_from_this/members/reinit.cc: Use

effective target not dg-options. Move check for feature-test macro to:
* testsuite/20_util/enable_shared_from_this/members/weak_from_this.cc:
New test.


I made a mess of this commit, failing to add shared_ptr.h and
shared_ptr_base.h to the ChangeLog, and failing to commit the new
test!

This adds the missing test.

Tested powerpc64le-linux, committed to trunk.

commit 18184a85f843290d16a88d527d7339df26aec98b
Author: Jonathan Wakely 
Date:   Wed Aug 31 14:20:34 2016 +0100

Add test accidentally not added in revision r239121

	* testsuite/20_util/enable_shared_from_this/members/weak_from_this.cc:
	New test.

diff --git a/libstdc++-v3/testsuite/20_util/enable_shared_from_this/members/weak_from_this.cc b/libstdc++-v3/testsuite/20_util/enable_shared_from_this/members/weak_from_this.cc
new file mode 100644
index 000..b5ebb81
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/enable_shared_from_this/members/weak_from_this.cc
@@ -0,0 +1,45 @@
+// Copyright (C) 2015-2016 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+// { dg-options "-std=gnu++17" }
+
+#include 
+#include 
+
+#if __cpp_lib_enable_shared_from_this < 201603
+# error "__cpp_lib_enable_shared_from_this < 201603"
+#endif
+
+struct X : public std::enable_shared_from_this { };
+
+void
+test01()
+{
+  std::shared_ptr sp(new X);
+  auto wp1 = sp->weak_from_this();
+  std::weak_ptr wp2 = sp;
+
+  std::owner_less<> less;
+  VERIFY( !less(wp1, wp2) && !less(wp2, wp1) );
+  VERIFY( !less(wp1, wp2) && !less(wp2, wp1) );
+}
+
+int
+main()
+{
+  test01();
+}

Re: Implement C _FloatN, _FloatNx types [version 6]

[James Greenhalgh]

On Fri, Aug 19, 2016 at 04:23:55PM +, Joseph Myers wrote:
> On Fri, 19 Aug 2016, Szabolcs Nagy wrote:
> 
> > On 17/08/16 21:17, Joseph Myers wrote:
> > > Although there is HFmode support for ARM and AArch64, use of that for
> > > _Float16 is not enabled.  Supporting _Float16 would require additional
> > > work on the excess precision aspects of TS 18661-3: there are new
> > > values of FLT_EVAL_METHOD, which are not currently supported in GCC,
> > > and FLT_EVAL_METHOD == 0 now means that operations and constants on
> > > types narrower than float are evaluated to the range and precision of
> > > float.  Implementing that, so that _Float16 gets evaluated with excess
> > > range and precision, would involve changes to the excess precision
> > > infrastructure so that the _Float16 case is enabled by default, unlike
> > > the x87 case which is only enabled for -fexcess-precision=standard.
> > > Other differences between _Float16 and __fp16 would also need to be
> > > disentangled.
> > 
> > i wonder how gcc can support _Float16 without excess
> > precision.
> > 
> > using FLT_EVAL_METHOD==16 can break conforming c99/c11
> > code which only expects 0,1,2 values to appear (and does
> > not use _Float16 at all), but it seems to be the better
> > fit for hardware with half precision instructions.
> 
> Maybe this indicates that -fexcess-precision=standard, whether explicit or 
> implies by a -std option, must cause FLT_EVAL_METHOD=0 for such hardware, 
> and some new -fexcess-precision= option is needed to select 
> FLT_EVAL_METHOD=16 (say -fexcess-precision=16, with no expectation that 
> most numeric -fexcess-precision= arguments are supported except where a 
> target hook says they are or where they are the default FLT_EVAL_METHOD 
> value).

That seems reasonable for -fexcess-precision=standard and where it is
implied by a -std=c?? option.

My concern with this is that the use of comparisons of FLT_EVAL_METHOD
against 0 that Szabolcs is referring to is common and can have performance
implications. In glibc for example, 

  static FLOAT
  overflow_value (int negative)
  {
__set_errno (ERANGE);
  #if FLT_EVAL_METHOD != 0
volatile
  #endif
FLOAT result = (negative ? -MAX_VALUE : MAX_VALUE) * MAX_VALUE;
return result;
  }

The effect of setting FLT_EVAL_METHOD = 16 for -fexcess-precision=fast (which
is the default) would be to introduce additional "volatile" qualifiers,
which is probably not what the user intends. Elsewhere (quick search of the
debian code archive) the effect is to introduce more complicated assignment
routines or (in Boost's math/tr1.hpp) to define float_t and double_t:

  #if !defined(FLT_EVAL_METHOD)
  typedef float float_t;
  typedef double double_t;
  #elif FLT_EVAL_METHOD == -1
  typedef float float_t;
  typedef double double_t;
  #elif FLT_EVAL_METHOD == 0
  typedef float float_t;
  typedef double double_t;
  #elif FLT_EVAL_METHOD == 1
  typedef double float_t;
  typedef double double_t;
  #else
  typedef long double float_t;
  typedef long double double_t;
  #endif

It seems to me that if I set FLT_EVAL_METHOD = 16, I'm opening the AArch64
port to a number of performance issues in floating-point code if a user
has configured --with-arch=armv8.2-a+fp16 .

On the other hand, it would be unfortunate if _Float16 could not use the
ARMv8.2-A floating-point arithmetic instructions where they were available.

> Then -std=c2x, if C2X integrates TS 18661-3, might not imply the 
> value 0 for such hardware, because the value 16 would also be OK as a 
> standard value in that case.  This can be part of the design issues to 
> address alongside those I mentioned in 
> .

I'm working on these. Right now, I have AArch64 set up as so:

If ARMv8.2-A extensions are available,

  -fexcess-precision=standard -> FLT_EVAL_METHOD = 0
  -fexcess-precision=fast -> FLT_EVAL_METHOD = 16

If ARMv8.2-A extensions are not available,

  -fexcess-precision=standard -> FLT_EVAL_METHOD = 0
  -fexcess-precision=fast -> FLT_EVAL_METHOD = 0

In c-family/c-cppbuiltin.c I've updated cpp_iec_559_value such that it
also allows setting __GEC_IEC_559 if FLT_EVAL_METHOD = 16, and I've updated
c_cpp_builtins to continue to set excess_precision = false if
FLT_EVAL_METHOD == 16. Then I've updated init_excess_precision in toplev.c
with logic to understand that for targets that provide _Float16, we can't
set flag_excess_precision to "fast" when FLT_EVAL_METHOD=0. This is because
FLT_EVAL_METHOD=0 requires predictably using the precision of float for
_Float16 types.
 
In tree.c, I've modified excess_precision_type with the logic discussed
above, promoting _Float16 to float (or wider) if we are required to.

The code needs some cleanup, and I need to decide what to do about the
issue Szabolcs brought up, but I'm hoping to be able to send patches in
the near future for GCC and soft-fp in glibc/libgcc.

Thanks,
James

Re: [PATCH] libstdc++/77334 move assign RB trees of non-copyable types

[Jonathan Wakely]

On 30/08/16 11:17 +0100, Jonathan Wakely wrote:

On 27/08/16 21:29 +0200, François Dumont wrote:

On 23/08/2016 15:17, Jonathan Wakely wrote:

This fixes move assignment of RB trees to tag dispatch so the branch
that copies nodes doesn't need to be well-formed.

  PR libstdc++/77334
  * include/bits/stl_tree.h (_Rb_tree::_M_move_assign): New functions.
  (_Rb_tree::operator=(_Rb_tree&&)): Dispatch to _M_move_assign.
  * testsuite/23_containers/map/77334.cc: New test.

Tested powerpc64le-linux, committed to trunk.

Backports to 5 and 6 to follow.


+operator=(_Rb_tree&& __x)
+noexcept(_Alloc_traits::_S_nothrow_move()
+&& is_nothrow_move_assignable<_Compare>::value)
+{
+  _M_impl._M_key_compare = __x._M_impl._M_key_compare;


Shouldn't it be:

+  _M_impl._M_key_compare = std::move(__x._M_impl._M_key_compare);

I can't find anything in Standard saying to do so but wouldn't it be more 
natural ?


Yes, I can't think of any reason that would cause a problem. The
moved-from container is empty after the move assignment so even if
moving the comparison object changes the order it defines, no elements
will be left in an invalid order.

We don't need to use the comparison object after it's moved, but I'll
add a test to guarantee that remains true.


Done with this patch.

Tested powerpc64le-linux, committed to trunk.

commit 308f80881e5368cc16aa32e3c5c65f341f981329
Author: Jonathan Wakely 
Date:   Tue Aug 30 12:14:29 2016 +0100

Move comparison object in map/set move assignment

	* include/bits/stl_tree.h (_Rb_tree::operator=(_Rb_tree&&)): Move
	comparison object.
	* testsuite/23_containers/set/move_comparison.cc: New test.

diff --git a/libstdc++-v3/include/bits/stl_tree.h b/libstdc++-v3/include/bits/stl_tree.h
index 25580e4..02b00b0 100644
--- a/libstdc++-v3/include/bits/stl_tree.h
+++ b/libstdc++-v3/include/bits/stl_tree.h
@@ -1436,7 +1436,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 noexcept(_Alloc_traits::_S_nothrow_move()
 	 && is_nothrow_move_assignable<_Compare>::value)
 {
-  _M_impl._M_key_compare = __x._M_impl._M_key_compare;
+  _M_impl._M_key_compare = std::move(__x._M_impl._M_key_compare);
   constexpr bool __move_storage =
 	  _Alloc_traits::_S_propagate_on_move_assign()
 	  || _Alloc_traits::_S_always_equal();
diff --git a/libstdc++-v3/testsuite/23_containers/set/move_comparison.cc b/libstdc++-v3/testsuite/23_containers/set/move_comparison.cc
new file mode 100644
index 000..317821f
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/set/move_comparison.cc
@@ -0,0 +1,58 @@
+// Copyright (C) 2016 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+// { dg-do run { target c++11 } }
+
+#include 
+#include 
+#include 
+
+struct Cmp
+{
+  Cmp() = default;
+  Cmp(const Cmp&) = default;
+  Cmp(Cmp&&) = default;
+  Cmp& operator=(const Cmp&) = default;
+  Cmp& operator=(Cmp&& c) { c.moved_from = true; return *this; }
+
+  bool moved_from = false;
+
+  bool operator()(int l, int r) const
+  {
+VERIFY(!moved_from);
+return l < r;
+  }
+};
+
+void
+test01()
+{
+  using allocator = __gnu_test::uneq_allocator;
+  using test_type = std::set;
+  test_type s1(allocator(1)), s2(allocator(2));
+  s1.insert(1);
+  s1.insert(2);
+  s1.insert(3);
+  s2 = std::move(s1);
+  VERIFY(s1.key_comp().moved_from);
+}
+
+int
+main()
+{
+  test01();
+}

[PATCH] Constrain std::shared_ptr assignment and resetting

[Jonathan Wakely]

The standard subtly requires us to constrain the assignment operators,
due to "Effects: Equivalent to [a constrained expression]".

Constraining reset() is not required, but a private discussion started
by Alisdair Meredith has convinced me that it's useful (and apparently
libc++ already does this).

* include/bits/shared_ptr.h (_Assignable): New alias template.
(shared_ptr::operator=(const shared_ptr<_Tp1>&))
(shared_ptr::operator=(shared_ptr<_Tp1>&&))
(shared_ptr::operator=(unique_ptr<_Tp1>&&)): Constrain with
_Assignable.
* include/bits/shared_ptr_base.h (_Assignable): New alias template.
(__shared_ptr::operator=(const __shared_ptr<_Tp1>&))
(__shared_ptr::operator=(__shared_ptr<_Tp1>&&))
(__shared_ptr::operator=(unique_ptr<_Tp1>&&)): Constrain with
_Assignable.
(__shared_ptr::reset(_Tp1*), __shared_ptr::reset(_Tp1*, _Deleter))
(__shared_ptr::reset(_Tp1*, _Deleter, _Alloc)): Constrain with
_Convertible.
* testsuite/20_util/shared_ptr/cons/43820_neg.cc: Change dg-error to
match on any line.
* testsuite/20_util/shared_ptr/cons/void_neg.cc: Likewise.
* testsuite/20_util/shared_ptr/assign/sfinae.cc: New test.
* testsuite/20_util/shared_ptr/assign/shared_ptr_neg.cc: Update
expected errors. Remove unnecessary code.
* testsuite/20_util/shared_ptr/modifiers/reset_sfinae.cc: New test.

Tested powerpc64le-linux, committed to trunk.

commit 1c8d8d7ddb83f6205137e4e62266f7c361158ef3
Author: Jonathan Wakely 
Date:   Wed Aug 31 14:15:38 2016 +0100

Constrain std::shared_ptr assignment and resetting

* include/bits/shared_ptr.h (_Assignable): New alias template.
(shared_ptr::operator=(const shared_ptr<_Tp1>&))
(shared_ptr::operator=(shared_ptr<_Tp1>&&))
(shared_ptr::operator=(unique_ptr<_Tp1>&&)): Constrain with
_Assignable.
* include/bits/shared_ptr_base.h (_Assignable): New alias template.
(__shared_ptr::operator=(const __shared_ptr<_Tp1>&))
(__shared_ptr::operator=(__shared_ptr<_Tp1>&&))
(__shared_ptr::operator=(unique_ptr<_Tp1>&&)): Constrain with
_Assignable.
(__shared_ptr::reset(_Tp1*), __shared_ptr::reset(_Tp1*, _Deleter))
(__shared_ptr::reset(_Tp1*, _Deleter, _Alloc)): Constrain with
_Convertible.
* testsuite/20_util/shared_ptr/cons/43820_neg.cc: Change dg-error to
match on any line.
* testsuite/20_util/shared_ptr/cons/void_neg.cc: Likewise.
* testsuite/20_util/shared_ptr/assign/sfinae.cc: New test.
* testsuite/20_util/shared_ptr/assign/shared_ptr_neg.cc: Update
expected errors. Remove unnecessary code.
* testsuite/20_util/shared_ptr/modifiers/reset_sfinae.cc: New test.

diff --git a/libstdc++-v3/include/bits/shared_ptr.h 
b/libstdc++-v3/include/bits/shared_ptr.h
index 747b09a..b2523b8 100644
--- a/libstdc++-v3/include/bits/shared_ptr.h
+++ b/libstdc++-v3/include/bits/shared_ptr.h
@@ -93,8 +93,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 class shared_ptr : public __shared_ptr<_Tp>
 {
   template
-   using _Convertible
- = typename enable_if::value>::type;
+   using _Convertible = typename
+ enable_if::value>::type;
+
+  template
+   using _Assignable = typename
+ enable_if::value, shared_ptr&>::type;
 
 public:
 
@@ -276,7 +280,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   shared_ptr& operator=(const shared_ptr&) noexcept = default;
 
   template
-   shared_ptr&
+   _Assignable<_Tp1*>
operator=(const shared_ptr<_Tp1>& __r) noexcept
{
  this->__shared_ptr<_Tp>::operator=(__r);
@@ -301,7 +305,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   }
 
   template
-   shared_ptr&
+   _Assignable<_Tp1*>
operator=(shared_ptr<_Tp1>&& __r) noexcept
{
  this->__shared_ptr<_Tp>::operator=(std::move(__r));
@@ -309,7 +313,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
 
   template
-   shared_ptr&
+   _Assignable::pointer>
operator=(std::unique_ptr<_Tp1, _Del>&& __r)
{
  this->__shared_ptr<_Tp>::operator=(std::move(__r));
diff --git a/libstdc++-v3/include/bits/shared_ptr_base.h 
b/libstdc++-v3/include/bits/shared_ptr_base.h
index 60b825c..4ae2668 100644
--- a/libstdc++-v3/include/bits/shared_ptr_base.h
+++ b/libstdc++-v3/include/bits/shared_ptr_base.h
@@ -873,6 +873,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
using _Convertible
  = typename enable_if::value>::type;
 
+  template
+   using _Assignable = typename
+ enable_if::value, __shared_ptr&>::type;
+
 public:
   typedef _Tp   element_type;
 
@@ -983,7 +987,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   constexpr __shared_ptr(nullptr_t) noexcept : __shared_ptr() { }
 
   template
-   __shared_ptr&
+   _Assignable<_Tp1*>
operator=(const __shared_ptr<_

[gomp4] vector reductions

[Nathan Sidwell]

This patch changes the implementation of vector reductions.  Currently we emit a 
sequence of shuffle/op pairs.  The size of the sequence depends on the vector size.


This changes the implementation to emit a loop, the number of iterations of 
which depends on the vector size.  the goal here is a code size reduction (and 
subsequent caching etc).  The tricky bit is that the loop's terminating branch 
must be marked as unified, so the PTX assembler knows it isn't a divergence point.


That's achieved with a new builtin, which simply tags a mov insn with an unspec, 
and then we propagate that marking to the branch instruction.  The mov tagging 
has to be unfortunately early, on the iterator var, rather than on the 
comparison result.  I experimented with BImode ssa vars, but that didn't work. 
Likewise I didn't want to have a builtin that took a label, and hide the looping 
nature from the compiler.


nathan
2016-08-31  Nathan Sidwell  

	* config/nvptx/nvptx.md (cond_uni): New pattern.
	* config/nvptx/nvptx.c (nvptx_propagate_unified): New.
	(nvptx_split_blocks): Call it for cond_uni insn.
	(nvptx_expand_cond_uni): New.
	(enum nvptx_builtins): Add NVPTX_BUILTIN_COND_UNI.
	(nvptx_init_builtins): Initialize it.
	(nvptx_generate_vector_shuffle): Change integral SHIFT operand to
	tree BITS operand.
	(nvptx_vector_reduction): New.
	(nvptx_goacc_reduction_fini): Call it for vector.

Index: config/nvptx/nvptx.c
===
--- config/nvptx/nvptx.c	(revision 239895)
+++ config/nvptx/nvptx.c	(working copy)
@@ -2265,6 +2265,49 @@ nvptx_reorg_subreg (void)
 }
 }
 
+/* UNIFIED is a cond_uni insn.  Find the branch insn it affects, and
+   mark that as unified.  We expect to be in a single block.  */
+
+static void
+nvptx_propagate_unified (rtx_insn *unified)
+{
+  rtx_insn *probe = unified;
+  rtx cond_reg = SET_DEST (PATTERN (unified));
+  rtx pat;
+
+  /* Find the comparison.  (We could skip this and simply scan to he
+ blocks' terminating branch, if we didn't care for self
+ checking.)  */
+  for (;;)
+{
+  probe = NEXT_INSN (probe);
+  pat = PATTERN (probe);
+
+  if (GET_CODE (pat) == SET
+	  && GET_RTX_CLASS (GET_CODE (SET_SRC (pat))) == RTX_COMPARE
+	  && XEXP (SET_SRC (pat), 0) == cond_reg)
+	break;
+  gcc_assert (NONJUMP_INSN_P (probe));
+}
+  rtx pred_reg = SET_DEST (pat);
+
+  /* Find the branch.  */
+  do
+probe = NEXT_INSN (probe);
+  while (!JUMP_P (probe));
+
+  pat = PATTERN (probe);
+  rtx itec = XEXP (SET_SRC (pat), 0);
+  gcc_assert (XEXP (itec, 0) == pred_reg);
+
+  /* Mark the branch's condition as unified.  */
+  rtx unspec = gen_rtx_UNSPEC (BImode, gen_rtvec (1, pred_reg),
+			   UNSPEC_BR_UNIFIED);
+  bool ok = validate_change (probe, &XEXP (itec, 0), unspec, false);
+
+  gcc_assert (ok);
+}
+
 /* Loop structure of the function.  The entire function is described as
a NULL loop.  */
 
@@ -2366,6 +2409,9 @@ nvptx_split_blocks (bb_insn_map_t *map)
 	continue;
 	  switch (recog_memoized (insn))
 	{
+	case CODE_FOR_cond_uni:
+	  nvptx_propagate_unified (insn);
+	  /* FALLTHROUGH */
 	default:
 	  seen_insn = true;
 	  continue;
@@ -4072,6 +4118,21 @@ nvptx_expand_cmp_swap (tree exp, rtx tar
   return target;
 }
 
+/* Expander for the compare unified builtin.  */
+
+static rtx
+nvptx_expand_cond_uni (tree exp, rtx target, machine_mode mode, int ignore)
+{
+  if (ignore)
+return target;
+  
+  rtx src = expand_expr (CALL_EXPR_ARG (exp, 0),
+			 NULL_RTX, mode, EXPAND_NORMAL);
+
+  emit_insn (gen_cond_uni (target, src));
+
+  return target;
+}
 
 /* Codes for all the NVPTX builtins.  */
 enum nvptx_builtins
@@ -4081,6 +4142,7 @@ enum nvptx_builtins
   NVPTX_BUILTIN_WORKER_ADDR,
   NVPTX_BUILTIN_CMP_SWAP,
   NVPTX_BUILTIN_CMP_SWAPLL,
+  NVPTX_BUILTIN_COND_UNI,
   NVPTX_BUILTIN_MAX
 };
 
@@ -4118,6 +4180,7 @@ nvptx_init_builtins (void)
(PTRVOID, ST, UINT, UINT, NULL_TREE));
   DEF (CMP_SWAP, "cmp_swap", (UINT, PTRVOID, UINT, UINT, NULL_TREE));
   DEF (CMP_SWAPLL, "cmp_swapll", (LLUINT, PTRVOID, LLUINT, LLUINT, NULL_TREE));
+  DEF (COND_UNI, "cond_uni", (integer_type_node, integer_type_node, NULL_TREE));
 
 #undef DEF
 #undef ST
@@ -4150,6 +4213,9 @@ nvptx_expand_builtin (tree exp, rtx targ
 case NVPTX_BUILTIN_CMP_SWAPLL:
   return nvptx_expand_cmp_swap (exp, target, mode, ignore);
 
+case NVPTX_BUILTIN_COND_UNI:
+  return nvptx_expand_cond_uni (exp, target, mode, ignore);
+
 default: gcc_unreachable ();
 }
 }
@@ -4303,7 +4369,7 @@ nvptx_get_worker_red_addr (tree type, tr
 
 static void
 nvptx_generate_vector_shuffle (location_t loc,
-			   tree dest_var, tree var, unsigned shift,
+			   tree dest_var, tree var, tree bits,
 			   gimple_seq *seq)
 {
   unsigned fn = NVPTX_BUILTIN_SHUFFLE;
@@ -4326,7 +4392,6 @@ nvptx_generate_vector_shuffle (location_
 }
   
   tree call = nvptx_builtin_decl (fn, true);
-  tree bits = build

Re: Implement C _FloatN, _FloatNx types [version 6]

[Joseph Myers]

On Wed, 31 Aug 2016, James Greenhalgh wrote:

> My concern with this is that the use of comparisons of FLT_EVAL_METHOD
> against 0 that Szabolcs is referring to is common and can have performance
> implications. In glibc for example, 
> 
>   static FLOAT
>   overflow_value (int negative)
>   {
> __set_errno (ERANGE);
>   #if FLT_EVAL_METHOD != 0
> volatile
>   #endif
> FLOAT result = (negative ? -MAX_VALUE : MAX_VALUE) * MAX_VALUE;
> return result;
>   }

Well, it's unavoidable that existing code may misbehave with a new value 
and need updating.  In the glibc case: use of volatile for excess 
precision is the old approach.  That function now (since my 2015-09-23 
patch) uses math_narrow_eval.  And math_narrow_eval is defined using an 
excess_precision macro in math_private.h.  So only the definition of that 
macro based on FLT_EVAL_METHOD needs updating for new values (and 
math_narrow_eval uses __asm__ ("" : "+m" (math_narrow_eval_tmp)); rather 
than volatile, which may be slightly better, though of course you want to 
avoid the load and store to memory in cases where there is no excess 
precision, or where truncating doesn't require a load and store).

(Of course if you want actual TS 18661-3 support for _Float16 in glibc 
that implies a strtof16 function and lots of other *f16 functions which 
also need to know about excess precision for _Float16 in the 
FLT_EVAL_METHOD = 0 case.  Even given the _Float128 support, supporting 
_Float16 in glibc would be a complicated project since no _Float16 
function implementations for glibc presently exist, although many could, 
correctly if suboptimally, wrap float versions.)

> On the other hand, it would be unfortunate if _Float16 could not use the
> ARMv8.2-A floating-point arithmetic instructions where they were available.

In the cases where the two FLT_EVAL_METHOD values are equivalent (e.g. 
where the result is assigned directly back to a _Float16 variable) of 
course you can anyway.

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

match.pd: Revert a * (1 << b) relaxation

[Marc Glisse]

Hello,

this patch was bootstrapped and regtested on 
powerpc64le-unknown-linux-gnu. Already committed, since this is simply 
reverting part of my patch.


--
Marc GlisseIndex: gcc/ChangeLog
===
--- gcc/ChangeLog	(revision 239901)
+++ gcc/ChangeLog	(working copy)
@@ -1,10 +1,15 @@
+2016-08-31  Marc Glisse  
+
+	PR tree-optimization/73714
+	* match.pd (a * (1 << b)): Revert change from 2016-05-23.
+
 2016-08-31  David Malcolm  
 
 	* selftest.c: Move "namespace selftest {" to top of file,
 	removing explicit "selftest::" qualifiers throughout.
 
 2016-08-31  Marc Glisse  
 
 	* config/i386/avx512fintrin.h (__m512_u, __m512i_u, __m512d_u):
 	New types.
 	(_mm512_loadu_pd, _mm512_storeu_pd, _mm512_loadu_ps,
Index: gcc/testsuite/gcc.dg/tree-ssa/pr73714.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/pr73714.c	(revision 0)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr73714.c	(revision 239902)
@@ -0,0 +1,8 @@
+/* { dg-do compile { target lp64 } } */
+/* { dg-options "-O2 -fdump-tree-optimized-raw" } */
+
+int f(int a, int b){
+  return a * (int)(1L << b);
+}
+
+/* { dg-final { scan-tree-dump "mult_expr" "optimized" } } */
Index: gcc/testsuite/ChangeLog
===
--- gcc/testsuite/ChangeLog	(revision 239901)
+++ gcc/testsuite/ChangeLog	(working copy)
@@ -1,10 +1,15 @@
+2016-08-31  Marc Glisse  
+
+	PR tree-optimization/73714
+	* gcc.dg/tree-ssa/pr73714.c: New test.
+
 2016-08-31  Jerry DeLisle  
 
 	PR libgfortran/77393
 	* gfortran.dg/fmt_f0_2.f90: New test.
 
 2016-08-31  Marc Glisse  
 
 	* gcc.target/i386/pr59539-2.c: Adapt options.
 	* gcc.target/i386/avx512f-vmovdqu32-1.c: Relax expected asm.
 
Index: gcc/match.pd
===
--- gcc/match.pd	(revision 239901)
+++ gcc/match.pd	(working copy)
@@ -461,22 +461,21 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 (for ops (conj negate)
  (for cabss (CABS)
   (simplify
(cabss (ops @0))
(cabss @0
 
 /* Fold (a * (1 << b)) into (a << b)  */
 (simplify
  (mult:c @0 (convert? (lshift integer_onep@1 @2)))
   (if (! FLOAT_TYPE_P (type)
-   && (element_precision (type) <= element_precision (TREE_TYPE (@1))
-	   || TYPE_UNSIGNED (TREE_TYPE (@1
+   && tree_nop_conversion_p (type, TREE_TYPE (@1)))
(lshift @0 @2)))
 
 /* Fold (C1/X)*C2 into (C1*C2)/X.  */
 (simplify
  (mult (rdiv@3 REAL_CST@0 @1) REAL_CST@2)
   (if (flag_associative_math
&& single_use (@3))
(with
 { tree tem = const_binop (MULT_EXPR, type, @0, @2); }
 (if (tem)

[patch] Fix segfault in param_change_prob

[Eric Botcazou]

Hi,

the attached Ada testcase triggers a segfault in param_change_prob because the 
parameter is VIEW_CONVERT_EXPR (or 

* ipa-inline-analysis.c (param_change_prob): Strip VIEW_CONVERT_EXPR.


2016-08-31  Eric Botcazou  

* gnat.dg/opt58.adb: New test.
* gnat.dg/opt58_pkg.ads: New helper.

-- 
Eric BotcazouIndex: ipa-inline-analysis.c
===
--- ipa-inline-analysis.c	(revision 239842)
+++ ipa-inline-analysis.c	(working copy)
@@ -2185,9 +2185,13 @@ param_change_prob (gimple *stmt, int i)
   basic_block bb = gimple_bb (stmt);
   tree base;
 
-  /* Global invariants neve change.  */
+  if (TREE_CODE (op) == VIEW_CONVERT_EXPR)
+op = TREE_OPERAND (op, 0);
+
+  /* Global invariants never change.  */
   if (is_gimple_min_invariant (op))
 return 0;
+
   /* We would have to do non-trivial analysis to really work out what
  is the probability of value to change (i.e. when init statement
  is in a sibling loop of the call). 
-- { dg-do compile }
-- { dg-options "-O" }

with Unchecked_Conversion;
with System; use System;
with Opt58_Pkg; use Opt58_Pkg;

procedure Opt58 is

   function Convert is new Unchecked_Conversion (Integer, Rec);

   Dword : Integer := 0;
   I : Small_Int := F1 (Convert (Dword));

begin
   if F2 (Null_Address, I = 0) then
  null;
   end if;
end Opt58;
with System; use System;

package Opt58_Pkg is

   pragma Pure (Opt58_Pkg);

   type Small_Int is range 0 .. 255;

   type Rec is record
 D1, D2, D3, D4 : Small_Int;
   end record;
   pragma Pack (Rec);
   for Rec'Size use 32;

   function F1 (R : Rec) return Small_Int;

   function F2 (A : Address; B : Boolean) return Boolean;

end Opt58_Pkg;

[committed] Fix ICE with !$omp atomic (PR fortran/77374)

[Jakub Jelinek]

Hi!

The resolve_omp_atomic code relied on gfc_resolve_blocks not actually
changing the kinds and number of statements, which is apparently no longer
the case, there are various changes where those can change, e.g. after
diagnosing an error EXEC_ASSIGN can be changed into EXEC_NOP, or for the F08
fn(arg) = val
where fn returns pointer.  I've committed following patch after
bootstrapping/regtesting it on x86_64-linux and i686-linux, which moves the
assertions earlier (before gfc_resolve_blocks is done in the nested stmts)
and tweak resolve_omp_atomic so that instead of assertions it either returns
early or diagnoses an error.

2016-08-31  Jakub Jelinek  

PR fortran/77374
* parse.c (parse_omp_oacc_atomic): Copy over cp->ext.omp_atomic
to cp->block->ext.omp_atomic.
* resolve.c (gfc_resolve_blocks): Assert block with one or two
EXEC_ASSIGNs for EXEC_*_ATOMIC.
* openmp.c (resolve_omp_atomic): Don't assert one or two
EXEC_ASSIGNs, instead return quietly for EXEC_NOPs and otherwise
error unexpected statements.

* gfortran.dg/gomp/pr77374.f08: New test.

--- gcc/fortran/parse.c.jj  2016-08-29 12:17:09.0 +0200
+++ gcc/fortran/parse.c 2016-08-30 16:57:16.982107686 +0200
@@ -4695,6 +4695,7 @@ parse_omp_oacc_atomic (bool omp_p)
   np = new_level (cp);
   np->op = cp->op;
   np->block = NULL;
+  np->ext.omp_atomic = cp->ext.omp_atomic;
   count = 1 + ((cp->ext.omp_atomic & GFC_OMP_ATOMIC_MASK)
   == GFC_OMP_ATOMIC_CAPTURE);
 
--- gcc/fortran/resolve.c.jj2016-08-29 12:17:09.0 +0200
+++ gcc/fortran/resolve.c   2016-08-30 17:18:09.607225924 +0200
@@ -9464,6 +9464,24 @@ gfc_resolve_blocks (gfc_code *b, gfc_nam
case EXEC_WAIT:
  break;
 
+   case EXEC_OMP_ATOMIC:
+   case EXEC_OACC_ATOMIC:
+ {
+   gfc_omp_atomic_op aop
+ = (gfc_omp_atomic_op) (b->ext.omp_atomic & GFC_OMP_ATOMIC_MASK);
+
+   /* Verify this before calling gfc_resolve_code, which might
+  change it.  */
+   gcc_assert (b->next && b->next->op == EXEC_ASSIGN);
+   gcc_assert (((aop != GFC_OMP_ATOMIC_CAPTURE)
+&& b->next->next == NULL)
+   || ((aop == GFC_OMP_ATOMIC_CAPTURE)
+   && b->next->next != NULL
+   && b->next->next->op == EXEC_ASSIGN
+   && b->next->next->next == NULL));
+ }
+ break;
+
case EXEC_OACC_PARALLEL_LOOP:
case EXEC_OACC_PARALLEL:
case EXEC_OACC_KERNELS_LOOP:
@@ -9476,9 +9494,7 @@ gfc_resolve_blocks (gfc_code *b, gfc_nam
case EXEC_OACC_CACHE:
case EXEC_OACC_ENTER_DATA:
case EXEC_OACC_EXIT_DATA:
-   case EXEC_OACC_ATOMIC:
case EXEC_OACC_ROUTINE:
-   case EXEC_OMP_ATOMIC:
case EXEC_OMP_CRITICAL:
case EXEC_OMP_DISTRIBUTE:
case EXEC_OMP_DISTRIBUTE_PARALLEL_DO:
--- gcc/fortran/openmp.c.jj 2016-08-15 10:13:26.0 +0200
+++ gcc/fortran/openmp.c2016-08-30 17:40:57.241654954 +0200
@@ -3946,12 +3946,33 @@ resolve_omp_atomic (gfc_code *code)
 = (gfc_omp_atomic_op) (atomic_code->ext.omp_atomic & GFC_OMP_ATOMIC_MASK);
 
   code = code->block->next;
-  gcc_assert (code->op == EXEC_ASSIGN);
-  gcc_assert (((aop != GFC_OMP_ATOMIC_CAPTURE) && code->next == NULL)
- || ((aop == GFC_OMP_ATOMIC_CAPTURE)
- && code->next != NULL
- && code->next->op == EXEC_ASSIGN
- && code->next->next == NULL));
+  /* resolve_blocks asserts this is initially EXEC_ASSIGN.
+ If it changed to EXEC_NOP, assume an error has been emitted already.  */
+  if (code->op == EXEC_NOP)
+return;
+  if (code->op != EXEC_ASSIGN)
+{
+unexpected:
+  gfc_error ("unexpected !$OMP ATOMIC expression at %L", &code->loc);
+  return;
+}
+  if (aop != GFC_OMP_ATOMIC_CAPTURE)
+{
+  if (code->next != NULL)
+   goto unexpected;
+}
+  else
+{
+  if (code->next == NULL)
+   goto unexpected;
+  if (code->next->op == EXEC_NOP)
+   return;
+  if (code->next->op != EXEC_ASSIGN || code->next->next)
+   {
+ code = code->next;
+ goto unexpected;
+   }
+}
 
   if (code->expr1->expr_type != EXPR_VARIABLE
   || code->expr1->symtree == NULL
--- gcc/testsuite/gfortran.dg/gomp/pr77374.f08.jj   2016-08-30 
17:42:25.168591066 +0200
+++ gcc/testsuite/gfortran.dg/gomp/pr77374.f08  2016-08-30 17:54:12.961042180 
+0200
@@ -0,0 +1,21 @@
+! PR fortran/77374
+! { dg-do compile }
+
+subroutine foo (a, b)
+  integer :: a, b
+!$omp atomic
+  b = b + a
+!$omp atomic
+  z(1) = z(1) + 1  ! { dg-error "must have the pointer attribute" }
+end subroutine
+subroutine bar (a, b)
+  integer :: a, b
+  interface
+function baz (i) result (res)
+  integer, pointer :: res
+  integer :: i
+end function
+  end interface
+!$omp atomic
+  ba

[committed] Fix ICE with !$omp parallel workshare (PR fortran/77352)

[Jakub Jelinek]

Hi!

This is something I've fixed recently for a couple of construct, but left
parallel workshare untouched.  Bootstrapped/regtested on x86_64-linux and
i686-linux, committed to trunk.

2016-08-31  Jakub Jelinek  

PR fortran/77352
* trans-openmp.c (gfc_trans_omp_parallel_workshare): Always add a
BIND_EXPR with BLOCK around what gfc_trans_omp_workshare returns.

* gfortran.dg/gomp/pr77352.f90: New test.

--- gcc/fortran/trans-openmp.c.jj   2016-08-19 17:27:03.0 +0200
+++ gcc/fortran/trans-openmp.c  2016-08-31 10:00:54.378338571 +0200
@@ -4001,10 +4001,7 @@ gfc_trans_omp_parallel_workshare (gfc_co
   code->loc);
   pushlevel ();
   stmt = gfc_trans_omp_workshare (code, &workshare_clauses);
-  if (TREE_CODE (stmt) != BIND_EXPR)
-stmt = build3_v (BIND_EXPR, NULL, stmt, poplevel (1, 0));
-  else
-poplevel (0, 0);
+  stmt = build3_v (BIND_EXPR, NULL, stmt, poplevel (1, 0));
   stmt = build2_loc (input_location, OMP_PARALLEL, void_type_node, stmt,
 omp_clauses);
   OMP_PARALLEL_COMBINED (stmt) = 1;
--- gcc/testsuite/gfortran.dg/gomp/pr77352.f90.jj   2016-08-31 
10:04:19.385733738 +0200
+++ gcc/testsuite/gfortran.dg/gomp/pr77352.f90  2016-08-31 10:05:48.744602500 
+0200
@@ -0,0 +1,16 @@
+! PR fortran/77352
+! { dg-do compile }
+! { dg-additional-options "-fstack-arrays -O2" }
+! { dg-additional-options "-fopenacc" { target fopenacc } }
+
+program pr77352
+  real, allocatable :: a(:,:), b(:)
+  integer :: m, n
+  m = 4
+  n = 2
+  allocate (a(m,n), b(m))
+  a = 1.0
+!$omp parallel workshare
+  b(:) = [ sum(a, dim=1) ]
+!$omp end parallel workshare
+end

Jakub

Re: [doc] doc/install.texi: www.opencsw.org now uses https.

[Gerald Pfeifer]

On Mon, 22 Aug 2016, Gerald Pfeifer wrote:
> 2016-08-22  Gerald Pfeifer  
> 
>   * doc/install.texi (Binaries): www.opencsw.org now uses https.
> 
> Applied.
> 
> I'll backport to GCC 6 as well, once Richi has unfrozen the branch.

Done.

Gerald

[committed] diagnostic-show-locus.c: handle fixits on lines outside the regular ranges

[David Malcolm]

The diagnostic_show_locus implementation determines the set
of line spans that need printing based on the ranges within the
rich_location (in layout::calculate_line_spans).

Currently this doesn't take into account fix-it hints, and hence
we fail to print fix-it hints that are on lines outside of
those ranges.

This patch updates the implementation to take fix-it hints into
account when calculating the pertinent line spans, so that such fix-it
hints do get printed.  It also adds some validation, to ensure that
we don't attempt to print fix-its hints affecting a different source
file.

Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.

Committed to trunk as r239906.

gcc/ChangeLog:
* diagnostic-show-locus.c (class layout): Add field m_fixit_hints.
(layout_range::intersects_line_p): New method.
(test_range_contains_point_for_single_point): Rename to...
(test_layout_range_for_single_point): ...this, and add testing
for layout_range::intersects_line_p.
(test_range_contains_point_for_single_line): Rename to...
(test_layout_range_for_single_line): ...this,  and add testing
for layout_range::intersects_line_p.
(test_range_contains_point_for_multiple_lines): Rename to...
(test_layout_range_for_multiple_lines): ...this,  and add testing
for layout_range::intersects_line_p.
(layout::layout): Populate m_fixit_hints.
(layout::get_expanded_location): Handle the case of a line-span
for a fix-it hint.
(layout::validate_fixit_hint_p): New method.
(get_line_span_for_fixit_hint): New function.
(layout::calculate_line_spans): Add spans for fixit-hints.
(layout::should_print_annotation_line_p): New method.
(layout::print_any_fixits): Drop param "richloc", instead using
validated fixits in m_fixit_hints.  Add "const" to hint pointers.
(diagnostic_show_locus): Avoid printing blank annotation lines.
(selftest::test_diagnostic_context::test_diagnostic_context):
Initialize show_column and start_span.
(selftest::test_diagnostic_context::start_span_cb): New static
function.
(selftest::test_diagnostic_show_locus_fixit_lines): New function.
(selftest::diagnostic_show_locus_c_tests): Update for function
renamings.  Call test_diagnostic_show_locus_fixit_lines.

libcpp/ChangeLog:
* include/line-map.h (class fixit_remove): Remove stray decl.
(fixit_hint::affects_line_p): Make const.
(fixit_insert::affects_line_p): Likewise.
(fixit_replace::affects_line_p): Likewise.
* line-map.c (fixit_insert::affects_line_p): Likewise.
(fixit_replace::affects_line_p): Likewise.
---
 gcc/diagnostic-show-locus.c | 292 
 libcpp/include/line-map.h   |   7 +-
 libcpp/line-map.c   |   4 +-
 3 files changed, 275 insertions(+), 28 deletions(-)

diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
index a22a660..00a95a1 100644
--- a/gcc/diagnostic-show-locus.c
+++ b/gcc/diagnostic-show-locus.c
@@ -129,6 +129,7 @@ class layout_range
const expanded_location *caret_exploc);
 
   bool contains_point (int row, int column) const;
+  bool intersects_line_p (int row) const;
 
   layout_point m_start;
   layout_point m_finish;
@@ -203,14 +204,17 @@ class layout
   expanded_location get_expanded_location (const line_span *) const;
 
   bool print_source_line (int row, line_bounds *lbounds_out);
+  bool should_print_annotation_line_p (int row) const;
   void print_annotation_line (int row, const line_bounds lbounds);
   bool annotation_line_showed_range_p (int line, int start_column,
   int finish_column) const;
-  void print_any_fixits (int row, const rich_location *richloc);
+  void print_any_fixits (int row);
 
   void show_ruler (int max_column) const;
 
  private:
+  bool validate_fixit_hint_p (const fixit_hint *hint);
+
   void calculate_line_spans ();
 
   void print_newline ();
@@ -237,6 +241,7 @@ class layout
   colorizer m_colorizer;
   bool m_colorize_source_p;
   auto_vec  m_layout_ranges;
+  auto_vec  m_fixit_hints;
   auto_vec  m_line_spans;
   int m_x_offset;
 };
@@ -460,9 +465,22 @@ layout_range::contains_point (int row, int column) const
   return column <= m_finish.m_column;
 }
 
+/* Does this layout_range contain any part of line ROW?  */
+
+bool
+layout_range::intersects_line_p (int row) const
+{
+  gcc_assert (m_start.m_line <= m_finish.m_line);
+  if (row < m_start.m_line)
+return false;
+  if (row > m_finish.m_line)
+return false;
+  return true;
+}
+
 #if CHECKING_P
 
-/* A helper function for testing layout_range::contains_point.  */
+/* A helper function for testing layout_range.  */
 
 static layout_range
 make_range (int start_line, int start_col, int end_line, int end_col)
@@ -475,16 +493,19 @@ make_range (int start_line, int start_col, int en

[PATCH] Fix up -fsanitize=address ctor order (PR sanitizer/77396)

[Jakub Jelinek]

Hi!

libasan currently has an assertion that __asan_before_dynamic_init
is called only after registering at least one global var.  I have no idea
how to always guarantee that without making the code too ugly or registering
dummy global vars, since the registration of globals is done at the end of
file compilation when all functions are already emitted, while the
before/after dynamic init pair needs to wrap the actual constructors in the
static initializers (also with a different priority), and when compiling
that function we don't know yet if all the globals will be optimized away or
not.  So, I'll work with upstream to tweak that.

That said, this patch tweaks at least the case where everything in the
static ctors function in between the wrapping function is optimized away,
plus makes sure that we ignore those builtins when checking if a function
is const/pure (if there are no memory accesses (direct or indirect) in
between those two, the function have no real effect).  If there are no
global ctors, we don't emit the static ctors function at all, if there are,
usually at least one global remains, and the case this patch solves is for
the case where all can be optimized away.  The only case which still fails
is when all the ctors are inlined and don't in the end reference any of the
globals they are constructing, just have some other side-effect.

Also, I've noticed that for non-__cxa_atexit compilation we emit the
__asan_*_dynamic_init calls even around dtors, which is undesirable.

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

2016-08-31  Jakub Jelinek  

PR sanitizer/77396
* sanopt.c (sanopt_optimize_walker): Optimize away
__asan_before_dynamic_init (...) immediately followed by
__asan_after_dynamic_init ().
* ipa-pure-const.c (special_builtin_state): Handle
BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT and
BUILT_IN_ASAN_AFTER_DYNAMIC_INIT.

* decl2.c (do_static_initialization_or_destruction): Only
call asan_dynamic_init_call if INITP is true.

* g++.dg/asan/pr77396.C: New test.

--- gcc/sanopt.c.jj 2016-08-19 11:04:33.0 +0200
+++ gcc/sanopt.c2016-08-30 12:15:53.466703044 +0200
@@ -538,6 +538,28 @@ sanopt_optimize_walker (basic_block bb,
   if (asan_check_optimize && !nonfreeing_call_p (stmt))
info->freeing_call_events++;
 
+  /* If __asan_before_dynamic_init ("module"); is followed immediately
+by __asan_after_dynamic_init ();, there is nothing to guard, so
+optimize both away.  */
+  if (asan_check_optimize
+ && gimple_call_builtin_p (stmt, BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT))
+   {
+ gimple_stmt_iterator gsi2 = gsi;
+ gsi_next_nondebug (&gsi2);
+ if (!gsi_end_p (gsi2))
+   {
+ gimple *g = gsi_stmt (gsi2);
+ if (is_gimple_call (g)
+ && gimple_call_builtin_p (g,
+   BUILT_IN_ASAN_AFTER_DYNAMIC_INIT))
+   {
+ unlink_stmt_vdef (g);
+ gsi_remove (&gsi2, true);
+ remove = true;
+   }
+   }
+   }
+
   if (gimple_call_internal_p (stmt))
switch (gimple_call_internal_fn (stmt))
  {
--- gcc/ipa-pure-const.c.jj 2016-05-20 09:05:08.0 +0200
+++ gcc/ipa-pure-const.c2016-08-30 13:14:34.134647275 +0200
@@ -508,6 +508,8 @@ special_builtin_state (enum pure_const_s
case BUILT_IN_FRAME_ADDRESS:
case BUILT_IN_APPLY:
case BUILT_IN_APPLY_ARGS:
+   case BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT:
+   case BUILT_IN_ASAN_AFTER_DYNAMIC_INIT:
  *looping = false;
  *state = IPA_CONST;
  return true;
--- gcc/cp/decl2.c.jj   2016-08-10 00:21:07.0 +0200
+++ gcc/cp/decl2.c  2016-08-30 13:21:30.416518361 +0200
@@ -3861,7 +3861,7 @@ do_static_initialization_or_destruction
  in other compilation units, or at least those that haven't been
  initialized yet.  Variables that need dynamic construction in
  the current compilation unit are kept accessible.  */
-  if (flag_sanitize & SANITIZE_ADDRESS)
+  if (initp && (flag_sanitize & SANITIZE_ADDRESS))
 finish_expr_stmt (asan_dynamic_init_call (/*after_p=*/false));
 
   node = vars;
@@ -3914,7 +3914,7 @@ do_static_initialization_or_destruction
 
   /* Revert what __asan_before_dynamic_init did by calling
  __asan_after_dynamic_init.  */
-  if (flag_sanitize & SANITIZE_ADDRESS)
+  if (initp && (flag_sanitize & SANITIZE_ADDRESS))
 finish_expr_stmt (asan_dynamic_init_call (/*after_p=*/true));
 
   /* Finish up the init/destruct if-stmt body.  */
--- gcc/testsuite/g++.dg/asan/pr77396.C.jj  2016-08-30 12:31:43.357025231 
+0200
+++ gcc/testsuite/g++.dg/asan/pr77396.C 2016-08-30 13:16:56.096898220 +0200
@@ -0,0 +1,12 @@
+// PR sanitizer/77396
+// { dg-do run }
+// { dg-set-target-env-var ASAN_OPTIONS "check_initialization_orde

Re: [x86] Disable STV pass if -mstackrealign is enabled.

[Uros Bizjak]

> the new STV pass generates SSE instructions in 32-bit mode very late in the
> pipeline and doesn't bother about realigning the stack, so it wreaks havoc on
> OSes where you need to realign the stack, e.g. Windows, but I guess Solaris is
> equally affected.  Therefore the attached patch disables it if -mstackrealign
> is enabled (the option is automatically enabled on Windows and Solaris when
> SSE support is enabled), as already done for -mpreferred-stack-boundary={2,3}
> and -mincoming-stack-boundary={2,3}.
>
> Tested on x86/Windows, OK for mainline and 6 branch?
>
>
> 2016-08-31  Eric Botcazou  
>
>* config/i386/i386.c (ix86_option_override_internal): Also disable the
>STV pass if -mstackrealign is enabled.

OK for mainline and gcc-6 branch.

Thanks,
Uros.

[PATCH, AVX-512, committed] Fix detection of AVX512IFMA in host_detect_local_cpu

[Ilya Verbin]

Hi!

I've committed this patch as obvious.


gcc/
* config/i386/driver-i386.c (host_detect_local_cpu): Fix detection of
AVX512IFMA.


Index: gcc/config/i386/driver-i386.c
===
--- gcc/config/i386/driver-i386.c (revision 239907)
+++ gcc/config/i386/driver-i386.c (working copy)
@@ -498,7 +498,7 @@
   has_avx512dq = ebx & bit_AVX512DQ;
   has_avx512bw = ebx & bit_AVX512BW;
   has_avx512vl = ebx & bit_AVX512VL;
-  has_avx512vl = ebx & bit_AVX512IFMA;
+  has_avx512ifma = ebx & bit_AVX512IFMA;

   has_prefetchwt1 = ecx & bit_PREFETCHWT1;
   has_avx512vbmi = ecx & bit_AVX512VBMI;


  -- Ilya

Re: [PATCH, rs6000] Fix PR72827 (ada bootstrap failure)

[Bill Schmidt]

> On Aug 31, 2016, at 9:00 AM, Bill Schmidt  wrote:
> 
> 
> On 8/31/16 1:19 AM, Segher Boessenkool wrote:
>>> 
>> If (say) base=r1 offset=r0 this will now adjust r1?  That cannot be good.
> Mm, yeah, that wasn't well-thought.  Was thinking 0, not r0.  Will have
> to avoid
> that.

Unfortunately this kind of patch won't be able to address the problem.  If
we have a fixed base register (stack or frame pointer) and the offset register
has been assigned to r0, then the offset register is illegal to use in
replace_equiv_address.  As far as I can see, we can only fix this by making
a change in secondary reload so that we have a scratch register available.
Volunteers? :)

Bill

Re: PR35503 - warn for restrict pointer

[Fabien Chêne]

2016-08-30 17:34 GMT+02:00 Prathamesh Kulkarni :
> On 30 August 2016 at 20:24, Tom de Vries  wrote:
>> On 26/08/16 13:39, Prathamesh Kulkarni wrote:
>>>
>>> Hi,
>>> The following patch adds option -Wrestrict that warns when an argument
>>> is passed to a restrict qualified parameter and it aliases with
>>> another argument.
>>>
>>> eg:
>>> int foo (const char *__restrict buf, const char *__restrict fmt, ...);
>>>
>>> void f(void)
>>> {
>>>   char buf[100] = "hello";
>>>   foo (buf, "%s-%s", buf, "world");
>>> }
>>
>>
>> Does -Wrestrict generate a warning for this example?
>>
>> ...
>> void h(int n, int * restrict p, int * restrict q, int * restrict r)
>> {
>>   int i;
>>   for (i = 0; i < n; i++)
>> p[i] = q[i] + r[i];
>> }
>>
>> h (100, a, b, b)
>> ...
>>
>> [ Note that this is valid C, and does not violate the restrict definition. ]
>>
>> If -Wrestrict indeed generates a warning, then we should be explicit in the
>> documentation that while the warning triggers on this type of example, the
>> code is correct.
> I am afraid it would warn for the above case. The patch just checks if
> the parameter is qualified
> with restrict, and if the corresponding argument has aliases in the
> call (by calling operand_equal_p).
> Is such code common in practice ? If it is, I wonder if we should keep
> the warning in -Wall ?
>
> To avoid such false positives, we would need to track which arguments
> are modified by the call.
> I suppose that could be done with ipa mod/ref analysis (and moving the
> warning to middle-end) ?
> I tried looking into gcc sources but couldn't find where it's implemented.

I don't know either which pass can be used, but I guess it should be doable.
If so, a first step would be to determine useless restrict-qualified
args (and possibly warn for them), and then perform the current
-Wrestrict analysis only on non-useless restrict-qualified args ?

--
Fabien

[PATCH] PR target/77408: Add -mcopy-reloc to avoid copy relocation

[H.J. Lu]

On x86, copy relocation is used in executable to access external data
defined in shared object as if it is defined locally. At run-time,
dynamic linker copies symbol data from shared object to executable and
its references from shared objects are resolved by GLOB_DAT relocation.
Since the copy of symbol data in executable is writable even if the
original symbol in shared object is read-only, this is a potential
security risk.

We can avoid copy relocation by always using PIC model to access
external data symbol.  If the external symbol is defined locally in
executable, linker can optimize instructions on memory operand with
GOTPCRELX/GOT32X relocation against external symbol into a different
form on immediate operand.

Without copy relocation, external symbols in executable are accessed
indirectly via the GOT slot. Even when the symbol is defined locally,
it is still referenced indirectly through a register. However, there
are a few side benefits:

1. Without copy relocation in executable for external symbol defined
in shared object, symbol size can be increased while still providing
backward binary compatibility.
2. Since protected symbols in shared objects are no longer copied to
executable at run-time, references of protected symbols within shared
objects can be resolved without GOT.

Tested on i686 and x86-64.  OK for master?


H.J.
---
gcc/

PR target/77408
* config/i386/i386.c (ix86_force_load_from_GOT_p): Return
true on data symbols for -mno-copy-reloc.
(legitimate_pic_address_disp_p): Allow copy relocation in
64-bit PIE only with -mcopy-reloc.
* config/i386/i386.opt (mcopy-reloc): New.
* doc/invoke.texi: Document -mcopy-reloc.

gcc/testsuite/

PR target/77408
* gcc.target/i386/no-copy-reloc-1.c: New test.
* gcc.target/i386/no-copy-reloc-2.c: Likewise.
* gcc.target/i386/no-copy-reloc-3.c: Likewise.
* gcc.target/i386/no-copy-reloc-4.c: Likewise.
* gcc.target/i386/no-copy-reloc-5.c: Likewise.
* gcc.target/i386/no-copy-reloc-6.c: Likewise.
* gcc.target/i386/no-copy-reloc-7.c: Likewise.
* gcc.target/i386/no-copy-reloc-8.c: Likewise.
* gcc.target/i386/no-copy-reloc-9.c: Likewise.
---
 gcc/config/i386/i386.c  | 22 +++---
 gcc/config/i386/i386.opt|  4 
 gcc/doc/invoke.texi |  7 ++-
 gcc/testsuite/gcc.target/i386/no-copy-reloc-1.c | 15 +++
 gcc/testsuite/gcc.target/i386/no-copy-reloc-2.c | 16 
 gcc/testsuite/gcc.target/i386/no-copy-reloc-3.c | 14 ++
 gcc/testsuite/gcc.target/i386/no-copy-reloc-4.c | 14 ++
 gcc/testsuite/gcc.target/i386/no-copy-reloc-5.c | 15 +++
 gcc/testsuite/gcc.target/i386/no-copy-reloc-6.c | 14 ++
 gcc/testsuite/gcc.target/i386/no-copy-reloc-7.c | 14 ++
 gcc/testsuite/gcc.target/i386/no-copy-reloc-8.c | 12 
 gcc/testsuite/gcc.target/i386/no-copy-reloc-9.c | 12 
 12 files changed, 151 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/no-copy-reloc-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/no-copy-reloc-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/no-copy-reloc-3.c
 create mode 100644 gcc/testsuite/gcc.target/i386/no-copy-reloc-4.c
 create mode 100644 gcc/testsuite/gcc.target/i386/no-copy-reloc-5.c
 create mode 100644 gcc/testsuite/gcc.target/i386/no-copy-reloc-6.c
 create mode 100644 gcc/testsuite/gcc.target/i386/no-copy-reloc-7.c
 create mode 100644 gcc/testsuite/gcc.target/i386/no-copy-reloc-8.c
 create mode 100644 gcc/testsuite/gcc.target/i386/no-copy-reloc-9.c

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index a229a73..c84a351 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -15172,13 +15172,20 @@ darwin_local_data_pic (rtx disp)
 bool
 ix86_force_load_from_GOT_p (rtx x)
 {
-  return ((TARGET_64BIT || HAVE_AS_IX86_GOT32X)
- && !TARGET_PECOFF && !TARGET_MACHO
- && !flag_plt && !flag_pic
- && ix86_cmodel != CM_LARGE
- && GET_CODE (x) == SYMBOL_REF
- && SYMBOL_REF_FUNCTION_P (x)
- && !SYMBOL_REF_LOCAL_P (x));
+  if ((TARGET_64BIT || HAVE_AS_IX86_GOT32X)
+  && !TARGET_PECOFF
+  && !flag_pic
+  && !TARGET_MACHO
+  && ix86_cmodel != CM_LARGE
+  && GET_CODE (x) == SYMBOL_REF
+  && !SYMBOL_REF_LOCAL_P (x))
+{
+  if (SYMBOL_REF_FUNCTION_P (x))
+   return !flag_plt;
+  else
+   return !flag_copy_reloc;
+}
+  return false;
 }
 
 /* Determine if a given RTX is a valid constant.  We already know this
@@ -15439,6 +15446,7 @@ legitimate_pic_address_disp_p (rtx disp)
  else if (!SYMBOL_REF_FAR_ADDR_P (op0)
   && (SYMBOL_REF_LOCAL_P (op0)
   || (HAVE_LD_PIE_COPYRELOC
+  && flag_copy_reloc

Re: [MPX] Fix for PR77267

[Ilya Enkovich]

2016-08-31 16:37 GMT+03:00 Alexander Ivchenko :
> 2016-08-31 12:18 GMT+03:00 Ilya Enkovich :
>> 2016-08-30 21:53 GMT+03:00 Alexander Ivchenko :
>>> Would something like that count?
>>>
>>> I did not do the warning thing, cause the problem only appears when
>>> you provide the -Wl,-as-needed option to the linker.
>>> (In all other cases warning would be redundant). Are we able to check
>>> that on runtime?
>>>
>>>
>>> diff --git a/gcc/config.in b/gcc/config.in
>>> index fc3321c..a736de3 100644
>>> --- a/gcc/config.in
>>> +++ b/gcc/config.in
>>> @@ -1538,6 +1538,12 @@
>>>  #endif
>>>
>>>
>>> +/* Define if your linker supports --push-state/--pop-state */
>>> +#ifndef USED_FOR_TARGET
>>> +#undef HAVE_LD_PUSHPOPSTATE_SUPPORT
>>> +#endif
>>> +
>>> +
>>>  /* Define if your linker links a mix of read-only and read-write sections 
>>> into
>>> a read-write section. */
>>>  #ifndef USED_FOR_TARGET
>>> diff --git a/gcc/config/i386/linux-common.h b/gcc/config/i386/linux-common.h
>>> index 4b9910f..6aa195d 100644
>>> --- a/gcc/config/i386/linux-common.h
>>> +++ b/gcc/config/i386/linux-common.h
>>> @@ -79,13 +79,23 @@ along with GCC; see the file COPYING3.  If not see
>>>  #endif
>>>  #endif
>>>
>>> +#ifdef HAVE_LD_PUSHPOPSTATE_SUPPORT
>>> +#define MPX_LD_AS_NEEDED_GUARD_PUSH "--push-state --no-as-needed"
>>> +#define MPX_LD_AS_NEEDED_GUARD_POP "--pop-state"
>>> +#else
>>> +#define MPX_LD_AS_NEEDED_GUARD_PUSH ""
>>> +#define MPX_LD_AS_NEEDED_GUARD_POP ""
>>> +#endif
>>> +
>>>  #ifndef LIBMPX_SPEC
>>>  #if defined(HAVE_LD_STATIC_DYNAMIC)
>>>  #define LIBMPX_SPEC "\
>>>  %{mmpx:%{fcheck-pointer-bounds:\
>>>  %{static:--whole-archive -lmpx --no-whole-archive" LIBMPX_LIBS "}\
>>>  %{!static:%{static-libmpx:" LD_STATIC_OPTION " --whole-archive}\
>>> --lmpx %{static-libmpx:--no-whole-archive " LD_DYNAMIC_OPTION \
>>> +" MPX_LD_AS_NEEDED_GUARD_PUSH  " -lmpx " MPX_LD_AS_NEEDED_GUARD_POP "\
>>> +%{static-libmpx:--no-whole-archive "\
>>> +LD_DYNAMIC_OPTION \
>>
>> Looks like you add guards for both static-libmpx and dynamic linking cases.
>> You shouldn't need it for static-libmpx case.
>
> Are you sure about that? May be I missed something, buy when I do
>> gcc out-of-bounds.c -mmpx -fcheck-pointer-bounds -static -###
>
> I got: ... -whole-archive -lmpx --no-whole-archive -lpthread
> -lmpxwrappers --start-group -lgcc -lgcc_eh -lc --end-group ...
> So no guards for static case. Could you clarify?
>

When you use -static or -static-libmpx then MPX runtime static library
should be linked
in a similar way. In your current version you don't have guards for
-static but have them
for -static-libmpx. So I propose to use"

%{!static-libmpx:" MPX_LD_AS_NEEDED_GUARD_PUSH " ...
%{!static-libmpx:" MPX_LD_AS_NEEDED_GUARD_POP " ...

>
>
>>>  LIBMPX_LIBS ""
>>>  #else
>>>  #define LIBMPX_SPEC "\
>>> @@ -99,7 +109,8 @@ along with GCC; see the file COPYING3.  If not see
>>>  %{mmpx:%{fcheck-pointer-bounds:%{!fno-chkp-use-wrappers:\
>>>  %{static:-lmpxwrappers}\
>>>  %{!static:%{static-libmpxwrappers:" LD_STATIC_OPTION " 
>>> --whole-archive}\
>>> --lmpxwrappers %{static-libmpxwrappers:--no-whole-archive "\
>>> +" MPX_LD_AS_NEEDED_GUARD_PUSH " -lmpxwrappers "
>>> MPX_LD_AS_NEEDED_GUARD_POP "\
>>> +%{static-libmpxwrappers:--no-whole-archive "\
>>>  LD_DYNAMIC_OPTION "}"
>>
>> I believe wrappers should work fine with --as-needed and don't need
>> this guard. Otherwise looks OK.
>
> wrappers also are not linked to the binary with -as-needed. So if I
> remove guards from libmpxwrappers:

MPX runtime library has to be linked in even if we don't use any
symbol from it due
to its static constructors used to initilize MPX. For wrappers we
actually don't need to
link it if we don't have any wrapper used. I think you should see
wrappers linked
with --as-needed if you actually use some wrapped function in your test code.

BTW looks like we have wrong '--whole-archive' for -static-libmpxwrappers case.
There is inconsistency because we don't use --whole-archive when just
-static is passed.

Ilya

>>> gcc out-of-bounds.c -mmpx -fcheck-pointer-bounds -Wl,-as-needed -###
> ... -as-needed --push-state --no-as-needed -lmpx --pop-state
> -lmpxwrappers -z bndplt -lgcc --as-needed -lgcc_s --no-as-needed -lc
> -lgcc --as-needed -lgcc_ ...
>
>> ldd a.out
> linux-vdso.so.1 (0x7ffd987cf000)
> libmpx.so.2 => not found
> libc.so.6 => /lib64/libc.so.6 (0x7f0f27bf3000)
> /lib64/ld-linux-x86-64.so.2 (0x55bfba052000)
>
>
>
>> Ilya
>>
>>>  #else
>>>  #define LIBMPXWRAPPERS_SPEC "\
>>> diff --git a/gcc/configure b/gcc/configure
>>> index 871ed0c..094 100755
>>> --- a/gcc/configure
>>> +++ b/gcc/configure
>>> @@ -29609,6 +29609,30 @@ fi
>>>  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ld_bndplt_support" >&5
>>>  $as_echo "$ld_bndplt_support" >&6; }
>>>
>>> +# Check linker supports '--push-state'/'--pop-state'
>>> +ld_pushpopstate_support=no
>>> +{ $as_echo "$as_me:

Re: [PING] Use correct location information for OpenACC shape and simple clauses in C/C++

[Gerald Pfeifer]

On Thu, 4 Aug 2016, David Malcolm wrote:
> On Thu, 2016-08-04 at 16:54 +0200, Thomas Schwinge wrote:
>> I think in your position as a maintainer for "diagnostic messages", 
>> you should be qualified to exercise that status to approve such a 
>> patch. :-)
> I don't know exactly where the boundaries of that role are; I've been
> assuming it means anything relating to the diagnostic subsystem itself
> (and location-tracking), as opposed to *usage* of the system.  The
> patch in question is more about the latter.  That said, your patch
> looks very reasonable to me (but as I mentioned, a test case
> demonstrating the improved caret locations would be good).
> 
> Steering committee: am I being too conservative in my interpretation of
> that role?

Somehow your question seems to have been missed?  Sorry about that.

For the sake of full disclosure, on the steering committee we
usually do not dive super deeply into what a maintainer's domain
entails and what not.  

What I generally see is that being a bit liberal, especially when 
you know that adjacent area and are confident, tends to work for 
everyone.  (It's not that we have excessive review capacity.)

Gerald

[PATCH] PR fortran/77406

[Steve Kargl]

Consider the code

   interface s
  subroutine foo(*)
  end subroutine foo
  subroutine bar(*)
  end subroutine bar
   end interface s
   end

gfortran currently ICE's, because she is ill-prepared
to deal with alternate returns in an interface blocks.
The attached patch fixes this problem.


While I was fixing the problem I took this opportunity
to improve the error reporting.  Consider the code

   interface s
  subroutine foo(i)
  end subroutine foo
  subroutine bar(j)
  end subroutine bar
   end interface s
   end

gfortran currently reports

% gfc6 a.f90
a.f90:7:21:

end subroutine bar
 1
Error: Ambiguous interfaces 'bar' and 'foo' in generic interface 's' at (1)

The (IMNSHO) improved error messages is now

% gfc7 -c a.f90
a.f90:3:17:

subroutine foo(i)
 1
a.f90:6:17:

subroutine bar(j)
 2
Error: Ambiguous interfaces in generic interface 's' for 'foo' at (1) and 'bar' 
at (2)

OK to commit on Saturday?


2016-09-03  Steven G. Kargl  

PR fortran/77406
* interface.c (gfc_compare_interfaces): Fix detection of ambiguous
interface involving alternate return.
(check_interface1): Improve error message and loci.

2016-09-03  Steven G. Kargl  

PR fortran/77406
* gfortran.dg/pr77406.f90: New test.
* gfortran.dg/assumed_type_3.f90: Update error messages.
* gfortran.dg/defined_operators_1.f90: Ditto.
* gfortran.dg/generic_26.f90: Ditto.
* gfortran.dg/generic_7.f90: Ditto.
* gfortran.dg/gomp/udr5.f90: Ditto.
* gfortran.dg/gomp/udr7.f90: Ditto.
* gfortran.dg/interface_1.f90: Ditto.
* gfortran.dg/interface_37.f90: Ditto.
* gfortran.dg/interface_5.f90: Ditto.
* gfortran.dg/interface_6.f90: Ditto.
* gfortran.dg/interface_7.f90
* gfortran.dg/no_arg_check_3.f90
* gfortran.dg/operator_5.f90
* gfortran.dg/proc_ptr_comp_20.f90: Ditto.

-- 
Steve
Index: gcc/fortran/interface.c
===
--- gcc/fortran/interface.c	(revision 239833)
+++ gcc/fortran/interface.c	(working copy)
@@ -1616,14 +1616,23 @@ gfc_compare_interfaces (gfc_symbol *s1, 
   f1 = gfc_sym_get_dummy_args (s1);
   f2 = gfc_sym_get_dummy_args (s2);
 
+  /* Special case: No arguments.  */
   if (f1 == NULL && f2 == NULL)
-return 1;			/* Special case: No arguments.  */
+return 1;
 
   if (generic_flag)
 {
   if (count_types_test (f1, f2, p1, p2)
 	  || count_types_test (f2, f1, p2, p1))
 	return 0;
+
+  /* Special case: alternate returns.  If both f1->sym and f2->sym are
+	 NULL, then the leading formal arguments are alternate returns.  
+	 The previous conditional should catch argument lists with 
+	 different number of argument.  */
+  if (f1 && f1->sym == NULL && f2 && f2->sym == NULL)
+	return 1;
+
   if (generic_correspondence (f1, f2, p1, p2)
 	  || generic_correspondence (f2, f1, p2, p1))
 	return 0;
@@ -1791,13 +1800,15 @@ check_interface1 (gfc_interface *p, gfc_
    generic_flag, 0, NULL, 0, NULL, NULL))
 	  {
 	if (referenced)
-	  gfc_error ("Ambiguous interfaces %qs and %qs in %s at %L",
-			 p->sym->name, q->sym->name, interface_name,
-			 &p->where);
+	  gfc_error ("Ambiguous interfaces in %s for %qs at %L "
+			 "and %qs at %L", interface_name,
+			 q->sym->name, &q->sym->declared_at,
+			 p->sym->name, &p->sym->declared_at);
 	else if (!p->sym->attr.use_assoc && q->sym->attr.use_assoc)
-	  gfc_warning (0, "Ambiguous interfaces %qs and %qs in %s at %L",
-			   p->sym->name, q->sym->name, interface_name,
-			   &p->where);
+	  gfc_warning (0, "Ambiguous interfaces in %s for %qs at %L "
+			 "and %qs at %L", interface_name,
+			 q->sym->name, &q->sym->declared_at,
+			 p->sym->name, &p->sym->declared_at);
 	else
 	  gfc_warning (0, "Although not referenced, %qs has ambiguous "
 			   "interfaces at %L", interface_name, &p->where);
Index: gcc/testsuite/gfortran.dg/assumed_type_3.f90
===
--- gcc/testsuite/gfortran.dg/assumed_type_3.f90	(revision 239847)
+++ gcc/testsuite/gfortran.dg/assumed_type_3.f90	(working copy)
@@ -66,12 +66,12 @@ subroutine nine()
 end subroutine okok2
   end interface
   interface three
-subroutine ambig1(x)
+subroutine ambig1(x) ! { dg-error "Ambiguous interfaces" }
   type(*) :: x
 end subroutine ambig1
-subroutine ambig2(x)
+subroutine ambig2(x) ! { dg-error "Ambiguous interfaces" }
   integer :: x
-end subroutine ambig2 ! { dg-error "Ambiguous interfaces 'ambig2' and 'ambig1' in generic interface 'three'" }
+end subroutine ambig2
   end interface
 end subroutine nine
 
Index: gcc/testsuite/gfortran.dg/defined_operators_1.f90
===
--- gcc/testsuite/gfortran.dg/defined_operators_1.f90	(re

[PATCH] Improvements to typed_splay_tree (v2)

[David Malcolm]

On Wed, 2016-08-31 at 16:12 +0200, Bernd Schmidt wrote:
> On 08/24/2016 03:28 AM, David Malcolm wrote:
> > +/* Helper type for typed_splay_tree::foreach.  */
> > +
> > +template 
> > +struct closure
> 
> Is this in the global namespace? If so, something more specific than
> "closure" might be more appropriate. Or move the struct into the
> typed_splay_tree class definition.
> 
> Looks ok otherwise.

Thanks.  Here's a revised version; I moved the "struct closure" into
the class itself.

Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.

OK for trunk?

gcc/ChangeLog:
* Makefile.in (OBJS): Add typed-splay-tree.o.
* selftest-run-tests.c (selftest::run_tests): Call
typed_splay_tree_c_tests.
* selftest.h (typed_splay_tree_c_tests): New decl.
* typed-splay-tree.c: New file.
* typed-splay-tree.h (typed_splay_tree::foreach_fn): New typedef.
(typed_splay_tree::max): New method.
(typed_splay_tree::min): New method.
(typed_splay_tree::foreach): New method.
(typed_splay_tree::closure): New struct.
(typed_splay_tree::inner_foreach_fn): New function.
---
 gcc/Makefile.in  |  1 +
 gcc/selftest-run-tests.c |  1 +
 gcc/selftest.h   |  1 +
 gcc/typed-splay-tree.c   | 79 
 gcc/typed-splay-tree.h   | 62 +
 5 files changed, 144 insertions(+)
 create mode 100644 gcc/typed-splay-tree.c

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index eb5ab61..b38a0c1 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1542,6 +1542,7 @@ OBJS = \
tree-vectorizer.o \
tree-vrp.o \
tree.o \
+   typed-splay-tree.o \
valtrack.o \
value-prof.o \
var-tracking.o \
diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
index 6453e31..e20bbd0 100644
--- a/gcc/selftest-run-tests.c
+++ b/gcc/selftest-run-tests.c
@@ -56,6 +56,7 @@ selftest::run_tests ()
   ggc_tests_c_tests ();
   sreal_c_tests ();
   fibonacci_heap_c_tests ();
+  typed_splay_tree_c_tests ();
 
   /* Mid-level data structures.  */
   input_c_tests ();
diff --git a/gcc/selftest.h b/gcc/selftest.h
index 47d7350..54a33f9 100644
--- a/gcc/selftest.h
+++ b/gcc/selftest.h
@@ -166,6 +166,7 @@ extern void selftest_c_tests ();
 extern void spellcheck_c_tests ();
 extern void spellcheck_tree_c_tests ();
 extern void sreal_c_tests ();
+extern void typed_splay_tree_c_tests ();
 extern void tree_c_tests ();
 extern void tree_cfg_c_tests ();
 extern void vec_c_tests ();
diff --git a/gcc/typed-splay-tree.c b/gcc/typed-splay-tree.c
new file mode 100644
index 000..33992c1
--- /dev/null
+++ b/gcc/typed-splay-tree.c
@@ -0,0 +1,79 @@
+/* Selftests for typed-splay-tree.h.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "typed-splay-tree.h"
+#include "selftest.h"
+
+#if CHECKING_P
+
+namespace selftest {
+
+/* Callback for use by test_str_to_int.  */
+
+static int
+append_cb (const char *, int value, void *user_data)
+{
+  auto_vec  *vec = (auto_vec  *)user_data;
+  vec->safe_push (value);
+  return 0;
+}
+
+/* Test of typed_splay_tree .  */
+
+static void
+test_str_to_int ()
+{
+  typed_splay_tree  t (strcmp, NULL, NULL);
+
+  t.insert ("a", 1);
+  t.insert ("b", 2);
+  t.insert ("c", 3);
+
+  ASSERT_EQ (1, t.lookup ("a"));
+  ASSERT_EQ (2, t.lookup ("b"));
+  ASSERT_EQ (3, t.lookup ("c"));
+
+  ASSERT_EQ (2, t.predecessor ("c"));
+  ASSERT_EQ (3, t.successor ("b"));
+  ASSERT_EQ (1, t.min ());
+  ASSERT_EQ (3, t.max ());
+
+  /* Test foreach by appending to a vec, and verifying the vec.  */
+  auto_vec  v;
+  t.foreach (append_cb, &v);
+  ASSERT_EQ (3, v.length ());
+  ASSERT_EQ (1, v[0]);
+  ASSERT_EQ (2, v[1]);
+  ASSERT_EQ (3, v[2]);
+}
+
+/* Run all of the selftests within this file.  */
+
+void
+typed_splay_tree_c_tests ()
+{
+  test_str_to_int ();
+}
+
+} // namespace selftest
+
+#endif /* #if CHECKING_P */
diff --git a/gcc/typed-splay-tree.h b/gcc/typed-splay-tree.h
index 9dd96d6..7b8afef 100644
--- a/gcc/typed-splay-tree.h
+++ b/gcc/typed-splay-tree.h
@@ -33,6 +33,7 @@ class typed_splay_tree
   typedef int (*compare_fn) (key_type, key_type);
   typedef void (*delete_key_fn) (key_type);
   typedef void (*delete_value_fn) (value_type

[PATCH 1/3] rs6000: Fix for AIX, for r239866

[Segher Boessenkool]

This should fix r239866 for AIX.  I missed two patterns that refer to LR
as "register_operand" "l" instead of as reg:P LR_REGNO.

David, could you please test if this fixes the problem for you?

Bootstrapped and regression checked on powerpc64-linux -m32,-m64, for
what that is worth :-)


Segher


2016-08-31  Segher Boessenkool  

* config/rs6000/rs6000.md
(define_insn "*return_and_restore_fpregs_aix__r11"): Delete
the use of the link register.
(define_insn "*return_and_restore_fpregs_aix__r1"): Ditto.

---
 gcc/config/rs6000/rs6000.md | 18 --
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 560cf1f..d86d27b 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -12735,26 +12735,24 @@ (define_insn "*return_and_restore_fpregs__r1"
 (define_insn "*return_and_restore_fpregs_aix__r11"
  [(match_parallel 0 "any_parallel_operand"
  [(return)
-  (use (match_operand:P 1 "register_operand" "l"))
-  (use (match_operand:P 2 "symbol_ref_operand" "s"))
+  (use (match_operand:P 1 "symbol_ref_operand" "s"))
   (use (reg:P 11))
-  (set (match_operand:DF 3 "gpc_reg_operand" "=d")
-   (match_operand:DF 4 "memory_operand" "m"))])]
+  (set (match_operand:DF 2 "gpc_reg_operand" "=d")
+   (match_operand:DF 3 "memory_operand" "m"))])]
  ""
- "b %2"
+ "b %1"
  [(set_attr "type" "branch")
   (set_attr "length" "4")])
 
 (define_insn "*return_and_restore_fpregs_aix__r1"
  [(match_parallel 0 "any_parallel_operand"
  [(return)
-  (use (match_operand:P 1 "register_operand" "l"))
-  (use (match_operand:P 2 "symbol_ref_operand" "s"))
+  (use (match_operand:P 1 "symbol_ref_operand" "s"))
   (use (reg:P 1))
-  (set (match_operand:DF 3 "gpc_reg_operand" "=d")
-   (match_operand:DF 4 "memory_operand" "m"))])]
+  (set (match_operand:DF 2 "gpc_reg_operand" "=d")
+   (match_operand:DF 3 "memory_operand" "m"))])]
  ""
- "b %2"
+ "b %1"
  [(set_attr "type" "branch")
   (set_attr "length" "4")])
 
-- 
1.9.3

[PATCH 2/3] rs6000: Use LR_REGNO instead of constant 65

[Segher Boessenkool]

Many places still use 65 instead of the symbolic constant LR_REGNO.  This
fixes them all (I looked for the string "65" only, in config/rs6000/ only,
I didn't read all code :-) )

I left it in *restore_world because Iain will remove it there soon.

Bootstrapped and regression checked on powerpc64-linux -m32,-m64.


Segher


2016-08-31  Segher Boessenkool  

* config/rs6000/altivec.md (*save_world, *save_vregs__r11,
save_vregs__r12, *restore_vregs__r11,
*restore_vregs__r12): Use LR_REGNO instead of 65.
* config/rs6000/darwin.md (load_macho_picbase, load_macho_picbase_si,
load_macho_picbase_di, *call_indirect_nonlocal_darwin64,
*call_nonlocal_darwin64, *call_value_indirect_nonlocal_darwin64,
*call_value_nonlocal_darwin64, reload_macho_picbase,
reload_macho_picbase_si, reload_macho_picbase_di): Ditto.
* config/rs6000/rs6000.h (RETURN_ADDR_IN_PREVIOUS_FRAME): Ditto.
* config/rs6000/rs6000.md (*save_gpregs__r11,
*save_gpregs__r12, *save_gpregs__r1,
*save_fpregs__r11, *save_fpregs__r12,
*save_fpregs__r1): Ditto.
* config/rs6000/spe.md (*save_gpregs_spe, *restore_gpregs_spe,
*return_and_restore_gpregs_spe): Ditto.

---
 gcc/config/rs6000/altivec.md | 12 ++--
 gcc/config/rs6000/darwin.md  | 20 ++--
 gcc/config/rs6000/rs6000.h   |  2 +-
 gcc/config/rs6000/rs6000.md  | 12 ++--
 gcc/config/rs6000/spe.md |  6 +++---
 5 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
index c39a0b6..25472c29 100644
--- a/gcc/config/rs6000/altivec.md
+++ b/gcc/config/rs6000/altivec.md
@@ -397,7 +397,7 @@ (define_insn "*set_vrsave_internal"
 
 (define_insn "*save_world"
  [(match_parallel 0 "save_world_operation"
-  [(clobber (reg:SI 65))
+  [(clobber (reg:SI LR_REGNO))
(use (match_operand:SI 1 "call_operand" "s"))])]
  "TARGET_MACHO && (DEFAULT_ABI == ABI_DARWIN) && TARGET_32BIT" 
  "bl %z1"
@@ -407,7 +407,7 @@ (define_insn "*save_world"
 (define_insn "*restore_world"
  [(match_parallel 0 "restore_world_operation"
   [(return)
-  (use (reg:SI 65))
+  (use (reg:SI LR_REGNO))
(use (match_operand:SI 1 "call_operand" "s"))
(clobber (match_operand:SI 2 "gpc_reg_operand" "=r"))])]
  "TARGET_MACHO && (DEFAULT_ABI == ABI_DARWIN) && TARGET_32BIT"
@@ -421,7 +421,7 @@ (define_insn "*restore_world"
 ;; to describe the operation to dwarf2out_frame_debug_expr.
 (define_insn "*save_vregs__r11"
   [(match_parallel 0 "any_parallel_operand"
- [(clobber (reg:P 65))
+ [(clobber (reg:P LR_REGNO))
   (use (match_operand:P 1 "symbol_ref_operand" "s"))
   (clobber (reg:P 11))
   (use (reg:P 0))
@@ -435,7 +435,7 @@ (define_insn "*save_vregs__r11"
 
 (define_insn "*save_vregs__r12"
   [(match_parallel 0 "any_parallel_operand"
- [(clobber (reg:P 65))
+ [(clobber (reg:P LR_REGNO))
   (use (match_operand:P 1 "symbol_ref_operand" "s"))
   (clobber (reg:P 12))
   (use (reg:P 0))
@@ -449,7 +449,7 @@ (define_insn "*save_vregs__r12"
 
 (define_insn "*restore_vregs__r11"
   [(match_parallel 0 "any_parallel_operand"
- [(clobber (reg:P 65))
+ [(clobber (reg:P LR_REGNO))
   (use (match_operand:P 1 "symbol_ref_operand" "s"))
   (clobber (reg:P 11))
   (use (reg:P 0))
@@ -463,7 +463,7 @@ (define_insn "*restore_vregs__r11"
 
 (define_insn "*restore_vregs__r12"
   [(match_parallel 0 "any_parallel_operand"
- [(clobber (reg:P 65))
+ [(clobber (reg:P LR_REGNO))
   (use (match_operand:P 1 "symbol_ref_operand" "s"))
   (clobber (reg:P 12))
   (use (reg:P 0))
diff --git a/gcc/config/rs6000/darwin.md b/gcc/config/rs6000/darwin.md
index 57ce30e..5870e0a 100644
--- a/gcc/config/rs6000/darwin.md
+++ b/gcc/config/rs6000/darwin.md
@@ -238,7 +238,7 @@ (define_split
   "")
 
 (define_expand "load_macho_picbase"
-  [(set (reg:SI 65)
+  [(set (reg:SI LR_REGNO)
 (unspec [(match_operand 0 "" "")]
UNSPEC_LD_MPIC))]
   "(DEFAULT_ABI == ABI_DARWIN) && flag_pic"
@@ -252,7 +252,7 @@ (define_expand "load_macho_picbase"
 })
 
 (define_insn "load_macho_picbase_si"
-  [(set (reg:SI 65)
+  [(set (reg:SI LR_REGNO)
(unspec:SI [(match_operand:SI 0 "immediate_operand" "s")
(pc)] UNSPEC_LD_MPIC))]
   "(DEFAULT_ABI == ABI_DARWIN) && flag_pic"
@@ -268,7 +268,7 @@ (define_insn "load_macho_picbase_si"
(set_attr "length" "4")])
 
 (define_insn "load_macho_picbase_di"
-  [(set (reg:DI 65)
+  [(set (reg:DI LR_REGNO)
(unspec:DI [(match_operand:DI 0 "immediate_operand" "s")
(pc)] UNSPEC_LD_MPIC))]
   "(DEFAULT_ABI == ABI_DARWIN) && flag_pic && TARGET_64BIT"
@@ -325,7 +325,7 @@ (define_insn "*call_indirect_nonlocal_darwin64"
   [(call (mem:SI (match_operand:DI 0 "register_operand" "c,*

[committed] C: Fix missing spaces in 'struct' fix-it hints

[David Malcolm]

In r237714 I added fix-it hints to the C frontend for missing "struct"
keywords e.g.:

spellcheck-typenames.c:69:1: error: unknown type name ‘foo_t’; use
‘struct’ keyword to refer to the type
 foo_t *foo_ptr;
 ^
 struct 

However when using the (not yet in trunk) option
 -fdiagnostics-generate-patch,
the generated patch is nonsensical:

  -foo_t *foo_ptr;
  +structfoo_t *foo_ptr;

Fix the fix-its by adding a trailing space to each one, giving:

  -foo_t *foo_ptr;
  +struct foo_t *foo_ptr;

Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.

Committed to trunk as r239912, as obvious.

gcc/c/ChangeLog:
* c-parser.c (c_parser_declaration_or_fndef): Add trailing space
to the insertion fixits for "struct", "union", and "enum".

---
 gcc/c/c-parser.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index a6281fc..531d94e 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -1685,7 +1685,7 @@ c_parser_declaration_or_fndef (c_parser *parser, bool 
fndef_ok,
   if (tag_exists_p (RECORD_TYPE, name))
{
  /* This is not C++ with its implicit typedef.  */
- richloc.add_fixit_insert ("struct");
+ richloc.add_fixit_insert ("struct ");
  error_at_rich_loc (&richloc,
 "unknown type name %qE;"
 " use % keyword to refer to the type",
@@ -1693,7 +1693,7 @@ c_parser_declaration_or_fndef (c_parser *parser, bool 
fndef_ok,
}
   else if (tag_exists_p (UNION_TYPE, name))
{
- richloc.add_fixit_insert ("union");
+ richloc.add_fixit_insert ("union ");
  error_at_rich_loc (&richloc,
 "unknown type name %qE;"
 " use % keyword to refer to the type",
@@ -1701,7 +1701,7 @@ c_parser_declaration_or_fndef (c_parser *parser, bool 
fndef_ok,
}
   else if (tag_exists_p (ENUMERAL_TYPE, name))
{
- richloc.add_fixit_insert ("enum");
+ richloc.add_fixit_insert ("enum ");
  error_at_rich_loc (&richloc,
 "unknown type name %qE;"
 " use % keyword to refer to the type",
-- 
1.8.5.3

[PATCH 3/3] rs6000: Use LR_REGNO directly in the save/restore patterns

[Segher Boessenkool]

Various patterns use "register_operand" "l" (or "=l") although those
patterns are only created refering to LR_REGNO directly.  This patch
changes those patterns to use the hard regs as well.

Bootstrapped and regression checked on powerpc64-linux -m32,-m64.


Segher


2016-08-31  Segher Boessenkool  

* config/rs6000/rs6000.md (*restore_gpregs__r11,
*restore_gpregs__r12, *restore_gpregs__r1,
*return_and_restore_gpregs__r11,
*return_and_restore_gpregs__r12,
*return_and_restore_gpregs__r1,
*return_and_restore_fpregs__r11,
*return_and_restore_fpregs__r12,
*return_and_restore_fpregs__r1): Use the hard register LR_REGNO
directly instead of via the "l" constraint.  Renumber operands.
Fix whitespace.

---
 gcc/config/rs6000/rs6000.md | 120 ++--
 1 file changed, 60 insertions(+), 60 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 1ecbb9d..7da4370 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -12620,115 +12620,115 @@ (define_insn "*lmw"
 
 (define_insn "*restore_gpregs__r11"
  [(match_parallel 0 "any_parallel_operand"
-  [(clobber (match_operand:P 1 "register_operand" "=l"))
-   (use (match_operand:P 2 "symbol_ref_operand" "s"))
-   (use (reg:P 11))
-  (set (match_operand:P 3 "gpc_reg_operand" "=r")
-   (match_operand:P 4 "memory_operand" "m"))])]
+ [(clobber (reg:P LR_REGNO))
+  (use (match_operand:P 1 "symbol_ref_operand" "s"))
+  (use (reg:P 11))
+  (set (match_operand:P 2 "gpc_reg_operand" "=r")
+   (match_operand:P 3 "memory_operand" "m"))])]
  ""
- "bl %2"
+ "bl %1"
  [(set_attr "type" "branch")
   (set_attr "length" "4")])
 
 (define_insn "*restore_gpregs__r12"
  [(match_parallel 0 "any_parallel_operand"
-  [(clobber (match_operand:P 1 "register_operand" "=l"))
-   (use (match_operand:P 2 "symbol_ref_operand" "s"))
-   (use (reg:P 12))
-  (set (match_operand:P 3 "gpc_reg_operand" "=r")
-   (match_operand:P 4 "memory_operand" "m"))])]
+ [(clobber (reg:P LR_REGNO))
+  (use (match_operand:P 1 "symbol_ref_operand" "s"))
+  (use (reg:P 12))
+  (set (match_operand:P 2 "gpc_reg_operand" "=r")
+   (match_operand:P 3 "memory_operand" "m"))])]
  ""
- "bl %2"
+ "bl %1"
  [(set_attr "type" "branch")
   (set_attr "length" "4")])
 
 (define_insn "*restore_gpregs__r1"
  [(match_parallel 0 "any_parallel_operand"
-  [(clobber (match_operand:P 1 "register_operand" "=l"))
-   (use (match_operand:P 2 "symbol_ref_operand" "s"))
-   (use (reg:P 1))
-  (set (match_operand:P 3 "gpc_reg_operand" "=r")
-   (match_operand:P 4 "memory_operand" "m"))])]
+ [(clobber (reg:P LR_REGNO))
+  (use (match_operand:P 1 "symbol_ref_operand" "s"))
+  (use (reg:P 1))
+  (set (match_operand:P 2 "gpc_reg_operand" "=r")
+   (match_operand:P 3 "memory_operand" "m"))])]
  ""
- "bl %2"
+ "bl %1"
  [(set_attr "type" "branch")
   (set_attr "length" "4")])
 
 (define_insn "*return_and_restore_gpregs__r11"
  [(match_parallel 0 "any_parallel_operand"
-  [(return)
-  (clobber (match_operand:P 1 "register_operand" "=l"))
-  (use (match_operand:P 2 "symbol_ref_operand" "s"))
-   (use (reg:P 11))
-  (set (match_operand:P 3 "gpc_reg_operand" "=r")
-   (match_operand:P 4 "memory_operand" "m"))])]
+ [(return)
+  (clobber (reg:P LR_REGNO))
+  (use (match_operand:P 1 "symbol_ref_operand" "s"))
+  (use (reg:P 11))
+  (set (match_operand:P 2 "gpc_reg_operand" "=r")
+   (match_operand:P 3 "memory_operand" "m"))])]
  ""
- "b %2"
+ "b %1"
  [(set_attr "type" "branch")
   (set_attr "length" "4")])
 
 (define_insn "*return_and_restore_gpregs__r12"
  [(match_parallel 0 "any_parallel_operand"
-  [(return)
-  (clobber (match_operand:P 1 "register_operand" "=l"))
-  (use (match_operand:P 2 "symbol_ref_operand" "s"))
-   (use (reg:P 12))
-  (set (match_operand:P 3 "gpc_reg_operand" "=r")
-   (match_operand:P 4 "memory_operand" "m"))])]
+ [(return)
+  (clobber (reg:P LR_REGNO))
+  (use (match_operand:P 1 "symbol_ref_operand" "s"))
+  (use (reg:P 12))
+  (set (match_operand:P 2 "gpc_reg_operand" "=r")
+  

[PATCH v2, rs6000] Fix PR72827 (ada bootstrap failure)

[Bill Schmidt]

Hi,

After much discussion we've concluded that the first patch isn't
salvageable.  There are register assignments for which we can't fix up
the addressing legally after reload using such tricks (for example:
base=r31, offset=r0).

This patch (suggested by Michael Meissner) instead prevents the problem
by disallowing reg+reg addressing for TImode, allowing D-form addressing
to be used for the separate stores of the GPRs.  This is not an ideal
permanent solution, because it disallows reg+reg addressing not only for
TImode in GPRs, but also for TImode in VSRs.  Our intent is to work on a
solution to provide a scratch register via secondary reload, but this
may take some time.  Thus I'd like to submit this patch as a short-term
solution for the bootstrap problem until then.

I plan to do a performance evaluation of this patch for informational
purposes, but I don't expect much change, and the results won't affect
our choice to handle this properly in secondary reload.

Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
regressions, and the gnattools now build properly so that the ada
bootstrap succeeds.  Is this ok for trunk?

Thanks,
Bill


2016-08-31  Bill Schmidt  
Michael Meissner 

PR target/72827
* config/rs6000/rs6000.c (rs6000_legitimize_address): Avoid
reg+reg addressing for TImode.
(rs6000_legitimate_address_p): Only allow register indirect
addressing for TImode, even without TARGET_QUAD_MEMORY.


Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c  (revision 239871)
+++ gcc/config/rs6000/rs6000.c  (working copy)
@@ -8409,7 +8409,7 @@ rs6000_legitimize_address (rtx x, rtx oldx ATTRIBU
 pointer, so it works with both GPRs and VSX registers.  */
   /* Make sure both operands are registers.  */
   else if (GET_CODE (x) == PLUS
-  && (mode != TImode || !TARGET_QUAD_MEMORY))
+  && (mode != TImode || !TARGET_VSX_TIMODE))
return gen_rtx_PLUS (Pmode,
 force_reg (Pmode, XEXP (x, 0)),
 force_reg (Pmode, XEXP (x, 1)));
@@ -9418,12 +9418,16 @@ rs6000_legitimate_address_p (machine_mode mode, rt
return 1;
 }
 
-  /* For TImode, if we have load/store quad and TImode in VSX registers, only
- allow register indirect addresses.  This will allow the values to go in
- either GPRs or VSX registers without reloading.  The vector types would
- tend to go into VSX registers, so we allow REG+REG, while TImode seems
+  /* For TImode, if we have TImode in VSX registers, only allow register
+ indirect addresses.  This will allow the values to go in either GPRs
+ or VSX registers without reloading.  The vector types would tend to
+ go into VSX registers, so we allow REG+REG, while TImode seems
  somewhat split, in that some uses are GPR based, and some VSX based.  */
-  if (mode == TImode && TARGET_QUAD_MEMORY && TARGET_VSX_TIMODE)
+  /* FIXME: We could loosen this by changing the following to
+   if (mode == TImode && TARGET_QUAD_MEMORY && TARGET_VSX_TIMODE)
+ but currently we cannot allow REG+REG addressing for TImode.  See
+ PR72827 for complete details on how this ends up hoodwinking DSE.  */
+  if (mode == TImode && TARGET_VSX_TIMODE)
 return 0;
   /* If not REG_OK_STRICT (before reload) let pass any stack offset.  */
   if (! reg_ok_strict

Re: PR35503 - warn for restrict pointer

[Richard Biener]

On Tue, 30 Aug 2016, Tom de Vries wrote:

> On 30/08/16 17:34, Prathamesh Kulkarni wrote:
> > On 30 August 2016 at 20:24, Tom de Vries  wrote:
> > > On 26/08/16 13:39, Prathamesh Kulkarni wrote:
> > > > 
> > > > Hi,
> > > > The following patch adds option -Wrestrict that warns when an argument
> > > > is passed to a restrict qualified parameter and it aliases with
> > > > another argument.
> > > > 
> > > > eg:
> > > > int foo (const char *__restrict buf, const char *__restrict fmt, ...);
> > > > 
> > > > void f(void)
> > > > {
> > > >   char buf[100] = "hello";
> > > >   foo (buf, "%s-%s", buf, "world");
> > > > }
> > > 
> > > 
> > > Does -Wrestrict generate a warning for this example?
> > > 
> > > ...
> > > void h(int n, int * restrict p, int * restrict q, int * restrict r)
> > > {
> > >   int i;
> > >   for (i = 0; i < n; i++)
> > > p[i] = q[i] + r[i];
> > > }
> > > 
> > > h (100, a, b, b)
> > > ...
> > > 
> > > [ Note that this is valid C, and does not violate the restrict definition.
> > > ]
> > > 
> > > If -Wrestrict indeed generates a warning, then we should be explicit in
> > > the
> > > documentation that while the warning triggers on this type of example, the
> > > code is correct.
> > I am afraid it would warn for the above case. The patch just checks if
> > the parameter is qualified
> > with restrict, and if the corresponding argument has aliases in the
> > call (by calling operand_equal_p).
> 
> > Is such code common in practice ?
> 
> [ The example is from the definition of restrict in the c99 standard. ]
> 
> According to the definition of restrict, only the restrict on p is required to
> know that p doesn't alias with q and that p doesn't alias with r.
> 
> AFAIK the current implementation of gcc already generates optimal code if
> restrict is only on p. But earlier versions (and possibly other compilers as
> well?) need the restrict on q and r as well.
> 
> So I expect this code to occur.
> 
> > If it is, I wonder if we should keep
> > the warning in -Wall ?
> > 
> 
> Hmm, I wonder if we can use the const keyword to silence the warning.
> 
> So if we generate a warning for:
> ...
> void h(int n, int * restrict p, int * restrict q, int * restrict r)
> {
>   int i;
>   for (i = 0; i < n; i++)
> p[i] = q[i] + r[i];
> }
> h (100, a, b, b)
> ...
> 
> but not for:
> ...
> void h(int n, int * restrict p, const int * restrict q, const int * restrict
> r)
> {
>   int i;
>   for (i = 0; i < n; i++)
> p[i] = q[i] + r[i];
> }
> h (100, a, b, b)
> ...
> 
> Then there's an easy way to rewrite the example such that the warning doesn't
> trigger, without having to remove the restrict.

Note that I've seen restrict being used even for

void h(int n, int * restrict p, int * restrict q)
{
  int i;
  for (i = 0; i < n; i++)
p[2*i] = p[2*i] + q[2*i + 1];
}

thus where the actual accesses do not alias (the pointers are used
to access every other element).  I think the definition of "object"
(based on accessed lvalues) makes this example valid.  So we
shouldn't warn about

h (n, p, p)

but we could warn about

h (n, p, p + 1)

and yes, only one of the pointers need to be restrict qualified.

Note that as with all other alias warnings if you want to avoid
false positives and rely on conservative analysis then you can
as well simply avoid taking advantate of the bug in the code
(as we do for TBAA and trivial must-alias cases).  If you allow
false positives then you ultimatively end up with a mess like
our existing -Wstrict-aliasing warnings (which are IMHO quite
useless).

Note that nowhere in GCC we implement anything closely matching
the formal definition of restrict as writte in the C standard --
only in fronted code could we properly derive the 'based-on'
property and note lvalues affected by restrict.  Currently we
are restricted to looking at restrict qualified parameters
because of this.

Richard.