Re: [RFC][IPA-VRP] Early VRP Implementation

[Richard Biener]

On September 14, 2016 11:36:16 PM GMT+02:00, Jan Hubicka  wrote:
>> +  /* Visit PHI stmts and discover any new VRs possible.  */
>> +  gimple_stmt_iterator gsi;
>> +  for (gphi_iterator gpi = gsi_start_phis (bb);
>> +   !gsi_end_p (gpi); gsi_next ())
>> +{
>> +  gphi *phi = gpi.phi ();
>> +  tree lhs = PHI_RESULT (phi);
>> +  value_range vr_result = VR_INITIALIZER;
>> +  if (! has_unvisived_preds
>>&& stmt_interesting_for_vrp (phi)
>> + && stmt_visit_phi_node_in_dom_p (phi))
>> +   extract_range_from_phi_node (phi, _result, true);
>> +  else
>> +   set_value_range_to_varying (_result);
>> +  update_value_range (lhs, _result);
>> +}
>> 
>> due to a bug in IRA you need to make sure to un-set BB_VISITED after
>> early-vrp is finished again.
>How IRA bugs affects early passes?

IRA bogously relies on BB_VISITED being cleared at pass start.

Richard.

>Honza

Re: [PATCH 1/8] change a few rtx_insn * to rtx_jump_insn *

[Kaz Kojima]

Trevor Saunders  wrote:
>> This hunk results several new failures for tree-profile tests on SH.
>> If the line "if (JUMP_P (cur_insn))" is restored, those failures
>> go away.
> 
> That's interesting because dyn_cast should include that check.  What is
> the error?

Here is a typical log:

spawn -ignore SIGHUP /home/ldroot/dodes/xsh-gcc/gcc/xgcc 
-B/home/ldroot/dodes/xsh-gcc/gcc/ 
/exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.dg/tree-prof/20041218-1.c 
-fno-diagnostics-show-caret -fdiagnostics-color=never -O2 
-freorder-blocks-and-partition -fprofile-use -D_PROFILE_USE -lm -o 
/home/ldroot/dodes/xsh-gcc/gcc/testsuite/gcc/20041218-1.x02
/exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.dg/tree-prof/20041218-1.c: In 
function 'check':
/exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.dg/tree-prof/20041218-1.c:55:1: 
internal compiler error: Segmentation fault
0x86cd24a crash_signal
../../LOCAL/trunk/gcc/toplev.c:336
0x8b4cf5e fix_crossing_unconditional_branches
../../LOCAL/trunk/gcc/bb-reorder.c:2214
0x8b4e18a execute
../../LOCAL/trunk/gcc/bb-reorder.c:2933

Regards,
kaz

Re: [PATCH 1/8] change a few rtx_insn * to rtx_jump_insn *

[Trevor Saunders]

On Thu, Sep 15, 2016 at 01:04:04PM +0900, Kaz Kojima wrote:
> tbsaunde+...@tbsaunde.org wrote:
> > @@ -2201,8 +2201,7 @@ fix_crossing_unconditional_branches (void)
> > {
> >   if (!BARRIER_P (cur_insn))
> > BLOCK_FOR_INSN (cur_insn) = cur_bb;
> > - if (JUMP_P (cur_insn))
> > -   jump_insn = cur_insn;
> > + jump_insn = dyn_cast (cur_insn);
> > }
> 
> This hunk results several new failures for tree-profile tests on SH.
> If the line "if (JUMP_P (cur_insn))" is restored, those failures
> go away.

That's interesting because dyn_cast should include that check.  What is
the error?

Thanks!

Trev

> 
> Regards,
>   kaz

Re: [PATCH 1/8] change a few rtx_insn * to rtx_jump_insn *

[Kaz Kojima]

tbsaunde+...@tbsaunde.org wrote:
> @@ -2201,8 +2201,7 @@ fix_crossing_unconditional_branches (void)
>   {
> if (!BARRIER_P (cur_insn))
>   BLOCK_FOR_INSN (cur_insn) = cur_bb;
> -   if (JUMP_P (cur_insn))
> - jump_insn = cur_insn;
> +   jump_insn = dyn_cast (cur_insn);
>   }

This hunk results several new failures for tree-profile tests on SH.
If the line "if (JUMP_P (cur_insn))" is restored, those failures
go away.

Regards,
kaz

Re: [patch] [gsoc] [gimplefe] GIMPLE FE Project

[Prasad Ghangal]

On 14 September 2016 at 18:54, Richard Biener
 wrote:
> On Fri, Sep 9, 2016 at 12:38 PM, Prasad Ghangal
>  wrote:
>> On 26 August 2016 at 14:28, Richard Biener  
>> wrote:
>>> On Fri, Aug 26, 2016 at 5:08 AM, Prasad Ghangal
>>>  wrote:
 On 24 August 2016 at 15:32, Richard Biener  
 wrote:
> On Mon, Aug 22, 2016 at 8:40 PM, Prasad Ghangal
>  wrote:
>> On 22 August 2016 at 16:55, Trevor Saunders  
>> wrote:
>>> On Sun, Aug 21, 2016 at 10:35:17PM +0530, Prasad Ghangal wrote:
 Hi all,

 As a part of my gsoc project. I have completed the following tasks:

 * Parsed gimple-expression
 * Parsed gimple-labels
 * Parsed local declaration
 * Parsed gimple-goto statement
 * Parsed gimple-if-else statement
 * Parsed gimple-switch statement
 * Parsed gimple-return statement
 * Parsed gimple-PHI function
 * Parsed gimple ssa-names along with default def
 * Parsed gimple-call

 * Hacked pass manager to add support for startwith (pass-name) to skip
 early opt passes
 * Modified gimple dump for making it parsable

 I am willing to continue work on the project, some TODOs for the 
 projects are:

 * Error handling
 * Parse more gimple syntax
 * Add startwith support for IPA passes

 The complete code of gimple fe project can be found at
 https://github.com/PrasadG193/gcc_gimple_fe


 PFA patch for complete project (rebased for latest trunk revision).
 I have successfully bootstrapped and tested on x86_64-pc-linux-gnu.
 Some testcases failed due to modified gimple dump as expected.


 Thanks,
 Prasad
>>>
>>> only some rather minor comments
>>>
>>>
>>> +++ b/gcc/c/c-parser.c
>>> @@ -59,6 +59,18 @@ along with GCC; see the file COPYING3.  If not see
>>>  #include "gimple-expr.h"
>>>  #include "context.h"
>>>  #include "gcc-rich-location.h"
>>> +#include "tree-vrp.h"
>>>
>>> given that you need these headers it might be better to put most of the
>>> gimple parsing in its own file so only what actually needs to know about
>>> this part of the compiler does now about it.
>>>
>>> +void
>>> +c_parser_parse_gimple_body (c_parser *parser)
>>> +{
>>> +  bool return_p = false;
>>> +  gimple_seq seq;
>>> +  gimple_seq body;
>>> +  tree stmt = push_stmt_list ();
>>>
>>> it would be nice to move the declarations down to their first use.
>>>
>>> +  gimple *ret;
>>> +  ret = gimple_build_return (NULL);
>>>
>>> there's no reason for a separate declaration and assignment ;)
>>>
>>> +  tree block = NULL;
>>> +  block = pop_scope ();
>>>
>>> same here, and a number of other places.
>>>
>>> +c_parser_gimple_compound_statement (c_parser *parser, gimple_seq *seq)
>>> +{
>>> +  bool return_p = false;
>>> +
>>> +  if (!c_parser_require (parser, CPP_OPEN_BRACE, "expected %<{%>"))
>>> +  return return_p;
>>>
>>> return false would work fine.
>>>
>>> +
>>> +  if (c_parser_next_token_is (parser, CPP_CLOSE_BRACE))
>>> +{
>>> +  c_parser_consume_token (parser);
>>> +  goto out;
>>>
>>> I don't see the need for the gotos, there's no cleanup in this function.
>>>
>>> +  /* gimple PHI expression.  */
>>> +  if (c_parser_next_token_is_keyword (parser, RID_PHI))
>>> +{
>>> +  c_parser_consume_token (parser);
>>> +
>>> +  if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
>>> +   {
>>> + return;
>>> +   }
>>> +
>>> +  gcall *call_stmt;
>>> +  tree arg = NULL_TREE;
>>> +  vec vargs = vNULL;
>>>
>>> I think you can use auto_vec here, as is I think this leaks the vectors
>>> storage.
>>>
>>> +c_parser_gimple_binary_expression (c_parser *parser, enum tree_code 
>>> *subcode)
>>>
>>> you can skip the explicit 'enum' keyword.
>>>
>>> +  struct {
>>> +/* The expression at this stack level.  */
>>> +struct c_expr expr;
>>>
>>> similar with struct here.
>>>
>>> +/* The precedence of the operator on its left, PREC_NONE at the
>>> +   bottom of the stack.  */
>>> +enum c_parser_prec prec;
>>> +/* The operation on its left.  */
>>> +enum tree_code op;
>>> +/* The source location of this operation.  */
>>> +location_t loc;
>>> +  } stack[2];
>>> +  int sp;
>>> +  /* Location of the binary operator.  */

Re: [PATCH] PR fortran/77420 -- take two

[Jerry DeLisle]

On 09/14/2016 02:49 PM, Steve Kargl wrote:

The attached patch appears to fix PR fortran/77420 without
causing regressions.  The problem raised by Andrew Benson at
https://gcc.gnu.org/ml/fortran/2016-09/msg00039.html
contained in pr77420_3.f90 and pr77420_4.f90.  The original
testcase from the PR is in pr77420_1.f90 and a variation
on that testcase is in pr77420_2.f90.  Patch has been
regression tested on x86_64-*-freebsd.  OK to commit?


Yes OK,

Thanks  Steve



2016-09-14  Steven G. Kargl  

PR fortran/77420
* trans-common.c:  Handle array elements in equivalence when
the lower and upper bounds of array spec are NULL.

2016-09-14  Steven G. Kargl  

PR fortran/77420
* gfortran.dg/pr77420_1.f90: New test.
* gfortran.dg/pr77420_2.f90: Ditto.
* gfortran.dg/pr77420_3.f90: New test. Requires ...
* gfortran.dg/pr77420_4.f90: this file.

Re: [PATCH, 5.x/6.x/7.x] Be more conservative in early inliner if FDO is enabled

[Yuan, Pengfei]

> I think the approach is reasonable though it might still have
> interesting effects on
> optimization for very small growth.  So for further experimenting it
> would be nice

Test it on SPEC CPU with FDO enabled?

> to have a separate PARAM_EARLY_FDO_INLINING_INSNS or maybe simply
> adjust the PARAM_EARLY_INLINING_INSNS default accordingly when FDO is
> enabled?

Setting PARAM_EARLY_INLINING_INSNS to 0 when FDO is enabled should be
equivalent to my patch.

Regards,
Yuan, Pengfei
 
> I'll let Honza also double-check the condition detecting FDO (it looks
> like we should
> have some abstraction for that).
> 
> Thanks,
> Richard.

Re: [PING] set libfunc entry for sdivmod_optab to NULL in optabs.def

[Richard Sandiford]

Richard Sandiford  writes:
> Prathamesh Kulkarni  writes:
>> Hi,
>> I would like to ping the following patch:
>> https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01015.html
>>
>> While implementing divmod transform:
>> https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01757.html
>>
>> I ran into an  issue with optab_libfunc().
>> It appears optab_libfunc (sdivmod_optab, mode) returns
>> bogus libfunc for unsupported modes, for instance
>> on x86_64, optab_libfunc (sdivmod_optab, DImode) returns
>> a libfunc with name "__divmoddi4", even though such a libfunc
>> does not exist in libgcc. This happens because in optabs.def
>> the libfunc entry for sdivmod_optab has gen_int_libfunc,
>> and call to optab_libfunc (sdivmo_optab, DImode) lazily
>> creates a bogus libfunc "__divmoddi4" by calling gen_int_libfunc().
>>
>> To work around this issue I set libfunc entry for sdivmod_optab to NULL
>> and verified that optab_libfunc (sdivmod_optab, DImode) returns NULL_RTX
>> instead of a bogus libfunc if it's not overriden by the target.
>>
>> Bootstrapped and tested on ppc64le-linux-gnu, x86_64-linux-gnu.
>> Cross tested on arm*-*-*, aarch64*-*-*.
>> OK for trunk ?
>
> I'm not a maintainer for this area, but:

...in https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01757.html
you said that c6x follows the return-by-pointer convention.
I'm no c6x expert, but from a quick look, I think its divrem
function returns a div/mod pair in A4/A5, which matches the
ARM convention of returning both results by value.

Does anyone know if the optab function registered by the SPU
backend is ever called directly?

You mention that this is latent as far as expand_twoval_binop_libfunc
is concerned.  AIUI expand_twoval_binop_libfunc implements the ARM/c6x
convention and expects the two values to be returned as a pair.
It then extracts one half of the pair and discards the other.
So my worry is that we're leaving the udivmod entry intact even though
the standard __udivmoddi4 doesn't do what expand_twoval_binop_libfunc
expects.

Would it make sense to set both entries to null and treat __udivmoddi4
as a non-optab function?  ARM and c6x could then continue to register
their current optab functions and a non-null optab function would
indicate a return value pair.

Sorry if this has already been covered.

Thanks,
Richard

Re: [PING] set libfunc entry for sdivmod_optab to NULL in optabs.def

[Richard Sandiford]

Prathamesh Kulkarni  writes:
> Hi,
> I would like to ping the following patch:
> https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01015.html
>
> While implementing divmod transform:
> https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01757.html
>
> I ran into an  issue with optab_libfunc().
> It appears optab_libfunc (sdivmod_optab, mode) returns
> bogus libfunc for unsupported modes, for instance
> on x86_64, optab_libfunc (sdivmod_optab, DImode) returns
> a libfunc with name "__divmoddi4", even though such a libfunc
> does not exist in libgcc. This happens because in optabs.def
> the libfunc entry for sdivmod_optab has gen_int_libfunc,
> and call to optab_libfunc (sdivmo_optab, DImode) lazily
> creates a bogus libfunc "__divmoddi4" by calling gen_int_libfunc().
>
> To work around this issue I set libfunc entry for sdivmod_optab to NULL
> and verified that optab_libfunc (sdivmod_optab, DImode) returns NULL_RTX
> instead of a bogus libfunc if it's not overriden by the target.
>
> Bootstrapped and tested on ppc64le-linux-gnu, x86_64-linux-gnu.
> Cross tested on arm*-*-*, aarch64*-*-*.
> OK for trunk ?

I'm not a maintainer for this area, but:

Re: [PATCH 5/9] regrename: Don't run if function was separately shrink-wrapped

[Segher Boessenkool]

On Wed, Sep 14, 2016 at 01:35:57PM -0600, Jeff Law wrote:
> On 09/14/2016 01:03 PM, Segher Boessenkool wrote:
> >
> >(There is no return insn at those exits; these are exits *without*
> >successor block, not the exit block).
> Hmm, I thought these were return blocks, but you're saying they're 
> no-return blocks?  I missed that.
> 
> In that case, aren't the restores dead because no callers can observe 
> their value changing unexpectedly?

Yes, but DCE can not remove the insns because dwarf2cfi would throw a
fit (and for good reason).  That is why adding all these USEs makes
the DCE patch unnecessary (that patch simply disallows deleting insns
with a REG_CFA_RESTORE note, much simpler than all that USE surgery,
and perfectly correct and pretty much the best we can do).

The problem is that regrename decides to rename a (volatile) register to
some non-volatile register that is *not* separately shrink-wrapped (and
not actually dead there).  Why would it do that, why would it not do that
if not separately shrink-wrapping at all?

(I did run this with checking=yes,rtl btw, it is not simple corruption).


Segher

Re: [PATCH 5/9] regrename: Don't run if function was separately shrink-wrapped

[Segher Boessenkool]

On Wed, Sep 14, 2016 at 01:33:04PM -0600, Jeff Law wrote:
> On 09/14/2016 01:03 PM, Segher Boessenkool wrote:
> >>If you think about it, conceptually we want the return insn to make the
> >>callee saved registers "used" so that DCE, regrename and friends don't
> >>muck with them.  The fact that we don't is as much never having to care
> >>all that much until recently.
> >
> >(There is no return insn at those exits; these are exits *without*
> >successor block, not the exit block).
> Ugh.  Anywhere we could attach this stuff in the insn chain?  If not, 
> the DF side of this problem gets uglier.

I put the USEs at the start of that noreturn basic block.

> >It is puzzling to me why adding USEs for just the registers that *are*
> >separately shrink-wrapped does not work; only all those that *could* be
> >shrink-wrapped does.  Do you have any idea about that?
> Nope.  No clue on that one.  It almost seems like a book-keeping error 
> somewhere.

Right.

> >>I pondered just doing it for the separately wrapped components on that
> >>particular path, but I've yet to convince myself that's actually correct.
> >
> >If that is not correct, how is the status quo correct?  That is what
> >puzzles me above, too.
> I couldn't convince myself that we could trivially find all the 
> separately wrapped components on a particular path -- ie, we may have 
> had components saved/restored in some sub-graph.  If an exit point from 
> the function is reachable from that sub-graph, then we need to make sure 
> any components from the subgraph are marked as live in addition to those 
> which are restored on the exit path.
> 
> While it is just a reachability problem, I don't think we need to solve 
> it if we mark anything that was separately shrink wrapped as live at all 
> the exit points.

Agreed, but why does it work if not separately shrink-wrapping anything?
And why does it break on things that are *not* separately wrapped *anywhere*?

> >There is a well-defined way to turn it off, via common/config/*/*-common.c 
> >,
> >TARGET_OPTION_OPTIMIZATION_TABLE.  We disagree on whether most targets will
> >want it enabled, i.e. whether it should (eventually) be enabled by default.
> I'm aware of TARGET_OPTION_OPTIMIZATION_TABLE -- I was thinking more 
> along the lines of trying to describe the conditions under which it is 
> not profitable and expose that.  TARGET_OPTION_OPTIMIZATION_TABLE is a 
> lame way to attack this problem.

Better than a target macro ;-)


Segher

Re: [PATCH 3/9] selftest.h: add temp_override fixture

[Trevor Saunders]

On Thu, Sep 08, 2016 at 08:30:47PM -0400, David Malcolm wrote:
> We have a lot of global state in our code.  Ideally we'd reduce the
> amount of such global state, but a prerequisite for sane refactoring
> is having automated testing in place to ensure that the refactoring
> doesn't break anything.
> 
> However, the global state itself makes it hard to write such automated
> testing.
> 
> To break this Catch-22, this patch introduces a class temp_override,
> for temporarily assigning a value to a global variable, saving the old
> value, and then restoring that old value in a dtor.

I expect there are uses for this outside of selftests, so I'd suggest
making it more general.  I guess the name is good enough, I would have
done auto_restore, but this is fine.  You just need to put it somewhere
generally accessible though I'm not sure what that would be.

Trev

[PATCH] PR fortran/77420 -- take two

[Steve Kargl]

The attached patch appears to fix PR fortran/77420 without
causing regressions.  The problem raised by Andrew Benson at
https://gcc.gnu.org/ml/fortran/2016-09/msg00039.html
contained in pr77420_3.f90 and pr77420_4.f90.  The original
testcase from the PR is in pr77420_1.f90 and a variation
on that testcase is in pr77420_2.f90.  Patch has been
regression tested on x86_64-*-freebsd.  OK to commit?

2016-09-14  Steven G. Kargl  

PR fortran/77420
* trans-common.c:  Handle array elements in equivalence when
the lower and upper bounds of array spec are NULL.
 
2016-09-14  Steven G. Kargl  

PR fortran/77420
* gfortran.dg/pr77420_1.f90: New test.
* gfortran.dg/pr77420_2.f90: Ditto.
* gfortran.dg/pr77420_3.f90: New test. Requires ...
* gfortran.dg/pr77420_4.f90: this file.

-- 
Steve
Index: gcc/fortran/trans-common.c
===
--- gcc/fortran/trans-common.c	(revision 240140)
+++ gcc/fortran/trans-common.c	(working copy)
@@ -805,13 +805,21 @@ element_number (gfc_array_ref *ar)
   if (ar->dimen_type[i] != DIMEN_ELEMENT)
 gfc_internal_error ("element_number(): Bad dimension type");
 
-  mpz_sub (n, *get_mpz (ar->start[i]), *get_mpz (as->lower[i]));
+  if (as && as->lower[i])
+	mpz_sub (n, *get_mpz (ar->start[i]), *get_mpz (as->lower[i]));
+  else
+	mpz_sub_ui (n, *get_mpz (ar->start[i]), 1);
  
   mpz_mul (n, n, multiplier);
   mpz_add (offset, offset, n);
  
-  mpz_sub (extent, *get_mpz (as->upper[i]), *get_mpz (as->lower[i]));
-  mpz_add_ui (extent, extent, 1);
+  if (as && as->upper[i] && as->lower[i])
+	{
+	  mpz_sub (extent, *get_mpz (as->upper[i]), *get_mpz (as->lower[i]));
+	  mpz_add_ui (extent, extent, 1);
+	}
+  else
+	mpz_set_ui (extent, 0);
  
   if (mpz_sgn (extent) < 0)
 mpz_set_ui (extent, 0);
Index: gcc/testsuite/gfortran.dg/pr77420_1.f90
===
--- gcc/testsuite/gfortran.dg/pr77420_1.f90	(nonexistent)
+++ gcc/testsuite/gfortran.dg/pr77420_1.f90	(working copy)
@@ -0,0 +1,15 @@
+! { dg-do compile }
+module test_equivalence
+  real, private :: array1(100)
+  real, private :: array2(100)
+  equivalence(array1(3),array2(3))
+end module test_equivalence
+
+module mymodule
+  use test_equivalence
+  real, dimension(:), allocatable :: array1
+end module mymodule
+
+program test
+  use mymodule
+end program test
Index: gcc/testsuite/gfortran.dg/pr77420_2.f90
===
--- gcc/testsuite/gfortran.dg/pr77420_2.f90	(nonexistent)
+++ gcc/testsuite/gfortran.dg/pr77420_2.f90	(working copy)
@@ -0,0 +1,15 @@
+! { dg-do compile }
+module test_equivalence
+  real, private :: array1(100)
+  real, private :: array2(100)
+  equivalence(array1,array2)
+end module test_equivalence
+
+module mymodule
+  use test_equivalence
+  real, dimension(:), allocatable :: array1
+end module mymodule
+
+program test
+  use mymodule
+end program test
Index: gcc/testsuite/gfortran.dg/pr77420_3.f90
===
--- gcc/testsuite/gfortran.dg/pr77420_3.f90	(nonexistent)
+++ gcc/testsuite/gfortran.dg/pr77420_3.f90	(working copy)
@@ -0,0 +1,9 @@
+! { dg-do link }
+! { dg-additional-sources pr77420_4.f90 }
+!
+module h5global
+  implicit none
+  integer :: h5p_default_f, h5p_flags
+  equivalence(h5p_flags, h5p_default_f)
+end module h5global
+! { dg-final { cleanup-modules "h5global" } }
Index: gcc/testsuite/gfortran.dg/pr77420_4.f90
===
--- gcc/testsuite/gfortran.dg/pr77420_4.f90	(nonexistent)
+++ gcc/testsuite/gfortran.dg/pr77420_4.f90	(working copy)
@@ -0,0 +1,10 @@
+! { dg-do compile { target { ! *-*-* } } }
+!
+program bug
+  use H5GLOBAL
+  implicit none
+  integer :: i
+  i=H5P_DEFAULT_F
+end program bug
+
+

Patch ping

[Jakub Jelinek]

Hi!

I'd like to ping a couple of patches:

C++
===

http://gcc.gnu.org/ml/gcc-patches/2016-08/msg01995.html
  - PR77375 - wrong-code with mutable members in base classes

http://gcc.gnu.org/ml/gcc-patches/2016-08/msg01998.html
  - PR77338 - fix constexpr ICE on PARM_DECL with incomplete type

http://gcc.gnu.org/ml/gcc-patches/2016-08/msg02002.html
  - PR77379 - fix testcase mangling regexps for 32-bit targets

http://gcc.gnu.org/ml/gcc-patches/2016-09/msg00228.html
  - PR77467 - fix constexpr switch handling

http://gcc.gnu.org/ml/gcc-patches/2016-09/msg00230.html
  - PR77482 - fix ICE in cp/error.c

x86
===

http://gcc.gnu.org/ml/gcc-patches/2016-09/msg00339.html
  - PR77475 - i386 backend part of the spellcheck option changes

sched
=

http://gcc.gnu.org/ml/gcc-patches/2016-09/msg00088.html
  - PR77425 - fix UB in sd_iterator_cond

Jakub

Re: Verify package integrity of downloaded prerequisites (partially fixes 61439)

[Mike Stump]

On Sep 14, 2016, at 1:19 PM, Moritz Klammler  wrote:
> 
> Joseph Myers  writes:
> 
>> On Wed, 14 Sep 2016, Moritz Klammler wrote:
>> 
>>> Ok, I didn't know about the workflow.  Do you think I should dike the
>>> `--strip-sums` option out again then?
>> 
>> I don't see any use for such an option.  Anyone changing the versions 
>> should always have a copy of the new tarball (obtained securely if 
>> possible) and should determine the sums from that.
> 
> Alright, below then without the option again.
> 
> Btw, how frequently am I supposed to post revisions of my patch to this
> list?

Feel free to address all reasonable concerns raised, and then post.

> Is it considered okay to do it immediately

Yes, as long as you address all the concerns you plan to address.  Before that 
point and it is usually premature.

> And should I also attach a "diff of the diffs" or will those interested be 
> happy to
> produce it themselves?

No diffs of diffs please.  You should just regenerate the entire patch and 
include it in a email as you discuss each point raised and the outcome of those 
points.

Re: [RFC][IPA-VRP] Early VRP Implementation

[Jan Hubicka]

> +  /* Visit PHI stmts and discover any new VRs possible.  */
> +  gimple_stmt_iterator gsi;
> +  for (gphi_iterator gpi = gsi_start_phis (bb);
> +   !gsi_end_p (gpi); gsi_next ())
> +{
> +  gphi *phi = gpi.phi ();
> +  tree lhs = PHI_RESULT (phi);
> +  value_range vr_result = VR_INITIALIZER;
> +  if (! has_unvisived_preds
>&& stmt_interesting_for_vrp (phi)
> + && stmt_visit_phi_node_in_dom_p (phi))
> +   extract_range_from_phi_node (phi, _result, true);
> +  else
> +   set_value_range_to_varying (_result);
> +  update_value_range (lhs, _result);
> +}
> 
> due to a bug in IRA you need to make sure to un-set BB_VISITED after
> early-vrp is finished again.
How IRA bugs affects early passes?

Honza

libgo patch committed: Fix typo in libgo/configure.ac

[Ian Lance Taylor]

This patch fixes a typo in libgo/configure.ac (PCQUANTUm ->
PCQUANTUM).  Bootstrapped on x86_64-pc-linux-gnu.  Committed to
mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 240083)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-841bea960b1f097e2cff5ad2618800296dcd4ec2
+b34c93bf00ec4f2ad043ec89ff96989e0d1b26aa
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/configure.ac
===
--- libgo/configure.ac  (revision 240083)
+++ libgo/configure.ac  (working copy)
@@ -222,7 +222,7 @@ case ${host} in
 GOARCH_FAMILY=ARM64
 GOARCH_CACHELINESIZE=32
 GOARCH_PHYSPAGESIZE=65536
-GOARCH_PCQUANTUm=4
+GOARCH_PCQUANTUM=4
 GOARCH_MINFRAMESIZE=8
 ;;
   arm*-*-* | strongarm*-*-* | ep9312*-*-* | xscale-*-*)

Re: Verify package integrity of downloaded prerequisites (partially fixes 61439)

[Moritz Klammler]

Joseph Myers  writes:

> On Wed, 14 Sep 2016, Moritz Klammler wrote:
>
>> Ok, I didn't know about the workflow.  Do you think I should dike the
>> `--strip-sums` option out again then?
>
> I don't see any use for such an option.  Anyone changing the versions 
> should always have a copy of the new tarball (obtained securely if 
> possible) and should determine the sums from that.

Alright, below then without the option again.

Btw, how frequently am I supposed to post revisions of my patch to this
list?  Is it considered okay to do it immediately or should I wait (how
long?) and gather more conclusive feedback first?  And should I also
attach a "diff of the diffs" or will those interested be happy to
produce it themselves?



* contrib/download_prerequisites: Verify integrity of downloaded
packages and added more command-line options.

* contrib/prerequisites.sha512: New.

* contrib/prerequisites.md5: New.
Index: contrib/download_prerequisites
===
--- contrib/download_prerequisites	(revision 240137)
+++ contrib/download_prerequisites	(working copy)
@@ -1,60 +1,215 @@
-#! /bin/sh
+#! /bin/sh -eu
+#! -*- coding:utf-8; mode:shell-script; -*-
 
-# Download some prerequisites needed by gcc.
-# Run this from the top level of the gcc source tree and the gcc
-# build will do the right thing.
+# Download some prerequisites needed by GCC.
+# Run this from the top level of the GCC source tree and the GCC build will do
+# the right thing.  Run it with the `--help` option for more information.
 #
-# (C) 2010-2016 Free Software Foundation
+# (C) 2016 Free Software Foundation
 #
-# This program is free software: you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or
-# (at your option) any later version.
-# 
-# This program is distributed in the hope that it will be useful, but
-# WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
-# General Public License for more details.
-# 
-# You should have received a copy of the GNU General Public License
-# along with this program. If not, see http://www.gnu.org/licenses/.
+# This program is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free Software
+# Foundation, either version 3 of the License, or (at your option) any later
+# version.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+# details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program.  If not, see http://www.gnu.org/licenses/.
 
-# If you want to disable Graphite loop optimizations while building GCC,
-# DO NOT set GRAPHITE_LOOP_OPT as yes so that the isl package will not
-# be downloaded.
-GRAPHITE_LOOP_OPT=yes
 
-if [ ! -e gcc/BASE-VER ] ; then
-	echo "You must run this script in the top level GCC source directory."
-	exit 1
-fi
+program='download_prerequisites'
+version='(unversioned)'
 
-# Necessary to build GCC.
-MPFR=mpfr-3.1.4
-GMP=gmp-6.1.0
-MPC=mpc-1.0.3
+# MAINTAINERS: If you update the package versions below, please
+# remember to also update the files `contrib/prerequisites.sha512` and
+# `contrib/prerequisites.md5` with the new checksums.
 
-wget ftp://gcc.gnu.org/pub/gcc/infrastructure/$MPFR.tar.bz2 || exit 1
-tar xjf $MPFR.tar.bz2 || exit 1
-if test -L mpfr; then rm -f mpfr; fi
-ln -sf $MPFR mpfr || exit 1
+gmp='gmp-6.1.0.tar.bz2'
+mpfr='mpfr-3.1.4.tar.bz2'
+mpc='mpc-1.0.3.tar.gz'
+isl='isl-0.16.1.tar.bz2'
 
-wget ftp://gcc.gnu.org/pub/gcc/infrastructure/$GMP.tar.bz2 || exit 1
-tar xjf $GMP.tar.bz2  || exit 1
-if test -L gmp; then rm -f gmp; fi
-ln -sf $GMP gmp || exit 1
+base_url='ftp://gcc.gnu.org/pub/gcc/infrastructure/'
 
-wget ftp://gcc.gnu.org/pub/gcc/infrastructure/$MPC.tar.gz || exit 1
-tar xzf $MPC.tar.gz || exit 1
-if test -L mpc; then rm -f mpc; fi
-ln -sf $MPC mpc || exit 1
+echo_archives() {
+echo "${gmp}"
+echo "${mpfr}"
+echo "${mpc}"
+if [ ${graphite} -gt 0 ]; then echo "${isl}"; fi
+}
 
-# Necessary to build GCC with the Graphite loop optimizations.
-if [ "$GRAPHITE_LOOP_OPT" = "yes" ] ; then
-  ISL=isl-0.16.1
+graphite=1
+verify=1
+force=0
+chksum='sha512'
+directory='.'
 
-  wget ftp://gcc.gnu.org/pub/gcc/infrastructure/$ISL.tar.bz2 || exit 1
-  tar xjf $ISL.tar.bz2  || exit 1
-  if test -L isl; then rm -f isl; fi
-  ln -sf $ISL isl || exit 1
+helptext="usage: ${program} [OPTION...]
+
+Downloads some prerequisites needed by GCC.  Run this from the top level of the
+GCC source tree and the GCC build will do the right thing.
+
+The 

Re: debug container mutex association

[François Dumont]

On 14/09/2016 11:00, Jonathan Wakely wrote:

On 13/09/16 22:37 +0200, François Dumont wrote:

Hi

   When I proposed to change std::hash for pointers my plan was to 
use this approach for the debug containers. So here is the patch 
leveraging on this technique to avoid going through _Hash_impl to 
hash address and get mutex index from it. I think it is obious that 
new access is faster so I didn't wrote a performance test to 
demonstrate it.


Is this actually a bottleneck that needs to be made faster?

I know it's nice if Debug Mode isn't very slow, but you've already
made it much better, and performance is not the primary goal of Debug
Mode.


Sure but my approach is that if I can make something faster then I just 
try. I considered that making this mode faster will allow its usage in 
an even more extended way.




diff --git a/libstdc++-v3/config/abi/pre/gnu.ver 
b/libstdc++-v3/config/abi/pre/gnu.ver

index 9b5bb23..c9a253e 100644
--- a/libstdc++-v3/config/abi/pre/gnu.ver
+++ b/libstdc++-v3/config/abi/pre/gnu.ver
@@ -1947,6 +1947,21 @@ GLIBCXX_3.4.23 {
_ZNSsC[12]ERKSs[jmy]RKSaIcE;
_ZNSbIwSt11char_traitsIwESaIwEEC[12]ERKS2_mRKS1_;

+# __gnu_debug::_Safe_sequence_base
+ _ZN11__gnu_debug19_Safe_sequence_base18_M_detach_singularEm;


The 'm' here should be [jmy] because std::size_t mangles differently
on different targets.


+ _ZN11__gnu_debug19_Safe_sequence_base22_M_revalidate_singularEm;
+_ZN11__gnu_debug19_Safe_sequence_base12_M_get_mutexEm;
+_ZN11__gnu_debug19_Safe_sequence_base7_M_swapERS0_m;
+
+# __gnu_debug::_Safe_iterator_base
+ 
_ZN11__gnu_debug19_Safe_iterator_base9_M_attachEPNS_19_Safe_sequence_baseEbm;

+_ZN11__gnu_debug19_Safe_iterator_base9_M_detachEm;
+
+# __gnu_debug::_Safe_unordered_container_base and 
_Safe_local_iterator_base

+ _ZN11__gnu_debug30_Safe_unordered_container_base7_M_swapERS0_m;
+ 
_ZN11__gnu_debug25_Safe_local_iterator_base9_M_attachEPNS_19_Safe_sequence_baseEbm;

+_ZN11__gnu_debug25_Safe_local_iterator_base9_M_detachEm;
+
} GLIBCXX_3.4.22;


It's unfortunate to have to export and maintain new symbols when we
don't offer the same level of ABI guarantee for Debug Mode  :-(


Yes, I have been told that Debug ABI compatibility was not guaranteed 
but at the same time if we don't do so abi-check is failing. Couldn't we 
have a special section in gnu.ver for unversioned symbols like those ?





@@ -174,6 +191,18 @@ namespace __gnu_debug
const _Safe_iterator_base* __last)
  { return __first->_M_can_compare(*__last); }

+  // Compute power of 2 not greater than __n.
+  template
+struct _Lowest_power_of_two
+{
+  static const size_t __val
+= _Lowest_power_of_two< (__n >> 1) >::__val + 1;
+};
+
+  template<>
+struct _Lowest_power_of_two<1>
+{ static const size_t __val = 1; };


The lowest power of two not greater than 1 is 2^0, but this gives the
result 1.

Similarly, the lowest power of two not greater than 2 is 2^1, i.e. 1.


Indeed, when writing the test case I decided to return 1 rather than 0 
for the _Lowest_power_of_two<1> specialization.It gave a better 
container instance - mutex mapping. But I forgot to change its name.




And the name is misleading, it doesn't compute a power of two, it
computes the base-2 logarithm (then adds one), so it's a compile-time
version of floor(log2(n))+1.


@@ -123,6 +125,36 @@ namespace __gnu_debug
  template
void
_M_transfer_from_if(_Safe_sequence& __from, _Predicate __pred);
+
+  static _GLIBCXX_CONSTEXPR std::size_t
+  _S_alignment() _GLIBCXX_NOEXCEPT
+  { return _Lowest_power_of_two<__alignof(_Sequence)>::__val; }


This function is called "alignment" but it returns log2(alignof(T))+1


Yes, I wasn't proud about the naming.



Does it need to be constexpr? Is that for std::array?


No, it is not used in std::array context. I just considered that it 
could be constexpr.




You can get the same result without _Lowest_power_of_two:

 static _GLIBCXX_CONSTEXPR size_t
 _S_alignbits() _GLIBCXX_NOEXCEPT
 { return __builtin_ctz(__alignof(_Sequence)) + 1; }

But I'm not convinced the +1 is correct, see below.


@@ -46,15 +47,30 @@ namespace
  /** Returns different instances of __mutex depending on the passed 
address
   *  in order to limit contention without breaking current library 
binary

   *  compatibility. */
+  const size_t mask = 0xf;
+
  __gnu_cxx::__mutex&
-  get_safe_base_mutex(void* __address)
+  get_mutex(size_t index)
  {
-const size_t mask = 0xf;
static __gnu_cxx::__mutex safe_base_mutex[mask + 1];
-const size_t index = _Hash_impl::hash(__address) & mask;
return safe_base_mutex[index];
  }


N.B. https://gcc.gnu.org/PR71312
Ok, debug mode is also impacted. Shouldn't the alignment be set on the 
__mutex type directly ?





+  __gnu_cxx::__mutex&
+  get_safe_base_mutex(void* address)
+  {
+const size_t index = _Hash_impl::hash(address) & mask;
+return get_mutex(index);
+  }
+
+  __gnu_cxx::__mutex&
+  

[PATCH,committed] Infer architecture from ABI for mips-img* and mips-mti*

[Matthew Fortune]

This patch allows the -mabi=n32 and -mabi=64 options to
automatically infer the of a 64-bit architecture in the mips-mti-*
and mips-img-* triplets. The default 64-bit architecture is
mips64r2 for MTI and mips64r6 for IMG.

Thanks,
Matthew

gcc/
* config.gcc (mips*-mti-elf*, mips*-mti-linux*): Set mips32r2
and mips64r2 as default 32-bit and 64-bit architectures.
(mips*-img-elf*, mips*-img-linux*): Set mips32r6 and mips64r6
as default 32-bit and 64-bit architectures.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@240145 
138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/ChangeLog  | 7 +++
 gcc/config.gcc | 8 
 2 files changed, 15 insertions(+)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 8527518..9b39906 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,10 @@
+2016-09-14  Matthew Fortune  
+
+   * config.gcc (mips*-mti-elf*, mips*-mti-linux*): Set mips32r2
+   and mips64r2 as default 32-bit and 64-bit architectures.
+   (mips*-img-elf*, mips*-img-linux*): Set mips32r6 and mips64r6
+   as default 32-bit and 64-bit architectures.
+
 2016-09-14  Pat Haugen  
 
* loop-unroll.c (unroll_loop_runtime_iterations): Set probability of 
succ edge.
diff --git a/gcc/config.gcc b/gcc/config.gcc
index e544d76..fc91ba7 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -1985,6 +1985,8 @@ mips*-img-linux*)
extra_options="${extra_options} linux-android.opt"
tmake_file="${tmake_file} mips/t-img-linux"
tm_defines="${tm_defines} MIPS_ISA_DEFAULT=37 MIPS_ABI_DEFAULT=ABI_32"
+   with_arch_32="mips32r6"
+   with_arch_64="mips64r6"
gnu_ld=yes
gas=yes
;;
@@ -1993,6 +1995,8 @@ mips*-mti-linux*)
extra_options="${extra_options} linux-android.opt"
tmake_file="${tmake_file} mips/t-mti-linux"
tm_defines="${tm_defines} MIPS_ISA_DEFAULT=33 MIPS_ABI_DEFAULT=ABI_32"
+   with_arch_32="mips32r2"
+   with_arch_64="mips64r2"
gnu_ld=yes
gas=yes
;;
@@ -2047,11 +2051,15 @@ mips*-mti-elf*)
tm_file="elfos.h newlib-stdint.h ${tm_file} mips/elf.h mips/n32-elf.h 
mips/sde.h mips/mti-elf.h"
tmake_file="mips/t-mti-elf"
tm_defines="${tm_defines} MIPS_ISA_DEFAULT=33 MIPS_ABI_DEFAULT=ABI_32"
+   with_arch_32="mips32r2"
+   with_arch_64="mips64r2"
;;
 mips*-img-elf*)
tm_file="elfos.h newlib-stdint.h ${tm_file} mips/elf.h mips/n32-elf.h 
mips/sde.h mips/mti-elf.h"
tmake_file="mips/t-img-elf"
tm_defines="${tm_defines} MIPS_ISA_DEFAULT=37 MIPS_ABI_DEFAULT=ABI_32"
+   with_arch_32="mips32r6"
+   with_arch_64="mips64r6"
;;
 mips*-sde-elf*)
tm_file="elfos.h newlib-stdint.h ${tm_file} mips/elf.h mips/n32-elf.h 
mips/sde.h"
-- 
2.2.1

Re: [PATCHv3] Add a warning for suspicious use of conditional expressions in boolean context

[Bernd Edlinger]

... resent, because message apparently bounced.

On 09/14/16 21:22, Bernd Edlinger wrote:
> On 09/14/16 20:11, Jason Merrill wrote:
>>>
>>> Yes.  The reasoning I initially had was that it is completely
>>> pointless to have something of the form "if (x ? 1 : 2)" or
>>> "if (x ? 0 : 0)" because the result does not even depend on x
>>> in this case.  But something like "if (x ? 4999 : 0)" looks
>>> bogus but does at least not ignore x.
>>>
>>> If the false-positives are becoming too much of a problem here,
>>> then I should of course revert to the previous heuristic again.
>>
>> I think we could have both, where the weaker form is part of -Wall and
>> people can explicitly select the stronger form.
>>
>
>
> Yes, agreed.  So here is what I would think will be the first version.
>
> It can later be extended to cover the more pedantic cases which
> will not be enabled by -Wall.
>
> I would like to send a follow-up patch for the warning on
> signed-integer shift left in boolean context, which I think
> should also be good for Wall.
> (I already had that feature in patch version 2 but that's meanwhile
> outdated).
>
>
> Bootstrap on x86_64-pc-linux-gnu and reg-testing not yet completed.
> Is it OK for trunk when reg-testing finished?
>
>
> Thanks
> Bernd.

Re: [PATCH 5/9] regrename: Don't run if function was separately shrink-wrapped

[Jeff Law]

On 09/14/2016 01:03 PM, Segher Boessenkool wrote:


(There is no return insn at those exits; these are exits *without*
successor block, not the exit block).
Hmm, I thought these were return blocks, but you're saying they're 
no-return blocks?  I missed that.


In that case, aren't the restores dead because no callers can observe 
their value changing unexpectedly?



Jeff

Re: [PATCH 5/9] regrename: Don't run if function was separately shrink-wrapped

[Jeff Law]

On 09/14/2016 01:03 PM, Segher Boessenkool wrote:

If you think about it, conceptually we want the return insn to make the
callee saved registers "used" so that DCE, regrename and friends don't
muck with them.  The fact that we don't is as much never having to care
all that much until recently.


(There is no return insn at those exits; these are exits *without*
successor block, not the exit block).
Ugh.  Anywhere we could attach this stuff in the insn chain?  If not, 
the DF side of this problem gets uglier.




It is puzzling to me why adding USEs for just the registers that *are*
separately shrink-wrapped does not work; only all those that *could* be
shrink-wrapped does.  Do you have any idea about that?
Nope.  No clue on that one.  It almost seems like a book-keeping error 
somewhere.





I continue to wonder if we could add something similar to
CALL_INSN_FUNCTION_USAGE where we attach uses for all the call-saved
registers onto a return insn.  We would attach them at the end of
prologue/epilogue generation and only attach those where were live
somewhere in the code.


Maybe adding the new insns to the {pro,epi}logue_insn_hash will help
something.  Or maybe it will blow up spectacularly.  Will know in a
bit.

I think that'll resolve the sel-sched issue, but I doubt the rest.




I pondered just doing it for the separately wrapped components on that
particular path, but I've yet to convince myself that's actually correct.


If that is not correct, how is the status quo correct?  That is what
puzzles me above, too.
I couldn't convince myself that we could trivially find all the 
separately wrapped components on a particular path -- ie, we may have 
had components saved/restored in some sub-graph.  If an exit point from 
the function is reachable from that sub-graph, then we need to make sure 
any components from the subgraph are marked as live in addition to those 
which are restored on the exit path.


While it is just a reachability problem, I don't think we need to solve 
it if we mark anything that was separately shrink wrapped as live at all 
the exit points.





There is a well-defined way to turn it off, via common/config/*/*-common.c ,
TARGET_OPTION_OPTIMIZATION_TABLE.  We disagree on whether most targets will
want it enabled, i.e. whether it should (eventually) be enabled by default.
I'm aware of TARGET_OPTION_OPTIMIZATION_TABLE -- I was thinking more 
along the lines of trying to describe the conditions under which it is 
not profitable and expose that.  TARGET_OPTION_OPTIMIZATION_TABLE is a 
lame way to attack this problem.


Jeff

Re: [PATCH 5/9] regrename: Don't run if function was separately shrink-wrapped

[Jeff Law]

On 09/14/2016 01:10 PM, Segher Boessenkool wrote:

On Wed, Sep 14, 2016 at 11:52:02AM -0600, Jeff Law wrote:

Yea, it'll certainly do that and I can imagine a design which would have
that property.  And there's other designs which benefit from spreading
across the register file as much as possible.

Which argues there needs to be a way to tune or disable the pass.


Yes, some targets want it, and some don't.  It seems to me that targets
that use sched1 have much less benefit from regrename.  Of course enabling
sched1 has its own problems.
I think that sched1 would decrease the benefit of reg renaming, but 
wouldn't totally eliminate the desire to avoid anti dependencies created 
by register allocation.


jeff

[PATCH 5/8] make prev_real_insn take rtx_insn *

[tbsaunde+gcc]

From: Trevor Saunders 

gcc/ChangeLog:

2016-09-13  Trevor Saunders  

* emit-rtl.c (prev_real_insn): Change argument type to rtx_insn *.
* rtl.h: Adjust prototype.
* config/sh/sh.md: Adjust.
* dwarf2out.c (add_var_loc_to_decl): Likewise.
---
 gcc/config/sh/sh.md | 3 ++-
 gcc/dwarf2out.c | 2 +-
 gcc/emit-rtl.c  | 4 +---
 gcc/rtl.h   | 2 +-
 4 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/gcc/config/sh/sh.md b/gcc/config/sh/sh.md
index edc4d15..25e03ef 100644
--- a/gcc/config/sh/sh.md
+++ b/gcc/config/sh/sh.md
@@ -7178,7 +7178,8 @@
  (label_ref (match_operand 1 "" ""
(use (label_ref (match_operand 2 "" "")))]
   "TARGET_SH2
-   && (! INSN_UID (operands[1]) || prev_real_insn (operands[1]) == insn)"
+   && (! INSN_UID (operands[1])
+   || prev_real_insn (as_a (operands[1])) == insn)"
   "braf%0%#"
   [(set_attr "needs_delay_slot" "yes")
(set_attr "type" "jump_ind")])
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 45eb684..fb8ec7d 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -5408,7 +5408,7 @@ add_var_loc_to_decl (tree decl, rtx loc_note, const char 
*label)
   && NOTE_VAR_LOCATION_LOC (temp->first->loc)
   && GET_CODE (NOTE_VAR_LOCATION_LOC (temp->first->loc))
 == GET_CODE (DECL_INCOMING_RTL (decl))
-  && prev_real_insn (temp->first->loc) == NULL_RTX
+  && prev_real_insn (as_a (temp->first->loc)) == NULL_RTX
   && (bitsize != -1
  || !rtx_equal_p (NOTE_VAR_LOCATION_LOC (temp->first->loc),
   NOTE_VAR_LOCATION_LOC (loc_note))
diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 6051326..df5d359 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -3461,10 +3461,8 @@ next_real_insn (rtx uncast_insn)
SEQUENCEs.  */
 
 rtx_insn *
-prev_real_insn (rtx uncast_insn)
+prev_real_insn (rtx_insn *insn)
 {
-  rtx_insn *insn = safe_as_a  (uncast_insn);
-
   while (insn)
 {
   insn = PREV_INSN (insn);
diff --git a/gcc/rtl.h b/gcc/rtl.h
index b557ffe..c253fda 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -2842,7 +2842,7 @@ extern rtx_insn *prev_nondebug_insn (rtx_insn *);
 extern rtx_insn *next_nondebug_insn (rtx_insn *);
 extern rtx_insn *prev_nonnote_nondebug_insn (rtx);
 extern rtx_insn *next_nonnote_nondebug_insn (rtx);
-extern rtx_insn *prev_real_insn (rtx);
+extern rtx_insn *prev_real_insn (rtx_insn *);
 extern rtx_insn *next_real_insn (rtx);
 extern rtx_insn *prev_active_insn (rtx);
 extern rtx_insn *next_active_insn (rtx);
-- 
2.9.3

[PATCH 7/8] make next/prev active_insn and active_insn_p take rtx_insn *

[tbsaunde+gcc]

From: Trevor Saunders 

gcc/ChangeLog:

2016-09-14  Trevor Saunders  

* emit-rtl.c (next_active_insn): Change argument type to
rtx_insn *.
(prev_active_insn): Likewise.
(active_insn_p): Likewise.
* rtl.h: Adjust prototypes.
* cfgcleanup.c (merge_blocks_move_successor_nojumps): Adjust.
* config/arc/arc.md: Likewise.
* config/pa/pa.c (branch_to_delay_slot_p): Likewise.
(branch_needs_nop_p): Likewise.
(use_skip_p): Likewise.
* config/sh/sh.c (gen_block_redirect): Likewise.
(split_branches): Likewise.
* reorg.c (optimize_skip): Likewise.
(fill_simple_delay_slots): Likewise.
(fill_slots_from_thread): Likewise.
(relax_delay_slots): Likewise.
* resource.c (mark_target_live_regs): Likewise.
---
 gcc/cfgcleanup.c  |  2 +-
 gcc/config/arc/arc.md | 33 +++--
 gcc/config/pa/pa.c|  6 +++---
 gcc/config/sh/sh.c|  8 
 gcc/emit-rtl.c| 10 +++---
 gcc/reorg.c   | 29 +
 gcc/resource.c|  2 +-
 gcc/rtl.h |  6 +++---
 8 files changed, 55 insertions(+), 41 deletions(-)

diff --git a/gcc/cfgcleanup.c b/gcc/cfgcleanup.c
index 023b9d2..2e2a635 100644
--- a/gcc/cfgcleanup.c
+++ b/gcc/cfgcleanup.c
@@ -708,7 +708,7 @@ merge_blocks_move_successor_nojumps (basic_block a, 
basic_block b)
   /* If there is a jump table following block B temporarily add the jump table
  to block B so that it will also be moved to the correct location.  */
   if (tablejump_p (BB_END (b), , )
-  && prev_active_insn (label) == BB_END (b))
+  && prev_active_insn (as_a (label)) == BB_END (b))
 {
   BB_END (b) = table;
 }
diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md
index 22fdbba..ac7346b 100644
--- a/gcc/config/arc/arc.md
+++ b/gcc/config/arc/arc.md
@@ -5122,16 +5122,29 @@
  scan = as_a  (XEXP (SET_SRC (PATTERN (scan)), 0));
  continue;
}
- if (JUMP_LABEL (scan)
- /* JUMP_LABEL might be simple_return instead if an insn.  */
- && (!INSN_P (JUMP_LABEL (scan))
- || (!next_active_insn (JUMP_LABEL (scan))
- || (recog_memoized (next_active_insn (JUMP_LABEL 
(scan)))
- != CODE_FOR_doloop_begin_i)))
- && (!next_active_insn (NEXT_INSN (PREV_INSN (scan)))
- || (recog_memoized
-  (next_active_insn (NEXT_INSN (PREV_INSN (scan
- != CODE_FOR_doloop_begin_i)))
+
+ rtx lab = JUMP_LABEL (scan);
+ if (!lab)
+   break;
+
+ rtx_insn *next_scan
+   = next_active_insn (NEXT_INSN (PREV_INSN (scan)));
+ if (next_scan
+ && recog_memoized (next_scan) != CODE_FOR_doloop_begin_i)
+   break;
+
+ /* JUMP_LABEL might be simple_return instead if an insn.  */
+ if (!INSN_P (lab))
+   {
+ n_insns++;
+ break;
+   }
+
+ rtx_insn *next_lab = next_active_insn (as_a (lab));
+ if (next_lab
+ && recog_memoized (next_lab) != CODE_FOR_doloop_begin_i)
+   break;
+
n_insns++;
}
  break;
diff --git a/gcc/config/pa/pa.c b/gcc/config/pa/pa.c
index 251c1ad..9c15504 100644
--- a/gcc/config/pa/pa.c
+++ b/gcc/config/pa/pa.c
@@ -6442,7 +6442,7 @@ branch_to_delay_slot_p (rtx_insn *insn)
   if (dbr_sequence_length ())
 return FALSE;
 
-  jump_insn = next_active_insn (JUMP_LABEL (insn));
+  jump_insn = next_active_insn (JUMP_LABEL_AS_INSN (insn));
   while (insn)
 {
   insn = next_active_insn (insn);
@@ -6476,7 +6476,7 @@ branch_needs_nop_p (rtx_insn *insn)
   if (dbr_sequence_length ())
 return FALSE;
 
-  jump_insn = next_active_insn (JUMP_LABEL (insn));
+  jump_insn = next_active_insn (JUMP_LABEL_AS_INSN (insn));
   while (insn)
 {
   insn = next_active_insn (insn);
@@ -6499,7 +6499,7 @@ branch_needs_nop_p (rtx_insn *insn)
 static bool
 use_skip_p (rtx_insn *insn)
 {
-  rtx_insn *jump_insn = next_active_insn (JUMP_LABEL (insn));
+  rtx_insn *jump_insn = next_active_insn (JUMP_LABEL_AS_INSN (insn));
 
   while (insn)
 {
diff --git a/gcc/config/sh/sh.c b/gcc/config/sh/sh.c
index b3e949e..a9b5a14 100644
--- a/gcc/config/sh/sh.c
+++ b/gcc/config/sh/sh.c
@@ -5503,7 +5503,8 @@ gen_block_redirect (rtx_insn *jump, int addr, int 
need_block)
 
   else if (optimize && need_block >= 0)
 {
-  rtx_insn *next = next_active_insn (next_active_insn (dest));
+  rtx_insn *next = next_active_insn (as_a (dest));
+  next = next_active_insn (next);
   if (next && JUMP_P (next)
  && GET_CODE (PATTERN (next)) == SET
  && 

[PATCH 8/8] make next_cc0_user take rtx_insn *

[tbsaunde+gcc]

From: Trevor Saunders 

gcc/ChangeLog:

2016-09-14  Trevor Saunders  

* emit-rtl.c (next_cc0_user): Make argument type rtx_insn *.
* rtl.h: Adjust prototype.
---
 gcc/emit-rtl.c | 4 +---
 gcc/rtl.h  | 2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index e794613..1519aa5 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -3540,10 +3540,8 @@ prev_active_insn (rtx_insn *insn)
Return 0 if we can't find the insn.  */
 
 rtx_insn *
-next_cc0_user (rtx uncast_insn)
+next_cc0_user (rtx_insn *insn)
 {
-  rtx_insn *insn = safe_as_a  (uncast_insn);
-
   rtx note = find_reg_note (insn, REG_CC_USER, NULL_RTX);
 
   if (note)
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 22ee2e6..ce1131b 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -2847,7 +2847,7 @@ extern rtx_insn *next_real_insn (rtx);
 extern rtx_insn *prev_active_insn (rtx_insn *);
 extern rtx_insn *next_active_insn (rtx_insn *);
 extern int active_insn_p (const rtx_insn *);
-extern rtx_insn *next_cc0_user (rtx);
+extern rtx_insn *next_cc0_user (rtx_insn *);
 extern rtx_insn *prev_cc0_setter (rtx_insn *);
 
 /* In emit-rtl.c  */
-- 
2.9.3

[PATCH 6/8] make next/prev nonnote_nondebug_insn take rtx_insn *

[tbsaunde+gcc]

From: Trevor Saunders 

gcc/ChangeLog:

2016-09-14  Trevor Saunders  

* config/cris/cris.c (cris_asm_output_case_end): Change argument
type to rtx_insn *.
* emit-rtl.c (next_nonnote_nondebug_insn): Likewise.
(prev_nonnote_nondebug_insn): Likewise.
* config/cris/cris-protos.h: Adjust prototype.
* rtl.h: Likewise.
* jump.c (rtx_renumbered_equal_p): Adjust.
---
 gcc/config/cris/cris-protos.h |  2 +-
 gcc/config/cris/cris.c| 12 ++--
 gcc/emit-rtl.c|  8 ++--
 gcc/jump.c|  6 --
 gcc/rtl.h |  4 ++--
 5 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/gcc/config/cris/cris-protos.h b/gcc/config/cris/cris-protos.h
index 5e0ae61..028c2b7 100644
--- a/gcc/config/cris/cris-protos.h
+++ b/gcc/config/cris/cris-protos.h
@@ -44,7 +44,7 @@ extern bool cris_store_multiple_op_p (rtx);
 extern bool cris_movem_load_rest_p (rtx, int);
 extern void cris_asm_output_symbol_ref (FILE *, rtx);
 extern int cris_cfun_uses_pic_table (void);
-extern void cris_asm_output_case_end (FILE *, int, rtx);
+extern void cris_asm_output_case_end (FILE *, int, rtx_insn *);
 extern rtx cris_gen_movem_load (rtx, rtx, int);
 extern rtx cris_emit_movem_store (rtx, rtx, int, bool);
 extern void cris_expand_pic_call_address (rtx *, rtx *);
diff --git a/gcc/config/cris/cris.c b/gcc/config/cris/cris.c
index 39118f0..bd49152 100644
--- a/gcc/config/cris/cris.c
+++ b/gcc/config/cris/cris.c
@@ -2586,11 +2586,11 @@ cris_asm_output_ident (const char *string)
 /* The ASM_OUTPUT_CASE_END worker.  */
 
 void
-cris_asm_output_case_end (FILE *stream, int num, rtx table)
+cris_asm_output_case_end (FILE *stream, int num, rtx_insn *table)
 {
   /* Step back, over the label for the table, to the actual casejump and
  assert that we find only what's expected.  */
-  rtx whole_jump_insn = prev_nonnote_nondebug_insn (table);
+  rtx_insn *whole_jump_insn = prev_nonnote_nondebug_insn (table);
   gcc_assert (whole_jump_insn != NULL_RTX && LABEL_P (whole_jump_insn));
   whole_jump_insn = prev_nonnote_nondebug_insn (whole_jump_insn);
   gcc_assert (whole_jump_insn != NULL_RTX
@@ -2598,15 +2598,15 @@ cris_asm_output_case_end (FILE *stream, int num, rtx 
table)
  || (TARGET_V32 && INSN_P (whole_jump_insn)
  && GET_CODE (PATTERN (whole_jump_insn)) == SEQUENCE)));
   /* Get the pattern of the casejump, so we can extract the default label.  */
-  whole_jump_insn = PATTERN (whole_jump_insn);
+  rtx whole_jump_pat = PATTERN (whole_jump_insn);
 
   if (TARGET_V32)
 {
   /* This can be a SEQUENCE, meaning the delay-slot of the jump is
 filled.  We also output the offset word a little differently.  */
   rtx parallel_jump
-   = (GET_CODE (whole_jump_insn) == SEQUENCE
-  ? PATTERN (XVECEXP (whole_jump_insn, 0, 0)) : whole_jump_insn);
+   = (GET_CODE (whole_jump_pat) == SEQUENCE
+  ? PATTERN (XVECEXP (whole_jump_pat, 0, 0)) : whole_jump_pat);
 
   asm_fprintf (stream,
   "\t.word %LL%d-.%s\n",
@@ -2621,7 +2621,7 @@ cris_asm_output_case_end (FILE *stream, int num, rtx 
table)
   "\t.word %LL%d-%LL%d%s\n",
   CODE_LABEL_NUMBER (XEXP
  (XEXP
-  (XEXP (XVECEXP (whole_jump_insn, 0, 0), 1), 
+  (XEXP (XVECEXP (whole_jump_pat, 0, 0), 1),
2), 0)),
   num,
   (TARGET_PDEBUG ? "; default" : ""));
diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index df5d359..20d8250 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -3405,10 +3405,8 @@ prev_nondebug_insn (rtx_insn *insn)
This routine does not look inside SEQUENCEs.  */
 
 rtx_insn *
-next_nonnote_nondebug_insn (rtx uncast_insn)
+next_nonnote_nondebug_insn (rtx_insn *insn)
 {
-  rtx_insn *insn = safe_as_a  (uncast_insn);
-
   while (insn)
 {
   insn = NEXT_INSN (insn);
@@ -3423,10 +3421,8 @@ next_nonnote_nondebug_insn (rtx uncast_insn)
This routine does not look inside SEQUENCEs.  */
 
 rtx_insn *
-prev_nonnote_nondebug_insn (rtx uncast_insn)
+prev_nonnote_nondebug_insn (rtx_insn *insn)
 {
-  rtx_insn *insn = safe_as_a  (uncast_insn);
-
   while (insn)
 {
   insn = PREV_INSN (insn);
diff --git a/gcc/jump.c b/gcc/jump.c
index 87a1a5d..2164c3b 100644
--- a/gcc/jump.c
+++ b/gcc/jump.c
@@ -1806,8 +1806,10 @@ rtx_renumbered_equal_p (const_rtx x, const_rtx y)
 in the same position in the instruction stream.  */
   else
{
- rtx_insn *xi = next_nonnote_nondebug_insn (LABEL_REF_LABEL (x));
- rtx_insn *yi = next_nonnote_nondebug_insn (LABEL_REF_LABEL (y));
+ rtx_insn *xi = next_nonnote_nondebug_insn
+   (as_a (LABEL_REF_LABEL (x)));
+ rtx_insn *yi = next_nonnote_nondebug_insn
+   (as_a 

[PATCH 3/8] make next/prev _nonnote_insn take rtx_insn *

[tbsaunde+gcc]

From: Trevor Saunders 

gcc/ChangeLog:

2016-09-13  Trevor Saunders  

* emit-rtl.c (next_nonnote_insn): Change argument type to
rtx_insn *.
(prev_nonnote_insn): Likewise.
* jump.c (reversed_comparison_code_parts): Likewise.
(reversed_comparison): Likewise.
* rtl.h: Adjust prototypes.
* config/arc/arc.md: Adjust.
* cse.c (find_comparison_args): Likewise.
* reorg.c (redundant_insn): Change return type to rtx_insn *.
(fix_reg_dead_note): Change argument type to rtx_insn *.
(delete_prior_computation): Likewise.
(delete_computation): Likewise.
(fill_slots_from_thread): Adjust.
(relax_delay_slots): Likewise.
* simplify-rtx.c (simplify_unary_operation_1): Likewise.
(simplify_relational_operation_1): Likewise.
(simplify_ternary_operation): Likewise.
---
 gcc/config/arc/arc.md | 10 ++
 gcc/cse.c |  2 +-
 gcc/emit-rtl.c|  7 ++-
 gcc/jump.c| 12 ++--
 gcc/reorg.c   | 22 +++---
 gcc/rtl.h |  8 
 gcc/simplify-rtx.c|  8 
 7 files changed, 34 insertions(+), 35 deletions(-)

diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md
index 1102c53..22fdbba 100644
--- a/gcc/config/arc/arc.md
+++ b/gcc/config/arc/arc.md
@@ -3879,7 +3879,7 @@
   ""
   "*
 {
-  rtx diff_vec = PATTERN (next_nonnote_insn (operands[3]));
+  rtx diff_vec = PATTERN (next_nonnote_insn (as_a (operands[3])));
 
   if (GET_CODE (diff_vec) != ADDR_DIFF_VEC)
 {
@@ -3907,10 +3907,12 @@
   [(set_attr "type" "load")
(set_attr_alternative "iscompact"
  [(cond
-   [(ne (symbol_ref "GET_MODE (PATTERN (next_nonnote_insn (operands[3])))")
+   [(ne (symbol_ref "GET_MODE (PATTERN (next_nonnote_insn
+  (as_a 
(operands[3]")
 (symbol_ref "QImode"))
 (const_string "false")
-(match_test "!ADDR_DIFF_VEC_FLAGS (PATTERN (next_nonnote_insn 
(operands[3]))).offset_unsigned")
+(match_test "!ADDR_DIFF_VEC_FLAGS (PATTERN (next_nonnote_insn
+  (as_a 
(operands[3].offset_unsigned")
 (const_string "false")]
(const_string "true"))
   (const_string "false")
@@ -3946,7 +3948,7 @@
   "TARGET_COMPACT_CASESI"
   "*
 {
-  rtx diff_vec = PATTERN (next_nonnote_insn (operands[1]));
+  rtx diff_vec = PATTERN (next_nonnote_insn (as_a (operands[1])));
   int unalign = arc_get_unalign ();
   rtx xop[3];
   const char *s;
diff --git a/gcc/cse.c b/gcc/cse.c
index 0bfd7ff..8637d57 100644
--- a/gcc/cse.c
+++ b/gcc/cse.c
@@ -3054,7 +3054,7 @@ find_comparison_args (enum rtx_code code, rtx *parg1, rtx 
*parg2,
 with floating-point operands.  */
   if (reverse_code)
{
- enum rtx_code reversed = reversed_comparison_code (x, NULL_RTX);
+ enum rtx_code reversed = reversed_comparison_code (x, NULL);
  if (reversed == UNKNOWN)
break;
  else
diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index a724608..cbea214 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -3301,9 +3301,8 @@ previous_insn (rtx_insn *insn)
look inside SEQUENCEs.  */
 
 rtx_insn *
-next_nonnote_insn (rtx uncast_insn)
+next_nonnote_insn (rtx_insn *insn)
 {
-  rtx_insn *insn = safe_as_a  (uncast_insn);
   while (insn)
 {
   insn = NEXT_INSN (insn);
@@ -3337,10 +3336,8 @@ next_nonnote_insn_bb (rtx_insn *insn)
not look inside SEQUENCEs.  */
 
 rtx_insn *
-prev_nonnote_insn (rtx uncast_insn)
+prev_nonnote_insn (rtx_insn *insn)
 {
-  rtx_insn *insn = safe_as_a  (uncast_insn);
-
   while (insn)
 {
   insn = PREV_INSN (insn);
diff --git a/gcc/jump.c b/gcc/jump.c
index 22f8a71..87a1a5d 100644
--- a/gcc/jump.c
+++ b/gcc/jump.c
@@ -62,7 +62,7 @@ static void mark_all_labels (rtx_insn *);
 static void mark_jump_label_1 (rtx, rtx_insn *, bool, bool);
 static void mark_jump_label_asm (rtx, rtx_insn *);
 static void redirect_exp_1 (rtx *, rtx, rtx, rtx);
-static int invert_exp_1 (rtx, rtx);
+static int invert_exp_1 (rtx, rtx_insn *);
 
 /* Worker for rebuild_jump_labels and rebuild_jump_labels_chain.  */
 static void
@@ -360,7 +360,7 @@ mark_all_labels (rtx_insn *f)
to help this function avoid overhead in these cases.  */
 enum rtx_code
 reversed_comparison_code_parts (enum rtx_code code, const_rtx arg0,
-   const_rtx arg1, const_rtx insn)
+   const_rtx arg1, const rtx_insn *insn)
 {
   machine_mode mode;
 
@@ -422,7 +422,7 @@ reversed_comparison_code_parts (enum rtx_code code, 
const_rtx arg0,
   /* These CONST_CAST's are okay because prev_nonnote_insn just
 returns its argument and we assign it to a const_rtx
 variable.  */
-  for (rtx_insn *prev = prev_nonnote_insn (CONST_CAST_RTX (insn));
+  for (rtx_insn 

[PATCH 4/8] make next/prev nondebug_insn take rtx_insn *

[tbsaunde+gcc]

From: Trevor Saunders 

gcc/ChangeLog:

2016-09-13  Trevor Saunders  

* emit-rtl.c (next_nondebug_insn): Change argument type to
rtx_insn *.
(prev_nondebug_insn): Likewise.
* loop-doloop.c (doloop_condition_get): Likewise.
* rtl.h: Adjust prototype.
* cfgloop.h: Likewise.
---
 gcc/cfgloop.h | 2 +-
 gcc/emit-rtl.c| 8 ++--
 gcc/loop-doloop.c | 2 +-
 gcc/rtl.h | 4 ++--
 4 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/gcc/cfgloop.h b/gcc/cfgloop.h
index 5c202eb..0448a61 100644
--- a/gcc/cfgloop.h
+++ b/gcc/cfgloop.h
@@ -371,7 +371,7 @@ extern bool just_once_each_iteration_p (const struct loop 
*, const_basic_block);
 gcov_type expected_loop_iterations_unbounded (const struct loop *,
  bool *read_profile_p = NULL);
 extern unsigned expected_loop_iterations (struct loop *);
-extern rtx doloop_condition_get (rtx);
+extern rtx doloop_condition_get (rtx_insn *);
 
 void mark_loop_for_removal (loop_p);
 
diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index cbea214..6051326 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -3373,10 +3373,8 @@ prev_nonnote_insn_bb (rtx uncast_insn)
routine does not look inside SEQUENCEs.  */
 
 rtx_insn *
-next_nondebug_insn (rtx uncast_insn)
+next_nondebug_insn (rtx_insn *insn)
 {
-  rtx_insn *insn = safe_as_a  (uncast_insn);
-
   while (insn)
 {
   insn = NEXT_INSN (insn);
@@ -3391,10 +3389,8 @@ next_nondebug_insn (rtx uncast_insn)
This routine does not look inside SEQUENCEs.  */
 
 rtx_insn *
-prev_nondebug_insn (rtx uncast_insn)
+prev_nondebug_insn (rtx_insn *insn)
 {
-  rtx_insn *insn = safe_as_a  (uncast_insn);
-
   while (insn)
 {
   insn = PREV_INSN (insn);
diff --git a/gcc/loop-doloop.c b/gcc/loop-doloop.c
index c311516..17a968f 100644
--- a/gcc/loop-doloop.c
+++ b/gcc/loop-doloop.c
@@ -70,7 +70,7 @@ along with GCC; see the file COPYING3.  If not see
if it is not a decrement and branch jump insn.  */
 
 rtx
-doloop_condition_get (rtx doloop_pat)
+doloop_condition_get (rtx_insn *doloop_pat)
 {
   rtx cmp;
   rtx inc;
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 0d121bc..b557ffe 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -2838,8 +2838,8 @@ extern rtx_insn *prev_nonnote_insn (rtx_insn *);
 extern rtx_insn *prev_nonnote_insn_bb (rtx);
 extern rtx_insn *next_nonnote_insn (rtx_insn *);
 extern rtx_insn *next_nonnote_insn_bb (rtx_insn *);
-extern rtx_insn *prev_nondebug_insn (rtx);
-extern rtx_insn *next_nondebug_insn (rtx);
+extern rtx_insn *prev_nondebug_insn (rtx_insn *);
+extern rtx_insn *next_nondebug_insn (rtx_insn *);
 extern rtx_insn *prev_nonnote_nondebug_insn (rtx);
 extern rtx_insn *next_nonnote_nondebug_insn (rtx);
 extern rtx_insn *prev_real_insn (rtx);
-- 
2.9.3

[PATCH 2/8] use rtx_insn * more

[tbsaunde+gcc]

From: Trevor Saunders 

gcc/ChangeLog:

2016-09-06  Trevor Saunders  

* config/arc/arc-protos.h (arc_label_align): Change type of
variables from rtx to rtx_insn *.
* config/arc/arc.c (arc_label_align): Likewise.
* config/arm/arm.c (any_sibcall_could_use_r3): Likewise.
* config/bfin/bfin.c (workaround_speculation): Likewise.
* config/c6x/c6x.c (find_next_cycle_insn): Likewise.
(find_last_same_clock): Likewise.
(reorg_split_calls): Likewise.
* config/cris/cris-protos.h (cris_cc0_user_requires_cmp): Likewise.
* config/cris/cris.c (cris_cc0_user_requires_cmp): Likewise.
* config/h8300/h8300-protos.h (same_cmp_preceding_p): Likewise.
(same_cmp_following_p): Likewise.
* config/h8300/h8300.c (same_cmp_preceding_p): Likewise.
(same_cmp_following_p): Likwise.
* config/m32r/m32r.c (m32r_expand_epilogue): Likewise.
* config/nds32/nds32-protos.h (nds32_target_alignment): Likewise.
* config/nds32/nds32.c (nds32_target_alignment): Likewise.
* config/rl78/rl78.c (rl78_alloc_physical_registers_op2):
* Likewise.
(rl78_alloc_physical_registers_cmp): Likewise.
(rl78_alloc_physical_registers_umul): Likewise.
(rl78_calculate_death_notes): Likewise.
* config/s390/s390-protos.h (s390_label_align): Likewise.
* config/s390/s390.c (s390_label_align): Likewise.
* config/sh/sh.c (barrier_align): Likewise.
* config/sparc/sparc-protos.h (emit_cbcond_nop): Likewise.
* config/sparc/sparc.c (sparc_asm_function_epilogue): Likewise.
(emit_cbcond_nop): Likewise.
---
 gcc/config/arc/arc-protos.h |  2 +-
 gcc/config/arc/arc.c|  2 +-
 gcc/config/arm/arm.c|  2 +-
 gcc/config/bfin/bfin.c  |  2 +-
 gcc/config/c6x/c6x.c| 19 +--
 gcc/config/cris/cris-protos.h   |  2 +-
 gcc/config/cris/cris.c  |  2 +-
 gcc/config/h8300/h8300-protos.h |  4 ++--
 gcc/config/h8300/h8300.c|  4 ++--
 gcc/config/m32r/m32r.c  |  2 +-
 gcc/config/nds32/nds32-protos.h |  2 +-
 gcc/config/nds32/nds32.c|  2 +-
 gcc/config/rl78/rl78.c  | 11 ++-
 gcc/config/s390/s390-protos.h   |  2 +-
 gcc/config/s390/s390.c  |  2 +-
 gcc/config/sh/sh.c  |  2 +-
 gcc/config/sparc/sparc-protos.h |  2 +-
 gcc/config/sparc/sparc.c|  8 +++-
 18 files changed, 35 insertions(+), 37 deletions(-)

diff --git a/gcc/config/arc/arc-protos.h b/gcc/config/arc/arc-protos.h
index 8630e2d..73aacbc 100644
--- a/gcc/config/arc/arc-protos.h
+++ b/gcc/config/arc/arc-protos.h
@@ -109,7 +109,7 @@ extern rtx arc_regno_use_in (unsigned int, rtx);
 extern int arc_attr_type (rtx_insn *);
 extern bool arc_scheduling_not_expected (void);
 extern bool arc_sets_cc_p (rtx_insn *insn);
-extern int arc_label_align (rtx label);
+extern int arc_label_align (rtx_insn *label);
 extern bool arc_need_delay (rtx_insn *insn);
 extern bool arc_text_label (rtx_insn *insn);
 
diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
index c0aa075..2b25b0b 100644
--- a/gcc/config/arc/arc.c
+++ b/gcc/config/arc/arc.c
@@ -9243,7 +9243,7 @@ arc_scheduling_not_expected (void)
long.)  */
 
 int
-arc_label_align (rtx label)
+arc_label_align (rtx_insn *label)
 {
   int loop_align = LOOP_ALIGN (LABEL);
 
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 43a832e..63d0067 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -20860,7 +20860,7 @@ any_sibcall_could_use_r3 (void)
   FOR_EACH_EDGE (e, ei, EXIT_BLOCK_PTR_FOR_FN (cfun)->preds)
 if (e->flags & EDGE_SIBCALL)
   {
-   rtx call = BB_END (e->src);
+   rtx_insn *call = BB_END (e->src);
if (!CALL_P (call))
  call = prev_nonnote_nondebug_insn (call);
gcc_assert (CALL_P (call) && SIBLING_CALL_P (call));
diff --git a/gcc/config/bfin/bfin.c b/gcc/config/bfin/bfin.c
index 086d548..4683a28 100644
--- a/gcc/config/bfin/bfin.c
+++ b/gcc/config/bfin/bfin.c
@@ -4452,7 +4452,7 @@ workaround_speculation (void)
  || cbranch_predicted_taken_p (insn)))
{
  rtx_insn *target = JUMP_LABEL_AS_INSN (insn);
- rtx label = target;
+ rtx_insn *label = target;
  rtx_insn *next_tgt;
 
  cycles_since_jump = 0;
diff --git a/gcc/config/c6x/c6x.c b/gcc/config/c6x/c6x.c
index d759482..c250f8a 100644
--- a/gcc/config/c6x/c6x.c
+++ b/gcc/config/c6x/c6x.c
@@ -4807,10 +4807,10 @@ convert_to_callp (rtx_insn *insn)
 /* Scan forwards from INSN until we find the next insn that has mode TImode
(indicating it starts a new cycle), and occurs in cycle CLOCK.
Return it if we find such an insn, NULL_RTX otherwise.  */
-static rtx
-find_next_cycle_insn (rtx insn, int clock)
+static rtx_insn *
+find_next_cycle_insn (rtx_insn *insn, int clock)
 {
-  rtx t = insn;
+  rtx_insn *t = insn;
   if 

[PATCH 1/8] change a few rtx_insn * to rtx_jump_insn *

[tbsaunde+gcc]

From: Trevor Saunders 

gcc/ChangeLog:

2016-09-06  Trevor Saunders  

* bb-reorder.c (fix_crossing_unconditional_branches): Make type
of jump_insn rtx_jump_insn *.
* reorg.c (steal_delay_list_from_target): Make type of insn
rtx_jump_insn *.
(follow_jumps): Make type of jump rtx_jump_insn *.
---
 gcc/bb-reorder.c |  5 ++---
 gcc/reorg.c  | 12 ++--
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/gcc/bb-reorder.c b/gcc/bb-reorder.c
index bb8435f..b26c041 100644
--- a/gcc/bb-reorder.c
+++ b/gcc/bb-reorder.c
@@ -2146,7 +2146,7 @@ fix_crossing_unconditional_branches (void)
   rtx label;
   rtx label_addr;
   rtx_insn *indirect_jump_sequence;
-  rtx_insn *jump_insn = NULL;
+  rtx_jump_insn *jump_insn = NULL;
   rtx new_reg;
   rtx_insn *cur_insn;
   edge succ;
@@ -2201,8 +2201,7 @@ fix_crossing_unconditional_branches (void)
{
  if (!BARRIER_P (cur_insn))
BLOCK_FOR_INSN (cur_insn) = cur_bb;
- if (JUMP_P (cur_insn))
-   jump_insn = cur_insn;
+ jump_insn = dyn_cast (cur_insn);
}
 
  /* Insert the new (indirect) jump sequence immediately before
diff --git a/gcc/reorg.c b/gcc/reorg.c
index c58d608..d798c6a 100644
--- a/gcc/reorg.c
+++ b/gcc/reorg.c
@@ -205,7 +205,7 @@ static int redirect_with_delay_slots_safe_p (rtx_insn *, 
rtx, rtx);
 static int redirect_with_delay_list_safe_p (rtx_insn *, rtx,
const vec &);
 static int check_annul_list_true_false (int, const vec &);
-static void steal_delay_list_from_target (rtx_insn *, rtx, rtx_sequence *,
+static void steal_delay_list_from_target (rtx_jump_insn *, rtx, rtx_sequence *,
  vec *,
  struct resources *,
  struct resources *,
@@ -1033,10 +1033,10 @@ check_annul_list_true_false (int annul_true_p,
execution should continue.  */
 
 static void
-steal_delay_list_from_target (rtx_insn *insn, rtx condition, rtx_sequence *seq,
- vec *delay_list, resources *sets,
- struct resources *needed,
- struct resources *other_needed,
+steal_delay_list_from_target (rtx_jump_insn *insn, rtx condition,
+ rtx_sequence *seq, vec *delay_list,
+ resources *sets, resources *needed,
+ resources *other_needed,
  int slots_to_fill, int *pslots_filled,
  int *pannul_p, rtx *pnew_thread)
 {
@@ -2265,7 +2265,7 @@ fill_simple_delay_slots (int non_jumps_p)
set *CROSSING to true, otherwise set it to false.  */
 
 static rtx
-follow_jumps (rtx label, rtx_insn *jump, bool *crossing)
+follow_jumps (rtx label, rtx_jump_insn *jump, bool *crossing)
 {
   rtx_insn *insn;
   rtx_insn *next;
-- 
2.9.3

[PATCH 0/8] make next_*_insn take rtx_insn * arguments

[tbsaunde+gcc]

From: Trevor Saunders 

Hi,

Basically $subject.  First change variable's type to rtx_insn * where possible.
Then change the functions and fixup callers where it is still necessary to
cast.

patches bootstrapped and regtested individually on x86_64-linux-gnu, and the
series was run through config-list.mk, ok?

Trev

Trevor Saunders (8):
  change a few rtx_insn * to rtx_jump_insn *
  use rtx_insn * more
  make next/prev _nonnote_insn take rtx_insn *
  make next/prev nondebug_insn take rtx_insn *
  make prev_real_insn take rtx_insn *
  make next/prev nonnote_nondebug_insn take rtx_insn *
  make next/prev active_insn and active_insn_p take rtx_insn *
  make next_cc0_user take rtx_insn *

 gcc/bb-reorder.c|  5 ++--
 gcc/cfgcleanup.c|  2 +-
 gcc/cfgloop.h   |  2 +-
 gcc/config/arc/arc-protos.h |  2 +-
 gcc/config/arc/arc.c|  2 +-
 gcc/config/arc/arc.md   | 43 +++-
 gcc/config/arm/arm.c|  2 +-
 gcc/config/bfin/bfin.c  |  2 +-
 gcc/config/c6x/c6x.c| 19 ++---
 gcc/config/cris/cris-protos.h   |  4 +--
 gcc/config/cris/cris.c  | 14 -
 gcc/config/h8300/h8300-protos.h |  4 +--
 gcc/config/h8300/h8300.c|  4 +--
 gcc/config/m32r/m32r.c  |  2 +-
 gcc/config/nds32/nds32-protos.h |  2 +-
 gcc/config/nds32/nds32.c|  2 +-
 gcc/config/pa/pa.c  |  6 ++--
 gcc/config/rl78/rl78.c  | 11 +++
 gcc/config/s390/s390-protos.h   |  2 +-
 gcc/config/s390/s390.c  |  2 +-
 gcc/config/sh/sh.c  | 10 +++
 gcc/config/sh/sh.md |  3 +-
 gcc/config/sparc/sparc-protos.h |  2 +-
 gcc/config/sparc/sparc.c|  8 ++
 gcc/cse.c   |  2 +-
 gcc/dwarf2out.c |  2 +-
 gcc/emit-rtl.c  | 41 +++
 gcc/jump.c  | 18 ++--
 gcc/loop-doloop.c   |  2 +-
 gcc/reorg.c | 63 ++---
 gcc/resource.c  |  2 +-
 gcc/rtl.h   | 26 -
 gcc/simplify-rtx.c  |  8 +++---
 33 files changed, 160 insertions(+), 159 deletions(-)

-- 
2.9.3

Re: [PATCH 5/9] regrename: Don't run if function was separately shrink-wrapped

[Segher Boessenkool]

On Wed, Sep 14, 2016 at 11:52:02AM -0600, Jeff Law wrote:
> Yea, it'll certainly do that and I can imagine a design which would have 
> that property.  And there's other designs which benefit from spreading 
> across the register file as much as possible.
> 
> Which argues there needs to be a way to tune or disable the pass.

Yes, some targets want it, and some don't.  It seems to me that targets
that use sched1 have much less benefit from regrename.  Of course enabling
sched1 has its own problems.


Segher

Re: [PATCH 5/9] regrename: Don't run if function was separately shrink-wrapped

[Segher Boessenkool]

On Wed, Sep 14, 2016 at 12:11:50PM -0600, Jeff Law wrote:
> On 09/14/2016 07:04 AM, Segher Boessenkool wrote:
> >>I'd
> >>probably start by fixing the dataflow issues and see if that fixes the
> >>regrename thing as a side effect.
> >
> >Have you seen https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00091.html ?
> I missed it.

Yeah thought so, too much stuff in flight here.

> My interpretation
> 
> The uses at each "strange" exit fixing the first issue with 
> shrink-wrapping definitely sounds like we've got a dataflow problem of 
> some sort.
> 
> If you think about it, conceptually we want the return insn to make the 
> callee saved registers "used" so that DCE, regrename and friends don't 
> muck with them.  The fact that we don't is as much never having to care 
> all that much until recently.

(There is no return insn at those exits; these are exits *without*
successor block, not the exit block).

It is puzzling to me why adding USEs for just the registers that *are*
separately shrink-wrapped does not work; only all those that *could* be
shrink-wrapped does.  Do you have any idea about that?

> I continue to wonder if we could add something similar to 
> CALL_INSN_FUNCTION_USAGE where we attach uses for all the call-saved 
> registers onto a return insn.  We would attach them at the end of 
> prologue/epilogue generation and only attach those where were live 
> somewhere in the code.

Maybe adding the new insns to the {pro,epi}logue_insn_hash will help
something.  Or maybe it will blow up spectacularly.  Will know in a
bit.

> By deferring that step until after prologue/epilogue generation we 
> shouldn't cause unnecessary register saves/restores.

Hrm.  I'll see about that as well.

> I pondered just doing it for the separately wrapped components on that 
> particular path, but I've yet to convince myself that's actually correct.

If that is not correct, how is the status quo correct?  That is what
puzzles me above, too.

> Bernd knows the regrename code better than anyone.  Is there any way the 
> two of you could work together to try and track down what's going on in 
> the hash_map_rand case?  Even throwing in some more debug stuff might 
> help narrow things down since it's renaming something to a non-volatile, 
> non-separately shrink wrapped register that's causing problems.

Okay with me, I could certainly use his help.  I'll try the above things
first though, so not before friday.

> Can we agree that there's a set of targets that will improve and a set 
> that are harmed? And that to enable regrename by default we need to 
> either better describe the pipeline characteristics we're optimizing for 
> or a well defined way for targets to turn it off?

There is a well-defined way to turn it off, via common/config/*/*-common.c ,
TARGET_OPTION_OPTIMIZATION_TABLE.  We disagree on whether most targets will
want it enabled, i.e. whether it should (eventually) be enabled by default.


Segher

Re: [PATCH 5/9] regrename: Don't run if function was separately shrink-wrapped

[Jeff Law]

On 09/14/2016 08:01 AM, Bernd Schmidt wrote:

On 09/14/2016 03:55 PM, Segher Boessenkool wrote:

On Wed, Sep 14, 2016 at 03:08:21PM +0200, Bernd Schmidt wrote:

The data that was posted showed a code size decrease on a number of
targets. I'm really not sure where this irrational hate for regrename
comes from.


It increases the number of active, "young" registers per thread.

There is no irrational hate.  Regrename is simply a de-optimisation on
some (heavily) out-of-order targets.


Can you point me at a processor manual for such a chip that explains why
this would be the case?
Ponder a processor where the cost to access a register is non-uniform 
and related to how long ago a particular register was used.   This can 
happen when the actual hardware register file is much larger than the 
exposed register file (to support hardware renaming, hyperthreading, 
partitioning, etc).



 I imagine it could be the case if it enables
more aggressive scheduling but that's kind of one of the intended effects.

Exactly.


jeff

Re: [PATCH, updated] Add a warning for suspicious use of conditional expressions in boolean context

[Jason Merrill]

On Wed, Sep 14, 2016 at 1:37 PM, Bernd Edlinger
 wrote:
> On 09/14/16 18:37, Jason Merrill wrote:
>> On Wed, Sep 14, 2016 at 12:10 PM, Bernd Edlinger
>>  wrote:
>>> The other false positive was in dwarf2out, where we have this:
>>>
>>> ../../gcc-trunk/gcc/dwarf2out.c:26166:35: error: ?: using integer
>>> constants in boolean context [-Werror=int-in-bool-context]
>>> if (s->refcount
>>> == ((DEBUG_STR_SECTION_FLAGS & SECTION_MERGE) ? 1 : 2))
>>> ~^~~~
>>>
>>> and:
>>> #define DEBUG_STR_SECTION_FLAGS \
>>> (HAVE_GAS_SHF_MERGE && flag_merge_debug_strings   \
>>>  ? SECTION_DEBUG | SECTION_MERGE | SECTION_STRINGS | 1\
>>>  : SECTION_DEBUG)
>>>
>>> which got folded in C++ to
>>> if (s->refcount ==
>>> ((flag_merge_debug_strings ? SECTION_MERGE : 0) ? 1 : 2))
>>
>> Hmm, I'd think this warning should be looking at unfolded trees.
>>
>
> Yes.  The truthvalue_conversion is happening in cp_convert_and_check
> at cp_convert, afterwards the cp_fully_fold happens and does this
> transformation that I did not notice before, but just because I did
> not have the right test case.  Then if the folding did change
> something the truthvalue_conversion happens again, and this time
> the tree is folded potentially in a surprising way.
>
> The new version of the patch disables the warning in the second
> truthvalue_conversion and as a consequence has to use fold_for_warn
> on the now unfolded parameters.  This looks like an improvement
> that I would keep, because I would certainly like to warn on
> x ? 1 : 1+1, which the previous version did, but just by accident.
>
>>> Additional to what you suggested, I relaxed the condition even more,
>>> because I think it is also suspicious, if one of the branches is known
>>> to be outside [0:1] and the other is unknown.
>>>
>>> That caused another warning in the fortran FE, which was caused by
>>> wrong placement of braces in gfc_simplify_repeat.
>>>
>>> We have:
>>> #define mpz_sgn(Z) ((Z)->size < 0 ? -1 : (Z)->size > 0)
>>>
>>> Therefore the condition must be  .. && mpz_sgn(X) != 0)
>>>
>>> Previously that did not match, because -1 is outside [0:1]
>>> but (Z)->size > 0 is unknown.
>>
>> But (Z)->size > 0 is known to be boolean; that seems like it should
>> suppress this warning.
>
> Hmm, maybe it got a bit too pedantic, but in some cases this
> warning is pointing out something that is at least questionable
> and in some cases real bugs:
>
> For instance PR 77574, where in gcc/predict.c we had this:
>
> /* If edge is already improbably or cold, just return.  */
> if (e->probability <= impossible ? PROB_VERY_UNLIKELY : 0
> && (!impossible || !e->count))
>return;
>
> thus if (x <= y ? 4999 : 0 && (...))
>
>>> Furthermore the C++ test case df-warn-signedunsigned1.C also
>>> triggered the warning, because here we have:
>>>
>>> #define MAK (fl < 0 ? 1 : (fl ? -1 : 0))
>>>
>>> And thus "if (MAK)" should be written as if (MAK != 0)
>>
>> This also seems like a false positive to me.
>>
>> It's very common for code to rely on the implicit !=0 in boolean
>> conversion; it seems to me that the generalization to warn about
>> expressions with 0 arms significantly increases the frequency of false
>> positives, making the flag less useful.  The original form of the
>> warning seems like a -Wall candidate to me, but the current form
>> doesn't.
>
> Yes.  The reasoning I initially had was that it is completely
> pointless to have something of the form "if (x ? 1 : 2)" or
> "if (x ? 0 : 0)" because the result does not even depend on x
> in this case.  But something like "if (x ? 4999 : 0)" looks
> bogus but does at least not ignore x.
>
> If the false-positives are becoming too much of a problem here,
> then I should of course revert to the previous heuristic again.

I think we could have both, where the weaker form is part of -Wall and
people can explicitly select the stronger form.

Jason

Re: [PATCH 5/9] regrename: Don't run if function was separately shrink-wrapped

[Jeff Law]

On 09/14/2016 07:04 AM, Segher Boessenkool wrote:

I'd
probably start by fixing the dataflow issues and see if that fixes the
regrename thing as a side effect.


Have you seen https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00091.html ?

I missed it.  My interpretation

The uses at each "strange" exit fixing the first issue with 
shrink-wrapping definitely sounds like we've got a dataflow problem of 
some sort.


If you think about it, conceptually we want the return insn to make the 
callee saved registers "used" so that DCE, regrename and friends don't 
muck with them.  The fact that we don't is as much never having to care 
all that much until recently.


I continue to wonder if we could add something similar to 
CALL_INSN_FUNCTION_USAGE where we attach uses for all the call-saved 
registers onto a return insn.  We would attach them at the end of 
prologue/epilogue generation and only attach those where were live 
somewhere in the code.


By deferring that step until after prologue/epilogue generation we 
shouldn't cause unnecessary register saves/restores.


I pondered just doing it for the separately wrapped components on that 
particular path, but I've yet to convince myself that's actually correct.


Bernd knows the regrename code better than anyone.  Is there any way the 
two of you could work together to try and track down what's going on in 
the hash_map_rand case?  Even throwing in some more debug stuff might 
help narrow things down since it's renaming something to a non-volatile, 
non-separately shrink wrapped register that's causing problems.



I think enabling regrename by default is the right thing to do long term.


Then rs6000 (and many other ports probably) will just turn it off again.
It is actively harmful.

I think you're over stating things here.

Can we agree that there's a set of targets that will improve and a set 
that are harmed? And that to enable regrename by default we need to 
either better describe the pipeline characteristics we're optimizing for 
or a well defined way for targets to turn it off?



jeff

Re: Verify package integrity of downloaded prerequisites (partially fixes 61439)

[Joseph Myers]

On Wed, 14 Sep 2016, Moritz Klammler wrote:

> Ok, I didn't know about the workflow.  Do you think I should dike the
> `--strip-sums` option out again then?

I don't see any use for such an option.  Anyone changing the versions 
should always have a copy of the new tarball (obtained securely if 
possible) and should determine the sums from that.

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

Re: [C++ PATCH] Fix ICE in C++ lookup_name_fuzzy (PR c++/77549, take 2)

[Jason Merrill]

OK, thanks.

On Wed, Sep 14, 2016 at 3:43 AM, Jakub Jelinek  wrote:
> On Tue, Sep 13, 2016 at 11:46:52PM -0400, Jason Merrill wrote:
>> On Tue, Sep 13, 2016 at 3:26 PM, Jakub Jelinek  wrote:
>> >for (tree t = lvl->names; t; t = TREE_CHAIN (t))
>> >  {
>> > +  /* OVERLOADs or decls from using declaration are wrapped into
>> > +TREE_LIST.  */
>> > +  if (TREE_CODE (t) == TREE_LIST)
>> > +   {
>> > + t = TREE_VALUE (t);
>> > + t = OVL_CURRENT (t);
>> > +   }
>>
>> Don't we want the loop increment to take the TREE_CHAIN of the
>> TREE_LIST element, rather than its TREE_VALUE?
>
> Oops, thanks for catching this brown paper bag bug.
> I've added a testcase that fails with the previous version of the patch
> and succeeds now.  Ok for trunk if it passes bootstrap/regtest?
> (as the code changes anything only if it would formerly ICE, I'm afraid
> it isn't tested by anything but this new testcase)
>
> 2016-09-14  Jakub Jelinek  
>
> PR c++/77549
> * name-lookup.c (consider_binding_level): Look through TREE_LIST
> and OVERLOAD.
>
> * g++.dg/lookup/pr77549.C: New test.
>
> --- gcc/cp/name-lookup.c.jj 2016-09-13 19:06:24.664238422 +0200
> +++ gcc/cp/name-lookup.c2016-09-14 09:33:13.471134995 +0200
> @@ -4707,19 +4707,29 @@ consider_binding_level (tree name, best_
>
>for (tree t = lvl->names; t; t = TREE_CHAIN (t))
>  {
> +  tree d = t;
> +
> +  /* OVERLOADs or decls from using declaration are wrapped into
> +TREE_LIST.  */
> +  if (TREE_CODE (d) == TREE_LIST)
> +   {
> + d = TREE_VALUE (d);
> + d = OVL_CURRENT (d);
> +   }
> +
>/* Don't use bindings from implicitly declared functions,
>  as they were likely misspellings themselves.  */
> -  if (TREE_TYPE (t) == error_mark_node)
> +  if (TREE_TYPE (d) == error_mark_node)
> continue;
>
>/* Skip anticipated decls of builtin functions.  */
> -  if (TREE_CODE (t) == FUNCTION_DECL
> - && DECL_BUILT_IN (t)
> - && DECL_ANTICIPATED (t))
> +  if (TREE_CODE (d) == FUNCTION_DECL
> + && DECL_BUILT_IN (d)
> + && DECL_ANTICIPATED (d))
> continue;
>
> -  if (DECL_NAME (t))
> -   bm.consider (DECL_NAME (t));
> +  if (DECL_NAME (d))
> +   bm.consider (DECL_NAME (d));
>  }
>  }
>
> --- gcc/testsuite/g++.dg/lookup/pr77549.C.jj2016-09-14 09:15:31.135528309 
> +0200
> +++ gcc/testsuite/g++.dg/lookup/pr77549.C   2016-09-14 09:32:05.951981052 
> +0200
> @@ -0,0 +1,76 @@
> +// PR c++/77549
> +// { dg-do compile }
> +
> +struct A
> +{
> +  static int x;
> +};
> +
> +void
> +f1 ()
> +{
> +  using ::A;
> +  x;   // { dg-error "'x' was not declared in this scope" }
> +}
> +
> +namespace N
> +{
> +  int bar;
> +}
> +
> +void
> +f2 ()
> +{
> +  using N::bar;
> +  baz++;   // { dg-error "'baz' was not declared in this scope" }
> +}  // { dg-message "note: suggested alternative: 'bar'" 
> "" { target *-*-* } 25 }
> +
> +int
> +bar ()
> +{
> +  return 0;
> +}
> +
> +namespace M
> +{
> +  int
> +  bar ()
> +  {
> +return 0;
> +  }
> +}
> +
> +void
> +f3 ()
> +{
> +  using M::bar;
> +  baz ();  // { dg-error "'baz' was not declared in this scope" }
> +}  // { dg-message "note: suggested alternative: 'bar'" 
> "" { target *-*-* } 47 }
> +
> +namespace O
> +{
> +  int
> +  foo ()
> +  {
> +return 0;
> +  }
> +}
> +
> +namespace P
> +{
> +  int
> +  bar ()
> +  {
> +return 0;
> +  }
> +}
> +
> +void
> +f4 ()
> +{
> +  using O::foo;
> +  using P::bar;
> +  fooo (); // { dg-error "'fooo' was not declared in this scope" 
> }
> +   // { dg-message "note: suggested alternative: 'foo'" 
> "" { target *-*-* } 73 }
> +  baz ();  // { dg-error "'baz' was not declared in this scope" }
> +}  // { dg-message "note: suggested alternative: 'bar'" 
> "" { target *-*-* } 75 }
>
>
> Jakub

Re: [PATCH] Improve string::clear() performance

[Cong Wang]

On Wed, Sep 14, 2016 at 10:28 AM, Jonathan Wakely  wrote:
> On 14/09/16 09:09 -0700, Cong Wang wrote:
>>
>> On Wed, Sep 14, 2016 at 4:06 AM, Jonathan Wakely 
>> wrote:
>>>
>>> If I understand the purpose of the change correctly, it's similar to
>>> https://gcc.gnu.org/ml/gcc-patches/2014-06/msg00278.html - is that
>>> right?
>>>
>>
>> Oh, yes, I didn't know your patch because I don't subscribe
>> gcc mailing list. I am wondering why your patch is not merged
>> after 2+ years?
>
>
> As I said in the patch email, I'm not sure if it's conforming, due to
> clearing the string's cpacity() as well as its size().
>

OK. My patch keeps the original logic except the case for
_GLIBCXX_FULLY_DYNAMIC_STRING, should be at least
as correct as the current code. ;)


>> Please let me know what you prefer: 1) You update your patch
>> and get it merged; 2) Use my patch if it looks good. I am fine with
>> either way. :)
>
>
> I'll refresh my memory of the various issues and reconsider applying
> my patch (that way we don't need to wait for your copyright
> assignment). The performance benefits you measured make it more
> compelling.

Sure.

For long term, I think gcc should have something as simple as
'Signed-off-by' for Linux kernel, otherwise too much work for first-time
contributors like me. We all want to save time on this, don't we? ;)

Thanks.

Re: [PATCH, updated] Add a warning for suspicious use of conditional expressions in boolean context

[Bernd Edlinger]

On 09/14/16 19:40, Steve Kargl wrote:
> On Wed, Sep 14, 2016 at 04:10:46PM +, Bernd Edlinger wrote:
>>
>> fortran:
>> 2016-09-14  Bernd Edlinger  
>>
>>  PR c++/77434
>>  * simplify.c (gfc_simplify_repeat): Fix a warning.
>>
>> Index: gcc/fortran/simplify.c
>> ===
>> --- gcc/fortran/simplify.c   (revision 240135)
>> +++ gcc/fortran/simplify.c   (working copy)
>> @@ -5127,7 +5127,7 @@ gfc_simplify_repeat (gfc_expr *e, gfc_expr *n)
>>
>> if (len ||
>> (e->ts.u.cl->length &&
>> -   mpz_sgn (e->ts.u.cl->length->value.integer)) != 0)
>> +   mpz_sgn (e->ts.u.cl->length->value.integer) != 0))
>>   {
>> const char *res = gfc_extract_int (n, );
>> gcc_assert (res == NULL);
>
> This part should be committed regardless of the
> outcome of a review of the complete patch.  The
> closing ')' is clearly missed placed.
>

OK, thanks, then I will go ahead and commit that one as obvious.


Bernd.

Re: [PATCH 5/9] regrename: Don't run if function was separately shrink-wrapped

[Jeff Law]

On 09/14/2016 07:55 AM, Segher Boessenkool wrote:

On Wed, Sep 14, 2016 at 03:08:21PM +0200, Bernd Schmidt wrote:

On 09/14/2016 03:04 PM, Segher Boessenkool wrote:

Then rs6000 (and many other ports probably) will just turn it off again.
It is actively harmful.


The data that was posted showed a code size decrease on a number of
targets. I'm really not sure where this irrational hate for regrename
comes from.


It increases the number of active, "young" registers per thread.
Yea, it'll certainly do that and I can imagine a design which would have 
that property.  And there's other designs which benefit from spreading 
across the register file as much as possible.


Which argues there needs to be a way to tune or disable the pass.

Jeff

Re: [PATCH, updated] Add a warning for suspicious use of conditional expressions in boolean context

[Steve Kargl]

On Wed, Sep 14, 2016 at 04:10:46PM +, Bernd Edlinger wrote:
> 
> fortran:
> 2016-09-14  Bernd Edlinger  
> 
>   PR c++/77434
>   * simplify.c (gfc_simplify_repeat): Fix a warning.
> 
> Index: gcc/fortran/simplify.c
> ===
> --- gcc/fortran/simplify.c(revision 240135)
> +++ gcc/fortran/simplify.c(working copy)
> @@ -5127,7 +5127,7 @@ gfc_simplify_repeat (gfc_expr *e, gfc_expr *n)
>  
>if (len ||
>(e->ts.u.cl->length &&
> -   mpz_sgn (e->ts.u.cl->length->value.integer)) != 0)
> +   mpz_sgn (e->ts.u.cl->length->value.integer) != 0))
>  {
>const char *res = gfc_extract_int (n, );
>gcc_assert (res == NULL);

This part should be committed regardless of the
outcome of a review of the complete patch.  The
closing ')' is clearly missed placed.

-- 
Steve

Re: C++ PATCH to forbid use of bool with the ++ operator

[Jason Merrill]

OK, thanks.

On Wed, Sep 14, 2016 at 11:28 AM, Marek Polacek  wrote:
> On Tue, Sep 13, 2016 at 11:37:20PM -0400, Jason Merrill wrote:
>> On Tue, Sep 13, 2016 at 3:55 PM, Martin Sebor  wrote:
>> > Having said all that, since this is C++ the message could and
>> > arguably should refer to a bool expression (or type) instead
>> > and avoid having to deal with this altogether. In fact, it
>> > would be simpler to rephrase the message as:
>> >
>> >   "use of an operand of type %qT in ... is deprecated",
>> >   boolean_type_node
>>
>> Yes.
>
> Should be done in the patch below.
>
> Bootstrapped/regtested on x86_64-linux and ppc64-linux, ok for trunk?
>
> 2016-09-14  Marek Polacek  
>
> * typeck.c (cp_build_unary_op): Diagnose incrementing boolean
> expressions.  Tweak an error message.
>
> * c-c++-common/gomp/atomic-12.c: Use -Wno-deprecated.
> * c-c++-common/gomp/atomic-13.c: Likewise.
> * c-c++-common/gomp/atomic-14.c: Likewise.
> * g++.dg/cpp1y/lambda-init11.C: Remove invalid code.
> * g++.dg/cpp1z/bool-increment1.C: New test.
> * c-c++-common/pr60439.c: Add dg-warning.
> * g++.dg/expr/bitfield4.C: Likewise.
> * g++.dg/expr/bitfield5.C: Likewise.
> * g++.dg/expr/bitfield6.C: Likewise.
> * g++.dg/expr/bool1.C: Likewise.
> * g++.dg/expr/bool3.C: Likewise.
> * g++.dg/expr/lval3.C: Likewise.
> * g++.dg/expr/lval4.C: Likewise.
> * g++.old-deja/g++.jason/bool5.C: Likewise.
> * g++.dg/expr/bitfield3.C: Adjust dg-error.
> * g++.dg/other/error18.C: Likewise.
> * g++.dg/gomp/atomic-14.C: Likewise.
> libgomp/
> * testsuite/libgomp.c++/atomic-3.C: Use -Wno-deprecated.
> libstdc++-v3/
> * testsuite/23_containers/vector/debug/insert6_neg.cc: Use
> -Wno-deprecated.
>
> diff --git gcc/gcc/cp/typeck.c gcc/gcc/cp/typeck.c
> index a591d29..808cb6c 100644
> --- gcc/gcc/cp/typeck.c
> +++ gcc/gcc/cp/typeck.c
> @@ -6030,16 +6030,32 @@ cp_build_unary_op (enum tree_code code, tree xarg, 
> int noconvert,
>   complain))
>   return error_mark_node;
>
> -   /* Forbid using -- on `bool'.  */
> +   /* Forbid using -- or ++ in C++17 on `bool'.  */
> if (TREE_CODE (declared_type) == BOOLEAN_TYPE)
>   {
> if (code == POSTDECREMENT_EXPR || code == PREDECREMENT_EXPR)
>   {
>  if (complain & tf_error)
> -  error ("invalid use of Boolean expression as operand "
> - "to %");
> + error ("use of an operand of type %qT in % "
> +"is forbidden", boolean_type_node);
> return error_mark_node;
>   }
> +   else
> + {
> +   if (cxx_dialect >= cxx1z)
> + {
> +   if (complain & tf_error)
> + error ("use of an operand of type %qT in "
> +"% is forbidden in C++1z",
> +boolean_type_node);
> +   return error_mark_node;
> + }
> +   /* Otherwise, [depr.incr.bool] says this is deprecated.  */
> +   else if (!in_system_header_at (input_location))
> + warning (OPT_Wdeprecated, "use of an operand of type %qT "
> +  "in % is deprecated",
> +  boolean_type_node);
> + }
> val = boolean_increment (code, arg);
>   }
> else if (code == POSTINCREMENT_EXPR || code == POSTDECREMENT_EXPR)
> diff --git gcc/gcc/testsuite/c-c++-common/gomp/atomic-12.c 
> gcc/gcc/testsuite/c-c++-common/gomp/atomic-12.c
> index 145e420..e9ca650 100644
> --- gcc/gcc/testsuite/c-c++-common/gomp/atomic-12.c
> +++ gcc/gcc/testsuite/c-c++-common/gomp/atomic-12.c
> @@ -1,6 +1,6 @@
>  /* PR middle-end/45423 */
>  /* { dg-do compile } */
> -/* { dg-options "-fopenmp -fdump-tree-gimple -g0" } */
> +/* { dg-options "-fopenmp -fdump-tree-gimple -g0 -Wno-deprecated" } */
>  /* atomicvar should never be referenced in between the barrier and
> following #pragma omp atomic_load.  */
>  /* { dg-final { scan-tree-dump-not "barrier\[^#\]*atomicvar" "gimple" } } */
> diff --git gcc/gcc/testsuite/c-c++-common/gomp/atomic-13.c 
> gcc/gcc/testsuite/c-c++-common/gomp/atomic-13.c
> index 2452035..7f4afcf 100644
> --- gcc/gcc/testsuite/c-c++-common/gomp/atomic-13.c
> +++ gcc/gcc/testsuite/c-c++-common/gomp/atomic-13.c
> @@ -1,6 +1,6 @@
>  /* PR middle-end/45423 */
>  /* { dg-do compile } */
> -/* { dg-options "-fopenmp -fdump-tree-gimple -g0 -O2" } */
> +/* { dg-options "-fopenmp -fdump-tree-gimple -g0 -O2 -Wno-deprecated" } */
>  /* atomicvar should never be referenced in between the barrier and
> following #pragma omp atomic_load.  

Re: [PATCH, updated] Add a warning for suspicious use of conditional expressions in boolean context

[Bernd Edlinger]

On 09/14/16 18:37, Jason Merrill wrote:
> On Wed, Sep 14, 2016 at 12:10 PM, Bernd Edlinger
>  wrote:
>> The other false positive was in dwarf2out, where we have this:
>>
>> ../../gcc-trunk/gcc/dwarf2out.c:26166:35: error: ?: using integer
>> constants in boolean context [-Werror=int-in-bool-context]
>> if (s->refcount
>> == ((DEBUG_STR_SECTION_FLAGS & SECTION_MERGE) ? 1 : 2))
>> ~^~~~
>>
>> and:
>> #define DEBUG_STR_SECTION_FLAGS \
>> (HAVE_GAS_SHF_MERGE && flag_merge_debug_strings   \
>>  ? SECTION_DEBUG | SECTION_MERGE | SECTION_STRINGS | 1\
>>  : SECTION_DEBUG)
>>
>> which got folded in C++ to
>> if (s->refcount ==
>> ((flag_merge_debug_strings ? SECTION_MERGE : 0) ? 1 : 2))
>
> Hmm, I'd think this warning should be looking at unfolded trees.
>

Yes.  The truthvalue_conversion is happening in cp_convert_and_check
at cp_convert, afterwards the cp_fully_fold happens and does this
transformation that I did not notice before, but just because I did
not have the right test case.  Then if the folding did change
something the truthvalue_conversion happens again, and this time
the tree is folded potentially in a surprising way.

The new version of the patch disables the warning in the second
truthvalue_conversion and as a consequence has to use fold_for_warn
on the now unfolded parameters.  This looks like an improvement
that I would keep, because I would certainly like to warn on
x ? 1 : 1+1, which the previous version did, but just by accident.

>> Additional to what you suggested, I relaxed the condition even more,
>> because I think it is also suspicious, if one of the branches is known
>> to be outside [0:1] and the other is unknown.
>>
>> That caused another warning in the fortran FE, which was caused by
>> wrong placement of braces in gfc_simplify_repeat.
>>
>> We have:
>> #define mpz_sgn(Z) ((Z)->size < 0 ? -1 : (Z)->size > 0)
>>
>> Therefore the condition must be  .. && mpz_sgn(X) != 0)
>>
>> Previously that did not match, because -1 is outside [0:1]
>> but (Z)->size > 0 is unknown.
>
> But (Z)->size > 0 is known to be boolean; that seems like it should
> suppress this warning.
>

Hmm, maybe it got a bit too pedantic, but in some cases this
warning is pointing out something that is at least questionable
and in some cases real bugs:

For instance PR 77574, where in gcc/predict.c we had this:

/* If edge is already improbably or cold, just return.  */
if (e->probability <= impossible ? PROB_VERY_UNLIKELY : 0
&& (!impossible || !e->count))
   return;

thus if (x <= y ? 4999 : 0 && (...))

>> Furthermore the C++ test case df-warn-signedunsigned1.C also
>> triggered the warning, because here we have:
>>
>> #define MAK (fl < 0 ? 1 : (fl ? -1 : 0))
>>
>> And thus "if (MAK)" should be written as if (MAK != 0)
>
> This also seems like a false positive to me.
>
> It's very common for code to rely on the implicit !=0 in boolean
> conversion; it seems to me that the generalization to warn about
> expressions with 0 arms significantly increases the frequency of false
> positives, making the flag less useful.  The original form of the
> warning seems like a -Wall candidate to me, but the current form
> doesn't.
>

Yes.  The reasoning I initially had was that it is completely
pointless to have something of the form "if (x ? 1 : 2)" or
"if (x ? 0 : 0)" because the result does not even depend on x
in this case.  But something like "if (x ? 4999 : 0)" looks
bogus but does at least not ignore x.

If the false-positives are becoming too much of a problem here,
then I should of course revert to the previous heuristic again.



Thanks
Bernd.

Re: Verify package integrity of downloaded prerequisites (partially fixes 61439)

[Moritz Klammler]

Joseph Myers  writes:

> On Wed, 14 Sep 2016, Moritz Klammler wrote:
>
>> be cleaner to only include those checksums that are actually needed.  On
>> the other hand, it means an increased maintenance burden each time the
>> version of the dependency is changed.  In order to mitigate this and
>
> I really don't see it as an increased burden.  The maintainer shouldn't be 
> using the checksum files on the server at all.  What they should do is:
>
> * Download the tar file from ftp.gnu.org (at least for GMP / MPFR / MPC), 
> *verify the GPG signature* and test with it.  (I'm not sure if the GNU 
> keyring is currently published.)  The GPG signatures on ftp.gnu.org are 
> from the maintainer who uploaded the package, whereas the checksum files 
> on gcc.gnu.org are automatically generated from cron.  (I don't know if a 
> secure way to download ISL from its origin has been added since 
>  raised the issue.)
>
> * Update the script and the to-be-checked-in checksums, using the file 
> they just downloaded and verified the signature of.
>
> * Add the new file to the server before the script changes get checked in.

Ok, I didn't know about the workflow.  Do you think I should dike the
`--strip-sums` option out again then?

Re: [PATCH] Improve string::clear() performance

[Jonathan Wakely]

On 14/09/16 09:09 -0700, Cong Wang wrote:

On Wed, Sep 14, 2016 at 4:06 AM, Jonathan Wakely  wrote:

On 13/09/16 11:02 -0700, Cong Wang wrote:


In !_GLIBCXX_USE_CXX11_ABI implementation, string::clear() calls
_M_mutate(), which could allocate memory as we do COW. This hurts
performance when string::clear() is on the hot path.

This patch improves it by using _S_empty_rep directly when
_GLIBCXX_FULLY_DYNAMIC_STRING is not enabled. And Linux distro like
Fedora doesn't enable this, this is why we caught it.

The copy-and-clear test shows it improves by 50%.

Ran all testsuites on Linux-x64.



Thank you for the patch (and changelog and test results!).

Do you have a GCC copyright assignment in place? See
https://gcc.gnu.org/contribute.html#legal for details.


Oh, didn't notice this, I thought gcc has something as quick
as the 'Signed-off-by' for Linux kernel (I am a Linux kernel developer).
I will do it.



If I understand the purpose of the change correctly, it's similar to
https://gcc.gnu.org/ml/gcc-patches/2014-06/msg00278.html - is that
right?



Oh, yes, I didn't know your patch because I don't subscribe
gcc mailing list. I am wondering why your patch is not merged
after 2+ years?


As I said in the patch email, I'm not sure if it's conforming, due to
clearing the string's cpacity() as well as its size().


Please let me know what you prefer: 1) You update your patch
and get it merged; 2) Use my patch if it looks good. I am fine with
either way. :)


I'll refresh my memory of the various issues and reconsider applying
my patch (that way we don't need to wait for your copyright
assignment). The performance benefits you measured make it more
compelling.

Re: Verify package integrity of downloaded prerequisites (partially fixes 61439)

[Joseph Myers]

On Wed, 14 Sep 2016, Moritz Klammler wrote:

> be cleaner to only include those checksums that are actually needed.  On
> the other hand, it means an increased maintenance burden each time the
> version of the dependency is changed.  In order to mitigate this and

I really don't see it as an increased burden.  The maintainer shouldn't be 
using the checksum files on the server at all.  What they should do is:

* Download the tar file from ftp.gnu.org (at least for GMP / MPFR / MPC), 
*verify the GPG signature* and test with it.  (I'm not sure if the GNU 
keyring is currently published.)  The GPG signatures on ftp.gnu.org are 
from the maintainer who uploaded the package, whereas the checksum files 
on gcc.gnu.org are automatically generated from cron.  (I don't know if a 
secure way to download ISL from its origin has been added since 
 raised the issue.)

* Update the script and the to-be-checked-in checksums, using the file 
they just downloaded and verified the signature of.

* Add the new file to the server before the script changes get checked in.

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

[PATCH] Fix incorrect setting of edge probability in loop unroller

[Pat Haugen]

The following patch corrects a problem with setting the probability on the pred 
edge instead of the newly created fallthru succ edge.

Bootstrap/regtest on powerpc64le with no new regressions. Committed as obvious.

-Pat


2016-09-14  Pat Haugen  

* loop-unroll.c (unroll_loop_runtime_iterations): Set probability of 
succ edge.

Index: gcc/loop-unroll.c
===
--- gcc/loop-unroll.c   (revision 240092)
+++ gcc/loop-unroll.c   (working copy)
@@ -979,7 +979,7 @@ unroll_loop_runtime_iterations (struct l
 
   swtch = split_edge_and_insert (single_pred_edge (swtch), branch_code);
   set_immediate_dominator (CDI_DOMINATORS, preheader, swtch);
-  single_pred_edge (swtch)->probability = REG_BR_PROB_BASE - p;
+  single_succ_edge (swtch)->probability = REG_BR_PROB_BASE - p;
   e = make_edge (swtch, preheader,
 single_succ_edge (swtch)->flags & EDGE_IRREDUCIBLE_LOOP);
   e->count = RDIV (preheader->count * REG_BR_PROB_BASE, p);

Re: [PATCH] Optimize strchr (s, 0) to strlen

[Szabolcs Nagy]

On 14/09/16 14:45, Jakub Jelinek wrote:
> I think for the middle-end, using strchr (a, 0) as canonical instead of a + 
> strlen (a)
> is better, and at expansion time we can decide what to use (a + strlen (a)
> if you'd expand strlen inline, rather than as a function call, or
> __rawmemchr (which if libc is sane should be fastest), or strchr, or a + 
> strlen (a)).
> 

i think the compiler should move away from generating
calls to nonstandard functions, __rawmemchr is much
less used than strlen so it's less likely to be optimized
in certain target/libc combination.

Re: [PATCH] accept flexible arrays in struct in unions (c++/71912 - [6/7 regression])

[Martin Sebor]

Attached is an updated patch that does away with the "overlapping"
warning and instead issues the same warnings as the C front end
(though with more context).

In struct flexmems_t I've retained the NEXT array even though only
the first element is used to diagnose problems.  The second element
is used by the find_flexarrays function to differentiate structs
with flexible array members in unions (which are valid) from those
in structs (which are not).

FWIW, I think this C restriction on structs is unnecessary and will
propose to remove it so that's valid to define a struct that contains
another struct (possibly recursively) with a flexible array member.
I.e., I think this should be made valid in C (and accepted without
the pedantic warning that GCC, and with this patch also G++, issues):

  struct X { int i, a[]; };
  struct S1 { struct X x; };
  struct S2 { struct S1 s1; };

z.C:2:22: warning: invalid use of ‘struct X’ with a flexible array 
member in ‘struct S1’ [-Wpedantic]

 struct S1 { struct X x; };
  ^
z.C:1:21: note: array member ‘int X::a []’ declared here
 struct X { int i, a[]; };
 ^
z.C:3:23: warning: invalid use of ‘struct X’ with a flexible array 
member in ‘struct S2’ [-Wpedantic]

 struct S2 { struct S1 s1; };
   ^~
z.C:1:21: note: array member ‘int X::a []’ declared here
 struct X { int i, a[]; };
 ^

Martin
PR c++/71912 - [6/7 regression] flexible array in struct in union rejected

gcc/cp/ChangeLog:
	PR c++/71912
	* class.c (struct flexmems_t):  Add members.
	(find_flexarrays): Add arguments.  Correct handling of anonymous
	structs.
	(diagnose_flexarrays): Adjust to issue warnings in addition to errors.
	(check_flexarrays): Add argument.
	(anon_context, diagnose_invalid_flexarray): New functions.

gcc/testsuite/ChangeLog:
	PR c++/71912
	* g++.dg/ext/flexary4.C: Adjust.
	* g++.dg/ext/flexary5.C: Same.
	* g++.dg/ext/flexary9.C: Same.
	* g++.dg/ext/flexary19.C: New test.
	* g++.dg/ext/flexary18.C: New test.
	* g++.dg/torture/pr64312.C: Add a dg-error directive to an ill-formed
	regression test.
* g++.dg/compat/struct-layout-1_generate.c (subfield): Add argument.
Avoid generating a flexible array member in an array.

diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index f7147e6..6ae4fe2 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -147,11 +147,12 @@ static void check_methods (tree);
 static void remove_zero_width_bit_fields (tree);
 static bool accessible_nvdtor_p (tree);
 
-/* Used by find_flexarrays and related.  */
+/* Used by find_flexarrays and related functions.  */
 struct flexmems_t;
-static void find_flexarrays (tree, flexmems_t *);
 static void diagnose_flexarrays (tree, const flexmems_t *);
-static void check_flexarrays (tree, flexmems_t * = NULL);
+static void find_flexarrays (tree, flexmems_t *, bool = false,
+			 tree = NULL_TREE, tree = NULL_TREE);
+static void check_flexarrays (tree, flexmems_t * = NULL, bool = false);
 static void check_bases (tree, int *, int *);
 static void check_bases_and_members (tree);
 static tree create_vtable_ptr (tree, tree *);
@@ -6711,53 +6712,155 @@ field_nonempty_p (const_tree fld)
   return false;
 }
 
-/* Used by find_flexarrays and related.  */
-struct flexmems_t {
+/* Used by find_flexarrays and related functions.  */
+
+struct flexmems_t
+{
   /* The first flexible array member or non-zero array member found
- in order of layout.  */
+ in the order of layout.  */
   tree array;
   /* First non-static non-empty data member in the class or its bases.  */
   tree first;
-  /* First non-static non-empty data member following either the flexible
- array member, if found, or the zero-length array member.  */
-  tree after;
+  /* The first non-static non-empty data member following either
+ the flexible array member, if found, or the zero-length array member
+ otherwise.  AFTER[1] refers to the first such data member of a union
+ of which the struct containing the flexible array member or zero-length
+ array is a member, or NULL when no such union exists.  This element is
+ only used during searching, not for diagnosing problems.  AFTER[0]
+ refers to the first such data member that is not a member of such
+ a union.  */
+  tree after[2];
+
+  /* Refers to a struct (not union) in which the struct of which the flexible
+ array is member is defined.  Used to diagnose strictly (according to C)
+ invalid uses of the latter structs.  */
+  tree enclosing;
 };
 
 /* Find either the first flexible array member or the first zero-length
-   array, in that order or preference, among members of class T (but not
-   its base classes), and set members of FMEM accordingly.  */
+   array, in that order of preference, among members of class T (but not
+   its base classes), and set members of FMEM accordingly.
+   BASE_P is true if T is a base class of another class.
+   PUN is set to the outermost union in 

Re: Verify package integrity of downloaded prerequisites (partially fixes 61439)

[Moritz Klammler]

Richard Biener  writes:

> On Tue, Sep 13, 2016 at 6:01 PM, Joseph Myers  wrote:
>> On Tue, 13 Sep 2016, Moritz Klammler wrote:
>>
>>> I have made an actual diff now, containing also the checksum files.
>>> I
>>
>> I don't think checksums of lots of miscellaneous files should be
>> included, just the checksums for those files the current script will
>> actually use.
>
> I generally like the script but agree with Joseph here.  We should be
> able to verify (upon committing changes to the script) to verify the
> sums by performing ./contrib/download_prerequesites [--md5] which
> means only including those that can be verified that way.
>
> Richard.

Thanks both of you for your feedback.  I generally agree that it would
be cleaner to only include those checksums that are actually needed.  On
the other hand, it means an increased maintenance burden each time the
version of the dependency is changed.  In order to mitigate this and
eventually get the best of both worlds, I have added another
(intentionally undocumented) option `--strip-sums` to the script that
will strip the checksum file to include only the packages it will
download.  A maintainer can run the script with this option to both
strip the checksum file and immediately test the new configuration.  A
comment in the script next to where the version numbers are set reminds
you to do that.

If there were also GPG signatures of the checksum files on the server, I
could further modify the script to even download (and verify) the new
checksum files automatically, assuming that the maintainer who applies
the update will have the GPG key of the person who maintains the FTP
directory.  (Might well be the same person anyway, I don't know.)

By the way: as mentioned in the bug tracker, there also is the
`./contrib/download_ecj` script but I believe that it will become
obsolete with the upcoming deletion of GCJ so we can ignore it for now.
Is this right?


* contrib/download_prerequisites: Verify integrity of downloaded
packages and added more command-line options.

* contrib/prerequisites.sha512: New.

* contrib/prerequisites.md5: New.
Index: contrib/download_prerequisites
===
--- contrib/download_prerequisites	(revision 240137)
+++ contrib/download_prerequisites	(working copy)
@@ -1,60 +1,240 @@
-#! /bin/sh
+#! /bin/sh -eu
+#! -*- coding:utf-8; mode:shell-script; -*-
 
-# Download some prerequisites needed by gcc.
-# Run this from the top level of the gcc source tree and the gcc
-# build will do the right thing.
+# Download some prerequisites needed by GCC.
+# Run this from the top level of the GCC source tree and the GCC build will do
+# the right thing.  Run it with the `--help` option for more information.
 #
-# (C) 2010-2016 Free Software Foundation
+# (C) 2016 Free Software Foundation
 #
-# This program is free software: you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or
-# (at your option) any later version.
-# 
-# This program is distributed in the hope that it will be useful, but
-# WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
-# General Public License for more details.
-# 
-# You should have received a copy of the GNU General Public License
-# along with this program. If not, see http://www.gnu.org/licenses/.
+# This program is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free Software
+# Foundation, either version 3 of the License, or (at your option) any later
+# version.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+# details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program.  If not, see http://www.gnu.org/licenses/.
 
-# If you want to disable Graphite loop optimizations while building GCC,
-# DO NOT set GRAPHITE_LOOP_OPT as yes so that the isl package will not
-# be downloaded.
-GRAPHITE_LOOP_OPT=yes
 
-if [ ! -e gcc/BASE-VER ] ; then
-	echo "You must run this script in the top level GCC source directory."
-	exit 1
-fi
+program='download_prerequisites'
+version='(unversioned)'
 
-# Necessary to build GCC.
-MPFR=mpfr-3.1.4
-GMP=gmp-6.1.0
-MPC=mpc-1.0.3
+# MAINTAINERS: If you update the package versions below, please
+# replace the files `contrib/prerequisites.sha512` and
+# `contrib/prerequisites.md5` with a new authoritative copy of the
+# checksum files and then run this script with the (intentionally
+# undocumented) `--strip-sums` option.  This will strip any unneeded
+# 

Re: [PATCH] Tree-level fix for PR 69526

[Jeff Law]

[ Another patch I'd started working through, but hadn't completed... ]

On 09/14/2016 07:05 AM, Richard Biener wrote:

On Mon, Aug 22, 2016 at 4:58 PM, Robin Dapp  wrote:

if !inner_ovf (just set that to false if !check_inner_ovf to simplify
checks please).
you know it's valid to transform the op to (T)@0 innerop (T)@1 outerop @2
(if T is wider than the inner type which I think you should check and
which should
simplify things).


I simplified the control flow a little and split both parts of the
combination into separate patterns.


You can then combine (T)@1 and @2 where I think you fail to handle mixed
MINUS_EXPR/PLUS_EXPR (practically you'll see only PLUS_EXPRs for

integers).

Is it ok to do something like this (see patch) in order to deal with
MINUS_EXPR and then perform a wi::add?

if (inner_op == MINUS_EXPR)
w1 = wi::neg (w1);

if (outer_op == MINUS_EXPR)
w2 = wi::neg (w2);


Yes.

I was concerned about the case where w1 or w2 might be the minimum 
(negative) integer for its type.  That creates a constant that can't be 
represented.  I'm not familiar enough with the rest of hte code yet to 
know if that's problematical.






tree.c doesn't look like the best fit.  I think putting it into
tree-vrp.c is better
and I think that extract_range_from_binary_expr_1 itself should

compute what we

want here as additional output.  Conservative handling for all but

plus/minus is

ok with me.


I kept the extra function for now because I find
extract_range_from_binary_expr_1 somewhat lengthy and hard to follow
already :)


Heh, but I think duplicating code is even worse.
I was going to suggest a pre-patch that just refactored the various 
cases in extract_range_from_binary_expr_1 into their own functions. 
THat might it easier to keep things manageable.


[ ... ]



Now looking at binop_overflow (that I don't like very much, but ...)
Note that the naked "return true" in there makes 95% of that function 
unreachable code.  That's where I stopped without looking deeply at the 
rest of the code.


Jeff

Re: [PATCH GCC 6/9]Simplify control flow graph for vectorized loop

[Jeff Law]

On 09/14/2016 07:21 AM, Richard Biener wrote:

On Tue, Sep 6, 2016 at 8:52 PM, Bin Cheng  wrote:

Hi,
This is the main patch improving control flow graph for vectorized loop.  It 
generally rewrites loop peeling stuff in vectorizer.  As described in patch, 
for a typical loop to be vectorized like:

   preheader:
 LOOP:
   header_bb:
 loop_body
 if (exit_loop_cond) goto exit_bb
 elsegoto header_bb
   exit_bb:

This patch peels prolog and epilog from the loop, adds guards skipping PROLOG 
and EPILOG for various conditions.  As a result, the changed CFG would look 
like:

   guard_bb_1:
 if (prefer_scalar_loop) goto merge_bb_1
 elsegoto guard_bb_2

   guard_bb_2:
 if (skip_prolog) goto merge_bb_2
 else goto prolog_preheader

   prolog_preheader:
 PROLOG:
   prolog_header_bb:
 prolog_body
 if (exit_prolog_cond) goto prolog_exit_bb
 else  goto prolog_header_bb
   prolog_exit_bb:

   merge_bb_2:

   vector_preheader:
 VECTOR LOOP:
   vector_header_bb:
 vector_body
 if (exit_vector_cond) goto vector_exit_bb
 else  goto vector_header_bb
   vector_exit_bb:

   guard_bb_3:
 if (skip_epilog) goto merge_bb_3
 else goto epilog_preheader

   merge_bb_1:

   epilog_preheader:
 EPILOG:
   epilog_header_bb:
 epilog_body
 if (exit_epilog_cond) goto merge_bb_3
 else  goto epilog_header_bb

   merge_bb_3:


Note this patch peels prolog and epilog only if it's necessary, as well as adds 
different guard_conditions/branches.  Also the first guard/branch could be 
further improved by merging it with loop versioning.

Before this patch, up to 4 branch instructions need to be executed before the 
vectorized loop is reached in the worst case, while the number is reduced to 2 
with this patch.  The patch also does better in compile time analysis to avoid 
unnecessary peeling/branching.
From implementation's point of view, vectorizer needs to update induction 
variables and iteration bounds along with control flow changes.  Unfortunately, 
it also becomes much harder to follow because slpeel_* functions updates SSA by 
itself, rather than using update_ssa interface.  This patch tries to factor out 
SSA/IV/Niter_bound changes from CFG changes.  This should make the 
implementation easier to read, and I think it maybe a step forward to replace 
slpeel_* functions with generic GIMPLE loop copy interfaces as Richard 
suggested.


I've skimmed over the patch and it looks reasonable to me.
THanks.  I was maybe 15% of the way through the main patch.  Nothing 
that gave me cause for concern, but I wasn't ready to ACK it myself yet.


jeff

Re: [PATCH, updated] Add a warning for suspicious use of conditional expressions in boolean context

[Jason Merrill]

On Wed, Sep 14, 2016 at 12:10 PM, Bernd Edlinger
 wrote:
> The other false positive was in dwarf2out, where we have this:
>
> ../../gcc-trunk/gcc/dwarf2out.c:26166:35: error: ?: using integer
> constants in boolean context [-Werror=int-in-bool-context]
>if (s->refcount
>== ((DEBUG_STR_SECTION_FLAGS & SECTION_MERGE) ? 1 : 2))
>~^~~~
>
> and:
> #define DEBUG_STR_SECTION_FLAGS \
>(HAVE_GAS_SHF_MERGE && flag_merge_debug_strings   \
> ? SECTION_DEBUG | SECTION_MERGE | SECTION_STRINGS | 1\
> : SECTION_DEBUG)
>
> which got folded in C++ to
>if (s->refcount ==
>((flag_merge_debug_strings ? SECTION_MERGE : 0) ? 1 : 2))

Hmm, I'd think this warning should be looking at unfolded trees.

> Additional to what you suggested, I relaxed the condition even more,
> because I think it is also suspicious, if one of the branches is known
> to be outside [0:1] and the other is unknown.
>
> That caused another warning in the fortran FE, which was caused by
> wrong placement of braces in gfc_simplify_repeat.
>
> We have:
> #define mpz_sgn(Z) ((Z)->size < 0 ? -1 : (Z)->size > 0)
>
> Therefore the condition must be  .. && mpz_sgn(X) != 0)
>
> Previously that did not match, because -1 is outside [0:1]
> but (Z)->size > 0 is unknown.

But (Z)->size > 0 is known to be boolean; that seems like it should
suppress this warning.

> Furthermore the C++ test case df-warn-signedunsigned1.C also
> triggered the warning, because here we have:
>
> #define MAK (fl < 0 ? 1 : (fl ? -1 : 0))
>
> And thus "if (MAK)" should be written as if (MAK != 0)

This also seems like a false positive to me.

It's very common for code to rely on the implicit !=0 in boolean
conversion; it seems to me that the generalization to warn about
expressions with 0 arms significantly increases the frequency of false
positives, making the flag less useful.  The original form of the
warning seems like a -Wall candidate to me, but the current form
doesn't.

Jason

Re: RFC: PATCH to consider MAX_OFILE_ALIGNMENT for targetm.absolute_biggest_alignment

[Thomas Schwinge]

Hi!

On Tue, 13 Sep 2016 16:27:39 +0200, Bernd Schmidt  wrote:
> On 09/13/2016 04:24 PM, Jason Merrill wrote:
> 
> > But we could define TARGET_ABSOLUTE_BIGGEST_ALIGNMENT on nvptx instead
> > of on x86; is this OK?
> 
> That's what I had in mind. It would be good if Thomas or Nathan could 
> give this patch a spin, I'm not currently really set up for it. But it 
> looks like a reasonable try to me.

I'm happy to report that this patch doesn't cause any changes in test
results both for nvptx target testing, and for nvptx offloading testing.
But I have not examined in detail what it actually does ;-) -- currently
occupied with too much other work already.

> > I'm still not sure why you need an alignment cap on nvptx, but I'm not
> > going to worry about it anymore.  :)
> 
> I think it was the cfgexpand machinery that uses dynamic allocations 
> when a variable has a bigger alignment than the stack, and you really 
> don't want these on ptx.

It will be good to document that, next to the definition in
gcc/config/nvptx/nvptx.h maybe?


Grüße
 Thomas


signature.asc
Description: PGP signature

Re: RFA (libstdc++): PATCH to implement C++17 over-aligned new

[Jason Merrill]

On Wed, Sep 14, 2016 at 12:08 PM, Christophe Lyon
 wrote:
> On 14 September 2016 at 14:11, Andreas Schwab  wrote:
>> On Sep 13 2016, Jason Merrill  wrote:
>>
>>> On Tue, Sep 13, 2016 at 9:03 AM, Andreas Schwab  
>>> wrote:
 On Sep 13 2016, Jason Merrill  wrote:

> Does this help?

 Unfortunatly no.
>>>
>>> It occurs to me that this function doesn't need to restrict types at
>>> all.  I'm checking this in; hopefully it will do the trick.
>>
>> That didn't change anything either.
>>
>
> I confirm no change on arm* either.
> On aarch64, gen-attrs-[25]1.C, and devirt-33 now work.
> name-clash11.C still fails on both targets

Ah, I needed to remove the limit on field alignment as well.  This
seems to fix things.
commit 367afdb008cfb122e9bb2c73ba0a8d8b29a738c0
Author: Jason Merrill 
Date:   Wed Sep 14 11:12:09 2016 -0400

* c-common.c (check_cxx_fundamental_alignment_constraints): Don't
limit FIELD_DECL, either.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index b561f9f..57b6671 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -7868,43 +7868,21 @@ check_cxx_fundamental_alignment_constraints (tree node,
   if (cxx_fundamental_alignment_p (requested_alignment))
 return true;
 
-  if (DECL_P (node))
+  if (VAR_P (node))
 {
   if (TREE_STATIC (node))
-   {
- /* For file scope variables and static members, the target
-supports alignments that are at most
-MAX_OFILE_ALIGNMENT.  */
- if (requested_alignment > (max_align = MAX_OFILE_ALIGNMENT))
-   alignment_too_large_p = true;
-   }
+   /* For file scope variables and static members, the target supports
+  alignments that are at most MAX_OFILE_ALIGNMENT.  */
+   max_align = MAX_OFILE_ALIGNMENT;
   else
-   {
-#ifdef BIGGEST_FIELD_ALIGNMENT
-#define MAX_TARGET_FIELD_ALIGNMENT BIGGEST_FIELD_ALIGNMENT
-#else
-#define MAX_TARGET_FIELD_ALIGNMENT BIGGEST_ALIGNMENT
-#endif
- /* For non-static members, the target supports either
-alignments that at most either BIGGEST_FIELD_ALIGNMENT
-if it is defined or BIGGEST_ALIGNMENT.  */
- max_align = MAX_TARGET_FIELD_ALIGNMENT;
- if (TREE_CODE (node) == FIELD_DECL
- && requested_alignment > (max_align = MAX_TARGET_FIELD_ALIGNMENT))
-   alignment_too_large_p = true;
-#undef MAX_TARGET_FIELD_ALIGNMENT
- /* For stack variables, the target supports at most
-MAX_STACK_ALIGNMENT.  */
- else if (decl_function_context (node) != NULL
-  && requested_alignment > (max_align = MAX_STACK_ALIGNMENT))
-   alignment_too_large_p = true;
-   }
-}
-  else if (TYPE_P (node))
-{
-  /* Let's be liberal for types; don't limit their alignment any more than
-check_user_alignment already did.  */
+   /* For stack variables, the target supports at most
+  MAX_STACK_ALIGNMENT.  */
+   max_align = MAX_STACK_ALIGNMENT;
+  if (requested_alignment > max_align)
+   alignment_too_large_p = true;
 }
+  /* Let's be liberal for types and fields; don't limit their alignment any
+ more than check_user_alignment already did.  */
 
   if (alignment_too_large_p)
 pedwarn (input_location, OPT_Wattributes,
diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-52.C 
b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-52.C
index 0f87fd4..ad7cffc 100644
--- a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-52.C
+++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-52.C
@@ -3,19 +3,22 @@
 struct A {int i;}  a [[gnu::aligned(16)]];
 struct B {int i;} __attribute__((aligned(16))) b;
 
+constexpr unsigned si = sizeof(int);
+constexpr unsigned ai = alignof(int);
+
 int
 main ()
 {
  A aa;
  B bb;
 
- static_assert (sizeof (a) == 4, "sizeof (a) should be 4");
+ static_assert (sizeof (a) == si, "sizeof (a) should be 4");
  static_assert (sizeof (b) == 16, "sizeof (b) should be 16");
- static_assert (sizeof (aa) == 4, "sizeof (aa) should be 4");
+ static_assert (sizeof (aa) == si, "sizeof (aa) should be 4");
  static_assert (sizeof (bb) == 16, "sizeof (bb) should be 16");
 
  static_assert (__alignof__  (a) == 16, "alignof (a) should be 16");
  static_assert (__alignof__  (b) == 16, "alignof (b) should be 16");
- static_assert (__alignof__  (aa) == 4, "alignof (aa) should be 4");
+ static_assert (__alignof__  (aa) == ai, "alignof (aa) should be 4");
  static_assert (__alignof__  (bb) == 16, "alignof (bb) should be 16");
 }

Re: [PATCH 5/9] regrename: Don't run if function was separately shrink-wrapped

[Segher Boessenkool]

Hi Bernd,

On Wed, Sep 14, 2016 at 04:01:34PM +0200, Bernd Schmidt wrote:
> On 09/14/2016 03:55 PM, Segher Boessenkool wrote:
> >On Wed, Sep 14, 2016 at 03:08:21PM +0200, Bernd Schmidt wrote:
> >>The data that was posted showed a code size decrease on a number of
> >>targets. I'm really not sure where this irrational hate for regrename
> >>comes from.
> >
> >It increases the number of active, "young" registers per thread.
> >
> >There is no irrational hate.  Regrename is simply a de-optimisation on
> >some (heavily) out-of-order targets.
> 
> Can you point me at a processor manual for such a chip that explains why 
> this would be the case?

The POWER8 UM explains it a bit.  I don't have an URL, everything is behind
a registration wall it seems.

> >(It also makes the generated code much harder to read, but you know that).
> 
> Can't say I do, really. I imagine it could be the case if it enables 
> more aggressive scheduling but that's kind of one of the intended effects.

We have 32 int regs, 32 float regs, 32 vec regs, 8 cond regs.  It becomes
really hard to read if things are pretty much randomly jumbled about, as
regrename does.

The dump files are almost impossible to read btw, regrename prints the
assembler name of registers only (not the RTL name), and the assembler name
of all of GPRn, FPRn, VRn, CRn is "n".  All (?) other passes print the RTL
register number (or both).


Segher

Re: [PATCH] fix-it hints for warn_uninit

[Jeff Law]

On 09/14/2016 05:34 AM, Bernd Schmidt wrote:

On 09/14/2016 02:45 AM, David Malcolm wrote:

In combination with -fdiagnostics-generate-patch this can generate output
like this:

  --- ../../src/gcc/testsuite/c-c++-common/fix-missing-initializer-1.c
  +++ ../../src/gcc/testsuite/c-c++-common/fix-missing-initializer-1.c
  @@ -2,7 +2,7 @@

   int test_int (void)
   {
  -  int ivar;
  +  int ivar = 0;
 return ivar;  /* { dg-warning "used uninitialized" } */
 /* { dg-begin-multiline-output "" }
  return ivar;


I have to admit I feel uneasy about this. Just initializing stuff to
zero is the naive approach to shutting up the warning, but there's no
reason to think it's the correct fix. I think this is a warning where a
human who understands what's supposed to be going on needs to take a
look. Automating the work of a bad programmer seems like it could lead
to rather unfortuante outcomes.
I'd started writing something longer which said the same essentially the 
same thing.


For uninitialized variable uses the thing to do is to provide a human 
readable explanation of the path (conditions) which lead to the 
uninitialized use.  The developer really needs to decide what to do to 
fix their code.


jeff

[PATCH, updated] Add a warning for suspicious use of conditional expressions in boolean context

[Bernd Edlinger]

Hi Jeff,

it turned out, that the changed symmetric condition in the warning hit
at several places, but also caused two false positives which I had to
address first.  I tried also building a recent glibc, which hit a false
positive when using __builtin_isinf_sign in boolean context, which
is used extensively by glibc.  Furtunately there were zero new warnings
in linux, well done Linus ;)

glibc's math.h has this definition:

# if __GNUC_PREREQ (4,4) && !defined __SUPPORT_SNAN__
#  define isinf(x) __builtin_isinf_sign (x)

and __builtin_isinf_sign(x) is internally folded as

isinf(x) ? (signbit(x) ? -1 : 1) : 0

which can trigger the warning if used in a boolean context.

We do not want to warn for if (isinf(x)) of course.

The other false positive was in dwarf2out, where we have this:

../../gcc-trunk/gcc/dwarf2out.c:26166:35: error: ?: using integer 
constants in boolean context [-Werror=int-in-bool-context]
   if (s->refcount
   == ((DEBUG_STR_SECTION_FLAGS & SECTION_MERGE) ? 1 : 2))
   ~^~~~

and:
#define DEBUG_STR_SECTION_FLAGS \
   (HAVE_GAS_SHF_MERGE && flag_merge_debug_strings   \
? SECTION_DEBUG | SECTION_MERGE | SECTION_STRINGS | 1\
: SECTION_DEBUG)

which got folded in C++ to
   if (s->refcount ==
   ((flag_merge_debug_strings ? SECTION_MERGE : 0) ? 1 : 2))

Because SECTION_MERGE = 0x08000, the condition for the warning is
satisfied here, but that is only an artifact that is created by the
folding of the outer COND_EXPR.

Additional to what you suggested, I relaxed the condition even more,
because I think it is also suspicious, if one of the branches is known
to be outside [0:1] and the other is unknown.

That caused another warning in the fortran FE, which was caused by
wrong placement of braces in gfc_simplify_repeat.

We have:
#define mpz_sgn(Z) ((Z)->size < 0 ? -1 : (Z)->size > 0)

Therefore the condition must be  .. && mpz_sgn(X) != 0)

Previously that did not match, because -1 is outside [0:1]
but (Z)->size > 0 is unknown.

To suppress the bogus C++ warnings I had to suppress the warning
when the condition is fully folded in cp_convert_and_check and
c_common_truthvalue_conversion is called for the second time.

To suppress the bogus C warning on __builtin_isinf_sign I found
no other way than changing the expansion of that to
isinf(x) ? (signbit(x) ? -1 : 1) + 0 : 0
which is just enough to hide the inner COND_EXPR from
c_common_truthvalue_conversion, but gets immediately folded
away by fold_binary so it will cause no different code.

Furthermore the C++ test case df-warn-signedunsigned1.C also
triggered the warning, because here we have:

#define MAK (fl < 0 ? 1 : (fl ? -1 : 0))

And thus "if (MAK)" should be written as if (MAK != 0)

Finally, what I already wrote in my previous mail
the macros in gensupport.c mix binary and ternary
logic and should be cleaned up.


Bootstrap and reg-test with all languages on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.
gcc:
2016-09-14  Bernd Edlinger  

PR c++/77434
* doc/invoke.texi: Document -Wcond-in-bool-context.
* gensupport.c (TRISTATE_AND, TRISTATE_OR,
TRISTATE_NOT): Fix a warning.
* tree.h (integer_zerop_or_onep): New helper function.

PR middle-end/77421
* dwarf2out.c (output_loc_operands): Fix an assertion.

c-family:
2016-09-14  Bernd Edlinger  

PR c++/77434
* c.opt (Wcond-in-bool-context): New warning.
* c-common.c (c_common_truthvalue_conversion): Warn on integer
constants in boolean context.

cp:
2016-09-14  Bernd Edlinger  

PR c++/77434
* cvt.c (cp_convert_and_check): Suppress Wint-in-bool-context here.

fortran:
2016-09-14  Bernd Edlinger  

PR c++/77434
* simplify.c (gfc_simplify_repeat): Fix a warning.

testsuite:
2016-09-14  Bernd Edlinger  

PR c++/77434
* c-c++-common/Wcond-in-bool-context.c: New test.
* g++.dg/delayedfold/df-warn-signedunsigned1.C: Fix a warning.
Index: gcc/builtins.c
===
--- gcc/builtins.c	(revision 240135)
+++ gcc/builtins.c	(working copy)
@@ -7887,15 +7887,18 @@ fold_builtin_classify (location_t loc, tree fndecl
 	tree isinf_call = build_call_expr_loc (loc, isinf_fn, 1, arg);
 
 	signbit_call = fold_build2_loc (loc, NE_EXPR, integer_type_node,
-	signbit_call, integer_zero_node);
+	signbit_call, integer_zero_node);
 	isinf_call = fold_build2_loc (loc, NE_EXPR, integer_type_node,
-  isinf_call, integer_zero_node);
+	  isinf_call, integer_zero_node);
 
-	tmp = fold_build3_loc (loc, COND_EXPR, integer_type_node, signbit_call,
-			   integer_minus_one_node, integer_one_node);
 	tmp = fold_build3_loc (loc, 

Re: [PATCH 3/4][Ada,DJGPP] Ada support for DJGPP

[Andris Pavenis]

On 09/05/2016 08:39 AM, Arnaud Charlet wrote:

GCC Ada compiler itself uses exceptions.

Yes, but the bootstrap doesn't use system-dgjpp.ads, it uses the generic
system.ads which uses front-end (GNAT) SJLJ, hence my question. Your
answer isn't very reassuring.
Tried several test examples which uses exceptions. These examples worked for DJGPP as expected with 
today
build of gcc-7.0.0-20160914. ZCX_By_Default in system-djgpp.ads had value true as in last version 
of patch.


Andris

Re: [PATCH] Improve string::clear() performance

[Cong Wang]

On Wed, Sep 14, 2016 at 4:06 AM, Jonathan Wakely  wrote:
> On 13/09/16 11:02 -0700, Cong Wang wrote:
>>
>> In !_GLIBCXX_USE_CXX11_ABI implementation, string::clear() calls
>> _M_mutate(), which could allocate memory as we do COW. This hurts
>> performance when string::clear() is on the hot path.
>>
>> This patch improves it by using _S_empty_rep directly when
>> _GLIBCXX_FULLY_DYNAMIC_STRING is not enabled. And Linux distro like
>> Fedora doesn't enable this, this is why we caught it.
>>
>> The copy-and-clear test shows it improves by 50%.
>>
>> Ran all testsuites on Linux-x64.
>
>
> Thank you for the patch (and changelog and test results!).
>
> Do you have a GCC copyright assignment in place? See
> https://gcc.gnu.org/contribute.html#legal for details.

Oh, didn't notice this, I thought gcc has something as quick
as the 'Signed-off-by' for Linux kernel (I am a Linux kernel developer).
I will do it.

>
> If I understand the purpose of the change correctly, it's similar to
> https://gcc.gnu.org/ml/gcc-patches/2014-06/msg00278.html - is that
> right?
>

Oh, yes, I didn't know your patch because I don't subscribe
gcc mailing list. I am wondering why your patch is not merged
after 2+ years?

Please let me know what you prefer: 1) You update your patch
and get it merged; 2) Use my patch if it looks good. I am fine with
either way. :)

Thanks.

Re: RFA (libstdc++): PATCH to implement C++17 over-aligned new

[Christophe Lyon]

On 14 September 2016 at 14:11, Andreas Schwab  wrote:
> On Sep 13 2016, Jason Merrill  wrote:
>
>> On Tue, Sep 13, 2016 at 9:03 AM, Andreas Schwab  
>> wrote:
>>> On Sep 13 2016, Jason Merrill  wrote:
>>>
 Does this help?
>>>
>>> Unfortunatly no.
>>
>> It occurs to me that this function doesn't need to restrict types at
>> all.  I'm checking this in; hopefully it will do the trick.
>
> That didn't change anything either.
>

I confirm no change on arm* either.
On aarch64, gen-attrs-[25]1.C, and devirt-33 now work.
name-clash11.C still fails on both targets

Thanks,

Christophe

> Andreas.
>
> --
> Andreas Schwab, sch...@linux-m68k.org
> GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
> "And now for something completely different."

Re: C++ PATCH to forbid use of bool with the ++ operator

[Marek Polacek]

On Tue, Sep 13, 2016 at 11:37:20PM -0400, Jason Merrill wrote:
> On Tue, Sep 13, 2016 at 3:55 PM, Martin Sebor  wrote:
> > Having said all that, since this is C++ the message could and
> > arguably should refer to a bool expression (or type) instead
> > and avoid having to deal with this altogether. In fact, it
> > would be simpler to rephrase the message as:
> >
> >   "use of an operand of type %qT in ... is deprecated",
> >   boolean_type_node
> 
> Yes.

Should be done in the patch below.

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

2016-09-14  Marek Polacek  

* typeck.c (cp_build_unary_op): Diagnose incrementing boolean
expressions.  Tweak an error message.

* c-c++-common/gomp/atomic-12.c: Use -Wno-deprecated.
* c-c++-common/gomp/atomic-13.c: Likewise.
* c-c++-common/gomp/atomic-14.c: Likewise.
* g++.dg/cpp1y/lambda-init11.C: Remove invalid code.
* g++.dg/cpp1z/bool-increment1.C: New test.
* c-c++-common/pr60439.c: Add dg-warning.
* g++.dg/expr/bitfield4.C: Likewise.
* g++.dg/expr/bitfield5.C: Likewise.
* g++.dg/expr/bitfield6.C: Likewise.
* g++.dg/expr/bool1.C: Likewise.
* g++.dg/expr/bool3.C: Likewise.
* g++.dg/expr/lval3.C: Likewise.
* g++.dg/expr/lval4.C: Likewise.
* g++.old-deja/g++.jason/bool5.C: Likewise.
* g++.dg/expr/bitfield3.C: Adjust dg-error.
* g++.dg/other/error18.C: Likewise.
* g++.dg/gomp/atomic-14.C: Likewise.
libgomp/
* testsuite/libgomp.c++/atomic-3.C: Use -Wno-deprecated.
libstdc++-v3/
* testsuite/23_containers/vector/debug/insert6_neg.cc: Use
-Wno-deprecated.

diff --git gcc/gcc/cp/typeck.c gcc/gcc/cp/typeck.c
index a591d29..808cb6c 100644
--- gcc/gcc/cp/typeck.c
+++ gcc/gcc/cp/typeck.c
@@ -6030,16 +6030,32 @@ cp_build_unary_op (enum tree_code code, tree xarg, int 
noconvert,
  complain))
  return error_mark_node;
 
-   /* Forbid using -- on `bool'.  */
+   /* Forbid using -- or ++ in C++17 on `bool'.  */
if (TREE_CODE (declared_type) == BOOLEAN_TYPE)
  {
if (code == POSTDECREMENT_EXPR || code == PREDECREMENT_EXPR)
  {
 if (complain & tf_error)
-  error ("invalid use of Boolean expression as operand "
- "to %");
+ error ("use of an operand of type %qT in % "
+"is forbidden", boolean_type_node);
return error_mark_node;
  }
+   else
+ {
+   if (cxx_dialect >= cxx1z)
+ {
+   if (complain & tf_error)
+ error ("use of an operand of type %qT in "
+"% is forbidden in C++1z",
+boolean_type_node);
+   return error_mark_node;
+ }
+   /* Otherwise, [depr.incr.bool] says this is deprecated.  */
+   else if (!in_system_header_at (input_location))
+ warning (OPT_Wdeprecated, "use of an operand of type %qT "
+  "in % is deprecated",
+  boolean_type_node);
+ }
val = boolean_increment (code, arg);
  }
else if (code == POSTINCREMENT_EXPR || code == POSTDECREMENT_EXPR)
diff --git gcc/gcc/testsuite/c-c++-common/gomp/atomic-12.c 
gcc/gcc/testsuite/c-c++-common/gomp/atomic-12.c
index 145e420..e9ca650 100644
--- gcc/gcc/testsuite/c-c++-common/gomp/atomic-12.c
+++ gcc/gcc/testsuite/c-c++-common/gomp/atomic-12.c
@@ -1,6 +1,6 @@
 /* PR middle-end/45423 */
 /* { dg-do compile } */
-/* { dg-options "-fopenmp -fdump-tree-gimple -g0" } */
+/* { dg-options "-fopenmp -fdump-tree-gimple -g0 -Wno-deprecated" } */
 /* atomicvar should never be referenced in between the barrier and
following #pragma omp atomic_load.  */
 /* { dg-final { scan-tree-dump-not "barrier\[^#\]*atomicvar" "gimple" } } */
diff --git gcc/gcc/testsuite/c-c++-common/gomp/atomic-13.c 
gcc/gcc/testsuite/c-c++-common/gomp/atomic-13.c
index 2452035..7f4afcf 100644
--- gcc/gcc/testsuite/c-c++-common/gomp/atomic-13.c
+++ gcc/gcc/testsuite/c-c++-common/gomp/atomic-13.c
@@ -1,6 +1,6 @@
 /* PR middle-end/45423 */
 /* { dg-do compile } */
-/* { dg-options "-fopenmp -fdump-tree-gimple -g0 -O2" } */
+/* { dg-options "-fopenmp -fdump-tree-gimple -g0 -O2 -Wno-deprecated" } */
 /* atomicvar should never be referenced in between the barrier and
following #pragma omp atomic_load.  */
 /* { dg-final { scan-tree-dump-not "barrier\[^#\]*atomicvar" "gimple" } } */
diff --git gcc/gcc/testsuite/c-c++-common/gomp/atomic-14.c 
gcc/gcc/testsuite/c-c++-common/gomp/atomic-14.c
index f8fc9d8..7e23453 100644
--- gcc/gcc/testsuite/c-c++-common/gomp/atomic-14.c
+++ 

Re: C/C++ PATCH to change {cp,}build_unary_op's parameter

[Marek Polacek]

On Wed, Sep 14, 2016 at 10:58:39AM -0400, Jason Merrill wrote:
> On Wed, Sep 14, 2016 at 9:22 AM, Marek Polacek  wrote:
> > * c-common.h (build_unary_op): Change a parameter type to bool.
> > (build_unary_op): Change a parameter type to bool, use true/false
> > (cp_build_unary_op): Change a parameter type to bool.  Use true 
> > instead
> > (build_unary_op): Change a parameter type to bool.
> 
> It would be more useful if these said "noconvert parameter" rather
> than "a parameter".  OK with that change.

Committed with that fixed, thanks.

Marek

Re: C++ PATCH to forbid use of bool with the ++ operator

[Jason Merrill]

On Wed, Sep 14, 2016 at 9:08 AM, Marek Polacek  wrote:
> On Tue, Sep 13, 2016 at 01:55:56PM -0600, Martin Sebor wrote:
>> > -   /* Forbid using -- on `bool'.  */
>> > +   /* Forbid using -- or ++ in C++17 on `bool'.  */
>> > if (TREE_CODE (declared_type) == BOOLEAN_TYPE)
>> >   {
>> > if (code == POSTDECREMENT_EXPR || code == PREDECREMENT_EXPR)
>> > @@ -6040,6 +6040,20 @@ cp_build_unary_op (enum tree_code code, tree xarg, 
>> > int noconvert,
>> >"to %");
>> > return error_mark_node;
>> >   }
>> > +   else
>> > + {
>> > +   if (cxx_dialect >= cxx1z)
>> > + {
>> > +   if (complain & tf_error)
>> > + error ("use of Boolean expression as operand "
>> > +"to % is forbidden in C++1z");
>>
>> The capitalization of Boolean here caught my eye because it's
>> inconsistent with the recent spelling adopted in the documentation.
>> (It's also missing an article "a Boolean expression," although
>> dropping those is common in diagnostics. Still, it would be nice
>> to have a guideline/convention and use it consistently.)
>>
>> Back to Boolean, I was actually going to comment on the Boolean
>> -> boolean change and suggest going in the opposite direction but
>> in the end decided not to (as Sandra's links showed, there's support
>> for both).
>>
>> But having seen Boolean capitalized here I have changed my mind
>> again. I'd like to (belatedly) speak up in support of Boolean
>> (though I feel less strongly about it than I do about consistency).
>
> Well, my point was to get rid of this inconsistency, I don't really
> care whether it's Boolean or boolean.  But since boolean was used
> most of the time, I went with that.

Martin's point, which I agree with, is that for C++ we want to use
"bool" rather than either of those.

Jason

Re: C/C++ PATCH to change {cp,}build_unary_op's parameter

[Jason Merrill]

On Wed, Sep 14, 2016 at 9:22 AM, Marek Polacek  wrote:
> * c-common.h (build_unary_op): Change a parameter type to bool.
> (build_unary_op): Change a parameter type to bool, use true/false
> (cp_build_unary_op): Change a parameter type to bool.  Use true 
> instead
> (build_unary_op): Change a parameter type to bool.

It would be more useful if these said "noconvert parameter" rather
than "a parameter".  OK with that change.

Jason

Re: C++ PATCH to forbid use of bool with the ++ operator

[Marek Polacek]

On Wed, Sep 14, 2016 at 10:54:35AM -0400, Jason Merrill wrote:
> Martin's point, which I agree with, is that for C++ we want to use
> "bool" rather than either of those.

Sure, I'm testing another version of this patch which has this fixed.

Marek

PR c++/77539

[Nathan Sidwell]

I've committed this backport to the gcc-6 branch.  I'll commit the new testcase 
to trunk, so we don't regress in future.


nathan
2016-09-14  Nathan Sidwell  

	cp/
	PR c++/77539
	* constexpr.c (get_fundef_copy): Use the original function for
	non-recursive evaluations.
	(save_fundef_copy): Always expect a slot to be available.

	testsuite/
	PR c++/77539
	* g++.dg/cpp0x/constexpr-recursion3.C: New.
	* g++.dg/ubsan/pr63956.C: Adjust error location.
	* g++.dg/cpp1y/pr77539.C: New.

Index: cp/constexpr.c
===
--- cp/constexpr.c	(revision 240079)
+++ cp/constexpr.c	(working copy)
@@ -994,7 +994,8 @@ maybe_initialize_fundef_copies_table ()
 }
 
 /* Reuse a copy or create a new unshared copy of the function FUN.
-   Return this copy.  */
+   Return this copy.  We use a TREE_LIST whose PURPOSE is body, VALUE
+   is parms, TYPE is result.  */
 
 static tree
 get_fundef_copy (tree fun)
@@ -1002,15 +1003,26 @@ get_fundef_copy (tree fun)
   maybe_initialize_fundef_copies_table ();
 
   tree copy;
-  tree *slot = fundef_copies_table->get (fun);
-  if (slot == NULL || *slot == NULL_TREE)
+  bool existed;
+  tree *slot = _copies_table->get_or_insert (fun, );
+
+  if (!existed)
 {
+  /* There is no cached function available, or in use.  We can use
+	 the function directly.  That the slot is now created records
+	 that this function is now in use.  */
+  copy = build_tree_list (DECL_SAVED_TREE (fun), DECL_ARGUMENTS (fun));
+  TREE_TYPE (copy) = DECL_RESULT (fun);
+}
+  else if (*slot == NULL_TREE)
+{
+  /* We've already used the function itself, so make a copy.  */
   copy = build_tree_list (NULL, NULL);
-  /* PURPOSE is body, VALUE is parms, TYPE is result.  */
   TREE_PURPOSE (copy) = copy_fn (fun, TREE_VALUE (copy), TREE_TYPE (copy));
 }
   else
 {
+  /* We have a cached function available.  */
   copy = *slot;
   *slot = TREE_CHAIN (copy);
 }
@@ -1018,12 +1030,14 @@ get_fundef_copy (tree fun)
   return copy;
 }
 
-/* Save the copy COPY of function FUN for later reuse by get_fundef_copy().  */
+/* Save the copy COPY of function FUN for later reuse by
+   get_fundef_copy().  By construction, there will always be an entry
+   to find.  */
 
 static void
 save_fundef_copy (tree fun, tree copy)
 {
-  tree *slot = _copies_table->get_or_insert (fun, NULL);
+  tree *slot = fundef_copies_table->get (fun);
   TREE_CHAIN (copy) = *slot;
   *slot = copy;
 }
Index: testsuite/g++.dg/cpp0x/constexpr-recursion3.C
===
--- testsuite/g++.dg/cpp0x/constexpr-recursion3.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/constexpr-recursion3.C	(working copy)
@@ -0,0 +1,14 @@
+// { dg-do compile { target c++11 } }
+
+constexpr int Foo (int i)
+{
+  return (i ? Foo (i - 1): 0) + i;
+}
+
+static int a = Foo (0);
+static int b = Foo (1);
+static int d = Foo (3);
+static int c = Foo (2);
+static int e = Foo (4);
+static int g = Foo (6);
+static int f = Foo (5);
Index: testsuite/g++.dg/cpp1y/pr77539.C
===
--- testsuite/g++.dg/cpp1y/pr77539.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/pr77539.C	(working copy)
@@ -0,0 +1,14 @@
+// PR c++/77539
+// { dg-do compile { target c++14 } }
+
+constexpr int foobar()
+{
+  int array[100] = {};
+  int *ar = array;
+  if (ar == nullptr) // Error...
+  {
+return 0;
+  }
+  return 1;
+}
+static_assert(foobar(),"");
Index: testsuite/g++.dg/ubsan/pr63956.C
===
--- testsuite/g++.dg/ubsan/pr63956.C	(revision 240079)
+++ testsuite/g++.dg/ubsan/pr63956.C	(working copy)
@@ -92,7 +92,7 @@ constexpr int
 fn6 (const int , int b)
 {
   if (b != 2)
-b = a;
+b = a;  // { dg-error "is not a constant expression" }
   return b;
 }
 
@@ -106,7 +106,7 @@ fn7 (const int *a, int b)
 
 constexpr int n1 = 7;
 constexpr int n2 = fn7 (, 5);
-constexpr int n3 = fn7 ((const int *) 0, 8); // { dg-error "is not a constant expression" }
+constexpr int n3 = fn7 ((const int *) 0, 8);
 
 constexpr int
 fn8 (int i)

Re: [PATCH 5/9] regrename: Don't run if function was separately shrink-wrapped

[Bernd Schmidt]

On 09/14/2016 03:55 PM, Segher Boessenkool wrote:

On Wed, Sep 14, 2016 at 03:08:21PM +0200, Bernd Schmidt wrote:

The data that was posted showed a code size decrease on a number of
targets. I'm really not sure where this irrational hate for regrename
comes from.


It increases the number of active, "young" registers per thread.

There is no irrational hate.  Regrename is simply a de-optimisation on
some (heavily) out-of-order targets.


Can you point me at a processor manual for such a chip that explains why 
this would be the case?



(It also makes the generated code much harder to read, but you know that).


Can't say I do, really. I imagine it could be the case if it enables 
more aggressive scheduling but that's kind of one of the intended effects.



Bernd

Re: [PR72835] Incorrect arithmetic optimization involving bitfield arguments

[Georg-Johann Lay]

On 09.08.2016 15:43, kugan wrote:

Hi,

The test-case in PR72835 is failing with -O2 and passing with
-fno-tree-reassoc. It also passes with -O2 -fno-tree-vrp.

diff of .115t.dse2 and .116t.reassoc1 for the c++ testcase is as follows, which
looks OK.

+  unsigned int _16;
+  unsigned int _17;
+  unsigned int _18;

   :
   _1 = s1.m2;
   _2 = (unsigned int) _1;
   _3 = s1.m3;
   _4 = (unsigned int) _3;
-  _5 = -_4;
-  _6 = _2 * _5;
+  _5 = _4;
+  _6 = _5 * _2;
   var_32.0_7 = var_32;
   _8 = (unsigned int) var_32.0_7;
   _9 = s1.m1;
   _10 = (unsigned int) _9;
-  _11 = -_10;
-  _12 = _8 * _11;
-  c_14 = _6 + _12;
+  _11 = _10;
+  _12 = _11 * _8;
+  _16 = _12 + _6;
+  _18 = _16;
+  _17 = -_18;
+  c_14 = _17;
   if (c_14 != 4098873984)


However, I noticed that when we re-associate and assign different operands to
the stmts, we are not resetting the flow information to the LHS. This looks
wrong. Attached patch resets it. With this, the testcases in TH PR is now 
working.


Bootstrap and regression testing for x86_64-linux-gnu is in progress. Is this
OK for trunk if there is no regression.

Thanks,
Kugan

gcc/testsuite/ChangeLog:

2016-08-09  Kugan Vivekanandarajah  

PR tree-optimization/72835
* gcc.dg/torture/pr72835.c: New test.

gcc/ChangeLog:

2016-08-09  Kugan Vivekanandarajah  

PR tree-optimization/72835
* tree-ssa-reassoc.c (rewrite_expr_tree): Reset value_range of LHS when
operands are changed.
(rewrite_expr_tree_parallel): Likewise.

Hi,

The test-case in PR72835 is failing with -O2 and passing with 
-fno-tree-reassoc. It also passes with -O2 -fno-tree-vrp.

diff of .115t.dse2 and .116t.reassoc1 for the c++ testcase is as follows, which 
looks OK.

+  unsigned int _16;
+  unsigned int _17;
+  unsigned int _18;

   :
   _1 = s1.m2;
   _2 = (unsigned int) _1;
   _3 = s1.m3;
   _4 = (unsigned int) _3;
-  _5 = -_4;
-  _6 = _2 * _5;
+  _5 = _4;
+  _6 = _5 * _2;
   var_32.0_7 = var_32;
   _8 = (unsigned int) var_32.0_7;
   _9 = s1.m1;
   _10 = (unsigned int) _9;
-  _11 = -_10;
-  _12 = _8 * _11;
-  c_14 = _6 + _12;
+  _11 = _10;
+  _12 = _11 * _8;
+  _16 = _12 + _6;
+  _18 = _16;
+  _17 = -_18;
+  c_14 = _17;
   if (c_14 != 4098873984)


However, I noticed that when we re-associate and assign different operands to 
the stmts, we are not resetting the flow information to the LHS. This looks 
wrong. Attached patch resets it. With this, the testcases in TH PR is now 
working.


Bootstrap and regression testing for x86_64-linux-gnu is in progress. Is this 
OK for trunk if there is no regression.

Thanks,
Kugan

gcc/testsuite/ChangeLog:

2016-08-09  Kugan Vivekanandarajah  

PR tree-optimization/72835
* gcc.dg/torture/pr72835.c: New test.

gcc/ChangeLog:

2016-08-09  Kugan Vivekanandarajah  

PR tree-optimization/72835
* tree-ssa-reassoc.c (rewrite_expr_tree): Reset value_range of LHS when
operands are changed.
(rewrite_expr_tree_parallel): Likewise.

pr72835.txt

diff --git a/gcc/testsuite/gcc.dg/torture/pr72835.c 
b/gcc/testsuite/gcc.dg/torture/pr72835.c
index e69de29..d7d0a8d 100644
--- a/gcc/testsuite/gcc.dg/torture/pr72835.c
+++ b/gcc/testsuite/gcc.dg/torture/pr72835.c
@@ -0,0 +1,36 @@
+/* PR tree-optimization/72835 */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+struct struct_1 {
+unsigned int m1 : 6 ;
+unsigned int m2 : 24 ;


This test needs dg-require effective target int32plus because it will break on 
int=16bit targets.



+unsigned int m3 : 6 ;
+};
+
+unsigned short var_32 = 0x2d10;
+
+struct struct_1 s1;
+
+void init ()
+{
+  s1.m1 = 4;
+  s1.m2 = 0x7ca4b8;
+  s1.m3 = 24;
+}
+
+void foo ()
+{
+  unsigned int c =
+((unsigned int) s1.m2) * (-((unsigned int) s1.m3))
++ (var_32) * (-((unsigned int) (s1.m1)));
+  if (c != 4098873984)
+__builtin_abort ();
+}
+
+int main ()
+{
+init ();
+foo ();
+return 0;
+}

Re: [PATCH 5/9] regrename: Don't run if function was separately shrink-wrapped

[Segher Boessenkool]

On Wed, Sep 14, 2016 at 03:08:21PM +0200, Bernd Schmidt wrote:
> On 09/14/2016 03:04 PM, Segher Boessenkool wrote:
> >Then rs6000 (and many other ports probably) will just turn it off again.
> >It is actively harmful.
> 
> The data that was posted showed a code size decrease on a number of 
> targets. I'm really not sure where this irrational hate for regrename 
> comes from.

It increases the number of active, "young" registers per thread.

There is no irrational hate.  Regrename is simply a de-optimisation on
some (heavily) out-of-order targets.  Not by much, but not a win either.
We would rather do without it, on current CPUs at least (and I doubt this
will change soon, but we'll see).

(It also makes the generated code much harder to read, but you know that).


Segher

Re: [PATCH 1/3] Put a TARGET_LRA_P into every target

[Segher Boessenkool]

On Wed, Sep 14, 2016 at 07:46:13AM -0500, Peter Bergner wrote:
> On 9/14/16 5:35 AM, Segher Boessenkool wrote:
> > (I hope the wording is strong enough).

> Maybe s/New ports should use LRA/New ports must use LRA/ ?

Yeah maybe.  Does anyone else have an opinion on this?  Cc:ing gcc@...

> >+ New ports  should use LRA, and existing ports are encouraged to convert.
>  ^^
>  extra space

That is auto-generated, and not new :-)  The various info backends do
not seem to care (TeX doesn't, do any others?)


Segher

Re: [PATCH] Optimize strchr (s, 0) to strlen

[Jakub Jelinek]

On Wed, Sep 14, 2016 at 03:41:33PM +0200, Richard Biener wrote:
> > We've seen several different proposals for where/how to do this 
> > simplification, why did you
> > say strlenopt is best? It would be an unconditional strchr (a, 0) -> a + 
> > strlen (a) rewrite,
> > ie. completely unrelated to what strlenopt does. We do all the other 
> > simplifications based
> > on constant arguments in builtins.c and gimple-fold.c, why is strchr (s, 0) 
> > different?
> 
> I was thinking about the case where strlen opt already knows strlen
> (a).  But sure, gimple-fold.c
> works as well.

I think for the middle-end, using strchr (a, 0) as canonical instead of a + 
strlen (a)
is better, and at expansion time we can decide what to use (a + strlen (a)
if you'd expand strlen inline, rather than as a function call, or
__rawmemchr (which if libc is sane should be fastest), or strchr, or a + strlen 
(a)).

Jakub

Re: [PATCH] Optimize strchr (s, 0) to strlen

[Richard Biener]

On Tue, Sep 13, 2016 at 2:36 PM, Wilco Dijkstra  wrote:
> Richard Biener  wrote:
>> On Wed, May 18, 2016 at 2:29 PM, Wilco Dijkstra  
>> wrote:
>>> Richard Biener wrote:
>>>
 Yeah ;)  I'm currently bootstrapping/testing the patch that makes it 
 possible to
 write all this in match.pd.
>>>
>>> So what was the conclusion? Improving match.pd to be able to handle more 
>>> cases
>>> like this seems like a nice thing.
>>
>> I'm stuck with fallout and making this work requires some serious
>> thought.  Don't
>> hold your breath here :/
>>
>> The restricted case of strchr (a, 0) -> strlen () can be made working
>> more easily
>> but I didn't yet try to implement a restriction only allowing the
>> cases that would work.
>>
>> Meanwhile the strlenopt pass would be an appropriate place to handle
>> this transform
>> (well, if we now agree on its usefulness).
>
> I'd like to pick this up again so we can make GCC7 a bit less inefficient for 
> this case.
> (original thread: https://gcc.gnu.org/ml/gcc-patches/2016-04/msg00870.html)
>
> We've seen several different proposals for where/how to do this 
> simplification, why did you
> say strlenopt is best? It would be an unconditional strchr (a, 0) -> a + 
> strlen (a) rewrite,
> ie. completely unrelated to what strlenopt does. We do all the other 
> simplifications based
> on constant arguments in builtins.c and gimple-fold.c, why is strchr (s, 0) 
> different?

I was thinking about the case where strlen opt already knows strlen
(a).  But sure, gimple-fold.c
works as well.

Richard.

> Wilco
>
>

Re: [PATCH 8/9] shrink-wrap: shrink-wrapping for separate components

[Segher Boessenkool]

On Mon, Sep 12, 2016 at 12:02:50PM -0600, Jeff Law wrote:
> >>>As a final optimisation, if a block needs a prologue and its immediate
> >>>dominator has the block as a post-dominator, the dominator gets the
> >>>prologue as well.
> >>So why not just put it in the idom and not in the dominated block?
> >
> >That's what it does :-)
> Then I must have mis-parsed.  Thanks for clarifying.

"As a final optimisation, if a block needs a prologue and its immediate
dominator has the block as a post-dominator, ***that immediate dominator***
gets the prologue as well."

That is clearer I hope :-)

> Hmm, then explain again why DCE is mucking up?  I don't immediately see 
> how EPILOGUE_BEG notes come into play with DCE.  It seems to rely on the 
> DF data and AFAICT DF only cares about the EPILOGUE_BEG note in 
> can_move_insns_across which shouldn't be used by DCE.

The register restore *is* dead code, but we need to have the same CFI
for all convergent paths.

> >>Consider using auto_sbitmap rather than manually managing
> >>allocation/releasing of the per-block structures.  In fact, can't all of
> >>SW become a class and we lose the explicit init/fini routines in favor
> >>of a ctor/dtor?
> >
> >Yes, you can always add indirection.  I do not think the code becomes
> >more readable that way (quite the opposite).  Explicit is *good*.
> The GCC project is moving away from this kind of explicit 
> allocation/deallocation and more towards a RAII.  Unless there is a 
> clear need for the explicit allocation/deallocation, please put this 
> stuff into a class with an appropriate ctor/dtor.
> 
> FWIW, I was a big opponent of how much stuff happens "behind your back" 
> with some languages (including C++).  But over the last few years my 
> personal stance has softened considerably after seeing how cleanly RAII 
> solves certain problems.

We then still cannot get rid of SW, which is a convenience macro to do
a nasty cast on bb->aux.  If bb->aux was some pretty class hierarchy,
easy to use and all that, I would of course agree with your suggestion.
But as it is it is just a bare pointer, so the less we hide the safer
it is.

> >>For the PPC R0 vs LR is the only thing that causes disqualification
> >>right?
> >
> >Currently, yes.
> >
> >>Can't that be handled when we build the set of components we
> >>want to insert for each edge/block?  Is there some advantage to handling
> >>disqualifications after all the potential insertion points have been
> >>handled?
> >
> >We do not know if an edge needs a prologue, epilogue, or neither, until
> >we have decided whether *both* ends of that edge want the component active
> >or not.
> Right.  Hmm, maybe I'm not asking the question clearly.
> 
> Whether or not an edge needs a prologue or epilogue is a function not 
> just of the state at the head or tail of the edge, but instead is a 
> function of global dataflow propagation?  Thus we can't disqualify until 
> after we've done the dataflow propagation?  Right?

We can figure out before we decide what blocks need what components, what
edges can not get a prologue or epilogue for which components.  This
complicates the selection algorithm a whole lot, for not much gain that
I have seen so far, so I just give up in the cases that end up "bad".

It is not easy at all to see what edges will need to get a *logue,
because not always both blocks that edge connects are in the same
dominator subtree (or tree even, for an epilogue-aware placement
algorithm, but this patch doesn't do that yet; it's a more minor
optimisation, only reduces code size a little).


Segher

Re: [libstdc++-v3] Fix dg-require before dg-run directives in testsuite.

[Christophe Lyon]

On 14 September 2016 at 15:27, Jonathan Wakely  wrote:
> On 14/09/16 15:17 +0200, Christophe Lyon wrote:
>>
>> It seems some tests still have dg-require or dg-skip before dg-do.
>> The attached patch fixes that, the only effect is that
>> 25_algorithms/lower_bound/debug/irreflexive.cc
>> is now UNSUPPORTED instead of PASS on arm* and aarch64* targets.
>>
>> I'm not sure if that's expected. I do see: "#error No debug mode" in the
>> logs.
>
>
> It's expected, because it has dg-require-debug-mode, so the test
> should only run when doing make check-debug, or adding
> -D_GLIBCXX_DEBUG to the options used for testing.
>
> It passes in normal mode too, but doesn't test anything very
> interesting, so doesn't need to be tested.
>
Thanks for the explanation.
Committed as r240135.

Christophe

>> OK for trunk?
>
>
> Yes, thanks.
>
>

Re: [patch] [gsoc] [gimplefe] GIMPLE FE Project

[Richard Biener]

On Wed, Sep 14, 2016 at 3:28 PM, Marek Polacek  wrote:
> On Wed, Sep 14, 2016 at 03:24:18PM +0200, Richard Biener wrote:
>> > PING.
>> >
>> > https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01837.html
>>
>> (that was a ping for the C FE maintainers)
>>
>> Prasad, can you update the git branch with the changes from the last patch 
>> you
>> sent out?  I don't think you need to keep it unchanged from this point on.
>
> So where exactly is the latest patch/branch?  I see the gimple-front-end
> branch, but that hasn't been updated for 2 years.

Latest patch was posted here:
https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01830.html

and the git (not up to date with that patch) is at
https://github.com/PrasadG193/gcc_gimple_fe.git

Richard.

> Marek

Re: [patch] [gsoc] [gimplefe] GIMPLE FE Project

[Marek Polacek]

On Wed, Sep 14, 2016 at 03:24:18PM +0200, Richard Biener wrote:
> > PING.
> >
> > https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01837.html
> 
> (that was a ping for the C FE maintainers)
> 
> Prasad, can you update the git branch with the changes from the last patch you
> sent out?  I don't think you need to keep it unchanged from this point on.

So where exactly is the latest patch/branch?  I see the gimple-front-end
branch, but that hasn't been updated for 2 years.

Marek

Re: [libstdc++-v3] Fix dg-require before dg-run directives in testsuite.

[Jonathan Wakely]

On 14/09/16 15:17 +0200, Christophe Lyon wrote:

It seems some tests still have dg-require or dg-skip before dg-do.
The attached patch fixes that, the only effect is that
25_algorithms/lower_bound/debug/irreflexive.cc
is now UNSUPPORTED instead of PASS on arm* and aarch64* targets.

I'm not sure if that's expected. I do see: "#error No debug mode" in the logs.


It's expected, because it has dg-require-debug-mode, so the test
should only run when doing make check-debug, or adding
-D_GLIBCXX_DEBUG to the options used for testing.

It passes in normal mode too, but doesn't test anything very
interesting, so doesn't need to be tested.


OK for trunk?


Yes, thanks.

Re: [patch] [gsoc] [gimplefe] GIMPLE FE Project

[Richard Biener]

On Fri, Sep 9, 2016 at 12:38 PM, Prasad Ghangal
 wrote:
> On 26 August 2016 at 14:28, Richard Biener  wrote:
>> On Fri, Aug 26, 2016 at 5:08 AM, Prasad Ghangal
>>  wrote:
>>> On 24 August 2016 at 15:32, Richard Biener  
>>> wrote:
 On Mon, Aug 22, 2016 at 8:40 PM, Prasad Ghangal
  wrote:
> On 22 August 2016 at 16:55, Trevor Saunders  wrote:
>> On Sun, Aug 21, 2016 at 10:35:17PM +0530, Prasad Ghangal wrote:
>>> Hi all,
>>>
>>> As a part of my gsoc project. I have completed the following tasks:
>>>
>>> * Parsed gimple-expression
>>> * Parsed gimple-labels
>>> * Parsed local declaration
>>> * Parsed gimple-goto statement
>>> * Parsed gimple-if-else statement
>>> * Parsed gimple-switch statement
>>> * Parsed gimple-return statement
>>> * Parsed gimple-PHI function
>>> * Parsed gimple ssa-names along with default def
>>> * Parsed gimple-call
>>>
>>> * Hacked pass manager to add support for startwith (pass-name) to skip
>>> early opt passes
>>> * Modified gimple dump for making it parsable
>>>
>>> I am willing to continue work on the project, some TODOs for the 
>>> projects are:
>>>
>>> * Error handling
>>> * Parse more gimple syntax
>>> * Add startwith support for IPA passes
>>>
>>> The complete code of gimple fe project can be found at
>>> https://github.com/PrasadG193/gcc_gimple_fe
>>>
>>>
>>> PFA patch for complete project (rebased for latest trunk revision).
>>> I have successfully bootstrapped and tested on x86_64-pc-linux-gnu.
>>> Some testcases failed due to modified gimple dump as expected.
>>>
>>>
>>> Thanks,
>>> Prasad
>>
>> only some rather minor comments
>>
>>
>> +++ b/gcc/c/c-parser.c
>> @@ -59,6 +59,18 @@ along with GCC; see the file COPYING3.  If not see
>>  #include "gimple-expr.h"
>>  #include "context.h"
>>  #include "gcc-rich-location.h"
>> +#include "tree-vrp.h"
>>
>> given that you need these headers it might be better to put most of the
>> gimple parsing in its own file so only what actually needs to know about
>> this part of the compiler does now about it.
>>
>> +void
>> +c_parser_parse_gimple_body (c_parser *parser)
>> +{
>> +  bool return_p = false;
>> +  gimple_seq seq;
>> +  gimple_seq body;
>> +  tree stmt = push_stmt_list ();
>>
>> it would be nice to move the declarations down to their first use.
>>
>> +  gimple *ret;
>> +  ret = gimple_build_return (NULL);
>>
>> there's no reason for a separate declaration and assignment ;)
>>
>> +  tree block = NULL;
>> +  block = pop_scope ();
>>
>> same here, and a number of other places.
>>
>> +c_parser_gimple_compound_statement (c_parser *parser, gimple_seq *seq)
>> +{
>> +  bool return_p = false;
>> +
>> +  if (!c_parser_require (parser, CPP_OPEN_BRACE, "expected %<{%>"))
>> +  return return_p;
>>
>> return false would work fine.
>>
>> +
>> +  if (c_parser_next_token_is (parser, CPP_CLOSE_BRACE))
>> +{
>> +  c_parser_consume_token (parser);
>> +  goto out;
>>
>> I don't see the need for the gotos, there's no cleanup in this function.
>>
>> +  /* gimple PHI expression.  */
>> +  if (c_parser_next_token_is_keyword (parser, RID_PHI))
>> +{
>> +  c_parser_consume_token (parser);
>> +
>> +  if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
>> +   {
>> + return;
>> +   }
>> +
>> +  gcall *call_stmt;
>> +  tree arg = NULL_TREE;
>> +  vec vargs = vNULL;
>>
>> I think you can use auto_vec here, as is I think this leaks the vectors
>> storage.
>>
>> +c_parser_gimple_binary_expression (c_parser *parser, enum tree_code 
>> *subcode)
>>
>> you can skip the explicit 'enum' keyword.
>>
>> +  struct {
>> +/* The expression at this stack level.  */
>> +struct c_expr expr;
>>
>> similar with struct here.
>>
>> +/* The precedence of the operator on its left, PREC_NONE at the
>> +   bottom of the stack.  */
>> +enum c_parser_prec prec;
>> +/* The operation on its left.  */
>> +enum tree_code op;
>> +/* The source location of this operation.  */
>> +location_t loc;
>> +  } stack[2];
>> +  int sp;
>> +  /* Location of the binary operator.  */
>> +  location_t binary_loc = UNKNOWN_LOCATION;  /* Quiet warning.  */
>> +#define POP 
>>  \
>>
>> it seems like it would be nicer to name the type, and 

Re: [PATCH 6/9] sel-sched: Don't mess with register restores

[Segher Boessenkool]

On Mon, Sep 12, 2016 at 11:24:01AM -0600, Jeff Law wrote:
> >sel-sched-ir.c says
> >
> >/* Certain instructions cannot be cloned, and frame related insns and
> >   the insn adjacent to NOTE_INSN_EPILOGUE_BEG cannot be moved out of
> >   their block.  */
> >if (prologue_epilogue_contains (insn))
> >
> >...
> >
> >and I'm just extending that to "epilogue" instructions not in the
> >"epilogue" ;-)
> >
> >If all such epilogue instructions always had a REG_CFA_RESTORE note,
> >we could drop the "old" thing; but even instructions restoring a register
> >do not always have such a note (they can be batched up, and they can be
> >not emitted at all if not shrink-wrapping).
> Can you fix this by registering the separate prologue/epilogue insns in 
> prologue_insn_hash and epilogue_insn_hash or does that have unintended 
> consequences?

An interesting plan, I'll try.


Segher

C/C++ PATCH to change {cp,}build_unary_op's parameter

[Marek Polacek]

I promised I'd do the int -> bool conversion for build_unary_op (and so
even for cp_build_unary_op), so here it is.

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

2016-09-14  Marek Polacek  

* c-common.c (c_common_truthvalue_conversion): Use false instead of 0.
* c-common.h (build_unary_op): Change a parameter type to bool.
* c-omp.c (c_finish_omp_atomic): Use false instead of 0.

* c-array-notation.c (create_cmp_incr): Use false instead of 0.
(fix_array_notation_expr): Likewise.
* c-decl.c (finish_decl): Likewise.
* c-parser.c (c_parser_postfix_expression_after_primary): Likewise.
* c-typeck.c (array_to_pointer_conversion): Use true instead of 1.
(function_to_pointer_conversion): Use false instead of 0.
(convert_lvalue_to_rvalue): Likewise.
(parser_build_unary_op): Likewise.
(build_atomic_assign): Likewise.
(build_unary_op): Change a parameter type to bool, use true/false
instead of 1/0.
(build_binary_op): Use true instead of 1.

* cp-tree.h (cp_build_unary_op): Change a parameter type to bool.
* decl2.c (one_static_initialization_or_destruction): Use true instead
of 1.
* init.c (build_vec_init): Use false instead of 0.
* pt.c (tsubst_copy_and_build): Likewise.
* semantics.c (simplify_loop_decl_cond): Likewise.
* typeck.c (rationalize_conditional_expr): Likewise.
(cp_build_binary_op): Use true instead of 1.
(cp_build_unary_op): Change a parameter type to bool.  Use true instead
of 1.
(build_unary_op): Change a parameter type to bool.
(unary_complex_lvalue): Use false instead of 0.

diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index 1132a03..b561f9f 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -4599,7 +4599,7 @@ c_common_truthvalue_conversion (location_t location, tree 
expr)
 : truthvalue_false_node;
 
 case FUNCTION_DECL:
-  expr = build_unary_op (location, ADDR_EXPR, expr, 0);
+  expr = build_unary_op (location, ADDR_EXPR, expr, false);
   /* Fall through.  */
 
 case ADDR_EXPR:
@@ -4739,10 +4739,10 @@ c_common_truthvalue_conversion (location_t location, 
tree expr)
? TRUTH_OR_EXPR : TRUTH_ORIF_EXPR),
c_common_truthvalue_conversion
   (location,
-   build_unary_op (location, REALPART_EXPR, t, 0)),
+   build_unary_op (location, REALPART_EXPR, t, false)),
c_common_truthvalue_conversion
   (location,
-   build_unary_op (location, IMAGPART_EXPR, t, 0)),
+   build_unary_op (location, IMAGPART_EXPR, t, false)),
   0));
   goto ret;
 }
diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h
index 2e211c4..5bbf951 100644
--- gcc/c-family/c-common.h
+++ gcc/c-family/c-common.h
@@ -964,7 +964,7 @@ extern tree build_real_imag_expr (location_t, enum 
tree_code, tree);
 /* These functions must be defined by each front-end which implements
a variant of the C language.  They are used in c-common.c.  */
 
-extern tree build_unary_op (location_t, enum tree_code, tree, int);
+extern tree build_unary_op (location_t, enum tree_code, tree, bool);
 extern tree build_binary_op (location_t, enum tree_code, tree, tree, int);
 extern tree perform_integral_promotions (tree);
 
diff --git gcc/c-family/c-omp.c gcc/c-family/c-omp.c
index 3b131ed..5ccb62e 100644
--- gcc/c-family/c-omp.c
+++ gcc/c-family/c-omp.c
@@ -212,7 +212,7 @@ c_finish_omp_atomic (location_t loc, enum tree_code code,
 
   /* Take and save the address of the lhs.  From then on we'll reference it
  via indirection.  */
-  addr = build_unary_op (loc, ADDR_EXPR, lhs, 0);
+  addr = build_unary_op (loc, ADDR_EXPR, lhs, false);
   if (addr == error_mark_node)
 return error_mark_node;
   if (!test)
@@ -303,14 +303,14 @@ c_finish_omp_atomic (location_t loc, enum tree_code code,
 loc, x, NULL_TREE);
   if (rhs1 && rhs1 != lhs)
{
- tree rhs1addr = build_unary_op (loc, ADDR_EXPR, rhs1, 0);
+ tree rhs1addr = build_unary_op (loc, ADDR_EXPR, rhs1, false);
  if (rhs1addr == error_mark_node)
return error_mark_node;
  x = omit_one_operand_loc (loc, type, x, rhs1addr);
}
   if (lhs1 && lhs1 != lhs)
{
- tree lhs1addr = build_unary_op (loc, ADDR_EXPR, lhs1, 0);
+ tree lhs1addr = build_unary_op (loc, ADDR_EXPR, lhs1, false);
  if (lhs1addr == error_mark_node)
return error_mark_node;
  if (code == OMP_ATOMIC_CAPTURE_OLD)
@@ -325,7 +325,7 @@ c_finish_omp_atomic (location_t loc, enum tree_code code,
 }
   else if (rhs1 && rhs1 != lhs)
 {
-  tree rhs1addr = build_unary_op (loc, ADDR_EXPR, rhs1, 0);
+  tree rhs1addr = build_unary_op (loc, ADDR_EXPR, rhs1, false);
  

Re: [PATCH GCC 6/9]Simplify control flow graph for vectorized loop

[Richard Biener]

On Tue, Sep 6, 2016 at 8:52 PM, Bin Cheng  wrote:
> Hi,
> This is the main patch improving control flow graph for vectorized loop.  It 
> generally rewrites loop peeling stuff in vectorizer.  As described in patch, 
> for a typical loop to be vectorized like:
>
>preheader:
>  LOOP:
>header_bb:
>  loop_body
>  if (exit_loop_cond) goto exit_bb
>  elsegoto header_bb
>exit_bb:
>
> This patch peels prolog and epilog from the loop, adds guards skipping PROLOG 
> and EPILOG for various conditions.  As a result, the changed CFG would look 
> like:
>
>guard_bb_1:
>  if (prefer_scalar_loop) goto merge_bb_1
>  elsegoto guard_bb_2
>
>guard_bb_2:
>  if (skip_prolog) goto merge_bb_2
>  else goto prolog_preheader
>
>prolog_preheader:
>  PROLOG:
>prolog_header_bb:
>  prolog_body
>  if (exit_prolog_cond) goto prolog_exit_bb
>  else  goto prolog_header_bb
>prolog_exit_bb:
>
>merge_bb_2:
>
>vector_preheader:
>  VECTOR LOOP:
>vector_header_bb:
>  vector_body
>  if (exit_vector_cond) goto vector_exit_bb
>  else  goto vector_header_bb
>vector_exit_bb:
>
>guard_bb_3:
>  if (skip_epilog) goto merge_bb_3
>  else goto epilog_preheader
>
>merge_bb_1:
>
>epilog_preheader:
>  EPILOG:
>epilog_header_bb:
>  epilog_body
>  if (exit_epilog_cond) goto merge_bb_3
>  else  goto epilog_header_bb
>
>merge_bb_3:
>
>
> Note this patch peels prolog and epilog only if it's necessary, as well as 
> adds different guard_conditions/branches.  Also the first guard/branch could 
> be further improved by merging it with loop versioning.
>
> Before this patch, up to 4 branch instructions need to be executed before the 
> vectorized loop is reached in the worst case, while the number is reduced to 
> 2 with this patch.  The patch also does better in compile time analysis to 
> avoid unnecessary peeling/branching.
> From implementation's point of view, vectorizer needs to update induction 
> variables and iteration bounds along with control flow changes.  
> Unfortunately, it also becomes much harder to follow because slpeel_* 
> functions updates SSA by itself, rather than using update_ssa interface.  
> This patch tries to factor out SSA/IV/Niter_bound changes from CFG changes.  
> This should make the implementation easier to read, and I think it maybe a 
> step forward to replace slpeel_* functions with generic GIMPLE loop copy 
> interfaces as Richard suggested.

I've skimmed over the patch and it looks reasonable to me.

Ok.

Thanks,
Richard.


> Thanks,
> bin
>
> 2016-09-01  Bin Cheng  
>
> * tree-vect-loop-manip.c (adjust_vec_debug_stmts): Don't release
> adjust_vec automatically.
> (slpeel_add_loop_guard): Remove param cond_expr_stmt_list.  Rename
> param exit_bb to guard_to.
> (slpeel_checking_verify_cfg_after_peeling):
> (set_prologue_iterations):
> (create_lcssa_for_virtual_phi): New func which is factored out from
> slpeel_tree_peel_loop_to_edge.
> (slpeel_tree_peel_loop_to_edge):
> (iv_phi_p): New func.
> (vect_can_advance_ivs_p): Call iv_phi_p.
> (vect_update_ivs_after_vectorizer): Call iv_phi_p.  Directly insert
> new gimple stmts in basic block.
> (vect_do_peeling_for_loop_bound):
> (vect_do_peeling_for_alignment):
> (vect_gen_niters_for_prolog_loop): Rename to...
> (vect_gen_prolog_loop_niters): ...Rename from.  Change parameters and
> adjust implementation.
> (vect_update_inits_of_drs): Fix code style issue.  Convert niters to
> sizetype if necessary.
> (vect_build_loop_niters): Move to here from tree-vect-loop.c.  Change
> it to external function.
> (vect_gen_scalar_loop_niters, vect_gen_vector_loop_niters): New.
> (vect_gen_vector_loop_niters_mult_vf): New.
> (slpeel_update_phi_nodes_for_loops): New.
> (slpeel_update_phi_nodes_for_guard1): Reimplement.
> (find_guard_arg, slpeel_update_phi_nodes_for_guard2): Reimplement.
> (slpeel_update_phi_nodes_for_lcssa, vect_do_peeling): New.
> * tree-vect-loop.c (vect_build_loop_niters): Move to file
> tree-vect-loop-manip.c
> (vect_generate_tmps_on_preheader): Delete.
> (vect_transform_loop): Rename vectorization_factor to vf.  Call
> vect_do_peeling instead of vect_do_peeling-* functions.
> * tree-vectorizer.h (vect_do_peeling): New decl.
> (vect_build_loop_niters, vect_gen_vector_loop_niters): New decls.
> (vect_do_peeling_for_loop_bound): Delete.
> 

Re: [PATCH v2 0/9] Separate shrink-wrapping

[Segher Boessenkool]

On Mon, Sep 12, 2016 at 10:49:46AM -0600, Jeff Law wrote:
> On 09/09/2016 02:56 PM, Segher Boessenkool wrote:
> >On Fri, Sep 09, 2016 at 12:57:32PM -0600, Jeff Law wrote:
> >>I think the lack of test coverage is something we'll want to address.
> >
> >Building and running the compiler, the various target libraries, and the
> >testsuite is more than enough coverage for correctness in my opinion --
> >I cannot make up anything that is not already in there.  No doubt some
> >PR will show up later, and we can (and should) of course add that then.
> >
> >For checking whether it makes "smart" decisions, I'll make some really
> >simple testcases that will not fail for random reasons all the time.
> I'm moving more and more towards wanting tests for new features. 
> There's no reason why verification of basic behavior shouldn't be part 
> of the feature patch itself.

Yes, but as I said, this is used millions of times during bootstrap
already, and at least *I* cannot think of more complex structures that
would fail.  If any show up we can add them as regression tests.

> I suspect the biggest problem with testing something like separate 
> shrink wrapping is making sure the test is stable over time -- and 
> that's a legitimate concern.

Yes exactly.

> But it ought to be do-able in one form or another,

Yes, as I promised, I'll make up some trivial tests.

> particularly since we have control over the debug dumps. 

I'm not a big fan of using the debug dumps in the testsuite -- very
often tests break because you changed something in the dump.  Debug
dumps are for debug, not for testing.

If we get to the point where we generically can use some piece of RTL
as input and just run one pass on it, that would be nice.  But we're
not there yet, and the kind of "unit tests" where you just write all
code twice and essentially only test if the test is correct: no thanks.


Segher

Re: [libstdc++-v3] Fix dg-require before dg-run directives in testsuite.

[Christophe Lyon]

On 6 September 2016 at 12:15, Jonathan Wakely  wrote:
> On 02/09/16 14:17 +0100, Matthew Wahab wrote:
>>
>> Hello,
>>
>> Tests in the libstdc++-v3 testsuite were recently changed to use { dg-do
>> .. { target c++11 } } instead of using a dg-options to set -std
>> (https://gcc.gnu.org/ml/libstdc++/2016-08/msg00102.html). As a
>> consequence, several tests were left with directive lists that had
>> dg-require preceding dg-do. This meant that some tests were run when
>> they should have been skipped as unsupported. In particular, most of the
>> 23_algorithms/*/complexity.cc tests became FAIL on aarch64-none-elf
>>
>> This patch adjusts the tests so that the dg-requires come after the
>> dg-do directives. The makes the tests that were previously
>> FAIL/UNRESOLVED become UNSUPPORTED. It also makes UNSUPPORTED some tests
>> that were PASS.
>>
>> The tests that move from FAIL/UNRESOLVED to UNSUPPORTED:
>>
>> - 22_locale/locale/cons/unicode.cc.
>> - 25_algorithms/{pop_heap,push_heap,sort_heap}/complexity.c.
>>
>> From PASS to UNSUPPORTED:
>>
>> - 23_containers/*/debug/60499.c
>> - 23_containers/vector/debug/52433.c
>>
>> Tested by running the testsuite for cross-compiled aarch64-none-elf.
>>
>> Ok for trunk?
>
>
> Yes, thanks for cleaning this up.
>
>
Hello,

It seems some tests still have dg-require or dg-skip before dg-do.
The attached patch fixes that, the only effect is that
25_algorithms/lower_bound/debug/irreflexive.cc
is now UNSUPPORTED instead of PASS on arm* and aarch64* targets.

I'm not sure if that's expected. I do see: "#error No debug mode" in the logs.

OK for trunk?

Thanks,

Christophe
libstdc++-v3/ChangeLog:

2016-09-14  Christophe Lyon  

* testsuite/23_containers/vector/bool/modifiers/insert/31370.cc:
Move dg-do directive before dg-skip.
* testsuite/21_strings/debug/iterator_self_move_assign_neg.cc:
Move dg-do directive before dg-require.
* testsuite/21_strings/debug/self_move_assign_neg.cc: Likewise.
* testsuite/23_containers/vector/debug/57779_neg.cc: Likewise.
* testsuite/23_containers/vector/debug/60587_neg.cc: Likewise.
* testsuite/23_containers/vector/debug/assign1_neg.cc: Likewise.
* testsuite/23_containers/vector/debug/assign2_neg.cc: Likewise.
* testsuite/23_containers/vector/debug/assign3_neg.cc: Likewise.
* testsuite/23_containers/vector/debug/construct1_neg.cc: Likewise.
* testsuite/23_containers/vector/debug/construct2_neg.cc: Likewise.
* testsuite/23_containers/vector/debug/construct3_neg.cc: Likewise.
* testsuite/23_containers/vector/debug/insert1_neg.cc: Likewise.
* testsuite/23_containers/vector/debug/insert2_neg.cc: Likewise.
* testsuite/23_containers/vector/debug/insert3_neg.cc: Likewise.
* testsuite/23_containers/vector/debug/insert5_neg.cc: Likewise.
* testsuite/25_algorithms/lower_bound/debug/irreflexive.cc: Likewise.
* testsuite/25_algorithms/partial_sort_copy/debug/irreflexive_neg.cc:
Likewise.
* testsuite/25_algorithms/pop_heap/empty2_neg.cc: Likewise.
* testsuite/25_algorithms/pop_heap/empty_neg.cc: Likewise.
* testsuite/27_io/objects/char/12048-5.cc: Likewise.
* testsuite/ext/special_functions/conf_hyperg/check_nan.cc: Likewise.
* testsuite/ext/special_functions/hyperg/check_nan.cc: Likewise.

gcc/testsuite/ChangeLog:

2016-09-14  Christophe Lyon  

* g++.dg/cpp0x/lambda/lambda-mangle.C: Move dg-do directive before
dg-require.
* g++.dg/ext/builtin_alloca.C: Likewise.
* g++.dg/template/spec35.C: Likewise.
* gcc.dg/builtins-68.c: Likewise.
* gcc.misc-tests/gcov-13.c: Likewise.
* gcc.misc-tests/gcov-14.c: Likewise.
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle.C 
b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle.C
index 20ef282..15b8b79 100644
--- a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle.C
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle.C
@@ -1,6 +1,6 @@
 // Test lambda mangling
-// { dg-require-weak "" }
 // { dg-do compile { target c++11 } }
+// { dg-require-weak "" }
 // { dg-options "-fno-inline" }
 
 template int algo(F fn) { return fn(); }
diff --git a/gcc/testsuite/g++.dg/ext/builtin_alloca.C 
b/gcc/testsuite/g++.dg/ext/builtin_alloca.C
index 7a0d331..e857502 100644
--- a/gcc/testsuite/g++.dg/ext/builtin_alloca.C
+++ b/gcc/testsuite/g++.dg/ext/builtin_alloca.C
@@ -1,7 +1,7 @@
 // PR middle-end/69780 - [4.9/5/6 Regression] ICE on
 // __builtin_alloca_with_align with small alignment
-// { dg-require-effective-target alloca }
 // { dg-do compile }
+// { dg-require-effective-target alloca }
 
 #define CHAR_BIT  __CHAR_BIT__
 #define SIZE_MAX  __SIZE_MAX__
diff --git a/gcc/testsuite/g++.dg/template/spec35.C 
b/gcc/testsuite/g++.dg/template/spec35.C
index 709ece5..e82323c 100644
--- 

Re: C++ PATCH to forbid use of bool with the ++ operator

[Marek Polacek]

On Tue, Sep 13, 2016 at 01:55:56PM -0600, Martin Sebor wrote:
> > -   /* Forbid using -- on `bool'.  */
> > +   /* Forbid using -- or ++ in C++17 on `bool'.  */
> > if (TREE_CODE (declared_type) == BOOLEAN_TYPE)
> >   {
> > if (code == POSTDECREMENT_EXPR || code == PREDECREMENT_EXPR)
> > @@ -6040,6 +6040,20 @@ cp_build_unary_op (enum tree_code code, tree xarg, 
> > int noconvert,
> >"to %");
> > return error_mark_node;
> >   }
> > +   else
> > + {
> > +   if (cxx_dialect >= cxx1z)
> > + {
> > +   if (complain & tf_error)
> > + error ("use of Boolean expression as operand "
> > +"to % is forbidden in C++1z");
> 
> The capitalization of Boolean here caught my eye because it's
> inconsistent with the recent spelling adopted in the documentation.
> (It's also missing an article "a Boolean expression," although
> dropping those is common in diagnostics. Still, it would be nice
> to have a guideline/convention and use it consistently.)
> 
> Back to Boolean, I was actually going to comment on the Boolean
> -> boolean change and suggest going in the opposite direction but
> in the end decided not to (as Sandra's links showed, there's support
> for both).
> 
> But having seen Boolean capitalized here I have changed my mind
> again. I'd like to (belatedly) speak up in support of Boolean
> (though I feel less strongly about it than I do about consistency).
 
Well, my point was to get rid of this inconsistency, I don't really
care whether it's Boolean or boolean.  But since boolean was used
most of the time, I went with that.

Marek

Re: [PATCH 5/9] regrename: Don't run if function was separately shrink-wrapped

[Bernd Schmidt]

On 09/14/2016 03:04 PM, Segher Boessenkool wrote:

Then rs6000 (and many other ports probably) will just turn it off again.
It is actively harmful.


The data that was posted showed a code size decrease on a number of 
targets. I'm really not sure where this irrational hate for regrename 
comes from.



Bernd

Re: Ping: Re: [PATCH 1/2] gcc: Remove unneeded global flag.

[Jakub Jelinek]

On Wed, Sep 14, 2016 at 02:00:48PM +0100, Andrew Burgess wrote:
> In an attempt to get this patch merged (as I still think that its
> correct) I've investigated, and documented a little more about how I
> think things currently work.  I'm sure most people reading this will
> already know this, but hopefully, if my understanding is wrong someone
> can point it out.

I wonder if user_defined_section_attribute instead shouldn't be moved
into struct function and be handled as a per-function flag then.

Jakub

Re: [PATCH 5/9] regrename: Don't run if function was separately shrink-wrapped

[Segher Boessenkool]

On Mon, Sep 12, 2016 at 10:33:22AM -0600, Jeff Law wrote:
> The answer is to debug the problem, and yes it may be painful.

Yes, the three weeks pretty much full-time I spent on that already attest
to that.

> I'd 
> probably start by fixing the dataflow issues and see if that fixes the 
> regrename thing as a side effect.

Have you seen https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00091.html ?

> I think enabling regrename by default is the right thing to do long term.

Then rs6000 (and many other ports probably) will just turn it off again.
It is actively harmful.


Segher

Re: [PATCH] Tree-level fix for PR 69526

[Richard Biener]

On Mon, Aug 22, 2016 at 4:58 PM, Robin Dapp  wrote:
>> if !inner_ovf (just set that to false if !check_inner_ovf to simplify
>> checks please).
>> you know it's valid to transform the op to (T)@0 innerop (T)@1 outerop @2
>> (if T is wider than the inner type which I think you should check and
>> which should
>> simplify things).
>
> I simplified the control flow a little and split both parts of the
> combination into separate patterns.
>
>> You can then combine (T)@1 and @2 where I think you fail to handle mixed
>> MINUS_EXPR/PLUS_EXPR (practically you'll see only PLUS_EXPRs for
> integers).
>
> Is it ok to do something like this (see patch) in order to deal with
> MINUS_EXPR and then perform a wi::add?
>
> if (inner_op == MINUS_EXPR)
> w1 = wi::neg (w1);
>
> if (outer_op == MINUS_EXPR)
> w2 = wi::neg (w2);

Yes.

>> The testcase is rather unspecific as to what testcases shoudl fold and
> what not
>> given your very simple scan and mixing should/should-not simplify
> cases.  Please
>> consider splitting it up and make it a run test that verifies the
>> bogus transforms
>> do not take place.
>
> I divided the testcases into signed/unsigned and should simplify/should
> not simplify. The bogus unsigned transforms are now checked via assert().
>
>> Doing
> [...]
>> causes such stmts to be folded twice as substitute_and_fold does
> [...]
>> which is less than ideal.  I think that given we have fold_stmt following
>> SSA edges and thus not only stmts we propagated into are possibly
>> interesting to fold (but also their single-uses, recursively), we should
>> evaluate the compile-time cost of doing the fold_stmt unconditionally.
>
> Only using update_stmt seems to avoid the double call to fold_stmt but
> is obviously the wrong thing to do as this then tries to update
> everything that matches the pattern in tree-vrp.c. Copying some checks
> from match.pd to tree-vrp.c would help but isn't there a proper way to
> prevent duplicating the checking logic?

I think what triggers it is that you return 'true', not that you call
update_stmt.

> In addition, is the compile-time cost checking something I could do for
> this patch or a general thing to be done later?

I meant to do sth like

Index: tree-ssa-propagate.c
===
--- tree-ssa-propagate.c(revision 240133)
+++ tree-ssa-propagate.c(working copy)
@@ -1105,10 +1105,10 @@ substitute_and_fold_dom_walker::before_d
   /* Replace real uses in the statement.  */
   did_replace |= replace_uses_in (stmt, get_value_fn);

-  /* If we made a replacement, fold the statement.  */
-  if (did_replace)
+  /* Fold the statement.  */
+  if (fold_stmt (, follow_single_use_edges))
{
- fold_stmt (, follow_single_use_edges);
+ did_replace = true;
  stmt = gsi_stmt (i);
}

this would need compile-time cost evaluation (and avoid the tree-vrp.c
folding part
of your patch).

>> tree.c doesn't look like the best fit.  I think putting it into
>> tree-vrp.c is better
>> and I think that extract_range_from_binary_expr_1 itself should
> compute what we
>> want here as additional output.  Conservative handling for all but
> plus/minus is
>> ok with me.
>
> I kept the extra function for now because I find
> extract_range_from_binary_expr_1 somewhat lengthy and hard to follow
> already :)

Heh, but I think duplicating code is even worse.

Just looking at the simpler pattern right now:

+  /* ((T)(A)) +- CST -> (T)(A +- CST)  */
+#if GIMPLE
+   (for outer_op (plus minus)
+(simplify
+ (outer_op (convert @0) INTEGER_CST@2)
+ /* Perform binary operation inside the cast if the constant fits
+   and there is no overflow.  */
+   (with
+   {
+ bool simplify = false;
+
+ wide_int cst = @2;
+ tree inner_type = TREE_TYPE (@0);
+ tree cst_inner = wide_int_to_tree (inner_type, cst);

What prevents CST being truncated here?  It looks like
(long)intvar + 0xL will be mishandled that way.

+ bool inner_ovf = true;
+
+ if (TYPE_PRECISION (inner_type) >= TYPE_PRECISION (type)
+ || TREE_CODE (inner_type) != INTEGER_TYPE)
+   simplify = false;
+ else
+ {
+   inner_ovf = binop_overflow (outer_op, @0, cst_inner, inner_type);
+   if (!inner_ovf)
+ simplify = true;
+ }
+   }
+   (if (simplify && @0)

@0 is always "true"

+(convert (outer_op @0 (convert { @2; }

You already have @2 converted -- cst_inner. Thus just use

(convert (outer_op @0 { cst_inner; }))

+   )))
+#endif


Now onto the other pattern.

+  /* ((T)(A +- CST)) +- CST -> (T)(A) +- CST)  */
+#if GIMPLE
+   (for outer_op (plus minus)
+ (for inner_op (plus minus)
+   (simplify
+(outer_op (convert (inner_op@3 @0 INTEGER_CST@1)) INTEGER_CST@2)
+  

Ping: Re: [PATCH 1/2] gcc: Remove unneeded global flag.

[Andrew Burgess]

In an attempt to get this patch merged (as I still think that its
correct) I've investigated, and documented a little more about how I
think things currently work.  I'm sure most people reading this will
already know this, but hopefully, if my understanding is wrong someone
can point it out.

I've updated the patch to include a few tests, however, I have a
little concern about the new test as they use '.text' and '.data'
section names specifically, and I guess there's likely targets that
don't use those names.  I've limited the tests to GNU/Linux systems,
but maybe I need to be even stricter?

Anyway, the following is a description of why I think this patch is
correct.  The updated patch can be found at the end of the email.

With a focus on the user_defined_section_attribute variable, here's
how things currently work:

1. The variable user_defined_section_attribute is initialised to false
   at compile time.  This is done in toplev.c

2. The only other place that user_defined_section_attribute is set to
   false is in the function rest_of_handle_final in final.c.  This is
   part of the final compiler optimisation pass.  The
   rest_of_handle_final function is called from the
   pass_final::execute method.

3. The user_defined_section_attribute is set to true in only one
   place, in handle_section_attribute in c-family/c-common.c.  Setting
   user_defined_section_attribute to true always happens when the
   handle_section_attribute function is called, unless the target does
   not support named sections.

4. The handle_section_attribute function is called whenever any
   function or data has the section attribute attached.

5. The attribute handling function handle_section_attribute is called
   with the following call-stack (as an example):
   1) handle_section_attribute
   2) decl_attributes
   3) c_decl_attributes
   4) start_decl
   5) c_parser_declaration
   6) c_parser_external_declaration
   7) c_parser_translation_unit
   8) c_parse_file
   9) c_common_parse_file
   10) compile_file
   11) do_compile
   12) toplev::main
   13) main
   The middle levels 2 to 8 could change depending on where the
   section attribute is encountered, but the interesting thing to take
   away is that calls to handle_section_attribute originate from a
   call to c_common_parse_file, which is called from do_compile.
   Right now I believe that this is always the case, this is crucial
   to the validity of this patch, if I've got this wrong then this
   patch is wrong.

6. Specifically, within the compile_file function (in toplev.c) it is
   the line 'lang_hooks.parse_file ();' which leads to the attribute
   handling functions being called.

7. The user_defined_section_attribute is checked in only one place,
   this is in the method pass_partition_blocks::gate, in the file
   bb-reorder.c.  Like the reset to false in final.c (mentioned above)
   this checking of user_defined_section_attribute is part of a
   compiler optimisation pass.

8. Like the file parsing, the compiler optimisation passes originate
   from a call in compile_file (in toplev.c), this call occurs after
   the call to parse the file (obviously).

The problem here that I see then is that we first parse the whole C
file, handling all attributes, we then perform the optimisation passes
on all functions.  As user_defined_section_attribute starts as false
and is set true during the C parsing whenever a section attribute is
encountered, the variable will be true by the end of the parse process
if there is any function or variable with a section attribute.

We then perform the partition-blocks pass on the first function, which
will be disabled (if user_defined_section_attribute) is true,
regardless of whether it was actually that function that has a section
attribute attached.

We then perform the final pass on the first function at which point we
reset the user_defined_section_attribute to false.

When we perform the partition-blocks pass on the second function we
will see user_defined_section_attribute set to false regardless of
whether the function has a section attribute or not (the attribute
handling functions are only called during file parse, not during the
optimisation passes).

There is another issue, that if a variable has a section attribute
this will also cause user_defined_section_attribute to be set to true,
(which makes sense based on the name 'user_defined_section_attribute')
however, given what user_defined_section_attribute is actually used
for, there's no reason to disable the partition-blocks pass just
because some variable is assigned to a specific section.

In the revised patch I disable the partition-blocks pass for a
function only when the function DECL has a section attribute
attached.  This information is already available on the function DECL,
so there's no need for us to keep a separate variable around, and so I
delete user_defined_section_attribute.

I've added three new tests in this latest revision of the patch.
These cover 

Re: [PATCH] Add -Wshadow-local and -Wshadow-compatible-local.

[Mark Wielaard]

On Wed, 2016-09-14 at 05:39 -0700, Ian Lance Taylor wrote:
> On Wed, Sep 14, 2016 at 1:30 AM, Mark Wielaard  wrote:
> > On Wed, 2016-09-14 at 00:00 -0400, Jason Merrill wrote:
> >> I wonder about spelling the options as
> >> -Wshadow={local,compatible-local} rather than with a dash, but
> >> otherwise the patch looks fine.
> >
> > That is a much nicer way to write the option. But if I do that I would
> > like to keep the old names as aliases because Google already ships a gcc
> > that accepts -Wshadow-local and -Wshadow-compatible-local and you can
> > find programs that already probe for those names in their configure
> > scripts. Can I make the existing names hidden aliases by marking them
> > Undocumented in the .opt file? Or is that too contrived/ugly?
> 
> I don't have any opinion as to what the option names should be, but I
> don't see the fact that Google's GCC has different option names as a
> concern.  That GCC is only used within Google

Google did release a gcc with those warning options (I believe as part
of the NDK) and there are various projects out there (firefox seems one
of them) that check to see if these options are available before
enabling/disabling them (I don't know if other compilers implemented
them). Given that there are public sources out there that already seem
to use/test for these option names I would prefer to keep the
compatibility.

Cheers,

Mark

Re: [PATCH 1/3] Put a TARGET_LRA_P into every target

[Peter Bergner]

On 9/14/16 5:35 AM, Segher Boessenkool wrote:

On Tue, Sep 13, 2016 at 02:38:54PM -0600, Jeff Law wrote:

I believe a doc update of some kind is in order.  With the doc update
the entire series is OK.


Good catch, thanks.  My tests finished, all results identical.  I'll add
the following patch for the doc update (I hope the wording is strong enough).


Maybe s/New ports should use LRA/New ports must use LRA/ ?




+ New ports  should use LRA, and existing ports are encouraged to convert.

 ^^
 extra space

Peter

Re: [PATCH] Add -Wshadow-local and -Wshadow-compatible-local.

[Ian Lance Taylor]

On Wed, Sep 14, 2016 at 1:30 AM, Mark Wielaard  wrote:
> On Wed, 2016-09-14 at 00:00 -0400, Jason Merrill wrote:
>> I wonder about spelling the options as
>> -Wshadow={local,compatible-local} rather than with a dash, but
>> otherwise the patch looks fine.
>
> That is a much nicer way to write the option. But if I do that I would
> like to keep the old names as aliases because Google already ships a gcc
> that accepts -Wshadow-local and -Wshadow-compatible-local and you can
> find programs that already probe for those names in their configure
> scripts. Can I make the existing names hidden aliases by marking them
> Undocumented in the .opt file? Or is that too contrived/ugly?

I don't have any opinion as to what the option names should be, but I
don't see the fact that Google's GCC has different option names as a
concern.  That GCC is only used within Google, and Google is moving to
LLVM in any case.  Changing the option names in GCC trunk is not a
problem for anybody.

Ian

Re: [PATCH] df: Keep return address register undefined until epilogue_completed

[Segher Boessenkool]

On Mon, Sep 12, 2016 at 10:30:56AM -0600, Jeff Law wrote:
> On 09/10/2016 01:17 AM, Segher Boessenkool wrote:
> >On Fri, Sep 09, 2016 at 04:40:12PM -0600, Jeff Law wrote:
> >>Right now the dataflow is conservatively correct WRT the return register.
> >
> >And conservatively incorrect wrt all other callee-saved regs!
> But prior to prologue/epilogue insertion it shouldn't matter.  In fact, 
> explicit references to callee saved regs prior to register allocation 
> has always been problematical.
> 
> I do think our handling of life information for callee-saved regs after 
> insertion of the prologue/epilogue could be improved.

And before: it is not really incorrect for a program to access a callee-
saved register (with "register asm", say).  Not very useful except for
debugging, but not really incorrect either.  This all is handled correctly
for LR already, I'm not sure about LIVE though.  Not sure it matters either,
everything seems to stumble along just fine.

> In a separate shrink wrapped world ISTM that we want to move to a model 
> where the return insn itself is a use of the appropriate callee saved 
> regs.  If we did that I suspect some of the hacks from the separate 
> shrink wrapping kit would just "go away".

That doesn't help, sorry.  In fact the rs6000 port used to do just as you
say (for return patterns; not for simple_return patterns), and that only
hindered optimisation (the exit block already has an artificial use of
the link register, so it is not needed; the return being a parallel was
not recognised by various things).

> Attaching actual USEs to those insns is obviously problematical though 
> from a recognition standpoint.  It'd probably have to be structured more 
> like what we do with CALL_INSNs to mark registers used/set.

And it already is, if I understand you correctly :-)

> >>If we made the change you want to make than the dataflow becomes overly
> >>optimistic about the range over which the return register is live prior
> >>to inserting the prologue/epilogue into the insn chain.
> >>
> >>This seems unsafe to me.
> >
> >Yes, but so does the current situation.  And it all seems to work
> >nevertheless :-)
> But that doesn't make it right...

Yes, but neither is the current situation, for powerpc at least: the link
register should *not* be live at function entry, just like the situation
with other callee-save registers, it does not hold a value the current
function has any business with -- before prologue generation.

Now, for separate shrink-wrapping, I want to use the LIVE problem to
determine which basic blocks have which concerns, sorry, need which
components set up; to do that i compute IN+GEN+KILL for the registers
of those components, for each basic block (all this is target code).
This needs those registers *not* marked as live in the entry and exit
blocks, which I probably have to force manually anyway.  And the setting
this patch corrects gets in the way for that a bit, and since it is
incorrect in general anyway, I wanted to do something about it.

But I'll just hard force it off during separate shrink-wrapping, and
we'll leave this problem for another day.


Segher

Re: [Patch, Fortran] Test for implied sequence in structures in common blocks

[Fritz Reese]

Committed revision 240134.

---
Fritz Reese


On Tue, Sep 13, 2016 at 10:52 AM, Jerry DeLisle  wrote:
> On 09/13/2016 07:33 AM, Fritz Reese wrote:
>>
>> Jim,
>>
>> While the test exhibits no particular regression, IMVHO I don't see
>> any reason not to add it. None of my previous testcases include a
>> STRUCTURE within a COMMON block so this would certainly tickle some
>> new code paths, which could expose a future regression.
>>
>> I'll give this through the weekend or so to see if any official
>> maintainers have a comment otherwise I wouldn't mind committing it as
>> trivial.
>>
>
> No problem with the test case being added.
>
> Jerry

Re: [PATCH] Add -Wshadow-local and -Wshadow-compatible-local.

[Eric Gallager]

On 9/14/16, Jason Merrill  wrote:
> On Mon, Sep 12, 2016 at 1:38 PM, Jim Meyering  wrote:
>> On Mon, Sep 12, 2016 at 7:05 AM, Eric Gallager 
>> wrote:
>>> On 9/11/16, Manuel López-Ibáñez  wrote:
 On 11/09/16 14:02, Mark Wielaard wrote:
>   -Wshadow-local which warns if a local variable shadows another local
>   variable or parameter,
>
>   -Wshadow-compatible-local which warns if a local variable shadows
>   another local variable or parameter whose type is compatible with
> that
>   of the shadowing variable.
>>
>> Hi Mark,
>> Thank you for doing this!
>>
 I honestly don't see the need for the second flag. Why not make Wshadow,
 or at
 least Wshadow-local, work in this way by default? Warning for shadowed
 variables that will nevertheless trigger errors/warnings if used
 wrongly
 seems not very useful anyway.
>>>
>>> It helps for readability and helps pre-emptively catch those other
>>> errors/warnings that come from using them wrongly before they occur in
>>> a more confusing manner. Plus more granularity makes it easier to
>>> manage the workload of warning-silencing.
>>
>> The new warnings will be especially welcome in C++ code.
>> -Wshadow is fine for most C code, but in C++ it is very common to have
>> method names that shadow class data member names and/or local
>> variables in any member function definition. Thus, using -Wshadow in
>> C++ code often forces undesirable (and unscalable) renaming to avoid
>> such shadowing, while the only important type of shadowing is that
>> caught by the new options. Many of us want the benefit of the new
>> options (spotting bad, easily-fixed code), without having to incur the
>> renaming (often hurting readability/maintainability) required to
>> enable -Werror=shadow.
>
> I wonder about spelling the options as
> -Wshadow={local,compatible-local} rather than with a dash, but
> otherwise the patch looks fine.
>
> Jason
>


What would the current behavior be called? Maybe
-Wshadow={all,local,compatible-local} instead? Also remember -Werror:
with another '=' you could end up with 2 of them with something like
-Werror=shadow=local which looks more confusing than the hyphenated
version.


Eric

Re: RFA (libstdc++): PATCH to implement C++17 over-aligned new

[Andreas Schwab]

On Sep 13 2016, Jason Merrill  wrote:

> On Tue, Sep 13, 2016 at 9:03 AM, Andreas Schwab  wrote:
>> On Sep 13 2016, Jason Merrill  wrote:
>>
>>> Does this help?
>>
>> Unfortunatly no.
>
> It occurs to me that this function doesn't need to restrict types at
> all.  I'm checking this in; hopefully it will do the trick.

That didn't change anything either.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."