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

[Richard Biener]

On Wed, 31 Aug 2016, Jakub Jelinek wrote:

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

Hmm, maybe simply do

  if (gimple_call_builtin_p (g, BUILT_IN_ASAN_AFTER_DYNAMIC_INIT))
{
  gimple *def = SSA_NAME_DEF_STMT (gimple_vuse (g));
  if (gimple_call_builtin_p (def, BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT))
... remove both ...

intervening non-memory-def stmts do not really matter, do they?  If
loads matter then checking for the single-vdef-use of
BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT to be BUILT_IN_ASAN_AFTER_DYNAMIC_INIT
would also work.

Otherwise looks ok but I wonder why libasan needs to assert this ..

Richard.

> 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.c  2016-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.c  2016-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.c2016-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=*/fals

Re: [patch] Fix PR fortran/72743

[Richard Biener]

On Wed, 31 Aug 2016, Chung-Lin Tang wrote:

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

If original is the ultimate alias target then this is indeed correct.

The IPA PTA code asserts that all decls that are an alias of the ultimate
alias target X have the same DECL_PT_UID as X DECL_UID (or if the new 
alias doesn't have DECL_PT_UID set it will set it that way).

So I'd say it should be

  SET_DECL_PT_UID (alias->decl, DECL_UID (original->decl));

instead.

If IPA-ICF would merge

static const int x = 1;
static const int xa __attribute__((alias("x")));
static const int y = 1;
static const int ya __attribute__((alias("y")));

we'd initially have DECL_PT_UID (xa) be DECL_UID (x) and
DECL_PT_UID (ya) be DECL_UID (y).  Depending on how exactly IPA-ICF
performs the merging of x and y (and thus adjusts the existing aliases)
IPA-ICF needs to adjust DECL_PT_UID of all aliases of the new
"ultimate alias target".  I don't think the current code with the
proposed patch makes sure this will happen.

Thanks,
Richard.

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

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

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

[Eric Botcazou]

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

Thanks for working on the problem and devising a fix!

-- 
Eric Botcazou

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

[Andreas Schwab]

On Sep 01 2016, David Malcolm  wrote:

> 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 ");

Perhaps add_fixit_insert should receive a second argument that says
whether to prepend/append a space when inserting.  Then it can avoid
printing the space when the hint is displayed.

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] Fix up -fsanitize=address ctor order (PR sanitizer/77396)

[Jakub Jelinek]

On Thu, Sep 01, 2016 at 08:59:57AM +0200, Richard Biener wrote:
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> Hmm, maybe simply do
> 
>   if (gimple_call_builtin_p (g, BUILT_IN_ASAN_AFTER_DYNAMIC_INIT))
> {
>   gimple *def = SSA_NAME_DEF_STMT (gimple_vuse (g));
>   if (gimple_call_builtin_p (def, BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT))
> ... remove both ...
> 
> intervening non-memory-def stmts do not really matter, do they?  If
> loads matter then checking for the single-vdef-use of
> BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT to be BUILT_IN_ASAN_AFTER_DYNAMIC_INIT
> would also work.

Loads matter too.  What I've been worried about is that some optimizations
would turn the original
  __asan_before_dynamic_init ("foobar");
  ...
  __asan_after_dynamic_init ();
into:
  __asan_before_dynamic_init ("foobar");
  ...
  if (...)
{
  ...
  __asan_after_dynamic_init ();
  return;
}
  ...
  __asan_after_dynamic_init ();
or something similar and then if removing both on seeing after and its vuse
before, we'd then keep around the other after call.
Maybe instead of removing both I could just remember both, if it ever finds
more than one of each kind of calls, punt, and otherwise remove at the end
of the stmt walk?
  
> Otherwise looks ok but I wonder why libasan needs to assert this ..

It a tiny bit simplifies the code, and of course, a perfect compiler would
never emit those if there isn't anything to protect, it is just that it is
hard on the compiler side to guarantee it always.

// PR sanitizer/77396
// { dg-do run }
// { dg-additional-options "-O2" }
// { dg-set-target-env-var ASAN_OPTIONS "check_initialization_order=true" }

struct S { S () { asm volatile ("" : : : "memory"); } };
static S c;

int
main ()
{
  return 0;
}

is what still fails even with the patch.

Jakub

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

[Richard Biener]

On Thu, 1 Sep 2016, Jakub Jelinek wrote:

> On Thu, Sep 01, 2016 at 08:59:57AM +0200, Richard Biener wrote:
> > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > 
> > Hmm, maybe simply do
> > 
> >   if (gimple_call_builtin_p (g, BUILT_IN_ASAN_AFTER_DYNAMIC_INIT))
> > {
> >   gimple *def = SSA_NAME_DEF_STMT (gimple_vuse (g));
> >   if (gimple_call_builtin_p (def, BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT))
> > ... remove both ...
> > 
> > intervening non-memory-def stmts do not really matter, do they?  If
> > loads matter then checking for the single-vdef-use of
> > BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT to be BUILT_IN_ASAN_AFTER_DYNAMIC_INIT
> > would also work.
> 
> Loads matter too.  What I've been worried about is that some optimizations
> would turn the original
>   __asan_before_dynamic_init ("foobar");
>   ...
>   __asan_after_dynamic_init ();
> into:
>   __asan_before_dynamic_init ("foobar");
>   ...
>   if (...)
> {
>   ...
>   __asan_after_dynamic_init ();
>   return;
> }
>   ...
>   __asan_after_dynamic_init ();
> or something similar and then if removing both on seeing after and its vuse
> before, we'd then keep around the other after call.

The dynamic init pairs form a SESE region initially, right?  Then I
don't see how we'd ever create sth that would mimic the above
but still pass the single-imm-use of the VDEF of the before-dynamic-init
stmt being the after one (in the above example you'd not have a single
use).  So we'd need to have instead

   if (...)
__asan_before_dynamic_init ();
  else
__asan_before_dynamic_init ();
  __asan_after_dynamic_init ();

but even then the single-uses would be the merging PHI.

> Maybe instead of removing both I could just remember both, if it ever finds
> more than one of each kind of calls, punt, and otherwise remove at the end
> of the stmt walk?

> > Otherwise looks ok but I wonder why libasan needs to assert this ..
> 
> It a tiny bit simplifies the code, and of course, a perfect compiler would
> never emit those if there isn't anything to protect, it is just that it is
> hard on the compiler side to guarantee it always.

I believe it's quite impossible even.  What do the expect to happen
in between the two?  I suppose some other asan runtime call?

> // PR sanitizer/77396
> // { dg-do run }
> // { dg-additional-options "-O2" }
> // { dg-set-target-env-var ASAN_OPTIONS "check_initialization_order=true" }
> 
> struct S { S () { asm volatile ("" : : : "memory"); } };
> static S c;
> 
> int
> main ()
> {
>   return 0;
> }
> 
> is what still fails even with the patch.
> 
>   Jakub
> 
> 

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

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

[Eric Botcazou]

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

PR target/77439

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

No, I guess some dg-skip-if would be in order, but I'm not able to devise one.

-- 
Eric Botcazou

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

[Jakub Jelinek]

On Thu, Sep 01, 2016 at 09:39:37AM +0200, Richard Biener wrote:
> > On Thu, Sep 01, 2016 at 08:59:57AM +0200, Richard Biener wrote:
> > > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > > 
> > > Hmm, maybe simply do
> > > 
> > >   if (gimple_call_builtin_p (g, BUILT_IN_ASAN_AFTER_DYNAMIC_INIT))
> > > {
> > >   gimple *def = SSA_NAME_DEF_STMT (gimple_vuse (g));
> > >   if (gimple_call_builtin_p (def, BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT))
> > > ... remove both ...
> > > 
> > > intervening non-memory-def stmts do not really matter, do they?  If
> > > loads matter then checking for the single-vdef-use of
> > > BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT to be BUILT_IN_ASAN_AFTER_DYNAMIC_INIT
> > > would also work.
> > 
> > Loads matter too.  What I've been worried about is that some optimizations
> > would turn the original
> >   __asan_before_dynamic_init ("foobar");
> >   ...
> >   __asan_after_dynamic_init ();
> > into:
> >   __asan_before_dynamic_init ("foobar");
> >   ...
> >   if (...)
> > {
> >   ...
> >   __asan_after_dynamic_init ();
> >   return;
> > }
> >   ...
> >   __asan_after_dynamic_init ();
> > or something similar and then if removing both on seeing after and its vuse
> > before, we'd then keep around the other after call.
> 
> The dynamic init pairs form a SESE region initially, right?  Then I

Yes.

> don't see how we'd ever create sth that would mimic the above
> but still pass the single-imm-use of the VDEF of the before-dynamic-init

Oh right, using single_imm_use of the VDEF should work.

> > It a tiny bit simplifies the code, and of course, a perfect compiler would
> > never emit those if there isn't anything to protect, it is just that it is
> > hard on the compiler side to guarantee it always.
> 
> I believe it's quite impossible even.  What do the expect to happen
> in between the two?  I suppose some other asan runtime call?

They do:

// Lazy-initialized and never deleted.
static VectorOfGlobals *dynamic_init_globals;
...
void __asan_before_dynamic_init(const char *module_name) {
  if (!flags()->check_initialization_order ||
  !CanPoisonMemory())
return;
  bool strict_init_order = flags()->strict_init_order;
  CHECK(dynamic_init_globals);
...
  for (uptr i = 0, n = dynamic_init_globals->size(); i < n; ++i) {

and thus if no globals have been registered, dynamic_init_globals is NULL,
and without the assertion it would crash on dynamic_init_globals->size().
The fix would be just to do:
   if (!flags()->check_initialization_order ||
+  !dynamic_init_globals ||
   !CanPoisonMemory())
 return;
   bool strict_init_order = flags()->strict_init_order;
-  CHECK(dynamic_init_globals);
or so (and similarly in the other function).

Jakub

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

[Richard Biener]

On Thu, 1 Sep 2016, Jakub Jelinek wrote:

> On Thu, Sep 01, 2016 at 09:39:37AM +0200, Richard Biener wrote:
> > > On Thu, Sep 01, 2016 at 08:59:57AM +0200, Richard Biener wrote:
> > > > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > > > 
> > > > Hmm, maybe simply do
> > > > 
> > > >   if (gimple_call_builtin_p (g, BUILT_IN_ASAN_AFTER_DYNAMIC_INIT))
> > > > {
> > > >   gimple *def = SSA_NAME_DEF_STMT (gimple_vuse (g));
> > > >   if (gimple_call_builtin_p (def, 
> > > > BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT))
> > > > ... remove both ...
> > > > 
> > > > intervening non-memory-def stmts do not really matter, do they?  If
> > > > loads matter then checking for the single-vdef-use of
> > > > BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT to be BUILT_IN_ASAN_AFTER_DYNAMIC_INIT
> > > > would also work.
> > > 
> > > Loads matter too.  What I've been worried about is that some optimizations
> > > would turn the original
> > >   __asan_before_dynamic_init ("foobar");
> > >   ...
> > >   __asan_after_dynamic_init ();
> > > into:
> > >   __asan_before_dynamic_init ("foobar");
> > >   ...
> > >   if (...)
> > > {
> > >   ...
> > >   __asan_after_dynamic_init ();
> > >   return;
> > > }
> > >   ...
> > >   __asan_after_dynamic_init ();
> > > or something similar and then if removing both on seeing after and its 
> > > vuse
> > > before, we'd then keep around the other after call.
> > 
> > The dynamic init pairs form a SESE region initially, right?  Then I
> 
> Yes.
> 
> > don't see how we'd ever create sth that would mimic the above
> > but still pass the single-imm-use of the VDEF of the before-dynamic-init
> 
> Oh right, using single_imm_use of the VDEF should work.
> 
> > > It a tiny bit simplifies the code, and of course, a perfect compiler would
> > > never emit those if there isn't anything to protect, it is just that it is
> > > hard on the compiler side to guarantee it always.
> > 
> > I believe it's quite impossible even.  What do the expect to happen
> > in between the two?  I suppose some other asan runtime call?
> 
> They do:
> 
> // Lazy-initialized and never deleted.
> static VectorOfGlobals *dynamic_init_globals;
> ...
> void __asan_before_dynamic_init(const char *module_name) {
>   if (!flags()->check_initialization_order ||
>   !CanPoisonMemory())
> return;
>   bool strict_init_order = flags()->strict_init_order;
>   CHECK(dynamic_init_globals);
> ...
>   for (uptr i = 0, n = dynamic_init_globals->size(); i < n; ++i) {
> 
> and thus if no globals have been registered, dynamic_init_globals is NULL,
> and without the assertion it would crash on dynamic_init_globals->size().
> The fix would be just to do:
>if (!flags()->check_initialization_order ||
> +  !dynamic_init_globals ||
>!CanPoisonMemory())
>  return;
>bool strict_init_order = flags()->strict_init_order;
> -  CHECK(dynamic_init_globals);
> or so (and similarly in the other function).

Ah, so it expects sth _before_ before-dynamic-init?  At least it seems
that globals are not only registered inbetween before/after-dynamic-init.

Maybe simply always init dynamic_init_globals?

Richard.

[PATCH, testsuite]: Use dg-add-options ieee in some _FloatN tests

[Uros Bizjak]

Hello!

Attached patch adds dg-add-options ieee in _FloatN tests where
advanced IEEE features are used (e.g. NaN, infinity, subnormals).

2016-09-01  Uros Bizjak  

* gcc.dg/torture/float128-builtin.c, gcc.dg/torture/float128-floath.c,
gcc.dg/torture/float128-ieee-nan.c, gcc.dg/torture/float128-tg-2.c,
gcc.dg/torture/float128-tg.c, gcc.dg/torture/float128x-builtin.c,
gcc.dg/torture/float128x-floath.c, gcc.dg/torture/float128x-nan.c,
gcc.dg/torture/float128x-tg-2.c, gcc.dg/torture/float128x-tg.c,
gcc.dg/torture/float16-builtin.c, gcc.dg/torture/float16-floath.c,
gcc.dg/torture/float16-nan.c, gcc.dg/torture/float16-tg-2.c,
gcc.dg/torture/float16-tg.c, gcc.dg/torture/float32-builtin.c,
gcc.dg/torture/float32-floath.c, gcc.dg/torture/float32-nan.c,
gcc.dg/torture/float32-tg-2.c, gcc.dg/torture/float32-tg.c,
gcc.dg/torture/float32x-builtin.c, gcc.dg/torture/float32x-floath.c,
gcc.dg/torture/float32x-nan.c, gcc.dg/torture/float32x-tg-2.c,
gcc.dg/torture/float32x-tg.c, gcc.dg/torture/float64-builtin.c,
gcc.dg/torture/float64-floath.c, gcc.dg/torture/float64-nan.c,
gcc.dg/torture/float64-tg-2.c, gcc.dg/torture/float64-tg.c,
gcc.dg/torture/float64x-builtin.c, gcc.dg/torture/float64x-floath.c,
gcc.dg/torture/float64x-nan.c, gcc.dg/torture/float64x-tg-2.c,
gcc.dg/torture/float64x-tg.c: Use dg-add-options ieee.

Tested on alphaev68-linux-gnu (with an ABI correction patch I plan to
propose soon).

OK for mainline?

Uros.
diff --git a/gcc/testsuite/gcc.dg/torture/float128-builtin.c 
b/gcc/testsuite/gcc.dg/torture/float128-builtin.c
index e4a50ce..ea3497c 100644
--- a/gcc/testsuite/gcc.dg/torture/float128-builtin.c
+++ b/gcc/testsuite/gcc.dg/torture/float128-builtin.c
@@ -2,6 +2,7 @@
 /* { dg-do run } */
 /* { dg-options "" } */
 /* { dg-add-options float128 } */
+/* { dg-add-options ieee } */
 /* { dg-require-effective-target float128_runtime } */
 
 #define WIDTH 128
diff --git a/gcc/testsuite/gcc.dg/torture/float128-floath.c 
b/gcc/testsuite/gcc.dg/torture/float128-floath.c
index 68147c3..7b5b046 100644
--- a/gcc/testsuite/gcc.dg/torture/float128-floath.c
+++ b/gcc/testsuite/gcc.dg/torture/float128-floath.c
@@ -2,6 +2,7 @@
 /* { dg-do run } */
 /* { dg-options "" } */
 /* { dg-add-options float128 } */
+/* { dg-add-options ieee } */
 /* { dg-require-effective-target float128_runtime } */
 
 #define WIDTH 128
diff --git a/gcc/testsuite/gcc.dg/torture/float128-ieee-nan.c 
b/gcc/testsuite/gcc.dg/torture/float128-ieee-nan.c
index 5dfbff9..2f3b5c0 100644
--- a/gcc/testsuite/gcc.dg/torture/float128-ieee-nan.c
+++ b/gcc/testsuite/gcc.dg/torture/float128-ieee-nan.c
@@ -2,6 +2,7 @@
 /* { dg-do run } */
 /* { dg-options "-fsignaling-nans" } */
 /* { dg-add-options float128 } */
+/* { dg-add-options ieee } */
 /* { dg-require-effective-target float128_runtime } */
 /* { dg-require-effective-target fenv_exceptions } */
 
diff --git a/gcc/testsuite/gcc.dg/torture/float128-tg-2.c 
b/gcc/testsuite/gcc.dg/torture/float128-tg-2.c
index c7a32b1..eda98e5 100644
--- a/gcc/testsuite/gcc.dg/torture/float128-tg-2.c
+++ b/gcc/testsuite/gcc.dg/torture/float128-tg-2.c
@@ -2,6 +2,7 @@
 /* { dg-do run } */
 /* { dg-options "" } */
 /* { dg-add-options float128 } */
+/* { dg-add-options ieee } */
 /* { dg-require-effective-target float128_runtime } */
 
 #define WIDTH 128
diff --git a/gcc/testsuite/gcc.dg/torture/float128-tg.c 
b/gcc/testsuite/gcc.dg/torture/float128-tg.c
index c1b6398..d252ec5 100644
--- a/gcc/testsuite/gcc.dg/torture/float128-tg.c
+++ b/gcc/testsuite/gcc.dg/torture/float128-tg.c
@@ -2,6 +2,7 @@
 /* { dg-do run } */
 /* { dg-options "" } */
 /* { dg-add-options float128 } */
+/* { dg-add-options ieee } */
 /* { dg-require-effective-target float128_runtime } */
 
 #define WIDTH 128
diff --git a/gcc/testsuite/gcc.dg/torture/float128x-builtin.c 
b/gcc/testsuite/gcc.dg/torture/float128x-builtin.c
index 2e6bbaf..d75bc55 100644
--- a/gcc/testsuite/gcc.dg/torture/float128x-builtin.c
+++ b/gcc/testsuite/gcc.dg/torture/float128x-builtin.c
@@ -2,6 +2,7 @@
 /* { dg-do run } */
 /* { dg-options "" } */
 /* { dg-add-options float128x } */
+/* { dg-add-options ieee } */
 /* { dg-require-effective-target float128x_runtime } */
 
 #define WIDTH 128
diff --git a/gcc/testsuite/gcc.dg/torture/float128x-floath.c 
b/gcc/testsuite/gcc.dg/torture/float128x-floath.c
index 0fc3db2..3ac96ce 100644
--- a/gcc/testsuite/gcc.dg/torture/float128x-floath.c
+++ b/gcc/testsuite/gcc.dg/torture/float128x-floath.c
@@ -2,6 +2,7 @@
 /* { dg-do run } */
 /* { dg-options "" } */
 /* { dg-add-options float128x } */
+/* { dg-add-options ieee } */
 /* { dg-require-effective-target float128x_runtime } */
 
 #define WIDTH 128
diff --git a/gcc/testsuite/gcc.dg/torture/float128x-nan.c 
b/gcc/testsuite/gcc.dg/torture/float128x-nan.c
index ad0052f..c3eaf12 100644
--- a/gcc/testsuite/gcc.dg/torture/float128x-nan.c
+++ b/gcc/testsuite/gcc.dg/torture/float128x-nan.c
@@ -2,6 +2,7 @@
 /* { dg-do run } */
 /* { d

Re: [PATCH][Aarch64][gcc] Fix vld2/3/4 on big endian systems

[Christophe Lyon]

On 30 August 2016 at 18:45, Tamar Christina  wrote:
>
>
> On 30/08/16 17:11, Christophe Lyon wrote:
>>
>> On 18 August 2016 at 11:15, Tamar Christina 
>> wrote:
>>>
>>> Hi all,
>>>
>>> This fixes a bug in the vector load functions in which they load the
>>> vector in the wrong order for big endian systems. This patch flips the
>>> order conditionally in the vec_concats.
>>>
>>> No testcase given because plenty of existing tests for vld functions.
>>> Ran regression tests on aarch64_be-none-elf and aarch64-none-elf.
>>> Vldx tests now pass on aarch64_be-none-elf and no regressions on both.
>>>
>> Before your patch, I can see aarch64/vldN_1.c and
>> aarch64/advsimd-intrinsics/vldX_lane.c failing.
>>
>> Do you know why aarch64/advsimd-intrinsics/vldX.c and vldX_dup
>> are not failing?
>
> That's weird, on my clean build and our nightlies I see
>
> aarch64/advsimd-intrinsics/vldX.c, aarch64/advsimd-intrinsics/vtbX.c and
> aarch64/vldN_1.c failing but
> aarch64/advsimd-intrinsics/vldX_lane.c is passing.
>
> I don't know why the vldX_lane was passing, but I'll look into the
> discrepancy.
>

Hmmm I must have looked at the wrong place :(

Your patch does fix vtbX, vldX and vldN_1 tests.

Sorry for the noise.

Christophe

> Cheers,
> Tamar
>
>> Thanks
>> Christophe
>>
>>> Ok for trunk?
>>>
>>> I do not have commit rights so if ok can someone apply it for me?
>>>
>>> Thanks,
>>> Tamar
>>>
>>> gcc/
>>> 2016-08-16  Tamar Christina  
>>>
>>>  * gcc/config/aarch64/aarch64-simd.md
>>>  (aarch64_ld2_dreg_le): New.
>>>  (aarch64_ld2_dreg_be): New.
>>>  (aarch64_ld2_dreg): Removed.
>>>  (aarch64_ld3_dreg_le): New.
>>>  (aarch64_ld3_dreg_be): New.
>>>  (aarch64_ld3_dreg): Removed.
>>>  (aarch64_ld4_dreg_le): New.
>>>  (aarch64_ld4_dreg_be): New.
>>>  (aarch64_ld4_dreg): Removed.
>>>  (aarch64_ld): Wrapper around _le, _be.
>
>

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

[Jakub Jelinek]

On Thu, Sep 01, 2016 at 09:58:44AM +0200, Richard Biener wrote:
> > and thus if no globals have been registered, dynamic_init_globals is NULL,
> > and without the assertion it would crash on dynamic_init_globals->size().
> > The fix would be just to do:
> >if (!flags()->check_initialization_order ||
> > +  !dynamic_init_globals ||
> >!CanPoisonMemory())
> >  return;
> >bool strict_init_order = flags()->strict_init_order;
> > -  CHECK(dynamic_init_globals);
> > or so (and similarly in the other function).
> 
> Ah, so it expects sth _before_ before-dynamic-init?  At least it seems
> that globals are not only registered inbetween before/after-dynamic-init.

Yes, usually __asan_register_globals is called before
__asan_before_dynamic_init, the former in the constructor created during
asan_finish_file very late during compilation.  dynamic_init_globals is
set to non-NULL only when registering the first global with the
__has_dynamic_init flag:
  if (g->has_dynamic_init) {
if (!dynamic_init_globals) {
  dynamic_init_globals = new(allocator_for_globals)
  VectorOfGlobals(kDynamicInitGlobalsInitialCapacity);
}
...
That flag corresponds to a varpool node with
vnode->dynamically_initialized

In any case, I'll discuss with upstream.  Perhaps LLVM never removes
local statics or local statics with former dynamic initialization or
something similar.

Jakub

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

[Iain Sandoe]

Hi,

> On 1 Sep 2016, at 01:49, Segher Boessenkool  
> wrote:

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

Here is the patch to fix up Darwin,
I guess it’s both Darwin-local and reasonably obvious now, but OK?
Iain

[PATCH] rs6000,Darwin: Remove uses of LR in restore_world.

Darwin had an additional use of LR in the restore_world machinery.  This patch 
removes it from the pattern in altivec.md and the relevant predicate.  Many 
thanks to Segher for detailed pointers to the solutions.

2016-09-01  Iain Sandoe  
Segher Boessenkool  

* config/rs6000/altivec.md (*restore_world): Remove LR use.
* config/rs6000/predicates.md (restore_world_operation): Adjust count 
op count, remove one USE.
---
 gcc/config/rs6000/altivec.md| 1 -
 gcc/config/rs6000/predicates.md | 3 +--
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
index c39a0b6..3f7312e 100644
--- a/gcc/config/rs6000/altivec.md
+++ b/gcc/config/rs6000/altivec.md
@@ -407,7 +407,6 @@
 (define_insn "*restore_world"
  [(match_parallel 0 "restore_world_operation"
   [(return)
-  (use (reg:SI 65))
(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"
diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
index a7d66ad..7d0a6d6 100644
--- a/gcc/config/rs6000/predicates.md
+++ b/gcc/config/rs6000/predicates.md
@@ -1332,13 +1332,12 @@
   rtx elt;
   int count = XVECLEN (op, 0);
 
-  if (count != 59)
+  if (count != 58)
 return 0;
 
   index = 0;
   if (GET_CODE (XVECEXP (op, 0, index++)) != RETURN
   || GET_CODE (XVECEXP (op, 0, index++)) != USE
-  || GET_CODE (XVECEXP (op, 0, index++)) != USE
   || GET_CODE (XVECEXP (op, 0, index++)) != CLOBBER)
 return 0;
 
-- 
2.8.1

[PATCH][AArch64] Add ANDS pattern for CMP+ZERO_EXTEND

[Kyrill Tkachov]

Hi all,

For the testcase in this patch combine tries to match:
(parallel [
(set (reg:CC 66 cc)
(compare:CC (zero_extend:SI (reg:QI 0 x0 [ x ]))
(const_int 0 [0])))
(set (reg/v:SI 77 [ x ])
(zero_extend:SI (reg:QI 0 x0 [ x ])))
])

which we should be matching down to an ANDS instruction. We already have a 
pattern for matching
the AND-immediate form of this. Teaching combine to jump between the two 
representations has in the past
been deemed undesirable (extending change_zero_ext in combine.c would increase 
the work it has to do by too much)
so I think it's much simpler to just add this pattern.

With this patch for the testcase in the patch we now generate:
f9:
andsw0, w0, 255
mov w2, 10
ccmpw1, 1, 4, eq
cselw0, w0, w2, le
ret


instead of:
f9:
uxtbw0, w0
mov w2, 10
cmp w0, 0
ccmpw1, 1, 4, eq
cselw0, w0, w2, le
ret

Bootstrapped and tested on aarch64-none-linux-gnu.
Ok for trunk?

Thanks,
Kyrill

2016-09-01  Kyrylo Tkachov  

* config/aarch64/aarch64.md (*ands_compare0): New pattern.
* config/aarch64/aarch64.c (aarch64_select_cc_mode): Return CC_NZmode
for comparisons of integer ZERO_EXTEND against zero.

2016-09-01  Kyrylo Tkachov  

* gcc.target/aarch64/ands_3.c: New test.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index ed5910aaf5f183bb068e31c85698a509cb8a10bc..43442a44a559f3191a10d8d3ce6b82cff0a41e1a 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -4264,6 +4264,14 @@ aarch64_select_cc_mode (RTX_CODE code, rtx x, rtx y)
   && (GET_MODE (x) == HImode || GET_MODE (x) == QImode))
 return CC_NZmode;
 
+  /* Similarly, comparisons of zero_extends from shorter modes can
+ be performed using and ANDS with an immediate mask.  */
+  if (y == const0_rtx && GET_CODE (x) == ZERO_EXTEND
+  && (GET_MODE (x) == SImode || GET_MODE (x) == DImode)
+  && (GET_MODE (XEXP (x, 0)) == HImode || GET_MODE (XEXP (x, 0)) == QImode)
+  && (code == EQ || code == NE))
+return CC_NZmode;
+
   if ((GET_MODE (x) == SImode || GET_MODE (x) == DImode)
   && y == const0_rtx
   && (code == EQ || code == NE || code == LT || code == GE)
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 137aed42c6b10511e7f558c9df65a73144f4bb94..68b715c9a7298f02e47abd1cbe146346ad1c3e30 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -3826,6 +3826,18 @@ (define_insn "*and_compare0"
   [(set_attr "type" "alus_imm")]
 )
 
+(define_insn "*ands_compare0"
+  [(set (reg:CC_NZ CC_REGNUM)
+	(compare:CC_NZ
+	 (zero_extend:GPI (match_operand:SHORT 1 "register_operand" "r"))
+	 (const_int 0)))
+   (set (match_operand:GPI 0 "register_operand" "=r")
+	(zero_extend:GPI (match_dup 1)))]
+  ""
+  "ands\\t%0, %1, "
+  [(set_attr "type" "alus_imm")]
+)
+
 (define_insn "*and3nr_compare0"
   [(set (reg:CC_NZ CC_REGNUM)
 	(compare:CC_NZ
diff --git a/gcc/testsuite/gcc.target/aarch64/ands_3.c b/gcc/testsuite/gcc.target/aarch64/ands_3.c
new file mode 100644
index ..42cb7f0f0bc86a4aceb09851c31eb2e888d93403
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ands_3.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+int
+f9 (unsigned char x, int y)
+{
+  if (y > 1 && x == 0)
+return 10;
+  return x;
+}
+
+/* { dg-final { scan-assembler "ands\t(x|w)\[0-9\]+,\[ \t\]*(x|w)\[0-9\]+,\[ \t\]*255" } } */

Re: [PATCH][AArch64] Add ANDS pattern for CMP+ZERO_EXTEND

[Richard Earnshaw (lists)]

On 01/09/16 09:50, Kyrill Tkachov wrote:
> Hi all,
> 
> For the testcase in this patch combine tries to match:
> (parallel [
> (set (reg:CC 66 cc)
> (compare:CC (zero_extend:SI (reg:QI 0 x0 [ x ]))
> (const_int 0 [0])))
> (set (reg/v:SI 77 [ x ])
> (zero_extend:SI (reg:QI 0 x0 [ x ])))
> ])
> 
> which we should be matching down to an ANDS instruction. We already have
> a pattern for matching
> the AND-immediate form of this. Teaching combine to jump between the two
> representations has in the past
> been deemed undesirable (extending change_zero_ext in combine.c would
> increase the work it has to do by too much)
> so I think it's much simpler to just add this pattern.
> 
> With this patch for the testcase in the patch we now generate:
> f9:
> andsw0, w0, 255
> mov w2, 10
> ccmpw1, 1, 4, eq
> cselw0, w0, w2, le
> ret
> 
> 
> instead of:
> f9:
> uxtbw0, w0
> mov w2, 10
> cmp w0, 0
> ccmpw1, 1, 4, eq
> cselw0, w0, w2, le
> ret
> 
> Bootstrapped and tested on aarch64-none-linux-gnu.
> Ok for trunk?
> 
> Thanks,
> Kyrill
> 
> 2016-09-01  Kyrylo Tkachov  
> 
> * config/aarch64/aarch64.md (*ands_compare0): New pattern.
> * config/aarch64/aarch64.c (aarch64_select_cc_mode): Return CC_NZmode
> for comparisons of integer ZERO_EXTEND against zero.
> 
> 2016-09-01  Kyrylo Tkachov  
> 
> * gcc.target/aarch64/ands_3.c: New test.
> 

OK.

R.

> aarch64-cmp-ands.patch
> 
> 
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 
> ed5910aaf5f183bb068e31c85698a509cb8a10bc..43442a44a559f3191a10d8d3ce6b82cff0a41e1a
>  100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -4264,6 +4264,14 @@ aarch64_select_cc_mode (RTX_CODE code, rtx x, rtx y)
>&& (GET_MODE (x) == HImode || GET_MODE (x) == QImode))
>  return CC_NZmode;
>  
> +  /* Similarly, comparisons of zero_extends from shorter modes can
> + be performed using and ANDS with an immediate mask.  */
> +  if (y == const0_rtx && GET_CODE (x) == ZERO_EXTEND
> +  && (GET_MODE (x) == SImode || GET_MODE (x) == DImode)
> +  && (GET_MODE (XEXP (x, 0)) == HImode || GET_MODE (XEXP (x, 0)) == 
> QImode)
> +  && (code == EQ || code == NE))
> +return CC_NZmode;
> +
>if ((GET_MODE (x) == SImode || GET_MODE (x) == DImode)
>&& y == const0_rtx
>&& (code == EQ || code == NE || code == LT || code == GE)
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 
> 137aed42c6b10511e7f558c9df65a73144f4bb94..68b715c9a7298f02e47abd1cbe146346ad1c3e30
>  100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -3826,6 +3826,18 @@ (define_insn "*and_compare0"
>[(set_attr "type" "alus_imm")]
>  )
>  
> +(define_insn "*ands_compare0"
> +  [(set (reg:CC_NZ CC_REGNUM)
> + (compare:CC_NZ
> +  (zero_extend:GPI (match_operand:SHORT 1 "register_operand" "r"))
> +  (const_int 0)))
> +   (set (match_operand:GPI 0 "register_operand" "=r")
> + (zero_extend:GPI (match_dup 1)))]
> +  ""
> +  "ands\\t%0, %1, "
> +  [(set_attr "type" "alus_imm")]
> +)
> +
>  (define_insn "*and3nr_compare0"
>[(set (reg:CC_NZ CC_REGNUM)
>   (compare:CC_NZ
> diff --git a/gcc/testsuite/gcc.target/aarch64/ands_3.c 
> b/gcc/testsuite/gcc.target/aarch64/ands_3.c
> new file mode 100644
> index 
> ..42cb7f0f0bc86a4aceb09851c31eb2e888d93403
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/ands_3.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +int
> +f9 (unsigned char x, int y)
> +{
> +  if (y > 1 && x == 0)
> +return 10;
> +  return x;
> +}
> +
> +/* { dg-final { scan-assembler "ands\t(x|w)\[0-9\]+,\[ \t\]*(x|w)\[0-9\]+,\[ 
> \t\]*255" } } */
> 

Re: [PATCH] Improvements to typed_splay_tree (v2)

[Bernd Schmidt]

On 09/01/2016 03:01 AM, David Malcolm wrote:

On Wed, 2016-08-31 at 16:12 +0200, Bernd Schmidt wrote:

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?


Sure.

Bernd

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

[Richard Earnshaw (lists)]

On 01/09/16 08:47, Eric Botcazou wrote:
>> Since you're going to need a back-port there should be a PR filed for this.
> 
> PR target/77439
> 
>> 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.
> 
> No, I guess some dg-skip-if would be in order, but I'm not able to devise one.
> 

Since the test is an execution test, why not just drop the problematic
parts of dg-options "-mfpu=vfp -mfloat-abi=softfp" and rely on multilib
permutations as appropriate.  That gives more all-round coverage as well.

R.

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

[Richard Earnshaw (lists)]

On 01/09/16 10:03, Richard Earnshaw (lists) wrote:
> On 01/09/16 08:47, Eric Botcazou wrote:
>>> Since you're going to need a back-port there should be a PR filed for this.
>>
>> PR target/77439
>>
>>> 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.
>>
>> No, I guess some dg-skip-if would be in order, but I'm not able to devise 
>> one.
>>
> 
> Since the test is an execution test, why not just drop the problematic
> parts of dg-options "-mfpu=vfp -mfloat-abi=softfp" and rely on multilib
> permutations as appropriate.  That gives more all-round coverage as well.
> 
> R.
> 

That would also allow you to drop the effective-target test as well.

R.

Re: [doc, SPU] Remove references to sites no longer carrying SPU information from doc/extend.texi

[Gerald Pfeifer]

On Sat, 27 Aug 2016, Gerald Pfeifer wrote:
> 2016-08-27  Gerald Pfeifer  
> 
>   * doc/extend.texi (SPU Built-in Functions): Remove stale
>   references to material formerly at IBM and Sony.

Now also pushed back to the GCC 6 branch.

Gerald

Re: PR35503 - warn for restrict pointer

[Prathamesh Kulkarni]

On 1 September 2016 at 12:25, Richard Biener  wrote:
> 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.
Hi,
The attached version passes bootstrap+test on ppc64le-linux-gnu.
Given that it only looks if parameters are restrict qualified and not
how they're used inside the callee,
this can have false positives as in above test-cases.
Should the warning be put in Wextra rather than Wall (I have left it
in Wall in the patch)  or only enabled with -Wrestrict ?

Thanks,
Prathamesh
>
> Richard.
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 3feb910..a3dae34 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -47,6 +47,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "gimplify.h"
 #include "substring-locations.h"
 #include "spellcheck.h"
+#include "gcc-rich-location.h"
 
 cpp_reader *parse_in;  /* Declared in c-pragma.h.  */
 
@@ -13057,4 +13058,76 @@ diagnose_mismatched_attributes (tree olddecl, tree 
newdecl)
   return warned;
 }
 
+/* Warn if an a

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

[Andreas Schwab]

On Aug 31 2016, Jerry DeLisle  wrote:

> ! { dg-do run }
> ! PR77393
> program testbigf0 ! Can enormous numbers be printed with F0.0 format?
>   implicit none
>   character(1) :: str
>   write(str, "(f0.0)") -huge(1.0) 
>   if (len(trim(str)).lt.41) error stop "FAILED AT 9"
>   write(str, "(f0.0)") -huge(1.0_8)
>   if (len(trim(str)).lt.311) error stop "FAILED AT 9"
>   write(str, "(f0.0)") -huge(1.0_10)
>   if (len(trim(str)).lt.4935) error stop "FAILED AT 9"
>   write(str, "(f0.10)") -huge(1.0_16)
>   if (len(trim(str)).lt.4945) error stop "FAILED AT 11"
> end program testbigf0

FAIL: gfortran.dg/fmt_f0_2.f90   -O0  (test for excess errors)
Excess errors:
/daten/aranym/gcc/gcc-20160901/gcc/testsuite/gfortran.dg/fmt_f0_2.f90:12:36: 
Error: Invalid real kind 16 at (1)

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: Implement C _FloatN, _FloatNx types [version 6]

[Szabolcs Nagy]

On 31/08/16 18:26, Joseph Myers wrote:
> 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 

the goal was not to provide full support for some TS, but to
provide reasonable access to half precision floats from c.

it seems that the FLT_EVAL_METHOD change in TS 18661 was a
bad decision: existing code have to be modified to conform
to the new semantics. this situation could have been avoided
by introducing new macros or only changing behaviour when
the user requested it via __STDC_WANT_IEC_60559_TYPES_EXT__
(consistently with all other library changes..).

i think it makes sense to provide half precision support
on older c standards, so gcc should allow _Float16 without
excess precision and whatever __FLT_EVAL_METHOD setting
based on some cmd line option, so users can work around
this mess. (FLT_EVAL_METHOD < 0 would be fully conforming,
but it can still break existing code). user code can use
sizeof(_Float16_t), libc can get the real eval method
through some additional gcc specific macro.

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

[Patch, testsuite] Fix more bogus failures for avr

[Senthil Kumar Selvaraj]

Hi,

  The below patch fixes some bogus testsuite failures for the avr
  target. The first three expect 32 bit ints, and the last one uses too
  much RAM for a typical avr device.

  Regtested with avr and x86_64-pc-linux, and committed to trunk.

Regards
Senthil


gcc/testsuite/ChangeLog

2016-09-01  Senthil Kumar Selvaraj  

* gcc.dg/pr64252.c: Require int32plus.
* gcc.dg/pr66299-1.c: Likewise.
* gcc.dg/pr66299-2.c: Likewise.
* gcc.dg/torture/20131115-1.c: Skip for avr.

Index: gcc/testsuite/gcc.dg/pr64252.c
===
--- gcc/testsuite/gcc.dg/pr64252.c  (revision 239920)
+++ gcc/testsuite/gcc.dg/pr64252.c  (working copy)
@@ -1,6 +1,7 @@
 /* PR target/64252 */
 /* { dg-do run } */
 /* { dg-options "-O2" } */
+/* { dg-require-effective-target int32plus } */
 
 typedef unsigned int V __attribute__((vector_size (32)));
 
Index: gcc/testsuite/gcc.dg/pr66299-1.c
===
--- gcc/testsuite/gcc.dg/pr66299-1.c(revision 239920)
+++ gcc/testsuite/gcc.dg/pr66299-1.c(working copy)
@@ -1,6 +1,7 @@
 /* PR tree-optimization/66299 */
 /* { dg-do run } */
 /* { dg-options "-fdump-tree-original" } */
+/* { dg-require-effective-target int32plus } */
 
 void
 test1 (int x)
Index: gcc/testsuite/gcc.dg/pr66299-2.c
===
--- gcc/testsuite/gcc.dg/pr66299-2.c(revision 239920)
+++ gcc/testsuite/gcc.dg/pr66299-2.c(working copy)
@@ -1,6 +1,7 @@
 /* PR tree-optimization/66299 */
 /* { dg-do run } */
 /* { dg-options "-fdump-tree-optimized -O" } */
+/* { dg-require-effective-target int32plus } */
 
 void
 test1 (int x, unsigned u)
Index: gcc/testsuite/gcc.dg/torture/20131115-1.c
===
--- gcc/testsuite/gcc.dg/torture/20131115-1.c   (revision 239920)
+++ gcc/testsuite/gcc.dg/torture/20131115-1.c   (working copy)
@@ -1,4 +1,5 @@
 /* { dg-do run } */
+/* { dg-skip-if "RAM usage too large" { "avr-*-*" } } */
 
 struct S { int i; };
 __attribute__((const, noinline, noclone))

Re: [patch] Fix segfault in param_change_prob

[Richard Biener]

On Wed, Aug 31, 2016 at 8:46 PM, Eric Botcazou  wrote:
> Hi,
>
> the attached Ada testcase triggers a segfault in param_change_prob because the
> parameter is VIEW_CONVERT_EXPR (or  it fools the logic and walk_aliased_vdefs is called on a NULL second argument
> as the call is to a pure function.  The proposed fix is just to strip the VCE.
>
> Tested on x86_64-suse-linux, OK for the mainline?

I think the fix is not enough if op is for example MEM[&"foo"] (no
V_C_E) then get_base_address
returns a STRING_CST and you run into exactly the same issue.

So I think it's better to apply get_base_address here rather than only
stripping VIEW_CONVERT_EXRPs.
(beware of it returning NULL_TREE for WITH_SIZE_EXPRs, thus maybe
strip those first as well)

You could after all have COMPONENT_REF> as well ...

Richard.

>
> 2016-08-31  Eric Botcazou  
>
> * 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 Botcazou

[committed] Backports to 6.x branch

[Jakub Jelinek]

Hi!

I've committed following backports of some trunk fixes,
after bootstrapping/regtesting them on x86_64-linux and i686-linux.

Jakub
2016-09-01  Jakub Jelinek  

Backported from mainline
2016-08-16  Jakub Jelinek  

PR target/71910
* tree-cfg.c (execute_fixup_cfg): Add node variable, use it.  Before 
inlining,
add cgraph edge for the added __builtin_unreachable call.

* g++.dg/gomp/pr71910.C: New test.

--- gcc/tree-cfg.c  (revision 239507)
+++ gcc/tree-cfg.c  (revision 239508)
@@ -8987,16 +8987,14 @@ execute_fixup_cfg (void)
   gcov_type count_scale;
   edge e;
   edge_iterator ei;
+  cgraph_node *node = cgraph_node::get (current_function_decl);
 
   count_scale
-  = GCOV_COMPUTE_SCALE (cgraph_node::get (current_function_decl)->count,
-   ENTRY_BLOCK_PTR_FOR_FN (cfun)->count);
+= GCOV_COMPUTE_SCALE (node->count, ENTRY_BLOCK_PTR_FOR_FN (cfun)->count);
 
-  ENTRY_BLOCK_PTR_FOR_FN (cfun)->count =
-   cgraph_node::get (current_function_decl)->count;
-  EXIT_BLOCK_PTR_FOR_FN (cfun)->count =
-   apply_scale (EXIT_BLOCK_PTR_FOR_FN (cfun)->count,
-   count_scale);
+  ENTRY_BLOCK_PTR_FOR_FN (cfun)->count = node->count;
+  EXIT_BLOCK_PTR_FOR_FN (cfun)->count
+= apply_scale (EXIT_BLOCK_PTR_FOR_FN (cfun)->count, count_scale);
 
   FOR_EACH_EDGE (e, ei, ENTRY_BLOCK_PTR_FOR_FN (cfun)->succs)
 e->count = apply_scale (e->count, count_scale);
@@ -9089,10 +9087,19 @@ execute_fixup_cfg (void)
{
  if (stmt && is_gimple_call (stmt))
gimple_call_set_ctrl_altering (stmt, false);
- stmt = gimple_build_call
- (builtin_decl_implicit (BUILT_IN_UNREACHABLE), 0);
+ tree fndecl = builtin_decl_implicit (BUILT_IN_UNREACHABLE);
+ stmt = gimple_build_call (fndecl, 0);
  gimple_stmt_iterator gsi = gsi_last_bb (bb);
  gsi_insert_after (&gsi, stmt, GSI_NEW_STMT);
+ if (!cfun->after_inlining)
+   {
+ gcall *call_stmt = dyn_cast  (stmt);
+ int freq
+   = compute_call_stmt_bb_frequency (current_function_decl,
+ bb);
+ node->create_edge (cgraph_node::get_create (fndecl),
+call_stmt, bb->count, freq);
+   }
}
}
 }
--- gcc/testsuite/g++.dg/gomp/pr71910.C (revision 0)
+++ gcc/testsuite/g++.dg/gomp/pr71910.C (revision 239508)
@@ -0,0 +1,13 @@
+// PR target/71910
+// { dg-do compile }
+// { dg-additional-options "-O2" }
+
+#include 
+
+int
+main ()
+{
+  std::vector vec(10);
+#pragma omp parallel
+  __builtin_exit (0);
+}
2016-09-01  Jakub Jelinek  

Backported from mainline
2016-08-17  Jakub Jelinek  

PR middle-end/77259
* tree-ssa-pre.c (eliminate_dom_walker::before_dom_children): If
turning a call into __builtin_unreachable-like noreturn call, adjust
gimple_call_set_fntype.
* tree-cfgcleanup.c (fixup_noreturn_call): Remove lhs also if
gimple_call_fntype has void return type.

* g++.dg/ipa/devirt-52.C: New test.

--- gcc/tree-cfgcleanup.c   (revision 239536)
+++ gcc/tree-cfgcleanup.c   (revision 239537)
@@ -602,10 +602,15 @@ fixup_noreturn_call (gimple *stmt)
   /* If there is an LHS, remove it, but only if its type has fixed size.
  The LHS will need to be recreated during RTL expansion and creating
  temporaries of variable-sized types is not supported.  Also don't
- do this with TREE_ADDRESSABLE types, as assign_temp will abort.  */
+ do this with TREE_ADDRESSABLE types, as assign_temp will abort.
+ Drop LHS regardless of TREE_ADDRESSABLE, if the function call
+ has been changed into a call that does not return a value, like
+ __builtin_unreachable or __cxa_pure_virtual.  */
   tree lhs = gimple_call_lhs (stmt);
-  if (lhs && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs))) == INTEGER_CST
-  && !TREE_ADDRESSABLE (TREE_TYPE (lhs)))
+  if (lhs
+  && ((TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs))) == INTEGER_CST
+  && !TREE_ADDRESSABLE (TREE_TYPE (lhs)))
+ || VOID_TYPE_P (TREE_TYPE (gimple_call_fntype (stmt)
 {
   gimple_call_set_lhs (stmt, NULL_TREE);
 
--- gcc/tree-ssa-pre.c  (revision 239536)
+++ gcc/tree-ssa-pre.c  (revision 239537)
@@ -4543,6 +4543,15 @@ eliminate_dom_walker::before_dom_childre
   lang_hooks.decl_printable_name (fn, 2));
}
  gimple_call_set_fndecl (call_stmt, fn);
+ /* If changing the call to __builtin_unreachable
+or similar noreturn function, adjust gimple_call_fntype
+too.  */
+ if (gimple_call_noreturn_p (call_stmt)
+  

[PTX] cbranch expanders

[Nathan Sidwell]

I noticed the conditional branch expanders unnecessarily forced alll ops to be 
registers.  Of course later optimization fixes that up, but why create the 
problem in the first place?


committed to trunk.

nathan

Re: [PATCH, docs] invoke.texi: random copy-editing

[Gerald Pfeifer]

Hi Sandra,

On Wed, 29 Aug 2012, Sandra Loosemore wrote:
> I've had this largish pile of random copy-edits to invoke.texi left over 
> from my previous passes through that file earlier this year.

that was an amazing amount of changes; I admire your patience and
thoroughness!

> I'll wait a few days before committing to give folks a chance to object 
> and/or volunteer to review the whole patch.  ;-)

That was more like a few years, but I did go through the patch. :-o

On thing I noticed is that you converted "nop" to "NOP", is that
a standard you generally suggest to establish?  If so, I've got a 
couple more cases.

Let me know, and I'll apply this patch.

Gerald


2016-09-01  Gerald Pfeifer  

* doc/invoke.texi (SPU Options): nops -> NOPs.
(x86 Options): Ditto.

Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 239904)
+++ doc/invoke.texi (working copy)
@@ -22807,16 +22807,16 @@
 @item -mdual-nops
 @itemx -mdual-nops=@var{n}
 @opindex mdual-nops
-By default, GCC inserts nops to increase dual issue when it expects
+By default, GCC inserts NOPs to increase dual issue when it expects
 it to increase performance.  @var{n} can be a value from 0 to 10.  A
-smaller @var{n} inserts fewer nops.  10 is the default, 0 is the
+smaller @var{n} inserts fewer NOPs.  10 is the default, 0 is the
 same as @option{-mno-dual-nops}.  Disabled with @option{-Os}.
 
 @item -mhint-max-nops=@var{n}
 @opindex mhint-max-nops
-Maximum number of nops to insert for a branch hint.  A branch hint must
+Maximum number of NOPs to insert for a branch hint.  A branch hint must
 be at least 8 instructions away from the branch it is affecting.  GCC
-inserts up to @var{n} nops to enforce this, otherwise it does not
+inserts up to @var{n} NOPs to enforce this, otherwise it does not
 generate the branch hint.
 
 @item -mhint-max-distance=@var{n}
@@ -24601,7 +24601,7 @@
 @itemx -mno-nop-mcount
 @opindex mnop-mcount
 If profiling is active (@option{-pg}), generate the calls to
-the profiling functions as nops. This is useful when they
+the profiling functions as NOPs. This is useful when they
 should be patched in later dynamically. This is likely only
 useful together with @option{-mrecord-mcount}.
 

Re: [PATCH, PR70955] Tag {ms,sysv}_va_list_type_node with {ms,sysv}_abi attribute

[Markus Trippelsdorf]

On 2016.08.26 at 17:33 +0200, Tom de Vries wrote:
> On 26/08/16 10:40, Richard Biener wrote:
> >I suppose
> >you could get away with using sth entirely private to the backend,
> >like "sysv abi valist", also not ever user-creatable.  Does this
> >side-step the FE issue?
> 
> It does.
> 
> Added comments and second test-case, as requested previously.
> 
> Bootstrapped and reg-tested on x86_64 -m64/-m32.
> 
> OK for trunk, 6 branch?

The patch causes: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77427

-- 
Markus

Re: [PATCH][AArch64 - v2] Simplify eh_return implementation

[Wilco Dijkstra]

 
Ping

I noticed it would still be a good idea to add an extra barrier in the epilog 
as the
scheduler doesn't appear to handle aliases of frame accesses properly.

This patch simplifies the handling of the EH return value.  We force the use of 
the
frame pointer so the return location is always at FP + 8.  This means we can 
emit
a simple volatile access in EH_RETURN_HANDLER_RTX without needing md
patterns, splitters and frame offset calculations.  The new implementation also
fixes various bugs in aarch64_final_eh_return_addr, which does not work with
-fomit-frame-pointer, alloca or outgoing arguments.

Bootstrap OK, GCC Regression OK, OK for trunk? Would it be useful to backport
this to GCC6.x?

ChangeLog:
2016-08-10  Wilco Dijkstra  
gcc/
    * config/aarch64/aarch64.md (eh_return): Remove pattern and splitter.
    * config/aarch64/aarch64.h (AARCH64_EH_STACKADJ_REGNUM): Remove.
    (EH_RETURN_HANDLER_RTX): New define.
    * config/aarch64/aarch64.c (aarch64_frame_pointer_required):
    Force frame pointer in EH return functions.
    (aarch64_expand_epilogue): Add barrier for eh_return.
    (aarch64_final_eh_return_addr): Remove.
    (aarch64_eh_return_handler_rtx): New function.
    * config/aarch64/aarch64-protos.h (aarch64_final_eh_return_addr):
    Remove.
    (aarch64_eh_return_handler_rtx): New prototype.
--

diff --git a/gcc/config/aarch64/aarch64-protos.h 
b/gcc/config/aarch64/aarch64-protos.h
index 
3cdd69b8af1089a839e5d45cda94bc70a15cd777..327c0a97f6f687604afef249b79ac22628418070
 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -358,7 +358,7 @@ int aarch64_hard_regno_mode_ok (unsigned, machine_mode);
 int aarch64_hard_regno_nregs (unsigned, machine_mode);
 int aarch64_uxt_size (int, HOST_WIDE_INT);
 int aarch64_vec_fpconst_pow_of_2 (rtx);
-rtx aarch64_final_eh_return_addr (void);
+rtx aarch64_eh_return_handler_rtx (void);
 rtx aarch64_mask_from_zextract_ops (rtx, rtx);
 const char *aarch64_output_move_struct (rtx *operands);
 rtx aarch64_return_addr (int, rtx);
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 
003fec87e41db618570663f28cc2387a87e8252a..fa81e4b853daf08842955288861ec7e7acca
 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -400,9 +400,9 @@ extern unsigned aarch64_architecture_version;
 #define ASM_DECLARE_FUNCTION_NAME(STR, NAME, DECL)  \
   aarch64_declare_function_name (STR, NAME, DECL)
 
-/* The register that holds the return address in exception handlers.  */
-#define AARCH64_EH_STACKADJ_REGNUM (R0_REGNUM + 4)
-#define EH_RETURN_STACKADJ_RTX gen_rtx_REG (Pmode, AARCH64_EH_STACKADJ_REGNUM)
+/* For EH returns X4 contains the stack adjustment.  */
+#define EH_RETURN_STACKADJ_RTX gen_rtx_REG (Pmode, R4_REGNUM)
+#define EH_RETURN_HANDLER_RTX  aarch64_eh_return_handler_rtx ()
 
 /* Don't use __builtin_setjmp until we've defined it.  */
 #undef DONT_USE_BUILTIN_SETJMP
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 
5a25fce17785af9f9dc12e0f2a9609af09af0b35..bb8baff1e7a06942c8b8f51c1d6b341673401ef9
 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -2739,6 +2739,10 @@ aarch64_frame_pointer_required (void)
   && (!crtl->is_leaf || df_regs_ever_live_p (LR_REGNUM)))
 return true;
 
+  /* Force a frame pointer for EH returns so the return address is at FP+8.  */
+  if (crtl->calls_eh_return)
+    return true;
+
   return false;
 }
 
@@ -3298,7 +3302,8 @@ aarch64_expand_epilogue (bool for_sibcall)
  + cfun->machine->frame.saved_varargs_size) != 0;
 
   /* Emit a barrier to prevent loads from a deallocated stack.  */
-  if (final_adjust > crtl->outgoing_args_size || cfun->calls_alloca)
+  if (final_adjust > crtl->outgoing_args_size || cfun->calls_alloca
+  || crtl->calls_eh_return)
 {
   emit_insn (gen_stack_tie (stack_pointer_rtx, stack_pointer_rtx));
   need_barrier_p = false;
@@ -3366,52 +3371,15 @@ aarch64_expand_epilogue (bool for_sibcall)
 emit_jump_insn (ret_rtx);
 }
 
-/* Return the place to copy the exception unwinding return address to.
-   This will probably be a stack slot, but could (in theory be the
-   return register).  */
+/* Implement EH_RETURN_HANDLER_RTX.  The return address is stored at FP + 8.
+   The access needs to be volatile to prevent it from being removed.  */
 rtx
-aarch64_final_eh_return_addr (void)
+aarch64_eh_return_handler_rtx (void)
 {
-  HOST_WIDE_INT fp_offset;
-
-  aarch64_layout_frame ();
-
-  fp_offset = cfun->machine->frame.frame_size
- - cfun->machine->frame.hard_fp_offset;
-
-  if (cfun->machine->frame.reg_offset[LR_REGNUM] < 0)
-    return gen_rtx_REG (DImode, LR_REGNUM);
-
-  /* DSE and CSELIB do not detect an alias between sp+k1 and fp+k2.  This can
- result in a store to save LR introduced by builtin_eh_return () being
- incorrectly deleted because the 

[arm-embedded] [PATCH, ARM 7/7] Enable atomics for ARMv8-M Mainline

[Thomas Preudhomme]

Hi,

We've decided to apply the following patch to ARM/embedded-6-branch.

Best regards,

Thomas

 Forwarded Message 
Subject: Re: [PATCH, ARM 7/7] Enable atomics for ARMv8-M Mainline
Date: Thu, 14 Jul 2016 17:34:44 +0100
From: Thomas Preudhomme 
Organization: ARM
To: Kyrill Tkachov 
CC: Richard Earnshaw , Ramana Radhakrishnan 
, gcc-patches@gcc.gnu.org


On Thursday 14 July 2016 17:23:46 Kyrill Tkachov wrote:

Hi Thomas,

On 14/07/16 14:37, Thomas Preudhomme wrote:
> Hi Kyrill,
>
> On Thursday 19 May 2016 17:18:29 Kyrill Tkachov wrote:
>> Hi Thomas,
>>
>> On 17/05/16 11:15, Thomas Preudhomme wrote:
>>> Ping?
>>>
>>> *** gcc/ChangeLog ***
>>>
>>> 2015-12-17  Thomas Preud'homme  
>>>
>>>   * config/arm/arm.h (TARGET_HAVE_LDACQ): Enable for ARMv8-M
>>>   Mainline.
>>>
>>> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
>>> index
>>> 347b5b0a5cc0bc1e3b5020c8124d968e76ce48a4..e154bd31b8084f9f45ad4409e7b38d
>>> e6
>>> 52538c51 100644
>>> --- a/gcc/config/arm/arm.h
>>> +++ b/gcc/config/arm/arm.h
>>> @@ -266,7 +266,7 @@ extern void
>>> (*arm_lang_output_object_attributes_hook)
>>> (void);
>>>
>>> || arm_arch7) && arm_arch_notm)
>>>
>>>/* Nonzero if this chip supports load-acquire and store-release.  */
>>>
>>> -#define TARGET_HAVE_LDACQ (TARGET_ARM_ARCH >= 8 && arm_arch_notm)
>>> +#define TARGET_HAVE_LDACQ (TARGET_ARM_ARCH >= 8 && TARGET_32BIT)
>>
>> So this change is correct because ARMv8-M Mainline uses Thumb2
>> and is therefore TARGET_32BIT.
>>
>> This is ok but I'd like to see a follow up patch to enable the tests
>> that exercise acquire-release instructions in the arm.exp testsuite
>> for ARMv8-M Mainline so that we can be sure they get proper testsuite
>> coverage.
>
> I've respinned the patch because of the changes to atomic_loaddi output
> template in config/arm/sync.md. This patch now creates a new macro
> TARGET_HAVE_LDACQEXD to guard LDACQEXD and STLEXD instructions that are
> not
> available in ARMv8-M Mainline. It took advantage of the respin to also add
> the tests you were asking for.
>
> ChangeLog entries are as follow:
>
> *** gcc/ChangeLog ***
>
> 2016-07-05  Thomas Preud'homme  
>
>  * config/arm/arm.h (TARGET_HAVE_LDACQ): Enable for ARMv8-M
>  Mainline.
>  (TARGET_HAVE_LDACQD): New macro.
>  * config/arm/sync.md (atomic_loaddi): Use TARGET_HAVE_LDACQD
>  rather
>  than TARGET_HAVE_LDACQ.
>  (arm_load_acquire_exclusivedi): Likewise.
>  (arm_store_release_exclusivedi): Likewise.
>
> *** gcc/testsuite/ChangeLog ***
>
> 2016-07-05  Thomas Preud'homme  
>
>  * gcc.target/arm/atomic-comp-swap-release-acquire.c: Rename into
>  ...
>  * gcc.target/arm/atomic-comp-swap-release-acquire-1.c: This.
>  * gcc.target/arm/atomic-op-acq_rel.c: Rename into ...
>  * gcc.target/arm/atomic-op-acq_rel-1.c: This.
>  * gcc.target/arm/atomic-op-acquire.c: Rename into ...
>  * gcc.target/arm/atomic-op-acquire-1.c: This.
>  * gcc.target/arm/atomic-op-char.c: Rename into ...
>  * gcc.target/arm/atomic-op-char-1.c: This.
>  * gcc.target/arm/atomic-op-consume.c: Rename into ...
>  * gcc.target/arm/atomic-op-consume-1.c: This.
>  * gcc.target/arm/atomic-op-int.c: Rename into ...
>  * gcc.target/arm/atomic-op-int-1.c: This.
>  * gcc.target/arm/atomic-op-relaxed.c: Rename into ...
>  * gcc.target/arm/atomic-op-relaxed-1.c: This.
>  * gcc.target/arm/atomic-op-release.c: Rename into ...
>  * gcc.target/arm/atomic-op-release-1.c: This.
>  * gcc.target/arm/atomic-op-seq_cst.c: Rename into ...
>  * gcc.target/arm/atomic-op-seq_cst-1.c: This.
>  * gcc.target/arm/atomic-op-short.c: Rename into ...
>  * gcc.target/arm/atomic-op-short-1.c: This.
>  * gcc.target/arm/atomic-comp-swap-release-acquire-2.c: New test.
>  * gcc.target/arm/atomic-op-acq_rel-2.c: Likewise.
>  * gcc.target/arm/atomic-op-acquire-2.c: Likewise.
>  * gcc.target/arm/atomic-op-char-2.c: Likewise.
>  * gcc.target/arm/atomic-op-consume-2.c: Likewise.
>  * gcc.target/arm/atomic-op-int-2.c: Likewise.
>  * gcc.target/arm/atomic-op-relaxed-2.c: Likewise.
>  * gcc.target/arm/atomic-op-release-2.c: Likewise.
>  * gcc.target/arm/atomic-op-seq_cst-2.c: Likewise.
>  * gcc.target/arm/atomic-op-short-2.c: Likewise.
>
> Testsuite shows no regression and atomic tests [1] are passing for ARMv8-M
> Mainline.
>
> [1] gcc.dg/atomic*, g++.dg/ext/atomic*, gcc.target/arm/atomic*,
> gcc.target/arm/sync* and libstdc++-v3/testsuite/29_atomic/*

Thanks, this is ok if testing on arm-none-linux-gnueabihf is ok.
In particular, please make sure that the tests still pass for an A-profile
target.


Oh yes, I forgot to mention that I also tested the same tests for armv8-a 
without any code generation change for bo

Re: [patch] Fix segfault in param_change_prob

[Eric Botcazou]

> So I think it's better to apply get_base_address here rather than only
> stripping VIEW_CONVERT_EXRPs.
> (beware of it returning NULL_TREE for WITH_SIZE_EXPRs, thus maybe
> strip those first as well)

Something like this?  The testsuite is still clean with it.


* ipa-inline-analysis.c (param_change_prob): Get to the base object
first in all cases.

-- 
Eric BotcazouIndex: ipa-inline-analysis.c
===
--- ipa-inline-analysis.c	(revision 239842)
+++ ipa-inline-analysis.c	(working copy)
@@ -2183,11 +2183,16 @@ param_change_prob (gimple *stmt, int i)
 {
   tree op = gimple_call_arg (stmt, i);
   basic_block bb = gimple_bb (stmt);
-  tree base;
 
-  /* Global invariants neve change.  */
-  if (is_gimple_min_invariant (op))
+  if (TREE_CODE (op) == WITH_SIZE_EXPR)
+op = TREE_OPERAND (op, 0);
+
+  tree base = get_base_address (op);
+
+  /* Global invariants never change.  */
+  if (is_gimple_min_invariant (base))
 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). 
@@ -2194,7 +2199,7 @@ param_change_prob (gimple *stmt, int i)
 
  We do an conservative estimate: when call is executed N times more often
  than the statement defining value, we take the frequency 1/N.  */
-  if (TREE_CODE (op) == SSA_NAME)
+  if (TREE_CODE (base) == SSA_NAME)
 {
   int init_freq;
 
@@ -2201,10 +2206,10 @@ param_change_prob (gimple *stmt, int i)
   if (!bb->frequency)
 	return REG_BR_PROB_BASE;
 
-  if (SSA_NAME_IS_DEFAULT_DEF (op))
+  if (SSA_NAME_IS_DEFAULT_DEF (base))
 	init_freq = ENTRY_BLOCK_PTR_FOR_FN (cfun)->frequency;
   else
-	init_freq = gimple_bb (SSA_NAME_DEF_STMT (op))->frequency;
+	init_freq = gimple_bb (SSA_NAME_DEF_STMT (base))->frequency;
 
   if (!init_freq)
 	init_freq = 1;
@@ -2213,9 +2218,7 @@ param_change_prob (gimple *stmt, int i)
   else
 	return REG_BR_PROB_BASE;
 }
-
-  base = get_base_address (op);
-  if (base)
+  else
 {
   ao_ref refd;
   int max;
@@ -2256,7 +2259,6 @@ param_change_prob (gimple *stmt, int i)
   else
 	return REG_BR_PROB_BASE;
 }
-  return REG_BR_PROB_BASE;
 }
 
 /* Find whether a basic block BB is the final block of a (half) diamond CFG

[PATCH] Fix PR77436

[Richard Biener]

The following fixes the wide-int conversion of tree_fold_binomial.

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

Richard.

2016-09-01  Richard Biener  

PR middle-end/77436
* tree-chrec.c (tree_fold_binomial): Use widest_int, properly
check whether the result fits the desired result type.

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

Index: gcc/tree-chrec.c
===
--- gcc/tree-chrec.c(revision 239856)
+++ gcc/tree-chrec.c(working copy)
@@ -490,7 +490,6 @@ tree_fold_binomial (tree type, tree n, u
 {
   bool overflow;
   unsigned int i;
-  tree res;
 
   /* Handle the most frequent cases.  */
   if (k == 0)
@@ -498,18 +497,20 @@ tree_fold_binomial (tree type, tree n, u
   if (k == 1)
 return fold_convert (type, n);
 
+  widest_int num = wi::to_widest (n);
+
   /* Check that k <= n.  */
-  if (wi::ltu_p (n, k))
+  if (wi::ltu_p (num, k))
 return NULL_TREE;
 
   /* Denominator = 2.  */
-  wide_int denom = wi::two (TYPE_PRECISION (TREE_TYPE (n)));
+  widest_int denom = 2;
 
   /* Index = Numerator-1.  */
-  wide_int idx = wi::sub (n, 1);
+  widest_int idx = num - 1;
 
   /* Numerator = Numerator*Index = n*(n-1).  */
-  wide_int num = wi::smul (n, idx, &overflow);
+  num = wi::smul (num, idx, &overflow);
   if (overflow)
 return NULL_TREE;
 
@@ -528,9 +529,10 @@ tree_fold_binomial (tree type, tree n, u
 }
 
   /* Result = Numerator / Denominator.  */
-  wide_int di_res = wi::udiv_trunc (num, denom);
-  res = wide_int_to_tree (type, di_res);
-  return int_fits_type_p (res, type) ? res : NULL_TREE;
+  num = wi::udiv_trunc (num, denom);
+  if (! wi::fits_to_tree_p (num, type))
+return NULL_TREE;
+  return wide_int_to_tree (type, num);
 }
 
 /* Helper function.  Use the Newton's interpolating formula for
Index: gcc/testsuite/gcc.dg/torture/pr77436.c
===
--- gcc/testsuite/gcc.dg/torture/pr77436.c  (revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr77436.c  (working copy)
@@ -0,0 +1,11 @@
+/* { dg-do run } */
+
+int main()
+{
+  unsigned short sum = 0;
+  for (short x = -(__SHRT_MAX__ -1); x <= (__SHRT_MAX__ -1); x++)
+sum += x;
+  if (sum != 0)
+__builtin_abort ();
+  return 0;
+}

Re: [patch] Fix segfault in param_change_prob

[Richard Biener]

On Thu, Sep 1, 2016 at 3:17 PM, Eric Botcazou  wrote:
>> So I think it's better to apply get_base_address here rather than only
>> stripping VIEW_CONVERT_EXRPs.
>> (beware of it returning NULL_TREE for WITH_SIZE_EXPRs, thus maybe
>> strip those first as well)
>
> Something like this?  The testsuite is still clean with it.

Yes.

Thanks,
Richard.

>
> * ipa-inline-analysis.c (param_change_prob): Get to the base object
> first in all cases.
>
> --
> Eric Botcazou

Re: [PATCH][AArch64 - v2] Simplify eh_return implementation

[Ramana Radhakrishnan]

On 10/08/16 17:26, Wilco Dijkstra wrote:
> I noticed it would still be a good idea to add an extra barrier in the epilog 
> as the
> scheduler doesn't appear to handle aliases of frame accesses properly.
> 
> This patch simplifies the handling of the EH return value.  We force the use 
> of the
> frame pointer so the return location is always at FP + 8.  This means we can 
> emit
> a simple volatile access in EH_RETURN_HANDLER_RTX without needing md
> patterns, splitters and frame offset calculations.  The new implementation 
> also
> fixes various bugs in aarch64_final_eh_return_addr, which does not work with
> -fomit-frame-pointer, alloca or outgoing arguments.
> 
> Bootstrap OK, GCC Regression OK, OK for trunk? Would it be useful to backport
> this to GCC6.x?


Can you please file a PR for this and add some testcases ?  This sounds like a 
serious enough problem that needs to be looked at probably going back since the 
dawn of time.



Ramana
> 
> ChangeLog:
> 2016-08-10  Wilco Dijkstra  
> gcc/
>   * config/aarch64/aarch64.md (eh_return): Remove pattern and splitter.
>   * config/aarch64/aarch64.h (AARCH64_EH_STACKADJ_REGNUM): Remove.
>   (EH_RETURN_HANDLER_RTX): New define.
>   * config/aarch64/aarch64.c (aarch64_frame_pointer_required):
>   Force frame pointer in EH return functions.
>   (aarch64_expand_epilogue): Add barrier for eh_return.
>   (aarch64_final_eh_return_addr): Remove.
>   (aarch64_eh_return_handler_rtx): New function.
>   * config/aarch64/aarch64-protos.h (aarch64_final_eh_return_addr):
>   Remove.
>   (aarch64_eh_return_handler_rtx): New prototype.
> --
> 
> diff --git a/gcc/config/aarch64/aarch64-protos.h 
> b/gcc/config/aarch64/aarch64-protos.h
> index 
> 3cdd69b8af1089a839e5d45cda94bc70a15cd777..327c0a97f6f687604afef249b79ac22628418070
>  100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -358,7 +358,7 @@ int aarch64_hard_regno_mode_ok (unsigned, machine_mode);
>  int aarch64_hard_regno_nregs (unsigned, machine_mode);
>  int aarch64_uxt_size (int, HOST_WIDE_INT);
>  int aarch64_vec_fpconst_pow_of_2 (rtx);
> -rtx aarch64_final_eh_return_addr (void);
> +rtx aarch64_eh_return_handler_rtx (void);
>  rtx aarch64_mask_from_zextract_ops (rtx, rtx);
>  const char *aarch64_output_move_struct (rtx *operands);
>  rtx aarch64_return_addr (int, rtx);
> diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
> index 
> 003fec87e41db618570663f28cc2387a87e8252a..fa81e4b853daf08842955288861ec7e7acca
>  100644
> --- a/gcc/config/aarch64/aarch64.h
> +++ b/gcc/config/aarch64/aarch64.h
> @@ -400,9 +400,9 @@ extern unsigned aarch64_architecture_version;
>  #define ASM_DECLARE_FUNCTION_NAME(STR, NAME, DECL)   \
>aarch64_declare_function_name (STR, NAME, DECL)
>  
> -/* The register that holds the return address in exception handlers.  */
> -#define AARCH64_EH_STACKADJ_REGNUM   (R0_REGNUM + 4)
> -#define EH_RETURN_STACKADJ_RTX   gen_rtx_REG (Pmode, 
> AARCH64_EH_STACKADJ_REGNUM)
> +/* For EH returns X4 contains the stack adjustment.  */
> +#define EH_RETURN_STACKADJ_RTX   gen_rtx_REG (Pmode, R4_REGNUM)
> +#define EH_RETURN_HANDLER_RTX  aarch64_eh_return_handler_rtx ()
>  
>  /* Don't use __builtin_setjmp until we've defined it.  */
>  #undef DONT_USE_BUILTIN_SETJMP
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 
> 5a25fce17785af9f9dc12e0f2a9609af09af0b35..bb8baff1e7a06942c8b8f51c1d6b341673401ef9
>  100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -2739,6 +2739,10 @@ aarch64_frame_pointer_required (void)
>&& (!crtl->is_leaf || df_regs_ever_live_p (LR_REGNUM)))
>  return true;
>  
> +  /* Force a frame pointer for EH returns so the return address is at FP+8.  
> */
> +  if (crtl->calls_eh_return)
> +return true;
> +
>return false;
>  }
>  
> @@ -3298,7 +3302,8 @@ aarch64_expand_epilogue (bool for_sibcall)
>+ cfun->machine->frame.saved_varargs_size) != 0;
>  
>/* Emit a barrier to prevent loads from a deallocated stack.  */
> -  if (final_adjust > crtl->outgoing_args_size || cfun->calls_alloca)
> +  if (final_adjust > crtl->outgoing_args_size || cfun->calls_alloca
> +  || crtl->calls_eh_return)
>  {
>emit_insn (gen_stack_tie (stack_pointer_rtx, stack_pointer_rtx));
>need_barrier_p = false;
> @@ -3366,52 +3371,15 @@ aarch64_expand_epilogue (bool for_sibcall)
>  emit_jump_insn (ret_rtx);
>  }
>  
> -/* Return the place to copy the exception unwinding return address to.
> -   This will probably be a stack slot, but could (in theory be the
> -   return register).  */
> +/* Implement EH_RETURN_HANDLER_RTX.  The return address is stored at FP + 8.
> +   The access needs to be volatile to prevent it from being removed.  */
>  rtx
> -aarch64_final_eh_return_addr (void)
> +aarch64_eh_return_handler_rtx (void)
>  {
> -  HOST_WIDE_INT fp_offset;

Implement -Wimplicit-fallthrough (version 8)

[Marek Polacek]

Yet another version.  Changes from version 7:

* no more fix-it hints untils we resolve some of the issues this patch
  has discovered; instead, I simply used an inform call,
* I've fixed a bogus -Wdeclaration-after-statement warning in the C FE,
  new test added,
* I've addressed Jason's comments in
  https://gcc.gnu.org/ml/gcc-patches/2016-08/msg02001.html
  except one diagnostic bit:
  https://gcc.gnu.org/ml/gcc-patches/2016-08/msg02110.html

Joseph, any comments on the C part?

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

2016-09-01  Marek Polacek  
Jakub Jelinek  

PR c/7652
gcc/
* common.opt (Wimplicit-fallthrough): New option.
* doc/extend.texi: Document statement attributes and the fallthrough
attribute.
* doc/invoke.texi: Document -Wimplicit-fallthrough.
* gimple.h (gimple_call_internal_p): New function.
* gimplify.c (struct gimplify_ctx): Add in_switch_expr.
(struct label_entry): New struct.
(find_label_entry): New function.
(case_label_p): New function.
(collect_fallthrough_labels): New function.
(last_stmt_in_scope): New function.
(should_warn_for_implicit_fallthrough): New function.
(warn_implicit_fallthrough_r): New function.
(maybe_warn_implicit_fallthrough): New function.
(expand_FALLTHROUGH_r): New function.
(expand_FALLTHROUGH): New function.
(gimplify_switch_expr): Call maybe_warn_implicit_fallthrough and
expand_FALLTHROUGH for the innermost GIMPLE_SWITCH.
(gimplify_label_expr): New function.
(gimplify_case_label_expr): Set location.
(gimplify_expr): Call gimplify_label_expr.
* internal-fn.c (expand_FALLTHROUGH): New function.
* internal-fn.def (FALLTHROUGH): New internal function.
* langhooks.c (lang_GNU_OBJC): New function.
* langhooks.h (lang_GNU_OBJC): Declare.
* system.h (gcc_fallthrough): Define.
* tree-core.h: Add FALLTHROUGH_LABEL_P comment.
* tree.h (FALLTHROUGH_LABEL_P): Define.
gcc/c-family/
* c-common.c (c_common_attribute_table): Add fallthrough attribute.
(handle_fallthrough_attribute): New function.
gcc/c/
* c-parser.c (struct c_token): Add flags field.
(c_lex_one_token): Pass it to c_lex_with_flags.
(c_parser_declaration_or_fndef): Turn __attribute__((fallthrough));
into IFN_FALLTHROUGH.
(c_parser_label): Set FALLTHROUGH_LABEL_P on labels.
(c_parser_statement_after_labels): Handle RID_ATTRIBUTE.
gcc/cp/
* constexpr.c (cxx_eval_internal_function): Handle IFN_FALLTHROUGH.
(potential_constant_expression_1): Likewise.
* constraint.cc (function_concept_check_p): Check fn for null.
* parser.c (cp_parser_primary_expression): Handle RID_ATTRIBUTE.
(cp_parser_statement): Handle fallthrough attribute.
(cp_parser_label_for_labeled_statement): Set FALLTHROUGH_LABEL_P on
labels.
(cp_parser_std_attribute): Handle fallthrough attribute.
(cp_parser_check_std_attribute): Detect duplicated fallthrough
attribute.
* pt.c (tsubst_copy_and_build): Handle internal functions.
(instantiation_dependent_scope_ref_p): Return if the expression is
null.
gcc/testsuite/
* c-c++-common/Wimplicit-fallthrough-1.c: New test.
* c-c++-common/Wimplicit-fallthrough-10.c: New test.
* c-c++-common/Wimplicit-fallthrough-11.c: New test.
* c-c++-common/Wimplicit-fallthrough-12.c: New test.
* c-c++-common/Wimplicit-fallthrough-13.c: New test.
* c-c++-common/Wimplicit-fallthrough-14.c: New test.
* c-c++-common/Wimplicit-fallthrough-15.c: New test.
* c-c++-common/Wimplicit-fallthrough-16.c: New test.
* c-c++-common/Wimplicit-fallthrough-17.c: New test.
* c-c++-common/Wimplicit-fallthrough-18.c: New test.
* c-c++-common/Wimplicit-fallthrough-19.c: New test.
* c-c++-common/Wimplicit-fallthrough-2.c: New test.
* c-c++-common/Wimplicit-fallthrough-3.c: New test.
* c-c++-common/Wimplicit-fallthrough-4.c: New test.
* c-c++-common/Wimplicit-fallthrough-5.c: New test.
* c-c++-common/Wimplicit-fallthrough-6.c: New test.
* c-c++-common/Wimplicit-fallthrough-7.c: New test.
* c-c++-common/Wimplicit-fallthrough-8.c: New test.
* c-c++-common/Wimplicit-fallthrough-9.c: New test.
* c-c++-common/attr-fallthrough-1.c: New test.
* c-c++-common/attr-fallthrough-2.c: New test.
* g++.dg/cpp0x/fallthrough1.C: New test.
* g++.dg/cpp0x/fallthrough2.C: New test.
* g++.dg/cpp1z/fallthrough1.C: New test.
* gcc.dg/Wimplicit-fallthrough-1.c: New test.
* obj-c++.dg/Wimplicit-fallthrough-1.mm: New test.
* objc.dg/Wimplicit-fallthrough-1.m: New test.
libcpp/
* include/cpplib.h (PREV_FALLTHROUGH): Defin

Implement -Wimplicit-fallthrough (version 8): add gcc_fallthrough()

[Marek Polacek]

Tobias pointed out that I need to add a new gcc_fallthrough into trans-io.c,
so done.

Bootstrapped/regtested on x86_64-linux and ppc64-linux.

2016-08-31  Marek Polacek  

PR c/7652
* Makefile.in (insn-attrtab.o-warn, insn-dfatab.o-warn,
insn-latencytab.o-warn, insn-output.o-warn, insn-emit.o-warn): Add
-Wno-switch-fallthrough.
* builtins.c (expand_builtin_int_roundingfn_2): Add gcc_fallthrough.
(expand_builtin): Likewise.
* config/rs6000/rs6000.c (rs6000_builtin_vectorized_libmass): Likewise.
* convert.c (convert_to_real_1): Likewise.
(convert_to_integer_1): Likewise.
* final.c (output_alternate_entry_point): Likewise.
* genattrtab.c (make_canonical): Likewise.
(write_test_expr): Likewise.
* genpreds.c (validate_exp): Likewise.
* gimple-ssa-strength-reduction.c
(find_candidates_dom_walker::before_dom_children): Likewise.
* godump.c (go_format_type): Likewise.
* reload1.c (elimination_effects): Likewise.
* resource.c (mark_referenced_resources): Likewise.
(mark_set_resources): Likewise.
* tree-ssa-loop-ivopts.c (find_deriving_biv_for_expr): Likewise.
* varasm.c (output_addressed_constants): Likewise.
gcc/c-family/
* c-common.c (resolve_overloaded_builtin): Add gcc_fallthrough.
gcc/c/
* c-decl.c (pop_scope): Add gcc_fallthrough.
* c-typeck.c (composite_type): Likewise.
gcc/gcc/cp/
* error.c (dump_type): Add gcc_fallthrough.
(dump_decl): Likewise.
(dump_expr): Likewise.
* parser.c (cp_parser_storage_class_specifier_opt): Likewise.
(cp_parser_skip_to_end_of_template_parameter_list): Likewise.
(cp_parser_cache_defarg): Likewise.
(cp_parser_omp_for_cond): Likewise.
* semantics.c (finish_decltype_type): Likewise.
* typeck.c (structural_comptypes): Likewise.
(cp_build_binary_op): Likewise.
(cp_build_modify_expr): Likewise.
gcc/fortran/
* arith.c (eval_intrinsic): Add gcc_fallthrough.
* frontend-passes.c (optimize_op): Likewise.
(gfc_expr_walker): Likewise.
* parse.c (next_fixed): Likewise.
* primary.c (match_variable): Likewise.
* trans-array.c: Likewise.
* trans-expr.c (flatten_array_ctors_without_strlen): Likewise.
* trans-io.c (transfer_expr): Likewise.
libstdc++-v3/
* libsupc++/hash_bytes.cc: Add [[gnu::fallthrough]].

diff --git gcc/gcc/Makefile.in gcc/gcc/Makefile.in
index eb5ab61..1046b31 100644
--- gcc/gcc/Makefile.in
+++ gcc/gcc/Makefile.in
@@ -218,6 +218,11 @@ libgcov-merge-tool.o-warn = -Wno-error
 gimple-match.o-warn = -Wno-unused
 generic-match.o-warn = -Wno-unused
 dfp.o-warn = -Wno-strict-aliasing
+insn-attrtab.o-warn = -Wno-implicit-fallthrough
+insn-dfatab.o-warn = -Wno-implicit-fallthrough
+insn-latencytab.o-warn = -Wno-implicit-fallthrough
+insn-output.o-warn = -Wno-implicit-fallthrough
+insn-emit.o-warn = -Wno-implicit-fallthrough
 
 # All warnings have to be shut off in stage1 if the compiler used then
 # isn't gcc; configure determines that.  WARN_CFLAGS will be either
diff --git gcc/gcc/builtins.c gcc/gcc/builtins.c
index b981bcd..9da37dc 100644
--- gcc/gcc/builtins.c
+++ gcc/gcc/builtins.c
@@ -2588,7 +2588,7 @@ expand_builtin_int_roundingfn_2 (tree exp, rtx target)
 {
 CASE_FLT_FN (BUILT_IN_IRINT):
   fallback_fn = BUILT_IN_LRINT;
-  /* FALLTHRU */
+  gcc_fallthrough ();
 CASE_FLT_FN (BUILT_IN_LRINT):
 CASE_FLT_FN (BUILT_IN_LLRINT):
   builtin_optab = lrint_optab;
@@ -2596,7 +2596,7 @@ expand_builtin_int_roundingfn_2 (tree exp, rtx target)
 
 CASE_FLT_FN (BUILT_IN_IROUND):
   fallback_fn = BUILT_IN_LROUND;
-  /* FALLTHRU */
+  gcc_fallthrough ();
 CASE_FLT_FN (BUILT_IN_LROUND):
 CASE_FLT_FN (BUILT_IN_LLROUND):
   builtin_optab = lround_optab;
@@ -5905,6 +5905,7 @@ expand_builtin (tree exp, rtx target, rtx subtarget, 
machine_mode mode,
 CASE_FLT_FN (BUILT_IN_ILOGB):
   if (! flag_unsafe_math_optimizations)
break;
+  gcc_fallthrough ();
 CASE_FLT_FN (BUILT_IN_ISINF):
 CASE_FLT_FN (BUILT_IN_FINITE):
 case BUILT_IN_ISFINITE:
diff --git gcc/gcc/c-family/c-common.c gcc/gcc/c-family/c-common.c
index b29334a..b08de64 100644
--- gcc/gcc/c-family/c-common.c
+++ gcc/gcc/c-family/c-common.c
@@ -11590,6 +11590,7 @@ resolve_overloaded_builtin (location_t loc, tree 
function,
gcc_unreachable ();
}
/* Fallthrough to the normal processing.  */
+   gcc_fallthrough ();
   }
 case BUILT_IN_ATOMIC_EXCHANGE_N:
 case BUILT_IN_ATOMIC_COMPARE_EXCHANGE_N:
@@ -11598,6 +11599,7 @@ resolve_overloaded_builtin (location_t loc, tree 
function,
   {
fetch_op = false;
/* Fallthrough to further processing.  */
+   gcc_fallthrough ();
   }
 case BUILT_IN_ATOMIC_ADD_FETCH_N:
 case BUILT_IN_ATOMIC_SUB_FETCH_N:

Ping Re: Make max_align_t respect _Float128

[Joseph Myers]

Ping.  This patch 
 is pending 
review (with either __i386__ or __SIZEOF_FLOAT128__, although __i386__ is 
safest in that it minimizes the cases where there is any ABI change at 
all even in the size of the type).

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

Re: Implement -Wimplicit-fallthrough (version 8): add gcc_fallthrough()

[Jakub Jelinek]

On Thu, Sep 01, 2016 at 03:42:12PM +0200, Marek Polacek wrote:
> --- gcc/gcc/c-family/c-common.c
> +++ gcc/gcc/c-family/c-common.c
> @@ -11590,6 +11590,7 @@ resolve_overloaded_builtin (location_t loc, tree 
> function,
>   gcc_unreachable ();
>   }
>   /* Fallthrough to the normal processing.  */
> + gcc_fallthrough ();
>}
>  case BUILT_IN_ATOMIC_EXCHANGE_N:
>  case BUILT_IN_ATOMIC_COMPARE_EXCHANGE_N:
> @@ -11598,6 +11599,7 @@ resolve_overloaded_builtin (location_t loc, tree 
> function,
>{
>   fetch_op = false;
>   /* Fallthrough to further processing.  */
> + gcc_fallthrough ();
>}
>  case BUILT_IN_ATOMIC_ADD_FETCH_N:
>  case BUILT_IN_ATOMIC_SUB_FETCH_N:
> @@ -11614,6 +11616,7 @@ resolve_overloaded_builtin (location_t loc, tree 
> function,
>{
>  orig_format = false;
>   /* Fallthru for parameter processing.  */
> + gcc_fallthrough ();
>}

These 3 look just like misformatted stuff, {}s around for no reason
whatsoever, the first case even badly indented and the latter two with a
single stmt inside of {}.
I think it would be better to just remove the useless outer {} pair
in all the cases, reindent and replace the comments with /* FALLTHRU */

> diff --git gcc/gcc/c/c-typeck.c gcc/gcc/c/c-typeck.c
> index 5194027..e89bdc8 100644
> --- gcc/gcc/c/c-typeck.c
> +++ gcc/gcc/c/c-typeck.c
> @@ -605,7 +605,7 @@ composite_type (tree t1, tree t2)
>  
>   t1 = build_function_type (valtype, newargs);
>   t1 = qualify_type (t1, t2);
> - /* ... falls through ...  */
> + gcc_fallthrough ();
>}

One option for here (other than removing the {}s, which would require
separate var declarations from their assignments) would be to just do:
-   /* ... falls through ...  */
   }
+  /* FALLTHRU */

> diff --git gcc/gcc/config/rs6000/rs6000.c gcc/gcc/config/rs6000/rs6000.c
> index 2f15a05..c25314e 100644
> --- gcc/gcc/config/rs6000/rs6000.c
> +++ gcc/gcc/config/rs6000/rs6000.c
> @@ -5489,7 +5489,7 @@ rs6000_builtin_vectorized_libmass (combined_fn fn, tree 
> type_out,
>  CASE_CFN_HYPOT:
>  CASE_CFN_POW:
>n_args = 2;
> -  /* fall through */
> +  gcc_fallthrough ();
>  
>  CASE_CFN_ACOS:
>  CASE_CFN_ACOSH:

This is needed becase CSE_CFN_ACOS is a macro, right?  I guess it is fine.

> --- gcc/gcc/cp/error.c
> +++ gcc/gcc/cp/error.c
> @@ -576,6 +576,7 @@ dump_type (cxx_pretty_printer *pp, tree t, int flags)
>  default:
>pp_unsupported_tree (pp, t);
>/* Fall through to error.  */
> +  gcc_fallthrough ();
>  
>  case ERROR_MARK:
>pp_string (pp, M_(""));
> @@ -1277,6 +1278,7 @@ dump_decl (cxx_pretty_printer *pp, tree t, int flags)
>  default:
>pp_unsupported_tree (pp, t);
>/* Fall through to error.  */
> +  gcc_fallthrough ();
>  
>  case ERROR_MARK:
>pp_string (pp, M_(""));
> @@ -2778,6 +2780,7 @@ dump_expr (cxx_pretty_printer *pp, tree t, int flags)
>  default:
>pp_unsupported_tree (pp, t);
>/* fall through to ERROR_MARK...  */
> +  gcc_fallthrough ();

Why the above ones?  Replacing the comments with /* FALLTHRU */ would be
sufficient IMHO.  Or is there any value in explaining why it falls through?

Jakub

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

[Segher Boessenkool]

On Wed, Aug 31, 2016 at 09:15:32PM -0500, Bill Schmidt wrote:
> 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?

Yes please.  Thanks,


Segher

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

[Bill Schmidt]

On Sep 1, 2016, at 9:32 AM, Segher Boessenkool  
wrote:
> 
> On Wed, Aug 31, 2016 at 09:15:32PM -0500, Bill Schmidt wrote:
>> 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?
> 
> Yes please.  Thanks,

r239938, thanks!

Bill
> 
> 
> Segher
> 

Re: Implement -Wimplicit-fallthrough (version 8): add gcc_fallthrough()

[Marek Polacek]

On Thu, Sep 01, 2016 at 04:27:01PM +0200, Jakub Jelinek wrote:
> On Thu, Sep 01, 2016 at 03:42:12PM +0200, Marek Polacek wrote:
> > --- gcc/gcc/c-family/c-common.c
> > +++ gcc/gcc/c-family/c-common.c
> > @@ -11590,6 +11590,7 @@ resolve_overloaded_builtin (location_t loc, tree 
> > function,
> > gcc_unreachable ();
> > }
> > /* Fallthrough to the normal processing.  */
> > +   gcc_fallthrough ();
> >}
> >  case BUILT_IN_ATOMIC_EXCHANGE_N:
> >  case BUILT_IN_ATOMIC_COMPARE_EXCHANGE_N:
> > @@ -11598,6 +11599,7 @@ resolve_overloaded_builtin (location_t loc, tree 
> > function,
> >{
> > fetch_op = false;
> > /* Fallthrough to further processing.  */
> > +   gcc_fallthrough ();
> >}
> >  case BUILT_IN_ATOMIC_ADD_FETCH_N:
> >  case BUILT_IN_ATOMIC_SUB_FETCH_N:
> > @@ -11614,6 +11616,7 @@ resolve_overloaded_builtin (location_t loc, tree 
> > function,
> >{
> >  orig_format = false;
> > /* Fallthru for parameter processing.  */
> > +   gcc_fallthrough ();
> >}
> 
> These 3 look just like misformatted stuff, {}s around for no reason
> whatsoever, the first case even badly indented and the latter two with a
> single stmt inside of {}.
> I think it would be better to just remove the useless outer {} pair
> in all the cases, reindent and replace the comments with /* FALLTHRU */
 
Ok, fixed.

> > diff --git gcc/gcc/c/c-typeck.c gcc/gcc/c/c-typeck.c
> > index 5194027..e89bdc8 100644
> > --- gcc/gcc/c/c-typeck.c
> > +++ gcc/gcc/c/c-typeck.c
> > @@ -605,7 +605,7 @@ composite_type (tree t1, tree t2)
> >  
> > t1 = build_function_type (valtype, newargs);
> > t1 = qualify_type (t1, t2);
> > -   /* ... falls through ...  */
> > +   gcc_fallthrough ();
> >}
> 
> One option for here (other than removing the {}s, which would require
> separate var declarations from their assignments) would be to just do:
> - /* ... falls through ...  */
>}
> +  /* FALLTHRU */

Done. 

> > diff --git gcc/gcc/config/rs6000/rs6000.c gcc/gcc/config/rs6000/rs6000.c
> > index 2f15a05..c25314e 100644
> > --- gcc/gcc/config/rs6000/rs6000.c
> > +++ gcc/gcc/config/rs6000/rs6000.c
> > @@ -5489,7 +5489,7 @@ rs6000_builtin_vectorized_libmass (combined_fn fn, 
> > tree type_out,
> >  CASE_CFN_HYPOT:
> >  CASE_CFN_POW:
> >n_args = 2;
> > -  /* fall through */
> > +  gcc_fallthrough ();
> >  
> >  CASE_CFN_ACOS:
> >  CASE_CFN_ACOSH:
> 
> This is needed becase CSE_CFN_ACOS is a macro, right?  I guess it is fine.

Yes, exactly.

> > --- gcc/gcc/cp/error.c
> > +++ gcc/gcc/cp/error.c
> > @@ -576,6 +576,7 @@ dump_type (cxx_pretty_printer *pp, tree t, int flags)
> >  default:
> >pp_unsupported_tree (pp, t);
> >/* Fall through to error.  */
> > +  gcc_fallthrough ();
> >  
> >  case ERROR_MARK:
> >pp_string (pp, M_(""));
> > @@ -1277,6 +1278,7 @@ dump_decl (cxx_pretty_printer *pp, tree t, int flags)
> >  default:
> >pp_unsupported_tree (pp, t);
> >/* Fall through to error.  */
> > +  gcc_fallthrough ();
> >  
> >  case ERROR_MARK:
> >pp_string (pp, M_(""));
> > @@ -2778,6 +2780,7 @@ dump_expr (cxx_pretty_printer *pp, tree t, int flags)
> >  default:
> >pp_unsupported_tree (pp, t);
> >/* fall through to ERROR_MARK...  */
> > +  gcc_fallthrough ();
> 
> Why the above ones?  Replacing the comments with /* FALLTHRU */ would be
> sufficient IMHO.  Or is there any value in explaining why it falls through?

I thought I'd keep it since it conveys something more than a pure "falls
through" comments, but I changed it anyway.  Here's the pure comment
changes patch which I hope can be committed right away.

2016-09-01  Marek Polacek  

PR c/7652
gcc/c-family/
* c-common.c (resolve_overloaded_builtin): Fix formatting.  Add
FALLTHRU comments.
gcc/c/
* c-typeck.c (composite_type): Add FALLTHRU comment.
gcc/gcc/cp/
* error.c (dump_type): Fix falls through comment.
(dump_decl): Likewise.
(dump_expr): Likewise.

diff --git gcc/gcc/c-family/c-common.c gcc/gcc/c-family/c-common.c
index b29334a..399ba97 100644
--- gcc/gcc/c-family/c-common.c
+++ gcc/gcc/c-family/c-common.c
@@ -11547,7 +11547,7 @@ resolve_overloaded_builtin (location_t loc, tree 
function,
/* Handle these 4 together so that they can fall through to the next
   case if the call is transformed to an _N variant.  */
 switch (orig_code)
-   {
+ {
  case BUILT_IN_ATOMIC_EXCHANGE:
{
  if (resolve_overloaded_atomic_exchange (loc, function, params,
@@ -11588,17 +11588,15 @@ resolve_overloaded_builtin (location_t loc, tree 
function,
}
  default:
gcc_unreachable ();
-   }
-   /* Fallthrough to the normal processing.  */
+ }
   }
+  /* FALLTHRU */
 case BUILT_IN_ATOMIC_EXCHANGE_N:
 case BUILT_IN_ATOMIC

Re: Implement -Wimplicit-fallthrough (version 8): add gcc_fallthrough()

[Jakub Jelinek]

On Thu, Sep 01, 2016 at 04:51:37PM +0200, Marek Polacek wrote:
> I thought I'd keep it since it conveys something more than a pure "falls
> through" comments, but I changed it anyway.  Here's the pure comment
> changes patch which I hope can be committed right away.
> 
> 2016-09-01  Marek Polacek  
> 
>   PR c/7652
> gcc/c-family/
>   * c-common.c (resolve_overloaded_builtin): Fix formatting.  Add
>   FALLTHRU comments.
> gcc/c/
>   * c-typeck.c (composite_type): Add FALLTHRU comment.
> gcc/gcc/cp/
>   * error.c (dump_type): Fix falls through comment.
>   (dump_decl): Likewise.
>   (dump_expr): Likewise.

Thanks.  Yes, this is ok for trunk.

Jakub

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

[Jonathan Wakely]

On 31/08/16 13:45 +0100, Jonathan Wakely wrote:

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)


iThis is true, but ...


and so the / and % results will be too.


This isn't.

If __swap_range is 3, then __comp_range is 10 and
__pospos is uniformly distributed in [0, 9].

(__pospos % __swap_range) is not uniformly distributed, we get
P(0) = 0.4, P(1) = 0.3, P(2) = 0.3.

Similarly, (__pospos / __swap_range) is not uniform, we get
P(0) = 0.3, P(1) = 0.3, P(2) = 0.3, P(3) = 0.1

This means that certain permuations of the input are more likely than
others, which fails to meet the requirements of the function.

Or is there a flaw in my reasoning?

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

[Segher Boessenkool]

On Thu, Sep 01, 2016 at 09:50:09AM +0100, Iain Sandoe wrote:
> > I left it in *restore_world because Iain will remove it there soon.
> 
> Here is the patch to fix up Darwin,
> I guess it’s both Darwin-local and reasonably obvious now, but OK?

Yes, okay for trunk, thanks!


Segher


> 2016-09-01  Iain Sandoe  
>   Segher Boessenkool  
> 
>   * config/rs6000/altivec.md (*restore_world): Remove LR use.
>   * config/rs6000/predicates.md (restore_world_operation): Adjust count 
> op count, remove one USE.

(Fix the long line here please).

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

[Marc Glisse]

On Thu, 1 Sep 2016, Jonathan Wakely wrote:


+ const __uc_type __comp_range = __swap_range * (__swap_range + 1);


If __swap_range is 3, then __comp_range is 10 and


???

--
Marc Glisse

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

[Eelis van der Weegen]

On 2016-09-01 17:14, Jonathan Wakely wrote:

On 31/08/16 13:45 +0100, Jonathan Wakely wrote:

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)


iThis is true, but ...


and so the / and % results will be too.


This isn't.

If __swap_range is 3, then __comp_range is 10 and
__pospos is uniformly distributed in [0, 9].

(__pospos % __swap_range) is not uniformly distributed, we get
P(0) = 0.4, P(1) = 0.3, P(2) = 0.3.

Similarly, (__pospos / __swap_range) is not uniform, we get
P(0) = 0.3, P(1) = 0.3, P(2) = 0.3, P(3) = 0.1

This means that certain permuations of the input are more likely than
others, which fails to meet the requirements of the function.

Or is there a flaw in my reasoning?


Just that if __swap_range is 3, then __comp_range is 3*(3+1)=12, not 10. :)

Thanks for the review! I'll send an updated patch addressing 

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

[Jonathan Wakely]

On 01/09/16 17:27 +0200, Marc Glisse wrote:

On Thu, 1 Sep 2016, Jonathan Wakely wrote:


+ const __uc_type __comp_range = __swap_range * (__swap_range + 1);


If __swap_range is 3, then __comp_range is 10 and


???


Bah :-)

Thanks. I guess I read the code correctly the other day at least!

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

[Jonathan Wakely]

On 01/09/16 17:31 +0200, Eelis van der Weegen wrote:

On 2016-09-01 17:14, Jonathan Wakely wrote:

On 31/08/16 13:45 +0100, Jonathan Wakely wrote:

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)


iThis is true, but ...


and so the / and % results will be too.


This isn't.

If __swap_range is 3, then __comp_range is 10 and
__pospos is uniformly distributed in [0, 9].

(__pospos % __swap_range) is not uniformly distributed, we get
P(0) = 0.4, P(1) = 0.3, P(2) = 0.3.

Similarly, (__pospos / __swap_range) is not uniform, we get
P(0) = 0.3, P(1) = 0.3, P(2) = 0.3, P(3) = 0.1

This means that certain permuations of the input are more likely than
others, which fails to meet the requirements of the function.

Or is there a flaw in my reasoning?


Just that if __swap_range is 3, then __comp_range is 3*(3+1)=12, not 10. :)


A minor detail!


Thanks 

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

[Joseph Myers]

On Thu, 1 Sep 2016, Szabolcs Nagy wrote:

> it seems that the FLT_EVAL_METHOD change in TS 18661 was a
> bad decision: existing code have to be modified to conform
> to the new semantics. this situation could have been avoided
> by introducing new macros or only changing behaviour when
> the user requested it via __STDC_WANT_IEC_60559_TYPES_EXT__
> (consistently with all other library changes..).

You could always raise the issue directly with WG14, since TS 18661 is 
supposed to be compatible with C11 (cf the issue I raised about whether 
the formula for DECIMAL_DIG in C11 is normatively required or a minimum).

Now a minimal fix for such an issue (interpreted as a DR against C11 
rather than against TS 18661-3) to make the definition of FLT_EVAL_METHOD 
extensible would be to remove the word "negative" in "All other negative 
values for FLT_EVAL_METHOD characterize implementation-defined behavior.", 
so making it more like FLT_ROUNDS.  Which would allow the value 16 to be 
used in existing standards modes, but not avoid the need to update 
existing code to handle it.  But you might prefer an interpretation as a 
DR against TS 18661-3 where the FLT_EVAL_METHOD definition does not have 
to reflect the new types unless __STDC_WANT_IEC_60559_TYPES_EXT__ is 
defined accordingly (and maybe for consistency DEC_EVAL_METHOD only has to 
reflect TS 18661-2 decimal types, not those from 18661-3, unless 
__STDC_WANT_IEC_60559_TYPES_EXT__ is defined).

I expect glibc will be defining __STDC_WANT_IEC_60559_TYPES_EXT__ soon 
anyway (in its  wrapper to get FLT128_* definitions from there 
where possible) so would still need adapting to be optimal with _Float16 
compiler support.

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

Re: [PATCH, testsuite]: Use dg-add-options ieee in some _FloatN tests

[Joseph Myers]

On Thu, 1 Sep 2016, Uros Bizjak wrote:

> Hello!
> 
> Attached patch adds dg-add-options ieee in _FloatN tests where
> advanced IEEE features are used (e.g. NaN, infinity, subnormals).
> 
> 2016-09-01  Uros Bizjak  
> 
> * gcc.dg/torture/float128-builtin.c, gcc.dg/torture/float128-floath.c,
> gcc.dg/torture/float128-ieee-nan.c, gcc.dg/torture/float128-tg-2.c,
> gcc.dg/torture/float128-tg.c, gcc.dg/torture/float128x-builtin.c,
> gcc.dg/torture/float128x-floath.c, gcc.dg/torture/float128x-nan.c,
> gcc.dg/torture/float128x-tg-2.c, gcc.dg/torture/float128x-tg.c,
> gcc.dg/torture/float16-builtin.c, gcc.dg/torture/float16-floath.c,
> gcc.dg/torture/float16-nan.c, gcc.dg/torture/float16-tg-2.c,
> gcc.dg/torture/float16-tg.c, gcc.dg/torture/float32-builtin.c,
> gcc.dg/torture/float32-floath.c, gcc.dg/torture/float32-nan.c,
> gcc.dg/torture/float32-tg-2.c, gcc.dg/torture/float32-tg.c,
> gcc.dg/torture/float32x-builtin.c, gcc.dg/torture/float32x-floath.c,
> gcc.dg/torture/float32x-nan.c, gcc.dg/torture/float32x-tg-2.c,
> gcc.dg/torture/float32x-tg.c, gcc.dg/torture/float64-builtin.c,
> gcc.dg/torture/float64-floath.c, gcc.dg/torture/float64-nan.c,
> gcc.dg/torture/float64-tg-2.c, gcc.dg/torture/float64-tg.c,
> gcc.dg/torture/float64x-builtin.c, gcc.dg/torture/float64x-floath.c,
> gcc.dg/torture/float64x-nan.c, gcc.dg/torture/float64x-tg-2.c,
> gcc.dg/torture/float64x-tg.c: Use dg-add-options ieee.
> 
> Tested on alphaev68-linux-gnu (with an ABI correction patch I plan to
> propose soon).
> 
> OK for mainline?

OK.

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

Re: Implement -Wimplicit-fallthrough (version 8)

[Joseph Myers]

On Thu, 1 Sep 2016, Marek Polacek wrote:

> Yet another version.  Changes from version 7:
> 
> * no more fix-it hints untils we resolve some of the issues this patch
>   has discovered; instead, I simply used an inform call,
> * I've fixed a bogus -Wdeclaration-after-statement warning in the C FE,
>   new test added,
> * I've addressed Jason's comments in
>   https://gcc.gnu.org/ml/gcc-patches/2016-08/msg02001.html
>   except one diagnostic bit:
>   https://gcc.gnu.org/ml/gcc-patches/2016-08/msg02110.html
> 
> Joseph, any comments on the C part?

I have no comments on the C part.

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

Re: PR35503 - warn for restrict pointer

[Martin Sebor]

The attached version passes bootstrap+test on ppc64le-linux-gnu.
Given that it only looks if parameters are restrict qualified and not
how they're used inside the callee,
this can have false positives as in above test-cases.
Should the warning be put in Wextra rather than Wall (I have left it
in Wall in the patch)  or only enabled with -Wrestrict ?


Awesome!  I've wished for years for a warning like this!

I'm curious if you've tested other examples from 6.7.3.1 of C11
besides Example 3.  Example 4 seems like something GCC should be
able to detect but I didn't get a warning with the patch.

I would expect the warning to be especially valuable with string
manipulation functions like memcpy that have undefined behavior
for overlapping regions, such as in:

  char a[4];

  void g (void)
  {
__builtin_memcpy (a, a + 1, 3);
  }

But here, too, I didn't get a warning.  I understand that this
case cannot be handled for arbitrary functions whose semantics
aren't known but with standard functions for which GCC provides
intrinsics the effects are known and aliasing violations can in
common cases be detected.

Martin

Re: [PATCH][msp430] Don't output __interrupt_vector sections for weak functions

[Joe Seymour]

On 29/08/2016 22:09, DJ Delorie wrote:
> 
> Which results in a more user-obvious case, ignoring the interrupt
> attribute or ignoring the weak attribute?  I would think that we never
> want to compile and link successfully if we can't do what the user
> wants, and omitting an interrupt handler is... bad.
> 
> I think this should either be a hard error, so the user must fix it
> right away, or ignore the weak, which results in a linker error
> (somehow? maybe?) if the user specifies two handlers for the same
> interrupt.

Thanks for the review.  My original intention was to make it an error, but
msp430_attr() seemed to set the precedent of emitting warnings for invalid
attributes, so I tried to follow that convention.

Here's a patch that produces an error instead:

2016-09-XX  Joe Seymour  

gcc/
PR target/70713
* config/msp430/msp430.c (msp430_start_function): Emit an error
if a function is both weak and specifies an interrupt number.

gcc/testsuite/
PR target/70713
* gcc.target/msp430/function-attributes-1.c: New test.
* gcc.target/msp430/function-attributes-2.c: New test.
* gcc.target/msp430/function-attributes-3.c: New test.

diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index dba4d19..c40d2da 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -2108,6 +2108,13 @@ msp430_start_function (FILE *file, const char *name, 
tree decl)
{
  char buf[101];
 
+ /* Interrupt vector sections should be unique, but use of weak
+functions implies multiple definitions.  */
+ if (DECL_WEAK (decl))
+   {
+ error ("argument to interrupt attribute is unsupported for weak 
functions");
+   }
+
  intr_vector = TREE_VALUE (intr_vector);
 
  /* The interrupt attribute has a vector value.  Turn this into a
diff --git a/gcc/testsuite/gcc.target/msp430/function-attributes-1.c 
b/gcc/testsuite/gcc.target/msp430/function-attributes-1.c
new file mode 100644
index 000..7a3b7be
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/function-attributes-1.c
@@ -0,0 +1,9 @@
+void __attribute__((weak, interrupt))
+weak_interrupt (void) {
+}
+
+void __attribute__((interrupt(11)))
+interrupt_number (void) {
+}
+
+/* { dg-final { scan-assembler-times "__interrupt_vector_" 1 } } */
diff --git a/gcc/testsuite/gcc.target/msp430/function-attributes-2.c 
b/gcc/testsuite/gcc.target/msp430/function-attributes-2.c
new file mode 100644
index 000..fcb2fb2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/function-attributes-2.c
@@ -0,0 +1,3 @@
+void __attribute__((weak, interrupt(10)))
+weak_interrupt_number (void) {
+} /* { dg-error "argument to interrupt attribute is unsupported for weak 
functions" } */
diff --git a/gcc/testsuite/gcc.target/msp430/function-attributes-3.c 
b/gcc/testsuite/gcc.target/msp430/function-attributes-3.c
new file mode 100644
index 000..b0acf4a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/function-attributes-3.c
@@ -0,0 +1,3 @@
+void __attribute__((interrupt("nmi"))) __attribute__((weak))
+interrupt_name_weak (void) {
+} /* { dg-error "argument to interrupt attribute is unsupported for weak 
functions" } */

Re: stray warning from gcc's cpp

[Gerald Pfeifer]

Hi David,

I found this older report (including patches for two approaches
even!) that it seems did not see any response despite the nice
analysis (and the patch).

Mind having a look?

Gerald

PS: If you prefer, I can put this into Bugzilla, too.

On Fri, 28 Mar 2014, Andriy Gapon wrote:
> on 19/03/2014 12:03 Andriy Gapon said the following:
>> 
>> I observe the following minor annoyance on FreeBSD systems where cpp is GCC's
>> cpp.  If a DTrace script has the following shebang line:
>> #!/usr/sbin/dtrace -Cs
>> then the following warning is produced when the script is run:
>> cc1: warning:  is shorter than expected
>> 
>> Some details.  dtrace(1) first forks. Then a child seeks on a file 
>> descriptor associated with the script file, so that the shebang line is 
>> skipped (because otherwise it would confuse cpp).  Then the child makes 
>> the file descriptor its standard input and then it execs cpp.  cpp 
>> performs fstat(2) on its standard input descriptor and determines that 
>> it points to a regular file.  Then it verifies that a number of bytes 
>> it reads from the file is the same as a size of the file.  The check 
>> makes sense if the file is opened by cpp itself, but it does not always 
>> make sense for the stdin as described above.
>> 
>> The following patch seems to fix the issue, but perhaps there is a 
>> better / smarter alternative.
> A patch that implements a different approach has been committed in FreeBSD:
> https://github.com/freebsd/freebsd/commit/6ceecddbc
> Please consider.  Thanks!
> 
>> --- a/libcpp/files.c
>> +++ b/libcpp/files.c
>> @@ -601,7 +601,8 @@ read_file_guts (cpp_reader *pfile, _cpp_file *file)
>>return false;
>>  }
>> 
>> -  if (regular && total != size && STAT_SIZE_RELIABLE (file->st))
>> +  if (regular && total != size && file->fd != 0
>> +  && STAT_SIZE_RELIABLE (file->st))
>>  cpp_error (pfile, CPP_DL_WARNING,
>> "%s is shorter than expected", file->path);

Re: [PATCH, docs] invoke.texi: random copy-editing

[Gerald Pfeifer]

On Wed, 29 Aug 2012, Sandra Loosemore wrote:
>   * doc/invoke.texi: Fix numerous typos and punctuation/grammatical
>   errors throughout the file.  Re-word some awkward sentences and
>   paragraphs.

I noticed you changed return-value and return-type to their 
variants without a dash.  Would it make sense to add the 
following to https://gcc.gnu.org/codingconventions.html#Spelling ?

Gerald

Index: codingconventions.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/codingconventions.html,v
retrieving revision 1.76
diff -u -r1.76 codingconventions.html
--- codingconventions.html  16 Aug 2016 09:24:33 -  1.76
+++ codingconventions.html  1 Sep 2016 19:02:58 -
@@ -457,6 +457,11 @@
 
   
   
+"return type", "return value"
+"return-type", "return-value"
+
+  
+  
 "run time" (noun), "run-time" (adjective);
   the time at which the program is run
 "runtime"

[PATCH, fortran testsuite]: Correct the calls to c functions in gfortran.dg/c_by_val_1.f

[Uros Bizjak]

Hello!

Functions c_to_c__ and c_to_c8__ in the supplemental testcase c file
are prototyped as:

extern void c_to_c__ (complex float*, complex float, complex float*,
complex float**);
extern void c_to_c8__ (complex double*, complex double, complex
double*, complex double**);

so, we have to call them according to their prototypes from the fortran source.

2016-09-01  Uros Bizjak  

* gfortran.dg/c_by_val_1.f: Correct the call to c_to_c and c_to_c8.

Tested on x86_64-linux-gnu {,-m32}  and alphaev68-linux-gnu.

BTW: The testcase works by accident. When compiling the testcase, the
fortran compiler declares half of the arguments to these two functions
as unnamed (aka variable arguments), and the testcase fails on targets
that handle named and unnamed arguments in a different way.

OK for mainline and release branches?

Uros.
Index: gfortran.dg/c_by_val_1.f
===
--- gfortran.dg/c_by_val_1.f(revision 239943)
+++ gfortran.dg/c_by_val_1.f(working copy)
@@ -9,8 +9,8 @@
   real(8)a8, b8, c8
   integer(4) i, j, k
   integer(8) i8, j8, k8
-  complexu, v, w, c_to_c
-  complex(8) u8, v8, w8, c_to_c8
+  complexu, v, w
+  complex(8) u8, v8, w8
 
   a = 42.0
   b = 0.0
@@ -39,13 +39,13 @@
   u = (-1.0, 2.0)
   v = (1.0, -2.0)
   w = u
-  v = c_to_c (%VAL (u), %REF (w), %LOC (w))
+  call c_to_c (v, %VAL (u), %REF (w), %LOC (w))
   if ((4.0 * u).ne.v) call abort ()
 
   u8 = (-1.0, 2.0)
   v8 = (1.0, -2.0)
   w8 = u8
-  v8 = c_to_c8 (%VAL (u8), %REF (w8), %LOC (w8))
+  call c_to_c8 (v8, %VAL (u8), %REF (w8), %LOC (w8))
   if ((4.0 * u8).ne.v8) call abort ()
 
   stop

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

[Jerry DeLisle]

On 09/01/2016 03:25 AM, Andreas Schwab wrote:
> On Aug 31 2016, Jerry DeLisle  wrote:
> 
>> ! { dg-do run }
>> ! PR77393
>> program testbigf0 ! Can enormous numbers be printed with F0.0 format?
>>   implicit none
>>   character(1) :: str
>>   write(str, "(f0.0)") -huge(1.0) 
>>   if (len(trim(str)).lt.41) error stop "FAILED AT 9"
>>   write(str, "(f0.0)") -huge(1.0_8)
>>   if (len(trim(str)).lt.311) error stop "FAILED AT 9"
>>   write(str, "(f0.0)") -huge(1.0_10)
>>   if (len(trim(str)).lt.4935) error stop "FAILED AT 9"
>>   write(str, "(f0.10)") -huge(1.0_16)
>>   if (len(trim(str)).lt.4945) error stop "FAILED AT 11"
>> end program testbigf0
> 
> FAIL: gfortran.dg/fmt_f0_2.f90   -O0  (test for excess errors)
> Excess errors:
> /daten/aranym/gcc/gcc-20160901/gcc/testsuite/gfortran.dg/fmt_f0_2.f90:12:36: 
> Error: Invalid real kind 16 at (1)
> 
> Andreas.
> 

Oh dang, thats right.  I can fix that sometime today.

Thanks for report

Jerry

[PATCH 1/4] Hack: non-symbolic numeric constant warning

[Segher Boessenkool]

This patch prints a warning if a register number in the machine description
patterns is non-symbolic, to catch places that should use a constant from a
define_constants instead (e.g., LR_REGNO).

I hardcoded this to not warn for regno < 32, i.e. the GPRs on PowerPC.
That of course is not acceptable like this.  If anyone wants to pick
this up and make that a target check, feel free :-)  (The global variable
is not so hot either).

The next three patches in the series are what this patch uncovered; I'll
commit those to trunk.


Segher


---
 gcc/read-md.c  | 15 +++
 gcc/read-md.h  |  3 +++
 gcc/read-rtl.c |  2 ++
 3 files changed, 20 insertions(+)

diff --git a/gcc/read-md.c b/gcc/read-md.c
index b422d8d..7f5cdaf 100644
--- a/gcc/read-md.c
+++ b/gcc/read-md.c
@@ -94,6 +94,9 @@ static htab_t enum_types;
 
 static void handle_file (directive_handler_t);
 
+/* For read_name: print a message if a number is non-symbolic.  */
+int warn_if_non_symbolic_number;
+
 /* Given an object that starts with a char * name field, return a hash
code for its name.  */
 
@@ -450,6 +453,18 @@ read_name (struct md_name *name)
   name->buffer[i] = 0;
   name->string = name->buffer;
 
+  if (warn_if_non_symbolic_number)
+{
+  const char *p = name->string;
+  while (*p && ISSPACE (*p))
+   p++;
+  if ((ISDIGIT (*p) || *p == '-' || *p == '+') && atoi (p) >= 32)
+   {
+ file_location loc (read_md_filename, read_md_lineno);
+ message_at (loc, "numeric constant %s is a plain number", p);
+   }
+}
+
   if (md_constants)
 {
   /* Do constant expansion.  */
diff --git a/gcc/read-md.h b/gcc/read-md.h
index fa25951..6f9c34e 100644
--- a/gcc/read-md.h
+++ b/gcc/read-md.h
@@ -103,6 +103,9 @@ extern const char *read_md_filename;
 extern struct obstack string_obstack;
 extern void (*include_callback) (const char *);
 
+/* For read_name: print a message if a number is non-symbolic.  */
+extern int warn_if_non_symbolic_number;
+
 /* Read the next character from the MD file.  */
 
 static inline int
diff --git a/gcc/read-rtl.c b/gcc/read-rtl.c
index 4614e35..14e53cf 100644
--- a/gcc/read-rtl.c
+++ b/gcc/read-rtl.c
@@ -1311,7 +1311,9 @@ read_rtx_code (const char *code_name)
break;
 
   case 'r':
+   warn_if_non_symbolic_number = 1;
read_name (&name);
+   warn_if_non_symbolic_number = 0;
validate_const_int (name.string);
set_regno_raw (return_rtx, atoi (name.string), 1);
REG_ATTRS (return_rtx) = NULL;
-- 
1.9.3

[PATCH 4/4] rs6000: Rename 110 -> VSCR_REGNO

[Segher Boessenkool]

2016-09-01  Segher Boessenkool  

* config/rs6000/altivec.md: Use VSCR_REGNO instead of 110 throughout.

---
 gcc/config/rs6000/altivec.md | 37 +++--
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
index 335c052..857f257 100644
--- a/gcc/config/rs6000/altivec.md
+++ b/gcc/config/rs6000/altivec.md
@@ -508,7 +508,7 @@ (define_insn "altivec_vaddus"
 (unspec:VI [(match_operand:VI 1 "register_operand" "v")
(match_operand:VI 2 "register_operand" "v")]
   UNSPEC_VADDU))
-   (set (reg:SI 110) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
+   (set (reg:SI VSCR_REGNO) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
   ""
   "vaddus %0,%1,%2"
   [(set_attr "type" "vecsimple")])
@@ -518,7 +518,7 @@ (define_insn "altivec_vaddss"
 (unspec:VI [(match_operand:VI 1 "register_operand" "v")
 (match_operand:VI 2 "register_operand" "v")]
   UNSPEC_VADDS))
-   (set (reg:SI 110) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
+   (set (reg:SI VSCR_REGNO) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
   "VECTOR_UNIT_ALTIVEC_P (mode)"
   "vaddss %0,%1,%2"
   [(set_attr "type" "vecsimple")])
@@ -554,7 +554,7 @@ (define_insn "altivec_vsubus"
 (unspec:VI [(match_operand:VI 1 "register_operand" "v")
 (match_operand:VI 2 "register_operand" "v")]
   UNSPEC_VSUBU))
-   (set (reg:SI 110) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
+   (set (reg:SI VSCR_REGNO) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
   "VECTOR_UNIT_ALTIVEC_P (mode)"
   "vsubus %0,%1,%2"
   [(set_attr "type" "vecsimple")])
@@ -564,7 +564,7 @@ (define_insn "altivec_vsubss"
 (unspec:VI [(match_operand:VI 1 "register_operand" "v")
 (match_operand:VI 2 "register_operand" "v")]
   UNSPEC_VSUBS))
-   (set (reg:SI 110) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
+   (set (reg:SI VSCR_REGNO) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
   "VECTOR_UNIT_ALTIVEC_P (mode)"
   "vsubss %0,%1,%2"
   [(set_attr "type" "vecsimple")])
@@ -830,7 +830,7 @@ (define_insn "altivec_vmsumuhs"
  (match_operand:V8HI 2 "register_operand" "v")
   (match_operand:V4SI 3 "register_operand" "v")]
 UNSPEC_VMSUMUHS))
-   (set (reg:SI 110) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
+   (set (reg:SI VSCR_REGNO) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
   "TARGET_ALTIVEC"
   "vmsumuhs %0,%1,%2,%3"
   [(set_attr "type" "veccomplex")])
@@ -841,7 +841,7 @@ (define_insn "altivec_vmsumshs"
  (match_operand:V8HI 2 "register_operand" "v")
   (match_operand:V4SI 3 "register_operand" "v")]
 UNSPEC_VMSUMSHS))
-   (set (reg:SI 110) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
+   (set (reg:SI VSCR_REGNO) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
   "TARGET_ALTIVEC"
   "vmsumshs %0,%1,%2,%3"
   [(set_attr "type" "veccomplex")])
@@ -902,7 +902,7 @@ (define_insn "altivec_vmhaddshs"
  (match_operand:V8HI 2 "register_operand" "v")
   (match_operand:V8HI 3 "register_operand" "v")]
 UNSPEC_VMHADDSHS))
-   (set (reg:SI 110) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
+   (set (reg:SI VSCR_REGNO) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
   "TARGET_ALTIVEC"
   "vmhaddshs %0,%1,%2,%3"
   [(set_attr "type" "veccomplex")])
@@ -913,7 +913,7 @@ (define_insn "altivec_vmhraddshs"
  (match_operand:V8HI 2 "register_operand" "v")
   (match_operand:V8HI 3 "register_operand" "v")]
 UNSPEC_VMHRADDSHS))
-   (set (reg:SI 110) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
+   (set (reg:SI VSCR_REGNO) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
   "TARGET_ALTIVEC"
   "vmhraddshs %0,%1,%2,%3"
   [(set_attr "type" "veccomplex")])
@@ -1699,7 +1699,7 @@ (define_insn "altivec_vsum4ubs"
 (unspec:V4SI [(match_operand:V16QI 1 "register_operand" "v")
   (match_operand:V4SI 2 "register_operand" "v")]
 UNSPEC_VSUM4UBS))
-   (set (reg:SI 110) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
+   (set (reg:SI VSCR_REGNO) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
   "TARGET_ALTIVEC"
   "vsum4ubs %0,%1,%2"
   [(set_attr "type" "veccomplex")])
@@ -1709,7 +1709,7 @@ (define_insn "altivec_vsum4ss"
 (unspec:V4SI [(match_operand:VIshort 1 "register_operand" "v")
   (match_operand:V4SI 2 "register_operand" "v")]
 UNSPEC_VSUM4S))
-   (set (reg:SI 110) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
+   (set (reg:SI VSCR_REGNO) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
   "TARGET_ALTIVEC"
   "vsum4ss %0,%1,%2"
   [(set_attr "type" "veccomplex")])
@@ -1722,7 +1722,7 @@ (define_insn "altivec_vsum2sws"
 (unspec:V4SI [(

[PATCH 3/4] rs6000: Rename 109 -> VRSAVE_REGNO

[Segher Boessenkool]

2016-09-01  Segher Boessenkool  

* config/rs6000/altivec.md: Use VRSAVE_REGNO instead of 109 throughout.

---
 gcc/config/rs6000/altivec.md | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
index 480e64e..335c052 100644
--- a/gcc/config/rs6000/altivec.md
+++ b/gcc/config/rs6000/altivec.md
@@ -371,7 +371,7 @@ (define_split
 
 (define_insn "get_vrsave_internal"
   [(set (match_operand:SI 0 "register_operand" "=r")
-   (unspec:SI [(reg:SI 109)] UNSPEC_GET_VRSAVE))]
+   (unspec:SI [(reg:SI VRSAVE_REGNO)] UNSPEC_GET_VRSAVE))]
   "TARGET_ALTIVEC"
 {
   if (TARGET_MACHO)
@@ -383,9 +383,9 @@ (define_insn "get_vrsave_internal"
 
 (define_insn "*set_vrsave_internal"
   [(match_parallel 0 "vrsave_operation"
- [(set (reg:SI 109)
+ [(set (reg:SI VRSAVE_REGNO)
   (unspec_volatile:SI [(match_operand:SI 1 "register_operand" "r")
-   (reg:SI 109)] UNSPECV_SET_VRSAVE))])]
+   (reg:SI VRSAVE_REGNO)] UNSPECV_SET_VRSAVE))])]
   "TARGET_ALTIVEC"
 {
   if (TARGET_MACHO)
-- 
1.9.3

[PATCH 2/4] rs6000: Rename 74 -> CR6_REGNO

[Segher Boessenkool]

2016-09-01  Segher Boessenkool  

* config/rs6000/altivec.md: Use CR6_REGNO instead of 74 throughout.
* config/rs6000/vector.md: Ditto.
* config/rs6000/vsx.md: Ditto.

---
 gcc/config/rs6000/altivec.md | 30 +++---
 gcc/config/rs6000/vector.md  | 16 
 gcc/config/rs6000/vsx.md |  6 +++---
 3 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
index 25472c29..480e64e 100644
--- a/gcc/config/rs6000/altivec.md
+++ b/gcc/config/rs6000/altivec.md
@@ -2274,7 +2274,7 @@ (define_insn "altivec_vupklpx"
 ;; Compare vectors producing a vector result and a predicate, setting CR6 to
 ;; indicate a combined status
 (define_insn "*altivec_vcmpequ_p"
-  [(set (reg:CC 74)
+  [(set (reg:CC CR6_REGNO)
(unspec:CC [(eq:CC (match_operand:VI2 1 "register_operand" "v")
   (match_operand:VI2 2 "register_operand" "v"))]
   UNSPEC_PREDICATE))
@@ -2286,7 +2286,7 @@ (define_insn "*altivec_vcmpequ_p"
   [(set_attr "type" "veccmpfx")])
 
 (define_insn "*altivec_vcmpgts_p"
-  [(set (reg:CC 74)
+  [(set (reg:CC CR6_REGNO)
(unspec:CC [(gt:CC (match_operand:VI2 1 "register_operand" "v")
   (match_operand:VI2 2 "register_operand" "v"))]
   UNSPEC_PREDICATE))
@@ -2298,7 +2298,7 @@ (define_insn "*altivec_vcmpgts_p"
   [(set_attr "type" "veccmpfx")])
 
 (define_insn "*altivec_vcmpgtu_p"
-  [(set (reg:CC 74)
+  [(set (reg:CC CR6_REGNO)
(unspec:CC [(gtu:CC (match_operand:VI2 1 "register_operand" "v")
(match_operand:VI2 2 "register_operand" "v"))]
   UNSPEC_PREDICATE))
@@ -2310,7 +2310,7 @@ (define_insn "*altivec_vcmpgtu_p"
   [(set_attr "type" "veccmpfx")])
 
 (define_insn "*altivec_vcmpeqfp_p"
-  [(set (reg:CC 74)
+  [(set (reg:CC CR6_REGNO)
(unspec:CC [(eq:CC (match_operand:V4SF 1 "register_operand" "v")
   (match_operand:V4SF 2 "register_operand" "v"))]
   UNSPEC_PREDICATE))
@@ -2322,7 +2322,7 @@ (define_insn "*altivec_vcmpeqfp_p"
   [(set_attr "type" "veccmp")])
 
 (define_insn "*altivec_vcmpgtfp_p"
-  [(set (reg:CC 74)
+  [(set (reg:CC CR6_REGNO)
(unspec:CC [(gt:CC (match_operand:V4SF 1 "register_operand" "v")
   (match_operand:V4SF 2 "register_operand" "v"))]
   UNSPEC_PREDICATE))
@@ -2334,7 +2334,7 @@ (define_insn "*altivec_vcmpgtfp_p"
   [(set_attr "type" "veccmp")])
 
 (define_insn "*altivec_vcmpgefp_p"
-  [(set (reg:CC 74)
+  [(set (reg:CC CR6_REGNO)
(unspec:CC [(ge:CC (match_operand:V4SF 1 "register_operand" "v")
   (match_operand:V4SF 2 "register_operand" "v"))]
   UNSPEC_PREDICATE))
@@ -2346,7 +2346,7 @@ (define_insn "*altivec_vcmpgefp_p"
   [(set_attr "type" "veccmp")])
 
 (define_insn "altivec_vcmpbfp_p"
-  [(set (reg:CC 74)
+  [(set (reg:CC CR6_REGNO)
(unspec:CC [(match_operand:V4SF 1 "register_operand" "v")
(match_operand:V4SF 2 "register_operand" "v")]
   UNSPEC_VCMPBFP))
@@ -3634,7 +3634,7 @@ (define_insn "bcd"
  (match_operand:V1TI 2 "register_operand" "")
  (match_operand:QI 3 "const_0_to_1_operand" "")]
 UNSPEC_BCD_ADD_SUB))
-   (clobber (reg:CCFP 74))]
+   (clobber (reg:CCFP CR6_REGNO))]
   "TARGET_P8_VECTOR"
   "bcd. %0,%1,%2,%3"
   [(set_attr "length" "4")
@@ -3646,7 +3646,7 @@ (define_insn "bcd"
 ;; probably should be one that can go in the VMX (Altivec) registers, so we
 ;; can't use DDmode or DFmode.
 (define_insn "*bcd_test"
-  [(set (reg:CCFP 74)
+  [(set (reg:CCFP CR6_REGNO)
(compare:CCFP
 (unspec:V2DF [(match_operand:V1TI 1 "register_operand" "v")
   (match_operand:V1TI 2 "register_operand" "v")
@@ -3665,7 +3665,7 @@ (define_insn "*bcd_test2"
  (match_operand:V1TI 2 "register_operand" "v")
  (match_operand:QI 3 "const_0_to_1_operand" "i")]
 UNSPEC_BCD_ADD_SUB))
-   (set (reg:CCFP 74)
+   (set (reg:CCFP CR6_REGNO)
(compare:CCFP
 (unspec:V2DF [(match_dup 1)
   (match_dup 2)
@@ -3699,7 +3699,7 @@ (define_insn "darn"
   [(set_attr "type" "integer")])
 
 (define_expand "bcd_"
-  [(parallel [(set (reg:CCFP 74)
+  [(parallel [(set (reg:CCFP CR6_REGNO)
   (compare:CCFP
(unspec:V2DF [(match_operand:V1TI 1 "register_operand" "")
  (match_operand:V1TI 2 "register_operand" "")
@@ -3708,7 +3708,7 @@ (define_expand "bcd_"
(match_dup 4)))
  (clobber (match_scratch:V1TI 5 ""))])
(set (match_operand:SI 0 "register_operand" "")
-   (BCD_TEST:SI (reg:CCFP 74)
+   (BCD_TEST:SI (reg:CCFP CR6_REGNO)
 (const_int 0)))]
   "TARGET_P8_VECTOR"
 {
@@ -3727,8 +3

Re: [PATCH, docs] invoke.texi: random copy-editing

[Gerald Pfeifer]

On Wed, 29 Aug 2012, Sandra Loosemore wrote:
>   * doc/invoke.texi: Fix numerous typos and punctuation/grammatical
>   errors throughout the file.  Re-word some awkward sentences and
>   paragraphs.

There are three questions (and to some extent suggestions) on this 
patch and the text covered by it that I'm wondering about.  Hope
that's still fine after all the time.

I'm happy to make any changes myself, but am looking at your expertise.


 Item 11:  Define a copy constructor and an assignment operator for classes
-with dynamically allocated memory.
+with dynamically-allocated memory.

Why the dash here?  Is this because it's seens as a technical term?  
(Usually it's the Germans with those absolutelylongandnonbreaking words. 
;-)


-(C++ only) A base class is not initialized in a derived class' copy
+(C++ only) A base class is not initialized in a derived class's copy
 constructor.

"class's" twists my brain a little.  What do you think about using
"in a copy constructor of a derived class" instead?


 When profile feedback is available (see @option{-fprofile-generate}) the actual
-recursion depth can be guessed from probability that function will recurse via
-given call expression.  This parameter limits inlining only to call expression
-whose probability exceeds given threshold (in percents).  The default value is
-10.
+recursion depth can be guessed from probability that function recurses via a
+given call expression.  This parameter limits inlining only to call expressions
+whose probability exceeds the given threshold (in percents).

This predates your patch, but should this be "the probability"?

Gerald