This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 012/236] Convert DF_REF_INSN to a function for now


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]