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
