How to easily identify that a FUNCTION_DECL is a lambda

[Martin Liška]

Hi.

For purpose of --coverage I would like to distinguish lambda functions
among DECL_ARTIFICIAL functions. Currently, cp-tree.h provides macro:

/* Test if FUNCTION_DECL is a lambda function.  */
#define LAMBDA_FUNCTION_P(FNDECL)   \
  (DECL_DECLARES_FUNCTION_P (FNDECL)\
   && DECL_OVERLOADED_OPERATOR_P (FNDECL)   \
   && DECL_OVERLOADED_OPERATOR_IS (FNDECL, CALL_EXPR)   \
   && LAMBDA_TYPE_P (CP_DECL_CONTEXT (FNDECL)))

That's FE-specific function that probably should not be called from
middle-end. Any idea how to distinguish lambdas?

Thanks,
Martin

Re: [RFC] Update coding conventions to restrict use of non-const references

[Richard Sandiford]

Aldy Hernandez  writes:
> On Thu, Jul 12, 2018 at 1:41 PM Jonathan Wakely  wrote:
>>
>> On Thu, 12 Jul 2018 at 11:41, Richard Sandiford wrote:
>> > +Only use non-constant references in the following situations:
>> > +
>> > +
>> > +
>> > +when they are necessary to conform to a standard interface, such as
>> > +the first argument to a non-member operator+=
>>
>> And the return value of such operators (which also applies to member
>> operators, which is the more conventional way to write compound
>> assignment operators).
>>
>> > +in a return value, when providing access to something that is known
>> > +to be nonnull
>> > +
>> > +
>> > +
>> > +In other situations the convention is to use pointers instead.
>> > +
>> > +
>> > +
>> > +HOST_WIDE_INT do_arith (..., bool *overflow);   // OK
>> > +HOST_WIDE_INT do_arith (..., bool &overflow);   // Please avoid
>>
>> I understand the objection to using references for out parameters (an
>> alternative to pointers is to return a struct with the wide int result
>> and the overflow flag), but ...
>
> Putting everything in a new struct is just going through more hoops to
> avoid a common language idiom that everyone in C++ is used to.
>
>>
>> > +int *elt = &array[i];  // OK
>> > +int &elt = array[i];   // Please avoid
>>
>> ... this seems unnecessary. If the function is so long that the fact
>> elt is a reference can easily get lost, the problem is the length of
>> the function, not the use of a reference.
>
> Agreed.
>
> Richard (Sandiford), this really looks like going out of your way to
> enforce a personal style issue across the entire code base.  It's sad
> that we try to come up with new ways to make it even harder for new
> developers to contribute to GCC.

It's not enforcing a personal style issue so much as trying to stop
the source base from becoming even more inconsistent, admittedly in one
particular area only.

Before the switch to C++, GCC was one of the most consistent source
bases I'd seen (more so even than LLVM IMO, having worked on both).
It's clearly less so now.  The problem isn't just legacy C-era code vs.
new C++ code, but different styles being used for the C++ code.

(It was interesting that Martin complained earlier in the thread about
how inconsistent GCC was.  I imagine that's the impression that new
developers would have now, rather than being glad that they can use
references to return things.)

In the case of pointers vs. reference parameters for returning values,
we have a clear precendent from the C days and very few existing C++
functions that deviate from it.  So if we were going to pick one,
it seemed obvious which we should pick.  Perhaps there's an argument
that eventually everything will move over to the new style due to
natural churn, but I think another half-transition is far more likely.

But weight of opinion definitely seems to be against adding that rule
to the standards, so I'll drop the patch.

Thanks,
Richard

Re: Understanding tree_swap_operands_p wrt SSA name versions

[Richard Biener]

On Mon, Jul 16, 2018 at 12:19 AM Michael Ploujnikov
 wrote:
>
> On 2018-07-04 04:52 AM, Richard Biener wrote:
> > On Tue, Jul 3, 2018 at 9:09 PM Jeff Law  wrote:
> >>
> >> On 07/03/2018 11:55 AM, Michael Ploujnikov wrote:
> >>> On 2018-07-03 12:46 PM, Richard Biener wrote:
>  On July 3, 2018 4:56:57 PM GMT+02:00, Michael Ploujnikov 
>   wrote:
> > On 2018-06-20 04:23 AM, Richard Biener wrote:
> >> On Wed, Jun 20, 2018 at 7:31 AM Jeff Law  wrote:
> >>>
> >>> On 06/19/2018 12:30 PM, Michael Ploujnikov wrote:
>  Hi everyone,
> 
>  (I hope this is the right place to ask, if not my apologies; please
>  point me in the right direction)
> 
>  I'm trying to get a better understanding of the following part in
>  tree_swap_operands_p():
> 
>    /* It is preferable to swap two SSA_NAME to ensure a canonical
> > form
>   for commutative and comparison operators.  Ensuring a
> > canonical
>   form allows the optimizers to find additional redundancies
> > without
>   having to explicitly check for both orderings.  */
>    if (TREE_CODE (arg0) == SSA_NAME
>    && TREE_CODE (arg1) == SSA_NAME
>    && SSA_NAME_VERSION (arg0) > SSA_NAME_VERSION (arg1))
>  return 1;
> 
>  My questions in no particular order: It looks like this was added
> > in
>  2004. I couldn't find any info other than what's in the
> > corresponding
>  commit (cc0bdf913) so I'm wondering if the canonical form/order
> > still
>  relevant/needed today? Does the ordering have to be done based on
> > the
>  name versions specifically? Or can it be based on something more
>  intrinsic to the input source code rather than a GCC internal
> > value, eg:
>  would alphabetic sort order of the variable names be a reasonable
>  replacement?
> >>> Canonicalization is still important and useful.
> >>
> >> Indeed.
> >>
> >>> However, canonicalizing on SSA_NAMEs is problematical due to the way
> > we
> >>> recycle nodes and re-pack them.
> >>
> >> In the past we made sure to not disrupt order - hopefully that didn't
> > change
> >> so the re-packing shoudln't invaidate previous canonicalization:
> >>
> >> static void
> >> release_free_names_and_compact_live_names (function *fun)
> >> {
> >> ...
> >>   /* And compact the SSA number space.  We make sure to not change
> > the
> >>  relative order of SSA versions.  */
> >>   for (i = 1, j = 1; i < fun->gimple_df->ssa_names->length (); ++i)
> >> {
> >>
> >>
> >>> I think defining additional rules for canonicalization prior to
> > using
> >>> SSA_NAME_VERSION as the fallback would be looked upon favorably.
> >>
> >> I don't see a good reason to do that, it will be harder to spot
> > canonicalization
> >> issues and it will take extra compile-time.
> >>
> >>> Note however, that many of the _DECL nodes referenced by SSA_NAMEs
> > are
> >>> temporaries generated by the compiler and do not correspond to any
> >>> declared/defined object in the original source.  So you'll still
> > need
> >>> the SSA_NAME_VERSION (or something as stable or better)
> > canonicalization
> >>> to handle those cases.
> >>
> >> And not all SSA_NAMEs have underlying _DECL nodes (or IDENTIFIER_NODE
> > names).
> >>
> >> Richard.
> >>
> >>> Jeff
> >
> > After a bit more digging I found that insert_phi_nodes inserts PHIs in
> > the order of UIDs, which indirectly affects the order of vars in
> > old_ssa_names, which in turn affects the order in which
> > make_ssa_name_fn
> > is called to pick SSA versions from FREE_SSANAMES so in the end
> > ordering by SSA_NAME_VERSION's is more or less equivalent to ordering
> > by
> > UIDs. I'm trying to figure out if there's a way to avoid depending on
> > UIDs being ordered in a certain way. So if tree_swap_operands_p stays
> > the same I'm wondering if there's some other info available at the
> > point
> > of insert_phi_nodes that would be a good replacement for UID. From my
> > very limited experience with a very small source input, and if I
> > understand things correctly, all of the var_infos have a var which is
> > DECL_P and thus should have an IDENTIFIER_NODE. Is that true in the
> > general case? I don't fully understand what are all the things that
> > insert_phi_nodes iterates over.
> 
>  Why do you want to remove the dependence on UID ordering? It's pervasive 
>  throughout the whole compilation...
> 
>  Richard.
> 
> > - Michael
> 
> >>>
> >>>
> >>> Well, I'm working on a reduction of the number of changes seen with
> >>> binary diffing (a la

Re: [RFC] Update coding conventions to restrict use of non-const references

[Aldy Hernandez]

On 07/16/2018 04:19 AM, Richard Sandiford wrote:

Aldy Hernandez  writes:

On Thu, Jul 12, 2018 at 1:41 PM Jonathan Wakely  wrote:


On Thu, 12 Jul 2018 at 11:41, Richard Sandiford wrote:

+Only use non-constant references in the following situations:
+
+
+
+when they are necessary to conform to a standard interface, such as
+the first argument to a non-member operator+=


And the return value of such operators (which also applies to member
operators, which is the more conventional way to write compound
assignment operators).


+in a return value, when providing access to something that is known
+to be nonnull
+
+
+
+In other situations the convention is to use pointers instead.
+
+
+
+HOST_WIDE_INT do_arith (..., bool *overflow);   // OK
+HOST_WIDE_INT do_arith (..., bool &overflow);   // Please avoid


I understand the objection to using references for out parameters (an
alternative to pointers is to return a struct with the wide int result
and the overflow flag), but ...


Putting everything in a new struct is just going through more hoops to
avoid a common language idiom that everyone in C++ is used to.




+int *elt = &array[i];  // OK
+int &elt = array[i];   // Please avoid


... this seems unnecessary. If the function is so long that the fact
elt is a reference can easily get lost, the problem is the length of
the function, not the use of a reference.


Agreed.

Richard (Sandiford), this really looks like going out of your way to
enforce a personal style issue across the entire code base.  It's sad
that we try to come up with new ways to make it even harder for new
developers to contribute to GCC.


It's not enforcing a personal style issue so much as trying to stop
the source base from becoming even more inconsistent, admittedly in one
particular area only.

Before the switch to C++, GCC was one of the most consistent source
bases I'd seen (more so even than LLVM IMO, having worked on both).
It's clearly less so now.  The problem isn't just legacy C-era code vs.
new C++ code, but different styles being used for the C++ code.


I certainly appreciate your concern, and the time you have taken to 
amend the guidelines.  However, perhaps we should get consensus here, as 
this is an issue that affects us all.




(It was interesting that Martin complained earlier in the thread about
how inconsistent GCC was.  I imagine that's the impression that new
developers would have now, rather than being glad that they can use
references to return things.)

In the case of pointers vs. reference parameters for returning values,
we have a clear precendent from the C days and very few existing C++
functions that deviate from it.  So if we were going to pick one,
it seemed obvious which we should pick.  Perhaps there's an argument
that eventually everything will move over to the new style due to
natural churn, but I think another half-transition is far more likely.


Heh.  I would definitely prefer moving to the new style ;-).



But weight of opinion definitely seems to be against adding that rule
to the standards, so I'll drop the patch.


Again, I have a preference for non constant references if it makes the 
code easier to read, but I don't feel so strongly as to fight about it, 
especially if there is consensus going the other way.


I welcome more discussion on this, and will gladly accept what the 
majority agrees on.


Aldy

Re: [RFC] Update coding conventions to restrict use of non-const references

[Richard Biener]

On Mon, Jul 16, 2018 at 11:48 AM Aldy Hernandez  wrote:
>
>
>
> On 07/16/2018 04:19 AM, Richard Sandiford wrote:
> > Aldy Hernandez  writes:
> >> On Thu, Jul 12, 2018 at 1:41 PM Jonathan Wakely  
> >> wrote:
> >>>
> >>> On Thu, 12 Jul 2018 at 11:41, Richard Sandiford wrote:
>  +Only use non-constant references in the following situations:
>  +
>  +
>  +
>  +when they are necessary to conform to a standard interface, such as
>  +the first argument to a non-member operator+=
> >>>
> >>> And the return value of such operators (which also applies to member
> >>> operators, which is the more conventional way to write compound
> >>> assignment operators).
> >>>
>  +in a return value, when providing access to something that is known
>  +to be nonnull
>  +
>  +
>  +
>  +In other situations the convention is to use pointers instead.
>  +
>  +
>  +
>  +HOST_WIDE_INT do_arith (..., bool *overflow);   // OK
>  +HOST_WIDE_INT do_arith (..., bool &overflow);   // Please avoid
> >>>
> >>> I understand the objection to using references for out parameters (an
> >>> alternative to pointers is to return a struct with the wide int result
> >>> and the overflow flag), but ...
> >>
> >> Putting everything in a new struct is just going through more hoops to
> >> avoid a common language idiom that everyone in C++ is used to.
> >>
> >>>
>  +int *elt = &array[i];  // OK
>  +int &elt = array[i];   // Please avoid
> >>>
> >>> ... this seems unnecessary. If the function is so long that the fact
> >>> elt is a reference can easily get lost, the problem is the length of
> >>> the function, not the use of a reference.
> >>
> >> Agreed.
> >>
> >> Richard (Sandiford), this really looks like going out of your way to
> >> enforce a personal style issue across the entire code base.  It's sad
> >> that we try to come up with new ways to make it even harder for new
> >> developers to contribute to GCC.
> >
> > It's not enforcing a personal style issue so much as trying to stop
> > the source base from becoming even more inconsistent, admittedly in one
> > particular area only.
> >
> > Before the switch to C++, GCC was one of the most consistent source
> > bases I'd seen (more so even than LLVM IMO, having worked on both).
> > It's clearly less so now.  The problem isn't just legacy C-era code vs.
> > new C++ code, but different styles being used for the C++ code.
>
> I certainly appreciate your concern, and the time you have taken to
> amend the guidelines.  However, perhaps we should get consensus here, as
> this is an issue that affects us all.
>
> >
> > (It was interesting that Martin complained earlier in the thread about
> > how inconsistent GCC was.  I imagine that's the impression that new
> > developers would have now, rather than being glad that they can use
> > references to return things.)
> >
> > In the case of pointers vs. reference parameters for returning values,
> > we have a clear precendent from the C days and very few existing C++
> > functions that deviate from it.  So if we were going to pick one,
> > it seemed obvious which we should pick.  Perhaps there's an argument
> > that eventually everything will move over to the new style due to
> > natural churn, but I think another half-transition is far more likely.
>
> Heh.  I would definitely prefer moving to the new style ;-).
>
> >
> > But weight of opinion definitely seems to be against adding that rule
> > to the standards, so I'll drop the patch.
>
> Again, I have a preference for non constant references if it makes the
> code easier to read, but I don't feel so strongly as to fight about it,
> especially if there is consensus going the other way.
>
> I welcome more discussion on this, and will gladly accept what the
> majority agrees on.

The only thing I would insist on is consistency within a group of
API functions or even within related APIs (poly-int vs. wide-int for example).

Richard.

>
> Aldy

Re: How to easily identify that a FUNCTION_DECL is a lambda

[Nathan Sidwell]

On 07/16/2018 03:23 AM, Martin Liška wrote:

Hi.

For purpose of --coverage I would like to distinguish lambda functions
among DECL_ARTIFICIAL functions. Currently, cp-tree.h provides macro:

/* Test if FUNCTION_DECL is a lambda function.  */
#define LAMBDA_FUNCTION_P(FNDECL)   \
   (DECL_DECLARES_FUNCTION_P (FNDECL)   \
&& DECL_OVERLOADED_OPERATOR_P (FNDECL)  \
&& DECL_OVERLOADED_OPERATOR_IS (FNDECL, CALL_EXPR)  \
&& LAMBDA_TYPE_P (CP_DECL_CONTEXT (FNDECL)))

That's FE-specific function that probably should not be called from
middle-end. Any idea how to distinguish lambdas?


You're going to need a language hook.  Lambda fns are just regular 
member fns outside the front-end.  Make the hook more than 
'lambda_fn_p', so it can extend to other exciting C++ features.  Perhaps 
something like:


enum synthetic_fn_kind {
  sfk_none,
  sfk_cxx_lambda,
};
synthetic_fn_kind (*categorize_artificial_fn) (tree decl);

We'll have to expose the union of FE's such sythetic fns to the middle 
end, but at least not how the FE distinguishes them.


Hm, but isn't this info lost if we're in LTO at this point?  Not sure if 
we'll need to propagate this through the LTO streaming.  I guess that's 
a later bug to handle though.


nathan

--
Nathan Sidwell

Re: [GSOC] LTO dump tool project

[Hrishikesh Kulkarni]

Hi,

As suggested I have created command line option -optimized=[none,
blocks, stats, vops] to dump the respective gimple bodies of all
functions.

for example:

-optimized=blocks will dump

Gimple body of function: main
main ()
{
;;   basic block 2, loop depth 0
;;pred:   ENTRY
  printf ("%d", 8);
  return 0;
;;succ:   EXIT

}


Gimple body of function: bar
bar (int a, int b, int cond)
{
  int ret;

;;   basic block 2, loop depth 0
;;pred:   ENTRY
  if (cond_2(D) != 0)
goto ; [50.00%]
  else
goto ; [50.00%]
;;succ:   3
;;4

;;   basic block 3, loop depth 0
;;pred:   2
  ret_5 = a_3(D) + b_4(D);
  goto ; [100.00%]
;;succ:   5

;;   basic block 4, loop depth 0
;;pred:   2
  ret_6 = a_3(D) - b_4(D);
;;succ:   5

;;   basic block 5, loop depth 0
;;pred:   3
;;4
  # ret_1 = PHI 
  return ret_1;
;;succ:   EXIT

}


Gimple body of function: foo
foo (int a, int b)
{
;;   basic block 2, loop depth 0
;;pred:   ENTRY
  _3 = a_1(D) + b_2(D);
  return _3;
;;succ:   EXIT

}

I have pushed the changes to the repo. Please find the diff file
attached herewith.

Regards,

Hrishikesh

On Fri, Jul 13, 2018 at 2:47 PM, Martin Liška  wrote:
> On 07/12/2018 08:05 PM, Hrishikesh Kulkarni wrote:
>> Hi,
>>
>> I have added command line options:
>>
>> -body=
>> To dump gimple body (TDF_NONE) of specific function.
>>
>> -optimized-blocks=
>> To dump gimple body (TDF_BLOCKS) of specific function.
>>
>> -optimized-stats=
>> To dump gimple body (TDF_STATS) of specific function.
>>
>> -optimized-vops=
>> To dump gimple body (TDF_VOPS) of specific function.
>
> Hi.
>
> Thanks for it. However I would expect something more smart:
> -optimized=[vops,stats,block]. Note that you should do similar
> what's done in: gcc/dumpfile.c
>
> int
> gcc::dump_manager::
> dump_switch_p_1 (const char *arg, struct dump_file_info *dfi, bool doglob)
>
> that will automatically parse   dump_flags_t flags;
>
> Then the copy&parse will not be needed.
>
>>
>> for example:
>>
>> -body=foo  will dump
>>
>> Gimple body of function: foo
>> foo (int a, int b)
>> {
>>[local count: 1073741825]:
>>   _3 = a_1(D) + b_2(D);
>>   return _3;
>>
>> }
>>
>>
>> -optimized-blocks=foo  will dump
>>
>> Gimple body of function: foo
>> foo (int a, int b)
>> {
>> ;;   basic block 2, loop depth 0
>> ;;pred:   ENTRY
>>   _3 = a_1(D) + b_2(D);
>>   return _3;
>> ;;succ:   EXIT
>>
>> }
>>
>>
>> -optimized-stats=foo  will dump
>>
>> Gimple body of function: foo
>> foo (int a, int b)
>> {
>>[local count: 1073741825]:
>>   _3 = a_1(D) + b_2(D);
>>   return _3;
>>
>> }
>>
>>
>> -optimized-vops=foo  will dump
>>
>> Gimple body of function: foo
>> foo (int a, int b)
>> {
>>[local count: 1073741825]:
>>   _3 = a_1(D) + b_2(D);
>>   # VUSE <.MEM_4(D)>
>>   return _3;
>>
>> }
>>
>> I have pushed the changes to the current branch. Please find the diff
>> file attached herewith.
>>
>> I have tried to follow the coding conventions as far as possible in
>> this patch. Please let me know if I am missing out something so that I
>> can improve the same while refactoring and clean up as suggested in
>> the previous mail.
>
> As mentioned in the previous email, indentation level is 2. And every 8 spaces
> are replaced with a tabular. In our patch, you use indentation level equal to
> one tab, which results in indentation level 8.
>
> Martin
>
>>
>> Regards,
>>
>> Hrishikesh
>>
>> On Wed, Jul 11, 2018 at 12:10 AM, Hrishikesh Kulkarni
>>  wrote:
>>> Hi,
>>>
>>> Thanks for suggestions. I would start working on these points and will
>>> try to complete as early as possible.
>>>
>>> Regards,
>>>
>>> Hrishikesh
>>>
>>> On Mon, Jul 9, 2018 at 2:04 PM, Martin Liška  wrote:
 On 07/09/2018 09:50 AM, Hrishikesh Kulkarni wrote:
> Hi,
>
> The command line option -gimple-stats will dump the statistics of
> gimple statements.
>
> For example:
>
> $ ../stage1-build/gcc/lto-dump test_hello.o -gimple-stats
>
> will dump:
>
> GIMPLE statements
> Kind   Stmts  Bytes
> ---
> assignments3264
> phi nodes  1248
> conditionals   1 80
> everything else   12704
> ---
> Total 17   1296
> ---
>
> I have pushed the changes to the repo. Please find the diff file
> attached herewith.
>
> Regards,
>
> Hrishikesh

 Hi.

 Thanks for the work. I briefly took a look at the code and I would
 focus now directly on refactoring:

 - please make a new branch, first copy lto-dump.c file and all the
 Makefile needed stuff and commit that.
 - next please rebase (squash) all your patches which you have on top

Re: How to easily identify that a FUNCTION_DECL is a lambda

[Richard Biener]

On July 16, 2018 4:30:42 PM GMT+02:00, Nathan Sidwell  wrote:
>On 07/16/2018 03:23 AM, Martin Liška wrote:
>> Hi.
>> 
>> For purpose of --coverage I would like to distinguish lambda
>functions
>> among DECL_ARTIFICIAL functions. Currently, cp-tree.h provides macro:
>> 
>> /* Test if FUNCTION_DECL is a lambda function.  */
>> #define LAMBDA_FUNCTION_P(FNDECL)\
>>(DECL_DECLARES_FUNCTION_P (FNDECL)\
>> && DECL_OVERLOADED_OPERATOR_P (FNDECL)   \
>> && DECL_OVERLOADED_OPERATOR_IS (FNDECL, CALL_EXPR)   \
>> && LAMBDA_TYPE_P (CP_DECL_CONTEXT (FNDECL)))
>> 
>> That's FE-specific function that probably should not be called from
>> middle-end. Any idea how to distinguish lambdas?
>
>You're going to need a language hook.  Lambda fns are just regular 
>member fns outside the front-end.  Make the hook more than 
>'lambda_fn_p', so it can extend to other exciting C++ features. 
>Perhaps 
>something like:
>
>enum synthetic_fn_kind {
>   sfk_none,
>   sfk_cxx_lambda,
>};
>synthetic_fn_kind (*categorize_artificial_fn) (tree decl);
>
>We'll have to expose the union of FE's such sythetic fns to the middle 
>end, but at least not how the FE distinguishes them.
>
>Hm, but isn't this info lost if we're in LTO at this point?  Not sure
>if 
>we'll need to propagate this through the LTO streaming.  I guess that's
>
>a later bug to handle though.

Just use a spare bit in function_decl, then we can simply stream it. 

Richard. 

>nathan

Re: How to easily identify that a FUNCTION_DECL is a lambda

[Nathan Sidwell]

On 07/16/2018 12:04 PM, Richard Biener wrote:


Just use a spare bit in function_decl, then we can simply stream it.


If there's one, then sure.  (you've reminded me that there are a bunch 
of mutually disjoint flags in function_decl that could be collapsed to 
an enumeration.  This may be another such bit.)


nathan

--
Nathan Sidwell

Re: [RFC] Update coding conventions to restrict use of non-const references

[Martin Sebor]

On 07/16/2018 02:19 AM, Richard Sandiford wrote:

Aldy Hernandez  writes:

On Thu, Jul 12, 2018 at 1:41 PM Jonathan Wakely  wrote:


On Thu, 12 Jul 2018 at 11:41, Richard Sandiford wrote:

+Only use non-constant references in the following situations:
+
+
+
+when they are necessary to conform to a standard interface, such as
+the first argument to a non-member operator+=


And the return value of such operators (which also applies to member
operators, which is the more conventional way to write compound
assignment operators).


+in a return value, when providing access to something that is known
+to be nonnull
+
+
+
+In other situations the convention is to use pointers instead.
+
+
+
+HOST_WIDE_INT do_arith (..., bool *overflow);   // OK
+HOST_WIDE_INT do_arith (..., bool &overflow);   // Please avoid


I understand the objection to using references for out parameters (an
alternative to pointers is to return a struct with the wide int result
and the overflow flag), but ...


Putting everything in a new struct is just going through more hoops to
avoid a common language idiom that everyone in C++ is used to.




+int *elt = &array[i];  // OK
+int &elt = array[i];   // Please avoid


... this seems unnecessary. If the function is so long that the fact
elt is a reference can easily get lost, the problem is the length of
the function, not the use of a reference.


Agreed.

Richard (Sandiford), this really looks like going out of your way to
enforce a personal style issue across the entire code base.  It's sad
that we try to come up with new ways to make it even harder for new
developers to contribute to GCC.


It's not enforcing a personal style issue so much as trying to stop
the source base from becoming even more inconsistent, admittedly in one
particular area only.

Before the switch to C++, GCC was one of the most consistent source
bases I'd seen (more so even than LLVM IMO, having worked on both).
It's clearly less so now.  The problem isn't just legacy C-era code vs.
new C++ code, but different styles being used for the C++ code.

(It was interesting that Martin complained earlier in the thread about
how inconsistent GCC was.  I imagine that's the impression that new
developers would have now, rather than being glad that they can use
references to return things.)


Just to be clear: mine wasn't meant to be a complaint so much
as an observation.  With a code base as large as GCC and with
so many people contributing to it, I wouldn't expect too much
more consistency unless it was enforced by tools.

I don't have a problem with adopting a new convention per se.
What I find a poor use of time and effort is relying on code
review as the only enforcement of a consistent coding style.
It gets especially frustrating when the same piece of code
that is acceptable to one reviewer is not good enough for
another only because it doesn't follow some convention that
GCC isn't consistent with to begin with (which to me seems
like it's more than those GCC is consistent with).

Martin

PS Here are some examples of documented GCC coding conventions
that I have noticed are either very inconsistent or virtually
ignored ignored (this isn't a survey):

  *  logical not!xvs  ! x  thousands of violations
  *  cast   (foo) x   vs  (foo)x   ditto
  *  C++-style casts vs C-stylemostly ignored
  *  explicit ctorsignored
  *  in class member definitions

Re: ICE building a libsupc++ file, pdp11 target

[Paul Koning]

> On Jul 13, 2018, at 3:12 PM, U.Mutlu  wrote:
> 
> Paul Koning wrote on 07/13/2018 08:56 PM:
>> 
>> 
>>> On Jul 13, 2018, at 2:52 PM, U.Mutlu  wrote:
>>> 
>>> Paul Koning wrote on 07/13/2018 08:27 PM:
 I'm trying to see if I can build the pdp11 target for languages other than 
 just C, and the answer is for the most part yes.  But I' running into an 
 ICE I can't figure out.  It's way before the back end comes into the 
 picture as far as I can see, and there's nothing particularly strange 
 looking in the input file that suggests anything.
 
 Any suggestions on where to look?  The failure is:
 
 libtool: compile:  /Users/pkoning/Documents/svn/buildpdp+/./gcc/xgcc 
 -shared-libgcc -B/Users/pkoning/Documents/svn/buildpdp+/./gcc -nostdinc++ 
 -L/Users/pkoning/Documents/svn/buildpdp+/pdp11-aout/libstdc++-v3/src 
 -L/Users/pkoning/Documents/svn/buildpdp+/pdp11-aout/libstdc++-v3/src/.libs 
 -L/Users/pkoning/Documents/svn/buildpdp+/pdp11-aout/libstdc++-v3/libsupc++/.libs
  -B/usr/local/pdp11-aout/pdp11-aout/bin/ 
 -B/usr/local/pdp11-aout/pdp11-aout/lib/ -isystem 
 /usr/local/pdp11-aout/pdp11-aout/include -isystem 
 /usr/local/pdp11-aout/pdp11-aout/sys-include 
 -I/Users/pkoning/Documents/svn/gcc/libstdc++-v3/../libgcc 
 -I/Users/pkoning/Documents/svn/buildpdp+/pdp11-aout/libstdc++-v3/include/pdp11-aout
  -I/Users/pkoning/Documents/svn/buildpdp+/pdp11-aout/libstdc++-v3/include 
 -I/Users/pkoning/Documents/svn/gcc/libstdc++-v3/libsupc++ 
 -fno-implicit-templates -Wall -Wextra -Wwrite-strings -Wcast-qual -Wabi 
 -fdiagnostics-show-location=once -frandom-seed=new_opa.lo -g -O2 
 -std=gnu++1z -c ../../../../gcc/l
> ib
>>> stdc++-v3/libsupc++/new_opa.cc -o new_opa.o
 cc1plus: warning: -Wabi won't warn about anything [-Wabi]
 cc1plus: note: -Wabi warns about differences from the most up-to-date ABI, 
 which is also used by default
 cc1plus: note: use e.g. -Wabi=11 to warn about changes from GCC 7
 ../../../../gcc/libstdc++-v3/libsupc++/new_opa.cc:112:1: internal compiler 
 error: in import_export_decl, at cp/decl2.c:2877
  }
  ^
 libbacktrace could not find executable to open
 Please submit a full bug report,
 with preprocessed source if appropriate.
 See  for instructions.
 make[3]: *** [new_opa.lo] Error 1
>>> 
>>> 
>>> It's failing at the last gcc_assert() below (file 
>>> ../gcc_src/gcc/cp/decl2.c:2877 ):
>> 
>> Sorry, I should have been more explicit.  I saw that, but my question is: 
>> what is the underlying problem that triggers the assert?  The comment is not 
>> much help to me.  And more specifically: what could a target be doing wrong 
>> that would make an early stage of the compiler fail like this on what seems 
>> like a pretty straightforward source file?
>> 
>> Many of the other libstdc++ bits compile just fine, as do plenty of 
>> testsuite cases and some test files of my own.
>> 
> 
> /* In a VAR_DECL, FUNCTION_DECL, NAMESPACE_DECL or TYPE_DECL,
>   nonzero means name is to be accessible from outside this translation unit.
>   In an IDENTIFIER_NODE, nonzero means an external declaration
>   accessible from outside this translation unit was previously seen
>   for this name in an inner scope.  */
> #define TREE_PUBLIC(NODE) ((NODE)->base.public_flag)
> 
> 
> Ie. it has todo with the value of the member var public_flag of the tree decl.

I'm still on the same question as before.  Why do I get an ICE in the tree 
phase of the compiler, complaining about flags of a declaration, based on 
something I apparently have wrong in my target description?  I can build C++ 
for other targets, so this isn't a general bug.  But I'm not used to target 
(back end) stuff affecting the compiler before I even get to the RTL part.  And 
in this case, looking at the failing code gives me no clue at all.  I can't do 
anything with the "tree" object to find out what it describes; it's a VAR_DECL 
but I don't know what to look at. I tried turning on tree dump files, those 
gave no clue either.  And there is nothing in the manuals.

paul

Re: Understanding tree_swap_operands_p wrt SSA name versions

[Michael Ploujnikov]

On 2018-07-16 04:30 AM, Richard Biener wrote:
> On Mon, Jul 16, 2018 at 12:19 AM Michael Ploujnikov
>  wrote:
>>
>> On 2018-07-04 04:52 AM, Richard Biener wrote:
>>> On Tue, Jul 3, 2018 at 9:09 PM Jeff Law  wrote:

 On 07/03/2018 11:55 AM, Michael Ploujnikov wrote:
> On 2018-07-03 12:46 PM, Richard Biener wrote:
>> On July 3, 2018 4:56:57 PM GMT+02:00, Michael Ploujnikov 
>>  wrote:
>>> On 2018-06-20 04:23 AM, Richard Biener wrote:
 On Wed, Jun 20, 2018 at 7:31 AM Jeff Law  wrote:
>
> On 06/19/2018 12:30 PM, Michael Ploujnikov wrote:
>> Hi everyone,
>>
>> (I hope this is the right place to ask, if not my apologies; please
>> point me in the right direction)
>>
>> I'm trying to get a better understanding of the following part in
>> tree_swap_operands_p():
>>
>>   /* It is preferable to swap two SSA_NAME to ensure a canonical
>>> form
>>  for commutative and comparison operators.  Ensuring a
>>> canonical
>>  form allows the optimizers to find additional redundancies
>>> without
>>  having to explicitly check for both orderings.  */
>>   if (TREE_CODE (arg0) == SSA_NAME
>>   && TREE_CODE (arg1) == SSA_NAME
>>   && SSA_NAME_VERSION (arg0) > SSA_NAME_VERSION (arg1))
>> return 1;
>>
>> My questions in no particular order: It looks like this was added
>>> in
>> 2004. I couldn't find any info other than what's in the
>>> corresponding
>> commit (cc0bdf913) so I'm wondering if the canonical form/order
>>> still
>> relevant/needed today? Does the ordering have to be done based on
>>> the
>> name versions specifically? Or can it be based on something more
>> intrinsic to the input source code rather than a GCC internal
>>> value, eg:
>> would alphabetic sort order of the variable names be a reasonable
>> replacement?
> Canonicalization is still important and useful.

 Indeed.

> However, canonicalizing on SSA_NAMEs is problematical due to the way
>>> we
> recycle nodes and re-pack them.

 In the past we made sure to not disrupt order - hopefully that didn't
>>> change
 so the re-packing shoudln't invaidate previous canonicalization:

 static void
 release_free_names_and_compact_live_names (function *fun)
 {
 ...
   /* And compact the SSA number space.  We make sure to not change
>>> the
  relative order of SSA versions.  */
   for (i = 1, j = 1; i < fun->gimple_df->ssa_names->length (); ++i)
 {


> I think defining additional rules for canonicalization prior to
>>> using
> SSA_NAME_VERSION as the fallback would be looked upon favorably.

 I don't see a good reason to do that, it will be harder to spot
>>> canonicalization
 issues and it will take extra compile-time.

> Note however, that many of the _DECL nodes referenced by SSA_NAMEs
>>> are
> temporaries generated by the compiler and do not correspond to any
> declared/defined object in the original source.  So you'll still
>>> need
> the SSA_NAME_VERSION (or something as stable or better)
>>> canonicalization
> to handle those cases.

 And not all SSA_NAMEs have underlying _DECL nodes (or IDENTIFIER_NODE
>>> names).

 Richard.

> Jeff
>>>
>>> After a bit more digging I found that insert_phi_nodes inserts PHIs in
>>> the order of UIDs, which indirectly affects the order of vars in
>>> old_ssa_names, which in turn affects the order in which
>>> make_ssa_name_fn
>>> is called to pick SSA versions from FREE_SSANAMES so in the end
>>> ordering by SSA_NAME_VERSION's is more or less equivalent to ordering
>>> by
>>> UIDs. I'm trying to figure out if there's a way to avoid depending on
>>> UIDs being ordered in a certain way. So if tree_swap_operands_p stays
>>> the same I'm wondering if there's some other info available at the
>>> point
>>> of insert_phi_nodes that would be a good replacement for UID. From my
>>> very limited experience with a very small source input, and if I
>>> understand things correctly, all of the var_infos have a var which is
>>> DECL_P and thus should have an IDENTIFIER_NODE. Is that true in the
>>> general case? I don't fully understand what are all the things that
>>> insert_phi_nodes iterates over.
>>
>> Why do you want to remove the dependence on UID ordering? It's pervasive 
>> throughout the whole compilation...
>>
>> Richard.
>>
>>> - Michael
>>
>
>
> Well, I'm working on a reduction of the nu