On Wed, 2014-08-13 at 20:55 -0600, Jeff Law wrote:
> On 08/13/14 18:11, David Malcolm wrote:
> > On Wed, 2014-08-13 at 14:34 -0600, Jeff Law wrote:
> >> On 08/13/14 14:28, David Malcolm wrote:
> >>> Thanks. Although this function gets converted back to a macro in patch
> >>> 191, I just realized that in the meantime that it's not inlined, as is
> >>> the case for some of the other macro->function conversions in patches
> >>> 13-16.
> >>>
> >>> Do I need to convert them to inline functions with the appropriate
> >>> headers, and is that regarded as a sufficiently trivial fix to the stuff
> >>> you've already reviewed to not need re-review? (I will bootstrap&test).
> >> I'd just make it a follow-up. #237 ;-)
> >
> > Right, but these would be modifications to stuff that only exists
> > between phases 1-3 of the kit, before going away in phase 4, so although
> > it might be #237, it would need to be applied when the individual
> > changes go in.
> If they're around just through phases #1-#3, then I wouldn't worry about
> it.
...and indeed, having experimented with it, the inline function approach
would need to include more header files: for example, an inline
implementation of, say, BB_HEAD() in basic-block.h would look like this:
inline rtx_insn *BB_HEAD (const_basic_block bb)
{
rtx insn = bb->il.x.head_;
return safe_as_a <rtx_insn *> (insn);
}
but this requires everything including basic-block.h to know about
safe_as_a <rtx_insn *> (rtx) and hence have access to
is_a_helper <rtx_insn *> (rtx), and hence needs to include rtl.h i.e.
either basic-block.h would need to include rtl.h, or everything
including it would. Similar considerations apply to the macros in the
various other header files. Touching the header file graph doesn't seem
like a good thing to be doing for a transient effort during phases 1-3
of applying the kit, so I'll hold off on making that change, and go with
the patches as-is.
> > Transient, yes, but given the amount of time for me to simply bootstrap
> > each candidate patch, the non-inlined functions could last in trunk for
> > a couple of weeks (there are only 168 hours in a week, and a bootstrap
> > +regrtest takes about 3 hours on my box, so for 236 patches we're
> > talking ~4 weeks of compute time just for that).
> Well, I'd suggest a few things.
>
> 1. For the config/ changes, a full bootstrap is not necessary. For
> those targets which are embedded, just build a stage1 cross to the
> target to verify it builds and call it good.
>
> 2. For targets where you can bootstrap, go ahead and do so, but just on
> those targets. Some of these you can probably have going in parallel.
>
> 3. Some of the changes are so trivial that I'd squash them together in a
> single build/test cycle.
Thanks.
FWIW I'm working on followup cleanups whilst waiting for the
build/tests.
> I would even consider seeing all the scaffolding go in as a single
> chunk. It's nice to see the individuals during the review process, but
> I wouldn't lose any sleep if bundled those together.
>
>
> >
> > I guess the underlying point here is that this is a big change and I'm
> > trying to be fastidious here. Murphy's Law suggests I'm going to break
> > at least *something* :(
> Understood, but we don't want to be so fastidious that the time this
> stuff is in flux is so long that it creates more problems than it would
> if we took some sensible and safe shortcuts.
(nods)
Thanks again for the reviews
Dave
