Re: GCC 5 Status Report (2015-01-19), Trunk in Stage 4

[Richard Biener <rguenther at suse dot de>]

On Tue, 27 Jan 2015, Dominik Vogt wrote:

> 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.

Thanks for the clarification.

Richard.