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.
