This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
Re: GCC 5 Status Report (2015-01-19), Trunk in Stage 4
- From: Dominik Vogt <vogt at linux dot vnet dot ibm dot com>
- To: gcc at gcc dot gnu dot org
- Cc: Jakub Jelinek <jakub at redhat dot com>, Andreas Krebbel <krebbel at linux dot vnet dot ibm dot com>, Richard Biener <rguenther at suse dot de>
- Date: Tue, 27 Jan 2015 11:04:08 +0100
- Subject: Re: GCC 5 Status Report (2015-01-19), Trunk in Stage 4
- Authentication-results: sourceware.org; auth=none
- References: <alpine dot LSU dot 2 dot 11 dot 1501191016120 dot 12482 at zhemvz dot fhfr dot qr> <20150127085256 dot GA5862 at maggie> <alpine dot LSU dot 2 dot 11 dot 1501271002380 dot 12482 at zhemvz dot fhfr dot qr> <20150127090803 dot GB1746 at tucnak dot redhat dot com> <alpine dot LSU dot 2 dot 11 dot 1501271009410 dot 12482 at zhemvz dot fhfr dot qr>
- Reply-to: vogt at linux dot vnet dot ibm dot com
On Tue, Jan 27, 2015 at 10:12:15AM +0100, Richard Biener wrote:
> On Tue, 27 Jan 2015, Jakub Jelinek wrote:
> > On Tue, Jan 27, 2015 at 10:04:38AM +0100, Richard Biener wrote:
> > > On Tue, 27 Jan 2015, Andreas Krebbel wrote:
> > > > [PATCH] S/390: -mhotpatch v2
> > > > https://gcc.gnu.org/ml/gcc-patches/2015-01/msg02370.html
> > > >
> > > > It is a backend only change to our existing -mhotpatch feature
> > > > requested by the Linux kernel guys for the ftrace implementation:
> > > > https://lkml.org/lkml/2015/1/26/320
> > > >
> > > > They need it in an upstream GCC asap. If we don't get it into 5.0 we
> > > > probably would need to commit it onto 5.1 branch right after the
> > > > release. I would rather try to avoid this since it would make the
> > > > hotpatch feature incompatible between 5.0 and 5.1.
> > > Did you consider using an alternate option name instead of changing
> > > it in an incompatible way? I realize SUSE will need to backport this
> >
> > Yeah, the option incompatibility worries me. Can't -mhotpatch without =
> > stand for the old behavior? Does it map to some -mhotpatch=X,Y value,
> > or is it not worth to support both?
>
> It maps to -mhotpatch=12.
The new expression for the old behaviour would be -mhotpatch=12,2
(or attribute((hotpatch(12,2))), and -mno-notpatch is expressed as
-mhotpatch=0,0 now.
> The old one had one argument while the new
> one has two... so eventually you can distinguish them this way,
> though for the inlining I'd have added -minline-hotpatched and
> if you switch the arguments of the new hotpatch then -mhotpatch=12
> would trivially become supported again...
The reasons for the incompatible changes are:
- With the old code it was really weird that you requested n
halfwords to be patched before the label and silently got two
halfwords patched after the label too.
- The interaction between inlining and hotpatching was really
awkward to code and led to behaviour hard to understand for the
user. Furthermore hotpatching is now compatible with
always_inline (which generated an error in the old code).
- With the simplification to the interaction between inlining and
hotpatching (needed by the kernel), the new code would only look
the same on the command line but actually do something else.
- The old code had already three options (-mhotpatch,
-mno-hotpatch and -mhotpatch=) and adding another one to define
the size to be patched after the label would have made an even
bigger mess of the already bad usability.
- The user who requested that feature does not use it yet and
will get the updated patch with the new semantics before he
starts to use it.
- We believe that nobody has tried to use the old semantics
(except the colleague next door who requested the new
functionality for use in the kernel).
To sum it up, given that we don't expect any current users, making
an incompatible change and thus avoiding the extra hassle in
favour of a clean design seemed acceptable. It would certainly be
possible to keep the interfaces compatible, but we think it is not
worth the effort.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany