Hi!
On Tue, May 18, 2021 at 07:43:49PM +0930, Alan Modra wrote:
> On Mon, May 10, 2021 at 04:39:55PM -0500, Segher Boessenkool wrote:
> > Huh, did it not already do that?! Hrm, all the other hooks seem to be
> > called via rs6000.c currently. But you do the same, so why do you need
> > to include the header in rs6000-logue.c?
>
> For the new call to default_print_patchable_function_entry in
> rs6000_output_function_prologue.
I see, thanks.
> > > +{
> > > + /* Always call default_print_patchable_function_entry when RECORD_P in
> > > + order to output the location of the NOPs, but use the size of the
> > > + area before the entry on both possible calls. If RECORD_P is true
> > > + on the second call then the area before the entry was zero size and
> > > + thus no NOPs will be output. */
> > > + if (record_p)
> > > + default_print_patchable_function_entry (file, crtl->patch_area_entry,
> > > + record_p);
> > > +}
> >
> > That is trickiness that will break later.
>
> There is a fine line between trickiness and elegance. Given the
> current available crtl variables dealing with the patch area and the
> current patchable_function_entry parameters, any supposedly "less
> hacky" but correct expression will simplify down to what I wrote
> here.
I wrote a bit later:
> > Can you make this less hacky please? Changing the generic code is just
> > fine as well, it needs some love.
In effect making a callback / hook without making that explicit is bad
for maintainability. We are in stage 1, now is a good time for
infrastructure changes.
Segher
