mbox series

[0/6] Add missing calls to `onlyjump_p'

Message ID alpine.LFD.2.21.2012030749370.656242@eddie.linux-mips.org
Headers show
Series Add missing calls to `onlyjump_p' | expand

Message

Maciej W. Rozycki Dec. 3, 2020, 11:34 a.m. UTC
Hi,

 As discussed here: 

<https://gcc.gnu.org/pipermail/gcc-patches/2020-December/560956.html>

here is a small patch series adding missing calls to `onlyjump_p' around 
`any_condjump_p' and `any_uncondjump_p' use where the jump in question is 
about to be removed.

 Note that I have included unrelated though contextually connected 6/6 as 
an RFC to verify whether this potential anomaly I have spotted has been 
intentional.  I'll be happy to drop it if that is the case.  The remaining 
changes are I believe actual bug fixes.

 These changes have been successfully bootstrapped and regression-tested 
with the `powerpc64le-linux-gnu' and `x86_64-linux-gnu' native systems; 
verification with the `vax-netbsdelf' target using `powerpc64le-linux-gnu' 
host has been underway.

 I meant to do size checks across the test suites with the native builds, 
but I forgot that the test framework deletes built test cases after use by 
default.  I have restarted verification now with a modified configuration 
and will have results sometime tomorrow.

  Maciej

Comments

Jeff Law Dec. 3, 2020, 10:06 p.m. UTC | #1
On 12/3/20 4:34 AM, Maciej W. Rozycki wrote:
> Hi,
>
>  As discussed here: 
>
> <https://gcc.gnu.org/pipermail/gcc-patches/2020-December/560956.html>
>
> here is a small patch series adding missing calls to `onlyjump_p' around 
> `any_condjump_p' and `any_uncondjump_p' use where the jump in question is 
> about to be removed.
>
>  Note that I have included unrelated though contextually connected 6/6 as 
> an RFC to verify whether this potential anomaly I have spotted has been 
> intentional.  I'll be happy to drop it if that is the case.  The remaining 
> changes are I believe actual bug fixes.
I doubt it's intentional.  I'd tend to think this specific patch in the
series should wait until gcc-12 out of an abundance of caution.    I've
ACK'd the rest.

Jeff
Maciej W. Rozycki Dec. 8, 2020, 6:28 p.m. UTC | #2
On Thu, 3 Dec 2020, Maciej W. Rozycki wrote:

>  These changes have been successfully bootstrapped and regression-tested 
> with the `powerpc64le-linux-gnu' and `x86_64-linux-gnu' native systems; 
> verification with the `vax-netbsdelf' target using `powerpc64le-linux-gnu' 
> host has been underway.
> 
>  I meant to do size checks across the test suites with the native builds, 
> but I forgot that the test framework deletes built test cases after use by 
> default.  I have restarted verification now with a modified configuration 
> and will have results sometime tomorrow.

 So for the record, `size' has reported no text changes whatsoever with 
the `x86_64-linux-gnu' compilation or the testsuite, which makes me fairly 
confident no code change has resulted.  With `powerpc64le-linux-gnu' there 
were a couple of text size changes across libgo objects, namely these:

powerpc64le-linux-gnu/libgo/cmd/go/internal/modfetch.gox
powerpc64le-linux-gnu/libgo/cmd/go/internal/modfetch.o
powerpc64le-linux-gnu/libgo/cmd/go/internal/modload.gox
powerpc64le-linux-gnu/libgo/cmd/go/internal/modload.o
powerpc64le-linux-gnu/libgo/html/template.gox
powerpc64le-linux-gnu/libgo/html/template.o
powerpc64le-linux-gnu/libgo/net/.libs/http.o

however upon a closer inspection all the differences turned out to be in 
`.go_export' sections, whose flags are "ALLOC, EXCLUDE".  Oddly they are 
considered text due to an obscure change to `size' from years ago:

Mon Jan 20 14:24:04 1997  Ian Lance Taylor  <ian@cygnus.com>

	* size.c (berkeley_sum): Rewrite.  Skip sections which are not
	SEC_ALLOC.  Count SEC_READONLY sections as text.

Switching to trunk binutils and `--format=gnu', added last year, made the 
phenomenon go away.  Sadly among all the wondering around the submission 
here: <https://sourceware.org/ml/binutils/2019-01/msg00260.html> nobody 
thought of actually asking Ian what the motivation for that old change of 
his might have been.

 Ian, it's been a while and the mailing lists carried much less actual 
discussion about changes made, but do you remember by any chance?

  Maciej
Maciej W. Rozycki Dec. 8, 2020, 6:35 p.m. UTC | #3
On Thu, 3 Dec 2020, Jeff Law wrote:

> >  Note that I have included unrelated though contextually connected 6/6 as
> > an RFC to verify whether this potential anomaly I have spotted has been
> > intentional.  I'll be happy to drop it if that is the case.  The remaining
> > changes are I believe actual bug fixes.
> I doubt it's intentional.  I'd tend to think this specific patch in the
> series should wait until gcc-12 out of an abundance of caution.

 Makes sense to me.  Since we have a working instance of patchwork and I 
actively use it for patch management (for own patches anyway, as I can't 
update the status of other people's patches) all I need to do is to just 
get back to it once we reopen for GCC 12 and see what's been outstanding 
there.

 Thanks for your review.

  Maciej
Maciej W. Rozycki April 27, 2021, 6:05 p.m. UTC | #4
On Tue, 8 Dec 2020, Maciej W. Rozycki wrote:

> On Thu, 3 Dec 2020, Jeff Law wrote:
> 
> > >  Note that I have included unrelated though contextually connected 6/6 as
> > > an RFC to verify whether this potential anomaly I have spotted has been
> > > intentional.  I'll be happy to drop it if that is the case.  The remaining
> > > changes are I believe actual bug fixes.
> > I doubt it's intentional.? I'd tend to think this specific patch in the
> > series should wait until gcc-12 out of an abundance of caution.
> 
>  Makes sense to me.  Since we have a working instance of patchwork and I 
> actively use it for patch management (for own patches anyway, as I can't 
> update the status of other people's patches) all I need to do is to just 
> get back to it once we reopen for GCC 12 and see what's been outstanding 
> there.

 With GCC 11 out of the door I have committed this change now.

 Thank you, Jeff, for your review again!

  Maciej