Possible Bugs in cgraphunit.c
Greetings, Seems that the extend_trucks return values are not returned when called in both, cnode::assemble_thunks_and_aliases and cnode::create_wrapper. I'm not sure if this is a set of edge case bugs or there was a reason for this. Seems not as its checked in the third function caller in the file, cgraph_node::analyze. Not sure if my assumptions are correct so I'm asking if there isn't another reason for this as the code seems to have at least directly no reason for it, Nick
Re: Branch and tag deletions
> On Dec 3, 2019, at 11:10 PM, Richard Earnshaw (lists)
> wrote:
>
> On 03/12/2019 18:56, Segher Boessenkool wrote:
>> On Tue, Dec 03, 2019 at 09:16:43AM +, Richard Earnshaw (lists) wrote:
>>> On 03/12/2019 00:56, Segher Boessenkool wrote:
That sounds simpler than it is... After using this for a while you'll
get names that you want to delete, but that name *already* is in
/refs/deleted. So what will you name it then? People will still need
to be able to find it.
But we could make an "old-svn" hierarchy or similar that just has
everything svn has now (and will never change, so it will never cause
conflicts).
>>>
>>> Well that's true of /any/ renaming scheme for dead branches.
>>
>> No, it is not. If you have a simple "deleted" hierarchy, to which you
>> add things, you will get conflicts. If you have one just for things
>> imported from SVN, it will never change (since SVN is set in stone after
>> the conversion), and there will not be conflicts.
>
> But you've still got the ongoing branch death issue to deal with, and
> that was my point. If you want to keep them, and you don't want them
> polluting the working namespace, you have to do *some* renaming of them.
>
>
>>
>>> There's
>>> nothing special about this one. On the other hand, there's nothing that
>>> says that the renamed branch has to have exactly the same name as the
>>> live one: it's a rename after all.
>>
>> Renamed tags and branches are *useless*, we could just as well delete
>> them completely, if people can no longer find them.
>
> They can be found, it's just that they don't appear in the standard list
> since they aren't pulled by default from the upstream repository. It's
> no different from the situation where if you clone from a clone, the
> things in the local repository that are in refs/remotes are not pulled
> into the secondary clone, unless you add an explicit fetch entry to your
> remote's configuration, something like
>
> [origin]
> ...
> fetch = refs/deleted/*:refs/remotes/origin/deleted/*
>
>
> So they're not useless as you put it. They're out of the way, but can
> be recovered if there's need. What's more, if the branch died without
> being merged, the blobs for it will stay *on the server* and not waste
> users' time and bandwidth by being repeatedly pulled.
>
>>
>>> You only have to run 'svn ls' on the current gcc branches directory in
>>> SVN to realize just what a mess our current naming scheme it, so I'd
>>> also suggest that we do some general reorganization of the other
>>> branches/tags we have, especially if we have a /refs/svn namespace after
>>> conversion:
>>>
>>> a) Only live development branches get moved to the normal git namespace,
>>> but see d) & e) below
>>
>> Yes, I called it "old-svn" for a reason. One idea is to move *everything*
>> there, and let people make a copy to the "live" stuff if they want it.
>> And we can pre-populate the things we know are still used, of course, and
>> all release branches (closed or not), that kind of thing. But we could
>> just shovel all that is in SVN into some nice tidy subdir, that normal
>> people never have to look at again :-)
>>
>>> b) vendor branches should move to something like
>>> refs/vendors//{heads/tags} (these won't be pulled by default by
>>> a normal clone, you'd have to add an explicit map request to see them.
>>> c) user branches should only be in something line
>>> refs/users//{heads/tags} (similar to above)
>>
>> Yup.
>
> And see above for the type of fetch spec you'd need to pull and see them
> locally, with the structure I suggest, you can even write
>
> fetch = refs/vendors/ibm/heads/*:refs/remotes/origin/ibm/*
>
> and only the ibm sub-part of that hierarchy would be fetched.
IMO, it is valuable to have user and vendor branches under default refs/heads/*
hierarchy. I find it useful to see that IBM's or Arm's branches were updated
since I last pulled from upstream. The fact that branches were updated means
that there are development I may want to look at or keep note of.
I feel strongly about vendor branches, and much less strongly about user
branches. Individual users can be less careful in following best git
practices, can commit random stuff and rewrite history of their branches.
Therefore, it may make for a smoother git experience to put user branches out
of sight. Vendors, otoh, tend to keep their branches very clean.
>
>>
>>> d) releases should go into refs/{heads/tags}/releases (makes it clearer
>>> to casual users of the repo that these are 'official')
>>
>> What are releases? Release branches?
>
> branches in the heads space, tags in the tags space.
>
>>
>> It would be very inconvenient to not have the recent releases immediately
>> accessible, fwiw, but those could be just a copy. And then delete that
>> one after a branch is closed?
>>
>>> e) other general development branches in refs/{heads/tags}/devt
>>
>> What
Re: Branch and tag deletions
On 05/12/2019 08:55, Maxim Kuvyrkov wrote:
On Dec 3, 2019, at 11:10 PM, Richard Earnshaw (lists)
wrote:
On 03/12/2019 18:56, Segher Boessenkool wrote:
On Tue, Dec 03, 2019 at 09:16:43AM +, Richard Earnshaw (lists) wrote:
On 03/12/2019 00:56, Segher Boessenkool wrote:
That sounds simpler than it is... After using this for a while you'll
get names that you want to delete, but that name *already* is in
/refs/deleted. So what will you name it then? People will still need
to be able to find it.
But we could make an "old-svn" hierarchy or similar that just has
everything svn has now (and will never change, so it will never cause
conflicts).
Well that's true of /any/ renaming scheme for dead branches.
No, it is not. If you have a simple "deleted" hierarchy, to which you
add things, you will get conflicts. If you have one just for things
imported from SVN, it will never change (since SVN is set in stone after
the conversion), and there will not be conflicts.
But you've still got the ongoing branch death issue to deal with, and
that was my point. If you want to keep them, and you don't want them
polluting the working namespace, you have to do *some* renaming of them.
There's
nothing special about this one. On the other hand, there's nothing that
says that the renamed branch has to have exactly the same name as the
live one: it's a rename after all.
Renamed tags and branches are *useless*, we could just as well delete
them completely, if people can no longer find them.
They can be found, it's just that they don't appear in the standard list
since they aren't pulled by default from the upstream repository. It's
no different from the situation where if you clone from a clone, the
things in the local repository that are in refs/remotes are not pulled
into the secondary clone, unless you add an explicit fetch entry to your
remote's configuration, something like
[origin]
...
fetch = refs/deleted/*:refs/remotes/origin/deleted/*
So they're not useless as you put it. They're out of the way, but can
be recovered if there's need. What's more, if the branch died without
being merged, the blobs for it will stay *on the server* and not waste
users' time and bandwidth by being repeatedly pulled.
You only have to run 'svn ls' on the current gcc branches directory in
SVN to realize just what a mess our current naming scheme it, so I'd
also suggest that we do some general reorganization of the other
branches/tags we have, especially if we have a /refs/svn namespace after
conversion:
a) Only live development branches get moved to the normal git namespace,
but see d) & e) below
Yes, I called it "old-svn" for a reason. One idea is to move *everything*
there, and let people make a copy to the "live" stuff if they want it.
And we can pre-populate the things we know are still used, of course, and
all release branches (closed or not), that kind of thing. But we could
just shovel all that is in SVN into some nice tidy subdir, that normal
people never have to look at again :-)
b) vendor branches should move to something like
refs/vendors//{heads/tags} (these won't be pulled by default by
a normal clone, you'd have to add an explicit map request to see them.
c) user branches should only be in something line
refs/users//{heads/tags} (similar to above)
Yup.
And see above for the type of fetch spec you'd need to pull and see them
locally, with the structure I suggest, you can even write
fetch = refs/vendors/ibm/heads/*:refs/remotes/origin/ibm/*
and only the ibm sub-part of that hierarchy would be fetched.
IMO, it is valuable to have user and vendor branches under default refs/heads/*
hierarchy. I find it useful to see that IBM's or Arm's branches were updated
since I last pulled from upstream. The fact that branches were updated means
that there are development I may want to look at or keep note of.
I feel strongly about vendor branches, and much less strongly about user
branches. Individual users can be less careful in following best git
practices, can commit random stuff and rewrite history of their branches.
Therefore, it may make for a smoother git experience to put user branches out
of sight. Vendors, otoh, tend to keep their branches very clean.
You only need to add fetch lines to your git config and you can see the
vendor branches as well. In fact,
fetch = refs/vendors/*:remotes/origin/vendors/*
is all you need. Or you can be more specific if you only want a
specific vendor.
So you can have them, if you want to, but most people, who won't be
interested will never see them. If everything goes in the top-level
name space, then everybody has to deal with them, rather than just those
who are interested in them.
R.
d) releases should go into refs/{heads/tags}/releases (makes it clearer
to casual users of the repo that these are 'official')
What are releases? Release branches?
branches in the heads space, tags in the ta
Re: PPC64 libmvec implementation of sincos
On Wed, Dec 4, 2019 at 9:53 PM GT wrote: > > ‐‐‐ Original Message ‐‐‐ > On Wednesday, November 27, 2019 3:19 AM, Richard Biener > wrote: > > ... > > > > Questions: > > > > > > 1. Should we aim to provide a vectorized version of __builtin_cexpi? If > > > so, it would have > > > to be a PPC64-only vector __builtin-cexpi, right? > > > > > > 2. Or should we require that vectorized sincos be available only when > > > -fno-builtin-sincos flag > > > is used in compilation? > > > > > > > > > I don't think we need to fix both types of vectorization failures in > > > order to obtain sincos > > > vectorization. > > > > I think we should have a vectorized cexpi since that's having a sane > > ABI. The complex > > return type of cexpi makes it a little awkward for the vectorizer but > > handling this should > > be manageable. It's a bit difficult to expose complex types to the > > vectorizer since > > most cases are lowered early. > > > > I'm trying to identify the source code which needs modification but I need > help proceeding. > > I am comparing two compilations: The first is a simple file with a call to > sin in a loop. > Vectorization succeeds. The second is an almost identical file but with a > call to sincos > in the loop. Vectorization fails. > > In gdb, the earliest code location where the two compilations differ is in > function > number_of_iterations_exit_assumptions in file tree-ssa-loop-niter.c. Line > > op0 = gimple_cond_lhs (stmt); > > returns a tree which when analyzed in function instantiate_scev_r (in file > tree-scalar-evolution.c) > results in the first branch of the switch being taken for sincos. For sin, > the 2nd branch of the > switch is taken. > > How can I correlate stmt in the source line above to the relevant line in any > dump among those created > using debugging dump option -fdump-tree-all? grep ;) Can you provide a testcase with a simd attribute annotated cexpi that one can play with? Richard. > > Thanks. > Bert.
Re: Commit messages and the move to git
On Wed, 4 Dec 2019 at 23:52, Richard Earnshaw (lists) wrote: > I've just pushed a new trial conversion: > > https://gitlab.com/rearnsha/gcc-trial2-20191130 > > The main differences between this and the previous trial are: > - The author attributions should now be fixed, please let me know if you > see any anomalies in this respect. > - the emptycommit-* tags/branches are now gone. > - the 'tags' used for revert and backport now use more gittish style > revert: and backport: > - the log entries for c++ style functions containing :: are now handled > correctly by my summary generation script. > > Other issues are still being worked on. There's a bogus libstdc++ directory at the top level. That hasn't been present for nearly 20 years, so some commit that should have deleted it seems to be missing or incorrect.
Re: Commit messages and the move to git
On Thu, 5 Dec 2019 at 10:25, Jonathan Wakely wrote: > > On Wed, 4 Dec 2019 at 23:52, Richard Earnshaw (lists) wrote: > > I've just pushed a new trial conversion: > > > > https://gitlab.com/rearnsha/gcc-trial2-20191130 > > > > The main differences between this and the previous trial are: > > - The author attributions should now be fixed, please let me know if you > > see any anomalies in this respect. > > - the emptycommit-* tags/branches are now gone. > > - the 'tags' used for revert and backport now use more gittish style > > revert: and backport: > > - the log entries for c++ style functions containing :: are now handled > > correctly by my summary generation script. > > > > Other issues are still being worked on. > > There's a bogus libstdc++ directory at the top level. That hasn't been > present for nearly 20 years, so some commit that should have deleted > it seems to be missing or incorrect. It was removed by r39445 i.e. this commit in the current Git mirror: commit 1030d23bf251914e12fc8c9d521e334e1f445afc Author: mmitchel Date: Mon Feb 5 01:38:47 2001 + Remove V2 C++ library. * configure.in: Remove --enable-libstdcxx_v3 support. * Makefile.in (site.exp): Always set HAVE_LIBSTDCXX_V3. * configure.in: Remove --enable-libstdcxx_v3 support. * configure: Regenerated. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@39445 138bc75d-0d04-0410-961f-82ee72b054a4
Re: Commit messages and the move to git
On 05/12/2019 10:32, Jonathan Wakely wrote: On Thu, 5 Dec 2019 at 10:25, Jonathan Wakely wrote: On Wed, 4 Dec 2019 at 23:52, Richard Earnshaw (lists) wrote: I've just pushed a new trial conversion: https://gitlab.com/rearnsha/gcc-trial2-20191130 The main differences between this and the previous trial are: - The author attributions should now be fixed, please let me know if you see any anomalies in this respect. - the emptycommit-* tags/branches are now gone. - the 'tags' used for revert and backport now use more gittish style revert: and backport: - the log entries for c++ style functions containing :: are now handled correctly by my summary generation script. Other issues are still being worked on. There's a bogus libstdc++ directory at the top level. That hasn't been present for nearly 20 years, so some commit that should have deleted it seems to be missing or incorrect. It was removed by r39445 i.e. this commit in the current Git mirror: commit 1030d23bf251914e12fc8c9d521e334e1f445afc Author: mmitchel Date: Mon Feb 5 01:38:47 2001 + Remove V2 C++ library. * configure.in: Remove --enable-libstdcxx_v3 support. * Makefile.in (site.exp): Always set HAVE_LIBSTDCXX_V3. * configure.in: Remove --enable-libstdcxx_v3 support. * configure: Regenerated. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@39445 138bc75d-0d04-0410-961f-82ee72b054a4 Ok, this is one to keep an eye on. There are a number of anomalous commmits at present, which Eric is working on with a new approach to replaying the SVN data into reposurgeon. Once that is done we're hoping that this sort of problem will go away. (of course, if it doesn't then the chances are we won't be using reposurgeon for the conversion at all). This repo conversion is really for checking that the summaries I'm generating are not just garbled, and the authors problem from the previous trial are now fixed. R.
Re: Commit messages and the move to git
On Thu, 5 Dec 2019 at 10:36, Richard Earnshaw (lists) wrote: > > On 05/12/2019 10:32, Jonathan Wakely wrote: > > On Thu, 5 Dec 2019 at 10:25, Jonathan Wakely wrote: > >> > >> On Wed, 4 Dec 2019 at 23:52, Richard Earnshaw (lists) wrote: > >>> I've just pushed a new trial conversion: > >>> > >>> https://gitlab.com/rearnsha/gcc-trial2-20191130 > >>> > >>> The main differences between this and the previous trial are: > >>> - The author attributions should now be fixed, please let me know if you > >>> see any anomalies in this respect. > >>> - the emptycommit-* tags/branches are now gone. > >>> - the 'tags' used for revert and backport now use more gittish style > >>> revert: and backport: > >>> - the log entries for c++ style functions containing :: are now handled > >>> correctly by my summary generation script. > >>> > >>> Other issues are still being worked on. > >> > >> There's a bogus libstdc++ directory at the top level. That hasn't been > >> present for nearly 20 years, so some commit that should have deleted > >> it seems to be missing or incorrect. > > > > It was removed by r39445 i.e. this commit in the current Git mirror: > > > > commit 1030d23bf251914e12fc8c9d521e334e1f445afc > > Author: mmitchel > > Date: Mon Feb 5 01:38:47 2001 + > > > > Remove V2 C++ library. > > * configure.in: Remove --enable-libstdcxx_v3 support. > > > > * Makefile.in (site.exp): Always set HAVE_LIBSTDCXX_V3. > > * configure.in: Remove --enable-libstdcxx_v3 support. > > * configure: Regenerated. > > > > > > git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@39445 > > 138bc75d-0d04-0410-961f-82ee72b054a4 > > > > Ok, this is one to keep an eye on. There are a number of anomalous > commmits at present, which Eric is working on with a new approach to > replaying the SVN data into reposurgeon. Once that is done we're hoping > that this sort of problem will go away. (of course, if it doesn't then > the chances are we won't be using reposurgeon for the conversion at all). OK. There are several other deletions missing, as I see these in your conversion which aren't on trunk (paths relative to the libstdc++-v3 dir because that's all I'm checking): Only in ./config/abi/post: solaris2.8 Only in ./config/os: irix Only in ./config/os: osf Only in ./config/os/solaris: solaris2.8 Only in ./include/ext/pb_ds/detail/cc_hash_table_map_: standard_policies.hpp Only in ./include/ext/pb_ds/detail/gp_hash_table_map_: standard_policies.hpp Only in ./src/c++98: compatibility-parallel_list-2.cc Only in ./src/c++98: compatibility-parallel_list.cc Only in ./testsuite/20_util: is_explicitly_convertible Only in ./testsuite/26_numerics: cmath Only in ./testsuite/28_regex/algorithms: 02_match Only in ./testsuite/28_regex/basic_regex: regex.cc Only in ./testsuite/28_regex/headers: 04_header > This repo conversion is really for checking that the summaries I'm > generating are not just garbled, and the authors problem from the > previous trial are now fixed. OK, apart from the artefacts caused by me not knowing my own email address (discussed off-list), those look good to me.
Re: Commit messages and the move to git
On Thu, 5 Dec 2019 at 10:41, Jonathan Wakely wrote: > > On Thu, 5 Dec 2019 at 10:36, Richard Earnshaw (lists) > wrote: > > > > On 05/12/2019 10:32, Jonathan Wakely wrote: > > > On Thu, 5 Dec 2019 at 10:25, Jonathan Wakely > > > wrote: > > >> > > >> On Wed, 4 Dec 2019 at 23:52, Richard Earnshaw (lists) wrote: > > >>> I've just pushed a new trial conversion: > > >>> > > >>> https://gitlab.com/rearnsha/gcc-trial2-20191130 > > >>> > > >>> The main differences between this and the previous trial are: > > >>> - The author attributions should now be fixed, please let me know if you > > >>> see any anomalies in this respect. > > >>> - the emptycommit-* tags/branches are now gone. > > >>> - the 'tags' used for revert and backport now use more gittish style > > >>> revert: and backport: > > >>> - the log entries for c++ style functions containing :: are now handled > > >>> correctly by my summary generation script. > > >>> > > >>> Other issues are still being worked on. > > >> > > >> There's a bogus libstdc++ directory at the top level. That hasn't been > > >> present for nearly 20 years, so some commit that should have deleted > > >> it seems to be missing or incorrect. > > > > > > It was removed by r39445 i.e. this commit in the current Git mirror: > > > > > > commit 1030d23bf251914e12fc8c9d521e334e1f445afc > > > Author: mmitchel > > > Date: Mon Feb 5 01:38:47 2001 + > > > > > > Remove V2 C++ library. > > > * configure.in: Remove --enable-libstdcxx_v3 support. > > > > > > * Makefile.in (site.exp): Always set HAVE_LIBSTDCXX_V3. > > > * configure.in: Remove --enable-libstdcxx_v3 support. > > > * configure: Regenerated. > > > > > > > > > git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@39445 > > > 138bc75d-0d04-0410-961f-82ee72b054a4 > > > > > > > Ok, this is one to keep an eye on. There are a number of anomalous > > commmits at present, which Eric is working on with a new approach to > > replaying the SVN data into reposurgeon. Once that is done we're hoping > > that this sort of problem will go away. (of course, if it doesn't then > > the chances are we won't be using reposurgeon for the conversion at all). > > OK. There are several other deletions missing, as I see these in your > conversion which aren't on trunk (paths relative to the libstdc++-v3 > dir because that's all I'm checking): > > Only in ./config/abi/post: solaris2.8 Should have been removed by r185392 which is present as ec7b5a78e90a4245792f8d9a9288e9f98f904f87 > Only in ./config/os: irix Should have been removed by r185390 aka 9cf50112d877a9b35faa68889ab5ba4ee73291b2 > Only in ./config/os: osf Should have been removed by r185240 aka ed3aca91ef9c7f81b7f7a66ae0b936fde69d8b63 > Only in ./include/ext/pb_ds/detail/cc_hash_table_map_: standard_policies.hpp > Only in ./include/ext/pb_ds/detail/gp_hash_table_map_: standard_policies.hpp Both removed by r194107 which is missing. > Only in ./src/c++98: compatibility-parallel_list-2.cc > Only in ./src/c++98: compatibility-parallel_list.cc Both renamed by r191837 aka 6099a280b4fceb232da81feb92f95b3d889f29c9 > Only in ./testsuite/20_util: is_explicitly_convertible Removed by r186617 which is missing. > Only in ./testsuite/26_numerics: cmath Renamed by r186445 aka 99686a77aa36b93313ae29e4fffd2ae3ed7b549e > Only in ./testsuite/28_regex/algorithms: 02_match Removed by r188923 aka 7202cd20761d4dee77cdbcb677b0b667c83ddea2 > Only in ./testsuite/28_regex/basic_regex: regex.cc Renamed by r192146 aka b70e38df276382a62d4dc13adf0751f435cc990b > > This repo conversion is really for checking that the summaries I'm > > generating are not just garbled, and the authors problem from the > > previous trial are now fixed. > > OK, apart from the artefacts caused by me not knowing my own email > address (discussed off-list), those look good to me.
Re: Possible Bugs in cgraphunit.c
On 12/5/19 9:00 AM, Nicholas Krause wrote: Greetings, Seems that the extend_trucks return values are not returned when called in both, cnode::assemble_thunks_and_aliases and cnode::create_wrapper. I'm not sure if this is a set of edge case bugs or there was a reason for this. Seems not as its checked in the third function caller in the file, cgraph_node::analyze. Hello. You are right that the return value of expand_thunk is not checked (except one place). The code is quite old, so I guess it's not causing issues. Martin Not sure if my assumptions are correct so I'm asking if there isn't another reason for this as the code seems to have at least directly no reason for it, Nick
Re: Commit messages and the move to git
On Thu, 5 Dec 2019, Richard Earnshaw (lists) wrote: > Ok, this is one to keep an eye on. There are a number of anomalous commmits > at present, which Eric is working on with a new approach to replaying the SVN > data into reposurgeon. Once that is done we're hoping that this sort of > problem will go away. (of course, if it doesn't then the chances are we won't > be using reposurgeon for the conversion at all). I think we currently have the following reposurgeon issues open for cases where the present code results in incorrect tree contents and we're hoping the new code will fix that (or make it much easier to find and fix the bugs). These are the issues that are most critical for being able to use reposurgeon for the conversion. https://gitlab.com/esr/reposurgeon/issues/167 https://gitlab.com/esr/reposurgeon/issues/171 https://gitlab.com/esr/reposurgeon/issues/172 https://gitlab.com/esr/reposurgeon/issues/178 -- Joseph S. Myers jos...@codesourcery.com
Re: Branch and tag deletions
Joseph Myers : > The avoidance of '.' in branch and tag names is, I'm pretty sure, a legacy > of CVS restrictions on valid names for branches and tags. Those > restrictions are not relevant to git or SVN; if picking any new convention > it seems appropriate for the tag for GCC 10.1 to say "10.1" somewhere in > its name rather than "10_1". That is correct. I recommend mapping tags from using "_" to using ".", they're just plain more readable that way. I have done this in previous conversions. -- http://www.catb.org/~esr/";>Eric S. Raymond
GCC conversion work in progress
Those of you with a direct interest in the conversion might want to watch #reposurgeon on freenode. This is where Daniel Brooks, Julien Rivaud and I are working on it. Here's where the code lives: reposurgeon: https://gitlab.com/esr/reposurgeon The conversion recipe: https://gitlab.com/esr/gcc-conversion In the next few days I expect the remaining problems to move from nechanism to policy choices. At that point, broader review of the recipe and the conversion progress starts to become desirable. -- http://www.catb.org/~esr/";>Eric S. Raymond "They that can give up essential liberty to obtain a little temporary safety deserve neither liberty nor safety." -- Benjamin Franklin, Historical Review of Pennsylvania, 1759.
Re: Commit messages and the move to git
Richard Earnshaw (lists) : > Ok, this is one to keep an eye on. There are a number of anomalous commmits > at present, which Eric is working on with a new approach to replaying the > SVN data into reposurgeon. Once that is done we're hoping that this sort of > problem will go away. Best case is it just goes away. Worst case is we'll need to figure out what surgical commands need to be patched into the recipe to deal with the remaining anomalies. I suspect the latter, in particular that we're going to end up needing to do something manually around r14877. Iy might be a trivial tweak to the splice command I commented out. -- http://www.catb.org/~esr/";>Eric S. Raymond
[RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)
Hi! ;-P Jakub, thanks for furnishing me a fit occasion here: On 2019-12-05T16:15:15+0100, Jakub Jelinek wrote: > [...] much more indented though, but you could > use a temporary, like: > tree nullarg = null_pointer_node; I object to cluttering the code by introducing temporary variables/names just for the sake of a few characters of screen width. Even if located close lexically, when reading the following code you still have to trace back from the 'nullarg' usage to its 'null_pointer_node' definition in order to figure out what a 'nullarg' might be: > if (present) > ptr > = gfc_build_conditional_assign_expr (block, present, > ptr, nullarg); > Another option would be shorten the name of the function, say > s/conditional/cond/. Likewise I object to "crippling" identifier names like that just for the sake of a few characters of screen width. (Here of course, "cond", or the existing "expr" might be fine abbreviations, but my point is about the general case.) > There were some discussions about lifting the 80 column restriction and bump > it to something like +-130, but nothing happened yet. Indeed. :-) In the relevant session at the GNU Tools Cauldron 2019, Michael Meissner stated that even he is not using a 80 x 24 terminal anymore, and that should tell us something. ;-) So, I formally propose that we lift this characters per line restriction from IBM punch card (80) to mainframe line printer (132). Nonwithstanding that, we should try to not overstrain that; deep indentation often is a sign that code should be split out into a separate function, for example. My point is just to avoid things like the two examples cited above. Also, I'm not proposing any mass-reformatting of the existing code, or re-writing all "expr" into "expression". Tasks: - Discussion. - Get agreement/make a decision (by means still to be determined). - Put suitable Emacs/Vim configuration files into the source tree? - Update coding style guidelines. Grüße Thomas signature.asc Description: PGP signature
Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)
On Thu, Dec 05, 2019 at 04:46:45PM +0100, Thomas Schwinge wrote:
> Hi!
>
> ;-P Jakub, thanks for furnishing me a fit occasion here:
>
> On 2019-12-05T16:15:15+0100, Jakub Jelinek wrote:
> > [...] much more indented though, but you could
> > use a temporary, like:
> > tree nullarg = null_pointer_node;
>
> I object to cluttering the code by introducing temporary variables/names
> just for the sake of a few characters of screen width. Even if located
> close lexically, when reading the following code you still have to trace
> back from the 'nullarg' usage to its 'null_pointer_node' definition in
> order to figure out what a 'nullarg' might be:
>
> > if (present)
> > ptr
> > = gfc_build_conditional_assign_expr (block, present,
> >ptr, nullarg);
>
> > Another option would be shorten the name of the function, say
> > s/conditional/cond/.
>
> Likewise I object to "crippling" identifier names like that just for the
> sake of a few characters of screen width. (Here of course, "cond", or
> the existing "expr" might be fine abbreviations, but my point is about
> the general case.)
The point about temporaries is general, and I believe they actually make
code much more readable. Mostly about coding style like:
t = fold_build2_loc (loc, code, fold_build2_loc (loc, code2,
something1,
something2),
fold_build2_loc (loc, code3, something3,
something4));
vs.
tree op1 = fold_build2_loc (loc, code2, something1, something2);
tree op2 = fold_build2_loc (loc, code3, something3, something4);
t = fold_build2_loc (loc, code, op1, op2);
The above case is extreme in both being indented quite a lot (general rule
is to consider outlining something into a function then) and using
way too long function names. If you look at the earlier suggestion where
the code is indented reasonably, using the temporary there makes the code more
readable and shorter.
>
> > There were some discussions about lifting the 80 column restriction and bump
> > it to something like +-130, but nothing happened yet.
>
> Indeed. :-)
>
> In the relevant session at the GNU Tools Cauldron 2019, Michael Meissner
> stated that even he is not using a 80 x 24 terminal anymore, and that
> should tell us something. ;-)
>
> So, I formally propose that we lift this characters per line restriction
> from IBM punch card (80) to mainframe line printer (132).
Such a proposal would need to be accompanied with a wwwdocs
codingconventions.html patch and contrib/check_GNU_style.{sh,py} patch I
guess ;)
Jakub
Re: Possible Bugs in cgraphunit.c
On 12/5/19 7:08 AM, Martin Liška wrote: On 12/5/19 9:00 AM, Nicholas Krause wrote: Greetings, Seems that the extend_trucks return values are not returned when called in both, cnode::assemble_thunks_and_aliases and cnode::create_wrapper. I'm not sure if this is a set of edge case bugs or there was a reason for this. Seems not as its checked in the third function caller in the file, cgraph_node::analyze. Hello. You are right that the return value of expand_thunk is not checked (except one place). The code is quite old, so I guess it's not causing issues. Martin Indeed or the edge cases are almost never hit or not at all. So the question is should we just change the function to return void as its always successful or fix up the callers? Nick Not sure if my assumptions are correct so I'm asking if there isn't another reason for this as the code seems to have at least directly no reason for it, Nick
Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)
On Thu, 5 Dec 2019, Thomas Schwinge wrote: > In the relevant session at the GNU Tools Cauldron 2019, Michael Meissner > stated that even he is not using a 80 x 24 terminal anymore, and that > should tell us something. ;-) > > So, I formally propose that we lift this characters per line restriction > from IBM punch card (80) to mainframe line printer (132). I thought these line lengths were based on readability studies suggesting lengths that lines shorter than 80 columns were more readable? Longer lines mean less space for multiple terminal / editor windows side-by-side to look at different pieces of code. I don't think that's an improvement. -- Joseph S. Myers jos...@codesourcery.com
Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)
> On Dec 5, 2019, at 11:17 AM, Joseph Myers wrote: > > On Thu, 5 Dec 2019, Thomas Schwinge wrote: > >> In the relevant session at the GNU Tools Cauldron 2019, Michael Meissner >> stated that even he is not using a 80 x 24 terminal anymore, and that >> should tell us something. ;-) >> >> So, I formally propose that we lift this characters per line restriction >> from IBM punch card (80) to mainframe line printer (132). > > I thought these line lengths were based on readability studies suggesting > lengths that lines shorter than 80 columns were more readable? That's certainly a general rule. There is a reason why books aren't wide, and why newspapers have columns. The eye can't deal well with long lines. So while 132 column lines are certainly possible with modern computers, it doesn't mean they are desirable. paul
Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)
On 12/5/19 9:24 AM, Paul Koning wrote: > > >> On Dec 5, 2019, at 11:17 AM, Joseph Myers >> wrote: >> >> On Thu, 5 Dec 2019, Thomas Schwinge wrote: >> >>> In the relevant session at the GNU Tools Cauldron 2019, Michael >>> Meissner stated that even he is not using a 80 x 24 terminal >>> anymore, and that should tell us something. ;-) >>> >>> So, I formally propose that we lift this characters per line >>> restriction from IBM punch card (80) to mainframe line printer >>> (132). >> >> I thought these line lengths were based on readability studies >> suggesting lengths that lines shorter than 80 columns were more >> readable? > > That's certainly a general rule. There is a reason why books aren't > wide, and why newspapers have columns. The eye can't deal well with > long lines. So while 132 column lines are certainly possible with > modern computers, it doesn't mean they are desirable. I'd like to see the restriction relaxed. THe 80 column limit really presents readability problems and excessive expression wrapping to accommodate the limit. 132 seems like a very reasonable compromise. My biggest worry with moving to 132 columns is that it will discourage refactoring when indention levels cause excessive wrapping. Jeff
Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)
Hello, (oh a flame bait :) ) On Thu, 5 Dec 2019, Thomas Schwinge wrote: > So, I formally propose that we lift this characters per line restriction > from IBM punch card (80) to mainframe line printer (132). > > Tasks: > > - Discussion. I object to cluttering code in excuse for using sensible function names or temporaries that otherwise can help clearing up code. Using 132-char lines is cluttering code: - long lines are harder to read/grasp: vertical eye movement is easier than horizontal, and source code should be optimized for reading, not writing - long lines make it impossible to have two files next to each other at a comfortable font size - long lines are incompatible with existing netiquette re emails, for instance So, at least for me, that my terminals are 80 wide (but not x24) has multiple reasons, and the _least_ of it is because that's what punch cards had. Ciao, Michael.
Re: [RFC] Characters per line: from punch card (80) to line printer (132)
* Paul Koning: > That's certainly a general rule. There is a reason why books aren't > wide, and why newspapers have columns. The eye can't deal well with > long lines. So while 132 column lines are certainly possible with > modern computers, it doesn't mean they are desirable. If the line starts at column 40 or so, I don't think readability suffers too much if it goes to column 100. If it starts at column 2, then it might be problematic. Frequent long lines reduce the usefulness of side-by-side diff viewers. And there are those of us who use screens in portrait mode, following the rule that a good function is not longer than a single screen. 1080 pixels give you 8 pixels per character, which isn't that much. Thanks, Florian
Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)
On Thu, 5 Dec 2019 at 16:44, Michael Matz wrote: > > Hello, > > (oh a flame bait :) ) > > On Thu, 5 Dec 2019, Thomas Schwinge wrote: > > > So, I formally propose that we lift this characters per line restriction > > from IBM punch card (80) to mainframe line printer (132). > > > > Tasks: > > > > - Discussion. > > I object to cluttering code in excuse for using sensible function names or > temporaries that otherwise can help clearing up code. Using 132-char > lines is cluttering code: > - long lines are harder to read/grasp: vertical eye movement is easier > than horizontal, and source code should be optimized for > reading, not writing > - long lines make it impossible to have two files next to each other at a > comfortable font size > - long lines are incompatible with existing netiquette re emails, for > instance > > So, at least for me, that my terminals are 80 wide (but not x24) has > multiple reasons, and the _least_ of it is because that's what punch cards > had. C++17 introduces a nice feature, with rationale similar to declaring variables in a for-loop init-statement: if (auto var = foo(); bar(var)) The variable is only in scope for the block where you need it, just like a for-loop. Unfortunately nearly every time I've tried to use this recently, I've found it's impossible in 80 columns, e.g. this from yesterday: if (auto __c = __builtin_memcmp(&*__first1, &*__first2, __len) <=> 0; __c != 0) return __c; When you're forced to uglify every variable with a leading __ you run out of characters pretty damn quickly. I can either not use the feature (and have the variable defined in a larger scope than it needs to be) or add fairly arbitrary line breaks: if (auto __c = __builtin_memcmp(&*__first1, &*__first2, __len) <=> 0; __c != 0) return __c; or try to give the variables shorter (and less meaningful) names. Adding line breaks or picking shorter names doesn't help readability. So I end up not using the feature. I'm loosely in favour of relaxing the rule for libstdc++ code. I don't really care what the rest of GCC looks like ;-)
Re: Commit messages and the move to git
Joseph Myers : > I think we currently have the following reposurgeon issues open for cases > where the present code results in incorrect tree contents and we're hoping > the new code will fix that (or make it much easier to find and fix the > bugs). These are the issues that are most critical for being able to use > reposurgeon for the conversion. > > https://gitlab.com/esr/reposurgeon/issues/167 > https://gitlab.com/esr/reposurgeon/issues/171 > https://gitlab.com/esr/reposurgeon/issues/172 > https://gitlab.com/esr/reposurgeon/issues/178 I'm aware these are the real blockers. I was much more worried about the conversion before we figured out that most of the remaining content mismatches seem to radiate out from something weird that happened at r14877. That's early enough that a leading-segment load including it doesn't take forever. Which means it's practical to do detailed forensics on the defect even if you don't have handy an EC12 instance with ridiculo-humongous amonts of RAM. Now I'm pretty certain we can finish this. A matter of when, not if. -- http://www.catb.org/~esr/";>Eric S. Raymond
Re: Commit messages and the move to git
On Thu, 5 Dec 2019, Eric S. Raymond wrote: > I was much more worried about the conversion before we figured out > that most of the remaining content mismatches seem to radiate out from > something weird that happened at r14877. That's early enough that a > leading-segment load including it doesn't take forever. Which means > it's practical to do detailed forensics on the defect even if you don't > have handy an EC12 instance with ridiculo-humongous amonts of RAM. I just tried a leading-segment load up to r14877, but it didn't reproduce the problems I see with r14877 in a full repository conversion - it seems the combination with something later in the history may be necessary to reproduce the issue. (With trunk-deletion-and-recreation being an obvious candidate, since *some* content problems definitely first appear at such commits.) If it's necessary to bisect to find what leading segment produces that problem (if it still appears with the new SVN dump reader), obviously things can be speeded up for the bisection by omitting slow things such as the deletion of emptycommit tags (a minimal gcc.lift makes sense for such a bisection anyway). -- Joseph S. Myers jos...@codesourcery.com
Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)
On Dec 5 2019, Michael Matz wrote: (oh a flame bait :) ) Quite. I shall try not to make it too much worse, but there's another point that needs mentioning. I find long names hard to read, with either short or long lines, especially when combined with variants like negotiate_twisty_little_passage, and negotiate_little_twisty_passage. And extending line lengths will probably encourage the use of longer names. As people say, there are conficting requirements, but I side with you in preferring 80 column lines. Actually, I tend to use less, but that's because of the (still current) frequency with which utilities make a mess of lines exactly 80 columns long in an 80 column field. Regards, Nick Maclaren.
Re: Commit messages and the move to git
Joseph Myers : > I just tried a leading-segment load up to r14877, but it didn't reproduce > the problems I see with r14877 in a full repository conversion - it seems > the combination with something later in the history may be necessary to > reproduce the issue. Great :-( Well, there's a bisection-like strategy for finding the minimum leading segment that produces misbehavior. My conversion crew will apply it as hard as we need to to get the job done. -- http://www.catb.org/~esr/";>Eric S. Raymond
Re: PPC64 libmvec implementation of sincos
‐‐‐ Original Message ‐‐‐
On Thursday, December 5, 2019 4:44 AM, Richard Biener
wrote:
...
...
...
> >
> > I'm trying to identify the source code which needs modification but I need
> > help proceeding.
> > I am comparing two compilations: The first is a simple file with a call to
> > sin in a loop.
> > Vectorization succeeds. The second is an almost identical file but with a
> > call to sincos
> > in the loop. Vectorization fails.
> > In gdb, the earliest code location where the two compilations differ is in
> > function
> > number_of_iterations_exit_assumptions in file tree-ssa-loop-niter.c. Line
> > op0 = gimple_cond_lhs (stmt);
> > returns a tree which when analyzed in function instantiate_scev_r (in file
> > tree-scalar-evolution.c)
> > results in the first branch of the switch being taken for sincos. For sin,
> > the 2nd branch of the
> > switch is taken.
> > How can I correlate stmt in the source line above to the relevant line in
> > any dump among those created
> > using debugging dump option -fdump-tree-all?
>
> grep ;)
>
> Can you provide a testcase with a simd attribute annotated cexpi that
> one can play with?
>
On an x86_64 system, run Example 2 at this link:
sourceware.org/glibc/wiki/libmvec
After verifying vectorization (by finding a name with prefix _ZGV and suffix
_sin in a.out), replace
the call to sin by one to sincos. The file should be similar to this:
#include
int N = 3200;
double c[3200];
double b[3200];
double a[3200];
int main (void)
{
int i;
for (i = 0; i < N; i += 1)
{
sincos (a[i], &b[i], &c[i]);
}
return (0);
}
In addition to the options shown in Example 2, I passed GCC flags
-fopt-info-all, -fopt-info-internal and
-fdump-tree-all to obtain more verbose messages.
That should show vectorization failing for sincos, and diagnostics on the
screen indicating reason(s) for
the failure.
To perform the runs on PPC64 requires building both GCC and GLIBC with
modifications not yet accepted
into the main development branches of the projects.
Please let me know if you are able to run on x86_64; if not, then perhaps I can
push the local GCC
changes to some github repository. GLIBC changes are available at branch
tuliom/libmvec of the
development repository.
Bert.
Re: [RFC] Characters per line: from punch card (80) to line printer (132)
On 05/12/2019 16:17, Joseph Myers wrote: Longer lines mean less space for multiple terminal / editor windows side-by-side to look at different pieces of code. I don't think that's an improvement. Here's a data-point My 1920 pixel-wide screen, in the default font, allows 239 columns; not enough for two 130-wide editors. Especially not with line numbers and "gutter" columns. On the other hand, 80 columns does tend to cause some formatting contortions, with long function names and deeper indentations. I think a nice round 100 would be a good compromise. Andrew
Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)
On Thu, Dec 05, 2019 at 05:03:43PM +, Jonathan Wakely wrote: > On Thu, 5 Dec 2019 at 16:44, Michael Matz wrote: > > > > Hello, > > > > (oh a flame bait :) ) > > > > On Thu, 5 Dec 2019, Thomas Schwinge wrote: > > > > > So, I formally propose that we lift this characters per line restriction > > > from IBM punch card (80) to mainframe line printer (132). > > > > > > Tasks: > > > > > > - Discussion. > > > > I object to cluttering code in excuse for using sensible function names or > > temporaries that otherwise can help clearing up code. Using 132-char > > lines is cluttering code: > > - long lines are harder to read/grasp: vertical eye movement is easier > > than horizontal, and source code should be optimized for > > reading, not writing > > - long lines make it impossible to have two files next to each other at a > > comfortable font size > > - long lines are incompatible with existing netiquette re emails, for > > instance > > > > So, at least for me, that my terminals are 80 wide (but not x24) has > > multiple reasons, and the _least_ of it is because that's what punch cards > > had. > > C++17 introduces a nice feature, with rationale similar to declaring > variables in a for-loop init-statement: > > if (auto var = foo(); bar(var)) > > The variable is only in scope for the block where you need it, just > like a for-loop. > > Unfortunately nearly every time I've tried to use this recently, I've > found it's impossible in 80 columns, e.g. this from yesterday: > > if (auto __c = __builtin_memcmp(&*__first1, &*__first2, __len) <=> > 0; __c != 0) > return __c; > > When you're forced to uglify every variable with a leading __ you run > out of characters pretty damn quickly. > > I can either not use the feature (and have the variable defined in a > larger scope than it needs to be) or add fairly arbitrary line breaks: > > if (auto __c > = __builtin_memcmp(&*__first1, &*__first2, __len) > <=> 0; __c != 0) > return __c; > > or try to give the variables shorter (and less meaningful) names. > Adding line breaks or picking shorter names doesn't help readability. > So I end up not using the feature. > > I'm loosely in favour of relaxing the rule for libstdc++ code. I don't > really care what the rest of GCC looks like ;-) Not using such a nice feature just because of formatting sounds really shameful. Would the compromise of 100 chars make things any better here? Marek
Re: [RFC] Characters per line: from punch card (80) to line printer (132)
On 12/5/19, Andrew Stubbs wrote: > On 05/12/2019 16:17, Joseph Myers wrote: >> Longer lines mean less space for multiple terminal / editor windows >> side-by-side to look at different pieces of code. I don't think that's >> an >> improvement. > > Here's a data-point > > My 1920 pixel-wide screen, in the default font, allows 239 columns; not > enough for two 130-wide editors. Especially not with line numbers and > "gutter" columns. > > On the other hand, 80 columns does tend to cause some formatting > contortions, with long function names and deeper indentations. > > I think a nice round 100 would be a good compromise. Here's mine: My 1280 pixel-wide screen allows 179 columns, which comes to 89.5 columns when divided by 2. I think rounding up to 90 columns would be a good compromise, although if that's too small, 100 is good as well. > > Andrew >
Re: [RFC] Characters per line: from punch card (80) to line printer (132)
My IBM Selectric golfball electronic printer only does 90 characters on A4 in portrait mode………(at 10 cps) (as for my all electric TELEX Teleprinter machine !) Is this debate for real ?! - or is this a Christmas spoof ? External observer…..keep up the great work. :) (while I punch out a few more 80 column cards). > On 5 Dec 2019, at 17:55, Andrew Stubbs wrote: > > On 05/12/2019 16:17, Joseph Myers wrote: >> Longer lines mean less space for multiple terminal / editor windows >> side-by-side to look at different pieces of code. I don't think that's an >> improvement. > > Here's a data-point > > My 1920 pixel-wide screen, in the default font, allows 239 columns; not > enough for two 130-wide editors. Especially not with line numbers and > "gutter" columns. > > On the other hand, 80 columns does tend to cause some formatting contortions, > with long function names and deeper indentations. > > I think a nice round 100 would be a good compromise. > > Andrew
Re: [RFC] Characters per line: from punch card (80) to line printer (132)
On 12/5/19 8:46 AM, Thomas Schwinge wrote: Hi! ;-P Jakub, thanks for furnishing me a fit occasion here: On 2019-12-05T16:15:15+0100, Jakub Jelinek wrote: [...] much more indented though, but you could use a temporary, like: tree nullarg = null_pointer_node; I object to cluttering the code by introducing temporary variables/names just for the sake of a few characters of screen width. Even if located close lexically, when reading the following code you still have to trace back from the 'nullarg' usage to its 'null_pointer_node' definition in order to figure out what a 'nullarg' might be: if (present) ptr = gfc_build_conditional_assign_expr (block, present, ptr, nullarg); The snippet of code above looks like it might be the symptom of another common problem: deeply nested conditionals, case statements, or loops in very large functions. Those usually make it much harder to follow code than local variables or expressions that are broken up to fit the width limit. Shorter functions typically also means fewer local variables. One thing I find improves readability in functions with many local variables is declaring const those that don't change after initialization. That also enforces the initialization- on-declaration coding style, and can result in more efficient code. Another solution that might help in this context is default function arguments: if the last argument may be null, making it the default in the function declaration avoids having to pass it explicitly. Another option would be shorten the name of the function, say s/conditional/cond/. As long as it doesn't compromise readability this sounds like a good suggestion for a change to the function above. _cond_ is no less clear or descriptive than _conditional_, similarly to _expr vs _expression. Likewise I object to "crippling" identifier names like that just for the sake of a few characters of screen width. (Here of course, "cond", or the existing "expr" might be fine abbreviations, but my point is about the general case.) There were some discussions about lifting the 80 column restriction and bump it to something like +-130, but nothing happened yet. Indeed. :-) In the relevant session at the GNU Tools Cauldron 2019, Michael Meissner stated that even he is not using a 80 x 24 terminal anymore, and that should tell us something. ;-) So, I formally propose that we lift this characters per line restriction from IBM punch card (80) to mainframe line printer (132). I'm not a fan of rigid rules, especially those that are subject to personal style preferences. At the same time I wouldn't like to see lines become as long as this as the norm. As others, I have windows om my desktop arranged in a way to maximize screen real estate: three columns of editor/debugger and a couple of terminals on of top of the other. They only fit because they're all 80 characters wide. But more important: deep indentation often is a sign that code should be split out into a separate function, for example. Exactly. Martin My point is just to avoid things like the two examples cited above. Also, I'm not proposing any mass-reformatting of the existing code, or re-writing all "expr" into "expression". Tasks: - Discussion. - Get agreement/make a decision (by means still to be determined). - Put suitable Emacs/Vim configuration files into the source tree? - Update coding style guidelines. Grüße Thomas
Re: [RFC] Characters per line: from punch card (80) to line printer (132)
All, As a certified Old Guy (coding in FORTRAN since 1967) my default mode is to stop at 72 characters. It would be nice to push the max out a bit, but I’ll most likely keep my lines (and my function and variable names!) short. As always, brevity is the soul of wit. And yes, please keep up the good work. Jim 3222 NE 89th St Seattle, WA 98115 (206) 430-0109 > On Dec 5, 2019, at 10:21 AM, Robin Curtis wrote: > > Is this debate for real ?! - or is this a Christmas spoof ? > > External observer…..keep up the great work. :) > > (while I punch out a few more 80 column cards).
Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)
Hi! On Thu, Dec 05, 2019 at 05:03:43PM +, Jonathan Wakely wrote: > C++17 introduces a nice feature, with rationale similar to declaring > variables in a for-loop init-statement: > > if (auto var = foo(); bar(var)) Similar to GNU C statement expressions, which are *also* only a good idea to use in limited cases. > The variable is only in scope for the block where you need it, just > like a for-loop. > > Unfortunately nearly every time I've tried to use this recently, I've > found it's impossible in 80 columns, e.g. this from yesterday: > > if (auto __c = __builtin_memcmp(&*__first1, &*__first2, __len) <=> > 0; __c != 0) > return __c; > > When you're forced to uglify every variable with a leading __ you run > out of characters pretty damn quickly. If using this "nice feature" forces you to uglify your code, then maybe it is not such a nice feature, and you should not use it. If you have issues with scoping your functions are WAY too long already. Segher
Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)
On Thu, Dec 05, 2019 at 04:44:21PM +, Michael Matz wrote: > (oh a flame bait :) ) I will blissfully ignore that warning. > On Thu, 5 Dec 2019, Thomas Schwinge wrote: > I object to cluttering code in excuse for using sensible function names or > temporaries that otherwise can help clearing up code. Using 132-char > lines is cluttering code: > - long lines are harder to read/grasp: vertical eye movement is easier > than horizontal, and source code should be optimized for > reading, not writing > - long lines make it impossible to have two files next to each other at a > comfortable font size > - long lines are incompatible with existing netiquette re emails, for > instance > > So, at least for me, that my terminals are 80 wide (but not x24) has > multiple reasons, and the _least_ of it is because that's what punch cards > had. I agree with all of this. If you have a hard time writing nicely readable code in 80 columns, you will have a much harder time still using more columns. 80 is somewhat too long already (~60 is better), but we have indents in source code, so that's alright. And you should not indent that far, so this all works out splendidly. Segher
Re: Commit messages and the move to git
On Thu, 5 Dec 2019, Eric S. Raymond wrote: > Joseph Myers : > > I just tried a leading-segment load up to r14877, but it didn't reproduce > > the problems I see with r14877 in a full repository conversion - it seems > > the combination with something later in the history may be necessary to > > reproduce the issue. > > Great :-( > > Well, there's a bisection-like strategy for finding the minimum > leading segment that produces misbehavior. My conversion crew will > apply it as hard as we need to to get the job done. I've now provided a reduced synthetic test (7 commits) for the issue observed at r14877, in issue 172. It wouldn't surprise me if a fix for this synthetic test fixes both issues 171 and 172 (and it wouldn't surprise me if it's fixed in the new SVN dump reader). -- Joseph S. Myers jos...@codesourcery.com
Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)
On Thu, Dec 05, 2019 at 05:04:12PM +0100, Jakub Jelinek wrote: > On Thu, Dec 05, 2019 at 04:46:45PM +0100, Thomas Schwinge wrote: > > On 2019-12-05T16:15:15+0100, Jakub Jelinek wrote: > > > [...] much more indented though, but you could > > > use a temporary, like: > > > tree nullarg = null_pointer_node; > > > > I object to cluttering the code by introducing temporary variables/names > > just for the sake of a few characters of screen width. Even if located > > close lexically, when reading the following code you still have to trace > > back from the 'nullarg' usage to its 'null_pointer_node' definition in > > order to figure out what a 'nullarg' might be: > > > > > if (present) > > > ptr > > > = gfc_build_conditional_assign_expr (block, present, > > > ptr, nullarg); > > > > > Another option would be shorten the name of the function, say > > > s/conditional/cond/. > > > > Likewise I object to "crippling" identifier names like that just for the > > sake of a few characters of screen width. (Here of course, "cond", or > > the existing "expr" might be fine abbreviations, but my point is about > > the general case.) > > The point about temporaries is general, and I believe they actually make > code much more readable. Mostly about coding style like: > t = fold_build2_loc (loc, code, fold_build2_loc (loc, code2, >something1, >something2), >fold_build2_loc (loc, code3, something3, > something4)); > vs. > tree op1 = fold_build2_loc (loc, code2, something1, something2); > tree op2 = fold_build2_loc (loc, code3, something3, something4); > t = fold_build2_loc (loc, code, op1, op2); Yes. And the names, even if they do not say much, *do* say enough to help comprehending the code. They help structure it. > The above case is extreme in both being indented quite a lot (general rule > is to consider outlining something into a function then) I hope you mean actual factoring, not just outlining :-) If you pick good factors you can give them good names, too. Good names help reading the code. And on the other hand, when it is hard to come up with a good name for a piece of code, it is probably not chosen as a good factor anyway! > and using > way too long function names. If you look at the earlier suggestion where > the code is indented reasonably, using the temporary there makes the code more > readable and shorter. Yup. Segher
Re: [RFC] Characters per line: from punch card (80) to line printer (132)
On Thu, Dec 05, 2019 at 11:54:04AM -0700, Martin Sebor wrote: > >> if (present) > >>ptr > >> = gfc_build_conditional_assign_expr (block, > >> present, > >> ptr, nullarg); > > The snippet of code above looks like it might be the symptom > of another common problem: deeply nested conditionals, case > statements, or loops in very large functions. Those usually > make it much harder to follow code than local variables or > expressions that are broken up to fit the width limit. > Shorter functions typically also means fewer local variables. Yes, and you only get problems that you do not know which var is which, or you do not know what value it is set to because it was set some 2800 (or 40 or whatever) lines ago, in WAY too long functions. All those problems do not exist in well-factored code. The point is not to have short routines: the point is to not have too much (external) complexity per routine. A routine should ideally only do one thing, and its name should describe what it does. Segher
Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)
On Thu, Dec 05, 2019 at 02:06:50PM -0600, Segher Boessenkool wrote: > Hi! > > On Thu, Dec 05, 2019 at 05:03:43PM +, Jonathan Wakely wrote: > > C++17 introduces a nice feature, with rationale similar to declaring > > variables in a for-loop init-statement: > > > > if (auto var = foo(); bar(var)) > > Similar to GNU C statement expressions, which are *also* only a good > idea to use in limited cases. > > > The variable is only in scope for the block where you need it, just > > like a for-loop. > > > > Unfortunately nearly every time I've tried to use this recently, I've > > found it's impossible in 80 columns, e.g. this from yesterday: > > > > if (auto __c = __builtin_memcmp(&*__first1, &*__first2, __len) <=> > > 0; __c != 0) > > return __c; > > > > When you're forced to uglify every variable with a leading __ you run > > out of characters pretty damn quickly. > > If using this "nice feature" forces you to uglify your code, then maybe > it is not such a nice feature, and you should not use it. I disagree, it is a nice feature, without quotes. It's Good Style not to leak variables into enclosing scopes. > If you have issues with scoping your functions are WAY too long already. I don't think that's the case here. -- Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)
On Thu, Dec 5, 2019 at 11:51 AM Michael Matz wrote: > Hello, > > (oh a flame bait :) ) > > On Thu, 5 Dec 2019, Thomas Schwinge wrote: > > > So, I formally propose that we lift this characters per line restriction > > from IBM punch card (80) to mainframe line printer (132). > > > > Tasks: > > > > - Discussion. > > I object to cluttering code in excuse for using sensible function names or > temporaries that otherwise can help clearing up code. Using 132-char > lines is cluttering code: > - long lines are harder to read/grasp: vertical eye movement is easier > than horizontal, and source code should be optimized for > reading, not writing > - long lines make it impossible to have two files next to each other at a > comfortable font size > - long lines are incompatible with existing netiquette re emails, for > instance > > So, at least for me, that my terminals are 80 wide (but not x24) has > multiple reasons, and the _least_ of it is because that's what punch cards > had. > Agreed. I work with two side-by-side terminals, one 80x50 and the other as wide as fits in the rest of the screen, which currently happens to be 111x50. Jason
Re: Commit messages and the move to git
On Thu, 5 Dec 2019, Joseph Myers wrote: > On Thu, 5 Dec 2019, Eric S. Raymond wrote: > > > Joseph Myers : > > > I just tried a leading-segment load up to r14877, but it didn't reproduce > > > the problems I see with r14877 in a full repository conversion - it seems > > > the combination with something later in the history may be necessary to > > > reproduce the issue. > > > > Great :-( > > > > Well, there's a bisection-like strategy for finding the minimum > > leading segment that produces misbehavior. My conversion crew will > > apply it as hard as we need to to get the job done. > > I've now provided a reduced synthetic test (7 commits) for the issue > observed at r14877, in issue 172. It wouldn't surprise me if a fix for > this synthetic test fixes both issues 171 and 172 (and it wouldn't > surprise me if it's fixed in the new SVN dump reader). And given the synthetic test I've added to issue 178, I suspect the same problem is behind at least some of the missing file/directory deletions as well. -- Joseph S. Myers jos...@codesourcery.com
Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)
On Thu, 5 Dec 2019 at 20:07, Segher Boessenkool wrote: > > Hi! > > On Thu, Dec 05, 2019 at 05:03:43PM +, Jonathan Wakely wrote: > > C++17 introduces a nice feature, with rationale similar to declaring > > variables in a for-loop init-statement: > > > > if (auto var = foo(); bar(var)) > > Similar to GNU C statement expressions, which are *also* only a good > idea to use in limited cases. > > > The variable is only in scope for the block where you need it, just > > like a for-loop. > > > > Unfortunately nearly every time I've tried to use this recently, I've > > found it's impossible in 80 columns, e.g. this from yesterday: > > > > if (auto __c = __builtin_memcmp(&*__first1, &*__first2, __len) <=> > > 0; __c != 0) > > return __c; > > > > When you're forced to uglify every variable with a leading __ you run > > out of characters pretty damn quickly. > > If using this "nice feature" forces you to uglify your code, then maybe > it is not such a nice feature, and you should not use it. The uglification has absolutely nothing to do with the 'if' init-statement feature, all code in libstdc++ headers has to be uglified, always. Blame the C preprocessor for that, not C++ features. My point is that 80 characters runs out quicker when 10% of it goes on visual noise that's only needed because the C preprocessor means we can't have nice names. > If you have issues with scoping your functions are WAY too long already. I don't have issues with scoping, it's just good practice to limit the scope to the minimum necessary. The example I'm talking about is: https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/include/bits/stl_algobase.h;h=a2fd306e6d0cca579b510148ba1a7089e2b2f3a2;hb=HEAD#l1499 That function is 46 lines long, including a micro-optimisation to use memcmpy when appropriate. About 15% of that is on assertions to help users debug their mistakes. Another five lines are the compile-time if-constexpr checks to decide when the optimisation is appropriate, which result in deep nesting, but not because of any complex if-else branches. It could be nested less deeply to reduce indentation by 4 spaces, but that would result in more template instantiations for users, which would take more time and memory to compile. When you have a million users including the header in every C++ program, little things like that help. So the code is written in an unidiomatic way to optimise for compilation speed, not readability. But the end result is that the call to __min_cmp is indented about 25% of the way into the available space, and that the FIRST line of executable code in the function. The code is not way too long, and writing it differently would compile slower. The constraints that apply to that code are quite different to the internals of GCC, which most users never see or even compile themselves. (Yes, maybe libstdc++ should stop indenting everything by 2 columns when it's inside a namespace, given that almost everything is inside at least one level of namespace ... that might be a good idea even if the column limit is extended to 100 or 132, but we'll still have all the __ prefixes eating up space.)
Re: Commit messages and the move to git
Joseph Myers : > On Thu, 5 Dec 2019, Eric S. Raymond wrote: > > > Joseph Myers : > > > I just tried a leading-segment load up to r14877, but it didn't reproduce > > > the problems I see with r14877 in a full repository conversion - it seems > > > the combination with something later in the history may be necessary to > > > reproduce the issue. > > > > Great :-( > > > > Well, there's a bisection-like strategy for finding the minimum > > leading segment that produces misbehavior. My conversion crew will > > apply it as hard as we need to to get the job done. > > I've now provided a reduced synthetic test (7 commits) for the issue > observed at r14877, in issue 172. It wouldn't surprise me if a fix for > this synthetic test fixes both issues 171 and 172 (and it wouldn't > surprise me if it's fixed in the new SVN dump reader). If not, I think it soon will be. I expect that little synthetic test to help a lot. -- http://www.catb.org/~esr/";>Eric S. Raymond
Re: Commit messages and the move to git
Joseph Myers : > On Thu, 5 Dec 2019, Joseph Myers wrote: > > > On Thu, 5 Dec 2019, Eric S. Raymond wrote: > > > > > Joseph Myers : > > > > I just tried a leading-segment load up to r14877, but it didn't > > > > reproduce > > > > the problems I see with r14877 in a full repository conversion - it > > > > seems > > > > the combination with something later in the history may be necessary to > > > > reproduce the issue. > > > > > > Great :-( > > > > > > Well, there's a bisection-like strategy for finding the minimum > > > leading segment that produces misbehavior. My conversion crew will > > > apply it as hard as we need to to get the job done. > > > > I've now provided a reduced synthetic test (7 commits) for the issue > > observed at r14877, in issue 172. It wouldn't surprise me if a fix for > > this synthetic test fixes both issues 171 and 172 (and it wouldn't > > surprise me if it's fixed in the new SVN dump reader). > > And given the synthetic test I've added to issue 178, I suspect the same > problem is behind at least some of the missing file/directory deletions as > well. Likely, yes. -- http://www.catb.org/~esr/";>Eric S. Raymond
Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)
On Thu, Dec 05, 2019 at 03:38:15PM -0500, Marek Polacek wrote: > On Thu, Dec 05, 2019 at 02:06:50PM -0600, Segher Boessenkool wrote: > > > When you're forced to uglify every variable with a leading __ you run > > > out of characters pretty damn quickly. > > > > If using this "nice feature" forces you to uglify your code, then maybe > > it is not such a nice feature, and you should not use it. > > I disagree, it is a nice feature, without quotes. It's Good Style not to > leak variables into enclosing scopes. It *is* a quote, from Jonathan's mail. Why is this Good Style? (And according to who?) > > If you have issues with scoping your functions are WAY too long already. > > I don't think that's the case here. Then it does not hurt to have a local that is visible in slightly longer scope than necessary. Simpler code is better code. Segher
Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)
On Thu, Dec 05, 2019 at 08:56:35PM +, Jonathan Wakely wrote: > On Thu, 5 Dec 2019 at 20:07, Segher Boessenkool > wrote: > > On Thu, Dec 05, 2019 at 05:03:43PM +, Jonathan Wakely wrote: > > > C++17 introduces a nice feature, with rationale similar to declaring > > > variables in a for-loop init-statement: > > > Unfortunately nearly every time I've tried to use this recently, I've > > > found it's impossible in 80 columns, e.g. this from yesterday: > > > > > > if (auto __c = __builtin_memcmp(&*__first1, &*__first2, __len) <=> > > > 0; __c != 0) > > > return __c; > > > > > > When you're forced to uglify every variable with a leading __ you run > > > out of characters pretty damn quickly. > > > > If using this "nice feature" forces you to uglify your code, then maybe > > it is not such a nice feature, and you should not use it. > > The uglification has absolutely nothing to do with the 'if' > init-statement feature, all code in libstdc++ headers has to be > uglified, always. Blame the C preprocessor for that, not C++ features. > > My point is that 80 characters runs out quicker when 10% of it goes on > visual noise that's only needed because the C preprocessor means we > can't have nice names. (Not sure where the preprocessor comes in, these underscores are just to satisfy language rules afaics, but maybe you mean something else?) Or you could write auto __c = (__builtin_memcmp(&*__first1, &*__first2, __len) <=> 0); if (__c) return __c; which is much easier to read, to my eyes anyway. And it is exactly the same for the compiler. Wrapping everything inside-out just for the artificial goal of having things defined in slightly tighter scopes feels futile. Segher
Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)
On Thu, 5 Dec 2019 at 22:19, Segher Boessenkool wrote: > > On Thu, Dec 05, 2019 at 08:56:35PM +, Jonathan Wakely wrote: > > On Thu, 5 Dec 2019 at 20:07, Segher Boessenkool > > wrote: > > > On Thu, Dec 05, 2019 at 05:03:43PM +, Jonathan Wakely wrote: > > > > C++17 introduces a nice feature, with rationale similar to declaring > > > > variables in a for-loop init-statement: > > > > > Unfortunately nearly every time I've tried to use this recently, I've > > > > found it's impossible in 80 columns, e.g. this from yesterday: > > > > > > > > if (auto __c = __builtin_memcmp(&*__first1, &*__first2, __len) <=> > > > > 0; __c != 0) > > > > return __c; > > > > > > > > When you're forced to uglify every variable with a leading __ you run > > > > out of characters pretty damn quickly. > > > > > > If using this "nice feature" forces you to uglify your code, then maybe > > > it is not such a nice feature, and you should not use it. > > > > The uglification has absolutely nothing to do with the 'if' > > init-statement feature, all code in libstdc++ headers has to be > > uglified, always. Blame the C preprocessor for that, not C++ features. > > > > My point is that 80 characters runs out quicker when 10% of it goes on > > visual noise that's only needed because the C preprocessor means we > > can't have nice names. > > (Not sure where the preprocessor comes in, these underscores are just to > satisfy language rules afaics, but maybe you mean something else?) The language rules are only necessary because of the preprocessor. Users can define macros with names like "pred" and "cmp" before including any standard library header, and because macros don't respect lexical scope, that would break any standard library code using "pred" and "cmp". So the std::lib has to use reserved names. It's entirely due to the preprocessor that we can't use sane names for local variables, or for any implementation detail in a header (unlike in C, our helper types and functions are in a namespace so won't collide with users' types and functions, but we still have to use reserved names because the preprocessor doesn't respect namespaces).
Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)
On Thu, 5 Dec 2019 at 22:19, Segher Boessenkool wrote:
> Or you could write
>
> auto __c = (__builtin_memcmp(&*__first1, &*__first2, __len) <=> 0);
> if (__c)
> return __c;
>
> which is much easier to read, to my eyes anyway. And it is exactly the
> same for the compiler.
In this case yes, but not in general.
Given:
auto x = foo();
if (bar(x))
{ }
some_type y;
The destructor of x won't run until after y has been destroyed. That's
not at all identical to:
if (auto x = foo(); bar(x))
{ }
some_type y;
Please don't try to tell me how C++ works :-)
Re: [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments)
On Thu, Dec 05, 2019 at 10:37:33PM +, Jonathan Wakely wrote:
> On Thu, 5 Dec 2019 at 22:19, Segher Boessenkool wrote:
> > Or you could write
> >
> > auto __c = (__builtin_memcmp(&*__first1, &*__first2, __len) <=> 0);
> > if (__c)
> > return __c;
> >
> > which is much easier to read, to my eyes anyway. And it is exactly the
> > same for the compiler.
>
> In this case yes, but not in general.
>
> Given:
>
> auto x = foo();
> if (bar(x))
> { }
> some_type y;
>
> The destructor of x won't run until after y has been destroyed. That's
> not at all identical to:
>
> if (auto x = foo(); bar(x))
> { }
> some_type y;
>
> Please don't try to tell me how C++ works :-)
I don't, I wouldn't even *know* that. But this is just the same as in C,
and I do know how to write good C code.
I don't think doing non-trivial things with constructors and destructors
(or anything else!) implicitly is a good idea at all, but that's an
altogether different subject.
Segher
